diff --git a/hermes_cli/main.py b/hermes_cli/main.py index ed8c24c8fa71b..137bc8f1b0931 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -6269,17 +6269,24 @@ def _update_node_dependencies() -> None: if not (path / "package.json").exists(): continue + # Stream npm output (no `--silent`, no `capture_output`) so optional + # dependency postinstall scripts — e.g. `@askjo/camofox-browser`'s + # `npx camoufox-js fetch` browser-binary download — 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 caac6d37278c9..f485fa0d42b0a 100644 --- a/tests/hermes_cli/test_cmd_update.py +++ b/tests/hermes_cli/test_cmd_update.py @@ -128,21 +128,43 @@ def test_update_refreshes_repo_and_tui_node_dependencies( # 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 == [ - (full_flags, PROJECT_ROOT), - (full_flags, PROJECT_ROOT / "ui-tui"), + (update_flags, PROJECT_ROOT), + (update_flags, PROJECT_ROOT / "ui-tui"), (["/usr/bin/npm", "ci", "--silent"], PROJECT_ROOT / "web"), (["/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 Camofox postinstall progress is visible" + ) + def test_update_non_interactive_skips_migration_prompt(self, mock_args, capsys): """When stdin/stdout aren't TTYs, config migration prompt is skipped.""" with patch("shutil.which", return_value=None), patch(