From fac2dd8a3454db0f0f6a555f8f6675251808b555 Mon Sep 17 00:00:00 2001 From: Nick Date: Sat, 5 Sep 2026 22:33:08 +0300 Subject: [PATCH 1/8] feat: add delivery-parity gate for review agent ports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds node lib/check-delivery-parity.mjs as a CI step, comparing the two deliveries of each review agent (agents/X.md for Claude Code vs opencode/agents/X.md for OpenCode) on the set of outcomes each side can report — mapped from the rubric's own words (Approve/Clean/Healthy, Warning/Concerns, Block/At-risk/UB-found, INCOMPLETE (not run)), never from the uppercase machine tokens: an OpenCode agent's VERDICT: X line enumerates all four tokens regardless of which the rubric above it can actually reach, so counting tokens would be blind to a port that lost an outcome entirely. It deliberately does not compare anything else: guidance, rubric detail, and worked examples in the two bodies can still diverge with this gate green. nix-reviewer stays unpaired by design (no OpenCode Nix profile). Also updates AGENTS.md's Commands table and the now-false "nothing checks that the two deliveries agree" gotcha, and drops an unused readFileSync import left over from check-delivery-parity.test.mjs's draft. --- .github/workflows/ci.yml | 5 + AGENTS.md | 3 +- lib/check-delivery-parity.mjs | 153 +++++++++++++++++++++++++++++ lib/check-delivery-parity.test.mjs | 114 +++++++++++++++++++++ 4 files changed, 274 insertions(+), 1 deletion(-) create mode 100644 lib/check-delivery-parity.mjs create mode 100644 lib/check-delivery-parity.test.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b27c89d..22b42c0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,6 +42,11 @@ jobs: # Validate skill/agent frontmatter and that every internal craft: reference resolves. - name: Check skills & agents run: node lib/check-skills.mjs + # Compares the SET OF OUTCOMES each review agent's two deliveries (agents/ vs opencode/agents/) + # can report, mapped from the rubric's words — not the two bodies in full, which can still + # diverge in guidance and detail with this gate green. + - name: Check delivery parity + run: node lib/check-delivery-parity.mjs # The opencode delivery's agents/commands answer to a different frontmatter contract than the # Claude Code ones above (no `name`, no pinned `model`, `mode: subagent`, `hidden: true`, # `tools` a map with write/edit false), so it gets its own checker rather than a widened one. diff --git a/AGENTS.md b/AGENTS.md index 5c34c8b..029912b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -154,6 +154,7 @@ Node.js 22 (CI pins `node-version: '22'`), plain ESM JavaScript. **No runtime de | Syntax-check workflow scripts | `node lib/check-workflows.mjs` | | Check skills and agents (frontmatter + `craft:` references) | `node lib/check-skills.mjs` | | Check the evals corpus | `node lib/check-evals.mjs` | +| Check the two deliveries of each review agent report the same outcomes | `node lib/check-delivery-parity.mjs` | | Validate plugin manifests | `claude plugin validate . --strict` | | Lint | `npm run lint` (`eslint lib opencode/plugin --max-warnings 0`, identical to CI) | | Typecheck the OpenCode plugin's TypeScript | `npm run check:types` (needs `npm ci --prefix opencode/plugin` once) | @@ -184,7 +185,7 @@ No formatter. There **is** a typechecker now: `tsc --noEmit --strict` over `open - **`lib/run-record.mjs` and `opencode/plugin/run-record.mjs` are NOT copies** — they are different modules sharing a filename (`lib` carries `countBySeverity`, `reviewVerdict`, the triage and fingerprint helpers; `opencode` carries `parseVerdict`, `buildAuditRecord`, `buildTriageRecord` and writes files directly). So editing "the other copy" edits the wrong file. **But they share exactly one export, `indexProjection`, and its two bodies already disagree** — verified 2026-09-03: the `lib` one emits `craftVersion`, `craftCommit`, `branch`, `head`, `round` and `outputTokens` and defaults `findingsTotal` to `0`; the OpenCode one emits none of the six and defaults to `null`. That is the trap in both directions: a column added to the index in `lib` is not added by the OpenCode writer, and lines from the two deliveries silently carry different shapes into one `index.jsonl`. No gate compares them. - **The genuinely duplicated surfaces are `lib/run-logging.mjs`, `lib/run-record.mjs` and `lib/review-coverage.mjs`**, each inlined verbatim into the workflow engines inside `// >>> craft-inline` fences (`run-logging` and `run-record` into all four). That one IS gated: `node lib/check-workflows.mjs` byte-compares every region against the source and `--fix` regenerates them, so change the source and regenerate — never hand-edit a region. One exception, and it is deliberate: `review.js`'s `shq` sits **outside** the fence (excluded to avoid a duplicate declaration), so the checker is blind to it; a tripwire in `lib/review-coverage.test.mjs` compares it instead. - **The OpenCode delivery has exactly one gate per surface, and no more.** `index.ts`, `orchestrator.ts`, `rust-audit.ts` and `triage-findings.ts` are compiled in CI by `tsc --noEmit --strict` (step "Typecheck opencode plugin") — but they remain outside ESLint (it globs `.js`/`.mjs`/`.cjs`, no TS parser configured) and outside `node --test`, so a type-clean file can still be wrong. `opencode/agents/*.md` and `opencode/commands/*.md` are shape-checked by `opencode/scripts/check-frontmatter.py` (step "Check opencode agent/command frontmatter"), which validates frontmatter shape only — it says nothing about whether a description triggers, and it never reads the body. Those two directories are also **not** in `lib/check-skills.mjs`'s foreign-plugin scan, which covers exactly `README.md`, `MAP.md`, `CLAUDE.md`, `opencode/README.md` and `opencode/install.sh` (plus `skills/`, `agents/`, `workflows/`): a `superpowers:` reference inside an OpenCode agent or command body is read by no checker at all. And `opencode/README.md` and `install.sh` are scanned for that and nothing else. - - **Nothing checks that the two deliveries agree — and that is the hole the gates do not close.** Every agent ships twice, once under `agents/` for Claude Code and once under `opencode/agents/`, and the two bodies are maintained by hand. No gate compares them, so they can diverge in *content* with CI fully green: the checkers above judge each file's shape in isolation, and shape is exactly what stays valid while meaning drifts apart. This is not hypothetical — as of 2026-09-02 on this branch, `agents/rust-reviewer.md` mentions `INCOMPLETE` 7 times and `opencode/agents/rust-reviewer.md` **0** times (it has no INCOMPLETE verdict path at all), the other three counterparts carry 2-3 against 6-7, and `agents/nix-reviewer.md` has no `opencode/agents/` counterpart whatsoever. Counted with `awk '{n+=gsub(/INCOMPLETE/,"")} END{print n+0}'` over each file. So: after editing an agent on either side, check the other side by hand — a green CI is not evidence that you did. + - **`node lib/check-delivery-parity.mjs` closes one hole and states plainly what it leaves open.** Every review agent ships twice, once under `agents/` for Claude Code and once under `opencode/agents/`, and the two bodies are maintained by hand. The gate compares the SET OF OUTCOMES each side can report — mapped onto `ok` / `concern` / `bad` / `incomplete` from the rubric's own words (`Approve`/`Clean`/`Healthy`, `Warning`/`Concerns`, `Block`/`At-risk`/`UB-found`, `INCOMPLETE (not run)`), never from the uppercase machine tokens: an OpenCode agent must additionally end with a `VERDICT: X` line whose instruction enumerates all four tokens, so a port that lost an outcome entirely would still contain the word — counting tokens would be blind to exactly the defect this gate exists for. It does **not** compare anything else: guidance, rubric detail, worked examples, and every other sentence in the two bodies can still diverge with this gate green. Re-measured 2026-09-05 with `awk '{n+=gsub(/INCOMPLETE/,"")} END{print n+0}'` over each file: `rust-reviewer` 7 (root) vs 8 (opencode), `rust-architecture-reviewer` 7 vs 5, `rust-security-scanner` 6 vs 4, `rust-miri` 6 vs 6 — the token now appears on both sides of all four pairs. The 2026-09-02 numbers this replaces (7 vs 0 for `rust-reviewer`, with the other three at 2-3 vs 6-7) did not just go stale — they described an actual missing-outcome defect that has since been fixed; the count was never the right measure of it, either then or now: the enumeration in an OpenCode agent's `VERDICT: X` instruction line names all four tokens regardless of which outcomes the rubric above it can actually reach, so a port that lost `INCOMPLETE` entirely still contains the word and a raw count reads it as covered. What the gate checks instead — reachability of the rubric phrase, not presence of the token — is what makes the current all-tokens-present state a real "all clean" rather than a coincidence. `agents/nix-reviewer.md` has no `opencode/agents/` counterpart by design (`UNPAIRED_BY_DESIGN` in `lib/check-delivery-parity.mjs`: the OpenCode delivery ships no Nix profile). So: after editing an agent on either side, still check the other side by hand for meaning — the gate proves the outcome set matches, not that the two bodies say the same thing. - **The two agent frontmatter contracts are genuinely different, hence two checkers.** Root `agents/*.md` (Claude Code) require `name` matching the filename, a pinned `model`, and `tools` as a list. `opencode/agents/*.md` require the opposite on two of those: no `name` (identity comes from the filename) and **no** `model` (an OpenCode sub-agent must inherit the session model), plus `mode: subagent`, `hidden: true`, and `tools` as a *map* with `write`/`edit` false. Widening `lib/check-skills.mjs` to cover both would mean two rule sets behind one entry point; the existing Python script already encodes the OpenCode one. - **`opencode/plugin/` pins its runtime dependencies exactly and commits `package-lock.json`.** `@opencode-ai/plugin` and `@opencode-ai/sdk` were `"*"` — any upstream release could break CI at an arbitrary moment. CI installs them with `npm ci --prefix opencode/plugin`. This closure is deliberately separate from the root `package.json`, which stays devDependencies-only so the shipped plugin installs with nothing to fetch. The toolchain is pinned exactly too (`typescript` `5.9.3`, `@types/node` `25.9.5`, not ranges): `npm ci` would honour the lockfile either way, but under a range an `npm install` here can pull a newer `tsc` whose new checks fail CI for reasons unrelated to the diff. Bumping either is then a visible, deliberate commit. - **Workflow scripts cannot be checked with `node --check`** — they have top-level `export` + `await` + `return`. `lib/check-workflows.mjs` reproduces the sandbox wrapper and compiles each one; when you change the shape of a workflow script, verify the wrapper still accepts it. diff --git a/lib/check-delivery-parity.mjs b/lib/check-delivery-parity.mjs new file mode 100644 index 0000000..534fd19 --- /dev/null +++ b/lib/check-delivery-parity.mjs @@ -0,0 +1,153 @@ +// Every review agent ships TWICE — once under `agents/` for Claude Code, once under +// `opencode/agents/` — and the two bodies are maintained by hand. Until this checker, nothing +// compared them: each existing gate judges one file's SHAPE in isolation, and shape is exactly what +// stays valid while meaning drifts apart. +// +// That is not a hypothetical. A fix shipped for the Claude Code delivery did not reach the port and +// survived there verbatim — an unrun Miri reported `Clean` — and no green CI showed it, because +// nothing in CI was looking at the pair. +// +// WHAT IS COMPARED, and why not more. Not the bodies: the two contracts differ on purpose (root +// agents carry `name` + a pinned `model` + `tools` as a list; opencode agents carry none of those +// and add `mode: subagent`). Not the counts either: "INCOMPLETE appears 7 times here and 5 there" +// is noise a reviewer cannot act on. What is compared is the SET OF VERDICTS EACH SIDE CAN REPORT. +// A verdict the root agent can reach and its port cannot is the exact failure above: a whole outcome +// missing from one delivery, which is meaning, not formatting. +// +// The vocabulary is not remembered — a tripwire in check-delivery-parity.test.mjs runs every word +// through the parser that actually reads these verdicts and asserts it lands on the outcome claimed +// here. A list held by memory drifts from the engine the moment either moves, which is the defect +// class this file exists inside. +import fs from 'node:fs' +import path from 'node:path' +import { fileURLToPath } from 'node:url' + +// The OUTCOMES an agent can reach, in its RUBRIC's words. The uppercase machine tokens are +// deliberately absent from this table, and that is the whole design. +// +// Each delivery expresses a verdict differently, and the difference is CONTRACTUAL, not a defect: an +// OpenCode agent must additionally end with a machine-read `VERDICT: X` line, because its dispatcher +// parses that line; a Claude Code agent reports in the rubric's words and emits a JSON record. +// Comparing the words as written flagged every pair — measuring the transport and calling it the +// meaning, which is the false-positive shape this repository keeps producing. Worse, the machine +// line ENUMERATES all four tokens, so a port that lost an outcome entirely still contains the word: +// counting `INCOMPLETE` would have been blind to exactly the defect this checker exists for. +// +// So `incomplete` is keyed on the rubric phrase `INCOMPLETE (not run)`, which names the outcome, +// rather than on the bare token, which merely enumerates it. +export const OUTCOMES = { + ok: ['Approve', 'Clean', 'Healthy'], + concern: ['Warning', 'Concerns'], + bad: ['Block', 'At-risk', 'UB-found'], + incomplete: ['INCOMPLETE (not run)'], +} + +// Which outcomes a body can report. Case-SENSITIVE, so the rubric word is not confused with the +// uppercase token that transports it. Whole-word, with the boundary spelled rather than `\b` — +// `At-risk` and `UB-found` carry a hyphen, and `\b` matches between `k` and `-`, which would find +// `At-risk` inside `At-risky`. +export function outcomesReported(body) { + const text = String(body ?? '') + const found = new Set() + for (const [outcome, words] of Object.entries(OUTCOMES)) { + for (const w of words) { + const esc = w.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + if (new RegExp(`(? !there.has(t)) + const onlyPort = [...there].filter((t) => !here.has(t)) + if (onlyRoot.length) { + problems.push( + `${name}: agents/ can report ${onlyRoot.join(', ')} and opencode/agents/ cannot — ` + + 'an outcome reachable in one delivery and not the other', + ) + } + if (onlyPort.length) { + problems.push( + `${name}: opencode/agents/ can report ${onlyPort.join(', ')} and agents/ cannot — ` + + 'an outcome reachable in one delivery and not the other', + ) + } + } + + return problems +} + +export function readAgents(dir) { + const out = new Map() + let names = [] + try { + names = fs.readdirSync(dir) + } catch { + return out + } + for (const f of names) { + if (!f.endsWith('.md')) continue + out.set(f.slice(0, -3), fs.readFileSync(path.join(dir, f), 'utf8')) + } + return out +} + +// ── CLI mode ────────────────────────────────────────────────────────────────────────────────── +if (import.meta.url === `file://${process.argv[1]}`) { + const rootDir = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..') + const root = readAgents(path.join(rootDir, 'agents')) + const opencode = readAgents(path.join(rootDir, 'opencode', 'agents')) + + if (root.size === 0) { + // An empty read is the one way this checker passes without checking anything. + console.error('FAIL agents/ :: no agent files found — the checker would pass vacuously') + process.exit(1) + } + + const problems = checkParity(root, opencode) + for (const p of problems) console.error('FAIL delivery parity ::', p) + const paired = [...root.keys()].filter((n) => opencode.has(n)).length + console.log(`compared ${paired} agent pair(s) across ${root.size} Claude Code and ${opencode.size} OpenCode agents`) + console.log(problems.length ? `\n${problems.length} problem(s)` : '\nall clean') + process.exit(problems.length ? 1 : 0) +} diff --git a/lib/check-delivery-parity.test.mjs b/lib/check-delivery-parity.test.mjs new file mode 100644 index 0000000..19a4204 --- /dev/null +++ b/lib/check-delivery-parity.test.mjs @@ -0,0 +1,114 @@ +// The gate that compares the two deliveries, and the one that must be shown FIRING: a checker for +// a defect nobody can reproduce is a checker nobody trusts. The first case here is the defect that +// actually happened. +import { test } from 'node:test' +import assert from 'node:assert/strict' +import { parseVerdict } from '../opencode/plugin/run-record.mjs' +import { OUTCOMES, outcomesReported, checkParity, readAgents } from './check-delivery-parity.mjs' + +// The shape an OpenCode agent has to carry: a rubric, then the machine-read line whose instruction +// ENUMERATES all four tokens. The enumeration is what makes a naive word-count blind. +const MACHINE_LINE = + 'End with `VERDICT: X` where X is exactly one of the four tokens `APPROVE`, `WARNING`, `BLOCK`, `INCOMPLETE`.' + +test('a port that lost the INCOMPLETE path is caught, though it still enumerates the token', () => { + // The defect this checker exists for, in its own words: a fix shipped for Claude Code did not + // reach the port, an unrun Miri reported Clean, and every gate stayed green because each judged + // one file's shape alone. Note the port below CONTAINS the string `INCOMPLETE` — counting the + // token would call it covered, which is why the rubric phrase is what is measured. + const root = new Map([ + ['rust-miri', 'Rate: **Clean** if Miri ran and found nothing, **UB-found** ⛔ Block otherwise.\nIf nightly or miri is missing, report `INCOMPLETE (not run)` — an unrun Miri is never Clean.'], + ]) + const port = new Map([ + ['rust-miri', `Rate: **Clean** if Miri ran and found nothing, **UB-found** otherwise.\n${MACHINE_LINE}`], + ]) + const problems = checkParity(root, port) + assert.equal(problems.length, 1) + assert.match(problems[0], /rust-miri/) + assert.match(problems[0], /agents\/ can report incomplete and opencode\/agents\/ cannot/) +}) + +test('the machine line alone is not a divergence', () => { + // The false positive the first two versions of this checker produced, twice: the OpenCode port + // must carry `VERDICT: X` and the Claude Code agent must not, so comparing the words as written + // flagged every pair. Measuring the transport and calling it the meaning is the shape this + // repository keeps producing; here it is pinned so a later "tightening" cannot bring it back. + const rubric = 'Rate **Healthy** / **Concerns** / **At-risk**, or `INCOMPLETE (not run)` if no graph was built.' + const root = new Map([['rust-architecture-reviewer', rubric]]) + const port = new Map([['rust-architecture-reviewer', `${rubric}\n${MACHINE_LINE}`]]) + assert.deepEqual(checkParity(root, port), []) +}) + +test('an outcome the port gained and the root lacks is caught too', () => { + // Both directions: a port that grew a verdict its origin cannot reach is the same divergence, + // and it is the likelier one now that the port is edited by hand more often. + const root = new Map([['rust-reviewer', 'Verdicts: **Approve** / **Block**, or `INCOMPLETE (not run)`.']]) + const port = new Map([ + ['rust-reviewer', 'Verdicts: **Approve** / **Warning** / **Block**, or `INCOMPLETE (not run)`.'], + ]) + const problems = checkParity(root, port) + assert.equal(problems.length, 1) + assert.match(problems[0], /opencode\/agents\/ can report concern and agents\/ cannot/) +}) + +test('an unpaired agent must be excused by name, and the excuse expires', () => { + const body = 'Verdicts: **Approve** / **Warning** / **Block**, or `INCOMPLETE (not run)`.' + + // Not excused: shipping for one delivery only is a divergence until someone says otherwise. + const surprise = checkParity(new Map([['rust-semver', body]]), new Map()) + assert.equal(surprise.length, 1) + assert.match(surprise[0], /ships only for Claude Code/) + + // Excused by the real list: nix-reviewer has no OpenCode profile to call it. + assert.deepEqual(checkParity(new Map([['nix-reviewer', body]]), new Map()), []) + + // And the excuse cannot outlive what it excused: once the port exists, the entry is stale. + const stale = checkParity(new Map([['nix-reviewer', body]]), new Map([['nix-reviewer', body]])) + assert.equal(stale.length, 1) + assert.match(stale[0], /UNPAIRED_BY_DESIGN/) + assert.match(stale[0], /now exists — drop the entry/) +}) + +test('a port of nothing is caught', () => { + const body = 'Verdicts: **Approve** / **Block**, or `INCOMPLETE (not run)`.' + const problems = checkParity(new Map(), new Map([['ghost', body]])) + assert.equal(problems.length, 1) + assert.match(problems[0], /has no counterpart under agents\//) +}) + +test('a rubric word is not found inside a longer one', () => { + // `\b` matches between `k` and `-`, so a naive boundary finds `At-risk` inside `At-risky` and + // `Block` inside `Blocking`. The hyphenated words are the reason the boundary is spelled out. + assert.deepEqual([...outcomesReported('Blocking issues are listed below.')], []) + assert.deepEqual([...outcomesReported('The design is At-risky at best.')], []) + assert.deepEqual([...outcomesReported('Cleanup is pending.')], []) + assert.deepEqual([...outcomesReported('Rate it **At-risk**.')], ['bad']) +}) + +test('the bare token is not the rubric phrase', () => { + // The enumeration in the machine line contains `INCOMPLETE`; only the rubric phrase names the + // outcome. This is the single distinction the first case above rests on. + assert.deepEqual([...outcomesReported(MACHINE_LINE)], []) + assert.deepEqual([...outcomesReported('report `INCOMPLETE (not run)` and stop')], ['incomplete']) +}) + +test('the vocabulary is what the parser actually reads', () => { + // The tripwire. Every word in the table is run through the parser that reads these verdicts in + // production, and must land on the outcome the table claims. A vocabulary held by memory drifts + // from the engine the moment either moves — which is the defect class this checker lives inside, + // so the checker is not allowed to hold one. + const expected = { ok: 'Approve', concern: 'Warning', bad: 'Block', incomplete: 'INCOMPLETE (not run)' } + for (const [outcome, words] of Object.entries(OUTCOMES)) { + for (const w of words) { + assert.equal(parseVerdict(w), expected[outcome], `${w} should read as ${expected[outcome]}`) + } + } +}) + +test('the repository itself is in parity', () => { + // The gate over the real tree, so a divergence landing on main fails here and not only in CI. + const root = readAgents(new URL('../agents', import.meta.url).pathname) + const port = readAgents(new URL('../opencode/agents', import.meta.url).pathname) + assert.ok(root.size > 0, 'agents/ must not read empty — an empty read passes vacuously') + assert.deepEqual(checkParity(root, port), []) +}) From 98f6d1ddea868ea8ed4c09f762ab89b878e07ba4 Mon Sep 17 00:00:00 2001 From: Nick Date: Sat, 5 Sep 2026 22:48:32 +0300 Subject: [PATCH 2/8] fix: measure the agent body, not the whole file, and close three vacuous checks in delivery-parity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit check-delivery-parity measured the whole agent file, so the YAML description line alone could satisfy every outcome — three of the four pairs on this branch derived their entire measured outcome set from the description line, with zero signal from the body. Strip frontmatter before measuring, so an outcome dropped from the body (e.g. INCOMPLETE gutted to APPROVE) is actually caught; pin it against the real opencode/agents/rust-reviewer.md. Also: the "ok" row's vocabulary tripwire asserted parseVerdict's return value, but its fallthrough is Approve by design, so any word (real or invented) passed — assert recognition via hasVerdictLine instead. The word-boundary test exercised only the \w part of the boundary class, never the hyphen the comment claims it pins — add assertions that actually require it (Semi-Clean, Block-list). And the UNPAIRED_BY_DESIGN staleness check only walked names present in root, so an entry whose root file was deleted was never revisited — iterate the allowlist's own keys too. --- AGENTS.md | 2 +- lib/check-delivery-parity.mjs | 32 +++++++++- lib/check-delivery-parity.test.mjs | 96 ++++++++++++++++++++++++++---- 3 files changed, 115 insertions(+), 15 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 029912b..97b2d63 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -185,7 +185,7 @@ No formatter. There **is** a typechecker now: `tsc --noEmit --strict` over `open - **`lib/run-record.mjs` and `opencode/plugin/run-record.mjs` are NOT copies** — they are different modules sharing a filename (`lib` carries `countBySeverity`, `reviewVerdict`, the triage and fingerprint helpers; `opencode` carries `parseVerdict`, `buildAuditRecord`, `buildTriageRecord` and writes files directly). So editing "the other copy" edits the wrong file. **But they share exactly one export, `indexProjection`, and its two bodies already disagree** — verified 2026-09-03: the `lib` one emits `craftVersion`, `craftCommit`, `branch`, `head`, `round` and `outputTokens` and defaults `findingsTotal` to `0`; the OpenCode one emits none of the six and defaults to `null`. That is the trap in both directions: a column added to the index in `lib` is not added by the OpenCode writer, and lines from the two deliveries silently carry different shapes into one `index.jsonl`. No gate compares them. - **The genuinely duplicated surfaces are `lib/run-logging.mjs`, `lib/run-record.mjs` and `lib/review-coverage.mjs`**, each inlined verbatim into the workflow engines inside `// >>> craft-inline` fences (`run-logging` and `run-record` into all four). That one IS gated: `node lib/check-workflows.mjs` byte-compares every region against the source and `--fix` regenerates them, so change the source and regenerate — never hand-edit a region. One exception, and it is deliberate: `review.js`'s `shq` sits **outside** the fence (excluded to avoid a duplicate declaration), so the checker is blind to it; a tripwire in `lib/review-coverage.test.mjs` compares it instead. - **The OpenCode delivery has exactly one gate per surface, and no more.** `index.ts`, `orchestrator.ts`, `rust-audit.ts` and `triage-findings.ts` are compiled in CI by `tsc --noEmit --strict` (step "Typecheck opencode plugin") — but they remain outside ESLint (it globs `.js`/`.mjs`/`.cjs`, no TS parser configured) and outside `node --test`, so a type-clean file can still be wrong. `opencode/agents/*.md` and `opencode/commands/*.md` are shape-checked by `opencode/scripts/check-frontmatter.py` (step "Check opencode agent/command frontmatter"), which validates frontmatter shape only — it says nothing about whether a description triggers, and it never reads the body. Those two directories are also **not** in `lib/check-skills.mjs`'s foreign-plugin scan, which covers exactly `README.md`, `MAP.md`, `CLAUDE.md`, `opencode/README.md` and `opencode/install.sh` (plus `skills/`, `agents/`, `workflows/`): a `superpowers:` reference inside an OpenCode agent or command body is read by no checker at all. And `opencode/README.md` and `install.sh` are scanned for that and nothing else. - - **`node lib/check-delivery-parity.mjs` closes one hole and states plainly what it leaves open.** Every review agent ships twice, once under `agents/` for Claude Code and once under `opencode/agents/`, and the two bodies are maintained by hand. The gate compares the SET OF OUTCOMES each side can report — mapped onto `ok` / `concern` / `bad` / `incomplete` from the rubric's own words (`Approve`/`Clean`/`Healthy`, `Warning`/`Concerns`, `Block`/`At-risk`/`UB-found`, `INCOMPLETE (not run)`), never from the uppercase machine tokens: an OpenCode agent must additionally end with a `VERDICT: X` line whose instruction enumerates all four tokens, so a port that lost an outcome entirely would still contain the word — counting tokens would be blind to exactly the defect this gate exists for. It does **not** compare anything else: guidance, rubric detail, worked examples, and every other sentence in the two bodies can still diverge with this gate green. Re-measured 2026-09-05 with `awk '{n+=gsub(/INCOMPLETE/,"")} END{print n+0}'` over each file: `rust-reviewer` 7 (root) vs 8 (opencode), `rust-architecture-reviewer` 7 vs 5, `rust-security-scanner` 6 vs 4, `rust-miri` 6 vs 6 — the token now appears on both sides of all four pairs. The 2026-09-02 numbers this replaces (7 vs 0 for `rust-reviewer`, with the other three at 2-3 vs 6-7) did not just go stale — they described an actual missing-outcome defect that has since been fixed; the count was never the right measure of it, either then or now: the enumeration in an OpenCode agent's `VERDICT: X` instruction line names all four tokens regardless of which outcomes the rubric above it can actually reach, so a port that lost `INCOMPLETE` entirely still contains the word and a raw count reads it as covered. What the gate checks instead — reachability of the rubric phrase, not presence of the token — is what makes the current all-tokens-present state a real "all clean" rather than a coincidence. `agents/nix-reviewer.md` has no `opencode/agents/` counterpart by design (`UNPAIRED_BY_DESIGN` in `lib/check-delivery-parity.mjs`: the OpenCode delivery ships no Nix profile). So: after editing an agent on either side, still check the other side by hand for meaning — the gate proves the outcome set matches, not that the two bodies say the same thing. + - **`node lib/check-delivery-parity.mjs` closes one hole and states plainly what it leaves open.** Every review agent ships twice, once under `agents/` for Claude Code and once under `opencode/agents/`, and the two bodies are maintained by hand. The gate compares the SET OF OUTCOMES each side can report — mapped onto `ok` / `concern` / `bad` / `incomplete` from the rubric's own words (`Approve`/`Clean`/`Healthy`, `Warning`/`Concerns`, `Block`/`At-risk`/`UB-found`, `INCOMPLETE (not run)`), never from the uppercase machine tokens: an OpenCode agent must additionally end with a `VERDICT: X` line whose instruction enumerates all four tokens, so a port that lost an outcome entirely would still contain the word — counting tokens would be blind to exactly the defect this gate exists for. The gate strips each file's leading YAML frontmatter before measuring, so it reads the BODY, not the file — the frontmatter description line alone can carry every outcome word, and did: three of the four pairs on the branch that introduced this gate derived their entire measured outcome set from the description line alone, with zero signal from the body. The two description lines are themselves now unmeasured; whether they agree is a separate, open parity question, not this gate's job. It does **not** compare anything else: guidance, rubric detail, worked examples, and every other sentence in the two bodies can still diverge with this gate green. Re-measured 2026-09-05 with `awk '{n+=gsub(/INCOMPLETE/,"")} END{print n+0}'` over each file: `rust-reviewer` 7 (root) vs 8 (opencode), `rust-architecture-reviewer` 7 vs 5, `rust-security-scanner` 6 vs 4, `rust-miri` 6 vs 6 — the token now appears on both sides of all four pairs. The 2026-09-02 numbers this replaces (7 vs 0 for `rust-reviewer`, with the other three at 2-3 vs 6-7) did not just go stale — they described an actual missing-outcome defect that has since been fixed; the count was never the right measure of it, either then or now: the enumeration in an OpenCode agent's `VERDICT: X` instruction line names all four tokens regardless of which outcomes the rubric above it can actually reach, so a port that lost `INCOMPLETE` entirely still contains the word and a raw count reads it as covered. What the gate checks instead — reachability of the rubric phrase, not presence of the token — is what makes the current all-tokens-present state a real "all clean" rather than a coincidence. `agents/nix-reviewer.md` has no `opencode/agents/` counterpart by design (`UNPAIRED_BY_DESIGN` in `lib/check-delivery-parity.mjs`: the OpenCode delivery ships no Nix profile). So: after editing an agent on either side, still check the other side by hand for meaning — the gate proves the outcome set matches, not that the two bodies say the same thing. - **The two agent frontmatter contracts are genuinely different, hence two checkers.** Root `agents/*.md` (Claude Code) require `name` matching the filename, a pinned `model`, and `tools` as a list. `opencode/agents/*.md` require the opposite on two of those: no `name` (identity comes from the filename) and **no** `model` (an OpenCode sub-agent must inherit the session model), plus `mode: subagent`, `hidden: true`, and `tools` as a *map* with `write`/`edit` false. Widening `lib/check-skills.mjs` to cover both would mean two rule sets behind one entry point; the existing Python script already encodes the OpenCode one. - **`opencode/plugin/` pins its runtime dependencies exactly and commits `package-lock.json`.** `@opencode-ai/plugin` and `@opencode-ai/sdk` were `"*"` — any upstream release could break CI at an arbitrary moment. CI installs them with `npm ci --prefix opencode/plugin`. This closure is deliberately separate from the root `package.json`, which stays devDependencies-only so the shipped plugin installs with nothing to fetch. The toolchain is pinned exactly too (`typescript` `5.9.3`, `@types/node` `25.9.5`, not ranges): `npm ci` would honour the lockfile either way, but under a range an `npm install` here can pull a newer `tsc` whose new checks fail CI for reasons unrelated to the diff. Bumping either is then a visible, deliberate commit. - **Workflow scripts cannot be checked with `node --check`** — they have top-level `export` + `await` + `return`. `lib/check-workflows.mjs` reproduces the sandbox wrapper and compiles each one; when you change the shape of a workflow script, verify the wrapper still accepts it. diff --git a/lib/check-delivery-parity.mjs b/lib/check-delivery-parity.mjs index 534fd19..0c1e300 100644 --- a/lib/check-delivery-parity.mjs +++ b/lib/check-delivery-parity.mjs @@ -14,6 +14,11 @@ // A verdict the root agent can reach and its port cannot is the exact failure above: a whole outcome // missing from one delivery, which is meaning, not formatting. // +// What is measured is the BODY, with the leading YAML frontmatter block stripped first — the +// frontmatter's `description:` line alone can carry every outcome word, which made the gate pass +// vacuously on a body gutted of its outcomes. The two `description:` lines are therefore now +// unmeasured by this gate; whether they agree with each other is a separate, open parity question. +// // The vocabulary is not remembered — a tripwire in check-delivery-parity.test.mjs runs every word // through the parser that actually reads these verdicts and asserts it lands on the outcome claimed // here. A list held by memory drifts from the engine the moment either moves, which is the defect @@ -42,12 +47,24 @@ export const OUTCOMES = { incomplete: ['INCOMPLETE (not run)'], } +// Strips a leading YAML frontmatter block (`---` … `---`) so the gate measures the BODY, not the +// file. Without this, the `description:` line alone can satisfy every outcome — verified: the real +// opencode/agents/rust-reviewer.md still reads as fully covered even with all 7 body occurrences of +// INCOMPLETE replaced by APPROVE, because the description line carries the words on its own. Three +// of the four pairs on this branch derived their whole outcome set from the description alone. +// NOTE: the `description:` lines themselves are now unmeasured by this gate — whether the two +// descriptions agree is its own, separate parity question, not addressed here. +function stripFrontmatter(text) { + const m = /^---\r?\n[\s\S]*?\r?\n---\r?\n/.exec(text) + return m ? text.slice(m[0].length) : text +} + // Which outcomes a body can report. Case-SENSITIVE, so the rubric word is not confused with the // uppercase token that transports it. Whole-word, with the boundary spelled rather than `\b` — // `At-risk` and `UB-found` carry a hyphen, and `\b` matches between `k` and `-`, which would find // `At-risk` inside `At-risky`. export function outcomesReported(body) { - const text = String(body ?? '') + const text = stripFrontmatter(String(body ?? '')) const found = new Set() for (const [outcome, words] of Object.entries(OUTCOMES)) { for (const w of words) { @@ -73,6 +90,19 @@ export const UNPAIRED_BY_DESIGN = { export function checkParity(root, opencode) { const problems = [] + // The staleness check on UNPAIRED_BY_DESIGN, in the direction the root-keyed loop below cannot + // reach: that loop only visits names present in `root`, so an entry whose ROOT file was deleted + // (present in neither map) is never visited and the excuse lingers for nothing. Iterating the + // allowlist's own keys catches that: an excuse for a name that exists on neither side is dead + // weight, not a real exception. + for (const name of Object.keys(UNPAIRED_BY_DESIGN)) { + if (!root.has(name) && !opencode.has(name)) { + problems.push( + `UNPAIRED_BY_DESIGN has an entry for "${name}", but it exists on neither side — drop the entry`, + ) + } + } + for (const name of opencode.keys()) { if (!root.has(name)) { problems.push(`opencode/agents/${name}.md has no counterpart under agents/ — a port of nothing`) diff --git a/lib/check-delivery-parity.test.mjs b/lib/check-delivery-parity.test.mjs index 19a4204..18e8866 100644 --- a/lib/check-delivery-parity.test.mjs +++ b/lib/check-delivery-parity.test.mjs @@ -3,14 +3,19 @@ // actually happened. import { test } from 'node:test' import assert from 'node:assert/strict' -import { parseVerdict } from '../opencode/plugin/run-record.mjs' -import { OUTCOMES, outcomesReported, checkParity, readAgents } from './check-delivery-parity.mjs' +import { parseVerdict, hasVerdictLine } from '../opencode/plugin/run-record.mjs' +import { OUTCOMES, outcomesReported, checkParity, readAgents, UNPAIRED_BY_DESIGN } from './check-delivery-parity.mjs' // The shape an OpenCode agent has to carry: a rubric, then the machine-read line whose instruction // ENUMERATES all four tokens. The enumeration is what makes a naive word-count blind. const MACHINE_LINE = 'End with `VERDICT: X` where X is exactly one of the four tokens `APPROVE`, `WARNING`, `BLOCK`, `INCOMPLETE`.' +// Present in `root` alone in every synthetic map below, so the real `nix-reviewer` entry in +// UNPAIRED_BY_DESIGN stays satisfied (it names a real, excused exception) and these unrelated tests +// don't also trip the "excuse for a name on neither side" staleness check added below. +const NIX_ENTRY = ['nix-reviewer', 'Verdicts: **Approve** / **Block**, or `INCOMPLETE (not run)`.'] + test('a port that lost the INCOMPLETE path is caught, though it still enumerates the token', () => { // The defect this checker exists for, in its own words: a fix shipped for Claude Code did not // reach the port, an unrun Miri reported Clean, and every gate stayed green because each judged @@ -18,6 +23,7 @@ test('a port that lost the INCOMPLETE path is caught, though it still enumerates // token would call it covered, which is why the rubric phrase is what is measured. const root = new Map([ ['rust-miri', 'Rate: **Clean** if Miri ran and found nothing, **UB-found** ⛔ Block otherwise.\nIf nightly or miri is missing, report `INCOMPLETE (not run)` — an unrun Miri is never Clean.'], + NIX_ENTRY, ]) const port = new Map([ ['rust-miri', `Rate: **Clean** if Miri ran and found nothing, **UB-found** otherwise.\n${MACHINE_LINE}`], @@ -34,7 +40,7 @@ test('the machine line alone is not a divergence', () => { // flagged every pair. Measuring the transport and calling it the meaning is the shape this // repository keeps producing; here it is pinned so a later "tightening" cannot bring it back. const rubric = 'Rate **Healthy** / **Concerns** / **At-risk**, or `INCOMPLETE (not run)` if no graph was built.' - const root = new Map([['rust-architecture-reviewer', rubric]]) + const root = new Map([['rust-architecture-reviewer', rubric], NIX_ENTRY]) const port = new Map([['rust-architecture-reviewer', `${rubric}\n${MACHINE_LINE}`]]) assert.deepEqual(checkParity(root, port), []) }) @@ -42,7 +48,7 @@ test('the machine line alone is not a divergence', () => { test('an outcome the port gained and the root lacks is caught too', () => { // Both directions: a port that grew a verdict its origin cannot reach is the same divergence, // and it is the likelier one now that the port is edited by hand more often. - const root = new Map([['rust-reviewer', 'Verdicts: **Approve** / **Block**, or `INCOMPLETE (not run)`.']]) + const root = new Map([['rust-reviewer', 'Verdicts: **Approve** / **Block**, or `INCOMPLETE (not run)`.'], NIX_ENTRY]) const port = new Map([ ['rust-reviewer', 'Verdicts: **Approve** / **Warning** / **Block**, or `INCOMPLETE (not run)`.'], ]) @@ -55,7 +61,8 @@ test('an unpaired agent must be excused by name, and the excuse expires', () => const body = 'Verdicts: **Approve** / **Warning** / **Block**, or `INCOMPLETE (not run)`.' // Not excused: shipping for one delivery only is a divergence until someone says otherwise. - const surprise = checkParity(new Map([['rust-semver', body]]), new Map()) + // nix-reviewer stands in `root` too, so the real UNPAIRED_BY_DESIGN entry stays satisfied here. + const surprise = checkParity(new Map([['rust-semver', body], ['nix-reviewer', body]]), new Map()) assert.equal(surprise.length, 1) assert.match(surprise[0], /ships only for Claude Code/) @@ -69,9 +76,30 @@ test('an unpaired agent must be excused by name, and the excuse expires', () => assert.match(stale[0], /now exists — drop the entry/) }) +test('a stale excuse for a deleted root file is caught too', () => { + // The direction the root-keyed loop cannot reach: it only visits names present in `root`, so an + // UNPAIRED_BY_DESIGN entry whose root file was DELETED (present in neither map) is never visited + // by that loop and its excuse lingers forever. A synthetic allowlist entry for a name absent from + // both sides pins this without touching the real `nix-reviewer` exception. + const original = { ...UNPAIRED_BY_DESIGN } + UNPAIRED_BY_DESIGN['ghost-reviewer'] = 'stood in for a deleted root file' + try { + const problems = checkParity(new Map(), new Map()) + assert.ok( + problems.some((p) => p.includes('ghost-reviewer') && p.includes('neither side')), + 'an UNPAIRED_BY_DESIGN entry for a name on neither side must be flagged as stale', + ) + } finally { + for (const k of Object.keys(UNPAIRED_BY_DESIGN)) delete UNPAIRED_BY_DESIGN[k] + Object.assign(UNPAIRED_BY_DESIGN, original) + } +}) + test('a port of nothing is caught', () => { const body = 'Verdicts: **Approve** / **Block**, or `INCOMPLETE (not run)`.' - const problems = checkParity(new Map(), new Map([['ghost', body]])) + // nix-reviewer stands in `root` (excused, no port) so the real UNPAIRED_BY_DESIGN entry stays + // satisfied and does not add an unrelated staleness problem to this map. + const problems = checkParity(new Map([['nix-reviewer', body]]), new Map([['ghost', body]])) assert.equal(problems.length, 1) assert.match(problems[0], /has no counterpart under agents\//) }) @@ -83,6 +111,14 @@ test('a rubric word is not found inside a longer one', () => { assert.deepEqual([...outcomesReported('The design is At-risky at best.')], []) assert.deepEqual([...outcomesReported('Cleanup is pending.')], []) assert.deepEqual([...outcomesReported('Rate it **At-risk**.')], ['bad']) + + // The hyphen specifically: `(? { @@ -93,18 +129,52 @@ test('the bare token is not the rubric phrase', () => { }) test('the vocabulary is what the parser actually reads', () => { - // The tripwire. Every word in the table is run through the parser that reads these verdicts in - // production, and must land on the outcome the table claims. A vocabulary held by memory drifts - // from the engine the moment either moves — which is the defect class this checker lives inside, - // so the checker is not allowed to hold one. - const expected = { ok: 'Approve', concern: 'Warning', bad: 'Block', incomplete: 'INCOMPLETE (not run)' } - for (const [outcome, words] of Object.entries(OUTCOMES)) { - for (const w of words) { + // The tripwire. `parseVerdict`'s fallthrough is `Approve` by design (`parseVerdict('banana')` is + // `Approve`), so asserting the RETURNED TOKEN for the `ok` row is vacuous — any word, real or + // invented, would pass. `hasVerdictLine` is the one that distinguishes recognised vocabulary from + // fallthrough, so the `ok` row is pinned by RECOGNITION: each word, in a realistic `Verdict: X` + // line, must be recognised — and a control non-word in the same shape must not be. The other three + // rows keep the direct `parseVerdict` assertion: their outcomes are not the fallthrough, so + // asserting the returned token there is not vacuous. + for (const w of OUTCOMES.ok) { + assert.equal(hasVerdictLine(`Verdict: ${w}`), true, `${w} should be recognised as a reported verdict`) + } + assert.equal( + hasVerdictLine('Verdict: banana'), + false, + 'an invented word must NOT be recognised — otherwise the ok-row assertions above prove nothing', + ) + + const expected = { concern: 'Warning', bad: 'Block', incomplete: 'INCOMPLETE (not run)' } + for (const outcome of ['concern', 'bad', 'incomplete']) { + for (const w of OUTCOMES[outcome]) { assert.equal(parseVerdict(w), expected[outcome], `${w} should read as ${expected[outcome]}`) } } }) +test('the gate reads the body, not the frontmatter — a real file gutted of body signal fails', () => { + // The falsifier from the review: take the real opencode/agents/rust-reviewer.md, replace every + // body occurrence of INCOMPLETE with APPROVE (gutting the not-run path), leave the frontmatter + // untouched. Before this gate stripped frontmatter, the description line alone kept the outcome + // set fully covered and the mutation went undetected. + const root = readAgents(new URL('../agents', import.meta.url).pathname) + const port = readAgents(new URL('../opencode/agents', import.meta.url).pathname) + const real = port.get('rust-reviewer') + const fmEnd = real.indexOf('\n---\n', real.indexOf('---\n') + 4) + 5 + const frontmatter = real.slice(0, fmEnd) + const body = real.slice(fmEnd) + const gutted = frontmatter + body.replaceAll('INCOMPLETE', 'APPROVE') + + const mutatedPort = new Map(port) + mutatedPort.set('rust-reviewer', gutted) + const problems = checkParity(root, mutatedPort) + assert.ok( + problems.some((p) => p.includes('rust-reviewer')), + 'a body-level INCOMPLETE→APPROVE mutation must be caught even though the frontmatter still names INCOMPLETE', + ) +}) + test('the repository itself is in parity', () => { // The gate over the real tree, so a divergence landing on main fails here and not only in CI. const root = readAgents(new URL('../agents', import.meta.url).pathname) From 70491a1da851cfe3df64b5f6d114af60a5d506d5 Mon Sep 17 00:00:00 2001 From: Nick Date: Sat, 5 Sep 2026 22:52:13 +0300 Subject: [PATCH 3/8] refactor: make checkParity's allowlist an injectable parameter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UNPAIRED_BY_DESIGN was read as a module-level constant inside checkParity, so every synthetic-map test had to pad in the real nix-reviewer entry just to stay quiet under the staleness loop added in 98f6d1d. A second entry in the real allowlist would have broken every one of those unrelated tests, and the obvious fix — pad again — would have made each test assert less about its own subject with every entry added. checkParity now takes `unpaired = UNPAIRED_BY_DESIGN` as a third parameter. Synthetic-map tests pass their own allowlist (usually {}); tests about the excuse mechanism state their own premise instead of borrowing the real one; the whole-tree test and the CLI keep the default so the real allowlist is still exercised against the real tree. Added a test pinning the property this buys: a second, unrelated allowlist entry must not disturb a test about something else. --- lib/check-delivery-parity.mjs | 12 +++--- lib/check-delivery-parity.test.mjs | 67 ++++++++++++++---------------- 2 files changed, 39 insertions(+), 40 deletions(-) diff --git a/lib/check-delivery-parity.mjs b/lib/check-delivery-parity.mjs index 0c1e300..71942f0 100644 --- a/lib/check-delivery-parity.mjs +++ b/lib/check-delivery-parity.mjs @@ -86,16 +86,18 @@ export const UNPAIRED_BY_DESIGN = { 'nix-reviewer': 'the OpenCode delivery ships no Nix profile — there is no nix-review command to call it', } -// `root` and `opencode` are Maps of agent name → body. Returns a list of problems (empty = clean). -export function checkParity(root, opencode) { +// `root` and `opencode` are Maps of agent name → body. `unpaired` defaults to the real +// UNPAIRED_BY_DESIGN allowlist; a caller may inject its own so a test about something else does +// not have to carry the whole real allowlist just to stay quiet under it (realm @nick/craft). +export function checkParity(root, opencode, unpaired = UNPAIRED_BY_DESIGN) { const problems = [] - // The staleness check on UNPAIRED_BY_DESIGN, in the direction the root-keyed loop below cannot + // The staleness check on `unpaired`, in the direction the root-keyed loop below cannot // reach: that loop only visits names present in `root`, so an entry whose ROOT file was deleted // (present in neither map) is never visited and the excuse lingers for nothing. Iterating the // allowlist's own keys catches that: an excuse for a name that exists on neither side is dead // weight, not a real exception. - for (const name of Object.keys(UNPAIRED_BY_DESIGN)) { + for (const name of Object.keys(unpaired)) { if (!root.has(name) && !opencode.has(name)) { problems.push( `UNPAIRED_BY_DESIGN has an entry for "${name}", but it exists on neither side — drop the entry`, @@ -110,7 +112,7 @@ export function checkParity(root, opencode) { } for (const [name, body] of root) { - const excuse = UNPAIRED_BY_DESIGN[name] + const excuse = unpaired[name] if (!opencode.has(name)) { if (!excuse) { problems.push( diff --git a/lib/check-delivery-parity.test.mjs b/lib/check-delivery-parity.test.mjs index 18e8866..6df59e5 100644 --- a/lib/check-delivery-parity.test.mjs +++ b/lib/check-delivery-parity.test.mjs @@ -4,18 +4,13 @@ import { test } from 'node:test' import assert from 'node:assert/strict' import { parseVerdict, hasVerdictLine } from '../opencode/plugin/run-record.mjs' -import { OUTCOMES, outcomesReported, checkParity, readAgents, UNPAIRED_BY_DESIGN } from './check-delivery-parity.mjs' +import { OUTCOMES, outcomesReported, checkParity, readAgents } from './check-delivery-parity.mjs' // The shape an OpenCode agent has to carry: a rubric, then the machine-read line whose instruction // ENUMERATES all four tokens. The enumeration is what makes a naive word-count blind. const MACHINE_LINE = 'End with `VERDICT: X` where X is exactly one of the four tokens `APPROVE`, `WARNING`, `BLOCK`, `INCOMPLETE`.' -// Present in `root` alone in every synthetic map below, so the real `nix-reviewer` entry in -// UNPAIRED_BY_DESIGN stays satisfied (it names a real, excused exception) and these unrelated tests -// don't also trip the "excuse for a name on neither side" staleness check added below. -const NIX_ENTRY = ['nix-reviewer', 'Verdicts: **Approve** / **Block**, or `INCOMPLETE (not run)`.'] - test('a port that lost the INCOMPLETE path is caught, though it still enumerates the token', () => { // The defect this checker exists for, in its own words: a fix shipped for Claude Code did not // reach the port, an unrun Miri reported Clean, and every gate stayed green because each judged @@ -23,12 +18,11 @@ test('a port that lost the INCOMPLETE path is caught, though it still enumerates // token would call it covered, which is why the rubric phrase is what is measured. const root = new Map([ ['rust-miri', 'Rate: **Clean** if Miri ran and found nothing, **UB-found** ⛔ Block otherwise.\nIf nightly or miri is missing, report `INCOMPLETE (not run)` — an unrun Miri is never Clean.'], - NIX_ENTRY, ]) const port = new Map([ ['rust-miri', `Rate: **Clean** if Miri ran and found nothing, **UB-found** otherwise.\n${MACHINE_LINE}`], ]) - const problems = checkParity(root, port) + const problems = checkParity(root, port, {}) assert.equal(problems.length, 1) assert.match(problems[0], /rust-miri/) assert.match(problems[0], /agents\/ can report incomplete and opencode\/agents\/ cannot/) @@ -40,37 +34,37 @@ test('the machine line alone is not a divergence', () => { // flagged every pair. Measuring the transport and calling it the meaning is the shape this // repository keeps producing; here it is pinned so a later "tightening" cannot bring it back. const rubric = 'Rate **Healthy** / **Concerns** / **At-risk**, or `INCOMPLETE (not run)` if no graph was built.' - const root = new Map([['rust-architecture-reviewer', rubric], NIX_ENTRY]) + const root = new Map([['rust-architecture-reviewer', rubric]]) const port = new Map([['rust-architecture-reviewer', `${rubric}\n${MACHINE_LINE}`]]) - assert.deepEqual(checkParity(root, port), []) + assert.deepEqual(checkParity(root, port, {}), []) }) test('an outcome the port gained and the root lacks is caught too', () => { // Both directions: a port that grew a verdict its origin cannot reach is the same divergence, // and it is the likelier one now that the port is edited by hand more often. - const root = new Map([['rust-reviewer', 'Verdicts: **Approve** / **Block**, or `INCOMPLETE (not run)`.'], NIX_ENTRY]) + const root = new Map([['rust-reviewer', 'Verdicts: **Approve** / **Block**, or `INCOMPLETE (not run)`.']]) const port = new Map([ ['rust-reviewer', 'Verdicts: **Approve** / **Warning** / **Block**, or `INCOMPLETE (not run)`.'], ]) - const problems = checkParity(root, port) + const problems = checkParity(root, port, {}) assert.equal(problems.length, 1) assert.match(problems[0], /opencode\/agents\/ can report concern and agents\/ cannot/) }) test('an unpaired agent must be excused by name, and the excuse expires', () => { const body = 'Verdicts: **Approve** / **Warning** / **Block**, or `INCOMPLETE (not run)`.' + const excuse = { 'nix-reviewer': 'the OpenCode delivery ships no Nix profile — there is no nix-review command to call it' } // Not excused: shipping for one delivery only is a divergence until someone says otherwise. - // nix-reviewer stands in `root` too, so the real UNPAIRED_BY_DESIGN entry stays satisfied here. - const surprise = checkParity(new Map([['rust-semver', body], ['nix-reviewer', body]]), new Map()) + const surprise = checkParity(new Map([['rust-semver', body]]), new Map(), {}) assert.equal(surprise.length, 1) assert.match(surprise[0], /ships only for Claude Code/) - // Excused by the real list: nix-reviewer has no OpenCode profile to call it. - assert.deepEqual(checkParity(new Map([['nix-reviewer', body]]), new Map()), []) + // Excused by an explicit allowlist entry: nix-reviewer has no OpenCode profile to call it. + assert.deepEqual(checkParity(new Map([['nix-reviewer', body]]), new Map(), excuse), []) // And the excuse cannot outlive what it excused: once the port exists, the entry is stale. - const stale = checkParity(new Map([['nix-reviewer', body]]), new Map([['nix-reviewer', body]])) + const stale = checkParity(new Map([['nix-reviewer', body]]), new Map([['nix-reviewer', body]]), excuse) assert.equal(stale.length, 1) assert.match(stale[0], /UNPAIRED_BY_DESIGN/) assert.match(stale[0], /now exists — drop the entry/) @@ -78,32 +72,35 @@ test('an unpaired agent must be excused by name, and the excuse expires', () => test('a stale excuse for a deleted root file is caught too', () => { // The direction the root-keyed loop cannot reach: it only visits names present in `root`, so an - // UNPAIRED_BY_DESIGN entry whose root file was DELETED (present in neither map) is never visited - // by that loop and its excuse lingers forever. A synthetic allowlist entry for a name absent from - // both sides pins this without touching the real `nix-reviewer` exception. - const original = { ...UNPAIRED_BY_DESIGN } - UNPAIRED_BY_DESIGN['ghost-reviewer'] = 'stood in for a deleted root file' - try { - const problems = checkParity(new Map(), new Map()) - assert.ok( - problems.some((p) => p.includes('ghost-reviewer') && p.includes('neither side')), - 'an UNPAIRED_BY_DESIGN entry for a name on neither side must be flagged as stale', - ) - } finally { - for (const k of Object.keys(UNPAIRED_BY_DESIGN)) delete UNPAIRED_BY_DESIGN[k] - Object.assign(UNPAIRED_BY_DESIGN, original) - } + // allowlist entry whose ROOT file was DELETED (present in neither map) is never visited by that + // loop and its excuse lingers forever. A synthetic allowlist entry for a name absent from both + // sides pins this — it states its own premise rather than depending on the real allowlist. + const problems = checkParity(new Map(), new Map(), { 'ghost-reviewer': 'stood in for a deleted root file' }) + assert.ok( + problems.some((p) => p.includes('ghost-reviewer') && p.includes('neither side')), + 'an UNPAIRED_BY_DESIGN entry for a name on neither side must be flagged as stale', + ) }) test('a port of nothing is caught', () => { const body = 'Verdicts: **Approve** / **Block**, or `INCOMPLETE (not run)`.' - // nix-reviewer stands in `root` (excused, no port) so the real UNPAIRED_BY_DESIGN entry stays - // satisfied and does not add an unrelated staleness problem to this map. - const problems = checkParity(new Map([['nix-reviewer', body]]), new Map([['ghost', body]])) + const problems = checkParity(new Map(), new Map([['ghost', body]]), {}) assert.equal(problems.length, 1) assert.match(problems[0], /has no counterpart under agents\//) }) +test('a second, unrelated allowlist entry does not disturb a test about something else', () => { + // The property the injectable parameter buys: adding a second entry to UNPAIRED_BY_DESIGN must + // not break a test whose subject is not the allowlist. Before this parameter, every synthetic + // map had to carry the real allowlist's contents to stay quiet, so a second real entry would have + // forced padding at every such call site (realm @nick/craft). + const body = 'Verdicts: **Approve** / **Block**, or `INCOMPLETE (not run)`.' + const root = new Map([['rust-reviewer', body], ['nix-reviewer', body], ['some-other-agent', body]]) + const port = new Map([['rust-reviewer', body]]) + const twoEntryAllowlist = { 'nix-reviewer': 'reason one', 'some-other-agent': 'reason two' } + assert.deepEqual(checkParity(root, port, twoEntryAllowlist), []) +}) + test('a rubric word is not found inside a longer one', () => { // `\b` matches between `k` and `-`, so a naive boundary finds `At-risk` inside `At-risky` and // `Block` inside `Blocking`. The hyphenated words are the reason the boundary is spelled out. From bc36bf1cdd70d8388dd420fb7523cbf109e615bd Mon Sep 17 00:00:00 2001 From: Nick Date: Sat, 5 Sep 2026 23:00:55 +0300 Subject: [PATCH 4/8] docs: correct the delivery-parity frontmatter claim, which was never measured MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The claim that "three of the four pairs derived their entire measured outcome set from the description line alone, with zero signal from the body" was a reviewer's phrasing relayed without verification. Measured directly with outcomesReported over all 8 agent files: every body carries the full outcome set on its own (frontmatter stripped or not), and it is the frontmatter ALONE that reaches the full set in 7 of 8 files (missing only rust-miri's `bad`) — the opposite of the recorded claim. That near- total frontmatter coverage is what actually justifies stripping it: it would mask a body that lost an outcome, exactly as the file's own demonstration (opencode/agents/rust-reviewer.md read as covered with all INCOMPLETE occurrences swapped for APPROVE) already showed. Also names a ceiling the gate did not state: outcomesReported matches a rubric word anywhere in the text, including ordinary prose, so a port that drops a verdict path while still mentioning the word in passing still passes. And switches check-delivery-parity.test.mjs to fileURLToPath, matching the rest of lib/ (new URL(...).pathname leaves percent-encoding and can misresolve on a checkout path with a space or #). Trims the AGENTS.md delivery-parity bullet, which had grown to duplicate the checker's own header comment. --- AGENTS.md | 2 +- lib/check-delivery-parity.mjs | 33 +++++++++++++++++++++++------- lib/check-delivery-parity.test.mjs | 9 ++++---- 3 files changed, 32 insertions(+), 12 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 97b2d63..c4de567 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -185,7 +185,7 @@ No formatter. There **is** a typechecker now: `tsc --noEmit --strict` over `open - **`lib/run-record.mjs` and `opencode/plugin/run-record.mjs` are NOT copies** — they are different modules sharing a filename (`lib` carries `countBySeverity`, `reviewVerdict`, the triage and fingerprint helpers; `opencode` carries `parseVerdict`, `buildAuditRecord`, `buildTriageRecord` and writes files directly). So editing "the other copy" edits the wrong file. **But they share exactly one export, `indexProjection`, and its two bodies already disagree** — verified 2026-09-03: the `lib` one emits `craftVersion`, `craftCommit`, `branch`, `head`, `round` and `outputTokens` and defaults `findingsTotal` to `0`; the OpenCode one emits none of the six and defaults to `null`. That is the trap in both directions: a column added to the index in `lib` is not added by the OpenCode writer, and lines from the two deliveries silently carry different shapes into one `index.jsonl`. No gate compares them. - **The genuinely duplicated surfaces are `lib/run-logging.mjs`, `lib/run-record.mjs` and `lib/review-coverage.mjs`**, each inlined verbatim into the workflow engines inside `// >>> craft-inline` fences (`run-logging` and `run-record` into all four). That one IS gated: `node lib/check-workflows.mjs` byte-compares every region against the source and `--fix` regenerates them, so change the source and regenerate — never hand-edit a region. One exception, and it is deliberate: `review.js`'s `shq` sits **outside** the fence (excluded to avoid a duplicate declaration), so the checker is blind to it; a tripwire in `lib/review-coverage.test.mjs` compares it instead. - **The OpenCode delivery has exactly one gate per surface, and no more.** `index.ts`, `orchestrator.ts`, `rust-audit.ts` and `triage-findings.ts` are compiled in CI by `tsc --noEmit --strict` (step "Typecheck opencode plugin") — but they remain outside ESLint (it globs `.js`/`.mjs`/`.cjs`, no TS parser configured) and outside `node --test`, so a type-clean file can still be wrong. `opencode/agents/*.md` and `opencode/commands/*.md` are shape-checked by `opencode/scripts/check-frontmatter.py` (step "Check opencode agent/command frontmatter"), which validates frontmatter shape only — it says nothing about whether a description triggers, and it never reads the body. Those two directories are also **not** in `lib/check-skills.mjs`'s foreign-plugin scan, which covers exactly `README.md`, `MAP.md`, `CLAUDE.md`, `opencode/README.md` and `opencode/install.sh` (plus `skills/`, `agents/`, `workflows/`): a `superpowers:` reference inside an OpenCode agent or command body is read by no checker at all. And `opencode/README.md` and `install.sh` are scanned for that and nothing else. - - **`node lib/check-delivery-parity.mjs` closes one hole and states plainly what it leaves open.** Every review agent ships twice, once under `agents/` for Claude Code and once under `opencode/agents/`, and the two bodies are maintained by hand. The gate compares the SET OF OUTCOMES each side can report — mapped onto `ok` / `concern` / `bad` / `incomplete` from the rubric's own words (`Approve`/`Clean`/`Healthy`, `Warning`/`Concerns`, `Block`/`At-risk`/`UB-found`, `INCOMPLETE (not run)`), never from the uppercase machine tokens: an OpenCode agent must additionally end with a `VERDICT: X` line whose instruction enumerates all four tokens, so a port that lost an outcome entirely would still contain the word — counting tokens would be blind to exactly the defect this gate exists for. The gate strips each file's leading YAML frontmatter before measuring, so it reads the BODY, not the file — the frontmatter description line alone can carry every outcome word, and did: three of the four pairs on the branch that introduced this gate derived their entire measured outcome set from the description line alone, with zero signal from the body. The two description lines are themselves now unmeasured; whether they agree is a separate, open parity question, not this gate's job. It does **not** compare anything else: guidance, rubric detail, worked examples, and every other sentence in the two bodies can still diverge with this gate green. Re-measured 2026-09-05 with `awk '{n+=gsub(/INCOMPLETE/,"")} END{print n+0}'` over each file: `rust-reviewer` 7 (root) vs 8 (opencode), `rust-architecture-reviewer` 7 vs 5, `rust-security-scanner` 6 vs 4, `rust-miri` 6 vs 6 — the token now appears on both sides of all four pairs. The 2026-09-02 numbers this replaces (7 vs 0 for `rust-reviewer`, with the other three at 2-3 vs 6-7) did not just go stale — they described an actual missing-outcome defect that has since been fixed; the count was never the right measure of it, either then or now: the enumeration in an OpenCode agent's `VERDICT: X` instruction line names all four tokens regardless of which outcomes the rubric above it can actually reach, so a port that lost `INCOMPLETE` entirely still contains the word and a raw count reads it as covered. What the gate checks instead — reachability of the rubric phrase, not presence of the token — is what makes the current all-tokens-present state a real "all clean" rather than a coincidence. `agents/nix-reviewer.md` has no `opencode/agents/` counterpart by design (`UNPAIRED_BY_DESIGN` in `lib/check-delivery-parity.mjs`: the OpenCode delivery ships no Nix profile). So: after editing an agent on either side, still check the other side by hand for meaning — the gate proves the outcome set matches, not that the two bodies say the same thing. + - **`node lib/check-delivery-parity.mjs` compares the SET OF OUTCOMES each agent's two deliveries (`agents/` and `opencode/agents/`) can report** — `ok`/`concern`/`bad`/`incomplete`, matched against the rubric's own words with the leading YAML frontmatter stripped so it reads the BODY, not the file (the checker's header comment carries the full rationale, including why frontmatter alone would mask a lost outcome). It does **not** compare guidance, rubric detail, or wording — those can diverge with this gate green. `agents/nix-reviewer.md` has no `opencode/agents/` counterpart by design (`UNPAIRED_BY_DESIGN` in the checker: the OpenCode delivery ships no Nix profile). Re-measured 2026-09-05 with `awk '{n+=gsub(/INCOMPLETE/,"")} END{print n+0}'` over each file: `rust-reviewer` 7 (root) vs 8 (opencode), `rust-architecture-reviewer` 7 vs 5, `rust-security-scanner` 6 vs 4, `rust-miri` 6 vs 6 — the token appears on both sides of all four pairs, but a raw count is not what the gate checks (see the checker's comment for why). After editing an agent on either side, still check the other side by hand for meaning — the gate proves the outcome set matches, not that the two bodies say the same thing. - **The two agent frontmatter contracts are genuinely different, hence two checkers.** Root `agents/*.md` (Claude Code) require `name` matching the filename, a pinned `model`, and `tools` as a list. `opencode/agents/*.md` require the opposite on two of those: no `name` (identity comes from the filename) and **no** `model` (an OpenCode sub-agent must inherit the session model), plus `mode: subagent`, `hidden: true`, and `tools` as a *map* with `write`/`edit` false. Widening `lib/check-skills.mjs` to cover both would mean two rule sets behind one entry point; the existing Python script already encodes the OpenCode one. - **`opencode/plugin/` pins its runtime dependencies exactly and commits `package-lock.json`.** `@opencode-ai/plugin` and `@opencode-ai/sdk` were `"*"` — any upstream release could break CI at an arbitrary moment. CI installs them with `npm ci --prefix opencode/plugin`. This closure is deliberately separate from the root `package.json`, which stays devDependencies-only so the shipped plugin installs with nothing to fetch. The toolchain is pinned exactly too (`typescript` `5.9.3`, `@types/node` `25.9.5`, not ranges): `npm ci` would honour the lockfile either way, but under a range an `npm install` here can pull a newer `tsc` whose new checks fail CI for reasons unrelated to the diff. Bumping either is then a visible, deliberate commit. - **Workflow scripts cannot be checked with `node --check`** — they have top-level `export` + `await` + `return`. `lib/check-workflows.mjs` reproduces the sandbox wrapper and compiles each one; when you change the shape of a workflow script, verify the wrapper still accepts it. diff --git a/lib/check-delivery-parity.mjs b/lib/check-delivery-parity.mjs index 71942f0..8eebbce 100644 --- a/lib/check-delivery-parity.mjs +++ b/lib/check-delivery-parity.mjs @@ -14,15 +14,32 @@ // A verdict the root agent can reach and its port cannot is the exact failure above: a whole outcome // missing from one delivery, which is meaning, not formatting. // -// What is measured is the BODY, with the leading YAML frontmatter block stripped first — the -// frontmatter's `description:` line alone can carry every outcome word, which made the gate pass -// vacuously on a body gutted of its outcomes. The two `description:` lines are therefore now +// What is measured is the BODY, with the leading YAML frontmatter block stripped first. Measured +// directly: every one of the 8 agent bodies carries the full outcome set on its own — the +// frontmatter strip changes nothing about body coverage. What it changes is what the FRONTMATTER +// ALONE would otherwise mask: the `description:` line alone reaches the full outcome set for 7 of +// the 8 files, and is one outcome short (`bad`, missing from `rust-miri`'s description) for the +// 8th — so measuring the unstripped file would have reported "all clean" almost everywhere even if +// a body had lost an outcome entirely, because the description sitting above it already covers for +// it. That is exactly the failure the frontmatter strip exists to prevent: see the demonstration two +// paragraphs below, where all 7 body occurrences of INCOMPLETE in the real +// opencode/agents/rust-reviewer.md are replaced by APPROVE and the file still reads as fully +// covered — by the description, not the body. The two `description:` lines are therefore now // unmeasured by this gate; whether they agree with each other is a separate, open parity question. // // The vocabulary is not remembered — a tripwire in check-delivery-parity.test.mjs runs every word // through the parser that actually reads these verdicts and asserts it lands on the outcome claimed // here. A list held by memory drifts from the engine the moment either moves, which is the defect // class this file exists inside. +// +// CEILING, stated plainly: `outcomesReported` matches any occurrence of a rubric word anywhere in +// the body, including inside ordinary prose — `outcomesReported('Never Block on style nits; do not +// report Concerns for formatting.')` returns `bad` and `concern` though neither is a verdict path. +// So a port that drops an actual outcome-reporting branch while leaving the word mentioned in +// passing prose (a caveat, a comparison, a removed example) still reads as covering that outcome. +// This gate proves the word is reachable somewhere in the text, not that the body still contains a +// working path to report it. Accepted: catching that would need parsing the rubric's control flow, +// which this checker does not attempt. import fs from 'node:fs' import path from 'node:path' import { fileURLToPath } from 'node:url' @@ -48,10 +65,12 @@ export const OUTCOMES = { } // Strips a leading YAML frontmatter block (`---` … `---`) so the gate measures the BODY, not the -// file. Without this, the `description:` line alone can satisfy every outcome — verified: the real -// opencode/agents/rust-reviewer.md still reads as fully covered even with all 7 body occurrences of -// INCOMPLETE replaced by APPROVE, because the description line carries the words on its own. Three -// of the four pairs on this branch derived their whole outcome set from the description alone. +// file. Without this, the `description:` line alone can satisfy nearly every outcome — measured +// across all 8 agent files: the description alone reaches the full outcome set in 7 of 8, missing +// only `bad` in `rust-miri`'s. That near-total coverage is what makes the frontmatter dangerous to +// measure: verified, the real opencode/agents/rust-reviewer.md still reads as fully covered even +// with all 7 body occurrences of INCOMPLETE replaced by APPROVE, because the description line above +// it already carries the words on its own — a body that lost an outcome entirely would still pass. // NOTE: the `description:` lines themselves are now unmeasured by this gate — whether the two // descriptions agree is its own, separate parity question, not addressed here. function stripFrontmatter(text) { diff --git a/lib/check-delivery-parity.test.mjs b/lib/check-delivery-parity.test.mjs index 6df59e5..44f4650 100644 --- a/lib/check-delivery-parity.test.mjs +++ b/lib/check-delivery-parity.test.mjs @@ -3,6 +3,7 @@ // actually happened. import { test } from 'node:test' import assert from 'node:assert/strict' +import { fileURLToPath } from 'node:url' import { parseVerdict, hasVerdictLine } from '../opencode/plugin/run-record.mjs' import { OUTCOMES, outcomesReported, checkParity, readAgents } from './check-delivery-parity.mjs' @@ -155,8 +156,8 @@ test('the gate reads the body, not the frontmatter — a real file gutted of bod // body occurrence of INCOMPLETE with APPROVE (gutting the not-run path), leave the frontmatter // untouched. Before this gate stripped frontmatter, the description line alone kept the outcome // set fully covered and the mutation went undetected. - const root = readAgents(new URL('../agents', import.meta.url).pathname) - const port = readAgents(new URL('../opencode/agents', import.meta.url).pathname) + const root = readAgents(fileURLToPath(new URL('../agents', import.meta.url))) + const port = readAgents(fileURLToPath(new URL('../opencode/agents', import.meta.url))) const real = port.get('rust-reviewer') const fmEnd = real.indexOf('\n---\n', real.indexOf('---\n') + 4) + 5 const frontmatter = real.slice(0, fmEnd) @@ -174,8 +175,8 @@ test('the gate reads the body, not the frontmatter — a real file gutted of bod test('the repository itself is in parity', () => { // The gate over the real tree, so a divergence landing on main fails here and not only in CI. - const root = readAgents(new URL('../agents', import.meta.url).pathname) - const port = readAgents(new URL('../opencode/agents', import.meta.url).pathname) + const root = readAgents(fileURLToPath(new URL('../agents', import.meta.url))) + const port = readAgents(fileURLToPath(new URL('../opencode/agents', import.meta.url))) assert.ok(root.size > 0, 'agents/ must not read empty — an empty read passes vacuously') assert.deepEqual(checkParity(root, port), []) }) From 4875a37a5eb31aedf7fc12e6d4f2d53e4ae2eb6c Mon Sep 17 00:00:00 2001 From: Nick Date: Sat, 5 Sep 2026 23:11:18 +0300 Subject: [PATCH 5/8] docs: state the frontmatter-masking property, not a count, in check-delivery-parity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This branch recorded an unmeasured claim about how many agent files carry a full outcome set TWICE — first "three of four pairs derived their outcomes from the description alone" (false), then bc36bf1's correction landed a second false claim in its place ("every one of 8 bodies carries the full set, description short one outcome for the 8th") when a third measurement found nine files, two short bodies, and frontmatter short two outcomes on both. Hand-written counts rot and get miscopied — demonstrated twice on this one branch. The fix is not better numbers: it is no numbers. The comment now states the PROPERTY (a description: line can mask a body that lost an outcome, verified against the real opencode/agents/rust-reviewer.md) and a copy-pasteable one-liner to re-measure the current table on demand, rather than a count that goes stale the moment either delivery changes. Also fixes a stale "see the demonstration two paragraphs below" cross-reference to name stripFrontmatter directly, adds the accepted (not implemented) engine-to-table ceiling gap, and adds check-delivery-parity.mjs to AGENTS.md's Reality table. --- AGENTS.md | 1 + lib/check-delivery-parity.mjs | 56 ++++++++++++++++++++++------------- 2 files changed, 37 insertions(+), 20 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c4de567..12ee639 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -110,6 +110,7 @@ Every carrier below was observed by running it on 2026-08-31; none is inferred. | "the manifests are valid" | the manifests as the official validator reads them | `npx --yes @anthropic-ai/claude-code plugin validate . --strict` | agent | | "this skill/agent is well-formed and its `craft:` refs resolve" | the checker's verdict over `skills/`, `agents/`, `workflows/` | `node lib/check-skills.mjs` | agent | | "this workflow script still parses in the sandbox" | the script compiled inside the sandbox wrapper | `node lib/check-workflows.mjs` | agent | +| "the two deliveries of a review agent report the same outcomes" | the set of outcomes each side's body can reach, per the checker's `OUTCOMES` table | `node lib/check-delivery-parity.mjs` | agent | | "the helper logic is correct" | the test run | `node --test 'lib/**/*.test.mjs' 'opencode/**/*.test.mjs'` | agent | | "the code is lint-clean" | ESLint over the linted scope | `npm run lint` — and read the raw exit code, not a wrapper's summary | agent | | "the eval corpus is well-formed" | the checker's verdict | `node lib/check-evals.mjs` | agent | diff --git a/lib/check-delivery-parity.mjs b/lib/check-delivery-parity.mjs index 8eebbce..3fdfbb8 100644 --- a/lib/check-delivery-parity.mjs +++ b/lib/check-delivery-parity.mjs @@ -14,18 +14,17 @@ // A verdict the root agent can reach and its port cannot is the exact failure above: a whole outcome // missing from one delivery, which is meaning, not formatting. // -// What is measured is the BODY, with the leading YAML frontmatter block stripped first. Measured -// directly: every one of the 8 agent bodies carries the full outcome set on its own — the -// frontmatter strip changes nothing about body coverage. What it changes is what the FRONTMATTER -// ALONE would otherwise mask: the `description:` line alone reaches the full outcome set for 7 of -// the 8 files, and is one outcome short (`bad`, missing from `rust-miri`'s description) for the -// 8th — so measuring the unstripped file would have reported "all clean" almost everywhere even if -// a body had lost an outcome entirely, because the description sitting above it already covers for -// it. That is exactly the failure the frontmatter strip exists to prevent: see the demonstration two -// paragraphs below, where all 7 body occurrences of INCOMPLETE in the real -// opencode/agents/rust-reviewer.md are replaced by APPROVE and the file still reads as fully -// covered — by the description, not the body. The two `description:` lines are therefore now -// unmeasured by this gate; whether they agree with each other is a separate, open parity question. +// What is measured is the BODY, with the leading YAML frontmatter block stripped first. This is not +// a stylistic choice — it is load-bearing, because an agent's `description:` line names the outcomes +// the agent reports, so a `description:` sitting above a body that LOST an outcome can still make +// the unstripped file read as fully covered: the frontmatter stands in for the very thing that broke. +// Verified against the real opencode/agents/rust-reviewer.md: replace every body occurrence of +// INCOMPLETE with APPROVE and the unstripped file still reads as fully covered — by the description, +// not the body — while the stripped body correctly shows the loss. See `stripFrontmatter` below for +// how the strip is performed, and re-run the one-liner there to see the current body-vs-frontmatter +// table for every agent file, including whether a given file's own `description:` line covers what +// its body does. The `description:` lines are therefore unmeasured by this gate; whether the two +// descriptions agree with each other is a separate, open parity question. // // The vocabulary is not remembered — a tripwire in check-delivery-parity.test.mjs runs every word // through the parser that actually reads these verdicts and asserts it lands on the outcome claimed @@ -40,6 +39,12 @@ // This gate proves the word is reachable somewhere in the text, not that the body still contains a // working path to report it. Accepted: catching that would need parsing the rubric's control flow, // which this checker does not attempt. +// +// A second, unstated limit: OUTCOMES is a closed four-bucket set. The tripwire in +// check-delivery-parity.test.mjs only proves the TABLE→ENGINE direction — every word claimed here is +// read by the parser as claimed. Nothing proves the ENGINE→TABLE direction: a fifth verdict added to +// `RANK` in opencode/plugin/run-record.mjs would fall outside this gate entirely, with every test +// here still green. Accepted, not implemented. import fs from 'node:fs' import path from 'node:path' import { fileURLToPath } from 'node:url' @@ -65,14 +70,25 @@ export const OUTCOMES = { } // Strips a leading YAML frontmatter block (`---` … `---`) so the gate measures the BODY, not the -// file. Without this, the `description:` line alone can satisfy nearly every outcome — measured -// across all 8 agent files: the description alone reaches the full outcome set in 7 of 8, missing -// only `bad` in `rust-miri`'s. That near-total coverage is what makes the frontmatter dangerous to -// measure: verified, the real opencode/agents/rust-reviewer.md still reads as fully covered even -// with all 7 body occurrences of INCOMPLETE replaced by APPROVE, because the description line above -// it already carries the words on its own — a body that lost an outcome entirely would still pass. -// NOTE: the `description:` lines themselves are now unmeasured by this gate — whether the two -// descriptions agree is its own, separate parity question, not addressed here. +// file. The property this guards against: an agent's `description:` line names the outcomes the +// agent reports, so a `description:` can stand in for a body that lost an outcome — the frontmatter +// keeps covering for a defect the body no longer has a path to. Verified on the real +// opencode/agents/rust-reviewer.md: replace every body occurrence of INCOMPLETE with APPROVE and the +// unstripped file still reads as fully covered, because the description above it already carries the +// words on its own. +// +// Counts here would rot the moment either delivery's agents change, which is exactly how this file +// twice recorded a false claim about how many files were "full" — so this file states no per-file +// counts. To see the CURRENT body-vs-frontmatter table for every agent file, re-run (verified +// working from the repo root): +// +// node -e 'import("./lib/check-delivery-parity.mjs").then(({OUTCOMES,outcomesReported})=>{const fs=require("fs");const setFor=t=>new Set(Object.entries(OUTCOMES).filter(([,ws])=>ws.some(w=>t.includes(w))).map(([o])=>o));const all=Object.keys(OUTCOMES);const fmt=s=>all.every(o=>s.has(o))?"FULL":([...s].sort().join(",")||"none");for (const f of [...fs.readdirSync("agents").filter(x=>x.endsWith(".md")).map(x=>"agents/"+x),...fs.readdirSync("opencode/agents").filter(x=>x.endsWith(".md")).map(x=>"opencode/agents/"+x)]) {const text=fs.readFileSync(f,"utf8");const m=/^---\r?\n[\s\S]*?\r?\n---\r?\n/.exec(text);console.log(f.padEnd(46),"body:",fmt(outcomesReported(text)).padEnd(20),"fm:",fmt(setFor(m?m[0]:"")))}})' +// +// The `description:` lines themselves are unmeasured by this gate; whether the two descriptions +// agree with each other is a separate, open parity question, not addressed here. +// +// One property this measurement does confirm: neither `rust-miri` body nor its frontmatter reports +// `concern` — miri has no Warning path, so that is correct for this agent, not a gap. function stripFrontmatter(text) { const m = /^---\r?\n[\s\S]*?\r?\n---\r?\n/.exec(text) return m ? text.slice(m[0].length) : text From f94df7700753b0303ecd718c84c641699369a69f Mon Sep 17 00:00:00 2001 From: Nick Date: Sat, 5 Sep 2026 23:19:22 +0300 Subject: [PATCH 6/8] docs: date the delivery-parity present-tense claim, name both verdict engines, match fm/body matching Four corrections from a fourth cold review of the delivery-parity gate: fold the "token appears on both sides" clause under the same measurement date as the counts it follows; name both worstVerdict (lib/run-record.mjs) and RANK (opencode/plugin/run-record.mjs) as the parsers this gate's vocabulary answers to, note the tripwire only runs words through the former, and flag Pass as a concrete instance of the ENGINE->TABLE gap; soften the Reality-table claim class to what the checker actually proves; and make the frontmatter column of the diagnostic one-liner use the same whole-word matching as outcomesReported instead of substring matching. --- AGENTS.md | 4 ++-- lib/check-delivery-parity.mjs | 21 +++++++++++++-------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 12ee639..b29a2a1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -110,7 +110,7 @@ Every carrier below was observed by running it on 2026-08-31; none is inferred. | "the manifests are valid" | the manifests as the official validator reads them | `npx --yes @anthropic-ai/claude-code plugin validate . --strict` | agent | | "this skill/agent is well-formed and its `craft:` refs resolve" | the checker's verdict over `skills/`, `agents/`, `workflows/` | `node lib/check-skills.mjs` | agent | | "this workflow script still parses in the sandbox" | the script compiled inside the sandbox wrapper | `node lib/check-workflows.mjs` | agent | -| "the two deliveries of a review agent report the same outcomes" | the set of outcomes each side's body can reach, per the checker's `OUTCOMES` table | `node lib/check-delivery-parity.mjs` | agent | +| "the two deliveries of a review agent name the same outcome words in their bodies" | the set of outcomes each side's body can reach, per the checker's `OUTCOMES` table | `node lib/check-delivery-parity.mjs` | agent | | "the helper logic is correct" | the test run | `node --test 'lib/**/*.test.mjs' 'opencode/**/*.test.mjs'` | agent | | "the code is lint-clean" | ESLint over the linted scope | `npm run lint` — and read the raw exit code, not a wrapper's summary | agent | | "the eval corpus is well-formed" | the checker's verdict | `node lib/check-evals.mjs` | agent | @@ -186,7 +186,7 @@ No formatter. There **is** a typechecker now: `tsc --noEmit --strict` over `open - **`lib/run-record.mjs` and `opencode/plugin/run-record.mjs` are NOT copies** — they are different modules sharing a filename (`lib` carries `countBySeverity`, `reviewVerdict`, the triage and fingerprint helpers; `opencode` carries `parseVerdict`, `buildAuditRecord`, `buildTriageRecord` and writes files directly). So editing "the other copy" edits the wrong file. **But they share exactly one export, `indexProjection`, and its two bodies already disagree** — verified 2026-09-03: the `lib` one emits `craftVersion`, `craftCommit`, `branch`, `head`, `round` and `outputTokens` and defaults `findingsTotal` to `0`; the OpenCode one emits none of the six and defaults to `null`. That is the trap in both directions: a column added to the index in `lib` is not added by the OpenCode writer, and lines from the two deliveries silently carry different shapes into one `index.jsonl`. No gate compares them. - **The genuinely duplicated surfaces are `lib/run-logging.mjs`, `lib/run-record.mjs` and `lib/review-coverage.mjs`**, each inlined verbatim into the workflow engines inside `// >>> craft-inline` fences (`run-logging` and `run-record` into all four). That one IS gated: `node lib/check-workflows.mjs` byte-compares every region against the source and `--fix` regenerates them, so change the source and regenerate — never hand-edit a region. One exception, and it is deliberate: `review.js`'s `shq` sits **outside** the fence (excluded to avoid a duplicate declaration), so the checker is blind to it; a tripwire in `lib/review-coverage.test.mjs` compares it instead. - **The OpenCode delivery has exactly one gate per surface, and no more.** `index.ts`, `orchestrator.ts`, `rust-audit.ts` and `triage-findings.ts` are compiled in CI by `tsc --noEmit --strict` (step "Typecheck opencode plugin") — but they remain outside ESLint (it globs `.js`/`.mjs`/`.cjs`, no TS parser configured) and outside `node --test`, so a type-clean file can still be wrong. `opencode/agents/*.md` and `opencode/commands/*.md` are shape-checked by `opencode/scripts/check-frontmatter.py` (step "Check opencode agent/command frontmatter"), which validates frontmatter shape only — it says nothing about whether a description triggers, and it never reads the body. Those two directories are also **not** in `lib/check-skills.mjs`'s foreign-plugin scan, which covers exactly `README.md`, `MAP.md`, `CLAUDE.md`, `opencode/README.md` and `opencode/install.sh` (plus `skills/`, `agents/`, `workflows/`): a `superpowers:` reference inside an OpenCode agent or command body is read by no checker at all. And `opencode/README.md` and `install.sh` are scanned for that and nothing else. - - **`node lib/check-delivery-parity.mjs` compares the SET OF OUTCOMES each agent's two deliveries (`agents/` and `opencode/agents/`) can report** — `ok`/`concern`/`bad`/`incomplete`, matched against the rubric's own words with the leading YAML frontmatter stripped so it reads the BODY, not the file (the checker's header comment carries the full rationale, including why frontmatter alone would mask a lost outcome). It does **not** compare guidance, rubric detail, or wording — those can diverge with this gate green. `agents/nix-reviewer.md` has no `opencode/agents/` counterpart by design (`UNPAIRED_BY_DESIGN` in the checker: the OpenCode delivery ships no Nix profile). Re-measured 2026-09-05 with `awk '{n+=gsub(/INCOMPLETE/,"")} END{print n+0}'` over each file: `rust-reviewer` 7 (root) vs 8 (opencode), `rust-architecture-reviewer` 7 vs 5, `rust-security-scanner` 6 vs 4, `rust-miri` 6 vs 6 — the token appears on both sides of all four pairs, but a raw count is not what the gate checks (see the checker's comment for why). After editing an agent on either side, still check the other side by hand for meaning — the gate proves the outcome set matches, not that the two bodies say the same thing. + - **`node lib/check-delivery-parity.mjs` compares the SET OF OUTCOMES each agent's two deliveries (`agents/` and `opencode/agents/`) can report** — `ok`/`concern`/`bad`/`incomplete`, matched against the rubric's own words with the leading YAML frontmatter stripped so it reads the BODY, not the file (the checker's header comment carries the full rationale, including why frontmatter alone would mask a lost outcome). It does **not** compare guidance, rubric detail, or wording — those can diverge with this gate green. `agents/nix-reviewer.md` has no `opencode/agents/` counterpart by design (`UNPAIRED_BY_DESIGN` in the checker: the OpenCode delivery ships no Nix profile). Re-measured 2026-09-05 with `awk '{n+=gsub(/INCOMPLETE/,"")} END{print n+0}'` over each file: `rust-reviewer` 7 (root) vs 8 (opencode), `rust-architecture-reviewer` 7 vs 5, `rust-security-scanner` 6 vs 4, `rust-miri` 6 vs 6 — as of that measurement, the token appears on both sides of all four pairs, but a raw count is not what the gate checks (see the checker's comment for why). After editing an agent on either side, still check the other side by hand for meaning — the gate proves the outcome set matches, not that the two bodies say the same thing. - **The two agent frontmatter contracts are genuinely different, hence two checkers.** Root `agents/*.md` (Claude Code) require `name` matching the filename, a pinned `model`, and `tools` as a list. `opencode/agents/*.md` require the opposite on two of those: no `name` (identity comes from the filename) and **no** `model` (an OpenCode sub-agent must inherit the session model), plus `mode: subagent`, `hidden: true`, and `tools` as a *map* with `write`/`edit` false. Widening `lib/check-skills.mjs` to cover both would mean two rule sets behind one entry point; the existing Python script already encodes the OpenCode one. - **`opencode/plugin/` pins its runtime dependencies exactly and commits `package-lock.json`.** `@opencode-ai/plugin` and `@opencode-ai/sdk` were `"*"` — any upstream release could break CI at an arbitrary moment. CI installs them with `npm ci --prefix opencode/plugin`. This closure is deliberately separate from the root `package.json`, which stays devDependencies-only so the shipped plugin installs with nothing to fetch. The toolchain is pinned exactly too (`typescript` `5.9.3`, `@types/node` `25.9.5`, not ranges): `npm ci` would honour the lockfile either way, but under a range an `npm install` here can pull a newer `tsc` whose new checks fail CI for reasons unrelated to the diff. Bumping either is then a visible, deliberate commit. - **Workflow scripts cannot be checked with `node --check`** — they have top-level `export` + `await` + `return`. `lib/check-workflows.mjs` reproduces the sandbox wrapper and compiles each one; when you change the shape of a workflow script, verify the wrapper still accepts it. diff --git a/lib/check-delivery-parity.mjs b/lib/check-delivery-parity.mjs index 3fdfbb8..5ecc02c 100644 --- a/lib/check-delivery-parity.mjs +++ b/lib/check-delivery-parity.mjs @@ -27,9 +27,10 @@ // descriptions agree with each other is a separate, open parity question. // // The vocabulary is not remembered — a tripwire in check-delivery-parity.test.mjs runs every word -// through the parser that actually reads these verdicts and asserts it lands on the outcome claimed -// here. A list held by memory drifts from the engine the moment either moves, which is the defect -// class this file exists inside. +// through one of the parsers that actually reads these verdicts (`worstVerdict` in +// lib/run-record.mjs) and asserts it lands on the outcome claimed here. A list held by memory +// drifts from the engine the moment either moves, which is the defect class this file exists +// inside. // // CEILING, stated plainly: `outcomesReported` matches any occurrence of a rubric word anywhere in // the body, including inside ordinary prose — `outcomesReported('Never Block on style nits; do not @@ -41,10 +42,14 @@ // which this checker does not attempt. // // A second, unstated limit: OUTCOMES is a closed four-bucket set. The tripwire in -// check-delivery-parity.test.mjs only proves the TABLE→ENGINE direction — every word claimed here is -// read by the parser as claimed. Nothing proves the ENGINE→TABLE direction: a fifth verdict added to -// `RANK` in opencode/plugin/run-record.mjs would fall outside this gate entirely, with every test -// here still green. Accepted, not implemented. +// check-delivery-parity.test.mjs only proves the TABLE→ENGINE direction, and only against ONE of the +// two engines that read these verdicts — every word claimed here is confirmed against `worstVerdict` +// in lib/run-record.mjs, but the Claude Code side is also read by a second, independent engine, +// `RANK` in opencode/plugin/run-record.mjs, and the tripwire's words never run through it. Nothing +// proves the ENGINE→TABLE direction either: a fifth verdict added to either engine would fall +// outside this gate entirely, with every test here still green. Concretely, `worstVerdict` also +// matches `Pass` (in `Approve|Healthy|Clean|Pass`), a word this OUTCOMES table does not carry at +// all — one instance of the gap, not a reason to add `Pass` here. Accepted, not implemented. import fs from 'node:fs' import path from 'node:path' import { fileURLToPath } from 'node:url' @@ -82,7 +87,7 @@ export const OUTCOMES = { // counts. To see the CURRENT body-vs-frontmatter table for every agent file, re-run (verified // working from the repo root): // -// node -e 'import("./lib/check-delivery-parity.mjs").then(({OUTCOMES,outcomesReported})=>{const fs=require("fs");const setFor=t=>new Set(Object.entries(OUTCOMES).filter(([,ws])=>ws.some(w=>t.includes(w))).map(([o])=>o));const all=Object.keys(OUTCOMES);const fmt=s=>all.every(o=>s.has(o))?"FULL":([...s].sort().join(",")||"none");for (const f of [...fs.readdirSync("agents").filter(x=>x.endsWith(".md")).map(x=>"agents/"+x),...fs.readdirSync("opencode/agents").filter(x=>x.endsWith(".md")).map(x=>"opencode/agents/"+x)]) {const text=fs.readFileSync(f,"utf8");const m=/^---\r?\n[\s\S]*?\r?\n---\r?\n/.exec(text);console.log(f.padEnd(46),"body:",fmt(outcomesReported(text)).padEnd(20),"fm:",fmt(setFor(m?m[0]:"")))}})' +// node -e 'import("./lib/check-delivery-parity.mjs").then(({OUTCOMES,outcomesReported})=>{const fs=require("fs");const setFor=t=>new Set(Object.entries(OUTCOMES).filter(([,ws])=>ws.some(w=>new RegExp(`(?o));const all=Object.keys(OUTCOMES);const fmt=s=>all.every(o=>s.has(o))?"FULL":([...s].sort().join(",")||"none");for (const f of [...fs.readdirSync("agents").filter(x=>x.endsWith(".md")).map(x=>"agents/"+x),...fs.readdirSync("opencode/agents").filter(x=>x.endsWith(".md")).map(x=>"opencode/agents/"+x)]) {const text=fs.readFileSync(f,"utf8");const m=/^---\r?\n[\s\S]*?\r?\n---\r?\n/.exec(text);console.log(f.padEnd(46),"body:",fmt(outcomesReported(text)).padEnd(20),"fm:",fmt(setFor(m?m[0]:"")))}})' // // The `description:` lines themselves are unmeasured by this gate; whether the two descriptions // agree with each other is a separate, open parity question, not addressed here. From dc2a48c11ba8b2ab033c368dd206f815c387f65e Mon Sep 17 00:00:00 2001 From: Nick Date: Tue, 8 Sep 2026 17:09:45 +0300 Subject: [PATCH 7/8] docs: fix inverted claim about which verdict engine the tripwire exercises MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit f94df77 inverted which reader the delivery-parity tripwire exercises: it claimed the vocabulary was confirmed against `worstVerdict` (lib/run-record.mjs) and named `RANK` (opencode/plugin/run-record.mjs) as the second, unexercised reader. Measured: the tripwire imports `parseVerdict`/`hasVerdictLine`, not `worstVerdict`; `RANK` is a precedence lookup over already-parsed tokens, not a reader of rubric words at all — `parseVerdict` is. So the exercised engine is the OpenCode one, and the unexercised one is `worstVerdict`. This is the third unmeasured claim recorded as fact on this branch, each time inside a commit meant to correct the previous one. Also: the two engines don't just disagree in principle — running `worstVerdict(['INCOMPLETE (not run)'])` returns `'Warning'`, while `parseVerdict` on the same input reports `'INCOMPLETE (not run)'` itself. Named as a second instance alongside the existing `Pass` gap, not reconciled here. AGENTS.md's Reality-table claim class over-corrected in the same commit: the gate compares outcome buckets, not literal words, so "name the same outcome words" was too strong — reworded to "can name the same set of outcomes". --- AGENTS.md | 2 +- lib/check-delivery-parity.mjs | 26 +++++++++++++++----------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index b29a2a1..c39bb51 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -110,7 +110,7 @@ Every carrier below was observed by running it on 2026-08-31; none is inferred. | "the manifests are valid" | the manifests as the official validator reads them | `npx --yes @anthropic-ai/claude-code plugin validate . --strict` | agent | | "this skill/agent is well-formed and its `craft:` refs resolve" | the checker's verdict over `skills/`, `agents/`, `workflows/` | `node lib/check-skills.mjs` | agent | | "this workflow script still parses in the sandbox" | the script compiled inside the sandbox wrapper | `node lib/check-workflows.mjs` | agent | -| "the two deliveries of a review agent name the same outcome words in their bodies" | the set of outcomes each side's body can reach, per the checker's `OUTCOMES` table | `node lib/check-delivery-parity.mjs` | agent | +| "the two deliveries of a review agent can name the same set of outcomes in their bodies" | the set of outcomes each side's body can reach, per the checker's `OUTCOMES` table | `node lib/check-delivery-parity.mjs` | agent | | "the helper logic is correct" | the test run | `node --test 'lib/**/*.test.mjs' 'opencode/**/*.test.mjs'` | agent | | "the code is lint-clean" | ESLint over the linted scope | `npm run lint` — and read the raw exit code, not a wrapper's summary | agent | | "the eval corpus is well-formed" | the checker's verdict | `node lib/check-evals.mjs` | agent | diff --git a/lib/check-delivery-parity.mjs b/lib/check-delivery-parity.mjs index 5ecc02c..b040578 100644 --- a/lib/check-delivery-parity.mjs +++ b/lib/check-delivery-parity.mjs @@ -27,10 +27,10 @@ // descriptions agree with each other is a separate, open parity question. // // The vocabulary is not remembered — a tripwire in check-delivery-parity.test.mjs runs every word -// through one of the parsers that actually reads these verdicts (`worstVerdict` in -// lib/run-record.mjs) and asserts it lands on the outcome claimed here. A list held by memory -// drifts from the engine the moment either moves, which is the defect class this file exists -// inside. +// through the parser that actually reads these verdicts on the OpenCode side (`parseVerdict` / +// `hasVerdictLine` in opencode/plugin/run-record.mjs) and asserts it lands on the outcome claimed +// here. A list held by memory drifts from the engine the moment either moves, which is the defect +// class this file exists inside. // // CEILING, stated plainly: `outcomesReported` matches any occurrence of a rubric word anywhere in // the body, including inside ordinary prose — `outcomesReported('Never Block on style nits; do not @@ -43,13 +43,17 @@ // // A second, unstated limit: OUTCOMES is a closed four-bucket set. The tripwire in // check-delivery-parity.test.mjs only proves the TABLE→ENGINE direction, and only against ONE of the -// two engines that read these verdicts — every word claimed here is confirmed against `worstVerdict` -// in lib/run-record.mjs, but the Claude Code side is also read by a second, independent engine, -// `RANK` in opencode/plugin/run-record.mjs, and the tripwire's words never run through it. Nothing -// proves the ENGINE→TABLE direction either: a fifth verdict added to either engine would fall -// outside this gate entirely, with every test here still green. Concretely, `worstVerdict` also -// matches `Pass` (in `Approve|Healthy|Clean|Pass`), a word this OUTCOMES table does not carry at -// all — one instance of the gap, not a reason to add `Pass` here. Accepted, not implemented. +// two engines that read these verdicts — every word claimed here is confirmed against `parseVerdict` +// in opencode/plugin/run-record.mjs, but the Claude Code side is also read by a second, independent +// engine, `worstVerdict` in lib/run-record.mjs, and the tripwire's words never run through it. That +// second engine is not merely unchecked — it actively DISAGREES with the one this gate exercises: +// `worstVerdict(['INCOMPLETE (not run)'])` returns `'Warning'`, while `parseVerdict` on the OpenCode +// side reports `'INCOMPLETE (not run)'` for the same input. Nothing proves the ENGINE→TABLE +// direction either: a fifth verdict added to either engine would fall outside this gate entirely, +// with every test here still green. Concretely, `worstVerdict` also matches `Pass` (in +// `Approve|Healthy|Clean|Pass`), a word this OUTCOMES table does not carry at all — a second, +// independent instance of the same gap, not a reason to add `Pass` here. Both are accepted, named +// ceilings; reconciling the two engines' vocabularies is a separate decision, not made here. import fs from 'node:fs' import path from 'node:path' import { fileURLToPath } from 'node:url' From 8383a9706ee28e3878fe2f30737b68a422f13faf Mon Sep 17 00:00:00 2001 From: Nick Date: Tue, 8 Sep 2026 22:01:26 +0300 Subject: [PATCH 8/8] docs: attach the delivery-parity tripwire's mechanism to the row it pins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fourth prose claim on this branch that the artifact did not support: the header said "every word claimed here is confirmed against parseVerdict", but the tripwire pins the ok row (Approve/Clean/Healthy) by RECOGNITION through hasVerdictLine, not by the returned token — because parseVerdict's fallthrough is Approve for any input, so asserting the returned token there would be vacuous (parseVerdict('banana') -> 'Approve' too). Only the other three rows (concern/bad/incomplete) are pinned by the direct parseVerdict assertion. Corrected both mentions to name which mechanism pins which row. Also: separated the "Pass" framing from the "second-engine-unexercised" limit it was folded into — Pass instances the ENGINE->TABLE gap two sentences earlier, not the same gap as the unexercised worstVerdict engine, even though both are true. And collapsed the frontmatter-masking rationale, which was stated twice near-verbatim (header and stripFrontmatter comment), down to one full statement in stripFrontmatter with a short pointer left in the header, so it can only go stale in one place. --- lib/check-delivery-parity.mjs | 58 ++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 25 deletions(-) diff --git a/lib/check-delivery-parity.mjs b/lib/check-delivery-parity.mjs index b040578..b96cd6f 100644 --- a/lib/check-delivery-parity.mjs +++ b/lib/check-delivery-parity.mjs @@ -14,23 +14,17 @@ // A verdict the root agent can reach and its port cannot is the exact failure above: a whole outcome // missing from one delivery, which is meaning, not formatting. // -// What is measured is the BODY, with the leading YAML frontmatter block stripped first. This is not -// a stylistic choice — it is load-bearing, because an agent's `description:` line names the outcomes -// the agent reports, so a `description:` sitting above a body that LOST an outcome can still make -// the unstripped file read as fully covered: the frontmatter stands in for the very thing that broke. -// Verified against the real opencode/agents/rust-reviewer.md: replace every body occurrence of -// INCOMPLETE with APPROVE and the unstripped file still reads as fully covered — by the description, -// not the body — while the stripped body correctly shows the loss. See `stripFrontmatter` below for -// how the strip is performed, and re-run the one-liner there to see the current body-vs-frontmatter -// table for every agent file, including whether a given file's own `description:` line covers what -// its body does. The `description:` lines are therefore unmeasured by this gate; whether the two -// descriptions agree with each other is a separate, open parity question. +// What is measured is the BODY, with the leading YAML frontmatter block stripped first — see +// `stripFrontmatter` below for why the strip is load-bearing, not a stylistic choice. The +// `description:` lines are unmeasured by this gate; whether the two descriptions agree with each +// other is a separate, open parity question. // // The vocabulary is not remembered — a tripwire in check-delivery-parity.test.mjs runs every word -// through the parser that actually reads these verdicts on the OpenCode side (`parseVerdict` / -// `hasVerdictLine` in opencode/plugin/run-record.mjs) and asserts it lands on the outcome claimed -// here. A list held by memory drifts from the engine the moment either moves, which is the defect -// class this file exists inside. +// through the parsers that actually read these verdicts on the OpenCode side, `parseVerdict` and +// `hasVerdictLine` in opencode/plugin/run-record.mjs, and asserts it lands on the outcome claimed +// here. (Which of the two pins which row is explained where the tripwire is discussed below.) A list +// held by memory drifts from the engine the moment either moves, which is the defect class this file +// exists inside. // // CEILING, stated plainly: `outcomesReported` matches any occurrence of a rubric word anywhere in // the body, including inside ordinary prose — `outcomesReported('Never Block on style nits; do not @@ -43,17 +37,21 @@ // // A second, unstated limit: OUTCOMES is a closed four-bucket set. The tripwire in // check-delivery-parity.test.mjs only proves the TABLE→ENGINE direction, and only against ONE of the -// two engines that read these verdicts — every word claimed here is confirmed against `parseVerdict` -// in opencode/plugin/run-record.mjs, but the Claude Code side is also read by a second, independent -// engine, `worstVerdict` in lib/run-record.mjs, and the tripwire's words never run through it. That -// second engine is not merely unchecked — it actively DISAGREES with the one this gate exercises: +// two engines that read these verdicts — the `ok` row is confirmed by recognition, through +// `hasVerdictLine`, and the other three rows by the returned token, through `parseVerdict`, both in +// opencode/plugin/run-record.mjs (see `stripFrontmatter` below for why the `ok` row needs the +// different mechanism) — but the Claude Code side is also read by a second, independent engine, +// `worstVerdict` in lib/run-record.mjs, and the tripwire's words never run through it. That second +// engine is not merely unchecked — it actively DISAGREES with the one this gate exercises: // `worstVerdict(['INCOMPLETE (not run)'])` returns `'Warning'`, while `parseVerdict` on the OpenCode -// side reports `'INCOMPLETE (not run)'` for the same input. Nothing proves the ENGINE→TABLE -// direction either: a fifth verdict added to either engine would fall outside this gate entirely, -// with every test here still green. Concretely, `worstVerdict` also matches `Pass` (in -// `Approve|Healthy|Clean|Pass`), a word this OUTCOMES table does not carry at all — a second, -// independent instance of the same gap, not a reason to add `Pass` here. Both are accepted, named -// ceilings; reconciling the two engines' vocabularies is a separate decision, not made here. +// side reports `'INCOMPLETE (not run)'` for the same input — the *second-engine-unexercised* limit. +// Nothing proves the ENGINE→TABLE direction either: a fifth verdict added to either engine would +// fall outside this gate entirely, with every test here still green — the *ENGINE→TABLE* limit. +// Concretely, `worstVerdict` also matches `Pass` (in `Approve|Healthy|Clean|Pass`), a word this +// OUTCOMES table does not carry at all: that is an instance of the ENGINE→TABLE limit, not of the +// second-engine gap above — `Pass` is unproven regardless of which engine is asked. Both limits are +// accepted, named ceilings; reconciling the two engines' vocabularies is a separate decision, not +// made here. import fs from 'node:fs' import path from 'node:path' import { fileURLToPath } from 'node:url' @@ -86,6 +84,16 @@ export const OUTCOMES = { // unstripped file still reads as fully covered, because the description above it already carries the // words on its own. // +// The vocabulary tripwire (check-delivery-parity.test.mjs) confirms OUTCOMES against two different +// mechanisms in opencode/plugin/run-record.mjs, and which row uses which is not interchangeable. +// `parseVerdict`'s fallthrough is `Approve` for ANY input, including an invented word — so asserting +// the RETURNED TOKEN would be vacuous for exactly the `ok` row (`Approve`, `Clean`, `Healthy`), since +// those words already map to the fallthrough value. That row is instead pinned by RECOGNITION, +// through `hasVerdictLine`: each `ok` word, in a realistic `Verdict: X` line, must be recognised, and +// a control non-word in the same shape must not be. The other three rows (`concern`, `bad`, +// `incomplete`) are not the fallthrough, so the direct `parseVerdict` assertion on the returned token +// is meaningful there and is what the tripwire uses. +// // Counts here would rot the moment either delivery's agents change, which is exactly how this file // twice recorded a false claim about how many files were "full" — so this file states no per-file // counts. To see the CURRENT body-vs-frontmatter table for every agent file, re-run (verified