Repository navigation
Drag a workspace into another window's sidebar to move it - #5399
Merged
Merged
Conversation
Today a sidebar workspace drag can only reorder within its own window; moving to another window required the right-click "Move Workspace to Window" menu. The drop delegate derived the dragged identity solely from per-window SidebarDragState.draggedTabId, which is only set in the window where the drag began — so a drop whose source was a different window had no identity to act on and was silently rejected. Unify the sidebar drop path on a single source of truth for the dragged workspace identity: SidebarDragState.draggedTabId when present, else the drag pasteboard payload (SidebarTabDragPayload.currentDraggedWorkspaceId), which is authoritative in any window. SidebarTabDropDelegate now keys one branch on whether the dragged workspace is already in this window: - In this window -> existing reorder (unchanged). - Another window -> cross-window move at the drop index, mirroring the existing "Move Workspace to Window" action. A foreign hover mirrors its identity into the destination's drag state so the existing drop-indicator / frame-anchor / failsafe machinery (all gated on draggedTabId != nil) activate unchanged; the id matches no local row so nothing dims, and the mouse-up failsafe clears it. A new pure planner SidebarDropPlanner.crossWindowInsertion computes the insertion index and indicator when the dragged workspace is not in the list. The group-header drop surface inherits cross-window support for free via the reorder delegate it wraps. Edge cases: dropping back on the source window is a plain reorder; dropping on the empty area appends; multi-selected workspaces move together (resolved from the source window's selection); detaching the last workspace leaves the source window with a fresh workspace. Tests: SidebarDropPlanner.crossWindowInsertion (edges, append, pinned- region clamping, indicator replay) and the detach/attach-at-index move core across two TabManagers. The drag gesture / drop targeting itself is UI-only and is exercised by dogfood, not unit tests. Closes #5395 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds a process-wide sidebar drag registry, computes legal cross-window insertion indices that respect pinned/unpinned partitions, forwards an optional insertion index through AppDelegate.moveWorkspaceToWindow to TabManager.attachWorkspace, mirrors foreign drags into local state for indicators, and adds planner and move tests. ChangesCross-window workspace drag-and-drop
Sequence Diagram(s)sequenceDiagram
participant User
participant ContentView
participant SidebarDropPlanner
participant AppDelegate
participant TabManagerSource
participant TabManagerDest
User->>ContentView: Drag workspace from source window
ContentView->>ContentView: Resolve effectiveDraggedTabId (local or registry)
ContentView->>SidebarDropPlanner: crossWindowInsertion(...)
SidebarDropPlanner-->>ContentView: insertionIndex, indicator
ContentView->>AppDelegate: moveWorkspaceToWindow(workspaceId, destWindowId, atIndex: insertionIndex)
AppDelegate->>TabManagerSource: detachWorkspace(workspaceId)
AppDelegate->>TabManagerDest: attachWorkspace(workspace, at: insertionIndex)
TabManagerDest-->>AppDelegate: confirm attach
AppDelegate-->>ContentView: success
ContentView->>ContentView: update selection / dragState
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (4 errors, 1 warning)
✅ Passed checks (13 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
Greptile SummaryImplements cross-window sidebar drag-and-drop: dragging a workspace from one window's sidebar into another moves it to the drop position, with multi-select, pinned/unpinned region clamping, and group-contiguity normalization all handled. The unified drop path resolves drag identity from a process-wide
Confidence Score: 5/5Safe to merge; well-tested cross-window drop path with no data-integrity issues identified. The core move primitives (detach/attach, normalizeWorkspaceGroupContiguity, atIndex propagation) are all covered by unit tests and the same-manager short-circuit is verified in the source. The two open items are non-blocking quality improvements that don't affect workspace data integrity. Sources/ContentView.swift — the validateDrop anchor check and dropExited cleanup are the two spots worth a follow-up pass. Important Files Changed
Sequence DiagramsequenceDiagram
participant A as Source Window (SidebarDragState A)
participant Reg as SidebarWorkspaceDragRegistry
participant B as Destination Window (SidebarDragState B)
participant AD as AppDelegate
participant TM as TabManager (dest)
A->>Reg: begin(workspaceId:) [drag start]
Note over A,Reg: originatedActiveDrag = true
B->>Reg: currentWorkspaceId (validateDrop / dropEntered)
B->>B: activateForeignDragIfNeeded()
Note over B: draggedTabId = foreignId, foreignDraggedIsPinned cached
B->>B: updateCrossWindowDropIndicator() [dropUpdated]
B->>AD: performCrossWindowDrop(draggedTabId:)
AD->>AD: moveWorkspaceToWindow(atIndex:) per tier
AD->>TM: detachWorkspace(source) + attachWorkspace(at:)
TM->>TM: normalizeWorkspaceGroupContiguity()
AD->>AD: moveWorkspaceToWindow(focus:true) — same-manager → focus only
A->>Reg: end(workspaceId:) [clearDrag on mouse-up]
B->>B: clearDrag() [failsafe mouse-up]
Note over B: draggedTabId = nil, foreignDraggedIsPinned = nil
Reviews (8): Last reviewed commit: "Preserve source order in cross-window mu..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+17757
to
+17768
| var didMove = false | ||
| for (offset, workspaceId) in movingIds.enumerated() { | ||
| let isLast = offset == movingIds.count - 1 | ||
| if app.moveWorkspaceToWindow( | ||
| workspaceId: workspaceId, | ||
| windowId: destinationWindowId, | ||
| atIndex: insertionIndex + offset, | ||
| focus: isLast | ||
| ) { | ||
| didMove = true | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
atIndex: insertionIndex + offset uses the enumeration index, not a counter of successfully-inserted workspaces. If any moveWorkspaceToWindow call in the batch returns false (e.g., workspace disappeared between list construction and the loop), the next workspace attempts insertion at insertionIndex + (N+1) when the destination has only grown by N-1, leaving a gap. The same stale IDs also end up in selectedTabIds regardless of which moves succeeded. While the failure path is unreachable under normal @mainactor execution, tracking successful-move count is more defensive and matches the invariant that each successful insertion shifts all later target indices by 1.
Contributor
Author
There was a problem hiding this comment.
Fixed in 1225cf0 — the loop now offsets each insert by the count of successful moves (atIndex: rawInsertIndex + movedIds.count) and only the workspaces that actually moved become the destination selection, so a skipped move can't leave an index gap or stale selection.
— Claude Code
Comment on lines
+17815
to
+17818
| private func updateCrossWindowDropIndicator(for info: DropInfo, draggedTabId: UUID) { | ||
| let draggedIsPinned = AppDelegate.shared? | ||
| .tabManagerFor(tabId: draggedTabId)? | ||
| .tabs.first { $0.id == draggedTabId }?.isPinned ?? false |
Contributor
There was a problem hiding this comment.
tabManagerFor lookup on every pointer-move event
draggedIsPinned is re-resolved from AppDelegate.shared?.tabManagerFor(tabId:)?.tabs.first { ... } on each call to updateCrossWindowDropIndicator, which fires for every dropUpdated (every pointer-move during the hover). tabManagerFor scans all open windows' tab managers — O(windows × workspaces) per event. The isPinned status of the dragged workspace cannot change mid-drag, so the result could be resolved once (e.g., in activateForeignDragIfNeeded and stashed, or cached as a local in dropUpdated) rather than re-fetched on each pointer-move event.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Contributor
Author
There was a problem hiding this comment.
Fixed in 1225cf0 — the foreign workspace's pin state is now resolved once when the drag is mirrored into the destination (stashed on SidebarDragState.foreignDraggedIsPinned) and reused for every hover, so there's no per-pointer-move tabManagerFor scan across windows.
— Claude Code
Address codex review: the cross-window drop inserted a foreign workspace at a raw tabs index, which could split a destination group's contiguous run or drop a pinned workspace below unpinned ones. Fix at the ownership boundary: TabManager.attachWorkspace now re-runs normalizeWorkspaceGroupContiguity() after insert — the same normalization addWorkspace uses — so every cross-window attach (this drag path and the CLI move-workspace-to-window path, which shared the latent bug) keeps the leading-pinned segment and contiguous group runs intact regardless of the requested drop index. This also makes per-pin-tier correctness for a mixed multi-selection fall out for free, since each attach normalizes by the moved workspace's own pin state. Also clamp the cross-window drop indicator for a pinned workspace dragged into a window with no existing pins to the front (index 0) instead of the raw pointer position, so the indicator matches where the workspace lands. Tests: pinned-into-no-pins planner clamp; pinned workspace lands at front even when dropped below unpinned rows; moving into the middle of a group run keeps the group contiguous. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/ContentView.swift`:
- Around line 17815-17827: The updateCrossWindowDropIndicator hot path rebuilds
tabManager.tabs snapshots every hover; fix by capturing a single snapshot of tab
IDs and pinned IDs at drag start and reusing it in
updateCrossWindowDropIndicator (and the eventual commit path) instead of calling
tabManager.tabs.map/filter each event. Add cached properties (e.g.,
dragSessionTabIds: [UUID] and dragSessionPinnedTabIds: Set<UUID>) that are
populated when the drag begins and cleared when the drag ends, then pass those
cached values into SidebarDropPlanner.crossWindowInsertion (replace
tabManager.tabs.map(\\.id) and
Set(tabManager.tabs.filter(\\.isPinned).map(\\.id)) with the cached variables).
Ensure the cache is updated/invalidated in the drag start/finish handlers so
other flows still see fresh data.
- Around line 17722-17775: performCrossWindowDrop currently always calls
AppDelegate.moveWorkspaceToWindow(...atIndex:) which flattens cross-window drops
and loses group/section membership; change it to resolve the destination
section/header for the foreign drop (using the same logic/path used by
SidebarTabDropDelegate.performDrop(info:)) and funnel moves through the
section-aware APIs (e.g. moveWorkspaceToSection(...) or
removeWorkspaceFromSection(...) when appropriate) instead of always using
moveWorkspaceToWindow. Ensure you compute the correct target section and
insertion index within that section (preserve draggedIsPinned and focus behavior
for the last moved item) and update selectedTabIds/syncSidebarSelection exactly
as before when didMove is true.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 78e6641d-e43f-472d-af6a-5dec30ea8c76
📒 Files selected for processing (5)
Sources/AppDelegate.swiftSources/ContentView.swiftSources/Sidebar/SidebarDropPlanner.swiftcmuxTests/SidebarOrderingTests.swiftcmuxTests/TabManagerUnitTests.swift
Address codex review (groups): - A group-header drag carries the group's anchor id. Moving only the anchor across windows would dissolve the source group and strand its members. Reject cross-window drops whose dragged workspace is a source group anchor (and drop any anchors from a multi-selection), so the group stays intact in its window — validateDrop returns false, no destination indicator is shown, and the drag snaps back. Members can still be dragged out individually. Full cross-window group migration is out of scope. - The destination drop planner and hover indicator now reason in the destination's top-level id space (group anchors represent their groups; a hovered group member maps to its anchor), matching the group/pin boundaries `attachWorkspace` normalizes incoming workspaces to. The top-level insertion slot is translated back to a raw `tabs` index for the attach, so the indicator the user sees matches where the workspace lands instead of pointing inside a protected group region. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/ContentView.swift`:
- Around line 17806-17814: Replace the compactMap use that returns nil when a
tab isn't selected with an idiomatic filter + map pipeline: instead of
sourceManager.tabs.compactMap { sourceSelection.contains($0.id) ? $0.id : nil }
call sourceManager.tabs.filter { sourceSelection.contains($0.id) }.map { $0.id }
so the intent for candidateIds (when computing selected tab IDs) is clearer;
update the branch that assigns candidateIds (and keep the rest using
sourceAnchorIds and movingIds unchanged).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: df435e45-9b41-4f41-87f7-a039056f379f
📒 Files selected for processing (1)
Sources/ContentView.swift
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 6 files
You’re at about 98% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
…eboard Address codex review: the cross-window path resolved the dragged workspace id by reading NSPasteboard(name: .drag) synchronously. SwiftUI's .onDrag registers the payload through an NSItemProvider whose data representation is delivered asynchronously (a Task), so a synchronous read inside a DropDelegate can race and return nil — which would make validateDrop reject a valid cross-window drop even though the drag type is present. Replace the pasteboard read with SidebarWorkspaceDragRegistry: a process- wide, main-actor-isolated record of the one workspace being sidebar-dragged. The originating window sets it synchronously at drag start (beginDragging) and clears it when its own drag ends; only the originating SidebarDragState owns the entry (a destination window mirroring a foreign id for rendering does not clear it). A destination drop delegate reads currentWorkspaceId to resolve the dragged workspace with no materialization race. Drop-type detection still uses the reliably-advertised UTType via hasItemsConforming. The unused SidebarTabDragPayload pasteboard reader is removed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Multi-move now offsets each insert by the count of *successful* moves (not the enumeration index) and selects only the workspaces that actually moved, so a skipped move can't leave an index gap or stale selection (Greptile). - Resolve the foreign dragged workspace's pin state once when the drag is mirrored into the destination (stashed on SidebarDragState) instead of scanning every window's tab manager on each pointer-move hover (Greptile). - Use filter + map instead of compactMap-with-nil for the selected ids (CodeRabbit nit). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/ContentView.swift`:
- Around line 17874-17885: The multi-select move uses a single rawInsertIndex
and draggedIsPinned for all movingIds (see movingIds loop and
app.moveWorkspaceToWindow) which breaks when selection mixes pinned and unpinned
items because attachWorkspace(_:at:) re-normalizes placement; update the logic
that plans and performs the move (the planning at the draggedIsPinned/insert
calculation and the loop that calls app.moveWorkspaceToWindow) to detect mixed
pin-state and either reject/disable cross-window multi-moves in that case or
split movingIds into pinned and unpinned groups and compute/apply separate
insert indices for each group so each subgroup is inserted relative to its
pinned section (ensuring the preview indicator and final placement match).
Ensure you reference draggedIsPinned, rawInsertIndex, movingIds,
attachWorkspace(_:at:), and app.moveWorkspaceToWindow when making the change so
both planning (17935-17941) and execution (loop 17874-17885) are updated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 62b75aaa-8fe7-4497-9c7e-75cc3eb0d63a
📒 Files selected for processing (1)
Sources/ContentView.swift
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
You’re at about 99% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Address review (cubic + CodeRabbit P2): a cross-window multi-selection that spans pinned and unpinned workspaces was planned from the drag initiator's single pin bit and applied to every workspace, so attachWorkspace's per-insert pinned/unpinned normalization could split the moved set away from the drop indicator. Plan each moved workspace by its own pin state against the live destination: the hover indicator still anchors the slot, the per-iteration recompute keeps each pin tier a contiguous block in source order (and is inherently safe against a skipped move — a failed insert simply doesn't grow the destination, so no index gap or stale selection), and the last successfully-moved workspace is focused via the same-manager focus path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
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.
Reviewed by Cursor Bugbot for commit c8d9de5. Configure here.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
You’re at about 99% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Address codex review: the extension / browser-stack sidebar drop delegates end a drag by writing nil through draggedTabIdBinding, which set dragState.draggedTabId directly and bypassed clearDrag(). Since beginDragging now records the drag in the process-wide SidebarWorkspaceDragRegistry, those drops left currentWorkspaceId stale, so a later cross-window drop in another window could resolve and move the wrong workspace. Route the binding's nil-clear through clearDrag() so any locally originated drag also ends its registry entry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…set) Address cubic review: recomputing the insertion index per workspace from the same fixed indicator re-anchored every insert to the hovered row, so for a "drop after row X" (bottom edge) the batch landed in reverse order. The prior single-base `+ movedIds.count` offset preserved order but planned the whole batch from one pin bit (mixed-tier scatter). Combine both: plan one base slot per pin tier (recomputed against the live destination so a later tier sees the earlier tier's inserts), then insert that tier's workspaces at base + running-offset. Same-tier workspaces stay a contiguous block in source order, mixed pinned/unpinned selections each land in their own region, and a skipped move doesn't advance the offset. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai review All previously-flagged findings have been addressed on the current HEAD (38f486b) and replied to inline: pinned/group ordering invariants, group-anchor handling, group-aware top-level indices, the NSPasteboard→registry change, the extension-sidebar registry-leak, mixed pin-tier multi-select, and the multi-move source-order fix. CI is green. Please re-review the current commit and clear the stale changes-requested if it's resolved. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get the PR details and latest diff
gh pr view 5399 --repo manaflow-ai/cmux --json state,headRefName,headRefOid,baseRefName
echo "---"
# Get current files changed
gh pr diff 5399 --repo manaflow-ai/cmux 2>/dev/null | head -3000Length of output: 38749 🏁 Script executed: #!/bin/bash
# Fetch the full moveWorkspaceToWindow function from AppDelegate
ast-grep --pattern 'func moveWorkspaceToWindow(workspaceId: UUID, windowId: UUID, $$$) -> Bool {
$$$
}'Length of output: 425 🏁 Script executed: #!/bin/bash
# Find the full moveWorkspaceToWindow function body
rg -n "moveWorkspaceToWindow" Sources/AppDelegate.swift -A 40 | head -120Length of output: 3602
One concern I had — calling Tests cover all the critical invariants (pin clamping, group contiguity, last-workspace detach, indicator replay). CI green. Approving. [approve] 🧠 Learnings used |
Contributor
Author
|
@coderabbitai full review Note: your last review only covered up to commit 1225cf0. The single actionable finding there (mixed pin-state multi-select at the cross-window move, ContentView.swift ~17874-17885) was fixed in commit 38f486b — the move now plans one base slot per pin tier and inserts each tier at base + running offset, so mixed pinned/unpinned selections land in their correct regions and same-tier order is preserved. Please re-review HEAD (38f486b); the prior changes-requested is stale. |
|
Re-review request from 🧠 Learnings used✅ Action performedFull review finished. |
Merged
This was referenced Jun 9, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements #5395: drag a workspace row out of one cmux window's sidebar and drop it into another window's sidebar to move it there (same effect as the existing right-click "Move Workspace to Window"), inserted at the drop position.
Previously a sidebar workspace drag could only reorder within its own window. The drop delegate derived the dragged identity solely from per-window
SidebarDragState.draggedTabId, which is only set in the window where the drag began — so a drop whose source was a different window had no identity to act on and was silently rejected.Unified drop path (one source of truth)
Rather than fork a parallel cross-window drop delegate, the sidebar drop path now resolves the dragged workspace identity from a single source of truth:
SidebarDragState.draggedTabIdwhen present (intra-window), elseSidebarTabDragPayload.currentDraggedWorkspaceId()— read synchronously from the drag pasteboard, authoritative in any window.SidebarTabDropDelegatekeys one branch on whether the dragged workspace is already in this window:AppDelegate.moveWorkspaceToWindow(now with anatIndex:parameter).A foreign hover mirrors its identity into the destination window's
SidebarDragStateso the existing drop-indicator / frame-anchor / failsafe machinery (all gated ondraggedTabId != nil) activate unchanged — the id matches no local row so nothing dims, and the mouse-up failsafe clears it. A new pureSidebarDropPlanner.crossWindowInsertion(...)computes the insertion index + indicator for the case where the dragged workspace is not in the list (no source index to remove). The group-header drop surface inherits cross-window support for free via the reorder delegate it wraps.Edge cases handled
detachWorkspaceleaves the source with a fresh workspace (no crash / broken empty window), same as the existing move action.Tests
SidebarDropPlannerTests:crossWindowInsertion— top/bottom edge, empty-area append, pinned-region clamping (both directions), and drop-time indicator replay (pure, no app launch).CrossWindowWorkspaceMoveTests: the detach/attach-at-index move core across twoTabManagers — workspace lands at the drop index in the destination, source stays consistent, last-workspace detach keeps source non-empty.The drag gesture / drop targeting itself is UI-only (SwiftUI
onDropacross windows) and is verified by dogfood rather than unit tests.Localization
No new user-facing strings introduced (drag/drop is visual; the only added strings are
#if DEBUGdiagnostics). No keyboard shortcut added (drag-driven).🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Touches multi-window tab routing, drag lifecycle, and sidebar ordering invariants (pins/groups); mistakes could mis-route workspaces or leave stale drag state, but behavior is heavily tested and gated.
Overview
Enables moving workspaces between windows by dragging from one sidebar into another, at the drop position (same outcome as “Move Workspace to Window”), instead of only reordering within the originating window.
Drag identity and drop path: A process-wide
SidebarWorkspaceDragRegistryrecords the dragged workspace synchronously (avoiding pasteboard races).SidebarTabDropDelegateresolves the dragged id from local state or the registry, mirrors foreign drags into destinationSidebarDragStatefor existing indicators/failsafes, and branches to cross-window commit vs in-window reorder. Clearing drag state via bindings now callsclearDrag()so the registry cannot go stale.Placement rules:
SidebarDropPlanner.crossWindowInsertionplans insertion and indicators when the dragged workspace is not in the destination list, including pinned/unpinned clamps.AppDelegate.moveWorkspaceToWindowgains optionalatIndex:; drops can move multi-selection (excluding group anchors), plan per pin tier, and focus the grabbed tab. Cross-window drops reject group anchor drags;attachWorkspacere-normalizes group contiguity after indexed inserts.Tests: New coverage for cross-window insertion math and detach/attach move behavior (index, last tab, pinned front, group contiguity).
Reviewed by Cursor Bugbot for commit 38f486b. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Add cross-window drag-and-drop for workspaces. Drag a workspace from one window’s sidebar into another window’s sidebar to move it to the drop position (implements #5395), while keeping pinned-first and group-contiguity rules intact; group headers cannot be moved across windows.
New Features
SidebarDragState.draggedTabId, else process-wideSidebarWorkspaceDragRegistry.currentWorkspaceId(no pasteboard race). Destination mirrors the id and caches the foreign pin state;SidebarDropPlanner.crossWindowInsertion(...)plans in top-level space;AppDelegate.moveWorkspaceToWindow(..., atIndex:)commits at the drop position.Bug Fixes
TabManager.attachWorkspace(at:)re-normalizes to preserve the leading pinned segment and contiguous group runs; clamp a pinned workspace dropped into a window with no pins to index 0 so the indicator matches final placement.nilthrough the drag binding, preventing stale cross-window drag ids.Written for commit 38f486b. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements
Tests