mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-19 10:02:16 +00:00
* 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=<abs>/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 <zheng@omegasys.eu> 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 <zheng@omegasys.eu> Co-authored-by: james47kjv <220877172+james47kjv@users.noreply.github.com>
135 lines
5.6 KiB
Python
135 lines
5.6 KiB
Python
"""Regression: the desktop Electron dependency must be an exact, consistent pin.
|
|
|
|
The Windows desktop install failed at "Building desktop app" because Electron
|
|
changed its install mechanism mid patch-series:
|
|
|
|
electron 40.9.3 .. 40.10.2 -> @electron/get@^2 + extract-zip@^2 (pure JS)
|
|
electron 40.10.3 / 40.10.4 -> @electron/get@^5 +
|
|
@electron-internal/extract-zip@^1 (native napi)
|
|
|
|
``apps/desktop/package.json`` declared ``electronVersion: 40.9.3`` (the tested,
|
|
JS-extract build) but pinned the dependency loosely as ``electron: ^40.9.3``.
|
|
``npm ci`` then resolved 40.10.3/40.10.4 — the new *native* extract-zip whose
|
|
win32-x64 binding fails to ``dlopen`` on some Windows hosts
|
|
(``ERR_DLOPEN_FAILED loading index.win32-x64-msvc.node``).
|
|
|
|
These tests lock the contract that prevents that drift, without hard-coding the
|
|
specific version (which is allowed to move):
|
|
|
|
1. the Electron dependency is an *exact* version (Electron Builder needs the
|
|
installed binary to match ``electronVersion`` / ``electronDist``), and
|
|
2. the dependency, ``build.electronVersion``, and the resolved lockfile entry
|
|
all agree — so ``npm ci`` installs exactly what the build packages.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import json
|
|
import re
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
|
|
REPO_ROOT = Path(__file__).resolve().parent.parent
|
|
DESKTOP_PKG = REPO_ROOT / "apps" / "desktop" / "package.json"
|
|
ROOT_LOCK = REPO_ROOT / "package-lock.json"
|
|
|
|
# An exact semver: digits.digits.digits with an optional prerelease/build tag,
|
|
# but NO range operators (^ ~ > < = * x || spaces || -range).
|
|
_EXACT_SEMVER = re.compile(r"^\d+\.\d+\.\d+(?:[-+][0-9A-Za-z.-]+)?$")
|
|
|
|
|
|
def _desktop_pkg() -> dict:
|
|
assert DESKTOP_PKG.is_file(), f"missing {DESKTOP_PKG}"
|
|
return json.loads(DESKTOP_PKG.read_text(encoding="utf-8"))
|
|
|
|
|
|
def _electron_spec(pkg: dict) -> str:
|
|
for section in ("dependencies", "devDependencies"):
|
|
spec = pkg.get(section, {}).get("electron")
|
|
if spec:
|
|
return spec
|
|
pytest.fail("electron is not listed in apps/desktop dependencies")
|
|
|
|
|
|
def test_electron_dependency_is_exactly_pinned():
|
|
"""A loose range lets npm drift onto an Electron with a different installer."""
|
|
spec = _electron_spec(_desktop_pkg())
|
|
assert _EXACT_SEMVER.match(spec), (
|
|
f"electron must be pinned to an exact version, got {spec!r}. "
|
|
"A range (^/~) lets npm ci resolve a newer Electron whose postinstall "
|
|
"may differ from the one the build was validated against."
|
|
)
|
|
|
|
|
|
def test_electron_dependency_matches_electron_version():
|
|
"""electron-builder packages build.electronVersion against the installed binary."""
|
|
pkg = _desktop_pkg()
|
|
spec = _electron_spec(pkg)
|
|
builder_version = pkg.get("build", {}).get("electronVersion")
|
|
assert builder_version, "build.electronVersion is missing"
|
|
assert spec == builder_version, (
|
|
f"electron dependency ({spec!r}) must equal build.electronVersion "
|
|
f"({builder_version!r}); otherwise electron-builder packages a different "
|
|
"version than npm installs into electronDist."
|
|
)
|
|
|
|
|
|
def test_lockfile_resolves_the_pinned_electron():
|
|
"""npm ci installs from the lockfile, so it must agree with the pin."""
|
|
if not ROOT_LOCK.is_file():
|
|
pytest.skip("root package-lock.json not present")
|
|
spec = _electron_spec(_desktop_pkg())
|
|
lock = json.loads(ROOT_LOCK.read_text(encoding="utf-8"))
|
|
packages = lock.get("packages", {})
|
|
resolved = [
|
|
meta.get("version")
|
|
for path, meta in packages.items()
|
|
if path.endswith("node_modules/electron") and meta.get("version")
|
|
]
|
|
assert resolved, "no electron entry found in package-lock.json"
|
|
assert all(v == spec for v in resolved), (
|
|
f"package-lock.json resolves electron to {sorted(set(resolved))}, "
|
|
f"but the pin is {spec!r}; run `npm install --package-lock-only` so "
|
|
"`npm ci` stays consistent."
|
|
)
|
|
|
|
|
|
DESKTOP_DIR = REPO_ROOT / "apps" / "desktop"
|
|
ELECTRON_BUILDER_WRAPPER = DESKTOP_DIR / "scripts" / "run-electron-builder.cjs"
|
|
|
|
|
|
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."
|
|
)
|