From a5c3476cb2f5bd59f53f0b7045d380b11c4a8152 Mon Sep 17 00:00:00 2001 From: Phil Larson Date: Sun, 9 Aug 2026 22:39:39 -0700 Subject: [PATCH] fix: harden pinned worktree recovery cleanup (#3402) ## Summary - serialize pinned-path classification, orphan preservation, quarantine reconciliation, and recreation under one reservation - preserve cross-filesystem orphans atomically beside the configured worktree root and retain the newest 10 generated entries per recovery root - exclude recovery containers from pool and self-healing scans, with fail-closed symlink and active-session guards - document recovery location and retention behavior ## Test plan - `pnpm --filter @fusion/engine exec vitest run src/__tests__/worktree-acquisition.test.ts src/__tests__/worktree-paths.test.ts src/__tests__/worktree-pool.test.ts src/__tests__/self-healing-tempdir-sweep.test.ts --silent=passed-only --reporter=dot` - `pnpm --filter @fusion/engine typecheck` - `pnpm --filter @fusion/engine build` - `pnpm test:gate:static` - `pnpm check:changesets --strict` - `pnpm check:fnxc-future-dates` ## Summary by CodeRabbit * **New Features** * Preserves orphaned pinned worktrees during recovery, including across filesystems. * Retains the 10 most recent recovery entries and safely skips active or invalid entries. * Keeps recovery data separate from normal worktree discovery, cleanup, and capacity checks. * Adds safeguards for path containment, active-session ownership, and concurrent recovery. * **Bug Fixes** * Prevents pinned worktree data from being lost during recreation or quarantine cleanup. * Ensures recovery cleanup failures do not interrupt worktree acquisition. * **Documentation** * Documented orphan recovery, retention, fallback behavior, and cleanup safeguards. --- ...inned-worktree-recovery-review-followup.md | 7 + docs/architecture.md | 1 + docs/task-management.md | 8 +- .../self-healing-tempdir-sweep.test.ts | 12 +- .../__tests__/worktree-acquisition.test.ts | 198 +++++++++++++++++- .../src/__tests__/worktree-paths.test.ts | 9 + .../src/__tests__/worktree-pool.test.ts | 18 +- packages/engine/src/self-healing.ts | 6 +- .../src/worktree/worktree-acquisition.ts | 145 ++++++++++--- .../engine/src/worktree/worktree-paths.ts | 9 + packages/engine/src/worktree/worktree-pool.ts | 10 +- 11 files changed, 380 insertions(+), 43 deletions(-) create mode 100644 .changeset/pinned-worktree-recovery-review-followup.md diff --git a/.changeset/pinned-worktree-recovery-review-followup.md b/.changeset/pinned-worktree-recovery-review-followup.md new file mode 100644 index 0000000000..32767096b0 --- /dev/null +++ b/.changeset/pinned-worktree-recovery-review-followup.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Harden task-pinned orphan preservation and retention against unsafe recovery cleanup. +category: fix +dev: Serializes recovery under the pinned-path reservation, bounds retained orphans, and excludes recovery containers from scans. diff --git a/docs/architecture.md b/docs/architecture.md index 4caa1e4b6f..b644b5dfb8 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -624,6 +624,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 + - **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. - **Reviewer**: `reviewStep()` (`reviewer.ts`) performs plan/code/spec reviews diff --git a/docs/task-management.md b/docs/task-management.md index cb19ac8e63..3e469ed239 100644 --- a/docs/task-management.md +++ b/docs/task-management.md @@ -110,7 +110,13 @@ This layer complements, rather than replaces, FN-4829 similarity detection, FN-4 Archiving a workspace (multi-repository) task now synchronously removes every recorded per-sub-repository worktree, including archives initiated by `fn_task_archive` and CLI paths that do not construct an executor. Each path is protected by a per-repository cross-process reservation until backend removal and branch cleanup finish. If one removal fails, its reservation is quarantined and the next acquisition reconciles that orphan; successful sibling repositories are still released. `archiveTask(..., { cleanup: false })` intentionally retains worktrees, and the self-healing workspace sweep remains an idempotent backstop. -#### Explicit duplicate-marker guard (FN-5220) +### Task-pinned orphan recovery + +When `worktreeNaming: "task-id"` finds an inactive pinned directory whose Git metadata is incomplete or unregistered, Fusion preserves the whole directory before recreating the task worktree at the same path. The normal preservation root is `/.fusion/recovery/worktrees`. If the configured worktree directory is on another filesystem, Fusion retries the atomic rename under `/.fusion-recovery/worktrees` instead of using copy-and-delete. + +Each preservation root retains the newest 10 Fusion-generated orphan directories. Pruning is best-effort and skips unknown names, symlinks, unreadable entries, and paths with active sessions. Copy artifacts elsewhere if they need indefinite retention. Worktree discovery, cleanup, and capacity scans treat `.fusion-recovery` as an internal container rather than a task worktree. + +### Explicit duplicate-marker guard (FN-5220) Fusion also recognizes the canonical one-line redirect marker: diff --git a/packages/engine/src/__tests__/self-healing-tempdir-sweep.test.ts b/packages/engine/src/__tests__/self-healing-tempdir-sweep.test.ts index f38cfd4bde..bcf910e40e 100644 --- a/packages/engine/src/__tests__/self-healing-tempdir-sweep.test.ts +++ b/packages/engine/src/__tests__/self-healing-tempdir-sweep.test.ts @@ -159,27 +159,33 @@ function sweepAudits(audits: any[]) { } describe("SelfHealingManager worktrees-dir sweeps", () => { - it("excludes the .ai-merge container from unregistered-orphan reap while removing genuine orphans", async () => { + it("excludes internal containers from unregistered-orphan reap while removing genuine orphans", async () => { const worktreesDir = join(projectRoot, ".worktrees"); const aiMergeContainer = join(worktreesDir, ".ai-merge"); + const recoveryContainer = join(worktreesDir, ".fusion-recovery"); const orphan = join(worktreesDir, "half-built"); mkdirSync(aiMergeContainer, { recursive: true }); + mkdirSync(recoveryContainer, { recursive: true }); mkdirSync(orphan, { recursive: true }); const { manager } = makeManager({ recycleWorktrees: true }); await expect((manager as any).reapUnregisteredOrphans()).resolves.toBe(1); expect(existsSync(aiMergeContainer)).toBe(true); + expect(existsSync(recoveryContainer)).toBe(true); expect(existsSync(orphan)).toBe(false); expect(fsState.rmCalls).toContain(orphan); expect(fsState.rmCalls).not.toContain(aiMergeContainer); + expect(fsState.rmCalls).not.toContain(recoveryContainer); }); - it("excludes the .ai-merge container from cap enforcement while removing genuine idle worktrees", async () => { + it("excludes internal containers from cap enforcement while removing genuine idle worktrees", async () => { const worktreesDir = join(projectRoot, ".worktrees"); const aiMergeContainer = join(worktreesDir, ".ai-merge"); + const recoveryContainer = join(worktreesDir, ".fusion-recovery"); const idle = join(worktreesDir, "idle-wt"); mkdirSync(aiMergeContainer, { recursive: true }); + mkdirSync(recoveryContainer, { recursive: true }); mkdirSync(idle, { recursive: true }); childState.execStdout = gitWorktreeList(["idle-wt"]); const { manager } = makeManager({ maxWorktrees: 0 }); @@ -187,7 +193,9 @@ describe("SelfHealingManager worktrees-dir sweeps", () => { await expect((manager as any).enforceWorktreeCap()).resolves.toBeUndefined(); expect(existsSync(aiMergeContainer)).toBe(true); + expect(existsSync(recoveryContainer)).toBe(true); expect(childState.execCalls.some((command) => command.includes(".ai-merge"))).toBe(false); + expect(childState.execCalls.some((command) => command.includes(".fusion-recovery"))).toBe(false); expect(childState.execCalls.some((command) => command.includes("idle-wt"))).toBe(true); }); }); diff --git a/packages/engine/src/__tests__/worktree-acquisition.test.ts b/packages/engine/src/__tests__/worktree-acquisition.test.ts index 1d97a905f7..b00a909c6c 100644 --- a/packages/engine/src/__tests__/worktree-acquisition.test.ts +++ b/packages/engine/src/__tests__/worktree-acquisition.test.ts @@ -1,9 +1,11 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { execSync } from "node:child_process"; -import { existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { randomUUID } from "node:crypto"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, realpathSync, rmSync, symlinkSync, utimesSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { promisify } from "node:util"; +import { acquireWorktreePathReservation } from "@fusion/core"; import { acquireTaskWorktree, RepoRootWorktreeError, WorktreeBaseRefreshError } from "../worktree/worktree-acquisition.js"; import { classifyTaskWorktree, PoolDoubleLeaseError } from "../worktree/worktree-pool.js"; import * as desktopArtifacts from "../worktree/worktree-desktop-artifacts.js"; @@ -78,8 +80,22 @@ function makeRepo(): string { return rootDir; } +function seedPreservedOrphans(recoveryRoot: string, count: number): string[] { + mkdirSync(recoveryRoot, { recursive: true }); + return Array.from({ length: count }, (_, index) => { + const name = `fn-${100 + index}-${randomUUID()}`; + const path = join(recoveryRoot, name); + mkdirSync(path); + writeFileSync(join(path, "artifact"), `${index}\n`, "utf-8"); + const timestamp = new Date(Date.now() - (count - index) * 60_000); + utimesSync(path, timestamp, timestamp); + return name; + }); +} + afterEach(() => { vi.restoreAllMocks(); + activeSessionRegistry.clear(); for (const path of cleanupPaths.splice(0)) { rmSync(path, { recursive: true, force: true }); } @@ -97,6 +113,7 @@ describe("acquireTaskWorktree", () => { let store: any; beforeEach(() => { vi.clearAllMocks(); + activeSessionRegistry.clear(); /* FNXC:EngineTests 2026-07-20-23:55: clearAllMocks wipes the hoisted classifyTaskWorktree mockResolvedValue({ ok: true }). @@ -421,6 +438,181 @@ describe("acquireTaskWorktree", () => { expect(renameWorktreeDirectory).toHaveBeenCalledTimes(2); }); + it("retains only the newest ten generated orphan directories in the primary recovery root", async () => { + const rootDir = makeRepo(); + const pinnedPath = join(rootDir, ".worktrees", "fn-1"); + const recoveryRoot = join(rootDir, ".fusion", "recovery", "worktrees"); + const seeded = seedPreservedOrphans(recoveryRoot, 11); + const unknownPath = join(recoveryRoot, "operator-notes"); + const symlinkTarget = track(mkdtempSync(join(tmpdir(), "fn-orphan-retention-symlink-target-"))); + const generatedSymlink = join(recoveryRoot, `fn-999-${randomUUID()}`); + mkdirSync(pinnedPath, { recursive: true }); + mkdirSync(unknownPath); + symlinkSync(symlinkTarget, generatedSymlink); + activeSessionRegistry.registerPath(join(realpathSync(recoveryRoot), seeded[0]), { + taskId: "FN-RETAIN", + kind: "executor", + ownerKey: "executor:FN-RETAIN", + }); + writeFileSync(join(pinnedPath, "artifact"), "fresh\n", "utf-8"); + vi.mocked(classifyTaskWorktree).mockResolvedValueOnce({ + ok: false, + classification: "unregistered", + reason: "not registered in git worktree list", + }); + + const result = await acquireTaskWorktree({ + task: { ...task, worktree: pinnedPath, branch: "fusion/fn-1" }, + rootDir, + store, + settings: { worktreeNaming: "task-id", recycleWorktrees: false }, + }); + + const generatedDirectories = readdirSync(recoveryRoot, { withFileTypes: true }) + .filter((entry) => entry.isDirectory() && /^fn-\d+-[0-9a-f-]{36}$/.test(entry.name)) + .map((entry) => entry.name); + expect(result.worktreePath).toBe(pinnedPath); + expect(generatedDirectories).toHaveLength(11); + expect(generatedDirectories).toContain(seeded[0]); + expect(generatedDirectories).not.toContain(seeded[1]); + expect(existsSync(unknownPath)).toBe(true); + expect(existsSync(generatedSymlink)).toBe(true); + }); + + it("retains only the newest ten generated orphan directories in the EXDEV fallback root", async () => { + const rootDir = makeRepo(); + const externalWorktrees = track(mkdtempSync(join(tmpdir(), "fn-external-retention-worktrees-"))); + const pinnedPath = join(externalWorktrees, "fn-1"); + const recoveryRoot = join(externalWorktrees, ".fusion-recovery", "worktrees"); + const seeded = seedPreservedOrphans(recoveryRoot, 11); + const unknownPath = join(recoveryRoot, "operator-notes"); + mkdirSync(pinnedPath, { recursive: true }); + mkdirSync(unknownPath); + activeSessionRegistry.registerPath(join(realpathSync(recoveryRoot), seeded[0]), { + taskId: "FN-RETAIN-EXDEV", + kind: "executor", + ownerKey: "executor:FN-RETAIN-EXDEV", + }); + writeFileSync(join(pinnedPath, "artifact"), "fresh\n", "utf-8"); + vi.mocked(classifyTaskWorktree).mockResolvedValueOnce({ + ok: false, + classification: "incomplete", + reason: "missing .git metadata", + }); + const actualFs = await vi.importActual("node:fs/promises"); + const renameWorktreeDirectory = vi.fn() + .mockRejectedValueOnce(Object.assign(new Error("cross-device link"), { code: "EXDEV" })) + .mockImplementationOnce(actualFs.rename); + + const result = await acquireTaskWorktree({ + task: { ...task, worktree: pinnedPath, branch: "fusion/fn-1" }, + rootDir, + store, + settings: { worktreeNaming: "task-id", worktreesDir: externalWorktrees, recycleWorktrees: false }, + renameWorktreeDirectory, + }); + + const generatedDirectories = readdirSync(recoveryRoot, { withFileTypes: true }) + .filter((entry) => entry.isDirectory() && /^fn-\d+-[0-9a-f-]{36}$/.test(entry.name)) + .map((entry) => entry.name); + expect(result.worktreePath).toBe(pinnedPath); + expect(generatedDirectories).toHaveLength(11); + expect(generatedDirectories).toContain(seeded[0]); + expect(generatedDirectories).not.toContain(seeded[1]); + expect(existsSync(unknownPath)).toBe(true); + }); + + it("continues recreation when orphan-preservation audit and task logging fail after rename", async () => { + const rootDir = makeRepo(); + const pinnedPath = join(rootDir, ".worktrees", "fn-1"); + mkdirSync(pinnedPath, { recursive: true }); + writeFileSync(join(pinnedPath, "artifact"), "stale\n", "utf-8"); + vi.mocked(classifyTaskWorktree).mockResolvedValueOnce({ + ok: false, + classification: "incomplete", + reason: "missing .git metadata", + }); + const auditFilesystem = vi.fn().mockRejectedValue(new Error("audit unavailable")); + const loggerWarn = vi.fn(); + store.logEntry.mockImplementation(async (_taskId: string, message: string) => { + if (message.includes("Preserved orphaned task-pinned directory")) throw new Error("task log unavailable"); + }); + + const result = await acquireTaskWorktree({ + task: { ...task, worktree: pinnedPath, branch: "fusion/fn-1" }, + rootDir, + store, + settings: { worktreeNaming: "task-id", recycleWorktrees: false }, + audit: { filesystem: auditFilesystem, git: vi.fn().mockResolvedValue(undefined) } as any, + logger: { log: vi.fn(), warn: loggerWarn }, + }); + + expect(result.worktreePath).toBe(pinnedPath); + expect(existsSync(join(pinnedPath, ".git"))).toBe(true); + expect(auditFilesystem).toHaveBeenCalledTimes(1); + expect(store.logEntry).toHaveBeenCalledWith( + "FN-1", + expect.stringContaining("Preserved orphaned task-pinned directory"), + expect.any(String), + undefined, + ); + expect(loggerWarn).toHaveBeenCalledWith(expect.stringContaining("audit unavailable")); + expect(loggerWarn).toHaveBeenCalledWith(expect.stringContaining("task log unavailable")); + expect(loggerWarn).not.toHaveBeenCalledWith(expect.stringContaining("[object Object]")); + }); + + it("reconciles a durable quarantine for an absent pinned path before recreation", async () => { + const rootDir = makeRepo(); + const worktreesDir = join(rootDir, ".worktrees"); + const pinnedPath = join(worktreesDir, "fn-1"); + mkdirSync(worktreesDir, { recursive: true }); + git(rootDir, `git worktree add -b fusion/fn-1 ${JSON.stringify(pinnedPath)} main`); + rmSync(pinnedPath, { recursive: true, force: true }); + const failedArchiveReservation = await acquireWorktreePathReservation({ + canonicalPath: pinnedPath, + worktreesDir, + rootDir, + }); + await failedArchiveReservation.quarantine("simulated archive failure"); + + const result = await acquireTaskWorktree({ + task: { ...task, worktree: pinnedPath, branch: "fusion/fn-1" }, + rootDir, + store, + settings: { worktreeNaming: "task-id", recycleWorktrees: false }, + }); + + expect(result.worktreePath).toBe(pinnedPath); + expect(existsSync(join(pinnedPath, ".git"))).toBe(true); + expect(git(rootDir, "git worktree list --porcelain")).toContain(pinnedPath); + }); + + it("refuses quarantine reconciliation when an active session owns the absent pinned path", async () => { + const rootDir = makeRepo(); + const worktreesDir = join(rootDir, ".worktrees"); + const pinnedPath = join(worktreesDir, "fn-1"); + mkdirSync(worktreesDir, { recursive: true }); + const failedArchiveReservation = await acquireWorktreePathReservation({ + canonicalPath: pinnedPath, + worktreesDir, + rootDir, + }); + await failedArchiveReservation.quarantine("simulated archive failure"); + activeSessionRegistry.registerPath(pinnedPath, { taskId: "FN-2", kind: "executor", ownerKey: "executor:FN-2" }); + const createWorktree = vi.fn(); + + await expect(acquireTaskWorktree({ + task: { ...task, worktree: pinnedPath, branch: "fusion/fn-1" }, + rootDir, + store, + settings: { worktreeNaming: "task-id", recycleWorktrees: false }, + createWorktree, + })).rejects.toThrow(`Refusing to reconcile absent task-pinned worktree owned by an active session: ${pinnedPath}`); + + expect(createWorktree).not.toHaveBeenCalled(); + expect(existsSync(pinnedPath)).toBe(false); + }); + it("preserves the orphan in place when its path becomes active during recovery", async () => { const rootDir = makeRepo(); const pinnedPath = join(rootDir, ".worktrees", "fn-1"); @@ -472,7 +664,7 @@ describe("acquireTaskWorktree", () => { rootDir, store, settings: { worktreeNaming: "task-id", recycleWorktrees: false }, - })).rejects.toThrow(/outside the project root/); + })).rejects.toThrow(`Refusing to use recovery directory outside ${realpathSync(join(rootDir, ".fusion", "recovery"))}`); expect(readFileSync(join(pinnedPath, ".build", "cache"), "utf-8")).toBe("stale\n"); expect(readdirSync(outside)).toHaveLength(0); @@ -497,7 +689,7 @@ describe("acquireTaskWorktree", () => { rootDir, store, settings: { worktreeNaming: "task-id", recycleWorktrees: false }, - })).rejects.toThrow(/outside the project root/); + })).rejects.toThrow(`Refusing to use recovery directory outside ${realpathSync(join(rootDir, ".fusion"))}`); expect(readFileSync(join(pinnedPath, ".build", "cache"), "utf-8")).toBe("stale\n"); expect(readdirSync(outside)).toHaveLength(0); diff --git a/packages/engine/src/__tests__/worktree-paths.test.ts b/packages/engine/src/__tests__/worktree-paths.test.ts index 390c647f54..e2e5c91a5e 100644 --- a/packages/engine/src/__tests__/worktree-paths.test.ts +++ b/packages/engine/src/__tests__/worktree-paths.test.ts @@ -3,7 +3,9 @@ import { homedir } from "node:os"; import { join, resolve } from "node:path"; import { AI_MERGE_DIRNAME, + WORKTREE_RECOVERY_DIRNAME, isAiMergeContainerDir, + isWorktreeContainerDir, isInsideConfiguredWorktreesDir, resolveAiMergeRootPath, resolveTaskWorktreePath, @@ -65,6 +67,13 @@ describe("worktree-paths", () => { expect(isAiMergeContainerDir(".ai-merge-child")).toBe(false); }); + it("identifies internal worktree containers without hiding task worktrees", () => { + expect(isWorktreeContainerDir(AI_MERGE_DIRNAME)).toBe(true); + expect(isWorktreeContainerDir(WORKTREE_RECOVERY_DIRNAME)).toBe(true); + expect(isWorktreeContainerDir("fusion-ai-merge-fn-1-abc")).toBe(false); + expect(isWorktreeContainerDir(".fusion-recovery-child")).toBe(false); + }); + it("detects paths inside and outside configured dir", () => { const dir = resolveWorktreesDir(rootDir, { worktreesDir: "../{repo}.worktrees" } as any); expect(isInsideConfiguredWorktreesDir(rootDir, { worktreesDir: "../{repo}.worktrees" } as any, join(dir, "fn-1"))).toBe(true); diff --git a/packages/engine/src/__tests__/worktree-pool.test.ts b/packages/engine/src/__tests__/worktree-pool.test.ts index 0cce03a5b5..6402d495ad 100644 --- a/packages/engine/src/__tests__/worktree-pool.test.ts +++ b/packages/engine/src/__tests__/worktree-pool.test.ts @@ -941,18 +941,24 @@ describe("scanIdleWorktrees", () => { ); }); - it("excludes the .ai-merge container even when git lists clean-room children", async () => { + it("excludes internal containers even when git lists their children", async () => { mockedReaddirSync.mockReturnValue([ makeDirEntry(".ai-merge"), + makeDirEntry(".fusion-recovery"), makeDirEntry("registered-wt"), ] as any); - mockRegisteredWorktrees("/root", [".ai-merge/fusion-ai-merge-fn-1-active", "registered-wt"]); + mockRegisteredWorktrees("/root", [ + ".ai-merge/fusion-ai-merge-fn-1-active", + ".fusion-recovery/worktrees/fn-1-preserved", + "registered-wt", + ]); const store = createMockStore([]); const idle = await scanIdleWorktrees("/root", store); expect(idle).toEqual(["/root/.worktrees/registered-wt"]); expect(idle).not.toContain("/root/.worktrees/.ai-merge"); + expect(idle).not.toContain("/root/.worktrees/.fusion-recovery"); }); it("does not return unregistered directories for pool rehydration", async () => { @@ -1113,9 +1119,10 @@ describe("cleanupOrphanedWorktrees", () => { expect(removeCalls).toHaveLength(0); }); - it("excludes the .ai-merge container while still removing genuine unregistered orphans", async () => { + it("excludes internal containers while still removing genuine unregistered orphans", async () => { mockedReaddirSync.mockReturnValue([ makeDirEntry(".ai-merge"), + makeDirEntry(".fusion-recovery"), makeDirEntry("broken-wt"), ] as any); mockRegisteredWorktrees("/root", []); @@ -1130,6 +1137,7 @@ describe("cleanupOrphanedWorktrees", () => { force: true, }); expect(mockedRmSync).not.toHaveBeenCalledWith("/root/.worktrees/.ai-merge", expect.anything()); + expect(mockedRmSync).not.toHaveBeenCalledWith("/root/.worktrees/.fusion-recovery", expect.anything()); }); it("removes unregistered directories even when stale active task metadata references them", async () => { @@ -1163,9 +1171,10 @@ describe("reapOrphanWorktrees", () => { mockedLstatSync.mockReturnValue({ isDirectory: () => true, isSymbolicLink: () => false } as any); }); - it("excludes the .ai-merge container while removing half-initialized task worktrees", async () => { + it("excludes internal containers while removing half-initialized task worktrees", async () => { mockedReaddirSync.mockReturnValue([ makeDirEntry(".ai-merge"), + makeDirEntry(".fusion-recovery"), makeDirEntry("half-built"), ] as any); @@ -1174,6 +1183,7 @@ describe("reapOrphanWorktrees", () => { expect(removed).toBe(1); expect(mockedRmSync).toHaveBeenCalledWith("/root/.worktrees/half-built", { recursive: true, force: true }); expect(mockedRmSync).not.toHaveBeenCalledWith("/root/.worktrees/.ai-merge", expect.anything()); + expect(mockedRmSync).not.toHaveBeenCalledWith("/root/.worktrees/.fusion-recovery", expect.anything()); }); // FN-6782 follow-up: a directory whose `.git` points to a missing admin entry is leak diff --git a/packages/engine/src/self-healing.ts b/packages/engine/src/self-healing.ts index e29e34c6d3..0ddc93014e 100644 --- a/packages/engine/src/self-healing.ts +++ b/packages/engine/src/self-healing.ts @@ -96,7 +96,7 @@ import { getTaskCompletionBlockerForStore } from "./execution/task-completion.js import { shouldReclaimWedgedMerge } from "./merge/merge-reclaim-policy.js"; import { advanceIntegrationBranchRef } from "./merge/merger-ref-update-advance.js"; -import { isAiMergeContainerDir, resolveAiMergeRootPath, resolveLegacyAiMergeRootPath, resolveWorktreesDir } from "./worktree/worktree-paths.js"; +import { isWorktreeContainerDir, resolveAiMergeRootPath, resolveLegacyAiMergeRootPath, resolveWorktreesDir } from "./worktree/worktree-paths.js"; import { canonicalFusionBranchName, resolveTaskWorkingBranch } from "./worktree/worktree-names.js"; import { preservedWorktreeTargetPathForTask } from "./worktree/worktree-pinning.js"; import { resolveIntegrationBranch } from "./merge/integration-branch.js"; @@ -15115,7 +15115,7 @@ const movedTask = await this.store.moveTask(task.id, completeLane); let dirs: string[]; try { dirs = readdirSync(worktreesDir, { withFileTypes: true }) - .filter((e) => e.isDirectory() && !isAiMergeContainerDir(e.name)) + .filter((e) => e.isDirectory() && !isWorktreeContainerDir(e.name)) .map((e) => join(worktreesDir, e.name)); } catch (err: unknown) { log.warn(`Failed to read .worktrees/ for unregistered orphan reap: ${err instanceof Error ? err.message : String(err)}`); @@ -15434,7 +15434,7 @@ const movedTask = await this.store.moveTask(task.id, completeLane); const cap = (settings.maxWorktrees ?? 4) * 2; const entries = readdirSync(worktreesDir, { withFileTypes: true }); - const dirs = entries.filter((e) => e.isDirectory() && !isAiMergeContainerDir(e.name)); + const dirs = entries.filter((e) => e.isDirectory() && !isWorktreeContainerDir(e.name)); if (dirs.length <= cap) return; diff --git a/packages/engine/src/worktree/worktree-acquisition.ts b/packages/engine/src/worktree/worktree-acquisition.ts index fb1a12c0ab..76c8f48b9e 100644 --- a/packages/engine/src/worktree/worktree-acquisition.ts +++ b/packages/engine/src/worktree/worktree-acquisition.ts @@ -1,12 +1,12 @@ import { existsSync } from "node:fs"; import { randomUUID } from "node:crypto"; -import { mkdir, readFile, realpath, rename, stat, writeFile } from "node:fs/promises"; +import { lstat, mkdir, readFile, readdir, realpath, rename, rm, stat, writeFile } from "node:fs/promises"; import { exec } from "node:child_process"; import { isAbsolute, join, relative, resolve } from "node:path"; import { promisify } from "node:util"; import {acquireWorktreePathReservation, canonicalizeWorktreePath, type RunMutationContext, type Settings, type Task, type TaskStore, type SecretsStore} from "@fusion/core"; import { generateWorktreeName, resolveTaskWorkingBranch, slugify } from "./worktree-names.js"; -import { resolveTaskWorktreePathForBackend, resolveWorktreesDir } from "./worktree-paths.js"; +import { resolveTaskWorktreePathForBackend, resolveWorktreesDir, WORKTREE_RECOVERY_DIRNAME } from "./worktree-paths.js"; import { hydrateWorktreeDb } from "./worktree-db-hydrate.js"; import { formatError } from "../logger.js"; import { classifyBootstrapMisbinding, isBranchConflictError, reanchorBranchToBase } from "../execution/branch-conflicts.js"; @@ -50,6 +50,8 @@ import { refreshReusedWorktreeBase, type WorktreeBaseRefreshResult } from "../wo const execAsync = promisify(exec); const WORKTREE_BACKEND_MARKER = "fusion-worktree-backend-kind"; +const PRESERVED_ORPHAN_RETENTION_COUNT = 10; +const PRESERVED_ORPHAN_NAME_PATTERN = /^[a-z0-9][a-z0-9-]*-[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/; async function resolveWorktreeBackendMarkerPath(worktreePath: string): Promise { const { stdout } = await execAsync(`git rev-parse --git-path ${JSON.stringify(WORKTREE_BACKEND_MARKER)}`, { @@ -158,7 +160,7 @@ async function ensureContainedDirectory(parentCanonicalPath: string, name: strin const canonicalCandidate = await realpath(candidate); const candidateRelative = relative(parentCanonicalPath, canonicalCandidate); if (candidateRelative === "" || candidateRelative.startsWith("..") || isAbsolute(candidateRelative)) { - throw new Error(`Refusing to use recovery directory outside the project root: ${canonicalCandidate}`); + throw new Error(`Refusing to use recovery directory outside ${parentCanonicalPath}: ${canonicalCandidate}`); } if (!(await stat(canonicalCandidate)).isDirectory()) { throw new Error(`Refusing to use non-directory recovery path: ${canonicalCandidate}`); @@ -166,6 +168,62 @@ async function ensureContainedDirectory(parentCanonicalPath: string, name: strin return canonicalCandidate; } +interface PreservedOrphanCandidate { + path: string; + canonicalPath: string; + mtimeMs: number; +} + +async function inspectPreservedOrphanCandidate( + canonicalRecoveryRoot: string, + name: string, +): Promise { + if (!PRESERVED_ORPHAN_NAME_PATTERN.test(name)) return null; + const path = join(canonicalRecoveryRoot, name); + try { + const pathStat = await lstat(path); + if (!pathStat.isDirectory() || pathStat.isSymbolicLink()) return null; + const canonicalPath = await realpath(path); + const candidateRelative = relative(canonicalRecoveryRoot, canonicalPath); + if (candidateRelative !== name || candidateRelative.includes("/") || candidateRelative.includes("\\") || isAbsolute(candidateRelative)) { + return null; + } + return { path, canonicalPath, mtimeMs: pathStat.mtimeMs }; + } catch { + return null; + } +} + +/** + * FNXC:TaskPinnedWorktrees 2026-08-10-01:12: + * Each actual orphan-recovery root retains its newest ten generated task-id-plus-UUID directories. Pruning is fail-soft and removes only direct canonical non-symlink directories after an immediate active-session check; unknown, unstatable, or active entries are preserved. + */ +async function prunePreservedOrphanDirectories( + canonicalRecoveryRoot: string, + logger?: { warn: (message: string) => void }, +): Promise { + try { + const entries = await readdir(canonicalRecoveryRoot, { withFileTypes: true }); + const candidates = (await Promise.all(entries.map((entry) => + inspectPreservedOrphanCandidate(canonicalRecoveryRoot, entry.name)))) + .filter((candidate): candidate is PreservedOrphanCandidate => candidate !== null) + .sort((left, right) => right.mtimeMs - left.mtimeMs || right.path.localeCompare(left.path)); + + for (const candidate of candidates.slice(PRESERVED_ORPHAN_RETENTION_COUNT)) { + try { + const current = await inspectPreservedOrphanCandidate(canonicalRecoveryRoot, candidate.path.slice(canonicalRecoveryRoot.length + 1)); + if (!current || current.canonicalPath !== candidate.canonicalPath) continue; + if (activeSessionRegistry.isPathActive(current.path) || activeSessionRegistry.isPathActive(current.canonicalPath)) continue; + await rm(current.path, { recursive: true, force: true }); + } catch (error) { + logger?.warn(`Failed to prune preserved orphan directory ${candidate.path}: ${formatError(error).message}`); + } + } + } catch (error) { + logger?.warn(`Failed to inspect preserved orphan retention root ${canonicalRecoveryRoot}: ${formatError(error).message}`); + } +} + function configuredCommandErrorMessage(result: { spawnError?: string | Error; timedOut?: boolean; exitCode?: number | null }): string { if (result.spawnError) return `Failed to start command: ${result.spawnError}`; if (result.timedOut) return "Command timed out"; @@ -727,6 +785,27 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro if (activeSessionRegistry.isPathActive(pinnedPath)) return true; return (await classifyTaskWorktree(rootDir, pinnedPath)).ok; }, + /* + * FNXC:TaskPinnedWorktrees 2026-08-10-01:12: + * Pinned acquisition owns the reservation through classification, orphan preservation, quarantine reconciliation, and recreation. Preserve an existing directory for classification; for an absent path, fail closed on containment or active in-process ownership before reusing the guarded backend removal path. + */ + reconcileQuarantined: async () => { + if (existsSync(pinnedPath)) return; + if (!isInsideWorktreesDir(rootDir, pinnedPath, settings)) { + throw new Error(`Refusing to reconcile quarantined task-pinned worktree outside configured worktrees directory: ${pinnedPath}`); + } + if (activeSessionRegistry.isPathActive(pinnedPath)) { + throw new Error(`Refusing to reconcile absent task-pinned worktree owned by an active session: ${pinnedPath}`); + } + await removeWorktree({ + worktreePath: pinnedPath, + rootDir, + settings, + taskId: task.id, + reason: RemovalReason.ExecutorDispose, + force: true, + }); + }, }); try { worktreePath = pinnedPath; @@ -778,16 +857,18 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro && !activeSessionRegistry.isPathActive(pinnedPath); if (preserveAsOrphanDirectory) { const canonicalRoot = await realpath(rootDir); + /* + * FNXC:TaskPinnedWorktrees 2026-08-10-01:12: + * Recovery directory components must resolve inside their canonical parent. A symlinked ancestor or container fails closed before orphan contents move. + */ const fusionRoot = await ensureContainedDirectory(canonicalRoot, ".fusion"); const recoveryRoot = await ensureContainedDirectory(fusionRoot, "recovery"); - const canonicalRecoveryRoot = await ensureContainedDirectory(recoveryRoot, "worktrees"); - // The path reservation serializes classification, preservation, and - // recreation across processes; this final probe also protects against - // an in-process owner that registered before the reservation was held. + let actualRecoveryRoot = await ensureContainedDirectory(recoveryRoot, "worktrees"); + // FNXC:TaskPinnedWorktrees 2026-08-10-01:12: The reservation serializes cross-process recovery; recheck in-process liveness immediately before the rename so a newly registered owner is never displaced. if (activeSessionRegistry.isPathActive(pinnedPath)) { throw new Error(`Task-pinned worktree ${pinnedPath} became active during orphan recovery`); } - let preservedPath = join(canonicalRecoveryRoot, `${task.id.toLowerCase()}-${randomUUID()}`); + let preservedPath = join(actualRecoveryRoot, `${task.id.toLowerCase()}-${randomUUID()}`); try { await renameWorktreeDirectory(pinnedPath, preservedPath); } catch (renameError) { @@ -798,27 +879,41 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro * configured worktree root instead of weakening recovery to recursive copy-and-delete. */ const canonicalWorktreesRoot = await realpath(resolveWorktreesDir(rootDir, settings)); - const localRecoveryRoot = await ensureContainedDirectory(canonicalWorktreesRoot, ".fusion-recovery"); + const localRecoveryRoot = await ensureContainedDirectory(canonicalWorktreesRoot, WORKTREE_RECOVERY_DIRNAME); const localRecoveryWorktrees = await ensureContainedDirectory(localRecoveryRoot, "worktrees"); + actualRecoveryRoot = localRecoveryWorktrees; preservedPath = join(localRecoveryWorktrees, `${task.id.toLowerCase()}-${randomUUID()}`); await renameWorktreeDirectory(pinnedPath, preservedPath); } - await audit?.filesystem({ - type: "file:write", - target: preservedPath, - metadata: { - taskId: task.id, - classification: classification.classification, - reason: "task-pinned-orphan-preserved", - sourcePath: pinnedPath, - }, - }); - await store.logEntry( - task.id, - `Preserved orphaned task-pinned directory ${pinnedPath} before recreation`, - preservedPath, - runContext, - ); + /* + * FNXC:TaskPinnedWorktrees 2026-08-10-01:12: + * Once rename has preserved the orphan, audit, task-log, and retention work are independent best-effort observability/housekeeping. Their failures must not strand the pinned path or block recreation, and warnings must retain the concrete formatted failure message. + */ + try { + await audit?.filesystem({ + type: "file:write", + target: preservedPath, + metadata: { + taskId: task.id, + classification: classification.classification, + reason: "task-pinned-orphan-preserved", + sourcePath: pinnedPath, + }, + }); + } catch (error) { + logger?.warn(`${task.id}: failed to audit preserved orphan ${preservedPath}: ${formatError(error).message}`); + } + try { + await store.logEntry( + task.id, + `Preserved orphaned task-pinned directory ${pinnedPath} before recreation`, + preservedPath, + runContext, + ); + } catch (error) { + logger?.warn(`${task.id}: failed to log preserved orphan ${preservedPath}: ${formatError(error).message}`); + } + await prunePreservedOrphanDirectories(actualRecoveryRoot, logger); } else { await removeWorktree({ rootDir, diff --git a/packages/engine/src/worktree/worktree-paths.ts b/packages/engine/src/worktree/worktree-paths.ts index b399444ed1..5fe938f99b 100644 --- a/packages/engine/src/worktree/worktree-paths.ts +++ b/packages/engine/src/worktree/worktree-paths.ts @@ -5,11 +5,20 @@ import type { WorktreeBackendKind } from "./worktree-backend.js"; import { canonicalizePath } from "./worktree-pool.js"; export const AI_MERGE_DIRNAME = ".ai-merge"; +export const WORKTREE_RECOVERY_DIRNAME = ".fusion-recovery"; export function isAiMergeContainerDir(name: string): boolean { return name === AI_MERGE_DIRNAME; } +/** + * FNXC:TaskPinnedWorktrees 2026-08-10-01:12: + * Cross-filesystem orphan recovery stores preserved task directories under a container inside the configured worktree root. Discovery, cleanup, and capacity scans must treat both internal containers as boundaries rather than task worktrees. + */ +export function isWorktreeContainerDir(name: string): boolean { + return isAiMergeContainerDir(name) || name === WORKTREE_RECOVERY_DIRNAME; +} + export function resolveAiMergeRootPath( rootDir: string, settings: Pick | undefined, diff --git a/packages/engine/src/worktree/worktree-pool.ts b/packages/engine/src/worktree/worktree-pool.ts index fcdb7a8c94..f577813771 100644 --- a/packages/engine/src/worktree/worktree-pool.ts +++ b/packages/engine/src/worktree/worktree-pool.ts @@ -8,7 +8,7 @@ import { assertCleanBranchAtBase, inspectBranchConflict } from "../execution/bra import { worktreePoolLog } from "../logger.js"; /* */ -import { isAiMergeContainerDir, isInsideConfiguredWorktreesDir, resolveWorktreesDir } from "./worktree-paths.js"; +import { isInsideConfiguredWorktreesDir, isWorktreeContainerDir, resolveWorktreesDir } from "./worktree-paths.js"; import { canonicalFusionBranchName } from "./worktree-names.js"; import { resolveWorktrunkBinary, @@ -893,7 +893,7 @@ export async function scanIdleWorktrees( try { const entries = readdirSync(worktreesDir, { withFileTypes: true }); dirs = entries - .filter((e) => e.isDirectory() && !isAiMergeContainerDir(e.name)) + .filter((e) => e.isDirectory() && !isWorktreeContainerDir(e.name)) .map((e) => join(worktreesDir, e.name)); } catch (err: unknown) { const errorMessage = err instanceof Error ? err.message : String(err); @@ -981,7 +981,7 @@ export async function cleanupOrphanedWorktrees( if (existsSync(worktreesDir)) { try { dirs = readdirSync(worktreesDir, { withFileTypes: true }) - .filter((e) => e.isDirectory() && !isAiMergeContainerDir(e.name)) + .filter((e) => e.isDirectory() && !isWorktreeContainerDir(e.name)) .map((e) => join(worktreesDir, e.name)); } catch (err: unknown) { const errorMessage = err instanceof Error ? err.message : String(err); @@ -1106,8 +1106,8 @@ export async function reapOrphanWorktrees( try { entries = readdirSync(worktreesDir, { withFileTypes: true }) .filter((e) => { - // Only real directories — never symlinks; never the dedicated AI-merge container. - if (!e.isDirectory() || isAiMergeContainerDir(e.name)) return false; + // Only real directories — never symlinks or internal worktree containers. + if (!e.isDirectory() || isWorktreeContainerDir(e.name)) return false; try { return lstatSync(join(worktreesDir, e.name)).isDirectory() && !lstatSync(join(worktreesDir, e.name)).isSymbolicLink(); } catch {