From c73f900b8ac23f1342c686b1bf1b09bbe33f7daa Mon Sep 17 00:00:00 2001 From: brennanb2025 Date: Sat, 27 Jun 2026 23:01:09 -0700 Subject: [PATCH] test(pty): assert resolved absolute PowerShell path in win32 spawn tests The PowerShell-resolution fix (#5161) now hands ConPTY a real absolute executable instead of a bare 'powershell.exe'/'pwsh.exe' family name. The pre-existing win32 spawn assertions in pty.test.ts (not in the worker's per-file affected set) still expected the bare names and went red on the full CI verify suite. Mock the fs seam and pin the install roots the resolver probes so the resolved path is deterministic on Linux CI and Windows alike, and assert the resolved System32 / Program Files paths. --- src/main/ipc/pty.test.ts | 55 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 5 deletions(-) diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index 4cb6aa044c2..d20e5905ecb 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -200,6 +200,13 @@ const POWERSHELL_OSC133_ARGS = [ '-EncodedCommand', encodePowerShellCommand(getPowerShellOsc133Bootstrap()) ] +// Why: on Windows the spawn path resolves a bare PowerShell family name to a +// real absolute executable before handing it to ConPTY (PR #6537 / issue +// #5161) — a bare/alias `pwsh.exe` makes CreateProcessW fail with error code 5. +// These match the deterministic install roots pinned in the win32 beforeEach. +const RESOLVED_WINDOWS_POWERSHELL = + 'C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe' +const RESOLVED_PWSH7 = 'C:\\Program Files\\PowerShell\\7\\pwsh.exe' const TEST_CODEX_HOME = process.platform === 'win32' ? 'C:\\Users\\test\\AppData\\Roaming\\orca\\codex-runtime-home\\home' @@ -4817,6 +4824,7 @@ describe('registerPtyHandlers', () => { describe('Windows UTF-8 code page', () => { let originalPlatform: string let originalComspec: string | undefined + const savedWindowsResolutionEnv: Record = {} beforeEach(() => { originalPlatform = process.platform @@ -4826,6 +4834,32 @@ describe('registerPtyHandlers', () => { value: 'win32' }) process.env.USERPROFILE = 'C:\\Users\\test' + // Why: the production spawn path resolves a bare PowerShell family name to + // a real absolute executable (PR #6537 / issue #5161). Pin the install + // roots it probes so the resolved path is deterministic regardless of the + // host OS the suite runs on (CI verify runs on Linux, devs on Windows). + for (const key of ['SystemRoot', 'ProgramW6432', 'ProgramFiles', 'ProgramFiles(x86)']) { + savedWindowsResolutionEnv[key] = process.env[key] + } + process.env.SystemRoot = 'C:\\Windows' + process.env.ProgramW6432 = 'C:\\Program Files' + process.env.ProgramFiles = 'C:\\Program Files' + delete process.env['ProgramFiles(x86)'] + // Why: the resolver treats any existing, non-zero-size `.exe` outside the + // Microsoft Store WindowsApps alias dir as a real executable. Directories + // (cwd validation) must still report isDirectory(); the default mock omits + // isFile()/size, which would make every PowerShell candidate fail to + // resolve and collapse the chain to cmd.exe. + statSyncMock.mockImplementation((target: string) => { + const isExe = /\.exe$/i.test(String(target)) + return { + isDirectory: () => !isExe, + isFile: () => isExe, + size: isExe ? 1024 : 0, + mode: 0o755 + } + }) + existsSyncMock.mockReturnValue(true) }) afterEach(() => { @@ -4838,6 +4872,13 @@ describe('registerPtyHandlers', () => { } else { process.env.COMSPEC = originalComspec } + for (const [key, value] of Object.entries(savedWindowsResolutionEnv)) { + if (value === undefined) { + delete process.env[key] + } else { + process.env[key] = value + } + } delete process.env.PYTHONUTF8 }) @@ -4935,7 +4976,7 @@ describe('registerPtyHandlers', () => { await handlers.get('pty:spawn')!(null, { cols: 80, rows: 24 }) expect(spawnMock).toHaveBeenCalledWith( - 'powershell.exe', + RESOLVED_WINDOWS_POWERSHELL, POWERSHELL_OSC133_ARGS, expect.any(Object) ) @@ -5059,7 +5100,7 @@ describe('registerPtyHandlers', () => { await handlers.get('pty:spawn')!(null, { cols: 80, rows: 24 }) expect(spawnMock).toHaveBeenCalledWith( - 'powershell.exe', + RESOLVED_WINDOWS_POWERSHELL, POWERSHELL_OSC133_ARGS, expect.any(Object) ) @@ -5081,7 +5122,11 @@ describe('registerPtyHandlers', () => { ) await handlers.get('pty:spawn')!(null, { cols: 80, rows: 24 }) - expect(spawnMock).toHaveBeenCalledWith('pwsh.exe', POWERSHELL_OSC133_ARGS, expect.any(Object)) + expect(spawnMock).toHaveBeenCalledWith( + RESOLVED_PWSH7, + POWERSHELL_OSC133_ARGS, + expect.any(Object) + ) }) it('falls back to powershell.exe when PowerShell 7 is selected but unavailable', async () => { @@ -5101,7 +5146,7 @@ describe('registerPtyHandlers', () => { await handlers.get('pty:spawn')!(null, { cols: 80, rows: 24 }) expect(spawnMock).toHaveBeenCalledWith( - 'powershell.exe', + RESOLVED_WINDOWS_POWERSHELL, POWERSHELL_OSC133_ARGS, expect.any(Object) ) @@ -5124,7 +5169,7 @@ describe('registerPtyHandlers', () => { await handlers.get('pty:spawn')!(null, { cols: 80, rows: 24, shellOverride: 'pwsh.exe' }) expect(spawnMock).toHaveBeenCalledWith( - 'powershell.exe', + RESOLVED_WINDOWS_POWERSHELL, POWERSHELL_OSC133_ARGS, expect.any(Object) )