diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index a09f8d8b7e..767ace7801 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -788,6 +788,49 @@ jobs: node scripts/check-self-test-wired.mjs --self-test node scripts/check-self-test-wired.mjs + # Self-test output vs. the runner's command parser (#11886). Sibling of + # the step above and sharing its population: that one asks whether CI RUNS + # each self-test, this one asks what each self-test PRINTS while it does. + # + # The defect it closes was live on every PR in this repo. A line of + # `scripts/pm/ci-failure.mjs`'s self-test prose named a workflow-command + # token literally, inside backticks, as documentation of what that tool + # anchors on. The runner does not know a token is being quoted: it parsed + # it and minted `annotation_level: failure` on a check run whose + # conclusion was SUCCESS. On `d63b01436` that artifact was the ONLY + # annotation the `Lint & Repo Gates` run carried. A failure-level + # annotation on a green required check is a claim that something failed, + # and a repo that mints one on every PR is teaching its readers to skim + # exactly the surface other gates report findings into. + # + # ⚠️ INVISIBLE TO EVERY LOCAL RUN, which is the whole reason it is a gate: + # the text is ordinary prose on disk and in a terminal, and only a runner + # transforms it. It was found by reading one job's annotations through the + # API, not by anything in this file. + # + # Two facts were measured on real runs and the gate's shape follows them: + # the legacy `##[...]` form is parsed ANYWHERE in a printed line (the + # measured token sat at column 18 and was still consumed), while the + # `::...::` form is parsed only at LINE START (two mid-sentence ones print + # on every PR from `check-prerelease-pin-watch` and mint nothing). So the + # first detector is unanchored and the second is anchored — flagging inert + # prose would be a false positive the next author would be right to delete. + # + # It RUNS the self-tests rather than reasoning about their source, because + # "is this token printed?" is only answerable by printing it. A static + # prefilter picks which ones to run — over-selecting freely, since the + # verdict never comes from it — which is what keeps this step at ~14 s + # instead of the ~4 min the whole population costs. + # + # Invoked as `node scripts/…` rather than through a `pnpm check:*` alias: + # see the GATE INVOCATION IDIOM note at the top of this file. Reads + # `scripts/` and `.github/workflows/` off disk and spawns the selected + # self-tests; no network. + - name: Self-test workflow-command gate + run: | + node scripts/check-self-test-workflow-commands.mjs --self-test + node scripts/check-self-test-workflow-commands.mjs + # Verify-lock entry-point self-test (#9661). `scripts/pm/os-verify-lock.sh` # is the ONE way an agent takes the container's shared heavy-verify lock, # and it is the enforcement of a rule that used to live only in prose: the diff --git a/scripts/check-self-test-workflow-commands.mjs b/scripts/check-self-test-workflow-commands.mjs new file mode 100644 index 0000000000..e9047b10d9 --- /dev/null +++ b/scripts/check-self-test-workflow-commands.mjs @@ -0,0 +1,399 @@ +#!/usr/bin/env node +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Self-test output vs. the Actions runner's command parser (#11886). + * + * node scripts/check-self-test-workflow-commands.mjs + * node scripts/check-self-test-workflow-commands.mjs --self-test + * + * ## The defect, measured rather than supposed + * + * `scripts/pm/ci-failure.mjs --self-test` printed a line of prose that NAMES a + * workflow-command token, inside backticks, as documentation of what the tool + * anchors on. The runner does not know the token is being quoted. It parsed it + * and minted a real annotation: + * + * check-run 97678882948, `Lint & Repo Gates` on `d63b01436`, conclusion + * SUCCESS, via `GET /repos/{owner}/{repo}/check-runs/{id}/annotations`: + * + * annotation_level = failure path = .github line = 32 title = "" + * message = "` in it is labelled a window rather than an anchor. And a `fix`" + * + * That was the ONLY annotation on the run. A failure-level annotation on a + * green required check is a claim that something failed; nothing did. The step + * has no `if:` and the workflow has no `paths:` filter, so it was minted on + * every pull request in the repo. + * + * ## Two measurements that decide this gate's shape + * + * **1. The legacy `##[...]` form is parsed ANYWHERE in a line, not only at the + * start.** The source line begins ` tail with no \`` and the token sits at + * column 18; the runner still consumed it, dropped everything before it, and + * took the remainder of the line as the message (which is why the annotation + * above starts mid-sentence, on a stray backtick). So a `##[` cannot be made + * safe by indenting it or by burying it in prose. + * + * **2. The `::...::` form is parsed only at LINE START.** Measured against the + * same population: `scripts/check-prerelease-pin-watch.mjs --self-test` prints + * two lines carrying `::error::` and `::warning::` mid-sentence on every PR, + * and its check run carries zero annotations. Those two lines are inert where + * they sit and one re-indentation away from not being; this gate reddens at the + * moment they move, which is the moment they start lying. + * + * ⚠️ Neither fact is inferable from a local run — the transformation happens on + * a runner and nowhere else. That is exactly why the property needs a gate + * rather than a reviewer. + * + * ## Why the population is "the self-tests CI runs" + * + * A self-test's output is deterministic and input-independent by construction + * (that is what makes it a self-test), so "what will this print on a runner" is + * answerable HERE, with no runner and no network. A production gate run prints + * findings that depend on the tree, and its deliberate `::warning::` emissions + * are the annotation surface working as intended — a different question, and + * not this gate's. + * + * Membership is not re-derived: it is imported from + * `scripts/check-self-test-wired.mjs`, which already owns the answer to "which + * scripts does CI run that ship a `--self-test`". One definition, two gates. + * + * ## The verdict comes from real output, never from a rule about source + * + * The static scan below only SELECTS which self-tests to run. Whether a token + * is printed is then answered by printing it — so the classic trap of this + * repo (a matching rule that silently stops matching, with an empty finding set + * as its fixed point) has no purchase on the verdict. Over-selection costs a + * subprocess and nothing else; the whole population is ~4 minutes and the + * selected set is ~14 seconds, which is the only reason the filter exists. + * + * Its stated blind spot: a script that BUILDS the token out of pieces + * (`'##' + '[error]'`) and prints the join. Nothing in this tree does, and the + * defect class is prose naming a token, which is written whole. A miss here + * leaves the tree exactly where it is today rather than degrading it. + * + * ⚠️ Note for anyone tempted to "break the literal" in SOURCE to fix a finding: + * splitting a JS string changes nothing. The runner parses the PRINTED LINE, so + * the printed bytes are what must change. + * + * ## No registered-command list, deliberately + * + * The set of names the runner honours (`error`, `warning`, `notice`, `group`, + * `add-mask`, ...) is GitHub's to change, and a stale copy of it here would + * UNDER-match in silence — a green gate over a token it no longer recognises. + * The detectors match the command SHAPE instead, which over-matches in the safe + * direction. Measured cost of that choice on this tree: zero false positives + * across the whole population. + * + * ## Refusals, never quiet passes (#4690) + * + * An empty population, an empty candidate set, a candidate that cannot be + * spawned, one that times out, and one that prints nothing at all are each + * exit 1 naming what could not be read. "Nothing to check" and "the walk found + * nothing" are the two readings this gate is built to keep apart. + */ + +import { readFileSync, readdirSync, existsSync, statSync } from 'node:fs'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { spawnSync } from 'node:child_process'; + +import { isEntrypoint } from './invoked-as.mjs'; +import { collectInvocations, carriesSelfTest, codeOf } from './check-self-test-wired.mjs'; + +const ROOT = join(fileURLToPath(new URL('.', import.meta.url)), '..'); +const WORKFLOW_DIR = '.github/workflows'; +const SCRIPT_EXT = /\.(mjs|mts|js|sh)$/; + +/** How long one self-test may take before the gate refuses rather than guesses. */ +const TIMEOUT_MS = 120_000; + +// --------------------------------------------------------------------------- +// Detectors — run against REAL OUTPUT, one line at a time +// --------------------------------------------------------------------------- + +/** + * The legacy form. Parsed anywhere in the line (measurement 1 in the header), + * so this is deliberately not anchored. + */ +export const V1_COMMAND = /##\[[A-Za-z][\w-]*(?:[ \t][^\]\n]*)?\]/; + +/** + * The current form. Parsed only at line start (measurement 2), so this IS + * anchored — a mid-sentence `::warning::` is inert and flagging it would be a + * false positive the next author would be right to delete. + */ +export const V2_COMMAND = /^[ \t]*::[A-Za-z][\w-]*[^\n]*?::/; + +/** + * Every line of `text` the Actions runner would parse as a workflow command. + * + * @param {string} text combined stdout + stderr of one self-test + * @returns {{line: number, form: 'legacy ##[...]' | 'current ::...::', text: string}[]} + */ +export function scanOutput(text) { + const findings = []; + text.split('\n').forEach((line, i) => { + if (V1_COMMAND.test(line)) findings.push({ line: i + 1, form: 'legacy ##[...]', text: line }); + else if (V2_COMMAND.test(line)) findings.push({ line: i + 1, form: 'current ::...::', text: line }); + }); + return findings; +} + +// --------------------------------------------------------------------------- +// Prefilter — selects which self-tests to run; never decides a verdict +// --------------------------------------------------------------------------- + +/** A `##[` in code can reach any column of any printed line, so any occurrence selects. */ +const CANDIDATE_V1 = /##\[/; + +/** + * A `::` command can only matter if it can land at the START of a printed line, + * which in source means it follows a string delimiter, an escaped newline, or a + * real newline inside a template literal. `Time::HiRes::time()` — a Perl + * namespace inside a shell string — is the measured reason this arm is not just + * `/::[A-Za-z]/`: that spelling selected a 65-second self-test with nothing to + * find in it. + */ +const CANDIDATE_V2 = /(?:^|["'`]|\\n)[ \t]*::[A-Za-z][\w-]/m; + +/** + * Could this script's CODE (comments masked — prose never prints) print a + * workflow command? + * + * @param {string} relPath + * @param {string} source + */ +export function isCandidate(relPath, source) { + const code = codeOf(relPath, source); + return CANDIDATE_V1.test(code) || CANDIDATE_V2.test(code); +} + +// --------------------------------------------------------------------------- + +function walkScripts(dir, out = []) { + for (const entry of readdirSync(dir).sort()) { + const abs = join(dir, entry); + if (statSync(abs).isDirectory()) walkScripts(abs, out); + else if (SCRIPT_EXT.test(entry)) out.push(abs.slice(ROOT.length + 1).split('\\').join('/')); + } + return out; +} + +/** + * Run one self-test and hand back everything it said. + * + * @param {string} relPath + */ +export function runSelfTest(relPath) { + const abs = join(ROOT, relPath); + const argv = relPath.endsWith('.sh') ? ['bash', [abs, '--self-test']] : ['node', [abs, '--self-test']]; + const r = spawnSync(argv[0], argv[1], { + cwd: ROOT, + encoding: 'utf8', + timeout: TIMEOUT_MS, + maxBuffer: 64 * 1024 * 1024, + }); + return { + spawnError: r.error ? String(r.error.message) : null, + timedOut: r.signal === 'SIGTERM' && Boolean(r.error), + output: `${r.stdout ?? ''}\n${r.stderr ?? ''}`, + bytes: (r.stdout ?? '').length + (r.stderr ?? '').length, + }; +} + +function main() { + const scriptsDir = join(ROOT, 'scripts'); + const workflowDir = join(ROOT, WORKFLOW_DIR); + const refuse = (message) => { + console.error(`\ncheck-self-test-workflow-commands: REFUSED — ${message}\n`); + process.exit(1); + }; + if (!existsSync(scriptsDir)) refuse('scripts/ does not exist, so nothing was read (#4690).'); + if (!existsSync(workflowDir)) refuse(`${WORKFLOW_DIR} does not exist, so nothing was read (#4690).`); + + const files = walkScripts(scriptsDir); + if (files.length === 0) refuse('the walk over scripts/ found no files — a broken walk, not a clean tree (#4690).'); + + const sources = new Map(); + const carriers = new Set(); + for (const relPath of files) { + let source; + try { + source = readFileSync(join(ROOT, relPath), 'utf8'); + } catch { + refuse(`${relPath} could not be read.`); + } + sources.set(relPath, source); + if (carriesSelfTest(relPath, source)) carriers.add(relPath); + } + if (carriers.size === 0) { + refuse('no script under scripts/ carries a `--self-test` — this tree has dozens, so the reader is broken (#4690).'); + } + + const workflowNames = readdirSync(workflowDir).filter((f) => /\.ya?ml$/.test(f)).sort(); + if (workflowNames.length === 0) refuse(`${WORKFLOW_DIR} holds no workflow files (#4690).`); + const workflows = workflowNames.map((name) => ({ name, text: readFileSync(join(workflowDir, name), 'utf8') })); + + let pkgScripts = {}; + try { + pkgScripts = JSON.parse(readFileSync(join(ROOT, 'package.json'), 'utf8')).scripts ?? {}; + } catch { + refuse('the root package.json could not be read or parsed.'); + } + + const { named } = collectInvocations(workflows, pkgScripts); + if (named.size === 0) refuse('no workflow names any scripts/ file — the workflow reader is broken (#4690).'); + + const population = [...carriers].filter((s) => named.has(s)).sort(); + if (population.length === 0) { + refuse('no script CI runs ships a `--self-test` — the population reader is broken, not the tree (#4690).'); + } + + const candidates = population.filter((s) => isCandidate(s, sources.get(s))); + if (candidates.length === 0) { + refuse( + 'not one of the self-tests CI runs even MENTIONS a workflow-command token, which this tree ' + + 'contradicts (`scripts/pm/ci-failure.mjs` documents what it anchors on). The prefilter is ' + + 'broken, and a broken prefilter reads exactly like a clean tree (#4690).', + ); + } + + const findings = []; + for (const relPath of candidates) { + const r = runSelfTest(relPath); + if (r.spawnError) refuse(`${relPath} --self-test could not be run: ${r.spawnError}`); + if (r.timedOut) refuse(`${relPath} --self-test did not finish within ${TIMEOUT_MS / 1000}s, so its output was never read.`); + if (r.bytes === 0) { + refuse( + `${relPath} --self-test printed nothing at all, so this gate read no output from it. ` + + 'An unread self-test is not a clean one (#4690).', + ); + } + for (const f of scanOutput(r.output)) findings.push({ script: relPath, ...f }); + } + + const scope = + ` scope: ${population.length} script(s) CI runs ship a \`--self-test\`; ${candidates.length} mention a ` + + 'workflow-command token in code (comments masked) and were RUN, and their real stdout+stderr was scanned.'; + + if (findings.length > 0) { + console.error(`\ncheck-self-test-workflow-commands: ${findings.length} finding(s)\n`); + for (const f of findings) { + console.error( + ` [${f.form}] ${f.script} --self-test, output line ${f.line}:\n` + + ` ${f.text.trim()}\n` + + ' The Actions runner parses this and mints a real annotation on a GREEN check run.\n' + + ' Change the PRINTED BYTES — splitting the literal in source changes nothing, because\n' + + ' the runner reads the printed line. Reword the prose; the file\'s own comments can\n' + + ' spell the token freely, because comments are never printed.\n', + ); + } + console.error(`${scope}\n`); + process.exit(1); + } + + console.log( + '✓ check-self-test-workflow-commands: no self-test CI runs prints a line the Actions runner ' + + 'would parse as a workflow command.', + ); + console.log(scope); +} + +// --------------------------------------------------------------------------- +// --self-test +// +// ⚠️ Nothing this function prints may itself carry a parseable token — this +// gate is inside its own population. The adversarial fixtures below are built +// from pieces at runtime for exactly that reason, and the failure path prints +// only a label, never the offending line. +// --------------------------------------------------------------------------- + +function selfTest() { + const failures = []; + const ok = (cond, label) => { + if (!cond) failures.push(label); + }; + const HASHES = '#'.repeat(2); + const COLONS = ':'.repeat(2); + const v1 = (name) => `${HASHES}[${name}]`; + const v2 = (name) => `${COLONS}${name}${COLONS}`; + + // ── The measured defect itself, reproduced from the real annotation ────── + { + const real = ` tail with no \`${v1('error')}\` in it is labelled a window rather than an anchor. And a \`fix\``; + const got = scanOutput(real); + ok(got.length === 1, 'the measured production line was not flagged at all'); + ok(got[0]?.form === 'legacy ##[...]', 'the measured production line was flagged as the wrong form'); + } + + // ── Measurement 1: the legacy form is parsed ANYWHERE in a line ────────── + ok(scanOutput(`${v1('error')}boom`).length === 1, 'a legacy command at column 0 was not flagged'); + ok(scanOutput(`prose about ${v1('error')} here`).length === 1, 'a legacy command MID-LINE was not flagged — the measured defect walks straight through'); + ok(scanOutput(` ${v1('warning')} x`).length === 1, 'an indented legacy command was not flagged'); + ok(scanOutput(`x ${v1('group')} y`).length === 1, 'a legacy `group` was not flagged'); + + // ── Measurement 2: the current form is parsed only at LINE START ───────── + ok(scanOutput(`${v2('error')}boom`).length === 1, 'a current-form command at line start was not flagged'); + ok(scanOutput(`${COLONS}error file=a.ts,line=1${COLONS}boom`).length === 1, 'a current-form command WITH PROPERTIES was not flagged'); + ok( + scanOutput(` the quiet run emits no ${v2('error')} and no ${v2('warning')}`).length === 0, + 'a mid-sentence current-form token was flagged — measured inert, and this false positive would be deleted by the next author', + ); + + // ── Innocent output must stay innocent, or the gate gets weakened ──────── + ok(scanOutput('✓ check-foo: 12 file(s) scanned, nothing to report').length === 0, 'an ordinary success line was flagged'); + ok(scanOutput(' see docs/x.md ## Heading and packages/spec [ok]').length === 0, 'a markdown heading plus a bracket was flagged'); + ok(scanOutput(`Time${COLONS}HiRes${COLONS}time()`).length === 0, 'a Perl namespace was flagged as a workflow command'); + ok(scanOutput('').length === 0, 'empty output produced a finding'); + + // ── The prefilter selects on CODE, never on prose ──────────────────────── + ok( + !isCandidate('scripts/x.mjs', `// the runner spells it ${v1('error')}\nconst a = 1;\n`), + 'a token that exists only in a JS comment selected the script — comments are never printed', + ); + ok( + isCandidate('scripts/x.mjs', `console.log('${v1('error')}');\n`), + 'control — the same literal in CODE must select, or the case above proves nothing', + ); + ok( + isCandidate('scripts/x.mjs', `console.log('${v2('warning')}x');\n`), + 'a current-form literal after a quote did not select', + ); + ok( + !isCandidate('scripts/x.sh', `raw="$(perl -MTime${COLONS}HiRes -e 'Time${COLONS}HiRes${COLONS}time()')"\n`), + 'a Perl namespace selected a shell script — the measured 65-second false positive this arm exists to drop', + ); + ok( + !isCandidate('scripts/x.mjs', 'const a = 1;\n'), + 'a script with no token at all selected', + ); + + // ── End to end: the real tree's real defect site, run for real ─────────── + { + const target = 'scripts/pm/ci-failure.mjs'; + const source = existsSync(join(ROOT, target)) ? readFileSync(join(ROOT, target), 'utf8') : null; + ok(source !== null, `${target} is missing, so the end-to-end case verified nothing`); + ok(source !== null && isCandidate(target, source), `${target} is no longer selected by the prefilter — the end-to-end case below would run nothing`); + const r = runSelfTest(target); + ok(r.spawnError === null, `${target} --self-test could not be spawned: ${r.spawnError}`); + ok(r.bytes > 0, `${target} --self-test printed nothing, so nothing was scanned`); + ok(scanOutput(r.output).length === 0, `${target} --self-test still prints a line the runner would parse — this is the #11886 defect, live`); + } + + if (failures.length > 0) { + console.error('check-self-test-workflow-commands --self-test FAILED:'); + for (const f of failures) console.error(` - ${f}`); + process.exit(1); + } + console.log( + 'check-self-test-workflow-commands --self-test: both measured parse rules pinned (legacy form ' + + 'anywhere in a line, current form only at line start), the innocent-output and Perl-namespace ' + + 'cases, the comment mask in both directions, and one end-to-end run of the real defect site.', + ); +} + +if (isEntrypoint(import.meta.url)) { + if (process.argv.includes('--self-test')) selfTest(); + else main(); +} diff --git a/scripts/pm/ci-failure.mjs b/scripts/pm/ci-failure.mjs index d4014e8186..2895d413e3 100644 --- a/scripts/pm/ci-failure.mjs +++ b/scripts/pm/ci-failure.mjs @@ -2315,6 +2315,16 @@ async function selfTest() { console.error(''); process.exit(1); } + // ⛔ This summary is printed by a step of `Lint & Repo Gates` on EVERY pull + // request, so it must not SPELL a workflow-command token. The runner parses + // the legacy `##[]` form anywhere in a printed line -- not only at + // column 0 -- and this blob used to name the anchor token literally inside + // backticks. That minted a real `annotation_level: failure` on a check run + // whose conclusion was SUCCESS, on every PR in the repo, permanently + // (#11886). Comments like this one are never printed and may spell the token + // freely; the printed prose may not, and splitting the JS string would not + // help -- the runner reads the printed line, not the source. + // `scripts/check-self-test-workflow-commands.mjs` holds this. console.log( 'OK self-test: supersession keeps the newest per name and reports the drops; the four measured\n' + ' annotation shapes classify, with the unrecognised one kept rather than dropped; a check that\n' + @@ -2340,9 +2350,9 @@ async function selfTest() { ' annotations carried none, a log that anchored nothing is still a shortfall rather than a\n' + ' manufactured answer, a 410 is named as expired retention rather than as an absence of\n' + ' evidence, a CONNECT refusal lands as transport with the blocked host to report, and a\n' + - ' tail with no `##[error]` in it is labelled a window rather than an anchor. And a `fix`\n' + - ' renders as the ONE remedy it is: a single `fix:` marker with its continuations padded\n' + - ' under it, keeping a copy-pasteable command on a line of its own.', + ' tail carrying no runner error line at all is labelled a window rather than an anchor.\n' + + ' And a `fix` renders as the ONE remedy it is: a single `fix:` marker with its\n' + + ' continuations padded under it, keeping a copy-pasteable command on a line of its own.', ); }