From 3cd1bd971f25c98698bcea9f1e846b09f27d14f8 Mon Sep 17 00:00:00 2001 From: Frowtek Date: Fri, 5 Jun 2026 07:30:19 +0300 Subject: [PATCH] fix(cli): require Chromium for local browser readiness in setup/status surfaces --- hermes_cli/nous_subscription.py | 45 +++++++- hermes_cli/setup.py | 4 +- tests/hermes_cli/test_nous_subscription.py | 108 ++++++++++++++++++ tests/hermes_cli/test_setup_model_provider.py | 40 +++++++ 4 files changed, 193 insertions(+), 4 deletions(-) diff --git a/hermes_cli/nous_subscription.py b/hermes_cli/nous_subscription.py index 65cc1ca574c..d68f9865835 100644 --- a/hermes_cli/nous_subscription.py +++ b/hermes_cli/nous_subscription.py @@ -159,6 +159,33 @@ def _has_agent_browser() -> bool: return bool(agent_browser_bin or local_bin.exists()) +def _local_browser_runnable() -> bool: + """Return True when the *local* browser backend would actually start. + + The ``agent-browser`` CLI being present is necessary but not sufficient for + local mode: agent-browser also needs a Chromium build on disk (without one + it hangs on first use until the command timeout fires), unless the + Lightpanda engine is selected — text-only navigation needs no Chromium. + + This mirrors the local-mode tail of + :func:`tools.browser_tool.check_browser_requirements`, so the setup/status + surfaces advertise local browser readiness only when the runtime would + actually run it. Cloud providers (Browserbase, Browser Use, Firecrawl) host + their own Chromium and therefore gate on :func:`_has_agent_browser` alone. + """ + if not _has_agent_browser(): + return False + try: + from tools.browser_tool import _chromium_installed, _using_lightpanda_engine + except Exception: + # If the runtime probe can't be imported, fall back to binary presence + # (prior behaviour) rather than crashing the setup/status surface. + return True + if _using_lightpanda_engine(): + return True + return _chromium_installed() + + def _browser_label(current_provider: str) -> str: mapping = { "browserbase": "Browserbase", @@ -188,13 +215,23 @@ def _resolve_browser_feature_state( browser_provider: str, browser_provider_explicit: bool, browser_local_available: bool, + browser_local_runnable: bool, direct_camofox: bool, direct_browserbase: bool, direct_browser_use: bool, direct_firecrawl: bool, managed_browser_available: bool, ) -> tuple[str, bool, bool, bool]: - """Resolve browser availability using the same precedence as runtime.""" + """Resolve browser availability using the same precedence as runtime. + + ``browser_local_available`` means "the agent-browser CLI is present" — the + only local requirement for cloud providers, which host their own Chromium. + ``browser_local_runnable`` additionally requires a usable local Chromium + build (or the Lightpanda engine), mirroring the local-mode tail of + :func:`tools.browser_tool.check_browser_requirements`. Local mode must gate + on the latter, or setup/status advertise a browser that fails on first use + when Chromium is missing. + """ if direct_camofox: return "camofox", True, bool(browser_tool_enabled), False @@ -223,7 +260,7 @@ def _resolve_browser_feature_state( return current_provider, False, False, False current_provider = "local" - available = bool(browser_local_available) + available = bool(browser_local_runnable) active = bool(browser_tool_enabled and available) return current_provider, available, active, False @@ -243,7 +280,7 @@ def _resolve_browser_feature_state( active = bool(browser_tool_enabled and available) return "browserbase", available, active, False - available = bool(browser_local_available) + available = bool(browser_local_runnable) active = bool(browser_tool_enabled and available) return "local", available, active, False @@ -445,6 +482,7 @@ def get_nous_subscription_features( tts_active = bool(tts_tool_enabled and tts_available) browser_local_available = _has_agent_browser() + browser_local_runnable = _local_browser_runnable() ( browser_current_provider, browser_available, @@ -455,6 +493,7 @@ def get_nous_subscription_features( browser_provider=browser_provider, browser_provider_explicit=browser_provider_explicit, browser_local_available=browser_local_available, + browser_local_runnable=browser_local_runnable, direct_camofox=direct_camofox, direct_browserbase=direct_browserbase, direct_browser_use=direct_browser_use, diff --git a/hermes_cli/setup.py b/hermes_cli/setup.py index fe304dee089..266eb9eaa39 100644 --- a/hermes_cli/setup.py +++ b/hermes_cli/setup.py @@ -415,7 +415,9 @@ def _print_setup_summary(config: dict, hermes_home): elif browser_provider == "Camofox": missing_browser_hint = "CAMOFOX_URL" elif browser_provider == "Local browser": - missing_browser_hint = "npm install -g agent-browser" + missing_browser_hint = ( + "npm install -g agent-browser && agent-browser install --with-deps" + ) tool_status.append( ("Browser Automation", False, missing_browser_hint) ) diff --git a/tests/hermes_cli/test_nous_subscription.py b/tests/hermes_cli/test_nous_subscription.py index 752f6fabc28..152e9e06477 100644 --- a/tests/hermes_cli/test_nous_subscription.py +++ b/tests/hermes_cli/test_nous_subscription.py @@ -257,6 +257,114 @@ def test_get_gateway_eligible_tools_ignores_quoted_false_opt_in(monkeypatch): assert set(unconfigured) == {"image_gen", "video_gen", "tts", "browser"} +def _stub_browser_probes(monkeypatch, *, has_agent_browser, chromium, lightpanda=False): + """Common monkeypatches for local-browser readiness scenarios. + + ``chromium`` / ``lightpanda`` drive the runtime probes that + ``_local_browser_runnable`` reuses from ``tools.browser_tool`` (lazy import, + so patching the module attributes is enough). + """ + monkeypatch.setattr(ns, "get_env_value", lambda name: "") + monkeypatch.setattr( + ns, "get_nous_portal_account_info", lambda: _account(logged_in=False) + ) + monkeypatch.setattr(ns, "_toolset_enabled", lambda config, key: key == "browser") + monkeypatch.setattr(ns, "_has_agent_browser", lambda: has_agent_browser) + monkeypatch.setattr(ns, "resolve_openai_audio_api_key", lambda: "") + monkeypatch.setattr(ns, "has_direct_modal_credentials", lambda: False) + monkeypatch.setattr(ns, "is_managed_tool_gateway_ready", lambda vendor: False) + monkeypatch.setattr("tools.browser_tool._chromium_installed", lambda: chromium) + monkeypatch.setattr( + "tools.browser_tool._using_lightpanda_engine", lambda: lightpanda + ) + + +def test_local_browser_unavailable_without_chromium(monkeypatch): + """agent-browser present but Chromium absent must NOT advertise local browser. + + The runtime (``check_browser_requirements``) refuses local mode without a + Chromium build, so the setup/status surface must report unavailable too — + otherwise the user sees "Browser Automation available" and the first real + call fails. Regression for the false-positive setup bug. + """ + _stub_browser_probes(monkeypatch, has_agent_browser=True, chromium=False) + + features = ns.get_nous_subscription_features( + {"browser": {"cloud_provider": "local"}} + ) + + assert features.browser.available is False + assert features.browser.active is False + assert features.browser.managed_by_nous is False + assert features.browser.current_provider == "Local browser" + + +def test_local_browser_available_with_chromium(monkeypatch): + _stub_browser_probes(monkeypatch, has_agent_browser=True, chromium=True) + + features = ns.get_nous_subscription_features( + {"browser": {"cloud_provider": "local"}} + ) + + assert features.browser.available is True + assert features.browser.active is True + assert features.browser.current_provider == "Local browser" + + +def test_local_browser_available_with_lightpanda_without_chromium(monkeypatch): + """Lightpanda is text-only and needs no Chromium, so it stays available. + + Guards against the fix over-correcting into a false-negative for the + legitimate Lightpanda-without-Chromium configuration. + """ + _stub_browser_probes( + monkeypatch, has_agent_browser=True, chromium=False, lightpanda=True + ) + + features = ns.get_nous_subscription_features( + {"browser": {"cloud_provider": "local"}} + ) + + assert features.browser.available is True + assert features.browser.active is True + + +def test_default_local_browser_unavailable_without_chromium(monkeypatch): + """The implicit (no cloud_provider) local fallthrough is gated on Chromium too.""" + _stub_browser_probes(monkeypatch, has_agent_browser=True, chromium=False) + + features = ns.get_nous_subscription_features({}) + + assert features.browser.available is False + assert features.browser.current_provider == "Local browser" + + +def test_cloud_browserbase_available_without_local_chromium(monkeypatch): + """Cloud providers host their own Chromium, so the new local gate must not + regress them: agent-browser binary present + Browserbase creds is enough.""" + env = {"BROWSERBASE_API_KEY": "bb-key", "BROWSERBASE_PROJECT_ID": "bb-project"} + monkeypatch.setattr(ns, "get_env_value", lambda name: env.get(name, "")) + monkeypatch.setattr( + ns, "get_nous_portal_account_info", lambda: _account(logged_in=False) + ) + monkeypatch.setattr(ns, "_toolset_enabled", lambda config, key: key == "browser") + monkeypatch.setattr(ns, "_has_agent_browser", lambda: True) + monkeypatch.setattr(ns, "resolve_openai_audio_api_key", lambda: "") + monkeypatch.setattr(ns, "has_direct_modal_credentials", lambda: False) + monkeypatch.setattr(ns, "is_managed_tool_gateway_ready", lambda vendor: False) + # Chromium absent locally — must not matter for a cloud provider. + monkeypatch.setattr("tools.browser_tool._chromium_installed", lambda: False) + monkeypatch.setattr("tools.browser_tool._using_lightpanda_engine", lambda: False) + + features = ns.get_nous_subscription_features( + {"browser": {"cloud_provider": "browserbase"}} + ) + + assert features.browser.available is True + assert features.browser.active is True + assert features.browser.current_provider == "Browserbase" + + def test_get_gateway_eligible_tools_pool_excludes_video(monkeypatch): """A free-tool-pool user is offered the covered tools but NOT video gen.""" monkeypatch.setattr(ns, "get_nous_portal_account_info", lambda **kw: _pool_account()) diff --git a/tests/hermes_cli/test_setup_model_provider.py b/tests/hermes_cli/test_setup_model_provider.py index 099f4eb94b2..b9db444ddc7 100644 --- a/tests/hermes_cli/test_setup_model_provider.py +++ b/tests/hermes_cli/test_setup_model_provider.py @@ -344,3 +344,43 @@ def test_setup_summary_does_not_mark_incomplete_browserbase_as_available(tmp_pat assert "Browser Automation (Browserbase)" not in output assert "Browser Automation" in output assert "BROWSERBASE_API_KEY/BROWSERBASE_PROJECT_ID" in output + + +def test_setup_summary_local_browser_unavailable_without_chromium( + tmp_path, monkeypatch, capsys +): + """End-to-end: agent-browser present but no Chromium in local mode must + render as unavailable with an install hint — not a false 'available'. + + Unlike the mocked-feature tests above, this drives the real + ``get_nous_subscription_features`` so the surface stays aligned with the + runtime gate in ``tools.browser_tool.check_browser_requirements``. + """ + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + _clear_provider_env(monkeypatch) + + cfg = load_config() + browser_cfg = cfg.get("browser") + if not isinstance(browser_cfg, dict): + browser_cfg = {} + cfg["browser"] = browser_cfg + browser_cfg["cloud_provider"] = "local" + save_config(cfg) + + # Only stub the readiness probes; the feature resolver itself is real. + monkeypatch.setattr("hermes_cli.nous_subscription._has_agent_browser", lambda: True) + monkeypatch.setattr( + "hermes_cli.nous_subscription.get_nous_portal_account_info", + lambda *a, **k: None, + ) + monkeypatch.setattr("tools.browser_tool._chromium_installed", lambda: False) + monkeypatch.setattr("tools.browser_tool._using_lightpanda_engine", lambda: False) + monkeypatch.setattr( + "agent.auxiliary_client.get_available_vision_backends", lambda: [] + ) + + _print_setup_summary(load_config(), tmp_path) + output = capsys.readouterr().out + + assert "Browser Automation (Local browser)" not in output + assert "agent-browser install --with-deps" in output