Align workspace completion and per-repo review behavior for tasks with no acquired worktrees. - Centralize zero-acquire classification for completion and review paths. - Approve proven commit-free tasks and immediately route deterministic unavailable reviews. - Add regression coverage, workflow documentation, and a patch changeset. Files changed: .changeset/fn-9060-workspace-zero-acquire.md | 7 +++ docs/workflow-steps.md | 2 +- .../__tests__/executor-workspace-taskdone.test.ts | 14 +++++ .../src/__tests__/executor-workspace.test.ts | 10 ++-- .../src/__tests__/reviewer-workspace.test.ts | 37 +++++++++++++- .../src/__tests__/workflow-step-review.test.ts | 14 +++++ .../src/__tests__/workspace-zero-acquire.test.ts | 59 ++++++++++++++++++++++ packages/engine/src/execution/reviewer.ts | 7 +++ .../create-authoritative-workflow-seams.ts | 26 ++++++++-- .../src/executor/workspace-review-per-repo.ts | 32 +++++++++--- .../engine/src/executor/workspace-zero-acquire.ts | 53 +++++++++++++++++++ .../engine/src/executor/worktree-verify-invariants.ts | 31 ++++++++---- .../engine/src/workflows/workflow-node-handlers.ts | 14 +++-- 13 files changed, 274 insertions(+), 32 deletions(-) Fusion-Task-Id: FN-9060 Fusion-Task-Lineage: eb88f881-7db2-49d2-b79a-796eacd4fa40 Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
297 lines
15 KiB
TypeScript
297 lines
15 KiB
TypeScript
/*
|
|
FNXC:Workspace 2026-06-21-12:00:
|
|
U1 executor session-scoping tests. REWRITTEN from the foundation's self-mocking version (which vi.mock'd the very functions under test and proved nothing). These tests use a REAL two-repo git fixture (`createWorkspaceFixture`) under a NON-git workspace root, so a leaked rootDir git preflight would actually fail. They drive the real TaskExecutor methods that U1 changed: the activeWorktrees Set conversion + every enumerated consumer (KTD2), the preflight gate + browse-only-root scoping (KTD1), and the synthetic-acquisition cwd.
|
|
|
|
Seam choice (FN-5048): `(executor as any).workspaceConfig` is set directly to drive the gating with real git — loadWorkspaceConfig is covered by its own unit and is not the subject here. No mock-the-world child_process/fs shell.
|
|
*/
|
|
import { afterEach, describe, expect, it, vi } from "vitest";
|
|
import { EventEmitter } from "node:events";
|
|
import { loadWorkspaceConfig, type Task, type TaskStore, type WorkspaceConfig } from "@fusion/core";
|
|
import { TaskExecutor, buildExecutionPrompt } from "../executor.js";
|
|
import { activeSessionRegistry } from "../agents/active-session-registry.js";
|
|
import { createWorkspaceFixture, hasGit, type WorkspaceFixture } from "./_workspace-fixture.js";
|
|
|
|
const describeIfGit = hasGit ? describe : describe.skip;
|
|
|
|
function createStore(overrides: Partial<Record<string, unknown>> = {}): TaskStore & EventEmitter {
|
|
const emitter = new EventEmitter();
|
|
return Object.assign(emitter, {
|
|
updateTask: vi.fn().mockResolvedValue(undefined),
|
|
logEntry: vi.fn().mockResolvedValue(undefined),
|
|
getSettings: vi.fn().mockResolvedValue({ autoMerge: false }),
|
|
getTask: vi.fn().mockResolvedValue(undefined),
|
|
on: emitter.on.bind(emitter),
|
|
...overrides,
|
|
}) as unknown as TaskStore & EventEmitter;
|
|
}
|
|
|
|
function makeTask(id = "FN-WS-1", overrides: Partial<Task> = {}): Task {
|
|
return {
|
|
id,
|
|
title: "Workspace task",
|
|
description: "",
|
|
column: "in-progress",
|
|
dependencies: [],
|
|
steps: [],
|
|
currentStep: 0,
|
|
log: [],
|
|
createdAt: new Date().toISOString(),
|
|
updatedAt: new Date().toISOString(),
|
|
...overrides,
|
|
} as Task;
|
|
}
|
|
|
|
const repoAPath = (fx: WorkspaceFixture) => `${fx.repoPath("repo-a")}/.worktrees/fn-ws-1`;
|
|
const repoBPath = (fx: WorkspaceFixture) => `${fx.repoPath("repo-b")}/.worktrees/fn-ws-1`;
|
|
|
|
describeIfGit("workspace fixture", () => {
|
|
let fx: WorkspaceFixture;
|
|
afterEach(() => fx?.cleanup());
|
|
|
|
it("builds a non-git root with two real git sub-repos and a resolvable workspace config", async () => {
|
|
fx = await createWorkspaceFixture();
|
|
// Root is NOT a git repo. Use "." so the check runs in fx.rootDir itself, not
|
|
// its parent (".." would resolve to the tmpdir and could pass spuriously).
|
|
expect(() => fx.git(".", "git rev-parse --git-dir")).toThrow();
|
|
// Each sub-repo is a real git repo with a commit on main.
|
|
expect(fx.git("repo-a", "git rev-parse --abbrev-ref HEAD")).toBe("main");
|
|
expect(fx.git("repo-b", "git rev-list --count HEAD")).toBe("1");
|
|
// loadWorkspaceConfig resolves the on-disk config the executor keys off.
|
|
const config = await loadWorkspaceConfig(fx.rootDir);
|
|
expect(config?.repos).toEqual(["repo-a", "repo-b"]);
|
|
});
|
|
});
|
|
|
|
describeIfGit("U1 KTD2 — activeWorktrees Set + every enumerated consumer", () => {
|
|
let fx: WorkspaceFixture;
|
|
/*
|
|
FNXC:TestInfrastructure 2026-07-29-22:05 (#2617 review — greptile P2):
|
|
`activeSessionRegistry` is PROCESS-GLOBAL and is itself a liveness signal
|
|
(hasLiveSessionSurface reads it), so a registration leaked by a failing assertion
|
|
makes the NEXT test see a phantom live session and refuse a clear it should permit.
|
|
Unregistering after the assertions only works on the happy path. Sweep in
|
|
guaranteed teardown instead — this suite is precisely the one that measures that
|
|
registry, so it must not be the one polluting it.
|
|
*/
|
|
afterEach(() => {
|
|
for (const path of activeSessionRegistry.pathsForTask("FN-WS-1")) {
|
|
activeSessionRegistry.unregisterPath(path);
|
|
}
|
|
});
|
|
afterEach(() => fx?.cleanup());
|
|
|
|
function workspaceExecutor() {
|
|
fx ??= undefined as never;
|
|
const store = createStore();
|
|
const executor = new TaskExecutor(store, fx.rootDir);
|
|
(executor as any).workspaceConfig = { repos: fx.repos } as WorkspaceConfig;
|
|
return executor;
|
|
}
|
|
|
|
it("a workspace task holding TWO sub-repo paths is found by membership, not equality", async () => {
|
|
fx = await createWorkspaceFixture();
|
|
const executor = workspaceExecutor();
|
|
const pA = repoAPath(fx);
|
|
const pB = repoBPath(fx);
|
|
(executor as any).addActiveWorktree("FN-WS-1", pA);
|
|
(executor as any).addActiveWorktree("FN-WS-1", pB);
|
|
|
|
// hasActiveWorktreeBinding: both held paths match; an unheld path does not.
|
|
expect((executor as any).hasActiveWorktreeBinding("FN-WS-1", pA)).toBe(true);
|
|
expect((executor as any).hasActiveWorktreeBinding("FN-WS-1", pB)).toBe(true);
|
|
expect((executor as any).hasActiveWorktreeBinding("FN-WS-1", "/nope")).toBe(false);
|
|
|
|
// findActiveWorktreeOwner: another task asking about either held path finds FN-WS-1.
|
|
await expect((executor as any).findActiveWorktreeOwner(pA, "FN-OTHER")).resolves.toBe("FN-WS-1");
|
|
await expect((executor as any).findActiveWorktreeOwner(pB, "FN-OTHER")).resolves.toBe("FN-WS-1");
|
|
// The owner itself is excluded.
|
|
await expect((executor as any).findActiveWorktreeOwner(pA, "FN-WS-1")).resolves.toBeNull();
|
|
});
|
|
|
|
it("listWorktreeHolders flat-maps the Set into N holder rows for one task", async () => {
|
|
fx = await createWorkspaceFixture();
|
|
const executor = workspaceExecutor();
|
|
const pA = repoAPath(fx);
|
|
const pB = repoBPath(fx);
|
|
(executor as any).addActiveWorktree("FN-WS-1", pA);
|
|
(executor as any).addActiveWorktree("FN-WS-1", pB);
|
|
|
|
const holders = executor.listWorktreeHolders();
|
|
expect(holders).toHaveLength(2);
|
|
expect(holders).toContainEqual({ taskId: "FN-WS-1", worktreePath: pA });
|
|
expect(holders).toContainEqual({ taskId: "FN-WS-1", worktreePath: pB });
|
|
});
|
|
|
|
it("shouldGenerateNewWorktreeName iterates the Set (conflict membership)", async () => {
|
|
fx = await createWorkspaceFixture();
|
|
const store = createStore({ listTasks: vi.fn().mockResolvedValue([]) });
|
|
const executor = new TaskExecutor(store, fx.rootDir);
|
|
(executor as any).workspaceConfig = { repos: fx.repos } as WorkspaceConfig;
|
|
const pA = repoAPath(fx);
|
|
(executor as any).addActiveWorktree("FN-HOLDER", pA);
|
|
|
|
// A different task contending for FN-HOLDER's path must be told to generate a new name.
|
|
await expect((executor as any).shouldGenerateNewWorktreeName(pA, "FN-WS-1")).resolves.toBe(true);
|
|
// The holder asking about its own path is not a conflict (excluded), and the
|
|
// DB liveness fallback returns no other user.
|
|
await expect((executor as any).shouldGenerateNewWorktreeName(pA, "FN-HOLDER")).resolves.toBe(false);
|
|
});
|
|
|
|
it("getWorktreePath returns undefined for a multi-worktree workspace task (Set-collapse contract)", async () => {
|
|
fx = await createWorkspaceFixture();
|
|
const executor = workspaceExecutor();
|
|
(executor as any).addActiveWorktree("FN-WS-1", repoAPath(fx));
|
|
(executor as any).addActiveWorktree("FN-WS-1", repoBPath(fx));
|
|
expect(executor.getWorktreePath("FN-WS-1")).toBeUndefined();
|
|
});
|
|
|
|
it("cleanup drops in-memory tracking in workspace mode but never removes the root", async () => {
|
|
fx = await createWorkspaceFixture();
|
|
const removeSpy = vi.fn();
|
|
const executor = workspaceExecutor();
|
|
(executor as any).removeOwnWorktreeWithReconcile = removeSpy;
|
|
(executor as any).addActiveWorktree("FN-WS-1", repoAPath(fx));
|
|
(executor as any).addActiveWorktree("FN-WS-1", repoBPath(fx));
|
|
|
|
await executor.cleanup("FN-WS-1");
|
|
|
|
expect(executor.getWorktreePath("FN-WS-1")).toBeUndefined();
|
|
expect((executor as any).activeWorktrees.has("FN-WS-1")).toBe(false);
|
|
// The browse-only root must never be torn down as if it were a worktree.
|
|
expect(removeSpy).not.toHaveBeenCalled();
|
|
});
|
|
|
|
/*
|
|
FNXC:NodeWorktreeIsolation 2026-07-29-18:20 (U9 re-green; consequence of PR #2531):
|
|
These two cases used to assert that `clearPhantomExecutorBinding` SUCCEEDS while
|
|
session-registry paths are held, and swept (or preserved) them. FN-6756 inverted
|
|
that: `hasLiveSessionSurface` now includes
|
|
`activeSessionRegistry.pathsForTask(taskId).length > 0`, and that guard runs
|
|
BEFORE both branches — so any registered path refuses the clear outright. The old
|
|
expectations describe the pre-#2531 contract.
|
|
|
|
Rewritten to assert the CURRENT contract, which is the P0 fix's actual promise and
|
|
had no direct coverage: a registered surface of any kind means someone is working
|
|
in that worktree, so the reclaim refuses rather than reaping it.
|
|
|
|
⚠️ FLAG FOR THE #2531 OWNER — not fixed here, because it is a product decision:
|
|
the guard appears to make BOTH of the branches it precedes unreachable for their
|
|
stated purpose.
|
|
- the default branch exists to unregister every held registry path (FN-6736);
|
|
- `preserveWorktrees: true` exists to KEEP those registry paths so a
|
|
`moveTask(preserveWorktree:true)` re-dispatch reattaches to the same worktree
|
|
(FN-7249), and its ONLY production caller is the self-healing reclaim at
|
|
self-healing.ts:3565.
|
|
Both require registered paths to do anything, and the guard rejects exactly that
|
|
case. With no registered paths, one sweeps nothing and the other preserves
|
|
nothing. The third case below pins this so the conflict is executable rather than
|
|
prose. Resolving it — e.g. exempting the non-destructive `preserveWorktrees` path,
|
|
or narrowing the guard by kind — belongs to whoever owns FN-6756.
|
|
*/
|
|
it("clearPhantomExecutorBinding refuses while a session-registry path is held (FN-6756)", async () => {
|
|
fx = await createWorkspaceFixture();
|
|
const executor = workspaceExecutor();
|
|
const pA = repoAPath(fx);
|
|
const pB = repoBPath(fx);
|
|
(executor as any).addActiveWorktree("FN-WS-1", pA);
|
|
(executor as any).addActiveWorktree("FN-WS-1", pB);
|
|
activeSessionRegistry.registerPath(pA, { taskId: "FN-WS-1", kind: "executor", ownerKey: "exec:FN-WS-1:a" });
|
|
activeSessionRegistry.registerPath(pB, { taskId: "FN-WS-1", kind: "executor", ownerKey: "exec:FN-WS-1:b" });
|
|
|
|
expect((executor as any).clearPhantomExecutorBinding("FN-WS-1")).toBe(false);
|
|
// Nothing may be torn down on a refusal — that is the whole point of the guard.
|
|
expect((executor as any).activeWorktrees.has("FN-WS-1")).toBe(true);
|
|
expect(activeSessionRegistry.pathsForTask("FN-WS-1")).toEqual(expect.arrayContaining([pA, pB]));
|
|
// Cleanup is in afterEach, so a failure above cannot leak these registrations.
|
|
});
|
|
|
|
it("clearPhantomExecutorBinding (FN-6736) clears every held path once no session surface remains", async () => {
|
|
fx = await createWorkspaceFixture();
|
|
const executor = workspaceExecutor();
|
|
const pA = repoAPath(fx);
|
|
const pB = repoBPath(fx);
|
|
(executor as any).addActiveWorktree("FN-WS-1", pA);
|
|
(executor as any).addActiveWorktree("FN-WS-1", pB);
|
|
|
|
// No registry paths held, so the guard permits the clear. KTD2: a workspace task
|
|
// holds N worktrees and ALL of them must leave the binding, not just the first.
|
|
expect((executor as any).clearPhantomExecutorBinding("FN-WS-1")).toBe(true);
|
|
expect((executor as any).activeWorktrees.has("FN-WS-1")).toBe(false);
|
|
expect((executor as any).executing.has("FN-WS-1")).toBe(false);
|
|
expect(activeSessionRegistry.pathsForTask("FN-WS-1")).toEqual([]);
|
|
});
|
|
|
|
it("FN-7249 preserveWorktrees cannot run while the paths it exists to preserve are registered", async () => {
|
|
fx = await createWorkspaceFixture();
|
|
const executor = workspaceExecutor();
|
|
const pA = repoAPath(fx);
|
|
(executor as any).addActiveWorktree("FN-WS-1", pA);
|
|
activeSessionRegistry.registerPath(pA, { taskId: "FN-WS-1", kind: "executor", ownerKey: "exec:FN-WS-1:a" });
|
|
|
|
// The guard precedes the preserveWorktrees branch, so the one scenario that
|
|
// branch was written for is the one it can never reach.
|
|
expect((executor as any).clearPhantomExecutorBinding("FN-WS-1", { preserveWorktrees: true })).toBe(false);
|
|
expect((executor as any).activeWorktrees.has("FN-WS-1")).toBe(true);
|
|
});
|
|
});
|
|
|
|
describeIfGit("U1 KTD2 — non-workspace task is a one-element Set (regression: unchanged)", () => {
|
|
let fx: WorkspaceFixture;
|
|
afterEach(() => fx?.cleanup());
|
|
|
|
it("getWorktreePath returns the sole path; listWorktreeHolders emits exactly one row", async () => {
|
|
fx = await createWorkspaceFixture();
|
|
const store = createStore();
|
|
const executor = new TaskExecutor(store, fx.repoPath("repo-a")); // single-repo root
|
|
// No workspaceConfig set → single-repo mode.
|
|
const wt = `${fx.repoPath("repo-a")}/.worktrees/fn-001`;
|
|
(executor as any).addActiveWorktree("FN-001", wt);
|
|
|
|
expect(executor.getWorktreePath("FN-001")).toBe(wt);
|
|
expect(executor.listWorktreeHolders()).toEqual([{ taskId: "FN-001", worktreePath: wt }]);
|
|
expect((executor as any).hasActiveWorktreeBinding("FN-001", wt)).toBe(true);
|
|
});
|
|
});
|
|
|
|
describeIfGit("U1 KTD1 — verifyWorktreeInvariants gated off in workspace mode", () => {
|
|
let fx: WorkspaceFixture;
|
|
afterEach(() => fx?.cleanup());
|
|
|
|
it("refuses an unproven zero-acquire workspace task so fn_task_done can requeue for acquisition", async () => {
|
|
fx = await createWorkspaceFixture();
|
|
const store = createStore();
|
|
const executor = new TaskExecutor(store, fx.rootDir);
|
|
(executor as any).workspaceConfig = { repos: fx.repos } as WorkspaceConfig;
|
|
|
|
// A workspace task that acquired ZERO sub-repos has no task.worktree. It must
|
|
// acquire one before claiming completion unless it proves commit-free intent.
|
|
const result = await (executor as any).verifyWorktreeInvariants(makeTask("FN-WS-1", { worktree: undefined }));
|
|
expect(result).toMatchObject({ ok: false, reason: "no_commits" });
|
|
});
|
|
|
|
it("non-workspace task with no worktree still fails the invariant (regression: gate is workspace-only)", async () => {
|
|
fx = await createWorkspaceFixture();
|
|
const store = createStore();
|
|
const executor = new TaskExecutor(store, fx.repoPath("repo-a"));
|
|
// No workspaceConfig.
|
|
const result = await (executor as any).verifyWorktreeInvariants(makeTask("FN-001", { worktree: undefined }));
|
|
expect(result.ok).toBe(false);
|
|
});
|
|
});
|
|
|
|
describeIfGit("U1 KTD1 — scopePromptToWorktree / buildExecutionPrompt no-op in workspace mode", () => {
|
|
let fx: WorkspaceFixture;
|
|
afterEach(() => fx?.cleanup());
|
|
|
|
it("does not rewrite root-anchored paths when a workspace config is present", async () => {
|
|
fx = await createWorkspaceFixture();
|
|
const task = makeTask("FN-WS-1", { prompt: `Edit ${fx.rootDir}/repo-a/src/index.ts and commit.` });
|
|
const config: WorkspaceConfig = { repos: fx.repos };
|
|
// worktreePath === rootDir in workspace mode; the prompt must be returned verbatim.
|
|
const prompt = buildExecutionPrompt(task as any, fx.rootDir, { autoMerge: false } as any, fx.rootDir, undefined, undefined, config);
|
|
expect(prompt).toContain(`${fx.rootDir}/repo-a/src/index.ts`);
|
|
// The workspace repo list is appended (foundation behavior).
|
|
expect(prompt).toContain("repo-a");
|
|
});
|
|
});
|