mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-18 04:41:56 +00:00
fix(update): stream npm install output so postinstall progress is visible (#18840)
`hermes update` ran the repo-root and ui-tui npm installs with both `--silent` and `subprocess.run(..., capture_output=True)`, which hides all output from optional postinstall scripts. The largest of those — `@askjo/camofox-browser`'s `npx camoufox-js fetch` — downloads a Firefox-fork browser binary that can take many minutes on slow connections. Because nothing was printed during that wait, the updater appeared to hang at "Updating Node.js dependencies..." and users Ctrl-C'd, sometimes leaving `node_modules` partially installed. Drop `--silent` and pass `capture_output=False` for the repo-root and ui-tui paths so npm streams its `info run …` postinstall lines straight to the terminal. Output is still mirrored to `~/.hermes/logs/update.log` by the existing `_UpdateOutputStream` wrapper, so SSH-disconnect safety is preserved. The `web/` install path is untouched — its build step is fast and does not run binary-fetching postinstalls. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
05af78c53d
commit
c844d15c3d
2 changed files with 36 additions and 6 deletions
|
|
@ -7226,17 +7226,24 @@ def _update_node_dependencies() -> None:
|
|||
if not (path / "package.json").exists():
|
||||
continue
|
||||
|
||||
# Stream npm output (no `--silent`, no `capture_output`) so any
|
||||
# optional dependency postinstall scripts (e.g. `agent-browser`'s
|
||||
# Chromium fetch on first install) print progress instead of
|
||||
# appearing to hang silently for minutes (#18840). The
|
||||
# `_UpdateOutputStream` wrapper installed by the updater mirrors
|
||||
# streamed output to ``~/.hermes/logs/update.log`` so nothing is lost.
|
||||
result = _run_npm_install_deterministic(
|
||||
npm,
|
||||
path,
|
||||
extra_args=("--silent", "--no-fund", "--no-audit", "--progress=false"),
|
||||
extra_args=("--no-fund", "--no-audit", "--progress=false"),
|
||||
capture_output=False,
|
||||
)
|
||||
if result.returncode == 0:
|
||||
print(f" ✓ {label}")
|
||||
continue
|
||||
|
||||
print(f" ⚠ npm install failed in {label}")
|
||||
stderr = (result.stderr or "").strip()
|
||||
stderr = (result.stderr or "").strip() if result.stderr else ""
|
||||
if stderr:
|
||||
print(f" {stderr.splitlines()[-1]}")
|
||||
|
||||
|
|
|
|||
|
|
@ -130,17 +130,22 @@ class TestCmdUpdateBranchFallback:
|
|||
# 1. repo root — slash-command / TUI bridge deps
|
||||
# 2. ui-tui/ — Ink TUI deps
|
||||
# 3. web/ — install + "npm run build" for the web frontend
|
||||
full_flags = [
|
||||
#
|
||||
# Repo-root and ui-tui installs intentionally omit `--silent` and run
|
||||
# without `capture_output` so optional postinstall scripts (e.g.
|
||||
# `@askjo/camofox-browser`'s browser-binary fetch) print progress —
|
||||
# otherwise long downloads look like a hang (#18840). The web/ install
|
||||
# keeps `--silent` because its build step is short and noisy.
|
||||
update_flags = [
|
||||
"/usr/bin/npm",
|
||||
"ci",
|
||||
"--silent",
|
||||
"--no-fund",
|
||||
"--no-audit",
|
||||
"--progress=false",
|
||||
]
|
||||
assert npm_calls[:2] == [
|
||||
(full_flags, PROJECT_ROOT),
|
||||
(full_flags, PROJECT_ROOT / "ui-tui"),
|
||||
(update_flags, PROJECT_ROOT),
|
||||
(update_flags, PROJECT_ROOT / "ui-tui"),
|
||||
]
|
||||
if len(npm_calls) > 2:
|
||||
assert npm_calls[2:] == [
|
||||
|
|
@ -148,6 +153,24 @@ class TestCmdUpdateBranchFallback:
|
|||
(["/usr/bin/npm", "run", "build"], PROJECT_ROOT / "web"),
|
||||
]
|
||||
|
||||
# Regression for #18840: repo root + ui-tui installs must stream
|
||||
# output (capture_output=False) so postinstall progress is visible
|
||||
# to the user.
|
||||
repo_and_tui_calls = [
|
||||
call
|
||||
for call in mock_run.call_args_list
|
||||
if call.args
|
||||
and call.args[0][0] == "/usr/bin/npm"
|
||||
and call.args[0][1] == "ci"
|
||||
and call.kwargs.get("cwd") in (PROJECT_ROOT, PROJECT_ROOT / "ui-tui")
|
||||
]
|
||||
assert len(repo_and_tui_calls) == 2
|
||||
for call in repo_and_tui_calls:
|
||||
assert call.kwargs.get("capture_output") is False, (
|
||||
"repo-root / ui-tui npm install must stream output "
|
||||
"(no capture_output) so postinstall progress is visible"
|
||||
)
|
||||
|
||||
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(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue