fix(reorder): drag and drop hook

[icecrasher321]

Summary

Drag and drop fix for reordering in sidebar

Type of Change

• Bug fix

Testing

Tested manually

Checklist

• Code follows project style guidelines
• Self-reviewed my changes
• Tests added/updated and passing
• No new warnings introduced
• I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Apr 1, 2026 0:27am

[cursor]

PR Summary

Medium Risk
Moderate risk because it changes sidebar drag/drop reordering logic and optimistic cache updates, which can affect item ordering and parent assignment across folders. MCP refresh also now mutates stored workflow block params (server URL/name), potentially touching many workflows during refresh.

Overview
Fixes sidebar drag-and-drop reordering to avoid stale dropIndicator state and incorrect insert indices when dropping onto the dragged row (now using a synchronous ref and a corrected insert-index calculation that accounts for excluded moving items).

Normalizes “root” scoping by treating folderId/parentId as null consistently (including query mappers and reorder optimistic updates) and adjusts sibling caching to avoid using stale cached siblings during an active drag.

Separately, MCP server refresh now also syncs stored serverUrl/serverName into MCP tool blocks when server metadata changes, preventing stale UI badges after server edits.

^{Written by Cursor Bugbot for commit d49a41e. Configure here.}

[greptile-apps]

Greptile Summary

This PR fixes several inter-related bugs in the sidebar drag-and-drop system that collectively caused incorrect reordering, items silently moving to the wrong folder, and dropped items landing in the wrong position.

Key fixes:

• Race condition in drop handler — handleDrop now reads dropIndicatorRef.current (a synchronously-maintained mirror of state) instead of the React state dropIndicator. This prevents a "drop lands at wrong position" bug where the browser fires drop before React commits the final dragOver state update.
• null vs undefined scope mismatch — The API/cache layer could emit folderId: undefined while the comparison logic expected null, causing items to be treated as belonging to a different folder. mapFolder now normalises parentId to null, mapWorkflow normalises folderId to null, and the new isSameFolderScope helper coerces both sides to null before comparing.
• Corrupted insert index when dragging onto the dragged item — The old calculateInsertIndex searched remaining (the list with moving items already removed) for indicator.targetId. When the target was a moving item, findIndex returned -1 and the splice was miscalculated. The renamed getInsertIndexInRemaining correctly uses the full siblingItems list for index lookup, then counts only non-moving items up to that index.
• Stale sibling cache — The cache is now bypassed entirely during an active drag so that each getSiblingItems call reads fresh data from the query store. The cache is also cleared on drag start, on drop, and when workspaceId changes.
• Missing useCallback dependencies — getSiblingItems and collectMovingItems both closed over workspaceId without declaring it; both are now corrected.
• useReorderFolders optimistic updater — Changed from (old ?? []).map(…) to guard on !old?.length, so a not-yet-loaded (undefined) cache stays undefined rather than being reset to [].

Confidence Score: 5/5

Safe to merge; the single remaining finding is a minor performance suggestion that does not affect correctness.

All findings are P2. The core race-condition fix, null/undefined normalisation, and insert-index correction are all well-reasoned and correctly implemented. The only open point is that createEdgeDropZone still bypasses the deduplication guard (a pre-existing pattern that is now slightly more impactful after throttle removal), which is a style/performance suggestion rather than a data-correctness issue.

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/hooks/use-drag-drop.ts — specifically the createEdgeDropZone handler around line 584.

Important Files Changed

Filename	Overview
apps/sim/app/workspace/[workspaceId]/w/components/sidebar/hooks/use-drag-drop.ts	Core drag-and-drop hook refactored to fix: race condition in drop indicator (ref mirrors state), null/undefined scope mismatch (isSameFolderScope helper), corrupted insert index when dragging item is the target (getInsertIndexInRemaining), stale sibling cache, and missing workspaceId in useCallback deps. One minor performance regression: createEdgeDropZone bypasses the deduplication guard present in setNormalizedDropIndicator, causing unnecessary re-renders on every dragover event now that throttling is removed.
apps/sim/hooks/queries/folders.ts	Two clean fixes: mapFolder now normalises parentId to null (was undefined), and the useReorderFolders optimistic updater guards against an uninitialised cache (returns old unchanged instead of mapping over an empty array).
apps/sim/hooks/queries/utils/workflow-list-query.ts	Single-line fix: folderId is now normalised to null instead of undefined, aligning the workflow cache with the null-based scope comparison used throughout the drag-and-drop hook.

Sequence Diagram

sequenceDiagram
    participant User
    participant DragHandlers
    participant initDragOver
    participant setNormalizedDropIndicator
    participant dropIndicatorRef
    participant ReactState
    participant handleDrop
    participant handleSelectionDrop

    User->>DragHandlers: dragstart
    DragHandlers->>dropIndicatorRef: clear cache, isDraggingRef=true
    DragHandlers->>ReactState: setIsDragging(true)

    loop dragover events (no throttle)
        User->>DragHandlers: dragover
        DragHandlers->>initDragOver: called
        initDragOver-->>DragHandlers: true
        DragHandlers->>setNormalizedDropIndicator: indicator candidate
        setNormalizedDropIndicator->>setNormalizedDropIndicator: isSameFolderScope() normalise null/undefined
        setNormalizedDropIndicator->>setNormalizedDropIndicator: deduplicate (prev === next?)
        alt changed
            setNormalizedDropIndicator->>dropIndicatorRef: dropIndicatorRef.current = next (sync)
            setNormalizedDropIndicator->>ReactState: setDropIndicator(next) (async)
        else unchanged
            setNormalizedDropIndicator->>dropIndicatorRef: dropIndicatorRef.current = prev (no re-render)
        end
    end

    User->>DragHandlers: drop
    DragHandlers->>handleDrop: called
    handleDrop->>dropIndicatorRef: read indicator (sync, no stale-state risk)
    handleDrop->>dropIndicatorRef: clear (null)
    handleDrop->>ReactState: setIsDragging(false), setDropIndicator(null)
    handleDrop->>handleSelectionDrop: (indicator, selection)
    handleSelectionDrop->>handleSelectionDrop: getSiblingItems() — fresh, bypass cache
    handleSelectionDrop->>handleSelectionDrop: getInsertIndexInRemaining() — uses full siblingItems list
    handleSelectionDrop->>handleSelectionDrop: buildAndSubmitUpdates()

Loading

Comments Outside Diff (1)

1. apps/sim/app/workspace/[workspaceId]/w/components/sidebar/hooks/use-drag-drop.ts, line 584-600 (link)

   Edge drop zone bypasses deduplication, causing unnecessary re-renders

   After removing the 16ms throttle, every dragover event now creates a new DropIndicator object literal ({ targetId: itemId, position, folderId: null }) and passes it directly to setDropIndicator. Because each call produces a fresh object reference, React will schedule a re-render on every mouse-move event — even when the indicator value hasn't changed. setNormalizedDropIndicator avoids this via its equality check (prev?.targetId === next.targetId && ...).

   Consider routing through setNormalizedDropIndicator (or adding an equivalent guard here) to bail out when the indicator hasn't actually changed:

   Note: If setNormalizedDropIndicator is used here, the folderId: null edge case means the 'after' → 'before' normalisation won't fire for an edge that is already at a null-folder scope, which matches the previous direct-set behaviour.

_{Reviews (1): Last reviewed commit: "fix(reorder): drag and drop hook" | Re-trigger Greptile}

[icecrasher321]

bugbot run

[icecrasher321]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

[cursor]

Ref update inside deferred updater defeats synchronous guarantee

High Severity

dropIndicatorRef.current is assigned inside the setDropIndicator updater callback, but in React 18 updater functions are deferred (not called synchronously). This means the ref is not updated synchronously during dragOver, so when handleDrop reads dropIndicatorRef.current, it can still hold a stale value — the exact race condition the ref was introduced to prevent (per the comment at lines 45-48). The next value is already computed synchronously before the setDropIndicator call; dropIndicatorRef.current = next needs to be set before entering the updater, similar to how createEdgeDropZone correctly does it.

Additional Locations (1)

• apps/sim/app/workspace/[workspaceId]/w/components/sidebar/hooks/use-drag-drop.ts#L458-L459