From 080a8e7134ae898b9759c8d5f8ff322c57c4fe15 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Tue, 21 Jul 2026 19:40:36 -0700 Subject: [PATCH] FN-8464: guard baseline capture against invalid worktrees Prevent baseline Git probes from using stale or non-directory task worktrees. - Gate baseline capture on an existing worktree directory - Defer graph step projection until worktree acquisition completes - Cover missing, non-directory, and filesystem-race worktree paths - Add a patch changeset for the operator-facing fix Files changed: .changeset/fn-8464-baseline-cwd.md | 7 ++ .../__tests__/executor-fast-mode-workflows.test.ts | 100 ++++++++++++++++++++- .../engine/src/__tests__/executor-test-helpers.ts | 6 +- packages/engine/src/__tests__/step-runner.test.ts | 62 +++++++++++++ packages/engine/src/executor.ts | 16 +++- packages/engine/src/step-runner.ts | 24 +++++ 6 files changed, 210 insertions(+), 5 deletions(-) Fusion-Task-Id: FN-8464 Fusion-Task-Lineage: e4116dd0-decd-4f9d-87f0-e695cc7f182b Co-authored-by: Fusion (runfusion.ai) --- .changeset/fn-8464-baseline-cwd.md | 7 ++ .../executor-fast-mode-workflows.test.ts | 100 +++++++++++++++++- .../src/__tests__/executor-test-helpers.ts | 6 +- .../engine/src/__tests__/step-runner.test.ts | 62 +++++++++++ packages/engine/src/executor.ts | 16 ++- packages/engine/src/step-runner.ts | 24 +++++ 6 files changed, 210 insertions(+), 5 deletions(-) create mode 100644 .changeset/fn-8464-baseline-cwd.md diff --git a/.changeset/fn-8464-baseline-cwd.md b/.changeset/fn-8464-baseline-cwd.md new file mode 100644 index 0000000000..cbf81c012e --- /dev/null +++ b/.changeset/fn-8464-baseline-cwd.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Stop spurious per-task `spawn /bin/sh ENOENT` noise during step baseline capture. +category: fix +dev: Graph step projection now defers missing, non-directory, and stat-error worktrees until a real checkout exists (FN-8464 / issue #2386). 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 ed19c4b387..9842e8b417 100644 --- a/packages/engine/src/__tests__/executor-fast-mode-workflows.test.ts +++ b/packages/engine/src/__tests__/executor-fast-mode-workflows.test.ts @@ -15,6 +15,8 @@ import { createMockStore, mockedCreateFnAgent, mockedExistsSync, + mockedExec, + mockedStatSync, resetExecutorMocks, } from "./executor-test-helpers.js"; @@ -239,14 +241,108 @@ describe("fast mode workflow/runtime invariants", () => { }); }); - it("applies fresh-worktree step ordering through the legacy graph seam", async () => { + it("defers truthy missing and non-directory worktrees until acquisition", async () => { + for (const [worktree, exists, directory] of [ + ["/tmp/fn-8464-missing-worktree", false, false], + ["/tmp/fn-8464-file-worktree", true, false], + ]) { + let liveTask = task({ + steps: [{ name: "Preflight", status: "pending" }], + worktree, + baseCommitSha: undefined, + }); + const store = createMockStore(); + store.getTask.mockImplementation(async () => liveTask); + mockedExistsSync.mockReturnValue(exists); + mockedStatSync.mockReturnValue({ isDirectory: () => directory } as any); + const executor = new TaskExecutor(store, "/tmp/project-root"); + vi.spyOn(executor as any, "runGraphTaskStep").mockImplementation(async () => { + expect(store.updateStep).not.toHaveBeenCalled(); + liveTask = { ...liveTask, worktree: "/tmp/acquired", baseCommitSha: "acquired-base" }; + return { success: true }; + }); + + const result = await (executor as any).runProjectedGraphTaskStep( + liveTask, + liveTask, + 0, + { foreachNodeId: "steps", stepIndex: 0, instanceId: "steps#0" }, + ); + + expect(result).toMatchObject({ outcome: "success", baselineSha: "acquired-base" }); + expect(mockedExec).not.toHaveBeenCalled(); + } + }); + + it("defers a worktree whose directory stat throws instead of propagating a cwd race", async () => { let liveTask = task({ steps: [{ name: "Preflight", status: "pending" }], - worktree: undefined, + worktree: "/tmp/fn-8464-stat-race", baseCommitSha: undefined, }); const store = createMockStore(); store.getTask.mockImplementation(async () => liveTask); + mockedStatSync.mockImplementation(() => { + throw new Error("simulated removal race"); + }); + const executor = new TaskExecutor(store, "/tmp/project-root"); + vi.spyOn(executor as any, "runGraphTaskStep").mockImplementation(async () => { + expect(store.updateStep).not.toHaveBeenCalled(); + liveTask = { ...liveTask, worktree: "/tmp/acquired", baseCommitSha: "acquired-base" }; + return { success: true }; + }); + + await expect( + (executor as any).runProjectedGraphTaskStep( + liveTask, + liveTask, + 0, + { foreachNodeId: "steps", stepIndex: 0, instanceId: "steps#0" }, + ), + ).resolves.toMatchObject({ outcome: "success", baselineSha: "acquired-base" }); + expect(mockedExec).not.toHaveBeenCalled(); + }); + + it("captures a pre-step baseline when the projected worktree is a directory", async () => { + const liveTask = task({ + steps: [{ name: "Preflight", status: "pending" }], + worktree: "/tmp/fn-8464-existing-worktree", + }); + const store = createMockStore(); + store.getTask.mockResolvedValue(liveTask); + mockedExistsSync.mockReturnValue(true); + mockedStatSync.mockReturnValue({ isDirectory: () => true } as any); + mockedExec.mockImplementation((_command: string, _options: unknown, callback: any) => { + callback(null, "existing-head\n", ""); + return {} as any; + }); + const executor = new TaskExecutor(store, "/tmp/project-root"); + const runGraphTaskStep = vi + .spyOn(executor as any, "runGraphTaskStep") + .mockResolvedValue({ success: true }); + + const result = await (executor as any).runProjectedGraphTaskStep( + liveTask, + liveTask, + 0, + { foreachNodeId: "steps", stepIndex: 0, instanceId: "steps#0" }, + ); + + expect(result).toMatchObject({ outcome: "success", baselineSha: "existing-head" }); + expect(runGraphTaskStep).toHaveBeenCalledOnce(); + expect(store.updateStep).toHaveBeenCalledWith("FN-6226", 0, "in-progress", { source: "graph" }); + expect(mockedExec).toHaveBeenCalledWith("git rev-parse HEAD", { cwd: liveTask.worktree }, expect.any(Function)); + }); + + it("applies missing-worktree step ordering through the legacy graph seam", async () => { + let liveTask = task({ + steps: [{ name: "Preflight", status: "pending" }], + worktree: "/tmp/fn-8464-legacy-missing-worktree", + baseCommitSha: undefined, + }); + mockedExistsSync.mockReturnValue(false); + const store = createMockStore(); + store.getTask.mockImplementation(async () => liveTask); const executor = new TaskExecutor(store, "/tmp/project-root"); vi.spyOn(executor as any, "runGraphTaskStep").mockImplementation(async () => { expect(store.updateStep).not.toHaveBeenCalled(); diff --git a/packages/engine/src/__tests__/executor-test-helpers.ts b/packages/engine/src/__tests__/executor-test-helpers.ts index 9c47223f75..fcab4a3694 100644 --- a/packages/engine/src/__tests__/executor-test-helpers.ts +++ b/packages/engine/src/__tests__/executor-test-helpers.ts @@ -318,6 +318,7 @@ vi.mock("node:fs", () => ({ existsSync: vi.fn().mockReturnValue(true), realpathSync: vi.fn((path: string) => path), lstatSync: vi.fn(() => ({ isSymbolicLink: () => false, isDirectory: () => true })), + statSync: vi.fn(() => ({ isDirectory: () => true })), })); export const mockExecuteAll: Mock<() => Promise> = vi.fn().mockResolvedValue([]); @@ -392,7 +393,7 @@ import { findWorktreeUser } from "../merger.js"; import { StepSessionExecutor } from "../step-session-executor.js"; import { withRateLimitRetry } from "../rate-limit-retry.js"; import { exec, execSync } from "node:child_process"; -import { existsSync, realpathSync } from "node:fs"; +import { existsSync, realpathSync, statSync } from "node:fs"; import { hydrateWorktreeDb } from "../worktree-db-hydrate.js"; import { classifyTaskWorktree, describeRegisteredWorktrees, isUsableTaskWorktree } from "../worktree-pool.js"; import { classifyStaleLock, tryRemoveStaleLock } from "../worktree-stale-lock.js"; @@ -410,6 +411,7 @@ export const mockedExec = vi.mocked(exec); export const mockedExecSync = vi.mocked(execSync); export const mockedExistsSync = vi.mocked(existsSync); export const mockedRealpathSync = vi.mocked(realpathSync); +export const mockedStatSync = vi.mocked(statSync); export const mockedHydrateWorktreeDb = vi.mocked(hydrateWorktreeDb); export const mockedClassifyTaskWorktree = vi.mocked(classifyTaskWorktree); export const mockedDescribeRegisteredWorktrees = vi.mocked(describeRegisteredWorktrees); @@ -750,6 +752,8 @@ export function resetExecutorMocks() { vi.clearAllMocks(); mockedExec.mockReset(); mockedExecSync.mockReset(); + mockedStatSync.mockReset(); + mockedStatSync.mockReturnValue({ isDirectory: () => true } as ReturnType); mockedIsUsableTaskWorktree.mockResolvedValue(true); mockedClassifyTaskWorktree.mockImplementation(async (rootDir: string, worktreePath: string) => { const usable = await mockedIsUsableTaskWorktree(rootDir, worktreePath); diff --git a/packages/engine/src/__tests__/step-runner.test.ts b/packages/engine/src/__tests__/step-runner.test.ts index f65e4033c3..15e2755fce 100644 --- a/packages/engine/src/__tests__/step-runner.test.ts +++ b/packages/engine/src/__tests__/step-runner.test.ts @@ -11,10 +11,14 @@ */ import { describe, it, expect, vi, beforeEach } from "vitest"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { runTaskStep, resetStepToBaseline, makeAncestryBlastRadiusGuard, + isUsableWorktreeDirectory, type StepRunnerTask, type SessionRef, } from "../step-runner.js"; @@ -158,6 +162,41 @@ describe("runTaskStep", () => { expect(result.checkpointId).toBe("leaf"); }); + it("defers default baseline capture for missing and non-directory worktree paths", async () => { + const missingPath = join(tmpdir(), `fn-8464-missing-${Date.now()}`); + const filePath = join(tmpdir(), `fn-8464-file-${Date.now()}`); + await writeFile(filePath, "not a directory"); + try { + for (const worktreePath of [missingPath, filePath]) { + const result = await runTaskStep( + { store: makeStore(), worktreePath, runStep: async () => ({ success: true }) }, + makeTask([{ status: "pending" }]), + 0, + ); + expect(result).toMatchObject({ outcome: "success", baselineSha: undefined }); + } + } finally { + await rm(filePath, { force: true }); + } + }); + + it("keeps pre-step baseline capture for a usable worktree directory", async () => { + const worktreePath = await mkdtemp(join(tmpdir(), "fn-8464-directory-")); + const gitRevParse = vi.fn().mockResolvedValue("pre-step-sha"); + const runStep = vi.fn().mockResolvedValue({ success: true }); + try { + const result = await runTaskStep( + { store: makeStore(), worktreePath, runStep, gitRevParse }, + makeTask([{ status: "pending" }]), + 0, + ); + expect(result.baselineSha).toBe("pre-step-sha"); + expect(gitRevParse.mock.invocationCallOrder[0]).toBeLessThan(runStep.mock.invocationCallOrder[0]); + } finally { + await rm(worktreePath, { recursive: true, force: true }); + } + }); + it("uses the default checkpoint capture from the session ref when none injected", async () => { const store = makeStore(); const sessionRef = makeSessionRef({ leafId: "leaf-xyz" }); @@ -176,6 +215,29 @@ describe("runTaskStep", () => { }); }); +describe("isUsableWorktreeDirectory", () => { + it("returns false for empty, missing, and non-directory candidates", async () => { + const filePath = join(tmpdir(), `fn-8464-helper-file-${Date.now()}`); + await writeFile(filePath, "not a directory"); + try { + expect(isUsableWorktreeDirectory(undefined)).toBe(false); + expect(isUsableWorktreeDirectory(join(tmpdir(), `fn-8464-helper-missing-${Date.now()}`))).toBe(false); + expect(isUsableWorktreeDirectory(filePath)).toBe(false); + } finally { + await rm(filePath, { force: true }); + } + }); + + it("returns true for an existing directory", async () => { + const worktreePath = await mkdtemp(join(tmpdir(), "fn-8464-helper-directory-")); + try { + expect(isUsableWorktreeDirectory(worktreePath)).toBe(true); + } finally { + await rm(worktreePath, { recursive: true, force: true }); + } + }); +}); + describe("resetStepToBaseline", () => { beforeEach(() => vi.clearAllMocks()); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 430416c976..552a2824e4 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -198,7 +198,13 @@ import type { StuckTaskDetector, StuckTaskEvent } from "./stuck-task-detector.js import type { PluginRunner } from "./plugin-runner.js"; import { isContextLimitError } from "./context-limit-detector.js"; import { StepSessionExecutor } from "./step-session-executor.js"; -import { makeAncestryBlastRadiusGuard, resetStepToBaseline, runTaskStep, type RunTaskStepResult } from "./step-runner.js"; +import { + isUsableWorktreeDirectory, + makeAncestryBlastRadiusGuard, + resetStepToBaseline, + runTaskStep, + type RunTaskStepResult, +} from "./step-runner.js"; // FNXC:MergerUnification 2026-06-21-19:05: the foundation branch imported `acquireWorkspaceRepoWorktree` here but never used it in executor.ts (the agent tool wraps it via agent-tools.ts), which fails lint on the inherited base. Removed until master-plan U1 re-adds it together with its per-repo acquisition usage. import { acquireTaskWorktree, type AcquireTaskWorktreeResult } from "./worktree-acquisition.js"; import { resolveCapturedBaseCommitSha } from "./base-commit-capture.js"; @@ -6883,7 +6889,13 @@ export class TaskExecutor { thinkingLevel, ); - if (!worktreePath) { + /* + * FNXC:BaselineCwdGating 2026-07-21-19:21: + * FN-8464 requires graph step projection to defer until this candidate is a real directory. + * A stale, non-directory, or inaccessible truthy path must follow fresh-worktree ordering so + * runTaskStep never spawns baseline git with an unusable cwd; acquisition supplies baseCommitSha. + */ + if (!worktreePath || !isUsableWorktreeDirectory(worktreePath)) { const result = await runStep(stepIndex); const refreshed = await this.store.getTask(task.id).catch(() => live); return { diff --git a/packages/engine/src/step-runner.ts b/packages/engine/src/step-runner.ts index 9549342f27..ec6ab80e75 100644 --- a/packages/engine/src/step-runner.ts +++ b/packages/engine/src/step-runner.ts @@ -26,6 +26,7 @@ */ import { exec } from "node:child_process"; +import { existsSync, statSync } from "node:fs"; import { promisify } from "node:util"; import type { TaskStore } from "@fusion/core"; @@ -383,7 +384,30 @@ export function makeAncestryBlastRadiusGuard(opts: { // ── Defaults (production adapters over real git/session) ───────────────── +/** + * FNXC:BaselineCwdGating 2026-07-21-19:21: + * A truthy task worktree path does not prove a checkout exists as a directory. + * Missing or non-directory cwd values make Node report the misleading `spawn /bin/sh ENOENT` + * during FN-8464 baseline capture (Runfusion/Fusion#2386). This check is total: empty, + * missing, non-directory, or any filesystem race/access error defers capture without failing + * graph step projection. + */ +export function isUsableWorktreeDirectory(candidate: string | undefined | null): boolean { + if (!candidate) return false; + try { + return existsSync(candidate) && statSync(candidate).isDirectory(); + } catch { + return false; + } +} + async function defaultGitRevParse(worktreePath: string): Promise { + /* + * FNXC:BaselineCwdGating 2026-07-21-19:21: + * Keep this defense at the git seam as callers beyond graph projection may pass stale paths. + * Never spawn git until the shared total directory check proves its cwd is usable. + */ + if (!isUsableWorktreeDirectory(worktreePath)) return undefined; const { stdout } = await execAsync("git rev-parse HEAD", { cwd: worktreePath }); const sha = stdout.trim(); return sha.length > 0 ? sha : undefined;