Repository navigation
Conversation
📝 WalkthroughWalkthroughAdds configurable actions to move the active surface to previous, next, or adjacent panes. Movement is exposed through keyboard shortcuts, the command palette, and the View menu, with directional splits created when needed and tests covering movement, focus, layout restrictions, and routing. ChangesSurface movement
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant AppDelegate
participant TabManager
participant Workspace
participant Bonsplit
User->>AppDelegate: Trigger surface-to-pane shortcut
AppDelegate->>TabManager: Request surface movement
TabManager->>Workspace: Forward movement request
Workspace->>Bonsplit: Move surface or create directional split
Bonsplit-->>Workspace: Movement result
Workspace-->>TabManager: Return success or failure
TabManager-->>AppDelegate: Report result
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (5 errors, 1 warning)
✅ Passed checks (19 passed)
✨ Finishing Touches🧪 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 |
This was referenced Jul 24, 2026
kyl33r
marked this pull request as ready for review
July 24, 2026 10:20
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Contributor
Greptile SummaryThis PR adds six new actions — "Move Surface to Previous/Next Pane" and "Move Surface to Pane Left/Right/Up/Down" — to the existing surface-navigation system, delegating to
Confidence Score: 5/5Safe to merge — all six new pane-move actions are unbound by default, both code paths (existing-pane move and split-creation) are guarded against Canvas layout and remote-tmux mirrors, and the test suite covers the full directional matrix. The implementation is narrowly scoped: two new workspace methods plus thin wiring through TabManager, the shortcut handler, Command Palette, and View menu. Canvas and remote-tmux guards are present at the entry point in each path. The 50/50 split-creation path reuses Bonsplit's movingTab split API, which already handles source-pane repair. Localization is complete for both supported locales. No new ambient globals, no blocking primitives, and no test seams were introduced in production source. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Shortcut / Menu / Command Palette"] --> B["TabManager.moveSelectedSurfaceToAdjacentPane or moveSelectedSurfaceToPane"]
B --> C["Workspace methods"]
C --> D{Layout mode?}
D -- Canvas --> E["return false (no-op)"]
D -- Remote tmux --> E
D -- Normal --> F{Adjacent pane exists?}
F -- Yes --> G["moveSurface to target pane"]
F -- No --> H["bonsplitController.splitPane 50/50"]
H --> I["focusPane + selectTab + focusPanel"]
G --> J["Success: return true"]
I --> J
Reviews (2): Last reviewed commit: "Merge branch 'manaflow-ai:main' into fea..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@cmuxTests/MoveSurfaceBetweenPanesTests.swift`:
- Around line 308-334: Extend MoveSurfaceBetweenPanesTests with a test covering
performSurfacePaneMoveShortcut when the Dock owns keyboard focus: trigger a
pane-move keypress while Dock focus is active, then assert the main-panel pane
tree remains unchanged. Reuse the existing test setup and pane-tree comparison
patterns used for canvas-layout and remote-tmux-mirror guard coverage.
In `@Resources/Localizable.xcstrings`:
- Around line 196270-196276: Update the affected localization entries and their
source labels to “Move Surface Left” and “Move Surface Right” rather than
“Reorder Surface Left/Right.” Restore the corresponding Japanese translations to
describe moving the surface between panes, keeping the labels aligned with the
existing pane-movement action semantics.
In `@Sources/ContentView.swift`:
- Around line 8506-8541: Route all pane-movement handlers through the shared
Dock-aware, window-routed action instead of directly calling tab managers.
Update the six palette handlers in Sources/ContentView.swift lines 8506-8541 and
the corresponding menu handlers in Sources/cmuxApp.swift lines 935-964 to invoke
that same action, preserving failure feedback while preventing movement when
Dock owns focus.
🪄 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 Plus
Run ID: c8d9f9de-b3e8-49c2-b641-ff754d03a0c4
📒 Files selected for processing (13)
Packages/macOS/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction+Defaults.swiftPackages/macOS/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction.swiftResources/Localizable.xcstringsSources/AppDelegate+AdjacentNavigationShortcut.swiftSources/ContentView+RightSidebarCommandPalette.swiftSources/ContentView.swiftSources/KeyboardShortcutSettings.swiftSources/TabManager.swiftSources/Workspace+SurfaceNavigation.swiftSources/Workspace.swiftSources/cmuxApp.swiftcmux.xcodeproj/project.pbxprojcmuxTests/MoveSurfaceBetweenPanesTests.swift
Comment on lines
+308
to
+334
| @Test func shortcutAndCommandPaletteMetadataIsCompleteAndUnambiguous() throws { | ||
| let paneMoveActions: [KeyboardShortcutSettings.Action] = [ | ||
| .moveSurfaceToPreviousPane, | ||
| .moveSurfaceToNextPane, | ||
| .moveSurfaceToPaneLeft, | ||
| .moveSurfaceToPaneRight, | ||
| .moveSurfaceToPaneUp, | ||
| .moveSurfaceToPaneDown, | ||
| ] | ||
|
|
||
| for action in paneMoveActions { | ||
| #expect(KeyboardShortcutSettings.publicShortcutActions.contains(action)) | ||
| #expect(KeyboardShortcutSettings.settingsVisibleActions.contains(action)) | ||
| let settingsAction = try #require(ShortcutAction(rawValue: action.rawValue)) | ||
| #expect(settingsAction.displayName == action.label) | ||
| #expect(settingsAction.defaultStroke == nil) | ||
| #expect(action.defaultShortcut == .unbound) | ||
| } | ||
|
|
||
| #expect(ContentView.commandPaletteShortcutAction(forCommandID: "palette.moveSurfaceToPreviousPane") == .moveSurfaceToPreviousPane) | ||
| #expect(ContentView.commandPaletteShortcutAction(forCommandID: "palette.moveSurfaceToNextPane") == .moveSurfaceToNextPane) | ||
| #expect(ContentView.commandPaletteShortcutAction(forCommandID: "palette.moveSurfaceToPaneLeft") == .moveSurfaceToPaneLeft) | ||
| #expect(ContentView.commandPaletteShortcutAction(forCommandID: "palette.moveSurfaceToPaneRight") == .moveSurfaceToPaneRight) | ||
| #expect(ContentView.commandPaletteShortcutAction(forCommandID: "palette.moveSurfaceToPaneUp") == .moveSurfaceToPaneUp) | ||
| #expect(ContentView.commandPaletteShortcutAction(forCommandID: "palette.moveSurfaceToPaneDown") == .moveSurfaceToPaneDown) | ||
| } | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether the Dock-focus guard for pane-move shortcuts has dedicated test coverage elsewhere.
rg -n "performFocusedDockShortcut" -C3 cmuxTests
rg -n "moveSurfaceToPane|moveSurfaceToPreviousPane|moveSurfaceToNextPane" cmuxTests -g '!MoveSurfaceBetweenPanesTests.swift'Repository: manaflow-ai/cmux
Length of output: 154
🏁 Script executed:
#!/bin/bash
set -u
echo "Repository files matching relevant names:"
fd -a 'MoveSurfaceBetweenPanesTests|AdjacentNavigationShortcut|MoveSurface|PaneMove|DashboardWindow' . | sed 's#^\./##' | head -200
echo
echo "Search for performFocusedDockShortcut outside cmuxTests:"
rg -n "performFocusedDockShortcut|focusedDockShortcut|Dock.own|keyboard focus|focus.*Dock|surface.moveToPane|moveSurfaceToPane|moveSurfaceToPreviousPane" -S . --glob '!cmuxTests/**' --glob '!web/**' --glob '!Packages/**/Tests/**' | head -300
echo
echo "Search relevant tests in cmuxTests:"
rg -n "moveSurfaceToPane|moveSurfaceToPreviousPane|moveSurfaceToNextPane|performFocusedDockShortcut|focusedDockShortcut|Dock" cmuxTests -C 3 --glob '*.swift' | head -300Repository: manaflow-ai/cmux
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -u
echo "Relevant Swift test/source files (limit names):"
git ls-files | rg '(^|/)(cmuxTests/MoveSurfaceBetweenPanesTests\.swift|AppDelegate\+AdjacentNavigationShortcut\.swift|Move|Pane|Surface|Shortcut).*\.swift$' | head -300
echo
echo "Targeted Dock/pane-move references outside huge binary assets:"
rg -n "performFocusedDockShortcut|focusedDockShortcut|dockFocus|Dock.*focus|moveSurfaceToPane|moveSurfaceToPreviousPane|moveSurfaceToNextPane|move_surface_to|AdjacentNavigationShortcut|move surface" -S \
--glob '!cmuxTests/**' --glob '!Packages/**/Tests/**' --glob '!web/**' \
--glob '*.swift' --glob '*.json' --glob '*.md' --glob '*.xcstrings' \
. | head -300
echo
echo "Test references outside MoveSurfaceBetweenPanesTests.swift:"
rg -n "performFocusedDockShortcut|focusedDockShortcut|Dock.*focus|moveSurfaceToPane|moveSurface to pane|AdjacentNavigationShortcut|moveSurfaceToPreviousPane|moveSurfaceToNextPane" cmuxTests -S --glob '*.swift' | head -300
echo
echo "Search only test function/comments for Dock:"
rg -n "function .*dock|Dock|dock|performFocusedDockShortcut|keyboard focus|background workspace|move background|unbound" cmuxTests -S --glob '*.swift' | head -300Repository: manaflow-ai/cmux
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -u
echo "Inspect AppDelegate+AdjacentNavigationShortcut.swift:"
cat -n Sources/AppDelegate+AdjacentNavigationShortcut.swift | sed -n '1,120p'
echo
echo "Inspect DockShortcutRouting implementation:"
cat -n Sources/AppDelegate+DockShortcutRouting.swift | sed -n '1,150p'
echo
echo "Locate MoveSurfaceBetweenPanesTests around guards:"
cat -n cmuxTests/MoveSurfaceBetweenPanesTests.swift | sed -n '240,360p'
echo
echo "Focused Dock shortcut tests in MoveSurfaceBetweenPanesTests.swift:"
rg -n "focusedDock|Dock.*focus|background|unbound|handleAdjacentNavigationShortcut|moveSurface" cmuxTests/MoveSurfaceBetweenPanesTests.swift -C 4
echo
echo "Focused Dock shortcut tests elsewhere in cmuxTests:"
rg -n "focusedDock|Dock.*focus|background|unbound|handleAdjacentNavigationShortcut|panes move|moveSurfaceToPane|moveSurfaceToPreviousPane|moveSurfaceToNextPane|moveSurface" cmuxTests -S --glob '*.swift' | head -400Repository: manaflow-ai/cmux
Length of output: 50372
Add coverage for the Dock-focus pane-move guard.
performSurfacePaneMoveShortcut guards Dock focus before every pane-move shortcut, but the test suite only exercises canvas-layout and remote-tmux-mirror guards. Add a test that a pane move keypress leaves the main-panel pane tree unchanged when the Dock owns keyboard focus.
🤖 Prompt for 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.
In `@cmuxTests/MoveSurfaceBetweenPanesTests.swift` around lines 308 - 334, Extend
MoveSurfaceBetweenPanesTests with a test covering performSurfacePaneMoveShortcut
when the Dock owns keyboard focus: trigger a pane-move keypress while Dock focus
is active, then assert the main-panel pane tree remains unchanged. Reuse the
existing test setup and pane-tree comparison patterns used for canvas-layout and
remote-tmux-mirror guard coverage.
Comment on lines
+196270
to
+196276
| "value": "Reorder Surface Left" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "サーフェスを左へ移動" | ||
| "value": "サーフェスを左へ並べ替え" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep the labels aligned with pane movement.
The PR objective and neighboring keys describe moving the active surface to another pane, but these labels now say “Reorder Surface,” which implies rearranging surfaces within the current pane. Restore “Move Surface Left/Right” and the corresponding Japanese translations unless the underlying actions intentionally have different semantics.
Suggested fix
- "value": "Reorder Surface Left"
+ "value": "Move Surface Left"
- "value": "サーフェスを左へ並べ替え"
+ "value": "サーフェスを左へ移動"
- "value": "Reorder Surface Right"
+ "value": "Move Surface Right"
- "value": "サーフェスを右へ並べ替え"
+ "value": "サーフェスを右へ移動"Also applies to: 196287-196293
🤖 Prompt for 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.
In `@Resources/Localizable.xcstrings` around lines 196270 - 196276, Update the
affected localization entries and their source labels to “Move Surface Left” and
“Move Surface Right” rather than “Reorder Surface Left/Right.” Restore the
corresponding Japanese translations to describe moving the surface between
panes, keeping the labels aligned with the existing pane-movement action
semantics.
Comment on lines
+8506
to
+8541
| registry.register(commandId: "palette.moveSurfaceToPreviousPane") { | ||
| guard tabManager.moveSelectedSurfaceToPane(offset: -1) else { | ||
| NSSound.beep() | ||
| return | ||
| } | ||
| } | ||
| registry.register(commandId: "palette.moveSurfaceToNextPane") { | ||
| guard tabManager.moveSelectedSurfaceToPane(offset: 1) else { | ||
| NSSound.beep() | ||
| return | ||
| } | ||
| } | ||
| registry.register(commandId: "palette.moveSurfaceToPaneLeft") { | ||
| guard tabManager.moveSelectedSurfaceToAdjacentPane(.left) else { | ||
| NSSound.beep() | ||
| return | ||
| } | ||
| } | ||
| registry.register(commandId: "palette.moveSurfaceToPaneRight") { | ||
| guard tabManager.moveSelectedSurfaceToAdjacentPane(.right) else { | ||
| NSSound.beep() | ||
| return | ||
| } | ||
| } | ||
| registry.register(commandId: "palette.moveSurfaceToPaneUp") { | ||
| guard tabManager.moveSelectedSurfaceToAdjacentPane(.up) else { | ||
| NSSound.beep() | ||
| return | ||
| } | ||
| } | ||
| registry.register(commandId: "palette.moveSurfaceToPaneDown") { | ||
| guard tabManager.moveSelectedSurfaceToAdjacentPane(.down) else { | ||
| NSSound.beep() | ||
| return | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Route pane moves through one Dock-aware action path.
The shortcut path blocks pane movement when Dock owns focus, but these palette and menu handlers directly mutate the active workspace. Invoking either while Dock is active can move a background surface. Centralize the operation behind the same Dock-aware, window-routed action.
Sources/ContentView.swift#L8506-L8541: replace directtabManagermoves with the shared action.Sources/cmuxApp.swift#L935-L964: invoke that same shared action rather thanactiveTabManagerdirectly.
As per coding guidelines, “Do not wire the same behavior separately through multiple surfaces; use one shared action path.” The PR objective also requires avoiding movement while Dock owns keyboard focus.
📍 Affects 2 files
Sources/ContentView.swift#L8506-L8541(this comment)Sources/cmuxApp.swift#L935-L964
🤖 Prompt for 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.
In `@Sources/ContentView.swift` around lines 8506 - 8541, Route all pane-movement
handlers through the shared Dock-aware, window-routed action instead of directly
calling tab managers. Update the six palette handlers in
Sources/ContentView.swift lines 8506-8541 and the corresponding menu handlers in
Sources/cmuxApp.swift lines 935-964 to invoke that same action, preserving
failure feedback while preventing movement when Dock owns focus.
Source: Coding guidelines
Contributor
Author
|
need more attention to this so that this can be released as soon as possible |
…ShortcutAction.swift add missing doc comments Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
This was referenced Jul 31, 2026
Collaborator
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
2 participants
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
Behavior
Verification
./scripts/test-unit.sh -skipPackageUpdates test -only-testing:cmuxTests/MoveSurfaceBetweenPanesTests** TEST SUCCEEDED **./scripts/reload.sh --tag move-pane --launchCMUX_TAG=move-pane scripts/cmux-debug-cli.sh workspace list.git diff --check, Swift parser validation, PBX validation, and test-wiring lint passed.Related issue
Supersedes #8751, which GitHub closed automatically when its head branch was renamed to include the issue number.
Closes #8752
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Adds actions to move the active surface between panes. Reuses adjacent panes or creates a 50/50 split when missing, while preserving identity, focus, and pane extents. Six unbound actions are available via shortcuts, Command Palette, and the View menu; closes #8752.
New Features
Bonsplitadjacency; if none, split the source pane 50/50 in that direction and move the live surface.Bug Fixes
Written for commit a8381b2. Summary will update on new commits.
Summary by CodeRabbit