From e09ea409f0f74b1e3f5373f3155643c49b6eae53 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 3 Nov 2026 17:32:57 -0800 Subject: [PATCH] fix(ship): a second full preflight stops while this checkout's /ship run is open (GST-1204) (#3950) * fix(ship-preflight): forward --timeout to Step 5's pregate (GST-1196) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship): a second full preflight stops while this checkout's /ship run is open (GST-1204) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship): a new /ship from the same harness replaces a stopped run's marker; a run the lease can't record stops (GST-1204 review) 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-lease | 18 ++- changelog.d/gst-1204-preflight-lease.md | 12 ++ docs/capy-install.md | 6 +- docs/troubleshooting.md | 43 ++++++ lib/ship-bin-common.ts | 5 + lib/ship-preflight.ts | 40 ++++- lib/ship-release.ts | 10 +- ship/DRIVER.md | 2 +- ship/DRIVER.md.tmpl | 2 +- ship/SKILL.md | 4 +- ship/SKILL.md.tmpl | 4 +- test/fixtures/golden/agy-ship-SKILL.md | 4 +- test/fixtures/golden/claude-ship-SKILL.md | 4 +- test/fixtures/golden/codex-ship-SKILL.md | 4 +- test/fixtures/golden/factory-ship-SKILL.md | 4 +- test/gstack-lease.test.ts | 32 ++++ test/ship-preflight.test.ts | 146 +++++++++++++++++- .../ship/gst-1204-preflight-lease.json | 4 + 18 files changed, 321 insertions(+), 23 deletions(-) create mode 100644 changelog.d/gst-1204-preflight-lease.md create mode 100644 test/size-ledger/ship/gst-1204-preflight-lease.json diff --git a/bin/gstack-lease b/bin/gstack-lease index 8fe9a0139..25b508700 100755 --- a/bin/gstack-lease +++ b/bin/gstack-lease @@ -30,6 +30,10 @@ # gstack-lease release --pid

--run [--root ] # close run on 's lease (gstack-skill-end); once every run it # recorded is closed the lease is idle and live skips it (GST-1205) +# gstack-lease open --run [--pid

| --from ] [--root ] +# exit 0 while run on a lease ('s, the harness's behind +# --from, or any) is open (not released, under 120 minutes) and its +# holder alive; 1 otherwise # gstack-lease list [--root ] # print ` m alive|idle|dead|stale` for every lease # A pid alone is not a holder: pids are reused after a reboot or a machine @@ -67,8 +71,8 @@ while [ $# -gt 0 ]; do shift done case "$VERB" in - touch|live|list|release) ;; - -h|--help|'') sed -n '2,34p' "$0" | sed 's/^# \{0,1\}//'; exit 0 ;; + touch|live|list|release|open) ;; + -h|--help|'') sed -n '2,38p' "$0" | sed 's/^# \{0,1\}//'; exit 0 ;; *) echo "gstack-lease: unknown verb '$VERB'" >&2; exit 2 ;; esac [ -n "$ROOT" ] || ROOT=$(gstack_source_root "$BIN") @@ -318,6 +322,16 @@ case "$VERB" in fi unlock exit 0 ;; + open) + [ -z "$FROM" ] || PID=$(gstack_harness_pid "$FROM") + case "$PID" in *[!0-9]*) echo "gstack-lease: open needs --run and an optional --pid " >&2; exit 2 ;; esac + [ -n "$RUN" ] || { echo "gstack-lease: open needs --run and an optional --pid " >&2; exit 2; } + for f in "$DIR"/${PID:-*}; do + [ -f "$f" ] || continue + [ "$(state "${f##*/}" "$(head -1 "$f" 2>/dev/null)" "$(age_min "$f")")" = alive ] || continue + fresh "$f" | awk -v id="$RUN" '$1 == "run" && $2 == id { found = 1 } END { exit !found }' && exit 0 + done + exit 1 ;; list) for f in "$DIR"/*; do [ -f "$f" ] || continue diff --git a/changelog.d/gst-1204-preflight-lease.md b/changelog.d/gst-1204-preflight-lease.md new file mode 100644 index 000000000..b172225b7 --- /dev/null +++ b/changelog.d/gst-1204-preflight-lease.md @@ -0,0 +1,12 @@ +**A full `gstack-ship-preflight` no longer merges the base under an open /ship run's reviews (GST-1204).** Running the full preflight again in the middle of /ship merged the base into the working tree (Step 3) while review readers were diffing that tree, which voided a review pass (gbrain-evals #317, a Capy lane). + +### Fixed +- **`gstack-ship-preflight`:** `--run ` records the /ship run once Steps 1-3 pass: a marker in the checkout's git dir, and an open run on the checkout's own lease, so the Capy driver needs no workflow lease. A later full preflight on that checkout stops with exit 3, before any fetch or merge, until `gstack-ship-release finish` clears the marker or the run is 120 minutes old (GST-1205's rule) or its holder is gone. The STOP names the run and the marker and says what to do: rerun Step 5 with `--tests-only`, let the /ship finish, or remove the marker of a run that stopped. `--tests-only` and `--audit` are never stopped. /ship's own first preflight, and a rerun after a Step 3 STOP, still work. A new `--run` id from the open run's own holder (the Claude Code session, or Capy's `capy-server`) replaces a run that stopped before finish; the same id, no `--run`, or another live session's run still stops. A `--run` the lease can't record (its lock held by a live process) stops before the fetch instead of going on unprotected. A `--run` value that isn't a plain id is a usage error (2). +- **`gstack-ship-release finish`:** clears the run's marker once Steps 15-16 pass. +- **`gstack-lease open --run [--pid

| --from ]`:** exits 0 while that run is open on a live holder and 1 otherwise, using the same released, 120-minute and dead or reused-pid rules as `live`. +- **/ship Step 1** passes `--run `, and **`ship/DRIVER.md` Phase 2** passes `--run `. +- **Docs:** `docs/troubleshooting.md#ship-preflight-run-open` explains the new STOP; `docs/capy-install.md` lists `gstack-lease open`. + +### Tests +- `test/ship-preflight.test.ts`: a full rerun with the run open stops before the fetch and leaves HEAD, `origin/main` and the tree untouched, with or without `--run`; the driver's Phase 2 sequence refuses a mid-run full preflight while `--audit` and `--tests-only` go on; a finished or aged-out run doesn't block; after a stopped run and `gstack-skill-end`'s real release, a new id from the same holder goes on while another live holder's run is still refused; an unrecordable `--run` stops before the fetch; /ship's first preflight and its rerun after a conflict STOP work; a bad `--run` is usage. +- `test/gstack-lease.test.ts`: `open` for an open, released, aged-out, reused-pid, dead and unrecorded run, with `--pid`, `--from` or neither, plus usage errors. diff --git a/docs/capy-install.md b/docs/capy-install.md index 8e4075529..7675b3c9d 100644 --- a/docs/capy-install.md +++ b/docs/capy-install.md @@ -232,7 +232,11 @@ rename can be broken and then publish into its successor's lock. While a live pr holds the lock, or it cannot be taken at all, `touch` exits 3 without writing (`gstack-skill-start` reports that the session holds no lease) and a reclaim keeps the lease. `bin/gstack-lease list` shows `alive|idle|dead|stale`, and -`GSTACK_UPGRADE_FORCE=1` overrides. +`GSTACK_UPGRADE_FORCE=1` overrides. `bin/gstack-lease open --run [--pid

| --from ]` +answers for one run (exit 0 while it is open on a live holder); +`gstack-ship-preflight` opens /ship runs on the project checkout's own lease and +uses it to refuse a base merge under an open run +([troubleshooting](troubleshooting.md#ship-preflight-run-open)). ## What the installer does diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index a0cb4b184..46c612b8e 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -631,6 +631,49 @@ fix-check record as Step 9's. Rerun Step 11's native pass on the same frozen tre **Expected result.** `gstack-review-read` shows both records at one tree with `review_freshness` `CURRENT`, and Step 11.5 continues to Step 12. + +### `STOP: /ship run is open on this checkout and past its preflight (; open until its finish or for 120 minutes) ...` + +**Meaning.** A full `gstack-ship-preflight` (Steps 1-3) ran on a checkout where a +/ship run already passed them and has not finished. A full preflight merges the +base into the working tree, and that run's review readers diff this tree, so the +merge would void their pass (gbrain-evals#317). /ship passes `--run ` (Step 1: +the preamble's `SESSION_ID`; `ship/DRIVER.md` Phase 2: the run dir's name). Once +Steps 1-3 succeed, preflight records the run in the checkout's git dir +(`gstack-ship-run`) and opens it on the checkout's own lease +(`/gstack-leases`, so the driver needs no workflow lease). The run +counts as open while the marker exists, which `gstack-ship-release finish` removes +when Steps 15-16 pass, and while `bin/gstack-lease open` says so: under 120 +minutes old and its holder (the harness that ran preflight) alive. A finished, +aged-out or dead run never blocks, and /ship's own first preflight (or a rerun +after a Step 3 STOP) is never refused. A different `--run` from the open run's +holder replaces it: that is a new /ship after one that stopped before finish. +The holder is the first non-shell process above preflight: the Claude Code +session, or on Capy the machine's one `capy-server`, so there only the same id or +a missing `--run` is refused. The same id, no `--run`, or an open run held by +another live process (another session) stops. + +**What is kept.** Nothing ran: no fetch, no merge, the tree is untouched. + +**Fix.** Rerun Step 5 with `gstack-ship-preflight --tests-only`, which never +merges, or let that /ship finish. If that run is no longer running (it stopped +before finish), remove the marker the STOP names (`rm `), then rerun. + +**Expected result.** The preflight continues to `STATUS:` and `MERGE:`. + + +### `STOP: cannot record /ship run on this checkout's lease (...)` + +**Meaning.** `--run` was given, but `gstack-lease touch` could not open the run +on the checkout's lease, for example while a live process holds its lock. +Unrecorded, a later full preflight could not be refused, so preflight stops +before the fetch instead of going on unprotected. + +**What is kept.** Nothing ran: no fetch, no merge. + +**Fix.** Do what the parenthesized `gstack-lease` line names (wait for the +process holding `/gstack-leases/.lock`), then rerun. + ### `gstack-wtree: cannot create a temp index under

; set TMPDIR to a writable directory` / `gstack-review-log: cannot create a private object directory under ; no working-tree fingerprint. Set TMPDIR to a writable directory.` diff --git a/lib/ship-bin-common.ts b/lib/ship-bin-common.ts index c76e51887..6cf1b3457 100644 --- a/lib/ship-bin-common.ts +++ b/lib/ship-bin-common.ts @@ -25,6 +25,11 @@ const done = (r: { status: number | null; stdout?: string | Buffer | null; stder export function git(cwd: string, args: string[], timeout = 120_000): Run { return done(spawnSync('git', args, { cwd, encoding: 'utf8', timeout, maxBuffer: 64 * 1024 * 1024, env: { ...process.env, GIT_TERMINAL_PROMPT: '0' } })); } +/** GST-1204: the /ship run that passed Steps 1-3 on this checkout, recorded in its own git dir (per worktree) until finish clears it; empty outside a checkout. */ +export function shipRunMarker(cwd: string): string { + const p = git(cwd, ['rev-parse', '--git-path', 'gstack-ship-run']); + return p.status === 0 && p.stdout.trim() ? path.resolve(cwd, p.stdout.trim()) : ''; +} export function bin(cwd: string, name: string, args: string[], timeout = 600_000): Run { return done(runScriptSync(path.join(BIN_DIR, name), args, { cwd, encoding: 'utf8', timeout, maxBuffer: 64 * 1024 * 1024 })); } diff --git a/lib/ship-preflight.ts b/lib/ship-preflight.ts index 203df22c9..fb7a498b8 100644 --- a/lib/ship-preflight.ts +++ b/lib/ship-preflight.ts @@ -15,11 +15,14 @@ import { buildRunCheck } from './pregate/run-check'; import { classifyDiff } from './review-depth'; import { wtreeFingerprint } from './script-invocation'; import { audit } from './ship-audit'; -import { bin, carryNoneLine, firstLine, flagReader, gh, git, hostedOrigin, laneConcurrency, laneLine, lanes, Report, runLanes, shipPolicy, short } from './ship-bin-common'; +import { bin, carryNoneLine, firstLine, flagReader, gh, git, hostedOrigin, laneConcurrency, laneLine, lanes, Report, runLanes, shipPolicy, shipRunMarker, short } from './ship-bin-common'; export const PREFLIGHT_HELP = `gstack-ship-preflight — /ship Steps 1-3 and Step 5's pregate and lanes in one call (ship/SKILL.md) gstack-ship-preflight --base [--lane