From 1c0acb23591ec71db6b28df04f62d0644260979b Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 6 Sep 2026 17:29:54 -0400 Subject: [PATCH] feat(#4422): block merging into next/main while the base branch's Tests run is red (#4428) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(#4422): block merging into next/main while the base branch's Tests run is red Adds a next-health job to test.yml that checks the base branch's own last push-triggered Tests run via the GitHub API and fails the existing "Required tests" required check when it's red, with a maintainer-applied "fix-next" label as the explicit escape hatch for the fix-forward PR itself. No branch-protection config change needed — it rides the already-required check. The job is deliberately not gated behind preflight, same reasoning as the changes job: a compute-free API read has nothing to save by waiting. Documents the fix-next label in CONTRIBUTING.md and adds a property test locking the CLEAN/RED/INDETERMINATE classification's iff-relationship. This closes the second half of the 2026-09-06 RCA: three unrelated PRs merged on top of an already-broken next before anyone noticed it was red. Co-Authored-By: Claude Sonnet 5 * fix: close two zero-margin CI timing gaps found while verifying #4422 Discovered while watching this branch's own CI, root-caused via /diagnose rather than dismissed as Windows flakiness: 1. tests/gsd-check-update-worker-atomic-cache.test.cjs's outer timeout (15000ms) exactly matched the inner npm-view timeout the worker wraps (NPM_VIEW_TIMEOUT_MS, gsd-core/bin/check-latest-version.cjs). A slow registry response raced two SIGKILLs at the same instant, killing the worker before it could catch its own timeout and degrade gracefully. Windows's shell-wrapped npm subprocess made the race lose more often there, but the zero margin was platform-agnostic. Fixed by giving the test real headroom (+10s) beyond the named constant it wraps, plus an invariant test so the two values can't silently collide again. 2. scripts/run-tests.cjs's per-chunk weight budget (MAX_FILES_PER_CHUNK) let a Windows full-matrix chunk that was well under budget by the Linux/macOS-calibrated weight table (~32/60 units) still exceed the 600s wall-clock backstop — codex-config.test.cjs's genuinely-measured weight (17.87) doesn't transfer 1:1 to Windows's slower install/ subprocess overhead. Windows now gets its own lower cap (40 vs 60). Co-Authored-By: Claude Sonnet 5 --------- Co-authored-by: sim Co-authored-by: Claude Sonnet 5 --- .github/workflows/test.yml | 77 +++ CONTRIBUTING.md | 15 + gsd-core/bin/check-latest-version.cjs | 11 +- scripts/ci-next-health.cjs | 271 +++++++++ scripts/run-tests.cjs | 16 +- tests/ci-next-health.test.cjs | 552 ++++++++++++++++++ ...-check-update-worker-atomic-cache.test.cjs | 32 +- 7 files changed, 969 insertions(+), 5 deletions(-) create mode 100644 scripts/ci-next-health.cjs create mode 100644 tests/ci-next-health.test.cjs diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3ea4c174d..b17038f8b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -45,6 +45,72 @@ jobs: contents: read pull-requests: read + # #4422: three unrelated PRs merged on top of an already-broken `next` on + # 2026-09-06 before anyone noticed — nothing checked the base branch's OWN + # health before letting a PR land on it. This job queries the GitHub API for + # the base branch's last push-triggered Tests run and blocks the merge if it + # is red, with a `fix-next` label escape hatch for the PR that is itself the + # fix-forward. See scripts/ci-next-health.cjs's header for the full risk- + # asymmetry reasoning (it deliberately does NOT fail open on a definite red + # signal, unlike the mergeability preflight above). + # + # No `if:` guard, for the same reason `preflight` has none: a SKIPPED + # dependency skips its dependents exactly like a failed one, so guarding this + # job would skip `required-tests` on every push/workflow_dispatch run too. + # scripts/ci-next-health.cjs itself no-ops (zero API calls, exit 0) on any + # event other than pull_request/merge_group. + # + # `next-health` is likewise deliberately NOT gated behind `preflight`, same + # rationale as `changes` above: it is a ~2-minute, compute-free API read that + # runs in parallel with the preflight job, and serializing it behind + # `preflight` would add latency to every healthy PR for no saving. + next-health: + name: Base branch health + runs-on: ubuntu-latest + timeout-minutes: 2 + permissions: + contents: read + actions: read + steps: + - name: Check out the next-health script from the base branch + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + # Same rationale as pr-mergeable-preflight.yml's checkout: pinned to + # the BASE sha so the script that decides the gate comes from a + # trusted commit, never PR-supplied code. Empty on a non-pull_request + # event (e.g. merge_group), where actions/checkout falls back to the + # triggering ref — fine, since the script no-ops before reading + # anything on those events too. + ref: ${{ github.event.pull_request.base.sha }} + fetch-depth: 1 + sparse-checkout: | + scripts + persist-credentials: false + + - name: Check base branch health + id: check + env: + # Every value arrives through env. CONTRIBUTING.md forbids `${{ }}` + # inside a `run:` block. + GITHUB_TOKEN: ${{ github.token }} + PR_LABELS: ${{ join(github.event.pull_request.labels.*.name, ',') }} + MERGE_GROUP_BASE_REF: ${{ github.event.merge_group.base_ref }} + run: | + # Bootstrap arm, mirroring pr-mergeable-preflight.yml's: the checkout + # above is of the BASE sha, so this step runs the script as it exists + # on the base branch — which means it is absent on the PR that + # INTRODUCES it, and on any PR branched from a base predating it. + # Absent is not "red": it is one more thing we cannot determine, so + # it takes the same fail-open path as any other unresolved read. + # Self-healing — once the script is on the base branch this arm never + # fires again. + if [ ! -f scripts/ci-next-health.cjs ]; then + echo "::warning::scripts/ci-next-health.cjs is not present at the base sha; skipping the base-branch health gate (fail-open). Expected on the PR that introduces it, or on a branch whose base predates it." + echo "verdict=INDETERMINATE" >> "$GITHUB_OUTPUT" + exit 0 + fi + node scripts/ci-next-health.cjs + changes: name: Detect test scope runs-on: ubuntu-latest @@ -857,6 +923,7 @@ jobs: name: Required tests needs: - preflight + - next-health - changes - lint-tests - test @@ -871,6 +938,7 @@ jobs: - name: Summarize required test gate env: PREFLIGHT_RESULT: ${{ needs.preflight.result }} + NEXT_HEALTH_RESULT: ${{ needs.next-health.result }} CODE_CHANGED: ${{ needs.changes.outputs.code_changed }} PRODUCT_CHANGED: ${{ needs.changes.outputs.product_changed }} CHANGES_RESULT: ${{ needs.changes.result }} @@ -883,6 +951,7 @@ jobs: run: | set -euo pipefail echo "preflight=$PREFLIGHT_RESULT" + echo "next-health=$NEXT_HEALTH_RESULT" echo "code_changed=$CODE_CHANGED" echo "product_changed=$PRODUCT_CHANGED" echo "changes=$CHANGES_RESULT" @@ -910,6 +979,14 @@ jobs: exit 1 fi + # #4422: applies UNCONDITIONALLY, not nested inside the + # PRODUCT_CHANGED/CODE_CHANGED branches below — a red base branch + # must block every PR, including doc-only ones. + if [ "$NEXT_HEALTH_RESULT" != "success" ]; then + echo "::error::the base branch's own last Tests run is red — see the 'Base branch health' job above for the failing run. Wait for a fix-forward merge, or if this PR IS the fix, ask a maintainer to apply the 'fix-next' label to override." + exit 1 + fi + if [ "$LINT_RESULT" != "success" ]; then echo "::error::lint-tests did not pass" exit 1 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c916ab889..d45e43c00 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1285,6 +1285,21 @@ the pipeline runs exactly as it did before, and the per-job including which lanes are deliberately *not* gated, is in [docs/TESTING-SUITES.md → The mergeability preflight](docs/TESTING-SUITES.md#the-mergeability-preflight). +### A PR cannot merge onto a red base branch + +The `Base branch health` required check queries GitHub for the base branch's +own last push-triggered Tests run and blocks your merge if that run is red — +independent of whether your own PR's changes pass. This needs no +branch-protection reconfiguration: it rides the existing "Required tests" +check, the same status GitHub already requires before merge. + +If your PR is itself the fix-forward and you need to land it while the base +branch is still red, a maintainer applies the `fix-next` label directly to +your PR to explicitly bypass this one check. Applying a label requires +GitHub write access to the repo, so a PR author cannot self-apply it to +bypass the gate — only a maintainer or another collaborator with label-write +permission can. Full decision logic is in `scripts/ci-next-health.cjs`. + ### CI Test Quality Checks The following checks run on every PR in addition to the test suite: diff --git a/gsd-core/bin/check-latest-version.cjs b/gsd-core/bin/check-latest-version.cjs index e00ff58c0..0fcb21085 100755 --- a/gsd-core/bin/check-latest-version.cjs +++ b/gsd-core/bin/check-latest-version.cjs @@ -42,6 +42,12 @@ const SEMVER_RE = /^\d+\.\d+\.\d+(?:[-+][0-9A-Za-z.-]+)?$/; // `npm view` to an empty or foreign dist-tag. const ALLOWED_TAGS = Object.freeze(['latest', 'next']); +// Bounded at 15s so a hung registry doesn't block /gsd-update (#2993 CR). +// Exported so callers (e.g. the worker's own test harness, #4091) can derive +// their own outer timeout with real margin above this inner bound instead of +// re-hardcoding 15000 and silently drifting into a zero-margin race. +const NPM_VIEW_TIMEOUT_MS = 15_000; + /** * Build the `npm view` args for a dist-tag. `latest` keeps the bare package * spec so the default invocation is byte-for-byte identical to before tag @@ -92,9 +98,8 @@ function checkLatestVersion(opts = {}) { // Windows shell-flag policy and timeout default). The injection point // remains spawnSync-shaped for test compatibility — the adapter below // translates { exitCode } → { status } so the consumer logic is unchanged. - // Bounded at 15s so a hung registry doesn't block /gsd-update (#2993 CR). const defaultSpawn = () => { - const r = execNpm(buildViewArgs(tag), { timeout: 15_000 }); + const r = execNpm(buildViewArgs(tag), { timeout: NPM_VIEW_TIMEOUT_MS }); return { status: r.exitCode, stdout: r.stdout, @@ -158,4 +163,4 @@ function main() { if (require.main === module) runMain(main); -module.exports = { checkLatestVersion, CHECK_REASON, PACKAGE_NAME, ALLOWED_TAGS, buildViewArgs, resolveTag }; +module.exports = { checkLatestVersion, CHECK_REASON, PACKAGE_NAME, ALLOWED_TAGS, NPM_VIEW_TIMEOUT_MS, buildViewArgs, resolveTag }; diff --git a/scripts/ci-next-health.cjs b/scripts/ci-next-health.cjs new file mode 100644 index 000000000..03e79dc69 --- /dev/null +++ b/scripts/ci-next-health.cjs @@ -0,0 +1,271 @@ +#!/usr/bin/env node +'use strict'; + +// ci-next-health.cjs — #4422 base-branch health gate. +// +// Risk asymmetry drives this design, same shape as scripts/ci-pr-mergeability.cjs +// but pointed the other direction: on 2026-09-06 three unrelated PRs merged on +// top of an already-broken `next` before anyone noticed, because nothing checked +// the base branch's OWN health before letting a PR land on it. A false positive +// here (a healthy `next` wrongly reported RED) blocks every PR merge in the repo +// until a human notices and applies the `fix-next` bypass label — annoying, but +// loud and immediately actionable. A false negative (a broken `next` reported +// healthy) reproduces the #4422 incident exactly. So, unlike the mergeability +// preflight, this gate does NOT fail open on a definite red signal — it fails +// open only when the signal itself is unavailable or inapplicable (wrong event, +// no resolvable base ref, an API read that throws). Once GitHub actually answers +// with a non-success conclusion for the base branch's last push-triggered Tests +// run, that is treated as authoritative and blocks — with one explicit, visible, +// human-operated escape hatch (the `fix-next` label) for the PR that is itself +// the fix-forward. +// +// Every non-success conclusion (`failure`, `cancelled`, `timed_out`, +// `action_required`, ...) is treated as RED, not just `failure` — a cancelled or +// timed-out run on the base branch is not evidence the branch is healthy, it is +// evidence nobody knows yet. + +const { ExitError, runMain } = require('./lib/cli-exit.cjs'); + +const VERDICT = Object.freeze({ + CLEAN: 'CLEAN', + RED: 'RED', + BYPASSED: 'BYPASSED', + SKIPPED_NOT_APPLICABLE: 'SKIPPED_NOT_APPLICABLE', + INDETERMINATE: 'INDETERMINATE', +}); + +const APPLICABLE_EVENTS = new Set(['pull_request', 'merge_group']); +const BYPASS_LABEL = 'fix-next'; +const TESTS_WORKFLOW_FILE = 'test.yml'; + +/** + * Pure, total classifier: the GitHub "list workflow runs" response payload -> + * CLEAN | RED | INDETERMINATE. Never throws. + * + * - Not an object, or `workflow_runs` isn't an array -> INDETERMINATE: the + * shape we depend on is not present, so nothing can be concluded. + * - No completed push runs found yet (e.g. a brand-new `release/**`/`hotfix/**` + * branch with no Tests history) -> CLEAN: absence of evidence of red is not + * evidence of red, and a brand-new branch must not be permanently unmergeable. + * - The most recent run's `conclusion === 'success'` -> CLEAN. + * - Anything else (failure, cancelled, timed_out, action_required, ...) -> RED. + */ +function classifyRunConclusion(payload) { + if (payload === null || typeof payload !== 'object') return VERDICT.INDETERMINATE; + if (!Array.isArray(payload.workflow_runs)) return VERDICT.INDETERMINATE; + if (payload.workflow_runs.length === 0) return VERDICT.CLEAN; + const [latest] = payload.workflow_runs; + if (latest && latest.conclusion === 'success') return VERDICT.CLEAN; + return VERDICT.RED; +} + +/** + * Dependency-injected orchestrator. `fetchLatestRun` is an async function + * taking the resolved base branch name and returning the parsed API payload + * (or throwing) — injected so tests never touch the network. + * + * @returns {Promise<{verdict:string, payload:*, reason:string}>} + */ +async function resolveNextHealth({ fetchLatestRun, eventName, baseRef, labels } = {}) { + if (!APPLICABLE_EVENTS.has(eventName)) { + return { verdict: VERDICT.SKIPPED_NOT_APPLICABLE, payload: null, reason: 'not-applicable-event' }; + } + + if (typeof baseRef !== 'string' || baseRef.trim() === '') { + return { verdict: VERDICT.INDETERMINATE, payload: null, reason: 'no-base-ref' }; + } + + let payload = null; + try { + payload = await fetchLatestRun(baseRef); + } catch (err) { + return { verdict: VERDICT.INDETERMINATE, payload: null, reason: `fetch-failed: ${err.message}` }; + } + + const classified = classifyRunConclusion(payload); + if (classified !== VERDICT.RED) { + return { verdict: classified, payload, reason: 'resolved' }; + } + + // Escape hatch. `labels` is empty/absent for merge_group — that event has no + // label surface today, which means a queued merge-group commit cannot use + // this bypass. Known gap, not solved here: a maintainer must land the + // fix-forward as a direct pull_request merge (where the label IS readable) + // rather than through the merge queue, until GitHub exposes an equivalent + // signal for merge_group. + const labelList = Array.isArray(labels) ? labels : []; + if (labelList.includes(BYPASS_LABEL)) { + return { verdict: VERDICT.BYPASSED, payload, reason: 'bypass-label' }; + } + + return { verdict: VERDICT.RED, payload, reason: 'red' }; +} + +function usage() { + return [ + 'Usage:', + ' node scripts/ci-next-health.cjs', + '', + 'CI-only gate: checks whether the PR/merge-group\'s base branch\'s own last', + 'push-triggered Tests run is red, and fails the job (exit 1) when it is —', + 'unless the PR carries the "fix-next" bypass label. Every other case', + '(wrong event, no resolvable base ref, an unreadable API read) fails open', + '(exit 0).', + '', + 'Environment variables read:', + ' GITHUB_EVENT_NAME workflow trigger event; only "pull_request" and', + ' "merge_group" are checked', + ' GITHUB_REPOSITORY owner/repo', + ' GITHUB_TOKEN optional bearer token for the API read', + ' GITHUB_BASE_REF PR base branch name (pull_request events; set', + ' automatically by GitHub Actions)', + ' MERGE_GROUP_BASE_REF merge-group base ref (merge_group events; the', + ' caller workflow must populate this from', + ' github.event.merge_group.base_ref — a leading', + ' "refs/heads/" prefix is stripped if present)', + ' GITHUB_API_URL GitHub API base URL (default: https://api.github.com)', + ' PR_LABELS comma-separated PR label names (pull_request events;', + ' the caller workflow must populate this from', + ' github.event.pull_request.labels.*.name); checked', + ' for the literal "fix-next" bypass label', + ' GITHUB_OUTPUT path to append verdict= step output to', + ' GITHUB_STEP_SUMMARY path to append a human-readable summary to', + ].join('\n'); +} + +function parseArgs(argv) { + for (const arg of argv) { + if (arg === '--help' || arg === '-h') { + process.stdout.write(`${usage()}\n`); + throw new ExitError(0); + } + throw new Error(`unknown argument: ${arg}`); + } +} + +function writeOutput(lines) { + const outputPath = process.env.GITHUB_OUTPUT; + if (typeof outputPath !== 'string' || outputPath === '') return; + try { + const fs = require('node:fs'); + fs.appendFileSync(outputPath, `${lines.join('\n')}\n`); + } catch (err) { + // A failure while REPORTING the verdict must never invert the gate. + process.stderr.write(`::warning::failed to write GITHUB_OUTPUT: ${err.message}\n`); + } +} + +function writeSummary(text) { + const summaryPath = process.env.GITHUB_STEP_SUMMARY; + if (typeof summaryPath !== 'string' || summaryPath === '') return; + try { + const fs = require('node:fs'); + fs.appendFileSync(summaryPath, `${text}\n`); + } catch (err) { + process.stderr.write(`::warning::failed to write GITHUB_STEP_SUMMARY: ${err.message}\n`); + } +} + +function resolveBaseRef(eventName) { + if (eventName === 'pull_request') { + return process.env.GITHUB_BASE_REF || ''; + } + if (eventName === 'merge_group') { + const raw = process.env.MERGE_GROUP_BASE_REF || ''; + return raw.startsWith('refs/heads/') ? raw.slice('refs/heads/'.length) : raw; + } + return ''; +} + +function parseLabels(raw) { + if (typeof raw !== 'string' || raw.trim() === '') return []; + return raw.split(',').map((s) => s.trim()).filter((s) => s !== ''); +} + +function runUrlOf(payload) { + const run = payload && Array.isArray(payload.workflow_runs) ? payload.workflow_runs[0] : undefined; + return run && typeof run.html_url === 'string' ? run.html_url : '(unknown run URL)'; +} + +// `argv` defaults to real CLI argv but is a parameter so tests can call +// main() in-process (e.g. main([])) without inheriting the test runner's own +// argv, which would otherwise trip parseArgs's "unknown argument" branch. +async function main(argv = process.argv.slice(2)) { + parseArgs(argv); + + const eventName = process.env.GITHUB_EVENT_NAME; + const repo = process.env.GITHUB_REPOSITORY || ''; + const token = process.env.GITHUB_TOKEN || ''; + const apiBase = process.env.GITHUB_API_URL || 'https://api.github.com'; + const baseRef = resolveBaseRef(eventName); + const labels = parseLabels(process.env.PR_LABELS); + + const fetchLatestRun = async (branch) => { + const url = `${apiBase}/repos/${repo}/actions/workflows/${TESTS_WORKFLOW_FILE}/runs` + + `?branch=${encodeURIComponent(branch)}&event=push&status=completed&per_page=1`; + const headers = { + accept: 'application/vnd.github+json', + 'x-github-api-version': '2022-11-28', + 'user-agent': 'gsd-core-ci-next-health', + }; + if (token) headers.authorization = `Bearer ${token}`; + const response = await fetch(url, { headers, signal: AbortSignal.timeout(10000) }); + if (!response.ok) { + throw new Error(`GitHub API responded ${response.status}`); + } + const text = await response.text(); + try { + return JSON.parse(text); + } catch { + return null; + } + }; + + const result = await resolveNextHealth({ fetchLatestRun, eventName, baseRef, labels }); + + writeOutput([`verdict=${result.verdict}`]); + writeSummary(`Base branch health: ${result.verdict}`); + + if (result.verdict === VERDICT.RED) { + const runUrl = runUrlOf(result.payload); + process.stderr.write( + `::error::the base branch "${baseRef}"'s own last Tests run is red: ${runUrl} — ` + + 'wait for a fix-forward merge, or if THIS pull request is the fix, ask a ' + + `maintainer to apply the "${BYPASS_LABEL}" label to explicitly override this gate.\n`, + ); + return 1; + } + + if (result.verdict === VERDICT.BYPASSED) { + const runUrl = runUrlOf(result.payload); + process.stderr.write( + `::warning::the base branch "${baseRef}"'s own last Tests run is red: ${runUrl} — ` + + `a human applied the "${BYPASS_LABEL}" label to explicitly override this gate.\n`, + ); + return 0; + } + + if (result.verdict === VERDICT.INDETERMINATE) { + process.stderr.write( + `::warning::could not determine "${baseRef || '(no base ref)'}"'s Tests health (${result.reason}); ` + + 'proceeding (fail-open).\n', + ); + return 0; + } + + process.stdout.write(`Base branch health: ${result.verdict}\n`); + return 0; +} + +if (require.main === module) { + runMain(main); +} + +module.exports = { + VERDICT, + BYPASS_LABEL, + TESTS_WORKFLOW_FILE, + classifyRunConclusion, + resolveNextHealth, + main, +}; diff --git a/scripts/run-tests.cjs b/scripts/run-tests.cjs index bfdadf75c..092207a38 100644 --- a/scripts/run-tests.cjs +++ b/scripts/run-tests.cjs @@ -1273,7 +1273,21 @@ function main() { // node process (also relieving per-process memory pressure from 170+ files at once). // Lowered from 90 to 60 after #1575 — macOS Node 22 shard 2/3 chunk 2 (~80 files // including state.test.cjs, perf-*, worktree-cleanup) exceeded 600s with 90. - const MAX_FILES_PER_CHUNK = positiveNumberEnv(process.env.RUN_TESTS_MAX_FILES_PER_CHUNK, 60); + // + // 2026-09-06 (PR #4428 CI): a Windows full-matrix chunk (chunk 3/6, ~32/60 + // weight-budget units, dominated by codex-config.test.cjs at a genuinely + // MEASURED weight of 17.87 — not a stale-table miss) still exceeded the + // 600s per-chunk backstop. The weight table's calibration does not + // transfer 1:1 to the Windows runner for install/subprocess-heavy work — + // it needs a smaller budget than Linux/macOS to stay inside the same + // wall-clock ceiling. Windows gets its own, lower cap (~33% reduction, + // proportionate to the >30% single-file share codex-config.test.cjs alone + // consumed of that chunk's budget); other platforms are unaffected. + const DEFAULT_MAX_FILES_PER_CHUNK = process.platform === 'win32' ? 40 : 60; + const MAX_FILES_PER_CHUNK = positiveNumberEnv( + process.env.RUN_TESTS_MAX_FILES_PER_CHUNK, + DEFAULT_MAX_FILES_PER_CHUNK, + ); // #2088 established that file COUNT is a poor proxy for a chunk's wall-clock: // install-heavy files (real installs) cost ~10x a unit file, and when several // land in the SAME chunk it blows the 600s backstop while unit-only chunks diff --git a/tests/ci-next-health.test.cjs b/tests/ci-next-health.test.cjs new file mode 100644 index 000000000..a71ca7619 --- /dev/null +++ b/tests/ci-next-health.test.cjs @@ -0,0 +1,552 @@ +'use strict'; + +// #4422 — base-branch health gate. +// +// Risk asymmetry drives this suite (see scripts/ci-next-health.cjs's header): +// false positive (a healthy `next` called RED) => blocks every PR merge +// until a human notices and applies the `fix-next` bypass label. +// false negative (a broken `next` called healthy) => reproduces the #4422 +// incident exactly (three unrelated PRs merged on top of an already-broken +// `next`). +// Unlike the mergeability preflight, this gate does NOT fail open on a +// definite RED signal — only on an unavailable/inapplicable one. + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const http = require('node:http'); +const fc = require('fast-check'); + +const { createTempDir, cleanup } = require('./helpers.cjs'); +const { runNode } = require('./helpers/process-seam.cjs'); +const { PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); + +const ROOT = path.join(__dirname, '..'); +const SCRIPT = path.join(ROOT, 'scripts', 'ci-next-health.cjs'); + +const { + VERDICT, + BYPASS_LABEL, + classifyRunConclusion, + resolveNextHealth, + main, +} = require('../scripts/ci-next-health.cjs'); + +// --------------------------------------------------------------------------- +// Fakes. The seams are parameters, not module patches — no global state, so +// every case is order-independent. +// --------------------------------------------------------------------------- + +/** A fetchLatestRun fake that replays a scripted response; an Error is thrown. */ +function scriptedFetch(response) { + const calls = []; + const fn = async (branch) => { + calls.push(branch); + if (response instanceof Error) throw response; + return response; + }; + fn.calls = calls; + return fn; +} + +function runPayload(conclusion, htmlUrl = 'https://github.com/open-gsd/gsd-core/actions/runs/1') { + return { workflow_runs: [{ conclusion, html_url: htmlUrl }] }; +} + +// --------------------------------------------------------------------------- +// A. classifyRunConclusion — pure +// --------------------------------------------------------------------------- + +describe('ci-next-health: classifyRunConclusion', () => { + test('classifies an undefined payload as INDETERMINATE', () => { + assert.equal(classifyRunConclusion(undefined), VERDICT.INDETERMINATE); + }); + + test('classifies a null payload as INDETERMINATE', () => { + assert.equal(classifyRunConclusion(null), VERDICT.INDETERMINATE); + }); + + test('classifies a non-object payload as INDETERMINATE', () => { + for (const value of [0, 'str', true, 42]) { + assert.equal(classifyRunConclusion(value), VERDICT.INDETERMINATE); + } + }); + + test('classifies a payload whose workflow_runs is not an array as INDETERMINATE', () => { + for (const value of [undefined, null, 'x', {}, 1]) { + assert.equal(classifyRunConclusion({ workflow_runs: value }), VERDICT.INDETERMINATE); + } + }); + + test('classifies an empty workflow_runs array as CLEAN', () => { + // A brand-new release/**/hotfix/** branch with no Tests history yet must + // not be permanently unmergeable. + assert.equal(classifyRunConclusion({ workflow_runs: [] }), VERDICT.CLEAN); + }); + + test('classifies conclusion: success as CLEAN', () => { + assert.equal(classifyRunConclusion(runPayload('success')), VERDICT.CLEAN); + }); + + test('classifies conclusion: failure as RED', () => { + assert.equal(classifyRunConclusion(runPayload('failure')), VERDICT.RED); + }); + + test('classifies conclusion: cancelled as RED', () => { + // A cancelled/timed-out run on the base branch is not evidence of health. + assert.equal(classifyRunConclusion(runPayload('cancelled')), VERDICT.RED); + }); + + test('classifies every non-success conclusion as RED', () => { + for (const conclusion of ['timed_out', 'action_required', 'neutral', 'skipped', 'stale']) { + assert.equal( + classifyRunConclusion(runPayload(conclusion)), + VERDICT.RED, + `conclusion=${conclusion} must be RED`, + ); + } + }); + + test('only reads the FIRST run in workflow_runs', () => { + const payload = { workflow_runs: [{ conclusion: 'success' }, { conclusion: 'failure' }] }; + assert.equal(classifyRunConclusion(payload), VERDICT.CLEAN); + }); + + test('VERDICT is frozen and its atom set is locked', () => { + assert.ok(Object.isFrozen(VERDICT)); + assert.deepEqual( + Object.keys(VERDICT).sort(), + ['BYPASSED', 'CLEAN', 'INDETERMINATE', 'RED', 'SKIPPED_NOT_APPLICABLE'], + ); + }); + + // A well-formed payload has an ARRAY workflow_runs of length 0 or 1, whose + // sole element (if present) carries an arbitrary `conclusion` string. A + // malformed payload is anything else: a non-object payload, or an object + // whose `workflow_runs` is not an array. + const wellFormedPayloadArb = fc.record({ + workflow_runs: fc.oneof( + fc.constant([]), + fc.tuple(fc.record({ conclusion: fc.string() })), + ), + }); + + const malformedNonArrayRunsArb = fc.oneof( + fc.constant(undefined), + fc.constant(null), + fc.string(), + fc.integer(), + fc.boolean(), + fc.dictionary(fc.string(), fc.string()), + ); + + const malformedPayloadArb = fc.oneof( + fc.constant(null), + fc.constant(undefined), + fc.string(), + fc.integer(), + fc.boolean(), + fc.record({ workflow_runs: malformedNonArrayRunsArb }), + ); + + const payloadArb = fc.oneof(wellFormedPayloadArb, malformedPayloadArb); + + test('CLEAN iff (workflow_runs is empty) or (first run succeeded); everything else RED or INDETERMINATE (property)', () => { + fc.assert( + fc.property(payloadArb, (payload) => { + const verdict = classifyRunConclusion(payload); + + const isObject = payload !== null && typeof payload === 'object'; + const hasArrayRuns = isObject && Array.isArray(payload.workflow_runs); + + if (!hasArrayRuns) { + return verdict === VERDICT.INDETERMINATE; + } + + const isClean = payload.workflow_runs.length === 0 + || payload.workflow_runs[0].conclusion === 'success'; + + if (isClean) return verdict === VERDICT.CLEAN; + return verdict === VERDICT.RED; + }), + { seed: 4422, numRuns: 500, verbose: true }, + ); + }); +}); + +// --------------------------------------------------------------------------- +// B. resolveNextHealth — dependency-injected orchestrator +// --------------------------------------------------------------------------- + +describe('ci-next-health: resolveNextHealth', () => { + test('a push event is SKIPPED_NOT_APPLICABLE without calling fetchLatestRun', async () => { + const fetchLatestRun = scriptedFetch(runPayload('failure')); + const result = await resolveNextHealth({ fetchLatestRun, eventName: 'push', baseRef: 'next', labels: [] }); + assert.equal(result.verdict, VERDICT.SKIPPED_NOT_APPLICABLE); + assert.equal(fetchLatestRun.calls.length, 0); + }); + + test('workflow_dispatch is SKIPPED_NOT_APPLICABLE without calling fetchLatestRun', async () => { + const fetchLatestRun = scriptedFetch(runPayload('failure')); + const result = await resolveNextHealth({ + fetchLatestRun, eventName: 'workflow_dispatch', baseRef: 'next', labels: [], + }); + assert.equal(result.verdict, VERDICT.SKIPPED_NOT_APPLICABLE); + assert.equal(fetchLatestRun.calls.length, 0); + }); + + test('an unknown/absent event name is SKIPPED_NOT_APPLICABLE without calling', async () => { + for (const eventName of ['', undefined, 'schedule', 'release', 'issues']) { + const fetchLatestRun = scriptedFetch(runPayload('failure')); + const result = await resolveNextHealth({ fetchLatestRun, eventName, baseRef: 'next', labels: [] }); + assert.equal(result.verdict, VERDICT.SKIPPED_NOT_APPLICABLE); + assert.equal(fetchLatestRun.calls.length, 0); + } + }); + + test('a pull_request event with no resolvable base ref is INDETERMINATE', async () => { + for (const baseRef of [undefined, null, '', ' ']) { + const fetchLatestRun = scriptedFetch(runPayload('failure')); + const result = await resolveNextHealth({ fetchLatestRun, eventName: 'pull_request', baseRef, labels: [] }); + assert.equal(result.verdict, VERDICT.INDETERMINATE); + assert.equal(fetchLatestRun.calls.length, 0); + } + }); + + test('a merge_group event with no resolvable base ref is INDETERMINATE', async () => { + const fetchLatestRun = scriptedFetch(runPayload('failure')); + const result = await resolveNextHealth({ fetchLatestRun, eventName: 'merge_group', baseRef: '', labels: [] }); + assert.equal(result.verdict, VERDICT.INDETERMINATE); + assert.equal(fetchLatestRun.calls.length, 0); + }); + + test('a throwing fetchLatestRun degrades to INDETERMINATE', async () => { + const fetchLatestRun = scriptedFetch(new Error('ECONNRESET')); + const result = await resolveNextHealth({ fetchLatestRun, eventName: 'pull_request', baseRef: 'next', labels: [] }); + assert.equal(result.verdict, VERDICT.INDETERMINATE); + }); + + test('a RED verdict with the fix-next label is BYPASSED', async () => { + const fetchLatestRun = scriptedFetch(runPayload('failure')); + const result = await resolveNextHealth({ + fetchLatestRun, eventName: 'pull_request', baseRef: 'next', labels: ['needs-triage', BYPASS_LABEL], + }); + assert.equal(result.verdict, VERDICT.BYPASSED); + }); + + test('a RED verdict with no matching label stays RED', async () => { + const fetchLatestRun = scriptedFetch(runPayload('failure')); + const result = await resolveNextHealth({ + fetchLatestRun, eventName: 'pull_request', baseRef: 'next', labels: ['needs-triage'], + }); + assert.equal(result.verdict, VERDICT.RED); + }); + + test('a RED verdict with an absent labels array stays RED (merge_group has no label surface)', async () => { + const fetchLatestRun = scriptedFetch(runPayload('failure')); + const result = await resolveNextHealth({ + fetchLatestRun, eventName: 'merge_group', baseRef: 'next', labels: undefined, + }); + assert.equal(result.verdict, VERDICT.RED); + }); + + test('a CLEAN verdict (success conclusion) is not affected by labels', async () => { + const fetchLatestRun = scriptedFetch(runPayload('success')); + const result = await resolveNextHealth({ + fetchLatestRun, eventName: 'pull_request', baseRef: 'next', labels: [BYPASS_LABEL], + }); + assert.equal(result.verdict, VERDICT.CLEAN); + }); + + test('a CLEAN verdict (empty workflow_runs array)', async () => { + const fetchLatestRun = scriptedFetch({ workflow_runs: [] }); + const result = await resolveNextHealth({ + fetchLatestRun, eventName: 'pull_request', baseRef: 'release/1.0', labels: [], + }); + assert.equal(result.verdict, VERDICT.CLEAN); + }); + + test('merge_group resolves the same way as pull_request given a base ref', async () => { + const fetchLatestRun = scriptedFetch(runPayload('success')); + const result = await resolveNextHealth({ + fetchLatestRun, eventName: 'merge_group', baseRef: 'next', labels: [], + }); + assert.equal(result.verdict, VERDICT.CLEAN); + assert.deepEqual(fetchLatestRun.calls, ['next']); + }); +}); + +// --------------------------------------------------------------------------- +// C. main() — integration through the process seam against a real local API. +// +// The CLI's real fetch path is exercised by pointing GITHUB_API_URL at a +// throwaway localhost server, so no production test-mode branch exists and +// nothing reaches api.github.com. +// --------------------------------------------------------------------------- + +const SENTINEL_TOKEN = 'ghs_sentinel_must_never_be_echoed_4422'; + +/** Start a one-shot API stub. `handler(requestCount)` returns { status, body }. */ +async function startApiStub(handler) { + let requestCount = 0; + const server = http.createServer((req, res) => { + const { status, body } = handler(requestCount++); + res.writeHead(status, { 'content-type': 'application/json' }); + res.end(typeof body === 'string' ? body : JSON.stringify(body)); + }); + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + const { port } = server.address(); + return { + url: `http://127.0.0.1:${port}`, + close: () => new Promise((resolve) => server.close(resolve)), + get requestCount() { return requestCount; }, + }; +} + +function runCli(env, { outputPath } = {}) { + return runNode([SCRIPT], { + cwd: ROOT, + timeoutMs: PROBE_TIMEOUT_MS, + env: { + ...process.env, + GITHUB_REPOSITORY: 'open-gsd/gsd-core', + GITHUB_TOKEN: SENTINEL_TOKEN, + GITHUB_BASE_REF: 'next', + ...(outputPath ? { GITHUB_OUTPUT: outputPath } : {}), + ...env, + }, + }); +} + +// `runNode` (tests/helpers/process-seam.cjs) is spawnSync: it blocks this +// process's event loop for the whole child lifetime. `startApiStub` is an +// http.createServer living in THIS process, so while spawnSync blocks, the +// stub can never accept the child's connection. Any test that needs the stub +// must instead call main() in-process, which keeps this process's event loop +// live so the stub can actually answer. Mirrors tests/ci-pr-mergeability.test.cjs. +async function callMain(t, env, { outputPath } = {}) { + const overrides = { + GITHUB_REPOSITORY: 'open-gsd/gsd-core', + GITHUB_TOKEN: SENTINEL_TOKEN, + GITHUB_BASE_REF: 'next', + ...(outputPath ? { GITHUB_OUTPUT: outputPath } : {}), + ...env, + }; + const saved = new Map(); + for (const key of Object.keys(overrides)) saved.set(key, process.env[key]); + Object.assign(process.env, overrides); + t.after(() => { + for (const [key, value] of saved) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + }); + + const stdout = []; + const stderr = []; + t.mock.method(process.stdout, 'write', (chunk) => { stdout.push(String(chunk)); return true; }); + t.mock.method(process.stderr, 'write', (chunk) => { stderr.push(String(chunk)); return true; }); + + const code = await main([]); + return { code, stdout: stdout.join(''), stderr: stderr.join('') }; +} + +function readOutputs(outputPath) { + const raw = fs.readFileSync(outputPath, 'utf8'); + const outputs = {}; + for (const line of raw.split(/\r?\n/)) { + const index = line.indexOf('='); + if (index > 0) outputs[line.slice(0, index)] = line.slice(index + 1); + } + return outputs; +} + +describe('ci-next-health: CLI', () => { + test('exits 0 and writes a skip verdict on a push event', (t) => { + const dir = createTempDir('next-health-skip-'); + t.after(() => cleanup(dir)); + const outputPath = path.join(dir, 'gh-output'); + + const result = runCli({ GITHUB_EVENT_NAME: 'push' }, { outputPath }); + + assert.equal(result.exitCode, 0, result.stderr); + assert.equal(readOutputs(outputPath).verdict, VERDICT.SKIPPED_NOT_APPLICABLE); + }); + + test('exits 1 and annotates the failing run on a RED base branch', async (t) => { + const dir = createTempDir('next-health-red-'); + const api = await startApiStub(() => ({ + status: 200, + body: { workflow_runs: [{ conclusion: 'failure', html_url: 'https://example.test/runs/99' }] }, + })); + t.after(async () => { await api.close(); cleanup(dir); }); + const outputPath = path.join(dir, 'gh-output'); + + const result = await callMain( + t, + { GITHUB_EVENT_NAME: 'pull_request', GITHUB_API_URL: api.url, PR_LABELS: '' }, + { outputPath }, + ); + + assert.equal(result.code, 1, `stdout: ${result.stdout}\nstderr: ${result.stderr}`); + const combined = `${result.stdout}${result.stderr}`; + assert.ok(combined.includes('::error::'), 'must emit a workflow error annotation'); + assert.match(combined, /https:\/\/example\.test\/runs\/99/, 'must name the failing run URL'); + assert.equal(readOutputs(outputPath).verdict, VERDICT.RED); + }); + + test('exits 0 with a warning when the fix-next label bypasses a RED base branch', async (t) => { + const dir = createTempDir('next-health-bypass-'); + const api = await startApiStub(() => ({ + status: 200, + body: { workflow_runs: [{ conclusion: 'failure', html_url: 'https://example.test/runs/100' }] }, + })); + t.after(async () => { await api.close(); cleanup(dir); }); + const outputPath = path.join(dir, 'gh-output'); + + const result = await callMain( + t, + { GITHUB_EVENT_NAME: 'pull_request', GITHUB_API_URL: api.url, PR_LABELS: `needs-triage,${BYPASS_LABEL}` }, + { outputPath }, + ); + + assert.equal(result.code, 0, `stdout: ${result.stdout}\nstderr: ${result.stderr}`); + const combined = `${result.stdout}${result.stderr}`; + assert.ok(combined.includes('::warning::'), 'must warn that a human bypassed the gate'); + assert.match(combined, /https:\/\/example\.test\/runs\/100/, 'the warning must name the failing run URL'); + assert.ok(!combined.includes('::error::'), 'a bypassed gate must not also emit an error annotation'); + assert.equal(readOutputs(outputPath).verdict, VERDICT.BYPASSED); + }); + + test('exits 0 on a clean base branch', async (t) => { + const dir = createTempDir('next-health-clean-'); + const api = await startApiStub(() => ({ status: 200, body: { workflow_runs: [{ conclusion: 'success' }] } })); + t.after(async () => { await api.close(); cleanup(dir); }); + const outputPath = path.join(dir, 'gh-output'); + + const result = await callMain( + t, + { GITHUB_EVENT_NAME: 'pull_request', GITHUB_API_URL: api.url, PR_LABELS: '' }, + { outputPath }, + ); + + assert.equal(result.code, 0, result.stderr); + assert.equal(readOutputs(outputPath).verdict, VERDICT.CLEAN); + }); + + test('resolves the merge_group base ref from MERGE_GROUP_BASE_REF, stripping refs/heads/', async (t) => { + const dir = createTempDir('next-health-mergequeue-'); + const api = await startApiStub(() => ({ status: 200, body: { workflow_runs: [{ conclusion: 'success' }] } })); + t.after(async () => { await api.close(); cleanup(dir); }); + const outputPath = path.join(dir, 'gh-output'); + + const result = await callMain( + t, + { GITHUB_EVENT_NAME: 'merge_group', MERGE_GROUP_BASE_REF: 'refs/heads/next', GITHUB_API_URL: api.url }, + { outputPath }, + ); + + assert.equal(result.code, 0, result.stderr); + assert.equal(readOutputs(outputPath).verdict, VERDICT.CLEAN); + }); + + test('fails open when the API rejects the read', async (t) => { + const dir = createTempDir('next-health-403-'); + const api = await startApiStub(() => ({ status: 403, body: { message: 'Resource not accessible' } })); + t.after(async () => { await api.close(); cleanup(dir); }); + const outputPath = path.join(dir, 'gh-output'); + + const result = await callMain( + t, + { GITHUB_EVENT_NAME: 'pull_request', GITHUB_API_URL: api.url }, + { outputPath }, + ); + + assert.equal(result.code, 0, 'an unreadable base branch is not a red base branch'); + assert.equal(readOutputs(outputPath).verdict, VERDICT.INDETERMINATE); + }); + + test('fails open when the API body is not a JSON object', async (t) => { + const dir = createTempDir('next-health-badjson-'); + const api = await startApiStub(() => ({ status: 200, body: 'not json at all' })); + t.after(async () => { await api.close(); cleanup(dir); }); + const outputPath = path.join(dir, 'gh-output'); + + const result = await callMain( + t, + { GITHUB_EVENT_NAME: 'pull_request', GITHUB_API_URL: api.url }, + { outputPath }, + ); + + assert.equal(result.code, 0); + assert.equal(readOutputs(outputPath).verdict, VERDICT.INDETERMINATE); + }); + + test('works when GITHUB_OUTPUT is not set', (t) => { + const dir = createTempDir('next-health-nooutput-'); + t.after(() => cleanup(dir)); + + const result = runCli({ GITHUB_EVENT_NAME: 'push', GITHUB_OUTPUT: '' }); + + assert.equal(result.exitCode, 0, result.stderr); + }); + + test('an unwritable GITHUB_OUTPUT does not change the exit code', async (t) => { + // A failure while REPORTING the verdict must never invert the gate. + // Injected by pointing at a path whose parent does not exist — no mode-bit + // tricks, which root Docker/CI silently bypasses. + const dir = createTempDir('next-health-badout-'); + const api = await startApiStub(() => ({ status: 200, body: { workflow_runs: [{ conclusion: 'failure' }] } })); + t.after(async () => { await api.close(); cleanup(dir); }); + const outputPath = path.join(dir, 'no-such-dir', 'gh-output'); + + const result = await callMain( + t, + { GITHUB_EVENT_NAME: 'pull_request', GITHUB_API_URL: api.url, PR_LABELS: '' }, + { outputPath }, + ); + + assert.equal(result.code, 1, 'a RED base branch must still exit 1 when the output write fails'); + }); + + test('never echoes the token', async (t) => { + const dir = createTempDir('next-health-token-'); + const api = await startApiStub(() => ({ status: 500, body: { message: 'boom' } })); + t.after(async () => { await api.close(); cleanup(dir); }); + + const result = await callMain( + t, + { GITHUB_EVENT_NAME: 'pull_request', GITHUB_API_URL: api.url }, + { outputPath: path.join(dir, 'gh-output') }, + ); + + assert.ok(!`${result.stdout}${result.stderr}`.includes(SENTINEL_TOKEN)); + }); + + test('reports a failure without a stack trace', async (t) => { + const dir = createTempDir('next-health-nostack-'); + const api = await startApiStub(() => ({ status: 200, body: { workflow_runs: [{ conclusion: 'failure' }] } })); + t.after(async () => { await api.close(); cleanup(dir); }); + + const result = await callMain( + t, + { GITHUB_EVENT_NAME: 'pull_request', GITHUB_API_URL: api.url, PR_LABELS: '' }, + { outputPath: path.join(dir, 'gh-output') }, + ); + + assert.ok(!`${result.stdout}${result.stderr}`.includes(' at '), 'no raw stack frames'); + }); + + test('prints usage', () => { + const result = runNode([SCRIPT, '--help'], { cwd: ROOT, timeoutMs: PROBE_TIMEOUT_MS }); + assert.equal(result.exitCode, 0); + assert.ok(/Usage/i.test(result.stdout)); + }); + + test('rejects an unknown argument', () => { + const result = runNode([SCRIPT, '--nope'], { cwd: ROOT, timeoutMs: PROBE_TIMEOUT_MS }); + assert.notEqual(result.exitCode, 0); + assert.ok(`${result.stdout}${result.stderr}`.includes('--nope')); + }); +}); diff --git a/tests/gsd-check-update-worker-atomic-cache.test.cjs b/tests/gsd-check-update-worker-atomic-cache.test.cjs index b129ab141..ae418d821 100644 --- a/tests/gsd-check-update-worker-atomic-cache.test.cjs +++ b/tests/gsd-check-update-worker-atomic-cache.test.cjs @@ -30,9 +30,25 @@ const fs = require('fs'); const path = require('path'); const { runHook: runHookSeam } = require('./helpers/process-seam.cjs'); const { createTempDir, cleanup } = require('./helpers.cjs'); +const { NPM_VIEW_TIMEOUT_MS } = require('../gsd-core/bin/check-latest-version.cjs'); const WORKER_PATH = path.join(__dirname, '..', 'hooks', 'gsd-check-update-worker.js'); +// #4091/2026-09-06 Windows CI incident: the worker's real `npm view` call +// (checkLatestVersion, gsd-core/bin/check-latest-version.cjs) is bounded at +// NPM_VIEW_TIMEOUT_MS. This outer test harness timeout used to be hardcoded +// to the SAME 15000ms, so a slow registry response raced two SIGKILLs at the +// exact same wall-clock instant: the worker's own inner npm-view timeout +// fires and it needs real time to catch that failure, build a degraded +// result, and atomically publish the cache — but the outer harness could +// kill the whole process tree first (exitCode: null, empty stderr) before +// the worker ever got the chance. Windows's shell-wrapped npm subprocess +// (cmd.exe wrapper, see src/shell-command-projection.cts) adds enough spawn +// overhead to make this race lose more often there, but the zero-margin race +// itself is platform-agnostic. This margin gives the worker real headroom +// beyond the inner timeout it wraps. +const WORKER_TEARDOWN_MARGIN_MS = 10_000; + // allow-test-rule: structural-regression-guard (#4091) // Feeds the real worker source (readFileSync) into the structural assertions // below. The behavior it guards — rename-atomic publish of a file shared @@ -108,11 +124,25 @@ describe('gsd-check-update-worker.js: atomic cache publish (#4091)', () => { GSD_PROJECT_VERSION_FILE: path.join(cacheDir, 'no-such-project', 'VERSION'), GSD_GLOBAL_VERSION_FILE: path.join(cacheDir, 'no-such-global', 'VERSION'), }; - const r = runHookSeam(WORKER_PATH, [], { env, timeoutMs: 15000 }); + const r = runHookSeam(WORKER_PATH, [], { env, timeoutMs: NPM_VIEW_TIMEOUT_MS + WORKER_TEARDOWN_MARGIN_MS }); assert.equal(r.exitCode, 0, `worker must exit 0; stderr: ${r.stderr}`); const cache = JSON.parse(fs.readFileSync(cacheFile, 'utf8')); assert.equal(cache.installed, '0.0.0', 'worker record replaced the pre-existing one'); const residue = fs.readdirSync(cacheDir).filter((f) => f.startsWith('cache.json.tmp') || f.includes('.tmp-')); assert.deepEqual(residue, [], 'no temp stage files may remain after a successful publish'); }); + + test('outer worker-run timeout keeps real margin beyond the inner npm-view timeout (#4091 exact-tie race)', () => { + assert.ok( + WORKER_TEARDOWN_MARGIN_MS >= 5000, + 'the outer test timeout must give the worker real margin beyond the inner npm-view ' + + 'timeout (NPM_VIEW_TIMEOUT_MS) it wraps, or a slow registry response races the two ' + + 'SIGKILLs (see #4091/2026-09-06 Windows CI incident: exact-tie timeout killed the worker ' + + 'before it could degrade gracefully)', + ); + assert.ok( + NPM_VIEW_TIMEOUT_MS + WORKER_TEARDOWN_MARGIN_MS > NPM_VIEW_TIMEOUT_MS, + 'the combined outer timeout must strictly exceed the inner npm-view timeout it wraps', + ); + }); });