diff --git a/lib/ship-receipt.ts b/lib/ship-receipt.ts index 8ad936535..4bb465dd4 100644 --- a/lib/ship-receipt.ts +++ b/lib/ship-receipt.ts @@ -329,7 +329,8 @@ export const HELD_WALK = 200; * `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 conflicted path is never held. Null when none within HELD_WALK holds it or git cannot tell. + * 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 }); @@ -338,9 +339,12 @@ export function heldBy(repoRoot: string, commit: string, currentBase: string, pa 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', at, commit]); - const tree = merged.status === 0 || merged.status === 1 ? merged.stdout.split('\n')[0]?.trim() : undefined; - if (merged.status === 0 && tree && run(['diff', '--quiet', at, tree, '--', ...pathspec]).status === 0) return 'held'; + const merged = run(['merge-tree', '--write-tree', '--no-messages', '--name-only', at, commit]); + const [tree, ...conflicted] = merged.status === 0 || merged.status === 1 ? merged.stdout.split('\n').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); diff --git a/lib/ship-release.ts b/lib/ship-release.ts index f33115c78..ecaf569a0 100644 --- a/lib/ship-release.ts +++ b/lib/ship-release.ts @@ -306,7 +306,7 @@ function route(cwd: string, base: string, reviewed: string, reviewedBase: Review if (loop) { const head = loop.fix_check.head; const since = touchedPaths(cwd, head, now); - const fork = parent && ancestor(parent, head) ? parent : git(cwd, ['merge-base', baseRef, head]).stdout.trim(); + const fork = patchBase(cwd, baseRef, head, parent); const atHead = fork ? touchedPaths(cwd, fork, head) : 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; diff --git a/test/ship-finish-stack-parent.test.ts b/test/ship-finish-stack-parent.test.ts index bd6ebd674..7035f04ac 100644 --- a/test/ship-finish-stack-parent.test.ts +++ b/test/ship-finish-stack-parent.test.ts @@ -210,6 +210,35 @@ describe('finish after a stacked parent squash-merges (GST-1399)', () => { 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 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', () => {