Skip to content

fix(desktop): pre-fetch Electron dist after clean install on update (#47917) - #49152

Closed
Bartok9 wants to merge 1 commit into
NousResearch:mainfrom
Bartok9:fix/47917-desktop-update-electron-reinstall
Closed

fix(desktop): pre-fetch Electron dist after clean install on update (#47917)#49152
Bartok9 wants to merge 1 commit into
NousResearch:mainfrom
Bartok9:fix/47917-desktop-update-electron-reinstall

Conversation

@Bartok9

@Bartok9 Bartok9 commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes the residual behind the #47917 "it resurfaced" reports that continue after the dynamic-electronDist fix (#48091) — the The specified electronDist does not exist / "no local electron dist; will fetch" build failures and delays on every hermes update.

Root cause

This is not the path-resolution class that #48091 fixed (that's resolved). It's the download class, driven by the update model:

  • hermes update deliberately installs repo-root deps with --workspaces=false (see cmd_update in hermes_cli/main.py and the rebuild note in apps/bootstrap-installer/src-tauri/src/update.rs). It does not reinstall the apps/desktop workspace.
  • So electron's dist-fetching postinstall never runs on update, and node_modules/electron/dist stays empty even though the install "succeeded".
  • On the next hermes desktop --build-only rebuild, run-electron-builder.cjs correctly finds no local dist and falls through to a fresh @electron/get download every single time — a slow network round-trip, or a hard failure where GitHub's release host is throttled/blocked (the version churn 40.9.3 / 40.10.2 / 42.3.3 users see in their electron cache).

cmd_gui already self-heals after a pack fails (_purge_electron_build_cache + _redownload_electron_dist + mirror retry), and repairs on the npm ci failure branch. But on the common case — npm ci succeeds but leaves dist/ empty — it goes straight to pack with no proactive check, so every update pays the download tax.

The fix

One proactive pre-build check on the install-success path: when npm ci succeeds but electron is staged with an empty dist/ (_electron_pkg_staged_missing_dist), repopulate it via the existing _try_redownload_electron_dist (canonical host → mirror) before the first pack. Then the build reuses the local binary instead of racing the network.

Reuses the helpers introduced in #48091 — no new download machinery, just closes the timing gap.

elif not source_mode and _electron_pkg_staged_missing_dist(PROJECT_ROOT):
    if _try_redownload_electron_dist(PROJECT_ROOT, env):
        print("  ⚠ Electron dist was missing after install …; repopulated it before building.")
    else:
        print("  ⚠ Electron dist missing after install and could not be pre-fetched; continuing …")

Tests

tests/hermes_cli/test_gui_command.py:

  • test_gui_prefetches_electron_dist_after_clean_install_before_pack — asserts the repair runs before the first pack (ordering, not just "called"), failing without this change since the old code never pre-fetched on success.
  • test_gui_skips_electron_prefetch_when_dist_present — no redundant fetch when dist/ is healthy; straight to pack.
$ python3 -m pytest tests/hermes_cli/test_gui_command.py -k prefetch -v
tests/hermes_cli/test_gui_command.py::test_gui_prefetches_electron_dist_after_clean_install_before_pack PASSED
tests/hermes_cli/test_gui_command.py::test_gui_skips_electron_prefetch_when_dist_present PASSED

(38/38 non-pathspec tests in the file pass; the 5 content_hash/build_stamp_round_trip failures are a pre-existing missing-pathspec dep on the test env, unaffected by this change and failing identically on clean upstream/main.)

How to test manually

  1. Build the desktop once (hermes desktop --build-only) so it works.
  2. Empty the dist: rm -rf node_modules/electron/dist (simulates the post-update state).
  3. Re-run hermes desktop --build-only. Before: log shows no local electron dist; …will fetch and a slow/blocked download. After: dist is repopulated before the pack and the build reuses it.

Fixes #47917

…ousResearch#47917)

`hermes update` reinstalls repo-root deps with --workspaces=false, so the
desktop workspace is never reinstalled on update and electron's dist-fetching
postinstall never runs. node_modules/electron/dist is therefore empty after a
successful `npm ci`, and electron-builder silently falls through to a fresh
@electron/get download on every update ("no local electron dist; will fetch")
— a slow network round-trip, or a hard failure where the release host is
blocked. This is the residual behind the NousResearch#47917 'resurfaced' reports after the
dynamic-electronDist fix (NousResearch#48091).

cmd_gui already self-heals after a pack *fails*, but never proactively on the
install-success path. Add a pre-build check: when npm ci succeeds but electron
is staged with an empty dist/, repopulate it (canonical host, then mirror)
before the first pack so the build reuses the local binary.

Tests: pre-fetch runs before the first pack when dist/ is empty; is skipped
when dist/ is healthy.
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists labels Jun 19, 2026
@Bartok9

Bartok9 commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Independently verified on Windows by @wordgao (see #47917): on this branch, with node_modules/electron/dist deleted, hermes desktop --force-build now logs

⚠ Electron dist was missing after install (update reinstalls repo-root deps only); repopulated it before building.

and the pack completes (~10s, vite build + electron-builder) with no network download — exactly the intended behavior. Their first run with --build-only (no source change) correctly short-circuited on the content stamp, which is why --force-build is the repro that exercises the path.

CI is green across all checks (incl. desktop-build, Windows footguns (blocking), all 6 test shards, e2e).

@teknium1

Copy link
Copy Markdown
Contributor

Closing in favor of #48091 (fix(desktop): resolve electronDist dynamically + self-heal blocked installs, merged).

The path-resolution bug class behind #47917electronDist losing a coin-flip against npm workspace hoisting — is fixed on main: run-electron-builder.cjs now resolves the dist via require.resolve("electron/package.json") at build time, and the install paths self-heal a genuinely-missing dist (canonical host → npmmirror fallback) when npm ci fails.

#48091 made a deliberate design choice for the npm-ci-succeeded-but-dist-empty case: don't proactively pre-fetch — let electron-builder fetch the binary itself ([run-electron-builder] no local electron dist; electron-builder will fetch …). This PR adds a proactive pre-fetch before that path, which is an optimization on top of an intentional design rather than a missing fix.

The residual symptom in the thread (connect ETIMEDOUT … :443) is the Electron binary download itself being throttled/blocked, not a path bug — and the npmmirror fallback already covers that. Thanks for the thorough diagnosis and the live Windows validation, @Bartok9 — the root-cause writeup on the thread was genuinely useful.

@teknium1 teknium1 closed this Jun 26, 2026
@Bartok9

Bartok9 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Makes sense — agreed, this is an optimization on top of an intentional design, not a missing fix. Letting electron-builder own the fetch (with the npmmirror fallback covering the throttled/blocked ETIMEDOUT … :443 case) keeps the update path simpler, and the proactive pre-fetch isn't worth the extra surface area for that tradeoff. Thanks for the clear writeup on the boundary between the path-resolution class (#48091) and the download class — happy to leave this closed. 🙏

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 comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Desktop build fails after update - electronDist does not exist (cache invalidated)

3 participants