Stop the merge queue from landing a stacked PR's unreviewed parent commits (GST-2153) (#4229)

* Add failing tests: ready and land accept a stacked child's receipt reviewed against its open parent branch Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Refuse a receipt reviewed against another branch than the queue base at ready, land and train build (GST-2153) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Regenerate the doctor result anchors for the new receipt base mismatch code Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes Train build now has a test that refuses a receipt reviewed against a parent branch. Review-Finding: p1/specialist-batched#1 reader=specialist-batched Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes Keep the base-contains-head diagnostic ahead of the receipt base check, normalize any remote's refs, and tell callers to merge the base in before the carry. Review-Finding: v1/outside-voice#3 reader=outside-voice Review-Finding: v1/outside-voice#1 reader=outside-voice Review-Finding: v1/outside-voice#2 reader=outside-voice Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Say in the changelog entry that the base is merged into the child before the carry Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * List the receipt base mismatch refusal in the land refusal tables Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Leave the crew captain refusal table for a follow-up so this PR changes no agent instructions after 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>
2026-11-10 02:30:39 +01:00 · 2026-11-09 18:34:34 +00:00
1 parent cfedd05bb9
commit f75ce6fdcc
@@ -303,6 +303,7 @@ gstack_result_anchor() {
    QUEUE_RECEIPT_MISSING) printf '%s' 'queue-receipt-missing' ;;
    QUEUE_RECEIPT_STALE) printf '%s' 'queue-receipt-stale' ;;
    QUEUE_RECEIPT_CONDITIONAL) printf '%s' 'queue-receipt-conditional' ;;
+    QUEUE_RECEIPT_BASE_MISMATCH) printf '%s' 'queue-receipt-base-mismatch' ;;
    BASE_RED_UNCONFIGURED) printf '%s' 'base-red-unconfigured' ;;
    BASE_RED_UNNAMED) printf '%s' 'base-red-unnamed' ;;
    BASE_RED_REPRO_ERROR) printf '%s' 'base-red-repro-error' ;;
@@ -0,0 +1 @@
+- **The merge queue no longer lands a stacked child's parent commits under the child's receipt (GST-2153).** A child PR shipped against its parent branch (#3570) has a receipt that covers only the child's own change. If that child was retargeted to main while the parent was still open, `gstack-ship-queue ready`, `land --pin` and train build used to accept the receipt, so the parent's commits could land without their own review, READY or merged checks. They now refuse it with `QUEUE_RECEIPT_BASE_MISMATCH` until the parent lands, the base is merged into the child, and `gstack-ship-receipt carry --base <base>` moves the receipt to the base. Receipts written against the base, including stacked children reviewed against main (GST-1199), are unaffected.
@@ -567,6 +567,8 @@ describe('train build', () => {
    expect(queue(fx, ['train', 'build', '--prs', `1@${h1},2@${h2}`, '--dry-run']).err).toMatch(/PR #2's ship receipt is invalid \(.*\).*\(QUEUE_RECEIPT_MISSING\)/);
    setPrBody(fx, '2', shippedBody('1', h2));
    expect(queue(fx, ['train', 'build', '--prs', `1@${h1},2@${h2}`, '--dry-run']).err).toMatch(/PR #2's ship receipt names pr=1, not this PR.*\(QUEUE_RECEIPT_STALE\)/);
+    setPrBody(fx, '2', shippedBody('2', h2, { base: 'pr-1' }));
+    expect(queue(fx, ['train', 'build', '--prs', `1@${h1},2@${h2}`, '--dry-run']).err).toMatch(/PR #2's ship receipt was reviewed against pr-1, not main.*\(QUEUE_RECEIPT_BASE_MISMATCH\)/);
    expect(remoteRefs(fx)).toBe(refsBefore);
    expect(dispatches(fx)).toEqual([]);
    expect(fs.existsSync(path.join(fx.state, 'ship-queue', 'trains'))).toBe(false);
@@ -1985,6 +1985,11 @@ and from then on names the base it was carried to. A parent that has not landed,
 since, or a parent that landed with changes the child never merged all leave a
 different diff and refuse with `RECEIPT_CARRY_PRODUCT_CHANGED`. An older
 receipt whose parent branch is gone refuses with `RECEIPT_CARRY_REFUSED`.
+Until the carry moves it, `ready`, `land --pin` and train build refuse a
+receipt whose `base` is not the queue's base with
+`QUEUE_RECEIPT_BASE_MISMATCH` (GST-2153): a child retargeted to the base while
+its parent is still open would otherwise land the parent's commits under a
+receipt that reviewed only the child.

 **Stacked PRs reviewed against the base (GST-1199).** A child PR that targets
 the base while stacked has a receipt whose `base` is the base and whose
@@ -2378,8 +2383,9 @@ restamping`, GST-1245), so a reset stamp needs no hand cleanup:
   without a READY receipt (`gstack-ship-queue ready --pr <n>` records one
   on `refs/notes/gstack-trains` once its pregate, full_e2e and receipt checks
   pass; land checks nothing weaker), a /ship receipt that is missing or
-   bound to another head (`QUEUE_RECEIPT_MISSING`, `QUEUE_RECEIPT_STALE`), and
-   required checks that are not green at the pin.
+   bound to another head (`QUEUE_RECEIPT_MISSING`, `QUEUE_RECEIPT_STALE`) or
+   reviewed against another branch than the base
+   (`QUEUE_RECEIPT_BASE_MISMATCH`), and required checks that are not green at the pin.
 2. **Lock.** Take the repository land lock, `refs/notes/gstack-land` (its own
   ref, never the trains notes ref), by exact lease. It records the PR, holder,
   phase and journal. A held lock is `QUEUE_LAND_LOCKED` naming the holder;
@@ -2453,6 +2459,7 @@ release-file rows):
 | the PR is open, on the base, from this repository | `QUEUE_NOT_READY`, `QUEUE_TRAIN_UNTRUSTED` |
 | the PR head is the pin | [`QUEUE_LAND_PIN_MOVED`](troubleshooting.md#queue-land-pin-moved) |
 | the PR body's /ship receipt is bound to the pin | `QUEUE_RECEIPT_MISSING`, `QUEUE_RECEIPT_STALE` |
+| the receipt's `base` is the queue's base | [`QUEUE_RECEIPT_BASE_MISMATCH`](troubleshooting.md#queue-receipt-base-mismatch) |
 | a READY note from `ready` exists at the pin | `QUEUE_NOT_READY` |
 | no freeze is active (with `--repair`: one is, and its repair slot is free or already this pin; a backstop freeze left from before `release_gate` only warns; see [direct freeze](#direct-freeze)) and no land lock is held | [`QUEUE_TRAIN_FROZEN`](troubleshooting.md#queue-train-frozen), `QUEUE_NOT_READY`, `QUEUE_LAND_LOCKED` |
 | 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) |
@@ -6238,6 +6238,26 @@ READYs, trains, carries or merges on it, except on a train with `gates`, where
 <file>`), then run `/ship --resume <record>`, which reruns Step 5 and rewrites
 the receipt without `base_red`.

+<a id="queue-receipt-base-mismatch"></a>
+### `QUEUE_RECEIPT_BASE_MISMATCH`
+
+**Meaning.** The PR body's ship receipt was reviewed against another branch
+than the queue's base: its `base` names a parent branch, as a
+[stacked child's receipt](ship-policy.md) does (#3570), while the PR now
+targets the base. That receipt covers only the change since the parent, so
+the parent's commits the PR carries were never reviewed under it. `ready`,
+`land --pin` and train build refuse it while the parent is open, instead of
+landing the parent's commits past the parent's own receipt and READY.
+A receipt whose `base` is the queue's base (`main` or `origin/main`),
+including a stacked child reviewed against the base (GST-1199), is
+unaffected.
+
+**Fix.** Land the parent first, merge the base into this branch and push.
+Then run `gstack-ship-receipt carry --pr <n> --base <base>`: it moves the receipt to the base when the parent fully landed
+and the child is unchanged, and from then on the receipt names the base. If
+the carry refuses, rerun /ship against the base. Then rerun
+`gstack-ship-queue ready --pr <n>`.
+
 <a id="base-red-unconfigured"></a>
 ### `BASE_RED_UNCONFIGURED`

@@ -244,6 +244,11 @@ export const RELEASE_RESULT_CODES = {
    summary: 'the PR body’s /ship receipt is conditional: Step 5 failed on cases gstack-base-red proved are the base branch’s own, so /ship paused before READY',
    fix: 'wait for the base to pass those cases (gstack-base-red wait --pause <file>), then /ship --resume <record>, which re-runs Step 5 and rewrites the receipt without base_red; only ready --base-red --fixer <pr> READYs a conditional receipt, and it rides only a train with gates whose slot 1 is its fixer',
  },
+  QUEUE_RECEIPT_BASE_MISMATCH: {
+    anchor: 'queue-receipt-base-mismatch',
+    summary: 'the PR body’s /ship receipt was reviewed against another branch than the queue’s base (a #3570 stacked child retargeted before its parent landed), so it covers only the change since that branch',
+    fix: 'land the parent first, merge the base into this branch and push, then gstack-ship-receipt carry --pr <n> --base <base> moves the receipt to the base (it carries only when the parent fully landed and the child is unchanged), else rerun /ship against the base; then gstack-ship-queue ready again',
+  },
  BASE_RED_UNCONFIGURED: {
    anchor: 'base-red-unconfigured',
    summary: 'the base branch’s ship policy declares no base_red block, so gstack-base-red cannot reproduce a Step 5 failure there and every failure stays in-branch',
@@ -504,6 +504,8 @@ export function bindShippedReceipt(root: string, baseRef: string, body: string,
  if (!b.ok) return b;
  const self = merged ? null : baseContainsHead(root, baseRef, b.head);
  if (self) return { ok: false, code: 'BASE_CONTAINS_HEAD', message: self, line: b.line };
+  const based = merged ? b : bindReceipt(body, pr, accepted, { base: baseRef });
+  if (!based.ok) return based;
  const ancestor = (commit: string) => b.receipt.carry?.ancestry === 'rewritten' || git(root, ['merge-base', '--is-ancestor', commit, b.head]).ok;
  const problems = exceptionProblems(body, b.receipt, ancestor);
  if (problems.length) return { ok: false, code: 'QUEUE_RECEIPT_EXCEPTION_INVALID', message: `the receipt's owner exception does not stand: ${problems.join('; ')}`, line: b.line };
@@ -71,7 +71,7 @@ export function extractReceipt(body: string): { found: boolean; receipt?: ShipRe
  }
 }

-export type ReceiptBindingCode = 'QUEUE_RECEIPT_MISSING' | 'QUEUE_RECEIPT_STALE' | 'QUEUE_RECEIPT_CONDITIONAL';
+export type ReceiptBindingCode = 'QUEUE_RECEIPT_MISSING' | 'QUEUE_RECEIPT_STALE' | 'QUEUE_RECEIPT_CONDITIONAL' | 'QUEUE_RECEIPT_BASE_MISMATCH';
 export type ReceiptBinding =
  | { ok: true; receipt: ShipReceipt; head: string; line: string }
  | { ok: false; code: ReceiptBindingCode; message: string; line: string };
@@ -87,9 +87,12 @@ export type ReceiptBinding =
 * A receipt with a `base_red` block (lib/base-red.ts) is conditional: its
 * Step 5 did not pass, so it binds only for a caller that asks for it
 * (`/ship`'s terminal line, which reports the pause) and is refused
- * everywhere else.
+ * everywhere else. With `base` (the queue's base), a receipt reviewed against
+ * another branch is refused: a #3570 child retargeted before its parent
+ * landed reviewed only its own change, and the carry rewrites `base` once the
+ * parent has landed (GST-2153).
 */
-export function bindReceipt(body: string, pr: string | undefined, accepted: readonly string[], opts: { conditional?: boolean } = {}): ReceiptBinding {
+export function bindReceipt(body: string, pr: string | undefined, accepted: readonly string[], opts: { conditional?: boolean; base?: string } = {}): ReceiptBinding {
  const subject = pr === undefined ? "the receipt's" : `PR #${pr}'s`;
  const ex = extractReceipt(body);
  if (!ex.found) return { ok: false, code: 'QUEUE_RECEIPT_MISSING', message: `${subject} body carries no ship receipt`, line: 'SHIP_RECEIPT: none in the PR body' };
@@ -105,6 +108,11 @@ export function bindReceipt(body: string, pr: string | undefined, accepted: read
    return { ok: false, code: 'QUEUE_RECEIPT_STALE', message: `${subject} ship receipt is for ${oneLine(r.head.slice(0, 12))}, not the head ${want} being merged`, line };
  }
  if (pr !== undefined && r.pr !== undefined && String(r.pr) !== pr) return { ok: false, code: 'QUEUE_RECEIPT_STALE', message: `${subject} ship receipt names pr=${r.pr}, not this PR`, line };
+  const branch = (ref: string) => ref.replace(/^refs\/remotes\/[^/]+\//, '').replace(/^origin\//, '');
+  if (opts.base !== undefined && branch(r.base) !== branch(opts.base)) {
+    const [was, want] = [oneLine(r.base).slice(0, 120), branch(opts.base)];
+    return { ok: false, code: 'QUEUE_RECEIPT_BASE_MISMATCH', message: `${subject} ship receipt was reviewed against ${was}, not ${want}, so it covers only the change since ${was}; land the parent (${was}) first, merge ${want} into this branch and push, then gstack-ship-receipt carry --pr ${pr ?? '<n>'} --base ${want} moves the receipt to ${want}`, line };
+  }
  if (r.base_red && !opts.conditional) return { ok: false, code: 'QUEUE_RECEIPT_CONDITIONAL', message: `${subject} ship receipt is paused on a red base (${oneLine(r.base_red.cases.join(', ')).slice(0, 160)} fail at ${oneLine(r.base_red.base.slice(0, 12))}); /ship --resume rewrites it once the base passes them`, line };
  return { ok: true, receipt: r, head, line };
 }
@@ -361,6 +361,23 @@ describe('ready', () => {
    expect(queue(repo, ['ready', '--pr', '1'], shim(moved, green)).err).toMatch(/uncommitted tracked changes: pr-a\.txt.*\(QUEUE_NOT_READY\)/);
  }, 180_000);

+  test('QUEUE_RECEIPT_BASE_MISMATCH: a #3570 child retargeted to main while its parent is open is refused; a GST-1199 child whose receipt names main READYs', () => {
+    const repo = fixture();
+    makePr(repo, 'parent');
+    repo.git('push', '-q', 'origin', 'HEAD:refs/heads/parent');
+    fs.writeFileSync(path.join(repo.work, 'child.txt'), 'child\n');
+    repo.git('add', '-A');
+    repo.git('commit', '-q', '-m', 'child');
+    const head = repo.git('rev-parse', 'HEAD');
+    const pregate = pregateFor(repo, head);
+    const shim = (base: string) => ghQueueShim(repo, { prs: { '2': { headRefOid: head, url: 'https://github.com/o/r/pull/2', baseRefName: 'main', body: shippedBody('2', head, { base }) } }, checks: { [head]: green } });
+    const retargeted = queue(repo, ['ready', '--pr', '2', '--pregate', pregate], shim('parent'));
+    expect(retargeted.status).toBe(EXIT.refused);
+    expect(retargeted.out).toContain(`SHIP_RECEIPT: pr=2 head=${head.slice(0, 12)} version=- base=parent`);
+    expect(retargeted.err).toMatch(/PR #2's ship receipt was reviewed against parent, not main.*\(QUEUE_RECEIPT_BASE_MISMATCH/);
+    for (const base of ['main', 'origin/main']) expect(queue(repo, ['ready', '--pr', '2', '--pregate', pregate], shim(base)).status).toBe(EXIT.ok);
+  }, 120_000);
+
  test('STAMP_VERSION is unstamped for a PR whose own diff leaves VERSION alone after main moved to a newer version, and the stamp for one that changes it (GST-770 follow-up)', () => {
    const repo = fixture();
    const head = makePr(repo, 'pr-a');
@@ -528,6 +528,35 @@ describe('land --pin: refusals (nothing merged)', () => {
    refused(fx, land(fx, '15', pin), 'QUEUE_RECEIPT_MISSING');
  }, 90_000);

+  test('QUEUE_RECEIPT_BASE_MISMATCH: a #3570 child retargeted to main while its parent is open lands nothing; a GST-1199 child reviewed against main still lands', () => {
+    const fx = setup();
+    const { repo } = fx;
+    openPr(fx, '17');
+    const child = (pr: string, base: string): string => {
+      repo.git('checkout', '-q', 'origin/pr-17', '-B', 'pr-' + pr);
+      fs.writeFileSync(path.join(repo.work, 'pr-' + pr + '.txt'), pr + '\n');
+      fs.writeFileSync(path.join(repo.work, 'changelog.d', 'pr-' + pr + '.md'), '- #' + pr + ' change\n');
+      repo.git('add', '-A');
+      repo.git('commit', '-q', '-m', 'pr ' + pr);
+      repo.git('push', '-q', '-f', 'origin', 'HEAD:refs/heads/pr-' + pr);
+      const head = repo.git('rev-parse', 'HEAD');
+      fx.write(d => { d.prs[pr] = { state: 'OPEN', baseRefName: 'main', headRefName: 'pr-' + pr, isCrossRepository: false, title: 'PR ' + pr, body: shippedBody(pr, head, { base }) }; });
+      markReady(fx, pr, head);
+      repo.git('checkout', '-q', '--detach', 'origin/main');
+      return head;
+    };
+    const before = remote(fx, 'main');
+    const retargeted = child('171', 'pr-17');
+    const r = land(fx, '171', retargeted);
+    refused(fx, r, 'QUEUE_RECEIPT_BASE_MISMATCH');
+    expect(r.err).toContain("PR #171's ship receipt was reviewed against pr-17, not main");
+    expect(remote(fx, 'main')).toBe(before);
+    const reviewedOnMain = child('172', 'origin/main');
+    const ok = land(fx, '172', reviewedOnMain);
+    expect(ok.err).toBe('');
+    expect(ok.status).toBe(EXIT.ok);
+  }, 90_000);
+
  test('QUEUE_NOT_READY: no READY note at the pin', () => {
    const fx = setup();
    const pin = openPr(fx, '16', { ready: false });
				`@@ -0,0 +1 @@`
				- The merge queue no longer lands a stacked child's parent commits under the child's receipt (GST-2153). A child PR shipped against its parent branch (#3570) has a receipt that covers only the child's own change. If that child was retargeted to main while the parent was still open, `gstack-ship-queue ready`, `land --pin` and train build used to accept the receipt, so the parent's commits could land without their own review, READY or merged checks. They now refuse it with `QUEUE_RECEIPT_BASE_MISMATCH` until the parent lands, the base is merged into the child, and `gstack-ship-receipt carry --base <base>` moves the receipt to the base. Receipts written against the base, including stacked children reviewed against main (GST-1199), are unaffected.