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>

changelog.d/gst-1316-land-stale-base.md

@@ -1,8 +1,8 @@
 **`gstack-ship-queue land --pin` no longer lands a PR whose base moved after its CI (GST-1316).** Under `merge: direct`, land checked the PR's own green checks, but those ran on the PR before newer commits reached main. #3886 was green on its own, landed onto a newer main, and pushed `ship/DRIVER.md` to 12,329 bytes against its 12,288-byte cap, so Free Tests on main went red.
 
 ### Fixed
-- **`gstack-ship-queue land --pin`:** refuses with the new `QUEUE_LAND_STALE_BASE` when the pin's merge base with `origin/main` is not main's tip, so the checks at the pin always ran on the exact tree that lands. The fix it prints: merge main into the branch, wait for the required checks at the new head, run /ship and `ready` there, then land the new pin. Nothing merges on a refusal; `--dry-run` reports it too.
+- **`gstack-ship-queue land --pin`:** refuses with the new `QUEUE_LAND_STALE_BASE` when the pin's merge base with `origin/main` is not main's tip, so the checks at the pin ran on the exact tree that lands. The fix it prints: merge main into the branch, wait for the required checks at the new head, run /ship and `ready` there, then land the new pin. Nothing merges on a refusal; `--dry-run` reports it too. After the merge, land also checks that the pin contains the commit GitHub merged it onto; if main moved between the check and the merge, it reports `QUEUE_LAND_UNVERIFIED` and writes no land note, on a rerun too.
 - **Docs:** `docs/ship-policy.md#land-direct` lists the new check and drops "no base merge and no extra CI round". The release section now says a release may not delete a release output, and that a rerun after GitHub merged a release fetches the pin, then reads its parent, with an unreadable parent refused as `QUEUE_NOT_READY`. `docs/troubleshooting.md#queue-land-stale-base` and the `docs/captain.md` refusal table cover the new code.
 
 ### Tests
-- `test/ship-queue-land-direct.test.ts`: a PR whose base moved after its pin is refused with `QUEUE_LAND_STALE_BASE` (also under `--dry-run`) and main stays put; once the branch merges main and is shipped and READY at the new head, it lands and main's tree equals that head's tree. This replaces the test that expected the stale-base PR to land as it was.
+- `test/ship-queue-land-direct.test.ts`: a PR whose base moved after its pin is refused with `QUEUE_LAND_STALE_BASE` (also under `--dry-run`) and main stays put; once the branch merges main and is shipped and READY at the new head, it lands and main's tree equals that head's tree. This replaces the test that expected the stale-base PR to land as it was. A second test moves main inside the fake `gh pr merge` and expects `QUEUE_LAND_UNVERIFIED` with no land note, on the first run and the rerun.

docs/ship-policy.md

@@ -2033,8 +2033,9 @@ still break main (#3886 pushed ship/DRIVER.md past its size cap that way), so
 the lane merges main and CI runs again. Then it runs `gh pr merge
 --squash --match-head-commit <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
+merge commit's parent and that the pin already contains that parent, so main
+didn't move between the check and the merge
+([`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
 names this command.

docs/troubleshooting.md

@@ -5265,10 +5265,15 @@ 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 isn't in the pin, so
+main moved between land's stale-base check and the merge and the pin's checks
+never ran on the tree that landed. 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`

lib/ship-queue-land-direct.ts

@@ -59,6 +59,7 @@ 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 && !git(root, ['merge-base', '--is-ancestor', parent, pin]).ok) return { ok: false, run, lines, failure: { code: 'QUEUE_LAND_UNVERIFIED', message: `#${pr} merged as ${merge.slice(0, 12)} onto ${parent.slice(0, 12)}, which the pin ${pin.slice(0, 12)} does not contain: main moved after the stale-base check, so the checks at the pin never ran on the tree that landed`, 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 (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]);

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

@@ -48,7 +48,8 @@ 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];",
+    "  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)}",
@@ -160,6 +161,21 @@ describe('land --pin: direct merge at the PR\'s own green head', () => {
     expect(show(fx, main + ':pr-8.txt')).toBe('8');
   }, 90_000);
 
+  test('main moved between the stale-base check and GitHub\'s merge: land reports QUEUE_LAND_UNVERIFIED and writes no land note, and so does a rerun', () => {
+    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.err).toContain('(QUEUE_LAND_UNVERIFIED)');
+    expect(landNotes(fx)).toEqual([]);
+  }, 90_000);
+
   test('a rerun after GitHub merged the PR goes straight to the tree check and lands', () => {
     const fx = setup();
     const pin = openPr(fx, '9');