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');