Skip to content

Commit 7ef2965

Browse files
hotlongclaude
andauthored
fix(scripts): classify a missing gate runner by resolving it, not by parsing shell prose (#17731)
Fixes #16717 `check:merge-driver` was permanently red on macOS, and the self-test failure was the cheap half of the problem. The expensive half is the field direction: with both legs of the runner-missing classifier blind, a genuinely **missing gate runner** classified as **`stale`** — the exact false claim the `PREREQUISITE NOT MET` grading exists to prevent. It told an operator to regenerate an artifact that nothing had read. ## What was broken, both legs, measured on this host Darwin 25.5.0, pnpm 10.31.0. **Leg 1, the shell line.** The matcher required a `\d+:` segment. macOS `/bin/sh` writes none — `sh: os-regen-fixture-absent-runner: command not found`. Measured alongside it, `zsh` writes the command **last** (`zsh:1: command not found: tsx`), a fourth spelling the old pattern could not have matched either. **Leg 2, the exit code.** `exitCode === 127` never fired because the immediate child is `pnpm`, not the shell, and pnpm reports **1** here. Measured directly with a throwaway package whose only script is the absent command: `pnpm exit=1`. On Linux the same path yields 127, which is why CI stayed green while every macOS checkout was red. ## What the classifier depends on now Per the card, I did not simply add the macOS spelling. Prose matching is not the only signal available, and it should not be the primary one. The classification now rests on **resolution** first: the gate's script is a string in a manifest this process can read, so the command it starts with is **looked for on disk**. That leg reads no prose and no exit code — it is a filesystem fact, identical on every platform, every shell and every pnpm. The two older legs remain as fallbacks for what resolution declines to judge (a compound command, a shell builtin, a script the manifest does not declare), with the shell line widened to the four spellings actually measured. **The searched set is deliberately a superset of pnpm's, and that asymmetry is load-bearing.** Measured inside a workspace package, pnpm puts two bin directories on PATH: the package's own `node_modules/.bin` and the repository root's. The probe walks every ancestor's plus PATH itself. A superset can only err toward "found", which costs one diagnosis. The opposite mistake would report a gate that really ran, and really found the artifact stale, as unmeasured — a refusal that swallows the finding it was spawned for. A dangling symlink counts as present for the same reason (`lstatSync` does not follow). **What would break it next.** Stated plainly, because this bug was a silent one: - The probe goes quiet, by design, when the gate's script is compound (`a && b`), starts with a shell builtin, or is not declared in the manifest pnpm was pointed at. Those fall through to the prose and exit-code legs — so a fifth shell spelling can still cost a diagnosis there, though no longer the whole classification. - An undeclared script stays `stale` on purpose: measured, pnpm exits 254 and prints nothing, and that is `gateCwd`'s documented fail-safe. The probe must not convert it into a refusal. - If pnpm ever runs scripts with a PATH the upward walk does not cover, the probe would start claiming absence for runners that resolve. That is the dangerous direction, so the walk is deliberately wide rather than exact. ## Evidence Watched failing before, passing after, on the macOS host that fails: | | before | after (`b82e4ef5f`) | |:--|:--|:--| | `node scripts/check-regen-pending.mjs --self-test` | exit 1, the 2 named cases red | exit 0 | | `pnpm check:merge-driver` | exit 1 | exit 0 | The self-test's two previously-failing cases already asserted the classification (`EXIT_PREREQUISITE_NOT_MET` plus the diagnosis sentence), and they still do. Added beside them are classifier-level rows that call the classifier directly, so every leg is judged on every host rather than only on whichever (platform, shell, pnpm) the fixture happens to run on — which is precisely how this stayed green in CI. **Each new row is load-bearing, proved by ablation** rather than asserted. Fix committed first, then one leg removed at a time; each mutation confirmed on disk by blob hash, each restore confirmed by `git diff HEAD` empty and the blob back to `203605c9`: | leg removed | rows that went red | |:--|:--| | widened shell line, old pattern restored | macOS `/bin/sh`; zsh — and **only** those two | | resolution probe | "neither prose nor 127" — and only that | | `exitCode === 127` | "a bare 127" — and only that | The third row is the Linux-path guarantee, constructed rather than reasoned about: the exit-127 case is carried by the 127 leg alone (its manifest resolves, its prose matches nothing), so removing that leg reds it and nothing else. Controls, all green: a gate that ran and failed is still `stale`; a gate whose own prose contains "not found" is not reclassified; a script starting with a shell builtin is judged by resolution not at all; and with no gate handed over, the shell-line leg still answers alone. ## Gates Derived from the actual diff with `node scripts/pm/dispatch-gates.mjs` (35 families, harvested with `--commands`): **32 green, 3 red — all three proved pre-existing** by re-running them with only this file reverted to the base commit, in the same installed tree: - `check:bash32-floor` — red at base too; a macOS bash-3.2 harness fact, same family as this card. - `check:browser-reachable-entries`, `check:generated` — red at base too; this worktree has no `packages/spec/dist`, and both gates say so themselves. `check:docs` was red in the first sweep and green on re-run; the variable was the generated `json-schema/` tree, not this file. The three consumers that import this module take only `schemaTreeIsStale`, `distIsStale` and `declarationStamp` — none of which this change touches. Also green: `check:declaration-mirrors`, and the two `packages/spec` test files that import this module (2 files, 25 tests). ⚠️ Declared, because the tool requires it: `os-verify-lock.sh` reported `UNLOCKED (declared) · no usable flock on this host, so the shared verify lock was NEVER taken and NOTHING was serialized`. No changeset: `scripts/` publishes from no package, so this releases nothing — `skip-changeset` applied. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 9bd4344 commit 7ef2965

1 file changed

Lines changed: 208 additions & 15 deletions

File tree

scripts/check-regen-pending.mjs

Lines changed: 208 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@
8484
*/
8585

8686
import { execFileSync, execSync, spawnSync } from 'node:child_process';
87-
import { appendFileSync, existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, realpathSync, rmSync, statSync, writeFileSync } from 'node:fs';
87+
import { appendFileSync, existsSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, realpathSync, rmSync, statSync, writeFileSync } from 'node:fs';
8888
import { tmpdir } from 'node:os';
8989
import { dirname, join, resolve } from 'node:path';
9090
import { fileURLToPath } from 'node:url';
@@ -438,6 +438,107 @@ export function decide({ blocked, merging, deferral, allowDefer = true }) {
438438
return 'refuse-stale';
439439
}
440440

441+
/**
442+
* Shell words that name no executable, so their absence from disk proves nothing.
443+
*
444+
* The `stale` stub in this file's own fixture is spelled `exit 1`. A probe that
445+
* judged that by resolution would classify the ONE control case here as a missing
446+
* runner -- the grading inverted, in the direction that hides a real finding. So
447+
* the probe DECLINES on these rather than guessing.
448+
*/
449+
const SHELL_BUILTINS = new Set([
450+
':', '.', '[', 'break', 'cd', 'continue', 'echo', 'eval', 'exec', 'exit', 'export',
451+
'false', 'printf', 'pwd', 'read', 'readonly', 'return', 'set', 'shift', 'source',
452+
'test', 'times', 'trap', 'true', 'type', 'ulimit', 'umask', 'unset', 'wait',
453+
]);
454+
455+
/**
456+
* The command a gate's script STARTS WITH, or `''` when it cannot be read off.
457+
*
458+
* Leading `FOO=bar` assignments are skipped -- shell syntax, not the command.
459+
* Anything carrying a shell metacharacter is declined outright: a script spelled
460+
* `(cd x && y)` or `a | b` has no single leading command whose absence would
461+
* explain the failure, and a guess there is a wrong diagnosis rather than none.
462+
*/
463+
function leadingCommand(commandLine) {
464+
for (const token of String(commandLine).trim().split(/\s+/)) {
465+
if (/^[A-Za-z_][A-Za-z0-9_]*=/.test(token)) continue;
466+
return /^[\w./@+-]+$/.test(token) ? token : '';
467+
}
468+
return '';
469+
}
470+
471+
/**
472+
* Is `cmd` absent from every directory the gate's runner could have come from?
473+
*
474+
* ⚠️ The set searched is deliberately a SUPERSET of the one pnpm builds, and the
475+
* asymmetry is load-bearing. Measured inside a workspace package, `pnpm` puts two
476+
* bin directories on `PATH`: that package's own `node_modules/.bin` and the
477+
* repository root's. This walks EVERY ancestor's, plus `PATH` itself. A superset
478+
* can only answer "found" where pnpm would have answered "not found", which costs
479+
* one diagnosis and nothing else. The opposite mistake reports a gate that really
480+
* did run, and really did find the artifact stale, as unmeasured -- a refusal
481+
* that swallows the finding it was spawned for.
482+
*
483+
* A DANGLING symlink counts as present for the same reason: `lstatSync` does not
484+
* follow, so a broken `node_modules/.bin` entry declines the claim instead of
485+
* making it.
486+
*/
487+
function commandIsAbsent(cmd, cwd) {
488+
const present = (p) => {
489+
try {
490+
lstatSync(p);
491+
return true;
492+
} catch {
493+
return false;
494+
}
495+
};
496+
if (cmd.includes('/')) return !present(resolve(cwd, cmd));
497+
const dirs = [];
498+
for (let dir = resolve(cwd); ;) {
499+
dirs.push(join(dir, 'node_modules', '.bin'));
500+
const up = dirname(dir);
501+
if (up === dir) break;
502+
dir = up;
503+
}
504+
for (const entry of String(process.env.PATH ?? '').split(':')) if (entry) dirs.push(entry);
505+
return !dirs.some((dir) => present(join(dir, cmd)));
506+
}
507+
508+
/**
509+
* The gate's runner, when it can be PROVED absent; `null` when nothing is claimed.
510+
*
511+
* The one leg of the runner-missing diagnosis that reads neither the shell's prose
512+
* nor an exit code (#16717). Both of those are written by something between the
513+
* failure and this function -- a shell whose wording differs per platform, a pnpm
514+
* that does not propagate 127 on every platform -- and each was independently
515+
* blind on macOS. The gate's script, by contrast, is a string in a manifest this
516+
* process can read, and "that command does not exist" is a fact about the disk.
517+
*
518+
* ⛔ Silent by design wherever the answer would be a guess: an undeclared script
519+
* (measured, pnpm exits 254 and prints nothing -- `gateCwd`'s documented fail-safe,
520+
* which reads as stale and must stay that way), an unreadable manifest, a builtin,
521+
* a compound command. Every one of those falls through to the other two legs.
522+
*
523+
* @param {string} script the `check:` script name the gate is spawned as
524+
* @param {string} cwd the directory pnpm is spawned in, whose manifest declares it
525+
* @returns {string|null} the absent command, or `null` when nothing can be claimed
526+
*/
527+
function absentGateRunner(script, cwd) {
528+
if (!script || !cwd) return null;
529+
let manifest;
530+
try {
531+
manifest = JSON.parse(readFileSync(join(cwd, 'package.json'), 'utf8'));
532+
} catch {
533+
return null;
534+
}
535+
const commandLine = manifest?.scripts?.[script];
536+
if (typeof commandLine !== 'string') return null;
537+
const cmd = leadingCommand(commandLine);
538+
if (!cmd || SHELL_BUILTINS.has(cmd)) return null;
539+
return commandIsAbsent(cmd, cwd) ? cmd : null;
540+
}
541+
441542
/**
442543
* A gate that could not RUN is not a verdict about the artifact (#15722).
443544
*
@@ -486,11 +587,15 @@ export function decide({ blocked, merging, deferral, allowDefer = true }) {
486587
* @param {string} output the child's combined stdout+stderr
487588
* @param {number} exitCode the child's exit status
488589
* @param {string} fromDir where to resolve from when the child named no importer
590+
* @param {{ script: string, cwd: string } | null} [gate] the gate AS SPAWNED -- the
591+
* `check:` script name and the directory whose manifest declares it. Supplied by
592+
* the production call site; omitted where there is nothing on disk to resolve, in
593+
* which case the two prose-and-exit-code legs answer alone, as they used to.
489594
* @returns {null | { headline: string, detail: string[], fix: string, kind: string }}
490595
* `null` when the gate RAN and reached a verdict of its own — the caller must
491596
* keep reporting that as `stale`, which is what it is.
492597
*/
493-
export function gateCouldNotRun(output, exitCode, fromDir) {
598+
export function gateCouldNotRun(output, exitCode, fromDir, gate = null) {
494599
const text = String(output ?? '');
495600

496601
// -- Shape 3: the child already answered in this exact frame --
@@ -515,24 +620,58 @@ export function gateCouldNotRun(output, exitCode, fromDir) {
515620
};
516621
}
517622

518-
// -- Shape 1: the runner is not on PATH --
519-
// Anchored on the shell's own line rather than on the words "not found", which
520-
// a gate's prose may legitimately contain. `sh` (dash) writes `sh: 1: tsx: not
521-
// found`; bash writes `bash: line 1: tsx: command not found`.
522-
const runner = text.match(/^(?:sh|bash|dash|zsh): (?:line )?\d+: ([^:\n]+): (?:command )?not found$/m);
523-
if (runner || exitCode === 127) {
524-
const cmd = runner?.[1] ?? '';
623+
// -- Shape 1: the gate's runner is not installed --
624+
//
625+
// THREE legs, ordered by how much each one can be trusted (#16717). The reading
626+
// they exist to prevent is `stale`: a claim about the artifact that nothing
627+
// measured, told to an operator whose real problem is a missing tool.
628+
//
629+
// 1. RESOLUTION -- the gate's script is on disk, in the manifest pnpm was
630+
// pointed at, so the command it starts with can simply be LOOKED FOR.
631+
// Reads no prose and no exit code: a filesystem fact, identical on every
632+
// platform, every shell and every pnpm.
633+
// 2. The shell's own line, for what leg 1 declines to judge (a compound
634+
// script, a builtin, a script this manifest does not declare).
635+
// 3. A raw 127, for a process chain that propagates the shell's exec failure.
636+
//
637+
// ⚠️ Legs 2 and 3 WERE the whole classification, and both miss on macOS --
638+
// measured on Darwin 25.5.0 / pnpm 10.31.0, not hypothesised:
639+
//
640+
// - macOS `/bin/sh` writes `sh: tsx: command not found`, with NO line-number
641+
// segment, which the matcher used to require. dash writes `sh: 1: tsx: not
642+
// found`, bash writes `bash: line 1: tsx: command not found`, and zsh writes
643+
// the command LAST (`zsh:1: command not found: tsx`) -- four spellings of one
644+
// event, and a fifth shell is free to invent a fifth.
645+
// - the immediate child is `pnpm`, not the shell, and pnpm reports 1 for this,
646+
// not 127. On Linux the same path yields 127, which is why CI stayed green
647+
// while every macOS checkout was red.
648+
//
649+
// So leg 3 is a property of the process chain and leg 2 of a shell's wording;
650+
// either can change and reopen this hole silently, in the dangerous direction.
651+
// Leg 1 depends on neither, which is why it is first.
652+
const runner = text.match(/^(?:\S*\/)?(?:sh|bash|dash|ksh|zsh): (?:(?:line )?\d+: )?([^:\n]+): (?:command )?not found$/m)
653+
?? text.match(/^(?:\S*\/)?zsh:(?:\d+:)? command not found: (\S+)$/m);
654+
const absent = gate ? absentGateRunner(gate.script, gate.cwd) : null;
655+
if (absent || runner || exitCode === 127) {
656+
const cmd = absent ?? runner?.[1] ?? '';
525657
return {
526658
kind: 'runner-missing',
527659
headline: cmd
528660
? `the gate's runner \`${cmd}\` is not installed`
529661
: 'the gate\'s runner is not installed',
530662
detail: [
531-
cmd
532-
? `The gate's script starts with \`${cmd}\`, and the shell could not find it. Runners`
533-
: 'The shell could not find the command the gate\'s script starts with. Runners',
534-
`like \`tsx\` live in \`node_modules/.bin\`, so a checkout with no \`node_modules\` has none`,
535-
`of them and the gate's own code was never reached.`,
663+
...(absent
664+
? [
665+
`\`${absent}\` is the first word of the gate's \`${gate.script}\` script, and it is in`,
666+
`neither that package's \`node_modules/.bin\`, nor any parent's, nor \`PATH\` -- looked`,
667+
`for on disk, rather than read off the shell's complaint, which is spelled`,
668+
`differently on every platform.`,
669+
]
670+
: cmd
671+
? [`The gate's script starts with \`${cmd}\`, and the shell could not find it.`]
672+
: ['The shell could not find the command the gate\'s script starts with.']),
673+
`Runners like \`tsx\` live in \`node_modules/.bin\`, so a checkout with no \`node_modules\``,
674+
`has none of them and the gate's own code was never reached.`,
536675
],
537676
fix: INSTALL_FIX,
538677
};
@@ -716,7 +855,7 @@ function main({ prePush = false } = {}) {
716855
// A gate that could not RUN answered nothing (#15722). It still counts as
717856
// BLOCKED — the artifact is not proven current, so the marker keeps it and the
718857
// refusal stands — but the line says what happened rather than naming the file.
719-
const prereq = gateCouldNotRun(output, code, cwd);
858+
const prereq = gateCouldNotRun(output, code, cwd, { script: check, cwd });
720859
if (prereq) {
721860
unmeasured.push({ check, paths, prereq });
722861
console.error(
@@ -1270,6 +1409,60 @@ function fixtureSelfTest() {
12701409
check(' …naming the command the shell could not find, in the diagnosis',
12711410
/the gate's runner `os-regen-fixture-absent-runner` is not installed/.test(runnerMissing.out));
12721411

1412+
// ── #16717: what the runner-missing CLASSIFICATION rests on ──────────────
1413+
//
1414+
// The two cases above reach the classifier through the fixture, so between
1415+
// them they exercise exactly one (platform, shell, pnpm) combination: this
1416+
// machine's. That is how BOTH legs of this classification came to be broken
1417+
// on macOS while CI stayed green -- the fixture could not fail on the host
1418+
// that could. The rows below call the classifier DIRECTLY with the output
1419+
// other platforms produce, so every leg is judged on every host.
1420+
//
1421+
// Each row asserts the CLASSIFICATION. `runner-missing` is the verdict under
1422+
// test and `null` is the one that means `stale` -- the false claim -- so a
1423+
// row proving only that "something was returned" would accept the bug.
1424+
const probeDir = mkdtempSync(join(tmpdir(), 'os-regen-classify-'));
1425+
// The gate AS SPAWNED: a manifest declaring the script, in the directory pnpm
1426+
// would be pointed at. Written per row, so no row depends on which `runHook`
1427+
// ran last.
1428+
const gateWith = (command) => {
1429+
writeFileSync(
1430+
join(probeDir, 'package.json'),
1431+
`${JSON.stringify({ name: 'os-regen-classifier-probe', scripts: { 'check:spec-changes': command } }, null, 2)}\n`,
1432+
);
1433+
return { script: 'check:spec-changes', cwd: probeDir };
1434+
};
1435+
// `node x.mjs` on the prose rows deliberately RESOLVES, so the resolution leg
1436+
// declines and each row judges the spelling in front of it and nothing else.
1437+
for (const [label, command, out, code, named] of [
1438+
['dash: a line number, no "command"', 'node x.mjs', 'sh: 1: tsx: not found', 1, 'tsx'],
1439+
['bash: `line N`, with "command"', 'node x.mjs', 'bash: line 1: tsx: command not found', 1, 'tsx'],
1440+
// THE HOST BUG: macOS `/bin/sh` writes no line number at all.
1441+
['macOS /bin/sh: NO line number', 'node x.mjs', 'sh: tsx: command not found', 1, 'tsx'],
1442+
// A FOURTH spelling, which no widening of the third would have reached.
1443+
['zsh: the command written LAST', 'node x.mjs', 'zsh:1: command not found: tsx', 1, 'tsx'],
1444+
// THE LINUX LEG, constructed rather than reasoned about: there pnpm
1445+
// propagates 127, and this file may not recognise a word of the output.
1446+
['a bare 127, prose in no spelling this file knows', 'node x.mjs', 'wrapper: cannot exec', 127, ''],
1447+
// THE RESOLUTION LEG, isolated: NEITHER signal macOS denies this gate is
1448+
// present, and the classification still holds.
1449+
['neither prose nor 127 -- the runner is LOOKED FOR', 'os-regen-fixture-absent-runner --check', '', 1,
1450+
'os-regen-fixture-absent-runner'],
1451+
]) {
1452+
const verdict = gateCouldNotRun(out, code, probeDir, gateWith(command));
1453+
check(` …${label} → runner-missing`,
1454+
verdict?.kind === 'runner-missing' && (named === '' || verdict.headline.includes(`\`${named}\``)));
1455+
}
1456+
// THE CONTROLS for the legs above. A gate that RAN and reached a verdict of
1457+
// its own must stay `null`, which the caller prints as `stale`.
1458+
check(' …a gate that RAN is still `stale` when its OWN prose says "not found"',
1459+
gateCouldNotRun(' ✗ 3 declarations not found in dist', 1, probeDir, gateWith('node x.mjs')) === null);
1460+
check(' …and a script starting with a shell builtin is judged by resolution NOT AT ALL',
1461+
gateCouldNotRun('the artifact is out of date', 1, probeDir, gateWith('exit 1')) === null);
1462+
check(' …with no gate to resolve, the shell-line leg still answers on its own',
1463+
gateCouldNotRun('bash: line 1: tsx: command not found', 1, probeDir)?.kind === 'runner-missing');
1464+
rmSync(probeDir, { recursive: true, force: true });
1465+
12731466
const gateRefused = runHook('gate-refused', ['--pre-push']);
12741467
check('a gate that ALREADY refused with an unmet prerequisite is propagated, not relabelled',
12751468
gateRefused.code === EXIT_PREREQUISITE_NOT_MET && !/ stale/.test(gateRefused.out));

0 commit comments

Comments
 (0)