From ec1d29e8181a830fc028e33566dc1386df82c7c5 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Mon, 22 Jun 2026 19:11:08 -0700 Subject: [PATCH] FN-6922: guard worktree acquisition returns from repo root Prevent task worktree acquisition from handing the project root back to executors. - Add a repo-root return guard across resume, pool, and fresh acquisition paths. - Clear invalid repo-root task assignments and create a fresh worktree fallback when safe. - Share repo-root canonicalization and expand regression coverage for acquisition liveness. - Document the acquisition guard and add a patch changeset. Files changed: .changeset/fn-6922-repo-root-acquisition-guard.md | 5 + .../repo-root-task-worktree-requeue-loop.md | 9 +- .../__tests__/executor-worktree-liveness.test.ts | 35 +++ .../worktree-acquisition-secrets-env.test.ts | 9 +- .../src/__tests__/worktree-acquisition.test.ts | 91 ++++++- packages/engine/src/worktree-acquisition.ts | 272 +++++++++++++-------- packages/engine/src/worktree-pool.ts | 8 +- 7 files changed, 304 insertions(+), 125 deletions(-) Fusion-Task-Id: FN-6922 Fusion-Task-Lineage: c16cee1b-de16-4d5f-90e0-132f94fa8377 --- .../fn-6922-repo-root-acquisition-guard.md | 5 + .../repo-root-task-worktree-requeue-loop.md | 9 +- .../executor-worktree-liveness.test.ts | 35 +++ .../worktree-acquisition-secrets-env.test.ts | 9 +- .../__tests__/worktree-acquisition.test.ts | 91 +++++- packages/engine/src/worktree-acquisition.ts | 272 +++++++++++------- packages/engine/src/worktree-pool.ts | 8 +- 7 files changed, 304 insertions(+), 125 deletions(-) create mode 100644 .changeset/fn-6922-repo-root-acquisition-guard.md diff --git a/.changeset/fn-6922-repo-root-acquisition-guard.md b/.changeset/fn-6922-repo-root-acquisition-guard.md new file mode 100644 index 0000000000..702a3d851e --- /dev/null +++ b/.changeset/fn-6922-repo-root-acquisition-guard.md @@ -0,0 +1,5 @@ +--- +"@runfusion/fusion": patch +--- + +Prevent task worktree acquisition from returning the project repository root by enforcing a non-root postcondition across resume, pooled, and fresh checkout paths. 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 index 9e3e706314..1014f80ecc 100644 --- a/docs/solutions/logic-errors/repo-root-task-worktree-requeue-loop.md +++ b/docs/solutions/logic-errors/repo-root-task-worktree-requeue-loop.md @@ -14,7 +14,7 @@ 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/worktree-acquisition.ts (resume fallback + return guard)" - "packages/engine/src/executor.ts (pre-session liveness gate)" tags: - worktrees @@ -34,7 +34,9 @@ A recovered task can carry `task.worktree` that canonicalizes to the project rep 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. +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. FN-6922 adds the same invariant as an acquisition return postcondition: every existing, pooled, and fresh-created return path is checked immediately before returning to executor/heartbeat callers. If a return candidate canonicalizes to the project root, acquisition emits `worktree:incomplete-detected` with `source: "acquire-return-guard"`, clears worktree metadata, and attempts one fresh checkout; if the fresh checkout is also root-equal, it throws `RepoRootWorktreeError` instead of returning the root. + +The executor liveness gate remains defense-in-depth and emits structured `worktree:incomplete-detected` evidence if a repo-root path still reaches it. ## Verification @@ -42,8 +44,9 @@ 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. +- Acquisition return guard: even if a classifier mock/regression marks a root path usable, or if a custom fresh backend returns the root, `acquireTaskWorktree` must either self-heal to a non-root checkout or throw `RepoRootWorktreeError`. - 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. +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. Any new `acquireTaskWorktree` return branch must also flow through the return guard so branch-local checks cannot be the only line of defense. diff --git a/packages/engine/src/__tests__/executor-worktree-liveness.test.ts b/packages/engine/src/__tests__/executor-worktree-liveness.test.ts index 6508155c01..ee02e5692d 100644 --- a/packages/engine/src/__tests__/executor-worktree-liveness.test.ts +++ b/packages/engine/src/__tests__/executor-worktree-liveness.test.ts @@ -57,6 +57,13 @@ describe("FN-4114 worktree liveness assertion", () => { }); it("FN-6861 aborts with structured audit when worktree realpath collides with repo root", async () => { + vi.spyOn(worktreeAcquisition, "acquireTaskWorktree").mockResolvedValue({ + worktreePath: "/repo", + branch: "fusion/fn-4114", + source: "existing", + hydrated: true, + isResume: true, + }); vi.spyOn(worktreePool, "classifyTaskWorktree").mockResolvedValue({ ok: true }); vi.spyOn(worktreePool, "describeRegisteredWorktrees").mockResolvedValue({ rawOutput: "worktree /repo\nworktree /repo/.worktrees/swift-falcon\n", @@ -93,6 +100,34 @@ describe("FN-4114 worktree liveness assertion", () => { })); }); + it("FN-6922 proceeds when acquisition self-heals a repo-root assignment to a fresh worktree", async () => { + vi.spyOn(worktreeAcquisition, "acquireTaskWorktree").mockResolvedValue({ + worktreePath: "/repo/.worktrees/fn-6922-fresh", + branch: "fusion/fn-4114", + source: "fresh", + hydrated: true, + isResume: false, + }); + vi.spyOn(worktreePool, "classifyTaskWorktree").mockResolvedValue({ ok: false, classification: "repo-root", reason: "would have been root before acquisition guard" }); + const store = createMockStore(); + store.recordRunAuditEvent = vi.fn().mockResolvedValue(undefined); + store.getTask.mockResolvedValue(task({ worktree: "/repo", sessionFile: null })); + + mockedCreateFnAgent.mockImplementation(async () => ({ + session: { prompt: vi.fn().mockResolvedValue(undefined), dispose: vi.fn() }, + }) as any); + + const executor = new TaskExecutor(store as any, "/repo"); + await executor.execute(task({ worktree: "/repo", sessionFile: null }) as any); + + expect(mockedCreateFnAgent).toHaveBeenCalled(); + expect(store.moveTask).not.toHaveBeenCalledWith("FN-4114", "todo", { preserveProgress: true }); + expect(store.recordRunAuditEvent).not.toHaveBeenCalledWith(expect.objectContaining({ + mutationType: "worktree:incomplete-detected", + metadata: expect.objectContaining({ classification: "repo-root", source: "executor-liveness-gate" }), + })); + }); + it.each([ { name: "default worktreesDir", settings: {}, outsidePath: "/repo/not-a-worktree" }, { name: "absolute worktreesDir", settings: { worktreesDir: "/custom/trees" }, outsidePath: "/repo/not-a-worktree" }, diff --git a/packages/engine/src/__tests__/worktree-acquisition-secrets-env.test.ts b/packages/engine/src/__tests__/worktree-acquisition-secrets-env.test.ts index 235ef66ea0..2b62b4140c 100644 --- a/packages/engine/src/__tests__/worktree-acquisition-secrets-env.test.ts +++ b/packages/engine/src/__tests__/worktree-acquisition-secrets-env.test.ts @@ -1,3 +1,5 @@ +import { dirname } from "node:path"; + import { describe, it, expect, vi, beforeEach } from "vitest"; const { writeSecretsEnvFile } = vi.hoisted(() => ({ writeSecretsEnvFile: vi.fn() })); @@ -60,9 +62,12 @@ describe("worktree-acquisition secrets env hook", () => { }); it("does not call writer for existing resume", async () => { + const existingWorktree = process.cwd(); + const projectRoot = dirname(existingWorktree); + await acquireTaskWorktree({ - task: { ...task, branch: "fusion/fn-1", worktree: process.cwd() }, - rootDir: process.cwd(), + task: { ...task, branch: "fusion/fn-1", worktree: existingWorktree }, + rootDir: projectRoot, store, settings: { secretsEnv: { enabled: true } } as any, createWorktree: vi.fn(), diff --git a/packages/engine/src/__tests__/worktree-acquisition.test.ts b/packages/engine/src/__tests__/worktree-acquisition.test.ts index cfe0bf4b2f..374b53a465 100644 --- a/packages/engine/src/__tests__/worktree-acquisition.test.ts +++ b/packages/engine/src/__tests__/worktree-acquisition.test.ts @@ -2,9 +2,9 @@ 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 { dirname, join } from "node:path"; import { promisify } from "node:util"; -import { acquireTaskWorktree } from "../worktree-acquisition.js"; +import { acquireTaskWorktree, RepoRootWorktreeError } from "../worktree-acquisition.js"; import { classifyTaskWorktree, PoolDoubleLeaseError } from "../worktree-pool.js"; import * as desktopArtifacts from "../worktree-desktop-artifacts.js"; import * as branchConflicts from "../branch-conflicts.js"; @@ -87,15 +87,16 @@ describe("acquireTaskWorktree", () => { }); it("reuses existing usable worktree", async () => { + const worktreePath = process.cwd(); const result = await acquireTaskWorktree({ - task: { ...task, worktree: process.cwd(), branch: "fusion/fn-1" }, - rootDir: process.cwd(), + task: { ...task, worktree: worktreePath, branch: "fusion/fn-1" }, + rootDir: dirname(worktreePath), store, settings: {}, createWorktree: vi.fn(), }); expect(result.source).toBe("existing"); - expect(result.worktreePath).toBe(process.cwd()); + expect(result.worktreePath).toBe(worktreePath); }); // Regression: FN-5475 — when a resumed worktree's branch was created from @@ -111,9 +112,10 @@ describe("acquireTaskWorktree", () => { nonAttributedCount: 0, }); + const worktreePath = process.cwd(); const result = await acquireTaskWorktree({ - task: { ...task, worktree: process.cwd(), branch: "fusion/fn-1" }, - rootDir: process.cwd(), + task: { ...task, worktree: worktreePath, branch: "fusion/fn-1" }, + rootDir: dirname(worktreePath), store, settings: {}, audit, @@ -129,9 +131,10 @@ describe("acquireTaskWorktree", () => { }); it("does not re-anchor a resumed branch when not misbound", async () => { + const worktreePath = process.cwd(); const result = await acquireTaskWorktree({ - task: { ...task, worktree: process.cwd(), branch: "fusion/fn-1" }, - rootDir: process.cwd(), + task: { ...task, worktree: worktreePath, branch: "fusion/fn-1" }, + rootDir: dirname(worktreePath), store, settings: {}, createWorktree: vi.fn(), @@ -328,6 +331,76 @@ describe("acquireTaskWorktree", () => { expect(store.updateTask).toHaveBeenCalledWith("FN-1", { worktree: freshPath, branch: "fusion/fn-1" }); }); + it("FN-6922 rejects a canonical-equal resumed repo root before returning", async () => { + const rootDir = makeRepo(); + const actualPool = await vi.importActual("../worktree-pool.js"); + vi.mocked(classifyTaskWorktree).mockImplementationOnce(actualPool.classifyTaskWorktree); + const freshPath = join(rootDir, ".worktrees", "fn-6922-trailing-slash"); + const createWorktree = vi.fn().mockResolvedValue({ path: freshPath, branch: "fusion/fn-1" }); + + const result = await acquireTaskWorktree({ + task: { ...task, worktree: `${rootDir}/`, branch: "fusion/fn-1", sessionFile: "/tmp/session.json" }, + rootDir, + store, + settings: {} as any, + createWorktree, + }); + + expect(result.worktreePath).toBe(freshPath); + expect(result.worktreePath).not.toBe(rootDir); + expect(result.isResume).toBe(false); + 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("FN-6922 self-heals when the return guard catches a mocked repo-root resume", async () => { + const rootDir = makeRepo(); + vi.mocked(classifyTaskWorktree).mockResolvedValueOnce({ ok: true }); + const freshPath = join(rootDir, ".worktrees", "fn-6922-guard-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, source: "fresh", isResume: false }); + expect(createWorktree).toHaveBeenCalledWith("fusion/fn-1", expect.stringContaining(`${join(rootDir, ".worktrees")}/`), "FN-1", undefined, false); + expect(auditGit).toHaveBeenCalledWith(expect.objectContaining({ + type: "worktree:incomplete-detected", + target: rootDir, + metadata: expect.objectContaining({ classification: "repo-root", source: "acquire-return-guard", returnSource: "existing" }), + })); + 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("FN-6922 throws a typed error when fresh creation returns the repo root", async () => { + const rootDir = makeRepo(); + const auditGit = vi.fn().mockResolvedValue(undefined); + + await expect(acquireTaskWorktree({ + task: { ...task, worktree: null, branch: null }, + rootDir, + store, + settings: {} as any, + createWorktree: vi.fn().mockResolvedValue({ path: rootDir, branch: "fusion/fn-1" }), + audit: { git: auditGit } as any, + })).rejects.toBeInstanceOf(RepoRootWorktreeError); + + expect(auditGit).toHaveBeenCalledWith(expect.objectContaining({ + type: "worktree:incomplete-detected", + target: rootDir, + metadata: expect.objectContaining({ classification: "repo-root", source: "acquire-return-guard", returnSource: "fresh" }), + })); + expect(store.updateTask).toHaveBeenCalledWith("FN-1", { worktree: null, branch: null, sessionFile: null }); + }); + 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/worktree-acquisition.ts b/packages/engine/src/worktree-acquisition.ts index 917d0939ef..042ef90e9a 100644 --- a/packages/engine/src/worktree-acquisition.ts +++ b/packages/engine/src/worktree-acquisition.ts @@ -11,6 +11,7 @@ import { type WorktreePool, classifyTaskWorktree, isInsideWorktreesDir, + isRepoRootPath, removeWorktree, RemovalReason, PoolDoubleLeaseError, @@ -88,6 +89,13 @@ export interface AcquireTaskWorktreeResult { type InitCommandResult = Awaited>>; +export class RepoRootWorktreeError extends Error { + constructor(public readonly taskId: string, public readonly rootDir: string, public readonly worktreePath: string, public readonly source: string) { + super(`Refusing to return repo root as task worktree for ${taskId}: ${worktreePath} (${source}) canonicalizes to ${rootDir}`); + this.name = "RepoRootWorktreeError"; + } +} + const INIT_OUTCOME_MAX_CHARS = 2_000; function configuredCommandErrorMessage(result: { spawnError?: string | Error; timedOut?: boolean; exitCode?: number | null }): string { @@ -239,6 +247,9 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro } } + let acquiredFromPool = false; + let branch = branchName; + const hydrate = async (path: string): Promise => { if (rootDir === path) return false; try { @@ -255,6 +266,155 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro } }; + const createWorktreeImpl = createWorktree + ? createWorktree + : async (createBranch: string, createPath: string, createTaskId: string, startPoint?: string, allowRename?: boolean) => { + try { + const created = await backend.create({ + rootDir, + branch: createBranch, + worktreePath: createPath, + startPoint, + taskId: createTaskId, + allowSiblingBranchRename: allowRename, + }); + if (backend.kind === "worktrunk") { + await audit?.git({ + type: "worktree:worktrunk-create", + target: created.path, + metadata: { branch: created.branch }, + }); + } + return created; + } catch (error) { + if (backend.kind === "worktrunk" && error instanceof WorktrunkOperationError) { + const nativeBackend = new NativeWorktreeBackend({ logger: logger ?? undefined }); + const fallback = () => nativeBackend.create({ + rootDir, + branch: createBranch, + worktreePath: createPath, + startPoint, + taskId: createTaskId, + allowSiblingBranchRename: allowRename, + }); + return await handleWorktrunkFailure("create", error, fallback) as { path: string; branch: string }; + } + throw error; + } + }; + + const emitRepoRootReturnGuardAudit = async (guardedPath: string, source: string) => { + await audit?.git({ + type: "worktree:incomplete-detected", + target: guardedPath, + metadata: { + classification: "repo-root", + reason: "acquireTaskWorktree return path canonicalizes to the project root", + source: "acquire-return-guard", + returnSource: source, + taskId: task.id, + }, + }); + }; + + const finalizeCreatedWorktree = async ( + created: { path: string; branch: string }, + source: "fresh" | "pool", + logOrigin: "normal" | "return-guard", + ): Promise => { + /* + * FNXC:WorktreeLiveness 2026-06-22-18:30: + * FN-6861 fixed the resume classifier path, but FN-6888 showed the repo root can still reach the executor through another acquisition return branch. FN-6922 makes acquisition itself enforce a return-value invariant: no resume, pool, or fresh branch may return the repo root, so the executor's realpath_matches_repo_root gate remains defense-in-depth instead of a requeue loop source. + */ + if (isRepoRootPath(rootDir, created.path)) { + await emitRepoRootReturnGuardAudit(created.path, source); + await store.updateTask(task.id, { worktree: null, branch: null, sessionFile: null }); + throw new RepoRootWorktreeError(task.id, rootDir, created.path, `fresh-create:${logOrigin}`); + } + + worktreePath = created.path; + branch = created.branch; + await store.updateTask(task.id, { worktree: created.path, branch: created.branch }); + await audit?.git({ type: "worktree:create", target: created.path, metadata: { branch: created.branch, source: logOrigin === "return-guard" ? "acquire-return-guard" : undefined } }); + await audit?.git({ type: "branch:create", target: created.branch }); + if (created.branch !== branchName) { + logger?.log(`Branch conflict resolved: using ${created.branch} instead of ${branchName}`); + await store.logEntry(task.id, `Worktree created at ${worktreePath} (branch conflict: using ${created.branch})`, undefined, runContext); + } else if (baseBranch) { + await store.logEntry(task.id, `Worktree created at ${worktreePath} (based on ${baseBranch})`, undefined, runContext); + } else { + await store.logEntry(task.id, `Worktree created at ${worktreePath}`, undefined, runContext); + } + + const cleanup = await removeDesktopBuildArtifacts(worktreePath, logger); + if (cleanup.removed.length > 0) { + await store.logEntry(task.id, `Removed desktop build artifacts from worktree: ${cleanup.removed.join(", ")}`, undefined, runContext); + } + + if (runInitCommand && settings.worktreeInitCommand && runConfiguredCommand) { + const initStartedAt = Date.now(); + let initResult: InitCommandResult | undefined; + try { + initResult = await runConfiguredCommand(settings.worktreeInitCommand, worktreePath, 300_000, taskEnv); + if (initResult.spawnError || initResult.timedOut || initResult.exitCode !== 0) { + throw new Error(configuredCommandErrorMessage(initResult)); + } + await store.logEntry(task.id, `[timing] Worktree init command completed in ${Date.now() - initStartedAt}ms`, settings.worktreeInitCommand, runContext); + } catch (err) { + if (err instanceof Error && err.name === "AbortError") { + throw err; + } + await store.logEntry(task.id, `[timing] Worktree init command failed after ${Date.now() - initStartedAt}ms`, undefined, runContext); + const message = err instanceof Error ? err.message : String(err); + const outcome = formatInitFailureOutcome(initResult, err); + logger?.error?.(`${task.id}: worktree init command failed — first test run will likely fail: ${message} (stderr captured in task log outcome)`); + await store.logEntry(task.id, `Worktree init command failed (first test run will likely fail): ${message}`, outcome, runContext); + } + } + + await maybeWarnForeignTaskStartPoint({ + baseBranch, + rootDir, + worktreePath, + taskId: task.id, + logger, + store, + runContext, + }); + const hydrated = await hydrate(worktreePath); + try { + await writeSecretsEnvFile({ + rootDir, + worktreePath, + taskId: task.id, + settings, + worktreeSource: "fresh", + secretsStore, + audit, + logger, + }); + } catch (err) { + logger?.warn?.(`${task.id}: secrets-env write failed (non-fatal): ${err instanceof Error ? err.message : String(err)}`); + } + return { worktreePath, branch, source, hydrated, isResume: false }; + }; + + const createFreshWorktreeFromReturnGuard = async (guardedPath: string, source: string): Promise => { + await emitRepoRootReturnGuardAudit(guardedPath, source); + logger?.warn(`${task.id}: acquisition ${source} returned repo root; clearing assignment and creating a fresh worktree`); + await store.logEntry(task.id, "Acquisition attempted to return the project root as a task worktree; creating a fresh worktree instead", guardedPath, runContext); + await store.updateTask(task.id, { worktree: null, branch: null, sessionFile: null }); + const fallbackName = generateWorktreeName(rootDir, settings); + const fallbackPath = await resolveTaskWorktreePathForBackend(rootDir, fallbackName, settings, backend, branchName); + const created = await createWorktreeImpl(branchName, fallbackPath, task.id, baseBranch ?? undefined, allowSiblingBranchRename); + return finalizeCreatedWorktree(created, "fresh", "return-guard"); + }; + + const guardAcquisitionReturn = async (result: AcquireTaskWorktreeResult): Promise => { + if (!isRepoRootPath(rootDir, result.worktreePath)) return result; + return createFreshWorktreeFromReturnGuard(result.worktreePath, result.source); + }; + if (task.worktree && isResume) { logger?.log(`Reusing existing worktree: ${worktreePath}`); const cleanup = await removeDesktopBuildArtifacts(worktreePath, logger); @@ -274,12 +434,9 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro runContext, }); // FN-4912: resume path reuses the prior on-disk .env (and its fingerprint sidecar). Rewrite is owned by the next fresh acquisition. - return { worktreePath, branch: resumedBranch, source: "existing", hydrated, isResume: true }; + return guardAcquisitionReturn({ worktreePath, branch: resumedBranch, source: "existing", hydrated, isResume: true }); } - let acquiredFromPool = false; - let branch = branchName; - if (!isResume && pool && settings.recycleWorktrees) { let pooled: string | null = null; try { @@ -379,7 +536,7 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro } catch (err) { logger?.warn?.(`${task.id}: secrets-env write failed (non-fatal): ${err instanceof Error ? err.message : String(err)}`); } - return { + return guardAcquisitionReturn({ worktreePath, branch, source: "pool", @@ -391,7 +548,7 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro strandedCommitCount: prepared.strandedCommitCount, } : undefined, - }; + }); } } catch (poolErr) { pool.release(pooled, task.id); @@ -407,112 +564,11 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro } } - const createWorktreeImpl = createWorktree - ? createWorktree - : async (branch: string, path: string, taskId: string, startPoint?: string, allowRename?: boolean) => { - try { - const created = await backend.create({ - rootDir, - branch, - worktreePath: path, - startPoint, - taskId, - allowSiblingBranchRename: allowRename, - }); - if (backend.kind === "worktrunk") { - await audit?.git({ - type: "worktree:worktrunk-create", - target: created.path, - metadata: { branch: created.branch }, - }); - } - return created; - } catch (error) { - if (backend.kind === "worktrunk" && error instanceof WorktrunkOperationError) { - const nativeBackend = new NativeWorktreeBackend({ logger: logger ?? undefined }); - const fallback = () => nativeBackend.create({ - rootDir, - branch, - worktreePath: path, - startPoint, - taskId, - allowSiblingBranchRename: allowRename, - }); - return await handleWorktrunkFailure("create", error, fallback) as { path: string; branch: string }; - } - throw error; - } - }; - // Worktree removal in merger.ts, worktree-pool.ts, and self-healing.ts is now // backend-mediated via WorktreeBackend.remove(). executor.ts and // step-session-executor.ts remain native-only paths (tracked separately). const created = await createWorktreeImpl(branchName, worktreePath, task.id, baseBranch ?? undefined, allowSiblingBranchRename); - worktreePath = created.path; - branch = created.branch; - await store.updateTask(task.id, { worktree: created.path, branch: created.branch }); - await audit?.git({ type: "worktree:create", target: created.path, metadata: { branch: created.branch } }); - await audit?.git({ type: "branch:create", target: created.branch }); - if (created.branch !== branchName) { - logger?.log(`Branch conflict resolved: using ${created.branch} instead of ${branchName}`); - await store.logEntry(task.id, `Worktree created at ${worktreePath} (branch conflict: using ${created.branch})`, undefined, runContext); - } else if (baseBranch) { - await store.logEntry(task.id, `Worktree created at ${worktreePath} (based on ${baseBranch})`, undefined, runContext); - } else { - await store.logEntry(task.id, `Worktree created at ${worktreePath}`, undefined, runContext); - } - - const cleanup = await removeDesktopBuildArtifacts(worktreePath, logger); - if (cleanup.removed.length > 0) { - await store.logEntry(task.id, `Removed desktop build artifacts from worktree: ${cleanup.removed.join(", ")}`, undefined, runContext); - } - - if (runInitCommand && settings.worktreeInitCommand && runConfiguredCommand) { - const initStartedAt = Date.now(); - let initResult: InitCommandResult | undefined; - try { - initResult = await runConfiguredCommand(settings.worktreeInitCommand, worktreePath, 300_000, taskEnv); - if (initResult.spawnError || initResult.timedOut || initResult.exitCode !== 0) { - throw new Error(configuredCommandErrorMessage(initResult)); - } - await store.logEntry(task.id, `[timing] Worktree init command completed in ${Date.now() - initStartedAt}ms`, settings.worktreeInitCommand, runContext); - } catch (err) { - if (err instanceof Error && err.name === "AbortError") { - throw err; - } - await store.logEntry(task.id, `[timing] Worktree init command failed after ${Date.now() - initStartedAt}ms`, undefined, runContext); - const message = err instanceof Error ? err.message : String(err); - const outcome = formatInitFailureOutcome(initResult, err); - logger?.error?.(`${task.id}: worktree init command failed — first test run will likely fail: ${message} (stderr captured in task log outcome)`); - await store.logEntry(task.id, `Worktree init command failed (first test run will likely fail): ${message}`, outcome, runContext); - } - } - - await maybeWarnForeignTaskStartPoint({ - baseBranch, - rootDir, - worktreePath, - taskId: task.id, - logger, - store, - runContext, - }); - const hydrated = await hydrate(worktreePath); - try { - await writeSecretsEnvFile({ - rootDir, - worktreePath, - taskId: task.id, - settings, - worktreeSource: "fresh", - secretsStore, - audit, - logger, - }); - } catch (err) { - logger?.warn?.(`${task.id}: secrets-env write failed (non-fatal): ${err instanceof Error ? err.message : String(err)}`); - } - return { worktreePath, branch, source: acquiredFromPool ? "pool" : "fresh", hydrated, isResume: false }; + return finalizeCreatedWorktree(created, acquiredFromPool ? "pool" : "fresh", "normal"); } /** diff --git a/packages/engine/src/worktree-pool.ts b/packages/engine/src/worktree-pool.ts index e0f379a5c6..2720af47d7 100644 --- a/packages/engine/src/worktree-pool.ts +++ b/packages/engine/src/worktree-pool.ts @@ -74,6 +74,10 @@ export function canonicalizePath(path: string): string { } } +export function isRepoRootPath(rootDir: string, candidate: string): boolean { + return canonicalizePath(rootDir) === canonicalizePath(candidate); +} + function getExecStdout(result: unknown): string { if (typeof result === "string") return result; if (result && typeof result === "object" && "stdout" in result) { @@ -268,13 +272,11 @@ export async function classifyTaskWorktree(rootDir: string, worktreePath: string return { ok: false, classification: "missing", reason: "worktree directory does not exist" }; } - const canonicalRootDir = canonicalizePath(rootDir); - const canonicalWorktreePath = canonicalizePath(worktreePath); /* * FNXC:WorktreeLiveness 2026-06-21-11:10: * The project root is a legitimately registered git worktree, but it is never a usable task worktree. Tasks must execute inside the configured worktrees directory, so classification rejects root-equal paths here to stop the resume↔executor-gate requeue loop observed in FN-6861/FN-6709. */ - if (canonicalWorktreePath === canonicalRootDir) { + if (isRepoRootPath(rootDir, worktreePath)) { return { ok: false, classification: "repo-root", reason: "worktree path is the project root, not a task worktree" }; }