fix(FN-7228): preserve plan review status during execution
This commit is contained in:
7
.changeset/fn-7228-plan-review-status-repair.md
Normal file
7
.changeset/fn-7228-plan-review-status-repair.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Keep Plan Review status visible while tasks execute after restart.
|
||||
category: fix
|
||||
dev: Preserves and repairs plan-review workflowStepResults when merge-state cleanup or old rows erased them.
|
||||
@@ -1625,6 +1625,62 @@ describe("FN-2883 fast-path guards", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("stale merge cleanup preserves passed plan review status while clearing post-execution gates", async () => {
|
||||
const store = createMockStore();
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
const task = {
|
||||
id: "FN-7228",
|
||||
title: "stale merge",
|
||||
description: "desc",
|
||||
column: "in-progress" as const,
|
||||
dependencies: [],
|
||||
steps: [{ name: "Step 0", status: "in-progress" }],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
mergeDetails: { strategy: "manual" },
|
||||
workflowStepResults: [
|
||||
{
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
phase: "pre-merge",
|
||||
status: "passed",
|
||||
},
|
||||
{
|
||||
workflowStepId: "code-review",
|
||||
workflowStepName: "Code Review",
|
||||
phase: "pre-merge",
|
||||
status: "passed",
|
||||
},
|
||||
],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
};
|
||||
store.getTask.mockResolvedValue({ ...task, mergeDetails: null });
|
||||
|
||||
await (executor as any).cleanupMergeStateForReverification(task, "cleanup stale merge state");
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith(
|
||||
"FN-7228",
|
||||
expect.objectContaining({
|
||||
workflowStepResults: [
|
||||
expect.objectContaining({
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
status: "passed",
|
||||
}),
|
||||
],
|
||||
}),
|
||||
);
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith(
|
||||
"FN-7228",
|
||||
expect.objectContaining({
|
||||
workflowStepResults: expect.arrayContaining([
|
||||
expect.objectContaining({ workflowStepId: "code-review" }),
|
||||
]),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("resumeOrphaned does not fast-path completed tasks that still have mergeDetails", async () => {
|
||||
const store = createMockStore();
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
|
||||
@@ -11,6 +11,10 @@ vi.mock("../pi.js", () => ({
|
||||
vi.mock("../agent-session-helpers.js", () => ({
|
||||
createResolvedAgentSession: vi.fn(),
|
||||
extractRuntimeHint: vi.fn().mockReturnValue(undefined),
|
||||
resolveValidatorSessionModel: vi.fn().mockReturnValue({
|
||||
provider: "mock-provider",
|
||||
modelId: "mock-model",
|
||||
}),
|
||||
}));
|
||||
|
||||
import { reviewStep } from "../reviewer.js";
|
||||
|
||||
@@ -165,7 +165,7 @@ describe("WorkflowGraphExecutor optional-group", () => {
|
||||
expect(disabledResult.outcome).toBe("success");
|
||||
});
|
||||
|
||||
it("treats defaultOn as a creation-time seed, not an execution-time fallback", async () => {
|
||||
it("uses defaultOn only when enabledWorkflowSteps is missing, while explicit empty disables", 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(0);
|
||||
expect(unsetResult.visitedNodeIds).not.toContain("group::optstep");
|
||||
expect(calls.filter((id) => id === "optstep")).toHaveLength(1);
|
||||
expect(unsetResult.visitedNodeIds).toContain("group::optstep");
|
||||
expect(explicitEmptyResult.visitedNodeIds).not.toContain("group::optstep");
|
||||
});
|
||||
|
||||
@@ -578,6 +578,75 @@ describe("WorkflowGraphExecutor optional-group", () => {
|
||||
expect(logs).toContain("[pre-merge] Workflow step already passed: Plan Review");
|
||||
});
|
||||
|
||||
it("repairs missing Plan Review result from the latest completed log before execution", async () => {
|
||||
const records: Array<{ workflowStepId: string; status: string; notes?: string }> = [];
|
||||
const calls: string[] = [];
|
||||
const logs: string[] = [];
|
||||
const ir: WorkflowIr = {
|
||||
version: "v2",
|
||||
name: "plan-review-log-repair",
|
||||
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); },
|
||||
recordWorkflowStepResult: async (_taskId, result) => { records.push(result); },
|
||||
});
|
||||
|
||||
const result = await executor.run({
|
||||
...taskWith(["plan-review"]),
|
||||
id: "FN-7228",
|
||||
workflowStepResults: [],
|
||||
log: [
|
||||
{ timestamp: "2026-06-29T10:31:20.000Z", action: "[pre-merge] Workflow step failed: Plan Review" },
|
||||
{
|
||||
timestamp: "2026-06-29T10:34:21.000Z",
|
||||
action: "[pre-merge] Workflow step completed: Plan Review",
|
||||
outcome: "approved after replan",
|
||||
},
|
||||
],
|
||||
} as TaskDetail, settingsOn(), ir);
|
||||
|
||||
expect(result.outcome).toBe("success");
|
||||
expect(calls).toEqual(["execute"]);
|
||||
expect(records).toEqual([
|
||||
expect.objectContaining({
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
status: "passed",
|
||||
notes: "approved after replan",
|
||||
}),
|
||||
]);
|
||||
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);
|
||||
|
||||
@@ -3130,11 +3130,12 @@ export class TaskExecutor {
|
||||
logMessage: string,
|
||||
options?: { preserveVerificationFailureCount?: boolean },
|
||||
): Promise<Task> {
|
||||
const preservedWorkflowStepResults = preservePreExecutionWorkflowStepResults(task.workflowStepResults);
|
||||
await this.store.updateTask(task.id, {
|
||||
mergeDetails: null,
|
||||
mergeRetries: 0,
|
||||
verificationFailureCount: options?.preserveVerificationFailureCount ? task.verificationFailureCount ?? 0 : 0,
|
||||
workflowStepResults: [],
|
||||
workflowStepResults: preservedWorkflowStepResults,
|
||||
});
|
||||
|
||||
const refreshedTask = await this.store.getTask(task.id);
|
||||
@@ -16706,6 +16707,18 @@ function hasNonTerminalWorkflowSteps(task: Pick<TaskDetail, "steps">): boolean {
|
||||
return task.steps.length > 0 && task.steps.some((step) => step.status !== "done" && step.status !== "skipped");
|
||||
}
|
||||
|
||||
function preservePreExecutionWorkflowStepResults(results: Task["workflowStepResults"]): CoreWorkflowStepResult[] {
|
||||
/*
|
||||
* FNXC:WorkflowLifecycle 2026-06-29-03:50:
|
||||
* Reverification cleanup must clear post-implementation verification residue
|
||||
* without erasing pre-execution Plan Review evidence. FN-7228 passed Plan
|
||||
* Review, then stale merge-state cleanup reset `workflowStepResults` to `[]`;
|
||||
* the dashboard showed Plan Review with no status while execution continued and
|
||||
* the graph no longer had durable proof to skip duplicate plan review.
|
||||
*/
|
||||
return (results ?? []).filter((result) => result.workflowStepId === "plan-review");
|
||||
}
|
||||
|
||||
/**
|
||||
* Detect whether the last assistant text output looks like a "pseudo-pause" —
|
||||
* where the agent ended a turn by asking for permission or summarizing progress
|
||||
|
||||
@@ -558,16 +558,17 @@ export class WorkflowGraphExecutor {
|
||||
* still reaches the same downstream node.
|
||||
*/
|
||||
/*
|
||||
* 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.
|
||||
* FNXC:WorkflowOptionalSteps 2026-06-29-03:43:
|
||||
* Distinguish an explicit empty toggle list from a missing one. Quick Add
|
||||
* and task forms persist `[]` when an operator unchecks Plan/Code Review,
|
||||
* so that must keep bypassing the group. Imported/legacy/resumed tasks may
|
||||
* have no `enabledWorkflowSteps` field at all; for those, honor the
|
||||
* workflow-authored `defaultOn` so default Coding still runs Plan Review
|
||||
* before execution and Code Review before merge.
|
||||
*/
|
||||
const enabled = Array.isArray(task.enabledWorkflowSteps)
|
||||
? task.enabledWorkflowSteps.includes(node.id)
|
||||
: false;
|
||||
: node.config?.defaultOn === true;
|
||||
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
|
||||
@@ -612,6 +613,15 @@ export class WorkflowGraphExecutor {
|
||||
this.deps.logTaskEntry?.("[pre-merge] Workflow step already passed: Plan Review");
|
||||
return await traverseChildren(node, { outcome: "success", value: "already-passed" });
|
||||
}
|
||||
const repairedPlanReview = node.id === PLAN_REVIEW_GROUP_ID
|
||||
? recoverPassedPlanReviewFromLatestLog(task)
|
||||
: undefined;
|
||||
if (repairedPlanReview) {
|
||||
await this.recordOptionalGroupStepResult(task.id, repairedPlanReview);
|
||||
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
|
||||
@@ -1136,3 +1146,33 @@ export class WorkflowGraphExecutor {
|
||||
return result;
|
||||
}
|
||||
}
|
||||
|
||||
function recoverPassedPlanReviewFromLatestLog(task: TaskDetail): WorkflowStepResult | undefined {
|
||||
/*
|
||||
* FNXC:WorkflowLifecycle 2026-06-29-03:55:
|
||||
* FN-7228 exposed persisted tasks where Plan Review completed successfully but
|
||||
* later cleanup erased `workflowStepResults`, leaving the dashboard with an
|
||||
* enabled Plan Review step and no status. Trust only the latest Plan Review
|
||||
* terminal log: completed repairs the missing projection; failed still blocks
|
||||
* and reruns/replans through the normal path.
|
||||
*/
|
||||
let latest: { status: "passed" | "failed"; timestamp?: string; outcome?: string } | undefined;
|
||||
for (const entry of task.log ?? []) {
|
||||
if (entry.action === "[pre-merge] Workflow step completed: Plan Review") {
|
||||
latest = { status: "passed", timestamp: entry.timestamp, outcome: entry.outcome };
|
||||
} else if (entry.action === "[pre-merge] Workflow step failed: Plan Review") {
|
||||
latest = { status: "failed", timestamp: entry.timestamp, outcome: entry.outcome };
|
||||
}
|
||||
}
|
||||
if (latest?.status !== "passed") return undefined;
|
||||
return {
|
||||
workflowStepId: PLAN_REVIEW_GROUP_ID,
|
||||
workflowStepName: "Plan Review",
|
||||
phase: "pre-merge",
|
||||
status: "passed",
|
||||
verdict: "APPROVE",
|
||||
...(latest.outcome ? { notes: latest.outcome } : {}),
|
||||
startedAt: latest.timestamp,
|
||||
completedAt: latest.timestamp,
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user