From f9fa397e0a5e1a2f6a51347c6be313dff8c6c0cd Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 2 Nov 2026 14:46:05 +0000 Subject: [PATCH] fix: pre-landing review fixes (GST-967 Step 11: grant row names review_loop_exception, a fix-check never earns another, voices route queued blockers to the plan) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-review-plan | 18 +++++-------- changelog.d/gst-967-gate-voices.md | 5 ++-- docs/review-policy.md | 19 +++++++------- lib/review-depth.ts | 8 +++--- ship/sections/adversarial.md | 6 ++--- ship/sections/adversarial.md.tmpl | 6 ++--- ship/sections/review-army.md | 2 +- ship/sections/review-army.md.tmpl | 2 +- test/review-depth.test.ts | 20 +++++++------- test/review-packets-removed.test.ts | 6 ++--- test/review-pass.test.ts | 2 +- test/review-plan-cli.test.ts | 26 ++++++++++++------- .../size-ledger/ship/gst-967-gate-voices.json | 2 +- 13 files changed, 61 insertions(+), 61 deletions(-) diff --git a/bin/gstack-review-plan b/bin/gstack-review-plan index 2ef4d2735..1ac958ca3 100755 --- a/bin/gstack-review-plan +++ b/bin/gstack-review-plan @@ -5,7 +5,7 @@ * gstack-review-plan --base [--pass ] [--last none|full|fix-check] [--fixed-blocking] [--fixed-below] * [--blocking-open] [--touched-gate] [--critical-found] [--adversarial-ran] [--gate-tagged ] [--gated a,b] [--force a,b|all] * [--reviewed-numbers ] [--depth-floor standard|full] [--range-base ] [--pass1-depth light|standard|full] [--no-tiers] - * [--targeted-ran ] [--grant [--fix-checks ]] [--json] + * [--targeted-ran ] [--grant ] [--json] * gstack-review-plan --fix-check-prompt --fix-list --hunks --tests [--callers ] * * Reads the diff against the merge base with (committed, tracked uncommitted @@ -55,11 +55,10 @@ const HELP = `gstack-review-plan — the review depth of the next /ship step (do --pass1-depth pass 1's REVIEW_DEPTH depth: full, or absent, keeps pass 2 full --targeted-ran the TARGETED line's targeted_patch_id (the whole committed diff, tests included) when the targeted reader last read it: the same patch skips it; any other id, none, or uncommitted edits dispatch it again - --grant the owner's grant of one scoped fix-check after round 2, recorded with gstack-approvals add (approved): - a round-2 stop for a fixed blocking finding or a gate-class edit plans that fix-check instead - (review_loop_exception); an id the branch's approval record does not hold approved is refused (exit 3) - --fix-checks fix-checks already run, the one that just finished included (default 1 after --last fix-check, else 0); - required with --grant after --last fix-check, so a granted fix-check never repeats + --grant the owner's grant of one scoped fix-check after round 2, recorded with gstack-approvals add (approved, + its ask or scope naming review_loop_exception): the stop after a round-2 full pass with a fixed blocking + finding or a gate-class edit plans that fix-check instead; a fix-check never earns another; an id the + branch's record does not hold that way is refused (exit 3) --fix-check-prompt print the fix-check reviewer's prompt from --fix-list, --hunks, --tests and --callers files Depth (R51, risk not size): full on gate class, a risk class (money, auth, secret, security, data) or any DEPTH_ERROR; @@ -99,11 +98,8 @@ function main(): number { if (!Number.isInteger(pass) || pass < 0) { process.stderr.write('gstack-review-plan: --pass needs an integer >= 0\n'); return EXIT.usage; } const floor = flag('--depth-floor') as Depth | undefined; if (has('--depth-floor') && floor !== 'standard' && floor !== 'full') { process.stderr.write('gstack-review-plan: --depth-floor must be standard or full (a floor only raises the depth)\n'); return EXIT.usage; } - const fixChecks = flag('--fix-checks') === undefined ? undefined : Number(flag('--fix-checks')); - if (has('--fix-checks') && (fixChecks === undefined || !Number.isInteger(fixChecks) || fixChecks < 0)) { process.stderr.write('gstack-review-plan: --fix-checks needs an integer >= 0\n'); return EXIT.usage; } const grant = flag('--grant'); if (has('--grant') && (!grant || grant.startsWith('-'))) { process.stderr.write('gstack-review-plan: --grant needs the approval id gstack-approvals add printed\n'); return EXIT.usage; } - if (grant && last === 'fix-check' && fixChecks === undefined) { process.stderr.write('gstack-review-plan: --grant after --last fix-check needs --fix-checks (fix-checks run so far), so a granted fix-check never repeats\n'); return EXIT.usage; } const rangeBase = flag('--range-base'); if (has('--range-base') && (!rangeBase || rangeBase.startsWith('-'))) { process.stderr.write('gstack-review-plan: --range-base needs the fix range\'s base commit (this pass\'s PASS_HEAD)\n'); return EXIT.usage; } const root = git(['rev-parse', '--show-toplevel'])?.trim(); @@ -112,7 +108,7 @@ function main(): number { if (grant) { const loc = locate({ cwd: root, env: process.env }); const rows = 'file' in loc ? readRecord(loc).rows : []; - if (!rows.some(r => r.id === grant && r.outcome === 'approved')) { process.stderr.write(`gstack-review-plan: --grant ${grant}: no approved row ${grant} in this branch's approval record (${'file' in loc ? 'gstack-approvals list' : loc.message}); record the owner's words with gstack-approvals add first\n`); return EXIT.refused; } + if (!rows.some(r => r.id === grant && r.outcome === 'approved' && `${r.ask} ${r.scope}`.includes('review_loop_exception'))) { process.stderr.write(`gstack-review-plan: --grant ${grant}: no approved row ${grant} naming review_loop_exception in this branch's approval record (${'file' in loc ? 'gstack-approvals list' : loc.message}); record the owner's grant with gstack-approvals add --ask '... (review_loop_exception)' first\n`); return EXIT.refused; } } const collected = collectLive(root, base); if (collected.errors.length) { @@ -162,7 +158,7 @@ function main(): number { const plan = planReview({ ...collected, scope, gateTagged, targeted: { classes: targeted.classes, ran: !['uncommitted', 'none'].includes(patch) && flag('--targeted-ran') === patch }, numbers: scanNumbers(added), force, gated: list('--gated'), floor, tiers: auditEffort !== 'host', criticalFound: has('--critical-found'), adversarialRan: has('--adversarial-ran'), reviewedNumbers: flag('--reviewed-numbers'), - history: { fullPasses: pass, last, fixedBlocking: has('--fixed-blocking'), fixedBelow: has('--fixed-below') && last !== 'fix-check', blockingOpen: has('--blocking-open'), touchedGate: has('--touched-gate') || !!fixRange?.touched_gate, pass1Depth, rangeRuntimeLines: fixRange?.runtime_lines, granted: grant, fixChecks }, + history: { fullPasses: pass, last, fixedBlocking: has('--fixed-blocking'), fixedBelow: has('--fixed-below') && last !== 'fix-check', blockingOpen: has('--blocking-open'), touchedGate: has('--touched-gate') || !!fixRange?.touched_gate, pass1Depth, rangeRuntimeLines: fixRange?.runtime_lines, granted: grant }, }); const gateScan = { tagged: gateTagged, head }; if (has('--json')) { diff --git a/changelog.d/gst-967-gate-voices.md b/changelog.d/gst-967-gate-voices.md index b7f5fa514..7ae04b0e1 100644 --- a/changelog.d/gst-967-gate-voices.md +++ b/changelog.d/gst-967-gate-voices.md @@ -1,8 +1,9 @@ -**/ship now runs Step 11's voices before any gate-class stop, and a recorded owner grant runs its fix-check.** On a gate-class change, a blocker fixed in review round 2 used to stop the run for the owner before Step 11 ran. If the owner then chose "ship with open notes", gate code shipped with no adversarial, outside or targeted review (C6-36 after-opus). `gstack-review-plan` now answers `NEXT: voices` for any gate-class stop while Step 11 has not run. /ship runs Step 11 as it does on `done`, asks the plan again with `--adversarial-ran`, and the stop then stands. The round cap still limits fix rounds; it no longer skips the required voices. Second, the owner's grant of one scoped fix-check after round 2 had no way in: the plan took no grant input, and the pass program printed `PASS: none next=stop`. Now `gstack-review-plan --grant ` checks that the branch's `gstack-approvals` record holds that id as approved (exit 3 if not), and plans the one fix-check in place of the round-2 stop. The pass program then dispatches that fix-check like any other. After a fix-check, `--fix-checks ` is required with `--grant`, so a granted fix-check never repeats. +**/ship now runs Step 11's voices before any gate-class stop, and a recorded owner grant runs its fix-check.** On a gate-class change, a blocker fixed in review round 2 used to stop the run for the owner before Step 11 ran. If the owner then chose "ship with open notes", gate code shipped with no adversarial, outside or targeted review (C6-36 after-opus). `gstack-review-plan` now answers `NEXT: voices` for any gate-class stop while Step 11 has not run. /ship runs Step 11 as it does on `done`, asks the plan again with `--adversarial-ran`, and the stop then stands. The round cap still limits fix rounds; it no longer skips the required voices. Second, the owner's grant of one scoped fix-check after round 2 had no way in: the plan took no grant input, and the pass program printed `PASS: none next=stop`. Now `gstack-review-plan --grant ` checks that the branch's `gstack-approvals` record holds that id as an approved grant naming `review_loop_exception` (exit 3 for any other row), and plans the one fix-check in place of the stop after a round-2 full pass. The pass program then dispatches that fix-check like any other. A fix-check never earns another, so the grant cannot repeat. After `NEXT: voices`, a blocking finding the voices queue goes back to the plan as `--blocking-open` instead of starting a new review pass. ### Tests - `test/review-depth.test.ts`: a gate-class round-2 blocker fix plans `voices` (adversarial, outside and targeted) before every stop and stops once they ran. A grant plans one fix-check after round 2, after the voices on gate class, and never a second. The transitions table and rule 3 in `docs/review-policy.md` say both. All four failed at main 1022f4f. -- `test/review-plan-cli.test.ts`: `--grant` with a recorded approval prints `NEXT: fix-check`, an unrecorded id exits 3, and `--grant` after `--last fix-check` without `--fix-checks` exits 2. Failed at main 1022f4f (`NEXT: stop`). +- `test/review-plan-cli.test.ts`: `--grant` with a recorded grant prints `NEXT: fix-check`; an unrelated approval or an unrecorded id exits 3; after a fix-check the grant never plans another. Failed at main 1022f4f (`NEXT: stop`). +- `test/review-depth.test.ts`: an invariant over every transitions-table state (gate class or not, Step 11 run or not, grant or not) and a dispatch-superset no-loss check against main's plans. - `test/review-pass.test.ts`: the pass program runs the granted fix-check reader instead of `PASS: none next=stop`. Failed at main 1022f4f. ### Docs diff --git a/docs/review-policy.md b/docs/review-policy.md index b1d55459e..5d2d40e82 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -103,11 +103,12 @@ incomplete coverage, which blocks until the reviewer is rerun. (`NEXT: voices`), so gate code never reaches the owner's "ship with open notes" without them (GST-967). A third round is the owner's grant of one scoped fix-check (recorded in `review_loop_exception`), never the - loop's: once `gstack-approvals add` records the grant, - `gstack-review-plan --grant ` plans that fix-check in place of a - round-2 stop for a fixed blocking finding or a gate-class edit (with - `--fix-checks ` after a fix-check), and refuses an id the branch's record - does not hold approved. The cap approves nothing. + loop's: once `gstack-approvals add` records the grant (its ask or scope + naming `review_loop_exception`), `gstack-review-plan --grant ` + plans that fix-check in place of the stop after a round-2 full pass with a + fixed blocking finding or a gate-class edit, and refuses any other row. A + fix-check never earns another, so the grant cannot repeat, and a stop after a + round-2 fix-check still goes to the owner. The cap approves nothing. ### Transitions @@ -130,7 +131,7 @@ incomplete coverage, which blocks until the reviewer is rerun. | a fix-check that found nothing blocking | done | | a fix-check that found a blocking finding | stop: name it for the owner | | a stop (any row above) on gate class before Step 11's voices ran | Step 11's voices (adversarial, outside, targeted), then ask again with `--adversarial-ran` | -| a round-2 stop for a fixed blocking finding or a gate-class edit, with the owner's recorded grant (`--grant `) | one scoped fix-check (round 3), after Step 11 on gate class | +| a round-2 full pass that stops for a fixed blocking finding or a gate-class edit, with the owner's recorded grant (`--grant `) | one scoped fix-check (round 3), after Step 11 on gate class | A **fix-check** is one reviewer reading the touched hunks, the fix list, the test results and the callers of every changed symbol, at full effort. It is @@ -138,9 +139,9 @@ logged as a `review-fix-check` row, so it never counts as a pass. No army, no Red Team and no outside voice run on it. `NEXT: voices` runs Step 11 as `done` does, from the last full pass's -manifest; /ship then asks the plan again with `--adversarial-ran` (plus the -flags of any fixes the voices queued), and the stop or the granted fix-check it -names stands. +manifest; /ship then asks the plan again with `--adversarial-ran` (a blocking +finding the voices queued counts as `--blocking-open`, never a new Step 9), and +the stop or the granted fix-check it names stands. A **scoped pass** is pass 2 after a blocking fix when pass 1 and the diff both diff --git a/lib/review-depth.ts b/lib/review-depth.ts index fe440df0c..37b1febe3 100644 --- a/lib/review-depth.ts +++ b/lib/review-depth.ts @@ -344,8 +344,6 @@ export interface History { adversarialRan?: boolean; /** The approval id of the owner's recorded grant of one scoped fix-check after round 2 (review_loop_exception). */ granted?: string; - /** Fix-checks already run, the step that just finished included; default 1 after a fix-check, else 0. */ - fixChecks?: number; } export interface NextStep { step: Step; reason: string } @@ -358,14 +356,14 @@ export interface NextStep { step: Step; reason: string } * one): after round 2, below-floor fixes finish with notes, and a blocker open or fixed * there, or a gate-class edit there, stops for the owner, whose grant may add one scoped fix-check. * The cap limits fix rounds, not Step 11's voices: on gate class any stop waits for them (GST-967), - * and the owner's recorded grant (`granted`) turns a round-2 stop for a fixed finding into its one fix-check. + * and the owner's recorded grant (`granted`) turns the stop after a round-2 full pass with a fixed finding into its + * one fix-check; a fix-check never earns another, so the grant cannot repeat. */ export function nextStep(h: History, depth?: Depth): NextStep { const n = loopStep(h, depth); if (n.step !== 'stop') return n; if (h.gate && !h.adversarialRan) return { step: 'voices', reason: `gate class: Step 11's voices (adversarial, outside, targeted) run before the owner's exit, then ask again with --adversarial-ran (${n.reason})` }; - const rounds = h.fullPasses + (h.fixChecks ?? (h.last === 'fix-check' ? 1 : 0)); - if (h.granted && !h.blockingOpen && rounds === FULL_PASS_CAP) return { step: 'fix-check', reason: `the owner's grant ${h.granted} adds one scoped fix-check after round ${FULL_PASS_CAP} (review_loop_exception), never a fourth round` }; + if (h.granted && !h.blockingOpen && h.last === 'full' && h.fullPasses === FULL_PASS_CAP) return { step: 'fix-check', reason: `the owner's grant ${h.granted} adds one scoped fix-check after round ${FULL_PASS_CAP} (review_loop_exception), never a fourth round` }; return n; } diff --git a/ship/sections/adversarial.md b/ship/sections/adversarial.md index a7fb81191..97f66a3d8 100644 --- a/ship/sections/adversarial.md +++ b/ship/sections/adversarial.md @@ -8,9 +8,9 @@ with neither line, take decision 3 of Finish the adversarial phase. On `DISPATCH (`gstack-review-dispatch render --role targeted --classes --base --diff-base `, the merge base on every pass, never RANGE_BASE) like the voices; `verify --targeted-out` feeds the receipt's `--review-targeted`. Under a token cap, reserve the planned voices as one batch (`gstack-ship-budget check --next dispatch:`) and add `--reservation ` to the `--start adversarial-review` call below. -After `NEXT: voices` (gate class: Step 11 before any stop), decision 3 of Finish the adversarial phase -reruns Step 9's repeat decision with `--adversarial-ran` instead; its `NEXT:` (the stop, or the owner's -granted fix-check) stands. +After `NEXT: voices` (gate class: Step 11 before any stop), Finish the adversarial phase takes neither +decision 2 nor 3: rerun Step 9's repeat decision with `--adversarial-ran` (a queued blocking finding as +`--blocking-open`); its `NEXT:` (the stop, or the owner's granted fix-check) stands. ## Step 11: Adversarial review diff --git a/ship/sections/adversarial.md.tmpl b/ship/sections/adversarial.md.tmpl index 744260b17..53bd90695 100644 --- a/ship/sections/adversarial.md.tmpl +++ b/ship/sections/adversarial.md.tmpl @@ -6,9 +6,9 @@ with neither line, take decision 3 of Finish the adversarial phase. On `DISPATCH (`gstack-review-dispatch render --role targeted --classes --base --diff-base `, the merge base on every pass, never RANGE_BASE) like the voices; `verify --targeted-out` feeds the receipt's `--review-targeted`. Under a token cap, reserve the planned voices as one batch (`gstack-ship-budget check --next dispatch:`) and add `--reservation ` to the `--start adversarial-review` call below. -After `NEXT: voices` (gate class: Step 11 before any stop), decision 3 of Finish the adversarial phase -reruns Step 9's repeat decision with `--adversarial-ran` instead; its `NEXT:` (the stop, or the owner's -granted fix-check) stands. +After `NEXT: voices` (gate class: Step 11 before any stop), Finish the adversarial phase takes neither +decision 2 nor 3: rerun Step 9's repeat decision with `--adversarial-ran` (a queued blocking finding as +`--blocking-open`); its `NEXT:` (the stop, or the owner's granted fix-check) stands. {{ADVERSARIAL_STEP}} diff --git a/ship/sections/review-army.md b/ship/sections/review-army.md index a3b615104..fa5cddae0 100644 --- a/ship/sections/review-army.md +++ b/ship/sections/review-army.md @@ -821,7 +821,7 @@ with each flag that holds (`--fixed-blocking`, `--fixed-below`, `--blocking-open `--touched-gate` for a fix to an item 2.5 path, `--critical-found` when this pass logged a CRITICAL, `--adversarial-ran` once Step 11's native pass ran, `--reviewed-numbers `, `--depth-floor ` when given, `--grant ` once the owner's grant of one scoped fix-check is -in `gstack-approvals`, with `--fix-checks ` after a fix-check), `--range-base $PASS_HEAD` and `--pass1-depth `; +in `gstack-approvals` naming `review_loop_exception`), `--range-base $PASS_HEAD` and `--pass1-depth `; its `NEXT:` alone picks the next decision: `stop` 2, `full` 3, `fix-check` 3.5, `done` 4, `voices` 4 (Step 11 runs before the stop). `scoped` is 3 too. A full pass 1 always gets a full pass 2. Other scopes apply decisions 2–4 in order. diff --git a/ship/sections/review-army.md.tmpl b/ship/sections/review-army.md.tmpl index 4622b4054..198512d0e 100644 --- a/ship/sections/review-army.md.tmpl +++ b/ship/sections/review-army.md.tmpl @@ -230,7 +230,7 @@ with each flag that holds (`--fixed-blocking`, `--fixed-below`, `--blocking-open `--touched-gate` for a fix to an item 2.5 path, `--critical-found` when this pass logged a CRITICAL, `--adversarial-ran` once Step 11's native pass ran, `--reviewed-numbers `, `--depth-floor ` when given, `--grant ` once the owner's grant of one scoped fix-check is -in `gstack-approvals`, with `--fix-checks ` after a fix-check), `--range-base $PASS_HEAD` and `--pass1-depth `; +in `gstack-approvals` naming `review_loop_exception`), `--range-base $PASS_HEAD` and `--pass1-depth `; its `NEXT:` alone picks the next decision: `stop` 2, `full` 3, `fix-check` 3.5, `done` 4, `voices` 4 (Step 11 runs before the stop). `scoped` is 3 too. A full pass 1 always gets a full pass 2. Other scopes apply decisions 2–4 in order. diff --git a/test/review-depth.test.ts b/test/review-depth.test.ts index b80aa0893..002da4b4c 100644 --- a/test/review-depth.test.ts +++ b/test/review-depth.test.ts @@ -88,24 +88,23 @@ describe('GST-967: the round cap limits fix rounds, not the gate-class voices; a expect([p.class, p.next.step, roles(p)]).toEqual(['gate', 'voices', ['adversarial-native:adversarial:full', 'outside-voice:outside-voice:host', 'targeted:targeted:full']]); expect(planReview(base({ gateTagged: 1, adversarialRan: true, targeted: { classes: ['gate'] }, history: h })).next.step).toBe('stop'); }); - test('a recorded grant dispatches one scoped fix-check after round 2, after the gate-class voices, and never a second', () => { + test('a recorded grant dispatches one scoped fix-check after a round-2 full pass, after the gate-class voices, and never a second', () => { const g = { ...capStop, granted: 'a-feb4aba31633' }; expect(nextStep(g)).toMatchObject({ step: 'fix-check', reason: expect.stringContaining('a-feb4aba31633') }); expect(roles(planReview(base({ history: g })))).toEqual(['fix-check:fix-check:full']); expect(nextStep({ ...g, gate: true }).step).toBe('voices'); expect(nextStep({ ...g, gate: true, adversarialRan: true }).step).toBe('fix-check'); expect(nextStep({ ...g, fixedBlocking: false, fixedBelow: true, touchedGate: true }).step).toBe('fix-check'); - expect(nextStep({ fullPasses: 1, last: 'fix-check', fixedBlocking: true, granted: 'a-1', fixChecks: 1 }).step).toBe('fix-check'); + expect(nextStep({ fullPasses: 1, last: 'fix-check', fixedBlocking: true, granted: 'a-1' }).step).toBe('stop'); expect(nextStep({ ...g, blockingOpen: true }).step).toBe('stop'); expect(nextStep({ fullPasses: FULL_PASS_CAP, last: 'fix-check', granted: 'a-1' }).step).toBe('done'); expect(nextStep({ fullPasses: FULL_PASS_CAP, last: 'fix-check', fixedBlocking: true, granted: 'a-1' }).step).toBe('stop'); - expect(nextStep({ fullPasses: 1, last: 'fix-check', fixedBlocking: true, granted: 'a-1', fixChecks: 2 }).step).toBe('stop'); }); test('docs/review-policy.md\'s transitions table and rule 3 say both', () => { const doc = fs.readFileSync(path.join(ROOT, 'docs', 'review-policy.md'), 'utf8'); const table = doc.slice(doc.indexOf(''), doc.indexOf('A **fix-check** is one reviewer')); expect(table).toContain("| a stop (any row above) on gate class before Step 11's voices ran | Step 11's voices (adversarial, outside, targeted), then ask again with `--adversarial-ran` |"); - expect(table).toContain('| a round-2 stop for a fixed blocking finding or a gate-class edit, with the owner\'s recorded grant (`--grant `) | one scoped fix-check (round 3), after Step 11 on gate class |'); + expect(table).toContain('| a round-2 full pass that stops for a fixed blocking finding or a gate-class edit, with the owner\'s recorded grant (`--grant `) | one scoped fix-check (round 3), after Step 11 on gate class |'); const rule3 = doc.slice(doc.indexOf('3. **Two rounds is the cap**'), doc.indexOf('')).replace(/\s+/g, ' '); expect(rule3).toContain("The cap limits fix rounds, not Step 11's voices"); expect(rule3).toContain('`gstack-review-plan --grant `'); @@ -115,18 +114,17 @@ describe('GST-967: the round cap limits fix rounds, not the gate-class voices; a /** * GST-967's safety property, as a permanent invariant over the transitions table (Captain, 2026-11-02: the invariant is the * gate, a reader the second check). Every state: passes 0-3, each last step, every mix of the four fix flags, gate class or - * not, Step 11 run or not, grant recorded or not, fix-checks counted or defaulted. + * not, Step 11 run or not, grant recorded or not. */ describe('GST-967 invariant: no gate-class owner exit before Step 11\'s voices; a recorded grant always gets its fix-check', () => { - // Value: protects=a gate-class plan never stops (the exit where the owner may ship with open notes) before Step 11's adversarial, outside and targeted voices ran, voices is planned only then and dispatches all three, and a recorded grant always turns a round-2 stop for a fixed finding into exactly one fix-check while changing nothing else; fails_when=any state breaks one of those, so a later edit to nextStep cannot fail open; + // Value: protects=a gate-class plan never stops (the exit where the owner may ship with open notes) before Step 11's adversarial, outside and targeted voices ran, voices is planned only then and dispatches all three, and a recorded grant always turns the stop after a round-2 full pass with a fixed finding into exactly one fix-check while changing nothing else, so it never repeats; fails_when=any state breaks one of those, so a later edit to nextStep cannot fail open; // why_new=GST-967 (three earlier fixes failed open and a reviewer caught each after the fact); seam=nextStep, planReview; no_claim=the parent following NEXT test('every state of the table holds the property', () => { let states = 0, voices = 0, grants = 0; for (const fullPasses of [0, 1, 2, 3]) for (const last of ['none', 'full', 'fix-check'] as const) for (let bits = 0; bits < 16; bits++) - for (const gate of [false, true]) for (const adversarialRan of [false, true]) for (const fixChecks of [undefined, 0, 1, 2]) { - const h = { fullPasses, last, fixedBlocking: !!(bits & 1), fixedBelow: !!(bits & 2), blockingOpen: !!(bits & 4), touchedGate: !!(bits & 8), gate, adversarialRan, fixChecks }; + for (const gate of [false, true]) for (const adversarialRan of [false, true]) { + const h = { fullPasses, last, fixedBlocking: !!(bits & 1), fixedBelow: !!(bits & 2), blockingOpen: !!(bits & 4), touchedGate: !!(bits & 8), gate, adversarialRan }; const plain = nextStep(h), granted = nextStep({ ...h, granted: 'a-1' }); - const rounds = fullPasses + (fixChecks ?? (last === 'fix-check' ? 1 : 0)); const ungatedStop = nextStep({ ...h, gate: false }).step === 'stop'; states++; for (const n of [plain, granted]) { @@ -134,12 +132,12 @@ describe('GST-967 invariant: no gate-class owner exit before Step 11\'s voices; if (n.step === 'voices') expect([gate, adversarialRan, ungatedStop]).toEqual([true, false, true]); } if (plain.step === 'voices') voices++; - const owed = ungatedStop && !h.blockingOpen && rounds === FULL_PASS_CAP && (!gate || adversarialRan); + const owed = ungatedStop && !h.blockingOpen && last === 'full' && fullPasses === FULL_PASS_CAP && (!gate || adversarialRan); if (owed) grants++; expect([granted.step === 'fix-check' && plain.step !== 'fix-check', granted.step === plain.step]).toEqual([owed, !owed]); if (owed) expect(granted.reason).toContain('a-1'); } - expect(states).toBe(4 * 3 * 16 * 2 * 2 * 4); + expect(states).toBe(4 * 3 * 16 * 2 * 2); expect([voices > 0, grants > 0]).toEqual([true, true]); const gatePlan = (o: Partial) => planReview(base({ gateTagged: 1, targeted: { classes: ['gate'] }, ...o })); for (const fullPasses of [1, 2]) for (const last of ['full', 'fix-check'] as const) for (let bits = 0; bits < 16; bits++) { diff --git a/test/review-packets-removed.test.ts b/test/review-packets-removed.test.ts index 10c7a86a1..aa1d856ab 100644 --- a/test/review-packets-removed.test.ts +++ b/test/review-packets-removed.test.ts @@ -65,8 +65,8 @@ const NO_LOSS_REPLAYS = { } as const; type Replay = keyof typeof NO_LOSS_REPLAYS; const REVIEWER_BRIEFS: Record = { - 'ship/sections/review-army.md': { sha: '12f0e686fc50bccdcd5d0ee30b1f2925e47cde11ac83c008916114029e17cce5', replay: 'GST-967' }, - 'ship/sections/adversarial.md': { sha: 'd83e95536d8e380c13e6282b93fd6a7deaa77dc1584e7325bd87b1601d1f44fb', replay: 'GST-967' }, + 'ship/sections/review-army.md': { sha: 'c22f1da7f5ce7ecfd8d83fdaa135ed2fd1207d7db6f04321bd92b1524fd70c94', replay: 'GST-967' }, + 'ship/sections/adversarial.md': { sha: '5d712bb568573931108a79b1bd40b74db6189610f47c547099c13098ef67bef8', replay: 'GST-967' }, 'review/sections/review-army.md': { sha: '78329b64640260cd8831ca81a9189f3f8dcade9c406431631d3f2cd798cd3967', replay: '#3638' }, 'review/sections/adversarial.md': { sha: 'e6962fa25b78f362654f2954f79683e524d2ebd452d1494f8a2b099888e8e255', replay: 'G1' }, 'review/checklist.md': { sha: '607788afeb2c83f2dff1dccd58d212616ec72393d28cda2fa18690c1a83df699', replay: '#3638' }, @@ -83,7 +83,7 @@ const REVIEWER_BRIEFS: Record = { 'review/specialists/testing.md': { sha: 'd6cc2bef7be562387d0c0a22fefe646dff9c295f1d34d8453a6cb496584150e4', replay: '#3638' }, 'review/specialists/targeted.md': { sha: '7f92a0d0a8319c3c0e076fb3b1a4689c45120b2be2c14dfc7d4cb03aafa9d6a5', replay: 'GST-257' }, 'scripts/resolvers/fix-check.ts': { sha: '712bf5b0c439e96bac32f19cbe25394dabb4c94aa66600100bfde9b100e15a08', replay: '#3638' }, - 'lib/review-depth.ts': { sha: '7768bfa3e31b914432dbebd3dcc574ae132d1edd5b7dee23dea3d2d1465aae07', replay: 'GST-967' }, + 'lib/review-depth.ts': { sha: 'c5fd958b6c9a1116be9fc3418844239c01168476abb72c1db85bc26ab0adf4b4', replay: 'GST-967' }, 'lib/review-risk.ts': { sha: '126eff0f1de6982e03dc24d8017c67e6807c91dfa518b18d2db3aa910aac911f', replay: 'GST-911' }, }; diff --git a/test/review-pass.test.ts b/test/review-pass.test.ts index 1dcb2ec5d..8f2fddc20 100644 --- a/test/review-pass.test.ts +++ b/test/review-pass.test.ts @@ -744,7 +744,7 @@ describe('Capy pass program (fake host, real bins)', () => { // why_new=GST-967; seam=the pass program; no_claim=the fix-check reviewer's verdict test('GST-967: a recorded grant after round 2 runs its one scoped fix-check through the program', async () => { const { dir, env } = repo(); - const add = spawnSync(path.join(BIN, 'gstack-approvals'), ['add', '--ask', 'Owner grant: one scoped fix-check after round 2', '--words', 'one scoped fix-check after round 2', '--by', 'GStack Minder', '--scope', 'this /ship invocation', '--json'], { cwd: dir, env: { ...env, GSTACK_APPROVALS_MIRROR: '0' }, encoding: 'utf8', timeout: 60_000 }); + const add = spawnSync(path.join(BIN, 'gstack-approvals'), ['add', '--ask', 'Owner grant: one scoped fix-check after round 2 (review_loop_exception)', '--words', 'one scoped fix-check after round 2', '--by', 'GStack Minder', '--scope', 'this /ship invocation', '--json'], { cwd: dir, env: { ...env, GSTACK_APPROVALS_MIRROR: '0' }, encoding: 'utf8', timeout: 60_000 }); expect(add.status, add.stderr).toBe(0); const id = JSON.parse(add.stdout).added[0] as string; const files = path.join(dir, '.state/gfc'); diff --git a/test/review-plan-cli.test.ts b/test/review-plan-cli.test.ts index 04045ff5a..cd1dee108 100644 --- a/test/review-plan-cli.test.ts +++ b/test/review-plan-cli.test.ts @@ -62,15 +62,19 @@ describe('gstack-review-plan', () => { expect(r.stdout).not.toContain('NEXT: stop'); expect(run(repo({ 'src/a.ts': 'export const a = 1;\n' }), ['--pass', '1', '--last', 'fix-check', '--fixed-below', '--blocking-open']).stdout).toContain('NEXT: stop'); }); - // Value: protects=a grant the owner recorded with gstack-approvals is something the plan can use: --grant plans the one scoped fix-check after round 2, a fix-check already counted keeps it from repeating, and an id the record does not hold is refused; fails_when=the CLI ignores the grant and prints NEXT: stop (C6-36 after-opus-grant), accepts an unrecorded id, or grants a second fix-check; + // Value: protects=a grant the owner recorded with gstack-approvals is something the plan can use: --grant plans the one scoped fix-check after a round-2 full pass, the fix-check never earns another, and an id the record does not hold as a review_loop_exception grant is refused; fails_when=the CLI ignores the grant and prints NEXT: stop (C6-36 after-opus-grant), accepts an unrelated approval or an unrecorded id, or grants a second fix-check; // why_new=GST-967; seam=bin/gstack-review-plan; no_claim=who may grant (the owner's words are the approval record's) test('GST-967: --grant names a recorded approval and plans its one scoped fix-check after round 2; an unrecorded id is refused', () => { const r = repo({ 'src/a.ts': 'export const a = 1;\n' }); const env: NodeJS.ProcessEnv = { ...process.env, GSTACK_STATE_ROOT: r.state, GSTACK_APPROVALS_MIRROR: '0' }; delete env.GSTACK_APPROVALS_DIR; - const add = spawnSync(path.join(path.dirname(BIN), 'gstack-approvals'), ['add', '--ask', 'Owner grant: one scoped fix-check after round 2', '--words', 'one scoped fix-check after round 2', '--by', 'GStack Minder', '--scope', 'this /ship invocation', '--json'], { cwd: r.dir, encoding: 'utf8', timeout: 60_000, env }); - expect(add.status, add.stderr).toBe(0); - const id = JSON.parse(add.stdout).added[0] as string; + const add = (ask: string) => { + const a = spawnSync(path.join(path.dirname(BIN), 'gstack-approvals'), ['add', '--ask', ask, '--words', 'do it', '--by', 'GStack Minder', '--scope', 'this /ship invocation', '--json'], { cwd: r.dir, encoding: 'utf8', timeout: 60_000, env }); + expect(a.status, a.stderr).toBe(0); + return JSON.parse(a.stdout).added[0] as string; + }; + const id = add('Owner grant: one scoped fix-check after round 2 (review_loop_exception)'); + const unrelated = add('Approve the plan for the README rewrite'); const plan = (a: string[]) => spawnSync(BIN, ['--base', 'main', '--gate-tagged', '0', ...a], { cwd: r.dir, encoding: 'utf8', timeout: 60_000, env }); const capStop = ['--pass', '2', '--last', 'full', '--fixed-blocking']; expect(plan(capStop).stdout).toContain('NEXT: stop'); @@ -78,12 +82,14 @@ describe('gstack-review-plan', () => { expect(granted.status, granted.stderr).toBe(0); expect(granted.stdout).toContain(`NEXT: fix-check (the owner's grant ${id}`); expect(granted.stdout).toContain('DISPATCH: fix-check fix-check effort=full'); - expect(plan(['--pass', '2', '--last', 'fix-check', '--fixed-blocking', '--grant', id, '--fix-checks', '1']).stdout).toContain('NEXT: stop'); - const uncounted = plan(['--pass', '1', '--last', 'fix-check', '--fixed-blocking', '--grant', id]); - expect([uncounted.status, uncounted.stderr]).toEqual([2, expect.stringContaining('--fix-checks')]); - const unknown = plan([...capStop, '--grant', 'a-000000000000']); - expect([unknown.status, unknown.stdout]).toEqual([3, '']); - expect(unknown.stderr).toContain('no approved row a-000000000000'); + expect(plan(['--pass', '2', '--last', 'fix-check', '--fixed-blocking', '--grant', id]).stdout).toContain('NEXT: stop'); + expect(plan(['--pass', '1', '--last', 'fix-check', '--fixed-blocking', '--grant', id]).stdout).toContain('NEXT: stop'); + for (const other of ['a-000000000000', unrelated]) { + const refused = plan([...capStop, '--grant', other]); + expect([refused.status, refused.stdout]).toEqual([3, '']); + expect(refused.stderr).toContain(`no approved row ${other} naming review_loop_exception`); + } + expect(plan([...capStop, '--grant', '--json']).status).toBe(2); }); test('--fix-check-prompt renders the fix-check reviewer prompt from the parent\'s files and refuses a missing one', () => { const r = repo({ 'docs/howto.md': '# x\n' }); diff --git a/test/size-ledger/ship/gst-967-gate-voices.json b/test/size-ledger/ship/gst-967-gate-voices.json index c2a168eef..d452e50fd 100644 --- a/test/size-ledger/ship/gst-967-gate-voices.json +++ b/test/size-ledger/ship/gst-967-gate-voices.json @@ -1,4 +1,4 @@ { - "bytes": 417, + "bytes": 463, "why": "GST-967: Step 11 voices before any gate-class stop (NEXT: voices) and the owner's recorded grant (--grant) in the repeat decision and Step 11" }