Skip to content
147 changes: 147 additions & 0 deletions packages/cli/src/commands/review/agent-prompt.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3424,6 +3424,153 @@ describe('per-chunk retirement — cold territories stop costing a round', () =>
expect(out).not.toContain('retirement:');
});

it('huge cap: a chunk dry in rounds 1 and 2 retires with a final certificate', () => {
// Under the reduced 3-round cap, chunk 13's next cold check (round 4) is
// past the cap, so the retirement note must read `certificate final`, not
// `next cold check round 4` — the same builder's admission gate refuses a
// round-4 build. Pins the plan-cap comparison (`nextColdCheck >
// planRoundCap`) at cap 3; the only other cap-3 test keeps every chunk
// yielding, so nothing retires there.
writeFileSync(
plan,
JSON.stringify({ ...PLAN, budget: { reverseAuditRounds: 3 } }),
);
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 });
const out = runRound(3);

expect(process.exitCode).toBeUndefined();
expect(out).toContain('2 auditors required this round');
expect(out).toContain('chunk 13 — retired: dry in rounds 1 and 2');
expect(out).toContain('certificate final');
expect(out).not.toContain('next cold check round 4');
Comment thread
wenshao marked this conversation as resolved.
});

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
// built, and — the robustness half — a marker compose-review caps on,
// so the verdict is capped whether or not the orchestrator relays.
writeFileSync(
plan,
JSON.stringify({ ...PLAN, budget: { reverseAuditRounds: 3 } }),
);
const old = new Date(2020, 0, 1);
utimesSync(plan, old, old);
answerRound(1, { 13: YIELD, 14: YIELD, 15: YIELD });
answerRound(2, { 13: YIELD, 14: YIELD, 15: YIELD });
answerRound(3, { 13: YIELD, 14: YIELD, 15: YIELD });
const out = runRound(4);

expect(process.exitCode).toBe(4);
expect(out).toBe('');
expect(keysOf(4)).toHaveLength(0);
const msg = (writeStderrLine as unknown as Mock).mock.calls
.map((c) => c[0])
.join('\n');
expect(msg).toContain('ROUND CAP');
expect(msg).toContain('round cap is 3');
// The marker is on disk so compose-review caps without the relay.
expect(readBudgetStop(plan)?.cause).toBe('round-cap');
expect(readBudgetStop(plan)?.cap).toBe(3);
});

it('huge cap: a --chunk build past the cap is refused too — the per-chunk gate', () => {
// The round-cap gate must fire on the per-chunk call site, not only
// through --all-chunks: a huge-diff review whose rounds are built or
// repaired per chunk would otherwise admit round 4+ against the cap and
// run ~90-minute rounds in the exact timeout band this cap sheds. Rounds
// 1-3 are built (non-converging), then a `--chunk 13 --round 4` build —
// an unadmitted round, so its first chunk build IS the round's admission
// — must be refused at the cap, writing the round-cap marker.
writeFileSync(
plan,
JSON.stringify({ ...PLAN, budget: { reverseAuditRounds: 3 } }),
);
const old = new Date(2020, 0, 1);
utimesSync(plan, old, old);
answerRound(1, { 13: YIELD, 14: YIELD, 15: YIELD });
answerRound(2, { 13: YIELD, 14: YIELD, 15: YIELD });
answerRound(3, { 13: YIELD, 14: YIELD, 15: YIELD });

(writeStdoutLine as unknown as Mock).mockClear();
(writeStderrLine as unknown as Mock).mockClear();
(agentPromptCommand.handler as (a: unknown) => void)({
plan,
role: 'reverse-audit',
findings,
chunk: 13,
round: 4,
});

expect(process.exitCode).toBe(4);
expect((writeStdoutLine as unknown as Mock).mock.calls).toHaveLength(0);
expect(keysOf(4)).toHaveLength(0);
const msg = (writeStderrLine as unknown as Mock).mock.calls
.map((c) => c[0])
.join('\n');
expect(msg).toContain('ROUND CAP');
expect(msg).toContain('round cap is 3');
expect(readBudgetStop(plan)?.cause).toBe('round-cap');
expect(readBudgetStop(plan)?.cap).toBe(3);
// The refusal precedes admission — no round-4 stamp is left behind.
expect(readRoundStamps(plan).some((s) => s.round === 4)).toBe(false);
});

it('huge cap: a chunkless single build past the cap is refused too — the 3A gate', () => {
// The chunkless whole-diff gate (Step 5's 3A single auditor) is the third
// call site the cap passes through. No history is needed — round 4 > cap
// 3 alone refuses it, exit 4 with the round-cap marker.
writeFileSync(
plan,
JSON.stringify({ ...PLAN, budget: { reverseAuditRounds: 3 } }),
);
const old = new Date(2020, 0, 1);
utimesSync(plan, old, old);

(writeStdoutLine as unknown as Mock).mockClear();
(writeStderrLine as unknown as Mock).mockClear();
(agentPromptCommand.handler as (a: unknown) => void)({
plan,
role: 'reverse-audit',
findings,
round: 4,
});

expect(process.exitCode).toBe(4);
expect((writeStdoutLine as unknown as Mock).mock.calls).toHaveLength(0);
expect(readRecordedPrompts(plan).size).toBe(0);
const msg = (writeStderrLine as unknown as Mock).mock.calls
.map((c) => c[0])
.join('\n');
expect(msg).toContain('ROUND CAP');
expect(msg).toContain('round cap is 3');
expect(readBudgetStop(plan)?.cause).toBe('round-cap');
expect(readBudgetStop(plan)?.cap).toBe(3);
});

it('the default 5-round cap is enforced by the builder, not just prose', () => {
// Pins the general ROUND CAP enforcement: the mutation `round > cap`
// → `round > cap && cap === 1` (a sixth round builds) fails here.
answerRound(1, { 13: YIELD, 14: YIELD, 15: YIELD });
answerRound(2, { 13: YIELD, 14: YIELD, 15: YIELD });
answerRound(3, { 13: YIELD, 14: YIELD, 15: YIELD });
answerRound(4, { 13: YIELD, 14: YIELD, 15: YIELD });
answerRound(5, { 13: YIELD, 14: YIELD, 15: YIELD });
const out = runRound(6);

expect(process.exitCode).toBe(4);
expect(out).toBe('');
expect(keysOf(6)).toHaveLength(0);
const msg = (writeStderrLine as unknown as Mock).mock.calls
.map((c) => c[0])
.join('\n');
expect(msg).toContain('ROUND CAP');
expect(msg).toContain('round cap is 5');
});

it('all retired and none due: exit 5, CONVERGED, nothing built, nothing stamped', () => {
answerRound(1, { 13: DRY, 14: DRY, 15: DRY });
answerRound(2, { 13: DRY, 14: DRY, 15: DRY });
Expand Down
78 changes: 65 additions & 13 deletions packages/cli/src/commands/review/agent-prompt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,18 @@ import { createHash } from 'node:crypto';
import { readFileSync } from 'node:fs';
import { dirname, join, resolve } from 'node:path';
import { writeStdoutLine, writeStderrLine } from '../../utils/stdioHelpers.js';
import { launchToolBudget } from './lib/budget.js';
import { launchToolBudget, reverseAuditRoundCap } from './lib/budget.js';
import {
expectedRoundSeconds,
readRoundStamps,
reverseAuditBudgetExhausted,
reverseAuditBudgetMessage,
roundCapStopEntry,
stampRound,
verifyBudgetExhausted,
verifyBudgetMessage,
writeBudgetStop,
writeRoundCapStop,
} from './lib/deadline.js';
import {
READ_FILE_CHAR_CAP,
Expand All @@ -64,7 +66,6 @@ import {
writeFindingsFile,
} from './lib/prompt-record.js';
import {
REVERSE_AUDIT_MAX_ROUNDS,
scheduleReverseAuditRound,
type RoundSchedule,
} from './lib/retirement.js';
Expand Down Expand Up @@ -1865,7 +1866,31 @@ function requireAuditableChunks(report: PlanReport): DiffChunk[] {
function admitReverseAuditRound(
planPath: string,
round: number | undefined,
cap: number,
): boolean {
// The plan's round cap first: deterministic, and cheaper than the
// deadline arithmetic. The full cap normally; a reduced cap for a huge
// diff, where a single reverse-audit round is ~90 minutes and the full
// loop cannot finish (measured: the 6-hour CI reviews that posted nothing
// were 4,000-5,300-line PRs). A round past the cap writes a marker so
// `compose-review` caps the verdict whether or not the orchestrator
// relays the entry — the round cap runs its full allotted rounds, so a
// non-converged stop here is real unaudited scope, exactly as the budget
// stop is.
if (typeof round === 'number' && round > cap) {
Comment thread
wenshao marked this conversation as resolved.
writeRoundCapStop(planPath, cap, round);
Comment thread
wenshao marked this conversation as resolved.
Comment thread
wenshao marked this conversation as resolved.
Comment thread
wenshao marked this conversation as resolved.
writeStderrLine(
`ROUND CAP: the plan's reverse-audit round cap is ${cap}, and round ` +
`${round} is past it. This is a termination rule, not an error — do ` +
`not rebuild or retry. A marker has been recorded and compose-review ` +
`will cap the verdict; still add \`${roundCapStopEntry(cap)}\` to ` +
`unreviewedDimensions so the terminal report agrees. If the cap ` +
`round reported findings whose verdicts have not landed, launch ` +
`their verifiers alone and wait, then proceed to Step 6.`,
);
process.exitCode = 4;
return false;
}
const spent = reverseAuditBudgetExhausted(
process.env,
expectedRoundSeconds(planPath, round),
Expand Down Expand Up @@ -1915,7 +1940,14 @@ function runAllChunks(
// three yielded in most: the loop earns its keep in the hot territories,
// and the cold ones were a third of its bill.
let schedule: RoundSchedule | null = null;
if (role === 'reverse-audit' && round !== undefined && round >= 3) {
// Retirement needs two consecutive dry audits, so nothing retires before
// round 3 (the scheduler's own guard says the same).
const retirementReadsFrom = 3;
if (
role === 'reverse-audit' &&
round !== undefined &&
round >= retirementReadsFrom
) {
try {
schedule = scheduleReverseAuditRound(
planPath,
Expand Down Expand Up @@ -1950,7 +1982,14 @@ function runAllChunks(
// that builds is an admission and stamps like any other; the converged
// round above built nothing and stamps nothing; a build that throws
// leaves no stamp for the next round's gate to misprice.
if (role === 'reverse-audit' && !admitReverseAuditRound(planPath, round)) {
if (
role === 'reverse-audit' &&
!admitReverseAuditRound(
planPath,
round,
reverseAuditRoundCap(report.budget),
)
) {
Comment thread
wenshao marked this conversation as resolved.
return;
}

Expand Down Expand Up @@ -1999,6 +2038,7 @@ function runAllChunks(
: `one per chunk still under audit (${skipped.length} retired ` +
`chunk(s) skipped; the retirement note after the end-of-round line ` +
`says which — relay it to the terminal)`;
const planRoundCap = reverseAuditRoundCap(report.budget);
const retirementNote =
Comment thread
wenshao marked this conversation as resolved.
skipped.length === 0
? []
Expand All @@ -2011,10 +2051,10 @@ function runAllChunks(
.map(
(s) =>
`chunk ${s.chunkId} — retired: dry in rounds ` +
`${s.dryRounds[0]} and ${s.dryRounds[1]}, ` +
(s.nextColdCheck > REVERSE_AUDIT_MAX_ROUNDS
`${s.dryRounds.join(' and ')}, ` +
(s.nextColdCheck > planRoundCap
? `certificate final — the ` +
`${REVERSE_AUDIT_MAX_ROUNDS}-round hard cap leaves ` +
`${planRoundCap}-round cap leaves ` +
`the loop no round for a cold check`
: `next cold check round ${s.nextColdCheck}`),
)
Expand Down Expand Up @@ -2356,7 +2396,11 @@ function runAgentPrompt(args: AgentPromptArgs): void {
args.role === 'reverse-audit' &&
!hasChunk &&
!args.allChunks &&
!admitReverseAuditRound(args.plan, args.round)
!admitReverseAuditRound(
args.plan,
args.round,
reverseAuditRoundCap(report.budget),
)
) {
return;
}
Expand Down Expand Up @@ -2430,7 +2474,14 @@ function runAgentPrompt(args: AgentPromptArgs): void {
return;
}
}
if (!admitReverseAuditRound(args.plan, args.round)) return;
if (
!admitReverseAuditRound(
args.plan,
args.round,
reverseAuditRoundCap(report.budget),
Comment thread
wenshao marked this conversation as resolved.
)
)
return;
}

if (args.allChunks && args.role && findingsContent !== undefined) {
Expand Down Expand Up @@ -2526,10 +2577,11 @@ export const agentPromptCommand: CommandModule = {
"Build a review agent's launch prompt from the plan (the diff path, its line " +
"ranges and the agent's own brief are welded in, not left to the caller to " +
'remember). Exit codes: 0 built; 4 a build was refused — the review time ' +
'budget refused another reverse-audit round (BUDGET line on stderr), or ' +
'the compose floor refused a verifier so compose/submit still fit ' +
'(VERIFY BUDGET line) — both termination rules, not errors: stop and ' +
'compose, do not retry; 5 the reverse audit CONVERGED — every chunk holds two ' +
'budget refused another reverse-audit round (BUDGET line on stderr), the ' +
"plan's round cap refused one (ROUND CAP line), or the compose floor " +
'refused a verifier so compose/submit still fit (VERIFY BUDGET line) — ' +
'all termination rules, not errors: stop and compose, do not retry; 5 ' +
'the reverse audit CONVERGED — every chunk holds two ' +
'consecutive substantive dry audits and none is due a cold check, so stop ' +
'the loop and proceed to Step 6 (also a termination rule, and a clean one: ' +
'no disclosure is owed); anything else is a bad call or a broken plan.',
Expand Down
25 changes: 24 additions & 1 deletion packages/cli/src/commands/review/compose-review.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { createHash } from 'node:crypto';
import { promptRecordDir, briefPath } from './lib/prompt-record.js';
import { writeBudgetStop } from './lib/deadline.js';
import { writeBudgetStop, writeRoundCapStop } from './lib/deadline.js';
import { getGhHost, setGhHost } from './lib/gh.js';
import { parseLedger } from './lib/ledger.js';
import { countInlineFindings } from './lib/inline-counts.js';
Expand Down Expand Up @@ -695,6 +695,29 @@ describe('composeReview — event caps (round-7 Critical #2: caps must reach eve
expect(r.body).not.toContain('no blockers');
});

it('a round-cap marker caps the verdict and dedups against the relayed entry', () => {
// A huge diff's reverse audit ran its full 3 rounds without converging;
// the builder refused round 4 and wrote a round-cap marker. compose-review
// caps on it whether or not the orchestrator relays — and says it once
// when the orchestrator does relay.
const plan = coveredPlan();
writeRoundCapStop(plan, 3, 4);
const r = composeReview(base({ planPath: plan }));
expect(r.event).toBe('COMMENT');
expect(r.body).toContain('reverse-audit round cap of 3');
expect(r.body).not.toContain('LGTM');

const r2 = composeReview(
base({
planPath: plan,
unreviewedDimensions: [
'reverse audit — did not converge within the reverse-audit round cap of 3',
],
}),
);
expect(r2.body.split('reverse-audit round cap').length - 1).toBe(1);
});

it('a budget-stop marker caps APPROVE at COMMENT with nothing relayed by the caller', () => {
// The round builder refused a round and recorded the refusal; the
// disclosure that caps the verdict is synthesized from that marker, not
Expand Down
17 changes: 15 additions & 2 deletions packages/cli/src/commands/review/compose-review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,12 @@ import {
} from './lib/coverage.js';
import {
BUDGET_STOP_PHRASE,
ROUND_CAP_PHRASE,
budgetStopDisclosure,
roundCapStopDisclosure,
readBudgetStop,
} from './lib/deadline.js';
import { MAX_REVERSE_AUDIT_ROUNDS } from './lib/budget.js';
import { shellQuotePath } from './lib/shell-quote.js';
import { gh, setGhHost } from './lib/gh.js';
import {
Expand Down Expand Up @@ -420,12 +423,22 @@ function composeReviewBody(
if (input.planPath) {
const stop = readBudgetStop(input.planPath);
if (stop !== null) {
// A round-cap stop and a time-budget stop both cap the verdict, but
// read differently and dedup against a different relayed phrase. The
// marker's `cause` picks which; an absent cause is a time stop, for
// markers written before the cause field existed.
const isRoundCap = stop.cause === 'round-cap';
const phrase = isRoundCap ? ROUND_CAP_PHRASE : BUDGET_STOP_PHRASE;
Comment thread
wenshao marked this conversation as resolved.
for (let i = unreviewed.length - 1; i >= 0; i--) {
if (unreviewed[i].includes(BUDGET_STOP_PHRASE)) {
if (unreviewed[i].includes(phrase)) {
unreviewed.splice(i, 1);
}
}
budgetEntry = budgetStopDisclosure(stop.round ?? undefined);
budgetEntry = isRoundCap
? roundCapStopDisclosure(
typeof stop.cap === 'number' ? stop.cap : MAX_REVERSE_AUDIT_ROUNDS,
)
: budgetStopDisclosure(stop.round ?? undefined);
coverageEntries.push(budgetEntry);
}
}
Expand Down
Loading
Loading