diff --git a/.changeset/workflow-transition-notifications.md b/.changeset/workflow-transition-notifications.md new file mode 100644 index 0000000000..48490e68ec --- /dev/null +++ b/.changeset/workflow-transition-notifications.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Harden workflow lifecycle recovery, post-merge gates, warnings, and notifications. +category: fix +dev: Adds post-merge gate blocking, lifecycle warning analysis, recovery-route audit metadata, and workflow transition notification classification. diff --git a/packages/core/src/__tests__/builtin-workflows.test.ts b/packages/core/src/__tests__/builtin-workflows.test.ts index 2f5058ef7b..51ff8c235f 100644 --- a/packages/core/src/__tests__/builtin-workflows.test.ts +++ b/packages/core/src/__tests__/builtin-workflows.test.ts @@ -113,6 +113,37 @@ describe("built-in workflows", () => { } }); + it("engineering built-in review failures loop through graph-owned remediation", () => { + const expectedLoops = [ + { gate: "plan-review", remediation: "plan-replan" }, + { gate: "browser-verification", remediation: "browser-verification-remediation" }, + { gate: "code-review", remediation: "code-review-remediation" }, + ]; + + for (const workflow of BUILTIN_WORKFLOWS) { + const nodeIds = new Set(workflow.ir.nodes.map((node) => node.id)); + if (!expectedLoops.some(({ gate }) => nodeIds.has(gate))) continue; + + for (const { gate, remediation } of expectedLoops) { + if (!nodeIds.has(gate)) continue; + expect(workflow.ir.edges, `${workflow.id}:${gate}:failure`).toEqual( + expect.arrayContaining([ + expect.objectContaining({ from: gate, to: remediation, condition: "failure" }), + ]), + ); + expect(workflow.ir.edges, `${workflow.id}:${remediation}:return`).toEqual( + expect.arrayContaining([ + expect.objectContaining({ from: remediation, to: gate, condition: "success", kind: "rework" }), + ]), + ); + expect(workflow.ir.nodes.find((node) => node.id === gate)?.config, `${workflow.id}:${gate}:reworkRegion`).toMatchObject({ + reworkRegion: true, + maxReworkCycles: 3, + }); + } + } + }); + it("all built-in workflows generate a task completion summary as a graph node", () => { for (const workflow of BUILTIN_WORKFLOWS) { if (workflow.kind === "fragment") continue; @@ -124,6 +155,38 @@ describe("built-in workflows", () => { } }); + it("merge-capable built-ins expose a default-off post-merge verification node after merge proof", () => { + for (const workflow of BUILTIN_WORKFLOWS) { + if (workflow.kind === "fragment") continue; + const mergeNode = workflow.ir.nodes.find((node) => node.id === "merge-attempt" || node.id === "merge"); + if (!mergeNode) continue; + + const postMerge = workflow.ir.nodes.find((node) => node.id === "post-merge-verification"); + expect(postMerge?.kind, workflow.id).toBe("optional-group"); + expect(postMerge?.config, workflow.id).toMatchObject({ + phase: "post-merge", + defaultOn: false, + }); + const template = postMerge?.config?.template as { nodes?: Array<{ config?: Record }> } | undefined; + expect(template?.nodes?.[0]?.config?.gateMode, workflow.id).toBe("gate"); + expect(workflow.ir.edges, `${workflow.id}:post-merge-entry`).toEqual( + expect.arrayContaining([ + expect.objectContaining({ from: mergeNode.id, to: "post-merge-verification", condition: "success" }), + ]), + ); + if (mergeNode.id === "merge-attempt") { + expect(workflow.ir.edges, `${workflow.id}:no-direct-merge-end`).not.toEqual( + expect.arrayContaining([ + expect.objectContaining({ from: "merge-attempt", to: "end", condition: "success" }), + ]), + ); + } + expect(workflow.ir.nodes.map((node) => node.id).indexOf("post-merge-verification"), workflow.id).toBeGreaterThan( + workflow.ir.nodes.map((node) => node.id).indexOf(mergeNode.id), + ); + } + }); + it("built-in workflow layouts cover every authored node", () => { for (const workflow of BUILTIN_WORKFLOWS) { const missingLayoutNodes = workflow.ir.nodes @@ -524,6 +587,7 @@ describe("built-in workflows", () => { "review", "completion-summary", "merge", + "post-merge-verification", "plan-replan", "browser-verification-remediation", "code-review-remediation", @@ -757,6 +821,7 @@ describe("built-in workflows", () => { "resolve-feedback", "completion-summary", "merge", + "post-merge-verification", "document", "plan-replan", "browser-verification-remediation", diff --git a/packages/core/src/__tests__/workflow-definition-store.test.ts b/packages/core/src/__tests__/workflow-definition-store.test.ts index b9a831c7e4..0b30a486de 100644 --- a/packages/core/src/__tests__/workflow-definition-store.test.ts +++ b/packages/core/src/__tests__/workflow-definition-store.test.ts @@ -52,6 +52,43 @@ describe("TaskStore workflow definitions (U1)", () => { expect(userList[0].layout.lint).toEqual({ x: 120, y: 0 }); }); + it("returns non-blocking lifecycle warnings for custom full workflows", async () => { + const created = await store.createWorkflowDefinition({ + name: "Unsafe terminal", + ir: makeIr({ + nodes: [ + { id: "start", kind: "start" }, + { id: "execute", kind: "prompt", config: { seam: "execute" } }, + { id: "end", kind: "end" }, + ], + edges: [ + { from: "start", to: "execute" }, + { from: "execute", to: "end", condition: "success" }, + ], + }), + }); + + expect(created.lifecycleWarnings?.map((warning) => warning.code)).toEqual(expect.arrayContaining([ + "missing-completion-summary", + "missing-merge-region", + ])); + const reloaded = await store.getWorkflowDefinition(created.id); + expect(reloaded?.lifecycleWarnings?.map((warning) => warning.code)).toEqual(expect.arrayContaining([ + "missing-completion-summary", + "missing-merge-region", + ])); + }); + + it("does not add lifecycle warnings to fragment definitions", async () => { + const created = await store.createWorkflowDefinition({ + name: "Fragment prompt", + kind: "fragment", + ir: makeIr(), + }); + + expect(created.lifecycleWarnings).toEqual([]); + }); + it("rejects a workflow whose IR is missing start/end", async () => { const bad = makeIr({ nodes: [{ id: "only", kind: "prompt" }], edges: [] }); await expect( diff --git a/packages/core/src/__tests__/workflow-lifecycle-validation.test.ts b/packages/core/src/__tests__/workflow-lifecycle-validation.test.ts new file mode 100644 index 0000000000..7267c2a3e9 --- /dev/null +++ b/packages/core/src/__tests__/workflow-lifecycle-validation.test.ts @@ -0,0 +1,114 @@ +import { describe, expect, it } from "vitest"; +import { analyzeWorkflowLifecycle, type WorkflowIr } from "../index.js"; + +function baseIr(nodes: WorkflowIr["nodes"], edges: WorkflowIr["edges"]): WorkflowIr { + return { + version: "v2", + name: "lifecycle-validation-test", + columns: [ + { id: "todo", name: "Todo", traits: [] }, + { id: "in-progress", name: "In progress", traits: [] }, + { id: "in-review", name: "In review", traits: [] }, + { id: "done", name: "Done", traits: [] }, + ], + nodes, + edges, + }; +} + +describe("analyzeWorkflowLifecycle", () => { + it("warns when a full custom workflow omits summary and merge lifecycle primitives", () => { + const warnings = analyzeWorkflowLifecycle(baseIr( + [ + { id: "start", kind: "start", column: "todo" }, + { id: "execute", kind: "prompt", column: "in-progress", config: { seam: "execute" } }, + { id: "end", kind: "end", column: "done" }, + ], + [ + { from: "start", to: "execute" }, + { from: "execute", to: "end", condition: "success" }, + ], + )); + + expect(warnings.map((warning) => warning.code)).toEqual(expect.arrayContaining([ + "missing-completion-summary", + "missing-merge-region", + ])); + }); + + it("warns about terminal success paths that bypass the merge region", () => { + const warnings = analyzeWorkflowLifecycle(baseIr( + [ + { id: "start", kind: "start", column: "todo" }, + { id: "execute", kind: "prompt", column: "in-progress", config: { seam: "execute" } }, + { id: "completion-summary", kind: "prompt", column: "in-review", config: { summaryTarget: "task" } }, + { id: "merge-gate", kind: "merge-gate", column: "in-review" }, + { id: "end", kind: "end", column: "done" }, + ], + [ + { from: "start", to: "execute" }, + { from: "execute", to: "completion-summary", condition: "success" }, + { from: "completion-summary", to: "merge-gate", condition: "success" }, + { from: "merge-gate", to: "end", condition: "success" }, + { from: "execute", to: "end", condition: "success" }, + ], + )); + + expect(warnings).toEqual(expect.arrayContaining([ + expect.objectContaining({ code: "unsafe-terminal-before-merge", nodeId: "execute" }), + ])); + }); + + it("warns when Plan Review is placed after execution and blocking gates lack failure routes", () => { + const warnings = analyzeWorkflowLifecycle(baseIr( + [ + { id: "start", kind: "start", column: "todo" }, + { id: "execute", kind: "prompt", column: "in-progress", config: { seam: "execute" } }, + { + id: "plan-review", + kind: "optional-group", + column: "in-progress", + config: { + name: "Plan Review", + defaultOn: true, + template: { + nodes: [{ id: "plan-review-step", kind: "prompt", config: { gateMode: "gate" } }], + edges: [], + }, + }, + }, + { id: "completion-summary", kind: "prompt", column: "in-review", config: { summaryTarget: "task" } }, + { id: "merge-gate", kind: "merge-gate", column: "in-review" }, + { id: "end", kind: "end", column: "done" }, + ], + [ + { from: "start", to: "execute" }, + { from: "execute", to: "plan-review", condition: "success" }, + { from: "plan-review", to: "completion-summary", condition: "success" }, + { from: "completion-summary", to: "merge-gate", condition: "success" }, + { from: "merge-gate", to: "end", condition: "success" }, + ], + )); + + expect(warnings).toEqual(expect.arrayContaining([ + expect.objectContaining({ code: "optional-group-after-execution", nodeId: "plan-review" }), + expect.objectContaining({ code: "review-gate-without-failure-route", nodeId: "plan-review" }), + ])); + }); + + it("does not warn for fragment templates", () => { + const warnings = analyzeWorkflowLifecycle(baseIr( + [ + { id: "start", kind: "start", column: "todo" }, + { id: "fragment-node", kind: "prompt", column: "todo", config: { prompt: "Reusable" } }, + { id: "end", kind: "end", column: "todo" }, + ], + [ + { from: "start", to: "fragment-node" }, + { from: "fragment-node", to: "end" }, + ], + ), { kind: "fragment" }); + + expect(warnings).toEqual([]); + }); +}); diff --git a/packages/core/src/builtin-browser-verification-group.ts b/packages/core/src/builtin-browser-verification-group.ts index f5eb3f3911..f684f72ca6 100644 --- a/packages/core/src/builtin-browser-verification-group.ts +++ b/packages/core/src/builtin-browser-verification-group.ts @@ -95,6 +95,8 @@ export function browserVerificationOptionalGroupNode( config: { name: BROWSER_VERIFICATION_NAME, defaultOn: options.defaultOn ?? false, + reworkRegion: true, + maxReworkCycles: 3, template: { nodes: [ { diff --git a/packages/core/src/builtin-code-review-group.ts b/packages/core/src/builtin-code-review-group.ts index e0edaa23ca..403151d17b 100644 --- a/packages/core/src/builtin-code-review-group.ts +++ b/packages/core/src/builtin-code-review-group.ts @@ -91,6 +91,8 @@ export function codeReviewOptionalGroupNode( // Default-ON: runs for every coding task by default, but operators can toggle it // off per task (remove `code-review` from enabledWorkflowSteps). defaultOn: options.defaultOn ?? true, + reworkRegion: true, + maxReworkCycles: 3, template: { nodes: [ { diff --git a/packages/core/src/builtin-coding-workflow-ir.ts b/packages/core/src/builtin-coding-workflow-ir.ts index bf2395a436..19c1ab10f1 100644 --- a/packages/core/src/builtin-coding-workflow-ir.ts +++ b/packages/core/src/builtin-coding-workflow-ir.ts @@ -5,6 +5,7 @@ import { builtinPromptConfig } from "./builtin-workflow-prompts.js"; import { browserVerificationOptionalGroupNode } from "./builtin-browser-verification-group.js"; import { codeReviewOptionalGroupNode } from "./builtin-code-review-group.js"; import { completionSummaryNode } from "./builtin-completion-summary-node.js"; +import { postMergeVerificationOptionalGroupNode } from "./builtin-post-merge-group.js"; import { planReviewOptionalGroupNode } from "./builtin-plan-review-group.js"; import { browserVerificationRemediationNode, @@ -112,6 +113,7 @@ const RAW_BUILTIN_CODING_WORKFLOW_IR: WorkflowIr = { config: { capability: "task-merge", reworkRegion: true, maxReworkCycles: 3 }, }, { id: "recovery-router", kind: "recovery-router", column: "in-review", config: { surfaces: ["merge", "retry"] } }, + postMergeVerificationOptionalGroupNode("done"), { id: "end", kind: "end", column: "done" }, ], edges: [ @@ -135,15 +137,19 @@ const RAW_BUILTIN_CODING_WORKFLOW_IR: WorkflowIr = { { from: "branch-group-member-integration", to: "merge-manual-hold", condition: "outcome:manual-required" }, { from: "branch-group-promotion", to: "merge-attempt", condition: "success" }, { from: "branch-group-promotion", to: "merge-manual-hold", condition: "outcome:manual-required" }, - { from: "merge-attempt", to: "end", condition: "success" }, + { from: "merge-attempt", to: "post-merge-verification", condition: "success" }, + { from: "post-merge-verification", to: "end", condition: "success" }, { from: "merge-attempt", to: "merge-retry", condition: "outcome:transient-failure" }, { from: "merge-attempt", to: "merge-manual-hold", condition: "outcome:manual-required" }, { from: "recovery-router", to: "merge-attempt", condition: "outcome:wake-merge", kind: "rework" }, { from: "planning", to: "end", condition: "failure" }, { from: "plan-review", to: "plan-replan", condition: "failure" }, + { from: "plan-replan", to: "plan-review", condition: "success", kind: "rework" }, { from: "execute", to: "end", condition: "failure" }, { from: "browser-verification", to: "browser-verification-remediation", condition: "failure" }, + { from: "browser-verification-remediation", to: "browser-verification", condition: "success", kind: "rework" }, { from: "code-review", to: "code-review-remediation", condition: "failure" }, + { from: "code-review-remediation", to: "code-review", condition: "success", kind: "rework" }, { from: "review", to: "end", condition: "failure" }, { from: "merge-attempt", to: "end", condition: "failure" }, ], diff --git a/packages/core/src/builtin-marketing-workflow-ir.ts b/packages/core/src/builtin-marketing-workflow-ir.ts index 8d17e4e110..10c07f220f 100644 --- a/packages/core/src/builtin-marketing-workflow-ir.ts +++ b/packages/core/src/builtin-marketing-workflow-ir.ts @@ -2,6 +2,7 @@ import type { WorkflowIr } from "./workflow-ir-types.js"; import { parseWorkflowIr } from "./workflow-ir.js"; import { BUILTIN_WORKFLOW_SETTINGS } from "./builtin-workflow-settings.js"; import { completionSummaryNode } from "./builtin-completion-summary-node.js"; +import { postMergeVerificationOptionalGroupNode } from "./builtin-post-merge-group.js"; /** * FNXC:WorkflowMarketing 2026-06-20-00:00: @@ -91,6 +92,7 @@ const RAW_BUILTIN_MARKETING_WORKFLOW_IR: WorkflowIr = { config: { capability: "task-merge", reworkRegion: true, maxReworkCycles: 3 }, }, { id: "recovery-router", kind: "recovery-router", column: "editorial-review", config: { surfaces: ["merge", "retry"] } }, + postMergeVerificationOptionalGroupNode("published"), { id: "end", kind: "end", column: "published" }, ], edges: [ @@ -107,7 +109,8 @@ const RAW_BUILTIN_MARKETING_WORKFLOW_IR: WorkflowIr = { { from: "branch-group-member-integration", to: "merge-manual-hold", condition: "outcome:manual-required" }, { from: "branch-group-promotion", to: "merge-attempt", condition: "success" }, { from: "branch-group-promotion", to: "merge-manual-hold", condition: "outcome:manual-required" }, - { from: "merge-attempt", to: "end", condition: "success" }, + { from: "merge-attempt", to: "post-merge-verification", condition: "success" }, + { from: "post-merge-verification", to: "end", condition: "success" }, { from: "merge-attempt", to: "merge-retry", condition: "outcome:transient-failure" }, { from: "merge-attempt", to: "merge-manual-hold", condition: "outcome:manual-required" }, { from: "recovery-router", to: "merge-attempt", condition: "outcome:wake-merge", kind: "rework" }, diff --git a/packages/core/src/builtin-plan-review-group.ts b/packages/core/src/builtin-plan-review-group.ts index 664c08dd7f..7dc9da2004 100644 --- a/packages/core/src/builtin-plan-review-group.ts +++ b/packages/core/src/builtin-plan-review-group.ts @@ -50,6 +50,12 @@ export function planReviewOptionalGroupNode( config: { name: PLAN_REVIEW_NAME, defaultOn: options.defaultOn ?? true, + /* + * FNXC:WorkflowRemediation 2026-06-29-12:14: + * Plan Review REVISE must loop through graph-owned replan and then return to Plan Review before execution. Mark the optional group as the bounded rework-region head so the top-level remediation edge is legal and cannot spin forever. + */ + reworkRegion: true, + maxReworkCycles: 3, template: { nodes: [ { diff --git a/packages/core/src/builtin-post-merge-group.ts b/packages/core/src/builtin-post-merge-group.ts index 7f6d7997fc..41203a68a9 100644 --- a/packages/core/src/builtin-post-merge-group.ts +++ b/packages/core/src/builtin-post-merge-group.ts @@ -9,16 +9,34 @@ for post-merge workflow steps (U7 spike). Mirrors `codeReviewOptionalGroupNode` 1. runs it only AFTER a successful merge (when wired off the merge region and the `graphNativePostMerge` flag is on), and 2. records its WorkflowStepResult with `phase: "post-merge"` + emits `[post-merge]` - logs (failures are NON-BLOCKING — the merged task still completes). + logs. Advisory post-merge failures are non-blocking; explicit gate-mode + verification failures block final graph success after merge proof. -There are NO built-in post-merge steps today, so this factory is intentionally generic -and is NOT wired into `builtin:coding` (which stays byte-identical, the parity oracle). -It is the reusable builder migrated/custom workflows (and the new test) use to author a -post-merge step. The group node id is the STABLE per-task enable key (`enabledWorkflowSteps`), -and the inner template node carries a DISTINCT id (`${id}-step`) — a template node id may -not collide with the group/top-level node id (optional-group validation). +FNXC:WorkflowPostMerge 2026-06-29-12:22: +Full task built-ins need an explicit default-off post-merge verification node so +post-merge audit/verification policy can live in workflow definitions instead of +merger-only fallback code. The group node id is the STABLE per-task enable key +(`enabledWorkflowSteps`), and the inner template node carries a DISTINCT id +(`${id}-step`) — a template node id may not collide with the group/top-level node id +(optional-group validation). */ +export const POST_MERGE_VERIFICATION_GROUP_ID = "post-merge-verification"; + +const POST_MERGE_VERIFICATION_PROMPT = `You are a post-merge verification reviewer. Verify that the task's merged result is safe after integration. + +## Review focus +1. Confirm the task has merge proof or already-on-main proof before treating the workflow as complete. +2. Check the final merged diff and task summary for obvious mismatches, missing verification evidence, or integration-only regressions. +3. If configured test/build commands are available in the task context, inspect their latest result or explain why no post-merge command was applicable. + +## Output Requirements +- APPROVE: post-merge verification is acceptable. +- APPROVE_WITH_NOTES: completion may proceed with non-blocking notes. +- REVISE: completion should be blocked; include the concrete post-merge issue and the needed follow-up. +- Final output: output exactly one trailing JSON object on the final line (no markdown fences, no surrounding prose): +{"verdict":"APPROVE|APPROVE_WITH_NOTES|REVISE","notes":"..."}`; + export interface PostMergeOptionalGroupSpec { /** Stable per-task enable key + group node id. */ id: string; @@ -71,3 +89,15 @@ export function postMergeOptionalGroupNode(spec: PostMergeOptionalGroupSpec): Wo }, }; } + +export function postMergeVerificationOptionalGroupNode(column = "done"): WorkflowIrNode { + return postMergeOptionalGroupNode({ + id: POST_MERGE_VERIFICATION_GROUP_ID, + name: "Post-merge verification", + column, + prompt: POST_MERGE_VERIFICATION_PROMPT, + description: "Verify the integrated result after merge proof before final completion", + gateMode: "gate", + defaultOn: false, + }); +} diff --git a/packages/core/src/builtin-stepwise-coding-workflow-ir.ts b/packages/core/src/builtin-stepwise-coding-workflow-ir.ts index 0c977db01b..dcf503caf6 100644 --- a/packages/core/src/builtin-stepwise-coding-workflow-ir.ts +++ b/packages/core/src/builtin-stepwise-coding-workflow-ir.ts @@ -5,6 +5,7 @@ import { builtinPromptConfig } from "./builtin-workflow-prompts.js"; import { browserVerificationOptionalGroupNode } from "./builtin-browser-verification-group.js"; import { codeReviewOptionalGroupNode } from "./builtin-code-review-group.js"; import { completionSummaryNode } from "./builtin-completion-summary-node.js"; +import { postMergeVerificationOptionalGroupNode } from "./builtin-post-merge-group.js"; import { planReviewOptionalGroupNode } from "./builtin-plan-review-group.js"; import { browserVerificationRemediationNode, @@ -178,6 +179,7 @@ const RAW_BUILTIN_STEPWISE_CODING_WORKFLOW_IR: WorkflowIr = { config: { capability: "task-merge", reworkRegion: true, maxReworkCycles: 3 }, }, { id: "recovery-router", kind: "recovery-router", column: "in-review", config: { surfaces: ["merge", "retry"] } }, + postMergeVerificationOptionalGroupNode("done"), { id: "end", kind: "end", column: "done" }, ], edges: [ @@ -186,6 +188,7 @@ const RAW_BUILTIN_STEPWISE_CODING_WORKFLOW_IR: WorkflowIr = { { from: "plan", to: "end", condition: "failure" }, { from: "plan-review", to: "parse", condition: "success" }, { from: "plan-review", to: "plan-replan", condition: "failure" }, + { from: "plan-replan", to: "plan-review", condition: "success", kind: "rework" }, { from: "parse", to: "steps", condition: "success" }, // parse-steps no-steps defaults to success; route it explicitly to the foreach // (zero steps → foreach no-ops through its success edge, KTD-8/R8). @@ -206,7 +209,9 @@ const RAW_BUILTIN_STEPWISE_CODING_WORKFLOW_IR: WorkflowIr = { { from: "code-review", to: "completion-summary", condition: "success" }, { from: "completion-summary", to: "review", condition: "success" }, { from: "browser-verification", to: "browser-verification-remediation", condition: "failure" }, + { from: "browser-verification-remediation", to: "browser-verification", condition: "success", kind: "rework" }, { from: "code-review", to: "code-review-remediation", condition: "failure" }, + { from: "code-review-remediation", to: "code-review", condition: "success", kind: "rework" }, { from: "steps", to: "end", condition: "failure" }, { from: "review", to: "merge-gate", condition: "success" }, { from: "review", to: "end", condition: "failure" }, @@ -218,7 +223,8 @@ const RAW_BUILTIN_STEPWISE_CODING_WORKFLOW_IR: WorkflowIr = { { from: "branch-group-member-integration", to: "merge-manual-hold", condition: "outcome:manual-required" }, { from: "branch-group-promotion", to: "merge-attempt", condition: "success" }, { from: "branch-group-promotion", to: "merge-manual-hold", condition: "outcome:manual-required" }, - { from: "merge-attempt", to: "end", condition: "success" }, + { from: "merge-attempt", to: "post-merge-verification", condition: "success" }, + { from: "post-merge-verification", to: "end", condition: "success" }, { from: "merge-attempt", to: "merge-retry", condition: "outcome:transient-failure" }, { from: "merge-attempt", to: "merge-manual-hold", condition: "outcome:manual-required" }, { from: "recovery-router", to: "merge-attempt", condition: "outcome:wake-merge", kind: "rework" }, diff --git a/packages/core/src/builtin-stepwise-final-review-coding-workflow-ir.ts b/packages/core/src/builtin-stepwise-final-review-coding-workflow-ir.ts index 4286ed7367..567514bf95 100644 --- a/packages/core/src/builtin-stepwise-final-review-coding-workflow-ir.ts +++ b/packages/core/src/builtin-stepwise-final-review-coding-workflow-ir.ts @@ -2,6 +2,7 @@ import type { WorkflowIr } from "./workflow-ir-types.js"; import { parseWorkflowIr } from "./workflow-ir.js"; import { BUILTIN_STEPWISE_CODING_WORKFLOW_IR } from "./builtin-stepwise-coding-workflow-ir.js"; import { planReviewOptionalGroupNode } from "./builtin-plan-review-group.js"; +import { planReplanNode } from "./builtin-workflow-remediation-nodes.js"; function cloneWorkflowIr(ir: WorkflowIr): WorkflowIr { return JSON.parse(JSON.stringify(ir)) as WorkflowIr; @@ -39,6 +40,9 @@ const RAW_BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR: WorkflowIr = (() => if (!ir.nodes.some((node) => node.id === "plan-review")) { ir.nodes.splice(planIndex + 1, 0, planReviewOptionalGroupNode("in-progress")); } + if (!ir.nodes.some((node) => node.id === "plan-replan")) { + ir.nodes.splice(planIndex + 2, 0, planReplanNode("triage")); + } template.nodes = template.nodes.filter((node) => node.id !== "step-review"); template.edges = [ @@ -57,8 +61,12 @@ const RAW_BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR: WorkflowIr = (() => if (!ir.edges.some((edge) => edge.from === "plan-review" && edge.to === "parse")) { ir.edges.push({ from: "plan-review", to: "parse", condition: "success" }); } - if (!ir.edges.some((edge) => edge.from === "plan-review" && edge.to === "end" && edge.condition === "failure")) { - ir.edges.push({ from: "plan-review", to: "end", condition: "failure" }); + ir.edges = ir.edges.filter((edge) => !(edge.from === "plan-review" && edge.to === "end" && edge.condition === "failure")); + if (!ir.edges.some((edge) => edge.from === "plan-review" && edge.to === "plan-replan" && edge.condition === "failure")) { + ir.edges.push({ from: "plan-review", to: "plan-replan", condition: "failure" }); + } + if (!ir.edges.some((edge) => edge.from === "plan-replan" && edge.to === "plan-review" && edge.condition === "success")) { + ir.edges.push({ from: "plan-replan", to: "plan-review", condition: "success", kind: "rework" }); } if (!ir.edges.some((edge) => edge.from === "code-review" && edge.to === "completion-summary" && edge.condition === "success")) { ir.edges.push({ from: "code-review", to: "completion-summary", condition: "success" }); diff --git a/packages/core/src/builtin-workflows.ts b/packages/core/src/builtin-workflows.ts index 3496cca37b..4bcc08137f 100644 --- a/packages/core/src/builtin-workflows.ts +++ b/packages/core/src/builtin-workflows.ts @@ -9,6 +9,7 @@ import { builtinPromptConfig } from "./builtin-workflow-prompts.js"; import { browserVerificationOptionalGroupNode } from "./builtin-browser-verification-group.js"; import { codeReviewOptionalGroupNode } from "./builtin-code-review-group.js"; import { completionSummaryNode } from "./builtin-completion-summary-node.js"; +import { postMergeVerificationOptionalGroupNode } from "./builtin-post-merge-group.js"; import { planReviewOptionalGroupNode } from "./builtin-plan-review-group.js"; import { browserVerificationRemediationNode, @@ -56,6 +57,8 @@ function ceCodeReviewOptionalGroupNode(column: string): WorkflowIrNode { */ name: "Code Review", defaultOn: true, + reworkRegion: true, + maxReworkCycles: 3, template: { nodes: [ { @@ -125,7 +128,7 @@ function linear(spec: BuiltinSpec): WorkflowDefinition { const specNodes = spec.engineeringOptionalGroups ? withEngineeringOptionalGroups(spec.nodes, spec.engineeringOptionalGroups) : spec.nodes; - const workflowNodes = withCompletionSummaryNode(specNodes); + const workflowNodes = withPostMergeVerificationNode(withCompletionSummaryNode(specNodes)); const hasPlanReview = workflowNodes.some((node) => node.id === "plan-review"); const hasBrowserVerification = workflowNodes.some((node) => node.id === "browser-verification"); const hasCodeReview = workflowNodes.some((node) => node.id === "code-review"); @@ -161,6 +164,24 @@ function linear(spec: BuiltinSpec): WorkflowDefinition { edges.push({ from: node.id, to: failureTarget, condition: "failure" }); } } + /* + * FNXC:WorkflowRemediation 2026-06-29-12:12: + * Review failures are workflow policy, not terminal executor failures. Linear built-ins that opt into Plan Review, Browser Verification, or Code Review must route remediation success back to the owning gate so retry/restart keeps executing the graph instead of parking at an orphan remediation node or falling through to done. + */ + if (hasPlanReview) { + edges.push({ from: "plan-replan", to: "plan-review", condition: "success", kind: "rework" }); + } + if (hasBrowserVerification) { + edges.push({ + from: "browser-verification-remediation", + to: "browser-verification", + condition: "success", + kind: "rework", + }); + } + if (hasCodeReview) { + edges.push({ from: "code-review-remediation", to: "code-review", condition: "success", kind: "rework" }); + } const layout: Record = {}; nodes.forEach((node, i) => { layout[node.id] = { x: 60 + i * 170, y: 160 }; @@ -231,6 +252,17 @@ function withCompletionSummaryNode(nodes: BuiltinSpec["nodes"]): BuiltinSpec["no ]; } +function withPostMergeVerificationNode(nodes: BuiltinSpec["nodes"]): BuiltinSpec["nodes"] { + if (nodes.some((node) => node.id === "post-merge-verification")) return nodes; + const mergeIndex = nodes.findIndex((node) => node.config?.seam === "merge" || node.id === "merge"); + if (mergeIndex < 0) return nodes; + return [ + ...nodes.slice(0, mergeIndex + 1), + postMergeVerificationOptionalGroupNode("done"), + ...nodes.slice(mergeIndex + 1), + ]; +} + /** * Read-only built-in workflow templates. Selectable like any workflow; they * cannot be edited or deleted. In compile mode (flag off) only the custom @@ -263,7 +295,8 @@ export const BUILTIN_WORKFLOWS: WorkflowDefinition[] = [ "merge-retry": { x: 2100, y: 80 }, "recovery-router": { x: 2100, y: 240 }, "merge-manual-hold": { x: 1590, y: 240 }, - end: { x: 2270, y: 160 }, + "post-merge-verification": { x: 2270, y: 160 }, + end: { x: 2440, y: 160 }, }, createdAt: BUILTIN_TS, updatedAt: BUILTIN_TS, @@ -297,7 +330,8 @@ export const BUILTIN_WORKFLOWS: WorkflowDefinition[] = [ "merge-retry": { x: 2100, y: 80 }, "recovery-router": { x: 2100, y: 240 }, "merge-manual-hold": { x: 1590, y: 240 }, - end: { x: 2270, y: 160 }, + "post-merge-verification": { x: 2270, y: 160 }, + end: { x: 2440, y: 160 }, }, createdAt: BUILTIN_TS, updatedAt: BUILTIN_TS, @@ -355,7 +389,8 @@ export const BUILTIN_WORKFLOWS: WorkflowDefinition[] = [ "merge-retry": { x: 1590, y: 80 }, "recovery-router": { x: 1590, y: 240 }, "merge-manual-hold": { x: 1080, y: 240 }, - end: { x: 1760, y: 160 }, + "post-merge-verification": { x: 1760, y: 160 }, + end: { x: 1930, y: 160 }, }, createdAt: BUILTIN_TS, updatedAt: BUILTIN_TS, @@ -511,7 +546,8 @@ export const BUILTIN_WORKFLOWS: WorkflowDefinition[] = [ "merge-retry": { x: 2270, y: 80 }, "recovery-router": { x: 2270, y: 240 }, "merge-manual-hold": { x: 1760, y: 240 }, - end: { x: 2440, y: 160 }, + "post-merge-verification": { x: 2440, y: 160 }, + end: { x: 2610, y: 160 }, }, createdAt: BUILTIN_TS, updatedAt: BUILTIN_TS, diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index e1334ff519..f8fc4f4929 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -100,6 +100,12 @@ export { WORKFLOW_SETTING_TYPES, SETTING_RENDER_WIDGETS, } from "./workflow-ir.js"; +export { + analyzeWorkflowLifecycle, + type AnalyzeWorkflowLifecycleOptions, + type WorkflowLifecycleWarning, + type WorkflowLifecycleWarningCode, +} from "./workflow-lifecycle-validation.js"; export type { WorkflowIr, WorkflowIrV1, @@ -1726,7 +1732,11 @@ export type { } from "./research-types.js"; export { isExperimentalFeatureEnabled, GRAPH_NATIVE_POST_MERGE_FLAG } from "./experimental-features.js"; -export { postMergeOptionalGroupNode } from "./builtin-post-merge-group.js"; +export { + POST_MERGE_VERIFICATION_GROUP_ID, + postMergeOptionalGroupNode, + postMergeVerificationOptionalGroupNode, +} from "./builtin-post-merge-group.js"; export type { PostMergeOptionalGroupSpec } from "./builtin-post-merge-group.js"; export { WORKFLOW_COMPARABLE_AUDIT_MUTATIONS, diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index c9e70fb2ee..d4b6f61096 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -115,6 +115,7 @@ import type { WorkflowNodeLayout, } from "./workflow-definition-types.js"; import { compileWorkflowToSteps, isInterpreterDeferredWorkflowCompileError } from "./workflow-compiler.js"; +import { analyzeWorkflowLifecycle } from "./workflow-lifecycle-validation.js"; import { resolveDefaultOnOptionalGroupIds } from "./workflow-optional-steps.js"; import { BUILTIN_WORKFLOWS, @@ -14738,14 +14739,17 @@ ${stepsSection}`; createdAt: string; updatedAt: string; }): WorkflowDefinition { + const kind = row.kind === "fragment" ? "fragment" : "workflow"; + const ir = parseWorkflowIr(row.ir); return { id: row.id, name: row.name, description: row.description, // Legacy rows (pre-migration-109) have no kind column; default to "workflow". - kind: row.kind === "fragment" ? "fragment" : "workflow", - ir: parseWorkflowIr(row.ir), + kind, + ir, layout: this.parseWorkflowLayout(row.layout), + lifecycleWarnings: analyzeWorkflowLifecycle(ir, { kind }), createdAt: row.createdAt, updatedAt: row.updatedAt, }; @@ -14794,15 +14798,23 @@ ${stepsSection}`; const layout = input.layout ?? {}; const now = new Date().toISOString(); const id = this.nextWorkflowDefinitionId(); + const kind = input.kind === "fragment" ? "fragment" : "workflow"; const definition: WorkflowDefinition = { id, name, description: input.description ?? "", // KTD-1: fragments are pure-v1 IRs and pass through downgradeIrToV1IfPure // unchanged; default to "workflow" when the caller omits the kind. - kind: input.kind === "fragment" ? "fragment" : "workflow", + kind, ir, layout, + /* + FNXC:WorkflowLifecycleValidation 2026-06-29-11:47: + Persisted custom workflow definitions should carry computed lifecycle + warnings back to authoring/API surfaces without blocking advanced graphs. + Hard safety still lives in parser/store/merge proof guards. + */ + lifecycleWarnings: analyzeWorkflowLifecycle(ir, { kind }), createdAt: now, updatedAt: now, }; @@ -15030,6 +15042,7 @@ ${stepsSection}`; description: updates.description !== undefined ? updates.description : existing.description, ir, layout: updates.layout !== undefined ? updates.layout : existing.layout, + lifecycleWarnings: analyzeWorkflowLifecycle(ir, { kind: existing.kind }), updatedAt: new Date().toISOString(), }; diff --git a/packages/core/src/workflow-definition-types.ts b/packages/core/src/workflow-definition-types.ts index 716c77d15c..d02013beec 100644 --- a/packages/core/src/workflow-definition-types.ts +++ b/packages/core/src/workflow-definition-types.ts @@ -1,4 +1,5 @@ import type { WorkflowIr } from "./workflow-ir-types.js"; +import type { WorkflowLifecycleWarning } from "./workflow-lifecycle-validation.js"; /** Editor layout position for a single workflow IR node. Persisted separately * from the IR because the v1 IR contract deliberately excludes node geometry. */ @@ -27,6 +28,8 @@ export interface WorkflowDefinition { ir: WorkflowIr; /** Editor node positions keyed by IR node id. May be empty (auto-layout). */ layout: Record; + /** Non-blocking lifecycle guidance for custom workflow authors. */ + lifecycleWarnings?: WorkflowLifecycleWarning[]; /** ISO-8601 timestamp of creation. */ createdAt: string; /** ISO-8601 timestamp of last update. */ diff --git a/packages/core/src/workflow-lifecycle-validation.ts b/packages/core/src/workflow-lifecycle-validation.ts new file mode 100644 index 0000000000..5c9f3dcdbd --- /dev/null +++ b/packages/core/src/workflow-lifecycle-validation.ts @@ -0,0 +1,152 @@ +import type { WorkflowDefinitionKind } from "./workflow-definition-types.js"; +import type { WorkflowIr, WorkflowIrEdge, WorkflowIrNode } from "./workflow-ir-types.js"; +import { MERGE_REGION_NODE_KINDS } from "./workflow-compiler.js"; + +export type WorkflowLifecycleWarningCode = + | "missing-completion-summary" + | "missing-merge-region" + | "unsafe-terminal-before-merge" + | "optional-group-after-execution" + | "review-gate-without-failure-route"; + +export interface WorkflowLifecycleWarning { + code: WorkflowLifecycleWarningCode; + nodeId?: string; + message: string; +} + +export interface AnalyzeWorkflowLifecycleOptions { + kind?: WorkflowDefinitionKind; +} + +function isSummaryNode(node: WorkflowIrNode): boolean { + return node.config?.summaryTarget === "task" || node.id === "completion-summary"; +} + +function isMergeNode(node: WorkflowIrNode): boolean { + return MERGE_REGION_NODE_KINDS.has(node.kind) || node.config?.seam === "merge"; +} + +function isExecutionNode(node: WorkflowIrNode): boolean { + return node.config?.seam === "execute" || node.kind === "foreach" || node.kind === "parse-steps"; +} + +function buildOutgoing(edges: readonly WorkflowIrEdge[]): Map { + const outgoing = new Map(); + for (const edge of edges) { + const list = outgoing.get(edge.from) ?? []; + list.push(edge); + outgoing.set(edge.from, list); + } + return outgoing; +} + +function reachableBefore( + startId: string, + targetId: string, + outgoing: Map, +): Set { + const reachable = new Set(); + const queue = [startId]; + while (queue.length > 0) { + const id = queue.shift()!; + if (id === targetId || reachable.has(id)) continue; + reachable.add(id); + for (const edge of outgoing.get(id) ?? []) { + if (edge.kind === "rework" || reachable.has(edge.to)) continue; + queue.push(edge.to); + } + } + return reachable; +} + +/* +FNXC:WorkflowLifecycleValidation 2026-06-29-11:47: +Custom workflow authors need lifecycle-specific guidance without turning every +advanced graph into a hard parse failure. Emit warnings for missing summary, +missing merge proof regions, unsafe terminal paths, misplaced optional gates, and +review gates with no failure route; engine/store merge-proof guards remain the +hard invariant that prevents unsafe done. +*/ +export function analyzeWorkflowLifecycle( + ir: WorkflowIr, + options: AnalyzeWorkflowLifecycleOptions = {}, +): WorkflowLifecycleWarning[] { + if (options.kind === "fragment") return []; + const warnings: WorkflowLifecycleWarning[] = []; + const nodes = ir.nodes; + const outgoing = buildOutgoing(ir.edges); + const hasSummary = nodes.some(isSummaryNode); + const mergeNodeIds = new Set(nodes.filter(isMergeNode).map((node) => node.id)); + const endNode = nodes.find((node) => node.kind === "end"); + const startId = nodes.find((node) => node.kind === "start")?.id ?? "start"; + + if (!hasSummary) { + warnings.push({ + code: "missing-completion-summary", + message: "Full task workflows should include a completion-summary node before review, merge, or done.", + }); + } + + if (mergeNodeIds.size === 0) { + warnings.push({ + code: "missing-merge-region", + message: "Full task workflows should include a merge region so done is backed by merge proof.", + }); + } + + if (endNode && mergeNodeIds.size > 0) { + const beforeEnd = reachableBefore(startId, endNode.id, outgoing); + const mergeReachableBeforeEnd = [...mergeNodeIds].some((id) => beforeEnd.has(id)); + for (const edge of ir.edges) { + if (edge.to !== endNode.id) continue; + if (edge.condition !== undefined && edge.condition !== "success") continue; + if (mergeNodeIds.has(edge.from)) continue; + if (!mergeReachableBeforeEnd || beforeEnd.has(edge.from)) { + warnings.push({ + code: "unsafe-terminal-before-merge", + nodeId: edge.from, + message: `Node '${edge.from}' can terminate the workflow before a merge-proof region.`, + }); + } + } + } + + const executionNodeIds = new Set(nodes.filter(isExecutionNode).map((node) => node.id)); + for (const node of nodes) { + if (node.kind !== "optional-group") continue; + const groupName = typeof node.config?.name === "string" ? node.config.name : node.id; + const beforeGroup = reachableBefore( + startId, + node.id, + outgoing, + ); + const isPlanReview = node.id === "plan-review" || /plan review/i.test(groupName); + if (isPlanReview && [...executionNodeIds].some((id) => beforeGroup.has(id))) { + warnings.push({ + code: "optional-group-after-execution", + nodeId: node.id, + message: "Plan Review should be ordered before parse/execution so rejected plans cannot start work.", + }); + } + + const template = node.config?.template; + const templateNodes = template && typeof template === "object" && Array.isArray((template as { nodes?: unknown }).nodes) + ? (template as { nodes: WorkflowIrNode[] }).nodes + : []; + const hasGateStep = templateNodes.some((inner) => inner.config?.gateMode === "gate"); + const isPostMergeGate = node.config?.phase === "post-merge"; + const hasFailureRoute = (outgoing.get(node.id) ?? []).some((edge) => + edge.condition === "failure" || String(edge.condition ?? "").startsWith("outcome:"), + ); + if (hasGateStep && !hasFailureRoute && !isPostMergeGate) { + warnings.push({ + code: "review-gate-without-failure-route", + nodeId: node.id, + message: `Review gate '${node.id}' should declare a failure/remediation route so blocking findings cannot fall through silently.`, + }); + } + } + + return warnings; +} diff --git a/packages/dashboard/app/components/WorkflowNodeEditor.css b/packages/dashboard/app/components/WorkflowNodeEditor.css index deba3b1459..fae25da4be 100644 --- a/packages/dashboard/app/components/WorkflowNodeEditor.css +++ b/packages/dashboard/app/components/WorkflowNodeEditor.css @@ -1510,6 +1510,55 @@ Built-in workflow prompts need visible override state and a reset action without flex-wrap: wrap; } +/* +FNXC:WorkflowLifecycleValidation 2026-06-29-12:18: +Workflow authors need store-produced lifecycle warnings visible in the editor before runtime. Keep them inline and non-blocking: parser/store/merge guards still enforce hard safety, while this banner exposes missing summary/merge/review-loop guidance during authoring. +*/ +.wf-lifecycle-warnings { + display: grid; + gap: var(--space-xs); + padding: var(--space-sm) var(--space-md); + border-bottom: 1px solid color-mix(in srgb, var(--ws-warning) 45%, var(--border)); + background: color-mix(in srgb, var(--ws-warning) 8%, var(--bg-secondary)); + color: var(--text); +} + +.wf-lifecycle-warnings-title { + display: inline-flex; + align-items: center; + gap: var(--space-xs); + font-size: 0.78rem; + font-weight: 650; + color: var(--ws-warning); +} + +.wf-lifecycle-warnings ul { + display: grid; + gap: 4px; + margin: 0; + padding: 0; + list-style: none; +} + +.wf-lifecycle-warnings li { + display: flex; + align-items: baseline; + gap: var(--space-xs); + flex-wrap: wrap; + font-size: 0.78rem; + line-height: 1.35; +} + +.wf-lifecycle-warning-code, +.wf-lifecycle-warning-node { + padding: 1px 5px; + border-radius: var(--radius-sm); + background: var(--bg-tertiary); + color: var(--text-secondary); + font-size: 0.7rem; + font-family: var(--font-mono, monospace); +} + .wf-workflow-name, .wf-workflow-name--readonly { font-size: 0.95rem; diff --git a/packages/dashboard/app/components/WorkflowNodeEditor.tsx b/packages/dashboard/app/components/WorkflowNodeEditor.tsx index 9a777af80c..354d1e0b76 100644 --- a/packages/dashboard/app/components/WorkflowNodeEditor.tsx +++ b/packages/dashboard/app/components/WorkflowNodeEditor.tsx @@ -927,6 +927,7 @@ function InnerEditor({ const activeWorkflow = useMemo(() => workflows.find((w) => w.id === activeId), [workflows, activeId]); const isBuiltin = !!activeWorkflow && isBuiltinWorkflowId(activeWorkflow.id); + const lifecycleWarnings = activeWorkflow?.lifecycleWarnings ?? []; // Live mirror of the active workflow id, readable inside async callbacks that // captured an earlier value before an await (e.g. the AI-design round-trip). @@ -2806,6 +2807,23 @@ function InnerEditor({ )} + {lifecycleWarnings.length > 0 && ( +
+
+ + {t("workflows.lifecycleWarningsTitle", "Lifecycle warnings")} +
+
    + {lifecycleWarnings.map((warning, index) => ( +
  • + {warning.code} + {warning.nodeId && {warning.nodeId}} + {warning.message} +
    • + ))} +
+
+ )} {simpleLayoutEnabled && (