From 88b8f36cb32a217fa2d6fdbf1a9dd92d9faf4172 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 11:22:23 +0000 Subject: [PATCH] fix(ship-receipt): a stacked carry ignores files the child's own earlier main merge brought in (GST-1214) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-ship-receipt | 15 +++++--- docs/ship-policy.md | 5 ++- test/ship-receipt-carry-squash.test.ts | 51 ++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 6 deletions(-) diff --git a/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index 41a4490e5..64a7b9b70 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -666,11 +666,16 @@ function cmdCarry(rest: string[], json: boolean): number { // receipt written against the base while stacked reviewed the parent's hunks too, and the squash moved them into the // base (GST-1199). A parent counts only when merging it into the current base changes no product path. const measure = (rangeBase: string) => { - const listed = [touchedPaths(repoRoot, rangeBase, from), touchedPaths(repoRoot, headBase, head)]; - if (!listed[0] || !listed[1]) return `git could not list the paths the branch touched against ${base}; run /ship on ${head.slice(0, 12)}`; - const touched = new Set([...listed[0], ...listed[1]]); - const spec = productSpec(repoRoot, rules, from, head, { touched, sides: [[rangeBase, from], [headBase, head]], own: branchSides(repoRoot, baseRef, from, '') }); - if (!spec) return `git diff ${from.slice(0, 12)} ${head.slice(0, 12)} failed`; + const listed = [touchedPaths(repoRoot, rangeBase, from), touchedPaths(repoRoot, headBase, head), rangeBase === fromBase ? new Set() : touchedPaths(repoRoot, fromBase, from)]; + if (!listed[0] || !listed[1] || !listed[2]) return `git could not list the paths the branch touched against ${base}; run /ship on ${head.slice(0, 12)}`; + // From a stack parent, a path whose reviewed content is the base's (an earlier base merge brought it in) is base + // context, not reviewed product, unless HEAD changed it against the base (GST-1214). + const baseMerged = rangeBase === fromBase ? [] : [...listed[0]].filter(p => !listed[2]!.has(p) && !listed[1]!.has(p)); + const touched = new Set([...listed[0], ...listed[1]].filter(p => !baseMerged.includes(p))); + const found = productSpec(repoRoot, rules, from, head, { touched, sides: [[rangeBase, from], [headBase, head]], own: branchSides(repoRoot, baseRef, from, '') }); + if (!found) return `git diff ${from.slice(0, 12)} ${head.slice(0, 12)} failed`; + const skip = baseMerged.map(p => `:(exclude,literal)${p}`); + const spec = { ...found, product: [...found.product, ...skip], tests: [...found.tests, ...skip] }; const ids = [patchId(repoRoot, rangeBase, from, spec.product), patchId(repoRoot, headBase, head, spec.product)]; for (const r of ids) if (typeof r !== 'string') return `git could not compute the product patch-id at one of the heads: ${r.error}`; return { rangeBase, touched, spec, productFrom: ids[0] as string, productHead: ids[1] as string }; diff --git a/docs/ship-policy.md b/docs/ship-policy.md index f25e55b85..120d89cb1 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1535,7 +1535,10 @@ the reviewed range (the reviewed head excluded, at most 100 commits) whose product change the current base already holds: `git merge-tree` of that commit into the base changes no product path. A conflicted product path never counts, so a parent that never landed is never treated as one, even when the child's -merge dropped its change. The comparison then runs from that commit: an equal +merge dropped its change. The comparison then runs from that commit, leaving out +every path whose content at the reviewed head is the base's (a base merge the +child made before its receipt brought it in) unless HEAD changed it against the +base (GST-1214): an equal patch-id carries with reason `stack parent landed`, base context next to the child's unchanged hunks is `RECEIPT_CARRY_SCOPED_REVIEW`, and any other difference refuses with `RECEIPT_CARRY_PRODUCT_CHANGED`. diff --git a/test/ship-receipt-carry-squash.test.ts b/test/ship-receipt-carry-squash.test.ts index a47e025c1..34474216f 100644 --- a/test/ship-receipt-carry-squash.test.ts +++ b/test/ship-receipt-carry-squash.test.ts @@ -240,3 +240,54 @@ describe('carry after a stack parent squash-merges, receipt written against main expect(landedStackParent(repo.work, { rangeBase: fork, reviewed, currentBase: mainTip }, ['.', ':(exclude,glob)test/**'])).toBeNull(); }, 60_000); }); + +describe('carry after a stack parent lands, the child having merged main before its receipt (GST-1214)', () => { + /** main <- parent (lib/util.js) <- feature (lib/retry.js), which merges main (wiki/) once before its receipt is written against main. */ + function mergedStack(repo: FixtureRepo): { parent: string; reviewed: string; body: string } { + repo.git('checkout', '-q', '-b', 'parent'); + const parent = commit(repo, { 'lib/util.js': SUB }, 'parent: sub'); + repo.git('push', '-q', 'origin', 'parent'); + repo.git('checkout', '-q', 'feature'); + repo.git('reset', '-q', '--hard', parent); + commit(repo, { 'lib/retry.js': RETRY4 }, 'child: four retries'); + repo.git('checkout', '-q', 'main'); + commit(repo, { 'wiki/a.md': '# a\n', 'wiki/b.md': '# b\n' }, 'main: wiki'); + repo.git('push', '-q', 'origin', 'main'); + repo.git('checkout', '-q', 'feature'); + repo.git('merge', '-q', '--no-edit', 'origin/main'); + const reviewed = repo.git('rev-parse', 'HEAD'); + const body = path.join(repo.root, 'body.md'); + const w = runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '8', '--base', 'main', '--version', '1.0.0', '--session-kind', 'unattended', '--body', body]); + expect(w.status).toBe(EXIT.ok); + return { parent, reviewed, body }; + } + + // Value: protects=a stacked child that merged main before its receipt keeps it once the parent squash-lands and main is + // merged again (gstack-evals#178 refused on 20 wiki/ paths its first base merge brought in); fails_when=the range from the + // landed parent counts paths that hold main's content at the reviewed head; seam=none; no_claim=gh pr edit (--body stands in) + test('the child carries when only its earlier base merge separates the parent range from HEAD\'s diff against main', () => { + const repo = makeFixtureRepo(); repos.push(repo); + const { parent, reviewed, body } = mergedStack(repo); + landOnMain(repo, { 'lib/util.js': SUB }); + const head = repo.git('rev-parse', 'HEAD'); + const out = carry(repo, body); + expect(out.err).not.toContain('RECEIPT_CARRY_PRODUCT_CHANGED'); + expect(out.status).toBe(EXIT.ok); + expect(out.out).toContain(`CARRY: carried from ${reviewed.slice(0, 12)} to ${head.slice(0, 12)}`); + expect(receiptIn(body).carry!.reason).toBe(`stack parent ${parent.slice(0, 12)} landed + base merge`); + }, 90_000); + + // Value: protects=fail closed: an edit to a file the earlier base merge brought in is product and refuses; fails_when=every + // base-merged path drops out of both sides, the child's own edit included; seam=none; no_claim=- + test('an edit to a base-merged file after the receipt refuses with RECEIPT_CARRY_PRODUCT_CHANGED', () => { + const repo = makeFixtureRepo(); repos.push(repo); + const { reviewed, body } = mergedStack(repo); + landOnMain(repo, { 'lib/util.js': SUB }); + commit(repo, { 'wiki/a.md': '# a, edited by the child\n' }, 'child: edit wiki'); + const out = carry(repo, body); + expect(out.status).toBe(EXIT.refused); + expect(out.out).toContain('CARRY: refused (RECEIPT_CARRY_PRODUCT_CHANGED)'); + expect(out.err).toContain('wiki/a.md'); + expect(receiptIn(body).head).toBe(reviewed); + }, 90_000); +});