Repository navigation
Wire bundled sidebar example providers into cmux - #4933
lawrencecchen wants to merge 4 commits into
Conversation
Add the bundled example providers (ProjectWorktree, AttentionQueue, DevServer, LastPrompt, SuperCompact, BrowserStack) so users can pick custom sidebars from the provider menu, and document where to drop a vibe-coded provider in Swift. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
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 a local Swift package dependency (CmuxExtensionSidebarExamples) to the Xcode project, imports it in ContentView to expose ChangesSidebar Provider Package Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (3 errors, 1 warning)
✅ Passed checks (14 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 wires
Confidence Score: 5/5Safe to merge; changes are confined to example provider wiring and a UI-layer notification subscription with no impact on auth, persistence, or core data paths. The xcodeproj linkage mirrors the existing CmuxExtensionKit pattern and introduces no new build risk. The SidebarExamples.providers substitution is straightforward and the providers were already shipping as dead code. The DispatchQueue.main.async wrapper and the BrowserStackSidebar-specific .onReceive are architectural patterns worth revisiting, but they do not introduce a current defect or crash path. No files require special attention for merge safety. The two architectural observations in BrowserStackSidebar.swift and ContentView.swift are worth tracking as the provider ecosystem grows but do not block this change. Important Files Changed
Sequence DiagramsequenceDiagram
participant V as VerticalTabsSidebar (Main)
participant BS as BrowserStackSidebar
participant Cache as BrowserStackSidebarStateCache
participant BG as Task.detached (utility)
participant NC as NotificationCenter
V->>V: .onReceive subscribed (BrowserStackSidebar.stateDidLoadNotification)
V->>BS: render(snapshot:)
BS->>Cache: state(for: snapshot)
Cache->>Cache: startLoadIfNeeded
Cache->>BG: Task.detached — store.load(scopeKey:)
BG-->>Cache: applyLoadedState(loaded)
Cache->>Cache: onAsyncStateLoaded?()
Cache->>BS: postStateDidLoadNotification()
BS->>NC: "DispatchQueue.main.async { post(stateDidLoadNotification) }"
NC-->>V: onReceive fires
V->>V: refreshExtensionSidebarSnapshot()
Reviews (4): Last reviewed commit: "Defer BrowserStack stateDidLoad post to ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| static var providers: [any CmuxExtensionSidebarProvider] { | ||
| [] | ||
| SidebarExamples.providers | ||
| } |
There was a problem hiding this comment.
Localizable.xcstrings carries translations for 20 locales (ar, bs, da, de, en, es, fr, it, ja, km, ko, nb, pl, pt-BR, ru, th, tr, uk, zh-Hans, zh-Hant), but every example.sidebar.* key — all 27 of them covering provider titles, subtitles, and section headings — is only translated for en and ja. Before this PR, SidebarExamples.providers was dead code ([]); wiring it in here makes these strings live in the provider menu for all locale users. On any non-English, non-Japanese device, every provider name and section header will fall back to the English default value instead of a translated string.
The fix is to add translated entries for all 18 remaining locales to each example.sidebar.* key in Resources/Localizable.xcstrings before shipping.
Rule Used: Flag production user-facing text that is not fully... (source)
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!
There was a problem hiding this comment.
Declining: per CLAUDE.md, the policy is "All user-facing strings must be localized [...] for all supported languages (currently English and Japanese)". The example.sidebar.* keys already match that policy with en+ja entries. These are clearly-labeled demo providers (com.example.cmux.sidebar.* IDs, subtitle "User extension") meant as templates for users vibe-coding their own sidebars, not first-class shipping features.
BrowserStackSidebar posts stateDidLoadNotification after its persisted layout finishes loading, but nothing observed it, so selecting Browser Stack or relaunching showed the default layout until the next 30s tick or workspace event. Observe the notification and refresh the snapshot. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The async load can finish on a background thread during the same render pass that subscribed via .onReceive, so the synchronous notification could arrive before SwiftUI installs the subscription. Hop to the main queue so the post lands on a later run-loop turn after the subscription is live. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
2 participants
Summary
End-user workflow for a custom sidebar:
Test plan
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Note
Low Risk
UI-only wiring and a timing fix for sidebar refresh; no auth, data, or security-sensitive paths.
Overview
The cmux app now depends on
CmuxExtensionSidebarExamplesand exposes its six example sidebar providers inCmuxExtensionSidebarSelection.providers(instead of an empty list), with a short comment on how to add custom providers.ContentViewlistens forBrowserStackSidebar.stateDidLoadNotificationand triggersrefreshExtensionSidebarSnapshot()so persisted Browser Stack layout appears after async load.BrowserStackSidebar.postStateDidLoadNotification()posts on the main queue asynchronously so SwiftUI.onReceivehandlers are installed before the notification fires when state loads in the same render pass.Reviewed by Cursor Bugbot for commit 4ad1718. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Wired
CmuxExtensionSidebarExamplesinto the app and exposed its six bundled sidebar providers. Sidebar snapshot now refreshes immediately and reliably whenBrowserStackSidebarloads its saved layout.New Features
ProjectWorktree,AttentionQueue,DevServer,LastPrompt,SuperCompact,BrowserStack—viaCmuxExtensionSidebarSelection.providers = SidebarExamples.providers, so they show alongside default workspaces.CmuxExtensionSidebarProviderand rebuilding with./scripts/reload.sh --tag <tag>.Bug Fixes
BrowserStackSidebar.stateDidLoadNotificationand callrefreshExtensionSidebarSnapshot()so Browser Stack shows the persisted layout immediately after selection or relaunch.stateDidLoadNotificationonDispatchQueue.main.asyncto prevent missed notifications during SwiftUI subscription install.Written for commit 4ad1718. Summary will update on new commits.
Review in cubic
Summary by CodeRabbit
New Features
Improvements
Documentation