feat(FN-5455): merge fusion/fn-5455

.changeset/FN-5455-pr-mode-pool-lease-release.md

@@ -0,0 +1,5 @@
+---
+"@runfusion/fusion": patch
+---
+
+PR-mode merge cleanup (`cleanupMergedTaskArtifacts`) now releases the `WorktreePool` lease for the merged task worktree before removing it, preventing stranded lease bookkeeping after pull-request merges (FN-5420 / FN-4954 follow-up).

packages/cli/src/commands/__tests__/task-lifecycle.test.ts

@@ -23,7 +23,11 @@ vi.mock("node:child_process", () => ({
   },
 }));
 
-import { processPullRequestMergeTask, getTaskBranchName } from "../task-lifecycle.js";
+import {
+  cleanupMergedTaskArtifacts,
+  processPullRequestMergeTask,
+  getTaskBranchName,
+} from "../task-lifecycle.js";
 
 interface MockTask {
   id: string;
@@ -915,3 +919,41 @@ describe("processPullRequestMergeTask", () => {
     });
   });
 });
+
+describe("cleanupMergedTaskArtifacts FN-5455", () => {
+  beforeEach(() => {
+    execMock.mockReset();
+    execMock.mockReturnValue("");
+  });
+
+  it("FN-5455: releases pool lease before removing worktree and deleting branch", async () => {
+    const pool = { release: vi.fn() };
+    await cleanupMergedTaskArtifacts("/repo", { id: "FN-5455-A", worktree: "/repo/wt" } as never, { pool } as never);
+    expect(pool.release).toHaveBeenCalledWith("/repo/wt", "FN-5455-A");
+    expect(execMock).toHaveBeenCalledWith(expect.stringContaining('git worktree remove "/repo/wt" --force'), expect.any(Object));
+    expect(execMock).toHaveBeenCalledWith(expect.stringContaining('git branch -d "fusion/fn-5455-a"'), expect.any(Object));
+  });
+
+  it("FN-5455: pool omitted keeps backward-compatible cleanup behavior", async () => {
+    await cleanupMergedTaskArtifacts("/repo", { id: "FN-5455-B", worktree: "/repo/wt-b" } as never);
+    expect(execMock).toHaveBeenCalledWith(expect.stringContaining('git worktree remove "/repo/wt-b" --force'), expect.any(Object));
+    expect(execMock).toHaveBeenCalledWith(expect.stringContaining('git branch -d "fusion/fn-5455-b"'), expect.any(Object));
+  });
+
+  it("FN-5455: undefined worktree skips pool interaction and worktree removal", async () => {
+    const pool = { release: vi.fn() };
+    await cleanupMergedTaskArtifacts("/repo", { id: "FN-5455-C", worktree: undefined } as never, { pool } as never);
+    expect(pool.release).not.toHaveBeenCalled();
+    expect(execMock).not.toHaveBeenCalledWith(expect.stringContaining("git worktree remove"), expect.anything());
+    expect(execMock).toHaveBeenCalledWith(expect.stringContaining('git branch -d "fusion/fn-5455-c"'), expect.any(Object));
+  });
+
+  it("FN-5455: release errors are swallowed and cleanup continues", async () => {
+    const pool = { release: vi.fn(() => { throw new Error("boom"); }) };
+    await expect(
+      cleanupMergedTaskArtifacts("/repo", { id: "FN-5455-D", worktree: "/repo/wt-d" } as never, { pool } as never),
+    ).resolves.toBeUndefined();
+    expect(execMock).toHaveBeenCalledWith(expect.stringContaining('git worktree remove "/repo/wt-d" --force'), expect.any(Object));
+    expect(execMock).toHaveBeenCalledWith(expect.stringContaining('git branch -d "fusion/fn-5455-d"'), expect.any(Object));
+  });
+});

packages/cli/src/commands/daemon.ts

@@ -332,8 +332,8 @@ export async function runDaemon(opts: DaemonOptions = {}) {
 
   const engineManager = new ProjectEngineManager(sharedCentralCore, {
     getMergeStrategy,
-    processPullRequestMerge: (s, wd, taskId) =>
+    processPullRequestMerge: (s, wd, taskId, pool) =>
-      processPullRequestMergeTask(s, wd, taskId, githubClient, getTaskMergeBlocker),
+      processPullRequestMergeTask(s, wd, taskId, githubClient, getTaskMergeBlocker, pool),
     getTaskMergeBlocker,
     onInsightRunProcessed: (s: unknown, r: unknown) => onMemoryInsightRunProcessed(s as ScheduledTask, r as AutomationRunResult),
   });

packages/cli/src/commands/dashboard.ts

@@ -1197,7 +1197,14 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?:
     const settings = await store.getSettings();
     if (getMergeStrategy(settings) === "pull-request") {
       const githubClient = new GitHubClient();
-      const outcome = await processPullRequestMergeTask(store, cwd, taskId, githubClient, getTaskMergeBlocker);
+      const outcome = await processPullRequestMergeTask(
+        store,
+        cwd,
+        taskId,
+        githubClient,
+        getTaskMergeBlocker,
+        worktreePool,
+      );
       const task = await store.getTask(taskId);
       return {
         task,
@@ -1510,8 +1517,8 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?:
 
     const engineManager = new ProjectEngineManager(centralCoreForEngine, {
       getMergeStrategy,
-      processPullRequestMerge: (s, wd, taskId) =>
+      processPullRequestMerge: (s, wd, taskId, pool) =>
-        processPullRequestMergeTask(s, wd, taskId, githubClient, getTaskMergeBlocker),
+        processPullRequestMergeTask(s, wd, taskId, githubClient, getTaskMergeBlocker, pool),
       getTaskMergeBlocker,
     });
 

packages/cli/src/commands/serve.ts

@@ -358,8 +358,8 @@ export async function runServe(
 
   const engineManager = new ProjectEngineManager(sharedCentralCore, {
     getMergeStrategy,
-    processPullRequestMerge: (s, wd, taskId) =>
+    processPullRequestMerge: (s, wd, taskId, pool) =>
-      processPullRequestMergeTask(s, wd, taskId, githubClient, getTaskMergeBlocker),
+      processPullRequestMergeTask(s, wd, taskId, githubClient, getTaskMergeBlocker, pool),
     getTaskMergeBlocker,
     onInsightRunProcessed: (s: unknown, r: unknown) => onMemoryInsightRunProcessed(s as ScheduledTask, r as AutomationRunResult),
   });

packages/cli/src/commands/task-lifecycle.ts

@@ -20,6 +20,7 @@ import type { TaskStore } from "@fusion/core";
 import { resolveTaskMergeTarget } from "@fusion/core";
 import type { Settings, TaskDetail, PrInfo, MergeResult } from "@fusion/core";
 import { resolveIntegrationBranch } from "@fusion/engine";
+import type { WorktreePool } from "@fusion/engine";
 
 /**
  * Minimal interface for GitHub operations needed by the PR merge workflow.
@@ -140,10 +141,26 @@ function buildPullRequestBody(task: Pick<TaskDetail, "id" | "description">): str
  * Clean up worktree and branch artifacts after a successful merge.
  * Both operations are best-effort; errors are logged but don't propagate.
  */
-export async function cleanupMergedTaskArtifacts(cwd: string, task: Pick<TaskDetail, "id" | "worktree">): Promise<void> {
+/**
+ * @param options.pool Optional runtime worktree pool; FN-5455/FN-4954 require best-effort
+ * release before force-removing merged PR worktrees.
+ */
+export async function cleanupMergedTaskArtifacts(
+  cwd: string,
+  task: Pick<TaskDetail, "id" | "worktree">,
+  options?: { pool?: WorktreePool },
+): Promise<void> {
   const branch = getTaskBranchName(task.id);
 
   if (task.worktree) {
+    if (options?.pool) {
+      try {
+        options.pool.release(task.worktree, task.id);
+      } catch {
+        // Best-effort cleanup — release may fail if pool state is already divergent.
+      }
+    }
+
     try {
       await execAsync(`git worktree remove "${task.worktree}" --force`, {
         cwd,
@@ -177,8 +194,9 @@ async function finalizePullRequestMerge(
   task: TaskDetail,
   prInfo: PrInfo,
   message = "Pull request merged",
+  pool?: WorktreePool,
 ): Promise<void> {
-  await cleanupMergedTaskArtifacts(cwd, task);
+  await cleanupMergedTaskArtifacts(cwd, task, { pool });
   await store.updateTask(task.id, { status: null, mergeRetries: 0 });
   const movedTask = await store.moveTask(task.id, "done");
   const mergedTask = movedTask ?? (await store.getTask(task.id));
@@ -240,6 +258,7 @@ export async function processPullRequestMergeTask(
   taskId: string,
   github: GitHubOperations,
   getTaskMergeBlocker: TaskMergeBlockerFn,
+  pool?: WorktreePool,
 ): Promise<ProcessPullRequestResult> {
   const task = await store.getTask(taskId);
   if (getTaskMergeBlocker(task)) {
@@ -304,7 +323,7 @@ export async function processPullRequestMergeTask(
   await store.updatePrInfo(task.id, refreshedPrInfo);
 
   if (mergeStatus.prInfo.status === "merged") {
-    await finalizePullRequestMerge(store, cwd, task, prInfo);
+    await finalizePullRequestMerge(store, cwd, task, prInfo, "Pull request merged", pool);
     return "merged";
   }
 
@@ -359,6 +378,7 @@ export async function processPullRequestMergeTask(
         task,
         refreshedAfterFailure,
         "Pull request already merged after merge command failed; reconciled task state from GitHub",
+        pool,
       );
       return "merged";
     }
@@ -366,6 +386,6 @@ export async function processPullRequestMergeTask(
     throw err;
   }
   await store.updatePrInfo(task.id, { ...mergedPr, lastCheckedAt: new Date().toISOString() });
-  await finalizePullRequestMerge(store, cwd, task, mergedPr);
+  await finalizePullRequestMerge(store, cwd, task, mergedPr, "Pull request merged", pool);
   return "merged";
 }

packages/engine/src/__tests__/reliability-interactions/pr-mode-worktree-invariants.test.ts

@@ -239,18 +239,54 @@ describe("FN-5420 reliability interactions: PR mode worktree invariants", () =>
     }
   });
 
-  it.fails("FN-5420/FN-4954/FN-4811 follow-up required: cleanup should release pool lease and clear active-session registry", async () => {
+  it("FN-5455: PR-mode cleanupMergedTaskArtifacts releases WorktreePool lease for task worktree", async () => {
     const pool = new WorktreePool();
-    const path = "/tmp/fn-5420-leased";
+    const path = "/tmp/fn-5455-leased";
-    (pool as any).getLeasedPaths().set(path, "FN-5420-LEAK");
+    (pool as any).getLeasedPaths().set(path, "FN-5455-LEAK");
-    activeSessionRegistry.registerPath(path, { taskId: "FN-5420-LEAK", kind: "executor", ownerKey: "FN-5420-LEAK" });
 
-    const spy = vi.spyOn((pool as any).constructor.prototype, "release");
+    const releaseSpy = vi.spyOn(pool, "release");
     const { cleanupMergedTaskArtifacts } = await loadPrLifecycleModule();
-    await cleanupMergedTaskArtifacts("/tmp", { id: "FN-5420-LEAK", worktree: path } as any);
+    await cleanupMergedTaskArtifacts("/tmp", { id: "FN-5455-LEAK", worktree: path } as any, { pool });
 
-    expect(spy).toHaveBeenCalled();
+    expect(releaseSpy).toHaveBeenCalledTimes(1);
-    expect(activeSessionRegistry.lookupByPath(path)).toBeNull();
+    expect(releaseSpy).toHaveBeenCalledWith(path, "FN-5455-LEAK");
+    expect(pool.getLeasedPaths().has(path)).toBe(false);
+    expect(pool.getLeasedPaths().get(path)).toBeUndefined();
+  });
+
+  it("FN-5455: cleanupMergedTaskArtifacts is a no-op for pool when options.pool is omitted", async () => {
+    const { cleanupMergedTaskArtifacts } = await loadPrLifecycleModule();
+    await expect(
+      cleanupMergedTaskArtifacts("/tmp", { id: "FN-5455-NO-POOL", worktree: "/tmp/fn-5455-no-pool" } as any),
+    ).resolves.toBeUndefined();
+  });
+
+  it("FN-5455: cleanupMergedTaskArtifacts calls pool.release even when worktree directory is already gone", async () => {
+    const pool = new WorktreePool();
+    const path = "/tmp/fn-5455-missing";
+    (pool as any).getLeasedPaths().set(path, "FN-5455-MISSING");
+    const releaseSpy = vi.spyOn(pool, "release");
+    const { cleanupMergedTaskArtifacts } = await loadPrLifecycleModule();
+    await cleanupMergedTaskArtifacts("/tmp", { id: "FN-5455-MISSING", worktree: path } as any, { pool });
+    expect(releaseSpy).toHaveBeenCalledWith(path, "FN-5455-MISSING");
+    expect(pool.getLeasedPaths().has(path)).toBe(false);
+  });
+
+  it("FN-5455: cleanupMergedTaskArtifacts swallows pool.release errors (best-effort)", async () => {
+    const pool = new WorktreePool();
+    const path = "/tmp/fn-5455-release-throws";
+    const releaseSpy = vi.spyOn(pool, "release").mockImplementation(() => {
+      throw new Error("release failed");
+    });
+    const { cleanupMergedTaskArtifacts } = await loadPrLifecycleModule();
+    await expect(cleanupMergedTaskArtifacts("/tmp", { id: "FN-5455-THROW", worktree: path } as any, { pool })).resolves.toBeUndefined();
+    expect(releaseSpy).toHaveBeenCalledWith(path, "FN-5455-THROW");
+  });
+
+  it.skip("FN-5456 follow-up required: cleanup should clear active-session registry entry", async () => {
+    const path = "/tmp/fn-5456-session";
+    activeSessionRegistry.registerPath(path, { taskId: "FN-5456", kind: "executor", ownerKey: "FN-5456" });
+    expect(activeSessionRegistry.lookupByPath(path)).not.toBeNull();
   });
 
   it("FN-5420/FN-5279: mergeIntegrationWorktree setting does not gate PR-mode processing", async () => {

packages/engine/src/project-engine.ts

@@ -14,6 +14,7 @@ import { compareTasksByPriorityThenAgeAndId, getTaskHardMergeBlocker, sortTasksB
 import { execFile } from "node:child_process";
 import { promisify } from "node:util";
 import { InProcessRuntime } from "./runtimes/in-process-runtime.js";
+import type { WorktreePool } from "./worktree-pool.js";
 import type { ProjectRuntimeConfig } from "./project-runtime.js";
 import { PrMonitor } from "./pr-monitor.js";
 import { PrCommentHandler } from "./pr-comment-handler.js";
@@ -55,6 +56,7 @@ export type ProcessPullRequestMergeFn = (
   store: TaskStore,
   cwd: string,
   taskId: string,
+  pool?: WorktreePool,
 ) => Promise<"merged" | "waiting" | "skipped">;
 
 const execFileAsync = promisify(execFile);
@@ -1483,7 +1485,12 @@ export class ProjectEngine {
           if (mergeStrategy === "pull-request" && this.options.processPullRequestMerge) {
             this.activeMergeTaskId = taskId;
             runtimeLog.log(`${manualResolver ? "Manual" : "Auto"}-merge processing PR flow for ${taskId}...`);
-            const result = await this.options.processPullRequestMerge(store, cwd, taskId);
+            const result = await this.options.processPullRequestMerge(
+              store,
+              cwd,
+              taskId,
+              (this.runtime as any).worktreePool,
+            );
             if (result === "merged") {
               runtimeLog.log(`${manualResolver ? "Manual" : "Auto"}-merge PR merged: ${taskId}`);
               const mergedTask = await store.getTask(taskId).catch(() => null);