Skip to content

fix(update): backup desktop executable before rebuild to prevent loss on build failure#44238

Closed
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/desktop-update-exe-backup
Closed

fix(update): backup desktop executable before rebuild to prevent loss on build failure#44238
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/desktop-update-exe-backup

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Backs up the existing packaged desktop executable before hermes update triggers an Electron rebuild, and restores it if the build fails. This prevents the desktop shortcut from becoming a dead link when electron-builder's before-pack hook deletes the unpacked app directory and the subsequent build fails.

Related Issue

Fixes #44225

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • hermes_cli/main.py: Added backup/restore logic around the hermes desktop --build-only call in _cmd_update_impl(). Before the build, the existing unpacked app directory is copied to a .bak suffix. On build failure, the backup is restored. On success, the backup is cleaned up.
  • tests/hermes_cli/test_cmd_update.py: Added TestDesktopBuildBackupRestore class with 3 tests covering: build success (backup cleaned up), build failure (backup restored), and no existing executable (no backup attempted).

How to Test

  1. Run python -m pytest tests/hermes_cli/test_cmd_update.py::TestDesktopBuildBackupRestore -v
  2. All 3 tests should pass
  3. For manual testing on Windows: install Hermes Desktop, run hermes update with a simulated build failure (e.g., corrupt Electron cache), verify the desktop executable is preserved

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Code Intelligence

  • Analyzed: _cmd_update_impl (callers: cmd_update), _desktop_packaged_executable (callers: 5)
  • Blast radius: LOW — only affects the desktop rebuild path during hermes update
  • Related patterns: before-pack.cjs cleanStaleAppOutDir() deletes unpacked dir; this fix adds a Python-side safety net

… on build failure

electron-builder's before-pack hook deletes the unpacked app directory
before staging new binaries. If the build fails (network, cache corruption,
etc.), the existing Hermes.exe is lost and desktop shortcuts become dead
links.

This fix backs up the existing packaged executable directory before the
build and restores it on failure. On success, the backup is cleaned up.
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have labels Jun 11, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related: competing fix for #44225 alongside #44234. This PR backs up via a sibling .bak rename; #44234 uses a release/.rebuild-backup/ holder dir and notes a sibling rename can be purged by the retry-cache cleanup. Same goal, different mechanism.

@liuhao1024

Copy link
Copy Markdown
Contributor Author

Thanks for the note @alt-glitch. Comparing the two approaches: #44234 uses a release/.rebuild-backup/ holder directory with +258 lines of implementation, while this PR uses a sibling .bak rename with +86 lines. #44234's approach is more robust against retry-cache cleanup purging the backup. Closing in favor of #44234.

@liuhao1024 liuhao1024 closed this Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hermes update destroys Hermes Desktop executable on failed Electron rebuild (desktop shortcut becomes dead link)

2 participants