From 06b863ed2f69d15e750bc4476a93f37c434e5bb5 Mon Sep 17 00:00:00 2001 From: Phil Larson Date: Mon, 29 Jun 2026 14:27:58 -0700 Subject: [PATCH] fix: address external review checkout review feedback --- .../src/__tests__/reviewer-workspace.test.ts | 6 +++--- packages/engine/src/review-checkout.ts | 14 ++++++++++---- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/packages/engine/src/__tests__/reviewer-workspace.test.ts b/packages/engine/src/__tests__/reviewer-workspace.test.ts index 03f00163aa..c0badffda7 100644 --- a/packages/engine/src/__tests__/reviewer-workspace.test.ts +++ b/packages/engine/src/__tests__/reviewer-workspace.test.ts @@ -41,7 +41,7 @@ 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" }); + execFileSync("git", ["init"], { cwd: dir, stdio: "ignore" }); return dir; } @@ -235,7 +235,7 @@ describe("U2 KTD3 — in-session fn_review_step (createReviewStepTool) loops per 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 seen = scriptReviewByCwd({ [expectedCheckout]: { verdict: "APPROVE", review: "external ok", summary: "external" } }); const tool = (executor as any).createReviewStepTool( task.id, WT_A, @@ -288,7 +288,7 @@ describe("U2 KTD3 — step-inversion review seam (executor.ts:5668) loops per su 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 seen = scriptReviewByCwd({ [expectedCheckout]: { 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); diff --git a/packages/engine/src/review-checkout.ts b/packages/engine/src/review-checkout.ts index 172cc42ceb..344950545c 100644 --- a/packages/engine/src/review-checkout.ts +++ b/packages/engine/src/review-checkout.ts @@ -1,12 +1,18 @@ import { execFileSync } from "node:child_process"; -import { existsSync, lstatSync, realpathSync } from "node:fs"; +import { existsSync, realpathSync, statSync } 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; - const direct = record.reviewCheckoutPath ?? record.externalReviewCheckoutPath; - if (typeof direct === "string" && direct.trim()) return direct.trim(); + /* + FNXC:ReviewCheckout 2026-06-29-14:05: + Explicit external review checkout metadata must survive legacy empty reviewCheckoutPath fields and symlinked checkout directories. + Treat blank/non-string direct metadata as absent before falling back, then verify the resolved target directory so external worktrees mounted via symlinks can still be reviewed. + */ + for (const direct of [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).path; @@ -25,7 +31,7 @@ export function resolveReviewCheckoutCwd(task: unknown, fallbackCwd: string): st const candidate = getTaskReviewCheckoutPath(task); if (!candidate || !isAbsolute(candidate)) return fallbackCwd; try { - if (!existsSync(candidate) || !lstatSync(candidate).isDirectory()) return fallbackCwd; + if (!existsSync(candidate) || !statSync(candidate).isDirectory()) return fallbackCwd; const realCandidate = realpathSync(candidate); const topLevel = execFileSync("git", ["rev-parse", "--show-toplevel"], { cwd: realCandidate,