From c844d15c3d27991a35bbc4ec56558d85122412c9 Mon Sep 17 00:00:00 2001 From: briandevans <252620095+briandevans@users.noreply.github.com> Date: Sat, 2 May 2026 08:15:11 -0700 Subject: [PATCH] fix(update): stream npm install output so postinstall progress is visible (#18840) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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) --- hermes_cli/main.py | 11 ++++++++-- tests/hermes_cli/test_cmd_update.py | 31 +++++++++++++++++++++++++---- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 41c4a23f932..a893ee85846 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -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]}") diff --git a/tests/hermes_cli/test_cmd_update.py b/tests/hermes_cli/test_cmd_update.py index f059e54ac05..2f4b836286b 100644 --- a/tests/hermes_cli/test_cmd_update.py +++ b/tests/hermes_cli/test_cmd_update.py @@ -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(