fix(FN-2209): run post-merge workflow steps in isolated worktrees

- Add helpers to create/remove temporary post-merge worktrees with graceful fallback to rootDir - Run post-merge workflow steps before worktree cleanup and pass explicit execution cwd to script and prompt modes - Detect enabled post-merge steps before provisioning isolated worktrees to avoid unnecessary git worktree operations - Expand merger tests to verify isolated cwd usage, fallback behavior, cleanup on failure, and no-worktree path
2026-04-21 01:29:02 -07:00
parent e1af5bbfe4
commit 16e3a5104a
2 changed files with 299 additions and 22 deletions
--- a/packages/engine/src/merger.test.ts
+++ b/packages/engine/src/merger.test.ts
@@ -114,11 +114,12 @@ import {
 } from "./merger.js";
 import { mergerLog } from "./logger.js";
 import { createFnAgent } from "./pi.js";
-import { execSync } from "node:child_process";
+import { execSync, exec } from "node:child_process";
 import { type TaskStore, type Task, type MergeResult, DEFAULT_SETTINGS } from "@fusion/core";

 const mockedCreateFnAgent = vi.mocked(createFnAgent);
 const mockedExecSync = vi.mocked(execSync);
+const mockedExec = vi.mocked(exec);
 const { existsSync: mockedExistsSyncRaw, readFileSync: mockedReadFileSyncRaw } = await import("node:fs");
 const mockedExistsSync = vi.mocked(mockedExistsSyncRaw);
 const mockedReadFileSync = vi.mocked(mockedReadFileSyncRaw);
@@ -3735,6 +3736,13 @@ describe("aiMergeTask — post-merge workflow steps", () => {
    // getWorkflowStep should have been called for the post-merge step
    expect((store as any).getWorkflowStep).toHaveBeenCalledWith("WS-001");

+    const postMergeAgentCall = mockedCreateFnAgent.mock.calls.find(
+      (c: any) => c[0]?.systemPrompt?.includes("post-merge workflow step agent"),
+    );
+    expect(postMergeAgentCall).toBeDefined();
+    expect(postMergeAgentCall?.[0]?.cwd).toMatch(/\.worktrees\/post-merge-FN-050-[a-z0-9]+/);
+    expect(postMergeAgentCall?.[0]?.cwd).not.toBe("/tmp/root");
+
    // Task should still move to done even though post-merge step ran
    expect(store.moveTask).toHaveBeenCalledWith("FN-050", "done");
  });
@@ -3957,9 +3965,201 @@ describe("aiMergeTask — post-merge workflow steps", () => {

    const result = await aiMergeTask(store, "/tmp/root", "FN-050");

+    const scriptExecCall = mockedExec.mock.calls.find((call: any) => String(call[0]) === "pnpm build");
+    expect(scriptExecCall).toBeDefined();
+    expect(scriptExecCall?.[1]?.cwd).toMatch(/\.worktrees\/post-merge-FN-050-[a-z0-9]+/);
+
    expect(result.merged).toBe(true);
    expect(store.moveTask).toHaveBeenCalledWith("FN-050", "done");
  });
+
+  it("creates temporary worktree for post-merge steps", async () => {
+    const store = createMockStore();
+    (store as any).getWorkflowStep = vi.fn().mockResolvedValue({
+      id: "WS-001",
+      name: "Post-merge Notify",
+      description: "Send notifications after merge",
+      prompt: "Check the merged code and confirm all is well.",
+      phase: "post-merge",
+      mode: "prompt",
+      enabled: true,
+      createdAt: new Date().toISOString(),
+      updatedAt: new Date().toISOString(),
+    });
+
+    const baseTask = {
+      id: "FN-050",
+      title: "Test task",
+      description: "Test",
+      column: "in-review",
+      dependencies: [],
+      worktree: "/tmp/root/.worktrees/KB-050",
+      steps: [],
+      currentStep: 0,
+      log: [],
+      enabledWorkflowSteps: ["WS-001"],
+      createdAt: new Date().toISOString(),
+      updatedAt: new Date().toISOString(),
+    };
+    store.getTask = vi.fn().mockResolvedValue({ ...baseTask, prompt: "# test" });
+
+    await aiMergeTask(store, "/tmp/root", "FN-050");
+
+    const worktreeAddCall = mockedExec.mock.calls.find((call: any) =>
+      String(call[0]).includes("git worktree add") && String(call[0]).includes("post-merge-FN-050-"),
+    );
+    const worktreeRemoveCall = mockedExec.mock.calls.find((call: any) =>
+      String(call[0]).includes("git worktree remove --force") && String(call[0]).includes("post-merge-FN-050-"),
+    );
+
+    expect(worktreeAddCall).toBeDefined();
+    expect(worktreeRemoveCall).toBeDefined();
+  });
+
+  it("falls back to rootDir when worktree creation fails", async () => {
+    const store = createMockStore();
+    (store as any).getWorkflowStep = vi.fn().mockResolvedValue({
+      id: "WS-001",
+      name: "Post-merge Notify",
+      description: "Send notifications after merge",
+      prompt: "Check the merged code and confirm all is well.",
+      phase: "post-merge",
+      mode: "prompt",
+      enabled: true,
+      createdAt: new Date().toISOString(),
+      updatedAt: new Date().toISOString(),
+    });
+
+    const baseTask = {
+      id: "FN-050",
+      title: "Test task",
+      description: "Test",
+      column: "in-review",
+      dependencies: [],
+      worktree: "/tmp/root/.worktrees/KB-050",
+      steps: [],
+      currentStep: 0,
+      log: [],
+      enabledWorkflowSteps: ["WS-001"],
+      createdAt: new Date().toISOString(),
+      updatedAt: new Date().toISOString(),
+    };
+    store.getTask = vi.fn().mockResolvedValue({ ...baseTask, prompt: "# test" });
+
+    const baseExecImpl = mockedExecSync.getMockImplementation();
+    mockedExecSync.mockImplementation((cmd: any, opts: any) => {
+      const cmdStr = String(cmd);
+      if (cmdStr.includes("git worktree add") && cmdStr.includes("post-merge-FN-050-")) {
+        const error: any = new Error("cannot create worktree");
+        error.stderr = "cannot create worktree";
+        throw error;
+      }
+      return baseExecImpl ? baseExecImpl(cmd, opts) : Buffer.from("");
+    });
+
+    const warnSpy = vi.spyOn(mergerLog, "warn");
+    await aiMergeTask(store, "/tmp/root", "FN-050");
+
+    const postMergeAgentCall = mockedCreateFnAgent.mock.calls.find(
+      (c: any) => c[0]?.systemPrompt?.includes("post-merge workflow step agent"),
+    );
+    expect(postMergeAgentCall?.[0]?.cwd).toBe("/tmp/root");
+    expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("could not create post-merge worktree — falling back to rootDir"));
+    expect(store.moveTask).toHaveBeenCalledWith("FN-050", "done");
+  });
+
+  it("cleans up temporary worktree even when post-merge step fails", async () => {
+    const store = createMockStore();
+    (store as any).getWorkflowStep = vi.fn().mockResolvedValue({
+      id: "WS-001",
+      name: "Post-merge Fail",
+      description: "Will fail",
+      prompt: "Fail this check.",
+      phase: "post-merge",
+      mode: "prompt",
+      enabled: true,
+      createdAt: new Date().toISOString(),
+      updatedAt: new Date().toISOString(),
+    });
+
+    const baseTask = {
+      id: "FN-050",
+      title: "Test task",
+      description: "Test",
+      column: "in-review",
+      dependencies: [],
+      worktree: "/tmp/root/.worktrees/KB-050",
+      steps: [],
+      currentStep: 0,
+      log: [],
+      enabledWorkflowSteps: ["WS-001"],
+      createdAt: new Date().toISOString(),
+      updatedAt: new Date().toISOString(),
+    };
+    store.getTask = vi.fn().mockResolvedValue({ ...baseTask, prompt: "# test" });
+
+    mockedCreateFnAgent.mockImplementation((async (opts: any) => {
+      if (opts.systemPrompt?.includes("post-merge")) {
+        throw new Error("Post-merge agent creation failed");
+      }
+      return {
+        session: {
+          prompt: vi.fn().mockResolvedValue(undefined),
+          dispose: vi.fn(),
+          subscribe: vi.fn(),
+          on: vi.fn(),
+          state: {},
+          sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") },
+        },
+      };
+    }) as any);
+
+    await aiMergeTask(store, "/tmp/root", "FN-050");
+
+    const worktreeRemoveCall = mockedExec.mock.calls.find((call: any) =>
+      String(call[0]).includes("git worktree remove --force") && String(call[0]).includes("post-merge-FN-050-"),
+    );
+    expect(worktreeRemoveCall).toBeDefined();
+    expect(store.moveTask).toHaveBeenCalledWith("FN-050", "done");
+  });
+
+  it("does not create post-merge worktree when no post-merge steps exist", async () => {
+    const store = createMockStore();
+    (store as any).getWorkflowStep = vi.fn().mockResolvedValue({
+      id: "WS-001",
+      name: "Pre-merge Check",
+      description: "Check before merge",
+      prompt: "Run pre-merge checks.",
+      phase: "pre-merge",
+      mode: "prompt",
+      enabled: true,
+      createdAt: new Date().toISOString(),
+      updatedAt: new Date().toISOString(),
+    });
+
+    const baseTask = {
+      id: "FN-050",
+      title: "Test task",
+      description: "Test",
+      column: "in-review",
+      dependencies: [],
+      worktree: "/tmp/root/.worktrees/KB-050",
+      steps: [],
+      currentStep: 0,
+      log: [],
+      enabledWorkflowSteps: ["WS-001"],
+      createdAt: new Date().toISOString(),
+      updatedAt: new Date().toISOString(),
+    };
+    store.getTask = vi.fn().mockResolvedValue({ ...baseTask, prompt: "# test" });
+
+    await aiMergeTask(store, "/tmp/root", "FN-050");
+
+    const worktreeAddCall = mockedExec.mock.calls.find((call: any) =>
+      String(call[0]).includes("git worktree add") && String(call[0]).includes("post-merge-FN-050-"),
+    );
+    expect(worktreeAddCall).toBeUndefined();
+  });
 });

 // ── Merge Details Collection Tests ─────────────────────────────────────
--- a/packages/engine/src/merger.ts
+++ b/packages/engine/src/merger.ts
@@ -1655,6 +1655,42 @@ export async function pushToRemoteAfterMerge(
  }
 }

+/**
+ * Create a temporary worktree from the current HEAD for isolated post-merge step execution.
+ * Returns the worktree path, or null if creation fails (graceful fallback to rootDir).
+ */
+async function createPostMergeWorktree(
+  rootDir: string,
+  taskId: string,
+): Promise<string | null> {
+  const randomSuffix = Math.random().toString(36).slice(2, 10);
+  const postMergeWorktree = join(rootDir, ".worktrees", `post-merge-${taskId}-${randomSuffix}`);
+
+  try {
+    await execAsync(`git worktree add ${quoteArg(postMergeWorktree)} HEAD`, { cwd: rootDir });
+    return postMergeWorktree;
+  } catch (err: unknown) {
+    mergerLog.warn(`${taskId}: failed to create post-merge worktree: ${getCommandErrorMessage(err)}`);
+    return null;
+  }
+}
+
+/**
+ * Remove a temporary worktree created for post-merge step execution.
+ * Non-fatal: logs and swallows errors.
+ */
+async function removePostMergeWorktree(
+  rootDir: string,
+  postMergeWorktree: string,
+  taskId: string,
+): Promise<void> {
+  try {
+    await execAsync(`git worktree remove --force ${quoteArg(postMergeWorktree)}`, { cwd: rootDir });
+  } catch (err: unknown) {
+    mergerLog.warn(`${taskId}: failed to remove post-merge worktree ${postMergeWorktree}: ${getCommandErrorMessage(err)}`);
+  }
+}
+
 /**
 * AI-powered merge with 3-attempt retry logic when autoResolveConflicts is enabled.
 *
@@ -2148,7 +2184,30 @@ export async function aiMergeTask(
    }
  }

-  // 7. Clean up worktree
+  // 7. Run post-merge workflow steps (in temporary worktree for isolation)
+  const hasPostMergeSteps = await hasEnabledPostMergeWorkflowSteps(store, taskId, task.enabledWorkflowSteps);
+  if (hasPostMergeSteps) {
+    const postMergeWorktree = await createPostMergeWorktree(rootDir, taskId);
+    const postMergeCwd = postMergeWorktree || rootDir;
+    if (postMergeWorktree) {
+      mergerLog.log(`${taskId}: running post-merge workflow steps in isolated worktree: ${postMergeWorktree}`);
+    } else {
+      mergerLog.warn(`${taskId}: could not create post-merge worktree — falling back to rootDir`);
+    }
+
+    try {
+      await runPostMergeWorkflowSteps(store, taskId, rootDir, postMergeCwd, settings, options);
+    } catch (err: any) {
+      mergerLog.error(`${taskId}: post-merge workflow steps error: ${err.message}`);
+      // Non-fatal — task still moves to done
+    } finally {
+      if (postMergeWorktree) {
+        await removePostMergeWorktree(rootDir, postMergeWorktree, taskId);
+      }
+    }
+  }
+
+  // 8. Clean up worktree
  if (worktreePath && existsSync(worktreePath)) {
    const otherUser = await findWorktreeUser(store, worktreePath, taskId);
    if (otherUser) {
@@ -2169,7 +2228,7 @@ export async function aiMergeTask(
    }
  }

-  // 7b. Push to remote if configured
+  // 8b. Push to remote if configured
  if (settings.pushAfterMerge && settings.mergeStrategy !== "pull-request") {
    try {
      const pushResult = await pushToRemoteAfterMerge(store, rootDir, taskId, settings, options);
@@ -2189,14 +2248,6 @@ export async function aiMergeTask(
    }
  }

-  // 8. Run post-merge workflow steps (failures logged but do not block completion)
-  try {
-    await runPostMergeWorkflowSteps(store, taskId, rootDir, settings, options);
-  } catch (err: any) {
-    mergerLog.error(`${taskId}: post-merge workflow steps error: ${err.message}`);
-    // Non-fatal — task still moves to done
-  }
-
  // 9. Move task to done
  // Audit trail: record merge completion (FN-1404)
  await audit.database({
@@ -2941,15 +2992,40 @@ export function buildMergePrompt(params: MergePromptParams): string {
  return parts.join("\n");
 }

+async function hasEnabledPostMergeWorkflowSteps(
+  store: TaskStore,
+  taskId: string,
+  enabledWorkflowSteps: string[] | undefined,
+): Promise<boolean> {
+  if (!enabledWorkflowSteps?.length) return false;
+
+  for (const wsId of enabledWorkflowSteps) {
+    try {
+      const ws = await store.getWorkflowStep(wsId);
+      if (!ws) continue;
+      const stepPhase = ws.phase || "pre-merge";
+      if (stepPhase === "post-merge") {
+        return true;
+      }
+    } catch (err: unknown) {
+      mergerLog.warn(`${taskId}: failed to inspect workflow step ${wsId} for post-merge phase: ${getCommandErrorMessage(err)}`);
+    }
+  }
+
+  return false;
+}
+
 /**
 * Run post-merge workflow steps for a task after the merge succeeds.
- * These steps run in the root directory (after merge, worktree may be cleaned up).
- * Failures are logged but do NOT block task completion — the merge is already committed.
+ * Steps execute in an isolated worktree (created from merged HEAD) to prevent
+ * modifications to the main project directory. Falls back to rootDir if worktree
+ * creation fails. Failures are logged but do NOT block task completion.
 */
 async function runPostMergeWorkflowSteps(
  store: TaskStore,
  taskId: string,
  rootDir: string,
+  cwd: string,
  settings: Settings,
  mergeOptions: MergerOptions = {},
 ): Promise<void> {
@@ -3009,8 +3085,8 @@ async function runPostMergeWorkflowSteps(

    try {
      const result = stepMode === "script"
-        ? await executePostMergeScriptStep(store, taskId, ws, rootDir, settings)
-        : await executePostMergePromptStep(store, taskId, ws, rootDir, settings, mergeOptions);
+        ? await executePostMergeScriptStep(store, taskId, ws, cwd, settings)
+        : await executePostMergePromptStep(store, taskId, ws, rootDir, cwd, settings, mergeOptions);
      const completedAt = new Date().toISOString();

      if (result.success) {
@@ -3059,12 +3135,12 @@ async function runPostMergeWorkflowSteps(
  }
 }

-/** Execute a script-mode post-merge workflow step */
+/** Execute a script-mode post-merge workflow step in the provided execution directory. */
 async function executePostMergeScriptStep(
  store: TaskStore,
  taskId: string,
  workflowStep: WorkflowStep,
-  rootDir: string,
+  cwd: string,
  settings: Settings,
 ): Promise<{ success: boolean; output?: string; error?: string }> {
  const scriptName = workflowStep.scriptName!.trim();
@@ -3077,7 +3153,7 @@ async function executePostMergeScriptStep(

  try {
    await execAsync(scriptCommand, {
-      cwd: rootDir,
+      cwd,
      encoding: "utf-8",
      timeout: 120_000,
      maxBuffer: 10 * 1024 * 1024,
@@ -3096,12 +3172,13 @@ async function executePostMergeScriptStep(
  }
 }

-/** Execute a prompt-mode post-merge workflow step using AI agent */
+/** Execute a prompt-mode post-merge workflow step using an AI agent in the provided execution directory. */
 async function executePostMergePromptStep(
  store: TaskStore,
  taskId: string,
  workflowStep: WorkflowStep,
  rootDir: string,
+  cwd: string,
  settings: Settings,
  mergeOptions: MergerOptions = {},
 ): Promise<{ success: boolean; output?: string; error?: string }> {
@@ -3111,7 +3188,7 @@ async function executePostMergePromptStep(
 Task Context:
 - Task ID: ${taskId}
 - The merge has already been completed successfully.
- You are running in the project's root directory with the merged code.
+- You are running in a temporary worktree with the merged code.

 Your Instructions:
 ${workflowStep.prompt}
@@ -3165,7 +3242,7 @@ If issues are found that need attention, describe them clearly.`;
    }

    const { session } = await createFnAgent({
-      cwd: rootDir,
+      cwd,
      systemPrompt: postMergeSystemPrompt,
      tools: toolMode,
      defaultProvider: stepProvider,
@@ -3193,7 +3270,7 @@ If issues are found that need attention, describe them clearly.`;
    await promptWithFallback(
      session,
      `Execute the post-merge workflow step "${workflowStep.name}" for task ${taskId}.\n\n` +
-      `Review the merged code in the project root and evaluate it against your instructions.`,
+      `Review the merged code in the temporary worktree and evaluate it against your instructions.`,
    );

    checkSessionError(session);