From c1f9eb0ec4b99b22346fbd5054991a976d2d39e7 Mon Sep 17 00:00:00 2001 From: brooklyn! Date: Wed, 17 Jun 2026 18:48:35 -0500 Subject: [PATCH] fix(desktop): resolve electronDist dynamically + self-heal blocked installs (supersedes #48081/#48082) (#48091) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(desktop): resolve electronDist dynamically + self-heal blocked installs Supersedes the static-path approach (#48081) and the install-step self-heal (#48082) with a fix that removes the whole failure class instead of chasing each symptom. Three distinct faults converged into the June desktop-build outage; this closes all three. Root cause (the part #48081 left open — "Gap B"): build.electronDist was a static relative path in apps/desktop/package.json, but npm workspace hoisting is NOT deterministic — depending on the npm version and what else is installed, npm nests the workspace-only electron devDep under apps/desktop/node_modules/electron OR hoists it to the repo root. A static path matches only one layout, so a clean install intermittently fails with "The specified electronDist does not exist". #48081 re-pointed the path at the nested layout (correct today) but electron-builder reads electronDist STATICALLY, so any future hoist change silently breaks it again — only caught by a CI invariant, never self-corrected. Fix: - scripts/run-electron-builder.cjs: resolve electron the way Node's runtime does — require.resolve("electron/package.json") walks node_modules from the desktop project upward and finds electron wherever npm actually put it. The path can never drift out of sync with the install layout again, on any OS/npm version. * dist present -> pass -c.electronDist=/dist so electron-builder reuses the unpacked runtime (keeps the #38673 fast path that dodges the 26.8.x missing-binary re-unpack bug). * dist absent -> omit electronDist; electron-builder fetches Electron itself via @electron/get honoring electronVersion + ELECTRON_MIRROR. package.json: builder script now runs the wrapper; the static build.electronDist is removed (the resolver owns it). - main.py / install.sh / install.ps1: on a dependency-install failure where the electron package staged but its dist is missing (electron's install.js process.exit(1) on a blocked/throttled binary download — #47266/#47917/#48021), repopulate the dist via electron's downloader (canonical, then npmmirror.com) and CONTINUE to the build instead of aborting. npm runs postinstall LAST, so the only casualty is electron/dist; bailing here is what made the pack-time mirror self-heal unreachable on a blocked network. Hard-fail only when electron never staged at all (a genuine dependency error). - The pack-time mirror fallback now retries the build even when the pre-fetch can't populate the dist: the wrapper lets electron-builder download Electron itself via the mirror, so the retry is no longer a no-op (it was, when electronDist was a static path). The exact 40.10.2 pin (already on main) keeps the third mode — the native @electron-internal/extract-zip win32 binding that 40.10.3/40.10.4 ship without a published prebuild — from recurring. Tests: - test_desktop_electron_pin.py: replace the static-path-matches-lockfile invariant with contracts that there is no hardcoded electronDist to drift, the builder script routes through the resolver, and the resolver uses Node module resolution + injects -c.electronDist. - test_gui_command.py: install-failure self-heal continues to build; genuine (electron-never-staged) install failure still hard-fails; pack retries under the mirror even when the pre-fetch is blocked. Salvages/supersedes the overlapping community work in #48003 (sitkarev), #48012 (omegazheng), #48033 (james47kjv), and #48082. Co-authored-by: sitkarev <59806492+sitkarev@users.noreply.github.com> Co-authored-by: omegazheng Co-authored-by: james47kjv <220877172+james47kjv@users.noreply.github.com> * fix(desktop): narrow Electron self-heal to real missing-dist failures Follow-up on #48091 to remove the remaining misdiagnosis risk from the installer/build fallback path (#46785 concern): only take the Electron repair/retry path when Electron's package files are staged and dist is actually missing/corrupt. - main.py: add _electron_pkg_staged_missing_dist() and use it to gate install failure recovery; fail fast for unrelated npm install errors. - main.py/install.sh/install.ps1: run cache purge + retry only when dist is missing; do not retry unrelated tsc/vite/build failures under an Electron-specific narrative. - install.sh/install.ps1: tighten install-stage self-heal guard to require both package.json + install.js and missing dist. - tests: add coverage that install failure hard-fails when Electron dist already exists, and update retry test to reflect the tightened recovery condition. Validation: - Python tests: 64 passed - install.sh-related tests included in the run - Real mac build on this machine: - npm ci at repo root: success - cd apps/desktop && npm run pack: success - electron-builder packaged darwin arm64 and used custom unpacked Electron dist * refactor(desktop): trim electron self-heal helpers and comments Deduplicate mirror-retry into _try_redownload_electron_dist / shell counterparts; shorten wrapper and install-script commentary without changing recovery semantics. --------- Co-authored-by: sitkarev <59806492+sitkarev@users.noreply.github.com> Co-authored-by: omegazheng Co-authored-by: james47kjv <220877172+james47kjv@users.noreply.github.com> --- apps/desktop/package.json | 3 +- apps/desktop/scripts/run-electron-builder.cjs | 57 +++++++++ hermes_cli/main.py | 116 ++++++++---------- scripts/install.ps1 | 109 +++++++--------- scripts/install.sh | 97 +++++---------- tests/hermes_cli/test_gui_command.py | 107 ++++++++++++++-- tests/test_desktop_electron_pin.py | 68 +++++----- 7 files changed, 310 insertions(+), 247 deletions(-) create mode 100644 apps/desktop/scripts/run-electron-builder.cjs diff --git a/apps/desktop/package.json b/apps/desktop/package.json index 7935e2611fb..cff1877a1ba 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -21,7 +21,7 @@ "build": "node scripts/assert-root-install.cjs && node scripts/write-build-stamp.cjs && node scripts/stage-native-deps.cjs && tsc -b && vite build && npm run postbuild", "postbuild": "node scripts/assert-dist-built.cjs", "prebuilder": "node scripts/patch-electron-builder-mac-binary.cjs", - "builder": "cross-env NODE_OPTIONS=--max-old-space-size=16384 electron-builder", + "builder": "cross-env NODE_OPTIONS=--max-old-space-size=16384 node scripts/run-electron-builder.cjs", "pack": "npm run build && npm run builder -- --dir", "dist": "npm run build && npm run builder", "dist:mac": "npm run build && npm run builder -- --mac", @@ -135,7 +135,6 @@ }, "build": { "electronVersion": "40.10.2", - "electronDist": "node_modules/electron/dist", "appId": "com.nousresearch.hermes", "productName": "Hermes", "executableName": "Hermes", diff --git a/apps/desktop/scripts/run-electron-builder.cjs b/apps/desktop/scripts/run-electron-builder.cjs new file mode 100644 index 00000000000..100d6c346e9 --- /dev/null +++ b/apps/desktop/scripts/run-electron-builder.cjs @@ -0,0 +1,57 @@ +"use strict" + +// Resolve electronDist at runtime (#38673, #47917): electron-builder 26.8.x can +// re-unpack a broken Electron.app; reusing the installed dist dodges that. +// npm workspace hoisting is non-deterministic — require.resolve finds electron +// wherever it landed. Dist present → -c.electronDist=/dist; absent → let +// electron-builder fetch via @electron/get (electronVersion + ELECTRON_MIRROR). + +const fs = require("node:fs") +const path = require("node:path") +const { spawnSync } = require("node:child_process") + +function electronDistDir() { + try { + return path.join(path.dirname(require.resolve("electron/package.json")), "dist") + } catch { + return null + } +} + +function distBinary(dist) { + if (process.platform === "darwin") { + return path.join(dist, "Electron.app", "Contents", "MacOS", "Electron") + } + if (process.platform === "win32") { + return path.join(dist, "electron.exe") + } + return path.join(dist, "electron") +} + +function electronBuilderCli() { + const pkgJson = require.resolve("electron-builder/package.json") + const bin = require(pkgJson).bin + const rel = typeof bin === "string" ? bin : bin["electron-builder"] + return path.join(path.dirname(pkgJson), rel) +} + +const dist = electronDistDir() +const args = [] +if (dist && fs.existsSync(distBinary(dist))) { + args.push(`-c.electronDist=${dist}`) +} else { + console.warn( + "[run-electron-builder] no local electron dist; electron-builder will fetch " + + "via @electron/get (electronVersion + ELECTRON_MIRROR)." + ) +} +args.push(...process.argv.slice(2)) + +const result = spawnSync(process.execPath, [electronBuilderCli(), ...args], { + stdio: "inherit", +}) +if (result.error) { + console.error(`[run-electron-builder] spawn failed: ${result.error.message}`) + process.exit(1) +} +process.exit(result.status == null ? 1 : result.status) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index e9f77945f1e..7ec89a41412 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -5110,6 +5110,11 @@ def _purge_electron_build_cache(desktop_dir: Path) -> list[Path]: return removed +# Last-resort Electron mirror after GitHub download fails (#47266). Only used +# when the user hasn't pinned ELECTRON_MIRROR. +_ELECTRON_FALLBACK_MIRROR = "https://npmmirror.com/mirrors/electron/" + + def _electron_dir(project_root: Path) -> Path: """Return the Electron package directory the desktop workspace installs. @@ -5158,31 +5163,23 @@ def _electron_dist_ok(project_root: Path) -> bool: return False +def _electron_pkg_staged_missing_dist(project_root: Path) -> bool: + """electron staged (package.json + install.js) but dist missing — blocked postinstall.""" + electron_dir = _electron_dir(project_root) + return ( + (electron_dir / "package.json").is_file() + and (electron_dir / "install.js").is_file() + and not _electron_dist_ok(project_root) + ) + + def _redownload_electron_dist( project_root: Path, env: dict, *, mirror: Optional[str] = None, ) -> bool: - """(Re)populate ``node_modules/electron/dist`` via electron's own downloader. - - Since #38673 the desktop build pins ``build.electronDist`` to - ``node_modules/electron/dist``, so electron-builder reads the Electron binary - straight from there and never downloads it during ``npm run pack``. That dist - tree is produced by the ``electron`` package's postinstall (``install.js``) - during ``npm ci``. When that download is blocked or throttled (GitHub's - release host is unreachable in some regions — #47266), the dist is missing - and re-running ``pack`` only re-throws "The specified electronDist does not - exist". The mirror fallback therefore has to drive *this* downloader, not - another ``pack``. - - No-op (returns True) when the dist binary is already present, so an unrelated - build failure doesn't trigger a needless ~200 MB re-download. Otherwise drops - any partial dist + version marker (electron's install.js short-circuits when - ``path.txt`` already matches) and runs the downloader once, optionally via a - mirror. Best-effort: never raises. Returns True iff the dist binary exists - afterward. - """ + """Best-effort: run electron's install.js to populate dist/ (optional mirror).""" if _electron_dist_ok(project_root): return True @@ -5211,6 +5208,15 @@ def _redownload_electron_dist( return _electron_dist_ok(project_root) +def _try_redownload_electron_dist(project_root: Path, env: dict) -> bool: + """Canonical download, then fallback mirror unless the user pinned one.""" + if _redownload_electron_dist(project_root, env): + return True + if env.get("ELECTRON_MIRROR"): + return False + return _redownload_electron_dist(project_root, env, mirror=_ELECTRON_FALLBACK_MIRROR) + + def _stop_desktop_processes_locking_build(desktop_dir: Path) -> list[int]: """Terminate any running desktop app executing from this build's ``release`` dir so a rebuild can replace its (otherwise locked) executable. @@ -5433,9 +5439,18 @@ def cmd_gui(args: argparse.Namespace): nixos_env = _nixos_build_env() install_result = _run_npm_install_deterministic(npm, PROJECT_ROOT, capture_output=False, env=nixos_env) if install_result.returncode != 0: - print("✗ Desktop dependency install failed") - print(f" Run manually: cd {PROJECT_ROOT} && npm ci") - sys.exit(install_result.returncode or 1) + if not _electron_pkg_staged_missing_dist(PROJECT_ROOT): + print("✗ Desktop dependency install failed") + print(f" Run manually: cd {PROJECT_ROOT} && npm ci") + sys.exit(install_result.returncode or 1) + repaired = _try_redownload_electron_dist(PROJECT_ROOT, env) + if repaired: + print(" ⚠ Dependency install failed with a missing Electron dist; " + "repopulated it and continuing.") + else: + print(" ⚠ Dependency install failed with a missing Electron dist; " + "continuing to the build so electron-builder can attempt " + "the Electron fetch itself.") build_label = "source build" if source_mode else "packaged app" print(f"→ Building desktop {build_label}...") @@ -5451,31 +5466,15 @@ def cmd_gui(args: argparse.Namespace): 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: - # A corrupt cached Electron zip makes `pack` fail with an ENOENT - # on the final `electron` -> `Hermes` rename: unpack-electron - # extracted a partial tree (missing the 193 MB binary) from the - # bad zip. We do NOT try to prove the zip is corrupt ourselves — - # stdlib zipfile silently tolerates the prepended/concatenated - # junk that is the most common corruption (a partial download - # resumed into the same file), so a `testzip()` gate would pass - # and never self-heal. Instead, on any packaged-build failure we - # purge the version's cached zip + the half-written unpacked dir - # and retry once: @electron/get re-downloads with its own SHASUM - # verification, which is the real source of truth. If the - # failure was something else, the clean re-download is harmless - # and the retry fails the same way. - purged = _purge_electron_build_cache(desktop_dir) - # electronDist is pinned to node_modules/electron/dist (#38673): - # electron-builder reads the Electron binary from there and `pack` - # never downloads it, so purging the cache + re-running pack can't - # by itself repopulate a missing/partial dist. When the dist is - # actually gone, re-run electron's own downloader so the retry has - # a binary to read. Gated on the dist check so an unrelated build - # failure (tsc/vite) doesn't trigger a pointless ~200 MB refetch. + # 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. + purged: list[Path] = [] restored = False if not _electron_dist_ok(PROJECT_ROOT): + purged = _purge_electron_build_cache(desktop_dir) restored = _redownload_electron_dist(PROJECT_ROOT, env) - if purged or restored: + if restored: print(" ⚠ Desktop build failed; refreshed the Electron download and retrying once...") for p in purged: print(f" - {p}") @@ -5484,35 +5483,16 @@ def cmd_gui(args: argparse.Namespace): _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"): - # Still failing and the user hasn't pinned a mirror: GitHub's - # Electron release host is likely blocked/throttled (the repeating - # "retrying" download log). Retry once via npmmirror.com — the - # de-facto Electron community mirror (Alibaba). @electron/get - # SHASUM-checks the download, but the SHASUMS come from the same - # mirror, so that guards against a corrupt/partial download, NOT - # a compromised mirror: reaching for it is an explicit trust - # trade-off we only make AFTER the canonical GitHub download has - # failed, and we never override a user-pinned ELECTRON_MIRROR. 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)") - mirror = "https://npmmirror.com/mirrors/electron/" + mirror = _ELECTRON_FALLBACK_MIRROR mirror_env = dict(env) mirror_env["ELECTRON_MIRROR"] = mirror - # electronDist is pinned (#38673), so `npm run pack` never - # downloads Electron — the mirror only helps if it drives - # electron's own downloader. Re-fetch the binary through the - # mirror first; otherwise the retry just re-reads the same missing - # dist and re-throws "electronDist does not exist" (#47266). - have_dist = _electron_dist_ok(PROJECT_ROOT) - if not have_dist: - have_dist = _redownload_electron_dist(PROJECT_ROOT, env, mirror=mirror) - if have_dist: - _stop_desktop_processes_locking_build(desktop_dir) - build_result = subprocess.run([npm, "run", build_script], cwd=desktop_dir, env=mirror_env, check=False) - else: - print(" ✗ Could not re-download Electron from the mirror " - "(node_modules/electron/dist still missing)") + if not _electron_dist_ok(PROJECT_ROOT): + _redownload_electron_dist(PROJECT_ROOT, env, mirror=mirror) + _stop_desktop_processes_locking_build(desktop_dir) + build_result = subprocess.run([npm, "run", build_script], cwd=desktop_dir, env=mirror_env, check=False) if build_result.returncode != 0: print("✗ Desktop GUI build failed") print(f" Run manually: cd apps/desktop && npm run {build_script}") diff --git a/scripts/install.ps1 b/scripts/install.ps1 index 0f04f2826cc..58f136207b2 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -2161,12 +2161,10 @@ function Clear-ElectronBuildCache { return $removed } -# Return the Electron package directory the desktop workspace installs. npm may -# nest workspace-only dev dependencies under apps\desktop\node_modules instead -# of hoisting them to the repo root; which layout you get depends on the npm -# version and what else is installed. apps\desktop\package.json points -# electron-builder's electronDist there, so prefer the workspace-local package -# and fall back to the root hoist. +# Last-resort Electron mirror after GitHub download fails (#47266). +$script:DesktopElectronFallbackMirror = "https://npmmirror.com/mirrors/electron/" + +# Electron package dir — workspace-local nest first, then root hoist. function Get-ElectronDir { param([string]$InstallDir) $desktopLocal = Join-Path $InstallDir 'apps\desktop\node_modules\electron' @@ -2174,11 +2172,7 @@ function Get-ElectronDir { return (Join-Path $InstallDir 'node_modules\electron') } -# True when the desktop workspace electronDist holds a usable Electron binary. -# electron-builder reads the binary from build.electronDist since #38673, so -# this is the exact file whose absence makes a pack fail with "The specified -# electronDist does not exist". A dist dir that exists but is missing -# electron.exe (partial extraction / aborted postinstall) is NOT ok. +# True when dist/ holds a usable Electron binary (#38673 / run-electron-builder.cjs). function Test-ElectronDist { param([string]$InstallDir) $electronDir = Get-ElectronDir -InstallDir $InstallDir @@ -2186,22 +2180,7 @@ function Test-ElectronDist { return (Test-Path -LiteralPath $distExe) } -# (Re)populate the desktop Electron dist via electron's own downloader. -# -# Since #38673 the desktop build pins build.electronDist, so electron-builder -# reads the Electron binary straight from there and never downloads it during -# `npm run pack`. That dist tree is produced by the electron package's -# postinstall (install.js) during `npm ci`. When that download is -# blocked/throttled (GitHub's release host is unreachable in some regions - -# #47266), dist is missing and re-running pack only re-throws "The specified -# electronDist does not exist". The mirror fallback therefore has to drive THIS -# downloader, not another pack. -# -# No-op (returns $true) when the dist binary is already present. Otherwise drops a -# partial dist + version marker (electron's install.js short-circuits when -# path.txt already matches) and runs the downloader once, optionally via a -# mirror. Best-effort: never throws. Returns $true iff the dist binary exists -# afterward. +# Best-effort: run electron/install.js to populate dist/ (optional mirror). function Restore-ElectronDist { param([string]$InstallDir, [string]$Mirror) if (Test-ElectronDist -InstallDir $InstallDir) { return $true } @@ -2234,6 +2213,23 @@ function Restore-ElectronDist { return (Test-Path -LiteralPath $distExe) } +function Test-ElectronPkgStagedMissingDist { + param([string]$InstallDir) + $electronDir = Get-ElectronDir -InstallDir $InstallDir + return ( + (Test-Path -LiteralPath (Join-Path $electronDir 'package.json')) -and + (Test-Path -LiteralPath (Join-Path $electronDir 'install.js')) -and + (-not (Test-ElectronDist -InstallDir $InstallDir)) + ) +} + +function Try-RestoreElectronDist { + param([string]$InstallDir) + if (Restore-ElectronDist -InstallDir $InstallDir) { return $true } + if ($env:ELECTRON_MIRROR) { return $false } + return Restore-ElectronDist -InstallDir $InstallDir -Mirror $script:DesktopElectronFallbackMirror +} + function Install-Desktop { # Build apps/desktop into a launchable Hermes.exe. Only called from # Stage-Desktop, which is itself only included in the manifest when @@ -2329,10 +2325,16 @@ function Install-Desktop { } $ErrorActionPreference = $prevEAP if ($code -ne 0) { - Show-NpmCertHint ($npmOut -join "`n") | Out-Null - throw "desktop workspace npm install failed (exit $code) -- see lines above for cause" + if (Test-ElectronPkgStagedMissingDist -InstallDir $InstallDir) { + Write-Warn "Desktop dependency install failed with a missing Electron dist; attempting self-heal..." + Try-RestoreElectronDist -InstallDir $InstallDir | Out-Null + } else { + Show-NpmCertHint ($npmOut -join "`n") | Out-Null + throw "desktop workspace npm install failed (exit $code) -- see lines above for cause" + } + } else { + Write-Success "Desktop workspace dependencies installed" } - Write-Success "Desktop workspace dependencies installed" } catch { if ($prevEAP) { $ErrorActionPreference = $prevEAP } Pop-Location @@ -2375,57 +2377,34 @@ function Install-Desktop { & $npmExe run pack 2>&1 | ForEach-Object { "$_" } | Tee-Object -FilePath $buildLog $code = $LASTEXITCODE if ($code -ne 0) { - # A corrupt cached Electron zip makes `pack` fail with an opaque - # ENOENT on the final `electron` -> `Hermes` rename: app-builder's - # unpack-electron extracted a partial tree (missing the binary) from - # the bad zip, and re-running reuses the poisoned cache forever. - # Purge the cached download + any stale unpacked output and retry - # once; @electron/get re-downloads with its own SHASUM check. Without - # this a corrupt download hard-fails the whole installer. - $purged = @(Clear-ElectronBuildCache -DesktopDir $desktopDir) - # electronDist is pinned to node_modules\electron\dist (#38673): - # electron-builder reads the Electron binary from there and `pack` - # never downloads it, so purging the cache + re-running pack can't by - # itself repopulate a missing/partial dist. When the dist is actually - # gone, re-run electron's own downloader so the retry has a binary to - # read. Gated on the dist check so an unrelated build failure - # (tsc/vite) doesn't trigger a pointless ~200MB refetch. + $purged = @() $restored = $false if (-not (Test-ElectronDist -InstallDir $InstallDir)) { + $purged = @(Clear-ElectronBuildCache -DesktopDir $desktopDir) $restored = Restore-ElectronDist -InstallDir $InstallDir } - if ($purged.Count -gt 0 -or $restored) { + if ($restored) { Write-Warn "Desktop build failed - refreshed the Electron download, retrying once:" foreach ($p in $purged) { Write-Info " - $p" } & $npmExe run pack 2>&1 | ForEach-Object { "$_" } | Tee-Object -FilePath $buildLog $code = $LASTEXITCODE } } - # Still failing and the user hasn't pinned their own mirror: GitHub's - # Electron release host is likely blocked/throttled (the repeating - # "retrying" log). Retry once via npmmirror.com — the de-facto Electron - # community mirror (Alibaba). @electron/get SHASUM-checks the download, - # but the SHASUMS come from the same mirror, so that guards against a - # corrupt/partial download, NOT a compromised mirror: an explicit trust - # trade-off we only make AFTER the canonical GitHub download has failed, - # and we never override a user-pinned ELECTRON_MIRROR. if ($code -ne 0 -and -not $env:ELECTRON_MIRROR) { - $mirror = "https://npmmirror.com/mirrors/electron/" + $mirror = $script:DesktopElectronFallbackMirror Write-Warn "Desktop build still failing - the Electron download from GitHub looks blocked." Write-Warn "Re-downloading Electron via a public mirror ($mirror), then rebuilding:" Write-Info " (set ELECTRON_MIRROR yourself to use a different/trusted mirror)" - # electronDist is pinned (#38673), so `npm run pack` never downloads - # Electron - the mirror only helps if it drives electron's own - # downloader. Re-fetch the binary through the mirror first; otherwise - # the retry just re-reads the same missing dist and re-throws - # "The specified electronDist does not exist" (#47266). - $haveDist = Test-ElectronDist -InstallDir $InstallDir - if (-not $haveDist) { $haveDist = Restore-ElectronDist -InstallDir $InstallDir -Mirror $mirror } - if ($haveDist) { + if (-not (Test-ElectronDist -InstallDir $InstallDir)) { + Restore-ElectronDist -InstallDir $InstallDir -Mirror $mirror | Out-Null + } + $prevMirror = $env:ELECTRON_MIRROR + $env:ELECTRON_MIRROR = $mirror + try { & $npmExe run pack 2>&1 | ForEach-Object { "$_" } | Tee-Object -FilePath $buildLog $code = $LASTEXITCODE - } else { - Write-Warn "Could not re-download Electron from the mirror (node_modules\electron\dist still missing)" + } finally { + $env:ELECTRON_MIRROR = $prevMirror } } $ErrorActionPreference = $prevEAP diff --git a/scripts/install.sh b/scripts/install.sh index 0703d245144..2ae81d4bc21 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -2398,21 +2398,10 @@ _desktop_pack() { fi } -# Public Electron mirror used as a last-resort fallback when GitHub's release -# host is blocked/throttled (the repeating "retrying" symptom). npmmirror.com is -# the de-facto Electron community mirror (Alibaba). @electron/get SHASUM-checks -# the download, but the SHASUMS come from the same mirror — that guards against a -# corrupt/partial download, NOT a compromised mirror. Reaching for it is an -# explicit trust trade-off we only make AFTER the canonical GitHub download has -# failed, and we never override a user-pinned ELECTRON_MIRROR. +# Last-resort Electron mirror after GitHub download fails (#47266). DESKTOP_ELECTRON_FALLBACK_MIRROR="https://npmmirror.com/mirrors/electron/" -# Return the Electron package directory the desktop workspace installs. npm may -# nest workspace-only dev dependencies under apps/desktop/node_modules instead -# of hoisting them to the repo root; which layout you get depends on the npm -# version and what else is installed. apps/desktop/package.json points -# electron-builder's electronDist there, so prefer the workspace-local package -# and fall back to the root hoist. $1 = the workspace root holding node_modules. +# Electron package dir — workspace-local nest first, then root hoist. _electron_dir() { local install_dir="$1" if [ -d "$install_dir/apps/desktop/node_modules/electron" ]; then @@ -2422,12 +2411,7 @@ _electron_dir() { fi } -# True (returns 0) when the desktop workspace electronDist holds a usable -# Electron binary. electron-builder reads the binary from build.electronDist -# since #38673, so this is the exact file whose absence makes a pack fail with -# "The specified electronDist does not exist". A dist dir that exists but is -# missing the binary (partial extraction / aborted postinstall) is NOT ok. -# $1 = the workspace root holding node_modules. +# True when dist/ holds a usable Electron binary (#38673 / run-electron-builder.cjs). _electron_dist_ok() { local install_dir="$1" local electron_dir @@ -2439,22 +2423,7 @@ _electron_dist_ok() { fi } -# (Re)populate the desktop Electron dist via electron's own downloader. -# -# Since #38673 the desktop build pins build.electronDist, so electron-builder -# reads the Electron binary straight from there and never downloads it during -# `npm run pack`. That dist tree is produced by the electron package's -# postinstall (install.js) during `npm ci`. When that download is -# blocked/throttled (GitHub's release host is unreachable in some regions - -# #47266), dist is missing and re-running pack only re-throws "The specified -# electronDist does not exist". The mirror fallback therefore has to drive THIS -# downloader, not another pack. -# -# No-op (returns 0) when the dist binary is already present. Otherwise drops a -# partial dist + version marker (electron's install.js short-circuits when -# path.txt already matches) and runs the downloader once. $1 = the workspace root -# holding node_modules; optional $2 = an ELECTRON_MIRROR base URL. Best-effort: -# returns 0 iff the dist binary exists afterward. +# Best-effort: run electron/install.js to populate dist/ (optional mirror). _restore_electron_dist() { local install_dir="$1" local mirror="${2:-}" @@ -2476,6 +2445,19 @@ _restore_electron_dist() { _electron_dist_ok "$install_dir" } +_electron_pkg_staged_missing_dist() { + local install_dir="$1" + local electron_dir + electron_dir="$(_electron_dir "$install_dir")" + [ -f "$electron_dir/package.json" ] && [ -f "$electron_dir/install.js" ] && ! _electron_dist_ok "$install_dir" +} + +_restore_electron_dist_with_fallback() { + local install_dir="$1" + _restore_electron_dist "$install_dir" \ + || { [ -z "${ELECTRON_MIRROR:-}" ] && _restore_electron_dist "$install_dir" "$DESKTOP_ELECTRON_FALLBACK_MIRROR"; } +} + # Build apps/desktop into a launchable native app. Mirrors install.ps1's # Install-Desktop: a root-level npm install so the apps/* workspace resolves # the desktop's own deps (Electron ~150MB), then `npm run pack` @@ -2517,7 +2499,12 @@ install_desktop() { # `tsc -b` failing with no obvious cause. Fall back to `npm install` # only if `npm ci` is unavailable or the lockfile is out of sync. log_info "Installing desktop workspace dependencies (includes Electron ~150MB, 1-3min)..." - ( cd "$INSTALL_DIR" && npm ci ) || ( cd "$INSTALL_DIR" && npm install ) || { + if ( cd "$INSTALL_DIR" && npm ci ) || ( cd "$INSTALL_DIR" && npm install ); then + log_success "Desktop workspace dependencies installed" + elif _electron_pkg_staged_missing_dist "$INSTALL_DIR"; then + log_warn "Desktop dependency install failed with a missing Electron dist; attempting self-heal..." + _restore_electron_dist_with_fallback "$INSTALL_DIR" || true + else log_error "Desktop workspace npm install failed" # Common cause: a previous 'sudo npm'/'sudo npx' left root-owned files in # ~/.npm, so this non-root install can't write the shared cache. npm hides @@ -2530,8 +2517,7 @@ install_desktop() { log_info "Then re-run this installer, or build manually:" log_info " cd \"$INSTALL_DIR\" && npm ci && cd apps/desktop && npm run pack" return 1 - } - log_success "Desktop workspace dependencies installed" + fi # 2. Build, with up to three escalating attempts so a transient/blocked # Electron download self-heals instead of failing the whole install: @@ -2545,21 +2531,13 @@ install_desktop() { if _desktop_pack "$desktop_dir"; then pack_ok=true else - # (b) Corrupt cached Electron zip is the most common self-healable cause. - local purged - purged="$(clear_electron_build_cache "$desktop_dir")" - # electronDist is pinned to node_modules/electron/dist (#38673): - # electron-builder reads the binary from there and `pack` never downloads - # it, so purging the cache + re-running pack can't by itself repopulate a - # missing/partial dist. When the dist is actually gone, re-run electron's - # own downloader so the retry has a binary to read. Gated on the dist - # check so an unrelated build failure (tsc/vite) doesn't trigger a - # pointless ~200MB refetch. + local purged="" local restored=false if ! _electron_dist_ok "$INSTALL_DIR"; then + purged="$(clear_electron_build_cache "$desktop_dir")" if _restore_electron_dist "$INSTALL_DIR"; then restored=true; fi fi - if [ -n "$purged" ] || [ "$restored" = true ]; then + if [ "$restored" = true ]; then log_warn "Desktop build failed; refreshed the Electron download and retrying once..." if _desktop_pack "$desktop_dir"; then pack_ok=true @@ -2567,27 +2545,14 @@ install_desktop() { fi fi - # (c) Still failing and the user hasn't pinned their own mirror: the GitHub - # release host is likely blocked/throttled. Re-download the Electron - # binary via a public mirror, then retry. The mirror MUST drive - # electron's own downloader — `npm run pack` reads the pinned electronDist - # and never downloads, so a mirror passed only to pack is a no-op (#47266). + # (c) GitHub blocked → mirror fallback (#47266). if [ "$pack_ok" = false ] && [ -z "${ELECTRON_MIRROR:-}" ]; then log_warn "Desktop build still failing — the Electron download from GitHub looks blocked." log_warn "Re-downloading Electron via a public mirror ($DESKTOP_ELECTRON_FALLBACK_MIRROR), then rebuilding..." log_warn " (set ELECTRON_MIRROR yourself to use a different/trusted mirror)" - local have_dist=false - if _electron_dist_ok "$INSTALL_DIR"; then - have_dist=true - elif _restore_electron_dist "$INSTALL_DIR" "$DESKTOP_ELECTRON_FALLBACK_MIRROR"; then - have_dist=true - fi - if [ "$have_dist" = true ]; then - if _desktop_pack "$desktop_dir" "$DESKTOP_ELECTRON_FALLBACK_MIRROR"; then - pack_ok=true - fi - else - log_warn "Could not re-download Electron from the mirror (node_modules/electron/dist still missing)" + _electron_dist_ok "$INSTALL_DIR" || _restore_electron_dist "$INSTALL_DIR" "$DESKTOP_ELECTRON_FALLBACK_MIRROR" || true + if _desktop_pack "$desktop_dir" "$DESKTOP_ELECTRON_FALLBACK_MIRROR"; then + pack_ok=true fi fi diff --git a/tests/hermes_cli/test_gui_command.py b/tests/hermes_cli/test_gui_command.py index 9f7805e02ec..4995a7897ac 100644 --- a/tests/hermes_cli/test_gui_command.py +++ b/tests/hermes_cli/test_gui_command.py @@ -485,13 +485,15 @@ def test_gui_retries_pack_once_after_purging_build_cache(tmp_path, monkeypatch): patch("hermes_cli.main._desktop_linux_sandbox_fixup", return_value=True), \ patch("hermes_cli.main._write_desktop_build_stamp"), \ 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, \ pytest.raises(SystemExit) as exc: cli_main.cmd_gui(_ns()) assert exc.value.code == 0 mock_purge.assert_called_once() - # pack(fail) → purge → pack(ok) → launch = 3 subprocess.run calls + # pack(fail) → repair succeeds → pack(ok) → launch = 3 subprocess.run calls assert mock_run.call_count == 3 assert mock_run.call_args_list[0].args[0] == ["/usr/bin/npm", "run", "pack"] assert mock_run.call_args_list[1].args[0] == ["/usr/bin/npm", "run", "pack"] @@ -535,10 +537,12 @@ def test_gui_redownloads_electron_via_mirror_then_repacks(tmp_path, monkeypatch, assert "Desktop GUI build failed" in capsys.readouterr().out -def test_gui_skips_pack_when_electron_redownload_unrecoverable(tmp_path, monkeypatch, capsys): - """When the Electron binary can't be fetched at all (mirror also blocked), - skip the pointless final pack — it would just re-throw the same missing - electronDist — and fail with a clear message instead.""" +def test_gui_retries_pack_under_mirror_even_when_prefetch_blocked(tmp_path, monkeypatch, capsys): + """When electron's own downloader can't fetch the binary (even via the + mirror), still retry pack under ELECTRON_MIRROR: the build resolves + electronDist dynamically and lets electron-builder fetch Electron itself + via @electron/get, which honors the mirror. That retry is no longer + 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") @@ -553,17 +557,96 @@ def test_gui_skips_pack_when_electron_redownload_unrecoverable(tmp_path, monkeyp patch("hermes_cli.main._purge_electron_build_cache", return_value=[]), \ patch("hermes_cli.main._electron_dist_ok", return_value=False), \ patch("hermes_cli.main._redownload_electron_dist", return_value=False), \ - patch("hermes_cli.main.subprocess.run", side_effect=[pack_fail]) as mock_run, \ + patch("hermes_cli.main.subprocess.run", side_effect=[pack_fail, pack_fail]) as mock_run, \ pytest.raises(SystemExit) as exc: cli_main.cmd_gui(_ns()) assert exc.value.code == 1 - # Only the initial pack ran; both retries were skipped because no binary - # could be produced. - assert mock_run.call_count == 1 - out = capsys.readouterr().out - assert "Could not re-download Electron from the mirror" in out - assert "Desktop GUI build failed" in out + # Initial pack + mirror-driven pack = 2; the mirror retry runs even though + # the pre-fetch failed, so electron-builder gets a shot at downloading. + assert mock_run.call_count == 2 + assert "ELECTRON_MIRROR" not in (mock_run.call_args_list[0].kwargs.get("env") or {}) + assert mock_run.call_args_list[1].kwargs["env"]["ELECTRON_MIRROR"] + assert "Desktop GUI build failed" in capsys.readouterr().out + + +def test_gui_install_failure_self_heals_electron_and_continues(tmp_path, monkeypatch, capsys): + """npm ci failing on electron's blocked binary download must NOT abort the + install: with the electron package staged, repopulate its dist and continue + to the build instead of sys.exit-ing before pack ever runs (#47266/#48021).""" + root = _make_desktop_tree(tmp_path) + monkeypatch.setattr(cli_main, "PROJECT_ROOT", root) + packaged_exe = _make_packaged_executable(root, monkeypatch, platform="linux") + # electron package staged on disk (postinstall download was the casualty). + (root / "apps" / "desktop" / "node_modules" / "electron").mkdir(parents=True) + (root / "apps" / "desktop" / "node_modules" / "electron" / "package.json").write_text("{}", encoding="utf-8") + (root / "apps" / "desktop" / "node_modules" / "electron" / "install.js").write_text("", encoding="utf-8") + + install_fail = subprocess.CompletedProcess(["npm", "ci"], 1) + pack_ok = subprocess.CompletedProcess(["npm", "run", "pack"], 0) + launch_ok = subprocess.CompletedProcess([str(packaged_exe)], 0) + + with patch("hermes_cli.main.shutil.which", return_value="/usr/bin/npm"), \ + patch("hermes_cli.main._run_npm_install_deterministic", return_value=install_fail), \ + patch("hermes_cli.main._desktop_linux_sandbox_fixup", return_value=True), \ + patch("hermes_cli.main._write_desktop_build_stamp"), \ + patch("hermes_cli.main._electron_dist_ok", return_value=False), \ + patch("hermes_cli.main._try_redownload_electron_dist", return_value=True) as mock_dl, \ + patch("hermes_cli.main.subprocess.run", side_effect=[pack_ok, launch_ok]) as mock_run, \ + pytest.raises(SystemExit) as exc: + cli_main.cmd_gui(_ns()) + + assert exc.value.code == 0 + mock_dl.assert_called() # tried to repopulate the dist + # pack + launch ran — the install failure did NOT abort the build. + assert mock_run.call_count == 2 + assert "repopulated" in capsys.readouterr().out.lower() + + +def test_gui_install_failure_hard_fails_when_electron_not_staged(tmp_path, monkeypatch, capsys): + """A dependency-install failure where electron never even staged is a genuine + error (not a blocked binary download) — hard-fail with guidance, don't try to + self-heal a tree that isn't there.""" + root = _make_desktop_tree(tmp_path) + monkeypatch.setattr(cli_main, "PROJECT_ROOT", root) + _make_packaged_executable(root, monkeypatch, platform="linux") + + install_fail = subprocess.CompletedProcess(["npm", "ci"], 1) + + with patch("hermes_cli.main.shutil.which", return_value="/usr/bin/npm"), \ + patch("hermes_cli.main._run_npm_install_deterministic", return_value=install_fail), \ + patch("hermes_cli.main.subprocess.run") as mock_run, \ + pytest.raises(SystemExit) as exc: + cli_main.cmd_gui(_ns()) + + assert exc.value.code == 1 + mock_run.assert_not_called() # build never started + assert "Desktop dependency install failed" in capsys.readouterr().out + + +def test_gui_install_failure_hard_fails_when_electron_dist_exists(tmp_path, monkeypatch, capsys): + """If npm install fails but Electron dist is already present, don't classify + it as the blocked-download shape; fail fast as a generic install error.""" + root = _make_desktop_tree(tmp_path) + monkeypatch.setattr(cli_main, "PROJECT_ROOT", root) + _make_packaged_executable(root, monkeypatch, platform="linux") + electron_dir = root / "apps" / "desktop" / "node_modules" / "electron" + electron_dir.mkdir(parents=True) + (electron_dir / "package.json").write_text("{}", encoding="utf-8") + (electron_dir / "install.js").write_text("", encoding="utf-8") + + install_fail = subprocess.CompletedProcess(["npm", "ci"], 1) + + with patch("hermes_cli.main.shutil.which", return_value="/usr/bin/npm"), \ + patch("hermes_cli.main._run_npm_install_deterministic", return_value=install_fail), \ + patch("hermes_cli.main._electron_dist_ok", return_value=True), \ + patch("hermes_cli.main.subprocess.run") as mock_run, \ + pytest.raises(SystemExit) as exc: + cli_main.cmd_gui(_ns()) + + assert exc.value.code == 1 + mock_run.assert_not_called() + assert "Desktop dependency install failed" in capsys.readouterr().out def test_gui_does_not_override_user_electron_mirror(tmp_path, monkeypatch, capsys): diff --git a/tests/test_desktop_electron_pin.py b/tests/test_desktop_electron_pin.py index ee29dfd0861..2943dc9c9fe 100644 --- a/tests/test_desktop_electron_pin.py +++ b/tests/test_desktop_electron_pin.py @@ -96,40 +96,40 @@ def test_lockfile_resolves_the_pinned_electron(): ) -def test_electron_dist_matches_lockfile_install_location(): - """build.electronDist must point at where the lockfile installs Electron. +DESKTOP_DIR = REPO_ROOT / "apps" / "desktop" +ELECTRON_BUILDER_WRAPPER = DESKTOP_DIR / "scripts" / "run-electron-builder.cjs" - electron-builder copies the unpacked Electron from ``build.electronDist`` - (resolved relative to ``apps/desktop``). npm workspace hoisting is not - deterministic across machines/npm versions: it may nest Electron under - ``apps/desktop/node_modules/electron`` or hoist it to the repo root. If - electronDist points at one location while the lockfile installs at the - other, packaging fails with ``The specified electronDist does not exist`` — - the "Building desktop app" failure reported after the June lockfile - regeneration floated Electron and reshuffled the hoist. Lock the two - together so a hoist change (root <-> nested) can't silently break the path - again. - """ - if not ROOT_LOCK.is_file(): - pytest.skip("root package-lock.json not present") - electron_dist = _desktop_pkg().get("build", {}).get("electronDist") - assert electron_dist, "build.electronDist is missing" - lock = json.loads(ROOT_LOCK.read_text(encoding="utf-8")) - electron_paths = [ - path - for path in lock.get("packages", {}) - if path.endswith("node_modules/electron") - ] - assert electron_paths, "no electron entry found in package-lock.json" - - desktop_dir = REPO_ROOT / "apps" / "desktop" - # electronDist is resolved relative to the apps/desktop project dir. - configured = (desktop_dir / electron_dist).resolve() - # Where the lockfile actually places Electron's unpacked dist. - installed = {(REPO_ROOT / p / "dist").resolve() for p in electron_paths} - assert configured in installed, ( - f"build.electronDist={electron_dist!r} resolves to {configured}, but the " - f"lockfile installs Electron at {sorted(str(p) for p in installed)}. " - "electron-builder will fail with 'electronDist does not exist'." +def test_no_static_electron_dist_that_can_drift(): + """build.electronDist must not be a static path — hoisting is non-deterministic.""" + assert "electronDist" not in _desktop_pkg().get("build", {}), ( + "build.electronDist is hardcoded again. npm hoisting is non-deterministic, " + "so a static path silently breaks packaging when the layout changes. Let " + "scripts/run-electron-builder.cjs resolve it dynamically instead." + ) + + +def test_builder_script_routes_through_dynamic_resolver(): + """npm run builder must invoke run-electron-builder.cjs, not bare electron-builder.""" + builder = _desktop_pkg().get("scripts", {}).get("builder", "") + assert "run-electron-builder.cjs" in builder, ( + f"the 'builder' script must run scripts/run-electron-builder.cjs, got " + f"{builder!r}" + ) + assert ELECTRON_BUILDER_WRAPPER.is_file(), ( + f"missing dynamic-resolver wrapper at {ELECTRON_BUILDER_WRAPPER}" + ) + + +def test_resolver_uses_node_module_resolution(): + """Wrapper must resolve electron via require.resolve and pass -c.electronDist.""" + src = ELECTRON_BUILDER_WRAPPER.read_text(encoding="utf-8") + assert 'require.resolve("electron/package.json")' in src, ( + "run-electron-builder.cjs must resolve electron via " + "require.resolve('electron/package.json') to stay hoist-proof." + ) + # And it must hand the resolved dist to electron-builder as an override. + assert "-c.electronDist=" in src, ( + "run-electron-builder.cjs must pass the resolved dist to electron-builder " + "via -c.electronDist." )