From 6bdafb2cf5acf05e6e9ab23a74e682e155d1fdde Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 4 Nov 2026 17:00:05 -0800 Subject: [PATCH] fix(ship): carry NEXT names the push and CI wait; docs give the real diff-plus-blocking cap (GST-1346 follow-up) (#4006) * docs: diff-plus-blocking stops at the third fix cycle; carry order matches ship Step 1 (Step 14.5) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship): carry NEXT names the push and CI wait; docs give the real diff-plus-blocking cap (GST-1346 follow-up) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship): carry route says push before the dry-run and that CI_CARRY: none is not a stop 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> --- changelog.d/capy-gst-1346-docs-carry-cap.md | 1 + docs/review-policy.md | 4 ++-- docs/ship-policy.md | 23 +++++++++++++-------- docs/unattended.md | 12 +++++++---- lib/ship-preflight.ts | 2 +- test/ship-rereview-triggers.test.ts | 18 ++++++++++++++++ 6 files changed, 44 insertions(+), 16 deletions(-) create mode 100644 changelog.d/capy-gst-1346-docs-carry-cap.md diff --git a/changelog.d/capy-gst-1346-docs-carry-cap.md b/changelog.d/capy-gst-1346-docs-carry-cap.md new file mode 100644 index 000000000..71fc5c380 --- /dev/null +++ b/changelog.d/capy-gst-1346-docs-carry-cap.md @@ -0,0 +1 @@ +- **/ship's carry route now says to push before it carries, and the docs give the real fix-loop cap.** `gstack-ship-preflight`'s carry `NEXT:` line went straight from the pregate to `carry`, which refuses a head that isn't pushed. It now says to commit, push with `gstack-ship-release push --early` (the secrets scan runs first), wait for CI with `gstack-ci-carry wait`, run the pregate and then carry. docs/ship-policy.md and docs/unattended.md give the same order. docs/review-policy.md, docs/ship-policy.md and docs/unattended.md now say `review_scope: diff-plus-blocking` stops at Step 9.4's third fix cycle, because the two-round review-plan cap runs only under `frozen`. (GST-1346) diff --git a/docs/review-policy.md b/docs/review-policy.md index 70bcd3aad..880187dce 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -562,7 +562,7 @@ 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.passes` | full review passes this ship ran (1 or 2; up to 3 under `review_scope: diff-plus-blocking` or `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.unbound` | what could not bind (GST-1168), each printed as `REVIEW_SCOPE_UNBOUND: : `: a row with no start token and no readable time, a `pN` ledger with no review row of pass N, a row finding's `ledger_id` no ledger collected, a `Review-Finding` trailer naming nothing; absent when 0 | @@ -638,7 +638,7 @@ difference passed a scoped review. | 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) | +| `diff-plus-blocking` | the diff | fixed in this PR | the range since the previous pass, blocking only | 3 fix cycles (Step 9.4 runs the review plan's two-round cap only under `frozen`) | | `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 diff --git a/docs/ship-policy.md b/docs/ship-policy.md index cb65139bd..f6f45bb61 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1573,9 +1573,10 @@ 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 +where deferred issues pile up: the same passes and range 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: +instead of `defer`, and the loop stops at Step 9.4's third fixing cycle +(`CYCLES >= 3`): only `frozen` runs the review plan's two-round cap. `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 @@ -1783,21 +1784,25 @@ 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: +review loop or Step 14.5's docs audit for a carried receipt: -1. `gstack-ship-receipt carry --pr --base --dry-run`. `CARRY: would +1. At the pushed HEAD (carry refuses an unpushed one, `--dry-run` included), + `gstack-ship-receipt carry --pr --base --dry-run`. `CARRY: would carry` is eligibility only; a refusal continues the full ship, except `RECEIPT_CARRY_SCOPED_REVIEW`, for which `gstack-ship-preflight` prints `NEXT: carry route`: fix what the base merge broke, commit and push, run carry without `--dry-run` to write the packet, have one fix-check reviewer read it, then carry with `--scoped-review ` and `--pregate`. A blocking finding 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. +2. Fix what the base merge broke (the CHANGELOG, the size caps the policy + declares) and commit, `gstack-ship-release push --branch --early`, + `gstack-ci-carry wait --base ` (`CI_CARRY: none` is not a stop), then + `gstack-pregate --allow-repo-commands` at that pinned HEAD (the remote tests stage when `requires_remote` asks for it). + A failing check, or HEAD moving after the pregate, continues the full ship + from Step 2. 3. `gstack-ship-receipt carry --pr --base --pregate ` - rebinds the receipt with a pregate current for HEAD and ends the run. + rebinds the receipt with a pregate current for HEAD and ends the run with its + `GSTACK_RESULT:` line. A receipt that records a pregate never rebinds without a current one: `carry` without `--pregate` (or with a stale one) is diff --git a/docs/unattended.md b/docs/unattended.md index 5900a770f..71caf1272 100644 --- a/docs/unattended.md +++ b/docs/unattended.md @@ -171,7 +171,7 @@ unpushed head is `QUEUE_RECEIPT_STALE` to the queue. The receipt's `review_loop` object, written from `gstack-review-scope summary --since `, is the loop's machine-readable outcome: `passes` -(Step 9 full passes this ship ran, 1 or 2; up to 3 under `review_scope: all`), +(Step 9 full passes this ship ran, 1 or 2; up to 3 under `review_scope: diff-plus-blocking` or `all`), `blocking_found` (every blocker the readers found, rows and dispatch ledgers both, [review-policy.md](review-policy.md#what-the-receipt-records)), `deferred` (blocking findings in unchanged code filed as issues, Step 11's late defers @@ -252,9 +252,13 @@ missing voice never reads as converged or as an unavailable Codex CLI. ### 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 --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 to ()` plus the +written, it pushes that commit with `gstack-ship-release push --branch --early` +(carry refuses an unpushed HEAD, `--dry-run` included), then runs +`gstack-ship-receipt carry --pr --base --dry-run` instead of another +/ship; on `CARRY: would carry` it commits and pushes any fix the same way, waits with +`gstack-ci-carry wait --base ` (`CI_CARRY: none` is not a stop), runs `gstack-pregate --allow-repo-commands` +at HEAD and reruns carry with `--pregate .gstack/tmp/pregate.json`, the order +`gstack-ship-preflight`'s carry route prints. `CARRY: carried from to ()` plus the `GSTACK_RESULT: skill=ship status=complete head= run=` 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` diff --git a/lib/ship-preflight.ts b/lib/ship-preflight.ts index fb7a498b8..e6537d13a 100644 --- a/lib/ship-preflight.ts +++ b/lib/ship-preflight.ts @@ -96,7 +96,7 @@ function integrate(cwd: string, base: string, run: string | undefined, r: Report const line = (carry.stdout.match(/^CARRY: .*$/m) ?? [`CARRY: check failed (${firstLine(carry.stderr)})`])[0]; r.add(`PR: #${n}`).add(line); if (line.startsWith('CARRY: would carry')) { - r.next(`carry route (Step 1.4): fix what the base merge broke (CHANGELOG, size caps), gstack-pregate --allow-repo-commands, then gstack-ship-receipt carry --pr ${n} --base ${base} --pregate .gstack/tmp/pregate.json`); + r.next(`carry route (Step 1.4): fix what the base merge broke (CHANGELOG, size caps), commit, gstack-ship-release push --branch ${branch} --early, gstack-ci-carry wait --base ${base} (CI_CARRY: none is not a stop), gstack-pregate --allow-repo-commands, then gstack-ship-receipt carry --pr ${n} --base ${base} --pregate .gstack/tmp/pregate.json`); return false; } if (line === 'CARRY: refused (RECEIPT_CARRY_PRODUCT_CHANGED)' && /^RECHECK: eligible /m.test(carry.stderr ?? '')) { diff --git a/test/ship-rereview-triggers.test.ts b/test/ship-rereview-triggers.test.ts index 467d96c7f..d6441f1f5 100644 --- a/test/ship-rereview-triggers.test.ts +++ b/test/ship-rereview-triggers.test.ts @@ -605,6 +605,24 @@ describe('preflight carry route for an adjacent base edit (GST-442)', () => { expect([wrote.status, fs.existsSync(path.join(f.work, '.git', 'gstack', `scoped-review-${head.slice(0, 12)}.md`))]).toEqual([EXIT.refused, true]); }); + // Value: protects=the would-carry NEXT line names the push before the carry, since carry --pr refuses a HEAD that is not the + // PR's pushed head (bin/gstack-ship-receipt), and the early push's secrets scan and CI wait come before the pregate; + // fails_when=the NEXT line sends the agent from the pregate straight to carry; seam=gh; no_claim=the carry itself + test("preflight's would-carry route commits, pushes early and waits for CI before the pregate and the carry", () => { + const src = (edit: Record = {}) => Array.from({ length: 12 }, (_, i) => edit[i + 1] ?? `exports.v${i + 1} = ${i + 1};`).join('\n') + '\n'; + const f = fx({ 'lib/rows.js': src() }, { 'lib/rows.js': src({ 6: 'exports.v6 = 600;' }) }); + const reviewed = f.git('rev-parse', 'HEAD').stdout.trim(); + f.advanceBase({ 'lib/other.js': 'exports.other = 1;\n' }); + mergeMain(f); + const head = f.git('rev-parse', 'HEAD').stdout.trim(); + const view = path.join(f.root, 'view.json'); + fs.writeFileSync(view, JSON.stringify({ body: placeReceiptFirst('## Summary\n\nProse.\n', { schema_version: 1, pr: 9, head: reviewed, base: 'main', queue_mode: 'stamp-at-merge' } as any), headRefOid: head })); + f.gh(`case "$1 $2" in "pr list") echo 9 ;; "pr view") cat '${view}' ;; *) exit 1 ;; esac`); + const r = f.run(PREFLIGHT, ['--base', 'main']); + expect(lineOf(r.stdout, 'CARRY')).toMatch(/^CARRY: would carry/); + expect(r.stdout).toMatch(/^NEXT: carry route \(Step 1\.4\): .*commit, gstack-ship-release push --branch \S+ --early, gstack-ci-carry wait --base main \(CI_CARRY: none is not a stop\), gstack-pregate --allow-repo-commands, then gstack-ship-receipt carry --pr 9 --base main --pregate/m); + }); + // Value: protects=the dry run offers the scoped route only when nothing else would refuse the carry, so preflight never sends // the agent through a commit, packet, reviewer and pregate that end in another refusal (adversarial-native#5, Step 11 // round 2); fails_when=the dry run returns the scoped refusal before the receipt's other carry checks; seam=gh; no_claim=none