From 42be5e49b05f30c7a6508c50443a1671120c8afc Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 28 Apr 2026 07:03:44 -0700 Subject: [PATCH] fix(browser): detect missing Chromium and fail fast with actionable error (#17039) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, check_browser_requirements() only checked for the agent-browser CLI, not the Chromium binary it drives. When the CLI was present but Chromium wasn't (common in Docker images predating the playwright install step), the browser tool was advertised to the agent, every call hung for the full command timeout (~30s each, ~220s for a chained navigate), and the agent eventually gave up with no useful error — users saw 'browser not working' with empty errors.log. Changes: - tools/browser_tool.py: add _chromium_installed() checking PLAYWRIGHT_BROWSERS_PATH + default Playwright cache paths for chromium-* / chromium_headless_shell-* dirs; wire into check_browser_requirements() for local mode (cloud providers unaffected). _run_browser_command fails fast with an actionable Docker vs. host message instead of hanging. _running_in_docker() checks /.dockerenv and /proc/1/cgroup. - hermes_cli/tools_config.py: post_setup for 'Local Browser' now runs 'agent-browser install --with-deps' after npm install to actually download Chromium. In Docker, points user at the updated image pull instead of trying to install into a read-only layer. Cloud-provider post_setup (browserbase) skips Chromium install entirely. - tests/tools/test_browser_chromium_check.py: new tests covering search roots, install detection, requirements branches (local/cloud/ camofox), and the fast-fail guard in docker/non-docker contexts. - tests/tools/test_browser_homebrew_paths.py: 5 existing subprocess-path tests now mock _chromium_installed=True since they exercise the post-guard subprocess path. Co-authored-by: teknium1 --- hermes_cli/tools_config.py | 91 ++++++++++- tests/tools/test_browser_chromium_check.py | 176 +++++++++++++++++++++ tests/tools/test_browser_homebrew_paths.py | 5 + tools/browser_tool.py | 130 ++++++++++++++- 4 files changed, 395 insertions(+), 7 deletions(-) create mode 100644 tests/tools/test_browser_chromium_check.py diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index aec4c131..b4a19f0b 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -467,7 +467,10 @@ def _run_post_setup(post_setup_key: str): import shutil if post_setup_key in ("agent_browser", "browserbase"): node_modules = PROJECT_ROOT / "node_modules" / "agent-browser" - if not node_modules.exists() and shutil.which("npm"): + npm_bin = shutil.which("npm") + npx_bin = shutil.which("npx") + # Step 1: install the agent-browser npm package into node_modules/ + if not node_modules.exists() and npm_bin: _print_info(" Installing Node.js dependencies for browser tools...") import subprocess result = subprocess.run( @@ -479,8 +482,94 @@ def _run_post_setup(post_setup_key: str): else: from hermes_constants import display_hermes_home _print_warning(f" npm install failed - run manually: cd {display_hermes_home()}/hermes-agent && npm install") + if result.stderr: + _print_info(f" {result.stderr.strip()[:200]}") elif not node_modules.exists(): _print_warning(" Node.js not found - browser tools require: npm install (in hermes-agent directory)") + return + + # Step 2: only the local browser provider actually needs Chromium on + # disk. Cloud providers (Browserbase, Browser Use, Firecrawl) host + # their own Chromium and don't need the local install. + if post_setup_key != "agent_browser": + return + + # Step 3: ensure the Chromium / headless-shell build agent-browser + # drives is actually installed. Without it the CLI hangs on first + # use until the command timeout fires. Skip inside Docker — the + # image bakes Chromium in at build time, and runtime users usually + # can't write to PLAYWRIGHT_BROWSERS_PATH anyway. + try: + # Import lazily so the tools_config UI doesn't pull in the full + # browser_tool module at import time. + from tools.browser_tool import ( + _chromium_installed, + _running_in_docker, + ) + except Exception as exc: # pragma: no cover — defensive + _print_warning(f" Could not check Chromium status: {exc}") + return + + if _chromium_installed(): + _print_success(" Chromium browser already installed") + return + + if _running_in_docker(): + _print_warning( + " Chromium is missing but you're running in Docker." + ) + _print_info( + " Pull the latest image to get the bundled Chromium:" + ) + _print_info( + " docker pull ghcr.io/nousresearch/hermes-agent:latest" + ) + return + + if not npx_bin: + _print_warning( + " npx not found - install Chromium manually: npx agent-browser install --with-deps" + ) + return + + _print_info(" Installing Chromium (~170MB one-time download)...") + import subprocess + # Prefer the bundled agent-browser install subcommand so the + # version of Chromium matches the CLI. Fall back to npx shim on + # setups where the local bin stub isn't present. + local_ab = PROJECT_ROOT / "node_modules" / ".bin" / "agent-browser" + if sys.platform == "win32": + local_ab_win = local_ab.with_suffix(".cmd") + if local_ab_win.exists(): + local_ab = local_ab_win + install_cmd = ( + [str(local_ab), "install", "--with-deps"] + if local_ab.exists() + else [npx_bin, "-y", "agent-browser", "install", "--with-deps"] + ) + try: + result = subprocess.run( + install_cmd, + capture_output=True, text=True, cwd=str(PROJECT_ROOT), timeout=600, + ) + if result.returncode == 0: + _print_success(" Chromium installed") + # Invalidate the cached "missing" result so subsequent + # check_browser_requirements() calls see the new install. + import tools.browser_tool as _bt + _bt._cached_chromium_installed = None + else: + _print_warning(" Chromium install failed:") + tail = (result.stderr or result.stdout or "").strip().splitlines()[-3:] + for line in tail: + _print_info(f" {line[:200]}") + _print_info(" Run manually: npx agent-browser install --with-deps") + except subprocess.TimeoutExpired: + _print_warning(" Chromium install timed out (>10min)") + _print_info(" Run manually: npx agent-browser install --with-deps") + except Exception as exc: + _print_warning(f" Chromium install failed: {exc}") + _print_info(" Run manually: npx agent-browser install --with-deps") elif post_setup_key == "camofox": camofox_dir = PROJECT_ROOT / "node_modules" / "@askjo" / "camofox-browser" diff --git a/tests/tools/test_browser_chromium_check.py b/tests/tools/test_browser_chromium_check.py new file mode 100644 index 00000000..a09758a2 --- /dev/null +++ b/tests/tools/test_browser_chromium_check.py @@ -0,0 +1,176 @@ +"""Tests for Chromium-presence detection in browser_tool. + +Regression guard for the "browser tool advertised but Chromium missing" +class of bug — where ``agent-browser`` CLI is discoverable but no +Chromium build is on disk, causing every browser_* tool call to hang +for the full command timeout before surfacing a useless error. +""" + +import os +from pathlib import Path + +import pytest + +from tools import browser_tool as bt + + +@pytest.fixture(autouse=True) +def _reset_chromium_cache(): + bt._cached_chromium_installed = None + yield + bt._cached_chromium_installed = None + + +class TestChromiumSearchRoots: + def test_respects_playwright_browsers_path_env(self, monkeypatch, tmp_path): + monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", str(tmp_path)) + roots = bt._chromium_search_roots() + assert str(tmp_path) == roots[0] + + def test_ignores_playwright_browsers_path_zero(self, monkeypatch): + # Playwright treats "0" as "skip browser download" — not a real path. + monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", "0") + roots = bt._chromium_search_roots() + assert "0" not in roots + + def test_always_includes_default_ms_playwright_cache(self, monkeypatch): + monkeypatch.delenv("PLAYWRIGHT_BROWSERS_PATH", raising=False) + roots = bt._chromium_search_roots() + home = os.path.expanduser("~") + assert any(r == os.path.join(home, ".cache", "ms-playwright") for r in roots) + + +class TestChromiumInstalled: + def test_true_when_chromium_dir_present(self, monkeypatch, tmp_path): + monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", str(tmp_path)) + (tmp_path / "chromium-1208").mkdir() + assert bt._chromium_installed() is True + + def test_true_when_headless_shell_present(self, monkeypatch, tmp_path): + monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", str(tmp_path)) + (tmp_path / "chromium_headless_shell-1208").mkdir() + assert bt._chromium_installed() is True + + def test_false_when_dir_empty(self, monkeypatch, tmp_path): + monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", str(tmp_path)) + monkeypatch.setattr("os.path.expanduser", lambda p: str(tmp_path / "fakehome")) + assert bt._chromium_installed() is False + + def test_false_when_only_unrelated_browsers(self, monkeypatch, tmp_path): + monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", str(tmp_path)) + monkeypatch.setattr("os.path.expanduser", lambda p: str(tmp_path / "fakehome")) + (tmp_path / "firefox-1234").mkdir() + (tmp_path / "webkit-5678").mkdir() + assert bt._chromium_installed() is False + + def test_false_when_path_not_a_dir(self, monkeypatch, tmp_path): + # User points PLAYWRIGHT_BROWSERS_PATH at a file by mistake. + bogus = tmp_path / "nope" + bogus.write_text("") + monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", str(bogus)) + monkeypatch.setattr("os.path.expanduser", lambda p: str(tmp_path / "fakehome")) + assert bt._chromium_installed() is False + + def test_result_cached(self, monkeypatch, tmp_path): + monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", str(tmp_path)) + (tmp_path / "chromium-1208").mkdir() + assert bt._chromium_installed() is True + # Delete after first call — cached True should still return True. + (tmp_path / "chromium-1208").rmdir() + assert bt._chromium_installed() is True + + +class TestCheckBrowserRequirementsChromium: + def test_local_mode_missing_chromium_returns_false(self, monkeypatch, tmp_path): + monkeypatch.setattr(bt, "_is_camofox_mode", lambda: False) + monkeypatch.setattr(bt, "_find_agent_browser", lambda: "/usr/local/bin/agent-browser") + monkeypatch.setattr(bt, "_requires_real_termux_browser_install", lambda _: False) + monkeypatch.setattr(bt, "_get_cloud_provider", lambda: None) + monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", str(tmp_path)) + monkeypatch.setattr("os.path.expanduser", lambda p: str(tmp_path / "fakehome")) + + assert bt.check_browser_requirements() is False + + def test_local_mode_with_chromium_returns_true(self, monkeypatch, tmp_path): + monkeypatch.setattr(bt, "_is_camofox_mode", lambda: False) + monkeypatch.setattr(bt, "_find_agent_browser", lambda: "/usr/local/bin/agent-browser") + monkeypatch.setattr(bt, "_requires_real_termux_browser_install", lambda _: False) + monkeypatch.setattr(bt, "_get_cloud_provider", lambda: None) + monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", str(tmp_path)) + (tmp_path / "chromium-1208").mkdir() + + assert bt.check_browser_requirements() is True + + def test_cloud_mode_does_not_require_local_chromium(self, monkeypatch, tmp_path): + """Cloud browsers (Browserbase etc.) host their own Chromium.""" + class FakeProvider: + def is_configured(self): + return True + def provider_name(self): + return "browserbase" + + monkeypatch.setattr(bt, "_is_camofox_mode", lambda: False) + monkeypatch.setattr(bt, "_find_agent_browser", lambda: "/usr/local/bin/agent-browser") + monkeypatch.setattr(bt, "_requires_real_termux_browser_install", lambda _: False) + monkeypatch.setattr(bt, "_get_cloud_provider", lambda: FakeProvider()) + # Point chromium search at an empty dir — should not matter for cloud. + monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", str(tmp_path)) + monkeypatch.setattr("os.path.expanduser", lambda p: str(tmp_path / "fakehome")) + + assert bt.check_browser_requirements() is True + + def test_camofox_mode_does_not_require_chromium(self, monkeypatch, tmp_path): + monkeypatch.setattr(bt, "_is_camofox_mode", lambda: True) + # Even with no chromium on disk, camofox drives its own backend. + monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", str(tmp_path)) + monkeypatch.setattr("os.path.expanduser", lambda p: str(tmp_path / "fakehome")) + + assert bt.check_browser_requirements() is True + + +class TestRunBrowserCommandChromiumGuard: + """Verify _run_browser_command fails fast (no timeout hang) when + Chromium is missing in local mode. + """ + + def test_local_mode_missing_chromium_returns_error_immediately(self, monkeypatch, tmp_path): + monkeypatch.setattr(bt, "_find_agent_browser", lambda: "/usr/local/bin/agent-browser") + monkeypatch.setattr(bt, "_requires_real_termux_browser_install", lambda _: False) + monkeypatch.setattr(bt, "_is_local_mode", lambda: True) + monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", str(tmp_path)) + monkeypatch.setattr("os.path.expanduser", lambda p: str(tmp_path / "fakehome")) + + # If we ever reached subprocess.Popen the test would hang — the + # fast-fail guard prevents that. + def _fail_popen(*args, **kwargs): + raise AssertionError("Should have failed before spawning subprocess") + + monkeypatch.setattr("subprocess.Popen", _fail_popen) + + result = bt._run_browser_command("task-1", "navigate", ["https://example.com"]) + assert result["success"] is False + assert "Chromium" in result["error"] + + def test_docker_hint_mentions_image_pull(self, monkeypatch, tmp_path): + monkeypatch.setattr(bt, "_find_agent_browser", lambda: "/usr/local/bin/agent-browser") + monkeypatch.setattr(bt, "_requires_real_termux_browser_install", lambda _: False) + monkeypatch.setattr(bt, "_is_local_mode", lambda: True) + monkeypatch.setattr(bt, "_running_in_docker", lambda: True) + monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", str(tmp_path)) + monkeypatch.setattr("os.path.expanduser", lambda p: str(tmp_path / "fakehome")) + + result = bt._run_browser_command("task-1", "navigate", ["https://example.com"]) + assert result["success"] is False + assert "docker pull" in result["error"].lower() + + def test_non_docker_hint_mentions_agent_browser_install(self, monkeypatch, tmp_path): + monkeypatch.setattr(bt, "_find_agent_browser", lambda: "/usr/local/bin/agent-browser") + monkeypatch.setattr(bt, "_requires_real_termux_browser_install", lambda _: False) + monkeypatch.setattr(bt, "_is_local_mode", lambda: True) + monkeypatch.setattr(bt, "_running_in_docker", lambda: False) + monkeypatch.setenv("PLAYWRIGHT_BROWSERS_PATH", str(tmp_path)) + monkeypatch.setattr("os.path.expanduser", lambda p: str(tmp_path / "fakehome")) + + result = bt._run_browser_command("task-1", "navigate", ["https://example.com"]) + assert result["success"] is False + assert "agent-browser install" in result["error"] diff --git a/tests/tools/test_browser_homebrew_paths.py b/tests/tools/test_browser_homebrew_paths.py index 772a0b46..eb4a6998 100644 --- a/tests/tools/test_browser_homebrew_paths.py +++ b/tests/tools/test_browser_homebrew_paths.py @@ -259,6 +259,7 @@ class TestRunBrowserCommandPathConstruction: hermes_home = str(tmp_path / "hermes-home") with patch("tools.browser_tool._find_agent_browser", return_value=browser_path), \ + patch("tools.browser_tool._chromium_installed", return_value=True), \ patch("tools.browser_tool._get_session_info", return_value=fake_session), \ patch("tools.browser_tool._socket_safe_tmpdir", return_value=str(tmp_path)), \ patch("tools.browser_tool._discover_homebrew_node_dirs", return_value=[]), \ @@ -310,6 +311,7 @@ class TestRunBrowserCommandPathConstruction: hermes_home = str(tmp_path / "hermes-home") with patch("tools.browser_tool._find_agent_browser", return_value="npx agent-browser"), \ + patch("tools.browser_tool._chromium_installed", return_value=True), \ patch("tools.browser_tool._get_session_info", return_value=fake_session), \ patch("tools.browser_tool._socket_safe_tmpdir", return_value=str(tmp_path)), \ patch("tools.browser_tool._discover_homebrew_node_dirs", return_value=[]), \ @@ -381,6 +383,7 @@ class TestRunBrowserCommandPathConstruction: return real_isdir(p) with patch("tools.browser_tool._find_agent_browser", return_value="/usr/local/bin/agent-browser"), \ + patch("tools.browser_tool._chromium_installed", return_value=True), \ patch("tools.browser_tool._get_session_info", return_value=fake_session), \ patch("tools.browser_tool._socket_safe_tmpdir", return_value=str(tmp_path)), \ patch("tools.browser_tool._discover_homebrew_node_dirs", return_value=fake_homebrew_dirs), \ @@ -429,6 +432,7 @@ class TestRunBrowserCommandPathConstruction: return real_isdir(p) with patch("tools.browser_tool._find_agent_browser", return_value="/usr/local/bin/agent-browser"), \ + patch("tools.browser_tool._chromium_installed", return_value=True), \ patch("tools.browser_tool._get_session_info", return_value=fake_session), \ patch("tools.browser_tool._socket_safe_tmpdir", return_value=str(tmp_path)), \ patch("tools.browser_tool._discover_homebrew_node_dirs", return_value=[]), \ @@ -477,6 +481,7 @@ class TestRunBrowserCommandPathConstruction: return real_isdir(path) with patch("tools.browser_tool._find_agent_browser", return_value="/usr/local/bin/agent-browser"), \ + patch("tools.browser_tool._chromium_installed", return_value=True), \ patch("tools.browser_tool._get_session_info", return_value=fake_session), \ patch("tools.browser_tool._socket_safe_tmpdir", return_value=str(tmp_path)), \ patch("tools.browser_tool._discover_homebrew_node_dirs", return_value=[]), \ diff --git a/tools/browser_tool.py b/tools/browser_tool.py index 39cdaf78..362a1575 100644 --- a/tools/browser_tool.py +++ b/tools/browser_tool.py @@ -1399,6 +1399,24 @@ def _run_browser_command( error = _termux_browser_install_error() logger.warning("browser command blocked on Termux: %s", error) return {"success": False, "error": error} + + # Local mode with no Chromium on disk: fail fast with an actionable + # message instead of hanging for _command_timeout seconds per call. + if _is_local_mode() and not _chromium_installed(): + if _running_in_docker(): + hint = ( + "Chromium browser is missing. You're running in Docker — pull " + "the latest image to get the bundled Chromium: " + "docker pull ghcr.io/nousresearch/hermes-agent:latest" + ) + else: + hint = ( + "Chromium browser is missing. Install it with: " + "npx agent-browser install --with-deps " + "(or: npx playwright install --with-deps chromium)" + ) + logger.warning("browser command blocked: %s", hint) + return {"success": False, "error": hint} from tools.interrupt import is_interrupted if is_interrupted(): @@ -2690,26 +2708,106 @@ def cleanup_all_browsers() -> None: # Reset cached lookups so they are re-evaluated on next use. global _cached_agent_browser, _agent_browser_resolved global _cached_command_timeout, _command_timeout_resolved + global _cached_chromium_installed _cached_agent_browser = None _agent_browser_resolved = False _discover_homebrew_node_dirs.cache_clear() _cached_command_timeout = None _command_timeout_resolved = False - + _cached_chromium_installed = None # ============================================================================ # Requirements Check # ============================================================================ + +# Cache for Chromium discovery. Invalidated by _reset_browser_caches. +_cached_chromium_installed: Optional[bool] = None + + +def _chromium_search_roots() -> List[str]: + """Directories to scan for a Chromium / headless-shell build. + + Order mirrors what agent-browser and Playwright actually probe: + + 1. ``PLAYWRIGHT_BROWSERS_PATH`` when set (Docker image sets this to + ``/opt/hermes/.playwright``). + 2. ``~/.cache/ms-playwright`` — Playwright's default on Linux/macOS. + 3. ``~/Library/Caches/ms-playwright`` — Playwright's default on macOS. + 4. ``%USERPROFILE%\\AppData\\Local\\ms-playwright`` — Playwright's default + on Windows. + """ + roots: List[str] = [] + env_path = os.environ.get("PLAYWRIGHT_BROWSERS_PATH", "").strip() + if env_path and env_path != "0": + roots.append(env_path) + home = os.path.expanduser("~") + roots.append(os.path.join(home, ".cache", "ms-playwright")) + if sys.platform == "darwin": + roots.append(os.path.join(home, "Library", "Caches", "ms-playwright")) + if sys.platform == "win32": + local = os.environ.get("LOCALAPPDATA") or os.path.join( + home, "AppData", "Local" + ) + roots.append(os.path.join(local, "ms-playwright")) + return roots + + +def _chromium_installed() -> bool: + """Return True when a usable Chromium (or headless-shell) build is on disk. + + agent-browser (0.26+) downloads Playwright's chromium / headless-shell + builds into ``PLAYWRIGHT_BROWSERS_PATH`` and won't start without them. + When the CLI is present but no browser build is, the first browser tool + call hangs for the full command timeout (often ~30s each) before + surfacing a useless error. Guarding the tool behind this check prevents + advertising a capability that will fail at runtime. + """ + global _cached_chromium_installed + if _cached_chromium_installed is not None: + return _cached_chromium_installed + + for root in _chromium_search_roots(): + if not root or not os.path.isdir(root): + continue + try: + entries = os.listdir(root) + except OSError: + continue + # Playwright names them ``chromium-`` and + # ``chromium_headless_shell-``; agent-browser accepts either. + for entry in entries: + if entry.startswith("chromium-") or entry.startswith( + "chromium_headless_shell-" + ): + _cached_chromium_installed = True + return True + + _cached_chromium_installed = False + return False + + +def _running_in_docker() -> bool: + """Best-effort detection of whether we're inside a Docker container.""" + if os.path.exists("/.dockerenv"): + return True + try: + with open("/proc/1/cgroup", "rt") as fp: + return "docker" in fp.read() + except OSError: + return False + + def check_browser_requirements() -> bool: """ Check if browser tool requirements are met. - In **local mode** (no cloud provider configured): only the - ``agent-browser`` CLI must be findable. + In **local mode** (no cloud provider configured): the ``agent-browser`` + CLI must be findable *and* a Chromium build must be installed on disk. In **cloud mode** (Browserbase, Browser Use, or Firecrawl): the CLI - *and* the provider's required credentials must be present. + and the provider's required credentials must be present. The cloud + provider hosts its own Chromium, so no local browser binary is needed. Returns: True if all requirements are met, False otherwise @@ -2731,9 +2829,15 @@ def check_browser_requirements() -> bool: if _requires_real_termux_browser_install(browser_cmd): return False - # In cloud mode, also require provider credentials + # In cloud mode, also require provider credentials. Cloud browsers + # don't need a local Chromium binary. provider = _get_cloud_provider() - if provider is not None and not provider.is_configured(): + if provider is not None: + return provider.is_configured() + + # Local mode: agent-browser needs a Chromium build on disk. Without it + # the CLI hangs on first use until the command timeout fires. + if not _chromium_installed(): return False return True @@ -2764,6 +2868,20 @@ if __name__ == "__main__": if _requires_real_termux_browser_install(browser_cmd): print(" - bare npx fallback found (insufficient on Termux local mode)") print(f" Install: {_browser_install_hint()}") + elif _cp is None and not _chromium_installed(): + print(" - Chromium browser binary not found") + searched = ", ".join(_chromium_search_roots()) or "(no candidate paths)" + print(f" Searched: {searched}") + if _running_in_docker(): + print( + " Docker: pull the latest image — the current one " + "predates the bundled Chromium install" + ) + print(" docker pull ghcr.io/nousresearch/hermes-agent:latest") + else: + print(" Install it with:") + print(" npx agent-browser install --with-deps") + print(" Or: npx playwright install --with-deps chromium") except FileNotFoundError: print(" - agent-browser CLI not found") print(f" Install: {_browser_install_hint()}")