fix(cli): teach doctor --live and dep_ensure the npx agent-browser cascade
Both probes only checked PATH and node_modules, so they disagreed with `hermes doctor` on npx-only installs (#43564): doctor --live reported the browser probe unavailable, and ensure_dependency("browser") could shell out to install.sh on installs doctor already reports healthy.
This commit is contained in:
parent
fa85964ac1
commit
b9cbcc6bf5
|
|
@ -35,6 +35,7 @@ _DEP_CHECKS = {
|
|||
agent_browser_runnable(shutil.which("agent-browser"))
|
||||
or _has_system_browser()
|
||||
or _has_hermes_agent_browser()
|
||||
or _has_npx_agent_browser()
|
||||
),
|
||||
"ripgrep": lambda: shutil.which("rg") is not None,
|
||||
"ffmpeg": lambda: shutil.which("ffmpeg") is not None,
|
||||
|
|
@ -59,6 +60,25 @@ def _has_system_browser() -> bool:
|
|||
return False
|
||||
|
||||
|
||||
def _has_npx_agent_browser() -> bool:
|
||||
"""agent-browser resolves lazily via npx on the default install (#43564),
|
||||
invisible to the PATH/managed-dir probes above. Mirror
|
||||
tools.browser_tool.check_browser_requirements's Termux carve-out so this
|
||||
check can't diverge from what browser tools actually find."""
|
||||
try:
|
||||
from tools.browser_tool import (
|
||||
_find_agent_browser,
|
||||
_is_npx_agent_browser_sentinel,
|
||||
_requires_real_termux_browser_install,
|
||||
)
|
||||
browser_cmd = _find_agent_browser(validate=False)
|
||||
except Exception:
|
||||
return False
|
||||
if not _is_npx_agent_browser_sentinel(browser_cmd):
|
||||
return False
|
||||
return not _requires_real_termux_browser_install(browser_cmd)
|
||||
|
||||
|
||||
def _has_hermes_agent_browser() -> bool:
|
||||
from hermes_constants import get_hermes_home
|
||||
home = get_hermes_home()
|
||||
|
|
|
|||
|
|
@ -91,7 +91,14 @@ def _browser_available() -> bool:
|
|||
return True
|
||||
except Exception:
|
||||
pass
|
||||
return False
|
||||
# agent-browser resolves lazily via npx on the default install (#43564),
|
||||
# invisible to the PATH/node_modules probes above. Mirror the rung
|
||||
# hermes_cli.doctor uses so this probe can't diverge from it.
|
||||
try:
|
||||
from tools.browser_tool import _find_agent_browser, _is_npx_agent_browser_sentinel
|
||||
return _is_npx_agent_browser_sentinel(_find_agent_browser(validate=False))
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
|
||||
def _launch_browser_probe(timeout: float) -> tuple:
|
||||
|
|
|
|||
|
|
@ -26,6 +26,38 @@ def test_find_install_script_from_checkout(tmp_path):
|
|||
|
||||
|
||||
|
||||
def test_has_npx_agent_browser_true_when_npx_resolves():
|
||||
"""agent-browser resolves lazily via npx on the default install (#43564)
|
||||
— _has_npx_agent_browser mirrors the runtime cascade so the "browser" dep
|
||||
check doesn't wrongly report it missing."""
|
||||
from hermes_cli.dep_ensure import _has_npx_agent_browser
|
||||
import tools.browser_tool as bt
|
||||
|
||||
with patch.object(bt, "_find_agent_browser", return_value="npx agent-browser"), \
|
||||
patch.object(bt, "_requires_real_termux_browser_install", return_value=False):
|
||||
assert _has_npx_agent_browser() is True
|
||||
|
||||
|
||||
def test_has_npx_agent_browser_false_on_termux_local_bare_npx():
|
||||
from hermes_cli.dep_ensure import _has_npx_agent_browser
|
||||
import tools.browser_tool as bt
|
||||
|
||||
with patch.object(bt, "_find_agent_browser", return_value="npx agent-browser"), \
|
||||
patch.object(bt, "_requires_real_termux_browser_install", return_value=True):
|
||||
assert _has_npx_agent_browser() is False
|
||||
|
||||
|
||||
def test_has_npx_agent_browser_false_when_nothing_resolves():
|
||||
from hermes_cli.dep_ensure import _has_npx_agent_browser
|
||||
import tools.browser_tool as bt
|
||||
|
||||
def _raise(**_kw):
|
||||
raise FileNotFoundError("agent-browser CLI not found")
|
||||
|
||||
with patch.object(bt, "_find_agent_browser", _raise):
|
||||
assert _has_npx_agent_browser() is False
|
||||
|
||||
|
||||
@pytest.mark.windows_only
|
||||
def test_ensure_dependency_uses_powershell_on_windows(tmp_path):
|
||||
"""``windows_only``: the assertion is that we shell out to a real
|
||||
|
|
|
|||
|
|
@ -17,6 +17,10 @@ from hermes_cli.doctor_live import (
|
|||
run_live_checks,
|
||||
)
|
||||
|
||||
# Captured before the autouse fixture below stubs doctor_live._browser_available
|
||||
# to a constant, so TestBrowserAvailableNpxRung can exercise the real function.
|
||||
_real_browser_available = doctor_live._browser_available
|
||||
|
||||
|
||||
def _args(live: bool = True) -> argparse.Namespace:
|
||||
return argparse.Namespace(live=live)
|
||||
|
|
@ -186,6 +190,36 @@ class TestConfiguredOnlySelection:
|
|||
assert results["Browser"].status == "pass"
|
||||
|
||||
|
||||
class TestBrowserAvailableNpxRung:
|
||||
"""agent-browser resolves lazily via npx on the default install (#43564),
|
||||
invisible to the bare PATH/node_modules probes _browser_available starts
|
||||
with. It must fall through to the same cascade `hermes doctor` uses."""
|
||||
|
||||
def _block_path_and_node_modules_checks(self, monkeypatch, tmp_path):
|
||||
monkeypatch.setattr("shutil.which", lambda *a, **k: None)
|
||||
monkeypatch.setattr("hermes_cli.doctor.HERMES_HOME", tmp_path / "home")
|
||||
monkeypatch.setattr("hermes_cli.doctor.PROJECT_ROOT", tmp_path / "root")
|
||||
|
||||
def test_true_when_npx_resolves_agent_browser(self, monkeypatch, tmp_path):
|
||||
self._block_path_and_node_modules_checks(monkeypatch, tmp_path)
|
||||
import tools.browser_tool as bt
|
||||
|
||||
monkeypatch.setattr(bt, "_find_agent_browser", lambda **_kw: "npx agent-browser")
|
||||
|
||||
assert _real_browser_available() is True
|
||||
|
||||
def test_false_when_nothing_resolves(self, monkeypatch, tmp_path):
|
||||
self._block_path_and_node_modules_checks(monkeypatch, tmp_path)
|
||||
import tools.browser_tool as bt
|
||||
|
||||
def _raise(**_kw):
|
||||
raise FileNotFoundError("agent-browser CLI not found")
|
||||
|
||||
monkeypatch.setattr(bt, "_find_agent_browser", _raise)
|
||||
|
||||
assert _real_browser_available() is False
|
||||
|
||||
|
||||
class TestFailureIsolation:
|
||||
def test_one_probe_raising_does_not_stop_others(self, monkeypatch):
|
||||
monkeypatch.setenv("FIRECRAWL_API_KEY", "fc-test")
|
||||
|
|
|
|||
Loading…
Reference in New Issue