Repository navigation
Conversation
|
@jcrsilva is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds per-URL Command-modifier passthrough so allowlisted embedded pages receive Cmd-key equivalents first; integrates the policy into AppDelegate and CmuxWebView, expands browser arrow-key routing, updates settings parsing/schema/example, adds tests, and localizes docs. ChangesPer-URL Cmd Shortcut Passthrough with Arrow Routing
Sequence DiagramsequenceDiagram
participant AppDelegate
participant CmuxWebView
participant BrowserPolicy
participant WebKit
AppDelegate->>CmuxWebView: key-equivalent event / focused webView check
CmuxWebView->>BrowserPolicy: shouldPassthroughCommandEquivalentToWebContent(event,responder,url)
BrowserPolicy-->>CmuxWebView: allowlist result (true/false)
alt allowlisted && Cmd-modifier
CmuxWebView->>WebKit: super.performKeyEquivalent(with:)
WebKit-->>CmuxWebView: consumed? (true/false)
alt consumed
CmuxWebView-->>AppDelegate: consume (suppress menu)
else not consumed
AppDelegate-->>WebKit: fallback menu/AppKit handling
end
else not allowlisted
AppDelegate-->>WebKit: normal cmux shortcut handling / menu dispatch
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (15 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 |
Author
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
@jcrsilva I have started the AI code review. It will take a few minutes to complete. |
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
|
To use Codex here, create a Codex account and connect to github. |
|
Tip For best results, initiate chat on the files or code changes.
🔴 Critical: Cmd+Q / Cmd+W / app-level shortcuts can be silently swallowedIn let result = firstResponderWebView.performKeyEquivalent(with: event)
// …
// Always return true so AppKit does not fall through to the main menu …
return trueThis means if WebKit doesn't handle the event (e.g. Cmd+Q, Cmd+W, Cmd+M, Cmd+`, Cmd+Tab), the event is swallowed entirely — the app cannot be quit while a passthrough-host URL is focused. A minimal safeguard: // Only suppress the fall-through when WebKit actually consumed the event.
// For events WebKit ignores, let AppKit continue (main-menu, system, etc.).
if result { return true }
// Critical app-level shortcuts should always reach AppKit regardless.
let criticalKeyCodes: Set<UInt16> = [12 /*Q*/, 13 /*W*/, 46 /*M*/]
if criticalKeyCodes.contains(event.keyCode) { return false }
return resultAt minimum, please re-examine whether always returning 🟡
|
Contributor
Greptile SummaryAdds a
Confidence Score: 5/5Safe to merge; default behavior is unchanged and the passthrough is fully opt-in via an empty-default allowlist. The three-layer dispatch architecture is well-reasoned and correctly propagates WebKit's return value so system shortcuts fall through when pages don't consume them. The @mainactor annotation fixes the prior runtime-trap concern. One locking pattern in the host cache can produce a transient stale entry under concurrent off-main-actor access, but all current callers go through @mainactor code paths, making the practical impact nil. The ShortcutPassthroughHostsCache in Sources/Panels/BrowserPanel.swift is worth a second look if BrowserLinkOpenSettings.shortcutPassthroughHosts is ever called from a non-main-actor context in the future. Important Files Changed
Sequence DiagramsequenceDiagram
participant EM as Local Event Monitor
participant WS as NSWindow Swizzle
participant WV as CmuxWebView
participant WK as WebKit
participant MM as AppKit Main Menu
Note over EM,MM: Cmd-modifier keystroke on a passthrough host
EM->>EM: shouldPassthroughCommandEquivalentToWebContent?
alt Host matches allowlist
EM-->>WS: return false (let event flow)
else No match
EM-->>MM: event consumed by configured shortcut
end
WS->>WS: shouldPassthroughCommandEquivalentToWebContent?
alt Host matches allowlist
WS->>WV: performKeyEquivalent(event)
WV->>WV: shouldPassthroughCommandEquivalentToWebContent?
WV->>WK: super.performKeyEquivalent(event)
WK-->>WV: consumed (true/false)
WV-->>WS: result
alt WebKit consumed chord
WS-->>EM: return true
else WebKit did not consume
WS->>MM: fall through to standard dispatch
MM-->>EM: Cmd+Q / Cmd+W / etc. handled normally
end
else No match
WS->>MM: normal cmux shortcut ladder
end
Reviews (14): Last reviewed commit: "Sync swift-file-length-budget.tsv to pos..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
Greptile SummaryAdds a new
Confidence Score: 3/5Safe to merge once the @mainactor annotation is added; the unconditional true-return design is worth a second look before shipping. The new shouldPassthroughCommandEquivalentToWebContent function accesses a @MainActor-isolated WKWebView property through a runtime-only assertion (MainActor.assumeIsolated) rather than a compile-time annotation. All current call sites are on the main thread, so it won't crash today, but the missing annotation leaves no compiler-enforced safety net for future refactors. The NSWindow intercept also unconditionally swallows every Cmd-equivalent on a passthrough host — including chords WebKit doesn't handle — which could silently kill shortcuts like Cmd+Q for users who list a broad host like localhost. Sources/App/ShortcutRoutingSupport.swift (missing @mainactor), Sources/AppDelegate.swift (unconditional true return in NSWindow passthrough) Important Files Changed
Sequence DiagramsequenceDiagram
participant EM as Local Event Monitor
participant AD as AppDelegate.handleCustomShortcut
participant NW as NSWindow.performKeyEquivalent
participant CWV as CmuxWebView.performKeyEquivalent
participant WK as WKWebView (super)
participant Menu as NSApp Main Menu
EM->>AD: NSEvent (Cmd+key)
AD->>AD: shouldPassthroughCommandEquivalentToWebContent?
alt host matches allowlist
AD-->>EM: false (chord cleared, event flows to AppKit)
EM->>NW: performKeyEquivalent
NW->>NW: shouldPassthroughCommandEquivalentToWebContent?
NW->>CWV: performKeyEquivalent
CWV->>CWV: shouldPassthroughCommandEquivalentToWebContent?
CWV->>WK: super.performKeyEquivalent
WK-->>CWV: handled / not handled
CWV-->>NW: result
NW-->>EM: true (always — menu blocked)
else host not in allowlist
AD->>AD: normal shortcut dispatch
AD-->>EM: true/false
Note over NW,Menu: Standard cmux shortcut routing
end
Reviews (2): Last reviewed commit: "Adds support for shortcut passthrough to..." | Re-trigger Greptile |
Contributor
Greptile SummaryThis PR adds a
Confidence Score: 3/5The core passthrough logic correctly delivers events to WebKit for the web-IDE use case, but two issues on the changed path should be addressed before merge. The NSWindow passthrough block silently drops Cmd+Q, Cmd+W, and Cmd+H when any passthrough host is focused, and the missing
Important Files Changed
Sequence DiagramsequenceDiagram
participant User as User (Cmd+key)
participant LEM as LocalEventMonitor<br/>(handleCustomShortcut)
participant WIN as NSWindow<br/>(cmux_performKeyEquivalent)
participant WV as CmuxWebView<br/>(performKeyEquivalent)
participant WK as WKWebView<br/>(super)
participant MENU as NSApp.mainMenu
User->>LEM: keyDown event
LEM->>LEM: shouldPassthroughCommandEquivalentToWebContent?
alt Host matches allowlist
LEM-->>WIN: return false (event continues)
WIN->>WIN: shouldPassthroughCommandEquivalentToWebContent?
WIN->>WV: performKeyEquivalent(event)
WV->>WV: shouldPassthroughCommandEquivalentToWebContent? (2nd eval)
WV->>WK: super.performKeyEquivalent(event)
WK-->>WV: handled (true/false)
WV-->>WIN: finish(result)
WIN-->>User: return true (always — swallows Cmd+Q etc.)
else Host not in allowlist
LEM->>LEM: process configured shortcuts
LEM-->>WIN: return true/false
WIN->>WIN: normal cmux shortcut chain
WIN->>MENU: unhandled → main menu
MENU-->>User: menu action fires
end
Reviews (3): Last reviewed commit: "Adds support for shortcut passthrough to..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@web/data/cmux.schema.json`:
- Around line 875-882: Update the description for the shortcutPassthroughHosts
schema entry to state that host matching strips port numbers before comparison;
e.g., add a sentence such as "Port numbers are ignored during matching — hosts
are matched after stripping any :port suffix, so 'localhost' will match
'localhost:3000'." Keep the existing notes about exact and wildcard prefix
matching and ensure the wording clarifies that wildcards and exact names apply
to the host portion only (ports are not considered).
🪄 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: 836b16a5-323c-460c-8b7a-a6ed7fb1c19c
📒 Files selected for processing (11)
Sources/App/ShortcutRoutingSupport.swiftSources/AppDelegate.swiftSources/CmuxSettingsJSONPathSupport.swiftSources/KeyboardShortcutSettingsFileStore.swiftSources/Panels/BrowserPanel.swiftSources/Panels/CmuxWebView.swiftcmux.xcodeproj/project.pbxprojcmuxTests/BrowserArrowKeyForwardingTests.swiftcmuxTests/BrowserShortcutPassthroughTests.swiftweb/app/[locale]/docs/configuration/page.tsxweb/data/cmux.schema.json
There was a problem hiding this comment.
Actionable comments posted: 22
🤖 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/BrowserConfigTests.swift`:
- Around line 1486-1502: The helper withPassthroughHost mutates
UserDefaults.standard which can leak between concurrent tests; replace its usage
by creating an isolated UserDefaults suite (implement makeIsolatedDefaults as
shown in the comment) and update tests that call
shouldPassthroughCommandEquivalentToWebContent to accept or be provided with
that isolated defaults instance so the policy reads from the suite instead of
UserDefaults.standard; specifically, add makeIsolatedDefaults, ensure defaults
are cleared/teardowned, and modify the call sites or the
shouldPassthroughCommandEquivalentToWebContent helper so it uses the injected
UserDefaults rather than relying on global state (reference symbols:
withPassthroughHost, makeIsolatedDefaults,
shouldPassthroughCommandEquivalentToWebContent,
BrowserDevToolsButtonDebugSettingsTests, BrowserShortcutPassthroughTests).
In `@web/data/cmux.schema.json`:
- Line 882: Update the description string for the "Hosts whose pages receive
Cmd-modifier shortcuts directly." property to explicitly document apex behavior
for wildcards: state whether patterns like "*.example.com" match the apex
"example.com" (e.g., clarify that "*.example.com" does NOT match "example.com"
and that exact host matches are required), and mention that port numbers are
ignored and empty list keeps existing behavior; ensure the new text replaces the
current description so runtime behavior is unambiguous.
In `@web/messages/ar.json`:
- Line 504: Update the Arabic locale string for
schemaDescriptions.browser.shortcutPassthroughHosts to explicitly state that
wildcard patterns like *.subdomain do not match the apex domain (example.com)
while patterns without a wildcard match the apex, and mention that exact host
and port rules still apply (ports ignored); locate the key
"shortcutPassthroughHosts" in web/messages/ar.json and edit its value to mirror
the schema/wildcard semantics (apex vs wildcard matching) using concise Arabic
phrasing consistent with other locale entries.
In `@web/messages/bs.json`:
- Line 504: The current Bosnian translation for
schemaDescriptions.browser.shortcutPassthroughHosts is ambiguous about whether a
wildcard like "*.example.com" matches the apex domain; update that string to
explicitly state the implementation semantics (e.g., "*.example.com" matches
subdomains only and does NOT match "example.com"), and add a short note
explaining to include the apex domain explicitly (e.g., add "example.com") if
you want it matched; modify the message text for shortcutPassthroughHosts to
include this clarification while keeping the existing explanation about ports,
exact matches, and empty-list behavior.
In `@web/messages/da.json`:
- Line 504: The Danish translation for
schemaDescriptions.browser.shortcutPassthroughHosts is ambiguous about whether
the wildcard form (*.example.com) includes the apex domain; update the string in
web/messages/da.json (schemaDescriptions.browser.shortcutPassthroughHosts) to
explicitly state the schema's wildcard semantics—i.e., clarify whether
"*.example.com" matches subdomains only or also the apex "example.com"—so it
matches the original schema wording and remains consistent with other locales;
keep the rest of the description unchanged.
In `@web/messages/de.json`:
- Around line 503-505: Update the "browser.shortcutPassthroughHosts" translation
string to explicitly state wildcard apex behavior: clarify that patterns like
"*.example.com" also match the apex host "example.com" (i.e., the root/apex is
included), so users know "*.subdomain" and "*.example.com" both match the base
domain as well as subdomains; keep the rest of the sentence about exact host
matching, port numbers being ignored, and empty list behavior unchanged.
In `@web/messages/en.json`:
- Around line 604-606: Update the "browser"."shortcutPassthroughHosts"
description to explicitly state the wildcard-apex behavior: clarify that
patterns like "*.example.com" also match the apex host "example.com" (i.e.,
wildcard covers the base/apex), and keep the rest of the existing details about
subdomain wildcards, exact matches, and port-number/empty-list behavior intact;
modify the string for the "shortcutPassthroughHosts" key accordingly.
In `@web/messages/es.json`:
- Around line 503-505: Update the "browser.shortcutPassthroughHosts" Spanish
message to explicitly state that wildcard patterns like "*.example.com" also
match the apex domain "example.com"; modify the text for the key
shortcutPassthroughHosts so it clarifies that wildcards include the apex host
(not only subdomains), mention ports are ignored and an empty list keeps
existing behavior, and keep the rest of the explanatory examples (e.g., VS Code
via code-server) intact.
In `@web/messages/fr.json`:
- Line 504: Update the browser.shortcutPassthroughHosts description to
explicitly state wildcard apex matching: clarify that patterns like
"*.example.com" also match the apex "example.com" (i.e., the bare domain), along
with existing notes about exact host matches, ignored port numbers, and empty
list behavior; update the value for the key "shortcutPassthroughHosts" in
fr.json to add a short sentence stating this apex-domain matching behavior.
In `@web/messages/it.json`:
- Line 504: Update the Italian translation for the "shortcutPassthroughHosts"
message to explicitly state the wildcard semantics used by
browser.shortcutPassthroughHosts: clarify that patterns like "*.example.com"
match both subdomains and the apex host "example.com" (ports ignored), and keep
the rest of the explanation about exact/wildcard matching, passthrough behavior,
and empty-list default intact.
In `@web/messages/ja.json`:
- Line 564: Update the description for browser.shortcutPassthroughHosts to
explicitly state that wildcard entries like *.example.com also match the apex
domain (example.com) when supported, so readers understand that *.サブドメイン は apex
ドメイン（例: example.com）にも一致すること、ポートは無視されること、完全一致とワイルドカードの両方がサポートされることを明記してください;
edit the text around the existing "*.サブドメイン ワイルドカードをサポートします" phrase to add a
short clause clarifying apex-domain matching and an example (e.g.,
「*.example.com は example.com にも一致します」).
In `@web/messages/km.json`:
- Around line 503-505: Update the browser.shortcutPassthroughHosts message to
explicitly state the wildcard matching semantics: clarify that patterns like
*.example.com will also match the apex domain example.com (i.e., the wildcard
covers the root/apex), in addition to subdomains, so readers understand that
*.subdomain patterns include the bare host; keep the rest of the existing
description intact and preserve Khmer localization style.
In `@web/messages/ko.json`:
- Around line 503-505: Update the "browser.shortcutPassthroughHosts" value in
web/messages/ko.json to explicitly state the wildcard semantics for apex
domains: add a brief sentence clarifying whether patterns like "*.example.com"
do or do not match the apex host "example.com" (and keep notes that ports are
ignored and empty list preserves default behavior) so readers know the exact
behavior; ensure the change is applied to the string for the
"browser.shortcutPassthroughHosts" key.
In `@web/messages/no.json`:
- Around line 503-505: Update the "browser.shortcutPassthroughHosts" message to
explicitly state whether wildcard entries include the apex host (e.g., clarify
if "*.example.com" also matches "example.com" or not); edit the Norwegian
description to add a single clear sentence about the wildcard semantics (apex
match or no apex match), keep the existing notes about exact host match,
subdomain wildcards, and port numbers being ignored, and include a brief example
to illustrate the behavior so readers know how "*.underdomene" relates to
"underdomene".
In `@web/messages/pl.json`:
- Around line 503-504: Update the "browser.shortcutPassthroughHosts" Polish
message to explicitly state wildcard matching includes the apex host (e.g.,
clarify that patterns like "*.example.com" also match "example.com"); edit the
string in web/messages/pl.json under the "browser" -> "shortcutPassthroughHosts"
key to append a short sentence stating that "*.subdomena" (e.g.,
"*.example.com") will match the apex host "example.com".
In `@web/messages/pt-BR.json`:
- Around line 503-504: Update the "browser.shortcutPassthroughHosts" schema
description to explicitly state wildcard behavior in Portuguese: add a sentence
clarifying that patterns like "*.example.com" also match the apex domain
"example.com" (i.e., the wildcard includes the bare/apex host), and ensure this
note is written in pt-BR alongside the existing explanation about subdomains,
ports being ignored, and empty list behavior so readers clearly understand that
*.subdomínio covers both subdomínio.example.com and example.com.
In `@web/messages/ru.json`:
- Around line 503-504: Update the "browser.shortcutPassthroughHosts" Russian
description to explicitly state wildcard apex-domain behavior: clarify that a
pattern like "*.example.com" also matches the apex host "example.com" (not only
subdomains). Mention that port numbers are ignored and that exact host or
wildcard forms may be used, keeping the rest of the existing guidance about Cmd
passthrough and empty-list behavior intact; edit the string under the
browser.shortcutPassthroughHosts key accordingly.
In `@web/messages/th.json`:
- Line 504: Update the translation for the key "shortcutPassthroughHosts" to
explicitly document wildcard apex matching: state that patterns like
"*.example.com" also match the apex host "example.com" (i.e., wildcard subdomain
patterns include the base domain), while keeping the existing notes that ports
are ignored, both exact and wildcard matches are supported, and empty lists
preserve default cmux behavior; modify the string value for
"shortcutPassthroughHosts" accordingly so the Thai locale clearly communicates
the apex-domain matching semantics.
In `@web/messages/tr.json`:
- Line 504: Update the translation for the "shortcutPassthroughHosts"
description to explicitly state that wildcard patterns like "*.altetkialan" also
match the apex domain (e.g., "altetkialan" or "example.com") in addition to
subdomains; keep the existing notes that port numbers are ignored and an empty
list preserves current cmux behavior. Edit the string value for the
shortcutPassthroughHosts key so it clearly mentions "joker desenler apex (ana)
alanı da eşler — örn. *.example.com hem example.com hem alt.example.com ile
eşleşir" while retaining the rest of the original explanation about passthrough
ordering and empty-list behavior.
In `@web/messages/uk.json`:
- Line 504: Update the "shortcutPassthroughHosts" translation string so it
explicitly states the wildcard does not match the apex domain — e.g., clarify
that "*.example.com" applies to subdomains like "foo.example.com" but does not
match "example.com"; keep the existing sentence about ports being ignored and
preserve style/terminology for browser.shortcutPassthroughHosts across locales.
In `@web/messages/zh-CN.json`:
- Line 504: Update the "shortcutPassthroughHosts" description to explicitly
state the wildcard apex-matching behavior for patterns like "*.example.com":
indicate whether such a pattern will match both the apex domain (example.com)
and its subdomains (e.g., sub.example.com) or only subdomains; keep the existing
notes that ports are ignored and empty lists preserve cmux behavior. Reference
the "shortcutPassthroughHosts" key and use clear example wording (e.g., “例如
*.example.com 将匹配 example.com 与 sub.example.com” or “例如 *.example.com 只匹配子域，不匹配
example.com”) to remove ambiguity.
In `@web/messages/zh-TW.json`:
- Line 504: Update the "shortcutPassthroughHosts" description to explicitly
state the wildcard matching behavior for patterns like "*.example.com": indicate
whether such a pattern matches only subdomains (e.g., "sub.example.com") or also
the apex domain ("example.com"), and give a short example of each case so users
know how to craft entries; keep mention of port being ignored and empty list
behavior as-is.
🪄 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: 200fe3da-59a4-47b4-8df1-de11b6d55c8d
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (27)
Sources/App/BrowserShortcutPassthroughPolicy.swiftSources/App/ShortcutRoutingSupport.swiftSources/AppDelegate.swiftSources/Panels/CmuxWebView.swiftcmux.xcodeproj/project.pbxprojcmuxTests/BrowserConfigTests.swiftweb/data/cmux.schema.jsonweb/messages/ar.jsonweb/messages/bs.jsonweb/messages/da.jsonweb/messages/de.jsonweb/messages/en.jsonweb/messages/es.jsonweb/messages/fr.jsonweb/messages/it.jsonweb/messages/ja.jsonweb/messages/km.jsonweb/messages/ko.jsonweb/messages/no.jsonweb/messages/pl.jsonweb/messages/pt-BR.jsonweb/messages/ru.jsonweb/messages/th.jsonweb/messages/tr.jsonweb/messages/uk.jsonweb/messages/zh-CN.jsonweb/messages/zh-TW.json
💤 Files with no reviewable changes (1)
- Sources/App/ShortcutRoutingSupport.swift
jcrsilva
force-pushed
the
feature/keyboard_passthrough
branch
from
May 27, 2026 17:26
62fafa6 to
04c7505
Compare
Author
|
@coderabbitai review |
|
✅ Actions performedReview triggered.
|
@jcrsilva I have started the AI code review. It will take a few minutes to complete. |
Author
|
@coderabbitai review |
|
✅ Actions performedReview triggered.
|
@jcrsilva I have started the AI code review. It will take a few minutes to complete. |
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 `@Sources/AppDelegate.swift`:
- Around line 11844-11852: Remove the suggested `@MainActor` change and instead
cache the parsed allowlist used during passthrough checks: modify the chain
starting at shouldPassthroughCommandEquivalentToWebContent →
BrowserLinkOpenSettings.urlMatchesShortcutPassthrough →
hostMatchesShortcutPassthrough so that shortcutPassthroughHosts does not reparse
UserDefaults on every call; implement a cached parsedPatterns property (or
similar) that is refreshed when UserDefaults.didChangeNotification fires (or
when the relevant key changes), and have hostMatchesShortcutPassthrough consult
the cached parsedPatterns for matching to avoid per-event parsing overhead.
In `@Sources/Panels/BrowserPanel.swift`:
- Around line 894-900: shortcutPassthroughHosts currently reparses the
newline-delimited string on every call (hot path during per-keystroke
command-key routing); add a static cached parsed array (e.g., a private static
var cachedShortcutPassthroughHosts: [String]?) and change
shortcutPassthroughHosts(defaults:) to return the cached value if present,
otherwise parse once, store in the cache, and return it; invalidate or refresh
this cache on settings changes by observing UserDefaults.didChangeNotification
(or call the same invalidation from any explicit setter path) and consider
synchronizing access with a serial DispatchQueue or lock to be thread-safe;
update the host-match call sites that currently call shortcutPassthroughHosts
repeatedly to rely on the cached result.
🪄 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: 7d51f0fe-9d21-4e65-8c93-f13fe42ff482
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (33)
Sources/App/BrowserShortcutPassthroughPolicy.swiftSources/App/ShortcutRoutingSupport.swiftSources/AppDelegate.swiftSources/CmuxSettingsJSONPathSupport.swiftSources/KeyboardShortcutSettingsFileStore.swiftSources/Panels/BrowserPanel.swiftSources/Panels/CmuxWebView.swiftcmux.xcodeproj/project.pbxprojcmuxTests/BrowserArrowKeyForwardingTests.swiftcmuxTests/BrowserConfigTests.swiftcmuxTests/BrowserShortcutPassthroughTests.swiftweb/app/[locale]/docs/configuration/page.tsxweb/data/cmux.schema.jsonweb/messages/ar.jsonweb/messages/bs.jsonweb/messages/da.jsonweb/messages/de.jsonweb/messages/en.jsonweb/messages/es.jsonweb/messages/fr.jsonweb/messages/it.jsonweb/messages/ja.jsonweb/messages/km.jsonweb/messages/ko.jsonweb/messages/no.jsonweb/messages/pl.jsonweb/messages/pt-BR.jsonweb/messages/ru.jsonweb/messages/th.jsonweb/messages/tr.jsonweb/messages/uk.jsonweb/messages/zh-CN.jsonweb/messages/zh-TW.json
💤 Files with no reviewable changes (26)
- web/app/[locale]/docs/configuration/page.tsx
- web/messages/pt-BR.json
- web/messages/pl.json
- web/data/cmux.schema.json
- web/messages/fr.json
- web/messages/zh-TW.json
- web/messages/zh-CN.json
- web/messages/es.json
- web/messages/ru.json
- web/messages/th.json
- web/messages/de.json
- web/messages/ja.json
- web/messages/uk.json
- web/messages/ar.json
- web/messages/da.json
- web/messages/it.json
- web/messages/km.json
- web/messages/no.json
- web/messages/en.json
- cmuxTests/BrowserArrowKeyForwardingTests.swift
- web/messages/ko.json
- web/messages/tr.json
- cmux.xcodeproj/project.pbxproj
- cmuxTests/BrowserShortcutPassthroughTests.swift
- web/messages/bs.json
- cmuxTests/BrowserConfigTests.swift
Author
|
@coderabbitai review |
@jcrsilva I have started the AI code review. It will take a few minutes to complete. |
|
✅ Actions performedReview triggered.
|
Contributor
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
New config option added under "browser" allowing for certain keyboard shortcuts which right now are intercepted by tmux to passthough to the embedded browser when in configured hosts. This has potentially other applications but the intent is to tackle [this class](manaflow-ai#2342) of issue, to more easily allow people to run web-based IDEs such as code-server in the integrated cmux browser.
jcrsilva
force-pushed
the
feature/keyboard_passthrough
branch
from
May 28, 2026 12:35
1d05e84 to
feb67a9
Compare
teamleaderleo
added
area: browser
Collaborator
|
This remains a live browser shortcut passthrough proposal; leaving the configuration and routing choice to the team. |
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
2 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.
Summary
New config option added under "browser" allowing for certain keyboard shortcuts which right now are intercepted by tmux to passthough to the embedded browser when in configured hosts.
This has potentially other applications but the intent is to tackle this class of issue, to more easily allow people to run web-based IDEs such as code-server in the integrated cmux browser.
Testing
Demo Video
Need to add one later, can't record rn.
Checklist
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds per‑host Cmd‑shortcut passthrough to the embedded browser so pages can handle Cmd+P, Cmd+Shift+P, Cmd+F, etc. Default behavior is unchanged; if a page doesn’t consume a chord, normal menu handling resumes.
New Features
browser.shortcutPassthroughHostsallowlist (exact hosts and*.example.com, ports ignored). On match, cmux forwards Cmd‑modifier chords to the page first at the event monitor, window, and webview layers; omnibar and Web Inspector excluded.web/data/cmux.schema.json, docs, and localized descriptions; added tests for passthrough policy, host matching, arrow routing, and WebView/menu fallthrough.Refactors
shortcutPassthroughHoststo avoid reparsing on every Cmd keystroke.Written for commit 2a5fd3c. Summary will update on new commits.
Review in cubic
Summary by CodeRabbit
New Features
Behavior Changes
Bug Fixes
Documentation
Tests