Repository navigation
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>
- 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>
Replaces the hardcoded Bitwarden path with a persisted, user-managed
extension registry:
- New browser.webExtensions JSONKey ([BrowserWebExtensionEntry]) in the
CmuxSettings catalog; entries carry id/kind/path/enabled and fail
closed on malformed config.
- Extensions card in the Browser settings section: every Safari web
extension installed on the Mac (discovered via pluginkit, the registry
Safari itself consults) with an enable toggle, unpacked-directory
entries with remove buttons, and an "Add Unpacked…" folder picker.
- BrowserWebExtensionSupport now observes the JSON key from app startup
and diffs desired vs. loaded: enabling loads live, disabling unloads
live, and extensions begin loading before the first browser page
navigates (shrinking the session-restore content-script race).
- One-time migration seeds the Bitwarden entry when the desktop app is
installed and no extensions were ever configured, so existing setups
keep working; CMUX_BROWSER_EXTENSIONS env paths still merge in.
- Curated settings-search entry ("extensions", "bitwarden", "addons")
and localized strings for all 19 languages.
Verified live: seeding wrote browser.webExtensions to cmux.json on first
launch, Bitwarden loaded at startup, toggling enabled=false unloaded the
extension within seconds and re-enabling reloaded it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Extensions card now shows only extensions the user explicitly added. Discovered Safari web extensions move behind an "Import Safari Extension" menu (listing installed-but-not-added extensions); imported entries capture a display name so rows stay readable across launches. Unpacked folders keep the Add Unpacked… picker. Every row has a Remove button. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…urce Extensions now load exclusively from browser.webExtensions entries (plus the CMUX_BROWSER_EXTENSIONS env override). The one-time Bitwarden grandfather-seed made sense while the path was hardcoded; with the import-based settings UI, nothing is added without an explicit user action. Bitwarden remains only in comments, doc examples, and settings- search keywords. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Settings entries win over CMUX_BROWSER_EXTENSIONS for the same path (standardized-path comparison), so a disabled toggle sticks and the same extension can't load twice under two ids. - Unloading an extension closes its open popout windows instead of leaving orphaned extension web views. - Discard pluginkit stderr instead of piping it undrained, which could block the child process and hang discovery. - Drop unused discoveredWebExtensions state from BrowserSection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- sendEvent's web-extension command routing runs in the swizzled NSApplication method, where AppDelegate's shortcutEventBrowserPanel helper is out of scope — call through AppDelegate.shared. - BrowserWebExtensionSupport.deinit called the @mainactor permission- observer removal helper from a nonisolated context; block-based NotificationCenter tokens auto-unregister on dealloc, so drop the call. - permissionMessage's key parameter must be StaticString to match the String(localized:defaultValue:) overload. Verified: cmux and cmux-unit schemes both build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Require a primary modifier before scanning all shortcut actions in the web-extension command routing hot path - Preserve developer tools visibility intent across web-extension webview replacements - Propagate the web-extension navigation context to page-initiated new tabs only when the target stays in the same extension context - Coalesce tab metadata change notifications and report only properties that actually changed; surface tab mute state to extension contexts Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
StateObject(wrappedValue:) takes an escaping autoclosure, which cannot capture mutating self in the App initializer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The fake host returned one shared WKWebViewConfiguration for every extension-page navigation, so the sibling-tab panel re-added fixed-name script message handlers (cmuxReactGrab) to the same user content controller — WebKit throws NSInvalidArgumentException and the app host crashes. Production hands out a configuration per navigation via WKWebExtensionContext.webViewConfiguration; mirror that. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e is active Focus mode suspends every configured cmux shortcut at the app-level monitor, but the web-extension command offer still declined events those suspended shortcuts claim, and the focus-mode branch of CmuxWebView.performKeyEquivalent force-consumed command equivalents for the page before the offer ran. Net effect: with the default ⌘⇧L Open Browser binding, Bitwarden's autofill command could never fire. Now manifest commands are offered first in the focus-mode forward path (browser-chrome shortcuts outrank page content, as in Safari), and the conflict scan is skipped when focus mode owns the event. Outside focus mode nothing changes: configured cmux shortcuts still win. The focus-mode key-equivalent handling moves to CmuxWebViewFocusModeKeyEquivalent.swift so CmuxWebView.swift stays inside its file-length budget.
…e-shortcut lint The identifier owner list moved from cmuxApp.swift to Sources/App/AuxiliaryWindowCloseShortcut.swift in the file-length budget refactor; the lint hardcodes the owner-list path, so workflow-guard-tests (and the linux-preflight/tests/ci-status gates behind it) went red. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
terminateActivePluginkitProcess() is synchronous and this call site is already actor-isolated, so the await performs no async work and trips the warning budget (tests-build-and-lag). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Extensions card showed the empty state whenever entries were empty, but before JSONValueModel's stream delivers its first value the entries are only the default [] — configured extensions briefly appeared missing. Gate the empty row on hasObservedValue, matching the Import menu's existing gate, and cover it in BrowserWebExtensionsCardStateTests. Addresses the Cursor Bugbot finding on BrowserWebExtensionsCard.swift. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@0xCUB3 is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds configurable macOS browser WebExtension support, including settings UI, Safari discovery, WebKit loading and permissions, context-aware navigation, toolbar actions, shortcut routing, lifecycle wiring, and regression tests. ChangesBrowser WebExtension integration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (5 errors, 2 warnings)
✅ Passed checks (18 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 |
Contributor
Greptile SummaryThis PR adds user-managed browser extensions to the built-in browser. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (4): Last reviewed commit: "Fix browser extension review regressions" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| /// Creates the app-wide web-extension host at launch on OS versions that | ||
| /// support `WKWebExtensionController`; returns nil elsewhere. | ||
| @MainActor | ||
| func makeBrowserWebExtensionHostAtLaunch( |
Contributor
There was a problem hiding this comment.
This adds a file-scope factory for app runtime behavior in production source. The host is owned by app bootstrap state and depends on the settings runtime, so leaving construction in a global function grows an ambient API surface instead of keeping the lifecycle on the app/bootstrap owner or an injectable factory.
Rule Used: Flag new ambient global state in production Swift:... (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!
Author
There was a problem hiding this comment.
Agreed. I moved host construction onto cmuxApp as a private static factory in b0fff2f130847c0ca146cded0b06fd6881a8afc3, so the app composition root now owns that lifecycle without exposing a file-scope factory.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5278d258f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _ controller: WKWebExtensionController, | ||
| focusedWindowFor extensionContext: WKWebExtensionContext | ||
| ) -> (any WKWebExtensionWindow)? { | ||
| focusedWebExtensionWindow(for: NSApp.keyWindow) |
There was a problem hiding this comment.
When two extensions are loaded and extension B's popout is the key window, this delegate ignores the extensionContext being queried and returns that popout to extension A. openWindowsFor already filters popouts by context, but then prepends the focused window even when it is not in A's window list, so A's window/tab APIs can act on another extension's popup. Please only return a popout here when it belongs to the requested context, otherwise fall back to the browser window or nil.
Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment.
Fixed in 8d0d68df0d396cc32169bf12a93ed2982138d3b7. Focused-window resolution now filters against the windows belonging to the requested extension context, so a popout from another extension is not exposed. I added coverage for the allowed and excluded window paths.
| informativeText: permissionMessage( | ||
| extensionContext: extensionContext, | ||
| details: requested, | ||
| key: "browser.webExtension.permissionPrompt.permissions.message", |
There was a problem hiding this comment.
When an extension requests permissions, these new browser.webExtension.permissionPrompt.* keys are not present in Resources/Localizable.xcstrings (same for the adjacent title/Allow/Deny keys), so non-English users will see the English defaultValue in a user-facing permission dialog despite the repo's localization contract. Please add these keys with translations before shipping the prompt.
Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment.
Fixed in 8d0d68df0d396cc32169bf12a93ed2982138d3b7. All six permission prompt keys now include translations for every locale already present in the catalog, with the %@ placeholders preserved.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🤖 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
`@Packages/macOS/CmuxSettings/Sources/CmuxSettings/Values/BrowserWebExtensionEntry.swift`:
- Around line 67-100: Update the shared standardizedPath helper to resolve
symlinks before standardizing the URL, then ensure standardizedResourceRootPath
and standardizedSafariAppExtensionResourceRootPath use that resolved path
consistently for both unpacked directories and Safari app extensions.
In
`@Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/BrowserWebExtensionsCardState.swift`:
- Around line 38-50: The reconcileObservedEntries method must not clear
pendingWriteID or optimistic state based solely on entries value equality, since
delayed observations can match newer edits. Add and propagate an observed
revision/write token, or otherwise require a causally newer observation tied to
the matching write before acknowledging it; retain pending state until
reconcileWriteResult confirms successful completion, while still handling
matching failures to set hasWriteError.
- Around line 4-51: Mark BrowserWebExtensionsCardState as nonisolated,
preserving its current stored properties and methods. Verify the declaration
compiles under Swift 6 and that its Sendable stored types satisfy concurrency
requirements.
In
`@Packages/macOS/CmuxSettingsUI/Tests/CmuxSettingsUITests/JSONValueModelLifecycleTests.swift`:
- Around line 105-110: Replace the fixed 100,000-iteration Task.yield loops
around model.hasObservedValue with the test suite’s deadline-bounded polling
helper, or add and await an explicit completion signal; update both affected
locations so assertions wait on real predicate completion rather than
scheduler-dependent spin counts.
In `@Resources/Localizable.xcstrings`:
- Around line 235961-236209: Remove the later duplicate localization entries for
settings.browser.webExtensions.duplicate.title and
settings.browser.webExtensions.duplicate.message from the provided block,
preserving the earlier definitions and their translations.
In `@Sources/AppDelegate`+BrowserWebExtensionShortcutRouting.swift:
- Around line 74-79: Update the modifier check in the keyboard shortcut routing
logic to use Set.isDisjoint(with:) instead of constructing
flags.intersection([.command, .control, .option]) and checking isEmpty; preserve
the guard’s behavior of returning false when no primary modifier is present.
In `@Sources/Panels/BrowserDevToolsIconOptions.swift`:
- Around line 24-43: Localize the picker labels in the title properties for both
icon and color options in BrowserDevToolsIconOptions: replace each hard-coded
English return value with String(localized:defaultValue:) using stable
localization keys, and add matching catalog entries for every supported locale
covering all cases.
In `@Sources/Panels/BrowserExternalNavigationPolicy.swift`:
- Around line 3-67: Move the top-level scheme set and routing functions into a
constructable, injectable BrowserExternalNavigationPolicy owned by the
browser-navigation layer. Convert browserShouldOpenURLExternally,
browserShouldRouteExternalNavigation, browserIntentFallbackURL, and
browserExternalNavigationAction into instance methods, keep the action/result
types appropriately scoped, and update all callers to receive and use the policy
instance instead of ambient globals.
In `@Sources/Panels/BrowserPanel.swift`:
- Around line 5756-5766: In navigateFromWebExtension(to:webViewConfiguration:),
only call replaceWebViewForWebExtensionNavigation when contextIdentifier differs
from webExtensionPageContextIdentifier; preserve the existing WebView for
matching extension contexts, consistent with
ensureWebExtensionNavigationConfiguration.
In `@Sources/Panels/BrowserWebExtensionDiscoveryService.swift`:
- Around line 30-31: Make discovery state reentrancy-safe: refactor
discoverInstalledSafariExtensions and its timeout/cancellation cleanup so each
invocation owns its own Process and Pipe, or coalesce concurrent calls through a
shared discovery Task. Do not overwrite activePluginkitProcess or
activePluginkitStdout between invocations, and ensure any shared task/state is
cleared only after the discovery operation fully completes.
In `@Sources/Panels/BrowserWebExtensionNavigationConfiguration.swift`:
- Around line 18-40: Make shouldBlockPageInitiatedWebExtensionNavigation and
shouldRoutePageInitiatedWebExtensionNavigationInCurrentTab mutually exclusive
for exit URLs targeting a different extension context. Adjust the blocking
predicate to exclude URLs satisfying the current-tab routing conditions, or
otherwise ensure only one method returns true, so BrowserNavigationDelegate can
route the exit instead of canceling it first.
In `@Sources/Panels/BrowserWebExtensionReconciliationPlanner.swift`:
- Around line 113-128: Build settingsPaths using only enabled entries, matching
the filtering already used for desired entries in the reconciliation planner.
Update the settingsEntries mapping near seenDesiredPaths so disabled entries are
excluded, allowing environment paths to override them; use
standardizedResourceRootPath(for:) consistently.
In `@Sources/Panels/BrowserWebExtensionSupport`+Loading.swift:
- Around line 153-160: Sanitize extension failures in the unload handler and the
corresponding code at the other referenced locations: replace raw
error.localizedDescription values passed to recordLoadError with localized,
product-safe messages, and update cmuxDebugLog diagnostics to include only the
error type and a stable redacted hash rather than paths, names, versions, entry
IDs, or upstream messages. Use shared sanitization helpers where available and
ensure all UI and debug error paths follow the same policy.
In
`@Sources/Panels/BrowserWebExtensionSupport`+WKWebExtensionControllerDelegate.swift:
- Around line 134-136: Replace the raw URL logging in the browser web extension
tab-opening debug block with non-sensitive metadata, such as the URL scheme,
host, or byte length, following the existing omnibar suggestion logging pattern;
do not log any URL path, query, fragment, or truncated URL content.
In `@Sources/Panels/BrowserWebExtensionTabAdapter.swift`:
- Around line 89-106: Update reload, goBack, and goForward to guard against a
nil panel before performing navigation. When panel is unavailable, call
completionHandler with the same unavailable-tab error used by loadURL and close;
otherwise execute the existing webView action and complete with nil.
In `@Sources/Panels/BrowserWebExtensionToolbarButtons.swift`:
- Around line 42-48: Remove the extension-supplied snapshot.displayName from the
cmuxDebugLog call in the performAction flow, retaining only safe metadata such
as the panel UUID prefix; do not log the display name even in DEBUG builds, or
mark it private if it must be included.
In `@Sources/Panels/BrowserWebExtensionWindowAdapter.swift`:
- Around line 47-66: Update the private hostWindow property in
BrowserWebExtensionWindowAdapter to return only
support?.activeTabAdapter?.panel?.webView.window, removing the NSApp.keyWindow
fallback so focus(for:completionHandler:) fails with its existing
no-browser-window error when no browser panel is available.
In `@Sources/Panels/CmuxWebView.swift`:
- Around line 673-675: The web-extension shortcut check currently runs before
verifying the Command modifier, causing unnecessary browser context resolution
for ordinary typing. In the event-handling method containing
cmuxPerformBrowserWebExtensionCommandKeyEquivalent(_:), add a fast precheck for
flags.contains(.command) and only invoke the web-extension command handler when
it passes, before returning finish(true).
In `@Sources/Workspace`+PendingTerminalInput.swift:
- Around line 4-6: Add a brief comment immediately above
WorkspacePendingTerminalInputObserver documenting that its mutable observer
property is confined to the main queue/main actor: registration uses queue:
.main, accesses occur only in main-actor Tasks, and removal clears it there,
justifying `@unchecked` Sendable.
🪄 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: 220f0ab8-8569-458e-8cdf-bb578226de4c
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (65)
Packages/macOS/CmuxSettings/Sources/CmuxSettings/Keys/BrowserCatalogSection.swiftPackages/macOS/CmuxSettings/Sources/CmuxSettings/Values/BrowserWebExtensionEntry.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Bindings/JSONValueModel.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Environment/SettingsDiscoveredBrowserExtension.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Environment/SettingsHostActions.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry+Default.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Scene/SettingsWindowScene.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/BrowserSection.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/BrowserWebExtensionsCard.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/BrowserWebExtensionsCardState.swiftPackages/macOS/CmuxSettingsUI/Tests/CmuxSettingsUITests/BrowserWebExtensionsCardStateTests.swiftPackages/macOS/CmuxSettingsUI/Tests/CmuxSettingsUITests/JSONValueModelLifecycleTests.swiftPackages/macOS/CmuxSettingsUI/Tests/CmuxSettingsUITests/SettingsRowAnchorResolutionTests.swiftResources/Localizable.xcstringsSources/App/AuxiliaryWindowCloseShortcut.swiftSources/AppDelegate+BrowserWebExtensionShortcutRouting.swiftSources/AppDelegate+WindowDock.swiftSources/AppDelegate.swiftSources/CmuxNotificationNames.swiftSources/DockSplitStore+PaneFocus.swiftSources/DockSplitStore.swiftSources/HostSettingsActions.swiftSources/KeyboardShortcutContext.swiftSources/Panels/BrowserDevToolsIconOptions.swiftSources/Panels/BrowserExternalNavigationPolicy.swiftSources/Panels/BrowserNavigationDelegate.swiftSources/Panels/BrowserNavigationPopupPolicy.swiftSources/Panels/BrowserPanel+PrewarmedWebViewAdoption.swiftSources/Panels/BrowserPanel.swiftSources/Panels/BrowserPanelDeveloperToolsLifecycle.swiftSources/Panels/BrowserPanelView.swiftSources/Panels/BrowserPrewarmedWebViewPool.swiftSources/Panels/BrowserWebExtensionActionSnapshot.swiftSources/Panels/BrowserWebExtensionActionSnapshotInvalidation.swiftSources/Panels/BrowserWebExtensionDiscoveryService.swiftSources/Panels/BrowserWebExtensionHosting.swiftSources/Panels/BrowserWebExtensionLoadedRecord.swiftSources/Panels/BrowserWebExtensionNavigationConfiguration.swiftSources/Panels/BrowserWebExtensionPermissionState.swiftSources/Panels/BrowserWebExtensionPermissionStateStore.swiftSources/Panels/BrowserWebExtensionPopoutWindowController.swiftSources/Panels/BrowserWebExtensionReconciliationPlanner.swiftSources/Panels/BrowserWebExtensionSupport+Loading.swiftSources/Panels/BrowserWebExtensionSupport+Permissions.swiftSources/Panels/BrowserWebExtensionSupport+WKWebExtensionControllerDelegate.swiftSources/Panels/BrowserWebExtensionSupport.swiftSources/Panels/BrowserWebExtensionTabAdapter.swiftSources/Panels/BrowserWebExtensionToolbarButtons.swiftSources/Panels/BrowserWebExtensionWindowAdapter.swiftSources/Panels/CmuxWebView.swiftSources/Panels/CmuxWebViewFocusModeKeyEquivalent.swiftSources/PricingPlansScreen.swiftSources/TabManager+DetachedWorkspace.swiftSources/TabManager.swiftSources/Workspace+DockBrowserLookup.swiftSources/Workspace+PendingTerminalInput.swiftSources/Workspace.swiftSources/cmuxApp.swiftcmux.xcodeproj/project.pbxprojcmuxTests/BrowserDeveloperToolsLifecycleTests.swiftcmuxTests/BrowserPrewarmedWebViewPoolTests.swiftcmuxTests/BrowserWebExtensionReviewRegressionTests.swiftcmuxTests/BrowserWebExtensionSupportTests.swiftscripts/lint_auxiliary_window_close_shortcuts.pytests/test_ci_auxiliary_window_close_shortcuts.sh
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort 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 b0fff2f. 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 `@Resources/Localizable.xcstrings`:
- Around line 44941-45190: The permission-prompt localization keys do not match
the runtime lookup keys, and the same mismatch exists in the additional affected
entries. Align all settings.browser.webExtension.permissionPrompt.* catalog keys
with the browser.webExtension.permissionPrompt.* keys requested by the
web-extension controller delegate, including title, allow, and deny, or update
every caller consistently to the settings-prefixed family.
🪄 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: bb277b53-bd18-41ff-a0b9-19fa6c390eae
📒 Files selected for processing (4)
Resources/Localizable.xcstringsSources/Panels/BrowserWebExtensionSupport+WKWebExtensionControllerDelegate.swiftSources/Panels/BrowserWebExtensionSupport.swiftcmuxTests/BrowserWebExtensionSupportTests.swift
Comment on lines
44941
to
45190
| }, | ||
| "fr": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "sidebar.notificationBadgePosition notification unread badge position left right leading trailing side workspace chargement indicateur gauche droite position" | ||
| "value": "Refuser" | ||
| } | ||
| }, | ||
| "it": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "sidebar.notificationBadgePosition notification unread badge position left right leading trailing side workspace caricamento indicatore sinistra destra posizione" | ||
| "value": "Nega" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "sidebar.notificationBadgePosition notification unread badge position left right leading trailing side workspace 通知 未読 バッジ 位置 左 右 サイド" | ||
| "value": "拒否" | ||
| } | ||
| }, | ||
| "km": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "បដិសេធ" | ||
| } | ||
| }, | ||
| "ko": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "sidebar.notificationBadgePosition notification unread badge position left right leading trailing side workspace 로딩 스피너 왼쪽 오른쪽 위치" | ||
| "value": "거부" | ||
| } | ||
| }, | ||
| "nb": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "sidebar.notificationBadgePosition notification unread badge position left right leading trailing side workspace lasting indikator venstre høyre plassering" | ||
| "value": "Avslå" | ||
| } | ||
| }, | ||
| "pl": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "sidebar.notificationBadgePosition notification unread badge position left right leading trailing side workspace ładowanie wskaźnik lewo prawo pozycja" | ||
| "value": "Odmów" | ||
| } | ||
| }, | ||
| "pt-BR": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "sidebar.notificationBadgePosition notification unread badge position left right leading trailing side workspace carregamento indicador esquerda direita posição" | ||
| "value": "Negar" | ||
| } | ||
| }, | ||
| "ru": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "sidebar.notificationBadgePosition notification unread badge position left right leading trailing side workspace загрузка индикатор слева справа положение" | ||
| "value": "Запретить" | ||
| } | ||
| }, | ||
| "th": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "sidebar.notificationBadgePosition notification unread badge position left right leading trailing side workspace โหลด ตัวบ่งชี้ ซ้าย ขวา ตำแหน่ง" | ||
| "value": "ปฏิเสธ" | ||
| } | ||
| }, | ||
| "tr": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "sidebar.notificationBadgePosition notification unread badge position left right leading trailing side workspace yükleme gösterge sol sağ konum" | ||
| "value": "Reddet" | ||
| } | ||
| }, | ||
| "uk": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "sidebar.notificationBadgePosition notification unread badge position left right leading trailing side workspace завантаження індикатор ліворуч праворуч розташування" | ||
| "value": "Заборонити" | ||
| } | ||
| }, | ||
| "zh-Hans": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "sidebar.notificationBadgePosition notification unread badge position left right leading trailing side workspace 加载 指示器 左 右 位置" | ||
| "value": "拒绝" | ||
| } | ||
| }, | ||
| "zh-Hant": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "sidebar.notificationBadgePosition notification unread badge position left right leading trailing side workspace 載入 指示器 左 右 位置" | ||
| "value": "拒絕" | ||
| } | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Align permission-prompt catalog keys with their runtime lookups.
Sources/Panels/BrowserWebExtensionSupport+WKWebExtensionControllerDelegate.swift:446-461 requests browser.webExtension.permissionPrompt.title, .allow, and .deny, but this catalog defines settings.browser.webExtension.permissionPrompt.*. These entries are therefore not used, so localized users fall back to the English defaults.
Rename the catalog keys or update every caller so both sides use one exact key family.
Also applies to: 45441-45565
🤖 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 `@Resources/Localizable.xcstrings` around lines 44941 - 45190, The
permission-prompt localization keys do not match the runtime lookup keys, and
the same mismatch exists in the additional affected entries. Align all
settings.browser.webExtension.permissionPrompt.* catalog keys with the
browser.webExtension.permissionPrompt.* keys requested by the web-extension
controller delegate, including title, allow, and deny, or update every caller
consistently to the settings-prefixed family.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
cmuxTests/BrowserWebExtensionReviewRegressionTests.swift (1)
181-200: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winTwo
Task.yield()calls are a fixed-count proxy, not a real completion signal, for a negative assertion.
unchangedMetadataDoesNotInvalidateExtensionActionsis a negative test (asserting revision does not change). IfnoteTabMetadataChangedschedules invalidation work that takes more than two scheduler hops to complete, this test would pass even with a regression that eventually does bump the revision — it just wouldn't have run yet by the time the assertion executes. Prefer a bounded poll ofinvalidation.revision(with a short deadline) or a real completion signal from the invalidation path instead of a fixed yield count.As per path instructions for
{cmuxTests,...}/**: "Tests must await real completion signals or deadline-bounded polls of real predicates rather than fixed-duration waits before assertions."🤖 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 `@cmuxTests/BrowserWebExtensionReviewRegressionTests.swift` around lines 181 - 200, Replace the two fixed Task.yield() calls in unchangedMetadataDoesNotInvalidateExtensionActions with a deadline-bounded poll of the real invalidation predicate, waiting briefly for invalidation.revision to change or until the short timeout expires. Keep the final assertion that the revision remains initial, and use the existing invalidation state as the completion condition rather than scheduler-hop counting.Source: Path instructions
Sources/Panels/BrowserPanel.swift (1)
5756-5768: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep the extension navigation context enabled.
Line 5768 calls
navigate(to:)withoutallowWebExtensionContext: true. This re-enters the normal navigation gate with its default disabled context, so an already validatedwebkit-extension:URL fromWKWebExtensionTab.loadURLcan be blocked while the adapter reports success.Proposed fix
- navigate(to: url) + navigate(to: url, allowWebExtensionContext: true)Add an assertion that the sibling extension URL actually navigates, not just that the WebView identity is preserved.
🤖 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/BrowserPanel.swift` around lines 5756 - 5768, Update navigateFromWebExtension(to:webViewConfiguration:) to call navigate(to:allowWebExtensionContext:) with allowWebExtensionContext set to true, preserving the validated extension context. Add an assertion verifying that the sibling extension URL navigation succeeds, in addition to confirming the WebView identity remains preserved.Sources/Panels/BrowserWebExtensionToolbarButtons.swift (1)
30-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSources/Panels/BrowserWebExtensionToolbarButtons.swift:30-36 — Remove the unused
panelproperty fromBrowserWebExtensionActionButton.BrowserPanelis anObservableObject, and the row doesn’t read it; keep the row on snapshot data plus the action closure only, with the parent capturingpanelfor the action call.🤖 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/BrowserWebExtensionToolbarButtons.swift` around lines 30 - 36, Remove the unused panel property from BrowserWebExtensionActionButton and update its initializer and call sites to pass only snapshot, iconPointSize, hitSize, and performAction; keep any panel usage in the parent closure that invokes the action.Source: Path instructions
🤖 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/BrowserWebExtensionSupport`+Loading.swift:
- Around line 338-340: The user-facing failure message in
BrowserWebExtensionDiagnostics.genericFailureMessage is hardcoded in English.
Replace it with a stable localization key resolved through the project’s
localization mechanism, then add the corresponding key and translations to
Localizable.xcstrings.
---
Outside diff comments:
In `@cmuxTests/BrowserWebExtensionReviewRegressionTests.swift`:
- Around line 181-200: Replace the two fixed Task.yield() calls in
unchangedMetadataDoesNotInvalidateExtensionActions with a deadline-bounded poll
of the real invalidation predicate, waiting briefly for invalidation.revision to
change or until the short timeout expires. Keep the final assertion that the
revision remains initial, and use the existing invalidation state as the
completion condition rather than scheduler-hop counting.
In `@Sources/Panels/BrowserPanel.swift`:
- Around line 5756-5768: Update
navigateFromWebExtension(to:webViewConfiguration:) to call
navigate(to:allowWebExtensionContext:) with allowWebExtensionContext set to
true, preserving the validated extension context. Add an assertion verifying
that the sibling extension URL navigation succeeds, in addition to confirming
the WebView identity remains preserved.
In `@Sources/Panels/BrowserWebExtensionToolbarButtons.swift`:
- Around line 30-36: Remove the unused panel property from
BrowserWebExtensionActionButton and update its initializer and call sites to
pass only snapshot, iconPointSize, hitSize, and performAction; keep any panel
usage in the parent closure that invokes the action.
🪄 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: 2277e9c3-6037-451d-8cc1-8dece27cf3c3
📒 Files selected for processing (14)
Packages/macOS/CmuxSettings/Sources/CmuxSettings/Values/BrowserWebExtensionEntry.swiftPackages/macOS/CmuxSettings/Tests/CmuxSettingsTests/BrowserWebExtensionEntryTests.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Bindings/JSONValueModel.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/BrowserWebExtensionsCard.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/BrowserWebExtensionsCardState.swiftPackages/macOS/CmuxSettingsUI/Tests/CmuxSettingsUITests/BrowserWebExtensionsCardStateTests.swiftSources/AppDelegate+BrowserWebExtensionShortcutRouting.swiftSources/Panels/BrowserPanel.swiftSources/Panels/BrowserWebExtensionSupport+Loading.swiftSources/Panels/BrowserWebExtensionTabAdapter.swiftSources/Panels/BrowserWebExtensionToolbarButtons.swiftSources/Panels/BrowserWebExtensionWindowAdapter.swiftcmuxTests/BrowserWebExtensionReviewRegressionTests.swiftcmuxTests/BrowserWebExtensionSupportTests.swift
Comment on lines
+338
to
+340
| enum BrowserWebExtensionDiagnostics { | ||
| static let genericFailureMessage = "Web extension failed" | ||
| enum Operation: String { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Localize the surfaced extension failure.
Line 339 reaches the user-facing loadErrors state, but uses a bare English literal. Use a stable localization key and add translations to Resources/Localizable.xcstrings.
🤖 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`+Loading.swift around lines 338 -
340, The user-facing failure message in
BrowserWebExtensionDiagnostics.genericFailureMessage is hardcoded in English.
Replace it with a stable localization key resolved through the project’s
localization mechanism, then add the corresponding key and translations to
Localizable.xcstrings.
Source: Coding guidelines
5 of 6 tasks
teamleaderleo
added
area: browser
Collaborator
|
This remains a broad browser-extension feature with a substantial security and interaction surface. Current main has significant conflicts, so I’m leaving it open with |
This branch has not been 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
5 participants
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.
WKWebExtensionon macOS 15.4+, including toolbar actions, tabs, windows, popouts, native messaging, and cmux lifecycle routing.Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
High Risk
Loads third-party extension code into WebKit with persisted filesystem paths and changes global keyboard routing in the browser, which is a broad security and interaction surface despite the macOS 15.4 gate.
Overview
Introduces user-managed web extensions for the built-in browser: a new
browser.webExtensionsentry incmux.json, theBrowserWebExtensionEntrymodel (Safari.appexvs unpacked folders, enable flag, standardized paths for deduping), and a Browser settings Extensions card to import discovered Safari extensions, add unpacked folders, toggle enablement, remove entries, and show host-reported load failures.Settings infrastructure gains
JSONValueModelwrite IDs, serialized writes,hasObservedValue/observationRevision, and card state so the UI does not flash an empty list or acknowledge stale observations during rapid edits.SettingsHostActionsadds discovery, macOS 15.4 capability checks, and a live load-error stream;HostSettingsActionsimplements Safari discovery and surfaces errors from the app extension host.App integration threads a shared
browserWebExtensionHostthroughTabManager, window docks, andDockSplitStorebrowser panels (registration, activation on focus, optional customWKWebViewConfiguration). Keyboard handling is extended so manifest extension commands can run when browser focus mode is active or when stale menu shortcuts would otherwise swallow them, only after configured cmux shortcuts do not claim the stroke. Auxiliary close-shortcut ownership now includescmux.webExtensionPopout. Localization and tests cover settings reconciliation, resource identity, and JSON model lifecycle.Reviewed by Cursor Bugbot for commit f2e19f1. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds user-managed web extensions to the built-in browser with Safari/unpacked import, live enable/disable, and full WebKit hosting on macOS 15.4+. Includes toolbar popups, tabs/windows, native messaging, and keyboard commands that work in focus mode and when defaults are remapped.
New Features
browser.webExtensionsentries and Enable/Remove. Import Safari extensions or Add Unpacked; de‑dupes paths, hides already‑imported items, waits for settings and discovery, and surfaces live load errors.CMUX_BROWSER_EXTENSIONSpaths are merged and de‑duplicated, and settings entries override duplicates. Toolbar action buttons with anchored popups; extension tabs/windows (including popouts); prewarmed web views attach the extension host for instant first render; native messaging supported.Bug Fixes
webkit-extension:or cross‑context navigations; propagate extension context to page‑initiated new tabs only when eligible; coalesce tab metadata updates and expose mute state.Written for commit f2e19f1. Summary will update on new commits.
Summary by CodeRabbit