diff --git a/hermes_cli/browser_connect.py b/hermes_cli/browser_connect.py index 67868837d7f..4fcc4cc63c5 100644 --- a/hermes_cli/browser_connect.py +++ b/hermes_cli/browser_connect.py @@ -174,6 +174,75 @@ def is_browser_debug_ready(url: str, timeout: float = 1.0) -> bool: return False +# Both loopback literals: Windows (and some Linux setups) can hand the IPv4 +# loopback to one process and the IPv6 loopback to another. Chrome asked to +# bind :9222 while e.g. VS Code's js-debug holds 127.0.0.1:9222 will come up +# on [::1]:9222 only — reachable, but invisible to an IPv4-only probe. +_LOOPBACK_PROBE_HOSTS = ("127.0.0.1", "[::1]") +_LOOPBACK_SOCKET_HOSTS = ("127.0.0.1", "::1") + + +def discover_local_cdp_url(port: int, timeout: float = 1.0) -> str | None: + """Return the first loopback URL (IPv4 first, then IPv6) speaking CDP. + + Dual-stack discovery: when another application squats the IPv4 + loopback on ``port``, a debug browser launched with + ``--remote-debugging-port`` may bind only ``[::1]``. Probing both + literals finds it either way. Returns ``None`` when neither + loopback exposes a CDP discovery endpoint. + """ + for host in _LOOPBACK_PROBE_HOSTS: + url = f"http://{host}:{port}" + if is_browser_debug_ready(url, timeout=timeout): + return url + return None + + +def local_port_in_use(port: int, timeout: float = 0.5) -> bool: + """Return True when either loopback accepts TCP on ``port``. + + Callers use this AFTER a failed CDP probe to distinguish "port is + free, we can launch a browser on it" from "another application + (IDE debugger, dev server) is squatting the port and a launch + would fight it". + """ + import socket + + for host in _LOOPBACK_SOCKET_HOSTS: + try: + with socket.create_connection((host, port), timeout=timeout): + return True + except OSError: + continue + return False + + +def find_free_debug_port(preferred: int = DEFAULT_BROWSER_CDP_PORT, attempts: int = 10) -> int: + """Return the first port after ``preferred`` bindable on both loopbacks. + + Used when ``preferred`` is occupied by a non-CDP application: rather + than launching a browser into a bind conflict, pick a nearby free + port. Falls back to ``preferred + 1`` if nothing binds (the launch + will then fail with a clear browser-side error instead of silently + doing nothing). + """ + import socket + + for port in range(preferred + 1, preferred + 1 + attempts): + bindable = True + for family, host in ((socket.AF_INET, "127.0.0.1"), (socket.AF_INET6, "::1")): + try: + with socket.socket(family, socket.SOCK_STREAM) as sock: + sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + sock.bind((host, port)) + except OSError: + bindable = False + break + if bindable: + return port + return preferred + 1 + + def manual_chrome_debug_command(port: int = DEFAULT_BROWSER_CDP_PORT, system: str | None = None) -> str | None: system = system or platform.system() candidates = get_chrome_debug_candidates(system) @@ -213,11 +282,12 @@ def _wait_for_browser_debug_ready_or_exit( candidate binary exists but exits immediately before exposing the CDP port. Slower browsers can still finish starting after this grace window. """ - cdp_url = f"http://127.0.0.1:{port}" deadline = time.monotonic() + timeout while time.monotonic() < deadline: - if is_browser_debug_ready(cdp_url, timeout=min(interval, 0.2)): + # Dual-stack: a squatter on the IPv4 loopback can push the browser + # to bind [::1] only — check both so a successful launch is seen. + if discover_local_cdp_url(port, timeout=min(interval, 0.2)): return "ready" if proc.poll() is not None: return "exited" diff --git a/hermes_cli/cli_commands_mixin.py b/hermes_cli/cli_commands_mixin.py index e1c1dfa78f6..77aaa523942 100644 --- a/hermes_cli/cli_commands_mixin.py +++ b/hermes_cli/cli_commands_mixin.py @@ -30,8 +30,11 @@ from rich.panel import Panel from hermes_constants import display_hermes_home, is_termux as _is_termux_environment from hermes_cli.browser_connect import ( DEFAULT_BROWSER_CDP_URL, + discover_local_cdp_url, + find_free_debug_port, is_browser_debug_ready, launch_chrome_debug, + local_port_in_use, manual_chrome_debug_command, ) @@ -1849,26 +1852,48 @@ class CLICommandsMixin: print() - # Check if a Chromium-family browser is already serving CDP on the debug port - _already_open = is_browser_debug_ready(cdp_url, timeout=1.0) + # Check if a Chromium-family browser is already serving CDP on the debug port. + # For the default-local URL, probe both loopbacks (IPv4 + IPv6): a + # squatter on 127.0.0.1: (e.g. an IDE's JS debugger) can push + # the debug browser to bind [::1] only. + _is_default = cdp_url == _DEFAULT_CDP + if _is_default: + _found = discover_local_cdp_url(_port, timeout=1.0) + _already_open = _found is not None + if _found: + cdp_url = _found + else: + _already_open = is_browser_debug_ready(cdp_url, timeout=1.0) if _already_open: - print(f" ✓ Chromium-family browser is already listening on port {_port}") - elif cdp_url == _DEFAULT_CDP: - # Try to auto-launch a Chromium-family browser with remote debugging - print(" Chromium-family browser isn't running with remote debugging — attempting to launch...") - _launch = launch_chrome_debug(_port, _plat.system()) + print(f" ✓ Chromium-family browser is already listening at {cdp_url}") + elif _is_default: + _launch_port = _port + if local_port_in_use(_port): + _launch_port = find_free_debug_port(_port) + print( + f" ⚠ Port {_port} is occupied by another application that isn't a CDP browser" + ) + print( + f" (an IDE debugger or dev server may be using it) — launching on port {_launch_port} instead..." + ) + else: + # Try to auto-launch a Chromium-family browser with remote debugging + print(" Chromium-family browser isn't running with remote debugging — attempting to launch...") + _launch = launch_chrome_debug(_launch_port, _plat.system()) if _launch.launched: # Wait for the DevTools discovery endpoint to come up for _wait in range(10): - if is_browser_debug_ready(cdp_url, timeout=1.0): + _found = discover_local_cdp_url(_launch_port, timeout=1.0) + if _found: + cdp_url = _found _already_open = True break time.sleep(0.5) if _already_open: - print(f" ✓ Chromium-family browser launched and listening on port {_port}") + print(f" ✓ Chromium-family browser launched and listening on port {_launch_port}") else: - print(f" ⚠ Browser launched but port {_port} isn't responding yet") + print(f" ⚠ Browser launched but port {_launch_port} isn't responding yet") print(" Try again in a few seconds — the debug instance may still be starting") else: print(" ⚠ Could not auto-launch a Chromium-family browser") @@ -1876,7 +1901,7 @@ class CLICommandsMixin: if _hint: print(f" {_hint}") sys_name = _plat.system() - chrome_cmd = manual_chrome_debug_command(_port, sys_name) + chrome_cmd = manual_chrome_debug_command(_launch_port, sys_name) if chrome_cmd: print(" Launch a Chromium-family browser manually:") print(f" {chrome_cmd}") diff --git a/tests/hermes_cli/test_browser_connect_dual_stack.py b/tests/hermes_cli/test_browser_connect_dual_stack.py new file mode 100644 index 00000000000..46af58adda9 --- /dev/null +++ b/tests/hermes_cli/test_browser_connect_dual_stack.py @@ -0,0 +1,122 @@ +"""Dual-stack loopback discovery + port-squatter handling for /browser connect. + +Regression context: on Windows, an IDE debugger (VS Code js-debug) holding +127.0.0.1:9222 pushes a Chrome launched with --remote-debugging-port=9222 +onto [::1]:9222 only. The old IPv4-only probe missed the live browser AND +hung against the squatter (accepts TCP, never answers HTTP), driving the +whole connect past the desktop GUI's RPC timeout +("error: request timed out: browser.manage"). +""" + +from __future__ import annotations + +import socket +import threading + +import pytest + +from hermes_cli.browser_connect import ( + DEFAULT_BROWSER_CDP_PORT, + discover_local_cdp_url, + find_free_debug_port, + local_port_in_use, +) + + +def _free_port() -> int: + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: + sock.bind(("127.0.0.1", 0)) + return sock.getsockname()[1] + + +@pytest.fixture +def ipv4_squatter(): + """A listener on the IPv4 loopback that accepts TCP but never speaks HTTP.""" + server = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + server.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + server.bind(("127.0.0.1", 0)) + server.listen(5) + port = server.getsockname()[1] + + conns: list[socket.socket] = [] + stop = threading.Event() + + def _accept_loop() -> None: + server.settimeout(0.2) + while not stop.is_set(): + try: + conn, _ = server.accept() + conns.append(conn) # hold open, say nothing — like a debug adapter + except OSError: + continue + + thread = threading.Thread(target=_accept_loop, daemon=True) + thread.start() + try: + yield port + finally: + stop.set() + thread.join(timeout=2) + for conn in conns: + try: + conn.close() + except OSError: + pass + server.close() + + +class TestDiscoverLocalCdpUrl: + def test_returns_none_when_nothing_listens(self): + port = _free_port() + assert discover_local_cdp_url(port, timeout=0.3) is None + + def test_does_not_hang_on_non_cdp_squatter(self, ipv4_squatter): + """A TCP-accepting, HTTP-silent squatter must fail the probe within + the timeout instead of being mistaken for a browser.""" + assert discover_local_cdp_url(ipv4_squatter, timeout=0.3) is None + + def test_finds_ipv6_only_endpoint(self, monkeypatch): + """When only [::1] speaks CDP (IPv4 side squatted), discovery + returns the IPv6 URL instead of giving up.""" + import hermes_cli.browser_connect as bc + + def _ready(url: str, timeout: float = 1.0) -> bool: + return "[::1]" in url + + monkeypatch.setattr(bc, "is_browser_debug_ready", _ready) + assert bc.discover_local_cdp_url(9222) == "http://[::1]:9222" + + def test_prefers_ipv4_when_both_answer(self, monkeypatch): + import hermes_cli.browser_connect as bc + + monkeypatch.setattr(bc, "is_browser_debug_ready", lambda *_a, **_k: True) + assert bc.discover_local_cdp_url(9222) == "http://127.0.0.1:9222" + + +class TestLocalPortInUse: + def test_free_port_reports_unused(self): + assert local_port_in_use(_free_port(), timeout=0.3) is False + + def test_squatted_port_reports_used(self, ipv4_squatter): + assert local_port_in_use(ipv4_squatter, timeout=0.5) is True + + +class TestFindFreeDebugPort: + def test_returns_port_above_preferred(self): + port = find_free_debug_port(DEFAULT_BROWSER_CDP_PORT) + assert port > DEFAULT_BROWSER_CDP_PORT + + def test_skips_occupied_successor(self): + """When preferred+1 is held on IPv4, the next candidate is chosen.""" + preferred = _free_port() + blocker = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + try: + blocker.bind(("127.0.0.1", preferred + 1)) + blocker.listen(1) + port = find_free_debug_port(preferred) + assert port != preferred + 1 + assert port > preferred + except OSError: + pytest.skip("successor port unavailable to bind in this environment") + finally: + blocker.close() diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index b58826bc117..a23804d5daa 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -8183,7 +8183,9 @@ def test_browser_manage_connect_sets_env_and_cleans_twice(monkeypatch): assert resp["result"]["connected"] is True assert resp["result"]["url"] == "http://127.0.0.1:9222" - assert resp["result"]["messages"] == ["Chromium-family browser is already listening on port 9222"] + assert resp["result"]["messages"] == [ + "Chromium-family browser is already listening at http://127.0.0.1:9222" + ] assert os.environ.get("BROWSER_CDP_URL") == "http://127.0.0.1:9222" # First cleanup runs against the OLD env (none here), second against the NEW. assert cleanup_calls == ["", "http://127.0.0.1:9222"] @@ -8203,7 +8205,9 @@ def test_browser_manage_connect_defaults_to_loopback(monkeypatch): assert resp["result"]["connected"] is True assert resp["result"]["url"] == "http://127.0.0.1:9222" - assert resp["result"]["messages"] == ["Chromium-family browser is already listening on port 9222"] + assert resp["result"]["messages"] == [ + "Chromium-family browser is already listening at http://127.0.0.1:9222" + ] assert urls[0] == "http://127.0.0.1:9222/json/version" @@ -8226,6 +8230,7 @@ def test_browser_manage_connect_default_local_reports_launch_hint(monkeypatch): "hermes_cli.browser_connect.launch_chrome_debug", return_value=ChromeDebugLaunch(), ), + patch("hermes_cli.browser_connect.local_port_in_use", return_value=False), patch("hermes_cli.browser_connect.manual_chrome_debug_command", return_value=None), patch( "hermes_cli.browser_connect.get_chrome_debug_candidates", @@ -8358,9 +8363,13 @@ def test_browser_manage_connect_default_local_retries_after_launch(monkeypatch): def __exit__(self, *_): return False + # IPv4 answers only from the 3rd probe onwards (browser still starting); + # the IPv6 loopback never answers. attempts = {"n": 0} - def _opener(_url, timeout=2.0): # noqa: ARG001 — match urllib signature + def _opener(url, timeout=2.0): # noqa: ARG001 — match urllib signature + if "[::1]" in url: + raise OSError("no IPv6 listener") attempts["n"] += 1 if attempts["n"] < 3: raise OSError("not ready") @@ -8369,9 +8378,14 @@ def test_browser_manage_connect_default_local_retries_after_launch(monkeypatch): import urllib.request monkeypatch.setattr(urllib.request, "urlopen", _opener) + launched = ChromeDebugLaunch(launched=True) with patch.dict(sys.modules, {"tools.browser_tool": fake}): - with patch( - "hermes_cli.browser_connect.try_launch_chrome_debug", return_value=True + with ( + patch( + "hermes_cli.browser_connect.launch_chrome_debug", + return_value=launched, + ), + patch("hermes_cli.browser_connect.local_port_in_use", return_value=False), ): resp = server.handle_request( {"id": "1", "method": "browser.manage", "params": {"action": "connect"}} @@ -8386,6 +8400,94 @@ def test_browser_manage_connect_default_local_retries_after_launch(monkeypatch): assert os.environ["BROWSER_CDP_URL"] == "http://127.0.0.1:9222" +def test_browser_manage_connect_finds_ipv6_only_browser(monkeypatch): + """Regression: an IDE debugger squatting 127.0.0.1:9222 pushes the debug + browser onto [::1]:9222. Connect must discover and adopt the IPv6 + endpoint instead of timing out against the squatter.""" + monkeypatch.delenv("BROWSER_CDP_URL", raising=False) + fake = types.SimpleNamespace( + cleanup_all_browsers=lambda: None, + _get_cdp_override=lambda: os.environ.get("BROWSER_CDP_URL", ""), + ) + + class _Resp: + status = 200 + + def __enter__(self): + return self + + def __exit__(self, *_): + return False + + def _opener(url, timeout=2.0): # noqa: ARG001 — match urllib signature + if "[::1]" in url: + return _Resp() + raise OSError("IPv4 loopback held by a non-CDP squatter") + + import urllib.request + + monkeypatch.setattr(urllib.request, "urlopen", _opener) + with patch.dict(sys.modules, {"tools.browser_tool": fake}): + resp = server.handle_request( + {"id": "1", "method": "browser.manage", "params": {"action": "connect"}} + ) + + assert resp["result"]["connected"] is True + assert resp["result"]["url"] == "http://[::1]:9222" + assert os.environ["BROWSER_CDP_URL"] == "http://[::1]:9222" + + +def test_browser_manage_connect_squatted_port_launches_on_alternate(monkeypatch): + """When neither loopback speaks CDP but the port is held by another + application, connect must pick an alternate port for the launch and + say so — never fight the squatter for 9222.""" + monkeypatch.delenv("BROWSER_CDP_URL", raising=False) + monkeypatch.setattr(server.time, "sleep", lambda _seconds: None) + fake = types.SimpleNamespace( + cleanup_all_browsers=lambda: None, + _get_cdp_override=lambda: os.environ.get("BROWSER_CDP_URL", ""), + ) + + class _Resp: + status = 200 + + def __enter__(self): + return self + + def __exit__(self, *_): + return False + + def _opener(url, timeout=2.0): # noqa: ARG001 — match urllib signature + if ":9223" in url and "127.0.0.1" in url: + return _Resp() # relaunched browser comes up on the alternate port + raise OSError("9222 squatted / nothing else listening") + + import urllib.request + + monkeypatch.setattr(urllib.request, "urlopen", _opener) + launch_ports: list[int] = [] + + def _launch(port, _system): + launch_ports.append(port) + return ChromeDebugLaunch(launched=True) + + with patch.dict(sys.modules, {"tools.browser_tool": fake}): + with ( + patch("hermes_cli.browser_connect.launch_chrome_debug", side_effect=_launch), + patch("hermes_cli.browser_connect.local_port_in_use", return_value=True), + patch("hermes_cli.browser_connect.find_free_debug_port", return_value=9223), + ): + resp = server.handle_request( + {"id": "1", "method": "browser.manage", "params": {"action": "connect"}} + ) + + assert launch_ports == [9223] + assert resp["result"]["connected"] is True + assert resp["result"]["url"] == "http://127.0.0.1:9223" + assert os.environ["BROWSER_CDP_URL"] == "http://127.0.0.1:9223" + assert any("occupied by another application" in m for m in resp["result"]["messages"]) + + def test_browser_manage_connect_rejects_unreachable_endpoint(monkeypatch): """An unreachable endpoint must NOT mutate the env or reap sessions.""" monkeypatch.setenv("BROWSER_CDP_URL", "http://existing:9222") diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 740fd3c4d02..280469f2ff5 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -14975,40 +14975,73 @@ def _browser_connect(rid, params: dict) -> dict: pass except OSError as e: return _err(rid, 5031, f"could not reach browser CDP at {url}: {e}") - else: - probes = _probe_urls(parsed) - ok = any(_http_ok(p, timeout=2.0) for p in probes) + elif _is_default_local_cdp(parsed): + from hermes_cli.browser_connect import ( + discover_local_cdp_url, + find_free_debug_port, + launch_chrome_debug, + local_port_in_use, + ) - if not ok and _is_default_local_cdp(parsed): - from hermes_cli.browser_connect import launch_chrome_debug + # Dual-stack discovery: when another app (an IDE debugger, + # a dev server) squats the IPv4 loopback on the debug port, + # a browser asked to bind that port comes up on [::1] only. + # An IPv4-only probe misses it AND hangs against squatters + # that accept TCP but never answer HTTP — the historic + # cause of `browser.manage` RPC timeouts. + discovered = discover_local_cdp_url(port, timeout=2.0) + launch_port = port - announce( - "Chromium-family browser isn't running with remote debugging — attempting to launch..." - ) + if discovered is None: + if local_port_in_use(port): + launch_port = find_free_debug_port(port) + announce( + f"Port {port} is occupied by another application that " + "isn't a CDP browser (an IDE debugger or dev server may " + f"be using it) — launching a debug browser on port " + f"{launch_port} instead..." + ) + else: + announce( + "Chromium-family browser isn't running with remote debugging — attempting to launch..." + ) - launch = launch_chrome_debug(port, system) + launch = launch_chrome_debug(launch_port, system) if launch.launched: - for _ in range(20): - time.sleep(0.5) - if any(_http_ok(p, timeout=1.0) for p in probes): - ok = True + # Bounded wait: the whole connect must finish well + # inside the client RPC timeout. + deadline = time.monotonic() + 10.0 + while time.monotonic() < deadline: + discovered = discover_local_cdp_url(launch_port, timeout=1.0) + if discovered: break + time.sleep(0.5) - if ok: - announce(f"Chromium-family browser launched and listening on port {port}") + if discovered: + announce( + f"Chromium-family browser launched and listening on port {launch_port}" + ) else: hint = launch.hint if hint: announce(hint, level="error") - for line in _failure_messages(url, port, system)[1:]: + for line in _failure_messages(url, launch_port, system)[1:]: announce(line, level="error") return _ok( rid, {"connected": False, "url": url, "messages": messages} ) - elif not ok: + else: + announce(f"Chromium-family browser is already listening at {discovered}") + + # Adopt whatever loopback/port actually answered (may be + # [::1] and/or an alternate port when 9222 was squatted). + url = discovered + parsed = urlparse(url) + else: + probes = _probe_urls(parsed) + ok = any(_http_ok(p, timeout=2.0) for p in probes) + if not ok: return _err(rid, 5031, f"could not reach browser CDP at {url}") - elif _is_default_local_cdp(parsed): - announce(f"Chromium-family browser is already listening on port {port}") normalized = _normalize_cdp_url(parsed)