From d1d9701d16b211e43be2e261bb78cf2544930dc1 Mon Sep 17 00:00:00 2001 From: verify Date: Sun, 9 Aug 2026 10:42:38 +0800 Subject: [PATCH 1/8] fix(review): polish the two budget-gap rendering defects a live body showed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first v0.21.8 CI reviews put the disclosure channel in production, and one posted body (#8732) showed two blemishes. `(none)` reached the body as a phantom gap — the wrapping parenthesis defeated the placeholder filter's leading-token match — so the normalized form now sheds wrapping brackets first. And the disclosing agent was named by the fallback label, the launch prompt's first line: "You are review agent `reverse-audit` — Reverse audit agen...", the run's own plumbing truncated onto a public PR page. A record whose launch matches a built role prompt is now named by its brief's publicLabel — the author's register, the same one the rest of the posted body speaks in. --- .../commands/review/check-coverage.test.ts | 34 +++++++++++++++++++ .../src/commands/review/lib/budget.test.ts | 4 +++ .../cli/src/commands/review/lib/budget.ts | 9 ++++- .../cli/src/commands/review/lib/coverage.ts | 23 ++++++++++++- 4 files changed, 68 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/commands/review/check-coverage.test.ts b/packages/cli/src/commands/review/check-coverage.test.ts index a791e878c15..6619985b3f1 100644 --- a/packages/cli/src/commands/review/check-coverage.test.ts +++ b/packages/cli/src/commands/review/check-coverage.test.ts @@ -39,6 +39,7 @@ import { findingsFilePath, } from './lib/prompt-record.js'; import { requiredAgents, type RosterPlan } from './lib/roster.js'; +import { BRIEFS } from './lib/agent-briefs.js'; import { checkCoverageCommand } from './check-coverage.js'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; @@ -858,6 +859,39 @@ describe('budget-gap disclosures — guarded, parsed, never punished', () => { expect(r.budgetGaps).toEqual([]); }); + it("names a rostered discloser by its brief's publicLabel, not its prompt", () => { + // The fallback label is the launch prompt's first line, and a real + // posted body rendered a disclosure as "You are review agent + // `reverse-audit` — Reverse audit agen...:" — plumbing, truncated, on a + // public PR page. A record matching a built role prompt gets the + // author-register name instead. + transcript('a1', good(1), { calls: 3, range: [0, 100] }); + transcript('a2', good(2), { calls: 2, range: [100, 100] }); + const p = plan(); + const key = 'reverse-audit'; + const d = promptRecordDir(p); + const brief = briefPath(p, key); + const prompt = + `You are ${key}.\n` + + `read_file(file_path="${brief}")\n` + + `read_file(file_path="${DIFF}")`; + writeFileSync(join(d, `${encodeURIComponent(key)}.txt`), prompt); + transcript('tm-gap', prompt, { + calls: 3, + opens: [brief], + text: + 'No issues found — mapped the behaviours.\n' + + 'Budget gap: the negative-path matrix rows', + }); + + const gaps = coverageFromTranscripts(p, ENV).budgetGaps; + const entry = gaps.find((g) => + g.gaps.includes('the negative-path matrix rows'), + ); + expect(entry?.agent).toBe(BRIEFS[key].publicLabel); + expect(entry?.agent).not.toContain('You are'); + }); + it('reports none when nobody disclosed one', () => { transcript('a1', good(1), { calls: 3 }); transcript('a2', good(2), { calls: 2 }); diff --git a/packages/cli/src/commands/review/lib/budget.test.ts b/packages/cli/src/commands/review/lib/budget.test.ts index eea0f756fb0..b531771631d 100644 --- a/packages/cli/src/commands/review/lib/budget.test.ts +++ b/packages/cli/src/commands/review/lib/budget.test.ts @@ -294,6 +294,10 @@ describe('budgetGapDisclosures — the one parser of the disclosure format', () 'Budget gap: N/A - stayed under budget', 'Budget gap: nothing skipped', 'Budget gap: no gaps', + // Bracket-wrapped non-answers reached a real posted body: the + // wrapping parenthesis defeated the leading-token match. + 'Budget gap: (none)', + 'Budget gap: [N/A]', 'Budget gap:', ]) { expect(budgetGapDisclosures(line)).toEqual([]); diff --git a/packages/cli/src/commands/review/lib/budget.ts b/packages/cli/src/commands/review/lib/budget.ts index fdd16fa9688..4fcaec0dd28 100644 --- a/packages/cli/src/commands/review/lib/budget.ts +++ b/packages/cli/src/commands/review/lib/budget.ts @@ -373,7 +373,14 @@ export function budgetGapDisclosures(finalText: string): string[] { raw = raw.slice(0, -1); } raw = stripWrappers(raw.trim()).trim(); - const normalized = raw.replace(/[.!…,;:\s]+$/, '').trim(); + // The normalized form also sheds wrapping brackets: `(none)` reached a + // real posted body as a phantom gap because the leading parenthesis + // defeated the leading-token match below. + const normalized = raw + .replace(/^[([{\s]+/, '') + .replace(/[)\]}\s]+$/, '') + .replace(/[.!…,;:\s]+$/, '') + .trim(); if (normalized.length === 0 || PLACEHOLDER_GAP_RE.test(normalized)) { continue; } diff --git a/packages/cli/src/commands/review/lib/coverage.ts b/packages/cli/src/commands/review/lib/coverage.ts index 57a51dba8ee..b2f9554a655 100644 --- a/packages/cli/src/commands/review/lib/coverage.ts +++ b/packages/cli/src/commands/review/lib/coverage.ts @@ -501,6 +501,22 @@ export function coverageFromTranscripts( } return g; }; + // The rostered role a record's launch was built for, said as the brief's + // publicLabel — the register the posted body renders disclosures in. + // `null` when the launch matches no built role prompt (a chunk agent's + // label is already its chunk; anything else keeps the fallback). + const gapAgentLabel = (rec: AgentRecord): string | null => { + for (const key of built.keys()) { + const b = builtOf(key); + if (b === undefined || !wasDeliveredVerbatim(rec.launchPrompt, b)) { + continue; + } + const brief = BRIEFS[key as keyof typeof BRIEFS]; + if (brief?.publicLabel) return brief.publicLabel; + } + return null; + }; + // A record's gaps are silenced only by a GAP-FREE superseding record — a // genuine repair. Two relaunches that both hit the ceiling and both // disclose would otherwise supersede each other and drop every gap. @@ -673,7 +689,12 @@ export function coverageFromTranscripts( // mutually supersede every disclosure into silence. const gaps = gapsOf(rec); if (gaps.length > 0 && !gapsSuperseded(rec, chunk)) { - budgetGaps.push({ agent: name, gaps }); + // Named in the AUTHOR's register where the record matches a rostered + // launch: the fallback label is the launch prompt's first line, and a + // real posted body rendered a disclosure as "You are review agent + // `reverse-audit` — Reverse audit agen...:" — the run's own plumbing, + // truncated, on a public PR page. + budgetGaps.push({ agent: gapAgentLabel(rec) ?? name, gaps }); } // What it was told to read, plus what it demonstrably read. The second From 4e1928839456a566883d6e984b09cbdfe0b45c20 Mon Sep 17 00:00:00 2001 From: verify Date: Sun, 9 Aug 2026 20:23:08 +0000 Subject: [PATCH 2/8] fix(review): label all coverage channels by role and shed wrapped non-answers Co-authored-by: Qwen-Coder --- .../commands/review/check-coverage.test.ts | 39 +++++++++++++++++-- .../src/commands/review/lib/budget.test.ts | 11 ++++++ .../cli/src/commands/review/lib/budget.ts | 13 ++++--- .../cli/src/commands/review/lib/coverage.ts | 10 +++-- 4 files changed, 60 insertions(+), 13 deletions(-) diff --git a/packages/cli/src/commands/review/check-coverage.test.ts b/packages/cli/src/commands/review/check-coverage.test.ts index 6619985b3f1..71ec24f9abc 100644 --- a/packages/cli/src/commands/review/check-coverage.test.ts +++ b/packages/cli/src/commands/review/check-coverage.test.ts @@ -864,15 +864,18 @@ describe('budget-gap disclosures — guarded, parsed, never punished', () => { // posted body rendered a disclosure as "You are review agent // `reverse-audit` — Reverse audit agen...:" — plumbing, truncated, on a // public PR page. A record matching a built role prompt gets the - // author-register name instead. + // author-register name instead. The key is spelled the way + // agent-prompt records a findings-taking role — with its digest + // suffix — because that is the shape the lookup has to survive. transcript('a1', good(1), { calls: 3, range: [0, 100] }); transcript('a2', good(2), { calls: 2, range: [100, 100] }); const p = plan(); - const key = 'reverse-audit'; + const role = 'reverse-audit'; + const key = `${role}--round-1--abc123def456`; const d = promptRecordDir(p); const brief = briefPath(p, key); const prompt = - `You are ${key}.\n` + + `You are ${role}.\n` + `read_file(file_path="${brief}")\n` + `read_file(file_path="${DIFF}")`; writeFileSync(join(d, `${encodeURIComponent(key)}.txt`), prompt); @@ -888,10 +891,38 @@ describe('budget-gap disclosures — guarded, parsed, never punished', () => { const entry = gaps.find((g) => g.gaps.includes('the negative-path matrix rows'), ); - expect(entry?.agent).toBe(BRIEFS[key].publicLabel); + expect(entry?.agent).toBe(BRIEFS[role].publicLabel); expect(entry?.agent).not.toContain('You are'); }); + it('names idle and unopened rostered agents in the same register', () => { + // The fallback name rides the posted body's coverage lines too: a + // rostered whole-diff agent that made no tool call, or none against + // the diff, must not read "You are ..." there either. + const p = plan(); + const d = promptRecordDir(p); + const launch = (key: string, offset: number): string => { + const brief = briefPath(p, key); + const prompt = + `You are ${key}.\n` + + `read_file(file_path="${brief}")\n` + + `read_file(file_path="${DIFF}", offset=${offset}, limit=100)`; + writeFileSync(join(d, `${encodeURIComponent(key)}.txt`), prompt); + return prompt; + }; + transcript('tm-idle', launch('reverse-audit--round-1--abc123def456', 0), { + calls: 0, + }); + const unopenedKey = 'verify--round-1--fed456abc123'; + transcript('tm-unopened', launch(unopenedKey, 100), { + opens: [briefPath(p, unopenedKey)], + }); + + const r = coverageFromTranscripts(p, ENV); + expect(r.idleAgents).toEqual([BRIEFS['reverse-audit'].publicLabel]); + expect(r.unopenedAgents).toEqual([BRIEFS['verify'].publicLabel]); + }); + it('reports none when nobody disclosed one', () => { transcript('a1', good(1), { calls: 3 }); transcript('a2', good(2), { calls: 2 }); diff --git a/packages/cli/src/commands/review/lib/budget.test.ts b/packages/cli/src/commands/review/lib/budget.test.ts index b531771631d..77a2d825fa6 100644 --- a/packages/cli/src/commands/review/lib/budget.test.ts +++ b/packages/cli/src/commands/review/lib/budget.test.ts @@ -298,6 +298,17 @@ describe('budgetGapDisclosures — the one parser of the disclosure format', () // wrapping parenthesis defeated the leading-token match. 'Budget gap: (none)', 'Budget gap: [N/A]', + // Punctuation-only placeholders wrapped in brackets, with and + // without a trailing period — pinned so a later cleanup cannot + // delete the trailing-strip and flip these into phantom gaps. + 'Budget gap: (-)', + 'Budget gap: (-).', + 'Budget gap: .(-)', + // Quote-wrapped non-answers, straight and smart. + 'Budget gap: "none"', + "Budget gap: 'none'", + 'Budget gap: “none”', + 'Budget gap: "N/A"', 'Budget gap:', ]) { expect(budgetGapDisclosures(line)).toEqual([]); diff --git a/packages/cli/src/commands/review/lib/budget.ts b/packages/cli/src/commands/review/lib/budget.ts index 4fcaec0dd28..e055083f7dc 100644 --- a/packages/cli/src/commands/review/lib/budget.ts +++ b/packages/cli/src/commands/review/lib/budget.ts @@ -373,13 +373,14 @@ export function budgetGapDisclosures(finalText: string): string[] { raw = raw.slice(0, -1); } raw = stripWrappers(raw.trim()).trim(); - // The normalized form also sheds wrapping brackets: `(none)` reached a - // real posted body as a phantom gap because the leading parenthesis - // defeated the leading-token match below. + // The normalized form also sheds wrapping brackets, quotes, and edge + // punctuation: `(none)` reached a real posted body as a phantom gap + // because the wrapping defeated the leading-token match below, and one + // merged pass per side is what lets `(-).` lose its bracket AND its + // period — a fixed-order pass strands the bracket. const normalized = raw - .replace(/^[([{\s]+/, '') - .replace(/[)\]}\s]+$/, '') - .replace(/[.!…,;:\s]+$/, '') + .replace(/^[([{'"“”‘’\s.!…,;:]+/, '') + .replace(/[)\]}'"“”‘’\s.!…,;:]+$/, '') .trim(); if (normalized.length === 0 || PLACEHOLDER_GAP_RE.test(normalized)) { continue; diff --git a/packages/cli/src/commands/review/lib/coverage.ts b/packages/cli/src/commands/review/lib/coverage.ts index b2f9554a655..a7f650a1c12 100644 --- a/packages/cli/src/commands/review/lib/coverage.ts +++ b/packages/cli/src/commands/review/lib/coverage.ts @@ -511,7 +511,9 @@ export function coverageFromTranscripts( if (b === undefined || !wasDeliveredVerbatim(rec.launchPrompt, b)) { continue; } - const brief = BRIEFS[key as keyof typeof BRIEFS]; + // Record keys carry run suffixes (`reverse-audit--round-N--`, + // `${role}--${file}`); BRIEFS is keyed by the bare role. + const brief = BRIEFS[key.split('--')[0] as keyof typeof BRIEFS]; if (brief?.publicLabel) return brief.publicLabel; } return null; @@ -588,7 +590,9 @@ export function coverageFromTranscripts( // is too long. A zero-tool-call agent that merely copied the template must not // be credited with a disclosed gap — that is the whiff wearing a costume. if (rec.successfulToolCalls === 0) { - if (!superseded(rec, chunk)) idleAgents.push(name); + if (!superseded(rec, chunk)) { + idleAgents.push(gapAgentLabel(rec) ?? name); + } continue; } @@ -663,7 +667,7 @@ export function coverageFromTranscripts( // handed both for one agent follows whichever came last. if (told.length > 0 && rec.diffToolCalls === 0) { if (!rewrittenThisRecord && !superseded(rec, chunk)) { - unopenedAgents.push(name); + unopenedAgents.push(gapAgentLabel(rec) ?? name); } continue; } From 17b7c2a198e34ae2eaee8d10849379b80512cfc7 Mon Sep 17 00:00:00 2001 From: verify Date: Mon, 10 Aug 2026 00:33:53 +0000 Subject: [PATCH 3/8] fix(review): keep role-labeled entries from shadowing the caller's own relays MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Naming idle and unopened agents by their brief's publicLabel put those labels into coverage subjects — the very register the orchestrator spells its scoped whiff relays in. The caller-echo filter then prefix-matched `reverse audit — chunk 2's auditor returned nothing substantive twice` against an idle `reverse audit` entry and dropped the disclosure the skill promises renders verbatim, in exactly the runs where a whiffed auditor makes such a relay likeliest. Entries named by a role's publicLabel — the budget stop's, idle/unopened agents', and the Step 4/5 floor's — now never prefix-match, the way the budget-stop entry already did not; a bare subject echo still dedups, and chunk and internal subjects keep the #7188 dedup. Three suggestions from the same round: a file-scoped launch keeps its file (`the invariant check (...) on src/a.ts`), resolved through the roster requirement rather than a key split — a split would misread `verify--` and `reverse-audit--chunk-N` as file-scoped and leak a digest onto the PR page; the role label is now resolved once at the single name-derivation point instead of wrapped at three push sites; and the bracket-strip gains a positive survival case, so a drop-if-wrapped regression can no longer pass the suite green. Co-authored-by: Qwen-Coder --- .../commands/review/check-coverage.test.ts | 73 ++++++++++++++++ .../commands/review/compose-review.test.ts | 87 +++++++++++++++++++ .../cli/src/commands/review/compose-review.ts | 44 +++++++--- .../src/commands/review/lib/budget.test.ts | 11 +++ .../cli/src/commands/review/lib/coverage.ts | 37 +++++--- 5 files changed, 225 insertions(+), 27 deletions(-) diff --git a/packages/cli/src/commands/review/check-coverage.test.ts b/packages/cli/src/commands/review/check-coverage.test.ts index 71ec24f9abc..a93c71ea3ba 100644 --- a/packages/cli/src/commands/review/check-coverage.test.ts +++ b/packages/cli/src/commands/review/check-coverage.test.ts @@ -923,6 +923,79 @@ describe('budget-gap disclosures — guarded, parsed, never punished', () => { expect(r.unopenedAgents).toEqual([BRIEFS['verify'].publicLabel]); }); + it('keeps the file on a file-scoped rostered label', () => { + // A `${role}--${file}` launch is rostered per heavy file; dropping the + // file makes N per-file agents of one role read as one repeated line in + // the posted body, indistinguishable — the author cannot tell which + // file's check stopped. The label keeps the file, the way + // `publicRoleLabel` renders these same roles elsewhere. Not `plan()`: + // its files are not heavy, so its roster carries no invariant agents + // for the label lookup to resolve against. + const p = join(dir, 'plan.json'); + writeFileSync( + p, + JSON.stringify({ + diffPathAbsolute: DIFF, + srcDiffLines: 5000, + diffLines: 5000, + files: [ + { path: 'src/a.ts', kind: 'source', removedLines: 0, heavy: true }, + { path: 'src/b.ts', kind: 'source', removedLines: 0, heavy: true }, + ], + chunks: [ + { id: 1, startLine: 1, endLine: 100 }, + { id: 2, startLine: 101, endLine: 200 }, + ], + }), + ); + const d = promptRecordDir(p); + mkdirSync(d, { recursive: true }); + const role = 'invariant-a'; + const launch = (file: string): string => { + const key = `${role}--${file}`; + const brief = briefPath(p, key); + const prompt = + `You are ${key}.\n` + + `read_file(file_path="${brief}")\n` + + `read_file(file_path="${DIFF}", offset=0, limit=100)`; + writeFileSync(join(d, `${encodeURIComponent(key)}.txt`), prompt); + return prompt; + }; + transcript('inv-idle-a', launch('src/a.ts'), { calls: 0 }); + transcript('inv-idle-b', launch('src/b.ts'), { calls: 0 }); + const old = new Date(2020, 0, 1); + utimesSync(p, old, old); + + const r = coverageFromTranscripts(p, ENV); + const base = BRIEFS[role].publicLabel; + expect(r.idleAgents).toHaveLength(2); + expect(r.idleAgents).toContain(`${base} on src/a.ts`); + expect(r.idleAgents).toContain(`${base} on src/b.ts`); + }); + + it('never reads a digest or chunk suffix as a file on a rostered label', () => { + // Two-segment keys are not all file-scoped: `verify--` and + // `reverse-audit--chunk-N` are Step 3B/4 plumbing. Only the roster's + // per-file requirements carry a file into the label; anything else + // keeps the bare publicLabel, so no digest or chunk id reaches the + // posted body. + const p = plan(); + const d = promptRecordDir(p); + const brief = briefPath(p, 'verify--abc123def456'); + const prompt = + `You are verify--abc123def456.\n` + + `read_file(file_path="${brief}")\n` + + `read_file(file_path="${DIFF}")`; + writeFileSync( + join(d, `${encodeURIComponent('verify--abc123def456')}.txt`), + prompt, + ); + transcript('tm-digest', prompt, { calls: 0 }); + + const r = coverageFromTranscripts(p, ENV); + expect(r.idleAgents).toEqual([BRIEFS['verify'].publicLabel]); + }); + it('reports none when nobody disclosed one', () => { transcript('a1', good(1), { calls: 3 }); transcript('a2', good(2), { calls: 2 }); diff --git a/packages/cli/src/commands/review/compose-review.test.ts b/packages/cli/src/commands/review/compose-review.test.ts index 4461c3522ee..aaecc50ec48 100644 --- a/packages/cli/src/commands/review/compose-review.test.ts +++ b/packages/cli/src/commands/review/compose-review.test.ts @@ -775,6 +775,93 @@ describe('composeReview — event caps (round-7 Critical #2: caps must reach eve expect(r.body.split('review time budget').length - 1).toBe(1); }); + it("an idle rostered agent does not shadow the caller's relay of the same role", () => { + // A whiffed auditor that made zero tool calls lands in `idleAgents`, + // named by its brief's publicLabel — the very subject the orchestrator + // spells its scoped whiff relay in. The caller-echo prefix filter must + // not let the idle entry swallow that relay: the entry explains the + // idleness, the relay carries the scope, and the skill's promise is + // that such prose renders verbatim. The Step 4/5 floor fails the same + // run — its `reverse audit` gap entry shares the subject, so it shares + // the exemption. + transcript('a1', goodPrompt(1), { toolCalls: 3 }); + transcript('a2', goodPrompt(2), { toolCalls: 2 }); + const p = plan({ step45: false }); + recordBuilt(p, 1); + recordBuilt(p, 2); + recordMatrix(p); + const d = promptRecordDir(p); + const brief = briefPath(p, 'reverse-audit'); + writeFileSync(brief, 'The reverse-audit brief.'); + const launch = + `You are review agent \`reverse-audit\`.\n` + + `read_file(file_path="${brief}")\n` + + `read_file(file_path="${DIFF}")`; + writeFileSync(join(d, 'reverse-audit.txt'), launch); + transcript('v-reverse_audit', launch, { toolCalls: 0 }); + + // Not base(): its planPath default runs coveredPlan() on the same + // path, which would relaunch a WORKING reverse auditor over this + // fixture's idle one. + const r = composeReview({ + criticalsInline: 0, + suggestionsInline: 0, + planPath: p, + env: ENV, + modelId: MODEL, + unreviewedDimensions: [ + "reverse audit — chunk 2's auditor returned nothing substantive twice", + ], + }); + expect(r.event).toBe('COMMENT'); + expect(r.body).toContain( + 'Not reviewed: reverse audit — the agent made no tool call: it read nothing.', + ); + expect(r.body).toContain( + "Not reviewed: reverse audit — chunk 2's auditor returned nothing substantive twice.", + ); + }); + + it("an unopened rostered agent does not shadow the caller's relay of the same role", () => { + // The sibling push site: an agent that worked but never opened the diff + // is named in the same register, and its entry must not swallow a relay + // the caller spelled with its own reason either. + transcript('a1', goodPrompt(1), { toolCalls: 3 }); + transcript('a2', goodPrompt(2), { toolCalls: 2 }); + const p = plan({ step45: false }); + recordBuilt(p, 1); + recordBuilt(p, 2); + recordMatrix(p); + const d = promptRecordDir(p); + const brief = briefPath(p, 'verify'); + writeFileSync(brief, 'The verify brief.'); + const launch = + `You are review agent \`verify\`.\n` + + `read_file(file_path="${brief}")\n` + + `read_file(file_path="${DIFF}", offset=0, limit=100)`; + writeFileSync(join(d, 'verify.txt'), launch); + transcript('v-verify', launch, { opens: [brief] }); + + // Not base(): same planPath-default hazard as the idle case above. + const r = composeReview({ + criticalsInline: 0, + suggestionsInline: 0, + planPath: p, + env: ENV, + modelId: MODEL, + unreviewedDimensions: [ + 'verification — the findings file was never ruled on', + ], + }); + expect(r.body).toContain( + 'Not reviewed: verification — pointed at diff lines it never opened: ' + + 'it made tool calls, but none of them read the diff.', + ); + expect(r.body).toContain( + 'Not reviewed: verification — the findings file was never ruled on.', + ); + }); + it('a round-1 budget stop stands alone — no rogue-audit gap, no rebuild FIX', () => { // The gate refused round 1, so no reverse-audit record exists. Without // the marker the floor would report the absence as a rogue/unlaunched diff --git a/packages/cli/src/commands/review/compose-review.ts b/packages/cli/src/commands/review/compose-review.ts index 3b3b25b5f55..c297c4eb595 100644 --- a/packages/cli/src/commands/review/compose-review.ts +++ b/packages/cli/src/commands/review/compose-review.ts @@ -415,7 +415,13 @@ function composeReviewBody( // dropped every OTHER reverse-audit scope the orchestrator disclosed // (`reverse audit — chunk 2's auditor returned nothing substantive // twice`), in exactly the runs where a partial audit makes such scopes - // likeliest. + // likeliest. The same holds for every entry named by a role's + // publicLabel — idle/unopened agents below, the Step 4/5 floor further + // down — so they join one exemption set; a bare subject echo still + // dedups. Chunk and internal-subject entries keep the prefix match: a + // caller echoing those is pasting the gate's own line (#7188), and the + // coverage-derived text wins the collision. + const roleLabelEntries = new Set<(typeof coverageEntries)[number]>(); let budgetEntry: (typeof coverageEntries)[number] | undefined; if (input.planPath) { const stop = readBudgetStop(input.planPath); @@ -427,6 +433,7 @@ function composeReviewBody( } budgetEntry = budgetStopDisclosure(stop.round ?? undefined); coverageEntries.push(budgetEntry); + roleLabelEntries.add(budgetEntry); } } // The fixes for the gaps above, for stderr — never for the body. The gap says @@ -569,11 +576,16 @@ function composeReviewBody( if (!already) uncoverable.push(prefix); } for (const label of cov.idleAgents) { - coverageEntries.push({ + const entry = { subject: label, reason: 'the agent made no tool call: it read nothing', reasonZh: '该 agent 未发起任何工具调用:它什么都没读', - }); + }; + coverageEntries.push(entry); + // A chunk agent's idle label is `chunk N` and keeps the prefix + // dedup; a rostered agent's is a role publicLabel — the caller's + // own relay register — and joins the exemption (see above). + if (!/^chunk \d+$/.test(label)) roleLabelEntries.add(entry); } if (cov.idleAgents.length > 0) { remediation.push( @@ -611,14 +623,16 @@ function composeReviewBody( // spent its run somewhere else, which on a diff with deletions means it // reviewed a file the removed lines are simply not in. for (const label of cov.unopenedAgents) { - coverageEntries.push({ + const entry = { subject: label, reason: 'pointed at diff lines it never opened: it made tool calls, but ' + 'none of them read the diff', reasonZh: '它被指向 diff 的行却从未打开:有工具调用,但没有一次读取 diff', - }); + }; + coverageEntries.push(entry); + if (!/^chunk \d+$/.test(label)) roleLabelEntries.add(entry); } if (cov.unopenedAgents.length > 0) { remediation.push( @@ -718,12 +732,16 @@ function composeReviewBody( // Structural, both languages — no boundary is recovered from rendered // prose (reparsing was the bug the disclosure entries already fixed). for (const gap of verification.gaps) { - coverageEntries.push({ + const entry = { subject: gap.subject, reason: gap.reason, subjectZh: gap.subjectZh, reasonZh: gap.reasonZh, - }); + }; + coverageEntries.push(entry); + // Step 4/5 gap subjects are role publicLabels — the exemption + // register (see `roleLabelEntries`). + roleLabelEntries.add(entry); } remediation.push(...verification.remediation); criticalsUnverified = @@ -1049,15 +1067,15 @@ function composeReviewBody( for (const d of unreviewed) { if (seenCaller.has(d)) continue; // a caller pasting itself twice seenCaller.add(d); - // The budget-stop entry never prefix-matches: its relays are already - // deduped by the marker phrase above, and letting its `reverse audit` - // subject claim the prefix swallowed unrelated reverse-audit scopes the - // caller disclosed with their own reasons (a bare subject echo still - // dedups). + // Role-publicLabel subjects never prefix-match (`roleLabelEntries`): + // a relay that begins with one and brings its own reason after the + // em-dash is a SCOPE those entries do not explain, and the skill + // promises such prose renders verbatim. A bare subject echo still + // dedups. const echoesCoverage = covEntries.some( (e) => d === e.subject || - (e !== budgetEntry && d.startsWith(`${e.subject} — `)), + (!roleLabelEntries.has(e) && d.startsWith(`${e.subject} — `)), ); if (!echoesCoverage) callerLeft.push(d); } diff --git a/packages/cli/src/commands/review/lib/budget.test.ts b/packages/cli/src/commands/review/lib/budget.test.ts index 77a2d825fa6..0d2c9ad7c1c 100644 --- a/packages/cli/src/commands/review/lib/budget.test.ts +++ b/packages/cli/src/commands/review/lib/budget.test.ts @@ -315,6 +315,17 @@ describe('budgetGapDisclosures — the one parser of the disclosure format', () } }); + it('keeps a bracket-wrapped gap that names a real check — wrappers and all', () => { + // The survival half of the edge-strip, which the negatives above cannot + // pin: a drop-if-wrapped mutant passes every negative case yet silently + // discloses nothing here, in the channel where a lost gap is + // unobservable downstream. The pushed gap keeps `raw`, wrappers + // included — that is what makes this assertion discriminate. + expect( + budgetGapDisclosures('Budget gap: (retry path) the remaining callers'), + ).toEqual(['(retry path) the remaining callers']); + }); + it('folds duplicate disclosures into one gap', () => { // An agent commonly states its gap mid-return and restates it in the // closing summary — one gap, not two, and duplicates must not consume diff --git a/packages/cli/src/commands/review/lib/coverage.ts b/packages/cli/src/commands/review/lib/coverage.ts index a7f650a1c12..d73cd14aca3 100644 --- a/packages/cli/src/commands/review/lib/coverage.ts +++ b/packages/cli/src/commands/review/lib/coverage.ts @@ -505,16 +505,27 @@ export function coverageFromTranscripts( // publicLabel — the register the posted body renders disclosures in. // `null` when the launch matches no built role prompt (a chunk agent's // label is already its chunk; anything else keeps the fallback). - const gapAgentLabel = (rec: AgentRecord): string | null => { + const rosteredLabel = (rec: AgentRecord): string | null => { for (const key of built.keys()) { const b = builtOf(key); if (b === undefined || !wasDeliveredVerbatim(rec.launchPrompt, b)) { continue; } // Record keys carry run suffixes (`reverse-audit--round-N--`, - // `${role}--${file}`); BRIEFS is keyed by the bare role. + // `verify--`); BRIEFS is keyed by the bare role. const brief = BRIEFS[key.split('--')[0] as keyof typeof BRIEFS]; - if (brief?.publicLabel) return brief.publicLabel; + if (!brief?.publicLabel) continue; + // A file-scoped launch (an invariant agent per heavy file) keeps its + // file, resolved through the ROSTER requirement rather than a key + // split: only invariant agents are rostered per file, the roster's + // key is spelled exactly the way `agent-prompt` records it, and a + // split would misread `verify--` or `reverse-audit--chunk-N` + // as file-scoped, leaking a digest or a chunk id onto the PR page. + // Without the file, N per-file agents of one role would read as one + // repeated line — the author could not tell which file's check + // stopped. + const req = rosterForRun.find((r) => r.key === key); + return (req && publicRoleLabel(req)) || brief.publicLabel; } return null; }; @@ -571,7 +582,12 @@ export function coverageFromTranscripts( for (const rec of records) { const chunk = assignedChunk(rec); - const name = label(rec, chunk); + // Resolved ONCE, at the single name-derivation point, so every report + // category below rides it: the fallback label is the launch prompt's + // first line, and a real posted body rendered a disclosure as "You are + // review agent `reverse-audit` — Reverse audit agen...:" — the run's + // own plumbing, truncated, on a public PR page. + const name = rosteredLabel(rec) ?? label(rec, chunk); // Could this agent have read the diff at all? The prompt is the harness's // record of what was asked of it. 23 of 23 real chunk agents were launched @@ -590,9 +606,7 @@ export function coverageFromTranscripts( // is too long. A zero-tool-call agent that merely copied the template must not // be credited with a disclosed gap — that is the whiff wearing a costume. if (rec.successfulToolCalls === 0) { - if (!superseded(rec, chunk)) { - idleAgents.push(gapAgentLabel(rec) ?? name); - } + if (!superseded(rec, chunk)) idleAgents.push(name); continue; } @@ -667,7 +681,7 @@ export function coverageFromTranscripts( // handed both for one agent follows whichever came last. if (told.length > 0 && rec.diffToolCalls === 0) { if (!rewrittenThisRecord && !superseded(rec, chunk)) { - unopenedAgents.push(gapAgentLabel(rec) ?? name); + unopenedAgents.push(name); } continue; } @@ -693,12 +707,7 @@ export function coverageFromTranscripts( // mutually supersede every disclosure into silence. const gaps = gapsOf(rec); if (gaps.length > 0 && !gapsSuperseded(rec, chunk)) { - // Named in the AUTHOR's register where the record matches a rostered - // launch: the fallback label is the launch prompt's first line, and a - // real posted body rendered a disclosure as "You are review agent - // `reverse-audit` — Reverse audit agen...:" — the run's own plumbing, - // truncated, on a public PR page. - budgetGaps.push({ agent: gapAgentLabel(rec) ?? name, gaps }); + budgetGaps.push({ agent: name, gaps }); } // What it was told to read, plus what it demonstrably read. The second From 4e591decfe0752b7c6f005bd544ea2b0920b2439 Mon Sep 17 00:00:00 2001 From: verify Date: Mon, 10 Aug 2026 06:41:16 +0000 Subject: [PATCH 4/8] test(review): pin the verification-gap relay exemption and the label fallback --- .../commands/review/check-coverage.test.ts | 26 +++++++++++++++ .../commands/review/compose-review.test.ts | 33 +++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/packages/cli/src/commands/review/check-coverage.test.ts b/packages/cli/src/commands/review/check-coverage.test.ts index a93c71ea3ba..cd8080335b7 100644 --- a/packages/cli/src/commands/review/check-coverage.test.ts +++ b/packages/cli/src/commands/review/check-coverage.test.ts @@ -895,6 +895,32 @@ describe('budget-gap disclosures — guarded, parsed, never punished', () => { expect(entry?.agent).not.toContain('You are'); }); + it('keeps the launch first line for a discloser no built prompt matches', () => { + // The boundary of the rename above: only a record matching a BUILT + // role prompt escapes the fallback, and the fallback is the launch + // prompt's first line, truncated — the exact register the production + // spill wore. A discloser whose prompt the run wrote itself keeps + // that name; a regression to a worse default fails right here, in + // the channel where the spill landed. + transcript('a1', good(1), { calls: 3, range: [0, 100] }); + transcript('a2', good(2), { calls: 2, range: [100, 100] }); + const p = plan(); + const prompt = + 'You are review agent `free-lance`, an extra pass this run wrote for itself.\n' + + `read_file(file_path="${DIFF}", offset=0, limit=100)`; + transcript('stray', prompt, { + calls: 2, + range: [0, 100], + text: 'Walked what I could.\nBudget gap: the stray pass', + }); + + const gaps = coverageFromTranscripts(p, ENV).budgetGaps; + const entry = gaps.find((g) => g.gaps.includes('the stray pass')); + expect(entry?.agent).toBe( + 'You are review agent `free-lance`, an extra pass this run...', + ); + }); + it('names idle and unopened rostered agents in the same register', () => { // The fallback name rides the posted body's coverage lines too: a // rostered whole-diff agent that made no tool call, or none against diff --git a/packages/cli/src/commands/review/compose-review.test.ts b/packages/cli/src/commands/review/compose-review.test.ts index aaecc50ec48..fca6f03739b 100644 --- a/packages/cli/src/commands/review/compose-review.test.ts +++ b/packages/cli/src/commands/review/compose-review.test.ts @@ -862,6 +862,39 @@ describe('composeReview — event caps (round-7 Critical #2: caps must reach eve ); }); + it("a Step 4/5 gap entry does not shadow the caller's relay of the same role", () => { + // The third push site of `roleLabelEntries`: when the reverse audit + // never ran at all, the floor's gap entry carries the subject + // `reverse audit` with no idle or unopened entry sharing it — that + // entry alone can swallow the scoped relay the orchestrator spelled + // with its own reason. The gap explains the missing floor, the relay + // carries the scope; both must render. The idle/unopened tests above + // would each still pass if the verification-gap addition were removed + // (their own entries cover the shared subject); this one pins the site. + const p = coveredPlan(['verify']); // reverse audit absent: the floor fails + + // Not base(): its planPath default runs coveredPlan() on the same + // path, which would re-record the very step this case means to lack. + const r = composeReview({ + criticalsInline: 0, + suggestionsInline: 0, + planPath: p, + env: ENV, + modelId: MODEL, + unreviewedDimensions: [ + "reverse audit — chunk 2's auditor returned nothing substantive twice", + ], + }); + expect(r.event).toBe('COMMENT'); + expect(r.body).toContain( + 'Not reviewed: reverse audit — no auditor was launched with a prompt ' + + 'this skill builds', + ); + expect(r.body).toContain( + "Not reviewed: reverse audit — chunk 2's auditor returned nothing substantive twice.", + ); + }); + it('a round-1 budget stop stands alone — no rogue-audit gap, no rebuild FIX', () => { // The gate refused round 1, so no reverse-audit record exists. Without // the marker the floor would report the absence as a rogue/unlaunched From b955fe8b88056c99550cb0a1e5ea8f30fcab5416 Mon Sep 17 00:00:00 2001 From: verify Date: Tue, 11 Aug 2026 14:45:39 +0800 Subject: [PATCH 5/8] fix(review): drop the dead stripTrailingGapChars the main merge stranded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The merge of main into this branch left `stripTrailingGapChars` with no caller — `stripGapWrappers` inlines the same two-pointer trailing strip and is the only consumer of the normalize/fold path — so `tsc --build` failed with TS6133 (declared but never read), failing the Test gate. Remove the dead function and de-dangle the comment in `stripGapWrappers` that named it; `TRAILING_GAP_CHAR_RE` stays (that walker uses it). --- packages/cli/src/commands/review/lib/budget.ts | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/packages/cli/src/commands/review/lib/budget.ts b/packages/cli/src/commands/review/lib/budget.ts index 0a59d9656dd..6b1cefd5dd0 100644 --- a/packages/cli/src/commands/review/lib/budget.ts +++ b/packages/cli/src/commands/review/lib/budget.ts @@ -457,16 +457,6 @@ function stripWrappers(s: string): string { const TRAILING_GAP_CHAR_RE = /[.!…,;:\s]/; -/** Trailing punctuation/whitespace strip for the normalize and fold keys. */ -function stripTrailingGapChars(s: string): string { - // Walked backwards rather than replaced with an end-anchored class run: - // that shape backtracks quadratically when a long run fails to reach - // the end. - let end = s.length; - while (end > 0 && TRAILING_GAP_CHAR_RE.test(s.charAt(end - 1))) end--; - return s.slice(0, end); -} - /** Wrapping bracket/quote pairs stripped only SYMMETRICALLY. */ const GAP_WRAPPER_CLOSES: Record = { '(': ')', @@ -486,8 +476,9 @@ const GAP_WRAPPER_CLOSES: Record = { * clause's closing paren intact for the classifier's parenthesized shape. */ function stripGapWrappers(s: string): string { - // Two-pointer like stripTrailingGapChars — this parse runs on every - // agent return, so no anchored class runs that could backtrack. + // Two-pointer (walk both ends inward) rather than end-anchored class + // runs — this parse runs on every agent return, so nothing that could + // backtrack quadratically when a long run fails to reach the edge. let start = 0; let end = s.length; for (;;) { From f358c31a4021cdb4e10e246ce55fc0ac7e80866f Mon Sep 17 00:00:00 2001 From: qwen-code-dev-bot Date: Tue, 11 Aug 2026 20:20:04 +0000 Subject: [PATCH 6/8] fix(review): render all same-role disclosures; strip more phantom gaps (#8778) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The body renderer deduped coverage entries on the subject alone, but the publicLabel rename made every round/shard of a role share one subject — so the second of two same-role failures (a budget stop beside an idle round, two differently-failed verify shards, an idle entry beside the Step 4/5 floor gap) never rendered. Key the dedup on the subject+printed-reason pair; bare chunk subjects keep the first-cause-wins shadow, where the later roster line would misclaim "no agent was launched" about a chunk an earlier line already showed launched. Also threaded through review feedback on the same surface: the blind-agent loop joins the caller-relay exemption like its idle/unopened siblings, rostered labels carry their Chinese twins into the bilingual body, and the five hand-spelled chunk-subject regexes ride one shared predicate. The budget-gap parser plugs the same defect class it exists for: balanced fullwidth/CJK wrapper pairs, dash/bullet list markers, and empty wrapper pairs (`()`, `""`) no longer survive normalization as phantom gaps. --- .../commands/review/compose-review.test.ts | 177 +++++++++++++++++- .../cli/src/commands/review/compose-review.ts | 94 +++++++--- .../src/commands/review/lib/budget.test.ts | 22 +++ .../cli/src/commands/review/lib/budget.ts | 16 +- .../cli/src/commands/review/lib/coverage.ts | 37 +++- 5 files changed, 310 insertions(+), 36 deletions(-) diff --git a/packages/cli/src/commands/review/compose-review.test.ts b/packages/cli/src/commands/review/compose-review.test.ts index 43fa599ee91..3849738d184 100644 --- a/packages/cli/src/commands/review/compose-review.test.ts +++ b/packages/cli/src/commands/review/compose-review.test.ts @@ -928,6 +928,176 @@ describe('composeReview — event caps (round-7 Critical #2: caps must reach eve ); }); + it('a budget stop does not swallow an idle round of the same role', () => { + // The publicLabel register is shared by every round of a role: a round-2 + // budget stop and a round-1 auditor that made zero tool calls are ONE + // subject with two distinct failures, and the render dedup keys on the + // subject+reason pair or the second disclosure never reaches the body — + // silently, in the channel whose whole promise is the disclosure. Both + // lines render; the floor's brief-unread gap for the same subject joins + // them, one sentence per reason. + transcript('a1', goodPrompt(1), { toolCalls: 3, range: [0, 100] }); + transcript('a2', goodPrompt(2), { toolCalls: 2, range: [100, 100] }); + const p = plan({ step45: false }); + recordBuilt(p, 1); + recordBuilt(p, 2); + recordMatrix(p); + const d = promptRecordDir(p); + const key = 'reverse-audit--round-1--abc123def456'; + const brief = briefPath(p, key); + writeFileSync(brief, 'The reverse-audit brief.'); + const launch = + `You are review agent \`reverse-audit\` round 1.\n` + + `read_file(file_path="${brief}")\n` + + `read_file(file_path="${DIFF}")`; + writeFileSync(join(d, `${encodeURIComponent(key)}.txt`), launch); + transcript('v-ra-idle', launch, { toolCalls: 0 }); + writeBudgetStop( + p, + { + remainingSeconds: 900, + reserveSeconds: 3600, + expectedRoundSeconds: 1800, + }, + 2, + ); + + // Not base(): its planPath default runs coveredPlan() on the same + // path, which would relaunch a WORKING reverse auditor over this + // fixture's idle one. + const r = composeReview({ + criticalsInline: 0, + suggestionsInline: 0, + planPath: p, + env: ENV, + modelId: MODEL, + }); + expect(r.event).toBe('COMMENT'); + expect(r.body).toContain( + 'Not reviewed: reverse audit — stopped before round 2 by the review time budget.', + ); + expect(r.body).toContain( + 'Not reviewed: reverse audit — the agent made no tool call: it read nothing.', + ); + }); + + it('an unopened chunk entry still dedups a caller relay of the same chunk', () => { + // The `chunk N` guard on `roleLabelEntries` earns its keep: chunk + // subjects keep the prefix dedup (a caller echoing `chunk 1 — …` is + // pasting the gate's own line, #7188), and only role-publicLabel entries + // join the exemption. Delete the guard at either loop and this relay + // renders twice — the coverage sentence and its verbatim echo. The + // fixture isolates the guard: an UNOPENED chunk agent (brief opened, + // zero diff reads) — a zero-tool-call idle chunk agent would also push a + // same-subject entry that dedups the relay regardless of the guard. + transcript('a2', goodPrompt(2), { toolCalls: 2, range: [100, 100] }); + const p = plan({ step45: false }); + recordBuilt(p, 1); + recordBuilt(p, 2); + recordMatrix(p); + transcript('a1', goodPrompt(1), { opens: [briefPath(p, 'chunk-1')] }); + + // Not base(): its planPath default runs coveredPlan() on the same + // path, which would relaunch a WORKING chunk-1 agent over this + // fixture's unopened one. + const r = composeReview({ + criticalsInline: 0, + suggestionsInline: 0, + planPath: p, + env: ENV, + modelId: MODEL, + unreviewedDimensions: [ + 'chunk 1 — the agent read its brief but never the diff', + ], + }); + // The chunk subject renders in the author's units (its files), the + // way every chunk gap does; the relay dedup keys on the internal + // `chunk 1` subject all the same. + expect(r.body).toContain( + 'Not reviewed: the diff section covering src/a.ts — pointed at diff ' + + 'lines it never opened: it made tool calls, but none of them read ' + + 'the diff.', + ); + expect(r.body).not.toContain('the agent read its brief but never the diff'); + }); + + it('an idle rostered agent keeps its Chinese twin in a bilingual body', () => { + // The rename resolves rostered names through the brief's publicLabel; + // the Chinese half must say them the way the sibling floor/budget-stop + // lines do (subjectZh), not fall back to the English label beside + // `反向审计` for the same missing pass. + transcript('a1', goodPrompt(1), { toolCalls: 3, range: [0, 100] }); + transcript('a2', goodPrompt(2), { toolCalls: 2, range: [100, 100] }); + const p = plan({ step45: false, han: true }); + recordBuilt(p, 1); + recordBuilt(p, 2); + recordMatrix(p); + const d = promptRecordDir(p); + const key = 'reverse-audit--round-1--abc123def456'; + const brief = briefPath(p, key); + writeFileSync(brief, 'The reverse-audit brief.'); + const launch = + `You are review agent \`reverse-audit\` round 1.\n` + + `read_file(file_path="${brief}")\n` + + `read_file(file_path="${DIFF}")`; + writeFileSync(join(d, `${encodeURIComponent(key)}.txt`), launch); + transcript('v-ra-idle', launch, { toolCalls: 0 }); + + // Not base(): same planPath-default hazard as the idle case above. + const r = composeReview({ + criticalsInline: 0, + suggestionsInline: 0, + planPath: p, + env: ENV, + modelId: MODEL, + }); + expect(r.body).toContain( + '未审查:反向审计——该 agent 未发起任何工具调用:它什么都没读。', + ); + }); + + it('a rostered budget-gap discloser keeps its Chinese twin in a bilingual body', () => { + // The budget-gap sentence's zh half names the agent too; it rides the + // same label lookup as the coverage entries. + transcript('a2', goodPrompt(2), { toolCalls: 2, range: [100, 100] }); + const p = plan({ step45: false, han: true }); + recordBuilt(p, 1); + recordBuilt(p, 2); + recordMatrix(p); + const d = promptRecordDir(p); + const key = 'reverse-audit'; + const brief = briefPath(p, key); + writeFileSync(brief, 'The reverse-audit brief.'); + const launch = + `You are review agent \`reverse-audit\`.\n` + + `read_file(file_path="${brief}")\n` + + `read_file(file_path="${DIFF}", offset=0, limit=100)`; + writeFileSync(join(d, `${encodeURIComponent(key)}.txt`), launch); + transcript('v-ra-gap', launch, { + toolCalls: 2, + range: [0, 100], + text: + 'No issues found — walked the diff.\n' + + 'Budget gap: the round-2 scope walk', + }); + + // Not base(): same planPath-default hazard as the idle case above. + const r = composeReview({ + criticalsInline: 0, + suggestionsInline: 0, + planPath: p, + env: ENV, + modelId: MODEL, + }); + expect(r.body).toContain( + 'Not explored to full depth (tool budget reached): ' + + 'reverse audit: `the round-2 scope walk`.', + ); + expect(r.body).toContain( + '未探索到全部深度(达到工具调用预算):反向审计:`the round-2 scope walk`。', + ); + }); + it('a round-1 budget stop stands alone — no rogue-audit gap, no rebuild FIX', () => { // The gate refused round 1, so no reverse-audit record exists. Without // the marker the floor would report the absence as a rogue/unlaunched @@ -1037,9 +1207,10 @@ describe('composeReview — event caps (round-7 Critical #2: caps must reach eve 'stopped before round 2 by the review time budget', ); // …and the rewritten round is NOT laundered: the operator channel carries - // its exact repair. (The posted body collapses same-subject disclosures — - // both say "reverse audit" — so the author sees the stop; the rewritten - // repair rides stderr, which is where repairs are acted on.) + // its exact repair, and the posted body renders the floor's rewritten gap + // beside the stop — one subject, two distinct reasons, and the dedup + // keeps both (the rewritten repair itself rides stderr, which is where + // repairs are acted on). expect(r.remediation.join(' ')).toContain('reverse audit:'); expect(r.remediation.join(' ')).toContain('EXACTLY what it prints'); }); diff --git a/packages/cli/src/commands/review/compose-review.ts b/packages/cli/src/commands/review/compose-review.ts index f2de61b9f31..28d5eaa793b 100644 --- a/packages/cli/src/commands/review/compose-review.ts +++ b/packages/cli/src/commands/review/compose-review.ts @@ -92,6 +92,14 @@ export type ReviewEvent = 'APPROVE' | 'REQUEST_CHANGES' | 'COMMENT'; */ export const LOW_SIGNAL_SRC_DIFF_LINES = 100; +/** + * The bare chunk subject, exactly as coverage's labels and the uncoverable + * push below spell it. One predicate for every site that must tell a chunk + * id from prose: the richer caller form `chunk 5 (src/big.min.js)` is + * prose, and deliberately does NOT match. + */ +const CHUNK_SUBJECT_RE = /^chunk (\d+)$/; + /** * Reads a PR's description body, given its `owner/repo` and number. The one * production implementation calls `gh pr view`; the bilingual fallback uses it @@ -683,6 +691,11 @@ function composeReviewBody( // zero-certified test falls to the `coverage` disclosure instead. let plannedChunks: Array<{ id: number; files: string[] }> = []; let coveredChunks: number[] = []; + // The Chinese twins of rostered public labels, keyed by the label — + // coverage resolves them once, at the single name-derivation point; the + // idle/unopened/blind entries and the budget-gap agent names below look + // them up for the bilingual body. + let publicLabelsZh: Record = {}; // The deterministic script-lint gate. `compose-review` is the authority here: // it reads the report the orchestrator's `qwen review script-lint` step wrote @@ -767,6 +780,7 @@ function composeReviewBody( const cov = coverageFromTranscripts(input.planPath, input.env); plannedChunks = cov.plannedChunks; coveredChunks = cov.coveredChunks; + publicLabelsZh = cov.publicLabelsZh; for (const id of cov.missingChunks) missingReceipts.push(id); for (const id of cov.uncoverableChunks) { // The caller may already have named this chunk, but in a richer form: @@ -784,12 +798,13 @@ function composeReviewBody( subject: label, reason: 'the agent made no tool call: it read nothing', reasonZh: '该 agent 未发起任何工具调用:它什么都没读', + subjectZh: publicLabelsZh[label], }; coverageEntries.push(entry); // A chunk agent's idle label is `chunk N` and keeps the prefix // dedup; a rostered agent's is a role publicLabel — the caller's // own relay register — and joins the exemption (see above). - if (!/^chunk \d+$/.test(label)) roleLabelEntries.add(entry); + if (!CHUNK_SUBJECT_RE.test(label)) roleLabelEntries.add(entry); } if (cov.idleAgents.length > 0) { remediation.push( @@ -806,13 +821,21 @@ function composeReviewBody( // this line: the line lands in the posted body, and `qwen review // agent-prompt` is not something a PR author can run. for (const label of cov.blindAgents) { - coverageEntries.push({ + const entry = { subject: label, reason: 'launched with a prompt that never named the diff file, so it ' + 'could not have read it', reasonZh: '启动 prompt 从未提到 diff 文件,它不可能读过 diff', - }); + subjectZh: publicLabelsZh[label], + }; + coverageEntries.push(entry); + // Mirror the idle/unopened loops: the rename means a blind agent + // can carry a role publicLabel (an orchestrator-inserted chunk + // phrase that matched a readsDiff-false role), and a caller relay + // spelled in that register must not be swallowed by the prefix + // match (see `roleLabelEntries`). + if (!CHUNK_SUBJECT_RE.test(label)) roleLabelEntries.add(entry); } if (cov.blindAgents.length > 0) { remediation.push( @@ -834,9 +857,10 @@ function composeReviewBody( 'none of them read the diff', reasonZh: '它被指向 diff 的行却从未打开:有工具调用,但没有一次读取 diff', + subjectZh: publicLabelsZh[label], }; coverageEntries.push(entry); - if (!/^chunk \d+$/.test(label)) roleLabelEntries.add(entry); + if (!CHUNK_SUBJECT_RE.test(label)) roleLabelEntries.add(entry); } if (cov.unopenedAgents.length > 0) { remediation.push( @@ -1239,7 +1263,7 @@ function composeReviewBody( const bareIds: number[] = []; const callerNamed: string[] = []; for (const e of uncoverable) { - const m = /^chunk (\d+)$/.exec(e); + const m = CHUNK_SUBJECT_RE.exec(e); if (m) bareIds.push(Number(m[1])); else callerNamed.push(e); } @@ -1315,9 +1339,19 @@ function composeReviewBody( // and the body must not say it twice in two registers. These are // "stopped at the budget", not "nobody looked": the phrasing must not // claim the stronger gap, and the entries do not join the capping lists. - const budgetGapItems: Array<{ agent: string; gap: string }> = []; + const budgetGapItems: Array<{ + agent: string; + agentZh?: string; + gap: string; + }> = []; for (const g of budgetGapNotes) { - for (const gap of g.gaps) budgetGapItems.push({ agent: g.agent, gap }); + for (const gap of g.gaps) { + budgetGapItems.push({ + agent: g.agent, + agentZh: publicLabelsZh[g.agent], + gap, + }); + } } const keptBudgetGaps = budgetGapItems.filter( (it) => !unreviewed.some((d) => d.includes(it.gap)), @@ -1329,8 +1363,9 @@ function composeReviewBody( shown.map((it) => `${it.agent}: ${mdField(it.gap)}`).join('; ') + (more > 0 ? `, and ${more} more` : ''); const zhList = - shown.map((it) => `${it.agent}:${mdField(it.gap)}`).join(';') + - (more > 0 ? `,另有 ${more} 条` : ''); + shown + .map((it) => `${it.agentZh ?? it.agent}:${mdField(it.gap)}`) + .join(';') + (more > 0 ? `,另有 ${more} 条` : ''); notReviewedParts.push({ en: `Not explored to full depth (tool budget reached): ${enList}.`, zh: `未探索到全部深度(达到工具调用预算):${zhList}。`, @@ -1341,38 +1376,47 @@ function composeReviewBody( // paragraphs — a posted body on #7166 was ninety-nine clauses over four // causes, the six real findings buried beneath. Grouped by the reason // STRING, so a reason embedding per-subject detail (an unread brief\'s own - // path) differs per entry and keeps its own line. One subject that appears - // under two causes keeps the FIRST — the categories push in precision - // order, and a chunk flagged `rewritten` is also, to the roster, a - // requirement with no verbatim launch; repeating it under the later, vaguer - // cause would tell the author "no agent was launched" about an agent that - // demonstrably ran. - const seenSubjects = new Set(); + // path) differs per entry and keeps its own line. + const seenEntries = new Set(); const byReason = new Map< string, Array<{ subject: string; publicSubject?: string; subjectZh?: string }> >(); const reasonZhOf = new Map(); for (const e of covEntries) { - if (seenSubjects.has(e.subject)) continue; - seenSubjects.add(e.subject); // Keyed on the reason the body will PRINT — public over internal. Two // unread briefs differ internally only by their brief paths; grouped on // those, the path-free public sentence would render once per role, which // is the per-subject repetition this map exists to kill. - const key = e.publicReason ?? e.reason; - const group = byReason.get(key) ?? []; + const reasonKey = e.publicReason ?? e.reason; + // Deduped on the subject AND the printed reason, not the subject alone: + // the publicLabel register is shared by every round and shard of a role, + // so one subject can owe several distinct disclosures — a budget stop + // and an idle round both render as `reverse audit`, and a subject-only + // shadow silently dropped the second, in the channel whose whole promise + // is the disclosure. True duplicates (one subject, one printed reason) + // still collapse. Bare chunk subjects keep the first-cause-wins shadow: + // the chunk categories derive from one another for a single chunk, and + // the later, vaguer cause would say "no agent was launched" about a + // chunk an earlier line already showed launched — the precise cause + // keeps the subject. + const dedupKey = CHUNK_SUBJECT_RE.test(e.subject) + ? e.subject + : `${e.subject}\u0000${reasonKey}`; + if (seenEntries.has(dedupKey)) continue; + seenEntries.add(dedupKey); + const group = byReason.get(reasonKey) ?? []; group.push({ subject: e.subject, publicSubject: e.publicSubject, subjectZh: e.subjectZh, }); - byReason.set(key, group); + byReason.set(reasonKey, group); // One printed reason, one translation: entries sharing the printed // English reason share the Chinese one by construction (both derive from // the same source string). Entries with none fall back to the English. - if (e.reasonZh !== undefined && !reasonZhOf.has(key)) { - reasonZhOf.set(key, e.reasonZh); + if (e.reasonZh !== undefined && !reasonZhOf.has(reasonKey)) { + reasonZhOf.set(reasonKey, e.reasonZh); } } for (const [reason, entries] of byReason) { @@ -1389,7 +1433,7 @@ function composeReviewBody( const named: string[] = []; const namedZh: string[] = []; for (const e of entries) { - const m = /^chunk (\d+)$/.exec(e.subject); + const m = CHUNK_SUBJECT_RE.exec(e.subject); if (m) chunkIds.push(Number(m[1])); else { named.push(e.publicSubject ?? e.subject); @@ -1600,7 +1644,7 @@ function composeReviewBody( // certified. const disclosedChunkIds = new Set(); for (const e of coverageEntries) { - const m = /^chunk (\d+)$/.exec(e.subject); + const m = CHUNK_SUBJECT_RE.exec(e.subject); if (m) disclosedChunkIds.add(Number(m[1])); } const nothingCertified = diff --git a/packages/cli/src/commands/review/lib/budget.test.ts b/packages/cli/src/commands/review/lib/budget.test.ts index 16d12b5de89..31a1ecf27a8 100644 --- a/packages/cli/src/commands/review/lib/budget.test.ts +++ b/packages/cli/src/commands/review/lib/budget.test.ts @@ -344,8 +344,30 @@ describe('budgetGapDisclosures — the one parser of the disclosure format', () // Quote-wrapped non-answers, straight and smart. 'Budget gap: "none"', "Budget gap: 'none'", + 'Budget gap: ‘none’', + 'Budget gap: {N/A}', 'Budget gap: “none”', 'Budget gap: "N/A"', + // Fullwidth/CJK wrappers — the disclosure marker is bilingual, and a + // bilingual agent's `(none)` defeated the classifier exactly the way + // `(none)` did before the wrapper pairs joined the strip. + 'Budget gap: (none)', + 'Budget gap: 【N/A】', + 'Budget gap: 「none」', + // List markers from the brief's bullet format wrap placeholders the + // same way brackets do. + 'Budget gap: - none', + 'Budget gap: — none', + 'Budget gap: * none', + 'Budget gap: _ none', + 'Budget gap: - None.', + // Empty wrapper pairs: nothing inside a balanced pair is nothing. + 'Budget gap: ""', + 'Budget gap: ()', + 'Budget gap: []', + 'Budget gap: {}', + "Budget gap: ''", + 'Budget gap: “”', 'Budget gap:', ]) { expect(budgetGapDisclosures(line)).toEqual([]); diff --git a/packages/cli/src/commands/review/lib/budget.ts b/packages/cli/src/commands/review/lib/budget.ts index 6b1cefd5dd0..aa09e875e2d 100644 --- a/packages/cli/src/commands/review/lib/budget.ts +++ b/packages/cli/src/commands/review/lib/budget.ts @@ -455,7 +455,10 @@ function stripWrappers(s: string): string { return out; } -const TRAILING_GAP_CHAR_RE = /[.!…,;:\s]/; +// Edge characters also include list markers (`- none`, `* none`): the +// budget brief hands agents a bullet format, so a placeholder can arrive +// marker-wrapped exactly the way it arrives bracket-wrapped. +const TRAILING_GAP_CHAR_RE = /[-—–*_.!…,;:\s]/; /** Wrapping bracket/quote pairs stripped only SYMMETRICALLY. */ const GAP_WRAPPER_CLOSES: Record = { @@ -466,6 +469,13 @@ const GAP_WRAPPER_CLOSES: Record = { "'": "'", '“': '”', '‘': '’', + // The disclosure marker is bilingual, and so are the wrappers a + // bilingual agent reaches for — `(none)` defeated the classifier + // exactly the way `(none)` did before these joined. + '(': ')', + '【': '】', + '「': '」', + '『': '』', }; /** @@ -474,6 +484,8 @@ const GAP_WRAPPER_CLOSES: Record = { * must lose its period AND its bracket), so a fixed-order single pass per * side strands one of the layers. The balance keeps an inner completion * clause's closing paren intact for the classifier's parenthesized shape. + * An empty wrapper pair strips to nothing — nothing inside a balanced + * pair is nothing, and a bare `()` or `""` cannot be ruled on as a gap. */ function stripGapWrappers(s: string): string { // Two-pointer (walk both ends inward) rather than end-anchored class @@ -484,7 +496,7 @@ function stripGapWrappers(s: string): string { for (;;) { while (start < end && TRAILING_GAP_CHAR_RE.test(s.charAt(start))) start++; while (end > start && TRAILING_GAP_CHAR_RE.test(s.charAt(end - 1))) end--; - if (end - start <= 2) break; + if (end - start < 2) break; const close = GAP_WRAPPER_CLOSES[s.charAt(start)]; if (close === undefined || s.charAt(end - 1) !== close) break; start++; diff --git a/packages/cli/src/commands/review/lib/coverage.ts b/packages/cli/src/commands/review/lib/coverage.ts index bf9eebcfc6a..06ea1ec36f9 100644 --- a/packages/cli/src/commands/review/lib/coverage.ts +++ b/packages/cli/src/commands/review/lib/coverage.ts @@ -166,6 +166,15 @@ export interface CoverageFromTranscripts { * hit its ceiling mid-check. */ budgetGaps: Array<{ agent: string; gaps: string[] }>; + /** + * The Chinese twins of the rostered public labels in this report, keyed + * by the label — for the Chinese half of a bilingual posted body. Only + * labels resolved through a brief's `publicLabel` carry a twin; a + * fallback label (a truncated launch line) has none, and the renderer + * falls back to the English text, the way it does for every subject + * without a Chinese variant. + */ + publicLabelsZh: Record; /** Chunk ids a working agent actually reviewed. */ coveredChunks: number[]; /** @@ -386,6 +395,11 @@ export function coverageFromTranscripts( const blindAgents: string[] = []; const idleAgents: string[] = []; const unopenedAgents: string[] = []; + // The Chinese twins of every rostered public label resolved below, keyed + // by the label — the bilingual body looks them up for the idle/unopened/ + // blind entries and the budget-gap agent names, the way the Step 4/5 + // floor gaps and the budget-stop marker carry theirs inline. + const publicLabelsZh: Record = {}; const rewrittenPrompts: string[] = []; const driftedLaunches: string[] = []; // Did this record's agent open the brief recorded under `key`? Compared as a @@ -502,10 +516,13 @@ export function coverageFromTranscripts( return g; }; // The rostered role a record's launch was built for, said as the brief's - // publicLabel — the register the posted body renders disclosures in. - // `null` when the launch matches no built role prompt (a chunk agent's - // label is already its chunk; anything else keeps the fallback). - const rosteredLabel = (rec: AgentRecord): string | null => { + // publicLabel — the register the posted body renders disclosures in — + // with its Chinese twin for the bilingual body. `null` when the launch + // matches no built role prompt (a chunk agent's label is already its + // chunk; anything else keeps the fallback). + const rosteredLabel = ( + rec: AgentRecord, + ): { label: string; labelZh?: string } | null => { for (const key of built.keys()) { const b = builtOf(key); if (b === undefined || !wasDeliveredVerbatim(rec.launchPrompt, b)) { @@ -525,7 +542,10 @@ export function coverageFromTranscripts( // repeated line — the author could not tell which file's check // stopped. const req = rosterForRun.find((r) => r.key === key); - return (req && publicRoleLabel(req)) || brief.publicLabel; + return { + label: (req && publicRoleLabel(req)) || brief.publicLabel, + labelZh: (req && publicRoleLabelZh(req)) || brief.publicLabelZh, + }; } return null; }; @@ -587,7 +607,11 @@ export function coverageFromTranscripts( // first line, and a real posted body rendered a disclosure as "You are // review agent `reverse-audit` — Reverse audit agen...:" — the run's // own plumbing, truncated, on a public PR page. - const name = rosteredLabel(rec) ?? label(rec, chunk); + const rostered = rosteredLabel(rec); + const name = rostered?.label ?? label(rec, chunk); + if (rostered?.labelZh !== undefined) { + publicLabelsZh[rostered.label] = rostered.labelZh; + } // Could this agent have read the diff at all? The prompt is the harness's // record of what was asked of it. 23 of 23 real chunk agents were launched @@ -1008,6 +1032,7 @@ export function coverageFromTranscripts( missingChunks, uncoverableChunks: [...uncoverable].sort((a, b) => a - b), budgetGaps, + publicLabelsZh, coveredChunks: [...covered].sort((a, b) => a - b), plannedChunks: plan.chunks.map((c) => ({ id: c.id, From cb4c6db7d3445e2b91ee9d97ef07d61e9c3ec479 Mon Sep 17 00:00:00 2001 From: qwen-code-dev-bot Date: Tue, 11 Aug 2026 23:59:30 +0000 Subject: [PATCH 7/8] fix(review): strip fullwidth phantom gaps; pin the relay-exemption register (#8778) --- .../commands/review/compose-review.test.ts | 315 ++++++++++++++++++ .../cli/src/commands/review/compose-review.ts | 38 ++- .../src/commands/review/lib/budget.test.ts | 21 ++ .../cli/src/commands/review/lib/budget.ts | 7 +- .../cli/src/commands/review/lib/coverage.ts | 9 +- 5 files changed, 379 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/commands/review/compose-review.test.ts b/packages/cli/src/commands/review/compose-review.test.ts index 3849738d184..f5de94a2859 100644 --- a/packages/cli/src/commands/review/compose-review.test.ts +++ b/packages/cli/src/commands/review/compose-review.test.ts @@ -1098,6 +1098,321 @@ describe('composeReview — event caps (round-7 Critical #2: caps must reach eve ); }); + it('a fallback-label entry keeps the prefix dedup for a caller relay', () => { + // The exemption register is ROSTERED publicLabels only: a non-rostered + // launch keeps its fallback label (the truncated `You are review agent + // …` first line), and that is internal register — a caller spelling + // a relay in it is pasting the gate's own line (#7188), so the prefix + // match keeps it. When the guard approximated "rostered" as "not a + // `chunk N` label", fallback-label entries joined the exemption and a + // relayed gate line double-rendered end-to-end (probe-reproduced): a + // free-lance agent goes idle, coverage names it by the fallback, the + // orchestrator relays in the same register. + transcript('a1', goodPrompt(1), { toolCalls: 3 }); + transcript('a2', goodPrompt(2), { toolCalls: 2 }); + const p = plan({ step45: false }); + recordBuilt(p, 1); + recordBuilt(p, 2); + recordMatrix(p); + const strayPrompt = + 'You are review agent `free-lance`, an extra pass this run wrote for itself.\n' + + `read_file(file_path="${DIFF}", offset=0, limit=100)`; + transcript('stray', strayPrompt, { toolCalls: 0 }); + const fallbackLabel = + 'You are review agent `free-lance`, an extra pass this run...'; + + // Not base(): its planPath default runs coveredPlan() on the same + // path, which would relaunch a WORKING agent over this idle one. + const r = composeReview({ + criticalsInline: 0, + suggestionsInline: 0, + planPath: p, + env: ENV, + modelId: MODEL, + unreviewedDimensions: [ + `${fallbackLabel} — made zero tool calls on the payment-flow walk`, + ], + }); + const sentence = `Not reviewed: ${fallbackLabel} — the agent made no tool call: it read nothing.`; + expect(r.body).toContain(sentence); + // The relay rides the internal subject register and loses the + // collision to the coverage-derived line — rendered once, not twice. + expect(r.body.split(sentence).length - 1).toBe(1); + expect(r.body).not.toContain( + 'made zero tool calls on the payment-flow walk', + ); + }); + + it('a blind chunk entry still dedups a caller relay of the same chunk', () => { + // The blind-loop sibling of the unopened-chunk guard: a blind agent's + // label is `chunk N` — internal register — and keeps the prefix dedup + // the exemption exists to waive for role publicLabels ONLY. Without the + // guard here, a nothing-built run's blind chunk-1 entry joined the + // exemption and a relayed `chunk 1 — …` line double-rendered; the + // mutant survives every other test, so this one is its oracle. + transcript('a1', blindPrompt(1), { toolCalls: 0 }); + const p = plan({ step45: false }); + + // Not base(): its planPath default runs coveredPlan() on the same + // path, which would relaunch a WORKING chunk-1 agent over this one. + const r = composeReview({ + criticalsInline: 0, + suggestionsInline: 0, + planPath: p, + env: ENV, + modelId: MODEL, + unreviewedDimensions: ['chunk 1 — the agent was launched blind'], + }); + expect(r.body).toContain('never named the diff file'); + expect(r.body).not.toContain('the agent was launched blind'); + }); + + it("a rewritten-launch disclosure in the role register does not shadow the caller's relay", () => { + // The rename gives a rewritten-launch disclosure a role publicLabel + // subject: a chunk agent launched wearing a built role prompt matches + // it (`wasDeliveredVerbatim` is line containment), so `disclose` names + // it `reverse audit`. A caller relay spelled in that register must not + // be swallowed by the prefix match — the exemption the idle/blind/ + // unopened loops earn applies at the `cov.disclosures` spread too. + // Pre-fix the relay below was silently lost end-to-end + // (probe-reproduced); the structural line and the scoped relay must + // both render. + transcript('a2', goodPrompt(2), { toolCalls: 2, range: [100, 100] }); + const p = plan({ step45: false }); + recordBuilt(p, 2); + recordMatrix(p); + const d = promptRecordDir(p); + const brief = briefPath(p, 'reverse-audit'); + writeFileSync(brief, 'The reverse-audit brief.'); + const rolePrompt = + `You are review agent \`reverse-audit\`.\n` + + `read_file(file_path="${brief}")\n` + + `read_file(file_path="${DIFF}", offset=0, limit=100)`; + writeFileSync(join(d, 'reverse-audit.txt'), rolePrompt); + transcript('v-ra-rewritten', `${rolePrompt}\nYou own chunk 1 of 2.`, { + toolCalls: 2, + range: [0, 100], + opens: [brief], + }); + + // Not base(): its planPath default runs coveredPlan() on the same + // path, which would re-record the very step this case means to lack. + const r = composeReview({ + criticalsInline: 0, + suggestionsInline: 0, + planPath: p, + env: ENV, + modelId: MODEL, + unreviewedDimensions: [ + "reverse audit — chunk 1's auditor returned nothing substantive twice", + ], + }); + expect(r.body).toContain( + 'Not reviewed: reverse audit — ran on a prompt the run wrote itself ' + + '(none was built for this chunk), so the brief with its method and ' + + 'rules never reached it.', + ); + expect(r.body).toContain( + "Not reviewed: reverse audit — chunk 1's auditor returned nothing substantive twice.", + ); + }); + + it('a rewritten-launch disclosure keeps its Chinese twin in a bilingual body', () => { + // The rename lets the rewritten-launch disclosures carry a role + // publicLabel subject; the bilingual body's Chinese half must say it + // in Chinese (subjectZh), the way every other rostered entry this + // report plumbs does — pre-fix the zh sentence embedded the English + // label beside proper Chinese siblings (probe-reproduced). + transcript('a2', goodPrompt(2), { toolCalls: 2, range: [100, 100] }); + const p = plan({ step45: false, han: true }); + recordBuilt(p, 2); + recordMatrix(p); + const d = promptRecordDir(p); + const brief = briefPath(p, 'reverse-audit'); + writeFileSync(brief, 'The reverse-audit brief.'); + const rolePrompt = + `You are review agent \`reverse-audit\`.\n` + + `read_file(file_path="${brief}")\n` + + `read_file(file_path="${DIFF}", offset=0, limit=100)`; + writeFileSync(join(d, 'reverse-audit.txt'), rolePrompt); + transcript('v-ra-rewritten', `${rolePrompt}\nYou own chunk 1 of 2.`, { + toolCalls: 2, + range: [0, 100], + opens: [brief], + }); + + // Not base(): its planPath default runs coveredPlan() on the same + // path, which would re-record the very step this case means to lack. + const r = composeReview({ + criticalsInline: 0, + suggestionsInline: 0, + planPath: p, + env: ENV, + modelId: MODEL, + }); + expect(r.body).toContain( + '未审查:反向审计——运行在这次 run 自行编写的 prompt 上(该 chunk 从未构建过 prompt),承载方法与规则的 brief 从未到达该 agent。', + ); + }); + + it('a bare caller echo of an idle rostered subject dedups against the idle entry', () => { + // The bare-echo arm (`d === e.subject`) earns its keep on the exempt + // register: an idle rostered auditor plus a bare relay of its subject + // (no reason after the em-dash) must not render the whiff sentence + // beside the true idle disclosure — a factually wrong claim about the + // run PLUS a double disclosure. Drop the arm and the mutant survives + // every other test; the flip renders the whiff. + transcript('a1', goodPrompt(1), { toolCalls: 3 }); + transcript('a2', goodPrompt(2), { toolCalls: 2 }); + const p = plan({ step45: false }); + recordBuilt(p, 1); + recordBuilt(p, 2); + recordMatrix(p); + const d = promptRecordDir(p); + const brief = briefPath(p, 'reverse-audit'); + writeFileSync(brief, 'The reverse-audit brief.'); + const launch = + `You are review agent \`reverse-audit\`.\n` + + `read_file(file_path="${brief}")\n` + + `read_file(file_path="${DIFF}")`; + writeFileSync(join(d, 'reverse-audit.txt'), launch); + transcript('v-reverse_audit', launch, { toolCalls: 0 }); + + // Not base(): same planPath-default hazard as the idle case above. + const r = composeReview({ + criticalsInline: 0, + suggestionsInline: 0, + planPath: p, + env: ENV, + modelId: MODEL, + unreviewedDimensions: ['reverse audit'], + }); + expect(r.body).toContain( + 'Not reviewed: reverse audit — the agent made no tool call: it read nothing.', + ); + expect(r.body).not.toContain( + 'the agent returned no evidence of its walk twice', + ); + }); + + it('a verbatim re-relay of a coverage sentence dedups in the exempt register too', () => { + // The exemption waives the PREFIX match for role publicLabels — never + // the echo check itself. A relay restating an entry's exact sentence + // carries no new scope (a #7188-style paste of a prior body's "Not + // reviewed" line is exactly this shape); without the exact-sentence + // arm the idle sentence below rendered twice (probe-reproduced). + transcript('a1', goodPrompt(1), { toolCalls: 3 }); + transcript('a2', goodPrompt(2), { toolCalls: 2 }); + const p = plan({ step45: false }); + recordBuilt(p, 1); + recordBuilt(p, 2); + recordMatrix(p); + const d = promptRecordDir(p); + const brief = briefPath(p, 'reverse-audit'); + writeFileSync(brief, 'The reverse-audit brief.'); + const launch = + `You are review agent \`reverse-audit\`.\n` + + `read_file(file_path="${brief}")\n` + + `read_file(file_path="${DIFF}")`; + writeFileSync(join(d, 'reverse-audit.txt'), launch); + transcript('v-reverse_audit', launch, { toolCalls: 0 }); + + // Not base(): same planPath-default hazard as the idle case above. + const r = composeReview({ + criticalsInline: 0, + suggestionsInline: 0, + planPath: p, + env: ENV, + modelId: MODEL, + unreviewedDimensions: [ + 'reverse audit — the agent made no tool call: it read nothing', + ], + }); + const sentence = + 'Not reviewed: reverse audit — the agent made no tool call: it read nothing.'; + expect(r.body.split(sentence).length - 1).toBe(1); + }); + + it('folds two rounds of one role disclosing the identical budget gap', () => { + // The publicLabel register folds every round of a role onto one agent + // name, so a same-round relaunch that hits the same ceiling and + // re-discloses the identical line arrives as two items that render two + // textually identical clauses — and a duplicate inside the + // MAX_BUDGET_GAP_LINES budget can push a distinct third gap into the + // truncation. (agent, gap) dedup folds them the way the parser folds a + // within-return restatement. + transcript('a2', goodPrompt(2), { toolCalls: 2, range: [100, 100] }); + const p = plan({ step45: false }); + recordBuilt(p, 1); + recordBuilt(p, 2); + recordMatrix(p); + const d = promptRecordDir(p); + const brief = briefPath(p, 'reverse-audit'); + writeFileSync(brief, 'The reverse-audit brief.'); + const launch = + `You are review agent \`reverse-audit\`.\n` + + `read_file(file_path="${brief}")\n` + + `read_file(file_path="${DIFF}", offset=0, limit=100)`; + writeFileSync(join(d, 'reverse-audit.txt'), launch); + transcript('v-ra-gap-1', launch, { + toolCalls: 2, + range: [0, 100], + text: 'Walked the diff.\nBudget gap: the retry-path walk', + }); + transcript('v-ra-gap-2', launch, { + toolCalls: 2, + range: [0, 100], + text: 'Walked the diff again.\nBudget gap: the retry-path walk', + }); + + // Not base(): same planPath-default hazard as the idle case above. + const r = composeReview({ + criticalsInline: 0, + suggestionsInline: 0, + planPath: p, + env: ENV, + modelId: MODEL, + }); + expect(r.body).toContain( + 'Not explored to full depth (tool budget reached): ' + + 'reverse audit: `the retry-path walk`.', + ); + expect( + r.body.split('reverse audit: `the retry-path walk`').length - 1, + ).toBe(1); + }); + + it('a chunk-agent budget-gap discloser keeps the English name in the zh half', () => { + // The zh sentence's agent name rides `it.agentZh ?? it.agent`: every + // bilingual budget-gap test above uses a rostered discloser carrying a + // zh twin, so the fallback side never executes — a mutant dropping it + // survives them all and prints literal `undefined:` for a + // non-rostered discloser. A chunk agent is exactly that. + transcript('a2', goodPrompt(2), { toolCalls: 2, range: [100, 100] }); + const p = plan({ step45: false, han: true }); + recordBuilt(p, 1); + recordBuilt(p, 2); + recordMatrix(p); + transcript('a1', goodPrompt(1), { + toolCalls: 2, + range: [0, 100], + text: 'No issues found.\nBudget gap: the negative-path walk', + }); + + // Not base(): same planPath-default hazard as the idle case above. + const r = composeReview({ + criticalsInline: 0, + suggestionsInline: 0, + planPath: p, + env: ENV, + modelId: MODEL, + }); + expect(r.body).toContain( + '未探索到全部深度(达到工具调用预算):chunk 1:`the negative-path walk`。', + ); + expect(r.body).not.toContain('undefined:'); + }); + it('a round-1 budget stop stands alone — no rogue-audit gap, no rebuild FIX', () => { // The gate refused round 1, so no reverse-audit record exists. Without // the marker the floor would report the absence as a rogue/unlaunched diff --git a/packages/cli/src/commands/review/compose-review.ts b/packages/cli/src/commands/review/compose-review.ts index 28d5eaa793b..24a462624e9 100644 --- a/packages/cli/src/commands/review/compose-review.ts +++ b/packages/cli/src/commands/review/compose-review.ts @@ -801,10 +801,11 @@ function composeReviewBody( subjectZh: publicLabelsZh[label], }; coverageEntries.push(entry); - // A chunk agent's idle label is `chunk N` and keeps the prefix - // dedup; a rostered agent's is a role publicLabel — the caller's - // own relay register — and joins the exemption (see above). - if (!CHUNK_SUBJECT_RE.test(label)) roleLabelEntries.add(entry); + // A chunk agent's idle label is `chunk N` and a non-rostered + // launch keeps its fallback label — both are internal register and + // keep the #7188 prefix dedup; only ROSTERED labels (the caller's + // own relay register) join the exemption (see above). + if (publicLabelsZh[label] !== undefined) roleLabelEntries.add(entry); } if (cov.idleAgents.length > 0) { remediation.push( @@ -835,7 +836,7 @@ function composeReviewBody( // phrase that matched a readsDiff-false role), and a caller relay // spelled in that register must not be swallowed by the prefix // match (see `roleLabelEntries`). - if (!CHUNK_SUBJECT_RE.test(label)) roleLabelEntries.add(entry); + if (publicLabelsZh[label] !== undefined) roleLabelEntries.add(entry); } if (cov.blindAgents.length > 0) { remediation.push( @@ -860,7 +861,7 @@ function composeReviewBody( subjectZh: publicLabelsZh[label], }; coverageEntries.push(entry); - if (!CHUNK_SUBJECT_RE.test(label)) roleLabelEntries.add(entry); + if (publicLabelsZh[label] !== undefined) roleLabelEntries.add(entry); } if (cov.unopenedAgents.length > 0) { remediation.push( @@ -880,7 +881,16 @@ function composeReviewBody( // rewritten, missing-role and unread-brief entries arrive structurally // (`cov.disclosures`, push order preserved) — their labels can carry // em-dashes of their own, which is why they are never reparsed here. - coverageEntries.push(...cov.disclosures); + for (const e of cov.disclosures) { + coverageEntries.push(e); + // The rename lets a rewritten-launch disclosure carry a role + // publicLabel as its subject; a caller relay spelled in that + // register must not be swallowed by the prefix match, so it joins + // the exemption exactly the way the idle/blind/unopened entries do. + // missingRoles/unreadBriefs entries keep internal subjects that + // never key into `publicLabelsZh` and keep the prefix dedup. + if (publicLabelsZh[e.subject] !== undefined) roleLabelEntries.add(e); + } if (cov.rewrittenPrompts.length > 0) { remediation.push( 'rewritten launches: re-run `"${QWEN_CODE_CLI:-qwen}" review ' + @@ -1299,10 +1309,14 @@ function composeReviewBody( // a relay that begins with one and brings its own reason after the // em-dash is a SCOPE those entries do not explain, and the skill // promises such prose renders verbatim. A bare subject echo still - // dedups. + // dedups, and so does a verbatim re-relay of an entry's own sentence — + // it carries no new scope, and a #7188-style paste of a prior body's + // line is exactly that shape. const echoesCoverage = covEntries.some( (e) => d === e.subject || + d === + `${e.publicSubject ?? e.subject} — ${e.publicReason ?? e.reason}` || (!roleLabelEntries.has(e) && d.startsWith(`${e.subject} — `)), ); if (!echoesCoverage) callerLeft.push(d); @@ -1344,8 +1358,16 @@ function composeReviewBody( agentZh?: string; gap: string; }> = []; + // The publicLabel register folds every round of a role onto one agent + // name, so two rounds disclosing the identical gap arrive as two items + // that would render two textually identical clauses — fold them the way + // the parser folds a within-return restatement. + const seenBudgetGapItems = new Set(); for (const g of budgetGapNotes) { for (const gap of g.gaps) { + const dedupKey = `${g.agent}\u0000${gap}`; + if (seenBudgetGapItems.has(dedupKey)) continue; + seenBudgetGapItems.add(dedupKey); budgetGapItems.push({ agent: g.agent, agentZh: publicLabelsZh[g.agent], diff --git a/packages/cli/src/commands/review/lib/budget.test.ts b/packages/cli/src/commands/review/lib/budget.test.ts index 31a1ecf27a8..59e13d04f0e 100644 --- a/packages/cli/src/commands/review/lib/budget.test.ts +++ b/packages/cli/src/commands/review/lib/budget.test.ts @@ -354,6 +354,20 @@ describe('budgetGapDisclosures — the one parser of the disclosure format', () 'Budget gap: (none)', 'Budget gap: 【N/A】', 'Budget gap: 「none」', + 'Budget gap: 『none』', + 'Budget gap: 《none》', + // Fullwidth terminal punctuation rides OUTSIDE the wrappers the same + // way a halfwidth period does — `(none)。` must lose the period + // before the wrapper strip can see the pair, or the phantom leaks + // through into the posted body's budget-gap sentence. + 'Budget gap: (none)。', + 'Budget gap: 「none」。', + 'Budget gap: 【N/A】。', + 'Budget gap: "none"。', + 'Budget gap: none。', + 'Budget gap: none.', + // The marker itself is bilingual: the line starts at the CJK marker. + '预算缺口:(none)。', // List markers from the brief's bullet format wrap placeholders the // same way brackets do. 'Budget gap: - none', @@ -383,6 +397,13 @@ describe('budgetGapDisclosures — the one parser of the disclosure format', () expect( budgetGapDisclosures('Budget gap: (retry path) the remaining callers'), ).toEqual(['(retry path) the remaining callers']); + // … and the same holds marker-wrapped: the brief hands agents a bullet + // format, so `- ` is plausible input. A drop-if-marker-leading + // mutant passes every negative above yet silently discards this — the + // loss of a real disclosure shipping green. + expect( + budgetGapDisclosures('Budget gap: - the auth flow is untested'), + ).toEqual(['- the auth flow is untested']); }); it('keeps a REAL gap in parentheses — the paren strip fires only for placeholders', () => { diff --git a/packages/cli/src/commands/review/lib/budget.ts b/packages/cli/src/commands/review/lib/budget.ts index aa09e875e2d..c3d8c967c8a 100644 --- a/packages/cli/src/commands/review/lib/budget.ts +++ b/packages/cli/src/commands/review/lib/budget.ts @@ -457,8 +457,10 @@ function stripWrappers(s: string): string { // Edge characters also include list markers (`- none`, `* none`): the // budget brief hands agents a bullet format, so a placeholder can arrive -// marker-wrapped exactly the way it arrives bracket-wrapped. -const TRAILING_GAP_CHAR_RE = /[-—–*_.!…,;:\s]/; +// marker-wrapped exactly the way it arrives bracket-wrapped. The +// fullwidth terminals join for the same reason the fullwidth wrappers do: +// `(none)。` must lose its period before the wrapper strip can see the pair. +const TRAILING_GAP_CHAR_RE = /[-—–*_.!…,;:。.,!?;:\s]/; /** Wrapping bracket/quote pairs stripped only SYMMETRICALLY. */ const GAP_WRAPPER_CLOSES: Record = { @@ -476,6 +478,7 @@ const GAP_WRAPPER_CLOSES: Record = { '【': '】', '「': '」', '『': '』', + '《': '》', }; /** diff --git a/packages/cli/src/commands/review/lib/coverage.ts b/packages/cli/src/commands/review/lib/coverage.ts index 06ea1ec36f9..c4ee68a3e3e 100644 --- a/packages/cli/src/commands/review/lib/coverage.ts +++ b/packages/cli/src/commands/review/lib/coverage.ts @@ -661,6 +661,10 @@ export function coverageFromTranscripts( 'ran on a prompt the run wrote itself (none was built for this ' + 'chunk), so the brief with its method and rules never reached it', { + // The rename can make `name` a rostered publicLabel; the + // bilingual body needs its twin, the way every other + // rostered entry this report plumbs carries one. + subjectZh: rostered?.labelZh, reasonZh: '运行在这次 run 自行编写的 prompt 上(该 chunk 从未构建过 ' + 'prompt),承载方法与规则的 brief 从未到达该 agent', @@ -688,7 +692,10 @@ export function coverageFromTranscripts( disclose( name, 'launched with a prompt that is not the one the CLI built', - { reasonZh: '启动时使用的 prompt 不是 CLI 构建的那一份' }, + { + subjectZh: rostered?.labelZh, + reasonZh: '启动时使用的 prompt 不是 CLI 构建的那一份', + }, ), ); } From 53c3f5cb3b18a8eff48ad108d5b0399af4ed6007 Mon Sep 17 00:00:00 2001 From: qwen-code-dev-bot Date: Wed, 12 Aug 2026 04:43:14 +0000 Subject: [PATCH 8/8] fix(review): strip the last CJK wrapper phantoms; pin nine unpinned behaviors (#8778) --- .../commands/review/check-coverage.test.ts | 24 +- .../commands/review/compose-review.test.ts | 209 +++++++++++++++++- .../src/commands/review/lib/budget.test.ts | 13 ++ .../cli/src/commands/review/lib/budget.ts | 6 +- 4 files changed, 249 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/commands/review/check-coverage.test.ts b/packages/cli/src/commands/review/check-coverage.test.ts index cd8080335b7..985a3c87425 100644 --- a/packages/cli/src/commands/review/check-coverage.test.ts +++ b/packages/cli/src/commands/review/check-coverage.test.ts @@ -997,6 +997,15 @@ describe('budget-gap disclosures — guarded, parsed, never punished', () => { expect(r.idleAgents).toHaveLength(2); expect(r.idleAgents).toContain(`${base} on src/a.ts`); expect(r.idleAgents).toContain(`${base} on src/b.ts`); + // The Chinese twin keeps the file too: dropping it renders two + // identical zh clauses with no file discriminator — the exact + // indistinguishability this test exists to prevent, on the zh side. + expect(r.publicLabelsZh[`${base} on src/a.ts`]).toBe( + `${BRIEFS[role].publicLabelZh}(src/a.ts)`, + ); + expect(r.publicLabelsZh[`${base} on src/b.ts`]).toBe( + `${BRIEFS[role].publicLabelZh}(src/b.ts)`, + ); }); it('never reads a digest or chunk suffix as a file on a rostered label', () => { @@ -1017,9 +1026,22 @@ describe('budget-gap disclosures — guarded, parsed, never punished', () => { prompt, ); transcript('tm-digest', prompt, { calls: 0 }); + // The chunk-suffixed findings-role key shape: a key-shape heuristic in + // place of the roster lookup would read `chunk-1` as a file and leak + // the chunk id onto the public label, so it keeps the bare publicLabel. + const chunkKey = 'reverse-audit--chunk-1--round-1--abc123def456'; + const chunkBrief = briefPath(p, chunkKey); + const chunkPrompt = + `You are ${chunkKey}.\n` + + `read_file(file_path="${chunkBrief}")\n` + + `read_file(file_path="${DIFF}")`; + writeFileSync(join(d, `${encodeURIComponent(chunkKey)}.txt`), chunkPrompt); + transcript('tm-chunk-key', chunkPrompt, { calls: 0 }); const r = coverageFromTranscripts(p, ENV); - expect(r.idleAgents).toEqual([BRIEFS['verify'].publicLabel]); + expect(r.idleAgents).toHaveLength(2); + expect(r.idleAgents).toContain(BRIEFS['verify'].publicLabel); + expect(r.idleAgents).toContain(BRIEFS['reverse-audit'].publicLabel); }); it('reports none when nobody disclosed one', () => { diff --git a/packages/cli/src/commands/review/compose-review.test.ts b/packages/cli/src/commands/review/compose-review.test.ts index f5de94a2859..a9bd8e30340 100644 --- a/packages/cli/src/commands/review/compose-review.test.ts +++ b/packages/cli/src/commands/review/compose-review.test.ts @@ -928,6 +928,52 @@ describe('composeReview — event caps (round-7 Critical #2: caps must reach eve ); }); + it("a blind rostered agent does not shadow the caller's relay of the same role", () => { + // The blind loop's push into `roleLabelEntries`: a readsDiff-false role + // prompt plus an orchestrator-inserted chunk phrase launches an agent + // that is blind YET rostered — named by the role's publicLabel. Its + // entry must not swallow a relay spelled in that register; delete the + // push and the relay below is silently dropped (probe-reproduced). + // No working chunk-1 agent: one would supersede the blind entry. + transcript('a2', goodPrompt(2), { toolCalls: 2 }); + const p = plan({ step45: false }); + recordBuilt(p, 1); + recordBuilt(p, 2); + recordMatrix(p); + const d = promptRecordDir(p); + const brief = briefPath(p, 'reverse-audit'); + writeFileSync(brief, 'The reverse-audit brief.'); + // The built block never names the diff file; the chunk phrase is the + // orchestrator's insertion at launch. + const builtBlock = + `You are review agent \`reverse-audit\`.\n` + + `read_file(file_path="${brief}")`; + writeFileSync(join(d, 'reverse-audit.txt'), builtBlock); + transcript('v-ra-blind', `${builtBlock}\nYou own chunk 1 of 2.`, { + toolCalls: 0, + }); + + // Not base(): same planPath-default hazard as the idle case above. + const r = composeReview({ + criticalsInline: 0, + suggestionsInline: 0, + planPath: p, + env: ENV, + modelId: MODEL, + unreviewedDimensions: [ + "reverse audit — chunk 1's auditor returned nothing substantive twice", + ], + }); + expect(r.event).toBe('COMMENT'); + expect(r.body).toContain( + 'Not reviewed: reverse audit — launched with a prompt that never ' + + 'named the diff file, so it could not have read it.', + ); + expect(r.body).toContain( + "Not reviewed: reverse audit — chunk 1's auditor returned nothing substantive twice.", + ); + }); + it('a budget stop does not swallow an idle round of the same role', () => { // The publicLabel register is shared by every round of a role: a round-2 // budget stop and a round-1 auditor that made zero tool calls are ONE @@ -979,6 +1025,14 @@ describe('composeReview — event caps (round-7 Critical #2: caps must reach eve expect(r.body).toContain( 'Not reviewed: reverse audit — the agent made no tool call: it read nothing.', ); + // The third reason the comment above promises: the floor's brief-unread + // gap shares the subject, and the subject+reason dedup keeps it beside + // the stop and the idle round. + expect(r.body).toContain( + 'Not reviewed: reverse audit — it was launched with the built prompt ' + + 'but never opened its brief, so it audited without the gaps-only ' + + 'method and the finding format it was launched to follow.', + ); }); it('an unopened chunk entry still dedups a caller relay of the same chunk', () => { @@ -1021,6 +1075,36 @@ describe('composeReview — event caps (round-7 Critical #2: caps must reach eve expect(r.body).not.toContain('the agent read its brief but never the diff'); }); + it('an idle chunk entry still dedups a caller relay of the same chunk', () => { + // The idle loop's twin of the unopened/blind chunk guards, on the + // nothing-built shape (#7012): a zero-call chunk agent keeps its + // `chunk N` subject — internal register — and the prefix dedup with it. + // Nothing is BUILT in this fixture: a built chunk brief the idle agent + // failed to open would add a same-subject unread-brief entry that + // dedups the relay on its own and hides the guard's chunk half. With + // the chunk half of the guard letting the idle entry join the + // exemption, the relayed `chunk 1 — …` line double-renders the subject. + transcript('a2', goodPrompt(2), { toolCalls: 2, range: [100, 100] }); + const p = plan({ step45: false }); + transcript('a1', goodPrompt(1), { toolCalls: 0 }); + + // Not base(): its planPath default runs coveredPlan() on the same + // path, which would relaunch a WORKING chunk-1 agent over this one. + const r = composeReview({ + criticalsInline: 0, + suggestionsInline: 0, + planPath: p, + env: ENV, + modelId: MODEL, + unreviewedDimensions: ['chunk 1 — the agent never started its walk'], + }); + expect(r.body).toContain( + 'Not reviewed: the diff section covering src/a.ts — the agent made ' + + 'no tool call: it read nothing.', + ); + expect(r.body).not.toContain('the agent never started its walk'); + }); + it('an idle rostered agent keeps its Chinese twin in a bilingual body', () => { // The rename resolves rostered names through the brief's publicLabel; // the Chinese half must say them the way the sibling floor/budget-stop @@ -1054,6 +1138,79 @@ describe('composeReview — event caps (round-7 Critical #2: caps must reach eve expect(r.body).toContain( '未审查:反向审计——该 agent 未发起任何工具调用:它什么都没读。', ); + // The floor's brief-unread gap shares the subject; its zh half rides + // the same subjectZh/reasonZh plumbing and renders its own sentence. + expect(r.body).toContain( + '未审查:反向审计——它用构建的 prompt 启动,却从未打开自己的 brief,审计时缺失了只报缺口的方法和它本应遵循的发现格式。', + ); + }); + + it('an unopened rostered agent keeps its Chinese twin in a bilingual body', () => { + // The unopened loop's subjectZh: every bilingual test before exercised + // idle, budget-gap, rewritten-launch or floor entries, never an + // unopened rostered agent — dropping the field there embeds the English + // publicLabel in the zh half beside its proper Chinese siblings. + transcript('a1', goodPrompt(1), { toolCalls: 3 }); + transcript('a2', goodPrompt(2), { toolCalls: 2 }); + const p = plan({ step45: false, han: true }); + recordBuilt(p, 1); + recordBuilt(p, 2); + recordMatrix(p); + const d = promptRecordDir(p); + const brief = briefPath(p, 'verify'); + writeFileSync(brief, 'The verify brief.'); + const launch = + `You are review agent \`verify\`.\n` + + `read_file(file_path="${brief}")\n` + + `read_file(file_path="${DIFF}", offset=0, limit=100)`; + writeFileSync(join(d, 'verify.txt'), launch); + transcript('v-verify', launch, { opens: [brief] }); + + // Not base(): same planPath-default hazard as the idle case above. + const r = composeReview({ + criticalsInline: 0, + suggestionsInline: 0, + planPath: p, + env: ENV, + modelId: MODEL, + }); + expect(r.body).toContain( + '未审查:验证——它被指向 diff 的行却从未打开:有工具调用,但没有一次读取 diff。', + ); + }); + + it('a blind rostered agent keeps its Chinese twin in a bilingual body', () => { + // The blind loop's subjectZh, sibling of the unopened case above: a + // blind YET rostered agent names the role's publicLabelZh in the zh + // half, not the English label. No working chunk-1 agent: one would + // supersede the blind entry. + transcript('a2', goodPrompt(2), { toolCalls: 2 }); + const p = plan({ step45: false, han: true }); + recordBuilt(p, 1); + recordBuilt(p, 2); + recordMatrix(p); + const d = promptRecordDir(p); + const brief = briefPath(p, 'reverse-audit'); + writeFileSync(brief, 'The reverse-audit brief.'); + const builtBlock = + `You are review agent \`reverse-audit\`.\n` + + `read_file(file_path="${brief}")`; + writeFileSync(join(d, 'reverse-audit.txt'), builtBlock); + transcript('v-ra-blind', `${builtBlock}\nYou own chunk 1 of 2.`, { + toolCalls: 0, + }); + + // Not base(): same planPath-default hazard as the idle case above. + const r = composeReview({ + criticalsInline: 0, + suggestionsInline: 0, + planPath: p, + env: ENV, + modelId: MODEL, + }); + expect(r.body).toContain( + '未审查:反向审计——启动 prompt 从未提到 diff 文件,它不可能读过 diff。', + ); }); it('a rostered budget-gap discloser keeps its Chinese twin in a bilingual body', () => { @@ -1255,6 +1412,48 @@ describe('composeReview — event caps (round-7 Critical #2: caps must reach eve ); }); + it('a rostered drifted-launch disclosure names the role in both registers', () => { + // The sibling `b === undefined` disclose site has a relay-shadow test + // and a Chinese-twin test; this site — a BUILT chunk prompt the launch + // drifted from — had neither. A rostered launch (built role prompt plus + // orchestrator chunk phrase, brief unopened) must disclose under the + // role's publicLabel with its zh twin, not the fallback plumbing line. + transcript('a2', goodPrompt(2), { toolCalls: 2, range: [100, 100] }); + const p = plan({ step45: false, han: true }); + recordBuilt(p, 1); + recordBuilt(p, 2); + recordMatrix(p); + const d = promptRecordDir(p); + const brief = briefPath(p, 'reverse-audit'); + writeFileSync(brief, 'The reverse-audit brief.'); + const rolePrompt = + `You are review agent \`reverse-audit\`.\n` + + `read_file(file_path="${brief}")\n` + + `read_file(file_path="${DIFF}", offset=0, limit=100)`; + writeFileSync(join(d, 'reverse-audit.txt'), rolePrompt); + transcript('v-ra-drifted', `${rolePrompt}\nYou own chunk 1 of 2.`, { + toolCalls: 2, + range: [0, 100], + opens: [], + }); + + // Not base(): its planPath default runs coveredPlan() again on the same + // path and would lay a verbatim reverse-audit pair over this fixture. + const r = composeReview({ + criticalsInline: 0, + suggestionsInline: 0, + planPath: p, + env: ENV, + modelId: MODEL, + }); + expect(r.body).toContain( + 'Not reviewed: reverse audit — launched with a prompt that is not the one the CLI built.', + ); + expect(r.body).toContain( + '未审查:反向审计——启动时使用的 prompt 不是 CLI 构建的那一份。', + ); + }); + it('a bare caller echo of an idle rostered subject dedups against the idle entry', () => { // The bare-echo arm (`d === e.subject`) earns its keep on the exempt // register: an idle rostered auditor plus a bare relay of its subject @@ -1455,7 +1654,12 @@ describe('composeReview — event caps (round-7 Critical #2: caps must reach eve // The round-cap marker still discloses and caps the verdict… expect(r.event).toBe('COMMENT'); expect(r.body).toContain('reverse-audit round cap of 3'); - // …but the not-built gap and its rebuild remediation are still owed. + // …but the not-built gap and its rebuild remediation are still owed — + // the gap in the posted body, not only the FIX on stderr. + expect(r.body).toContain( + 'Not reviewed: reverse audit — no auditor was launched with a prompt ' + + 'this skill builds', + ); expect(r.remediation.join(' ')).toContain('reverse audit:'); }); @@ -1526,6 +1730,9 @@ describe('composeReview — event caps (round-7 Critical #2: caps must reach eve // beside the stop — one subject, two distinct reasons, and the dedup // keeps both (the rewritten repair itself rides stderr, which is where // repairs are acted on). + expect(r.body).toContain( + 'Not reviewed: reverse audit — an auditor ran and opened its brief', + ); expect(r.remediation.join(' ')).toContain('reverse audit:'); expect(r.remediation.join(' ')).toContain('EXACTLY what it prints'); }); diff --git a/packages/cli/src/commands/review/lib/budget.test.ts b/packages/cli/src/commands/review/lib/budget.test.ts index 59e13d04f0e..c1fc68d8908 100644 --- a/packages/cli/src/commands/review/lib/budget.test.ts +++ b/packages/cli/src/commands/review/lib/budget.test.ts @@ -356,6 +356,10 @@ describe('budgetGapDisclosures — the one parser of the disclosure format', () 'Budget gap: 「none」', 'Budget gap: 『none』', 'Budget gap: 《none》', + 'Budget gap: 〈none〉', + 'Budget gap: 〔N/A〕', + 'Budget gap: [none]', + 'Budget gap: {N/A}', // Fullwidth terminal punctuation rides OUTSIDE the wrappers the same // way a halfwidth period does — `(none)。` must lose the period // before the wrapper strip can see the pair, or the phantom leaks @@ -363,9 +367,18 @@ describe('budgetGapDisclosures — the one parser of the disclosure format', () 'Budget gap: (none)。', 'Budget gap: 「none」。', 'Budget gap: 【N/A】。', + 'Budget gap: 〈none〉。', 'Budget gap: "none"。', 'Budget gap: none。', 'Budget gap: none.', + // Interrogative non-answers, fullwidth and its halfwidth twin: + // `none?` is the same non-answer, and a bare token in ANY trailing + // punctuation drops. + 'Budget gap: none?', + 'Budget gap: none?', + 'Budget gap: (none?)', + 'Budget gap: "none?"', + 'Budget gap: N/A?', // The marker itself is bilingual: the line starts at the CJK marker. '预算缺口:(none)。', // List markers from the brief's bullet format wrap placeholders the diff --git a/packages/cli/src/commands/review/lib/budget.ts b/packages/cli/src/commands/review/lib/budget.ts index c3d8c967c8a..12f1cb3969f 100644 --- a/packages/cli/src/commands/review/lib/budget.ts +++ b/packages/cli/src/commands/review/lib/budget.ts @@ -460,7 +460,7 @@ function stripWrappers(s: string): string { // marker-wrapped exactly the way it arrives bracket-wrapped. The // fullwidth terminals join for the same reason the fullwidth wrappers do: // `(none)。` must lose its period before the wrapper strip can see the pair. -const TRAILING_GAP_CHAR_RE = /[-—–*_.!…,;:。.,!?;:\s]/; +const TRAILING_GAP_CHAR_RE = /[-—–*_.!?…,;:。.,!?;:\s]/; /** Wrapping bracket/quote pairs stripped only SYMMETRICALLY. */ const GAP_WRAPPER_CLOSES: Record = { @@ -479,6 +479,10 @@ const GAP_WRAPPER_CLOSES: Record = { '「': '」', '『': '』', '《': '》', + '〈': '〉', + '〔': '〕', + '[': ']', + '{': '}', }; /**