fix(engine): route review via explicit external checkout metadata (#1865)

## Summary
- Add explicit `sourceMetadata.externalReviewCheckout` review routing
with fail-closed validation.
- Log the selected review checkout so operator/runtime tasks can verify
where review ran.
- Cover metadata extraction, invalid metadata fallback,
`fn_review_step`, and workflow `stepReview` routing.

## Test Plan
- `cd packages/engine && corepack pnpm exec vitest run
src/__tests__/review-checkout.test.ts
src/__tests__/reviewer-workspace.test.ts`


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

* **New Features**
* Review routing now supports explicit external checkout overrides via
review checkout metadata, using the resolved external checkout as the
review working directory when valid.
* Logging now highlights the selected review routing target and warns
when external checkout metadata is present but invalid.

* **Bug Fixes**
* Missing/blank/invalid/relative/nonexistent/non-git paths now fail
closed to the task worktree instead of falling back.
* Metadata priority is enforced strictly (custom fields → branch context
→ source metadata → root), preventing lower-priority fallback when
higher-priority data is invalid.

* **Tests**
* Added/expanded automated coverage for override and fail-closed
behavior, including workspace-mode routing semantics.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
gsxdsm
2026-07-01 17:02:30 -07:00
committed by GitHub
9 changed files with 470 additions and 7 deletions

View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Enforce explicit external checkout metadata for review routing.
category: fix
dev: Reviews now use sourceMetadata.externalReviewCheckout only when it points at a valid git checkout, otherwise they fail closed to the task worktree and log the selected review target.

View File

@@ -57,6 +57,16 @@ vi.mock("@fusion/dashboard", () => ({
vi.mock("@fusion/engine", () => ({
createFnAgent: vi.fn(),
fetchWebContent: vi.fn(),
emitGoalRetrievalAudit: vi.fn(),
createWorkflowAuthoringTools: vi.fn(() => ({})),
workflowListParams: {},
workflowGetParams: {},
workflowSelectParams: {},
workflowCreateParams: {},
workflowUpdateParams: {},
workflowDeleteParams: {},
workflowSettingsParams: {},
traitListParams: {},
defaultGitOps: vi.fn(() => ({})),
ExperimentFinalizeService: makeConstructibleMock(() => ({ previewPlan: previewPlanMock, finalize: finalizeMock })),
ExperimentFinalizeStateError: mockErrors.StateError,

View File

@@ -13,6 +13,16 @@ vi.mock("@fusion/engine", () => ({
createFnAgent: vi.fn(),
fetchWebContent: vi.fn(),
assertNoSecretPlaintext: assertNoSecretPlaintextMock,
emitGoalRetrievalAudit: vi.fn(),
createWorkflowAuthoringTools: vi.fn(() => ({})),
workflowListParams: {},
workflowGetParams: {},
workflowSelectParams: {},
workflowCreateParams: {},
workflowUpdateParams: {},
workflowDeleteParams: {},
workflowSettingsParams: {},
traitListParams: {},
}));
vi.mock("@fusion/core", async () => {

View File

@@ -31,6 +31,16 @@ vi.mock("@fusion/engine", () => ({
createFnAgent: vi.fn(),
fetchWebContent: vi.fn(),
assertNoSecretPlaintext: vi.fn(),
emitGoalRetrievalAudit: vi.fn(),
createWorkflowAuthoringTools: vi.fn(() => ({})),
workflowListParams: {},
workflowGetParams: {},
workflowSelectParams: {},
workflowCreateParams: {},
workflowUpdateParams: {},
workflowDeleteParams: {},
workflowSettingsParams: {},
traitListParams: {},
}));
async function loadExtension() {

View File

@@ -9,6 +9,16 @@ vi.mock("@fusion/dashboard", () => ({
vi.mock("@fusion/engine", () => ({
createFnAgent: vi.fn(),
fetchWebContent: fetchWebContentMock,
emitGoalRetrievalAudit: vi.fn(),
createWorkflowAuthoringTools: vi.fn(() => ({})),
workflowListParams: {},
workflowGetParams: {},
workflowSelectParams: {},
workflowCreateParams: {},
workflowUpdateParams: {},
workflowDeleteParams: {},
workflowSettingsParams: {},
traitListParams: {},
}));
import kbExtension from "../extension.js";

View File

@@ -0,0 +1,261 @@
/*
FNXC:ReviewRouting 2026-07-01-16:36:
External review checkout contract tests pin fail-closed resolution: absent, blank, relative, or non-git metadata resolves to the task worktree fallback; valid explicit absolute git checkouts resolve to their realpath; sourceMetadata.externalReviewCheckout is the canonical external field; invalid higher-priority metadata must not silently widen to lower-priority metadata.
*/
import { describe, expect, it, beforeEach, afterEach } from "vitest";
import { execFileSync } from "node:child_process";
import { mkdtempSync, mkdirSync, writeFileSync, realpathSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { resolveReviewCheckoutCwd, getTaskReviewCheckoutPath } from "../review-checkout.js";
const FALLBACK = "/some/fallback/worktree";
const cleanupDirs: string[] = [];
function makeGitCheckout(): string {
const dir = mkdtempSync(join(tmpdir(), "review-git-checkout-"));
cleanupDirs.push(dir);
execFileSync("git", ["init"], { cwd: dir, stdio: "ignore" });
return dir;
}
function makeNonGitDir(): string {
const dir = mkdtempSync(join(tmpdir(), "review-nongit-"));
cleanupDirs.push(dir);
return dir;
}
function makeRegularFile(): string {
const dir = mkdtempSync(join(tmpdir(), "review-file-"));
cleanupDirs.push(dir);
const file = join(dir, "file.txt");
writeFileSync(file, "not a directory");
return file;
}
beforeEach(() => {
cleanupDirs.length = 0;
});
afterEach(() => {
while (cleanupDirs.length > 0) {
const dir = cleanupDirs.pop();
if (dir) rmSync(dir, { recursive: true, force: true });
}
});
describe("getTaskReviewCheckoutPath — extracts metadata path candidates", () => {
it("returns undefined for null/undefined task", () => {
expect(getTaskReviewCheckoutPath(null)).toBeUndefined();
expect(getTaskReviewCheckoutPath(undefined)).toBeUndefined();
});
it("returns undefined when no metadata fields are present", () => {
expect(getTaskReviewCheckoutPath({})).toBeUndefined();
expect(getTaskReviewCheckoutPath({ id: "TASK-1", title: "T" })).toBeUndefined();
});
it("returns undefined for blank/whitespace-only reviewCheckoutPath", () => {
expect(getTaskReviewCheckoutPath({ customFields: { reviewCheckoutPath: "" } })).toBeUndefined();
expect(getTaskReviewCheckoutPath({ customFields: { reviewCheckoutPath: " " } })).toBeUndefined();
expect(getTaskReviewCheckoutPath({ customFields: { externalReviewCheckoutPath: "" } })).toBeUndefined();
});
it("returns undefined for non-string reviewCheckoutPath", () => {
expect(getTaskReviewCheckoutPath({ customFields: { reviewCheckoutPath: 42 } })).toBeUndefined();
expect(getTaskReviewCheckoutPath({ customFields: { reviewCheckoutPath: true } })).toBeUndefined();
expect(getTaskReviewCheckoutPath({ customFields: { reviewCheckoutPath: {} } })).toBeUndefined();
});
it("reads reviewCheckoutPath from customFields", () => {
const task = { customFields: { reviewCheckoutPath: "/custom/path" } };
expect(getTaskReviewCheckoutPath(task)).toBe("/custom/path");
});
it("reads externalReviewCheckoutPath from customFields", () => {
const task = { customFields: { externalReviewCheckoutPath: "/external/path" } };
expect(getTaskReviewCheckoutPath(task)).toBe("/external/path");
});
it("reads nested reviewCheckout.path from customFields", () => {
const task = { customFields: { reviewCheckout: { path: "/nested/path" } } };
expect(getTaskReviewCheckoutPath(task)).toBe("/nested/path");
});
it("reads sourceMetadata.externalReviewCheckout as a candidate", () => {
const task = { sourceMetadata: { externalReviewCheckout: "/meta/checkout" } };
expect(getTaskReviewCheckoutPath(task)).toBe("/meta/checkout");
});
it("trims whitespace from metadata values", () => {
const task = { customFields: { reviewCheckoutPath: " /trimmed/path " } };
expect(getTaskReviewCheckoutPath(task)).toBe("/trimmed/path");
});
it("customFields takes priority over branchContext, sourceMetadata, and root", () => {
const task = {
customFields: { reviewCheckoutPath: "/custom" },
branchContext: { reviewCheckoutPath: "/branch" },
sourceMetadata: { externalReviewCheckout: "/meta" },
reviewCheckoutPath: "/root",
};
expect(getTaskReviewCheckoutPath(task)).toBe("/custom");
});
it("branchContext takes priority over sourceMetadata and root-level fields", () => {
const task = {
branchContext: { reviewCheckoutPath: "/branch" },
sourceMetadata: { externalReviewCheckout: "/meta" },
reviewCheckoutPath: "/root",
};
expect(getTaskReviewCheckoutPath(task)).toBe("/branch");
});
it("sourceMetadata takes priority over root-level fields", () => {
const task = {
sourceMetadata: { externalReviewCheckout: "/meta" },
reviewCheckoutPath: "/root",
};
expect(getTaskReviewCheckoutPath(task)).toBe("/meta");
});
});
describe("resolveReviewCheckoutCwd — fail-closed defaults", () => {
it("returns fallback when task has no metadata", () => {
expect(resolveReviewCheckoutCwd({}, FALLBACK)).toBe(FALLBACK);
});
it("returns fallback for null/undefined task", () => {
expect(resolveReviewCheckoutCwd(null, FALLBACK)).toBe(FALLBACK);
expect(resolveReviewCheckoutCwd(undefined, FALLBACK)).toBe(FALLBACK);
});
it("returns fallback when reviewCheckoutPath is blank", () => {
const task = { customFields: { reviewCheckoutPath: "" } };
expect(resolveReviewCheckoutCwd(task, FALLBACK)).toBe(FALLBACK);
});
it("returns fallback for relative path (not absolute)", () => {
const task = { customFields: { reviewCheckoutPath: "relative/path" } };
expect(resolveReviewCheckoutCwd(task, FALLBACK)).toBe(FALLBACK);
});
it("returns fallback for non-existent path", () => {
const task = { customFields: { reviewCheckoutPath: "/nonexistent/path/that/does/not/exist" } };
expect(resolveReviewCheckoutCwd(task, FALLBACK)).toBe(FALLBACK);
});
it("returns fallback for path that is a regular file, not a directory", () => {
const file = makeRegularFile();
const task = { customFields: { reviewCheckoutPath: file } };
expect(resolveReviewCheckoutCwd(task, FALLBACK)).toBe(FALLBACK);
});
it("returns fallback for non-git directory", () => {
const dir = makeNonGitDir();
const task = { customFields: { reviewCheckoutPath: dir } };
expect(resolveReviewCheckoutCwd(task, FALLBACK)).toBe(FALLBACK);
});
it("returns resolved realpath for valid git checkout", () => {
const checkout = makeGitCheckout();
const expected = realpathSync(checkout);
const task = { customFields: { reviewCheckoutPath: checkout } };
expect(resolveReviewCheckoutCwd(task, FALLBACK)).toBe(expected);
});
it("resolves sourceMetadata.externalReviewCheckout for valid git checkout", () => {
const checkout = makeGitCheckout();
const expected = realpathSync(checkout);
const task = { sourceMetadata: { externalReviewCheckout: checkout } };
expect(resolveReviewCheckoutCwd(task, FALLBACK)).toBe(expected);
});
it("does not infer external checkout from prompt text or task description", () => {
const task = {
description: "Please review changes in /tmp/external-runtime",
prompt: "Look at /tmp/some-checkout",
};
expect(resolveReviewCheckoutCwd(task, FALLBACK)).toBe(FALLBACK);
});
it("conflicting metadata between customFields and sourceMetadata: customFields wins when valid", () => {
const checkoutA = makeGitCheckout();
const checkoutB = makeGitCheckout();
const expectedA = realpathSync(checkoutA);
const task = {
customFields: { reviewCheckoutPath: checkoutA },
sourceMetadata: { externalReviewCheckout: checkoutB },
};
expect(resolveReviewCheckoutCwd(task, FALLBACK)).toBe(expectedA);
});
it("valid customFields + invalid sourceMetadata: uses customFields", () => {
const checkout = makeGitCheckout();
const expected = realpathSync(checkout);
const task = {
customFields: { reviewCheckoutPath: checkout },
sourceMetadata: { externalReviewCheckout: "/nonexistent" },
};
expect(resolveReviewCheckoutCwd(task, FALLBACK)).toBe(expected);
});
// Priority-based resolution means the first candidate from the highest-priority
// source (customFields > branchContext > sourceMetadata > root) is the only
// candidate validated. When that candidate is invalid, the resolver fails
// closed to the fallback instead of trying lower-priority sources. That keeps a
// stale high-priority value from being bypassed by a coincidentally valid lower
// priority path.
it("invalid customFields → falls back even when sourceMetadata has a valid checkout (fail-closed priority)", () => {
const checkout = makeGitCheckout();
const task = {
customFields: { reviewCheckoutPath: "/nonexistent" },
sourceMetadata: { externalReviewCheckout: checkout },
};
// customFields wins priority; /nonexistent fails validation → fallback
expect(resolveReviewCheckoutCwd(task, FALLBACK)).toBe(FALLBACK);
});
it("workspace-mode task without explicit metadata: returns fallback (task worktree)", () => {
const task = {
workspaceWorktrees: {
"repo-a": { worktreePath: "/tmp/ws/repo-a/.worktrees/task-1" },
},
};
expect(resolveReviewCheckoutCwd(task, FALLBACK)).toBe(FALLBACK);
});
});
describe("resolveReviewCheckoutCwd — does NOT fabricate approval or widen scope", () => {
it("metadata pointing to a parent directory of the fallback: still validates as git dir", () => {
const parent = makeGitCheckout();
const task = { customFields: { reviewCheckoutPath: parent } };
const result = resolveReviewCheckoutCwd(task, FALLBACK);
expect(result).toBe(realpathSync(parent));
expect(result).not.toBe(FALLBACK);
});
it("empty sourceMetadata object: returns fallback", () => {
const task = { sourceMetadata: {} };
expect(resolveReviewCheckoutCwd(task, FALLBACK)).toBe(FALLBACK);
});
it("sourceMetadata with unrelated fields only: returns fallback", () => {
const task = { sourceMetadata: { fileScope: ["src/**"], contentFingerprint: "abc" } };
expect(resolveReviewCheckoutCwd(task, FALLBACK)).toBe(FALLBACK);
});
it("blank sourceMetadata.externalReviewCheckout: returns fallback", () => {
const task = { sourceMetadata: { externalReviewCheckout: "" } };
expect(resolveReviewCheckoutCwd(task, FALLBACK)).toBe(FALLBACK);
});
it("non-string sourceMetadata.externalReviewCheckout: returns fallback", () => {
const task = { sourceMetadata: { externalReviewCheckout: 42 } };
expect(resolveReviewCheckoutCwd(task, FALLBACK)).toBe(FALLBACK);
});
it("relative sourceMetadata.externalReviewCheckout: returns fallback", () => {
const task = { sourceMetadata: { externalReviewCheckout: "relative/path" } };
expect(resolveReviewCheckoutCwd(task, FALLBACK)).toBe(FALLBACK);
});
});

View File

@@ -229,7 +229,7 @@ describe("U2 KTD3 — in-session fn_review_step (createReviewStepTool) loops per
expect(seen).toEqual([WT_A]);
});
it("explicit external review checkout overrides the Atlas task worktree for fn_review_step", async () => {
it("explicit external review checkout overrides the task worktree for fn_review_step", async () => {
const externalCheckout = makeGitCheckout();
const expectedCheckout = realpathSync(externalCheckout);
const task = makeTask({ customFields: { reviewCheckoutPath: externalCheckout } } as any);
@@ -334,3 +334,137 @@ describe("U2 KTD3 — step-inversion review seam (executor.ts:5668) loops per su
expect(seen).toEqual([expectedCheckout]);
});
});
describe("sourceMetadata.externalReviewCheckout for fn_review_step", () => {
it("sourceMetadata.externalReviewCheckout overrides the task worktree for fn_review_step", async () => {
const externalCheckout = makeGitCheckout();
const expectedCheckout = realpathSync(externalCheckout);
const task = makeTask({ sourceMetadata: { externalReviewCheckout: externalCheckout } } as any);
const store = makeStore(task);
const executor = new TaskExecutor(store, ROOT);
const seen = scriptReviewByCwd({ [expectedCheckout]: { 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]);
});
it("no metadata → fn_review_step reviews the task worktree (default fallback)", async () => {
const task = makeTask(); // no customFields, no sourceMetadata
const store = makeStore(task);
const executor = new TaskExecutor(store, ROOT);
const seen = scriptReviewByCwd({ [WT_A]: { verdict: "APPROVE", review: "ok", summary: "ok" } });
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([WT_A]);
});
it("invalid external metadata (non-existent path) → falls back to task worktree", async () => {
const task = makeTask({ sourceMetadata: { externalReviewCheckout: "/nonexistent/path/918" } } as any);
const store = makeStore(task);
const executor = new TaskExecutor(store, ROOT);
const seen = scriptReviewByCwd({ [WT_A]: { verdict: "APPROVE", review: "ok", summary: "ok" } });
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([WT_A]);
});
it("invalid external metadata (non-git directory) → falls back to task worktree", async () => {
const nonGitDir = mkdtempSync(join(tmpdir(), "review-nongit-integ-"));
cleanupDirs.push(nonGitDir);
const task = makeTask({ sourceMetadata: { externalReviewCheckout: nonGitDir } } as any);
const store = makeStore(task);
const executor = new TaskExecutor(store, ROOT);
const seen = scriptReviewByCwd({ [WT_A]: { verdict: "APPROVE", review: "ok", summary: "ok" } });
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([WT_A]);
});
});
describe("sourceMetadata.externalReviewCheckout for workflow stepReview", () => {
it("sourceMetadata.externalReviewCheckout overrides the active graph worktree for stepReview", async () => {
const externalCheckout = makeGitCheckout();
const expectedCheckout = realpathSync(externalCheckout);
const task = makeTask({ worktree: WT_A, sourceMetadata: { externalReviewCheckout: externalCheckout } } as any);
const store = makeStore(task);
const executor = new TaskExecutor(store, ROOT);
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);
expect(seen).toEqual([expectedCheckout]);
});
it("no metadata → stepReview reviews the task worktree (default fallback)", async () => {
const task = makeTask({ worktree: WT_A });
const store = makeStore(task);
const executor = new TaskExecutor(store, ROOT);
const seen = scriptReviewByCwd({ [WT_A]: { verdict: "APPROVE", review: "a", summary: "a" } });
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([WT_A]);
});
it("invalid external metadata (non-existent path) → stepReview falls back to task worktree", async () => {
const task = makeTask({ worktree: WT_A, sourceMetadata: { externalReviewCheckout: "/nonexistent/path/918b" } } as any);
const store = makeStore(task);
const executor = new TaskExecutor(store, ROOT);
const seen = scriptReviewByCwd({ [WT_A]: { verdict: "APPROVE", review: "a", summary: "a" } });
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([WT_A]);
});
it("workspace-mode task without explicit metadata: stepReview still reviews per sub-repo", async () => {
const task = makeTask({ workspaceWorktrees: TWO_REPO_WORKTREES, worktree: ROOT });
const store = makeStore(task);
const executor = workspaceExecutor(store);
const seen = scriptReviewByCwd({
[WT_A]: { verdict: "APPROVE", review: "a", summary: "a" },
[WT_B]: { verdict: "APPROVE", review: "b", summary: "b" },
});
const seams = executor.createAuthoritativeWorkflowSeams({ autoMerge: false } as any);
const context = { [FOREACH_ACTIVE_CONTEXT_KEY]: { stepIndex: 1, worktreePath: ROOT, baselineSha: "base" } } as any;
await seams.stepReview!(task as any, context, { type: "code", advisory: true } as any);
expect(seen).toEqual([WT_A, WT_B]);
expect(seen).not.toContain(ROOT);
});
});

View File

@@ -24,7 +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 { getTaskReviewCheckoutPath, 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";
@@ -1736,6 +1736,26 @@ export class TaskExecutor {
}
}
/**
* FNXC:ReviewRouting 2026-07-01-16:36:
* Review routing must expose whether the reviewer is using an explicit external checkout or the task worktree, but the invalid-sourceMetadata warning is only valid when sourceMetadata supplied the selected candidate. Higher-priority metadata can fail closed before sourceMetadata is considered, so centralize the logging to keep both review seams consistent and avoid false invalid-path warnings.
*/
private logReviewCheckoutRouting(taskId: string, task: unknown, reviewCwd: string, worktreePath: string): void {
if (reviewCwd !== worktreePath) {
reviewerLog.log(`${taskId}: review routed to external checkout ${reviewCwd} (task worktree: ${worktreePath})`);
return;
}
const selectedCandidate = getTaskReviewCheckoutPath(task);
const sourceMetadata = task && typeof task === "object" ? (task as Record<string, unknown>).sourceMetadata : undefined;
const sourceRecord = sourceMetadata && typeof sourceMetadata === "object" ? sourceMetadata as Record<string, unknown> : undefined;
const sourceExternalReviewCheckout = sourceRecord?.externalReviewCheckout;
const sourceExternalReviewCheckoutPath = typeof sourceExternalReviewCheckout === "string" ? sourceExternalReviewCheckout.trim() : undefined;
if (sourceExternalReviewCheckoutPath && selectedCandidate === sourceExternalReviewCheckoutPath) {
reviewerLog.warn(`${taskId}: external review checkout metadata present (${sourceExternalReviewCheckoutPath}) but invalid — reviewing task worktree ${worktreePath}`);
}
}
private markCompletionFinalized(taskId: string): void {
this.markPausedAborted(taskId, "completion-finalize", "completion-finalize");
this.completionFinalizedTaskIds.add(taskId);
@@ -6363,6 +6383,7 @@ export class TaskExecutor {
// 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);
this.logReviewCheckoutRouting(seamTask.id, detail, reviewCwd, worktreePath);
const stepName = detail.steps[stepIndex]?.name ?? `Step ${stepIndex}`;
const promptContent = detail.prompt ?? "";
const userComments = selectUserCommentsForAgentContext(detail, { limit: null });
@@ -12979,6 +13000,7 @@ export class TaskExecutor {
const userComments = selectUserCommentsForAgentContext(latestDetailForReview, { limit: null });
const settings = await mergeEffectiveSettings(store, latestDetailForReview, await store.getSettings());
const reviewCwd = resolveReviewCheckoutCwd(latestDetailForReview, worktreePath);
this.logReviewCheckoutRouting(taskId, latestDetailForReview, reviewCwd, 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

View File

@@ -6,11 +6,10 @@ function readMetadataPath(value: unknown): string | undefined {
if (!value || typeof value !== "object") return undefined;
const record = value as Record<string, unknown>;
/*
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.
FNXC:ReviewRouting 2026-07-01-16:36:
External review routing must be explicit metadata, not inferred from prompt text or task descriptions. The resolver only accepts known metadata fields, treats blank/non-string values as absent, and later validates that the chosen path is an absolute git checkout. Source priority is fixed (customFields > branchContext > sourceMetadata > root); an invalid higher-priority candidate fails closed to the task worktree rather than silently falling through to a lower-priority path.
*/
for (const direct of [record.reviewCheckoutPath, record.externalReviewCheckoutPath]) {
for (const direct of [record.reviewCheckoutPath, record.externalReviewCheckoutPath, record.externalReviewCheckout]) {
if (typeof direct === "string" && direct.trim()) return direct.trim();
}
const nested = record.reviewCheckout;
@@ -24,7 +23,7 @@ function readMetadataPath(value: unknown): string | 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);
return readMetadataPath(record.customFields) ?? readMetadataPath(record.branchContext) ?? readMetadataPath(record.sourceMetadata) ?? readMetadataPath(record);
}
export function resolveReviewCheckoutCwd(task: unknown, fallbackCwd: string): string {