diff --git a/.changeset/fn-7016-optional-group-edges.md b/.changeset/fn-7016-optional-group-edges.md new file mode 100644 index 0000000000..26ae938ee3 --- /dev/null +++ b/.changeset/fn-7016-optional-group-edges.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix workflow editor so the Browser Verification block shows connected edges. +category: fix +dev: optional-group/foreach/loop container nodes now render connectable handles without adjacent layer overlap in WorkflowNodeEditor. diff --git a/packages/dashboard/app/components/__tests__/workflow-auto-layout.test.ts b/packages/dashboard/app/components/__tests__/workflow-auto-layout.test.ts index 4a19724167..2c1ad86765 100644 --- a/packages/dashboard/app/components/__tests__/workflow-auto-layout.test.ts +++ b/packages/dashboard/app/components/__tests__/workflow-auto-layout.test.ts @@ -8,6 +8,8 @@ import { bandTop, columnBandNodeId, COLUMN_BAND_HEIGHT, + FOREACH_GROUP_WIDTH, + WF_CARD_MAX_WIDTH, } from "../workflow-flow-mapping"; type N = FlowNode; @@ -156,6 +158,22 @@ describe("autoLayout — v1 (free placement)", () => { }); describe("autoLayout — foreach / unreachable / cycles", () => { + it("spaces layers by container width so group handles do not overlap following nodes", () => { + const nodes: N[] = [ + node("start", "start", 0, midBand(0), { column: "triage" }), + node("verify", "optional-group", 0, midBand(1), { + column: "in-progress", + style: { width: FOREACH_GROUP_WIDTH, height: 220 }, + }), + node("review", "prompt", 0, midBand(1), { column: "in-progress" }), + node("end", "end", 0, midBand(2), { column: "done" }), + ]; + const pos = autoLayout(nodes, [edge("start", "verify"), edge("verify", "review"), edge("review", "end")], COLUMNS_3); + + expect(pos.get("verify")!.x).toBeGreaterThanOrEqual(pos.get("start")!.x + WF_CARD_MAX_WIDTH); + expect(pos.get("review")!.x).toBeGreaterThanOrEqual(pos.get("verify")!.x + FOREACH_GROUP_WIDTH); + }); + it("repositions a foreach group but leaves its parentId children untouched", () => { const childPos = { x: 30, y: 56 }; const nodes: N[] = [ 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 ab16bb6cab..e5d127b3f3 100644 --- a/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts +++ b/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts @@ -1,7 +1,15 @@ +import { createElement } from "react"; +import { render } from "@testing-library/react"; import { describe, expect, it } from "vitest"; import type { WorkflowDefinition, WorkflowIrNodeKind } from "@fusion/core"; -import { parseWorkflowIr, validateColumnTraits } from "@fusion/core"; -import type { Node as FlowNode } from "@xyflow/react"; +import { + BUILTIN_CODING_WORKFLOW_IR, + BUILTIN_STEPWISE_CODING_WORKFLOW_IR, + parseWorkflowIr, + validateColumnTraits, +} from "@fusion/core"; +import type { Edge as FlowEdge, Node as FlowNode } from "@xyflow/react"; +import { ReactFlowProvider } from "@xyflow/react"; import { irToFlow, flowToIr, @@ -34,22 +42,43 @@ import { FOREACH_CHILD_X, FOREACH_CHILD_Y, } from "../workflow-flow-mapping"; +import { workflowNodeTypes } from "../nodes/WorkflowNodeTypes"; import type { WorkflowEditorNodeKind, WorkflowFlowNodeData } from "../nodes/WorkflowNodeTypes"; import type { TraitCatalogEntry } from "../../api"; -function makeDef(ir: WorkflowDefinition["ir"]): WorkflowDefinition { +function makeDef(ir: WorkflowDefinition["ir"], layout: WorkflowDefinition["layout"] = {}): WorkflowDefinition { return { id: "WF-001", kind: "workflow", name: ir.name, description: "", ir, - layout: {}, + layout, createdAt: "2024-01-01T00:00:00.000Z", updatedAt: "2024-01-01T00:00:00.000Z", }; } +function nodeWidth(node: FlowNode): number { + const width = node.style?.width; + return typeof width === "number" ? width : WF_CARD_WIDTH; +} + +function assertContainerHandles(kind: "optional-group" | "foreach" | "loop", data: WorkflowFlowNodeData): void { + const Component = workflowNodeTypes[kind]; + const { container, unmount } = render( + createElement(ReactFlowProvider, null, createElement(Component, { data, id: `${kind}-handle-check` })), + ); + 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); + } finally { + unmount(); + } +} + describe("workflow-flow-mapping name preservation", () => { it("does not inject synthetic names for unnamed start/end/merge nodes on round-trip", () => { const ir: WorkflowDefinition["ir"] = { @@ -829,6 +858,123 @@ describe("workflow-flow-mapping foreach + rework round-trip", () => { expect(out.edges.map((e) => `${e.from}->${e.to}`)).toEqual(["start->opt", "opt->end"]); }); + /* + * 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. + */ + it("keeps container edges and handles connected across built-in workflow containers", () => { + const loopNode = { + id: "diagnostic-loop", + kind: "loop" as const, + column: "in-progress", + config: { + maxIterations: 2, + template: { + nodes: [{ id: "loop-check", kind: "prompt" as const, config: { prompt: "Check once" } }], + edges: [], + }, + }, + }; + const codingWithLoop: WorkflowDefinition["ir"] = { + ...BUILTIN_CODING_WORKFLOW_IR, + name: "builtin-coding-with-loop-test", + nodes: BUILTIN_CODING_WORKFLOW_IR.nodes.flatMap((node) => + node.id === "browser-verification" ? [loopNode, node] : [node], + ), + edges: [ + ...BUILTIN_CODING_WORKFLOW_IR.edges.filter( + (edge) => !(edge.from === "execute" && edge.to === "browser-verification"), + ), + { from: "execute", to: "diagnostic-loop", condition: "success" }, + { from: "diagnostic-loop", to: "browser-verification", condition: "success" }, + ], + }; + const stepwiseLoopNode = { ...loopNode, id: "post-steps-loop" }; + const stepwiseWithLoop: WorkflowDefinition["ir"] = { + ...BUILTIN_STEPWISE_CODING_WORKFLOW_IR, + name: "builtin-stepwise-with-loop-test", + nodes: BUILTIN_STEPWISE_CODING_WORKFLOW_IR.nodes.flatMap((node) => + node.id === "browser-verification" ? [stepwiseLoopNode, node] : [node], + ), + edges: [ + ...BUILTIN_STEPWISE_CODING_WORKFLOW_IR.edges.filter( + (edge) => !(edge.from === "steps" && edge.to === "browser-verification"), + ), + { from: "steps", to: "post-steps-loop", condition: "success" }, + { from: "post-steps-loop", to: "browser-verification", condition: "success" }, + ], + }; + + const cases: Array<{ + name: string; + ir: WorkflowDefinition["ir"]; + id: string; + kind: "optional-group" | "foreach" | "loop"; + }> = [ + { name: "coding browser verification", ir: BUILTIN_CODING_WORKFLOW_IR, id: "browser-verification", kind: "optional-group" }, + { name: "stepwise browser verification", ir: BUILTIN_STEPWISE_CODING_WORKFLOW_IR, id: "browser-verification", kind: "optional-group" }, + { name: "stepwise foreach", ir: BUILTIN_STEPWISE_CODING_WORKFLOW_IR, id: "steps", kind: "foreach" }, + { name: "coding inserted loop", ir: codingWithLoop, id: "diagnostic-loop", kind: "loop" }, + { name: "stepwise inserted loop", ir: stepwiseWithLoop, id: "post-steps-loop", kind: "loop" }, + ]; + + for (const testCase of cases) { + const { nodes, edges } = irToFlow(makeDef(testCase.ir)); + const byId = new Map(nodes.map((node) => [node.id, node] as const)); + const group = byId.get(testCase.id); + expect(group, testCase.name).toBeTruthy(); + expect(group?.type, testCase.name).toBe(testCase.kind); + expect(group?.data.kind, testCase.name).toBe(testCase.kind); + 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); + 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); + expect(edge.target.includes("::"), `${testCase.name} target should be top-level`).toBe(false); + expect(edge.zIndex, `${testCase.name} edge should layer above group`).toBeGreaterThan(group!.zIndex ?? 0); + const source = byId.get(edge.source); + const target = byId.get(edge.target); + expect(source, `${testCase.name} source node ${edge.source}`).toBeTruthy(); + expect(target, `${testCase.name} target node ${edge.target}`).toBeTruthy(); + if (source && target) { + const sourceRight = source.position.x + nodeWidth(source); + const targetRight = target.position.x + nodeWidth(target); + if (source.position.x <= target.position.x) { + expect(sourceRight, `${testCase.name} ${edge.source}->${edge.target} should not overlap`).toBeLessThanOrEqual(target.position.x); + } else { + expect(targetRight, `${testCase.name} ${edge.source}->${edge.target} should not overlap`).toBeLessThanOrEqual(source.position.x); + } + } + } + + const templateEdges = edges.filter((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 staleBuiltInLayout: WorkflowDefinition["layout"] = { + start: { x: 60, y: 160 }, + execute: { x: 230, y: 160 }, + review: { x: 400, y: 160 }, + end: { x: 1420, y: 160 }, + }; + const { nodes: staleNodes, edges: staleEdges } = irToFlow(makeDef(BUILTIN_CODING_WORKFLOW_IR, staleBuiltInLayout)); + const staleById = new Map(staleNodes.map((node) => [node.id, node] as const)); + for (const edge of staleEdges.filter((candidate) => candidate.source === "browser-verification" || candidate.target === "browser-verification")) { + const source = staleById.get(edge.source)!; + const target = staleById.get(edge.target)!; + const sourceRight = source.position.x + nodeWidth(source); + const targetRight = target.position.x + nodeWidth(target); + if (source.position.x <= target.position.x) { + expect(sourceRight, `stale layout ${edge.source}->${edge.target} should not overlap`).toBeLessThanOrEqual(target.position.x); + } else { + expect(targetRight, `stale layout ${edge.source}->${edge.target} should not overlap`).toBeLessThanOrEqual(source.position.x); + } + } + }); + // FNXC:WorkflowOptionalGroup 2026-06-21-11:30: Deleting an optional-group must // cascade its parentId children (no orphans) — same rule foreach/loop follow. it("cascade-deletes an optional-group's template children", () => { 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 037f015736..6a3a8c6634 100644 --- a/packages/dashboard/app/components/__tests__/workflow-mobile-graph.test.ts +++ b/packages/dashboard/app/components/__tests__/workflow-mobile-graph.test.ts @@ -1,8 +1,9 @@ import { describe, expect, it } from "vitest"; +import { BUILTIN_CODING_WORKFLOW_IR, BUILTIN_STEPWISE_CODING_WORKFLOW_IR } from "@fusion/core"; import type { Edge as FlowEdge, Node as FlowNode } from "@xyflow/react"; import { buildMobileWorkflowGraph, reorderWorkflowNode } from "../workflow-mobile-graph"; import type { WorkflowFlowNodeData } from "../nodes/WorkflowNodeTypes"; -import { columnBandNodeId, foreachChildFlowId } from "../workflow-flow-mapping"; +import { columnBandNodeId, foreachChildFlowId, irToFlow } from "../workflow-flow-mapping"; function node( id: string, @@ -30,6 +31,19 @@ function edge(id: string, source: string, target: string, condition = "success") }; } +function workflowDef(ir: typeof BUILTIN_CODING_WORKFLOW_IR) { + return { + id: ir.name, + kind: "workflow" as const, + name: ir.name, + description: "", + ir, + layout: {}, + createdAt: "2024-01-01T00:00:00.000Z", + updatedAt: "2024-01-01T00:00:00.000Z", + }; +} + function rowOrder(nodes: FlowNode[]): string[] { return buildMobileWorkflowGraph(nodes, []).map((row) => row.id); } @@ -119,6 +133,27 @@ describe("buildMobileWorkflowGraph", () => { expect(rows[1].summary).toBe("Gate (blocks)"); }); + it("summarizes Browser Verification container connections for built-in mobile outlines", () => { + for (const [name, ir, incoming] of [ + ["coding", BUILTIN_CODING_WORKFLOW_IR, "execute"], + ["stepwise", BUILTIN_STEPWISE_CODING_WORKFLOW_IR, "steps"], + ] as const) { + const { nodes, edges } = irToFlow(workflowDef(ir)); + const rows = buildMobileWorkflowGraph(nodes, edges, ir.version === "v2" ? ir.columns : []); + const browserVerification = rows.find((row) => row.id === "browser-verification"); + const incomingRow = rows.find((row) => row.id === incoming); + + expect(browserVerification?.kind, name).toBe("optional-group"); + expect(incomingRow?.outgoing.some((out) => out.target === "browser-verification"), name).toBe(true); + expect(browserVerification?.outgoing.map((out) => [out.target, out.label]), name).toEqual( + expect.arrayContaining([ + ["code-review", "success"], + ["end", "failure"], + ]), + ); + } + }); + it("preserves branch edges and column labels while ignoring column band nodes", () => { const rows = buildMobileWorkflowGraph( [ diff --git a/packages/dashboard/app/components/workflow-auto-layout.ts b/packages/dashboard/app/components/workflow-auto-layout.ts index b2c200188b..fd4c51a830 100644 --- a/packages/dashboard/app/components/workflow-auto-layout.ts +++ b/packages/dashboard/app/components/workflow-auto-layout.ts @@ -54,6 +54,11 @@ function isLayoutable(node: LayoutNode): boolean { return true; } +function layoutNodeWidth(node: LayoutNode): number { + const width = node.style?.width; + return typeof width === "number" ? width : WF_CARD_MAX_WIDTH; +} + /** * Assign each layoutable node a layer index via longest-path layering from the * start node. Rework edges (data.kind === "rework") are ignored for layering. @@ -150,6 +155,25 @@ export function autoLayout( const v2 = columns.length > 0; + /* + * FNXC:WorkflowContainerEdges 2026-06-26-08:05: + * Auto-layout must space graph layers by the widest rendered node in each prior layer. Container nodes render wider than cards, so fixed card-width layer spacing can place the next optional-group/foreach/loop on top of the previous container's source handle. + */ + const layerMaxWidths = new Map(); + for (const [layerIndex, layerIds] of layers) { + layerMaxWidths.set( + layerIndex, + Math.max(...layerIds.map((id) => layoutNodeWidth(byId.get(id)!)), WF_CARD_MAX_WIDTH), + ); + } + const sortedLayerIndexes = [...layers.keys()].sort((a, b) => a - b); + const layerXByIndex = new Map(); + let nextLayerX = ORIGIN_X; + for (const layerIndex of sortedLayerIndexes) { + layerXByIndex.set(layerIndex, nextLayerX); + nextLayerX += (layerMaxWidths.get(layerIndex) ?? WF_CARD_MAX_WIDTH) + WF_AUTO_LAYOUT_GAP_X; + } + for (const [layerIndex, layerIds] of layers) { const sorted = [...layerIds].sort((a, b) => { const na = byId.get(a)!; @@ -158,7 +182,7 @@ export function autoLayout( return a < b ? -1 : a > b ? 1 : 0; }); - const layerX = ORIGIN_X + layerIndex * WF_AUTO_LAYOUT_SPACING; + const layerX = layerXByIndex.get(layerIndex) ?? ORIGIN_X; if (!v2) { // v1: free placement — within-layer index × row height. @@ -203,7 +227,7 @@ export function autoLayout( const y = firstY + rowInBucket * ROW_HEIGHT; // Stagger x by half-spacing per overflow bucket so wrapped rows don't // collide with the un-staggered column while staying in the same band. - const x = layerX + staggerBucket * (WF_AUTO_LAYOUT_SPACING / 2); + const x = layerX + staggerBucket * ((layerMaxWidths.get(layerIndex) ?? WF_CARD_MAX_WIDTH) + WF_AUTO_LAYOUT_GAP_X) / 2; positions.set(id, { x, y }); rowsPerColumn.set(colId ?? `__idx${safeColIndex}`, used + 1); diff --git a/packages/dashboard/app/components/workflow-flow-mapping.ts b/packages/dashboard/app/components/workflow-flow-mapping.ts index 66b5b9e67d..66f5450c26 100644 --- a/packages/dashboard/app/components/workflow-flow-mapping.ts +++ b/packages/dashboard/app/components/workflow-flow-mapping.ts @@ -78,6 +78,8 @@ export const FOREACH_GROUP_HEIGHT = 220; export const FOREACH_CHILD_X = 30; export const FOREACH_CHILD_Y = 56; export const FOREACH_CHILD_STEP_X = 260; +export const WF_FALLBACK_GRAPH_X = 80; +export const WF_FALLBACK_NODE_GAP = WF_CARD_WIDTH / 2; const FOREACH_CHILD_SEP = "::"; /** Compose a globally-unique flow-node id for a template child. */ @@ -286,6 +288,24 @@ function groupTemplateConfigOf( return foreachConfigOf(node) ?? loopConfigOf(node) ?? optionalGroupConfigOf(node); } +function fallbackWidthForNode(node: WorkflowIrNode): number { + return groupTemplateConfigOf(node) ? FOREACH_GROUP_WIDTH : WF_CARD_WIDTH; +} + +/** + * FNXC:WorkflowContainerEdges 2026-06-26-07:30: + * Container nodes are much wider than step cards, so fallback graph layout must advance by each rendered node width. Fixed index spacing lets optional-group/foreach/loop backgrounds overlap adjacent handles, making correct top-level edges look visually disconnected in the workflow editor. + */ +function fallbackXPositionsForNodes(nodes: readonly WorkflowIrNode[], originX = WF_FALLBACK_GRAPH_X): Map { + const positions = new Map(); + let nextX = originX; + for (const node of nodes) { + positions.set(node.id, nextX); + nextX += fallbackWidthForNode(node) + WF_FALLBACK_NODE_GAP; + } + return positions; +} + /** CSS class for an edge given its condition + rework kind. Rework takes * precedence; failure edges get the distinct failure styling; success and other * conditions get no class (default styling). R2's two-channel rule (label always @@ -341,13 +361,20 @@ export function irToFlow(def: WorkflowDefinition): { const childNodes: FlowNode[] = []; const childEdges: FlowEdge[] = []; - const stepNodes = def.ir.nodes.map((node, index): FlowNode => { - const pos = def.layout?.[node.id]; + const fallbackXById = fallbackXPositionsForNodes(def.ir.nodes); + /* + * FNXC:WorkflowContainerEdges 2026-06-26-07:58: + * Built-in workflow layouts can lag behind newly inserted container nodes. Mixing stale saved positions with fallback-only optional-group/foreach/loop positions reintroduces overlapping handles, so an incomplete top-level layout falls back as one coherent graph instead of preserving disconnected partial coordinates. + */ + const hasCompleteTopLevelLayout = def.ir.nodes.every((node) => Boolean(def.layout?.[node.id])); + const stepNodes = def.ir.nodes.map((node): FlowNode => { + const pos = hasCompleteTopLevelLayout ? def.layout?.[node.id] : undefined; const kind = editorKind(node); const column = isV2(def.ir) ? node.column : undefined; const colIndex = column ? columns.findIndex((c) => c.id === column) : -1; // Default placement seeds the node inside its column band when no persisted // layout exists; otherwise we honor the saved absolute position. + const fallbackX = fallbackXById.get(node.id) ?? WF_FALLBACK_GRAPH_X; const fallbackY = colIndex >= 0 ? bandTop(colIndex) + 70 : 120; const groupCfg = groupTemplateConfigOf(node); @@ -382,7 +409,7 @@ export function irToFlow(def: WorkflowDefinition): { return { id: node.id, type: kind, - position: pos ?? { x: 80 + index * 180, y: fallbackY }, + position: pos ?? { x: fallbackX, y: fallbackY }, data: { kind, ...dataIrKind(node, kind), @@ -400,7 +427,7 @@ export function irToFlow(def: WorkflowDefinition): { return { id: node.id, type: kind, - position: pos ?? { x: 80 + index * 180, y: fallbackY }, + position: pos ?? { x: fallbackX, y: fallbackY }, data: { kind, ...dataIrKind(node, kind), @@ -1159,6 +1186,7 @@ export function insertFragment( const minX = placed.length ? Math.min(...placed.map((p) => p.x)) : 0; const minY = placed.length ? Math.min(...placed.map((p) => p.y)) : 0; + const fallbackXById = fallbackXPositionsForNodes(bodyNodes, position.x); const insertedNodeIds: string[] = []; // Template group children are expanded into parented child flow nodes (the // same way irToFlow does), so an inserted group round-trips its full template @@ -1166,13 +1194,13 @@ export function insertFragment( // otherwise rebuild as an empty template from the absent children). const childNodes: FlowNode[] = []; const childEdges: FlowEdge[] = []; - const newNodes = bodyNodes.map((node, index): FlowNode => { + const newNodes = bodyNodes.map((node): FlowNode => { const id = idMap.get(node.id)!; insertedNodeIds.push(id); const fromLayout = layout?.[node.id]; const pos = fromLayout ? { x: position.x + (fromLayout.x - minX), y: position.y + (fromLayout.y - minY) } - : { x: position.x + index * 180, y: position.y }; + : { x: fallbackXById.get(node.id) ?? position.x, y: position.y }; const groupCfg = groupTemplateConfigOf(node); if (groupCfg) { const template = groupCfg.template;