From 148d28ea81b7be4daad89d91b911c9efd85391eb Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 27 Oct 2026 22:45:54 -0700 Subject: [PATCH] feat(ship): /ship and /autoplan size their review to the diff (GSTA-131) (#3527) * wip(GSTA-131): review depth, fix-check transitions, dispatch counters, carry pregate, autoplan round policy Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * wip(GSTA-131): reviewers stay at full effort; audits light Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * feat(ship): Step 9 dispatches what gstack-review-plan plans; fix-check after below-floor fixes; cap two full passes plus one fix-check Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test: review-dispatch-compare has a free lane; review-depth fixtures registered free; resumed max_rounds case off the old default Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(GSTA-131): ship review pass-1 repairs: fix-check binds under its own skill, Step 11.5 accepts a fix-check or a planned adversarial skip, delta carry with a pregate, carry --pregate pinned to the checkout, quiet-after-cap revise regates, skill-dir markdown and rule docs classed code, gate signals from tests class up, plan fails closed on git errors, per-class compare ratios Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(GSTA-131): ship review pass-2 repairs: Step 11 follows the last full-pass plan, Important Rules restored, ship goldens regenerated, true per-class medians, review-policy transitions and once-per-invocation voices, docs/** assets stay docs-class, CLI and fix-check binding tests Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(changelog): GSTA-131 review depth, fix-check, delta /ship and autoplan rounds Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs: ship-policy cap and counters, review-policy /review cycle cap, unattended passes (docs audit) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(review): 400-word findings cap fits the /review size pins (five host pins lowered by 2); CHANGELOG max_rounds wording Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(changelog): gate integrity names the size-ledger entry and the lowered /review pins Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test: the round-budget case asserts the default through ROUNDS output only (pregate literals heuristic) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test: curate ship-receipt-carry-pregate out of the Windows lane like its --pregate siblings Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(changelog): end the Windows curation sentence instead of a mid-sentence colon Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship): one decision each for the review-loop exits, the planned adversarial skip and Step 16's TODOS rerun (screen judge repair) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship): fix-check repairs: pin the renamed Step 11 heading, keep the below-floor exit at the cap, name the plan run a repeat reuses, print the native skip on outside-only plans Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 23 ++ autoplan/DRIVER.md | 16 +- autoplan/DRIVER.md.tmpl | 12 +- bin/gstack-autoplan | 21 +- bin/gstack-config | 12 +- bin/gstack-doctor-components.sh | 1 + bin/gstack-review-plan | 126 +++++++++ bin/gstack-ship-receipt | 47 +++- docs/model-policy.md | 16 ++ docs/review-policy.md | 105 ++++++-- docs/ship-policy.md | 36 ++- docs/troubleshooting.md | 13 + docs/unattended.md | 24 +- lib/autoplan-budget.ts | 32 ++- lib/autoplan-export.ts | 22 +- lib/autoplan-prompts.ts | 16 +- lib/autoplan-revise.ts | 56 +++- lib/autoplan-run.ts | 8 +- lib/autoplan-scope.ts | 28 +- lib/autoplan-state.ts | 6 + lib/headless-artifacts.ts | 6 +- lib/result-codes-release.ts | 5 + lib/review-depth.ts | 223 ++++++++++++++++ lib/review-evidence.ts | 2 +- lib/review-scope.ts | 50 +++- lib/ship-receipt.ts | 10 +- review/sections/review-army.md | 2 +- scripts/lib/windows-curation.ts | 1 + scripts/resolvers/fix-check.ts | 25 ++ scripts/resolvers/outside-voice-steps.ts | 14 +- scripts/resolvers/review-army.ts | 2 +- scripts/resolvers/review-dashboard.ts | 4 +- scripts/review-dispatch-compare.ts | 175 +++++++++++++ ship/SKILL.md | 34 +-- ship/SKILL.md.tmpl | 31 +-- ship/sections/adversarial.md | 16 +- ship/sections/adversarial.md.tmpl | 5 + ship/sections/review-army.md | 35 ++- ship/sections/review-army.md.tmpl | 33 ++- ship/sections/test-coverage.md | 4 +- ship/sections/test-coverage.md.tmpl | 4 +- test/autoplan-answer-invalidation.test.ts | 6 +- test/autoplan-budget.test.ts | 12 +- test/autoplan-capy-e2e.test.ts | 4 +- test/autoplan-revise.test.ts | 6 +- test/autoplan-scope.test.ts | 2 +- test/autoplan-token-efficiency.test.ts | 242 ++++++++++++++++++ test/fixtures/golden/agy-ship-SKILL.md | 34 +-- test/fixtures/golden/claude-ship-SKILL.md | 34 +-- test/fixtures/golden/codex-ship-SKILL.md | 34 +-- test/fixtures/golden/factory-ship-SKILL.md | 34 +-- test/fixtures/review-depth/code.json | 67 +++++ test/fixtures/review-depth/docs-only.json | 46 ++++ test/fixtures/skill-md-size-allowlist.json | 10 +- test/fixtures/workflow-required-steps.json | 2 +- test/helpers/autoplan-run-fixture.ts | 4 +- test/helpers/carve-guards.ts | 2 +- test/helpers/free-fixtures-data.ts | 1 + test/outside-voice-preflight.test.ts | 2 +- test/review-depth.test.ts | 169 ++++++++++++ test/review-dispatch-compare.test.ts | 42 +++ test/review-dispatch-counters.test.ts | 48 ++++ test/review-log-findings.test.ts | 13 + test/review-plan-cli.test.ts | 111 ++++++++ test/ship-control-flow.test.ts | 21 ++ test/ship-receipt-carry-pregate.test.ts | 77 ++++++ .../garrytan-gsta131-token-efficiency.json | 4 + test/skill-validation.test.ts | 7 +- 68 files changed, 2069 insertions(+), 266 deletions(-) create mode 100755 bin/gstack-review-plan create mode 100644 lib/review-depth.ts create mode 100644 scripts/resolvers/fix-check.ts create mode 100755 scripts/review-dispatch-compare.ts create mode 100644 test/autoplan-token-efficiency.test.ts create mode 100644 test/fixtures/review-depth/code.json create mode 100644 test/fixtures/review-depth/docs-only.json create mode 100644 test/review-depth.test.ts create mode 100644 test/review-dispatch-compare.test.ts create mode 100644 test/review-dispatch-counters.test.ts create mode 100644 test/review-plan-cli.test.ts create mode 100644 test/ship-receipt-carry-pregate.test.ts create mode 100644 test/size-ledger/ship/garrytan-gsta131-token-efficiency.json diff --git a/CHANGELOG.md b/CHANGELOG.md index b0f5f92ad..3fbfcdb17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,28 @@ # Changelog +## [Unreleased] + +**/ship and /autoplan size their review to the diff.** A /ship review used to send the same specialist army, Red Team, adversarial pass and outside voice at every diff and repeat the whole army after any fix, up to three passes. Now `gstack-review-plan` classifies the diff (docs, tests, code or gate) and plans the next step through one transition table, so a README edit gets the core checklist and a gate change still gets everything. Measured on two captured diffs across five loop scenarios with measured reviewer output (`scripts/review-dispatch-compare.ts`), estimated review tokens fall to a median 0.26x of before on docs-only diffs and 0.85x on code diffs (0.52x over all ten runs, range 0.23–0.99). Every reviewer that can block a ship stays at full effort on the lane's model; a paid replay showed a medium-effort fix-check miss a fail-open that the full-effort check caught. + +### Added +- **`gstack-review-plan`**: prints `REVIEW_CLASS`, `REVIEW_PLAN`, `NEXT` (full, fix-check, stop or done), one `DISPATCH:` line per reviewer with its effort, `SKIP:` lines with reasons, and `AUDIT_EFFORT`; `--json` for scripts. Docs-class diffs run the core checklist only, tests-class adds the testing specialist, code and gate keep scope-selected specialists, and gate adds the Red Team every time. Markdown an agent reads (skills, sections, templates, policy and rule docs, any markdown in a skill directory) counts as code. A git error fails closed instead of planning an empty diff. +- **One fix-check instead of a repeat army**: below-floor fixes, or a blocker fixed at the two-pass cap, go to one full-effort reviewer that reads the fix list, the touched hunks, the callers and the test results (`gstack-review-plan --fix-check-prompt`). It logs as `review-fix-check`, never a pass, and Step 11.5 binds it. The cap is two full passes plus one fix-check; a blocker still open stops for the owner. +- **Step 11's voices run once per invocation**, from the last full-pass plan; the outside voice reruns only when a published number changes (`--reviewed-numbers`). A docs diff that publishes numbers keeps the cross-family voice. +- **Delta /ship after a base merge**: Step 1 runs `gstack-ship-receipt carry --dry-run`; when the product patch-id is unchanged it skips the review loop, repairs only the CHANGELOG and size caps, runs `gstack-pregate` at HEAD and carries the receipt with `--pregate`. Replayed on #3497's history, 3 of its 6 base merges would have carried. +- **Receipt counters**: `review_loop` gains `fix_checks`, `dispatches` by role and `est_input_tokens`; `/autoplan export` prints `DISPATCHES: est_input_tokens=`. +- **`/autoplan` rounds**: one round by default, a second only for High or Critical findings, `ESCALATE: evidence sufficient, decision needed` at the cap; a revise whose decisions all sit below High applies without re-review, unless an earlier capped revise is still waiting on the owner. Docs-only plans (`--docs-only` or detected) run CEO on the native voice, then Eng. +- **`review_effort_tiers`** (`gstack-config`, default `on`): coverage, plan-completion and docs audits, trail reviewers and watchers run at `light` on the same model family; `off` runs them at the host default. + +### Changed +- `gstack-ship-receipt carry --pregate` refuses when the checkout is not at `--head` (`RECEIPT_CARRY_PREGATE_STALE`), so one head's pregate cannot certify another. +- Specialist prompts cap findings at 400 words. `docs/unattended.md` adds report-length limits (READY at most 150 words plus required lines, reports at most 300). + +### Gate integrity +- Ship's union growth is a +1,757-byte entry in GSTA-132's size ledger (test/size-ledger/ship/garrytan-gsta131-token-efficiency.json; measured 330,781 bytes, SIZE_LEDGER: ok), and the ship skeleton cap in test/helpers/carve-guards.ts moves 60,913 → 60,960 bytes, pinned at the measured size. The five /review size pins in test/fixtures/skill-md-size-allowlist.json go down by 2 bytes each. +- `test/autoplan-token-efficiency.test.ts` gives its subprocess-driven cases a 30-second timeout; its resumed-run case persists `max_rounds: 1` instead of 3, a value inside the new cap of 2. +- `test/ship-receipt-carry-pregate.test.ts` joins `KNOWN_WINDOWS_INCOMPATIBLE` in `scripts/lib/windows-curation.ts`, like the four other tests that pass `--pregate` to the receipt CLI. On a Windows host the fixture's Linux lane is a requires-remote obligation, so `write` refuses the pregate (windows-free-shard run 40611810409). It still runs on Linux and macOS. +- Ship's Step 11 now runs the adversarial passes the review plan dispatches, so three prose pins move with it: `test/skill-validation.test.ts` and `test/outside-voice-preflight.test.ts` expect the ship heading without "always-on" and the native pass "on `DISPATCH: adversarial-native`" (/review keeps "always runs"), `test/ship-control-flow.test.ts` pins the one plan call that picks decisions 2–4, and `test/fixtures/workflow-required-steps.json` pins the renamed Step 11 heading. + ## [1.91.290.0] - 2026-10-28 **Installing gstack inside an agent harness now gives that harness gstack's skills, the harness's own way.** Until now OpenClaw and Hermes were instruction-only: `./setup --host hermes` printed a digest path and changed nothing, so an agent working inside OpenClaw or Hermes could not find a single gstack skill until a person wired one in by hand, and QM, dots, Muse Code and Grok were unknown to gstack (Garry, 2026-10-27: "when gstack is installed inside OpenClaw, Hermes, qm, muse, grok bot, dots or any harness it auto resolves and figures out how to use the skills"). `./setup` run inside OpenClaw, Hermes or a Dot now registers gstack there in the same run; `gstack-uninstall` takes it back out. diff --git a/autoplan/DRIVER.md b/autoplan/DRIVER.md index a90aa0116..f1e1a3f96 100644 --- a/autoplan/DRIVER.md +++ b/autoplan/DRIVER.md @@ -14,9 +14,10 @@ ends with `()`. ## Phases and scope Order: ceo → design → dx → eng. Design runs only with UI scope, DX only with DX scope, Eng always and last. -Scope flags on the first `next`: `--ui`, `--no-ui`, `--no-dx`, `--developer-tool`, `--agent-primary`. `--developer-tool` and `--agent-primary` only enable DX. -The first `next` prints `SCOPE: ui=… (terms; visual; files) dx=… (detected) source=ui:,dx:` -(each axis is `flag`, `marker` or `detected`). UI needs a rendered file path or a UI term next +Scope flags on the first `next`: `--ui`, `--no-ui`, `--no-dx`, `--developer-tool`, `--agent-primary`, `--docs-only`. `--developer-tool` and `--agent-primary` only enable DX. +The first `next` prints `SCOPE: ui=… (terms; visual; files) dx=… (detected) docs_only=yes|no () source=ui:,dx:` +(each is `flag`, `marker` or `detected`). Docs-only (`--docs-only`, or every Implementation-plan path is README*, CHANGELOG*, +TODOS*, docs/** but not docs/*-policy.md, or other markdown outside a skill dir) runs CEO on the native voice only, then Eng. UI needs a rendered file path or a UI term next to a visual noun in the Implementation plan. To keep flags for later runs on the same plan, add the `SCOPE_PIN:` marker it prints (``) as the plan's first line yourself; gstack never writes the plan. @@ -34,7 +35,9 @@ line yourself; gstack never writes the plan. accepted plan changes (status `amend_pending`): edit the plan, then `revise` (the printed `NEXT:` names it), or `revise --settle ` when the line says no plan edit is needed. Until then `next` refuses with `PLAN_AMEND_PENDING` and `export` will not mark the run complete. -5. `REOPENED: ` means loop `next` again (Eng last). `APPROVED:` means `export`. +5. `REOPENED: ` means loop `next` again (Eng last). `APPROVED:` means `export` (`DISPATCHES: est_input_tokens=`, relative). +6. Rounds: one by default; a second only for High or Critical findings; cap 2 (`--max-rounds`). A `revise` whose decisions all resolve below + High prints `APPLIED: without re-review (below High)`. At the cap with High open, `ESCALATE: evidence sufficient, decision needed`: relay it, never ask for another round. ### On Capy @@ -55,7 +58,7 @@ line yourself; gstack never writes the plan. ### Commands - `gstack-autoplan next --out --plan [scope flags] [--light] [--spend-cap ] [--max-rounds ] [--scaffold]` - first call: init the run, print PHASE:, SCOPE:, DRIVER: and one ATTEMPT: per voice; --max-rounds caps review rounds per phase (default 3 unattended), and an unattended Capy run reserves 5.00 per host attempt unless --estimate-usd says otherwise, so --spend-cap binds. + first call: init the run, print PHASE:, SCOPE:, DRIVER: and one ATTEMPT: per voice; --max-rounds caps review rounds per phase (default 2 unattended), and an unattended Capy run reserves 5.00 per host attempt unless --estimate-usd says otherwise, so --spend-cap binds. - `gstack-autoplan next --out [--redispatch ]` later calls: open the next phase or reprint open attempts; never redispatches on its own. - `gstack-autoplan submit --out --phase

--voice native|outside --record --model [--attempt ]` @@ -91,8 +94,7 @@ line yourself; gstack never writes the plan. ## Reviewer result format -Every prompt ends with this contract; `submit --result` enforces it, `submit --record` accepts -the looser record shapes above. +Every prompt ends with this contract; `submit --result` enforces it, `submit --record` accepts the looser shapes above. RESULT FORMAT (required by gstack-autoplan; the parent binds your file by this contract): 1. The first line of your result is exactly: INPUT: diff --git a/autoplan/DRIVER.md.tmpl b/autoplan/DRIVER.md.tmpl index 60452a558..b2ff2b001 100644 --- a/autoplan/DRIVER.md.tmpl +++ b/autoplan/DRIVER.md.tmpl @@ -12,8 +12,9 @@ ends with `()`. ## Phases and scope {{AUTOPLAN_DRIVER_PHASES}} -The first `next` prints `SCOPE: ui=… (terms; visual; files) dx=… (detected) source=ui:,dx:` -(each axis is `flag`, `marker` or `detected`). UI needs a rendered file path or a UI term next +The first `next` prints `SCOPE: ui=… (terms; visual; files) dx=… (detected) docs_only=yes|no () source=ui:,dx:` +(each is `flag`, `marker` or `detected`). Docs-only (`--docs-only`, or every Implementation-plan path is README*, CHANGELOG*, +TODOS*, docs/** but not docs/*-policy.md, or other markdown outside a skill dir) runs CEO on the native voice only, then Eng. UI needs a rendered file path or a UI term next to a visual noun in the Implementation plan. To keep flags for later runs on the same plan, add the `SCOPE_PIN:` marker it prints (``) as the plan's first line yourself; gstack never writes the plan. @@ -31,7 +32,9 @@ line yourself; gstack never writes the plan. accepted plan changes (status `amend_pending`): edit the plan, then `revise` (the printed `NEXT:` names it), or `revise --settle ` when the line says no plan edit is needed. Until then `next` refuses with `PLAN_AMEND_PENDING` and `export` will not mark the run complete. -5. `REOPENED: ` means loop `next` again (Eng last). `APPROVED:` means `export`. +5. `REOPENED: ` means loop `next` again (Eng last). `APPROVED:` means `export` (`DISPATCHES: est_input_tokens=`, relative). +6. Rounds: one by default; a second only for High or Critical findings; cap 2 (`--max-rounds`). A `revise` whose decisions all resolve below + High prints `APPLIED: without re-review (below High)`. At the cap with High open, `ESCALATE: evidence sufficient, decision needed`: relay it, never ask for another round. ### On Capy @@ -55,8 +58,7 @@ line yourself; gstack never writes the plan. ## Reviewer result format -Every prompt ends with this contract; `submit --result` enforces it, `submit --record` accepts -the looser record shapes above. +Every prompt ends with this contract; `submit --result` enforces it, `submit --record` accepts the looser shapes above. {{AUTOPLAN_DRIVER_RESULT_FORMAT}} diff --git a/bin/gstack-autoplan b/bin/gstack-autoplan index 705a6bd64..0b0a1c45b 100755 --- a/bin/gstack-autoplan +++ b/bin/gstack-autoplan @@ -2,7 +2,7 @@ /** * gstack-autoplan — the unattended /autoplan runner as an explicit, durable state machine (plan B2). * - * gstack-autoplan next --out

[--plan ] [--plan-heading ] [--ui|--no-ui] [--developer-tool] [--agent-primary] + * gstack-autoplan next --out [--plan ] [--plan-heading ] [--ui|--no-ui] [--developer-tool] [--agent-primary] [--docs-only] * [--no-outside] [--light] [--spend-cap ] [--estimate-usd ] [--deadline ] [--redispatch ] [--json] * gstack-autoplan submit --out --phase

--voice native|outside --result | --record [--attest ] * --model [--attempt ] [--runner ] [--usage-usd ] [--reconcile ] [--json] @@ -32,11 +32,11 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { amendmentSteps, answerGate, closeDeferred, finishGate, recommendGate } from '../lib/autoplan-gate'; import { gateCost, gateCostPayload, renderGateCost, type GateCost } from '../lib/autoplan-gate-cost'; -import { attemptEstimate, changeBudget, liftStop } from '../lib/autoplan-budget'; +import { attemptEstimate, changeBudget, escalateLine, liftStop } from '../lib/autoplan-budget'; import { ANSWERED_BY_RE } from '../lib/gate-list'; import { checkpointLines, checkpointRun, restoreCheckpoint, type CheckpointMoment } from '../lib/autoplan-checkpoint'; import { admit, touchInstallLease } from '../lib/autoplan-admission'; -import { exportRun } from '../lib/autoplan-export'; +import { dispatchLine, exportRun } from '../lib/autoplan-export'; import { capyRelayLine } from '../lib/captain-brief'; import { exportPublic } from '../lib/autoplan-public'; import { migrateRun } from '../lib/autoplan-migrate'; @@ -53,9 +53,10 @@ import { COORDINATOR_CONTRACT } from '../lib/coordinator-contract'; const HELP = `gstack-autoplan — unattended /autoplan: next/submit loop, answer, resume (docs/unattended.md) DRIVER: ${driverFile()} - next --out

[--plan ] [--plan-heading ] [--scaffold] [--ui|--no-ui] [--developer-tool] [--agent-primary] [--no-dx] + next --out [--plan ] [--plan-heading ] [--scaffold] [--ui|--no-ui] [--developer-tool] [--agent-primary] [--no-dx] [--docs-only] [--no-outside] [--light] [--spend-cap ] [--max-rounds ] [--estimate-usd ] [--deadline ] [--redispatch ] [--json] - --max-rounds (first next): review rounds per phase (default 3 unattended); at the cap a phase closes accepted-with-findings + --max-rounds (first next): review rounds per phase (default 2 unattended); at the cap a phase closes accepted-with-findings + --docs-only (first next; also detected when every Implementation-plan path is docs-class): CEO (native voice only) + Eng --estimate-usd: reserved per new attempt; unattended Capy runs reserve a default for host attempts so --spend-cap binds --scaffold (first next): a plan missing one of its two sections is reviewed from /plan-source.md with the heading added --light loads each phase's checklist plus the sections its scope needs (the unattended default); interactive kinds load everything @@ -70,7 +71,8 @@ DRIVER: ${driverFile()} the outside voice could not run: closes the phase native_only once native is bound revise --out --plan [--incorporates ] [--withdraw ] | --settle review a revised plan in this run: every phase whose reviewed Implementation-plan hash differs reopens (Eng last); - --incorporates discharges the named accepted changes (the plan must change); --withdraw (alone or with --plan) drops one; + --incorporates discharges the named accepted changes (the plan must change); when every one resolves below High and no phase + comes into scope, the edit applies without re-review (APPLIED:); --withdraw (alone or with --plan) drops one; --settle records a packaging/ordering change that needs no plan edit and reopens nothing (ids: decision id or d4) run --out --plan --native-model --outside-model [--runner codex-cli|api] [--native-runner api] ... next in a loop, only when a runner executes both voices here; --deadline cancels the children it started @@ -179,7 +181,7 @@ function doNext(out: string, rest: string[], json: boolean): number { const maxRounds = num(rest, '--max-rounds'); if (maxRounds !== undefined && (!Number.isInteger(maxRounds) || maxRounds < 1)) throw new RunError('ARTIFACT_SCHEMA', '--max-rounds needs an integer >= 1', 1); initRun({ plan, out, cwd, planHeading: flag(rest, '--plan-heading'), outsideEnabled: !rest.includes('--no-outside'), spendCap: num(rest, '--spend-cap') ?? null, maxRounds, - deadlineMin: num(rest, '--deadline'), light: rest.includes('--light'), scaffold: rest.includes('--scaffold'), scope: { ui, developerTool: rest.includes('--developer-tool'), agentPrimary: rest.includes('--agent-primary'), dx: rest.includes('--no-dx') ? false : undefined } }, initLines); + deadlineMin: num(rest, '--deadline'), light: rest.includes('--light'), scaffold: rest.includes('--scaffold'), scope: { ui, developerTool: rest.includes('--developer-tool'), agentPrimary: rest.includes('--agent-primary'), dx: rest.includes('--no-dx') ? false : undefined, ...(rest.includes('--docs-only') ? { docsOnly: true } : {}) } }, initLines); } const redispatch = rest.flatMap((a, i) => (a === '--redispatch' && rest[i + 1] ? [rest[i + 1]!] : [])); const r = nextStep({ out, cwd, estimateUsd: num(rest, '--estimate-usd'), redispatch }); @@ -197,7 +199,8 @@ function doNext(out: string, rest: string[], json: boolean): number { function gateLines(state: RunState, out: string): string[] { const relay = capyRelayLine(out); const cost = gateCostOf(state, out); - return [`GATE: gate_rev=${state.gate_rev} render with: gstack-gate render ${path.join(out, 'gate.json')}`, ...(cost ? renderGateCost(cost) : []), ...(relay ? [relay] : [])]; + const escalate = escalateLine(state); + return [`GATE: gate_rev=${state.gate_rev} render with: gstack-gate render ${path.join(out, 'gate.json')}`, ...(escalate ? [escalate] : []), ...(cost ? renderGateCost(cost) : []), ...(relay ? [relay] : [])]; } function gateCostOf(state: RunState, out: string): GateCost | undefined { const gateFile = path.join(out, 'gate.json'); @@ -397,7 +400,7 @@ async function main(argv: string[]): Promise { const r = exportRun(state); writeState(state); for (const w of r.warnings) process.stderr.write(`gstack-autoplan export: ${w}\n`); - emit(json, r.files.map(f => `EXPORTED: ${f}`), { run: state.run, files: r.files, warnings: r.warnings }); + emit(json, [...r.files.map(f => `EXPORTED: ${f}`), dispatchLine(state)], { run: state.run, files: r.files, warnings: r.warnings, dispatches: state.dispatches, est_input_tokens: state.est_input_tokens }); return EXIT.ok; }); } diff --git a/bin/gstack-config b/bin/gstack-config index 0e239428f..6fff279f9 100755 --- a/bin/gstack-config +++ b/bin/gstack-config @@ -200,6 +200,9 @@ CONFIG_HEADER='# gstack configuration — edit freely, changes take effect on ne # # Absent = every gate keeps its effort. GSTACK_CODEX_EFFORT # # overrides it per run, /codex --effort per request; # # /spec keeps medium (120 s budget). +# review_effort_tiers: on # on | off. on: audits, trail reviewers and watchers +# # (nothing that can block a ship) run at light effort on +# # the same model; every reviewer keeps full. # Docs: docs/model-policy.md # # ─── Advanced ──────────────────────────────────────────────────────── @@ -360,6 +363,7 @@ lookup_default() { implementation_tier) echo "smart" ;; # frontier | smart model_frontier_claude|model_frontier_openai|model_smart_claude|model_smart_openai) echo "" ;; # empty = shipped catalog ID (gstack-models list) codex_effort) echo "" ;; # empty = each gate's own effort; /codex --effort, then GSTACK_CODEX_EFFORT, win (gstack-codex-probe run-with-timeout --effort) + review_effort_tiers) echo "on" ;; # on | off — gstack-review-plan effort tiers (docs/model-policy.md#reasoning-effort) ship_greptile_early) echo "true" ;; # true | false — /ship's early Greptile PR pr_wait_gating_bots) echo "greptile,macroscope" ;; # review bots gstack-pr-wait --until ready waits for pr_wait_advisory_bots) echo "coderabbit" ;; # review bots gstack-pr-wait reports but never waits for @@ -821,6 +825,10 @@ $_gstack_legacy_root" fi # codex_effort feeds model_reasoning_effort on paid Codex calls: reject a # typo instead of coercing it (the probe refuses an invalid stored value too). + if [ "$KEY" = "review_effort_tiers" ] && [ "$VALUE" != "on" ] && [ "$VALUE" != "off" ]; then + echo "Error: review_effort_tiers '$VALUE' not recognized. Valid values: on, off. Existing value left unchanged." >&2 + exit 1 + fi if [ "$KEY" = "codex_effort" ]; then case "$VALUE" in minimal|low|medium|high|xhigh|max|ultra) ;; @@ -1078,7 +1086,7 @@ $_gstack_legacy_root" ship_measure_judge_outputs ship_measure_ask_per_trial_usd ship_measure_budget_usd \ ship_measure_max_rounds ship_measure_sweep_cases ship_measure_sweep_budget_usd ship_rerun_backend test_backend \ plan_review_tier implementation_tier model_frontier_claude model_frontier_openai \ - model_smart_claude model_smart_openai codex_effort; do + model_smart_claude model_smart_openai codex_effort review_effort_tiers; do gstack_config_select "$KEY" VALUE="$_gstack_cfg_value" SOURCE="default" @@ -1103,7 +1111,7 @@ $_gstack_legacy_root" ship_measure_judge_outputs ship_measure_ask_per_trial_usd ship_measure_budget_usd \ ship_measure_max_rounds ship_measure_sweep_cases ship_measure_sweep_budget_usd ship_rerun_backend test_backend \ plan_review_tier implementation_tier model_frontier_claude model_frontier_openai \ - model_smart_claude model_smart_openai codex_effort; do + model_smart_claude model_smart_openai codex_effort review_effort_tiers; do printf ' %-24s %s\n' "$KEY:" "$(lookup_default "$KEY")" done ;; diff --git a/bin/gstack-doctor-components.sh b/bin/gstack-doctor-components.sh index b9343c1c9..221e5c993 100644 --- a/bin/gstack-doctor-components.sh +++ b/bin/gstack-doctor-components.sh @@ -309,6 +309,7 @@ gstack_result_anchor() { RECEIPT_CARRY_PRODUCT_CHANGED) printf '%s' 'receipt-carry-product-changed' ;; DOCS_AUDIT_BLOCKED) printf '%s' 'docs-audit-blocked' ;; RECEIPT_PROSE_MISMATCH) printf '%s' 'receipt-prose-mismatch' ;; + RECEIPT_CARRY_PREGATE_STALE) printf '%s' 'receipt-carry-pregate-stale' ;; RECEIPT_GIT_TOO_OLD) printf '%s' 'receipt-git-too-old' ;; PR_WAIT_BLOCKED) printf '%s' 'pr-wait-blocked' ;; PR_WAIT_REFUSED) printf '%s' 'pr-wait-refused' ;; diff --git a/bin/gstack-review-plan b/bin/gstack-review-plan new file mode 100755 index 000000000..a5424c25a --- /dev/null +++ b/bin/gstack-review-plan @@ -0,0 +1,126 @@ +#!/usr/bin/env -S bun --no-env-file --config=/dev/null +/** + * gstack-review-plan — how much review the next /ship step dispatches (docs/review-policy.md#review-depth). + * + * 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 ] [--no-tiers] [--json] + * gstack-review-plan --fix-check-prompt --fix-list --hunks --tests [--callers ] + * + * Reads the diff against the merge base with (committed, working tree and + * untracked), runs gstack-diff-scope and, unless --gate-tagged is given, + * gstack-gate-diff; prints REVIEW_CLASS:, REVIEW_PLAN:, one DISPATCH: line per + * reviewer and one SKIP: line per reviewer left out. Logic: lib/review-depth.ts. + */ +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { EXIT, exitTableHelp } from '../lib/headless-artifacts'; +import { runScriptSync } from '../lib/script-invocation'; +import { planReview, reviewPlanLine, scanNumbers, type History } from '../lib/review-depth'; +import { renderFixCheckPrompt } from '../scripts/resolvers/fix-check'; + +const HELP = `gstack-review-plan — the review depth of the next /ship step (docs/review-policy.md#review-depth) + + --base the base branch (merge base with HEAD is the diff base); required + --pass full review passes already run in this invocation (default 0) + --last none|full|fix-check the step that just finished (default none: plan pass 1) + --fixed-blocking the finished step fixed a blocking finding + --fixed-below the finished step fixed findings below the floor + --blocking-open a blocking finding is still open in fix scope + --touched-gate a fix touched a gate-class path + --critical-found a specialist reported a CRITICAL finding (Red Team on code class) + --adversarial-ran Step 11's native adversarial pass already ran (a repeat skips it) + --gate-tagged gate hunks the gate scan tagged (default: run gstack-gate-diff; failure means unavailable) + --gated a,b specialists the hit-rate stats tag GATE_CANDIDATE + --force a,b|all specialists forced by prompt flags (--all-specialists is all) + --reviewed-numbers the numeric-claim fingerprint the outside voice already reviewed + --no-tiers audits and watchers at the host's default effort too (as gstack-config review_effort_tiers off does) + --json the plan as JSON + --fix-check-prompt print the fix-check reviewer's prompt from --fix-list, --hunks, --tests and --callers files +${exitTableHelp()}`; + +const args = process.argv.slice(2); +const flag = (n: string) => { const i = args.indexOf(n); return i >= 0 ? args[i + 1] : undefined; }; +const has = (n: string) => args.includes(n); +const list = (n: string) => (flag(n) ?? '').split(',').map(s => s.trim()).filter(Boolean); +const git = (a: string[]) => { const r = spawnSync('git', ['-c', 'core.quotePath=false', ...a], { encoding: 'utf8', timeout: 60_000, maxBuffer: 64 * 1024 * 1024 }); return r.status === 0 ? r.stdout : null; }; + +function main(): number { + if (has('--help') || has('-h')) { process.stdout.write(HELP + '\n'); return EXIT.ok; } + if (has('--fix-check-prompt')) { + const read = (n: string, required: boolean) => { + const f = flag(n); + if (!f) return required ? null : ''; + try { return fs.readFileSync(f, 'utf8'); } catch { return null; } + }; + const [fixList, touchedHunks, testResults, callers] = [read('--fix-list', true), read('--hunks', true), read('--tests', true), read('--callers', false)]; + if (fixList === null || touchedHunks === null || testResults === null || callers === null) { process.stderr.write('gstack-review-plan: --fix-check-prompt needs readable --fix-list, --hunks and --tests files (--callers optional)\n'); return EXIT.usage; } + process.stdout.write(renderFixCheckPrompt({ fixList, touchedHunks, testResults, callers })); + return EXIT.ok; + } + const base = flag('--base'); + if (!base) { process.stderr.write(`gstack-review-plan: --base is required\n${HELP}\n`); return EXIT.usage; } + const last = (flag('--last') ?? 'none') as History['last']; + if (!['none', 'full', 'fix-check'].includes(last)) { process.stderr.write('gstack-review-plan: --last must be none, full or fix-check\n'); return EXIT.usage; } + const pass = Number(flag('--pass') ?? '0'); + if (!Number.isInteger(pass) || pass < 0) { process.stderr.write('gstack-review-plan: --pass needs an integer >= 0\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); + const ref = git(['rev-parse', '--verify', '-q', `origin/${base}`]) !== null ? `origin/${base}` : base; + const mergeBase = git(['merge-base', ref, 'HEAD'])?.trim(); + if (!mergeBase) { process.stderr.write(`gstack-review-plan: no merge base with ${ref} (fetch the base first)\n`); return EXIT.fail; } + const nameStatusRaw = git(['diff', '--name-status', '--no-renames', mergeBase]); + const untrackedRaw = git(['ls-files', '--others', '--exclude-standard']); + const numstatRaw = git(['diff', '--numstat', mergeBase]); + if (nameStatusRaw === null || untrackedRaw === null || numstatRaw === null) { process.stderr.write('gstack-review-plan: git could not list the diff; no plan is safer than a plan for an empty diff\n'); return EXIT.fail; } + const scratch = (p: string) => p.startsWith('.gstack/tmp/'); + const nameStatus = nameStatusRaw.split('\n').filter(Boolean).map(l => l.split('\t')).filter(c => !scratch(c[1] ?? '')); + const untracked = untrackedRaw.split('\n').filter(Boolean).filter(p => !scratch(p)); + const paths = [...new Set([...nameStatus.map(c => c[1]!).filter(Boolean), ...untracked])].sort(); + const deleted = nameStatus.filter(c => c[0] === 'D').map(c => c[1]!); + const stampOnly = paths.filter(p => p.endsWith('package.json') && !untracked.includes(p)).filter(p => { + const changed = (git(['diff', '-U0', '--no-color', mergeBase, '--', p]) ?? '').split('\n').filter(l => /^[+-]/.test(l) && !/^(\+\+\+|---) /.test(l)); + return changed.length > 0 && changed.every(l => /^[+-]\s*"version"\s*:/.test(l)); + }); + const diffLines = numstatRaw.split('\n').reduce((n, l) => { const [a, d] = l.split('\t'); return n + (Number(a) || 0) + (Number(d) || 0); }, 0) + + untracked.reduce((n, p) => { try { return n + fs.readFileSync(p, 'utf8').split('\n').length; } catch { return n; } }, 0); + const added: Record = {}; + let current = ''; + for (const line of (git(['diff', '-U0', '--no-color', mergeBase]) ?? '').split('\n')) { + if (line.startsWith('+++ b/')) { current = line.slice(6); continue; } + if (line.startsWith('+') && !line.startsWith('+++') && current) (added[current] ??= []).push(line.slice(1)); + } + for (const p of untracked) { try { added[p] = fs.readFileSync(p, 'utf8').split('\n'); } catch { /* unreadable: no claims */ } } + const binDir = path.dirname(Bun.fileURLToPath(import.meta.url)); + const scopeRun = runScriptSync(path.join(binDir, 'gstack-diff-scope'), [base], { encoding: 'utf8', timeout: 60_000 }); + const scope: Record = {}; + for (const l of (scopeRun.stdout ?? '').split('\n')) { const m = /^(SCOPE_[A-Z_]+)=(.*)$/.exec(l); if (m) scope[m[1]!] = m[2]!; } + if (scopeRun.status !== 0 && !scope.SCOPE_ERROR) scope.SCOPE_ERROR = `exit ${scopeRun.status}`; + let gateTagged: number | null = null; + if (flag('--gate-tagged') !== undefined) gateTagged = Number(flag('--gate-tagged')); + else { + const g = runScriptSync(path.join(binDir, 'gstack-gate-diff'), [base], { encoding: 'utf8', timeout: 120_000, maxBuffer: 16 * 1024 * 1024 }); + const m = /GATE_SUMMARY: .*?tagged=\{([^}]*)\}/.exec(g.stdout ?? ''); + gateTagged = g.status === 0 && m ? m[1]!.split(',').reduce((n, kv) => n + (Number(kv.split(':')[1]) || 0), 0) : null; + } + if (gateTagged !== null && !Number.isFinite(gateTagged)) gateTagged = null; + const force = flag('--force') === 'all' ? 'all' as const : list('--force'); + const tiersSetting = runScriptSync(path.join(binDir, 'gstack-config'), ['get', 'review_effort_tiers'], { encoding: 'utf8', timeout: 15_000 }); + const auditEffort = has('--no-tiers') || (tiersSetting.stdout ?? '').trim() === 'off' ? 'host' : 'light'; + const plan = planReview({ + paths, deleted, stampOnly, root, scope, gateTagged, diffLines, numbers: scanNumbers(added), force, gated: list('--gated'), + criticalFound: has('--critical-found'), adversarialRan: has('--adversarial-ran'), reviewedNumbers: flag('--reviewed-numbers'), + history: { fullPasses: pass, last, fixedBlocking: has('--fixed-blocking'), fixedBelow: has('--fixed-below'), blockingOpen: has('--blocking-open'), touchedGate: has('--touched-gate') }, + }); + if (has('--json')) { process.stdout.write(JSON.stringify({ ...plan, diff_lines: diffLines, pass, audit_effort: auditEffort, scope, gate_tagged: gateTagged, paths, deleted, stamp_only: stampOnly }) + '\n'); return EXIT.ok; } + const out = [`REVIEW_CLASS: ${plan.class} (${plan.reasons.slice(0, 3).join('; ') || 'no signals'})`, reviewPlanLine(plan, pass + (plan.next.step === 'full' ? 1 : 0)), `NEXT: ${plan.next.step} (${plan.next.reason})`]; + for (const d of plan.dispatches) out.push(`DISPATCH: ${d.role} ${d.name} effort=${d.effort}`); + for (const s of plan.skipped) out.push(`SKIP: ${s}`); + out.push(`AUDIT_EFFORT: ${auditEffort} (coverage, plan-completion and docs audits, decision-trail reviewers, watchers; reviewers above stay full)`); + process.stdout.write(out.join('\n') + '\n'); + return EXIT.ok; +} + +process.exit(main()); diff --git a/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index 3614720be..d7d3940a0 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -12,7 +12,7 @@ * gstack-ship-receipt read --body [--json] * gstack-ship-receipt result (--pr [--repo ] | --body ) [--head ] [--out ] [--json] * gstack-ship-receipt census --repo --since [--cohort ] [--limit ] [--json] - * gstack-ship-receipt carry (--pr [--repo ] | --body ) [--base ] [--head ] [--dry-run] [--out ] [--json] + * gstack-ship-receipt carry (--pr [--repo ] | --body ) [--base ] [--head ] [--pregate ] [--dry-run] [--out ] [--json] * gstack-ship-receipt docs-caps [--base ] [--policy ] [--json] * * `write` validates against `gstack-artifact schema ship-receipt`, prints the @@ -60,11 +60,12 @@ const HELP = `gstack-ship-receipt — the \`\`\`gstack-ship-receipt\`\`\` block GSTACK_RESULT: skill=ship status=complete run= (refused: status=refused run=-) census --repo --since [--cohort ] the P7 adoption number over merged PRs (deduplicated by run and thread) - carry --pr [--repo ] | --body [--base ] [--head ] [--dry-run] [--out ] + carry --pr [--repo ] | --body [--base ] [--head ] [--pregate ] [--dry-run] [--out ] carry the receipt to HEAD when only a base merge, tests or release metadata changed since its head (product patch-id unchanged): rewrites the receipt with a \`carry\` record, posts the body (gh pr edit) and prints GSTACK_RESULT: skill=ship status=complete; RECEIPT_CARRY_PRODUCT_CHANGED / RECEIPT_CARRY_REFUSED leave the body unchanged (exit 3); + a receipt that records a pregate needs --pregate current for HEAD (RECEIPT_CARRY_PREGATE_STALE); needs git >= 2.39 (RECEIPT_GIT_TOO_OLD) docs-caps [--base ] /ship Step 14.5: one DOCS_CAP: / ok|over line per docs.caps entry of the reviewed policy (origin/'s .gstack/ship-policy.json; --policy reads that file @@ -144,18 +145,12 @@ function cmdWrite(rest: string[], json: boolean): number { const r = spawnSync(path.join(path.dirname(Bun.fileURLToPath(import.meta.url)), 'gstack-session-kind'), [], { encoding: 'utf8', timeout: 15_000 }); sessionKind = r.status === 0 ? r.stdout.trim() || undefined : undefined; } - let pregate: { pregate: string; requires_remote?: string[] } | undefined; + let pregate: PregateField | undefined; const pregateFile = flag(rest, '--pregate'); if (pregateFile) { - const read = readPregateFile(pregateFile); - if (!read.file) { - for (const e of read.errors) process.stderr.write(resultLine('gstack-ship-receipt write', e.code, `${e.message} at ${e.path}`) + '\n'); - return EXIT.fail; - } - const v = verifyPregate(repoRoot, read.file); - if (!v.ok) { process.stderr.write(resultLine('gstack-ship-receipt write', v.code!, `${v.message}; nothing written`) + '\n'); return EXIT.fail; } - const remote = read.file.requires_remote ?? []; - pregate = { pregate: `${read.file.verdict ?? 'pass'} @${read.file.tree.slice(0, 12)} (${read.file.checks.length} checks, current for ${head.slice(0, 12)})`, requires_remote: remote.length ? remote.map(r => `${r.lane}: ${r.receipt}`) : undefined }; + const read = pregateFieldFor(repoRoot, pregateFile, head); + if (!read.ok) { for (const e of read.errors) process.stderr.write(resultLine('gstack-ship-receipt write', e.code, `${e.message}; nothing written`) + '\n'); return EXIT.fail; } + pregate = read.field; } const base = flag(rest, '--base') ?? resolveBaseBranch(repoRoot); const bodyFile = flag(rest, '--body'); @@ -361,6 +356,17 @@ function gitVersionBelow(repoRoot: string, minimum: string): string | null { return major > minMajor || (major === minMajor && minor >= minMinor) ? null : m[0]; } +type PregateField = { pregate: string; requires_remote?: string[] }; +/** A pregate result file → the receipt's pregate field, verified current for this checkout's tree (shared by write and carry). */ +function pregateFieldFor(repoRoot: string, file: string, head: string): { ok: true; field: PregateField } | { ok: false; errors: Array<{ code: string; message: string }> } { + const read = readPregateFile(file); + if (!read.file) return { ok: false, errors: read.errors.map(e => ({ code: e.code, message: `${e.message} at ${e.path}` })) }; + const v = verifyPregate(repoRoot, read.file); + if (!v.ok) return { ok: false, errors: [{ code: v.code!, message: v.message ?? 'pregate is not current' }] }; + const remote = read.file.requires_remote ?? []; + return { ok: true, field: { pregate: `${read.file.verdict ?? 'pass'} @${read.file.tree.slice(0, 12)} (${read.file.checks.length} checks, current for ${head.slice(0, 12)})`, requires_remote: remote.length ? remote.map(r => `${r.lane}: ${r.receipt}`) : undefined } }; +} + /** * The receipt follows HEAD when the reviewed product is unchanged * (docs/unattended.md#the-terminal-line; harness lane E row 3). Four Captains @@ -378,7 +384,7 @@ function cmdCarry(rest: string[], json: boolean): number { if (!repoRoot) { process.stderr.write('gstack-ship-receipt carry: run it inside the git checkout whose head carries the receipt\n'); return EXIT.usage; } const head = (flag(rest, '--head') ?? git(['rev-parse', 'HEAD'], repoRoot))?.toLowerCase(); if (!head || !/^[0-9a-f]{40,64}$/.test(head)) { process.stderr.write('gstack-ship-receipt carry: cannot resolve HEAD\n'); return EXIT.usage; } - const refused = (code: 'QUEUE_RECEIPT_MISSING' | 'RECEIPT_CARRY_REFUSED' | 'RECEIPT_CARRY_PRODUCT_CHANGED' | 'RECEIPT_GIT_TOO_OLD', message: string) => { + const refused = (code: 'QUEUE_RECEIPT_MISSING' | 'RECEIPT_CARRY_REFUSED' | 'RECEIPT_CARRY_PRODUCT_CHANGED' | 'RECEIPT_GIT_TOO_OLD' | 'RECEIPT_CARRY_PREGATE_STALE', message: string) => { process.stderr.write(resultLineWithDocs('gstack-ship-receipt carry', code, message) + '\n'); process.stdout.write(json ? JSON.stringify({ skill: 'ship', status: 'refused', run: '-', code, message }) + '\n' : `CARRY: refused (${code})\nGSTACK_RESULT: skill=ship status=refused run=-\n`); return EXIT.refused; @@ -480,7 +486,20 @@ function cmdCarry(rest: string[], json: boolean): number { const rangeDiff = rangeRaw.status === 0 ? (rangeRaw.stdout.trim().split('\n').slice(0, 20).join('\n') || '(empty: identical ranges)') : `unavailable: ${(rangeRaw.stderr ?? '').trim().split('\n')[0] || (rangeRaw.signal ? `range-diff killed by ${rangeRaw.signal} after 15 s` : 'range-diff failed')}`; const plan = planCarry({ from, head, changed, productPatchId: { from: productFrom, head: productHead }, testPatchId: { from: testFrom, head: testHead }, changedProduct, mergeCommits: Number(mergeCommits) || 0, rangeDiff }); if (!plan.ok) return refused(plan.code, plan.message); - const carried = carryReceipt(receipt, head, plan.carry); + // Eligibility is not publication: a receipt that recorded a pregate is rebound only with a pregate current for HEAD, + // so a carried receipt never claims the old tree's checks (docs/ship-policy.md#receipt-carry). + const pregateFile = flag(rest, '--pregate'); + let pregateField: PregateField | undefined; + if (pregateFile) { + const local = git(['rev-parse', 'HEAD'], repoRoot)?.trim(); + if (local !== head) return refused('RECEIPT_CARRY_PREGATE_STALE', `--pregate verifies this checkout's tree, but the checkout is at ${local?.slice(0, 12) ?? 'no commit'}, not the carried head ${head.slice(0, 12)}; check out ${head.slice(0, 12)} and run gstack-pregate there`); + const read = pregateFieldFor(repoRoot, pregateFile, head); + if (!read.ok) return refused('RECEIPT_CARRY_PREGATE_STALE', `--pregate ${pregateFile}: ${read.errors[0]?.code ?? 'invalid'} ${read.errors[0]?.message ?? ''}`.trim()); + pregateField = read.field; + } else if (receipt.pregate && !rest.includes('--dry-run')) { + return refused('RECEIPT_CARRY_PREGATE_STALE', `the receipt records a pregate for ${receipt.head.slice(0, 12)}; run gstack-pregate at HEAD ${head.slice(0, 12)} and pass --pregate `); + } + const carried = carryReceipt(pregateField ? { ...receipt, ...pregateField } : receipt, head, plan.carry); const errors = validateReceipt(carried); if (errors.length) return refused('RECEIPT_CARRY_REFUSED', `the carried receipt does not validate: ${errors[0]!.message} at ${errors[0]!.path}`); const newBody = placeReceiptFirst(body, carried); diff --git a/docs/model-policy.md b/docs/model-policy.md index b3ecd6e2c..2c40f0ba4 100644 --- a/docs/model-policy.md +++ b/docs/model-policy.md @@ -154,6 +154,22 @@ set, because a higher effort would spend the time the gate is sized for. `gstack-models list` prints the effective effort and where it came from (`--json`: `codexEffort`). The Claude Code runner takes no effort setting. +### Review effort tiers + +Every reviewer that can block a ship runs at `full` effort on the lane's +model: the core checklist, each specialist, the Red Team, the native +adversarial pass and the fix-check. `gstack-review-plan` names each +dispatch's effort, and `host` for the outside voice (its provider and +`codex_effort` above). Only work that cannot block runs `light`, the same +model at a medium or low setting and never a cheaper model family: the coverage, +plan-completion and docs audits, decision-trail reviewers and watchers +(`AUDIT_EFFORT:` in the plan's output). A paid replay showed why reviewers stay +at full: a medium-effort fix-check missed a fail-open in gate code that the same +check at full effort caught. A host that cannot set a subagent's effort runs it +at its default. `gstack-config set review_effort_tiers off` (default `on`) +runs the audits and watchers at the host default too +([review-policy.md](review-policy.md#review-depth)). + ## Which workflows use the tiers? The plan-review role applies to autoplan's CEO, design, DX and engineering diff --git a/docs/review-policy.md b/docs/review-policy.md index 046220eb6..149abc89b 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -87,22 +87,85 @@ incomplete coverage, which blocks until the reviewer is rerun. (the changed line behind a defect reported elsewhere, a deleted file included) places it in the diff. Below-floor findings outside the diff are `note`, listed under `## Review notes`. -2. **Pass 2 and later review only the range since the previous pass.** Each pass - saves its head; the next pass diffs `RANGE_BASE..HEAD` for every reviewer (the - gate scan and QA probes keep the whole candidate) and classifies with - `--range-base`. Only a blocking finding in that range or in the diff is `fix`; - every below-floor finding is a `note`. A finding the diff causes in code it did - not change names the changed line in `cause`, and the locus follows it. -3. **Three passes is the cap.** A pass that applies no edits is the exit: the - PR ships with its deferred and noted items named in the body. A blocking - finding still in `fix` scope on pass 3 stops the run and names the finding - (path, severity, problem) for the owner; a fourth pass is the owner's call, - never the loop's. +2. **What comes next is the transition table below**, not "any fix earns + another pass". A full pass after pass 1 reviews only the range since the + previous pass: each pass saves its head, the next diffs `RANGE_BASE..HEAD` + for every reviewer (the gate scan and QA probes keep the whole candidate) and + classifies with `--range-base`. Only a blocking finding in that range or in + the diff is `fix`; every below-floor finding is a `note`. +3. **Two full passes is the cap**, plus at most one fix-check. A blocking + finding still open after the last allowed step stops the run and names the + finding (path, severity, problem) for the owner; a third full pass is the + owner's call, never the loop's. The cap approves nothing. + + +### Transitions + +`gstack-review-plan` (`lib/review-depth.ts` `nextStep`) owns this table, and +/ship asks it after every step: + +| The step that just finished | Next | +|---|---| +| none | full pass 1 on the whole diff | +| a full pass with no edits | done | +| a full pass that fixed a blocking finding, below the cap | full pass on the range | +| a full pass whose fix touched a gate-class path, below the cap | full pass on the range | +| a full pass that fixed only below-floor findings | one fix-check | +| a full pass at the cap that fixed a blocking finding | one fix-check | +| 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 **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. The same floor and the same `NO FINDINGS` rule apply to `/review` on its own; its re-review loop keeps its three-cycle fix cap (`CYCLES`), and a pass that applies no edits is its exit too. + +## Review depth by diff class + +`gstack-review-plan --base ` classifies the diff and prints the reviewers +the next step dispatches (`REVIEW_CLASS:`, `REVIEW_PLAN:`, one `DISPATCH:` per +reviewer with its effort, one `SKIP:` per reviewer left out, `--json`). A mixed +diff takes its most restrictive class; a `gstack-diff-scope` error fails closed +to code, and code with the gate scan unavailable fails closed to gate. + +| Class | What it is | Pass 1 dispatches | +|---|---|---| +| **docs** | markdown no agent or gate reads: README, CHANGELOG, TODOS, how-to and explanation docs | the parent's core checklist only | +| **tests** | test files and fixtures only | core plus the testing specialist | +| **code** | everything else, including every `SKILL.md` and template, any markdown in a skill directory, `*/sections/**`, `scripts/resolvers/**`, `docs/*-policy.md`, `docs/captain.md`, `docs/unattended.md`, `docs/pregate.md`, checklists, specialists and bins | specialists by scope (as before), the native adversarial pass, the outside voice; the Red Team past 200 lines or after a CRITICAL | +| **gate** | code plus auth or migration scope, a hunk the gate scan tags (an RH rule: a relaxed threshold, a skipped test), a deleted code file, CI workflows, credential or state-root paths | code's set, plus the Red Team always, at full effort, and no hit-rate gating | + +**Numbers keep the outside voice.** On a docs or tests diff the outside voice is +skipped unless the diff publishes numeric claims: `gstack-review-plan` scans the +added lines of every path for percentages, money, multipliers, durations, token +or credit counts and score, rate or speed claims (`NUMBERS: yes`), and a +claim always dispatches it. The claims' fingerprint (`fingerprint=`) is passed +back as `--reviewed-numbers`; when a later fix changes a claim, only the +outside voice reruns, on the numbers. + +**The adversarial and outside voices run once per invocation** (Step 11, on the +final tree), as the last full-pass plan lists them; after they ran, decision 2 +passes `--adversarial-ran` so a repeat lists them no more. Later full passes rerun the +specialists on the range only. Step 11.5 binds a planned skip through the Step 9 +record's `CURRENT` freshness instead of a native record. + +**Effort.** Every reviewer that can block a ship (the core checklist, +specialists, the Red Team, the native adversarial pass, fix-checks) runs at +`full` effort. Work that cannot block (the Step 7 coverage audit, the Step 8 +plan-completion audit, the Step 14.5 docs audit, decision-trail reviewers, +watchers) runs at `light`: the same model at a lower reasoning effort, never a +cheaper model family. The outside voice keeps its own provider and effort +(`codex_effort`). A host that cannot set a subagent's effort runs it at its +default. `gstack-config set review_effort_tiers off` (or `--no-tiers`) runs +the audits and watchers at the host default too +([model-policy.md](model-policy.md#reasoning-effort)). + ## What the receipt records `gstack-review-scope summary --since ` reads this invocation's @@ -111,15 +174,24 @@ puts in the receipt: | Field | Meaning | |---|---| -| `review_loop.passes` | review passes this ship ran (1 to 3) | +| `review_loop.passes` | full review passes this ship ran (1 or 2; up to 3 under `review_scope: all`) | | `review_loop.blocking_found` | distinct blocking findings across the passes | | `review_loop.deferred` | blocking findings in unchanged code filed as issues (a Step 11 late defer, logged as a `review-late-defer` row so it never displaces the newest `review` pass, counts here and is not a pass) | | `review_loop.noted` | below-floor findings listed in the PR body | | `review_loop.fixed` | findings fixed in this PR | | `review_loop.scope` | the `review_scope` the loop ran under | +| `review_loop.fix_checks` | fix-checks this ship ran (never passes) | +| `review_loop.dispatches` | reviewer dispatches by role: `specialist`, `red-team`, `adversarial-native`, `outside-voice`, `fix-check` | +| `review_loop.est_input_tokens` | prompt plus diff bytes the dispatches read, divided by 4: a relative estimate for comparing runs, not a bill | + +Each review row may carry `dispatches: [{role, effort, prompt_bytes, +diff_bytes, status}]`; `summary` adds them up (older rows fall back to the +specialist stats they record). The summary line appends `fix_checks=`, +`dispatches=` and `est_input_tokens=` only when they were recorded, so an old +loop prints the old line. The receipt's summary line prints `passes= blocking_found= deferred=`, -which is what a scorecard tracks (target: a median of 2 passes per PR). A +which is what a scorecard tracks (target: a median of 1 pass per PR). A receipt carried across a base merge or a test-only fix ([ship-policy.md#receipt-carry](ship-policy.md#receipt-carry)) keeps these counters, because the reviewed product diff is unchanged. @@ -130,8 +202,8 @@ counters, because the reviewed product diff is unchanged. | Value | Pass 1 | Blocking finding in unchanged code | Pass 2+ | Cap | |---|---|---|---|---| -| `frozen` (default) | the diff | filed as an issue, listed as deferred | the range since the previous pass, blocking only | 3 passes | -| `diff-plus-blocking` | the diff | fixed in this PR | the range since the previous pass, blocking only | 3 passes | +| `frozen` (default) | the diff | filed as an issue, listed as deferred | the range since the previous pass, blocking only | 2 full passes + 1 fix-check | +| `diff-plus-blocking` | the diff | fixed in this PR | the range since the previous pass, blocking only | 2 full passes + 1 fix-check | | `all` | everything | fixed in this PR | the whole diff, every finding | 3 fix cycles | `diff-plus-blocking` is the fallback for a repository where deferred issues pile @@ -198,7 +270,8 @@ trailers, appended }`. - `scripts/resolvers/review-army.ts`: the floor and `NO FINDINGS` in every specialist and Red Team prompt (rendered into `review/sections/review-army.md` and `ship/sections/review-army.md`), and the lead judgment's "On record" line that turns a [ruled class](#rulings-below-the-floor) into a note; in `/ship` every specialist and Red Team prompt it renders carries its diff base as a literal sha the parent writes in (`DIFF_BASE=`: the merge base on pass 1, `RANGE_BASE` from pass 2 on), because a reviewer is its own process and never sees the parent's shell variables; the fan-out batches above. - `ship/sections/review-army.md` Step 9: passes, `RANGE_BASE`, classify, defer, render, the cap check before any edit, the Step 11 late-defer row, scratch files under `.gstack/tmp/`, the cap decisions; `ship/sections/adversarial.md` Step 11: the outside voice reads the same range and scopes queued fixes the same way. -- `lib/review-scope.ts`, `bin/gstack-review-scope`: `classify`, `defer`, `render`, `summary`; refusals are `REVIEW_SCOPE_INPUT` and `REVIEW_SCOPE_RANGE` ([troubleshooting.md](troubleshooting.md#review-scope-input)). +- `lib/review-depth.ts`, `bin/gstack-review-plan`: the diff class, the transition table, the numeric-claim scan and each role's effort tier (`test/review-depth.test.ts`, `test/review-plan-cli.test.ts`). +- `lib/review-scope.ts`, `bin/gstack-review-scope`: `classify`, `defer`, `render`, `summary` (with `fix_checks`, `dispatches`, `est_input_tokens`); refusals are `REVIEW_SCOPE_INPUT` and `REVIEW_SCOPE_RANGE` ([troubleshooting.md](troubleshooting.md#review-scope-input)). - `lib/ship-policy.ts`: the `review_scope` key and its `ship: review_scope …` consequence line. - `review/checklist.md`: `prereg-with-results` under Gate Integrity and `test-keyboard-count-loop` under Pass 2; `bin/gstack-coauthor-trailers` for the contributor-credit trailers of a port. - `lib/ship-receipt-prose.ts`, `gstack-ship-receipt read --check-prose`: the body's quoted counters against the receipt (`RECEIPT_PROSE_MISMATCH`, [ship-policy.md](ship-policy.md#ship-receipt)). diff --git a/docs/ship-policy.md b/docs/ship-policy.md index d8b40e32c..e2cd9f208 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -860,15 +860,18 @@ construction: fix-range`) or in the diff is `scope: fix`; nothing below the floor earns a pass. `NO FINDINGS` is the expected reviewer result for a clean range, never a failed review. -3. **Three passes is the cap.** A pass that applied no edits continues to Step 10: - the loop ships with its deferred and noted items named in the PR body. A - blocking finding still in `fix` scope on pass 3 stops the run and names the - finding (path, severity, problem) for the owner instead of a fourth pass. +3. **Two full passes plus one fix-check is the cap**, stepped by the + [transition table](review-policy.md#transitions). A pass that applied no edits + continues to Step 10: the loop ships with its deferred and noted items named in + the PR body. A blocking finding still open after the last allowed step stops + the run and names the finding (path, severity, problem) for the owner instead + of a third full pass. `gstack-review-scope summary --since ` reads the branch's review-log rows for this invocation and writes the counters the ship receipt carries (`gstack-ship-receipt write … --review-loop @`): `review_loop: { passes, -blocking_found, deferred, noted, fixed, scope }`, printed on the receipt summary +blocking_found, deferred, noted, fixed, scope }`, plus `fix_checks`, `dispatches` +and `est_input_tokens` when the run recorded them, printed on the receipt summary line as `passes= blocking_found= deferred=`, so a scorecard can track passes per PR. @@ -933,11 +936,28 @@ A changed product path, or a product patch-id that differs, is (exit 3; the refusal names the paths), and a receipt whose head is not in HEAD's history (a rebase or force-push) or a working tree with uncommitted changes is [`RECEIPT_CARRY_REFUSED`](troubleshooting.md#receipt-carry-refused): run `/ship` -on the new head. `/ship` Step 1 runs the carry check itself when the branch has -an open PR, and a `CARRY: carried` result ends that run with the receipt bound -to HEAD; the queue's `ready`, `train build` and `verify --train` read the +on the new head. The queue's `ready`, `train build` and `verify --train` read the carried receipt like any other. +**Delta /ship: eligibility, then checks, then publication.** `/ship` Step 1 runs +the carry when the branch has an open PR, in this order, and never reruns the +review loop for a carried receipt: + +1. `gstack-ship-receipt carry --pr --base --dry-run`. `CARRY: would + carry` is eligibility only; a refusal continues the full ship. +2. At that pinned HEAD: the CHANGELOG check, the size-cap check the policy + declares, and `gstack-pregate` (the remote tests stage when + `requires_remote` asks for it). A failing check, or HEAD moving, continues + the full ship from Step 2. +3. `gstack-ship-receipt carry --pr --base --pregate ` + rebinds the receipt with a pregate current for HEAD and ends the run. + +A receipt that records a pregate never rebinds without a current one: +`carry` without `--pregate` (or with a stale one) is +[`RECEIPT_CARRY_PREGATE_STALE`](troubleshooting.md#receipt-carry-pregate-stale) +and leaves the body unchanged. A crew that reran the whole `/ship` after every +base merge should run this sequence instead. + ## What `/land-and-deploy` reads from it Step 3.4 runs `show-effective --base ` before the merge and prints one diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 231a8537d..03a377edb 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -4722,6 +4722,19 @@ run did not produce. The `PROSE:` line names each field with both values. stale one (the run was repeated), rerun `/ship` so the receipt is rewritten ([ship-policy.md](ship-policy.md#ship-receipt)). + +### `RECEIPT_CARRY_PREGATE_STALE` + +**Meaning.** The receipt in the body records a pregate result for its old head, +and `gstack-ship-receipt carry` was asked to rebind it to HEAD without a pregate +result current for HEAD (or the one passed with `--pregate` is not current). A +carry certifies the new head, so it never keeps the old tree's checks. Nothing +was rewritten. + +**Fix.** Run `gstack-pregate` at HEAD (the remote tests stage when the policy +requires it), then rerun `carry --pregate `. `--dry-run` still +reports eligibility without one ([ship-policy.md](ship-policy.md#receipt-carry)). + ### `RECEIPT_GIT_TOO_OLD` diff --git a/docs/unattended.md b/docs/unattended.md index 2bc59fa6d..7418fceb4 100644 --- a/docs/unattended.md +++ b/docs/unattended.md @@ -165,14 +165,27 @@ unpushed head is `QUEUE_RECEIPT_STALE` to the queue. The receipt's `review_loop` object, written from `gstack-review-scope summary --since `, is the loop's machine-readable outcome: `passes` -(Step 9 passes this ship ran, 1 to 3), `blocking_found` (distinct blocking -findings across them), `deferred` (blocking findings in unchanged code filed -as issues, Step 11's late defers included), `noted`, `fixed` and `scope`. The +(Step 9 full passes this ship ran, 1 or 2; up to 3 under `review_scope: all`), +`blocking_found` (distinct blocking findings across them), `deferred` +(blocking findings in unchanged code filed as issues, Step 11's late defers +included), `noted`, `fixed` and `scope`. The summary line a parent reads prints it as `passes= blocking_found= deferred=`; the PR body lists the deferred issues under `## Deferred findings` and the below-floor items under `## Review notes`. A pass that finds nothing is `NO FINDINGS` from every reviewer and a pass with no edits, which is the loop's exit, never a failed review ([review-policy.md](review-policy.md)). +Three counters appear only when the run recorded them: `fix_checks` (fix-check +rows, never passes), `dispatches` (reviewer dispatches by role) and +`est_input_tokens` (prompt plus diff bytes the dispatches read, / 4, a relative +estimate and never a bill). + +### Report length + +A parent reads many lanes, so length is a cost it pays per lane. A READY block +holds its required lines plus at most 150 words; a completion report holds at +most 300 words; a subagent brief names the files to read instead of pasting +them. Evidence goes in files the report points to. Waits go on operations, +never on polling wakes. ### The outside voice on Capy @@ -205,8 +218,9 @@ coverage, not as a clean outside review. ### The carry across a base merge or a test-only fix When the parent merges the base in or fixes a test after the receipt was -written, it runs `gstack-ship-receipt carry --pr --base ` instead of -another /ship. `CARRY: carried from to ()` plus the +written, it runs `gstack-ship-receipt carry --pr --base --dry-run` +instead of another /ship; on `CARRY: would carry` it runs `gstack-pregate` at HEAD +and reruns carry with `--pregate .gstack/tmp/pregate.json`. `CARRY: carried from to ()` plus the `GSTACK_RESULT: skill=ship status=complete run=` line means the receipt now names HEAD with a `carry` object holding the `git range-diff` evidence, and `ship-run.json` gains `carried_from` and `carry_reason`; the `review_loop` diff --git a/lib/autoplan-budget.ts b/lib/autoplan-budget.ts index e87189a8e..acb10f2ec 100644 --- a/lib/autoplan-budget.ts +++ b/lib/autoplan-budget.ts @@ -1,21 +1,26 @@ /** * autoplan-budget — the run budget of the unattended runner (field list 2, * W5). Review rounds are counted per phase from `state.history` reopen events - * and capped by `run.json.max_rounds` (default 3 for unattended runs) plus the + * and capped by `run.json.max_rounds` (default 2 for new unattended runs; a + * resumed run keeps the value it persisted) plus the * owner's per-phase grants at the gate. Unattended runs on Capy reserve a * default per-voice estimate for unestimated host attempts, so `--spend-cap` * binds. A dispatch the cap refuses stops the run at the terminal `incomplete` * status with a structured `stop` object (`reason: budget_exhausted`) and * printed recovery commands; `gstack-autoplan budget` journals the owner's cap * change and `resume` lifts the stop only after one. Phase close at the cap - * lives in lib/autoplan-gate.ts (`closeDeferred`). + * lives in lib/autoplan-gate.ts (`closeDeferred`); `escalateLine` names the + * capped phases still holding High or Critical findings. */ +import * as fs from 'node:fs'; import * as path from 'node:path'; import { ANSWERED_BY_RE, type GateItem } from './gate-list'; -import { FILES, RunError, journal, now, readJournal, type RunState, type RunStop } from './autoplan-state'; +import { settledIds } from './autoplan-confirm'; +import type { ConsensusRecord } from './autoplan-reconcile'; +import { FILES, RunError, journal, now, readJournal, voiceRows, type RunState, type RunStop } from './autoplan-state'; import { readLedger, release, setCap, summarize } from './spend-ledger'; -export const DEFAULT_MAX_ROUNDS = 3; +export const DEFAULT_MAX_ROUNDS = 2; /** Reserved per voice for a host-subagent attempt with no `--estimate-usd` in an unattended Capy run. */ export const DEFAULT_HOST_ESTIMATE_USD = 5; @@ -30,6 +35,25 @@ export const atCap = (state: RunState, phase: string): boolean => { const cap = /** The phase is about to open past its cap (a rerun-all reply reopened it). */ export const overCap = (state: RunState, phase: string): boolean => { const cap = roundCap(state, phase); return cap !== null && roundOf(state, phase) > cap; }; +/** Open High or Critical findings of a closed phase: the deferred counts, else bound rows not settled or answered, an outside row paired with a native one counted once. */ +function openHigh(state: RunState, phase: string): number { + const ps = state.phases[phase]; + if (ps?.deferred) return (ps.deferred.counts.High ?? 0) + (ps.deferred.counts.Critical ?? 0); + const file = path.join(state.out, `${phase}-consensus.json`); + let rows: ConsensusRecord['rows'] = []; + try { rows = fs.existsSync(file) ? (JSON.parse(fs.readFileSync(file, 'utf8')) as ConsensusRecord).rows : []; } catch {} + const paired = new Set(rows.filter(r => r.native_id).map(r => r.outside_id)); + const answered = settledIds(state); + return [...voiceRows(state, phase, 'native'), ...voiceRows(state, phase, 'outside')] + .filter(r => !paired.has(r.id) && !(r as { settled_by?: string }).settled_by && !answered.has(r.id) && (r.severity === 'High' || r.severity === 'Critical')).length; +} + +/** At the round cap with High or Critical findings still open, more rounds buy nothing: the owner decides. */ +export function escalateLine(state: RunState): string | undefined { + const held = state.phase_order.filter(p => state.phases[p]?.closed_at && atCap(state, p)).map(p => [p, openHigh(state, p)] as const).filter(([, n]) => n > 0); + return held.length ? `ESCALATE: evidence sufficient, decision needed (${held.map(([p, n]) => `${p}: ${n} High open`).join('; ')})` : undefined; +} + export function roundsLine(state: RunState): string { const cell = (p: string) => `${p}=${roundOf(state, p)}/${roundCap(state, p) ?? 'none'}`; return `ROUNDS: ${state.phase_order.map(cell).join(' ')} max_rounds=${state.max_rounds ?? 'none'} host_spend=unknown`; diff --git a/lib/autoplan-export.ts b/lib/autoplan-export.ts index 3b65c4177..81bb8a276 100644 --- a/lib/autoplan-export.ts +++ b/lib/autoplan-export.ts @@ -6,7 +6,8 @@ * tables, the gate and the GUARD_NOT_INSTALLED line the validator requires), * `findings.jsonl`, `tasks.jsonl`, `decisions.jsonl` (already written by the * gate), `timing.json`, and `run.json` with every artifact's path and hash - * and the counts `gstack-artifact validate` checks against the files. The + * and the counts `gstack-artifact validate` checks against the files, plus + * the reviewer dispatch counts and their relative input-token estimate. The * snapshot bin stays unchanged: this is `gstack-autoplan export --out `. */ import * as fs from 'node:fs'; @@ -14,7 +15,7 @@ import * as path from 'node:path'; import { PHASES } from './autoplan-prompts'; import type { ConsensusRecord } from './autoplan-reconcile'; import { decisionId } from './gate-list'; -import { readDecisions, voiceRows, type RunState } from './autoplan-state'; +import { readDecisions, readJournal, voiceRows, type DispatchCounts, type RunState, type Voice } from './autoplan-state'; import type { FindingRow, TaskRow } from './headless-artifacts'; import { readJsonl, renderUrgentBlock, sha256File } from './headless-artifacts'; @@ -105,6 +106,22 @@ function runLog(state: RunState, findings: number, tasks: number, decisions: { t export interface ExportResult { warnings: string[]; files: string[] } +/** Every dispatched attempt in attempts.jsonl, whatever became of it; prompt bytes come from the row (older rows: the prompt file if still on disk). */ +export function dispatchStats(out: string): { dispatches: DispatchCounts; est_input_tokens: number } { + const dispatches: DispatchCounts = { total: 0, by_phase: {} }; + let bytes = 0; + for (const r of readJournal(out)) { + if (r.event !== 'dispatched') continue; + const by = (dispatches.by_phase[r.phase] ??= {}); + by[r.voice as Voice] = (by[r.voice as Voice] ?? 0) + 1; + dispatches.total += 1; + const file = typeof r.prompt === 'string' ? path.resolve(out, r.prompt) : ''; + bytes += typeof r.prompt_bytes === 'number' ? r.prompt_bytes : file && fs.existsSync(file) ? fs.statSync(file).size : 0; + } + return { dispatches, est_input_tokens: Math.round(bytes / 4) }; +} +export const dispatchLine = (state: RunState) => `DISPATCHES: ${state.dispatches?.total ?? 0} est_input_tokens=${state.est_input_tokens ?? 0} (relative estimate)`; + /** Write the file set and bind every artifact into run.json (counts and hashes); the caller writes run.json. */ export function exportRun(state: RunState): ExportResult { const warnings: string[] = []; @@ -134,6 +151,7 @@ export function exportRun(state: RunState): ExportResult { for (const phase of state.phase_order) for (const name of [`${phase}-consensus.md`, `${phase}-consensus.json`]) { if (fs.existsSync(path.join(state.out, name))) state.artifacts[name] = { path: name, sha256: sha256File(path.join(state.out, name)) }; } + Object.assign(state, dispatchStats(state.out)); state.counts = { decisions: decisions.length, decisions_pending: decisions.filter(d => d.status === 'pending').length, findings: findings.length, tasks: tasks.length }; return { warnings, files }; } diff --git a/lib/autoplan-prompts.ts b/lib/autoplan-prompts.ts index c584e0016..011feba2c 100644 --- a/lib/autoplan-prompts.ts +++ b/lib/autoplan-prompts.ts @@ -12,7 +12,7 @@ import { spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as path from 'node:path'; import { bunIsolationArgs } from './bun-isolation'; -import { readScopeMarker, scopeMarker, scopeTokens, type ScopeFlags, type UiEvidence } from './autoplan-scope'; +import { detectDocsOnly, readScopeMarker, scopeMarker, scopeTokens, type ScopeFlags, type UiEvidence } from './autoplan-scope'; import { COORDINATOR_CONTRACT } from './coordinator-contract'; import { capyBootstrapCommand } from './doctor-components'; import { DX_ERROR_COMPACT } from './dx-error-contract'; @@ -39,6 +39,8 @@ export interface Scope { pin?: { marker: string; file: string } | null; /** Runs created before per-axis provenance. */ source?: 'detected' | 'flags'; + /** A docs-only plan runs CEO (native voice only) and Eng; `--docs-only`, the marker, or every Implementation-plan path docs-class. */ + docs_only?: boolean; docs_only_source?: ScopeSource; } export function snapshotTool(args: string[], cwd: string): any { @@ -61,13 +63,19 @@ export function detectScope(activePlan: string, cwd: string, flags: ScopeFlags, const dxSource: ScopeSource = flags.dx !== undefined ? 'flag' : m.dx !== undefined ? 'marker' : flags.developerTool || flags.agentPrimary ? 'flag' : m.developerTool || m.agentPrimary ? 'marker' : 'detected'; const cli = scopeTokens(flags); - const merged = scopeTokens({ ui: flags.ui ?? m.ui, dx: flags.dx ?? m.dx, developerTool, agentPrimary }); + const merged = scopeTokens({ ui: flags.ui ?? m.ui, dx: flags.dx ?? m.dx, developerTool, agentPrimary, docsOnly: !!(flags.docsOnly || m.docsOnly) }); const pinned = !cli.length || (marker !== null && [...merged].sort().join(' ') === [...marker.tokens].sort().join(' ')); + // A flag or marker that turns a phase on outranks docs-only detection; --docs-only itself outranks everything. + const enables = flags.ui === true || m.ui === true || developerTool || agentPrimary; + const planText = fs.readFileSync(activePlan, 'utf8'); + const docsOnlySource: ScopeSource = flags.docsOnly ? 'flag' : m.docsOnly ? 'marker' : 'detected'; + const docsOnly = !!(flags.docsOnly || m.docsOnly) || (!enables && detectDocsOnly(extractHeading(planText, 'Implementation plan') ?? planText, cwd).docsOnly); return { ui: flags.ui ?? m.ui ?? detected.uiRequired, dx: flags.dx ?? m.dx ?? detected.dxRequired, ui_matches: detected.uiMatches, dx_matches: detected.matchCount, developer_tool: developerTool, agent_primary: agentPrimary, sources: { ui: flags.ui !== undefined ? 'flag' : m.ui !== undefined ? 'marker' : 'detected', dx: dxSource }, ui_evidence: detected.uiEvidence, dx_terms: detected.matches, cli_flags: cli, marker: marker?.tokens ?? null, pin: pinned ? null : { marker: scopeMarker(merged), file: sourcePlan }, + docs_only: docsOnly, docs_only_source: docsOnlySource, }; } @@ -85,12 +93,12 @@ export function scopeLines(scope: Scope): string[] { const ev = scope.ui_evidence; const ui = `ui=${scope.ui ? 'yes' : 'no'} (terms: ${list(ev?.terms)}; visual: ${ev?.visual.length ? ev.visual.join(', ') : 'none'}; files: ${ev?.files.length ? ev.files.join(', ') : 'none'})`; const dx = `dx=${scope.dx ? 'yes' : 'no'} (detected: ${list(scope.dx_terms)}${scope.developer_tool ? '; developer tool' : ''}${scope.agent_primary ? '; agent primary' : ''})`; - const lines = [`SCOPE: ${ui} ${dx} source=${scopeSourceLabel(scope)}`]; + const lines = [`SCOPE: ${ui} ${dx} docs_only=${scope.docs_only ? 'yes' : 'no'} (${scope.docs_only_source ?? 'detected'}) source=${scopeSourceLabel(scope)}`]; if (scope.pin) lines.push(`SCOPE_PIN: add ${scope.pin.marker} as the first line of ${scope.pin.file} to keep these flags`); return lines; } export function phaseOrder(scope: Scope): Phase[] { - return PHASES.filter(p => (p === 'design' ? scope.ui : p === 'dx' ? scope.dx : true)); + return PHASES.filter(p => (p === 'design' ? scope.ui && !scope.docs_only : p === 'dx' ? scope.dx && !scope.docs_only : true)); } /** Extract `## ` … next same-level heading, for repos whose plans use a custom heading. */ diff --git a/lib/autoplan-revise.ts b/lib/autoplan-revise.ts index 572927853..77c790de5 100644 --- a/lib/autoplan-revise.ts +++ b/lib/autoplan-revise.ts @@ -9,7 +9,11 @@ * when named: `--incorporates ` binds each id to the old and new hashes * and refuses an unchanged plan; `--withdraw ` is the owner's journaled * decision to drop one. Archives keep `.r` names with `.v` only on - * collision. One recoverable transaction: the staged plan and state are + * collision. An edit that only incorporates decisions whose source findings + * are all below High (no user challenge, no newly in-scope phase) applies + * without re-review: the touched phases' reviewed hash is rebound to the + * revision and `applied_without_rereview` is journaled; anything unsure + * reopens. One recoverable transaction: the staged plan and state are * written and `revise_intent` journaled before anything moves; `resume` * (recoverRevise) completes an interrupted revise from the stage. */ @@ -17,13 +21,13 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { exportRun } from './autoplan-export'; import { applyRenames, approve, closeDraftDirections, finishGate, planReopen } from './autoplan-gate'; -import { atCap, pendingDeltas, roundCap, roundOf } from './autoplan-budget'; +import { atCap, escalateLine, pendingDeltas, roundCap, roundOf } from './autoplan-budget'; import { diffStat, lineDiff } from './autoplan-confirm'; import { SETTLEABLE_CHANGES } from './autoplan-reconcile'; import { normalizeRevision, reviewHash, scaffold } from './autoplan-plan-text'; import { parseScopeTokens } from './autoplan-scope'; import { PHASES, detectScope, extractHeading, phaseOrder, type Phase, type ScopeFlags } from './autoplan-prompts'; -import { FILES, RunError, journal, now, pendingAmendments, readJournal, readState, writeState, type RunState } from './autoplan-state'; +import { FILES, RunError, journal, now, pendingAmendments, readDecisions, readJournal, readState, voiceRows, writeState, type RunState } from './autoplan-state'; export interface ReviseOptions { out: string; plan?: string; incorporates?: string[]; withdraw?: string[]; settle?: string[]; cwd: string } export interface ReviseResult { state: RunState; reopened: Phase[]; lines: string[] } @@ -82,11 +86,31 @@ function settleEvidence(state: RunState, ids: string[], revised?: string): { pla return { plan_sha256: active, changes }; } +const SEVERITY_ORDER = ['Informational', 'Low', 'Medium', 'High', 'Critical']; +export const BELOW_HIGH = ['Informational', 'Low', 'Medium']; + +/** + * The severity an incorporated decision resolves to: the highest of the findings its gate item named (the + * decisions.jsonl origin's ids, looked up in that phase's bound rows). `unknown` when the row, its origin or a + * finding is missing; `challenge` for a user challenge, whose direction change is always re-reviewed. + */ +export function decisionSeverity(state: RunState, decision: string): string { + const row = readDecisions(state.out).find(d => d.id === decision); + const [phase, ids] = row?.origin?.split('|') ?? []; + if (!phase || !ids || !state.phases[phase]?.closed_at) return 'unknown'; + const rows = [...voiceRows(state, phase, 'native'), ...voiceRows(state, phase, 'outside')]; + const found = ids.split('+').filter(id => id && id !== 'null').map(id => rows.find(r => r.id === id)); + if (!found.length || found.some(f => !f)) return 'unknown'; + if (found.some(f => (f as { user_challenge?: boolean }).user_challenge)) return 'challenge'; + return found.map(f => f!.severity).reduce((a, b) => (SEVERITY_ORDER.indexOf(b) > SEVERITY_ORDER.indexOf(a) ? b : a)); +} + /** Every accepted change is in: approve, unless a plan delta after a round cap still needs the owner (a new gate revision). */ function approveOrRegate(state: RunState, at: string): string[] { if (pendingDeltas(state).length) { finishGate(state); - return [`GATE: gate_rev=${state.gate_rev} (a plan change after a round cap needs the owner) render with: gstack-gate render ${path.join(state.out, 'gate.json')}`]; + const escalate = escalateLine(state); + return [`GATE: gate_rev=${state.gate_rev} (a plan change after a round cap needs the owner) render with: gstack-gate render ${path.join(state.out, 'gate.json')}`, ...(escalate ? [escalate] : [])]; } return [...approve(state, at), `APPROVED: gate_rev=${state.gate_rev} (every accepted change is in the plan or settled)`]; } @@ -142,10 +166,14 @@ export function reviseRun(o: ReviseOptions): ReviseResult { const opened = state.phase_order.filter(p => state.phases[p]?.snapshot); const changed = opened.filter(p => state.phases[p]!.snapshot !== newHash); const touched = state.phase_order.filter(p => changed.includes(p) || (p === 'eng' && opened.includes('eng') && changed.length > 0)); + // Below-High decisions only, every touched phase closed, nothing newly in scope (no added phase, docs-only still holds): the edit is theirs, so no re-review. + const severities = incorporates.map(id => [id, decisionSeverity(state, id)] as const); + const quiet = severities.length > 0 && severities.every(([, sev]) => BELOW_HIGH.includes(sev)) && touched.length > 0 && !added.length + && touched.every(p => state.phases[p]?.closed_at) && !(state.scope.docs_only && !detected.docs_only); // A phase that used its last round is not reopened: the delta becomes an owner gate item, so the cap neither loops nor hides it. - const capped = touched.filter(p => atCap(state, p)); - const reopen = touched.filter(p => !capped.includes(p)); - const delta = capped.length ? diffStat(lineDiff(fs.readFileSync(state.active_plan, 'utf8'), fs.readFileSync(path.join(staging, FILES.active), 'utf8'))) : { added: 0, removed: 0 }; + const capped = quiet ? [] : touched.filter(p => atCap(state, p)); + const reopen = quiet ? [] : touched.filter(p => !capped.includes(p)); + const delta = capped.length || quiet ? diffStat(lineDiff(fs.readFileSync(state.active_plan, 'utf8'), fs.readFileSync(path.join(staging, FILES.active), 'utf8'))) : { added: 0, removed: 0 }; const atGate = state.status === 'gate_pending' ? state.gate_rev : undefined; journal(out, { event: 'revise_intent', k, staging: path.relative(out, staging), plan: file, old_plan_sha256: oldHash, new_plan_sha256: newHash, at_gate_rev: atGate ?? null, incorporates, withdraw, settle }); @@ -154,6 +182,11 @@ export function reviseRun(o: ReviseOptions): ReviseResult { (state.capped_deltas ??= []).push({ k, phase, round: roundOf(state, phase), cap: roundCap(state, phase)!, old_plan_sha256: oldHash, new_plan_sha256: newHash, ...delta, status: 'pending', at }); journal(out, { event: 'cap_delta', k, phase, round: roundOf(state, phase), cap: roundCap(state, phase), old_plan_sha256: oldHash, new_plan_sha256: newHash, ...delta }); } + for (const p of quiet ? touched : []) { + const ps = state.phases[p]!; + (ps.rebound ??= []).push({ k, from: ps.snapshot!, to: newHash, decisions: incorporates, at }); + ps.snapshot = newHash; + } for (const p of added) state.phases[p] = {}; state.phase_order = order; state.required_phases = order; state.scope = { ...detected, ui: detected.ui || state.scope.ui, dx: detected.dx || state.scope.dx }; @@ -161,13 +194,16 @@ export function reviseRun(o: ReviseOptions): ReviseResult { state.plan_input = file; state.plan_source = path.join(out, `plan-source.v${k}.md`); state.plan_revision = k; delete state.plan_reality; const settled = [...settleAmendments(state, incorporates, 'incorporated', at, { old_plan_sha256: oldHash, new_plan_sha256: newHash }), ...settleAmendments(state, withdraw, 'withdrawn', at, { by: 'owner' }), - ...settleAmendments(state, settle, 'settled', at, { plan_sha256: evidence?.plan_sha256 }).map(r => ({ ...r, changes: evidence!.changes[r.decision as string] }))]; + ...settleAmendments(state, settle, 'settled', at, { plan_sha256: evidence?.plan_sha256 }).map(r => ({ ...r, changes: evidence!.changes[r.decision as string] })), + ...(quiet ? [{ event: 'applied_without_rereview', k, decisions: incorporates, severities: Object.fromEntries(severities), phases: touched, old_plan_sha256: oldHash, new_plan_sha256: newHash }] : [])]; if (amending) state.status = pendingAmendments(state).length ? 'amend_pending' : state.phase_order.some(p => !state.phases[p]?.closed_at) ? 'running' : pendingDeltas(state).length ? 'gate_pending' : 'complete'; - const regate = capped.length > 0 && state.status === 'gate_pending'; + // A quiet revise that ends at gate_pending still has an earlier capped delta waiting: the owner gets a new gate for it. + const regate = (capped.length > 0 || quiet) && state.status === 'gate_pending'; const committed = { event: 'plan_revised', k, plan: file, old_plan_sha256: oldHash, new_plan_sha256: newHash, reopened: reopen, added, at_gate_rev: atGate ?? null }; fs.writeFileSync(path.join(staging, FILES.run), JSON.stringify({ state, renames, committed, settled }, null, 2) + '\n'); publish(state, staging, renames, committed, settled); - const lines = [`REVISED: plan=${newHash.slice(0, 12)} reopened=${reopen.join(',') || 'none'}${added.length ? ` added=${added.join(',')}` : ''}${capped.length ? ` capped=${capped.join(',')}` : ''}`, amendmentLine(state)]; + const lines = [`REVISED: plan=${newHash.slice(0, 12)} reopened=${reopen.join(',') || 'none'}${added.length ? ` added=${added.join(',')}` : ''}${capped.length ? ` capped=${capped.join(',')}` : ''}`, + ...(quiet ? [`APPLIED: ${incorporates.join(',')} without re-review (below High; plan +${delta.added} -${delta.removed} lines)`] : []), amendmentLine(state)]; for (const phase of capped) lines.push(`CAPPED: ${phase} used round ${roundOf(state, phase)} of ${roundCap(state, phase)}: not reopened; the gate asks the owner about the unreviewed delta (+${delta.added} -${delta.removed} lines)`); if (regate) { lines.push(...approveOrRegate(state, at)); diff --git a/lib/autoplan-run.ts b/lib/autoplan-run.ts index 3b03ef132..f1cce543f 100644 --- a/lib/autoplan-run.ts +++ b/lib/autoplan-run.ts @@ -109,7 +109,7 @@ export function initRun(o: InitOptions, lines: string[] = []): RunState { const consentMissing = consent && !consent.allowed ? consent.lines[0] : undefined; const sessionKind = o.sessionKind ?? process.env.GSTACK_SESSION_KIND ?? 'unattended'; const phases: Record = {}; - for (const p of PHASES) if (!order.includes(p)) phases[p] = { skipped: p === 'design' ? 'no UI scope detected' : 'no developer-facing scope detected', native: { status: 'skipped' }, outside: { status: 'skipped' } }; + for (const p of PHASES) if (!order.includes(p)) phases[p] = { skipped: scope.docs_only ? 'docs-only plan' : p === 'design' ? 'no UI scope detected' : 'no developer-facing scope detected', native: { status: 'skipped' }, outside: { status: 'skipped' } }; const state: RunState = { schema_version: 1, run, status: 'running', required_phases: order, phases, artifacts: {}, revision: 0, out, plan_source: init.sourcePlan, active_plan: init.activePlan, restore: init.restorePath, scope, phase_order: order, @@ -169,7 +169,8 @@ function openPhase(state: RunState, phase: Phase, cwd: string): void { ps.sections = { loaded: sections.loaded, skipped: sections.skipped, checklist: !!sections.checklist }; ps.native = { status: 'pending', attempts: [] }; const notRun = state.outside_not_run as { reason: string; detail: string } | undefined; - ps.outside = state.outside_enabled ? { status: 'pending', attempts: [] } + ps.outside = phase === 'ceo' && state.scope.docs_only ? { status: 'disabled', outcome: 'not_run', reason: 'docs_only', detail: 'docs-only plan: CEO runs the native voice only; Eng keeps both voices' } + : state.outside_enabled ? { status: 'pending', attempts: [] } : notRun ? { status: 'disabled', outcome: 'not_run', reason: notRun.reason, detail: notRun.detail } : { status: 'disabled' }; ps.prompts = prompts; journal(state.out, { event: 'phase_opened', phase, snapshot: snap.sha256, prompts: Object.fromEntries(Object.entries(prompts).map(([k, p]) => [k, path.relative(state.out, p)])) }); @@ -202,6 +203,7 @@ export function nextStep(o: NextOptions): NextResult { if (drift) lines.push(drift); if (first) lines.push(...scopeLines(state.scope), `DRIVER: ${driverFile()}`); if (ps.outside?.reason === 'consent_missing') lines.push(`OUTSIDE: ${phase} not_run reason=consent_missing`, String(ps.outside.detail)); + if (ps.outside?.reason === 'docs_only') lines.push(`OUTSIDE: ${phase} not_run reason=docs_only (native voice only)`); if (ps.sections) lines.push(`Skipped sections: ${ps.sections.skipped.length ? ps.sections.skipped.join(',') : 'none'} (scope: ui=${state.scope.ui ? 'yes' : 'no'},dx=${state.scope.dx ? 'yes' : 'no'},source=${scopeSourceLabel(state.scope)}; checklist=${ps.sections.checklist ? 'loaded' : 'none'})`); const prompts = ps.prompts!; const estimate = attemptEstimate(state, o.estimateUsd, process.env); @@ -231,7 +233,7 @@ export function nextStep(o: NextOptions): NextResult { catch (e: any) { if (e.code === 'SPEND_CAP_EXCEEDED') { state.attempt_counter--; writeState(state); throw new RunError('SPEND_CAP_EXCEEDED', e.message, 3); } throw e; } const row: AttemptState = { attempt, state: 'dispatched', prompt: prompts[voice], result: path.join(state.out, attemptFile(phase, voice, attempt.slice(state.run.length + 1), 'md')), estimate_usd: estimate.usd, dispatched_at: now() }; bindPromptResult(row.prompt, row.result); - journal(state.out, { event: 'dispatched', attempt, phase, voice, snapshot: ps.snapshot, prompt: path.relative(state.out, row.prompt), prompt_sha256: sha256File(row.prompt), result: path.relative(state.out, row.result), estimate_usd: estimate.usd }); + journal(state.out, { event: 'dispatched', attempt, phase, voice, snapshot: ps.snapshot, prompt: path.relative(state.out, row.prompt), prompt_sha256: sha256File(row.prompt), prompt_bytes: fs.statSync(row.prompt).size, result: path.relative(state.out, row.result), estimate_usd: estimate.usd }); (v.attempts ??= []).push(row); v.attempt = attempt; v.status = 'running'; result.attempts.push({ attempt, voice, prompt: row.prompt, result: row.result, model_family: familyHint(state, voice), new: true }); diff --git a/lib/autoplan-scope.ts b/lib/autoplan-scope.ts index 142305862..736bcf437 100644 --- a/lib/autoplan-scope.ts +++ b/lib/autoplan-scope.ts @@ -7,8 +7,11 @@ * file, or a UI term next to a visual noun. Bare UI words are weak evidence: * they keep the runner's conservative section fallback but do not add the * Design phase. The scope marker is the author's way to pin flags in the plan - * itself; it is read, never written. + * itself; it is read, never written. `detectDocsOnly` classifies the paths + * the Implementation plan names: a plan that touches only markdown no agent + * or gate consumes is docs-only (CEO + Eng, outside voice on Eng only). */ +import { classifyPath } from './review-depth'; export const UI_TERMS = ['component', 'screen', 'form', 'button', 'modal', 'layout', 'dashboard', 'sidebar', 'nav', 'dialog'] as const; export const VISUAL_NOUNS = ['page', 'layout', 'style', 'pixel', 'view', 'browser', 'css'] as const; @@ -44,13 +47,29 @@ export function detectUiScope(implementationPlan: string): UiScope { return { uiRequired: files.length > 0 || visual.length > 0, uiMatches: terms.reduce((s, t) => s + t.count, 0), uiEvidence: { terms, visual: [...new Set(visual)], files } }; } -export interface ScopeFlags { ui?: boolean; developerTool?: boolean; agentPrimary?: boolean; dx?: boolean } -export const SCOPE_FLAG_TOKENS = ['--ui', '--no-ui', '--no-dx', '--developer-tool', '--agent-primary'] as const; +/** Docs-class: what /ship's review classifies as docs (lib/review-depth.ts classifyPath: README, CHANGELOG, TODOS and prose markdown outside agent-read and skill-directory paths), plus a docs directory named without a file. */ +export function docsClassPath(file: string, root: string): boolean { + const p = file.replace(/^\.\//, ''); + if (p.startsWith('docs/') && !/\.mdx?$/i.test(p)) return true; + return classifyPath(p, root) === 'docs'; +} + +export interface DocsOnlyScope { docsOnly: boolean; paths: string[]; code: string[] } +/** Docs-only when the Implementation plan names at least one path and every one is docs-class; unsure means not docs-only. */ +export function detectDocsOnly(implementationPlan: string, root: string): DocsOnlyScope { + const paths = [...new Set([...implementationPlan.matchAll(/[\w@~$.\/*-]+/g)].map(m => m[0].replace(/[.\-]+$/, '')) + .filter(t => !/^[a-z]+:\/\//i.test(t) && t.length > 1 && (t.includes('/') || /\.[a-z][a-z0-9]*$/i.test(t))))]; + const code = paths.filter(p => !docsClassPath(p, root)); + return { docsOnly: paths.length > 0 && code.length === 0, paths, code }; +} + +export interface ScopeFlags { ui?: boolean; developerTool?: boolean; agentPrimary?: boolean; dx?: boolean; docsOnly?: boolean } +export const SCOPE_FLAG_TOKENS = ['--ui', '--no-ui', '--no-dx', '--developer-tool', '--agent-primary', '--docs-only'] as const; const MARKER_RE = /^\s*$/; export function scopeTokens(f: ScopeFlags): string[] { return [...(f.ui === true ? ['--ui'] : f.ui === false ? ['--no-ui'] : []), ...(f.dx === false ? ['--no-dx'] : []), - ...(f.developerTool ? ['--developer-tool'] : []), ...(f.agentPrimary ? ['--agent-primary'] : [])]; + ...(f.developerTool ? ['--developer-tool'] : []), ...(f.agentPrimary ? ['--agent-primary'] : []), ...(f.docsOnly ? ['--docs-only'] : [])]; } export function parseScopeTokens(tokens: readonly string[], where: string): ScopeFlags { @@ -62,6 +81,7 @@ export function parseScopeTokens(tokens: readonly string[], where: string): Scop ...(tokens.includes('--no-dx') ? { dx: false } : {}), ...(tokens.includes('--developer-tool') ? { developerTool: true } : {}), ...(tokens.includes('--agent-primary') ? { agentPrimary: true } : {}), + ...(tokens.includes('--docs-only') ? { docsOnly: true } : {}), }; } diff --git a/lib/autoplan-state.ts b/lib/autoplan-state.ts index f82ce08c9..c10b3551f 100644 --- a/lib/autoplan-state.ts +++ b/lib/autoplan-state.ts @@ -50,7 +50,11 @@ export interface PhaseState { deferred?: DeferredClose; /** A confirm round (r > 1): the revision this phase last reviewed. */ confirm?: { round: number; previous_sha256?: string; added: number; removed: number }; + /** Revisions bound to this phase without re-review: every incorporated decision was below High (lib/autoplan-revise.ts). */ + rebound?: Array<{ k: number; from: string; to: string; decisions: string[]; at: string }>; } +/** Reviewer attempts journaled as dispatched (failed, unknown and superseded ones included), per phase and voice. */ +export interface DispatchCounts { total: number; by_phase: Record>> } export interface RunState extends RunManifest { revision: number; out: string; plan_source: string; active_plan: string; restore: string; scope: Scope; phase_order: Phase[]; phases: Record; native_model?: string; outside_model?: string; outside_enabled: boolean; attempt_counter: number; @@ -64,6 +68,8 @@ export interface RunState extends RunManifest { /** Review rounds per phase (null: uncapped); round_grants are the owner's per-phase raises at the gate. */ max_rounds?: number | null; round_grants?: Record; stop?: RunStop; stops?: RunStop[]; capped_deltas?: CappedDelta[]; + /** Written by export from attempts.jsonl; est_input_tokens is dispatched prompt bytes / 4, a relative estimate only. Absent on older runs. */ + dispatches?: DispatchCounts; est_input_tokens?: number; } export const TERMINAL_VOICE_STATUSES = ['completed', 'disabled', 'unavailable', 'failed', 'skipped'] as const; diff --git a/lib/headless-artifacts.ts b/lib/headless-artifacts.ts index c9fa91335..4bd39e3c7 100644 --- a/lib/headless-artifacts.ts +++ b/lib/headless-artifacts.ts @@ -98,7 +98,7 @@ export interface ShipReceipt { // Related open PRs by other authors (Lane D): `supersedes` / `credit` only on the owner's confirmation. issues?: { fixes: number[]; refs: number[]; flakes: Array<{ issue: number; run: string }>; related?: Array<{ pr: number; repo?: string; author: string; closes: number[]; supersedes?: boolean; credit?: boolean }> }; /** Harness lane E: the review loop's counters (gstack-review-scope summary) and the carry record when the receipt followed a head whose product diff was unchanged (gstack-ship-receipt carry). */ - review_loop?: { passes: number; blocking_found: number; deferred: number; noted?: number; fixed?: number; scope?: string }; + review_loop?: { passes: number; blocking_found: number; deferred: number; noted?: number; fixed?: number; scope?: string; fix_checks?: number; dispatches?: Record; est_input_tokens?: number }; carry?: { from: string; reason: string; product_patch_id: string; changed_paths: string[]; merge_commits: number; range_diff: string; covers: string[]; reruns: string[] }; } @@ -195,6 +195,8 @@ export const ARTIFACT_SCHEMAS = { gate_rev: { type: 'integer' }, gate: { type: 'object' }, consumed_by: { type: 'array', items: { type: 'object', required: ['consumer', 'at'], properties: { consumer: str, at: str } } }, deviations: strList, + dispatches: { type: 'object', description: 'total and by_phase → { native, outside }: reviewer attempts from attempts.jsonl, failed and unknown included' }, + est_input_tokens: { type: 'integer', minimum: 0, description: 'dispatched prompt bytes / 4: a relative estimate, never a bill' }, }, }, pregate: { @@ -216,7 +218,7 @@ export const ARTIFACT_SCHEMAS = { thread: str, tree: { enum: ['same-modulo-stamps', 'changed', 'not-compared'] }, gate_reuse: str, policy: str, queue_mode: str, preregistration: str, history: str, pregate: { ...str, description: 'pass|warn|requires-remote cleared … @ from pregate.json, current for this head' }, requires_remote: strList, approval: { type: 'object', description: 'approval classes: { source, status: checked|unavailable, classes: [{ class, scope, effects, citation: verified|unverified, approved_by, date, via, binding, evidence }], report_only }' }, - review_loop: { type: 'object', required: ['passes', 'blocking_found', 'deferred'], properties: { passes: { type: 'integer', minimum: 0 }, blocking_found: { type: 'integer', minimum: 0 }, deferred: { type: 'integer', minimum: 0 }, noted: { type: 'integer', minimum: 0 }, fixed: { type: 'integer', minimum: 0 }, scope: str } }, + review_loop: { type: 'object', required: ['passes', 'blocking_found', 'deferred'], properties: { passes: { type: 'integer', minimum: 0 }, blocking_found: { type: 'integer', minimum: 0 }, deferred: { type: 'integer', minimum: 0 }, noted: { type: 'integer', minimum: 0 }, fixed: { type: 'integer', minimum: 0 }, scope: str, fix_checks: { type: 'integer', minimum: 0 }, dispatches: { type: 'object', description: 'reviewer dispatches by role' }, est_input_tokens: { type: 'integer', minimum: 0 } } }, carry: { type: 'object', required: ['from', 'reason', 'product_patch_id'], properties: { from: str, reason: str, product_patch_id: str, changed_paths: strList, merge_commits: { type: 'integer', minimum: 0 }, range_diff: str, covers: strList, reruns: strList } }, issues: { type: 'object', properties: { fixes: { type: 'array', items: { type: 'integer' } }, refs: { type: 'array', items: { type: 'integer' } }, flakes: { type: 'array', items: { type: 'object', required: ['issue', 'run'], properties: { issue: { type: 'integer' }, run: str } } }, related: { type: 'array', items: { type: 'object', required: ['pr', 'author', 'closes'], properties: { pr: { type: 'integer' }, repo: str, author: str, closes: { type: 'array', items: { type: 'integer' } }, supersedes: { type: 'boolean' }, credit: { type: 'boolean' } } } } } }, }, diff --git a/lib/result-codes-release.ts b/lib/result-codes-release.ts index 3fc648cb1..8c7e752ea 100644 --- a/lib/result-codes-release.ts +++ b/lib/result-codes-release.ts @@ -269,6 +269,11 @@ export const RELEASE_RESULT_CODES = { summary: 'the PR body quotes a counter the ship receipt also records (review passes, deferred findings, gate case counts) with a different number, so the prose and the receipt disagree about the same run', fix: 'correct the body to the receipt\'s number (the PROSE: line names each field with both values); when the receipt is the stale one, rerun /ship so the receipt is rewritten from the run', }, + RECEIPT_CARRY_PREGATE_STALE: { + anchor: 'receipt-carry-pregate-stale', + summary: 'gstack-ship-receipt carry would rebind a receipt that records a pregate without a pregate result current for HEAD, so the carried receipt would claim the old tree\'s checks', + fix: 'run gstack-pregate at HEAD (the remote tests stage when the policy requires it), then rerun carry with --pregate ', + }, RECEIPT_GIT_TOO_OLD: { anchor: 'receipt-git-too-old', summary: 'gstack-ship-receipt carry needs git 2.39 or later for `git patch-id --verbatim`; the installed git is older, so the product diff could not be compared', diff --git a/lib/review-depth.ts b/lib/review-depth.ts new file mode 100644 index 000000000..f7331dbba --- /dev/null +++ b/lib/review-depth.ts @@ -0,0 +1,223 @@ +/** + * review-depth — the one policy owner for how much review a /ship run dispatches + * (docs/review-policy.md#review-depth). A pure function from the changed paths, + * gstack-diff-scope flags, the diff size, the gate scan and the pass history to a + * diff class, the next step of the loop and the reviewers that step dispatches, + * each with its reasoning-effort tier. bin/gstack-review-plan prints it; the + * review-army resolver and the skills read the same definitions. + */ +import { createHash } from 'node:crypto'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; + +export type DiffClass = 'docs' | 'tests' | 'code' | 'gate'; +export type PathClass = 'docs' | 'tests' | 'code'; +export type Effort = 'light' | 'full' | 'host'; +export type Role = 'specialist' | 'red-team' | 'adversarial-native' | 'outside-voice' | 'fix-check'; +export type Step = 'full' | 'fix-check' | '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]; +export const FULL_PASS_CAP = 2; +export const SMALL_DIFF_LINES = 50; +export const RED_TEAM_LINES = 200; +export const LARGE_DIFF_LINES = 100; +const CLASS_RANK: Record = { docs: 0, tests: 1, code: 2, gate: 3 }; + +const AGENT_PROSE = [/(^|\/)SKILL\.md(\.tmpl)?$/, /(^|\/)sections\//, /^scripts\/resolvers\//, /^docs\/[^/]*-policy\.md$/, /^docs\/(captain|unattended|pregate)\.md$/, /(^|\/)DRIVER\.md(\.tmpl)?$/, /(^|\/)templates\//, /(^|\/)(AGENTS|CLAUDE|ETHOS)\.md$/, /(^|\/)specialists\//, /(^|\/)checklist\.md$/, /^\.gstack\//]; +/** gstack-diff-scope's TEST_PATH_RE (the test-bootstrap rule), plus fixture directories. */ +const TEST_PATH = [/(^|\/)(tests?|spec|__tests__)\/|(^|\/)tests?\.py$|(^|\/)test_[^/]+\.py$|_test\.(go|py|rb|ts|js|exs|dart)$|\.(test|spec)\.[jt]sx?$|_spec\.rb$|Test\.(java|kt)$/, /(^|\/)fixtures?\//]; +const DOC_PATH = /\.(md|mdx|markdown|rst|txt|adoc)$/i; +/** Release metadata a version stamp writes: never a reason for review depth on its own. */ +const RELEASE_METADATA = [/^CHANGELOG\.md$/, /^VERSION$/, /^agents-digest\//]; +const GATE_PATH = [/(^|\/)\.github\/workflows\//, /(^|\/)(secrets?|credentials?|keychain|token[s]?)([._/-]|$)/i, /state-root/, /(^|\/)(pregate|gate|ship-receipt|ship-policy)[^/]*\.(ts|sh|json)$/, /(^|\/)bin\/gstack-(pregate|gate|ship-receipt|verify-gate|ci-gate)$/]; + +/** One changed path's class. Markdown an agent or a gate reads (skills, sections, resolvers, policy and rule docs, templates, any markdown in a skill directory under `root`) is code: it is gstack's product. */ +export function classifyPath(p: string, root?: string): PathClass { + if (RELEASE_METADATA.some(r => r.test(p))) return 'docs'; + if (AGENT_PROSE.some(r => r.test(p))) return 'code'; + if (TEST_PATH.some(r => r.test(p))) return 'tests'; + if (!DOC_PATH.test(p)) return 'code'; + const top = p.includes('/') ? p.split('/')[0]! : ''; + if (root && top && ['SKILL.md', 'SKILL.md.tmpl'].some(f => fs.existsSync(path.join(root, top, f)))) return 'code'; + return 'docs'; +} + +export interface ClassInput { + paths: readonly string[]; + /** gstack-diff-scope output (SCOPE_* keys). */ + scope?: Record; + /** Gate hunks the gate scan tagged (an RH rule: a relaxed threshold, a skipped test, a loosened check); null when the scan did not run or failed. Plain test edits are listed, never tagged, and do not count. */ + gateTagged: number | null; + /** Paths the diff deletes or renames away. */ + deleted?: readonly string[]; + /** Paths whose whole change is a version stamp (package.json's version key): classified as docs. */ + stampOnly?: readonly string[]; + /** The repository root: markdown inside a skill directory there is code. */ + root?: string; +} +export interface ClassResult { class: DiffClass; reasons: string[] } + +/** The diff's class: the most restrictive of its paths, raised to gate by any gate signal; errors fail closed. */ +export function classifyDiff(i: ClassInput): ClassResult { + const reasons: string[] = []; + let cls: DiffClass = 'docs'; + const raise = (c: DiffClass, why: string) => { if (CLASS_RANK[c] > CLASS_RANK[cls]) cls = c; reasons.push(why); }; + if (!i.paths.length) raise('docs', 'no changed paths'); + for (const p of i.paths) { const c = (i.stampOnly ?? []).includes(p) ? 'docs' : classifyPath(p, i.root); if (CLASS_RANK[c] > CLASS_RANK[cls]) raise(c, `${c}: ${p}`); } + const s = i.scope ?? {}; + if (s.SCOPE_ERROR) raise('code', `scope error: ${s.SCOPE_ERROR}`); + if (CLASS_RANK[cls] >= CLASS_RANK.tests) { + if (i.gateTagged === null) raise('gate', 'gate scan unavailable'); + else if (i.gateTagged > 0) raise('gate', `gate scan tagged ${i.gateTagged} hunk(s)`); + const deletedGuarded = (i.deleted ?? []).filter(p => classifyPath(p, i.root) !== 'docs'); + if (deletedGuarded.length) raise('gate', `deletes ${deletedGuarded.length} code or test path(s)`); + } + if (CLASS_RANK[cls] >= CLASS_RANK.code) { + if (s.SCOPE_AUTH === 'true') raise('gate', 'auth paths'); + if (s.SCOPE_MIGRATIONS === 'true') raise('gate', 'migrations'); + for (const p of i.paths) if (GATE_PATH.some(r => r.test(p))) { raise('gate', `gate path: ${p}`); break; } + } + return { class: cls, reasons }; +} + +// --------------------------------------------------------------------------- +// numeric claims: the outside voice's trigger on docs and tests diffs +// --------------------------------------------------------------------------- +const CLAIM = new RegExp([ + String.raw`\d+(?:\.\d+)?\s?%`, String.raw`[$€£]\s?\d`, String.raw`\b\d+(?:\.\d+)?\s?x\b`, + String.raw`\b\d+(?:\.\d+)?\s?(?:ms|s|seconds?|minutes?|hours?|tokens?|credits?|[kKmM]tok)\b`, + String.raw`(?≤≥]\s*-?\d`, String.raw`\b\d+\s*/\s*\d+\b`, String.raw`\b\d+(?:\.\d+)?e-?\d+\b`, + String.raw`\b(?:p|n|median|mean|average|rate|score|accuracy|recall|precision|speedup|faster|slower|cheaper|cost|hits?|misses)\b[^\n]{0,40}\d`, +].join('|'), 'i'); +/** Not claims: dates, versions, issue numbers, step numbers. Stripped before the scan. */ +const NOT_CLAIM = /\b\d{4}-\d{2}-\d{2}\b|\bv?\d+(?:\.\d+){2,}\b|#\d+\b|\b(?:step|phase|pass|rule|item|round)\s+\d+(?:\.\d+)?\b/gi; +export interface NumbersResult { publishes: boolean; claims: number; fingerprint: string } + +/** Where a number is published: prose and data files. Code and test source are not publications. */ +const PUBLISHED = /\.(md|mdx|markdown|rst|txt|adoc|json|csv|tsv)$/i; + +/** Added lines (path → lines) → whether the diff publishes numeric claims in prose or data files, and a fingerprint of those claims and their files. */ +export function scanNumbers(added: Record): NumbersResult { + const hits: string[] = []; + for (const p of Object.keys(added).filter(f => PUBLISHED.test(f)).sort()) for (const line of added[p] ?? []) if (CLAIM.test(line.replace(NOT_CLAIM, ' '))) hits.push(`${p}\t${line.trim()}`); + const fingerprint = hits.length ? createHash('sha256').update(hits.join('\n')).digest('hex').slice(0, 16) : 'none'; + return { publishes: hits.length > 0, claims: hits.length, fingerprint }; +} + +// --------------------------------------------------------------------------- +// the loop: which step comes next +// --------------------------------------------------------------------------- +export interface History { + /** Full review passes already run in this invocation (0 before pass 1). */ + fullPasses: number; + /** The step that just finished; none before pass 1. */ + last: 'none' | 'full' | 'fix-check'; + /** The finished step fixed at least one blocking finding. */ + fixedBlocking?: boolean; + /** The finished step fixed findings below the floor (pass 1 only fixes those). */ + fixedBelow?: boolean; + /** A blocking finding is still open in fix scope (unfixed). */ + blockingOpen?: boolean; + /** A fix in the finished step touched a gate-class path. */ + touchedGate?: boolean; +} +export interface NextStep { step: Step; reason: string } + +/** + * The transition table (docs/review-policy.md#transitions). Pass 1 is full. A fixed + * blocking finding earns a second full pass on the range; any other fix earns one + * fix-check; two full passes is the cap; a blocker still open after any step + * stops for the owner. + */ +export function nextStep(h: History): 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 || fixed ? { step: 'stop', reason: 'the fix-check found a blocking finding: name it for the owner, never a third full pass' } : { step: 'done', reason: 'the fix-check found nothing blocking' }; + if (h.blockingOpen && h.fullPasses >= FULL_PASS_CAP) return { step: 'stop', reason: `a blocking finding is open after ${FULL_PASS_CAP} full passes: name it for the owner` }; + if (h.blockingOpen) return { step: 'stop', reason: 'a blocking finding is still open in fix scope: name it for the owner' }; + if (!fixed) return { step: 'done', reason: 'the step applied no edits' }; + if ((h.fixedBlocking || h.touchedGate) && h.fullPasses < FULL_PASS_CAP) return { step: 'full', reason: h.fixedBlocking ? 'a blocking finding was fixed: full pass on the range' : 'a fix touched a gate-class path: full pass on the range' }; + return { step: 'fix-check', reason: h.fixedBlocking ? `full-pass cap ${FULL_PASS_CAP} reached: one fix-check reads the fix` : 'only below-floor fixes: one fix-check reads the touched hunks' }; +} + +// --------------------------------------------------------------------------- +// the dispatch plan of one step +// --------------------------------------------------------------------------- +export interface PlanInput extends ClassInput { + diffLines: number; + history: History; + numbers: NumbersResult; + /** Specialists force-included by prompt flags; 'all' for --all-specialists. */ + force?: readonly string[] | 'all'; + /** Specialists the hit-rate stats tag GATE_CANDIDATE. */ + gated?: readonly string[]; + /** A specialist produced a CRITICAL finding (Red Team activation on code class). */ + criticalFound?: boolean; + /** Step 11's native adversarial pass already ran in this invocation. */ + adversarialRan?: boolean; + /** The numeric-claim fingerprint the outside voice last reviewed; a change reruns it. */ + reviewedNumbers?: string; +} +export interface Dispatch { role: Role; name: string; effort: Effort } +export interface ReviewPlan { class: DiffClass; reasons: string[]; next: NextStep; numbers: NumbersResult; dispatches: Dispatch[]; skipped: string[] } + +/** The specialists a full pass selects for a class: class sets the eligible set, hit-rate gating applies inside it, gate class ignores gating. */ +export function specialistsFor(cls: DiffClass, i: PlanInput, skipped: string[] = []): Specialist[] { + const s = i.scope ?? {}; + const forced = (n: string) => i.force === 'all' || (Array.isArray(i.force) && i.force.includes(n)); + if (cls === 'docs') { skipped.push('specialists (docs class)'); return SPECIALISTS.filter(forced); } + if (cls === 'tests') { skipped.push('specialists except testing (tests class)'); return SPECIALISTS.filter(n => n === 'testing' || forced(n)); } + const want = (n: Specialist): boolean => { + if (forced(n)) return true; + if (n === 'state-machines') return s.SCOPE_STATE_MACHINE === 'true' || s.SCOPE_STATE_MACHINE === 'unknown' || s.SCOPE_STATE_MACHINE === undefined; + if (i.diffLines < SMALL_DIFF_LINES) return false; + switch (n) { + case 'testing': case 'maintainability': return true; + case 'security': return s.SCOPE_AUTH === 'true' || (s.SCOPE_BACKEND === 'true' && i.diffLines > LARGE_DIFF_LINES); + case 'performance': return s.SCOPE_BACKEND === 'true' || s.SCOPE_FRONTEND === 'true'; + case 'data-migration': return s.SCOPE_MIGRATIONS === 'true'; + case 'api-contract': return s.SCOPE_API === 'true'; + case 'design': return s.SCOPE_FRONTEND === 'true'; + case 'simplification': return i.diffLines > LARGE_DIFF_LINES; + } + }; + return SPECIALISTS.filter(n => { + if (!want(n)) return false; + if (cls !== 'gate' && !forced(n) && (i.gated ?? []).includes(n)) { skipped.push(`${n} (gated by hit rate)`); return false; } + return true; + }); +} + +/** The reviewers the next step dispatches, with each role's effort tier. */ +export function planReview(i: PlanInput): ReviewPlan { + const { class: cls, reasons } = classifyDiff(i); + const next = nextStep(i.history); + const skipped: string[] = []; + const dispatches: Dispatch[] = []; + const numbersChanged = i.numbers.publishes && i.reviewedNumbers !== undefined && i.reviewedNumbers !== i.numbers.fingerprint; + if (next.step === 'full') { + for (const n of specialistsFor(cls, i, skipped)) dispatches.push({ role: 'specialist', name: n, effort: 'full' }); + const redTeam = cls === 'gate' || (cls === 'code' && (i.diffLines > RED_TEAM_LINES || !!i.criticalFound)); + if (redTeam) dispatches.push({ role: 'red-team', name: 'red-team', effort: 'full' }); + else skipped.push(`red-team (${cls} class${cls === 'code' ? `, ${i.diffLines} lines, no critical` : ''})`); + if (i.history.fullPasses === 0 || (!i.adversarialRan && (cls === 'code' || cls === 'gate'))) { + if (cls === 'docs' || cls === 'tests') skipped.push(`adversarial-native (${cls} class)`); + else dispatches.push({ role: 'adversarial-native', name: 'adversarial', effort: 'full' }); + if (cls === 'code' || cls === 'gate' || i.numbers.publishes) dispatches.push({ role: 'outside-voice', name: 'outside-voice', effort: 'host' }); + else skipped.push(`outside-voice (${cls} class, no numeric claims)`); + } else { + skipped.push('adversarial-native and outside-voice (run once per invocation)'); + if (numbersChanged) dispatches.push({ role: 'outside-voice', name: 'outside-voice (numbers changed)', effort: 'host' }); + } + } else if (next.step === 'fix-check') { + dispatches.push({ role: 'fix-check', name: 'fix-check', effort: 'full' }); + if (numbersChanged) dispatches.push({ role: 'outside-voice', name: 'outside-voice (numbers changed)', effort: 'host' }); + } + return { class: cls, reasons, next, numbers: i.numbers, dispatches, skipped }; +} + +export function reviewPlanLine(p: ReviewPlan, pass: number): string { + return `REVIEW_PLAN: class=${p.class} pass=${pass} next=${p.next.step} dispatches=${p.dispatches.length} numbers=${p.numbers.publishes ? `yes(${p.numbers.claims})` : 'no'} fingerprint=${p.numbers.fingerprint}`; +} diff --git a/lib/review-evidence.ts b/lib/review-evidence.ts index 418d08ab5..777971f50 100644 --- a/lib/review-evidence.ts +++ b/lib/review-evidence.ts @@ -5,7 +5,7 @@ import { join } from 'node:path'; import { deriveReviewStatus, readJsonl } from './headless-artifacts'; import { resultLine, type ResultCodeName } from './result-codes'; -const DIFF_REVIEWS = new Set(['review', 'adversarial-review', 'codex-review', 'design-review-lite', 'ship']); +const DIFF_REVIEWS = new Set(['review', 'review-fix-check', 'adversarial-review', 'codex-review', 'design-review-lite', 'ship']); const SHARED_LIBS_COVERAGE_VERSION = 1; const HASH = /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/; diff --git a/lib/review-scope.ts b/lib/review-scope.ts index f6e77d1fc..8dc6709aa 100644 --- a/lib/review-scope.ts +++ b/lib/review-scope.ts @@ -234,7 +234,15 @@ export function renderDeferredSection(deferred: readonly DeferredRecord[], noted // --------------------------------------------------------------------------- // loop summary: what the ship receipt carries for the factory scorecard // --------------------------------------------------------------------------- -export interface ReviewLoopSummary { passes: number; blocking_found: number; deferred: number; noted: number; fixed: number; scope: ReviewScopePolicy | 'unknown' } +export interface ReviewLoopSummary { + passes: number; blocking_found: number; deferred: number; noted: number; fixed: number; scope: ReviewScopePolicy | 'unknown'; + /** One-reviewer re-checks of a fix (docs/review-policy.md#transitions); never passes. */ + fix_checks?: number; + /** Reviewer dispatches of this invocation by role (specialist, red-team, adversarial-native, outside-voice, fix-check). */ + dispatches?: Record; + /** Prompt plus diff bytes the dispatches read, / 4: a relative estimate, never a bill. */ + est_input_tokens?: number; +} /** Review-log rows of one /ship invocation (skill review, via ship, at or after `since`) → the loop summary. */ /** @@ -259,11 +267,27 @@ function rowTime(row: Record): number { * row without a start token would read as UNVERIFIED and displace the pass). */ export const LATE_DEFER_SKILL = 'review-late-defer'; +/** A fix-check (one reviewer reading the touched hunks and the fix list) logs under its own skill, so it is never a pass. */ +export const FIX_CHECK_SKILL = 'review-fix-check'; +const DISPATCH_SKILLS = ['review', LATE_DEFER_SKILL, FIX_CHECK_SKILL, 'adversarial-review']; + +/** Dispatch records of one row: its `dispatches` array, else the specialists and Red Team its stats say ran (older rows). */ +function rowDispatches(row: Record): Array<{ role: string; bytes: number | null }> { + if (Array.isArray(row.dispatches)) { + return row.dispatches.filter((d: any) => d && typeof d.role === 'string').map((d: any) => { + const b = [d.prompt_bytes, d.diff_bytes].filter((x: unknown) => typeof x === 'number' && Number.isFinite(x) && x >= 0) as number[]; + return { role: String(d.role).slice(0, 40), bytes: b.length ? b.reduce((a, c) => a + c, 0) : null }; + }); + } + if (row.skill !== 'review' || !row.specialists || typeof row.specialists !== 'object') return []; + return Object.entries(row.specialists as Record).filter(([, s]) => s && s.dispatched === true) + .map(([name]) => ({ role: name === 'red-team' ? 'red-team' : 'specialist', bytes: null })); +} export function summarizeReviewLoop(rows: readonly Record[], since: string): ReviewLoopSummary { const sinceMs = Date.parse(since); - const loopRows = rows.filter(r => r && (r.skill === 'review' || r.skill === LATE_DEFER_SKILL) && r.via === 'ship' && Number.isFinite(rowTime(r)) - && (!Number.isFinite(sinceMs) || rowTime(r) >= sinceMs)); + const inRun = (r: Record) => r && r.via === 'ship' && Number.isFinite(rowTime(r)) && (!Number.isFinite(sinceMs) || rowTime(r) >= sinceMs); + const loopRows = rows.filter(r => inRun(r) && (r.skill === 'review' || r.skill === LATE_DEFER_SKILL || r.skill === FIX_CHECK_SKILL)); const passes = loopRows.filter(r => r.skill === 'review'); const blocking = new Set(); const deferred = new Set(); @@ -281,9 +305,25 @@ export function summarizeReviewLoop(rows: readonly Record[], since: if (f.action === 'fixed' || f.action === 'auto-fixed') fixed.add(id); } } - return { passes: passes.length, blocking_found: blocking.size, deferred: deferred.size, noted: noted.size, fixed: fixed.size, scope }; + const out: ReviewLoopSummary = { passes: passes.length, blocking_found: blocking.size, deferred: deferred.size, noted: noted.size, fixed: fixed.size, scope }; + const fixChecks = loopRows.filter(r => r.skill === FIX_CHECK_SKILL).length; + if (fixChecks) out.fix_checks = fixChecks; + const byRole: Record = {}; + let bytes = 0; let measured = false; + for (const row of rows.filter(r => inRun(r) && DISPATCH_SKILLS.includes(r.skill))) { + for (const d of rowDispatches(row)) { byRole[d.role] = (byRole[d.role] ?? 0) + 1; if (d.bytes !== null) { bytes += d.bytes; measured = true; } } + } + if (Object.keys(byRole).length) out.dispatches = byRole; + if (measured) out.est_input_tokens = Math.round(bytes / 4); + return out; +} + +export function dispatchTotal(d: Record | undefined): number | undefined { + return d ? Object.values(d).reduce((a, c) => a + c, 0) : undefined; } export function reviewLoopLine(s: ReviewLoopSummary): string { - return `REVIEW_LOOP: passes=${s.passes} blocking_found=${s.blocking_found} deferred=${s.deferred} noted=${s.noted} fixed=${s.fixed} scope=${s.scope}`; + const total = dispatchTotal(s.dispatches); + return `REVIEW_LOOP: passes=${s.passes} blocking_found=${s.blocking_found} deferred=${s.deferred} noted=${s.noted} fixed=${s.fixed} scope=${s.scope}` + + (s.fix_checks ? ` fix_checks=${s.fix_checks}` : '') + (total !== undefined ? ` dispatches=${total}` : '') + (s.est_input_tokens !== undefined ? ` est_input_tokens=${s.est_input_tokens}` : ''); } diff --git a/lib/ship-receipt.ts b/lib/ship-receipt.ts index 298484cd4..3dd216d86 100644 --- a/lib/ship-receipt.ts +++ b/lib/ship-receipt.ts @@ -120,7 +120,8 @@ export function receiptSummaryLine(r: ShipReceipt): string { if (r.artifacts_consumed) parts.push(`artifacts_consumed=${r.artifacts_consumed}`); if (r.approval) parts.push(`approval=${approvalSummary(r.approval)}`); const loop = parseReviewLoop(r.review_loop); - if (loop) parts.push(`passes=${loop.passes} blocking_found=${loop.blocking_found} deferred=${loop.deferred}`); + if (loop) parts.push(`passes=${loop.passes} blocking_found=${loop.blocking_found} deferred=${loop.deferred}` + + (loop.fix_checks ? ` fix_checks=${loop.fix_checks}` : '') + (loop.dispatches ? ` dispatches=${Object.values(loop.dispatches).reduce((a, c) => a + c, 0)}` : '')); if (r.carry && typeof r.carry.from === 'string') parts.push(`carried_from=${r.carry.from.slice(0, 12)}`); return `SHIP_RECEIPT: ${oneLine(parts.join(' '))}`; } @@ -268,6 +269,13 @@ export function parseReviewLoop(value: unknown): ReviewLoopReceipt | undefined { if (n('noted') !== undefined) out.noted = n('noted'); if (n('fixed') !== undefined) out.fixed = n('fixed'); if (typeof v.scope === 'string') out.scope = v.scope.slice(0, 20); + if (n('fix_checks') !== undefined) out.fix_checks = n('fix_checks'); + if (n('est_input_tokens') !== undefined) out.est_input_tokens = n('est_input_tokens'); + if (v.dispatches && typeof v.dispatches === 'object' && !Array.isArray(v.dispatches)) { + const d = Object.fromEntries(Object.entries(v.dispatches as Record) + .filter(([k, c]) => /^[a-z][a-z0-9-]{0,39}$/.test(k) && typeof c === 'number' && Number.isInteger(c) && c >= 0).slice(0, 12)) as Record; + if (Object.keys(d).length) out.dispatches = d; + } return out; } diff --git a/review/sections/review-army.md b/review/sections/review-army.md index ea2a468c7..fa3d1de5b 100644 --- a/review/sections/review-army.md +++ b/review/sections/review-army.md @@ -101,7 +101,7 @@ Every other specialist, including Testing, reads the full diff with fixtures. "You are a specialist code reviewer. Read the checklist at {checklist path}, then run `DIFF_BASE=$(git merge-base origin/ HEAD) && git diff "$DIFF_BASE"` to get the full diff. Apply the checklist against the diff. -`NO FINDINGS` alone is the expected, complete result for a clean diff; never invent an item to show work. Otherwise output one JSON object per finding, on its own line: +`NO FINDINGS` alone is the expected, complete result for a clean diff; never invent an item. Otherwise output one JSON object per finding per line, 400 words at most: {\"severity\":\"CRITICAL|INFORMATIONAL\",\"confidence\":N,\"path\":\"file\",\"line\":N,\"category\":\"category\",\"summary\":\"description\",\"fix\":\"recommended fix\",\"fingerprint\":\"path:line:category\",\"specialist\":\"name\"} Required fields: severity, confidence, path, category, summary, specialist. diff --git a/scripts/lib/windows-curation.ts b/scripts/lib/windows-curation.ts index 7098f3c7b..cf9787d22 100644 --- a/scripts/lib/windows-curation.ts +++ b/scripts/lib/windows-curation.ts @@ -80,6 +80,7 @@ export const KNOWN_WINDOWS_INCOMPATIBLE: Array<{ file: string; reason: string }> // test/helpers/pregate-fixture.ts runBin. { file: 'test/stamp-templates.test.ts', reason: 'spawns bin/gstack-tree-receipt through test/helpers/restamp-fixture.ts runBin (shebang via CreateProcess)' }, { file: 'test/ship-issue-intent.test.ts', reason: 'spawns bin/gstack-issue-links and bin/gstack-ship-receipt through test/helpers/pregate-fixture.ts runBin (shebang via CreateProcess) with a `gh` shell shim on PATH' }, + { file: 'test/ship-receipt-carry-pregate.test.ts', reason: 'passes a gstack-pregate result to gstack-ship-receipt write and carry; on Windows the fixture\'s Linux lane is a requires-remote obligation, so write refuses the pregate as uncleared and exits 1 (windows-free-shard run 40611810409), like the other --pregate receipt tests' }, // PR E2: lane ownership and evidence verification run bin/gstack-lane-check and bin/gstack-evidence // through test/helpers/restamp-fixture.ts runBin (shebang via CreateProcess); the lib paths are // exercised in the same file, so there is no Windows-safe split to keep. diff --git a/scripts/resolvers/fix-check.ts b/scripts/resolvers/fix-check.ts new file mode 100644 index 000000000..58775b2f3 --- /dev/null +++ b/scripts/resolvers/fix-check.ts @@ -0,0 +1,25 @@ +/** + * Fix-check prompt (docs/review-policy.md#transitions): the one reviewer /ship dispatches after a + * step whose fixes do not earn a full pass. It reads the fixes, never the whole diff. The parent + * writes every input in literally: a reviewer never sees the parent's shell. + */ +export interface FixCheckInput { fixList: string; touchedHunks: string; callers: string; testResults: string } + +export const FIX_CHECK_INSTRUCTIONS = `You are the fix-check reviewer for a pre-landing review. An earlier review pass found +findings and the author fixed them. You check only those fixes: the fix list, the hunks the fixes +touched, the callers of every symbol the fixes changed, and the test results after the fixes. + +Find any defect a fix introduced or left in place: a caller the change breaks, a guard that now +fails open (accepts what it refused) or fails closed (refuses what it accepted), a regression of +behaviour the code had before the fix, an error or unreadable state now read as success, a test +made weaker. Name the input or call site that reaches each defect. + +Rate each finding against the blocking floor: CRITICAL only when it would fail a test, break +production, or lose data or secrets; everything else is INFORMATIONAL. Output one JSON object per +line, {"severity":"CRITICAL|INFORMATIONAL","path":"","line":,"summary":""}, +or the two words NO FINDINGS when the fixes are sound.`; + +export function renderFixCheckPrompt(i: FixCheckInput): string { + return [FIX_CHECK_INSTRUCTIONS, '', '## Fix list', i.fixList.trim(), '', '## Touched hunks', '```diff', i.touchedHunks.trim(), '```', '', + '## Callers of changed symbols', i.callers.trim() || '(none found)', '', '## Test results after the fixes', i.testResults.trim()].join('\n') + '\n'; +} diff --git a/scripts/resolvers/outside-voice-steps.ts b/scripts/resolvers/outside-voice-steps.ts index 93d403b6f..f95740e02 100644 --- a/scripts/resolvers/outside-voice-steps.ts +++ b/scripts/resolvers/outside-voice-steps.ts @@ -116,7 +116,7 @@ If A: revise the premise and note the revision. If B: proceed (and note that the // ─── Adversarial Review (always-on) ────────────────────────────────── function adversarialNativePass(ctx: TemplateContext, isShip: boolean): string { - return `### ${outsideVoiceFor(ctx).nativeLabel} adversarial subagent (always runs) + return `### ${outsideVoiceFor(ctx).nativeLabel} adversarial subagent (${isShip ? 'on \`DISPATCH: adversarial-native\`' : 'always runs'}) Before dispatch, run \`~/.claude/skills/gstack/bin/gstack-review-log --start adversarial-review\` and save the returned token for this native attempt. Do the same before each outside @@ -235,9 +235,9 @@ export function generateAdversarialStep(ctx: TemplateContext): string { const isShip = ctx.skillName === 'ship'; const stepNum = isShip ? '11' : '4.8'; - return `## Step ${stepNum}: Adversarial review (always-on) + return `## Step ${stepNum}: Adversarial review${isShip ? '' : ' (always-on)'} -Every diff gets the ${outsideVoiceFor(ctx).nativeLabel} adversarial pass. Add ${outsideVoiceFor(ctx).label} when its preflight is ready; unavailable or disabled outside coverage stays explicit. +${isShip ? 'Unavailable' : `Every diff gets the ${outsideVoiceFor(ctx).nativeLabel} adversarial pass. Add ${outsideVoiceFor(ctx).label} when its preflight is ready; unavailable`} or disabled outside coverage stays explicit. **Detect diff size:** @@ -252,8 +252,8 @@ echo "DIFF_SIZE: $DIFF_TOTAL" ${outsideVoicePreflight(ctx, { disabledBehavior: 'codex-only', nativeReview: true })} \`CODEX_MODE: disabled\` means skip the ${outsideVoiceFor(ctx).label} passes ONLY. -\`ready\` and \`unverified\` run them; every other mode skips them with the printed reason. -The ${outsideVoiceFor(ctx).nativeLabel} adversarial subagent always runs. +\`ready\` and \`unverified\` run them; every other mode skips them with the printed reason.${isShip ? '' : ` +The ${outsideVoiceFor(ctx).nativeLabel} adversarial subagent always runs.`} **User override:** If the user explicitly requested "full review", "structured review", or "P1 gate", also run the ${outsideVoiceFor(ctx).label} structured review regardless of diff size (still requires \`CODEX_MODE: ready\` (or \`unverified\`)). @@ -274,7 +274,7 @@ ${outsideVoiceProvenance(ctx, 'adversarial')} After all passes complete, synthesize findings across all sources: \`\`\` -ADVERSARIAL REVIEW SYNTHESIS (always-on, N lines): +ADVERSARIAL REVIEW SYNTHESIS (${isShip ? '' : 'always-on, '}N lines): ════════════════════════════════════════════════════════════ High confidence (found by multiple sources): [findings agreed on by >1 pass] Unique to the parent checklist/specialists: [from earlier steps] @@ -309,7 +309,7 @@ in order before leaving Step 11: Step 9 completes full review before fixes; any further repair inserts its checks ahead of the remaining items. These fresh reviews after code edits are not recovery retries. Returning here never resets Step 9's three-cycle fix limit. -3. **Native complete with no queued fixes:** Finish the memory updates below, +3. **Native complete with no queued fixes, or not dispatched:** Finish the memory updates below, then continue to Step 11.5. Never jump directly to release preparation.` : 'The native pass is required for Step 5.8 completion. Optional outside failures remain separately recorded, not completed by native coverage. Return all findings and structured-review decisions to Step 5; the parent owns fixes and the full rerun.'} ---`; diff --git a/scripts/resolvers/review-army.ts b/scripts/resolvers/review-army.ts index 301b2573a..1c174ac94 100644 --- a/scripts/resolvers/review-army.ts +++ b/scripts/resolvers/review-army.ts @@ -141,7 +141,7 @@ Every other specialist, including Testing, reads the full diff with fixtures. "You are a specialist code reviewer. Read the checklist at {checklist path}, then run \`${diffBase(ctx)} && git diff "$DIFF_BASE"\` to get the ${ctx.skillName === 'ship' ? 'diff (``: write the sha in before dispatch, the merge base on pass 1 and `RANGE_BASE` from pass 2 on; never a shell variable)' : 'full diff'}. Apply the checklist against the diff. -\`NO FINDINGS\` alone is the expected, complete result for a clean diff; never invent an item to show work. Otherwise output one JSON object per finding, on its own line: +\`NO FINDINGS\` alone is the expected, complete result for a clean diff; never invent an item. Otherwise output one JSON object per finding per line, 400 words at most: {\\"severity\\":\\"CRITICAL|INFORMATIONAL\\",\\"confidence\\":N,\\"path\\":\\"file\\",\\"line\\":N,\\"category\\":\\"category\\",\\"summary\\":\\"description\\",\\"fix\\":\\"recommended fix\\",\\"fingerprint\\":\\"path:line:category\\",\\"specialist\\":\\"name\\"} Required fields: severity, confidence, path, category, summary, specialist. diff --git a/scripts/resolvers/review-dashboard.ts b/scripts/resolvers/review-dashboard.ts index 89a6c1a73..4b2e58237 100644 --- a/scripts/resolvers/review-dashboard.ts +++ b/scripts/resolvers/review-dashboard.ts @@ -65,9 +65,7 @@ ${ctx.skillName === 'ship' ? 'This verdict never skips Step 9 or its finding, ap Other rows provide context, not a substitute for Eng Review: - Recommend CEO Review for product/business or scope decisions, not routine fixes or cleanup. - Recommend Design Review for UI/UX work, not backend, infrastructure or prompt-only work. -- Adversarial review always includes a native pass. Available, enabled outside - challenges supplement it; diffs of 200+ lines also get the structured P1 gate. -- Outside Voice is the default-on plan review after CEO/Eng review. \`codex_reviews\` +${ctx.skillName === 'ship' ? '- Step 9\'s review plan decides the adversarial passes (Step 11).\n' : '- Adversarial review always includes a native pass. Available, enabled outside\n challenges supplement it; diffs of 200+ lines also get the structured P1 gate.\n'}- Outside Voice is the default-on plan review after CEO/Eng review. \`codex_reviews\` disables that extra step. Provider failure uses native fallback and records missing outside coverage; this dashboard row never gates shipping. diff --git a/scripts/review-dispatch-compare.ts b/scripts/review-dispatch-compare.ts new file mode 100755 index 000000000..966e4e1e6 --- /dev/null +++ b/scripts/review-dispatch-compare.ts @@ -0,0 +1,175 @@ +#!/usr/bin/env -S bun --no-env-file --config=/dev/null +/** + * review-dispatch-compare — how many reviewers a /ship run and an /autoplan run dispatch under the + * legacy policy and under docs/review-policy.md#review-depth, on fixed fixture diffs, with an input + * plus output token estimate per run. The proof-of-saving table in the GSTA-131 PR comes from here. + * + * bun scripts/review-dispatch-compare.ts [--fixture ]... [--tokens ] [--json] + * bun scripts/review-dispatch-compare.ts --capture --name --out + * + * --capture writes a fixture from one commit (its diff against its first parent) through + * gstack-review-plan --json. --tokens takes the live per-dispatch output-token medians by role and + * effort ({ "specialist": { "full": n, "light": n }, ... }); without it the script uses the shipped + * defaults below and says so. Counts are exact for the modelled scenarios; tokens are estimates. + */ +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { runScriptSync } from '../lib/script-invocation'; +import { planReview, specialistsFor, type DiffClass, type Dispatch, type History, type NumbersResult } from '../lib/review-depth'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const BYTES_PER_TOKEN = 4; +const FIXED_AUDITS = ['coverage-audit', 'plan-completion', 'docs-audit']; +const RANGE_FRACTION = 0.15; +const FIX_CHECK_FRACTION = 0.1; +/** Output plus reasoning tokens per dispatch when no live measurement is given (labelled DEFAULT in the output). */ +const DEFAULT_OUT: Record = { + specialist: { full: 6000, light: 3000 }, 'red-team': { full: 8000, light: 4000 }, 'adversarial-native': { full: 8000, light: 4000 }, + 'outside-voice': { full: 6000, light: 6000 }, 'fix-check': { full: 4000, light: 2000 }, audit: { full: 5000, light: 2500 }, 'autoplan-voice': { full: 9000, light: 4500 }, +}; + +export interface Fixture { name: string; commit: string; paths: string[]; deleted: string[]; stamp_only?: string[]; scope: Record; gate_tagged: number | null; diff_lines: number; diff_bytes: number; numbers: NumbersResult } +/** clean: pass 1 finds nothing; nits: pass 1 fixes below-floor findings; blocker: pass 1 fixes a blocker; adversarial-fix: Step 11 finds a should that is fixed; fix-regression: a pass-1 fix introduces a blocker the next step finds. */ +type Scenario = 'clean' | 'nits' | 'blocker' | 'adversarial-fix' | 'fix-regression'; +const SCENARIOS: Scenario[] = ['clean', 'nits', 'blocker', 'adversarial-fix', 'fix-regression']; +interface Call { role: string; name?: string; effort: 'full' | 'light' | 'host'; diffBytes: number } + +const fileBytes = (rel: string) => { try { return fs.statSync(path.join(ROOT, rel)).size; } catch { return 0; } }; +function promptBytes(role: string, name: string): number { + if (role === 'specialist') return fileBytes(`review/specialists/${name}.md`) + fileBytes('review/checklist.md') / 4; + if (role === 'red-team') return fileBytes('review/specialists/red-team.md'); + if (role === 'adversarial-native' || role === 'outside-voice') return fileBytes('ship/sections/adversarial.md') / 2; + if (role === 'fix-check') return fileBytes('review/checklist.md') / 2; + return 20_000; +} + +function legacyShip(f: Fixture, sc: Scenario): Call[] { + const calls: Call[] = []; + const passes = sc === 'clean' ? 1 : sc === 'fix-regression' ? 3 : 2; + const step11Runs = sc === 'adversarial-fix' ? 2 : 1; + const legacySpecialists = specialistsFor('code', { ...base(f), history: { fullPasses: 0, last: 'none' } }); + for (let p = 1; p <= passes; p++) { + const diffBytes = p === 1 ? f.diff_bytes : f.diff_bytes * RANGE_FRACTION; + for (const name of legacySpecialists) calls.push({ role: 'specialist', name, effort: 'full', diffBytes }); + const critical = (sc === 'blocker' && p === 1) || (sc === 'fix-regression' && p === 2); + if (f.diff_lines > 200 || critical) calls.push({ role: 'red-team', effort: 'full', diffBytes }); + } + for (let s = 0; s < step11Runs; s++) calls.push({ role: 'adversarial-native', effort: 'full', diffBytes: f.diff_bytes }, { role: 'outside-voice', effort: 'host', diffBytes: f.diff_bytes }); + for (const a of FIXED_AUDITS) calls.push({ role: 'audit', effort: 'full', diffBytes: f.diff_bytes / 2 }); + return calls; +} + +function base(f: Fixture) { + return { paths: f.paths, deleted: f.deleted, stampOnly: f.stamp_only ?? [], scope: f.scope, gateTagged: f.gate_tagged, diffLines: f.diff_lines, numbers: f.numbers }; +} + +/** The new policy, step by step through lib/review-depth's own transition table. */ +function newShip(f: Fixture, sc: Scenario): { calls: Call[]; ending: string } { + const calls: Call[] = []; + const outcomes: Record>> = { + clean: [{}], + nits: [{ fixedBelow: true }, {}], + blocker: [{ fixedBlocking: true }, {}], + 'adversarial-fix': [{ fixedBelow: true }, {}], + 'fix-regression': [{ fixedBelow: true }, { blockingOpen: true }], + }; + let h: History = { fullPasses: 0, last: 'none' }; + let ending = 'done'; + let adversarialRan = false; + for (let i = 0; i < 6; i++) { + const plan = planReview({ ...base(f), history: h, criticalFound: sc === 'blocker' && i === 0, adversarialRan }); + if (plan.next.step === 'done' || plan.next.step === 'stop') { ending = plan.next.step; break; } + 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'); + const o = outcomes[sc][Math.min(i, outcomes[sc].length - 1)] ?? {}; + h = { fullPasses: h.fullPasses + (plan.next.step === 'full' ? 1 : 0), last: plan.next.step, ...o }; + } + for (const a of FIXED_AUDITS) calls.push({ role: 'audit', effort: 'light', diffBytes: f.diff_bytes / 2 }); + return { calls, ending }; +} + +function tokens(calls: Call[], out: typeof DEFAULT_OUT): number { + return Math.round(calls.reduce((n, c) => { + const o = out[c.role] ?? out.audit!; + return n + (promptBytes(c.role, c.name ?? '') + c.diffBytes) / BYTES_PER_TOKEN + (c.effort === 'light' ? o.light : o.full); + }, 0)); +} + +/** /autoplan: phases x voices x rounds. Legacy reopens every touched phase on any accepted change, up to 3 rounds. */ +function autoplanRuns(docsOnly: boolean): Array<{ scenario: string; legacy: number; next: number }> { + const phases = ['ceo', 'eng']; + const legacyRounds = { 'no change': 1, 'medium-only change': 2, 'high change': 2, 'high twice': 3 } as Record; + const newRounds = { 'no change': 1, 'medium-only change': 1, 'high change': 2, 'high twice': 2 } as Record; + return Object.keys(legacyRounds).map(s => ({ + scenario: s, + legacy: phases.length * 2 * legacyRounds[s]!, + next: (docsOnly ? 3 : phases.length * 2) * newRounds[s]!, + })); +} + +function capture(sha: string, name: string, out: string): number { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'review-compare-')); + const captureBase = `review-compare-base-${process.pid}-${sha.slice(0, 8)}`; + const git = (args: string[], cwd = ROOT) => spawnSync('git', args, { cwd, encoding: 'utf8', timeout: 120_000, maxBuffer: 64 * 1024 * 1024 }); + try { + if (git(['worktree', 'add', '-q', '--detach', dir, sha]).status !== 0) throw new Error(`cannot check out ${sha}`); + git(['branch', '-f', captureBase, `${sha}^`], dir); + const r = runScriptSync(path.join(ROOT, 'bin', 'gstack-review-plan'), ['--base', captureBase, '--json'], { cwd: dir, encoding: 'utf8', timeout: 300_000, maxBuffer: 64 * 1024 * 1024 }); + if (r.status !== 0) throw new Error(`gstack-review-plan failed: ${r.stderr}`); + const plan = JSON.parse(r.stdout); + const diffBytes = Buffer.byteLength(git(['diff', `${sha}^`, sha], dir).stdout ?? ''); + const fixture: Fixture = { name, commit: sha, paths: plan.paths, deleted: plan.deleted, stamp_only: plan.stamp_only, scope: plan.scope, gate_tagged: plan.gate_tagged, diff_lines: plan.diff_lines, diff_bytes: diffBytes, numbers: plan.numbers }; + fs.mkdirSync(path.dirname(out), { recursive: true }); + fs.writeFileSync(out, JSON.stringify(fixture, null, 2) + '\n'); + process.stdout.write(`CAPTURED: ${name} ${sha} class=${plan.class} paths=${plan.paths.length} lines=${plan.diff_lines} -> ${out}\n`); + return 0; + } finally { + git(['worktree', 'remove', '--force', dir]); + git(['branch', '-D', captureBase]); + } +} + +export function compare(fixtures: Fixture[], out: typeof DEFAULT_OUT) { + const rows: Array> = []; + for (const f of fixtures) { + const cls: DiffClass = planReview({ ...base(f), history: { fullPasses: 0, last: 'none' } }).class; + for (const sc of SCENARIOS) { + const legacy = legacyShip(f, sc); + const next = newShip(f, sc); + const lt = tokens(legacy, out); const nt = tokens(next.calls, out); + rows.push({ fixture: f.name, class: cls, scenario: sc, legacy_dispatches: legacy.length, new_dispatches: next.calls.length, legacy_tokens: lt, new_tokens: nt, ratio: Math.round((nt / lt) * 100) / 100, ending: next.ending }); + } + } + return rows; +} + +function main(argv: string[]): number { + const flag = (n: string) => { const i = argv.indexOf(n); return i >= 0 ? argv[i + 1] : undefined; }; + const cap = flag('--capture'); + if (cap) return capture(cap, flag('--name') ?? cap, flag('--out') ?? path.join(ROOT, 'test/fixtures/review-depth', `${flag('--name') ?? cap}.json`)); + const files = argv.flatMap((a, i) => (a === '--fixture' ? [argv[i + 1]!] : [])); + const fixtureFiles = files.length ? files : fs.readdirSync(path.join(ROOT, 'test/fixtures/review-depth')).filter(f => f.endsWith('.json')).map(f => path.join(ROOT, 'test/fixtures/review-depth', f)); + const fixtures = fixtureFiles.map(f => JSON.parse(fs.readFileSync(f, 'utf8')) as Fixture); + const tokFile = flag('--tokens'); + const out = tokFile ? { ...DEFAULT_OUT, ...JSON.parse(fs.readFileSync(tokFile, 'utf8')) } : DEFAULT_OUT; + const rows = compare(fixtures, out); + const ap = fixtures.map(f => ({ fixture: f.name, runs: autoplanRuns(planReview({ ...base(f), history: { fullPasses: 0, last: 'none' } }).class === 'docs') })); + if (argv.includes('--json')) { process.stdout.write(JSON.stringify({ tokens_source: tokFile ?? 'DEFAULT', ship: rows, autoplan: ap }) + '\n'); return 0; } + const lines = [`TOKENS: ${tokFile ? `measured (${tokFile})` : 'DEFAULT estimates (pass --tokens for measured output)'}`, '', '| fixture | class | scenario | /ship dispatches legacy → new | est. tokens legacy → new | ratio | ending |', '|---|---|---|---|---|---|---|']; + for (const r of rows) lines.push(`| ${r.fixture} | ${r.class} | ${r.scenario} | ${r.legacy_dispatches} → ${r.new_dispatches} | ${r.legacy_tokens} → ${r.new_tokens} | ${r.ratio} | ${r.ending} |`); + lines.push('', '| fixture | /autoplan scenario | voice dispatches legacy → new |', '|---|---|---|'); + for (const a of ap) for (const r of a.runs) lines.push(`| ${a.fixture} | ${r.scenario} | ${r.legacy} → ${r.next} |`); + const median = (v: number[]) => { const m = Math.floor(v.length / 2); return v.length % 2 ? v[m]! : Math.round(((v[m - 1]! + v[m]!) / 2) * 100) / 100; }; + const ratios = rows.map(r => Number(r.ratio)).sort((a, b) => a - b); + lines.push('', `SHIP_TOKEN_RATIO: median=${median(ratios)} min=${ratios[0]} max=${ratios[ratios.length - 1]} (n=${ratios.length})`); + const byClass = new Map(); + for (const r of rows) byClass.set(String(r.class), [...(byClass.get(String(r.class)) ?? []), Number(r.ratio)].sort((x, y) => x - y)); + lines.push(`SHIP_TOKEN_RATIO_BY_CLASS: ${[...byClass].map(([c, v]) => `${c} median=${median(v)} (n=${v.length})`).join('; ')}`); + process.stdout.write(lines.join('\n') + '\n'); + return 0; +} + +if (import.meta.main) process.exit(main(process.argv.slice(2))); diff --git a/ship/SKILL.md b/ship/SKILL.md index 04062758f..881cd3934 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -205,9 +205,9 @@ not a commit ID. Use `git diff ` to compare these snapshots **Review policy** (docs/review-policy.md): a finding blocks another review pass only when it would fail a test, break production, or lose data or secrets; everything below that floor ships as a named note in the PR body. Pass 1 reviews -the diff; later passes read only the range since the previous pass; three passes -is the cap. `NO FINDINGS` is a complete reviewer result. The ship receipt's -`review_loop` counters record the outcome; `ship: review_scope` (Step 3) is the knob. +the diff; a later pass reads only the range; two full passes plus one fix-check +is the cap. `NO FINDINGS` is a complete reviewer result. `ship: review_scope` (Step 3) +is the knob; the receipt's `review_loop` records the outcome. ### Ship control flow @@ -328,12 +328,11 @@ repository-landing asks, including on Apple repos. `git log origin/..HEAD --oneline`. 4. **Carry check.** If `gh pr list --head --state open --json number` - names a PR, run `~/.claude/skills/gstack/bin/gstack-ship-receipt carry --pr --base `. - `CARRY: carried` means the product diff against `origin/` has the same - patch-id as the reviewed head's (only a base merge, tests or release metadata - changed; the receipt's `carry` object holds the range-diff evidence): the receipt - now binds HEAD and the run is complete at its `GSTACK_RESULT` line, so stop. - `CARRY: refused` or `current` continues; a product change always re-runs Steps 2–16. + names a PR, run `~/.claude/skills/gstack/bin/gstack-ship-receipt carry --pr --base --dry-run`. + `CARRY: would carry` (base merge committed, same product patch-id): skip review, fix + what it broke (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry + without `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD and + ends the run. A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. 5. Display historical readiness using the dashboard below, then finish Step 1. Prior CLEAR reviews or dashboard skips never replace Step 9's gates. @@ -397,8 +396,7 @@ This verdict never skips Step 9 or its finding, approval and convergence gates. Other rows provide context, not a substitute for Eng Review: - Recommend CEO Review for product/business or scope decisions, not routine fixes or cleanup. - Recommend Design Review for UI/UX work, not backend, infrastructure or prompt-only work. -- Adversarial review always includes a native pass. Available, enabled outside - challenges supplement it; diffs of 200+ lines also get the structured P1 gate. +- Step 9's review plan decides the adversarial passes (Step 11). - Outside Voice is the default-on plan review after CEO/Eng review. `codex_reviews` disables that extra step. Provider failure uses native fallback and records missing outside coverage; this dashboard row never gates shipping. @@ -506,12 +504,14 @@ repeat Step 2 (decisions included) on the merged content, then Step 4. ## Step 11.5: Bind the reviews 1. **Select the two reviews.** Run `~/.claude/skills/gstack/bin/gstack-review-read`. - Select this invocation's final Step 9.4 record (`skill:"review"`, `via:"ship"`) - and Step 11 native record (`skill:"adversarial-review"`). Match each to its saved + Select this invocation's final Step 9 record (`skill:"review"`, `via:"ship"`, or a + last 3.5 `review-fix-check`) and Step 11 native record (`skill:"adversarial-review"`; + none after `ADVERSARIAL: skipped`). Match each to its saved handle, original token and source; reject outside-provider or older records. -2. **Compare their content.** Require the native record's `review_binding.state` - to be `verified`. All three snapshots must match: its `wtree`, Step 9.4's - `review_binding.start_wtree` and `review_binding.end_wtree`. A mismatch or missing +2. **Compare their content.** All three snapshots must match: the native record's + `wtree` (its `review_binding.state` `verified`), the Step 9 record's + `review_binding.start_wtree` and `review_binding.end_wtree`; after a planned skip, the + last two with `review_freshness` `CURRENT`. A mismatch or a missing required record/field blocks release preparation: report **Review records missing or mismatched** and insert `9 → 10 → 11 → 11.5` before Step 12. Bind the new records at 11.5. Never attach new tokens to old work. @@ -751,7 +751,7 @@ command, result/counts, timestamp and log, never resample. Mandatory reviews sti `--allow-paths` exempts only release metadata. A `package.json` version-only edit can qualify; scripts, dependencies and runtime configuration require live tests. Uncertain edits cannot be exempted. Docs, TODO edits, new/generated tests and fixes -make evidence STALE even without a new code review. Use this example only once +make evidence STALE even without a new code review; a Step 14 TODOS.md edit reruns them. Use this example only once every allowed edit is release metadata: ```bash diff --git a/ship/SKILL.md.tmpl b/ship/SKILL.md.tmpl index f943e38ad..16f65656e 100644 --- a/ship/SKILL.md.tmpl +++ b/ship/SKILL.md.tmpl @@ -71,9 +71,9 @@ not a commit ID. Use `git diff ` to compare these snapshots **Review policy** (docs/review-policy.md): a finding blocks another review pass only when it would fail a test, break production, or lose data or secrets; everything below that floor ships as a named note in the PR body. Pass 1 reviews -the diff; later passes read only the range since the previous pass; three passes -is the cap. `NO FINDINGS` is a complete reviewer result. The ship receipt's -`review_loop` counters record the outcome; `ship: review_scope` (Step 3) is the knob. +the diff; a later pass reads only the range; two full passes plus one fix-check +is the cap. `NO FINDINGS` is a complete reviewer result. `ship: review_scope` (Step 3) +is the knob; the receipt's `review_loop` records the outcome. ### Ship control flow @@ -135,12 +135,11 @@ repository-landing asks, including on Apple repos. `git log origin/..HEAD --oneline`. 4. **Carry check.** If `gh pr list --head --state open --json number` - names a PR, run `~/.claude/skills/gstack/bin/gstack-ship-receipt carry --pr --base `. - `CARRY: carried` means the product diff against `origin/` has the same - patch-id as the reviewed head's (only a base merge, tests or release metadata - changed; the receipt's `carry` object holds the range-diff evidence): the receipt - now binds HEAD and the run is complete at its `GSTACK_RESULT` line, so stop. - `CARRY: refused` or `current` continues; a product change always re-runs Steps 2–16. + names a PR, run `~/.claude/skills/gstack/bin/gstack-ship-receipt carry --pr --base --dry-run`. + `CARRY: would carry` (base merge committed, same product patch-id): skip review, fix + what it broke (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry + without `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD and + ends the run. A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. 5. Display historical readiness using the dashboard below, then finish Step 1. Prior CLEAR reviews or dashboard skips never replace Step 9's gates. @@ -229,12 +228,14 @@ repeat Step 2 (decisions included) on the merged content, then Step 4. ## Step 11.5: Bind the reviews 1. **Select the two reviews.** Run `~/.claude/skills/gstack/bin/gstack-review-read`. - Select this invocation's final Step 9.4 record (`skill:"review"`, `via:"ship"`) - and Step 11 native record (`skill:"adversarial-review"`). Match each to its saved + Select this invocation's final Step 9 record (`skill:"review"`, `via:"ship"`, or a + last 3.5 `review-fix-check`) and Step 11 native record (`skill:"adversarial-review"`; + none after `ADVERSARIAL: skipped`). Match each to its saved handle, original token and source; reject outside-provider or older records. -2. **Compare their content.** Require the native record's `review_binding.state` - to be `verified`. All three snapshots must match: its `wtree`, Step 9.4's - `review_binding.start_wtree` and `review_binding.end_wtree`. A mismatch or missing +2. **Compare their content.** All three snapshots must match: the native record's + `wtree` (its `review_binding.state` `verified`), the Step 9 record's + `review_binding.start_wtree` and `review_binding.end_wtree`; after a planned skip, the + last two with `review_freshness` `CURRENT`. A mismatch or a missing required record/field blocks release preparation: report **Review records missing or mismatched** and insert `9 → 10 → 11 → 11.5` before Step 12. Bind the new records at 11.5. Never attach new tokens to old work. @@ -472,7 +473,7 @@ command, result/counts, timestamp and log, never resample. Mandatory reviews sti `--allow-paths` exempts only release metadata. A `package.json` version-only edit can qualify; scripts, dependencies and runtime configuration require live tests. Uncertain edits cannot be exempted. Docs, TODO edits, new/generated tests and fixes -make evidence STALE even without a new code review. Use this example only once +make evidence STALE even without a new code review; a Step 14 TODOS.md edit reruns them. Use this example only once every allowed edit is release metadata: ```bash diff --git a/ship/sections/adversarial.md b/ship/sections/adversarial.md index 45e860108..3436b5c24 100644 --- a/ship/sections/adversarial.md +++ b/ship/sections/adversarial.md @@ -1,8 +1,13 @@ -## Step 11: Adversarial review (always-on) +**Step 9's last full-pass plan governs this step:** run the native pass only on +`DISPATCH: adversarial-native` and the outside voice only on +`DISPATCH: outside-voice`. Without the native line, print `ADVERSARIAL: skipped (review plan)`; +with neither line, take decision 3 of Finish the adversarial phase. -Every diff gets the Claude adversarial pass. Add Codex when its preflight is ready; unavailable or disabled outside coverage stays explicit. +## Step 11: Adversarial review + +Unavailable or disabled outside coverage stays explicit. **Detect diff size:** @@ -73,13 +78,12 @@ Branch on the echoed `CODEX_MODE`: `CODEX_MODE: disabled` means skip the Codex passes ONLY. `ready` and `unverified` run them; every other mode skips them with the printed reason. -The Claude adversarial subagent always runs. **User override:** If the user explicitly requested "full review", "structured review", or "P1 gate", also run the Codex structured review regardless of diff size (still requires `CODEX_MODE: ready` (or `unverified`)). --- -### Claude adversarial subagent (always runs) +### Claude adversarial subagent (on `DISPATCH: adversarial-native`) Before dispatch, run `~/.claude/skills/gstack/bin/gstack-review-log --start adversarial-review` and save the returned token for this native attempt. Do the same before each outside @@ -277,7 +281,7 @@ Retain the historical review-log skill ID; add `"host":"claude","outside_provide After all passes complete, synthesize findings across all sources: ``` -ADVERSARIAL REVIEW SYNTHESIS (always-on, N lines): +ADVERSARIAL REVIEW SYNTHESIS (N lines): ════════════════════════════════════════════════════════════ High confidence (found by multiple sources): [findings agreed on by >1 pass] Unique to the parent checklist/specialists: [from earlier steps] @@ -312,7 +316,7 @@ in order before leaving Step 11: Step 9 completes full review before fixes; any further repair inserts its checks ahead of the remaining items. These fresh reviews after code edits are not recovery retries. Returning here never resets Step 9's three-cycle fix limit. -3. **Native complete with no queued fixes:** Finish the memory updates below, +3. **Native complete with no queued fixes, or not dispatched:** Finish the memory updates below, then continue to Step 11.5. Never jump directly to release preparation. --- diff --git a/ship/sections/adversarial.md.tmpl b/ship/sections/adversarial.md.tmpl index fe27bb106..abb9615ae 100644 --- a/ship/sections/adversarial.md.tmpl +++ b/ship/sections/adversarial.md.tmpl @@ -1,3 +1,8 @@ +**Step 9's last full-pass plan governs this step:** run the native pass only on +`DISPATCH: adversarial-native` and the outside voice only on +`DISPATCH: outside-voice`. Without the native line, print `ADVERSARIAL: skipped (review plan)`; +with neither line, take decision 3 of Finish the adversarial phase. + {{ADVERSARIAL_STEP}} {{LEARNINGS_LOG}} diff --git a/ship/sections/review-army.md b/ship/sections/review-army.md index b9c3f17ef..32f18e440 100644 --- a/ship/sections/review-army.md +++ b/ship/sections/review-army.md @@ -13,6 +13,15 @@ Steps 10–11 queue findings without editing; include those findings in this pas Every repeat starts before the checklist read and captures a fresh REVIEW_START. Finish the complete review and QA before applying any fix in Step 9.4. +**Review plan.** Pass 1 runs `~/.claude/skills/gstack/bin/gstack-review-plan +--base `; a repeat uses the plan run that picked it. Save its `REVIEW_PLAN:` line and +fingerprint. Dispatch exactly its `DISPATCH:` lines (specialists, Red Team, Step +11's voices), each at its `effort=` on the same model family, in place of 9.1's and +9.2's own selection rules; a `SKIP:` reviewer +logs `{"dispatched":false,"reason":"scope"}`. Audits run at `AUDIT_EFFORT:`. The +core checklist, gate scan and QA always run. Item 6's row adds +`"dispatches":[{"role":R,"prompt_bytes":N,"diff_bytes":N}]`. + **Pass 2+ reads the range, not the tree** (`review_scope frozen`): the core read in item 2 becomes `git diff $RANGE_BASE`, and every dispatched prompt (specialists, Red Team, the Step 11 reviewers) carries a literal sha you write in: @@ -384,7 +393,7 @@ Every other specialist, including Testing, reads the full diff with fixtures. "You are a specialist code reviewer. Read the checklist at {checklist path}, then run `DIFF_BASE= && git diff "$DIFF_BASE"` to get the diff (``: write the sha in before dispatch, the merge base on pass 1 and `RANGE_BASE` from pass 2 on; never a shell variable). Apply the checklist against the diff. -`NO FINDINGS` alone is the expected, complete result for a clean diff; never invent an item to show work. Otherwise output one JSON object per finding, on its own line: +`NO FINDINGS` alone is the expected, complete result for a clean diff; never invent an item. Otherwise output one JSON object per finding per line, 400 words at most: {\"severity\":\"CRITICAL|INFORMATIONAL\",\"confidence\":N,\"path\":\"file\",\"line\":N,\"category\":\"category\",\"summary\":\"description\",\"fix\":\"recommended fix\",\"fingerprint\":\"path:line:category\",\"specialist\":\"name\"} Required fields: severity, confidence, path, category, summary, specialist. @@ -790,25 +799,33 @@ finding makes that PR a draft. ### Decide whether to repeat Step 9 After persistence, record missing output, CYCLES and fixes in the invocation record. -Apply these decisions in order: +Decision 1 always applies first. Then, under `frozen`, run `gstack-review-plan --base --pass $PASS --last full` +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 `); +its `NEXT:` alone picks the next decision: `stop` 2, `full` 3, `fix-check` 3.5, `done` 4. +Other scopes apply decisions 2–4 in order. 1. **Dispatched reviewer output missing:** STOP and name each failed specialist or Red Team. Retain queued fixes and restore coverage. If this pass made edits, resume at the next decision; otherwise run a fresh complete Step 9. A successful peer or a QA exception cannot replace missing dispatched coverage. 2. **Third fixing cycle reached (`CYCLES >= 3`):** STOP and report recurring findings with - `converged:false`; do not run a fourth fixing cycle. Under `frozen` the cap comes - sooner: a blocking finding still in `fix` scope on pass 3 (`PASS >= 3`) stops - here too, unfixed. Name each blocking finding (path, severity, problem) for the - owner. Deferred and noted items are never recurring findings, so a capped pass - whose remaining findings are all below the floor applies no edits and continues - at decision 4: the loop ships below the floor. The cap is a stop, not an - approval: a blocker it leaves standing is reported, never waved through. + `converged:false`; do not run a fourth fixing cycle. Name each blocking finding (path, + severity, problem) for the owner, unfixed. The cap is a stop, not an approval. A capped + pass whose remaining findings are all deferred or noted applied no edits: decision 4. 3. **Fixes applied below the cap:** Insert Step 5, affected Steps 6–8 and all of Step 9 before the pending Step 10 in the work list. If the fixes grew the diff past what the request needs (a helper, a guard, a case nothing takes), cut that growth before anything else; the repeat reviews the cut as part of the range. Tests must pass or retain approval for the same verified pre-existing failures and scope. Keep CYCLES and scoped approvals across this repeat; PASS increments on entry. +3.5. **One fix-check.** Take a `gstack-review-log --start review-fix-check` token; write the + fix list, touched hunks (`git diff $PASS_HEAD`), callers of changed symbols and + test results to `.gstack/tmp/fix-check-*.txt`; dispatch one full-effort reviewer + on `gstack-review-plan --fix-check-prompt --fix-list --hunks --tests + --callers `, plus the outside voice only if the plan lists it. Log item 6's + row as `"skill":"review-fix-check"` with `--finish `; not a pass. A + CRITICAL goes to decision 2; otherwise decision 4. 4. **No edits in this pass:** Resolve the required-probe gate below. Only after it clears may you continue to Step 10. A pass whose only findings were deferred or noted applied no edits and continues here: the loop converges below the floor. diff --git a/ship/sections/review-army.md.tmpl b/ship/sections/review-army.md.tmpl index 1daa1035c..6317b79c6 100644 --- a/ship/sections/review-army.md.tmpl +++ b/ship/sections/review-army.md.tmpl @@ -11,6 +11,15 @@ Steps 10–11 queue findings without editing; include those findings in this pas Every repeat starts before the checklist read and captures a fresh REVIEW_START. Finish the complete review and QA before applying any fix in Step 9.4. +**Review plan.** Pass 1 runs `~/.claude/skills/gstack/bin/gstack-review-plan +--base `; a repeat uses the plan run that picked it. Save its `REVIEW_PLAN:` line and +fingerprint. Dispatch exactly its `DISPATCH:` lines (specialists, Red Team, Step +11's voices), each at its `effort=` on the same model family, in place of 9.1's and +9.2's own selection rules; a `SKIP:` reviewer +logs `{"dispatched":false,"reason":"scope"}`. Audits run at `AUDIT_EFFORT:`. The +core checklist, gate scan and QA always run. Item 6's row adds +`"dispatches":[{"role":R,"prompt_bytes":N,"diff_bytes":N}]`. + **Pass 2+ reads the range, not the tree** (`review_scope frozen`): the core read in item 2 becomes `git diff $RANGE_BASE`, and every dispatched prompt (specialists, Red Team, the Step 11 reviewers) carries a literal sha you write in: @@ -204,25 +213,33 @@ finding makes that PR a draft. ### Decide whether to repeat Step 9 After persistence, record missing output, CYCLES and fixes in the invocation record. -Apply these decisions in order: +Decision 1 always applies first. Then, under `frozen`, run `gstack-review-plan --base --pass $PASS --last full` +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 `); +its `NEXT:` alone picks the next decision: `stop` 2, `full` 3, `fix-check` 3.5, `done` 4. +Other scopes apply decisions 2–4 in order. 1. **Dispatched reviewer output missing:** STOP and name each failed specialist or Red Team. Retain queued fixes and restore coverage. If this pass made edits, resume at the next decision; otherwise run a fresh complete Step 9. A successful peer or a QA exception cannot replace missing dispatched coverage. 2. **Third fixing cycle reached (`CYCLES >= 3`):** STOP and report recurring findings with - `converged:false`; do not run a fourth fixing cycle. Under `frozen` the cap comes - sooner: a blocking finding still in `fix` scope on pass 3 (`PASS >= 3`) stops - here too, unfixed. Name each blocking finding (path, severity, problem) for the - owner. Deferred and noted items are never recurring findings, so a capped pass - whose remaining findings are all below the floor applies no edits and continues - at decision 4: the loop ships below the floor. The cap is a stop, not an - approval: a blocker it leaves standing is reported, never waved through. + `converged:false`; do not run a fourth fixing cycle. Name each blocking finding (path, + severity, problem) for the owner, unfixed. The cap is a stop, not an approval. A capped + pass whose remaining findings are all deferred or noted applied no edits: decision 4. 3. **Fixes applied below the cap:** Insert Step 5, affected Steps 6–8 and all of Step 9 before the pending Step 10 in the work list. If the fixes grew the diff past what the request needs (a helper, a guard, a case nothing takes), cut that growth before anything else; the repeat reviews the cut as part of the range. Tests must pass or retain approval for the same verified pre-existing failures and scope. Keep CYCLES and scoped approvals across this repeat; PASS increments on entry. +3.5. **One fix-check.** Take a `gstack-review-log --start review-fix-check` token; write the + fix list, touched hunks (`git diff $PASS_HEAD`), callers of changed symbols and + test results to `.gstack/tmp/fix-check-*.txt`; dispatch one full-effort reviewer + on `gstack-review-plan --fix-check-prompt --fix-list --hunks --tests + --callers `, plus the outside voice only if the plan lists it. Log item 6's + row as `"skill":"review-fix-check"` with `--finish `; not a pass. A + CRITICAL goes to decision 2; otherwise decision 4. 4. **No edits in this pass:** Resolve the required-probe gate below. Only after it clears may you continue to Step 10. A pass whose only findings were deferred or noted applied no edits and continues here: the loop converges below the floor. diff --git a/ship/sections/test-coverage.md b/ship/sections/test-coverage.md index 1d4dfb904..8acd31fe6 100644 --- a/ship/sections/test-coverage.md +++ b/ship/sections/test-coverage.md @@ -8,7 +8,9 @@ For Steps 7, 8 and 10, use the Agent tool with `run_in_background: false`. Omitting the flag runs the subagent in the background. The explicit flag waits for a result while keeping a fresh context. Do not invoke the target as a Skill or run it inline instead. Inline work is allowed only under that section's -documented fallback, after a failed subagent has stopped. +documented fallback, after a failed subagent has stopped. The Step 7 and 8 audits +cannot block, so they run at `light` effort on the same model family (the host +default when `gstack-config get review_effort_tiers` prints `off`). Dispatch the audit through Agent with `subagent_type: "general-purpose"` and `run_in_background: false`, using the shared foreground-dispatch rule above. diff --git a/ship/sections/test-coverage.md.tmpl b/ship/sections/test-coverage.md.tmpl index 671fb1410..d164dab9b 100644 --- a/ship/sections/test-coverage.md.tmpl +++ b/ship/sections/test-coverage.md.tmpl @@ -6,7 +6,9 @@ For Steps 7, 8 and 10, use the Agent tool with `run_in_background: false`. Omitting the flag runs the subagent in the background. The explicit flag waits for a result while keeping a fresh context. Do not invoke the target as a Skill or run it inline instead. Inline work is allowed only under that section's -documented fallback, after a failed subagent has stopped. +documented fallback, after a failed subagent has stopped. The Step 7 and 8 audits +cannot block, so they run at `light` effort on the same model family (the host +default when `gstack-config get review_effort_tiers` prints `off`). Dispatch the audit through Agent with `subagent_type: "general-purpose"` and `run_in_background: false`, using the shared foreground-dispatch rule above. diff --git a/test/autoplan-answer-invalidation.test.ts b/test/autoplan-answer-invalidation.test.ts index d84c72cfb..e63a6923e 100644 --- a/test/autoplan-answer-invalidation.test.ts +++ b/test/autoplan-answer-invalidation.test.ts @@ -19,8 +19,8 @@ const TMP = fs.mkdtempSync(path.join(os.tmpdir(), 'autoplan-answer-')); afterAll(() => fs.rmSync(TMP, { recursive: true, force: true })); /** A gated run with a disagreement in ceo (taste item d1) and the fixture's user challenges (uc1 ceo, uc2 eng). */ -function gatedRun(name: string): string { - const out = startSmallRun(path.join(TMP, name)); +function gatedRun(name: string, extra: string[] = []): string { + const out = startSmallRun(path.join(TMP, name), extra); const disagree = JSON.stringify([ { id: 'O1', severity: 'Low', title: 'The install check lies by omission', file: 'bin/gstack-doctor', line: 12, fix: 'minor' }, { id: 'O2', severity: 'High', title: 'Gate reuse on tree equality alone is unsound', file: 'lib/tree-receipt.ts', line: 3 }, @@ -166,7 +166,7 @@ describe('answer', () => { }, 30_000); test('an eng-only override reopens nothing at answer; revise reruns every phase that reviewed the old plan; p1d reopens every phase; p1c keeps the gate; p1e ends incomplete', () => { - const out = gatedRun('eng-only'); + const out = gatedRun('eng-only', ['--max-rounds', '3']); const engOnly = autoplan(['answer', '--by', 'human', '--out', out, '--gate-rev', '1', '--reply', 'all except uc2b', '--json']); const runId = readRun(out).run; expect(JSON.parse(engOnly.stdout)).toMatchObject({ outcome: 'amend_pending', reopened: [], amendments: [`${runId}-uc2`] }); diff --git a/test/autoplan-budget.test.ts b/test/autoplan-budget.test.ts index 719bd04dd..82976daf1 100644 --- a/test/autoplan-budget.test.ts +++ b/test/autoplan-budget.test.ts @@ -43,15 +43,15 @@ describe('spend: the default host estimate binds and budget_exhausted is a resum const first = r.ap(['next', '--out', out, '--plan', plan, '--no-ui', '--spend-cap', '12']); expect(first.status, first.stderr).toBe(0); expect(first.stdout).toMatch(/^ESTIMATE: 5\.00 per voice reserved for each host-subagent attempt/m); - expect(first.stdout).toMatch(/^ROUNDS: ceo=1\/3 eng=1\/3 max_rounds=3 host_spend=unknown$/m); + expect(first.stdout).toMatch(/^ROUNDS: ceo=1\/2 eng=1\/2 max_rounds=2 host_spend=unknown$/m); expect(spend(out).reserved.map((x: any) => x.estimate_usd)).toEqual([5, 5]); - expect(readRunJson(out).max_rounds).toBe(3); + expect(readRunJson(out).max_rounds).toBe(2); phaseVia(r, out, 'ceo', NONE); expect(unknownUsd(out)).toBe(10); const stopped = r.ap(['next', '--out', out]); expect(stopped.status).toBe(3); expect(stopped.stderr).toMatch(/\(SPEND_CAP_EXCEEDED\)$/m); - expect(stopped.stdout).toMatch(/^STOP: reason=budget_exhausted phase=eng cap=12\.00 spent=0\.00 reserved=0\.00 unknown=2\(10\.00\) next_estimate=10\.00 rounds=ceo:1\/3,eng:1\/3$/m); + expect(stopped.stdout).toMatch(/^STOP: reason=budget_exhausted phase=eng cap=12\.00 spent=0\.00 reserved=0\.00 unknown=2\(10\.00\) next_estimate=10\.00 rounds=ceo:1\/2,eng:1\/2$/m); expect(stopped.stdout).toContain(`RECOVER: gstack-autoplan close --out ${out} --phase eng --deferred`); expect(stopped.stdout).toContain(`RECOVER: gstack-autoplan budget --out ${out} --spend-cap `); expect(stopped.stdout).toContain(`GSTACK_RESULT: skill=autoplan status=incomplete run=${out}`); @@ -97,7 +97,7 @@ describe('close --deferred', () => { expect(noBy.stderr).toMatch(/\(ANSWER_BY_REQUIRED\)$/m); const close = r.ap(['close', '--out', s.out, '--phase', 'ceo', '--deferred', 'owner stop', '--by', 'human']); expect(close.status, close.stderr).toBe(0); - expect(close.stdout).toMatch(/^PHASE_DEFERRED: ceo round=1\/3 by=human open=2 \(Critical=1 Medium=1\) blockers=\S+-ceo-native-1 reason="owner stop"$/m); + expect(close.stdout).toMatch(/^PHASE_DEFERRED: ceo round=1\/2 by=human open=2 \(Critical=1 Medium=1\) blockers=\S+-ceo-native-1 reason="owner stop"$/m); expect(close.stdout).toContain('GSTACK_RESULT: skill=autoplan status=running'); const run = readRunJson(s.out); expect(run.phases.ceo.outside.status).toBe('skipped'); @@ -126,9 +126,9 @@ describe('close --deferred', () => { expect(v.status, v.stdout + v.stderr).toBe(0); const reply = answer(s.out, 1, 'rec'); expect(reply.status, reply.stderr).toBe(0); - expect(reply.stdout).toMatch(/^ROUND_GRANTED: ceo cap=4 \(uc\d+b by human\)$/m); + expect(reply.stdout).toMatch(/^ROUND_GRANTED: ceo cap=3 \(uc\d+b by human\)$/m); expect(reply.stdout).toMatch(/^REOPENED: ceo,eng/m); - expect(reply.stdout).toMatch(/^ROUND_GRANTED: eng cap=4 \(reruns after ceo; uc\d+b by human\)$/m); + expect(reply.stdout).toMatch(/^ROUND_GRANTED: eng cap=3 \(reruns after ceo; uc\d+b by human\)$/m); expect(readRunJson(s.out).round_grants).toEqual({ ceo: 1, eng: 1 }); }); }); diff --git a/test/autoplan-capy-e2e.test.ts b/test/autoplan-capy-e2e.test.ts index f559646b8..f29a7c372 100644 --- a/test/autoplan-capy-e2e.test.ts +++ b/test/autoplan-capy-e2e.test.ts @@ -49,7 +49,7 @@ const ok = (res: { status: number | null; stderr: string; stdout: string }) => { describe('unattended runs on the Capy host', () => { test('a three-revision run driven only through the CLI validates with unique ids and records gstack_version', () => { - const s = start(); + const s = start(['--max-rounds', '3']); expect(readRunJson(s.out).gstack_version).toBe(VERSION); for (const rev of [1, 2]) { phaseVia(r, s.out, 'ceo'); @@ -134,7 +134,7 @@ describe('unattended runs on the Capy host', () => { describe('conformance: one run through every mechanism', () => { test('canonical and attested binds, an unavailable voice, partial then full replies, PLAN_AMEND_PENDING, revise --incorporates, an interruption, three gate revisions', () => { const c: CapyRunner = r; - const s = start(); + const s = start(['--max-rounds', '3']); const run = readRunJson(s.out).run; // ceo: native canonical (fixture), outside attested record ok(c.review(path.join(s.out, 'ceo-native-prompt.md'))); diff --git a/test/autoplan-revise.test.ts b/test/autoplan-revise.test.ts index d3566df66..e58dbbdb3 100644 --- a/test/autoplan-revise.test.ts +++ b/test/autoplan-revise.test.ts @@ -31,8 +31,8 @@ function start(args: string[] = [], text = SMALL_PLAN): { out: string; dir: stri return { out, dir, plan }; } const v2File = (dir: string, text = V2) => { const f = path.join(dir, `plan-v2-${++n}.md`); fs.writeFileSync(f, text); return f; }; -function gated(): { out: string; dir: string; plan: string } { - const s = start(); +function gated(args: string[] = []): { out: string; dir: string; plan: string } { + const s = start(args); phaseVia(r, s.out, 'ceo', { native: [], outside: [] }); expect(phaseVia(r, s.out, 'eng', { native: [], outside: [] }).outside.stdout).toContain('status=gate_pending'); return s; @@ -97,7 +97,7 @@ describe('revise', () => { }); test('a second revise never overwrites the first archive', () => { - const s = gated(); + const s = gated(['--max-rounds', '3']); expect(r.ap(['revise', '--out', s.out, '--plan', v2File(s.dir)]).status).toBe(0); const first = fs.readFileSync(path.join(s.out, 'ceo-native-prompt.r1.md')); phaseVia(r, s.out, 'ceo', { native: [], outside: [] }); diff --git a/test/autoplan-scope.test.ts b/test/autoplan-scope.test.ts index d83b1c6d3..9ec86577f 100644 --- a/test/autoplan-scope.test.ts +++ b/test/autoplan-scope.test.ts @@ -94,7 +94,7 @@ describe('per-axis provenance, --no-dx and the plan marker', () => { const out = path.join(host.base, 'run-pin'); const r = next(['--out', out, '--plan', file, '--no-ui', '--developer-tool']); expect(r.status, r.stderr).toBe(0); - expect(r.stdout).toMatch(/^SCOPE: ui=no \(terms: component×1, screen×1, form×1; visual: none; files: none\) dx=yes \(detected: [^)]*flag×2[^)]*\) source=ui:flag,dx:flag$/m); + expect(r.stdout).toMatch(/^SCOPE: ui=no \(terms: component×1, screen×1, form×1; visual: none; files: none\) dx=yes \(detected: [^)]*flag×2[^)]*\) docs_only=no \(detected\) source=ui:flag,dx:flag$/m); expect(r.stdout).toContain(`SCOPE_PIN: add as the first line of ${file} to keep these flags`); expect(fs.readFileSync(file).equals(before)).toBe(true); expect(readRun(out).scope).toMatchObject({ cli_flags: ['--no-ui', '--developer-tool'], marker: null, sources: { ui: 'flag', dx: 'flag' } }); diff --git a/test/autoplan-token-efficiency.test.ts b/test/autoplan-token-efficiency.test.ts new file mode 100644 index 000000000..d0ccb8e12 --- /dev/null +++ b/test/autoplan-token-efficiency.test.ts @@ -0,0 +1,242 @@ +/** + * GSTA-131 token efficiency in the unattended /autoplan runner, on the + * simulated Capy host: new unattended runs default to two review rounds (a + * resumed run keeps what it persisted); `revise --incorporates` applies an + * edit without re-review when every incorporated decision resolves below High + * and nothing new comes into scope, and reopens on anything else; the gate at + * the round cap prints ESCALATE while High findings stay open; a docs-only + * plan runs CEO (native voice only) and Eng; export counts every dispatched + * attempt and a relative input-token estimate. Every spawn has a timeout. + */ +import { afterAll, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { detectDocsOnly, docsClassPath } from '../lib/autoplan-scope'; +import { ROOT, SMALL_PLAN } from './helpers/autoplan-run-fixture'; +import { capyRunner, phaseVia, readJsonl, readRunJson, writePlan } from './helpers/autoplan-capy'; + +const r = capyRunner(); +afterAll(() => r.cleanup()); +let n = 0; +const NONE = { native: [], outside: [] }; +const V2 = SMALL_PLAN.replace('cap 30 s', 'cap 30 s, jitter drawn from a seeded source'); +const DX_V2 = SMALL_PLAN.replace('Tests cover', 'Add a CLI flag --max-retries and an error message for a bad value. Tests cover'); +const DOCS_PLAN = '# Retry docs\n\n## Implementation plan\n\nRewrite README.md and docs/howto-retry.md so they explain the retry backoff, and add a CHANGELOG.md entry.\n\n## Review record\n'; +const journal = (out: string) => readJsonl(path.join(out, 'attempts.jsonl')); +const voices = (stdout: string) => stdout.split('\n').filter(l => l.startsWith('ATTEMPT: ')).map(l => /voice=(\w+)/.exec(l)![1]); +function start(args: string[] = [], text = SMALL_PLAN) { + const dir = path.join(r.host.base, `tok${++n}`); + const plan = writePlan(dir, text); + const out = path.join(dir, 'run'); + const first = r.ap(['next', '--out', out, '--plan', plan, '--no-ui', ...args]); + if (first.status !== 0) throw new Error(`first next failed: ${first.stderr}\n${first.stdout}`); + return { out, dir, plan, first }; +} +const v2File = (dir: string, text = V2) => { const f = path.join(dir, `plan-v2-${++n}.md`); fs.writeFileSync(f, text); return f; }; +const disagreement = (native: string, outside: string) => ({ + native: [{ id: 'F1', severity: native, title: 'Jitter source is unstated', file: 'lib/worker.ts', line: 19, fix: 'name the source' }], + outside: [{ id: 'O1', severity: outside, title: 'Jitter source is unstated', file: 'lib/worker.ts', line: 19, fix: 'name the source' }], +}); +/** A run gated on one ceo disagreement d1, answered with its plan-changing option: amend_pending on d1. */ +function amended(native: string, outside: string, args: string[] = []) { + const s = start(args); + phaseVia(r, s.out, 'ceo', disagreement(native, outside)); + expect(phaseVia(r, s.out, 'eng', NONE).outside.stdout).toContain('status=gate_pending'); + const res = r.ap(['answer', '--by', 'human', '--out', s.out, '--gate-rev', '1', '--reply', 'all except d1b']); + expect(res.status, res.stderr).toBe(0); + expect(readRunJson(s.out).status).toBe('amend_pending'); + return { ...s, run: readRunJson(s.out).run as string }; +} +function bindNative(out: string) { + const run = readRunJson(out); + expect(r.review(path.join(out, 'ceo-native-prompt.md'), { FIXTURE_FINDINGS: '[]' }).status).toBe(0); + return r.ap(['submit', '--out', out, '--phase', 'ceo', '--voice', 'native', '--result', run.phases.ceo.native.attempts[0].result, '--model', 'anthropic/claude-opus-5-5', '--attempt', run.phases.ceo.native.attempt]); +} + +describe('a quiet revise after a capped one', () => { + // Value: protects=the owner still sees an earlier capped delta; fails_when=a below-High revise after a capped one leaves the + // answered gate in place (gate_rev unchanged, the delta item never rendered); seam=none; no_claim=none + test('regates so the capped delta reaches the owner', () => { + const s = start(['--max-rounds', '1']); + phaseVia(r, s.out, 'ceo', { + native: [{ id: 'F1', severity: 'High', title: 'Jitter source is unstated', file: 'lib/worker.ts', line: 19, fix: 'name the source' }, + { id: 'F2', severity: 'Medium', title: 'Cap value is unexplained', file: 'lib/config.ts', line: 3, fix: 'explain the cap' }], + outside: [{ id: 'O1', severity: 'Low', title: 'Jitter source is unstated', file: 'lib/worker.ts', line: 19, fix: 'name the source' }, + { id: 'O2', severity: 'Informational', title: 'Cap value is unexplained', file: 'lib/config.ts', line: 3, fix: 'explain the cap' }], + }); + phaseVia(r, s.out, 'eng', NONE); + expect(r.ap(['answer', '--by', 'human', '--out', s.out, '--gate-rev', '1', '--reply', 'all except d1b d2b']).status).toBe(0); + const first = r.ap(['revise', '--out', s.out, '--plan', v2File(s.dir), '--incorporates', 'd1']); + expect(first.stdout).toMatch(/capped=/); + const second = r.ap(['revise', '--out', s.out, '--plan', v2File(s.dir, SMALL_PLAN.replace('cap 30 s', 'cap 30 s (explained), jitter drawn from a seeded source')), '--incorporates', 'd2']); + expect(second.status, second.stderr).toBe(0); + expect(second.stdout).toContain('without re-review (below High; plan +'); + const run = readRunJson(s.out); + expect(run.status).toBe('gate_pending'); + expect(run.gate_rev).toBe(2); + expect(JSON.parse(fs.readFileSync(path.join(s.out, 'gate.json'), 'utf8')).gate_rev).toBe(2); + }, 30_000); +}); + +describe('round budget', () => { + test('a new unattended run defaults to two rounds per phase', () => { + const s = start(); + expect(s.first.stdout).toMatch(/^ROUNDS: ceo=1\/2 eng=1\/2 max_rounds=2 host_spend=unknown$/m); + expect(readRunJson(s.out).max_rounds).toBe(2); + }); + + test('a resumed run keeps the max_rounds it persisted', () => { + const s = start(); + fs.writeFileSync(path.join(s.out, 'run.json'), JSON.stringify({ ...readRunJson(s.out), max_rounds: 1 }, null, 2) + '\n'); + expect(r.ap(['resume', '--out', s.out]).status).toBe(0); + expect(readRunJson(s.out).max_rounds).toBe(1); + expect(r.ap(['status', '--out', s.out]).stdout).toMatch(/^ROUNDS: ceo=1\/1 eng=1\/1 max_rounds=1 /m); + }); +}); + +describe('revise by severity', () => { + test('below-High decisions apply without re-review: no phase reopens, snapshots rebind, the run completes', () => { + const g = amended('Medium', 'Informational'); + const before = readRunJson(g.out); + const rev = r.ap(['revise', '--out', g.out, '--plan', v2File(g.dir), '--incorporates', 'd1']); + expect(rev.status, rev.stderr).toBe(0); + expect(rev.stdout).toMatch(/^REVISED: plan=[0-9a-f]{12} reopened=none$/m); + expect(rev.stdout).toContain(`APPLIED: ${g.run}-d1 without re-review (below High; plan +`); + expect(rev.stdout).toContain('GSTACK_RESULT: skill=autoplan status=complete'); + const run = readRunJson(g.out); + const row = journal(g.out).find(e => e.event === 'applied_without_rereview'); + expect(row).toMatchObject({ decisions: [`${g.run}-d1`], severities: { [`${g.run}-d1`]: 'Medium' }, phases: ['ceo', 'eng'], old_plan_sha256: before.phases.ceo.snapshot }); + for (const p of ['ceo', 'eng']) { + expect(run.phases[p].snapshot).toBe(row.new_plan_sha256); + expect(run.phases[p].rebound).toEqual([expect.objectContaining({ from: before.phases[p].snapshot, to: row.new_plan_sha256, decisions: [`${g.run}-d1`] })]); + expect(run.phases[p].closed_at).toBe(before.phases[p].closed_at); + } + expect(run.attempt_counter).toBe(before.attempt_counter); + expect(journal(g.out).filter(e => e.event === 'reopened')).toEqual([]); + expect(run.amendments[0]).toMatchObject({ status: 'incorporated' }); + const v = r.validate(g.out); + expect(v.status, v.stdout + v.stderr).toBe(0); + }, 30_000); + + test('a High decision reopens as before', () => { + const g = amended('High', 'Low'); + const rev = r.ap(['revise', '--out', g.out, '--plan', v2File(g.dir), '--incorporates', 'd1']); + expect(rev.stdout).toMatch(/^REVISED: plan=\S+ reopened=ceo,eng$/m); + expect(rev.stdout).not.toContain('APPLIED:'); + expect(journal(g.out).some(e => e.event === 'applied_without_rereview')).toBe(false); + }, 30_000); + + test('a decision of unknown severity reopens', () => { + const g = amended('Medium', 'Informational'); + const file = path.join(g.out, 'decisions.jsonl'); + fs.writeFileSync(file, readJsonl(file).map(d => JSON.stringify(d.label === 'D1' ? { ...d, origin: undefined } : d)).join('\n') + '\n'); + const rev = r.ap(['revise', '--out', g.out, '--plan', v2File(g.dir), '--incorporates', 'd1']); + expect(rev.status, rev.stderr).toBe(0); + expect(rev.stdout).toMatch(/reopened=ceo,eng$/m); + expect(rev.stdout).not.toContain('APPLIED:'); + }, 30_000); + + test('a below-High edit that brings a phase into scope reopens and adds it', () => { + const g = amended('Medium', 'Informational'); + const rev = r.ap(['revise', '--out', g.out, '--plan', v2File(g.dir, DX_V2), '--incorporates', 'd1']); + expect(rev.status, rev.stderr).toBe(0); + expect(rev.stdout).toMatch(/^REVISED: plan=\S+ reopened=ceo,eng added=dx$/m); + expect(rev.stdout).not.toContain('APPLIED:'); + }, 30_000); + + test('at the round cap a below-High edit applies without a capped delta; a High one keeps the unreviewed-delta gate item', () => { + const low = amended('Medium', 'Informational', ['--max-rounds', '1']); + const quiet = r.ap(['revise', '--out', low.out, '--plan', v2File(low.dir), '--incorporates', 'd1']); + expect(quiet.stdout).toContain(`APPLIED: ${low.run}-d1 without re-review (below High; plan +`); + expect(readRunJson(low.out).capped_deltas).toBeUndefined(); + const high = amended('High', 'Low', ['--max-rounds', '1']); + const capped = r.ap(['revise', '--out', high.out, '--plan', v2File(high.dir), '--incorporates', 'd1']); + expect(capped.stdout).toMatch(/^REVISED: plan=\S+ reopened=none capped=ceo,eng$/m); + expect(capped.stdout).not.toContain('APPLIED:'); + expect(readRunJson(high.out).capped_deltas.map((d: any) => d.phase)).toEqual(['ceo', 'eng']); + }, 30_000); +}); + +describe('ESCALATE at the round cap', () => { + const HIGH = { native: [{ severity: 'High', title: 'Retries can double-charge', fix: 'idempotency key' }], outside: [] }; + test('the gate at the cap with a High finding open names the decision', () => { + const s = start(['--max-rounds', '1']); + phaseVia(r, s.out, 'ceo', HIGH); + const eng = phaseVia(r, s.out, 'eng', NONE); + expect(eng.outside.stdout).toMatch(/^GATE: gate_rev=1 /m); + expect(eng.outside.stdout.split('\n').filter(l => l.startsWith('ESCALATE:'))).toEqual(['ESCALATE: evidence sufficient, decision needed (ceo: 1 High open)']); + }, 30_000); + test('control: below the cap, or at it with nothing High open, no ESCALATE', () => { + const s = start(); + phaseVia(r, s.out, 'ceo', HIGH); + expect(phaseVia(r, s.out, 'eng', NONE).outside.stdout).not.toContain('ESCALATE:'); + const m = start(['--max-rounds', '1']); + phaseVia(r, m.out, 'ceo', { native: [{ severity: 'Medium', title: 'Jitter source is unstated' }], outside: [] }); + const gate = phaseVia(r, m.out, 'eng', NONE).outside.stdout; + expect(gate).toMatch(/^GATE: /m); + expect(gate).not.toContain('ESCALATE:'); + }, 30_000); +}); + +describe('docs-only', () => { + test('docs-class paths', () => { + for (const p of ['README.md', 'CHANGELOG.md', 'TODOS.md', 'docs/howto-retry.md', 'docs/designs/x/PLAN.md', 'notes.md', './README.md', 'docs/images/flow.png', 'docs/designs']) expect(docsClassPath(p, ROOT), p).toBe(true); + for (const p of ['docs/review-policy.md', 'autoplan/SKILL.md.tmpl', 'autoplan/SKILL.md', 'review/sections/army.md', 'scripts/resolvers/autoplan-driver.ts', 'autoplan/DRIVER.md', + 'captain/templates/brief.md', 'autoplan/notes.md', 'AGENTS.md', 'CLAUDE.md', 'lib/worker.ts', 'bin/gstack-autoplan']) expect(docsClassPath(p, ROOT), p).toBe(false); + expect(detectDocsOnly('Rewrite README.md and docs/howto.md.', ROOT)).toMatchObject({ docsOnly: true, code: [] }); + expect(detectDocsOnly('Rewrite README.md and docs/review-policy.md.', ROOT)).toMatchObject({ docsOnly: false, code: ['docs/review-policy.md'] }); + expect(detectDocsOnly('Make the retry docs clearer.', ROOT).docsOnly).toBe(false); + }); + + test('detected: CEO dispatches the native voice only, Eng both; SCOPE reports it; DISPATCHES counts what ran', () => { + const s = start([], DOCS_PLAN); + expect(s.first.stdout).toMatch(/^SCOPE: ui=no .* docs_only=yes \(detected\) source=ui:flag,dx:detected$/m); + expect(s.first.stdout).toMatch(/^PHASE: ceo snapshot=[0-9a-f]{64} order=ceo,eng$/m); + expect(s.first.stdout).toContain('OUTSIDE: ceo not_run reason=docs_only (native voice only)'); + expect(voices(s.first.stdout)).toEqual(['native']); + expect(readRunJson(s.out).phases.ceo.outside).toMatchObject({ status: 'disabled', outcome: 'not_run', reason: 'docs_only' }); + expect(bindNative(s.out).stdout).toMatch(/^PHASE_CLOSED: ceo /m); + const eng = phaseVia(r, s.out, 'eng', NONE); + expect(voices(eng.next.stdout)).toEqual(['native', 'outside']); + expect(eng.outside.stdout).toContain('status=gate_pending'); + const exp = r.ap(['export', '--out', s.out]); + expect(exp.status, exp.stderr).toBe(0); + const run = readRunJson(s.out); + const bytes = journal(s.out).filter(e => e.event === 'dispatched').reduce((t, e) => t + e.prompt_bytes, 0); + expect(run.dispatches).toEqual({ total: 3, by_phase: { ceo: { native: 1 }, eng: { native: 1, outside: 1 } } }); + expect(run.est_input_tokens).toBe(Math.round(bytes / 4)); + expect(exp.stdout).toContain(`DISPATCHES: 3 est_input_tokens=${run.est_input_tokens} (relative estimate)`); + const v = r.validate(s.out); + expect(v.status, v.stdout + v.stderr).toBe(0); + }, 30_000); + + test('--docs-only on the first next forces CEO + Eng on a code plan; a phase-enabling flag outranks detection', () => { + const flagged = start(['--docs-only', '--developer-tool']); + expect(flagged.first.stdout).toMatch(/ docs_only=yes \(flag\) source=/m); + expect(readRunJson(flagged.out).phase_order).toEqual(['ceo', 'eng']); + const code = start(); + expect(code.first.stdout).toMatch(/ docs_only=no \(detected\) source=/m); + expect(voices(code.first.stdout)).toEqual(['native', 'outside']); + const dx = start(['--developer-tool'], DOCS_PLAN); + expect(dx.first.stdout).toMatch(/ docs_only=no \(detected\) source=/m); + expect(readRunJson(dx.out).phase_order).toEqual(['ceo', 'dx', 'eng']); + }); +}); + +describe('dispatch counts', () => { + test('failed attempts count; a run.json without the fields still validates', () => { + const s = start(); + expect(bindNative(s.out).status).toBe(0); + expect(r.ap(['submit', '--out', s.out, '--phase', 'ceo', '--voice', 'outside', '--failed', 'runner crashed']).stdout).toMatch(/^PHASE_CLOSED: ceo /m); + phaseVia(r, s.out, 'eng', NONE); + const exp = r.ap(['export', '--out', s.out, '--json']); + expect(JSON.parse(exp.stdout)).toMatchObject({ dispatches: { total: 4, by_phase: { ceo: { native: 1, outside: 1 }, eng: { native: 1, outside: 1 } } } }); + const run = readRunJson(s.out); + expect(run.est_input_tokens).toBeGreaterThan(0); + delete run.dispatches; delete run.est_input_tokens; + fs.writeFileSync(path.join(s.out, 'run.json'), JSON.stringify(run, null, 2) + '\n'); + const v = r.validate(s.out); + expect(v.status, v.stdout + v.stderr).toBe(0); + expect(r.ap(['status', '--out', s.out]).status).toBe(0); + }, 30_000); +}); diff --git a/test/fixtures/golden/agy-ship-SKILL.md b/test/fixtures/golden/agy-ship-SKILL.md index b1367a6c8..7494fe0c4 100644 --- a/test/fixtures/golden/agy-ship-SKILL.md +++ b/test/fixtures/golden/agy-ship-SKILL.md @@ -205,9 +205,9 @@ not a commit ID. Use `git diff ` to compare these snapshots **Review policy** (docs/review-policy.md): a finding blocks another review pass only when it would fail a test, break production, or lose data or secrets; everything below that floor ships as a named note in the PR body. Pass 1 reviews -the diff; later passes read only the range since the previous pass; three passes -is the cap. `NO FINDINGS` is a complete reviewer result. The ship receipt's -`review_loop` counters record the outcome; `ship: review_scope` (Step 3) is the knob. +the diff; a later pass reads only the range; two full passes plus one fix-check +is the cap. `NO FINDINGS` is a complete reviewer result. `ship: review_scope` (Step 3) +is the knob; the receipt's `review_loop` records the outcome. ### Ship control flow @@ -328,12 +328,11 @@ repository-landing asks, including on Apple repos. `git log origin/..HEAD --oneline`. 4. **Carry check.** If `gh pr list --head --state open --json number` - names a PR, run `~/.gemini/antigravity-cli/skills/gstack/bin/gstack-ship-receipt carry --pr --base `. - `CARRY: carried` means the product diff against `origin/` has the same - patch-id as the reviewed head's (only a base merge, tests or release metadata - changed; the receipt's `carry` object holds the range-diff evidence): the receipt - now binds HEAD and the run is complete at its `GSTACK_RESULT` line, so stop. - `CARRY: refused` or `current` continues; a product change always re-runs Steps 2–16. + names a PR, run `~/.gemini/antigravity-cli/skills/gstack/bin/gstack-ship-receipt carry --pr --base --dry-run`. + `CARRY: would carry` (base merge committed, same product patch-id): skip review, fix + what it broke (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry + without `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD and + ends the run. A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. 5. Display historical readiness using the dashboard below, then finish Step 1. Prior CLEAR reviews or dashboard skips never replace Step 9's gates. @@ -397,8 +396,7 @@ This verdict never skips Step 9 or its finding, approval and convergence gates. Other rows provide context, not a substitute for Eng Review: - Recommend CEO Review for product/business or scope decisions, not routine fixes or cleanup. - Recommend Design Review for UI/UX work, not backend, infrastructure or prompt-only work. -- Adversarial review always includes a native pass. Available, enabled outside - challenges supplement it; diffs of 200+ lines also get the structured P1 gate. +- Step 9's review plan decides the adversarial passes (Step 11). - Outside Voice is the default-on plan review after CEO/Eng review. `codex_reviews` disables that extra step. Provider failure uses native fallback and records missing outside coverage; this dashboard row never gates shipping. @@ -506,12 +504,14 @@ repeat Step 2 (decisions included) on the merged content, then Step 4. ## Step 11.5: Bind the reviews 1. **Select the two reviews.** Run `~/.gemini/antigravity-cli/skills/gstack/bin/gstack-review-read`. - Select this invocation's final Step 9.4 record (`skill:"review"`, `via:"ship"`) - and Step 11 native record (`skill:"adversarial-review"`). Match each to its saved + Select this invocation's final Step 9 record (`skill:"review"`, `via:"ship"`, or a + last 3.5 `review-fix-check`) and Step 11 native record (`skill:"adversarial-review"`; + none after `ADVERSARIAL: skipped`). Match each to its saved handle, original token and source; reject outside-provider or older records. -2. **Compare their content.** Require the native record's `review_binding.state` - to be `verified`. All three snapshots must match: its `wtree`, Step 9.4's - `review_binding.start_wtree` and `review_binding.end_wtree`. A mismatch or missing +2. **Compare their content.** All three snapshots must match: the native record's + `wtree` (its `review_binding.state` `verified`), the Step 9 record's + `review_binding.start_wtree` and `review_binding.end_wtree`; after a planned skip, the + last two with `review_freshness` `CURRENT`. A mismatch or a missing required record/field blocks release preparation: report **Review records missing or mismatched** and insert `9 → 10 → 11 → 11.5` before Step 12. Bind the new records at 11.5. Never attach new tokens to old work. @@ -751,7 +751,7 @@ command, result/counts, timestamp and log, never resample. Mandatory reviews sti `--allow-paths` exempts only release metadata. A `package.json` version-only edit can qualify; scripts, dependencies and runtime configuration require live tests. Uncertain edits cannot be exempted. Docs, TODO edits, new/generated tests and fixes -make evidence STALE even without a new code review. Use this example only once +make evidence STALE even without a new code review; a Step 14 TODOS.md edit reruns them. Use this example only once every allowed edit is release metadata: ```bash diff --git a/test/fixtures/golden/claude-ship-SKILL.md b/test/fixtures/golden/claude-ship-SKILL.md index 04062758f..881cd3934 100644 --- a/test/fixtures/golden/claude-ship-SKILL.md +++ b/test/fixtures/golden/claude-ship-SKILL.md @@ -205,9 +205,9 @@ not a commit ID. Use `git diff ` to compare these snapshots **Review policy** (docs/review-policy.md): a finding blocks another review pass only when it would fail a test, break production, or lose data or secrets; everything below that floor ships as a named note in the PR body. Pass 1 reviews -the diff; later passes read only the range since the previous pass; three passes -is the cap. `NO FINDINGS` is a complete reviewer result. The ship receipt's -`review_loop` counters record the outcome; `ship: review_scope` (Step 3) is the knob. +the diff; a later pass reads only the range; two full passes plus one fix-check +is the cap. `NO FINDINGS` is a complete reviewer result. `ship: review_scope` (Step 3) +is the knob; the receipt's `review_loop` records the outcome. ### Ship control flow @@ -328,12 +328,11 @@ repository-landing asks, including on Apple repos. `git log origin/..HEAD --oneline`. 4. **Carry check.** If `gh pr list --head --state open --json number` - names a PR, run `~/.claude/skills/gstack/bin/gstack-ship-receipt carry --pr --base `. - `CARRY: carried` means the product diff against `origin/` has the same - patch-id as the reviewed head's (only a base merge, tests or release metadata - changed; the receipt's `carry` object holds the range-diff evidence): the receipt - now binds HEAD and the run is complete at its `GSTACK_RESULT` line, so stop. - `CARRY: refused` or `current` continues; a product change always re-runs Steps 2–16. + names a PR, run `~/.claude/skills/gstack/bin/gstack-ship-receipt carry --pr --base --dry-run`. + `CARRY: would carry` (base merge committed, same product patch-id): skip review, fix + what it broke (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry + without `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD and + ends the run. A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. 5. Display historical readiness using the dashboard below, then finish Step 1. Prior CLEAR reviews or dashboard skips never replace Step 9's gates. @@ -397,8 +396,7 @@ This verdict never skips Step 9 or its finding, approval and convergence gates. Other rows provide context, not a substitute for Eng Review: - Recommend CEO Review for product/business or scope decisions, not routine fixes or cleanup. - Recommend Design Review for UI/UX work, not backend, infrastructure or prompt-only work. -- Adversarial review always includes a native pass. Available, enabled outside - challenges supplement it; diffs of 200+ lines also get the structured P1 gate. +- Step 9's review plan decides the adversarial passes (Step 11). - Outside Voice is the default-on plan review after CEO/Eng review. `codex_reviews` disables that extra step. Provider failure uses native fallback and records missing outside coverage; this dashboard row never gates shipping. @@ -506,12 +504,14 @@ repeat Step 2 (decisions included) on the merged content, then Step 4. ## Step 11.5: Bind the reviews 1. **Select the two reviews.** Run `~/.claude/skills/gstack/bin/gstack-review-read`. - Select this invocation's final Step 9.4 record (`skill:"review"`, `via:"ship"`) - and Step 11 native record (`skill:"adversarial-review"`). Match each to its saved + Select this invocation's final Step 9 record (`skill:"review"`, `via:"ship"`, or a + last 3.5 `review-fix-check`) and Step 11 native record (`skill:"adversarial-review"`; + none after `ADVERSARIAL: skipped`). Match each to its saved handle, original token and source; reject outside-provider or older records. -2. **Compare their content.** Require the native record's `review_binding.state` - to be `verified`. All three snapshots must match: its `wtree`, Step 9.4's - `review_binding.start_wtree` and `review_binding.end_wtree`. A mismatch or missing +2. **Compare their content.** All three snapshots must match: the native record's + `wtree` (its `review_binding.state` `verified`), the Step 9 record's + `review_binding.start_wtree` and `review_binding.end_wtree`; after a planned skip, the + last two with `review_freshness` `CURRENT`. A mismatch or a missing required record/field blocks release preparation: report **Review records missing or mismatched** and insert `9 → 10 → 11 → 11.5` before Step 12. Bind the new records at 11.5. Never attach new tokens to old work. @@ -751,7 +751,7 @@ command, result/counts, timestamp and log, never resample. Mandatory reviews sti `--allow-paths` exempts only release metadata. A `package.json` version-only edit can qualify; scripts, dependencies and runtime configuration require live tests. Uncertain edits cannot be exempted. Docs, TODO edits, new/generated tests and fixes -make evidence STALE even without a new code review. Use this example only once +make evidence STALE even without a new code review; a Step 14 TODOS.md edit reruns them. Use this example only once every allowed edit is release metadata: ```bash diff --git a/test/fixtures/golden/codex-ship-SKILL.md b/test/fixtures/golden/codex-ship-SKILL.md index e45732e74..c4d937f91 100644 --- a/test/fixtures/golden/codex-ship-SKILL.md +++ b/test/fixtures/golden/codex-ship-SKILL.md @@ -203,9 +203,9 @@ not a commit ID. Use `git diff ` to compare these snapshots **Review policy** (docs/review-policy.md): a finding blocks another review pass only when it would fail a test, break production, or lose data or secrets; everything below that floor ships as a named note in the PR body. Pass 1 reviews -the diff; later passes read only the range since the previous pass; three passes -is the cap. `NO FINDINGS` is a complete reviewer result. The ship receipt's -`review_loop` counters record the outcome; `ship: review_scope` (Step 3) is the knob. +the diff; a later pass reads only the range; two full passes plus one fix-check +is the cap. `NO FINDINGS` is a complete reviewer result. `ship: review_scope` (Step 3) +is the knob; the receipt's `review_loop` records the outcome. ### Ship control flow @@ -326,12 +326,11 @@ repository-landing asks, including on Apple repos. `git log origin/..HEAD --oneline`. 4. **Carry check.** If `gh pr list --head --state open --json number` - names a PR, run `$GSTACK_ROOT/bin/gstack-ship-receipt carry --pr --base `. - `CARRY: carried` means the product diff against `origin/` has the same - patch-id as the reviewed head's (only a base merge, tests or release metadata - changed; the receipt's `carry` object holds the range-diff evidence): the receipt - now binds HEAD and the run is complete at its `GSTACK_RESULT` line, so stop. - `CARRY: refused` or `current` continues; a product change always re-runs Steps 2–16. + names a PR, run `$GSTACK_ROOT/bin/gstack-ship-receipt carry --pr --base --dry-run`. + `CARRY: would carry` (base merge committed, same product patch-id): skip review, fix + what it broke (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry + without `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD and + ends the run. A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. 5. Display historical readiness using the dashboard below, then finish Step 1. Prior CLEAR reviews or dashboard skips never replace Step 9's gates. @@ -396,8 +395,7 @@ This verdict never skips Step 9 or its finding, approval and convergence gates. Other rows provide context, not a substitute for Eng Review: - Recommend CEO Review for product/business or scope decisions, not routine fixes or cleanup. - Recommend Design Review for UI/UX work, not backend, infrastructure or prompt-only work. -- Adversarial review always includes a native pass. Available, enabled outside - challenges supplement it; diffs of 200+ lines also get the structured P1 gate. +- Step 9's review plan decides the adversarial passes (Step 11). - Outside Voice is the default-on plan review after CEO/Eng review. `codex_reviews` disables that extra step. Provider failure uses native fallback and records missing outside coverage; this dashboard row never gates shipping. @@ -505,12 +503,14 @@ repeat Step 2 (decisions included) on the merged content, then Step 4. ## Step 11.5: Bind the reviews 1. **Select the two reviews.** Run `$GSTACK_ROOT/bin/gstack-review-read`. - Select this invocation's final Step 9.4 record (`skill:"review"`, `via:"ship"`) - and Step 11 native record (`skill:"adversarial-review"`). Match each to its saved + Select this invocation's final Step 9 record (`skill:"review"`, `via:"ship"`, or a + last 3.5 `review-fix-check`) and Step 11 native record (`skill:"adversarial-review"`; + none after `ADVERSARIAL: skipped`). Match each to its saved handle, original token and source; reject outside-provider or older records. -2. **Compare their content.** Require the native record's `review_binding.state` - to be `verified`. All three snapshots must match: its `wtree`, Step 9.4's - `review_binding.start_wtree` and `review_binding.end_wtree`. A mismatch or missing +2. **Compare their content.** All three snapshots must match: the native record's + `wtree` (its `review_binding.state` `verified`), the Step 9 record's + `review_binding.start_wtree` and `review_binding.end_wtree`; after a planned skip, the + last two with `review_freshness` `CURRENT`. A mismatch or a missing required record/field blocks release preparation: report **Review records missing or mismatched** and insert `9 → 10 → 11 → 11.5` before Step 12. Bind the new records at 11.5. Never attach new tokens to old work. @@ -754,7 +754,7 @@ command, result/counts, timestamp and log, never resample. Mandatory reviews sti `--allow-paths` exempts only release metadata. A `package.json` version-only edit can qualify; scripts, dependencies and runtime configuration require live tests. Uncertain edits cannot be exempted. Docs, TODO edits, new/generated tests and fixes -make evidence STALE even without a new code review. Use this example only once +make evidence STALE even without a new code review; a Step 14 TODOS.md edit reruns them. Use this example only once every allowed edit is release metadata: ```bash diff --git a/test/fixtures/golden/factory-ship-SKILL.md b/test/fixtures/golden/factory-ship-SKILL.md index 0c756e903..374d879a4 100644 --- a/test/fixtures/golden/factory-ship-SKILL.md +++ b/test/fixtures/golden/factory-ship-SKILL.md @@ -205,9 +205,9 @@ not a commit ID. Use `git diff ` to compare these snapshots **Review policy** (docs/review-policy.md): a finding blocks another review pass only when it would fail a test, break production, or lose data or secrets; everything below that floor ships as a named note in the PR body. Pass 1 reviews -the diff; later passes read only the range since the previous pass; three passes -is the cap. `NO FINDINGS` is a complete reviewer result. The ship receipt's -`review_loop` counters record the outcome; `ship: review_scope` (Step 3) is the knob. +the diff; a later pass reads only the range; two full passes plus one fix-check +is the cap. `NO FINDINGS` is a complete reviewer result. `ship: review_scope` (Step 3) +is the knob; the receipt's `review_loop` records the outcome. ### Ship control flow @@ -328,12 +328,11 @@ repository-landing asks, including on Apple repos. `git log origin/..HEAD --oneline`. 4. **Carry check.** If `gh pr list --head --state open --json number` - names a PR, run `$GSTACK_ROOT/bin/gstack-ship-receipt carry --pr --base `. - `CARRY: carried` means the product diff against `origin/` has the same - patch-id as the reviewed head's (only a base merge, tests or release metadata - changed; the receipt's `carry` object holds the range-diff evidence): the receipt - now binds HEAD and the run is complete at its `GSTACK_RESULT` line, so stop. - `CARRY: refused` or `current` continues; a product change always re-runs Steps 2–16. + names a PR, run `$GSTACK_ROOT/bin/gstack-ship-receipt carry --pr --base --dry-run`. + `CARRY: would carry` (base merge committed, same product patch-id): skip review, fix + what it broke (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry + without `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD and + ends the run. A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. 5. Display historical readiness using the dashboard below, then finish Step 1. Prior CLEAR reviews or dashboard skips never replace Step 9's gates. @@ -398,8 +397,7 @@ This verdict never skips Step 9 or its finding, approval and convergence gates. Other rows provide context, not a substitute for Eng Review: - Recommend CEO Review for product/business or scope decisions, not routine fixes or cleanup. - Recommend Design Review for UI/UX work, not backend, infrastructure or prompt-only work. -- Adversarial review always includes a native pass. Available, enabled outside - challenges supplement it; diffs of 200+ lines also get the structured P1 gate. +- Step 9's review plan decides the adversarial passes (Step 11). - Outside Voice is the default-on plan review after CEO/Eng review. `codex_reviews` disables that extra step. Provider failure uses native fallback and records missing outside coverage; this dashboard row never gates shipping. @@ -507,12 +505,14 @@ repeat Step 2 (decisions included) on the merged content, then Step 4. ## Step 11.5: Bind the reviews 1. **Select the two reviews.** Run `$GSTACK_ROOT/bin/gstack-review-read`. - Select this invocation's final Step 9.4 record (`skill:"review"`, `via:"ship"`) - and Step 11 native record (`skill:"adversarial-review"`). Match each to its saved + Select this invocation's final Step 9 record (`skill:"review"`, `via:"ship"`, or a + last 3.5 `review-fix-check`) and Step 11 native record (`skill:"adversarial-review"`; + none after `ADVERSARIAL: skipped`). Match each to its saved handle, original token and source; reject outside-provider or older records. -2. **Compare their content.** Require the native record's `review_binding.state` - to be `verified`. All three snapshots must match: its `wtree`, Step 9.4's - `review_binding.start_wtree` and `review_binding.end_wtree`. A mismatch or missing +2. **Compare their content.** All three snapshots must match: the native record's + `wtree` (its `review_binding.state` `verified`), the Step 9 record's + `review_binding.start_wtree` and `review_binding.end_wtree`; after a planned skip, the + last two with `review_freshness` `CURRENT`. A mismatch or a missing required record/field blocks release preparation: report **Review records missing or mismatched** and insert `9 → 10 → 11 → 11.5` before Step 12. Bind the new records at 11.5. Never attach new tokens to old work. @@ -756,7 +756,7 @@ command, result/counts, timestamp and log, never resample. Mandatory reviews sti `--allow-paths` exempts only release metadata. A `package.json` version-only edit can qualify; scripts, dependencies and runtime configuration require live tests. Uncertain edits cannot be exempted. Docs, TODO edits, new/generated tests and fixes -make evidence STALE even without a new code review. Use this example only once +make evidence STALE even without a new code review; a Step 14 TODOS.md edit reruns them. Use this example only once every allowed edit is release metadata: ```bash diff --git a/test/fixtures/review-depth/code.json b/test/fixtures/review-depth/code.json new file mode 100644 index 000000000..4620310e1 --- /dev/null +++ b/test/fixtures/review-depth/code.json @@ -0,0 +1,67 @@ +{ + "name": "code", + "commit": "392f50d", + "paths": [ + "CHANGELOG.md", + "VERSION", + "agents-digest/gstack-AGENTS.md", + "package.json", + "plan-ceo-review/sections/review-sections.md", + "plan-devex-review/sections/review-sections.md", + "plan-eng-review/sections/review-sections.md", + "review/SKILL.md", + "review/SKILL.md.tmpl", + "review/sections/manifest.json", + "review/sections/plan-completion.md", + "review/sections/review-army.md", + "scripts/resolvers/outside-voice-steps.ts", + "scripts/resolvers/plan-gates.ts", + "scripts/resolvers/review-army.ts", + "scripts/resolvers/review-scope.ts", + "scripts/resolvers/sideways.ts", + "ship/sections/review-army.md", + "test/docsync-command-grammar.test.ts", + "test/fixtures/design-floor-focus-cover-all.json", + "test/fixtures/design-floor-focus-picker.ts", + "test/fixtures/docsync-branch-listing-public.json", + "test/fixtures/docsync-two-attempts-public.json", + "test/fixtures/ship-local-hook-train-replies.json", + "test/fixtures/skill-md-size-allowlist.json", + "test/helpers/carve-guards.ts", + "test/helpers/docsync-contract.ts", + "test/helpers/docsync-interface.ts", + "test/helpers/docsync-observer.ts", + "test/helpers/free-fixtures-data.ts", + "test/helpers/ship-hook-actor.ts", + "test/outside-voice-preflight.test.ts", + "test/plan-design-floor-fixture.test.ts", + "test/plan-review-cases.test.ts", + "test/review-workflow-clarity.test.ts", + "test/ship-document-release-dispatch.test.ts", + "test/ship-hook-actor.test.ts", + "test/skill-e2e-ship-docsync.test.ts" + ], + "deleted": [], + "stamp_only": [ + "package.json" + ], + "scope": { + "SCOPE_FRONTEND": "false", + "SCOPE_BACKEND": "true", + "SCOPE_PROMPTS": "false", + "SCOPE_TESTS": "true", + "SCOPE_DOCS": "true", + "SCOPE_CONFIG": "true", + "SCOPE_MIGRATIONS": "false", + "SCOPE_API": "false", + "SCOPE_AUTH": "false" + }, + "gate_tagged": 0, + "diff_lines": 1491, + "diff_bytes": 205941, + "numbers": { + "publishes": false, + "claims": 0, + "fingerprint": "none" + } +} diff --git a/test/fixtures/review-depth/docs-only.json b/test/fixtures/review-depth/docs-only.json new file mode 100644 index 000000000..36b4abbe5 --- /dev/null +++ b/test/fixtures/review-depth/docs-only.json @@ -0,0 +1,46 @@ +{ + "name": "docs-only", + "commit": "ab9a854", + "paths": [ + "CHANGELOG.md", + "README.md", + "VERSION", + "agents-digest/gstack-AGENTS.md", + "docs/ADDING_A_HOST.md", + "docs/domain-skills.md", + "docs/gbrain-quick-tour.md", + "docs/hosts.md", + "docs/install.md", + "docs/privacy.md", + "docs/qa-without-a-webpage.md", + "docs/standalone-binaries.md", + "docs/troubleshooting.md", + "docs/uninstall.md", + "docs/voice-input.md", + "docs/why-gstack.md", + "package.json" + ], + "deleted": [], + "stamp_only": [ + "package.json" + ], + "scope": { + "SCOPE_FRONTEND": "false", + "SCOPE_BACKEND": "false", + "SCOPE_PROMPTS": "false", + "SCOPE_TESTS": "false", + "SCOPE_DOCS": "true", + "SCOPE_CONFIG": "true", + "SCOPE_MIGRATIONS": "false", + "SCOPE_API": "false", + "SCOPE_AUTH": "false" + }, + "gate_tagged": 0, + "diff_lines": 1329, + "diff_bytes": 153962, + "numbers": { + "publishes": true, + "claims": 34, + "fingerprint": "36e9f8f080a2cdcd" + } +} diff --git a/test/fixtures/skill-md-size-allowlist.json b/test/fixtures/skill-md-size-allowlist.json index e0737588d..686b6b63d 100644 --- a/test/fixtures/skill-md-size-allowlist.json +++ b/test/fixtures/skill-md-size-allowlist.json @@ -22,13 +22,13 @@ "cursor/gstack-plan-design-review": 108900, "cursor/gstack-plan-devex-review": 115228, "cursor/gstack-plan-eng-review": 145996, - "cursor/gstack-review": 121628, + "cursor/gstack-review": 121626, "factory/gstack-autoplan": 110763, "factory/gstack-land-and-deploy": 100105, "factory/gstack-plan-design-review": 111675, "factory/gstack-plan-devex-review": 118591, "factory/gstack-plan-eng-review": 149104, - "factory/gstack-review": 125401, + "factory/gstack-review": 125399, "gbrain/gstack-autoplan": 108892, "gbrain/gstack-plan-eng-review": 128623, "hermes/gstack-autoplan": 108982, @@ -37,18 +37,18 @@ "kiro/gstack-plan-design-review": 108752, "kiro/gstack-plan-devex-review": 115080, "kiro/gstack-plan-eng-review": 145858, - "kiro/gstack-review": 121452, + "kiro/gstack-review": 121450, "openclaw/gstack-autoplan": 108715, "openclaw/gstack-plan-eng-review": 127138, "opencode/gstack-autoplan": 109234, "opencode/gstack-plan-design-review": 110187, "opencode/gstack-plan-devex-review": 115836, "opencode/gstack-plan-eng-review": 147265, - "opencode/gstack-review": 122250, + "opencode/gstack-review": 122248, "slate/gstack-autoplan": 108372, "slate/gstack-plan-design-review": 108826, "slate/gstack-plan-devex-review": 115154, "slate/gstack-plan-eng-review": 145927, - "slate/gstack-review": 121540 + "slate/gstack-review": 121538 } } diff --git a/test/fixtures/workflow-required-steps.json b/test/fixtures/workflow-required-steps.json index 5c65ce590..300f5ad22 100644 --- a/test/fixtures/workflow-required-steps.json +++ b/test/fixtures/workflow-required-steps.json @@ -123,7 +123,7 @@ "## Step 22: Terminal line (every run)" ], "sections/adversarial.md": [ - "## Step 11: Adversarial review (always-on)" + "## Step 11: Adversarial review" ], "sections/changelog.md": [ "## Step 13: CHANGELOG (auto-generate)" diff --git a/test/helpers/autoplan-run-fixture.ts b/test/helpers/autoplan-run-fixture.ts index c602886f6..de053d821 100644 --- a/test/helpers/autoplan-run-fixture.ts +++ b/test/helpers/autoplan-run-fixture.ts @@ -69,12 +69,14 @@ export interface PhaseRunOptions { nativeModel?: string; outsideModel?: string; export function runPhase(out: string, phase: string, o: PhaseRunOptions = {}): { next: SpawnSyncReturns; native: SpawnSyncReturns; outside: SpawnSyncReturns } { const next = autoplan(['next', '--out', out]); if (next.status !== 0) throw new Error(`next failed: ${next.stderr}\n${next.stdout}`); - for (const voice of ['native', 'outside'] as const) { + const outsideDisabled = readRun(out).phases[phase]?.outside?.status === 'disabled'; + for (const voice of outsideDisabled ? ['native'] as const : ['native', 'outside'] as const) { const r = review(path.join(out, `${phase}-${voice}-prompt.md`), o.breakVoice === voice && o.breakKind ? { FIXTURE_BREAK: o.breakKind } : {}); if (r.status !== 0) throw new Error(`fixture reviewer failed: ${r.stderr}`); } const ids = attemptsOf(out, phase); const native = autoplan(['submit', '--out', out, '--phase', phase, '--voice', 'native', '--result', resultOf(out, phase, 'native'), '--model', o.nativeModel ?? 'anthropic/claude-opus-4-7', '--attempt', ids.native, '--runner', 'host-subagent']); + if (outsideDisabled) return { next, native, outside: native }; const outside = autoplan(['submit', '--out', out, '--phase', phase, '--voice', 'outside', '--result', resultOf(out, phase, 'outside'), '--model', o.outsideModel ?? 'openai/gpt-6-astra', '--attempt', ids.outside, '--runner', 'host-subagent', ...(o.reconcile ? ['--reconcile', o.reconcile] : [])]); return { next, native, outside }; } diff --git a/test/helpers/carve-guards.ts b/test/helpers/carve-guards.ts index b7e67febb..2197ad31d 100644 --- a/test/helpers/carve-guards.ts +++ b/test/helpers/carve-guards.ts @@ -161,7 +161,7 @@ export const CARVE_GUARDS: Record = { }, behavioral: 'external', externalTest: 'test/skill-e2e-ship-section-loading.test.ts', - maxSkeletonBytes: 60_913, // GSTA-96 hoist merged with GSTA-92 phase 2 (2026-10-26): the hoisted skeleton (59,706) plus this PR's growth, keeping the hoist's 65-byte headroom; measured 60,848. Before: + v2.0 {{ASIDE_RESEARCH}} (Aside first, WebSearch fallback); measured 78_275. + gate integrity (HONEST_WORK_GATE_INTEGRITY W3): Step 16 stage 4 ZERO-RUN/unknown receipt rows, stage 5 honesty inventory, Step 17 open-gate ready-PR gate; measured 80,540 (2026-10-10). + rule 22 / GBRA-108 (2026-10-25): Step 22's terminal line (gstack-ship-receipt result, the GSTACK_RESULT line a parent greps) and Step 18's unattended routing (an open PR at HEAD gets its body updated, else handoff), offset in part by tightening Steps 14, 16, 18 and 20 prose after the 2026-10-14 tightening left 2 bytes of headroom; measured 81,535. + harness lane E (2026-10-26, issue row 3: four Captains re-ran /ship on identical product code): Step 1's carry check (gstack-ship-receipt carry binds an earlier receipt to HEAD when the product patch-id is unchanged); measured 82,165. + the Review policy paragraph (the blocking floor, pass structure and cap, NO FINDINGS, the review_loop receipt counters, the review_scope knob), stated in the skeleton because gstack is the canonical holder of its review policy (docs/review-policy.md); measured 82,645. + maxSkeletonBytes: 60_960, // GSTA-131 token efficiency (2026-10-27, Garry's D1): Step 1's delta carry (dry run, pregate at HEAD, then carry --pregate) and Step 11.5's fix-check and planned-skip binding, +60 bytes after tightening Step 1 and the Review policy paragraph; measured 60,960 (before: 60,900 on 10b77de4). Before that: GSTA-96 hoist merged with GSTA-92 phase 2 (2026-10-26): the hoisted skeleton (59,706) plus this PR's growth, keeping the hoist's 65-byte headroom; measured 60,848. Before: + v2.0 {{ASIDE_RESEARCH}} (Aside first, WebSearch fallback); measured 78_275. + gate integrity (HONEST_WORK_GATE_INTEGRITY W3): Step 16 stage 4 ZERO-RUN/unknown receipt rows, stage 5 honesty inventory, Step 17 open-gate ready-PR gate; measured 80,540 (2026-10-10). + rule 22 / GBRA-108 (2026-10-25): Step 22's terminal line (gstack-ship-receipt result, the GSTACK_RESULT line a parent greps) and Step 18's unattended routing (an open PR at HEAD gets its body updated, else handoff), offset in part by tightening Steps 14, 16, 18 and 20 prose after the 2026-10-14 tightening left 2 bytes of headroom; measured 81,535. + harness lane E (2026-10-26, issue row 3: four Captains re-ran /ship on identical product code): Step 1's carry check (gstack-ship-receipt carry binds an earlier receipt to HEAD when the product patch-id is unchanged); measured 82,165. + the Review policy paragraph (the blocking floor, pass structure and cap, NO FINDINGS, the review_loop receipt counters, the review_scope knob), stated in the skeleton because gstack is the canonical holder of its review policy (docs/review-policy.md); measured 82,645. minUnionBytes: 181_000, // token-reduction Phases 1-2 (v1.69.x branch); measured union 201,464 mustContain: ['VERSION', 'CHANGELOG', 'review', 'merge', 'PR'], // v1.58.5.0: pre-push-guard install (#2077) stacks on the shared first-run-guidance preamble. diff --git a/test/helpers/free-fixtures-data.ts b/test/helpers/free-fixtures-data.ts index d33731499..25c6fc978 100644 --- a/test/helpers/free-fixtures-data.ts +++ b/test/helpers/free-fixtures-data.ts @@ -189,6 +189,7 @@ export const FREE_FIXTURES: Readonly> = { 'test/fixtures/pty-idle-turn-end.json': ['test/helpers/claude-pty-runner.runners.unit.test.ts', 'test/helpers/claude-pty-runner.screen.unit.test.ts'], 'test/fixtures/redact-diagram/**': ['test/redact-context-exemptions.test.ts'], 'test/fixtures/remote-identity-vectors.json': ['test/bin-context-windows-slug.test.ts', 'test/gstack-slug-parity.test.ts', 'test/remote-identity-vectors.test.ts'], + 'test/fixtures/review-depth/**': ['test/review-dispatch-compare.test.ts'], 'test/fixtures/review-design-lite-reports-ci-36633323521.json': ['test/review-finalization-budget.test.ts'], 'test/fixtures/review-gate-integrity-opus-reports.json': ['test/gate-integrity-templates.test.ts'], 'test/fixtures/session-update-heartbeat.sh': ['test/session-update-heartbeat.test.ts'], diff --git a/test/outside-voice-preflight.test.ts b/test/outside-voice-preflight.test.ts index fc81af532..3420603f1 100644 --- a/test/outside-voice-preflight.test.ts +++ b/test/outside-voice-preflight.test.ts @@ -23,7 +23,7 @@ test('adversarial outside failures retain the required native pass without dupli expect(preflight).toMatch(/(?:do not dispatch a duplicate|without duplicating it)/); expect(preflight).not.toMatch(/fall(?:ing)? back to (?:a|the) .*subagent/i); const output = generateAdversarialStep(ctx); - expect(output).toContain('adversarial subagent (always runs)'); + expect(output).toContain(skillName === 'ship' ? 'adversarial subagent (on `DISPATCH: adversarial-native`)' : 'adversarial subagent (always runs)'); expectMentions(output, [['do not', 'dispatch', 'retain']], 'output'); expect(output.match(/Retain the required native pass without duplicating it; it cannot complete outside coverage\./g)).toHaveLength(2); expect(output).not.toContain("Use the caller's fallback"); diff --git a/test/review-depth.test.ts b/test/review-depth.test.ts new file mode 100644 index 000000000..d8c0ab4fa --- /dev/null +++ b/test/review-depth.test.ts @@ -0,0 +1,169 @@ +import { describe, test, expect } from 'bun:test'; +import * as path from 'node:path'; +import { classifyDiff, classifyPath, nextStep, planReview, scanNumbers, FULL_PASS_CAP, type PlanInput } from '../lib/review-depth'; + +const ROOT = path.join(import.meta.dir, '..'); +const none = scanNumbers({}); +const base = (o: Partial): PlanInput => ({ paths: ['src/app.ts'], scope: { SCOPE_BACKEND: 'true', SCOPE_STATE_MACHINE: 'false' }, gateTagged: 0, diffLines: 120, history: { fullPasses: 0, last: 'none' }, numbers: none, ...o }); +const roles = (p: ReturnType) => p.dispatches.map(d => `${d.role}:${d.name}:${d.effort}`); + +describe('classifyPath: agent-read markdown is code', () => { + test.each([ + ['README.md', 'docs'], ['CHANGELOG.md', 'docs'], ['docs/howto-x.md', 'docs'], ['TODOS.md', 'docs'], + ['ship/SKILL.md.tmpl', 'code'], ['review/SKILL.md', 'code'], ['ship/sections/review-army.md', 'code'], ['scripts/resolvers/review-army.ts', 'code'], + ['docs/review-policy.md', 'code'], ['autoplan/DRIVER.md.tmpl', 'code'], ['captain/templates/brief.md', 'code'], ['AGENTS.md', 'code'], ['review/specialists/security.md', 'code'], + ['test/review-depth.test.ts', 'tests'], ['src/a.test.ts', 'tests'], ['pkg/x_test.go', 'tests'], ['test/fixtures/a.json', 'tests'], + ['lib/review-depth.ts', 'code'], ['package.json', 'code'], + ])('%s -> %s', (p, c) => expect(classifyPath(p)).toBe(c as any)); +}); + +describe('classifyDiff', () => { + test('docs only stays docs', () => expect(classifyDiff({ paths: ['README.md', 'docs/a.md'], gateTagged: null }).class).toBe('docs')); + test('a skill-template-only diff is code, not docs', () => expect(classifyDiff({ paths: ['ship/SKILL.md.tmpl'], gateTagged: 0 }).class).toBe('code')); + test('mixed diff takes the most restrictive class', () => { + expect(classifyDiff({ paths: ['README.md', 'test/a.test.ts'], gateTagged: 0 }).class).toBe('tests'); + expect(classifyDiff({ paths: ['README.md', 'lib/a.ts'], gateTagged: 0 }).class).toBe('code'); + }); + test('a scope error fails closed to code', () => expect(classifyDiff({ paths: ['README.md'], scope: { SCOPE_ERROR: 'unmatched' }, gateTagged: 0 }).class).toBe('code')); + test('code with the gate scan unavailable fails closed to gate', () => expect(classifyDiff({ paths: ['lib/a.ts'], gateTagged: null }).class).toBe('gate')); + test.each([ + [{ scope: { SCOPE_AUTH: 'true' } }], [{ scope: { SCOPE_MIGRATIONS: 'true' } }], [{ gateTagged: 2 }], + [{ paths: ['lib/a.ts', '.github/workflows/ci.yml'] }], [{ paths: ['lib/a.ts', 'lib/credentials.ts'] }], [{ deleted: ['lib/old.ts'] }], + ])('gate signal %j raises code to gate', (o: any) => expect(classifyDiff({ paths: ['lib/a.ts'], gateTagged: 0, ...o }).class).toBe('gate')); + test('docs never become gate from code-only signals', () => expect(classifyDiff({ paths: ['README.md'], gateTagged: null, deleted: ['docs/old.md'] }).class).toBe('docs')); +}); + +describe('nextStep: the transition table', () => { + test('pass 1 is full', () => expect(nextStep({ fullPasses: 0, last: 'none' }).step).toBe('full')); + test('no edits is done', () => expect(nextStep({ fullPasses: 1, last: 'full' }).step).toBe('done')); + test('a fixed blocker earns full pass 2', () => expect(nextStep({ fullPasses: 1, last: 'full', fixedBlocking: true }).step).toBe('full')); + test('below-floor fixes earn one fix-check', () => expect(nextStep({ fullPasses: 1, last: 'full', fixedBelow: true }).step).toBe('fix-check')); + test('a fix touching gate code earns a full pass instead of a fix-check', () => expect(nextStep({ fullPasses: 1, last: 'full', fixedBelow: true, touchedGate: true }).step).toBe('full')); + test('at the full-pass cap a fixed blocker gets a fix-check', () => expect(nextStep({ fullPasses: FULL_PASS_CAP, last: 'full', fixedBlocking: true }).step).toBe('fix-check')); + test('at the cap a gate-touching fix still gets a fix-check, never a third full pass', () => expect(nextStep({ fullPasses: FULL_PASS_CAP, last: 'full', fixedBlocking: true, touchedGate: true }).step).toBe('fix-check')); + test('a blocker open at the cap stops', () => expect(nextStep({ fullPasses: FULL_PASS_CAP, last: 'full', blockingOpen: true }).step).toBe('stop')); + test('a blocker open with no fix stops', () => expect(nextStep({ fullPasses: 1, last: 'full', blockingOpen: true }).step).toBe('stop')); + test('a fix-check that finds a blocker stops, never a third full pass', () => { + expect(nextStep({ fullPasses: 1, last: 'fix-check', blockingOpen: true }).step).toBe('stop'); + expect(nextStep({ fullPasses: 2, last: 'fix-check', fixedBlocking: true }).step).toBe('stop'); + }); + test('a clean fix-check is done', () => expect(nextStep({ fullPasses: 1, last: 'fix-check' }).step).toBe('done')); +}); + +describe('scanNumbers', () => { + test('finds claims and fingerprints them', () => { + const a = scanNumbers({ 'docs/bench.md': ['Median dropped 48% to 1.2s', 'no numbers here'] }); + expect(a).toMatchObject({ publishes: true, claims: 1 }); + expect(scanNumbers({ 'docs/bench.md': ['Median dropped 47% to 1.2s'] }).fingerprint).not.toBe(a.fingerprint); + }); + test('versions, dates, issue and step numbers and plain prose are not claims', () => expect(scanNumbers({ 'CHANGELOG.md': ['## [1.91.283.0] - 2026-10-27', 'Fixes the ship loop (#3516), see Step 9.4 and rule 23.'] }).publishes).toBe(false)); + test.each([ + ['the smallest two-sided p with 16 clusters is 2 x 0.5^6 = 0.031'], + ['the miss split is 10/52'], ['p falls to 3.05e-5'], ['median 1.4 passes per PR'], ['Hot-memory hits: 10 of 62'], + ])('a bare numeric claim counts: %s', line => expect(scanNumbers({ 'docs/comparing-runs.md': [line] }).publishes).toBe(true)); +}); + +describe('planReview', () => { + test('docs class: core only, no outside voice', () => { + const p = planReview(base({ paths: ['README.md'], scope: {}, gateTagged: null, diffLines: 300 })); + expect(p.class).toBe('docs'); + expect(p.dispatches).toEqual([]); + }); + test('docs class with numeric claims keeps the outside voice', () => { + const p = planReview(base({ paths: ['docs/bench.md'], scope: {}, numbers: scanNumbers({ 'docs/bench.md': ['3x faster'] }) })); + expect(roles(p)).toEqual(['outside-voice:outside-voice:host']); + }); + test('tests class: testing specialist only', () => { + const p = planReview(base({ paths: ['test/a.test.ts'], scope: { SCOPE_TESTS: 'true' } })); + expect(roles(p)).toEqual(['specialist:testing:full']); + }); + test('code class pass 1: every reviewer at full effort, the outside voice at its own', () => { + const p = planReview(base({})); + expect(roles(p)).toEqual(['specialist:testing:full', 'specialist:maintainability:full', 'specialist:security:full', 'specialist:performance:full', 'specialist:simplification:full', 'adversarial-native:adversarial:full', 'outside-voice:outside-voice:host']); + }); + test('gate class forces red team and ignores hit-rate gating', () => { + const p = planReview(base({ gateTagged: 1, gated: ['security'] })); + expect(roles(p)).toContain('red-team:red-team:full'); + expect(roles(p)).toContain('specialist:security:full'); + }); + test('code class honours hit-rate gating and small-diff skip', () => { + expect(roles(planReview(base({ gated: ['simplification'] })))).not.toContain('specialist:simplification:full'); + expect(planReview(base({ diffLines: 20 })).dispatches.filter(d => d.role === 'specialist')).toEqual([]); + }); + test('pass 2 reruns the army on the range but not the adversarial or outside voice once Step 11 ran', () => { + const p = planReview(base({ adversarialRan: true, history: { fullPasses: 1, last: 'full', fixedBlocking: true } })); + expect(p.next.step).toBe('full'); + expect(roles(p).some(r => r.startsWith('outside-voice') || r.startsWith('adversarial'))).toBe(false); + }); + test('a repeat that became code class keeps the adversarial pass Step 11 has not run yet', () => { + const p = planReview(base({ history: { fullPasses: 1, last: 'full', fixedBlocking: true } })); + expect(roles(p)).toContain('adversarial-native:adversarial:full'); + expect(planReview(base({ paths: ['docs/a.md'], history: { fullPasses: 1, last: 'full', fixedBlocking: true } })).dispatches.some(d => d.role === 'adversarial-native')).toBe(false); + }); + test('a fix-check is one full-effort dispatch', () => { + expect(roles(planReview(base({ history: { fullPasses: 1, last: 'full', fixedBelow: true } })))).toEqual(['fix-check:fix-check:full']); + }); + test('changed numeric claims rerun only the outside voice', () => { + const numbers = scanNumbers({ 'docs/bench.md': ['3x faster'] }); + const p = planReview(base({ numbers, reviewedNumbers: 'old', history: { fullPasses: 1, last: 'full', fixedBelow: true } })); + expect(roles(p)).toEqual(['fix-check:fix-check:full', 'outside-voice:outside-voice (numbers changed):host']); + }); + test('no reviewer runs below full effort (GSTA-131 eval: a light fix-check missed a fail-open a full one caught)', () => { + for (const h of [{ fullPasses: 0, last: 'none' }, { fullPasses: 1, last: 'full', fixedBelow: true }, { fullPasses: 1, last: 'full', fixedBlocking: true }] as const) + expect(planReview(base({ gateTagged: 1, history: h })).dispatches.filter(d => d.role !== 'outside-voice').every(d => d.effort === 'full')).toBe(true); + }); + test('done dispatches nothing', () => expect(planReview(base({ history: { fullPasses: 1, last: 'full' } })).dispatches).toEqual([])); +}); + +describe('GSTA-131 pass-1 review repairs', () => { + test('rule docs agents follow are code, and markdown inside a skill directory is code when the root is known', () => { + for (const p of ['docs/captain.md', 'docs/unattended.md', 'docs/pregate.md']) expect(classifyPath(p)).toBe('code'); + expect(classifyPath('autoplan/notes.md')).toBe('docs'); + expect(classifyPath('autoplan/notes.md', ROOT)).toBe('code'); + expect(classifyPath('docs/howto-x.md', ROOT)).toBe('docs'); + }); + test.each([ + [{ gateTagged: 3 }], [{ gateTagged: null }], [{ deleted: ['test/old.test.ts'] }], + ])('a tests-only diff with gate signal %j is gate class', (o: any) => expect(classifyDiff({ paths: ['test/a.test.ts'], gateTagged: 0, ...o }).class).toBe('gate')); + test('a version-only package.json stays docs; the same path with any other edit is code', () => { + expect(classifyDiff({ paths: ['package.json', 'CHANGELOG.md'], stampOnly: ['package.json'], gateTagged: 0 }).class).toBe('docs'); + expect(classifyDiff({ paths: ['package.json', 'CHANGELOG.md'], gateTagged: 0 }).class).toBe('code'); + }); + test('an open blocker never routes to a fix-check, even when other fixes landed', () => { + expect(nextStep({ fullPasses: 1, last: 'full', fixedBelow: true, blockingOpen: true }).step).toBe('stop'); + expect(nextStep({ fullPasses: 1, last: 'full', fixedBlocking: true, blockingOpen: true }).step).toBe('stop'); + }); + test('code and test source publish no numbers; prose and data files do', () => { + expect(scanNumbers({ 'test/a.test.ts': ['const n = 3;', 'for (let i = 0; i < 10; i++)', 'expect(x).toBe(0.5)'] }).publishes).toBe(false); + expect(scanNumbers({ 'test/fixtures/bench.json': ['{ "ratio": 0.74 }'] }).publishes).toBe(true); + }); + test('--all-specialists forces every specialist; a code diff past 200 lines or with a CRITICAL adds the Red Team', () => { + expect(planReview(base({ force: 'all' })).dispatches.filter(d => d.role === 'specialist').length).toBe(9); + const has = (o: Partial) => roles(planReview(base(o))).some(r => r.startsWith('red-team')); + expect(has({ diffLines: 120 })).toBe(false); + expect(has({ diffLines: 201 })).toBe(true); + expect(has({ diffLines: 120, criticalFound: true })).toBe(true); + }); +}); + +describe('GSTA-131 coverage: Red Team and specialist arms', () => { + const roles = (p: ReturnType) => p.dispatches.map(d => `${d.role}:${d.name}`); + test('Red Team runs on code past 200 lines or after a CRITICAL, and on every gate diff', () => { + expect(roles(planReview(base({ diffLines: 120 })))).not.toContain('red-team:red-team'); + expect(roles(planReview(base({ diffLines: 201 })))).toContain('red-team:red-team'); + expect(roles(planReview(base({ diffLines: 120, criticalFound: true })))).toContain('red-team:red-team'); + expect(roles(planReview(base({ diffLines: 20, gateTagged: 1 })))).toContain('red-team:red-team'); + }); + test('scope arms select their specialist, state machines default on when scope is unknown, and --force all reaches docs class', () => { + const picked = (scope: Record) => roles(planReview(base({ scope: { SCOPE_STATE_MACHINE: 'false', ...scope } }))); + expect(picked({ SCOPE_MIGRATIONS: 'true' })).toContain('specialist:data-migration'); + expect(picked({ SCOPE_API: 'true' })).toContain('specialist:api-contract'); + expect(picked({ SCOPE_FRONTEND: 'true' })).toContain('specialist:design'); + expect(roles(planReview(base({ scope: {} })))).toContain('specialist:state-machines'); + expect(roles(planReview(base({ scope: { SCOPE_STATE_MACHINE: 'false' } })))).not.toContain('specialist:state-machines'); + const docsAll = planReview(base({ paths: ['docs/a.md'], force: 'all' })); + expect(docsAll.class).toBe('docs'); + expect(docsAll.dispatches.filter(d => d.role === 'specialist').length).toBeGreaterThan(5); + expect(roles(planReview(base({ paths: ['docs/a.md'], force: ['security'] })))).toEqual(['specialist:security']); + }); +}); diff --git a/test/review-dispatch-compare.test.ts b/test/review-dispatch-compare.test.ts new file mode 100644 index 000000000..804d9e0cf --- /dev/null +++ b/test/review-dispatch-compare.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { runScriptSync } from '../lib/script-invocation'; + +const ROOT = path.join(import.meta.dir, '..'); +const SCRIPT = path.join(ROOT, 'scripts', 'review-dispatch-compare.ts'); +const FIXTURES = ['test/fixtures/review-depth/docs-only.json', 'test/fixtures/review-depth/code.json'].map(f => path.join(ROOT, f)); +const run = (args: string[]) => { + const r = runScriptSync(SCRIPT, [...FIXTURES.flatMap(f => ['--fixture', f]), ...args], { cwd: ROOT, encoding: 'utf8', timeout: 120_000 }); + expect(r.status).toBe(0); + return r.stdout as string; +}; + +describe('scripts/review-dispatch-compare.ts', () => { + test('the new policy never dispatches more reviewers than the legacy one, and a docs diff costs under a third', () => { + const out = JSON.parse(run(['--json'])); + expect(out.tokens_source).toBe('DEFAULT'); + expect(out.ship.length).toBe(10); + for (const row of out.ship) { + expect(row.new_dispatches).toBeLessThanOrEqual(row.legacy_dispatches); + expect(row.new_tokens).toBeLessThanOrEqual(row.legacy_tokens); + } + for (const row of out.ship.filter((r: any) => r.class === 'docs')) expect(row.ratio).toBeLessThan(0.34); + for (const f of out.autoplan) for (const r of f.runs) expect(r.next).toBeLessThanOrEqual(r.legacy); + }); + + test('--tokens replaces the default output estimates and the table says which it used', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'dispatch-compare-')); + try { + const tok = path.join(dir, 'measured.json'); + fs.writeFileSync(tok, JSON.stringify({ specialist: { full: 6929, light: 5446 } })); + const table = run(['--tokens', tok]); + expect(table).toContain(`TOKENS: measured (${tok})`); + expect(table).toMatch(/^SHIP_TOKEN_RATIO: median=0\.\d+ min=0\.\d+ max=\d\.\d+ \(n=10\)$/m); + expect(table).toMatch(/^SHIP_TOKEN_RATIO_BY_CLASS: docs median=0\.\d+ \(n=5\); code median=0\.\d+ \(n=5\)$/m); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/test/review-dispatch-counters.test.ts b/test/review-dispatch-counters.test.ts new file mode 100644 index 000000000..dbd34eab2 --- /dev/null +++ b/test/review-dispatch-counters.test.ts @@ -0,0 +1,48 @@ +import { describe, test, expect } from 'bun:test'; +import { summarizeReviewLoop, reviewLoopLine, FIX_CHECK_SKILL } from '../lib/review-scope'; +import { parseReviewLoop, receiptSummaryLine } from '../lib/ship-receipt'; + +const since = '2026-10-27T10:00:00Z'; +const row = (o: Record) => ({ via: 'ship', review_scope: 'frozen', findings: [], ...o }); + +describe('review loop dispatch counters', () => { + test('fix-check rows count as fix_checks, never passes, and dispatch events add up by role', () => { + const rows = [ + row({ skill: 'review', timestamp: '2026-10-27T10:01:00Z', findings: [{ fingerprint: 'a:1:x', action: 'auto-fixed' }], + dispatches: [{ role: 'specialist', prompt_bytes: 4000, diff_bytes: 8000 }, { role: 'specialist', prompt_bytes: 4000, diff_bytes: 8000 }, { role: 'red-team', prompt_bytes: 2000 }] }), + row({ skill: 'adversarial-review', timestamp: '2026-10-27T10:05:00Z', dispatches: [{ role: 'adversarial-native', prompt_bytes: 1000 }, { role: 'outside-voice', status: 'unknown' }] }), + row({ skill: FIX_CHECK_SKILL, timestamp: '2026-10-27T10:10:00Z', dispatches: [{ role: 'fix-check', prompt_bytes: 400, diff_bytes: 600 }] }), + row({ skill: 'review', timestamp: '2026-10-27T09:00:00Z', dispatches: [{ role: 'specialist', prompt_bytes: 99999 }] }), + ]; + const s = summarizeReviewLoop(rows, since); + expect(s).toMatchObject({ passes: 1, fixed: 1, fix_checks: 1, dispatches: { specialist: 2, 'red-team': 1, 'adversarial-native': 1, 'outside-voice': 1, 'fix-check': 1 }, est_input_tokens: Math.round(28000 / 4) }); + expect(reviewLoopLine(s)).toBe('REVIEW_LOOP: passes=1 blocking_found=0 deferred=0 noted=0 fixed=1 scope=frozen fix_checks=1 dispatches=6 est_input_tokens=7000'); + }); + + test('older rows without dispatch events fall back to the specialist stats and print the old line plus a count', () => { + const rows = [row({ skill: 'review', timestamp: '2026-10-27T10:01:00Z', specialists: { testing: { dispatched: true }, security: { dispatched: false, reason: 'scope' }, 'red-team': { dispatched: true } } })]; + const s = summarizeReviewLoop(rows, since); + expect(s.dispatches).toEqual({ specialist: 1, 'red-team': 1 }); + expect(s.est_input_tokens).toBeUndefined(); + expect(reviewLoopLine(s)).toBe('REVIEW_LOOP: passes=1 blocking_found=0 deferred=0 noted=0 fixed=0 scope=frozen dispatches=2'); + }); + + test('rows with neither keep the historical line byte for byte', () => { + const s = summarizeReviewLoop([row({ skill: 'review', timestamp: '2026-10-27T10:01:00Z' })], since); + expect(reviewLoopLine(s)).toBe('REVIEW_LOOP: passes=1 blocking_found=0 deferred=0 noted=0 fixed=0 scope=frozen'); + }); +}); + +describe('receipt round trip', () => { + test('parseReviewLoop keeps the new counters and drops malformed ones; an old receipt parses unchanged', () => { + expect(parseReviewLoop({ passes: 1, blocking_found: 0, deferred: 0 })).toEqual({ passes: 1, blocking_found: 0, deferred: 0 }); + expect(parseReviewLoop({ passes: 1, blocking_found: 0, deferred: 0, fix_checks: 1, est_input_tokens: 7000, dispatches: { specialist: 2, 'Bad Key': 3, 'red-team': -1, 'fix-check': 1 } })) + .toEqual({ passes: 1, blocking_found: 0, deferred: 0, fix_checks: 1, est_input_tokens: 7000, dispatches: { specialist: 2, 'fix-check': 1 } }); + }); + test('the SHIP_RECEIPT line carries fix_checks and the dispatch total only when recorded', () => { + const r = { schema: 1, skill: 'ship', head: 'a'.repeat(40), base: 'main', pr: 7 } as any; + expect(receiptSummaryLine({ ...r, review_loop: { passes: 1, blocking_found: 0, deferred: 0 } })).toContain('passes=1 blocking_found=0 deferred=0'); + expect(receiptSummaryLine({ ...r, review_loop: { passes: 1, blocking_found: 0, deferred: 0 } })).not.toContain('dispatches='); + expect(receiptSummaryLine({ ...r, review_loop: { passes: 1, blocking_found: 0, deferred: 0, fix_checks: 1, dispatches: { specialist: 2, 'fix-check': 1 } } })).toContain('deferred=0 fix_checks=1 dispatches=3'); + }); +}); diff --git a/test/review-log-findings.test.ts b/test/review-log-findings.test.ts index 361fdd824..c463521c3 100644 --- a/test/review-log-findings.test.ts +++ b/test/review-log-findings.test.ts @@ -178,6 +178,19 @@ describe('lib: deriveFromFindings, bindReview order and reviewFreshness', () => expect(fs.existsSync(path.join(dir, `${token}.json`))).toBe(false); }); + test('a review-fix-check record binds like a pass, so Step 11.5 can read its CURRENT freshness', () => { + const dir = path.join(home, 'starts', '.review-starts'); + fs.mkdirSync(dir, { recursive: true }); + const token = '66666666-2222-3333-4444-555555555555'; + const w = 'c'.repeat(64); + fs.writeFileSync(path.join(dir, `${token}.json`), JSON.stringify({ skill: 'review-fix-check', repo: '/r', branch: 'b', wtree: w, started_at: 't' })); + const env = { GSTACK_REVIEW_DIR: path.join(home, 'starts'), GSTACK_REVIEW_REPO: '/r', GSTACK_REVIEW_BRANCH: 'b', GSTACK_STAMP_WTREE: w }; + const bound = bindReview({ skill: 'review-fix-check', status: 'clean', completed: true, converged: true }, token, env); + expect(bound.review_binding).toMatchObject({ state: 'verified', start_wtree: w, end_wtree: w }); + expect(reviewFreshness(bound, w)?.status).toBe('CURRENT'); + expect(reviewFreshness(bound, 'd'.repeat(64))?.status).toBe('STALE'); + }); + test('reviewFreshness reads findings_open: resolved criticals are CURRENT, claimed issues_found stays conservative', () => { const w = 'b'.repeat(64); const base = { skill: 'review', completed: true, converged: true, wtree: w, review_binding: { state: 'verified', start_wtree: w, end_wtree: w } }; diff --git a/test/review-plan-cli.test.ts b/test/review-plan-cli.test.ts new file mode 100644 index 000000000..c8aedb90e --- /dev/null +++ b/test/review-plan-cli.test.ts @@ -0,0 +1,111 @@ +import { afterEach, describe, expect, test } from 'bun:test'; +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; + +const BIN = path.join(import.meta.dir, '..', 'bin', 'gstack-review-plan'); +const dirs: string[] = []; +afterEach(() => { while (dirs.length) fs.rmSync(dirs.pop()!, { recursive: true, force: true }); }); + +function repo(files: Record): { dir: string; state: string } { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'review-plan-')); + const state = fs.mkdtempSync(path.join(os.tmpdir(), 'review-plan-state-')); + dirs.push(dir, state); + const git = (...a: string[]) => spawnSync('git', a, { cwd: dir, encoding: 'utf8', timeout: 10_000 }); + git('init', '-q', '-b', 'main'); git('config', 'user.email', 't@t'); git('config', 'user.name', 't'); git('config', 'commit.gpgsign', 'false'); + fs.writeFileSync(path.join(dir, 'README.md'), '# x\n'); git('add', '.'); git('commit', '-qm', 'base'); + git('checkout', '-qb', 'feat'); + for (const [p, text] of Object.entries(files)) { fs.mkdirSync(path.dirname(path.join(dir, p)), { recursive: true }); fs.writeFileSync(path.join(dir, p), text); } + git('add', '.'); git('commit', '-qm', 'feat'); + return { dir, state }; +} +const run = (r: { dir: string; state: string }, args: string[]) => spawnSync(BIN, ['--base', 'main', '--gate-tagged', '0', ...args], { cwd: r.dir, encoding: 'utf8', timeout: 60_000, env: { ...process.env, GSTACK_STATE_ROOT: r.state } }); + +describe('gstack-review-plan', () => { + test('a docs-only diff plans no dispatch and says why', () => { + const r = run(repo({ 'docs/howto.md': '# How to\nStep one.\n' }), []); + expect(r.status).toBe(0); + expect(r.stdout).toContain('REVIEW_PLAN: class=docs pass=1 next=full dispatches=0 numbers=no'); + expect(r.stdout).toContain('SKIP: outside-voice (docs class, no numeric claims)'); + }); + test('a docs diff that publishes numbers keeps the outside voice', () => { + const r = run(repo({ 'docs/bench.md': 'Median latency fell 42% to 180 ms.\n' }), []); + expect(r.stdout).toContain('DISPATCH: outside-voice outside-voice effort=host'); + }); + test('--json carries the class, the next step, every dispatch with its effort and the audit effort; --no-tiers renders audits at host effort', () => { + const r = repo({ 'src/a.ts': Array.from({ length: 80 }, (_, i) => `export const v${i} = ${i};`).join('\n') + '\n' }); + const plan = JSON.parse(run(r, ['--json']).stdout); + expect(plan).toMatchObject({ class: 'code', next: { step: 'full' }, pass: 0 }); + expect(plan.dispatches.find((d: any) => d.role === 'specialist').effort).toBe('full'); + expect(plan.audit_effort).toBe('light'); + expect(JSON.parse(run(r, ['--json', '--no-tiers']).stdout).audit_effort).toBe('host'); + }); + test('a below-floor fix after pass 1 plans one fix-check', () => { + const r = run(repo({ 'src/a.ts': 'export const a = 1;\n' }), ['--pass', '1', '--last', 'full', '--fixed-below']); + expect(r.stdout).toContain('next=fix-check dispatches=1'); + expect(r.stdout).toContain('DISPATCH: fix-check fix-check effort=full'); + }); + 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; }; + const files = ['--fix-list', w('fix.txt', 'lib/a.ts:3 guard now refuses unknown'), '--hunks', w('hunks.diff', '+if (s !== "MERGED") return false;'), '--tests', w('tests.txt', '12 pass 0 fail')]; + const ok = spawnSync(BIN, ['--fix-check-prompt', ...files], { cwd: r.dir, encoding: 'utf8', timeout: 60_000 }); + expect(ok.status).toBe(0); + expect(ok.stdout).toContain('You are the fix-check reviewer'); + expect(ok.stdout).toContain('lib/a.ts:3 guard now refuses unknown'); + expect(ok.stdout).toContain('(none found)'); + const missing = spawnSync(BIN, ['--fix-check-prompt', '--fix-list', path.join(r.state, 'nope.txt'), '--hunks', files[3]!, '--tests', files[5]!], { cwd: r.dir, encoding: 'utf8', timeout: 60_000 }); + expect(missing.status).toBe(2); + }); + test('untracked files count, a version-only package.json bump stays docs, and review_effort_tiers off renders audits at host effort', () => { + const r = repo({ 'docs/a.md': '# a\n' }); + const git = (...a: string[]) => spawnSync('git', a, { cwd: r.dir, encoding: 'utf8', timeout: 10_000 }); + git('checkout', '-q', 'main'); + fs.writeFileSync(path.join(r.dir, 'package.json'), '{\n "name": "x",\n "version": "1.0.0"\n}\n'); + git('add', '.'); git('commit', '-qm', 'package'); + git('checkout', '-q', 'feat'); git('reset', '-q', '--hard', 'main'); + fs.writeFileSync(path.join(r.dir, 'package.json'), '{\n "name": "x",\n "version": "1.0.1"\n}\n'); + git('commit', '-qam', 'stamp'); + const stamp = JSON.parse(run(r, ['--json']).stdout); + expect(stamp.stamp_only).toContain('package.json'); + expect(stamp.class).toBe('docs'); + fs.mkdirSync(path.join(r.dir, 'lib'), { recursive: true }); + fs.writeFileSync(path.join(r.dir, 'lib', 'new.ts'), 'export const a = 1;\n'); + const withUntracked = JSON.parse(run(r, ['--json']).stdout); + expect(withUntracked.paths).toContain('lib/new.ts'); + expect(withUntracked.class).toBe('code'); + const cfg = spawnSync(path.join(import.meta.dir, '..', 'bin', 'gstack-config'), ['set', 'review_effort_tiers', 'off'], { cwd: r.dir, encoding: 'utf8', timeout: 30_000, env: { ...process.env, GSTACK_STATE_ROOT: r.state } }); + expect(cfg.status).toBe(0); + expect(JSON.parse(run(r, ['--json']).stdout).audit_effort).toBe('host'); + const bad = spawnSync(path.join(import.meta.dir, '..', 'bin', 'gstack-config'), ['set', 'review_effort_tiers', 'maybe'], { cwd: r.dir, encoding: 'utf8', timeout: 30_000, env: { ...process.env, GSTACK_STATE_ROOT: r.state } }); + expect(bad.status).not.toBe(0); + expect(bad.stderr).toContain("review_effort_tiers 'maybe' not recognized"); + }); + test('without --gate-tagged the plan runs the gate scan itself', () => { + const r = repo({ 'lib/a.ts': 'export const a = 1;\n' }); + const res = spawnSync(BIN, ['--base', 'main', '--json'], { cwd: r.dir, encoding: 'utf8', timeout: 120_000, env: { ...process.env, GSTACK_STATE_ROOT: r.state } }); + expect(res.status, res.stderr).toBe(0); + expect(typeof JSON.parse(res.stdout).gate_tagged).toBe('number'); + }); + test('usage errors exit 2', () => { + expect(spawnSync(BIN, [], { encoding: 'utf8', timeout: 10_000 }).status).toBe(2); + expect(run(repo({ 'a.md': 'x' }), ['--last', 'later']).status).toBe(2); + }); + test('scratch under .gstack/tmp/ is ignored, a subdirectory run plans the whole repository, and --adversarial-ran drops the repeat adversarial pass', () => { + const r = repo({ 'docs/a.md': '# a\n' }); + fs.mkdirSync(path.join(r.dir, '.gstack', 'tmp'), { recursive: true }); + fs.writeFileSync(path.join(r.dir, '.gstack', 'tmp', 'fix-check-list.ts'), 'export const x = 1;\n'); + const plan = JSON.parse(run(r, ['--json']).stdout); + expect(plan.class).toBe('docs'); + expect(plan.paths).toEqual(['docs/a.md']); + const sub = spawnSync(BIN, ['--base', 'main', '--gate-tagged', '0', '--json'], { cwd: path.join(r.dir, 'docs'), encoding: 'utf8', timeout: 60_000, env: { ...process.env, GSTACK_STATE_ROOT: r.state } }); + expect(JSON.parse(sub.stdout).paths).toEqual(['docs/a.md']); + const code = repo({ 'src/a.ts': Array.from({ length: 80 }, (_, i) => `export const v${i} = ${i};`).join('\n') + '\n' }); + const repeat = ['--pass', '1', '--last', 'full', '--fixed-blocking']; + expect(run(code, repeat).stdout).toContain('DISPATCH: adversarial-native adversarial effort=full'); + const ran = run(code, [...repeat, '--adversarial-ran']).stdout; + expect(ran).not.toContain('DISPATCH: adversarial-native'); + expect(ran).toContain('SKIP: adversarial-native and outside-voice (run once per invocation)'); + }); +}); diff --git a/test/ship-control-flow.test.ts b/test/ship-control-flow.test.ts index 10988d45c..9c0048eec 100644 --- a/test/ship-control-flow.test.ts +++ b/test/ship-control-flow.test.ts @@ -40,6 +40,27 @@ describe('ship source controller', () => { expect(freshness).not.toContain('**No attempt remains or no repair is available:**'); }); + test('the review plan drives the repeat, the fix-check binds under its own skill, and Step 11.5 accepts a fix-check or a planned adversarial skip', () => { + const decide = review.slice(review.indexOf('### Decide whether to repeat Step 9'), review.indexOf('**Required-probe parent gate:**')); + for (const flag of ['--fixed-blocking', '--fixed-below', '--blocking-open', '--touched-gate', '--critical-found', '--adversarial-ran', '--reviewed-numbers ']) expect(decide).toContain(flag); + expect(decide).toContain('its `NEXT:` alone picks the next decision: `stop` 2, `full` 3, `fix-check` 3.5, `done` 4.'); + expect(decide).toContain('`gstack-review-log --start review-fix-check` token'); + expect(decide).toContain('`"skill":"review-fix-check"` with `--finish `'); + expect(compact(read('ship/sections/adversarial.md.tmpl'))).toContain("Step 9's last full-pass plan governs this step"); + const bind = compact(entry.slice(entry.indexOf('## Step 11.5:'), entry.indexOf('## Step 12:'))); + expect(bind).toContain("last 3.5 `review-fix-check`"); + expect(bind).toContain('none after `ADVERSARIAL: skipped`'); + expect(bind).toContain('`review_freshness` `CURRENT`'); + }); + + test('Step 1 carries a receipt only after a dry run and a pregate at HEAD', () => { + const carry = compact(entry.slice(entry.indexOf('4. **Carry check.**'), entry.indexOf('5. Display historical readiness'))); + const order = ['carry --pr --base --dry-run', '`CARRY: would carry`', 'skip review', '`gstack-pregate --allow-repo-commands`', 'carry without `--dry-run`', '`--pregate .gstack/tmp/pregate.json`', '`CARRY: carried` binds HEAD', 'A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16.']; + const at = order.map(t => carry.indexOf(t)); + expect(at.every(i => i >= 0)).toBe(true); + expect(at).toEqual([...at].sort((a, b) => a - b)); + }); + test('repairs use one ordered work list without a return stack', () => { const text = compact(controller); expect(compact(entry)).toContain('**Next steps:**'); diff --git a/test/ship-receipt-carry-pregate.test.ts b/test/ship-receipt-carry-pregate.test.ts new file mode 100644 index 000000000..7e0bbf81f --- /dev/null +++ b/test/ship-receipt-carry-pregate.test.ts @@ -0,0 +1,77 @@ +/** + * gstack-ship-receipt carry with a pregate: eligibility is not publication. A receipt that recorded a + * pregate rebinds to a new head only with a pregate current for that head (docs/ship-policy.md#receipt-carry). + */ +import { afterEach, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { EXIT } from '../lib/headless-artifacts'; +import { extractReceipt } from '../lib/ship-receipt'; +import { cleanup, commit, makeFixtureRepo, runBin, write, type FixtureRepo } from './helpers/pregate-fixture'; + +const repos: FixtureRepo[] = []; +afterEach(() => { while (repos.length) cleanup(repos.pop()!); }); +const pregate = (repo: FixtureRepo) => runBin(repo, 'gstack-pregate', ['--base', 'main', '--allow-repo-commands']); +const outFile = (repo: FixtureRepo) => path.join(repo.work, '.gstack/tmp/pregate.json'); + +describe('carry with a recorded pregate', () => { + // Value: protects=a carried receipt never claims the old tree's pregate; fails_when=carry rebinds without a current pregate + // or accepts a stale one; seam=none; no_claim=gh pr edit (--body stands in) + test('refuses without a current --pregate, dry-run still reports eligibility, and a current pregate rebinds', () => { + const repo = makeFixtureRepo(); repos.push(repo); + const reviewed = commit(repo, { 'lib/util.js': "exports.add = (a, b) => a + b + 0;\n" }); + expect(pregate(repo).status).toBe(EXIT.ok); + const body = path.join(repo.root, 'body.md'); + const w = runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '7', '--base', 'main', '--version', '1.0.0', '--session-kind', 'unattended', '--pregate', outFile(repo), '--body', body]); + expect(w.status).toBe(EXIT.ok); + const oldPregate = fs.readFileSync(outFile(repo), 'utf8'); + repo.git('checkout', '-q', 'main'); + write(repo.work, 'docs/README.md', '# docs moved on main\n'); + repo.git('commit', '-qam', 'main: docs'); repo.git('push', '-q', 'origin', 'main'); + repo.git('checkout', '-q', 'feature'); + repo.git('merge', '-q', '--no-edit', 'origin/main'); + const head = repo.git('rev-parse', 'HEAD'); + + const bare = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main']); + expect(bare.status).toBe(EXIT.refused); + expect(bare.out).toContain('CARRY: refused (RECEIPT_CARRY_PREGATE_STALE)'); + expect(bare.err).toContain(`run gstack-pregate at HEAD ${head.slice(0, 12)}`); + expect(extractReceipt(fs.readFileSync(body, 'utf8')).receipt!.head).toBe(reviewed); + + const dry = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main', '--dry-run']); + expect([dry.status, dry.out]).toEqual([EXIT.ok, expect.stringContaining('CARRY: would carry')]); + + const stalePath = path.join(repo.root, 'old-pregate.json'); + fs.writeFileSync(stalePath, oldPregate); + const stale = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main', '--pregate', stalePath]); + expect(stale.status).toBe(EXIT.refused); + expect(stale.err).toContain('PREGATE_STALE'); + expect(extractReceipt(fs.readFileSync(body, 'utf8')).receipt!.head).toBe(reviewed); + + expect(pregate(repo).status).toBe(EXIT.ok); + const carried = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main', '--pregate', outFile(repo)]); + expect(carried.status).toBe(EXIT.ok); + expect(carried.out).toContain(`CARRY: carried from ${reviewed.slice(0, 12)} to ${head.slice(0, 12)}`); + const after = extractReceipt(fs.readFileSync(body, 'utf8')).receipt!; + expect(after.head).toBe(head); + expect(after.pregate).toContain(`current for ${head.slice(0, 12)}`); + }, 60_000); + + // Value: protects=--head cannot borrow another tree's pregate; fails_when=carry verifies the checkout's tree but stamps the + // pregate current for a different --head; seam=none; no_claim=gh pr edit (--body stands in) + test('a --pregate from a checkout at another commit never rebinds the receipt to --head', () => { + const repo = makeFixtureRepo(); repos.push(repo); + const reviewed = commit(repo, { 'lib/util.js': "exports.add = (a, b) => a + b + 0;\n" }); + expect(pregate(repo).status).toBe(EXIT.ok); + const body = path.join(repo.root, 'body.md'); + expect(runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '7', '--base', 'main', '--version', '1.0.0', '--session-kind', 'unattended', '--pregate', outFile(repo), '--body', body]).status).toBe(EXIT.ok); + const target = commit(repo, { 'test/util.test.js': "require('node:assert').strictEqual(1, 2);\n" }); + repo.git('checkout', '-q', reviewed); + expect(pregate(repo).status).toBe(EXIT.ok); + const borrowed = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main', '--head', target, '--pregate', outFile(repo)]); + expect(borrowed.status).toBe(EXIT.refused); + expect(borrowed.out).toContain('CARRY: refused (RECEIPT_CARRY_PREGATE_STALE)'); + expect(borrowed.err).toContain(`not the carried head ${target.slice(0, 12)}`); + expect(extractReceipt(fs.readFileSync(body, 'utf8')).receipt!.head).toBe(reviewed); + }, 60_000); +}); diff --git a/test/size-ledger/ship/garrytan-gsta131-token-efficiency.json b/test/size-ledger/ship/garrytan-gsta131-token-efficiency.json new file mode 100644 index 000000000..d50ebe107 --- /dev/null +++ b/test/size-ledger/ship/garrytan-gsta131-token-efficiency.json @@ -0,0 +1,4 @@ +{ + "bytes": 1757, + "why": "GSTA-131 token efficiency: review plan dispatch, fix-check 3.5, plan-governed adversarial step, Step 1 delta carry, Step 11.5 fix-check and planned-skip binding, 400-word findings cap, light Step 7/8 audits; screen repair: one decision each for the loop exits, adversarial dispatch and Step 16 receipts" +} diff --git a/test/skill-validation.test.ts b/test/skill-validation.test.ts index 4a1837992..e4feb7a26 100644 --- a/test/skill-validation.test.ts +++ b/test/skill-validation.test.ts @@ -1619,13 +1619,14 @@ describe('Codex skill', () => { expect(content).toContain('200'); }); - test('adversarial review in /ship always runs both passes', () => { + test('adversarial review in /ship runs the passes the review plan dispatches', () => { const content = readShipUnion(); - expect(content).toContain('Adversarial review (always-on)'); + expect(content).toContain("Step 9's last full-pass plan governs this step"); + expect(content).toContain('## Step 11: Adversarial review\n'); expect(content).toContain('adversarial-review'); expect(content).toContain('run-with-timeout --effort high '); expect(content).toContain('Investigate and fix'); - expect(content).toContain('Claude adversarial subagent (always runs)'); + expect(content).toContain('Claude adversarial subagent (on `DISPATCH: adversarial-native`)'); }); test('scope drift detection in /review and /ship', () => {