From 5d85380a74a550660360cb2071d32d500b147e04 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Thu, 14 May 2026 17:06:54 -0400 Subject: [PATCH] fix(pty): add Windows daemon spawn diagnostics (#1862) --- src/main/daemon/pty-subprocess.test.ts | 190 ++++++++++++++++++++----- src/main/daemon/pty-subprocess.ts | 61 ++++++-- 2 files changed, 204 insertions(+), 47 deletions(-) diff --git a/src/main/daemon/pty-subprocess.test.ts b/src/main/daemon/pty-subprocess.test.ts index 2505c60bb8a..08b2ab70d3b 100644 --- a/src/main/daemon/pty-subprocess.test.ts +++ b/src/main/daemon/pty-subprocess.test.ts @@ -27,6 +27,7 @@ const ORCA_SHELL_WRAPPER_ENV = [ const POWERSHELL_PROFILE_COMMAND = expect.stringMatching( /ORCA_OPENCODE_CONFIG_DIR[\s\S]*ORCA_PI_CODING_AGENT_DIR[\s\S]*UTF8/ ) +const ZSH_SHELL_READY_DIR = /shell-ready[\\/]zsh/ function mockPtyProcess(pid = 12345) { const onDataListeners: ((data: string) => void)[] = [] @@ -87,14 +88,22 @@ describe('createPtySubprocess', () => { it('spawns node-pty with correct options', () => { const proc = mockPtyProcess() spawnMock.mockReturnValue(proc) + const platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { value: 'linux' }) - createPtySubprocess({ - sessionId: 'test', - cols: 80, - rows: 24, - cwd: '/home/user', - env: { SHELL: '/bin/bash', FOO: 'bar' } - }) + try { + createPtySubprocess({ + sessionId: 'test', + cols: 80, + rows: 24, + cwd: '/home/user', + env: { SHELL: '/bin/bash', FOO: 'bar' } + }) + } finally { + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + } expect(spawnMock).toHaveBeenCalledWith( '/bin/bash', @@ -254,62 +263,86 @@ describe('createPtySubprocess', () => { it('uses shell wrapper when attribution shims must survive shell startup', () => { const proc = mockPtyProcess() spawnMock.mockReturnValue(proc) + const platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { value: 'linux' }) - createPtySubprocess({ - sessionId: 'test', - cols: 80, - rows: 24, - env: { - SHELL: '/bin/zsh', - ORCA_ATTRIBUTION_SHIM_DIR: '/tmp/orca-terminal-attribution/posix' + try { + createPtySubprocess({ + sessionId: 'test', + cols: 80, + rows: 24, + env: { + SHELL: '/bin/zsh', + ORCA_ATTRIBUTION_SHIM_DIR: '/tmp/orca-terminal-attribution/posix' + } + }) + } finally { + if (platform) { + Object.defineProperty(process, 'platform', platform) } - }) + } const lastCall = spawnMock.mock.calls.at(-1)! expect(lastCall[1]).toEqual(['-l']) - expect(lastCall[2].env.ZDOTDIR).toContain('shell-ready/zsh') + expect(lastCall[2].env.ZDOTDIR).toMatch(ZSH_SHELL_READY_DIR) expect(lastCall[2].env.ORCA_SHELL_READY_MARKER).toBe('0') }) it('uses shell wrapper when OpenCode config must survive shell startup', () => { const proc = mockPtyProcess() spawnMock.mockReturnValue(proc) + const platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { value: 'linux' }) - createPtySubprocess({ - sessionId: 'test', - cols: 80, - rows: 24, - env: { - SHELL: '/bin/zsh', - OPENCODE_CONFIG_DIR: '/tmp/orca-opencode-overlay', - ORCA_OPENCODE_CONFIG_DIR: '/tmp/orca-opencode-overlay' + try { + createPtySubprocess({ + sessionId: 'test', + cols: 80, + rows: 24, + env: { + SHELL: '/bin/zsh', + OPENCODE_CONFIG_DIR: '/tmp/orca-opencode-overlay', + ORCA_OPENCODE_CONFIG_DIR: '/tmp/orca-opencode-overlay' + } + }) + } finally { + if (platform) { + Object.defineProperty(process, 'platform', platform) } - }) + } const lastCall = spawnMock.mock.calls.at(-1)! expect(lastCall[1]).toEqual(['-l']) - expect(lastCall[2].env.ZDOTDIR).toContain('shell-ready/zsh') + expect(lastCall[2].env.ZDOTDIR).toMatch(ZSH_SHELL_READY_DIR) expect(lastCall[2].env.ORCA_SHELL_READY_MARKER).toBe('0') }) it('uses shell wrapper when Pi config must survive shell startup', () => { const proc = mockPtyProcess() spawnMock.mockReturnValue(proc) + const platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { value: 'linux' }) - createPtySubprocess({ - sessionId: 'test', - cols: 80, - rows: 24, - env: { - SHELL: '/bin/zsh', - PI_CODING_AGENT_DIR: '/tmp/orca-pi-agent-overlay', - ORCA_PI_CODING_AGENT_DIR: '/tmp/orca-pi-agent-overlay' + try { + createPtySubprocess({ + sessionId: 'test', + cols: 80, + rows: 24, + env: { + SHELL: '/bin/zsh', + PI_CODING_AGENT_DIR: '/tmp/orca-pi-agent-overlay', + ORCA_PI_CODING_AGENT_DIR: '/tmp/orca-pi-agent-overlay' + } + }) + } finally { + if (platform) { + Object.defineProperty(process, 'platform', platform) } - }) + } const lastCall = spawnMock.mock.calls.at(-1)! expect(lastCall[1]).toEqual(['-l']) - expect(lastCall[2].env.ZDOTDIR).toContain('shell-ready/zsh') + expect(lastCall[2].env.ZDOTDIR).toMatch(ZSH_SHELL_READY_DIR) expect(lastCall[2].env.ORCA_SHELL_READY_MARKER).toBe('0') }) @@ -501,10 +534,82 @@ describe('createPtySubprocess', () => { ) }) - it('keeps WSL UNC worktree terminals in the matching distro cwd on Windows', () => { + it('rejects a missing explicit native Windows cwd before node-pty spawn', () => { + const platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { value: 'win32' }) + + try { + expect(() => + createPtySubprocess({ + sessionId: 'test', + cols: 80, + rows: 24, + cwd: 'C:\\definitely-missing-orca-cwd', + shellOverride: 'powershell.exe' + }) + ).toThrow(/Working directory "C:\\definitely-missing-orca-cwd" does not exist/) + } finally { + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + } + + expect(spawnMock).not.toHaveBeenCalled() + }) + + it('validates the requested Windows cwd before launching WSL on Windows', () => { + const platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { value: 'win32' }) + + try { + expect(() => + createPtySubprocess({ + sessionId: 'test', + cols: 80, + rows: 24, + cwd: 'C:\\definitely-missing-orca-wsl-cwd', + shellOverride: 'wsl.exe' + }) + ).toThrow(/Working directory "C:\\definitely-missing-orca-wsl-cwd" does not exist/) + } finally { + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + } + + expect(spawnMock).not.toHaveBeenCalled() + }) + + it('adds shell and cwd context when node-pty reports File not found on Windows', () => { + const platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { value: 'win32' }) + spawnMock.mockImplementation(() => { + throw new Error('File not found: ') + }) + + try { + expect(() => + createPtySubprocess({ + sessionId: 'test', + cols: 80, + rows: 24, + shellOverride: 'not-a-real-shell.exe' + }) + ).toThrow( + /Daemon failed to spawn shell "not-a-real-shell\.exe" with cwd ".+": File not found:/ + ) + } finally { + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + } + }) + + it('falls back to /mnt/c before launching WSL when cwd is not a native Windows path', () => { const proc = mockPtyProcess() spawnMock.mockReturnValue(proc) const platform = Object.getOwnPropertyDescriptor(process, 'platform') + const cwd = mkdtempSync(join(tmpdir(), 'daemon-pty-wsl-cwd-test-')) Object.defineProperty(process, 'platform', { value: 'win32' }) @@ -513,18 +618,25 @@ describe('createPtySubprocess', () => { sessionId: 'test', cols: 80, rows: 24, - cwd: '\\\\wsl.localhost\\Ubuntu\\home\\alice\\repo', + cwd, shellOverride: 'wsl.exe' }) } finally { if (platform) { Object.defineProperty(process, 'platform', platform) } + rmSync(cwd, { recursive: true, force: true }) } + const normalizedCwd = cwd.replace(/\\/g, '/') + const driveMatch = normalizedCwd.match(/^([A-Za-z]):\/?(.*)$/) + const expectedLinuxCwd = driveMatch + ? `/mnt/${driveMatch[1].toLowerCase()}${driveMatch[2] ? `/${driveMatch[2]}` : ''}` + : '/mnt/c' + expect(spawnMock).toHaveBeenCalledWith( 'wsl.exe', - ['-d', 'Ubuntu', '--', 'bash', '-c', "cd '/home/alice/repo' && exec bash -l"], + ['--', 'bash', '-c', `cd '${expectedLinuxCwd}' && exec bash -l`], expect.objectContaining({ cwd: expect.any(String) }) ) }) diff --git a/src/main/daemon/pty-subprocess.ts b/src/main/daemon/pty-subprocess.ts index 80b6fd3c4b1..10584751100 100644 --- a/src/main/daemon/pty-subprocess.ts +++ b/src/main/daemon/pty-subprocess.ts @@ -11,7 +11,8 @@ import { import { isValidPtySize, normalizePtySize } from './daemon-pty-size' import { ensureNodePtySpawnHelperExecutable, - getNodePtySpawnHelperCandidates + getNodePtySpawnHelperCandidates, + validateWorkingDirectory } from '../providers/local-pty-utils' import { resolveWindowsShellLaunchArgs } from '../providers/windows-shell-args' import { resolveEffectiveWindowsPowerShell } from '../providers/windows-powershell' @@ -97,6 +98,36 @@ function preflightMacNodePtySpawnEnvironment(): void { throw formatMissingDaemonPathError('helper', candidates[0] ?? '') } +function isNativeWindowsPath(path: string): boolean { + return /^[A-Za-z]:[\\/]/.test(path) || path.startsWith('\\\\') +} + +function preflightWindowsPtySpawnEnvironment(args: { + validationCwd: string + cwdWasExplicit: boolean +}): void { + if (process.platform !== 'win32' || !args.cwdWasExplicit) { + return + } + + if (!isNativeWindowsPath(args.validationCwd)) { + return + } + + validateWorkingDirectory(args.validationCwd) +} + +function formatPtySpawnError(err: unknown, shellPath: string, spawnCwd: string): Error { + const message = err instanceof Error ? err.message : String(err) + const formatted = new DaemonProtocolError( + `Daemon failed to spawn shell "${shellPath}" with cwd "${spawnCwd}": ${message}` + ) + if (err instanceof Error && err.stack) { + formatted.stack = err.stack + } + return formatted +} + export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandle { const size = normalizePtySize(opts.cols, opts.rows) const env: Record = { @@ -129,6 +160,7 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl let shellPath = opts.shellOverride || resolvePtyShellPath(env) let shellArgs: string[] let spawnCwd = opts.cwd || getDefaultCwd() + let validationCwd = spawnCwd if (process.platform === 'win32') { const normalizedShellFamily = pathWin32.basename(shellPath).toLowerCase() @@ -160,6 +192,7 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl const resolved = resolveWindowsShellLaunchArgs(shellPath, spawnCwd, getDefaultCwd()) shellArgs = resolved.shellArgs spawnCwd = resolved.effectiveCwd + validationCwd = resolved.validationCwd } else { // Why: any Orca-injected overlay env that user rc files can clobber // needs the wrapper so the post-rc restore line runs. @@ -181,15 +214,27 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl // runs in a separate forked process with its own code path. ensureNodePtySpawnHelperExecutable() preflightMacNodePtySpawnEnvironment() - - const proc = pty.spawn(shellPath, shellArgs, { - name: 'xterm-256color', - cols: size.cols, - rows: size.rows, - cwd: spawnCwd, - env + preflightWindowsPtySpawnEnvironment({ + validationCwd, + cwdWasExplicit: opts.cwd !== undefined }) + let proc: pty.IPty + try { + proc = pty.spawn(shellPath, shellArgs, { + name: 'xterm-256color', + cols: size.cols, + rows: size.rows, + cwd: spawnCwd, + env + }) + } catch (err) { + if (process.platform === 'win32') { + throw formatPtySpawnError(err, shellPath, spawnCwd) + } + throw err + } + let onDataCb: ((data: string) => void) | null = null let onExitCb: ((code: number) => void) | null = null