From 26ea9fd40a0bf5ca82f33dacad2e4cab271701c5 Mon Sep 17 00:00:00 2001 From: Phil Larson Date: Sun, 9 Aug 2026 18:06:15 -0700 Subject: [PATCH] fix: preserve external checkout routing through recovery (#3400) ## Summary - keep persisted operator-routed external checkouts authoritative during executor recovery, remediation, verification, and cleanup - fail closed when a configured external route is invalid instead of falling back to a Fusion-managed worktree - prevent Fusion from cleaning up operator-owned external checkouts - add dashboard and executor regression coverage for the routing handoffs ## Test plan - `pnpm --filter @fusion/engine exec vitest run src/__tests__/verify-worktree-invariants-missing.test.ts` - `pnpm --filter @fusion/engine exec vitest run src/__tests__/external-execution-checkout.test.ts src/__tests__/executor-triage-column-audit.test.ts` - `pnpm --filter @fusion/engine exec vitest run src/__tests__/executor-fast-mode-workflows.test.ts -t 'external execution|authoritative executor route|completed-task recovery captures the live external|pre-merge remediation reuses the live external'` - `FUSION_DASHBOARD_DEEP=1 pnpm --filter @fusion/dashboard exec vitest run src/__tests__/routes-tasks-near-duplicate.test.ts -t 'PATCH external-checkout persists one clean Git checkout for execution and review'` - `pnpm --filter @fusion/engine typecheck` - `pnpm --filter @fusion/dashboard typecheck` - `pnpm test:gate:static` - `pnpm check:changesets --strict` ## Summary by CodeRabbit - **Bug Fixes** - Improved external checkout routing across execution, verification, recovery, retries, and remediation. - Operations now use the latest persisted checkout details, preventing stale routing information from directing work to the wrong location. - Invalid or missing checkout routes fail safely with clear verification errors. - External checkouts are protected from unintended managed worktree or branch cleanup. - **Documentation** - Clarified external checkout routing and validation behavior. --- .../external-checkout-routing-followup.md | 7 + .../routes-tasks-near-duplicate.test.ts | 6 + .../routes/register-task-workflow-routes.ts | 7 +- .../executor-fast-mode-workflows.test.ts | 145 +++++++++++++- .../executor-triage-column-audit.test.ts | 28 ++- .../external-execution-checkout.test.ts | 7 +- ...verify-worktree-invariants-missing.test.ts | 38 ++++ .../execution/external-execution-checkout.ts | 4 + packages/engine/src/executor.ts | 189 +++++++++++++----- 9 files changed, 371 insertions(+), 60 deletions(-) create mode 100644 .changeset/external-checkout-routing-followup.md diff --git a/.changeset/external-checkout-routing-followup.md b/.changeset/external-checkout-routing-followup.md new file mode 100644 index 0000000000..45c2840c35 --- /dev/null +++ b/.changeset/external-checkout-routing-followup.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Keep operator-routed external checkouts authoritative across recovery, remediation, verification, and cleanup. +category: fix +dev: Re-reads persisted checkout metadata and prevents managed-worktree fallback or cleanup on external routes. diff --git a/packages/dashboard/src/__tests__/routes-tasks-near-duplicate.test.ts b/packages/dashboard/src/__tests__/routes-tasks-near-duplicate.test.ts index c32a08e719..0f5d33ba8e 100644 --- a/packages/dashboard/src/__tests__/routes-tasks-near-duplicate.test.ts +++ b/packages/dashboard/src/__tests__/routes-tasks-near-duplicate.test.ts @@ -365,11 +365,17 @@ describe("routes /api/tasks near duplicate", () => { ); expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(inspection).toHaveBeenCalledWith("/tmp/external-runtime", { + requireClean: true, + }); expect((res.body as Task).sourceMetadata).toMatchObject({ externalExecutionCheckout: "/tmp/external-runtime", externalExecutionBranch: "local/runtime-fixes", externalReviewCheckout: "/tmp/external-runtime", }); + expect((res.body as Task).sourceMetadata?.externalExecutionCheckout).toBe("/tmp/external-runtime"); + expect((res.body as Task).sourceMetadata?.externalExecutionBranch).toBe("local/runtime-fixes"); + expect((res.body as Task).sourceMetadata?.externalReviewCheckout).toBe("/tmp/external-runtime"); expect(tasks[0]?.sourceMetadata).toMatchObject((res.body as Task).sourceMetadata ?? {}); inspection.mockResolvedValueOnce({ valid: false, reason: "checkoutPath must be clean before routing" }); diff --git a/packages/dashboard/src/routes/register-task-workflow-routes.ts b/packages/dashboard/src/routes/register-task-workflow-routes.ts index c51635655b..15ec47deed 100644 --- a/packages/dashboard/src/routes/register-task-workflow-routes.ts +++ b/packages/dashboard/src/routes/register-task-workflow-routes.ts @@ -6412,9 +6412,10 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork } }); - // Persist one operator-validated external checkout for both implementation - // and enforced review. Keep filesystem routing out of the user-defined - // workflow custom-field schema. + /** + * FNXC:ExternalTaskCheckoutRouting 2026-08-09-22:43: + * Persist one operator-validated external checkout for both implementation and enforced review. The execution/review route belongs in task source metadata, not the user-defined workflow custom-field schema, and clearing the route must null every persisted routing key. + */ router.patch("/tasks/:id/external-checkout", async (req, res) => { try { const { store: scopedStore } = await getProjectContext(req); diff --git a/packages/engine/src/__tests__/executor-fast-mode-workflows.test.ts b/packages/engine/src/__tests__/executor-fast-mode-workflows.test.ts index e57efbabe3..da3f5e1ef8 100644 --- a/packages/engine/src/__tests__/executor-fast-mode-workflows.test.ts +++ b/packages/engine/src/__tests__/executor-fast-mode-workflows.test.ts @@ -257,6 +257,10 @@ describe("fast mode workflow/runtime invariants", () => { }); }); + /* + FNXC:ExternalExecutionCheckout 2026-08-09-23:53: + A valid persisted external execution checkout takes precedence over worktree and branch values in the caller snapshot; the executor must resolve the live task row before routing. + */ it("prepares a persisted external execution checkout instead of the project task worktree", async () => { const routedTask = task({ id: "FN-6097", @@ -278,13 +282,20 @@ describe("fast mode workflow/runtime invariants", () => { }); const executor = new TaskExecutor(store, "/tmp/project-root"); + const runnerSnapshot = { + ...routedTask, + worktree: "/tmp/stale-project-task-worktree", + branch: "fusion/stale-fn-6097", + sourceMetadata: undefined, + }; const result = await (executor as any) .createAuthoritativeWorkflowPrimitives({ experimentalFeatures: { workflowGraphExecutor: true } }) .prepareWorktree( { run: { taskId: "FN-6097" }, node: { node: { id: "execute" }, context: {} } }, - routedTask, + runnerSnapshot, ); + expect(mockedResolveExternalExecutionCheckoutRoute).toHaveBeenCalledWith(routedTask); expect(result).toMatchObject({ outcome: "success", data: { @@ -294,6 +305,63 @@ describe("fast mode workflow/runtime invariants", () => { }); }); + it("fails closed when the persisted external execution route is invalid", async () => { + const routedTask = task({ + id: "FN-6098", + sourceMetadata: { + externalExecutionCheckout: "/tmp/external-runtime", + externalExecutionBranch: "local/runtime-fixes", + }, + }); + const store = createMockStore(); + store.getTask.mockResolvedValue(routedTask); + mockedResolveExternalExecutionCheckoutRoute.mockResolvedValueOnce({ + configured: true, + valid: false, + reason: "external execution checkout branch mismatch", + }); + const executor = new TaskExecutor(store, "/tmp/project-root"); + + const result = await (executor as any) + .createAuthoritativeWorkflowPrimitives({ experimentalFeatures: { workflowGraphExecutor: true } }) + .prepareWorktree( + { run: { taskId: "FN-6098" }, node: { node: { id: "execute" }, context: {} } }, + task({ id: "FN-6098", worktree: "/tmp/project-task-worktree" }), + ); + + expect(result).toEqual({ + outcome: "failure", + value: "external-execution-checkout-invalid: external execution checkout branch mismatch", + }); + expect(mockedExistsSync).not.toHaveBeenCalledWith("/tmp/project-task-worktree"); + }); + + it("the authoritative executor route resolver re-reads persisted metadata instead of trusting a stale snapshot", async () => { + const routedTask = task({ + id: "FN-6099", + sourceMetadata: { + externalExecutionCheckout: "/tmp/external-runtime", + externalExecutionBranch: "local/runtime-fixes", + }, + }); + const store = createMockStore(); + store.getTask.mockResolvedValue(routedTask); + mockedResolveExternalExecutionCheckoutRoute.mockResolvedValueOnce({ + configured: true, + valid: false, + reason: "external execution checkout branch mismatch", + }); + const executor = new TaskExecutor(store, "/tmp/project-root"); + + const result = await (executor as any).resolveAuthoritativeExternalExecutionRoute( + task({ id: "FN-6099", sourceMetadata: undefined }), + ); + + expect(result.task).toEqual(routedTask); + expect(mockedResolveExternalExecutionCheckoutRoute).toHaveBeenCalledWith(routedTask); + expect(result.route).toMatchObject({ configured: true, valid: false }); + }); + it("does not project a fresh graph step or capture its baseline before the executor creates its worktree", async () => { let liveTask = task({ steps: [{ name: "Preflight", status: "pending" }], @@ -984,6 +1052,81 @@ describe("fast mode workflow/runtime invariants", () => { expect(graph).toHaveBeenCalledWith(liveTask); }); + it("completed-task recovery captures the live external checkout instead of a stale task worktree", async () => { + const liveTask = task({ + id: "FN-7283-EXTERNAL-RECOVERY", + executionMode: "fast", + enabledWorkflowSteps: [], + worktree: "/tmp/stale-managed-worktree", + baseCommitSha: "base", + steps: [{ name: "Do it", status: "done" }], + workflowStepResults: [], + sourceMetadata: { + externalExecutionCheckout: "/tmp/external-runtime", + externalExecutionBranch: "local/runtime-fixes", + }, + }); + const staleSnapshot = { ...liveTask, sourceMetadata: undefined }; + const { executor } = makeExecutorForTask(liveTask); + mockedResolveExternalExecutionCheckoutRoute.mockResolvedValue({ + configured: true, + valid: true, + checkoutPath: "/tmp/external-runtime", + branch: "local/runtime-fixes", + }); + const captureModifiedFiles = vi.spyOn(executor as any, "captureModifiedFiles").mockResolvedValue([]); + + const recovered = await executor.recoverCompletedTask(staleSnapshot as any); + + expect(recovered).toBe(true); + expect(captureModifiedFiles).toHaveBeenCalledWith( + "/tmp/external-runtime", + "base", + "FN-7283-EXTERNAL-RECOVERY", + undefined, + "recovery", + ); + }); + + it("pre-merge remediation reuses the live external checkout without persisting it as task.worktree", async () => { + const liveTask = task({ + id: "FN-7283-EXTERNAL-REMEDIATION", + worktree: "/tmp/stale-managed-worktree", + steps: [{ name: "Do it", status: "done" }], + sourceMetadata: { + externalExecutionCheckout: "/tmp/external-runtime", + externalExecutionBranch: "local/runtime-fixes", + }, + }); + const staleSnapshot = { ...liveTask, sourceMetadata: undefined }; + const { executor } = makeExecutorForTask(liveTask); + mockedResolveExternalExecutionCheckoutRoute.mockResolvedValue({ + configured: true, + valid: true, + checkoutPath: "/tmp/external-runtime", + branch: "local/runtime-fixes", + }); + vi.spyOn(executor as any, "injectWorkflowStepFailureInstructions").mockResolvedValue(undefined); + vi.spyOn(executor as any, "reopenLastStepForRevision").mockResolvedValue(null); + const scheduleWorkflowRerun = vi.spyOn(executor as any, "scheduleWorkflowRerun").mockImplementation(() => undefined); + + await (executor as any).sendTaskBackForFix( + staleSnapshot, + "/tmp/stale-managed-worktree", + "fix it", + "Code Review", + "Review requested changes", + ); + + expect(scheduleWorkflowRerun).toHaveBeenCalledWith( + "FN-7283-EXTERNAL-REMEDIATION", + "/tmp/external-runtime", + expect.any(String), + true, + false, + ); + }); + /* FNXC:EngineTests 2026-07-19-18:20 (U10b): The requirement under test is a store that CANNOT resolve a workflow selection (minimal/older embedded diff --git a/packages/engine/src/__tests__/executor-triage-column-audit.test.ts b/packages/engine/src/__tests__/executor-triage-column-audit.test.ts index bc11c1acb4..33ae87db27 100644 --- a/packages/engine/src/__tests__/executor-triage-column-audit.test.ts +++ b/packages/engine/src/__tests__/executor-triage-column-audit.test.ts @@ -13,7 +13,7 @@ import { describe, expect, it, vi } from "vitest"; import type { Task, TaskDetail, WorkflowIr } from "@fusion/core"; import "./executor-test-helpers.js"; import { TaskExecutor } from "../executor.js"; -import { createMockStore, resetExecutorMocks } from "./executor-test-helpers.js"; +import { createMockStore, mockedExec, resetExecutorMocks } from "./executor-test-helpers.js"; import { UsageLimitPauser } from "../errors/usage-limit-detector.js"; const WF = "custom:planning-only"; @@ -39,7 +39,7 @@ describe("dependency-abort cleanup requeues to a DECLARED column", () => { resetExecutorMocks(); const store = createMockStore(); const selection = { workflowId: WF, stepIds: [] }; - store.getTask.mockResolvedValue({ id: "FN-DEP", column: "in-progress", branch: null } as TaskDetail); + store.getTask.mockResolvedValue({ id: "FN-DEP", column: "in-progress", branch: null } as unknown as TaskDetail); store.getTaskWorkflowSelection = vi.fn(() => selection); store.getTaskWorkflowSelectionAsync = vi.fn(async () => selection); store.getWorkflowDefinition = vi.fn(async () => ({ id: WF, ir: planningOnlyIr() })); @@ -54,6 +54,30 @@ describe("dependency-abort cleanup requeues to a DECLARED column", () => { expect(store.moveTask).toHaveBeenCalledWith("FN-DEP", "todo"); expect(store.moveTask).not.toHaveBeenCalledWith("FN-DEP", "triage"); }); + + it("does not remove or delete an operator-owned external execution checkout", async () => { + resetExecutorMocks(); + const store = createMockStore(); + store.getTask.mockResolvedValue({ + id: "FN-EXT", + column: "in-progress", + branch: "fusion/fn-ext", + sourceMetadata: { + externalExecutionCheckout: "/tmp/operator-owned-checkout", + externalExecutionBranch: "operator/runtime-fixes", + }, + } as unknown as TaskDetail); + const executor = new TaskExecutor(store, "/tmp/test"); + const removeManagedWorktree = vi.spyOn(executor as any, "removeOwnWorktreeWithReconcile"); + + await (executor as any).handleDepAbortCleanup("FN-EXT", "/tmp/operator-owned-checkout"); + + expect(removeManagedWorktree).not.toHaveBeenCalled(); + expect(mockedExec).not.toHaveBeenCalledWith( + expect.stringContaining("git branch -D"), + expect.anything(), + ); + }); }); describe("usage-limit fan-out still recognises the planning lane", () => { diff --git a/packages/engine/src/__tests__/external-execution-checkout.test.ts b/packages/engine/src/__tests__/external-execution-checkout.test.ts index 93c67d4290..9a581e6da9 100644 --- a/packages/engine/src/__tests__/external-execution-checkout.test.ts +++ b/packages/engine/src/__tests__/external-execution-checkout.test.ts @@ -1,8 +1,7 @@ /* - * Persisted external checkout routing is an explicit operator contract. Execution - * must use the same validated checkout as review, and stale path/branch metadata - * must fail closed instead of silently falling back to the project task worktree. - */ +FNXC:ExternalExecutionCheckout 2026-08-09-23:53: +Persisted external checkout routing is an explicit operator contract. Execution must use the same validated checkout as review, and stale path or branch metadata must fail closed instead of silently falling back to the project task worktree. +*/ import { afterAll, beforeAll, describe, expect, it } from "vitest"; import { execFileSync } from "node:child_process"; import { mkdtempSync, realpathSync, rmSync, writeFileSync } from "node:fs"; diff --git a/packages/engine/src/__tests__/verify-worktree-invariants-missing.test.ts b/packages/engine/src/__tests__/verify-worktree-invariants-missing.test.ts index 4d985d08a0..f8a2827ab1 100644 --- a/packages/engine/src/__tests__/verify-worktree-invariants-missing.test.ts +++ b/packages/engine/src/__tests__/verify-worktree-invariants-missing.test.ts @@ -28,6 +28,8 @@ describe("FN-009: verifyWorktreeInvariants with missing worktree directory", () updatedAt: new Date().toISOString(), }; + store.getTask.mockResolvedValue(task as any); + // Mock existsSync to return false for the worktree path mockedExistsSync.mockImplementation((path: any) => { if (path === "/repo/.worktrees/missing") { @@ -57,6 +59,7 @@ describe("FN-009: verifyWorktreeInvariants with missing worktree directory", () updatedAt: new Date().toISOString(), }; + store.getTask.mockResolvedValue(task as any); // Mock existsSync to return true for the worktree path mockedExistsSync.mockReturnValue(true); @@ -85,6 +88,7 @@ describe("FN-009: verifyWorktreeInvariants with missing worktree directory", () updatedAt: new Date().toISOString(), }; + store.getTask.mockResolvedValue(task as any); mockedExistsSync.mockReturnValue(true); mockedExecSync.mockImplementation((cmd: string) => { if (cmd.includes("rev-parse --show-toplevel")) return Buffer.from("/repo/.worktrees/gentle-flame\n"); @@ -117,6 +121,7 @@ describe("FN-009: verifyWorktreeInvariants with missing worktree directory", () updatedAt: new Date().toISOString(), }; + store.getTask.mockResolvedValue(task as any); mockedExistsSync.mockReturnValue(true); mockedExecSync.mockImplementation((cmd: string) => { if (cmd.includes("rev-parse --show-toplevel")) return Buffer.from("/repo\n"); @@ -130,6 +135,38 @@ describe("FN-009: verifyWorktreeInvariants with missing worktree directory", () expect(store.updateTask).not.toHaveBeenCalledWith("FN-9005", expect.objectContaining({ worktree: expect.any(String) })); }); + it("fails closed from the live external route when the verification snapshot is stale", async () => { + const staleTask = { + id: "FN-9006", + title: "Test", + description: "Test", + column: "in-progress", + worktree: "/repo/.worktrees/stale", + branch: "fusion/fn-9006", + dependencies: [], + steps: [], + currentStep: 0, + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + }; + store.getTask.mockResolvedValue({ + ...staleTask, + sourceMetadata: { + externalExecutionCheckout: "/tmp/missing-operator-checkout", + externalExecutionBranch: "operator/runtime-fixes", + }, + }); + + const result = await (executor as any).verifyWorktreeInvariants(staleTask); + + expect(result).toMatchObject({ + ok: false, + reason: "wrong_toplevel", + observed: expect.stringContaining("checkoutPath"), + expected: "valid persisted external execution checkout", + }); + }); + it("preserves validation failure when worktree path is null", async () => { const task = { id: "FN-9003", @@ -145,6 +182,7 @@ describe("FN-009: verifyWorktreeInvariants with missing worktree directory", () updatedAt: new Date().toISOString(), }; + store.getTask.mockResolvedValue(task as any); const result = await (executor as any).verifyWorktreeInvariants(task); expect(result.ok).toBe(false); diff --git a/packages/engine/src/execution/external-execution-checkout.ts b/packages/engine/src/execution/external-execution-checkout.ts index fdb58dff39..a6c178c6a0 100644 --- a/packages/engine/src/execution/external-execution-checkout.ts +++ b/packages/engine/src/execution/external-execution-checkout.ts @@ -83,6 +83,10 @@ export async function inspectExternalGitCheckout( } } +/** + * FNXC:ExternalTaskCheckoutRouting 2026-08-09-22:43: + * Resolve only a persisted, absolute Git top-level whose checked-out branch still matches the branch captured when the operator configured the route. A missing or null route preserves normal Fusion-managed worktree behavior; malformed or drifted persisted routes fail closed. + */ export async function resolveExternalExecutionCheckoutRoute(task: unknown): Promise { const sourceMetadata = readSourceMetadata(task); if (!sourceMetadata || !Object.prototype.hasOwnProperty.call(sourceMetadata, "externalExecutionCheckout")) { diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index f1a0efd965..0305c39b76 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -36,7 +36,10 @@ import { WorkflowAgentCapacity } from "./agents/workflow-agent-capacity.js"; import { createExecutorColumnBoundaryHooks } from "./workflow-column-boundary-hooks.js"; import { ensureWorkflowCompletionSummary } from "./workflows/workflow-completion-summary.js"; import { createCodeNodeRunner } from "./execution/code-node-runner.js"; -import { resolveExternalExecutionCheckoutRoute } from "./execution/external-execution-checkout.js"; +import { + resolveExternalExecutionCheckoutRoute, + type ExternalExecutionCheckoutResolution, +} from "./execution/external-execution-checkout.js"; import { getTaskReviewCheckoutPath, resolveReviewCheckoutCwd } from "./execution/review-checkout.js"; import { getActiveNotificationService } from "./util/notifier.js"; import type { ParseStepsHandlerDeps, CodeNodeRunner } from "./workflows/workflow-node-handlers.js"; @@ -3841,6 +3844,8 @@ export class TaskExecutor { }); /* FNXC:WorkflowLifecycle 2026-07-16-10:00: Executor replaces the baseline only for its own TaskStore, so archive awaits abort/sweep/removal before branch deletion without cross-store coupling. */ this.unregisterArchiveWorktreeDisposer = registerArchiveWorktreeDisposer(store, async (task) => { + const externalExecutionRoute = await resolveExternalExecutionCheckoutRoute(task); + if (externalExecutionRoute.configured) return; if (!task.worktree || await canonicalizeWorktreePath(task.worktree) === await canonicalizeWorktreePath(this.rootDir)) return; await this.awaitAbortInFlightTaskWork(task.id, "task archived"); for (const path of activeSessionRegistry.pathsForTask(task.id)) activeSessionRegistry.unregisterPath(path); @@ -4842,6 +4847,7 @@ export class TaskExecutor { taskId: string, worktreePath: string, preserveResumeState: boolean = true, + persistWorktreePath: boolean = true, ): Promise<"bounced" | "skipped-pending" | "deferred-paused"> { const pauseLabel = await this.getExecutionPauseLabel(); if (pauseLabel) { @@ -4915,7 +4921,7 @@ export class TaskExecutor { // preserveResumeState is false. Keep the writes so callers and // tests can observe the restoration deterministically. await this.store.updateTask(taskId, { - worktree: worktreePath, + ...(persistWorktreePath ? { worktree: worktreePath } : {}), executionStartedAt: originalExecutionStartedAt ?? null, }); const pauseLabelAfterTodo = await this.getExecutionPauseLabel(); @@ -4931,7 +4937,7 @@ export class TaskExecutor { } if (latestTask.column === await resolveReboundColumnFor(this.store, taskId)) { - await this.store.updateTask(taskId, { worktree: worktreePath }); + if (persistWorktreePath) await this.store.updateTask(taskId, { worktree: worktreePath }); const pauseLabelBeforeResume = await this.getExecutionPauseLabel(); if (pauseLabelBeforeResume) { executorLog.log(`${taskId}: workflow rerun parked in todo — ${pauseLabelBeforeResume} became active before resume`); @@ -4955,12 +4961,18 @@ export class TaskExecutor { worktreePath: string, successMessage: string, preserveResumeState: boolean = true, + persistWorktreePath: boolean = true, ): void { this.clearWorkflowRerunWatchdog(taskId); setTimeout(async () => { try { - const outcome = await this.performWorkflowRerunBounce(taskId, worktreePath, preserveResumeState); + const outcome = await this.performWorkflowRerunBounce( + taskId, + worktreePath, + preserveResumeState, + persistWorktreePath, + ); if (outcome === "bounced") { executorLog.log(successMessage); } else if (outcome === "skipped-pending") { @@ -5012,7 +5024,12 @@ export class TaskExecutor { ).catch(() => undefined); try { - const outcome = await this.performWorkflowRerunBounce(taskId, worktreePath, preserveResumeState); + const outcome = await this.performWorkflowRerunBounce( + taskId, + worktreePath, + preserveResumeState, + persistWorktreePath, + ); if (outcome === "bounced") { executorLog.warn(`${taskId}: workflow rerun watchdog retry succeeded`); } else if (outcome === "skipped-pending") { @@ -5574,9 +5591,19 @@ export class TaskExecutor { return false; } - // Capture modified files if the worktree still exists - if (task.worktree && existsSync(task.worktree)) { - const modifiedFiles = await this.captureModifiedFiles(task.worktree, task.baseCommitSha, task.id, undefined, "recovery"); + const { task: authoritativeRecoveryTask, route: externalExecutionRoute } = + await this.resolveAuthoritativeExternalExecutionRoute(task); + if (externalExecutionRoute.configured && !externalExecutionRoute.valid) { + executorLog.warn(`${task.id}: completed-task recovery refused invalid external execution checkout: ${externalExecutionRoute.reason ?? "unknown error"}`); + return false; + } + const recoveryWorktreePath = externalExecutionRoute.configured + ? externalExecutionRoute.checkoutPath + : authoritativeRecoveryTask.worktree; + + // Capture modified files if the authoritative execution checkout still exists. + if (recoveryWorktreePath && existsSync(recoveryWorktreePath)) { + const modifiedFiles = await this.captureModifiedFiles(recoveryWorktreePath, authoritativeRecoveryTask.baseCommitSha, task.id, undefined, "recovery"); if (modifiedFiles.length > 0) { await this.store.updateTask(task.id, { modifiedFiles }); executorLog.log(`${task.id}: recovered ${modifiedFiles.length} modified files`); @@ -10542,6 +10569,8 @@ export class TaskExecutor { try { const live = await this.store.getTask(taskId); if (!live?.worktree) return false; + const externalExecutionRoute = await resolveExternalExecutionCheckoutRoute(live); + if (externalExecutionRoute.configured) return false; if (live.firstExecutionAt || live.executionStartedAt) return false; if (activeSessionRegistry.isPathActive(live.worktree) || activeSessionRegistry.isPathActive(resolvePath(live.worktree))) return false; if (this.hasLiveTaskSessionSurface(taskId) || executingTaskLock.has(taskId)) return false; @@ -14134,11 +14163,19 @@ export class TaskExecutor { // reviewHandoffPolicy, …) pick up workflow values with zero read-site changes. // Behavior-inert when nothing is customized (declaration defaults === legacy // defaults; absent-default lanes never override). - const settings = await mergeEffectiveSettings(this.store, task, await this.store.getSettings()); - const externalExecutionRoute = await resolveExternalExecutionCheckoutRoute(task); + /* + FNXC:ExternalExecutionCheckout 2026-08-09-23:53: + Execution must re-read persisted routing state and fail closed before worktree acquisition when an operator-owned checkout has drifted or become invalid. + */ + const { task: authoritativeExecutionTask, route: externalExecutionRoute } = + await this.resolveAuthoritativeExternalExecutionRoute(task); + const settings = await mergeEffectiveSettings(this.store, authoritativeExecutionTask, await this.store.getSettings()); if (externalExecutionRoute.configured && !externalExecutionRoute.valid) { const message = `Persisted external execution checkout is invalid: ${externalExecutionRoute.reason ?? "unknown error"}`; await this.store.logEntry(task.id, message, undefined, this.getRunContextFor(task.id)); + this.executing.delete(task.id); + executingTaskLock.release(task.id); + if (dropPreHeldExecutorSlot(task.id)) this.options.semaphore?.release(); throw new Error(message); } @@ -15252,7 +15289,7 @@ export class TaskExecutor { executorLog.warn(`⚡ ${task.id} transient error — retry ${attempt}/${MAX_RECOVERY_RETRIES} in ${delay}: ${errorMessage}`); await this.store.logEntry(task.id, `Transient error (retry ${attempt}/${MAX_RECOVERY_RETRIES} in ${delay}): ${errorMessage}`, undefined, this.getRunContextFor(task.id)); } - if (worktreePath && existsSync(worktreePath)) { + if (!externalExecutionRoute.configured && worktreePath && existsSync(worktreePath)) { try { const settings = await this.store.getSettings(); await removeWorktree({ @@ -15352,9 +15389,11 @@ export class TaskExecutor { FNXC:StuckRequeue 2026-06-27-23:15: Stuck requeue may destroy a checkout that contains only uncommitted step output. Always reconcile lost-work step state before worktree removal, even when preserve-progress is enabled, so a retry cannot skip code that no longer exists. */ - await this.resetStepsIfWorkLost(latestTask); + if (!externalExecutionRoute.configured) { + await this.resetStepsIfWorkLost(latestTask); + } - if (worktreePath && existsSync(worktreePath)) { + if (!externalExecutionRoute.configured && worktreePath && existsSync(worktreePath)) { try { await removeWorktree({ worktreePath, @@ -16788,7 +16827,7 @@ export class TaskExecutor { return; } else { executorLog.log(`${task.id} paused — moving to todo`); - if (worktreePath && existsSync(worktreePath)) { + if (!externalExecutionRoute.configured && worktreePath && existsSync(worktreePath)) { try { const settings = await this.store.getSettings(); await removeWorktree({ @@ -17311,8 +17350,8 @@ export class TaskExecutor { executorLog.warn(`⚡ ${task.id} transient error — retry ${attempt}/${MAX_RECOVERY_RETRIES} in ${delay}: ${errorMessage}`); await this.store.logEntry(task.id, `Transient error (retry ${attempt}/${MAX_RECOVERY_RETRIES} in ${delay}): ${errorMessage}`, undefined, this.getRunContextFor(task.id)); } - // Clean up the old worktree so the retry gets a fresh one - if (worktreePath && existsSync(worktreePath)) { + // Clean up only Fusion-managed worktrees so retries never remove an operator-owned external checkout. + if (!externalExecutionRoute.configured && worktreePath && existsSync(worktreePath)) { try { const settings = await this.store.getSettings(); await removeWorktree({ @@ -17504,10 +17543,12 @@ export class TaskExecutor { FNXC:StuckRequeue 2026-06-27-23:15: Preserve-progress stuck requeues still remove the old checkout. Reconcile steps first so uncommitted-only output is reset to pending while committed progress can remain complete. */ - await this.resetStepsIfWorkLost(latestTask); + if (!externalExecutionRoute.configured) { + await this.resetStepsIfWorkLost(latestTask); + } - // Clean up the old worktree so the retry gets a fresh one - if (worktreePath && existsSync(worktreePath)) { + // Clean up only Fusion-managed worktrees so retries never remove an operator-owned external checkout. + if (!externalExecutionRoute.configured && worktreePath && existsSync(worktreePath)) { try { await removeWorktree({ worktreePath, @@ -18224,7 +18265,12 @@ export class TaskExecutor { } return { ok: true }; } - const externalExecutionRoute = await resolveExternalExecutionCheckoutRoute(task); + /* + FNXC:ExternalExecutionCheckout 2026-08-09-23:53: + Completion verification must use the live external route and reject invalid persisted metadata rather than falling back to a stale Fusion-managed worktree snapshot. + */ + const { task: authoritativeVerificationTask, route: externalExecutionRoute } = + await this.resolveAuthoritativeExternalExecutionRoute(task); if (externalExecutionRoute.configured && !externalExecutionRoute.valid) { return { ok: false, @@ -18235,13 +18281,14 @@ export class TaskExecutor { } const branchName = externalExecutionRoute.configured ? externalExecutionRoute.branch ?? "" - : resolveTaskWorkingBranch(task); + : resolveTaskWorkingBranch(authoritativeVerificationTask); // Non-workspace tasks hold a one-element set; fall back to its sole member to preserve the original singular resolution. - const worktreePath = worktreePathOverride - ?? (externalExecutionRoute.configured ? externalExecutionRoute.checkoutPath : undefined) - ?? task.worktree - ?? this.getActiveWorktreePaths(task.id)[0] - ?? null; + const worktreePath = externalExecutionRoute.configured + ? externalExecutionRoute.checkoutPath ?? null + : worktreePathOverride + ?? authoritativeVerificationTask.worktree + ?? this.getActiveWorktreePaths(task.id)[0] + ?? null; if (!worktreePath) { return { @@ -18289,6 +18336,10 @@ export class TaskExecutor { if (observedTopLevelRaw) { const observedTopLevel = canonicalizePath(observedTopLevelRaw); + /* + FNXC:ExternalExecutionCheckout 2026-08-09-23:53: + An operator-routed checkout must match its validated Git top-level exactly. Nested-worktree re-anchoring is reserved for Fusion-managed worktrees and must not widen this ownership boundary. + */ const violatesCheckoutBoundary = externalExecutionRoute.configured ? observedTopLevel !== expectedWorktreeRealpath : observedTopLevel === expectedRoot @@ -19151,30 +19202,39 @@ export class TaskExecutor { private async handleDepAbortCleanup(taskId: string, worktreePath: string): Promise { executorLog.log(`${taskId} dependency added — work discarded, moved to triage for re-planning`); - // Remove worktree - try { - const settings = await this.store.getSettings(); - await this.removeOwnWorktreeWithReconcile({ - worktreePath, - settings, - taskId, - reason: RemovalReason.ExecutorDispose, - }); - } catch (err: unknown) { - const msg = err instanceof Error ? err.message : String(err); - executorLog.warn(`${taskId}: failed to remove worktree during dep-abort cleanup (${worktreePath}): ${msg}`); + const task = await this.store.getTask(taskId); + const externalExecutionRoute = await resolveExternalExecutionCheckoutRoute(task); + + /* + FNXC:ExternalExecutionCheckout 2026-08-09-22:43: + Persisted external execution routes are operator-owned checkouts. Executor cleanup may clear Fusion's managed task pointers, but it must never remove the routed directory or delete its branch during dependency abort, retry, pause, stuck-kill, or remediation recovery. + */ + if (!externalExecutionRoute.configured) { + try { + const settings = await this.store.getSettings(); + await this.removeOwnWorktreeWithReconcile({ + worktreePath, + settings, + taskId, + reason: RemovalReason.ExecutorDispose, + }); + } catch (err: unknown) { + const msg = err instanceof Error ? err.message : String(err); + executorLog.warn(`${taskId}: failed to remove worktree during dep-abort cleanup (${worktreePath}): ${msg}`); + } } - // Delete the branch — use stored branch name if available, fall back to convention - const task = await this.store.getTask(taskId); + // Delete only a Fusion-managed branch. External routes remain operator-owned. const branch = resolveTaskWorkingBranch(task); let branchDeleted = false; - try { - await execAsync(`git branch -D "${branch}"`, { cwd: this.rootDir }); - branchDeleted = true; - } catch (err: unknown) { - const msg = err instanceof Error ? err.message : String(err); - executorLog.warn(`${taskId}: failed to delete branch during dep-abort cleanup (${branch}): ${msg}`); + if (!externalExecutionRoute.configured) { + try { + await execAsync(`git branch -D "${branch}"`, { cwd: this.rootDir }); + branchDeleted = true; + } catch (err: unknown) { + const msg = err instanceof Error ? err.message : String(err); + executorLog.warn(`${taskId}: failed to delete branch during dep-abort cleanup (${branch}): ${msg}`); + } } if (branchDeleted) { // FN-2165 regression guard: null baseBranch on any task that stored this branch @@ -19552,6 +19612,14 @@ ${failureContext.output.slice(0, VERIFICATION_LOG_MAX_CHARS)} ): Promise { const taskId = task.id; this.clearCompletedTaskWatchdog(taskId); + const { task: authoritativeRemediationTask, route: externalExecutionRoute } = + await this.resolveAuthoritativeExternalExecutionRoute(task); + if (externalExecutionRoute.configured && !externalExecutionRoute.valid) { + throw new Error(`Persisted external execution checkout is invalid: ${externalExecutionRoute.reason ?? "unknown error"}`); + } + const remediationWorktreePath = externalExecutionRoute.configured + ? externalExecutionRoute.checkoutPath ?? "" + : worktreePath; // 1. Add a task comment explaining the failure await this.store.addTaskComment( @@ -19577,7 +19645,7 @@ ${failureContext.output.slice(0, VERIFICATION_LOG_MAX_CHARS)} * this display, remains the safety boundary for unchanged remediation loops. */ await this.injectWorkflowStepFailureInstructions( - task, + authoritativeRemediationTask, failureFeedback, stepName, retryPresentation ?? { attempt: MAX_WORKFLOW_STEP_RETRIES, max: MAX_WORKFLOW_STEP_RETRIES }, @@ -19606,9 +19674,10 @@ ${failureContext.output.slice(0, VERIFICATION_LOG_MAX_CHARS)} // 6. Schedule the move after the guard unwinds (per guard-unwind requirement) this.scheduleWorkflowRerun( taskId, - worktreePath, + remediationWorktreePath, `${taskId}: sent back to in-progress for remediation`, preserveResumeState, + !externalExecutionRoute.configured, ); } @@ -23267,7 +23336,10 @@ You have access to the file system to review changes.${inlineFixBlock}${verdictB const settings = await this.store.getSettings(); const preserveProgress = settings.preserveProgressOnStuckRequeue !== false; const latestTask = await this.store.getTask(taskId); - const worktreePath = this.getWorktreePath(taskId) ?? latestTask.worktree; + const externalExecutionRoute = await resolveExternalExecutionCheckoutRoute(latestTask); + const worktreePath = externalExecutionRoute.configured + ? undefined + : this.getWorktreePath(taskId) ?? latestTask.worktree; /* FNXC:Workspace 2026-06-21-22:30: F8 — observability for the workspace case. A workspace task has no singular @@ -23304,7 +23376,9 @@ You have access to the file system to review changes.${inlineFixBlock}${verdictB FNXC:StuckRequeue 2026-06-27-23:15: The force path mirrors normal stuck-requeue cleanup: before reaping a hung executor's worktree, reconcile step progress against committed branch state so preserved progress never points at deleted uncommitted work. */ - await this.resetStepsIfWorkLost(latestTask); + if (!externalExecutionRoute.configured) { + await this.resetStepsIfWorkLost(latestTask); + } let cleanupFailed = false; if (worktreePath && existsSync(worktreePath)) { @@ -23468,6 +23542,21 @@ You have access to the file system to review changes.${inlineFixBlock}${verdictB return true; } + /** + * FNXC:ExternalExecutionCheckout 2026-08-09-22:43: + * External checkout routing is durable task state. Long-lived executor callbacks must re-read the matching task row before choosing a checkout so a stale graph snapshot cannot route execution, verification, remediation, or cleanup back to a Fusion-managed worktree. + */ + private async resolveAuthoritativeExternalExecutionRoute( + task: Task, + ): Promise<{ task: Task; route: ExternalExecutionCheckoutResolution }> { + const live = await this.store.getTask(task.id).catch(() => null); + const authoritativeTask = live?.id === task.id ? live : task; + return { + task: authoritativeTask, + route: await resolveExternalExecutionCheckoutRoute(authoritativeTask), + }; + } + /** * FNXC:Workspace 2026-06-21-12:00: KTD2 single-path-getter contract. Returns the task's sole worktree path for single-repo tasks (one-element set). For a multi-worktree workspace task there is no single answer — callers must read the per-repo `task.workspaceWorktrees` entry instead — so this returns undefined. A workspace task tracked only at the browse-only root also returns undefined, matching the "no removable single worktree" semantics. */