FN-9053: prevent workspace acquisition from persisting singular worktrees

Keep workspace tasks classified as multi-repository throughout sub-repository worktree acquisition. - Suppress singular worktree and branch persistence for workspace sub-repo acquisition. - Preserve the single-repository persistence contract and add regression coverage for acquisition failures. - Document the workspace acquisition-state invariant and add a patch changeset. Files changed: .../fn-9053-workspace-acquisition-atomicity.md | 7 ++ docs/architecture.md | 1 + .../worktree-acquisition-workspace.test.ts | 98 ++++++++++++++++++++-- .../engine/src/worktree/worktree-acquisition.ts | 66 +++++++++------ 4 files changed, 142 insertions(+), 30 deletions(-) Fusion-Task-Id: FN-9053 Fusion-Task-Lineage: e27d1e90-fc99-4bb6-ac83-033f71abc521 Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
2026-08-14 22:03:16 -07:00
parent 9ba6a67695
commit da4e4bca44
4 changed files with 142 additions and 30 deletions
--- a/.changeset/fn-9053-workspace-acquisition-atomicity.md
+++ b/.changeset/fn-9053-workspace-acquisition-atomicity.md
@@ -0,0 +1,7 @@
+---
+"@runfusion/fusion": patch
+---
+
+summary: Workspace tasks no longer briefly look single-repo while acquiring a sub-repo worktree.
+category: fix
+dev: acquireTaskWorktree gains opt-in `suppressSingularWorktreePersist`; acquireWorkspaceRepoWorktree sets it so the merged `workspaceWorktrees` write is the only durable acquisition write.
--- a/docs/architecture.md
+++ b/docs/architecture.md
@@ -626,6 +626,7 @@ See [Memory Plugin Contract](./memory-plugin-contract.md) for the full plan.
 ### Agent roles
 - **Planning**: the planning processor generates task plans (`PROMPT.md`) and selects eligible planning tasks by priority first, then FIFO (`createdAt` ascending) within each priority tier. Each attempt captures the authoritative artifact baseline and owns its fallback callback provenance. Only a settled, fallback-free attempt that changed that exact baseline and passes deterministic validation may hand off to workflow Plan Review. Empty, unchanged, or fallback-engaged attempts use the shared bounded `recoveryRetryCount`/`nextRecoveryAt` backoff; exhaustion persists an actionable planning error and never signals successful handoff. After a prompt settles, triage awaits the originating runtime's finite `settleFallbackDispatch` lifecycle signal, then awaits every observer callback admitted by that signal before deciding. A configured runtime that cannot supply this signal fails closed through the same bounded planning recovery rather than handing a potentially fallback-authored plan to review. This deliberately never inspects arbitrary Node timers: clean planner housekeeping can schedule unrelated one-shot or recurring timers without delaying admission. A callback from an obsolete attempt remains scoped to that attempt. Explicit duplicate-marker closure runs only after this same clean-attempt admission. If the stuck-task detector kills a not-yet-approved planning session after a non-empty `PROMPT.md` draft exists, the retry is requeued as `needs-replan` and seeds the next prompt in revision mode from that draft instead of cold-starting. A newly added dependency in a hold lane follows the same durable `needs-replan` path; it never clears status, so a planner interrupted after prompt persistence remains claimable and cannot silently bypass the approval/release handoff. When `PROMPT.md` is absent, a non-empty `plan` task document written through `fn_task_document_write` is the fallback seed; missing or whitespace-only drafts still cold-start.
 - **Executor**: `TaskExecutor` (`executor.ts`) implements tasks in worktrees
+  - **Workspace acquisition shape:** per-repo acquisition persists only `workspaceWorktrees`; it never exposes a sub-repo path or branch through singular `task.worktree`/`task.branch`, including if the final workspace-state write fails. This preserves workspace classification for dashboard rendering, self-healing, and executor dispatch.
  - **Task-pinned orphan recovery:** task-ID-pinned acquisition holds one path reservation across classification, preservation, quarantine reconciliation, and recreation. Inactive incomplete or unregistered directories are atomically moved to `<project>/.fusion/recovery/worktrees`, or to `<worktreesDir>/.fusion-recovery/worktrees` after an `EXDEV` cross-filesystem refusal. Each actual recovery root retains the newest 10 recognized Fusion-generated entries; pruning is fail-soft and preserves unknown, symlinked, unreadable, or active paths. Worktree pool and self-healing scans exclude both `.ai-merge` and `.fusion-recovery` as internal container boundaries.
  <!-- FNXC:MergerUnification 2026-08-09-12:04: Master-plan U0 made clean-room `runAiMerge` the sole production merge path. The legacy `aiMergeTask` auto-prerebase policy is retained but inert, so executor reused-base refresh must not describe it as live merger behavior. -->
  - **Execution-only reused-base refresh (FN-8693):** planning creates isolated worktrees but does not refresh them; immediately before a graph `code` node, normal executor dispatch, or durable-agent heartbeat session, refresh-enabled reuse resolves the current integration target C1 and compares it with durable `task.baseCommitSha`. A clean no-own-commit checkout resets to C1; a clean own-commit checkout rebases and retains its resulting C2 `HEAD`, while storing C1—not C2—as the baseline. A durable C0/C1 mismatch is rechecked from git and durable metadata on every acquisition, so restart reconciliation needs no in-memory marker. Dirty, unresolved, unsupported worktrunk, git, conflict, persistence, and unprovable-reconciliation cases are typed non-execution outcomes that park before session start. If baseline persistence fails after git moves `HEAD`, the engine compensates to the original clean checkout and emits `worktree:base-refresh-persistence-failed-compensated`; otherwise it requires later proof-based reconciliation. Audit events are `worktree:base-refreshed`, `worktree:base-refresh-blocked`, `worktree:base-refresh-conflict`, `worktree:base-refresh-persistence-failed-compensated`, and `worktree:base-refresh-reconciled`. Plan/review/gate acquisition and merger acquisition remain excluded; production merger behavior is the unified clean-room `runAiMerge` path.
--- a/packages/engine/src/tests/worktree-acquisition-workspace.test.ts
+++ b/packages/engine/src/tests/worktree-acquisition-workspace.test.ts
@@ -14,6 +14,7 @@ import { join } from "node:path";
 import { afterEach, describe, expect, it } from "vitest";
 import type { Settings, Task, TaskStore } from "@fusion/core";
 import {
+  acquireTaskWorktree,
  acquireWorkspaceRepoWorktree,
  WorkspaceRepoAcquireBusyError,
 } from "../worktree/worktree-acquisition.js";
@@ -31,11 +32,17 @@ function git(repo: string, command: string): string {
 * and its acquireTaskWorktree callee touch: updateTask (merge-in-place so the
 * idempotency re-read sees persisted workspaceWorktrees), logEntry, getTask.
 */
-function makeFakeStore(task: Task): { store: TaskStore; current: () => Task; logs: string[] } {
+function makeFakeStore(
+  task: Task,
+  options: { failWhen?: (patch: Partial<Task>) => boolean } = {},
+): { store: TaskStore; current: () => Task; logs: string[]; patches: Partial<Task>[] } {
  let current = task;
  const logs: string[] = [];
+  const patches: Partial<Task>[] = [];
  const store = {
    async updateTask(id: string, patch: Partial<Task>): Promise<void> {
+      patches.push(patch);
+      if (options.failWhen?.(patch)) throw new Error("injected update failure");
      if (id === current.id) current = { ...current, ...patch };
    },
    async logEntry(_id: string, message: string): Promise<void> {
@@ -45,7 +52,7 @@ function makeFakeStore(task: Task): { store: TaskStore; current: () => Task; log
      return id === current.id ? current : null;
    },
  } as unknown as TaskStore;
-  return { store, current: () => current, logs };
+  return { store, current: () => current, logs, patches };
 }

 function makeTask(id: string): Task {
@@ -201,7 +208,7 @@ describeIfGit("acquireWorkspaceRepoWorktree (U2 per-repo hardening)", { timeout:
    // prove a second task is rejected while it is held.
    registry.registerPath(repoAbs, { taskId: "FN-A", kind: "workspace-repo-acquire", ownerKey: "workspace-repo-acquire" });

-    const { store, current } = makeFakeStore(makeTask("FN-B"));
+    const { store, current, patches } = makeFakeStore(makeTask("FN-B"));
    await expect(
      acquireWorkspaceRepoWorktree({
        repoRelPath: "repo-a",
@@ -215,6 +222,7 @@ describeIfGit("acquireWorkspaceRepoWorktree (U2 per-repo hardening)", { timeout:

    // The holder's entry is untouched by the rejected loser.
    expect(registry.lookupByPath(repoAbs)?.taskId).toBe("FN-A");
+    expect(patches).toHaveLength(0);

    // Once released, the same task acquires cleanly and the registry is freed.
    registry.unregisterPath(repoAbs);
@@ -233,7 +241,7 @@ describeIfGit("acquireWorkspaceRepoWorktree (U2 per-repo hardening)", { timeout:

  it("is idempotent across (taskId, repo): re-acquire returns the existing entry without re-capture", async () => {
    fixture = await createWorkspaceFixture(["repo-a"]);
-    const { store, current } = makeFakeStore(makeTask("FN-4"));
+    const { store, current, patches } = makeFakeStore(makeTask("FN-4"));
    const registry = new ActiveSessionRegistry();

    const first = await acquireWorkspaceRepoWorktree({
@@ -259,6 +267,7 @@ describeIfGit("acquireWorkspaceRepoWorktree (U2 per-repo hardening)", { timeout:
    expect(second.alreadyAcquired).toBe(true);
    expect(second.worktreePath).toBe(first.worktreePath);
    expect(second.baseCommitSha).toBe(first.baseCommitSha);
+    expect(patches).toHaveLength(1);
    expect(registry.isPathActive(fixture.repoPath("repo-a"))).toBe(false);
  });

@@ -336,7 +345,7 @@ describeIfGit("acquireWorkspaceRepoWorktree (U2 per-repo hardening)", { timeout:
  */
  it("preserves a sibling sub-repo's workspaceWorktrees entry across two different-repo acquires (F5)", async () => {
    fixture = await createWorkspaceFixture(["repo-a", "repo-b"]);
-    const { store, current } = makeFakeStore(makeTask("FN-7"));
+    const { store, current, patches } = makeFakeStore(makeTask("FN-7"));
    const registry = new ActiveSessionRegistry();

    const first = await acquireWorkspaceRepoWorktree({
@@ -364,5 +373,84 @@ describeIfGit("acquireWorkspaceRepoWorktree (U2 per-repo hardening)", { timeout:
    const persisted = current().workspaceWorktrees ?? {};
    expect(persisted["repo-a"]?.worktreePath).toBe(first.worktreePath);
    expect(persisted["repo-b"]?.worktreePath).toBe(second.worktreePath);
+    expect(patches.every((patch) => !patch.worktree && !patch.branch)).toBe(true);
+  });
+
+  it("re-acquires a dead remembered workspace entry without singular persistence", async () => {
+    fixture = await createWorkspaceFixture(["repo-a"]);
+    const initial = {
+      ...makeTask("FN-8"),
+      workspaceWorktrees: {
+        "repo-a": { worktreePath: join(fixture.rootDir, "missing-worktree"), branch: "fusion/fn-8" },
+      },
+    };
+    const { store, current, patches } = makeFakeStore(initial);
+
+    const result = await acquireWorkspaceRepoWorktree({
+      repoRelPath: "repo-a", workspaceRootDir: fixture.rootDir, task: initial, store,
+      settings: SETTINGS, registry: new ActiveSessionRegistry(),
+    });
+
+    expect(result.alreadyAcquired).toBe(false);
+    expect(current().workspaceWorktrees?.["repo-a"]?.worktreePath).toBe(result.worktreePath);
+    expect(patches.every((patch) => !patch.worktree && !patch.branch)).toBe(true);
+  });
+
+  it("never exposes a singular worktree assignment while acquiring a workspace repo", async () => {
+    fixture = await createWorkspaceFixture(["repo-a"]);
+    const initial = makeTask("FN-8");
+    const { store, patches } = makeFakeStore(initial);
+
+    await acquireWorkspaceRepoWorktree({
+      repoRelPath: "repo-a", workspaceRootDir: fixture.rootDir, task: initial, store,
+      settings: SETTINGS, registry: new ActiveSessionRegistry(),
+    });
+
+    expect(patches).toHaveLength(1);
+    expect(patches[0]).toHaveProperty("workspaceWorktrees");
+    expect(patches.every((patch) => !patch.worktree && !patch.branch)).toBe(true);
+
+    let replayed = initial;
+    for (const patch of patches) {
+      replayed = { ...replayed, ...patch };
+      expect(replayed.worktree).toBeFalsy();
+      expect(replayed.branch).toBeFalsy();
+    }
+  });
+
+  it("leaves the task workspace-shaped when the final workspace state write fails", async () => {
+    fixture = await createWorkspaceFixture(["repo-a"]);
+    const initial = makeTask("FN-9");
+    const { store, current, patches } = makeFakeStore(initial, {
+      failWhen: (patch) => "workspaceWorktrees" in patch,
+    });
+
+    await expect(acquireWorkspaceRepoWorktree({
+      repoRelPath: "repo-a", workspaceRootDir: fixture.rootDir, task: initial, store,
+      settings: SETTINGS, registry: new ActiveSessionRegistry(),
+    })).rejects.toThrow("injected update failure");
+
+    expect(patches).toHaveLength(1);
+    expect(patches[0]).toHaveProperty("workspaceWorktrees");
+    expect(patches.every((patch) => !patch.worktree && !patch.branch)).toBe(true);
+    expect(current().worktree).toBeFalsy();
+    expect(current().branch).toBeFalsy();
+    expect(Boolean(current().worktree)).toBe(false);
+    expect(current().workspaceWorktrees).toBeUndefined();
+  });
+
+  it("keeps the single-repo acquisition persistence contract when suppression is absent", async () => {
+    fixture = await createWorkspaceFixture(["repo-a"]);
+    const initial = makeTask("FN-10");
+    const { store, patches } = makeFakeStore(initial);
+
+    const result = await acquireTaskWorktree({
+      task: initial,
+      rootDir: fixture.repoPath("repo-a"),
+      store,
+      settings: SETTINGS,
+    });
+
+    expect(patches).toContainEqual({ worktree: result.worktreePath, branch: result.branch });
  });
 });
--- a/packages/engine/src/worktree/worktree-acquisition.ts
+++ b/packages/engine/src/worktree/worktree-acquisition.ts
@@ -116,6 +116,14 @@ export interface AcquireTaskWorktreeOptions {
  renameWorktreeDirectory?: typeof rename;
  /** Execution callers opt in; planning, review, and merge reuse remain unchanged. */
  refreshStaleBase?: boolean;
+  /*
+   * FNXC:Workspace 2026-08-15-04:28:
+   * Workspace sub-repo acquisition must never persist its per-repo path or branch in the task's
+   * singular worktree columns. Only that caller opts in; all single-repo callers retain the
+   * existing persistence contract.
+   */
+  /** Suppress singular `worktree` and `branch` persistence for workspace sub-repo acquisition. */
+  suppressSingularWorktreePersist?: boolean;
 }

 export interface AcquireTaskWorktreeResult {
@@ -319,6 +327,16 @@ async function pinnedWorktreeBranchMatches(rootDir: string, worktreePath: string

 export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Promise<AcquireTaskWorktreeResult> {
  const { task, rootDir, store, settings, pool, logger, audit, runContext, createWorktree, runConfiguredCommand, runInitCommand, taskEnv, secretsStore } = opts;
+  const persistWorktreeAssignment = async (patch: Parameters<TaskStore["updateTask"]>[1]): Promise<void> => {
+    if (!opts.suppressSingularWorktreePersist) {
+      await store.updateTask(task.id, patch);
+      return;
+    }
+    const { worktree: _worktree, branch: _branch, ...nonSingularPatch } = patch;
+    if (Object.keys(nonSingularPatch).length > 0) {
+      await store.updateTask(task.id, nonSingularPatch);
+    }
+  };
  const renameWorktreeDirectory = opts.renameWorktreeDirectory ?? rename;
  const refreshExistingWorktree = async (
    path: string,
@@ -467,7 +485,7 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro
      });
      logger?.log(`${task.id}: assigned worktree is not usable; creating a fresh worktree instead: ${worktreePath}`);
      await store.logEntry(task.id, "Assigned worktree is not a registered, usable git worktree; creating a fresh worktree instead", worktreePath, runContext);
-      await store.updateTask(task.id, { worktree: null, branch: null, sessionFile: null });
+      await persistWorktreeAssignment({ worktree: null, branch: null, sessionFile: null });
      const fallbackName = generateWorktreeName(rootDir, settings);
      worktreePath = await resolveTaskWorktreePathForBackend(rootDir, fallbackName, settings, backend, branchName);
      isResume = false;
@@ -637,13 +655,13 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro
     */
    if (isRepoRootPath(rootDir, created.path)) {
      await emitRepoRootReturnGuardAudit(created.path, source);
-      await store.updateTask(task.id, { worktree: null, branch: null, sessionFile: null });
+      await persistWorktreeAssignment({ worktree: null, branch: null, sessionFile: null });
      throw new RepoRootWorktreeError(task.id, rootDir, created.path, `fresh-create:${logOrigin}`);
    }

    worktreePath = created.path;
    branch = created.branch;
-    await store.updateTask(task.id, { worktree: created.path, branch: created.branch });
+    await persistWorktreeAssignment({ worktree: created.path, branch: created.branch });
    await audit?.git({ type: "worktree:create", target: created.path, metadata: { branch: created.branch, source: logOrigin === "return-guard" ? "acquire-return-guard" : undefined } });
    await audit?.git({ type: "branch:create", target: created.branch });
    if (created.branch !== branchName) {
@@ -719,7 +737,7 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro
    await emitRepoRootReturnGuardAudit(guardedPath, source);
    logger?.warn(`${task.id}: acquisition ${source} returned repo root; clearing assignment and creating a fresh worktree`);
    await store.logEntry(task.id, "Acquisition attempted to return the project root as a task worktree; creating a fresh worktree instead", guardedPath, runContext);
-    await store.updateTask(task.id, { worktree: null, branch: null, sessionFile: null });
+    await persistWorktreeAssignment({ worktree: null, branch: null, sessionFile: null });
    const fallbackName = generateWorktreeName(rootDir, settings);
    const fallbackPath = await resolveTaskWorktreePathForBackend(rootDir, fallbackName, settings, backend, branchName);
    const created = await createWorktreeImpl(branchName, fallbackPath, task.id, freshStartPoint, allowSiblingBranchRename);
@@ -774,7 +792,7 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro
        metadata: { taskId: task.id, previous: task.worktree, derived: pinnedPath, source: "acquire" },
      });
      await store.logEntry(task.id, "Re-derived task-pinned worktree path from task id", `${task.worktree} -> ${pinnedPath}`, runContext);
-      await store.updateTask(task.id, { worktree: pinnedPath });
+      await persistWorktreeAssignment({ worktree: pinnedPath });
    }

    const reservation = await acquireWorktreePathReservation({
@@ -826,7 +844,7 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro
         * was already correct.
         */
        if (task.worktree !== pinnedPath || task.branch !== resumedBranch) {
-          await store.updateTask(task.id, { worktree: pinnedPath, branch: resumedBranch });
+          await persistWorktreeAssignment({ worktree: pinnedPath, branch: resumedBranch });
        }
        return reuseWarmWorktree(pinnedPath, resumedBranch, "existing");
      }
@@ -1055,7 +1073,7 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro
          });
          acquiredFromPool = true;
          logger?.log(`Acquired worktree from pool: ${worktreePath}`);
-          await store.updateTask(task.id, { worktree: worktreePath, branch });
+          await persistWorktreeAssignment({ worktree: worktreePath, branch });
          await audit?.git({ type: "worktree:reuse", target: worktreePath, metadata: { branch, reclaimed: prepared.reclaimed } });
          if (prepared.reclaimed) {
            await store.logEntry(task.id, `Acquired reclaimed worktree from pool: ${worktreePath} (${prepared.strandedCommitCount ?? 0} commits preserved)`, undefined, runContext);
@@ -1114,7 +1132,7 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro
        if (poolErr instanceof WorktreeBaseRefreshError) {
          // FNXC:WorktreeBaseRefresh 2026-08-09-03:30: Clear every durable resume binding before returning the
          // checkout to the pool. If persistence fails, retain the lease so no other task can mutate it.
-          await store.updateTask(task.id, { worktree: null, branch: null, sessionFile: null });
+          await persistWorktreeAssignment({ worktree: null, branch: null, sessionFile: null });
          pool.release(pooled, task.id);
          throw poolErr;
        }
@@ -1376,18 +1394,20 @@ export async function acquireWorkspaceRepoWorktree(

  try {
    /*
+    FNXC:Workspace 2026-08-15-04:28:
+    No persisted intermediate state may make a workspace task read as single-repo. Dashboard
+    `isWorkspaceTask`, self-healing sweeps, and executor `hadAssignedWorktree` all read this row,
+    so suppress singular persistence as well as stripping the helper's in-memory task copy.
+
    FNXC:WorkspaceWorktree 2026-06-21-19:05:
    Workspace mode acquires one worktree per sub-repo for a single task. `acquireTaskWorktree`
-    is single-repo: it reads `task.worktree`/`task.branch` to decide resume-vs-fresh and rewrites
-    those singular fields on the task row after each acquisition. Passing the live task straight
-    through means the second repo's acquisition sees the first repo's `task.worktree` (which exists
-    on disk), classifies it as a resume, and reuses repo A's worktree inside repo B — cross-repo
-    contamination. Clear the singular worktree/branch fields on the copy handed to the single-repo
-    helper so each sub-repo always gets a fresh worktree; per-repo state is tracked in
-    `task.workspaceWorktrees`, not the singular column.
+    is single-repo: it reads `task.worktree`/`task.branch` to decide resume-vs-fresh. Passing the
+    live task through means a later repo can reuse the first repo's worktree. Clear singular fields
+    on the copy so every sub-repo acquires freshly; per-repo state is `task.workspaceWorktrees`.
    */
    const result = await acquireTaskWorktree({
      task: { ...task, worktree: undefined, branch: undefined },
+      suppressSingularWorktreePersist: true,
      rootDir: repoAbsPath,
      store,
      // FNXC:Workspace 2026-07-07-08:40 (FN-7360 regression — strip shared branch overrides for per-repo start-point):
@@ -1513,16 +1533,12 @@ export async function acquireWorkspaceRepoWorktree(
      [repoRelPath]: { worktreePath: result.worktreePath, branch: result.branch, baseCommitSha },
    };
    /*
-    FNXC:Workspace 2026-06-22-09:00:
-    F10 — reset the singular worktree/branch columns to null in the SAME write that
-    persists workspaceWorktrees. The single-repo `acquireTaskWorktree` above wrote
-    `task.worktree`/`task.branch` (the sub-repo path/branch) to the real task row;
-    clearing the in-memory copy passed in only stops the NEXT sub-repo from resuming
-    into this one's worktree — the DB row stays polluted. A non-null `task.worktree`
-    makes `isWorkspaceTask(task)` return false (its first guard), so the dashboard
-    stops rendering WorkspaceWorktreesSummary and instead shows the sub-repo branch in
-    the standard chip — the blank/wrong-card state U10 prevents. Nulling them here
-    keeps `task.worktree` null for the workspace task's whole lifetime.
+    FNXC:Workspace 2026-08-15-04:28:
+    F10 — this is the one durable acquisition-state write. The helper suppresses every earlier
+    singular assignment, so null worktree/branch are an idempotent defensive re-assertion rather
+    than cleanup after a visible pollution window. A failed write leaves the row unchanged and
+    never makes dashboard workspace rendering, self-healing, or executor dispatch read it as
+    single-repo.
    */
    await store.updateTask(task.id, { workspaceWorktrees: updated, worktree: null, branch: null });