feat(FN-3050): preserve failed review tasks during recovery to prevent loop

The merge strengthens task recovery logic to prevent in-review tasks from entering merge-recovery loops and preserves failed review tasks during retry sweeps instead of incorrectly resetting them. It adds targeted test coverage for these edge cases in the self-healing and project engine modules.

Fusion-Task-Id: FN-3050

docs/architecture.md

@@ -329,6 +329,8 @@ See [Memory Plugin Contract](./memory-plugin-contract.md) for the full plan.
 - `GridlockDetector` (`gridlock-detector.ts`) — detects all-blocked todo pipelines and emits notification events (plus explicit clear signals when gridlock resolves)
 - `TransientErrorDetector` (`transient-error-detector.ts`) — retriable error classification
 - `SelfHealingManager` (`self-healing.ts`) — auto-unpause/maintenance recovery actions
+  - `recoverGhostReviewTasks()` is a fallback only for idle, non-terminal `in-review` states. Terminal/actionable states (notably `status: "failed"`) are preserved and **not** auto-kicked back to `todo`.
+  - `recoverMergeableReviewTasks()` only re-enqueues truly eligible tasks; retry-exhausted review tasks are skipped to avoid re-enqueue/no-op loops that keep refreshing `updatedAt`.
 - `UsageLimitPauser` (`usage-limit-detector.ts`) and `withRateLimitRetry` (`rate-limit-retry.ts`)
 
 ### Worktree and naming helpers

docs/task-management.md

@@ -59,6 +59,7 @@ Fusion task columns:
 2. **todo** — ready for scheduling
 3. **in-progress** — executor active in isolated worktree
 4. **in-review** — implementation complete; awaiting finalization
+   - 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`).
 5. **done** — merged/finalized
 6. **archived** — preserved history, optionally cleaned from filesystem
 

packages/engine/src/__tests__/project-engine.test.ts

@@ -1205,6 +1205,35 @@ describe("ProjectEngine paused in-review auto-merge behavior", () => {
     await engine.stop();
   });
 
+  it("periodic merge sweep does not re-enqueue failed in-review tasks", async () => {
+    vi.useFakeTimers();
+    const mockStore = createMockStore({ ...baseSettings, autoMerge: true, pollIntervalMs: 15_000 });
+    mocks.currentStore = mockStore.store;
+    const engine = createEngine();
+    const privateEngine = engine as unknown as { internalEnqueueMerge: (taskId: string) => void };
+    const enqueueSpy = vi.spyOn(privateEngine, "internalEnqueueMerge");
+
+    await engine.start();
+    enqueueSpy.mockClear();
+
+    mockStore.store.listTasks.mockResolvedValueOnce([
+      // Retry exhausted + failed (FN-2997 observed state after merge error)
+      { id: "FN-failed", column: "in-review", paused: false, mergeRetries: 3, status: "failed", updatedAt: new Date(Date.now() - 60 * 60_000).toISOString() },
+      // Failed status must block even when retries are below the cap.
+      { id: "FN-failed-low-retries", column: "in-review", paused: false, mergeRetries: 0, status: "failed", updatedAt: new Date().toISOString() },
+      { id: "FN-ready", column: "in-review", paused: false, mergeRetries: 0, status: null, updatedAt: new Date().toISOString() },
+    ]);
+
+    await vi.advanceTimersByTimeAsync(15_000);
+
+    expect(enqueueSpy).toHaveBeenCalledWith("FN-ready");
+    expect(enqueueSpy).not.toHaveBeenCalledWith("FN-failed");
+    expect(enqueueSpy).not.toHaveBeenCalledWith("FN-failed-low-retries");
+
+    await engine.stop();
+    vi.useRealTimers();
+  });
+
   it("engine unpause sweep does not enqueue paused in-review tasks", async () => {
     const mockStore = createMockStore({ ...baseSettings, autoMerge: true });
     mocks.currentStore = mockStore.store;

packages/engine/src/__tests__/self-healing.test.ts

@@ -1694,6 +1694,46 @@ describe("SelfHealingManager", () => {
       managerWithRecovery.stop();
     });
 
+    it("does not re-enqueue retry-exhausted review tasks", 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-2997",
+          column: "in-review",
+          paused: false,
+          status: null,
+          error: null,
+          mergeRetries: 3,
+          worktree: "/tmp/test-project/.worktrees/fn-2997",
+          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.logEntry).not.toHaveBeenCalledWith(
+        "FN-2997",
+        expect.stringContaining("re-enqueued for merge"),
+      );
+
+      managerWithRecovery.stop();
+    });
+
     it("moves stale in-review tasks with incomplete steps back to todo for retry", async () => {
       const managerWithRecovery = new SelfHealingManager(store, {
         rootDir: "/tmp/test-project",
@@ -2051,7 +2091,7 @@ describe("SelfHealingManager", () => {
   });
 
   describe("recoverGhostReviewTasks", () => {
-    it("kicks idle in-review tasks back to todo regardless of status or worktree", async () => {
+    it("preserves failed in-review tasks so actionable merge failures are not ghost-retried", async () => {
       const managerWithRecovery = new SelfHealingManager(store, {
         rootDir: "/tmp/test-project",
       });
@@ -2075,13 +2115,9 @@ describe("SelfHealingManager", () => {
 
       const result = await managerWithRecovery.recoverGhostReviewTasks();
 
-      expect(result).toBe(1);
-      expect(store.updateTask).toHaveBeenCalledWith("FN-9001", { status: null, error: null });
-      expect(store.moveTask).toHaveBeenCalledWith("FN-9001", "todo");
-      expect(store.logEntry).toHaveBeenCalledWith(
-        "FN-9001",
-        expect.stringContaining("idle past stuck-task timeout"),
-      );
+      expect(result).toBe(0);
+      expect(store.updateTask).not.toHaveBeenCalled();
+      expect(store.moveTask).not.toHaveBeenCalled();
 
       managerWithRecovery.stop();
     });

packages/engine/src/self-healing.ts

@@ -84,6 +84,7 @@ const NON_TERMINAL_STEP_STATUSES = new Set(["pending", "in-progress"]);
 /** Statuses that represent an explicit human-handoff or active merge —
  *  the ghost-review fallback must not disturb tasks parked in these states. */
 const GHOST_REVIEW_PRESERVED_STATUSES = new Set([
+  "failed",
   "awaiting-user-review",
   "awaiting-approval",
   "merging",
@@ -103,6 +104,7 @@ const ORPHANED_WITH_WORKTREE_GRACE_MS = 300_000;
  * forever; when exhausted the task stays in `in-review` for human inspection.
  */
 const MAX_TASK_DONE_RETRIES = 3;
+const MAX_AUTO_MERGE_RETRIES = 3;
 
 interface LandedTaskCommit {
   sha: string;
@@ -803,6 +805,11 @@ export class SelfHealingManager {
         !t.paused &&
         Boolean(t.worktree) &&
         t.mergeDetails?.mergeConfirmed !== true &&
+        // Mirror ProjectEngine.canMergeTask retry gate. If retries are already
+        // exhausted, re-enqueueing here is a no-op and each recovery log write
+        // refreshes updatedAt, preventing cooldown-based retries from ever
+        // becoming eligible.
+        (t.mergeRetries ?? 0) < MAX_AUTO_MERGE_RETRIES &&
         getTaskMergeBlocker(t) === undefined,
       );