CalvinBackup/gstack
1
0 Fork 0
mirror of https://github.com/garrytan/gstack.git synced 2026-10-31 22:14:25 +01:00
Code Issues Packages Projects Releases Wiki Activity

feat(ship): /ship and /autoplan size their review to the diff (GSTA-131) (#3527)

Browse Source 
* wip(GSTA-131): review depth, fix-check transitions, dispatch counters, carry pregate, autoplan round policy

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

* wip(GSTA-131): reviewers stay at full effort; audits light

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

* feat(ship): Step 9 dispatches what gstack-review-plan plans; fix-check after below-floor fixes; cap two full passes plus one fix-check

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

* test: review-dispatch-compare has a free lane; review-depth fixtures registered free; resumed max_rounds case off the old default

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

* fix(GSTA-131): ship review pass-1 repairs: fix-check binds under its own skill, Step 11.5 accepts a fix-check or a planned adversarial skip, delta carry with a pregate, carry --pregate pinned to the checkout, quiet-after-cap revise regates, skill-dir markdown and rule docs classed code, gate signals from tests class up, plan fails closed on git errors, per-class compare ratios

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

* fix(GSTA-131): ship review pass-2 repairs: Step 11 follows the last full-pass plan, Important Rules restored, ship goldens regenerated, true per-class medians, review-policy transitions and once-per-invocation voices, docs/** assets stay docs-class, CLI and fix-check binding tests

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

* docs(changelog): GSTA-131 review depth, fix-check, delta /ship and autoplan rounds

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

* docs: ship-policy cap and counters, review-policy /review cycle cap, unattended passes (docs audit)

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

* fix(review): 400-word findings cap fits the /review size pins (five host pins lowered by 2); CHANGELOG max_rounds wording

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

* docs(changelog): gate integrity names the size-ledger entry and the lowered /review pins

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

* test: the round-budget case asserts the default through ROUNDS output only (pregate literals heuristic)

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

* test: curate ship-receipt-carry-pregate out of the Windows lane like its --pregate siblings

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

* docs(changelog): end the Windows curation sentence instead of a mid-sentence colon

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

* fix(ship): one decision each for the review-loop exits, the planned adversarial skip and Step 16's TODOS rerun (screen judge repair)

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

* fix(ship): fix-check repairs: pin the renamed Step 11 heading, keep the below-floor exit at the cap, name the plan run a repeat reuses, print the native skip on outside-only plans

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

---------

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
Garry Tanandcapy-ai[bot] authored and GitHub committed 2026-10-28 05:45:54 +00:00
1 parent 44f19d0106
commit 148d28ea81
68 files changed
+2069 -266
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

docs/model-policy.md
+16
View File
@@ -154,6 +154,22 @@ set, because a higher effort would spend the time the gate is sized for.
`gstack-models list` prints the effective effort and where it came from
(`--json`: `codexEffort`). The Claude Code runner takes no effort setting.
### Review effort tiers
Every reviewer that can block a ship runs at `full` effort on the lane's
model: the core checklist, each specialist, the Red Team, the native
adversarial pass and the fix-check. `gstack-review-plan` names each
dispatch's effort, and `host` for the outside voice (its provider and
`codex_effort` above). Only work that cannot block runs `light`, the same
model at a medium or low setting and never a cheaper model family: the coverage,
plan-completion and docs audits, decision-trail reviewers and watchers
(`AUDIT_EFFORT:` in the plan's output). A paid replay showed why reviewers stay
at full: a medium-effort fix-check missed a fail-open in gate code that the same
check at full effort caught. A host that cannot set a subagent's effort runs it
at its default. `gstack-config set review_effort_tiers off` (default `on`)
runs the audits and watchers at the host default too
([review-policy.md](review-policy.md#review-depth)).
## Which workflows use the tiers?
The plan-review role applies to autoplan's CEO, design, DX and engineering
docs/review-policy.md
+89 -16
View File
@@ -87,22 +87,85 @@ incomplete coverage, which blocks until the reviewer is rerun.
(the changed line behind a defect reported elsewhere, a deleted file included)
places it in the diff.
Below-floor findings outside the diff are `note`, listed under `## Review notes`.
2. **Pass 2 and later review only the range since the previous pass.** Each pass
saves its head; the next pass diffs `RANGE_BASE..HEAD` for every reviewer (the
gate scan and QA probes keep the whole candidate) and classifies with
`--range-base`. Only a blocking finding in that range or in the diff is `fix`;
every below-floor finding is a `note`. A finding the diff causes in code it did
not change names the changed line in `cause`, and the locus follows it.
3. **Three passes is the cap.** A pass that applies no edits is the exit: the
PR ships with its deferred and noted items named in the body. A blocking
finding still in `fix` scope on pass 3 stops the run and names the finding
(path, severity, problem) for the owner; a fourth pass is the owner's call,
never the loop's.
2. **What comes next is the transition table below**, not "any fix earns
another pass". A full pass after pass 1 reviews only the range since the
previous pass: each pass saves its head, the next diffs `RANGE_BASE..HEAD`
for every reviewer (the gate scan and QA probes keep the whole candidate) and
classifies with `--range-base`. Only a blocking finding in that range or in
the diff is `fix`; every below-floor finding is a `note`.
3. **Two full passes is the cap**, plus at most one fix-check. A blocking
finding still open after the last allowed step stops the run and names the
finding (path, severity, problem) for the owner; a third full pass is the
owner's call, never the loop's. The cap approves nothing.
<a id="transitions"></a>
### Transitions
`gstack-review-plan` (`lib/review-depth.ts` `nextStep`) owns this table, and
/ship asks it after every step:
| The step that just finished | Next |
|---|---|
| none | full pass 1 on the whole diff |
| a full pass with no edits | done |
| a full pass that fixed a blocking finding, below the cap | full pass on the range |
| a full pass whose fix touched a gate-class path, below the cap | full pass on the range |
| a full pass that fixed only below-floor findings | one fix-check |
| a full pass at the cap that fixed a blocking finding | one fix-check |
| a full pass that leaves a blocking finding open (fixed or not) | stop: name it for the owner |
| a fix-check that found nothing blocking | done |
| a fix-check that found a blocking finding | stop: name it for the owner |
A **fix-check** is one reviewer reading the touched hunks, the fix list, the
test results and the callers of every changed symbol, at full effort. It is
logged as a `review-fix-check` row, so it never counts as a pass. No army, no
Red Team and no outside voice run on it.
The same floor and the same `NO FINDINGS` rule apply to `/review` on its own;
its re-review loop keeps its three-cycle fix cap (`CYCLES`), and a pass that
applies no edits is its exit too.
<a id="review-depth"></a>
## Review depth by diff class
`gstack-review-plan --base <base>` classifies the diff and prints the reviewers
the next step dispatches (`REVIEW_CLASS:`, `REVIEW_PLAN:`, one `DISPATCH:` per
reviewer with its effort, one `SKIP:` per reviewer left out, `--json`). A mixed
diff takes its most restrictive class; a `gstack-diff-scope` error fails closed
to code, and code with the gate scan unavailable fails closed to gate.
| Class | What it is | Pass 1 dispatches |
|---|---|---|
| **docs** | markdown no agent or gate reads: README, CHANGELOG, TODOS, how-to and explanation docs | the parent's core checklist only |
| **tests** | test files and fixtures only | core plus the testing specialist |
| **code** | everything else, including every `SKILL.md` and template, any markdown in a skill directory, `*/sections/**`, `scripts/resolvers/**`, `docs/*-policy.md`, `docs/captain.md`, `docs/unattended.md`, `docs/pregate.md`, checklists, specialists and bins | specialists by scope (as before), the native adversarial pass, the outside voice; the Red Team past 200 lines or after a CRITICAL |
| **gate** | code plus auth or migration scope, a hunk the gate scan tags (an RH rule: a relaxed threshold, a skipped test), a deleted code file, CI workflows, credential or state-root paths | code's set, plus the Red Team always, at full effort, and no hit-rate gating |
**Numbers keep the outside voice.** On a docs or tests diff the outside voice is
skipped unless the diff publishes numeric claims: `gstack-review-plan` scans the
added lines of every path for percentages, money, multipliers, durations, token
or credit counts and score, rate or speed claims (`NUMBERS: yes`), and a
claim always dispatches it. The claims' fingerprint (`fingerprint=`) is passed
back as `--reviewed-numbers`; when a later fix changes a claim, only the
outside voice reruns, on the numbers.
**The adversarial and outside voices run once per invocation** (Step 11, on the
final tree), as the last full-pass plan lists them; after they ran, decision 2
passes `--adversarial-ran` so a repeat lists them no more. Later full passes rerun the
specialists on the range only. Step 11.5 binds a planned skip through the Step 9
record's `CURRENT` freshness instead of a native record.
**Effort.** Every reviewer that can block a ship (the core checklist,
specialists, the Red Team, the native adversarial pass, fix-checks) runs at
`full` effort. Work that cannot block (the Step 7 coverage audit, the Step 8
plan-completion audit, the Step 14.5 docs audit, decision-trail reviewers,
watchers) runs at `light`: the same model at a lower reasoning effort, never a
cheaper model family. The outside voice keeps its own provider and effort
(`codex_effort`). A host that cannot set a subagent's effort runs it at its
default. `gstack-config set review_effort_tiers off` (or `--no-tiers`) runs
the audits and watchers at the host default too
([model-policy.md](model-policy.md#reasoning-effort)).
## What the receipt records
`gstack-review-scope summary --since <REVIEW_SINCE>` reads this invocation's
@@ -111,15 +174,24 @@ puts in the receipt:
| Field | Meaning |
|---|---|
| `review_loop.passes` | review passes this ship ran (1 to 3) |
| `review_loop.passes` | full review passes this ship ran (1 or 2; up to 3 under `review_scope: all`) |
| `review_loop.blocking_found` | distinct blocking findings across the passes |
| `review_loop.deferred` | blocking findings in unchanged code filed as issues (a Step 11 late defer, logged as a `review-late-defer` row so it never displaces the newest `review` pass, counts here and is not a pass) |
| `review_loop.noted` | below-floor findings listed in the PR body |
| `review_loop.fixed` | findings fixed in this PR |
| `review_loop.scope` | the `review_scope` the loop ran under |
| `review_loop.fix_checks` | fix-checks this ship ran (never passes) |
| `review_loop.dispatches` | reviewer dispatches by role: `specialist`, `red-team`, `adversarial-native`, `outside-voice`, `fix-check` |
| `review_loop.est_input_tokens` | prompt plus diff bytes the dispatches read, divided by 4: a relative estimate for comparing runs, not a bill |
Each review row may carry `dispatches: [{role, effort, prompt_bytes,
diff_bytes, status}]`; `summary` adds them up (older rows fall back to the
specialist stats they record). The summary line appends `fix_checks=`,
`dispatches=` and `est_input_tokens=` only when they were recorded, so an old
loop prints the old line.
The receipt's summary line prints `passes=<n> blocking_found=<k> deferred=<m>`,
which is what a scorecard tracks (target: a median of 2 passes per PR). A
which is what a scorecard tracks (target: a median of 1 pass per PR). A
receipt carried across a base merge or a test-only fix
([ship-policy.md#receipt-carry](ship-policy.md#receipt-carry)) keeps these
counters, because the reviewed product diff is unchanged.
@@ -130,8 +202,8 @@ counters, because the reviewed product diff is unchanged.
| Value | Pass 1 | Blocking finding in unchanged code | Pass 2+ | Cap |
|---|---|---|---|---|
| `frozen` (default) | the diff | filed as an issue, listed as deferred | the range since the previous pass, blocking only | 3 passes |
| `diff-plus-blocking` | the diff | fixed in this PR | the range since the previous pass, blocking only | 3 passes |
| `frozen` (default) | the diff | filed as an issue, listed as deferred | the range since the previous pass, blocking only | 2 full passes + 1 fix-check |
| `diff-plus-blocking` | the diff | fixed in this PR | the range since the previous pass, blocking only | 2 full passes + 1 fix-check |
| `all` | everything | fixed in this PR | the whole diff, every finding | 3 fix cycles |
`diff-plus-blocking` is the fallback for a repository where deferred issues pile
@@ -198,7 +270,8 @@ trailers, appended }`.
- `scripts/resolvers/review-army.ts`: the floor and `NO FINDINGS` in every specialist and Red Team prompt (rendered into `review/sections/review-army.md` and `ship/sections/review-army.md`), and the lead judgment's "On record" line that turns a [ruled class](#rulings-below-the-floor) into a note; in `/ship` every specialist and Red Team prompt it renders carries its diff base as a literal sha the parent writes in (`DIFF_BASE=<diff-base>`: the merge base on pass 1, `RANGE_BASE` from pass 2 on), because a reviewer is its own process and never sees the parent's shell variables; the fan-out batches above.
- `ship/sections/review-army.md` Step 9: passes, `RANGE_BASE`, classify, defer, render, the cap check before any edit, the Step 11 late-defer row, scratch files under `.gstack/tmp/`, the cap decisions; `ship/sections/adversarial.md` Step 11: the outside voice reads the same range and scopes queued fixes the same way.
- `lib/review-scope.ts`, `bin/gstack-review-scope`: `classify`, `defer`, `render`, `summary`; refusals are `REVIEW_SCOPE_INPUT` and `REVIEW_SCOPE_RANGE` ([troubleshooting.md](troubleshooting.md#review-scope-input)).
- `lib/review-depth.ts`, `bin/gstack-review-plan`: the diff class, the transition table, the numeric-claim scan and each role's effort tier (`test/review-depth.test.ts`, `test/review-plan-cli.test.ts`).
- `lib/review-scope.ts`, `bin/gstack-review-scope`: `classify`, `defer`, `render`, `summary` (with `fix_checks`, `dispatches`, `est_input_tokens`); refusals are `REVIEW_SCOPE_INPUT` and `REVIEW_SCOPE_RANGE` ([troubleshooting.md](troubleshooting.md#review-scope-input)).
- `lib/ship-policy.ts`: the `review_scope` key and its `ship: review_scope …` consequence line.
- `review/checklist.md`: `prereg-with-results` under Gate Integrity and `test-keyboard-count-loop` under Pass 2; `bin/gstack-coauthor-trailers` for the contributor-credit trailers of a port.
- `lib/ship-receipt-prose.ts`, `gstack-ship-receipt read --check-prose`: the body's quoted counters against the receipt (`RECEIPT_PROSE_MISMATCH`, [ship-policy.md](ship-policy.md#ship-receipt)).
docs/ship-policy.md
+28 -8
View File
@@ -860,15 +860,18 @@ construction:
fix-range`) or in the diff is `scope: fix`; nothing below the floor earns a
pass. `NO FINDINGS` is the expected reviewer result for a clean range, never a
failed review.
3. **Three passes is the cap.** A pass that applied no edits continues to Step 10:
the loop ships with its deferred and noted items named in the PR body. A
blocking finding still in `fix` scope on pass 3 stops the run and names the
finding (path, severity, problem) for the owner instead of a fourth pass.
3. **Two full passes plus one fix-check is the cap**, stepped by the
[transition table](review-policy.md#transitions). A pass that applied no edits
continues to Step 10: the loop ships with its deferred and noted items named in
the PR body. A blocking finding still open after the last allowed step stops
the run and names the finding (path, severity, problem) for the owner instead
of a third full pass.
`gstack-review-scope summary --since <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
blocking_found, deferred, noted, fixed, scope }`, plus `fix_checks`, `dispatches`
and `est_input_tokens` when the run recorded them, printed on the receipt summary
line as `passes=<n> blocking_found=<k> deferred=<m>`, so a scorecard can track
passes per PR.
@@ -933,11 +936,28 @@ A changed product path, or a product patch-id that differs, is
(exit 3; the refusal names the paths), and a receipt whose head is not in HEAD's
history (a rebase or force-push) or a working tree with uncommitted changes is
[`RECEIPT_CARRY_REFUSED`](troubleshooting.md#receipt-carry-refused): run `/ship`
on the new head. `/ship` Step 1 runs the carry check itself when the branch has
an open PR, and a `CARRY: carried` result ends that run with the receipt bound
to HEAD; the queue's `ready`, `train build` and `verify --train` read the
on the new head. The queue's `ready`, `train build` and `verify --train` read the
carried receipt like any other.
**Delta /ship: eligibility, then checks, then publication.** `/ship` Step 1 runs
the carry when the branch has an open PR, in this order, and never reruns the
review loop for a carried receipt:
1. `gstack-ship-receipt carry --pr <n> --base <base> --dry-run`. `CARRY: would
carry` is eligibility only; a refusal continues the full ship.
2. At that pinned HEAD: the CHANGELOG check, the size-cap check the policy
declares, and `gstack-pregate` (the remote tests stage when
`requires_remote` asks for it). A failing check, or HEAD moving, continues
the full ship from Step 2.
3. `gstack-ship-receipt carry --pr <n> --base <base> --pregate <pregate.json>`
rebinds the receipt with a pregate current for HEAD and ends the run.
A receipt that records a pregate never rebinds without a current one:
`carry` without `--pregate` (or with a stale one) is
[`RECEIPT_CARRY_PREGATE_STALE`](troubleshooting.md#receipt-carry-pregate-stale)
and leaves the body unchanged. A crew that reran the whole `/ship` after every
base merge should run this sequence instead.
## What `/land-and-deploy` reads from it
Step 3.4 runs `show-effective --base <base>` before the merge and prints one
docs/troubleshooting.md
+13
View File
@@ -4722,6 +4722,19 @@ run did not produce. The `PROSE:` line names each field with both values.
stale one (the run was repeated), rerun `/ship` so the receipt is rewritten
([ship-policy.md](ship-policy.md#ship-receipt)).
<a id="receipt-carry-pregate-stale"></a>
### `RECEIPT_CARRY_PREGATE_STALE`
**Meaning.** The receipt in the body records a pregate result for its old head,
and `gstack-ship-receipt carry` was asked to rebind it to HEAD without a pregate
result current for HEAD (or the one passed with `--pregate` is not current). A
carry certifies the new head, so it never keeps the old tree's checks. Nothing
was rewritten.
**Fix.** Run `gstack-pregate` at HEAD (the remote tests stage when the policy
requires it), then rerun `carry --pregate <pregate.json>`. `--dry-run` still
reports eligibility without one ([ship-policy.md](ship-policy.md#receipt-carry)).
<a id="receipt-git-too-old"></a>
### `RECEIPT_GIT_TOO_OLD`
docs/unattended.md
+19 -5
View File
@@ -165,14 +165,27 @@ unpushed head is `QUEUE_RECEIPT_STALE` to the queue.
The receipt's `review_loop` object, written from `gstack-review-scope summary
--since <REVIEW_SINCE>`, is the loop's machine-readable outcome: `passes`
(Step 9 passes this ship ran, 1 to 3), `blocking_found` (distinct blocking
findings across them), `deferred` (blocking findings in unchanged code filed
as issues, Step 11's late defers included), `noted`, `fixed` and `scope`. The
(Step 9 full passes this ship ran, 1 or 2; up to 3 under `review_scope: all`),
`blocking_found` (distinct blocking findings across them), `deferred`
(blocking findings in unchanged code filed as issues, Step 11's late defers
included), `noted`, `fixed` and `scope`. The
summary line a parent reads prints it as `passes=<n> blocking_found=<k>
deferred=<m>`; the PR body lists the deferred issues under `## Deferred
findings` and the below-floor items under `## Review notes`. A pass that finds
nothing is `NO FINDINGS` from every reviewer and a pass with no edits, which
is the loop's exit, never a failed review ([review-policy.md](review-policy.md)).
Three counters appear only when the run recorded them: `fix_checks` (fix-check
rows, never passes), `dispatches` (reviewer dispatches by role) and
`est_input_tokens` (prompt plus diff bytes the dispatches read, / 4, a relative
estimate and never a bill).
### Report length
A parent reads many lanes, so length is a cost it pays per lane. A READY block
holds its required lines plus at most 150 words; a completion report holds at
most 300 words; a subagent brief names the files to read instead of pasting
them. Evidence goes in files the report points to. Waits go on operations,
never on polling wakes.
### The outside voice on Capy
@@ -205,8 +218,9 @@ coverage, not as a clean outside review.
### The carry across a base merge or a test-only fix
When the parent merges the base in or fixes a test after the receipt was
written, it runs `gstack-ship-receipt carry --pr <n> --base <base>` instead of
another /ship. `CARRY: carried from <old> to <new> (<reason>)` plus the
written, it runs `gstack-ship-receipt carry --pr <n> --base <base> --dry-run`
instead of another /ship; on `CARRY: would carry` it runs `gstack-pregate` at HEAD
and reruns carry with `--pregate .gstack/tmp/pregate.json`. `CARRY: carried from <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, and
`ship-run.json` gains `carried_from` and `carry_reason`; the `review_loop`