Repository navigation
Open Settings and iPhone pairing as panes - #8046
lawrencecchen wants to merge 25 commits into
Conversation
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. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds an ChangesApp Utility Surfaces
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant EntryPoint
participant AppDelegate
participant Workspace
participant AppUtilityPanel
participant SettingsWindowRoot
EntryPoint->>AppDelegate: request settings or mobile pairing
AppDelegate->>Workspace: open or focus app utility surface
Workspace->>AppUtilityPanel: reuse or create panel
AppUtilityPanel->>SettingsWindowRoot: render scoped settings content
SettingsWindowRoot->>SettingsWindowRoot: accept matching navigation notifications
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
Greptile SummaryThis PR hosts Settings and iPhone pairing in reusable workspace panes. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (21): Last reviewed commit: "Resolve portal reconciliation merge" | Re-trigger Greptile |
| .task(id: panel.settingsNavigationRevision) { | ||
| guard let target = panel.settingsNavigationTarget else { return } | ||
| SettingsNavigationRequest.post( | ||
| target, | ||
| scope: panel.settingsNavigationScope | ||
| ) | ||
| } |
There was a problem hiding this comment.
Initial Navigation Races Restoration
When a new Settings pane is opened with a target, this task posts that target while SettingsWindowRoot independently posts its persisted selection from onAppear. If restoration runs second, it overwrites the requested section, so menu and search actions can open Settings on the previously selected page instead of the requested destination.
Rule Used: Flag Swift fixes that patch symptoms while leaving... (source)
| agentSessionSnapshot = nil | ||
| case .extensionBrowser: | ||
| return nil | ||
| case .cloudVMLoading: | ||
| case .cloudVMLoading, .appUtility: |
There was a problem hiding this comment.
Utility Panes Lose Restore State
Returning nil excludes Settings and Pair iPhone panes from the snapshot used by closed-item history and restoration. Closing one therefore creates no restorable entry despite the new .appUtility history label, and layout capture replaces the unsupported surface with a terminal rather than restoring the utility pane.
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/Panels/AppUtilityPanel.swift`:
- Line 32: Thread the panel-local settingsNavigationScope from AppUtilityPanel
through SettingsNavigationRequest.post(_:scope:) in
SettingsWindowNavigationDelivery, and make the receiver observe/filter
notifications using that same scope object. Ensure requests from other panels
are ignored; alternatively remove settingsNavigationScope if scoped routing is
not required.
🪄 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: 3d116c8d-c718-4210-b64d-7c986a704dd6
📒 Files selected for processing (27)
Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Scene/SettingsWindowScene.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Core/Values/SurfaceKind.swiftPackages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/Core/WorkspaceCoreValueTests.swiftResources/Localizable.xcstringsSources/App/AppDelegateSettingsPresentation.swiftSources/App/ConfiguredMobileConnectAction.swiftSources/AppDelegate.swiftSources/Canvas/WorkspaceCanvasHostView.swiftSources/ClosedItemHistory.swiftSources/CmuxLifecycleEventPublishing.swiftSources/ContentView+CommandPaletteSurfaceMetadata.swiftSources/ContentView.swiftSources/HostSettingsActions.swiftSources/Mobile/Pairing/MobilePairingView.swiftSources/MobileConnectTitlebarAccessory.swiftSources/Panels/AppUtilityPanel.swiftSources/Panels/Panel.swiftSources/Panels/PanelContentView.swiftSources/Search/GlobalSearchDocuments.swiftSources/SettingsNavigation.swiftSources/TerminalPaneDropTargetView.swiftSources/Workspace+AppUtilitySurface.swiftSources/Workspace+LayoutCapture.swiftSources/Workspace+SurfaceNavigation.swiftSources/Workspace.swiftcmux.xcodeproj/project.pbxprojcmuxTests/AppUtilityPanelTests.swift
| let stableSurfaceIdentity = PanelStableSurfaceIdentity() | ||
| let panelType: PanelType = .appUtility | ||
| let kind: Kind | ||
| let settingsNavigationScope = UUID().uuidString |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'settingsNavigationScope' -C4 --glob '*.swift'Repository: manaflow-ai/cmux
Length of output: 154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files 'Sources/**/*.swift' | sed -n '1,200p'
printf '\n-- matches --\n'
rg -n 'settingsNavigationScope|SettingsNavigation|SettingsWindowScene|NotificationCenter.*settings|notification.*settings|scope' Sources --glob '*.swift' -C 3Repository: manaflow-ai/cmux
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' Sources/Panels/AppUtilityPanel.swift
printf '\n-- related files --\n'
fd -a 'SettingsWindowScene.swift|SettingsNavigation.swift' SourcesRepository: manaflow-ai/cmux
Length of output: 2027
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- AppUtilityPanel.swift ---'
sed -n '1,140p' Sources/Panels/AppUtilityPanel.swift
printf '\n%s\n' '--- settings-related matches ---'
rg -n 'SettingsWindowScene|SettingsNavigation|settingsNavigationScope|NavigationScope|NotificationCenter' Sources --glob '*.swift' -C 2Repository: manaflow-ai/cmux
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- settings navigation symbols ---'
rg -n 'SettingsNavigationRequest|settingsNavigationScope|SettingsNavigationTarget|NavigationScope|NotificationCenter\.default\.(post|publisher|addObserver)' Sources/App/SettingsWindowNavigationDelivery.swift Sources/App/SettingsWindowPresenter.swift Sources/App/SettingsWindowFactory.swift Sources/SettingsNavigation.swift Sources/App/SettingsWindowRoot.swift Sources/App/AppUtilityPanel.swift --glob '*.swift' -C 3
printf '\n%s\n' '--- SettingsNavigation.swift ---'
sed -n '1,220p' Sources/SettingsNavigation.swift
printf '\n%s\n' '--- SettingsWindowNavigationDelivery.swift ---'
sed -n '1,220p' Sources/App/SettingsWindowNavigationDelivery.swiftRepository: manaflow-ai/cmux
Length of output: 12406
Thread settingsNavigationScope through the request. Sources/App/SettingsWindowNavigationDelivery.swift:19,36 posts SettingsNavigationRequest without the panel-local UUID from Sources/Panels/AppUtilityPanel.swift:32, so the receiver has nothing to filter on. Pass the same scope via SettingsNavigationRequest.post(_:scope:) and observe by notification object, or drop the unused property.
🤖 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/Panels/AppUtilityPanel.swift` at line 32, Thread the panel-local
settingsNavigationScope from AppUtilityPanel through
SettingsNavigationRequest.post(_:scope:) in SettingsWindowNavigationDelivery,
and make the receiver observe/filter notifications using that same scope object.
Ensure requests from other panels are ignored; alternatively remove
settingsNavigationScope if scoped routing is not required.
Source: Path instructions
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
# Conflicts: # Sources/App/SettingsWindowFactory.swift # Sources/ContentView.swift # Sources/Mobile/Pairing/MobilePairingView.swift # Sources/Panels/PanelContentView.swift # Sources/Search/GlobalSearchDocuments.swift # Sources/Workspace+LayoutCapture.swift
# Conflicts: # .github/swift-file-length-budget.tsv
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. |
# Conflicts: # Sources/Workspace.swift
Summary
Testing
swift test --package-path Packages/macOS/CmuxWorkspaces --filter SurfaceKindTestsswift test --package-path Packages/macOS/CmuxSettingsUI --filter SettingsSearchIndexTests./scripts/lint-pbxproj-test-wiring.shscripts/swift_file_length_budget.pyiospanNeed help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Open Settings and iPhone pairing as right-side panes that reuse an existing pane of the same kind. Non-activating opens restore and order-front without changing selection, focus, or zoom; panes are registered in the canvas, and Settings search is faster and scoped per pane with correct clearing and text scaling.
New Features
AppUtilityPanelwith kinds.settingsand.mobilePairing; introducedPanelType.appUtility,SurfaceKind.appUtility, command palette labels/keywords (“App”), and “App” localization in Recently Closed.SettingsPresentationStyle.pane; added a pane-native sidebar, scoped navigation viaSettingsNavigationNotificationRouter, and a per-paneSettingsSidebarModelfor faster filtering.openAppUtilityPane(menu, titlebar, command palette, configured actions, Terminal control, surface tab bar). Opens to the right, reuses same-kind panes, focuses on activating; remote-tmux mirrors are bypassed.Bug Fixes
@AppStoragewith per-root state and explicit persistence viaSettingsInitialSelectionResolver; initial navigation overrides persisted values without writing defaults.Written for commit 88d83f4. Summary will update on new commits.
Summary by CodeRabbit