From e5bb3e20fc4dcff063dbcc4aa2cf47c4de8cde1f Mon Sep 17 00:00:00 2001 From: garrytan Date: Wed, 4 Nov 2026 03:45:28 +0000 Subject: [PATCH] fix(ship-queue): land --pin refuses a stale base with QUEUE_LAND_STALE_BASE (GST-1316) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-doctor-components.sh | 1 + changelog.d/gst-1316-land-stale-base.md | 8 ++++++++ docs/captain.md | 1 + docs/ship-policy.md | 21 ++++++++++++------- docs/troubleshooting.md | 12 +++++++++++ lib/result-codes-release.ts | 5 +++++ lib/ship-queue-land-direct.ts | 8 +++++--- test/ship-queue-land-direct.test.ts | 27 +++++++++++++++++++------ 8 files changed, 67 insertions(+), 16 deletions(-) create mode 100644 changelog.d/gst-1316-land-stale-base.md diff --git a/bin/gstack-doctor-components.sh b/bin/gstack-doctor-components.sh index 5652eab65..0f2fdddf6 100644 --- a/bin/gstack-doctor-components.sh +++ b/bin/gstack-doctor-components.sh @@ -350,6 +350,7 @@ gstack_result_anchor() { QUEUE_LAND_STAMPED) printf '%s' 'queue-land-stamped' ;; QUEUE_LAND_PIN_MOVED) printf '%s' 'queue-land-pin-moved' ;; QUEUE_LAND_CHECKS) printf '%s' 'queue-land-checks' ;; + QUEUE_LAND_STALE_BASE) printf '%s' 'queue-land-stale-base' ;; QUEUE_LAND_NOT_CLEAN) printf '%s' 'queue-land-not-clean' ;; QUEUE_LAND_UNVERIFIED) printf '%s' 'queue-land-unverified' ;; RELEASE_STAMP_EMPTY) printf '%s' 'release-stamp-empty' ;; diff --git a/changelog.d/gst-1316-land-stale-base.md b/changelog.d/gst-1316-land-stale-base.md new file mode 100644 index 000000000..55300f287 --- /dev/null +++ b/changelog.d/gst-1316-land-stale-base.md @@ -0,0 +1,8 @@ +**`gstack-ship-queue land --pin` no longer lands a PR whose base moved after its CI (GST-1316).** Under `merge: direct`, land checked the PR's own green checks, but those ran on the PR before newer commits reached main. #3886 was green on its own, landed onto a newer main, and pushed `ship/DRIVER.md` to 12,329 bytes against its 12,288-byte cap, so Free Tests on main went red. + +### Fixed +- **`gstack-ship-queue land --pin`:** refuses with the new `QUEUE_LAND_STALE_BASE` when the pin's merge base with `origin/main` is not main's tip, so the checks at the pin always ran on the exact tree that lands. The fix it prints: merge main into the branch, wait for the required checks at the new head, run /ship and `ready` there, then land the new pin. Nothing merges on a refusal; `--dry-run` reports it too. +- **Docs:** `docs/ship-policy.md#land-direct` lists the new check and drops "no base merge and no extra CI round". The release section now says a release may not delete a release output, and that a rerun after GitHub merged a release fetches the pin, then reads its parent, with an unreadable parent refused as `QUEUE_NOT_READY`. `docs/troubleshooting.md#queue-land-stale-base` and the `docs/captain.md` refusal table cover the new code. + +### Tests +- `test/ship-queue-land-direct.test.ts`: a PR whose base moved after its pin is refused with `QUEUE_LAND_STALE_BASE` (also under `--dry-run`) and main stays put; once the branch merges main and is shipped and READY at the new head, it lands and main's tree equals that head's tree. This replaces the test that expected the stale-base PR to land as it was. diff --git a/docs/captain.md b/docs/captain.md index a6c618e9f..9e09e04c7 100644 --- a/docs/captain.md +++ b/docs/captain.md @@ -347,6 +347,7 @@ What to do with each refusal (nothing was merged unless the code says so): | `QUEUE_RECEIPT_MISSING` / `QUEUE_RECEIPT_STALE` | lane | /ship at the pin | | `QUEUE_NOT_READY` | lane, or Captain | no READY note: `ready --pr --no-screen` at the pin; `merge: train`: use the train; closed or retargeted PR: as named | | `QUEUE_LAND_CHECKS` | Captain, then lane | pending: `gstack-pr-wait --until green` and rerun; red: back to the lane (rule 16 rerun for a flaky case); missing: dispatch the workflow on the branch | +| `QUEUE_LAND_STALE_BASE` | lane | main moved since the pin's merge base: the lane merges main (rule 19), waits for CI, /ship, `ready`, new READY | | `QUEUE_LAND_NOT_CLEAN` | lane, or Captain | `CONFLICTING`: the lane merges main (rule 19), /ship, `ready`; `UNKNOWN`: rerun in a minute | | `QUEUE_TRAIN_FROZEN` / `QUEUE_LAND_LOCKED` | Captain | finish the repair or the holder first | | `QUEUE_TRAIN_UNTRUSTED` | Captain | a fork head; rewrite on a crew branch | diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 6c286e92b..bdf982e8e 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -2002,9 +2002,10 @@ tree, taking no lock and pushing nothing. Under `"merge": "direct"` in the base policy, the Captain merges one READY PR at a time with `gstack-ship-queue land --pr --pin `, where `` -is the head the READY names. There's no train, no stamp commit, no base merge -and no extra CI round. GitHub's required checks plus `--match-head-commit` -stay the gate. Versions aren't set per PR: a PR carries only its +is the head the READY names. There's no train and no stamp commit. GitHub's +required checks plus `--match-head-commit` stay the gate, so the pin must +already hold the tip of `origin/main`: then the checks at the pin ran on the +exact tree that lands. Versions aren't set per PR: a PR carries only its `changelog.d` fragment, VERSION stays where it is, and the [release PR](#release-stamp) stamps the version and folds the fragments. Land refuses, and merges nothing, unless all of these hold (a `release/` @@ -2021,12 +2022,15 @@ release-file rows): | no freeze is active and no land lock is held | `QUEUE_TRAIN_FROZEN`, `QUEUE_LAND_LOCKED` | | VERSION, the `package.json` version, the CHANGELOG and `.gstack/version-path` are unchanged since the merge base | [`QUEUE_LAND_STAMPED`](troubleshooting.md#queue-land-stamped) | | every check at the pin is green with none pending, and each `changelog_fragments.required_checks` entry passed there | [`QUEUE_LAND_CHECKS`](troubleshooting.md#queue-land-checks) | +| the pin's merge base with `origin/main` is its tip | [`QUEUE_LAND_STALE_BASE`](troubleshooting.md#queue-land-stale-base) | | GitHub reports the PR `MERGEABLE` and `CLEAN` | [`QUEUE_LAND_NOT_CLEAN`](troubleshooting.md#queue-land-not-clean) | Release outputs (the agents digest) are checked by the Version Gate, which must be green at the pin: it accepts an output only when it equals what the release -tool generates at the merge base's VERSION. If the base moved and GitHub still -reports the PR clean, land merges it as it is. Then it runs `gh pr merge +tool generates at the merge base's VERSION. If the base moved, land refuses +even when GitHub reports the PR clean: a green PR merged onto newer commits can +still break main (#3886 pushed ship/DRIVER.md past its size cap that way), so +the lane merges main and CI runs again. Then it runs `gh pr merge --squash --match-head-commit ` (`--merge` under `history: merge-only`), reads the merge commit, and checks that its tree is the pin merged onto the merge commit's parent ([`QUEUE_LAND_UNVERIFIED`](troubleshooting.md#queue-land-unverified) @@ -2065,9 +2069,12 @@ ships under a version its fragment missed): every fragment there folded and none added, the version equal to the base's plus their bump, every other change a stamp by the [tree receipt](#tree-receipt)'s rules, and a changed release output equal to what the release tool generates at the new -version. Anything more is +version. A release may not delete a release output. Anything more is [`RELEASE_STAMP_INVALID`](troubleshooting.md#release-stamp-invalid), and -nothing merges. The Version Gate (`Check VERSION is not stale vs queue`) +nothing merges. A rerun after GitHub merged the release fetches the pin first, +then reads the pin's parent, the main commit it was stamped on, and checks that +the release merged onto it; a parent it can't read is `QUEUE_NOT_READY`, never +a pass. The Version Gate (`Check VERSION is not stale vs queue`) applies the same test, so a release head passes it while any other PR that changes VERSION still fails it. Once the release PR lands, the next stable cut tags `v` ([releases.md](releases.md)). diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 96c58f3d6..b05425ea6 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -5213,6 +5213,18 @@ ran there. Nothing was merged. the lane fixes it at a new head (a flaky case gets its one rerun under rule 16). Missing: dispatch that workflow on the branch, then rerun land. + +### `QUEUE_LAND_STALE_BASE` + +**Meaning.** `origin/main` moved since the pin's merge base, so the checks +at the pin never ran on the tree that would land. A PR that is green on its +own can still break main once merged onto newer commits (#3886 pushed +ship/DRIVER.md past its size cap that way). Nothing was merged. + +**Fix.** The lane merges `origin/main` into the branch, pushes, waits for +the required checks at the new head, runs /ship and `ready` there, and sends +a new READY; land that pin. + ### `QUEUE_LAND_NOT_CLEAN` diff --git a/lib/result-codes-release.ts b/lib/result-codes-release.ts index 478d30ac7..6b8151ea1 100644 --- a/lib/result-codes-release.ts +++ b/lib/result-codes-release.ts @@ -488,6 +488,11 @@ export const RELEASE_RESULT_CODES = { summary: 'a check at the pin is red or pending, or a required check never ran there', fix: 'wait for pending checks (gstack-pr-wait --until green), fix a red one at a new head, or dispatch a missing required workflow on the branch; then rerun land', }, + QUEUE_LAND_STALE_BASE: { + anchor: 'queue-land-stale-base', + summary: 'the base branch moved since the pin\'s merge base, so the checks at the pin never ran on the tree that would land (GST-1316: a stale-base PR pushed a budgeted file past its cap on main)', + fix: 'merge the base into the branch, push, wait for the required checks at the new head, run /ship and gstack-ship-queue ready there, then land the new pin', + }, QUEUE_LAND_NOT_CLEAN: { anchor: 'queue-land-not-clean', summary: 'GitHub does not report the PR mergeable and clean (a conflict, a branch rule, or a state GitHub is still computing)', diff --git a/lib/ship-queue-land-direct.ts b/lib/ship-queue-land-direct.ts index ef5ded962..095b5fcad 100644 --- a/lib/ship-queue-land-direct.ts +++ b/lib/ship-queue-land-direct.ts @@ -1,8 +1,9 @@ /** * ship-queue-land-direct — B1's merge step, behind `gstack-ship-queue land --pr --pin ` under the base - * policy's merge: direct (docs/ship-policy.md#land-direct). The PR merges at its own green head: no train, no stamp - * commit, no base merge and no extra CI round. It refuses unless the /ship receipt and the READY note are bound to the - * pin, the pin is the PR's head, the diff leaves VERSION, the manifest version and the CHANGELOG at its merge base + * policy's merge: direct (docs/ship-policy.md#land-direct). The PR merges at its own green head: no train and no stamp + * commit. It refuses unless the /ship receipt and the READY note are bound to the pin, the pin is the PR's head, the + * pin's merge base is the base tip (so the checks at the pin ran on the tree that lands, GST-1316), the diff leaves + * VERSION, the manifest version and the CHANGELOG at its merge base * (versions are set at release), every check at the pin is green with none pending and each required check present, * and GitHub reports the PR mergeable and clean. Then gh pr merge --squash --match-head-commit , a check that the * merge commit's tree is the pin merged onto that commit's parent, and a land note on refs/notes/gstack-trains. A rerun @@ -132,6 +133,7 @@ export function landDirect(ctx: QueueContext, o: DirectOptions): StepOutput { const passed = new Set([...ci.checks.filter(c => c.conclusion === 'success').map(c => c.name), ...ci.contexts.filter(c => c.state === 'success').map(c => c.name)]); const missing = (policy.changelog_fragments?.required_checks ?? []).filter(n => !passed.has(n)); if (missing.length) return fail(lines, { code: 'QUEUE_LAND_CHECKS', message: `required check(s) ${missing.join(', ')} did not pass at the pin ${o.pin.slice(0, 12)}`, fix: `dispatch them on the branch (gh workflow run --ref ${info.headRefName}) and rerun land once they pass` }); + if (mergeBase !== tip) return fail(lines, { code: 'QUEUE_LAND_STALE_BASE', message: `origin/${ctx.base} moved to ${tip.slice(0, 12)} since the pin's merge base ${mergeBase.slice(0, 12)}, so the checks at the pin never ran on the tree that would land`, fix: `merge the base into the branch (git merge origin/${ctx.base}), push, wait for the required checks at the new head, run /ship and gstack-ship-queue ready --pr ${o.pr} there, then land the new pin` }); if (info.mergeable !== 'MERGEABLE' || info.mergeStateStatus !== 'CLEAN') return fail(lines, { code: 'QUEUE_LAND_NOT_CLEAN', message: `GitHub reports #${o.pr} mergeable=${info.mergeable ?? 'unknown'} mergeStateStatus=${info.mergeStateStatus ?? 'unknown'}`, fix: info.mergeable === 'CONFLICTING' ? `the lane merges origin/${ctx.base} into the branch, resolves, runs /ship and ready at the new head` : 'UNKNOWN means GitHub is still computing: rerun land in a minute; BLOCKED or BEHIND names the branch rule to meet first' }); if (o.dryRun) return { ok: true, run, lines: [...lines, `DRY-RUN: nothing merged (would run gh pr merge ${o.pr} --squash --match-head-commit ${o.pin})`] }; diff --git a/test/ship-queue-land-direct.test.ts b/test/ship-queue-land-direct.test.ts index bb8d32a34..6373acef8 100644 --- a/test/ship-queue-land-direct.test.ts +++ b/test/ship-queue-land-direct.test.ts @@ -131,18 +131,33 @@ describe('land --pin: direct merge at the PR\'s own green head', () => { expect(notes[0]).toMatchObject({ kind: 'land', pr: '7', pin, merge_sha: main, base_sha: base }); }, 90_000); - test('the base moved and the PR still merges clean: it lands as it is, on the new base, with no restamp or extra CI round', () => { + test('GST-1316: the base moved after the pin\'s CI, so its green checks never saw the tree that would land; land refuses with QUEUE_LAND_STALE_BASE, then lands once the branch holds main', () => { const fx = setup(); - const pin = openPr(fx, '8'); + const pin = openPr(fx, '8', { edit: w => fs.writeFileSync(path.join(w, 'budget.md'), 'pr line\n') }); const moved = moveMain(fx, 'other.txt', 'other\n'); const r = land(fx, '8', pin); - expect(r.err).toBe(''); - expect(r.status).toBe(EXIT.ok); + expect(r.status).not.toBe(EXIT.ok); + expect(r.err).toContain('(QUEUE_LAND_STALE_BASE)'); + expect(r.err).toContain(moved.slice(0, 12)); + expect(r.err).toContain('git merge origin/main'); + expect(fx.read().mergeArgs).toBeUndefined(); + expect(remote(fx, 'main')).toBe(moved); + expect(land(fx, '8', pin, ['--dry-run']).err).toContain('(QUEUE_LAND_STALE_BASE)'); + + fx.repo.git('checkout', '-q', '-B', 'pr-8', pin); + fx.repo.git('merge', '-q', '--no-edit', 'origin/main'); + fx.repo.git('push', '-q', 'origin', 'HEAD:refs/heads/pr-8'); + const merged = fx.repo.git('rev-parse', 'HEAD'); + fx.repo.git('checkout', '-q', '--detach', 'origin/main'); + fx.write(d => { d.prs['8'].body = shippedBody('8', merged); }); + markReady(fx, '8', merged); + const again = land(fx, '8', merged); + expect(again.err).toBe(''); + expect(again.status).toBe(EXIT.ok); const main = remote(fx, 'main'); expect(fx.repo.git('rev-parse', main + '^1')).toBe(moved); - expect(show(fx, main + ':other.txt')).toBe('other'); + expect(fx.repo.git('rev-parse', main + '^{tree}')).toBe(fx.repo.git('rev-parse', merged + '^{tree}')); expect(show(fx, main + ':pr-8.txt')).toBe('8'); - expect(fx.repo.git('rev-parse', 'origin/pr-8')).toBe(pin); }, 90_000); test('a rerun after GitHub merged the PR goes straight to the tree check and lands', () => {