feat(FN-3951): fix merge recovery re-enqueue guard to prevent auto-merge lo

Hardens merge recovery logic (FN-3951) with guards against invalid state transitions during terminal merge errors, backed by new tests in `merge-error-recovery.test.ts` and `self-healing.test.ts`, and documented in the task management docs.

Fusion-Task-Id: FN-3951
This commit is contained in:
Fusion
2026-05-10 16:57:13 -07:00
committed by gsxdsm
parent 32e76c8cc0
commit 48aea5001b
6 changed files with 108 additions and 1 deletions

View File

@@ -0,0 +1,12 @@
---
"@runfusion/fusion": patch
---
Prevent auto-merge loops on terminal invalid done-transition failures during merge recovery.
When merge finalization encounters a non-recoverable state-machine error like
`Invalid transition: 'todo' → 'done'`, auto-recovery now keeps that task parked
in a stable failed review state instead of repeatedly re-enqueuing it for merge.
The merge-confirmed fast path also now re-checks task ownership and skips
finalization if the task has already left `in-review`.

View File

@@ -94,6 +94,7 @@ Fusion task columns:
- If merge/finalization hits a terminal error, tasks can remain in `in-review` with `status: "failed"` for explicit follow-up. This state is intentionally preserved by recovery (not auto-bounced to `todo`). - If merge/finalization hits a terminal error, tasks can remain in `in-review` with `status: "failed"` for explicit follow-up. This state is intentionally preserved by recovery (not auto-bounced to `todo`).
- Retry behavior splits by step completion: `in-review` tasks with incomplete steps (`pending`/`in-progress`) are treated as execution failures and retried back to `todo` with `preserveProgress: true`; `in-review` tasks with all steps `done` are treated as merge/finalization failures and stay in `in-review` with merge retry state reset. - Retry behavior splits by step completion: `in-review` tasks with incomplete steps (`pending`/`in-progress`) are treated as execution failures and retried back to `todo` with `preserveProgress: true`; `in-review` tasks with all steps `done` are treated as merge/finalization failures and stay in `in-review` with merge retry state reset.
- Self-healing can still auto-finalize retry-exhausted failed review tasks when it can prove their branch content already landed on the merge target, so already-merged work does not deadlock in `in-review`. - Self-healing can still auto-finalize retry-exhausted failed review tasks when it can prove their branch content already landed on the merge target, so already-merged work does not deadlock in `in-review`.
- Non-recoverable state-machine errors during finalization (for example `Invalid transition: 'todo' → 'done'`) are treated as terminal review failures: recovery must not re-enqueue these tasks for merge unless task state changes prove they are recoverable.
5. **done** — merged/finalized 5. **done** — merged/finalized
6. **archived** — preserved history, optionally cleaned from filesystem 6. **archived** — preserved history, optionally cleaned from filesystem

View File

@@ -35,6 +35,7 @@ type MockTask = {
mergeRetries: number; mergeRetries: number;
status: string | null; status: string | null;
error: string | null; error: string | null;
mergeDetails?: { mergeConfirmed?: boolean } | null;
verificationFailureCount?: number; verificationFailureCount?: number;
mergeConflictBounceCount?: number; mergeConflictBounceCount?: number;
branch?: string; branch?: string;
@@ -583,6 +584,35 @@ describe("ProjectEngine merge error recovery", () => {
expect(hasErrorLog(errorSpy, "after non-conflict error")).toBe(false); expect(hasErrorLog(errorSpy, "after non-conflict error")).toBe(false);
}); });
it("does not park merge-confirmed tasks as failed when finalize loses in-review ownership", async () => {
const store = makeStore({
tasks: [
makeTask({
mergeDetails: { mergeConfirmed: true },
}),
makeTask({ column: "todo" }),
],
});
store.moveTask.mockRejectedValueOnce(
new Error("Invalid transition: 'todo' → 'done'. Valid targets: in-progress, triage"),
);
const engine = createEngine(store);
await runMergeCycle(engine);
expect(store.moveTask).toHaveBeenCalledWith(TASK_ID, "done");
expect(store.updateTask).toHaveBeenCalledWith(TASK_ID, { status: null });
expect(store.updateTask).not.toHaveBeenCalledWith(TASK_ID, {
status: "failed",
mergeRetries: 3,
error: expect.stringContaining("Invalid transition"),
});
expect(store.logEntry).toHaveBeenCalledWith(
TASK_ID,
expect.stringContaining("finalize skipped"),
);
});
it("logs when non-conflict direct merge error recovery update fails", async () => { it("logs when non-conflict direct merge error recovery update fails", async () => {
const store = makeStore({ const store = makeStore({
updateTask: vi.fn(async () => { updateTask: vi.fn(async () => {

View File

@@ -2244,6 +2244,42 @@ describe("SelfHealingManager", () => {
managerWithRecovery.stop(); managerWithRecovery.stop();
}); });
it("does not re-enqueue review tasks carrying terminal invalid done-transition errors", async () => {
const enqueueMerge = vi.fn();
const managerWithRecovery = new SelfHealingManager(store, {
rootDir: "/tmp/test-project",
enqueueMerge,
});
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
autoMerge: true,
globalPause: false,
enginePaused: false,
});
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
{
id: "FN-3946",
column: "in-review",
paused: false,
status: null,
error: "Invalid transition: 'todo' → 'done'. Valid targets: in-progress, triage",
mergeRetries: 0,
worktree: "/tmp/test-project/.worktrees/fn-3946",
steps: [{ name: "Ship it", status: "done" }],
workflowStepResults: [{ id: "ws-1", status: "passed", phase: "pre-merge" }],
mergeDetails: undefined,
log: [],
},
]);
const result = await managerWithRecovery.recoverMergeableReviewTasks();
expect(result).toBe(0);
expect(enqueueMerge).not.toHaveBeenCalled();
expect(store.mergeTask).not.toHaveBeenCalled();
managerWithRecovery.stop();
});
it("moves stale in-review tasks with incomplete steps back to todo for retry", async () => { it("moves stale in-review tasks with incomplete steps back to todo for retry", async () => {
const managerWithRecovery = new SelfHealingManager(store, { const managerWithRecovery = new SelfHealingManager(store, {
rootDir: "/tmp/test-project", rootDir: "/tmp/test-project",

View File

@@ -80,6 +80,11 @@ function formatErrorDetails(error: unknown): { message: string; detail: string }
return { message: detail, detail }; return { message: detail, detail };
} }
function isInvalidDoneTransitionError(error: unknown): boolean {
const message = error instanceof Error ? error.message : String(error);
return message.includes("Invalid transition:") && message.includes("→ 'done'");
}
export interface AutomationSubsystemHealth { export interface AutomationSubsystemHealth {
status: "not-initialized" | "initializing" | "ready" | "degraded"; status: "not-initialized" | "initializing" | "ready" | "degraded";
message: string; message: string;
@@ -1265,7 +1270,24 @@ export class ProjectEngine {
"Merge already confirmed; completing task (recovered from post-merge state inconsistency)", "Merge already confirmed; completing task (recovered from post-merge state inconsistency)",
); );
await store.updateTask(taskId, { status: null }); await store.updateTask(taskId, { status: null });
try {
await store.moveTask(taskId, "done"); await store.moveTask(taskId, "done");
} catch (error) {
if (isInvalidDoneTransitionError(error)) {
const latest = await store.getTask(taskId).catch(() => null);
if (latest && latest.column !== "in-review") {
runtimeLog.warn(
`Auto-merge: ${taskId} merge-confirmed finalize skipped — task moved to ${latest.column} before done transition`,
);
await store.logEntry(
taskId,
`Merge confirmed finalize skipped: task moved to '${latest.column}' before in-review → done transition`,
);
continue;
}
}
throw error;
}
continue; continue;
} }

View File

@@ -168,6 +168,11 @@ function parseShortstat(output: string): Pick<LandedTaskCommit, "filesChanged" |
}; };
} }
function hasTerminalInvalidDoneTransition(task: Pick<Task, "error">): boolean {
const error = task.error ?? "";
return error.includes("Invalid transition:") && error.includes("→ 'done'");
}
export class SelfHealingManager { export class SelfHealingManager {
// ── Auto-unpause state ────────────────────────────────────────────── // ── Auto-unpause state ──────────────────────────────────────────────
private unpauseTimer: ReturnType<typeof setTimeout> | null = null; private unpauseTimer: ReturnType<typeof setTimeout> | null = null;
@@ -1181,6 +1186,7 @@ export class SelfHealingManager {
t.status !== "merging-pr" && t.status !== "merging-pr" &&
Boolean(t.worktree) && Boolean(t.worktree) &&
t.mergeDetails?.mergeConfirmed !== true && t.mergeDetails?.mergeConfirmed !== true &&
!hasTerminalInvalidDoneTransition(t) &&
// Mirror ProjectEngine.canMergeTask retry gate. If retries are already // Mirror ProjectEngine.canMergeTask retry gate. If retries are already
// exhausted, re-enqueueing here is a no-op and each recovery log write // exhausted, re-enqueueing here is a no-op and each recovery log write
// refreshes updatedAt, preventing cooldown-based retries from ever // refreshes updatedAt, preventing cooldown-based retries from ever