mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-03 07:18:03 +01:00
v1.91.256.0 feat(ship): review loop converges (frozen scope, 3-pass cap, NO FINDINGS) and receipts carry across conflict-only merges (#3481)
* 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>
* test(ship): keep the Step 9 first-entry and Fix-First phrasing the skip/requeue contract asserts; fill the review record's pass, review_scope and range_base placeholders in the caller template test; narrow a test type
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
* fix: pre-landing review fixes (pass 1) and the review-loop budget wording
Two blocking findings from /ship's own pass 1, both reproduced in the QA smoke before the fix:
gstack-ship-receipt carry hashed the product diff with git patch-id --stable, which strips
whitespace, so a space moved in a reviewed shell line carried as "no product change"; and it
classed package.json and every lockfile as release metadata, so a dependency swap carried too.
The patch-id is now --verbatim, package.json and VERSION are metadata only while the change is a
version stamp (lib/pregate/stamp-only.ts), lockfiles are product unless the policy names them,
the test globs name code extensions, a dirty working tree refuses before anything is compared,
and a receipt carried twice keeps the reviewed head as carry.from.
gstack-review-scope: an unresolvable base is POLICY_SOURCE_UNAVAILABLE instead of a silent
working-tree read; --range-base is validated; git runs with core.quotePath=false; range_base is
null on pass 1; a blocking finding with no path is fix; category "testing" is a test finding
below the floor; defer searches once, files nothing on a failed search and exits 0 unfiled;
summary refuses zero matching passes and the receipt write is chained to it with &&; a finding
the diff causes elsewhere names the changed line in `cause` and classify follows it.
Review-loop budget wording (ideas from tstack's sol-loop and pstack's interrogate, rewritten;
NOTICE.md): the blocker / should / nit glossary and two stop rules in Step 9.4, the checklist
and docs/review-policy.md; a lead-judgment stage in the finding merge both /review and /ship
render. The core read diffs $RANGE_BASE from pass 2 on. Caps and pins re-measured with reasons.
Step 7 coverage audit: four tests added in existing files, the --review-loop case moved to its
own test (the shared test tripped bun's 5 s default).
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
* fix: pin the renumbered review-army stages and point the wording test at product code
CI at c3e27fc failed three free-tier tests, all this branch's own: the
workflow-required-steps fixture and shared-libs-rendering still pinned the
pre-Lead-judgment stage numbers, and test-of-test-ratchet counted
review-loop-budget-wording.test.ts as test-only code because it read only
generated markdown. The fixture and rendering test now pin stages 1-8, and the
wording test renders both armies through generateReviewArmy and reads the
authored ship template as well as the generated sections.
Pass-2 maintainability notes folded in: docs/troubleshooting.md names the two
new RECEIPT_CARRY_REFUSED causes (dirty tree, unreadable status) and the two new
REVIEW_SCOPE_INPUT causes (non-hex --range-base, summary with no matching
pass); gstack-review-scope's help states that classify reads the base branch's
policy and refuses an unresolvable base unless --policy is passed.
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
* fix: date a review-log row from its binding start when it has no timestamp
The Step 9.4 template asks the writer to fill `timestamp`; the logger stamps
`review_binding.started_at` but never `timestamp`. `gstack-review-scope
summary` dropped such a row silently, so this PR's own pass 2 was missing from
REVIEW_LOOP (passes=1 for a two-pass loop). A row now dates from `timestamp`,
else from its binding start; a row with neither is still not counted.
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
* fix: pass-4 review fixes (carry stamp rule, deleted-file locus, scanned defer, diff prefixes, row time)
Five findings at the blocking floor from the Step 11 outside voice (gpt-6.1-sol)
and the native adversarial pass, fixed as a fourth pass approved by GStack
Minder under the three-pass policy:
- carry: a path the policy names under stamp_files, stamp_paths or
release_outputs is metadata only while its change is a stamp (versionStampOnly
for a manifest kind, stampOnlyText otherwise), the rule the tree receipt
applies; a logic change in a stamped source file is product. Every
metadata-listed path is read at both heads and a non-stamp change drops its
globs from the product exclusion.
- review-scope: parseHunks keeps a deleted file as a whole changed location, so
a cause anchored in a removed export is the diff's to fix, not deferred.
- defer: issues are filed through lib/gstack-post's publication scan; a HIGH
finding in the title or body refuses and a MEDIUM one needs a human, both
recorded as unfiled with the reason; --repo names the destination.
- classify: both diffs pin --src-prefix=a/ --dst-prefix=b/ --no-textconv, so
diff.mnemonicPrefix cannot send every finding outside.
- summary: a row dates from review_binding.started_at before the writer-typed
timestamp, so a date-only value cannot drop a real pass.
Docs: ship-policy.md#receipt-carry, review-policy.md, the Step 9.4 template
and the CHANGELOG gate-integrity note record the rule and the pass.
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
* test(review-army): assert the Red Team branch the run took in the N+1 case
Under the blocking floor the Performance specialist rates a 12-line N+1
INFORMATIONAL, so the conditional Red Team does not activate; the case
encoded the old severity (N+1 is CRITICAL, a Red Team always follows). It
now asserts the branch the run took: a Red Team that ran must follow the
Performance specialist; no Red Team is correct only when the report's own
count says 0 critical and its Red Team line names that reason. The N+1
detection assertion is unchanged. The free contract test gains the
informational-not-activated and informational-claimed controls and the
budget test's synthetic report carries the count line.
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
* v1.91.256.0 stamp
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:
1 parent
b004c06dbc
commit
576839cb8c
48 files changed
+2638
-186
No files matched your search
@@ -0,0 +1,124 @@
|
||||
# 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.
|
||||
|
||||
## `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. **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.
|
||||
|
||||
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 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.
|
||||
|
||||
## 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.
|
||||
@@ -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`. |
|
||||
|
||||
@@ -570,10 +571,110 @@ 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 the release that added `review_scope` (`REVIEW_SCOPE_KEY_VERSION` in
|
||||
`lib/ship-policy.ts`). `/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; the title and body go through the same publication scan as the PR
|
||||
body, so a finding that quotes a credential stays `unfiled (publication scan
|
||||
refused: …)`; `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 (the core checklist, specialists, Red Team, the outside
|
||||
voice; the gate scan and QA probes keep the whole candidate) 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 the release that added `review_scope`. 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 working tree is clean, the receipt's head is an
|
||||
ancestor of HEAD and `git patch-id --verbatim` of the product diff against the
|
||||
base is the same at both heads (verbatim: whitespace counts, hunk line numbers
|
||||
do not). Product means every changed path that is not test code (`test/**`,
|
||||
`spec/**`, `e2e/**`, `**/*.test.<code ext>`, `**/*.spec.<code ext>` and the
|
||||
other `CARRY_TEST_GLOBS` in `lib/ship-receipt.ts`; an `openapi.spec.yaml` is
|
||||
product) or release metadata (`CHANGELOG.md`, `agents-digest/**`, and `VERSION`
|
||||
or `package.json` only while the change is a version stamp and nothing else: a
|
||||
dependency, a script or a new key makes them product). A path the policy's
|
||||
`stamp_files`, `stamp_paths` or `release_outputs` name is metadata only while
|
||||
its change is a stamp, the rule the tree receipt applies to the same paths: a
|
||||
manifest by its version key, any other file when its new text reads as the old
|
||||
once the new release version is replaced by the old one. A logic change in a
|
||||
stamped source file is product. Lockfiles are product unless the policy names
|
||||
them. A base merge contributes no product
|
||||
change of its own. A receipt carried twice keeps the reviewed head as
|
||||
`carry.from` and adds up the merges and paths. 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) 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
|
||||
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
|
||||
|
||||
@@ -4298,6 +4298,67 @@ 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, a pass 2+ call came without
|
||||
`--range-base`, `--range-base` was not a commit id (7 to 64 hex digits), or
|
||||
`summary --since` matched no review-log row of this branch written via /ship
|
||||
(so no `REVIEW_LOOP:` line could be derived, and the ship receipt's
|
||||
`--review-loop` input was not written). 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, as a
|
||||
hex commit id. For `summary`, check `REVIEW_SINCE` against the invocation
|
||||
record's start time and that each pass was finished with `via:"ship"`.
|
||||
|
||||
<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), the working tree has uncommitted
|
||||
or untracked changes (a carry certifies a commit, never a tree that differs from
|
||||
it), or git could not compare the two product diffs or read the working tree's
|
||||
status. 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), commit or stash every working-tree change, then rerun
|
||||
`gstack-ship-receipt carry --pr <n>` at the new head. 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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user