root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(executor): preserve external checkout ownership guards (#3404)

Browse Source 
## Summary
- keep operator-routed external checkouts out of managed worktree
preflight, cleanup, and lost-work reconciliation paths
- mark injected custom graph worktree creation as native so workspace
mode accepts the managed backend
- add an extraction regression guard for the ownership fences

## Test plan
- `pnpm --filter @fusion/engine exec vitest run
src/executor/__tests__/external-checkout-extraction-guards.test.ts
--silent=passed-only --reporter=dot`
- `pnpm --filter @fusion/engine typecheck`
- `pnpm check:changesets`
- targeted ESLint on the changed TypeScript files


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* External execution checkouts are no longer treated as Fusion-managed
worktrees.
* Prevented unnecessary Git checks, cleanup, and reconciliation during
retries, pauses, recovery, and stuck-task handling.
* Invalid external checkout configurations now fail safely with an
error.
* Graph-injected worktrees now use the native worktree backend for
consistent setup.
* **Tests**
* Added coverage verifying external checkouts remain excluded from
managed worktree operations.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Phil Larson
2026-08-09 22:40:01 -07:00
committed by GitHub
parent a5c3476cb2
commit b30508c685
6 changed files with 118 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
.changeset/executor-split-external-checkout-guards.md Normal file
View File

@@ -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.

44
packages/engine/src/executor/__tests__/external-checkout-extraction-guards.test.ts Normal file
View File

@@ -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"');
});
});

25
packages/engine/src/executor/__tests__/session-worktree-pure-helpers.test.ts
View File

@@ -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<string, Set<string>>([
["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<string, Set<string>>([
["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<string>(["a1"]);

12
packages/engine/src/executor/active-worktrees.ts
View File

@@ -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<string, Set<string>>,
taskId: string,
externalExecutionConfigured: boolean,
): void {
if (externalExecutionConfigured) activeWorktrees.delete(taskId);
}

1
packages/engine/src/executor/ensure-graph-custom-node-worktree.ts
View File

@@ -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,

41
packages/engine/src/executor/run-implementation.ts
View File

@@ -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,