diff --git a/CHANGELOG.md b/CHANGELOG.md index 8673abd3..b0771997 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1324,7 +1324,7 @@ All notable changes to MeMesh are documented here. can shift a centroid and move a member. Identical membership is still de-duplicated exactly; an *overlapping* cluster is not, so a pending proposal can end up beside a later one covering most of the same entries. Both are - staged, neither touches a source entity, and `memesh dream review` shows the + staged, neither touches a source entity, and `memesh dream show` shows the source ids — but it is a real difference from the old behaviour. ### Fixed diff --git a/docs/api/API_REFERENCE.md b/docs/api/API_REFERENCE.md index c9c5d156..6aed8a89 100644 --- a/docs/api/API_REFERENCE.md +++ b/docs/api/API_REFERENCE.md @@ -800,8 +800,9 @@ node. Archived entities are excluded. `evidenceCounts` maps a work-node name to its number of incoming `evidences` edges; a node with no such edge is absent from the map. Those edges are drawn -by `memesh kg backfill`, not by the hooks — a graph where every count is zero -means the backfill has not run, not that the work happened without evidence. +by `memesh kg backfill-relations`, not by the hooks — a graph where every +count is zero means the backfill has not run, not that the work happened +without evidence. Any other `layer` value is a `400` with `errorCode: "validation.bad-param"`. There is no `layer=evidence`: the evidence layer is an order of magnitude diff --git a/scripts/audit/baseline.json b/scripts/audit/baseline.json index a0dd8420..586e76de 100644 --- a/scripts/audit/baseline.json +++ b/scripts/audit/baseline.json @@ -303,7 +303,7 @@ }, "C3 scripts/audit/measure-injection-tokens.mjs": { "class": "NOT-A-GATE", - "reason": "manual measurement tool a human runs to compare session-start injection size before/after a change; spawns the hook against a throwaway HOME and gates nothing. Note: this file's header names measure-work-topology-baseline.mjs, and C3 counts any basename reference — which is why that script stopped appearing as a hit and its entry was pruned. If this cross-reference is ever removed, it becomes an untriaged hit again and needs its own NOT-A-GATE entry.", + "reason": "manual measurement tool a human runs to compare session-start injection size before/after a change; spawns the hook against a throwaway HOME and gates nothing. (This reason used to carry a note that its own header's mention of measure-work-topology-baseline.mjs was what kept THAT script from being a C3 hit. The detector no longer counts a filename written in a comment as a caller, so the note is obsolete and the other script now carries its own entry.)", "triaged": "2026-08-16" }, "C1 tests/core/task-state.test.ts": { @@ -621,6 +621,11 @@ "reason": "One test reads src/db.ts and asserts that markReindexOwed() is NOT inside the once-only notice brace. That is a structural pin chosen ON PURPOSE: the behavioural scenario (a fresh open while the notice flag is already set) is unreachable in one process — openDatabase returns the existing singleton, and the only reopen path (closeDatabase) resets the flag. Two behavioural tests were written first and both passed for the wrong reason; the review's brace-move mutant survived them. The text IS the property here.", "triaged": "2026-08-23" }, + "C3 scripts/audit/measure-work-topology-baseline.mjs": { + "class": "NOT-A-GATE", + "reason": "Read-only measurement tool a human runs once to size three work-topology decisions (work-layer share, recall-hit false negatives, opening-exploration cost). Opens the DB with readOnly and writes no file; it decides nothing and enforces nothing, so having no automated caller is correct. Surfaced 2026-08-23 when C3 stopped counting a filename written in a comment as a caller \u2014 measure-injection-tokens.mjs's header names this file, and that single prose mention had been enough to hide it and to get its entry pruned as stale.", + "triaged": "2026-08-23" + }, "C6 tests/release-preconditions.test.ts": { "class": "WIRING-PIN", "reason": "The decision itself is behavioural: checkReleasePreconditions is a pure function and 21 tests in the same file drive both directions through it. What is read as text is scripts/finish-release.mjs, and only to pin five wiring properties whose happy path cannot be run — cutting the release creates a public GitHub Release and publishes to npm. The regression guarded is someone splitting the single `gh release create --target` back into `git tag` + `git push` + `gh release create`, whose middle leaves a pushed tag with no release: nothing publishes, and the coherence gate now sees the tag and reports ok. Break-tested 2026-08-23: 7/7 mutants killed, including both directions of this pin.", diff --git a/scripts/audit/verification-audit.mjs b/scripts/audit/verification-audit.mjs index 827ddffc..41d7f733 100644 --- a/scripts/audit/verification-audit.mjs +++ b/scripts/audit/verification-audit.mjs @@ -17,6 +17,7 @@ // file stays fast enough for verify:release. import fs from 'node:fs'; import path from 'node:path'; +import { stripComments } from '../lib/reference-corpus.mjs'; import { fileURLToPath } from 'node:url'; const REPO = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); @@ -88,12 +89,15 @@ function record(cls, denominator, hits, note) { const scriptFiles = walk('scripts', ['.mjs', '.sh']) .filter(f => !f.includes('/lib/') && !f.includes('/hooks/')); const candidates = [...gateScripts.map(k => `npm:${k}`), ...scriptFiles]; + // Comments are stripped before counting: a filename written in prose is not + // a caller. See scripts/lib/reference-corpus.mjs for the two times a single + // sentence hid an uncalled script from this detector. const corpusParts = [ ...walk('.github/workflows', ['.yml']).map(f => [f, read(f)]), ...scriptFiles.map(f => [f, read(f)]), ...walk('tests', ['.ts', '.tsx']).map(f => [f, read(f)]), ['package.json', read('package.json')], - ]; + ].map(([f, txt]) => [f, stripComments(txt, f)]); const hits = []; for (const c of candidates) { const needle = c.startsWith('npm:') ? `run ${c.slice(4)}` : path.basename(c); diff --git a/scripts/check-doc-claims.mjs b/scripts/check-doc-claims.mjs index 08633e53..edda0ceb 100644 --- a/scripts/check-doc-claims.mjs +++ b/scripts/check-doc-claims.mjs @@ -561,6 +561,76 @@ if (!hasBearerAuth) { else if (badCommands.length) fail(`agent docs name CLI subcommands that do not exist:\n ${badCommands.join('\n ')}`); else if (agentDocs.length) ok(`${mentions} \`memesh \` mentions in agent docs all resolve to registered CLI commands`); + // (a1) a NESTED subcommand must exist under its parent. + // + // (a) above captures only the first word after `memesh`, so `memesh kg + // backfill` reads as `kg` — a real command — and passed. The real name is + // `kg backfill-relations`, and that bullet survived every gate in the 4.6.1 + // release notes until somebody happened to read it while promoting the + // section. A gate that checks the parent and ignores the child is a gate for + // the half of the name that is hardly ever wrong. + // + // Parents and children are both derived from cli.ts. `patterns` is + // registered twice — top-level and under `dream` — which is why the child + // sets are per-parent rather than one flat set. + const parentVars = new Map( + [...cliSrc.matchAll(/const (\w+)\s*=\s*program\s*\.command\('([\w-]+)'/g)].map(m => [m[1], m[2]]), + ); + const childrenOf = new Map(); + for (const [varName, parentName] of parentVars) { + const re = new RegExp(`(?:^|\\n)${varName}\\s*\\.command\\('([\\w-]+)`, 'g'); + childrenOf.set(parentName, new Set([...cliSrc.matchAll(re)].map(m => m[1]))); + } + // Absence is not evidence: an extraction that stopped matching would report + // "no bad nested commands found" forever. + if (parentVars.size === 0) fail('no `const xCmd = program.command(...)` parents matched — the nested-command extraction stopped working'); + for (const [parentName, kids] of childrenOf) { + if (kids.size === 0) fail(`\`${parentName}\` matched no subcommands — the nested-command extraction stopped working`); + } + + // Scanned across EVERY tracked markdown file, whole-document. + // + // The first version looked only inside single backticks, in a hand-listed + // set of documents. Measured against the repository, that missed 19 distinct + // nested commands living in fenced code blocks — more than the 25 it saw, + // and the fenced ones are what an agent copies out of `llms-install.md` and + // runs. It also missed `skills/memesh-review/SKILL.md`, which was not on the + // list. + // + // Whole-document, every tracked `.md`, was then measured too: 89 mentions, + // exactly one of them wrong — `memesh dream review` in the 4.5.1 notes, a + // command `git log -S` finds no trace of in cli.ts's whole history. So the + // narrower rule was not trading coverage for quiet; it was hiding a live + // error in published release notes. + // + // The CHANGELOG is included in full, older sections and all, for the same + // reason. If a nested subcommand is ever legitimately retired, its + // announcement will fail this gate — and that is the moment to decide what + // to do about it, not a reason to stop looking now. + const nestedTargets = [...tracked].filter(f => f.endsWith('.md')).map(f => [f, read(f)]); + if (nestedTargets.length === 0) fail('no tracked markdown files found — the nested-command scan has nothing to read'); + + // Same lookbehind as (a): it keeps `@pcircle/memesh`, `memesh-mcp` and + // `pcircle-memesh` out, so only a real invocation is read as one. + const nestedRe = new RegExp( + String.raw`(? ${m[0]}`); + } + } + // Zero mentions is not a pass. If the pattern ever stops matching — a doc + // style change, a regex edit — this would print "0 mentions all resolve" + // forever, which is the exact shape of gate it was added to close. + if (nestedMentions === 0) fail('found no nested `memesh ` mentions in any document — the extraction stopped matching'); + else if (badNested.length) fail(`documents name nested subcommands that do not exist:\n ${badNested.join('\n ')}`); + else ok(`${nestedMentions} nested \`memesh <${[...childrenOf.keys()].join('|')}> \` mentions across ${nestedTargets.length} markdown files all resolve`); + // (a2) every CLI flag an option table documents is a flag cli.ts registers. // // The missing direction. A sibling test already scans SOURCE files so no diff --git a/scripts/lib/check-settling.mjs b/scripts/lib/check-settling.mjs new file mode 100644 index 00000000..c958d9d0 --- /dev/null +++ b/scripts/lib/check-settling.mjs @@ -0,0 +1,100 @@ +// Is a green tally actually a verdict, or just an early poll? +// +// `wait-for-checks.mjs` asked one question — "is pending zero?" — and returned +// success when the answer was yes. That is sound only if every check has +// already been registered, and GitHub registers check runs in batches. Measured +// on PR #190 (2026-08-23): one poll returned a single row, +// `Analyze (javascript-typescript)`, while the other twelve did not exist yet. +// Had that row been `pass` at that moment, the script would have reported +// `pass=1 pending=0 fail=0 exit=0` — thirteen legs, one of them run, reported +// green. +// +// That is the same shape as every gate this repository has had to fix: absence +// of a failure signal read as success. This script exists BECAUSE `gh pr checks` +// exit codes could not be trusted; it should not carry the same hole one level +// down. +// +// The rule: a PASS is only a verdict once the SET OF CHECK NAMES has been +// identical across two consecutive successful polls. A FAIL is a verdict +// immediately — a red leg is red whether or not its siblings have registered, +// and making someone wait for it helps nobody. +// +// Keyed on the name set rather than the count: `concurrency.cancel-in-progress` +// can swap one run's legs for another's at the same cardinality, and "13 rows, +// then a different 13 rows" is not settled. + +/** Consecutive polls that must agree on the check-name set before PASS is real. */ +export const REQUIRED_STABLE_POLLS = 2; + +/** + * A stable, order-independent key for the set of check names in one poll. + * + * @param {Array<{name?: string}>} checks + * @returns {string} + */ +export function checkNamesKey(checks) { + if (!Array.isArray(checks)) return ''; + return checks + .map((c) => (c && typeof c.name === 'string' ? c.name : '(unnamed)')) + .sort() + .join(' '); +} + +/** + * Decide what one poll means, given what the previous poll saw. + * + * @param {object} input + * @param {{pass: number, pending: number, fail: number}} input.tally + * @param {string} input.namesKey Key for this poll. + * @param {string|null} input.prevNamesKey Key for the previous successful poll. + * @param {number} input.stablePolls Consecutive polls that have agreed so far. + * @param {number} [input.requiredStablePolls] + * @returns {{verdict: 'fail'|'pass'|'wait', stablePolls: number, reason: string}} + */ +export function evaluatePoll({ + tally, + namesKey, + prevNamesKey, + stablePolls, + requiredStablePolls = REQUIRED_STABLE_POLLS, +}) { + // No rows at all is never a verdict, and it resets the count. Right after a + // push GitHub can take seconds to register anything; and mid-run, a response + // that lost every row is the clearest possible evidence that the list is NOT + // holding still. Folding it in here rather than short-circuiting in the + // caller is what keeps "13 green, then zero rows, then the same 13 green" + // from counting as two consecutive agreeing polls. + const total = tally.pass + tally.pending + tally.fail; + if (total === 0) { + return { verdict: 'wait', stablePolls: 0, reason: 'no checks reported yet' }; + } + + // A red leg is a verdict on its own. Waiting for the set to settle before + // reporting a failure would only delay bad news that cannot get better. + if (tally.fail > 0) { + return { verdict: 'fail', stablePolls: 0, reason: `${tally.fail} check(s) failed` }; + } + + const next = namesKey === prevNamesKey ? stablePolls + 1 : 1; + + if (tally.pending > 0) { + return { verdict: 'wait', stablePolls: next, reason: `${tally.pending} pending` }; + } + + if (next < requiredStablePolls) { + return { + verdict: 'wait', + stablePolls: next, + reason: + `nothing pending, but the check list has only been stable for ${next} of ` + + `${requiredStablePolls} polls. GitHub registers check runs in batches, so ` + + `"no pending rows yet" is not "every check passed"`, + }; + } + + return { + verdict: 'pass', + stablePolls: next, + reason: `${tally.pass} passed, check list stable for ${next} polls`, + }; +} diff --git a/scripts/lib/reference-corpus.mjs b/scripts/lib/reference-corpus.mjs new file mode 100644 index 00000000..3e6e0e00 --- /dev/null +++ b/scripts/lib/reference-corpus.mjs @@ -0,0 +1,55 @@ +// What counts as a REFERENCE to a script, when you are asking "does anything +// actually run this?" +// +// The C3 detector in `scripts/audit/verification-audit.mjs` finds gate-like +// scripts with no automated caller by counting how often their basename +// appears across workflows, scripts, tests and `package.json`. It counted the +// raw text, so a filename written in prose counted as a caller. +// +// That is not hypothetical. `scripts/audit/measure-injection-tokens.mjs` opens +// with "Companion to measure-work-topology-baseline.mjs", and that one +// sentence was enough to make the companion look called — its C3 entry was +// then reported as stale and pruned. Somebody noticed and, instead of fixing +// the detector, wrote the explanation into the OTHER script's baseline reason, +// where it sat as a warning not to delete the comment. The same thing happened +// again the moment a test's header comment named `wait-for-checks.mjs`. +// +// A detector for uncalled gates that any sentence can silence is not a +// detector. Comments come out before counting. + +/** + * Blank out comments so a filename mentioned in prose cannot read as a caller. + * + * Over-stripping is the safe direction: a reference lost this way makes a + * script look UNcalled, which fails the audit loudly and is triaged by a + * human. A reference wrongly kept hides a finding, silently, forever. + * + * Comments are replaced with a space rather than removed, so nothing on either + * side is accidentally joined into a new token. + * + * @param {string} text + * @param {string} file Path or filename; the extension selects the syntax. + * @returns {string} + */ +export function stripComments(text, file) { + if (typeof text !== 'string') return ''; + const name = String(file ?? ''); + + if (/\.(mjs|cjs|js|ts|tsx)$/.test(name)) { + return ( + text + .replace(/\/\*[\s\S]*?\*\//g, ' ') + // `[^:]` guards the `//` in a URL (`https://…`), which is not a comment + // and can legitimately carry a filename. + .replace(/(^|[^:])\/\/[^\n]*/g, '$1 ') + ); + } + + if (/\.(yml|yaml|sh|bash)$/.test(name)) { + return text.replace(/#[^\n]*/g, ' '); + } + + // JSON and anything else: no comment syntax to strip. Returned unchanged so + // an unknown extension can never silently lose a real reference. + return text; +} diff --git a/scripts/wait-for-checks.mjs b/scripts/wait-for-checks.mjs index 6fd74929..45e17a9a 100644 --- a/scripts/wait-for-checks.mjs +++ b/scripts/wait-for-checks.mjs @@ -1,4 +1,5 @@ import { execFileSync } from 'node:child_process'; +import { checkNamesKey, evaluatePoll, REQUIRED_STABLE_POLLS } from './lib/check-settling.mjs'; /** * Block until every GitHub Actions check on a PR has concluded, and print an @@ -47,6 +48,14 @@ import { execFileSync } from 'node:child_process'; * Right after a push, GitHub Actions can take a few seconds to register * any check runs at all; treating "no rows yet" as "nothing to wait for" * would reproduce the exact failure mode in evidence #1 above. + * - ZERO ROWS was guarded; a PARTIAL set was not. GitHub registers check + * runs in batches, and a poll landing on "the three that exist so far, all + * green" satisfied `pending === 0` and returned 0 — a full matrix reported + * green on a fraction of it. Measured on PR #190 (2026-08-23): one poll + * saw a single row while twelve more were seconds away. A PASS now also + * requires the SET OF CHECK NAMES to be identical across two consecutive + * polls; a FAIL is still returned immediately. See + * `scripts/lib/check-settling.mjs`. * * Usage: * node scripts/wait-for-checks.mjs [--timeout-min N] [--interval-sec N] @@ -212,6 +221,10 @@ async function main(argv) { const deadline = Date.now() + timeoutMin * 60 * 1000; let consecutiveFailures = 0; + // The check list has to hold still before a green tally counts. See + // scripts/lib/check-settling.mjs for the poll this was measured against. + let prevNamesKey = null; + let stablePolls = 0; for (;;) { const raw = invokeGhChecks(prNumber); @@ -239,28 +252,40 @@ async function main(argv) { consecutiveFailures = 0; const { checks } = result; - if (checks.length === 0) { - log(`${now} pass=0 pending=0 fail=0 (no checks reported yet)`); - if (Date.now() >= deadline) { - log(resultLine({ pass: 0, fail: 0, pending: 0 }, 2)); - return 2; - } - await delay(intervalSec * 1000); - continue; - } - const tally = tallyChecks(checks); const notPassedSuffix = tally.notPassed.length > 0 ? ` not-passed=[${tally.notPassed.join(', ')}]` : ''; - log(`${now} pass=${tally.pass} pending=${tally.pending} fail=${tally.fail}${notPassedSuffix}`); + // A zero-row response used to be handled by its own early return above, + // which meant it also had to keep the settle bookkeeping in step — and it + // did not. `evaluatePoll` owns that case now; here it is only a label. + const emptySuffix = checks.length === 0 ? ' (no checks reported yet)' : ''; + log(`${now} pass=${tally.pass} pending=${tally.pending} fail=${tally.fail}${notPassedSuffix}${emptySuffix}`); + + const namesKey = checkNamesKey(checks); + const decision = evaluatePoll({ + tally, + namesKey, + prevNamesKey, + stablePolls, + requiredStablePolls: REQUIRED_STABLE_POLLS, + }); + prevNamesKey = namesKey; + stablePolls = decision.stablePolls; - if (tally.fail > 0) { + if (decision.verdict === 'fail') { log(resultLine(tally, 1)); return 1; } - if (tally.pending === 0) { + if (decision.verdict === 'pass') { + log(`${now} settled: ${decision.reason}`); log(resultLine(tally, 0)); return 0; } + // 'wait'. When nothing is pending the reason is the settle guard, and that + // is worth saying out loud — a silent extra poll after "pending=0" reads + // like the script is stuck. + if (tally.pending === 0 && checks.length > 0) { + log(`${now} holding: ${decision.reason}`); + } if (Date.now() >= deadline) { log(resultLine(tally, 2)); return 2; diff --git a/tests/check-settling.test.ts b/tests/check-settling.test.ts new file mode 100644 index 00000000..73c5eedf --- /dev/null +++ b/tests/check-settling.test.ts @@ -0,0 +1,156 @@ +/** + * Pins the guard that stops `wait-for-checks.mjs` calling a partial matrix green. + * + * The script's whole reason to exist is that `gh pr checks`' exit code cannot be + * trusted as a verdict. It then carried the same hole one level down: it asked + * "is pending zero?" and returned 0 when the answer was yes — sound only if + * every check has already been registered, which GitHub does in batches. + * Measured on PR #190 (2026-08-23): a poll returned one row, + * `Analyze (javascript-typescript)`, with twelve more seconds away. Had that row + * been green at that instant, thirteen legs would have been reported green on + * one of them. + * + * Both directions are asserted. In particular the FIRST poll of an all-green + * list must NOT pass, because that is precisely the shape of the bug. + */ +import { describe, it, expect } from 'vitest'; +import { + checkNamesKey, + evaluatePoll, + REQUIRED_STABLE_POLLS, +} from '../scripts/lib/check-settling.mjs'; + +type Tally = { pass: number; pending: number; fail: number }; + +const green = (n: number): Tally => ({ pass: n, pending: 0, fail: 0 }); +const waiting = (pass: number, pending: number): Tally => ({ pass, pending, fail: 0 }); + +describe('a green tally is only a verdict once the check list holds still', () => { + it('does NOT pass on the first poll, even with nothing pending', () => { + // The regression, exactly: three registered, all green, ten not yet created. + const r = evaluatePoll({ + tally: green(3), + namesKey: 'a b c', + prevNamesKey: null, + stablePolls: 0, + }); + expect(r.verdict).toBe('wait'); + expect(r.reason).toContain('batches'); + }); + + it('passes on the second poll when the same names come back', () => { + const first = evaluatePoll({ + tally: green(13), + namesKey: 'a b c', + prevNamesKey: null, + stablePolls: 0, + }); + expect(first.verdict).toBe('wait'); + + const second = evaluatePoll({ + tally: green(13), + namesKey: 'a b c', + prevNamesKey: 'a b c', + stablePolls: first.stablePolls, + }); + expect(second.verdict).toBe('pass'); + }); + + it('resets the counter when the list grows, and does not pass on that poll', () => { + // The batch that was missing shows up. Two green polls have now happened, + // but they were not about the same set. + const r = evaluatePoll({ + tally: green(13), + namesKey: 'a b c d', + prevNamesKey: 'a b c', + stablePolls: 1, + }); + expect(r.verdict).toBe('wait'); + expect(r.stablePolls).toBe(1); + }); + + it('does not pass when the same COUNT arrives under different names', () => { + // `concurrency.cancel-in-progress` can swap one run's legs for another's at + // the same cardinality. Keying on the count would call that settled. + const r = evaluatePoll({ + tally: green(3), + namesKey: 'x y z', + prevNamesKey: 'a b c', + stablePolls: 1, + }); + expect(r.verdict).toBe('wait'); + }); + + it('keeps waiting while anything is pending, however stable the list is', () => { + const r = evaluatePoll({ + tally: waiting(11, 2), + namesKey: 'a b c', + prevNamesKey: 'a b c', + stablePolls: 5, + }); + expect(r.verdict).toBe('wait'); + expect(r.reason).toContain('2 pending'); + }); + + it('never passes on a response with no rows, and resets the count', () => { + // Right after a push GitHub can report nothing for a few seconds; mid-run, + // a response that lost every row is the clearest evidence the list is not + // holding still. This used to be a short-circuit in the caller that forgot + // to touch the bookkeeping, so "13 green, zero rows, the same 13 green" + // counted as two agreeing polls. + const r = evaluatePoll({ + tally: { pass: 0, pending: 0, fail: 0 }, + namesKey: '', + prevNamesKey: '', + stablePolls: 1, + }); + expect(r.verdict).toBe('wait'); + expect(r.stablePolls).toBe(0); + }); + + it('does not settle across a zero-row poll', () => { + const first = evaluatePoll({ tally: green(13), namesKey: 'a b c', prevNamesKey: null, stablePolls: 0 }); + const empty = evaluatePoll({ tally: { pass: 0, pending: 0, fail: 0 }, namesKey: '', prevNamesKey: 'a b c', stablePolls: first.stablePolls }); + const third = evaluatePoll({ tally: green(13), namesKey: 'a b c', prevNamesKey: '', stablePolls: empty.stablePolls }); + expect(third.verdict).toBe('wait'); + }); + + it('fails immediately, without waiting for the list to settle', () => { + // A red leg is red whether or not its siblings have registered. Delaying + // bad news that cannot improve helps nobody. + const r = evaluatePoll({ + tally: { pass: 1, pending: 4, fail: 1 }, + namesKey: 'a b', + prevNamesKey: null, + stablePolls: 0, + }); + expect(r.verdict).toBe('fail'); + }); + + it('needs more than one poll by construction', () => { + // If this ever became 1 the guard would be inert while still looking present. + expect(REQUIRED_STABLE_POLLS).toBeGreaterThan(1); + }); +}); + +describe('the name key identifies the SET, not the order', () => { + it('is order-independent', () => { + expect(checkNamesKey([{ name: 'b' }, { name: 'a' }])).toBe(checkNamesKey([{ name: 'a' }, { name: 'b' }])); + }); + + it('separates different sets of the same size', () => { + expect(checkNamesKey([{ name: 'a' }, { name: 'b' }])).not.toBe( + checkNamesKey([{ name: 'a' }, { name: 'c' }]) + ); + }); + + it('does not collapse unnamed rows into each other silently', () => { + // Two rows must not key the same as one. `gh` has never omitted `name`, + // but a key that loses cardinality would let a shrinking list read stable. + expect(checkNamesKey([{}, {}])).not.toBe(checkNamesKey([{}])); + }); + + it('returns an empty key for a non-array, rather than throwing', () => { + expect(checkNamesKey(null as unknown as { name?: string }[])).toBe(''); + }); +}); diff --git a/tests/reference-corpus.test.ts b/tests/reference-corpus.test.ts new file mode 100644 index 00000000..aab5ad67 --- /dev/null +++ b/tests/reference-corpus.test.ts @@ -0,0 +1,88 @@ +/** + * Pins the rule that stops a sentence silencing the uncalled-gate detector. + * + * C3 in `scripts/audit/verification-audit.mjs` answers "does anything actually + * run this script?" by counting basename occurrences. It counted raw text, so + * prose counted. That is not a hypothetical: `measure-injection-tokens.mjs`'s + * header sentence "Companion to measure-work-topology-baseline.mjs" made the + * companion look called, and the companion's correct C3 entry was reported as + * stale and pruned. The finding was then preserved as a WARNING inside the + * other entry's reason field rather than fixed. It happened a second time the + * moment a test header named `wait-for-checks.mjs`. + * + * Both directions matter here, and the second one more than usual: a + * `stripComments` that stripped too little re-opens the hole, and one that + * stripped everything would make every script look uncalled — loud, but + * useless. + */ +// The fixtures below use INVENTED filenames on purpose. A first version used +// the real ones, and the strings holding them counted as references in exactly +// the detector this file exists to protect — the header comment above is +// stripped, a string literal is not. The property under test is "a comment is +// not a caller"; it does not need real names to be true. +import { describe, it, expect } from 'vitest'; +import { stripComments } from '../scripts/lib/reference-corpus.mjs'; + +describe('a filename in a comment is not a caller', () => { + it('drops a JS line comment', () => { + const out = stripComments("// Companion to some-companion-script.mjs\nconst a = 1;", 'x.mjs'); + expect(out).not.toContain('some-companion-script.mjs'); + expect(out).toContain('const a = 1;'); + }); + + it('drops a JSDoc block, which is where both real cases lived', () => { + const out = stripComments('/**\n * Pins the guard in `some-watcher-script.mjs`.\n */\nrun();', 'x.ts'); + expect(out).not.toContain('some-watcher-script.mjs'); + expect(out).toContain('run();'); + }); + + it('drops a YAML comment but keeps the command on the same line', () => { + const out = stripComments(' run: node scripts/smoke-test.mjs # see also scripts/other.mjs\n', 'ci.yml'); + expect(out).toContain('node scripts/smoke-test.mjs'); + expect(out).not.toContain('scripts/other.mjs'); + }); + + it('drops a shell comment', () => { + expect(stripComments('# calls scripts/foo.mjs\nnode scripts/bar.mjs\n', 'x.sh')).not.toContain('foo.mjs'); + }); +}); + +describe('what must survive stripping', () => { + it('keeps a real invocation', () => { + const src = "execFileSync('node', ['scripts/some-watcher-script.mjs']);"; + expect(stripComments(src, 'x.mjs')).toContain('scripts/some-watcher-script.mjs'); + }); + + it('keeps an import specifier', () => { + const src = "import { x } from './lib/some-lib-module.mjs';"; + expect(stripComments(src, 'x.mjs')).toContain('some-lib-module.mjs'); + }); + + it('does not treat the `//` in a URL as a comment', () => { + // A URL can legitimately carry a filename, and truncating the line there + // would delete whatever followed on it. + const src = "const u = 'https://example.com/a.mjs'; run('scripts/real.mjs');"; + expect(stripComments(src, 'x.mjs')).toContain('scripts/real.mjs'); + }); + + it('leaves package.json alone — JSON has no comments to strip', () => { + const src = '{"scripts": {"test": "node scripts/some-runner-script.mjs"}}'; + expect(stripComments(src, 'package.json')).toBe(src); + }); + + it('returns the text unchanged for an extension it does not know', () => { + // Never silently lose a reference in a file type this has no rule for. + const src = 'node scripts/foo.mjs # not necessarily a comment here'; + expect(stripComments(src, 'notes.txt')).toBe(src); + }); + + it('replaces a comment with a space rather than joining its neighbours', () => { + // `a/* c */b` must not become `ab` — that would manufacture a token that + // was never in the file. + expect(stripComments('a/* c */b', 'x.mjs')).toBe('a b'); + }); + + it('returns an empty string for a non-string, rather than throwing', () => { + expect(stripComments(null as unknown as string, 'x.mjs')).toBe(''); + }); +});