feat(dashboard): node/edge deletion with safe cascade semantics

This commit is contained in:
gsxdsm
2026-06-04 19:45:44 -07:00
parent ffc4d8a6ed
commit f67a2542b9
12 changed files with 365 additions and 1 deletions

View File

@@ -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);

View File

@@ -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<string[]>([...BUILTIN_STEP_PARSERS]);
// Wrapper around <ReactFlow> so keyboard deletion can return focus to the
// canvas container (R6) instead of leaving it on a now-removed node.
const canvasRef = useRef<HTMLDivElement>(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<string>) => {
const idSet = new Set(ids);
let next: { nodes: FlowNode<WorkflowFlowNodeData>[]; 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<WorkflowFlowNodeData>[]; edges: FlowEdge[] }) => {
if (isBuiltin) return false;
const ids = new Set<string>([...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({
</div>
)}
<div className="wf-editor-canvas">
<div className="wf-editor-canvas" ref={canvasRef} tabIndex={-1}>
<WorkflowEditorCatalogContext.Provider value={catalogs}>
<ReactFlow
nodes={nodesForRender}
@@ -776,6 +835,10 @@ function InnerEditor({
onEdgesChange={onEdgesChange}
onConnect={onConnect}
onNodeDragStop={onNodeDragStop}
deleteKeyCode={isBuiltin ? null : ["Backspace", "Delete"]}
onBeforeDelete={onBeforeDelete}
onNodesDelete={onNodesDelete}
onEdgesDelete={onEdgesDelete}
onNodeClick={(_, node) => {
setSelectedNodeId(node.id);
setSelectedEdgeId(null);
@@ -1391,6 +1454,19 @@ function InnerEditor({
</p>
) : null}
</fieldset>
{!isBuiltin && (
<button
type="button"
className="wf-editor-delete wf-inspector-delete"
data-testid="wf-delete-node"
onClick={() => {
applyDelete([selectedNode.id]);
setSelectedNodeId(null);
}}
>
<Trash2 size={13} /> {t("workflowNodes.deleteNode", "Delete node")}
</button>
)}
</aside>
)}
@@ -1459,6 +1535,19 @@ function InnerEditor({
</p>
)}
</fieldset>
{!isBuiltin && (
<button
type="button"
className="wf-editor-delete wf-inspector-delete"
data-testid="wf-delete-edge"
onClick={() => {
applyDelete([selectedEdge.id]);
setSelectedEdgeId(null);
}}
>
<Trash2 size={13} /> {t("workflowNodes.deleteEdge", "Delete edge")}
</button>
)}
</aside>
)}
</div>

View File

@@ -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(<WorkflowNodeEditor isOpen onClose={() => {}} 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(<WorkflowNodeEditor isOpen onClose={() => {}} 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)

View File

@@ -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);
});
});

View File

@@ -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<string>(["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<WorkflowFlowNodeData>): 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<WorkflowFlowNodeData>[],
edges: FlowEdge[],
ids: Iterable<string>,
): { nodes: FlowNode<WorkflowFlowNodeData>[]; 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<string>();
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<string>();
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

View File

@@ -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",

View File

@@ -6761,6 +6761,8 @@
"codeSource": "Origen (TypeScript)",
"codeTimeout": "Tiempo de espera (ms)",
"cycleBlocked": "",
"deleteEdge": "",
"deleteNode": "",
"edgeCondition": "",
"edgeConditionLabel": "Condición: {{condition}}",
"edgeInspector": "Conexión",

View File

@@ -6761,6 +6761,8 @@
"codeSource": "Source (TypeScript)",
"codeTimeout": "Délai d’expiration (ms)",
"cycleBlocked": "",
"deleteEdge": "",
"deleteNode": "",
"edgeCondition": "",
"edgeConditionLabel": "Condition : {{condition}}",
"edgeInspector": "Lien",

View File

@@ -6761,6 +6761,8 @@
"codeSource": "소스(TypeScript)",
"codeTimeout": "제한 시간(ms)",
"cycleBlocked": "",
"deleteEdge": "",
"deleteNode": "",
"edgeCondition": "",
"edgeConditionLabel": "조건: {{condition}}",
"edgeInspector": "에지",

View File

@@ -6761,6 +6761,8 @@
"codeSource": "源代码(TypeScript)",
"codeTimeout": "超时(毫秒)",
"cycleBlocked": "",
"deleteEdge": "",
"deleteNode": "",
"edgeCondition": "",
"edgeConditionLabel": "条件:{{condition}}",
"edgeInspector": "连线",

View File

@@ -6761,6 +6761,8 @@
"codeSource": "原始碼(TypeScript)",
"codeTimeout": "逾時(毫秒)",
"cycleBlocked": "",
"deleteEdge": "",
"deleteNode": "",
"edgeCondition": "",
"edgeConditionLabel": "條件:{{condition}}",
"edgeInspector": "連線",

View File

@@ -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",