From 7df682f28192439406b719b8f41a7bd8ab6255e6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 05:17:07 +0000 Subject: [PATCH] test(cli): mask comments in the nightly-tier readers of this package's own source WIP: the twelve nightly-tier test files that read `packages/cli`'s own source text now read it through the shared `maskComments`, and three byte-exact pins are rewritten to bind a derived property instead of a spelling. Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3 Co-authored-by: Claude --- ...build-json-failure-conversions.e2e.test.ts | 10 +- .../build-json-failure-warnings.e2e.test.ts | 10 +- .../test/cloud-login-json-ndjson.e2e.test.ts | 73 ++++++++-- .../test/diff-usage-error-stream.e2e.test.ts | 12 +- .../cli/test/helpers/config-miss-family.ts | 9 +- .../cli/test/json-stdout-purity.e2e.test.ts | 24 +++- .../cli/test/login-json-ndjson.e2e.test.ts | 73 ++++++++-- .../login-json-noninteractive.e2e.test.ts | 8 +- .../run-dev-unbuilt-workspace.e2e.test.ts | 8 +- ...e-app-anchored-optional-import.e2e.test.ts | 130 ++++++++++++++++-- ...idate-json-failure-conversions.e2e.test.ts | 23 +++- ...validate-json-failure-warnings.e2e.test.ts | 23 +++- 12 files changed, 352 insertions(+), 51 deletions(-) diff --git a/packages/cli/test/build-json-failure-conversions.e2e.test.ts b/packages/cli/test/build-json-failure-conversions.e2e.test.ts index 62012ce7c88..3af723d9a67 100644 --- a/packages/cli/test/build-json-failure-conversions.e2e.test.ts +++ b/packages/cli/test/build-json-failure-conversions.e2e.test.ts @@ -64,6 +64,7 @@ import { mkdtempSync, rmSync, writeFileSync, mkdirSync, readFileSync } from 'nod import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; +import { maskComments } from '../../../scripts/js-comment-mask.mjs'; import { childEnv } from './helpers/serve-process.js'; const HERE = resolve(fileURLToPath(import.meta.url), '..'); @@ -407,7 +408,14 @@ function payloadLiterals(src: string): string[] { } describe('#12125 — the contract is exhaustive over `compile.ts`, not just over the exits pinned above', () => { - const SRC = readFileSync(COMPILE_TS, 'utf8'); + // Comments are masked before a single thing is read off this file. A raw + // read cannot tell CODE from PROSE, and this package has been bitten in both + // directions: a docblock quoting a shape has satisfied a pin with no code + // behind it, and a docblock quoting one 281 lines above the code has broken a + // pin whose code never moved. `maskComments` BLANKS comment spans in place — + // spaces for text, newlines kept — so every byte offset, every line number and + // every brace-matching walk below reads exactly as it did on raw text (#18520). + const SRC = maskComments(readFileSync(COMPILE_TS, 'utf8')); it('the extractor produces a POSITIVE before its negative is trusted', () => { const SYNTHETIC = [ diff --git a/packages/cli/test/build-json-failure-warnings.e2e.test.ts b/packages/cli/test/build-json-failure-warnings.e2e.test.ts index 6d165f29bdb..b6a68ae82c4 100644 --- a/packages/cli/test/build-json-failure-warnings.e2e.test.ts +++ b/packages/cli/test/build-json-failure-warnings.e2e.test.ts @@ -87,6 +87,7 @@ import { mkdtempSync, rmSync, writeFileSync, mkdirSync, readFileSync } from 'nod import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; +import { maskComments } from '../../../scripts/js-comment-mask.mjs'; import { childEnv } from './helpers/serve-process.js'; const HERE = resolve(fileURLToPath(import.meta.url), '..'); @@ -482,7 +483,14 @@ function payloadLiterals(src: string): string[] { } describe('#11772 — the contract is exhaustive over `compile.ts`, not just over the exits pinned above', () => { - const SRC = readFileSync(COMPILE_TS, 'utf8'); + // Comments are masked before a single thing is read off this file. A raw + // read cannot tell CODE from PROSE, and this package has been bitten in both + // directions: a docblock quoting a shape has satisfied a pin with no code + // behind it, and a docblock quoting one 281 lines above the code has broken a + // pin whose code never moved. `maskComments` BLANKS comment spans in place — + // spaces for text, newlines kept — so every byte offset, every line number and + // every brace-matching walk below reads exactly as it did on raw text (#18520). + const SRC = maskComments(readFileSync(COMPILE_TS, 'utf8')); it('the extractor produces a POSITIVE before its negative is trusted', () => { // ⭐ A "no payload lacks `warnings`" pass is worthless from an instrument diff --git a/packages/cli/test/cloud-login-json-ndjson.e2e.test.ts b/packages/cli/test/cloud-login-json-ndjson.e2e.test.ts index 2c438726e64..bc0d596989a 100644 --- a/packages/cli/test/cloud-login-json-ndjson.e2e.test.ts +++ b/packages/cli/test/cloud-login-json-ndjson.e2e.test.ts @@ -70,6 +70,7 @@ import { mkdtempSync, rmSync, readFileSync, existsSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; +import { maskComments } from '../../../scripts/js-comment-mask.mjs'; import { childEnv } from './helpers/serve-process.js'; const HERE = resolve(fileURLToPath(import.meta.url), '..'); @@ -440,8 +441,42 @@ describe('os cloud login --json — the declared NDJSON stream (#6730)', () => { }); }); +/** + * The `{ … }` body of a declaration, brace-matched from its own `(`, as a span + * in the MASKED source — so a `{` inside a comment cannot close it early and + * the offsets are still the file's own line numbers. + */ +function bodySpan(src: string, declaration: RegExp): { start: number; end: number } | null { + const m = declaration.exec(src); + if (!m) return null; + let i = src.indexOf('(', m.index); + let depth = 0; + for (; i < src.length; i++) { + if (src[i] === '(') depth++; + else if (src[i] === ')') { + depth--; + if (depth === 0) break; + } + } + const open = src.indexOf('{', i); + if (open === -1) return null; + depth = 0; + for (let j = open; j < src.length; j++) { + if (src[j] === '{') depth++; + else if (src[j] === '}') { + depth--; + if (depth === 0) return { start: open, end: j }; + } + } + return null; +} + describe('the exception stays declared, not just implemented (#6730 ruling)', () => { - const cloudLoginSrc = () => readFileSync(CLOUD_LOGIN_SRC, 'utf-8'); + // Masked before anything is read: every case in this describe decides from + // the TEXT of `cloud/login.ts`, and a docblock that quotes `emitJson(` — the natural + // way to explain why one emitter exists — is indistinguishable from a call to + // it in a raw read (#18520). + const cloudLoginSrc = () => maskComments(readFileSync(CLOUD_LOGIN_SRC, 'utf-8')); it('routes every --json write through the single compact emitter', () => { // The contract is "one document per line" for the WHOLE command, so a new @@ -449,16 +484,38 @@ describe('the exception stays declared, not just implemented (#6730 ruling)', () // record on a path the e2e above does not drive. One emitter is what makes // that structurally impossible; this is the guard on the emitter. const src = cloudLoginSrc(); - const direct = src - .split('\n') - .map((line, i) => ({ line, n: i + 1 })) - .filter(({ line }) => /\bemitJson\s*\(/.test(line)) - .filter(({ line }) => !/^\s*await emitJson\(payload, exitCode, \{ compact: true \}\);$/.test(line)); + + // ⛔ This used to subtract ONE BYTE-EXACT LINE — `await emitJson(payload, + // exitCode, { compact: true });` — from the `emitJson(` line hits, and call + // anything left an offender. That binds the argument LIST, and binding an + // argument list is the defect this file's own tier cannot survive: a pull + // request that adds a parameter, renames `payload`, or simply wraps the call + // over two lines moves the spelling, the per-PR run never collects this file + // to say so, and the red arrives on `main` days later under whatever card + // happens to be open. Bind the PROPERTY the ruling actually made instead — + // ONE emitter — by partitioning the call sites against the emitter's own + // brace-matched body: outside must be empty, inside must not be, so neither + // half can pass by finding nothing (#18520). + const emitter = bodySpan(src, /async function emitRecord\s*\(/); + expect(emitter, '`cloud/login.ts` no longer declares the single `emitRecord` emitter').not.toBeNull(); + + const sites = [...src.matchAll(/\bemitJson\s*\(/g)]; + const lineOf = (at: number): number => src.slice(0, at).split('\n').length; + const outside = sites + .filter((m) => m.index < emitter!.start || m.index > emitter!.end) + .map((m) => { + const eol = src.indexOf('\n', m.index); + return `${lineOf(m.index)}: ${src.slice(m.index, eol === -1 ? undefined : eol).trim()}`; + }); + expect( - direct.map(({ n, line }) => `${n}: ${line.trim()}`), + outside, 'every --json write in cloud/login.ts must go through emitRecord()', ).toEqual([]); - expect(/async function emitRecord\(/.test(src)).toBe(true); + expect( + sites.length - outside.length, + 'the emitter itself no longer calls `emitJson`, so the partition above is vacuous', + ).toBeGreaterThanOrEqual(1); }); it('declares NDJSON in the --json flag help text', () => { diff --git a/packages/cli/test/diff-usage-error-stream.e2e.test.ts b/packages/cli/test/diff-usage-error-stream.e2e.test.ts index 4ce74314d6f..bda0ece82bd 100644 --- a/packages/cli/test/diff-usage-error-stream.e2e.test.ts +++ b/packages/cli/test/diff-usage-error-stream.e2e.test.ts @@ -63,6 +63,7 @@ import { mkdtempSync, rmSync, existsSync, readFileSync, readdirSync, statSync } import { tmpdir } from 'node:os'; import { join, resolve, relative, sep } from 'node:path'; import { fileURLToPath } from 'node:url'; +import { maskComments } from '../../../scripts/js-comment-mask.mjs'; import { childEnv } from './helpers/serve-process.js'; const HERE = resolve(fileURLToPath(import.meta.url), '..'); @@ -227,10 +228,17 @@ describe('no new command has grown a stdout write above its --json guard', () => const WRITES_TO_STDOUT = /\b(?:console\.log|printHeader|printStep|printInfo|printSuccess|printWarning|printError)\(/; + // ⛔ Every read below is MASKED. This scan is the exact shape that has already + // reported a change that never happened: it decides by LINE POSITION — `run()` + // above the first `flags.json` read — and a raw read cannot tell a docblock + // quoting `printSuccess(` from a call to it. A comment is enough to invent an + // offender here, and enough to hide one by pushing the guard's line above a + // write's. `maskComments` blanks spans in place, so the indices stay the + // file's own (#18520). function offenders(): string[] { const found: string[] = []; for (const abs of commandFiles(COMMANDS_DIR)) { - const lines = readFileSync(abs, 'utf-8').split('\n'); + const lines = maskComments(readFileSync(abs, 'utf-8')).split('\n'); if (!lines.some((l) => /\bjson:\s*Flags\.boolean\(/.test(l))) continue; const runIdx = lines.findIndex((l) => /async run\s*\(/.test(l)); if (runIdx < 0) continue; @@ -250,7 +258,7 @@ describe('no new command has grown a stdout write above its --json guard', () => // moved or renamed the commands directory would otherwise report "no // offenders" from an empty sweep. const withJson = commandFiles(COMMANDS_DIR).filter((abs) => - /\bjson:\s*Flags\.boolean\(/.test(readFileSync(abs, 'utf-8')), + /\bjson:\s*Flags\.boolean\(/.test(maskComments(readFileSync(abs, 'utf-8'))), ); expect(withJson.length).toBeGreaterThan(10); }); diff --git a/packages/cli/test/helpers/config-miss-family.ts b/packages/cli/test/helpers/config-miss-family.ts index 16475983c6a..fa22521aef5 100644 --- a/packages/cli/test/helpers/config-miss-family.ts +++ b/packages/cli/test/helpers/config-miss-family.ts @@ -26,6 +26,7 @@ import { readFileSync, readdirSync, statSync } from 'node:fs'; import { join, resolve, relative, sep } from 'node:path'; import { fileURLToPath } from 'node:url'; +import { maskComments } from '../../../../scripts/js-comment-mask.mjs'; const HERE = resolve(fileURLToPath(import.meta.url), '..'); @@ -135,7 +136,13 @@ function commandId(abs: string): string { */ export function discoverConfigMissFamily(): string[] { const files = commandFiles(COMMANDS_DIR); - const sources = new Map(files.map((abs) => [abs, readFileSync(abs, 'utf-8')])); + // Masked. Both halves below are regexes over command SOURCE, and both are + // satisfiable by prose: a docblock naming `json: Flags.boolean(` beside an + // import of `utils/config.js` invents a direct member, and a commented-out + // `export default class X extends Y` invents an alias. The discovery feeds a + // `toEqual` in two nightly-tier files, where a phantom member is a red no + // pull request can be shown (#18520). + const sources = new Map(files.map((abs) => [abs, maskComments(readFileSync(abs, 'utf-8'))])); const direct = new Set(); for (const [abs, src] of sources) { diff --git a/packages/cli/test/json-stdout-purity.e2e.test.ts b/packages/cli/test/json-stdout-purity.e2e.test.ts index d30b0f4d06e..7f339e88620 100644 --- a/packages/cli/test/json-stdout-purity.e2e.test.ts +++ b/packages/cli/test/json-stdout-purity.e2e.test.ts @@ -62,6 +62,7 @@ import { mkdtempSync, rmSync, writeFileSync, readFileSync, readdirSync, statSync import { tmpdir } from 'node:os'; import { join, resolve, relative, sep } from 'node:path'; import { fileURLToPath } from 'node:url'; +import { maskComments } from '../../../scripts/js-comment-mask.mjs'; import { childEnv } from './helpers/serve-process.js'; import { CONFIG_MISS_FAMILY, discoverConfigMissFamily } from './helpers/config-miss-family.js'; @@ -137,7 +138,12 @@ function commandFiles(dir: string): string[] { function discoverFamily(): string[] { const ids: string[] = []; for (const abs of commandFiles(COMMANDS_DIR)) { - const src = readFileSync(abs, 'utf-8'); + // Masked: both halves are text probes, and both are satisfiable by prose. A + // command whose only `bootSchemaStack(` is inside a docblock explaining that + // it does NOT boot one would join this family and be driven against boot + // diagnostics it never writes — a red in a tier no pull request collects, + // wearing this file's title rather than the docblock's (#18520). + const src = maskComments(readFileSync(abs, 'utf-8')); if (!src.includes('bootSchemaStack(')) continue; if (!/\bjson:\s*Flags\.boolean\(/.test(src)) continue; const rel = relative(COMMANDS_DIR, abs).replace(/\.ts$/, ''); @@ -233,7 +239,21 @@ describe('the family this contract has to hold across', () => { // assertion goes red. const preBoot = discoverConfigMissFamily(); expect(preBoot).toEqual(Object.keys(CONFIG_MISS_FAMILY).sort()); - expect(preBoot).toHaveLength(10); + + // ⛔ Not `toHaveLength(10)`. The line above already binds the SET, against a + // map a sibling nightly file drives member by member — that equality IS the + // contract, and a new member reddening it until it is driven is the point. + // The integer bound nothing that equality did not, and it bound it in a + // SECOND place that has to be hand-edited: two frozen numbers over one fact, + // in a tier where the author who moves the fact is never shown the failure. + // What it was load-bearing for is the vacuum the pair shares — a discovery + // that stops matching returns `[]`, and an emptied map would agree with it — + // so it stays as a FLOOR. Ten is the population the pre-boot family was + // measured over, not a count of today (#18520). + expect( + preBoot.length, + 'the pre-boot `--json` discovery reports fewer faces than the family was measured over', + ).toBeGreaterThanOrEqual(10); // The two families are NOT disjoint, and measuring that was worth more // than assuming it: `os migrate meta` is in both, legitimately and by diff --git a/packages/cli/test/login-json-ndjson.e2e.test.ts b/packages/cli/test/login-json-ndjson.e2e.test.ts index 3c77c541581..5713a8ec363 100644 --- a/packages/cli/test/login-json-ndjson.e2e.test.ts +++ b/packages/cli/test/login-json-ndjson.e2e.test.ts @@ -66,6 +66,7 @@ import { mkdtempSync, rmSync, readFileSync, existsSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; +import { maskComments } from '../../../scripts/js-comment-mask.mjs'; import { childEnv } from './helpers/serve-process.js'; const HERE = resolve(fileURLToPath(import.meta.url), '..'); @@ -388,8 +389,42 @@ describe('os login --json — the declared NDJSON stream (#6531)', () => { }); }); +/** + * The `{ … }` body of a declaration, brace-matched from its own `(`, as a span + * in the MASKED source — so a `{` inside a comment cannot close it early and + * the offsets are still the file's own line numbers. + */ +function bodySpan(src: string, declaration: RegExp): { start: number; end: number } | null { + const m = declaration.exec(src); + if (!m) return null; + let i = src.indexOf('(', m.index); + let depth = 0; + for (; i < src.length; i++) { + if (src[i] === '(') depth++; + else if (src[i] === ')') { + depth--; + if (depth === 0) break; + } + } + const open = src.indexOf('{', i); + if (open === -1) return null; + depth = 0; + for (let j = open; j < src.length; j++) { + if (src[j] === '{') depth++; + else if (src[j] === '}') { + depth--; + if (depth === 0) return { start: open, end: j }; + } + } + return null; +} + describe('the exception stays declared, not just implemented (#6531 ruling)', () => { - const loginSrc = () => readFileSync(LOGIN_SRC, 'utf-8'); + // Masked before anything is read: every case in this describe decides from + // the TEXT of `login.ts`, and a docblock that quotes `emitJson(` — the natural + // way to explain why one emitter exists — is indistinguishable from a call to + // it in a raw read (#18520). + const loginSrc = () => maskComments(readFileSync(LOGIN_SRC, 'utf-8')); it('routes every --json write through the single compact emitter', () => { // The contract is "one document per line" for the WHOLE command, so a new @@ -397,16 +432,38 @@ describe('the exception stays declared, not just implemented (#6531 ruling)', () // record on a path the e2e above does not drive. One emitter is what makes // that structurally impossible; this is the guard on the emitter. const src = loginSrc(); - const direct = src - .split('\n') - .map((line, i) => ({ line, n: i + 1 })) - .filter(({ line }) => /\bemitJson\s*\(/.test(line)) - .filter(({ line }) => !/^\s*await emitJson\(payload, exitCode, \{ compact: true \}\);$/.test(line)); + + // ⛔ This used to subtract ONE BYTE-EXACT LINE — `await emitJson(payload, + // exitCode, { compact: true });` — from the `emitJson(` line hits, and call + // anything left an offender. That binds the argument LIST, and binding an + // argument list is the defect this file's own tier cannot survive: a pull + // request that adds a parameter, renames `payload`, or simply wraps the call + // over two lines moves the spelling, the per-PR run never collects this file + // to say so, and the red arrives on `main` days later under whatever card + // happens to be open. Bind the PROPERTY the ruling actually made instead — + // ONE emitter — by partitioning the call sites against the emitter's own + // brace-matched body: outside must be empty, inside must not be, so neither + // half can pass by finding nothing (#18520). + const emitter = bodySpan(src, /async function emitRecord\s*\(/); + expect(emitter, '`login.ts` no longer declares the single `emitRecord` emitter').not.toBeNull(); + + const sites = [...src.matchAll(/\bemitJson\s*\(/g)]; + const lineOf = (at: number): number => src.slice(0, at).split('\n').length; + const outside = sites + .filter((m) => m.index < emitter!.start || m.index > emitter!.end) + .map((m) => { + const eol = src.indexOf('\n', m.index); + return `${lineOf(m.index)}: ${src.slice(m.index, eol === -1 ? undefined : eol).trim()}`; + }); + expect( - direct.map(({ n, line }) => `${n}: ${line.trim()}`), + outside, 'every --json write in login.ts must go through emitRecord()', ).toEqual([]); - expect(/async function emitRecord\(/.test(src)).toBe(true); + expect( + sites.length - outside.length, + 'the emitter itself no longer calls `emitJson`, so the partition above is vacuous', + ).toBeGreaterThanOrEqual(1); }); it('declares NDJSON in the --json flag help text', () => { diff --git a/packages/cli/test/login-json-noninteractive.e2e.test.ts b/packages/cli/test/login-json-noninteractive.e2e.test.ts index 18809de8ed8..13c4090d8b7 100644 --- a/packages/cli/test/login-json-noninteractive.e2e.test.ts +++ b/packages/cli/test/login-json-noninteractive.e2e.test.ts @@ -56,6 +56,7 @@ import { mkdtempSync, rmSync, readFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; +import { maskComments } from '../../../scripts/js-comment-mask.mjs'; import { childEnv } from './helpers/serve-process.js'; const HERE = resolve(fileURLToPath(import.meta.url), '..'); @@ -316,7 +317,12 @@ describe('the paths that must keep working (#6728 did not narrow them)', () => { }); describe('the refusal stays structural, not one call site (#6728)', () => { - const loginSrc = () => readFileSync(LOGIN_SRC, 'utf-8'); + // Masked: both cases below decide from the TEXT of `login.ts`. A docblock + // that quotes `rl.question(` without the abort signal — which is exactly what + // a comment explaining `askOrFailAtEof` would carry — reads as a live + // unsettleable prompt, and the flag-help extractor's non-greedy `})` stops at + // a `})` inside a comment. Neither failure is about the product (#18520). + const loginSrc = () => maskComments(readFileSync(LOGIN_SRC, 'utf-8')); it('asks no question that can outlive its input', () => { // `rl.question(...)` without the abort signal is the unsettleable form — diff --git a/packages/cli/test/run-dev-unbuilt-workspace.e2e.test.ts b/packages/cli/test/run-dev-unbuilt-workspace.e2e.test.ts index e1a844a5d24..48be2ea0d50 100644 --- a/packages/cli/test/run-dev-unbuilt-workspace.e2e.test.ts +++ b/packages/cli/test/run-dev-unbuilt-workspace.e2e.test.ts @@ -74,6 +74,7 @@ import { mkdtempSync, readFileSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; +import { maskComments } from '../../../scripts/js-comment-mask.mjs'; import { childEnv } from './helpers/serve-process.js'; const HERE = resolve(fileURLToPath(import.meta.url), '..'); @@ -450,7 +451,12 @@ describe('the mirror direction: a reader that is never coming back', () => { // nothing holding it is how a ceiling ends up sized around a bound that // moved. Same discipline as `INVOCATION_PREFIX` vs `CLI_NAME`: kept in // sync by a case, not by an import. - const declared = /const STDERR_DRAIN_STALL_MS = ([\d_]+);/.exec(readFileSync(SHIM, 'utf8'))?.[1]; + // Masked — `exec` takes the FIRST match, so a docblock carrying the old + // value (the natural way to record "this used to be 5_000") would be read + // as the shim's bound. The sibling pin over the other published entry, + // `published-entry-stderr-nonblocking.e2e.test.ts`, already masks + // `bin/run.js`; this is the same entry pair read the same way (#18520). + const declared = /const STDERR_DRAIN_STALL_MS = ([\d_]+);/.exec(maskComments(readFileSync(SHIM, 'utf8')))?.[1]; expect(declared, `no STDERR_DRAIN_STALL_MS declaration found in ${SHIM}`).toBeDefined(); expect(Number(String(declared).replaceAll('_', ''))).toBe(SHIM_DRAIN_STALL_MS); }); diff --git a/packages/cli/test/serve-app-anchored-optional-import.e2e.test.ts b/packages/cli/test/serve-app-anchored-optional-import.e2e.test.ts index 42444a1cd0d..b52724d8531 100644 --- a/packages/cli/test/serve-app-anchored-optional-import.e2e.test.ts +++ b/packages/cli/test/serve-app-anchored-optional-import.e2e.test.ts @@ -168,6 +168,7 @@ import { createRequire } from 'node:module'; import { tmpdir } from 'node:os'; import { dirname, join, resolve } from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; +import { maskComments } from '../../../scripts/js-comment-mask.mjs'; import { childEnv, portContentionError, portDriftError, randomPort } from './helpers/serve-process.js'; const HERE = dirname(fileURLToPath(import.meta.url)); @@ -574,35 +575,136 @@ describe('os serve → optional service resolution is anchored at the app (#1118 ); }); +/** + * The parameter list of a declaration, paren-matched from its own `(` so a + * default argument that itself takes parentheses cannot truncate it — which the + * obvious `\(([^)]*)\)` does, on the very declaration below. + */ +function paramsOf(code: string, declaration: RegExp): string | null { + const m = declaration.exec(code); + if (!m) return null; + const open = code.indexOf('(', m.index); + if (open === -1) return null; + let depth = 0; + for (let i = open; i < code.length; i++) { + if (code[i] === '(') depth++; + else if (code[i] === ')') { + depth--; + if (depth === 0) return code.slice(open + 1, i); + } + } + return null; +} + +/** Top-level commas only — a default like `= f(a, b)` is one parameter, not two. */ +function splitParams(params: string): string[] { + const out: string[] = []; + let depth = 0; + let start = 0; + for (let i = 0; i < params.length; i++) { + const c = params[i]; + if (c === '(' || c === '[' || c === '{') depth++; + else if (c === ')' || c === ']' || c === '}') depth--; + else if (c === ',' && depth === 0) { + out.push(params.slice(start, i).trim()); + start = i + 1; + } + } + const last = params.slice(start).trim(); + if (last) out.push(last); + return out; +} + describe('os serve → the anchor is wired where it cannot be forgotten', () => { - const SERVE_SOURCE = readFileSync(resolve(HERE, '../src/commands/serve.ts'), 'utf8'); + // ⛔ MASKED, and read as structure rather than as spelling. Every case here + // used to be a `toContain` over a whole statement of `serve.ts` — destructuring + // aliases, parameter list, return type annotation and all — in a file the + // nightly tier alone collects. That pairing is the defect this card names: the + // per-PR run cannot redden a byte-exact pin, so the pull request that reflows + // the statement is never told, and the failure surfaces on `main` days later + // wearing an unrelated card's title. A raw read cannot tell a docblock quoting + // the statement from the statement either, and `serve.ts` carries `{@link + // anchorServedApp}` and `{@link servedAppRootOrCwd}` in the prose above both. + // So: comments masked, and each case binds the PROPERTY it was written for + // (#18520). + const SERVE_CODE = maskComments(readFileSync(resolve(HERE, '../src/commands/serve.ts'), 'utf8')); it('resolves the config path and the app root in ONE call', () => { // If `run()` ever computes the config path itself again, the anchor becomes // a separate statement someone can write too late — or not at all — and the // behavioural tests above would be the only thing standing between that and - // a silent return to CWD-based resolution. - expect(SERVE_SOURCE).toContain( - 'const { configPath: absolutePath, configExists } = anchorServedApp(args.config!);', + // a silent return to CWD-based resolution. So the property is: exactly ONE + // site resolves it, and it is handed the raw config argument. The + // destructured local names are deliberately NOT bound — renaming + // `absolutePath` is not a defect, and pinning it is how a nightly-only pin + // goes red over a change that never touched the behaviour. + const sites = [...SERVE_CODE.matchAll(/\banchorServedApp\s*\(/g)].filter( + // The declaration is not a call site — same exclusion the per-PR sibling + // `src/commands/serve-cluster-host-resolution.test.ts` makes for + // `function importFromHost(`. + (m) => !/\bfunction\s+$/.test(SERVE_CODE.slice(Math.max(0, m.index - 16), m.index)), ); - expect(SERVE_SOURCE).not.toMatch( - /const absolutePath = path\.resolve\(process\.cwd\(\), args\.config!\)/, + expect( + sites.map((m) => SERVE_CODE.slice(0, m.index).split('\n').length), + 'the config path is resolved at a number of sites other than one', + ).toHaveLength(1); + + const argument = paramsOf(SERVE_CODE, /(? { - expect(SERVE_SOURCE).toContain( - 'function importFromHost(specifier: string, hostRoot: string = servedAppRootOrCwd())', - ); - expect(SERVE_SOURCE).toContain('const hostRoot = servedAppRootOrCwd();'); - expect(SERVE_SOURCE).toContain('const root = hostRoot ?? servedAppRootOrCwd();'); + // ⛔ The EXISTENCE and module scope of `importFromHost` are not re-pinned + // here: `src/commands/serve-cluster-host-resolution.test.ts` binds them + // per-PR — exactly one module-scope `function importFromHost(`, never a + // `const` — and that is the tier where a byte-exact spelling belongs, + // because the pull request that moves it is shown the red. What this case + // owns, and that file does not bind, is the DEFAULT. + const params = paramsOf(SERVE_CODE, /\bfunction importFromHost\s*\(/); + expect(params, 'serve.ts declares no `function importFromHost(`').not.toBeNull(); + const hostRootParam = splitParams(params ?? '').find((x) => /^hostRoot\b/.test(x)); + expect( + hostRootParam, + '`importFromHost` no longer takes a `hostRoot` parameter at all', + ).toBeDefined(); + expect( + hostRootParam, + '`importFromHost`\'s `hostRoot` defaults to something other than the served app', + ).toMatch(/=\s*servedAppRootOrCwd\(\)\s*$/); + + // And every place a host root is BOUND resolves through the same function. + // Derived, not listed: the two statements this replaced were pinned by their + // exact text, so a reflow read as a regression. + const bindings = [...SERVE_CODE.matchAll(/\b(?:const|let)\s+(hostRoot|root)\s*=\s*([^;\n]+)/g)] + .filter((m) => m[1] === 'hostRoot' || /\bhostRoot\b/.test(m[2])); + expect( + bindings.length, + 'no host-root binding found at all — the partition below would be vacuous', + ).toBeGreaterThanOrEqual(2); + expect( + bindings + .filter((m) => !/\bservedAppRootOrCwd\(\)/.test(m[2])) + .map((m) => `${SERVE_CODE.slice(0, m.index).split('\n').length}: ${m[0].trim()}`), + 'a host root is bound without resolving through servedAppRootOrCwd()', + ).toEqual([]); }); it('reads the app root through a function, never a module-scope copy', () => { // A `const` captured at module-evaluation time would freeze the pre-boot // answer (`process.cwd()`) into every call site, which is the defect wearing - // a different hat. - expect(SERVE_SOURCE).toMatch(/^function servedAppRootOrCwd\(\): string \{$/m); - expect(SERVE_SOURCE).not.toMatch(/\b(?:const|let|var)\s+servedAppRootOrCwd\b/); + // a different hat. The return type annotation and the brace that used to be + // part of this pattern are not that defect, so they are no longer bound. + expect( + [...SERVE_CODE.matchAll(/^function servedAppRootOrCwd\s*\(/gm)], + 'servedAppRootOrCwd is not declared exactly once at module scope', + ).toHaveLength(1); + expect(SERVE_CODE).not.toMatch(/^[ \t]+function servedAppRootOrCwd\s*\(/m); + expect(SERVE_CODE).not.toMatch(/\b(?:const|let|var)\s+servedAppRootOrCwd\b/); }); }); diff --git a/packages/cli/test/validate-json-failure-conversions.e2e.test.ts b/packages/cli/test/validate-json-failure-conversions.e2e.test.ts index 74a50e94b5e..3336f8580c1 100644 --- a/packages/cli/test/validate-json-failure-conversions.e2e.test.ts +++ b/packages/cli/test/validate-json-failure-conversions.e2e.test.ts @@ -92,6 +92,7 @@ import { mkdtempSync, rmSync, writeFileSync, mkdirSync, readFileSync } from 'nod import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; +import { maskComments } from '../../../scripts/js-comment-mask.mjs'; import { childEnv } from './helpers/serve-process.js'; const HERE = resolve(fileURLToPath(import.meta.url), '..'); @@ -393,7 +394,11 @@ function payloadLiterals(src: string): string[] { } describe('#12125 — the contract is exhaustive over `validate.ts`, not just over the exits pinned above', () => { - const SRC = readFileSync(VALIDATE_TS, 'utf8'); + // Comments masked before anything is read off this file — see the paragraph + // in the exhaustiveness case below for why, and note that `maskComments` + // blanks spans IN PLACE, so `payloadLiterals`'s brace arithmetic and the + // `indexOf` order case further down read the same offsets they always did. + const SRC = maskComments(readFileSync(VALIDATE_TS, 'utf8')); it('the extractor produces a POSITIVE before its negative is trusted', () => { // ⭐ A "no payload lacks `conversions`" pass is worthless from an instrument @@ -433,11 +438,17 @@ describe('#12125 — the contract is exhaustive over `validate.ts`, not just ove // reddens HERE instead of quietly turning that negative into a vacuous // pass over fewer exits than the file has. // - // Both sides read RAW source, which is what keeps them symmetric: a - // commented-out `await emitJson(` is counted by the extractor and by the - // pattern alike. The one asymmetric case — prose naming the call with its - // paren but no `await` — reddens, and that is the accepted price for not - // importing a comment masker into this file. + // Both sides read the MASKED source, which is what keeps them symmetric AND + // keeps prose out of both. This paragraph used to record the opposite — + // both sides raw, with "prose naming the call with its paren but no + // `await`" accepted as the price of not importing a masker. That price was + // the whole of a nightly red once already, on a print-ORDER pin in this + // same package, and it is not a price a pin only a cron can read should + // pay: the author who writes the docblock cannot be shown the failure + // (#18520). Masking BLANKS spans in place, so the extractor's brace walk + // and this pattern still agree byte for byte — measured on the commit that + // landed this, raw and masked give the same 7 exits and the same 7 call + // sites, so the conversion moved no verdict, only the future hazard. const callSites = SRC.match(/\bemitJson\s*\(/g) ?? []; expect( literals, diff --git a/packages/cli/test/validate-json-failure-warnings.e2e.test.ts b/packages/cli/test/validate-json-failure-warnings.e2e.test.ts index 972a9f5d1c6..c59f6e4fee9 100644 --- a/packages/cli/test/validate-json-failure-warnings.e2e.test.ts +++ b/packages/cli/test/validate-json-failure-warnings.e2e.test.ts @@ -113,6 +113,7 @@ import { mkdtempSync, rmSync, writeFileSync, mkdirSync, readFileSync } from 'nod import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; +import { maskComments } from '../../../scripts/js-comment-mask.mjs'; import { childEnv } from './helpers/serve-process.js'; const HERE = resolve(fileURLToPath(import.meta.url), '..'); @@ -478,7 +479,11 @@ function payloadLiterals(src: string): string[] { } describe('#12047 — the contract is exhaustive over `validate.ts`, not just over the exits pinned above', () => { - const SRC = readFileSync(VALIDATE_TS, 'utf8'); + // Comments masked before anything is read off this file — see the paragraph + // in the exhaustiveness case below for why, and note that `maskComments` + // blanks spans IN PLACE, so `payloadLiterals`'s brace arithmetic and the + // `indexOf` order case further down read the same offsets they always did. + const SRC = maskComments(readFileSync(VALIDATE_TS, 'utf8')); it('the extractor produces a POSITIVE before its negative is trusted', () => { // ⭐ A "no payload lacks `warnings`" pass is worthless from an instrument @@ -518,11 +523,17 @@ describe('#12047 — the contract is exhaustive over `validate.ts`, not just ove // reddens HERE instead of quietly turning that negative into a vacuous // pass over fewer exits than the file has. // - // Both sides read RAW source, which is what keeps them symmetric: a - // commented-out `await emitJson(` is counted by the extractor and by the - // pattern alike. The one asymmetric case — prose naming the call with its - // paren but no `await` — reddens, and that is the accepted price for not - // importing a comment masker into this file. + // Both sides read the MASKED source, which is what keeps them symmetric AND + // keeps prose out of both. This paragraph used to record the opposite — + // both sides raw, with "prose naming the call with its paren but no + // `await`" accepted as the price of not importing a masker. That price was + // the whole of a nightly red once already, on a print-ORDER pin in this + // same package, and it is not a price a pin only a cron can read should + // pay: the author who writes the docblock cannot be shown the failure + // (#18520). Masking BLANKS spans in place, so the extractor's brace walk + // and this pattern still agree byte for byte — measured on the commit that + // landed this, raw and masked give the same 7 exits and the same 7 call + // sites, so the conversion moved no verdict, only the future hazard. const callSites = SRC.match(/\bemitJson\s*\(/g) ?? []; expect( literals,