fix(FN-7225): keep plan review in triage
This commit is contained in:
7
.changeset/fn-plan-review-triage-gate.md
Normal file
7
.changeset/fn-plan-review-triage-gate.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Keep Plan Review in triage and prevent duplicate execution-time plan reviews.
|
||||
category: fix
|
||||
dev: Triage now runs enabled Plan Review before releasing tasks to execution; execution graph skips an already-passed Plan Review.
|
||||
@@ -922,7 +922,13 @@ describe("built-in workflows", () => {
|
||||
enabledWorkflowSteps: [],
|
||||
});
|
||||
|
||||
expect((await store.getTask(task.id)).enabledWorkflowSteps ?? []).toEqual([]);
|
||||
/*
|
||||
FNXC:WorkflowOptionalSteps 2026-06-29-02:55:
|
||||
An explicit empty optional-step selection must hydrate back as `[]`, not
|
||||
`undefined`; otherwise later workflow execution can confuse "all disabled"
|
||||
with "not materialized" and re-run default-on Plan Review / Code Review.
|
||||
*/
|
||||
expect((await store.getTask(task.id)).enabledWorkflowSteps).toEqual([]);
|
||||
expect(store.getTaskWorkflowSelection(task.id)).toEqual({
|
||||
workflowId: "builtin:coding",
|
||||
stepIds: [],
|
||||
|
||||
@@ -963,6 +963,7 @@ describe("TaskStore", () => {
|
||||
const result = await store.applyReplicatedTaskCreate(payload);
|
||||
expect(result.applied).toBe(true);
|
||||
expect(result.task.enabledWorkflowSteps).toBeUndefined();
|
||||
expect((await store.getTask(payload.taskId)).enabledWorkflowSteps).toEqual([]);
|
||||
});
|
||||
|
||||
it("applyReplicatedTaskCreate is idempotent and detects collisions", async () => {
|
||||
|
||||
@@ -555,7 +555,7 @@ Standard triage must not infer workflow changes from task type. Agents preserve
|
||||
|
||||
## Plan Review
|
||||
|
||||
Workflow Plan Review is the single optional plan quality gate before execution. Your job in triage is to write a complete PROMPT.md; do not call \`fn_review_spec()\` or any other review tool. If Plan Review is enabled for the task, the workflow graph runs it after triage and before execution.
|
||||
Workflow Plan Review is the single optional plan quality gate before execution. Your job in triage is to write a complete PROMPT.md; do not call \`fn_review_spec()\` or any other review tool. If Plan Review is enabled for the task, the triage engine runs it before releasing the task to execution, and the task stays in triage while that review runs.
|
||||
|
||||
## PROMPT.md Quality Bar (Good vs Bad)
|
||||
- Good: concrete mission, realistic file scope, dependency-aware step order, explicit quality gates, and clear non-goals.
|
||||
|
||||
@@ -2200,7 +2200,11 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
})(),
|
||||
breakIntoSubtasks: row.breakIntoSubtasks ? true : undefined,
|
||||
noCommitsExpected: row.noCommitsExpected ? true : undefined,
|
||||
enabledWorkflowSteps: (() => { const e = fromJson<string[]>(row.enabledWorkflowSteps); return e && e.length > 0 ? e : undefined; })(),
|
||||
/*
|
||||
FNXC:WorkflowOptionalSteps 2026-06-29-02:55:
|
||||
Preserve an explicitly empty optional-step selection as `[]`. Quick Add, inline create, and task details use `[]` to mean "the operator disabled every optional workflow group"; converting it back to `undefined` lets later workflow hydration re-seed default-on Plan Review / Code Review and run gates the task opted out of.
|
||||
*/
|
||||
enabledWorkflowSteps: (() => { const e = fromJson<string[]>(row.enabledWorkflowSteps); return Array.isArray(e) ? e : undefined; })(),
|
||||
modifiedFiles: (() => { const m = fromJson<string[]>(row.modifiedFiles); return m && m.length > 0 ? m : undefined; })(),
|
||||
missionId: row.missionId || undefined,
|
||||
sliceId: row.sliceId || undefined,
|
||||
@@ -4612,7 +4616,7 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS}
|
||||
}
|
||||
}
|
||||
} else if (input.enabledWorkflowSteps.length === 0) {
|
||||
resolvedWorkflowSteps = undefined;
|
||||
resolvedWorkflowSteps = [];
|
||||
}
|
||||
|
||||
// U7c: selection seeds are optional-group node ids (not materialized
|
||||
@@ -4807,7 +4811,7 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS}
|
||||
}
|
||||
}
|
||||
} else if (Array.isArray(input.enabledWorkflowSteps) && input.enabledWorkflowSteps.length === 0) {
|
||||
resolvedWorkflowSteps = undefined;
|
||||
resolvedWorkflowSteps = [];
|
||||
}
|
||||
|
||||
// U7c: selection seeds are optional-group node ids (not materialized
|
||||
|
||||
@@ -477,7 +477,11 @@ export function InlineCreateCard({
|
||||
validatorModelId: hasValidatorOverride ? validatorModelId : undefined,
|
||||
planningModelProvider: hasPlanningOverride ? planningProvider : undefined,
|
||||
planningModelId: hasPlanningOverride ? planningModelId : undefined,
|
||||
enabledWorkflowSteps: enabledOptionalStepIds.length ? enabledOptionalStepIds : undefined,
|
||||
/*
|
||||
FNXC:InlineCreateWorkflowSteps 2026-06-29-02:45:
|
||||
Inline create optional-step toggles are explicit task intent. When a workflow exposes optional steps and the operator unchecks all of them, submit `[]` so default-on Plan Review / Code Review stay disabled on the created task instead of reappearing from workflow defaults.
|
||||
*/
|
||||
enabledWorkflowSteps: optionalSteps.length > 0 ? enabledOptionalStepIds : undefined,
|
||||
priority,
|
||||
nodeId,
|
||||
};
|
||||
@@ -494,7 +498,7 @@ export function InlineCreateCard({
|
||||
}
|
||||
|
||||
await submitTask(input);
|
||||
}, [description, submitting, dependencies, selectedAgentId, selectedPresetId, hasExecutorOverride, executorProvider, executorModelId, hasValidatorOverride, validatorProvider, validatorModelId, hasPlanningOverride, planningProvider, planningModelId, enabledOptionalStepIds, priority, nodeId, projectId, addToast, submitTask]);
|
||||
}, [description, submitting, dependencies, selectedAgentId, selectedPresetId, hasExecutorOverride, executorProvider, executorModelId, hasValidatorOverride, validatorProvider, validatorModelId, hasPlanningOverride, planningProvider, planningModelId, optionalSteps.length, enabledOptionalStepIds, priority, nodeId, projectId, addToast, submitTask]);
|
||||
|
||||
const handleDuplicateProceed = useCallback(async () => {
|
||||
const matches = duplicateMatches;
|
||||
|
||||
@@ -978,6 +978,18 @@ The execution-time badge is part of the footer's bottom-right chip cluster, so i
|
||||
flex-shrink: 0;
|
||||
}
|
||||
|
||||
.card-progress-active {
|
||||
flex-shrink: 0;
|
||||
padding: 1px var(--space-xs);
|
||||
border: 1px solid color-mix(in srgb, var(--in-progress) 45%, transparent);
|
||||
border-radius: var(--radius-pill);
|
||||
background: color-mix(in srgb, var(--in-progress) 12%, transparent);
|
||||
color: var(--in-progress);
|
||||
font-size: 0.625rem;
|
||||
font-weight: 650;
|
||||
line-height: 1.2;
|
||||
}
|
||||
|
||||
/* Steps toggle and list */
|
||||
.card-steps-toggle {
|
||||
display: flex;
|
||||
@@ -1082,6 +1094,23 @@ executing. These map 1:1 to the unified progress status so the dot color encodes
|
||||
opacity: 0.6;
|
||||
}
|
||||
|
||||
.card-step-name.active {
|
||||
color: var(--text);
|
||||
font-weight: 650;
|
||||
}
|
||||
|
||||
.card-step-active-badge {
|
||||
flex-shrink: 0;
|
||||
padding: 1px var(--space-xs);
|
||||
border: 1px solid color-mix(in srgb, var(--in-progress) 40%, transparent);
|
||||
border-radius: var(--radius-pill);
|
||||
color: var(--in-progress);
|
||||
font-size: 0.5625rem;
|
||||
font-weight: 650;
|
||||
line-height: 1.2;
|
||||
text-transform: uppercase;
|
||||
}
|
||||
|
||||
.card-step-workflow-badge {
|
||||
margin-left: auto;
|
||||
padding: 0 var(--space-xs);
|
||||
|
||||
@@ -1074,6 +1074,14 @@ function TaskCardComponent({
|
||||
() => getUnifiedTaskProgress(task),
|
||||
[task.steps, task.enabledWorkflowSteps, task.workflowStepResults],
|
||||
);
|
||||
/*
|
||||
FNXC:TaskCardProgress 2026-06-29-02:26:
|
||||
Operators need to see active step work on the card before it becomes `done`. Keep the completed count strict, but surface `in-progress` task steps and running workflow checks as an active badge so card progress does not look stale while execution is underway.
|
||||
*/
|
||||
const activeProgressCount = useMemo(
|
||||
() => unifiedProgress.items.filter((item) => item.status === "in-progress" || item.status === "running").length,
|
||||
[unifiedProgress.items],
|
||||
);
|
||||
const showProgressSection =
|
||||
unifiedProgress.total > 0 && (task.status === "executing" || task.column === "in-progress");
|
||||
|
||||
@@ -2281,6 +2289,11 @@ function TaskCardComponent({
|
||||
/>
|
||||
</div>
|
||||
<span className="card-progress-label">{unifiedProgress.completed}/{unifiedProgress.total}</span>
|
||||
{activeProgressCount > 0 && (
|
||||
<span className="card-progress-active">
|
||||
{t("tasks.activeStepCount", "{{count}} active", { count: activeProgressCount })}
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
<button
|
||||
type="button"
|
||||
@@ -2312,9 +2325,14 @@ function TaskCardComponent({
|
||||
className={`card-step-dot card-step-dot--${step.status}`}
|
||||
aria-hidden="true"
|
||||
/>
|
||||
<span className={`card-step-name${step.status === "done" ? " completed" : ""}`}>
|
||||
<span className={`card-step-name${step.status === "done" ? " completed" : ""}${step.status === "in-progress" || step.status === "running" ? " active" : ""}`}>
|
||||
{step.name}
|
||||
</span>
|
||||
{(step.status === "in-progress" || step.status === "running") && (
|
||||
<span className="card-step-active-badge">
|
||||
{t("tasks.active", "active")}
|
||||
</span>
|
||||
)}
|
||||
{step.source === "workflow" && (
|
||||
<span
|
||||
className={`card-step-workflow-badge card-step-workflow-badge--${step.phase}`}
|
||||
|
||||
@@ -1066,7 +1066,8 @@ describe("InlineCreateCard button visibility when collapsed", () => {
|
||||
expect(document.querySelector(".inline-create-optional-steps")).toBeNull();
|
||||
});
|
||||
|
||||
it("submits with enabledWorkflowSteps undefined", async () => {
|
||||
it("submits with enabledWorkflowSteps undefined when workflow has no optional steps", async () => {
|
||||
vi.mocked(fetchWorkflowOptionalSteps).mockResolvedValueOnce([]);
|
||||
const mockOnSubmit = vi.fn().mockResolvedValue(createMockTask());
|
||||
renderCard([], { onSubmit: mockOnSubmit });
|
||||
expandCard();
|
||||
@@ -1084,6 +1085,42 @@ describe("InlineCreateCard button visibility when collapsed", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("submits an empty enabledWorkflowSteps list when optional steps are all unchecked", async () => {
|
||||
vi.mocked(fetchWorkflowOptionalSteps).mockResolvedValueOnce([
|
||||
{
|
||||
templateId: "plan-review",
|
||||
name: "Plan Review",
|
||||
description: "Review PROMPT.md before execution",
|
||||
icon: "clipboard-check",
|
||||
phase: "pre-merge",
|
||||
defaultOn: true,
|
||||
},
|
||||
]);
|
||||
const mockOnSubmit = vi.fn().mockResolvedValue(createMockTask());
|
||||
renderCard([], { onSubmit: mockOnSubmit });
|
||||
expandCard();
|
||||
|
||||
const trigger = await screen.findByTestId("inline-create-optional-steps-trigger");
|
||||
expect(trigger).toHaveTextContent("Steps: 1 selected");
|
||||
fireEvent.click(trigger);
|
||||
const option = await screen.findByTestId("wf-optional-steps-dropdown-option-plan-review");
|
||||
fireEvent.click(option);
|
||||
expect(trigger).toHaveTextContent("Steps: none");
|
||||
|
||||
fireEvent.change(screen.getByPlaceholderText("What needs to be done?"), {
|
||||
target: { value: "Task without optional review gates" },
|
||||
});
|
||||
fireEvent.click(screen.getByTestId("save-button"));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockOnSubmit).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
enabledWorkflowSteps: [],
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
it("submits browser-verification workflow step when browser verification is enabled", async () => {
|
||||
const mockOnSubmit = vi.fn().mockResolvedValue(createMockTask());
|
||||
renderCard([], { onSubmit: mockOnSubmit });
|
||||
|
||||
@@ -2063,6 +2063,29 @@ describe("TaskCard", () => {
|
||||
expect(screen.getByText("5 steps")).toBeDefined();
|
||||
});
|
||||
|
||||
it("surfaces in-progress implementation steps on the collapsed card", () => {
|
||||
const { container } = render(
|
||||
<TaskCard
|
||||
task={makeTask({
|
||||
column: "in-progress",
|
||||
status: "executing" as any,
|
||||
steps: [
|
||||
{ name: "Step 0", status: "done" },
|
||||
{ name: "Step 1", status: "in-progress" },
|
||||
{ name: "Step 2", status: "pending" },
|
||||
],
|
||||
})}
|
||||
onOpenDetail={noop}
|
||||
addToast={noop}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByText("1/3")).toBeDefined();
|
||||
expect(screen.getByText("1 active")).toBeDefined();
|
||||
expect(screen.getByText("active")).toBeDefined();
|
||||
expect(container.querySelector(".card-step-name.active")?.textContent).toBe("Step 1");
|
||||
});
|
||||
|
||||
it("uses singular step label when unified progress total is one", () => {
|
||||
render(
|
||||
<TaskCard
|
||||
|
||||
@@ -1279,6 +1279,115 @@ describe("TriageProcessor", () => {
|
||||
expect(processor).toBeInstanceOf(TriageProcessor);
|
||||
});
|
||||
|
||||
it("runs enabled Plan Review in triage before moving to todo", async () => {
|
||||
const task = createTriageTask({
|
||||
id: "FN-PLAN-APPROVE",
|
||||
title: "Plan approve",
|
||||
status: "planning",
|
||||
enabledWorkflowSteps: ["plan-review", "code-review"],
|
||||
} as Partial<Task>);
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(task);
|
||||
mockReviewStep.mockResolvedValue({
|
||||
verdict: "APPROVE",
|
||||
review: "### Verdict: APPROVE\n\n### Summary\nReady.",
|
||||
summary: "Ready.",
|
||||
});
|
||||
|
||||
await (processor as unknown as {
|
||||
finalizeApprovedTask(task: Task, writtenInput: string, settings: Settings): Promise<void>;
|
||||
}).finalizeApprovedTask(
|
||||
task,
|
||||
"# Task: FN-PLAN-APPROVE - Plan approve\n\n## Mission\n\nDo it.\n",
|
||||
{ requirePlanApproval: false } as Settings,
|
||||
);
|
||||
|
||||
expect(mockReviewStep).toHaveBeenCalledWith(
|
||||
rootDir,
|
||||
"FN-PLAN-APPROVE",
|
||||
0,
|
||||
"PROMPT.md",
|
||||
"plan",
|
||||
expect.any(String),
|
||||
undefined,
|
||||
expect.objectContaining({ taskId: "FN-PLAN-APPROVE" }),
|
||||
);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-PLAN-APPROVE", expect.objectContaining({
|
||||
workflowStepResults: expect.arrayContaining([
|
||||
expect.objectContaining({ workflowStepId: "plan-review", status: "passed", verdict: "APPROVE" }),
|
||||
]),
|
||||
}));
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-PLAN-APPROVE", "todo");
|
||||
});
|
||||
|
||||
it("keeps the task in triage when Plan Review requests revision", async () => {
|
||||
const task = createTriageTask({
|
||||
id: "FN-PLAN-REVISE",
|
||||
title: "Plan revise",
|
||||
status: "planning",
|
||||
enabledWorkflowSteps: ["plan-review", "code-review"],
|
||||
} as Partial<Task>);
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(task);
|
||||
mockReviewStep.mockResolvedValue({
|
||||
verdict: "REVISE",
|
||||
review: "### Verdict: REVISE\n\n### Issues Found\nMissing verification.",
|
||||
summary: "Missing verification.",
|
||||
});
|
||||
|
||||
await (processor as unknown as {
|
||||
finalizeApprovedTask(task: Task, writtenInput: string, settings: Settings): Promise<void>;
|
||||
}).finalizeApprovedTask(
|
||||
task,
|
||||
"# Task: FN-PLAN-REVISE - Plan revise\n\n## Mission\n\nDo it.\n",
|
||||
{ requirePlanApproval: false } as Settings,
|
||||
);
|
||||
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-PLAN-REVISE", expect.objectContaining({
|
||||
status: "needs-replan",
|
||||
error: null,
|
||||
}));
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-PLAN-REVISE", expect.objectContaining({
|
||||
workflowStepResults: expect.arrayContaining([
|
||||
expect.objectContaining({ workflowStepId: "plan-review", status: "failed", verdict: "REVISE" }),
|
||||
]),
|
||||
}));
|
||||
});
|
||||
|
||||
it("keeps the task in triage with retry backoff when Plan Review is unavailable", async () => {
|
||||
const task = createTriageTask({
|
||||
id: "FN-PLAN-UNAVAILABLE",
|
||||
title: "Plan unavailable",
|
||||
status: "planning",
|
||||
enabledWorkflowSteps: ["plan-review", "code-review"],
|
||||
} as Partial<Task>);
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(task);
|
||||
mockReviewStep.mockRejectedValue(new Error("runtime unavailable"));
|
||||
|
||||
await (processor as unknown as {
|
||||
finalizeApprovedTask(task: Task, writtenInput: string, settings: Settings): Promise<void>;
|
||||
}).finalizeApprovedTask(
|
||||
task,
|
||||
"# Task: FN-PLAN-UNAVAILABLE - Plan unavailable\n\n## Mission\n\nDo it.\n",
|
||||
{ requirePlanApproval: false } as Settings,
|
||||
);
|
||||
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-PLAN-UNAVAILABLE", expect.objectContaining({
|
||||
status: "plan-review-unavailable",
|
||||
error: "Plan Review did not produce a verdict; retrying from triage.",
|
||||
nextRecoveryAt: expect.any(String),
|
||||
}));
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-PLAN-UNAVAILABLE", expect.objectContaining({
|
||||
workflowStepResults: expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
workflowStepId: "plan-review",
|
||||
status: "failed",
|
||||
output: expect.stringContaining("runtime unavailable"),
|
||||
}),
|
||||
]),
|
||||
}));
|
||||
});
|
||||
|
||||
it("includes workflow discovery and selection tools in the full triage toolset", async () => {
|
||||
const task = createTriageTask({ id: "FN-WORKFLOW-TOOLS" });
|
||||
const detailedTask = { ...mockTaskDetail, id: task.id, attachments: [], comments: [] };
|
||||
|
||||
@@ -165,7 +165,7 @@ describe("WorkflowGraphExecutor optional-group", () => {
|
||||
expect(disabledResult.outcome).toBe("success");
|
||||
});
|
||||
|
||||
it("runs default-on optional groups when the task has no explicit optional-step selection", async () => {
|
||||
it("treats defaultOn as a creation-time seed, not an execution-time fallback", async () => {
|
||||
const ir = optionalGroupIr();
|
||||
const group = ir.nodes.find((node) => node.id === "group");
|
||||
if (group?.config) group.config.defaultOn = true;
|
||||
@@ -182,8 +182,8 @@ describe("WorkflowGraphExecutor optional-group", () => {
|
||||
const unsetResult = await executor.run(taskWith(undefined), settingsOn(), ir);
|
||||
const explicitEmptyResult = await executor.run(taskWith([]), settingsOn(), ir);
|
||||
|
||||
expect(calls.filter((id) => id === "optstep")).toHaveLength(1);
|
||||
expect(unsetResult.visitedNodeIds).toContain("group::optstep");
|
||||
expect(calls.filter((id) => id === "optstep")).toHaveLength(0);
|
||||
expect(unsetResult.visitedNodeIds).not.toContain("group::optstep");
|
||||
expect(explicitEmptyResult.visitedNodeIds).not.toContain("group::optstep");
|
||||
});
|
||||
|
||||
@@ -524,6 +524,60 @@ describe("WorkflowGraphExecutor optional-group", () => {
|
||||
]));
|
||||
});
|
||||
|
||||
it("skips Plan Review in the execution graph when triage already passed it", async () => {
|
||||
const requestFix = vi.fn(async () => true);
|
||||
const calls: string[] = [];
|
||||
const logs: string[] = [];
|
||||
const ir: WorkflowIr = {
|
||||
version: "v2",
|
||||
name: "plan-review-already-passed",
|
||||
columns: [{ id: "work", name: "Work", traits: [] }],
|
||||
nodes: [
|
||||
{ id: "start", kind: "start" },
|
||||
{
|
||||
id: "plan-review",
|
||||
kind: "optional-group",
|
||||
config: {
|
||||
name: "Plan Review",
|
||||
defaultOn: true,
|
||||
template: {
|
||||
nodes: [{ id: "plan-review-step", kind: "prompt", config: { prompt: "review plan" } }],
|
||||
edges: [],
|
||||
},
|
||||
},
|
||||
},
|
||||
{ id: "execute", kind: "prompt", config: { prompt: "execute" } },
|
||||
{ id: "end", kind: "end" },
|
||||
],
|
||||
edges: [
|
||||
{ from: "start", to: "plan-review" },
|
||||
{ from: "plan-review", to: "execute", condition: "success" },
|
||||
{ from: "execute", to: "end" },
|
||||
],
|
||||
};
|
||||
const executor = new WorkflowGraphExecutor({
|
||||
handlers: {
|
||||
prompt: async (node) => {
|
||||
calls.push(node.id);
|
||||
return { outcome: "success" };
|
||||
},
|
||||
},
|
||||
logTaskEntry: (summary) => { logs.push(summary); },
|
||||
requestPreMergeOptionalStepFix: requestFix,
|
||||
});
|
||||
|
||||
const result = await executor.run({
|
||||
...taskWith(["plan-review"]),
|
||||
id: "FN-plan-review-passed",
|
||||
workflowStepResults: [{ workflowStepId: "plan-review", workflowStepName: "Plan Review", phase: "pre-merge", status: "passed" }],
|
||||
} as TaskDetail, settingsOn(), ir);
|
||||
|
||||
expect(result.outcome).toBe("success");
|
||||
expect(calls).toEqual(["execute"]);
|
||||
expect(requestFix).not.toHaveBeenCalled();
|
||||
expect(logs).toContain("[pre-merge] Workflow step already passed: Plan Review");
|
||||
});
|
||||
|
||||
it("cycles REVISE findings across graph runs until APPROVE, and falls through only after the budget seam declines", async () => {
|
||||
const verdicts = ["REVISE", "REVISE", "APPROVE"];
|
||||
const requestFix = vi.fn(async () => true);
|
||||
|
||||
@@ -6,9 +6,11 @@ import type {
|
||||
TaskDetail,
|
||||
TaskAttachment,
|
||||
Settings,
|
||||
WorkflowStepResult,
|
||||
} from "@fusion/core";
|
||||
import {
|
||||
DUPLICATE_OF_METADATA_KEY,
|
||||
PLAN_REVIEW_GROUP_ID,
|
||||
TaskDeletedError,
|
||||
buildTriageMemoryInstructions,
|
||||
getTaskDuplicateLineage,
|
||||
@@ -135,6 +137,7 @@ import { archiveAsGhostBug } from "./self-healing.js";
|
||||
import { createRunAuditor, generateSyntheticRunId } from "./run-audit.js";
|
||||
import { resolveAndEmitGoalContext } from "./goal-injection-diagnostics.js";
|
||||
import { accumulateSessionTokenUsage } from "./session-token-usage.js";
|
||||
import { reviewStep } from "./reviewer.js";
|
||||
|
||||
|
||||
export interface TriageProcessorOptions {
|
||||
@@ -1237,7 +1240,7 @@ export class TriageProcessor {
|
||||
|
||||
/*
|
||||
FNXC:PlanReview 2026-06-29-01:52:
|
||||
Workflow Plan Review is the single operator-controlled AI plan gate. Triage must not remind agents to call fn_review_spec or retry planning only because that legacy tool was not approved; the graph runs optional Plan Review before parse/execution and routes failed plans back to triage.
|
||||
Workflow Plan Review is the single operator-controlled AI plan gate. Triage must not remind agents to call fn_review_spec or retry planning only because that legacy tool was not approved; after PROMPT.md is written, triage itself runs optional Plan Review before releasing the task to execution.
|
||||
*/
|
||||
|
||||
const written = await readFile(
|
||||
@@ -1773,6 +1776,146 @@ export class TriageProcessor {
|
||||
return null;
|
||||
}
|
||||
|
||||
private isPlanReviewEnabled(task: Task): boolean {
|
||||
/*
|
||||
FNXC:PlanReview 2026-06-29-02:40:
|
||||
Plan Review is a triage-owned pre-release gate. Task creation materializes default-on optional groups into `enabledWorkflowSteps`; an explicit empty array from Quick Add means the operator disabled every optional group. Use only that materialized list here so triage does not resurrect disabled Plan Review.
|
||||
*/
|
||||
return Array.isArray(task.enabledWorkflowSteps) && task.enabledWorkflowSteps.includes(PLAN_REVIEW_GROUP_ID);
|
||||
}
|
||||
|
||||
private async recordPlanReviewWorkflowResult(task: Task, result: WorkflowStepResult): Promise<void> {
|
||||
const live = await this.store.getTask(task.id).catch(() => task);
|
||||
const existing = Array.isArray(live?.workflowStepResults)
|
||||
? [...live.workflowStepResults]
|
||||
: [];
|
||||
const idx = existing.findIndex((entry) => entry.workflowStepId === PLAN_REVIEW_GROUP_ID);
|
||||
if (idx >= 0) existing[idx] = result;
|
||||
else existing.push(result);
|
||||
await this.store.updateTask(task.id, { workflowStepResults: existing });
|
||||
}
|
||||
|
||||
private async runPlanReviewBeforeExecution(task: Task, promptContent: string, settings: Settings): Promise<"approved" | "blocked"> {
|
||||
if (!this.isPlanReviewEnabled(task)) {
|
||||
return "approved";
|
||||
}
|
||||
|
||||
const alreadyPassed = task.workflowStepResults?.some(
|
||||
(result) => result.workflowStepId === PLAN_REVIEW_GROUP_ID && result.status === "passed",
|
||||
);
|
||||
if (alreadyPassed) {
|
||||
return "approved";
|
||||
}
|
||||
|
||||
const startedAt = new Date().toISOString();
|
||||
await this.recordPlanReviewWorkflowResult(task, {
|
||||
workflowStepId: PLAN_REVIEW_GROUP_ID,
|
||||
workflowStepName: "Plan Review",
|
||||
phase: "pre-merge",
|
||||
status: "pending",
|
||||
startedAt,
|
||||
});
|
||||
await this.store.logEntry(task.id, "[pre-merge] Starting workflow step: Plan Review");
|
||||
|
||||
const review = await reviewStep(
|
||||
this.rootDir,
|
||||
task.id,
|
||||
0,
|
||||
"PROMPT.md",
|
||||
"plan",
|
||||
promptContent,
|
||||
undefined,
|
||||
{
|
||||
store: this.store,
|
||||
taskId: task.id,
|
||||
taskTitle: task.title,
|
||||
settings,
|
||||
task,
|
||||
rootDir: this.rootDir,
|
||||
agentStore: this.options.agentStore,
|
||||
pluginRunner: this.options.pluginRunner,
|
||||
onSessionCreated: (session) => this.registerSubagentSession(task.id, session),
|
||||
onSessionEnded: (session) => this.unregisterSubagentSession(task.id, session),
|
||||
},
|
||||
).catch((error: unknown) => {
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
planLog.warn(`${task.id}: Plan Review unavailable before execution (${message})`);
|
||||
return {
|
||||
verdict: "UNAVAILABLE" as const,
|
||||
review: `Plan Review session failed before producing a verdict: ${message}`,
|
||||
summary: "Plan Review session unavailable.",
|
||||
};
|
||||
});
|
||||
|
||||
const completedAt = new Date().toISOString();
|
||||
if (review.verdict === "APPROVE") {
|
||||
await this.recordPlanReviewWorkflowResult(task, {
|
||||
workflowStepId: PLAN_REVIEW_GROUP_ID,
|
||||
workflowStepName: "Plan Review",
|
||||
phase: "pre-merge",
|
||||
status: "passed",
|
||||
verdict: "APPROVE",
|
||||
output: review.review,
|
||||
notes: review.summary,
|
||||
startedAt,
|
||||
completedAt,
|
||||
});
|
||||
await this.store.logEntry(task.id, "[pre-merge] Workflow step completed: Plan Review", review.summary);
|
||||
return "approved";
|
||||
}
|
||||
|
||||
if (review.verdict === "REVISE" || review.verdict === "RETHINK") {
|
||||
await this.recordPlanReviewWorkflowResult(task, {
|
||||
workflowStepId: PLAN_REVIEW_GROUP_ID,
|
||||
workflowStepName: "Plan Review",
|
||||
phase: "pre-merge",
|
||||
status: "failed",
|
||||
verdict: "REVISE",
|
||||
output: review.review,
|
||||
notes: review.summary,
|
||||
startedAt,
|
||||
completedAt,
|
||||
});
|
||||
await this.store.logEntry(task.id, "[pre-merge] Workflow step failed: Plan Review", review.review);
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
"AI spec revision requested",
|
||||
`Plan Review requested a planning revision before execution.\n\nStatus: ${review.verdict}\nFeedback:\n${review.review || review.summary || "(no feedback captured)"}`,
|
||||
);
|
||||
await this.store.updateTask(task.id, {
|
||||
status: "needs-replan",
|
||||
error: null,
|
||||
recoveryRetryCount: null,
|
||||
nextRecoveryAt: null,
|
||||
});
|
||||
return "blocked";
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:PlanReview 2026-06-29-02:40:
|
||||
UNAVAILABLE means the reviewer session did not produce a usable verdict. Keep the task in triage and retry with backoff; do not fabricate a REVISE or send the planner through another full rewrite loop when no reviewer actually rejected the plan.
|
||||
*/
|
||||
const retryAt = new Date(Date.now() + 30_000).toISOString();
|
||||
const unavailableOutput = review.review || review.summary || "Plan Review was unavailable before producing a verdict.";
|
||||
await this.recordPlanReviewWorkflowResult(task, {
|
||||
workflowStepId: PLAN_REVIEW_GROUP_ID,
|
||||
workflowStepName: "Plan Review",
|
||||
phase: "pre-merge",
|
||||
status: "failed",
|
||||
output: unavailableOutput,
|
||||
notes: review.summary,
|
||||
startedAt,
|
||||
completedAt,
|
||||
});
|
||||
await this.store.logEntry(task.id, "[pre-merge] Workflow step unavailable: Plan Review", unavailableOutput);
|
||||
await this.store.updateTask(task.id, {
|
||||
status: "plan-review-unavailable",
|
||||
error: "Plan Review did not produce a verdict; retrying from triage.",
|
||||
nextRecoveryAt: retryAt,
|
||||
});
|
||||
return "blocked";
|
||||
}
|
||||
|
||||
private async tryFinalizeExplicitDuplicateMarker(
|
||||
task: Task,
|
||||
written: string,
|
||||
@@ -2185,6 +2328,13 @@ export class TriageProcessor {
|
||||
return;
|
||||
}
|
||||
|
||||
const planReviewTask = latestTransitionTask ?? task;
|
||||
const planReviewResult = await this.runPlanReviewBeforeExecution(planReviewTask, written, settings);
|
||||
if (planReviewResult === "blocked") {
|
||||
planLog.log(`${task.id} Plan Review blocked execution — staying in triage`);
|
||||
return;
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:PlanApproval 2026-06-26-00:00:
|
||||
Project planApprovalMode has precedence over the workflow-resolved requirePlanApproval value so operators can force auto-approval or manual approval for every task in this project.
|
||||
|
||||
@@ -558,15 +558,16 @@ export class WorkflowGraphExecutor {
|
||||
* still reaches the same downstream node.
|
||||
*/
|
||||
/*
|
||||
* FNXC:WorkflowOptionalSteps 2026-06-29-02:05:
|
||||
* `enabledWorkflowSteps === undefined` means the task has no explicit
|
||||
* optional-step selection, so default-on workflow nodes must run. An
|
||||
* explicit empty array still means the operator disabled every optional
|
||||
* step from Quick Add or task details.
|
||||
* FNXC:WorkflowOptionalSteps 2026-06-29-02:45:
|
||||
* Optional-group execution is driven only by the task's materialized
|
||||
* `enabledWorkflowSteps` list. Workflow `defaultOn` seeds that list at
|
||||
* task creation/selection time; using it here as a fallback resurrects
|
||||
* unchecked Quick Add steps and makes legacy/in-memory tasks run review
|
||||
* gates that were never explicitly selected.
|
||||
*/
|
||||
const enabled = Array.isArray(task.enabledWorkflowSteps)
|
||||
? task.enabledWorkflowSteps.includes(node.id)
|
||||
: node.config?.defaultOn === true;
|
||||
: false;
|
||||
if (!enabled) {
|
||||
// FNXC:WorkflowOptionalGroup 2026-06-21-16:30: record the group's own
|
||||
// outcome on bypass too (mirrors the enabled path + every other node
|
||||
@@ -597,6 +598,20 @@ export class WorkflowGraphExecutor {
|
||||
const groupName = typeof node.config?.name === "string" && node.config.name.trim()
|
||||
? node.config.name.trim()
|
||||
: node.id;
|
||||
/*
|
||||
FNXC:PlanReview 2026-06-29-02:40:
|
||||
Triage runs Plan Review before releasing a task to execution so the task stays in the triage column during review. When the execution graph later reaches the same optional group, treat an existing passed Plan Review result as satisfied and do not launch a duplicate reviewer session.
|
||||
*/
|
||||
if (
|
||||
node.id === PLAN_REVIEW_GROUP_ID
|
||||
&& task.workflowStepResults?.some(
|
||||
(result) => result.workflowStepId === PLAN_REVIEW_GROUP_ID && result.status === "passed",
|
||||
)
|
||||
) {
|
||||
context[`node:${node.id}:outcome`] = "success";
|
||||
this.deps.logTaskEntry?.("[pre-merge] Workflow step already passed: Plan Review");
|
||||
return await traverseChildren(node, { outcome: "success", value: "already-passed" });
|
||||
}
|
||||
/*
|
||||
* FNXC:WorkflowPostMerge 2026-06-26-09:00:
|
||||
* Phase is read from the optional-group node's `config.phase` (defaults to
|
||||
|
||||
Reference in New Issue
Block a user