diff --git a/hermes_cli/nous_subscription.py b/hermes_cli/nous_subscription.py index 4a05aa8491..735b685866 100644 --- a/hermes_cli/nous_subscription.py +++ b/hermes_cli/nous_subscription.py @@ -159,35 +159,43 @@ def _toolset_enabled(config: Dict[str, object], toolset_key: str) -> bool: def _has_agent_browser() -> bool: import shutil - from hermes_constants import agent_browser_runnable, with_hermes_node_path + from hermes_constants import agent_browser_runnable - # Validate the resolved binary actually runs — a dangling global symlink - # (issue #48521) is reported by ``which`` but fails at exec. Fall through to - # the local node_modules copy, which the validator also checks. - if agent_browser_runnable(shutil.which("agent-browser")): - return True - - # Hermes-managed Node dirs (Windows installer / POSIX $HERMES_HOME/node) - # are prepended to PATH at runtime but usually absent from the *probe* - # process's PATH — the same rung `_find_agent_browser` searches. Without - # it a successful install keeps reporting "needs setup" on Windows. - managed_path = with_hermes_node_path().get("PATH", "") - if managed_path: - managed_hit = shutil.which("agent-browser", path=managed_path) - if managed_hit and agent_browser_runnable(managed_hit): + # agent-browser is no longer a root package.json dependency (#43564) — it + # resolves lazily via npx for most installs, which a bare PATH + + # node_modules probe can't see. Mirror the local-CLI tail of + # :func:`tools.browser_tool.check_browser_requirements` (same cascade, same + # Termux carve-out) so the setup/status surfaces can't diverge from what + # browser tools actually find at runtime; validate=False keeps this a cheap + # existence check with no subprocess spawn. + try: + from tools.browser_tool import ( + _find_agent_browser, + _requires_real_termux_browser_install, + ) + except Exception: + # If the runtime probe can't be imported, fall back to binary presence + # (prior behaviour) rather than crashing the setup/status surface. + # Validate the resolved binary actually runs — a dangling global + # symlink (issue #48521) is reported by ``which`` but fails at exec. + # Fall through to the local node_modules copy, which the validator + # also checks. + if agent_browser_runnable(shutil.which("agent-browser")): return True + local_bin = ( + Path(__file__).parent.parent / "node_modules" / ".bin" / "agent-browser" + ) + return agent_browser_runnable(str(local_bin)) if local_bin.exists() else False - # Local node_modules/.bin: resolve via PATHEXT-aware ``shutil.which`` so - # Windows picks the executable ``.cmd`` shim. Probing the extensionless - # POSIX shim directly fails exec (WinError 193) even right after a - # successful ``npm install`` — the bug that pinned every browser row on - # "Setup required" in the desktop GUI. - local_bin_dir = Path(__file__).parent.parent / "node_modules" / ".bin" - if local_bin_dir.is_dir(): - local_which = shutil.which("agent-browser", path=str(local_bin_dir)) - if local_which and agent_browser_runnable(local_which): - return True - return False + try: + browser_cmd = _find_agent_browser(validate=False) + except FileNotFoundError: + return False + # On Termux, the bare npx fallback is too fragile to advertise as ready — + # require a real install, matching check_browser_requirements. + if _requires_real_termux_browser_install(browser_cmd): + return False + return True def _local_browser_runnable() -> bool: diff --git a/tests/hermes_cli/test_nous_subscription.py b/tests/hermes_cli/test_nous_subscription.py index 052b88dd0f..6397cc9179 100644 --- a/tests/hermes_cli/test_nous_subscription.py +++ b/tests/hermes_cli/test_nous_subscription.py @@ -1,5 +1,8 @@ """Tests for Nous subscription feature detection.""" +import shutil +import sys + from hermes_cli.nous_account import NousPortalAccountInfo, NousToolAccessInfo from hermes_cli import nous_subscription as ns @@ -207,27 +210,96 @@ def _stt_features_stub(*, account_info): -def test_has_agent_browser_resolves_via_hermes_managed_node_path(monkeypatch, tmp_path): - """The managed-Node rung: a runnable agent-browser under the Hermes Node - dir must count even when it's absent from the probe process's PATH (the - Windows installer shape — install succeeded, GUI still said needs setup).""" - import shutil as _shutil - - managed_dir = tmp_path / "node" - managed_dir.mkdir() - managed_bin = managed_dir / "agent-browser" - managed_bin.write_text("#!/bin/sh\nexit 0\n") - managed_bin.chmod(0o755) - - monkeypatch.setattr(_shutil, "which", lambda cmd, path=None: str(managed_bin) if path else None) +def _block_legacy_agent_browser_checks(monkeypatch): + """Make the legacy checks (PATH lookup + local node_modules/.bin) find nothing.""" + real_which = shutil.which monkeypatch.setattr( - "hermes_constants.with_hermes_node_path", lambda: {"PATH": str(managed_dir)} + shutil, + "which", + lambda cmd, *args, **kwargs: ( + None if cmd == "agent-browser" else real_which(cmd, *args, **kwargs) + ), + ) + monkeypatch.setattr("hermes_constants.agent_browser_runnable", lambda path: False) + + +def test_has_agent_browser_true_for_npx_only_resolution(monkeypatch): + """No PATH binary and no runnable node_modules copy, but the browser_tool + cascade resolves the npx fallback: browser capability is available.""" + _block_legacy_agent_browser_checks(monkeypatch) + import tools.browser_tool as browser_tool + + calls = [] + + def fake_find_agent_browser(*, validate=True): + calls.append({"validate": validate}) + return "npx agent-browser" + + monkeypatch.setattr(browser_tool, "_find_agent_browser", fake_find_agent_browser) + monkeypatch.setattr( + browser_tool, "_requires_real_termux_browser_install", lambda cmd: False + ) + + assert ns._has_agent_browser() is True + # A readiness probe must resolve without spawning the daemon. + assert calls and all(call["validate"] is False for call in calls) + + +def test_has_agent_browser_false_for_termux_local_bare_npx(monkeypatch): + """On Termux in local mode the bare npx fallback is not a usable install.""" + _block_legacy_agent_browser_checks(monkeypatch) + import tools.browser_tool as browser_tool + + monkeypatch.setattr( + browser_tool, + "_find_agent_browser", + lambda *, validate=True: "npx agent-browser", + ) + monkeypatch.setattr( + browser_tool, + "_requires_real_termux_browser_install", + lambda cmd: cmd.strip() == "npx agent-browser", + ) + + assert ns._has_agent_browser() is False + + +def test_has_agent_browser_false_when_nothing_resolvable(monkeypatch): + _block_legacy_agent_browser_checks(monkeypatch) + import tools.browser_tool as browser_tool + + def raise_not_found(*, validate=True): + raise FileNotFoundError("agent-browser CLI not found") + + monkeypatch.setattr(browser_tool, "_find_agent_browser", raise_not_found) + + assert ns._has_agent_browser() is False + + +def test_has_agent_browser_import_failure_falls_back_to_path_check(monkeypatch): + """If tools.browser_tool cannot be imported, the old PATH + node_modules + check must still answer (prior behaviour), not crash.""" + monkeypatch.setitem(sys.modules, "tools.browser_tool", None) + real_which = shutil.which + monkeypatch.setattr( + shutil, + "which", + lambda cmd, *args, **kwargs: ( + "/fake/bin/agent-browser" + if cmd == "agent-browser" + else real_which(cmd, *args, **kwargs) + ), ) monkeypatch.setattr( "hermes_constants.agent_browser_runnable", - lambda p: bool(p) and str(p) == str(managed_bin), + lambda path: path == "/fake/bin/agent-browser", ) assert ns._has_agent_browser() is True +def test_has_agent_browser_import_failure_and_no_binary_is_false(monkeypatch): + monkeypatch.setitem(sys.modules, "tools.browser_tool", None) + _block_legacy_agent_browser_checks(monkeypatch) + + assert ns._has_agent_browser() is False