fix(engine): let concurrent tasks run Plan Review on the shared repo root

Plan Review needs no worktree, so it runs rooted at the project root. The
activeSessionRegistry key was the bare root path, so the second task to reach
Plan Review hit ActiveSessionPathHeldByForeignTaskError ("path ... is held by
task FN-1398; task FN-1403 may not overwrite it"). That surfaced as a Plan
Review provider failure, burned the in-place retry budget against a hold no
retry could clear, and left the task parked.

Task-scope the registry key for any session rooted at rootDir, in every project
mode — the workspace fix already did this for the shared browse-root. Root
exclusivity protects nothing here: write-capable nodes are refused at the root
outright, and every isPathActive consumer guards removable worktree paths.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

.changeset/plan-review-shared-root-session-collision.md

@@ -0,0 +1,7 @@
+---
+"@runfusion/fusion": patch
+---
+
+summary: Two tasks can now run Plan Review at the same time instead of one failing and parking.
+category: fix
+dev: `TaskExecutor.sessionRegistryPath` now task-scopes the activeSessionRegistry key for any session rooted at `rootDir`, not just in workspace mode. Read-only graph nodes (Plan Review) run at the repo root, so the bare-root key made the second concurrent task throw `ActiveSessionPathHeldByForeignTaskError`, which surfaced as a Plan Review provider failure and burned the in-place retry budget.

packages/engine/src/__tests__/executor-archive-releases-active-session.test.ts

@@ -45,10 +45,16 @@ describe("archiving a task releases its active-session registry entries (FN-7717
   it("releases a workflow-step session held by a task archived from triage, letting a successor acquire the same path", async () => {
     const { executor, store } = makeExecutor();
 
-    // Task A registers a workflow-step (Plan Review) session on the shared root — the
-    // reported NEXT-508 case.
+    /*
+    FNXC:PlanReviewWorktree 2026-07-25-20:40:
+    A root-rooted session is now registered under a TASK-SCOPED synthetic key (sessionRegistryPath),
+    so assert release through `pathsForTask` — the bare-root key never exists. What archive must still
+    guarantee is unchanged: the task holds exactly one live entry before, and none after.
+    */
     (executor as any).setActiveWorkflowStepSession("TASK-A", {}, SHARED_ROOT);
-    expect(activeSessionRegistry.isPathActive(SHARED_ROOT)).toBe(true);
+    const [heldPath] = activeSessionRegistry.pathsForTask("TASK-A");
+    expect(heldPath).toBeDefined();
+    expect(activeSessionRegistry.isPathActive(heldPath)).toBe(true);
 
     // Drive the archive transition: to === "archived", from a NON-in-progress column
     // (Plan Review runs in triage), exactly like archiveTask emits.
@@ -57,12 +63,12 @@ describe("archiving a task releases its active-session registry entries (FN-7717
     // Await the disposal chain the handler kicked off via trackTaskDisposal.
     await (executor as any).pendingTaskDisposals.get("TASK-A");
 
-    expect(activeSessionRegistry.isPathActive(SHARED_ROOT)).toBe(false);
+    expect(activeSessionRegistry.isPathActive(heldPath)).toBe(false);
     expect(activeSessionRegistry.pathsForTask("TASK-A")).toHaveLength(0);
 
     // Successor task B can now register the same path without throwing.
     expect(() =>
-      activeSessionRegistry.registerPath(SHARED_ROOT, { taskId: "TASK-B", kind: "workflow-step", ownerKey: "TASK-B#workflow-step" }),
+      activeSessionRegistry.registerPath(heldPath, { taskId: "TASK-B", kind: "workflow-step", ownerKey: "TASK-B#workflow-step" }),
     ).not.toThrow();
   });
 
@@ -149,11 +155,18 @@ describe("archiving a task releases its active-session registry entries (FN-7717
   it("reproduces the original ActiveSessionPathHeldByForeignTaskError before archive, and confirms it is gone after", async () => {
     const { executor, store } = makeExecutor();
 
-    (executor as any).setActiveWorkflowStepSession("NEXT-508", {}, SHARED_ROOT);
+    /*
+    FNXC:PlanReviewWorktree 2026-07-25-20:40:
+    The original NEXT-508 symptom was reproduced on the shared ROOT. That path can no longer collide at
+    all (root keys are task-scoped), so the leak symptom is now reproduced on a per-task WORKTREE path,
+    where the foreign-task guard still applies and only archive can release the holder.
+    */
+    const heldWorktree = `${SHARED_ROOT}-next508-worktree`;
+    (executor as any).setActiveWorkflowStepSession("NEXT-508", {}, heldWorktree);
 
     // Before archive: a second task trying to register the same path is rejected.
     expect(() =>
-      activeSessionRegistry.registerPath(SHARED_ROOT, { taskId: "NEXT-433", kind: "workflow-step", ownerKey: "NEXT-433#workflow-step" }),
+      activeSessionRegistry.registerPath(heldWorktree, { taskId: "NEXT-433", kind: "workflow-step", ownerKey: "NEXT-433#workflow-step" }),
     ).toThrow(ActiveSessionPathHeldByForeignTaskError);
 
     store.emit("task:moved", { task: makeTask("NEXT-508"), from: "triage", to: "archived", source: "user" });
@@ -161,7 +174,7 @@ describe("archiving a task releases its active-session registry entries (FN-7717
 
     // After archive: the successor can now acquire the path.
     expect(() =>
-      activeSessionRegistry.registerPath(SHARED_ROOT, { taskId: "NEXT-433", kind: "workflow-step", ownerKey: "NEXT-433#workflow-step" }),
+      activeSessionRegistry.registerPath(heldWorktree, { taskId: "NEXT-433", kind: "workflow-step", ownerKey: "NEXT-433#workflow-step" }),
     ).not.toThrow();
   });
 });

packages/engine/src/__tests__/executor-root-rooted-concurrent-session.test.ts

@@ -0,0 +1,106 @@
+/*
+FNXC:PlanReviewWorktree 2026-07-25-20:40 (concurrent root-rooted step sessions — single-repo regression):
+Read-only graph nodes that need no worktree run rooted at the executor's `rootDir`, and a todo task has
+no worktree of its own. Plan Review is the canonical one. With the bare root as the activeSessionRegistry
+key, the SECOND task to reach such a node failed with "active-session path <root> is held by task <other>;
+task <self> may not overwrite it" — narrated as a Plan Review provider failure, retried in place against a
+hold retrying can never clear, then parked once the budget was spent (reported: FN-1398 holding
+/home/ubuntu/dev/freemap-svelte while FN-1403 planned).
+
+Invariant under test: on a plain single-repo project (NO workspaceConfig — the workspace fix did not cover
+this), two different tasks register root-rooted sessions concurrently across ALL THREE registration
+surfaces (executor / step-session / workflow-step) without collision, each stays discoverable by liveness,
+and each delete surface cleans up its synthetic key. Negative control: a genuine shared per-task worktree
+path (not the root) still rejects the foreign-task overwrite.
+*/
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { EventEmitter } from "node:events";
+import type { TaskStore } from "@fusion/core";
+import { TaskExecutor } from "../executor.js";
+import { activeSessionRegistry, ActiveSessionPathHeldByForeignTaskError } from "../active-session-registry.js";
+
+const ROOT = "/tmp/fusion-test-single-repo-project-root";
+
+function createStore(): TaskStore & EventEmitter {
+  const emitter = new EventEmitter();
+  return Object.assign(emitter, {
+    logEntry: vi.fn().mockResolvedValue(undefined),
+    getRunContextFor: vi.fn(),
+    getSettings: vi.fn().mockResolvedValue({}),
+  }) as unknown as TaskStore & EventEmitter;
+}
+
+/** Plain single-repo executor: no workspaceConfig, so the pre-fix bare-root key applied. */
+function makeSingleRepoExecutor(): TaskExecutor {
+  return new TaskExecutor(createStore(), ROOT);
+}
+
+describe("root-rooted concurrent session registration (single-repo project)", () => {
+  beforeEach(() => activeSessionRegistry.clear());
+  afterEach(() => activeSessionRegistry.clear());
+
+  const surfaces: Array<[string, (executor: TaskExecutor, taskId: string) => void]> = [
+    ["setActiveSession", (executor, taskId) => (executor as any).setActiveSession(taskId, {}, ROOT)],
+    ["setActiveStepExecutor", (executor, taskId) => (executor as any).setActiveStepExecutor(taskId, {}, ROOT)],
+    ["setActiveWorkflowStepSession", (executor, taskId) => (executor as any).setActiveWorkflowStepSession(taskId, {}, ROOT)],
+  ];
+
+  for (const [name, register] of surfaces) {
+    it(`lets two tasks register concurrently on the repo root via ${name}`, () => {
+      const executor = makeSingleRepoExecutor();
+
+      expect(() => register(executor, "FN-1398")).not.toThrow();
+      expect(() => register(executor, "FN-1403")).not.toThrow();
+
+      const holder = activeSessionRegistry.pathsForTask("FN-1398");
+      const planner = activeSessionRegistry.pathsForTask("FN-1403");
+      expect(holder).toHaveLength(1);
+      expect(planner).toHaveLength(1);
+      expect(holder[0]).not.toEqual(planner[0]);
+      expect(activeSessionRegistry.isPathActive(holder[0])).toBe(true);
+      expect(activeSessionRegistry.isPathActive(planner[0])).toBe(true);
+    });
+  }
+
+  const cleanups: Array<[string, (executor: TaskExecutor, taskId: string) => void, (executor: TaskExecutor, taskId: string) => void]> = [
+    [
+      "deleteActiveSession",
+      (executor, taskId) => (executor as any).setActiveSession(taskId, {}, ROOT),
+      (executor, taskId) => (executor as any).deleteActiveSession(taskId),
+    ],
+    [
+      "deleteActiveStepExecutor",
+      (executor, taskId) => (executor as any).setActiveStepExecutor(taskId, {}, ROOT),
+      (executor, taskId) => (executor as any).deleteActiveStepExecutor(taskId),
+    ],
+    [
+      "deleteActiveWorkflowStepSession",
+      (executor, taskId) => (executor as any).setActiveWorkflowStepSession(taskId, {}, ROOT),
+      (executor, taskId) => (executor as any).deleteActiveWorkflowStepSession(taskId),
+    ],
+  ];
+
+  for (const [name, register, remove] of cleanups) {
+    it(`cleans up the task-scoped root key on ${name} (no leak)`, () => {
+      const executor = makeSingleRepoExecutor();
+      // The in-memory activeWorktrees Set holds the REAL root; the delete surface must map it back to
+      // the synthetic key that was registered.
+      (executor as any).addActiveWorktree("FN-1403", ROOT);
+      register(executor, "FN-1403");
+      expect(activeSessionRegistry.pathsForTask("FN-1403")).toHaveLength(1);
+
+      remove(executor, "FN-1403");
+      expect(activeSessionRegistry.pathsForTask("FN-1403")).toHaveLength(0);
+    });
+  }
+
+  it("still rejects a foreign-task overwrite on a shared per-task worktree path", () => {
+    const executor = makeSingleRepoExecutor();
+    const sharedWorktree = `${ROOT}-worktrees/shared`;
+
+    (executor as any).setActiveWorkflowStepSession("FN-1398", {}, sharedWorktree);
+    expect(() => (executor as any).setActiveWorkflowStepSession("FN-1403", {}, sharedWorktree)).toThrow(
+      ActiveSessionPathHeldByForeignTaskError,
+    );
+  });
+});

packages/engine/src/__tests__/executor-workspace-concurrent-session.test.ts

@@ -79,8 +79,14 @@ describe("workspace concurrent session registration", () => {
   });
 
   it("still rejects a foreign-task overwrite for NON-workspace tasks (clobber guard preserved)", () => {
+    /*
+    FNXC:PlanReviewWorktree 2026-07-25-20:40:
+    The shared path under test must be a real per-task WORKTREE path, distinct from the executor's
+    rootDir — the root itself is now task-scoped in every mode (see sessionRegistryPath). Two tasks
+    landing on the identical worktree path is the cross-phase clobber this guard exists for.
+    */
     const sharedWorktree = "/tmp/fusion-test-single-repo-worktree";
-    const executor = new TaskExecutor(createStore(), sharedWorktree); // no workspaceConfig → singular path
+    const executor = new TaskExecutor(createStore(), "/tmp/fusion-test-single-repo-root"); // no workspaceConfig → singular path
 
     (executor as any).setActiveSession("FN-A", {}, sharedWorktree);
     // A second, different task on the identical real worktree path must still be rejected — this is the

packages/engine/src/executor.ts

@@ -1973,8 +1973,23 @@ export class TaskExecutor {
   getActiveWorktreePaths() consumers that cd into a path are unaffected; only the registry key changes.
   Non-workspace tasks (unique worktree path != rootDir) are returned unchanged.
   */
+  /*
+  FNXC:PlanReviewWorktree 2026-07-25-20:40 (concurrent root-rooted step sessions — single-repo collision):
+  The task-scoped key must apply to the shared repo root in EVERY project mode, not only workspace mode.
+  Read-only graph nodes that need no worktree (Plan Review is the canonical one — it reviews the
+  store-injected PROMPT.md, see FNXC:PlanReviewSpecInjection) run rooted at `this.rootDir`, and a todo
+  task has no worktree of its own. With the bare root as the registry key, two tasks reaching Plan Review
+  at the same time collided: the second failed with "active-session path <root> is held by task <other>;
+  task <self> may not overwrite it", which surfaced as a Plan Review provider failure, burned the
+  in-place retry budget against a hold that retrying can never clear, and left the task parked
+  (reported: FN-1398 holding /home/ubuntu/dev/freemap-svelte while FN-1403 planned).
+  Path-exclusivity on the shared root is not what keeps these sessions correct: write-capable nodes are
+  refused at the root outright (no-worktree-for-write-node above), real per-sub-repo exclusivity is the
+  workspace-repo-acquire lease, and every isPathActive consumer guards removable WORKTREE paths — the
+  root is never one. Liveness still works because the synthetic key stays in the registry under the task.
+  */
   private sessionRegistryPath(taskId: string, worktreePath: string): string {
-    if (this.workspaceConfig && worktreePath === this.rootDir) {
+    if (worktreePath === this.rootDir) {
       return `${worktreePath}#session:${taskId}`;
     }
     return worktreePath;