mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-01 12:02:05 +00:00
fix(update): route loud build/installer output to update.log instead of the terminal (#53616)
* fix(update): route loud build/installer output to update.log instead of the terminal hermes update flooded the terminal with the full vite asset dump, electron-builder logs, npm deprecation warnings from the desktop build, and the cua-driver installer's 'Next steps' wall. All of that is low-signal noise the user doesn't need on a successful update. - Capture the desktop --build-only subprocess (vite + electron-builder) into ~/.hermes/logs/update.log; print a one-line status, and on failure surface the last 15 lines + a pointer to the full log. - Capture the cua-driver installer's output when verbose=False (the hermes update refresh path); concise upgrade line is unchanged. - Add _log_only_write() / _run_logged_subprocess() helpers that write to the update.log handle without echoing to the terminal. The repo-root npm install keeps streaming (capture_output=False) — that is the deliberate #18840 guard so a slow postinstall download doesn't look hung. The desktop npm install is a separate Electron process with no such progress concern and is captured. * fix(update): persist full cua-driver installer output to update.log The captured cua-driver installer output was only sent to logger.debug (agent.log) on failure, so the 'Next steps' wall was lost from update.log entirely on success. Write the full captured output straight to the update.log handle (sys.stdout._log) on both success and failure, matching the desktop-build capture, so update.log keeps the complete record of everything an update did.
This commit is contained in:
parent
f53b184c48
commit
27322612b4
3 changed files with 163 additions and 7 deletions
|
|
@ -7817,6 +7817,11 @@ def _update_node_dependencies() -> None:
|
|||
nixos_env = with_hermes_node_path(_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 = _run_npm_install_deterministic(
|
||||
npm,
|
||||
|
|
@ -8005,6 +8010,50 @@ def _install_hangup_protection(gateway_mode: bool = False):
|
|||
return state
|
||||
|
||||
|
||||
def _log_only_write(text: str) -> None:
|
||||
"""Write ``text`` to ``~/.hermes/logs/update.log`` only, never the terminal.
|
||||
|
||||
During ``hermes update`` ``sys.stdout`` is an ``_UpdateOutputStream`` that
|
||||
mirrors to both the terminal and ``update.log``. Loud, low-signal
|
||||
subprocess output (npm installs, the Electron/vite build, the cua-driver
|
||||
installer's "Next steps" wall) should be captured and tucked into the log
|
||||
so failures stay debuggable, without flooding the user's terminal. This
|
||||
reaches past the mirroring stream straight to the underlying log handle.
|
||||
"""
|
||||
if not text:
|
||||
return
|
||||
stream = sys.stdout
|
||||
log_file = getattr(stream, "_log", None)
|
||||
if log_file is None:
|
||||
return
|
||||
try:
|
||||
log_file.write(text if text.endswith("\n") else text + "\n")
|
||||
log_file.flush()
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
|
||||
def _run_logged_subprocess(cmd, *, cwd=None, env=None):
|
||||
"""Run ``cmd`` capturing combined output into update.log (not the terminal).
|
||||
|
||||
Returns the ``CompletedProcess`` (with ``stdout`` populated) so the caller
|
||||
can decide whether to surface the captured output on failure.
|
||||
"""
|
||||
result = subprocess.run(
|
||||
cmd,
|
||||
cwd=cwd,
|
||||
env=env,
|
||||
check=False,
|
||||
stdout=subprocess.PIPE,
|
||||
stderr=subprocess.STDOUT,
|
||||
text=True,
|
||||
encoding="utf-8",
|
||||
errors="replace",
|
||||
)
|
||||
_log_only_write(result.stdout or "")
|
||||
return result
|
||||
|
||||
|
||||
def _finalize_update_output(state):
|
||||
"""Restore stdio and close the update.log handle opened by ``_install_hangup_protection``."""
|
||||
if not state:
|
||||
|
|
@ -9344,15 +9393,24 @@ def _cmd_update_impl(args, gateway_mode: bool):
|
|||
if (desktop_dir / "package.json").exists() and find_node_executable("npm") and has_desktop_app:
|
||||
print("→ Checking if desktop app needs rebuilding...")
|
||||
_desktop_build_cmd = [sys.executable, "-m", "hermes_cli.main", "desktop", "--build-only"]
|
||||
# Stream the build output live (long Electron builds otherwise
|
||||
# look hung). On the rare nonzero exit, retry once after waiting
|
||||
# again for the venv — this covers a still-settling rebuild window
|
||||
# the first wait didn't fully catch.
|
||||
build_result = subprocess.run(_desktop_build_cmd, cwd=PROJECT_ROOT, check=False)
|
||||
# Capture the (very loud) Electron/vite build output into
|
||||
# update.log instead of streaming it to the terminal. On the rare
|
||||
# nonzero exit, retry once after waiting again for the venv — this
|
||||
# covers a still-settling rebuild window the first wait didn't fully
|
||||
# catch — then surface the captured tail so the failure is
|
||||
# debuggable.
|
||||
build_result = _run_logged_subprocess(_desktop_build_cmd, cwd=PROJECT_ROOT)
|
||||
if build_result.returncode != 0:
|
||||
build_result = subprocess.run(_desktop_build_cmd, cwd=PROJECT_ROOT, check=False)
|
||||
build_result = _run_logged_subprocess(_desktop_build_cmd, cwd=PROJECT_ROOT)
|
||||
if build_result.returncode != 0:
|
||||
print(" ⚠ Desktop build failed (non-fatal; run `hermes desktop` to retry)")
|
||||
tail = "\n".join((build_result.stdout or "").strip().splitlines()[-15:])
|
||||
if tail:
|
||||
print(tail)
|
||||
from hermes_constants import display_hermes_home as _dhh
|
||||
print(f" Full build log: {_dhh()}/logs/update.log")
|
||||
else:
|
||||
print(" ✓ Desktop app up to date")
|
||||
|
||||
print()
|
||||
print("✓ Code updated!")
|
||||
|
|
|
|||
|
|
@ -880,7 +880,38 @@ def _run_cua_driver_installer(label: str = "Installing", verbose: bool = True) -
|
|||
_print_info(f" {label} cua-driver...")
|
||||
driver_cmd = _cua_driver_cmd()
|
||||
try:
|
||||
result = subprocess.run(install_cmd, shell=use_shell, timeout=300, env=_cua_driver_env())
|
||||
# When not verbose (e.g. `hermes update`'s refresh), capture the
|
||||
# installer's chatty "Next steps" wall instead of dumping it to the
|
||||
# terminal. The combined output is logged so a failure stays
|
||||
# debuggable. Verbose installs (interactive `computer-use install`)
|
||||
# keep streaming live.
|
||||
if verbose:
|
||||
result = subprocess.run(install_cmd, shell=use_shell, timeout=300, env=_cua_driver_env())
|
||||
else:
|
||||
result = subprocess.run(
|
||||
install_cmd, shell=use_shell, timeout=300, env=_cua_driver_env(),
|
||||
stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
|
||||
text=True, encoding="utf-8", errors="replace",
|
||||
)
|
||||
# Preserve the full installer output. During `hermes update`,
|
||||
# sys.stdout is the mirroring _UpdateOutputStream whose `_log`
|
||||
# handle is ~/.hermes/logs/update.log — write straight to it so
|
||||
# the captured "Next steps" wall is kept in full (success AND
|
||||
# failure), without echoing it to the terminal.
|
||||
if result.stdout:
|
||||
_update_log = getattr(sys.stdout, "_log", None)
|
||||
if _update_log is not None:
|
||||
try:
|
||||
_update_log.write(
|
||||
"\n--- cua-driver installer output ---\n"
|
||||
+ result.stdout
|
||||
+ "\n"
|
||||
)
|
||||
_update_log.flush()
|
||||
except Exception:
|
||||
pass
|
||||
if result.returncode != 0:
|
||||
logger.debug("cua-driver installer output:\n%s", result.stdout)
|
||||
if result.returncode == 0 and shutil.which(driver_cmd):
|
||||
if verbose:
|
||||
_print_success(f" {driver_cmd} installed.")
|
||||
|
|
|
|||
|
|
@ -18,6 +18,8 @@ from hermes_cli.main import (
|
|||
_UpdateOutputStream,
|
||||
_finalize_update_output,
|
||||
_install_hangup_protection,
|
||||
_log_only_write,
|
||||
_run_logged_subprocess,
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -320,3 +322,68 @@ class TestFinalizeUpdateOutput:
|
|||
|
||||
assert sys.stdout is before_out
|
||||
assert sys.stderr is before_err
|
||||
|
||||
|
||||
# -----------------------------------------------------------------------------
|
||||
# _log_only_write / _run_logged_subprocess (spam suppression)
|
||||
# -----------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestLogOnlyWrite:
|
||||
def test_writes_to_log_not_terminal(self, monkeypatch):
|
||||
"""During an update, loud output should land in update.log only —
|
||||
never on the mirroring terminal stream."""
|
||||
terminal = io.StringIO()
|
||||
log = io.StringIO()
|
||||
stream = _UpdateOutputStream(terminal, log)
|
||||
monkeypatch.setattr(sys, "stdout", stream)
|
||||
|
||||
_log_only_write("npm warn deprecated foo\nadded 1302 packages")
|
||||
|
||||
assert terminal.getvalue() == "" # terminal stays quiet
|
||||
assert "npm warn deprecated foo" in log.getvalue()
|
||||
assert "added 1302 packages" in log.getvalue()
|
||||
|
||||
def test_noop_without_update_stream(self, monkeypatch):
|
||||
"""When stdout isn't the mirroring update stream (no ``_log``), it must
|
||||
be a silent no-op rather than crash."""
|
||||
plain = io.StringIO()
|
||||
monkeypatch.setattr(sys, "stdout", plain)
|
||||
_log_only_write("something") # should not raise
|
||||
assert plain.getvalue() == ""
|
||||
|
||||
def test_empty_text_is_noop(self, monkeypatch):
|
||||
terminal = io.StringIO()
|
||||
log = io.StringIO()
|
||||
monkeypatch.setattr(sys, "stdout", _UpdateOutputStream(terminal, log))
|
||||
_log_only_write("")
|
||||
assert log.getvalue() == ""
|
||||
|
||||
|
||||
class TestRunLoggedSubprocess:
|
||||
def test_captures_output_to_log_only(self, monkeypatch):
|
||||
terminal = io.StringIO()
|
||||
log = io.StringIO()
|
||||
monkeypatch.setattr(sys, "stdout", _UpdateOutputStream(terminal, log))
|
||||
|
||||
result = _run_logged_subprocess(
|
||||
[sys.executable, "-c", "print('LOUD BUILD OUTPUT')"]
|
||||
)
|
||||
|
||||
assert result.returncode == 0
|
||||
assert "LOUD BUILD OUTPUT" in (result.stdout or "")
|
||||
assert terminal.getvalue() == "" # not echoed to terminal
|
||||
assert "LOUD BUILD OUTPUT" in log.getvalue() # but kept in the log
|
||||
|
||||
def test_nonzero_exit_still_captures(self, monkeypatch):
|
||||
terminal = io.StringIO()
|
||||
log = io.StringIO()
|
||||
monkeypatch.setattr(sys, "stdout", _UpdateOutputStream(terminal, log))
|
||||
|
||||
result = _run_logged_subprocess(
|
||||
[sys.executable, "-c", "import sys; print('boom'); sys.exit(3)"]
|
||||
)
|
||||
|
||||
assert result.returncode == 3
|
||||
assert "boom" in (result.stdout or "")
|
||||
assert terminal.getvalue() == ""
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue