fix: defer merge on unrun pre-merge gates instead of failing the task
An enabled pre-merge gate that has not reported yet is a not-yet condition, not a failure. FN-9191 proved the difference is load-bearing: the in-review auto-merge sweep enqueued the card ~2s after fn_task_done and ~18s before the graph started its own Code Review node, the merge door correctly refused, and the auto-merge error path parked it status="failed". Code Review APPROVED two minutes later, but every subsequent merge — including the graph's own merge node — then died on "task is marked 'failed'". - Merge doors throw the typed PreMergeStepsNotRunError for that blocker. - The auto-merge error path treats it as a deferral: no status write, no mergeRetries burn, no operator handoff. - enqueueEligibleInReviewTasks holds a card out of the merge queue until every enabled pre-merge group has a result, so the race stops at admission. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
7
.changeset/pre-merge-gate-deferral.md
Normal file
7
.changeset/pre-merge-gate-deferral.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: A task no longer fails permanently when auto-merge runs before its Code Review gate.
|
||||
category: fix
|
||||
dev: Merge doors throw the typed `PreMergeStepsNotRunError` for the unrun-enabled-gate blocker; the auto-merge error path treats it as a deferral (no `status:"failed"` park, no retry burn), and `enqueueEligibleInReviewTasks` holds in-review cards out of the merge queue until every enabled pre-merge group has a result (`findUnrunRequiredPreMergeStepIds`).
|
||||
@@ -1,6 +1,6 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { BUILTIN_CODING_WORKFLOW_IR } from "../workflows/builtin-coding-workflow-ir.js";
|
||||
import { resolveRequiredPreMergeStepIds } from "../merge/required-pre-merge-steps.js";
|
||||
import { findUnrunRequiredPreMergeStepIds, resolveRequiredPreMergeStepIds } from "../merge/required-pre-merge-steps.js";
|
||||
|
||||
describe("resolveRequiredPreMergeStepIds", () => {
|
||||
it("includes default-on pre-merge groups when no explicit selection exists", () => {
|
||||
@@ -23,3 +23,46 @@ describe("resolveRequiredPreMergeStepIds", () => {
|
||||
)).toEqual(new Set(["browser-verification"]));
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:RequiredPreMergeSteps 2026-08-22-22:40 (FN-9191 wedge):
|
||||
The auto-merge sweep's admission uses this to hold a card out of the merge queue until every
|
||||
enabled pre-merge gate has reported. FN-9191's exact shape — both gates enabled, Plan Review
|
||||
already passed, Code Review not yet started — must read as "unrun", and the same task after
|
||||
Code Review lands must read as ready.
|
||||
*/
|
||||
describe("findUnrunRequiredPreMergeStepIds", () => {
|
||||
const planReviewPassed = {
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
status: "passed" as const,
|
||||
phase: "pre-merge" as const,
|
||||
};
|
||||
const codeReviewPassed = {
|
||||
workflowStepId: "code-review",
|
||||
workflowStepName: "Code Review",
|
||||
status: "passed" as const,
|
||||
phase: "pre-merge" as const,
|
||||
};
|
||||
|
||||
it("reports the FN-9191 window: Code Review enabled but not yet started", () => {
|
||||
expect(findUnrunRequiredPreMergeStepIds(BUILTIN_CODING_WORKFLOW_IR, {
|
||||
enabledWorkflowSteps: ["plan-review", "code-review"],
|
||||
workflowStepResults: [planReviewPassed],
|
||||
})).toEqual(["code-review"]);
|
||||
});
|
||||
|
||||
it("reports nothing once every enabled gate has a result", () => {
|
||||
expect(findUnrunRequiredPreMergeStepIds(BUILTIN_CODING_WORKFLOW_IR, {
|
||||
enabledWorkflowSteps: ["plan-review", "code-review"],
|
||||
workflowStepResults: [planReviewPassed, codeReviewPassed],
|
||||
})).toEqual([]);
|
||||
});
|
||||
|
||||
it("reports nothing when the gates are disabled for the task", () => {
|
||||
expect(findUnrunRequiredPreMergeStepIds(BUILTIN_CODING_WORKFLOW_IR, {
|
||||
enabledWorkflowSteps: [],
|
||||
workflowStepResults: [],
|
||||
})).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import type { PrInfo, StepStatus } from "../types.js";
|
||||
import {
|
||||
isPreMergeStepsNotRunBlocker,
|
||||
PreMergeStepsNotRunError,
|
||||
PRE_MERGE_STEPS_NOT_RUN_BLOCKER,
|
||||
BLOCKING_TASK_STATUSES,
|
||||
collectLandedMemberReviewAdvisories,
|
||||
HARD_BLOCKING_TASK_STATUSES,
|
||||
@@ -518,6 +521,18 @@ describe("getTaskMergeBlocker", () => {
|
||||
expect(getTaskMergeBlocker(baseTask)).toBeUndefined();
|
||||
});
|
||||
|
||||
/* FNXC:RequiredPreMergeSteps 2026-08-22-22:40: the blocker text is a shared contract — the
|
||||
auto-merge error path classifies on the typed error built from it, so it may not drift. */
|
||||
it("exposes the unrun-gate reason as a classifiable constant", () => {
|
||||
expect(getTaskMergeBlocker(baseTask, { requiredPreMergeStepIds: new Set(["code-review"]) }))
|
||||
.toBe(PRE_MERGE_STEPS_NOT_RUN_BLOCKER);
|
||||
expect(isPreMergeStepsNotRunBlocker(PRE_MERGE_STEPS_NOT_RUN_BLOCKER)).toBe(true);
|
||||
expect(isPreMergeStepsNotRunBlocker("task has failed pre-merge workflow steps")).toBe(false);
|
||||
expect(isPreMergeStepsNotRunBlocker(undefined)).toBe(false);
|
||||
expect(new PreMergeStepsNotRunError("FN-9191").message)
|
||||
.toBe(`Cannot merge FN-9191: ${PRE_MERGE_STEPS_NOT_RUN_BLOCKER}`);
|
||||
});
|
||||
|
||||
it("accepts a skipped result for a required pre-merge group", () => {
|
||||
expect(getTaskMergeBlocker({
|
||||
...baseTask,
|
||||
|
||||
@@ -1086,6 +1086,9 @@ export { getPrimaryPrInfo, taskHasManualOpenPullRequest } from "./tasks/task-hel
|
||||
export {
|
||||
collectLandedMemberReviewAdvisories,
|
||||
getTaskMergeBlocker,
|
||||
isPreMergeStepsNotRunBlocker,
|
||||
PreMergeStepsNotRunError,
|
||||
PRE_MERGE_STEPS_NOT_RUN_BLOCKER,
|
||||
getTaskHardMergeBlocker,
|
||||
REVIEW_ELIGIBLE_SENTINEL_COLUMN,
|
||||
MERGE_CONFIRMED_TRANSIENT_STATUSES,
|
||||
|
||||
@@ -350,7 +350,7 @@ export {
|
||||
isWorkflowOptionalGroupEnabled,
|
||||
} from "./workflows/workflow-optional-steps.js";
|
||||
export type { ResolvedWorkflowOptionalStep } from "./workflows/workflow-optional-steps.js";
|
||||
export { resolveRequiredPreMergeStepIds } from "./merge/required-pre-merge-steps.js";
|
||||
export { resolveRequiredPreMergeStepIds, findUnrunRequiredPreMergeStepIds } from "./merge/required-pre-merge-steps.js";
|
||||
export {
|
||||
applyPromptOverridesToIr,
|
||||
enumeratePromptBearingWorkflowNodes,
|
||||
@@ -1252,6 +1252,9 @@ export { getPrimaryPrInfo, taskHasManualOpenPullRequest } from "./tasks/task-hel
|
||||
export {
|
||||
collectLandedMemberReviewAdvisories,
|
||||
getTaskMergeBlocker,
|
||||
isPreMergeStepsNotRunBlocker,
|
||||
PreMergeStepsNotRunError,
|
||||
PRE_MERGE_STEPS_NOT_RUN_BLOCKER,
|
||||
getTaskHardMergeBlocker,
|
||||
REVIEW_ELIGIBLE_SENTINEL_COLUMN,
|
||||
MERGE_CONFIRMED_TRANSIENT_STATUSES,
|
||||
|
||||
@@ -24,3 +24,24 @@ export function resolveRequiredPreMergeStepIds(
|
||||
.map((step) => step.templateId),
|
||||
);
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:RequiredPreMergeSteps 2026-08-22-22:40 (FN-9191 wedge):
|
||||
Admission-side twin of the door's check. The in-review auto-merge sweep must be able to ask
|
||||
"has every enabled pre-merge gate reported yet?" BEFORE it queues a card, because its sync
|
||||
`canMergeTask` admission sees result rows only — and a gate that has not started has no row.
|
||||
FN-9191 was queued ~2s after `fn_task_done` and ~18s before its own Code Review node started.
|
||||
Shared with the door so the two can never answer differently.
|
||||
*/
|
||||
/** Enabled pre-merge group ids that have produced no result row on this task yet. */
|
||||
export function findUnrunRequiredPreMergeStepIds(
|
||||
ir: WorkflowIr,
|
||||
task: {
|
||||
enabledWorkflowSteps?: readonly string[];
|
||||
workflowStepResults?: ReadonlyArray<{ workflowStepId?: string }>;
|
||||
},
|
||||
): string[] {
|
||||
const results = task.workflowStepResults ?? [];
|
||||
return [...resolveRequiredPreMergeStepIds(ir, task.enabledWorkflowSteps)]
|
||||
.filter((workflowStepId) => !results.some((result) => result.workflowStepId === workflowStepId));
|
||||
}
|
||||
|
||||
@@ -339,6 +339,44 @@ const NON_TERMINAL_WORKFLOW_STATUSES = new Set<WorkflowStepResult["status"]>([
|
||||
"pending",
|
||||
]);
|
||||
|
||||
/*
|
||||
FNXC:RequiredPreMergeSteps 2026-08-22-22:40 (FN-9191 wedge):
|
||||
"An enabled pre-merge gate has not produced a result yet" is a NOT-YET condition, not a
|
||||
failure. FN-9191 proved the difference is load-bearing: the in-review auto-merge sweep
|
||||
enqueued the card ~2s after `fn_task_done`, BEFORE the graph started its own Code Review
|
||||
node, so the door correctly refused — and the auto-merge error path then parked the card
|
||||
`status:"failed"` as a non-conflict failure. Code Review completed and APPROVED 2 minutes
|
||||
later, but every subsequent merge (including the graph's own merge node) then failed with
|
||||
`task is marked 'failed'`. A correct refusal became a permanent wedge.
|
||||
|
||||
The blocker message is exported so merge doors can throw the typed error below instead of a
|
||||
bare `Error`, and so the auto-merge error path can classify it as a deferral rather than a
|
||||
terminal park.
|
||||
*/
|
||||
export const PRE_MERGE_STEPS_NOT_RUN_BLOCKER =
|
||||
"task has enabled pre-merge workflow steps that never ran";
|
||||
|
||||
/**
|
||||
* Thrown by merge doors when the ONLY thing standing between a card and merge is an
|
||||
* enabled pre-merge gate that has not run yet. Callers must treat it as "retry after the
|
||||
* gate reports", never as a terminal failure: no `status:"failed"` park, no retry-budget
|
||||
* burn, no operator handoff.
|
||||
*/
|
||||
export class PreMergeStepsNotRunError extends Error {
|
||||
readonly code = "pre-merge-steps-not-run" as const;
|
||||
readonly taskId: string;
|
||||
constructor(taskId: string, message = `Cannot merge ${taskId}: ${PRE_MERGE_STEPS_NOT_RUN_BLOCKER}`) {
|
||||
super(message);
|
||||
this.name = "PreMergeStepsNotRunError";
|
||||
this.taskId = taskId;
|
||||
}
|
||||
}
|
||||
|
||||
/** True when a `getTaskMergeBlocker` reason is the deferrable unrun-gate reason. */
|
||||
export function isPreMergeStepsNotRunBlocker(blocker: string | undefined): boolean {
|
||||
return blocker === PRE_MERGE_STEPS_NOT_RUN_BLOCKER;
|
||||
}
|
||||
|
||||
export const TASK_DONE_BYPASS_BLOCKER_MESSAGE =
|
||||
"done bypass requires merge confirmation or explicit no-commits policy";
|
||||
|
||||
@@ -426,7 +464,7 @@ export function getTaskMergeBlocker(
|
||||
(workflowStepId) => !(task.workflowStepResults ?? []).some((result) => result.workflowStepId === workflowStepId),
|
||||
)
|
||||
) {
|
||||
return "task has enabled pre-merge workflow steps that never ran";
|
||||
return PRE_MERGE_STEPS_NOT_RUN_BLOCKER;
|
||||
}
|
||||
|
||||
// Only pre-merge workflow step failures block merge.
|
||||
|
||||
@@ -11,7 +11,7 @@ import {existsSync} from "node:fs";
|
||||
import type {Task, MergeResult, MergeQueueEntry, MergeQueueAcquireOptions} from "../types.js";
|
||||
import {assertNotWorkspaceTaskMerge} from "../types.js";
|
||||
import "../builtin-traits.js";
|
||||
import {getTaskMergeBlocker, resolveTaskMergeTarget} from "../merge/task-merge.js";
|
||||
import {getTaskMergeBlocker, isPreMergeStepsNotRunBlocker, PreMergeStepsNotRunError, resolveTaskMergeTarget} from "../merge/task-merge.js";
|
||||
import {resolveRequiredPreMergeStepIds} from "../merge/required-pre-merge-steps.js";
|
||||
import {resolveWorkflowIrForTask} from "../workflows/workflow-ir-resolver.js";
|
||||
import {resolveReviewColumns, resolveTaskLifecycleColumns} from "../workflows/workflow-lifecycle-traits.js";
|
||||
@@ -440,6 +440,8 @@ export async function mergeTaskImpl(store: TaskStore, id: string): Promise<Merge
|
||||
} catch { /* degraded: the board told us nothing, so the legacy id stands */ }
|
||||
const mergeBlocker = getTaskMergeBlocker(task, { reviewColumns, requiredPreMergeStepIds });
|
||||
if (mergeBlocker) {
|
||||
/* FNXC:RequiredPreMergeSteps 2026-08-22-22:40: an unrun enabled gate is a deferral (typed), not a failure. */
|
||||
if (isPreMergeStepsNotRunBlocker(mergeBlocker)) throw new PreMergeStepsNotRunError(id);
|
||||
throw new Error(`Cannot merge ${id}: ${mergeBlocker}`);
|
||||
}
|
||||
|
||||
|
||||
@@ -1,5 +1,11 @@
|
||||
import { beforeEach, describe, expect, it, vi, type MockInstance } from "vitest";
|
||||
import { validateCustomFieldPatch, type Settings, type Task } from "@fusion/core";
|
||||
import {
|
||||
PreMergeStepsNotRunError,
|
||||
PRE_MERGE_STEPS_NOT_RUN_BLOCKER,
|
||||
validateCustomFieldPatch,
|
||||
type Settings,
|
||||
type Task,
|
||||
} from "@fusion/core";
|
||||
|
||||
const testState = vi.hoisted(() => {
|
||||
class MockVerificationError extends Error {
|
||||
@@ -390,6 +396,53 @@ describe("ProjectEngine merge error recovery", () => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:RequiredPreMergeSteps 2026-08-22-22:40 (FN-9191 wedge):
|
||||
SYMPTOM: FN-9191 sat `in-review` with `status:"failed"` and
|
||||
`error: "Cannot merge FN-9191: task has enabled pre-merge workflow steps that never ran"`,
|
||||
even though BOTH enabled gates (Plan Review, Code Review) later ran and APPROVED. The sweep
|
||||
enqueued the card ~2s after `fn_task_done`, ~18s before the graph started its own Code Review
|
||||
node; the door refused correctly, and THIS error path turned a not-yet answer into a terminal
|
||||
park. Every later merge — including the graph's own merge node at 02:04:38 — then died on
|
||||
`task is marked 'failed'`.
|
||||
ASSERTION: a `PreMergeStepsNotRunError` writes no status, burns no retry, and moves nothing.
|
||||
*/
|
||||
it("defers (does not park) when a merge door refuses only because a pre-merge gate has not run", async () => {
|
||||
vi.useFakeTimers();
|
||||
const setTimeoutSpy = vi.spyOn(globalThis, "setTimeout");
|
||||
const store = makeStore({ tasks: [makeTask({ mergeRetries: 0 }), makeTask({ mergeRetries: 0 })] });
|
||||
vi.mocked(runAiMerge).mockRejectedValueOnce(new PreMergeStepsNotRunError(TASK_ID));
|
||||
|
||||
const engine = createEngine(store);
|
||||
await runMergeCycle(engine);
|
||||
|
||||
expect(store.updateTask).not.toHaveBeenCalled();
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
expect(store.addTaskComment).not.toHaveBeenCalled();
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
TASK_ID,
|
||||
expect.stringContaining(PRE_MERGE_STEPS_NOT_RUN_BLOCKER),
|
||||
"MergeDeferredPendingPreMergeSteps",
|
||||
);
|
||||
expect(store.logEntry).not.toHaveBeenCalledWith(TASK_ID, expect.any(String), "MergeNonConflictFailure");
|
||||
expect(setTimeoutSpy).not.toHaveBeenCalledWith(expect.any(Function), expect.any(Number));
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("still parks other non-conflict merge failures as failed", async () => {
|
||||
const store = makeStore({ tasks: [makeTask({ mergeRetries: 0 }), makeTask({ mergeRetries: 0 })] });
|
||||
vi.mocked(runAiMerge).mockRejectedValueOnce(new Error("remote rejected the push"));
|
||||
|
||||
const engine = createEngine(store);
|
||||
await runMergeCycle(engine);
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith(
|
||||
TASK_ID,
|
||||
expect.objectContaining({ status: "failed", error: expect.stringContaining("remote rejected the push") }),
|
||||
);
|
||||
expect(store.logEntry).toHaveBeenCalledWith(TASK_ID, expect.any(String), "MergeNonConflictFailure");
|
||||
});
|
||||
|
||||
it("logs when bouncing fails after conflict retries are exhausted", async () => {
|
||||
const store = makeStore({
|
||||
tasks: [makeTask({ mergeRetries: 2 }), makeTask({ mergeRetries: 3 })],
|
||||
|
||||
@@ -48,6 +48,8 @@ import {
|
||||
getPlannerInterventionTimeline,
|
||||
getPrimaryPrInfo,
|
||||
getTaskMergeBlocker,
|
||||
isPreMergeStepsNotRunBlocker,
|
||||
PreMergeStepsNotRunError,
|
||||
normalizeMergeAdvanceAutoSyncMode,
|
||||
resolvePersistAgentThinkingLog,
|
||||
resolveTaskMergeTarget,
|
||||
@@ -1432,6 +1434,8 @@ export async function runAiMerge(
|
||||
reviewColumns: aiReviewColumns,
|
||||
requiredPreMergeStepIds,
|
||||
});
|
||||
/* FNXC:RequiredPreMergeSteps 2026-08-22-22:40: an unrun enabled gate is a deferral (typed), not a failure. */
|
||||
if (blocker && isPreMergeStepsNotRunBlocker(blocker)) throw new PreMergeStepsNotRunError(taskId);
|
||||
if (blocker) throw new Error(`Cannot merge ${taskId}: ${blocker}`);
|
||||
|
||||
const settings = await store.getSettings();
|
||||
|
||||
@@ -91,6 +91,8 @@ import {
|
||||
buildTaskLineageTrailer,
|
||||
evaluateNoCommitsNoOpFinalize,
|
||||
getTaskMergeBlocker,
|
||||
isPreMergeStepsNotRunBlocker,
|
||||
PreMergeStepsNotRunError,
|
||||
normalizeMergeConflictStrategy,
|
||||
normalizeMergeStrategyOverlapBehavior,
|
||||
normalizePostMergeAuditMode,
|
||||
@@ -6817,6 +6819,8 @@ export async function aiMergeTask(
|
||||
requiredPreMergeStepIds,
|
||||
});
|
||||
if (mergeBlocker) {
|
||||
/* FNXC:RequiredPreMergeSteps 2026-08-22-22:40: an unrun enabled gate is a deferral (typed), not a failure — see PreMergeStepsNotRunError. */
|
||||
if (isPreMergeStepsNotRunBlocker(mergeBlocker)) throw new PreMergeStepsNotRunError(taskId);
|
||||
throw new Error(`Cannot merge ${taskId}: ${mergeBlocker}`);
|
||||
}
|
||||
|
||||
|
||||
@@ -35,7 +35,7 @@ import {
|
||||
getTaskHardMergeBlocker,
|
||||
PreMergeStepsNotRunError,
|
||||
PRE_MERGE_STEPS_NOT_RUN_BLOCKER,
|
||||
resolveRequiredPreMergeStepIds,
|
||||
findUnrunRequiredPreMergeStepIds,
|
||||
isLiveSharedBranchGroupMemberIntegration,
|
||||
isSharedBranchGroupMemberIntegration,
|
||||
isWorkspaceTask,
|
||||
@@ -3435,8 +3435,31 @@ export class ProjectEngine {
|
||||
);
|
||||
}) as Task[];
|
||||
const allowFlags = await Promise.all(candidates.map((t) => this.allowInReviewMergeProcessing(t, settings, this.runtime.getTaskStore())));
|
||||
/*
|
||||
FNXC:RequiredPreMergeSteps 2026-08-22-22:40 (FN-9191 wedge):
|
||||
Admission cannot see unrun pre-merge gates: `canMergeTask` is sync and the injected
|
||||
`getTaskMergeBlocker` has no workflow IR, so it answers on RESULT ROWS only — and a gate that
|
||||
has not started yet has no row. FN-9191 was enqueued ~2s after `fn_task_done`, ~18s before its
|
||||
Code Review node started, and the door then had to refuse it.
|
||||
|
||||
This sweep already resolves each card's IR, so ask the same question the door asks and hold the
|
||||
card out of the queue until every enabled pre-merge group has a result. Failure to resolve the
|
||||
IR admits the card (the door remains the authority); this filter exists to stop the race, not
|
||||
to become a second gate.
|
||||
*/
|
||||
const unrunGateFlags = await Promise.all(
|
||||
candidates.map(async (t) => {
|
||||
try {
|
||||
const ir = await resolveWorkflowIrForTask(this.runtime.getTaskStore(), t.id, reviewLaneIrCache);
|
||||
if (!ir) return false;
|
||||
return findUnrunRequiredPreMergeStepIds(ir, t).length > 0;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}),
|
||||
);
|
||||
const eligible = sortTasksByPriorityThenAgeAndId(
|
||||
candidates.filter((_, i) => allowFlags[i]),
|
||||
candidates.filter((_, i) => allowFlags[i] && !unrunGateFlags[i]),
|
||||
);
|
||||
for (const t of eligible) {
|
||||
this.internalEnqueueMerge(t.id);
|
||||
|
||||
Reference in New Issue
Block a user