fix(review): polish the two budget-gap rendering defects a live body showed

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,143 @@ 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. 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 role = 'reverse-audit';
+    const key = `${role}--round-1--abc123def456`;
+    const d = promptRecordDir(p);
+    const brief = briefPath(p, key);
+    const prompt =
+      `You are ${role}.\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[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('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--<digest>` 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 });

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

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);
   }

packages/cli/src/commands/review/lib/budget.test.ts

@@ -294,12 +294,38 @@ 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]',
+      // 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([]);
     }
   });
 
+  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