From 2d3d90cd0c60bd1915e7dddf2d9c9382fb4dc5be Mon Sep 17 00:00:00 2001 From: garrytan Date: Fri, 30 Oct 2026 02:55:02 +0000 Subject: [PATCH] fix(ship-budget): Step 9's Red Team batch takes no start of its own; --finish settles it gstack-evals #34 round 3: the pass program's start-once-per-role rule gave the Red Team batch an extra --start review token that no row ever finished. A later batch of the same Step 9 pass (the Red Team or an ADMIT remainder) now takes no start, and --finish REVIEW_START settles the open later batches of the step reserved after the started one, so none stays reserved at close. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- lib/ship-budget.ts | 9 ++++++--- ship/sections/budget-handoff.md | 2 +- ship/sections/budget-handoff.md.tmpl | 2 +- ship/sections/review-army.md | 2 +- ship/sections/review-army.md.tmpl | 2 +- test/ship-budget-review-log.test.ts | 22 ++++++++++++++++++++++ test/ship-budget-section.test.ts | 8 ++++++++ 7 files changed, 40 insertions(+), 7 deletions(-) diff --git a/lib/ship-budget.ts b/lib/ship-budget.ts index 1bffe4af0..6c7946c13 100644 --- a/lib/ship-budget.ts +++ b/lib/ship-budget.ts @@ -514,8 +514,10 @@ export function admitReviewStart(marker: string, reservation: string | undefined /** * gstack-review-log --finish TOKEN: the pass is over, so settle the reservation - * that start consumed and every other dispatch reservation started in the same - * step (a pass logs one row even when its reviewers were admitted one by one). + * that start consumed, every other dispatch reservation started in the same + * step (a pass logs one row even when its reviewers were admitted one by one), + * and every later batch of the pass reserved after it that took no start (Step + * 9's Red Team or an ADMIT remainder). */ export function settleReviewFinish(marker: string, token: string, env: NodeJS.ProcessEnv = process.env): string | undefined { const active = activeLedger(marker); @@ -524,7 +526,8 @@ export function settleReviewFinish(marker: string, token: string, env: NodeJS.Pr return mutateLedger(active.file, env, l => { const r = l.reservations.find(started); if (!r) return undefined; - const pass = l.reservations.filter(x => x.kind === 'dispatch' && x.state === 'consumed' && x.step === r.step); + const at = l.reservations.indexOf(r); + const pass = l.reservations.filter((x, i) => x.kind === 'dispatch' && x.step === r.step && (x.state === 'consumed' || (x.state === 'open' && i > at))); for (const x of pass) settleIn(l, x); return pass.map(x => x.id).join(','); }); diff --git a/ship/sections/budget-handoff.md b/ship/sections/budget-handoff.md index c0efba2e1..be5a7f1cb 100644 --- a/ship/sections/budget-handoff.md +++ b/ship/sections/budget-handoff.md @@ -33,7 +33,7 @@ after they settle; `close` flags roles never re-checked. `--finish` settles the **One program per review pass.** One execute program (one shell script off Capy) runs, in order: `gstack-review-packet build` (a later build in the pass is `build --like `), `check --next "$BUDGET_NEXT"`, -`gstack-review-log --start --reservation ` once per row the step logs, the reading `BEFORE`, the readers +`gstack-review-log --start --reservation ` once per row the step logs (a later batch of the same pass (the Red Team or an ADMIT remainder) takes no start; `--finish` settles it), the reading `BEFORE`, the readers (`tools.subagent.create` on Capy), the poll until the last settles (`tools.subagent.get`), the reading `AFTER`, `gstack-ship-budget settle`, then `gstack-review-packet collect`, returning only collect's output. Settle each batch in the program that dispatched it: read the counter right before the readers diff --git a/ship/sections/budget-handoff.md.tmpl b/ship/sections/budget-handoff.md.tmpl index a9b66aa46..71dec5c51 100644 --- a/ship/sections/budget-handoff.md.tmpl +++ b/ship/sections/budget-handoff.md.tmpl @@ -31,7 +31,7 @@ after they settle; `close` flags roles never re-checked. `--finish` settles the **One program per review pass.** One execute program (one shell script off Capy) runs, in order: `gstack-review-packet build` (a later build in the pass is `build --like `), `check --next "$BUDGET_NEXT"`, -`gstack-review-log --start --reservation ` once per row the step logs, the reading `BEFORE`, the readers +`gstack-review-log --start --reservation ` once per row the step logs (a later batch of the same pass (the Red Team or an ADMIT remainder) takes no start; `--finish` settles it), the reading `BEFORE`, the readers (`tools.subagent.create` on Capy), the poll until the last settles (`tools.subagent.get`), the reading `AFTER`, `gstack-ship-budget settle`, then `gstack-review-packet collect`, returning only collect's output. Settle each batch in the program that dispatched it: read the counter right before the readers diff --git a/ship/sections/review-army.md b/ship/sections/review-army.md index 9db9197be..0c2f0a8f9 100644 --- a/ship/sections/review-army.md +++ b/ship/sections/review-army.md @@ -105,7 +105,7 @@ This pass is static; defer product probes to Step 9.2.1. 1. Read `~/.claude/skills/gstack/review/checklist.md`. If the file cannot be read, **STOP** and report the error. -2. Before reading the diff, run `~/.claude/skills/gstack/bin/gstack-review-log --start review` and save its token as REVIEW_START (under a token cap, run Step 9.1's `gstack-review-packet build` and `gstack-ship-budget check` first, then `gstack-review-log --start review --reservation `; 9.1's program then dispatches that batch with no second check or start). Then run `git diff origin/` (`git diff $RANGE_BASE` from pass 2 on). Read non-ignored untracked source files too (`git ls-files --others --exclude-standard`); the snapshot includes them. +2. Before reading the diff, run `~/.claude/skills/gstack/bin/gstack-review-log --start review` and save its token as REVIEW_START (under a token cap, run Step 9.1's `gstack-review-packet build` and `gstack-ship-budget check` first, then `gstack-review-log --start review --reservation `; 9.1's program then dispatches that batch with no second check or start; a later batch of the same Step 9 pass (the Red Team or an ADMIT remainder) takes no start, and `--finish REVIEW_START` settles it). Then run `git diff origin/` (`git diff $RANGE_BASE` from pass 2 on). Read non-ignored untracked source files too (`git ls-files --others --exclude-standard`); the snapshot includes them. Every pass captures a full token (no `--fix-range`), so the record binds the whole tree even when reviewers read only the range. diff --git a/ship/sections/review-army.md.tmpl b/ship/sections/review-army.md.tmpl index 88b0c927e..5a36881a3 100644 --- a/ship/sections/review-army.md.tmpl +++ b/ship/sections/review-army.md.tmpl @@ -50,7 +50,7 @@ This pass is static; defer product probes to Step 9.2.1. 1. Read `~/.claude/skills/gstack/review/checklist.md`. If the file cannot be read, **STOP** and report the error. -2. Before reading the diff, run `~/.claude/skills/gstack/bin/gstack-review-log --start review` and save its token as REVIEW_START (under a token cap, run Step 9.1's `gstack-review-packet build` and `gstack-ship-budget check` first, then `gstack-review-log --start review --reservation `; 9.1's program then dispatches that batch with no second check or start). Then run `git diff origin/` (`git diff $RANGE_BASE` from pass 2 on). Read non-ignored untracked source files too (`git ls-files --others --exclude-standard`); the snapshot includes them. +2. Before reading the diff, run `~/.claude/skills/gstack/bin/gstack-review-log --start review` and save its token as REVIEW_START (under a token cap, run Step 9.1's `gstack-review-packet build` and `gstack-ship-budget check` first, then `gstack-review-log --start review --reservation `; 9.1's program then dispatches that batch with no second check or start; a later batch of the same Step 9 pass (the Red Team or an ADMIT remainder) takes no start, and `--finish REVIEW_START` settles it). Then run `git diff origin/` (`git diff $RANGE_BASE` from pass 2 on). Read non-ignored untracked source files too (`git ls-files --others --exclude-standard`); the snapshot includes them. Every pass captures a full token (no `--fix-range`), so the record binds the whole tree even when reviewers read only the range. diff --git a/test/ship-budget-review-log.test.ts b/test/ship-budget-review-log.test.ts index 745a83cd6..c5b2f3a6a 100644 --- a/test/ship-budget-review-log.test.ts +++ b/test/ship-budget-review-log.test.ts @@ -117,6 +117,28 @@ describe('gstack-review-log --start under a token cap', () => { expect(close.code, close.out).toBe(0); }); + // Value: protects=a later batch of the same Step 9 pass (the Red Team, built after collect) takes no start of its own and still settles when the pass finishes, so no start token is left unfinished and no reservation stays open (gstack-evals #34 round 3: an extra red-team --start review token was never finished); fails_when=--finish settles only reservations a start consumed, leaving the Red Team's open and counted as reserved at close; + // why_new=the --finish tests above start every batch; seam=none; no_claim=that the program's own settle ran + test('--finish REVIEW_START settles the pass\'s later Red Team batch that took no start; close finds nothing reserved or unreserved', () => { + const h = fresh(); + const run = capped(h); + const first = reserve(h, run.run, 'dispatch:specialist'); + const start = h.reviewLog(['--start', 'review', '--reservation', first]); + expect(start.code, start.err).toBe(0); + const redTeam = reserve(h, run.run, 'dispatch:red-team'); + const earlier = h.ledger(run.ledger).reservations.find((r: any) => r.id === redTeam); + expect(earlier.state).toBe('open'); + const row = { skill: 'review', timestamp: new Date().toISOString(), status: 'clean', issues_found: 0, critical: 0, informational: 0 }; + const fin = h.reviewLog([JSON.stringify(row), '--finish', start.out.trim()]); + expect(fin.code, fin.err).toBe(0); + expect(fin.err).toContain(`settled budget reservations ${first},${redTeam}`); + expect(h.ledger(run.ledger).reservations.filter((r: any) => r.kind === 'dispatch').map((r: any) => r.state)).toEqual(['settled', 'settled']); + const close = h.sb(['close', '--run', run.run, '--reading', '10']); + expect(close.out).toContain('reserved=0'); + expect(close.out).not.toContain('BUDGET_CHECK_SKIPPED'); + expect(close.code, close.out).toBe(0); + }); + test('close prints one BUDGET_CHECK_SKIPPED line per unreserved dispatch and ends incomplete', () => { const h = fresh(); const run = capped(h); diff --git a/test/ship-budget-section.test.ts b/test/ship-budget-section.test.ts index c0bff9b29..f60e2ff21 100644 --- a/test/ship-budget-section.test.ts +++ b/test/ship-budget-section.test.ts @@ -48,6 +48,14 @@ describe('ship budget skill structure', () => { expect(handoff).not.toContain('once per role on the printed reservation'); }); + // Value: protects=the pass program says a later batch of the same Step 9 pass takes no start, so the Red Team leaves no unfinished token; fails_when=the section tells every batch to start once per role; + // why_new=the start test above pins the row rule only; seam=none; no_claim=the agent follows it + test('a later batch of the same Step 9 pass (Red Team, an ADMIT remainder) takes no start of its own', () => { + for (const rel of ['ship/sections/budget-handoff.md.tmpl', 'ship/sections/review-army.md.tmpl']) { + expect(read(rel), rel).toMatch(/later batch of the same (?:Step 9 )?pass \(the Red Team or an ADMIT remainder\) takes no start/); + } + }); + test('every review-log --start in the dispatch sections takes a reservation when a cap is set', () => { for (const rel of ['ship/sections/review-army.md.tmpl', 'ship/sections/adversarial.md.tmpl']) expect(read(rel)).toContain('--reservation'); });