Add file explorer sidebar entrypoint - #3976
austinywang wants to merge 12 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds an AppDelegate API to show/focus the right sidebar in a mode, routes command-palette requests to it, adds a sidebar footer "files" button that invokes it (beeps on failure), and covers fallback behavior with unit tests that preserve user defaults. ChangesFiles focus and command routing
Sequence Diagram(s)sequenceDiagram
participant SidebarFileExplorerButton
participant ContentView
participant AppDelegate
participant KeyboardFocusCoordinator
participant FileExplorerState
participant NSSound
SidebarFileExplorerButton->>ContentView: tap -> showFileExplorer()
ContentView->>AppDelegate: showRightSidebarModeInActiveMainWindow(.files, focusFirstItem:true, preferredWindow)
AppDelegate->>KeyboardFocusCoordinator: focusRightSidebar(mode:.files, focusFirstItem:true)
alt focus succeeded
KeyboardFocusCoordinator-->>AppDelegate: true
else focus failed or no context
AppDelegate->>FileExplorerState: set mode = .files
AppDelegate->>FileExplorerState: setVisible(true)
end
alt overall failure
AppDelegate-->>NSSound: NSSound.beep()
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors, 1 warning, 1 inconclusive)
✅ 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}
Greptile SummaryThis PR adds a persistent folder button to the left sidebar footer that opens the right-sidebar Files view, and introduces
Confidence Score: 5/5Safe to merge; the new shared entrypoint correctly orders guards before side-effects, and both callers handle the false return with a beep. All previously flagged issues (window-focus ordering, dual-path state mutation, ObservableObject in leaf view) are addressed. The only remaining concern is a new @ObservedObject subscription added to an already-legacy parent struct, which has no impact on correctness or runtime behavior. No files require special attention; Sources/ContentView.swift has a minor style note about @ObservedObject footprint in SidebarFooterButtons. Important Files Changed
Sequence DiagramsequenceDiagram
participant Button as SidebarFileExplorerButton
participant Footer as SidebarFooterButtons
participant AD as AppDelegate
participant Ctx as MainWindowContext
participant GS as FileExplorerState (global)
Button->>Footer: action()
Footer->>AD: showRightSidebarModeInActiveMainWindow(mode:.files)
AD->>AD: guard mode.isAvailable()
AD->>AD: preferredRegisteredMainWindowContext()
alt context found
AD->>AD: "guard context.fileExplorerState != nil"
AD->>Ctx: focusForInWindowCommand(window)
AD->>Ctx: focusRightSidebar(mode:focusFirstItem:)
Ctx-->>AD: Bool
AD-->>Footer: Bool
else no context
AD->>GS: "state.mode = mode"
AD->>GS: setVisible(true)
AD-->>Footer: true
end
alt "result == false"
Footer->>Footer: NSSound.beep()
end
Reviews (10): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | 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: 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/AppDelegate.swift`:
- Around line 5496-5509: The code currently sets state.mode and calls
state.setVisible(true) before calling
context.keyboardFocusCoordinator.focusRightSidebar(...), which ignores failure
and returns true even if focusRightSidebar fails; change this so that
focusRightSidebar is attempted first (or if UI mutation must occur first,
capture previous state and on focusRightSidebar returning false roll back to the
previous mode/visibility) and propagate the failure by returning false; use the
symbols state.mode, state.setVisible(true), and
context.keyboardFocusCoordinator.focusRightSidebar(mode:focusFirstItem:) to
locate and either reorder the calls (call focusRightSidebar before mutating
state) or implement a rollback that restores the prior state when
focusRightSidebar returns false, then only return true when focusRightSidebar
succeeded.
🪄 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: d24e56f5-3173-447d-9fab-717808dc64a3
📒 Files selected for processing (4)
Sources/AppDelegate.swiftSources/ContentView+RightSidebarCommandPalette.swiftSources/ContentView.swiftcmuxTests/RightSidebarCommandPaletteTests.swift
| if let window { | ||
| mainWindowVisibilityController.focusForInWindowCommand(window, reason: .rightSidebarFocus) | ||
| } | ||
|
|
||
| guard let state = context?.fileExplorerState ?? fileExplorerState else { | ||
| return false | ||
| } |
There was a problem hiding this comment.
mainWindowVisibilityController.focusForInWindowCommand is called unconditionally before the guard let state = context?.fileExplorerState ?? fileExplorerState check. When a registered context has a resolved window but a nil fileExplorerState (the exact scenario covered by testShowRightSidebarModeDoesNotMutateGlobalStateWhenRegisteredFocusFails), the window is brought into focus and then the function returns false — callers beep. The user sees a window activation with no sidebar change and hears the error sound. Moving the if let window block to after the state guard ensures the focus side-effect only happens when the sidebar will actually be shown.
There was a problem hiding this comment.
Fixed in 3ca32d9 by moving validation ahead of the window focus side effect; the stale pre-fix path no longer applies.
— Claude Code
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 3ca32d9. Configure here.
| return context.keyboardFocusCoordinator.focusRightSidebar( | ||
| mode: mode, | ||
| focusFirstItem: focusFirstItem | ||
| ) |
There was a problem hiding this comment.
Sidebar state mutated before focus result causes false beep
Medium Severity
When showRightSidebarModeInActiveMainWindow delegates to context.keyboardFocusCoordinator.focusRightSidebar, the internal implementation of focusRightSidebar unconditionally calls state.setVisible(true) and sets state.mode before determining the focus result. If focus ultimately fails (endpoint and fallback both fail), focusRightSidebar returns false, which propagates back to callers in SidebarFooterButtons.showFileExplorer and handleCommandPaletteRightSidebarMode, causing NSSound.beep(). The user sees the sidebar open but also hears an error beep — contradicting the PR's goal of not partially applying sidebar state on failure.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 3ca32d9. Configure here.
There was a problem hiding this comment.
Not changed: this is not a real failure path in the current coordinator contract. Once context.fileExplorerState exists, focusRightSidebar sets the pending request and returns success even if the concrete endpoint is not mounted yet; the false-return paths are validated before any sidebar mutation in showRightSidebarModeInActiveMainWindow.
— Claude Code
# Conflicts: # cmuxTests/RightSidebarCommandPaletteTests.swift
|
cmux-reconcile: partly-useful Usefulness verdict: Treat as a discoverability enhancement, not implementation of a missing file explorer. The diff adds a persistent folder button and shared focus routing into the already-existing right-sidebar Files tree. current Files view and current CLI already provide that capability. Keep only if the extra entrypoint improves the current sidebar layout; compare it against current buttons/command palette. #888’s basic explorer requirement is not a reason to merge this UI change automatically. Reviewed patch head: Older issue/PR tracking index — remaining scope and competing implementations are recorded there. |
|
Mac fleet instructions for head JOB_JSON=$(~/.local/bin/cmux-ci submit --kind cmux --command 'CMUX_FLEET_BUILD_TAG=pr-3976-30b8234a /Users/Shared/cmux-build-fleet/recipes/cmux.sh https://github.com/manaflow-ai/cmux.git 30b8234af1753e3adf90589e151b6a539d3d31fc' --artifact artifacts/cmux.app.zip --workspace https://github.com/manaflow-ai/cmux/pull/3976 --source-digest 30b8234af1753e3adf90589e151b6a539d3d31fc --cache-key cmux:pr-3976 --min-free-bytes 268435456000 --label cmux --label ram48)
JOB_ID=$(python3 -c 'import json,sys; print(json.load(sys.stdin)["id"])' <<<"$JOB_JSON")
~/.local/bin/cmux-ci wait "$JOB_ID" --receipt artifacts/fleet/$JOB_ID.json
~/.local/bin/cmux-ci publish-hq "$JOB_ID"Use an existing campaign job ID if one is already posted; do not submit a duplicate. A wait timeout leaves the remote job running. Published results will include an exact-head artifact link and timing/disk receipt. This recipe validates the macOS app only, not iOS or tests. Never use maclease or put credentials in a PR comment. |
teamleaderleo
added
area: sidebar
This branch was successfully deployed
3 participants
Summary
Closes #888
Testing
Note
Low Risk
UI and focus-routing changes around the file explorer; no auth, data, or security-sensitive paths.
Overview
Adds a folder control in the left sidebar footer that opens the right sidebar in Files mode with focus, using the same shortcut labels/tooltips as keyboard settings and showing selected state when Files is visible.
Introduces
AppDelegate.showRightSidebarModeInActiveMainWindowas the shared path: prefer the active main-window context (focus window, thenfocusRightSidebar), otherwise update globalfileExplorerState; it does not fall back to global state when a registered window exists but sidebar state or focus fails. The command palette’s right-sidebar mode actions now call this API and beep on failure instead of locally toggling visibility.Tests cover global-state fallback with no window context and the case where a registered context has no sidebar state—global mode/visibility must stay unchanged.
Reviewed by Cursor Bugbot for commit 30b8234. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds a persistent folder button in the left sidebar footer that opens and focuses the Files tree in the right sidebar. Introduces a shared entrypoint to show and focus right-sidebar modes, used by both the button and the command palette. Closes #888.
New Features
Refactors
Written for commit 30b8234. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Bug Fixes / UX
Tests