From 602141b573b93043ce0929897003594ff18fa90f Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 6 Aug 2026 11:12:51 +0000 Subject: [PATCH] fix(builder): batch multi-select drag position saves atomically Concurrent per-node PATCH requests during multi-select drag used read-modify-write on the full nodes array, so the last write could revert another node's new position. Coalesce drag-stop updates into one workflow PATCH per microtask batch. Co-authored-by: esadrianno --- .../hooks/use-builder-graph-mutations.ts | 58 +++++++++--- .../use-builder-graph-mutations.test.tsx | 94 +++++++++++++++++++ 2 files changed, 141 insertions(+), 11 deletions(-) create mode 100644 tests/components/use-builder-graph-mutations.test.tsx diff --git a/src/components/builder/hooks/use-builder-graph-mutations.ts b/src/components/builder/hooks/use-builder-graph-mutations.ts index 4eda492..c633fb9 100644 --- a/src/components/builder/hooks/use-builder-graph-mutations.ts +++ b/src/components/builder/hooks/use-builder-graph-mutations.ts @@ -48,6 +48,11 @@ export function useBuilderGraphMutations(options: { } = options const layoutTransitionTimeoutRef = useRef | null>(null) + const pendingDragBatchRef = useRef<{ + positions: Map + previous: Workflow | null + flushScheduled: boolean + }>({ positions: new Map(), previous: null, flushScheduled: false }) const [isLayoutTransitioning, setIsLayoutTransitioning] = useState(false) useEffect(() => { @@ -184,25 +189,56 @@ export function useBuilderGraphMutations(options: { [workflowId, workflow, saveToHistory, mutateWorkflow, safeFetch], ) + const flushPendingDragPositions = useCallback(async () => { + const batch = pendingDragBatchRef.current + batch.flushScheduled = false + if (!workflowId || !workflow || batch.positions.size === 0) { + batch.positions.clear() + batch.previous = null + return + } + + const previous = batch.previous ?? workflow + const positionUpdates = new Map(batch.positions) + batch.positions.clear() + batch.previous = null + + const updatedNodes = workflow.nodes.map((node) => { + const position = positionUpdates.get(node.id) + return position ? { ...node, position } : node + }) + + const response = await safeFetch(`/api/workflows/${workflowId}`, { + method: "PATCH", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ nodes: updatedNodes }), + }) + if (response.ok) { + saveToHistory(previous) + } + mutateWorkflow(workflowId) + }, [workflowId, workflow, saveToHistory, mutateWorkflow, safeFetch]) + const handleNodeDragStop = useCallback( - async (_: React.MouseEvent, node: Node) => { + (_: React.MouseEvent, node: Node) => { if (!workflowId || !workflow) return - const previous = workflow const snappedPosition = { x: Math.round(node.position.x / GRID_SIZE) * GRID_SIZE, y: Math.round(node.position.y / GRID_SIZE) * GRID_SIZE, } - const response = await safeFetch(`/api/workflows/${workflowId}/nodes/${node.id}`, { - method: "PATCH", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ position: snappedPosition }), - }) - if (response.ok) { - saveToHistory(previous) + const batch = pendingDragBatchRef.current + if (batch.previous === null) { + batch.previous = workflow + } + batch.positions.set(node.id, snappedPosition) + if (!batch.flushScheduled) { + batch.flushScheduled = true + queueMicrotask(() => { + void flushPendingDragPositions() + }) } - mutateWorkflow(workflowId) }, - [workflowId, workflow, saveToHistory, mutateWorkflow, safeFetch], + [workflowId, workflow, flushPendingDragPositions], ) const handleAddNode = useCallback( diff --git a/tests/components/use-builder-graph-mutations.test.tsx b/tests/components/use-builder-graph-mutations.test.tsx new file mode 100644 index 0000000..3efcb8c --- /dev/null +++ b/tests/components/use-builder-graph-mutations.test.tsx @@ -0,0 +1,94 @@ +// @vitest-environment jsdom +import { describe, it, expect, vi, beforeEach } from "vitest" +import { renderHook, act } from "@testing-library/react" +import type { Node } from "@xyflow/react" +import { useBuilderGraphMutations } from "@/components/builder/hooks/use-builder-graph-mutations" +import type { WorkflowNodeData } from "@/components/builder/canvas-node" +import type { NodeType, Workflow } from "@/lib/workflow-types" + +const workflow: Workflow = { + id: "wf-1", + name: "Test", + description: "", + nodes: [ + { id: "n1", type: "agent", position: { x: 0, y: 0 }, data: { label: "A" } }, + { id: "n2", type: "agent", position: { x: 200, y: 0 }, data: { label: "B" } }, + ], + connections: [], + version: 1, + createdAt: new Date(), + updatedAt: new Date(), +} + +function dragNode( + id: string, + position: { x: number; y: number }, +): Node { + return { + id, + type: "agent", + position, + data: { label: id }, + } +} + +describe("useBuilderGraphMutations handleNodeDragStop", () => { + const saveToHistory = vi.fn() + const mutateWorkflow = vi.fn() + const safeFetch = vi.fn() + const toast = vi.fn() + const onNodesChange = vi.fn() + const onEdgesChange = vi.fn() + + beforeEach(() => { + vi.clearAllMocks() + safeFetch.mockResolvedValue({ ok: true }) + }) + + it("batches multi-select drag stops into one workflow PATCH", async () => { + const { result } = renderHook(() => + useBuilderGraphMutations({ + workflowId: "wf-1", + workflow, + edges: [], + saveToHistory, + mutateWorkflow, + safeFetch, + toast, + screenToFlowPosition: (pos) => pos, + onNodesChange, + onEdgesChange, + }), + ) + + act(() => { + result.current.handleNodeDragStop( + {} as React.MouseEvent, + dragNode("n1", { x: 41, y: 61 }), + ) + result.current.handleNodeDragStop( + {} as React.MouseEvent, + dragNode("n2", { x: 241, y: 61 }), + ) + }) + + await act(async () => { + await Promise.resolve() + }) + + expect(safeFetch).toHaveBeenCalledTimes(1) + expect(safeFetch).toHaveBeenCalledWith("/api/workflows/wf-1", { + method: "PATCH", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + nodes: [ + { ...workflow.nodes[0], position: { x: 40, y: 60 } }, + { ...workflow.nodes[1], position: { x: 240, y: 60 } }, + ], + }), + }) + expect(saveToHistory).toHaveBeenCalledTimes(1) + expect(saveToHistory).toHaveBeenCalledWith(workflow) + expect(mutateWorkflow).toHaveBeenCalledWith("wf-1") + }) +})