feat(FN-4019): prevent merge-confirmed finalization loops via self-healing
Added self-healing logic to prevent merge-confirmed tasks from re-entering the executor, with test coverage for both merge error recovery and self-healing behavior, plus corresponding architecture and task-management docs. Fusion-Task-Id: FN-4019
This commit is contained in:
@@ -1,5 +1,5 @@
|
||||
import { beforeEach, describe, expect, it, vi, type MockInstance } from "vitest";
|
||||
import type { Settings } from "@fusion/core";
|
||||
import type { Settings, Task } from "@fusion/core";
|
||||
|
||||
const testState = vi.hoisted(() => {
|
||||
class MockVerificationError extends Error {
|
||||
@@ -49,6 +49,7 @@ type MockTask = {
|
||||
mergeRetries: number;
|
||||
status: string | null;
|
||||
error: string | null;
|
||||
steps?: Array<{ status: string }>;
|
||||
mergeDetails?: { mergeConfirmed?: boolean } | null;
|
||||
verificationFailureCount?: number;
|
||||
mergeConflictBounceCount?: number;
|
||||
@@ -137,6 +138,7 @@ function createEngine(
|
||||
options: {
|
||||
getMergeStrategy?: (settings: Settings) => "direct" | "pull-request";
|
||||
processPullRequestMerge?: (...args: unknown[]) => Promise<"merged" | "waiting" | "skipped">;
|
||||
getTaskMergeBlocker?: (task: Task) => string | null | undefined;
|
||||
} = {},
|
||||
): ProjectEngine {
|
||||
testState.currentStore = store;
|
||||
@@ -598,6 +600,35 @@ describe("ProjectEngine merge error recovery", () => {
|
||||
expect(hasErrorLog(errorSpy, "after non-conflict error")).toBe(false);
|
||||
});
|
||||
|
||||
it("parks merge-confirmed tasks in stable failed state when finalization is blocked by incomplete steps", async () => {
|
||||
const store = makeStore({
|
||||
tasks: [
|
||||
makeTask({
|
||||
mergeDetails: { mergeConfirmed: true },
|
||||
steps: [{ status: "in-progress" }],
|
||||
}),
|
||||
],
|
||||
});
|
||||
|
||||
const engine = createEngine(store, {
|
||||
getTaskMergeBlocker: (task) =>
|
||||
task.steps?.some((step) => step.status === "in-progress")
|
||||
? "task has incomplete steps"
|
||||
: undefined,
|
||||
});
|
||||
await runMergeCycle(engine);
|
||||
|
||||
expect(store.moveTask).not.toHaveBeenCalledWith(TASK_ID, "done");
|
||||
expect(store.updateTask).toHaveBeenCalledWith(TASK_ID, {
|
||||
status: "failed",
|
||||
error: "Merge confirmed but finalization blocked: task has incomplete steps",
|
||||
});
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
TASK_ID,
|
||||
expect.stringContaining("finalization blocked"),
|
||||
);
|
||||
});
|
||||
|
||||
it("does not park merge-confirmed tasks as failed when finalize loses in-review ownership", async () => {
|
||||
const store = makeStore({
|
||||
tasks: [
|
||||
@@ -615,7 +646,7 @@ describe("ProjectEngine merge error recovery", () => {
|
||||
await runMergeCycle(engine);
|
||||
|
||||
expect(store.moveTask).toHaveBeenCalledWith(TASK_ID, "done");
|
||||
expect(store.updateTask).toHaveBeenCalledWith(TASK_ID, { status: null });
|
||||
expect(store.updateTask).toHaveBeenCalledWith(TASK_ID, { status: null, error: null });
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith(TASK_ID, {
|
||||
status: "failed",
|
||||
mergeRetries: 3,
|
||||
|
||||
@@ -3039,6 +3039,43 @@ describe("SelfHealingManager", () => {
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("parks merge-confirmed tasks when finalization is blocked by incomplete steps", async () => {
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
});
|
||||
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
id: "FN-353",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
status: null,
|
||||
error: null,
|
||||
mergeDetails: {
|
||||
mergeConfirmed: true,
|
||||
mergedAt: "2026-01-01T00:00:00.000Z",
|
||||
},
|
||||
steps: [{ status: "in-progress" }],
|
||||
log: [],
|
||||
},
|
||||
]);
|
||||
|
||||
const result = await managerWithRecovery.recoverMergedReviewTasks();
|
||||
|
||||
expect(result).toBe(0);
|
||||
expect(store.moveTask).not.toHaveBeenCalledWith("FN-353", "done");
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-353", {
|
||||
status: "failed",
|
||||
error: "Merge confirmed but finalization blocked: task has incomplete steps",
|
||||
});
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-353",
|
||||
expect.stringContaining("finalization blocked"),
|
||||
);
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
});
|
||||
|
||||
describe("recoverStuckMergeDeadlocks", () => {
|
||||
@@ -3340,6 +3377,47 @@ describe("SelfHealingManager", () => {
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("keeps already-landed tasks in-review when merge blocker still reports incomplete steps", async () => {
|
||||
const managerWithRecovery = new SelfHealingManager(store, { rootDir: "/tmp/test-project" });
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({ globalPause: false, enginePaused: false });
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
id: "FN-incomplete",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
status: "failed",
|
||||
mergeRetries: 3,
|
||||
mergeDetails: undefined,
|
||||
baseBranch: "main",
|
||||
branch: "fusion/fn-incomplete",
|
||||
steps: [{ status: "in-progress" }],
|
||||
log: [],
|
||||
},
|
||||
]);
|
||||
mockedExecSync.mockImplementation((command: string | Buffer) => {
|
||||
if (String(command).includes("Fusion-Task-Id: FN-incomplete")) return "abc123\n" as any;
|
||||
return "tip\n" as any;
|
||||
});
|
||||
|
||||
const result = await managerWithRecovery.recoverAlreadyMergedReviewTasks();
|
||||
|
||||
expect(result).toBe(0);
|
||||
expect(store.moveTask).not.toHaveBeenCalledWith("FN-incomplete", "done");
|
||||
expect(store.updateTask).toHaveBeenCalledWith(
|
||||
"FN-incomplete",
|
||||
expect.objectContaining({
|
||||
status: "failed",
|
||||
error: "Merge confirmed but finalization blocked: task has incomplete steps",
|
||||
}),
|
||||
);
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-incomplete",
|
||||
expect.stringContaining("finalization blocked"),
|
||||
);
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
});
|
||||
|
||||
describe("recoverReviewTasksWithFailedPreMergeSteps", () => {
|
||||
|
||||
@@ -1073,8 +1073,12 @@ export class ProjectEngine {
|
||||
updatedAt?: string | null;
|
||||
mergeDetails?: { mergeConfirmed?: boolean } | null;
|
||||
}): boolean {
|
||||
// Already-confirmed merges always eligible — just need to move to done
|
||||
if (task.mergeDetails?.mergeConfirmed) return true;
|
||||
// Merge-confirmed tasks use the fast-path finalizer, which applies blocker
|
||||
// checks after clearing transient status/error state. Once that path parks
|
||||
// a blocked task as failed, skip future auto-merge retries.
|
||||
if (task.mergeDetails?.mergeConfirmed) {
|
||||
return task.status !== "failed";
|
||||
}
|
||||
if (this.options.getTaskMergeBlocker?.(task as Task)) return false;
|
||||
// Terminal failure: don't let the cooldown sweep re-attempt a merge that
|
||||
// already gave up (verification cap, conflict-bounce cap, or non-conflict
|
||||
@@ -1262,6 +1266,26 @@ export class ProjectEngine {
|
||||
// in-review by auto-recovery after a successful merge) — just
|
||||
// complete the task without re-running the merge process.
|
||||
if (task.mergeDetails?.mergeConfirmed) {
|
||||
const blockerReason = this.options.getTaskMergeBlocker?.({
|
||||
...(task as Task),
|
||||
status: undefined,
|
||||
error: undefined,
|
||||
});
|
||||
if (blockerReason) {
|
||||
await store.updateTask(taskId, {
|
||||
status: "failed",
|
||||
error: `Merge confirmed but finalization blocked: ${blockerReason}`,
|
||||
});
|
||||
await store.logEntry(
|
||||
taskId,
|
||||
`Merge confirmed finalization blocked — ${blockerReason}. Task parked in in-review for manual completion.`,
|
||||
);
|
||||
runtimeLog.warn(
|
||||
`Auto-merge: ${taskId} merge-confirmed finalize blocked — ${blockerReason}`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
runtimeLog.log(
|
||||
`Auto-merge: ${taskId} already has mergeConfirmed — moving to done`,
|
||||
);
|
||||
@@ -1269,7 +1293,7 @@ export class ProjectEngine {
|
||||
taskId,
|
||||
"Merge already confirmed; completing task (recovered from post-merge state inconsistency)",
|
||||
);
|
||||
await store.updateTask(taskId, { status: null });
|
||||
await store.updateTask(taskId, { status: null, error: null });
|
||||
try {
|
||||
await store.moveTask(taskId, "done");
|
||||
} catch (error) {
|
||||
|
||||
@@ -1875,6 +1875,25 @@ export class SelfHealingManager {
|
||||
let recovered = 0;
|
||||
for (const task of mergedButNotDone) {
|
||||
try {
|
||||
const blocker = getTaskMergeBlocker({
|
||||
...task,
|
||||
status: undefined,
|
||||
error: undefined,
|
||||
steps: task.steps ?? [],
|
||||
workflowStepResults: task.workflowStepResults,
|
||||
});
|
||||
if (blocker) {
|
||||
await this.store.updateTask(task.id, {
|
||||
status: "failed",
|
||||
error: `Merge confirmed but finalization blocked: ${blocker}`,
|
||||
});
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`Auto-recovery skipped: merge confirmed but finalization blocked — ${blocker}`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
await this.store.updateTask(task.id, {
|
||||
status: null,
|
||||
error: null,
|
||||
@@ -2070,6 +2089,26 @@ export class SelfHealingManager {
|
||||
prNumber: task.prInfo?.number,
|
||||
};
|
||||
|
||||
const blocker = getTaskMergeBlocker({
|
||||
...task,
|
||||
status: undefined,
|
||||
error: undefined,
|
||||
steps: task.steps ?? [],
|
||||
workflowStepResults: task.workflowStepResults,
|
||||
});
|
||||
if (blocker) {
|
||||
await this.store.updateTask(task.id, {
|
||||
status: "failed",
|
||||
error: `Merge confirmed but finalization blocked: ${blocker}`,
|
||||
mergeDetails,
|
||||
});
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`Auto-recovery parked task in in-review: merged content found on ${baseBranch} (${landed.sha.slice(0, 8)}) but finalization blocked — ${blocker}`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
await this.store.updateTask(task.id, {
|
||||
status: null,
|
||||
error: null,
|
||||
|
||||
Reference in New Issue
Block a user