From 272be13f2d171fe823925909954e76c74691bf8e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 05:29:08 +0000 Subject: [PATCH 1/2] fix(pm): the dispatch-gates wrapper's kill branch exits 3, so its code says what its text says MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `result.signal` branch printed "⛔ Do not record it as a run" and then exited 2. `dispatch-gates.mjs --ran` classifies a recorded exit FROM THE CODE: 3 is the repo-wide NOT MEASURED code, 124 and the 128 + signum floor are kills, and everything else is a run. 2 is none of those, so a cap-killed battery recorded honestly as `:: exit 2` reconciled inside the `run` total — a family counted as measured on a run that measured nothing. The branch now exits EXIT_PREREQUISITE_NOT_MET and names the code in its own remedy text. A child that RAN keeps `result.status`; the spawn-failure branch keeps 2, because "the tool is gone" and "the box could not fork" reach it wearing the same error and only the second is a not-measured reading. Pinned by this file's first self-test: three stub children (one that signals its own pid, one red, one green) driven through a `--self-test-child` flag, asserting the wrapper's exit for each AND how `runReconciliation` reads a record of it — including a regression pin that 2 still reconciles as a run. Wired the sibling way (`--self-test && `), so its row leaves check-self-test-wired's SELF_TEST_RUN_OTHERWISE ledger. Claude-Session: https://claude.ai/code/session_01HZfg2AwVX191qCizp88gQr Co-authored-by: Claude --- package.json | 2 +- scripts/check-self-test-wired.mjs | 6 - scripts/pm/check-dispatch-gates.mjs | 233 +++++++++++++++++++++++++++- 3 files changed, 229 insertions(+), 12 deletions(-) diff --git a/package.json b/package.json index bc01947e96e..d908d8be504 100644 --- a/package.json +++ b/package.json @@ -73,7 +73,7 @@ "check:pm-skill-ratchet": "node scripts/pm/check-skill-line-ratchet.mjs --self-test && node scripts/pm/check-skill-line-ratchet.mjs", "check:pm-skill-id-lint": "node scripts/pm/check-skill-id-lint.mjs --self-test && node scripts/pm/check-skill-id-lint.mjs", "check:pm-label-desc-cap": "node scripts/pm/check-label-desc-cap.mjs --self-test && node scripts/pm/check-label-desc-cap.mjs", - "check:pm-dispatch-gates": "node scripts/pm/check-dispatch-gates.mjs", + "check:pm-dispatch-gates": "node scripts/pm/check-dispatch-gates.mjs --self-test && node scripts/pm/check-dispatch-gates.mjs", "check:watch-hint-literal": "node scripts/check-watch-hint-literal.mjs --self-test && node scripts/check-watch-hint-literal.mjs", "check:declared-population-live": "node scripts/check-declared-population-live.mjs --self-test && node scripts/check-declared-population-live.mjs", "check:pm-half-states": "node scripts/pm/check-half-states.mjs --self-test", diff --git a/scripts/check-self-test-wired.mjs b/scripts/check-self-test-wired.mjs index db95b1025a4..c1ae8e30a10 100644 --- a/scripts/check-self-test-wired.mjs +++ b/scripts/check-self-test-wired.mjs @@ -233,12 +233,6 @@ const INVOCATION_RE = * unrun self-test gets wired instead. */ const SELF_TEST_RUN_OTHERWISE = [ - { - script: 'scripts/pm/check-dispatch-gates.mjs', - via: 'drives', - evidence: 'scripts/pm/dispatch-gates.mjs', - why: 'its whole body spawns the tool with --self-test; lint.yml runs the wrapper bare', - }, { script: 'scripts/docs-audit/check-affected-docs.mjs', via: 'drives', diff --git a/scripts/pm/check-dispatch-gates.mjs b/scripts/pm/check-dispatch-gates.mjs index 8f0e2d3160d..f4c07c29f46 100644 --- a/scripts/pm/check-dispatch-gates.mjs +++ b/scripts/pm/check-dispatch-gates.mjs @@ -53,6 +53,47 @@ * on a quiet box reaches a verdict at all, where before it could only ever be * killed. * + * ## The exit contract, and why the kill branch does not keep its old code + * + * Four endings, four codes, and what fixes them is not this file's taste — it + * is what READS them. A dev records a gate's exit beside the command it ran + * (a `:: exit N` tail) and `dispatch-gates.mjs --ran` reconciles that record + * against the families it derives, classifying each line FROM THE CODE. A code + * here is therefore a claim addressed to a reconciler, and this file had one of + * them wrong: + * + * the battery passed result.status (0) a run that passed + * the battery failed result.status a run that failed + * the battery was KILLED EXIT_PREREQUISITE_NOT_MET NOT MEASURED + * the tool could not spawn 2 neither, on purpose + * + * The kill row used to exit 2, and 2 is in none of the reconciler's classes — + * not its NOT-MEASURED code, and not its kill set (coreutils `timeout`'s 124 + * and the 128 + signum floor), so it landed inside the `run` total. A family + * counted as measured on the strength of a run that measured nothing is the + * exact false green that reconciliation exists to refuse. The branch's own text + * said the opposite in the same breath — do not record it as a run — and was + * the only carrier saying it, so a reader who copied the number rather than the + * sentence filed a red run over a gate that never reached a verdict. + * + * ⛔ The fix is NOT 143, the shape a shell reports for a SIGTERM'd child. The + * reconciler reads a kill code as UNRUN unless the runner ALSO writes a + * NOT-MEASURED claim with a stated reason beside it — deliberately, because an + * unexplained kill is where an unfinished run hides. That is the right default + * for a runner relaying a kill it did not diagnose. It is the wrong one here, + * where this branch has already identified the kill, printed the reason and + * named the remedy before it exits: everything the claim line would carry is + * already on stderr, and requiring a second line to be remembered is how the + * reading gets lost again. EXIT_PREREQUISITE_NOT_MET carries it in the code + * itself, and it is the repo-wide code for this reading. + * + * The spawn-failure row keeps 2 deliberately; its reason sits at that branch. + * + * ⛔ None of this moves CI. lint.yml runs this gate as an ordinary step with no + * `continue-on-error`, so every non-zero code above is equally red there. These + * codes are read by devs and by `--ran`, and the contract is pinned by this + * file's own `--self-test` rather than by this paragraph. + * * ## Why the gate exists * * scripts/pm/dispatch-gates.mjs derives the "local gates for this card" line of @@ -183,8 +224,13 @@ */ import { spawnSync } from 'node:child_process'; -import { join } from 'node:path'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; import process from 'node:process'; +import { fileURLToPath } from 'node:url'; + +import { EXIT_PREREQUISITE_NOT_MET } from '../import-prerequisite.mjs'; const ROOT = new URL('../..', import.meta.url).pathname; @@ -223,8 +269,54 @@ const SURFACE_MODULE = 'scripts/i18n-bundle-surface.mjs'; */ const FRAME_MODULE = 'scripts/check-skill-frame-sync.mjs'; +/** + * The flag under which this file drives a child OTHER than TOOL, spelled so + * that any invocation carrying it reads as a test invocation wherever it is + * written down. + * + * ⛔ Not `--tool`, and not an `OS_TEST_*` environment variable. The exit + * contract below cannot be pinned without standing the 435-second battery + * down, and the battery is named by a module-body constant, so SOMETHING has + * to be substitutable. What that something must never be is quiet: a run + * against a stub grades nothing about the real tool, so the one failure this + * affordance could introduce is a green gate over a child that is not TOOL. + * An environment variable is the shape that fails that way — it is inherited + * from whatever shell the runner was started in, so it can arrive without + * appearing in any invocation anyone reads. A flag cannot: it is spelled at + * the call site, `package.json` holds the only invocation CI runs, and the + * word `self-test` is in the flag itself. The substituted run also announces + * itself on stderr, and the self-test asserts that it does — so the loudness + * is live rather than promised. + * + * The right boundary matters to a sibling gate: `check-self-test-wired` + * matches `--self-test` with one, so this longer flag is not read as an + * invocation of the self-test, and no row is credited for it. + */ +const SELF_TEST_CHILD_FLAG = '--self-test-child'; + +/** The flag that runs this file's own battery instead of the tool's. */ +const SELF_TEST_FLAG = '--self-test'; + +const argv = process.argv.slice(2); + +if (argv.includes(SELF_TEST_FLAG)) await selfTest(); + +const childAt = argv.indexOf(SELF_TEST_CHILD_FLAG); +const child = childAt < 0 ? TOOL : argv[childAt + 1]; +if (child === undefined || child === '') { + console.error(`✗ check:pm-dispatch-gates: ${SELF_TEST_CHILD_FLAG} needs a path after it.`); + process.exit(2); +} +if (child !== TOOL) { + console.error( + `⛔ check:pm-dispatch-gates: SUBSTITUTED CHILD — this run spawned ${child}, not ${TOOL}, so it grades` + + " NOTHING about the tool. It exists so this file's own self-test can drive the exit contract below" + + ' without standing up the real battery. A production run never prints this line.', + ); +} + const started = Date.now(); -const result = spawnSync(process.execPath, [join(ROOT, TOOL), '--self-test'], { stdio: 'inherit' }); +const result = spawnSync(process.execPath, [resolve(ROOT, child), '--self-test'], { stdio: 'inherit' }); /** * What the battery cost on THIS box, printed rather than frozen anywhere. * @@ -237,7 +329,13 @@ const result = spawnSync(process.execPath, [join(ROOT, TOOL), '--self-test'], { const seconds = ((Date.now() - started) / 1000).toFixed(1); if (result.error) { - console.error(`✗ check:pm-dispatch-gates: could not run ${TOOL} — ${result.error.message}`); + // ⛔ NOT the kill branch's code, and the difference is argued in the header's + // exit-contract section: a spawn that never started has two causes this file + // cannot tell apart — the tool is gone from the tree, which is a finding + // about the tree, and the box could not fork, which is not. 3 would assert + // the second reading over both. Until one measurement separates them this + // stays the code that claims neither. + console.error(`✗ check:pm-dispatch-gates: could not run ${child} — ${result.error.message}`); process.exit(2); } if (result.signal) { @@ -246,18 +344,143 @@ if (result.signal) { // the question a killed caller actually has — what do I do now — rather // than naming the signal and stopping, which is what it used to do. console.error( - `✗ check:pm-dispatch-gates: ${TOOL} --self-test was killed by ${result.signal} after ${seconds}s — NOTHING was measured.`, + `✗ check:pm-dispatch-gates: ${child} --self-test was killed by ${result.signal} after ${seconds}s — NOTHING was measured.`, ); console.error( ' Every case decided before the kill is in the output above and every case after it is unjudged, so this run' + ' grades neither the tool nor your diff. ⛔ Do not record it as a run.', ); + console.error( + ` This exits ${EXIT_PREREQUISITE_NOT_MET}, the repo-wide NOT MEASURED code, so a record line` + + ` \` :: exit ${EXIT_PREREQUISITE_NOT_MET}\` reconciles as NOT-MEASURED rather than as a run that failed.`, + ); console.error( " Remedy — detach it and read the log, the invocation this file's header prescribes for a capped container:", ); console.error(' nohup pnpm check:pm-dispatch-gates > pm-dispatch-gates.log 2>&1 &'); console.error(' then tail that log until it stops growing. CI runs this step with no such cap.'); - process.exit(2); + process.exit(EXIT_PREREQUISITE_NOT_MET); } console.error(`check:pm-dispatch-gates: the battery took ${seconds}s on this box.`); process.exit(result.status ?? 2); + +/** + * This file's own battery — the exit contract, driven against stub children. + * + * ## Why it cannot simply run the gate + * + * Every other assertion about this file would be a reading of the 435-second + * tool battery, and the branch under test only fires when that battery is + * KILLED. A self-test that reproduced the real conditions would have to stand + * the battery up and then race it, which is the one shape that cannot be run + * on every PR. So the child is substituted and the three ways a child can end + * are written directly: killed by a signal, exited red, exited green. + * + * ## Why the killed stub kills ITSELF + * + * Measured both ways on this box. Signalling from outside — spawn the wrapper, + * find its child, send it a SIGTERM — needs the grandchild's pid, so it races + * the spawn and reads the process table to get it; and `timeout -s TERM` on + * the wrapper does not exercise this branch AT ALL, because the wrapper has no + * SIGTERM handler and dies with the child, leaving `timeout`'s own 124 and no + * `result.signal` anywhere. A stub that signals its own pid has no race and no + * pid lookup: node with no SIGTERM listener takes the default disposition, so + * `spawnSync` reports `signal: 'SIGTERM'` and `status: null` — the exact shape + * a foreground-cap kill produces, reached deterministically. + * + * ## The half that is NOT about this file + * + * A number is only a contract if something reads it that way, so each exit is + * also pushed through the reconciler that consumes it, on a derivation of one + * family. That is what makes `3` mean NOT MEASURED here rather than merely + * being three — and the old `2` is pinned alongside it, still reconciling as a + * run, so the case that motivated the change cannot quietly come back. + */ +async function selfTest() { + let failures = 0; + const t = (name, ok) => { + console.log(`${ok ? '✓' : '✗'} ${name}`); + if (!ok) failures += 1; + }; + + const SELF = fileURLToPath(import.meta.url); + const dir = mkdtempSync(join(tmpdir(), 'check-dispatch-gates-selftest-')); + try { + const stub = (name, body) => { + const at = join(dir, name); + writeFileSync(at, body); + return at; + }; + // Signals its own pid: no listener is registered, so the default + // disposition ends the process and the timer only keeps the loop alive in + // case delivery is not synchronous. + const killedChild = stub('killed.mjs', "process.kill(process.pid, 'SIGTERM');\nsetTimeout(() => {}, 5000);\n"); + const redChild = stub('red.mjs', 'process.exit(1);\n'); + const greenChild = stub('green.mjs', 'process.exit(0);\n'); + + const drive = (childPath) => + spawnSync(process.execPath, [SELF, SELF_TEST_CHILD_FLAG, childPath], { encoding: 'utf8' }); + + const killed = drive(killedChild); + const red = drive(redChild); + const green = drive(greenChild); + + t( + 'CONTROL: the killed stub really died by SIGNAL, so the branch under test is the one that ran', + killed.stderr.includes('was killed by SIGTERM'), + ); + t( + `⭐ a child killed by a signal exits ${EXIT_PREREQUISITE_NOT_MET} — the code this branch's own text asks for`, + killed.status === EXIT_PREREQUISITE_NOT_MET, + ); + t( + "…and the text that asks for it is still printed, so the two carriers cannot drift apart silently", + killed.stderr.includes('Do not record it as a run') && killed.stderr.includes('NOTHING was measured'), + ); + t('a child that RAN and failed keeps its own status', red.status === 1); + t('a child that RAN and passed keeps its own status', green.status === 0); + t( + '⛔ and a substituted child announces itself on every one of those runs — a stub run is never quiet', + [killed, red, green].every((r) => r.stderr.includes('SUBSTITUTED CHILD')), + ); + + // The reconciler's own reading of those three codes. Imported rather than + // spawned: what is under test is how a RECORDED code classifies, not how + // the derivation finds this family, and a spawned derivation would add a + // full workflow-tree walk per case to every run of this gate. + const { parseRunRecord, runReconciliation, RUN_RECORD_EXIT_PREFIX, RUN_RECORD_REASON_SEPARATOR } = await import( + resolve(ROOT, TOOL) + ); + const COMMAND = 'pnpm check:pm-dispatch-gates'; + const reconcile = (code) => + runReconciliation({ + derived: [COMMAND], + record: parseRunRecord(`${COMMAND}${RUN_RECORD_REASON_SEPARATOR}${RUN_RECORD_EXIT_PREFIX}${code}`), + }); + + const onKill = reconcile(killed.status); + t( + `⭐ …and a record of that exit reconciles as NOT-MEASURED, which is the whole contract`, + onKill.notMeasured.length === 1 && onKill.ran.length === 0 && onKill.unrun.length === 0, + ); + t( + '…derived from the CODE, not claimed by the runner — so a killed battery cannot be recorded as a run by hand', + onKill.notMeasured[0]?.source === 'exit-code', + ); + t('a red run reconciles as a RUN, and the reconciliation holds', reconcile(red.status).ran.length === 1); + t('a green run reconciles as a RUN too', reconcile(green.status).ran.length === 1); + t( + '⛔ REGRESSION PIN: the code this branch used to exit still reconciles as a RUN — which is why it moved', + reconcile(2).ran.length === 1 && reconcile(2).notMeasured.length === 0, + ); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + + console.log( + failures === 0 + ? '✓ check:pm-dispatch-gates --self-test: the exit contract holds in all three directions.' + : `✗ check:pm-dispatch-gates --self-test: ${failures} case(s) failed.`, + ); + process.exit(failures === 0 ? 0 : 1); +} From fed9c9e28dda731469e7a2f094954efb374eecd7 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 05:39:13 +0000 Subject: [PATCH 2/2] fix(pm): keep the production spawn naming TOOL, so the gate keeps its RUN edge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first draft routed both the real and the substituted child through one `resolve(ROOT, child)` call. The tool's derivation follows that spawn as a RUN edge to inherit TOOL's watch hints — the workflow tree among them — and its run scan refuses a rebound program component, so the edge vanished: five cases of the tool's own 1730-case battery red, and a workflows-only surface stopped deriving `check:pm-dispatch-gates` at all (measured 1 -> 0 on .github/workflows/board-snapshot.yml). The substituted child now gets its own call and the production one is the expression that was there before. Claude-Session: https://claude.ai/code/session_01HZfg2AwVX191qCizp88gQr Co-authored-by: Claude --- scripts/pm/check-dispatch-gates.mjs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/scripts/pm/check-dispatch-gates.mjs b/scripts/pm/check-dispatch-gates.mjs index f4c07c29f46..c5864194b2c 100644 --- a/scripts/pm/check-dispatch-gates.mjs +++ b/scripts/pm/check-dispatch-gates.mjs @@ -316,7 +316,23 @@ if (child !== TOOL) { } const started = Date.now(); -const result = spawnSync(process.execPath, [resolve(ROOT, child), '--self-test'], { stdio: 'inherit' }); +/** + * ⛔ The production spawn names TOOL DIRECTLY, and it has to keep doing so. + * + * The tool's derivation follows this call as a RUN edge, which is how this gate + * inherits TOOL's own watch hints — the workflow tree among them — so that a + * card touching only `.github/workflows` derives this gate at all. That scan + * refuses a REBOUND program component on purpose, so collapsing both spawns + * into one `resolve(ROOT, child)` silently cuts the edge. Measured when this + * file's self-test was first written that way: five cases of the tool's own + * battery red, and a workflows-only derivation stopped naming this gate + * entirely. The substituted child therefore gets its OWN call, and the + * production one is byte-for-byte the expression that was here before. + */ +const result = + childAt < 0 + ? spawnSync(process.execPath, [join(ROOT, TOOL), '--self-test'], { stdio: 'inherit' }) + : spawnSync(process.execPath, [resolve(ROOT, child), '--self-test'], { stdio: 'inherit' }); /** * What the battery cost on THIS box, printed rather than frozen anywhere. *