mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-09 08:21:50 +00:00
fix(cli): require Chromium for local browser readiness in setup/status surfaces
This commit is contained in:
parent
ec46f5912e
commit
3cd1bd971f
4 changed files with 193 additions and 4 deletions
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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())
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue