diff --git a/packages/dashboard/app/components/WorkflowNodeEditor.css b/packages/dashboard/app/components/WorkflowNodeEditor.css index 08612bedac..2fb92fe56c 100644 --- a/packages/dashboard/app/components/WorkflowNodeEditor.css +++ b/packages/dashboard/app/components/WorkflowNodeEditor.css @@ -194,6 +194,12 @@ color: var(--ws-error); } +/* Inspector delete buttons (U3): sit below the field group, sized to the panel. */ +.wf-inspector-delete { + margin-top: var(--space-sm); + justify-content: center; +} + .wf-editor-banner { padding: var(--space-sm) var(--space-md); background: var(--bg-secondary); diff --git a/packages/dashboard/app/components/WorkflowNodeEditor.tsx b/packages/dashboard/app/components/WorkflowNodeEditor.tsx index 2460ce241c..327d036aa3 100644 --- a/packages/dashboard/app/components/WorkflowNodeEditor.tsx +++ b/packages/dashboard/app/components/WorkflowNodeEditor.tsx @@ -53,6 +53,7 @@ import { edgeClassName, edgeConditionEditability, buildConnectionEdge, + cascadeDelete, WF_EDGE_INTERACTION_WIDTH, FOREACH_GROUP_WIDTH, FOREACH_GROUP_HEIGHT, @@ -152,6 +153,9 @@ function InnerEditor({ // built-in pair so the select is never empty; replaced by the live catalog // (built-ins + plugin parsers) once GET /api/step-parsers resolves. const [stepParsers, setStepParsers] = useState([...BUILTIN_STEP_PARSERS]); + // Wrapper around so keyboard deletion can return focus to the + // canvas container (R6) instead of leaving it on a now-removed node. + const canvasRef = useRef(null); const activeWorkflow = useMemo(() => workflows.find((w) => w.id === activeId), [workflows, activeId]); const isBuiltin = !!activeWorkflow && isBuiltinWorkflowId(activeWorkflow.id); @@ -403,6 +407,61 @@ function InnerEditor({ [selectedEdgeId, setEdges], ); + // ── Deletion (U3, R6) ────────────────────────────────────────────────────── + // Apply cascadeDelete to the current graph for the given node/edge ids, + // clearing any selection that pointed at a removed element. Shared by the + // inspector delete buttons and the keyboard-delete path. + const applyDelete = useCallback( + (ids: Iterable) => { + const idSet = new Set(ids); + let next: { nodes: FlowNode[]; edges: FlowEdge[] } | null = null; + setNodes((ns) => { + next = cascadeDelete(ns, edges, idSet); + return next.nodes; + }); + if (next) setEdges((next as { edges: FlowEdge[] }).edges); + if (selectedNodeId !== null && idSet.has(selectedNodeId)) setSelectedNodeId(null); + if (selectedEdgeId !== null && idSet.has(selectedEdgeId)) setSelectedEdgeId(null); + }, + [edges, setNodes, setEdges, selectedNodeId, selectedEdgeId], + ); + + // Keyboard delete (Backspace/Delete) flows through React Flow's onBeforeDelete: + // it hands us the nodes/edges it intends to remove, and we return the + // cascadeDelete-expanded set (foreach children + incident edges, protected + // nodes filtered out) so React Flow deletes exactly the right elements. After + // deletion, focus returns to the canvas container (R6). Built-ins never reach + // here (deleteKeyCode is null and selection is read-only), but the protection + // in cascadeDelete is the backstop. + const onBeforeDelete = useCallback( + async ({ nodes: delNodes, edges: delEdges }: { nodes: FlowNode[]; edges: FlowEdge[] }) => { + if (isBuiltin) return false; + const ids = new Set([...delNodes.map((n) => n.id), ...delEdges.map((e) => e.id)]); + const result = cascadeDelete(nodes, edges, ids); + const removedNodeIds = new Set(nodes.map((n) => n.id)); + for (const n of result.nodes) removedNodeIds.delete(n.id); + const removedEdgeIds = new Set(edges.map((e) => e.id)); + for (const e of result.edges) removedEdgeIds.delete(e.id); + if (removedNodeIds.size === 0 && removedEdgeIds.size === 0) return false; + return { + nodes: nodes.filter((n) => removedNodeIds.has(n.id)), + edges: edges.filter((e) => removedEdgeIds.has(e.id)), + }; + }, + [isBuiltin, nodes, edges], + ); + + // After React Flow removes the elements, drop any dangling selection and move + // focus to the canvas so keyboard nav continues from a live element (R6). + const onNodesDelete = useCallback(() => { + setSelectedNodeId(null); + canvasRef.current?.focus(); + }, []); + const onEdgesDelete = useCallback(() => { + setSelectedEdgeId(null); + canvasRef.current?.focus(); + }, []); + const handleCreateWorkflow = useCallback(async () => { const name = window.prompt("New workflow name"); if (!name?.trim()) return; @@ -766,7 +825,7 @@ function InnerEditor({ )} -
+
{ setSelectedNodeId(node.id); setSelectedEdgeId(null); @@ -1391,6 +1454,19 @@ function InnerEditor({

) : null} + {!isBuiltin && ( + + )} )} @@ -1459,6 +1535,19 @@ function InnerEditor({

)} + {!isBuiltin && ( + + )} )}
diff --git a/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx b/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx index 53f752aea5..b4c587b0d8 100644 --- a/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx +++ b/packages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsx @@ -292,6 +292,42 @@ describe("WorkflowNodeEditor — U10 columns/traits/holds", () => { }); }); +// ── U3: deletion UX (delete buttons + cascade) ────────────────────────────── + +describe("WorkflowNodeEditor — U3 deletion", () => { + beforeEach(() => { + vi.mocked(fetchTraits).mockResolvedValue(TRAIT_CATALOG); + vi.mocked(fetchStepParsers).mockResolvedValue(["step-headings", "json-steps"]); + vi.mocked(fetchModels).mockResolvedValue({ models: [] }); + }); + afterEach(() => cleanup()); + + it("shows a Delete node button when a node is selected and removes the node on click", async () => { + vi.mocked(fetchWorkflows).mockResolvedValue([def()]); + render( {}} addToast={() => {}} />); + const gate = await screen.findByTestId("wf-node-gate"); + fireEvent.click(gate); + const delBtn = await screen.findByTestId("wf-delete-node"); + fireEvent.click(delBtn); + // The gate node is removed from the canvas. + await waitFor(() => expect(screen.queryByTestId("wf-node-gate")).not.toBeInTheDocument()); + // Selecting nothing → the delete button is gone too. + expect(screen.queryByTestId("wf-delete-node")).not.toBeInTheDocument(); + }); + + it("does not render a Delete node button for built-in workflows", async () => { + vi.mocked(fetchWorkflows).mockResolvedValue([ + { ...def(), id: "builtin:coding", name: "Built-in" }, + ]); + render( {}} addToast={() => {}} />); + const gate = await screen.findByTestId("wf-node-gate"); + fireEvent.click(gate); + // Inspector renders (read-only note) but no delete button. + await screen.findByTestId("wf-readonly-banner"); + expect(screen.queryByTestId("wf-delete-node")).not.toBeInTheDocument(); + }); +}); + // ── U8: step-inversion authoring (foreach/step-review/parse-steps/code) ────── /** A custom v2 workflow with a foreach (one step-execute child + a step-review) 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 fba698414a..c70b8f74cd 100644 --- a/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts +++ b/packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts @@ -18,6 +18,7 @@ import { edgeConditionEditability, wouldCreateCycle, buildConnectionEdge, + cascadeDelete, COLUMN_BAND_HEIGHT, WF_CARD_WIDTH, WF_CARD_MAX_WIDTH, @@ -601,3 +602,152 @@ describe("edge-condition authoring (U2)", () => { expect("edge" in res).toBe(true); }); }); + +describe("cascadeDelete (U3, R6)", () => { + // start → a → b → c → end (a/b/c are prompt nodes), so deleting a mid-chain + // node must drop its two incident edges with no bridge created. + const chainDef = (): WorkflowDefinition => + makeDef({ + version: "v1", + name: "chain", + nodes: [ + { id: "start", kind: "start" }, + { id: "a", kind: "prompt", config: { prompt: "a" } }, + { id: "b", kind: "prompt", config: { prompt: "b" } }, + { id: "c", kind: "prompt", config: { prompt: "c" } }, + { id: "end", kind: "end" }, + ], + edges: [ + { from: "start", to: "a", condition: "success" }, + { from: "a", to: "b", condition: "success" }, + { from: "b", to: "c", condition: "success" }, + { from: "c", to: "end", condition: "success" }, + ], + }); + + it("deletes a mid-chain node + both incident edges, with NO bridge edge", () => { + const { nodes, edges } = irToFlow(chainDef()); + const bEdge = edges.find((e) => e.source === "a" && e.target === "b")!; + const result = cascadeDelete(nodes, edges, [/* node */ "b"]); + expect(result.nodes.find((n) => n.id === "b")).toBeUndefined(); + // Both incident edges (a→b and b→c) are gone. + expect(result.edges.find((e) => e.source === "a" && e.target === "b")).toBeUndefined(); + expect(result.edges.find((e) => e.source === "b" && e.target === "c")).toBeUndefined(); + // No auto-bridge a→c. + expect(result.edges.find((e) => e.source === "a" && e.target === "c")).toBeUndefined(); + // Untouched edges survive. + expect(result.edges.find((e) => e.source === "start" && e.target === "a")).toBeTruthy(); + expect(result.edges.find((e) => e.source === "c" && e.target === "end")).toBeTruthy(); + void bEdge; + }); + + // A foreach group with two template children (exec → review, review → exec + // rework), plus top-level edges parse→loop→end. + const foreachDef = (): WorkflowDefinition => + makeDef({ + version: "v1", + name: "loopwf", + nodes: [ + { id: "start", kind: "start" }, + { + id: "loop", + kind: "foreach", + config: { + source: "task-steps", + template: { + nodes: [ + { id: "exec", kind: "prompt", config: { seam: "step-execute", prompt: "do" } }, + { 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" }, + ], + edges: [ + { from: "start", to: "loop", condition: "success" }, + { from: "loop", to: "end", condition: "success" }, + ], + }); + + it("deleting a foreach group removes the group + children + template edges + incident edges", () => { + const { nodes, edges } = irToFlow(foreachDef()); + const execId = foreachChildFlowId("loop", "exec"); + const reviewId = foreachChildFlowId("loop", "review"); + // Sanity: children + their template edges exist before delete. + expect(nodes.find((n) => n.id === execId)).toBeTruthy(); + expect(edges.some((e) => e.source === execId && e.target === reviewId)).toBe(true); + + const result = cascadeDelete(nodes, edges, ["loop"]); + // Group + both children gone. + expect(result.nodes.find((n) => n.id === "loop")).toBeUndefined(); + expect(result.nodes.find((n) => n.id === execId)).toBeUndefined(); + expect(result.nodes.find((n) => n.id === reviewId)).toBeUndefined(); + // Intra-template edges gone. + expect(result.edges.some((e) => e.source === execId || e.target === execId)).toBe(false); + expect(result.edges.some((e) => e.source === reviewId || e.target === reviewId)).toBe(false); + // The group's own incident edges (start→loop, loop→end) gone. + expect(result.edges.some((e) => e.source === "loop" || e.target === "loop")).toBe(false); + // start and end nodes survive. + expect(result.nodes.find((n) => n.id === "start")).toBeTruthy(); + expect(result.nodes.find((n) => n.id === "end")).toBeTruthy(); + }); + + it("deleting only a template child removes the child + its edges, leaving the group", () => { + const { nodes, edges } = irToFlow(foreachDef()); + const execId = foreachChildFlowId("loop", "exec"); + const reviewId = foreachChildFlowId("loop", "review"); + + const result = cascadeDelete(nodes, edges, [execId]); + // The exec child is gone; the group + sibling remain. + expect(result.nodes.find((n) => n.id === execId)).toBeUndefined(); + expect(result.nodes.find((n) => n.id === "loop")).toBeTruthy(); + expect(result.nodes.find((n) => n.id === reviewId)).toBeTruthy(); + // Edges touching exec (both directions) are gone. + expect(result.edges.some((e) => e.source === execId || e.target === execId)).toBe(false); + }); + + it("never deletes start/end nodes (and preserves their edges)", () => { + const { nodes, edges } = irToFlow(chainDef()); + const result = cascadeDelete(nodes, edges, ["start", "end"]); + expect(result.nodes.find((n) => n.id === "start")).toBeTruthy(); + expect(result.nodes.find((n) => n.id === "end")).toBeTruthy(); + // Their incident edges survive too (start→a, c→end). + expect(result.edges.some((e) => e.source === "start")).toBe(true); + expect(result.edges.some((e) => e.target === "end")).toBe(true); + // Nothing was removed at all. + expect(result.nodes).toHaveLength(nodes.length); + expect(result.edges).toHaveLength(edges.length); + }); + + it("never deletes column band nodes", () => { + const v2: WorkflowDefinition["ir"] = { + version: "v2", + name: "wf", + columns: [{ id: "col1", name: "Col 1", traits: [] }], + nodes: [ + { id: "start", kind: "start", column: "col1" }, + { id: "end", kind: "end", column: "col1" }, + ], + edges: [{ from: "start", to: "end", condition: "success" }], + }; + const { nodes, edges } = irToFlow(makeDef(v2)); + const bandId = nodes.find((n) => isColumnBandNode(n.id))!.id; + const result = cascadeDelete(nodes, edges, [bandId]); + expect(result.nodes.find((n) => n.id === bandId)).toBeTruthy(); + }); + + it("deletes an edge id directly, removing just that edge", () => { + const { nodes, edges } = irToFlow(chainDef()); + const target = edges.find((e) => e.source === "b" && e.target === "c")!; + const result = cascadeDelete(nodes, edges, [target.id]); + expect(result.edges.find((e) => e.id === target.id)).toBeUndefined(); + // No nodes removed, all other edges intact. + expect(result.nodes).toHaveLength(nodes.length); + expect(result.edges).toHaveLength(edges.length - 1); + }); +}); diff --git a/packages/dashboard/app/components/workflow-flow-mapping.ts b/packages/dashboard/app/components/workflow-flow-mapping.ts index 84c15b0795..ec20ac9aa9 100644 --- a/packages/dashboard/app/components/workflow-flow-mapping.ts +++ b/packages/dashboard/app/components/workflow-flow-mapping.ts @@ -415,6 +415,75 @@ function flowEdgeToIr(edge: FlowEdge, groupId?: string): WorkflowIrEdge { return { from, to, condition, ...(isRework ? { kind: "rework" as const } : {}) }; } +// ── Deletion with cascade semantics (U3, R6) ───────────────────────────────── +// +// Pure node/edge transformation for deleting nodes and/or edges. React Flow's +// built-in deletion removes incident edges but does NOT cascade group children +// (deleting a foreach group leaves its `parentId` children orphaned), so the +// editor routes all deletions through this helper for explicit, testable +// behavior. + +/** Node kinds that may never be deleted (start/end are structural). */ +const PROTECTED_NODE_KINDS = new Set(["start", "end"]); + +/** True when a flow node is protected from deletion: start/end kinds and column + * band group nodes are never removable, regardless of the requested ids. */ +function isProtectedFromDelete(node: FlowNode): boolean { + return isColumnBandNode(node.id) || PROTECTED_NODE_KINDS.has(node.data.kind); +} + +/** + * Delete the requested node and/or edge ids from the flow graph, applying R6's + * cascade rules: + * - Deleting a node removes ALL edges incident to it (no auto-bridging). + * - Deleting a `foreach` group node also deletes its template children + * (nodes with `parentId === groupId`) and every edge incident to those + * children (React Flow does not cascade parents — handled explicitly). + * - `start`/`end` nodes and column band nodes are never deleted: they are + * filtered out of the requested ids up front (and their incident edges are + * therefore preserved). + * - Edge ids in `ids` are removed directly. + * + * Pure and order-independent: the same `ids` set always yields the same result. + */ +export function cascadeDelete( + nodes: FlowNode[], + edges: FlowEdge[], + ids: Iterable, +): { nodes: FlowNode[]; edges: FlowEdge[] } { + const requested = new Set(ids); + const nodeById = new Map(nodes.map((n) => [n.id, n])); + + // Resolve which node ids are actually deletable, expanding foreach groups to + // their template children. Protected nodes are dropped from the request. + const deleteNodeIds = new Set(); + for (const id of requested) { + const node = nodeById.get(id); + if (!node || isProtectedFromDelete(node)) continue; + deleteNodeIds.add(id); + if (node.data.kind === "foreach") { + for (const child of nodes) { + if (child.parentId === id) deleteNodeIds.add(child.id); + } + } + } + + // Edge ids requested directly (only ones that exist as edges). + const deleteEdgeIds = new Set(); + for (const e of edges) { + if (requested.has(e.id)) deleteEdgeIds.add(e.id); + } + + const nextNodes = nodes.filter((n) => !deleteNodeIds.has(n.id)); + const nextEdges = edges.filter( + (e) => + !deleteEdgeIds.has(e.id) && + !deleteNodeIds.has(e.source) && + !deleteNodeIds.has(e.target), + ); + return { nodes: nextNodes, edges: nextEdges }; +} + // ── Edge-condition authoring (U2) ──────────────────────────────────────────── /** Editor node kinds whose edges expose a success/failure condition select diff --git a/packages/i18n/locales/en/app.json b/packages/i18n/locales/en/app.json index b3abf434ca..a9dbb929e6 100644 --- a/packages/i18n/locales/en/app.json +++ b/packages/i18n/locales/en/app.json @@ -6761,6 +6761,8 @@ "codeSource": "Source (TypeScript)", "codeTimeout": "Timeout (ms)", "cycleBlocked": "That connection would create a cycle — only rework edges inside a for-each template may loop back", + "deleteEdge": "Delete edge", + "deleteNode": "Delete node", "edgeCondition": "Condition", "edgeConditionLabel": "Condition: {{condition}}", "edgeInspector": "Edge", diff --git a/packages/i18n/locales/es/app.json b/packages/i18n/locales/es/app.json index abeb62cc89..5be4d4ee30 100644 --- a/packages/i18n/locales/es/app.json +++ b/packages/i18n/locales/es/app.json @@ -6761,6 +6761,8 @@ "codeSource": "Origen (TypeScript)", "codeTimeout": "Tiempo de espera (ms)", "cycleBlocked": "", + "deleteEdge": "", + "deleteNode": "", "edgeCondition": "", "edgeConditionLabel": "Condición: {{condition}}", "edgeInspector": "Conexión", diff --git a/packages/i18n/locales/fr/app.json b/packages/i18n/locales/fr/app.json index 562e0af93f..cdcacf9465 100644 --- a/packages/i18n/locales/fr/app.json +++ b/packages/i18n/locales/fr/app.json @@ -6761,6 +6761,8 @@ "codeSource": "Source (TypeScript)", "codeTimeout": "Délai d’expiration (ms)", "cycleBlocked": "", + "deleteEdge": "", + "deleteNode": "", "edgeCondition": "", "edgeConditionLabel": "Condition : {{condition}}", "edgeInspector": "Lien", diff --git a/packages/i18n/locales/ko/app.json b/packages/i18n/locales/ko/app.json index 7e45c6f55a..2e2ee82d14 100644 --- a/packages/i18n/locales/ko/app.json +++ b/packages/i18n/locales/ko/app.json @@ -6761,6 +6761,8 @@ "codeSource": "소스(TypeScript)", "codeTimeout": "제한 시간(ms)", "cycleBlocked": "", + "deleteEdge": "", + "deleteNode": "", "edgeCondition": "", "edgeConditionLabel": "조건: {{condition}}", "edgeInspector": "에지", diff --git a/packages/i18n/locales/zh-CN/app.json b/packages/i18n/locales/zh-CN/app.json index 8b6d98c993..0c66e13717 100644 --- a/packages/i18n/locales/zh-CN/app.json +++ b/packages/i18n/locales/zh-CN/app.json @@ -6761,6 +6761,8 @@ "codeSource": "源代码（TypeScript）", "codeTimeout": "超时（毫秒）", "cycleBlocked": "", + "deleteEdge": "", + "deleteNode": "", "edgeCondition": "", "edgeConditionLabel": "条件：{{condition}}", "edgeInspector": "连线", diff --git a/packages/i18n/locales/zh-TW/app.json b/packages/i18n/locales/zh-TW/app.json index ffb98650ec..8af8ded56b 100644 --- a/packages/i18n/locales/zh-TW/app.json +++ b/packages/i18n/locales/zh-TW/app.json @@ -6761,6 +6761,8 @@ "codeSource": "原始碼（TypeScript）", "codeTimeout": "逾時（毫秒）", "cycleBlocked": "", + "deleteEdge": "", + "deleteNode": "", "edgeCondition": "", "edgeConditionLabel": "條件：{{condition}}", "edgeInspector": "連線", diff --git a/packages/i18n/src/resources.d.ts b/packages/i18n/src/resources.d.ts index 651ef0dbb2..aeb63bb853 100644 --- a/packages/i18n/src/resources.d.ts +++ b/packages/i18n/src/resources.d.ts @@ -6763,6 +6763,8 @@ export default interface Resources { "codeSource": "Source (TypeScript)", "codeTimeout": "Timeout (ms)", "cycleBlocked": "That connection would create a cycle — only rework edges inside a for-each template may loop back", + "deleteEdge": "Delete edge", + "deleteNode": "Delete node", "edgeCondition": "Condition", "edgeConditionLabel": "Condition: {{condition}}", "edgeInspector": "Edge",