From 4aa9f738cedbe8a69fbd08595d0fb67f812ce2d3 Mon Sep 17 00:00:00 2001 From: konsisumer Date: Sat, 15 Aug 2026 02:22:14 +0200 Subject: [PATCH] fix(update): rebuild Desktop after release artifact loss --- hermes_cli/update_cmd.py | 160 +++++++++++++++++----------- tests/hermes_cli/test_cmd_update.py | 123 ++++++++++++++++++++- 2 files changed, 222 insertions(+), 61 deletions(-) diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index 9a5c994eb6..dcdd357079 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -773,7 +773,7 @@ def _print_update_completion(message: str) -> None: print(f"=== hermes-update completed {action_id} ===") -def _update_via_zip(args): +def _update_via_zip(args, *, had_desktop_app_before_update: bool = False): """Update Hermes Agent by downloading a ZIP archive. Used on Windows when git file I/O is broken (antivirus, NTFS filter @@ -1011,6 +1011,10 @@ def _update_via_zip(args): node_failures = _update_node_dependencies() _m()._build_web_ui(_m().PROJECT_ROOT / "web") + _rebuild_desktop_after_update( + _m().PROJECT_ROOT / "apps" / "desktop", + had_desktop_app_before_update=had_desktop_app_before_update, + ) # Sync skills try: @@ -3981,6 +3985,83 @@ def _normalize_managed_eol(git_cmd, repo_root): # Never let line-ending cleanup block an update. pass + +def _desktop_app_present(desktop_dir: Path) -> bool: + """Return whether a packaged or source Desktop build exists.""" + return ( + _m()._desktop_packaged_executable(desktop_dir) is not None + or _m()._desktop_dist_exists(desktop_dir) + ) + + +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.""" + # 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. + has_desktop_app = had_desktop_app_before_update or _desktop_app_present(desktop_dir) + if not ( + (desktop_dir / "package.json").exists() + and _m()._resolve_node_runtime_npm() + and has_desktop_app + ): + return + + print("→ Checking if desktop app needs rebuilding...") + # Consult the content-hash stamp IN-PROCESS first. The spawned + # `hermes desktop --build-only` subprocess re-imports the whole CLI stack + # (~1-3 s) just to reach the same _m()._desktop_build_needed check; when + # the stamp already says "up to date" we can skip the spawn entirely. The + # update path never passes --source, so the subprocess would run with + # source_mode=False — mirror that here. Any error in the pre-check falls + # through to the subprocess. + skip_desktop_build = False + try: + skip_desktop_build = not _m()._desktop_build_needed( + desktop_dir, _m().PROJECT_ROOT, source_mode=False + ) + except Exception: + skip_desktop_build = False + if skip_desktop_build: + print(" ✓ Desktop app up to date") + return + + desktop_build_cmd = [sys.executable, "-m", "hermes_cli.main", "desktop", "--build-only"] + # 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. + # + # Start the build subprocess with the Hermes-managed Node on PATH: when + # `hermes update` runs inside the desktop updater chain (Desktop → + # hermes-setup → hermes update), the shell PATH customizations are lost, + # so a bare-PATH child would fail with `node: not found` before cmd_gui can + # self-heal. + from hermes_constants import with_hermes_node_path + + build_env = with_hermes_node_path() + build_result = _m()._run_logged_subprocess( + desktop_build_cmd, cwd=_m().PROJECT_ROOT, env=build_env + ) + if build_result.returncode != 0: + build_result = _m()._run_logged_subprocess( + 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)") + 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") + + def _cmd_update_impl(args, gateway_mode: bool): """Body of ``cmd_update`` — kept separate so the wrapper can always restore stdio even on ``sys.exit``.""" @@ -4121,6 +4202,11 @@ def _cmd_update_impl(args, gateway_mode: bool): _m()._resume_windows_gateways_after_update(_windows_gateway_resume) sys.exit(2) + # Capture this after every fail-closed venv guard, but before either + # update path can remove the ignored release tree. + desktop_dir = _m().PROJECT_ROOT / "apps" / "desktop" + had_desktop_app_before_update = _desktop_app_present(desktop_dir) + # Try git-based update first, fall back to ZIP download on Windows # when git file I/O is broken (antivirus, NTFS filter drivers, etc.) use_zip_update = False @@ -4183,7 +4269,10 @@ 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(args) + _update_via_zip( + args, + had_desktop_app_before_update=had_desktop_app_before_update, + ) finally: _m()._resume_windows_gateways_after_update(_windows_gateway_resume) return @@ -4721,62 +4810,10 @@ def _cmd_update_impl(args, gateway_mode: bool): node_failures = _update_node_dependencies() _m()._build_web_ui(_m().PROJECT_ROOT / "web") - # Rebuild the desktop app if the source tree changed since the last - # build. ``hermes desktop --build-only`` uses the content-hash stamp - # internally, so this is effectively a no-op when nothing changed. - # Only bother if the user has a desktop app installed (indicated by - # an existing packaged executable or desktop dist); people who have - # never run ``hermes desktop`` shouldn't be forced into a full - # Electron build by ``hermes update``. - desktop_dir = _m().PROJECT_ROOT / "apps" / "desktop" - has_desktop_app = _m()._desktop_packaged_executable(desktop_dir) is not None or _m()._desktop_dist_exists(desktop_dir) - if (desktop_dir / "package.json").exists() and _m()._resolve_node_runtime_npm() and has_desktop_app: - print("→ Checking if desktop app needs rebuilding...") - # Consult the content-hash stamp IN-PROCESS first. The spawned - # `hermes desktop --build-only` subprocess re-imports the whole - # CLI stack (~1-3 s) just to reach the same _m()._desktop_build_needed - # check; when the stamp already says "up to date" we can skip the - # spawn entirely. The update path never passes --source, so the - # subprocess would run with source_mode=False — mirror that here. - # Any error in the pre-check falls through to the subprocess. - _skip_desktop_build = False - try: - _skip_desktop_build = not _m()._desktop_build_needed( - desktop_dir, _m().PROJECT_ROOT, source_mode=False - ) - except Exception: - _skip_desktop_build = False - if _skip_desktop_build: - print(" ✓ Desktop app up to date") - else: - _desktop_build_cmd = [sys.executable, "-m", "hermes_cli.main", "desktop", "--build-only"] - # 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. - # - # Start the build subprocess with the Hermes-managed Node on PATH: - # when `hermes update` runs inside the desktop updater chain - # (Desktop → hermes-setup → hermes update), the shell PATH - # customizations are lost, so a bare-PATH child would fail with - # `node: not found` before cmd_gui can self-heal. - from hermes_constants import with_hermes_node_path - - _build_env = with_hermes_node_path() - build_result = _m()._run_logged_subprocess(_desktop_build_cmd, cwd=_m().PROJECT_ROOT, env=_build_env) - if build_result.returncode != 0: - build_result = _m()._run_logged_subprocess(_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)") - 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") + _rebuild_desktop_after_update( + desktop_dir, + had_desktop_app_before_update=had_desktop_app_before_update, + ) print() print("✓ Code updated!") @@ -6011,11 +6048,14 @@ def _cmd_update_impl(args, gateway_mode: bool): sys.exit(1) except subprocess.CalledProcessError as e: - if sys.platform == "win32": + if _m()._is_windows(): print(f"⚠ Git update failed: {e}") print("→ Falling back to ZIP download...") print() - _update_via_zip(args) + _update_via_zip( + args, + had_desktop_app_before_update=had_desktop_app_before_update, + ) else: print(f"✗ Update failed: {e}") sys.exit(1) diff --git a/tests/hermes_cli/test_cmd_update.py b/tests/hermes_cli/test_cmd_update.py index 5badf69213..b9aac2a1ee 100644 --- a/tests/hermes_cli/test_cmd_update.py +++ b/tests/hermes_cli/test_cmd_update.py @@ -3,7 +3,7 @@ import hashlib import subprocess from types import SimpleNamespace -from unittest.mock import patch +from unittest.mock import ANY, patch import pytest @@ -809,6 +809,127 @@ class TestNodeRuntimeNpmResolution: for call in mock_run.call_args_list ) + def test_update_rebuilds_desktop_that_disappears_mid_update(self): + """A previously packaged Desktop must be rebuilt when its release tree vanishes.""" + from hermes_cli import main as hm + from hermes_cli import update_cmd + + desktop_dir = PROJECT_ROOT / "apps" / "desktop" + packaged_exe = desktop_dir / "release" / "win-unpacked" / "Hermes.exe" + build_ok = subprocess.CompletedProcess([], 0, stdout="", stderr="") + + with ( + patch.object( + hm, "_desktop_packaged_executable", side_effect=[packaged_exe, None] + ) as packaged, + patch.object(hm, "_desktop_dist_exists", return_value=False), + patch.object(hm, "_resolve_node_runtime_npm", return_value="npm.cmd"), + patch.object(hm, "_desktop_build_needed", return_value=True), + patch.object(hm, "_run_logged_subprocess", return_value=build_ok) as desktop_build, + ): + had_desktop_app_before_update = update_cmd._desktop_app_present(desktop_dir) + assert not update_cmd._desktop_app_present(desktop_dir) + update_cmd._rebuild_desktop_after_update( + desktop_dir, + had_desktop_app_before_update=had_desktop_app_before_update, + ) + + assert packaged.call_count == 2 + desktop_build.assert_called_once_with( + [hm.sys.executable, "-m", "hermes_cli.main", "desktop", "--build-only"], + cwd=PROJECT_ROOT, + env=ANY, + ) + + def test_git_failure_zip_fallback_rebuilds_missing_desktop(self, tmp_path, monkeypatch): + """The Windows ZIP fallback restores Desktop after replacing ``apps/``.""" + import zipfile + + from hermes_cli import main as hm + from hermes_cli import update_cmd + + project_root = tmp_path / "hermes-agent" + (project_root / ".git").mkdir(parents=True) + desktop_dir = project_root / "apps" / "desktop" + packaged_exe = desktop_dir / "release" / "win-unpacked" / "Hermes.exe" + packaged_exe.parent.mkdir(parents=True) + packaged_exe.write_bytes(b"desktop") + + def write_source_zip(_url, destination): + with zipfile.ZipFile(destination, "w") as archive: + archive.writestr("hermes-agent-main/apps/desktop/package.json", "{}") + + def fail_git_fetch(command, **_kwargs): + if "fetch" in command: + raise subprocess.CalledProcessError(1, command) + return subprocess.CompletedProcess(command, 0, stdout="", stderr="") + + desktop_builds = [] + + def rebuild_desktop(*_args, **_kwargs): + desktop_builds.append(not packaged_exe.exists()) + return subprocess.CompletedProcess([], 0, stdout="", stderr="") + + monkeypatch.setattr(hm, "PROJECT_ROOT", project_root) + monkeypatch.setattr(hm, "_is_windows", lambda: True) + monkeypatch.setattr(hm, "_run_pre_update_backup", lambda _args: None) + monkeypatch.setattr(hm, "_pause_windows_gateways_for_update", lambda: None) + monkeypatch.setattr(hm, "_get_origin_url", lambda *_args: "") + monkeypatch.setattr( + hm, + "_desktop_packaged_executable", + lambda _desktop_dir: packaged_exe if packaged_exe.exists() else None, + ) + monkeypatch.setattr(hm, "_desktop_dist_exists", lambda _desktop_dir: False) + monkeypatch.setattr(hm, "_resolve_node_runtime_npm", lambda: "npm.cmd") + monkeypatch.setattr(hm, "_desktop_build_needed", lambda *_args, **_kwargs: True) + monkeypatch.setattr(hm, "_run_logged_subprocess", rebuild_desktop) + monkeypatch.setattr(hm, "_clear_bytecode_cache", lambda *_args: 0) + monkeypatch.setattr(hm, "_record_bytecode_fingerprint", lambda: None) + monkeypatch.setattr(hm, "_refresh_bootstrap_cache_scripts", lambda _branch: None) + monkeypatch.setattr( + hm, "_install_python_dependencies_with_optional_fallback", lambda *_args, **_kwargs: None + ) + monkeypatch.setattr(hm, "_refresh_active_memory_provider_dependencies", lambda: None) + monkeypatch.setattr(hm, "_build_web_ui", lambda *_args: None) + monkeypatch.setattr(update_cmd, "_discard_lockfile_churn", lambda *_args: None) + monkeypatch.setattr(update_cmd, "_normalize_managed_eol", lambda *_args: None) + monkeypatch.setattr( + update_cmd, + "_validate_critical_modules_import", + lambda *_args: (True, None, None), + ) + monkeypatch.setattr(update_cmd, "_update_node_dependencies", lambda: []) + monkeypatch.setattr(update_cmd, "_print_curator_first_run_notice", lambda: None) + monkeypatch.setattr(update_cmd, "_print_curator_recent_run_notice", lambda: None) + monkeypatch.setattr(update_cmd, "_finish_dashboard_update_cleanup", lambda _failures: None) + monkeypatch.setattr(update_cmd, "get_hermes_home", lambda: tmp_path / "hermes-home") + + with ( + patch("hermes_cli.config.load_config", return_value={}), + patch("subprocess.run", side_effect=fail_git_fetch), + patch("urllib.request.urlretrieve", side_effect=write_source_zip), + patch("hermes_cli.managed_uv.ensure_uv", return_value="uv"), + patch("hermes_cli.managed_uv.update_managed_uv"), + patch( + "tools.skills_sync.sync_skills", + return_value={ + "copied": [], + "updated": [], + "user_modified": [], + "cleaned": [], + "relocated": [], + }, + ), + patch("hermes_cli.model_catalog.seed_cache_from_checkout", return_value=False), + ): + update_cmd._cmd_update_impl( + SimpleNamespace(yes=True, force=True, force_venv=True, branch=None), + gateway_mode=False, + ) + + assert desktop_builds == [True] + class TestUpdateNodeDependencies: """Unit tests for _update_node_dependencies — issue #43564.