From f1db31374a9d1a7678a2209226ab3683702c0a17 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Tue, 7 Jul 2026 13:46:25 -0700 Subject: [PATCH] FN-7642: emit diagnostic output for dispatch/infra failures in optional-group and CE gate nodes Fixes the code-review/plan-review/CE gate workflow node failing with a blank "(no feedback captured)" message when a dispatch or infra exception (not a reviewer verdict) causes the step to fail. - WorkflowGraphExecutor now synthesizes a non-blank WorkflowStepResult.output when an enabled optional-group (code-review, plan-review, browser-verification) or CE source:"node" skill-gate template node fails via dispatch/infra exception - Diagnostic output is derived from the node::error context-patch key, falling back to the failure value, then a stable sentinel - status, verdict extraction, edge routing, and self-healing's latestFailedPreMergeStep selection are unchanged - Added regression test coverage: workflow-graph-optional-group-no-feedback.test.ts - Added changeset (patch) documenting the fix for Runfusion/Fusion#1946 Files changed: .changeset/fn-7642-code-review-no-feedback-diagnostic.md | 7 + packages/engine/src/__tests__/workflow-graph-optional-group-no-feedback.test.ts | 246 +++++++++++++++++++++ packages/engine/src/workflow-graph-executor.ts | 104 ++++++++- 3 files changed, 355 insertions(+), 2 deletions(-) Fusion-Task-Id: FN-7642 Fusion-Task-Lineage: 1329e907-652f-4230-a945-5a9d7040ae69 Co-authored-by: Fusion (runfusion.ai) --- ...7642-code-review-no-feedback-diagnostic.md | 7 + ...w-graph-optional-group-no-feedback.test.ts | 246 ++++++++++++++++++ .../engine/src/workflow-graph-executor.ts | 104 +++++++- 3 files changed, 355 insertions(+), 2 deletions(-) create mode 100644 .changeset/fn-7642-code-review-no-feedback-diagnostic.md create mode 100644 packages/engine/src/__tests__/workflow-graph-optional-group-no-feedback.test.ts diff --git a/.changeset/fn-7642-code-review-no-feedback-diagnostic.md b/.changeset/fn-7642-code-review-no-feedback-diagnostic.md new file mode 100644 index 0000000000..a6a041fca7 --- /dev/null +++ b/.changeset/fn-7642-code-review-no-feedback-diagnostic.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Code Review/Plan Review/CE gate failures now record a diagnostic instead of "(no feedback captured)". +category: fix +dev: When an enabled optional-group (`code-review`, `plan-review`, `browser-verification`) or CE `source:"node"` skill-gate template node fails via a dispatch/infra exception rather than a reviewer verdict, `WorkflowGraphExecutor` now synthesizes a non-blank `WorkflowStepResult.output` from the underlying `node::error` context-patch key (falling back to the failure `value`, then a stable sentinel) instead of leaving `output`/`notes` field-absent. Fixes Runfusion/Fusion#1946. `status`, verdict extraction, edge routing, and self-healing's `latestFailedPreMergeStep` selection are unchanged. diff --git a/packages/engine/src/__tests__/workflow-graph-optional-group-no-feedback.test.ts b/packages/engine/src/__tests__/workflow-graph-optional-group-no-feedback.test.ts new file mode 100644 index 0000000000..dc038ff1e3 --- /dev/null +++ b/packages/engine/src/__tests__/workflow-graph-optional-group-no-feedback.test.ts @@ -0,0 +1,246 @@ +import { describe, expect, it, vi } from "vitest"; +import type { TaskDetail, WorkflowIr } from "@fusion/core"; + +import { WorkflowGraphExecutor, type WorkflowNodeHandler } from "../workflow-graph-executor.js"; + +/* +FNXC:WorkflowStepResults 2026-07-07-00:00: +Regression coverage for Runfusion/Fusion#1946: a non-verdict optional-group / +`source:"node"` failure (dispatch/infra exception, not a reviewer verdict) must +never be recorded with `status:"failed"` and an absent `output` — the +`(no feedback captured)` signature that stranded cards in `in-review`. These +tests drive `WorkflowGraphExecutor` with a recorder-fake (mirrors +`workflow-graph-optional-group.test.ts` / `builtin-coding-workflow-step-results.test.ts`) +and assert the synthesized diagnostic `output`, while control cases prove +genuine verdicts and disabled groups are byte-inert. +*/ + +const settingsOn = () => ({ experimentalFeatures: { workflowGraphExecutor: true } }); + +function taskWith(enabled: string[] | undefined): TaskDetail { + return { id: "FN-NFC", enabledWorkflowSteps: enabled } as TaskDetail; +} + +/** A single-node `code-review` optional-group between start/end, configurable phase. */ +function codeReviewGroupIr(options: { phase?: "pre-merge" | "post-merge" } = {}): WorkflowIr { + return { + version: "v2", + name: "code-review-no-feedback-test", + columns: [{ id: "work", name: "Work", traits: [] }], + nodes: [ + { id: "start", kind: "start" }, + { + id: "code-review", + kind: "optional-group", + config: { + name: "Code Review", + defaultOn: true, + phase: options.phase, + template: { + nodes: [{ id: "review", kind: "prompt", config: { prompt: "review" } }], + edges: [], + }, + }, + }, + { id: "after", kind: "prompt", config: { prompt: "after" } }, + { id: "end", kind: "end" }, + ], + edges: [ + { from: "start", to: "code-review" }, + { from: "code-review", to: "after", condition: "success" }, + { from: "code-review", to: "end", condition: "failure" }, + { from: "after", to: "end" }, + ], + }; +} + +/** A single top-level `gate` node (CE `source:"node"` skill gate) — no optional-group wrapper. */ +function nodeGateIr(): WorkflowIr { + return { + version: "v2", + name: "node-gate-no-feedback-test", + columns: [{ id: "work", name: "Work", traits: [] }], + nodes: [ + { id: "start", kind: "start" }, + { id: "gatecheck", kind: "gate", config: { prompt: "check", skillName: "security-gate" } }, + { id: "end", kind: "end" }, + ], + edges: [ + { from: "start", to: "gatecheck" }, + { from: "gatecheck", to: "end" }, + ], + }; +} + +describe("workflow-graph-executor: non-verdict failure diagnostic (Runfusion/Fusion#1946)", () => { + it("SYMPTOM: a code-review dispatch exception records a non-empty diagnostic output, never (no feedback captured)", async () => { + const records: Array> = []; + const handler: WorkflowNodeHandler = async (node) => { + if (node.id === "review") throw new Error("model provider dispatch failed"); + return { outcome: "success" }; + }; + const executor = new WorkflowGraphExecutor({ + handlers: { prompt: handler }, + maxRetriesPerNode: 3, + recordWorkflowStepResult: async (_taskId, result) => { records.push(result as unknown as Record); }, + }); + + await executor.run(taskWith(["code-review"]), settingsOn(), codeReviewGroupIr()); + + const terminal = records.find((r) => r.workflowStepId === "code-review" && r.status === "failed"); + expect(terminal).toBeDefined(); + expect(terminal?.verdict).toBeUndefined(); + expect(typeof terminal?.output).toBe("string"); + expect((terminal?.output as string).length).toBeGreaterThan(0); + expect(terminal?.output).toContain("model provider dispatch failed"); + expect(terminal?.output).not.toBe("(no feedback captured)"); + }); + + it("SYMPTOM (post-merge phase): a post-merge optional-group dispatch exception also records a diagnostic output", async () => { + const records: Array> = []; + const handler: WorkflowNodeHandler = async (node) => { + if (node.id === "review") throw new Error("session dispatch race"); + return { outcome: "success" }; + }; + const executor = new WorkflowGraphExecutor({ + handlers: { prompt: handler }, + maxRetriesPerNode: 3, + recordWorkflowStepResult: async (_taskId, result) => { records.push(result as unknown as Record); }, + }); + + await executor.run(taskWith(["code-review"]), settingsOn(), codeReviewGroupIr({ phase: "post-merge" })); + + const terminal = records.find((r) => r.workflowStepId === "code-review" && r.status === "failed"); + expect(terminal).toBeDefined(); + expect(terminal?.phase).toBe("post-merge"); + expect(terminal?.output).toContain("session dispatch race"); + }); + + it("SYMPTOM (source:'node'): a CE skill-gate node exception records a diagnostic output, not a field-absent failure", async () => { + const records: Array> = []; + const handler: WorkflowNodeHandler = async () => { + throw new Error("gate dispatch exploded"); + }; + const executor = new WorkflowGraphExecutor({ + handlers: { gate: handler }, + maxRetriesPerNode: 2, + recordWorkflowStepResult: async (_taskId, result) => { records.push(result as unknown as Record); }, + }); + + await executor.run(taskWith(undefined), settingsOn(), nodeGateIr()); + + const terminal = records.find((r) => r.workflowStepId === "gatecheck" && r.status === "failed"); + expect(terminal).toBeDefined(); + expect(terminal?.status).toBe("failed"); + expect(terminal?.source).toBe("node"); + expect(typeof terminal?.output).toBe("string"); + expect((terminal?.output as string).length).toBeGreaterThan(0); + expect(terminal?.output).toContain("gate dispatch exploded"); + }); + + it("SURFACE: an 'aborted' failure value with no recoverable error text still yields a non-blank fallback output", async () => { + const records: Array> = []; + // The template node itself directly returns a failure with a bare `value` and + // no `contextPatch` (the same shape `runOptionalGroup`/`executeNodeWithRetries` + // produce for a mid-retry abort) — no recoverable `:error` text anywhere. + const handler: WorkflowNodeHandler = async (node) => + node.id === "review" ? { outcome: "failure", value: "aborted" } : { outcome: "success" }; + const executor = new WorkflowGraphExecutor({ + handlers: { prompt: handler }, + recordWorkflowStepResult: async (_taskId, result) => { records.push(result as unknown as Record); }, + }); + + await executor.run(taskWith(["code-review"]), settingsOn(), codeReviewGroupIr()); + + const terminal = records.find((r) => r.workflowStepId === "code-review" && r.status === "failed"); + expect(terminal).toBeDefined(); + expect(typeof terminal?.output).toBe("string"); + expect((terminal?.output as string).trim().length).toBeGreaterThan(0); + }); + + it("CONTROL: a genuine REVISE verdict keeps its verdict + populated output unchanged (not overwritten by the diagnostic path)", async () => { + const records: Array> = []; + const handler: WorkflowNodeHandler = async (node) => + node.id === "review" + ? { outcome: "failure", value: "REVISE", contextPatch: { output: "Please add tests for the edge case" } } + : { outcome: "success" }; + const executor = new WorkflowGraphExecutor({ + handlers: { prompt: handler }, + recordWorkflowStepResult: async (_taskId, result) => { records.push(result as unknown as Record); }, + }); + + await executor.run(taskWith(["code-review"]), settingsOn(), codeReviewGroupIr()); + + const terminal = records.find((r) => r.workflowStepId === "code-review" && r.status === "failed"); + expect(terminal).toBeDefined(); + expect(terminal?.verdict).toBe("REVISE"); + expect(terminal?.output).toBe("Please add tests for the edge case"); + }); + + it("CONTROL: APPROVE and APPROVE_WITH_NOTES verdicts are unchanged", async () => { + for (const verdict of ["APPROVE", "APPROVE_WITH_NOTES"] as const) { + const records: Array> = []; + const handler: WorkflowNodeHandler = async (node) => + node.id === "review" + ? { outcome: "success", value: verdict, contextPatch: { output: `${verdict} notes` } } + : { outcome: "success" }; + const executor = new WorkflowGraphExecutor({ + handlers: { prompt: handler }, + recordWorkflowStepResult: async (_taskId, result) => { records.push(result as unknown as Record); }, + }); + + await executor.run(taskWith(["code-review"]), settingsOn(), codeReviewGroupIr()); + + const terminal = records.find((r) => r.workflowStepId === "code-review" && r.status !== "pending"); + expect(terminal).toBeDefined(); + expect(terminal?.verdict).toBe(verdict); + expect(terminal?.output).toBe(`${verdict} notes`); + } + }); + + it("CONTROL: a disabled code-review group records nothing (byte-inert)", async () => { + const records: Array> = []; + const handler: WorkflowNodeHandler = async (node) => { + if (node.id === "review") throw new Error("should never run"); + return { outcome: "success" }; + }; + const executor = new WorkflowGraphExecutor({ + handlers: { prompt: handler }, + recordWorkflowStepResult: async (_taskId, result) => { records.push(result as unknown as Record); }, + }); + + // defaultOn: true but not explicitly enabled — group defaults follow the + // fixture's `enabledWorkflowSteps` gate the same way as the sibling suite. + const ir = codeReviewGroupIr(); + (ir.nodes.find((n) => n.id === "code-review")!.config as { defaultOn?: boolean }).defaultOn = false; + + const result = await executor.run(taskWith(undefined), settingsOn(), ir); + + expect(records.filter((r) => r.workflowStepId === "code-review")).toHaveLength(0); + expect(result.outcome).toBe("success"); + }); + + it("keeps status/verdict/edge-routing untouched so self-healing's status==='failed' selection is unaffected", async () => { + const records: Array> = []; + const calls: string[] = []; + const handler: WorkflowNodeHandler = async (node) => { + calls.push(node.id); + if (node.id === "review") throw new Error("dispatch race"); + return { outcome: "success" }; + }; + const executor = new WorkflowGraphExecutor({ + handlers: { prompt: handler }, + maxRetriesPerNode: 2, + recordWorkflowStepResult: async (_taskId, result) => { records.push(result as unknown as Record); }, + }); + + await executor.run(taskWith(["code-review"]), settingsOn(), codeReviewGroupIr()); + + // The failure edge routes to `end`, not the success edge to `after` — + // `self-healing.ts`'s `latestFailedPreMergeStep` relies on this same + // `status:"failed"` signal; only `output` gained a diagnostic, nothing else. + expect(calls).not.toContain("after"); + const terminal = records.find((r) => r.workflowStepId === "code-review" && r.status === "failed"); + expect(terminal?.status).toBe("failed"); + }); +}); diff --git a/packages/engine/src/workflow-graph-executor.ts b/packages/engine/src/workflow-graph-executor.ts index fb7483113f..0d4e6c7cb0 100644 --- a/packages/engine/src/workflow-graph-executor.ts +++ b/packages/engine/src/workflow-graph-executor.ts @@ -726,8 +726,32 @@ export class WorkflowGraphExecutor { else if (verdict === "REVISE") stepStatus = "advisory_failure"; else stepStatus = "passed"; const exitContextPatch = exitResult?.contextPatch; - const stepOutput = typeof exitContextPatch?.output === "string" ? exitContextPatch.output : undefined; + let stepOutput = typeof exitContextPatch?.output === "string" ? exitContextPatch.output : undefined; const stepNotes = typeof exitContextPatch?.notes === "string" ? exitContextPatch.notes : undefined; + /* + * FNXC:WorkflowStepResults 2026-07-07-00:00: + * A non-verdict `stepStatus === "failed"` (dispatch/infra exception, not a + * reviewer verdict) with no `output`/`notes` recovered from the exit + * context-patch must never be recorded field-absent — that is the + * `(no feedback captured)` signature from Runfusion/Fusion#1946. Synthesize + * a diagnostic from the template node's `node::error` context-patch key + * (derived from the last visited template node id) with a fallback to the + * failure `value` (e.g. "exception", "aborted"). Genuine REVISE/APPROVE + * records already carry `stepOutput`/`stepNotes` and are unaffected. + */ + if (stepStatus === "failed" && !verdict && stepOutput === undefined && stepNotes === undefined) { + const lastVisited = groupResult.visitedNodeIds[groupResult.visitedNodeIds.length - 1]; + const templateNodeId = lastVisited?.includes("::") ? lastVisited.slice(lastVisited.indexOf("::") + 2) : undefined; + stepOutput = this.synthesizeNonVerdictFailureOutput({ + stepLabel: groupName, + contextPatch: exitContextPatch, + templateNodeId, + failureValue: verdictRaw, + fallbackText: node.id === PLAN_REVIEW_GROUP_ID + ? "Plan Review failed before execution. Re-run triage to revise PROMPT.md before implementation continues." + : undefined, + }); + } await this.recordOptionalGroupStepResult(task.id, { workflowStepId: node.id, workflowStepName: groupName, @@ -1059,6 +1083,67 @@ export class WorkflowGraphExecutor { } } + /* + * FNXC:WorkflowStepResults 2026-07-07-00:00: + * A hard optional-group / node-gate failure that originates from a dispatch or + * infra exception (rather than a reviewer verdict) must never be persisted as a + * `WorkflowStepResult` with `verdict`/`output`/`notes` entirely absent — that is + * the field-absent `(no feedback captured)` signature reported in + * Runfusion/Fusion#1946 (3 confirmed instances: card stranded in `in-review`, + * ~3s duration from `executeNodeWithRetries` exhausting fast dispatch retries, + * no reviewer-agent error). `executeNodeWithRetries` stores the underlying error + * text under a `node::error` context-patch key (see the + * `plugin-node-handler-error`/`exception` failure branches ~line 1140/1238), but + * the terminal recorder previously derived `output`/`notes` only from + * `contextPatch.output`/`.notes`, silently dropping that diagnostic. This helper + * synthesizes a non-blank diagnostic `output` from the `:error`-suffixed patch + * key (preferring the exact `node::error` key when the template + * node id is known) with a fallback to the failure `value` (e.g. `"exception"`, + * `"aborted"`, `"plugin-node-handler-error"`) and finally a stable non-blank + * fallback sentence — never an empty string. Shared by the optional-group + * terminal recorder and `recordNodeProgressFinish` (CE `source:"node"` skill + * gates) so both surfaces carry the identical guarantee. `status` (`"failed"`), + * verdict extraction, edge routing, and `self-healing.ts`'s + * `latestFailedPreMergeStep` (which filters on `status === "failed"`) are + * untouched — this only adds `output` to an already-failed record. + */ + private synthesizeNonVerdictFailureOutput(params: { + stepLabel: string; + contextPatch?: Record; + templateNodeId?: string; + failureValue?: string; + /** + * FNXC:WorkflowStepResults 2026-07-07-00:00: + * `PLAN_REVIEW_GROUP_ID`'s existing hard-failure handoff (~line 802) already + * has a dedicated, non-blank sentinel ("Plan Review failed before execution...") + * for the fully-unrecoverable case (no `:error` key, no failure `value`). Pass + * it through so the recorded `output` and the pre-merge fix `feedback` stay + * byte-identical for that path when no real diagnostic exists to surface; + * every other caller keeps the generic fallback sentence. + */ + fallbackText?: string; + }): string { + const { stepLabel, contextPatch, templateNodeId, failureValue, fallbackText } = params; + let errorText: string | undefined; + if (contextPatch) { + if (templateNodeId) { + const exact = contextPatch[`node:${templateNodeId}:error`]; + if (typeof exact === "string" && exact.trim()) errorText = exact.trim(); + } + if (!errorText) { + for (const [key, value] of Object.entries(contextPatch)) { + if (key.endsWith(":error") && typeof value === "string" && value.trim()) { + errorText = value.trim(); + break; + } + } + } + } + const detail = errorText || (typeof failureValue === "string" && failureValue.trim() ? failureValue.trim() : undefined); + if (detail) return `${stepLabel} failed before producing a verdict: ${detail}`; + return fallbackText || "Workflow step failed before producing a verdict (no reviewer output captured)."; + } + /* * FNXC:WorkflowStepResults 2026-06-25-12:00: * Fail-soft forward to the `recordWorkflowStepResult` persistence sink (plan U2). @@ -1280,8 +1365,23 @@ export class WorkflowGraphExecutor { ): Promise { const status: WorkflowStepResult["status"] = nodeResult.outcome === "success" ? "passed" : "failed"; const contextPatch = nodeResult.contextPatch ?? {}; - const output = typeof contextPatch.output === "string" ? contextPatch.output : undefined; + let output = typeof contextPatch.output === "string" ? contextPatch.output : undefined; const notes = typeof contextPatch.notes === "string" ? contextPatch.notes : undefined; + /* + * FNXC:WorkflowStepResults 2026-07-07-00:00: + * CE `source:"node"` skill-gate failures share the same `(no feedback + * captured)` defect as the optional-group path (Runfusion/Fusion#1946): a + * `failed` node with no `contextPatch.output`/`.notes` must still record a + * non-blank diagnostic sourced from `node::error`. + */ + if (status === "failed" && output === undefined && notes === undefined) { + output = this.synthesizeNonVerdictFailureOutput({ + stepLabel: this.workflowNodeProgressName(node), + contextPatch, + templateNodeId: node.id, + failureValue: nodeResult.value, + }); + } await this.recordOptionalGroupStepResult(taskId, { workflowStepId: node.id, workflowStepName: this.workflowNodeProgressName(node),