fix(canvas): reject invalid edge splits before mutation

components/canvas/__tests__/use-canvas-node-interactions.test.tsx

@@ -0,0 +1,162 @@
+// @vitest-environment jsdom
+
+import React, { act, useEffect, useRef, useState } from "react";
+import { createRoot, type Root } from "react-dom/client";
+import type { Edge as RFEdge, Node as RFNode } from "@xyflow/react";
+import { afterEach, describe, expect, it, vi } from "vitest";
+
+import type { Id } from "@/convex/_generated/dataModel";
+import { useCanvasNodeInteractions } from "@/components/canvas/use-canvas-node-interactions";
+import type { CanvasConnectionValidationReason } from "@/lib/canvas-connection-policy";
+
+vi.mock("@/components/canvas/canvas-helpers", async () => {
+  const actual = await vi.importActual<
+    typeof import("@/components/canvas/canvas-helpers")
+  >("@/components/canvas/canvas-helpers");
+
+  return {
+    ...actual,
+    getNodeCenterClientPosition: vi.fn(() => ({ x: 240, y: 140 })),
+    getIntersectedEdgeId: vi.fn(() => "edge-image-curves"),
+  };
+});
+
+const asCanvasId = (id: string): Id<"canvases"> => id as Id<"canvases">;
+
+type HarnessProps = {
+  nodes: RFNode[];
+  edges: RFEdge[];
+  runMoveNodeMutation: ReturnType<typeof vi.fn>;
+  runBatchMoveNodesMutation: ReturnType<typeof vi.fn>;
+  runResizeNodeMutation: ReturnType<typeof vi.fn>;
+  runSplitEdgeAtExistingNodeMutation: ReturnType<typeof vi.fn>;
+  onInvalidConnection: ReturnType<typeof vi.fn<(reason: CanvasConnectionValidationReason) => void>>;
+  syncPendingMoveForClientRequest: ReturnType<typeof vi.fn>;
+};
+
+const latestHandlersRef: {
+  current: ReturnType<typeof useCanvasNodeInteractions> | null;
+} = { current: null };
+
+(globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
+
+function HookHarness(props: HarnessProps) {
+  const [, setNodes] = useState<RFNode[]>(props.nodes);
+  const [edges, setEdges] = useState<RFEdge[]>(props.edges);
+  const isDragging = useRef(false);
+  const isResizing = useRef(false);
+  const pendingLocalPositionUntilConvexMatchesRef = useRef(new Map());
+  const preferLocalPositionNodeIdsRef = useRef(new Set<string>());
+  const pendingMoveAfterCreateRef = useRef(new Map());
+  const resolvedRealIdByClientRequestRef = useRef(new Map());
+  const pendingEdgeSplitByClientRequestRef = useRef(new Map());
+
+  const handlers = useCanvasNodeInteractions({
+    canvasId: asCanvasId("canvas-1"),
+    nodes: props.nodes,
+    edges,
+    setNodes,
+    setEdges,
+    refs: {
+      isDragging,
+      isResizing,
+      pendingLocalPositionUntilConvexMatchesRef,
+      preferLocalPositionNodeIdsRef,
+      pendingMoveAfterCreateRef,
+      resolvedRealIdByClientRequestRef,
+      pendingEdgeSplitByClientRequestRef,
+    },
+    runResizeNodeMutation: props.runResizeNodeMutation,
+    runMoveNodeMutation: props.runMoveNodeMutation,
+    runBatchMoveNodesMutation: props.runBatchMoveNodesMutation,
+    runSplitEdgeAtExistingNodeMutation: props.runSplitEdgeAtExistingNodeMutation,
+    onInvalidConnection: props.onInvalidConnection,
+    syncPendingMoveForClientRequest: props.syncPendingMoveForClientRequest,
+  });
+
+  useEffect(() => {
+    latestHandlersRef.current = handlers;
+  }, [handlers]);
+
+  return null;
+}
+
+describe("useCanvasNodeInteractions", () => {
+  let container: HTMLDivElement | null = null;
+  let root: Root | null = null;
+
+  afterEach(async () => {
+    latestHandlersRef.current = null;
+    vi.clearAllMocks();
+    if (root) {
+      await act(async () => {
+        root?.unmount();
+      });
+    }
+    container?.remove();
+    root = null;
+    container = null;
+  });
+
+  it("does not call splitEdgeAtExistingNode for an invalid drag-split", async () => {
+    const runMoveNodeMutation = vi.fn(async () => undefined);
+    const runBatchMoveNodesMutation = vi.fn(async () => undefined);
+    const runResizeNodeMutation = vi.fn(async () => undefined);
+    const runSplitEdgeAtExistingNodeMutation = vi.fn(async () => undefined);
+    const onInvalidConnection = vi.fn<(reason: CanvasConnectionValidationReason) => void>();
+    const syncPendingMoveForClientRequest = vi.fn(async () => undefined);
+
+    container = document.createElement("div");
+    document.body.appendChild(container);
+    root = createRoot(container);
+
+    const draggedNode: RFNode = {
+      id: "node-video",
+      type: "video",
+      position: { x: 320, y: 180 },
+      data: {},
+    };
+
+    await act(async () => {
+      root?.render(
+        <HookHarness
+          nodes={[
+            { id: "node-image", type: "image", position: { x: 0, y: 0 }, data: {} },
+            { id: "node-curves", type: "curves", position: { x: 400, y: 120 }, data: {} },
+            draggedNode,
+          ]}
+          edges={[
+            {
+              id: "edge-image-curves",
+              source: "node-image",
+              target: "node-curves",
+            },
+          ]}
+          runMoveNodeMutation={runMoveNodeMutation}
+          runBatchMoveNodesMutation={runBatchMoveNodesMutation}
+          runResizeNodeMutation={runResizeNodeMutation}
+          runSplitEdgeAtExistingNodeMutation={runSplitEdgeAtExistingNodeMutation}
+          onInvalidConnection={onInvalidConnection}
+          syncPendingMoveForClientRequest={syncPendingMoveForClientRequest}
+        />,
+      );
+    });
+
+    await act(async () => {
+      latestHandlersRef.current?.onNodeDrag({} as React.MouseEvent, draggedNode);
+      latestHandlersRef.current?.onNodeDragStop(
+        {} as React.MouseEvent,
+        draggedNode,
+        [draggedNode],
+      );
+    });
+
+    expect(runSplitEdgeAtExistingNodeMutation).not.toHaveBeenCalled();
+    expect(onInvalidConnection).toHaveBeenCalledWith("adjustment-source-invalid");
+    expect(runMoveNodeMutation).toHaveBeenCalledWith({
+      nodeId: "node-video",
+      positionX: 320,
+      positionY: 180,
+    });
+  });
+});

components/canvas/canvas-connection-validation.ts

@@ -0,0 +1,76 @@
+import type { Connection, Edge as RFEdge, Node as RFNode } from "@xyflow/react";
+
+import {
+  validateCanvasConnectionPolicy,
+  type CanvasConnectionValidationReason,
+} from "@/lib/canvas-connection-policy";
+
+export function validateCanvasConnection(
+  connection: Connection,
+  nodes: RFNode[],
+  edges: RFEdge[],
+  edgeToReplaceId?: string,
+): CanvasConnectionValidationReason | null {
+  if (!connection.source || !connection.target) return "incomplete";
+  if (connection.source === connection.target) return "self-loop";
+
+  const sourceNode = nodes.find((node) => node.id === connection.source);
+  const targetNode = nodes.find((node) => node.id === connection.target);
+  if (!sourceNode || !targetNode) return "unknown-node";
+
+  return validateCanvasConnectionByType({
+    sourceType: sourceNode.type ?? "",
+    targetType: targetNode.type ?? "",
+    targetNodeId: connection.target,
+    edges,
+    edgeToReplaceId,
+  });
+}
+
+export function validateCanvasConnectionByType(args: {
+  sourceType: string;
+  targetType: string;
+  targetNodeId: string;
+  edges: RFEdge[];
+  edgeToReplaceId?: string;
+}): CanvasConnectionValidationReason | null {
+  const targetIncomingCount = args.edges.filter(
+    (edge) => edge.target === args.targetNodeId && edge.id !== args.edgeToReplaceId,
+  ).length;
+
+  return validateCanvasConnectionPolicy({
+    sourceType: args.sourceType,
+    targetType: args.targetType,
+    targetIncomingCount,
+  });
+}
+
+export function validateCanvasEdgeSplit(args: {
+  nodes: RFNode[];
+  edges: RFEdge[];
+  splitEdge: RFEdge;
+  middleNode: RFNode;
+}): CanvasConnectionValidationReason | null {
+  const sourceNode = args.nodes.find((node) => node.id === args.splitEdge.source);
+  const targetNode = args.nodes.find((node) => node.id === args.splitEdge.target);
+
+  if (!sourceNode || !targetNode) {
+    return "unknown-node";
+  }
+
+  return (
+    validateCanvasConnectionByType({
+      sourceType: sourceNode.type ?? "",
+      targetType: args.middleNode.type ?? "",
+      targetNodeId: args.middleNode.id,
+      edges: args.edges,
+    }) ??
+    validateCanvasConnectionByType({
+      sourceType: args.middleNode.type ?? "",
+      targetType: targetNode.type ?? "",
+      targetNodeId: targetNode.id,
+      edges: args.edges,
+      edgeToReplaceId: args.splitEdge.id,
+    })
+  );
+}

components/canvas/canvas.tsx

@@ -30,7 +30,6 @@ import { toast } from "@/lib/toast";
 import {
   CANVAS_NODE_DND_MIME,
   type CanvasConnectionValidationReason,
-  validateCanvasConnectionPolicy,
 } from "@/lib/canvas-connection-policy";
 import { showCanvasConnectionRejectedToast } from "@/lib/toast-messages";
 import { useMutation } from "convex/react";
@@ -43,6 +42,10 @@ import {
 } from "@/lib/canvas-node-types";
 
 import { nodeTypes } from "./node-types";
+import {
+  validateCanvasConnection,
+  validateCanvasConnectionByType,
+} from "./canvas-connection-validation";
 import {
   NODE_DEFAULTS,
   NODE_HANDLE_MAP,
@@ -92,45 +95,6 @@ interface CanvasInnerProps {
   canvasId: Id<"canvases">;
 }
 
-function validateCanvasConnection(
-  connection: Connection,
-  nodes: RFNode[],
-  edges: RFEdge[],
-  edgeToReplaceId?: string,
-): CanvasConnectionValidationReason | null {
-  if (!connection.source || !connection.target) return "incomplete";
-  if (connection.source === connection.target) return "self-loop";
-
-  const sourceNode = nodes.find((node) => node.id === connection.source);
-  const targetNode = nodes.find((node) => node.id === connection.target);
-  if (!sourceNode || !targetNode) return "unknown-node";
-
-  return validateCanvasConnectionPolicy({
-    sourceType: sourceNode.type ?? "",
-    targetType: targetNode.type ?? "",
-    targetIncomingCount: edges.filter(
-      (edge) => edge.target === connection.target && edge.id !== edgeToReplaceId,
-    ).length,
-  });
-}
-
-function validateCanvasConnectionByType(args: {
-  sourceType: string;
-  targetType: string;
-  targetNodeId: string;
-  edges: RFEdge[];
-}): CanvasConnectionValidationReason | null {
-  const targetIncomingCount = args.edges.filter(
-    (edge) => edge.target === args.targetNodeId,
-  ).length;
-
-  return validateCanvasConnectionPolicy({
-    sourceType: args.sourceType,
-    targetType: args.targetType,
-    targetIncomingCount,
-  });
-}
-
 function CanvasInner({ canvasId }: CanvasInnerProps) {
   const t = useTranslations('toasts');
   const showConnectionRejectedToast = useCallback(
@@ -378,6 +342,7 @@ function CanvasInner({ canvasId }: CanvasInnerProps) {
     onNodeDragStop,
   } = useCanvasNodeInteractions({
     canvasId,
+    nodes,
     edges,
     setNodes,
     setEdges,
@@ -394,6 +359,7 @@ function CanvasInner({ canvasId }: CanvasInnerProps) {
     runMoveNodeMutation,
     runBatchMoveNodesMutation,
     runSplitEdgeAtExistingNodeMutation,
+    onInvalidConnection: showConnectionRejectedToast,
     syncPendingMoveForClientRequest,
   });
 

components/canvas/use-canvas-node-interactions.ts

@@ -15,6 +15,7 @@ import {
 
 import type { Id } from "@/convex/_generated/dataModel";
 import { NODE_HANDLE_MAP } from "@/lib/canvas-utils";
+import type { CanvasConnectionValidationReason } from "@/lib/canvas-connection-policy";
 import {
   clientRequestIdFromOptimisticNodeId,
   EDGE_INTERSECTION_HIGHLIGHT_STYLE,
@@ -25,6 +26,7 @@ import {
   isOptimisticNodeId,
   normalizeHandle,
 } from "./canvas-helpers";
+import { validateCanvasEdgeSplit } from "./canvas-connection-validation";
 import { adjustNodeDimensionChanges } from "./canvas-node-change-helpers";
 
 type PositionPin = { x: number; y: number };
@@ -87,6 +89,7 @@ type CanvasNodeInteractionRefs = {
 
 export function useCanvasNodeInteractions(args: {
   canvasId: Id<"canvases">;
+  nodes: RFNode[];
   edges: RFEdge[];
   setNodes: Dispatch<SetStateAction<RFNode[]>>;
   setEdges: Dispatch<SetStateAction<RFEdge[]>>;
@@ -95,6 +98,7 @@ export function useCanvasNodeInteractions(args: {
   runMoveNodeMutation: RunMoveNodeMutation;
   runBatchMoveNodesMutation: RunBatchMoveNodesMutation;
   runSplitEdgeAtExistingNodeMutation: RunSplitEdgeAtExistingNodeMutation;
+  onInvalidConnection: (reason: CanvasConnectionValidationReason) => void;
   syncPendingMoveForClientRequest: (
     clientRequestId: string,
     realId?: Id<"nodes">,
@@ -102,6 +106,7 @@ export function useCanvasNodeInteractions(args: {
 }) {
   const {
     canvasId,
+    nodes,
     edges,
     setNodes,
     setEdges,
@@ -109,6 +114,7 @@ export function useCanvasNodeInteractions(args: {
     runMoveNodeMutation,
     runBatchMoveNodesMutation,
     runSplitEdgeAtExistingNodeMutation,
+    onInvalidConnection,
     syncPendingMoveForClientRequest,
   } = args;
   const {
@@ -327,6 +333,22 @@ export function useCanvasNodeInteractions(args: {
             hasHandleKey(splitHandles, "source") &&
             hasHandleKey(splitHandles, "target");
 
+          const splitValidationError =
+            splitEligible && intersectedEdge
+              ? validateCanvasEdgeSplit({
+                  nodes,
+                  edges,
+                  splitEdge: intersectedEdge,
+                  middleNode: primaryNode,
+                })
+              : null;
+
+          if (splitValidationError) {
+            onInvalidConnection(splitValidationError);
+          }
+
+          const canSplit = splitEligible && intersectedEdge && !splitValidationError;
+
           if (draggedNodes.length > 1) {
             for (const draggedNode of draggedNodes) {
               const clientRequestId = clientRequestIdFromOptimisticNodeId(
@@ -354,7 +376,7 @@ export function useCanvasNodeInteractions(args: {
               });
             }
 
-            if (!splitEligible || !intersectedEdge) {
+            if (!canSplit || !intersectedEdge) {
               return;
             }
 
@@ -401,7 +423,7 @@ export function useCanvasNodeInteractions(args: {
             return;
           }
 
-          if (!splitEligible || !intersectedEdge) {
+          if (!canSplit || !intersectedEdge) {
             const singleClientRequestId = clientRequestIdFromOptimisticNodeId(
               primaryNode.id,
             );
@@ -490,8 +512,10 @@ export function useCanvasNodeInteractions(args: {
     [
       canvasId,
       clearHighlightedIntersectionEdge,
+      nodes,
       edges,
       isDragging,
+      onInvalidConnection,
       pendingEdgeSplitByClientRequestRef,
       pendingMoveAfterCreateRef,
       resolvedRealIdByClientRequestRef,

vitest.config.ts

@@ -13,6 +13,7 @@ export default defineConfig({
       "tests/**/*.test.ts",
       "components/canvas/__tests__/canvas-flow-reconciliation-helpers.test.ts",
       "components/canvas/__tests__/use-canvas-flow-reconciliation.test.ts",
+      "components/canvas/__tests__/use-canvas-node-interactions.test.tsx",
       "components/canvas/__tests__/use-canvas-sync-engine.test.ts",
       "components/canvas/__tests__/use-canvas-sync-engine-hook.test.tsx",
     ],