diff --git a/src/main/runtime/harness-runtime.ts b/src/main/runtime/harness-runtime.ts index d398bb072..233a8fe20 100644 --- a/src/main/runtime/harness-runtime.ts +++ b/src/main/runtime/harness-runtime.ts @@ -197,6 +197,37 @@ export function buildHarnessArguments( ] } +/** + * The captured PATH, looked up the way Windows actually stores it. + * + * `resolveShellEnvironment()` returns a plain object built by `parseEnvOutput`, + * keyed by whatever case the environment block reported — and Windows does not + * normalise that case, it follows the registry value name. A machine whose + * PATH value name is stored lowercase yields the key `path`, which an + * exact-case read misses entirely, launching the Harness with an empty PATH + * (issue #232). `process.env` never has this problem because Node makes it + * case-insensitive on win32 — but spreading it into a plain object keeps + * only the stored casing, so every copy needs this lookup too. + * + * POSIX keeps the exact read: there `path` and `PATH` are genuinely + * different variables. + */ +export function resolveEnvironmentPath( + environment: NodeJS.ProcessEnv, + platform: NodeJS.Platform = process.platform +): string { + if (platform !== 'win32') return environment.PATH ?? '' + // Exact-case reads first; the scan is the last resort for other casings. + // A real Windows block stores a single casing, so the order between them + // is never observable outside synthetic inputs. + const direct = environment.Path ?? environment.PATH + if (direct !== undefined) return direct + for (const [name, value] of Object.entries(environment)) { + if (/^path$/iu.test(name) && value !== undefined) return value + } + return '' +} + export function buildHarnessSpawnOptions( launchDirectory: string, dshHome: string, @@ -233,7 +264,7 @@ export function buildHarnessSpawnOptions( // the dedicated lock-recovery runner instead (see pnpm-runner.mjs). npm_config_side_effects_cache: 'false', PNPM_CONFIG_SIDE_EFFECTS_CACHE: 'false', - [pathKey]: environment[pathKey] ?? environment.PATH ?? '' + [pathKey]: resolveEnvironmentPath(environment, platform) }, stdio: ['pipe', 'pipe', 'pipe'], windowsHide: true, diff --git a/src/main/runtime/profile-plugin-command.ts b/src/main/runtime/profile-plugin-command.ts index 2b5b8b8ef..d976476a9 100644 --- a/src/main/runtime/profile-plugin-command.ts +++ b/src/main/runtime/profile-plugin-command.ts @@ -2,6 +2,7 @@ import { spawn } from 'node:child_process' import { existsSync } from 'node:fs' import { chmod, mkdir, readdir, stat, writeFile } from 'node:fs/promises' import { delimiter, dirname, join } from 'node:path' +import { resolveEnvironmentPath } from './harness-runtime' const PROFILE = 'web' const OPERATION_TIMEOUT_MS = 15 * 60 * 1000 @@ -146,11 +147,10 @@ export function buildProfilePluginCommandEnvironment( const result = { ...environment } delete result.ELECTRON_RUN_AS_NODE - const currentPath = - (process.platform === 'win32' ? result.Path : result.PATH) ?? - result.PATH ?? - result.Path ?? - '' + // The spread above keeps only the casing the OS block actually stores — + // even for `process.env`, whose case-insensitivity does not survive a + // copy — so the PATH read must be case-insensitive itself (issue #232). + const currentPath = resolveEnvironmentPath(result) const parts = currentPath.split(delimiter).filter(Boolean) const additions = [shimDirectory, dirname(nodeExecutablePath)].filter( (directory) => !parts.includes(directory) diff --git a/test/profile-plugin-command.test.ts b/test/profile-plugin-command.test.ts index 2bcfeb17e..574f4dd74 100644 --- a/test/profile-plugin-command.test.ts +++ b/test/profile-plugin-command.test.ts @@ -3,6 +3,7 @@ import { join } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' import { buildPnpmShimCommand, + buildProfilePluginCommandEnvironment, buildProfilePluginRemoveArguments, diagnosticLine, removeProfilePluginWithDsh @@ -142,3 +143,24 @@ describe('profile pnpm shim and failure reporting', () => { expect(diagnosticLine(' ')).toBeUndefined() }) }) + +describe('buildProfilePluginCommandEnvironment', () => { + it('keeps the user PATH when the environment block stores it lowercase', () => { + // Spreading `process.env` keeps only the casing the OS block stores, so + // on a machine whose registry PATH value name is lowercase the previous + // exact-case read produced an empty base PATH — plugin commands then ran + // with just the shim and bundled-node directories (issue #232). + const userPath = 'C:\\Windows\\System32;C:\\Users\\tester\\bin' + const result = buildProfilePluginCommandEnvironment( + { path: userPath }, + 'C:\\shim', + 'C:\\bundled\\node.exe' + ) + if (process.platform === 'win32') { + expect(result.PATH).toContain(userPath) + } else { + // POSIX: `path` is a different variable and must stay out of PATH. + expect(result.PATH).not.toContain(userPath) + } + }) +}) diff --git a/test/runtime.test.ts b/test/runtime.test.ts index ae35fea54..09d7b30e2 100644 --- a/test/runtime.test.ts +++ b/test/runtime.test.ts @@ -14,6 +14,7 @@ import { extractSlotConflictName, formatExitCode, isHarnessStartupProbeHealthy, + resolveEnvironmentPath, resolveShellEnvironment, updateReadyStability } from '../src/main/runtime/harness-runtime' @@ -133,6 +134,25 @@ describe('Harness launch contract', () => { } }) + it('finds the Windows PATH when the environment block stores it lowercase', () => { + // Windows environment variable names are case-insensitive and the captured + // block is not normalised, so a machine whose registry PATH value name is + // lowercase hands `resolveShellEnvironment()` the key `path`. An exact-case + // read misses it and the Harness launches with no PATH at all — every + // PATH-resolved tool call fails with ENOENT (issue #232). + const userPath = 'C:\\Windows\\System32;C:\\Users\\tester\\bin' + const options = buildHarnessSpawnOptions('C:\\launch-root', 'C:\\harness', 'win32', { + path: userPath + }) + // Every key spelling PATH must carry the value: whichever of them survives + // Node's win32 case-dedupe, the child receives the user's PATH. + const pathEntries = Object.entries(options.env ?? {}).filter(([name]) => + /^path$/iu.test(name) + ) + expect(pathEntries.length).toBeGreaterThan(0) + for (const [, value] of pathEntries) expect(value).toBe(userPath) + }) + it('passes the internal-loader flag directly to bundled Node.js', () => { expect( buildNodeArguments( @@ -247,8 +267,8 @@ describe('shell environment resolution', () => { () => { const env = resolveShellEnvironment() expect(env).toBeDefined() - // Windows may preserve the conventional mixed-case key. - expect(env.PATH ?? env.Path).toBeTruthy() + // Windows preserves whatever casing the environment block stores. + expect(resolveEnvironmentPath(env)).toBeTruthy() }, 20_000 ) @@ -262,7 +282,7 @@ describe('shell environment resolution', () => { it('produces a PATH that includes platform-standard system directories', () => { const env = resolveShellEnvironment() - const path = env.PATH ?? env.Path ?? '' + const path = resolveEnvironmentPath(env) if (process.platform === 'win32') { expect(path).toMatch(/[A-Za-z]:\\/) } else { diff --git a/test/shell-environment-encoding.test.ts b/test/shell-environment-encoding.test.ts index 2496c8464..fed8814a1 100644 --- a/test/shell-environment-encoding.test.ts +++ b/test/shell-environment-encoding.test.ts @@ -1,5 +1,8 @@ import { describe, expect, it } from 'vitest' -import { withoutUndecodableValues } from '../src/main/runtime/harness-runtime' +import { + resolveEnvironmentPath, + withoutUndecodableValues +} from '../src/main/runtime/harness-runtime' /** * Windows PowerShell writes stdout in the console codepage. Decoding that as @@ -42,4 +45,17 @@ describe('captured shell environment', () => { it('preserves an empty value', () => { expect(withoutUndecodableValues({ EMPTY: '' }, {})).toEqual({ EMPTY: '' }) }) + + // Windows does not normalise environment variable name casing either — the + // captured block follows the registry value name, so PATH can arrive as + // `path` or any other spelling and an exact-case read misses it (issue #232). + it('reads PATH case-insensitively only on Windows', () => { + expect(resolveEnvironmentPath({ path: 'C:\\lower' }, 'win32')).toBe('C:\\lower') + expect(resolveEnvironmentPath({ PaTh: 'C:\\mixed' }, 'win32')).toBe('C:\\mixed') + // POSIX: `path` is a different variable, not a spelling of PATH. + expect(resolveEnvironmentPath({ path: '/ignored' }, 'linux')).toBe('') + expect(resolveEnvironmentPath({ PATH: '/usr/bin', path: '/ignored' }, 'linux')).toBe( + '/usr/bin' + ) + }) })