Corrupt Electron download cache breaks desktop build

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #39032 fix(desktop): recover from corrupt cached Electron download on build

  merged to main

  files: apps/desktop/package.json, apps/desktop/scripts/before-pack.cjs, apps/desktop/scripts/before-pack.test.cjs, hermes_cli/main.py, tests/hermes_cli/test_gui_command.py

  Show the diff

  • apps/desktop/package.json

    @@ -146,6 +146,7 @@
           "package.json"
         ],
         "beforeBuild": "scripts/before-build.cjs",
    +    "beforePack": "scripts/before-pack.cjs",
         "afterPack": "scripts/after-pack.cjs",
         "extraResources": [
           {

  • apps/desktop/scripts/before-pack.cjs

    @@ -0,0 +1,78 @@
    +'use strict'
    +
    +/**
    + * before-pack.cjs — electron-builder beforePack hook.
    + *
    + * Removes any stale unpacked app directory (`appOutDir`) before
    + * electron-builder stages the Electron binaries into it.
    + *
    + * WHY THIS EXISTS
    + * ---------------
    + * electron-builder's final packaging step copies the stock `electron`
    + * binary into `release/<platform>-unpacked/` and then renames it to the
    + * product name (`Hermes`). If a PREVIOUS `npm run pack` was interrupted
    + * (Ctrl-C, OOM kill, crash, full disk) the unpacked directory is left in a
    + * corrupted partial state: it keeps the already-renamed `LICENSE.electron.txt`
    + * and the Chromium payload (.pak/.so/icudtl.dat/chrome-sandbox) but is MISSING
    + * the `electron` binary itself.
    + *
    + * On the next run, electron-builder sees the destination directory already
    + * populated, skips re-copying the binary it thinks is present, then tries to
    + * rename a `electron` file that no longer exists. The build dies with:
    + *
    + *   ENOENT: no such file or directory, rename
    + *   '.../release/linux-unpacked/electron' -> '.../release/linux-unpacked/Hermes'
    + *
    + * This is a hard failure with no obvious cause for the user — `hermes desktop`
    + * just prints "Desktop GUI build failed" and the only fix is to manually
    + * `rm -rf` the release directory, which a normal user has no way to know.
    + *
    + * The packaging step is not idempotent across an interrupted run, so we make
    + * it idempotent ourselves: wipe the target unpacked directory up front so
    + * electron-builder always stages into a clean tree. This is safe — the
    + * directory is a pure build artifact that electron-builder fully recreates
    + * on every pack; nothing else depends on its prior contents.
    + *
    + * Cross-platform: the same partial-state trap exists on macOS
    + * (the mac-unpacked Hermes.app bundle) and Windows (win-unpacked), so we
    + * clean whatever `appOutDir` electron-builder hands us regardless of platform.
    + *
    + * Best-effort: a cleanup failure must never mask the real build. We log and
    + * resolve rather than throw — worst case electron-builder hits the original
    + * ENOENT, which is no worse than not having this hook at all.
    + *
    + * electron-builder passes a context with:
    + *   - appOutDir:            the unpacked app directory about to be staged
    + *   - electronPlatformName: 'win32' | 'darwin' | 'linux'
    + */
    +
    +const fs = require('node:fs')
    +
    +function cleanStaleAppOutDir(appOutDir) {
    +  if (!appOutDir || typeof appOutDir !== 'string') {
    +    return false
    +  }
    +  if (!fs.existsSync(appOutDir)) {
    +    return false
    +  }
    +  // Recursive + force so a half-written tree (read-only bits, partial files)
    +  // can't block the wipe. retry/maxRetries rides out transient EBUSY on
    +  // Windows where an AV/indexer may briefly hold a handle.
    +  fs.rmSync(appOutDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 })
    +  return true
    +}
    +
    +exports.cleanStaleAppOutDir = cleanStaleAppOutDir
    +
    +exports.default = async function beforePack(context) {
    +  const appOutDir = context && context.appOutDir
    +  try {
    +    if (cleanStaleAppOutDir(appOutDir)) {
    +      console.log(`[before-pack] removed stale unpacked dir before staging: ${appOutDir}`)
    +    }
    +  } catch (err) {
    +    // Never fail the build over cleanup; surface why so a genuinely stuck
    +    // directory (permissions, mount) is still diagnosable.
    +    console.warn(`[before-pack] could not clean ${appOutDir} (${err.message}); continuing`)
    +  }
    +}

  • apps/desktop/scripts/before-pack.test.cjs

    @@ -0,0 +1,53 @@
    +const assert = require('node:assert/strict')
    +const fs = require('node:fs')
    +const os = require('node:os')
    +const path = require('node:path')
    +const test = require('node:test')
    +
    +const { cleanStaleAppOutDir } = require('../scripts/before-pack.cjs')
    +
    +test('cleanStaleAppOutDir removes a populated unpacked directory', () => {
    +  const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-before-pack-'))
    +  try {
    +    const appOutDir = path.join(tempRoot, 'linux-unpacked')
    +    fs.mkdirSync(appOutDir, { recursive: true })
    +    // Reproduce the corrupted partial state: license + payload present,
    +    // electron binary missing — exactly what trips the ENOENT rename.
    +    fs.writeFileSync(path.join(appOutDir, 'LICENSE.electron.txt'), 'x', 'utf8')
    +    fs.writeFileSync(path.join(appOutDir, 'resources.pak'), 'x', 'utf8')
    +    fs.mkdirSync(path.join(appOutDir, 'resources'), { recursive: true })
    +    fs.writeFileSync(path.join(appOutDir, 'resources', 'app.asar'), 'x', 'utf8')
    +
    +    const removed = cleanStaleAppOutDir(appOutDir)
    +
    +    assert.equal(removed, true)
    +    assert.equal(fs.existsSync(appOutDir), false)
    +  } finally {
    +    fs.rmSync(tempRoot, { recursive: true, force: true })
    +  }
    +})
    +
    +test('cleanStaleAppOutDir is a no-op when the directory is absent', () => {
    +  const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-before-pack-'))
    +  try {
    +    const missing = path.join(tempRoot, 'does-not-exist')
    +    assert.equal(cleanStaleAppOutDir(missing), false)
    +  } finally {
    +    fs.rmSync(tempRoot, { recursive: true, force: true })
    +  }
    +})
    +
    +test('cleanStaleAppOutDir ignores empty or invalid input', () => {
    +  assert.equal(cleanStaleAppOutDir(''), false)
    +  assert.equal(cleanStaleAppOutDir(undefined), false)
    +  assert.equal(cleanStaleAppOutDir(null), false)
    +  assert.equal(cleanStaleAppOutDir(42), false)
    +})
    +
    +test('beforePack default export resolves even when cleanup throws', async () => {
    +  const { default: beforePack } = require('../scripts/before-pack.cjs')
    +  // A directory path that rmSync can't remove is simulated by passing a
    +  // context whose appOutDir is a file the hook will try (and be allowed) to
    +  // remove; the contract under test is that the hook never rejects.
    +  await assert.doesNotReject(beforePack({ appOutDir: '', electronPlatformName: 'linux' }))
    +})

  • hermes_cli/main.py

    @@ -7233,6 +7233,102 @@ def _desktop_packaged_executable(desktop_dir: Path) -> Optional[Path]:
         return max(existing, key=lambda p: p.stat().st_mtime)
     
     
    +def _electron_download_cache_dirs() -> list[Path]:
    +    """Return the per-user Electron download cache directories for this OS.
    +
    +    electron-builder's ``app-builder unpack-electron`` extracts the Electron
    +    distribution from a zip stored in this cache (NOT from node_modules), so a
    +    corrupt zip here — not a bad workspace install — is what poisons the build.
    +    Honors the ``electron_config_cache`` / ``ELECTRON_CACHE`` overrides that
    +    ``@electron/get`` respects, then falls back to the platform defaults.
    +    """
    +    home = Path.home()
    +    candidates: list[Path] = []
    +    override = os.environ.get("electron_config_cache") or os.environ.get("ELECTRON_CACHE")
    +    if override:
    +        candidates.append(Path(override))
    +    if sys.platform == "darwin":
    +        candidates.append(home / "Library" / "Caches" / "electron")
    +    elif sys.platform == "win32":
    +        local = os.environ.get("LOCALAPPDATA")
    +        if local:
    +            candidates.append(Path(local) / "electron" / "Cache")
    +        candidates.append(home / "AppData" / "Local" / "electron" / "Cache")
    +    else:
    +        xdg = os.environ.get("XDG_CACHE_HOME")
    +        if xdg:
    +            candidates.append(Path(xdg) / "electron")
    +        candidates.append(home / ".cache" / "electron")
    +
    +    seen: set[Path] = set()
    +    out: list[Path] = []
    +    for c in candidates:
    +        rc = c.expanduser()
    +        if rc not in seen:
    +            seen.add(rc)
    +            out.append(rc)
    +    return out
    +
    +
    +def _purge_electron_build_cache(desktop_dir: Path) -> list[Path]:
    +    """Clear the cached Electron download + half-written unpacked dir so the
    +    next ``pack`` re-downloads and re-stages from scratch.
    +
    +    Root cause of the ``ENOENT … rename '…/linux-unpacked/electron' ->
    +    '…/linux-unpacked/Hermes'`` desktop build failure: a corrupt zip in the
    +    per-user Electron download cache (a partial download resumed into the same
    +    file leaves prepended/concatenated junk, or an interrupted write truncates
    +    it). electron-builder's ``app-builder unpack-electron`` extracts the
    +    distribution from that cached zip (NOT from node_modules); a bad zip yields
    +    a partial tree MISSING the 193 MB ``electron`` binary, so the final rename
    +    dies. Re-running repeats the same broken extraction forever.
    +
    +    We deliberately do NOT try to detect corruption ourselves. stdlib
    +    ``zipfile`` silently tolerates the prepended/concatenated junk that is the
    +    most common corruption here — it reads from the end-of-central-directory
    +    backward, so ``testzip()`` returns clean on exactly the zips ``unzip -t``
    +    and ``@electron/get`` reject. Gating the purge on a self-rolled validator
    +    would therefore skip the real-world case and never self-heal. Instead, on a
    +    packaged-build failure we unconditionally remove the version's cached zips
    +    and the stale unpacked dir, then let the caller retry once: ``@electron/get``
    +    re-downloads with its own SHASUM verification (the real source of truth),
    +    and ``before-pack.cjs`` re-wipes the unpacked dir. If the failure was
    +    unrelated, a clean re-download is harmless and the retry fails the same way.
    +
    +    Best-effort: never raises. Returns the paths removed so the caller can log
    +    them and decide whether a retry is worthwhile (empty list ⇒ nothing to
    +    clear, so no point retrying).
    +    """
    +    removed: list[Path] = []
    +
    +    for cache_dir in _electron_download_cache_dirs():
    +        if not cache_dir.is_dir():
    +            continue
    +        for zip_path in sorted(cache_dir.rglob("electron-*.zip")):
    +            try:
    +                zip_path.unlink()
    +                removed.append(zip_path)
    +            except OSError:
    +                # Locked/permission-denied entry is out of our hands; let the
    +                # build report its own error rather than masking it.
    +                pass
    +
    +    # Drop the half-written unpacked dir too: an interrupted prior pack leaves
    +    # a partial tree that poisons the rename even after the zip is fixed.
    +    # (before-pack.cjs also handles this, but clearing it here makes the retry
    +    # robust even if the hook is somehow skipped.)
    +    release_dir = desktop_dir / "release"
    +    if release_dir.is_dir():
    +        for unpacked in release_dir.glob("*-unpacked"):
    +            try:
    +                shutil.rmtree(unpacked, ignore_errors=True)
    +                removed.append(unpacked)
    +            except OSError:
    +                pass
    +
    +    return removed
    +
    +
     def _desktop_macos_relaunchable_fixup(desktop_dir: Path) -> None:
         """Make a locally-built (unsigned) macOS desktop app survive in-place self-update.
     
    @@ -7389,6 +7485,26 @@ def cmd_gui(args: argparse.Namespace):
                 print(f"→ Building desktop {build_label}...")
                 build_script = "build" if source_mode else "pack"
                 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)
    +                if purged:
    +                    print("  ⚠ Desktop build failed; cleared cached Electron download and retrying once...")
    +                    for p in purged:
    +                        print(f"    - {p}")
    +                    build_result = subprocess.run([npm, "run", build_script], cwd=desktop_dir, env=env, check=False)
                 if build_result.returncode != 0:
                     print("✗ Desktop GUI build failed")
                     print(f"  Run manually:  cd apps/desktop && npm run {build_script}")

  • tests/hermes_cli/test_gui_command.py

    @@ -411,3 +411,111 @@ def test_compute_desktop_content_hash_respects_gitignore(tmp_path, monkeypatch):
         cli_main._DESKTOP_STAMP_SPEC = None
         h3 = cli_main._compute_desktop_content_hash(root)
         assert h1 != h3, "changing a tracked file should change the hash"
    +
    +
    +# ── Electron build-cache recovery tests ───────────────────────────────
    +
    +
    +def _write_zip(path: Path) -> None:
    +    import zipfile
    +
    +    path.parent.mkdir(parents=True, exist_ok=True)
    +    with zipfile.ZipFile(path, "w") as zf:
    +        zf.writestr("electron", "fake binary payload")
    +
    +
    +def test_purge_electron_build_cache_clears_all_zips_and_unpacked_dir(tmp_path, monkeypatch):
    +    """Purge is unconditional: it removes every electron-*.zip (regardless of
    +    whether stdlib zipfile thinks it's corrupt) plus the half-written unpacked
    +    dir, because @electron/get's own SHASUM check on re-download is the real
    +    validator — not a self-rolled one."""
    +    cache = tmp_path / "electron-cache"
    +    # A "clean" zip and a prepended-junk zip — the latter is the real-world
    +    # corruption that zipfile.testzip() silently passes (it reads from the
    +    # end-of-central-directory backward), which is why we don't gate on it.
    +    clean = cache / "electron-v40.9.3-linux-x64.zip"
    +    prepended = cache / "hashdir" / "electron-v40.9.3-linux-x64.zip"
    +    _write_zip(clean)
    +    _write_zip(prepended)
    +    prepended.write_bytes(b"\x00" * 4096 + prepended.read_bytes())
    +
    +    desktop_dir = tmp_path / "apps" / "desktop"
    +    unpacked = desktop_dir / "release" / "linux-unpacked"
    +    unpacked.mkdir(parents=True)
    +    (unpacked / "LICENSE.electron.txt").write_text("x", encoding="utf-8")
    +    (unpacked / "resources.pak").write_text("x", encoding="utf-8")
    +
    +    monkeypatch.setattr(cli_main, "_electron_download_cache_dirs", lambda: [cache])
    +
    +    removed = cli_main._purge_electron_build_cache(desktop_dir)
    +
    +    assert clean in removed
    +    assert prepended in removed
    +    assert unpacked in removed
    +    assert not clean.exists()
    +    assert not prepended.exists()
    +    assert not unpacked.exists()
    +
    +
    +def test_purge_electron_build_cache_empty_when_nothing_present(tmp_path, monkeypatch):
    +    """No cached zips and no unpacked dir → nothing removed, so the caller
    +    knows a retry is pointless."""
    +    cache = tmp_path / "electron-cache"
    +    cache.mkdir()
    +    desktop_dir = tmp_path / "apps" / "desktop"
    +    monkeypatch.setattr(cli_main, "_electron_download_cache_dirs", lambda: [cache])
    +
    +    assert cli_main._purge_electron_build_cache(desktop_dir) == []
    +
    +
    +def test_gui_retries_pack_once_after_purging_build_cache(tmp_path, monkeypatch):
    +    """First pack fails, purge clears the cache, second pack succeeds, launch."""
    +    root = _make_desktop_tree(tmp_path)
    +    monkeypatch.setattr(cli_main, "PROJECT_ROOT", root)
    +    packaged_exe = _make_packaged_executable(root, monkeypatch, platform="linux")
    +
    +    install_ok = subprocess.CompletedProcess(["npm", "ci"], 0)
    +    pack_fail = subprocess.CompletedProcess(["npm", "run", "pack"], 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_ok), \
    +         patch("hermes_cli.main._desktop_macos_relaunchable_fixup"), \
    +         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.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
    +    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"]
    +    assert mock_run.call_args_list[2].args[0] == [str(packaged_exe)]
    +
    +
    +def test_gui_does_not_retry_when_purge_finds_nothing(tmp_path, monkeypatch, capsys):
    +    """If the purge clears nothing, there's no point retrying — fail fast."""
    +    root = _make_desktop_tree(tmp_path)
    +    monkeypatch.setattr(cli_main, "PROJECT_ROOT", root)
    +    _make_packaged_executable(root, monkeypatch, platform="linux")
    +
    +    install_ok = subprocess.CompletedProcess(["npm", "ci"], 0)
    +    pack_fail = subprocess.CompletedProcess(["npm", "run", "pack"], 1)
    +
    +    with patch("hermes_cli.main.shutil.which", return_value="/usr/bin/npm"), \
    +         patch("hermes_cli.main._run_npm_install_deterministic", return_value=install_ok), \
    +         patch("hermes_cli.main._desktop_macos_relaunchable_fixup"), \
    +         patch("hermes_cli.main._purge_electron_build_cache", return_value=[]) as mock_purge, \
    +         patch("hermes_cli.main.subprocess.run", side_effect=[pack_fail]) as mock_run, \
    +         pytest.raises(SystemExit) as exc:
    +        cli_main.cmd_gui(_ns())
    +
    +    assert exc.value.code == 1
    +    mock_purge.assert_called_once()
    +    assert mock_run.call_count == 1
    +    assert "Desktop GUI build failed" in capsys.readouterr().out