fix: make macOS desktop self-update swap+relaunch fail-fast and recoverable - #134
Merged
Conversation
…erable The macOS in-app updater's detached swap script ran with plain 'set -u' and a success-gated ditto: when ditto or the destination move failed, the script fell through silently -- the app had already quit, the old bundle stayed (or was left moved aside), and 'open "$DST"' either relaunched the stale build or nothing at all. This matches the reported "Update now does not actually update" behavior. Make the swap fail-fast and recoverable: - set -euo pipefail so unexpected failures stop the script instead of compounding. - Detect ditto and destination-replace failures explicitly; on either, fall back to opening the freshly rebuilt bundle directly so the user is never left with a dead quit. - Clean up the .hermes-update-old copy via an EXIT trap so it is removed on every exit path. Recovered from the pre-force-push head of desktop-mac-swap-fix (NousResearch#38410, which lost this work to a force-push and was closed). The sqlite busy_timeout half of that PR is intentionally not revived: the BEGIN IMMEDIATE + jitter-retry redesign (NousResearch#3385) deliberately keeps the connection busy handler short, and a 5s busy_timeout would override that design. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🔎 Lint report:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Revives the work from NousResearch#38410, which was closed after an old force-push left its branch without the changes its description promised (the branch's only remaining commit was a subset of the NousResearch#38446 codesign re-seal). The original commit was recovered from the fork's pre-force-push branch head via the repo activity log.
The macOS in-app updater can quit Hermes.app and never reinstall/reopen: the detached swap script gated only
dittoon success and fell through silently when the bundle swap failed, leaving the user on the old build — or with no app at all — after a dead quit.What changed
apps/desktop/electron/main.cjs: theapplyUpdatesPosixInAppswap script now runsset -euo pipefail, detectsdittoand destination-replace failures immediately, cleans up the.hermes-update-oldstaging copy via an EXIT trap, and on any swap failure falls back to opening the freshly rebuilt bundle directly instead of leaving the user with a dead quit.Deliberately not revived from the original PR:
hermes_state.pyPRAGMA busy_timeout = 5000— superseded by the BEGIN IMMEDIATE + jitter-retry redesign (fix: SQLite WAL write-lock contention causing 15-20s TUI freeze NousResearch/hermes-agent#3385), which intentionally keeps the SQLite busy handler short (timeout=1.0) and handles contention with application-level retries. A 5s busy_timeout would override that design.9cf5343ae, "harden relaunch and wait loop") — it regressed the fail-fast invariant (droppedset -eand the failure guards, so a faileddittowould have deleted the app with no replacement) and stripped load-bearing comments. It reads as an abandoned experiment, not part of the reviewed fix.How to review
Review the
swapScripttemplate inapplyUpdatesPosixInApp(apps/desktop/electron/main.cjs, ~line 1815). The invariant: on ANY swap failure the user ends up with a working app launched (the rebuilt bundle via theopen "$SRC"fallback) and no orphaned.hermes-update-*staging copies.Evidence
Functional simulation of the rendered swap script with
/usr/bin/openand/usr/bin/xattrstubbed:ditto-failure path (source missing): destination untouched (old bundle intact),[updates] ditto failedlogged to stderr, fallback open targets the rebuilt source bundle, no leftovers, rc=0.Verification
node --check apps/desktop/electron/main.cjsbash -non the rendered swap scriptRisks & gaps
rm -rfof it also fails,set -eaborts with the original app intact but no relaunch (the user reopens manually) — strictly better than the prior silent fall-through.open "$SRC"fallback launches the rebuilt bundle from the build tree rather than/Applications; this avoids a no-app state but may not match the user's preferred installed location (same trade-off as reviewed in the original PR).