feat(FN-4368): complete Step 3 — advisory workflow failures never block
Fusion-Task-Id: FN-4368 Fusion-Task-Lineage: ac9b12e6-2101-4b51-89a0-6422fd7850ed
This commit is contained in:
@@ -2132,6 +2132,7 @@ describe("Workflow Steps Execution", () => {
|
||||
id: "WS-001",
|
||||
name: "Security Audit",
|
||||
description: "Check for vulnerabilities",
|
||||
gateMode: "gate",
|
||||
prompt: "Scan for security issues.",
|
||||
gateMode: "gate",
|
||||
enabled: true,
|
||||
@@ -2233,6 +2234,114 @@ describe("Workflow Steps Execution", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("treats prompt advisory workflow revision requests as non-blocking findings", async () => {
|
||||
const store = createMockStore();
|
||||
|
||||
store.getTask.mockResolvedValue({
|
||||
id: "FN-001",
|
||||
title: "Test",
|
||||
description: "Test task",
|
||||
column: "in-progress",
|
||||
dependencies: [],
|
||||
steps: [{ name: "Preflight", status: "pending" }],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
enabledWorkflowSteps: ["WS-001"],
|
||||
prompt: "# test\n## Steps\n### Step 0: Preflight\n- [ ] check",
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
store.getWorkflowStep.mockResolvedValue({
|
||||
id: "WS-001",
|
||||
name: "Frontend UX Design",
|
||||
description: "Polish pass",
|
||||
gateMode: "advisory",
|
||||
mode: "prompt",
|
||||
prompt: "Review polish quality.",
|
||||
enabled: true,
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
store.parseFileScopeFromPrompt.mockResolvedValue(["src/auth.ts"]);
|
||||
|
||||
let callIdx = 0;
|
||||
let subscribeHandler: any;
|
||||
mockedCreateFnAgent.mockImplementation((async (opts: any) => {
|
||||
callIdx++;
|
||||
if (callIdx === 1) {
|
||||
const customTools = opts.customTools || [];
|
||||
const session = {
|
||||
prompt: vi.fn().mockImplementation(async () => {
|
||||
const taskDoneTool = customTools.find((t: any) => t.name === "fn_task_done");
|
||||
if (taskDoneTool) await taskDoneTool.execute("tool-1", {});
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
subscribe: vi.fn(),
|
||||
on: vi.fn(),
|
||||
sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") },
|
||||
state: {},
|
||||
};
|
||||
return { session };
|
||||
}
|
||||
|
||||
const session = {
|
||||
prompt: vi.fn().mockImplementation(async () => {
|
||||
subscribeHandler?.({
|
||||
type: "message_update",
|
||||
assistantMessageEvent: { type: "text_delta", delta: "REQUEST REVISION\n\nPolish note: tighten auth error copy." },
|
||||
});
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
subscribe: vi.fn((handler: any) => {
|
||||
subscribeHandler = handler;
|
||||
}),
|
||||
state: {},
|
||||
};
|
||||
return { session };
|
||||
}) as any);
|
||||
|
||||
const onComplete = vi.fn();
|
||||
const executor = new TaskExecutor(store, "/tmp/test", { onComplete });
|
||||
|
||||
await executor.execute({
|
||||
id: "FN-001",
|
||||
title: "Test",
|
||||
description: "Test task",
|
||||
column: "in-progress",
|
||||
dependencies: [],
|
||||
steps: [{ name: "Preflight", status: "pending" }],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
enabledWorkflowSteps: ["WS-001"],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith(
|
||||
"FN-001",
|
||||
expect.objectContaining({
|
||||
workflowStepResults: expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
workflowStepId: "WS-001",
|
||||
status: "advisory_failure",
|
||||
notes: expect.stringContaining("Polish note"),
|
||||
}),
|
||||
]),
|
||||
}),
|
||||
);
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "in-review");
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-001",
|
||||
expect.stringContaining("gateMode=advisory"),
|
||||
);
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith(
|
||||
"FN-001",
|
||||
expect.objectContaining({ status: "failed", error: "Workflow step failed" }),
|
||||
);
|
||||
expect(onComplete).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("forks out-of-scope workflow revision feedback into a follow-up task and leaves the original task untouched", async () => {
|
||||
const store = createMockStore();
|
||||
|
||||
@@ -2256,6 +2365,7 @@ describe("Workflow Steps Execution", () => {
|
||||
id: "WS-001",
|
||||
name: "Security Audit",
|
||||
description: "Check for vulnerabilities",
|
||||
gateMode: "gate",
|
||||
prompt: "Scan for security issues.",
|
||||
gateMode: "gate",
|
||||
enabled: true,
|
||||
@@ -2354,6 +2464,7 @@ describe("Workflow Steps Execution", () => {
|
||||
id: "WS-001",
|
||||
name: "Security Audit",
|
||||
description: "Check for vulnerabilities",
|
||||
gateMode: "gate",
|
||||
prompt: "Scan for security issues.",
|
||||
gateMode: "gate",
|
||||
enabled: true,
|
||||
@@ -2459,6 +2570,7 @@ describe("Workflow Steps Execution", () => {
|
||||
id: "WS-001",
|
||||
name: "Security Audit",
|
||||
description: "Check for vulnerabilities",
|
||||
gateMode: "gate",
|
||||
prompt: "Scan for security issues.",
|
||||
gateMode: "gate",
|
||||
enabled: true,
|
||||
|
||||
@@ -4116,6 +4116,32 @@ describe("SelfHealingManager", () => {
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("ignores advisory pre-merge workflow findings", async () => {
|
||||
const recoverFn = vi.fn().mockResolvedValue(true);
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
recoverFailedPreMergeStep: recoverFn,
|
||||
});
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
maxPostReviewFixes: 1,
|
||||
});
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
...baseTask,
|
||||
workflowStepResults: [{
|
||||
...baseTask.workflowStepResults[0],
|
||||
status: "advisory_failure" as const,
|
||||
}],
|
||||
},
|
||||
]);
|
||||
|
||||
const result = await managerWithRecovery.recoverReviewTasksWithFailedPreMergeSteps();
|
||||
|
||||
expect(result).toBe(0);
|
||||
expect(recoverFn).not.toHaveBeenCalled();
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("disables itself when maxPostReviewFixes is 0", async () => {
|
||||
const recoverFn = vi.fn().mockResolvedValue(true);
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
|
||||
@@ -2167,13 +2167,26 @@ export class TaskExecutor {
|
||||
*/
|
||||
async recoverFailedPreMergeWorkflowStep(task: Task): Promise<boolean> {
|
||||
try {
|
||||
const failed = (task.workflowStepResults ?? [])
|
||||
const preMergeFailed = (task.workflowStepResults ?? [])
|
||||
.filter((r) => (r.phase || "pre-merge") === "pre-merge" && r.status === "failed")
|
||||
.sort((a, b) => {
|
||||
const aTs = Date.parse(a.completedAt || a.startedAt || "");
|
||||
const bTs = Date.parse(b.completedAt || b.startedAt || "");
|
||||
return (Number.isFinite(bTs) ? bTs : 0) - (Number.isFinite(aTs) ? aTs : 0);
|
||||
});
|
||||
|
||||
const gateModeCache = new Map<string, "gate" | "advisory">();
|
||||
const failed: typeof preMergeFailed = [];
|
||||
for (const result of preMergeFailed) {
|
||||
let mode = gateModeCache.get(result.workflowStepId);
|
||||
if (!mode) {
|
||||
const step = await this.store.getWorkflowStep(result.workflowStepId).catch(() => null);
|
||||
mode = step?.gateMode || (step?.mode === "script" ? "gate" : "advisory");
|
||||
gateModeCache.set(result.workflowStepId, mode);
|
||||
}
|
||||
if (mode === "gate") failed.push(result);
|
||||
}
|
||||
|
||||
const target = failed[0];
|
||||
if (!target) {
|
||||
executorLog.warn(`${task.id}: no failed pre-merge workflow step to recover from`);
|
||||
@@ -6185,6 +6198,7 @@ ${failureFeedback}
|
||||
...results[existingIdx],
|
||||
status: gateMode === "advisory" ? "advisory_failure" : "failed",
|
||||
output: scopeLeakMessage,
|
||||
notes: scopeLeakMessage,
|
||||
completedAt,
|
||||
};
|
||||
}
|
||||
@@ -6235,6 +6249,7 @@ ${failureFeedback}
|
||||
...results[existingIdx],
|
||||
status: gateMode === "advisory" ? "advisory_failure" : "failed",
|
||||
output: result.output || "Revision requested",
|
||||
notes: result.output || "Revision requested",
|
||||
completedAt,
|
||||
};
|
||||
}
|
||||
@@ -6265,6 +6280,7 @@ ${failureFeedback}
|
||||
...results[existingIdx],
|
||||
status: gateMode === "advisory" ? "advisory_failure" : "failed",
|
||||
output: result.error || "Workflow step failed",
|
||||
notes: result.error || "Workflow step failed",
|
||||
completedAt,
|
||||
};
|
||||
}
|
||||
@@ -6300,6 +6316,7 @@ ${failureFeedback}
|
||||
...results[existingIdx],
|
||||
status: gateMode === "advisory" ? "advisory_failure" : "failed",
|
||||
output: errorMessage || "Workflow step error",
|
||||
notes: errorMessage || "Workflow step error",
|
||||
completedAt,
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user