Fix leaked orphan worktree dirs failing execute node

Directories under .worktrees/ that survive with a dangling .git pointer
(present on disk, but their .git/worktrees/<name> admin entry is gone) are
invisible to `git worktree list`/`prune` yet collide with freshly generated
worktree names. The executor's conflict cleanup then fails with
"is not a working tree", failing the workflow graph at node 'execute' after
3 attempts.

- executor.ts: extend FN-4813 stale-conflict recovery to also treat
  "is not a working tree" and ENOENT (not just "validation failed, cannot
  remove working tree") as "no live worktree here" — prune the admin entry,
  force-remove the leftover dir, and proceed with fresh creation.
- worktree-pool.ts: reapOrphanWorktrees skipped any dir on mere .git-file
  presence, contradicting its own documented invariant. Resolve the .git
  pointer and only skip when the gitdir target exists; reap dangling
  pointers like any other orphan so they stop accumulating across runs.
- Tests for both the dangling (reaped) and valid (skipped) .git cases.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-06-20 09:58:23 -07:00
parent eca96fbd0b
commit 438cd75eaf
4 changed files with 134 additions and 18 deletions

View File

@@ -0,0 +1,10 @@
---
"@runfusion/fusion": patch
---
Fix worktree-creation failures (and the `Workflow graph terminated with failure at node 'execute'` they surface as) caused by leaked orphan worktree directories.
A directory under `.worktrees/` that survives with a *dangling* `.git` pointer — present on disk, but the `.git/worktrees/<name>` admin entry it references is gone — is invisible to `git worktree list` and untouched by `git worktree prune`, yet collides with a freshly generated worktree name. When the executor then tries to clean up the "conflict", `git worktree remove --force` fails with `is not a working tree` and the whole `execute` node fails after 3 attempts.
- **On-demand recovery (`executor.ts`):** the FN-4813 stale-conflict recovery now also treats `is not a working tree` and `ENOENT` (not just `validation failed, cannot remove working tree`) as "no live worktree at this path" — it prunes any admin entry, force-removes the leftover directory, and proceeds with fresh worktree creation instead of failing.
- **Leak prevention (`worktree-pool.ts`):** `reapOrphanWorktrees` previously skipped any dir on the mere *presence* of a `.git` file ("may be partially registered"), contradicting its own documented invariant. It now resolves the `.git` pointer and only skips when the gitdir target actually exists; a dangling pointer is reaped like any other half-initialized orphan, so these directories no longer accumulate across runs.

View File

@@ -52,6 +52,7 @@ vi.mock("node:fs", () => ({
existsSync: vi.fn().mockReturnValue(true), existsSync: vi.fn().mockReturnValue(true),
lstatSync: vi.fn().mockReturnValue({ isDirectory: () => true, isSymbolicLink: () => false }), lstatSync: vi.fn().mockReturnValue({ isDirectory: () => true, isSymbolicLink: () => false }),
readdirSync: vi.fn().mockReturnValue([]), readdirSync: vi.fn().mockReturnValue([]),
readFileSync: vi.fn().mockReturnValue(""),
rmSync: vi.fn(), rmSync: vi.fn(),
})); }));
@@ -73,13 +74,14 @@ import {
import { BranchConflictError } from "../branch-conflicts.js"; import { BranchConflictError } from "../branch-conflicts.js";
import * as branchConflictModule from "../branch-conflicts.js"; import * as branchConflictModule from "../branch-conflicts.js";
import { execSync } from "node:child_process"; import { execSync } from "node:child_process";
import { existsSync, lstatSync, readdirSync, rmSync } from "node:fs"; import { existsSync, lstatSync, readdirSync, readFileSync, rmSync } from "node:fs";
import type { Task, Column } from "@fusion/core"; import type { Task, Column } from "@fusion/core";
const mockedExecSync = vi.mocked(execSync); const mockedExecSync = vi.mocked(execSync);
const mockedExistsSync = vi.mocked(existsSync); const mockedExistsSync = vi.mocked(existsSync);
const mockedLstatSync = vi.mocked(lstatSync); const mockedLstatSync = vi.mocked(lstatSync);
const mockedReaddirSync = vi.mocked(readdirSync); const mockedReaddirSync = vi.mocked(readdirSync);
const mockedReadFileSync = vi.mocked(readFileSync);
const mockedRmSync = vi.mocked(rmSync); const mockedRmSync = vi.mocked(rmSync);
const mockedPruneWorktreeAdminEntries = vi.mocked(worktreePrune.pruneWorktreeAdminEntries); const mockedPruneWorktreeAdminEntries = vi.mocked(worktreePrune.pruneWorktreeAdminEntries);
const TEST_TASK_ID = "FN-test"; const TEST_TASK_ID = "FN-test";
@@ -1109,5 +1111,50 @@ describe("reapOrphanWorktrees", () => {
expect(mockedRmSync).toHaveBeenCalledWith("/root/.worktrees/half-built", { recursive: true, force: true }); 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/.ai-merge", expect.anything());
}); });
// FN-6782 follow-up: a directory whose `.git` points to a missing admin entry is leak
// residue (invisible to `git worktree list`/`prune`), not "partially registered". It
// must be reaped — otherwise it collides with freshly generated worktree names and
// breaks `execute`. Previously the reaper skipped on mere `.git` presence.
it("reaps a dir with a dangling .git pointer (admin gitdir missing)", async () => {
mockedReaddirSync.mockReturnValue([makeDirEntry("leaked-wt")] as any);
// `.git` is a link FILE (not a dir); the worktree dir itself is a dir.
mockedLstatSync.mockImplementation((p: any) =>
(String(p).endsWith("/.git")
? { isDirectory: () => false, isSymbolicLink: () => false }
: { isDirectory: () => true, isSymbolicLink: () => false }) as any,
);
mockedReadFileSync.mockReturnValue("gitdir: /root/.git/worktrees/leaked-wt\n" as any);
mockedExistsSync.mockImplementation((p) => {
const s = String(p);
// .worktrees root exists; the .git link file exists; the gitdir target does NOT.
return s === "/root/.worktrees" || s === "/root/.worktrees/leaked-wt/.git";
});
const removed = await reapOrphanWorktrees("/root");
expect(removed).toBe(1);
expect(mockedRmSync).toHaveBeenCalledWith("/root/.worktrees/leaked-wt", { recursive: true, force: true });
});
it("skips a dir with a valid .git pointer (admin gitdir exists)", async () => {
mockedReaddirSync.mockReturnValue([makeDirEntry("live-wt")] as any);
mockedLstatSync.mockImplementation((p: any) =>
(String(p).endsWith("/.git")
? { isDirectory: () => false, isSymbolicLink: () => false }
: { isDirectory: () => true, isSymbolicLink: () => false }) as any,
);
mockedReadFileSync.mockReturnValue("gitdir: /root/.git/worktrees/live-wt\n" as any);
mockedExistsSync.mockImplementation((p) => {
const s = String(p);
// The gitdir target exists too → treat as (maybe) registered, leave it alone.
return s === "/root/.worktrees" || s === "/root/.worktrees/live-wt/.git" || s === "/root/.git/worktrees/live-wt";
});
const removed = await reapOrphanWorktrees("/root");
expect(removed).toBe(0);
expect(mockedRmSync).not.toHaveBeenCalledWith("/root/.worktrees/live-wt", expect.anything());
});
}); });

View File

@@ -14247,14 +14247,24 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
return true; return true;
} catch (error: unknown) { } catch (error: unknown) {
const errorMessage = error instanceof Error ? error.message : String(error); const errorMessage = error instanceof Error ? error.message : String(error);
// FN-4811 follow-up (FN-4813): when `git worktree remove --force` fails with // FN-4811 follow-up (FN-4813): when `git worktree remove --force` fails because the
// "fatal: validation failed, cannot remove working tree", the worktree directory // conflicting path isn't a recoverable git worktree, treat it as already-cleaned:
// doesn't exist on disk and the git admin entry (if any) is stale. Treat as // prune any stale admin entry, force-remove the leftover directory, best-effort delete
// already-cleaned: prune the stale admin entry, best-effort delete the branch, and // the branch, and return success so the caller can proceed with fresh worktree creation.
// return success so the caller can proceed with fresh worktree creation. Without // Without this recovery, every `tryCreateWorktree` retry on such a path fails with
// this recovery, every `tryCreateWorktree` retry on a stale conflict path fails // "automatic cleanup failed".
// with "automatic cleanup failed". //
if (/validation failed, cannot remove working tree/i.test(errorMessage)) { // Three variants land here, all meaning "no live worktree to preserve at this path":
// 1. `validation failed, cannot remove working tree` — stale admin entry, dir missing.
// 2. `is not a working tree` — an orphan directory exists on disk but git never
// registered it (e.g. a leaked worktree dir that outlived its admin entry). This
// is the FN-6782 leak residue that collides with freshly generated worktree names.
// 3. `No such file or directory` / ENOENT — the path is already gone.
const staleConflictPath =
/validation failed, cannot remove working tree/i.test(errorMessage) ||
/is not a working tree/i.test(errorMessage) ||
/no such file or directory|ENOENT/i.test(errorMessage);
if (staleConflictPath) {
try { try {
await execAsync("git worktree prune", { await execAsync("git worktree prune", {
cwd: this.rootDir, cwd: this.rootDir,
@@ -14265,6 +14275,14 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
const pruneMsg = pruneErr instanceof Error ? pruneErr.message : String(pruneErr); const pruneMsg = pruneErr instanceof Error ? pruneErr.message : String(pruneErr);
executorLog.warn(`${taskId}: git worktree prune failed during stale-path cleanup of ${worktreePath}: ${pruneMsg}`); executorLog.warn(`${taskId}: git worktree prune failed during stale-path cleanup of ${worktreePath}: ${pruneMsg}`);
} }
// An orphan directory ("is not a working tree") won't be removed by prune — git
// doesn't track it. Force-remove the leftover dir so the colliding name is free.
try {
await rm(worktreePath, { recursive: true, force: true });
} catch (rmErr: unknown) {
const rmMsg = rmErr instanceof Error ? rmErr.message : String(rmErr);
executorLog.warn(`${taskId}: failed to remove orphan worktree directory ${worktreePath}: ${rmMsg}`);
}
try { try {
await execAsync(`git branch -D "${branch}"`, { cwd: this.rootDir }); await execAsync(`git branch -D "${branch}"`, { cwd: this.rootDir });
this.store.clearStaleExecutionStartBranchReferences([branch], taskId); this.store.clearStaleExecutionStartBranchReferences([branch], taskId);
@@ -14273,7 +14291,7 @@ Backward compat fallback: if JSON is unavailable, you may still begin output wit
} }
await this.store.logEntry( await this.store.logEntry(
taskId, taskId,
`Cleaned up stale conflicting worktree admin entry (validation failed — path likely missing on disk)`, `Cleaned up stale conflicting worktree (no live worktree at path — pruned admin entry and removed orphan directory)`,
worktreePath, worktreePath,
); );
return true; return true;

View File

@@ -1,7 +1,7 @@
import { exec } from "node:child_process"; import { exec } from "node:child_process";
import { promisify } from "node:util"; import { promisify } from "node:util";
import { existsSync, lstatSync, readdirSync, rmSync, realpathSync } from "node:fs"; import { existsSync, lstatSync, readdirSync, readFileSync, rmSync, realpathSync } from "node:fs";
import { basename, join, relative, resolve, isAbsolute } from "node:path"; import { basename, dirname, join, relative, resolve, isAbsolute } from "node:path";
import type { ColumnId, SecretsStore, Settings, TaskStore, WorktrunkSettings } from "@fusion/core"; import type { ColumnId, SecretsStore, Settings, TaskStore, WorktrunkSettings } from "@fusion/core";
import { assertCleanBranchAtBase, inspectBranchConflict } from "./branch-conflicts.js"; import { assertCleanBranchAtBase, inspectBranchConflict } from "./branch-conflicts.js";
import { worktreePoolLog } from "./logger.js"; import { worktreePoolLog } from "./logger.js";
@@ -848,6 +848,34 @@ export async function cleanupOrphanedWorktrees(
* @param projectRoot - Absolute path to the project root (parent of `.worktrees/`) * @param projectRoot - Absolute path to the project root (parent of `.worktrees/`)
* @returns Number of orphan directories removed * @returns Number of orphan directories removed
*/ */
/**
* Resolve a worktree's `.git` pointer to the gitdir admin path it references.
*
* Returns:
* - `"directory"` if `.git` is a real directory (a normal repo, not a worktree
* link) — callers should treat that as "leave it alone".
* - an absolute path string for a `gitdir: <path>` link file (relative targets
* are resolved against the worktree directory).
* - `null` if the pointer can't be read or parsed.
*
* Callers decide whether the target exists; a missing target means the link is
* dangling (leak residue) and the directory is safe to reap.
*/
function resolveGitdirPointer(dotGitPath: string): string | "directory" | null {
try {
if (lstatSync(dotGitPath).isDirectory()) {
return "directory";
}
const raw = readFileSync(dotGitPath, "utf8").trim();
const match = /^gitdir:\s*(.+)$/.exec(raw);
if (!match) return null;
const target = match[1].trim();
return isAbsolute(target) ? target : resolve(dirname(dotGitPath), target);
} catch {
return null;
}
}
export async function reapOrphanWorktrees( export async function reapOrphanWorktrees(
projectRoot: string, projectRoot: string,
settings?: Pick<Settings, "worktreesDir">, settings?: Pick<Settings, "worktreesDir">,
@@ -902,16 +930,29 @@ export async function reapOrphanWorktrees(
// Belt-and-suspenders: skip if a .git file exists AND points to an existing gitdir. // Belt-and-suspenders: skip if a .git file exists AND points to an existing gitdir.
// This guards against races where git registered the worktree between our list // This guards against races where git registered the worktree between our list
// call and now, or against a broken repo whose porcelain is unreliable. // call and now, or against a broken repo whose porcelain is unreliable.
//
// FN-6782 follow-up: a *dangling* `.git` (file present, but the admin entry it
// points to is gone) is NOT "partially registered" — it is leak residue from a
// worktree whose admin entry was pruned while the directory survived. Such a dir
// is invisible to `git worktree list`/`prune` yet collides with freshly generated
// worktree names and breaks `execute` (cleanup can't `git worktree remove` a path
// git never registered). Only skip when the gitdir target actually exists; reap
// dangling pointers like any other half-initialized orphan.
const dotGit = join(resolvedFull, ".git"); const dotGit = join(resolvedFull, ".git");
if (existsSync(dotGit)) { if (existsSync(dotGit)) {
// If there's a .git file/dir, don't touch it — assertValidWorktreeSession const gitdirTarget = resolveGitdirPointer(dotGit);
// will handle it on the next agent start. if (gitdirTarget === "directory" || (gitdirTarget && existsSync(gitdirTarget))) {
worktreePoolLog.log(`reapOrphanWorktrees: skipping ${name} (has .git entry but not in registered list — may be partially registered)`); // Valid registration (or a real .git dir) — leave it; assertValidWorktreeSession
continue; // will handle it on the next agent start.
worktreePoolLog.log(`reapOrphanWorktrees: skipping ${name} (has .git entry but not in registered list — may be partially registered)`);
continue;
}
worktreePoolLog.log(`reapOrphanWorktrees: ${name} has a dangling .git pointer (admin entry missing) — treating as orphan`);
// fall through to removal
} }
// This directory is on disk but has no .git entry and is not a registered // This directory is on disk but has no valid .git entry and is not a registered
// worktree — it is a half-initialized orphan. Remove it. // worktree — it is a half-initialized / leaked orphan. Remove it.
try { try {
try { try {
await cleanupSecretsEnvFile({ await cleanupSecretsEnvFile({