From 7f640ca904bd980cae9da560feb4ebcfeb7d5b2c Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 17 Jun 2026 20:49:47 -0700 Subject: [PATCH] Speed up Codex terminal startup (#5664) * Speed up Codex terminal startup Co-authored-by: Orca * Address Codex startup review comments Co-authored-by: Orca --------- Co-authored-by: Orca --- src/main/daemon/daemon-pty-adapter.ts | 11 +++++- src/main/daemon/daemon-server.ts | 3 ++ src/main/daemon/pty-subprocess.ts | 31 ++++++++++------ src/main/daemon/session.test.ts | 18 +++++++++- src/main/daemon/session.ts | 6 +++- src/main/daemon/terminal-host.ts | 6 +++- src/main/daemon/types.ts | 5 +-- src/main/ipc/pty.test.ts | 46 +++++++++++++++++++++--- src/main/providers/local-pty-provider.ts | 28 +++++++++------ 9 files changed, 123 insertions(+), 31 deletions(-) diff --git a/src/main/daemon/daemon-pty-adapter.ts b/src/main/daemon/daemon-pty-adapter.ts index 18ded8ec5..61140d91e 100644 --- a/src/main/daemon/daemon-pty-adapter.ts +++ b/src/main/daemon/daemon-pty-adapter.ts @@ -9,6 +9,7 @@ import { HistoryManager } from './history-manager' import { HistoryReader } from './history-reader' import { mintPtySessionId, parsePtySessionId } from './pty-session-id' import { supportsPtyStartupBarrier } from './shell-ready' +import { CODEX_SHELL_READY_TIMEOUT_MS } from './session' import { PROTOCOL_VERSION, type CreateOrAttachResult, @@ -20,6 +21,7 @@ import { } from './types' import type { IPtyProvider, PtySpawnOptions, PtySpawnResult } from '../providers/types' import { isShellProcess } from '../../shared/agent-detection' +import { recognizeAgentProcessFromCommandLine } from '../../shared/agent-process-recognition' export type DaemonPtyAdapterOptions = { socketPath: string @@ -133,6 +135,12 @@ export class DaemonPtyAdapter implements IPtyProvider { await this.ensureConnected() + const shellReadySupported = opts.command ? supportsPtyStartupBarrier(opts.env ?? {}) : false + const shellReadyTimeoutMs = + shellReadySupported && recognizeAgentProcessFromCommandLine(opts.command)?.agent === 'codex' + ? CODEX_SHELL_READY_TIMEOUT_MS + : undefined + const result = await this.client.request('createOrAttach', { sessionId, cols: effectiveCols, @@ -149,7 +157,8 @@ export class DaemonPtyAdapter implements IPtyProvider { shellOverride: opts.shellOverride, terminalWindowsWslDistro: opts.terminalWindowsWslDistro, terminalWindowsPowerShellImplementation: opts.terminalWindowsPowerShellImplementation, - shellReadySupported: opts.command ? supportsPtyStartupBarrier(opts.env ?? {}) : false + shellReadySupported, + ...(shellReadyTimeoutMs !== undefined ? { shellReadyTimeoutMs } : {}) }) if (effectiveCwd) { diff --git a/src/main/daemon/daemon-server.ts b/src/main/daemon/daemon-server.ts index b69d0464e..f7b93ea3c 100644 --- a/src/main/daemon/daemon-server.ts +++ b/src/main/daemon/daemon-server.ts @@ -278,6 +278,9 @@ export class DaemonServer { terminalWindowsWslDistro: p.terminalWindowsWslDistro, terminalWindowsPowerShellImplementation: p.terminalWindowsPowerShellImplementation, shellReadySupported: p.shellReadySupported, + ...(p.shellReadyTimeoutMs !== undefined + ? { shellReadyTimeoutMs: p.shellReadyTimeoutMs } + : {}), streamClient: { onData: (data) => { const lastInputAt = this.lastInputAtBySessionId.get(p.sessionId) diff --git a/src/main/daemon/pty-subprocess.ts b/src/main/daemon/pty-subprocess.ts index fb02428f2..8b732978e 100644 --- a/src/main/daemon/pty-subprocess.ts +++ b/src/main/daemon/pty-subprocess.ts @@ -387,6 +387,8 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl let shellPath = cwdWslInfo || sessionWslContext ? 'wsl.exe' : opts.shellOverride || resolvePtyShellPath(env) let shellArgs: string[] + const startupAgentRecognition = recognizeAgentProcessFromCommandLine(opts.command) + const isCodexStartupCommand = startupAgentRecognition?.agent === 'codex' const requestedCwd = opts.cwd || getDefaultCwd() let spawnCwd = requestedCwd let validationCwd = spawnCwd @@ -500,16 +502,24 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl } // Why: any Orca-injected overlay env that user rc files can clobber // needs the wrapper so the post-rc restore line runs. - const shellLaunch = opts.command - ? getShellReadyLaunchConfig(shellPath) - : env.ORCA_ATTRIBUTION_SHIM_DIR || - env.ORCA_OPENCODE_CONFIG_DIR || - env.ORCA_PI_CODING_AGENT_DIR || - env.ORCA_OMP_CODING_AGENT_DIR || - env.ORCA_CODEX_HOME || - env.ORCA_AGENT_TEAMS_SHIM_DIR - ? getAttributionShellLaunchConfig(shellPath) - : null + let shellLaunch: ReturnType | null = null + if (opts.command && isCodexStartupCommand) { + // Why: Codex needs the env-restoring wrapper, but waiting for a shell + // marker delays the first useful TUI frame. + shellLaunch = getAttributionShellLaunchConfig(shellPath) + } else if (opts.command) { + shellLaunch = getShellReadyLaunchConfig(shellPath) + } else { + shellLaunch = + env.ORCA_ATTRIBUTION_SHIM_DIR || + env.ORCA_OPENCODE_CONFIG_DIR || + env.ORCA_PI_CODING_AGENT_DIR || + env.ORCA_OMP_CODING_AGENT_DIR || + env.ORCA_CODEX_HOME || + env.ORCA_AGENT_TEAMS_SHIM_DIR + ? getAttributionShellLaunchConfig(shellPath) + : null + } if (shellLaunch) { Object.assign(env, shellLaunch.env) } @@ -562,7 +572,6 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl cwd: opts.cwd, worktreeId: parsePtySessionId(opts.sessionId).worktreeId }) - const startupAgentRecognition = recognizeAgentProcessFromCommandLine(opts.command) let startupAgentForeground: { processName: string; expiresAt: number } | null = startupAgentRecognition ? { diff --git a/src/main/daemon/session.test.ts b/src/main/daemon/session.test.ts index f1215d09b..4d8f4e746 100644 --- a/src/main/daemon/session.test.ts +++ b/src/main/daemon/session.test.ts @@ -73,6 +73,7 @@ describe('Session', () => { function createSession(opts?: { shellReadySupported?: boolean + shellReadyTimeoutMs?: number cols?: number rows?: number }): Session { @@ -81,7 +82,10 @@ describe('Session', () => { cols: opts?.cols ?? 80, rows: opts?.rows ?? 24, subprocess, - shellReadySupported: opts?.shellReadySupported ?? false + shellReadySupported: opts?.shellReadySupported ?? false, + ...(opts?.shellReadyTimeoutMs !== undefined + ? { shellReadyTimeoutMs: opts.shellReadyTimeoutMs } + : {}) }) return session } @@ -213,6 +217,18 @@ describe('Session', () => { expect(subprocess.written).toEqual(['waiting input']) }) + it('honors a shorter shell-ready timeout for Codex startup sessions', () => { + createSession({ shellReadySupported: true, shellReadyTimeoutMs: 300 }) + session.write('codex\n') + + vi.advanceTimersByTime(299) + expect(subprocess.written).toEqual([]) + + vi.advanceTimersByTime(1) + expect(session.shellState).toBe('timed_out' satisfies ShellReadyState) + expect(subprocess.written).toEqual(['codex\n']) + }) + it('detects marker split across data chunks', () => { createSession({ shellReadySupported: true }) diff --git a/src/main/daemon/session.ts b/src/main/daemon/session.ts index 9c3b6da63..94e434d7f 100644 --- a/src/main/daemon/session.ts +++ b/src/main/daemon/session.ts @@ -11,6 +11,9 @@ import type { } from './types' const SHELL_READY_TIMEOUT_MS = 15_000 +// Why: Codex startup skips marker-gated command delivery; this only bounds +// older daemon/local paths that still report shell-ready support for Codex. +export const CODEX_SHELL_READY_TIMEOUT_MS = 300 const KILL_TIMEOUT_MS = 5_000 const SHELL_READY_MARKER = '\x1b]777;orca-shell-ready\x07' // Why: pending records exist so the 5s checkpoint can persist increments @@ -47,6 +50,7 @@ export type SessionOptions = { rows: number subprocess: SubprocessHandle shellReadySupported: boolean + shellReadyTimeoutMs?: number scrollback?: number } @@ -94,7 +98,7 @@ export class Session { this._shellState = 'pending' this.shellReadyTimer = setTimeout(() => { this.onShellReadyTimeout() - }, SHELL_READY_TIMEOUT_MS) + }, opts.shellReadyTimeoutMs ?? SHELL_READY_TIMEOUT_MS) } else { this._shellState = 'unsupported' } diff --git a/src/main/daemon/terminal-host.ts b/src/main/daemon/terminal-host.ts index 8ba3ea910..dc96e72e8 100644 --- a/src/main/daemon/terminal-host.ts +++ b/src/main/daemon/terminal-host.ts @@ -27,6 +27,7 @@ export type CreateOrAttachOptions = { terminalWindowsWslDistro?: string | null terminalWindowsPowerShellImplementation?: 'auto' | 'powershell.exe' | 'pwsh.exe' shellReadySupported?: boolean + shellReadyTimeoutMs?: number streamClient: { onData: (data: string) => void; onExit: (code: number) => void } } @@ -122,7 +123,10 @@ export class TerminalHost { cols: size.cols, rows: size.rows, subprocess, - shellReadySupported: opts.shellReadySupported ?? false + shellReadySupported: opts.shellReadySupported ?? false, + ...(opts.shellReadyTimeoutMs !== undefined + ? { shellReadyTimeoutMs: opts.shellReadyTimeoutMs } + : {}) }) this.sessions.set(opts.sessionId, session) diff --git a/src/main/daemon/types.ts b/src/main/daemon/types.ts index cea72e284..e0afca9ad 100644 --- a/src/main/daemon/types.ts +++ b/src/main/daemon/types.ts @@ -3,9 +3,9 @@ // when daemon-baked behavior cannot be delivered by on-disk wrapper refresh. // Why: bump when adding daemon wire behavior so same-version old daemons do // not silently accept the handshake and then reject new RPCs. -export const PROTOCOL_VERSION = 14 +export const PROTOCOL_VERSION = 15 export const PREVIOUS_DAEMON_PROTOCOL_VERSIONS = [ - 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13 + 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14 ] as const // ─── Session State Machine ────────────────────────────────────────── @@ -101,6 +101,7 @@ export type CreateOrAttachRequest = { * PTY path resolves the same effective executable as LocalPtyProvider. */ terminalWindowsPowerShellImplementation?: 'auto' | 'powershell.exe' | 'pwsh.exe' shellReadySupported?: boolean + shellReadyTimeoutMs?: number } } diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index 9e6d1f321..edc78d0c4 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -4780,7 +4780,41 @@ describe('registerPtyHandlers', () => { } ) - posixOnlyIt('falls back to a max wait when the shell emits no readiness output', async () => { + posixOnlyIt( + 'uses the no-marker wrapper and writes quickly for Codex startup commands', + async () => { + vi.useFakeTimers() + const mockProc = createMockProc() + spawnMock.mockReturnValue(mockProc.proc) + + try { + registerPtyHandlers(mainWindow as never) + await handlers.get('pty:spawn')!(null, { + cols: 80, + rows: 24, + cwd: '/tmp', + command: 'codex' + }) + + const [, , options] = spawnMock.mock.calls[0]! + expect(options.env.ORCA_SHELL_READY_MARKER).toBe('0') + + await Promise.resolve() + vi.advanceTimersByTime(49) + await Promise.resolve() + expect(mockProc.proc.write).not.toHaveBeenCalled() + + vi.advanceTimersByTime(1) + await Promise.resolve() + vi.runAllTimers() + expect(mockProc.proc.write).toHaveBeenCalledWith('codex\n') + } finally { + vi.useRealTimers() + } + } + ) + + posixOnlyIt('keeps the conservative max wait for non-agent startup commands', async () => { vi.useFakeTimers() const mockProc = createMockProc() spawnMock.mockReturnValue(mockProc.proc) @@ -4791,13 +4825,17 @@ describe('registerPtyHandlers', () => { cols: 80, rows: 24, cwd: '/tmp', - command: 'codex' + command: 'printf "hello"' }) - vi.advanceTimersByTime(1500) + vi.advanceTimersByTime(1499) + await Promise.resolve() + expect(mockProc.proc.write).not.toHaveBeenCalled() + + vi.advanceTimersByTime(1) await Promise.resolve() vi.runAllTimers() - expect(mockProc.proc.write).toHaveBeenCalledWith('codex\n') + expect(mockProc.proc.write).toHaveBeenCalledWith('printf "hello"\n') } finally { vi.useRealTimers() } diff --git a/src/main/providers/local-pty-provider.ts b/src/main/providers/local-pty-provider.ts index 9adcd940e..ca2ab29ff 100644 --- a/src/main/providers/local-pty-provider.ts +++ b/src/main/providers/local-pty-provider.ts @@ -41,6 +41,7 @@ import { import { WINDOWS_GIT_BASH_SHELL } from '../../shared/windows-terminal-shell' import { resolveAgentForegroundProcess } from './agent-foreground-process' import { getAgentForegroundContextPaths } from './agent-foreground-context-paths' +import { recognizeAgentProcessFromCommandLine } from '../../shared/agent-process-recognition' const PANE_IDENTITY_ENV_KEYS = ['ORCA_PANE_KEY', 'ORCA_TAB_ID', 'ORCA_WORKTREE_ID'] as const @@ -472,16 +473,23 @@ export class LocalPtyProvider implements IPtyProvider { finalEnv.ORCA_OMP_CODING_AGENT_DIR || finalEnv.ORCA_CODEX_HOME || finalEnv.ORCA_AGENT_TEAMS_SHIM_DIR - getFallbackShellReadyConfig = args.command - ? (shell) => getShellReadyLaunchConfig(shell) - : needsNoMarkerWrapper - ? (shell) => getAttributionShellLaunchConfig(shell) - : undefined - const shellLaunch = args.command - ? getShellReadyLaunchConfig(shellPath) - : needsNoMarkerWrapper - ? getAttributionShellLaunchConfig(shellPath) - : null + const isCodexStartupCommand = + recognizeAgentProcessFromCommandLine(args.command)?.agent === 'codex' + let shellLaunch: ReturnType | null = null + if (args.command && isCodexStartupCommand) { + // Why: Codex needs the env-restoring wrapper, but waiting for a shell + // marker delays the first useful TUI frame. + getFallbackShellReadyConfig = (shell) => getAttributionShellLaunchConfig(shell) + shellLaunch = getAttributionShellLaunchConfig(shellPath) + } else if (args.command) { + getFallbackShellReadyConfig = (shell) => getShellReadyLaunchConfig(shell) + shellLaunch = getShellReadyLaunchConfig(shellPath) + } else if (needsNoMarkerWrapper) { + getFallbackShellReadyConfig = (shell) => getAttributionShellLaunchConfig(shell) + shellLaunch = getAttributionShellLaunchConfig(shellPath) + } else { + getFallbackShellReadyConfig = undefined + } if (shellLaunch) { Object.assign(finalEnv, shellLaunch.env) shellArgs = shellLaunch.args ?? shellArgs