fix(desktop,install): keep bundled Node ahead of system Node on Windows
Two paths let a pre-existing system Node win over the Hermes-managed one. The desktop backend spawn built its managed-Node PATH entry as `<home>/node/bin` only. That is the POSIX layout install.sh produces; install.ps1 unpacks portable Node straight into `%LOCALAPPDATA%\hermes\node` with node.exe at the root and no `bin\`. On Windows the entry therefore pointed at a directory that does not exist, and the backend fell through to whatever Node was already on PATH. main.ts already had the correct platform-ordered list, behind a "keep this in sync with iter_hermes_node_dirs()" comment on a second copy of the rule. The two copies had drifted. Export the ordering from backend-env.ts and have main.ts consume it so there is one source of truth on the Node side (the Electron main process cannot import hermes_constants.py, so a mirror is unavoidable — but one mirror, not two). install.ps1 appended the node dir to the persisted User PATH instead of prepending it. The session PATH was already prepended correctly, so this only bit later processes: any shell opened after install, and a standalone hermes-setup.exe run that inherits User PATH rather than a curated env, both resolved a system Node ahead of the bundled one. Not a bug, for the record: update.rs's prepend list omits the same Windows root, but it inherits PATH from the desktop, which supplies the correct entries — so it is redundant rather than broken, and no installer rebuild is needed for this fix. Tests: managed dirs lead with the platform-native layout while always offering both shapes, empty without a home, and every managed dir outranks the inherited PATH on darwin and win32. The three existing tests that pinned `entries[1]` by index asserted the old single-dir shape and now assert the relationship instead. install.ps1 has no behavioral test here: CI has no PowerShell host, and AGENTS.md bans source-reading tests (the neighbouring test_install_ps1_node_path_for_npm.py predates that rule).
This commit is contained in:
@@ -7,6 +7,7 @@ import {
|
||||
appendUniquePathEntries,
|
||||
buildDesktopBackendEnv,
|
||||
buildDesktopBackendPath,
|
||||
hermesManagedNodePathEntries,
|
||||
normalizeHermesHomeRoot,
|
||||
pathEnvKey,
|
||||
POSIX_SANE_PATH_ENTRIES
|
||||
@@ -22,8 +23,12 @@ test('desktop backend PATH adds Hermes-managed bins and missing POSIX sane entri
|
||||
})
|
||||
|
||||
const entries = result.split(':')
|
||||
assert.equal(entries[0], '/Users/test/.hermes/node/bin')
|
||||
assert.equal(entries[1], '/Users/test/.hermes/hermes-agent/venv/bin')
|
||||
// Both managed-Node layouts lead, POSIX-native shape first, then the venv.
|
||||
assert.deepEqual(entries.slice(0, 3), [
|
||||
'/Users/test/.hermes/node/bin',
|
||||
'/Users/test/.hermes/node',
|
||||
'/Users/test/.hermes/hermes-agent/venv/bin'
|
||||
])
|
||||
assert.ok(entries.includes('/opt/homebrew/bin'), 'Apple Silicon Homebrew bin is added')
|
||||
assert.ok(entries.includes('/opt/homebrew/sbin'), 'Apple Silicon Homebrew sbin is added')
|
||||
assert.ok(entries.includes('/usr/local/sbin'), 'missing standard sbin is added')
|
||||
@@ -33,6 +38,56 @@ test('desktop backend PATH adds Hermes-managed bins and missing POSIX sane entri
|
||||
}
|
||||
})
|
||||
|
||||
test('managed Node dirs lead with the platform-native layout but always offer both', () => {
|
||||
const posix = hermesManagedNodePathEntries('/Users/test/.hermes', {
|
||||
platform: 'darwin',
|
||||
pathModule: path.posix
|
||||
})
|
||||
|
||||
const windows = hermesManagedNodePathEntries('C:\\Users\\test\\AppData\\Local\\hermes', {
|
||||
platform: 'win32',
|
||||
pathModule: path.win32
|
||||
})
|
||||
|
||||
// install.sh uses node/bin; install.ps1 unpacks node.exe into node\ itself.
|
||||
// Both shapes are always emitted so migrated installs keep resolving.
|
||||
assert.deepEqual(posix, ['/Users/test/.hermes/node/bin', '/Users/test/.hermes/node'])
|
||||
assert.deepEqual(windows, [
|
||||
'C:\\Users\\test\\AppData\\Local\\hermes\\node',
|
||||
'C:\\Users\\test\\AppData\\Local\\hermes\\node\\bin'
|
||||
])
|
||||
})
|
||||
|
||||
test('managed Node dirs are empty without a Hermes home', () => {
|
||||
assert.deepEqual(hermesManagedNodePathEntries(undefined, { platform: 'darwin', pathModule: path.posix }), [])
|
||||
assert.deepEqual(hermesManagedNodePathEntries('', { platform: 'win32', pathModule: path.win32 }), [])
|
||||
})
|
||||
|
||||
test('every managed Node dir outranks the inherited PATH on both platforms', () => {
|
||||
for (const [platform, pathModule, home, inherited, delimiter] of [
|
||||
['darwin', path.posix, '/Users/test/.hermes', '/usr/local/bin:/usr/bin', ':'],
|
||||
['win32', path.win32, 'C:\\hermes', 'C:\\Program Files\\nodejs;C:\\Windows\\System32', ';']
|
||||
] as const) {
|
||||
const entries = buildDesktopBackendPath({
|
||||
hermesHome: home,
|
||||
venvRoot: null,
|
||||
currentPath: inherited,
|
||||
platform,
|
||||
pathModule
|
||||
}).split(delimiter)
|
||||
|
||||
const managed = hermesManagedNodePathEntries(home, { platform, pathModule })
|
||||
const firstInherited = Math.min(...inherited.split(delimiter).map(entry => entries.indexOf(entry)))
|
||||
|
||||
for (const dir of managed) {
|
||||
assert.ok(
|
||||
entries.indexOf(dir) >= 0 && entries.indexOf(dir) < firstInherited,
|
||||
`${dir} must precede the inherited PATH on ${platform}`
|
||||
)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
test('desktop backend PATH preserves first occurrence and avoids duplicates', () => {
|
||||
const result = buildDesktopBackendPath({
|
||||
hermesHome: '/Users/test/.hermes',
|
||||
@@ -64,7 +119,9 @@ test('buildDesktopBackendEnv extends PYTHONPATH and backend PATH together', () =
|
||||
})
|
||||
|
||||
assert.equal(env.PYTHONPATH, '/repo/hermes-agent:/existing/pythonpath')
|
||||
assert.ok(env.PATH.startsWith('/Users/test/.hermes/node/bin:/Users/test/.hermes/hermes-agent/venv/bin:'))
|
||||
assert.ok(
|
||||
env.PATH.startsWith('/Users/test/.hermes/node/bin:/Users/test/.hermes/node:/Users/test/.hermes/hermes-agent/venv/bin:')
|
||||
)
|
||||
assert.ok(env.PATH.includes('/opt/homebrew/bin'))
|
||||
})
|
||||
|
||||
@@ -115,7 +172,13 @@ test('Windows PATH casing and delimiter are preserved without POSIX sane entries
|
||||
|
||||
assert.equal(pathEnvKey({ Path: 'x' }, 'win32'), 'Path')
|
||||
assert.equal(env.PATH, undefined)
|
||||
assert.ok(env.Path.startsWith('C:\\Users\\test\\AppData\\Local\\hermes\\node\\bin;'))
|
||||
// Windows leads with the portable layout (install.ps1 unpacks node.exe
|
||||
// straight into node\, no bin\), then the POSIX shape for migrated installs.
|
||||
assert.ok(
|
||||
env.Path.startsWith(
|
||||
'C:\\Users\\test\\AppData\\Local\\hermes\\node;C:\\Users\\test\\AppData\\Local\\hermes\\node\\bin;'
|
||||
)
|
||||
)
|
||||
assert.ok(env.Path.includes('\\venv\\Scripts;'))
|
||||
assert.ok(env.Path.includes(';C:\\Windows\\System32;C:\\Windows'))
|
||||
assert.equal(env.Path.includes('/opt/homebrew/bin'), false)
|
||||
|
||||
@@ -60,6 +60,34 @@ function appendUniquePathEntries(entries, { delimiter = path.delimiter } = {}) {
|
||||
return ordered.join(delimiter)
|
||||
}
|
||||
|
||||
/**
|
||||
* Hermes-managed Node.js directories, in preferred lookup order.
|
||||
*
|
||||
* There are two on-disk layouts. `scripts/install.ps1` unpacks portable Node
|
||||
* straight into `%LOCALAPPDATA%\hermes\node` (node.exe at the root, no `bin\`);
|
||||
* `scripts/install.sh` and the node-bootstrap helper use the POSIX
|
||||
* `$HERMES_HOME/node/bin`. Emit BOTH on every platform so mixed and migrated
|
||||
* installs resolve, leading with the layout native to the current platform.
|
||||
*
|
||||
* This is the single source of truth for the ordering rule on the Node side —
|
||||
* `main.ts` imports it rather than keeping its own copy. Mirrors
|
||||
* `iter_hermes_node_dirs()` in hermes_constants.py, which the Electron main
|
||||
* process cannot import.
|
||||
*/
|
||||
function hermesManagedNodePathEntries(
|
||||
hermesHome,
|
||||
{ platform = process.platform, pathModule = pathModuleForPlatform(platform) }: any = {}
|
||||
) {
|
||||
if (!hermesHome) {
|
||||
return []
|
||||
}
|
||||
|
||||
const root = pathModule.join(hermesHome, 'node')
|
||||
const bin = pathModule.join(root, 'bin')
|
||||
|
||||
return platform === 'win32' ? [root, bin] : [bin, root]
|
||||
}
|
||||
|
||||
function buildDesktopBackendPath({
|
||||
hermesHome,
|
||||
venvRoot,
|
||||
@@ -68,11 +96,11 @@ function buildDesktopBackendPath({
|
||||
pathModule = pathModuleForPlatform(platform)
|
||||
}: any = {}) {
|
||||
const delimiter = delimiterForPlatform(platform)
|
||||
const hermesNodeBin = hermesHome ? pathModule.join(hermesHome, 'node', 'bin') : null
|
||||
const hermesNodeDirs = hermesManagedNodePathEntries(hermesHome, { platform, pathModule })
|
||||
const venvBin = venvRoot ? pathModule.join(venvRoot, platform === 'win32' ? 'Scripts' : 'bin') : null
|
||||
const saneEntries = platform === 'win32' ? [] : POSIX_SANE_PATH_ENTRIES
|
||||
|
||||
return appendUniquePathEntries([hermesNodeBin, venvBin, currentPath, saneEntries], { delimiter })
|
||||
return appendUniquePathEntries([hermesNodeDirs, venvBin, currentPath, saneEntries], { delimiter })
|
||||
}
|
||||
|
||||
function normalizeHermesHomeRoot(hermesHome, { pathModule = pathModuleForPlatform(process.platform) }: any = {}) {
|
||||
@@ -126,6 +154,7 @@ export {
|
||||
buildDesktopBackendEnv,
|
||||
buildDesktopBackendPath,
|
||||
delimiterForPlatform,
|
||||
hermesManagedNodePathEntries,
|
||||
normalizeHermesHomeRoot,
|
||||
pathEnvKey,
|
||||
POSIX_SANE_PATH_ENTRIES
|
||||
|
||||
@@ -35,7 +35,7 @@ import { classifyActiveRuntime } from './active-runtime-state'
|
||||
import { stopBackendChild as stopBackendChildImpl } from './backend-child'
|
||||
import { dashboardFallbackArgs, sourceDeclaresServe } from './backend-command'
|
||||
import { createBackendConnectionState } from './backend-connection-state'
|
||||
import { buildDesktopBackendEnv, normalizeHermesHomeRoot } from './backend-env'
|
||||
import { buildDesktopBackendEnv, hermesManagedNodePathEntries, normalizeHermesHomeRoot } from './backend-env'
|
||||
import { isReauthRequiredError, waitForHermesReady } from './backend-health'
|
||||
import {
|
||||
canImportHermesCli,
|
||||
@@ -574,19 +574,10 @@ function resolveHermesHome() {
|
||||
|
||||
const HERMES_HOME = resolveHermesHome()
|
||||
|
||||
function hermesManagedNodePathEntries() {
|
||||
// NOTE: keep this ordering in sync with iter_hermes_node_dirs() in
|
||||
// hermes_constants.py — this Node main process cannot import the Python
|
||||
// module, so the platform-ordering rule is mirrored here.
|
||||
const root = path.join(HERMES_HOME, 'node')
|
||||
const bin = path.join(root, 'bin')
|
||||
const entries = IS_WINDOWS ? [root, bin] : [bin, root]
|
||||
|
||||
return entries.filter(directoryExists)
|
||||
}
|
||||
|
||||
function pathWithHermesManagedNode(...entries) {
|
||||
return [...hermesManagedNodePathEntries(), ...entries, process.env.PATH].filter(Boolean).join(path.delimiter)
|
||||
const managed = hermesManagedNodePathEntries(HERMES_HOME).filter(directoryExists)
|
||||
|
||||
return [...managed, ...entries, process.env.PATH].filter(Boolean).join(path.delimiter)
|
||||
}
|
||||
|
||||
// ACTIVE_HERMES_ROOT — the canonical mutable Hermes install. Same path
|
||||
|
||||
@@ -1133,11 +1133,17 @@ function Test-Node {
|
||||
# Persist to User PATH so fresh shells (and future stages
|
||||
# in cross-process driver mode) see it. Matches the
|
||||
# pattern Install-Git uses for PortableGit.
|
||||
#
|
||||
# PREPEND, don't append. Appending leaves a pre-existing
|
||||
# system Node ahead of the bundled one in every new shell,
|
||||
# so anything launched without a curated environment (a
|
||||
# standalone hermes-setup.exe run, a user typing `npm`)
|
||||
# silently resolves the wrong Node. Bundled must win.
|
||||
$nodeDir = "$HermesHome\node"
|
||||
$userPath = [Environment]::GetEnvironmentVariable("Path", "User")
|
||||
$userPathItems = if ($userPath) { $userPath -split ";" } else { @() }
|
||||
if ($userPathItems -notcontains $nodeDir) {
|
||||
$userPathItems += $nodeDir
|
||||
$userPathItems = @($nodeDir) + $userPathItems
|
||||
[Environment]::SetEnvironmentVariable("Path", ($userPathItems -join ";"), "User")
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user