diff --git a/.changeset/fn-7228-plan-review-status-repair.md b/.changeset/fn-7228-plan-review-status-repair.md new file mode 100644 index 0000000000..ec15867285 --- /dev/null +++ b/.changeset/fn-7228-plan-review-status-repair.md @@ -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. diff --git a/packages/engine/src/__tests__/executor-pause.test.ts b/packages/engine/src/__tests__/executor-pause.test.ts index 5f645d76cb..7539d0586b 100644 --- a/packages/engine/src/__tests__/executor-pause.test.ts +++ b/packages/engine/src/__tests__/executor-pause.test.ts @@ -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"); diff --git a/packages/engine/src/__tests__/plan-review-unavailable-recovery.test.ts b/packages/engine/src/__tests__/plan-review-unavailable-recovery.test.ts index 58683ca948..07674b3a37 100644 --- a/packages/engine/src/__tests__/plan-review-unavailable-recovery.test.ts +++ b/packages/engine/src/__tests__/plan-review-unavailable-recovery.test.ts @@ -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"; diff --git a/packages/engine/src/__tests__/workflow-graph-optional-group.test.ts b/packages/engine/src/__tests__/workflow-graph-optional-group.test.ts index 362eb8e3e7..e374250021 100644 --- a/packages/engine/src/__tests__/workflow-graph-optional-group.test.ts +++ b/packages/engine/src/__tests__/workflow-graph-optional-group.test.ts @@ -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); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 42d5cd3c58..3bb49d61c7 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -3130,11 +3130,12 @@ export class TaskExecutor { logMessage: string, options?: { preserveVerificationFailureCount?: boolean }, ): Promise { + 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): 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 diff --git a/packages/engine/src/workflow-graph-executor.ts b/packages/engine/src/workflow-graph-executor.ts index 3c956a3a27..bd5e412f7b 100644 --- a/packages/engine/src/workflow-graph-executor.ts +++ b/packages/engine/src/workflow-graph-executor.ts @@ -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, + }; +}