mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-01 12:02:05 +00:00
fix(desktop): don't purge Electron cache / mirror-retry after a late build failure
`hermes desktop` / `hermes update` recover from a corrupt Electron download by purging the cached zip + re-downloading and retrying the pack, and then by falling back to a public mirror. That recovery is only meaningful when the packaged executable is MISSING — the signature of a partial/corrupt unpack. A LATE failure such as macOS code signing (#40187) leaves `Hermes.app/Contents/MacOS/Hermes` (or the platform equivalent) in place. Re-downloading Electron can't repair a signing failure, so the purge + slow mirror retry just grind through another identical failure before the build finally errors out. Gate both recovery blocks on `_desktop_packaged_executable(desktop_dir) is None` so a build that already produced the executable fails fast instead of triggering the destructive download recovery. The corrupt-download path (executable missing) is unchanged. Salvage of #42782, re-applied onto current main (the surrounding recovery was refactored to `_electron_dist_ok` / `_redownload_electron_dist` since the PR was opened). Adds a regression test asserting no purge / mirror retry runs when the executable exists, and updates the existing retry/mirror tests to model the corrupt-download case (executable absent) the recovery is actually for. Related to #40187 (the residual cache-purge sub-issue; the signing failure itself is fixed by #52591).
This commit is contained in:
parent
1ef19bad90
commit
25ec01f79f
2 changed files with 80 additions and 8 deletions
|
|
@ -5586,10 +5586,20 @@ def cmd_gui(args: argparse.Namespace):
|
|||
if stopped:
|
||||
print(f" ⚠ Stopped running desktop app to free the build output (pid {', '.join(map(str, stopped))})")
|
||||
build_result = subprocess.run([npm, "run", build_script], cwd=desktop_dir, env=env, check=False)
|
||||
if build_result.returncode != 0 and not source_mode:
|
||||
if (
|
||||
build_result.returncode != 0
|
||||
and not source_mode
|
||||
and _desktop_packaged_executable(desktop_dir) is None
|
||||
):
|
||||
# Corrupt cached Electron zip → partial unpack → ENOENT on rename.
|
||||
# stdlib zipfile won't catch the common concat-junk case, so purge
|
||||
# and retry once; @electron/get SHASUM is the real gate.
|
||||
#
|
||||
# Gate on a MISSING packaged executable: that is the signature of
|
||||
# the corrupt-download class this recovery exists for. A late
|
||||
# failure such as macOS code signing leaves the executable in
|
||||
# place — redownloading Electron can't repair it, so the purge +
|
||||
# retry would only add another slow, identical failure (#40187).
|
||||
purged: list[Path] = []
|
||||
restored = False
|
||||
if not _electron_dist_ok(PROJECT_ROOT):
|
||||
|
|
@ -5603,7 +5613,12 @@ def cmd_gui(args: argparse.Namespace):
|
|||
# is still locked by a running instance; stop it before retry.
|
||||
_stop_desktop_processes_locking_build(desktop_dir)
|
||||
build_result = subprocess.run([npm, "run", build_script], cwd=desktop_dir, env=env, check=False)
|
||||
if build_result.returncode != 0 and not source_mode and not env.get("ELECTRON_MIRROR"):
|
||||
if (
|
||||
build_result.returncode != 0
|
||||
and not source_mode
|
||||
and not env.get("ELECTRON_MIRROR")
|
||||
and _desktop_packaged_executable(desktop_dir) is None
|
||||
):
|
||||
print(" ⚠ Desktop build still failing; the Electron download from "
|
||||
"GitHub looks blocked. Re-downloading via a public mirror "
|
||||
"(npmmirror.com)... (set ELECTRON_MIRROR to use another mirror)")
|
||||
|
|
|
|||
|
|
@ -469,16 +469,34 @@ def test_purge_electron_build_cache_empty_when_nothing_present(tmp_path, monkeyp
|
|||
|
||||
|
||||
def test_gui_retries_pack_once_after_purging_build_cache(tmp_path, monkeypatch):
|
||||
"""First pack fails, purge clears the cache, second pack succeeds, launch."""
|
||||
"""First pack fails with NO packaged executable (corrupt-download shape),
|
||||
purge clears the cache, second pack succeeds and produces the exe, launch."""
|
||||
root = _make_desktop_tree(tmp_path)
|
||||
monkeypatch.setattr(cli_main, "PROJECT_ROOT", root)
|
||||
packaged_exe = _make_packaged_executable(root, monkeypatch, platform="linux")
|
||||
# Executable is ABSENT at build-failure time — that is the corrupt-download
|
||||
# signature the cache purge + retry exist for (#40187). Only the successful
|
||||
# retry produces it (via the side_effect below).
|
||||
monkeypatch.setattr(cli_main.sys, "platform", "linux")
|
||||
packaged_exe = root / "apps" / "desktop" / "release" / "linux-unpacked" / "hermes"
|
||||
|
||||
install_ok = subprocess.CompletedProcess(["npm", "ci"], 0)
|
||||
pack_fail = subprocess.CompletedProcess(["npm", "run", "pack"], 1)
|
||||
pack_ok = subprocess.CompletedProcess(["npm", "run", "pack"], 0)
|
||||
launch_ok = subprocess.CompletedProcess([str(packaged_exe)], 0)
|
||||
|
||||
_calls = {"n": 0}
|
||||
|
||||
def run_side_effect(*args, **kwargs):
|
||||
_calls["n"] += 1
|
||||
if _calls["n"] == 1:
|
||||
return pack_fail
|
||||
if _calls["n"] == 2:
|
||||
# Successful retry materializes the packaged executable.
|
||||
packaged_exe.parent.mkdir(parents=True, exist_ok=True)
|
||||
packaged_exe.write_text("", encoding="utf-8")
|
||||
return pack_ok
|
||||
return launch_ok
|
||||
|
||||
with patch("hermes_cli.main.shutil.which", return_value="/usr/bin/npm"), \
|
||||
patch("hermes_cli.main._run_npm_install_deterministic", return_value=install_ok), \
|
||||
patch("hermes_cli.main._desktop_macos_relaunchable_fixup"), \
|
||||
|
|
@ -487,7 +505,7 @@ def test_gui_retries_pack_once_after_purging_build_cache(tmp_path, monkeypatch):
|
|||
patch("hermes_cli.main._purge_electron_build_cache", return_value=[Path("/c/electron.zip")]) as mock_purge, \
|
||||
patch("hermes_cli.main._electron_dist_ok", return_value=False), \
|
||||
patch("hermes_cli.main._redownload_electron_dist", return_value=True), \
|
||||
patch("hermes_cli.main.subprocess.run", side_effect=[pack_fail, pack_ok, launch_ok]) as mock_run, \
|
||||
patch("hermes_cli.main.subprocess.run", side_effect=run_side_effect) as mock_run, \
|
||||
pytest.raises(SystemExit) as exc:
|
||||
cli_main.cmd_gui(_ns())
|
||||
|
||||
|
|
@ -506,7 +524,8 @@ def test_gui_redownloads_electron_via_mirror_then_repacks(tmp_path, monkeypatch,
|
|||
which never downloads Electron) and only then retry pack (#47266)."""
|
||||
root = _make_desktop_tree(tmp_path)
|
||||
monkeypatch.setattr(cli_main, "PROJECT_ROOT", root)
|
||||
_make_packaged_executable(root, monkeypatch, platform="linux")
|
||||
# No packaged executable: the corrupt-download recovery must run.
|
||||
monkeypatch.setattr(cli_main.sys, "platform", "linux")
|
||||
monkeypatch.delenv("ELECTRON_MIRROR", raising=False)
|
||||
|
||||
install_ok = subprocess.CompletedProcess(["npm", "ci"], 0)
|
||||
|
|
@ -545,7 +564,8 @@ def test_gui_retries_pack_under_mirror_even_when_prefetch_blocked(tmp_path, monk
|
|||
pointless (it was, back when electronDist was a static path)."""
|
||||
root = _make_desktop_tree(tmp_path)
|
||||
monkeypatch.setattr(cli_main, "PROJECT_ROOT", root)
|
||||
_make_packaged_executable(root, monkeypatch, platform="linux")
|
||||
# No packaged executable: the corrupt-download recovery must run.
|
||||
monkeypatch.setattr(cli_main.sys, "platform", "linux")
|
||||
monkeypatch.delenv("ELECTRON_MIRROR", raising=False)
|
||||
|
||||
install_ok = subprocess.CompletedProcess(["npm", "ci"], 0)
|
||||
|
|
@ -649,12 +669,49 @@ def test_gui_install_failure_hard_fails_when_electron_dist_exists(tmp_path, monk
|
|||
assert "Desktop dependency install failed" in capsys.readouterr().out
|
||||
|
||||
|
||||
def test_gui_does_not_retry_after_packaged_executable_exists(tmp_path, monkeypatch, capsys):
|
||||
"""A build that already produced a packaged executable did NOT fail from the
|
||||
Electron-download problem the cache purge + mirror retries exist to repair.
|
||||
|
||||
Regression for #40187: a late failure such as macOS code signing leaves
|
||||
Hermes.app/Contents/MacOS/Hermes in place. Re-downloading Electron can't
|
||||
repair a signing failure, so the destructive purge + slow mirror retry must
|
||||
be skipped — we fail directly instead of grinding through an identical retry.
|
||||
"""
|
||||
root = _make_desktop_tree(tmp_path)
|
||||
monkeypatch.setattr(cli_main, "PROJECT_ROOT", root)
|
||||
# Executable EXISTS at failure time → late failure, not a corrupt download.
|
||||
_make_packaged_executable(root, monkeypatch, platform="darwin")
|
||||
monkeypatch.delenv("ELECTRON_MIRROR", raising=False)
|
||||
|
||||
install_ok = subprocess.CompletedProcess(["npm", "ci"], 0)
|
||||
pack_fail = subprocess.CompletedProcess(["npm", "run", "pack"], 1)
|
||||
|
||||
with patch("hermes_cli.main.shutil.which", return_value="/usr/bin/npm"), \
|
||||
patch("hermes_cli.main._run_npm_install_deterministic", return_value=install_ok), \
|
||||
patch("hermes_cli.main._desktop_macos_relaunchable_fixup"), \
|
||||
patch("hermes_cli.main._purge_electron_build_cache", return_value=[Path("/c/electron.zip")]) as mock_purge, \
|
||||
patch("hermes_cli.main._redownload_electron_dist", return_value=True) as mock_dl, \
|
||||
patch("hermes_cli.main.subprocess.run", return_value=pack_fail) as mock_run, \
|
||||
pytest.raises(SystemExit) as exc:
|
||||
cli_main.cmd_gui(_ns())
|
||||
|
||||
assert exc.value.code == 1
|
||||
# Neither destructive recovery runs, and there is exactly ONE pack attempt.
|
||||
mock_purge.assert_not_called()
|
||||
mock_dl.assert_not_called()
|
||||
assert mock_run.call_count == 1
|
||||
assert "Desktop GUI build failed" in capsys.readouterr().out
|
||||
|
||||
|
||||
def test_gui_does_not_override_user_electron_mirror(tmp_path, monkeypatch, capsys):
|
||||
"""A user-pinned ELECTRON_MIRROR is respected: no extra mirror fallback
|
||||
attempt (and we never swap in our default mirror)."""
|
||||
root = _make_desktop_tree(tmp_path)
|
||||
monkeypatch.setattr(cli_main, "PROJECT_ROOT", root)
|
||||
_make_packaged_executable(root, monkeypatch, platform="linux")
|
||||
# No packaged executable: the build failure is the download-class the
|
||||
# mirror fallback handles (and we assert the user's pin is respected).
|
||||
monkeypatch.setattr(cli_main.sys, "platform", "linux")
|
||||
monkeypatch.setenv("ELECTRON_MIRROR", "https://mirror.example/electron/")
|
||||
|
||||
install_ok = subprocess.CompletedProcess(["npm", "ci"], 0)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue