fix(ship-queue): a lock push GitHub applied but answered 502 no longer leaks the land lock (GST-1344) (#3994)

* fix(ship-queue): a lock push GitHub applied but answered 502 no longer leaks the land lock (GST-1344)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: pre-landing review fixes

Review-Finding: p1/specialist-batched#1 reader=testing
Review-Finding: p1/specialist-batched#2 reader=testing
Review-Finding: p1/red-team#1 reader=red-team
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: pre-landing review fixes

Review-Finding: p2/outside-voice#1 reader=outside-voice
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* docs: troubleshooting covers a land lock push GitHub applied but answered 502 (GST-1344)

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>

changelog.d/garrytan-gst-1344-land-lock-502.md

@@ -0,0 +1 @@
+- **`land --pin` no longer leaves the land lock stuck in `merging` when GitHub applies a lock push and still answers HTTP 502.** On 2026-11-04, during GitHub 502s, the first land of #3990 reported "the land lock push to origin failed and the lock there is unchanged" while origin's `refs/notes/gstack-land` already held #3990 in `merging`. Its release then failed against the old lease, and two reruns refused the same way until `--break-lock`. Now a lock push that reports failure asks origin (`git ls-remote`) what the ref holds, and treats a push origin applied as written. When a lock fetch fails, the read takes origin's sha from `ls-remote` if this clone already has that commit, so a rerun resumes its own lock instead of leasing a stale sha. When a phase change or release is rejected because origin holds a newer lock from this same run, land adopts that sha and retries once, so a refusal still releases the lock it wrote. A push that origin can't confirm either way now says so instead of claiming the lock is unchanged.

docs/troubleshooting.md

@@ -4991,13 +4991,16 @@ uses it while a slot cannot merge yet: checks on the stamp commit are not
 green, a name in `changelog_fragments.required_checks` is missing or not green
 there, the PR's base is no longer the train's base, the stamp push failed while
 the branch still read the pin, `gh pr merge` refused, or the merge is not yet
-visible on main.
+visible on main. `land` also uses it when a land lock push to origin failed and
+origin's lock ref could not be read to tell whether the push landed.
 
 **Fix.** Commit and push, wait for CI on that head, rerun
 `gstack-pregate run --allow-repo-commands` when the tree changed, then rerun
 `gstack-ship-queue ready --pr <n>`. The block it prints before refusing shows
 which line is not yet true. For `train land`, do what `NEXT:` says, then rerun
-the same `train land` command; it adopts its own journaled stamp commit.
+the same `train land` command; it adopts its own journaled stamp commit. For an
+unreadable land lock, rerun the same `land` command once origin answers; it
+resumes its own lock.
 
 <a id="queue-proof-missing"></a>
 ### `QUEUE_PROOF_MISSING`
@@ -5140,6 +5143,10 @@ released by the verify that clears the freeze
 next `land` or `train land` takes the lock over once the freeze record it fetches
 shows the freeze cleared after the lock was written; `--break-lock` never does.
 
+A lock push that GitHub applied but answered with an error (an HTTP 502) is
+checked against origin's ref and counts as written, so a rerun resumes this
+run's own lock without `--break-lock`.
+
 <a id="queue-train-unstamped"></a>
 ### `QUEUE_TRAIN_UNSTAMPED`
 

lib/ship-queue-land-lock.ts

@@ -2,7 +2,9 @@
  * The repository land lock (docs/ship-policy.md#land, #3599): one ref, refs/notes/gstack-land, whose commit
  * holds lock.json, shared by `gstack-ship-queue land` (owner `{ pr }`) and `train land` (owner
  * `{ train, slot, pr }`), so a standalone land and a train slot never both publish onto one main. Every write
- * is an exact lease on the sha read, so two writers racing one sha leave one winner. A lock is resumed only by
+ * is an exact lease on the sha read, so two writers racing one sha leave one winner. A push that reports failure is
+ * checked against origin's ref (GitHub can apply a push and still answer HTTP 502, GST-1344), and a read whose fetch
+ * fails takes origin's sha from ls-remote when this clone already holds that commit. A lock is resumed only by
  * the same owner kind, identity and journal; anything else is a takeover the caller allows (a drift repair, a
  * train's next slot, the admitted repair train) or a --break-lock under the staleness, mid-push, drift, freeze
  * and live-train rules.
@@ -24,11 +26,19 @@ export const lockLabel = (lock: LandLock): string => (lock.train ? `train ${lock
 export const ownsLock = (lock: LandLock, owner: LockOwner, journal: string): boolean =>
   lock.pr === owner.pr && (lock.train ?? null) === (owner.train ?? null) && (lock.slot ?? null) === (owner.slot ?? null) && lock.journal === journal;
 
+/** The sha origin's lock ref holds: '' when origin has none, null when origin cannot be read. */
+function originSha(root: string, remote: string): string | null {
+  const listed = git(root, ['ls-remote', remote, LAND_LOCK_REF]);
+  if (!listed.ok) return null;
+  return listed.out.split('\n').map(l => l.split('\t')).find(([, ref]) => ref === LAND_LOCK_REF)?.[0] ?? '';
+}
+
 export function readLock(root: string, remote = 'origin'): LockRead {
   const fetched = git(root, ['fetch', '-q', remote, `+${LAND_LOCK_REF}:${LAND_LOCK_REF}`]);
   if (!fetched.ok) {
-    const listed = git(root, ['ls-remote', remote, LAND_LOCK_REF]);
-    if (listed.ok && !listed.out.trim()) git(root, ['update-ref', '-d', LAND_LOCK_REF]);
+    const at = originSha(root, remote);
+    if (at === '') git(root, ['update-ref', '-d', LAND_LOCK_REF]);
+    else if (at && git(root, ['cat-file', '-e', `${at}^{commit}`]).ok) git(root, ['update-ref', LAND_LOCK_REF, at]);
   }
   const sha = git(root, ['rev-parse', '--verify', '--quiet', LAND_LOCK_REF]).out;
   if (!sha) return { state: 'absent', sha: '' };
@@ -41,15 +51,18 @@ export function readLock(root: string, remote = 'origin'): LockRead {
   return { state: 'held', sha, lock: value as LandLock };
 }
 
-export function writeLock(root: string, expected: string, value: LandLock | Released, remote = 'origin'): { ok: boolean; sha: string } {
+/** Push the lock under the lease. On a failed push, `origin` is the sha origin's ref now holds ('' none, null unreadable); a push origin applied anyway is a success. */
+export function writeLock(root: string, expected: string, value: LandLock | Released, remote = 'origin'): { ok: boolean; sha: string; origin?: string | null } {
   const blob = git(root, ['hash-object', '-w', '--stdin'], undefined, JSON.stringify(value, null, 2) + '\n').out;
   const tree = git(root, ['mktree'], undefined, `100644 blob ${blob}\tlock.json\n`).out;
   const message = 'released' in value ? `land lock released by ${value.by}` : `land lock ${value.train ? `train ${value.train} slot ${value.slot} ` : ''}#${value.pr} ${value.phase}`;
   const commit = git(root, ['commit-tree', tree, ...(expected ? ['-p', expected] : []), '-m', message]).out;
-  if (![blob, tree, commit].every(isSha)) return { ok: false, sha: '' };
+  if (![blob, tree, commit].every(isSha)) return { ok: false, sha: '', origin: expected };
   const push = git(root, ['push', '-q', `--force-with-lease=${LAND_LOCK_REF}:${expected}`, remote, `${commit}:${LAND_LOCK_REF}`]);
-  if (push.ok) git(root, ['update-ref', LAND_LOCK_REF, commit]);
-  return { ok: push.ok, sha: commit };
+  const origin = push.ok ? commit : originSha(root, remote);
+  if (origin !== commit) return { ok: false, sha: commit, origin };
+  git(root, ['update-ref', LAND_LOCK_REF, commit]);
+  return { ok: true, sha: commit };
 }
 
 export interface HeldLandLock {
@@ -114,11 +127,20 @@ export function acquireLandLock(o: AcquireOptions): { held: HeldLandLock; lines:
   }
   const startedAt = own?.started_at ?? o.now().toISOString();
   const always = o.extra?.(own) ?? {};
+  const write = (value: LandLock | Released) => {
+    const w = writeLock(o.root, sha, value, remote);
+    if (w.ok || w.origin === sha) return w;
+    const seen = readLock(o.root, remote);
+    if (seen.state !== 'held' || seen.sha === sha || !ownsLock(seen.lock, o.owner, o.journal) || seen.lock.holder !== o.holder) return w;
+    sha = seen.sha;
+    return writeLock(o.root, sha, value, remote);
+  };
   const set = (phase: LandLockPhase, extra: Partial<LandLock> = {}): QueueFailure | null => {
     const next: LandLock = { pr: o.owner.pr, ...(o.owner.train ? { train: o.owner.train, slot: o.owner.slot } : {}), holder: o.holder, started_at: startedAt, phase, journal: o.journal, phase_at: o.now().toISOString(), ...always, ...extra };
-    const w = writeLock(o.root, sha, next, remote);
+    const w = write(next);
     if (!w.ok) {
-      return readLock(o.root, remote).sha === sha
+      if (w.origin === null) return { code: 'QUEUE_NOT_READY', message: `the land lock push to ${remote} failed and ${remote}'s lock could not be read, so this run cannot tell whether the push landed`, fix: 'rerun the same command once origin answers; it resumes this run\'s own lock' };
+      return w.origin === sha
         ? { code: 'QUEUE_NOT_READY', message: `the land lock push to ${remote} failed and the lock there is unchanged; nothing was pushed` }
         : { code: 'QUEUE_LAND_LOCKED', message: 'the land lock moved under this run (another lander took it)' };
     }
@@ -131,7 +153,7 @@ export function acquireLandLock(o: AcquireOptions): { held: HeldLandLock; lines:
     get lock() { return current!; },
     resumed: own,
     set,
-    release: () => writeLock(o.root, sha, { released: true, by: o.holder, at: o.now().toISOString() }, remote).ok,
+    release: () => write({ released: true, by: o.holder, at: o.now().toISOString() }).ok,
   };
   return { held, lines };
 }

test/helpers/flaky-git.ts

@@ -0,0 +1,60 @@
+/**
+ * A git shim that fails the land lock's network calls the way GitHub did during the 2026-11-04 HTTP 502s
+ * (GST-1344): a lock push that updates origin's ref and then exits non-zero, and lock fetches or ls-remotes that
+ * fail outright. Only commands naming refs/notes/gstack-land are touched; everything else runs the real git.
+ * Arm it with the returned setters; put `dir` first on PATH (or pass `env` to a child).
+ */
+import { execFileSync } from 'node:child_process';
+import * as fs from 'node:fs';
+import * as path from 'node:path';
+
+export interface FlakyGit {
+  dir: string;
+  env: { PATH: string; FLAKY_GIT: string };
+  /** The lock pushes (1-based, counted from now) that reach origin and then report an HTTP 502. */
+  pushLandsThenFails(...ordinals: number[]): void;
+  /** The next n lock pushes fail before reaching origin. */
+  pushFails(n: number): void;
+  /** Lock fetches fail while on. */
+  fetchFails(on: boolean): void;
+  /** The next n lock ls-remotes fail. */
+  lsRemoteFails(n: number): void;
+  reset(): void;
+}
+
+export function flakyGit(root: string): FlakyGit {
+  const dir = path.join(root, 'flaky-git');
+  const state = path.join(dir, 'state');
+  fs.mkdirSync(state, { recursive: true });
+  const real = execFileSync('bash', ['-c', 'command -v git'], { encoding: 'utf8', timeout: 10_000 }).trim();
+  fs.writeFileSync(path.join(dir, 'git'), [
+    '#!/bin/bash',
+    `REAL='${real}'`,
+    `D=':${dir}:'`,
+    'P=":$PATH:"; P="${P//"$D"/:}"; P="${P#:}"; export PATH="${P%:}"',
+    'S="$FLAKY_GIT"',
+    'case " $* " in *refs/notes/gstack-land*) ;; *) exec "$REAL" "$@" ;; esac',
+    'bad() { echo "error: RPC failed; HTTP 502 curl 22 The requested URL returned error: 502" >&2; exit 128; }',
+    'take() { local n; n=$(cat "$S/$1" 2>/dev/null || echo 0); [ "$n" -gt 0 ] || return 1; echo $((n-1)) > "$S/$1"; }',
+    'case "$1" in',
+    '  push)',
+    '    n=$(( $(cat "$S/pushes" 2>/dev/null || echo 0) + 1 )); echo $n > "$S/pushes"',
+    '    take push-fails && bad',
+    '    if grep -qx "$n" "$S/lands-then-fails" 2>/dev/null; then "$REAL" "$@" || exit $?; bad; fi ;;',
+    '  fetch) [ -f "$S/fetch-fails" ] && bad ;;',
+    '  ls-remote) take ls-remote-fails && bad ;;',
+    'esac',
+    'exec "$REAL" "$@"',
+    '',
+  ].join('\n'), { mode: 0o755 });
+  const put = (name: string, value: string | null) => { const f = path.join(state, name); if (value === null) fs.rmSync(f, { force: true }); else fs.writeFileSync(f, value); };
+  return {
+    dir,
+    env: { PATH: `${dir}:${process.env.PATH ?? ''}`, FLAKY_GIT: state },
+    pushLandsThenFails: (...ordinals) => { put('pushes', '0'); put('lands-then-fails', ordinals.join('\n') + '\n'); },
+    pushFails: n => put('push-fails', String(n)),
+    fetchFails: on => put('fetch-fails', on ? '1' : null),
+    lsRemoteFails: n => put('ls-remote-fails', String(n)),
+    reset: () => { for (const f of fs.readdirSync(state)) fs.rmSync(path.join(state, f), { force: true }); },
+  };
+}

test/ship-queue-land-direct.test.ts

@@ -20,6 +20,7 @@ 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';
+import { flakyGit } from './helpers/flaky-git';
 import { runBackstop } from '../scripts/train-backstop';
 import { readTrainNotes } from '../scripts/release-cut';
 import { gitIn } from '../scripts/train-gate';
@@ -290,6 +291,66 @@ describe('land --pin: direct merge at the PR\'s own green head', () => {
     expect(dry.out).toContain('DRY-RUN: nothing verified');
   }, 120_000);
 
+  describe('GST-1344: a lock push GitHub applied but answered with an HTTP 502', () => {
+    const lockAt = (fx: Fx) => JSON.parse(fx.repo.git('--git-dir', fx.repo.origin, 'show', 'refs/notes/gstack-land:lock.json'));
+    const flaky = (fx: Fx) => { const g = flakyGit(fx.repo.root); return { g, run: (pr: string, pin: string, extra: string[] = []) => runBin(fx.repo, 'gstack-ship-queue', ['land', '--pr', pr, '--pin', pin, '--base', 'main', ...extra], { ...fx.env, ...g.env, PATH: `${g.dir}:${fx.env.PATH}` }) }; };
+
+    test('the merging push lands but reports failure while lock fetches fail: land reads origin, sees its own commit, merges and releases', () => {
+      const fx = setup();
+      const { g, run } = flaky(fx);
+      const pin = openPr(fx, '95');
+      g.pushLandsThenFails(2);
+      g.fetchFails(true);
+      const r = run('95', pin);
+      expect(r.err).toBe('');
+      expect(r.status).toBe(EXIT.ok);
+      expect(r.out).toContain('MERGED: #95 as ');
+      expect(fx.read().lockAtMerge).toMatchObject({ pr: '95', phase: 'merging', journal: 'land-pin:' + pin });
+      expect(lockAt(fx)).toMatchObject({ released: true });
+    }, 120_000);
+
+    test('origin cannot be read after the push either: the run refuses, and the rerun takes its lease from origin (ls-remote) while fetches still fail, then merges and releases', () => {
+      const fx = setup();
+      const { g, run } = flaky(fx);
+      const pin = openPr(fx, '96');
+      g.pushLandsThenFails(2);
+      g.fetchFails(true);
+      g.lsRemoteFails(99);
+      const first = run('96', pin);
+      expect(first.status).toBe(EXIT.refused);
+      expect(first.err).toContain('(QUEUE_NOT_READY)');
+      expect(first.err).not.toContain('the lock there is unchanged');
+      expect(fx.read().prs['96'].state).toBe('OPEN');
+      expect(lockAt(fx)).toMatchObject({ pr: '96', phase: 'merging', journal: 'land-pin:' + pin });
+      g.reset();
+      g.fetchFails(true);
+      const again = run('96', pin);
+      expect(again.err).toBe('');
+      expect(again.status).toBe(EXIT.ok);
+      expect(again.out).toContain('MERGED: #96 as ');
+      expect(lockAt(fx)).toMatchObject({ released: true });
+    }, 120_000);
+
+    test('never left in merging: the merging push lands and origin stays unreadable through the lock read, the post-push check and the re-read (three ls-remotes), so the run refuses, yet its release re-reads origin, finds its own lock and releases it; a rerun lands', () => {
+      const fx = setup();
+      const { g, run } = flaky(fx);
+      const pin = openPr(fx, '97');
+      g.pushLandsThenFails(2);
+      g.lsRemoteFails(3);
+      g.fetchFails(true);
+      const first = run('97', pin);
+      expect(first.status).toBe(EXIT.refused);
+      expect(first.err).toContain('(QUEUE_NOT_READY)');
+      expect(first.out).not.toContain('LOCK: warn');
+      expect(fx.read().prs['97'].state).toBe('OPEN');
+      expect(lockAt(fx)).toMatchObject({ released: true });
+      g.reset();
+      const again = run('97', pin);
+      expect(again.err).toBe('');
+      expect(again.out).toContain('MERGED: #97 as ');
+    }, 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');

test/ship-queue-land-lock.test.ts

@@ -12,6 +12,7 @@ import * as path from 'node:path';
 import { EXIT } from '../lib/headless-artifacts';
 import { acquireLandLock, LAND_LOCK_REF, readLock, writeLock, type LandLock, type LockOwner } from '../lib/ship-queue-land-lock';
 import { BASE_POLICY, cleanup, makeFixtureRepo, type FixtureRepo } from './helpers/restamp-fixture';
+import { flakyGit } from './helpers/flaky-git';
 import { coordinatorClone } from './helpers/ship-queue-fixture';
 import {
   GREEN_GATE, TRAIN_POLICY, fragmentPr, nonceOf, queue, repoKey, trainFixture, trainJson, wrapLand, type TrainFixture,
@@ -117,6 +118,59 @@ describe('the lock module', () => {
     });
   }, 60_000);
 
+  test('GST-1344: a push origin applied but answered with an HTTP 502 counts as written; a push that never landed reports the lock unchanged; an unreadable origin says so instead', () => {
+    const repo = bareRepo();
+    const g = flakyGit(repo.root);
+    withRepoEnv({ ...repo, env: { ...repo.env, ...g.env } }, () => {
+      const root = repo.work;
+      const lock: LandLock = { pr: '11', holder: 'h11', started_at: new Date(T0).toISOString(), phase: 'stamping', journal: 'land/11' };
+      g.pushLandsThenFails(1);
+      const w = writeLock(root, '', lock);
+      expect(w.ok).toBe(true);
+      expect(repo.git('rev-parse', LAND_LOCK_REF)).toBe(w.sha);
+      expect(repo.git('--git-dir', repo.origin, 'rev-parse', LAND_LOCK_REF)).toBe(w.sha);
+
+      g.reset();
+      const got = take(root, standalone('11'), { journal: 'land/11' });
+      expect(failureOf(got)).toBeNull();
+      if (!('held' in got)) return;
+      g.pushFails(1);
+      expect(got.held.set('waiting')).toMatchObject({ code: 'QUEUE_NOT_READY', message: expect.stringContaining('the lock there is unchanged') });
+      g.pushFails(1);
+      g.lsRemoteFails(2);
+      g.fetchFails(true);
+      expect(got.held.set('waiting')).toMatchObject({ code: 'QUEUE_NOT_READY', message: expect.stringContaining('could not be read') });
+      g.reset();
+      expect(got.held.release()).toBe(true);
+      expect(readLock(root)).toMatchObject({ state: 'released' });
+    });
+  }, 60_000);
+
+  test('GST-1344: a phase change rejected because origin holds a newer lock from this same run adopts that sha and retries once; the same journal under another holder is not adopted', () => {
+    const repo = bareRepo();
+    withRepoEnv(repo, () => {
+      const root = repo.work;
+      const got = take(root, standalone('12'), { journal: 'land/12' });
+      if (!('held' in got)) throw new Error('lock not taken');
+      const ahead = (holder: string) => {
+        const r = readLock(root);
+        const w = writeLock(root, r.sha, { ...got.held.lock, holder, phase: 'waiting' });
+        expect(w.ok).toBe(true);
+        repo.git('update-ref', LAND_LOCK_REF, r.sha);
+        return w.sha;
+      };
+      const mine = ahead('test:1');
+      expect(got.held.set('pushing')).toBeNull();
+      const after = readLock(root);
+      expect(after).toMatchObject({ state: 'held', lock: { phase: 'pushing' } });
+      expect(repo.git('rev-parse', `${after.sha}^`)).toBe(mine);
+
+      const theirs = ahead('other:2');
+      expect(got.held.set('merging')).toMatchObject({ code: 'QUEUE_LAND_LOCKED' });
+      expect(repo.git('--git-dir', repo.origin, 'rev-parse', LAND_LOCK_REF)).toBe(theirs);
+    });
+  }, 60_000);
+
   test('--break-lock prints the holder, refuses a live train run\'s lock without --break-train-lock, and keeps the staleness, mid-push, between and freeze rules', () => {
     const repo = bareRepo();
     withRepoEnv(repo, () => {