diff --git a/.changeset/executor-split-external-checkout-guards.md b/.changeset/executor-split-external-checkout-guards.md new file mode 100644 index 0000000000..2c9eefbec2 --- /dev/null +++ b/.changeset/executor-split-external-checkout-guards.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Prevent Fusion from modifying or deleting operator-owned external execution checkouts. +category: fix +dev: Preserves base capture while fencing managed cleanup, reconciliation, and worktree backend behavior. diff --git a/packages/engine/src/executor/__tests__/external-checkout-extraction-guards.test.ts b/packages/engine/src/executor/__tests__/external-checkout-extraction-guards.test.ts new file mode 100644 index 0000000000..863f5c8567 --- /dev/null +++ b/packages/engine/src/executor/__tests__/external-checkout-extraction-guards.test.ts @@ -0,0 +1,44 @@ +import { readFileSync } from "node:fs"; +import { join, resolve } from "node:path"; +import { describe, expect, it } from "vitest"; + +/* +FNXC:CodeOrganization 2026-08-10-02:15: +The executor split must preserve external-checkout ownership fences that used to live in executor.ts. +*/ +const REPO_ROOT = resolve(import.meta.dirname, "../../../../.."); + +function readSource(path: string): string { + return readFileSync(join(REPO_ROOT, path), "utf8"); +} + +describe("executor extraction safety guards", () => { + it("keeps operator-owned external checkouts outside managed worktree preflight and cleanup", () => { + const source = readSource("packages/engine/src/executor/run-implementation.ts"); + + expect(source).toMatch( + /if \(!deps\.workspaceConfig && !acquisition\.isResume\) \{\n\s+await captureBaseCommitSha\(deps\.store, task, worktreePath, audit, \{ isResume: false \}\);\n\s+\}\n\n\s+if \(!deps\.workspaceConfig && !externalExecutionRoute\.configured\) \{/, + ); + expect(source).toContain("if (!deps.workspaceConfig && !externalExecutionRoute.configured)"); + expect( + source.match(/^\s*if \(!externalExecutionRoute\.configured && worktreePath && existsSync\(worktreePath\)\) \{/gm) ?? [], + ).toHaveLength(5); + expect( + source.match(/^\s*if \(!externalExecutionRoute\.configured\) \{\n\s+await deps\.resetStepsIfWorkLost\(latestTask\);\n\s*\}/gm) ?? [], + ).toHaveLength(2); + expect(source).toMatch( + /\} finally \{\n\s+\/\*\n\s+FNXC:ExternalExecutionCheckout 2026-08-10-03:13:\n\s+External checkouts remain operator-owned[\s\S]*?\n\s+\*\/\n\s+releaseExternalExecutionActiveWorktree\(/, + ); + expect(source).toMatch( + /releaseExternalExecutionActiveWorktree\(\n\s+deps\.activeWorktrees,\n\s+task\.id,\n\s+externalExecutionRoute\.configured,\n\s+\);\n\n\s+if \(reviewAddressingActivated\) \{[\s\S]*?deps\.executing\.delete\(task\.id\);\n\s+executingTaskLock\.release\(task\.id\);/, + ); + expect(source).not.toMatch(/^\s*if \(worktreePath && existsSync\(worktreePath\)\) \{/m); + expect(source.match(/^\s*await deps\.resetStepsIfWorkLost\(latestTask\);$/gm) ?? []).toHaveLength(2); + }); + + it("marks the injected graph-node worktree creator as native", () => { + const source = readSource("packages/engine/src/executor/ensure-graph-custom-node-worktree.ts"); + + expect(source).toContain('createWorktreeBackendKind: "native"'); + }); +}); diff --git a/packages/engine/src/executor/__tests__/session-worktree-pure-helpers.test.ts b/packages/engine/src/executor/__tests__/session-worktree-pure-helpers.test.ts index 3b261ea5ab..e48c6fb688 100644 --- a/packages/engine/src/executor/__tests__/session-worktree-pure-helpers.test.ts +++ b/packages/engine/src/executor/__tests__/session-worktree-pure-helpers.test.ts @@ -14,6 +14,7 @@ import { isEphemeralDeletionPending, } from "../ephemeral-deletion-pending.js"; import { buildInjectedRuntimeEnv } from "../build-injected-runtime-env.js"; +import { releaseExternalExecutionActiveWorktree } from "../active-worktrees.js"; describe("hasLiveSessionSurface", () => { it("is true when any session map owns the task", () => { @@ -75,6 +76,30 @@ describe("getWorktreePath", () => { }); }); +describe("releaseExternalExecutionActiveWorktree", () => { + it("releases only the external task binding and leaves unrelated holders intact", () => { + const activeWorktrees = new Map>([ + ["external", new Set(["/operator-owned"])], + ["other", new Set(["/managed"])], + ]); + + releaseExternalExecutionActiveWorktree(activeWorktrees, "external", true); + + expect(activeWorktrees.has("external")).toBe(false); + expect(activeWorktrees.get("other")).toEqual(new Set(["/managed"])); + }); + + it("preserves managed worktree bindings", () => { + const activeWorktrees = new Map>([ + ["managed", new Set(["/managed"])], + ]); + + releaseExternalExecutionActiveWorktree(activeWorktrees, "managed", false); + + expect(activeWorktrees.get("managed")).toEqual(new Set(["/managed"])); + }); +}); + describe("ephemeral deletion helpers", () => { it("tracks pending deletes and clears on dispose", () => { const pending = new Set(["a1"]); diff --git a/packages/engine/src/executor/active-worktrees.ts b/packages/engine/src/executor/active-worktrees.ts index 68517ea178..adf9c69803 100644 --- a/packages/engine/src/executor/active-worktrees.ts +++ b/packages/engine/src/executor/active-worktrees.ts @@ -25,3 +25,15 @@ export function getActiveWorktreePaths( const set = activeWorktrees.get(taskId); return set ? Array.from(set) : []; } + +/** + * FNXC:ExternalExecutionCheckout 2026-08-10-03:13: + * Operator-owned external checkouts stay on disk, but the executor must release their in-memory ownership binding on every run exit. Managed worktrees keep their existing lifecycle because their cleanup paths own that binding separately. + */ +export function releaseExternalExecutionActiveWorktree( + activeWorktrees: Map>, + taskId: string, + externalExecutionConfigured: boolean, +): void { + if (externalExecutionConfigured) activeWorktrees.delete(taskId); +} diff --git a/packages/engine/src/executor/ensure-graph-custom-node-worktree.ts b/packages/engine/src/executor/ensure-graph-custom-node-worktree.ts index 84758c8a4a..05cf4c9389 100644 --- a/packages/engine/src/executor/ensure-graph-custom-node-worktree.ts +++ b/packages/engine/src/executor/ensure-graph-custom-node-worktree.ts @@ -89,6 +89,7 @@ export async function ensureGraphCustomNodeWorktree( runContext: deps.getRunContextFor(task.id), runInitCommand: true, createWorktree: deps.createWorktree, + createWorktreeBackendKind: "native", runConfiguredCommand: (command, cwd, timeoutMs, env) => deps.runConfiguredCommand( command, diff --git a/packages/engine/src/executor/run-implementation.ts b/packages/engine/src/executor/run-implementation.ts index ef82131fbb..b191ebf32f 100644 --- a/packages/engine/src/executor/run-implementation.ts +++ b/packages/engine/src/executor/run-implementation.ts @@ -152,6 +152,7 @@ import { StepSessionExecutor } from "../execution/step-session-executor.js"; import { isResearchToolSurfaceEnabled } from "../execution/tool-availability.js"; import { summarizeVerificationOutput } from "../execution/verification-utils.js"; import { buildAgentPersona } from "./agent-binding-pure.js"; +import { releaseExternalExecutionActiveWorktree } from "./active-worktrees.js"; import { evaluateImplicitCompletionRefusal } from "./completion-predicates.js"; import { configuredCommandErrorMessage, @@ -762,15 +763,17 @@ export async function runImplementation( /* FNXC:Workspace 2026-06-21-12:00: - KTD1 — every preflight below (base-commit capture, contamination check, worktree-liveness gate) runs git against `worktreePath`, which equals the non-git workspace root in workspace mode. They would all fail. Gate the whole block off in workspace mode; the per-repo equivalents return in Phase B (master U3) against each acquired sub-repo worktree. The non-workspace branch is unchanged. + KTD1 — the git preflights below run against `worktreePath`, which equals the non-git workspace root in workspace mode. The per-repo equivalents return in Phase B (master U3) against each acquired sub-repo worktree. + + FNXC:ExternalExecutionCheckout 2026-08-10-03:05: + An operator-routed checkout still needs the read-only base snapshot used by modified-file capture. It must not enter contamination or managed-worktree liveness checks: the persisted checkout is deliberately operator-owned and lives outside Fusion's worktree directory. */ - if (!deps.workspaceConfig) { - // Capture the base commit SHA for diff computation whenever a task - // starts with a newly assigned worktree. - if (!acquisition.isResume) { + if (!deps.workspaceConfig && !acquisition.isResume) { await captureBaseCommitSha(deps.store, task, worktreePath, audit, { isResume: false }); } + if (!deps.workspaceConfig && !externalExecutionRoute.configured) { + // Contamination check must use a FRESH merge-base with the integration // branch — NOT task.baseCommitSha. baseCommitSha is intentionally // preserved across sessions for stable diff math, which makes it @@ -1511,7 +1514,7 @@ export async function runImplementation( executorLog.warn(`⚡ ${task.id} transient error — retry ${attempt}/${MAX_RECOVERY_RETRIES} in ${delay}: ${errorMessage}`); await deps.store.logEntry(task.id, `Transient error (retry ${attempt}/${MAX_RECOVERY_RETRIES} in ${delay}): ${errorMessage}`, undefined, deps.getRunContextFor(task.id)); } - if (worktreePath && existsSync(worktreePath)) { + if (!externalExecutionRoute.configured && worktreePath && existsSync(worktreePath)) { try { const settings = await deps.store.getSettings(); await removeWorktree({ @@ -1611,9 +1614,11 @@ export async function runImplementation( 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 deps.resetStepsIfWorkLost(latestTask); + if (!externalExecutionRoute.configured) { + await deps.resetStepsIfWorkLost(latestTask); + } - if (worktreePath && existsSync(worktreePath)) { + if (!externalExecutionRoute.configured && worktreePath && existsSync(worktreePath)) { try { await removeWorktree({ worktreePath, @@ -3042,7 +3047,7 @@ export async function runImplementation( return; } else { executorLog.log(`${task.id} paused — moving to todo`); - if (worktreePath && existsSync(worktreePath)) { + if (!externalExecutionRoute.configured && worktreePath && existsSync(worktreePath)) { try { const settings = await deps.store.getSettings(); await removeWorktree({ @@ -3566,7 +3571,7 @@ export async function runImplementation( await deps.store.logEntry(task.id, `Transient error (retry ${attempt}/${MAX_RECOVERY_RETRIES} in ${delay}): ${errorMessage}`, undefined, deps.getRunContextFor(task.id)); } // Clean up the old worktree so the retry gets a fresh one - if (worktreePath && existsSync(worktreePath)) { + if (!externalExecutionRoute.configured && worktreePath && existsSync(worktreePath)) { try { const settings = await deps.store.getSettings(); await removeWorktree({ @@ -3621,6 +3626,16 @@ export async function runImplementation( deps.options.onError?.(task, err instanceof Error ? err : new Error(errorMessage)); } } finally { + /* + FNXC:ExternalExecutionCheckout 2026-08-10-03:13: + External checkouts remain operator-owned and are never removed by Fusion, but every run exit must clear their in-memory active-worktree ownership before any awaited teardown or executor-lock release. This prevents teardown errors from retaining a phantom holder and prevents an old run from deleting a successor run's binding. + */ + releaseExternalExecutionActiveWorktree( + deps.activeWorktrees, + task.id, + externalExecutionRoute.configured, + ); + if (reviewAddressingActivated) { const latestTask = await deps.store.getTask(task.id); if (taskDone) { @@ -3758,10 +3773,12 @@ export async function runImplementation( 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 deps.resetStepsIfWorkLost(latestTask); + if (!externalExecutionRoute.configured) { + await deps.resetStepsIfWorkLost(latestTask); + } // Clean up the old worktree so the retry gets a fresh one - if (worktreePath && existsSync(worktreePath)) { + if (!externalExecutionRoute.configured && worktreePath && existsSync(worktreePath)) { try { await removeWorktree({ worktreePath,