fix(desktop): preserve the packaged app when an Electron rebuild fails

[AIalliAI]

Fixes #44225

Problem

hermes update (including the desktop GUI's Update Now) rebuilds the desktop app via hermes desktop --build-only after git pull. electron-builder's beforePack hook (apps/desktop/scripts/before-pack.cjs) wipes appOutDir — e.g. release/win-unpacked/ containing the live Hermes.exe — before staging, and the Electron download/extract runs after that wipe. So any pack failure (corrupt cached Electron zip, blocked/timed-out download) destroys the only executable the user has: the desktop shortcut becomes a dead link and the app is gone until a manual rebuild succeeds.

The CLI's existing retry ladder (cache purge → retry → mirror fallback) reduces how often the pack fails, but restores nothing when all retries fail.

Fix

In the packaged-build path of _cmd_desktop (so it covers hermes update, GUI-triggered updates, the installer's headless --update rebuild, and manual hermes desktop --force-build alike):

• Before the pack: move the current unpacked app aside with a cheap same-volume rename into release/.rebuild-backup/<name> (_backup_desktop_unpacked_app). The beforePack hook's stale-dir cleanup semantics are preserved — it simply finds a clean tree.
• On final failure (after all existing retries): restore the parked copy over any partial tree, so the existing install and its shortcuts keep working (_restore_desktop_unpacked_app), then fail with the same exit code as before.
• On success: discard the backup. If a zero-exit pack somehow produced no launchable app, restore instead of discarding.

The holder directory is deliberately not a sibling rename: _purge_electron_build_cache rmtree's release/*-unpacked between retries and _desktop_packaged_executable globs mac*, so a sibling name would either get purged mid-retry or be mistaken for the freshly-built app. There's a regression test pinning each of those interactions.

All helpers are best-effort and never raise; if the backup rename fails, behavior is exactly what it was before this change.

This is the issue's proposed option A/B, but placed in the CLI pack path rather than _cmd_update_impl (option A would miss manual rebuilds) or before-pack.cjs (electron-builder has no on-failure hook to drive a restore from).

Tests

8 new tests in tests/hermes_cli/test_gui_command.py: backup/restore unit coverage (including the macOS Hermes.app → appOutDir resolution), the purge-glob and detection-glob non-interference pins, and end-to-end cmd_gui runs for fail-restores / success-discards / zero-exit-without-artifact-restores.

tests/hermes_cli/test_gui_command.py (36) and tests/hermes_cli/test_cmd_update.py (29) all pass.

[liuhao1024]

Code Review — looks solid

The backup-before-pack approach is well-designed. A few things verified:

1. Backup location avoids purge glob: release/.rebuild-backup/ sits under release/ but doesn't match release/*-unpacked, so _purge_electron_build_cache rmtree won't destroy it. Confirmed.

2. Backup doesn't masquerade as packaged app: The macOS mac* glob for _desktop_packaged_executable won't match .rebuild-backup/ since it doesn't start with mac. Confirmed.

3. Race between backup and pack: _backup_desktop_unpacked_app runs synchronously before subprocess.run([npm, "run", "pack"]), so the backup is complete before the pack wipes appOutDir. No race.

4. Win32 lock handling: The backup rename happens after _stop_desktop_processes_locking_build, so the Hermes.exe file handle is released before the rename. Correct order.

5. Best-effort semantics: All three helper functions (_backup, _restore, _discard) catch OSError and return None/False — the build proceeds exactly as before if backup fails. Good.

6. Edge case: pack succeeds but produces no artifact: The post-build check _desktop_packaged_executable(desktop_dir) is None correctly restores the backup. This handles misconfigured targets.

One minor note: _restore_desktop_unpacked_app does shutil.rmtree(original) then backup.rename(original). If the rmtree succeeds but the rename fails (e.g., disk full), the backup is still intact at its .rebuild-backup/ path — the user would have lost the partial build output but can manually recover. Acceptable for best-effort.

[alt-glitch]

Related: competing fix for #44225 alongside #44238. This PR parks the unpacked app in a release/.rebuild-backup/ holder dir (deliberately not a sibling .bak rename, which the retry-purge / mac-glob paths would clobber); #44238 uses the sibling .bak rename. Same goal, different mechanism — maintainer to pick one.

[AIalliAI]

Requesting maintainer review — this is ready to land from my side. Standalone fork CI is pending first-run approval here; the rollup branch in #44061 carrying this session's batch is fully green on upstream CI (all test shards, typecheck, e2e).

[teknium1]

Thanks for the focused recovery path. Current main remains vulnerable: apps/desktop/scripts/before-pack.mjs:75 removes the live unpacked app before staging, while hermes_cli/main.py:5738-5793 invokes the packaged build and exits on final failure without restoration. The PR's backup outside the release/*-unpacked purge scope directly addresses that path and includes targeted regression coverage in tests/hermes_cli/test_gui_command.py (commit a59cc9111c7d).

Automated hermes-sweeper review.

[andrexibiza]

Current-main successor/interlock: #91079 preserves this PR's rollback design and adds the missing builder-side settlement owner.

The newer slice keeps the original pre-build generation authoritative across multi-target beforePack calls, restores it on builder failure or false success, and discards it only after structural PE verification. The #91079 body and #44225 update preserve @AIalliAI's authorship and treat this PR as the design/provenance source rather than erasing it.

Exact successor head: 8e641c78f1073321a79195b62c105982f6a01240, with CI, Docker, and Nix green on that exact object.