Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
175 changes: 175 additions & 0 deletions packages/cli/src/commands/review/agent-prompt.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,74 @@ describe('agent-prompt (command boundary)', () => {
}
});

it('takes the round cap from the plan topology at the --chunk gate too', () => {
// The fourth of the four cap call sites, and the only one with no tier-10
// coverage: a 3A-sized plan can carry chunks (the chunk budget is 400
// lines while the 3A gate admits 3200 total), so a round rebuilt or
// repaired one --chunk at a time on a small plan reaches THIS gate. A
// regression touching only it would stay green suite-wide.
const dir = mkdtempSync(join(tmpdir(), 'ap-chunk-tier-'));
try {
const findings = join(dir, 'f.md');
writeFileSync(findings, '- x');
const handler = agentPromptCommand.handler as (a: unknown) => void;
delete process.env[DEADLINE_ENV];
const stderr = () =>
(writeStderrLine as unknown as Mock).mock.calls
.map((c) => c[0])
.join('\n');

const small = join(dir, 'small.json');
writeFileSync(
small,
JSON.stringify({ ...PLAN, srcDiffLines: 100, diffLines: 100 }),
);
process.exitCode = undefined;
(writeStderrLine as unknown as Mock).mockClear();
handler({
plan: small,
role: 'reverse-audit',
chunk: 14,
findings,
round: 6,
});
expect(process.exitCode).toBeUndefined();
expect(readRecordedPrompts(small).size).toBe(1);

(writeStderrLine as unknown as Mock).mockClear();
handler({
plan: small,
role: 'reverse-audit',
chunk: 14,
findings,
round: 11,
});
expect(process.exitCode).toBe(4);
expect(stderr()).toContain('round cap is 10');

const large = join(dir, 'large.json');
writeFileSync(
large,
JSON.stringify({ ...PLAN, srcDiffLines: 900, diffLines: 900 }),
);
process.exitCode = undefined;
(writeStderrLine as unknown as Mock).mockClear();
handler({
plan: large,
role: 'reverse-audit',
chunk: 14,
findings,
round: 6,
});
expect(process.exitCode).toBe(4);
expect(stderr()).toContain('round cap is 5');
expect(readRecordedPrompts(large).size).toBe(0);
} finally {
process.exitCode = undefined;
rmSync(dir, { recursive: true, force: true });
}
});

it('lets --role reverse-audit --chunk N through and keys the record by its chunk', () => {
// The unit tests build the launch prompt directly, bypassing the guard and the
// key derivation. This drives the real handler: the guard must let the one legal
Expand Down Expand Up @@ -960,6 +1028,55 @@ describe('--round — the CLI bakes the round into the identity line and the key
}
});

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
// same round 6 off the same builder: admitted under the 3A tier, refused
// under the 3B one. A flat cap cannot produce both.
const dir = mkdtempSync(join(tmpdir(), 'ap-cap-tier-'));
try {
const findings = join(dir, 'f.md');
writeFileSync(findings, '- x');
const handler = agentPromptCommand.handler as (a: unknown) => void;
delete process.env[DEADLINE_ENV];
const stderr = () =>
(writeStderrLine as unknown as Mock).mock.calls
.map((c) => c[0])
.join('\n');

const small = join(dir, 'small.json');
writeFileSync(
small,
JSON.stringify({ ...PLAN, srcDiffLines: 100, diffLines: 100 }),
);
process.exitCode = undefined;
(writeStderrLine as unknown as Mock).mockClear();
handler({ plan: small, role: 'reverse-audit', findings, round: 6 });
expect(process.exitCode).toBeUndefined();
expect(readRecordedPrompts(small).size).toBe(1);

(writeStderrLine as unknown as Mock).mockClear();
handler({ plan: small, role: 'reverse-audit', findings, round: 11 });
expect(process.exitCode).toBe(4);
expect(stderr()).toContain('round cap is 10');

const large = join(dir, 'large.json');
writeFileSync(
large,
JSON.stringify({ ...PLAN, srcDiffLines: 900, diffLines: 900 }),
);
process.exitCode = undefined;
(writeStderrLine as unknown as Mock).mockClear();
handler({ plan: large, role: 'reverse-audit', findings, round: 6 });
expect(process.exitCode).toBe(4);
expect(stderr()).toContain('round cap is 5');
expect(readRecordedPrompts(large).size).toBe(0);
} finally {
process.exitCode = undefined;
rmSync(dir, { recursive: true, force: true });
}
});

it('carries the round through --all-chunks: every key and every identity line', () => {
const dir = mkdtempSync(join(tmpdir(), 'ap-round-batch-'));
try {
Expand Down Expand Up @@ -3744,6 +3861,30 @@ describe('per-chunk retirement — cold territories stop costing a round', () =>
expect(out).not.toContain('next cold check round 6');
});

it('the cap in the retirement note is the plan’s tier, not a constant', () => {
// The third of the four cap call sites. Same history as the cap-5 test
// above, on a 3A-sized plan: round 5's retirement schedules its cold check
// for round 6, which the 3A tier ALLOWS — so the note must promise that
// check rather than close the certificate. The two tests are the same
// scenario with opposite outcomes, which is what makes this site's read of
// the plan observable at all.
writeFileSync(
plan,
JSON.stringify({ ...PLAN, srcDiffLines: 100, diffLines: 100 }),
);
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: DRY, 14: YIELD, 15: YIELD });
answerRound(4, { 13: DRY, 14: YIELD, 15: YIELD });

const out = runRound(5);
expect(out).toContain('chunk 13 — retired: dry in rounds 3 and 4');
expect(out).toContain('next cold check round 6');
expect(out).not.toContain('certificate final');
});

it('the cold check comes due on parity — the retired chunk is built again', () => {
answerRound(1, { 13: DRY, 14: YIELD, 15: YIELD });
answerRound(2, { 13: DRY, 14: YIELD, 15: YIELD });
Expand Down Expand Up @@ -3932,6 +4073,10 @@ describe('per-chunk retirement — cold territories stop costing a round', () =>
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.
//
// Five because `PLAN` carries no `srcDiffLines`/`diffLines`, so the tier
// read is the unsized fallback — deliberately the large tier, which is
// what every plan got before tiering. The sized 3A case is the next test.
answerRound(1, { 13: YIELD, 14: YIELD, 15: YIELD });
answerRound(2, { 13: YIELD, 14: YIELD, 15: YIELD });
answerRound(3, { 13: YIELD, 14: YIELD, 15: YIELD });
Expand All @@ -3949,6 +4094,36 @@ describe('per-chunk retirement — cold territories stop costing a round', () =>
expect(msg).toContain('round cap is 5');
});

it('a 3A-sized plan runs to ten rounds, not five', () => {
// The gate reads the plan's topology tier, so a small diff — where a
// round is one auditor, not one per chunk — keeps auditing where the 3B
// number would have stopped it. Round 6 is the whole change: it is
// refused in the test above and admitted here off the same builder, so a
// revert to a single flat cap fails on the admission, not just on the
// number in the refusal text.
writeFileSync(
plan,
JSON.stringify({ ...PLAN, srcDiffLines: 100, diffLines: 100 }),
);
const old = new Date(2020, 0, 1);
utimesSync(plan, old, old);
for (const r of [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]) {
answerRound(r, { 13: YIELD, 14: YIELD, 15: YIELD });
expect(process.exitCode).toBeUndefined();
}
expect(keysOf(6)).not.toHaveLength(0);

const out = runRound(11);
expect(process.exitCode).toBe(4);
expect(out).toBe('');
expect(keysOf(11)).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 10');
});

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
31 changes: 21 additions & 10 deletions packages/cli/src/commands/review/agent-prompt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,13 +139,22 @@ interface PlanReport {
mergeBaseSha?: unknown;
host?: unknown;
repositoryContext?: unknown;
budget?: { agentToolBudget?: unknown };
// The two size fields the topology gate reads (#9242). Declared so the
// per-chunk build paths can notice a fan-out the plan's own numbers never
// asked for; `isTerritoryFanOut` already tolerates the `unknown` via the
// `RosterPlan` cast, same bridge `runRoster` uses.
/**
* The two size fields the topology gate reads (#9242) and the ones
* `reverseAuditRoundCap` derives this plan's round-cap tier from — the same
* pair, read by two callers for two reasons, which is why one declaration
* serves both. Declared even though those functions take `unknown` (they
* parse a file, so they validate at runtime whatever the type says) because
* the declaration is what makes the coupling visible: without it a rename on
* the writing side compiles clean, the per-chunk paths stop noticing a
* fan-out the plan never asked for, and every cap here silently collapses to
* the fallback tier — a quieter failure than a wrong number.
* `isTerritoryFanOut` tolerates the `unknown` via the `RosterPlan` cast, the
* same bridge `runRoster` uses.
*/
srcDiffLines?: unknown;
diffLines?: unknown;
budget?: { agentToolBudget?: unknown; reverseAuditRounds?: unknown };
}

/** A heavy file's entry, which is the only kind an invariant agent can be built from. */
Expand Down Expand Up @@ -1993,7 +2002,9 @@ function admitReverseAuditRound(
fanOutWidth: 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
// deadline arithmetic. One value per topology (`reverseAuditRoundTier`) —
// ten on a 3A diff, where a round is one auditor; five on a 3B one, where
// it is one per non-retired chunk; a reduced three 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
Expand Down Expand Up @@ -2166,7 +2177,7 @@ function runAllChunks(
!admitReverseAuditRound(
planPath,
round,
reverseAuditRoundCap(report.budget),
reverseAuditRoundCap(report),
chunks.length,
)
) {
Expand Down Expand Up @@ -2222,7 +2233,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 planRoundCap = reverseAuditRoundCap(report);
Comment thread
wenshao marked this conversation as resolved.
const retirementNote =
skipped.length === 0
? []
Expand Down Expand Up @@ -2585,7 +2596,7 @@ function runAgentPrompt(args: AgentPromptArgs): void {
!admitReverseAuditRound(
args.plan,
args.round,
reverseAuditRoundCap(report.budget),
reverseAuditRoundCap(report),
1,
)
) {
Expand Down Expand Up @@ -2668,7 +2679,7 @@ function runAgentPrompt(args: AgentPromptArgs): void {
!admitReverseAuditRound(
args.plan,
args.round,
reverseAuditRoundCap(report.budget),
reverseAuditRoundCap(report),
planChunkIds.length,
Comment thread
wenshao marked this conversation as resolved.
)
)
Expand Down
6 changes: 4 additions & 2 deletions packages/cli/src/commands/review/compose-review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import {
roundCapStopDisclosure,
readBudgetStop,
} from './lib/deadline.js';
import { MAX_REVERSE_AUDIT_ROUNDS } from './lib/budget.js';
import { LARGE_REVERSE_AUDIT_ROUNDS } from './lib/budget.js';
import { shellQuotePath } from './lib/shell-quote.js';
import {
HOSTNAME_RE,
Expand Down Expand Up @@ -973,7 +973,9 @@ function composeReviewBody(
}
budgetEntry = isRoundCap
? roundCapStopDisclosure(
typeof stop.cap === 'number' ? stop.cap : MAX_REVERSE_AUDIT_ROUNDS,
typeof stop.cap === 'number'
? stop.cap
: LARGE_REVERSE_AUDIT_ROUNDS,
)
: budgetStopDisclosure(stop.round ?? undefined);
coverageEntries.push(budgetEntry);
Expand Down
Loading
Loading