From b071dcc0fc8217b8adea9d98df466c347d94a631 Mon Sep 17 00:00:00 2001 From: garrytan Date: Fri, 30 Oct 2026 03:34:45 +0000 Subject: [PATCH] fix(ship-budget): --finish settles a later open batch after the pass program settled the first; Step 14.5 check passes --reading Pass-2 review of #3633 (GSTA-167): the outside voice found the started match required the first batch to stay consumed, so the documented Red Team fallback never ran; the adversarial pass found the docs-audit check had no --reading, which exits 2 on the Capy meter. Adds tests for the finish filter cells the batched specialist named. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- lib/ship-budget.ts | 2 +- ship/sections/documentation.md | 2 +- ship/sections/documentation.md.tmpl | 2 +- test/ship-budget-review-log.test.ts | 48 +++++++++++++++++++++++++++++ test/ship-budget-section.test.ts | 10 ++++++ 5 files changed, 61 insertions(+), 3 deletions(-) diff --git a/lib/ship-budget.ts b/lib/ship-budget.ts index 9b8cae7f6..5b013efa0 100644 --- a/lib/ship-budget.ts +++ b/lib/ship-budget.ts @@ -547,7 +547,7 @@ export function admitReviewStart(marker: string, reservation: string | undefined */ export function settleReviewFinish(marker: string, token: string, env: NodeJS.ProcessEnv = process.env): string | undefined { const active = activeLedger(marker); - const started = (r: Reservation) => r.state === 'consumed' && (r.token === token || !!r.starts?.some(s => s.token === token)); + const started = (r: Reservation) => (r.state === 'consumed' || r.state === 'settled') && (r.token === token || !!r.starts?.some(s => s.token === token)); if (!active || !active.reservations.some(started)) return undefined; return mutateLedger(active.file, env, l => { const r = l.reservations.find(started); diff --git a/ship/sections/documentation.md b/ship/sections/documentation.md index 447d8f4e0..a36fde7c1 100644 --- a/ship/sections/documentation.md +++ b/ship/sections/documentation.md @@ -42,7 +42,7 @@ lines that name a changed code path and the diff of the changed docs plus that c build blocks before launch. Dispatch one subagent whose whole brief is Step 7's packet brief with this packet's `read=` ranges; retain the child id. It proposes edits as findings and never writes a doc. Under a token cap the dispatch is one program, like a review pass: -`gstack-ship-budget check --run --step 14.5 --next "$BUDGET_NEXT"` (build's line), the reading `BEFORE` +`gstack-ship-budget check --run --step 14.5 --next "$BUDGET_NEXT" --reading ` (build's line, a fresh reading), the reading `BEFORE` right before the reader starts, `tools.subagent.create`, a `tools.subagent.get` poll until it settles, the reading `AFTER`, then `gstack-ship-budget settle --run --reservation --dispatched-at --reading --readers 1`, all in that same program (`--reservation` is check's `RESERVATION_ID:` line); a settle from a later program is unmeasured. diff --git a/ship/sections/documentation.md.tmpl b/ship/sections/documentation.md.tmpl index 44520cf57..ce3b9ac9c 100644 --- a/ship/sections/documentation.md.tmpl +++ b/ship/sections/documentation.md.tmpl @@ -40,7 +40,7 @@ lines that name a changed code path and the diff of the changed docs plus that c build blocks before launch. Dispatch one subagent whose whole brief is Step 7's packet brief with this packet's `read=` ranges; retain the child id. It proposes edits as findings and never writes a doc. Under a token cap the dispatch is one program, like a review pass: -`gstack-ship-budget check --run --step 14.5 --next "$BUDGET_NEXT"` (build's line), the reading `BEFORE` +`gstack-ship-budget check --run --step 14.5 --next "$BUDGET_NEXT" --reading ` (build's line, a fresh reading), the reading `BEFORE` right before the reader starts, `tools.subagent.create`, a `tools.subagent.get` poll until it settles, the reading `AFTER`, then `gstack-ship-budget settle --run --reservation --dispatched-at --reading --readers 1`, all in that same program (`--reservation` is check's `RESERVATION_ID:` line); a settle from a later program is unmeasured. diff --git a/test/ship-budget-review-log.test.ts b/test/ship-budget-review-log.test.ts index 65877f004..d2a93c91a 100644 --- a/test/ship-budget-review-log.test.ts +++ b/test/ship-budget-review-log.test.ts @@ -117,6 +117,54 @@ describe('gstack-review-log --start under a token cap', () => { expect(close.code, close.out).toBe(0); }); + // Value: protects=--finish settles a later Red Team batch even after the pass program already settled the first batch, which is the documented order (the outside voice on #3633 pass 2: the started match required the first batch to stay consumed, so the fallback never ran); fails_when=settleReviewFinish only matches a still-consumed start reservation; + // why_new=the test above finishes before any settle; seam=none; no_claim=that the Red Team program's own settle ran + test('--finish still settles the open Red Team batch after the pass program settled the first batch', () => { + 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 settled = h.sb(['settle', '--run', run.run, '--reservation', first, '--dispatched-at', '0', '--reading', '5', '--readers', '1']); + expect(settled.code, settled.out).toBe(0); + const redTeam = h.field(h.sb(['check', '--run', run.run, '--step', '9', '--next', 'dispatch:red-team', '--reading', '5']).out, 'RESERVATION', 'id'); + 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 ${redTeam}`); + expect(h.ledger(run.ledger).reservations.find((r: any) => r.id === redTeam).state).toBe('settled'); + expect(h.sb(['close', '--run', run.run, '--reading', '10']).out).toContain('reserved=0'); + }); + + // Value: protects=--finish settles only its own pass: an open dispatch reserved before the start in the same step, and an open dispatch of another step stay open (the batched specialist on #3633 pass 2: untested cells of the filter); fails_when=the index or step filter in settleReviewFinish is dropped; + // why_new=the tests above hold only the pass's own batches; seam=none; no_claim=the filters are the right pass boundary for design-review-lite + test('--finish leaves an earlier open batch of its step and another step\'s open batch open', () => { + const h = fresh(); + const run = capped(h); + const earlier = reserve(h, run.run, 'dispatch:red-team'); + const first = reserve(h, run.run, 'dispatch:specialist'); + const start = h.reviewLog(['--start', 'review', '--reservation', first]); + expect(start.code, start.err).toBe(0); + const other = h.field(h.sb(['check', '--run', run.run, '--step', '11', '--next', 'dispatch:adversarial-native', '--reading', '0']).out, 'RESERVATION', 'id'); + const state = (id: string) => h.ledger(run.ledger).reservations.find((r: any) => r.id === id)?.state; + expect([state(earlier), state(other)]).toEqual(['open', 'open']); + const row = { skill: 'review', timestamp: new Date().toISOString(), status: 'clean', issues_found: 0, critical: 0, informational: 0 }; + expect(h.reviewLog([JSON.stringify(row), '--finish', start.out.trim()]).code).toBe(0); + expect([state(first), state(earlier), state(other)]).toEqual(['settled', 'open', 'open']); + }); + + test('--finish leaves a step reservation made after the start open', () => { + 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 step = h.field(h.sb(['check', '--run', run.run, '--step', '9', '--next', '10', '--reading', '0']).out, 'RESERVATION', 'id'); + const row = { skill: 'review', timestamp: new Date().toISOString(), status: 'clean', issues_found: 0, critical: 0, informational: 0 }; + expect(h.reviewLog([JSON.stringify(row), '--finish', start.out.trim()]).code).toBe(0); + expect(h.ledger(run.ledger).reservations.find((r: any) => r.id === step)?.state).toBe('open'); + }); + // 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', () => { diff --git a/test/ship-budget-section.test.ts b/test/ship-budget-section.test.ts index c83458181..588f1703a 100644 --- a/test/ship-budget-section.test.ts +++ b/test/ship-budget-section.test.ts @@ -69,6 +69,16 @@ describe('ship budget skill structure', () => { expect(capy).toMatch(/all in that same (?:execute )?program/); }); + // Value: protects=every concrete check command in the ship sections passes the counter, since on the Capy meter a check without --reading exits 2 and the dispatch it gates never starts (#3633 pass 2: Step 14.5's docs-audit check had no reading); fails_when=a section spells `gstack-ship-budget check --run` without `--reading`; + // why_new=the order test above matches the command as a prefix; seam=none; no_claim=the agent fills the reading + test('every gstack-ship-budget check --run command in the ship sections passes --reading', () => { + for (const rel of fs.readdirSync(path.join(ROOT, 'ship/sections')).filter(n => n.endsWith('.md.tmpl'))) { + for (const cmd of read(`ship/sections/${rel}`).match(/gstack-ship-budget check --run [^`]*/g) ?? []) { + expect(cmd, rel).toContain('--reading'); + } + } + }); + // Value: protects=every prose site that takes a reservation id reads the RESERVATION_ID: line, never a field cut from RESERVATION:; fails_when=a dispatch section or the Capy skill names the id without that line; // why_new=the line is new; seam=none; no_claim=the agent copies it test('the review pass, Steps 9, 11 and 14.5 and the Capy skill read the id from the RESERVATION_ID: line', () => {