From 764cc2dcb2dda30b0531b4d14970921b97c54d3b Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 07:11:34 +0000 Subject: [PATCH 1/9] fix(ship-receipt): carry a stacked PR's receipt after its parent squash-merges into main (GST-1199) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-ship-receipt | 43 +++++++----- .../gst-1199-carry-landed-stack-parent.md | 1 + docs/ship-policy.md | 16 +++++ lib/ship-receipt.ts | 27 +++++++- test/ship-receipt-carry-squash.test.ts | 65 ++++++++++++++++++- 5 files changed, 133 insertions(+), 19 deletions(-) create mode 100644 changelog.d/gst-1199-carry-landed-stack-parent.md diff --git a/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index cc4e80dae..6d4e6c686 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -44,7 +44,7 @@ import { isUnavailable, loadPolicyFromBase, matchesAny, policyUnavailableLine, r import { approvalForReceipt } from '../lib/approval-citations'; import { CARRY_METADATA_GLOBS, CARRY_NAMED_PATHS, CARRY_TEST_GLOBS, - bindReceipt, carryReceipt, classifyCarryPath, computeCensus, extractReceipt, missingIssueLines, orderReceipt, parseReviewLoop, placeReceiptFirst, planCarry, planTreeCarry, + bindReceipt, carryReceipt, classifyCarryPath, computeCensus, extractReceipt, landedStackParent, missingIssueLines, orderReceipt, parseReviewLoop, placeReceiptFirst, planCarry, planTreeCarry, receiptIssuesFrom, receiptSummaryLine, renderCensus, renderReceiptBlock, validateReceipt, type CensusPr, } from '../lib/ship-receipt'; import { checkDocsCaps, docsCapLine, docsExceptionLine, parseReceiptDocs } from '../lib/ship-receipt-docs'; @@ -656,18 +656,29 @@ function cmdCarry(rest: string[], json: boolean): number { } else { const fromBase = git(['merge-base', baseRef, from], repoRoot); if (!fromBase) return refused('RECEIPT_CARRY_REFUSED', `cannot resolve the merge base of ${baseRef} (fetch it first)`); - const listed = [touchedPaths(repoRoot, fromBase, from), touchedPaths(repoRoot, headBase, head)]; - if (!listed[0] || !listed[1]) return refused('RECEIPT_CARRY_REFUSED', `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: [[fromBase, from], [headBase, head]], own: branchSides(repoRoot, baseRef, from, '') }); - if (!spec) return refused('RECEIPT_CARRY_REFUSED', `git diff ${from.slice(0, 12)} ${head.slice(0, 12)} failed`); + // The reviewed range runs from the fork point, or from a stack parent inside it that has since landed on the base: a + // 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 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 }; + }; + let measured = measure(fromBase); + if (typeof measured === 'string') return refused('RECEIPT_CARRY_REFUSED', measured); + const stackParent = !retarget && measured.productFrom !== measured.productHead ? landedStackParent(repoRoot, { rangeBase: fromBase, reviewed: from, currentBase: headBase }, measured.spec.product) : null; + if (stackParent) measured = measure(stackParent); + if (typeof measured === 'string') return refused('RECEIPT_CARRY_REFUSED', measured); + const { rangeBase, touched, spec, productFrom, productHead } = measured; const { changed } = spec; - const productFrom = patchId(repoRoot, fromBase, from, spec.product); - const productHead = patchId(repoRoot, headBase, head, spec.product); const asId = (r: string | { error: string }) => (typeof r === 'string' ? r : ''); - const testFrom = asId(patchId(repoRoot, fromBase, from, spec.tests)); + const testFrom = asId(patchId(repoRoot, rangeBase, from, spec.tests)); const testHead = asId(patchId(repoRoot, headBase, head, spec.tests)); - for (const r of [productFrom, productHead]) if (typeof r !== 'string') return refused('RECEIPT_CARRY_REFUSED', `git could not compute the product patch-id at one of the heads: ${r.error}`); // A stacked child whose parent landed on the base (#3570): the receipt reviewed the child's own range against the // parent branch, and the PR now targets the base. That reviewed range (the recorded product patch-id, else the one // against origin/ when it still resolves) must be HEAD's whole product diff against the base, computed @@ -691,8 +702,8 @@ function cmdCarry(rest: string[], json: boolean): number { // which can differ between the two base diffs. A touched path with the same blob at both heads and at both bases has // the same patch, so only paths that moved on one side are compared (GST-448: 6000 touched paths). A path whose only difference is base context next to unchanged hunks // (lib/ship-patch.ts) needs a scoped review instead of the loop. - const sides = { reviewedBase: fromBase, reviewed: from, currentBase: headBase, current: head }; - const moved = [touchedPaths(repoRoot, from, head), touchedPaths(repoRoot, fromBase, headBase)]; + const sides = { reviewedBase: rangeBase, reviewed: from, currentBase: headBase, current: head }; + const moved = [touchedPaths(repoRoot, from, head), touchedPaths(repoRoot, rangeBase, headBase)]; if (moved.some(l => !l)) return refused('RECEIPT_CARRY_REFUSED', `git could not list the paths the branch touched against ${base}; run /ship on ${head.slice(0, 12)}`); const [movedHead, movedBase] = moved as Set[]; // A stamp file the branch changed as product (its own row) whose reviewed-to-current change is a stamp on its own lines @@ -710,14 +721,14 @@ function cmdCarry(rest: string[], json: boolean): number { if (!changedProduct.length && !left.length) changedProduct.push('(a product path outside the branch\'s touched files; compare the two base diffs)'); else if (!changedProduct.length) { const rest = [...spec.product, ...left.map(p => `:(exclude,literal)${p}`)]; - const restFrom = patchId(repoRoot, fromBase, from, rest); + const restFrom = patchId(repoRoot, rangeBase, from, rest); const restHead = patchId(repoRoot, headBase, head, rest); if (typeof restFrom !== 'string' || typeof restHead !== 'string' || restFrom !== restHead) changedProduct.push(`(a product path outside ${left.join(', ')}; compare the two base diffs without them)`); } } const stampedRows = changedProduct.length ? [] : stampOnly; let scopedReview: string | undefined; - const packetBody = !changedProduct.length && contextPaths.length ? contextPacket(repoRoot, { reviewedBase: fromBase, reviewed: from, currentBase: headBase, current: head }, contextPaths) : null; + const packetBody = !changedProduct.length && contextPaths.length ? contextPacket(repoRoot, { reviewedBase: rangeBase, reviewed: from, currentBase: headBase, current: head }, contextPaths) : null; if (packetBody?.over.length) changedProduct.push(...packetBody.over.map(p => `${p} (too large for the scoped review: over ${PACKET_SECTION_CAP} lines in a section)`)); if (packetBody && !changedProduct.length) { const record = flag(rest, '--scoped-review'); @@ -743,9 +754,9 @@ function cmdCarry(rest: string[], json: boolean): number { } const mergeCommits = (git(['rev-list', '--merges', '--count', `${from}..${head}`], repoRoot) ?? '0').trim(); // Evidence for the record, never the decision: a slow range-diff is reported unavailable rather than waited for. - const rangeRaw = spawnSync('git', ['range-diff', '--no-color', `${fromBase}..${from}`, `${headBase}..${head}`], { cwd: repoRoot, encoding: 'utf8', timeout: 15_000, maxBuffer: 64 * 1024 * 1024 }); + const rangeRaw = spawnSync('git', ['range-diff', '--no-color', `${rangeBase}..${from}`, `${headBase}..${head}`], { cwd: repoRoot, encoding: 'utf8', timeout: 15_000, maxBuffer: 64 * 1024 * 1024 }); const rangeDiff = rangeRaw.status === 0 ? (rangeRaw.stdout.trim().split('\n').slice(0, 20).join('\n') || '(empty: identical ranges)') : `unavailable: ${(rangeRaw.stderr ?? '').trim().split('\n')[0] || (rangeRaw.signal ? `range-diff killed by ${rangeRaw.signal} after 15 s` : 'range-diff failed')}`; - plan = planCarry({ from, head, changed, productPatchId: { from: reviewed as string, head: current as string }, testPatchId: { from: testFrom, head: testHead }, changedProduct, mergeCommits: Number(mergeCommits) || 0, rangeDiff, ...(retarget ? { retargetedFrom: reviewedBase } : {}), ...(scopedReview ? { scopedReview } : {}), ...(stampedRows.length ? { stampedRows } : {}) }); + plan = planCarry({ from, head, changed, productPatchId: { from: reviewed as string, head: current as string }, testPatchId: { from: testFrom, head: testHead }, changedProduct, mergeCommits: Number(mergeCommits) || 0, rangeDiff, ...(retarget ? { retargetedFrom: reviewedBase } : {}), ...(stackParent ? { stackParent } : {}), ...(scopedReview ? { scopedReview } : {}), ...(stampedRows.length ? { stampedRows } : {}) }); } if (!plan.ok) return refused(plan.code, plan.message); // Eligibility is not publication: a receipt that recorded a pregate is rebound only with a pregate current for HEAD, diff --git a/changelog.d/gst-1199-carry-landed-stack-parent.md b/changelog.d/gst-1199-carry-landed-stack-parent.md new file mode 100644 index 000000000..ccede1eca --- /dev/null +++ b/changelog.d/gst-1199-carry-landed-stack-parent.md @@ -0,0 +1 @@ +- **A stacked PR keeps its /ship receipt after its parent squash-merges, even when the receipt was written against main (GST-1199).** `gstack-ship-receipt carry` used to refuse with `RECEIPT_CARRY_PRODUCT_CHANGED` once the parent's hunks moved into main, forcing a full re-review of unchanged code. The carry now finds the newest reviewed commit that main already holds (`git merge-tree` into main changes no product path) and compares from there: an unchanged child carries as `stack parent landed`, base edits next to the child's hunks go to `RECEIPT_CARRY_SCOPED_REVIEW`, and any other change still refuses. A parent that never landed never counts, even if the child's merge dropped its change. diff --git a/docs/ship-policy.md b/docs/ship-policy.md index a6fcabd18..004766f17 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1503,6 +1503,7 @@ receipt when the base moves: | a rebase or force-push onto a tree the receipt covers, product diff against the new base unchanged | carries (`carry.ancestry: rewritten`) | | any other rebase or force-push | a fresh /ship (`RECEIPT_CARRY_TREE_MISMATCH`) | | a stacked PR whose parent landed (squash or merge), retargeted to the base with the base merged in, HEAD's product diff exactly the one reviewed against the parent branch | carries (reason `stack parent landed`) | +| a stacked PR whose receipt was written against the base, after its parent landed and the base was merged in, HEAD's product diff exactly the reviewed range from the parent's head | carries (reason `stack parent landed`); base context next to the child's hunks is `RECEIPT_CARRY_SCOPED_REVIEW` | | a stamp on the branch's own rows in a stamp file the branch also changed as product: every changed line at a branch-added position, the file's mode unchanged, the base not touching it, the rest of the product diff unchanged | carries (reason `a stamp on the branch's own rows ()`) | **Stacked PRs (#3570).** A child PR's receipt is written against its parent @@ -1520,6 +1521,21 @@ 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`. +**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 +reviewed range holds the parent's hunks too, and the receipt records no stack +parent. Once the parent squash-merges and the base is merged into the child, +the two heads' product diffs differ by exactly the parent's hunks, which now +sit in the base. When they differ, the carry looks for the newest commit inside +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 +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`. + Since the release that added `review_scope`. A `/ship` receipt names the head it reviewed, so a conflict-only merge of the base or a test-only fix used to invalidate it and cost a full re-run on identical product code. diff --git a/lib/ship-receipt.ts b/lib/ship-receipt.ts index b1baa4b61..5895d5bbb 100644 --- a/lib/ship-receipt.ts +++ b/lib/ship-receipt.ts @@ -6,6 +6,7 @@ * computes the P7 adoption census over merged PRs (deduplicated by run and * thread id against an owner-maintained thread cohort). */ +import { spawnSync } from 'node:child_process'; import { ARTIFACT_SCHEMAS, checkSchema, type ShipReceipt, type ValidationError } from './headless-artifacts'; import { approvalSummary } from './approval-citations'; import { FRAGMENT_README } from './changelog-fragments'; @@ -177,6 +178,8 @@ export interface CarryPlanInput { rangeDiff: string; /** The base the receipt was reviewed against when it differs from the carry's (a stack parent that landed, #3570): productPatchId.from is then the recorded diff against it. */ retargetedFrom?: string; + /** A commit inside the reviewed range whose product change the base already holds (a stack parent that squash-merged under a receipt written against the base, GST-1199): productPatchId.from is then the range from it. */ + stackParent?: string; /** A clean scoped review of product paths whose only difference is base context next to unchanged hunks (lib/ship-patch.ts, GST-384): the carry reason it adds. */ scopedReview?: string; /** Stamp files the branch changed as product whose only change since the reviewed head is a stamp on the branch's own rows, the base untouched, and without which the two product patches match (GST-980). */ @@ -253,7 +256,8 @@ export function planCarry(input: CarryPlanInput): CarryPlan { const named = input.changedProduct.slice(0, CARRY_NAMED_PATHS); return { ok: false, code: 'RECEIPT_CARRY_PRODUCT_CHANGED', message: `product diff changed between ${input.from.slice(0, 12)} and ${input.head.slice(0, 12)}: ${named.join(', ') || 'patch-id differs'}${input.changedProduct.length > named.length ? ` (+${input.changedProduct.length - named.length} more)` : ''}` }; } - const reasons: string[] = input.retargetedFrom ? [`stack parent ${input.retargetedFrom} landed`] : []; + const parent = input.retargetedFrom ?? input.stackParent?.slice(0, 12); + const reasons: string[] = parent ? [`stack parent ${parent} landed`] : []; if (input.mergeCommits > 0) reasons.push(input.mergeCommits === 1 ? 'base merge' : `${input.mergeCommits} base merges`); if (input.scopedReview && input.productPatchId.from !== input.productPatchId.head) reasons.push(input.scopedReview); if (input.stampedRows?.length && input.productPatchId.from !== input.productPatchId.head) reasons.push(`a stamp on the branch's own rows (${input.stampedRows.join(', ')})`); @@ -271,6 +275,27 @@ export function planCarry(input: CarryPlanInput): CarryPlan { }; } +/** How many commits of the reviewed range landedStackParent tries, newest first. */ +export const STACK_PARENT_WALK = 100; + +/** + * The newest commit inside the reviewed range (rangeBase..reviewed, the reviewed head itself excluded) whose product + * change the current base already holds: merging it into currentBase changes no product path. That is a stack parent + * that squash-merged (or merged) into the base after the receipt was written against the base (GST-1199). Null when no + * commit within STACK_PARENT_WALK qualifies or git cannot tell; a conflicted product path is never held. + */ +export function landedStackParent(repoRoot: string, s: { rangeBase: string; reviewed: string; currentBase: string }, product: readonly string[]): string | null { + const run = (args: string[]) => spawnSync('git', args, { cwd: repoRoot, encoding: 'utf8', timeout: 120_000, maxBuffer: 64 * 1024 * 1024 }); + const commits = run(['rev-list', '--topo-order', `--max-count=${STACK_PARENT_WALK + 1}`, `${s.rangeBase}..${s.reviewed}`]); + if (commits.status !== 0) return null; + for (const commit of commits.stdout.split('\n').filter(c => c && c !== s.reviewed)) { + const merged = run(['merge-tree', '--write-tree', '--no-messages', s.currentBase, commit]); + const tree = merged.status === 0 || merged.status === 1 ? merged.stdout.split('\n')[0]?.trim() : undefined; + if (tree && run(['diff', '--quiet', s.currentBase, tree, '--', ...product]).status === 0) return commit; + } + return null; +} + export interface TreeCarryInput { from: string; head: string; /** HEAD^{tree}, and every tree the receipt already covers (its head_tree plus earlier carries' trees). */ diff --git a/test/ship-receipt-carry-squash.test.ts b/test/ship-receipt-carry-squash.test.ts index 2c763e7b3..5cd0f6af8 100644 --- a/test/ship-receipt-carry-squash.test.ts +++ b/test/ship-receipt-carry-squash.test.ts @@ -21,7 +21,7 @@ const SUB = "exports.add = (a, b) => a + b;\nexports.sub = (a, b) => a - b;\n"; const RETRY4 = "exports.MAX_RETRIES = 4;\nexports.TIMEOUT_MS = 5000;\nexports.retry = (fn) => fn();\n"; /** main <- parent (lib/util.js) <- feature (lib/retry.js, its /ship receipt written against the parent branch). */ -function stack(repo: FixtureRepo, parentFiles: Record = { 'lib/util.js': SUB }, childFiles: Record = { 'lib/retry.js': RETRY4 }): { parent: string; child: string; body: string } { +function stack(repo: FixtureRepo, parentFiles: Record = { 'lib/util.js': SUB }, childFiles: Record = { 'lib/retry.js': RETRY4 }, base = 'parent'): { parent: string; child: string; body: string } { repo.git('checkout', '-q', '-b', 'parent'); const parent = commit(repo, parentFiles, 'parent: sub'); repo.git('push', '-q', 'origin', 'parent'); @@ -29,7 +29,7 @@ function stack(repo: FixtureRepo, parentFiles: Record = { 'lib/u repo.git('reset', '-q', '--hard', parent); const child = commit(repo, childFiles, 'child: four retries'); const body = path.join(repo.root, 'body.md'); - const w = runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '8', '--base', 'parent', '--version', '1.0.0', '--session-kind', 'unattended', '--body', body]); + const w = runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '8', '--base', base, '--version', '1.0.0', '--session-kind', 'unattended', '--body', body]); expect(w.status).toBe(EXIT.ok); return { parent, child, body }; } @@ -166,3 +166,64 @@ describe('carry after a stack parent squash-merges (#3570)', () => { expect(receiptIn(body).head).toBe(child); }, 90_000); }); + +const CONFIG = ['a = 1', 'b = 2', 'c = 3', 'd = 4', 'e = 5', 'f = 6', 'g = 7', 'h = 8', ''].join('\n'); +const configWith = (edits: Record) => CONFIG.split('\n').map(l => edits[l] ?? l).join('\n'); + +describe('carry after a stack parent squash-merges, receipt written against main (GST-1199)', () => { + // Value: protects=a child PR stacked on its parent but targeting main keeps its receipt once the parent squash-merges and + // main is merged in (gstack-evals#178 faced a 45-60M-token gate re-review for it); fails_when=carry compares the child's + // whole range from the old fork point, parent hunks included, with HEAD's diff against a base that now holds them; + // seam=none; no_claim=gh pr edit (--body stands in) + test('the child carries when its range from the landed parent is HEAD\'s whole product diff against main', () => { + const repo = makeFixtureRepo(); repos.push(repo); + const { parent, child, body } = stack(repo, undefined, undefined, 'main'); + 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 ${child.slice(0, 12)} to ${head.slice(0, 12)}`); + const r = receiptIn(body); + expect(r.head).toBe(head); + expect(r.base).toBe('main'); + expect(r.carry!.reason).toBe(`stack parent ${parent.slice(0, 12)} landed + base merge`); + expect(bindShippedReceipt(repo.work, 'origin/main', fs.readFileSync(body, 'utf8'), '8', [head]).ok).toBe(true); + }, 90_000); + + // Value: protects=fail closed: a product edit after the receipt, or a parent that never landed and whose change the + // child's merge dropped, takes a fresh /ship; fails_when=any reviewed ancestor counts as a landed parent, or the edit + // hides behind the shorter range; seam=none; no_claim=- + test('a product edit after the receipt, or a dropped parent that never landed, refuses with RECEIPT_CARRY_PRODUCT_CHANGED', () => { + const repo = makeFixtureRepo(); repos.push(repo); + const { child, body } = stack(repo, undefined, undefined, 'main'); + landOnMain(repo, { 'lib/util.js': SUB }); + commit(repo, { 'lib/retry.js': RETRY4.replace('4', '5') }, 'child: five'); + const edited = carry(repo, body); + expect(edited.status).toBe(EXIT.refused); + expect(edited.out).toContain('CARRY: refused (RECEIPT_CARRY_PRODUCT_CHANGED)'); + expect(receiptIn(body).head).toBe(child); + + const other = makeFixtureRepo(); repos.push(other); + const s = stack(other, undefined, undefined, 'main'); + landOnMain(other, {}); + commit(other, { 'lib/util.js': other.git('show', 'origin/main:lib/util.js') + '\n' }, 'drop the parent change'); + const dropped = carry(other, s.body); + expect(dropped.status).toBe(EXIT.refused); + expect(dropped.out).toContain('CARRY: refused (RECEIPT_CARRY_PRODUCT_CHANGED)'); + expect(receiptIn(s.body).head).toBe(s.child); + }, 120_000); + + // Value: protects=a base change next to the child's reviewed hunks gets one fix-check reviewer, not a carry and not a full + // /ship; fails_when=the landed-parent comparison ignores base context or refuses it outright; seam=none; no_claim=- + test('a base change next to the child\'s reviewed hunks routes to RECEIPT_CARRY_SCOPED_REVIEW', () => { + const repo = makeFixtureRepo({ baseFiles: { 'lib/config.js': CONFIG } }); repos.push(repo); + const { child, body } = stack(repo, undefined, { 'lib/config.js': configWith({ 'd = 4': 'd = 40' }) }, 'main'); + landOnMain(repo, { 'lib/util.js': SUB, 'lib/config.js': configWith({ 'f = 6': 'f = 60' }) }); + const out = carry(repo, body); + expect(out.status).toBe(EXIT.refused); + expect(out.out).toContain('CARRY: refused (RECEIPT_CARRY_SCOPED_REVIEW)'); + expect(out.err).toContain('lib/config.js'); + expect(receiptIn(body).head).toBe(child); + }, 90_000); +}); From 1f1bd4d0c108d437b04ac86b6168ce1cba4648c2 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 07:20:51 +0000 Subject: [PATCH 2/9] fix(ship-receipt): a commit with no product change is no landed stack parent (review fix, GST-1199) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- lib/ship-receipt.ts | 4 +++- test/ship-receipt-carry-squash.test.ts | 15 ++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/lib/ship-receipt.ts b/lib/ship-receipt.ts index 5895d5bbb..bbb145358 100644 --- a/lib/ship-receipt.ts +++ b/lib/ship-receipt.ts @@ -281,7 +281,8 @@ export const STACK_PARENT_WALK = 100; /** * The newest commit inside the reviewed range (rangeBase..reviewed, the reviewed head itself excluded) whose product * change the current base already holds: merging it into currentBase changes no product path. That is a stack parent - * that squash-merged (or merged) into the base after the receipt was written against the base (GST-1199). Null when no + * that squash-merged (or merged) into the base after the receipt was written against the base (GST-1199). A commit with no + * product change of its own (a test-first commit) is no parent, since any base trivially holds it. Null when no * commit within STACK_PARENT_WALK qualifies or git cannot tell; a conflicted product path is never held. */ export function landedStackParent(repoRoot: string, s: { rangeBase: string; reviewed: string; currentBase: string }, product: readonly string[]): string | null { @@ -289,6 +290,7 @@ export function landedStackParent(repoRoot: string, s: { rangeBase: string; revi const commits = run(['rev-list', '--topo-order', `--max-count=${STACK_PARENT_WALK + 1}`, `${s.rangeBase}..${s.reviewed}`]); if (commits.status !== 0) return null; for (const commit of commits.stdout.split('\n').filter(c => c && c !== s.reviewed)) { + if (run(['diff', '--quiet', s.rangeBase, commit, '--', ...product]).status !== 1) continue; const merged = run(['merge-tree', '--write-tree', '--no-messages', s.currentBase, commit]); const tree = merged.status === 0 || merged.status === 1 ? merged.stdout.split('\n')[0]?.trim() : undefined; if (tree && run(['diff', '--quiet', s.currentBase, tree, '--', ...product]).status === 0) return commit; diff --git a/test/ship-receipt-carry-squash.test.ts b/test/ship-receipt-carry-squash.test.ts index 5cd0f6af8..a47e025c1 100644 --- a/test/ship-receipt-carry-squash.test.ts +++ b/test/ship-receipt-carry-squash.test.ts @@ -8,7 +8,7 @@ import { afterEach, describe, expect, test } from 'bun:test'; import * as fs from 'node:fs'; import * as path from 'node:path'; import { EXIT } from '../lib/headless-artifacts'; -import { extractReceipt, renderReceiptBlock } from '../lib/ship-receipt'; +import { extractReceipt, landedStackParent, renderReceiptBlock } from '../lib/ship-receipt'; import { bindShippedReceipt } from '../lib/ship-receipt-depth'; import { cleanup, commit, makeFixtureRepo, runBin, write, type FixtureRepo } from './helpers/pregate-fixture'; @@ -226,4 +226,17 @@ describe('carry after a stack parent squash-merges, receipt written against main expect(out.err).toContain('lib/config.js'); expect(receiptIn(body).head).toBe(child); }, 90_000); + + // Value: protects=the receipt never names a stack parent that does not exist; fails_when=a test-first commit with no + // product change counts as landed (any base holds it), so the reason claims a parent and the test range shrinks; seam=none; + // no_claim=- + test('a test-only commit in a branch that was never stacked is no landed parent', () => { + const repo = makeFixtureRepo({ baseFiles: { 'lib/config.js': CONFIG } }); repos.push(repo); + const fork = repo.git('rev-parse', 'HEAD'); + commit(repo, { 'test/config.test.js': "const test = require('node:test');\ntest('d', () => {});\n" }, 'test first'); + const reviewed = commit(repo, { 'lib/config.js': configWith({ 'd = 4': 'd = 40' }) }, 'fix d'); + repo.git('checkout', '-q', 'main'); + const mainTip = commit(repo, { 'docs/README.md': '# main moves on\n' }, 'main moves on'); + expect(landedStackParent(repo.work, { rangeBase: fork, reviewed, currentBase: mainTip }, ['.', ':(exclude,glob)test/**'])).toBeNull(); + }, 60_000); }); From 24ace27de0d10b2cae6d2726b1ca6728a405301f Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 07:31:22 +0000 Subject: [PATCH 3/9] changelog: name the fragment by the branch slug (GST-1199) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- ...9-carry-landed-stack-parent.md => capy-carry-a-stacked-prs.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{gst-1199-carry-landed-stack-parent.md => capy-carry-a-stacked-prs.md} (100%) diff --git a/changelog.d/gst-1199-carry-landed-stack-parent.md b/changelog.d/capy-carry-a-stacked-prs.md similarity index 100% rename from changelog.d/gst-1199-carry-landed-stack-parent.md rename to changelog.d/capy-carry-a-stacked-prs.md From 8d183032123fb7bc4903672dac325cf89584e4e2 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 07:32:55 +0000 Subject: [PATCH 4/9] docs: troubleshooting names the base-written stacked carry (Step 14.5 audit, GST-1199) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- docs/troubleshooting.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 7ae72bf86..f220a5ea7 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -5981,7 +5981,10 @@ the reviewed tree but the base moved under it, so the diff a reviewer would see is not the reviewed one. For a stacked PR retargeted to the base after its parent landed, it means HEAD's product diff against the base is not the range reviewed against the parent branch: the parent has not fully landed, or the -branch changed since. The reviewed verdict does not cover HEAD, so the +branch changed since. A stacked PR whose receipt was written against the base +carries from the newest reviewed commit the base already holds +([ship-policy.md](ship-policy.md#receipt-carry)); it refuses here only when no +such commit exists or the branch changed since. The reviewed verdict does not cover HEAD, so the receipt is not carried. The refusal names the changed product paths, or for a retargeted stacked PR the parent branch and the base. From 0b36079b2a512bce361d92714b3174e10cd25282 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 09:29:43 +0000 Subject: [PATCH 5/9] test: failing-first carry cases for stamp-file reverts and ported harness pins (GST-1210) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- ...-receipt-carry-fragment-conversion.test.ts | 100 ++++++++++++++++++ test/ship-receipt-carry-ported-pin.test.ts | 78 ++++++++++++++ 2 files changed, 178 insertions(+) create mode 100644 test/ship-receipt-carry-fragment-conversion.test.ts create mode 100644 test/ship-receipt-carry-ported-pin.test.ts diff --git a/test/ship-receipt-carry-fragment-conversion.test.ts b/test/ship-receipt-carry-fragment-conversion.test.ts new file mode 100644 index 000000000..5a0ff6f55 --- /dev/null +++ b/test/ship-receipt-carry-fragment-conversion.test.ts @@ -0,0 +1,100 @@ +/** + * gstack-ship-receipt carry across a changelog-fragment conversion (GST-1210, HARN-1 harness-evals#206): a PR reviewed + * with its own version stamp (VERSION, package.json, CHANGELOG) moves to a changelog.d fragment after the base adopts + * fragments, which puts every stamp file back at the base's content. A stamp file the branch no longer changes against + * the base, or changes only in its stamp fields, holds no unreviewed change, so the receipt carries; a non-stamp change + * in package.json or a logic edit next to the reviewed one still refuses. + */ +import { afterEach, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { EXIT } from '../lib/headless-artifacts'; +import { extractReceipt } from '../lib/ship-receipt'; +import { cleanup, commit, makeFixtureRepo, runBin, write, type FixtureRepo } from './helpers/pregate-fixture'; + +const repos: FixtureRepo[] = []; +afterEach(() => { while (repos.length) cleanup(repos.pop()!); }); +const receiptIn = (body: string) => extractReceipt(fs.readFileSync(body, 'utf8')).receipt!; +const carry = (repo: FixtureRepo, body: string) => runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main']); +const pkg = (version: string, scripts: Record = {}) => JSON.stringify({ name: 'fixture', version, scripts: { test: 'node --test test/*.test.js', generate: 'node gen.js', ...scripts } }, null, 2) + '\n'; +const POLICY = (fragments: boolean) => JSON.stringify({ min_gstack_version: '1.91.340.0', stamp_files: ['VERSION', 'package.json', 'CHANGELOG.md'], ...(fragments ? { changelog_fragments: { dir: 'changelog.d' } } : {}) }, null, 2) + '\n'; +const RETRY4 = "exports.MAX_RETRIES = 4;\nexports.TIMEOUT_MS = 5000;\nexports.retry = (fn) => fn();\n"; + +/** + * main at 1.0.0 <- feature: four retries plus its own 1.0.1 stamp, /ship receipt written there. main then ships 1.0.3, + * adopts changelog fragments and (`mainScripts`) changes package.json beyond its version. The branch merges main and + * converts: `converted` is the package.json it keeps (main's by default), plus a fragment and any `extra` files. + */ +function converted(repo: FixtureRepo, opts: { mainScripts?: Record; keep?: (main: string) => string; extra?: Record } = {}): { reviewed: string; body: string } { + repo.git('checkout', '-q', 'main'); + commit(repo, { VERSION: '1.0.0\n', 'CHANGELOG.md': '# Changelog\n\n## 1.0.0\n- base\n', '.gstack/ship-policy.json': POLICY(false) }, 'main: stamp files'); + repo.git('push', '-q', 'origin', 'main'); + repo.git('checkout', '-q', 'feature'); + repo.git('reset', '-q', '--hard', 'origin/main'); + const reviewed = commit(repo, { 'lib/retry.js': RETRY4, VERSION: '1.0.1\n', 'package.json': pkg('1.0.1'), 'CHANGELOG.md': '# Changelog\n\n## 1.0.1\n- four retries\n\n## 1.0.0\n- base\n' }, 'feat: four retries (v1.0.1)'); + const body = path.join(repo.root, 'body.md'); + const w = runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '206', '--base', 'main', '--version', '1.0.1', '--session-kind', 'unattended', '--body', body]); + expect(w.status).toBe(EXIT.ok); + repo.git('checkout', '-q', 'main'); + const mainPkg = pkg('1.0.3', opts.mainScripts); + commit(repo, { VERSION: '1.0.3\n', 'package.json': mainPkg, 'CHANGELOG.md': '# Changelog\n\n## 1.0.3\n- other PRs\n\n## 1.0.0\n- base\n', '.gstack/ship-policy.json': POLICY(true), 'changelog.d/README.md': '# fragments\n' }, 'main: v1.0.3, changelog fragments'); + repo.git('push', '-q', 'origin', 'main'); + repo.git('checkout', '-q', 'feature'); + try { repo.git('merge', '-q', '--no-edit', 'origin/main'); } catch { /* the stamp files conflict; the conversion resolves them below */ } + repo.git('checkout', 'origin/main', '--', 'VERSION', 'CHANGELOG.md', 'package.json'); + write(repo.work, 'package.json', (opts.keep ?? (m => m))(mainPkg)); + write(repo.work, 'changelog.d/four-retries.md', '- four retries\n'); + for (const [rel, text] of Object.entries(opts.extra ?? {})) write(repo.work, rel, text); + repo.git('add', '-A'); + repo.git('commit', '-q', '--no-edit', '-m', 'convert to a changelog fragment'); + return { reviewed, body }; +} + +describe('carry across a changelog-fragment conversion (GST-1210)', () => { + // Value: protects=a PR converted to a changelog fragment after review keeps its receipt when the base also changed + // package.json beyond its version (HARN-1 harness-evals#206 paid a fresh /ship); fails_when=carry reads the base's own + // package.json change as the branch's product change; seam=none; no_claim=gh pr edit (--body stands in) + test('a stamp file back at the base\'s content carries as release metadata', () => { + const repo = makeFixtureRepo(); repos.push(repo); + const { reviewed, body } = converted(repo, { mainScripts: { lint: 'node lint.js' } }); + const head = repo.git('rev-parse', 'HEAD'); + const out = carry(repo, body); + expect(out.err).not.toContain('RECEIPT_CARRY'); + 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).toContain('release metadata'); + }, 90_000); + + // Value: protects=the converted PR whose package.json is the base's except its version field carries as stamp-only; + // fails_when=the version difference against the base reads as a product change; seam=none; no_claim=gh pr edit + test('a package.json equal to the base\'s except the version field carries', () => { + const repo = makeFixtureRepo(); repos.push(repo); + const { body } = converted(repo, { mainScripts: { lint: 'node lint.js' }, keep: m => m.replace('"version": "1.0.3"', '"version": "1.0.4"') }); + const out = carry(repo, body); + expect(out.err).not.toContain('RECEIPT_CARRY'); + expect(out.status).toBe(EXIT.ok); + expect(receiptIn(body).carry!.reason).toContain('release metadata'); + }, 90_000); + + // Value: protects=fail closed on a package.json field the branch changed beyond the stamp (a new script runs code); + // fails_when=a stamp file that is not the base's content is read as stamp-only; seam=none; no_claim=gh pr edit + test('a non-stamp field the branch adds to package.json still refuses', () => { + const repo = makeFixtureRepo(); repos.push(repo); + const { body } = converted(repo, { mainScripts: { lint: 'node lint.js' }, keep: m => m.replace('"lint": "node lint.js"', '"lint": "node lint.js",\n "postinstall": "node evil.js"') }); + const out = carry(repo, body); + expect(out.status).toBe(EXIT.refused); + expect(out.err).toContain('RECEIPT_CARRY_PRODUCT_CHANGED'); + expect(out.err).toContain('package.json'); + }, 90_000); + + // Value: protects=fail closed on a logic edit beside the reviewed one riding the conversion; fails_when=the stamp-file + // rule widens to the whole diff; seam=none; no_claim=gh pr edit + test('a logic edit next to the reviewed constant still refuses', () => { + const repo = makeFixtureRepo(); repos.push(repo); + const { body } = converted(repo, { mainScripts: { lint: 'node lint.js' }, extra: { 'lib/retry.js': RETRY4.replace('TIMEOUT_MS = 5000', 'TIMEOUT_MS = 50') } }); + const out = carry(repo, body); + expect(out.status).toBe(EXIT.refused); + expect(out.err).toContain('RECEIPT_CARRY_PRODUCT_CHANGED'); + expect(out.err).toContain('lib/retry.js'); + }, 90_000); +}); diff --git a/test/ship-receipt-carry-ported-pin.test.ts b/test/ship-receipt-carry-ported-pin.test.ts new file mode 100644 index 000000000..71b8f55ad --- /dev/null +++ b/test/ship-receipt-carry-ported-pin.test.ts @@ -0,0 +1,78 @@ +/** + * gstack-ship-receipt carry across a base merge that ports one harness-pin entry (GST-1210, GSTA-202 #3903 at f0613db8): + * the branch splits the base's JSON map of pins into one file per entry, and a later merge from the base copies the base's + * edit of one entry into its pin file, byte for byte. That file holds the base's change and nothing of the branch's, so + * the receipt carries; a pin that differs from the base's entry, or lands in another entry's file, still refuses. + */ +import { afterEach, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { EXIT } from '../lib/headless-artifacts'; +import { extractReceipt } from '../lib/ship-receipt'; +import { cleanup, commit, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/pregate-fixture'; + +const repos: FixtureRepo[] = []; +afterEach(() => { while (repos.length) cleanup(repos.pop()!); }); +const receiptIn = (body: string) => extractReceipt(fs.readFileSync(body, 'utf8')).receipt!; +const carry = (repo: FixtureRepo, body: string) => runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main']); +const RETRY = '{"blob":"aaa","since":6,"nonBehavioral":"first pin"}'; +const UTIL = '{"blob":"bbb","since":6}'; +const RETRY_MAIN = '{"blob":"ccc","since":6,"nonBehavioral":"main repinned retry"}'; +const manifest = (retry: string) => `{\n "version": 6,\n "files": {\n "lib/retry.js": ${retry},\n "lib/util.js": ${UTIL}\n }\n}\n`; +const PIN = 'scripts/harness-pins/lib/retry.js.json'; + +/** main holds the pin map; feature splits it into one file per entry beside a product change, /ship receipt there; main repins retry; feature merges main and writes `ports`. */ +function ported(repo: FixtureRepo, ports: Record): { reviewed: string; body: string } { + repo.git('checkout', '-q', 'main'); + commit(repo, { 'scripts/harness-version.json': manifest(RETRY) }, 'main: harness pins'); + repo.git('push', '-q', 'origin', 'main'); + repo.git('checkout', '-q', 'feature'); + repo.git('reset', '-q', '--hard', 'origin/main'); + const reviewed = commit(repo, { 'lib/retry.js': "exports.MAX_RETRIES = 4;\nexports.TIMEOUT_MS = 5000;\nexports.retry = (fn) => fn();\n", [PIN]: `${RETRY}\n`, 'scripts/harness-pins/lib/util.js.json': `${UTIL}\n` }, 'feat: one file per harness pin'); + const body = path.join(repo.root, 'body.md'); + expect(runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '3903', '--base', 'main', '--version', '1.0.0', '--session-kind', 'unattended', '--body', body]).status).toBe(EXIT.ok); + repo.git('checkout', '-q', 'main'); + commit(repo, { 'scripts/harness-version.json': manifest(RETRY_MAIN), 'docs/README.md': '# docs after the repin\n' }, 'main: repin retry'); + repo.git('push', '-q', 'origin', 'main'); + repo.git('checkout', '-q', 'feature'); + repo.git('merge', '-q', '--no-edit', 'origin/main'); + commit(repo, ports, 'port the base repin into its pin file'); + return { reviewed, body }; +} + +describe('carry across a ported harness-pin entry (GST-1210)', () => { + // Value: protects=a branch that splits harness pins into one file per entry keeps its receipt when a base merge ports + // the base's repin byte for byte (#3903 at f0613db8 was refused); fails_when=carry reads the ported pin as the branch's + // product change; seam=none; no_claim=gh pr edit (--body stands in) + test('a pin file equal to the base\'s entry byte for byte carries', () => { + const repo = makeFixtureRepo(); repos.push(repo); + const { reviewed, body } = ported(repo, { [PIN]: `${RETRY_MAIN}\n` }); + const head = repo.git('rev-parse', 'HEAD'); + const out = carry(repo, body); + expect(out.err).not.toContain('RECEIPT_CARRY'); + 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).toContain(`base entries ported byte for byte (${PIN})`); + }, 90_000); + + // Value: protects=fail closed on a pin the branch changed beyond the base's entry (an unreviewed repin); + // fails_when=any pin file change carries; seam=none; no_claim=gh pr edit + test('a pin entry that differs from the base\'s still refuses', () => { + const repo = makeFixtureRepo(); repos.push(repo); + const { body } = ported(repo, { [PIN]: `${RETRY_MAIN.replace('ccc', 'ddd')}\n` }); + const out = carry(repo, body); + expect(out.status).toBe(EXIT.refused); + expect(out.err).toContain('RECEIPT_CARRY_PRODUCT_CHANGED'); + expect(out.err).toContain(PIN); + }, 90_000); + + // Value: protects=fail closed when the base's entry lands in another entry's pin file; fails_when=the port matches the + // value without its key; seam=none; no_claim=gh pr edit + test('the base\'s entry written into another entry\'s pin file still refuses', () => { + const repo = makeFixtureRepo(); repos.push(repo); + const { body } = ported(repo, { [PIN]: `${RETRY_MAIN}\n`, 'scripts/harness-pins/lib/util.js.json': `${RETRY_MAIN}\n` }); + const out = carry(repo, body); + expect(out.status).toBe(EXIT.refused); + expect(out.err).toContain('scripts/harness-pins/lib/util.js.json'); + }, 90_000); +}); From 3e12281871670ead62495007bdfa23864b7eea30 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 09:29:43 +0000 Subject: [PATCH 6/9] fix(ship-receipt): carry a stamp file back at the base and a byte-for-byte ported pin entry (GST-1210) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-ship-receipt | 16 ++++++---- .../capy-gst1210-carry-stamp-reverts.md | 1 + docs/ship-policy.md | 2 ++ docs/troubleshooting.md | 5 +++- lib/ship-patch.ts | 30 +++++++++++++++++++ lib/ship-receipt.ts | 5 +++- 6 files changed, 52 insertions(+), 7 deletions(-) create mode 100644 changelog.d/capy-gst1210-carry-stamp-reverts.md diff --git a/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index 6d4e6c686..5d6353be8 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -48,7 +48,7 @@ import { receiptIssuesFrom, receiptSummaryLine, renderCensus, renderReceiptBlock, validateReceipt, type CensusPr, } from '../lib/ship-receipt'; import { checkDocsCaps, docsCapLine, docsExceptionLine, parseReceiptDocs } from '../lib/ship-receipt-docs'; -import { comparePath, contextPacket, PACKET_SECTION_CAP, readScopedReview, touchedPaths } from '../lib/ship-patch'; +import { comparePath, contextPacket, PACKET_SECTION_CAP, portedEntry, readScopedReview, touchedPaths } from '../lib/ship-patch'; import { deeper, depthBelow, exceptionProblems, parseReviewLoopException, parseReviewTargeted, receiptExclude, receiptReviewAtHead, receiptTargetedAtHead, reviewLoopExceptionLine, targetedClassesSince } from '../lib/ship-receipt-depth'; import { checkReceiptProse, proseLine } from '../lib/ship-receipt-prose'; import { resolveStateRoot } from '../lib/state-root'; @@ -535,7 +535,11 @@ function productSpec(repoRoot: string, rules: ProductRules, from: string, to: st const version = (ref: string) => { if (!versions.has(ref)) versions.set(ref, versionAt(ref)); return versions.get(ref)!; }; const stamped = (p: string) => matchesAny(rules.metadata, p) && !matchesAny(rules.extra.outputs, p); const classify = (p: string, a: string, b: string) => classifyCarryPath(p, rules.extra, stamped(p) ? { before: blobAt(a, p), after: blobAt(b, p), oldVersion: version(a), newVersion: version(b), templates: rules.templates, side: a === from && b === to ? branch?.own(p) : undefined } : undefined); - const changed = changedRaw.split('\n').filter(Boolean).map(p => ({ path: p, class: classify(p, from, to) })); + // A stamp file the branch changed on neither side beyond a stamp against that side's base (back at the base's content + // after a changelog-fragment conversion, or only its version moved) holds none of the branch's product: whatever else + // differs between the two heads came from the base (GST-1210). + const ownStampOnly = (p: string) => !!branch?.touched.has(p) && stamped(p) && branch.sides.every(([a, b]) => blobAt(a, p) === blobAt(b, p) || classify(p, a, b) === 'metadata'); + const changed = changedRaw.split('\n').filter(Boolean).map(p => ({ path: p, class: ownStampOnly(p) ? 'metadata' as const : classify(p, from, to) })); const excludeGlobs = (globs: readonly string[]) => globs.map(g => `:(exclude,glob)${g}`); const demoted = !branch ? changed.filter(c => c.class === 'product' && matchesAny(rules.metadata, c.path)).map(c => c.path) : [...branch.touched].filter(p => stamped(p) && (changed.some(c => c.path === p && c.class === 'product') || branch.sides.some(([a, b]) => blobAt(a, p) !== blobAt(b, p) && classify(p, a, b) === 'product'))); @@ -695,6 +699,7 @@ function cmdCarry(rest: string[], json: boolean): number { const changedProduct: string[] = []; const contextPaths: string[] = []; const stampOnly: string[] = []; + const ported: string[] = []; if (retarget && reviewed !== current) changedProduct.push(`HEAD's product diff against ${base} is not the one reviewed against ${reviewedBase} (the parent has not fully landed on ${base}, or the branch changed since)`); else if (productFrom !== productHead && !retarget) { // planCarry names at most CARRY_NAMED_PATHS differing paths, so the walk stops there, and it reads only the paths the @@ -714,10 +719,11 @@ function cmdCarry(rest: string[], json: boolean): number { if (cls === 'metadata' && spec.ownRows.includes(p) && !movedBase.has(p)) stampOnly.push(p); if (cls !== 'product') continue; const verdict = comparePath(repoRoot, sides, p); - if (verdict === 'changed') changedProduct.push(p); + if (verdict === 'changed' && portedEntry(repoRoot, sides, p, movedBase)) ported.push(p); + else if (verdict === 'changed') changedProduct.push(p); else if (verdict === 'context') contextPaths.push(p); } - const left = [...contextPaths, ...stampOnly]; + const left = [...contextPaths, ...stampOnly, ...ported]; if (!changedProduct.length && !left.length) changedProduct.push('(a product path outside the branch\'s touched files; compare the two base diffs)'); else if (!changedProduct.length) { const rest = [...spec.product, ...left.map(p => `:(exclude,literal)${p}`)]; @@ -756,7 +762,7 @@ function cmdCarry(rest: string[], json: boolean): number { // Evidence for the record, never the decision: a slow range-diff is reported unavailable rather than waited for. const rangeRaw = spawnSync('git', ['range-diff', '--no-color', `${rangeBase}..${from}`, `${headBase}..${head}`], { cwd: repoRoot, encoding: 'utf8', timeout: 15_000, maxBuffer: 64 * 1024 * 1024 }); const rangeDiff = rangeRaw.status === 0 ? (rangeRaw.stdout.trim().split('\n').slice(0, 20).join('\n') || '(empty: identical ranges)') : `unavailable: ${(rangeRaw.stderr ?? '').trim().split('\n')[0] || (rangeRaw.signal ? `range-diff killed by ${rangeRaw.signal} after 15 s` : 'range-diff failed')}`; - plan = planCarry({ from, head, changed, productPatchId: { from: reviewed as string, head: current as string }, testPatchId: { from: testFrom, head: testHead }, changedProduct, mergeCommits: Number(mergeCommits) || 0, rangeDiff, ...(retarget ? { retargetedFrom: reviewedBase } : {}), ...(stackParent ? { stackParent } : {}), ...(scopedReview ? { scopedReview } : {}), ...(stampedRows.length ? { stampedRows } : {}) }); + plan = planCarry({ from, head, changed, productPatchId: { from: reviewed as string, head: current as string }, testPatchId: { from: testFrom, head: testHead }, changedProduct, mergeCommits: Number(mergeCommits) || 0, rangeDiff, ...(retarget ? { retargetedFrom: reviewedBase } : {}), ...(stackParent ? { stackParent } : {}), ...(scopedReview ? { scopedReview } : {}), ...(stampedRows.length ? { stampedRows } : {}), ...(!changedProduct.length && ported.length ? { portedEntries: ported } : {}) }); } if (!plan.ok) return refused(plan.code, plan.message); // Eligibility is not publication: a receipt that recorded a pregate is rebound only with a pregate current for HEAD, diff --git a/changelog.d/capy-gst1210-carry-stamp-reverts.md b/changelog.d/capy-gst1210-carry-stamp-reverts.md new file mode 100644 index 000000000..c24dcd56b --- /dev/null +++ b/changelog.d/capy-gst1210-carry-stamp-reverts.md @@ -0,0 +1 @@ +- **A /ship receipt now survives two edits that change no reviewed code (GST-1210).** `gstack-ship-receipt carry` used to refuse with `RECEIPT_CARRY_PRODUCT_CHANGED` when a PR moved to a changelog fragment after review, putting `package.json` and the other stamp files back at main's content, if main had also changed `package.json` beyond its version (HARN-1, harness-evals#206). A stamp file the branch changes by no more than a stamp against each head's base now carries as release metadata. It also refused when a merge from main copied main's repin of one harness-pin entry, byte for byte, into the branch's one-file-per-entry pin file (#3903). That now carries as `base entries ported byte for byte`. A non-stamp `package.json` change, a logic edit, or a pin that differs from main's entry still refuses. diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 004766f17..cbf200b9d 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1505,6 +1505,8 @@ receipt when the base moves: | a stacked PR whose parent landed (squash or merge), retargeted to the base with the base merged in, HEAD's product diff exactly the one reviewed against the parent branch | carries (reason `stack parent landed`) | | a stacked PR whose receipt was written against the base, after its parent landed and the base was merged in, HEAD's product diff exactly the reviewed range from the parent's head | carries (reason `stack parent landed`); base context next to the child's hunks is `RECEIPT_CARRY_SCOPED_REVIEW` | | a stamp on the branch's own rows in a stamp file the branch also changed as product: every changed line at a branch-added position, the file's mode unchanged, the base not touching it, the rest of the product diff unchanged | carries (reason `a stamp on the branch's own rows ()`) | +| a stamp file the branch changes, against each head's base, by no more than a stamp (back at the base's content after a changelog-fragment conversion, or only its version field moved), whatever else the base changed in it | carries (reason `release metadata`, GST-1210) | +| a one-line `*.json` pin file the branch added, named `…/.json`, whose change since the reviewed head is byte for byte the base's own edit of entry `` in a JSON map line of a base file (a merge that ports a base repin into a one-file-per-entry split) | carries (reason `base entries ported byte for byte ()`, GST-1210); a pin that differs from the base's entry, or sits in another entry's file, refuses | **Stacked PRs (#3570).** A child PR's receipt is written against its parent branch, so its `base` names that branch and its `product_patch_id` is the diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index f220a5ea7..882aea8c4 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -5994,7 +5994,10 @@ output the pregate's regen check proved, a rebase onto a covered tree with the recorded product diff, a stacked PR's retarget once its parent fully landed on the base, a stamp on the branch's own rows in a stamp file it also changed as product (every changed line at a branch-added position, the mode -unchanged, the base not touching the file), or base context next to unchanged +unchanged, the base not touching the file), a stamp file the branch changes by +no more than a stamp against each head's base (a changelog-fragment conversion +that puts `package.json` back at the base's content), a one-entry pin file +whose change is byte for byte the base's edit of that entry, or base context next to unchanged hunks after a clean scoped review ([`RECEIPT_CARRY_SCOPED_REVIEW`](#receipt-carry-scoped-review)), which the `carry` object in the carried receipt records with the product patch-id and the range-diff evidence ([unattended.md](unattended.md#the-terminal-line)). diff --git a/lib/ship-patch.ts b/lib/ship-patch.ts index d069dc5cc..36acfa7a2 100644 --- a/lib/ship-patch.ts +++ b/lib/ship-patch.ts @@ -120,6 +120,36 @@ export function comparePath(repoRoot: string, s: PatchSides, rel: string): PathV return ids[0] === ids[1] ? 'same' : 'context'; } +/** + * The base's JSON map file a one-entry pin file ports (GST-1210, #3903 at f0613db8): `rel` is a one-line `*.json` file the + * branch added (absent at both bases, a regular file at both heads) named `…/.json`, whose whole text at each head is + * byte for byte the value of entry `` in one JSON map line of a base file `baseMoved` lists: the old value at the + * reviewed base, the new one at the current base. The branch then holds the base's own edit of that entry and nothing of + * its own. Null for anything else, so a pin that differs from the base's entry stays a product change. + */ +export function portedEntry(repoRoot: string, s: PatchSides, rel: string, baseMoved: Iterable): string | null { + if (!rel.endsWith('.json')) return null; + const blob = (ref: string, p: string) => git(repoRoot, ['show', `${ref}:${p}`]); + const regular = (ref: string) => /^100644 blob /.test(git(repoRoot, ['ls-tree', ref, '--', rel]) ?? ''); + const value = (text: string | null) => (text !== null && /^[^\n]+\n$/.test(text) ? text.slice(0, -1) : null); + const before = value(blob(s.reviewed, rel)), after = value(blob(s.current, rel)); + if (before === null || after === null || before === after || !regular(s.reviewed) || !regular(s.current)) return null; + if (blob(s.reviewedBase, rel) !== null || blob(s.currentBase, rel) !== null) return null; + const entries = (text: string | null) => new Set((text ?? '').split('\n').map(l => l.trim().replace(/,$/, ''))); + for (const file of baseMoved) { + if (file === rel || !file.endsWith('.json')) continue; + const now = entries(blob(s.currentBase, file)); + for (const line of now) { + const m = /^("(?:[^"\\]|\\.)*"): (.*)$/.exec(line); + if (!m || m[2] !== after) continue; + let key: unknown; + try { key = JSON.parse(m[1]!); } catch { continue; } + if (typeof key === 'string' && key && rel.endsWith(`/${key}.json`) && entries(blob(s.reviewedBase, file)).has(`${m[1]}: ${before}`)) return file; + } + } + return null; +} + function hunkTexts(diff: string): string[] { return diff.split(/^(?=@@ )/m).slice(1).map(h => h.replace(/^@@ [^@]*@@/, '@@')); } diff --git a/lib/ship-receipt.ts b/lib/ship-receipt.ts index bbb145358..3130cbea8 100644 --- a/lib/ship-receipt.ts +++ b/lib/ship-receipt.ts @@ -184,6 +184,8 @@ export interface CarryPlanInput { scopedReview?: string; /** Stamp files the branch changed as product whose only change since the reviewed head is a stamp on the branch's own rows, the base untouched, and without which the two product patches match (GST-980). */ stampedRows?: string[]; + /** Branch-added one-entry pin files whose change since the reviewed head is, byte for byte, the base's own edit of that entry in a base JSON map (`portedEntry`, GST-1210). */ + portedEntries?: string[]; } export interface CarryRecord { from: string; reason: string; product_patch_id: string; changed_paths: string[]; merge_commits: number; range_diff: string; covers: string[]; reruns: string[]; trees?: string[]; ancestry?: 'linear' | 'rewritten' } export type CarryPlan = { ok: true; carry: CarryRecord } | { ok: false; code: 'RECEIPT_CARRY_PRODUCT_CHANGED' | 'RECEIPT_CARRY_REFUSED' | 'RECEIPT_CARRY_TREE_MISMATCH'; message: string }; @@ -252,7 +254,7 @@ export function classifyCarryPath(rel: string, extra: { metadata?: readonly stri * branch's own rows in a stamp file the branch also changed as product (`stampedRows`, GST-980). */ export function planCarry(input: CarryPlanInput): CarryPlan { - if (input.productPatchId.from !== input.productPatchId.head && (input.changedProduct.length || (!input.scopedReview && !input.stampedRows?.length))) { + if (input.productPatchId.from !== input.productPatchId.head && (input.changedProduct.length || (!input.scopedReview && !input.stampedRows?.length && !input.portedEntries?.length))) { const named = input.changedProduct.slice(0, CARRY_NAMED_PATHS); return { ok: false, code: 'RECEIPT_CARRY_PRODUCT_CHANGED', message: `product diff changed between ${input.from.slice(0, 12)} and ${input.head.slice(0, 12)}: ${named.join(', ') || 'patch-id differs'}${input.changedProduct.length > named.length ? ` (+${input.changedProduct.length - named.length} more)` : ''}` }; } @@ -261,6 +263,7 @@ export function planCarry(input: CarryPlanInput): CarryPlan { if (input.mergeCommits > 0) reasons.push(input.mergeCommits === 1 ? 'base merge' : `${input.mergeCommits} base merges`); if (input.scopedReview && input.productPatchId.from !== input.productPatchId.head) reasons.push(input.scopedReview); if (input.stampedRows?.length && input.productPatchId.from !== input.productPatchId.head) reasons.push(`a stamp on the branch's own rows (${input.stampedRows.join(', ')})`); + if (input.portedEntries?.length && input.productPatchId.from !== input.productPatchId.head) reasons.push(`base entries ported byte for byte (${input.portedEntries.join(', ')})`); if (input.testPatchId.from !== input.testPatchId.head) reasons.push('test-only change'); if (input.changed.some(c => c.class === 'metadata')) reasons.push('release metadata'); if (input.changed.some(c => c.class === 'generated')) reasons.push('regenerated outputs'); From c0e5fc620fdfaf7a6066d87bf3705b57c215045b Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 09:45:26 +0000 Subject: [PATCH 7/9] fix(ship-receipt): review fixes: a pin needs exactly one naming map key; stamp-file sides compare object ids (GST-1210) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-ship-receipt | 2 +- lib/ship-patch.ts | 15 ++++++++------- test/ship-receipt-carry-ported-pin.test.ts | 18 ++++++++++++++---- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index 5d6353be8..c3e2460ca 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -538,7 +538,7 @@ function productSpec(repoRoot: string, rules: ProductRules, from: string, to: st // A stamp file the branch changed on neither side beyond a stamp against that side's base (back at the base's content // after a changelog-fragment conversion, or only its version moved) holds none of the branch's product: whatever else // differs between the two heads came from the base (GST-1210). - const ownStampOnly = (p: string) => !!branch?.touched.has(p) && stamped(p) && branch.sides.every(([a, b]) => blobAt(a, p) === blobAt(b, p) || classify(p, a, b) === 'metadata'); + const ownStampOnly = (p: string) => !!branch?.touched.has(p) && stamped(p) && branch.sides.every(([a, b]) => (git(['rev-parse', '--verify', '--quiet', `${a}:${p}`], repoRoot) ?? 'absent') === (git(['rev-parse', '--verify', '--quiet', `${b}:${p}`], repoRoot) ?? 'absent') || classify(p, a, b) === 'metadata'); const changed = changedRaw.split('\n').filter(Boolean).map(p => ({ path: p, class: ownStampOnly(p) ? 'metadata' as const : classify(p, from, to) })); const excludeGlobs = (globs: readonly string[]) => globs.map(g => `:(exclude,glob)${g}`); const demoted = !branch ? changed.filter(c => c.class === 'product' && matchesAny(rules.metadata, c.path)).map(c => c.path) diff --git a/lib/ship-patch.ts b/lib/ship-patch.ts index 36acfa7a2..242ba4157 100644 --- a/lib/ship-patch.ts +++ b/lib/ship-patch.ts @@ -125,7 +125,8 @@ export function comparePath(repoRoot: string, s: PatchSides, rel: string): PathV * branch added (absent at both bases, a regular file at both heads) named `…/.json`, whose whole text at each head is * byte for byte the value of entry `` in one JSON map line of a base file `baseMoved` lists: the old value at the * reviewed base, the new one at the current base. The branch then holds the base's own edit of that entry and nothing of - * its own. Null for anything else, so a pin that differs from the base's entry stays a product change. + * its own. Exactly one entry of that map may name the file. Null for anything else, so a pin that differs from the base's + * entry stays a product change. */ export function portedEntry(repoRoot: string, s: PatchSides, rel: string, baseMoved: Iterable): string | null { if (!rel.endsWith('.json')) return null; @@ -138,14 +139,14 @@ export function portedEntry(repoRoot: string, s: PatchSides, rel: string, baseMo const entries = (text: string | null) => new Set((text ?? '').split('\n').map(l => l.trim().replace(/,$/, ''))); for (const file of baseMoved) { if (file === rel || !file.endsWith('.json')) continue; - const now = entries(blob(s.currentBase, file)); - for (const line of now) { + const named = [...entries(blob(s.currentBase, file))].flatMap(line => { const m = /^("(?:[^"\\]|\\.)*"): (.*)$/.exec(line); - if (!m || m[2] !== after) continue; let key: unknown; - try { key = JSON.parse(m[1]!); } catch { continue; } - if (typeof key === 'string' && key && rel.endsWith(`/${key}.json`) && entries(blob(s.reviewedBase, file)).has(`${m[1]}: ${before}`)) return file; - } + try { key = m ? JSON.parse(m[1]!) : null; } catch { return []; } + return typeof key === 'string' && key && rel.endsWith(`/${key}.json`) ? [{ quoted: m![1]!, value: m![2]! }] : []; + }); + // One entry of the map names the pin file; two (`retry.js` and `lib/retry.js` for `…/lib/retry.js.json`) is no port. + if (named.length === 1 && named[0]!.value === after && entries(blob(s.reviewedBase, file)).has(`${named[0]!.quoted}: ${before}`)) return file; } return null; } diff --git a/test/ship-receipt-carry-ported-pin.test.ts b/test/ship-receipt-carry-ported-pin.test.ts index 71b8f55ad..d27856bfa 100644 --- a/test/ship-receipt-carry-ported-pin.test.ts +++ b/test/ship-receipt-carry-ported-pin.test.ts @@ -18,13 +18,13 @@ const carry = (repo: FixtureRepo, body: string) => runBin(repo, 'gstack-ship-rec const RETRY = '{"blob":"aaa","since":6,"nonBehavioral":"first pin"}'; const UTIL = '{"blob":"bbb","since":6}'; const RETRY_MAIN = '{"blob":"ccc","since":6,"nonBehavioral":"main repinned retry"}'; -const manifest = (retry: string) => `{\n "version": 6,\n "files": {\n "lib/retry.js": ${retry},\n "lib/util.js": ${UTIL}\n }\n}\n`; +const manifest = (retry: string, short?: string) => `{\n "version": 6,\n "files": {\n "lib/retry.js": ${retry},\n "lib/util.js": ${UTIL}${short ? `,\n "retry.js": ${short}` : ''}\n }\n}\n`; const PIN = 'scripts/harness-pins/lib/retry.js.json'; /** main holds the pin map; feature splits it into one file per entry beside a product change, /ship receipt there; main repins retry; feature merges main and writes `ports`. */ -function ported(repo: FixtureRepo, ports: Record): { reviewed: string; body: string } { +function ported(repo: FixtureRepo, ports: Record, maps: [string, string] = [manifest(RETRY), manifest(RETRY_MAIN)]): { reviewed: string; body: string } { repo.git('checkout', '-q', 'main'); - commit(repo, { 'scripts/harness-version.json': manifest(RETRY) }, 'main: harness pins'); + commit(repo, { 'scripts/harness-version.json': maps[0] }, 'main: harness pins'); repo.git('push', '-q', 'origin', 'main'); repo.git('checkout', '-q', 'feature'); repo.git('reset', '-q', '--hard', 'origin/main'); @@ -32,7 +32,7 @@ function ported(repo: FixtureRepo, ports: Record): { reviewed: s const body = path.join(repo.root, 'body.md'); expect(runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '3903', '--base', 'main', '--version', '1.0.0', '--session-kind', 'unattended', '--body', body]).status).toBe(EXIT.ok); repo.git('checkout', '-q', 'main'); - commit(repo, { 'scripts/harness-version.json': manifest(RETRY_MAIN), 'docs/README.md': '# docs after the repin\n' }, 'main: repin retry'); + commit(repo, { 'scripts/harness-version.json': maps[1], 'docs/README.md': '# docs after the repin\n' }, 'main: repin retry'); repo.git('push', '-q', 'origin', 'main'); repo.git('checkout', '-q', 'feature'); repo.git('merge', '-q', '--no-edit', 'origin/main'); @@ -75,4 +75,14 @@ describe('carry across a ported harness-pin entry (GST-1210)', () => { expect(out.status).toBe(EXIT.refused); expect(out.err).toContain('scripts/harness-pins/lib/util.js.json'); }, 90_000); + + // Value: protects=fail closed when a shorter key's repin lands in a longer key's pin file (`retry.js` for + // `…/lib/retry.js.json`); fails_when=the key is matched as a bare path suffix; seam=none; no_claim=gh pr edit + test('a repin of a key that is only a suffix of the pin path still refuses', () => { + const repo = makeFixtureRepo(); repos.push(repo); + const { body } = ported(repo, { [PIN]: `${RETRY_MAIN}\n` }, [manifest(RETRY, RETRY), manifest(RETRY, RETRY_MAIN)]); + const out = carry(repo, body); + expect(out.status).toBe(EXIT.refused); + expect(out.err).toContain(PIN); + }, 90_000); }); From f3c18d9d40861cdd4e8a323a245398f57a23899a Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 10:02:46 +0000 Subject: [PATCH 8/9] fix(ship-receipt): drop the pin-entry guess; stamp-file sides compare ls-tree entries (GST-1210, GSTA-36 ruling A) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-ship-receipt | 15 ++++----- .../capy-gst1210-carry-stamp-reverts.md | 2 +- docs/ship-policy.md | 1 - docs/troubleshooting.md | 3 +- lib/ship-patch.ts | 31 ------------------- lib/ship-receipt.ts | 5 +-- ...-receipt-carry-fragment-conversion.test.ts | 14 ++++++++- test/ship-receipt-carry-ported-pin.test.ts | 24 +++----------- 8 files changed, 28 insertions(+), 67 deletions(-) diff --git a/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index c3e2460ca..96f7630a3 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -48,7 +48,7 @@ import { receiptIssuesFrom, receiptSummaryLine, renderCensus, renderReceiptBlock, validateReceipt, type CensusPr, } from '../lib/ship-receipt'; import { checkDocsCaps, docsCapLine, docsExceptionLine, parseReceiptDocs } from '../lib/ship-receipt-docs'; -import { comparePath, contextPacket, PACKET_SECTION_CAP, portedEntry, readScopedReview, touchedPaths } from '../lib/ship-patch'; +import { comparePath, contextPacket, PACKET_SECTION_CAP, readScopedReview, touchedPaths } from '../lib/ship-patch'; import { deeper, depthBelow, exceptionProblems, parseReviewLoopException, parseReviewTargeted, receiptExclude, receiptReviewAtHead, receiptTargetedAtHead, reviewLoopExceptionLine, targetedClassesSince } from '../lib/ship-receipt-depth'; import { checkReceiptProse, proseLine } from '../lib/ship-receipt-prose'; import { resolveStateRoot } from '../lib/state-root'; @@ -538,7 +538,10 @@ function productSpec(repoRoot: string, rules: ProductRules, from: string, to: st // A stamp file the branch changed on neither side beyond a stamp against that side's base (back at the base's content // after a changelog-fragment conversion, or only its version moved) holds none of the branch's product: whatever else // differs between the two heads came from the base (GST-1210). - const ownStampOnly = (p: string) => !!branch?.touched.has(p) && stamped(p) && branch.sides.every(([a, b]) => (git(['rev-parse', '--verify', '--quiet', `${a}:${p}`], repoRoot) ?? 'absent') === (git(['rev-parse', '--verify', '--quiet', `${b}:${p}`], repoRoot) ?? 'absent') || classify(p, a, b) === 'metadata'); + // The same `ls-tree` entry (mode and object id, empty when absent) is an unchanged side; a failed read never is. + const entryAt = (ref: string, p: string) => git(['ls-tree', ref, '--', p], repoRoot); + const sameEntry = (a: string, b: string, p: string) => { const x = entryAt(a, p); return x !== undefined && x === entryAt(b, p); }; + const ownStampOnly = (p: string) => !!branch?.touched.has(p) && stamped(p) && branch.sides.every(([a, b]) => sameEntry(a, b, p) || classify(p, a, b) === 'metadata'); const changed = changedRaw.split('\n').filter(Boolean).map(p => ({ path: p, class: ownStampOnly(p) ? 'metadata' as const : classify(p, from, to) })); const excludeGlobs = (globs: readonly string[]) => globs.map(g => `:(exclude,glob)${g}`); const demoted = !branch ? changed.filter(c => c.class === 'product' && matchesAny(rules.metadata, c.path)).map(c => c.path) @@ -699,7 +702,6 @@ function cmdCarry(rest: string[], json: boolean): number { const changedProduct: string[] = []; const contextPaths: string[] = []; const stampOnly: string[] = []; - const ported: string[] = []; if (retarget && reviewed !== current) changedProduct.push(`HEAD's product diff against ${base} is not the one reviewed against ${reviewedBase} (the parent has not fully landed on ${base}, or the branch changed since)`); else if (productFrom !== productHead && !retarget) { // planCarry names at most CARRY_NAMED_PATHS differing paths, so the walk stops there, and it reads only the paths the @@ -719,11 +721,10 @@ function cmdCarry(rest: string[], json: boolean): number { if (cls === 'metadata' && spec.ownRows.includes(p) && !movedBase.has(p)) stampOnly.push(p); if (cls !== 'product') continue; const verdict = comparePath(repoRoot, sides, p); - if (verdict === 'changed' && portedEntry(repoRoot, sides, p, movedBase)) ported.push(p); - else if (verdict === 'changed') changedProduct.push(p); + if (verdict === 'changed') changedProduct.push(p); else if (verdict === 'context') contextPaths.push(p); } - const left = [...contextPaths, ...stampOnly, ...ported]; + const left = [...contextPaths, ...stampOnly]; if (!changedProduct.length && !left.length) changedProduct.push('(a product path outside the branch\'s touched files; compare the two base diffs)'); else if (!changedProduct.length) { const rest = [...spec.product, ...left.map(p => `:(exclude,literal)${p}`)]; @@ -762,7 +763,7 @@ function cmdCarry(rest: string[], json: boolean): number { // Evidence for the record, never the decision: a slow range-diff is reported unavailable rather than waited for. const rangeRaw = spawnSync('git', ['range-diff', '--no-color', `${rangeBase}..${from}`, `${headBase}..${head}`], { cwd: repoRoot, encoding: 'utf8', timeout: 15_000, maxBuffer: 64 * 1024 * 1024 }); const rangeDiff = rangeRaw.status === 0 ? (rangeRaw.stdout.trim().split('\n').slice(0, 20).join('\n') || '(empty: identical ranges)') : `unavailable: ${(rangeRaw.stderr ?? '').trim().split('\n')[0] || (rangeRaw.signal ? `range-diff killed by ${rangeRaw.signal} after 15 s` : 'range-diff failed')}`; - plan = planCarry({ from, head, changed, productPatchId: { from: reviewed as string, head: current as string }, testPatchId: { from: testFrom, head: testHead }, changedProduct, mergeCommits: Number(mergeCommits) || 0, rangeDiff, ...(retarget ? { retargetedFrom: reviewedBase } : {}), ...(stackParent ? { stackParent } : {}), ...(scopedReview ? { scopedReview } : {}), ...(stampedRows.length ? { stampedRows } : {}), ...(!changedProduct.length && ported.length ? { portedEntries: ported } : {}) }); + plan = planCarry({ from, head, changed, productPatchId: { from: reviewed as string, head: current as string }, testPatchId: { from: testFrom, head: testHead }, changedProduct, mergeCommits: Number(mergeCommits) || 0, rangeDiff, ...(retarget ? { retargetedFrom: reviewedBase } : {}), ...(stackParent ? { stackParent } : {}), ...(scopedReview ? { scopedReview } : {}), ...(stampedRows.length ? { stampedRows } : {}) }); } if (!plan.ok) return refused(plan.code, plan.message); // Eligibility is not publication: a receipt that recorded a pregate is rebound only with a pregate current for HEAD, diff --git a/changelog.d/capy-gst1210-carry-stamp-reverts.md b/changelog.d/capy-gst1210-carry-stamp-reverts.md index c24dcd56b..569eb9d5d 100644 --- a/changelog.d/capy-gst1210-carry-stamp-reverts.md +++ b/changelog.d/capy-gst1210-carry-stamp-reverts.md @@ -1 +1 @@ -- **A /ship receipt now survives two edits that change no reviewed code (GST-1210).** `gstack-ship-receipt carry` used to refuse with `RECEIPT_CARRY_PRODUCT_CHANGED` when a PR moved to a changelog fragment after review, putting `package.json` and the other stamp files back at main's content, if main had also changed `package.json` beyond its version (HARN-1, harness-evals#206). A stamp file the branch changes by no more than a stamp against each head's base now carries as release metadata. It also refused when a merge from main copied main's repin of one harness-pin entry, byte for byte, into the branch's one-file-per-entry pin file (#3903). That now carries as `base entries ported byte for byte`. A non-stamp `package.json` change, a logic edit, or a pin that differs from main's entry still refuses. +- **A /ship receipt now survives a changelog-fragment conversion (GST-1210).** When a PR moved to a `changelog.d` fragment after review, `VERSION`, `package.json` and `CHANGELOG.md` went back to main's content, and if main had also changed `package.json` beyond its version, `gstack-ship-receipt carry` read that as the branch's change and refused with `RECEIPT_CARRY_PRODUCT_CHANGED` (HARN-1, harness-evals#206). A stamp file the branch changes by no more than a stamp against each head's base now carries as release metadata. A non-stamp `package.json` change, a logic edit, or a stamp file whose entry can't be read still refuses. diff --git a/docs/ship-policy.md b/docs/ship-policy.md index cbf200b9d..abc075469 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1506,7 +1506,6 @@ receipt when the base moves: | a stacked PR whose receipt was written against the base, after its parent landed and the base was merged in, HEAD's product diff exactly the reviewed range from the parent's head | carries (reason `stack parent landed`); base context next to the child's hunks is `RECEIPT_CARRY_SCOPED_REVIEW` | | a stamp on the branch's own rows in a stamp file the branch also changed as product: every changed line at a branch-added position, the file's mode unchanged, the base not touching it, the rest of the product diff unchanged | carries (reason `a stamp on the branch's own rows ()`) | | a stamp file the branch changes, against each head's base, by no more than a stamp (back at the base's content after a changelog-fragment conversion, or only its version field moved), whatever else the base changed in it | carries (reason `release metadata`, GST-1210) | -| a one-line `*.json` pin file the branch added, named `…/.json`, whose change since the reviewed head is byte for byte the base's own edit of entry `` in a JSON map line of a base file (a merge that ports a base repin into a one-file-per-entry split) | carries (reason `base entries ported byte for byte ()`, GST-1210); a pin that differs from the base's entry, or sits in another entry's file, refuses | **Stacked PRs (#3570).** A child PR's receipt is written against its parent branch, so its `base` names that branch and its `product_patch_id` is the diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 882aea8c4..24cc0ec57 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -5996,8 +5996,7 @@ landed on the base, a stamp on the branch's own rows in a stamp file it also changed as product (every changed line at a branch-added position, the mode unchanged, the base not touching the file), a stamp file the branch changes by no more than a stamp against each head's base (a changelog-fragment conversion -that puts `package.json` back at the base's content), a one-entry pin file -whose change is byte for byte the base's edit of that entry, or base context next to unchanged +that puts `package.json` back at the base's content), or base context next to unchanged hunks after a clean scoped review ([`RECEIPT_CARRY_SCOPED_REVIEW`](#receipt-carry-scoped-review)), which the `carry` object in the carried receipt records with the product patch-id and the range-diff evidence ([unattended.md](unattended.md#the-terminal-line)). diff --git a/lib/ship-patch.ts b/lib/ship-patch.ts index 242ba4157..d069dc5cc 100644 --- a/lib/ship-patch.ts +++ b/lib/ship-patch.ts @@ -120,37 +120,6 @@ export function comparePath(repoRoot: string, s: PatchSides, rel: string): PathV return ids[0] === ids[1] ? 'same' : 'context'; } -/** - * The base's JSON map file a one-entry pin file ports (GST-1210, #3903 at f0613db8): `rel` is a one-line `*.json` file the - * branch added (absent at both bases, a regular file at both heads) named `…/.json`, whose whole text at each head is - * byte for byte the value of entry `` in one JSON map line of a base file `baseMoved` lists: the old value at the - * reviewed base, the new one at the current base. The branch then holds the base's own edit of that entry and nothing of - * its own. Exactly one entry of that map may name the file. Null for anything else, so a pin that differs from the base's - * entry stays a product change. - */ -export function portedEntry(repoRoot: string, s: PatchSides, rel: string, baseMoved: Iterable): string | null { - if (!rel.endsWith('.json')) return null; - const blob = (ref: string, p: string) => git(repoRoot, ['show', `${ref}:${p}`]); - const regular = (ref: string) => /^100644 blob /.test(git(repoRoot, ['ls-tree', ref, '--', rel]) ?? ''); - const value = (text: string | null) => (text !== null && /^[^\n]+\n$/.test(text) ? text.slice(0, -1) : null); - const before = value(blob(s.reviewed, rel)), after = value(blob(s.current, rel)); - if (before === null || after === null || before === after || !regular(s.reviewed) || !regular(s.current)) return null; - if (blob(s.reviewedBase, rel) !== null || blob(s.currentBase, rel) !== null) return null; - const entries = (text: string | null) => new Set((text ?? '').split('\n').map(l => l.trim().replace(/,$/, ''))); - for (const file of baseMoved) { - if (file === rel || !file.endsWith('.json')) continue; - const named = [...entries(blob(s.currentBase, file))].flatMap(line => { - const m = /^("(?:[^"\\]|\\.)*"): (.*)$/.exec(line); - let key: unknown; - try { key = m ? JSON.parse(m[1]!) : null; } catch { return []; } - return typeof key === 'string' && key && rel.endsWith(`/${key}.json`) ? [{ quoted: m![1]!, value: m![2]! }] : []; - }); - // One entry of the map names the pin file; two (`retry.js` and `lib/retry.js` for `…/lib/retry.js.json`) is no port. - if (named.length === 1 && named[0]!.value === after && entries(blob(s.reviewedBase, file)).has(`${named[0]!.quoted}: ${before}`)) return file; - } - return null; -} - function hunkTexts(diff: string): string[] { return diff.split(/^(?=@@ )/m).slice(1).map(h => h.replace(/^@@ [^@]*@@/, '@@')); } diff --git a/lib/ship-receipt.ts b/lib/ship-receipt.ts index 3130cbea8..bbb145358 100644 --- a/lib/ship-receipt.ts +++ b/lib/ship-receipt.ts @@ -184,8 +184,6 @@ export interface CarryPlanInput { scopedReview?: string; /** Stamp files the branch changed as product whose only change since the reviewed head is a stamp on the branch's own rows, the base untouched, and without which the two product patches match (GST-980). */ stampedRows?: string[]; - /** Branch-added one-entry pin files whose change since the reviewed head is, byte for byte, the base's own edit of that entry in a base JSON map (`portedEntry`, GST-1210). */ - portedEntries?: string[]; } export interface CarryRecord { from: string; reason: string; product_patch_id: string; changed_paths: string[]; merge_commits: number; range_diff: string; covers: string[]; reruns: string[]; trees?: string[]; ancestry?: 'linear' | 'rewritten' } export type CarryPlan = { ok: true; carry: CarryRecord } | { ok: false; code: 'RECEIPT_CARRY_PRODUCT_CHANGED' | 'RECEIPT_CARRY_REFUSED' | 'RECEIPT_CARRY_TREE_MISMATCH'; message: string }; @@ -254,7 +252,7 @@ export function classifyCarryPath(rel: string, extra: { metadata?: readonly stri * branch's own rows in a stamp file the branch also changed as product (`stampedRows`, GST-980). */ export function planCarry(input: CarryPlanInput): CarryPlan { - if (input.productPatchId.from !== input.productPatchId.head && (input.changedProduct.length || (!input.scopedReview && !input.stampedRows?.length && !input.portedEntries?.length))) { + if (input.productPatchId.from !== input.productPatchId.head && (input.changedProduct.length || (!input.scopedReview && !input.stampedRows?.length))) { const named = input.changedProduct.slice(0, CARRY_NAMED_PATHS); return { ok: false, code: 'RECEIPT_CARRY_PRODUCT_CHANGED', message: `product diff changed between ${input.from.slice(0, 12)} and ${input.head.slice(0, 12)}: ${named.join(', ') || 'patch-id differs'}${input.changedProduct.length > named.length ? ` (+${input.changedProduct.length - named.length} more)` : ''}` }; } @@ -263,7 +261,6 @@ export function planCarry(input: CarryPlanInput): CarryPlan { if (input.mergeCommits > 0) reasons.push(input.mergeCommits === 1 ? 'base merge' : `${input.mergeCommits} base merges`); if (input.scopedReview && input.productPatchId.from !== input.productPatchId.head) reasons.push(input.scopedReview); if (input.stampedRows?.length && input.productPatchId.from !== input.productPatchId.head) reasons.push(`a stamp on the branch's own rows (${input.stampedRows.join(', ')})`); - if (input.portedEntries?.length && input.productPatchId.from !== input.productPatchId.head) reasons.push(`base entries ported byte for byte (${input.portedEntries.join(', ')})`); if (input.testPatchId.from !== input.testPatchId.head) reasons.push('test-only change'); if (input.changed.some(c => c.class === 'metadata')) reasons.push('release metadata'); if (input.changed.some(c => c.class === 'generated')) reasons.push('regenerated outputs'); diff --git a/test/ship-receipt-carry-fragment-conversion.test.ts b/test/ship-receipt-carry-fragment-conversion.test.ts index 5a0ff6f55..c1897e5a9 100644 --- a/test/ship-receipt-carry-fragment-conversion.test.ts +++ b/test/ship-receipt-carry-fragment-conversion.test.ts @@ -25,7 +25,7 @@ const RETRY4 = "exports.MAX_RETRIES = 4;\nexports.TIMEOUT_MS = 5000;\nexports.re * adopts changelog fragments and (`mainScripts`) changes package.json beyond its version. The branch merges main and * converts: `converted` is the package.json it keeps (main's by default), plus a fragment and any `extra` files. */ -function converted(repo: FixtureRepo, opts: { mainScripts?: Record; keep?: (main: string) => string; extra?: Record } = {}): { reviewed: string; body: string } { +function converted(repo: FixtureRepo, opts: { mainScripts?: Record; keep?: (main: string) => string; extra?: Record; executable?: boolean } = {}): { reviewed: string; body: string } { repo.git('checkout', '-q', 'main'); commit(repo, { VERSION: '1.0.0\n', 'CHANGELOG.md': '# Changelog\n\n## 1.0.0\n- base\n', '.gstack/ship-policy.json': POLICY(false) }, 'main: stamp files'); repo.git('push', '-q', 'origin', 'main'); @@ -45,6 +45,7 @@ function converted(repo: FixtureRepo, opts: { mainScripts?: Record m))(mainPkg)); write(repo.work, 'changelog.d/four-retries.md', '- four retries\n'); for (const [rel, text] of Object.entries(opts.extra ?? {})) write(repo.work, rel, text); + if (opts.executable) fs.chmodSync(path.join(repo.work, 'package.json'), 0o755); repo.git('add', '-A'); repo.git('commit', '-q', '--no-edit', '-m', 'convert to a changelog fragment'); return { reviewed, body }; @@ -87,6 +88,17 @@ describe('carry across a changelog-fragment conversion (GST-1210)', () => { expect(out.err).toContain('package.json'); }, 90_000); + // Value: protects=fail closed on a stamp file whose content is the base's but whose mode the branch flipped (a mode is + // not a stamp); fails_when=an unchanged side is read from the object id alone; seam=none; no_claim=gh pr edit + test('a package.json back at the base\'s content with a flipped mode still refuses', () => { + const repo = makeFixtureRepo(); repos.push(repo); + const { body } = converted(repo, { mainScripts: { lint: 'node lint.js' }, executable: true }); + const out = carry(repo, body); + expect(out.status).toBe(EXIT.refused); + expect(out.err).toContain('RECEIPT_CARRY_PRODUCT_CHANGED'); + expect(out.err).toContain('package.json'); + }, 90_000); + // Value: protects=fail closed on a logic edit beside the reviewed one riding the conversion; fails_when=the stamp-file // rule widens to the whole diff; seam=none; no_claim=gh pr edit test('a logic edit next to the reviewed constant still refuses', () => { diff --git a/test/ship-receipt-carry-ported-pin.test.ts b/test/ship-receipt-carry-ported-pin.test.ts index d27856bfa..7e80a18ab 100644 --- a/test/ship-receipt-carry-ported-pin.test.ts +++ b/test/ship-receipt-carry-ported-pin.test.ts @@ -1,19 +1,17 @@ /** - * gstack-ship-receipt carry across a base merge that ports one harness-pin entry (GST-1210, GSTA-202 #3903 at f0613db8): - * the branch splits the base's JSON map of pins into one file per entry, and a later merge from the base copies the base's - * edit of one entry into its pin file, byte for byte. That file holds the base's change and nothing of the branch's, so - * the receipt carries; a pin that differs from the base's entry, or lands in another entry's file, still refuses. + * gstack-ship-receipt carry never guesses which base entry a branch-added pin file holds (GST-1210, GSTA-202 #3903 at + * f0613db8): the branch splits the base's JSON map of pins into one file per entry, and a pin file that changes after review + * is a product change whatever the base did to a same-named entry. Inferring the mapping from names and values let an + * unrelated base edit carry an unreviewed change (GSTA-36 ruling A on #3937), so these controls keep every such pin refused. */ import { afterEach, describe, expect, test } from 'bun:test'; import * as fs from 'node:fs'; import * as path from 'node:path'; import { EXIT } from '../lib/headless-artifacts'; -import { extractReceipt } from '../lib/ship-receipt'; import { cleanup, commit, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/pregate-fixture'; const repos: FixtureRepo[] = []; afterEach(() => { while (repos.length) cleanup(repos.pop()!); }); -const receiptIn = (body: string) => extractReceipt(fs.readFileSync(body, 'utf8')).receipt!; const carry = (repo: FixtureRepo, body: string) => runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main']); const RETRY = '{"blob":"aaa","since":6,"nonBehavioral":"first pin"}'; const UTIL = '{"blob":"bbb","since":6}'; @@ -41,20 +39,6 @@ function ported(repo: FixtureRepo, ports: Record, maps: [string, } describe('carry across a ported harness-pin entry (GST-1210)', () => { - // Value: protects=a branch that splits harness pins into one file per entry keeps its receipt when a base merge ports - // the base's repin byte for byte (#3903 at f0613db8 was refused); fails_when=carry reads the ported pin as the branch's - // product change; seam=none; no_claim=gh pr edit (--body stands in) - test('a pin file equal to the base\'s entry byte for byte carries', () => { - const repo = makeFixtureRepo(); repos.push(repo); - const { reviewed, body } = ported(repo, { [PIN]: `${RETRY_MAIN}\n` }); - const head = repo.git('rev-parse', 'HEAD'); - const out = carry(repo, body); - expect(out.err).not.toContain('RECEIPT_CARRY'); - 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).toContain(`base entries ported byte for byte (${PIN})`); - }, 90_000); - // Value: protects=fail closed on a pin the branch changed beyond the base's entry (an unreviewed repin); // fails_when=any pin file change carries; seam=none; no_claim=gh pr edit test('a pin entry that differs from the base\'s still refuses', () => { From bae00b1ea92a95f137173cd22b9a5dbf3a395d4f Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 11:26:41 +0000 Subject: [PATCH 9/9] test: stage the mode flip with update-index so the control holds on Windows (GST-1210, run 41626229324) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- test/ship-receipt-carry-fragment-conversion.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/ship-receipt-carry-fragment-conversion.test.ts b/test/ship-receipt-carry-fragment-conversion.test.ts index c1897e5a9..453c48cd9 100644 --- a/test/ship-receipt-carry-fragment-conversion.test.ts +++ b/test/ship-receipt-carry-fragment-conversion.test.ts @@ -47,6 +47,7 @@ function converted(repo: FixtureRepo, opts: { mainScripts?: Record