fix(workspace): persist workspaceWorktrees and isolate concurrent session leases

Multiworkspace tasks could not complete due to two independent bugs:

1. task.workspaceWorktrees had no SQLite column / rowToTask mapping, so
   fn_acquire_repo_worktree's updateTask write was dropped on every persist
   (applyTaskPatch writes the DB-round-tripped task back to task.json). Every
   later getTask returned undefined, so fn_task_done's scope verifier read {}
   and blocked with "acquired no sub-repo worktrees", and isWorkspaceTask()
   consumers misfired. Persist it mirroring mergeDetails (schema column + v129
   migration + db-migrate + defineTaskColumn + TaskRow + rowToTask).

2. In workspace mode every task ran rooted at the shared browse-only root, and
   setActiveSession registered that path keyed only by path — so a second
   concurrent workspace task was rejected by the foreign-task guard
   ("active-session path ... is held by ..."). Give each task a task-scoped
   synthetic session key (sessionRegistryPath), applied at all register and
   unregister sites; the in-memory worktree Set still holds the real root.

Regression tests assert the persistence invariant across getTask/listTasks/
store-reopen and concurrent session registration across all three session
surfaces; both verified to fail without the fix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-06-24 15:13:43 -07:00
parent be2866ee66
commit f06281961e
7 changed files with 227 additions and 10 deletions

View File

@@ -0,0 +1,5 @@
---
"@runfusion/fusion": patch
---
Fix multiworkspace tasks failing to complete. `task.workspaceWorktrees` is now durably persisted (it previously had no SQLite column, so `fn_acquire_repo_worktree`'s write was dropped on every persist and `fn_task_done` always reported "acquired no sub-repo worktrees"). Concurrent workspace tasks no longer collide on the shared browse-root active-session path — each task gets a task-scoped session key, so a second workspace task no longer fails with "active-session path … is held by …".

View File

@@ -59,6 +59,59 @@ describe("TaskStore", () => {
});
});
// FNXC:Workspace 2026-06-24-15:30 (multiworkspace fn_task_done regression):
// task.workspaceWorktrees previously had NO SQLite column / no rowToTask mapping, so
// fn_acquire_repo_worktree's updateTask({workspaceWorktrees}) set it only in memory and the very
// next getTask (SQLite round-trip) dropped it. fn_task_done's scope verifier then read `{}` and
// refused with "acquired no sub-repo worktrees", and every isWorkspaceTask() consumer misfired.
// The invariant: the per-sub-repo worktree map survives write→read across ALL surfaces —
// getTask, listTasks, AND a full store reopen (SQLite + task.json + reconcile).
describe("workspaceWorktrees persistence", () => {
const sampleMap = {
swarmclaw: { worktreePath: "/ws/swarmclaw/.worktrees/ivory-raven", branch: "fusion/mult-002", baseCommitSha: "a327402" },
OpenVide: { worktreePath: "/ws/OpenVide/.worktrees/light-ember", branch: "fusion/mult-001" },
};
it("round-trips the per-sub-repo worktree map through write and getTask", async () => {
const task = await harness.store().createTask({ description: "Workspace task" });
const updated = await harness.store().updateTask(task.id, { workspaceWorktrees: sampleMap });
expect(updated.workspaceWorktrees).toEqual(sampleMap);
// The smoking-gun assertion: getTask reads back from SQLite (rowToTask), not the in-memory
// mutation. Before the fix this returned undefined because no column persisted the map.
const detail = await harness.store().getTask(task.id);
expect(detail.workspaceWorktrees).toEqual(sampleMap);
});
it("returns the map from listTasks", async () => {
const task = await harness.store().createTask({ description: "Workspace task in list" });
await harness.store().updateTask(task.id, { workspaceWorktrees: sampleMap });
const listed = (await harness.store().listTasks()).find((t) => t.id === task.id);
expect(listed?.workspaceWorktrees).toEqual(sampleMap);
});
it("survives a full store reopen (SQLite + task.json + reconcile)", async () => {
const task = await harness.store().createTask({ description: "Workspace task across reopen" });
await harness.store().updateTask(task.id, { workspaceWorktrees: sampleMap });
await harness.reopenDiskBackedStore();
const detail = await harness.store().getTask(task.id);
expect(detail.workspaceWorktrees).toEqual(sampleMap);
});
it("normalizes an empty map to undefined so isWorkspaceTask stays false", async () => {
const task = await harness.store().createTask({ description: "Empty workspace map" });
const updated = await harness.store().updateTask(task.id, { workspaceWorktrees: {} });
expect(updated.workspaceWorktrees ?? {}).toEqual({});
const detail = await harness.store().getTask(task.id);
expect(detail.workspaceWorktrees).toBeUndefined();
});
});
describe("tokenUsage persistence", () => {
it("round-trips per-model token buckets through write and read", async () => {
const task = await harness.store().createTask({ description: "Per-model token task" });

View File

@@ -226,10 +226,11 @@ async function migrateTasks(fusionDir: string, db: Database): Promise<void> {
columnMovedAt, dependencies, steps, log, attachments, steeringComments,
comments, workflowStepResults, prInfo, issueInfo,
sourceIssueProvider, sourceIssueRepository, sourceIssueExternalIssueId, sourceIssueNumber, sourceIssueUrl, sourceIssueClosedAt,
mergeDetails, breakIntoSubtasks, noCommitsExpected, enabledWorkflowSteps, modifiedFiles, sliceId
mergeDetails, breakIntoSubtasks, noCommitsExpected, enabledWorkflowSteps, modifiedFiles, sliceId,
workspaceWorktrees
) VALUES (
?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?,
?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?
?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?
)
`);
@@ -300,6 +301,9 @@ async function migrateTasks(fusionDir: string, db: Database): Promise<void> {
toJson(task.enabledWorkflowSteps || []),
toJson(task.modifiedFiles || []),
task.sliceId ?? null,
// FNXC:Workspace 2026-06-24-15:30: carry the per-sub-repo worktree map through the legacy
// task.json→SQLite rebuild so a workspace task migrated from disk keeps its acquired worktrees.
toJsonNullable(task.workspaceWorktrees),
);
migrated++;
} catch (err) {

View File

@@ -162,7 +162,7 @@ export function isFts5CorruptionError(error: unknown): boolean {
// ── Schema Definition ────────────────────────────────────────────────
const SCHEMA_VERSION = 128;
const SCHEMA_VERSION = 129;
const TASKS_FTS_AUTOMERGE = 8;
const TASKS_FTS_CRISISMERGE = 16;
@@ -347,7 +347,11 @@ CREATE TABLE IF NOT EXISTS tasks (
deletedAt TEXT,
allowResurrection INTEGER DEFAULT 0,
transitionPending TEXT,
customFields TEXT DEFAULT '{}'
customFields TEXT DEFAULT '{}',
-- FNXC:Workspace 2026-06-24-15:30: per-sub-repo worktree map (JSON) for workspace-mode tasks.
-- Source of truth for getSchemaCompatibilityTableSchemas(), so existing DBs are backfilled by
-- ensureSchemaCompatibility() at boot and fresh DBs get it here. See store.ts TaskRow note.
workspaceWorktrees TEXT
);
-- Config table (single row with project settings)
@@ -5289,6 +5293,16 @@ export class Database {
});
}
if (version < 129) {
// FNXC:Workspace 2026-06-24-15:30: add the workspaceWorktrees column so workspace-mode tasks
// can durably persist their per-sub-repo worktree map. Backfill is also covered by
// ensureSchemaCompatibility() (SCHEMA_SQL is its source of truth); this versioned migration keeps
// migrated and fresh-from-SCHEMA_SQL DBs converged.
this.applyMigration(129, () => {
this.addColumnIfMissing("tasks", "workspaceWorktrees", "TEXT");
});
}
}
/**

View File

@@ -277,6 +277,13 @@ interface TaskRow {
sourceIssueUrl: string | null;
sourceIssueClosedAt: string | null;
mergeDetails: string | null;
// FNXC:Workspace 2026-06-24-15:30 (FN-multiworkspace persistence): the per-sub-repo worktree
// map MUST have its own SQLite column. Before this it was a Task field with NO column/rowToTask
// mapping, so updateTask set it only in-memory and applyTaskPatch wrote the DB-round-tripped
// task (without it) back to task.json — the map was silently dropped on every persist. That made
// fn_task_done's scope verifier always read `{}` ("acquired no sub-repo worktrees") and broke
// every isWorkspaceTask() consumer. Stored as JSON text, same shape as mergeDetails.
workspaceWorktrees: string | null;
breakIntoSubtasks: number | null;
noCommitsExpected: number | null;
enabledWorkflowSteps: string | null;
@@ -429,6 +436,10 @@ const TASK_COLUMN_DESCRIPTORS: TaskColumnDescriptor[] = [
defineTaskColumn("sourceIssueUrl", (task) => task.sourceIssue?.url ?? null),
defineTaskColumn("sourceIssueClosedAt", (task) => task.sourceIssue?.closedAt ?? null),
defineTaskColumn("mergeDetails", (task) => toJsonNullable(task.mergeDetails)),
// FNXC:Workspace 2026-06-24-15:30: persist the per-sub-repo worktree map so fn_acquire_repo_worktree's
// write survives the SQLite round-trip getChangedTaskColumns/rowToTask use. Without this descriptor the
// column diff never sees a change and the field never reaches the DB.
defineTaskColumn("workspaceWorktrees", (task) => toJsonNullable(task.workspaceWorktrees)),
defineTaskColumn("breakIntoSubtasks", (task) => task.breakIntoSubtasks ? 1 : 0),
defineTaskColumn("noCommitsExpected", (task) => task.noCommitsExpected ? 1 : 0),
defineTaskColumn("enabledWorkflowSteps", (task) => toJson(task.enabledWorkflowSteps || [])),
@@ -2150,6 +2161,13 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
};
})(),
mergeDetails: fromJson<import("./types.js").MergeDetails>(row.mergeDetails),
// FNXC:Workspace 2026-06-24-15:30: deserialize the per-sub-repo worktree map. An empty/null map
// normalizes to undefined so isWorkspaceTask() (keys-length>0) and the scope verifier behave the
// same as a task that never acquired a sub-repo.
workspaceWorktrees: (() => {
const w = fromJson<import("./types.js").Task["workspaceWorktrees"]>(row.workspaceWorktrees);
return w && Object.keys(w).length > 0 ? w : undefined;
})(),
breakIntoSubtasks: row.breakIntoSubtasks ? true : undefined,
noCommitsExpected: row.noCommitsExpected ? true : undefined,
enabledWorkflowSteps: (() => { const e = fromJson<string[]>(row.enabledWorkflowSteps); return e && e.length > 0 ? e : undefined; })(),

View File

@@ -0,0 +1,92 @@
/*
FNXC:Workspace 2026-06-24-15:45 (concurrent workspace tasks — shared browse-root collision regression):
In workspace mode every task runs its agent session rooted at the SHARED browse-only workspace root
(`this.rootDir`); per-sub-repo worktrees are acquired on demand. The session registrations
(executor / step-session / workflow-step) are keyed in the GLOBAL path-keyed activeSessionRegistry,
whose foreign-task guard rejects a second task registering a path already held by a different task.
With the bare root as the key, the SECOND concurrent workspace task failed with
"active-session path <root> is held by task <other>; task <self> may not overwrite it" — so only ONE
task per workspace could ever run (the reported MULT-001 vs MULT-002 failure).
Invariant under test (across ALL session-registration surfaces): two different workspace tasks sharing
the browse-root register concurrently WITHOUT collision, each remains discoverable by liveness
(pathsForTask returns a task-scoped key), and cleanup leaves no leaked entry. Negative control: a
NON-workspace executor (unique worktree path) still rejects a foreign-task overwrite, so the
cross-phase-clobber guard is preserved.
*/
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 WORKSPACE_ROOT = "/tmp/fusion-test-workspace-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;
}
function makeWorkspaceExecutor(): TaskExecutor {
const executor = new TaskExecutor(createStore(), WORKSPACE_ROOT);
(executor as any).workspaceConfig = { repos: ["swarmclaw", "OpenVide"] };
return executor;
}
describe("workspace concurrent session registration", () => {
beforeEach(() => activeSessionRegistry.clear());
afterEach(() => activeSessionRegistry.clear());
it("lets two workspace tasks register executor sessions on the shared browse-root without collision", () => {
const executor = makeWorkspaceExecutor();
// Both tasks pass the SAME shared workspace root as worktreePath — the pre-fix collision point.
expect(() => (executor as any).setActiveSession("MULT-001", {}, WORKSPACE_ROOT)).not.toThrow();
expect(() => (executor as any).setActiveSession("MULT-002", {}, WORKSPACE_ROOT)).not.toThrow();
// Each task stays discoverable by liveness via a DISTINCT task-scoped registry key.
const a = activeSessionRegistry.pathsForTask("MULT-001");
const b = activeSessionRegistry.pathsForTask("MULT-002");
expect(a).toHaveLength(1);
expect(b).toHaveLength(1);
expect(a[0]).not.toEqual(b[0]);
expect(a[0]).toContain("MULT-001");
expect(b[0]).toContain("MULT-002");
});
it("cleans up the task-scoped session key on deleteActiveSession (no leak)", () => {
const executor = makeWorkspaceExecutor();
// The in-memory activeWorktrees Set holds the REAL root; deleteActiveSession must still map it
// back to the synthetic key it registered.
(executor as any).addActiveWorktree("MULT-001", WORKSPACE_ROOT);
(executor as any).setActiveSession("MULT-001", {}, WORKSPACE_ROOT);
expect(activeSessionRegistry.pathsForTask("MULT-001")).toHaveLength(1);
(executor as any).deleteActiveSession("MULT-001");
expect(activeSessionRegistry.pathsForTask("MULT-001")).toHaveLength(0);
});
it("does not collide across the step-session and workflow-step surfaces either", () => {
const executor = makeWorkspaceExecutor();
expect(() => (executor as any).setActiveStepExecutor("MULT-001", {}, WORKSPACE_ROOT)).not.toThrow();
expect(() => (executor as any).setActiveStepExecutor("MULT-002", {}, WORKSPACE_ROOT)).not.toThrow();
expect(() => (executor as any).setActiveWorkflowStepSession("MULT-001", {}, WORKSPACE_ROOT)).not.toThrow();
expect(() => (executor as any).setActiveWorkflowStepSession("MULT-002", {}, WORKSPACE_ROOT)).not.toThrow();
});
it("still rejects a foreign-task overwrite for NON-workspace tasks (clobber guard preserved)", () => {
const sharedWorktree = "/tmp/fusion-test-single-repo-worktree";
const executor = new TaskExecutor(createStore(), sharedWorktree); // 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
// cross-phase-clobber protection the workspace fix must not weaken.
expect(() => (executor as any).setActiveSession("FN-B", {}, sharedWorktree)).toThrow(
ActiveSessionPathHeldByForeignTaskError,
);
});
});

View File

@@ -1620,9 +1620,31 @@ export class TaskExecutor {
this.completionFinalizedTaskIds.delete(taskId);
}
/*
FNXC:Workspace 2026-06-24-15:45 (concurrent workspace tasks — shared browse-root collision):
In workspace mode `this.rootDir` is the SHARED browse-only (non-git) workspace root, and EVERY
workspace task runs its agent session rooted there (per-sub-repo worktrees are acquired on demand).
The session registrations below are keyed in the GLOBAL path-keyed activeSessionRegistry, whose
foreign-task guard rejects a second task registering a path already held by a different task. With
the bare root as the key, the second concurrent workspace task fails with "active-session path
<root> is held by task <other>; task <self> may not overwrite it" — so only ONE task per workspace
could ever run. Per-task session liveness does NOT require path-exclusivity on the shared root
(real per-sub-repo exclusivity is enforced separately by the workspace-repo-acquire lease in
worktree-acquisition.ts, keyed by sub-repo path). Give each task a task-scoped synthetic session
key so the registry stays per-task. The in-memory activeWorktrees Set still holds the REAL root, so
getActiveWorktreePaths() consumers that cd into a path are unaffected; only the registry key changes.
Non-workspace tasks (unique worktree path != rootDir) are returned unchanged.
*/
private sessionRegistryPath(taskId: string, worktreePath: string): string {
if (this.workspaceConfig && worktreePath === this.rootDir) {
return `${worktreePath}#session:${taskId}`;
}
return worktreePath;
}
private setActiveSession(taskId: string, sessionState: ActiveExecutorSessionState, worktreePath: string): void {
this.activeSessions.set(taskId, sessionState);
activeSessionRegistry.registerPath(worktreePath, { taskId, kind: "executor", ownerKey: taskId });
activeSessionRegistry.registerPath(this.sessionRegistryPath(taskId, worktreePath), { taskId, kind: "executor", ownerKey: taskId });
}
private markGraphExecuteSelfRequeued(taskId: string): void {
@@ -1638,14 +1660,17 @@ export class TaskExecutor {
// FNXC:Workspace 2026-06-21-12:00: KTD2 — when no explicit path is given, unregister EVERY worktree path the task holds (a workspace task holds N sub-repo paths); single-repo tasks resolve a one-element set.
const resolvedWorktreePaths = worktreePath ? [worktreePath] : this.getActiveWorktreePaths(taskId);
for (const path of resolvedWorktreePaths) {
activeSessionRegistry.unregisterPath(path);
// FNXC:Workspace 2026-06-24-15:45: map through sessionRegistryPath so the task-scoped synthetic
// session key registered for the shared workspace browse-root is the one we unregister (the
// in-memory Set holds the REAL root). Non-workspace/sub-repo paths pass through unchanged.
activeSessionRegistry.unregisterPath(this.sessionRegistryPath(taskId, path));
}
}
private setActiveStepExecutor(taskId: string, stepExecutor: StepSessionExecutor, worktreePath: string, seenSteeringIds = new Set<string>()): void {
this.activeStepExecutors.set(taskId, stepExecutor);
this.activeStepExecutorSeenSteeringIds.set(taskId, seenSteeringIds);
activeSessionRegistry.registerPath(worktreePath, { taskId, kind: "step-session", ownerKey: `${taskId}#step-session` });
activeSessionRegistry.registerPath(this.sessionRegistryPath(taskId, worktreePath), { taskId, kind: "step-session", ownerKey: `${taskId}#step-session` });
}
private deleteActiveStepExecutor(taskId: string, worktreePath?: string): void {
@@ -1656,14 +1681,17 @@ export class TaskExecutor {
// FNXC:Workspace 2026-06-21-12:00: KTD2 — unregister every held worktree path (Set), not one.
const resolvedWorktreePaths = worktreePath ? [worktreePath] : this.getActiveWorktreePaths(taskId);
for (const path of resolvedWorktreePaths) {
activeSessionRegistry.unregisterPath(path);
// FNXC:Workspace 2026-06-24-15:45: map through sessionRegistryPath so the task-scoped synthetic
// session key registered for the shared workspace browse-root is the one we unregister (the
// in-memory Set holds the REAL root). Non-workspace/sub-repo paths pass through unchanged.
activeSessionRegistry.unregisterPath(this.sessionRegistryPath(taskId, path));
}
}
private setActiveWorkflowStepSession(taskId: string, session: AgentSession, worktreePath: string, seenSteeringIds = new Set<string>()): void {
this.activeWorkflowStepSessions.set(taskId, session);
this.activeWorkflowStepSessionSeenSteeringIds.set(taskId, seenSteeringIds);
activeSessionRegistry.registerPath(worktreePath, { taskId, kind: "workflow-step", ownerKey: `${taskId}#workflow-step` });
activeSessionRegistry.registerPath(this.sessionRegistryPath(taskId, worktreePath), { taskId, kind: "workflow-step", ownerKey: `${taskId}#workflow-step` });
}
private deleteActiveWorkflowStepSession(taskId: string, worktreePath?: string): void {
@@ -1672,7 +1700,10 @@ export class TaskExecutor {
// FNXC:Workspace 2026-06-21-12:00: KTD2 — unregister every held worktree path (Set), not one.
const resolvedWorktreePaths = worktreePath ? [worktreePath] : this.getActiveWorktreePaths(taskId);
for (const path of resolvedWorktreePaths) {
activeSessionRegistry.unregisterPath(path);
// FNXC:Workspace 2026-06-24-15:45: map through sessionRegistryPath so the task-scoped synthetic
// session key registered for the shared workspace browse-root is the one we unregister (the
// in-memory Set holds the REAL root). Non-workspace/sub-repo paths pass through unchanged.
activeSessionRegistry.unregisterPath(this.sessionRegistryPath(taskId, path));
}
}