feat(ship): review loop converges (frozen finding scope, 3-pass cap, NO FINDINGS first-class) and receipts carry across conflict-only merges

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

docs/review-policy.md

@@ -0,0 +1,99 @@
+# 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 gap, 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.
+
+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.
+
+## `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; `unfiled (<reason>)`
+   when `gh` is unavailable) and the PR body lists it under `## Deferred findings`.
+   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 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. **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.
+
+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.
+
+## What the receipt records
+
+`gstack-review-scope summary --since <REVIEW_SINCE>` reads this invocation's
+review-log rows and writes the counters `gstack-ship-receipt write --review-loop`
+puts in the receipt:
+
+| Field | Meaning |
+|---|---|
+| `review_loop.passes` | review passes this ship ran (1 to 3) |
+| `review_loop.blocking_found` | distinct blocking findings across the passes |
+| `review_loop.deferred` | blocking findings in unchanged code filed as issues |
+| `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 |
+
+The receipt's summary line prints `passes=<n> blocking_found=<k> deferred=<m>`,
+which is what a scorecard tracks (target: a median of 2 passes 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.
+
+## The scope knob
+
+`.gstack/ship-policy.json` `review_scope` ([ship-policy.md#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 | 3 passes |
+| `diff-plus-blocking` | the diff | fixed in this PR | the range since the previous pass, blocking only | 3 passes |
+| `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 1.91.249.0. A value other than `frozen`
+needs `min_gstack_version` at 1.91.249.0 or later so an older gstack refuses the
+file by name instead of ignoring the key.
+
+## Where this is enforced
+
+- `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`).
+- `ship/sections/review-army.md` Step 9: passes, `RANGE_BASE`, classify, defer, render, 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/ship-policy.ts`: the `review_scope` key and its `ship: review_scope …` consequence line.
+- `lib/ship-receipt.ts`, `lib/headless-artifacts.ts`: the `review_loop` receipt object, its schema and the summary line.

docs/ship-policy.md

@@ -57,6 +57,7 @@ refuses the file (`POLICY_GSTACK_TOO_OLD`). `validate` ends with
 | `migrations` | `{ dir, id }` or `null` | `null` | The migration directory and a regex with exactly one capture group for the number in a file name (`"^v(\\d+)-"`); `gstack-pregate regate` reports `MIGRATION_COLLISION`. |
 | `gate_command` | command or `null` | `null` | The repo's full pre-merge gate. `gstack-ship-queue gate-ahead` and `ready` print it for the caller to run (in the background or in CI); gstack never executes it. Needs `min_gstack_version`. |
 | `full_e2e` | `true` \| `false` | `false` | `true`: every merge needs a passing full-gate result bound to a tree equal to the merging one modulo stamps; `gstack-ship-queue ready`, `stamp` and `verify` refuse without one (`QUEUE_GATE_MISSING`): a gate-ahead record, or the lane's `--gate-exit 0 --gate-log <file>` from its run on the gated head. Needs `min_gstack_version`. |
+| `review_scope` | `frozen` \| `diff-plus-blocking` \| `all` | `frozen` | The finding scope of `/ship`'s review loop; see [review_scope](#review-scope). `frozen` fixes the diff on pass 1, files blocking findings in unchanged code as issues, and later passes read only the range since the previous pass. `diff-plus-blocking` is the same but fixes a blocking finding in unchanged code in this PR instead of filing it. `all` is the earlier loop: every finding is fixed on every pass. Needs `min_gstack_version` when not `frozen`. |
 | `min_gstack_version` | version or `null` | `null` | The oldest gstack that may read the file; see the compatibility rule above. |
 | `train` | `{ gate_workflow, free_workflow, full_gate_check, record_artifact, max_size, fill_hours, screen_selection, trial_reservation_usd }` or `null` | `null` | Turns on [merge trains](#run-a-merge-train), read from the base revision only. Every field is required and an unknown field is `POLICY_INVALID`: `max_size` 1 to 6, `fill_hours` above 0 and at most 24, `screen_selection` `gstack-paid` \| `none`, `trial_reservation_usd` above 0 and at most 100. `null` keeps the per-PR queue exactly as it is. Needs `min_gstack_version`. |
 
@@ -568,10 +569,95 @@ which carry the workflow consequences the skill prose would otherwise spell out:
 | `ship: bump patch: …` | Step 12 never asks for MINOR or MAJOR; PATCH or MICRO by size |
 | `ship: preregistration: …` | `gstack-evidence ancestor --from-policy --base <base>` must print only `ANCESTOR: ok` before push |
 | `ship: release_tool: …` | the command runs only inside `gstack-restamp` (staging worktree, outputs confined to `release_outputs`) |
+| `ship: review_scope …` | Step 9.4 item 0 runs `gstack-review-scope classify` on each pass's findings; see [review_scope](#review-scope) |
 
 The PR body's first section is the ship receipt (`gstack-ship-receipt write … --body`),
 written by the pr-body section of `/ship` after the body is composed.
 
+<a id="review-scope"></a>
+## `review_scope`: how the review loop converges
+
+Since 1.91.249.0. `/ship` Step 9 runs the pre-landing review in passes and fixes
+what it finds between them ([review-policy.md](review-policy.md) is the canonical
+statement). Under `review_scope: frozen` (the default) the loop converges by
+construction:
+
+1. **Pass 1 reviews the whole diff.** Every finding goes through
+   `gstack-review-scope classify --base <base> --pass 1 --findings <file>`, which
+   gives each one a `locus` (`diff`: a hunk the PR changed, with three lines of
+   context; `outside`: anything else), a `kind`, `blocking` and a `scope`.
+   `blocking` is a CRITICAL defect: a test that would fail, a production break, a
+   security or data-loss defect. Everything else (an advisory, an informational
+   item, a test gap, a wording note) is below the floor. Pass 1 fixes findings in
+   the diff (`scope: fix`). A blocking finding in code the PR did not change is
+   `scope: defer`: `gstack-review-scope defer` files it as a GitHub issue (reusing
+   an open issue that carries the same `<!-- gstack-deferred-finding: <fingerprint> -->`
+   marker; `unfiled (<reason>)` without `gh`) and `render` writes it under
+   `## Deferred findings` in the PR body. Below-floor findings outside the diff,
+   and every below-floor finding from pass 2 on, are `scope: note` and go under
+   `## Review notes` in the PR body.
+2. **Pass 2 and later review the range since the previous pass.** Each pass saves
+   `PASS_HEAD`; the next pass sets `RANGE_BASE` to it, diffs `RANGE_BASE..HEAD`
+   for every reviewer (specialists, Red Team, the outside voice) and classifies
+   with `--range-base <sha>`. Only a blocking finding in that range (`locus:
+   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.
+
+`gstack-review-scope summary --since <REVIEW_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 @<file>`): `review_loop: { passes,
+blocking_found, deferred, noted, fixed, scope }`, printed on the receipt summary
+line as `passes=<n> blocking_found=<k> deferred=<m>`, so a scorecard can track
+passes per PR.
+
+The floor and the pass structure are stated once, in
+[review-policy.md](review-policy.md); this section documents the knob.
+
+`review_scope: diff-plus-blocking` is the documented fallback for a repository
+where deferred issues pile up: the same passes, range and cap as `frozen`, but a
+blocking finding in code the PR did not change is `scope: fix` in this PR
+instead of `defer`. `review_scope: all` is the opt-back to the earlier loop:
+every finding is `scope: fix` on every pass, reviewers read the whole diff each
+time, and only the three-cycle fix cap stops the loop. `show-effective` prints the choice as a `ship: review_scope …`
+line, and the codes a refusal ends with are
+[`REVIEW_SCOPE_INPUT`](troubleshooting.md#review-scope-input) and
+[`REVIEW_SCOPE_RANGE`](troubleshooting.md#review-scope-range).
+
+<a id="receipt-carry"></a>
+## Carrying a receipt across a base merge or a test-only fix
+
+Since 1.91.249.0. A `/ship` receipt names the head it reviewed, so a conflict-only
+merge of the base or a test-only fix used to invalidate it and cost a full
+re-run on identical product code. `gstack-ship-receipt carry --pr <n> --base <base>`
+(or `--body <file>`) moves the receipt to HEAD when the receipt's head is an
+ancestor of HEAD and `git patch-id` of the product diff against the base is the
+same at both heads. Product means every changed path that is not a test
+(`test/**`, `**/*.test.*`, `**/*.spec.*`, `spec/**`, `e2e/**` and the other
+`CARRY_TEST_GLOBS` in `lib/ship-receipt.ts`) or release metadata (`VERSION`,
+`CHANGELOG.md`, `package.json`, the lockfiles, `agents-digest/**`); a base
+merge contributes no product change of its own. The carried receipt records
+`carry: { from, reason, product_patch_id, changed_paths, merge_commits,
+range_diff, covers, reruns }` with the `git range-diff` evidence, prints
+`CARRY: carried from <old> to <new> (<reason>)` and the `GSTACK_RESULT:
+skill=ship status=complete` line, and its summary line ends with
+`carried_from=<old12>`. `CARRY: current` means the receipt already names HEAD;
+`--dry-run` prints `CARRY: would carry …` and writes nothing.
+
+A changed product path, or a product patch-id that differs, is
+[`RECEIPT_CARRY_PRODUCT_CHANGED`](troubleshooting.md#receipt-carry-product-changed)
+(exit 3; the refusal names the paths), and a receipt whose head is not in HEAD's
+history (a rebase or force-push) 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
+carried receipt like any other.
+
 ## What `/land-and-deploy` reads from it
 
 Step 3.4 runs `show-effective --base <base>` before the merge and prints one

docs/troubleshooting.md

@@ -4231,6 +4231,58 @@ has incomplete coverage, never a clean result.
 **Fix.** Rerun the state-machines specialist on the same diff and check its
 output again.
 
+<a id="review-scope-input"></a>
+### `REVIEW_SCOPE_INPUT`
+
+**Meaning.** `gstack-review-scope` (the finding scope of /ship's review loop,
+[ship-policy.md#review-scope](ship-policy.md#review-scope)) was given input it
+cannot classify: the findings were not a JSON array of objects, `--pass` or
+`--policy` was outside its values, or a pass 2+ call came without
+`--range-base`. Nothing was classified, deferred or summarized.
+
+**Fix.** Pass the Step 9.3 findings as a JSON array (`--findings <file>` or
+stdin), `--pass <n>` from the invocation record, and from pass 2 on
+`--range-base <sha>`, the HEAD the previous pass saved when it started.
+
+<a id="review-scope-range"></a>
+### `REVIEW_SCOPE_RANGE`
+
+**Meaning.** `gstack-review-scope classify` could not read the diff it scopes
+findings against: `origin/<base>` or `--range-base` does not resolve, or the
+range base is not an ancestor of HEAD (history was rewritten since the previous
+pass).
+
+**Fix.** Fetch `origin/<base>`, check `--range-base` against the pass heads in
+the invocation record, and start a full pass (`--pass 1`) when the branch was
+rebased; /ship's own rule is to merge the base in, never rebase a reviewed head.
+
+<a id="receipt-carry-refused"></a>
+### `RECEIPT_CARRY_REFUSED`
+
+**Meaning.** `gstack-ship-receipt carry` could not carry the PR's /ship receipt
+to HEAD: the PR body has no valid receipt, the receipt's head is not an ancestor
+of HEAD (a rebase or force-push dropped it), or git could not compare the two
+product diffs. The receipt in the body is unchanged and still names the old
+head, so `gstack-ship-queue ready` refuses `QUEUE_RECEIPT_STALE`.
+
+**Fix.** Keep the reviewed head in HEAD's history (merge the base in instead of
+rebasing), then rerun `gstack-ship-receipt carry --pr <n>`. When the history is
+already rewritten, run /ship on the new head.
+
+<a id="receipt-carry-product-changed"></a>
+### `RECEIPT_CARRY_PRODUCT_CHANGED`
+
+**Meaning.** Between the receipt's head and HEAD the product diff against the
+base changed: at least one path that is neither a test, release metadata nor
+part of the base merge has a different patch than the reviewed head had. The
+reviewed verdict does not cover HEAD, so the receipt is not carried. The
+refusal names the changed product paths.
+
+**Fix.** Run /ship on the new head. The receipt carries only across a
+conflict-only base merge, a test-only change or release metadata, which the
+`carry` object in the carried receipt records with the product patch-id and
+the range-diff evidence ([unattended.md](unattended.md#the-terminal-line)).
+
 ## gstack-error-capture
 
 `gstack-error-capture` runs one target CLI for `/devex-review` Step 3 with no

docs/unattended.md

@@ -111,6 +111,23 @@ Not every workflow writes this line or a run directory today:
   parent pushes those commits itself, then reruns the receipt write, the body
   post and `result` at the pushed head: a receipt for an unpushed head is
   `QUEUE_RECEIPT_STALE` to the queue.
+  The receipt's `review_loop` object (`passes`, `blocking_found`, `deferred`,
+  `noted`, `fixed`, `scope`), written from `gstack-review-scope summary`, is
+  the review loop's machine-readable outcome, and the summary line a parent
+  reads prints it as `passes=<n> blocking_found=<k> deferred=<m>`; the PR body
+  lists the deferred issues under `## Deferred findings` and the below-floor
+  items under `## Review notes` ([review-policy.md](review-policy.md)).
+
+  When the parent merges the base in or fixes a test after the receipt was
+  written, it runs `gstack-ship-receipt carry --pr <n> --base <base>` instead
+  of another /ship: `CARRY: carried from <old> to <new> (<reason>)` plus the
+  `GSTACK_RESULT: skill=ship status=complete run=<dir>` line means the receipt
+  now names HEAD with a `carry` object holding the `git range-diff` evidence
+  (`ship-run.json` gains `carried_from` and `carry_reason`); `CARRY: current`
+  means nothing moved; `CARRY: refused (RECEIPT_CARRY_PRODUCT_CHANGED)` or
+  `(RECEIPT_CARRY_REFUSED)` with the refused result line means the product diff
+  changed or the reviewed head left HEAD's history, so /ship runs again
+  ([ship-policy.md](ship-policy.md#receipt-carry)).
 - **Every other workflow skill** (`/review`, `/qa`, `/design-review`,
   `/spec`, `/land-and-deploy` and the rest) gets the session rules above (no
   prompts, approvals stay pending, nothing published), but has no step that