From 29e2a0099b3c28195a9ec20e0ea0d3ff47434744 Mon Sep 17 00:00:00 2001 From: wenshao Date: Tue, 18 Aug 2026 20:44:20 +0800 Subject: [PATCH] refactor(review): define each certification-bar atom exactly once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two-author certification bar — a CLI-built prompt delivered verbatim, plus the agent demonstrably opening its brief, the diff, or the findings list its prompt named — was re-implemented in four places: the coverage walk, the layer-audit gate, retirement, and the resume recovery command. Re-implementing a bar means drifting from it, and each copy had drifted in its own direction. Extract the atoms into `lib/certification.ts` — `chunkOfKey`, `declaresOwnUncoverable`, `openedBrief`, `readBrief`, `readFindingsPointer` — and have coverage, layer-audit-gate, retirement and `recover-findings` compose the same functions. No behavior change: the atoms are the exact predicates the live pipeline already ran, now spelled once so a future edit to the bar reaches every consumer. --- .../commands/review/check-coverage.test.ts | 113 ++++++++++- .../commands/review/lib/certification.test.ts | 176 ++++++++++++++++++ .../src/commands/review/lib/certification.ts | 109 +++++++++++ .../cli/src/commands/review/lib/coverage.ts | 78 +++----- .../commands/review/lib/layer-audit-gate.ts | 17 +- .../cli/src/commands/review/lib/retirement.ts | 26 +-- .../src/commands/review/recover-findings.ts | 60 ++---- 7 files changed, 447 insertions(+), 132 deletions(-) create mode 100644 packages/cli/src/commands/review/lib/certification.test.ts create mode 100644 packages/cli/src/commands/review/lib/certification.ts diff --git a/packages/cli/src/commands/review/check-coverage.test.ts b/packages/cli/src/commands/review/check-coverage.test.ts index 649fd5dbf1b..1c5f48fd2fe 100644 --- a/packages/cli/src/commands/review/check-coverage.test.ts +++ b/packages/cli/src/commands/review/check-coverage.test.ts @@ -797,6 +797,71 @@ describe('budget-gap disclosures — guarded, parsed, never punished', () => { ]); }); + it('a whole-diff disclosure is silenced only by a compliant gap-free relaunch', () => { + // `gapsSuperseded`'s whole-diff branch: the superseding record must have + // OPENED the key's brief and be gap-free itself. Neither conjunct was + // reached by any test — a revert of the branch shipped green. + const p = plan3a(); + const d = promptRecordDir(p); + mkdirSync(d, { recursive: true }); + const brief = briefPath(p, 'audit-w'); + writeFileSync(brief, 'The audit-w brief.'); + const prompt = + 'You are review agent `audit-w`.\n' + + `read_file(file_path="${brief}")\n` + + wholeDiff(); + writeFileSync(join(d, 'audit-w.txt'), prompt); + transcript('g1', prompt, { + calls: 3, + text: 'Walked the diff.\nBudget gap: the reconnect state machine', + }); + // A gap-free relaunch that opened the brief silences the disclosure. + transcript('g2', prompt, { calls: 3 }); + expect(coverageFromTranscripts(p, ENV).budgetGaps).toEqual([]); + }); + + it('a relaunch that never opened the brief cannot silence the disclosure', () => { + const p = plan3a(); + const d = promptRecordDir(p); + mkdirSync(d, { recursive: true }); + const brief = briefPath(p, 'audit-w'); + writeFileSync(brief, 'The audit-w brief.'); + const prompt = + 'You are review agent `audit-w`.\n' + + `read_file(file_path="${brief}")\n` + + wholeDiff(); + writeFileSync(join(d, 'audit-w.txt'), prompt); + transcript('g1', prompt, { + calls: 3, + text: 'Walked the diff.\nBudget gap: the reconnect state machine', + }); + transcript('g2', prompt, { calls: 3, opens: [] }); + expect(coverageFromTranscripts(p, ENV).budgetGaps).toHaveLength(1); + }); + + it('a relaunch still disclosing gaps of its own cannot silence anything', () => { + const p = plan3a(); + const d = promptRecordDir(p); + mkdirSync(d, { recursive: true }); + const brief = briefPath(p, 'audit-w'); + writeFileSync(brief, 'The audit-w brief.'); + const prompt = + 'You are review agent `audit-w`.\n' + + `read_file(file_path="${brief}")\n` + + wholeDiff(); + writeFileSync(join(d, 'audit-w.txt'), prompt); + transcript('g1', prompt, { + calls: 3, + text: 'Walked the diff.\nBudget gap: the reconnect state machine', + }); + transcript('g2', prompt, { + calls: 3, + text: 'Walked again.\nBudget gap: the remaining call sites', + }); + // Two live disclosures, neither silenced by the other. + expect(coverageFromTranscripts(p, ENV).budgetGaps).toHaveLength(2); + }); + it('a disclosure costs no coverage credit — the gate must not punish it', () => { // An earlier draft narrowed a disclosing agent's credit to its ranged // reads. `rangeOf` records only reads carrying a positive `limit`, so @@ -2291,6 +2356,7 @@ describe('coverage — a resumed run credits the prior attempt through the ledge // The run ledger `fetch-pr` writes: S0 is the interrupted attempt, S1 the // resumed continuation this suite's ENV runs as. Entries carry a current // atMs, which sits inside the epoch fence of the backdated plan. + let ledgerNowMs = 0; function ledger(planPath: string, ...ids: string[]): void { const d = promptRecordDir(planPath); mkdirSync(d, { recursive: true }); @@ -2299,6 +2365,7 @@ describe('coverage — a resumed run credits the prior attempt through the ledge // evidence at all. The current attempt is stamped last, since each // attempt's window closes when the next one opened. const nowMs = Date.now(); + ledgerNowMs = nowMs; ids.forEach((id, i) => appendRunSession( planPath, @@ -2325,6 +2392,10 @@ describe('coverage — a resumed run credits the prior attempt through the ledge ), ); rmSync(from, { force: true }); + if (ledgerNowMs > 0) { + const at = new Date(ledgerNowMs); + utimesSync(to, at, at); + } } it('passes 3D on work the interrupted attempt completed, and discloses it', () => { @@ -2391,19 +2462,26 @@ describe('verificationGaps — a resumed run reads the prior attempt', () => { function moveToSession(id: string, session: string): void { mkdirSync(join(dir, 'subagents', session), { recursive: true }); const from = join(dir, 'subagents', 'S1', `agent-${id}.jsonl`); + const to = join(dir, 'subagents', session, `agent-${id}.jsonl`); writeFileSync( - join(dir, 'subagents', session, `agent-${id}.jsonl`), + to, readFileSync(from, 'utf8').replaceAll( '"sessionId":"S1"', `"sessionId":"${session}"`, ), ); rmSync(from, { force: true }); + if (ledgerNowMs > 0) { + const at = new Date(ledgerNowMs); + utimesSync(to, at, at); + } } /** The ledger `fetch-pr` writes, through the real writers. */ + let ledgerNowMs = 0; function ledger(planPath: string, ...ids: string[]): void { const nowMs = Date.now(); + ledgerNowMs = nowMs; ids.forEach((id, i) => appendRunSession( planPath, @@ -2518,6 +2596,7 @@ describe('verificationGaps — a resumed run reads the prior attempt', () => { }); describe('coverage — a stale Uncoverable declaration cannot cap live coverage', () => { + let ledgerNowMs = 0; function ledger(planPath: string, ...ids: string[]): void { const d = promptRecordDir(planPath); mkdirSync(d, { recursive: true }); @@ -2526,6 +2605,7 @@ describe('coverage — a stale Uncoverable declaration cannot cap live coverage' // evidence at all. The current attempt is stamped last, since each // attempt's window closes when the next one opened. const nowMs = Date.now(); + ledgerNowMs = nowMs; ids.forEach((id, i) => appendRunSession( planPath, @@ -2551,6 +2631,10 @@ describe('coverage — a stale Uncoverable declaration cannot cap live coverage' ), ); rmSync(from, { force: true }); + if (ledgerNowMs > 0) { + const at = new Date(ledgerNowMs); + utimesSync(to, at, at); + } } it('a superseded prior-attempt declaration does not delete the chunk it covers', () => { @@ -2630,6 +2714,33 @@ describe('coverage — a stale Uncoverable declaration cannot cap live coverage' expect(r.recoveredAgents).toBe(0); }); + it('a whole-diff recovery is superseded only by a relaunch that opened the brief', () => { + // `keySatisfied` — the chunk-less arm of the supersession predicates — + // was reached by no test: its brief requirement could be deleted (or + // left dangling) with the suite green. The deciding conjunct is the + // relaunch's brief read, so both arms pin it. + const p = plan(); + ledger(p, 'S0', 'S1'); + const d = promptRecordDir(p); + mkdirSync(d, { recursive: true }); + const brief = briefPath(p, 'audit-w'); + writeFileSync(brief, 'The audit-w brief.'); + const prompt = + 'You are review agent `audit-w`.\n' + + `read_file(file_path="${brief}")\n` + + wholeDiff(); + writeFileSync(join(d, 'audit-w.txt'), prompt); + transcript('w1', prompt, { calls: 3 }); + moveToSession('w1', 'S0'); + // The current relaunch never opened its brief: no supersession, the + // prior work still counts as recovered. + transcript('w2', prompt, { calls: 3, opens: [] }); + expect(coverageFromTranscripts(p, ENV).recoveredAgents).toBe(1); + // A compliant relaunch supersedes it. + transcript('w3', prompt, { calls: 3 }); + expect(coverageFromTranscripts(p, ENV).recoveredAgents).toBe(0); + }); + it('does NOT credit a prior agent whose text is progress, not a return', () => { // `finalText` keeps the last non-empty assistant text, and agents narrate // between tool calls — so an agent that said "reading the diff now" and diff --git a/packages/cli/src/commands/review/lib/certification.test.ts b/packages/cli/src/commands/review/lib/certification.test.ts new file mode 100644 index 00000000000..dbad83254c3 --- /dev/null +++ b/packages/cli/src/commands/review/lib/certification.test.ts @@ -0,0 +1,176 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it } from 'vitest'; +import { + chunkOfKey, + declaresOwnUncoverable, + openedBrief, + readBrief, + readFindingsPointer, +} from './certification.js'; +import { briefPath } from './prompt-record.js'; +import type { AgentRecord } from './transcripts.js'; + +const PLAN = '/tmp/certification-test/plan.json'; + +function rec(over: Partial): AgentRecord { + return { + agentId: 'a1', + agentName: 'agent-a1', + launchPrompt: '', + successfulToolCalls: 0, + diffToolCalls: 0, + diffReads: [], + successfulCallArgs: [], + successfulReadFileArgs: [], + recordedSession: 's1', + finalText: '', + returned: true, + mtimeMs: 0, + ...over, + }; +} + +describe('chunkOfKey', () => { + it('parses the bare chunk key', () => { + expect(chunkOfKey('chunk-13')).toBe(13); + }); + + it('parses the per-chunk audit key from its --chunk-N segment', () => { + expect(chunkOfKey('reverse-audit--chunk-13--round-2--0a1b2c')).toBe(13); + }); + + it('parses a trailing --chunk-N segment', () => { + expect(chunkOfKey('reverse-audit--chunk-7')).toBe(7); + }); + + it('assigns no chunk to chunk-free keys', () => { + expect(chunkOfKey('verify--0a1b2c')).toBeNull(); + expect(chunkOfKey('reverse-audit--round-2--0a1b2c')).toBeNull(); + }); + + it('does not mistake a digest containing "chunk" for an assignment', () => { + // Only the exact `--chunk-` segment form assigns; an embedded + // word does not. + expect(chunkOfKey('verify--chunky')).toBeNull(); + }); +}); + +describe('declaresOwnUncoverable', () => { + const declaring = 'Uncoverable: chunk 3 — binary payload, no readable lines'; + + it('vetoes the record whose own return declares its own chunk', () => { + expect(declaresOwnUncoverable(rec({ finalText: declaring }), 3)).toBe(true); + }); + + it('does not veto a declaration about a DIFFERENT chunk', () => { + expect(declaresOwnUncoverable(rec({ finalText: declaring }), 4)).toBe( + false, + ); + }); + + it('never vetoes a chunk-less record — a quotation is not a declaration', () => { + // The whole-diff auditor quoting the evidence it audited. + expect(declaresOwnUncoverable(rec({ finalText: declaring }), null)).toBe( + false, + ); + }); + + it('vetoes a declaration preceded by prose on an earlier line (pins /m)', () => { + expect( + declaresOwnUncoverable( + rec({ finalText: `Read what I could.\n${declaring}` }), + 3, + ), + ).toBe(true); + }); + + it('matches a lowercase marker (pins /i)', () => { + expect( + declaresOwnUncoverable( + rec({ finalText: 'uncoverable: chunk 3 — binary payload' }), + 3, + ), + ).toBe(true); + }); + + it('ignores a declaration indented into prose', () => { + // The `^` anchor with only whitespace allowed before the marker: a + // quoted line inside a bullet does not match. + const quoted = `- the agent said "${declaring}" which I verified`; + expect(declaresOwnUncoverable(rec({ finalText: quoted }), 3)).toBe(false); + }); +}); + +describe('openedBrief / readBrief', () => { + const key = 'chunk-2'; + const needle = JSON.stringify(briefPath(PLAN, key)); + const arg = `{"absolute_path":${needle}}`; + + it('credits any successful tool whose args name the exact brief path', () => { + const r = rec({ successfulCallArgs: [arg] }); + expect(openedBrief(r, PLAN, key)).toBe(true); + }); + + it('does not credit a record that made zero successful tool calls', () => { + // `[].every(...)` is true: the existential quantifier needs its own + // negative, like its two sibling atoms already have. + expect(openedBrief(rec({}), PLAN, key)).toBe(false); + }); + + it('does not credit a sibling file sharing the prefix', () => { + const bak = JSON.stringify(`${briefPath(PLAN, key)}.bak`); + const r = rec({ successfulCallArgs: [`{"absolute_path":${bak}}`] }); + expect(openedBrief(r, PLAN, key)).toBe(false); + }); + + it('does not credit another key’s brief', () => { + const r = rec({ successfulCallArgs: [arg] }); + expect(openedBrief(r, PLAN, 'chunk-3')).toBe(false); + }); + + it('readBrief does not credit a sibling file sharing the prefix', () => { + const bak = JSON.stringify(`${briefPath(PLAN, key)}.bak`); + const r = rec({ successfulReadFileArgs: [`{"absolute_path":${bak}}`] }); + expect(readBrief(r, PLAN, key)).toBe(false); + }); + + it('readBrief requires a successful read_file, not a mention', () => { + // A grep whose args contain the path opened nothing. + const r = rec({ successfulCallArgs: [arg], successfulReadFileArgs: [] }); + expect(openedBrief(r, PLAN, key)).toBe(true); + expect(readBrief(r, PLAN, key)).toBe(false); + const read = rec({ successfulReadFileArgs: [arg] }); + expect(readBrief(read, PLAN, key)).toBe(true); + }); +}); + +describe('readFindingsPointer', () => { + const pointer = + '/tmp/certification-test/plan.json-prompts/round-2.findings.md'; + const arg = `{"absolute_path":${JSON.stringify(pointer)}}`; + + it('owes nothing when the prompt names no pointer', () => { + expect(readFindingsPointer(rec({}), null)).toBe(true); + }); + + it('does not credit a sibling of the pointer', () => { + const bak = JSON.stringify(`${pointer}.bak`); + const r = rec({ successfulReadFileArgs: [`{"absolute_path":${bak}}`] }); + expect(readFindingsPointer(r, pointer)).toBe(false); + }); + + it('requires a successful read_file of the exact pointer', () => { + expect( + readFindingsPointer(rec({ successfulReadFileArgs: [arg] }), pointer), + ).toBe(true); + // Named by a listing, read by nothing. + expect( + readFindingsPointer(rec({ successfulCallArgs: [arg] }), pointer), + ).toBe(false); + }); +}); diff --git a/packages/cli/src/commands/review/lib/certification.ts b/packages/cli/src/commands/review/lib/certification.ts new file mode 100644 index 00000000000..e85ad8ac124 --- /dev/null +++ b/packages/cli/src/commands/review/lib/certification.ts @@ -0,0 +1,109 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * The atoms of the certification bar, defined once. + * + * Every consumer that decides whether a transcript's agent did the work it + * was launched for — the coverage walk, its supersession predicates, the + * recovered-work counter, the layer-audit gate, retirement's pairing, and + * `recover-findings`' recovery bar — asks some conjunction of the same five + * questions: did it return, did it disown its own territory, did it open its + * brief, did it read the findings list its prompt names, which chunk was it + * assigned. Five review rounds found five places where a re-implemented copy + * of one of these atoms drifted from the original; this module exists so + * there is nothing left to drift. Compositions still differ per consumer — + * that is their job — and the `returned` question stays a plain field read + * (`rec.returned`) at each site rather than an atom here; every OTHER atom + * has exactly one definition below. + */ + +import type { AgentRecord } from './transcripts.js'; +import { briefPath } from './prompt-record.js'; + +/** + * An agent's own declaration that its chunk cannot be reviewed. Anchored to a + * line start so a quotation indented into prose does not match; the chunk + * number is captured so the veto can stay scoped to the declarer's own + * territory. + */ +const UNCOVERABLE_RE = /^\s*Uncoverable:\s*chunk\s+(\d+)\b/im; + +/** + * The chunk a KEY assigns: `chunk-13` → 13, and the per-chunk audit shapes — + * `reverse-audit--chunk-13--round-2--` — carry theirs in a + * `--chunk-N` segment. Parsing only the bare form left those keys + * chunk-less, and with the production launch prompt carrying no + * `chunk N of M` line either, the bar's diff-read requirement silently + * vanished for exactly the auditors whose territory is a chunk. + */ +export function chunkOfKey(key: string): number | null { + const m = /^chunk-(\d+)$/.exec(key) ?? /--chunk-(\d+)(?:--|$)/.exec(key); + return m ? Number(m[1]) : null; +} + +/** + * Does this record's own return declare ITS OWN chunk unreachable? The veto + * is chunk-SCOPED: applied raw, the regex also matches a QUOTATION, and + * quoting the evidence verbatim is exactly what a reverse-audit brief + * instructs — a whole-diff record quoting a declaration is not declaring. + */ +export function declaresOwnUncoverable( + rec: AgentRecord, + chunk: number | null, +): boolean { + if (chunk === null) return false; + const m = UNCOVERABLE_RE.exec(rec.finalText); + return m !== null && Number(m[1]) === chunk; +} + +/** + * Did this record's agent open the brief recorded under `key`? Compared as a + * whole JSON string value (`successfulCallArgs` are serialized args), so a + * `${brief}.bak` cannot be credited for the brief — the same trap + * `parseTranscript` avoids for the diff path. "Open" is mention-level: any + * successful tool whose args name the exact path. + */ +export function openedBrief( + rec: AgentRecord, + planPath: string, + key: string, +): boolean { + const needle = JSON.stringify(briefPath(planPath, key)); + return rec.successfulCallArgs.some((a) => a.includes(needle)); +} + +/** + * Did this record's agent READ the brief recorded under `key`? Stricter than + * `openedBrief`: only a successful `read_file` of the exact path counts — a + * grep or listing whose args merely CONTAIN the brief path named it without + * opening a line of it. The layer-audit gate's bar. + */ +export function readBrief( + rec: AgentRecord, + planPath: string, + key: string, +): boolean { + const needle = JSON.stringify(briefPath(planPath, key)); + return rec.successfulReadFileArgs.some((a) => a.includes(needle)); +} + +/** + * Did this record's agent successfully `read_file` the findings pointer its + * prompt names? True when the prompt names none. Successful read_file calls + * ONLY: every tool serializes its args, and a `search_file_content` or a + * `list_directory` over the record dir names the path without reading a line + * of it — the floor certifies that the list was OPENED, and a mention is not + * an open. + */ +export function readFindingsPointer( + rec: AgentRecord, + pointer: string | null, +): boolean { + if (pointer === null) return true; + const needle = JSON.stringify(pointer); + return rec.successfulReadFileArgs.some((a) => a.includes(needle)); +} diff --git a/packages/cli/src/commands/review/lib/coverage.ts b/packages/cli/src/commands/review/lib/coverage.ts index d68c273710f..30c4b38aae6 100644 --- a/packages/cli/src/commands/review/lib/coverage.ts +++ b/packages/cli/src/commands/review/lib/coverage.ts @@ -67,6 +67,11 @@ import { findingsFilePath, recordedPromptPath, } from './prompt-record.js'; +import { + declaresOwnUncoverable, + openedBrief, + readFindingsPointer, +} from './certification.js'; import { requiredAgents, type RequiredAgent, @@ -348,8 +353,6 @@ function merge(ranges: Array<[number, number]>): Array<[number, number]> { return out; } -const UNCOVERABLE_RE = /^\s*Uncoverable:\s*chunk\s+(\d+)\b/im; - /** The exact rebuild flags for one required agent — operator-facing (stderr). */ function selectorOf(req: RequiredAgent): string { if (req.role === 'chunk') return `--chunk ${req.chunk}`; @@ -445,16 +448,10 @@ export function coverageFromTranscripts( const unopenedAgents: string[] = []; const rewrittenPrompts: string[] = []; const driftedLaunches: string[] = []; - // Did this record's agent open the brief recorded under `key`? Compared as a - // whole JSON string value (`successfulCallArgs` are serialized args), so a - // `${brief}.bak` cannot be credited for the brief — the same trap - // `parseTranscript` avoids for the diff path. Used by the verbatim-drift - // rescue in both the chunk loop and the roster walk, and by the roster's - // matching seed below. - const openedBriefOf = (rec: AgentRecord, key: string): boolean => { - const needle = JSON.stringify(briefPath(planPath, key)); - return rec.successfulCallArgs.some((a) => a.includes(needle)); - }; + // Used by the verbatim-drift rescue in both the chunk loop and the roster + // walk, and by the roster's matching seed below. + const openedBriefOf = (rec: AgentRecord, key: string): boolean => + openedBrief(rec, planPath, key); const disclosures: CoverageFromTranscripts['disclosures'] = []; // The one source for both registers: the structural entry feeds the posted // body (compose-review), and the returned prose feeds the stderr arrays — @@ -552,7 +549,6 @@ export function coverageFromTranscripts( const b = builtOf(key); if (b === undefined) continue; if (!wasDeliveredVerbatim(rec.launchPrompt, b)) continue; - const needle = JSON.stringify(briefPath(planPath, key)); if ( records.some( (r) => @@ -561,7 +557,7 @@ export function coverageFromTranscripts( // Same return requirement as the chunk branch above. r.returned && wasDeliveredVerbatim(r.launchPrompt, b) && - r.successfulCallArgs.some((a) => a.includes(needle)), + openedBrief(r, planPath, key), ) ) { return true; @@ -629,14 +625,13 @@ export function coverageFromTranscripts( const b = builtOf(key); if (b === undefined) continue; if (!wasDeliveredVerbatim(rec.launchPrompt, b)) continue; - const needle = JSON.stringify(briefPath(planPath, key)); if ( records.some( (r) => r !== rec && r.returned && wasDeliveredVerbatim(r.launchPrompt, b) && - r.successfulCallArgs.some((a) => a.includes(needle)) && + openedBrief(r, planPath, key) && gapsOf(r).length === 0, ) ) { @@ -790,8 +785,7 @@ export function coverageFromTranscripts( const ranges = merge([...told, ...rec.diffReads]); if (ranges.length === 0) continue; - const u = UNCOVERABLE_RE.exec(rec.finalText); - if (u && chunk !== null && Number(u[1]) === chunk) { + if (chunk !== null && declaresOwnUncoverable(rec, chunk)) { // The same supersession guard the sibling flags carry. Without it a // stale declaration — a prior attempt's agent on a resumed run, or a // relaunched agent's first try — permanently deletes live coverage @@ -805,11 +799,9 @@ export function coverageFromTranscripts( // other — the chunk lands in `missingChunks`, whose remediation // relaunches an agent that re-declares, forever. `gapsSuperseded` // below excludes same-shape records for exactly this reason. - const redeclares = (r: AgentRecord): boolean => { - const ru = UNCOVERABLE_RE.exec(r.finalText); - return ru !== null && Number(ru[1]) === chunk; - }; - if (!chunkSatisfied(chunk, rec, (r) => !redeclares(r))) { + if ( + !chunkSatisfied(chunk, rec, (r) => !declaresOwnUncoverable(r, chunk)) + ) { uncoverable.add(chunk); } continue; @@ -1035,19 +1027,13 @@ export function coverageFromTranscripts( // Every role, territory agents included. Their brief is where the severity // definitions, the paging rule, the uncoverable rule and the project rules live. const brief = briefPath(planPath, req.key); - // The brief as a whole JSON string value (`successfulCallArgs` are already - // serialized args): a bare substring would credit `${brief}.bak` for the brief, - // the same trap `parseTranscript` avoids for the diff path. // The ASSIGNED transcript must have opened this requirement's brief. The // matching SEEDS on brief-opening edges, but maximizing satisfied // requirements can displace an opened match onto an unopened edge — so an // unread flag here describes this assignment, not an impossibility. That is // the right trade: missing-role claims stay provable, and an unread brief // still caps. - const opened = pick.successfulCallArgs.some((a) => - a.includes(JSON.stringify(brief)), - ); - if (!opened) { + if (!openedBrief(pick, planPath, req.key)) { // The brief PATH is the operator's — it names the file to make the agent // open. The author's copy drops it: a filesystem path in a posted PR // body is the same register leak as a chunk id. @@ -1088,18 +1074,11 @@ export function coverageFromTranscripts( // A record whose own return declares ITS OWN chunk unreachable did not // review it; counting it as recovered would have the body announce work // "counted as reviewed" beside the gap that same record disclosed. The - // veto is chunk-scoped like the walk's: applied raw it also matches a - // QUOTATION, and a recovered whole-diff auditor legitimately quotes the - // declarations it audited. - const declaredUnc = UNCOVERABLE_RE.exec(r.finalText); - if (declaredUnc !== null) { - const own = assignedChunk(r); - if (own !== null && Number(declaredUnc[1]) === own) return false; - if (own === null && r.diffToolCalls > 0 && assignedChunk(r) === null) { - // A whole-diff record quoting a declaration is not declaring. - } - } + // veto is chunk-scoped like the walk's: a recovered whole-diff auditor + // legitimately quotes the declarations it audited, and a quotation is + // not a declaration. const c = assignedChunk(r); + if (declaresOwnUncoverable(r, c)) return false; if (c !== null) { const b = builtOf(`chunk-${c}`); return ( @@ -1592,24 +1571,15 @@ export function verificationGaps( // replaced: the floor proves the findings file was OPENED (one successful // read_file of the path — no other tool's args count), not that it was // paged to completion — `read_file` truncates, so a first-page-only read - // still leaves a matching `fNeedle`. + // still leaves a matching serialized pointer (the needle built inside `readFindingsPointer`). // The old `wasDeliveredVerbatim` required the whole list in the delivered // prompt; the pointer proves delivery of the pointer line, not receipt of // the whole list. Accepted: the brief now orders the full read, and a // verifier that under-reads surfaces in the verdicts it gets wrong. - const needle = JSON.stringify(briefPath(planPath, key)); - const opened = (r: AgentRecord) => - r.successfulCallArgs.some((a) => a.includes(needle)); + const opened = (r: AgentRecord) => openedBrief(r, planPath, key); const findingsPointer = findingsPointerOf(b); - const readTheFindings = (r: AgentRecord) => { - if (findingsPointer === null) return true; - const fNeedle = JSON.stringify(findingsPointer); - // Successful read_file calls ONLY: every tool serializes its args, and - // a `search_file_content` or a `list_directory` over the record dir - // names the path without reading a line of it. The floor certifies - // that the list was OPENED, and a mention is not an open. - return r.successfulReadFileArgs.some((a) => a.includes(fNeedle)); - }; + const readTheFindings = (r: AgentRecord) => + readFindingsPointer(r, findingsPointer); const gotTheBuiltPrompt = records.filter((r) => wasDeliveredVerbatim(r.launchPrompt, b), ); diff --git a/packages/cli/src/commands/review/lib/layer-audit-gate.ts b/packages/cli/src/commands/review/lib/layer-audit-gate.ts index 740f845cdce..c56a6bd0f36 100644 --- a/packages/cli/src/commands/review/lib/layer-audit-gate.ts +++ b/packages/cli/src/commands/review/lib/layer-audit-gate.ts @@ -57,11 +57,8 @@ import { statSync, readFileSync } from 'node:fs'; import { readRunTranscripts } from './transcripts.js'; import { bakedRanges, openedTheTerritory } from './retirement.js'; -import { - readRecordedPrompts, - wasDeliveredVerbatim, - briefPath, -} from './prompt-record.js'; +import { readRecordedPrompts, wasDeliveredVerbatim } from './prompt-record.js'; +import { readBrief } from './certification.js'; import { repositoryContextOf, type RepositoryContext, @@ -141,13 +138,9 @@ function readReverseAuditReturns( matched = key; } if (matched === null) return false; - const needle = JSON.stringify(briefPath(planPath, matched)); - // READ, not named: `successfulCallArgs` covers every successful - // tool, so a grep or listing whose args merely CONTAIN the brief - // path cleared this — an auditor that never opened its instructions - // supplied a receipt. Only a successful `read_file` of the exact - // brief is opening it. - return t.successfulReadFileArgs.some((a) => a.includes(needle)); + // READ, not named — the `readBrief` atom: only a successful + // `read_file` of the exact brief is opening it. + return readBrief(t, planPath, matched); }; const corroborated = auditors .filter( diff --git a/packages/cli/src/commands/review/lib/retirement.ts b/packages/cli/src/commands/review/lib/retirement.ts index 2500ef95c6e..c858c28652f 100644 --- a/packages/cli/src/commands/review/lib/retirement.ts +++ b/packages/cli/src/commands/review/lib/retirement.ts @@ -44,6 +44,7 @@ import { readFileSync, statSync } from 'node:fs'; import { resolve, sep } from 'node:path'; import { readRunTranscripts, type AgentRecord } from './transcripts.js'; import { REVERSE_AUDIT_EXAMPLE_RECEIPT } from './agent-briefs.js'; +import { readFindingsPointer } from './certification.js'; import { INLINE_LAYER_WALKED_RE, LAYER_RECEIPT_LINE_RE, @@ -508,24 +509,6 @@ function stripLayerReceiptLines(finalText: string): string { return kept.join('\n'); } -/** - * Did this transcript's agent successfully `read_file` the findings pointer - * its record's prompt names? True when the prompt names none. - * - * Takes the POINTER, extracted once from the RAW prompt by the same call - * `findingsListFor` uses: extracting again from trim-normalized lines asked - * the same question under a different normalization, and trimming defeats - * the `^…$` anchors that exist to reject indented quotations. - */ -function readTheFindingsPointer( - rec: AgentRecord, - pointer: string | null, -): boolean { - if (pointer === null) return true; - const needle = JSON.stringify(pointer); - return rec.successfulReadFileArgs.some((a) => a.includes(needle)); -} - /** * Classify one auditor's return. * @@ -847,12 +830,15 @@ export function scheduleReverseAuditRound( // the dry branch there: applied out here as a filter it also // suppressed filed YIELDS, flipping a round to dry and retiring a // chunk that had a live finding. The POINTER was extracted once from - // the RAW prompt by the same call `findingsListFor` uses. + // the RAW prompt by the same call `findingsListFor` uses: extracting + // again from trim-normalized lines asks the same question under a + // different normalization, and trimming defeats the `^…$` anchors + // that reject indented quotations. classifyReturn( t, records[i].territory, records[i].findings, - readTheFindingsPointer(t, records[i].pointer), + readFindingsPointer(t, records[i].pointer), ), ); classificationsByRecord.push(classifications); diff --git a/packages/cli/src/commands/review/recover-findings.ts b/packages/cli/src/commands/review/recover-findings.ts index 9e231e55d8b..0cc016443fa 100644 --- a/packages/cli/src/commands/review/recover-findings.ts +++ b/packages/cli/src/commands/review/recover-findings.ts @@ -42,11 +42,16 @@ import { deliveredVerbatimLines, flattenPrompt, promptLines, - briefPath, findingsPointerOf, } from './lib/prompt-record.js'; import { priorSessionIds } from './lib/run-ledger.js'; import { assignedChunk, pointedAt } from './lib/coverage.js'; +import { + chunkOfKey, + declaresOwnUncoverable, + openedBrief, + readFindingsPointer, +} from './lib/certification.js'; import { readBudgetStop, type BudgetStop } from './lib/deadline.js'; interface RecoverFindingsArgs { @@ -96,40 +101,18 @@ export interface RecoverFindingsResult { const ROUND_IN_KEY_RE = /--round-(\d+)(?:--|$)/; -/** - * Certify one transcript against one built prompt — the same bar coverage - * holds a live launch to: the CLI-built prompt arrived verbatim, and the - * agent demonstrably opened its brief or the diff. Prose proves nothing. - */ -const UNCOVERABLE_RE = /^\s*Uncoverable:\s*chunk\s+(\d+)\b/im; - /** * Certify one transcript against one built prompt — the SAME bar the live * pipeline holds a launch to, branch for branch. * * It used to be a re-implementation, and re-implementing a bar means drifting - * from it. `openedBrief || diffToolCalls > 0` certified three things the - * pipeline refuses: a chunk agent that opened its brief and never the diff - * (coverage requires the diff read for a chunk-assigned record), a - * verify/reverse-audit agent that opened the diff and never its brief (the - * brief carries the method and the cumulative findings list), and a verifier - * that skipped the findings-list read the compose-time gate requires of the - * same key. Each handed the resumed orchestrator uncertified prose labelled - * as certified. - */ -/** - * The chunk a KEY assigns: `chunk-13` → 13, and the per-chunk audit shapes — - * `reverse-audit--chunk-13--round-2--` — carry theirs in a `--chunk-N` - * segment. Parsing only the bare form left those keys chunk-less, and with - * the production launch prompt carrying no `chunk N of M` line either, the - * bar's diff-read requirement silently vanished for exactly the auditors - * whose territory is a chunk. + * from it: five review rounds found five branches where the copy was weaker + * than the original. The atoms now come from `lib/certification.ts` — the + * one definition each of the chunk-scoped uncoverable veto, the + * brief-opened floor, and the findings-pointer read — so what remains here + * is only the composition recovery needs (the `returned` field read among + * them). */ -function chunkOfKey(key: string): number | null { - const m = /^chunk-(\d+)$/.exec(key) ?? /--chunk-(\d+)(?:--|$)/.exec(key); - return m ? Number(m[1]) : null; -} - function meetsBar( rec: AgentRecord, planPath: string, @@ -156,19 +139,12 @@ function meetsBar( // two authorities of one pipeline answering oppositely about one // transcript. `latestReverseAuditRound` regressed with the drop and the // resumed run restarted a round the dead attempt had completed. - const own = assignedChunk(rec); - const declared = UNCOVERABLE_RE.exec(rec.finalText); - if (declared !== null && own !== null && Number(declared[1]) === own) { - return false; - } + if (declaresOwnUncoverable(rec, assignedChunk(rec))) return false; // EVERY role opens its brief — the live walk gates `ok` on `unreadBriefs` // for chunk agents too: the brief carries the severity bar, the finding // format and the project's own rules, and a chunk agent that skipped it // reviewed against rules it never saw. - const openedBrief = rec.successfulCallArgs.some((a) => - a.includes(JSON.stringify(briefPath(planPath, key))), - ); - if (!openedBrief) return false; + if (!openedBrief(rec, planPath, key)) return false; if (/^chunk-\d+$/.test(key)) { // The chunk ROLE only — its proof of territory is the diff it opened, // and its prompt names no findings list. Keyed on the exact bare form: @@ -192,13 +168,7 @@ function meetsBar( // Deriving from the key made the floor silently vanish for exactly those // auditors, and compose-time then ruled the same key `findings-unread`. const pointer = findingsPointerOf(builtPrompt); - if (pointer === null) { - return chunk !== null ? rec.diffToolCalls > 0 : chunklessFloor; - } - const readList = rec.successfulReadFileArgs.some((a) => - a.includes(JSON.stringify(pointer)), - ); - if (!readList) return false; + if (!readFindingsPointer(rec, pointer)) return false; return chunk !== null ? rec.diffToolCalls > 0 : chunklessFloor; }