fix: v2 - proper undo for del action on canvas

src/routes/v2/pages/Editor/components/FlowCanvas/FlowCanvas.tsx

@@ -30,11 +30,10 @@ import { FloatingSelectionToolbar } from "./components/FloatingSelectionToolbar"
 import { useClipboardShortcuts } from "./hooks/useClipboardShortcuts";
 import { useConnectionBehavior } from "./hooks/useConnectionBehavior";
 import { useDropBehavior } from "./hooks/useDropBehavior";
+import { useFlowCanvasOnBeforeDelete } from "./hooks/useFlowCanvasOnBeforeDelete";
 import { useNodeEdgeChanges } from "./hooks/useNodeEdgeChanges";
 import { usePaneClickBehavior } from "./hooks/usePaneClickBehavior";
 
-const DELETE_KEY_CODE = ["Delete", "Backspace"];
-
 interface FlowCanvasProps {
   spec: ComponentSpec | null;
   className?: string;
@@ -67,6 +66,8 @@ export const FlowCanvas = observer(function FlowCanvas({
     selectionBehavior,
   } = useFlowCanvasState({ spec, metaKeyPressed, isConnecting });
 
+  const onBeforeDelete = useFlowCanvasOnBeforeDelete(spec);
+
   useFitViewOnFocus();
   useAutoLayout(spec);
   useClipboardShortcuts(spec, containerRef, reactFlowInstance);
@@ -109,8 +110,9 @@ export const FlowCanvas = observer(function FlowCanvas({
         onEdgeClick={onEdgeClick}
         onInit={setReactFlowInstance}
         onViewportChange={handleViewportChange}
+        onBeforeDelete={onBeforeDelete}
         connectionLineComponent={ConnectionLine}
-        deleteKeyCode={DELETE_KEY_CODE}
+        deleteKeyCode={["Delete", "Backspace"]}
         className={cn(
           shiftKeyPressed && !isConnecting && "cursor-crosshair",
           !isDetailedView && "connections-disabled",

src/routes/v2/pages/Editor/components/FlowCanvas/canvasDeleteSelection.ts

@@ -0,0 +1,177 @@
+import type { Edge, Node } from "@xyflow/react";
+
+import type { ComponentSpec } from "@/models/componentSpec";
+import {
+  deleteSelectedEdgesByEdgeIds,
+  edgeIdToBindingId,
+  removeBindingsAndStripConduits,
+} from "@/routes/v2/pages/Editor/store/actions/connection.actions";
+import {
+  deleteSelectedNodes,
+  deleteSelectedNodesCore,
+} from "@/routes/v2/pages/Editor/store/actions/task.actions";
+import type { NodeTypeRegistry } from "@/routes/v2/shared/nodes/registry";
+import type { UndoGroupable } from "@/routes/v2/shared/nodes/types";
+import type {
+  EditorStore,
+  NodeEntityType,
+  SelectedNode,
+} from "@/routes/v2/shared/store/editorStore";
+import type { ParentContext } from "@/routes/v2/shared/store/navigationStore";
+
+interface CanvasDeleteContext {
+  undo: UndoGroupable;
+  spec: ComponentSpec;
+  parentContext: ParentContext | null | undefined;
+  selectedEdges: Edge[];
+  nodeSelection: SelectedNode[];
+}
+
+function bindingIdsFromEdges(edges: Edge[]): string[] {
+  return edges
+    .map((e) => edgeIdToBindingId(e.id))
+    .filter((id): id is string => id !== null);
+}
+
+function deleteMixedEdgeAndNodeSelection(
+  undo: UndoGroupable,
+  spec: ComponentSpec,
+  selectedEdges: Edge[],
+  nodeSelection: SelectedNode[],
+  parentContext: ParentContext | null | undefined,
+): void {
+  const bindingIds = bindingIdsFromEdges(selectedEdges);
+  undo.withGroup("Delete selection", () => {
+    removeBindingsAndStripConduits(spec, bindingIds, undo);
+    deleteSelectedNodesCore(undo, spec, nodeSelection, parentContext);
+  });
+}
+
+function deleteEdgesOnlySelection(
+  undo: UndoGroupable,
+  spec: ComponentSpec,
+  selectedEdges: Edge[],
+): void {
+  deleteSelectedEdgesByEdgeIds(
+    undo,
+    spec,
+    selectedEdges.map((e) => e.id),
+  );
+}
+
+function applyCanvasDeleteSelection(ctx: CanvasDeleteContext): void {
+  const { selectedEdges, nodeSelection } = ctx;
+
+  if (selectedEdges.length > 0 && nodeSelection.length > 0) {
+    deleteMixedEdgeAndNodeSelection(
+      ctx.undo,
+      ctx.spec,
+      selectedEdges,
+      nodeSelection,
+      ctx.parentContext,
+    );
+    return;
+  }
+
+  if (selectedEdges.length > 0) {
+    deleteEdgesOnlySelection(ctx.undo, ctx.spec, selectedEdges);
+    return;
+  }
+
+  deleteSelectedNodes(ctx.undo, ctx.spec, nodeSelection, ctx.parentContext);
+}
+
+function clearEditorSelectionAfterDelete(editor: EditorStore): void {
+  editor.clearSelection();
+  editor.clearMultiSelection();
+}
+
+const NODE_ENTITY_TYPES = [
+  "task",
+  "input",
+  "output",
+  "conduit",
+  "flex",
+] as const satisfies readonly NodeEntityType[];
+
+function isNodeEntityType(value: string): value is NodeEntityType {
+  for (const allowed of NODE_ENTITY_TYPES) {
+    if (value === allowed) return true;
+  }
+  return false;
+}
+
+export function getSelectedEdgesFromInstance(
+  reactFlowInstance: { getEdges: () => Edge[] } | null,
+): Edge[] {
+  return reactFlowInstance?.getEdges().filter((e) => e.selected) ?? [];
+}
+
+/**
+ * Maps React Flow nodes slated for deletion to editor `SelectedNode` shapes
+ * using the node registry (same source of truth as `getEffectiveSelection`).
+ */
+function selectedNodesFromFlowNodes(
+  registry: NodeTypeRegistry,
+  spec: ComponentSpec,
+  nodes: Node[],
+): SelectedNode[] {
+  const result: SelectedNode[] = [];
+  for (const node of nodes) {
+    const manifest = registry.getByNodeId(spec, node.id);
+    if (!manifest) continue;
+    const position = manifest.getPosition(spec, node.id);
+    if (!position) continue;
+    const entityType = manifest.entityType;
+    if (!isNodeEntityType(entityType)) continue;
+    result.push({
+      id: node.id,
+      type: entityType,
+      position,
+    });
+  }
+  return result;
+}
+
+// --- React Flow delete pipeline (pure handler + caller-owned deps) ---
+
+export interface FlowCanvasDeleteDeps {
+  spec: ComponentSpec | null;
+  undo: UndoGroupable;
+  editor: EditorStore;
+  parentContext: ParentContext | null | undefined;
+  registry: NodeTypeRegistry;
+}
+
+/**
+ * Runs spec/undo deletion for the elements React Flow is about to remove, then
+ * aborts RF’s internal removal so controlled `nodes`/`edges` stay driven by the spec.
+ */
+export async function runFlowCanvasOnBeforeDelete(
+  { spec, undo, editor, parentContext, registry }: FlowCanvasDeleteDeps,
+  {
+    nodes,
+    edges,
+  }: {
+    nodes: Node[];
+    edges: Edge[];
+  },
+): Promise<boolean | { nodes: Node[]; edges: Edge[] }> {
+  if (!spec) return true;
+  if (nodes.length === 0 && edges.length === 0) return true;
+
+  const nodeSelection = selectedNodesFromFlowNodes(registry, spec, nodes);
+  if (nodes.length > 0 && nodeSelection.length !== nodes.length) {
+    return false;
+  }
+
+  applyCanvasDeleteSelection({
+    undo,
+    spec,
+    parentContext,
+    selectedEdges: edges,
+    nodeSelection,
+  });
+  clearEditorSelectionAfterDelete(editor);
+  return false;
+}

src/routes/v2/pages/Editor/components/FlowCanvas/components/FloatingSelectionToolbar.tsx

@@ -2,6 +2,7 @@ import { NodeToolbar, useReactFlow } from "@xyflow/react";
 import { observer } from "mobx-react-lite";
 
 import type { ComponentSpec } from "@/models/componentSpec";
+import { getSelectedEdgesFromInstance } from "@/routes/v2/pages/Editor/components/FlowCanvas/canvasDeleteSelection";
 import { usePipelineActions } from "@/routes/v2/pages/Editor/store/actions/usePipelineActions";
 import { useTaskActions } from "@/routes/v2/pages/Editor/store/actions/useTaskActions";
 import { useSharedStores } from "@/routes/v2/shared/store/SharedStoreContext";
@@ -11,12 +12,8 @@ import { SelectionToolbar } from "./SelectionToolbar";
 export const FloatingSelectionToolbar = observer(
   function FloatingSelectionToolbar({ spec }: { spec: ComponentSpec | null }) {
     const { editor } = useSharedStores();
-    const {
-      duplicateSelectedNodes,
-      copySelectedNodes,
-      pasteNodes,
-      deleteSelectedNodes,
-    } = useTaskActions();
+    const { duplicateSelectedNodes, copySelectedNodes, pasteNodes } =
+      useTaskActions();
     const { createSubgraph } = usePipelineActions();
     const { multiSelection } = editor;
     const reactFlow = useReactFlow();
@@ -44,8 +41,12 @@ export const FloatingSelectionToolbar = observer(
     };
 
     const handleDelete = () => {
-      if (!spec) return;
-      deleteSelectedNodes(spec, multiSelection);
+      if (!spec || !reactFlow.viewportInitialized) return;
+      const selectedEdges = getSelectedEdgesFromInstance(reactFlow);
+      void reactFlow.deleteElements({
+        nodes: multiSelection.map((n) => ({ id: n.id })),
+        edges: selectedEdges.map((e) => ({ id: e.id })),
+      });
     };
 
     const selectedTasks = multiSelection.filter((n) => n.type === "task");

src/routes/v2/pages/Editor/components/FlowCanvas/hooks/useFlowCanvasOnBeforeDelete.ts

@@ -0,0 +1,34 @@
+import type { OnBeforeDelete } from "@xyflow/react";
+
+import type { ComponentSpec } from "@/models/componentSpec";
+import {
+  type FlowCanvasDeleteDeps,
+  runFlowCanvasOnBeforeDelete,
+} from "@/routes/v2/pages/Editor/components/FlowCanvas/canvasDeleteSelection";
+import { useEditorSession } from "@/routes/v2/pages/Editor/store/EditorSessionContext";
+import { useNodeRegistry } from "@/routes/v2/shared/nodes/NodeRegistryContext";
+import { useSharedStores } from "@/routes/v2/shared/store/SharedStoreContext";
+
+/**
+ * `onBeforeDelete` for React Flow: applies editor/spec deletion and aborts RF’s
+ * internal removal so controlled `nodes`/`edges` stay spec-driven.
+ */
+export function useFlowCanvasOnBeforeDelete(
+  spec: ComponentSpec | null,
+): OnBeforeDelete {
+  const { editor, navigation } = useSharedStores();
+  const { undo } = useEditorSession();
+  const registry = useNodeRegistry();
+
+  return (params) =>
+    runFlowCanvasOnBeforeDelete(
+      {
+        spec,
+        undo,
+        editor,
+        parentContext: navigation.parentContext,
+        registry,
+      } satisfies FlowCanvasDeleteDeps,
+      params,
+    );
+}

src/routes/v2/pages/Editor/components/FlowCanvas/hooks/useNodeEdgeChanges.ts

@@ -1,25 +1,22 @@
 import type { EdgeChange, NodeChange, ReactFlowProps } from "@xyflow/react";
 
 import type { ComponentSpec } from "@/models/componentSpec";
-import { cleanupDeletedBinding } from "@/routes/v2/pages/Editor/nodes/ConduitNode/conduits.actions";
-import { deleteEdge } from "@/routes/v2/pages/Editor/store/actions";
-import { cleanupAggregatorInputForBinding } from "@/routes/v2/pages/Editor/store/actions/aggregator.actions";
 import { useEditorSession } from "@/routes/v2/pages/Editor/store/EditorSessionContext";
 import { useNodeRegistry } from "@/routes/v2/shared/nodes/NodeRegistryContext";
-import { useSharedStores } from "@/routes/v2/shared/store/SharedStoreContext";
 
 export function useNodeEdgeChanges(
   spec: ComponentSpec | null,
   rfOnNodesChange: (changes: NodeChange[]) => void,
   rfOnEdgesChange: (changes: EdgeChange[]) => void,
 ): Required<Pick<ReactFlowProps, "onNodesChange" | "onEdgesChange">> {
   const registry = useNodeRegistry();
-  const { editor, navigation } = useSharedStores();
   const { undo } = useEditorSession();
 
   const onNodesChange = (changes: NodeChange[]) => {
+    const rfChanges = changes.filter((c) => c.type !== "remove");
+
     if (!spec) {
-      rfOnNodesChange(changes);
+      rfOnNodesChange(rfChanges);
       return;
     }
 
@@ -39,59 +36,15 @@ export function useNodeEdgeChanges(
       });
     }
 
-    const removeChanges = changes.filter((change) => change.type === "remove");
-    for (const change of removeChanges) {
-      if ("id" in change) {
-        const manifest = registry.getByNodeId(spec, change.id);
-        // todo: move action to a separate file
-        undo.withGroup("Delete node", () => {
-          manifest?.deleteNode(undo, spec, change.id, navigation.parentContext);
-        });
-
-        // deselect removed nodes
-        editor.clearSelection();
-        editor.clearMultiSelection();
-      }
-    }
-
-    rfOnNodesChange(changes);
+    rfOnNodesChange(rfChanges);
   };
 
+  // Edge/node deletes go through React Flow (`deleteKeyCode` or `deleteElements`) +
+  // `runFlowCanvasOnBeforeDelete` (spec + undo), which aborts RF removal; RF may
+  // still emit `remove` changes — drop them so we don't double-apply.
+  // `useFlowCanvasState` re-syncs nodes/edges from the spec.
   const onEdgesChange = (changes: EdgeChange[]) => {
-    if (!spec) {
-      rfOnEdgesChange(changes);
-      return;
-    }
-
-    const removeChanges = changes.filter((change) => change.type === "remove");
-    for (const change of removeChanges) {
-      if ("id" in change) {
-        const edgeId = change.id;
-        const bindingIdMatch = edgeId.match(/^edge_(.+)$/);
-        const binding = bindingIdMatch
-          ? spec.bindings.find((b) => b.$id === bindingIdMatch[1])
-          : undefined;
-        const aggregatorTarget = binding
-          ? {
-              targetEntityId: binding.targetEntityId,
-              targetPortName: binding.targetPortName,
-            }
-          : null;
-
-        deleteEdge(undo, spec, edgeId);
-
-        if (bindingIdMatch) {
-          // todo: find out how to decouple
-          cleanupDeletedBinding(undo, spec, bindingIdMatch[1]);
-        }
-
-        if (aggregatorTarget) {
-          cleanupAggregatorInputForBinding(undo, spec, aggregatorTarget);
-        }
-      }
-    }
-
-    rfOnEdgesChange(changes);
+    rfOnEdgesChange(changes.filter((c) => c.type !== "remove"));
   };
 
   return { onNodesChange, onEdgesChange };