From c96ded8dfd18c1992e0123c3d3151a48f1efaaa7 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Tue, 11 Aug 2026 11:50:21 -0700 Subject: [PATCH] fix(startup): restore Windows PATH before shell changes (#13792) --- config/tsconfig.cli.json | 1 + src/main/startup/hydrate-shell-path.test.ts | 4 + src/main/startup/hydrate-shell-path.ts | 43 ++--- .../windows-shell-path-ownership.test.ts | 50 ++++++ .../startup/windows-shell-path-ownership.ts | 68 ++++++++ ...ows-shell-path-restoration.windows.test.ts | 165 ++++++++++++++++++ 6 files changed, 303 insertions(+), 28 deletions(-) create mode 100644 src/main/startup/windows-shell-path-ownership.test.ts create mode 100644 src/main/startup/windows-shell-path-ownership.ts create mode 100644 src/main/startup/windows-shell-path-restoration.windows.test.ts diff --git a/config/tsconfig.cli.json b/config/tsconfig.cli.json index 747780e416b..a5d0acd6987 100644 --- a/config/tsconfig.cli.json +++ b/config/tsconfig.cli.json @@ -75,6 +75,7 @@ "../src/main/openclaude/hook-service.ts", "../src/main/rolling-file-backup.ts", "../src/main/startup/hydrate-shell-path.ts", + "../src/main/startup/windows-shell-path-ownership.ts", // Why: serve-electron-flag-parity.test.ts checks the Electron-side serve argv rewrite against this // project's serve spec; the module has no imports, so listing it pulls in nothing else. "../src/main/startup/serve-mode-argv.ts", diff --git a/src/main/startup/hydrate-shell-path.test.ts b/src/main/startup/hydrate-shell-path.test.ts index 316baad7280..f096dd03ed3 100644 --- a/src/main/startup/hydrate-shell-path.test.ts +++ b/src/main/startup/hydrate-shell-path.test.ts @@ -182,6 +182,10 @@ describe('hydrateShellPath', () => { describe('mergePathSegments', () => { const originalPath = process.env.PATH + beforeEach(() => { + _resetHydrateShellPathCache() + }) + afterEach(() => { if (originalPath === undefined) { delete process.env.PATH diff --git a/src/main/startup/hydrate-shell-path.ts b/src/main/startup/hydrate-shell-path.ts index 87747399615..9cf7dc75608 100644 --- a/src/main/startup/hydrate-shell-path.ts +++ b/src/main/startup/hydrate-shell-path.ts @@ -2,6 +2,7 @@ import { spawn } from 'node:child_process' import { delimiter, win32 as pathWin32 } from 'node:path' import type { ShellHydrationFailureReason } from '../../shared/types' import { resolveWindowsShellStartupFamily } from '../../shared/windows-terminal-shell' +import { WindowsShellPathOwnership, windowsPathSegmentKey } from './windows-shell-path-ownership' // Why: GUI-launched Electron can miss PATH entries added by shell profiles. // Tools installed into ~/.opencode/bin, ~/.cargo/bin, pyenv/volta/fnm @@ -35,7 +36,7 @@ let configuredWindowsShell = 'powershell.exe' let configuredWindowsGitBashPath: string | null = null let configuredWindowsFallbackShell: string | null = null let windowsShellConfigurationVersion = 0 -const windowsIntroducedPathKeys = new Set() +const windowsPathOwnership = new WindowsShellPathOwnership() /** @internal - tests need a clean hydration cache between cases. */ export function _resetHydrateShellPathCache(): void { @@ -45,7 +46,7 @@ export function _resetHydrateShellPathCache(): void { configuredWindowsGitBashPath = null configuredWindowsFallbackShell = null windowsShellConfigurationVersion = 0 - windowsIntroducedPathKeys.clear() + windowsPathOwnership.reset() } function pickShell(): string | null { @@ -211,6 +212,9 @@ export function hydrateShellPath(options: HydrateOptions = {}): Promise { + if (platform === 'win32') { + windowsPathOwnership.restore(process.env) + } const result = await spawner(shell) if (!result.ok && result.failureReason === 'spawn_error' && fallbackShell) { return spawner(fallbackShell) @@ -248,7 +252,7 @@ export function configureWindowsShellPathHydration( ) { return } - clearWindowsIntroducedPathSegments() + windowsPathOwnership.restore(process.env) configuredWindowsShell = next configuredWindowsGitBashPath = gitBashPath configuredWindowsFallbackShell = fallbackShell @@ -268,25 +272,6 @@ function uniquePathSegments(segments: string[], pathKey: (segment: string) => st }) } -function windowsPathKey(segment: string): string { - const normalized = pathWin32.normalize(segment) - const root = pathWin32.parse(normalized).root - const withoutTrailingSlash = - normalized.length > root.length ? normalized.replace(/[\\/]+$/, '') : normalized - return withoutTrailingSlash.toLowerCase() -} - -function clearWindowsIntroducedPathSegments(): void { - if (process.platform !== 'win32' || windowsIntroducedPathKeys.size === 0) { - return - } - const currentSegments = (process.env.PATH ?? '').split(pathWin32.delimiter).filter(Boolean) - process.env.PATH = currentSegments - .filter((segment) => !windowsIntroducedPathKeys.has(windowsPathKey(segment))) - .join(pathWin32.delimiter) - windowsIntroducedPathKeys.clear() -} - /** * Promote shell-discovered PATH segments to the front of process.env.PATH, * preserving shell ordering and avoiding duplicates. Returns the segments that @@ -296,11 +281,14 @@ export function mergePathSegments(segments: string[]): string[] { if (segments.length === 0) { return [] } - const current = process.env.PATH ?? '' + if (process.platform === 'win32') { + windowsPathOwnership.restore(process.env) + } + const current = process.env.PATH ?? process.env.Path ?? '' const pathDelimiter = process.platform === 'win32' ? pathWin32.delimiter : delimiter const currentSegments = current.split(pathDelimiter).filter(Boolean) const pathKey = - process.platform === 'win32' ? windowsPathKey : (segment: string): string => segment + process.platform === 'win32' ? windowsPathSegmentKey : (segment: string): string => segment const shellSegments = uniquePathSegments(segments, pathKey) const shellSegmentSet = new Set(shellSegments.map(pathKey)) const existing = new Set(currentSegments.map(pathKey)) @@ -316,11 +304,10 @@ export function mergePathSegments(segments: string[]): string[] { // Why: shell-provided entries must win over hardcoded packaged-app fallbacks. // A seeded fallback can point at a stale CLI while the user's shell resolves // a healthy one from the same directory list in a different order. - process.env.PATH = next if (process.platform === 'win32') { - for (const segment of added) { - windowsIntroducedPathKeys.add(windowsPathKey(segment)) - } + windowsPathOwnership.apply(process.env, next) + } else { + process.env.PATH = next } return added } diff --git a/src/main/startup/windows-shell-path-ownership.test.ts b/src/main/startup/windows-shell-path-ownership.test.ts new file mode 100644 index 00000000000..e4e4b128d93 --- /dev/null +++ b/src/main/startup/windows-shell-path-ownership.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from 'vitest' +import { WindowsShellPathOwnership, windowsPathSegmentKey } from './windows-shell-path-ownership' + +describe('Windows shell PATH ownership', () => { + it.each([ + ['C:\\', 'c:\\'], + ['C:\\Tools\\', 'c:\\tools'], + ['c:/TOOLS', 'c:\\tools'], + ['\\\\Server\\Share\\', '\\\\server\\share\\'], + ['\\\\SERVER\\Share', '\\\\server\\share\\'] + ])('normalizes %s to %s', (segment, expected) => { + expect(windowsPathSegmentKey(segment)).toBe(expected) + }) + + it.each(['PATH', 'Path'])( + 'restores the complete %s baseline without changing its casing', + (key) => { + const env: Record = { + [key]: 'C:\\B;c:\\b;C:\\;\\\\Server\\Share\\' + } + const ownership = new WindowsShellPathOwnership() + ownership.apply(env, 'C:\\Profile;C:\\B;C:\\;\\\\Server\\Share\\') + + ownership.restore(env) + + expect(env).toEqual({ [key]: 'C:\\B;c:\\b;C:\\;\\\\Server\\Share\\' }) + } + ) + + it('preserves PATH entries appended outside shell hydration', () => { + const env = { Path: 'C:\\B;C:\\A' } + const ownership = new WindowsShellPathOwnership() + ownership.apply(env, 'C:\\Profile;C:\\A;C:\\B') + env.Path += ';C:\\NewlyInstalled' + + ownership.restore(env) + + expect(env.Path).toBe('C:\\B;C:\\A;C:\\NewlyInstalled') + }) + + it('updates the first effective key when both Windows casings exist', () => { + const env = { Path: 'C:\\B;C:\\A', PATH: 'C:\\ignored' } + const ownership = new WindowsShellPathOwnership() + ownership.apply(env, 'C:\\Profile;C:\\A;C:\\B') + + ownership.restore(env) + + expect(env).toEqual({ Path: 'C:\\B;C:\\A', PATH: 'C:\\ignored' }) + }) +}) diff --git a/src/main/startup/windows-shell-path-ownership.ts b/src/main/startup/windows-shell-path-ownership.ts new file mode 100644 index 00000000000..8a4d63ef02a --- /dev/null +++ b/src/main/startup/windows-shell-path-ownership.ts @@ -0,0 +1,68 @@ +import { win32 as pathWin32 } from 'node:path' + +type PathEnvironment = Record + +type AppliedWindowsPath = { + appliedValue: string + baselineValue: string + pathKey: string +} + +function firstPathKey(env: PathEnvironment): string | undefined { + return Object.keys(env).find((key) => key.toLowerCase() === 'path' && env[key] !== undefined) +} + +export function windowsPathSegmentKey(segment: string): string { + const normalized = pathWin32.normalize(segment) + const root = pathWin32.parse(normalized).root + const withoutTrailingSlash = + normalized.length > root.length ? normalized.replace(/[\\/]+$/, '') : normalized + return withoutTrailingSlash.toLowerCase() +} + +function splitPath(pathValue: string): string[] { + return pathValue.split(pathWin32.delimiter).filter(Boolean) +} + +function externalAdditions(application: AppliedWindowsPath, currentValue: string): string[] { + if (currentValue === application.appliedValue) { + return [] + } + // Why: forced Windows preflight appends newly installed registry paths after hydration. + const appliedKeys = new Set(splitPath(application.appliedValue).map(windowsPathSegmentKey)) + return splitPath(currentValue).filter( + (segment) => !appliedKeys.has(windowsPathSegmentKey(segment)) + ) +} + +export class WindowsShellPathOwnership { + private application: AppliedWindowsPath | null = null + + reset(): void { + this.application = null + } + + restore(env: PathEnvironment): void { + const application = this.application + if (!application) { + return + } + const pathKey = firstPathKey(env) ?? application.pathKey + const currentValue = env[pathKey] ?? '' + const additions = externalAdditions(application, currentValue) + env[pathKey] = [application.baselineValue, ...additions] + .filter(Boolean) + .join(pathWin32.delimiter) + this.application = null + } + + apply(env: PathEnvironment, value: string): void { + const pathKey = firstPathKey(env) ?? 'Path' + this.application = { + appliedValue: value, + baselineValue: env[pathKey] ?? '', + pathKey + } + env[pathKey] = value + } +} diff --git a/src/main/startup/windows-shell-path-restoration.windows.test.ts b/src/main/startup/windows-shell-path-restoration.windows.test.ts new file mode 100644 index 00000000000..d612c3726eb --- /dev/null +++ b/src/main/startup/windows-shell-path-restoration.windows.test.ts @@ -0,0 +1,165 @@ +import { spawnSync } from 'node:child_process' +import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join, win32 as pathWin32 } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + _resetHydrateShellPathCache, + configureWindowsShellPathHydration, + hydrateShellPath, + mergePathSegments, + type HydrationResult +} from './hydrate-shell-path' +import { createWindowsShellPathHydration } from './windows-shell-path-hydration' + +type Deferred = { + promise: Promise + resolve: (value: T) => void +} + +function deferred(): Deferred { + let resolve!: (value: T) => void + const promise = new Promise((next) => { + resolve = next + }) + return { promise, resolve } +} + +function successfulHydration(segments: string[]): HydrationResult { + return { segments, ok: true, failureReason: 'none' } +} + +function resolveProbe(pathValue: string): string { + const env: NodeJS.ProcessEnv = { ...process.env, PATH: pathValue, PATHEXT: '.CMD;.EXE' } + delete env.Path + const result = spawnSync( + process.env.ComSpec ?? 'C:\\Windows\\System32\\cmd.exe', + ['/d', '/s', '/c', 'orca-path-probe.cmd'], + { encoding: 'utf8', env } + ) + expect(result.error).toBeUndefined() + expect(result.status).toBe(0) + return result.stdout.trim() +} + +describe.runIf(process.platform === 'win32')('Windows shell PATH restoration', () => { + const originalPath = process.env.PATH + let fixtureRoot = '' + let shellADir = '' + let shellBDir = '' + let shellBProfileDir = '' + + beforeEach(() => { + _resetHydrateShellPathCache() + fixtureRoot = mkdtempSync(join(tmpdir(), 'orca-shell-path-')) + shellADir = join(fixtureRoot, 'shell-a') + shellBDir = join(fixtureRoot, 'shell-b') + shellBProfileDir = join(fixtureRoot, 'shell-b-profile') + mkdirSync(shellADir) + mkdirSync(shellBDir) + mkdirSync(shellBProfileDir) + writeFileSync(join(shellADir, 'orca-path-probe.cmd'), '@echo shell-a\r\n') + writeFileSync(join(shellBDir, 'orca-path-probe.cmd'), '@echo shell-b\r\n') + }) + + afterEach(() => { + _resetHydrateShellPathCache() + if (originalPath === undefined) { + delete process.env.PATH + } else { + process.env.PATH = originalPath + } + rmSync(fixtureRoot, { force: true, recursive: true }) + }) + + it('restores the inherited baseline before applying a different shell profile', () => { + const baseline = [shellBDir, shellADir] + process.env.PATH = baseline.join(pathWin32.delimiter) + + mergePathSegments([shellADir, shellBDir]) + expect(process.env.PATH?.split(pathWin32.delimiter)).toEqual([shellADir, shellBDir]) + expect(resolveProbe(process.env.PATH ?? '')).toBe('shell-a') + + configureWindowsShellPathHydration('git-bash', 'C:\\Git\\bin\\bash.exe') + const shellBOutput = [shellBProfileDir, ...(process.env.PATH ?? '').split(pathWin32.delimiter)] + mergePathSegments(shellBOutput) + + expect({ + path: process.env.PATH?.split(pathWin32.delimiter), + selectedExecutable: resolveProbe(process.env.PATH ?? '') + }).toEqual({ + path: [shellBProfileDir, ...baseline], + selectedExecutable: 'shell-b' + }) + }) + + it('generation-fences an active shell A probe before shell B inherits the baseline', async () => { + const baseline = [shellBDir, shellADir] + process.env.PATH = baseline.join(pathWin32.delimiter) + const shellAResult = deferred() + const hydrate = vi.fn<() => Promise>() + hydrate + .mockReturnValueOnce(shellAResult.promise) + .mockImplementationOnce(async () => + successfulHydration([ + shellBProfileDir, + ...(process.env.PATH ?? '').split(pathWin32.delimiter) + ]) + ) + const coordinator = createWindowsShellPathHydration({ hydrate }) + + const shellAReady = coordinator.hydrate('powershell.exe') + await vi.waitFor(() => expect(hydrate).toHaveBeenCalledOnce()) + const shellBReady = coordinator.hydrate('git-bash') + shellAResult.resolve(successfulHydration([shellADir, shellBDir])) + await Promise.all([shellAReady, shellBReady]) + + expect(process.env.PATH?.split(pathWin32.delimiter)).toEqual([shellBProfileDir, ...baseline]) + expect(resolveProbe(process.env.PATH ?? '')).toBe('shell-b') + }) + + it('preserves a newly installed PATH entry across shell changes', () => { + const baseline = [shellBDir, shellADir] + const installedDir = join(fixtureRoot, 'newly-installed') + process.env.PATH = baseline.join(pathWin32.delimiter) + mergePathSegments([shellADir, shellBDir]) + process.env.PATH += `${pathWin32.delimiter}${installedDir}` + + configureWindowsShellPathHydration('git-bash', 'C:\\Git\\bin\\bash.exe') + mergePathSegments([shellBProfileDir, ...(process.env.PATH ?? '').split(pathWin32.delimiter)]) + + expect(process.env.PATH?.split(pathWin32.delimiter)).toEqual([ + shellBProfileDir, + ...baseline, + installedDir + ]) + }) + + it.each(['cmd.exe', 'wsl.exe'])('restores the exact baseline before switching to %s', (shell) => { + const baseline = `${shellBDir};${shellBDir.toUpperCase()}\\;C:\\;\\\\Server\\Share\\` + process.env.PATH = baseline + mergePathSegments([shellADir, shellBDir]) + + configureWindowsShellPathHydration(shell) + + expect(process.env.PATH).toBe(baseline) + }) + + it('restores the baseline before a forced same-shell probe', async () => { + const baseline = [shellBDir, shellADir].join(pathWin32.delimiter) + process.env.PATH = baseline + mergePathSegments([shellADir, shellBDir]) + let inheritedPath = '' + + await hydrateShellPath({ + force: true, + spawner: async () => { + inheritedPath = process.env.PATH ?? '' + return { segments: [], ok: false, failureReason: 'empty_path' } + } + }) + + expect(inheritedPath).toBe(baseline) + expect(process.env.PATH).toBe(baseline) + }) +})