Repository navigation
Add external URL bypass rules for embedded browser opens - #768
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe pull request introduces configurable external URL opening across the cmux application. Users can specify URL patterns in settings that should open in the system browser rather than in-app. Pattern matching supports literal substring matches (case-insensitive) and regex patterns (prefixed with "re:"). The feature integrates across the CLI, terminal controller, and browser panel layers. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![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: 222a83f6b0
ℹ️ 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".
Contributor
Greptile SummaryAdded a configurable external URL bypass feature that allows users to specify URLs that should always open in the system browser instead of cmux's embedded browser. The implementation supports both plain substring matching and regex patterns (prefixed with
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[URL Open Request] --> B{Source?}
B -->|Terminal Link Click| C[GhosttyTerminalView]
B -->|open command| D[Resources/bin/open wrapper]
C --> E{BrowserLinkOpenSettings<br/>shouldOpenExternally?}
D --> F[Load browserExternalOpenPatterns<br/>from UserDefaults]
F --> G{url_matches_external_open_patterns?}
E -->|Yes| H[Open in System Browser]
E -->|No| I[Continue to embedded browser logic]
G -->|Yes| J[Add to failed_urls array]
G -->|No| K{Host matches whitelist?}
J --> L[Fallback to system open]
K -->|Yes| M[Open in cmux browser via CLI]
K -->|No| N[Add to failed_urls array]
N --> L
I --> O[Check host whitelist]
O --> P{Matches?}
P -->|Yes| Q[Open in embedded browser]
P -->|No| H
style E fill:#e1f5ff
style G fill:#e1f5ff
style H fill:#ffe1e1
style Q fill:#e1ffe1
style M fill:#e1ffe1
Last reviewed commit: 222a83f |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_open_wrapper.py (1)
242-329: Add one negative-match regression case for external patterns.The new coverage validates matching and invalid-regex behavior well. I’d also add a non-matching pattern case to lock in intended behavior when
external_patternsis present but does not match the URL.➕ Suggested test addition
+def test_external_pattern_non_match_behavior_is_stable(failures: list[str]) -> None: + url = "https://unmatched.example.net/path" + open_log, cmux_log, code, stderr = run_wrapper( + args=[url], + intercept_setting="1", + whitelist="", + external_patterns="platform.openai.com/account/usage", + ) + expect(code == 0, f"external non-match: wrapper exited {code}: {stderr}", failures) + # Assert the intended contract explicitly (choose expected branch and lock it in). + # Example if deferral-for-all is intended: + expect(cmux_log == [f"browser open {url}"], f"external non-match: unexpected cmux log {cmux_log}", failures) + expect(open_log == [], f"external non-match: expected no system open calls, got {open_log}", failures)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_open_wrapper.py` around lines 242 - 329, Add a negative-match test that ensures when external_patterns is provided but doesn't match the URL the wrapper falls back to the system open path: create a new test function (e.g., test_external_pattern_non_matching_opens_system) that calls run_wrapper with a URL like "https://example.com/path", intercept_setting="1", whitelist="", and external_patterns set to a non-matching literal (e.g., "platform.openai.com"), then assert code == 0, cmux_log == [] (no cmux/browser open), open_log == [f"system open {url}"], and that stderr does not contain "invalid regular expression"; add this next to the other test_* functions so it covers the non-matching pattern regression.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/test_open_wrapper.py`:
- Around line 242-329: Add a negative-match test that ensures when
external_patterns is provided but doesn't match the URL the wrapper falls back
to the system open path: create a new test function (e.g.,
test_external_pattern_non_matching_opens_system) that calls run_wrapper with a
URL like "https://example.com/path", intercept_setting="1", whitelist="", and
external_patterns set to a non-matching literal (e.g., "platform.openai.com"),
then assert code == 0, cmux_log == [] (no cmux/browser open), open_log ==
[f"system open {url}"], and that stderr does not contain "invalid regular
expression"; add this next to the other test_* functions so it covers the
non-matching pattern regression.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
CLI/cmux.swiftResources/bin/openSources/GhosttyTerminalView.swiftSources/Panels/BrowserPanel.swiftSources/TerminalController.swiftSources/cmuxApp.swiftcmuxTests/CmuxWebViewKeyEquivalentTests.swifttests/test_open_wrapper.py
bn-l
pushed a commit
to bn-l/cmux
that referenced
this pull request
Apr 3, 2026
…#768) * Add external URL bypass rules for embedded browser opens * Align open-wrapper external regex handling with app-side matcher
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
plain substringorre:regex)Resources/bin/openwrapper path for interceptedopen http(s)commandsTesting
python3 tests/test_open_wrapper.py(pass)xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux-unit -configuration Debug -destination 'platform=macOS' -only-testing:cmuxTests/BrowserLinkOpenSettingsTests test(pass)xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux-unit -configuration Debug -destination 'platform=macOS' -only-testing:cmuxTests/CmuxWebViewKeyEquivalentTests test(pass)xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux -configuration Debug -destination 'platform=macOS' build(pass)codex exec review --uncommitted -m gpt-5.3-codex --dangerously-bypass-approvals-and-sandbox -c model_reasoning_effort="medium"(0 actionable findings)Issues
/extra-usageand similar flows auto-open inside cmux browser instead of default browserSummary by CodeRabbit
New Features
Tests