fix(engine): review explicit external checkouts

.changeset/safe-ducks-review.md

@@ -0,0 +1,7 @@
+---
+"@runfusion/fusion": patch
+---
+
+summary: Allow review steps to target a validated external checkout.
+category: fix
+dev: Resolves explicit review checkout metadata before spawning read-only step reviewers.

packages/engine/src/__tests__/reviewer-workspace.test.ts

@@ -14,6 +14,10 @@ the cwd of each call. Coverage:
 */
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import { EventEmitter } from "node:events";
+import { execFileSync } from "node:child_process";
+import { mkdtempSync, realpathSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
 import type { ReviewResult } from "../reviewer.js";
 
 // Narrow AI seam: only reviewStep (the agent boundary) is mocked. Everything else is the real executor.
@@ -32,6 +36,14 @@ const mockedReviewStep = vi.mocked(mockedReviewStepFn);
 const ROOT = "/tmp/ws-root"; // NON-git workspace root — must never be a review cwd in workspace mode.
 const WT_A = "/tmp/ws-root/repo-a/.worktrees/fn-1";
 const WT_B = "/tmp/ws-root/repo-b/.worktrees/fn-1";
+const cleanupDirs: string[] = [];
+
+function makeGitCheckout(): string {
+  const dir = mkdtempSync(join(tmpdir(), "fusion-review-checkout-"));
+  cleanupDirs.push(dir);
+  execFileSync("git", ["init", "-b", "main"], { cwd: dir, stdio: "ignore" });
+  return dir;
+}
 
 function makeStore(task: Task): TaskStore & EventEmitter {
   const emitter = new EventEmitter();
@@ -93,6 +105,10 @@ beforeEach(() => {
 });
 afterEach(() => {
   vi.clearAllMocks();
+  while (cleanupDirs.length > 0) {
+    const dir = cleanupDirs.pop();
+    if (dir) rmSync(dir, { recursive: true, force: true });
+  }
 });
 
 describe("U2 KTD3 — reviewWorkspacePerRepo conjunction + tagging (the shared loop both call sites use)", () => {
@@ -212,6 +228,27 @@ describe("U2 KTD3 — in-session fn_review_step (createReviewStepTool) loops per
     await tool.execute("call-1", { step: 1, type: "code", step_name: "Step 1", baseline: "base" });
     expect(seen).toEqual([WT_A]);
   });
+
+  it("explicit external review checkout overrides the Atlas task worktree for fn_review_step", async () => {
+    const externalCheckout = makeGitCheckout();
+    const expectedCheckout = realpathSync(externalCheckout);
+    const task = makeTask({ customFields: { reviewCheckoutPath: externalCheckout } } as any);
+    const store = makeStore(task);
+    const executor = new TaskExecutor(store, ROOT);
+    const seen = scriptReviewByCwd({ [externalCheckout]: { verdict: "APPROVE", review: "external ok", summary: "external" } });
+    const tool = (executor as any).createReviewStepTool(
+      task.id,
+      WT_A,
+      "PROMPT",
+      new Map(),
+      { current: null },
+      new Map(),
+      task,
+      undefined,
+    );
+    await tool.execute("call-1", { step: 1, type: "code", step_name: "Step 1", baseline: "base" });
+    expect(seen).toEqual([expectedCheckout]);
+  });
 });
 
 describe("U2 KTD3 — step-inversion review seam (executor.ts:5668) loops per sub-repo", () => {
@@ -244,4 +281,17 @@ describe("U2 KTD3 — step-inversion review seam (executor.ts:5668) loops per su
     await seams.stepReview!(task as any, context, { type: "code", advisory: true } as any);
     expect(seen).toEqual([WT_A]);
   });
+
+  it("explicit external review checkout overrides the active graph worktree", async () => {
+    const externalCheckout = makeGitCheckout();
+    const expectedCheckout = realpathSync(externalCheckout);
+    const task = makeTask({ worktree: WT_A, customFields: { reviewCheckoutPath: externalCheckout } } as any);
+    const store = makeStore(task);
+    const executor = new TaskExecutor(store, ROOT);
+    const seen = scriptReviewByCwd({ [externalCheckout]: { verdict: "APPROVE", review: "external", summary: "external" } });
+    const seams = executor.createAuthoritativeWorkflowSeams({ autoMerge: false } as any);
+    const context = { [FOREACH_ACTIVE_CONTEXT_KEY]: { stepIndex: 1, worktreePath: WT_A, baselineSha: "base" } } as any;
+    await seams.stepReview!(task as any, context, { type: "code", advisory: true } as any);
+    expect(seen).toEqual([expectedCheckout]);
+  });
 });

packages/engine/src/executor.ts

@@ -24,6 +24,7 @@ import {
 import { WorkflowGraphTaskRunner, type WorkflowGraphTaskRunResult } from "./workflow-graph-task-runner.js";
 import { ensureWorkflowCompletionSummary } from "./workflow-completion-summary.js";
 import { createCodeNodeRunner } from "./code-node-runner.js";
+import { resolveReviewCheckoutCwd } from "./review-checkout.js";
 import { getActiveNotificationService } from "./notifier.js";
 import type { ParseStepsHandlerDeps, CodeNodeRunner } from "./workflow-node-handlers.js";
 import type { WorkflowBranchPersistence, WorkflowBranchRunState } from "./workflow-graph-branches.js";
@@ -6135,6 +6136,7 @@ export class TaskExecutor {
         const detail = await this.store.getTask(seamTask.id);
         // Worktree isolation (KTD-11): review the instance's OWN worktree when set.
         const worktreePath = active.worktreePath || detail.worktree || this.rootDir;
+        const reviewCwd = resolveReviewCheckoutCwd(detail, worktreePath);
         const stepName = detail.steps[stepIndex]?.name ?? `Step ${stepIndex}`;
         const promptContent = detail.prompt ?? "";
         // Merge per-task effective workflow settings (U3, KTD-3) so the validator
@@ -6190,9 +6192,9 @@ export class TaskExecutor {
           return sem ? sem.runNested(invoke) : invoke();
         };
         const invokeReviewer = () =>
-          this.workspaceConfig
+          this.workspaceConfig && reviewCwd === worktreePath
             ? this.reviewWorkspacePerRepo(detail, (cwd) => runForCwd(cwd))
-            : runForCwd(worktreePath);
+            : runForCwd(reviewCwd);
 
         let review: { verdict: ReviewVerdict; review: string; summary: string };
         try {
@@ -12634,6 +12636,7 @@ export class TaskExecutor {
           const latestDetailForReview = await store.getTask(taskId);
           const userComments = selectUserCommentsForAgentContext(latestDetailForReview);
           const settings = await mergeEffectiveSettings(store, latestDetailForReview, await store.getSettings());
+          const reviewCwd = resolveReviewCheckoutCwd(latestDetailForReview, worktreePath);
           // Run the reviewer via semaphore.runNested so its slot accounting
           // is honest: activeCount transiently bumps to reflect the second
           // agent session, but the reviewer doesn't enter the wait queue
@@ -12693,9 +12696,9 @@ export class TaskExecutor {
             const invoke = () => invokeReviewerForCwd(cwd);
             return sem ? sem.runNested(invoke) : invoke();
           };
-          const result = this.workspaceConfig
+          const result = this.workspaceConfig && reviewCwd === worktreePath
             ? await this.reviewWorkspacePerRepo(currentTask, (cwd) => runForCwd(cwd))
-            : await runForCwd(worktreePath);
+            : await runForCwd(reviewCwd);
 
           await store.logEntry(
             taskId,

packages/engine/src/review-checkout.ts

@@ -0,0 +1,40 @@
+import { execFileSync } from "node:child_process";
+import { existsSync, lstatSync, realpathSync } from "node:fs";
+import { isAbsolute } from "node:path";
+
+function readMetadataPath(value: unknown): string | undefined {
+  if (!value || typeof value !== "object") return undefined;
+  const record = value as Record<string, unknown>;
+  const direct = record.reviewCheckoutPath ?? record.externalReviewCheckoutPath;
+  if (typeof direct === "string" && direct.trim()) return direct.trim();
+  const nested = record.reviewCheckout;
+  if (nested && typeof nested === "object") {
+    const path = (nested as Record<string, unknown>).path;
+    if (typeof path === "string" && path.trim()) return path.trim();
+  }
+  return undefined;
+}
+
+export function getTaskReviewCheckoutPath(task: unknown): string | undefined {
+  if (!task || typeof task !== "object") return undefined;
+  const record = task as Record<string, unknown>;
+  return readMetadataPath(record.customFields) ?? readMetadataPath(record.branchContext) ?? readMetadataPath(record);
+}
+
+export function resolveReviewCheckoutCwd(task: unknown, fallbackCwd: string): string {
+  const candidate = getTaskReviewCheckoutPath(task);
+  if (!candidate || !isAbsolute(candidate)) return fallbackCwd;
+  try {
+    if (!existsSync(candidate) || !lstatSync(candidate).isDirectory()) return fallbackCwd;
+    const realCandidate = realpathSync(candidate);
+    const topLevel = execFileSync("git", ["rev-parse", "--show-toplevel"], {
+      cwd: realCandidate,
+      encoding: "utf8",
+      stdio: ["ignore", "pipe", "ignore"],
+    }).trim();
+    if (!topLevel) return fallbackCwd;
+    return realpathSync(topLevel);
+  } catch {
+    return fallbackCwd;
+  }
+}