mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-19 10:02:16 +00:00
fix(desktop): resolve electronDist to the actual electron install location (#48081)
After the June lockfile regeneration (#46652) floated electron and reshuffled npm workspace hoisting, the desktop pack fails with "The specified electronDist does not exist". apps/desktop/package.json pointed electronDist at the repo root (../../node_modules/electron/dist) while npm now installs electron nested under apps/desktop/node_modules/electron. The two contradict, so a clean install can never package the app (Windows + macOS). - electronDist -> node_modules/electron/dist (resolved relative to apps/desktop, i.e. the workspace-local install npm actually produces). - hermes_cli/main.py, scripts/install.sh, scripts/install.ps1: add a runtime electron-dir resolver that prefers apps/desktop/node_modules/electron and falls back to the root hoist, so dist checks + the mirror re-download work under either npm layout. - patch-electron-builder-mac-binary.cjs: try the workspace-local Electron.app before the root hoist in the macOS binary-restore fallback (sibling site no PR touched). - test: assert build.electronDist resolves to where the lockfile installs electron, so a future hoist change (root <-> nested) can't silently break it. Salvages the overlapping work in #48003 (sitkarev), #48012 (omegazheng), and #48033 (james47kjv). 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>
This commit is contained in:
parent
016bce1a09
commit
f8098c6b6f
8 changed files with 167 additions and 46 deletions
|
|
@ -135,7 +135,7 @@
|
|||
},
|
||||
"build": {
|
||||
"electronVersion": "40.10.2",
|
||||
"electronDist": "../../node_modules/electron/dist",
|
||||
"electronDist": "node_modules/electron/dist",
|
||||
"appId": "com.nousresearch.hermes",
|
||||
"productName": "Hermes",
|
||||
"executableName": "Hermes",
|
||||
|
|
|
|||
|
|
@ -24,6 +24,11 @@ const replacement = ` // ${marker}: electron-builder 26.8.x can sometimes cop
|
|||
if (!fs.existsSync(bundledElectronBinary)) {
|
||||
const candidates = [
|
||||
path.join(packager.info.framework.distMacOsAppName, "Contents", "MacOS", electronBranding.productName),
|
||||
// npm may nest the workspace-only electron devDep under
|
||||
// apps/desktop/node_modules (process.cwd() during pack), or hoist
|
||||
// it to the repo root. Try the workspace-local install first, then
|
||||
// the root hoist, so the fallback works under either layout.
|
||||
path.join(process.cwd(), "node_modules", "electron", "dist", "Electron.app", "Contents", "MacOS", electronBranding.productName),
|
||||
path.join(process.cwd(), "..", "..", "node_modules", "electron", "dist", "Electron.app", "Contents", "MacOS", electronBranding.productName),
|
||||
];
|
||||
const sourceBinary = candidates.find(candidate => fs.existsSync(candidate));
|
||||
|
|
|
|||
|
|
@ -5110,16 +5110,33 @@ def _purge_electron_build_cache(desktop_dir: Path) -> list[Path]:
|
|||
return removed
|
||||
|
||||
|
||||
def _electron_dist_binary(project_root: Path) -> Path:
|
||||
"""Return the path to the Electron main binary inside ``node_modules``.
|
||||
def _electron_dir(project_root: Path) -> Path:
|
||||
"""Return the Electron package directory the desktop workspace installs.
|
||||
|
||||
electron-builder reads the binary from ``build.electronDist``
|
||||
(``node_modules/electron/dist``) since #38673, so this is the exact file
|
||||
whose absence makes a pack fail with "The specified electronDist does not
|
||||
exist". The basename differs per OS (the platform Electron is named for the
|
||||
host the build runs on).
|
||||
npm may keep 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,
|
||||
so a build path that assumes one or the other breaks intermittently across
|
||||
machines. ``apps/desktop/package.json`` points electron-builder's
|
||||
``electronDist`` at ``node_modules/electron/dist`` relative to the desktop
|
||||
project, so prefer the workspace-local package and fall back to the root
|
||||
hoist when that's where npm landed it.
|
||||
"""
|
||||
dist = project_root / "node_modules" / "electron" / "dist"
|
||||
desktop_local = project_root / "apps" / "desktop" / "node_modules" / "electron"
|
||||
if desktop_local.exists():
|
||||
return desktop_local
|
||||
return project_root / "node_modules" / "electron"
|
||||
|
||||
|
||||
def _electron_dist_binary(project_root: Path) -> Path:
|
||||
"""Return the path to the Electron main binary inside the installed package.
|
||||
|
||||
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". The basename differs per OS (the
|
||||
platform Electron is named for the host the build runs on).
|
||||
"""
|
||||
dist = _electron_dir(project_root) / "dist"
|
||||
if sys.platform == "darwin":
|
||||
return dist / "Electron.app" / "Contents" / "MacOS" / "Electron"
|
||||
if sys.platform == "win32":
|
||||
|
|
@ -5169,7 +5186,7 @@ def _redownload_electron_dist(
|
|||
if _electron_dist_ok(project_root):
|
||||
return True
|
||||
|
||||
electron_dir = project_root / "node_modules" / "electron"
|
||||
electron_dir = _electron_dir(project_root)
|
||||
installer = electron_dir / "install.js"
|
||||
if not installer.is_file():
|
||||
return False
|
||||
|
|
@ -5387,8 +5404,8 @@ def cmd_gui(args: argparse.Namespace):
|
|||
print(" Pre-build first: cd apps/desktop && npm run build")
|
||||
print(" Or drop --skip-build to install dependencies and build automatically.")
|
||||
sys.exit(1)
|
||||
if not (PROJECT_ROOT / "node_modules" / "electron" / "package.json").exists():
|
||||
print("✗ --skip-build --source requires existing workspace dependencies.")
|
||||
if not (_electron_dir(PROJECT_ROOT) / "package.json").exists():
|
||||
print("✗ --skip-build --source requires existing desktop workspace dependencies.")
|
||||
print(f" Install first: cd {PROJECT_ROOT} && npm ci")
|
||||
print(" Or drop --skip-build to install dependencies and build automatically.")
|
||||
sys.exit(1)
|
||||
|
|
|
|||
|
|
@ -2161,28 +2161,41 @@ function Clear-ElectronBuildCache {
|
|||
return $removed
|
||||
}
|
||||
|
||||
# True when node_modules\electron\dist holds a usable Electron binary.
|
||||
# electron-builder reads the binary from build.electronDist
|
||||
# (node_modules\electron\dist) 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.
|
||||
# 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.
|
||||
function Get-ElectronDir {
|
||||
param([string]$InstallDir)
|
||||
$desktopLocal = Join-Path $InstallDir 'apps\desktop\node_modules\electron'
|
||||
if (Test-Path -LiteralPath $desktopLocal) { return $desktopLocal }
|
||||
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.
|
||||
function Test-ElectronDist {
|
||||
param([string]$InstallDir)
|
||||
$distExe = Join-Path $InstallDir 'node_modules\electron\dist\electron.exe'
|
||||
$electronDir = Get-ElectronDir -InstallDir $InstallDir
|
||||
$distExe = Join-Path $electronDir 'dist\electron.exe'
|
||||
return (Test-Path -LiteralPath $distExe)
|
||||
}
|
||||
|
||||
# (Re)populate node_modules\electron\dist via electron's own downloader.
|
||||
# (Re)populate the desktop 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/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.
|
||||
# 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
|
||||
|
|
@ -2193,7 +2206,7 @@ function Restore-ElectronDist {
|
|||
param([string]$InstallDir, [string]$Mirror)
|
||||
if (Test-ElectronDist -InstallDir $InstallDir) { return $true }
|
||||
|
||||
$electronDir = Join-Path $InstallDir 'node_modules\electron'
|
||||
$electronDir = Get-ElectronDir -InstallDir $InstallDir
|
||||
$distExe = Join-Path $electronDir 'dist\electron.exe'
|
||||
$installer = Join-Path $electronDir 'install.js'
|
||||
if (-not (Test-Path -LiteralPath $installer)) { return $false }
|
||||
|
|
|
|||
|
|
@ -2407,15 +2407,31 @@ _desktop_pack() {
|
|||
# failed, and we never override a user-pinned ELECTRON_MIRROR.
|
||||
DESKTOP_ELECTRON_FALLBACK_MIRROR="https://npmmirror.com/mirrors/electron/"
|
||||
|
||||
# True (returns 0) when node_modules/electron/dist holds a usable Electron
|
||||
# binary. electron-builder reads the binary from build.electronDist
|
||||
# (node_modules/electron/dist) 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.
|
||||
# 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_dir() {
|
||||
local install_dir="$1"
|
||||
if [ -d "$install_dir/apps/desktop/node_modules/electron" ]; then
|
||||
printf '%s\n' "$install_dir/apps/desktop/node_modules/electron"
|
||||
else
|
||||
printf '%s\n' "$install_dir/node_modules/electron"
|
||||
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.
|
||||
_electron_dist_ok() {
|
||||
local install_dir="$1"
|
||||
local electron_dir="$install_dir/node_modules/electron"
|
||||
local electron_dir
|
||||
electron_dir="$(_electron_dir "$install_dir")"
|
||||
if [ "$OS" = "macos" ]; then
|
||||
[ -e "$electron_dir/dist/Electron.app/Contents/MacOS/Electron" ]
|
||||
else
|
||||
|
|
@ -2423,16 +2439,16 @@ _electron_dist_ok() {
|
|||
fi
|
||||
}
|
||||
|
||||
# (Re)populate node_modules/electron/dist via electron's own downloader.
|
||||
# (Re)populate the desktop 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/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.
|
||||
# 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
|
||||
|
|
@ -2442,7 +2458,8 @@ _electron_dist_ok() {
|
|||
_restore_electron_dist() {
|
||||
local install_dir="$1"
|
||||
local mirror="${2:-}"
|
||||
local electron_dir="$install_dir/node_modules/electron"
|
||||
local electron_dir
|
||||
electron_dir="$(_electron_dir "$install_dir")"
|
||||
_electron_dist_ok "$install_dir" && return 0
|
||||
|
||||
[ -f "$electron_dir/install.js" ] || return 1
|
||||
|
|
|
|||
|
|
@ -45,6 +45,9 @@ ACP_REGISTRY_MANIFEST = REPO_ROOT / "acp_registry" / "agent.json"
|
|||
|
||||
# Auto-extracted from noreply emails + manual overrides
|
||||
AUTHOR_MAP = {
|
||||
"59806492+sitkarev@users.noreply.github.com": "sitkarev",
|
||||
"zheng@omegasys.eu": "omegazheng",
|
||||
"220877172+james47kjv@users.noreply.github.com": "james47kjv",
|
||||
"yuhanglin@YuhangdeMac-mini.local": "1960697431",
|
||||
"despitemeguru@gmail.com": "definitelynotguru",
|
||||
"chaslui@outlook.com": "ChasLui",
|
||||
|
|
|
|||
|
|
@ -616,6 +616,33 @@ def test_electron_dist_ok_per_platform(tmp_path, monkeypatch, platform, rel):
|
|||
assert cli_main._electron_dist_ok(tmp_path) is True
|
||||
|
||||
|
||||
def test_electron_dir_prefers_workspace_local_package(tmp_path):
|
||||
"""npm may nest electron under apps/desktop; resolve there over the root hoist."""
|
||||
root_electron = tmp_path / "node_modules" / "electron"
|
||||
local_electron = tmp_path / "apps" / "desktop" / "node_modules" / "electron"
|
||||
root_electron.mkdir(parents=True)
|
||||
local_electron.mkdir(parents=True)
|
||||
|
||||
assert cli_main._electron_dir(tmp_path) == local_electron
|
||||
|
||||
|
||||
def test_electron_dir_falls_back_to_root_hoist(tmp_path):
|
||||
"""When npm hoists electron to the repo root, resolve there."""
|
||||
root_electron = tmp_path / "node_modules" / "electron"
|
||||
root_electron.mkdir(parents=True)
|
||||
|
||||
assert cli_main._electron_dir(tmp_path) == root_electron
|
||||
|
||||
|
||||
def test_electron_dist_ok_finds_workspace_local_binary(tmp_path, monkeypatch):
|
||||
"""A nested apps/desktop electron with a valid binary counts as ok."""
|
||||
monkeypatch.setattr(cli_main.sys, "platform", "linux")
|
||||
binp = tmp_path / "apps" / "desktop" / "node_modules" / "electron" / "dist" / "electron"
|
||||
binp.parent.mkdir(parents=True)
|
||||
binp.write_text("", encoding="utf-8")
|
||||
assert cli_main._electron_dist_ok(tmp_path) is True
|
||||
|
||||
|
||||
def test_redownload_electron_dist_noop_when_present(tmp_path, monkeypatch):
|
||||
"""Already-healthy dist → no download, so an unrelated build failure can't
|
||||
trigger a needless ~200 MB refetch."""
|
||||
|
|
|
|||
|
|
@ -94,3 +94,42 @@ def test_lockfile_resolves_the_pinned_electron():
|
|||
f"but the pin is {spec!r}; run `npm install --package-lock-only` so "
|
||||
"`npm ci` stays consistent."
|
||||
)
|
||||
|
||||
|
||||
def test_electron_dist_matches_lockfile_install_location():
|
||||
"""build.electronDist must point at where the lockfile installs Electron.
|
||||
|
||||
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'."
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue