Serve ide browse on localhost so cmux's embedded browser catches it#58
Conversation
…browser catches ide browse
|
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:
📝 WalkthroughWalkthroughThe CLI browse URL and validation now use ChangesLoopback URL canonicalization
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces 127.0.0.1 with localhost across several files, including documentation, shell scripts, and the Rust HTTP server. The reviewer pointed out that using localhost for internal script-to-server communication and health checks can introduce latency or connection failures on systems where localhost resolves to IPv6 ::1 first, since the server only binds to IPv4. It is recommended to revert these internal calls back to 127.0.0.1 to ensure reliability.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Pull request overview
This PR updates the loopback launch/CLI-facing URLs from 127.0.0.1:17877 to localhost:17877 so that cmux’s open shim routes ide browse <path> into cmux’s embedded browser (since cmux whitelists localhost but not the raw IP).
Changes:
- Swaps CLI and dev-runner health checks and browse URLs to use
http://localhost:17877. - Updates the macOS Finder Quick Action runner template to call the loopback API via
localhost. - Adjusts the Rust MCP status host fallback and updates README documentation for the MCP endpoint.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src-tauri/src/http_server.rs | Changes the Host-header fallback used to build the reported MCP endpoint. |
| scripts/install-macos-finder-quick-action.sh | Updates the Finder Quick Action runner template’s API base to localhost. |
| scripts/install-cli-command.sh | Updates the generated ide launcher’s health check and ide browse URL to localhost. |
| run.sh | Updates ide-dev handoff base URL and reachability probe to localhost. |
| README.md | Updates the documented MCP endpoint URL to localhost. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The verification suite still asserted the old 127.0.0.1 URLs in the generated launch runners, but this PR changed the templates to localhost. Update the paired assertions for the exact files this PR edited.
…lls to 127.0.0.1 Bots correctly flagged that localhost can resolve to IPv6 [::1] first while the server binds IPv4 only, and can route through http_proxy — a real risk for internal curl/health-check calls. Only the browser-facing browse_url (and the README MCP doc url) need localhost for cmux's open() whitelist; every internal call reverts to 127.0.0.1. Validation scripts updated to match.
|
@coderabbitai review |
|
/gemini review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Code Review
This pull request replaces 127.0.0.1 with localhost in the README documentation, CLI installation script, and validation runner. Feedback suggests reverting the change in the README back to 127.0.0.1 because some MCP clients might fail to connect if localhost resolves to the IPv6 loopback address (::1) while the server only binds to IPv4.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
MCP clients are programmatic, not routed through cmux's open() whitelist, so they don't need localhost — and gemini's IPv6 ::1 concern applies here the same as the other internal endpoints. browse_url stays localhost as the only browser-facing exception.
|
@coderabbitai review |
|
/gemini review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Code Review
This pull request updates the CLI installation script to use localhost instead of 127.0.0.1 for the browse URL, and updates the corresponding validation test script to reflect this change. There are no review comments, and I have no additional feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
GordonBeeming
changed the title
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@coderabbitai review |
|
/gemini review |
|
@codex review |
|
@codex review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Code Review
This pull request updates the ide browse command to use localhost instead of 127.0.0.1 so that cmux's embedded browser can intercept and open the workspace. To avoid IPv6 loopback and proxy issues, the backend and frontend canonicalize localhost back to 127.0.0.1 for copyable endpoints and MCP status endpoints. The review feedback highlights a potential bug in the Rust canonical_loopback_host implementation when parsing IPv6 loopback addresses without a port (e.g., [::1]), which would result in an invalid host/port string. It is recommended to validate that the parsed port is numeric and to add test cases covering IPv6 loopback scenarios.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@docs/faq/cmux-embedded-browser.md`:
- Around line 51-53: Update the FAQ entry for
browser.insecureHttpHostsAllowedInEmbeddedBrowser to state that it only
suppresses warnings for plain http:// URLs, not whether they open in the
embedded browser. Direct routing guidance to
browser.hostsToOpenInEmbeddedBrowser and
browser.interceptTerminalOpenCommandInCmuxBrowser, removing the implication that
the insecure-host allowlist controls interception.
In `@src-tauri/src/http_server.rs`:
- Around line 1142-1145: Update canonical_loopback_host to parse the suffix
after the final colon as a non-zero u16, falling back to port 17877 when parsing
fails or yields zero, rather than embedding the raw suffix. Add a regression
test covering a value such as localhost:17877@attacker.example and verify the
reported URL remains on 127.0.0.1.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 712fe6dc-d14f-4ea2-83a6-9cc7ce355942
📒 Files selected for processing (4)
docs/faq/cmux-embedded-browser.mdsrc-tauri/src/http_server.rssrc/tauri.test.tssrc/tauri.ts
The Host header is client-controlled input, not trusted, so canonical_loopback_host
was parsing it wrong on three counts CodeRabbit/gemini/copilot each caught:
a malicious 'localhost:17877@attacker.example' Host smuggled a userinfo/host
through the port slot into the reported MCP URL, an unbracketed IPv6 host like
'[::1]' mis-split on rsplit_once(':'), and an empty or non-numeric port
produced an invalid '127.0.0.1:' endpoint. Parsing the suffix as a non-zero
u16 fixes all three at the source. Also switched canonicalizeLoopbackOrigin's
ambiguous $1127.0.0.1$2 replacement string to a replacer callback, and
clarified that insecureHttpHostsAllowedInEmbeddedBrowser only suppresses a
warning, not routing.
|
@coderabbitai review |
|
/gemini review |
|
@codex review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Code Review
This pull request changes the ide browse URL from 127.0.0.1 to localhost to allow cmux's open shim to intercept and route it to the embedded browser. To avoid IPv6 loopback and proxy issues, both the Rust backend and TypeScript frontend now canonicalize localhost origins back to 127.0.0.1 for internal endpoints and copyable links. Relevant documentation and unit tests have also been added. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
The comment said parsing 'rejects' an IPv6 host, but a bracketed IPv6+port like [::1]:17877 splits cleanly and keeps its real port — only the bare [::1] (no port) hits the malformed-suffix fallback. The IPv6 hostname itself is never propagated either way. Comment-only, no logic or test change.
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@coderabbitai review |
|
/gemini review |
|
@codex review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Code Review
This pull request updates the ide browse URL to use localhost instead of 127.0.0.1 to allow cmux's open whitelist to intercept and route it to the embedded browser. To prevent connection issues with other tools, both the Rust backend and TypeScript frontend canonicalize loopback origins back to 127.0.0.1 for copyable endpoints and MCP statuses. Relevant documentation and tests have also been added. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Summary
ide browse <path>runsopen <url>on the loopback API. Inside cmux,openis a shim that routes an HTTP URL to the embedded browser only when the URL's host is on cmux's whitelist; otherwise it falls through to the system default browser. The URL used127.0.0.1, and cmux's stock whitelist shipslocalhostbut not the raw IP, soide browsekept opening in the real browser instead of the embedded one.The fix is narrow
Only the browser-facing
browse_urlinscripts/install-cli-command.shmoves tolocalhost:17877, along with its assertion inscripts/validate-launch-runners.mjs. Everything else stays on127.0.0.1: the health-check probes, theide-devAPI base inrun.sh, the Finder quick action, and the MCP Host fallback insrc-tauri/src/http_server.rs. Those are internal HTTP-client calls that never go through cmux's browser routing, and127.0.0.1keeps them clear of the IPv6::1resolution race (the server binds IPv4 loopback only). Theopentarget is the one spot that has to match cmux's whitelist, so it's the one spot that changes.Keeping copied endpoints on 127.0.0.1
Opening the SPA on
localhostmeans the Integrations dialog would otherwise hand outlocalhostendpoints too:getHttpEndpoint()falls back towindow.location.origin, and/api/codex-mcpbuilds its endpoint from the requestHostheader. Both now canonicalize the host back to127.0.0.1while preserving the actual port (canonicalizeLoopbackOrigininsrc/tauri.ts,canonical_loopback_hostinhttp_server.rs), so the copyable Browser and Codex MCP config stay consistent with every other non-browse client. TheHostparse validates the port as a non-zerou16and falls back to17877on anything malformed, so a header likelocalhost:17877@attacker.examplecan't smuggle a host into the reported URL.Docs
Adds
docs/faq/with an index and a first entry,cmux-embedded-browser.md, explaining the symptom and the~/.config/cmux/cmux.jsonbrowser.*settings that control whether cmux interceptsopen.Test plan
open http://localhost:17877/browse?path=…routes to a cmux embedded pane rather than the system browser; a marker on the shim's system-open fallback never fired.cargo test,cargo fmt --check, and both launcher-validation scripts.