fix(cli): drop agent-browser/@streamdown-math from root npm deps

`hermes update` was pruning root-level Node dependencies (agent-browser)
because npm ci always wipes and reifies node_modules according to its
active filter -- no root-first/workspace-first ordering or flag
combination (--workspaces=false, --include-workspace-root, etc.) can
reliably keep a root-only package.json dependency from being pruned by
a subsequent workspace-scoped npm ci. Confirmed empirically and via
npm/cli source (isArboristCmd hardcodes includeWorkspaceRoot=false for
ci/install), so no amount of install-order juggling fixes this for good.

Instead of chasing install order, remove the root-only dependencies
that made the npm step fragile in the first place:

- agent-browser is no longer a root package.json dependency. It
  resolves lazily via `npx agent-browser` (tools/browser_tool.py
  already had this as a fallback; it's now the primary path).
  warm_agent_browser_npx_cache() is called fire-and-forget from both
  `hermes update` and `hermes doctor --fix` to keep npx's cache warm,
  preserving the "available before any session starts" property
  agent-browser had as an eager dependency without re-entangling it
  with the npm workspace graph.
- @streamdown/math moves to apps/desktop/package.json, where it's
  actually imported (markdown-text.tsx, katex-memo.ts) -- it was
  never used anywhere else and was subject to the same pruning risk.
- _update_node_dependencies() collapses to a single
  `npm ci --workspace ui-tui --workspace web` call now that root has
  no dependencies of its own to protect, and keeps its original spot
  ahead of `_build_web_ui()` at both call sites in update_cmd.py --
  with no root-only dependencies left to protect, there's no reason
  for the Node refresh and the web build to run in any particular
  order relative to each other.
- hermes_cli/tools_config.py's post-setup Chromium-install path and
  hermes_cli/doctor.py's agent-browser check both now resolve through
  the same PATH -> Homebrew/Hermes-managed-node -> npx cascade
  (_find_agent_browser / _resolve_npx_bin) instead of hand-rolling
  their own node_modules/.bin lookups, so they can't diverge from what
  browser tools actually invoke at runtime.
- tests-js/package-json-lazy-deps.test.ts gets a lockfile-level check
  mirroring the existing camofox one, so a future regression that
  reintroduces agent-browser into package-lock.json fails this test
  directly instead of relying on manual review to catch it.

Fixes #43564.
This commit is contained in:
Zak B. Elep
2026-07-15 00:58:37 +08:00
committed by Teknium
parent 136a911065
commit 5f5f8d5b62
12 changed files with 874 additions and 220 deletions
+1
View File
@@ -90,6 +90,7 @@
"@nanostores/react": "1.1.0",
"@nous-research/ui": "0.18.2",
"@streamdown/code": "1.1.1",
"@streamdown/math": "1.0.2",
"@tabler/icons-react": "3.44.0",
"@tailwindcss/typography": "0.5.20",
"@tailwindcss/vite": "4.3.3",
+28 -34
View File
@@ -2107,49 +2107,43 @@ def run_doctor(args):
# Node.js + agent-browser (for browser automation tools)
if _safe_which("node"):
check_ok("Node.js")
# Check if agent-browser is installed
agent_browser_path = PROJECT_ROOT / "node_modules" / "agent-browser"
# agent-browser is no longer a root package.json dependency (#43564)
# — it resolves lazily via npx (or a global/Hermes-managed install)
# at first use. Mirror tools.browser_tool._find_agent_browser's own
# resolution cascade here so doctor can't diverge from what browser
# tools will actually find; validate=False keeps this a cheap
# existence check with no subprocess spawn or install side effects.
agent_browser_ok = False
_which_ab = shutil.which("agent-browser")
# `hermes acp --setup-browser` installs agent-browser into the
# Hermes-managed node prefix, which isn't necessarily on PATH. Mirror
# dep_ensure._has_hermes_agent_browser() so doctor and dep_ensure agree
# on what "installed" means; otherwise doctor false-negatives (#53192).
# Resolve with PATHEXT-aware ``shutil.which`` (not a bare is_file())
# so Windows picks the executable ``.cmd`` shim — the same class of
# miss fixed for _has_agent_browser() in #73932.
def _which_in(directory) -> str | None:
try:
if not directory.is_dir():
return None
return shutil.which("agent-browser", path=str(directory))
except Exception:
return None
try:
from tools.browser_tool import _find_agent_browser
_resolved_ab = _find_agent_browser(validate=False)
except Exception:
_resolved_ab = None
_managed_ab = (
_which_in(HERMES_HOME / "node" / "bin")
or _which_in(HERMES_HOME / "node")
)
_legacy_ab = _which_in(HERMES_HOME / "node_modules" / ".bin")
if agent_browser_path.exists():
check_ok("agent-browser (Node.js)", "(browser automation)")
if _resolved_ab == "npx agent-browser":
check_ok("agent-browser", "(resolves via npx on first use)")
agent_browser_ok = True
elif _which_ab and agent_browser_runnable(_which_ab):
if should_fix:
# Doctor can't tell from here whether npx's cache already
# has agent-browser warm — just fire the same warm-up
# `hermes update` does, so a session's first browser call
# doesn't pay the registry fetch either way.
from tools.browser_tool import warm_agent_browser_npx_cache
if warm_agent_browser_npx_cache():
check_ok(" Warmed npx cache for agent-browser")
fixed_count += 1
else:
check_info(" Could not warm npx cache (offline or npx unavailable)")
elif _resolved_ab and agent_browser_runnable(_resolved_ab):
check_ok("agent-browser", "(browser automation)")
agent_browser_ok = True
elif _managed_ab and agent_browser_runnable(_managed_ab):
check_ok("agent-browser", "(browser automation)")
agent_browser_ok = True
elif _legacy_ab and agent_browser_runnable(_legacy_ab):
check_ok("agent-browser", "(browser automation)")
agent_browser_ok = True
elif _which_ab:
elif _resolved_ab:
# Found on PATH but won't run — almost always a dangling global
# symlink left behind by agent-browser's npm postinstall after a
# `hermes update` wiped node_modules (issue #48521).
check_warn(
"agent-browser found but not runnable",
f"(broken symlink at {_which_ab}? run: npm install)",
f"(broken symlink at {_resolved_ab}? run: npx agent-browser --version)",
)
elif _is_termux():
check_info("agent-browser is not installed (expected in the tested Termux path)")
@@ -2158,7 +2152,7 @@ def run_doctor(args):
for step in _termux_browser_setup_steps(node_installed=True):
check_info(step)
else:
check_warn("agent-browser not installed", "(run: npm install)")
check_warn("agent-browser not installed", "(requires npm/npx on PATH)")
# Chromium presence — the browser tools silently fail to register when
# agent-browser is found but no Playwright-managed Chromium is on disk
+48 -67
View File
@@ -1644,69 +1644,50 @@ def _run_cua_driver_installer(
def _run_post_setup(post_setup_key: str):
"""Run post-setup hooks for tools that need extra installation steps."""
import shutil
from hermes_constants import find_node_executable
if post_setup_key in {"agent_browser", "browserbase"}:
node_modules = PROJECT_ROOT / "node_modules" / "agent-browser"
# Managed Node first — $HERMES_HOME/node is not on PATH, so a bare
# which() reports "no npm" on installs whose only Node is the one
# Hermes installed for exactly this toolchain.
npm_bin = find_node_executable("npm")
npx_bin = find_node_executable("npx")
# Step 1: install the agent-browser npm package into node_modules/
if not node_modules.exists() and npm_bin:
_print_info(" Installing Node.js dependencies for browser tools...")
import subprocess
# Use the resolved npm_bin absolute path so subprocess.Popen can
# execute npm.cmd on Windows (CreateProcessW otherwise rejects
# batch shims). On POSIX npm_bin is the plain path — same
# behaviour as before.
result = subprocess.run(
# --workspaces=false restricts the install to the repo root
# only, avoiding the apps/* glob which would pull in
# apps/desktop (Electron + node-pty) unnecessarily. See #38772.
[npm_bin, "install", "--silent", "--workspaces=false"],
capture_output=True, text=True, encoding="utf-8", errors="replace", cwd=str(PROJECT_ROOT),
creationflags=_post_setup_no_window_flags(),
)
if result.returncode == 0:
_print_success(" Node.js dependencies installed")
else:
from hermes_constants import display_hermes_home
_print_warning(f" npm install failed - run manually: cd {display_hermes_home()}/hermes-agent && npm install --workspaces=false")
if result.stderr:
_print_info(f" {result.stderr.strip()[:200]}")
elif node_modules.exists():
# Distinct message for the re-run case so the GUI action log tells
# the truth ("nothing to do") instead of implying a fresh install.
_print_success(" agent-browser already installed, nothing to do")
else:
_print_warning(" Node.js not found - browser tools require: npm install (in hermes-agent directory)")
return
# Step 2: only the local browser provider actually needs Chromium on
# disk. Cloud providers (Browserbase, Browser Use, Firecrawl) host
# their own Chromium and don't need the local install.
if post_setup_key != "agent_browser":
return
# Step 3: ensure the Chromium / headless-shell build agent-browser
# drives is actually installed. Without it the CLI hangs on first
# use until the command timeout fires. Skip inside Docker — the
# image bakes Chromium in at build time, and runtime users usually
# can't write to PLAYWRIGHT_BROWSERS_PATH anyway.
# agent-browser is no longer a root package.json dependency (#43564)
# — it resolves lazily via npx (or a global/Hermes-managed install)
# instead of a local `npm install`, so there's no node_modules/
# population step here anymore.
try:
# Import lazily so the tools_config UI doesn't pull in the full
# browser_tool module at import time.
from tools.browser_tool import (
_chromium_installed,
_running_in_docker,
_find_agent_browser,
_resolve_npx_bin,
)
except Exception as exc: # pragma: no cover — defensive
_print_warning(f" Could not check Chromium status: {exc}")
return
# Reuse the same resolution cascade browser tools use at runtime
# (PATH -> Homebrew/Hermes-managed node -> npx) rather than a bare
# shutil.which — Hermes-managed-Node-only setups resolve agent-browser
# / npx only through the extended fallback path, which a bare
# shutil.which("npx") lookup misses.
try:
browser_cmd = _find_agent_browser(validate=False)
except FileNotFoundError:
_print_warning(
" npx not found - browser tools require Node.js: https://nodejs.org"
)
return
# Step 1: only the local browser provider actually needs Chromium on
# disk. Cloud providers (Browserbase, Browser Use, Firecrawl) host
# their own Chromium and don't need the local install.
if post_setup_key != "agent_browser":
return
# Step 2: ensure the Chromium / headless-shell build agent-browser
# drives is actually installed. Without it the CLI hangs on first
# use until the command timeout fires. Skip inside Docker — the
# image bakes Chromium in at build time, and runtime users usually
# can't write to PLAYWRIGHT_BROWSERS_PATH anyway.
if _chromium_installed():
_print_success(" Chromium browser already installed, nothing to do")
return
@@ -1723,27 +1704,27 @@ def _run_post_setup(post_setup_key: str):
)
return
if not npx_bin:
_print_warning(
" npx not found - install Chromium manually: npx agent-browser install --with-deps"
)
return
# browser_cmd was already resolved above (same PATH -> Homebrew ->
# Hermes-managed-node -> npx cascade _find_agent_browser uses at
# runtime), so this can't diverge from what actually gets invoked.
if browser_cmd == "npx agent-browser":
# Re-resolve via the same PATH + extended-PATH cascade
# _find_agent_browser used, rather than a bare shutil.which("npx")
# — Hermes-managed-Node-only setups resolve npx only through the
# extended fallback path, and a bare lookup here would silently
# diverge and hand subprocess.run a None argument.
npx_bin = _resolve_npx_bin()
if not npx_bin:
_print_warning(
" npx not found - install Chromium manually: npx agent-browser install --with-deps"
)
return
install_cmd = [npx_bin, "-y", "agent-browser", "install", "--with-deps"]
else:
install_cmd = [browser_cmd, "install", "--with-deps"]
_print_info(" Installing Chromium (~170MB one-time download)...")
import subprocess
# Prefer the bundled agent-browser install subcommand so the
# version of Chromium matches the CLI. Fall back to npx shim on
# setups where the local bin stub isn't present.
local_ab = PROJECT_ROOT / "node_modules" / ".bin" / "agent-browser"
if sys.platform == "win32":
local_ab_win = local_ab.with_suffix(".cmd")
if local_ab_win.exists():
local_ab = local_ab_win
install_cmd = (
[str(local_ab), "install", "--with-deps"]
if local_ab.exists()
else [npx_bin, "-y", "agent-browser", "install", "--with-deps"]
)
try:
result = subprocess.run(
install_cmd,
+47 -46
View File
@@ -2023,12 +2023,13 @@ def _npm_manifest_paths() -> tuple[Path, ...]:
The workspace list is pulled from the root package.json's `workspaces`
globs (npm's own source of truth) rather than hardcoded, so adding a
workspace can never silently escape the skip key. The root install
(step 1, --workspaces=false) still hoists shared deps for EVERY
workspace — desktop included — so all of them belong in the key, not
just the ones step 2 installs. Falls back to hashing just root
manifests if package.json is unreadable (never skips more than main
would have installed).
workspace can never silently escape the skip key. Every workspace
manifest belongs in the key — desktop included, even though the
install only names ui-tui and web — because the single lockfile spans
the whole workspace graph, so any manifest edit can put the lockfile
out of sync and change what the install must do. Falls back to hashing
just root manifests if package.json is unreadable (never skips more
than main would have installed).
"""
root_pkg = _m().PROJECT_ROOT / "package.json"
paths = [_m().PROJECT_ROOT / "package-lock.json", root_pkg]
@@ -2102,7 +2103,7 @@ def _record_npm_lockfile_hash(hermes_root: Path) -> None:
logger.debug("Could not write npm lockfile hash cache")
def _update_node_dependencies() -> list[str]:
"""Refresh Node deps in the repo root and update workspaces.
"""Refresh Node deps for the ui-tui and web workspaces.
Returns the list of labels whose npm install failed (empty on success),
so the caller can treat a Node refresh failure as a partial update rather
@@ -2124,7 +2125,7 @@ def _update_node_dependencies() -> list[str]:
print(" ⚠ Skipped: only a Windows npm is reachable from this WSL shell.")
print(" Install Node.js inside the WSL distro (nvm, or your distro's")
print(" package manager), then re-run `hermes update`.")
failed = ["repo root"]
failed = []
if any(
(_m().PROJECT_ROOT / workspace / "package.json").exists()
for workspace in ("ui-tui", "web")
@@ -2143,12 +2144,15 @@ def _update_node_dependencies() -> list[str]:
logger.info("npm lockfile unchanged, skipping npm install")
return []
# With a single workspace lockfile the root install would cover ALL
# workspaces — but apps/desktop pulls in Electron as a devDependency,
# and its postinstall downloads a ~200MB binary. Most users don't
# need desktop during `hermes update`, so we install root-only first
# then add just the workspaces the CLI/TUI/web build actually requires.
# Desktop deps are installed on demand by the desktop launcher
# Root package.json has no dependencies of its own (agent-browser and
# @streamdown/math were moved out — see #43564): agent-browser resolves
# at runtime via `npx agent-browser` (tools/browser_tool.py), and
# @streamdown/math is a desktop-only import now declared in
# apps/desktop/package.json. That means a plain workspace-scoped install
# can never prune anything root-only, so we only need to name the
# workspaces the CLI/TUI/web build actually requires. apps/desktop pulls
# in Electron as a devDependency with a ~200MB postinstall download, so
# it's deliberately never named here — desktop deps install on demand
# (see _desktop_build_needed).
print("→ Updating Node.js dependencies...")
@@ -2159,53 +2163,50 @@ def _update_node_dependencies() -> list[str]:
print(" deps). Fix npm and re-run `hermes update`.")
return list(labels)
extra_args = ["--no-fund", "--no-audit", "--prefer-offline", "--progress=false"]
install_args = [
"--no-fund", "--no-audit", "--prefer-offline", "--progress=false",
"--workspace", "ui-tui", "--workspace", "web",
]
from hermes_constants import with_hermes_node_path
nixos_env = with_hermes_node_path(_m()._nixos_build_env())
# Step 1: root install (no workspace recursion).
# NOTE: capture_output=False here is deliberate (#18840) — optional
# postinstall scripts (e.g. @askjo/camofox-browser's browser-binary fetch)
# print download progress, and capturing it makes a long download look
# hung. The chatty npm-deprecation noise during `hermes update` comes from
# the *desktop* build, not this step; that one is captured to update.log.
root_args = [*extra_args, "--workspaces=false"]
root_result = _m()._run_npm_install_deterministic(
# postinstall scripts print download progress, and capturing it makes a
# long download look hung. The chatty npm-deprecation noise during
# `hermes update` comes from the *desktop* build, not this step; that
# one is captured to update.log.
result = _m()._run_npm_install_deterministic(
npm,
_m().PROJECT_ROOT,
extra_args=tuple(root_args),
extra_args=tuple(install_args),
capture_output=False,
env=nixos_env,
)
if root_result.returncode != 0:
print(" ⚠ npm install failed in repo root")
stderr = (root_result.stderr or "").strip() if root_result.stderr else ""
if result.returncode == 0:
_record_npm_lockfile_hash(shared_hermes_root)
print(" ✓ ui-tui, web workspaces installed (desktop skipped)")
failures: list[str] = []
else:
print(" ⚠ npm install failed")
stderr = (result.stderr or "").strip() if result.stderr else ""
if stderr:
print(f" {stderr.splitlines()[-1]}")
return _partial_update_failure("repo root")
failures = _partial_update_failure("ui-tui, web workspaces")
# Step 2: install only the workspaces update needs (ui-tui, web).
# --workspace selects specific workspaces; the rest (desktop) are skipped.
ws_args = [*extra_args, "--workspace", "ui-tui", "--workspace", "web"]
ws_result = _m()._run_npm_install_deterministic(
npm,
_m().PROJECT_ROOT,
extra_args=tuple(ws_args),
capture_output=False,
env=nixos_env,
)
if ws_result.returncode == 0:
_record_npm_lockfile_hash(shared_hermes_root)
print(" ✓ repo root + ui-tui, web workspaces (desktop skipped)")
return []
# Fire-and-forget: warm npx's cache for agent-browser so the first
# browser-tool call in a session doesn't pay a registry fetch that used
# to happen here for free back when agent-browser was an eager root
# dependency (see #43564). Independent of the workspace install result
# above — never blocks or fails `hermes update`.
try:
from tools.browser_tool import warm_agent_browser_npx_cache
warm_agent_browser_npx_cache()
except Exception:
pass
print(" ⚠ npm workspace install failed")
stderr = (ws_result.stderr or "").strip() if ws_result.stderr else ""
if stderr:
print(f" {stderr.splitlines()[-1]}")
return _partial_update_failure("ui-tui, web workspaces")
return failures
def _log_only_write(text: str) -> None:
"""Write ``text`` to ``~/.hermes/logs/update.log`` only, never the terminal.
+1 -14
View File
@@ -16,10 +16,6 @@
"web",
"tests-js"
],
"dependencies": {
"@streamdown/math": "1.0.2",
"agent-browser": "0.26.0"
},
"devDependencies": {
"@eslint/js": "9.39.5",
"eslint-plugin-perfectionist": "5.10.0",
@@ -90,6 +86,7 @@
"@nanostores/react": "1.1.0",
"@nous-research/ui": "0.18.2",
"@streamdown/code": "1.1.1",
"@streamdown/math": "1.0.2",
"@tabler/icons-react": "3.44.0",
"@tailwindcss/typography": "0.5.20",
"@tailwindcss/vite": "4.3.3",
@@ -7053,16 +7050,6 @@
"node": ">= 14"
}
},
"node_modules/agent-browser": {
"version": "0.26.0",
"resolved": "https://registry.npmjs.org/agent-browser/-/agent-browser-0.26.0.tgz",
"integrity": "sha512-pdqSfjwbFSp+qnwlb2g23e9wXveIOfMi19xpPA9xZUbzEAUp6W4YBZj6Ybj8z4M7WkcbGDDYc+oDIHDt9R3EDQ==",
"hasInstallScript": true,
"license": "Apache-2.0",
"bin": {
"agent-browser": "bin/agent-browser.js"
}
},
"node_modules/aggregate-error": {
"version": "3.1.0",
"resolved": "https://registry.npmjs.org/aggregate-error/-/aggregate-error-3.1.0.tgz",
+1 -6
View File
@@ -11,7 +11,7 @@
"tests-js"
],
"scripts": {
"postinstall": "echo '\u2705 Browser tools ready. Run: python run_agent.py --help'",
"postinstall": "echo '\u2705 Node dependencies installed. Run: python run_agent.py --help'",
"install:root": "npm install --workspaces=false",
"install:web": "npm install --workspace web",
"install:tui": "npm install --workspace ui-tui",
@@ -34,10 +34,6 @@
"url": "https://github.com/NousResearch/Hermes-Agent/issues"
},
"homepage": "https://github.com/NousResearch/Hermes-Agent#readme",
"dependencies": {
"@streamdown/math": "1.0.2",
"agent-browser": "0.26.0"
},
"devDependencies": {
"@eslint/js": "9.39.5",
"typescript-eslint": "8.64.0",
@@ -71,7 +67,6 @@
"esbuild@0.28.1": true,
"node-pty@1.1.0": true,
"electron-winstaller@5.4.0": true,
"agent-browser@0.26.0": true,
"electron@40.10.2": true,
"fsevents@2.3.2": true,
"fsevents@2.3.3": true,
+74 -16
View File
@@ -4,14 +4,29 @@
* The root ``package.json`` is installed by ``hermes update`` on every user,
* including users who never opted into a given browser backend. Anything
* listed in ``dependencies`` therefore runs its npm postinstall script for
* everyone — including binary-fetching backends, on every update.
* everyone, and — per #43564 — is also part of the npm workspace install
* graph, where a workspace-scoped ``npm ci`` (``--workspace ui-tui
* --workspace web``) can silently prune it right back out on the next
* ``hermes update``.
*
* The contract:
*
* - ``agent-browser`` IS eager. It is the default Chromium-driving backend
* used whenever the agent makes a browser call without a cloud provider
* configured, so it must already be installed before any session starts.
* Its postinstall is also small.
* - ``agent-browser`` is NOT a root dependency. It used to be eager (see
* #27055, which reasoned its postinstall was small enough to keep eager
* unlike Camofox's) but #43564 found that keeping ANY dependency in root
* ``package.json`` — however small its postinstall — entangles it with
* the ui-tui/web workspace install and risks it being pruned. It now
* resolves at runtime via ``npx agent-browser`` (see
* ``tools/browser_tool.py::_find_agent_browser``), which sidesteps the
* workspace graph entirely. ``hermes update`` and ``hermes doctor --fix``
* both fire-and-forget ``warm_agent_browser_npx_cache()`` to keep npx's
* own cache warm, preserving the "available before any session starts"
* property #27055 cared about without re-entangling the dependency.
*
* - ``@streamdown/math`` is NOT a root dependency either. It's imported only
* by desktop's own TS code (``apps/desktop/src/...``), so it belongs in
* ``apps/desktop/package.json`` (alongside its sibling ``@streamdown/code``)
* — not root, where it was subject to the exact same pruning risk.
*
* - ``@askjo/camofox-browser`` is NOT eager. It is an explicit opt-in
* alternative browser backend, selected by the user via
@@ -23,10 +38,9 @@
* installed on demand by ``tools_config.py`` ``post_setup_key ==
* "camofox"`` when the user actually selects Camofox.
*
* If a future PR re-adds Camofox (or any other binary-postinstall package)
* to root ``dependencies``, this test fails — read the lazy-install
* guidance in the ``hermes-agent-dev`` skill before changing the
* expectations.
* If a future PR re-adds any of these to root ``dependencies``, this test
* fails — read the lazy-install guidance in the ``hermes-agent-dev`` skill
* before changing the expectations.
*/
import assert from 'node:assert/strict'
@@ -38,6 +52,7 @@ import { test } from 'vitest'
const REPO_ROOT = path.resolve(__dirname, '..')
const ROOT_PKG = path.join(REPO_ROOT, 'package.json')
const ROOT_LOCK = path.join(REPO_ROOT, 'package-lock.json')
const DESKTOP_PKG = path.join(REPO_ROOT, 'apps', 'desktop', 'package.json')
function rootPackageJson(): Record<string, unknown> {
return JSON.parse(fs.readFileSync(ROOT_PKG, 'utf-8'))
@@ -55,15 +70,41 @@ test('camofox is not in root dependencies (must stay opt-in)', () => {
)
})
test('agent-browser stays eager (default backend)', () => {
test('agent-browser is not in root dependencies (resolves via npx, #43564)', () => {
const deps = (rootPackageJson().dependencies ?? {}) as Record<string, string>
assert.ok(
'agent-browser' in deps,
'agent-browser is the default browser-tool backend used by every ' +
'session that doesn\'t have a cloud browser provider configured. ' +
'It must stay in root package.json dependencies so it is present ' +
'after `hermes setup` / `hermes update` without an explicit ' +
'post_setup step.'
!('agent-browser' in deps),
'agent-browser must not be a root package.json dependency — it ' +
'resolves lazily via `npx agent-browser` instead (see ' +
'tools/browser_tool.py::_find_agent_browser and ' +
'warm_agent_browser_npx_cache). Putting it back in root ' +
'dependencies re-entangles it with the ui-tui/web workspace ' +
'install graph and reintroduces #43564.'
)
})
test('@streamdown/math is not in root dependencies (desktop-only import)', () => {
const deps = (rootPackageJson().dependencies ?? {}) as Record<string, string>
assert.ok(
!('@streamdown/math' in deps),
'@streamdown/math is only imported by apps/desktop\'s own TS code ' +
'(markdown-text.tsx, katex-memo.ts) — it belongs in ' +
'apps/desktop/package.json alongside its sibling @streamdown/code, ' +
'not root, where it\'s subject to the same workspace-pruning risk ' +
'agent-browser had (#43564).'
)
})
test('@streamdown/math is in desktop dependencies', () => {
const deps = (JSON.parse(fs.readFileSync(DESKTOP_PKG, 'utf-8')).dependencies ??
{}) as Record<string, string>
assert.ok(
'@streamdown/math' in deps,
'@streamdown/math is imported by apps/desktop\'s own TS code ' +
'(markdown-text.tsx, katex-memo.ts) and must be declared in ' +
'apps/desktop/package.json now that it is no longer a root ' +
'dependency.'
)
})
@@ -87,3 +128,20 @@ test('root lockfile has no camofox entries', () => {
'@askjo/camofox-browser). Regenerate the lockfile.'
)
})
test('root lockfile has no agent-browser entry (#43564)', () => {
if (!fs.existsSync(ROOT_LOCK)) {
// Some CI matrix shards skip lockfile materialization.
return
}
const text = fs.readFileSync(ROOT_LOCK, 'utf-8')
assert.ok(
!text.includes('"node_modules/agent-browser"'),
'package-lock.json still has a node_modules/agent-browser entry. ' +
'It must resolve lazily via `npx agent-browser` instead — ' +
'regenerate the lockfile after removing the dep: ' +
'`rm package-lock.json && npm install --package-lock-only ' +
'--ignore-scripts --no-fund --no-audit`.'
)
})
+214 -5
View File
@@ -234,7 +234,6 @@ class TestCmdUpdateBranchFallback:
captured = capsys.readouterr()
assert "Already up to date!" in captured.out
def test_update_non_interactive_runs_safe_config_migrations(self, mock_args, capsys):
"""Dashboard/web updates apply non-interactive migrations before restart."""
with patch("shutil.which", return_value=None), patch(
@@ -744,13 +743,14 @@ class TestNodeRuntimeNpmResolution:
lambda *a, **k: subprocess.CompletedProcess([], 1, stdout="", stderr=""),
)
failed = hm._update_node_dependencies()
assert failed == ["repo root"]
with patch(
"tools.browser_tool.warm_agent_browser_npx_cache", return_value=True
):
failed = hm._update_node_dependencies()
assert failed == ["ui-tui, web workspaces"]
out = capsys.readouterr().out
assert "mixed state" in out
def test_wsl_update_skips_windows_npm_build_paths(self, mock_args, monkeypatch):
"""A Windows-only npm on WSL must not reach web or desktop builds."""
from hermes_cli import main as hm
@@ -790,3 +790,212 @@ class TestNodeRuntimeNpmResolution:
not call.args or not call.args[0] or call.args[0][0] != windows_npm
for call in mock_run.call_args_list
)
class TestUpdateNodeDependencies:
"""Unit tests for _update_node_dependencies — issue #43564.
Root package.json has no dependencies of its own: agent-browser
resolves at runtime via npx (tools/browser_tool.py), and @streamdown/math
moved to apps/desktop/package.json since it's a desktop-only import.
With nothing root-only left to protect, a single workspace-scoped
install (ui-tui, web) is safe — apps/desktop is simply never named, so
its ~200 MB Electron devDependency is never resolved. Skipping is
governed by _npm_lockfile_changed (content hash over the lockfile +
every workspace package.json), tested separately in
TestNpmLockfileChanged.
Uses a tmp_path root so tests never touch real node_modules.
"""
@pytest.fixture(autouse=True)
def _stub_npx_warmup(self):
"""The npx cache warm-up is covered by its own dedicated test below;
stub it out everywhere else so it doesn't add a spurious npm/npx
call to the workspace-install assertions in this class."""
with patch("tools.browser_tool.warm_agent_browser_npx_cache", return_value=True):
yield
def _npm_calls(self, mock_run):
return [
call.args[0]
for call in mock_run.call_args_list
if call.args and "npm" in str(call.args[0][0])
]
@patch("subprocess.run")
@patch("shutil.which", return_value="/usr/bin/npm")
def test_install_names_ui_tui_and_web_workspaces(self, _which, mock_run, tmp_path, monkeypatch):
"""Regression for #43564: install ui-tui + web directly. apps/desktop
must never appear, so its Electron postinstall is never triggered.
"""
from hermes_cli import main as hm
(tmp_path / "package.json").write_text("{}")
(tmp_path / "package-lock.json").write_text("{}")
monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path)
monkeypatch.setattr(hm, "_npm_lockfile_changed", lambda root: True)
mock_run.return_value = subprocess.CompletedProcess([], 0, stdout="", stderr="")
hm._update_node_dependencies()
calls = self._npm_calls(mock_run)
assert len(calls) == 1, f"expected exactly 1 npm call, got: {calls}"
joined = " ".join(str(a) for a in calls[0])
assert "--workspace ui-tui" in joined and "--workspace web" in joined, (
f"expected ui-tui + web workspace selectors; actual: {calls[0]}"
)
assert "desktop" not in joined, (
f"apps/desktop must not appear (avoids ~200 MB Electron download); actual: {calls[0]}"
)
assert "--workspaces=false" not in joined, (
f"no root-only deps remain to protect; --workspaces=false is unnecessary now; actual: {calls[0]}"
)
@patch("subprocess.run")
@patch("shutil.which", return_value="/usr/bin/npm")
def test_install_preserves_standard_flags(self, _which, mock_run, tmp_path, monkeypatch):
"""--no-fund, --no-audit, --progress=false must survive."""
from hermes_cli import main as hm
(tmp_path / "package.json").write_text("{}")
(tmp_path / "package-lock.json").write_text("{}")
monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path)
monkeypatch.setattr(hm, "_npm_lockfile_changed", lambda root: True)
mock_run.return_value = subprocess.CompletedProcess([], 0, stdout="", stderr="")
hm._update_node_dependencies()
calls = self._npm_calls(mock_run)
assert len(calls) == 1
joined = " ".join(str(a) for a in calls[0])
for flag in ("--no-fund", "--no-audit", "--progress=false"):
assert flag in joined, f"{flag} missing from npm call; actual: {calls[0]}"
@patch("subprocess.run")
@patch("shutil.which", return_value="/usr/bin/npm")
def test_skips_install_when_deps_up_to_date(self, _which, mock_run, tmp_path, monkeypatch):
"""When _npm_lockfile_changed reports no change, npm must not be called."""
from hermes_cli import main as hm
(tmp_path / "package.json").write_text("{}")
(tmp_path / "package-lock.json").write_text("{}")
monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path)
monkeypatch.setattr(hm, "_npm_lockfile_changed", lambda root: False)
hm._update_node_dependencies()
assert not self._npm_calls(mock_run), (
"npm must not run when _npm_lockfile_changed reports no change"
)
@patch("subprocess.run")
@patch("shutil.which", return_value="/usr/bin/npm")
def test_runs_install_when_lockfile_changed(self, _which, mock_run, tmp_path, monkeypatch):
"""When _npm_lockfile_changed reports a change, npm must run."""
from hermes_cli import main as hm
(tmp_path / "package.json").write_text("{}")
(tmp_path / "package-lock.json").write_text("{}")
monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path)
monkeypatch.setattr(hm, "_npm_lockfile_changed", lambda root: True)
mock_run.return_value = subprocess.CompletedProcess([], 0, stdout="", stderr="")
hm._update_node_dependencies()
calls = self._npm_calls(mock_run)
assert len(calls) == 1, f"expected npm to run when lockfile changed; got: {calls}"
@patch("subprocess.run")
@patch("shutil.which", return_value="/usr/bin/npm")
def test_records_lockfile_hash_only_on_success(self, _which, mock_run, tmp_path, monkeypatch):
"""A failed install must not record the lockfile hash (so the next
run retries instead of wrongly believing deps are up to date)."""
from hermes_cli import main as hm
(tmp_path / "package.json").write_text("{}")
(tmp_path / "package-lock.json").write_text("{}")
monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path)
monkeypatch.setattr(hm, "_npm_lockfile_changed", lambda root: True)
recorded = []
monkeypatch.setattr(hm, "_record_npm_lockfile_hash", lambda root: recorded.append(root))
mock_run.return_value = subprocess.CompletedProcess(
[], 1, stdout="", stderr="npm ERR!"
)
hm._update_node_dependencies()
assert not recorded, "lockfile hash must not be recorded when npm install fails"
@patch("subprocess.run")
@patch("shutil.which", return_value="/usr/bin/npm")
def test_warms_npx_agent_browser_cache_regardless_of_install_result(
self, _which, mock_run, tmp_path, monkeypatch
):
"""The npx warm-up must fire even when the workspace install fails —
it's independent of ui-tui/web dependency state (#43564)."""
from hermes_cli import main as hm
(tmp_path / "package.json").write_text("{}")
(tmp_path / "package-lock.json").write_text("{}")
monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path)
monkeypatch.setattr(hm, "_npm_lockfile_changed", lambda root: True)
mock_run.return_value = subprocess.CompletedProcess(
[], 1, stdout="", stderr="npm ERR!"
)
with patch(
"tools.browser_tool.warm_agent_browser_npx_cache", return_value=True
) as mock_warm:
hm._update_node_dependencies()
mock_warm.assert_called_once()
@patch("subprocess.run")
@patch("shutil.which", return_value=None)
def test_returns_silently_when_npm_not_found(self, _which, mock_run, tmp_path, monkeypatch):
"""No npm on PATH → return without calling subprocess."""
from hermes_cli import main as hm
(tmp_path / "package.json").write_text("{}")
monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path)
hm._update_node_dependencies()
mock_run.assert_not_called()
@patch("subprocess.run")
@patch("shutil.which", return_value="/usr/bin/npm")
def test_returns_silently_when_package_json_absent(self, _which, mock_run, tmp_path, monkeypatch):
"""No package.json → return without calling npm."""
from hermes_cli import main as hm
monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path)
hm._update_node_dependencies()
mock_run.assert_not_called()
@patch("subprocess.run")
@patch("shutil.which", return_value="/usr/bin/npm")
def test_install_runs_from_project_root(self, _which, mock_run, tmp_path, monkeypatch):
"""npm install must execute from PROJECT_ROOT, not a workspace subdir."""
from hermes_cli import main as hm
(tmp_path / "package.json").write_text("{}")
(tmp_path / "package-lock.json").write_text("{}")
monkeypatch.setattr(hm, "PROJECT_ROOT", tmp_path)
cwd_calls = []
def capture(cmd, **kwargs):
if "npm" in str(cmd[0]):
cwd_calls.append(kwargs.get("cwd"))
return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
mock_run.side_effect = capture
hm._update_node_dependencies()
assert cwd_calls, "expected at least one npm call"
for cwd in cwd_calls:
assert cwd == tmp_path, f"npm must run from PROJECT_ROOT; got cwd={cwd}"
+74 -27
View File
@@ -680,41 +680,24 @@ def test_run_doctor_termux_does_not_mark_browser_available_without_agent_browser
assert "npm install -g agent-browser && agent-browser install" in out
def _run_doctor_with_managed_agent_browser(monkeypatch, tmp_path, runnable):
"""Set up run_doctor with node present, agent-browser only in the
Hermes-managed node bin (~/.hermes/node/bin), not on PATH or in
PROJECT_ROOT/node_modules. Returns the captured stdout."""
def _doctor_env_for_agent_browser(monkeypatch, tmp_path):
"""Shared non-Termux fixture setup for the agent-browser npx-resolution
branch in run_doctor (hermes_cli/doctor.py ~1557-1605)."""
home = tmp_path / ".hermes"
(home / "node" / "bin").mkdir(parents=True, exist_ok=True)
home.mkdir(parents=True, exist_ok=True)
(home / "config.yaml").write_text("memory: {}\n", encoding="utf-8")
managed_ab = home / "node" / "bin" / "agent-browser"
managed_ab.write_text("#!/bin/sh\n", encoding="utf-8")
managed_ab.chmod(0o755)
project = tmp_path / "project"
project.mkdir(exist_ok=True) # no node_modules/agent-browser here
project.mkdir(exist_ok=True)
monkeypatch.delenv("TERMUX_VERSION", raising=False)
monkeypatch.delenv("PREFIX", raising=False)
monkeypatch.setenv("PREFIX", "/usr")
monkeypatch.setattr(doctor_mod, "HERMES_HOME", home)
monkeypatch.setattr(doctor_mod, "PROJECT_ROOT", project)
monkeypatch.setattr(doctor_mod, "_DHH", str(home))
# node on PATH, agent-browser is NOT on PATH (only in the managed bin).
# The managed-dir rung resolves via shutil.which(..., path=<dir>) so
# Windows picks the .cmd shim — mirror that shape here.
def _fake_which(cmd, path=None):
if path is not None:
if cmd == "agent-browser" and str(managed_ab.parent) == str(path):
return str(managed_ab)
return None
return "/usr/bin/node" if cmd in {"node", "npm"} else None
monkeypatch.setattr(doctor_mod.shutil, "which", _fake_which)
# agent_browser_runnable is imported into doctor's namespace
monkeypatch.setattr(
doctor_mod,
"agent_browser_runnable",
lambda path: runnable and str(path) == str(managed_ab),
doctor_mod.shutil,
"which",
lambda cmd: "/usr/bin/node" if cmd in {"node", "npm"} else None,
)
fake_model_tools = types.SimpleNamespace(
@@ -731,10 +714,74 @@ def _run_doctor_with_managed_agent_browser(monkeypatch, tmp_path, runnable):
except Exception:
pass
def test_run_doctor_reports_agent_browser_resolves_via_npx(monkeypatch, tmp_path):
"""When agent-browser has no local/global install, _find_agent_browser
falls through to 'npx agent-browser' — doctor must report that as OK
(#43564: agent-browser is no longer a root package.json dependency, so
this is the expected common case now, not a warning)."""
_doctor_env_for_agent_browser(monkeypatch, tmp_path)
import tools.browser_tool as bt
monkeypatch.setattr(bt, "_find_agent_browser", lambda **_kw: "npx agent-browser")
warm_calls = []
monkeypatch.setattr(
bt, "warm_agent_browser_npx_cache", lambda *a, **kw: warm_calls.append(1) or True
)
buf = io.StringIO()
with contextlib.redirect_stdout(buf):
doctor_mod.run_doctor(Namespace(fix=False))
return buf.getvalue()
out = buf.getvalue()
assert "agent-browser" in out
assert "resolves via npx on first use" in out
assert "agent-browser not installed" not in out
# --fix was not requested: the warm-up must not fire on a plain check.
assert not warm_calls
def test_run_doctor_fix_warms_npx_cache_when_agent_browser_resolves_via_npx(
monkeypatch, tmp_path
):
"""`hermes doctor --fix` must actually call warm_agent_browser_npx_cache()
when agent-browser resolves via npx, and report success."""
_doctor_env_for_agent_browser(monkeypatch, tmp_path)
import tools.browser_tool as bt
monkeypatch.setattr(bt, "_find_agent_browser", lambda **_kw: "npx agent-browser")
warm_calls = []
monkeypatch.setattr(
bt, "warm_agent_browser_npx_cache", lambda *a, **kw: warm_calls.append(1) or True
)
buf = io.StringIO()
with contextlib.redirect_stdout(buf):
doctor_mod.run_doctor(Namespace(fix=True))
out = buf.getvalue()
assert warm_calls, "warm_agent_browser_npx_cache() must be called under --fix"
assert "Warmed npx cache for agent-browser" in out
assert "Could not warm npx cache" not in out
def test_run_doctor_fix_reports_when_npx_warmup_fails(monkeypatch, tmp_path):
"""If warm_agent_browser_npx_cache() fails (offline, npx missing from
PATH at call time, etc.), doctor must say so instead of silently
claiming success — and must not count it as a fix."""
_doctor_env_for_agent_browser(monkeypatch, tmp_path)
import tools.browser_tool as bt
monkeypatch.setattr(bt, "_find_agent_browser", lambda **_kw: "npx agent-browser")
monkeypatch.setattr(bt, "warm_agent_browser_npx_cache", lambda *a, **kw: False)
buf = io.StringIO()
with contextlib.redirect_stdout(buf):
doctor_mod.run_doctor(Namespace(fix=True))
out = buf.getvalue()
assert "Could not warm npx cache (offline or npx unavailable)" in out
assert "Warmed npx cache for agent-browser" not in out
def test_run_doctor_kimi_cn_env_is_detected_and_probe_is_null_safe(monkeypatch, tmp_path):
+256
View File
@@ -1,6 +1,7 @@
"""Tests for hermes_cli.tools_config platform tool persistence."""
import logging
import subprocess
from types import SimpleNamespace
from unittest.mock import patch
@@ -345,6 +346,261 @@ def test_numeric_mcp_server_name_does_not_crash_sorted():
class TestAgentBrowserPostSetup:
"""_run_post_setup('agent_browser'/'browserbase') — #43564.
agent-browser is no longer a root package.json dependency (there's no
local `npm install` step anymore); it resolves at runtime via
tools.browser_tool._find_agent_browser (PATH -> Homebrew/Hermes-managed
node -> local .bin -> npx). This class exercises the Chromium-install
branch of _run_post_setup, which now delegates to that same resolution
cascade instead of hand-rolling its own node_modules/.bin/agent-browser
(and Windows .cmd-shim) lookup.
"""
def test_warns_when_neither_npx_nor_agent_browser_on_path(self):
with patch("shutil.which", return_value=None), patch(
"subprocess.run"
) as run, patch("hermes_cli.tools_config._print_warning") as warn:
_run_post_setup("agent_browser")
run.assert_not_called()
warn.assert_called_once()
assert "npx not found" in warn.call_args.args[0]
def test_browserbase_returns_before_any_chromium_check(self):
"""browserbase hosts its own Chromium; it must never reach the
agent-browser-only Chromium-install branch."""
with patch("shutil.which", return_value="/usr/bin/npx"), patch(
"subprocess.run"
) as run, patch(
"tools.browser_tool._chromium_installed"
) as chromium_check:
_run_post_setup("browserbase")
run.assert_not_called()
chromium_check.assert_not_called()
def test_chromium_already_installed_skips_subprocess(self):
with patch("shutil.which", return_value="/usr/bin/npx"), patch(
"subprocess.run"
) as run, patch(
"tools.browser_tool._chromium_installed", return_value=True
), patch(
"hermes_cli.tools_config._print_success"
) as success:
_run_post_setup("agent_browser")
run.assert_not_called()
success.assert_called_once()
assert "already installed" in success.call_args.args[0]
def test_docker_with_missing_chromium_warns_instead_of_installing(self):
with patch("shutil.which", return_value="/usr/bin/npx"), patch(
"subprocess.run"
) as run, patch(
"tools.browser_tool._chromium_installed", return_value=False
), patch(
"tools.browser_tool._running_in_docker", return_value=True
), patch(
"hermes_cli.tools_config._print_warning"
) as warn:
_run_post_setup("agent_browser")
run.assert_not_called()
assert any("Docker" in c.args[0] for c in warn.call_args_list)
def test_find_agent_browser_not_found_warns_before_any_chromium_check(self):
"""_find_agent_browser is resolved up front now (shared with the
browserbase early-return gate), so a FileNotFoundError here must
short-circuit before even checking Chromium/Docker status."""
with patch("shutil.which", return_value="/usr/bin/npx"), patch(
"subprocess.run"
) as run, patch(
"tools.browser_tool._chromium_installed"
) as chromium_check, patch(
"tools.browser_tool._running_in_docker"
) as docker_check, patch(
"tools.browser_tool._find_agent_browser",
side_effect=FileNotFoundError("agent-browser CLI not found"),
), patch(
"hermes_cli.tools_config._print_warning"
) as warn:
_run_post_setup("agent_browser")
run.assert_not_called()
chromium_check.assert_not_called()
docker_check.assert_not_called()
assert any("browser tools require Node.js" in c.args[0] for c in warn.call_args_list)
def test_installs_chromium_via_npx_when_no_local_binary_resolved(self):
"""When _find_agent_browser falls through to npx, the install command
must shell out to npx directly (not the unresolved 'npx agent-browser'
string as a single argv element)."""
with patch(
"shutil.which",
side_effect=lambda name: "/usr/bin/npx" if name == "npx" else None,
), patch("subprocess.run") as run, patch(
"tools.browser_tool._chromium_installed", return_value=False
), patch(
"tools.browser_tool._running_in_docker", return_value=False
), patch(
"tools.browser_tool._find_agent_browser", return_value="npx agent-browser"
), patch(
"hermes_cli.tools_config._print_success"
):
run.return_value = SimpleNamespace(returncode=0, stdout="", stderr="")
_run_post_setup("agent_browser")
run.assert_called_once()
assert run.call_args.args[0] == [
"/usr/bin/npx", "-y", "agent-browser", "install", "--with-deps",
]
def test_installs_chromium_via_npx_resolved_only_through_extended_path(self):
"""Hermes-managed-Node-only setups: npx resolves via
_find_agent_browser's extended-PATH fallback, not a bare PATH lookup.
The install command must use that same resolved npx, not silently
hand subprocess.run a None argument from a bare shutil.which('npx')
re-derivation (#43564 regression — Copilot review, task #9)."""
hermes_npx = "/home/user/.hermes/node/bin/npx"
with patch("shutil.which", return_value=None), patch(
"subprocess.run"
) as run, patch(
"tools.browser_tool._chromium_installed", return_value=False
), patch(
"tools.browser_tool._running_in_docker", return_value=False
), patch(
"tools.browser_tool._find_agent_browser", return_value="npx agent-browser"
), patch(
"tools.browser_tool._resolve_npx_bin", return_value=hermes_npx
), patch(
"hermes_cli.tools_config._print_success"
):
run.return_value = SimpleNamespace(returncode=0, stdout="", stderr="")
_run_post_setup("agent_browser")
run.assert_called_once()
assert run.call_args.args[0] == [
hermes_npx, "-y", "agent-browser", "install", "--with-deps",
]
def test_warns_instead_of_crashing_when_npx_unresolvable_after_all(self):
"""Defensive: if _resolve_npx_bin somehow returns None even though
_find_agent_browser resolved "npx agent-browser" (e.g. a race where
npx disappears between the two calls), warn and return instead of
building a command with a None argv element."""
with patch("shutil.which", return_value=None), patch(
"subprocess.run"
) as run, patch(
"tools.browser_tool._chromium_installed", return_value=False
), patch(
"tools.browser_tool._running_in_docker", return_value=False
), patch(
"tools.browser_tool._find_agent_browser", return_value="npx agent-browser"
), patch(
"tools.browser_tool._resolve_npx_bin", return_value=None
), patch(
"hermes_cli.tools_config._print_warning"
) as warn:
_run_post_setup("agent_browser") # must not raise
run.assert_not_called()
assert any("npx not found" in c.args[0] for c in warn.call_args_list)
def test_installs_chromium_via_resolved_local_binary_path(self):
"""When _find_agent_browser resolves a concrete executable (global
install, Homebrew, or the Windows .cmd shim it already knows how to
pick), that path must be invoked directly — not re-wrapped in npx."""
with patch("shutil.which", return_value="/usr/bin/npx"), patch(
"subprocess.run"
) as run, patch(
"tools.browser_tool._chromium_installed", return_value=False
), patch(
"tools.browser_tool._running_in_docker", return_value=False
), patch(
"tools.browser_tool._find_agent_browser",
return_value="/usr/local/bin/agent-browser",
), patch(
"hermes_cli.tools_config._print_success"
):
run.return_value = SimpleNamespace(returncode=0, stdout="", stderr="")
_run_post_setup("agent_browser")
run.assert_called_once()
assert run.call_args.args[0] == [
"/usr/local/bin/agent-browser", "install", "--with-deps",
]
def test_install_success_invalidates_chromium_cache(self):
import tools.browser_tool as _bt
with patch("shutil.which", return_value="/usr/bin/npx"), patch(
"subprocess.run",
return_value=SimpleNamespace(returncode=0, stdout="", stderr=""),
), patch(
"tools.browser_tool._chromium_installed", return_value=False
), patch(
"tools.browser_tool._running_in_docker", return_value=False
), patch(
"tools.browser_tool._find_agent_browser", return_value="npx agent-browser"
), patch(
"hermes_cli.tools_config._print_success"
):
_bt._cached_chromium_installed = True
_run_post_setup("agent_browser")
assert _bt._cached_chromium_installed is None, (
"a successful install must invalidate the cached chromium-missing "
"result so the next check_browser_requirements() call re-probes"
)
def test_install_failure_prints_stderr_tail_and_does_not_invalidate_cache(self):
import tools.browser_tool as _bt
with patch("shutil.which", return_value="/usr/bin/npx"), patch(
"subprocess.run",
return_value=SimpleNamespace(
returncode=1, stdout="", stderr="line1\nline2\nfatal: network error"
),
), patch(
"tools.browser_tool._chromium_installed", return_value=False
), patch(
"tools.browser_tool._running_in_docker", return_value=False
), patch(
"tools.browser_tool._find_agent_browser", return_value="npx agent-browser"
), patch(
"hermes_cli.tools_config._print_warning"
) as warn, patch(
"hermes_cli.tools_config._print_info"
) as info:
_bt._cached_chromium_installed = "sentinel"
_run_post_setup("agent_browser")
assert any("Chromium install failed" in c.args[0] for c in warn.call_args_list)
assert any("fatal: network error" in c.args[0] for c in info.call_args_list)
assert _bt._cached_chromium_installed == "sentinel", (
"a failed install must not invalidate the chromium cache"
)
def test_install_timeout_warns_without_raising(self):
with patch("shutil.which", return_value="/usr/bin/npx"), patch(
"subprocess.run",
side_effect=subprocess.TimeoutExpired(cmd=["npx"], timeout=600),
), patch(
"tools.browser_tool._chromium_installed", return_value=False
), patch(
"tools.browser_tool._running_in_docker", return_value=False
), patch(
"tools.browser_tool._find_agent_browser", return_value="npx agent-browser"
), patch(
"hermes_cli.tools_config._print_warning"
) as warn:
_run_post_setup("agent_browser") # must not raise
assert any("timed out" in c.args[0] for c in warn.call_args_list)
class TestImagegenBackendRegistry:
"""IMAGEGEN_BACKENDS tags drive the model picker flow in tools_config."""
+75
View File
@@ -0,0 +1,75 @@
"""Tests for tools.browser_tool.warm_agent_browser_npx_cache (#43564).
agent-browser was moved out of root package.json dependencies and now
resolves lazily via `npx agent-browser`. warm_agent_browser_npx_cache() is
the fire-and-forget helper `hermes update` / `hermes doctor --fix` call to
pre-fetch it so the first real browser-tool invocation in a session doesn't
pay npx's registry-lookup cost. It must never raise and must accurately
report success/failure via its return value — hermes_cli/doctor.py's
`--fix` path branches on that return value; hermes_cli/update_cmd.py calls
it fire-and-forget inside its own try/except and ignores the result.
"""
from __future__ import annotations
import subprocess
from unittest.mock import patch
from tools.browser_tool import warm_agent_browser_npx_cache
def test_returns_false_without_calling_subprocess_when_npx_missing():
with patch("shutil.which", return_value=None), patch(
"subprocess.run"
) as mock_run:
assert warm_agent_browser_npx_cache() is False
mock_run.assert_not_called()
def test_invokes_npx_with_prefer_offline_version_check():
with patch("shutil.which", return_value="/usr/bin/npx"), patch(
"subprocess.run",
return_value=subprocess.CompletedProcess([], 0, stdout="1.2.3\n", stderr=""),
) as mock_run:
assert warm_agent_browser_npx_cache() is True
mock_run.assert_called_once()
args, kwargs = mock_run.call_args
assert args[0] == ["/usr/bin/npx", "--prefer-offline", "-y", "agent-browser", "--version"]
assert kwargs.get("check") is False
def test_custom_timeout_is_forwarded_to_subprocess_run():
with patch("shutil.which", return_value="/usr/bin/npx"), patch(
"subprocess.run",
return_value=subprocess.CompletedProcess([], 0, stdout="", stderr=""),
) as mock_run:
warm_agent_browser_npx_cache(timeout=5.0)
assert mock_run.call_args.kwargs.get("timeout") == 5.0
def test_returns_false_on_nonzero_exit():
with patch("shutil.which", return_value="/usr/bin/npx"), patch(
"subprocess.run",
return_value=subprocess.CompletedProcess([], 1, stdout="", stderr="registry unreachable"),
):
assert warm_agent_browser_npx_cache() is False
def test_returns_false_instead_of_raising_on_timeout():
with patch("shutil.which", return_value="/usr/bin/npx"), patch(
"subprocess.run",
side_effect=subprocess.TimeoutExpired(cmd=["npx"], timeout=60.0),
):
assert warm_agent_browser_npx_cache() is False
def test_returns_false_instead_of_raising_on_unexpected_exception():
"""Fire-and-forget contract: hermes_cli/doctor.py calls this bare (no
try/except of its own), so any exception inside the subprocess call
must be swallowed here, not propagated."""
with patch("shutil.which", return_value="/usr/bin/npx"), patch(
"subprocess.run", side_effect=OSError("fork failed")
):
assert warm_agent_browser_npx_cache() is False
+55 -5
View File
@@ -2350,6 +2350,23 @@ def _agent_browser_candidate_present(path: str | None) -> bool:
return os.path.exists(path) and (os.name == "nt" or os.access(path, os.X_OK))
def _resolve_npx_bin() -> Optional[str]:
"""Resolve the npx binary via the same PATH + extended-PATH cascade
_find_agent_browser uses, so callers that need npx's actual path
(rather than the "npx agent-browser" sentinel) can't diverge from what
_find_agent_browser itself would have found. A bare ``shutil.which("npx")``
misses Hermes-managed-Node-only setups where npx only resolves via the
extended fallback PATH (Homebrew, $HERMES_HOME/node, etc.).
"""
npx_path = shutil.which("npx")
if npx_path:
return npx_path
extended_path = _merge_browser_path("")
if extended_path:
return shutil.which("npx", path=extended_path)
return None
def _find_agent_browser(*, validate: bool = True) -> str:
"""
Find the agent-browser CLI executable.
@@ -2369,7 +2386,6 @@ def _find_agent_browser(*, validate: bool = True) -> str:
raise FileNotFoundError(
"agent-browser CLI not found (cached). Install it with: "
f"{_browser_install_hint()}\n"
"Or run 'npm install' in the repo root to install locally.\n"
"Or ensure npx is available in your PATH."
)
return _cached_agent_browser
@@ -2434,9 +2450,7 @@ def _find_agent_browser(*, validate: bool = True) -> str:
return _cached_agent_browser
# Check common npx locations (also search the extended fallback PATH)
npx_path = shutil.which("npx")
if not npx_path and extended_path:
npx_path = shutil.which("npx", path=extended_path)
npx_path = _resolve_npx_bin()
if npx_path:
if not validate:
return "npx agent-browser"
@@ -2470,11 +2484,47 @@ def _find_agent_browser(*, validate: bool = True) -> str:
raise FileNotFoundError(
"agent-browser CLI not found. Install it with: "
f"{_browser_install_hint()}\n"
"Or run 'npm install' in the repo root to install locally.\n"
"Or ensure npx is available in your PATH."
)
def warm_agent_browser_npx_cache(timeout: float = 60.0) -> bool:
"""Best-effort pre-fetch of the agent-browser npm package via npx.
agent-browser is no longer a root package.json dependency (#43564) —
it resolves lazily via ``npx agent-browser`` instead, which keeps it
out of the npm workspace install graph entirely (nothing to prune it
anymore) but means the first real invocation in a session would
otherwise pay npx's registry-lookup/fetch cost. Calling this during
``hermes update`` (or ``hermes doctor --fix``) warms npx's own cache
ahead of time, restoring the "available before any session starts"
property agent-browser had while it was an eager root dependency —
without re-entangling it with the workspace graph.
Fire-and-forget: never raises, always safe to call opportunistically.
Returns True only if npx actually ran successfully (npx unavailable,
a timeout, or a nonzero exit all return False silently).
"""
npx_bin = _resolve_npx_bin()
if not npx_bin:
return False
try:
result = subprocess.run(
# --prefer-offline: once cached, repeat `hermes update`/`doctor
# --fix` runs shouldn't hit the registry just to re-confirm
# "latest" is still latest — that would defeat the point of
# warming the cache in the first place.
[npx_bin, "--prefer-offline", "-y", "agent-browser", "--version"],
capture_output=True,
text=True,
timeout=timeout,
check=False,
)
return result.returncode == 0
except Exception:
return False
def _extract_screenshot_path_from_text(text: str) -> Optional[str]:
"""Extract a screenshot file path from agent-browser human-readable output."""
if not text: