Skip to content

Commit 5a2a209

Browse files
committed
fix(scripts): check-regen-pending --self-test no longer spawns its stubs through an ambient package manager
The fixture wrote its gate stub into a mkdtemp directory under tmpdir() and ran it as `pnpm -s check:spec-changes`. That directory declares no `packageManager` and has no parent manifest to inherit one from, so the verdict was decided by whatever launcher resolved there rather than by the tree. Measured against CI's own Corepack store: `.github/actions/setup-pnpm` materialises the pinned pnpm and writes no `lastKnownGood.json`, so in a directory with no pin Corepack ignores the pin and resolves pnpm's `latest` dist-tag instead. That is pnpm 12, whose CLI rejects `-s` outright (`error: unexpected argument '-s' found`, exit 2). Every stub then collapsed to "the gate exited non-zero", which the script correctly grades as `stale`, so exactly the cases whose expected outcome was not stale went red -- byte identically on an innocent PR and on origin/main's own push build. The gate now takes its launcher from `OS_REGEN_GATE_LAUNCHER`, which production leaves unset (`pnpm -s`, unchanged) and the fixture points at a launcher it writes itself: it reads the named script out of the manifest at cwd and runs the body under /bin/sh with argv0 `sh`, the same producer and the same diagnostic shapes the classifier already anchors on. No case is skipped, disabled or relaxed; two cases are ADDED, asserting that a hostile pnpm first on PATH leaves both the `stale` and the `clean` readings byte-identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
1 parent f7db8f4 commit 5a2a209

1 file changed

Lines changed: 126 additions & 4 deletions

File tree

scripts/check-regen-pending.mjs

Lines changed: 126 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@
6767
import { execFileSync, execSync, spawnSync } from 'node:child_process';
6868
import { appendFileSync, existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, statSync, writeFileSync } from 'node:fs';
6969
import { tmpdir } from 'node:os';
70-
import { dirname, join, resolve } from 'node:path';
70+
import { delimiter, dirname, join, resolve } from 'node:path';
7171
import { fileURLToPath } from 'node:url';
7272

7373
import { PENDING_MARKER, entryForPath, ownerDir, ownerOf, ownerRunCommand } from './regen-artifacts.mjs';
@@ -90,6 +90,39 @@ const SPEC_DIR = join(REPO_ROOT, 'packages/spec');
9090
*/
9191
const GATE_CWD_OVERRIDE = process.env.OS_REGEN_GATE_CWD || null;
9292

93+
/**
94+
* The launcher the gates are spawned THROUGH. Production leaves this unset and
95+
* every gate runs as `pnpm -s <script>`, which resolves the script in the
96+
* manifest at `gateCwd()` and puts that workspace's `node_modules/.bin` on PATH.
97+
* Nothing in the repo sets it; `--self-test` does, and only `--self-test`.
98+
*
99+
* ## Why the fixture may not inherit an ambient one
100+
*
101+
* The stubs run in a `mkdtemp` directory under `tmpdir()`. That directory is
102+
* OUTSIDE the repo, so no parent manifest supplies a `packageManager` pin, and a
103+
* launcher asked to resolve there answers with whatever the ambient toolchain
104+
* decides — which is not a fact about this script, and was measured deciding its
105+
* verdict outright.
106+
*
107+
* Measured 2026-09-05, on CI's own Corepack store: `.github/actions/setup-pnpm`
108+
* materialises the pinned pnpm and writes NO `lastKnownGood.json`, so in a
109+
* directory with no pin Corepack ignores the pin and resolves pnpm's `latest`
110+
* dist-tag instead — pnpm 12, whose CLI rejects `-s` outright (`error: unexpected
111+
* argument '-s' found`, exit 2). Every stub then collapsed to "the gate exited
112+
* non-zero", which this script correctly grades as `stale`, so exactly the cases
113+
* whose expected outcome was NOT stale went red. The verdict was a pure function
114+
* of the runner: byte-identical on an innocent PR and on `origin/main`'s own push
115+
* build (run 33981169123), which is what made it read as one PR's fault.
116+
*
117+
* The remedy is the one AGENTS.md already states for `check:cross-package-test-inputs`
118+
* — a detector with no dependencies cannot itself fail to resolve in CI. The
119+
* fixture supplies its own launcher, so the self-test asserts this script's
120+
* GRADING of an exit code, which is all those cases were ever about, without
121+
* importing an ambient toolchain into the assertion. `hostile pnpm` cases in
122+
* `fixtureSelfTest` pin that both ways.
123+
*/
124+
const GATE_LAUNCHER = process.env.OS_REGEN_GATE_LAUNCHER || 'pnpm -s';
125+
93126
/**
94127
* Where ONE artifact's gate is spawned: the directory of the package that declares
95128
* it (#13585).
@@ -430,7 +463,7 @@ export function gateCouldNotRun(output, exitCode, fromDir) {
430463

431464
function runCheck(script, cwd) {
432465
try {
433-
execSync(`pnpm -s ${script}`, { cwd, stdio: ['ignore', 'pipe', 'pipe'] });
466+
execSync(`${GATE_LAUNCHER} ${script}`, { cwd, stdio: ['ignore', 'pipe', 'pipe'] });
434467
return { ok: true, output: '', code: 0 };
435468
} catch (err) {
436469
return {
@@ -881,10 +914,52 @@ function batteryFloorFailures(invoked) {
881914
* flips from failing to passing — the two-commit shape is what is under test, not
882915
* the spec gates, and spawning the real ones here would make the self-test cost
883916
* a full spec build.
917+
*
918+
* They are also spawned through the fixture's OWN launcher via
919+
* `OS_REGEN_GATE_LAUNCHER` (see that constant), because this directory has no
920+
* `packageManager` in scope and an ambient package manager resolving here decided
921+
* the verdict — the defect the `hostile pnpm` cases below now pin.
884922
*/
885923
function fixtureSelfTest() {
886924
registerCase('fixtureSelfTest');
887925
const dir = mkdtempSync(join(tmpdir(), 'os-regen-defer-'));
926+
927+
// The launcher the fixture supplies for itself. `runCheck` spawns
928+
// `<launcher> <script>` in the gate directory; production leaves that `pnpm -s`,
929+
// and the fixture points it here so that no ambient package manager is asked to
930+
// resolve in a directory which pins none (see `GATE_LAUNCHER` for the measured
931+
// defect). It does what `pnpm <script>` does for a stub that needs no
932+
// `node_modules/.bin`: read the named script out of the manifest at `cwd` and run
933+
// its body in the same `/bin/sh` that `execSync` would have used anyway.
934+
//
935+
// It lives in a SEPARATE temp dir, outside the fixture repo, deliberately: a file
936+
// written into `dir` would be swept into the index by the `git add -A` on `side2`
937+
// below and re-open the stat-dirty `merge --abort` crash of #9258 — the hazard the
938+
// `info/exclude` note guards `package.json` against.
939+
const launcherDir = mkdtempSync(join(tmpdir(), 'os-regen-launcher-'));
940+
const launcherPath = join(launcherDir, 'pm-stub.mjs');
941+
writeFileSync(launcherPath, `// The fixture launcher named by OS_REGEN_GATE_LAUNCHER.
942+
import { spawnSync } from 'node:child_process';
943+
import { readFileSync } from 'node:fs';
944+
945+
const script = process.argv[2];
946+
const manifest = JSON.parse(readFileSync('package.json', 'utf8'));
947+
const body = manifest.scripts?.[script];
948+
if (body === undefined) {
949+
console.error("pm-stub: no such script: " + script);
950+
process.exit(1);
951+
}
952+
// /bin/sh by ABSOLUTE path, so no PATH lookup decides anything here — but argv0
953+
// "sh", because dash prefixes its diagnostics with argv0 and gateCouldNotRun()
954+
// anchors on the bare shell name a package manager produces. Measured: the same
955+
// body under execSync says "/bin/sh: 1: x: not found", which that regex does not
956+
// match, and the runner name drops out of the diagnosis while exit 127 still
957+
// classifies. The fixture owes production shape, not a widened matcher.
958+
// stdio inherit hands the body the pipes the caller opened, so the shell own
959+
// not-found line and node ERR_MODULE_NOT_FOUND reach the classifier unchanged.
960+
const r = spawnSync('/bin/sh', ['-c', body], { argv0: 'sh', stdio: 'inherit' });
961+
process.exit(r.status ?? 1);
962+
`);
888963
const git = (args, opts = {}) =>
889964
execFileSync('git', args, { cwd: dir, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], ...opts });
890965
const results = [];
@@ -928,7 +1003,7 @@ function fixtureSelfTest() {
9281003
'node -e "console.error(\'stub-gate: PREREQUISITE NOT MET — the dependency yaml is not installed\'); process.exit(3)"',
9291004
};
9301005

931-
const runHook = (gate, args = []) => {
1006+
const runHook = (gate, args = [], extraEnv = {}) => {
9321007
writeFileSync(
9331008
join(dir, 'package.json'),
9341009
`${JSON.stringify({ name: 'os-regen-fixture', scripts: { 'check:spec-changes': GATE_STUBS[gate] } }, null, 2)}\n`,
@@ -939,7 +1014,12 @@ function fixtureSelfTest() {
9391014
const r = spawnSync(process.execPath, [fileURLToPath(import.meta.url), ...args], {
9401015
cwd: dir,
9411016
encoding: 'utf8',
942-
env: { ...process.env, OS_REGEN_GATE_CWD: dir },
1017+
env: {
1018+
...process.env,
1019+
OS_REGEN_GATE_CWD: dir,
1020+
OS_REGEN_GATE_LAUNCHER: `"${process.execPath}" "${launcherPath}"`,
1021+
...extraEnv,
1022+
},
9431023
});
9441024
return { code: r.status ?? 1, out: `${r.stdout ?? ''}${r.stderr ?? ''}` };
9451025
};
@@ -1087,6 +1167,47 @@ function fixtureSelfTest() {
10871167
check('a gate that RAN and failed is still `stale`, exit 1 — the grading is not a blanket pass',
10881168
genuine.code === 1 && / stale/.test(genuine.out) && !/PREREQUISITE NOT MET/.test(genuine.out));
10891169

1170+
// ── #15990: the grading may not be a function of the ambient launcher ───
1171+
// These stubs used to be spawned as `pnpm -s check:spec-changes` in a
1172+
// `mkdtemp` directory with no `packageManager` in scope, so the verdict was
1173+
// decided by whatever launcher happened to resolve there. Measured against
1174+
// CI own Corepack store: that is pnpm `latest` rather than the repo pin —
1175+
// pnpm 12, which rejects `-s` and exits 2 — so every stub collapsed to "the
1176+
// gate exited non-zero", which this script correctly grades `stale`. Every
1177+
// case above whose expected outcome is NOT stale went red, identically on an
1178+
// innocent PR and on `origin/main` push build 33981169123.
1179+
//
1180+
// The control is the diagnosis technique itself: a hostile `pnpm` FIRST on
1181+
// PATH, refusing the way pnpm 12 refuses. Each reading below is asserted
1182+
// EQUAL to the same call made without it — a launcher that is present and
1183+
// refuses has to be exactly as irrelevant as one that is absent. ⛔ Not a
1184+
// skip and not a retry: should a launcher ever re-enter this path, these go
1185+
// red and name the cause.
1186+
const hostileBin = join(launcherDir, 'hostile-bin');
1187+
mkdirSync(hostileBin, { recursive: true });
1188+
writeFileSync(join(hostileBin, 'pnpm'), [
1189+
'#!/bin/sh',
1190+
`echo "error: unexpected argument '-s' found" >&2`,
1191+
'exit 2',
1192+
'',
1193+
].join('\n'), { mode: 0o755 });
1194+
const hostilePath = { PATH: `${hostileBin}${delimiter}${process.env.PATH ?? ''}` };
1195+
1196+
// `genuine` immediately above is the reference reading for this one.
1197+
const hostileStale = runHook('stale', ['--pre-push'], hostilePath);
1198+
check('a hostile `pnpm` first on PATH leaves the `stale` reading identical',
1199+
hostileStale.code === genuine.code && hostileStale.out === genuine.out);
1200+
1201+
// The half a broken launcher actually erased: `clean` is the verdict it can
1202+
// never produce. Reference taken first, then the marker restored so both
1203+
// calls see the same input state.
1204+
const cleanRef = runHook('clean', ['--pre-push']);
1205+
const cleanRefCleared = !existsSync(marker);
1206+
writeFileSync(marker, `${pendingPath}\n`);
1207+
const hostileClean = runHook('clean', ['--pre-push'], hostilePath);
1208+
check(' …and a CLEAN gate still clears — the reading a broken launcher erased',
1209+
hostileClean.code === 0 && hostileClean.code === cleanRef.code
1210+
&& hostileClean.out === cleanRef.out && cleanRefCleared && !existsSync(marker));
10901211
// ── #15722, half 2 (WITHDRAWN, pinned): a fast-forward is not a merge ────
10911212
// The card's second half read the refusal as a predicate over paths changed
10921213
// between the previous and the new HEAD. Measured, it is not: the pending set
@@ -1110,6 +1231,7 @@ function fixtureSelfTest() {
11101231
afterFf.code === 0 && afterFf.out.trim() === '');
11111232
} finally {
11121233
rmSync(dir, { recursive: true, force: true });
1234+
rmSync(launcherDir, { recursive: true, force: true });
11131235
}
11141236
return results.every(Boolean);
11151237
}

0 commit comments

Comments
 (0)