From 56787766ed17cbb999681203045679183b36bde5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 02:19:57 +0000 Subject: [PATCH 1/3] fix(scripts): guard qa-rollup's --self-test dispatch against importers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `qa-rollup.mjs` exports bindings and tested `--self-test` BEFORE the entry guard, so the branch read the IMPORTER's argv: any tool that imported this module for its exports while carrying `--self-test` in its own argv ran qa-rollup's entire self-test inside itself. The leak is invisible to the two signals a caller usually has. The self-test does not exit on success, so the importer survives with status 0 and finishes its own work; the only trace is 82 bytes of foreign output on the importer's stdout. Measured with a probe that prints a sentinel AFTER the dynamic import and counts bytes that are not the sentinel: before LEAK scripts/qa/qa-rollup.mjs argv --self-test FOREIGN-BYTES=82 after CLEAN scripts/qa/qa-rollup.mjs argv --self-test Guard first, mode second. CLI behaviour is unchanged — `--self-test` output is byte-identical before and after (85 bytes, `cmp` clean), because the guard is true for every direct invocation. `check:entry-guard` names the file STALE once it is inert, so its KNOWN_IMPORT_UNSAFE line goes in this same commit: the ledger shrinks 5 -> 4 and the gate's own count moves to `108 of them inert on import (4 known-unsafe)`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015ahemw8RcTgqtxrj15PEZx --- scripts/check-entry-guard.mjs | 1 - scripts/qa/qa-rollup.mjs | 13 +++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/scripts/check-entry-guard.mjs b/scripts/check-entry-guard.mjs index 9fed65fe83..3858429a73 100644 --- a/scripts/check-entry-guard.mjs +++ b/scripts/check-entry-guard.mjs @@ -461,7 +461,6 @@ const KNOWN_IMPORT_UNSAFE = new Set([ 'scripts/check-changeset-no-major.mjs', 'scripts/check-empty-changeset.mjs', 'scripts/objectui-range.mjs', - 'scripts/qa/qa-rollup.mjs', 'scripts/ts-parse.mjs', ]); diff --git a/scripts/qa/qa-rollup.mjs b/scripts/qa/qa-rollup.mjs index a1719da9bd..d800c9bc09 100755 --- a/scripts/qa/qa-rollup.mjs +++ b/scripts/qa/qa-rollup.mjs @@ -1125,8 +1125,13 @@ async function selfTest() { ); } -if (process.argv.includes('--self-test')) { - await selfTest(); -} else if (isEntrypoint(import.meta.url)) { - await main(process.argv.slice(2)); +// The guard comes FIRST, then the mode. The other order — `--self-test` +// tested before `isEntrypoint` — read the IMPORTER's argv: a tool that imports +// this module for its exports while carrying `--self-test` in its own argv ran +// this file's self-test inside itself. That leak is invisible to exit status +// (this branch does not exit on success) and shows only as foreign output on +// the importer's stdout. +if (isEntrypoint(import.meta.url)) { + if (process.argv.includes('--self-test')) await selfTest(); + else await main(process.argv.slice(2)); } From e5b20a51c928257cc75910e4c72f90c2758d8955 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 02:21:33 +0000 Subject: [PATCH 2/3] fix(scripts): move objectui-range's --help out of module top level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `objectui-range.mjs` exports bindings and answered `-h`/`--help` from a bare top-level `if`, so the test read the IMPORTER's argv. An importer carrying either flag got this file's 4666-byte `//` header written to its stdout and then `process.exit(0)`. That is the worst-reading shape in this class: the importer's process ends mid-import with a SUCCESS status, so a caller holding only `result.status` cannot tell it from a clean import. Measured with a probe that prints a sentinel AFTER the dynamic import — the sentinel is what makes the two distinguishable, exit status alone is not: before LEAK argv --help NO-SENTINEL(status=0) FOREIGN-BYTES=4666 before LEAK argv -h NO-SENTINEL(status=0) FOREIGN-BYTES=4666 after CLEAN argv --help after CLEAN argv -h The help text is unchanged, and deliberately so: it is read back out of this file's own `//` lines, so a comment added at column 0 would rewrite it. The new rationale is a `/** */` block for that reason and the column-0 `//` count is still 79. All four CLI surfaces are byte-identical before and after — `--help` 4693 bytes, `-h` 4693, no-args 93 bytes on stderr with status 1, and `--self-test` 1578 bytes, every one `cmp`-clean on both streams. `check:entry-guard` names the file STALE once it is inert, so its KNOWN_IMPORT_UNSAFE line goes in this same commit: the ledger shrinks 4 -> 3. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015ahemw8RcTgqtxrj15PEZx --- scripts/check-entry-guard.mjs | 1 - scripts/objectui-range.mjs | 16 ++++++++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/scripts/check-entry-guard.mjs b/scripts/check-entry-guard.mjs index 3858429a73..7fe361c509 100644 --- a/scripts/check-entry-guard.mjs +++ b/scripts/check-entry-guard.mjs @@ -460,7 +460,6 @@ export function importUnsafeStatements(source) { const KNOWN_IMPORT_UNSAFE = new Set([ 'scripts/check-changeset-no-major.mjs', 'scripts/check-empty-changeset.mjs', - 'scripts/objectui-range.mjs', 'scripts/ts-parse.mjs', ]); diff --git a/scripts/objectui-range.mjs b/scripts/objectui-range.mjs index 03762eb10c..f662d862c9 100644 --- a/scripts/objectui-range.mjs +++ b/scripts/objectui-range.mjs @@ -95,7 +95,18 @@ const positional = argv.filter( const JSON_OUT = has('--json'); const SHOW_EXCLUDED = has('--all'); -if (has('-h') || has('--help')) { +/** + * Usage text, read back out of this file's own `//` lines so the two cannot + * drift. + * + * It is a FUNCTION rather than a top-level `if` because this module exports + * bindings. As a bare top-level statement the test read the IMPORTER's argv: + * an importer that happened to carry `-h` or `--help` got this header written + * to its stdout and then `process.exit(0)` — its process ended mid-import + * carrying a SUCCESS status, which no caller reading the status alone can + * tell apart from a clean import. + */ +function printHelp() { console.log( readFileSync(fileURLToPath(import.meta.url), 'utf8') .split('\n') @@ -103,7 +114,7 @@ if (has('-h') || has('--help')) { .map((l) => l.slice(3)) .join('\n'), ); - process.exit(0); + return 0; } function die(msg) { @@ -617,5 +628,6 @@ function selfTest() { } if (isEntrypoint(import.meta.url)) { + if (has('-h') || has('--help')) process.exit(printHelp()); process.exit(has('--self-test') ? selfTest() : main()); } From e253ab930bf2ae13ced9c57620be4efedbc5b51c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 02:24:20 +0000 Subject: [PATCH 3/3] fix(scripts): arm ts-parse's census report on first parse, not on import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ts-parse.mjs` registered its `OS_TOOLING_PARSE_CENSUS` exit report from a top-level `if (process.env...)`. This module is a LIBRARY — eleven gates in `scripts/` import it for its exports — so with the env set, that registration ran inside every one of those importers and wrote a line to a process whose only involvement was having loaded it. The entry-point guard is NOT the fix here, and that is the interesting half. As an entrypoint this module parses nothing, so `if (isEntrypoint(...))` would arm the census on the one run that has nothing to count and leave it silent on every run that does — import-safe and permanently empty. The condition had to MOVE, not acquire a guard: it is now armed, once, by the first parse through any of the three doors. One measurable consequence, stated rather than left to be discovered: a process that imports this module and never parses now prints nothing where it used to print `0 parse(s)`. Nothing read that line — `OS_TOOLING_PARSE_CENSUS` appears in no other file in the tree — and a census whose numerator is zero is the case with nothing to report. The self-test pins BOTH directions, so neither the leak nor the over-correction returns unnoticed: a child that parses still gets `[ts-parse census] 1 parse(s)` on stderr, and a child that only imports gets no census line. The child harness grew an optional env argument for it. Measured with a sentinel-after-import probe: before LEAK scripts/ts-parse.mjs env CENSUS=1 FOREIGN-BYTES=102 after CLEAN scripts/ts-parse.mjs env CENSUS=1 CLI: `node scripts/ts-parse.mjs` is byte-identical (45 bytes). `--self-test` is NOT, by design — it reports `30 cases pass` where it reported `28`, and the case count is the only difference in the line. `check:entry-guard` names the file STALE once it is inert, so its KNOWN_IMPORT_UNSAFE line goes in this same commit: the ledger shrinks 3 -> 2 and now holds only the two files fenced by the changesets-v3 epic. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015ahemw8RcTgqtxrj15PEZx --- scripts/check-entry-guard.mjs | 1 - scripts/ts-parse.mjs | 82 +++++++++++++++++++++++++++++------ 2 files changed, 68 insertions(+), 15 deletions(-) diff --git a/scripts/check-entry-guard.mjs b/scripts/check-entry-guard.mjs index 7fe361c509..72b03556e6 100644 --- a/scripts/check-entry-guard.mjs +++ b/scripts/check-entry-guard.mjs @@ -460,7 +460,6 @@ export function importUnsafeStatements(source) { const KNOWN_IMPORT_UNSAFE = new Set([ 'scripts/check-changeset-no-major.mjs', 'scripts/check-empty-changeset.mjs', - 'scripts/ts-parse.mjs', ]); /** diff --git a/scripts/ts-parse.mjs b/scripts/ts-parse.mjs index a2f11b3b2c..a49e92a954 100644 --- a/scripts/ts-parse.mjs +++ b/scripts/ts-parse.mjs @@ -130,8 +130,10 @@ * * With `OS_TOOLING_PARSE_CENSUS` set, this module prints how many parses ran, * over how many distinct file names, and how many refused, when the process - * exits. It only ADDS observation. There is deliberately no env var that turns - * the refusal off: a guard with a documented bypass is a guard that will be + * exits. It only ADDS observation. The report is armed by the FIRST PARSE, not + * by the import -- see `armCensusReport` for why the entry-point guard is the + * wrong shape for a library. There is deliberately no env var that turns the + * refusal off: a guard with a documented bypass is a guard that will be * bypassed. */ @@ -172,6 +174,45 @@ export function parseCensus() { }; } +let censusReportArmed = false; + +/** + * Arm the exit report ONCE, on the first parse rather than on the import. + * + * This module is a LIBRARY -- eleven gates in `scripts/` import it for its + * exports -- and the report used to be registered by a top-level + * `if (process.env.OS_TOOLING_PARSE_CENSUS)`. That ran inside every one of + * those importers: a module wrote to a process whose only involvement was + * having loaded it. + * + * The entry-point guard is NOT the fix here, and this is the interesting half. + * As an entrypoint this module parses nothing at all, so + * `if (isEntrypoint(...))` would arm the census on the one run that has + * nothing to count and leave it silent on every run that does -- a report that + * is now import-safe and also permanently empty. The condition had to move, + * not acquire a guard. + * + * So the trigger becomes "this module was USED" instead of "this module was + * LOADED", which is the event the number is about anyway. One measurable + * consequence, stated here rather than left to be discovered: a process that + * imports this module and never parses now prints nothing where it used to + * print `0 parse(s)`. Nothing read that line -- `OS_TOOLING_PARSE_CENSUS` + * appears in no other file in the tree -- and a census whose numerator is zero + * is the case with nothing to report. The self-test pins BOTH directions, so + * neither the leak nor the over-correction can come back unnoticed. + */ +function armCensusReport() { + if (censusReportArmed || !process.env.OS_TOOLING_PARSE_CENSUS) return; + censusReportArmed = true; + process.on('exit', () => { + const c = parseCensus(); + process.stderr.write( + `[ts-parse census] ${c.parses} parse(s) over ${c.files} distinct file name(s) ` + + `(${c.programs} program(s), ${c.transpiles} transpile(s)); ${c.refusals} refusal(s)\n`, + ); + }); +} + /** * The parse errors TypeScript recorded for `sourceFile`, as plain rows. * @@ -310,6 +351,7 @@ export function refusalReport(fileName, scriptKind, diagnostics) { * return: an unparseable source ends the process with {@link EXIT_UNPARSEABLE}. */ export function parseSourceFile(fileName, text, scriptKind) { + armCensusReport(); census.parses += 1; census.files.add(fileName); @@ -401,6 +443,7 @@ export function transpileRefusalReport(fileName, rows) { * return: unparseable sources end the process with {@link EXIT_UNPARSEABLE}. */ export function createProgramChecked(rootNames, options, host) { + armCensusReport(); const roots = [...rootNames]; census.programs += 1; census.parses += roots.length; @@ -441,6 +484,7 @@ export function createProgramChecked(rootNames, options, host) { * @returns {ts.TranspileOutput} Output emitted from a source that parsed. */ export function transpileChecked(fileName, text, transpileOptions = {}) { + armCensusReport(); census.transpiles += 1; census.parses += 1; census.files.add(fileName); @@ -460,16 +504,6 @@ export function transpileChecked(fileName, text, transpileOptions = {}) { return result; } -if (process.env.OS_TOOLING_PARSE_CENSUS) { - process.on('exit', () => { - const c = parseCensus(); - process.stderr.write( - `[ts-parse census] ${c.parses} parse(s) over ${c.files} distinct file name(s) ` - + `(${c.programs} program(s), ${c.transpiles} transpile(s)); ${c.refusals} refusal(s)\n`, - ); - }); -} - // --------------------------------------------------------------------------- // Self-test -- real child processes, because the refusal IS a process exit // --------------------------------------------------------------------------- @@ -508,7 +542,7 @@ export function selfTest() { // exercises the real export through the real module graph rather than a // re-implementation of it. const TS_URL = import.meta.resolve('typescript'); - const run = (body) => { + const run = (body, env) => { const probe = join(dir, `probe-${cases.length}-${Math.random().toString(36).slice(2)}.mjs`); writeFileSync( probe, @@ -516,7 +550,10 @@ export function selfTest() { + `import { parseSourceFile, parseCensus } from ${JSON.stringify(pathToFileURL(SELF).href)};\n` + `void ts;\n${body}\n`, ); - const r = spawnSync(process.execPath, [probe], { encoding: 'utf8' }); + const r = spawnSync(process.execPath, [probe], { + encoding: 'utf8', + env: env === undefined ? process.env : { ...process.env, ...env }, + }); rmSync(probe, { force: true }); return { status: r.status, out: (r.stdout || '').trim(), err: r.stderr || '' }; }; @@ -587,6 +624,23 @@ export function selfTest() { && counted.out === '{"parses":3,"programs":0,"transpiles":0,"files":2,"refusals":0}', JSON.stringify(counted)); + // -- and the report is armed by the first PARSE, not by the IMPORT. Both + // directions, because only the pair is a claim: a library that writes + // to your stderr because you imported it is the defect, and a census + // that can no longer report is the over-correction. ------------------- + const reported = run( + `parseSourceFile('a.ts', 'const a = 1;');\n`, + { OS_TOOLING_PARSE_CENSUS: '1' }, + ); + t('with the census env set, a run that PARSED still reports at exit', + reported.status === 0 + && /\[ts-parse census\] 1 parse\(s\) over 1 distinct file name\(s\)/.test(reported.err), + JSON.stringify(reported)); + const importedOnly = run(`void parseCensus();\n`, { OS_TOOLING_PARSE_CENSUS: '1' }); + t('…and a run that only IMPORTED this module writes no census line at all', + importedOnly.status === 0 && !importedOnly.err.includes('[ts-parse census]'), + JSON.stringify(importedOnly)); + // -- ts.createProgram: the syntax lives behind a SECOND call ------------- const PROGRAM_OPTIONS = `{ noLib: true, skipLibCheck: true, noEmit: true, types: [],`