From 6907c9784771fecc9d2e091fe29591fbc257458e Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 19 Aug 2026 02:10:37 -0500 Subject: [PATCH 1/2] fix(update): don't report success when the Desktop rebuild failed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hermes update treated a failed desktop pack as non-fatal and still printed ✓ Update complete!, so Windows users kept running an old Hermes.exe after a "successful" update. Withhold the success banner, surface the stale app in the summary, and write .update_exit_code=1 for gateway watchers. Supersedes #88359, #87984. Co-authored-by: joaomarcos Co-authored-by: liuhao1024 --- hermes_cli/update_cmd.py | 116 ++++++++---- .../test_update_desktop_stale_warning.py | 168 ++++++++++++++++++ 2 files changed, 246 insertions(+), 38 deletions(-) create mode 100644 tests/hermes_cli/test_update_desktop_stale_warning.py diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index 107f6f778c378..a39420e3ac7d2 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -1034,11 +1034,50 @@ def _update_complete_message(pre_version: str | None) -> str: return "✓ Update complete!" -def _update_via_zip(args, *, had_desktop_app_before_update: bool = False): +def _print_update_summary( + *, + node_failures: list, + desktop_build_ok: bool, + pre_update_version: str | None, +) -> None: + """Final update banner. A failed Desktop rebuild is non-fatal for the + Python side, but must not print ``✓ Update complete!`` (#88251).""" + print() + if node_failures or not desktop_build_ok: + parts = [] + if node_failures: + parts.append( + f"Node.js dependencies for {', '.join(node_failures)} did not refresh" + ) + if not desktop_build_ok: + parts.append( + "the desktop app was not rebuilt and is still on the previous build" + ) + print("⚠ Update partially complete — " + "; ".join(parts) + ".") + if node_failures: + print(" Code and Python deps are updated, but the dashboard/TUI may") + print(" be in a mixed state until the Node deps are rebuilt.") + if not desktop_build_ok: + print(" Run `hermes desktop` to retry the desktop rebuild.") + else: + _print_update_completion(_update_complete_message(pre_update_version)) + + +def _write_gateway_update_exit_code(ok: bool) -> None: + path = get_hermes_home() / ".update_exit_code" + try: + path.write_text("0" if ok else "1", encoding="utf-8") + except OSError: + pass + + +def _update_via_zip(args, *, had_desktop_app_before_update: bool = False) -> bool: """Update Hermes Agent by downloading a ZIP archive. Used on Windows when git file I/O is broken (antivirus, NTFS filter drivers causing 'Invalid argument' errors on file creation). + + Returns ``False`` when a Desktop rebuild ran and failed; ``True`` otherwise. """ active_tool_dependencies = _m()._capture_active_tool_dependencies() @@ -1291,7 +1330,7 @@ def _update_via_zip(args, *, had_desktop_app_before_update: bool = False): node_failures = _update_node_dependencies() _m()._build_web_ui(_m().PROJECT_ROOT / "web") - _rebuild_desktop_after_update( + desktop_build_ok = _rebuild_desktop_after_update( _m().PROJECT_ROOT / "apps" / "desktop", had_desktop_app_before_update=had_desktop_app_before_update, ) @@ -1396,16 +1435,11 @@ def _update_via_zip(args, *, had_desktop_app_before_update: bool = False): "Post-update state.db integrity check (zip path) failed: %s", exc ) - print() - if node_failures: - print( - "⚠ Update partially complete — Node.js dependencies for " - f"{', '.join(node_failures)} did not refresh." - ) - print(" Code and Python deps are updated, but the dashboard/TUI may") - print(" be in a mixed state until the Node deps are rebuilt.") - else: - _print_update_completion(_update_complete_message(pre_update_version)) + _print_update_summary( + node_failures=node_failures, + desktop_build_ok=desktop_build_ok, + pre_update_version=pre_update_version, + ) try: _print_curator_first_run_notice() except Exception as e: @@ -1417,6 +1451,7 @@ def _update_via_zip(args, *, had_desktop_app_before_update: bool = False): # Don't stop a working dashboard when the Node refresh failed — see the # git-update path for rationale (#30271). _finish_dashboard_update_cleanup(node_failures) + return desktop_build_ok def _stash_local_changes_if_needed(git_cmd: list[str], cwd: Path) -> Optional[str]: status = subprocess.run( @@ -4535,8 +4570,15 @@ def _desktop_app_present(desktop_dir: Path) -> bool: def _rebuild_desktop_after_update( desktop_dir: Path, *, had_desktop_app_before_update: bool -) -> None: - """Rebuild an installed Desktop app when its source or artifact changed.""" +) -> bool: + """Rebuild an installed Desktop app when its source or artifact changed. + + Returns ``False`` only when a rebuild was attempted and failed, so the + caller can withhold ``✓ Update complete!`` and (in gateway mode) write + a failing ``.update_exit_code`` (#88251). Every other outcome — nothing + to rebuild, up to date, build succeeded, Desktop never installed — + returns ``True``. + """ # The release tree is ignored by git and can disappear during an update. # Its pre-update presence is enough to restore it; do not make people who # have never used Desktop pay for an Electron build. @@ -4546,7 +4588,7 @@ def _rebuild_desktop_after_update( and _m()._resolve_node_runtime_npm() and has_desktop_app ): - return + return True print("→ Checking if desktop app needs rebuilding...") # Consult the content-hash stamp IN-PROCESS first. The spawned @@ -4565,7 +4607,7 @@ def _rebuild_desktop_after_update( skip_desktop_build = False if skip_desktop_build: print(" ✓ Desktop app up to date") - return + return True desktop_build_cmd = [sys.executable, "-m", "hermes_cli.main", "desktop", "--build-only"] # Capture the (very loud) Electron/vite build output into update.log @@ -4590,15 +4632,16 @@ def _rebuild_desktop_after_update( desktop_build_cmd, cwd=_m().PROJECT_ROOT, env=build_env ) if build_result.returncode != 0: - print(" ⚠ Desktop build failed (non-fatal; run `hermes desktop` to retry)") + print(" ⚠ Desktop build failed (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") + return False + print(" ✓ Desktop app up to date") + return True def _cmd_update_impl(args, gateway_mode: bool): @@ -4815,12 +4858,14 @@ def _cmd_update_impl(args, gateway_mode: bool): if use_zip_update: # ZIP-based update for Windows when git is broken try: - _update_via_zip( + desktop_build_ok = _update_via_zip( args, had_desktop_app_before_update=had_desktop_app_before_update, ) finally: _m()._resume_windows_gateways_after_update(_windows_gateway_resume) + if gateway_mode: + _write_gateway_update_exit_code(desktop_build_ok) return # Fetch and pull @@ -5468,7 +5513,7 @@ def _cmd_update_impl(args, gateway_mode: bool): node_failures = _update_node_dependencies() _m()._build_web_ui(_m().PROJECT_ROOT / "web") - _rebuild_desktop_after_update( + desktop_build_ok = _rebuild_desktop_after_update( desktop_dir, had_desktop_app_before_update=had_desktop_app_before_update, ) @@ -5845,16 +5890,11 @@ def _print_items(items, label, key, fallback_key=None): # Never let the cron safety net break an otherwise-good update. logger.debug("Cron jobs auto-restore check failed: %s", exc) - print() - if node_failures: - print( - "⚠ Update partially complete — Node.js dependencies for " - f"{', '.join(node_failures)} did not refresh." - ) - print(" Code and Python deps are updated, but the dashboard/TUI may") - print(" be in a mixed state until the Node deps are rebuilt.") - else: - _print_update_completion(_update_complete_message(pre_update_version)) + _print_update_summary( + node_failures=node_failures, + desktop_build_ok=desktop_build_ok, + pre_update_version=pre_update_version, + ) # Search-index optimization notice (v23). Existing installs keep their # working search index untouched on update; the compact v23 layout — @@ -5959,13 +5999,11 @@ def _print_items(items, label, key, fallback_key=None): # # Writing the marker here — after git pull + pip install succeed but # before we attempt the restart — ensures the new gateway sees it - # regardless of how we die. + # regardless of how we die. Gated on desktop_build_ok (#88251): a + # Desktop rebuild failure must not be reported as "0" — the gateway's + # /update watcher (gateway/run.py) polls this file. if gateway_mode: - _exit_code_path = get_hermes_home() / ".update_exit_code" - try: - _exit_code_path.write_text("0", encoding="utf-8") - except OSError: - pass + _write_gateway_update_exit_code(desktop_build_ok) gateway_fleet_restart_incomplete = False # Snapshot of gateways running before we touch anything. Stays empty @@ -6790,10 +6828,12 @@ def _on_unit_timeout(svc_name: str, exc: subprocess.TimeoutExpired) -> None: print(f"⚠ Git update failed: {e}") print("→ Falling back to ZIP download...") print() - _update_via_zip( + desktop_build_ok = _update_via_zip( args, had_desktop_app_before_update=had_desktop_app_before_update, ) + if gateway_mode: + _write_gateway_update_exit_code(desktop_build_ok) else: print(f"✗ Update failed: {e}") sys.exit(1) diff --git a/tests/hermes_cli/test_update_desktop_stale_warning.py b/tests/hermes_cli/test_update_desktop_stale_warning.py new file mode 100644 index 0000000000000..294c9ca17de47 --- /dev/null +++ b/tests/hermes_cli/test_update_desktop_stale_warning.py @@ -0,0 +1,168 @@ +"""A failed Desktop pack must not look like a successful update. + +#88251: ``hermes update`` treated a failed desktop pack as non-fatal, printed +an early warning, then still ended with ``✓ Update complete!``. The Python +side moved on; the Electron app stayed on the previous build. + +``_rebuild_desktop_after_update`` returns False only when a rebuild was +attempted and failed. The final banner then prints ``⚠ Update partially +complete`` instead of the success line, and gateway mode writes ``1`` to +``.update_exit_code``. +""" + +import pytest + +from hermes_cli import update_cmd +from hermes_cli.update_cmd import ( + _print_update_summary, + _rebuild_desktop_after_update, + _write_gateway_update_exit_code, +) + + +class _Result: + def __init__(self, returncode: int, stdout: str = ""): + self.returncode = returncode + self.stdout = stdout + + +@pytest.fixture() +def desktop_env(tmp_path, monkeypatch): + """A desktop dir that looks installed and a faked CLI main module.""" + desktop_dir = tmp_path / "apps" / "desktop" + desktop_dir.mkdir(parents=True) + (desktop_dir / "package.json").write_text("{}", encoding="utf-8") + + calls = {"builds": 0, "build_needed": True} + + class _FakeMain: + PROJECT_ROOT = tmp_path + + @staticmethod + def _resolve_node_runtime_npm(): + return "/fake/npm" + + @staticmethod + def _desktop_build_needed(*_a, **_kw): + return calls["build_needed"] + + @staticmethod + def _run_logged_subprocess(cmd, cwd=None, env=None): + calls["builds"] += 1 + return _Result(1, stdout="Error: [stage-native-deps] boom") + + monkeypatch.setattr(update_cmd, "_m", lambda: _FakeMain) + monkeypatch.setattr( + "hermes_constants.with_hermes_node_path", lambda: {}, raising=False + ) + monkeypatch.setattr( + "hermes_constants.display_hermes_home", lambda: str(tmp_path), raising=False + ) + return desktop_dir, calls + + +def _run(desktop_dir): + return _rebuild_desktop_after_update( + desktop_dir, had_desktop_app_before_update=True + ) + + +def test_failed_rebuild_returns_false_and_keeps_the_retry_hint(desktop_env, capsys): + desktop_dir, calls = desktop_env + assert _run(desktop_dir) is False + assert calls["builds"] == 2 + out = capsys.readouterr().out + assert "Desktop build failed" in out + assert "stage-native-deps" in out + assert "Update complete" not in out + + +def test_successful_rebuild_returns_true(desktop_env, monkeypatch, capsys): + desktop_dir, _calls = desktop_env + builds = [] + monkeypatch.setattr( + update_cmd._m(), + "_run_logged_subprocess", + staticmethod(lambda cmd, cwd=None, env=None: builds.append(cmd) or _Result(0)), + ) + assert _run(desktop_dir) is True + assert len(builds) == 1 + assert "Desktop app up to date" in capsys.readouterr().out + + +def test_up_to_date_desktop_returns_true_without_spawning(desktop_env): + desktop_dir, calls = desktop_env + calls["build_needed"] = False + assert _run(desktop_dir) is True + assert calls["builds"] == 0 + + +def test_desktop_never_installed_returns_true(tmp_path, monkeypatch): + spawned = [] + monkeypatch.setattr( + update_cmd, + "_m", + lambda: type( + "_M", + (), + { + "PROJECT_ROOT": tmp_path, + "_resolve_node_runtime_npm": staticmethod(lambda: "/fake/npm"), + "_run_logged_subprocess": staticmethod( + lambda *a, **k: spawned.append(1) or _Result(0) + ), + }, + ), + ) + missing = tmp_path / "apps" / "desktop" + missing.mkdir(parents=True) + assert _run(missing) is True + assert spawned == [] + + +def test_summary_omits_success_banner_when_desktop_rebuild_failed(capsys): + _print_update_summary( + node_failures=[], + desktop_build_ok=False, + pre_update_version="0.20.1", + ) + out = capsys.readouterr().out + assert "Update complete" not in out + assert "partially complete" in out + assert "desktop app was not rebuilt" in out + assert "hermes desktop" in out + + +def test_summary_keeps_success_banner_when_desktop_ok(capsys, monkeypatch): + monkeypatch.setattr( + update_cmd, "_update_complete_message", lambda _v: "✓ Update complete! (v0.20.2)" + ) + monkeypatch.setattr(update_cmd, "_branch_head_suffix", lambda *a, **k: "") + _print_update_summary( + node_failures=[], + desktop_build_ok=True, + pre_update_version="0.20.1", + ) + out = capsys.readouterr().out + assert "✓ Update complete!" in out + assert "partially complete" not in out + + +def test_summary_combines_node_and_desktop_failures(capsys): + _print_update_summary( + node_failures=["dashboard"], + desktop_build_ok=False, + pre_update_version="0.20.1", + ) + out = capsys.readouterr().out + assert "Update complete" not in out + assert "dashboard" in out + assert "desktop app was not rebuilt" in out + + +def test_gateway_exit_code_file_tracks_desktop_rebuild(tmp_path, monkeypatch): + monkeypatch.setattr(update_cmd, "get_hermes_home", lambda: tmp_path) + _write_gateway_update_exit_code(True) + assert (tmp_path / ".update_exit_code").read_text(encoding="utf-8") == "0" + _write_gateway_update_exit_code(False) + assert (tmp_path / ".update_exit_code").read_text(encoding="utf-8") == "1" From ff3dbed3f7513eb67545a93543453dd7158879ec Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 19 Aug 2026 02:42:48 -0500 Subject: [PATCH 2/2] ci: retrigger workflows after the orchestrator died before scheduling jobs