diff --git a/packages/cli/src/commands/review/agent-prompt.test.ts b/packages/cli/src/commands/review/agent-prompt.test.ts index a08e98c5d27..2362c3af1b4 100644 --- a/packages/cli/src/commands/review/agent-prompt.test.ts +++ b/packages/cli/src/commands/review/agent-prompt.test.ts @@ -1084,6 +1084,141 @@ describe('--round — the CLI bakes the round into the identity line and the key } }); + it('reads the same clock on the --chunk build gate (#9256)', () => { + // The clock argument is passed at every cap call site, but only the + // sibling paths were exercised: a mutation confined to the `--chunk` + // gate's call site survived. Same sized huge plan and both clock arms as + // the test above, driven through the per-chunk gate instead. + const dir = mkdtempSync(join(tmpdir(), 'ap-clock-chunk-')); + try { + const findings = join(dir, 'f.md'); + writeFileSync(findings, '- x'); + const handler = agentPromptCommand.handler as (a: unknown) => void; + const before = process.env[DEADLINE_ENV]; + const stderr = () => + (writeStderrLine as unknown as Mock).mock.calls + .map((c) => c[0]) + .join('\n'); + // Separate plans per arm: a successful --chunk build stamps the round's + // admission, and a stamped round's --chunk rebuilds are exempt from the + // gate — the second arm must gate against an unstamped plan of its own. + const noClockPlan = join(dir, 'huge-noclock.json'); + const withClockPlan = join(dir, 'huge-withclock.json'); + const sizedPlan = JSON.stringify({ + ...PLAN, + srcDiffLines: 5000, + diffLines: 5000, + }); + writeFileSync(noClockPlan, sizedPlan); + writeFileSync(withClockPlan, sizedPlan); + try { + // No clock: the 3B tier stands and round 4 builds chunk 13. + delete process.env[DEADLINE_ENV]; + process.exitCode = undefined; + (writeStderrLine as unknown as Mock).mockClear(); + handler({ + plan: noClockPlan, + role: 'reverse-audit', + findings, + round: 4, + chunk: 13, + }); + expect(process.exitCode).toBeUndefined(); + expect(readRecordedPrompts(noClockPlan).size).toBe(1); + + // A clock: the same round refused at the reduced tier. + process.env[DEADLINE_ENV] = String( + Math.floor(Date.now() / 1000) + 7200, + ); + process.exitCode = undefined; + (writeStderrLine as unknown as Mock).mockClear(); + handler({ + plan: withClockPlan, + role: 'reverse-audit', + findings, + round: 4, + chunk: 13, + }); + expect(process.exitCode).toBe(4); + expect(stderr()).toContain('round cap is 3'); + expect(readRecordedPrompts(withClockPlan).size).toBe(0); + } finally { + if (before === undefined) delete process.env[DEADLINE_ENV]; + else process.env[DEADLINE_ENV] = before; + } + } finally { + process.exitCode = undefined; + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('reads the same clock on the --all-chunks round gate (#9256)', () => { + // The --chunk pin above closes the per-chunk build gate only; a 3B + // round's PRIMARY admission is --all-chunks, and its gate reads the same + // expression at its own call site. Same sized huge plan and both clock + // arms, driven through the round builder instead. + const dir = mkdtempSync(join(tmpdir(), 'ap-clock-allchunks-')); + try { + const findings = join(dir, 'f.md'); + writeFileSync(findings, '- x'); + const handler = agentPromptCommand.handler as (a: unknown) => void; + const before = process.env[DEADLINE_ENV]; + const stderr = () => + (writeStderrLine as unknown as Mock).mock.calls + .map((c) => c[0]) + .join('\n'); + // Separate plans per arm: a successful build records the round's + // prompts, and the refused arm must show its own plan stayed empty. + const noClockPlan = join(dir, 'huge-noclock.json'); + const withClockPlan = join(dir, 'huge-withclock.json'); + const sizedPlan = JSON.stringify({ + ...PLAN, + srcDiffLines: 5000, + diffLines: 5000, + }); + writeFileSync(noClockPlan, sizedPlan); + writeFileSync(withClockPlan, sizedPlan); + try { + // No clock: the 3B tier stands and round 4 builds all three chunks. + delete process.env[DEADLINE_ENV]; + process.exitCode = undefined; + (writeStderrLine as unknown as Mock).mockClear(); + handler({ + plan: noClockPlan, + role: 'reverse-audit', + findings, + round: 4, + 'all-chunks': true, + }); + expect(process.exitCode).toBeUndefined(); + expect(readRecordedPrompts(noClockPlan).size).toBe(3); + + // A clock: the same round refused at the reduced tier. + process.env[DEADLINE_ENV] = String( + Math.floor(Date.now() / 1000) + 7200, + ); + process.exitCode = undefined; + (writeStderrLine as unknown as Mock).mockClear(); + handler({ + plan: withClockPlan, + role: 'reverse-audit', + findings, + round: 4, + 'all-chunks': true, + }); + expect(process.exitCode).toBe(4); + expect(stderr()).toContain('round cap is 3'); + expect(readRecordedPrompts(withClockPlan).size).toBe(0); + } finally { + if (before === undefined) delete process.env[DEADLINE_ENV]; + else process.env[DEADLINE_ENV] = before; + } + } finally { + process.exitCode = undefined; + rmSync(dir, { recursive: true, force: true }); + } + }); + it('takes the round cap from the plan’s topology on the chunkless path', () => { // 3A is the topology that actually runs this path — one auditor a round, // the whole diff — and it is the one the tier raises. Both arms use the @@ -4181,6 +4316,38 @@ describe('per-chunk retirement — cold territories stop costing a round', () => expect(out).not.toContain('next cold check round 4'); }); + it('huge cap: the retirement note reads the same clock as the gate', () => { + // The cap-3 retirement tests above STORE their cap, so the note's own + // clock read is mutation-invisible there. This plan carries no stored cap + // — the tier comes from the sized diff and the clock: without a deadline + // the huge tier is 5 and round 4's cold check fits; with one it is 3 and + // the certificate closes. Same history as the final-certificate test + // above, both clock arms. + writeFileSync( + plan, + JSON.stringify({ ...PLAN, srcDiffLines: 5000, diffLines: 5000 }), + ); + const old = new Date(2020, 0, 1); + utimesSync(plan, old, old); + answerRound(1, { 13: DRY, 14: YIELD, 15: YIELD }); + answerRound(2, { 13: DRY, 14: YIELD, 15: YIELD }); + + delete process.env[DEADLINE_ENV]; + const out = runRound(3); + expect(process.exitCode).toBeUndefined(); + expect(out).toContain('chunk 13 — retired: dry in rounds 1 and 2'); + expect(out).toContain('next cold check round 4'); + expect(out).not.toContain('certificate final'); + + process.env[DEADLINE_ENV] = String(Math.floor(Date.now() / 1000) + 7200); + const clocked = runRound(3); + expect(process.exitCode).toBeUndefined(); + expect(clocked).toContain('chunk 13 — retired: dry in rounds 1 and 2'); + expect(clocked).toContain('certificate final'); + expect(clocked).toContain('3-round cap leaves'); + expect(clocked).not.toContain('next cold check round 4'); + }); + it('huge cap: a non-converging loop is refused past the reduced 3-round cap', () => { // A huge diff caps at 3 rounds. Rounds 1-3 never converge (every chunk // keeps yielding), so round 4 is refused at the cap: exit 4, nothing diff --git a/packages/cli/src/commands/review/fetch-pr.test.ts b/packages/cli/src/commands/review/fetch-pr.test.ts index 6c52d261a17..1ad46e4b7ed 100644 --- a/packages/cli/src/commands/review/fetch-pr.test.ts +++ b/packages/cli/src/commands/review/fetch-pr.test.ts @@ -24,8 +24,11 @@ import { reviewLeaseHeldByAnotherSession, } from '../../services/review-worktree-lease.js'; import { classifyHeavy } from './lib/heavy.js'; +import { DEADLINE_ENV } from './lib/deadline.js'; +import type { MergeBaseResult } from './lib/merge-base.js'; import { buildRoleBrief } from './agent-prompt.js'; import { PARSE_ARGS_REPORT, worktreePath } from './lib/paths.js'; +import { makeDiff } from './lib/test-utils.js'; describe('classifyHeavy', () => { it('flags a substantially rewritten existing file', () => { @@ -238,7 +241,7 @@ const producerMocks = vi.hoisted(() => ({ gitOpt: vi.fn((..._args: string[]): string | null => null), gitRaw: vi.fn((..._args: string[]): Buffer => Buffer.from('')), resolveMergeBase: vi.fn( - (): { sha: string | null; baseFetchFailed: boolean } => ({ + (): MergeBaseResult => ({ sha: null, baseFetchFailed: false, }), @@ -339,10 +342,10 @@ describe('fetch-pr report assembly', () => { beforeEach(() => { vi.clearAllMocks(); // clearAllMocks resets call history but NOT implementations, so a - // mockReturnValue a prior test set on readFileSync would leak into a test - // that relies on the default. Re-assert the default (no prior report → - // ENOENT) here so every test starts from a known state regardless of - // order. + // mockReturnValue a prior test set would leak into a test that relies on + // the default. Re-assert the defaults (no prior report → ENOENT, no + // merge base → no diff) here so every test starts from a known state + // regardless of order. producerMocks.readFileSync.mockImplementation(() => { throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }); }); @@ -440,6 +443,39 @@ describe('fetch-pr report assembly', () => { expect(report.host).toBe('ghe.example.com'); }); + it('records the round cap its capture wiring writes — huge tier only with a clock (#9256)', async () => { + // plan-diff and capture-local pin this wiring in their own handlers; the + // fetch-pr side had no assertion because this harness steers the lightest + // real path (no merge base → no diff). Override the two mocks that steer + // it into a real diff instead: a resolvable merge base and a raw diff + // buffer. A handler that forgot the deadline read — or the capture-time + // tier call — would keep every budget unit test green and this one red. + producerMocks.resolveMergeBase.mockReturnValue({ + sha: 'beef0000', + baseFetchFailed: false, + }); + producerMocks.gitRaw.mockReturnValue( + Buffer.from(makeDiff('src/huge.ts', 9000)), + ); + + const before = process.env[DEADLINE_ENV]; + try { + delete process.env[DEADLINE_ENV]; + producerMocks.writeFileSync.mockClear(); + const noClock = await reportFor({}); + expect(noClock.srcDiffLines).toBeGreaterThanOrEqual(3000); + expect(noClock.budget.reverseAuditRounds).toBe(5); + + process.env[DEADLINE_ENV] = String(Math.floor(Date.now() / 1000) + 7200); + producerMocks.writeFileSync.mockClear(); + const withClock = await reportFor({}); + expect(withClock.budget.reverseAuditRounds).toBe(3); + } finally { + if (before === undefined) delete process.env[DEADLINE_ENV]; + else process.env[DEADLINE_ENV] = before; + } + }); + // The lease is also a lock (#9205): a concurrent same-PR fetch-pr used to // stale-clean the holder's worktree before failing on, destroying it. The // refusal must precede every destructive step, including the lease write. diff --git a/packages/cli/src/commands/review/lib/report.test.ts b/packages/cli/src/commands/review/lib/report.test.ts index 35c4eb2c202..94ab30e8c75 100644 --- a/packages/cli/src/commands/review/lib/report.test.ts +++ b/packages/cli/src/commands/review/lib/report.test.ts @@ -7,28 +7,7 @@ import { describe, it, expect } from 'vitest'; import { buildDiffPlan } from './diff-plan.js'; import { buildPlanReport, stringifyPlanReport } from './report.js'; - -/** A diff adding `n` lines to `path`, shaped so the planner can cut it. */ -function addFile(path: string, n: number): string { - const body: string[] = []; - while (body.length < n) { - body.push(`+function f${body.length}() {`); - for (let k = 0; k < 8 && body.length < n; k++) { - body.push(`+ const x = ${k};`); - } - body.push('+}'); - body.push('+'); - } - body.length = n; - return [ - `diff --git a/${path} b/${path}`, - '--- /dev/null', - `+++ b/${path}`, - `@@ -0,0 +1,${n} @@`, - ...body, - '', - ].join('\n'); -} +import { makeDiff } from './test-utils.js'; /** A diff that edits an existing file: `ctx` context lines then `add` new ones. */ function editFile(path: string, ctx: number, add: number): string { @@ -73,7 +52,7 @@ describe('buildPlanReport', () => { it('treats a null resolver as "no tree to read", so nothing is heavy', () => { // `plan-diff` has a bare diff file and no ref. It must not guess. - const plan = buildDiffPlan(addFile('src/big.ts', 2000), 400); + const plan = buildDiffPlan(makeDiff('src/big.ts', 2000), 400); const report = buildPlanReport(plan, null, {}); expect(report.files[0].fileLines).toBe(0); expect(report.files[0].preLines).toBe(0); @@ -102,7 +81,8 @@ describe('buildPlanReport', () => { }); it('emits addedRanges only on heavy files', () => { - const diff = editFile('src/heavy.ts', 3, 900) + addFile('src/light.ts', 20); + const diff = + editFile('src/heavy.ts', 3, 900) + makeDiff('src/light.ts', 20); const report = buildPlanReport( buildDiffPlan(diff, 400), (p) => (p === 'src/heavy.ts' ? 6000 : 30), @@ -151,7 +131,7 @@ describe('buildPlanReport', () => { it('withholds the diff range from files no invariant agent will read', () => { const report = buildPlanReport( - buildDiffPlan(addFile('src/a.ts', 20), 400), + buildDiffPlan(makeDiff('src/a.ts', 20), 400), () => 30, {}, ); @@ -161,10 +141,10 @@ describe('buildPlanReport', () => { it('carries the per-kind topology counts through unchanged', () => { const diff = - addFile('src/a.ts', 10) + - addFile('src/a.test.ts', 20) + - addFile('docs/g.md', 30) + - addFile('package-lock.json', 40); + makeDiff('src/a.ts', 10) + + makeDiff('src/a.test.ts', 20) + + makeDiff('docs/g.md', 30) + + makeDiff('package-lock.json', 40); const plan = buildDiffPlan(diff, 400); const report = buildPlanReport(plan, () => 100, {}); expect(report.srcDiffLines).toBe(plan.srcDiffLines); @@ -177,7 +157,8 @@ describe('buildPlanReport', () => { describe('stringifyPlanReport', () => { it('round-trips: the collapsed text parses back to the same object', () => { - const diff = editFile('src/heavy.ts', 3, 900) + addFile('src/light.ts', 20); + const diff = + editFile('src/heavy.ts', 3, 900) + makeDiff('src/light.ts', 20); const report = buildPlanReport( buildDiffPlan(diff, 400), (p) => (p === 'src/heavy.ts' ? 6000 : 30), diff --git a/packages/cli/src/commands/review/lib/test-utils.ts b/packages/cli/src/commands/review/lib/test-utils.ts index 2b29ca9e986..91cea1cbb56 100644 --- a/packages/cli/src/commands/review/lib/test-utils.ts +++ b/packages/cli/src/commands/review/lib/test-utils.ts @@ -57,6 +57,30 @@ export function seedParseArgs(dir: string, effort: unknown): void { ); } +/** + * A diff adding `n` lines to a new file, shaped like real source: top-level + * declarations separated by blank lines, so the planner has somewhere to cut. + */ +export function makeDiff(path: string, n: number): string { + const body: string[] = []; + while (body.length < n) { + body.push(`+function f${body.length}() {`); + for (let k = 0; k < 8 && body.length < n; k++) + body.push(`+ const x = ${k};`); + body.push('+}'); + body.push('+'); + } + body.length = n; + return [ + `diff --git a/${path} b/${path}`, + '--- /dev/null', + `+++ b/${path}`, + `@@ -0,0 +1,${n} @@`, + ...body, + '', + ].join('\n'); +} + /** * The fs calls the fixture builders make. Callers hand over their own * bindings: the parse-args suite mocks `node:fs` for the whole file, so diff --git a/packages/cli/src/commands/review/plan-diff.test.ts b/packages/cli/src/commands/review/plan-diff.test.ts index c82a29a52cf..ade8713008f 100644 --- a/packages/cli/src/commands/review/plan-diff.test.ts +++ b/packages/cli/src/commands/review/plan-diff.test.ts @@ -16,7 +16,7 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { planDiffCommand } from './plan-diff.js'; import { chunksCoverDiff } from './lib/diff-plan.js'; -import { seedParseArgs } from './lib/test-utils.js'; +import { makeDiff, seedParseArgs } from './lib/test-utils.js'; import { DEADLINE_ENV } from './lib/deadline.js'; let dir: string; @@ -43,30 +43,6 @@ afterEach(() => { if (dir) rmSync(dir, { recursive: true, force: true }); }); -/** - * A diff adding `n` lines to a new file, shaped like real source: top-level - * declarations separated by blank lines, so the planner has somewhere to cut. - */ -function makeDiff(path: string, n: number): string { - const body: string[] = []; - while (body.length < n) { - body.push(`+function f${body.length}() {`); - for (let k = 0; k < 8 && body.length < n; k++) - body.push(`+ const x = ${k};`); - body.push('+}'); - body.push('+'); - } - body.length = n; - return [ - `diff --git a/${path} b/${path}`, - '--- /dev/null', - `+++ b/${path}`, - `@@ -0,0 +1,${n} @@`, - ...body, - '', - ].join('\n'); -} - describe('plan-diff — the round cap the handler actually records', () => { // The capture handlers are where the two machine facts enter a plan, and // until now nothing exercised that wiring: the budget unit tests pass the diff --git a/packages/core/src/skills/bundled/review/SKILL.md b/packages/core/src/skills/bundled/review/SKILL.md index dcdbfbde534..22ce8e33afa 100644 --- a/packages/core/src/skills/bundled/review/SKILL.md +++ b/packages/core/src/skills/bundled/review/SKILL.md @@ -635,7 +635,7 @@ After deduplication, run reverse audit **iteratively** — the first launch ride - **Small diffs (Step 3A path):** one reverse audit agent per round, reading the whole diff — except rounds 1 and 2, which are **the convergence pair** and launch together (below). - **Large diffs (Step 3B path):** one reverse audit agent **per chunk** per round, launched together in a single response — and rounds 1 and 2 are **the convergence pair** here too, their per-chunk auditors launched together (below). A single agent asked to re-read a 5 800-line diff with a growing finding list appended is the most context-starved agent in the pipeline — precisely on the PRs where the reverse audit matters most. Each per-chunk auditor gets the same territory as its Step 3B counterpart, plus the cumulative finding list for the **whole** diff (so it knows what is already covered elsewhere). -- **The builder schedules the 3B fan-out; you do not.** Rounds 1 and 2 audit every chunk — they are what establishes each territory's record. From round 3 on, `--all-chunks` reads the harness transcripts and **retires** any chunk whose own last two audits were substantively dry (the receipt named what it examined AND the transcript shows the diff was opened): a retired chunk is cold-checked on alternating rounds instead of every round, and a cold check that yields anything returns it to every-round auditing. The savings land on the odd rounds — every retired chunk cold-checks together on the even ones, so an even round's fan-out is unchanged; expect the odd rounds to shrink, not the even ones (under the 3-round huge-diff cap only round 3 can shrink — the cap ends the loop before round 5). The blocks it prints are the round; the `retirement:` note after the `end of round` line names each skipped chunk and its certificate — relay that note in your narration, and do not hand-build an auditor for a chunk the builder skipped. Why, measured: on a real 6-chunk run, two chunks were dry in **all five rounds** — a third of the loop's auditors re-certifying territories that had already converged, while the three hot chunks were where every finding came from. Attention follows evidence; the certificate a retired chunk holds (two consecutive substantive dry audits) is exactly the one the whole loop used to end on. +- **The builder schedules the 3B fan-out; you do not.** Rounds 1 and 2 audit every chunk — they are what establishes each territory's record. From round 3 on, `--all-chunks` reads the harness transcripts and **retires** any chunk whose own last two audits were substantively dry (the receipt named what it examined AND the transcript shows the diff was opened): a retired chunk is cold-checked on alternating rounds instead of every round, and a cold check that yields anything returns it to every-round auditing. The savings land on the odd rounds — every retired chunk cold-checks together on the even ones, so an even round's fan-out is unchanged; expect the odd rounds to shrink, not the even ones (under the 3-round huge-diff cap — the reduction a run earns only when it has a deadline — only round 3 can shrink, because the cap ends the loop before round 5). The blocks it prints are the round; the `retirement:` note after the `end of round` line names each skipped chunk and its certificate — relay that note in your narration, and do not hand-build an auditor for a chunk the builder skipped. Why, measured: on a real 6-chunk run, two chunks were dry in **all five rounds** — a third of the loop's auditors re-certifying territories that had already converged, while the three hot chunks were where every finding came from. Attention follows evidence; the certificate a retired chunk holds (two consecutive substantive dry audits) is exactly the one the whole loop used to end on. One anomaly the builder flags but does not refuse (#9242): a per-chunk build on a plan whose own `srcDiffLines`/`diffLines` say Step 3A prints a stderr note — the plan's numbers price one whole-diff auditor per round (the reverse-audit round cap reads them), yet per-chunk auditors were built. It fires on `--all-chunks` and on a `--chunk` build of a round that has no admission stamp yet; a stamped round's `--chunk` rebuilds are exempt — their fan-out was ruled on at admission. If the note fires and the fan-out is deliberate — you decided against the plan's numbers (the routing is yours, as Step 1 says), or this is a whole-round `--all-chunks` rebuild of an already-admitted round on a hand-maintained plan — say so in the round; if it was not deliberate, stop and re-derive the topology from Step 1 instead of spending a fan-out the plan never owed. @@ -690,7 +690,7 @@ The brief holds what the auditor is for: hunt only the **gaps** no prior agent c - **On the 3B path the builder is also the convergence ledger**: when every chunk holds two consecutive substantive dry audits and none is due a cold check, `--all-chunks` builds nothing, prints a `CONVERGED` explanation to stderr and exits **5**. Stop the loop and proceed to Step 6 — this is a **clean** convergence, not a gap: no `unreviewedDimensions` entry is owed, because each chunk holds the two-dry rule's evidence chunk by chunk — two consecutive dry **audits**, though not necessarily in consecutive rounds (a chunk dry in rounds 1 and 2 skips round 3 and cold-checks dry in round 4, holding rounds 2 and 4). If an earlier round-cap or budget refusal told you to add its stop entry to `unreviewedDimensions`, remove it now — this convergence supersedes that stop (the marker on disk is cleared the same way). Exit 5 is mainly the CLI enforcing the stop the two-dry-rounds rule above used to leave to orchestrator discretion; the new savings are the odd-round skips and a convergence at the cap round (round 5 on a 3B diff, round 3 under the huge-diff cap when the run has a deadline and round 5 when it does not — this ledger is 3B's, so the 3A tier's ten never applies here). (It cannot owe a verification launch: a reporting round makes its chunk hot, so every verifier launched with a later round that did run.) - Stop at the plan's **`reverseAuditRounds` cap** — 10 on a 3A diff, 5 on a 3B one, and 3 for a huge diff (effective ≥ 3000 lines) **when the run has a deadline**, 5 when it does not (the huge reduction answers a six-hour ceiling, so it applies only where there is one) — and say so in the output rather than implying convergence. The cap is per topology because it prices a round, and a 3A round is one auditor where a huge-diff round is ~90 minutes; you never work this out yourself, the builder reads the plan's tier. The builder enforces this itself: a round past the cap gets a `ROUND CAP:` refusal on stderr and exit **4**, and — like the time-budget gate — writes a marker `compose-review` caps the verdict on whether or not you relay anything; still add the entry the message names to `unreviewedDimensions` so the terminal report agrees. If the cap round reported findings, its verifiers have NOT launched — that launch rides the next round's build, which the cap forbids — so verify them before Step 6 through `agent-prompt --role verify` **only** (never a hand-rolled agent), under the same bounded tail as the budget stop below: that builder is gated on the compose floor and refuses once too little time remains, and when the deadline is within the floor you stop waiting on any verifier batch still out and compose with the tags in hand — no fresh re-verification pass, and nothing already confirmed re-verified. This matters most on exactly the huge diffs the cap targets: a time-budgeted CI run that stops at the cap with ~30-90 minutes left must not spend it on an unbounded tail and die before compose. The tag backstop below (and `compose-review`'s machine-read of it) is what catches a miss. - Findings **reported** by each round are merged into the cumulative list **before** the next round begins, so each round sees an updated baseline. **The merge runs unconditionally — before every round build and before Step 6, whether or not the previous round reported findings**: under the pipelined loop below, round _k_'s verdicts land during round _k+1_, and every termination mode (two dry rounds, CONVERGED, budget stop, the round cap) can arrive with the final rounds dry — a merge keyed to "some round reported something" would never apply the last verdicts that landed. Each merge applies every Step 4 verdict that has landed: confirmed removes the tag, rejected removes the entry. Verification status does not gate the merge — the list exists so auditors do not re-report what is already filed, and an unverified entry serves that purpose exactly as well as a confirmed one. The trade, named: an entry a verifier later rejects will have suppressed one round of rediscovery in its neighbourhood — the window is one round in one location, and the plan's round cap still bounds the loop. The tag is what keeps this mechanical rather than remembered: an entry enters the list tagged `— [unverified]`; the merge after its Step 4 verdict removes the tag (confirmed) or the entry (rejected). Step 6's confirmed-only read then has something to key on — anything still tagged is left out of the confirmed set — instead of a memory of which round each entry arrived in. The tag rides inside the findings file, which is hashed into the record key and copied to the digest-named list file each block points at — so a launch that drops the pointer matches no record, and the delivery floor counts the agent's read of that file exactly as it counts the brief's. -- **A reporting round whose every finding the verifier rejected is retroactively dry.** The merge already removes a rejected entry from the cumulative list; from the merge that applies the last of a round's rejections, the round also stops counting as a reporting round, and the two-consecutive-dry rule reads rounds' **effective** status. Rejected means rejected — an entry confirmed at low confidence keeps its round a reporting round. Under the pipelined loop a round's verdicts land while the next round runs, so the upgrade usually arrives one round late, and that is still one round saved: a measured run held round 2 dry, watched round 3's sole finding be rejected, and then ran rounds 4 **and 5** — round 4's dry return plus the rejection already in hand was the two-dry evidence, and the fifth round audited nothing the loop had not already answered (measured; DESIGN.md — The rounds a rejected finding bought (PR #8353)). The rule leans on the rejection bar the verifier's brief already enforces — a rejection claims direct counter-evidence, never mere unverifiability — so a round retired by rejections is retired on evidence, not on doubt. **It pairs forward only, and is consulted when a round returns**: on round _k_'s dry return, first apply every verdict that has landed (the unconditional merge — the retirement takes effect at this application, not at some earlier moment), then end the loop if round _k−1_ was dry or is now retired. Round _k−1_ counts **launches, not labels**: the convergence pair is one round here — a pair member is never round _k−1_ on its own (the pair bullet's not-carried-forward rule stands), and a reporting pair retires only when every finding from **both** members is rejected. The upgrade never ends the loop by itself — a preceding dry round plus a freshly-retired round stops nothing while the next round is already in flight: that round was launched, and its return is taken whatever it says, because a launched auditor can be carrying a real Critical. This is the measured shape (round 4's return is where the loop closes under this rule — the measured run, which predates it, ran a fifth round; a cap-5 shape — under the 3-round huge-diff tier the upgrade can only ever retire rounds 1–2, since the cap round's verdicts land during its solo verification, after the loop has already ended) and the only pairing licensed here. It softens nothing else: a whiffed scope stays not-audited whatever the verdicts say, and on 3B the retirement ledger's per-chunk certificates are untouched — this rule reads at the level the round counter reads. +- **A reporting round whose every finding the verifier rejected is retroactively dry.** The merge already removes a rejected entry from the cumulative list; from the merge that applies the last of a round's rejections, the round also stops counting as a reporting round, and the two-consecutive-dry rule reads rounds' **effective** status. Rejected means rejected — an entry confirmed at low confidence keeps its round a reporting round. Under the pipelined loop a round's verdicts land while the next round runs, so the upgrade usually arrives one round late, and that is still one round saved: a measured run held round 2 dry, watched round 3's sole finding be rejected, and then ran rounds 4 **and 5** — round 4's dry return plus the rejection already in hand was the two-dry evidence, and the fifth round audited nothing the loop had not already answered (measured; DESIGN.md — The rounds a rejected finding bought (PR #8353)). The rule leans on the rejection bar the verifier's brief already enforces — a rejection claims direct counter-evidence, never mere unverifiability — so a round retired by rejections is retired on evidence, not on doubt. **It pairs forward only, and is consulted when a round returns**: on round _k_'s dry return, first apply every verdict that has landed (the unconditional merge — the retirement takes effect at this application, not at some earlier moment), then end the loop if round _k−1_ was dry or is now retired. Round _k−1_ counts **launches, not labels**: the convergence pair is one round here — a pair member is never round _k−1_ on its own (the pair bullet's not-carried-forward rule stands), and a reporting pair retires only when every finding from **both** members is rejected. The upgrade never ends the loop by itself — a preceding dry round plus a freshly-retired round stops nothing while the next round is already in flight: that round was launched, and its return is taken whatever it says, because a launched auditor can be carrying a real Critical. This is the measured shape (round 4's return is where the loop closes under this rule — the measured run, which predates it, ran a fifth round; a cap-5 shape — under the 3-round huge-diff tier, which a run only gets when it has a deadline, the upgrade can only ever retire rounds 1–2, since the cap round's verdicts land during its solo verification, after the loop has already ended) and the only pairing licensed here. It softens nothing else: a whiffed scope stays not-audited whatever the verdicts say, and on 3B the retirement ledger's per-chunk certificates are untouched — this rule reads at the level the round counter reads. - **Verification rides alongside the next round, not ahead of it.** When round _k_ returns with new findings, one response launches BOTH round _k_'s verifiers (Step 4, `--role verify --round k` with that round's new findings) AND round _k+1_'s auditors — build the two prompt sets first, then fire every agent together, exactly as Step 3 fans out. (Step 4's initial verification is the k=0 case of the same rule: its shards ride with the first reverse-audit launch — the convergence pair, whole-diff on 3A and per-chunk rounds 1 and 2 on 3B. The convergence pair is the one exception on the LAUNCH side: a pair member's return never triggers this rule per member — round 2's auditors are already in flight — and the pair bullets above define the one transition; the pair's findings still verify as the k=2 case, riding round 3.) The serial shape (audit → wait for verification → next round) spent 5-8 minutes per round waiting for verifiers whose results the next round's auditors never needed. Two orderings still hold: the **last** round's verification must complete before Step 6 (that ordering is what keeps unverified entries out of the report and the PR, backed by the tag backstop at the end of this step — which `compose-review` machine-checks from `findingsPath`, Step 6), and a rejected finding leaves the cumulative list at the next merge. - **The round builder is also the loop's clock.** In a time-budgeted run (CI exports `QWEN_REVIEW_DEADLINE_EPOCH`; a local run normally has no deadline and is untouched), `agent-prompt --role reverse-audit` refuses to build a round that no longer fits: the remaining time must cover **the round itself** (estimated from the costliest round's measured cost so far — a repair relaunch can make one round the expensive one, and the gate prices the worst case the run has proved, not the newest dip — or a conservative constant for round 1) **plus** the reserve kept for its verification, compose-review and submission. On refusal it prints a `BUDGET:` line to stderr and exits **4**. That refusal is a termination rule, not an error — do not rebuild the round, do not relaunch auditors, and do not retry the command. The builder also records a budget-stop marker that `compose-review` reads directly, so the verdict is capped whether or not you relay anything; still add the exact entry the message names (`reverse audit — stopped before round by the review time budget`) to `unreviewedDimensions` so the terminal report and the body agree, and proceed to Step 6. **The tail after a budget stop is bounded, and its order is load-bearing.** Verify the last round's findings — the ones whose verifiers would have ridden the round the gate just refused — **only through `agent-prompt --role verify`, never a hand-rolled `agent`**: that builder is gated on a **compose floor** and prints a `VERIFY BUDGET:` refusal (exit 4) once too little time remains, at which point you stop verifying and compose **immediately** — findings still carrying `— [unverified]` keep the tag, and `compose-review` caps the verdict on it and never treats an unverified finding as a confirmed blocker; everything earlier rounds confirmed still posts. **Bound the wait, not just the launch:** the builder gate stops a verifier from being _built_ below the floor, but a verifier admitted _above_ it can still run a real filesystem/git E2E workload past the floor while you wait on its batch — and `agent-prompt` builds prompts, it cannot cancel a running agent. So when the deadline is within the compose floor and a verifier batch has not returned, **stop waiting on it yourself**: take the findings in hand at their current tag and compose. A verifier you stopped waiting on leaves its findings `— [unverified]`, which caps the verdict exactly as a refused build would. Do **not** re-verify findings already confirmed in earlier rounds, and do **not** invent a fresh re-verification pass — that is the unbounded work a wall runs into. Compose and submit are non-negotiable; they always run. Why this exists, measured twice: a +1699-line PR's CI review ran the audit loop to the 5-round cap and was killed while round 5's findings were still being verified (#8368); and a 4,269-line cross-worktree git guard stopped the audit correctly with ~110 minutes left, then a single hand-rolled agent re-running a 15-family shell/git bypass battery with real filesystem E2E consumed all of it — the wall hit mid-verification, compose never ran, and ~20 E2E-confirmed Critical bypasses were never posted (measured; DESIGN.md — The killed-before-compose tail (PR #8687)). A review that stops on the budget still reports everything it proved; one that runs past it reports nothing.