From eae9f172365f7fffd90b006993dd82c5e908dd07 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 5 Sep 2026 03:12:06 -0700 Subject: [PATCH] refactor(pty): separate startup identity from eager shell argv --- .../pty-subprocess-managed-agent-env.test.ts | 52 ++--- ...ty-subprocess-windows-shell-launch.test.ts | 177 ++++++++++++------ .../daemon/pty-subprocess-wsl-launch.test.ts | 96 +++++----- src/main/daemon/pty-subprocess.ts | 2 + .../pty-subprocess/shell-launch-plan.ts | 7 +- 5 files changed, 203 insertions(+), 131 deletions(-) diff --git a/src/main/daemon/pty-subprocess-managed-agent-env.test.ts b/src/main/daemon/pty-subprocess-managed-agent-env.test.ts index d5dbc33246c..72532ce6b3d 100644 --- a/src/main/daemon/pty-subprocess-managed-agent-env.test.ts +++ b/src/main/daemon/pty-subprocess-managed-agent-env.test.ts @@ -261,33 +261,37 @@ describe('createPtySubprocess', () => { expect(lastCall[2].env.ORCA_SHELL_FEATURES).not.toContain('ready') }) - it('enables readiness and shell identity for plain Codex startup', async () => { - const proc = mockPtyProcess() - spawnMock.mockReturnValue(proc) - const platform = Object.getOwnPropertyDescriptor(process, 'platform') - Object.defineProperty(process, 'platform', { value: 'linux' }) + it.each([false, true])( + 'preserves Codex readiness and identity when deferred=%s', + async (deferStartupCommand) => { + const proc = mockPtyProcess() + spawnMock.mockReturnValue(proc) + const platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { value: 'linux' }) - try { - await createPtySubprocess({ - sessionId: 'test', - cols: 80, - rows: 24, - cwd: '/repo', - command: 'codex', - env: { SHELL: '/bin/zsh' } - }) - } finally { - if (platform) { - Object.defineProperty(process, 'platform', platform) + try { + await createPtySubprocess({ + sessionId: 'test', + cols: 80, + rows: 24, + cwd: '/repo', + command: 'codex', + deferStartupCommand, + env: { SHELL: '/bin/zsh' } + }) + } 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).toMatch(ZSH_SHELL_READY_DIR) - expect(lastCall[2].env.ORCA_SHELL_FEATURES).toContain('ready') - expect(lastCall[2].env.ORCA_SHELL_FEATURES).toContain('identity') - }) + const lastCall = spawnMock.mock.calls.at(-1)! + expect(lastCall[1]).toEqual(['-l']) + expect(lastCall[2].env.ZDOTDIR).toMatch(ZSH_SHELL_READY_DIR) + expect(lastCall[2].env.ORCA_SHELL_FEATURES).toContain('ready') + expect(lastCall[2].env.ORCA_SHELL_FEATURES).toContain('identity') + } + ) it('uses shell-ready wrapper for delivery-hinted Codex startup commands', async () => { const proc = mockPtyProcess() diff --git a/src/main/daemon/pty-subprocess-windows-shell-launch.test.ts b/src/main/daemon/pty-subprocess-windows-shell-launch.test.ts index 5831274a9c6..86c231ffd13 100644 --- a/src/main/daemon/pty-subprocess-windows-shell-launch.test.ts +++ b/src/main/daemon/pty-subprocess-windows-shell-launch.test.ts @@ -207,70 +207,127 @@ describe('createPtySubprocess', () => { expect(isPwshAvailableMock).not.toHaveBeenCalled() }) - it('ignores the PowerShell implementation setting for cmd.exe on Windows', async () => { - const proc = mockPtyProcess() - spawnMock.mockReturnValue(proc) - const platform = Object.getOwnPropertyDescriptor(process, 'platform') + it.each([false, true])( + 'preserves cmd preflight when deferred=%s', + async (deferStartupCommand) => { + const proc = mockPtyProcess() + spawnMock.mockReturnValue(proc) + const platform = Object.getOwnPropertyDescriptor(process, 'platform') - Object.defineProperty(process, 'platform', { value: 'win32' }) - isPwshAvailableMock.mockReturnValue(true) + Object.defineProperty(process, 'platform', { value: 'win32' }) + isPwshAvailableMock.mockReturnValue(true) - try { - await createPtySubprocess({ - sessionId: 'test', - cols: 80, - rows: 24, - shellOverride: 'cmd.exe', - terminalWindowsPowerShellImplementation: 'pwsh.exe', - env: { ORCA_CODEX_LAUNCH_PREFLIGHT: CODEX_LAUNCH_PREFLIGHT } - }) - } finally { - if (platform) { - Object.defineProperty(process, 'platform', platform) + try { + await createPtySubprocess({ + sessionId: 'test', + cols: 80, + rows: 24, + shellOverride: 'cmd.exe', + terminalWindowsPowerShellImplementation: 'pwsh.exe', + command: 'codex DEFERRED_AGENT_MARKER', + deferStartupCommand, + env: { ORCA_CODEX_LAUNCH_PREFLIGHT: CODEX_LAUNCH_PREFLIGHT } + }) + } finally { + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + } + + expect(spawnMock).toHaveBeenCalledWith( + 'cmd.exe', + [ + '/K', + `chcp 65001 > nul & if defined ORCA_CODEX_LAUNCH_PREFLIGHT call %ORCA_CODEX_LAUNCH_PREFLIGHT_CMD_QUOTE%%ORCA_CODEX_LAUNCH_PREFLIGHT%%ORCA_CODEX_LAUNCH_PREFLIGHT_CMD_QUOTE% agent hooks prepare-codex > nul 2>&1${ + deferStartupCommand ? '' : ' & codex DEFERRED_AGENT_MARKER' + }` + ], + expect.objectContaining({ + env: expect.objectContaining({ ORCA_CODEX_LAUNCH_PREFLIGHT_CMD_QUOTE: '"' }) + }) + ) + } + ) + + it.each([false, true])( + 'holds PowerShell startup argv only when deferred=%s', + async (deferStartupCommand) => { + const proc = mockPtyProcess() + spawnMock.mockReturnValue(proc) + const platform = Object.getOwnPropertyDescriptor(process, 'platform') + + Object.defineProperty(process, 'platform', { value: 'win32' }) + + let handle: Awaited> + try { + handle = await createPtySubprocess({ + sessionId: 'test', + cols: 80, + rows: 24, + cwd: 'C:\\repo\\orca', + shellOverride: 'powershell.exe', + command: "& 'codex' '--no-alt-screen'", + deferStartupCommand + }) + } finally { + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + } + + const lastCall = spawnMock.mock.calls.at(-1)! + const encoded = String(lastCall[1][3]) + const command = Buffer.from(encoded, 'base64').toString('utf16le') + expect(command.includes("& 'codex' '--no-alt-screen'")).toBe(!deferStartupCommand) + expect(Boolean(handle!.startupCommandDeliveredInShellArgs)).toBe(!deferStartupCommand) + } + ) + + it.each([1, 2])( + 'withholds deferred startup through %s failed Windows shells', + async (failures) => { + const platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { value: 'win32' }) + isPwshAvailableMock.mockReturnValue(true) + const proc = mockPtyProcess() + for (let index = 0; index < failures; index++) { + spawnMock.mockImplementationOnce(() => { + throw new Error('shell unavailable') + }) + } + spawnMock.mockReturnValue(proc) + try { + const handle = await createPtySubprocess({ + sessionId: 'deferred-fallback', + cols: 80, + rows: 24, + cwd: 'C:\\repo\\orca', + shellOverride: 'pwsh.exe', + command: 'codex DEFERRED_AGENT_MARKER', + deferStartupCommand: true, + launchAgent: 'codex', + env: { AGENT_TEST_VALUE: 'preserved' } + }) + expect(spawnMock).toHaveBeenCalledTimes(failures + 1) + for (const [, argv, options] of spawnMock.mock.calls) { + const encodedIndex = argv.indexOf('-EncodedCommand') + const payload = + encodedIndex === -1 + ? argv.join(' ') + : Buffer.from(argv[encodedIndex + 1], 'base64').toString('utf16le') + expect(payload).not.toContain('DEFERRED_AGENT_MARKER') + expect(options.env.AGENT_TEST_VALUE).toBe('preserved') + } + expect(handle.startupCommandDeliveredInShellArgs).not.toBe(true) + expect(proc.write).not.toHaveBeenCalled() + handle.dispose() + } finally { + if (platform) { + Object.defineProperty(process, 'platform', platform) + } } } - - expect(spawnMock).toHaveBeenCalledWith( - 'cmd.exe', - [ - '/K', - 'chcp 65001 > nul & if defined ORCA_CODEX_LAUNCH_PREFLIGHT call %ORCA_CODEX_LAUNCH_PREFLIGHT_CMD_QUOTE%%ORCA_CODEX_LAUNCH_PREFLIGHT%%ORCA_CODEX_LAUNCH_PREFLIGHT_CMD_QUOTE% agent hooks prepare-codex > nul 2>&1' - ], - expect.objectContaining({ - env: expect.objectContaining({ ORCA_CODEX_LAUNCH_PREFLIGHT_CMD_QUOTE: '"' }) - }) - ) - }) - - it('embeds short PowerShell startup commands in the Windows shell launch', async () => { - const proc = mockPtyProcess() - spawnMock.mockReturnValue(proc) - const platform = Object.getOwnPropertyDescriptor(process, 'platform') - - Object.defineProperty(process, 'platform', { value: 'win32' }) - - let handle: Awaited> - try { - handle = await createPtySubprocess({ - sessionId: 'test', - cols: 80, - rows: 24, - cwd: 'C:\\repo\\orca', - shellOverride: 'powershell.exe', - command: "& 'codex' '--no-alt-screen'" - }) - } finally { - if (platform) { - Object.defineProperty(process, 'platform', platform) - } - } - - const lastCall = spawnMock.mock.calls.at(-1)! - const encoded = String(lastCall[1][3]) - const command = Buffer.from(encoded, 'base64').toString('utf16le') - expect(command.trimEnd().endsWith("& 'codex' '--no-alt-screen'")).toBe(true) - expect(handle!.startupCommandDeliveredInShellArgs).toBe(true) - }) + ) it('keeps oversized Windows startup commands on PTY stdin delivery', async () => { const proc = mockPtyProcess() diff --git a/src/main/daemon/pty-subprocess-wsl-launch.test.ts b/src/main/daemon/pty-subprocess-wsl-launch.test.ts index 067dc729aad..9c745aae246 100644 --- a/src/main/daemon/pty-subprocess-wsl-launch.test.ts +++ b/src/main/daemon/pty-subprocess-wsl-launch.test.ts @@ -282,53 +282,61 @@ describe('createPtySubprocess', () => { ) }) - it('routes daemon default WSL terminals to the Codex home distro without losing cwd', async () => { - const proc = mockPtyProcess() - spawnMock.mockReturnValue(proc) - const platform = Object.getOwnPropertyDescriptor(process, 'platform') - const cwd = mkdtempSync(join(tmpdir(), 'daemon-pty-wsl-codex-home-cwd-')) + it.each([false, true])( + 'preserves WSL home routing when deferred=%s', + async (deferStartupCommand) => { + const proc = mockPtyProcess() + spawnMock.mockReturnValue(proc) + const platform = Object.getOwnPropertyDescriptor(process, 'platform') + const cwd = mkdtempSync(join(tmpdir(), 'daemon-pty-wsl-codex-home-cwd-')) - Object.defineProperty(process, 'platform', { value: 'win32' }) + Object.defineProperty(process, 'platform', { value: 'win32' }) - try { - await createPtySubprocess({ - sessionId: 'test', - cols: 80, - rows: 24, - cwd, - shellOverride: 'wsl.exe', - env: { - CODEX_HOME: - '\\\\wsl.localhost\\Ubuntu\\home\\jin\\.local\\share\\orca\\codex-accounts\\a\\home', - ORCA_CODEX_HOME: - '\\\\wsl.localhost\\Ubuntu\\home\\jin\\.local\\share\\orca\\codex-accounts\\a\\home' - } - }) - } 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', '--exec', 'sh', '-c', expect.stringContaining(`cd '${expectedLinuxCwd}'`)], - expect.objectContaining({ - env: expect.objectContaining({ - CODEX_HOME: '/home/jin/.local/share/orca/codex-accounts/a/home', - ORCA_CODEX_HOME: '/home/jin/.local/share/orca/codex-accounts/a/home', - WSLENV: expect.stringContaining('CODEX_HOME') + try { + await createPtySubprocess({ + sessionId: 'test', + cols: 80, + rows: 24, + cwd, + shellOverride: 'wsl.exe', + command: 'codex DEFERRED_AGENT_MARKER', + deferStartupCommand, + env: { + CODEX_HOME: + '\\\\wsl.localhost\\Ubuntu\\home\\jin\\.local\\share\\orca\\codex-accounts\\a\\home', + ORCA_CODEX_HOME: + '\\\\wsl.localhost\\Ubuntu\\home\\jin\\.local\\share\\orca\\codex-accounts\\a\\home' + } }) - }) - ) - }) + } finally { + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + rmSync(cwd, { recursive: true, force: true }) + } + + expect(JSON.stringify(spawnMock.mock.calls.at(-1)?.[1])).not.toContain( + 'DEFERRED_AGENT_MARKER' + ) + 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', '--exec', 'sh', '-c', expect.stringContaining(`cd '${expectedLinuxCwd}'`)], + expect.objectContaining({ + env: expect.objectContaining({ + CODEX_HOME: '/home/jin/.local/share/orca/codex-accounts/a/home', + ORCA_CODEX_HOME: '/home/jin/.local/share/orca/codex-accounts/a/home', + WSLENV: expect.stringContaining('CODEX_HOME') + }) + }) + ) + } + ) it('preserves an explicit Linux Codex home in daemon WSL terminals', async () => { const proc = mockPtyProcess() diff --git a/src/main/daemon/pty-subprocess.ts b/src/main/daemon/pty-subprocess.ts index 8e8d2314783..785708d58de 100644 --- a/src/main/daemon/pty-subprocess.ts +++ b/src/main/daemon/pty-subprocess.ts @@ -24,6 +24,8 @@ export type PtySubprocessOptions = { env?: Record envToDelete?: string[] command?: string + /** Preserve command-derived environment while the owner holds command delivery. */ + deferStartupCommand?: boolean startupCommandDelivery?: StartupCommandDelivery launchAgent?: TuiAgent /** Explicit shell executable path/basename requested by the renderer. */ diff --git a/src/main/daemon/pty-subprocess/shell-launch-plan.ts b/src/main/daemon/pty-subprocess/shell-launch-plan.ts index ba60e592743..2f56350871d 100644 --- a/src/main/daemon/pty-subprocess/shell-launch-plan.ts +++ b/src/main/daemon/pty-subprocess/shell-launch-plan.ts @@ -60,6 +60,7 @@ export function createPtyShellLaunchPlan( let startupCommandDeliveredInShellArgs = false let windowsFallbackAttempts: WindowsShellSpawnAttempt[] = [] const startupAgentRecognition = recognizeAgentProcessFromCommandLine(opts.command) + const argvStartupCommand = opts.deferStartupCommand ? undefined : opts.command const requestedCwd = opts.cwd || resolveSafePtyDefaultCwd() if (opts.command && startupAgentRecognition) { assertSafeAgentStartupCwd(requestedCwd, opts.command) @@ -107,7 +108,7 @@ export function createPtyShellLaunchPlan( cwd: spawnCwd, defaultCwd: resolveSafePtyDefaultCwd(), wslContext: resolvedWslContext, - startupCommand: opts.command + startupCommand: argvStartupCommand }) const primaryAttempt = windowsFallbackAttempts[0] if (primaryAttempt) { @@ -122,7 +123,7 @@ export function createPtyShellLaunchPlan( spawnCwd, resolveSafePtyDefaultCwd(), resolvedWslContext, - opts.command, + argvStartupCommand, env.ORCA_CODEX_LAUNCH_PREFLIGHT ) shellArgs = resolved.shellArgs @@ -150,7 +151,7 @@ export function createPtyShellLaunchPlan( requestedCwd, resolveSafePtyDefaultCwd(), { distro: codexHomeWslInfo.distro }, - opts.command, + argvStartupCommand, env.ORCA_CODEX_LAUNCH_PREFLIGHT ) shellArgs = resolved.shellArgs