diff --git a/packages/core/src/__tests__/task-merge.test.ts b/packages/core/src/__tests__/task-merge.test.ts index 7e01d11f28..71e36f9fdf 100644 --- a/packages/core/src/__tests__/task-merge.test.ts +++ b/packages/core/src/__tests__/task-merge.test.ts @@ -1103,6 +1103,7 @@ while a genuinely rejecting pre-merge review still does. describe("getMergeConfirmedFinalizationBlocker", () => { const landedWithUnfinishedWork = { ...baseTask, + mergeDetails: { mergeConfirmed: true, commitSha: "eaa1d47c" } as never, steps: [ { name: "Preflight", status: "in-progress" as StepStatus }, { name: "Remove the dead CSS custom-property reference", status: "pending" as StepStatus }, @@ -1118,6 +1119,7 @@ describe("getMergeConfirmedFinalizationBlocker", () => { it("survives the restart loop that re-plans fresh pending steps", () => { const replanned = { ...baseTask, + mergeDetails: { mergeConfirmed: true, commitSha: "eaa1d47c" } as never, steps: Array.from({ length: 7 }, (_, i) => ({ name: `Step ${i}`, status: "pending" as StepStatus })), }; expect(getMergeConfirmedFinalizationBlocker(replanned)).toBeUndefined(); @@ -1125,15 +1127,23 @@ describe("getMergeConfirmedFinalizationBlocker", () => { /* A no-op merge with no commit sha landed NOTHING, so incomplete steps stay an honest blocker — and the executor's no-op branch depends on that reason still firing. */ - it("still blocks incomplete steps when the merge landed no content", () => { + /* The exemption needs a DURABLE merge record naming the landed commit. A no-op merge with no sha + landed nothing, and the content-scan recovery path (mergeDetails absent, landing inferred from + branch content) must keep the blocker or it would launder an unfinished task to done on a + heuristic — see `landed-content-soft-blocker.real-git.test.ts`. */ + it("still blocks incomplete steps without a durable merge record", () => { expect(getMergeConfirmedFinalizationBlocker({ ...landedWithUnfinishedWork, - mergeDetails: { noOpMerge: true } as never, + mergeDetails: { mergeConfirmed: true, noOpMerge: true } as never, + })).toBe("task has incomplete steps"); + expect(getMergeConfirmedFinalizationBlocker({ + ...landedWithUnfinishedWork, + mergeDetails: undefined, })).toBe("task has incomplete steps"); // A no-op that still produced a commit did land something; the exemption applies. expect(getMergeConfirmedFinalizationBlocker({ ...landedWithUnfinishedWork, - mergeDetails: { noOpMerge: true, commitSha: "abc123" } as never, + mergeDetails: { mergeConfirmed: true, noOpMerge: true, commitSha: "abc123" } as never, })).toBeUndefined(); }); diff --git a/packages/core/src/merge/task-merge.ts b/packages/core/src/merge/task-merge.ts index ff577a3a79..e650224598 100644 --- a/packages/core/src/merge/task-merge.ts +++ b/packages/core/src/merge/task-merge.ts @@ -616,13 +616,26 @@ export function getMergeConfirmedFinalizationBlocker( options: { reviewColumns?: ReadonlySet; requiredPreMergeStepIds?: ReadonlySet } = {}, ): string | undefined { /* - The exemption is scoped to a merge that actually LANDED CONTENT. A no-op merge with no commit sha - landed nothing, so its card is not "already done however you look at it" — there incomplete steps - are the honest signal that the work is unfinished, and the executor's own no-op branch - (`merge-confirmed-finalize.ts`) depends on that blocker still firing. + THE EXEMPTION NEEDS A DURABLE MERGE RECORD, not merely a belief that content landed. Two nearby + paths look similar and are not: + + - FN-9193's shape: `mergeConfirmed` with the landed `commitSha` — this engine performed the + merge and recorded it. Incomplete steps here describe work the landed branch superseded, so + they must not hold the card. + - The content-scan recovery (`recoverAlreadyMergedReviewTasks`): mergeDetails is ABSENT and the + sweep infers landing by finding matching content on the base branch, then synthesizes a + record. That heuristic can match a cherry-pick or a similar commit, so exempting steps there + would launder a genuinely unfinished task to `done` on a guess. Its own reliability test + (`landed-content-soft-blocker.real-git.test.ts`) pins that distinction: soft blockers clear, + incomplete steps hold. + + A no-op merge with no commit sha also fails this test, which is what the executor's no-op branch + in `merge-confirmed-finalize.ts` depends on. */ - const landedNothing = task.mergeDetails?.noOpMerge === true && !task.mergeDetails?.commitSha; - return getTaskHardMergeBlocker(landedNothing ? task : { ...task, steps: [] }, options); + const hasDurableMergeRecord = task.mergeDetails?.mergeConfirmed === true + && typeof task.mergeDetails.commitSha === "string" + && task.mergeDetails.commitSha.length > 0; + return getTaskHardMergeBlocker(hasDurableMergeRecord ? { ...task, steps: [] } : task, options); } /** Non-terminal steps on a card being finalized after a proven merge — recorded, never silently dropped. */ diff --git a/packages/engine/src/__tests__/_project-engine-merge-lane-fixture.ts b/packages/engine/src/__tests__/_project-engine-merge-lane-fixture.ts index 9a1577df88..bda5c584a4 100644 --- a/packages/engine/src/__tests__/_project-engine-merge-lane-fixture.ts +++ b/packages/engine/src/__tests__/_project-engine-merge-lane-fixture.ts @@ -6,6 +6,7 @@ type MergeLaneState = { capacityDeferredMerges: Map; coordinatorAdmittedMergeTaskIds: Set; pausedReviewTaskIds: Set; + mergeSweepHoldReasons: Map; mergeRunning: boolean; mergeRunningSince: number; activeMergeSession: { dispose(): void } | null; @@ -41,6 +42,9 @@ export function seedMergeLaneState( capacityDeferredMerges: new Map(), coordinatorAdmittedMergeTaskIds: new Set(), pausedReviewTaskIds: new Set(), + /* FNXC:MergeAuthority 2026-08-23-21:40: the merge-sweep hold-reason log de-duplicator. Empty is + the production-equivalent default — a fresh engine has held nothing yet. */ + mergeSweepHoldReasons: new Map(), mergeRunning: false, mergeRunningSince: 0, activeMergeSession: null,