Repository navigation
Browser extensions via WKWebExtension — Bitwarden works (popup, autofill, ⌘⇧L, passkeys) - #7692
KyloJorgensen wants to merge 2 commits into
Conversation
…ort) Adds native web-extension support to the built-in browser using WebKit's WKWebExtension API, availability-gated to macOS 15.4+ (deployment target unchanged). Extensions are discovered from the Bitwarden desktop app's bundled Safari web extension and from unpacked directories listed in the CMUX_BROWSER_EXTENSIONS environment variable. - BrowserWebExtensionSupport owns a persistent WKWebExtensionController attached to every browser WKWebViewConfiguration, auto-grants manifest permissions, and implements the controller delegate (action popups, permission prompts, window/tab creation, native messaging). - Tab/window adapters expose browser panels to extensions as tabs of one virtual window; Workspace.focusPanel drives tab-activation events. - Popout window controller implements windows.create for extension pages (Bitwarden passkey confirmation, unlock, and 2FA popouts). - Toolbar action button per loaded extension anchors the extension popup. - Extension keyboard commands (e.g. Bitwarden autofill ⌘⇧L) are offered from CmuxWebView.performKeyEquivalent, and from the stale-menu-shortcut suppression path in sendEvent so a remapped-away cmux default (⌘⇧L is Open Browser's default) stays usable by extensions. Notable behaviors learned the hard way, encoded in comments: - Extension web views must present a Safari UA (applicationNameForUserAgent); Bitwarden's UI crashes at boot on WebKit's default UA. - Native-messaging requests are parked unresolved instead of erroring; Bitwarden retry-loops without backoff on error replies. Verified with Bitwarden 2026.2.0: vault login, popup UI, content-script autofill, ⌘⇧L autofill command, and passkey sign-in via popout. Closes-Discussion: manaflow-ai#2001 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@KyloJorgensen is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds macOS 15.4+ Web Extensions support for browser panels, including controller loading, popup hosting, toolbar actions, shortcut routing, panel lifecycle wiring, and project integration. Also adds a localized help string. ChangesBrowser Web Extensions Support
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (5 errors, 1 warning)
✅ Passed checks (19 passed)
✨ 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 |
Greptile SummaryThis PR adds native WebKit browser-extension support to the built-in browser. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (2): Last reviewed commit: "fix(browser): address review findings on..." | Re-trigger Greptile |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
|
||
| func activate(for context: WKWebExtensionContext, completionHandler: @escaping (Error?) -> Void) { | ||
| support?.noteActivated(panelID: panelID) | ||
| completionHandler(nil) |
There was a problem hiding this comment.
Extension tab activate ignores focus
Medium Severity
WKWebExtensionTab.activate only calls noteActivated, updating extension bookkeeping without focusing the corresponding BrowserPanel in cmux. The UI can stay on another panel while the extension treats a different tab as active.
Reviewed by Cursor Bugbot for commit 73b5d1b. Configure here.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| for permission in webExtension.requestedPermissions { | ||
| context.setPermissionStatus(.grantedExplicitly, for: permission) | ||
| } | ||
| for pattern in webExtension.allRequestedMatchPatterns { | ||
| context.setPermissionStatus(.grantedExplicitly, for: pattern) | ||
| } |
There was a problem hiding this comment.
When cmux loads an extension from CMUX_BROWSER_EXTENSIONS or the auto-discovered app extension path, this loop marks every manifest permission and host match pattern as explicitly granted before any policy or user decision runs. An extension that requests broad host access can immediately read or modify pages in every registered browser panel.
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!
| NSWorkspace.shared.open(url) | ||
| completionHandler(nil, nil) |
There was a problem hiding this comment.
When an extension calls tabs.create with an http or https URL, this path opens the URL through NSWorkspace and returns no extension tab. Password-manager flows such as “open website” then land in the user's default browser with a different profile/session, so the extension cannot observe or autofill the tab it just requested.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Panels/CmuxWebView.swift (1)
696-708: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMove the extension-command hook after cmux-owned shortcuts.
BrowserWebExtensionSupport.shared.performCommand(for:)is a generic command bridge, and nothing here filters it against cmux’s own reserved combos. In the current order, an extension command can shadow paste-as-plain-text, find, undo/redo, or main-menu routing for any colliding Command shortcut. Route cmux-owned shortcuts first, then offer the event to extensions only if it’s still unclaimed.🤖 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/CmuxWebView.swift` around lines 696 - 708, The extension-command hook in performKeyEquivalent(with:) is taking precedence too early and can shadow cmux-reserved Command shortcuts. Reorder the routing so cmux-owned shortcuts (paste-as-plain-text, find, undo/redo, main-menu/tab handling) are checked and handled first, then call BrowserWebExtensionSupport.shared.performCommand(for:) only if the event is still unclaimed. Keep the existing finish(...) flow consistent so claimed commands stop propagation and unhandled events continue to super.performKeyEquivalent(with:).
🤖 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/BrowserWebExtensionPopoutWindowController.swift`:
- Line 51: The window title fallback in
BrowserWebExtensionPopoutWindowController is using a bare English literal, so
update the title assignment to keep context.webExtension.displayName as-is and
route the generic fallback through String(localized:defaultValue:) (or an
equivalent localized API). Use the existing window.title setup in
BrowserWebExtensionPopoutWindowController to replace the "Extension" fallback
with a localized string that still defaults to the same text.
- Around line 32-47: The centering logic in
BrowserWebExtensionPopoutWindowController should match the same fallback
condition used when choosing the window frame. Update the window.center() check
so it also covers frames that are non-null/non-empty but smaller than the
minimum size threshold, using the same frame validation used before creating the
NSWindow. This keeps the fallback defaultSize popup centered instead of leaving
it at the origin.
- Around line 36-51: The new standalone popout window in
BrowserWebExtensionPopoutWindowController is missing the stable cmux identifier
and close-shortcut wiring. Assign the NSWindow a fixed identifier such as
cmux.browserWebExtensionPopout, then add that identifier to
cmuxAuxiliaryWindowIdentifiers and ensure cmuxWindowShouldOwnCloseShortcut
recognizes it so the shared Cmd+W/close path targets this window correctly.
- Line 43: The popout window is forced to stay above other apps via window.level
= .floating in BrowserWebExtensionPopoutWindowController, so it needs to stop
lingering when cmux loses focus. Update the window setup to either set
hidesOnDeactivate = true or restore the level to a normal value when the
controller resigns active / deactivates, using the existing window configuration
code in BrowserWebExtensionPopoutWindowController.
In `@Sources/Panels/BrowserWebExtensionSupport.swift`:
- Line 15: Replace the new SwiftUI state in BrowserWebExtensionSupport with the
modern `@Observable` pattern instead of ObservableObject, and remove any
`@Published` usage by making contexts/loadErrors plain stored properties. Update
the BrowserWebExtensionActionButton consumer to observe the support object
directly without `@ObservedObject/`@StateObject, and mark non-observed internals
such as didStartLoading and windowAdapter with `@ObservationIgnored` where needed.
- Around line 121-135: Remove the temporary timing-based debug scaffolding from
BrowserWebExtensionSupport’s post-load flow: delete the `postLoad+5s`
`Task.sleep` diagnostic, the `CMUX_WEBEXT_AUTOPROBE` `Task` block, and the
paired `probePopup` implementation unless they are explicitly intended as
permanent diagnostics. Keep only sparse, non-timing-based logging in the
relevant `logDiagnostics(for:label:)` and `cmuxDebugLog` paths, and avoid
fixed-delay readiness coordination in `context.performAction(for:)` logic.
In `@Sources/Panels/BrowserWebExtensionWindowAdapter.swift`:
- Around line 47-49: The main-window implementation of focus(for:) in
BrowserWebExtensionWindowAdapter currently returns success without actually
bringing the host window to the front. Update this method to activate hostWindow
and NSApp before completing, and then ensure the active browser window is marked
focused so windows.update({ focused: true }) can take effect. Use
BrowserWebExtensionWindowAdapter.focus(for:) as the place to apply the fix,
mirroring the behavior expected from
BrowserWebExtensionPopoutWindowController.focus(for:).
---
Outside diff comments:
In `@Sources/Panels/CmuxWebView.swift`:
- Around line 696-708: The extension-command hook in performKeyEquivalent(with:)
is taking precedence too early and can shadow cmux-reserved Command shortcuts.
Reorder the routing so cmux-owned shortcuts (paste-as-plain-text, find,
undo/redo, main-menu/tab handling) are checked and handled first, then call
BrowserWebExtensionSupport.shared.performCommand(for:) only if the event is
still unclaimed. Keep the existing finish(...) flow consistent so claimed
commands stop propagation and unhandled events continue to
super.performKeyEquivalent(with:).
🪄 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: fcb63fe3-7690-4580-a7ed-f0ba57193dec
📒 Files selected for processing (13)
Resources/Localizable.xcstringsSources/App/ShortcutRoutingSupport.swiftSources/AppDelegate.swiftSources/Panels/BrowserPanel.swiftSources/Panels/BrowserPanelView.swiftSources/Panels/BrowserWebExtensionPopoutWindowController.swiftSources/Panels/BrowserWebExtensionSupport.swiftSources/Panels/BrowserWebExtensionTabAdapter.swiftSources/Panels/BrowserWebExtensionToolbarButtons.swiftSources/Panels/BrowserWebExtensionWindowAdapter.swiftSources/Panels/CmuxWebView.swiftSources/Workspace.swiftcmux.xcodeproj/project.pbxproj
| window = NSWindow( | ||
| contentRect: frame, | ||
| styleMask: [.titled, .closable, .resizable], | ||
| backing: .buffered, | ||
| defer: false | ||
| ) | ||
| window.isReleasedWhenClosed = false | ||
| window.level = .floating | ||
| window.contentView = webView | ||
| if configuration.frame.isNull || configuration.frame.isEmpty { | ||
| window.center() | ||
| } | ||
|
|
||
| super.init() | ||
| window.delegate = self | ||
| window.title = context.webExtension.displayName ?? "Extension" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
New standalone NSWindow lacks a stable cmux.* identifier and close-shortcut registration.
This constructs a user-visible, closable NSWindow for extension popouts (e.g. Bitwarden's passkey/2FA popup), but never assigns window.identifier, and there's no visible registration with the shared close-shortcut path. Per the repo's window-management rule, standalone closable windows must have a stable cmux.* identifier and be covered by cmuxWindowShouldOwnCloseShortcut/cmuxAuxiliaryWindowIdentifiers so Cmd+W (and the shared close path) target this window correctly instead of falling through to workspace panel closing.
🔧 Suggested fix
window = NSWindow(
contentRect: frame,
styleMask: [.titled, .closable, .resizable],
backing: .buffered,
defer: false
)
window.isReleasedWhenClosed = false
+window.identifier = NSUserInterfaceItemIdentifier("cmux.browserWebExtensionPopout")
window.level = .floatingThen register "cmux.browserWebExtensionPopout" with cmuxAuxiliaryWindowIdentifiers and ensure cmuxWindowShouldOwnCloseShortcut covers it.
As per coding guidelines: "When a standalone user-closable window is introduced, assign it a stable cmux.* identifier and register it so the shared path can close or hide the active window instead of falling through to workspace panel closing."
🤖 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/BrowserWebExtensionPopoutWindowController.swift` around lines
36 - 51, The new standalone popout window in
BrowserWebExtensionPopoutWindowController is missing the stable cmux identifier
and close-shortcut wiring. Assign the NSWindow a fixed identifier such as
cmux.browserWebExtensionPopout, then add that identifier to
cmuxAuxiliaryWindowIdentifiers and ensure cmuxWindowShouldOwnCloseShortcut
recognizes it so the shared Cmd+W/close path targets this window correctly.
Sources: Coding guidelines, Path instructions
| defer: false | ||
| ) | ||
| window.isReleasedWhenClosed = false | ||
| window.level = .floating |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,220p' Sources/Panels/BrowserWebExtensionPopoutWindowController.swiftRepository: manaflow-ai/cmux
Length of output: 5612
🏁 Script executed:
rg -n "hidesOnDeactivate|window.level = \\.floating|orderFront|makeKeyAndOrderFront|NSWindowDelegate|popoutDidClose|auxiliary window|web extension" SourcesRepository: manaflow-ai/cmux
Length of output: 11643
🏁 Script executed:
sed -n '1,320p' Sources/Panels/BrowserPopupWindowController.swiftRepository: manaflow-ai/cmux
Length of output: 13433
🏁 Script executed:
sed -n '220,320p' Sources/Panels/BrowserWebExtensionSupport.swiftRepository: manaflow-ai/cmux
Length of output: 4062
🌐 Web query:
AppKit NSWindow floating level hidesOnDeactivate normal window behavior when app deactivates
💡 Result:
The behavior you are observing is caused by the hidesOnDeactivate property of the NSWindow [1][2]. When this property is set to true, the window is automatically removed from the screen when your application becomes inactive (i.e., when the user switches to another app) [1][2]. Key technical points regarding this behavior: 1. Automatic Hiding: If you assign a window to the floating level (NSFloatingWindowLevel) or use a panel (NSPanel), you may inadvertently inherit or explicitly set behaviors that trigger hiding on deactivation [3][4][5]. Panels (NSPanel) have a default hidesOnDeactivate value of true, whereas standard NSWindows have a default of false [5]. 2. Conflicting Levels: If you want a window to remain visible while your application is inactive, ensure that hidesOnDeactivate is explicitly set to false [1]. Conversely, if you want it to hide, ensure it is true [1][2]. 3. Managing "Always on Top": If you wish to have a window that floats over other windows within your own application but does not persist over other applications, there is no single built-in window level constant for this [6]. Developers commonly handle this by: - Responding to NSApplicationWillResignActiveNotification and NSApplicationDidBecomeActiveNotification [6]. - When the application resigns active, programmatically set the window level to normal (NSNormalWindowLevel) or use orderOut: to hide it [6][7]. - When the application becomes active again, restore the window level to floating (NSFloatingWindowLevel) [6]. If you find your window is hiding unintentionally, verify the hidesOnDeactivate property both in your code and in Interface Builder, as it may be inherited from the window's class (e.g., if you are using an NSPanel) [5].
Citations:
- 1: https://developer.apple.com/documentation/appkit/nswindow/hidesondeactivate
- 2: https://developer.apple.com/documentation/appkit/nswindow
- 3: https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/WinPanel/Concepts/WindowLevel.html
- 4: https://leopard-adc.pepas.com/documentation/Cocoa/Conceptual/WinPanel/Concepts/WindowLevel.html
- 5: https://stackoverflow.com/questions/30305901/nswindow-unintentionally-hides-when-application-resigns-active
- 6: https://stackoverflow.com/questions/23944223/creating-an-nswindow-that-floats-over-all-other-windows-in-the-app-but-not-over
- 7: https://stackoverflow.com/questions/10441060/nswindow-deactivate-on-hide-isvisible
Hide the extension popout when cmux deactivates
window.level = .floating will keep this auth popup above other apps unless it hides or drops back to a normal level on resign-active. If that isn’t intended for passkey/2FA/unlock flows, set hidesOnDeactivate = true or lower the level when cmux loses focus.
🤖 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/BrowserWebExtensionPopoutWindowController.swift` at line 43,
The popout window is forced to stay above other apps via window.level =
.floating in BrowserWebExtensionPopoutWindowController, so it needs to stop
lingering when cmux loses focus. Update the window setup to either set
hidesOnDeactivate = true or restore the level to a normal value when the
controller resigns active / deactivates, using the existing window configuration
code in BrowserWebExtensionPopoutWindowController.
| /// - the Bitwarden desktop app's bundled Safari web extension, when installed. | ||
| @available(macOS 15.4, *) | ||
| @MainActor | ||
| final class BrowserWebExtensionSupport: NSObject, ObservableObject { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Prefer @Observable over ObservableObject/@Published for this new state.
This is new cmux-owned SwiftUI state, and the modern shape is the @Observable macro with plain properties rather than ObservableObject + @Published. The toolbar surface (BrowserWebExtensionActionButton holding let support: BrowserWebExtensionSupport) can observe contexts/loadErrors directly under @Observable.
As per coding guidelines: "Do not introduce or materially expand ObservableObject, @Published, @StateObject, or @EnvironmentObject for new cmux-owned SwiftUI state when @Observable plus @State or value snapshots is the modern shape."
♻️ Sketch of the conversion
+import Observation
`@available`(macOS 15.4, *)
`@MainActor`
-final class BrowserWebExtensionSupport: NSObject, ObservableObject {
+@Observable
+final class BrowserWebExtensionSupport: NSObject {
static let shared = BrowserWebExtensionSupport()
@@
- `@Published` private(set) var contexts: [WKWebExtensionContext] = []
- `@Published` private(set) var loadErrors: [String] = []
+ private(set) var contexts: [WKWebExtensionContext] = []
+ private(set) var loadErrors: [String] = []Note: @Observable needs stored properties it should not track (e.g. didStartLoading, windowAdapter) marked @ObservationIgnored where appropriate; update SwiftUI consumers that used @ObservedObject/@StateObject.
Also applies to: 29-30
🧰 Tools
🪛 SwiftLint (0.65.0)
[Warning] 15-15: Classes should have an explicit deinit method
(required_deinit)
🤖 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/BrowserWebExtensionSupport.swift` at line 15, Replace the new
SwiftUI state in BrowserWebExtensionSupport with the modern `@Observable` pattern
instead of ObservableObject, and remove any `@Published` usage by making
contexts/loadErrors plain stored properties. Update the
BrowserWebExtensionActionButton consumer to observe the support object directly
without `@ObservedObject/`@StateObject, and mark non-observed internals such as
didStartLoading and windowAdapter with `@ObservationIgnored` where needed.
Source: Coding guidelines
| func focus(for context: WKWebExtensionContext, completionHandler: @escaping (Error?) -> Void) { | ||
| completionHandler(nil) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files and inspect the focus implementations.
git ls-files 'Sources/Panels/BrowserWebExtensionWindowAdapter.swift' 'Sources/Panels/*' 'Sources/**/Browser*Extension*' 'Sources/**/Popout*' | sed 's#^`#FILE` #'
echo
echo "== BrowserWebExtensionWindowAdapter.swift =="
wc -l Sources/Panels/BrowserWebExtensionWindowAdapter.swift
sed -n '1,220p' Sources/Panels/BrowserWebExtensionWindowAdapter.swift
echo
echo "== focus() implementations in Sources/Panels =="
rg -n "func focus\\(for: .*completionHandler" Sources/Panels -A 6 -B 4
echo
echo "== window focus helpers / hostWindow references =="
rg -n "makeKeyAndOrderFront|activate\\(ignoringOtherApps|hostWindow\\?" Sources/Panels Sources -A 3 -B 3Repository: manaflow-ai/cmux
Length of output: 11129
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== BrowserWebExtensionPopoutWindowController.swift =="
wc -l Sources/Panels/BrowserWebExtensionPopoutWindowController.swift
sed -n '1,220p' Sources/Panels/BrowserWebExtensionPopoutWindowController.swift
echo
echo "== BrowserWebExtensionSupport focus-related surface =="
rg -n "WKWebExtensionWindow|focus\\(for:|activeTabAdapter|orderedTabAdapters|hostWindow|makeKeyAndOrderFront|activate\\(ignoringOtherApps" Sources/Panels/BrowserWebExtensionSupport.swift Sources/Panels/BrowserWebExtensionWindowAdapter.swift Sources/Panels/BrowserWebExtensionPopoutWindowController.swift -A 4 -B 4
echo
echo "== all WKWebExtensionWindow conformances =="
rg -n "WKWebExtensionWindow" Sources -A 8 -B 4Repository: manaflow-ai/cmux
Length of output: 28845
focus(for:) should bring the host window forward
Sources/Panels/BrowserWebExtensionWindowAdapter.swift: this is the main-window counterpart to BrowserWebExtensionPopoutWindowController.focus(for:), but it only completes successfully. windows.update({ focused: true }) on the active browser window will no-op unless this activates hostWindow/NSApp first.
🤖 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/BrowserWebExtensionWindowAdapter.swift` around lines 47 - 49,
The main-window implementation of focus(for:) in
BrowserWebExtensionWindowAdapter currently returns success without actually
bringing the host window to the front. Update this method to activate hostWindow
and NSApp before completing, and then ensure the active browser window is marked
focused so windows.update({ focused: true }) can take effect. Use
BrowserWebExtensionWindowAdapter.focus(for:) as the place to apply the fix,
mirroring the behavior expected from
BrowserWebExtensionPopoutWindowController.focus(for:).
- Fire didActivateTab for the successor tab when the active browser panel closes, so extensions don't keep acting on the closed tab. - Register the extension popout window as a cmux auxiliary window (stable cmux.webExtensionPopout identifier) so ⌘W and the shared close-shortcut routing target it instead of workspace panels. - Center the popout whenever the fallback size is applied, not only for null/empty frames (small non-empty frames previously landed at 0,0). - Localize the popout window's fallback title. - Remove the timing-based debug probes (popup API probe, autoprobe, delayed re-diagnostic, per-keystroke unmatched-command log); the one-shot post-load diagnostic remains. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e48a0d5. Configure here.
| markExplicitFocusIntent(on: panelId) | ||
| if #available(macOS 15.4, *), panels[panelId] is BrowserPanel { | ||
| BrowserWebExtensionSupport.shared.noteActivated(panelID: panelId) | ||
| } |
There was a problem hiding this comment.
Extension active before focus guard
Low Severity
focusPanel calls BrowserWebExtensionSupport.shared.noteActivated before surfaceIdFromPanelId is validated. If focus setup returns early, WebKit can already have been told the tab activated even though cmux did not complete focusing that panel.
Reviewed by Cursor Bugbot for commit e48a0d5. Configure here.
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/BrowserWebExtensionPopoutWindowController.swift`:
- Around line 55-58: The popout window title fallback in
BrowserWebExtensionPopoutWindowController currently reuses the action-button
help localization key, which couples two unrelated strings. Update the title
assignment to use a dedicated window-title localization key instead of
browser.webExtension.action.help, and add that new key to
Resources/Localizable.xcstrings with translations for all supported locales so
the extension popup title can diverge from the toolbar help text.
🪄 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: b69b286d-38b2-4f1d-a939-dae2ab5dc23c
📒 Files selected for processing (3)
Sources/Panels/BrowserWebExtensionPopoutWindowController.swiftSources/Panels/BrowserWebExtensionSupport.swiftSources/cmuxApp.swift
| window.title = context.webExtension.displayName ?? String( | ||
| localized: "browser.webExtension.action.help", | ||
| defaultValue: "Extension" | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Window-title fallback reuses the action-button help key.
browser.webExtension.action.help is the toolbar action button's help/tooltip string, not a window title. Reusing it couples two unrelated strings: if the help text is ever retranslated (e.g. "Open extension popup"), the popout's title inherits that wording. Use a dedicated key so the two can diverge.
🔧 Suggested fix
- window.title = context.webExtension.displayName ?? String(
- localized: "browser.webExtension.action.help",
- defaultValue: "Extension"
- )
+ window.title = context.webExtension.displayName ?? String(
+ localized: "browser.webExtension.popout.defaultTitle",
+ defaultValue: "Extension"
+ )Add the new key to Resources/Localizable.xcstrings with translations for every supported locale.
🤖 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/BrowserWebExtensionPopoutWindowController.swift` around lines
55 - 58, The popout window title fallback in
BrowserWebExtensionPopoutWindowController currently reuses the action-button
help localization key, which couples two unrelated strings. Update the title
assignment to use a dedicated window-title localization key instead of
browser.webExtension.action.help, and add that new key to
Resources/Localizable.xcstrings with translations for all supported locales so
the extension popup title can diverge from the toolbar help text.
Source: Coding guidelines
|
Addressed the review findings in e48a0d5:
Acknowledged, deliberately deferred (tracked in the PR description's known gaps):
🤖 Generated with Claude Code |
|
Superseded by #7699, which now carries all of this PR's commits (plus review fixes and merges of latest main). Closing to keep review in one place. |
1 participant
Summary
Adds web-extension support to cmux's built-in browser using WebKit's native
WKWebExtension/WKWebExtensionControllerAPI (macOS 15.4+, fully availability-gated — deployment target stays 14.0). Motivated by discussion #2001 (1Password/extensions in the cmux browser).Verified end-to-end with Bitwarden 2026.2.0 (loaded from the Bitwarden desktop app's bundled Safari web extension): vault login in the toolbar popup, content-script autofill, the ⌘⇧L autofill command, and passkey sign-in confirmed via Bitwarden's popout window on a real site.
Unpacked (Chrome/Firefox-style) extensions also load via
CMUX_BROWSER_EXTENSIONS=/path/one:/path/two, mirroring agent-browser's--extensionergonomics.How it works
BrowserWebExtensionSupportowns one persistentWKWebExtensionController(fixed identifier, so extension storage survives relaunches) attached inBrowserPanel.configureWebViewConfiguration, and implements the controller delegate: action popups (WebKit's ownNSPopover), permission prompts (auto-grant for now),windows.createpopouts, native messaging, and keyboard commands.BrowserWebExtensionTabAdapter/BrowserWebExtensionWindowAdapterpresent browser panels to extensions as tabs of one virtual window (cmux has panes, not a tab strip);Workspace.focusPanelfires tab-activation events. Verified from inside the extension:tabs.query({active:true, currentWindow:true})returns the focused panel with correct URL/title.BrowserWebExtensionPopoutWindowControllerimplementswindows.create({type:"popup"})as a floating native window hosting the extension page — this is what makes Bitwarden's passkey confirmation, unlock, and 2FA popouts work.CmuxWebView.performKeyEquivalent(⌘-combos only, outside the typing-latency-sensitive path), and from the stale-menu-shortcut suppression insendEvent— ⌘⇧L is the remapped-away default of cmux's Open Browser action, and previously got swallowed app-wide.Sharp edges encoded in comments (found the hard way)
" Safari/"token — Bitwarden's UI crashes at boot classifying the browser. The controller configuration'swebViewConfigurationsetsapplicationNameForUserAgentto the same Safari identity browser panels present.runtime.sendNativeMessagewith an error: Bitwarden reconnect-loops with zero backoff (observed ~7M messages / 453 MB of debug log in 40 s). Requests are parked unresolved instead; consequence is biometric unlock is unavailable (master-password unlock works).Known gaps / follow-ups
tabs.createfor extension pages is unimplemented (http/https URLs open externally).pluginkit, load-unpacked picker, enable/disable).Verification
Tagged Debug build (
reload.sh --tag bitwarden-ext), dogfooded live: extension load diagnostics clean (injected=1 background=1, no context/extension errors), popup boots, passkey login succeeded on a production site, ⌘⇧L routed and handled (debug-log traced end-to-end).🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Touches global event routing and grants extension permissions without UI; behavior is gated to macOS 15.4+ but autofill/passkey flows depend on WebKit extension APIs and third-party extension quirks.
Overview
Adds native web extension hosting in the built-in browser via WebKit
WKWebExtensionController(macOS 15.4+, availability-gated). A shared persistent controller attaches to every browserWKWebViewConfiguration; extensions load fromCMUX_BROWSER_EXTENSIONSpaths and Bitwarden’s bundled Safari.appexwhen present.Browser panels are exposed to extensions as tabs in one virtual window (
BrowserWebExtensionTabAdapter/BrowserWebExtensionWindowAdapter), with activation wired through panel register/unregister andWorkspace.focusPanel. Toolbar buttons trigger extension actions with anchored popovers;windows.createpopups useBrowserWebExtensionPopoutWindowController(registered as an auxiliary close-shortcut window).Keyboard routing offers manifest-declared commands from
CmuxWebViewand, when a browser web view is focused, from stale menu-shortcut suppression insendEventviacmuxRespondersContainBrowserWebView+performCommand(e.g. ⌘⇧L after Open Browser is remapped). Extension web views get a Safari-style user agent; native messaging is intentionally not replied to avoid reconnect storms; permissions are auto-granted for now.Adds localized
browser.webExtension.action.helpand Xcode project entries for the new panel sources.Reviewed by Cursor Bugbot for commit e48a0d5. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Add native browser extension support to the built-in browser using WebKit’s
WKWebExtensionon macOS 15.4+, verified end-to-end with Bitwarden (popup, autofill, ⌘⇧L, passkeys). Also supports loading unpacked extensions viaCMUX_BROWSER_EXTENSIONS, and fixes tab-activation after close, localized popout titles, and proper ⌘W handling for popouts.New Features
WKWebExtensionControlleracross browser views; storage survives relaunches.CMUX_BROWSER_EXTENSIONS.windows.createpopouts for extension pages (passkeys, unlock, 2FA).tabs.createfor extension pages is not implemented; native messaging is not bridged; permissions are auto-granted and there’s no in-app management yet.Bug Fixes
Written for commit e48a0d5. Summary will update on new commits.
Summary by CodeRabbit