Files
gstack/docs/review-policy.md
T
Garry Tanandcapy-ai[bot] 14d7414af3 v1.93.33.0 wave(review): #3705 #3744 #3745 #3764 #3765 #3795 (#3852)
* fix(ship): base merges, adjacent base edits, test-only and audit-requested fixes no longer force a second review loop (GST-407, GST-384, GST-391)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* ship driver: trim two phrases for headroom with #3662

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* ship driver: keep the Next steps heading phrase the handoff check reads; trim elsewhere

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* test: regenerate ship goldens

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: pre-landing review fixes

comparePath returns same only when every hunk sits where the base change puts it (patch-ids ignore positions) and treats a file-mode change as changed; the scoped-review packet says when a fence is truncated; --docs-result must be the posted audit's result; the --context-reviewed record goes beside the packet, outside the worktree. Negative controls for each.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: pass-2 review fixes

A test edit routes as tests= only when it adds lines with no gate tag; a removed or changed line or an unreadable test diff goes through the full loop. Diffs skip textconv. A hunkless patch (binary, empty, mode-only) is same when its patch-id matches. Without --reviewed-base test files route as before. finish's packet path and attempt's state directory have fallbacks.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: Step 11 review fixes; a version-only stamp_files edit after review is release metadata (GBRA-251)

gateEditedPaths reads removed lines only inside hunks; carry refuses when a path listing fails; docs freshness stops on a failed listing; attempt counts an unreadable state file. releasePaths includes stamp_files paths whose edit is a version swap under stamp_patterns.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: no test edit skips review; guard packet writes; exclusive attempt state

A test-only edit with no gate tag gets the scoped fix-check of its test hunks instead of a lanes-only rerun. Packet write failures print a STOP or RECEIPT_CARRY_REFUSED. attempt's allowance uses an exclusive create.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: attempt prints gstack's install command; carry checks the product diff outside context paths

The attempt STOP no longer echoes the audit child's fix text. A scoped-review carry now requires the product patch-id without the context paths to match at both heads.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: oversized packets take the full loop; exact --docs-requested match; name-only test globs only in test dirs

A scoped-review packet section over 200 lines sends that path to the full loop instead of a truncated packet. The docs-result path match is exact. test_*.py, *Test.java and similar count as test code only under a test directory.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* docs(unattended): carry's scoped-review refusal and what it keeps

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* docs: carry's scoped-review route in ship-policy, review-policy and troubleshooting (docs-a-1)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* test(windows): curate ship-rereview-triggers out of the Windows lane like ship-release (run 41193336209)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix(review-log): record a notes-only /ship pass clean; name the cause and repair when a pass's tree moved (GST-430)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix(receipt): carry and write handle PRs with thousands of files; tree receipt fails closed on a failed diff (GST-448)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix(ship): preflight routes RECEIPT_CARRY_SCOPED_REVIEW to the scoped review; carry --dry-run writes no packet (GST-442)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* docs(ship): Step 1.4 and Step 16 route the scoped-review refusal and the --docs-requested fix; trims keep the cap (GST-442, GST-444)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: /ship review pass 1: notes-only floor, scoped carry order, named temp and diff refusals

- A notes-only pass is recorded clean only when every finding is fixed, or noted or deferred below the
  blocking floor (never CRITICAL or blocking), by its logger-stamped action, and the findings account
  for the claimed issue and critical counts (GST-430).
- The preflight scoped carry route commits and pushes the base-merge fix before writing the packet (GST-442).
- patchId refuses by name on an unwritable TMPDIR; the tree receipt names why its diff failed (GST-448).
- Step 16 stage 3 spells out --docs-requested with its --docs-result (GST-444).

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* test: pin the Step 16 --docs-requested route with its --docs-result

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: a claimed clean review row with a logger-stamped open gate finding reads UNVERIFIED (red-team, pass 1)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: Step 11 findings: gate rows from --findings are stamped, one open rule, stale patch-id dirs swept

- deriveFromFindings stamps gate rows with the session kind before deriving status, so a findings file cannot
  dispose of a gate finding the unattended logger forces open (outside-voice#1).
- reviewFreshness reads a claimed row's findings with isOpenByAction, the rule the notes-only rewrite uses;
  noted and deferred clear a finding only when it is marked blocking: false and is not CRITICAL.
- patchId sweeps gstack-patch-id-* directories over an hour old that a killed carry left in TMPDIR.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: Step 11 round 2: stamped-open gates win over status and disposition; resolved gate, skipped and below-floor actions stay CURRENT

- isOpenFinding treats action 'open' (the logger's forced-open stamp) as open before a row's status or disposition.
- reviewFreshness flags a claimed clean row only for an 'open' action or a CRITICAL or blocking deferral or note, so
  interactive kept and restored gates, skipped advisories and INFORMATIONAL deferrals read CURRENT as before.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: carry dry run offers the scoped route last; patch-id diff under the git dir; changelog matches the review-log floor

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: a claimed clean review row with any unresolved non-gate CRITICAL finding reads UNVERIFIED (pass 2 red-team)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* docs: carry dry-run scoped route, patch-id directory, tree-receipt refusal (docs-d-1 audit)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix(ship): Step 3 names merge conflicts in its STOP sentence again (ship-workflow-clarity pin)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix(tree-receipt): keep the gated-to-current diff at the file helper timeout (60 s); pregate literals check

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* test(carry): large-diff fixture sets core.longpaths so Git for Windows can add its long receipt paths (windows-free-shard 1, run 41210080055)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix(review): a stale local base branch yields to its origin twin in gate-diff and diff-scope (GST-462)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix(ship): Step 11 saves the targeted record and the receipt writes carry it; Step 9 clears it (GST-464)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* changelog: GST-464 and GST-462

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix(review): pass 1: ahead and diverged labels, diff-scope needs shared history; preflight refreshes origin/<base> and stops on a diff far beyond the PR (GST-462, HARN-15)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: pass-2 review fixes for base twin, prSized stop and targeted record

Resolve the tracking twin by refs/remotes/<remote>/<ref> and only for a bare
branch name, keep a local base that is ahead or diverged, quote the prSized
stop's base and name the fork case, and have every Step 9 entry create,
exclude and clear .gstack/tmp/targeted.json.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: origin's tracking base wins whenever related; prSized lower bound

The fix-check found that keeping a local base ahead of origin left unpushed
base commits out of the gate scan while review-plan listed them. The tracking
branch now wins whenever it shares history with HEAD, an explicit origin/main
reads refs/remotes/origin/main, the preflight also stops when the diff lists
fewer files than the PR, and the Step 9 exclude line survives a missing
.git/info and an exclude file without a final newline.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* changelog: the tracking-ref claim holds while the tracking branch exists

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* test: repin the two ship section briefs under no-loss replay GST-464

GST-464 brief-identity check: every reviewer brief lib/review-brief.ts cuts
from ship/sections/review-army.md and adversarial.md stays byte-identical to
the bodies the L and GST-257 replays admitted; only lead instructions outside
the cut spans changed. The check is a test CI runs.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* ship: keep Step 9's PASS_HEAD sentence whole; repin review-army under GST-464

The targeted-record line follows the PASS_HEAD/RANGE_BASE sentence instead of
splitting it (test/review-fix-range.test.ts pins that sentence). The brief-body
check still holds: every reviewer brief is byte-identical.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: restore bin/gstack-ship-receipt after the main merge

The merge commit b10e2db wrote a truncated copy of the file. This is the
three-way merge of both sides with the refused() union: main's codes plus
RECEIPT_CARRY_SCOPED_REVIEW.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* review-collect: narrow the consumer scan and give it one 300 s deadline (GST-544)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* review-collect: keep the consumer scan deadline under the 180 s plan timeout (review fix)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* review-collect: plain paths under --literal-pathspecs; whole-millisecond timeout override (review fix)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* review-collect: whole-tree grep for non-UTF-8 names, no argument spread, name spawn errors (review fix)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* review-collect: cap the consumer grep output at 256 MB across calls; per-scan deadline wording (review fix, cycle 4)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* review-collect: read a missing spawn status as null (Bun 1.4.2 returns undefined on ENOENT)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* test: review-collect whole-tree helper reads a null status as -1 (typecheck:test ratchet)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* review-bots: /review and /ship triage every configured review bot, not only Greptile (#3122)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: pre-landing review fixes (gh before 2.48 pages without --slurp; driver routes Step 10; tests for guards, review-bot fence, REMOTE_SLUG owner)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: drop the review-bot fence test that could not fail (the tool-fence WARN degrade is unreachable: placeholder spans are skipped first)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: Step 11 review fixes (record by comment id so a filename never reaches a shell; lookup failures are errors; tojson pages; longest bot prefix wins)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: pass-2 review fixes (record refuses * paths, colliding ids and a missing fetch dir; parent requires id and dir; botOwning doc)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* review: classify the PR type and always check repo conventions (#356)

The checklist now opens with a PR-type line (application, CI/infra,
scripts/tooling, docs-only or mixed). Application and mixed diffs run
every category; other types always run five categories, including the
new Repo Conventions & Platform Fit, and report the rest as not
applicable in one line instead of skipping silently.

No-loss replay: GSTA-224 H1 preregistered no-loss run (run id
gsta224-h1-20261031, Opus 5.5, 81 calls, $9.53) against main's checklist
with a no-gstack arm; every preregistered bar passed. The
review/checklist.md pin in test/review-packets-removed.test.ts moves to
a32963ef with replay 'GSTA-224 H1'.

Refs GSTA-224

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* review-bots: allowlist the record placeholders and match Step 10's id rule in tests

The record fence uses <comment-id>, <fix|fp|already-fixed> and <category>,
each with an enum or numeric grammar; <greptile-dir> becomes <bot-dir>.
review/greptile-triage.md also lists every REVIEW_BOTS skip reason.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* docs: review-bot triage covers every configured bot (#3122)

README, CONTRIBUTING, the contributor reference and standalone-binaries
name gstack-review-bots and the review-bot fence; TODOS drops the #3122
item; the changelog fragment covers record-by-id and lookup errors.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* test: the PR-type test also pins the compose, Terraform and README/CHANGELOG phrases

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* test: follow the review-bot rename in the remote free tier's three reds

The cross-skill consistency judge describes review-bot-history.md and its
record helper; the tracker-guard exemption for the removed raw Greptile
fetch in review/greptile-triage.md is dropped (the fetch now lives in
gstack-review-bots, whose bodies still reach context only through
gstack-issue-guard); the Step 10 PR-check test expects 'Review bots: not
run'. HARNESS_VERSION stays 6: skill-fixture.ts and llm-judge.ts are
recorded non-behavioral for every other case.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* test: keep the cross-skill judge's rubric to one renamed bullet

Name review-bot-history.md beside the older greptile-history.md so the
calibration corpus still matches, and keep the REMOTE_SLUG bullet. One
live 3-sample panel at this tree: consistent 3/3, scores 4/4/4.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* changelog: H1 fragment passes prose lint

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* changelog: name the H1 fragment after its branch; TODOS drops #356

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* docs: /review's PR type step in docs/skills.md (#356)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* ship-policy: review_exclude_paths key and loadPolicyAt (GST-508)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* review: apply the base policy's review_exclude_paths and the reviewer diff ceiling (GST-508)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* receipt: bind review_exclude in write, carry and the queue (GST-508)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* docs, tests and changelog for review_exclude_paths (GST-508)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: pre-landing review fixes (binary by bytes, links kept, untracked files out of the selection)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: keep binary files at code paths under review_exclude_paths

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* docs: name the reasons that keep a binary file under review_exclude_paths

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: keep every review_exclude_paths candidate when the gate scan is unknown; docs example names its version

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* test: keep the NUL fixture clear of the sync-spawn tripwire

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* changelog: split the ceiling sentence

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: keep a live binary file whose bytes cannot be read; tests render outside voices from a clean checkout and read the ship fixture text

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* test(review-exclude): explicit +x for the census script; skip the chmod-000 read case on Windows and root

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* refactor: move loadPolicyAt to lib/ship-policy-at.ts so lib/ship-policy.ts stays under the 800-line module cap

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: refuse a base that contains HEAD; skip a self-pointing origin/HEAD (GST-774)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* chore: regenerate gstack-doctor-components.sh for BASE_CONTAINS_HEAD

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: land resumes a merged merge-only PR instead of refusing BASE_CONTAINS_HEAD (GST-774 review)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* docs: changelog fragment for GST-774

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* docs: troubleshooting names diff-scope base_contains_head and the self-pointing no_base case (GST-774 docs audit)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* test: repin the reviewer briefs to GST-508's no-loss-tested bytes

ship/sections/review-army.md, ship/sections/adversarial.md and lib/review-depth.ts now pin
the bytes the GST-508 preregistered no-loss replay ran at ef2760f (handoffs/GSTA-202/noloss-results.md
in the GStack Minder drive): Arm V 33/41 vs R 33/41, delta 4, exact sign flip, p_NI 0.0279 (pass).
The merge of main 83ae96e leaves these three files byte-identical to ef2760f, and an Arm P unit
(3593) renders byte-identical briefs at ef2760f and at the merge.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: GST-774 review round 1 — merged resume depth, aliased and detached PR checkouts, develop/trunk defaults

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* ship: Step 20 says a metrics row, keeping DRIVER.md at 12 KB after the main merge

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* test: ship-parts resend case leaves room for the first phase-3 part after the Step 9 targeted-record sentence

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: /ship delivers review/checklist.md in two parts so checklist v1 fits the 24576-byte part cap (#3670)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* changelog: drop the GST-407 and GST-430 fragments, already released on main (stacked history)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* wave review-53cfb5bfe011: lib/ship-policy.ts back to 800 lines (module-size ratchet after merging #3744 and #3795)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* wave review-53cfb5bfe011: /ship pass 1 fixes

- lib/ship-receipt-depth.ts: a merged resume (land after a true merge) no longer recomputes the review_exclude digest over the emptied range, so it lands instead of RECEIPT_INVALID (failing-first land test with review_exclude_paths active).
- bin/gstack-review-dispatch render: a standalone targeted render under an active review_exclude_paths refuses and points at the pass manifest voices stage, instead of writing a brief with no selection that leaves verify dirty (failing-first test).
- test/default-base.test.ts: #3795 expectations follow main R51 depth (light for an 80-line code diff; runtime_lines=80 still proves the real base).
- test/review-dispatch.test.ts: the GST-897 fingerprint test renders from the clean checkout, like its neighbor, because CI rebuilds the tracked lib/diagram-render/dist/diagram-render.html and the reviewer diff ceiling refused the ROOT render.

* wave review-53cfb5bfe011: /ship pass 2 fixes

- lib/review-exclude.ts: binaryContent reads each blob's first 8,000 bytes (git cat-file blob, 8 KB buffer) instead of one --batch read capped at 256 MB, so head mode (receipt, carry, queue) and the plan agree past 256 MB of blobs; an unreadable blob is kept, as in live mode. Failing-first test with two 270 MB blobs.
- lib/review-bots.ts: a file-level bot review comment (subject_type file, no line position) is triaged instead of read as outdated. Failing-first test.
- test/ship-queue-land.test.ts: the merged-resume exclusion test runs its gate scan under the fixture state root, not the real home (CI free-suite 6 home containment).

* fix: merge follow-ups for main 3d99c22: origin/* stays qualified in diff-scope's twin step, retro skeleton back under its cap, combined no-loss pin GST-508+R51+GST-911

* fix(review): receipt write gate-scans whenever the base declares review_exclude_paths; collectLive never re-qualifies origin/<base> (#3852 /ship red-team#1, outside-voice#1)

* test: the docs-only receipt digest test keeps its gate scan out of the real home (free-suite shard 5 FREE_SUBSHARD_REJECTED)

* fix(merge fc292e22f): tentative REVIEW_EXCLUDE_KEY_VERSION, classify targeted.json, render main's new outside-voice test from a clean checkout

- lib/ship-policy.ts: REVIEW_EXCLUDE_KEY_VERSION 1.93.0.0 -> 1.94.0.0, the next-minor tentative form stamp_paths moves (docs/ship-policy.md, #3512); 1.93.0.0 is a released heading that never names the key.
- test/gstack-clean-producers.test.ts (new from main): KEPT gains targeted.json, the wave's --review-targeted input.
- test/review-dispatch.test.ts: main's wrong-tree test rendered the outside voice from ROOT; a CI build that dirties ROOT pushes that diff past the wave's REVIEW_DIFF_MAX_BYTES ceiling, so render refused and verify saw no brief (exit 3). It now runs in cleanCheckout like the wave's other outside-voice tests.

* test(merge 68410828f): GST-967's brief-body pin carries the wave's GST-508 placeholders

The merged sectionTexts differ from main's by exactly the wave's GST-508 exclude-pathspec lines (merged minus main equals wave minus fc292e22f); the 1,440-case plan grid still hashes to main's MAIN_PLANS. Checks: GStack Minder drive handoffs/GSTA-202/merge-6841-checks.md.

* fix(merge e85005482): targeted classes since the read accept an empty range; the wave's receipt tests pass main's targeted record

- lib/ship-receipt-depth.ts targetedClassesSince (main, GST-986) reads the range from the commit the targeted reader read to HEAD through headInputs, which since #3795 runs gstack-diff-scope's BASE_CONTAINS_HEAD check. When the reader read HEAD itself the range is empty, diff-scope reported base_contains_head, and every write with --review-targeted was refused (the product patch matched). It now passes merged=true (--merged), which skips only that check; an ancestor check already guards the range.
- test/review-exclude.test.ts and test/review-targeted.test.ts: since #3887 write refuses a money head with no targeted record and requires the record's head, so the wave's two writes that must succeed pass one, as main's own tests do (targetedAtHead in test/ship-receipt-depth.test.ts).

* test(merge e85005482): drop the wave's duplicate targeted.json KEPT entry

Main (#3887) classified .gstack/tmp/targeted.json in KEPT too; the auto-merge kept both keys, which typecheck:test rejects (TS1117). Main's entry stays.

* test(merge e85005482): ship size ledger entry for the merged tree (-232 B)

Main's ship size ledger plus the wave's entries summed to 345,334 B; the merged tree measures 345,102 B because DRIVER.md takes main's shorter Step 19 line. Recorded with scripts/size-ledger.ts record ship.

---------

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
2026-11-03 01:40:07 +00:00

55 KiB

Review policy: how gstack's reviews converge

This is the canonical statement of what a gstack review may block on, how the /ship review loop converges, and what the ship receipt records about it. The skills (/review, /ship) and the tools (gstack-review-scope, gstack-ship-receipt) implement it; a team's own rules should point here instead of restating it. A one-line crew rule is enough:

Review convergence: gstack review policy (docs/review-policy.md), scope knob default.

The blocking floor

A finding is blocking when it is a CRITICAL defect: it would fail a test, break production, or lose data or secrets. Everything else is below the floor: an advisory, an informational item, a test finding (category testing: a gap, a weak or flaky test), a wording or naming note, a style or simplification suggestion. Below-floor findings are reported, never fixed after pass 1 of a ship, and never earn another review pass.

Three words cover every finding, and the loop budgets by them: a blocker is the intended case gone wrong (the floor above); a should is a real bug a caller will hit inside the intended case (fixed when it is in the diff on pass 1, deferred when it is not); a nit is the rest (a named note). Two stop rules follow: if a pass's fixes grew the diff past what the request needs, cut that growth first; and the cap approves nothing, so a standing blocker is reported, never waved through.

Before a merged finding counts, the parent applies lead judgment (review/sections/review-army.md, ship/sections/review-army.md, stage 4 of the merge): a reviewer whose whole return is nits found nothing; a defect names the call site or input that reaches it, or it is a note; structure for a case nothing takes is advisory; "I would have done it differently" is not a finding; a finding that assumes a rule the repository documents otherwise is dropped with the pointer. Independent agreement between reviewers is the strongest signal.

Every reviewer prompt gstack dispatches (the core checklist, each specialist, the Red Team, the outside voice) carries this definition: CRITICAL means the floor above; everything else is INFORMATIONAL. Data loss, a trust-boundary bypass and uncapped spend stay forced CRITICAL (the sideways sweep rule); no lane, specialist, dedup or saved decision downgrades them.

Rulings below the floor

The floor is a test, and these classes have been put to it, each more than once, and failed it (#3485 from the /ship of #3484, #3504 from the /ship of #3487). A finding in one of them is a note from the first pass on, named by its ruling in the PR body's ## Review notes; it never earns a fix round or another review pass, and the lead judgment stage drops it from the defect count with the pointer. The floor itself does not move, so a finding that also loses data, crosses a trust boundary or spends without a cap stays CRITICAL whatever class it resembles.

Ruling What it looks like Why it is below the floor
own-artifact hardening A check missing between two things the repository's own tooling writes: a screen verdict not compared with the receipt it belongs to; a receipt schema that does not declare a key the writer records; a policy loader that defaults when the base branch's own file is invalid; a size read that follows a symlink inside the repository. The only writer is the repository's maintainers through its tools; no caller inside the intended case reaches the gap, and no test fails. A writer outside that trust boundary makes it a bypass, and CRITICAL.
unwired or unread A helper with no caller, a field written and never read, a duplicated helper, a header comment naming a step that does not run it, a key-version constant that moved before any release carried the old value. Nothing takes the path, so nothing breaks; it is dead code to delete or wire in its own change.
bounded repeat work A re-dispatch on every call, a redundant screen after main moves, an orphaned scratch branch, a refusal that could come one step earlier in the sequence, a settle that could wait one more check. The operation completes, nothing is lost, and the cost is one more bounded run; uncapped spend is the sideways sweep's CRITICAL, not this class.
conservative refusal Stale state scoped wider than its cause: a RED verdict that keeps refusing a head after main moved, a pending result that keeps refusing after it settled. A refusal costs a rerun and never passes something wrong; a wrong pass would be the floor.
artifact presentation A string from an artifact printed verbatim into a parsed line (whitespace, control characters); a classifier that reads a non-zero exit code beside a non-RED verdict as RED. Display and classification of the repository's own output; it forges no parsed field.
test gap A state-matrix cell without a test, a coverage percentage, a weak or flaky test, a fixture that shares the coordinator's environment. Category testing, below the floor by definition above; a gap is work for the test audit, not a defect in the diff. Precedence for state machines: the specialist still reports each untested cell CRITICAL so gstack-state-matrix check proves the matrix complete, and gstack-state-matrix findings maps it to INFORMATIONAL per matrix cell before the merge; a wrong row or collapsed kinds stays CRITICAL, and a cell with a second finding keeps both.

A reviewer that wants a class promoted argues it against the floor with a reaching input, in the finding, and the owner rules; the table records the rulings, it does not close them.

NO FINDINGS is a result

A reviewer that finds nothing answers NO FINDINGS and nothing else. That is a completed, clean result, never a failed review and never a reason to invent an item to show work. Missing or unusable reviewer output is the opposite: incomplete coverage, which blocks until the reviewer is rerun.

How /ship converges (review_scope: frozen, the default)

  1. Pass 1 reviews the whole diff against the merge base. Every finding goes through gstack-review-scope classify, which stamps it with locus (diff: a hunk the PR changed, with three lines of context; outside: anything else), kind, blocking and scope. Findings in the diff are fix. A blocking finding in code the PR did not change is defer: gstack-review-scope defer files it as a GitHub issue (one per finding, reused across reruns through an <!-- gstack-deferred-finding: <fingerprint> --> marker; the issue text is scanned like the PR body before anything is posted, so a finding that quotes a secret is unfiled (publication scan refused: …); unfiled (<reason>) when gh is unavailable or its search fails; every unfiled case still exits 0) and the PR body lists it under ## Deferred findings. A finding's cause (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. 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 rounds is the cap (R51): a full pass, a scoped pass (below) or a fix-check is a round. After round 2 the run finishes with its open notes listed in the PR body and receipt, or stops for the owner when a blocking finding is open, or was fixed in round 2 and is unverified, or a round-2 fix edited a gate-class path, naming it (path, severity, problem). The cap limits fix rounds, not Step 11's voices: on gate class a stop waits for Step 11's adversarial, outside and targeted voices (NEXT: voices), so gate code never reaches the owner's "ship with open notes" without them (GST-967). A third round is the owner's grant of one scoped fix-check (recorded in review_loop_exception), never the loop's: once gstack-approvals add records the grant (its ask or scope naming review_loop_grant and the stop head's sha), gstack-review-plan --grant <approval id> --range-base <round 2's PASS_HEAD> plans that fix-check in place of the stop after a round-2 full pass with a fixed blocking finding or a gate-class edit, and refuses any other row, including a grant for a stop head before --range-base (an earlier /ship). A fix-check never earns another, so the grant cannot repeat, and a stop after a round-2 fix-check still goes to the owner. The cap approves nothing.

Transitions

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, on a diff below full depth with a fix range of at most 150 runtime lines scoped pass on the range
a full pass that fixed a blocking finding, below the cap (full depth, a larger range, or a range or depth it could not read) 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 (round 2)
a light pass 1 whose below-floor fixes change no runtime lines done
round 2 (a full pass or a fix-check) that fixed a blocking finding or edited a gate-class path stop: name it for the owner, who may grant one scoped fix-check
round 2 (a full pass or a fix-check) that fixed only below-floor findings done: notes in the PR body
a full pass that leaves a blocking finding open (fixed or not) stop: name it for the owner
a fix-check that found nothing blocking done
a fix-check that found a blocking finding stop: name it for the owner
a stop (any row above) on gate class before Step 11's voices ran Step 11's voices (adversarial, outside, targeted), then ask again with --adversarial-ran
a round-2 full pass that stops for a fixed blocking finding or a gate-class edit, with the owner's recorded grant (--grant <approval id>) one scoped fix-check (round 3), after Step 11 on gate class

A fix-check is one reviewer reading the touched hunks, the fix list, the test results and the callers of every changed symbol, at full effort. It is logged as a review-fix-check row, so it never counts as a pass. No army, no Red Team and no outside voice run on it.

NEXT: voices runs Step 11 as done does, from the last full pass's manifest; /ship then asks the plan again with --adversarial-ran (a blocking finding the voices queued counts as --blocking-open, never a new Step 9), and the stop or the granted fix-check it names stands.

A scoped pass is pass 2 after a blocking fix when pass 1 and the diff both ran below full depth (no gate class, no depth error, no --depth-floor full). A full pass 1 always gets a full pass 2: --pass1-depth carries pass 1's REVIEW_DEPTH, and an absent flag counts as full. The fix range RANGE_BASE..HEAD must have at most SCOPED_RANGE_LINES (150) runtime lines, counted as the depth counts them (gstack-review-plan --range-base $PASS_HEAD prints FIX_RANGE:). It dispatches one fix-check on the range, the Red Team on the range, the native adversarial pass when the range changes runtime lines, every force-included specialist and the outside voice when published numbers changed. Step 11's voices that have not run yet stay listed, as on a full pass 2. It writes a full-pass review row with "scope":"scoped", so it counts toward the cap, and a blocker it leaves open stops for the owner as before. A fix that touched a gate-class path (read from the range with its own scope and gate scan), a larger range, an empty range, or a range, classifier or depth it cannot read keeps the full pass 2. The 150-line figure is a starting value from a 24-PR backtest (GSTA-152 D6), to revisit once classified findings carry raised_by.

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 <base> classifies the diff, picks a depth and prints the reviewers the next step dispatches: REVIEW_CLASS:, REVIEW_DEPTH: <d> runtime_lines=<n> rules=<hash> reasons=<…> codes=<…>, GATE_SCAN: tagged=<n>|unavailable head=<sha>, one DEPTH_ERROR: per input that failed closed, one DISPATCH: per reviewer with its effort and a shell-quoted DIFF_CMD: (plus DIFF_EXCLUDE: <name> none when it reads everything), one SKIP: per reviewer left out, and --json with the same fields. A plan that fails closed to full exits 0; a diff git cannot list exits 1.

Classes are unchanged. A mixed diff takes its most restrictive class: docs (markdown no agent or gate reads), tests (test files and fixtures), code (everything else, including every SKILL.md and template, markdown in a skill directory, */sections/**, scripts/resolvers/**, rule docs, checklists and bins) and gate (code plus auth or migration scope, a hunk the gate scan tags, a deleted code or test path, CI workflows, credential, state-root and the paths below).

Runtime lines are the changed lines of code-class paths, minus outputs the base commit's .gstack/generated.json registers. Docs, tests and release metadata are never runtime; a .tmpl edit plus its regenerated SKILL.md counts the template once.

Data files no code names are docs. A changed or deleted JSON, JSONL, NDJSON, CSV, TSV or raster image (PNG, JPEG, GIF, WebP, BMP, ICO, AVIF) counts as docs when no non-docs file names its basename at the head, at the merge base or, for live /ship, in the working tree with untracked files; a data file named only by another data file follows that file, four files deep. Precedence: agent prose (.gstack/**), a test or fixture path, a gate path, a config file (.github/**, anything under a config/ directory, dotfiles, *.config.*, package.json, lockfiles, tsconfig*.json, manifest.json and the other tool manifests lib/review-depth.ts lists) and a named consumer each keep the path code, and YAML and TOML are always code. An SVG counts as data only when its whole text at the head is a drawing (GST-911): SVG drawing elements and RDF metadata, CSS made of declarations only (no @ rule and no function but a same-file url(#id), which closes @import, image-set(), image(), element() and every other CSS load), no backslash escape, every href a same-file # reference or an inline raster image, no event handler and no entity declaration, and no DOCTYPE but a bare <!DOCTYPE svg> or W3C's own SVG DTD, read after character references are decoded; anything else, a script or foreignObject included, keeps it code. More than 2,000 data paths keep every one code; a git grep failure is consumer_scan_failed and fails the depth closed. The scan first lists the files that contain a candidate's extension and then searches only those, docs files left out, so a large data tree costs one cheap pass. Each scan gets one 120-second deadline, set in milliseconds with GSTACK_CONSUMER_SCAN_TIMEOUT_MS; a scan that runs past it, or whose matches pass 256 MB, is also consumer_scan_failed, and its detail says what stopped it and names the deadline. One scan fits a minute inside the 180 seconds /ship gives gstack-review-plan; a plan that also scans a fix range runs two scans, and if they pass that limit the plan fails, which also leads to a full review. Only classifyDiff applies the rule (lib/review-collect.ts finds the consumers, gstack-ship-preflight's RUNTIME_LINES reads the same function); every path-only caller (the receipt's gate-scan decision, the Step 16 post-review route, /autoplan's docs-only scope) keeps data files code. A numeric claim in a data file still schedules the outside voice.

Depth When Pass 1 dispatches
light, runtime 0 docs, tests, eval write-ups, or only registered outputs one reader: the core checklist; the outside voice instead reads a diff that publishes numbers
light, runtime above 0 code with no risk class, at any size (R51: size never raises the depth) core, one batched specialist (the batched checklists below plus the Red Team prompt in one context), the native adversarial pass (kept: backtest inconclusive, GSTA-147 D5) and the outside voice on numbers
standard --depth-floor standard, or a light diff raised by a fixed blocker or a CRITICAL one batched specialist at medium and the Red Team at 200 runtime lines (or on a CRITICAL or a raise); the batched reader leaves docs and registered outputs out of its diff; adversarial and outside voice read everything
full gate class, a risk class, or any classifier error one batched specialist on the whole diff, the Red Team always, no hit-rate gating

Risk classes (R51). lib/review-risk.ts reads a code diff's paths and its added and deleted lines; any class makes the depth full (risk:<class> in reasons=, review_depth.risk in the receipt). money, auth and secret are the targeted pass's matchers. money also reads metering and charge words in a path or on a line (meter, metering, metered, charge, charged, billable, billed, credits) and three pairs: usage beside a token or cost word, proxy beside billing, and a client beside a cost or charge word, so a CLI's usage text and a plain proxy stay out (GST-850, from gbrain-evals #289, whose metering proxy and billing clients read required=none). security is a trust-boundary path (trust, redact, sanitize, neutralize, consent, escape) or line (shell: true, exec( and execSync(, eval, new Function(, raw HTML sinks, a shell run through argv, recursive deletes), any call to an outbound HTTP client (fetch, http(s).get or .request, axios, got, undici, requests, httpx, urllib, with no exemption for a literal URL), new URL from input, redirect toggles, proxy settings and host allowlists, archive extraction and path traversal (tar.x, adm-zip, yauzl, extract-zip, extractall, tar -x, unzip, path.join or resolve on request or entry input, a '..' check added or removed), and any changed line that names the child_process or node:child_process module, in any import or require spelling. A changed SVG line that can run code (a script or foreignObject element, an event handler, a javascript: URL) is security too (GST-911). The GST-850 rules are coarse on purpose and parse no import clause or call arguments; test/review-risk-linear.test.ts holds each of them under 100 ms on 200,000-character hostile lines, its anchor followed by a long run of the characters it repeats included. test/review-risk-redos.test.ts (GST-898) holds every risk pattern, R51's rm and rmSync rules included, under 100 ms on 100,000-character hostile lines. An rmSync( call with more than 500 characters before the first ) after it counts as security on its own. data is user data: a path naming pii, gdpr, ccpa, privacy, personal, account, customer, cookie, persistence, storage or database, or a line that drops a table or column, deletes rows or bulk-deletes records. Auth scope, migrations and deleted code paths were already gate class. RISK_RULES hashes the patterns and DEPTH_RULES includes it.

UI text stays code (light unless a risk class applies). A docs path for UI copy and locale values was built as an allowlist in GST-850 and dropped for review cost: its first review round found a new bypass class in each version, the same pattern as three rounds of R51. Chart SVGs use the drawing allowlist above, which reads the whole file rather than changed lines. When a diff publishes numbers, the batched specialist's diff keeps the write-up's text files instead of leaving docs out.

Batched checklists (the lean ensemble, GSTA-167 part 2). One specialist subagent reads Testing and Security on every code or gate diff, plus State-machines, Data-migration, API-contract and Design when scope asks (a forced checklist always joins). Maintainability, Performance and Simplification are read only when forced (SKIP: names them). The Red Team reads the batched reader's merged findings. A free re-score of saved C and D findings lost 0 of the 29 blockers (handoffs/GSTA-167/lean-ensemble-rescore.md); the live round 5 replay passed (L∘G1 found 24 of 29 blockers against the control's 21, handoffs/GSTA-167/noloss5-results.md).

A fixed blocking finding, a CRITICAL or an open blocker raises a light diff to standard for the next step (step=standard): its specialists read at least 50 lines and the Red Team runs, so a raise never dispatches less than light did. When the next step is a scoped pass, the raise applies to the scoped set only. A fix-check is the same at every depth.

Fail closed. Each of these makes the depth full and is listed in errors[] with the input it could not read, a sanitized cause and the repair: a gstack-diff-scope error, a gate scan that is unavailable, a base registry that cannot be read or is invalid (REGEN_REGISTRY_INVALID or REGEN_CONTAINMENT in codes=), a numstat git could not produce, and a binary code-class path (it is never counted as 0 lines).

The registry rule. Only the base commit's registry counts (the merge base's origin/<base> commit for live /ship and the current base ref's commit for the queue: origin/<base> in ready, <remote>/<base> in train build), and only entries with a command, which pregate's regen check proves for that tree. An entry added on the branch never excludes anything; command: null entries, vendored code and lockfiles stay runtime. To adopt it, commit an entry like this on the base branch:

{
  "$schema": "https://github.com/garrytan/gstack/docs/pregate.md#generated-v1",
  "entries": [
    { "id": "skill-docs", "command": "bun run gen:skill-docs --host all", "outputs": ["*/SKILL.md", "*/sections/*.md"], "inputs": ["*/SKILL.md.tmpl", "*/sections/*.tmpl"] }
  ]
}

Gate paths for the machinery itself. An edit to lib/review-depth.ts, lib/review-collect.ts, bin/gstack-review-plan, lib/review-scope.ts, lib/review-targeted.ts, lib/review-risk.ts, review/specialists/targeted.md, lib/ship-queue*.ts, lib/regen.ts, .gstack/generated.json, ship/sections/review-army.md.tmpl, ship/sections/adversarial.md.tmpl or scripts/resolvers/review-army.ts is gate class: changing how much review runs gets the most review. rules= (DEPTH_RULES) hashes the deciding constants and patterns, not file bytes, so a comment edit keeps it.

--depth-floor standard|full (a /ship prompt flag, passed to every plan run) raises the depth and never lowers it.

Numbers keep the outside voice. At light depth the outside voice runs only when the diff publishes numeric claims: gstack-review-plan scans the added lines of prose and data files for percentages, money, multipliers, durations, token or credit counts and score, rate or speed claims (NUMBERS: yes). 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. It always reads the whole diff.

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. The core checklist, the Red Team, the native adversarial pass and fix-checks run at full effort (the lane's reasoning); specialists run at medium (GSTA-167 part 3: the no-loss controls ran them there), except light's batched specialist, which carries the Red Team prompt and stays full. The outside voice and the native adversarial pass carry the floor sentence and end each finding with Severity: and Blocks: yes|no; a finding is CRITICAL when its label rule says so (native FIXABLE, except as the collect step below relaxes it on a runtime-only code or docs plan; outside Critical or High) or it says Blocks: yes, and Blocks: no never lowers one (gstack-review-dispatch collect). 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 specialists, audits and watchers at the host default too (model-policy.md).

Behavior changes

  • Pass 2 after a blocking fix on a diff below full depth, with a fix range of at most 150 runtime lines, is a scoped pass: a fix-check and the Red Team on the range, plus the native adversarial pass (on the range once Step 11 has run, otherwise on the whole branch), instead of the specialist army.
  • A tests-only diff (runtime 0) no longer dispatches the Testing specialist: the core checklist and the gate scan review it.
  • Every depth dispatches one batched specialist instead of one reader per checklist; Maintainability, Performance and Simplification run only when forced.
  • Light no longer runs a separate Red Team: the batched specialist carries its prompt. At runtime 0 light runs no adversarial pass either.
  • Standard reads thresholds on runtime lines, not the whole diff, so a test-heavy diff with few runtime lines dispatches fewer specialists.
  • R51: size never raises the depth; non-risky code is light at any size, and a risk class makes it full. Two rounds is the cap, a fix-check included: a blocker fixed in round 2 stops for the owner instead of earning a fix-check, and a light pass 1 whose below-floor fixes change no runtime lines is done.
  • A JSON, CSV, TSV or raster data file no code names is docs, not runtime (GSTA-167 part 4a). A binary image no code names is no longer a binary_code_path error that forces full depth; one that code names still is.

Targeted pass on money, gate, auth and secret code

Some code is costly to get wrong: code that spends money, code that decides whether a check passes, code that decides who gets in, and code that handles secrets. When a diff touches one of these, gstack-review-plan dispatches one more reader, targeted, at every depth, light included. It reads the whole diff, fixtures and tests included, with the blocks of review/specialists/targeted.md for the classes that hit, and it runs in Step 11 beside the native adversarial pass and the outside voice (GST-257).

lib/review-targeted.ts decides the classes from added and deleted lines, so removing a cap, a check or a redaction counts. Docs and tests paths never count, and matching is token-bounded: costume never reads as cost.

Class Signal
money a path segment naming cost, spend, budget, ledger, billing, price, fee or invoice; or a changed line with an identifier ending in _usd or Usd, any spend_cap spelling, or a call to a paid provider lib/pricing.ts prices
gate a hunk the gate scan tagged: a relaxed threshold, a skipped test, a loosened check
auth SCOPE_AUTH from gstack-diff-scope, on a code path with an auth, session, jwt, permission or role token in its name; a data file named for sessions (recovered-sessions.jsonl) outside a config location does not set SCOPE_AUTH
secret a secrets, credentials, keychain or token path, or a changed line that reads a key-named environment variable (OPENAI_API_KEY, GH_TOKEN) or a credential store

An input the classifier cannot read (the diff, gstack-diff-scope, the gate scan) fails closed to all four classes. TARGETED_RULES hashes the patterns. The classifier and the brief are gate paths, so an edit that weakens them gets full review.

The plan prints TARGETED: classes=<money,gate,auth,secret|none> rules=<hash> brief=<sha256 of the brief> targeted_patch_id=<id> and, when a class hit, DISPATCH: targeted targeted effort=full. The targeted reader always reads the whole diff: render --role targeted refuses a --diff-base other than the merge base, so on pass 2 and later it gets the merge base, not RANGE_BASE. After a fix, pass --targeted-ran <targeted_patch_id> from the line the reader ran on: the same committed patch skips the reader, and a changed one, uncommitted edits or none dispatch it again. targeted_patch_id is the patch-id of the whole committed diff, tests included, so a test-only fix that adds a gate hunk dispatches it again; it is not the receipt's product_patch_id.

The brief states the blocking floor in one sentence, and its money block cites the checklist's Forced-CRITICAL rule on uncapped spend, so a money finding can block without new floor words. The reader is read-only, and NO FINDINGS is a result. gstack-review-dispatch render --role targeted --classes <classes> renders its brief; verify --targeted-out .gstack/tmp/targeted.json writes the record that gstack-ship-receipt write --review-targeted @.gstack/tmp/targeted.json puts in the receipt (Step 19, the handoff and the driver pass it when the file exists). The record is done when the reader got its brief verbatim and its reply was collected on a committed diff, whatever happened to HEAD after, and it names the commit it read (head). write binds it only while that commit is an ancestor of HEAD, its product patch is the receipt's, and nothing changed after it carries a targeted class, with the gate scan run over that range: any product change after the read refuses with RECEIPT_INVALID, docs and TODOS.md commits included, and so does a test edit the gate scan tags (a skipped case). Every verify removes --targeted-out before it writes a record, so an earlier run's record never reaches Step 19. write and carry (which rescans the gate at the new head) also refuse a receipt whose head needs a class no done record covers (RECEIPT_INVALID, RECEIPT_CARRY_REFUSED). The queue refuses the same receipts at ready (#3859). gstack-ship-queue ready, train build and land print REVIEW_TARGETED: required=<classes|none> receipt=<classes|none|legacy> verdict=ok|missing|failed and refuse QUEUE_RECEIPT_TARGETED_MISSING when a required class has no done pass covering it.

Over the last 100 non-release commits on main (8663c3d2..8be7f213), money hit 21, gate 18, auth 3 and secret 8. The gate class first used the gate paths, which hit 43 of 100; those paths already get full depth, so the gate class was narrowed to the gate scan's tagged hunks.

What readers read under review_exclude_paths

A repository can list content folders its reviewers leave out, on its base branch only (ship-policy.md). Under it, every reader (specialists, the Red Team, the native and outside voices, the targeted reader) reads the same selection: its DIFF_CMD: with one :(top,exclude,glob) item per applied pattern, the fixture command with the same items, and the DIFF_KEEP: command for the kept files under those patterns. The outside voice's embedded diff is that selection. Each brief says how many files and bytes were left out and that the reader must not report on their absence. Excluded paths count as docs for the depth and never trigger the numeric-claim scan or a targeted class; kept files keep their own class.

Independently of the key, no reader is handed more than 2,000,000 bytes of diff: gstack-review-dispatch stops with REVIEW_DIFF_TOO_LARGE before it writes anything, instead of a review of the part a reader happened to see.

What the receipt records

gstack-review-scope summary --since <REVIEW_SINCE> --run <run dir> reads this invocation's review-log rows and the dispatch ledgers under the run directory (--ledger <f> names one; --no-ledger when no reader ran through gstack-review-dispatch) and writes the counters gstack-ship-receipt write --review-loop puts in the receipt:

Field Meaning
review_loop.passes full review passes this ship ran (1 or 2; up to 3 under review_scope: all)
review_loop.blocking_found every blocker found (GST-951): the rows' blocking findings (classified, or an unclassified CRITICAL), plus each ledger CRITICAL (as collected or judged) that no row finding from the same reviewer at the same path:line accounts for (each row finding pairs with one per ledger), so an adversarial, outside-voice or not-yet-classified blocker counts and one whose newest classification is below the floor does not. An unreadable ledger, merged file or judged set, or a judged set changed since judge, is REVIEW_SCOPE_LEDGER, never a count
review_loop.unrecorded blockers found that no row gives a disposition (fixed, deferred, noted, skipped, a gate answer); absent when 0
review_loop.ledgers_read dispatch ledgers the count read; absent under --no-ledger
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): the larger of the review-fix-check rows and the ledgers holding a fix-check reader's review. When fix-checks ran without their rows, a blocker no row disposes counts as fixed once a later fix-check ledger followed it (decision 3.5 runs one only after the fixes)
review_loop.rounds passes plus fix-checks (R51 caps it at 2; the queue's REVIEW_DEPTH: line prints it)
review_loop.dispatches reviewer dispatches by role: specialist, red-team, adversarial-native, outside-voice, fix-check, targeted
review_loop.est_input_tokens prompt plus diff bytes the dispatches read, divided by 4: a relative estimate for comparing runs, not a bill
review_targeted the targeted pass: the classes it covered, rules, brief_sha, the receipt's own product_patch_id, status (done only with its brief sent verbatim and its output collected on a committed diff, else failed; write refuses a record when a change after its read commit carries a targeted class or the read commit is not an ancestor, and refuses to write without a record when the head needs a class), findings and raised_by; a carry keeps it while the product patch is unchanged

Each review row may carry dispatches: [{role, effort, prompt_bytes, diff_bytes, status}], and each finding raised_by (the reviewer that raised it, stamped by gstack-review-scope classify; unknown when untagged, absent on older rows); 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.

Beside the counters, write records review_depth (the depth, runtime lines, rules hash, base and registry commits, the gate scan bound to the head and the risk classes) and the queue refuses a receipt below the depth its head needs (ship-policy.md#ship-receipt). A stop the owner accepted after a fix-check edit goes in review_loop_exception, never in prose under REVIEW_LOOP:. Step 16 takes the same record: gstack-ship-release finish --review-loop-exception <json|@file> routes the paths the branch touched at fix_check.head (narrowed to the record's fix_check.files when it lists them) and changed since as accepted= in ROUTE:, prints them on a REVIEW_LOOP_EXCEPTION: line, and carries them through the docs stage. A path outside that scope and a gate-edited test still route as behavior, with --reviewed-base (as /ship passes it) so does a file too large for the scoped review, and without the record nothing changes.

The receipt's summary line prints passes=<n> blocking_found=<k> deferred=<m>, 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) keeps these counters, because the reviewed product diff is unchanged or its only difference passed a scoped review.

The scope knob

.gstack/ship-policy.json review_scope (ship-policy.md#review-scope):

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 2 rounds (a fix-check is one)
diff-plus-blocking the diff fixed in this PR the range since the previous pass, blocking only 2 rounds (a fix-check is one)
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 up; all is the loop as it ran before the key existed. A value other than frozen needs min_gstack_version at or past the release that added the key (REVIEW_SCOPE_KEY_VERSION in lib/ship-policy.ts) so an older gstack refuses the file by name instead of ignoring the key.

Reviewer fan-out

The review army launches every selected specialist in one message so they run in parallel; a host that caps running subagents makes that impossible past the cap. In /ship the knob is a prompt flag, --review-fanout auto|serial|parallel (default auto), read where the force flags (--all-specialists) are (/review on its own keeps the single parallel batch: its generated SKILL.md sits at its size pin, and a refused launch there is a failed reader, never dropped coverage):

Value Batches
auto when the host reports fewer free subagent slots than selected specialists, batches of the free-slot count (at least one), each launched in one message, the next only after every reader of the previous one finished or is confirmed stopped; a host that reports no slot count runs one parallel batch
serial batches of one
parallel one batch regardless of slots; a refused launch is a failed reader, never silently dropped coverage

Capy caps a thread at 8 running subagents, and waiting or idle ones hold a slot, so under auto a nine-specialist review on an otherwise idle Capy thread runs as one batch of 8 and one of 1 (Fan-out: 9 specialists in 2 batches of 8). Batching changes the order, never the set: every selected specialist runs before the merge stage, and the Red Team's activation reads the merged result as before.

Two checklist rules with ids

Two findings review/checklist.md names by rule id so a PR body, a review log and a later reader agree on what was flagged:

  • prereg-with-results (Pass 1, Gate Integrity, ASK): one diff adds or changes an eval's preregistration (bars, arms, decoys, stop rules, a plan file) and also adds that eval's results, scores or verdict. The bar could have been set after the outcome, so the pairing is the finding. The repair is a split: the preregistration lands first and the results PR cites its merged commit.
  • test-keyboard-count-loop (Pass 2, informational): a browser-driver test reaches an element by a counted loop of key presses and asserts on whatever has focus, so the count encodes today's DOM order. Locate the target by role, label or test id; a counted loop stays only when the count is the behavior under test.

Porting a contributor's work

A ported branch keeps the contributor's commits as theirs and adds the porter's fixes on top, each crediting the original authors. gstack-coauthor-trailers [<range> | --base <ref>] [--message <file>] [--exclude <email>]... lists every distinct author of the range's commits (default origin/<base>..HEAD) other than the checkout's own committer identity, in first-appearance order, as Co-authored-by: Name <email> lines; --message <file> adds the ones the commit message file lacks to its trailer block (git interpret-trailers, pinned placement, so an existing co-author keeps attribution), matched by email case-folded, so running it twice changes nothing. The COAUTHORS: <n> in <range> line on stderr is the receipt; --json prints { range, authors, trailers, appended }.

Verbatim dispatch

A driven /ship (ship/DRIVER.md) never lets the model write a reviewer's brief or merge a reviewer's findings. The no-loss replays showed why: with briefs the parent filled in by hand, only 37 of 112 dispatched prompts matched the rendered ones, and the arm lost blockers. bin/gstack-review-dispatch closes both ends:

  1. render --role <role> [--name <n>] --out <dir> cuts the brief out of ship/sections/review-army.md or ship/sections/adversarial.md (the fix-check from scripts/resolvers/fix-check.ts) and fills only declared slots: the literal diff base, the plan's DIFF_CMD excludes, stack, test framework, learnings, checklist paths, the Red Team's merged findings and specialist count, and the outside voice's branch diff. A section that lost an anchor or placeholder refuses (BRIEF_RENDER_FAILED) instead of rendering something else, and so does a slot the section leaves unfilled (every reader but fix-check and the outside voice). That check runs before the findings and learnings go in, and they reach the reader as written, so a finding that quotes origin/<base> neither fails the render nor gets filled (GST-797).
  2. The dispatch program passes that file's content unchanged as the reader's prompt and runs sent on what it passed: a hash that differs from the rendered one is BRIEF_MISMATCH, and that reader's review does not count.
  3. collect hashes each reader's raw output and appends its findings to merged.jsonl with the label as written (a JSON line's severity, or the most severe of a prose line's Critical, High, Medium, Low, P0-P3, FIXABLE or INVESTIGATE wherever they stand, headings and table rows included, except that a line with its own Severity field takes that field's label (any letter case; a field stating more than one label, "High/Critical", counts as none), so a severity word in the finding's text does not raise it, and a label leading the line stays its floor (GST-976); a top-level list item under a label-only heading takes the stronger of the two). A Severity: or Blocks: line continues the finding above it until a list item, heading, bold paragraph opener or a second line for the same field starts another: Severity sets its label (a label leading the finding stays its floor), Blocks: yes makes it CRITICAL and Blocks: no never lowers it; an inline Blocks: yes is CRITICAL too, and with no finding above it is one. The native adversarial voice's FIXABLE collects as CRITICAL (normalizer v2, GSTA-167) unless collect --manifest names this ledger's pass and its plan is the code or docs class with only runtime=<n> depth reasons (no risk:, error:, history: or floor: reason). On such a plan a native finding's own Severity field decides and FIXABLE beside it raises nothing once it also states Blocks: no; a native FIXABLE finding without Blocks: no, with no Severity field, or whose field names only FIXABLE or INVESTIGATE still collects as CRITICAL (GST-987). An unnumbered, unlabeled bullet with no file:line under a paragraph finding whose Severity or Blocks line has not come yet is that finding's detail, so the field line after it lands on the finding; a numbered item is its own finding. A negated severity word still collects at its label ("found nothing CRITICAL", "No P0/P1 issues"), so a reader's closing summary is a known phantom: it stays at that label in the judged set, lead judgment can fold it only into a finding at least as severe, and it counts toward what blocks. GST-766 skipped negated labels and lost real blockers named in an exception clause ("No CRITICAL issues except the token leak in log.ts:3"); GSTA-167 reverted that. Each prose finding's line also records the Severity and Blocks its own text states as stated, beside the collected label and never in its place; a classified finding that carries stated shows it in its deferred-review issue next to the label ("CRITICAL (the reviewer wrote Severity: Low, Blocks: no)"). /ship's Step 9.4 does not copy stated into its findings file yet: that instruction lives in a reviewer brief that changes only after a no-loss replay. What blocks is unchanged. A phantom finding costs a fold; a lost one costs the gate. A reply with no finding counts as a review only with the clean marker its own brief asks for: NO FINDINGS alone on a line for the specialists, the batched specialist, the targeted reader, the Red Team and fix-check, the Recommendation: line for the outside voice, either for the native adversarial voice. Without one (a refusal, an empty reply, a reader that never saw the diff) collect records review: false, prints COLLECT_NOT_REVIEWED and exits 1.
  4. Lead judgment writes judged.jsonl; judge refuses a set that drops a merged finding, lowers its severity, folds it into a less severe duplicate, or adds or promotes one without a reason (JUDGMENT_REJECTED). A merged file missing any id the ledger collected is a drop. Each addition and promotion is logged with its reason.
  5. verify --ledger <f> replays all of it from the run's ledger: rendered against dispatched hashes per render (each render is one dispatch of its reader, so a two-pass ledger checks pass-1 sends against pass-1 briefs; a later render prints as <reader>@2), a collected output that is a review for every sent reader (NOT_COLLECTED otherwise, and the output row NOT_REVIEWED; the marker is checked again in the output file when it is unchanged, so an older ledger meets the same rule; a non-review the same dispatch later replaced with a review, by re-sending the brief, is VOIDED and counted in voided=N). The outside voice may fail without blocking: collect --unavailable '<reason>' records its failure text as missing coverage (COLLECT_UNAVAILABLE); an empty reason or an output that holds findings is refused (exit 2), so a partial answer is collected normally and loses nothing. Verify prints its brief and output UNAVAILABLE and unavailable=N on the verdict line, and its coverage is logged unavailable in the review-log row; any other reader is refused (exit 2), because its missing coverage leaves the review incomplete. A mismatched send re-sent byte-exact before anything was collected is void and counted (voided=N on the verdict line); one that produced an output stays MISMATCH. Then output hashes against the merged lines they produced, and each merged line against the finding collect parses at that id's index from the unchanged output file, label and raw text byte for byte (merged_line_differs on the OUTPUT and JUDGE rows otherwise, GST-541), so an edited merged.jsonl cannot lower a finding or swap its text. A recorded output file that is missing (output_file_missing), a judgment run on a merged file collect did not write (judge_merged_not_collected) and merged rows collect never recorded (merged_rows_not_collected) are dirty too, and so is an output whose re-parsed findings outnumber or undercount the ids collect recorded (merged_count_differs). The one exception is a reply collect refused for its tree: verify re-checks the reply's REVIEWED_TREE line against the render's tree, and such an output stays NOT_REVIEWED until a re-sent brief is answered by a reply that passes the same check. Deleting a whole output event with its rows is not caught yet, because the ledger has no anchor for it. The judgment is then rerun against the recorded judged set, covering every collected id. A ledger from a run that kept no prompts prints VERIFY: no brief records (exit 3), never clean.
  6. pass --base <b> --out <dir> --pass <id> -- <plan flags> runs one pass from a staged manifest (<dir>/pass-manifest.json): audit readers marked first, then the specialists with the other audit readers, then the Red Team, then Step 11's voices. A QA extra is refused before any reader is created: Step 9.2.1's probes are the parent's. --diff-base sets every reader's base, a scoped pass's Red Team included; the targeted reader keeps the merge base. Each reviewer keeps its plan entry and is rendered by the same code as render, once its inputs exist; the Red Team's findings file is the specialists' merged.jsonl lines byte for byte. A brief leaves the bin in indexed chunks of at most 65,536 characters (chunk), so no tool output line carries a whole brief and a host's line or output cap cannot cut one. Each reader's intent is recorded before its subagent is created and its path right after (mark), so a resumed pass finds a create whose result was lost by its title and never starts a reader twice; a missing or doubled title stops the pass. On Capy the pass runs as one call to the checked-in program lib/capy-review-pass.js, served by program and loaded unchanged by the gstack Capy skill. A full repeat pass takes --diff-base (RANGE_BASE; a scoped pass, none), a fix-check plan renders its reader from --fix-list, --hunks, --tests and --callers, --audit-only runs just the --extras readers (Step 14.5's docs audit), and the fork point is found against origin/<base> first, as preflight does.
  7. Every brief, audit readers' included, opens with a fingerprint step and ends with a workspace paragraph naming the checkout the bins run in (GST-482): a reader on a shared machine starts in the machine's working directory, so it is told to cd there on every command, to give every file tool an absolute path under it, and to start its reply with the REVIEWED_TREE: <HEAD^{tree}> <toplevel> line the step's command outputs there. The step comes first because a reader following a findings-only format skipped the request when it only closed the brief (GST-897). The section's render sits between the two, byte for byte. collect and mark --state collected compare that line with the fingerprint recorded at render: a missing or foreign line is COLLECT_WRONG_TREE, a later commit in the same checkout COLLECT_TREE_MOVED; nothing is merged, the reader becomes failed, and verify reports NOT_COLLECTED. An outside voice's tree failure is recorded unavailable only when its reply holds no finding. The line catches a reader that worked elsewhere by accident; it is not proof of reading, since diff text can predict it. A printed: label before the line (plain, bold or inside its backticks) is still read, and its tree and toplevel are still compared exactly (GST-776). Capy cannot start a subagent in a chosen directory, so before a stage takes a start token or sends its first reader, the Capy pass program compares the checkout with the one shared readers start in (the thread's working directory) and stops when that is another checkout or cannot be read: run /ship in the thread's own checkout. A start directory outside any checkout, such as a multi-repo workspace root, holds no tree to review by mistake and dispatches. Step 11's voices take the adversarial-review start token by default, and a logStart list without it stops before anything runs.

Where this is enforced

  • lib/review-brief.ts, lib/review-ledger.ts, lib/review-pass.ts, lib/capy-review-pass.js, bin/gstack-review-dispatch: verbatim dispatch (test/review-dispatch.test.ts, test/review-pass.test.ts).

  • lib/ship-parts.ts, bin/gstack-ship-parts: the section text each /ship phase uses, returned once per run inside the phase program before it; next --resend sends an unreceived batch again (test/ship-parts.test.ts).

  • 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 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=<diff-base>: the merge base on pass 1, RANGE_BASE from pass 2 on) beside its plan's DIFF_CMD excludes, 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: the plan's REVIEW_DEPTH, GATE_SCAN and --depth-floor on every run, the depth fields of the review-log row, 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-exclude.ts: the base policy's review_exclude_paths (matched set, kept reasons, the kept-file argument bound, the digest the receipt, carry and the queue compare) and the 2,000,000-byte reader ceiling (test/review-exclude.test.ts).

  • lib/review-depth.ts, bin/gstack-review-plan: the diff class, the depth (depthFor, DEPTH_RULES, depthOfCommitInput), each dispatch's exclude pathspecs and DIFF_CMD, 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-collect.ts: collectCommit (git objects only, from any checkout) and collectLive (the working tree), per-path lines and the base commit's registry (test/review-collect.test.ts).

  • lib/review-scope.ts, bin/gstack-review-scope: classify, defer, render, summary (with fix_checks, dispatches, est_input_tokens, unrecorded, ledgers_read); refusals are REVIEW_SCOPE_INPUT, REVIEW_SCOPE_RANGE and REVIEW_SCOPE_LEDGER (troubleshooting.md).

  • 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).

  • lib/ship-receipt.ts, lib/headless-artifacts.ts: the review_loop receipt object, its schema and the summary line.