Repository navigation
Restore right-click sidebar-button view switcher and built-in views (#5173) - #5182
Merged
Merged
Conversation
The right-click sidebar-button menu lost its built-in views in v0.64.11. This test asserts the seven built-in views (Default Workspaces + the six presets) are available via CmuxExtensionSidebarSelection regardless of the experimental Extensions beta flag, and that a selected view resolves to itself (which drives the menu checkmark). Failing test only — no fix yet, so CI goes red and proves the test catches the regression. The fix follows in the next commit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#4994 ("Replace sidebar extension kit contract") replaced the old in-process sidebar-provider contract with an XPC-based ExtensionKit host. It kept the in-process compatibility render layer (CmuxExtensionCompatibility.swift) and ContentView's render pipeline, but it: - stubbed CmuxExtensionSidebarSelection.providers to `[]` (was SidebarExamples.providers — the six built-in preset views) and unlinked the CmuxExtensionSidebarExamples package from the app target; - gated the sidebar-button right-click menu behind the experimental Extensions beta flag (`guard isEnabled` in showMenu, and effectiveExtensionSidebarProviderId forcing Default Workspaces whenever the flag was off); - dropped the BrowserStackSidebar.stateDidLoadNotification refresh. Net effect on a default install (beta off): the right-click menu and all of its built-in views (Default Workspaces, Project Worktrees, Attention Queue, Dev Servers, Last Prompt, Super Compact, Browser Stack) disappeared. This restores them on top of #4994's new contract rather than reverting it. The experimental flag now gates only the new XPC surface (the hosted "Extension Sidebar" entry, the puzzle button, the extensions browser); the built-in views are always available again: - re-link CmuxExtensionSidebarExamples into the cmux target and restore `providers`/import (the examples compile unchanged against the kept compatibility layer); - split builtInDescriptors (always) from descriptors (adds the hosted entry only when the beta is on) and allDescriptors (handler superset); - add the pure effectiveProviderId(_:extensionsEnabled:) so the menu checkmark and the rendered provider track the selection, downgrading only a hosted selection to default while the beta is off; - drop the `guard isEnabled` in showMenu and un-gate the command-palette "Sidebar: <view>" contributions; - restore the BrowserStackSidebar live-refresh subscription. Closes #5173 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughRefactors the sidebar provider descriptor model for runtime Extensions-beta toggling, updates command-palette and right-click menu wiring to remain resolvable after flag flips, imports example providers, refreshes the browser-stack sidebar snapshot on load, and adds regression tests and an example test suite. ChangesSidebar provider menu restoration and feature-flag resilience
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (16 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 |
Contributor
Greptile SummaryFixes the v0.64.11 regression (#5173) where right-clicking the sidebar button no longer showed the view-switcher menu and all seven built-in sidebar views were gone on a default install (Extensions beta off). The fix re-links
Confidence Score: 5/5Safe to merge — the changes are scoped to UserDefaults-driven sidebar selection and menu gating, with no auth or data surface affected. The fix correctly un-gates the menu and restores built-in providers, effectiveProviderId is a pure function with no side-effects, the existential dispatch bug is resolved by promoting render(snapshot:) to a protocol requirement, and regression tests lock in descriptor availability and the full checkmark-selection roundtrip. No logic errors or dangerous state transitions were found in the changed paths. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Right-click sidebar button] --> B[showMenu]
B --> C[Read persisted ID from UserDefaults]
C --> D[effectiveProviderId\npersisted, extensionsEnabled: isEnabled]
D -->|hosted + beta OFF| E[defaultProviderId]
D -->|built-in OR hosted + beta ON| F[persistedProviderId]
E --> G[descriptor for: effectiveID]
F --> G
G --> H[Build NSMenu from descriptors]
H -->|beta OFF| I[builtInDescriptors\n7 built-in views]
H -->|beta ON| J[builtInDescriptors\n+ hostedExtensionsDescriptor]
I --> K[Show menu with checkmark]
J --> K
L[VerticalTabsSidebar render] --> M[effectiveExtensionSidebarProviderId\ncomputed property]
M --> N[effectiveProviderId\nselectedID, extensionsEnabled]
N -->|hosted + beta OFF| O[Render Default Workspaces]
N -->|any built-in| P[Render built-in provider via existential]
N -->|hosted + beta ON| Q[Render CMUXInstalledExtensionSidebarHostView]
Reviews (3): Last reviewed commit: "Restore render(snapshot:) as a protocol ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+83
to
+96
| @Test | ||
| func selectedBuiltInViewResolvesToItselfForCheckmark() { | ||
| withExtensionsBeta(false) { | ||
| for builtInID in Self.builtInViewIDs { | ||
| withSelectedProvider(builtInID) { | ||
| let active = CmuxExtensionSidebarSelection.descriptor(for: builtInID) | ||
| #expect( | ||
| active.id == builtInID, | ||
| "Selecting \(builtInID) did not resolve to itself (menu checkmark would be wrong)" | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
withSelectedProvider wrapper is dead code in this test
CmuxExtensionSidebarSelection.descriptor(for: builtInID) takes the provider ID as a direct parameter and never reads UserDefaults, so the withSelectedProvider(builtInID) setup is never consumed by the assertion. The test therefore doesn't verify the full UserDefaults → effectiveProviderId → descriptor(for:) → .id roundtrip that actually drives the menu checkmark. The assertion is correct for what it does check (that the descriptor lookup returns the right ID), but the withSelectedProvider framing gives a misleading impression of broader coverage.
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 4f5d45b. The test now reads the persisted id back from CmuxExtensionSidebarSelection.defaultsKey, resolves it through effectiveProviderId(_:extensionsEnabled:), and asserts the resulting descriptor(for:).id equals the selected view — mirroring showMenu's checkmark logic. The withSelectedProvider scaffolding is now actually consumed, so the full UserDefaults → effectiveProviderId → descriptor(for:) roundtrip is exercised.
— Claude Code
![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 `@cmuxTests/SidebarProviderMenuRegressionTests.swift`:
- Around line 38-46: The test selectedBuiltInViewResolvesToItselfForCheckmark
currently writes a persisted value via withSelectedProvider but then ignores it
by directly calling CmuxExtensionSidebarSelection.descriptor(for: builtInID);
update the test to actually exercise persisted-selection logic by reading the
persisted provider id from CmuxExtensionSidebarSelection.defaultsKey into a
variable (persistedProviderId), computing the resolved id with
effectiveProviderId(persistedProviderId, extensionsEnabled: isEnabled), and
asserting against CmuxExtensionSidebarSelection.descriptor(for: resolvedId);
alternatively remove the dead withSelectedProvider scaffolding if you prefer to
keep the original direct-descriptor assertion.
- Around line 84-96: The test selectedBuiltInViewResolvesToItselfForCheckmark is
writing a persisted selection via withSelectedProvider but then validates using
CmuxExtensionSidebarSelection.descriptor(for:) which does not read UserDefaults;
update the test to mirror the real showMenu path by reading the persisted
provider id (CmuxExtensionSidebarSelection.defaultsKey) then computing the
effective id via effectiveProviderId(..., extensionsEnabled: isEnabled) and
compare that effective selectedProviderId against each descriptor.id (or
alternatively invoke CmuxExtensionSidebarSelection.showMenu and assert each
NSMenuItem.state directly); change references to withSelectedProvider,
CmuxExtensionSidebarSelection.descriptor(for:),
CmuxExtensionSidebarSelection.showMenu, and effectiveProviderId accordingly so
the test verifies the persisted-selection → menu checkmark behavior.
🪄 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: 2c5e7b1c-7e98-4aa6-8b88-760b09fc4b21
📒 Files selected for processing (3)
Sources/ContentView.swiftcmux.xcodeproj/project.pbxprojcmuxTests/SidebarProviderMenuRegressionTests.swift
![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 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
CodeRabbit, Greptile, and cubic all flagged that the checkmark regression test wrote a persisted selection via withSelectedProvider but then asserted on descriptor(for: builtInID) with the id passed directly — never reading UserDefaults — so it didn't validate the persisted → effectiveProviderId → descriptor(for:) roundtrip that showMenu actually uses to place the checkmark. Rewrite it to read the persisted id back from defaultsKey, resolve it through effectiveProviderId(_:extensionsEnabled:), and assert the resolved descriptor id is the selected view — mirroring showMenu exactly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The built-in views rendered empty even with workspaces present: the host renders each view through an `any CmuxExtensionSidebarProvider` existential (`provider(for:)?.render(snapshot:)`), and #4994 left `render(snapshot:)` only as a protocol-extension default — so the call static-dispatched to the empty default instead of the concrete view. These tests exercise that exact path and fail without the fix: - cmuxTests: the host's `provider(for:)?.render(snapshot:)` returns 0 rows. - CmuxExtensionSidebarExamples: rendering a provider through the existential diverges from the concrete call (empty vs populated). Failing test only — fix follows. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#4994 demoted CmuxExtensionSidebarProvider.render(snapshot:) from a protocol requirement to a protocol-extension default returning no sections. Because the host renders views through an `any CmuxExtensionSidebarProvider` existential, a method defined only in the extension static-dispatches to that empty default — so every built-in view (Project Worktrees, Attention Queue, Dev Servers, Last Prompt, Super Compact, Browser Stack) rendered an empty sidebar. Restore it as a protocol requirement (keeping the extension default) so the call dynamic-dispatches to the concrete view via the witness table — matching v0.64.10. Verified live: Super Compact now renders all 51 workspaces (sections=1 rows=[51]); the contextual/mutable default render(snapshot:context:) also routes correctly since it forwards to render(snapshot:). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Merged
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
Fixes the v0.64.11 regression where right-clicking the sidebar button (top-left, by the traffic lights) no longer opened the context menu for switching between built-in sidebar views — and the views themselves (Default Workspaces, Project Worktrees, Attention Queue, Dev Servers, Last Prompt, Super Compact, Browser Stack) were gone.
Closes #5173
What #4994 changed
a086dd1fd("Replace sidebar extension kit contract") swapped the old in-process sidebar-provider contract for an XPC-based ExtensionKit host. It kept the in-process compatibility render layer (CmuxExtensionCompatibility.swift) andContentView's render pipeline, but it:CmuxExtensionSidebarSelection.providersto[](wasSidebarExamples.providers, the six built-in preset views) and unlinked theCmuxExtensionSidebarExamplespackage from the app target;guard isEnabledinshowMenu, andeffectiveExtensionSidebarProviderIdforcing Default Workspaces whenever the flag was off — seeBetaFeaturesCatalogSection.extensions, which is off by default);BrowserStackSidebar.stateDidLoadNotificationlive-refresh.Net effect on a default install (beta off): the menu and all of its built-in views disappeared.
How this restores them (on top of the new contract, not a revert)
The new XPC contract is intentional, so this re-implements the built-in views on top of it and lets both coexist. The experimental flag now gates only the new XPC surface (the hosted "Extension Sidebar" entry, the puzzle button, the extensions browser); the built-in views are always available again, exactly as in v0.64.10.
CmuxExtensionSidebarExamplesinto thecmuxtarget and restoreproviders+ the import. The example providers compile unchanged against the kept compatibility layer (verified with a standaloneswift build).builtInDescriptors(always: Default + 6 presets, in v0.64.10 menu order) vsdescriptors(adds the hosted entry only when the beta is on) vsallDescriptors(handler superset for command registration).effectiveProviderId(_:extensionsEnabled:)so the menu checkmark and the rendered provider track the persisted selection, downgrading only a hosted selection to Default while the beta is off (so the experimental feature can't strand the user). The view'seffectiveExtensionSidebarProviderIdnow delegates to it.guard isEnabledinshowMenu; the checkmark reflects the effective selection. Also un-gate the command-palette "Sidebar: " contributions.BrowserStackSidebarlive-refresh subscription.Tests
Two-commit red/green structure:
cmuxTests/SidebarProviderMenuRegressionTests.swift, wired into the project) that asserts all seven built-in views are present inCmuxExtensionSidebarSelection.descriptorswith the Extensions beta off, and that a selected built-in view resolves to itself (drives the checkmark). CI should be red on this commit.effectiveProviderIdgates the hosted entry but never the built-in views.xcodebuild -scheme cmux-unitcould not be run locally (submodules/GhosttyKit not provisioned in this worktree and disk is constrained); relying on thetestsCI job to gate the build per the repo's testing policy. The Examples↔compatibility-layer contract was verified locally withswift build.🤖 Generated with Claude Code
Note
Medium Risk
Touches primary sidebar selection/rendering paths and Swift protocol dispatch; risk is mitigated by focused regression tests but behavior spans AppKit menus, command palette, and SwiftUI effective provider routing.
Overview
Fixes the v0.64.11 regression where built-in sidebar views and the sidebar-button view switcher disappeared when the Extensions beta was off (#5173).
CmuxExtensionKit:render(snapshot:)is declared as a protocol requirement onCmuxExtensionSidebarProviderso calls throughany CmuxExtensionSidebarProviderdynamic-dispatch to concrete implementations instead of the empty extension default.App wiring: Re-links
CmuxExtensionSidebarExamples, restoresprovidersfromSidebarExamples.providers, and splits descriptor lists (builtInDescriptors,descriptorswith hosted entry only when beta is on,allDescriptorsfor palette handler registration). AddseffectiveProviderIdso built-in selections always render; only a persisted hosted extension selection falls back to Default Workspaces when the beta is off. The right-click switcher and command-palette “Sidebar: …” entries are no longer gated entirely on the beta; menu checkmarks use the effective id.BrowserStackSidebar.stateDidLoadNotificationagain triggers sidebar snapshot refresh.Tests: Regression suites assert built-in menu availability, checkmark resolution, existential rendering, and host-path rendering.
Reviewed by Cursor Bugbot for commit d27b449. Bugbot is set up for automated code reviews on this repo. Configure here.
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Summary by cubic
Restores the right-click sidebar-button view switcher and the seven built-in views lost in v0.64.11, and fixes an empty-render regression by restoring dynamic dispatch for
render(snapshot:)(fixes #5173). Keeps the new XPC sidebar host; the Extensions beta now gates only the hosted “Extension Sidebar” entry.CmuxExtensionSidebarExamplesand restored itsproviders.effectiveProviderId(...)to honor built-ins; a hosted selection downgrades to Default when the beta is off.allDescriptors.CmuxExtensionSidebarProvider.render(snapshot:)a protocol requirement inCmuxExtensionKit; added existential-dispatch tests in app tests andCmuxExtensionSidebarExamplesto lock this in.Written for commit d27b449. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests
Documentation