Repository navigation
Handle Cmd+O in handleCustomShortcut to prevent Documents folder open - #2034
Merged
lawrencecchen merged 4 commits intoMar 25, 2026
Merged
Conversation
Cmd+O for "Open Folder" was only handled in SwiftUI menu, which can fail due to focus bugs when terminal is focused. This caused AppKit's default NSDocumentController to open the Documents folder instead. Now Cmd+O is intercepted in handleCustomShortcut like other shortcuts. Fixes manaflow-ai#2010 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@anthhub is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds AppDelegate.showOpenFolderPanel() to present a directory-only NSOpenPanel and routes the Cmd+O / Open Folder command to it; selected directory is used to open or create a workspace in the preferred main window. cmuxApp now invokes AppDelegate for this action. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App as "cmuxApp / Menu"
participant AppDelegate
participant NSOpenPanel
participant WorkspaceMgr as "WorkspaceManager"
User->>App: Presses ⌘O (Open Folder)
App->>AppDelegate: showOpenFolderPanel()
AppDelegate->>NSOpenPanel: present directory-only picker
NSOpenPanel-->>AppDelegate: selected URL / cancel
alt URL selected
AppDelegate->>WorkspaceMgr: openWorkspaceForExternalDirectory(path, debugSource)
alt workspace opened
WorkspaceMgr-->>AppDelegate: workspace (done)
else workspace not opened
AppDelegate->>WorkspaceMgr: addWorkspaceInPreferredMainWindow(workingDirectory)
WorkspaceMgr-->>AppDelegate: created main window / workspace
end
else cancelled
NSOpenPanel-->>AppDelegate: nil (no-op)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
Greptile SummaryThis PR intercepts Key changes:
Minor observation: The doc comment on Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant EventMonitor
participant handleCustomShortcut
participant showOpenFolderPanel
participant NSOpenPanel
participant addWorkspaceInPreferredMainWindow
participant openNewMainWindow
User->>EventMonitor: Press Cmd+O (terminal focused)
EventMonitor->>handleCustomShortcut: event
handleCustomShortcut->>handleCustomShortcut: matchShortcut(.openFolder)?
handleCustomShortcut->>showOpenFolderPanel: call (returns true, swallows event)
Note over handleCustomShortcut: NSDocumentController never sees the event
showOpenFolderPanel->>NSOpenPanel: runModal()
NSOpenPanel-->>showOpenFolderPanel: .OK + url
showOpenFolderPanel->>addWorkspaceInPreferredMainWindow: workingDirectory: url.path
alt workspace added successfully
addWorkspaceInPreferredMainWindow-->>showOpenFolderPanel: UUID (non-nil)
else no main window available
addWorkspaceInPreferredMainWindow-->>showOpenFolderPanel: nil
showOpenFolderPanel->>openNewMainWindow: nil
end
Reviews (1): Last reviewed commit: "Handle Cmd+O in handleCustomShortcut to ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+5366
to
+5383
| /// Shows the "Open Folder" panel and creates a workspace for the selected directory. | ||
| /// Extracted so it can be called from both the SwiftUI menu and `handleCustomShortcut`. | ||
| func showOpenFolderPanel() { | ||
| let panel = NSOpenPanel() | ||
| panel.canChooseFiles = false | ||
| panel.canChooseDirectories = true | ||
| panel.allowsMultipleSelection = false | ||
| panel.title = String(localized: "menu.file.openFolder.panelTitle", defaultValue: "Open Folder") | ||
| panel.prompt = String(localized: "menu.file.openFolder.panelPrompt", defaultValue: "Open") | ||
| if panel.runModal() == .OK, let url = panel.url { | ||
| if addWorkspaceInPreferredMainWindow( | ||
| workingDirectory: url.path, | ||
| debugSource: "shortcut.openFolder" | ||
| ) == nil { | ||
| openNewMainWindow(nil) | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
The doc comment says "Extracted so it can be called from both the SwiftUI menu and handleCustomShortcut", but cmuxApp.swift (lines 589–608) was not updated to call showOpenFolderPanel(). The inline panel setup there is byte-for-byte identical to this new function, so the extraction is only half-done and the docstring is currently inaccurate.
This means there are now two divergent code paths for the same action:
AppDelegate.showOpenFolderPanel()→ called fromhandleCustomShortcut(this PR)- Inline
NSOpenPanelblock incmuxApp.swift→ still called from the SwiftUI menu
A future change to panel titles, localization keys, or the addWorkspaceInPreferredMainWindow fallback would need to be applied in both places. Consider either:
- Updating
cmuxApp.swiftto callAppDelegate.shared?.showOpenFolderPanel()so there is one canonical implementation, or - Updating the docstring to reflect that this function is only for the
handleCustomShortcutpath, e.g. "Shows the 'Open Folder' panel when the shortcut is triggered viahandleCustomShortcut."
Contributor
![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 the current code and only fix it if needed.
Inline comments:
In `@Sources/AppDelegate.swift`:
- Around line 5368-5375: In showOpenFolderPanel, NSOpenPanel is never seeded
with a starting directory so Cmd+O opens in AppKit’s default; before running
panel.runModal() set panel.directoryURL to the currently focused pane/workspace
directory (e.g. get the URL from your focused pane or workspace controller —
e.g. focusedPane.directoryURL or
WorkspaceController.shared.activeWorkingDirectory) so the open panel starts in
the active working directory; keep the fallback to nil if no active directory is
available.
- Around line 5375-5381: When
addWorkspaceInPreferredMainWindow(workingDirectory: url.path, debugSource:
"shortcut.openFolder") returns nil you currently call openNewMainWindow(nil)
which discards the selected url.path; instead preserve and open the chosen
folder by either calling openWorkspaceForExternalDirectory(url.path) or invoking
openNewMainWindow with initialWorkingDirectory: url.path (i.e. replace the
fallback openNewMainWindow(nil) with a call that passes url.path so the selected
folder is opened).
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/AppDelegate.swift">
<violation number="1" location="Sources/AppDelegate.swift:5380">
P2: Open Folder fallback drops the selected directory by opening a blank window when workspace routing returns nil.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Address review feedback: 1. Pass selected directory URL to fallback window creation so the user's folder choice is not silently discarded 2. Replace inline NSOpenPanel code in cmuxApp.swift menu action with a call to AppDelegate.showOpenFolderPanel() to avoid future divergence between the two code paths Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Sources/AppDelegate.swift (1)
5369-5375:⚠️ Potential issue | 🟠 MajorSeed the Open Folder panel with the active directory before showing it.
Line 5375 runs
panel.runModal()without settingpanel.directoryURL, so Cmd+O can still start from AppKit’s default/last-used location instead of the focused workspace/pane directory.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/AppDelegate.swift` around lines 5369 - 5375, Set panel.directoryURL to the currently focused workspace/pane directory before calling panel.runModal(); specifically, before the line that checks panel.runModal() == .OK, compute the active directory from the focused workspace or pane (e.g., the focused workspace/pane’s directory property or the frontmost editor’s file URL’s parent) and assign it to panel.directoryURL so the NSOpenPanel opens seeded to the active directory rather than AppKit’s default. Use the existing identifiers panel, panel.directoryURL and panel.runModal() to locate where to add this assignment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Sources/AppDelegate.swift`:
- Around line 5369-5375: Set panel.directoryURL to the currently focused
workspace/pane directory before calling panel.runModal(); specifically, before
the line that checks panel.runModal() == .OK, compute the active directory from
the focused workspace or pane (e.g., the focused workspace/pane’s directory
property or the frontmost editor’s file URL’s parent) and assign it to
panel.directoryURL so the NSOpenPanel opens seeded to the active directory
rather than AppKit’s default. Use the existing identifiers panel,
panel.directoryURL and panel.runModal() to locate where to add this assignment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3d2c6747-fa3a-44a7-9c4d-6617d953110c
📒 Files selected for processing (2)
Sources/AppDelegate.swiftSources/cmuxApp.swift
Address review feedback: set panel.directoryURL to the focused terminal's working directory so Open Folder starts in a contextually relevant location instead of AppKit's default. Co-Authored-By: Claude Opus 4.6 (1M context) <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 the current code and only fix it if needed.
Inline comments:
In `@Sources/AppDelegate.swift`:
- Around line 5375-5385: The current File → Open Folder flow uses
NSApp.keyWindow directly for panel.directoryURL and then calls
addWorkspaceInPreferredMainWindow(...) which can leave shouldBringToFront false;
change it to reuse the shared main-window resolver and open path logic by: use
contextForMainWindow(...) (the same resolver used elsewhere) to set
panel.directoryURL instead of directly accessing NSApp.keyWindow, and after the
user selects a URL call openWorkspaceForExternalDirectory(workingDirectory:
url.path, debugSource: "shortcut.openFolder") (or the existing helper that
ensures shouldBringToFront is handled the same as other open paths) instead of
addWorkspaceInPreferredMainWindow(...); this keeps behavior consistent with
add/open flows (references: contextForMainWindow, panel.directoryURL,
addWorkspaceInPreferredMainWindow, openWorkspaceForExternalDirectory,
shouldBringToFront).
… in showOpenFolderPanel Address review feedback: use preferredMainWindowContextForWorkspaceCreation for directory seeding (works when auxiliary windows are key) and openWorkspaceForExternalDirectory for workspace creation (ensures shouldBringToFront and consistent fallback behavior).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/AppDelegate.swift`:
- Around line 5382-5386: The code currently marks folder picks as explicit
startup opens too early; only mark them after the user actually confirmed a URL.
Move the call to prepareForExplicitOpenIntentAtStartup() so it runs after you
verify panel.url (i.e., inside the if where panel.runModal() == .OK and let url
= panel.url) and before calling
openWorkspaceForExternalDirectory(workingDirectory:debugSource:); this ensures
openWorkspaceForExternalDirectory can still fall back to
createMainWindow(initialWorkingDirectory:) and that registerMainWindow will not
overwrite the confirmed explicit open with the saved session.
Comment on lines
+5382
to
+5386
| if panel.runModal() == .OK, let url = panel.url { | ||
| openWorkspaceForExternalDirectory( | ||
| workingDirectory: url.path, | ||
| debugSource: "shortcut.openFolder" | ||
| ) |
There was a problem hiding this comment.
Mark confirmed folder picks as explicit startup opens.
If this runs before the initial session-restore attempt and there is no main window yet, openWorkspaceForExternalDirectory(...) can fall back to createMainWindow(initialWorkingDirectory:), and registerMainWindow(...) will still restore the saved session over the selected folder. Call prepareForExplicitOpenIntentAtStartup() only after panel.url is confirmed.
💡 Suggested fix
if panel.runModal() == .OK, let url = panel.url {
+ prepareForExplicitOpenIntentAtStartup()
openWorkspaceForExternalDirectory(
workingDirectory: url.path,
debugSource: "shortcut.openFolder"
)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/AppDelegate.swift` around lines 5382 - 5386, The code currently marks
folder picks as explicit startup opens too early; only mark them after the user
actually confirmed a URL. Move the call to
prepareForExplicitOpenIntentAtStartup() so it runs after you verify panel.url
(i.e., inside the if where panel.runModal() == .OK and let url = panel.url) and
before calling openWorkspaceForExternalDirectory(workingDirectory:debugSource:);
this ensures openWorkspaceForExternalDirectory can still fall back to
createMainWindow(initialWorkingDirectory:) and that registerMainWindow will not
overwrite the confirmed explicit open with the saved session.
Contributor
|
Thank you for the contribution! |
This was referenced Mar 25, 2026
5 tasks
bn-l
pushed a commit
to bn-l/cmux
that referenced
this pull request
Apr 3, 2026
…manaflow-ai#2034) * Handle Cmd+O in handleCustomShortcut to prevent Documents folder open Cmd+O for "Open Folder" was only handled in SwiftUI menu, which can fail due to focus bugs when terminal is focused. This caused AppKit's default NSDocumentController to open the Documents folder instead. Now Cmd+O is intercepted in handleCustomShortcut like other shortcuts. Fixes manaflow-ai#2010 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix fallback directory loss and deduplicate Open Folder logic Address review feedback: 1. Pass selected directory URL to fallback window creation so the user's folder choice is not silently discarded 2. Replace inline NSOpenPanel code in cmuxApp.swift menu action with a call to AppDelegate.showOpenFolderPanel() to avoid future divergence between the two code paths Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Set NSOpenPanel directoryURL to current terminal working directory Address review feedback: set panel.directoryURL to the focused terminal's working directory so Open Folder starts in a contextually relevant location instead of AppKit's default. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Use shared main-window resolver and openWorkspaceForExternalDirectory in showOpenFolderPanel Address review feedback: use preferredMainWindowContextForWorkspaceCreation for directory seeding (works when auxiliary windows are key) and openWorkspaceForExternalDirectory for workspace creation (ensures shouldBringToFront and consistent fallback behavior). --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
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
performKeyEquivalentcan fail due to known focus bugs, causing AppKit's defaultNSDocumentControllerto handle the event and open the Documents folderhandleCustomShortcut(same pattern as Cmd+N, Cmd+W, etc.) to reliably show the Open Folder panelFixes #2010
Test plan
🤖 Generated with Claude Code
Summary by cubic
Handle Cmd+O in
handleCustomShortcutto always show the Open Folder panel, preventingNSDocumentControllerfrom opening the Documents folder when SwiftUI menu dispatch fails with terminal focus. ExtractedshowOpenFolderPanel()and use it from both the menu and shortcut; it seeds the panel with the current workspace’s directory via the shared main‑window resolver and opens the selection withopenWorkspaceForExternalDirectory, ensuring it’s added to a workspace or a new window with that folder.Written for commit 43d8469. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes