fix: require a durable merge record for the step-finalization exemption

Full-suite set-diff against 3f448f7292 caught three regressions the raw counts
hid (that suite is chronically red: 297 failures at baseline, 294 with the
change).

The step exemption was too broad. It also applied to
recoverAlreadyMergedReviewTasks, the content-scan recovery where mergeDetails is
ABSENT and landing is inferred by finding matching content on the base branch.
That heuristic can match a cherry-pick, so exempting incomplete steps there
would launder a genuinely unfinished task to done on a guess — which is exactly
what landed-content-soft-blocker.real-git.test.ts exists to prevent. The
exemption now requires mergeConfirmed AND a commitSha: FN-9193's actual state,
and nothing weaker. Content-scan recovery and no-op merges keep the blocker.

Also seeds mergeSweepHoldReasons in the shared merge-lane fixture, which the
fixture-drift guard requires of every auto-merge state field.

Verified by set-diff: zero test files now fail that did not fail at baseline.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-08-23 09:45:10 -07:00
parent a879ead0fc
commit ea48af7ab5
3 changed files with 36 additions and 9 deletions

View File

@@ -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();
});

View File

@@ -616,13 +616,26 @@ export function getMergeConfirmedFinalizationBlocker(
options: { reviewColumns?: ReadonlySet<string>; requiredPreMergeStepIds?: ReadonlySet<string> } = {},
): 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. */

View File

@@ -6,6 +6,7 @@ type MergeLaneState = {
capacityDeferredMerges: Map<string, unknown>;
coordinatorAdmittedMergeTaskIds: Set<string>;
pausedReviewTaskIds: Set<string>;
mergeSweepHoldReasons: Map<string, string>;
mergeRunning: boolean;
mergeRunningSince: number;
activeMergeSession: { dispose(): void } | null;
@@ -41,6 +42,9 @@ export function seedMergeLaneState<T extends object>(
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,