From 45cb2a022c3c84e496eda2ae36b12b3dff164462 Mon Sep 17 00:00:00 2001 From: KT <677465+kevintseng@users.noreply.github.com> Date: Sun, 23 Aug 2026 14:29:51 +0800 Subject: [PATCH 1/4] fix(ci): a green tally is not a verdict until the check list holds still MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `wait-for-checks.mjs` exists because `gh pr checks`' exit code could not 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. That is only sound if every check has 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 green at that instant, thirteen legs would have been reported green on one of them — and the whole point of this script is that I read its exit code instead of looking. A PASS now also requires the SET OF CHECK NAMES to be identical across two consecutive polls. A FAIL is still returned immediately: a red leg is red whether or not its siblings have registered. Keyed on the name set rather than the count, because `concurrency.cancel-in-progress` can swap one run's legs for another's at the same cardinality. Cost: one extra poll interval on a green run. That is the price of the verdict meaning what it says. The decision is a pure function in scripts/lib/check-settling.mjs so both directions can be pinned without a live PR — the same split, and the same reason, as scripts/lib/published-version.mjs. Break-tested: accept the first green poll / set the threshold to 1 / make the name key order-dependent — 3/3 KILLED, restore verified by sha256. --- scripts/lib/check-settling.mjs | 89 ++++++++++++++++++++++ scripts/wait-for-checks.mjs | 35 ++++++++- tests/check-settling.test.ts | 133 +++++++++++++++++++++++++++++++++ 3 files changed, 255 insertions(+), 2 deletions(-) create mode 100644 scripts/lib/check-settling.mjs create mode 100644 tests/check-settling.test.ts diff --git a/scripts/lib/check-settling.mjs b/scripts/lib/check-settling.mjs new file mode 100644 index 00000000..09a7625c --- /dev/null +++ b/scripts/lib/check-settling.mjs @@ -0,0 +1,89 @@ +// 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, +}) { + // 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/wait-for-checks.mjs b/scripts/wait-for-checks.mjs index 6fd74929..543fb07a 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); @@ -253,14 +266,32 @@ async function main(argv) { const notPassedSuffix = tally.notPassed.length > 0 ? ` not-passed=[${tally.notPassed.join(', ')}]` : ''; log(`${now} pass=${tally.pass} pending=${tally.pending} fail=${tally.fail}${notPassedSuffix}`); - if (tally.fail > 0) { + const namesKey = checkNamesKey(checks); + const decision = evaluatePoll({ + tally, + namesKey, + prevNamesKey, + stablePolls, + requiredStablePolls: REQUIRED_STABLE_POLLS, + }); + prevNamesKey = namesKey; + stablePolls = decision.stablePolls; + + 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) { + 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..a9e58da2 --- /dev/null +++ b/tests/check-settling.test.ts @@ -0,0 +1,133 @@ +/** + * 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('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(''); + }); +}); From 4ad4aad19b19192beed7563a10ea2c7794a39b74 Mon Sep 17 00:00:00 2001 From: KT <677465+kevintseng@users.noreply.github.com> Date: Sun, 23 Aug 2026 14:29:51 +0800 Subject: [PATCH 2/4] fix(docs): check the child of a nested subcommand, not just the parent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check-doc-claims` verified that every `memesh ` in the agent docs names a registered command. It captured only the FIRST word, so `memesh kg backfill` read as `kg` — a real command — and passed. The real name is `kg backfill-relations`. A gate that checks the parent and ignores the child is a gate for the half of the name that is hardly ever wrong. The gate found a live one the moment it was added: docs/api/API_REFERENCE.md:803 told readers the evidence edges are drawn by `memesh kg backfill`, a command that does not exist. Fixed here. Parents and their children are both derived from cli.ts. Per-parent sets, not one flat set: `patterns` is registered twice — top-level and under `dream` — so a flat set would have accepted `memesh config patterns`. Scanned across the living documents plus the CHANGELOG sections still being WRITTEN (`[Unreleased]` and the current version's). Older CHANGELOG sections are frozen history and may legitimately name a command that has since been retired; a typo is only cheap to fix while the entry is being written, which is also when it was missed last time. Zero mentions FAILS. Without that, a doc style change or a regex edit would leave it printing "0 mentions all resolve" forever — the exact shape of gate it was added to close. Break-tested: a mistyped child in a doc / a mistyped child in the live CHANGELOG section / the extraction stops matching / the parent extraction loses a parent — 4/4 KILLED, restore verified by sha256. --- docs/api/API_REFERENCE.md | 5 +-- scripts/check-doc-claims.mjs | 62 ++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) 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/check-doc-claims.mjs b/scripts/check-doc-claims.mjs index 08633e53..8b964538 100644 --- a/scripts/check-doc-claims.mjs +++ b/scripts/check-doc-claims.mjs @@ -40,6 +40,7 @@ import path from 'path'; import { execFileSync } from 'child_process'; import { fileURLToPath } from 'url'; import { listHookFiles } from './lib/hook-files.mjs'; +import { extractChangelogSection } from './lib/release-preconditions.mjs'; const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); const read = p => fs.readFileSync(path.join(repoRoot, p), 'utf8'); @@ -561,6 +562,67 @@ 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 in prose docs, plus the CHANGELOG sections still being WRITTEN. + // Older CHANGELOG sections are frozen history and may legitimately name a + // command that has since been retired; `[Unreleased]` and the current + // version's section are the ones a typo is still cheap to fix in. + const changelogForNested = read('CHANGELOG.md'); + const liveChangelog = [ + extractChangelogSection(changelogForNested, 'Unreleased'), + extractChangelogSection(changelogForNested, pkg.version), + ].filter(Boolean).join('\n'); + const nestedTargets = [ + ...livingDocs.map(d => [d, read(d)]), + ...(liveChangelog ? [['CHANGELOG.md (live sections)', liveChangelog]] : []), + ]; + + // Only inside backticks. That is how every document here writes a command, + // and it keeps prose like `memesh dream to consolidate` from being read as a + // subcommand named `to`. + let nestedMentions = 0; + const badNested = []; + for (const [label, text] of nestedTargets) { + for (const m of text.matchAll(/`memesh ([a-z][a-z-]*)(?: ([a-z][a-z-]*))?[^`]*`/g)) { + const [, parent, child] = m; + if (!childrenOf.has(parent) || !child) continue; + nestedMentions++; + if (!childrenOf.get(parent).has(child)) badNested.push(`${label} -> memesh ${parent} ${child}`); + } + } + // Zero mentions is not a pass. If the backtick 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 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 From cbe6f299e885291facb66f8e1763df8cb6bab3a9 Mon Sep 17 00:00:00 2001 From: KT <677465+kevintseng@users.noreply.github.com> Date: Sun, 23 Aug 2026 14:29:51 +0800 Subject: [PATCH 3/4] fix(audit): a filename written in a comment is not a caller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C3 finds gate-like scripts that nothing runs, by counting how often their basename appears across workflows, scripts, tests and package.json. It counted raw text, so prose counted. This had already happened, been diagnosed, and been worked around instead of fixed. `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 correct C3 entry was then reported as stale and pruned. Somebody noticed and wrote the explanation into the OTHER entry's reason field, where it sat as a warning not to delete the comment. It happened again in this branch the moment a test's header comment named `wait-for-checks.mjs`, and the audit invited me to prune that entry too. A detector for uncalled gates that any sentence can silence is not a detector. Comments come out before counting. Over-stripping is the safe direction: a reference lost this way makes a script look UNcalled, which fails the audit loudly and gets triaged. A reference wrongly kept hides a finding silently. Consequences, both correct: - `wait-for-checks.mjs` is a C3 hit again, and its NOT-A-GATE entry stands. - `measure-work-topology-baseline.mjs` surfaced and now carries its own entry, which the workaround note said it would need. - that obsolete note is gone from the other entry's reason. The stripper is a pure function so the "strips too much" direction can be pinned too, and the test's fixtures use INVENTED filenames — the first version used the real ones, and the strings holding them counted as references in exactly the detector the test protects. Break-tested: stop stripping block comments / line comments / YAML comments / strip too much / treat the `//` in a URL as a comment — 5/5 KILLED, restore verified by sha256. --- scripts/audit/baseline.json | 7 ++- scripts/audit/verification-audit.mjs | 6 +- scripts/lib/reference-corpus.mjs | 55 +++++++++++++++++ tests/reference-corpus.test.ts | 88 ++++++++++++++++++++++++++++ 4 files changed, 154 insertions(+), 2 deletions(-) create mode 100644 scripts/lib/reference-corpus.mjs create mode 100644 tests/reference-corpus.test.ts 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/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/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(''); + }); +}); From 7774a2cceeffd20346d60657182ba8a13badd9d0 Mon Sep 17 00:00:00 2001 From: KT <677465+kevintseng@users.noreply.github.com> Date: Sun, 23 Aug 2026 15:00:43 +0800 Subject: [PATCH 4/4] fix: three more, from reviewing the previous three MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewing my own branch before merging it, on the rule that a gate is not done until it has been looked at the way the thing it replaced should have been. **The nested-command gate was looking in the wrong half of the documents.** It scanned single-backtick spans only, in a hand-listed set of files. Measured: that missed 19 distinct nested commands living in FENCED code blocks — more than the 25 it saw — and the fenced ones are exactly what an agent copies out of llms-install.md and runs. It also never opened skills/memesh-review/SKILL.md, which was not on the list. Now whole-document, every tracked `.md`, derived from `git ls-files` rather than a list that can drift. 89 mentions across 31 files. **And the widened gate found one.** `memesh dream review`, in the 4.5.1 release notes: `git log -S "dream review" -- src/transports/cli/cli.ts` returns nothing for the file's entire history, so it is not a retired command in frozen history — it is a command that has never existed, published, telling readers to run it. The passage means `memesh dream show`, whose description is "…ALL observations, tags, source — so you can review the whole thing". Fixed. That finding also settled a design question. The first version deliberately scanned only `[Unreleased]` and the current version's section, reasoning that older sections are frozen history that may name since-retired commands. The reasoning was sound and the rule was wrong: it would have walked straight past this. The CHANGELOG is now read in full. If a nested subcommand is ever legitimately retired, its announcement will fail this gate — that is the moment to decide what to do, not a reason to stop looking now. **A zero-row poll bypassed the settle bookkeeping.** `wait-for-checks.mjs` short-circuited on an empty check list with its own early return, and that return did not touch `prevNamesKey`/`stablePolls` — so "13 green, then zero rows, then the same 13 green" counted as two consecutive agreeing polls and settled. A response that lost every row is the clearest possible evidence that the list is NOT holding still. Fixed by deleting the early return rather than patching it: `evaluatePoll` owns the empty case now (and must, or two consecutive empty polls would settle on an empty list). The script is 6 lines shorter and the case is finally reachable from a test. Break-tested, 10/10 KILLED — including the two new coverage directions (a mistyped child inside a fenced block; a mistyped child in a FROZEN older CHANGELOG section) and "let a zero-row poll count toward settling". --- CHANGELOG.md | 2 +- scripts/check-doc-claims.mjs | 60 +++++++++++++++++++--------------- scripts/lib/check-settling.mjs | 11 +++++++ scripts/wait-for-checks.mjs | 18 ++++------ tests/check-settling.test.ts | 23 +++++++++++++ 5 files changed, 75 insertions(+), 39 deletions(-) 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/scripts/check-doc-claims.mjs b/scripts/check-doc-claims.mjs index 8b964538..edda0ceb 100644 --- a/scripts/check-doc-claims.mjs +++ b/scripts/check-doc-claims.mjs @@ -40,7 +40,6 @@ import path from 'path'; import { execFileSync } from 'child_process'; import { fileURLToPath } from 'url'; import { listHookFiles } from './lib/hook-files.mjs'; -import { extractChangelogSection } from './lib/release-preconditions.mjs'; const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); const read = p => fs.readFileSync(path.join(repoRoot, p), 'utf8'); @@ -589,39 +588,48 @@ if (!hasBearerAuth) { if (kids.size === 0) fail(`\`${parentName}\` matched no subcommands — the nested-command extraction stopped working`); } - // Scanned in prose docs, plus the CHANGELOG sections still being WRITTEN. - // Older CHANGELOG sections are frozen history and may legitimately name a - // command that has since been retired; `[Unreleased]` and the current - // version's section are the ones a typo is still cheap to fix in. - const changelogForNested = read('CHANGELOG.md'); - const liveChangelog = [ - extractChangelogSection(changelogForNested, 'Unreleased'), - extractChangelogSection(changelogForNested, pkg.version), - ].filter(Boolean).join('\n'); - const nestedTargets = [ - ...livingDocs.map(d => [d, read(d)]), - ...(liveChangelog ? [['CHANGELOG.md (live sections)', liveChangelog]] : []), - ]; - - // Only inside backticks. That is how every document here writes a command, - // and it keeps prose like `memesh dream to consolidate` from being read as a - // subcommand named `to`. + // 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`(? memesh ${parent} ${child}`); + if (!childrenOf.get(m[1]).has(m[2])) badNested.push(`${label} -> ${m[0]}`); } } - // Zero mentions is not a pass. If the backtick 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. + // 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 all resolve`); + 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. // diff --git a/scripts/lib/check-settling.mjs b/scripts/lib/check-settling.mjs index 09a7625c..c958d9d0 100644 --- a/scripts/lib/check-settling.mjs +++ b/scripts/lib/check-settling.mjs @@ -58,6 +58,17 @@ export function evaluatePoll({ 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) { diff --git a/scripts/wait-for-checks.mjs b/scripts/wait-for-checks.mjs index 543fb07a..45e17a9a 100644 --- a/scripts/wait-for-checks.mjs +++ b/scripts/wait-for-checks.mjs @@ -252,19 +252,13 @@ 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({ @@ -289,7 +283,7 @@ async function main(argv) { // '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) { + if (tally.pending === 0 && checks.length > 0) { log(`${now} holding: ${decision.reason}`); } if (Date.now() >= deadline) { diff --git a/tests/check-settling.test.ts b/tests/check-settling.test.ts index a9e58da2..73c5eedf 100644 --- a/tests/check-settling.test.ts +++ b/tests/check-settling.test.ts @@ -92,6 +92,29 @@ describe('a green tally is only a verdict once the check list holds still', () = 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.