feat(FN-4482): complete Step 2 — add plan-only scope leak guard
Fusion-Task-Id: FN-4482 Fusion-Task-Lineage: 5850cda2-ded5-42ac-a206-396839580748
This commit is contained in:
@@ -229,6 +229,7 @@ export const DEFAULT_PROJECT_SETTINGS = {
|
|||||||
mergeAuditAutoRecovery: "ai-assisted",
|
mergeAuditAutoRecovery: "ai-assisted",
|
||||||
workflowStepTimeoutMs: 360_000,
|
workflowStepTimeoutMs: 360_000,
|
||||||
workflowStepScopeEnforcement: "block",
|
workflowStepScopeEnforcement: "block",
|
||||||
|
planOnlyScopeLeakEnforcement: "warn",
|
||||||
workflowRevisionForkOnScopeMismatch: true,
|
workflowRevisionForkOnScopeMismatch: true,
|
||||||
strictScopeEnforcement: false,
|
strictScopeEnforcement: false,
|
||||||
buildRetryCount: 0,
|
buildRetryCount: 0,
|
||||||
|
|||||||
@@ -2434,6 +2434,11 @@ export interface ProjectSettings {
|
|||||||
* - "warn": log off-scope writes but allow the step to pass
|
* - "warn": log off-scope writes but allow the step to pass
|
||||||
* - "off": disable workflow-step scope enforcement and keep legacy behavior */
|
* - "off": disable workflow-step scope enforcement and keep legacy behavior */
|
||||||
workflowStepScopeEnforcement?: "block" | "warn" | "off";
|
workflowStepScopeEnforcement?: "block" | "warn" | "off";
|
||||||
|
/** Executor-side scope-leak policy at fn_task_done time for plan-only tasks (review level 1).
|
||||||
|
* - "off": disable guard
|
||||||
|
* - "warn" (default): log [scope-leak] activity but allow completion
|
||||||
|
* - "block": refuse fn_task_done when off-scope files are detected */
|
||||||
|
planOnlyScopeLeakEnforcement?: "off" | "warn" | "block";
|
||||||
/** When true (default), workflow revision feedback that explicitly names files
|
/** When true (default), workflow revision feedback that explicitly names files
|
||||||
* outside the task's declared File Scope is forked into a dependent follow-up
|
* outside the task's declared File Scope is forked into a dependent follow-up
|
||||||
* task instead of being appended to the original PROMPT.md. Set to false to
|
* task instead of being appended to the original PROMPT.md. Set to false to
|
||||||
|
|||||||
@@ -0,0 +1,156 @@
|
|||||||
|
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||||
|
import "./executor-test-helpers.js";
|
||||||
|
import { TaskExecutor } from "../executor.js";
|
||||||
|
import { executorLog } from "../logger.js";
|
||||||
|
import { createMockStore, mockedCreateFnAgent, mockedExecSync, resetExecutorMocks } from "./executor-test-helpers.js";
|
||||||
|
|
||||||
|
function baseTask(overrides: Record<string, unknown> = {}) {
|
||||||
|
return {
|
||||||
|
id: "FN-4482",
|
||||||
|
title: "Scope leak guard",
|
||||||
|
description: "",
|
||||||
|
prompt: "## Review Level: 1",
|
||||||
|
column: "in-progress",
|
||||||
|
worktree: "/repo/.worktrees/swift-falcon",
|
||||||
|
branch: "fusion/fn-4482",
|
||||||
|
baseCommitSha: "abc123",
|
||||||
|
taskDoneRetryCount: 0,
|
||||||
|
steps: [{ name: "Step 1", status: "in-progress" as const }],
|
||||||
|
currentStep: 0,
|
||||||
|
dependencies: [],
|
||||||
|
createdAt: new Date().toISOString(),
|
||||||
|
updatedAt: new Date().toISOString(),
|
||||||
|
...overrides,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
async function setup(params?: {
|
||||||
|
reviewLevel?: number;
|
||||||
|
enforcement?: "off" | "warn" | "block";
|
||||||
|
scope?: string[];
|
||||||
|
scopeOverride?: boolean;
|
||||||
|
unstaged?: string[];
|
||||||
|
staged?: string[];
|
||||||
|
committed?: string[];
|
||||||
|
gitFailure?: boolean;
|
||||||
|
}) {
|
||||||
|
const store = createMockStore();
|
||||||
|
let task = baseTask({
|
||||||
|
prompt: `## Review Level: ${params?.reviewLevel ?? 1}`,
|
||||||
|
scopeOverride: params?.scopeOverride,
|
||||||
|
});
|
||||||
|
let tool: any;
|
||||||
|
|
||||||
|
store.getTask.mockImplementation(async () => ({ ...task, steps: task.steps.map((s: any) => ({ ...s })) }));
|
||||||
|
store.parseFileScopeFromPrompt.mockResolvedValue(params?.scope ?? ["docs/foo.md"]);
|
||||||
|
store.getSettings.mockResolvedValue({
|
||||||
|
maxConcurrent: 2,
|
||||||
|
maxWorktrees: 4,
|
||||||
|
pollIntervalMs: 15000,
|
||||||
|
groupOverlappingFiles: false,
|
||||||
|
autoMerge: false,
|
||||||
|
worktreeInitCommand: undefined,
|
||||||
|
planOnlyScopeLeakEnforcement: params?.enforcement ?? "warn",
|
||||||
|
});
|
||||||
|
|
||||||
|
mockedExecSync.mockImplementation((cmd: string) => {
|
||||||
|
if (cmd.includes("rev-parse --show-toplevel")) return Buffer.from("/repo/.worktrees/swift-falcon\n");
|
||||||
|
if (cmd.includes("rev-parse --abbrev-ref HEAD")) return Buffer.from("fusion/fn-4482\n");
|
||||||
|
if (cmd.includes("rev-list --count")) return Buffer.from("1\n");
|
||||||
|
if (cmd.includes("git diff --name-only --cached")) {
|
||||||
|
if (params?.gitFailure) throw new Error("git failed");
|
||||||
|
return Buffer.from(`${(params?.staged ?? []).join("\n")}\n`);
|
||||||
|
}
|
||||||
|
if (cmd.includes("git diff --name-only abc123..HEAD")) {
|
||||||
|
return Buffer.from(`${(params?.committed ?? []).join("\n")}\n`);
|
||||||
|
}
|
||||||
|
if (cmd.includes("git diff --name-only")) {
|
||||||
|
if (params?.gitFailure) throw new Error("git failed");
|
||||||
|
return Buffer.from(`${(params?.unstaged ?? []).join("\n")}\n`);
|
||||||
|
}
|
||||||
|
return Buffer.from("");
|
||||||
|
});
|
||||||
|
|
||||||
|
mockedCreateFnAgent.mockImplementation(async ({ customTools }: any) => {
|
||||||
|
tool = customTools.find((t: any) => t.name === "fn_task_done");
|
||||||
|
return { session: { prompt: vi.fn().mockResolvedValue(undefined), dispose: vi.fn() } } as any;
|
||||||
|
});
|
||||||
|
|
||||||
|
const executor = new TaskExecutor(store as any, "/repo");
|
||||||
|
await executor.execute(task as any);
|
||||||
|
|
||||||
|
return { store, tool };
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("FN-4482 plan-only scope leak guard", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
resetExecutorMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("allows plan-only completion when edits are in-scope", async () => {
|
||||||
|
const { store, tool } = await setup({ unstaged: ["docs/foo.md"] });
|
||||||
|
const result = await tool.execute("id", {});
|
||||||
|
expect(result.content[0].text).toContain("Task marked complete");
|
||||||
|
expect(store.logEntry.mock.calls.some(([_, message]) => String(message).includes("[scope-leak] reviewLevel="))).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("warns but allows plan-only off-scope edits in default warn mode", async () => {
|
||||||
|
const { store, tool } = await setup({ unstaged: ["packages/core/src/db.ts"] });
|
||||||
|
const result = await tool.execute("id", {});
|
||||||
|
expect(result.content[0].text).toContain("Task marked complete");
|
||||||
|
expect(store.logEntry).toHaveBeenCalledWith(
|
||||||
|
"FN-4482",
|
||||||
|
expect.stringContaining("[scope-leak] reviewLevel=1 enforcement=warn"),
|
||||||
|
undefined,
|
||||||
|
undefined,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("blocks plan-only off-scope edits when enforcement is block", async () => {
|
||||||
|
const { store, tool } = await setup({ enforcement: "block", unstaged: ["packages/core/src/db.ts"] });
|
||||||
|
const result = await tool.execute("id", {});
|
||||||
|
expect(result.content[0].text).toContain("Plan-Only scope-leak guard refused fn_task_done");
|
||||||
|
expect(result.content[0].text).toContain("packages/core/src/db.ts");
|
||||||
|
expect(store.updateStep).not.toHaveBeenCalled();
|
||||||
|
expect(store.moveTask).not.toHaveBeenCalledWith("FN-4482", "in-review");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("bypasses guard when scopeOverride=true", async () => {
|
||||||
|
const { store, tool } = await setup({ scopeOverride: true, unstaged: ["packages/core/src/db.ts"] });
|
||||||
|
const result = await tool.execute("id", {});
|
||||||
|
expect(result.content[0].text).toContain("Task marked complete");
|
||||||
|
expect(store.logEntry).toHaveBeenCalledWith(
|
||||||
|
"FN-4482",
|
||||||
|
"[scope-leak] scope guard bypassed via task.scopeOverride",
|
||||||
|
undefined,
|
||||||
|
undefined,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("skips checks when planOnlyScopeLeakEnforcement=off", async () => {
|
||||||
|
const { store, tool } = await setup({ enforcement: "off", unstaged: ["packages/core/src/db.ts"] });
|
||||||
|
const result = await tool.execute("id", {});
|
||||||
|
expect(result.content[0].text).toContain("Task marked complete");
|
||||||
|
expect(store.logEntry.mock.calls.some(([_, message]) => String(message).includes("[scope-leak] reviewLevel="))).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it.each([0, 2])("uses warn-only behavior for non-plan-only review level %s", async (reviewLevel) => {
|
||||||
|
const { store, tool } = await setup({ reviewLevel, enforcement: "block", unstaged: ["packages/core/src/db.ts"] });
|
||||||
|
const result = await tool.execute("id", {});
|
||||||
|
expect(result.content[0].text).toContain("Task marked complete");
|
||||||
|
expect(store.logEntry).toHaveBeenCalledWith(
|
||||||
|
"FN-4482",
|
||||||
|
expect.stringContaining(`[scope-leak] reviewLevel=${reviewLevel} enforcement=warn`),
|
||||||
|
undefined,
|
||||||
|
undefined,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("fails open on git capture failure", async () => {
|
||||||
|
const { store, tool } = await setup({ gitFailure: true });
|
||||||
|
const result = await tool.execute("id", {});
|
||||||
|
expect(result.content[0].text).toContain("Task marked complete");
|
||||||
|
expect((executorLog.warn as any).mock.calls.some(([message]: [string]) => message.includes("Failed to capture uncommitted modified files"))).toBe(true);
|
||||||
|
expect(store.logEntry.mock.calls.some(([_, message]) => String(message).includes("[scope-leak] reviewLevel="))).toBe(false);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -229,6 +229,11 @@ export function workflowPathMatchesDeclaredScope(filePath: string, scopePatterns
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function parseReviewLevelFromPrompt(prompt: string): number {
|
||||||
|
const reviewMatch = prompt.match(/##\s*Review Level[:\s]*(\d)/);
|
||||||
|
return reviewMatch ? parseInt(reviewMatch[1], 10) : 0;
|
||||||
|
}
|
||||||
|
|
||||||
export function partitionWorkflowRevisionFeedback(
|
export function partitionWorkflowRevisionFeedback(
|
||||||
feedback: string,
|
feedback: string,
|
||||||
declaredFileScope: readonly string[],
|
declaredFileScope: readonly string[],
|
||||||
@@ -4867,6 +4872,62 @@ export class TaskExecutor {
|
|||||||
return { ok: true };
|
return { ok: true };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private async evaluateTaskDoneScopeLeak(
|
||||||
|
task: Task,
|
||||||
|
worktreePath: string,
|
||||||
|
settings: Settings,
|
||||||
|
): Promise<{ blocked: false } | { blocked: true; message: string }> {
|
||||||
|
if (task.scopeOverride === true) {
|
||||||
|
executorLog.log(`${task.id}: scope-leak guard bypassed (scopeOverride=true)`);
|
||||||
|
await this.store.logEntry(task.id, "[scope-leak] scope guard bypassed via task.scopeOverride", undefined, this.currentRunContext);
|
||||||
|
return { blocked: false };
|
||||||
|
}
|
||||||
|
|
||||||
|
const declaredScope = await this.store.parseFileScopeFromPrompt(task.id).catch(() => [] as string[]);
|
||||||
|
if (declaredScope.length === 0) {
|
||||||
|
return { blocked: false };
|
||||||
|
}
|
||||||
|
|
||||||
|
const reviewLevel = parseReviewLevelFromPrompt(task.prompt ?? "");
|
||||||
|
const configuredMode = settings.planOnlyScopeLeakEnforcement ?? "warn";
|
||||||
|
const enforcementMode: "off" | "warn" | "block" = reviewLevel === 1
|
||||||
|
? configuredMode
|
||||||
|
: "warn";
|
||||||
|
|
||||||
|
if (enforcementMode === "off") {
|
||||||
|
return { blocked: false };
|
||||||
|
}
|
||||||
|
|
||||||
|
const [uncommittedTouchedFiles, branchCommittedFiles] = await Promise.all([
|
||||||
|
this.captureUncommittedModifiedFiles(worktreePath),
|
||||||
|
this.captureModifiedFiles(worktreePath, task.baseCommitSha),
|
||||||
|
]);
|
||||||
|
|
||||||
|
const touchedFiles = [...new Set([...uncommittedTouchedFiles, ...branchCommittedFiles])];
|
||||||
|
if (touchedFiles.length === 0) {
|
||||||
|
return { blocked: false };
|
||||||
|
}
|
||||||
|
|
||||||
|
const offScopeFiles = touchedFiles
|
||||||
|
.filter((filePath) => !workflowPathMatchesDeclaredScope(filePath, declaredScope));
|
||||||
|
if (offScopeFiles.length === 0) {
|
||||||
|
return { blocked: false };
|
||||||
|
}
|
||||||
|
|
||||||
|
const message = `[scope-leak] reviewLevel=${reviewLevel} enforcement=${enforcementMode} off-scope touched files [${offScopeFiles.join(", ")}]; declared scope [${declaredScope.join(", ")}]`;
|
||||||
|
executorLog.warn(`${task.id}: ${message}`);
|
||||||
|
await this.store.logEntry(task.id, message, undefined, this.currentRunContext);
|
||||||
|
|
||||||
|
if (enforcementMode === "block") {
|
||||||
|
return {
|
||||||
|
blocked: true,
|
||||||
|
message: `Plan-Only scope-leak guard refused fn_task_done. Off-scope paths: [${offScopeFiles.join(", ")}]. Revert them before retrying (for example: git checkout -- <paths>).`,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
return { blocked: false };
|
||||||
|
}
|
||||||
|
|
||||||
private createTaskDoneTool(taskId: string, worktreePath: string, onDone: () => void): ToolDefinition {
|
private createTaskDoneTool(taskId: string, worktreePath: string, onDone: () => void): ToolDefinition {
|
||||||
const store = this.store;
|
const store = this.store;
|
||||||
return {
|
return {
|
||||||
@@ -4946,6 +5007,23 @@ export class TaskExecutor {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const settings = await store.getSettings();
|
||||||
|
const scopeLeakCheck = await this.evaluateTaskDoneScopeLeak(task, worktreePath, settings)
|
||||||
|
.catch((error: unknown) => {
|
||||||
|
const errorMessage = error instanceof Error ? error.message : String(error);
|
||||||
|
executorLog.warn(`${taskId}: scope-leak guard failed open: ${errorMessage}`);
|
||||||
|
return { blocked: false } as const;
|
||||||
|
});
|
||||||
|
if (scopeLeakCheck.blocked) {
|
||||||
|
await store.logEntry(taskId, `[scope-leak] blocked fn_task_done: ${scopeLeakCheck.message}`, undefined, this.currentRunContext);
|
||||||
|
return {
|
||||||
|
content: [{ type: "text" as const, text: scopeLeakCheck.message }],
|
||||||
|
details: {
|
||||||
|
error: scopeLeakCheck.message,
|
||||||
|
},
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
onDone();
|
onDone();
|
||||||
|
|
||||||
// Mark all pending/in-progress steps as done
|
// Mark all pending/in-progress steps as done
|
||||||
@@ -4971,7 +5049,6 @@ export class TaskExecutor {
|
|||||||
await store.updateTask(taskId, { summary: params.summary });
|
await store.updateTask(taskId, { summary: params.summary });
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
const settings = await store.getSettings();
|
|
||||||
const hardPauseActive = Boolean(settings.globalPause);
|
const hardPauseActive = Boolean(settings.globalPause);
|
||||||
// Task-level pause prevents new work from starting, not completion of
|
// Task-level pause prevents new work from starting, not completion of
|
||||||
// in-flight work. Always clear it on explicit agent completion so the
|
// in-flight work. Always clear it on explicit agent completion so the
|
||||||
@@ -6148,7 +6225,7 @@ ${failureFeedback}
|
|||||||
return [...new Set(files)];
|
return [...new Set(files)];
|
||||||
} catch (err: unknown) {
|
} catch (err: unknown) {
|
||||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||||
executorLog.log(`Failed to capture uncommitted modified files: ${errorMessage}`);
|
executorLog.warn(`Failed to capture uncommitted modified files: ${errorMessage}`);
|
||||||
return [];
|
return [];
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -8740,8 +8817,7 @@ export function buildExecutionPrompt(
|
|||||||
pluginRunner?: PluginRunner,
|
pluginRunner?: PluginRunner,
|
||||||
): string {
|
): string {
|
||||||
const prompt = scopePromptToWorktree(task.prompt, rootDir, worktreePath);
|
const prompt = scopePromptToWorktree(task.prompt, rootDir, worktreePath);
|
||||||
const reviewMatch = prompt.match(/##\s*Review Level[:\s]*(\d)/);
|
const reviewLevel = parseReviewLevelFromPrompt(prompt);
|
||||||
const reviewLevel = reviewMatch ? parseInt(reviewMatch[1], 10) : 0;
|
|
||||||
|
|
||||||
// Build author arg for git commits based on settings
|
// Build author arg for git commits based on settings
|
||||||
const authorArg = settings?.commitAuthorEnabled !== false
|
const authorArg = settings?.commitAuthorEnabled !== false
|
||||||
|
|||||||
Reference in New Issue
Block a user