diff --git a/bin/gstack-review-plan b/bin/gstack-review-plan index 40610335b..2ef4d2735 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 ] [--json] + * [--targeted-ran ] [--grant [--fix-checks ]] [--json] * gstack-review-plan --fix-check-prompt --fix-list --hunks --tests [--callers ] * * Reads the diff against the merge base with (committed, tracked uncommitted @@ -21,6 +21,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { spawnSync } from 'node:child_process'; +import { locate, readRecord } from '../lib/approval-record'; import { EXIT, exitTableHelp } from '../lib/headless-artifacts'; import { runScriptSync } from '../lib/script-invocation'; import { collectCommit, collectLive } from '../lib/review-collect'; @@ -54,12 +55,18 @@ 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 --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; standard only by --depth-floor or a raise; otherwise light. TARGETED: the money, gate, auth and secret classes the diff touches; any class dispatches the targeted reader at every depth. NEXT: full, scoped -(a full pass on the range: fix-check, Red Team, native adversarial), fix-check, stop or done. A full plan +(a full pass on the range: fix-check, Red Team, native adversarial), fix-check, voices (gate class: Step 11 before any stop, +then ask again with --adversarial-ran), stop or done. A full plan from an error exits 0; exit 1 means no plan could be built (git could not list the diff). ${exitTableHelp()}`; @@ -92,11 +99,21 @@ 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(); if (!root) { process.stderr.write('gstack-review-plan: run it inside a git checkout\n'); return EXIT.fail; } process.chdir(root); + 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; } + } const collected = collectLive(root, base); if (collected.errors.length) { for (const e of collected.errors) process.stderr.write(`gstack-review-plan: git could not list the diff (${e.input}: ${e.detail}); fix: ${e.repair}; no plan is safer than a plan for an empty diff\n`); @@ -145,7 +162,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 }, + 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 }, }); 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 new file mode 100644 index 000000000..b7f5fa514 --- /dev/null +++ b/changelog.d/gst-967-gate-voices.md @@ -0,0 +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. + +### 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-pass.test.ts`: the pass program runs the granted fix-check reader instead of `PASS: none next=stop`. Failed at main 1022f4f. + +### Docs +- `docs/review-policy.md`: rule 3 and two new rows in the transitions table. `ship/DRIVER.md`, `ship/sections/review-army.md` and `ship/sections/adversarial.md` route `voices` and `--grant`. diff --git a/docs/review-policy.md b/docs/review-policy.md index 16e93e891..b1d55459e 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -98,9 +98,16 @@ incomplete coverage, which blocks until the reviewer is rerun. finishes with its open notes listed in the PR body and receipt, or stops for the owner when a blocking finding is open, or was fixed in round 2 and is unverified, or a round-2 fix edited a gate-class path, naming it (path, - severity, problem). A third round is the owner's + severity, problem). The cap limits fix rounds, not Step 11's voices: on gate + class a stop waits for Step 11's adversarial, outside and targeted voices + (`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. The cap approves nothing. + 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. ### Transitions @@ -122,12 +129,19 @@ incomplete coverage, which blocks until the reviewer is rerun. | a full pass that leaves a blocking finding open (fixed or not) | stop: name it for the owner | | 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 **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 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. + A **scoped pass** is pass 2 after a blocking fix when pass 1 and the diff both ran below full depth (no gate class, no depth error, no `--depth-floor full`). diff --git a/lib/review-depth.ts b/lib/review-depth.ts index 59340ac8e..fe440df0c 100644 --- a/lib/review-depth.ts +++ b/lib/review-depth.ts @@ -20,7 +20,7 @@ export type PathClass = 'docs' | 'tests' | 'code'; export type Depth = 'light' | 'standard' | 'full'; export type Effort = 'light' | 'medium' | 'full' | 'host'; export type Role = 'specialist' | 'red-team' | 'adversarial-native' | 'outside-voice' | 'fix-check' | 'targeted'; -export type Step = 'full' | 'scoped' | 'fix-check' | 'done' | 'stop'; +export type Step = 'full' | 'scoped' | 'fix-check' | 'voices' | 'done' | 'stop'; export const SPECIALISTS = ['testing', 'maintainability', 'security', 'performance', 'data-migration', 'api-contract', 'design', 'simplification', 'state-machines'] as const; export type Specialist = (typeof SPECIALISTS)[number]; @@ -338,6 +338,14 @@ export interface History { /** Runtime lines of the fix range (RANGE_BASE..HEAD) as classifyDiff counts them, 0 when it touches none; undefined or null when the range was not read. */ rangeRuntimeLines?: number | null; pass1Depth?: Depth; + /** The diff is gate class (planReview sets it): Step 11's voices run before any stop. */ + gate?: boolean; + /** Step 11's native adversarial pass already ran (planReview sets it from PlanInput.adversarialRan). */ + 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 } @@ -349,8 +357,19 @@ export interface NextStep { step: Step; reason: string } * changes no runtime lines; two rounds is the cap (R51: a scoped pass or a fix-check is * 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. */ 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` }; + return n; +} + +function loopStep(h: History, depth?: Depth): NextStep { if (h.last === 'none') return { step: 'full', reason: 'pass 1 reviews the whole diff' }; const fixed = !!(h.fixedBlocking || h.fixedBelow); if (h.last === 'fix-check') return h.blockingOpen || h.fixedBlocking || h.touchedGate ? { step: 'stop', reason: `the fix-check ${h.touchedGate && !h.blockingOpen && !h.fixedBlocking ? 'edited a gate-class path' : 'found a blocking finding'}: name it for the owner, whose grant may add one scoped fix-check, never a third round` } : { step: 'done', reason: fixed ? 'round cap 2 reached: the below-floor fixes ship as notes in the PR body and receipt' : 'the fix-check found nothing blocking' }; @@ -475,7 +494,7 @@ export function planReview(i: PlanInput): ReviewPlan { const c = classifyDiff(i); const depth = decideDepth(c, c.runtimeLines ?? (CLASS_RANK[c.class] >= CLASS_RANK.code ? i.diffLines : 0), i.floor); const { class: cls, runtimeLines } = depth; - const next = nextStep(i.history, depth.depth); + const next = nextStep({ ...i.history, gate: cls === 'gate', adversarialRan: !!i.adversarialRan }, depth.depth); const h = i.history; const raise = depth.depth === 'light' && (h.fixedBlocking || h.blockingOpen || i.criticalFound); const stepDepth: Depth = raise ? 'standard' : depth.depth; @@ -535,8 +554,11 @@ export function planReview(i: PlanInput): ReviewPlan { } else if (next.step === 'fix-check') { add('fix-check', 'fix-check', 'full'); if (numbersChanged) add('outside-voice', 'outside-voice (numbers changed)', 'host'); + } else if (next.step === 'voices') { + add('adversarial-native', 'adversarial', 'full'); + add('outside-voice', 'outside-voice', 'host'); } - if (i.targeted?.classes.length && ['full', 'scoped', 'fix-check'].includes(next.step)) { + if (i.targeted?.classes.length && ['full', 'scoped', 'fix-check', 'voices'].includes(next.step)) { if (i.targeted.ran) skipped.push('targeted (already read this product patch)'); else add('targeted', 'targeted', ROLE_EFFORT.targeted); } diff --git a/scripts/review-dispatch-compare.ts b/scripts/review-dispatch-compare.ts index ad8790dad..efe817232 100755 --- a/scripts/review-dispatch-compare.ts +++ b/scripts/review-dispatch-compare.ts @@ -85,6 +85,7 @@ function newShip(f: Fixture, sc: Scenario): { calls: Call[]; ending: string } { const diffBytes = plan.next.step === 'fix-check' ? f.diff_bytes * FIX_CHECK_FRACTION : h.fullPasses === 0 ? f.diff_bytes : f.diff_bytes * RANGE_FRACTION; for (const d of plan.dispatches) calls.push({ role: d.role, name: d.name, effort: d.effort, diffBytes: d.role === 'outside-voice' ? f.diff_bytes : diffBytes }); adversarialRan ||= plan.dispatches.some(d => d.role === 'adversarial-native'); + if (plan.next.step === 'voices') continue; const o = outcomes[sc][Math.min(i, outcomes[sc].length - 1)] ?? {}; const last = plan.next.step === 'scoped' ? 'full' : plan.next.step; h = { fullPasses: h.fullPasses + (last === 'full' ? 1 : 0), last, ...o }; diff --git a/ship/DRIVER.md b/ship/DRIVER.md index a7da6798b..ca721a499 100644 --- a/ship/DRIVER.md +++ b/ship/DRIVER.md @@ -84,11 +84,10 @@ reader ran. ## 4. Judgment: Steps 8.1, 8.2, 9.3, 9.4 and Step 11 Step 8.1, 9.2.1's probes, 8.2 (`Scope Check:`) on the todos and commits files, then the core checklist, Steps -9.3, 9.4 and the repeat decision as written (`gstack-review-plan` included; -`JUDGMENT: ok` before any fix), fixes and persistence in +9.3, 9.4 and the repeat decision as written (`JUDGMENT: ok` before any fix), fixes and persistence in one program. Prints `Pre-Landing Review:`, `Gate edits:`. `NEXT: stop` stops; `full` or `scoped`: one program runs the failing test, fix, `--tests-only` and Phase 3, each gated (`diffBase` RANGE_BASE on `full` only; scoped fix-check: Step 3.5); `fix-check`: the -pass with `fixCheck: {fix_list, hunks, tests, callers}`; `done`: that program runs Step 11 from +pass with `fixCheck: {fix_list, hunks, tests, callers}`; `done` or `voices`: that program runs Step 11 from `sections/adversarial.md` as written (above), only the voices the last full-pass plan lists (neither prints `ADVERSARIAL: skipped (review plan)`). diff --git a/ship/DRIVER.md.tmpl b/ship/DRIVER.md.tmpl index f7b98d4c8..a0f0d8ac1 100644 --- a/ship/DRIVER.md.tmpl +++ b/ship/DRIVER.md.tmpl @@ -82,11 +82,10 @@ reader ran. ## 4. Judgment: Steps 8.1, 8.2, 9.3, 9.4 and Step 11 Step 8.1, 9.2.1's probes, 8.2 (`Scope Check:`) on the todos and commits files, then the core checklist, Steps -9.3, 9.4 and the repeat decision as written (`gstack-review-plan` included; -`JUDGMENT: ok` before any fix), fixes and persistence in +9.3, 9.4 and the repeat decision as written (`JUDGMENT: ok` before any fix), fixes and persistence in one program. Prints `Pre-Landing Review:`, `Gate edits:`. `NEXT: stop` stops; `full` or `scoped`: one program runs the failing test, fix, `--tests-only` and Phase 3, each gated (`diffBase` RANGE_BASE on `full` only; scoped fix-check: Step 3.5); `fix-check`: the -pass with `fixCheck: {fix_list, hunks, tests, callers}`; `done`: that program runs Step 11 from +pass with `fixCheck: {fix_list, hunks, tests, callers}`; `done` or `voices`: that program runs Step 11 from `sections/adversarial.md` as written (above), only the voices the last full-pass plan lists (neither prints `ADVERSARIAL: skipped (review plan)`). diff --git a/ship/sections/adversarial.md b/ship/sections/adversarial.md index 0bc8600b3..a7fb81191 100644 --- a/ship/sections/adversarial.md +++ b/ship/sections/adversarial.md @@ -8,6 +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. ## Step 11: Adversarial review diff --git a/ship/sections/adversarial.md.tmpl b/ship/sections/adversarial.md.tmpl index ea99ddfdd..744260b17 100644 --- a/ship/sections/adversarial.md.tmpl +++ b/ship/sections/adversarial.md.tmpl @@ -6,6 +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. {{ADVERSARIAL_STEP}} diff --git a/ship/sections/review-army.md b/ship/sections/review-army.md index 450f3adae..a3b615104 100644 --- a/ship/sections/review-army.md +++ b/ship/sections/review-army.md @@ -820,8 +820,9 @@ Decision 1 always applies first. Then, under `frozen`, run `gstack-review-plan - 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), `--range-base $PASS_HEAD` and `--pass1-depth `; -its `NEXT:` alone picks the next decision: `stop` 2, `full` 3, `fix-check` 3.5, `done` 4. +`--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 `; +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 8b349c4e9..4622b4054 100644 --- a/ship/sections/review-army.md.tmpl +++ b/ship/sections/review-army.md.tmpl @@ -229,8 +229,9 @@ Decision 1 always applies first. Then, under `frozen`, run `gstack-review-plan - 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), `--range-base $PASS_HEAD` and `--pass1-depth `; -its `NEXT:` alone picks the next decision: `stop` 2, `full` 3, `fix-check` 3.5, `done` 4. +`--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 `; +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 4769dd47c..084d5ce87 100644 --- a/test/review-depth.test.ts +++ b/test/review-depth.test.ts @@ -70,6 +70,46 @@ describe('nextStep: the transition table', () => { test('a clean fix-check is done', () => expect(nextStep({ fullPasses: 1, last: 'fix-check' }).step).toBe('done')); }); +// Value: protects=on gate class Step 11's adversarial, outside and targeted voices run before any owner exit the round cap reaches, and a recorded owner grant dispatches its one scoped fix-check after round 2, once; fails_when=a gate-class round-2 blocker fix stops before Step 11 (C6-36 after-opus: next=stop, no voice ran), or a recorded grant still stops (C6-36 after-opus-grant: the lane hand-dispatched the fix-check); +// why_new=GST-967; seam=nextStep, planReview; no_claim=the voices' or the fix-check's verdicts +describe('GST-967: the round cap limits fix rounds, not the gate-class voices; a recorded grant runs its fix-check', () => { + const capStop = { fullPasses: FULL_PASS_CAP, last: 'full', fixedBlocking: true } as const; + test('a gate-class round-2 blocker fix runs Step 11\'s voices before the owner stop, then stops', () => { + expect(nextStep({ ...capStop, gate: true })).toMatchObject({ step: 'voices', reason: expect.stringContaining("Step 11's voices") }); + for (const o of [{ blockingOpen: true }, { fixedBlocking: false, fixedBelow: true, touchedGate: true }, { fullPasses: 1, last: 'fix-check' as const }, { fullPasses: 1, blockingOpen: true }]) expect(nextStep({ ...capStop, gate: true, ...o }).step).toBe('voices'); + expect(nextStep({ ...capStop, gate: true, adversarialRan: true }).step).toBe('stop'); + expect(nextStep(capStop).step).toBe('stop'); + }); + test('C6-36 run 1 replanned: the plan lists Step 11\'s three voices, and once they ran the stop stands', () => { + const h = { ...capStop, touchedGate: true }; + const p = planReview(base({ gateTagged: 1, targeted: { classes: ['gate'] }, history: h })); + 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', () => { + 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({ ...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 |'); + 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 `'); + }); +}); + describe('scanNumbers', () => { test('finds claims and fingerprints them', () => { const a = scanNumbers({ 'docs/bench.md': ['Median dropped 48% to 1.2s', 'no numbers here'] }); diff --git a/test/review-pass.test.ts b/test/review-pass.test.ts index 10ffa4f8d..1dcb2ec5d 100644 --- a/test/review-pass.test.ts +++ b/test/review-pass.test.ts @@ -740,6 +740,25 @@ describe('Capy pass program (fake host, real bins)', () => { expect(h.creates.find(c => c.title === 'ship x21p2 red-team')!.prompt).toContain('DIFF_BASE=' + rangeBase); }, 240_000); + // Value: protects=the pass program dispatches the owner's granted fix-check from the plan, as decision 3.5 runs any fix-check, instead of PASS: none; fails_when=a recorded grant still prints PASS: none next=stop and the lane renders, sends and collects the fix-check by hand (C6-36 after-opus-grant); + // 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 }); + expect(add.status, add.stderr).toBe(0); + const id = JSON.parse(add.stdout).added[0] as string; + const files = path.join(dir, '.state/gfc'); + fs.mkdirSync(files, { recursive: true }); + const fc: Record = {}; + for (const [k, v] of [['fix_list', '1. src/a.ts: guard v80\n'], ['hunks', 'diff --git a/src/a.ts b/src/a.ts\n+export const v81 = 81;\n'], ['tests', 'bun test: 3 pass, 0 fail\n'], ['callers', 'none\n']]) { fc[k] = path.join(files, k + '.txt'); fs.writeFileSync(fc[k]!, v!); } + const h = host(dir, env); + const res = await runPass(h, { base: 'main', out: path.join(dir, '.state/run/gfc'), pass: 'x30gfc', planFlags: ['--pass', '2', '--last', 'full', '--fixed-blocking', '--grant', id], fixCheck: fc, logStart: ['review-fix-check'] }); + expect(res).not.toContain('PASS: none'); + expect(res).toContain(`NEXT: fix-check (the owner's grant ${id}`); + expect(h.creates.map(c => c.title)).toEqual(['ship x30gfc fix-check']); + expect(res).toContain('NEXT: judge (Phase 4)'); + }, 240_000); + // Value: protects=the Capy pass program runs a scoped repeat with no fix-check files, dispatches its Red Team and leaves the fix-check to Step 3.5 as GST-619 does on the CLI; fails_when=the program stops a scoped plan for missing files or creates a fix-check reader in the scoped manifest; // why_new=#3670 Step 11 (adversarial 1, red team 1), re-scoped to GST-619's model in the merge of main; seam=none; no_claim=the Step 3.5 fix-check itself test('a scoped repeat runs without fixCheck files and leaves its fix-check to Step 3.5 (GST-619)', async () => { diff --git a/test/review-plan-cli.test.ts b/test/review-plan-cli.test.ts index eee3e5159..04045ff5a 100644 --- a/test/review-plan-cli.test.ts +++ b/test/review-plan-cli.test.ts @@ -62,6 +62,29 @@ 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; + // 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 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'); + const granted = plan([...capStop, '--grant', id]); + 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'); + }); 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' }); const w = (n: string, t: string) => { const f = path.join(r.state, n); fs.writeFileSync(f, t); return f; }; diff --git a/test/size-ledger/ship/gst-967-gate-voices.json b/test/size-ledger/ship/gst-967-gate-voices.json new file mode 100644 index 000000000..c2a168eef --- /dev/null +++ b/test/size-ledger/ship/gst-967-gate-voices.json @@ -0,0 +1,4 @@ +{ + "bytes": 417, + "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" +}