fix(tui): align /browser connect local CDP handling - #17238
Conversation
Share Chrome CDP launch helpers between the classic CLI and TUI so default /browser connect uses loopback consistently, retries local Chrome launch, and reports a copyable manual-start command instead of claiming a dead connection.
Detect an actual Chrome/Chromium executable before printing a manual CDP launch command, including common WSL-mounted Windows browser paths, so /browser connect does not suggest google-chrome when it is unavailable.
There was a problem hiding this comment.
Pull request overview
Aligns TUI /browser connect behavior with the classic CLI by sharing Chrome CDP helpers, standardizing the default local CDP URL, and improving connection validation/auto-launch behavior when the debug port isn’t reachable.
Changes:
- Default
/browser connect(TUI + gateway) now targetshttp://127.0.0.1:9222. - Added shared
hermes_cli.browser_connecthelpers for default URL/port, candidate Chrome detection, auto-launch, and manual launch instructions. - Gateway
/browser connectnow retries after attempting a local Chrome launch and returns a manual launch command when unreachable; classic CLI avoids claiming “connected” when the port never becomes reachable.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ui-tui/src/app/slash/commands/ops.ts | Switches default connect URL to loopback IP. |
| ui-tui/src/tests/createSlashHandler.test.ts | Adds coverage for the default /browser connect payload. |
| tui_gateway/server.py | Uses shared default CDP URL; adds local auto-launch + retry + improved error messaging. |
| tests/test_tui_gateway_server.py | Adds tests for default loopback, launch hint, and retry-after-launch behavior. |
| tests/cli/test_cli_browser_connect.py | Updates mocks to patch the new shared helper module. |
| hermes_cli/browser_connect.py | New shared implementation for default CDP URL/port and Chrome launch helpers. |
| cli.py | Replaces inlined Chrome launch logic with shared helpers and prevents false “connected” state. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Return CLI-style browser connect status messages from the gateway and render them in the TUI so local Chrome launch attempts are visible instead of ending in a silent delayed failure.
Emit browser.progress JSON-RPC notifications during the connect work and render them in the TUI as system transcript lines, so users see the same step-by-step status the base CLI prints instead of nothing for ~1m followed by a final result.
Split browser.manage into a small dispatcher with named connect/disconnect helpers, fold _http_ok / _probe_urls / _normalize_cdp_url out of the nested probe loop, collapse the failure-message scaffolding, and DRY the chrome candidate path tables. Behaviour and event shape unchanged.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fixes from Copilot's two passes on PR #17238: * Validate parsed URL once: reject missing host, invalid port, and unsupported scheme up front so malformed inputs (e.g. http://:9222 or http://localhost:abc) don't fall through to a generic 5031. * Tighten _is_default_local_cdp to require a discovery-style path so ws://127.0.0.1:9222/devtools/browser/<id> is not collapsed to bare http://127.0.0.1:9222 (which would lose the path and break the connect). * Move browser.manage into _LONG_HANDLERS so the up-to-10s launch-and-retry loop runs on the RPC pool instead of blocking the main dispatcher. * try_launch_chrome_debug uses Windows-appropriate detach kwargs (creationflags=DETACHED_PROCESS|CREATE_NEW_PROCESS_GROUP) instead of POSIX-only start_new_session=True. * manual_chrome_debug_command uses subprocess.list2cmdline on Windows so the printed instruction is cmd.exe-compatible. * Mirror host/port validation in cli.py /browser connect so the classic CLI never persists an invalid BROWSER_CDP_URL.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* Gate `browser.progress` emit on truthy `session_id`. The TUI prints `messages` from the response when there's no session, so emitting events too would double-render. Now: with a session → events stream live; without one → bundled messages only. * Resolve `system = platform.system()` once in `_browser_connect` and thread it through `try_launch_chrome_debug` and `_failure_messages` → `manual_chrome_debug_command`, so the generated hint is consistent (and tests are deterministic) on any host. * Add `test_browser_manage_connect_no_session_skips_progress_events` to lock in the gating behavior.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* Reject unsupported schemes (anything outside http/https/ws/wss) in
cli.py /browser connect before probing or persisting, matching the
gateway's existing 4015 path.
* Defend gateway browser.manage against `{"url": null}` and
non-string urls: empty/null falls back to DEFAULT_BROWSER_CDP_URL,
non-string returns a 4015 instead of slipping into the generic
5031 catch via TypeError on `"://" in url`.
* Add regression tests for both null-url fallback and non-string
rejection.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fixes from Copilot's two passes on PR #17238: * Validate parsed URL once: reject missing host, invalid port, and unsupported scheme up front so malformed inputs (e.g. http://:9222 or http://localhost:abc) don't fall through to a generic 5031. * Tighten _is_default_local_cdp to require a discovery-style path so ws://127.0.0.1:9222/devtools/browser/<id> is not collapsed to bare http://127.0.0.1:9222 (which would lose the path and break the connect). * Move browser.manage into _LONG_HANDLERS so the up-to-10s launch-and-retry loop runs on the RPC pool instead of blocking the main dispatcher. * try_launch_chrome_debug uses Windows-appropriate detach kwargs (creationflags=DETACHED_PROCESS|CREATE_NEW_PROCESS_GROUP) instead of POSIX-only start_new_session=True. * manual_chrome_debug_command uses subprocess.list2cmdline on Windows so the printed instruction is cmd.exe-compatible. * Mirror host/port validation in cli.py /browser connect so the classic CLI never persists an invalid BROWSER_CDP_URL.
Fixes from Copilot's two passes on PR NousResearch#17238: * Validate parsed URL once: reject missing host, invalid port, and unsupported scheme up front so malformed inputs (e.g. http://:9222 or http://localhost:abc) don't fall through to a generic 5031. * Tighten _is_default_local_cdp to require a discovery-style path so ws://127.0.0.1:9222/devtools/browser/<id> is not collapsed to bare http://127.0.0.1:9222 (which would lose the path and break the connect). * Move browser.manage into _LONG_HANDLERS so the up-to-10s launch-and-retry loop runs on the RPC pool instead of blocking the main dispatcher. * try_launch_chrome_debug uses Windows-appropriate detach kwargs (creationflags=DETACHED_PROCESS|CREATE_NEW_PROCESS_GROUP) instead of POSIX-only start_new_session=True. * manual_chrome_debug_command uses subprocess.list2cmdline on Windows so the printed instruction is cmd.exe-compatible. * Mirror host/port validation in cli.py /browser connect so the classic CLI never persists an invalid BROWSER_CDP_URL.
Fixes from Copilot's two passes on PR NousResearch#17238: * Validate parsed URL once: reject missing host, invalid port, and unsupported scheme up front so malformed inputs (e.g. http://:9222 or http://localhost:abc) don't fall through to a generic 5031. * Tighten _is_default_local_cdp to require a discovery-style path so ws://127.0.0.1:9222/devtools/browser/<id> is not collapsed to bare http://127.0.0.1:9222 (which would lose the path and break the connect). * Move browser.manage into _LONG_HANDLERS so the up-to-10s launch-and-retry loop runs on the RPC pool instead of blocking the main dispatcher. * try_launch_chrome_debug uses Windows-appropriate detach kwargs (creationflags=DETACHED_PROCESS|CREATE_NEW_PROCESS_GROUP) instead of POSIX-only start_new_session=True. * manual_chrome_debug_command uses subprocess.list2cmdline on Windows so the printed instruction is cmd.exe-compatible. * Mirror host/port validation in cli.py /browser connect so the classic CLI never persists an invalid BROWSER_CDP_URL.
Fixes from Copilot's two passes on PR NousResearch#17238: * Validate parsed URL once: reject missing host, invalid port, and unsupported scheme up front so malformed inputs (e.g. http://:9222 or http://localhost:abc) don't fall through to a generic 5031. * Tighten _is_default_local_cdp to require a discovery-style path so ws://127.0.0.1:9222/devtools/browser/<id> is not collapsed to bare http://127.0.0.1:9222 (which would lose the path and break the connect). * Move browser.manage into _LONG_HANDLERS so the up-to-10s launch-and-retry loop runs on the RPC pool instead of blocking the main dispatcher. * try_launch_chrome_debug uses Windows-appropriate detach kwargs (creationflags=DETACHED_PROCESS|CREATE_NEW_PROCESS_GROUP) instead of POSIX-only start_new_session=True. * manual_chrome_debug_command uses subprocess.list2cmdline on Windows so the printed instruction is cmd.exe-compatible. * Mirror host/port validation in cli.py /browser connect so the classic CLI never persists an invalid BROWSER_CDP_URL.
Fixes from Copilot's two passes on PR NousResearch#17238: * Validate parsed URL once: reject missing host, invalid port, and unsupported scheme up front so malformed inputs (e.g. http://:9222 or http://localhost:abc) don't fall through to a generic 5031. * Tighten _is_default_local_cdp to require a discovery-style path so ws://127.0.0.1:9222/devtools/browser/<id> is not collapsed to bare http://127.0.0.1:9222 (which would lose the path and break the connect). * Move browser.manage into _LONG_HANDLERS so the up-to-10s launch-and-retry loop runs on the RPC pool instead of blocking the main dispatcher. * try_launch_chrome_debug uses Windows-appropriate detach kwargs (creationflags=DETACHED_PROCESS|CREATE_NEW_PROCESS_GROUP) instead of POSIX-only start_new_session=True. * manual_chrome_debug_command uses subprocess.list2cmdline on Windows so the printed instruction is cmd.exe-compatible. * Mirror host/port validation in cli.py /browser connect so the classic CLI never persists an invalid BROWSER_CDP_URL.
Fixes from Copilot's two passes on PR NousResearch#17238: * Validate parsed URL once: reject missing host, invalid port, and unsupported scheme up front so malformed inputs (e.g. http://:9222 or http://localhost:abc) don't fall through to a generic 5031. * Tighten _is_default_local_cdp to require a discovery-style path so ws://127.0.0.1:9222/devtools/browser/<id> is not collapsed to bare http://127.0.0.1:9222 (which would lose the path and break the connect). * Move browser.manage into _LONG_HANDLERS so the up-to-10s launch-and-retry loop runs on the RPC pool instead of blocking the main dispatcher. * try_launch_chrome_debug uses Windows-appropriate detach kwargs (creationflags=DETACHED_PROCESS|CREATE_NEW_PROCESS_GROUP) instead of POSIX-only start_new_session=True. * manual_chrome_debug_command uses subprocess.list2cmdline on Windows so the printed instruction is cmd.exe-compatible. * Mirror host/port validation in cli.py /browser connect so the classic CLI never persists an invalid BROWSER_CDP_URL.
Fixes from Copilot's two passes on PR NousResearch#17238: * Validate parsed URL once: reject missing host, invalid port, and unsupported scheme up front so malformed inputs (e.g. http://:9222 or http://localhost:abc) don't fall through to a generic 5031. * Tighten _is_default_local_cdp to require a discovery-style path so ws://127.0.0.1:9222/devtools/browser/<id> is not collapsed to bare http://127.0.0.1:9222 (which would lose the path and break the connect). * Move browser.manage into _LONG_HANDLERS so the up-to-10s launch-and-retry loop runs on the RPC pool instead of blocking the main dispatcher. * try_launch_chrome_debug uses Windows-appropriate detach kwargs (creationflags=DETACHED_PROCESS|CREATE_NEW_PROCESS_GROUP) instead of POSIX-only start_new_session=True. * manual_chrome_debug_command uses subprocess.list2cmdline on Windows so the printed instruction is cmd.exe-compatible. * Mirror host/port validation in cli.py /browser connect so the classic CLI never persists an invalid BROWSER_CDP_URL.
Fixes from Copilot's two passes on PR NousResearch#17238: * Validate parsed URL once: reject missing host, invalid port, and unsupported scheme up front so malformed inputs (e.g. http://:9222 or http://localhost:abc) don't fall through to a generic 5031. * Tighten _is_default_local_cdp to require a discovery-style path so ws://127.0.0.1:9222/devtools/browser/<id> is not collapsed to bare http://127.0.0.1:9222 (which would lose the path and break the connect). * Move browser.manage into _LONG_HANDLERS so the up-to-10s launch-and-retry loop runs on the RPC pool instead of blocking the main dispatcher. * try_launch_chrome_debug uses Windows-appropriate detach kwargs (creationflags=DETACHED_PROCESS|CREATE_NEW_PROCESS_GROUP) instead of POSIX-only start_new_session=True. * manual_chrome_debug_command uses subprocess.list2cmdline on Windows so the printed instruction is cmd.exe-compatible. * Mirror host/port validation in cli.py /browser connect so the classic CLI never persists an invalid BROWSER_CDP_URL.
Summary
/browser connectusehttp://127.0.0.1:9222, retry local Chrome launch, and return a manual launch command when Chrome is unavailable.Test plan
scripts/run_tests.sh tests/test_tui_gateway_server.py tests/cli/test_cli_browser_connect.pycd ui-tui && npm test -- --run src/__tests__/createSlashHandler.test.tscd ui-tui && npm run type-checkgit diff --check