diff --git a/lib/ship-queue-land.ts b/lib/ship-queue-land.ts index ee71af2f6..f25fcc815 100644 --- a/lib/ship-queue-land.ts +++ b/lib/ship-queue-land.ts @@ -360,7 +360,7 @@ export function land(ctx: QueueContext, o: LandOptions): StepOutput { const pin = j?.reviewed_pin ?? recovered ?? info.headRefOid; const lines = [`LAND: #${o.pr} branch=${info.headRefName} pin=${pin}${j ? ` (resuming at ${j.phase})` : recovered ? ` (the head ${info.headRefOid.slice(0, 12)} is land's stamp commit; restamping from its pin)` : ''}`]; git(root, ['fetch', '-q', 'origin', ctx.base]); - const receipt = bindShippedReceipt(root, `origin/${ctx.base}`, info.body ?? '', o.pr, [pin]); + const receipt = bindShippedReceipt(root, `origin/${ctx.base}`, info.body ?? '', o.pr, [pin], mergedResume); lines.push(receipt.line, ...(receipt.depthLine ? [receipt.depthLine] : [])); if (!receipt.ok) return fail(lines, { code: receipt.code, message: receipt.message, fix: 'run /ship at the pinned head first' }, run); if (!policy.changelog_fragments) return fail(lines, { code: 'QUEUE_NOT_READY', message: 'the base policy has changelog_fragments off; land merges only under the key, where ready records the READY receipt it checks' }, run); diff --git a/lib/ship-receipt-depth.ts b/lib/ship-receipt-depth.ts index b89226766..85bc36630 100644 --- a/lib/ship-receipt-depth.ts +++ b/lib/ship-receipt-depth.ts @@ -190,13 +190,13 @@ export function receiptDepthVerdict(root: string, baseRef: string, r: ShipReceip return { ok: false, line, message: `the receipt ${why}, but head ${head.slice(0, 12)} needs ${need.depth} (${need.reasons.join(', ')}${errs ? `; ${errs}` : ''}); the receipt is unchanged; run /ship on ${head.slice(0, 12)}` }; } -/** bindReceipt plus the owner exceptions and the review depth: what READY and train build accept; the targeted line is reported, never refused on. A base that already contains the head refuses (BASE_CONTAINS_HEAD, GST-774) before any depth is computed. */ -export function bindShippedReceipt(root: string, baseRef: string, body: string, pr: string | undefined, accepted: readonly string[]): +/** bindReceipt plus the owner exceptions and the review depth: what READY and train build accept; the targeted line is reported, never refused on. A base that already contains the head refuses (BASE_CONTAINS_HEAD, GST-774) before any depth is computed, except for a PR GitHub already merged (`merged`: land resuming after a merge-only true merge). */ +export function bindShippedReceipt(root: string, baseRef: string, body: string, pr: string | undefined, accepted: readonly string[], merged = false): | { ok: true; receipt: ShipReceipt; head: string; line: string; depthLine: string; targetedLine: string } | { ok: false; code: ReceiptBindingCode | 'BASE_CONTAINS_HEAD' | 'QUEUE_RECEIPT_DEPTH_RAISED' | 'QUEUE_RECEIPT_EXCEPTION_INVALID'; message: string; line: string; depthLine?: string; targetedLine?: string } { const b = bindReceipt(body, pr, accepted); if (!b.ok) return b; - const self = baseContainsHead(root, baseRef, b.head); + const self = merged ? null : baseContainsHead(root, baseRef, b.head); if (self) return { ok: false, code: 'BASE_CONTAINS_HEAD', message: self, line: b.line }; 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); diff --git a/test/ship-queue-land.test.ts b/test/ship-queue-land.test.ts index 4b8fabfb8..b965919a7 100644 --- a/test/ship-queue-land.test.ts +++ b/test/ship-queue-land.test.ts @@ -47,7 +47,7 @@ function setup(policy: Record = POLICY): Fx { " if(head!==want){console.error('Head branch was modified');process.exit(1)}", " if(d.mergeRefuse){console.error('Pull request is not mergeable: the base branch policy prohibits the merge');process.exit(1)}", " if(d.driftOnMerge){const m=g('rev-parse','refs/heads/main');const t=g('rev-parse',m+'^{tree}');g('update-ref','refs/heads/main',g('commit-tree',t,'-p',m,'-m','outside merge'));d.driftOnMerge=false}", - " const main=g('rev-parse','refs/heads/main');const sq=g('commit-tree',g('rev-parse',want+'^{tree}'),'-p',main,'-m','squash #'+a[2]);g('update-ref','refs/heads/main',sq);if(d.outsideAfter){g('update-ref','refs/heads/main',g('commit-tree',g('rev-parse',sq+'^{tree}'),'-p',sq,'-m','outside after'));d.outsideAfter=false}d.mergeArgs=a.slice(3);p.state='MERGED';save();if(d.mergeErrorAfter){d.mergeErrorAfter=false;save();console.error('HTTP 502: Bad Gateway');process.exit(1)}process.exit(0)}", + " const main=g('rev-parse','refs/heads/main');const sq=g('commit-tree',g('rev-parse',want+'^{tree}'),'-p',main,...(d.trueMerge&&a.includes('--merge')?['-p',want]:[]),'-m','squash #'+a[2]);g('update-ref','refs/heads/main',sq);if(d.outsideAfter){g('update-ref','refs/heads/main',g('commit-tree',g('rev-parse',sq+'^{tree}'),'-p',sq,'-m','outside after'));d.outsideAfter=false}d.mergeArgs=a.slice(3);p.state='MERGED';save();if(d.mergeErrorAfter){d.mergeErrorAfter=false;save();console.error('HTTP 502: Bad Gateway');process.exit(1)}process.exit(0)}", "if(a[0]==='pr'&&a[1]==='create'&&d.createFail){d.createFail=false;save();console.error('timeout');process.exit(1)}", "if(a[0]==='pr'&&a[1]==='create'){const h=a[a.indexOf('--head')+1];const had=Object.entries(d.prs).find(([,p])=>p.headRefName===h&&p.state==='OPEN');if(had){console.error('a pull request for branch \"'+h+'\" into branch \"main\" already exists:\\nhttps://github.com/o/r/pull/'+had[0]);process.exit(1)}}", "if(a[0]==='pr'&&a[1]==='create'){const n=String(Math.max(0,...Object.keys(d.prs).map(Number))+1);d.prs[n]={state:'OPEN',headRefName:a[a.indexOf('--head')+1],isCrossRepository:false,title:a[a.indexOf('--title')+1],body:''};save();console.log('https://github.com/o/r/pull/'+n);process.exit(0)}", @@ -562,6 +562,20 @@ describe('land: one pass, base, lock and history', () => { expect(r.out).toContain('LANDED: '); expect(JSON.parse(fs.readFileSync(fx.data, 'utf8')).mergeArgs).toContain('--merge'); }, 90_000); + + // Value: protects=a merge-only land interrupted after GitHub merged resumes to LANDED, though a true merge puts the pin inside + // the base; fails_when=the receipt check refuses the merged pin as BASE_CONTAINS_HEAD and the lock is never released; + // why_new=GST-774's base guard, and the shim only squashed; seam=none; no_claim=GitHub's merge-commit shape beyond two parents + test('merge-only: a true merge that errored after GitHub merged resumes to LANDED, not BASE_CONTAINS_HEAD', () => { + const fx = setup({ ...POLICY, history: 'merge-only' }); + openPr(fx, '14'); + land(fx, ['--pr', '14']); + fx.write(d => { d.trueMerge = true; d.mergeErrorAfter = true; }); + expect(land(fx, ['--pr', '14']).err).toContain('HTTP 502'); + const again = land(fx, ['--pr', '14']); + expect(again.out).not.toContain('BASE_CONTAINS_HEAD'); + expect(again.out).toContain('LANDED: '); + }, 90_000); }); // /ship adversarial (Claude) and outside-voice (Codex) findings (GSTA-154 PR A): each case failed or was unreachable before its fix.