mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 16:34:00 +01:00
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>
This commit is contained in:
1 parent
7c918efcda
commit
6bdafb2cf5
6 files changed
+44
-16
No files matched your search
@@ -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)
|
||||
@@ -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: <what>: <why>`: 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
|
||||
|
||||
+14
-9
@@ -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 <n> --base <base> --dry-run`. `CARRY: would
|
||||
1. At the pushed HEAD (carry refuses an unpushed one, `--dry-run` included),
|
||||
`gstack-ship-receipt carry --pr <n> --base <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 <record.json>` 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 <branch-name> --early`,
|
||||
`gstack-ci-carry wait --base <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 <n> --base <base> --pregate <pregate.json>`
|
||||
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
|
||||
|
||||
+8
-4
@@ -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 <REVIEW_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 <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
|
||||
written, it pushes that commit with `gstack-ship-release push --branch <branch-name> --early`
|
||||
(carry refuses an unpushed HEAD, `--dry-run` included), then runs
|
||||
`gstack-ship-receipt carry --pr <n> --base <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 <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 <old> to <new> (<reason>)` plus the
|
||||
`GSTACK_RESULT: skill=ship status=complete head=<sha> 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`
|
||||
|
||||
@@ -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 ?? '')) {
|
||||
|
||||
@@ -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<number, string> = {}) => 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
|
||||
|
||||
Reference in new issue
Block a user