feat(workspace): Phase B U2 — per-repo review (both sites) + fn_task_done verify + scope-leak

In workspace mode both review entry points and the completion guards now iterate
every acquired sub-repo. A shared reviewWorkspacePerRepo loops task.workspaceWorktrees
and invokes the existing single-cwd reviewStep once per repo (cwd = the sub-repo —
the reviewer agent runs its own git diff there), aggregating repo-tagged verdicts
as a conjunction: the task is reviewed only if every repo APPROVEs; the first
non-APPROVE repo's verdict becomes the aggregate. Both call sites loop — the
in-session fn_review_step tool AND the step-inversion seam (createReviewStepTool
and the stepReview workflow seam) — so no review surface silently scopes to the
non-git root (FN-5893). reviewStep itself stays single-cwd; the callers loop.

fn_task_done completion verification iterates per repo: verifyWorktreeInvariants
(from U1) already covers all worktrees, and evaluateTaskDoneScopeLeak now loops
each sub-repo (cwd + repo.baseCommitSha, repo-prefixed touched files vs the
repo-prefixed declared File Scope), blocking on the first repo with off-scope
files and naming it. Both return shapes preserved (ReviewResult; {blocked,message}).

New workspace-paths.ts repo-prefix helper (deriveRepoForPath/splitRepoScopedPath/
deriveRepoScopeSubset; segment-wise longest-prefix match, unscoped fallback) —
master U5 reuses it. Singular non-workspace path unchanged. 16 new fixture tests.
Gate green: typecheck, lint, build, test:gate (649+58).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-06-21 22:44:40 -07:00
parent fc9423e465
commit 81edbeefbd
5 changed files with 708 additions and 27 deletions

View File

@@ -0,0 +1,5 @@
---
"@runfusion/fusion": minor
---
Workspace mode (Phase B, U2): per-repo review at both review entry points plus per-repo `fn_task_done` completion + scope-leak verification. In workspace mode both review call sites (the in-session `fn_review_step` tool and the step-inversion review seam) now loop the single-cwd `reviewStep` once per acquired sub-repo (cwd = each repo's worktree) and aggregate the repo-tagged verdicts as a conjunction — the task is reviewed only when every sub-repo approves, and the first failing sub-repo's verdict (with repo-tagged findings) drives the existing verdict→edge mapping. `fn_task_done` now verifies worktree invariants per acquired repo and iterates the scope-leak guard per sub-repo (cwd = repo worktree, repo `baseCommitSha`), blocking completion on any sub-repo carrying off-scope changes and naming the repo. Adds a minimal shared repo-prefix-derivation helper (`workspace-paths.ts`). Single-repo behavior is unchanged.

View File

@@ -0,0 +1,220 @@
/*
FNXC:Workspace 2026-06-22-00:30:
U2 KTD4 — per-repo fn_task_done completion verification: per-repo scope-leak guard + per-repo worktree-invariant
verify. These drive the REAL TaskExecutor methods against a REAL two-repo git fixture under a NON-git workspace
root (createWorkspaceFixture), so a leaked singular-root capture/verify would silently pass and the test would
catch it. Narrow seams (FN-5048): we set `(executor as any).workspaceConfig` directly and stub only the store
methods the guards read (parseFileScopeFromPrompt, logEntry, getRunContextFor) — no mock-the-world child_process.
Coverage:
- scope-leak error: an uncommitted in-scope vs OFF-scope change in repo A → evaluateTaskDoneScopeLeak blocks,
message NAMES repo-a (per-repo guard fires; singular root would silently pass).
- verify error: a worktree HEAD off fusion/<id> → verifyWorktreeInvariants blocks (wrong_branch, repo-tagged).
- all-clean: a two-repo task with only in-scope changes → scope-leak does NOT block.
- helper: deriveRepoForPath / deriveRepoScopeSubset / splitRepoScopedPath unit cases (wolf-server/src/** → wolf-server;
non-matching first segment → unscoped).
- regression: single-repo (non-workspace) task → singular scope-leak path unchanged.
*/
import { afterEach, describe, expect, it, vi } from "vitest";
import { EventEmitter } from "node:events";
import { execSync } from "node:child_process";
import { mkdirSync, writeFileSync } from "node:fs";
import path from "node:path";
import type { Task, TaskStore, WorkspaceConfig, Settings } from "@fusion/core";
import { TaskExecutor } from "../executor.js";
import {
deriveRepoForPath,
deriveRepoScopeSubset,
splitRepoScopedPath,
UNSCOPED_REPO,
} from "../workspace-paths.js";
import { createWorkspaceFixture, hasGit, type WorkspaceFixture } from "./_workspace-fixture.js";
const describeIfGit = hasGit ? describe : describe.skip;
const TASK_ID = "FN-1001";
const BRANCH = "fusion/fn-1001";
// reviewLevel=1 + block enforcement is the only mode that BLOCKS (else warn).
const SETTINGS: Settings = { autoMerge: false, planOnlyScopeLeakEnforcement: "block" } as Settings;
const PROMPT = "## Review Level: 1 (Plan Only)\n";
function createStore(declaredScope: string[]): TaskStore & EventEmitter {
const emitter = new EventEmitter();
return Object.assign(emitter, {
parseFileScopeFromPrompt: vi.fn().mockResolvedValue(declaredScope),
logEntry: vi.fn().mockResolvedValue(undefined),
getRunContextFor: vi.fn(),
getSettings: vi.fn().mockResolvedValue(SETTINGS),
}) as unknown as TaskStore & EventEmitter;
}
function makeTask(overrides: Partial<Task> = {}): Task {
return {
id: TASK_ID,
title: "WS",
description: "",
column: "in-progress",
dependencies: [],
steps: [],
currentStep: 0,
log: [],
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
...overrides,
} as Task;
}
function configureIdentity(dir: string): void {
execSync('git config user.email "test@example.com"', { cwd: dir, stdio: "pipe" });
execSync('git config user.name "Test"', { cwd: dir, stdio: "pipe" });
}
/** Add a fusion/<id> worktree to a sub-repo with one committed in-scope edit; return its handle. */
function addRepoWorktree(fx: WorkspaceFixture, repoRel: string, fileName: string): { worktreePath: string; baseCommitSha: string } {
const repoDir = fx.repoPath(repoRel);
const baseCommitSha = fx.git(repoRel, "git rev-parse HEAD");
const worktreePath = path.join(repoDir, ".worktrees", "fn-ws-1");
fx.git(repoRel, `git worktree add -b ${BRANCH} ${worktreePath} HEAD`);
configureIdentity(worktreePath);
mkdirSync(path.dirname(path.join(worktreePath, fileName)), { recursive: true });
writeFileSync(path.join(worktreePath, fileName), "// in-scope\n", "utf-8");
execSync(`git add ${fileName}`, { cwd: worktreePath, stdio: "pipe" });
execSync(`git commit -m "feat(${TASK_ID}): edit ${fileName}"`, { cwd: worktreePath, stdio: "pipe" });
return { worktreePath, baseCommitSha };
}
function workspaceExecutor(fx: WorkspaceFixture, store: TaskStore & EventEmitter): TaskExecutor {
const executor = new TaskExecutor(store, fx.rootDir);
(executor as any).workspaceConfig = { repos: fx.repos } as WorkspaceConfig;
return executor;
}
describe("U2 — workspace-paths repo-prefix helper (unit)", () => {
const repos = ["wolf-server", "repo-a", "apps/web"];
it("deriveRepoForPath: first-segment match → that repo", () => {
expect(deriveRepoForPath("wolf-server/src/index.ts", repos)).toBe("wolf-server");
expect(deriveRepoForPath("repo-a/src/a.ts", repos)).toBe("repo-a");
});
it("deriveRepoForPath: longest nested-key match wins", () => {
expect(deriveRepoForPath("apps/web/page.tsx", repos)).toBe("apps/web");
});
it("deriveRepoForPath: non-matching first segment → unscoped", () => {
expect(deriveRepoForPath(".changeset/x.md", repos)).toBe(UNSCOPED_REPO);
expect(deriveRepoForPath("other/thing.ts", repos)).toBe(UNSCOPED_REPO);
expect(deriveRepoForPath("repo-ab/x.ts", repos)).toBe(UNSCOPED_REPO); // segment-wise, not substring
});
it("splitRepoScopedPath: strips the repo prefix for the repo-local remainder", () => {
expect(splitRepoScopedPath("wolf-server/src/x.ts", repos)).toEqual({ repo: "wolf-server", relativePath: "src/x.ts" });
expect(splitRepoScopedPath("other/x.ts", repos)).toEqual({ repo: UNSCOPED_REPO, relativePath: "other/x.ts" });
});
it("deriveRepoScopeSubset: returns repo-local scope patterns for one repo", () => {
const scope = ["wolf-server/src/**", "repo-a/lib/x.ts", "apps/web/page.tsx"];
expect(deriveRepoScopeSubset(scope, "wolf-server")).toEqual(["src/**"]);
expect(deriveRepoScopeSubset(scope, "repo-a")).toEqual(["lib/x.ts"]);
// repo-root scope entry maps to whole-repo **
expect(deriveRepoScopeSubset(["repo-a"], "repo-a")).toEqual(["**"]);
});
});
describeIfGit("U2 KTD4 — per-repo scope-leak guard in fn_task_done", () => {
let fx: WorkspaceFixture;
afterEach(() => fx?.cleanup());
it("error: an off-scope change in repo A blocks completion and NAMES repo-a", async () => {
fx = await createWorkspaceFixture();
const a = addRepoWorktree(fx, "repo-a", "src/a.ts");
const b = addRepoWorktree(fx, "repo-b", "src/b.ts");
// Off-scope STAGED-but-uncommitted change in repo-a (outside declared `repo-a/src/**`).
// captureUncommittedModifiedFiles reads `git diff`/`--cached`, so the leak must be tracked
// (staged) to register — an untracked file is invisible to the guard by design.
writeFileSync(path.join(a.worktreePath, "OFFSCOPE.md"), "// leak\n", "utf-8");
execSync("git add OFFSCOPE.md", { cwd: a.worktreePath, stdio: "pipe" });
// Declared scope is repo-prefixed and only covers src/** in each repo.
const store = createStore(["repo-a/src/**", "repo-b/src/**"]);
const executor = workspaceExecutor(fx, store);
const task = makeTask({
branch: BRANCH,
workspaceWorktrees: {
"repo-a": { worktreePath: a.worktreePath, branch: BRANCH, baseCommitSha: a.baseCommitSha },
"repo-b": { worktreePath: b.worktreePath, branch: BRANCH, baseCommitSha: b.baseCommitSha },
},
});
const result = await (executor as any).evaluateTaskDoneScopeLeak(task, fx.rootDir, PROMPT, SETTINGS);
expect(result.blocked).toBe(true);
expect(result.message).toContain("repo-a");
expect(result.message).toContain("OFFSCOPE.md");
});
it("all-clean: only in-scope changes in both repos → not blocked", async () => {
fx = await createWorkspaceFixture();
const a = addRepoWorktree(fx, "repo-a", "src/a.ts");
const b = addRepoWorktree(fx, "repo-b", "src/b.ts");
const store = createStore(["repo-a/src/**", "repo-b/src/**"]);
const executor = workspaceExecutor(fx, store);
const task = makeTask({
branch: BRANCH,
workspaceWorktrees: {
"repo-a": { worktreePath: a.worktreePath, branch: BRANCH, baseCommitSha: a.baseCommitSha },
"repo-b": { worktreePath: b.worktreePath, branch: BRANCH, baseCommitSha: b.baseCommitSha },
},
});
const result = await (executor as any).evaluateTaskDoneScopeLeak(task, fx.rootDir, PROMPT, SETTINGS);
expect(result.blocked).toBe(false);
});
});
describeIfGit("U2 KTD4 — per-repo worktree-invariant verify in fn_task_done", () => {
let fx: WorkspaceFixture;
afterEach(() => fx?.cleanup());
it("error: a worktree off fusion/<id> blocks completion via per-repo verify", async () => {
fx = await createWorkspaceFixture();
const a = addRepoWorktree(fx, "repo-a", "src/a.ts");
const b = addRepoWorktree(fx, "repo-b", "src/b.ts");
execSync("git checkout -b drifted-branch", { cwd: b.worktreePath, stdio: "pipe" });
const store = createStore(["repo-a/src/**", "repo-b/src/**"]);
const executor = workspaceExecutor(fx, store);
const task = makeTask({
branch: BRANCH,
workspaceWorktrees: {
"repo-a": { worktreePath: a.worktreePath, branch: BRANCH, baseCommitSha: a.baseCommitSha },
"repo-b": { worktreePath: b.worktreePath, branch: BRANCH, baseCommitSha: b.baseCommitSha },
},
});
const result = await (executor as any).verifyWorktreeInvariants(task);
expect(result.ok).toBe(false);
expect(result.reason).toBe("wrong_branch");
expect(result.repo).toBe("repo-b");
});
});
describeIfGit("U2 — single-repo (non-workspace) task: scope-leak unchanged", () => {
let fx: WorkspaceFixture;
afterEach(() => fx?.cleanup());
it("regression: singular scope-leak path still flags an off-scope change in the singular worktree", async () => {
fx = await createWorkspaceFixture();
const repoDir = fx.repoPath("repo-a");
const worktreePath = path.join(repoDir, ".worktrees", "fn-001");
const base = execSync("git rev-parse HEAD", { cwd: repoDir, encoding: "utf-8" }).trim();
execSync(`git worktree add -b fusion/fn-001 ${worktreePath} HEAD`, { cwd: repoDir, stdio: "pipe" });
configureIdentity(worktreePath);
// Off-scope STAGED change (declared scope is `src/**`). Tracked so the guard sees it.
writeFileSync(path.join(worktreePath, "OFFSCOPE.md"), "// leak\n", "utf-8");
execSync("git add OFFSCOPE.md", { cwd: worktreePath, stdio: "pipe" });
const store = createStore(["src/**"]);
const executor = new TaskExecutor(store, repoDir); // no workspaceConfig → singular path
const task = makeTask({ id: "FN-001", branch: "fusion/fn-001", worktree: worktreePath, baseCommitSha: base });
const result = await (executor as any).evaluateTaskDoneScopeLeak(task, worktreePath, PROMPT, SETTINGS);
expect(result.blocked).toBe(true);
expect(result.message).toContain("OFFSCOPE.md");
// Singular message carries no repo tag.
expect(result.message).not.toContain("repo=");
});
});

View File

@@ -0,0 +1,213 @@
/*
FNXC:Workspace 2026-06-22-00:30:
U2 KTD3 — per-repo review (BOTH call sites) + conjunction aggregation tests. The reviewer is an AGENT
spawned with `cwd = worktree`; per-repo review means ONE reviewer agent per sub-repo with the CALLERS
looping the single-cwd `reviewStep`. These tests assert the LOOP + aggregation, not the reviewer's content:
`reviewStep` is mocked (the narrow AI seam — FN-5048: no mock-the-world, no real AI spawn) and we record
the cwd of each call. Coverage:
- conjunction: two-repo task → two reviewer passes (one per repo cwd); review record reflects both; reviewed
only when BOTH pass; one repo REVISE → aggregate REVISE tagged with that repo.
- finding tag: a finding in repo B is repo-tagged in the aggregated review body.
- in-session seam (createReviewStepTool / fn_review_step): a workspace task reviews each sub-repo cwd, not the root.
- step-inversion seam (createAuthoritativeWorkflowSeams().stepReview, executor.ts:5668): same — each sub-repo, not root.
- regression: single-repo (non-workspace) task → exactly one reviewStep call at the singular worktree.
*/
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { EventEmitter } from "node:events";
import type { ReviewResult } from "../reviewer.js";
// Narrow AI seam: only reviewStep (the agent boundary) is mocked. Everything else is the real executor.
vi.mock("../reviewer.js", async (importOriginal) => {
const actual = await importOriginal<typeof import("../reviewer.js")>();
return { ...actual, reviewStep: vi.fn() };
});
import { reviewStep as mockedReviewStepFn } from "../reviewer.js";
import { TaskExecutor } from "../executor.js";
import { FOREACH_ACTIVE_CONTEXT_KEY } from "../workflow-node-handlers.js";
import type { Task, TaskStore, WorkspaceConfig } from "@fusion/core";
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";
function makeStore(task: Task): TaskStore & EventEmitter {
const emitter = new EventEmitter();
return Object.assign(emitter, {
getTask: vi.fn().mockResolvedValue(task),
getSettings: vi.fn().mockResolvedValue({ autoMerge: false }),
updateStep: vi.fn().mockResolvedValue(undefined),
logEntry: vi.fn().mockResolvedValue(undefined),
getRunContextFor: vi.fn(),
// mergeEffectiveSettings degrades to base on any resolver error; these reject → base used.
getTaskWorkflowSelection: vi.fn().mockRejectedValue(new Error("no workflow")),
getWorkflowDefinition: vi.fn().mockRejectedValue(new Error("no workflow")),
getWorkflowSettingValues: vi.fn().mockRejectedValue(new Error("no workflow")),
}) as unknown as TaskStore & EventEmitter;
}
function makeTask(overrides: Partial<Task> = {}): Task {
return {
id: "FN-1",
title: "WS",
description: "",
column: "in-progress",
dependencies: [],
steps: [
{ name: "Step 0", status: "done" },
{ name: "Step 1", status: "in-progress" },
],
currentStep: 1,
log: [],
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
...overrides,
} as Task;
}
const TWO_REPO_WORKTREES = {
"repo-a": { worktreePath: WT_A, branch: "fusion/fn-1", baseCommitSha: "aaa" },
"repo-b": { worktreePath: WT_B, branch: "fusion/fn-1", baseCommitSha: "bbb" },
};
/** Script reviewStep to return a per-cwd verdict and record the cwd it was called with. */
function scriptReviewByCwd(byCwd: Record<string, ReviewResult>): string[] {
const seenCwds: string[] = [];
mockedReviewStep.mockImplementation((async (cwd: string) => {
seenCwds.push(cwd);
return byCwd[cwd] ?? { verdict: "APPROVE", review: `ok ${cwd}`, summary: `ok ${cwd}` };
}) as any);
return seenCwds;
}
function workspaceExecutor(store: TaskStore & EventEmitter): TaskExecutor {
const executor = new TaskExecutor(store, ROOT);
(executor as any).workspaceConfig = { repos: ["repo-a", "repo-b"] } as WorkspaceConfig;
return executor;
}
beforeEach(() => {
mockedReviewStep.mockReset();
});
afterEach(() => {
vi.clearAllMocks();
});
describe("U2 KTD3 — reviewWorkspacePerRepo conjunction + tagging (the shared loop both call sites use)", () => {
it("conjunction: two repos both APPROVE → aggregate APPROVE, one reviewer pass per repo cwd", async () => {
const task = makeTask({ workspaceWorktrees: TWO_REPO_WORKTREES });
const executor = workspaceExecutor(makeStore(task));
const seen: string[] = [];
const result = await (executor as any).reviewWorkspacePerRepo(task, async (cwd: string, repo: string) => {
seen.push(cwd);
return { verdict: "APPROVE", review: `clean in ${repo}`, summary: `clean ${repo}` };
});
expect(seen).toEqual([WT_A, WT_B]); // one pass per sub-repo cwd, never ROOT
expect(result.verdict).toBe("APPROVE");
expect(result.review).toContain("repo-a");
expect(result.review).toContain("repo-b");
});
it("conjunction: one repo REVISE → aggregate REVISE, tagged with the failing repo", async () => {
const task = makeTask({ workspaceWorktrees: TWO_REPO_WORKTREES });
const executor = workspaceExecutor(makeStore(task));
const result = await (executor as any).reviewWorkspacePerRepo(task, async (_cwd: string, repo: string) => {
return repo === "repo-b"
? { verdict: "REVISE", review: `bug in ${repo}`, summary: `revise ${repo}` }
: { verdict: "APPROVE", review: `clean ${repo}`, summary: `clean ${repo}` };
});
expect(result.verdict).toBe("REVISE");
expect(result.review).toContain("repo-b"); // finding repo-tagged
expect(result.review).toContain("bug in repo-b");
expect(result.summary).toMatch(/^repo-b:/);
});
it("zero-acquire workspace task → UNAVAILABLE (caller routes; no fabricated APPROVE)", async () => {
const task = makeTask({ workspaceWorktrees: {} });
const executor = workspaceExecutor(makeStore(task));
const invoke = vi.fn();
const result = await (executor as any).reviewWorkspacePerRepo(task, invoke);
expect(result.verdict).toBe("UNAVAILABLE");
expect(invoke).not.toHaveBeenCalled();
});
});
describe("U2 KTD3 — in-session fn_review_step (createReviewStepTool) loops per sub-repo", () => {
it("workspace task: code review spawns one reviewer per sub-repo cwd, not the root", async () => {
const task = makeTask({ workspaceWorktrees: TWO_REPO_WORKTREES });
const store = makeStore(task);
const executor = workspaceExecutor(store);
const seen = scriptReviewByCwd({
[WT_A]: { verdict: "APPROVE", review: "a ok", summary: "a" },
[WT_B]: { verdict: "APPROVE", review: "b ok", summary: "b" },
});
const tool = (executor as any).createReviewStepTool(
task.id,
ROOT, // singular worktreePath = the non-git root; workspace mode must NOT review it
"PROMPT",
new Map(),
{ current: null },
new Map(),
task,
undefined,
);
const res = await tool.execute("call-1", { step: 1, type: "code", step_name: "Step 1", baseline: "base" });
expect(seen).toEqual([WT_A, WT_B]);
expect(seen).not.toContain(ROOT);
// Aggregate APPROVE flows through the tool's verdict→text mapping unchanged.
expect(res.content[0].text).toBe("APPROVE");
});
it("regression: single-repo (non-workspace) task → exactly one reviewStep call at the singular worktree", async () => {
const task = makeTask();
const store = makeStore(task);
const executor = new TaskExecutor(store, ROOT); // no workspaceConfig → singular path
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("U2 KTD3 — step-inversion review seam (executor.ts:5668) loops per sub-repo", () => {
it("workspace task: stepReview spawns one reviewer per sub-repo cwd, not active.worktreePath/root", 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);
// Drive the foreach-active step-review handler directly with a scripted active context.
const context = {
[FOREACH_ACTIVE_CONTEXT_KEY]: { stepIndex: 1, worktreePath: ROOT, baselineSha: "base" },
} as any;
const result = 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);
expect(result.verdict).toBe("APPROVE");
});
it("regression: single-repo stepReview reviews the active worktree once", async () => {
const task = makeTask({ worktree: WT_A });
const store = makeStore(task);
const executor = new TaskExecutor(store, ROOT); // no workspaceConfig
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]);
});
});

View File

@@ -77,7 +77,7 @@ import {
resolveExecutorSessionModel, resolveExecutorSessionModel,
} from "./agent-session-helpers.js"; } from "./agent-session-helpers.js";
import { buildSessionSkillContext } from "./session-skill-context.js"; import { buildSessionSkillContext } from "./session-skill-context.js";
import { reviewStep, type ReviewVerdict } from "./reviewer.js"; import { reviewStep, type ReviewVerdict, type ReviewResult } from "./reviewer.js";
import { resolveSandboxBackend } from "./sandbox/index.js"; import { resolveSandboxBackend } from "./sandbox/index.js";
import type { SandboxBackend } from "./sandbox/types.js"; import type { SandboxBackend } from "./sandbox/types.js";
import { ModelRegistry, SessionManager, type ToolDefinition, type AgentSession } from "@earendil-works/pi-coding-agent"; import { ModelRegistry, SessionManager, type ToolDefinition, type AgentSession } from "@earendil-works/pi-coding-agent";
@@ -5664,9 +5664,14 @@ export class TaskExecutor {
const settings = await mergeEffectiveSettings(this.store, detail, await this.store.getSettings()); const settings = await mergeEffectiveSettings(this.store, detail, await this.store.getSettings());
const sem = this.options.semaphore; const sem = this.options.semaphore;
const invokeReviewer = () => // FNXC:Workspace 2026-06-22-00:30: KTD3 — step-inversion review seam loops per sub-repo.
// `reviewStep` stays single-cwd; THIS CALLER loops. Single-cwd by default reviews
// `worktreePath`; in workspace mode that is the browse-only non-git root, so we instead spawn
// one reviewer per acquired sub-repo (cwd = repo.worktreePath) via reviewWorkspacePerRepo and
// aggregate as a conjunction. `invokeReviewerForCwd` is the per-cwd reviewStep call both modes share.
const invokeReviewerForCwd = (cwd: string) =>
reviewStep( reviewStep(
worktreePath, cwd,
seamTask.id, seamTask.id,
stepIndex, stepIndex,
stepName, stepName,
@@ -5702,10 +5707,18 @@ export class TaskExecutor {
onSessionEnded: (s) => this.unregisterSubagentSession(seamTask.id, s), onSessionEnded: (s) => this.unregisterSubagentSession(seamTask.id, s),
}, },
); );
const runForCwd = (cwd: string) => {
const invoke = () => invokeReviewerForCwd(cwd);
return sem ? sem.runNested(invoke) : invoke();
};
const invokeReviewer = () =>
this.workspaceConfig
? this.reviewWorkspacePerRepo(detail, (cwd) => runForCwd(cwd))
: runForCwd(worktreePath);
let review: { verdict: ReviewVerdict; review: string; summary: string }; let review: { verdict: ReviewVerdict; review: string; summary: string };
try { try {
review = sem ? await sem.runNested(invokeReviewer) : await invokeReviewer(); review = await invokeReviewer();
} catch (err) { } catch (err) {
const message = err instanceof Error ? err.message : String(err); const message = err instanceof Error ? err.message : String(err);
reviewerLog.error(`${seamTask.id}: step-review failed: ${message}`); reviewerLog.error(`${seamTask.id}: step-review failed: ${message}`);
@@ -10855,24 +10868,62 @@ export class TaskExecutor {
return { blocked: false }; return { blocked: false };
} }
const [uncommittedTouchedFiles, branchCommittedFiles] = await Promise.all([ // FNXC:Workspace 2026-06-22-00:30: KTD4 — per-repo scope-leak guard.
this.captureUncommittedModifiedFiles(worktreePath), // The singular capture below runs `captureUncommittedModifiedFiles` + `captureModifiedFiles`
this.captureModifiedFiles(worktreePath, task.baseCommitSha, task.id, audit, "scope-leak-guard"), // against `worktreePath`. In workspace mode `worktreePath` is the browse-only non-git workspace
]); // root, so both silently return [] (git failures swallowed) and the uncommitted-in-scope block
// never fires — a workspace task could complete with off-scope changes in any sub-repo. So we
const touchedFiles = [...new Set([...uncommittedTouchedFiles, ...branchCommittedFiles])]; // ITERATE every acquired sub-repo (cwd = repo.worktreePath, base = repo.baseCommitSha),
if (touchedFiles.length === 0) { // repo-prefix each repo's touched files (`<repoRel>/<file>`) so they compare against the task's
return { blocked: false }; // repo-prefixed declared File Scope, and block on the FIRST repo carrying off-scope changes —
// naming the repo. The task-level preamble above (scopeOverride / declaredScope / enforcementMode)
// is shared and runs once. Return shape is preserved: `{blocked:false} | {blocked:true; message}`.
let touchedFiles: string[];
let offendingRepo: string | undefined;
if (this.workspaceConfig) {
const workspaceWorktrees = task.workspaceWorktrees ?? {};
const aggregatedOffScope: string[] = [];
for (const [repoRel, repo] of Object.entries(workspaceWorktrees)) {
const [repoUncommitted, repoCommitted] = await Promise.all([
this.captureUncommittedModifiedFiles(repo.worktreePath),
this.captureModifiedFiles(repo.worktreePath, repo.baseCommitSha, task.id, audit, "scope-leak-guard"),
]);
const repoTouched = [...new Set([...repoUncommitted, ...repoCommitted])].map((f) => `${repoRel}/${f}`);
const repoOffScope = repoTouched
.filter((filePath) => !workflowPathMatchesDeclaredScope(filePath, declaredScope))
.filter((filePath) => !isAlwaysAllowedScopeLeakPath(filePath));
if (repoOffScope.length > 0) {
// First offending repo wins (mirrors verifyWorktreeInvariants' first-failing-repo return).
if (!offendingRepo) offendingRepo = repoRel;
aggregatedOffScope.push(...repoOffScope);
}
}
touchedFiles = aggregatedOffScope;
if (touchedFiles.length === 0) {
return { blocked: false };
}
} else {
const [uncommittedTouchedFiles, branchCommittedFiles] = await Promise.all([
this.captureUncommittedModifiedFiles(worktreePath),
this.captureModifiedFiles(worktreePath, task.baseCommitSha, task.id, audit, "scope-leak-guard"),
]);
touchedFiles = [...new Set([...uncommittedTouchedFiles, ...branchCommittedFiles])];
if (touchedFiles.length === 0) {
return { blocked: false };
}
} }
const offScopeFiles = touchedFiles const offScopeFiles = (this.workspaceConfig
.filter((filePath) => !workflowPathMatchesDeclaredScope(filePath, declaredScope)) // In workspace mode `touchedFiles` is already the off-scope set (filtered per repo above).
// FN-4811 follow-up: by convention every task may add its own changeset entry ? touchedFiles
// under `.changeset/`, so changeset files are always considered in-scope and : touchedFiles
// never flagged by the scope-leak guard. The file-scope invariant at squash and .filter((filePath) => !workflowPathMatchesDeclaredScope(filePath, declaredScope))
// the broader contamination guards still catch cross-task changeset leakage at // FN-4811 follow-up: by convention every task may add its own changeset entry
// a higher signal-to-noise ratio than the per-execution scope-leak warning. // under `.changeset/`, so changeset files are always considered in-scope and
.filter((filePath) => !isAlwaysAllowedScopeLeakPath(filePath)); // never flagged by the scope-leak guard. The file-scope invariant at squash and
// the broader contamination guards still catch cross-task changeset leakage at
// a higher signal-to-noise ratio than the per-execution scope-leak warning.
.filter((filePath) => !isAlwaysAllowedScopeLeakPath(filePath)));
if (offScopeFiles.length === 0) { if (offScopeFiles.length === 0) {
return { blocked: false }; return { blocked: false };
} }
@@ -10887,14 +10938,16 @@ export class TaskExecutor {
const offScopePreview = renderListPreview(offScopeFiles); const offScopePreview = renderListPreview(offScopeFiles);
const declaredScopePreview = renderListPreview(declaredScope); const declaredScopePreview = renderListPreview(declaredScope);
const message = `[scope-leak] reviewLevel=${reviewLevel} enforcement=${enforcementMode} off-scope touched files [${offScopePreview}]; declared scope [${declaredScopePreview}]; total off-scope=${offScopeFiles.length} total scope=${declaredScope.length}`; // Name the offending sub-repo in workspace mode so the operator/agent knows where to revert.
const repoTag = offendingRepo ? ` repo=${offendingRepo}` : "";
const message = `[scope-leak] reviewLevel=${reviewLevel} enforcement=${enforcementMode}${repoTag} off-scope touched files [${offScopePreview}]; declared scope [${declaredScopePreview}]; total off-scope=${offScopeFiles.length} total scope=${declaredScope.length}`;
executorLog.warn(`${task.id}: ${message}`); executorLog.warn(`${task.id}: ${message}`);
await this.store.logEntry(task.id, message, undefined, this.getRunContextFor(task.id)); await this.store.logEntry(task.id, message, undefined, this.getRunContextFor(task.id));
if (enforcementMode === "block") { if (enforcementMode === "block") {
return { return {
blocked: true, blocked: true,
message: `Plan-Only scope-leak guard refused fn_task_done. Off-scope paths: [${offScopePreview}]. Revert them before retrying (for example: git checkout -- <paths>).`, message: `Plan-Only scope-leak guard refused fn_task_done${offendingRepo ? ` (sub-repo ${offendingRepo})` : ""}. Off-scope paths: [${offScopePreview}]. Revert them before retrying (for example: git checkout -- <paths>).`,
}; };
} }
@@ -11337,8 +11390,13 @@ export class TaskExecutor {
// result, so the soft breach of `limit` does not push real // result, so the soft breach of `limit` does not push real
// LLM-active concurrency above the configured cap. // LLM-active concurrency above the configured cap.
const sem = options.semaphore; const sem = options.semaphore;
const invokeReviewer = () => reviewStep( // FNXC:Workspace 2026-06-22-00:30: KTD3 — in-session fn_review_step loops per sub-repo.
worktreePath, taskId, step, step_name, // `reviewStep` stays single-cwd; THIS CALLER loops. Single-cwd by default reviews `worktreePath`;
// in workspace mode that is the browse-only non-git root, so we spawn one reviewer per acquired
// sub-repo (cwd = repo.worktreePath) via reviewWorkspacePerRepo and aggregate as a conjunction.
// `invokeReviewerForCwd` is the per-cwd reviewStep call both modes share.
const invokeReviewerForCwd = (cwd: string) => reviewStep(
cwd, taskId, step, step_name,
reviewType, promptContent, baseline, reviewType, promptContent, baseline,
{ {
onText: (delta) => options.onAgentText?.(taskId, delta), onText: (delta) => options.onAgentText?.(taskId, delta),
@@ -11377,9 +11435,13 @@ export class TaskExecutor {
onSessionEnded: (s) => this.unregisterSubagentSession(taskId, s), onSessionEnded: (s) => this.unregisterSubagentSession(taskId, s),
}, },
); );
const result = sem const runForCwd = (cwd: string) => {
? await sem.runNested(invokeReviewer) const invoke = () => invokeReviewerForCwd(cwd);
: await invokeReviewer(); return sem ? sem.runNested(invoke) : invoke();
};
const result = this.workspaceConfig
? await this.reviewWorkspacePerRepo(currentTask, (cwd) => runForCwd(cwd))
: await runForCwd(worktreePath);
await store.logEntry( await store.logEntry(
taskId, taskId,
@@ -12403,6 +12465,70 @@ ${failureFeedback}
return aggregated; return aggregated;
} }
/**
* FNXC:Workspace 2026-06-22-00:30: KTD3 — per-repo review by looping the EXISTING single-cwd reviewStep.
* The reviewer is an AGENT spawned with `cwd = worktree`, told (in prompt text, reviewer.ts) to run `git diff`
* itself — it does NOT read a diff passed in code. So per-repo review = ONE reviewer agent per sub-repo. We keep
* `reviewStep` single-cwd; the CALLERS loop. This helper is the shared loop+aggregate so both review entry points
* (`createReviewStepTool` and the step-inversion `stepReview` seam) iterate identically: it invokes the caller's
* own `invokeForCwd(cwd)` once per acquired worktree (cwd = repo.worktreePath) and aggregates the repo-tagged
* verdicts as a CONJUNCTION — the task is "reviewed" only if EVERY repo passes; the FIRST non-APPROVE repo's
* verdict becomes the aggregate verdict (mirroring verifyWorktreeInvariants' first-failing-repo return), and its
* findings are repo-tagged. A zero-acquire workspace task (empty map) returns UNAVAILABLE so the caller routes it
* rather than fabricating an APPROVE.
*
* Verdict severity for the conjunction: any RETHINK/REVISE/UNAVAILABLE fails the whole review; only all-APPROVE
* (or all-skipped UNAVAILABLE-advisory, handled by the caller) approves. We surface the first failing repo's exact
* verdict so the caller's existing verdict→edge mapping (APPROVE done-marking, REVISE block, RETHINK reset,
* UNAVAILABLE retry) is unchanged.
*/
private async reviewWorkspacePerRepo(
task: Task,
invokeForCwd: (cwd: string, repoRel: string) => Promise<ReviewResult>,
): Promise<ReviewResult> {
const workspaceWorktrees = task.workspaceWorktrees ?? {};
const entries = Object.entries(workspaceWorktrees);
if (entries.length === 0) {
// No acquired worktree — surface UNAVAILABLE so the caller routes it rather than
// fabricating an authoritative APPROVE for an un-reviewable workspace task.
return {
verdict: "UNAVAILABLE",
review: "No acquired sub-repo worktree to review (workspace task with zero worktrees).",
summary: "Skipped: no sub-repo worktree",
};
}
const reviewSections: string[] = [];
const summarySections: string[] = [];
let firstFailing: { repo: string; result: ReviewResult } | undefined;
for (const [repoRel, repo] of entries) {
const result = await invokeForCwd(repo.worktreePath, repoRel);
// Tag every per-repo finding with its sub-repo so downstream readers attribute it correctly.
reviewSections.push(`### [${repoRel}] ${result.verdict}\n${result.review}`);
summarySections.push(`[${repoRel}] ${result.verdict}: ${result.summary}`);
if (result.verdict !== "APPROVE" && !firstFailing) {
firstFailing = { repo: repoRel, result };
}
}
if (firstFailing) {
// Conjunction failed: the aggregate carries the FIRST failing repo's verdict (so the caller's
// verdict→edge mapping is identical to single-cwd), with the full repo-tagged review body.
return {
verdict: firstFailing.result.verdict,
review: `Workspace review failed in sub-repo \`${firstFailing.repo}\` (verdict ${firstFailing.result.verdict}). Per-repo verdicts:\n\n${reviewSections.join("\n\n")}`,
summary: `${firstFailing.repo}: ${firstFailing.result.verdict} — ${summarySections.join(" | ")}`,
};
}
// Every sub-repo approved → the task is reviewed (conjunction satisfied).
return {
verdict: "APPROVE",
review: `All ${entries.length} sub-repo(s) approved. Per-repo verdicts:\n\n${reviewSections.join("\n\n")}`,
summary: `APPROVE across ${entries.length} sub-repo(s): ${summarySections.join(" | ")}`,
};
}
private async captureUncommittedModifiedFiles(worktreePath: string): Promise<string[]> { private async captureUncommittedModifiedFiles(worktreePath: string): Promise<string[]> {
try { try {
const [unstaged, staged] = await Promise.all([ const [unstaged, staged] = await Promise.all([

View File

@@ -0,0 +1,117 @@
/*
FNXC:Workspace 2026-06-22-00:30:
Minimal shared repo-prefix-derivation helper for workspace mode (Phase B U2; master U5 reuses it). A workspace task's File Scope, modified-file list, and review/scope-leak findings are all repo-prefixed (`<repoRel>/<file>`). Per-repo review and per-repo scope-leak need to map a path → its owning sub-repo, and to derive each repo's File-Scope subset (so a reviewer at `cwd = repo.worktreePath` and a per-repo scope-leak check evaluate only that repo's declared paths).
NO lease logic lives here (file-scope leases are Phase C / master U7). This module is intentionally dependency-light (pure string/path math) so it can be reused across the executor, reviewer callers, and the later merge loop without pulling in executor state.
Matching rule: canonicalize the path to forward-slash relative segments, then pick the LONGEST configured repo key that is a path-segment prefix of the file path. Longest-prefix (not naive first-segment) correctly handles nested repo keys like `apps/web` while still satisfying the simple `wolf-server/src/** → wolf-server` case. A path that matches no configured repo (absolute paths outside the workspace, root-level files like `.changeset/x.md`, or a first segment that is not a repo) derives to the `UNSCOPED` sentinel.
*/
/** Sentinel returned when a path does not belong to any configured sub-repo. */
export const UNSCOPED_REPO = "unscoped" as const;
/**
* Normalize a workspace-relative path token to forward-slash form with no leading
* `./`, no leading/trailing slashes, and collapsed duplicate slashes. Mirrors the
* executor's `normalizeWorkflowScopePath` shape so File-Scope tokens and modified
* files compare consistently, but kept local to avoid an executor import cycle.
*/
function normalizeRepoRelPath(value: string): string {
return value
.trim()
.replace(/\\/g, "/")
.replace(/^\.\//, "")
.replace(/\/+/g, "/")
.replace(/^\/+/, "")
.replace(/\/+$/, "");
}
/** Split a normalized path into non-empty segments. */
function segmentsOf(value: string): string[] {
const normalized = normalizeRepoRelPath(value);
return normalized ? normalized.split("/") : [];
}
/**
* Return true when `repoSegs` is a leading segment-prefix of `pathSegs`.
* Segment-wise (not substring) so `repo-a` does NOT match `repo-ab/...`.
*/
function isSegmentPrefix(repoSegs: string[], pathSegs: string[]): boolean {
if (repoSegs.length === 0 || repoSegs.length > pathSegs.length) return false;
for (let i = 0; i < repoSegs.length; i++) {
if (repoSegs[i] !== pathSegs[i]) return false;
}
return true;
}
/**
* Derive the configured sub-repo that owns `filePath`, or {@link UNSCOPED_REPO}.
*
* `repos` are the configured workspace sub-repo relative keys (from
* `workspaceConfig.repos` or `Object.keys(task.workspaceWorktrees)`). The LONGEST
* matching repo key wins so nested repos (`apps/web` vs `apps`) resolve to the
* most specific owner.
*/
export function deriveRepoForPath(filePath: string, repos: readonly string[]): string {
const pathSegs = segmentsOf(filePath);
if (pathSegs.length === 0) return UNSCOPED_REPO;
let best: string | null = null;
let bestLen = 0;
for (const repo of repos) {
const repoSegs = segmentsOf(repo);
if (repoSegs.length === 0) continue;
if (isSegmentPrefix(repoSegs, pathSegs) && repoSegs.length > bestLen) {
best = normalizeRepoRelPath(repo);
bestLen = repoSegs.length;
}
}
return best ?? UNSCOPED_REPO;
}
/**
* Result of splitting a repo-prefixed File-Scope entry into its owning repo and
* the repo-relative remainder (the path AS the reviewer at `cwd = repo` sees it).
*/
export interface RepoScopedPath {
/** Owning sub-repo key, or {@link UNSCOPED_REPO}. */
repo: string;
/** The path with the repo prefix stripped (repo-local). Equals `path` when unscoped. */
relativePath: string;
}
/**
* Split a repo-prefixed path into `{ repo, relativePath }`. For `repo-a/src/x.ts`
* with `repos=["repo-a"]` → `{ repo:"repo-a", relativePath:"src/x.ts" }`. An
* unscoped path returns the whole normalized path as `relativePath`.
*/
export function splitRepoScopedPath(filePath: string, repos: readonly string[]): RepoScopedPath {
const repo = deriveRepoForPath(filePath, repos);
const normalized = normalizeRepoRelPath(filePath);
if (repo === UNSCOPED_REPO) {
return { repo, relativePath: normalized };
}
const repoNormalized = normalizeRepoRelPath(repo);
const remainder = normalized.slice(repoNormalized.length).replace(/^\/+/, "");
return { repo, relativePath: remainder };
}
/**
* Derive a single sub-repo's File-Scope subset from the task's full (repo-prefixed)
* declared scope. Returns the repo-LOCAL scope patterns (prefix stripped) so a
* per-repo reviewer or per-repo scope-leak check — operating with `cwd = repo` —
* can compare repo-local paths directly. Entries owned by other repos (or unscoped)
* are excluded. A scope entry whose prefix-stripped remainder is empty (the repo
* root itself, e.g. `repo-a` or `repo-a/`) maps to `**` (whole-repo scope).
*/
export function deriveRepoScopeSubset(declaredScope: readonly string[], repoRel: string): string[] {
const repoSegs = segmentsOf(repoRel);
if (repoSegs.length === 0) return [];
const subset: string[] = [];
for (const entry of declaredScope) {
const entrySegs = segmentsOf(entry);
if (!isSegmentPrefix(repoSegs, entrySegs)) continue;
const remainder = entrySegs.slice(repoSegs.length).join("/");
subset.push(remainder === "" ? "**" : remainder);
}
return subset;
}