mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-12 19:44:48 +01:00
fix(ship): a required edit after review takes one scoped reader, not another review round (GST-1276, GST-1311) (#4020)
* feat(ship): bounded post-review edits take one scoped reader, not a third review round (GST-1276, GST-1311) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(ship): a scoped-review STOP takes one reader, not a fix-check round (GST-1276) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test: bound the diff-scope spawn and record the ship size ledger (GST-1276) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test: repin lib/review-depth.ts on the GST-1276 no-loss replay Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes Review-Finding: p1/specialist-batched#1 reader=specialist-batched Review-Finding: p1/specialist-batched#2 reader=specialist-batched Review-Finding: p1/specialist-batched#3 reader=specialist-batched Review-Finding: p1/red-team#2 reader=red-team Review-Finding: p1/red-team#3 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: a blank line or a template literal no longer hides a changed expected value (review fix-check) Review-Finding: p2/outside-voice#1 reader=outside-voice Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship): a bounded lanes edit changes only dependencies; any other pregate key stops (GST-1276) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * ship: take the receipt-write targeted rebind out of GST-1276 (follow-up); document the lanes key check Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs: list always-run.json as a gate path and finish's bounded=N stop Step 14.5 audit da1-gst1276 corrections. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs: keep review-policy.md at its reviewed bytes main's finish reads docs/review-policy.md as agent prose (behavior); the gate-path list omission of .gstack/always-run.json is recorded as doc debt in the PR body. 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
a4d058b078
commit
e4f2930812
23 files changed
+509
-53
No files matched your search
@@ -648,6 +648,49 @@ refuses a `review-fix-check` row without boolean `completed` and `converged`,
|
||||
before it consumes the start token, because without them the row reads
|
||||
`UNVERIFIED` and finish cannot count it.
|
||||
|
||||
<a id="bounded-post-review-edits"></a>
|
||||
### Bounded post-review edits (GST-1276, GST-1311)
|
||||
|
||||
Some edits are required after the review binds: the Step 14.5 docs audit
|
||||
corrects stale policy text, pregate asks for a lane mapping, and a verify
|
||||
step renames rows in a size baseline. Finish proves each one's kind from
|
||||
the diff and sends it to one scoped reader instead of Steps 5–11.5. That
|
||||
read is not a review round under R51 (Garry, 2026-11-04: "Do it."), so it
|
||||
needs no grant. It is the same stage 2.1 reader that test code changed
|
||||
after review already takes. A path is bounded when finish runs with
|
||||
`--reviewed-base`, the path changed at most 20 lines since review, its
|
||||
diff is readable text, the gate scan tags none of its hunks, and it is one
|
||||
of these kinds:
|
||||
|
||||
- **audit prose**: `docs/*.md` or the root `AGENTS.md` or `CLAUDE.md`,
|
||||
never a gate path, whose bytes now are the ones the post-audit snapshot
|
||||
(`--docs-post`) recorded. An edit made after the snapshot isn't the
|
||||
audit's.
|
||||
- **lanes**: `.gstack/pregate.json` changes that pregate's own
|
||||
`headPolicyDiff` reads as added tests only, over the reviewed copy. A
|
||||
removed mapping, a new `[]` declaration or any other key stops.
|
||||
- **bookkeeping**: a path matching a `bookkeeping` glob in the base
|
||||
branch's ship policy ([ship-policy.md#bookkeeping](ship-policy.md#bookkeeping)),
|
||||
first touched after review or not, a `.txt` table included, never a gate
|
||||
path or a path under `.gstack/` or `.github/`.
|
||||
|
||||
`ROUTE:` prints `bounded=<n>`, and finish writes the packet with every
|
||||
bounded hunk, a reading note per kind and the targeted brief for every
|
||||
class. The reader's clean record must carry the packet's `packet_sha256`,
|
||||
which the STOP line prints. A candidate that fails a proof stays behavior,
|
||||
and finish prints `ROUTE_BOUNDED_REFUSED: <path> <reason> (fix: …)`
|
||||
([troubleshooting](troubleshooting.md#route-bounded-refused)). A lane
|
||||
change whose other top-level keys differ from the reviewed copy stops too. A
|
||||
targeted record read before the edits does not carry to the new head:
|
||||
receipt write still asks for the targeted reader on HEAD. A test file whose only removed lines are swapped
|
||||
one for one with lines that differ inside plain quoted strings, in
|
||||
statements with no assertion or test call, takes the plain test route (the
|
||||
scoped reader) instead of behavior. A changed expected value in the
|
||||
assertion's own statement (a multi-line one too) or in a template literal,
|
||||
code inside an interpolated or prefixed string, or a deleted assertion still
|
||||
stops. A value in a case table that a later test reads takes the scoped
|
||||
reader.
|
||||
|
||||
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
|
||||
|
||||
@@ -67,6 +67,7 @@ refuses the file (`POLICY_GSTACK_TOO_OLD`). `validate` ends with
|
||||
| `train` | `{ gate_workflow, free_workflow, full_gate_check, record_artifact, max_size, fill_hours, screen_selection, trial_reservation_usd, land? }`, `{ gates, land, max_size, fill_hours }` 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. A block with `gates` ([any repository](#train-gates)) takes `max_size` 1 to 5 and needs `min_gstack_version` at least `TRAIN_GATES_KEY_VERSION`. `null` keeps the per-PR queue exactly as it is. Needs `min_gstack_version`. |
|
||||
| `merge` | `train` \| `direct` | `train` | `direct` lands each READY PR at its own green head with [`land --pr <n> --pin <sha>`](#land-direct): no train and no stamp commit, and `train build` refuses. A PR then leaves VERSION, the `package.json` version and the CHANGELOG at its merge base, and the release sets them. Needs `changelog_fragments`, and the stamping `land --pr <n>` refuses under it. `train` keeps merge trains. Read from the base revision only. Needs `min_gstack_version`. |
|
||||
| `land` | `{ merged_checks }` or `null` | `null` | Under `merge: direct`, the shell commands (1 to 16, one line of at most 512 characters each, 15 minutes each) [`land --pin`](#land-direct) runs on the pin merged onto the base tip when the base moved past the pin's merge base; it lands only when every one exits 0. They are repo commands, so land runs them only with `--allow-repo-commands`, never under `--dry-run`. `null` lands a stale base untested with a `STALE_BASE: warn` line. Read from the base revision only. Needs `min_gstack_version`. |
|
||||
| `bookkeeping` | glob list | `[]` | <a id="bookkeeping"></a>Root-anchored globs of bookkeeping tables a verify or ratchet step rewrites (size baselines, serial lists, limits), each ending in `.tsv`, `.csv` or `.txt` and never under `.gstack/` or `.github/` (`["scripts/*-baseline.tsv"]`). A matching file changed at most 20 lines after review binds, first touched or not, takes finish's [one scoped reader](review-policy.md#bounded-post-review-edits) instead of Steps 5–11.5; the reader still reads every changed row. Read from the base revision only. Needs `min_gstack_version`. |
|
||||
| `base_red` | `{ setup, select, timeout_sec }` or `null` | `null` | How `/ship` Step 5 proves a failure is the base branch's own before pausing on it; see [base_red](#base-red). Read from the base revision only. Needs `min_gstack_version`. |
|
||||
| `changelog_fragments` | `{ dir, required_checks? }` or `null` | `null` | PRs carry unversioned [CHANGELOG fragments](#changelog-fragments) in `dir` (one flat, repo-relative directory, no `..`) and the merge step ([`land`](#land)) assigns the version (under `merge: direct`, the [release PR](#release-stamp) does). `required_checks` (up to 50 distinct names) are the checks [`train land`](#stamped-trains-under-changelog_fragments-train-land) requires present and green on a stamp commit. `null` keeps `[Unreleased]` sections and per-PR stamps. Needs `min_gstack_version`. |
|
||||
|
||||
|
||||
+27
-1
@@ -3143,6 +3143,29 @@ table above and end each error line with a code in parentheses. The policy
|
||||
file is described in [docs/ship-policy.md](ship-policy.md); the queue flow in
|
||||
[docs/ship-policy.md#stamp-at-merge](ship-policy.md#stamp-at-merge).
|
||||
|
||||
<a id="route-bounded-refused"></a>
|
||||
### `ROUTE_BOUNDED_REFUSED: <path> <reason> (fix: …)`
|
||||
|
||||
**Meaning.** `gstack-ship-release finish` found a post-review edit of a
|
||||
bounded kind (audit prose, a `.gstack/pregate.json` lane change, or a
|
||||
declared bookkeeping file) that failed one of its proofs, so the path stays
|
||||
behavior and finish stops for Steps 5–11.5
|
||||
([review-policy.md#bounded-post-review-edits](review-policy.md#bounded-post-review-edits)).
|
||||
The reason names the failed proof: `needs --reviewed-base`, `changed <n>
|
||||
lines since review (at most 20)`, `is binary or unreadable`, `carries a
|
||||
hunk the gate scan tagged`, `could not be gate-scanned`, `is not a doc the Step 14.5 audit snapshot
|
||||
recorded`, `is not the bytes the post-audit snapshot recorded (edited after
|
||||
the audit)`, `does not parse at both the reviewed and the current tree`,
|
||||
`loosens the reviewed policy: <what>`, `changes <key>; a lanes edit changes
|
||||
only dependencies`, `adds no lane`, or `is a gate path or under .gstack/ or
|
||||
.github/, never bookkeeping`.
|
||||
|
||||
**Fix.** Do what the line's `fix:` says. Rerun finish with the merge base
|
||||
saved with the reviewed tree. Restore the audited bytes, or the mapping a
|
||||
lane change removed (a `[]` declaration lands on the base first). Or take
|
||||
Steps 5–11.5 for the edit. Never shrink an edit to get under the cap by
|
||||
moving its lines into another file.
|
||||
|
||||
<a id="policy-source-unavailable"></a>
|
||||
### `policy source unavailable — fix: git fetch origin <base> --depth=1 (POLICY_SOURCE_UNAVAILABLE)`
|
||||
|
||||
@@ -6412,7 +6435,10 @@ on the new head. `gstack-ship-release finish` stops the same way with
|
||||
write that record beside the packet, outside the worktree, so it does not change
|
||||
the tree it is bound to. Finish also stops this way for test code changed since
|
||||
review with no gate edit (`tests=N`): the packet holds those test hunks, and the
|
||||
reviewer checks that each test still tests what it did. A packet that cannot be
|
||||
reviewer checks that each test still tests what it did. It stops this way for
|
||||
[bounded post-review edits](review-policy.md#bounded-post-review-edits) too
|
||||
(`bounded=N`): one scoped reader, not a review round, reads the packet, and
|
||||
its clean record must also carry the `packet_sha256` the STOP line prints. A packet that cannot be
|
||||
written (an unwritable `.git`) is a STOP or `RECEIPT_CARRY_REFUSED` naming the path.
|
||||
`carry --dry-run` reports this code without writing the packet, and only when
|
||||
none of carry's other checks would refuse; carry without `--dry-run` writes it.
|
||||
|
||||
Reference in new issue
Block a user