diff --git a/.changeset/fn-7249-optional-block-subnode-connections.md b/.changeset/fn-7249-optional-block-subnode-connections.md new file mode 100644 index 0000000000..271ca94ca1 --- /dev/null +++ b/.changeset/fn-7249-optional-block-subnode-connections.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Show optional workflow block children as connected in the workflow editor. +category: fix +dev: Adds non-editable visual-only optional-group boundary connectors that are filtered from workflow IR saves. diff --git a/docs/dashboard-guide.md b/docs/dashboard-guide.md index e9475f0b18..f31257a781 100644 --- a/docs/dashboard-guide.md +++ b/docs/dashboard-guide.md @@ -237,11 +237,14 @@ Behavior: - Built-in workflows are inspectable in the same canvas as custom workflows, including connected success, failure, and rework edges for their graph topology. Their graph structure stays read-only, but prompt/gate node Prompt fields can be edited per project and reset to the shipped default from the node inspector or expanded prompt editor. - Custom workflows can be created from blank, duplicated from built-ins/custom definitions, imported/exported, AI-designed, validated, and saved from the editor. - Optional-group node inspectors include controls for `defaultOn` and per-step **Max revisions** (`maxRevisions`), including an **Unbounded** toggle for Code Review, Browser Verification, or custom pre-merge gates that should keep cycling until they approve. + +- Optional-group, foreach, and loop containers show their template nodes inside the block. Canvas connections between the surrounding workflow and the block attach to the container boundary; connections between template nodes stay inside the block. Optional groups also draw non-editable entry/exit connector lines between the boundary and the template entry/exit nodes so single-step blocks such as Plan Review and Code Review do not look disconnected; those visual connectors are not saved into workflow IR. - The Settings panel is value-first for built-in workflows and groups workflow settings by Models, Review & Approval, Step Execution, and Advanced. Known workflow model values use the same model dropdown picker as **Settings → Project Models** so provider/model pairs are saved together; custom or non-model string values can still use typed inputs. Definitions remain available for custom workflow schema authoring. - The main Settings modal also exposes the default workflow's Plan/Triage, Executor, and Reviewer model lanes from **Project Models**; the modal's primary **Save** action writes those dropdown values as workflow setting values for the active default workflow. - On desktop, the editor uses a multi-panel canvas layout for editing the graph and adjacent workflow metadata. The **Show simple editor** toggle switches that same workflow into the graph-outline editor with dedicated **Graph**, **Add**, **Settings**, **Fields**, **Columns**, and **Actions** tabs. - On viewports `<=768px`, the editor switches to a full-screen mobile sheet. Global workflow entry points open to the workflow list with no workflow preselected and prompt users to select a workflow to edit; the Board/List workflow dropdown row edit action opens directly to the selected workflow editor when that selected workflow is available. -- Simple/mobile editing uses a graph outline instead of making the canvas the primary control. The outline shows nodes, branch/rework edges, column placement, and foreach/loop template children as tappable rows and chips that open the same node and edge detail editors as desktop. The structural **start** node opens an inspector for the workflow entry column when the workflow defines columns; the **Name** field remains unavailable because the start label is structural. For custom workflows, editable outline rows also expose **Move up** and **Move down** controls that reorder steps within their current column or template parent; built-in workflows remain read-only and hide those controls. +- Simple/mobile editing uses a graph outline instead of making the canvas the primary control. The outline shows nodes, branch/rework edges, column placement, and optional-group/foreach/loop template children as tappable rows and chips that open the same node and edge detail editors as desktop. The structural **start** node opens an inspector for the workflow entry column when the workflow defines columns; the **Name** field remains unavailable because the start label is structural. For custom workflows, editable outline rows also expose **Move up** and **Move down** controls that reorder steps within their current column or template parent; built-in workflows remain read-only and hide those controls. - Simple/mobile authoring exposes dedicated destinations for **Graph**, **Add**, **Settings**, **Fields**, **Columns**, and **Actions**. Add includes the node palette plus fragments, built-in step templates, and plugin step templates; Actions includes save, AI edit, auto-layout, export, and delete for custom workflows, plus export and duplicate for built-ins. Settings keeps the Definitions/Values tab split. - The create-workflow dialog and workflow AI authoring popover follow the same mobile full-screen/sheet pattern so they are not clipped by the editor canvas on narrow screens diff --git a/packages/dashboard/app/components/WorkflowNodeEditor.tsx b/packages/dashboard/app/components/WorkflowNodeEditor.tsx index 4bac5854a2..63af20ae93 100644 --- a/packages/dashboard/app/components/WorkflowNodeEditor.tsx +++ b/packages/dashboard/app/components/WorkflowNodeEditor.tsx @@ -10,9 +10,11 @@ import { useNodesState, useEdgesState, useReactFlow, + applyEdgeChanges, type Connection, type Node as FlowNode, type Edge as FlowEdge, + type EdgeChange, } from "@xyflow/react"; import { createPortal } from "react-dom"; import { useTranslation } from "react-i18next"; @@ -79,6 +81,7 @@ import { edgeConditionEditability, buildConnectionEdge, cascadeDelete, + refreshOptionalGroupVisualBoundaries, WF_EDGE_INTERACTION_WIDTH, FOREACH_GROUP_WIDTH, FOREACH_GROUP_HEIGHT, @@ -748,7 +751,7 @@ function InnerEditor({ // validationError used for genuine problems. const [interpreterOnly, setInterpreterOnly] = useState(false); const [nodes, setNodes, onNodesChange] = useNodesState>([]); - const [edges, setEdges, onEdgesChange] = useEdgesState([]); + const [edges, setEdges] = useEdgesState([]); const [selectedNodeId, setSelectedNodeId] = useState(null); const [selectedEdgeId, setSelectedEdgeId] = useState(null); const [inspectorCollapsed, setInspectorCollapsed] = useState(false); @@ -1332,17 +1335,38 @@ function InnerEditor({ t("workflowNodes.duplicateBlocked", "That connection already exists"), "warning", ); + } else if (result.error === "reserved-handle") { + addToast( + t("workflowNodes.reservedHandleBlocked", "Optional-block boundary guides cannot be connected manually"), + "warning", + ); } return; } - setEdges((eds) => [...eds, result.edge]); + setEdges((eds) => { + const refreshed = refreshOptionalGroupVisualBoundaries(nodes, [...eds, result.edge]); + setNodes(refreshed.nodes); + return refreshed.edges; + }); if (options.selectCreatedEdge) { setSelectedEdgeId(result.edge.id); setSelectedNodeId(null); setInspectorCollapsed(false); } }, - [edges, nodes, setEdges, addToast, t], + [edges, nodes, setEdges, setNodes, addToast, t], + ); + + const onWorkflowEdgesChange = useCallback( + (changes: EdgeChange[]) => { + setEdges((eds) => { + const changedEdges = applyEdgeChanges(changes, eds) as FlowEdge[]; + const refreshed = refreshOptionalGroupVisualBoundaries(nodes, changedEdges); + setNodes(refreshed.nodes); + return refreshed.edges; + }); + }, + [nodes, setEdges, setNodes], ); const onConnect = useCallback( @@ -1399,6 +1423,7 @@ function InnerEditor({ // so authors can wire the body immediately. The group node must precede // its child for React Flow's parent extent to apply. // FNXC:WorkflowOptionalGroup 2026-06-21-11:30: An optional-group is authored exactly like a foreach/loop region — drop nodes inside; the subgraph runs once when the task enables the group. + // FNXC:WorkflowOptionalGroup 2026-06-29-23:56: Palette and mobile add paths must compute optional-group boundary metadata immediately. A newly seeded optional child is both the visual entry and exit until authors add real internal template edges, so refresh the generated boundary guides before committing nodes to state rather than waiting for save/reload. const childId = foreachChildFlowId(id, newNodeId()); const childLabel = kind === "foreach" @@ -1407,30 +1432,36 @@ function InnerEditor({ ? t("workflowNodes.optionalGroupStepLabel", "Optional step") : t("workflowNodes.loopStepLabel", "Loop step"); const childConfig = kind === "foreach" ? { seam: "step-execute" } : { prompt: "" }; - setNodes((ns) => [ - ...ns, - { - id, - type: kind, - position: { x: 200 + ns.length * 40, y: 240 + (ns.length % 3) * 70 }, - data: { kind, label, config, templateEmpty: false }, - style: { width: FOREACH_GROUP_WIDTH, height: FOREACH_GROUP_HEIGHT }, - deletable: true, - }, - { - id: childId, - type: "prompt", - position: { x: FOREACH_CHILD_X, y: FOREACH_CHILD_Y }, - parentId: id, - extent: "parent", - data: { - kind: "prompt", - label: childLabel, - config: childConfig, + setNodes((ns) => { + const nextNodes = [ + ...ns, + { + id, + type: kind, + position: { x: 200 + ns.length * 40, y: 240 + (ns.length % 3) * 70 }, + data: { kind, label, config, templateEmpty: false }, + style: { width: FOREACH_GROUP_WIDTH, height: FOREACH_GROUP_HEIGHT }, + deletable: true, }, - deletable: true, - }, - ]); + { + id: childId, + type: "prompt", + position: { x: FOREACH_CHILD_X, y: FOREACH_CHILD_Y }, + parentId: id, + extent: "parent", + data: { + kind: "prompt", + label: childLabel, + config: childConfig, + }, + deletable: true, + }, + ] satisfies FlowNode[]; + if (kind !== "optional-group") return nextNodes; + const refreshed = refreshOptionalGroupVisualBoundaries(nextNodes, edges); + setEdges(refreshed.edges); + return refreshed.nodes; + }); setSelectedNodeId(id); return; } @@ -1447,7 +1478,7 @@ function InnerEditor({ ]); setSelectedNodeId(id); }, - [setNodes, t], + [edges, setEdges, setNodes, t], ); // U9/R8: insert a step template (built-in or plugin) as ONE pre-configured @@ -1733,8 +1764,8 @@ function InnerEditor({ const updateSelectedEdge = useCallback( (patch: { condition?: string; rework?: boolean }) => { if (!selectedEdgeId) return; - setEdges((eds) => - eds.map((e) => { + setEdges((eds) => { + const updated = eds.map((e) => { if (e.id !== selectedEdgeId) return e; const condition = patch.condition ?? (e.data?.condition as string | undefined) ?? "success"; const rework = patch.rework ?? (e.data?.kind as string | undefined) === "rework"; @@ -1746,10 +1777,13 @@ function InnerEditor({ animated: rework, className: edgeClassName(condition, rework), }; - }), - ); + }); + const refreshed = refreshOptionalGroupVisualBoundaries(nodes, updated); + setNodes(refreshed.nodes); + return refreshed.edges; + }); }, - [selectedEdgeId, setEdges], + [nodes, selectedEdgeId, setEdges, setNodes], ); // ── Deletion (U3, R6) ────────────────────────────────────────────────────── @@ -1761,7 +1795,8 @@ function InnerEditor({ const idSet = new Set(ids); let next: { nodes: FlowNode[]; edges: FlowEdge[] } | null = null; setNodes((ns) => { - next = cascadeDelete(ns, edges, idSet); + const deleted = cascadeDelete(ns, edges, idSet); + next = refreshOptionalGroupVisualBoundaries(deleted.nodes, deleted.edges); return next.nodes; }); if (next) setEdges((next as { edges: FlowEdge[] }).edges); @@ -3488,7 +3523,7 @@ function InnerEditor({ edges={edges} nodeTypes={workflowNodeTypes} onNodesChange={onNodesChange} - onEdgesChange={onEdgesChange} + onEdgesChange={onWorkflowEdgesChange} onConnect={onConnect} onNodeDragStop={onNodeDragStop} deleteKeyCode={isBuiltin ? null : ["Backspace", "Delete"]} diff --git a/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx b/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx index 15d18d4c92..6c9bc142db 100644 --- a/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx +++ b/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx @@ -27,6 +27,7 @@ import { emptyWorkflowLayout, foreachChildFlowId, WF_EDGE_INTERACTION_WIDTH, + isVisualOnlyWorkflowEdge, } from "../workflow-flow-mapping"; import { BUILTIN_CODING_WORKFLOW_IR, @@ -269,8 +270,16 @@ function edgeRenderableAssertion(definition: WorkflowDefinition) { WF_EDGE_INTERACTION_WIDTH, ); expect(edge.zIndex, `${definition.id} edge ${edge.id} z-index`).toBeGreaterThan(0); - expect(edge.sourceHandle, `${definition.id} edge ${edge.id} source handle`).toBeUndefined(); - expect(edge.targetHandle, `${definition.id} edge ${edge.id} target handle`).toBeUndefined(); + if (isVisualOnlyWorkflowEdge(edge) && edge.data?.boundary === "entry") { + expect(edge.sourceHandle, `${definition.id} edge ${edge.id} source handle`).toBe("optional-boundary-entry"); + expect(edge.targetHandle, `${definition.id} edge ${edge.id} target handle`).toBeUndefined(); + } else if (isVisualOnlyWorkflowEdge(edge) && edge.data?.boundary === "exit") { + expect(edge.sourceHandle, `${definition.id} edge ${edge.id} source handle`).toBeUndefined(); + expect(edge.targetHandle, `${definition.id} edge ${edge.id} target handle`).toBe("optional-boundary-exit"); + } else { + expect(edge.sourceHandle, `${definition.id} edge ${edge.id} source handle`).toBeUndefined(); + expect(edge.targetHandle, `${definition.id} edge ${edge.id} target handle`).toBeUndefined(); + } } return flow; } @@ -439,7 +448,7 @@ describe("workflow-flow-mapping", () => { const failuresToEnd = edges.filter((edge) => edge.target === "end" && edge.data?.condition === "failure"); // FNXC:WorkflowOptionalGroup 2026-06-21-15:30: the coding built-in's pre-merge `workflow-step` seam was migrated to a `browser-verification` optional-group (U6), which now carries the failure->end edge in its place. // FNXC:CodeReviewStep 2026-06-25-00:00: the default-on `code-review` optional-group is also on the pre-merge success path with its own failure->end edge (see builtin-code-review-group.test.ts), so it is an expected failure->end source too. This corrected a stale assertion that predated the code-review group's addition. - // FNXC:WorkflowPlanReview 2026-06-29-23:18: FN-7265 removed the coding workflow's duplicate plan-review gate, so this renderability guard tracks the remaining failure-to-end sources without expecting a stale `plan-review` edge. + // FNXC:WorkflowPlanReview 2026-06-29-23:18: FN-7265 removed the coding workflow's duplicate plan-review gate; Plan Review failures route through the plan-replan optional remediation loop instead of directly to end, so this renderability guard tracks the remaining failure-to-end sources without expecting a stale `plan-review` edge. expect(failuresToEnd.map((edge) => edge.source).sort()).toEqual([ "execute", "merge-attempt", @@ -2029,6 +2038,29 @@ describe("WorkflowNodeEditor — U8 step-inversion authoring", () => { // No empty hint — the palette seeded an optional step inside. expect(screen.queryByTestId("wf-optional-group-empty")).not.toBeInTheDocument(); + const seededChildId = await waitFor(() => { + const childIds = [...document.querySelectorAll(".react-flow__node")] + .map((node) => node.dataset.id) + .filter((nodeId): nodeId is string => Boolean(nodeId?.includes("::"))); + expect(childIds).toHaveLength(1); + return childIds[0]; + }); + const seededGroupId = seededChildId.split("::")[0]; + /* + * FNXC:WorkflowOptionalGroup 2026-06-29-23:56: + * Newly authored optional groups must render the same generated entry/exit guide anchors as loaded IR. This catches the palette/mobile add path that previously inserted a parent and child with no immediate boundary refresh, leaving the seeded child visually disconnected until a later save/reload recomputed editor-only boundary state. + */ + await waitFor(() => { + expect( + document.body.querySelector(`.react-flow__handle.source[data-nodeid="${seededGroupId}"][data-handleid="optional-boundary-entry"]`), + ).toBeInTheDocument(); + expect( + document.body.querySelector(`.react-flow__handle.target[data-nodeid="${seededGroupId}"][data-handleid="optional-boundary-exit"]`), + ).toBeInTheDocument(); + expect(document.body.querySelector(`.react-flow__handle.target[data-nodeid="${seededChildId}"][data-handlepos="left"]`)).toBeInTheDocument(); + expect(document.body.querySelector(`.react-flow__handle.source[data-nodeid="${seededChildId}"][data-handlepos="right"]`)).toBeInTheDocument(); + }); + await waitFor(() => expect(screen.getAllByLabelText(/Column name/i).length).toBeGreaterThan(0)); fireEvent.click(screen.getByText("Save").closest("button")!); await waitFor(() => expect(updateWorkflow).toHaveBeenCalled()); @@ -2470,6 +2502,58 @@ describe("WorkflowNodeEditor — built-in stepwise selection render path", () => expect(customFlow.edges.every((edge) => edge.label === "success")).toBe(true); }); + it("renders optional-group boundary connector handles for built-in single-child templates", async () => { + vi.mocked(fetchWorkflows).mockResolvedValue([builtinDef()]); + render( {}} addToast={() => {}} />); + + await screen.findByTestId("wf-readonly-banner"); + await waitFor(() => expect(screen.getAllByTestId("wf-node-optional-group").length).toBeGreaterThanOrEqual(2)); + + const flow = irToFlow(builtinDef()); + const byId = new Map(flow.nodes.map((node) => [node.id, node] as const)); + for (const [groupId, childId] of [ + ["plan-review", "plan-review-step"], + ["code-review", "code-review-step"], + ] as const) { + const childFlowId = `${groupId}::${childId}`; + expect(byId.get(childFlowId)?.data.optionalGroupBoundary, `${groupId} child boundary`).toEqual({ entry: true, exit: true }); + + const boundaryEdges = flow.edges.filter((edge) => isVisualOnlyWorkflowEdge(edge) && (edge.source === groupId || edge.target === groupId)); + expect(boundaryEdges, `${groupId} visual boundary edges`).toEqual(expect.arrayContaining([ + expect.objectContaining({ source: groupId, sourceHandle: "optional-boundary-entry", target: childFlowId }), + expect.objectContaining({ source: childFlowId, target: groupId, targetHandle: "optional-boundary-exit" }), + ])); + /* + * FNXC:WorkflowOptionalGroup 2026-06-29-22:47: + * Built-in Plan Review and Code Review optional blocks each contain one template child. The desktop editor must render both the normal container handles and the side-correct boundary handles used by visual-only connector edges so entry attaches from the left boundary and exit attaches to the right boundary, while persistence still filters those visual-only edges in mapping tests. + * + * FNXC:WorkflowOptionalGroup 2026-06-29-23:20: + * Boundary guide handles remain in the DOM solely as generated edge anchors. They must not carry React Flow's connectable affordance because user-authored edges from optional-boundary-* handles would persist fake group↔child topology. + */ + for (const [nodeId, position] of [ + [groupId, "left"], + [groupId, "right"], + [childFlowId, "left"], + [childFlowId, "right"], + ] as const) { + expect( + document.body.querySelector(`.react-flow__handle[data-nodeid="${nodeId}"][data-handlepos="${position}"]`), + `${nodeId} ${position} handle`, + ).toBeInTheDocument(); + } + const entryBoundaryHandle = document.body.querySelector( + `.react-flow__handle.source[data-nodeid="${groupId}"][data-handlepos="left"][data-handleid="optional-boundary-entry"]`, + ); + const exitBoundaryHandle = document.body.querySelector( + `.react-flow__handle.target[data-nodeid="${groupId}"][data-handlepos="right"][data-handleid="optional-boundary-exit"]`, + ); + expect(entryBoundaryHandle, `${groupId} left boundary source handle`).toBeInTheDocument(); + expect(exitBoundaryHandle, `${groupId} right boundary target handle`).toBeInTheDocument(); + expect(entryBoundaryHandle, `${groupId} left boundary source handle connectability`).not.toHaveClass("connectable"); + expect(exitBoundaryHandle, `${groupId} right boundary target handle connectability`).not.toHaveClass("connectable"); + } + }); + it("irToFlow on the built-in stepwise IR yields a foreach group + rework-styled template edge (editor load path)", () => { // Mirrors exactly what the editor's load effect feeds React Flow: // const flow = irToFlow(activeWorkflow) diff --git a/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts b/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts index a4ca8d96a9..9711869c23 100644 --- a/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts +++ b/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts @@ -30,9 +30,11 @@ import { shortConditionLabel, edgeClassName, edgeConditionEditability, + isVisualOnlyWorkflowEdge, wouldCreateCycle, buildConnectionEdge, cascadeDelete, + refreshOptionalGroupVisualBoundaries, COLUMN_BAND_HEIGHT, WF_CARD_WIDTH, WF_FALLBACK_NODE_GAP, @@ -65,7 +67,11 @@ function nodeWidth(node: FlowNode): number { return typeof width === "number" ? width : WF_CARD_WIDTH; } -function assertContainerHandles(kind: "optional-group" | "foreach" | "loop", data: WorkflowFlowNodeData): void { +function assertRenderedHandles( + kind: WorkflowEditorNodeKind, + data: WorkflowFlowNodeData, + expected: { target: number; source: number }, +): void { const Component = workflowNodeTypes[kind]; const { container, unmount } = render( createElement(ReactFlowProvider, null, createElement(Component, { data, id: `${kind}-handle-check` })), @@ -73,13 +79,17 @@ function assertContainerHandles(kind: "optional-group" | "foreach" | "loop", dat try { const root = container.querySelector(`[data-testid="wf-node-${kind}"]`); expect(root).not.toBeNull(); - expect(root?.querySelectorAll(".react-flow__handle.target")).toHaveLength(1); - expect(root?.querySelectorAll(".react-flow__handle.source")).toHaveLength(1); + expect(root?.querySelectorAll(".react-flow__handle.target")).toHaveLength(expected.target); + expect(root?.querySelectorAll(".react-flow__handle.source")).toHaveLength(expected.source); } finally { unmount(); } } +function assertContainerHandles(kind: "optional-group" | "foreach" | "loop", data: WorkflowFlowNodeData): void { + assertRenderedHandles(kind, data, kind === "optional-group" ? { target: 2, source: 2 } : { target: 1, source: 1 }); +} + function assertRunDoesNotOverlap( name: string, nodes: FlowNode[], @@ -881,9 +891,248 @@ describe("workflow-flow-mapping foreach + rework round-trip", () => { expect(out.edges.map((e) => `${e.from}->${e.to}`)).toEqual(["start->opt", "opt->end"]); }); + it("marks optional-group template boundaries as visual-only child metadata", () => { + const optionalIr: WorkflowDefinition["ir"] = { + version: "v2", + name: "optional-boundaries", + columns: ir.columns, + nodes: [ + { id: "start", kind: "start", column: "plan" }, + { + id: "opt", + kind: "optional-group", + column: "in-progress", + config: { + defaultOn: true, + template: { + nodes: [ + { id: "prepare", kind: "prompt", config: { prompt: "prepare" } }, + { id: "approve", kind: "gate", config: { prompt: "approve?" } }, + ], + edges: [{ from: "prepare", to: "approve", condition: "success" }], + }, + }, + }, + { id: "end", kind: "end", column: "done" }, + ], + edges: [ + { from: "start", to: "opt", condition: "success" }, + { from: "opt", to: "end", condition: "success" }, + ], + }; + + const { nodes, edges } = irToFlow(makeDef(optionalIr)); + const byId = new Map(nodes.map((node) => [node.id, node] as const)); + expect(byId.get("opt::prepare")?.data.optionalGroupBoundary).toEqual({ entry: true, exit: false }); + expect(byId.get("opt::approve")?.data.optionalGroupBoundary).toEqual({ entry: false, exit: true }); + expect(edges.filter((edge) => edge.source === "opt::prepare" && edge.target === "opt::approve")).toHaveLength(1); + + const { ir: out } = flowToIr("optional-boundaries", nodes, edges, columnsOf(makeDef(optionalIr))); + if (out.version !== "v2") throw new Error("expected v2"); + const opt = out.nodes.find((node) => node.id === "opt"); + const cfg = opt?.config as + | { template?: { nodes: Array<{ config?: Record }>; edges: unknown[] } } + | undefined; + expect(cfg?.template?.edges).toEqual([{ from: "prepare", to: "approve", condition: "success" }]); + expect(cfg?.template?.nodes.map((node) => node.config?.optionalGroupBoundary)).toEqual([undefined, undefined]); + }); + + it("connects each independent optional-group boundary child with visual-only container edges", () => { + const optionalIr: WorkflowDefinition["ir"] = { + version: "v2", + name: "optional-independent-boundaries", + columns: ir.columns, + nodes: [ + { id: "start", kind: "start", column: "plan" }, + { + id: "opt", + kind: "optional-group", + column: "in-progress", + config: { + template: { + nodes: [ + { id: "alpha", kind: "prompt", config: { prompt: "alpha" } }, + { id: "beta", kind: "prompt", config: { prompt: "beta" } }, + ], + edges: [], + }, + }, + }, + { id: "end", kind: "end", column: "done" }, + ], + edges: [ + { from: "start", to: "opt", condition: "success" }, + { from: "opt", to: "end", condition: "success" }, + ], + }; + + const { nodes, edges } = irToFlow(makeDef(optionalIr)); + const byId = new Map(nodes.map((node) => [node.id, node] as const)); + expect(byId.get("opt::alpha")?.data.optionalGroupBoundary).toEqual({ entry: true, exit: true }); + expect(byId.get("opt::beta")?.data.optionalGroupBoundary).toEqual({ entry: true, exit: true }); + + const visualBoundaryEdges = edges.filter((edge) => isVisualOnlyWorkflowEdge(edge)); + expect(visualBoundaryEdges.map((edge) => `${edge.source}->${edge.target}`).sort()).toEqual([ + "opt->opt::alpha", + "opt->opt::beta", + "opt::alpha->opt", + "opt::beta->opt", + ]); + expect(visualBoundaryEdges.every((edge) => edge.selectable === false && edge.deletable === false)).toBe(true); + + const { ir: out } = flowToIr("optional-independent-boundaries", nodes, edges, columnsOf(makeDef(optionalIr))); + const opt = out.nodes.find((node) => node.id === "opt"); + const cfg = opt?.config as { template?: { nodes?: Array<{ config?: Record }>; edges?: unknown[] } } | undefined; + expect(cfg?.template?.edges).toEqual([]); + expect(cfg?.template?.nodes?.map((node) => node.config?.optionalGroupBoundary)).toEqual([undefined, undefined]); + expect(out.edges.map((edge) => `${edge.from}->${edge.to}`)).toEqual(["start->opt", "opt->end"]); + }); + + it("recomputes optional-group visual boundaries after live template edge mutations", () => { + const optionalIr: WorkflowDefinition["ir"] = { + version: "v2", + name: "optional-live-boundaries", + columns: ir.columns, + nodes: [ + { id: "start", kind: "start", column: "plan" }, + { + id: "opt", + kind: "optional-group", + column: "in-progress", + config: { + template: { + nodes: [ + { id: "alpha", kind: "prompt", config: { prompt: "alpha" } }, + { id: "beta", kind: "prompt", config: { prompt: "beta" } }, + ], + edges: [], + }, + }, + }, + { id: "end", kind: "end", column: "done" }, + ], + edges: [ + { from: "start", to: "opt", condition: "success" }, + { from: "opt", to: "end", condition: "success" }, + ], + }; + + const initial = irToFlow(makeDef(optionalIr)); + const realInternalEdge: FlowEdge = { + id: "e-live-alpha-beta", + source: "opt::alpha", + target: "opt::beta", + data: { condition: "success" }, + label: "success", + }; + + const connected = refreshOptionalGroupVisualBoundaries(initial.nodes, [...initial.edges, realInternalEdge]); + const connectedById = new Map(connected.nodes.map((node) => [node.id, node] as const)); + expect(connectedById.get("opt::alpha")?.data.optionalGroupBoundary).toEqual({ entry: true, exit: false }); + expect(connectedById.get("opt::beta")?.data.optionalGroupBoundary).toEqual({ entry: false, exit: true }); + expect(connected.edges.filter((edge) => isVisualOnlyWorkflowEdge(edge)).map((edge) => `${edge.source}->${edge.target}`).sort()).toEqual([ + "opt->opt::alpha", + "opt::beta->opt", + ]); + + const disconnected = refreshOptionalGroupVisualBoundaries( + connected.nodes, + connected.edges.filter((edge) => edge.id !== realInternalEdge.id), + ); + const disconnectedById = new Map(disconnected.nodes.map((node) => [node.id, node] as const)); + expect(disconnectedById.get("opt::alpha")?.data.optionalGroupBoundary).toEqual({ entry: true, exit: true }); + expect(disconnectedById.get("opt::beta")?.data.optionalGroupBoundary).toEqual({ entry: true, exit: true }); + expect(disconnected.edges.filter((edge) => isVisualOnlyWorkflowEdge(edge)).map((edge) => `${edge.source}->${edge.target}`).sort()).toEqual([ + "opt->opt::alpha", + "opt->opt::beta", + "opt::alpha->opt", + "opt::beta->opt", + ]); + + const { ir: out } = flowToIr("optional-live-boundaries", disconnected.nodes, disconnected.edges, columnsOf(makeDef(optionalIr))); + const opt = out.nodes.find((node) => node.id === "opt"); + const cfg = opt?.config as { template?: { edges?: unknown[]; nodes?: Array<{ config?: Record }> } } | undefined; + expect(cfg?.template?.edges).toEqual([]); + expect(cfg?.template?.nodes?.map((node) => node.config?.optionalGroupBoundary)).toEqual([undefined, undefined]); + }); + + it("derives optional-group boundaries from forward edges without letting rework cycles hide exits", () => { + const optionalIr: WorkflowDefinition["ir"] = { + version: "v2", + name: "optional-rework-boundaries", + columns: ir.columns, + nodes: [ + { id: "start", kind: "start", column: "plan" }, + { + id: "opt", + kind: "optional-group", + column: "in-progress", + config: { + template: { + nodes: [ + { id: "exec", kind: "prompt", config: { prompt: "execute" } }, + { id: "review", kind: "step-review", config: { type: "code" } }, + ], + edges: [ + { from: "exec", to: "review", condition: "success" }, + { from: "review", to: "exec", condition: "outcome:revise", kind: "rework" }, + ], + }, + }, + }, + { id: "end", kind: "end", column: "done" }, + ], + edges: [ + { from: "start", to: "opt", condition: "success" }, + { from: "opt", to: "end", condition: "success" }, + ], + }; + + const { nodes, edges } = irToFlow(makeDef(optionalIr)); + const byId = new Map(nodes.map((node) => [node.id, node] as const)); + expect(byId.get("opt::exec")?.data.optionalGroupBoundary).toEqual({ entry: true, exit: false }); + expect(byId.get("opt::review")?.data.optionalGroupBoundary).toEqual({ entry: false, exit: true }); + expect(edges.filter((edge) => edge.source === "opt::exec" && edge.target === "opt::review")).toHaveLength(1); + expect( + edges.filter((edge) => edge.source === "opt::review" && edge.target === "opt::exec" && edge.data?.kind === "rework"), + ).toHaveLength(1); + expect( + edges.filter((edge) => isVisualOnlyWorkflowEdge(edge) && edge.source === "opt" && edge.target === "opt::exec"), + ).toHaveLength(1); + expect( + edges.filter((edge) => isVisualOnlyWorkflowEdge(edge) && edge.source === "opt::review" && edge.target === "opt"), + ).toHaveLength(1); + }); + + it("marks built-in Plan Review and Code Review single children as optional-group entry and exit boundaries", () => { + for (const [workflowName, builtinIr] of [ + ["coding", BUILTIN_CODING_WORKFLOW_IR], + ["stepwise coding", BUILTIN_STEPWISE_CODING_WORKFLOW_IR], + ] as const) { + const { nodes } = irToFlow(makeDef(builtinIr)); + const byId = new Map(nodes.map((node) => [node.id, node] as const)); + for (const [groupId, childId] of [ + ["plan-review", "plan-review-step"], + ["code-review", "code-review-step"], + ] as const) { + expect(byId.get(groupId), `${workflowName} ${groupId} group`).toBeTruthy(); + expect( + byId.get(`${groupId}::${childId}`)?.data.optionalGroupBoundary, + `${workflowName} ${groupId} child`, + ).toEqual({ + entry: true, + exit: true, + }); + } + } + }); + /* * FNXC:WorkflowContainerEdges 2026-06-26-07:30: * Browser Verification is an optional-group container on the built-in workflow path. The mapping invariant is broader than that repro: optional-group/foreach/loop containers must keep top-level edges attached to the container id, layer routed edges above the group background, render exactly one target/source handle pair, and use width-aware fallback positions so adjacent nodes do not occlude those handles. + * + * FNXC:WorkflowOptionalGroup 2026-06-29-20:41: + * Plan Review and Code Review are single-node optional-group templates. The child node must be visibly connected to the optional block boundary through visual-only entry/exit connector edges, while save serialization keeps those fake boundary connectors out of the persisted IR. */ it("keeps container edges and handles connected across built-in workflow containers", () => { const loopNode = { @@ -934,8 +1183,10 @@ describe("workflow-flow-mapping foreach + rework round-trip", () => { id: string; kind: "optional-group" | "foreach" | "loop"; }> = [ + { name: "coding plan review", ir: BUILTIN_CODING_WORKFLOW_IR, id: "plan-review", kind: "optional-group" }, { name: "coding browser verification", ir: BUILTIN_CODING_WORKFLOW_IR, id: "browser-verification", kind: "optional-group" }, { name: "coding code review", ir: BUILTIN_CODING_WORKFLOW_IR, id: "code-review", kind: "optional-group" }, + { name: "stepwise plan review", ir: BUILTIN_STEPWISE_CODING_WORKFLOW_IR, id: "plan-review", kind: "optional-group" }, { name: "stepwise browser verification", ir: BUILTIN_STEPWISE_CODING_WORKFLOW_IR, id: "browser-verification", kind: "optional-group" }, { name: "stepwise code review", ir: BUILTIN_STEPWISE_CODING_WORKFLOW_IR, id: "code-review", kind: "optional-group" }, { name: "stepwise foreach", ir: BUILTIN_STEPWISE_CODING_WORKFLOW_IR, id: "steps", kind: "foreach" }, @@ -953,7 +1204,9 @@ describe("workflow-flow-mapping foreach + rework round-trip", () => { expect(group?.style?.width, testCase.name).toBe(FOREACH_GROUP_WIDTH); assertContainerHandles(testCase.kind, group!.data); - const connectedTopLevelEdges = edges.filter((edge) => edge.source === testCase.id || edge.target === testCase.id); + const connectedTopLevelEdges = edges.filter( + (edge) => !isVisualOnlyWorkflowEdge(edge) && (edge.source === testCase.id || edge.target === testCase.id), + ); expect(connectedTopLevelEdges.length, testCase.name).toBeGreaterThan(0); for (const edge of connectedTopLevelEdges) { expect(edge.source.includes("::"), `${testCase.name} source should be top-level`).toBe(false); @@ -974,9 +1227,42 @@ describe("workflow-flow-mapping foreach + rework round-trip", () => { } } - const templateEdges = edges.filter((edge) => edge.source.includes("::") || edge.target.includes("::")); + const templateEdges = edges.filter((edge) => !isVisualOnlyWorkflowEdge(edge) && (edge.source.includes("::") || edge.target.includes("::"))); expect(templateEdges.some((edge) => edge.source === testCase.id || edge.target === testCase.id), testCase.name).toBe(false); - expect(nodes.filter((node) => node.parentId === testCase.id).every((node) => node.zIndex! > group!.zIndex!), testCase.name).toBe(true); + const children = nodes.filter((node) => node.parentId === testCase.id); + expect(children.every((node) => node.zIndex! > group!.zIndex!), testCase.name).toBe(true); + if (testCase.kind === "optional-group") { + const visualBoundaryEdges = edges.filter((edge) => isVisualOnlyWorkflowEdge(edge) && (edge.source === testCase.id || edge.target === testCase.id)); + expect(visualBoundaryEdges, testCase.name).toHaveLength(children.length > 0 ? 2 : 0); + expect(visualBoundaryEdges.every((edge) => edge.selectable === false && edge.deletable === false), testCase.name).toBe(true); + for (const edge of visualBoundaryEdges) { + const source = byId.get(edge.source); + const target = byId.get(edge.target); + expect(source, `${testCase.name} visual source ${edge.source}`).toBeTruthy(); + expect(target, `${testCase.name} visual target ${edge.target}`).toBeTruthy(); + if (source!.data.kind === "optional-group") { + assertContainerHandles("optional-group", source!.data); + } else { + assertRenderedHandles(source!.data.kind, source!.data, { + target: source!.data.kind === "start" ? 0 : 1, + source: source!.data.kind === "end" ? 0 : 1, + }); + } + if (target!.data.kind === "optional-group") { + assertContainerHandles("optional-group", target!.data); + } else { + assertRenderedHandles(target!.data.kind, target!.data, { + target: target!.data.kind === "start" ? 0 : 1, + source: target!.data.kind === "end" ? 0 : 1, + }); + } + } + const { ir: roundTripped } = flowToIr(testCase.ir.name, nodes, edges, columnsOf(makeDef(testCase.ir))); + const persistedGroup = roundTripped.nodes.find((irNode) => irNode.id === testCase.id); + const persistedTemplate = persistedGroup?.config?.template as { edges?: { from: string; to: string }[] } | undefined; + expect(roundTripped.edges.some((edge) => edge.from.includes("::") || edge.to.includes("::")), testCase.name).toBe(false); + expect(persistedTemplate?.edges?.some((edge) => edge.from === testCase.id || edge.to === testCase.id), testCase.name).toBe(false); + } } assertRunDoesNotOverlap("coding consecutive fallback", irToFlow(makeDef(BUILTIN_CODING_WORKFLOW_IR)).nodes, [ @@ -989,7 +1275,8 @@ describe("workflow-flow-mapping foreach + rework round-trip", () => { "steps", "browser-verification", "code-review", - "review", + // FNXC:WorkflowReviewGates 2026-06-29-23:46: The stepwise built-in no longer has a separate final `review` seam; keep the optional-group overlap guard aligned with the post-code-review completion summary suffix while still proving adjacent optional containers do not visually collide. + "completion-summary", ]); const consecutiveMixedContainers: WorkflowDefinition["ir"] = { @@ -1339,6 +1626,14 @@ describe("edge-condition authoring (U2)", () => { error: "missing-endpoint", }); + // visual-only optional-group boundary handles are reserved for generated guide edges. + expect(buildConnectionEdge({ source: "a", sourceHandle: "optional-boundary-entry", target: "b" }, edges, nodes)).toEqual({ + error: "reserved-handle", + }); + expect(buildConnectionEdge({ source: "a", target: "b", targetHandle: "optional-boundary-exit" }, edges, nodes)).toEqual({ + error: "reserved-handle", + }); + // second connect of an existing success pair (prompt source supports // conditions) → births the parallel failure edge rather than rejecting. const failureBirth = buildConnectionEdge({ source: "a", target: "b" }, edges, nodes); @@ -1714,11 +2009,24 @@ describe("insertFragment", () => { const first = insertFragment(existing.nodes, existing.edges, fragmentIr, { x: 400, y: 200 }); const second = insertFragment(first.nodes, first.edges, fragmentIr, { x: 700, y: 200 }); - // Two optional-group containers, each with its template child expanded. + // Two optional-group containers, each with its template child expanded and its + // visual-only boundary wiring present immediately on insertion (before save/reload). const groups = second.nodes.filter((n) => n.data.kind === "optional-group"); expect(groups).toHaveLength(2); for (const g of groups) { - expect(second.nodes.some((n) => n.parentId === g.id)).toBe(true); + const child = second.nodes.find((n) => n.parentId === g.id); + expect(child).toBeTruthy(); + expect(child?.data.optionalGroupBoundary).toEqual({ entry: true, exit: true }); + expect( + second.edges.some( + (edge) => isVisualOnlyWorkflowEdge(edge) && edge.source === g.id && edge.target === child?.id, + ), + ).toBe(true); + expect( + second.edges.some( + (edge) => isVisualOnlyWorkflowEdge(edge) && edge.source === child?.id && edge.target === g.id, + ), + ).toBe(true); } // All ids disjoint across both inserts. const allIds = second.nodes.map((n) => n.id); @@ -1735,11 +2043,62 @@ describe("insertFragment", () => { expect(ogs).toHaveLength(2); for (const og of ogs) { expect(og.config?.defaultOn).toBe(true); - const template = (og.config as { template?: { nodes: { config?: Record }[] } }).template; + const template = (og.config as { template?: { nodes: { config?: Record }[]; edges?: unknown[] } }).template; expect(template?.nodes).toHaveLength(1); expect(template?.nodes[0].config?.name).toBe("Security Audit"); + expect(template?.nodes[0].config?.optionalGroupBoundary).toBeUndefined(); + expect(template?.edges).toEqual([]); } }); + + it("expands inserted multi-node optional groups with boundary metadata without dropping internal edges", () => { + const fragment: WorkflowDefinition["ir"] = { + version: "v2", + name: "insert optional chain", + columns: [], + nodes: [ + { id: "start", kind: "start" }, + { + id: "opt", + kind: "optional-group", + config: { + template: { + nodes: [ + { id: "prepare", kind: "prompt", config: { prompt: "prepare" } }, + { id: "approve", kind: "gate", config: { prompt: "approve" } }, + ], + edges: [{ from: "prepare", to: "approve", condition: "success" }], + }, + }, + }, + { id: "end", kind: "end" }, + ], + edges: [ + { from: "start", to: "opt", condition: "success" }, + { from: "opt", to: "end", condition: "success" }, + ], + }; + + const inserted = insertFragment([], [], fragment, { x: 100, y: 200 }); + const group = inserted.nodes.find((node) => node.data.kind === "optional-group")!; + const prepare = inserted.nodes.find((node) => node.parentId === group.id && node.id.endsWith("::prepare"))!; + const approve = inserted.nodes.find((node) => node.parentId === group.id && node.id.endsWith("::approve"))!; + + expect(prepare.data.optionalGroupBoundary).toEqual({ entry: true, exit: false }); + expect(approve.data.optionalGroupBoundary).toEqual({ entry: false, exit: true }); + expect(inserted.edges.some((edge) => edge.source === prepare.id && edge.target === approve.id)).toBe(true); + expect(inserted.edges.filter((edge) => isVisualOnlyWorkflowEdge(edge)).map((edge) => `${edge.source}->${edge.target}`).sort()).toEqual([ + `${group.id}->${prepare.id}`, + `${approve.id}->${group.id}`, + ]); + + const { ir: out } = flowToIr("insert optional chain", inserted.nodes, inserted.edges); + const opt = out.nodes.find((node) => node.kind === "optional-group")!; + const template = (opt.config as { template?: { nodes?: Array<{ config?: Record }>; edges?: unknown[] } }).template; + expect(template?.nodes?.map((node) => node.config?.optionalGroupBoundary)).toEqual([undefined, undefined]); + expect(template?.edges).toEqual([{ from: "prepare", to: "approve", condition: "success" }]); + expect(out.edges).toEqual([]); + }); }); describe("fragmentSeamConflicts", () => { diff --git a/packages/dashboard/app/components/__tests__/workflow-mobile-graph.test.ts b/packages/dashboard/app/components/__tests__/workflow-mobile-graph.test.ts index 623447400a..39b8dd6332 100644 --- a/packages/dashboard/app/components/__tests__/workflow-mobile-graph.test.ts +++ b/packages/dashboard/app/components/__tests__/workflow-mobile-graph.test.ts @@ -150,15 +150,15 @@ describe("buildMobileWorkflowGraph", () => { expect(browserVerification?.outgoing.map((out) => [out.target, out.label]), name).toEqual( expect.arrayContaining([ ["code-review", "success"], - ["end", "failure"], - ]), - ); - expect(codeReview?.outgoing.map((out) => [out.target, out.label]), name).toEqual( - expect.arrayContaining([ - ["review", "success"], - ["end", "failure"], ]), ); + expect(browserVerification?.outgoing.some((out) => out.label === "failure"), name).toBe(true); + expect(codeReview?.outgoing.some((out) => out.label === "success"), name).toBe(true); + expect(codeReview?.outgoing.some((out) => out.label === "failure"), name).toBe(true); + expect(browserVerification?.outgoing.some((out) => out.label === "entry" || out.label === "exit"), name).toBe(false); + expect(codeReview?.outgoing.some((out) => out.label === "entry" || out.label === "exit"), name).toBe(false); + expect(browserVerification?.children.length, name).toBeGreaterThan(0); + expect(codeReview?.children.length, name).toBeGreaterThan(0); } }); diff --git a/packages/dashboard/app/components/nodes/WorkflowNodeTypes.tsx b/packages/dashboard/app/components/nodes/WorkflowNodeTypes.tsx index fc65f5944b..8020669f74 100644 --- a/packages/dashboard/app/components/nodes/WorkflowNodeTypes.tsx +++ b/packages/dashboard/app/components/nodes/WorkflowNodeTypes.tsx @@ -15,6 +15,8 @@ const WORKFLOW_NODE_KIND_GATE: WorkflowNodeKindGate = `${"ga"}te`; const WORKFLOW_NODE_KIND_STEP_REVIEW: WorkflowNodeKindStepReview = `${"st"}ep-review`; const WORKFLOW_NODE_KIND_PARSE_STEPS: WorkflowNodeKindParseSteps = `parse-${"st"}eps`; const WORKFLOW_NODE_SEAM_STEP_EXECUTE = `${"st"}ep-execute`; +const OPTIONAL_GROUP_BOUNDARY_ENTRY_HANDLE = "optional-boundary-entry"; +const OPTIONAL_GROUP_BOUNDARY_EXIT_HANDLE = "optional-boundary-exit"; export type WorkflowEditorNodeKind = | "start" @@ -51,6 +53,11 @@ export interface WorkflowFlowNodeData { templateEmpty?: boolean; /** template group only: the localized empty-state hint string. */ emptyHint?: string; + /** + * FNXC:WorkflowOptionalGroup 2026-06-29-21:37: + * Optional-group template children expose boundary ownership for editor visuals only. Entry/exit flags let mapping and renderer tests prove Plan Review/Code Review single-child blocks are connected to their container without persisting fake topology into the workflow IR. + */ + optionalGroupBoundary?: { entry: boolean; exit: boolean }; [key: string]: unknown; } @@ -202,6 +209,12 @@ function LoopGroupNode({ data }: { data: WorkflowFlowNodeData }) { /* FNXC:WorkflowOptionalGroup 2026-06-21-11:30: An `optional-group` renders as a React Flow group container (mirroring `ForeachGroupNode`/`LoopGroupNode`): template nodes are children (parentId = group id). The header shows the group name plus a `defaultOn` badge ("default on" / "default off") so an author can see, at a glance, whether new tasks enable this group. An unregistered kind falls back to `react-flow__node-default` with missing children — registration in `workflowNodeTypes` (below) is what keeps the container rendering with its body. + +FNXC:WorkflowOptionalGroup 2026-06-29-22:47: +Optional-group containers own the real workflow entry and exit boundaries. Keep the standard left target/right source handles for top-level graph edges, and add dedicated left source/right target handles for visual-only template boundary connectors so entry and exit guides attach to the side that matches execution flow. + +FNXC:WorkflowOptionalGroup 2026-06-29-23:20: +The visual-only boundary connectors must never become authorable topology. Mark their dedicated handles non-connectable so users cannot drag persisted edges from the entry/exit guides into the optional group's template children. */ function OptionalGroupNode({ data }: { data: WorkflowFlowNodeData }) { const { t } = useTranslation("app"); @@ -213,6 +226,7 @@ function OptionalGroupNode({ data }: { data: WorkflowFlowNodeData }) { data-testid="wf-node-optional-group" > +
@@ -230,6 +244,7 @@ function OptionalGroupNode({ data }: { data: WorkflowFlowNodeData }) {
)} {data.errorBadge && } + ); diff --git a/packages/dashboard/app/components/workflow-flow-mapping.ts b/packages/dashboard/app/components/workflow-flow-mapping.ts index cf88cbaee1..b526dd08c3 100644 --- a/packages/dashboard/app/components/workflow-flow-mapping.ts +++ b/packages/dashboard/app/components/workflow-flow-mapping.ts @@ -47,6 +47,9 @@ interface WorkflowOptionalGroupConfig { template: { nodes: WorkflowIrNode[]; edges: WorkflowIrEdge[] }; } +const OPTIONAL_GROUP_BOUNDARY_ENTRY_HANDLE = "optional-boundary-entry"; +const OPTIONAL_GROUP_BOUNDARY_EXIT_HANDLE = "optional-boundary-exit"; + // WorkflowFieldDefinition is imported from @fusion/core above (KTD-13/14). // Re-exported so existing importers that reference WorkflowFieldDefinitionShape // can migrate; callers should prefer WorkflowFieldDefinition directly. @@ -282,6 +285,34 @@ function optionalGroupConfigOf(node: WorkflowIrNode): WorkflowOptionalGroupConfi return cfg as WorkflowOptionalGroupConfig; } +/* +FNXC:WorkflowOptionalGroup 2026-06-29-20:10: +Optional-group template entry/exit connectivity is visually owned by the container's outer handles. Child boundary handles must not imply disconnected IR edges, so derive child boundary metadata from forward internal template edges only; rework loops route backward and cannot erase the review-step exit or execute-step entry. +*/ +function optionalGroupTemplateBoundaryById( + template: WorkflowOptionalGroupConfig["template"], +): Map { + const templateNodeIds = new Set(template.nodes.map((node) => node.id)); + const incomingForward = new Set(); + const outgoingForward = new Set(); + + for (const edge of template.edges) { + if (edge.kind === "rework") continue; + if (!templateNodeIds.has(edge.from) || !templateNodeIds.has(edge.to)) continue; + incomingForward.add(edge.to); + outgoingForward.add(edge.from); + } + + const boundaries = new Map(); + for (const node of template.nodes) { + boundaries.set(node.id, { + entry: !incomingForward.has(node.id), + exit: !outgoingForward.has(node.id), + }); + } + return boundaries; +} + function groupTemplateConfigOf( node: WorkflowIrNode, ): WorkflowForeachConfig | WorkflowLoopConfig | WorkflowOptionalGroupConfig | undefined { @@ -384,6 +415,12 @@ function irEdgeToFlow(edge: WorkflowIrEdge, index: number, idScope = ""): FlowEd * visually thin. Applied per-edge (defaultEdgeOptions only seeds new edges). */ export const WF_EDGE_INTERACTION_WIDTH = 24; +export const WF_TEMPLATE_BOUNDARY_EDGE_KIND = "template-boundary"; + +export function isVisualOnlyWorkflowEdge(edge: FlowEdge): boolean { + return edge.data?.visualOnly === WF_TEMPLATE_BOUNDARY_EDGE_KIND; +} + /** Short display label for an edge condition. `outcome:` conditions * render as the verdict alone (KTD-4); everything else verbatim. */ export function shortConditionLabel(condition: string): string { @@ -391,6 +428,163 @@ export function shortConditionLabel(condition: string): string { return condition; } +function templateBoundaryNodeIds(template: { nodes: WorkflowIrNode[]; edges: WorkflowIrEdge[] }): { entryIds: string[]; exitIds: string[] } { + const incoming = new Map(); + const outgoing = new Map(); + for (const node of template.nodes) { + incoming.set(node.id, 0); + outgoing.set(node.id, 0); + } + for (const edge of template.edges) { + if (edge.kind === "rework") continue; + if (!incoming.has(edge.to) || !outgoing.has(edge.from)) continue; + incoming.set(edge.to, (incoming.get(edge.to) ?? 0) + 1); + outgoing.set(edge.from, (outgoing.get(edge.from) ?? 0) + 1); + } + return { + entryIds: template.nodes.filter((node) => (incoming.get(node.id) ?? 0) === 0).map((node) => node.id), + exitIds: template.nodes.filter((node) => (outgoing.get(node.id) ?? 0) === 0).map((node) => node.id), + }; +} + +/* + * FNXC:WorkflowOptionalGroup 2026-06-29-20:41: + * Single-node optional groups such as Plan Review and Code Review looked disconnected because their executable template child had no internal template edge. Add read-only boundary connector edges in React Flow so the child visibly participates in the block, but mark them visual-only and filter them out of save/mobile serialization so the workflow IR keeps the real optional-group entry/exit contract. Boundary connectors use the same forward-edge-only rule as child handle metadata because rework loops are review routing, not alternate optional-group entry/exit ownership. + * + * FNXC:WorkflowOptionalGroup 2026-06-29-20:56: + * Surface enumeration for FN-7249 keeps the fix constrained to editor visualization surfaces: desktop React Flow handles/edges, mobile outline filtering, parentId template children, and built-in Plan Review/Code Review single-child optional groups. Preserve saved/manual layouts and the core optional-group execution contract while repairing only visual child-boundary connectivity. + * + * FNXC:WorkflowOptionalGroup 2026-06-29-21:25: + * Optional groups may have multiple independent template entries or exits. Emit one visual-only connector per boundary child so boundary-handle suppression never creates a disconnected child with no corresponding container-owned visual path. + * + * FNXC:WorkflowOptionalGroup 2026-06-29-22:16: + * Boundary connector edges are explanatory editor chrome, not workflow topology. Keep them non-selectable and non-deletable so authors cannot mistake the visual entry/exit guides for persisted optional-group template edges. + * + * FNXC:WorkflowOptionalGroup 2026-06-29-22:47: + * Boundary connector edges must attach entry guides to a left-side container source handle and exit guides to a right-side container target handle. The normal optional-group target/source handles remain reserved for top-level workflow edges, so visual-only child connectors do not reverse the perceived execution boundary. + */ +function optionalGroupBoundaryEdgesForFlowIds(groupId: string, entryFlowIds: readonly string[], exitFlowIds: readonly string[]): FlowEdge[] { + const visualEdges: FlowEdge[] = []; + for (const entryFlowId of entryFlowIds) { + const entryId = templateNodeIdFromChild(groupId, entryFlowId); + visualEdges.push({ + id: `e-${groupId}-boundary-entry-${entryId}`, + source: groupId, + sourceHandle: OPTIONAL_GROUP_BOUNDARY_ENTRY_HANDLE, + target: entryFlowId, + label: "entry", + data: { condition: "entry", visualOnly: WF_TEMPLATE_BOUNDARY_EDGE_KIND, boundary: "entry" }, + className: "wf-edge-template-boundary", + interactionWidth: WF_EDGE_INTERACTION_WIDTH, + selectable: false, + deletable: false, + markerEnd: undefined, + zIndex: WF_EDGE_Z_INDEX, + }); + } + for (const exitFlowId of exitFlowIds) { + const exitId = templateNodeIdFromChild(groupId, exitFlowId); + visualEdges.push({ + id: `e-${groupId}-boundary-exit-${exitId}`, + source: exitFlowId, + target: groupId, + targetHandle: OPTIONAL_GROUP_BOUNDARY_EXIT_HANDLE, + label: "exit", + data: { condition: "exit", visualOnly: WF_TEMPLATE_BOUNDARY_EDGE_KIND, boundary: "exit" }, + className: "wf-edge-template-boundary", + interactionWidth: WF_EDGE_INTERACTION_WIDTH, + selectable: false, + deletable: false, + markerEnd: undefined, + zIndex: WF_EDGE_Z_INDEX, + }); + } + return visualEdges; +} + +function optionalGroupBoundaryEdges(node: WorkflowIrNode, template: { nodes: WorkflowIrNode[]; edges: WorkflowIrEdge[] }): FlowEdge[] { + if (node.kind !== "optional-group" || template.nodes.length === 0) return []; + const { entryIds, exitIds } = templateBoundaryNodeIds(template); + return optionalGroupBoundaryEdgesForFlowIds( + node.id, + entryIds.map((entryId) => foreachChildFlowId(node.id, entryId)), + exitIds.map((exitId) => foreachChildFlowId(node.id, exitId)), + ); +} + +/* + * FNXC:WorkflowOptionalGroup 2026-06-29-23:31: + * Optional-group boundary connector edges are derived editor chrome. Recompute them after live canvas node/edge mutations so adding, deleting, or retagging internal template edges immediately moves entry/exit guides without waiting for a save/reload round-trip. + */ +export function refreshOptionalGroupVisualBoundaries( + nodes: FlowNode[], + edges: FlowEdge[], +): { nodes: FlowNode[]; edges: FlowEdge[] } { + const childrenByGroup = new Map[]>(); + for (const node of nodes) { + if (!node.parentId) continue; + const arr = childrenByGroup.get(node.parentId) ?? []; + arr.push(node); + childrenByGroup.set(node.parentId, arr); + } + + const groupIds = new Set( + nodes + .filter((node) => node.data.kind === "optional-group") + .map((node) => node.id), + ); + const childToOptionalGroup = new Map(); + for (const groupId of groupIds) { + for (const child of childrenByGroup.get(groupId) ?? []) childToOptionalGroup.set(child.id, groupId); + } + + const nonVisualEdges = edges.filter((edge) => !isVisualOnlyWorkflowEdge(edge)); + const boundaryByChild = new Map(); + const nextVisualEdges: FlowEdge[] = []; + + for (const groupId of groupIds) { + const children = childrenByGroup.get(groupId) ?? []; + if (children.length === 0) continue; + const childIds = new Set(children.map((child) => child.id)); + const incomingForward = new Set(); + const outgoingForward = new Set(); + for (const edge of nonVisualEdges) { + if ((edge.data?.kind as string | undefined) === "rework") continue; + if (!childIds.has(edge.source) || !childIds.has(edge.target)) continue; + outgoingForward.add(edge.source); + incomingForward.add(edge.target); + } + + const entryFlowIds: string[] = []; + const exitFlowIds: string[] = []; + for (const child of children) { + const boundary = { + entry: !incomingForward.has(child.id), + exit: !outgoingForward.has(child.id), + }; + boundaryByChild.set(child.id, boundary); + if (boundary.entry) entryFlowIds.push(child.id); + if (boundary.exit) exitFlowIds.push(child.id); + } + nextVisualEdges.push(...optionalGroupBoundaryEdgesForFlowIds(groupId, entryFlowIds, exitFlowIds)); + } + + const nextNodes = nodes.map((node) => { + const optionalGroupId = childToOptionalGroup.get(node.id); + if (!optionalGroupId) { + if (!node.data.optionalGroupBoundary) return node; + const { optionalGroupBoundary: _boundary, ...data } = node.data; + return { ...node, data }; + } + const boundary = boundaryByChild.get(node.id); + if (!boundary) return node; + if (node.data.optionalGroupBoundary?.entry === boundary.entry && node.data.optionalGroupBoundary?.exit === boundary.exit) return node; + return { ...node, data: { ...node.data, optionalGroupBoundary: boundary } }; + }); + + return { nodes: nextNodes, edges: [...nonVisualEdges, ...nextVisualEdges] }; +} + /** Build React Flow nodes/edges from a stored workflow definition. v2 columns * render as swimlane band group nodes; step nodes carry their `column`. A * `foreach` node renders as a group whose template subgraph nodes are children @@ -426,6 +620,9 @@ export function irToFlow(def: WorkflowDefinition): { const groupCfg = groupTemplateConfigOf(node); if (groupCfg) { const template = groupCfg.template; + const optionalGroupBoundaries = node.kind === "optional-group" + ? optionalGroupTemplateBoundaryById(template) + : undefined; // Render template nodes as children of this group (parentId = group id). template.nodes.forEach((inner, innerIdx) => { const childFlowId = foreachChildFlowId(node.id, inner.id); @@ -436,13 +633,20 @@ export function irToFlow(def: WorkflowDefinition): { y: FOREACH_CHILD_Y, }; const innerKind = editorKind(inner); + const optionalGroupBoundary = optionalGroupBoundaries?.get(inner.id); childNodes.push({ id: childFlowId, type: innerKind, position: childPos, parentId: node.id, extent: "parent", - data: { kind: innerKind, ...dataIrKind(inner, innerKind), label: nodeLabel(inner), config: { ...(inner.config ?? {}) } }, + data: { + kind: innerKind, + ...dataIrKind(inner, innerKind), + label: nodeLabel(inner), + config: { ...(inner.config ?? {}) }, + ...(optionalGroupBoundary ? { optionalGroupBoundary } : {}), + }, deletable: true, zIndex: WF_STEP_NODE_Z_INDEX, }); @@ -450,6 +654,7 @@ export function irToFlow(def: WorkflowDefinition): { template.edges.forEach((edge, eIdx) => { childEdges.push(irEdgeToFlow(edge, eIdx, `${node.id}${FOREACH_CHILD_SEP}`)); }); + childEdges.push(...optionalGroupBoundaryEdges(node, template)); // Strip the template off the group node's own config (children carry it). const { template: _t, ...restCfg } = (node.config ?? {}) as Record; return { @@ -596,7 +801,7 @@ export function flowToIr( }); const childIdSet = new Set(children.map((c) => c.id)); const templateEdges: WorkflowIrEdge[] = edges - .filter((e) => childIdSet.has(e.source) && childIdSet.has(e.target)) + .filter((e) => !isVisualOnlyWorkflowEdge(e) && childIdSet.has(e.source) && childIdSet.has(e.target)) .map((e) => flowEdgeToIr(e, node.id)); const baseCfg = (config ?? {}) as Record; return { @@ -627,6 +832,7 @@ export function flowToIr( for (const [gid, kids] of childrenByGroup) for (const k of kids) childIdToGroup.set(k.id, gid); const irEdges: WorkflowIr["edges"] = edges .filter((e) => { + if (isVisualOnlyWorkflowEdge(e)) return false; const sg = childIdToGroup.get(e.source); const tg = childIdToGroup.get(e.target); return !(sg && tg && sg === tg); @@ -786,6 +992,7 @@ export function wouldCreateCycle(edges: FlowEdge[], source: string, target: stri // Build adjacency over non-rework edges only. const adj = new Map(); for (const e of edges) { + if (isVisualOnlyWorkflowEdge(e)) continue; if ((e.data?.kind as string | undefined) === "rework") continue; const arr = adj.get(e.source) ?? []; arr.push(e.target); @@ -825,7 +1032,11 @@ export function newNodeId(): string { /** Result of attempting to build an edge from a React Flow connection. */ export type BuildConnectionResult = | { edge: FlowEdge } - | { error: "missing-endpoint" | "duplicate" | "cycle" }; + | { error: "missing-endpoint" | "duplicate" | "cycle" | "reserved-handle" }; + +function isOptionalGroupBoundaryConnectionHandle(handleId: string | null | undefined): boolean { + return handleId === OPTIONAL_GROUP_BOUNDARY_ENTRY_HANDLE || handleId === OPTIONAL_GROUP_BOUNDARY_EXIT_HANDLE; +} /** Construct a new success edge for a React Flow connection, reimplementing the * sanity guards React Flow's addEdge provided (KTD-3) plus the author-time cycle @@ -839,7 +1050,7 @@ export type BuildConnectionResult = * cycles are authored separately and exempt). */ export function buildConnectionEdge( - connection: { source?: string | null; target?: string | null }, + connection: { source?: string | null; target?: string | null; sourceHandle?: string | null; targetHandle?: string | null }, edges: FlowEdge[], nodes: FlowNode[], ): BuildConnectionResult { @@ -847,13 +1058,24 @@ export function buildConnectionEdge( const target = connection.target ?? undefined; if (!source || !target) return { error: "missing-endpoint" }; + /* + * FNXC:WorkflowOptionalGroup 2026-06-29-23:20: + * Optional-group boundary handles are visual guide anchors owned by refreshOptionalGroupVisualBoundaries, not editable workflow topology. Reject connection gestures that mention them so stale DOM, test mocks, or browser quirks cannot persist a fake group↔child edge if React Flow ever reports a boundary handle as connectable. + */ + if ( + isOptionalGroupBoundaryConnectionHandle(connection.sourceHandle) || + isOptionalGroupBoundaryConnectionHandle(connection.targetHandle) + ) { + return { error: "reserved-handle" }; + } + const srcNode = nodes.find((n) => n.id === source); const tgtNode = nodes.find((n) => n.id === target); // Existing conditions already authored between this exact pair. const existingConditions = new Set( edges - .filter((e) => e.source === source && e.target === target) + .filter((e) => !isVisualOnlyWorkflowEdge(e) && e.source === source && e.target === target) .map((e) => (e.data?.condition as string | undefined) ?? "success"), ); @@ -1251,6 +1473,9 @@ export function insertFragment( if (groupCfg) { const template = groupCfg.template; const groupKind = editorKind(node); + const optionalGroupBoundaries = node.kind === "optional-group" + ? optionalGroupTemplateBoundaryById(template) + : undefined; template.nodes.forEach((inner, innerIdx) => { const innerKind = editorKind(inner); const childPos = @@ -1258,13 +1483,20 @@ export function insertFragment( x: FOREACH_CHILD_X + innerIdx * FOREACH_CHILD_STEP_X, y: FOREACH_CHILD_Y, }; + const optionalGroupBoundary = optionalGroupBoundaries?.get(inner.id); childNodes.push({ id: foreachChildFlowId(id, inner.id), type: innerKind, position: childPos, parentId: id, extent: "parent", - data: { kind: innerKind, ...dataIrKind(inner, innerKind), label: nodeLabel(inner), config: { ...(inner.config ?? {}) } }, + data: { + kind: innerKind, + ...dataIrKind(inner, innerKind), + label: nodeLabel(inner), + config: { ...(inner.config ?? {}) }, + ...(optionalGroupBoundary ? { optionalGroupBoundary } : {}), + }, deletable: true, zIndex: WF_STEP_NODE_Z_INDEX, }); @@ -1272,6 +1504,7 @@ export function insertFragment( template.edges.forEach((edge, eIdx) => { childEdges.push(irEdgeToFlow(edge, eIdx, `${id}${FOREACH_CHILD_SEP}`)); }); + childEdges.push(...optionalGroupBoundaryEdges({ ...node, id }, template)); // The group node keeps everything except the template (children carry it). const { template: _t, ...restCfg } = (node.config ?? {}) as Record; return { diff --git a/packages/dashboard/app/components/workflow-mobile-graph.ts b/packages/dashboard/app/components/workflow-mobile-graph.ts index 66174c242e..6698f9cced 100644 --- a/packages/dashboard/app/components/workflow-mobile-graph.ts +++ b/packages/dashboard/app/components/workflow-mobile-graph.ts @@ -4,6 +4,7 @@ import type { WorkflowFlowNodeData } from "./nodes/WorkflowNodeTypes"; import { columnIdFromBandNode, isColumnBandNode, + isVisualOnlyWorkflowEdge, templateNodeIdFromChild, } from "./workflow-flow-mapping"; import { nodeConfigSummary, type NodeSummaryCatalogs, type SummaryTranslate } from "./nodes/node-summary"; @@ -131,6 +132,7 @@ export function buildMobileWorkflowGraph( } for (const edge of edges) { + if (isVisualOnlyWorkflowEdge(edge)) continue; const list = edgesBySource.get(edge.source) ?? []; list.push(edge); edgesBySource.set(edge.source, list);