From 5639239018bc92cb946d38f98d0079735db31bab Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 17:46:56 +0000 Subject: [PATCH] fix(ci): judge discovered self-test sets in check-step-collectors The gate's population was keyed on one spelling -- a `scripts/`|`packages/` path carrying a `--self-test` flag -- which is a rule about where a self-test lives rather than what it is. A second family (`*.selftest.sh` standalone matrices) was invisible, and lint.yml's tolerate-and-collect block over that family scored zero targets: its collector shape was held by a review comment. Recognition is now keyed on two self-declaring markers, neither naming a directory: a `--self-test` flag after any repo script path, or a basename ending `.selftest.sh`. Generalising the flag marker's path moved nothing on this tree (31 steps with >=1 target, 2 with >=2, before and after). Widening the markers alone was not enough, and this is the half the filing did not have: that block names no self-test at all -- it enumerates them with `find` and loops. So an enumerated set is recognised too, and judged PLURAL BY CONSTRUCTION: the set can gain a member with no edit to the workflow, which is the #10814 defect with a wider blast radius than the named case. The recognised enumeration spellings are published in the header and the failure text, and each is pinned. The dynamic half drives a discovered-set block by planting stubs where the block's own enumeration reaches them, so the block discovers them itself and nothing is substituted into its text. Both reds are pinned (bare loop; set routed past an existing collector), plus the empty-discovery #4690 control and the pre-fix ablation. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015ahemw8RcTgqtxrj15PEZx --- scripts/check-step-collectors.mjs | 513 +++++++++++++++++++++++++++++- 1 file changed, 496 insertions(+), 17 deletions(-) diff --git a/scripts/check-step-collectors.mjs b/scripts/check-step-collectors.mjs index f287e29b07..47df432307 100644 --- a/scripts/check-step-collectors.mjs +++ b/scripts/check-step-collectors.mjs @@ -95,6 +95,75 @@ * that file's `@changesets/cli` range and its `version` script, not the file, * so a `check:step-collectors` key would have been allowed. Recorded because * the over-broad reading of that fence propagates as a constraint nobody has. + * + * ## Two families of self-test, and why one was invisible (#11801) + * + * The population above was keyed on ONE spelling -- a `scripts/`|`packages/` + * path carrying a `--self-test` flag. That is a rule about WHERE a self-test + * lives, not about what it is, and a second family exists: `*.selftest.sh` + * standalone matrices (`.claude/hooks/*.selftest.sh`, + * `scripts/bump-objectui.selftest.sh`), invoked as bare executables with no + * flag. lint.yml's `Claude hook guard self-tests` step is a real + * tolerate-and-collect block over that family, and this gate scored it ZERO + * targets -- so its collector shape was held by a review comment, which is + * precisely the state this gate exists to end. + * + * A step is therefore judged on SELF-DECLARING markers, neither of which names + * a directory: + * + * M1 a repo script path followed by `--self-test` on the same command + * M2 a path whose BASENAME ends `.selftest.sh` -- self-testing is in the name + * + * NOT a `.claude/` exception. A hard-coded directory would be this defect one + * level up: the next self-test outside the listed directories is invisible + * again. + * + * ## Named sets and DISCOVERED sets + * + * Widening the markers alone would still not have seen that step, and this is + * the half the filing did not have: the block names no self-test AT ALL. It + * runs `find .claude/hooks -name '*.selftest.sh'` and loops over the result, so + * its only literals are a directory and a glob, and its plurality is a run-time + * fact. Measured: with the markers widened and nothing else, that step still + * reports 0 targets. + * + * A discovered set is therefore treated as PLURAL BY CONSTRUCTION. That is not + * a convenience. An enumeration is chosen precisely because the set is open and + * expected to grow, so a bare loop over one is the #10814 defect with a WIDER + * blast radius than the named case: the set can gain a member with no edit to + * the workflow at all, and nothing goes red. + * + * The enumeration spellings recognised are PUBLISHED -- here and in the failure + * text -- and every one is pinned by `--self-test`, on the precedent AGENTS.md + * sets for `check:cross-package-test-inputs`: a source-text detector sees only + * the spellings it knows, and an unrecognised one produces no flag SILENTLY. + * Reaching for a spelling that is not here? Extend the list and pin it in the + * same edit -- never route around it. + * + * mapfile -t VAR < <(find ROOT ... -name '*.selftest.sh' ...) + * readarray -t VAR < <(find ROOT ... -name '*.selftest.sh' ...) + * VAR=( ROOT/*.selftest.sh ) + * for VAR in ROOT/*.selftest.sh; do + * + * Routing follows the binding ONE HOP: an array bound by a discovery and then + * consumed by `for X in "${VAR[@]}"` counts as routed when `run_self_test` + * receives `$X`. Without that hop the real block reads as unrouted, because the + * collector never sees the array's own name. + * + * ## Population movement, measured before and after (#11801) + * + * Generalising M1's path from `scripts|packages` to any repo-relative directory + * moved NOTHING on this tree: 31 steps carry >=1 target and 2 carry >=2, before + * and after, both on `main` and on the tree that adds the hook step. The + * generalisation drops a location rule at measured zero cost; it fabricates no + * pair and re-attributes no file. + * + * M2's LITERAL form currently matches zero live steps -- no workflow invokes a + * `*.selftest.sh` by name today. It is carried because the family exists and it + * is pinned by FIXTURES rather than by the tree, so a first literal use is + * judged on arrival instead of arriving unjudged. Its positive control is the + * fixture, never the population -- a rule with an empty population cannot be + * shown to work by the population. */ import { existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'node:fs'; @@ -112,10 +181,63 @@ const WORKFLOW_DIR = join('.github', 'workflows'); const COLLECTOR_ANCHOR = /^\s*run_self_test\s*\(\)\s*\{/m; /** One invocation of a self-test through the collector helper. */ const COLLECTED_CALL = /^[ \t]*run_self_test[ \t]+(\S.*)$/gm; -/** A repo script path carrying `--self-test` somewhere after it on the same command. */ -const SELF_TEST_TARGET = /(?:^|\s)((?:\.\/)?(?:scripts|packages)\/[\w./-]+\.(?:mjs|mts|cjs|js|sh|ts))(?=\s)[^\n]*?\s--self-test\b/; + +/** Extensions a repo script can carry. */ +const SCRIPT_EXT = String.raw`(?:mjs|mts|cjs|js|sh|ts)`; +/** A repo-relative path with at least one directory segment -- ANY directory. */ +const REPO_PATH = String.raw`(?:\./)?[\w.][\w./-]*\/[\w./-]+`; + +/** MARKER 1 -- a repo script path carrying `--self-test` after it on the same command. */ +const SELF_TEST_FLAG_TARGET = new RegExp( + String.raw`(?:^|\s)(${REPO_PATH}\.${SCRIPT_EXT})(?=\s)[^\n]*?\s--self-test\b`, +); +/** MARKER 2 -- a path whose BASENAME declares it a self-test; no flag required. */ +const SELF_TEST_NAMED_TARGET = new RegExp(String.raw`(?:^|\s)(${REPO_PATH}\.selftest\.sh)(?=\s|$)`); /** The first repo-relative script path in a command -- the file a stub stands in for. */ -const SCRIPT_TOKEN = /(?:^|\s)((?:scripts|packages)\/[\w./-]+\.(?:mjs|mts|cjs|js|sh|ts))(?=\s|$)/; +const SCRIPT_TOKEN = new RegExp(String.raw`(?:^|\s)(${REPO_PATH}\.${SCRIPT_EXT})(?=\s|$)`); + +/** A `*.selftest.sh` pattern as it appears in a `find -name` argument or a shell glob. */ +const SELFTEST_PATTERN = String.raw`[\w.*?/-]*\*?\.selftest\.sh`; + +/** Split a glob into the directory it searches and the pattern it matches. */ +function splitGlob(glob) { + const at = glob.lastIndexOf('/'); + return at === -1 ? { root: '.', pattern: glob } : { root: glob.slice(0, at), pattern: glob.slice(at + 1) }; +} + +/** + * The enumeration spellings that bind a DISCOVERED set of self-tests to a + * variable. Published deliberately (see the header): a source-text detector + * sees only what it knows, and an unrecognised spelling produces no flag + * silently. Every entry is pinned by `--self-test`. + */ +export const DISCOVERY_SPELLINGS = [ + { + id: 'mapfile-find', + example: "mapfile -t VAR < <(find ROOT ... -name '*.selftest.sh' ...)", + re: new RegExp( + String.raw`(?:^|\s)(?:mapfile|readarray)\s+(?:-\S+\s+)*(\w+)\s*<\s*<\(\s*find\s+(\S+)[^)]*?-name\s+['"]?(${SELFTEST_PATTERN})['"]?`, + ), + read: (m) => ({ variable: m[1], root: m[2], pattern: m[3] }), + }, + { + id: 'array-glob', + example: 'VAR=( ROOT/*.selftest.sh )', + re: new RegExp(String.raw`(?:^|\s)(\w+)=\(\s*['"]?(${SELFTEST_PATTERN})['"]?\s*\)`), + read: (m) => ({ variable: m[1], ...splitGlob(m[2]) }), + }, + { + id: 'for-glob', + example: 'for VAR in ROOT/*.selftest.sh; do', + re: new RegExp(String.raw`(?:^|\s)for\s+(\w+)\s+in\s+['"]?(${SELFTEST_PATTERN})['"]?\s*;?`), + read: (m) => ({ variable: m[1], ...splitGlob(m[2]) }), + }, +]; + +/** The published spellings, for failure text -- the list is not a private detail. */ +function discoverySpellingHelp() { + return DISCOVERY_SPELLINGS.map((s) => ` ${s.example}`).join('\n'); +} /** * The top-level commands of a `run:` block: whole-line `#` comments dropped and @@ -148,7 +270,7 @@ export function topLevelCommands(runText) { } /** - * The DISTINCT scripts a `run:` block asks to self-test. + * The DISTINCT scripts a `run:` block NAMES as self-tests, by either marker. * * @param {string} runText * @returns {string[]} @@ -156,12 +278,82 @@ export function topLevelCommands(runText) { export function selfTestTargets(runText) { const seen = new Set(); for (const command of topLevelCommands(runText)) { - const m = SELF_TEST_TARGET.exec(command); - if (m) seen.add(m[1].replace(/^\.\//, '')); + for (const re of [SELF_TEST_FLAG_TARGET, SELF_TEST_NAMED_TARGET]) { + const m = re.exec(command); + if (m) seen.add(m[1].replace(/^\.\//, '')); + } } return [...seen]; } +/** + * The DISCOVERED sets of self-tests a `run:` block enumerates rather than + * names. A discovery is PLURAL BY CONSTRUCTION -- see the header. + * + * @param {string} runText + * @returns {Array<{ spelling: string, variable: string, root: string, pattern: string }>} + */ +export function selfTestDiscoveries(runText) { + const out = []; + const seen = new Set(); + for (const command of topLevelCommands(runText)) { + for (const spelling of DISCOVERY_SPELLINGS) { + const m = spelling.re.exec(command); + if (!m) continue; + const read = spelling.read(m); + const discovery = { + spelling: spelling.id, + variable: read.variable, + root: String(read.root).replace(/^\.\//, '').replace(/\/+$/, '') || '.', + pattern: read.pattern, + }; + const key = `${discovery.root}|${discovery.pattern}|${discovery.variable}`; + if (seen.has(key)) break; + seen.add(key); + out.push(discovery); + break; + } + } + return out; +} + +/** + * The variable names that carry a discovery's items, following the binding ONE + * HOP through `for X in "${VAR[@]}"`. Without this the real block reads as + * unrouted: the collector receives the LOOP variable, never the array's name. + * + * @param {string} runText + * @param {string} variable + * @returns {Set} + */ +export function routedVariables(runText, variable) { + const names = new Set([variable]); + const mentions = (text, name) => new RegExp(String.raw`\$\{?${name}\b`).test(text); + for (let pass = 0; pass < 8; pass++) { + let grew = false; + for (const m of String(runText).matchAll(/(?:^|\s)for\s+(\w+)\s+in\s+([^\n;]*)/g)) { + if (names.has(m[1])) continue; + if ([...names].some((n) => mentions(m[2], n))) { + names.add(m[1]); + grew = true; + } + } + if (!grew) break; + } + return names; +} + +/** Is this discovery's set routed through the collector helper? */ +function discoveryIsRouted(runText, discovery, collected) { + const names = routedVariables(runText, discovery.variable); + return collected.some((c) => [...names].some((n) => new RegExp(String.raw`\$\{?${n}\b`).test(c))); +} + +/** How a discovery reads in a message. */ +function describeDiscovery(d) { + return `${d.root}/${d.pattern} (discovered at run time, bound to \`${d.variable}\`)`; +} + /** Does this block use the collector helper rather than a bare sequence? */ export function isCollector(runText) { return COLLECTOR_ANCHOR.test(String(runText)); @@ -199,29 +391,47 @@ export function scanWorkflowText(text, file, parseYaml) { if (typeof step?.run !== 'string') continue; steps++; const targets = selfTestTargets(step.run); - if (targets.length < 2) continue; + const discoveries = selfTestDiscoveries(step.run); + // A DISCOVERED set is plural by construction -- the enumeration is chosen + // because the set is open, so a bare loop over it masks a member the + // workflow never named. See the header. + if (targets.length < 2 && discoveries.length === 0) continue; + const subjects = [...targets, ...discoveries.map(describeDiscovery)]; const name = typeof step.name === 'string' ? step.name : '(unnamed step)'; if (!isCollector(step.run)) { + // A discovered set has no static count -- saying "1" would be the very + // defect this gate's header names: a number that stays right while its + // subject grows. + const how = + discoveries.length > 0 + ? 'a DISCOVERED set of independent self-tests' + : `${subjects.length} independent self-tests`; problems.push( - `${file}: job \`${job}\`, step "${name}" sequences ${targets.length} independent self-tests ` + - `(${targets.join(', ')}) in one \`run:\` block. Under \`bash -e\` the first non-zero exit aborts ` + - `the step, so the ones after it are never run -- neither green nor red (#10814). Route them ` + - `through a \`run_self_test\` collector that runs each unconditionally and exits non-zero at the ` + - `end naming every failure.`, + `${file}: job \`${job}\`, step "${name}" runs ${how} ` + + `(${subjects.join(', ')}) as a bare sequence in one \`run:\` block. Under \`bash -e\` the first ` + + `non-zero exit aborts the step, so the ones after it are never run -- neither green nor red ` + + `(#10814). Route them through a \`run_self_test\` collector that runs each unconditionally and ` + + `exits non-zero at the end naming every failure.` + + (discoveries.length > 0 + ? ` The set is enumerated at run time, so it can gain a member with no edit to this workflow ` + + `at all -- which is why an enumeration is judged plural on sight.` + : ''), ); continue; } const collected = collectedCommands(step.run); const uncollected = targets.filter((t) => !collected.some((c) => c.includes(t))); - if (uncollected.length > 0) { + const unrouted = discoveries.filter((d) => !discoveryIsRouted(step.run, d, collected)); + const missed = [...uncollected, ...unrouted.map(describeDiscovery)]; + if (missed.length > 0) { problems.push( `${file}: job \`${job}\`, step "${name}" defines a collector but does not route ` + - `${uncollected.join(', ')} through it -- a self-test outside the collector is masked exactly ` + + `${missed.join(', ')} through it -- a self-test outside the collector is masked exactly ` + `as before (#10814).`, ); continue; } - collectors.push({ file, job, name, run: step.run }); + collectors.push({ file, job, name, run: step.run, discoveries }); } } return { problems, steps, collectors }; @@ -259,8 +469,9 @@ export function scanWorkflows(root, parseYaml) { } if (collectors.length === 0 && problems.length === 0) { problems.push( - `scanned ${steps} \`run:\` steps and found no collector at all. Two live in lint.yml's \`lint\` job ` + - `(#10814); zero means this scan stopped reading them, not that the tree stopped needing them (#4690).`, + `scanned ${steps} \`run:\` steps and found no collector at all. Several live in lint.yml's \`lint\` ` + + `job (#10814); zero means this scan stopped reading them, not that the tree stopped needing them ` + + `(#4690). Recognised enumeration spellings for a discovered set:\n${discoverySpellingHelp()}`, ); } return { problems, steps, collectors, files: files.length }; @@ -320,6 +531,65 @@ export function bareSequence(commands) { return `${commands.join('\n')}\n`; } +/** + * Run a DISCOVERED-set block the way Actions runs it, against planted stubs. + * + * The block enumerates its own self-tests, so the stubs are planted where its + * OWN `find`/glob will reach them and the block is left to discover them -- + * nothing is substituted into the text. Whether a stub ran is read from its + * side effect, never inferred from the block's output. + * + * @param {string} runText the block, verbatim + * @param {{ root: string, pattern: string }} discovery where its enumeration looks + * @param {string[]} stubNames basenames to plant, in `find | sort` order + * @param {Set} failing indices of the stubs that exit 1 + * @returns {{ status: number, output: string, executed: string[] }} + */ +export function driveDiscoveryBlock(runText, discovery, stubNames, failing) { + const dir = mkdtempSync(join(tmpdir(), 'os-step-collector-disc-')); + try { + const log = join(dir, 'executed.log'); + const root = discovery.root && discovery.root !== '.' ? discovery.root : '.'; + mkdirSync(join(dir, root), { recursive: true }); + stubNames.forEach((base, i) => { + const rel = root === '.' ? `./${base}` : `${root}/${base}`; + const code = failing.has(i) ? 1 : 0; + writeFileSync(join(dir, root, base), `#!/usr/bin/env bash\nprintf '%s\\n' "${rel}" >> "$OS_STUB_LOG"\nexit ${code}\n`, { + mode: 0o755, + }); + }); + writeFileSync(log, ''); + const script = join(dir, 'block.sh'); + writeFileSync(script, runText); + const proc = spawnSync('bash', ['-e', script], { + cwd: dir, + encoding: 'utf8', + env: { ...process.env, OS_STUB_LOG: log }, + }); + return { + status: proc.status ?? -1, + output: `${proc.stdout ?? ''}${proc.stderr ?? ''}`, + executed: readFileSync(log, 'utf8').split('\n').filter(Boolean), + }; + } finally { + rmSync(dir, { recursive: true, force: true }); + } +} + +/** + * The pre-fix shape for a discovered set: the same enumeration, looped bare. + * This is the "simplification" the gate exists to reject, written the way + * someone reaching for one would write it. + */ +export function bareDiscoveryLoop(discovery) { + const root = discovery.root && discovery.root !== '.' ? discovery.root : '.'; + return [ + `mapfile -t os_ablation < <(find ${root} -type f -name '${discovery.pattern}' | sort)`, + 'for os_t in "${os_ablation[@]}"; do "$os_t"; done', + '', + ].join('\n'); +} + // -- Entry points ------------------------------------------------------------- async function loadYamlParser() { @@ -400,6 +670,209 @@ async function selfTest() { '#4690: a missing workflow directory is a failure, never a pass', ); + // ---- DISCOVERED sets: recognition, both reds, and the driven block ------- + // + // Pinned by FIXTURES as well as by the tree. The live tree may hold no + // discovery collector at a given moment, and coverage that silently drops to + // zero when a workflow is edited elsewhere is the #4690 shape this family + // exists to distrust. + const asWorkflow = (title, body) => + [ + 'jobs:', + ' lint:', + ' steps:', + ` - name: ${title}`, + ' run: |', + ...body.map((l) => (l ? ` ${l}` : '')), + '', + ].join('\n'); + + const DISCOVER = "mapfile -t selftests < <(find .claude/hooks -type f -name '*.selftest.sh' | sort)"; + + // GREEN: enumerate, then route every discovered item through the collector. + const discoveryCollectorBody = [ + DISCOVER, + 'if [ "${#selftests[@]}" -eq 0 ]; then', + ' echo "DISCOVERED NOTHING -- verified nothing, which is a failure and not a pass (#4690)"', + ' exit 1', + 'fi', + 'echo "discovered ${#selftests[@]} hook self-test(s)"', + 'failed=""', + 'run_self_test() {', + ' echo "-- $*"', + ' if "$@"; then', + ' echo "PASS $*"', + ' else', + ' echo "FAIL $*"', + ' failed="${failed} $*"', + ' fi', + ' return 0', + '}', + 'for selftest in "${selftests[@]}"; do', + ' run_self_test "$selftest"', + 'done', + 'if [ -n "$failed" ]; then', + ' echo "FAILED:$failed"', + ' exit 1', + 'fi', + ]; + + // RED 1: the same discovery "simplified" into a bare loop -- no collector. + const discoveryBareBody = [DISCOVER, 'for selftest in "${selftests[@]}"; do', ' "$selftest"', 'done']; + + // RED 2: a collector exists, but the discovered set walks past it. + const discoveryUnroutedBody = [ + DISCOVER, + 'run_self_test() { "$@"; }', + 'run_self_test node scripts/alpha.mjs --self-test', + 'for selftest in "${selftests[@]}"; do', + ' "$selftest"', + 'done', + ]; + + const discGreen = scanWorkflowText(asWorkflow('Discovered self-tests, collected', discoveryCollectorBody), 'fixture.yml', parseYaml); + assert(discGreen.problems.length === 0, `a DISCOVERED-set collector is accepted (${discGreen.problems[0] ?? ''})`); + assert(discGreen.collectors.length === 1, `a DISCOVERED-set collector is JUDGED, not skipped (found ${discGreen.collectors.length})`); + assert( + discGreen.collectors[0]?.discoveries?.length === 1 && + discGreen.collectors[0].discoveries[0].root === '.claude/hooks' && + discGreen.collectors[0].discoveries[0].pattern === '*.selftest.sh' && + discGreen.collectors[0].discoveries[0].variable === 'selftests', + 'the discovery descriptor reads back its root, pattern and bound variable', + ); + + const discBare = scanWorkflowText(asWorkflow('Discovered self-tests, bare loop', discoveryBareBody), 'fixture.yml', parseYaml); + assert(discBare.problems.length === 1, `a bare loop over a DISCOVERED set is one problem (got ${discBare.problems.length})`); + assert( + discBare.problems[0]?.includes('.claude/hooks/*.selftest.sh') && discBare.problems[0]?.includes('discovered at run time'), + 'the problem NAMES the discovered set rather than reporting an anonymous count', + ); + + const discUnrouted = scanWorkflowText(asWorkflow('Discovered self-tests, not routed', discoveryUnroutedBody), 'fixture.yml', parseYaml); + assert( + discUnrouted.problems.length === 1 && discUnrouted.problems[0].includes('does not route'), + 'a DISCOVERED set left outside an existing collector is still flagged', + ); + + // A named `*.selftest.sh` with no `--self-test` flag: MARKER 2's literal form. + // Its live population is zero (no workflow invokes one by name), so this + // fixture IS its positive control -- a rule with an empty population cannot be + // shown to work by the population. + const namedSelftestBody = ['.claude/hooks/guard-a.selftest.sh', 'scripts/bump-objectui.selftest.sh']; + const namedBare = scanWorkflowText(asWorkflow('Two named .selftest.sh, sequenced', namedSelftestBody), 'fixture.yml', parseYaml); + assert( + namedBare.problems.length === 1 && + namedBare.problems[0].includes('.claude/hooks/guard-a.selftest.sh') && + namedBare.problems[0].includes('scripts/bump-objectui.selftest.sh'), + 'MARKER 2: two bare `*.selftest.sh` invocations are flagged, in ANY directory and with no flag', + ); + assert( + selfTestTargets('bash tools/local/thing.sh --self-test\nbash ops/other.sh --self-test').length === 2, + 'MARKER 1: the `--self-test` flag is recognised outside `scripts/`|`packages/` too', + ); + assert( + scanWorkflowText(asWorkflow('One named .selftest.sh', ['.claude/hooks/guard-a.selftest.sh']), 'fixture.yml', parseYaml).problems.length === 0, + 'a SINGLE named self-test is not a masking pair -- the threshold still bites', + ); + + // Every published discovery spelling is pinned. An unrecognised spelling + // produces no flag SILENTLY, so the list is only as good as its pins. + for (const spelling of DISCOVERY_SPELLINGS) { + const sample = { + 'mapfile-find': DISCOVER, + 'array-glob': 'selftests=( .claude/hooks/*.selftest.sh )', + 'for-glob': 'for selftest in .claude/hooks/*.selftest.sh; do', + }[spelling.id]; + assert(typeof sample === 'string', `discovery spelling \`${spelling.id}\` has a pinned sample`); + if (typeof sample !== 'string') continue; + const found = selfTestDiscoveries(sample); + assert( + found.length === 1 && found[0].root === '.claude/hooks' && found[0].pattern === '*.selftest.sh', + `discovery spelling \`${spelling.id}\` is recognised and reads back its root and pattern (got ${JSON.stringify(found)})`, + ); + } + assert( + selfTestDiscoveries('for selftest in "${selftests[@]}"; do').length === 0, + 'expanding an already-bound array is NOT a second discovery -- the pattern marker is required', + ); + assert( + routedVariables('for selftest in "${selftests[@]}"; do', 'selftests').has('selftest'), + 'routing follows the binding one hop, so the collector may receive the LOOP variable', + ); + + /** + * Drive one DISCOVERED-set block: plant stubs where the block's own + * enumeration will find them, then read what ran from the stubs' side effect. + */ + const checkDiscoveryCollector = (label, runText, discovery) => { + const names = ['zz-os-a.selftest.sh', 'zz-os-b.selftest.sh', 'zz-os-c.selftest.sh']; + const n = names.length; + + const allPass = driveDiscoveryBlock(runText, discovery, names, new Set()); + assert(allPass.status === 0, `${label}: all green => the step exits 0 (got ${allPass.status})`); + assert( + allPass.executed.length === n, + `${label}: all green => every DISCOVERED self-test runs (${allPass.executed.length}/${n})`, + ); + assert((allPass.output.match(/^PASS /gm) ?? []).length === n, `${label}: all green => a PASS verdict per discovered self-test`); + + for (let i = 0; i < n; i++) { + const one = driveDiscoveryBlock(runText, discovery, names, new Set([i])); + assert( + one.executed.length === n, + `${label}: discovered #${i + 1} red => every one still RUNS (${one.executed.length}/${n}) -- a failure ` + + `must not hide the others' verdicts`, + ); + assert( + (one.output.match(/^(PASS|FAIL) /gm) ?? []).length === n, + `${label}: discovered #${i + 1} red => a verdict is printed for every discovered self-test`, + ); + assert( + one.status !== 0, + `${label}: discovered #${i + 1} red => the step still FAILS (got ${one.status}) -- a collector that ` + + `swallows the exit code is worse than the masking it replaces`, + ); + assert(one.output.includes(names[i]), `${label}: discovered #${i + 1} red => the summary NAMES it`); + } + + const allFail = driveDiscoveryBlock(runText, discovery, names, new Set(names.map((_, i) => i))); + assert(allFail.status !== 0, `${label}: all red => the step fails`); + assert(allFail.executed.length === n, `${label}: all red => every discovered self-test still runs`); + + // #4690 positive control, and the proof that the harness drives the block's + // OWN enumeration: were it reading the repo instead of the fixture, it would + // find the real matrices here and pass. + const empty = driveDiscoveryBlock(runText, discovery, [], new Set()); + assert(empty.status !== 0, `${label}: EMPTY discovery => red, never a green pass over nothing (#4690) (got ${empty.status})`); + assert(empty.executed.length === 0, `${label}: EMPTY discovery => nothing ran, so the fixture really is this block's input`); + + // ABLATION: the pre-fix shape over the SAME discovery must mask. + const masked = driveDiscoveryBlock(bareDiscoveryLoop(discovery), discovery, names, new Set([0])); + assert( + masked.executed.length === 1, + `${label}: ABLATION -- the pre-fix bare loop with the first red must run exactly 1 of ${n} (ran ` + + `${masked.executed.length}); a harness that cannot reproduce the defect cannot certify the fix`, + ); + assert(masked.status !== 0, `${label}: ABLATION -- the pre-fix shape does fail, it just fails silently`); + const unmasked = driveDiscoveryBlock(bareDiscoveryLoop(discovery), discovery, names, new Set()); + assert( + unmasked.executed.length === n && unmasked.status === 0, + `${label}: ABLATION control -- with nothing red the pre-fix loop runs all ${n}, so the mask above is ` + + `caused by the FAILURE and not by the harness`, + ); + }; + + // Guarded: if the recognition above regressed, the assertions have already + // recorded it by name. Reaching in anyway would replace that list with a + // stack trace -- red either way, but a red that says nothing specific. + if (discGreen.collectors[0]?.discoveries?.length === 1) { + checkDiscoveryCollector( + 'fixture.yml "Discovered self-tests, collected"', + discGreen.collectors[0].run, + discGreen.collectors[0].discoveries[0], + ); + } + // ---- The dynamic half: the LIVE blocks, under a real `bash -e` ------------ const live = scanWorkflows(REPO_ROOT, parseYaml); assert(live.problems.length === 0, `the checked-in workflows pass the static half (${live.problems[0] ?? ''})`); @@ -407,6 +880,12 @@ async function selfTest() { for (const collector of live.collectors) { const label = `${collector.file} "${collector.name}"`; + // A DISCOVERED-set collector names no command, so it is driven by planting + // stubs where its own enumeration reaches -- not by substituting text. + if ((collector.discoveries ?? []).length > 0) { + for (const discovery of collector.discoveries) checkDiscoveryCollector(label, collector.run, discovery); + continue; + } const commands = collectedCommands(collector.run); assert(commands.length >= 2, `${label}: the collector drives 2+ commands (found ${commands.length})`); if (commands.length < 2) continue;