From da4e4bca4426d0b47afe3687d69fa6ae094143bc Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 14 Aug 2026 22:03:16 -0700 Subject: [PATCH] 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) --- ...fn-9053-workspace-acquisition-atomicity.md | 7 ++ docs/architecture.md | 1 + .../worktree-acquisition-workspace.test.ts | 98 ++++++++++++++++++- .../src/worktree/worktree-acquisition.ts | 66 ++++++++----- 4 files changed, 142 insertions(+), 30 deletions(-) create mode 100644 .changeset/fn-9053-workspace-acquisition-atomicity.md diff --git a/.changeset/fn-9053-workspace-acquisition-atomicity.md b/.changeset/fn-9053-workspace-acquisition-atomicity.md new file mode 100644 index 0000000000..3290c9d993 --- /dev/null +++ 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. diff --git a/docs/architecture.md b/docs/architecture.md index ff64a25ee6..e7d38995f5 100644 --- 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 `/.fusion/recovery/worktrees`, or to `/.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. - **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. diff --git a/packages/engine/src/__tests__/worktree-acquisition-workspace.test.ts b/packages/engine/src/__tests__/worktree-acquisition-workspace.test.ts index 49ac8efabd..f9557ad63d 100644 --- 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) => boolean } = {}, +): { store: TaskStore; current: () => Task; logs: string[]; patches: Partial[] } { let current = task; const logs: string[] = []; + const patches: Partial[] = []; const store = { async updateTask(id: string, patch: Partial): Promise { + 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 { @@ -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 }); }); }); diff --git a/packages/engine/src/worktree/worktree-acquisition.ts b/packages/engine/src/worktree/worktree-acquisition.ts index 76c8f48b9e..23f880ed51 100644 --- 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 { const { task, rootDir, store, settings, pool, logger, audit, runContext, createWorktree, runConfiguredCommand, runInitCommand, taskEnv, secretsStore } = opts; + const persistWorktreeAssignment = async (patch: Parameters[1]): Promise => { + 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 });