diff --git a/packages/dashboard/app/components/WorkflowNodeEditor.css b/packages/dashboard/app/components/WorkflowNodeEditor.css index 2d7db49766..61999843e8 100644 --- a/packages/dashboard/app/components/WorkflowNodeEditor.css +++ b/packages/dashboard/app/components/WorkflowNodeEditor.css @@ -1642,6 +1642,12 @@ Workflow authors need store-produced lifecycle warnings visible in the editor be color: var(--text); } +/* +FNXC:WorkflowEditor 2026-07-12-10:30: +The banner is a
disclosure: collapsed it reads as one quiet count +line ("2 lifecycle warnings"); expanding reveals the full list. Hide the +native marker and rotate the chevron on open. +*/ .wf-lifecycle-warnings-title { display: inline-flex; align-items: center; @@ -1649,6 +1655,32 @@ Workflow authors need store-produced lifecycle warnings visible in the editor be font-size: 0.78rem; font-weight: 650; color: var(--ws-warning); + cursor: pointer; + list-style: none; + user-select: none; +} + +.wf-lifecycle-warnings-title::-webkit-details-marker { + display: none; +} + +.wf-lifecycle-warnings-title:focus-visible { + outline: 2px solid var(--accent); + outline-offset: 2px; + border-radius: var(--radius-sm); +} + +.wf-lifecycle-warnings-chevron { + transition: transform var(--transition-fast); + opacity: 0.75; +} + +.wf-lifecycle-warnings[open] .wf-lifecycle-warnings-chevron { + transform: rotate(180deg); +} + +.wf-lifecycle-warnings[open] ul { + margin-top: var(--space-xs); } .wf-lifecycle-warnings ul { diff --git a/packages/dashboard/app/components/WorkflowNodeEditor.tsx b/packages/dashboard/app/components/WorkflowNodeEditor.tsx index b63894f5ea..208c269648 100644 --- a/packages/dashboard/app/components/WorkflowNodeEditor.tsx +++ b/packages/dashboard/app/components/WorkflowNodeEditor.tsx @@ -91,7 +91,7 @@ import { FOREACH_CHILD_Y, } from "./workflow-flow-mapping"; import { autoLayout, applyAutoLayout } from "./workflow-auto-layout"; -import { insertNodeOnEdge, findAppendEdgeId } from "./workflow-simple-layout"; +import { insertNodeOnEdge, findAppendEdgeId, spliceInsertedSubgraphOnEdge } from "./workflow-simple-layout"; import { WorkflowSimpleCanvas } from "./WorkflowSimpleCanvas"; import { WorkflowAddStepModal, type AddStepPaletteEntry } from "./WorkflowAddStepModal"; import { fetchTraits, fetchStepParsers, type TraitCatalogEntry } from "../api"; @@ -1620,7 +1620,10 @@ function InnerEditor({ surfaces label it. */ const handleInsertStepTemplateAsOptionalGroup = useCallback( - (tpl: WorkflowStepTemplate) => { + // FNXC:WorkflowSimpleView 2026-07-12-10:30: PR #2006 review — when the + // add-step dialog targeted an edge "+", this path must splice the new + // optional-group into that edge instead of dropping it free-floating. + (tpl: WorkflowStepTemplate, targetEdgeId?: string | null) => { if (isBuiltin) return; const { kind, config } = stepTemplateToNode(tpl); const fragmentIr = optionalGroupFragmentIr( @@ -1631,8 +1634,11 @@ function InnerEditor({ x: 240, y: 200 + (nodes.length % 4) * 40, }); - setNodes(result.nodes); - setEdges(result.edges); + const spliced = targetEdgeId + ? spliceInsertedSubgraphOnEdge(result.nodes, result.edges, targetEdgeId, result.insertedNodeIds) + : null; + setNodes(spliced?.nodes ?? result.nodes); + setEdges(spliced?.edges ?? result.edges); setSelectedNodeId(result.insertedNodeIds[0] ?? null); }, [isBuiltin, nodes, edges, setNodes, setEdges], @@ -1644,7 +1650,11 @@ function InnerEditor({ // insertFragment remaps ids + rewires internal edges, landing nodes at a fixed // offset from the canvas origin. const handleInsertFragment = useCallback( - (fragment: WorkflowDefinition) => { + // FNXC:WorkflowSimpleView 2026-07-12-10:30: PR #2006 review — an + // edge-targeted pick ("+" on an edge) splices the fragment's entry/exit + // boundary into that edge; the toolbar/free path keeps the classic + // fixed-position landing. + (fragment: WorkflowDefinition, targetEdgeId?: string | null) => { if (isBuiltin) return false; const loadedIrFallback = canvasNodesMaterializedRef.current ? undefined : activeWorkflow?.ir; const conflicts = fragmentSeamConflicts(fragment.ir, nodes, loadedIrFallback); @@ -1660,8 +1670,11 @@ function InnerEditor({ { x: 240, y: 200 + (nodes.length % 4) * 40 }, fragment.layout, ); - setNodes(result.nodes); - setEdges(result.edges); + const spliced = targetEdgeId + ? spliceInsertedSubgraphOnEdge(result.nodes, result.edges, targetEdgeId, result.insertedNodeIds) + : null; + setNodes(spliced?.nodes ?? result.nodes); + setEdges(spliced?.edges ?? result.edges); setSelectedNodeId(result.insertedNodeIds[0] ?? null); return true; }, @@ -3077,11 +3090,21 @@ function InnerEditor({ )} {lifecycleWarnings.length > 0 && ( -
-
+ /* FNXC:WorkflowEditor 2026-07-12-10:30: the lifecycle + warnings banner previously rendered fully expanded and + dominated the editor header (every fresh workflow starts + with two warnings). It is now a one-line disclosure — + count summary, details on demand — in every view mode. */ +
+ - {t("workflows.lifecycleWarningsTitle", "Lifecycle warnings")} -
+ + {t("workflows.lifecycleWarningsCount", "{{count}} lifecycle warnings", { + count: lifecycleWarnings.length, + })} + + +
    {lifecycleWarnings.map((warning, index) => (
  • @@ -3091,7 +3114,7 @@ function InnerEditor({
  • ))}
-
+
)} {simpleLayoutEnabled && (
@@ -5479,11 +5502,11 @@ function InnerEditor({ templateConflict={templateConflict} onPickPalette={handleAddStepPalettePick} onPickFragment={(fragment) => { - if (handleInsertFragment(fragment)) setAddStepTarget(null); + if (handleInsertFragment(fragment, addStepTarget?.edgeId)) setAddStepTarget(null); }} onPickStepTemplate={handleAddStepTemplatePick} onPickStepTemplateAsOptionalGroup={(tpl) => { - handleInsertStepTemplateAsOptionalGroup(tpl); + handleInsertStepTemplateAsOptionalGroup(tpl, addStepTarget?.edgeId); setAddStepTarget(null); }} /> diff --git a/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx b/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx index c9f9a35153..f3df7675e6 100644 --- a/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx +++ b/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx @@ -570,8 +570,13 @@ describe("WorkflowNodeEditor", () => { render( {}} addToast={() => {}} />); + // FNXC:WorkflowEditor 2026-07-12-10:30: banner is a collapsed-by-default + // disclosure: a count summary line, with details revealed on expand. const banner = await screen.findByTestId("wf-lifecycle-warnings"); - expect(banner).toHaveTextContent("Lifecycle warnings"); + expect(banner).toHaveTextContent("2 lifecycle warnings"); + expect(banner).not.toHaveAttribute("open"); + fireEvent.click(screen.getByTestId("wf-lifecycle-warnings-toggle")); + expect(banner).toHaveAttribute("open"); expect(banner).toHaveTextContent("missing-merge-region"); expect(banner).toHaveTextContent("optional-group-after-execution"); expect(banner).toHaveTextContent("plan-review"); @@ -4160,6 +4165,34 @@ describe("WorkflowNodeEditor simplified view modes", () => { expect(document.querySelector('[data-testid^="wf-simple-insert-"]')).toBeNull(); }); + it("splices an edge-targeted fragment pick into the targeted edge", async () => { + // FNXC:WorkflowSimpleView 2026-07-12-10:30: PR #2006 review — fragment + // picks from an edge-targeted add-step dialog must rewire + // source→fragment→target instead of dropping a disconnected subgraph. + vi.mocked(fetchWorkflows).mockResolvedValue([def(), fragmentDef()]); + vi.mocked(updateWorkflow).mockImplementation(async (_id, updates) => ({ ...def(), ...(updates as object) })); + render( {}} addToast={() => {}} />); + await screen.findByTestId("wf-simple-canvas"); + + // def() has a single edge into end, so the toolbar add targets that edge. + fireEvent.click(screen.getByTestId("wf-simple-toolbar-add-step")); + const dialog = await screen.findByTestId("wf-add-step-modal"); + fireEvent.click(within(dialog).getByTestId("wf-add-step-fragment-WF-FRAG")); + await waitFor(() => expect(screen.queryByTestId("wf-add-step-modal")).not.toBeInTheDocument()); + + // Save and inspect the serialized IR: merge no longer feeds end directly; + // the fragment's lint gate sits between them. + fireEvent.click(screen.getByText("Save").closest("button")!); + await waitFor(() => expect(updateWorkflow).toHaveBeenCalledTimes(1)); + const [, updates] = vi.mocked(updateWorkflow).mock.calls[0]; + const ir = (updates as { ir: { nodes: Array<{ id: string; kind: string }>; edges: Array<{ from: string; to: string; condition?: string }> } }).ir; + const insertedGate = ir.nodes.find((n) => n.kind === "gate" && n.id !== "lint"); + expect(insertedGate).toBeDefined(); + expect(ir.edges.some((e) => e.from === "merge" && e.to === "end")).toBe(false); + expect(ir.edges.some((e) => e.from === "merge" && e.to === insertedGate!.id)).toBe(true); + expect(ir.edges.some((e) => e.from === insertedGate!.id && e.to === "end")).toBe(true); + }); + it("defaults the mobile graph tab to the simplified canvas with a list fallback", async () => { mockWorkflowEditorViewport("mobile"); render( {}} addToast={() => {}} />); diff --git a/packages/dashboard/app/components/__tests__/workflow-simple-layout.test.ts b/packages/dashboard/app/components/__tests__/workflow-simple-layout.test.ts index ddb349a758..6a67af72b8 100644 --- a/packages/dashboard/app/components/__tests__/workflow-simple-layout.test.ts +++ b/packages/dashboard/app/components/__tests__/workflow-simple-layout.test.ts @@ -6,6 +6,7 @@ import { edgeSupportsSimpleInsert, insertNodeOnEdge, findAppendEdgeId, + spliceInsertedSubgraphOnEdge, SIMPLE_NODE_WIDTH, } from "../workflow-simple-layout"; @@ -203,3 +204,67 @@ describe("findAppendEdgeId", () => { expect(findAppendEdgeId(nodes, [edge("e1", "start", "a")])).toBeNull(); }); }); + + +/* +FNXC:WorkflowSimpleView 2026-07-12-10:30: +PR #2006 review coverage: edge-targeted fragment / optional-group inserts must +splice the already-inserted subgraph into the targeted edge (entries inherit +the original condition, exits feed the old target, the original edge is +removed, the subgraph moves into the source's y band). +*/ +describe("spliceInsertedSubgraphOnEdge", () => { + const baseNodes = (): N[] => [ + node("start", "start", 0, 120), + node("a", "prompt", 300, 120), + node("end", "end", 900, 120), + ]; + const baseEdges = (): FlowEdge[] => [edge("e1", "start", "a"), edge("e2", "a", "end", "failure")]; + + it("wires source→entry and exit→target, removes the edge, preserves condition", () => { + const nodes = [ + ...baseNodes(), + node("f1", "gate", 240, 700), + node("f2", "script", 520, 700), + ]; + const edges = [...baseEdges(), edge("f-e", "f1", "f2")]; + const result = spliceInsertedSubgraphOnEdge(nodes, edges, "e2", ["f1", "f2"]); + expect(result).not.toBeNull(); + const { nodes: nextNodes, edges: nextEdges } = result!; + expect(nextEdges.find((e) => e.id === "e2")).toBeUndefined(); + const inbound = nextEdges.find((e) => e.source === "a" && e.target === "f1"); + const outbound = nextEdges.find((e) => e.source === "f2" && e.target === "end"); + expect(inbound?.data?.condition).toBe("failure"); + expect(outbound?.data?.condition).toBe("success"); + // Subgraph translated into the source's y band, preserving relative layout. + const f1 = nextNodes.find((n) => n.id === "f1")!; + const f2 = nextNodes.find((n) => n.id === "f2")!; + expect(f1.position.y).toBe(120 + 8); + expect(f2.position.x - f1.position.x).toBe(280); + }); + + it("wires an optional-group container while leaving its template children alone", () => { + const nodes = [ + ...baseNodes(), + node("grp", "optional-group", 240, 700, { style: { width: 560, height: 220 } }), + node("grp-child", "prompt", 30, 56, { parentId: "grp" }), + ]; + const result = spliceInsertedSubgraphOnEdge(nodes, baseEdges(), "e2", ["grp", "grp-child"]); + expect(result).not.toBeNull(); + const inbound = result!.edges.find((e) => e.source === "a" && e.target === "grp"); + const outbound = result!.edges.find((e) => e.source === "grp" && e.target === "end"); + expect(inbound).toBeDefined(); + expect(outbound).toBeDefined(); + // Child keeps its parent-relative position. + const child = result!.nodes.find((n) => n.id === "grp-child")!; + expect(child.position).toEqual({ x: 30, y: 56 }); + }); + + it("returns null when the target edge is gone or ineligible", () => { + const nodes = [...baseNodes(), node("f1", "gate", 240, 700)]; + expect(spliceInsertedSubgraphOnEdge(nodes, baseEdges(), "missing", ["f1"])).toBeNull(); + const rework = [edge("r1", "a", "start", "failure", "rework")]; + expect(spliceInsertedSubgraphOnEdge(nodes, rework, "r1", ["f1"])).toBeNull(); + expect(spliceInsertedSubgraphOnEdge(baseNodes(), baseEdges(), "e2", ["not-present"])).toBeNull(); + }); +}); diff --git a/packages/dashboard/app/components/workflow-simple-layout.ts b/packages/dashboard/app/components/workflow-simple-layout.ts index dc6009d44f..72c234759d 100644 --- a/packages/dashboard/app/components/workflow-simple-layout.ts +++ b/packages/dashboard/app/components/workflow-simple-layout.ts @@ -269,6 +269,101 @@ export function insertNodeOnEdge( return { nodes: refreshed.nodes, edges: refreshed.edges, newNodeId: id }; } +/* +FNXC:WorkflowSimpleView 2026-07-12-10:30: +PR #2006 review: fragment and "insert as optional group" picks from an +edge-targeted add-step dialog previously fell through to the advanced +fixed-position insertFragment path, leaving the "+" edge untouched and the +inserted subgraph disconnected. This splice helper wires an ALREADY-inserted +subgraph into the targeted edge: source→(subgraph entries) preserving the +original routing condition, (subgraph exits)→target as success, original edge +removed, and the subgraph translated next to the source node's y so v2 +column banding stays valid for simple-view authors who cannot drag. +*/ + +/** + * Wire an inserted subgraph (from insertFragment) into an existing edge. + * Entries/exits derive from in/out degree over the subgraph's internal + * non-rework edges (mirroring template boundary semantics). Returns null when + * the edge is gone/ineligible or the subgraph has no top-level nodes — the + * caller keeps the plain fixed-position insert in that case. + */ +export function spliceInsertedSubgraphOnEdge( + nodes: LayoutNode[], + edges: FlowEdge[], + edgeId: string, + insertedNodeIds: readonly string[], +): { nodes: LayoutNode[]; edges: FlowEdge[] } | null { + const edge = edges.find((e) => e.id === edgeId); + if (!edge || !edgeSupportsSimpleInsert(edge)) return null; + const source = nodes.find((n) => n.id === edge.source); + const target = nodes.find((n) => n.id === edge.target); + if (!source || !target) return null; + + const insertedSet = new Set(insertedNodeIds); + const insertedTop = nodes.filter( + (n) => insertedSet.has(n.id) && !n.parentId && !isColumnBandNode(n.id), + ); + if (insertedTop.length === 0) return null; + const topIds = new Set(insertedTop.map((n) => n.id)); + + const indegree = new Map(); + const outdegree = new Map(); + for (const id of topIds) { + indegree.set(id, 0); + outdegree.set(id, 0); + } + for (const e of edges) { + if (isVisualOnlyWorkflowEdge(e)) continue; + if ((e.data?.kind as string | undefined) === "rework") continue; + if (!topIds.has(e.source) || !topIds.has(e.target)) continue; + outdegree.set(e.source, (outdegree.get(e.source) ?? 0) + 1); + indegree.set(e.target, (indegree.get(e.target) ?? 0) + 1); + } + const entries = insertedTop.filter((n) => (indegree.get(n.id) ?? 0) === 0); + const exits = insertedTop.filter((n) => (outdegree.get(n.id) ?? 0) === 0); + const entryNodes = entries.length > 0 ? entries : insertedTop; + const exitNodes = exits.length > 0 ? exits : insertedTop; + + // Translate the subgraph beside the wiring point: same y band as the source + // (column validity), x around the source/target midpoint. Relative layout + // inside the subgraph is preserved. + const minX = Math.min(...insertedTop.map((n) => n.position.x)); + const minY = Math.min(...insertedTop.map((n) => n.position.y)); + const deltaX = (source.position.x + target.position.x) / 2 + 24 - minX; + const deltaY = source.position.y + 8 - minY; + const nextNodes = nodes.map((n) => + topIds.has(n.id) + ? { ...n, position: { x: n.position.x + deltaX, y: n.position.y + deltaY } } + : n, + ); + + const inboundCondition = (edge.data?.condition as string | undefined) ?? "success"; + const newEdges: FlowEdge[] = [ + ...entryNodes.map((entry) => ({ + id: `e-${newNodeId()}`, + source: edge.source, + target: entry.id, + label: shortConditionLabel(inboundCondition), + data: { condition: inboundCondition, kind: undefined }, + className: edgeClassName(inboundCondition, false), + interactionWidth: WF_EDGE_INTERACTION_WIDTH, + })), + ...exitNodes.map((exit) => ({ + id: `e-${newNodeId()}`, + source: exit.id, + target: edge.target, + label: shortConditionLabel("success"), + data: { condition: "success", kind: undefined }, + className: edgeClassName("success", false), + interactionWidth: WF_EDGE_INTERACTION_WIDTH, + })), + ]; + + const nextEdges = [...edges.filter((e) => e.id !== edgeId), ...newEdges]; + return refreshTemplateContainerVisualBoundaries(nextNodes, nextEdges); +} + /** * The simple view's "+ Add step" (no specific edge): insert before the `end` * node when a single wiring point is unambiguous, otherwise signal the caller diff --git a/packages/i18n/locales/en/app.json b/packages/i18n/locales/en/app.json index 3484efa5e8..8728b524f9 100644 --- a/packages/i18n/locales/en/app.json +++ b/packages/i18n/locales/en/app.json @@ -8806,6 +8806,8 @@ "importInvalidJson": "That file isn't valid JSON.", "importStripped": "Auto-approval flags were removed from imported nodes", "importTooltip": "Import a workflow from a JSON file", + "lifecycleWarningsCount_one": "{{count}} lifecycle warning", + "lifecycleWarningsCount_other": "{{count}} lifecycle warnings", "loadFailed": "Failed to load workflows", "loading": "Loading…", "migrationNotice": "Your legacy workflow steps were converted — find them as templates in the palette and as the \"Migrated steps\" workflow.", diff --git a/packages/i18n/locales/es/app.json b/packages/i18n/locales/es/app.json index edc88b80c7..3226b43905 100644 --- a/packages/i18n/locales/es/app.json +++ b/packages/i18n/locales/es/app.json @@ -8795,6 +8795,8 @@ "importInvalidJson": "Ese archivo no es JSON válido.", "importStripped": "Se eliminaron los indicadores de aprobación automática de los nodos importados", "importTooltip": "Importar un flujo de trabajo desde un archivo JSON", + "lifecycleWarningsCount_one": "", + "lifecycleWarningsCount_other": "", "loadFailed": "", "loading": "", "migrationNotice": "Los pasos del flujo de trabajo heredado fueron convertidos — encuéntralos como plantillas en la paleta y como el flujo de trabajo «Pasos migrados».", diff --git a/packages/i18n/locales/fr/app.json b/packages/i18n/locales/fr/app.json index 44be17b52b..262fb6e1f0 100644 --- a/packages/i18n/locales/fr/app.json +++ b/packages/i18n/locales/fr/app.json @@ -8796,6 +8796,8 @@ "importInvalidJson": "Ce fichier n'est pas un JSON valide.", "importStripped": "Les indicateurs d'approbation automatique ont été supprimés des nœuds importés", "importTooltip": "Importer un workflow depuis un fichier JSON", + "lifecycleWarningsCount_one": "", + "lifecycleWarningsCount_other": "", "loadFailed": "", "loading": "", "migrationNotice": "Vos anciennes étapes de workflow ont été converties — retrouvez-les sous forme de modèles dans la palette et sous le workflow « Étapes migrées ».", diff --git a/packages/i18n/locales/ko/app.json b/packages/i18n/locales/ko/app.json index d593dcec7b..4d94cf400b 100644 --- a/packages/i18n/locales/ko/app.json +++ b/packages/i18n/locales/ko/app.json @@ -8795,6 +8795,8 @@ "importInvalidJson": "이 파일은 유효한 JSON이 아닙니다.", "importStripped": "가져온 노드에서 자동 승인 플래그가 제거되었습니다", "importTooltip": "JSON 파일에서 워크플로 가져오기", + "lifecycleWarningsCount_one": "", + "lifecycleWarningsCount_other": "", "loadFailed": "", "loading": "", "migrationNotice": "이전 워크플로 단계가 변환되었습니다 — 팔레트의 템플릿과 \"마이그레이션된 단계\" 워크플로에서 찾을 수 있습니다.", diff --git a/packages/i18n/locales/zh-CN/app.json b/packages/i18n/locales/zh-CN/app.json index 08b7ec7e96..675d4bda76 100644 --- a/packages/i18n/locales/zh-CN/app.json +++ b/packages/i18n/locales/zh-CN/app.json @@ -8795,6 +8795,8 @@ "importInvalidJson": "该文件不是有效的 JSON。", "importStripped": "已从导入的节点中移除自动审批标志", "importTooltip": "从 JSON 文件导入工作流", + "lifecycleWarningsCount_one": "", + "lifecycleWarningsCount_other": "", "loadFailed": "", "loading": "", "migrationNotice": "您的旧版工作流步骤已转换——在面板模板和“已迁移步骤”工作流中查找。", diff --git a/packages/i18n/locales/zh-TW/app.json b/packages/i18n/locales/zh-TW/app.json index 689aff0484..53229b3011 100644 --- a/packages/i18n/locales/zh-TW/app.json +++ b/packages/i18n/locales/zh-TW/app.json @@ -8795,6 +8795,8 @@ "importInvalidJson": "該檔案不是有效的 JSON。", "importStripped": "已從匯入的節點移除自動核准旗標", "importTooltip": "從 JSON 檔案匯入工作流程", + "lifecycleWarningsCount_one": "", + "lifecycleWarningsCount_other": "", "loadFailed": "", "loading": "", "migrationNotice": "你的舊版工作流程步驟已轉換 — 可在選盤中以範本形式找到它們,以及名為「已遷移步驟」的工作流程。",