diff --git a/hermes_cli/main.py b/hermes_cli/main.py index ac0bd41fdef..ab56d9986d6 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -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)") diff --git a/tests/hermes_cli/test_gui_command.py b/tests/hermes_cli/test_gui_command.py index c488944855f..728039aa81d 100644 --- a/tests/hermes_cli/test_gui_command.py +++ b/tests/hermes_cli/test_gui_command.py @@ -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)