diff --git a/packages/cli/src/commands/review/check-coverage.test.ts b/packages/cli/src/commands/review/check-coverage.test.ts index 2845e90b515..647f6dffac9 100644 --- a/packages/cli/src/commands/review/check-coverage.test.ts +++ b/packages/cli/src/commands/review/check-coverage.test.ts @@ -1607,4 +1607,53 @@ describe('verificationGaps — Step 4 and Step 5 ran, and read their briefs', () /verification — its prompt was built, but no agent was launched with it/, ); }); + + it('merges both steps into one gap when they failed the same way', () => { + // #7268: the posted body carried the verify and reverse-audit `rewritten` + // sentences back to back, near-identical but for the tail. One shape, one + // sentence, two subjects — and still both consequences and both honesty + // limits (each demonstrably RAN and opened its brief). + const p = plan(); + step45(p, 'reverse-audit', { rewritten: true }); + step45(p, 'verify', { rewritten: true }); + const r = verificationGaps(p, { postsFindings: true }, ENV); + expect(r.ok).toBe(false); + expect(r.gaps).toHaveLength(1); + const gap = r.gaps[0]; + expect(gap).toMatch(/^verification and reverse audit — /); + expect(gap).toMatch(/each ran and opened its brief/); + expect(gap).toMatch(/written by hand/); + expect(gap).toMatch(/cannot be counted as verified/); + // The remediation stays per-role: the two rebuild commands differ. + const fix = r.remediation.join(' '); + expect(fix).toContain('--role reverse-audit'); + expect(fix).toContain('--role verify'); + expect(r.unverifiedFindings).toBe(true); + }); + + it('keeps two precise gaps when the steps failed differently', () => { + // Mixed shapes have different mechanisms and different fixes; a sentence + // vague enough to cover both would misname one of them. + const p = plan(); + step45(p, 'reverse-audit', { rewritten: true }); + step45(p, 'verify', { launch: false }); + const r = verificationGaps(p, { postsFindings: true }, ENV); + expect(r.gaps).toHaveLength(2); + expect(r.gaps.join(' ')).toMatch( + /reverse audit — an auditor ran and opened its brief/, + ); + expect(r.gaps.join(' ')).toMatch( + /verification — its prompt was built, but no agent was launched with it/, + ); + expect(r.gaps.join(' ')).not.toMatch(/verification and reverse audit/); + }); + + it('does not merge when the review posts no findings — verify was never owed', () => { + // A zero-finding review with the reverse audit skipped keeps the solo + // reverse-audit text: there is no verify failure to share a sentence with. + const p = plan(); // neither step on record + const r = verificationGaps(p, { postsFindings: false }, ENV); + expect(r.gaps).toHaveLength(1); + expect(r.gaps[0]).toMatch(/^reverse audit — /); + }); }); diff --git a/packages/cli/src/commands/review/compose-review.test.ts b/packages/cli/src/commands/review/compose-review.test.ts index dbf296cdac6..e0569d5d401 100644 --- a/packages/cli/src/commands/review/compose-review.test.ts +++ b/packages/cli/src/commands/review/compose-review.test.ts @@ -1126,9 +1126,11 @@ describe('coverage is recomputed, never accepted', () => { 'a subject only the caller noticed — the auditor returned nothing twice', ], }); - // One clause for the shared subject — and it is the machine's sentence, - // not the caller's paraphrase. - expect(r.body.split(label)).toHaveLength(2); + // One clause for the shared subject — the machine's sentence, not the + // caller's paraphrase, and in the author's register: the internal + // codename stays off the posted body (it is the stderr selector). + expect(r.body.split('the whole-diff test-coverage check')).toHaveLength(2); + expect(r.body).not.toContain(label); expect(r.body).toContain('no record shows its brief reaching an agent'); expect(r.body).not.toContain('described this gap in its own words'); // A subject the coverage recomputation cannot see survives untouched. @@ -1250,7 +1252,10 @@ describe('coverage is recomputed, never accepted', () => { ], modelId: MODEL, }); - expect(r.body.split(label)).toHaveLength(2); + // The caller's echo (internal label) dedupes against the internal + // subject; the one surviving sentence prints the author's phrase. + expect(r.body).not.toContain(label); + expect(r.body.split('the whole-diff test-coverage check')).toHaveLength(2); expect( r.body.match(/no record shows its brief reaching an agent/g) ?? [], ).toHaveLength(1); @@ -1510,8 +1515,12 @@ describe('coverage is recomputed, never accepted', () => { expect(fixes).toMatch(/rewritten launches: re-run/); expect(fixes).toMatch(/unread briefs: relaunch/); expect(fixes).toMatch(/agents that never opened the diff: relaunch/); - // And none of the three disclosures drags a command into the body. + // And none of the three disclosures drags a command into the body — + // nor the unread brief's filesystem path: the path names the file an + // OPERATOR makes the agent open, and it stays on stderr with the fix. expect(r.body).not.toMatch(/agent-prompt|--roster|--chunk/); + expect(r.body).not.toContain('.brief.md'); + expect(r.body).toContain('never opened its brief, so it reviewed without'); }); it('the handler prints every FIX to stderr, before the verdict, never to stdout', () => { @@ -1630,6 +1639,29 @@ describe('the Step 4/5 gate — verify and reverse audit must have run (high eff ); }); + it('says one sentence when verify and the reverse audit failed the same way', () => { + // #7268's posted body carried the two `rewritten` sentences back to back, + // near-identical but for the tail. Both steps down the same way is one + // failure with two subjects — while the stderr remediation keeps BOTH + // rebuild commands, which differ. + const r = composeReview({ + criticalsInline: 0, + suggestionsInline: 1, + planPath: coveredPlan([]), // neither verify nor reverse audit on record + env: ENV, + modelId: MODEL, + }); + expect(r.event).toBe('COMMENT'); + expect(r.body).toMatch( + /Not reviewed: verification and reverse audit — neither the verifier nor the reverse auditor was launched with a prompt this skill builds/, + ); + expect(r.body).not.toMatch(/reverse audit — no auditor/); + expect(r.body).not.toMatch(/verification — the review posts findings/); + const fixes = r.remediation.join(' '); + expect(fixes).toContain('--role reverse-audit'); + expect(fixes).toContain('--role verify'); + }); + it('softens an unverified Request changes to Comment — no verifier, no blocker', () => { // This test used to pin the opposite: "a confirmed Critical still blocks — // a cap never softens a REQUEST_CHANGES". The never-soften rule presumes diff --git a/packages/cli/src/commands/review/compose-review.ts b/packages/cli/src/commands/review/compose-review.ts index 811d21af349..0aaa0ae46e0 100644 --- a/packages/cli/src/commands/review/compose-review.ts +++ b/packages/cli/src/commands/review/compose-review.ts @@ -207,7 +207,15 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { // The coverage-derived disclosures, kept STRUCTURAL ({subject, reason}) // from the site that knows the boundary — reparsing the rendered prose for // it was the bug. `unreviewed` above stays what the caller wrote, verbatim. - const coverageEntries: Array<{ subject: string; reason: string }> = []; + // The `public*` fields are the body's register (`Brief.publicLabel`, a + // path-free reason); `subject`/`reason` stay the internal keys every dedup + // and certification check below matches on. + const coverageEntries: Array<{ + subject: string; + reason: string; + publicSubject?: string; + publicReason?: string; + }> = []; // The fixes for the gaps above, for stderr — never for the body. The gap says // what the review cannot certify, to the PR author; the remediation names the // command that repairs it, to the orchestrator. #7012's public body was fourteen @@ -678,29 +686,38 @@ export function composeReview(input: ComposeReviewInput): ComposeReviewResult { // cause would tell the author "no agent was launched" about an agent that // demonstrably ran. const seenSubjects = new Set(); - const byReason = new Map(); - for (const { subject, reason } of covEntries) { - if (seenSubjects.has(subject)) continue; - seenSubjects.add(subject); - const subjects = byReason.get(reason) ?? []; - subjects.push(subject); - byReason.set(reason, subjects); + const byReason = new Map< + string, + Array<{ subject: string; publicSubject?: string }> + >(); + 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) ?? []; + group.push({ subject: e.subject, publicSubject: e.publicSubject }); + byReason.set(key, group); } - for (const [reason, subjects] of byReason) { + for (const [reason, entries] of byReason) { // Chunk subjects leave in the author's units, not the run's. `chunk 28` // is bookkeeping — the id selects a rebuild command on stderr, and // nothing on the PR page maps it to code. #7268's posted body enumerated // all 49 of them, unsorted, across two of these sentences; the author's // units are their files and, at the limit, the diff itself, which is what - // `describeChunkGap` renders. Role labels stay verbatim: they were - // written to be read (`roleLabel`), and reworking them here would fork - // the register the stderr twin shares. + // `describeChunkGap` renders. Role subjects ride their `publicSubject` + // (`Brief.publicLabel`) — the codename stays on stderr, where it is the + // selector — and the partition below keys on the INTERNAL subject, so a + // public phrase can never shadow a chunk id out of the chunk collapse. const chunkIds: number[] = []; const named: string[] = []; - for (const s of subjects) { - const m = /^chunk (\d+)$/.exec(s); + for (const e of entries) { + const m = /^chunk (\d+)$/.exec(e.subject); if (m) chunkIds.push(Number(m[1])); - else named.push(s); + else named.push(e.publicSubject ?? e.subject); } const shown = [ ...(chunkIds.length > 0 diff --git a/packages/cli/src/commands/review/lib/agent-briefs.ts b/packages/cli/src/commands/review/lib/agent-briefs.ts index 6d2637a5b6b..79d2e9693de 100644 --- a/packages/cli/src/commands/review/lib/agent-briefs.ts +++ b/packages/cli/src/commands/review/lib/agent-briefs.ts @@ -56,6 +56,17 @@ export type RoleId = export interface Brief { /** How the role is named to a human reading a coverage failure. */ label: string; + /** + * How the role is named in the POSTED review body — the author's register. + * + * `label` above carries the run's own codename (`Agent 1c: …`), which is the + * selector an operator acts on and means nothing on a PR page; #7550 moved + * chunk ids out of the posted body for exactly that reason, and this field + * does the same for role names: the dimension, said as what it checks. + * Distinct per role — two roles sharing a phrase would merge under one + * subject in a grouped disclosure. + */ + publicLabel: string; /** * Does a path rule belong in this agent's brief? * @@ -118,6 +129,7 @@ export interface Brief { export const BRIEFS: Record = { '0': { label: 'Agent 0: Issue fidelity & root-cause ownership', + publicLabel: 'the linked-issue fidelity pass', readsDiff: true, brief: `You are **Agent 0: Issue Fidelity & Root-Cause Ownership**. Your scope is issue fidelity, not general code review — do not report ordinary code defects; other agents own those. @@ -140,6 +152,7 @@ If \`gh\` fails (auth, rate limit, network), **retry that fetch once**. If it fa '1a': { reviewsCode: true, label: 'Agent 1a: Line-by-line correctness', + publicLabel: 'the line-by-line correctness pass', readsDiff: true, brief: `You are **Agent 1a: the line-by-line scan**. Your dimension is defined by *how you walk*, not by a topic — a topical "find correctness bugs" brief makes every agent converge on the same visibly-suspicious hunks, which is redundancy, not coverage. @@ -157,6 +170,7 @@ Scope guard: reading the enclosing function is for **context**. A defect entirel '1b': { reviewsCode: true, label: 'Agent 1b: Removed-behavior audit', + publicLabel: 'the removed-behavior audit', readsDiff: true, brief: `You are **Agent 1b: the removed-behavior audit**. You own the diff's **deleted side**, and you are the only agent who can see it: the \`-\` lines exist *only* in the diff. The post-change tree carries no trace of what was removed — the line is simply not there, and nothing marks where it was — so no agent reading the new code alone can find this class of defect. @@ -174,6 +188,7 @@ Each failure scenario must name what input or state now slips past the removed b '1c': { reviewsCode: true, label: 'Agent 1c: Cross-file tracer', + publicLabel: 'the cross-file consistency pass', readsDiff: true, brief: `You are **Agent 1c: the cross-file tracer**. You own the *whole* cross-file walk, end to end. It used to be a duty shared by six agents, and a duty shared by six agents is a duty nobody finishes while the same symbols get grepped six times. @@ -200,6 +215,7 @@ Expect the three ends to be far apart. The declaration, the pass-through, and th '2': { reviewsCode: true, label: 'Agent 2: Security', + publicLabel: 'the security pass', readsDiff: true, brief: `You are **Agent 2: Security**. Review the diff for: @@ -216,6 +232,7 @@ Expect the three ends to be far apart. The declaration, the pass-through, and th '3': { reviewsCode: true, label: 'Agent 3: Code quality', + publicLabel: 'the code-quality pass', readsDiff: true, brief: `You are **Agent 3: Code Quality**. Review the diff for: @@ -229,6 +246,7 @@ Expect the three ends to be far apart. The declaration, the pass-through, and th '4': { reviewsCode: true, label: 'Agent 4: Performance & efficiency', + publicLabel: 'the performance pass', readsDiff: true, brief: `You are **Agent 4: Performance & Efficiency**. Review the diff for: @@ -243,6 +261,7 @@ Expect the three ends to be far apart. The declaration, the pass-through, and th '5': { reviewsCode: true, label: 'Agent 5: Test coverage', + publicLabel: 'the test-coverage pass', readsDiff: true, brief: `You are **Agent 5: Test Coverage**. Review the diff for: @@ -259,6 +278,7 @@ Expect the three ends to be far apart. The declaration, the pass-through, and th '6a': { reviewsCode: true, label: 'Agent 6a: Undirected audit — attacker mindset', + publicLabel: 'the open-ended audit (attacker mindset)', readsDiff: true, brief: `You are **Agent 6a: the undirected audit, attacker mindset.** @@ -278,6 +298,7 @@ You are undirected on purpose. Do not restrict yourself to the list.`, '6b': { reviewsCode: true, label: 'Agent 6b: Undirected audit — 3 AM oncall mindset', + publicLabel: 'the open-ended audit (oncall mindset)', readsDiff: true, brief: `You are **Agent 6b: the undirected audit, 3 AM oncall mindset.** @@ -297,6 +318,7 @@ You are undirected on purpose. Do not restrict yourself to the list.`, '6c': { reviewsCode: true, label: 'Agent 6c: Undirected audit — six-months-later maintainer', + publicLabel: 'the open-ended audit (maintainer mindset)', readsDiff: true, brief: `You are **Agent 6c: the undirected audit, six-months-later maintainer mindset.** @@ -315,6 +337,7 @@ You are undirected on purpose. Do not restrict yourself to the list.`, '7': { label: 'Agent 7: Build & test verification', + publicLabel: 'the build-and-test check', readsDiff: false, brief: `You are **Agent 7: Build & Test Verification**. You do not review the diff — you run the project's own deterministic checks and report what they say. Your evidence is **the commands you ran and their output**; a return that names no command has not done this job. @@ -330,6 +353,7 @@ Use \`Source: [build]\` or \`Source: [test]\`, never \`[review]\`.`, 'test-matrix': { label: 'Test coverage matrix (whole-diff)', + publicLabel: 'the whole-diff test-coverage check', readsDiff: true, brief: `You are the **test-coverage matrix** agent — Agent 5's cross-chunk counterpart. The territory agents each see either an implementation or a test, rarely both. You see the whole diff, so you own the pairing. @@ -341,6 +365,7 @@ Use \`Source: [build]\` or \`Source: [test]\`, never \`[review]\`.`, 'invariant-a': { reviewsCode: true, label: 'Invariant agent A: state, timers, collections', + publicLabel: 'the invariant check (state, timers, collections)', readsDiff: true, brief: `You are **invariant agent A: state, timers, and collections.** @@ -358,6 +383,8 @@ Report a **Critical** for each violation, and give **both** locations that toget 'invariant-b': { reviewsCode: true, label: 'Invariant agent B: counters, return values, error taxonomies', + publicLabel: + 'the invariant check (counters, return values, error taxonomies)', readsDiff: true, brief: `You are **invariant agent B: counters, return values, and error taxonomies.** @@ -375,6 +402,7 @@ Report a **Critical** for each violation, and give **both** locations that toget 'invariant-c': { reviewsCode: true, label: 'Invariant agent C: config fields, early returns', + publicLabel: 'the invariant check (config fields, early returns)', readsDiff: true, brief: `You are **invariant agent C: config fields and early returns.** @@ -393,6 +421,7 @@ Report a **Critical** for each violation, and give **both** locations that toget output: 'verdicts', acceptsFindings: true, label: 'Verification agent', + publicLabel: 'verification', readsDiff: true, brief: `You are a **verification agent**. You do not look for new problems — you rule on the findings you were handed, listed in the message that launched you, each with a file, a line, an issue, and a **failure scenario**. The failure scenario is the finding's testable claim, and your verdict is the **result of tracing it through the real code**, not a plausibility vote on how the finding reads. @@ -422,6 +451,7 @@ Return, for each finding, one verdict: acceptsChunk: true, acceptsFindings: true, label: 'Reverse audit agent', + publicLabel: 'reverse audit', readsDiff: true, brief: `You are a **reverse audit agent**. Prior agents have already reviewed this diff and their confirmed findings are listed in the message that launched you. Your job is not to re-report them — it is to find the **gaps**: the important issues no prior agent or round caught. diff --git a/packages/cli/src/commands/review/lib/coverage.ts b/packages/cli/src/commands/review/lib/coverage.ts index 599b07cc268..306a7d15793 100644 --- a/packages/cli/src/commands/review/lib/coverage.ts +++ b/packages/cli/src/commands/review/lib/coverage.ts @@ -150,7 +150,24 @@ export interface CoverageFromTranscripts { * can carry anything), so a subject/reason boundary recovered from rendered * prose garbles exactly the entries it matters for. */ - disclosures: Array<{ subject: string; reason: string }>; + disclosures: Array<{ + subject: string; + reason: string; + /** + * The subject, said in the POSTED body's register (`Brief.publicLabel`) — + * absent when the internal subject already is that register (`chunk N` + * is translated downstream by `describeChunkGap`; `every dimension`, + * `coverage` and the Step 4/5 subjects are plain English). The internal + * `subject` stays the dedup and certification key, and the stderr twin + * keeps it: the codename is the selector an operator acts on. + */ + publicSubject?: string; + /** + * The reason for the POSTED body, when the internal one carries something + * only an operator can use — today, the unread brief's filesystem path. + */ + publicReason?: string; + }>; /** * Every planned chunk with the source files it covers, in plan order — the * body renderer's translation table. A chunk id is the run's own @@ -280,6 +297,20 @@ function roleLabel(req: RequiredAgent): string { return req.file ? `${base} — ${req.file}` : base; } +/** + * The same requirement, named for the PR author — or undefined when the + * internal label already is that register. A chunk requirement stays `chunk N` + * here on purpose: the body renderer translates chunk ids collectively + * (`describeChunkGap`), and a public subject would hide the id from that + * partition. The invariant agents' file rides `on`, not ` — `: in the posted + * sentence an em-dash reads as the subject/reason boundary. + */ +function publicRoleLabel(req: RequiredAgent): string | undefined { + if (req.role === 'chunk') return undefined; + const base = BRIEFS[req.role].publicLabel; + return req.file ? `${base} on ${req.file}` : base; +} + /** Something a reader can act on. `agentName` is `general-purpose` for all of them. */ function label(rec: AgentRecord, chunk: number | null): string { if (chunk !== null) return `chunk ${chunk}`; @@ -312,13 +343,25 @@ export function coverageFromTranscripts( const idleAgents: string[] = []; const unopenedAgents: string[] = []; const rewrittenPrompts: string[] = []; - const disclosures: Array<{ subject: string; reason: string }> = []; + const disclosures: CoverageFromTranscripts['disclosures'] = []; // The one source for both registers: the structural entry feeds the posted // body (compose-review), and the returned prose feeds the stderr arrays — // maintained as a pair, an edit to one and not the other would silently - // diverge what the operator reads from what the author was told. - const disclose = (subject: string, reason: string): string => { - disclosures.push({ subject, reason }); + // diverge what the operator reads from what the author was told. `pub` + // carries the body-register variants; the returned prose always keeps the + // internal subject and reason, because stderr is where the codename and the + // path are the things a reader acts on. + const disclose = ( + subject: string, + reason: string, + pub?: { subject?: string; reason?: string }, + ): string => { + disclosures.push({ + subject, + reason, + publicSubject: pub?.subject, + publicReason: pub?.reason, + }); return `${subject} — ${reason}`; }; const covered = new Set(); @@ -596,6 +639,7 @@ export function coverageFromTranscripts( roleLabel(req), 'no record shows its brief reaching an agent, so this dimension ' + 'was reviewed, if at all, from a prompt the run wrote for itself', + { subject: publicRoleLabel(req) }, ), ); } @@ -617,6 +661,7 @@ export function coverageFromTranscripts( 'transcript cannot certify two dimensions' : 'its prompt was built, but no agent on record was launched ' + 'with it', + { subject: publicRoleLabel(req) }, ), ); missingRoleSelectors.push(selectorOf(req)); @@ -646,11 +691,20 @@ export function coverageFromTranscripts( a.includes(JSON.stringify(brief)), ); if (!opened) { + // The brief PATH is the operator's — it names the file to make the agent + // open. The author's copy drops it: a filesystem path in a posted PR + // body is the same register leak as a chunk id. unreadBriefs.push( disclose( roleLabel(req), `never opened its brief (${brief}), so it reviewed without the ` + 'instructions it was launched to follow', + { + subject: publicRoleLabel(req), + reason: + 'never opened its brief, so it reviewed without the ' + + 'instructions it was launched to follow', + }, ), ); } @@ -839,6 +893,39 @@ const VERIFY_GAP: GapText = { }, }; +/** + * Both steps down the same way is ONE failure with two subjects, not two + * paragraphs. #7268's posted body carried the verify and reverse-audit + * `rewritten` sentences back to back, near-identical but for the tail — the + * same repetition the chunk grouping exists to kill, one layer up. Merged only + * on an EXACT shape match: mixed shapes have different mechanisms and + * different fixes, and a sentence vague enough to cover both would misname + * one of them. Each text keeps both steps' consequences and both honesty + * limits of its per-role twins: `not-built`/`not-launched` may not claim + * nobody ran, `rewritten` may not claim the brief never arrived. The + * remediation stays per-role — the two rebuild commands differ. + */ +const COMBINED_STEP45_GAP: Record, string> = { + 'not-built': + 'neither the verifier nor the reverse auditor was launched with a prompt ' + + 'this skill builds — the posted findings were ruled on, and the misses ' + + 'the rest of the review left were hunted, if at all, without the briefs ' + + 'this skill certifies against', + 'not-launched': + 'both prompts were built, but no agent was launched with either — the ' + + 'posted findings cannot be counted as verified, and the pass that hunts ' + + 'what the rest of the review missed cannot be certified', + rewritten: + 'each ran and opened its brief, but neither was launched with the prompt ' + + 'the CLI built — the launches were written by hand, so the posted ' + + 'findings cannot be counted as verified, and what the agents were ' + + 'actually asked is not what this skill certifies', + 'brief-unread': + 'each was launched with its built prompt and never opened its brief, so ' + + 'the findings were ruled on without the verdict bar, and the audit ran ' + + 'without the gaps-only method it was launched to follow', +}; + export interface VerificationReport { /** True when every required Step 4/5 agent ran and read its brief. */ ok: boolean; @@ -962,7 +1049,6 @@ export function verificationGaps( ); const reverse = bestDelivery(reverseKeys); if (reverse !== 'ok') { - gaps.push(`reverse audit — ${REVERSE_AUDIT_GAP[reverse].gap}`); // The fix template carries `--plan `; a literal `` pasted into a // POSIX shell parses as input redirection, so the one repair round Step 6 // prescribes could never run. This function is handed the real path. @@ -982,6 +1068,7 @@ export function verificationGaps( // findings, which are pre-confirmed and skip verification by design. A review that // confirmed nothing has nothing to verify. let unverifiedFindings = false; + let verify: Delivery | null = null; if (opts.postsFindings) { // The whole key family: `verify--` per shard (the record now folds // the findings in, so a launch that dropped them matches nothing), plus the @@ -989,10 +1076,9 @@ export function verificationGaps( const verifyKeys = [...built.keys()].filter( (k) => k === 'verify' || k.startsWith('verify--'), ); - const verify = bestDelivery(verifyKeys); + verify = bestDelivery(verifyKeys); if (verify !== 'ok') { unverifiedFindings = true; - gaps.push(`verification — ${VERIFY_GAP[verify].gap}`); remediation.push( `verification: ${VERIFY_GAP[verify].fix.replace( '--plan ', @@ -1004,6 +1090,24 @@ export function verificationGaps( } } + // The gaps, after both shapes are known: both steps failing the SAME way is + // one sentence with two subjects (see COMBINED_STEP45_GAP); anything else + // keeps its own precise text. The remediation above stays per-role either + // way — the two rebuild commands differ, and the combined sentence lands in + // the posted body while the fixes land on stderr. + if (reverse !== 'ok' && verify !== null && verify === reverse) { + gaps.push( + `verification and reverse audit — ${COMBINED_STEP45_GAP[reverse]}`, + ); + } else { + if (reverse !== 'ok') { + gaps.push(`reverse audit — ${REVERSE_AUDIT_GAP[reverse].gap}`); + } + if (verify !== null && verify !== 'ok') { + gaps.push(`verification — ${VERIFY_GAP[verify].gap}`); + } + } + return { ok: gaps.length === 0, gaps, remediation, unverifiedFindings }; }