fix(cli): teach _has_agent_browser the npx resolution cascade
The truthful per-provider readiness work (#67201) gates the desktop Capabilities panel on _has_agent_browser, which only probes PATH and node_modules/.bin. Now that agent-browser is no longer a root package.json dependency (#43564), npx-only installs report needs_setup in the panel while the browser tools themselves resolve fine at runtime — and existing installs flip to needs_setup as soon as a hermes update prunes node_modules. Mirror the local-CLI tail of check_browser_requirements: resolve via _find_agent_browser(validate=False), honor the Termux bare-npx carve-out, and keep the old probe as the import-failure fallback. Existing shutil.which test stubs gain the real signature so the cascade's path= keyword calls don't break them.
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user