test(review): sync round-cap prose and pin the deferred coverage gaps (#9256)

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

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.

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),

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