mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 08:23:59 +01:00
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 343dd2bf16.
* 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>
This commit is contained in:
1 parent
068087f33c
commit
7ba250ba60
11 files changed
+540
-66
No files matched your search
@@ -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' ;;
|
||||
|
||||
@@ -14,7 +14,7 @@
|
||||
* gstack-ship-queue gate-ahead --list --pr <n>
|
||||
* gstack-ship-queue verify --pr <n> --screen (train policy: the head's screen verdict from its run)
|
||||
* gstack-ship-queue verify --post-merge <sha> [--timeout <s>] (after a merge: main's push workflows at that commit until they settle)
|
||||
* gstack-ship-queue land --pr <n> --pin <sha> [--dry-run] (merge: direct; lib/ship-queue-land-direct.ts)
|
||||
* gstack-ship-queue land --pr <n> --pin <sha> [--dry-run] [--allow-repo-commands] [--break-lock] (merge: direct; lib/ship-queue-land-direct.ts)
|
||||
* gstack-ship-queue land --pr <n> [--dry-run] [--no-wait] [--break-lock [--break-train-lock]] (the merge step; lib/ship-queue-land.ts)
|
||||
* gstack-ship-queue land --pr <n> --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 <n> --screen (train policy) the PR head's screen verdict read from its run (SCREEN_VERDICT)
|
||||
verify --post-merge <sha> [--timeout <s>] 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 <n> --pin <sha> [--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 <n> --pin <sha> [--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 <n> [--dry-run] [--no-wait] [--timeout <min>] [--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<number> {
|
||||
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 }));
|
||||
|
||||
@@ -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.
|
||||
+5
-3
@@ -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 <n> --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 <n> --pin <sha>`. It merges with
|
||||
2. Captain: `gstack-ship-queue land --pr <n> --pin <sha>` (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 <n> --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 <n> --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/<version>` 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 |
|
||||
|
||||
|
||||
+54
-11
@@ -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 <base>` (the caps come from `origin/<base>`'s policy, never the branch's copy; `--policy <file>` is for a repository bootstrapping its policy) before the verification gate and prints one `DOCS_CAP: <path> <bytes>/<max> 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 <n> --pin <sha>`](#land-direct): no train and no stamp commit, and `train build` refuses. A PR then leaves VERSION, the `package.json` version and the CHANGELOG at its merge base, and the release sets them. Needs `changelog_fragments`, and the stamping `land --pr <n>` refuses under it. `train` keeps merge trains. Read from the base revision only. Needs `min_gstack_version`. |
|
||||
| `land` | `{ merged_checks }` or `null` | `null` | Under `merge: direct`, the shell commands (1 to 16, one line of at most 512 characters each, 15 minutes each) [`land --pin`](#land-direct) runs on the pin merged onto the base tip when the base moved past the pin's merge base; it lands only when every one exits 0. They are repo commands, so land runs them only with `--allow-repo-commands`, never under `--dry-run`. `null` lands a stale base untested with a `STALE_BASE: warn` line. Read from the base revision only. Needs `min_gstack_version`. |
|
||||
| `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 <n> --pin <sha>`, where `<sha>`
|
||||
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/<version>`
|
||||
@@ -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:<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=<tree> base=<tip> pin=<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 <pin>` (`--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.
|
||||
|
||||
<a id="release-stamp"></a>
|
||||
@@ -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<version>` ([releases.md](releases.md)).
|
||||
|
||||
+30
-9
@@ -3228,10 +3228,12 @@ nothing.
|
||||
<a id="repo-commands-not-allowed"></a>
|
||||
### `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.
|
||||
|
||||
<a id="queue-land-merged-checks"></a>
|
||||
### `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.
|
||||
|
||||
<a id="queue-land-not-clean"></a>
|
||||
### `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 <parent>
|
||||
<pin>`, 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 <parent> <pin>`, 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.
|
||||
|
||||
<a id="release-stamp-empty"></a>
|
||||
### `RELEASE_STAMP_EMPTY`
|
||||
|
||||
@@ -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<string, unknown> | 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 });
|
||||
}
|
||||
}
|
||||
@@ -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 <n> --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)',
|
||||
|
||||
+7
-2
@@ -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<PolicyKey, string> = {
|
||||
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 <n> --pin <sha>` (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<PolicyKey, string> = {
|
||||
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<PolicyKey, string> = {
|
||||
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;
|
||||
|
||||
@@ -1,24 +1,31 @@
|
||||
/**
|
||||
* ship-queue-land-direct — B1's merge step, behind `gstack-ship-queue land --pr <n> --pin <sha>` 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 <pin>, 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 <pin>, 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 <workflow> --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`] };
|
||||
}
|
||||
@@ -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<string, unknown> = 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<string, unknown> = 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<string, unknown>) => 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();
|
||||
|
||||
Reference in new issue
Block a user