fix(FN-4343): isolate workflow-step scope checks to post-step delta
Fusion-Task-Id: FN-4343 Fusion-Task-Lineage: b880b71a-9250-4d9e-bdac-8298a73058f7
This commit is contained in:
@@ -44,6 +44,21 @@ function mockDiffFiles(files: string[]) {
|
||||
});
|
||||
}
|
||||
|
||||
function mockDiffSequence(preStepFiles: string[], postStepFiles: string[]) {
|
||||
let diffCallCount = 0;
|
||||
mockedExecSync.mockImplementation((cmd: string | string[]) => {
|
||||
if (typeof cmd === "string" && cmd.includes("git merge-base HEAD origin/main")) {
|
||||
return Buffer.from("abc123\n");
|
||||
}
|
||||
if (typeof cmd === "string" && cmd.includes("git diff --name-only abc123..HEAD")) {
|
||||
diffCallCount += 1;
|
||||
const files = diffCallCount === 1 ? preStepFiles : postStepFiles;
|
||||
return Buffer.from(files.join("\n"));
|
||||
}
|
||||
return Buffer.from("");
|
||||
});
|
||||
}
|
||||
|
||||
describe("executor workflow step scope gating", () => {
|
||||
beforeEach(() => {
|
||||
resetExecutorMocks();
|
||||
@@ -113,7 +128,7 @@ describe("executor workflow step scope gating", () => {
|
||||
store.getTask.mockResolvedValue(task as any);
|
||||
store.getWorkflowStep.mockResolvedValue(createWorkflowStep({ id: "WS-001", name: "Workflow Review" }) as any);
|
||||
store.parseFileScopeFromPrompt.mockResolvedValue(["packages/engine/src/executor.ts"]);
|
||||
mockDiffFiles(["packages/dashboard/app/components/TaskDetailModal.tsx"]);
|
||||
mockDiffSequence([], ["packages/dashboard/app/components/TaskDetailModal.tsx"]);
|
||||
|
||||
const executor = new TaskExecutor(store as any, "/tmp/test", {} as any);
|
||||
vi.spyOn(executor as any, "executeWorkflowStep").mockResolvedValue({ success: true, output: "ok" });
|
||||
@@ -124,13 +139,34 @@ describe("executor workflow step scope gating", () => {
|
||||
expect(String((result as any).feedback)).toContain("wrote files outside declared File Scope");
|
||||
});
|
||||
|
||||
it("detects off-scope delta even when pre-step diff has in-scope files", async () => {
|
||||
const store = createMockStore();
|
||||
const task = { ...createTask(), enabledWorkflowSteps: ["WS-001"] };
|
||||
store.getTask.mockResolvedValue(task as any);
|
||||
store.getWorkflowStep.mockResolvedValue(createWorkflowStep({ id: "WS-001", name: "Workflow Review" }) as any);
|
||||
store.parseFileScopeFromPrompt.mockResolvedValue(["packages/engine/src/executor.ts"]);
|
||||
|
||||
mockDiffSequence(
|
||||
["packages/engine/src/executor.ts"],
|
||||
["packages/engine/src/executor.ts", "packages/dashboard/app/components/TaskDetailModal.tsx"],
|
||||
);
|
||||
|
||||
const executor = new TaskExecutor(store as any, "/tmp/test", {} as any);
|
||||
vi.spyOn(executor as any, "executeWorkflowStep").mockResolvedValue({ success: true, output: "ok" });
|
||||
|
||||
const result = await (executor as any).runWorkflowSteps(task as any, "/tmp/test", { workflowStepScopeEnforcement: "block" } as any);
|
||||
|
||||
expect(result).toEqual(expect.objectContaining({ allPassed: false, revisionRequested: true }));
|
||||
expect(String((result as any).feedback)).toContain("TaskDetailModal.tsx");
|
||||
});
|
||||
|
||||
it("warn mode logs but passes on off-scope writes", async () => {
|
||||
const store = createMockStore();
|
||||
const task = { ...createTask(), enabledWorkflowSteps: ["WS-001"] };
|
||||
store.getTask.mockResolvedValue(task as any);
|
||||
store.getWorkflowStep.mockResolvedValue(createWorkflowStep({ id: "WS-001", name: "Workflow Review" }) as any);
|
||||
store.parseFileScopeFromPrompt.mockResolvedValue(["packages/engine/src/executor.ts"]);
|
||||
mockDiffFiles(["packages/dashboard/app/components/TaskDetailModal.tsx"]);
|
||||
mockDiffSequence([], ["packages/dashboard/app/components/TaskDetailModal.tsx"]);
|
||||
|
||||
const executor = new TaskExecutor(store as any, "/tmp/test", {} as any);
|
||||
vi.spyOn(executor as any, "executeWorkflowStep").mockResolvedValue({ success: true, output: "ok" });
|
||||
@@ -164,7 +200,7 @@ describe("executor workflow step scope gating", () => {
|
||||
store.getTask.mockResolvedValue(task as any);
|
||||
store.getWorkflowStep.mockResolvedValue(createWorkflowStep({ id: "WS-001", name: "Workflow Review" }) as any);
|
||||
store.parseFileScopeFromPrompt.mockResolvedValue(["packages/engine/src/executor.ts"]);
|
||||
mockDiffFiles(["packages/dashboard/app/components/TaskDetailModal.tsx"]);
|
||||
mockDiffSequence([], ["packages/dashboard/app/components/TaskDetailModal.tsx"]);
|
||||
|
||||
const executor = new TaskExecutor(store as any, "/tmp/test", {} as any);
|
||||
vi.spyOn(executor as any, "executeWorkflowStep").mockResolvedValue({ success: true, output: "ok" });
|
||||
|
||||
@@ -5847,6 +5847,23 @@ ${failureFeedback}
|
||||
}
|
||||
}
|
||||
|
||||
private async captureUncommittedModifiedFiles(worktreePath: string): Promise<string[]> {
|
||||
try {
|
||||
const [unstaged, staged] = await Promise.all([
|
||||
execAsync("git diff --name-only", { cwd: worktreePath, encoding: "utf-8" }),
|
||||
execAsync("git diff --name-only --cached", { cwd: worktreePath, encoding: "utf-8" }),
|
||||
]);
|
||||
const files = [...unstaged.stdout.split("\n"), ...staged.stdout.split("\n")]
|
||||
.map((entry) => entry.trim())
|
||||
.filter(Boolean);
|
||||
return [...new Set(files)];
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
executorLog.log(`Failed to capture uncommitted modified files: ${errorMessage}`);
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
// ── Worktree management ────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
@@ -5978,6 +5995,13 @@ ${failureFeedback}
|
||||
|
||||
const startedAt = new Date().toISOString();
|
||||
const stepStartedAtMs = Date.now();
|
||||
const workflowStepScopeEnforcement = settings.workflowStepScopeEnforcement ?? "block";
|
||||
const shouldCheckWorkflowStepScope = stepPhase === "pre-merge"
|
||||
&& stepMode === "prompt"
|
||||
&& workflowStepScopeEnforcement !== "off";
|
||||
const preStepModifiedFiles = shouldCheckWorkflowStepScope
|
||||
? await this.captureModifiedFiles(worktreePath, currentTask.baseCommitSha)
|
||||
: [];
|
||||
|
||||
// Push pending entry BEFORE execution so dashboard can show live status
|
||||
results.push({
|
||||
@@ -5999,18 +6023,21 @@ ${failureFeedback}
|
||||
const completedAt = new Date().toISOString();
|
||||
|
||||
if (result.success) {
|
||||
const workflowStepScopeEnforcement = settings.workflowStepScopeEnforcement ?? "block";
|
||||
if (stepPhase === "pre-merge" && stepMode === "prompt" && workflowStepScopeEnforcement !== "off") {
|
||||
if (shouldCheckWorkflowStepScope) {
|
||||
const declaredScope = await this.store.parseFileScopeFromPrompt(task.id).catch(() => [] as string[]);
|
||||
const refreshedTask = await this.store.getTask(task.id);
|
||||
if (declaredScope.length > 0 && refreshedTask?.scopeOverride !== true) {
|
||||
const postStepModifiedFiles = await this.captureModifiedFiles(worktreePath, currentTask.baseCommitSha);
|
||||
const hasScopeOverlap = postStepModifiedFiles.some((filePath) => workflowPathMatchesDeclaredScope(filePath, declaredScope));
|
||||
if (postStepModifiedFiles.length > 0 && !hasScopeOverlap) {
|
||||
const scopeLeakMessage = `Workflow step '${ws.name}' wrote files outside declared File Scope. Staged: [${postStepModifiedFiles.join(", ")}]. Declared: [${declaredScope.join(", ")}]. (FN-4343)`;
|
||||
const preStepSet = new Set(preStepModifiedFiles);
|
||||
const stepCommittedFiles = postStepModifiedFiles.filter((filePath) => !preStepSet.has(filePath));
|
||||
const stepUncommittedFiles = await this.captureUncommittedModifiedFiles(worktreePath);
|
||||
const stepTouchedFiles = [...new Set([...stepCommittedFiles, ...stepUncommittedFiles])];
|
||||
const hasScopeOverlap = stepTouchedFiles.some((filePath) => workflowPathMatchesDeclaredScope(filePath, declaredScope));
|
||||
if (stepTouchedFiles.length > 0 && !hasScopeOverlap) {
|
||||
const scopeLeakMessage = `Workflow step '${ws.name}' wrote files outside declared File Scope. Staged: [${stepTouchedFiles.join(", ")}]. Declared: [${declaredScope.join(", ")}]. (FN-4343)`;
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`[pre-merge] Workflow step scope leak: ${ws.name} wrote off-scope files [${postStepModifiedFiles.join(", ") || "<none>"}]`,
|
||||
`[pre-merge] Workflow step scope leak: ${ws.name} wrote off-scope files [${stepTouchedFiles.join(", ") || "<none>"}]`,
|
||||
);
|
||||
if (workflowStepScopeEnforcement === "warn") {
|
||||
await this.store.logEntry(task.id, `[pre-merge] workflowStepScopeEnforcement=warn — ${scopeLeakMessage}`);
|
||||
|
||||
Reference in New Issue
Block a user