fix(update): rebuild Desktop after release artifact loss
This commit is contained in:
konsisumer
+100
-60
@@ -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)
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user