From 7ba250ba60e361640f221e5f99424e72cde4bce8 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 4 Nov 2026 01:18:11 -0800 Subject: [PATCH] fix(ship-queue): land --pin tests the merged tree on a stale base, with consent and a scrubbed env (GST-1316) (#3984) * 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> * fix: pre-landing review fixes Review-Finding: p1/red-team#1 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(captain): QUEUE_LAND_UNVERIFIED row names the main-moved-during-merge cause Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * chore: name the changelog fragment after the branch Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * chore: changelog and TODOS Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Revert "docs(captain): QUEUE_LAND_UNVERIFIED row names the main-moved-during-merge cause" This reverts commit 343dd2bf161d2745812ead964de787576ef9551b. * feat(ship-queue): land --pin tests the merged tree on a stale base with land.merged_checks (GST-1316) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship-queue): merged checks on a merged commit fail on a timeout or a tracked-file rewrite; no land key warns and lands Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship-queue): land.merged_checks need --allow-repo-commands and a scrubbed env; a no-key rerun checks the tree only; lock before merge Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship-queue): land --pin recovers its own leaked lock, takes --break-lock, refreshes the lock before merging; dry rerun verifies nothing Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship-queue): merged checks run git with hooks off and log to a file, so a lingering child cannot stall a passing check Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship-queue): merged checks stop at the timeout with SIGKILL, refuse a dependency change or a moved HEAD, and land re-reads the base policy under the lock Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(captain): land refusal rows cover a stale-base conflict and a main that moved after land's check Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test(ship-queue): a placeholder GH_TOKEN value the secrets scan does not flag 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> --- bin/gstack-doctor-components.sh | 1 + bin/gstack-ship-queue | 11 +- changelog.d/gsta258-f-land-stale-base.md | 12 ++ docs/captain.md | 8 +- docs/ship-policy.md | 65 +++++- docs/troubleshooting.md | 39 +++- lib/land-merged-checks.ts | 93 +++++++++ lib/result-codes-release.ts | 5 + lib/ship-policy.ts | 9 +- lib/ship-queue-land-direct.ts | 119 ++++++++--- test/ship-queue-land-direct.test.ts | 244 ++++++++++++++++++++++- 11 files changed, 540 insertions(+), 66 deletions(-) create mode 100644 changelog.d/gsta258-f-land-stale-base.md create mode 100644 lib/land-merged-checks.ts diff --git a/bin/gstack-doctor-components.sh b/bin/gstack-doctor-components.sh index b5d840701..e3843b635 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_MERGED_CHECKS) printf '%s' 'queue-land-merged-checks' ;; 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/bin/gstack-ship-queue b/bin/gstack-ship-queue index 30c1c051c..bdf9fec6a 100755 --- a/bin/gstack-ship-queue +++ b/bin/gstack-ship-queue @@ -14,7 +14,7 @@ * gstack-ship-queue gate-ahead --list --pr * gstack-ship-queue verify --pr --screen (train policy: the head's screen verdict from its run) * gstack-ship-queue verify --post-merge [--timeout ] (after a merge: main's push workflows at that commit until they settle) - * gstack-ship-queue land --pr --pin [--dry-run] (merge: direct; lib/ship-queue-land-direct.ts) + * gstack-ship-queue land --pr --pin [--dry-run] [--allow-repo-commands] [--break-lock] (merge: direct; lib/ship-queue-land-direct.ts) * gstack-ship-queue land --pr [--dry-run] [--no-wait] [--break-lock [--break-train-lock]] (the merge step; lib/ship-queue-land.ts) * gstack-ship-queue land --pr --repair (after QUEUE_LAND_DRIFT: open the outside merge's release PR) * gstack-ship-queue land --census @@ -58,8 +58,11 @@ const HELP = `gstack-ship-queue — the shared merge queue's protocol steps (doc verify --pr --screen (train policy) the PR head's screen verdict read from its run (SCREEN_VERDICT) verify --post-merge [--timeout ] wait for every check run at a merged commit to settle: POST_MERGE: green|red|pending and one CHECK: line per workflow; resumable, default timeout 2700 s - land --pr --pin [--dry-run] merge: direct: merge the PR at its own green head (receipt and READY at the pin, no stamp - files, checks green, mergeable clean), verify the tree, write a land note + land --pr --pin [--dry-run] [--allow-repo-commands] [--break-lock] + merge: direct: merge the PR at its own green head (receipt and READY at the pin, no stamp + files, checks green, mergeable clean), verify the tree, write a land note; a stale base + runs land.merged_checks, repo commands, only with --allow-repo-commands (never --dry-run); + it holds the land lock from the re-fetch to the merge (--break-lock: a stale one) land --pr [--dry-run] [--no-wait] [--timeout ] [--break-lock [--break-train-lock]] [--allow-repo-commands] the merge step: lock, merge the base, stamp (fragments folded), push the stamp commit, wait for its checks, squash with --match-head-commit, confirm the base tree; resumable @@ -179,7 +182,7 @@ async function main(argv: string[]): Promise { if (args.includes('--pin')) { const pin = flag(args, '--pin') ?? ''; if (!/^[0-9a-f]{40,64}$/.test(pin)) return usage('--pin needs the full head sha the READY names'); - return finish('land', (await import('../lib/ship-queue-land-direct')).landDirect(ctx, { pr, pin, dryRun: args.includes('--dry-run') })); + return finish('land', (await import('../lib/ship-queue-land-direct')).landDirect(ctx, { pr, pin, dryRun: args.includes('--dry-run'), allowRepoCommands: args.includes('--allow-repo-commands'), breakLock: args.includes('--break-lock') })); } const timeout = flag(args, '--timeout'); return finish('land', l.land(ctx, { pr, repair: args.includes('--repair'), dryRun: args.includes('--dry-run'), noWait: args.includes('--no-wait'), breakLock: args.includes('--break-lock'), breakTrainLock: args.includes('--break-train-lock'), allowRepoCommands: args.includes('--allow-repo-commands'), waitTimeoutMin: timeout && /^\d+$/.test(timeout) ? Number(timeout) : undefined })); diff --git a/changelog.d/gsta258-f-land-stale-base.md b/changelog.d/gsta258-f-land-stale-base.md new file mode 100644 index 000000000..2d5d74ad9 --- /dev/null +++ b/changelog.d/gsta258-f-land-stale-base.md @@ -0,0 +1,12 @@ +**`gstack-ship-queue land --pin` can test the merged tree when a PR's 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. + +### Added +- **`land` policy key:** `land.merged_checks` lists the commands land runs when the base moved past the pin's merge base. Land builds a throwaway commit of the pin merged onto main's tip, runs each command in a worktree of it, and merges only when every one passes. It prints `MERGED_CHECKS: pass tree=… base=… pin=… checks: …`. A failure, a timeout or a check that rewrites a tracked file refuses with the new `QUEUE_LAND_MERGED_CHECKS`, a conflict refuses with `QUEUE_LAND_NOT_CLEAN`, and nothing merges. No lane has to merge main, rerun CI and re-ship each time another PR lands first. Without the key, a stale base lands as before with a `STALE_BASE: warn … no land.merged_checks configured` line. The commands are repo commands (the merged tree's `package.json` decides what they run), so land runs them only with `--allow-repo-commands` (else `REPO_COMMANDS_NOT_ALLOWED`), with `PATH` and the locale as the whole environment and a throwaway `HOME` (no sandbox: they can still write the checkout's `.git` and `node_modules`), runs git around them with hooks off, kills a command's whole process group when it ends or times out (so a passing check that leaves a child behind passes at once), and skips them under `--dry-run` with a `DRY-RUN: land.merged_checks not run` line. A check that moves HEAD fails, land re-reads the base policy after its locked fetch, and a merged tree whose lockfile or dependencies differ from the checkout's is refused with `QUEUE_NOT_READY` before any check runs, since the linked `node_modules` can't vouch for it. gstack's own policy adopts the key after the release that carries it. + +### Fixed +- **`gstack-ship-queue land --pin`:** land now checks that GitHub reports the PR clean before it runs any merged check, then takes the land lock and re-fetches main before it checks and merges, refreshes the lock right before the merge, and releases it afterward. A rerun takes over or releases a lock an earlier run of the same PR left, and `land --pin --break-lock` breaks another lander's stale lock. A rerun after GitHub merged a stale base with no `land.merged_checks` checks the tree only, with a `STALE_BASE: warn` line, instead of reporting `QUEUE_LAND_UNVERIFIED`. +- **`gstack-ship-queue land --pin`:** after the merge, land checks that GitHub merged onto the base it checked or one the pin contains. If main moved in between, it reruns `land.merged_checks` on the landed tree and reports `QUEUE_LAND_UNVERIFIED` with no land note when they fail or the key is absent. +- **Docs:** `docs/ship-policy.md#land-direct` describes the merged-tree check and the `land` key. 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` and the `docs/captain.md` refusal table cover the new code. + +### Tests +- `test/ship-queue-land-direct.test.ts`: on a stale base, a pin whose merged tree breaks a stand-in budget check is refused with `QUEUE_LAND_MERGED_CHECKS` and main stays put. A pin that passes lands with main's tree equal to the checked merged tree, and a rerun checks again. A conflicting pin is refused with `QUEUE_LAND_NOT_CLEAN`, and with no key a stale base lands with the warn line. The checks see the merged commit as HEAD, and a timed-out check (even one exiting 0 on SIGTERM) or a tracked-file rewrite fails. Without `--allow-repo-commands` the checks are refused and never run, under `--dry-run` and on a PR GitHub doesn't report clean they never run, and when they run they see no token and a throwaway `HOME` while land holds the land lock. A timed-out check's background child is killed with it, a passing check that leaves a child behind passes at once, and the checkout's post-checkout hook does not run when land builds the merged worktree. A check that ignores SIGTERM still stops at its timeout, a check that moves HEAD fails, a dependency change is refused before the checks, and a `land.merged_checks` that main gains while land runs is applied. A no-key rerun lands with the tree check. A lock left by an interrupted run is taken over, another PR's lock refuses (and `--break-lock` breaks only a stale one), a lock left after the merge is released by the rerun, and a rerun without `--allow-repo-commands` that must run the checks on the landed tree refuses. The key's validation is covered, and the mid-land race test still expects `QUEUE_LAND_UNVERIFIED` with no land note on the first run. diff --git a/docs/captain.md b/docs/captain.md index a6c618e9f..f395dbf7d 100644 --- a/docs/captain.md +++ b/docs/captain.md @@ -330,7 +330,8 @@ commits. Each PR merges at its own green head, and versions are set at release 1. Lane: /ship, then `gstack-ship-queue ready --pr --no-screen` at the pinned head, then READY to the Captain with that pin. The PR carries a `changelog.d` fragment and leaves VERSION, the `package.json` version and CHANGELOG.md alone. -2. Captain: `gstack-ship-queue land --pr --pin `. It merges with +2. Captain: `gstack-ship-queue land --pr --pin ` (add `--allow-repo-commands` when + the base policy lists `land.merged_checks`, so a stale base can run them). It merges with `--match-head-commit`, checks main's tree and writes a land note. 3. Release: before each stable cut (Monday and Thursday, 14:00 UTC), the Captain runs `gstack-release-stamp open`, waits for the release PR's checks, then lands it with @@ -347,10 +348,11 @@ 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_NOT_CLEAN` | lane, or Captain | `CONFLICTING`: the lane merges main (rule 19), /ship, `ready`; `UNKNOWN`: rerun in a minute | +| `QUEUE_LAND_MERGED_CHECKS` | lane | a `land.merged_checks` command failed on the pin merged onto main: the lane merges main (rule 19), fixes it, /ship, `ready`, new READY | +| `QUEUE_LAND_NOT_CLEAN` | lane, or Captain | `CONFLICTING`, or the pin conflicts with main's tip on a stale base: 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 | -| `QUEUE_LAND_UNVERIFIED` | Captain | merged, but main isn't the pin on its parent: revert the stray change, report it | +| `QUEUE_LAND_UNVERIFIED` | Captain | merged, but main isn't the pin on its parent, or main moved after land's check and `land.merged_checks` failed or is absent on the landed tree: revert the stray change or fix forward, report it | | `RELEASE_STAMP_INVALID` | Captain | the `release/` head is more than its stamp: close it, `gstack-release-stamp open` again | | `RELEASE_STAMP_EMPTY` | Captain | no fragment landed since the last release: nothing to release | diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 3a619e173..267b59d9e 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -65,6 +65,7 @@ refuses the file (`POLICY_GSTACK_TOO_OLD`). `validate` ends with | `docs` | `{ caps: [{ path, max_bytes }] }` or `null` | `null` | Documentation files with a byte cap. `/ship` Step 14.5 runs `gstack-ship-receipt docs-caps --base ` (the caps come from `origin/`'s policy, never the branch's copy; `--policy ` is for a repository bootstrapping its policy) before the verification gate and prints one `DOCS_CAP: / ok\|over` line per cap; any `over` (an absent file counts) exits 1 and blocks the ship until the file is trimmed or the cap raised on the base branch (`DOCS_AUDIT_BLOCKED`). At most 64 caps, each path contained. See [the ship receipt](#ship-receipt). Needs `min_gstack_version`. | | `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 --pin `](#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 ` 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`. | | `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`. | @@ -2006,9 +2007,11 @@ 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. When the pin +doesn't hold the tip of `origin/main`, those checks never ran on the tree that +would land, so land tests that tree itself with the base policy's +`land.merged_checks` (below). 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/` @@ -2026,17 +2029,54 @@ release-file rows): | 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) | | GitHub reports the PR `MERGEABLE` and `CLEAN` | [`QUEUE_LAND_NOT_CLEAN`](troubleshooting.md#queue-land-not-clean) | +| land takes the land lock (`land-pin:` journal, phase `merging`) and re-fetches `origin/main`; it takes over a lock an earlier `land --pin` run of the same PR left, and `--break-lock` breaks another lander's stale lock under the usual rules | `QUEUE_LAND_LOCKED`, `QUEUE_NOT_READY` | +| when the base moved past the pin's merge base and the base policy lists `land.merged_checks`: `--allow-repo-commands` was passed, the pin merges onto the tip without a conflict, and every command passes on that merged tree | [`REPO_COMMANDS_NOT_ALLOWED`](troubleshooting.md#repo-commands-not-allowed), [`QUEUE_LAND_NOT_CLEAN`](troubleshooting.md#queue-land-not-clean), [`QUEUE_LAND_MERGED_CHECKS`](troubleshooting.md#queue-land-merged-checks) | 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. A green PR merged onto newer +commits can still break main (#3886 pushed ship/DRIVER.md past its size cap +that way). So when the base moved, land builds the pin merged onto the tip of +`origin/main` in a throwaway worktree (the checkout's `node_modules` linked +in), runs each `land.merged_checks` command there in order, and prints +`MERGED_CHECKS: pass tree= base= pin= checks: …`. The +commands are repo commands: the merged tree's `package.json` decides what +`bun run` does, so the PR controls them. Land runs them only with +`--allow-repo-commands`, with `PATH` and the locale as the whole environment +and a throwaway `HOME`, so no token reaches them through the environment, +runs git around them with hooks and fsmonitor off, and kills a command's whole +process group with SIGKILL when it ends or times out. A check that moves HEAD +off the merged commit fails. Land re-reads the base policy after the locked +fetch, so a `land.merged_checks` that main gained meanwhile applies. When the +merged tree's lockfile or `package.json` dependencies differ from the +checkout's, the linked `node_modules` can't vouch for it: land refuses with +`QUEUE_NOT_READY` before any check runs, and the lane merges main and re-ships. It doesn't sandbox +the filesystem: a command can still write the checkout's `.git` (the worktree +shares it) and the linked `node_modules`, so pass the flag only for a base +policy and PRs you trust to run. A list such +as typecheck plus the size ratchets takes about two minutes, against a main +merge, a CI round and a new receipt per PR. Without the key, land prints +`STALE_BASE: warn … no land.merged_checks configured` and merges the stale +base untested, as it did before the key. Then land 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) -if not). Last, it writes a `land` note on `refs/notes/gstack-trains`. A rerun -after GitHub merged goes straight to the tree check. `--dry-run` runs every -check and merges nothing. Under `merge: direct`, `train build` refuses and +merge commit's parent. The land lock keeps other gstack landers off main +from the re-fetch to the merge, but a merge outside gstack can still land in +between. When that parent is neither the base land checked nor +one the pin contains, main moved between the check and the merge: land runs +`land.merged_checks` again on the landed tree, and reports +[`QUEUE_LAND_UNVERIFIED`](troubleshooting.md#queue-land-unverified) when they +fail or the key is absent. Last, it writes a `land` note on `refs/notes/gstack-trains` +and releases the lock. Land refreshes the lock right before the merge and +refuses if another lander broke it meanwhile. A rerun after GitHub merged goes +straight to the tree check and releases a lock an earlier run of the same land +left. It doesn't know which base the first run checked, so when GitHub merged +onto a base the pin doesn't contain, it runs `land.merged_checks` on the landed +tree (they need `--allow-repo-commands` there too), and without the key it +prints a `STALE_BASE: warn` line and checks the tree only. `--dry-run` takes +no lock, runs every other check, prints `DRY-RUN: land.merged_checks not run` +in place of the merged checks, and merges nothing; on a PR that already +merged it verifies nothing. Under `merge: direct`, `train build` refuses and names this command. @@ -2069,9 +2109,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 c8362e4df..6fd02a060 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -3228,10 +3228,12 @@ nothing. ### `repo commands not executed (pass --allow-repo-commands) (REPO_COMMANDS_NOT_ALLOWED)` -**Meaning.** The policy declares a command (`release_tool`), and the tool was -run standalone. File presence never authorizes execution: outside `/ship` and -`/land-and-deploy` a repo-declared command runs only with -`--allow-repo-commands`. The commands were listed and nothing was written. +**Meaning.** The policy declares a command (`release_tool`, or +`land.merged_checks` for `gstack-ship-queue land --pin` on a stale base), and +the tool was run without consent. File presence never authorizes execution: +outside `/ship` and `/land-and-deploy` a repo-declared command runs only with +`--allow-repo-commands`. The commands were listed and nothing was written or +merged. **Fix.** Pass `--allow-repo-commands` when you mean to execute the repo's code (you already do when you run its tests), or drop the release tool. @@ -5241,11 +5243,24 @@ 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_MERGED_CHECKS` + +**Meaning.** `origin/main` moved since the pin's merge base, and a +`land.merged_checks` command failed on the pin merged onto main's tip. The +message names the command, its exit and its last output lines. Nothing was +merged. + +**Fix.** The lane merges `origin/main` into the branch, fixes what the +command names, pushes, waits for the required checks, runs /ship and `ready` +at the new head, and sends a new READY. + ### `QUEUE_LAND_NOT_CLEAN` -**Meaning.** GitHub doesn't report the PR `MERGEABLE` and `CLEAN`. Nothing -was merged. +**Meaning.** GitHub doesn't report the PR `MERGEABLE` and `CLEAN`, or, on a +stale base, the pin conflicts with main's tip (the message names the files). +Nothing was merged. **Fix.** `CONFLICTING`: the lane merges the base into its branch, resolves, runs /ship and `ready` at the new head. `UNKNOWN`: GitHub is still @@ -5257,10 +5272,16 @@ rule it names. **Meaning.** GitHub merged the PR, but the base branch's merge commit tree isn't the pin merged onto that commit's parent, so something besides the -reviewed pin reached main. No land note was written. +reviewed pin reached main, or the merge commit's parent is neither in the pin +nor the base land checked, so main moved between land's check and the merge, +and `land.merged_checks` then failed on the landed tree or the base policy +lists none. No land note was written. -**Fix.** Compare the merge commit with `git merge-tree --write-tree -`, revert what doesn't belong to the PR, and report it to the Captain. +**Fix.** For a tree mismatch, compare the merge commit with `git merge-tree +--write-tree `, revert what doesn't belong to the PR, and report +it to the Captain. For a parent the pin doesn't contain, run the free checks on +main at the merge commit; if one is red, fix forward or revert the PR, and +report it to the Captain. ### `RELEASE_STAMP_EMPTY` diff --git a/lib/land-merged-checks.ts b/lib/land-merged-checks.ts new file mode 100644 index 000000000..ceddd186e --- /dev/null +++ b/lib/land-merged-checks.ts @@ -0,0 +1,93 @@ +/** + * The `land` block of .gstack/ship-policy.json and its merged-tree run (docs/ship-policy.md#land-direct, GST-1316). + * When the base moved past a pin's merge base, `gstack-ship-queue land --pin` builds the pin merged onto the base + * tip in a throwaway worktree and runs `land.merged_checks` there; it lands only when every command passes. A + * conflict or a failing command refuses; with no key, land warns and merges as before. The commands are repo commands + * (the merged tree's package.json decides what `bun run` does), so they run only under --allow-repo-commands, never + * under --dry-run, with an environment that keeps PATH and locale only and a throwaway HOME. Validated here so + * lib/ship-policy.ts stays one schema file. + */ +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import type { PolicyError } from './ship-policy'; + +/** A local git (lib/ship-queue-gate's shape) so this module, which lib/ship-policy imports, pulls in no queue code. */ +function git(cwd: string, args: string[], env?: NodeJS.ProcessEnv): { ok: boolean; out: string; err: string; status: number | null } { + const r = spawnSync('git', args, { cwd, encoding: 'utf8', timeout: 120_000, env: env ?? process.env, maxBuffer: 64 * 1024 * 1024 }); + return { ok: r.status === 0, out: (r.stdout ?? '').replace(/\n$/, ''), err: (r.stderr ?? '').trim(), status: r.status }; +} + +export interface LandPolicy { + /** Shell commands run in the pin merged onto the base tip, in order; each must exit 0. */ + merged_checks: string[]; +} + +const MAX_CHECKS = 16, MAX_COMMAND = 512, CHECK_TIMEOUT_MS = 900_000; +const KEPT_ENV = ['PATH', 'LANG', 'LC_ALL', 'TERM', 'TMPDIR']; +/** No hook or fsmonitor from the repository's config or the merged tree runs in the git calls around the checks. */ +const NO_HOOKS = ['-c', 'core.hooksPath=/dev/null', '-c', 'core.fsmonitor=false']; + +export const LAND_EXPLANATION = '{ merged_checks } or null, read from the base revision only: under merge: direct, when the base moved past a pin\'s merge base, `land --pin` builds the pin merged onto the base tip in a throwaway worktree (the checkout\'s node_modules linked in) and runs each merged_checks command there (at most 16, 512 characters each, 15 minutes each); it lands only when every one exits 0, and refuses on a conflict (QUEUE_LAND_NOT_CLEAN) or a failure (QUEUE_LAND_MERGED_CHECKS). They are repo commands: land runs them only with --allow-repo-commands (else REPO_COMMANDS_NOT_ALLOWED), never under --dry-run, with PATH and locale as the only environment and a throwaway HOME. null lands a stale base unchecked with a STALE_BASE: warn line.'; + +export function landPolicyErrors(raw: unknown): PolicyError[] { + const l = raw as Record | null; + if (!l || typeof l !== 'object' || Array.isArray(l)) return [{ code: 'POLICY_INVALID', path: 'land', message: 'must be { merged_checks } or null' }]; + const out: PolicyError[] = Object.keys(l).filter(k => k !== 'merged_checks').map(k => ({ code: 'POLICY_INVALID', path: `land.${k}`, message: 'unknown key' })); + const c = l.merged_checks; + if (!Array.isArray(c) || !c.length || c.length > MAX_CHECKS) return [...out, { code: 'POLICY_INVALID', path: 'land.merged_checks', message: `must list 1 to ${MAX_CHECKS} commands` }]; + c.forEach((cmd, i) => { + if (typeof cmd !== 'string' || !cmd.trim()) out.push({ code: 'POLICY_INVALID', path: `land.merged_checks[${i}]`, message: 'must be a non-empty command' }); + else if (cmd.length > MAX_COMMAND || /[\0\r\n]/.test(cmd)) out.push({ code: 'POLICY_CONTAINMENT', path: `land.merged_checks[${i}]`, message: `must be one line of at most ${MAX_COMMAND} characters` }); + }); + return out; +} + +/** `setup`: land could not run the checks at all (git failed, or the merged tree's dependencies differ from the linked node_modules): the lane's code is not at fault. */ +export type MergedRun = { ok: true; tree: string } | { ok: false; conflict: boolean; setup?: boolean; tree?: string; detail: string }; +const LOCKFILES = ['bun.lock', 'bun.lockb', 'package-lock.json', 'yarn.lock', 'pnpm-lock.yaml']; +const DEP_KEYS = ['dependencies', 'devDependencies', 'optionalDependencies', 'peerDependencies', 'workspaces', 'overrides']; +const read = (file: string): string | null => { try { return fs.readFileSync(file, 'utf8'); } catch { return null; } }; +const deps = (file: string): string => { try { const p = JSON.parse(read(file) ?? 'null') ?? {}; return JSON.stringify(DEP_KEYS.map(k => p[k] ?? null)); } catch { return 'unreadable'; } }; + +/** Run `checks` on a throwaway commit of `pin` merged onto `base` (HEAD is that commit), in a worktree of `root` that is always removed, with a scrubbed environment; a check that does not finish (its whole process group is killed), moves HEAD or changes a tracked file fails, and a merged tree whose dependencies differ from the checkout's is refused before any check runs. */ +export function runMergedChecks(root: string, base: string, pin: string, checks: string[], timeoutMs = CHECK_TIMEOUT_MS): MergedRun { + const mt = git(root, ['merge-tree', '--write-tree', '--name-only', base, pin]); + const [tree = '', ...rest] = mt.out.split('\n'); + if (!mt.ok) return { ok: false, conflict: mt.status === 1, setup: mt.status !== 1, detail: mt.status === 1 ? `the pin conflicts with ${base.slice(0, 12)} in ${rest.slice(0, rest.indexOf('') < 0 ? undefined : rest.indexOf('')).join(', ') || 'files git did not name'}` : `git merge-tree failed: ${mt.err.split('\n')[0]}` }; + const scratch = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-land-merged-')); + const dir = path.join(scratch, 'tree'), home = path.join(scratch, 'home'); + fs.mkdirSync(home); + const env: NodeJS.ProcessEnv = { HOME: home, ...Object.fromEntries(KEPT_ENV.filter(k => process.env[k] !== undefined).map(k => [k, process.env[k]])) }; + try { + const who = { GIT_AUTHOR_NAME: 'gstack land', GIT_AUTHOR_EMAIL: 'land@gstack.invalid', GIT_COMMITTER_NAME: 'gstack land', GIT_COMMITTER_EMAIL: 'land@gstack.invalid' }; + const commit = git(root, ['commit-tree', tree, '-p', base, '-p', pin, '-m', 'gstack land: merged-tree check'], { ...process.env, ...who }); + const add = commit.ok ? git(root, [...NO_HOOKS, 'worktree', 'add', '-q', '--detach', dir, commit.out]) : commit; + if (!add.ok) return { ok: false, conflict: false, setup: true, tree, detail: `cannot build the merged tree ${tree.slice(0, 12)}: ${add.err.split('\n')[0]}` }; + const drift = [...LOCKFILES.filter(l => read(path.join(root, l)) !== read(path.join(dir, l))), ...(deps(path.join(root, 'package.json')) !== deps(path.join(dir, 'package.json')) ? ['package.json dependencies'] : [])]; + if (drift.length) return { ok: false, conflict: false, setup: true, tree, detail: `the merged tree's ${drift.join(', ')} differ from this checkout's, so its node_modules cannot vouch for the merged tree` }; + const modules = path.join(root, 'node_modules'); + if (fs.existsSync(modules) && !fs.existsSync(path.join(dir, 'node_modules'))) fs.symlinkSync(modules, path.join(dir, 'node_modules')); + for (const cmd of checks) { + const group = process.platform !== 'win32'; + const log = path.join(scratch, 'check.log'); + const fd = fs.openSync(log, 'w'); + const opts = { cwd: dir, env, stdio: ['ignore', fd, fd] as ['ignore', number, number], timeout: timeoutMs, killSignal: 'SIGKILL' as const, detached: group }; + const r = spawnSync('sh', ['-c', cmd], opts); + fs.closeSync(fd); + if (group && r.pid) try { process.kill(-r.pid, 'SIGKILL'); } catch {} + if (r.status === 0 && !r.error) continue; + const tail = fs.readFileSync(log, 'utf8').trim().split('\n').slice(-6).join('\n'); + return { ok: false, conflict: false, tree, detail: `\`${cmd}\` ${r.error ? `did not finish (${r.error.message})` : `exited ${r.status ?? r.signal}`} on the merged tree ${tree.slice(0, 12)}${tail ? `:\n${tail}` : ''}` }; + } + if (git(dir, [...NO_HOOKS, 'rev-parse', 'HEAD']).out !== commit.out) return { ok: false, conflict: false, tree, detail: `the checks moved HEAD off the merged commit ${commit.out.slice(0, 12)}, so it was not the tree they passed` }; + const changed = git(dir, [...NO_HOOKS, 'status', '--porcelain', '--untracked-files=no']); + if (!changed.ok) return { ok: false, conflict: false, setup: true, tree, detail: `git status failed on the merged tree ${tree.slice(0, 12)}: ${changed.err.split('\n')[0]}` }; + if (changed.out) return { ok: false, conflict: false, tree, detail: `the checks changed tracked files on the merged tree ${tree.slice(0, 12)}, so it was not the tree they passed: ${changed.out.split('\n').slice(0, 6).join(', ')}` }; + return { ok: true, tree }; + } finally { + git(root, [...NO_HOOKS, 'worktree', 'remove', '--force', dir]); + fs.rmSync(scratch, { recursive: true, force: true }); + } +} diff --git a/lib/result-codes-release.ts b/lib/result-codes-release.ts index f9b6b2020..571be831f 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_MERGED_CHECKS: { + anchor: 'queue-land-merged-checks', + summary: 'the base moved past the pin\'s merge base, and a land.merged_checks command failed on the pin merged onto the base tip (GST-1316: a stale-base PR pushed a budgeted file past its cap on main); nothing merged', + fix: 'merge the base into the branch, fix what the failing command names, push, wait for the required checks, run /ship and gstack-ship-queue ready at the new head, then land it', + }, 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-policy.ts b/lib/ship-policy.ts index 4cba19c2d..8a567e6ec 100644 --- a/lib/ship-policy.ts +++ b/lib/ship-policy.ts @@ -17,6 +17,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { defaultBaseBranch, noDefaultBaseLine } from './default-base'; import { RELEASE_GATE_EXPLANATION, releaseGateErrors, type ReleaseGatePolicy } from './release-gate-policy'; +import { LAND_EXPLANATION, landPolicyErrors, type LandPolicy } from './land-merged-checks'; import { RESULT_CODES, type ResultCodeName } from './result-codes'; import { reviewErrors, type ReviewPolicy } from './ship-policy-review'; import { cmpVersion, parseVersion } from './version-source'; @@ -90,6 +91,7 @@ export interface ShipPolicy { /** The release gate on the base branch (docs/ship-policy.md#release-gate); null keeps the per-push main backstop. */ release_gate: ReleaseGatePolicy | null; /** `direct` lands each READY PR at its own green head (`land --pin`, docs/ship-policy.md#land-direct); `train` keeps merge trains. */ merge: 'direct' | 'train'; + /** What `land --pin` runs on the pin merged onto a base that moved past its merge base (lib/land-merged-checks.ts); null lands a stale base untested with a STALE_BASE: warn line. */ land: LandPolicy | null; } /** `base_red`: `setup` installs dependencies in a scratch checkout (null when none are needed); `select` runs the failing test files, named by its `{files}` slot. */ @@ -180,7 +182,7 @@ export const DEFAULT_POLICY: ShipPolicy = { min_gstack_version: null, train: null, docs: null, - base_red: null, merge: 'train', + base_red: null, merge: 'train', land: null, release_gate: null, }; @@ -216,6 +218,7 @@ export const POLICY_EXPLANATIONS: Record = { docs: '{ caps: [{ path, max_bytes }] } or null: documentation files with a byte cap. /ship Step 14.5 runs `gstack-ship-receipt docs-caps` before the verification gate and prints one DOCS_CAP: line per cap; a file over its cap (or absent) blocks the ship until the doc is trimmed or the cap raised on the base branch. null declares none.', base_red: '{ setup, select, timeout_sec } or null: how /ship Step 5 proves a failure is the base branch\'s own (docs/ship-policy.md#base-red). gstack-base-red checks out the base head and the PR head in scratch worktrees, runs setup (dependency install, or null), then select with the failing test files in its {files} slot; only the same failures in both, in files the PR does not touch, are base-red, and /ship then pauses before READY instead of failing. Read from the base branch only. null keeps every failure in-branch.', release_gate: RELEASE_GATE_EXPLANATION, + land: LAND_EXPLANATION, merge: 'train (default) | direct. direct: the Captain lands each READY PR at its own green head with `gstack-ship-queue land --pr --pin ` (no train, no stamp commit; the PR leaves VERSION, the manifest version and the CHANGELOG at the base and the release stamps them), and train build refuses. train: merge trains as before (docs/ship-policy.md#land-direct).', min_gstack_version: 'The oldest gstack that may read this file. Any key newer than 1.91.98.0 set to a non-default value requires it at that key’s introducing version (validate prints each); a gstack older than it refuses the file (POLICY_GSTACK_TOO_OLD).', }; @@ -258,6 +261,7 @@ export const REVIEW_EXCLUDE_KEY_VERSION = '1.93.33.0'; export const RELEASE_GATE_KEY_VERSION = '1.96.0.0'; export const REVIEW_POLICY_KEY_VERSION = '1.94.30.0'; export const MERGE_KEY_VERSION = '1.95.0.0'; +export const LAND_KEY_VERSION = '1.96.0.0'; export const POLICY_KEY_INTRODUCED: Record = { bump: '1.91.73.0', queue_mode: '1.91.73.0', release_tool: '1.91.73.0', release_outputs: '1.91.73.0', stamp_paths: '1.91.73.0', changelog: '1.91.73.0', mirror: '1.91.73.0', history: '1.91.73.0', preregistration_shas: '1.91.73.0', stamp_sensitive_gates: '1.91.73.0', @@ -266,7 +270,7 @@ export const POLICY_KEY_INTRODUCED: Record = { train: TRAIN_KEYS_VERSION, review_scope: REVIEW_SCOPE_KEY_VERSION, docs: DOCS_CAPS_KEY_VERSION, changelog_fragments: CHANGELOG_FRAGMENTS_KEY_VERSION, base_red: BASE_RED_KEY_VERSION, review_exclude_paths: REVIEW_EXCLUDE_KEY_VERSION, review: REVIEW_POLICY_KEY_VERSION, release_gate: RELEASE_GATE_KEY_VERSION, - stamp_placeholders: STAMP_PLACEHOLDERS_KEY_VERSION, merge: MERGE_KEY_VERSION, + stamp_placeholders: STAMP_PLACEHOLDERS_KEY_VERSION, merge: MERGE_KEY_VERSION, land: LAND_KEY_VERSION, }; const newer = (a: string, b: string): boolean => cmpVersion(parseVersion(a)!, parseVersion(b)!) > 0; @@ -520,6 +524,7 @@ export function validatePolicy(raw: unknown, opts: { gstackVersion?: string } = if ('changelog_fragments' in obj && obj.changelog_fragments !== null) fragmentsErrors(obj.changelog_fragments, errors); if ('base_red' in obj && obj.base_red !== null) baseRedErrors(obj.base_red, errors); if ('release_gate' in obj && obj.release_gate !== null) errors.push(...releaseGateErrors(obj.release_gate)); + if ('land' in obj && obj.land !== null) errors.push(...landPolicyErrors(obj.land)); if ('stamp_paths' in obj) listErrors('stamp_paths', obj.stamp_paths, errors, globError); for (const key of ['changelog', 'mirror'] as const) { if (!(key in obj) || obj[key] === null) continue; diff --git a/lib/ship-queue-land-direct.ts b/lib/ship-queue-land-direct.ts index ef5ded962..98dd0ed91 100644 --- a/lib/ship-queue-land-direct.ts +++ b/lib/ship-queue-land-direct.ts @@ -1,24 +1,31 @@ /** * 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 + * 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 - * after GitHub merged goes straight to the tree check. + * and GitHub reports the PR mergeable and clean. Then land takes the land lock and re-fetches the base. When the base + * moved past the pin's merge base, the checks at the pin never saw the tree that would land (GST-1316), so land builds + * the pin merged onto the base tip and runs the base policy's land.merged_checks there (lib/land-merged-checks.ts; + * repo commands, so only under --allow-repo-commands and never under --dry-run), refusing on a conflict or a failure; + * with no list it prints a STALE_BASE: warn line and lands. 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 (its merged checks rerun when that parent is + * neither the base land checked nor in the pin), a land note on refs/notes/gstack-trains, and the lock's release. A + * rerun after GitHub merged goes straight to the tree check. */ +import { runMergedChecks } from './land-merged-checks'; import { classifyRelease, RELEASE_BRANCH } from './release-stamp'; import { versionAtRef } from './restamp'; import type { ShipPolicy } from './ship-policy'; import { ciAtHead, loadBasePolicy, type QueueContext, type StepOutput } from './ship-queue'; import { git, type QueueFailure } from './ship-queue-gate'; -import { lockLabel, readLock } from './ship-queue-land-lock'; +import { acquireLandLock, lockLabel, ownsLock, readLock, type HeldLandLock, type LandLock } from './ship-queue-land-lock'; +import * as os from 'node:os'; import { activeFreeze, loadReady, persist, rebuildFromNotes, repoIdentity } from './ship-queue-train-record'; import { bindShippedReceipt } from './ship-receipt-depth'; interface DirectPr { state: string; baseRefName?: string; headRefName: string; headRefOid: string; isCrossRepository: boolean; body: string; mergeable?: string; mergeStateStatus?: string; mergeCommit?: { oid: string } | null } -export interface DirectOptions { pr: string; pin: string; dryRun?: boolean; now?: () => Date } +export interface DirectOptions { pr: string; pin: string; dryRun?: boolean; allowRepoCommands?: boolean; breakLock?: boolean; now?: () => Date } const isSha = (s: string | undefined) => /^[0-9a-f]{40,64}$/.test(s ?? ''); @@ -50,7 +57,7 @@ function havePin(root: string, pr: string, pin: string, branch?: string): boolea } /** The merge commit's tree is the pin merged onto the merge commit's first parent: GitHub merged exactly the pin. A read that fails is "cannot verify", never a mismatch. */ -function verifyMerge(ctx: QueueContext, root: string, pr: string, pin: string, merge: string, lines: string[], run: string, now: Date, stamped?: string | 'pin-parent'): StepOutput { +function verifyMerge(ctx: QueueContext, root: string, pr: string, pin: string, merge: string, lines: string[], run: string, now: Date, stamped?: string | 'pin-parent', merged: { checks: string[]; on?: string; allow?: boolean } = { checks: [] }): StepOutput { const cannot = (why: string): StepOutput => ({ ok: false, run, lines, failure: { code: 'QUEUE_NOT_READY', message: `#${pr} merged as ${merge.slice(0, 12)}, but land cannot verify it: ${why}`, fix: `fix the read named, then rerun gstack-ship-queue land --pr ${pr} --pin ${pin}; it goes straight to the tree check` } }); if (!git(root, ['fetch', '-q', 'origin', ctx.base]).ok) return cannot(`git fetch origin ${ctx.base} failed`); if (!havePin(root, pr, pin)) return cannot(`the pin ${pin.slice(0, 12)} is not in this clone and refs/pull/${pr}/head did not fetch it`); @@ -58,6 +65,18 @@ function verifyMerge(ctx: QueueContext, root: string, pr: string, pin: string, m if (stampedOn !== undefined && !isSha(stampedOn)) return cannot(`cannot read the parent of the release pin ${pin.slice(0, 12)}, the ${ctx.base} commit it was stamped on`); if (!git(root, ['merge-base', '--is-ancestor', merge, `origin/${ctx.base}`]).ok) return { ok: false, run, lines, failure: { code: 'QUEUE_LAND_UNVERIFIED', message: `the merge commit ${merge.slice(0, 12)} is not on origin/${ctx.base}` } }; const parent = git(root, ['rev-parse', `${merge}^1`]).out; + if (!stampedOn && parent !== merged.on && !git(root, ['merge-base', '--is-ancestor', parent, pin]).ok) { + const onto = `#${pr} merged as ${merge.slice(0, 12)} onto ${parent.slice(0, 12)}, which the pin ${pin.slice(0, 12)} does not contain`; + const unverified = (why: string): StepOutput => ({ ok: false, run, lines, failure: { code: 'QUEUE_LAND_UNVERIFIED', message: `${onto}, so the checks at the pin never ran on the tree that landed, and ${why}`, fix: `run the free checks on origin/${ctx.base} at ${merge.slice(0, 12)} now; if one is red, fix forward or revert #${pr}, and report it to the Captain` } }); + if (merged.on === undefined && !merged.checks.length) lines.push(`STALE_BASE: warn ${onto}; no land.merged_checks configured, so the rerun checks the tree only`); + else if (!merged.checks.length) return unverified('the base policy lists no land.merged_checks to run on it'); + else if (!merged.allow) return { ok: false, run, lines, failure: { code: 'REPO_COMMANDS_NOT_ALLOWED', message: `${onto}, so land must run land.merged_checks on the tree that landed; they are repo commands, so nothing ran`, fix: `rerun gstack-ship-queue land --pr ${pr} --pin ${pin} --allow-repo-commands` } }; + else { + const mc = runMergedChecks(root, parent, pin, merged.checks); + if (!mc.ok) return mc.setup ? cannot(mc.detail) : unverified(`land.merged_checks did not pass on it: ${mc.detail}`); + lines.push(`MERGED_CHECKS: pass tree=${mc.tree} base=${parent} (after the merge, on the base GitHub merged onto)`); + } + } if (stampedOn && parent !== stampedOn) return { ok: false, run, lines, failure: { code: 'QUEUE_LAND_UNVERIFIED', message: `the release merged as ${merge.slice(0, 12)} onto ${parent.slice(0, 12)}, not onto ${stampedOn.slice(0, 12)} it was stamped on: a PR landed in between and ships under this version while its fragment waits for the next release`, fix: 'tell the Captain; the next release folds that fragment, so note the PR in its release notes' } }; const tree = git(root, ['rev-parse', `${merge}^{tree}`]).out; const mt = git(root, ['merge-tree', '--write-tree', parent, pin]); @@ -87,9 +106,21 @@ export function landDirect(ctx: QueueContext, o: DirectOptions): StepOutput { const lines = [`LAND: #${o.pr} pin=${o.pin} direct`]; if (info.isCrossRepository) return fail(lines, { code: 'QUEUE_TRAIN_UNTRUSTED', message: `#${o.pr} is a fork head; land merges only crew branches` }); if (info.baseRefName && info.baseRefName !== ctx.base) return fail(lines, { code: 'QUEUE_NOT_READY', message: `#${o.pr} targets ${info.baseRefName}, not ${ctx.base}`, fix: `retarget it to ${ctx.base}, or land its parent first` }); + const remote = ctx.remote ?? 'origin'; + const journal = `land-pin:${o.pin}`; + const takeLock = () => acquireLandLock({ + root, remote, holder: `${os.hostname()}:${process.pid}`, owner: { pr: o.pr }, journal, now, phase: () => 'merging', breakLock: o.breakLock, + takeover: (held: LandLock) => !held.train && held.pr === o.pr && held.journal.startsWith('land-pin:') ? `LOCK: taking over #${o.pr}'s land lock that an earlier land --pin run left (${held.journal}, since ${held.started_at})` : null, + }); if (info.state === 'MERGED' && info.headRefOid === o.pin && isSha(info.mergeCommit?.oid)) { lines.push(`#${o.pr} is already merged as ${info.mergeCommit!.oid.slice(0, 12)}; checking the tree`); - return verifyMerge(ctx, root, o.pr, o.pin, info.mergeCommit!.oid, lines, run, now(), RELEASE_BRANCH.test(info.headRefName) ? 'pin-parent' : undefined); + if (o.dryRun) return { ok: true, run, lines: [...lines, 'DRY-RUN: nothing verified (a rerun without --dry-run runs the tree check and writes the land note)'] }; + const out = verifyMerge(ctx, root, o.pr, o.pin, info.mergeCommit!.oid, lines, run, now(), RELEASE_BRANCH.test(info.headRefName) ? 'pin-parent' : undefined, { checks: policy.land?.merged_checks ?? [], allow: o.allowRepoCommands }); + const left = readLock(root, remote); + if (left.state !== 'held' || !ownsLock(left.lock, { pr: o.pr }, journal)) return out; + const got = takeLock(); + const freed = 'held' in got && got.held.release(); + return { ...out, lines: [...out.lines, freed ? `LOCK: released the land lock an earlier run of this land left` : `LOCK: warn the land lock an earlier run of this land left is still held; rerun this land to release it`] }; } if (info.state !== 'OPEN') return fail(lines, { code: 'QUEUE_NOT_READY', message: `#${o.pr} is ${info.state}` }); if (info.headRefOid !== o.pin) return fail(lines, { code: 'QUEUE_LAND_PIN_MOVED', message: `#${o.pr}'s head is ${info.headRefOid.slice(0, 12)}, not the pin ${o.pin.slice(0, 12)}`, fix: `the lane runs /ship and gstack-ship-queue ready --pr ${o.pr} at the new head and reports READY with it` }); @@ -101,8 +132,9 @@ export function landDirect(ctx: QueueContext, o: DirectOptions): StepOutput { const release = named ? classifyRelease({ repoRoot: root, policy, mergeBase, head: o.pin }) : null; if (release?.ok && release.version !== named) return fail(lines, { code: 'RELEASE_STAMP_INVALID', message: `${info.headRefName} stamps v${release.version}, not the version its branch names`, fix: 'close it and run gstack-release-stamp open again' }); if (release && !release.ok) return fail(lines, { code: 'RELEASE_STAMP_INVALID', message: `${info.headRefName} is not the release stamp of its merge base ${mergeBase.slice(0, 12)}: ${release.reason}`, fix: 'close it and run gstack-release-stamp open again; never edit a release branch by hand' }); - const tip = git(root, ['rev-parse', `origin/${ctx.base}`]).out; - if (release && mergeBase !== tip) return fail(lines, { code: 'RELEASE_STAMP_INVALID', message: `origin/${ctx.base} moved to ${tip.slice(0, 12)} since ${info.headRefName} was stamped on ${mergeBase.slice(0, 12)}; the PRs that landed since would ship under this version while their fragments wait for the next release`, fix: 'run gstack-release-stamp open again (it replaces the release branch), then land the new head' }); + const releaseMoved = (tip: string): QueueFailure | null => release && mergeBase !== tip ? { code: 'RELEASE_STAMP_INVALID', message: `origin/${ctx.base} moved to ${tip.slice(0, 12)} since ${info.headRefName} was stamped on ${mergeBase.slice(0, 12)}; the PRs that landed since would ship under this version while their fragments wait for the next release`, fix: 'run gstack-release-stamp open again (it replaces the release branch), then land the new head' } : null; + const moved = releaseMoved(git(root, ['rev-parse', `origin/${ctx.base}`]).out); + if (moved) return fail(lines, moved); const keys = repoIdentity(root).keys; rebuildFromNotes(root, ctx.remote ?? 'origin'); if (release) lines.push(`RELEASE: v${release.version} folds ${release.fragments.length} fragment(s) and changes nothing but the stamp, so it needs no /ship receipt or READY note`); @@ -114,8 +146,8 @@ export function landDirect(ctx: QueueContext, o: DirectOptions): StepOutput { } const freeze = activeFreeze(keys); if (freeze) return fail(lines, { code: 'QUEUE_TRAIN_FROZEN', message: `main is frozen by train ${freeze.train} (${freeze.reason})` }); - const lock = readLock(root, ctx.remote ?? 'origin'); - if (lock.state === 'held') return fail(lines, { code: 'QUEUE_LAND_LOCKED', message: `${lockLabel(lock.lock)} holds the land lock; a direct land would race it onto main` }); + const lock = o.dryRun ? readLock(root, remote) : null; + if (lock?.state === 'held') return fail(lines, { code: 'QUEUE_LAND_LOCKED', message: `${lockLabel(lock.lock)} holds the land lock; a direct land would race it onto main` }); const stamped = release ? [] : stampedFiles(root, policy, mergeBase, o.pin); if ('error' in stamped) return fail(lines, { code: 'QUEUE_NOT_READY', message: `cannot read the version at the pin or its merge base: ${stamped.error}` }); @@ -133,17 +165,52 @@ export function landDirect(ctx: QueueContext, o: DirectOptions): StepOutput { 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 (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})`] }; - - const method = policy.history === 'merge-only' ? '--merge' : '--squash'; - const m = ctx.gh(['pr', 'merge', o.pr, method, '--match-head-commit', o.pin]); - let merge: string | undefined; - for (let i = 0; i < (m.ok ? 4 : 1) && !isSha(merge); i++) { - if (i) Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 3_000); - const after = view(ctx, o.pr); - merge = 'code' in after || after.state !== 'MERGED' ? undefined : after.mergeCommit?.oid; + let held: HeldLandLock | undefined; + if (!o.dryRun) { + const got = takeLock(); + lines.push(...got.lines); + if ('failure' in got) return fail(lines, got.failure); + held = got.held; } - if (!isSha(merge) && m.ok) return fail(lines, { code: 'QUEUE_NOT_READY', message: `gh pr merge accepted #${o.pr}, but GitHub does not show it merged yet; nothing is verified`, fix: `rerun gstack-ship-queue land --pr ${o.pr} --pin ${o.pin} in a minute; once GitHub shows the merge it goes straight to the tree check` }); - if (!isSha(merge)) return fail(lines, { code: 'QUEUE_NOT_READY', message: `gh pr merge refused: ${(m.err ?? m.out).trim().slice(0, 200) || 'the PR is not merged'}`, fix: `read the refusal; rerun land --pr ${o.pr} --pin ${o.pin} once it is fixed` }); - return verifyMerge(ctx, root, o.pr, o.pin, merge!, lines, run, now(), release ? mergeBase : undefined); + const locked = (): StepOutput => { + if (held && !git(root, ['fetch', '-q', 'origin', ctx.base]).ok) return fail(lines, { code: 'QUEUE_NOT_READY', message: `cannot re-fetch origin/${ctx.base} under the land lock`, fix: 'fix the fetch, then rerun land' }); + const tip = git(root, ['rev-parse', `origin/${ctx.base}`]).out; + const current = loadBasePolicy(ctx); + if ('message' in current) return fail(lines, current); + if (current.policy.merge !== 'direct') return fail(lines, { code: 'QUEUE_NOT_READY', message: `the base policy at ${tip.slice(0, 12)} no longer sets merge: direct`, fix: 'land through the train' }); + const checks = current.policy.land?.merged_checks ?? []; + const staleRelease = releaseMoved(tip); + if (staleRelease) return fail(lines, staleRelease); + if (mergeBase !== tip) { + const moved = `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`; + if (!checks.length) lines.push(`STALE_BASE: warn ${moved}; no land.merged_checks configured, so land merges it untested`); + else if (o.dryRun) lines.push(`DRY-RUN: land.merged_checks not run (repo commands never run under --dry-run); a real land with --allow-repo-commands runs ${checks.length} on the pin merged onto ${tip.slice(0, 12)}`); + else if (!o.allowRepoCommands) return fail(lines, { code: 'REPO_COMMANDS_NOT_ALLOWED', message: `${moved}, so land must run land.merged_checks on the pin merged onto it; they are repo commands (the merged tree's package.json decides what they run), so nothing ran and nothing merged`, fix: `rerun gstack-ship-queue land --pr ${o.pr} --pin ${o.pin} --allow-repo-commands` }); + else { + const mc = runMergedChecks(root, tip, o.pin, checks); + if (!mc.ok && mc.setup) return fail(lines, { code: 'QUEUE_NOT_READY', message: `${moved}, and land could not run land.merged_checks on the pin merged onto it: ${mc.detail}; nothing merged`, fix: `if the checkout is the problem, fix it and rerun land; if the dependencies changed, the lane merges origin/${ctx.base} into the branch, waits for the required checks, runs /ship and ready at the new head` }); + if (!mc.ok) return fail(lines, mc.conflict ? { code: 'QUEUE_LAND_NOT_CLEAN', message: `${moved}: ${mc.detail}`, fix: `the lane merges origin/${ctx.base} into the branch, resolves, runs /ship and ready at the new head` } : { code: 'QUEUE_LAND_MERGED_CHECKS', message: `${moved}; on the pin merged onto it, ${mc.detail}`, fix: `merge the base into the branch (git merge origin/${ctx.base}), fix what the failing check names, 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` }); + lines.push(`MERGED_CHECKS: pass tree=${mc.tree} base=${tip} pin=${o.pin.slice(0, 12)} checks: ${checks.join('; ')}`); + } + } + 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})`] }; + + const method = policy.history === 'merge-only' ? '--merge' : '--squash'; + const lost = held!.set('merging'); + if (lost) return fail(lines, lost); + const m = ctx.gh(['pr', 'merge', o.pr, method, '--match-head-commit', o.pin]); + let merge: string | undefined; + for (let i = 0; i < (m.ok ? 4 : 1) && !isSha(merge); i++) { + if (i) Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 3_000); + const after = view(ctx, o.pr); + merge = 'code' in after || after.state !== 'MERGED' ? undefined : after.mergeCommit?.oid; + } + if (!isSha(merge) && m.ok) return fail(lines, { code: 'QUEUE_NOT_READY', message: `gh pr merge accepted #${o.pr}, but GitHub does not show it merged yet; nothing is verified`, fix: `rerun gstack-ship-queue land --pr ${o.pr} --pin ${o.pin} in a minute; once GitHub shows the merge it goes straight to the tree check` }); + if (!isSha(merge)) return fail(lines, { code: 'QUEUE_NOT_READY', message: `gh pr merge refused: ${(m.err ?? m.out).trim().slice(0, 200) || 'the PR is not merged'}`, fix: `read the refusal; rerun land --pr ${o.pr} --pin ${o.pin} once it is fixed` }); + return verifyMerge(ctx, root, o.pr, o.pin, merge!, lines, run, now(), release ? mergeBase : undefined, { checks, on: tip, allow: o.allowRepoCommands }); + }; + let out: StepOutput; + let released = true; + try { out = locked(); } finally { if (held) released = held.release(); } + return released ? out : { ...out, lines: [...out.lines, `LOCK: warn the land lock release push failed, so the lock stays held; rerun gstack-ship-queue land --pr ${o.pr} --pin ${o.pin} to resume and release it`] }; } diff --git a/test/ship-queue-land-direct.test.ts b/test/ship-queue-land-direct.test.ts index bb8d32a34..b82ba03ca 100644 --- a/test/ship-queue-land-direct.test.ts +++ b/test/ship-queue-land-direct.test.ts @@ -8,18 +8,23 @@ import { afterEach, describe, expect, test } from 'bun:test'; import { spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; +import * as os from 'node:os'; import * as path from 'node:path'; import { EXIT } from '../lib/headless-artifacts'; -import { MERGE_KEY_VERSION, validatePolicy } from '../lib/ship-policy'; +import { LAND_KEY_VERSION, MERGE_KEY_VERSION, validatePolicy } from '../lib/ship-policy'; import { BASE_POLICY, cleanup, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/restamp-fixture'; import { shippedBody } from './helpers/ship-queue-train-fixture'; import { persist, repoIdentity } from '../lib/ship-queue-train-record'; import { rewriteStamps, tentativeMentions } from '../lib/restamp'; import { bumpVersion } from '../lib/version-gate-train'; import { openRelease } from '../lib/release-stamp'; +import { runMergedChecks } from '../lib/land-merged-checks'; +import { readLock, writeLock } from '../lib/ship-queue-land-lock'; const REQUIRED = ['free-tests', 'Check VERSION is not stale vs queue']; const POLICY = { ...BASE_POLICY, release_tool: null, release_outputs: [], changelog_fragments: { dir: 'changelog.d', required_checks: REQUIRED }, merge: 'direct', min_gstack_version: MERGE_KEY_VERSION }; +/** The base policy with land.merged_checks: the first proves the checks see the merged tree, the second stands in for a budget cap a stale base can break (#3886). */ +const CHECKED = { ...POLICY, land: { merged_checks: ['test -e other.txt && ls pr-*.txt >/dev/null', "test ! -e budget.md || test ! -e other.txt || { echo 'budget.md over its cap' >&2; exit 1; }"] }, min_gstack_version: LAND_KEY_VERSION }; const repos: FixtureRepo[] = []; afterEach(() => { while (repos.length) cleanup(repos.pop()!); }); @@ -39,6 +44,7 @@ function setup(policy: Record = POLICY): Fx { "const g=(...x)=>execFileSync('git',['--git-dir',O,...x],{encoding:'utf8',timeout:30000}).trim();", "const save=()=>fs.writeFileSync(D,JSON.stringify(d));", "if(a[0]==='pr'&&a[1]==='view'){const p=d.prs[a[2]];if(!p){console.error('no pr');process.exit(1)}", + " if(d.mainOnView){g('update-ref','refs/heads/main',d.mainOnView);delete d.mainOnView;save()}", " let head='';try{head=g('rev-parse','refs/heads/'+p.headRefName)}catch{};", " console.log(JSON.stringify({mergeable:'MERGEABLE',mergeStateStatus:'CLEAN',...p,headRefOid:p.headRefOid??head,mergeCommit:p.mergeSha?{oid:p.mergeSha}:null}));process.exit(0)}", "if(a[0]==='api'){const m=/commits\\/([0-9a-f]+)\\/(check-runs|status)/.exec(a[1]);if(!m)process.exit(1);", @@ -48,7 +54,9 @@ function setup(policy: Record = POLICY): Fx { " console.log(JSON.stringify({total_count:runs.length,check_runs:runs}));process.exit(0)}", "if(a[0]==='pr'&&a[1]==='merge'){const p=d.prs[a[2]];const want=a[a.indexOf('--match-head-commit')+1];const head=g('rev-parse','refs/heads/'+p.headRefName);", " if(head!==want){console.error('Head branch was modified');process.exit(1)}", - " const main=g('rev-parse','refs/heads/main');let tree=g('merge-tree','--write-tree',main,want).split('\\n')[0];", + " try{d.lockAtMerge=JSON.parse(g('show','refs/notes/gstack-land:lock.json'))}catch{}", + " let main=g('rev-parse','refs/heads/main');if(d.raceMain){main=g('commit-tree',g('rev-parse',main+'^{tree}'),'-p',main,'-m','landed during the check');g('update-ref','refs/heads/main',main)}", + " let tree=g('merge-tree','--write-tree',main,want).split('\\n')[0];", " if(d.strayOnMerge){tree=g('rev-parse',main+'^{tree}')}", " const sq=g('commit-tree',tree,'-p',main,'-m','squash #'+a[2]);g('update-ref','refs/heads/main',sq);", " d.mergeArgs=a.slice(2);p.state='MERGED';p.mergeSha=sq;save();process.exit(0)}", @@ -131,18 +139,181 @@ 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', () => { - const fx = setup(); - const pin = openPr(fx, '8'); + test('GST-1316: the base moved after the pin\'s CI, so land runs land.merged_checks on the pin merged onto main: a failure refuses with QUEUE_LAND_MERGED_CHECKS and merges nothing, a pass lands and names the merged tree, and a rerun checks again', () => { + const fx = setup(CHECKED); + const red = openPr(fx, '8', { edit: w => fs.writeFileSync(path.join(w, 'budget.md'), 'pr line\n') }); + const green = openPr(fx, '82'); const moved = moveMain(fx, 'other.txt', 'other\n'); - const r = land(fx, '8', pin); - expect(r.err).toBe(''); - expect(r.status).toBe(EXIT.ok); + const r = land(fx, '8', red, ['--allow-repo-commands']); + expect(r.status).not.toBe(EXIT.ok); + expect(r.err).toContain('(QUEUE_LAND_MERGED_CHECKS)'); + expect(r.err).toContain(moved.slice(0, 12)); + expect(r.err).toContain('budget.md over its cap'); + expect(fx.read().mergeArgs).toBeUndefined(); + expect(remote(fx, 'main')).toBe(moved); + + const want = fx.repo.git('merge-tree', '--write-tree', moved, green); + const ok = land(fx, '82', green, ['--allow-repo-commands']); + expect(ok.err).toBe(''); + expect(ok.status).toBe(EXIT.ok); + expect(ok.out).toContain(`MERGED_CHECKS: pass tree=${want} base=${moved} pin=${green.slice(0, 12)} checks: test -e other.txt && ls pr-*.txt >/dev/null; `); const main = remote(fx, 'main'); expect(fx.repo.git('rev-parse', main + '^1')).toBe(moved); - expect(show(fx, main + ':other.txt')).toBe('other'); - expect(show(fx, main + ':pr-8.txt')).toBe('8'); - expect(fx.repo.git('rev-parse', 'origin/pr-8')).toBe(pin); + expect(fx.repo.git('rev-parse', main + '^{tree}')).toBe(want); + expect(landNotes(fx)).toHaveLength(1); + const again = land(fx, '82', green, ['--allow-repo-commands']); + expect(again.err).toBe(''); + expect(again.status).toBe(EXIT.ok); + expect(again.out).toContain(`MERGED_CHECKS: pass tree=${want} base=${moved} (after the merge`); + }, 90_000); + + test('no land key: a stale base lands untested as before, with a STALE_BASE: warn line naming the missing land.merged_checks', () => { + const fx = setup(); + const pin = openPr(fx, '84'); + const moved = moveMain(fx, 'other.txt', 'other\n'); + const r = land(fx, '84', pin); + expect(r.err).toBe(''); + expect(r.status).toBe(EXIT.ok); + expect(r.out).toContain(`STALE_BASE: warn origin/main moved to ${moved.slice(0, 12)}`); + expect(r.out).toContain('no land.merged_checks configured'); + expect(r.out).not.toContain('MERGED_CHECKS:'); + const main = remote(fx, 'main'); + expect(fx.repo.git('rev-parse', main + '^1')).toBe(moved); + expect(fx.repo.git('rev-parse', main + '^{tree}')).toBe(fx.repo.git('merge-tree', '--write-tree', moved, pin)); + expect(landNotes(fx)).toHaveLength(1); + const again = land(fx, '84', pin); + expect(again.err).toBe(''); + expect(again.status).toBe(EXIT.ok); + expect(again.out).toContain('already merged'); + expect(again.out).toContain('no land.merged_checks configured, so the rerun checks the tree only'); + expect(again.out).toMatch(/^LANDED: /m); + }, 90_000); + + test('land.merged_checks are repo commands: refused without --allow-repo-commands, skipped under --dry-run, not run when GitHub is not clean, and run with a scrubbed environment under the land lock', () => { + const marker = path.join(os.tmpdir(), `gstack-land-marker-${process.pid}-${Date.now()}`); + const fx = setup({ ...POLICY, land: { merged_checks: [`echo "${'$'}HOME|${'$'}{GH_TOKEN-}|${'$'}{LAND_SHIM_DATA-}" > ${marker}`] }, min_gstack_version: LAND_KEY_VERSION }); + try { + const pin = openPr(fx, '87'); + moveMain(fx, 'other.txt', 'other\n'); + fx.env.GH_TOKEN = 'placeholder'; + const no = land(fx, '87', pin); + expect(no.status).not.toBe(EXIT.ok); + expect(no.err).toContain('(REPO_COMMANDS_NOT_ALLOWED)'); + expect(fs.existsSync(marker)).toBe(false); + expect(fx.read().mergeArgs).toBeUndefined(); + + const dry = land(fx, '87', pin, ['--dry-run', '--allow-repo-commands']); + expect(dry.err).toBe(''); + expect(dry.status).toBe(EXIT.ok); + expect(dry.out).toContain('DRY-RUN: land.merged_checks not run'); + expect(fs.existsSync(marker)).toBe(false); + + fx.write(d => { d.prs['87'].mergeStateStatus = 'BLOCKED'; }); + const blocked = land(fx, '87', pin, ['--allow-repo-commands']); + expect(blocked.err).toContain('(QUEUE_LAND_NOT_CLEAN)'); + expect(fs.existsSync(marker)).toBe(false); + fx.write(d => { d.prs['87'].mergeStateStatus = 'CLEAN'; }); + + const ok = land(fx, '87', pin, ['--allow-repo-commands']); + expect(ok.err).toBe(''); + expect(ok.status).toBe(EXIT.ok); + const [home, token, shim] = fs.readFileSync(marker, 'utf8').trim().split('|'); + expect(home).not.toBe(process.env.HOME); + expect(home).not.toBe(''); + expect(token).toBe(''); + expect(shim).toBe(''); + expect(fx.read().lockAtMerge).toMatchObject({ pr: '87', phase: 'merging' }); + expect(JSON.parse(fx.repo.git('--git-dir', fx.repo.origin, 'show', 'refs/notes/gstack-land:lock.json'))).toMatchObject({ released: true }); + } finally { fs.rmSync(marker, { force: true }); } + }, 90_000); + + test('QUEUE_LAND_NOT_CLEAN: on a stale base the pin conflicts with main, so no merged check runs and nothing merges', () => { + const fx = setup(CHECKED); + const pin = openPr(fx, '83', { edit: w => fs.writeFileSync(path.join(w, 'other.txt'), 'pr\n') }); + const moved = moveMain(fx, 'other.txt', 'main\n'); + const r = land(fx, '83', pin, ['--allow-repo-commands']); + expect(r.status).not.toBe(EXIT.ok); + expect(r.err).toContain('(QUEUE_LAND_NOT_CLEAN)'); + expect(r.err).toContain('conflicts with ' + moved.slice(0, 12) + ' in other.txt'); + expect(fx.read().mergeArgs).toBeUndefined(); + }, 90_000); + + test('main moved between the stale-base check and GitHub\'s merge: land reports QUEUE_LAND_UNVERIFIED and writes no land note; with no land.merged_checks a rerun cannot tell which base land checked, so it checks the tree only', () => { + const fx = setup(); + const pin = openPr(fx, '81'); + fx.write(d => { d.raceMain = true; }); + const r = land(fx, '81', pin); + expect(r.status).not.toBe(EXIT.ok); + expect(r.err).toContain('(QUEUE_LAND_UNVERIFIED)'); + expect(r.err).toContain('does not contain'); + expect(landNotes(fx)).toEqual([]); + const again = land(fx, '81', pin); + expect(again.out).toContain('already merged'); + expect(again.out).toContain('so the rerun checks the tree only'); + expect(again.status).toBe(EXIT.ok); + }, 90_000); + + test('the land lock: a lock an interrupted run left for the same PR is taken over; another PR\'s lock refuses, --break-lock cannot break it mid-merge but breaks a stale one; a lock left after the merge is released by the rerun; a dry rerun verifies nothing', () => { + const fx = setup(); + const hold = (pr: string, journal: string, minutesAgo: number) => { + const at = new Date(Date.now() - minutesAgo * 60_000).toISOString(); + expect(writeLock(fx.repo.work, readLock(fx.repo.work).sha, { pr, holder: 'gone:1', started_at: at, phase_at: at, phase: 'merging', journal }).ok).toBe(true); + }; + const pin = openPr(fx, '88'); + hold('88', 'land-pin:' + '0'.repeat(40), 0); + const took = land(fx, '88', pin); + expect(took.err).toBe(''); + expect(took.out).toContain("LOCK: taking over #88's land lock"); + expect(readLock(fx.repo.work).state).toBe('released'); + + const other = openPr(fx, '89'); + hold('90', 'land-pin:' + '1'.repeat(40), 0); + expect(land(fx, '89', other).err).toContain('(QUEUE_LAND_LOCKED)'); + expect(land(fx, '89', other, ['--break-lock']).err).toContain('mid-merge'); + expect(fx.read().prs['89'].state).toBe('OPEN'); + hold('90', 'land-pin:' + '1'.repeat(40), 11); + const broke = land(fx, '89', other, ['--break-lock']); + expect(broke.err).toBe(''); + expect(broke.out).toContain("LOCK: broke #90's lock"); + + hold('89', 'land-pin:' + other, 0); + const again = land(fx, '89', other); + expect(again.err).toBe(''); + expect(again.out).toContain('LOCK: released the land lock an earlier run of this land left'); + expect(readLock(fx.repo.work).state).toBe('released'); + const dry = land(fx, '89', other, ['--dry-run']); + expect(dry.out).toContain('DRY-RUN: nothing verified'); + }, 120_000); + + test('main moved during the merge with land.merged_checks: they run on the landed tree; a rerun without --allow-repo-commands refuses with REPO_COMMANDS_NOT_ALLOWED and runs nothing', () => { + const fx = setup(CHECKED); + const pin = openPr(fx, '91'); + moveMain(fx, 'other.txt', 'other\n'); + fx.write(d => { d.raceMain = true; }); + const r = land(fx, '91', pin, ['--allow-repo-commands']); + expect(r.err).toBe(''); + expect(r.out).toContain('(after the merge, on the base GitHub merged onto)'); + const again = land(fx, '91', pin); + expect(again.err).toContain('(REPO_COMMANDS_NOT_ALLOWED)'); + expect(again.out).not.toContain('MERGED_CHECKS:'); + }, 90_000); + + test('land re-reads the base policy after the locked fetch: a main that adds land.merged_checks while land runs gets them run on the stale base', () => { + const fx = setup(); + const pin = openPr(fx, '93'); + const { repo } = fx; + repo.git('fetch', '-q', 'origin'); + repo.git('checkout', '-q', '--detach', 'origin/main'); + fs.writeFileSync(path.join(repo.work, '.gstack', 'ship-policy.json'), JSON.stringify({ ...POLICY, land: { merged_checks: ['echo merged check ran >&2; exit 1'] }, min_gstack_version: LAND_KEY_VERSION }, null, 2) + '\n'); + repo.git('commit', '-qam', 'main adds land.merged_checks'); + repo.git('push', '-q', 'origin', 'HEAD:refs/heads/next-main'); + const next = repo.git('rev-parse', 'HEAD'); + repo.git('checkout', '-q', '--detach', 'origin/main'); + fx.write(d => { d.mainOnView = next; }); + const r = land(fx, '93', pin, ['--allow-repo-commands']); + expect(r.err).toContain('(QUEUE_LAND_MERGED_CHECKS)'); + expect(r.err).toContain('merged check ran'); + expect(fx.read().mergeArgs).toBeUndefined(); }, 90_000); test('a rerun after GitHub merged the PR goes straight to the tree check and lands', () => { @@ -386,6 +557,57 @@ describe('merge: direct | train', () => { expect(errs({ merge: 'direct' })).toContain('introduced in gstack ' + MERGE_KEY_VERSION); }); + test('runMergedChecks: HEAD is the merged commit, no hook from the merged tree runs, a passing check that leaves a child behind passes at once; a check that times out (even one exiting 0 on SIGTERM) or that rewrites a tracked file fails', () => { + const fx = setup(CHECKED); + const hookMark = path.join(fx.repo.root, 'hook-ran'); + const pin = openPr(fx, '86'); + const moved = moveMain(fx, 'other.txt', 'other\n'); + fs.mkdirSync(path.join(fx.repo.work, '.hooks')); + fs.writeFileSync(path.join(fx.repo.work, '.hooks', 'post-checkout'), `#!/bin/sh\ntouch ${hookMark}\n`, { mode: 0o755 }); + fx.repo.git('config', 'core.hooksPath', '.hooks'); + const run = (checks: string[], ms?: number) => runMergedChecks(fx.repo.work, moved, pin, checks, ms); + const head = run(['git show HEAD:pr-86.txt && git show HEAD:other.txt && test "$(git rev-parse HEAD^1)" = ' + moved]); + expect(head.ok).toBe(true); + expect(fs.existsSync(hookMark)).toBe(false); + const started = Date.now(); + expect(run(['sleep 30 & exit 0'], 20_000).ok).toBe(true); + expect(Date.now() - started).toBeLessThan(10_000); + const slow = run(["trap 'exit 0' TERM; sleep 5; exit 1"], 500); + expect(slow.ok).toBe(false); + if (!slow.ok) expect(slow.detail).toContain('did not finish'); + const pidFile = path.join(fx.repo.root, 'grandchild.pid'); + const orphan = run([`sleep 30 & echo $! > ${pidFile}; wait`], 500); + expect(orphan.ok).toBe(false); + const alive = (pid: number) => { try { process.kill(pid, 0); return true; } catch { return false; } }; + expect(alive(Number(fs.readFileSync(pidFile, 'utf8').trim()))).toBe(false); + const stubborn = Date.now(); + expect(run(["trap '' TERM; sleep 5"], 300).ok).toBe(false); + expect(Date.now() - stubborn).toBeLessThan(3_000); + const moved2 = run(['git checkout -q HEAD^1']); + expect(moved2.ok).toBe(false); + if (!moved2.ok) expect(moved2.detail).toContain('moved HEAD'); + const depPin = openPr(fx, '92', { edit: w => { const p = JSON.parse(fs.readFileSync(path.join(w, 'package.json'), 'utf8')); p.dependencies['is-odd'] = '3.0.1'; fs.writeFileSync(path.join(w, 'package.json'), JSON.stringify(p, null, 2) + '\n'); } }); + const dep = runMergedChecks(fx.repo.work, moved, depPin, ['true']); + expect(dep.ok).toBe(false); + if (!dep.ok) { expect(dep.setup).toBe(true); expect(dep.detail).toContain('package.json dependencies differ'); } + const rewrite = run(['echo fixed > pr-86.txt', 'test "$(cat pr-86.txt)" = fixed']); + expect(rewrite.ok).toBe(false); + if (!rewrite.ok) expect(rewrite.detail).toContain('changed tracked files'); + expect(fx.repo.git('worktree', 'list').split('\n')).toHaveLength(1); + }, 90_000); + + test('land.merged_checks validates its commands and needs min_gstack_version', () => { + const errs = (o: Record) => validatePolicy(o).map(e => e.path + ': ' + e.message).join('\n'); + expect(errs({ land: { merged_checks: ['bun run typecheck'] }, min_gstack_version: LAND_KEY_VERSION })).toBe(''); + expect(errs({ land: null })).toBe(''); + expect(errs({ land: { merged_checks: ['bun run typecheck'] } })).toContain('introduced in gstack ' + LAND_KEY_VERSION); + const bad = (land: unknown) => errs({ land, min_gstack_version: LAND_KEY_VERSION }); + expect(bad({ merged_checks: [] })).toContain('land.merged_checks: must list 1 to 16 commands'); + expect(bad({ merged_checks: ['a'], timeout: 5 })).toContain('land.timeout: unknown key'); + expect(bad({ merged_checks: ['a\nb', ' '] })).toBe('land.merged_checks[0]: must be one line of at most 512 characters\nland.merged_checks[1]: must be a non-empty command'); + expect(bad(['a'])).toContain('land: must be { merged_checks } or null'); + }); + test('the key floor follows the stamp: the train restamp moves MERGE_KEY_VERSION to the stamped 1.x.N, an N install reads a policy with min N, an N-1 install gets the upgrade message', () => { const root = path.resolve(import.meta.dir, '..'); const fork = fs.readFileSync(path.join(root, 'VERSION'), 'utf8').trim();