diff --git a/docs/architecture.md b/docs/architecture.md index cdba754b47..6c5b28926f 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -1795,7 +1795,7 @@ This section preserves the detailed lifecycle/self-healing contracts that were f - **Completion fan-out is synchronous**: `SelfHealingManager.reconcileCompletedTask()` runs on `in-review → done`. Downstream stale `blockedBy` links and residual `fusion/` branch/worktree artifacts are reconciled immediately, not on a periodic sweep. - **In-review stall deadlock**: identical stalls (same code + reason) repeated past `inReviewStallDeadlockThreshold` (default 3) auto-pause with `pausedReason: "in-review-stall-deadlock"` and `status: "failed"`. User-initiated retry paths (dashboard retry, `fn_task_retry`, and CLI `task retry`) clear that automatic deadlock pause so the retry can execute, but they never override explicit/manual pauses or unrelated automatic pause reasons. - **Restart recovery**: `RestartRecoveryCoordinator` classifies interrupted `in-progress` runs. Unusable-worktree session-start failures (`missing`, `incomplete`, `unregistered git worktree`) are recoverable; retries are capped at `MAX_WORKTREE_SESSION_RETRIES=3` before escalating. -- **Executor pre-session liveness gate (FN-4935)**: the gate now skips for fresh acquisitions (`acquisition.source === "fresh"`), emits structured `not_usable_task_worktree:` diagnostics (including canonicalized registered-path snapshots) and a `worktree:incomplete-detected` audit event with `source: "executor-liveness-gate"`, while preserving the existing `taskDoneRetryCount` / `MAX_TASK_DONE_REQUEUE_RETRIES` requeue contract. FN-5772 adds a bounded nested-root self-heal: when `task.worktree` points at a strict descendant of a registered worktree root inside the configured worktrees dir, executor re-anchors `task.worktree` to the git top-level, emits `worktree:reanchored` (`fromPath`, `toPath`, `source`), and proceeds; repo-root/outside-dir/unregistered top-level mismatches still fail. FN-4651 `worktreeSessionRetryCount` remains scoped to the in-review/session-start recovery path. +- **Executor pre-session liveness gate (FN-4935/FN-6861)**: the gate now skips for fresh acquisitions (`acquisition.source === "fresh"`), emits structured `not_usable_task_worktree:` diagnostics (including canonicalized registered-path snapshots) and a `worktree:incomplete-detected` audit event with `source: "executor-liveness-gate"`, while preserving the existing `taskDoneRetryCount` / `MAX_TASK_DONE_REQUEUE_RETRIES` requeue contract. The project repo root is never a usable task worktree even though it is a legitimately registered Git worktree; `classifyTaskWorktree` returns `repo-root` for canonical root-equal paths, and resume acquisition treats that as self-healable stale metadata by clearing `task.worktree` and creating a fresh checkout under the configured worktrees directory. FN-5772 adds a bounded nested-root self-heal: when `task.worktree` points at a strict descendant of a registered worktree root inside the configured worktrees dir, executor re-anchors `task.worktree` to the git top-level, emits `worktree:reanchored` (`fromPath`, `toPath`, `source`), and proceeds; repo-root/outside-dir/unregistered top-level mismatches still fail. FN-4651 `worktreeSessionRetryCount` remains scoped to the in-review/session-start recovery path. - **Stale self-owned active-session reconcile on conflict cleanup (FN-4973)**: when executor worktree-conflict cleanup finds only a same-task stale `activeSessionRegistry` entry and no live in-memory `activeWorktrees` binding for that task/path, it must unregister the stale entry before `removeWorktree` (plus one-shot backstop reconcile on same-task `ActiveSessionWorktreeRemovalError` races). Foreign-task entries remain protected by FN-4811 and must never be reconciled by the requesting task. - **Same-task stale removal canonical helper (FN-5346)**: executor same-task cleanup paths now route pre-removal reconciliation through `reconcileSelfOwnedActiveSessionForRemoval` (via executor helper wiring), so stale self-owned `activeSessionRegistry` residues are cleared only when no live in-memory binding exists, while FN-4811 foreign-owner refusals and live-owner protections remain intact. - **Task title/ID drift (FN-4898)**: active and archived title writes normalize foreign embedded `FN-NNN` tokens via `packages/core/src/task-title-id-drift.ts`. Empty placeholder groups (`()`, `[]`, `{}`) left behind by token stripping are also removed in both `normalizeTitleForTaskId` and `sanitizeTitle` (FN-4978). Lineage is preserved in `sourceParentTaskId` / description markers, not title embeds. FN-5077 extends drift normalization to reject dangling-connector fragments (`"Close as duplicate of"`) so token-stripped residuals never persist as task titles. diff --git a/docs/solutions/logic-errors/repo-root-task-worktree-requeue-loop.md b/docs/solutions/logic-errors/repo-root-task-worktree-requeue-loop.md new file mode 100644 index 0000000000..9e3e706314 --- /dev/null +++ b/docs/solutions/logic-errors/repo-root-task-worktree-requeue-loop.md @@ -0,0 +1,49 @@ +--- +title: "Repo-root task worktree causes executor requeue loop" +date: 2026-06-21 +category: docs/solutions/logic-errors +module: "engine worktree acquisition + executor liveness" +problem_type: logic_error +component: engine +symptoms: + - "A resumed task is repeatedly requeued to todo with realpath_matches_repo_root" + - "git worktree list includes the project root, so worktree classification treats the main checkout as usable" + - "Acquisition returns the repo root again after recovery, and the executor gate rejects it again" +root_cause: invariant_gap +resolution_type: code_fix +severity: high +related_components: + - "packages/engine/src/worktree-pool.ts (classifyTaskWorktree)" + - "packages/engine/src/worktree-acquisition.ts (resume fallback)" + - "packages/engine/src/executor.ts (pre-session liveness gate)" +tags: + - worktrees + - executor + - self-healing + - liveness + - requeue-loop +--- + +# Repo-root task worktree causes executor requeue loop + +## Problem + +A recovered task can carry `task.worktree` that canonicalizes to the project repository root. The root is a valid Git worktree and appears in `git worktree list`, but it is the main checkout, not an isolated task checkout. Before FN-6861, `classifyTaskWorktree(rootDir, rootDir)` returned usable, so resume acquisition returned the root unchanged. The executor then rejected the same path via `realpath_matches_repo_root` and requeued the task, setting up an acquisition → gate → requeue loop. + +## Solution + +Make the invariant explicit at the shared classification boundary: the project root is never a usable task worktree. `classifyTaskWorktree` now compares canonicalized paths and returns `classification: "repo-root"` for root-equal paths even when Git reports the path as registered. + +Because `acquireTaskWorktree` already treats non-usable resume classifications as self-healable stale metadata, a root-valued `task.worktree` is cleared and replaced with a fresh checkout under the configured worktrees directory. The executor liveness gate remains defense-in-depth and emits structured `worktree:incomplete-detected` evidence if a repo-root path still reaches it. + +## Verification + +Cover the invariant at three seams: + +- Classification: real Git repo root registered in `git worktree list` must classify as `repo-root`, including canonical-equal variants such as trailing slashes or symlink-normalized paths. +- Acquisition: resume with `task.worktree === rootDir` must return a fresh `.worktrees/*` (or configured worktrees-dir) checkout and must not return the root. +- Executor diagnostics: if the root reaches the pre-session liveness gate, the audit payload must identify `classification: "repo-root"`, the observed path, the registered snapshot, and that the expected task-worktree pattern excludes the root. + +## Prevention + +Registered Git worktree membership is necessary but not sufficient for task execution. Any new worktree-liveness or self-healing path should call the shared classifier and preserve the distinction between the main checkout (`repo-root`) and isolated task checkouts under the configured worktrees directory. diff --git a/packages/engine/src/__tests__/executor-worktree-liveness.test.ts b/packages/engine/src/__tests__/executor-worktree-liveness.test.ts index 3150ef1a22..6508155c01 100644 --- a/packages/engine/src/__tests__/executor-worktree-liveness.test.ts +++ b/packages/engine/src/__tests__/executor-worktree-liveness.test.ts @@ -56,13 +56,18 @@ describe("FN-4114 worktree liveness assertion", () => { expect(store.moveTask).toHaveBeenCalledWith("FN-4114", "todo", { preserveProgress: true }); }); - it("FN-4114 aborts when worktree realpath collides with repo root", async () => { + it("FN-6861 aborts with structured audit when worktree realpath collides with repo root", async () => { vi.spyOn(worktreePool, "classifyTaskWorktree").mockResolvedValue({ ok: true }); + vi.spyOn(worktreePool, "describeRegisteredWorktrees").mockResolvedValue({ + rawOutput: "worktree /repo\nworktree /repo/.worktrees/swift-falcon\n", + canonicalized: ["/repo", "/repo/.worktrees/swift-falcon"], + }); mockedExecSync.mockImplementation((cmd: string) => { if (cmd.includes("rev-parse HEAD")) return Buffer.from("abc123\n"); return Buffer.from(""); }); const store = createMockStore(); + store.recordRunAuditEvent = vi.fn().mockResolvedValue(undefined); store.getTask.mockResolvedValue(task({ worktree: "/repo" })); const executor = new TaskExecutor(store as any, "/repo"); @@ -70,6 +75,22 @@ describe("FN-4114 worktree liveness assertion", () => { expect(mockedCreateFnAgent).not.toHaveBeenCalled(); expect(store.moveTask).toHaveBeenCalledWith("FN-4114", "todo", { preserveProgress: true }); + expect(store.recordRunAuditEvent).toHaveBeenCalledWith(expect.objectContaining({ + domain: "git", + mutationType: "worktree:incomplete-detected", + target: "/repo", + metadata: expect.objectContaining({ + classification: "repo-root", + observed: "/repo", + observedRealpath: "/repo", + expected: "/repo/.worktrees/* (usable, registered)", + registered: ["/repo", "/repo/.worktrees/swift-falcon"], + registeredContainsObserved: true, + invalidCheckoutPath: "repo-root", + expectedPatternExcludesRepoRoot: true, + terminalAction: "requeue-todo", + }), + })); }); it.each([ diff --git a/packages/engine/src/__tests__/worktree-acquisition.test.ts b/packages/engine/src/__tests__/worktree-acquisition.test.ts index 0a8ad73f2d..cfe0bf4b2f 100644 --- a/packages/engine/src/__tests__/worktree-acquisition.test.ts +++ b/packages/engine/src/__tests__/worktree-acquisition.test.ts @@ -1,4 +1,8 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { execSync } from "node:child_process"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { promisify } from "node:util"; import { acquireTaskWorktree } from "../worktree-acquisition.js"; import { classifyTaskWorktree, PoolDoubleLeaseError } from "../worktree-pool.js"; @@ -36,6 +40,33 @@ vi.mock("../worktree-desktop-artifacts.js", () => ({ removeDesktopBuildArtifacts: vi.fn().mockResolvedValue({ removed: [], skipped: [], failures: [] }), })); +const cleanupPaths: string[] = []; +function track(path: string): string { + cleanupPaths.push(path); + return path; +} + +function git(cwd: string, command: string): string { + return execSync(command, { cwd, encoding: "utf-8", stdio: ["pipe", "pipe", "pipe"] }).trim(); +} + +function makeRepo(): string { + const rootDir = track(mkdtempSync(join(tmpdir(), "fn-6861-acquisition-root-"))); + git(rootDir, "git init -b main"); + git(rootDir, 'git config user.email "test@example.com"'); + git(rootDir, 'git config user.name "Test User"'); + writeFileSync(join(rootDir, "README.md"), "root\n", "utf-8"); + git(rootDir, "git add README.md"); + git(rootDir, 'git commit -m "init"'); + return rootDir; +} + +afterEach(() => { + for (const path of cleanupPaths.splice(0)) { + rmSync(path, { recursive: true, force: true }); + } +}); + describe("acquireTaskWorktree", () => { const task = { id: "FN-1", @@ -263,6 +294,40 @@ describe("acquireTaskWorktree", () => { expect(store.updateTask).toHaveBeenCalledWith("FN-1", { worktree: null, branch: null, sessionFile: null }); }); + it("FN-6861 creates a fresh configured worktree when a resumed assignment points at the repo root", async () => { + const rootDir = makeRepo(); + const actualPool = await vi.importActual("../worktree-pool.js"); + vi.mocked(classifyTaskWorktree).mockImplementationOnce(actualPool.classifyTaskWorktree); + const freshPath = join(rootDir, ".worktrees", "fn-6861-fresh"); + const createWorktree = vi.fn().mockResolvedValue({ path: freshPath, branch: "fusion/fn-1" }); + const auditGit = vi.fn().mockResolvedValue(undefined); + + const result = await acquireTaskWorktree({ + task: { ...task, worktree: rootDir, branch: "fusion/fn-1", sessionFile: "/tmp/session.json" }, + rootDir, + store, + settings: {} as any, + createWorktree, + audit: { git: auditGit } as any, + }); + + expect(result).toMatchObject({ + worktreePath: freshPath, + branch: "fusion/fn-1", + source: "fresh", + isResume: false, + }); + expect(result.worktreePath).not.toBe(rootDir); + expect(result.worktreePath).toContain(`${join(rootDir, ".worktrees")}/`); + expect(auditGit).toHaveBeenCalledWith(expect.objectContaining({ + type: "worktree:incomplete-detected", + target: rootDir, + metadata: expect.objectContaining({ classification: "repo-root", source: "resume" }), + })); + expect(store.updateTask).toHaveBeenCalledWith("FN-1", { worktree: null, branch: null, sessionFile: null }); + expect(store.updateTask).toHaveBeenCalledWith("FN-1", { worktree: freshPath, branch: "fusion/fn-1" }); + }); + it("falls through to fresh creation when pool acquire throws PoolDoubleLeaseError", async () => { const createWorktree = vi.fn().mockResolvedValue({ path: "/tmp/new", branch: "fusion/fn-1" }); const result = await acquireTaskWorktree({ diff --git a/packages/engine/src/__tests__/worktree-pool-liveness.test.ts b/packages/engine/src/__tests__/worktree-pool-liveness.test.ts index ef39b442b9..f73f6b0fce 100644 --- a/packages/engine/src/__tests__/worktree-pool-liveness.test.ts +++ b/packages/engine/src/__tests__/worktree-pool-liveness.test.ts @@ -93,6 +93,20 @@ describeIfGit("worktree liveness gating (FN-4682)", () => { }, expected: { ok: true } as const, }, + { + name: "repo-root", + setup: () => { + const rootDir = track(makeRepo((dir) => { + git(dir, 'git commit --allow-empty -m "init"'); + })); + return { rootDir, worktreePath: rootDir }; + }, + expected: { + ok: false, + classification: "repo-root", + reason: "worktree path is the project root, not a task worktree", + } as const, + }, { name: "missing", setup: () => { @@ -162,6 +176,17 @@ describeIfGit("worktree liveness gating (FN-4682)", () => { await expect(classifyTaskWorktree(rootDir, worktreePath)).resolves.toEqual(expected); }); + it("FN-6861: rejects canonical-equal repo root paths before accepting registered worktrees", async () => { + const rootDir = track(makeRepo((dir) => { + git(dir, 'git commit --allow-empty -m "init"'); + })); + await expect(classifyTaskWorktree(rootDir, `${rootDir}/`)).resolves.toEqual({ + ok: false, + classification: "repo-root", + reason: "worktree path is the project root, not a task worktree", + }); + }); + it("FN-4682: rejects missing worktree directory", async () => { const rootDir = track(makeRepo((dir) => { git(dir, 'git commit --allow-empty -m "init"'); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index c9a997913f..6da67fd11e 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -7603,18 +7603,34 @@ export class TaskExecutor { const priorRequeues = task.taskDoneRetryCount ?? 0; const nextRequeueCount = priorRequeues + 1; const terminalAction = priorRequeues < MAX_TASK_DONE_REQUEUE_RETRIES ? "requeue-todo" : "park-in-review"; - if (livenessClassification) { + const isRepoRootCollision = livenessFailure === "realpath_matches_repo_root"; + const auditClassification = livenessClassification ?? (isRepoRootCollision ? "repo-root" : null); + const auditReason = livenessFailureReason ?? (isRepoRootCollision ? "worktree path realpath matches the project root, not a task worktree" : null); + /* + * FNXC:WorktreeLiveness 2026-06-21-11:10: + * The executor still keeps the repo-root realpath check as defense in depth. If acquisition ever hands the root to this gate, emit structured evidence that separates the invalid checkout path from the normal git registered-worktree snapshot and the configured task-worktree pattern. + */ + if (auditClassification) { + const registeredContainsObserved = registeredPaths.includes(observedWorktreeRealpath); await audit.git({ type: "worktree:incomplete-detected", target: worktreePath, metadata: { - classification: livenessClassification, - reason: livenessFailureReason ?? undefined, + classification: auditClassification, + reason: auditReason ?? undefined, source: "executor-liveness-gate", taskId: task.id, retryCount: nextRequeueCount, maxRetries: MAX_TASK_DONE_REQUEUE_RETRIES, terminalAction, + observed: worktreePath, + observedRealpath: observedWorktreeRealpath, + expected, + registered: visibleRegistered, + registeredTotal: registeredPaths.length, + registeredContainsObserved, + invalidCheckoutPath: isRepoRootCollision ? "repo-root" : undefined, + expectedPatternExcludesRepoRoot: isRepoRootCollision, }, }); } diff --git a/packages/engine/src/worktree-acquisition.ts b/packages/engine/src/worktree-acquisition.ts index 11ca2c0850..917d0939ef 100644 --- a/packages/engine/src/worktree-acquisition.ts +++ b/packages/engine/src/worktree-acquisition.ts @@ -220,6 +220,10 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro let isResume = Boolean(task.worktree && existsSync(worktreePath)); if (task.worktree && isResume) { const resumeClassification = await classifyTaskWorktree(rootDir, worktreePath); + /* + * FNXC:WorktreeLiveness 2026-06-21-11:10: + * A resumed task can carry a stale or recovered `task.worktree` that points at the repository root. Treat every non-usable classification, including repo-root, as self-healable metadata so acquisition clears the assignment and creates a fresh task worktree instead of feeding the executor's defensive gate forever. + */ if (!resumeClassification.ok) { await audit?.git({ type: "worktree:incomplete-detected", diff --git a/packages/engine/src/worktree-pool.ts b/packages/engine/src/worktree-pool.ts index 9e72a546ae..e0f379a5c6 100644 --- a/packages/engine/src/worktree-pool.ts +++ b/packages/engine/src/worktree-pool.ts @@ -191,7 +191,7 @@ export async function isInsideGitWorkTree(worktreePath: string): Promise