From 186c91dca035aa4898cb4c65def03ba4ffdbdad9 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 4 Sep 2026 15:33:30 -0700 Subject: [PATCH] fix: wait for terminal line editor before agent startup input --- src/main/daemon/daemon-pty-adapter.test.ts | 32 +++------ src/main/daemon/daemon-pty-session-spawn.ts | 20 +----- src/main/daemon/daemon-pty-spawn-request.ts | 4 -- src/main/daemon/post-ready-flush-gate.test.ts | 46 ++++++++++++ src/main/daemon/post-ready-flush-gate.ts | 70 +++++++++++++------ .../pty-subprocess-managed-agent-env.test.ts | 5 +- .../pty-subprocess/shell-launch-plan.ts | 11 +-- .../daemon/session-shell-ready-barrier.ts | 8 ++- 8 files changed, 115 insertions(+), 81 deletions(-) diff --git a/src/main/daemon/daemon-pty-adapter.test.ts b/src/main/daemon/daemon-pty-adapter.test.ts index fc2f9365f23..46819e6fed1 100644 --- a/src/main/daemon/daemon-pty-adapter.test.ts +++ b/src/main/daemon/daemon-pty-adapter.test.ts @@ -344,35 +344,23 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { } }) - itOnPosix('keeps plain Codex startup on the short daemon shell-ready timeout', async () => { - await adapter.spawn({ - cols: 80, - rows: 24, - command: 'codex', - env: { SHELL: '/bin/zsh' } - }) - - await waitFor(() => vi.mocked(lastSubprocess.write).mock.calls.length > 0) - expect(lastSubprocess.write).toHaveBeenCalledWith('codex\n') - }) - - itOnPosix('waits for shell-ready for delivery-hinted Codex startup', async () => { - await adapter.spawn({ - cols: 80, - rows: 24, - command: "codex 'linked issue context'", - startupCommandDelivery: 'shell-ready', - env: { SHELL: '/bin/zsh' } - }) + itOnPosix.each([ + { command: 'codex' }, + { command: 'codex', startupCommandDelivery: 'fast' as const }, + { command: "codex 'linked issue context'", startupCommandDelivery: 'shell-ready' as const } + ])('waits past 300ms and submits once after readiness: %j', async (startup) => { + await adapter.spawn({ cols: 80, rows: 24, ...startup, env: { SHELL: '/bin/zsh' } }) await new Promise((resolve) => setTimeout(resolve, 350)) expect(lastSubprocess.write).not.toHaveBeenCalled() - + expect(lastSpawnOpts).toEqual( + expect.objectContaining({ startupCommandDelivery: 'shell-ready' }) + ) lastSubprocess._simulateData('\x1b]777;orca-shell-ready\x07') lastSubprocess._simulateData('\r\nuser@host $ ') await waitFor(() => vi.mocked(lastSubprocess.write).mock.calls.length > 0) - expect(lastSubprocess.write).toHaveBeenCalledWith("codex 'linked issue context'\n") + expect(lastSubprocess.write).toHaveBeenCalledExactlyOnceWith(`${startup.command}\n`) }) }) diff --git a/src/main/daemon/daemon-pty-session-spawn.ts b/src/main/daemon/daemon-pty-session-spawn.ts index 235b286b0bc..5799e817ac9 100644 --- a/src/main/daemon/daemon-pty-session-spawn.ts +++ b/src/main/daemon/daemon-pty-session-spawn.ts @@ -1,5 +1,3 @@ -import { recognizeAgentProcessFromCommandLine } from '../../shared/agent-process-recognition' -import { shouldUseShellReadyStartupDelivery } from '../../shared/codex-startup-delivery' import type { HistoryRecoveryContext, PendingDaemonSpawnOperation @@ -11,7 +9,6 @@ import { DaemonPtySpawnResult } from './daemon-pty-spawn-result' import type { DaemonPtySpawnContext } from './daemon-pty-spawn-request' import type { ColdRestoreInfo } from './history-reader' import { mintPtySessionId } from './pty-session-id' -import { CODEX_SHELL_READY_TIMEOUT_MS } from './session-shell-ready-barrier' import { supportsPtyStartupBarrier } from './shell-ready' import { getRecoveredHistorySeedSegments } from './terminal-history-seed-segments' import { AGENT_SESSION_CLAIM_DAEMON_PROTOCOL_VERSION, type CreateOrAttachResult } from './types' @@ -213,21 +210,9 @@ export abstract class DaemonPtySessionSpawn extends DaemonPtySpawnResult { let effectiveRows = restoreInfo?.rows ?? opts.rows const shellReadySupported = opts.command ? supportsPtyStartupBarrier(opts.env ?? {}) : false - const isCodexStartupCommand = - recognizeAgentProcessFromCommandLine(opts.command)?.agent === 'codex' - const shouldWaitForShellReady = - isCodexStartupCommand && - shouldUseShellReadyStartupDelivery({ - command: opts.command, - startupCommandDelivery: opts.startupCommandDelivery - }) - const shellReadyTimeoutMs = - shellReadySupported && isCodexStartupCommand && !shouldWaitForShellReady - ? CODEX_SHELL_READY_TIMEOUT_MS - : undefined - const context: DaemonPtySpawnContext = { - opts, + // Older daemons also need the existing hint to enable their ready marker. + opts: opts.command ? { ...opts, startupCommandDelivery: 'shell-ready' } : opts, operation, historyRecovery, requestedSessionId, @@ -241,7 +226,6 @@ export abstract class DaemonPtySessionSpawn extends DaemonPtySpawnResult { effectiveCols, effectiveRows, shellReadySupported, - shellReadyTimeoutMs, historySeedSegments: restoreInfo ? getRecoveredHistorySeedSegments(restoreInfo) : null, detectColdRestore } diff --git a/src/main/daemon/daemon-pty-spawn-request.ts b/src/main/daemon/daemon-pty-spawn-request.ts index 7de9b21926a..61bf9b4eb85 100644 --- a/src/main/daemon/daemon-pty-spawn-request.ts +++ b/src/main/daemon/daemon-pty-spawn-request.ts @@ -33,7 +33,6 @@ export type DaemonPtySpawnContext = { effectiveCols: number effectiveRows: number shellReadySupported: boolean - shellReadyTimeoutMs: number | undefined historySeedSegments: readonly string[] | null detectColdRestore(options?: { ignoreCleanEnd?: boolean }): Promise } @@ -123,9 +122,6 @@ export abstract class DaemonPtySpawnRequest extends DaemonPtyRuntimeState { ? undefined : opts.terminalWindowsPowerShellImplementation, shellReadySupported: context.attachOnly ? false : context.shellReadySupported, - ...(!context.attachOnly && context.shellReadyTimeoutMs !== undefined - ? { shellReadyTimeoutMs: context.shellReadyTimeoutMs } - : {}), ...(historySeed ? { historySeed } : {}), ...(historySeedTransferId ? { historySeedTransferId } : {}), ...(this.supportsStartupIngress && !context.attachOnly && opts.startupIngress diff --git a/src/main/daemon/post-ready-flush-gate.test.ts b/src/main/daemon/post-ready-flush-gate.test.ts index f8e58ba7c7d..7152e4be464 100644 --- a/src/main/daemon/post-ready-flush-gate.test.ts +++ b/src/main/daemon/post-ready-flush-gate.test.ts @@ -20,6 +20,52 @@ describe('PostReadyFlushGate', () => { vi.useRealTimers() }) + it('waits while a slow prompt still has cooked echo enabled', async () => { + const probe = vi.fn().mockResolvedValue('other') + gate = new PostReadyFlushGate(onFlush, probe) + gate.arm(true) + await vi.advanceTimersByTimeAsync(1000) + expect(onFlush).not.toHaveBeenCalled() + expect(gate.isPending).toBe(true) + probe.mockResolvedValue('line-editor') + await vi.advanceTimersByTimeAsync(250) + expect(onFlush).toHaveBeenCalledTimes(1) + expect(gate.isPending).toBe(false) + }) + + it('drops an in-flight probe result after teardown', async () => { + let resolveProbe!: (state: 'line-editor') => void + gate = new PostReadyFlushGate( + onFlush, + () => + new Promise((resolve) => { + resolveProbe = resolve + }) + ) + gate.arm(true) + await vi.advanceTimersByTimeAsync(POST_READY_FLUSH_DELAY_MS) + gate.clear() + resolveProbe('line-editor') + await vi.advanceTimersByTimeAsync(1000) + expect(onFlush).not.toHaveBeenCalled() + expect(gate.isPending).toBe(false) + }) + + it.each(['unknown', 'unavailable'])('retains the fallback when probing is %s', async (state) => { + gate = new PostReadyFlushGate(onFlush, vi.fn().mockResolvedValue(state)) + gate.arm() + await vi.advanceTimersByTimeAsync(POST_READY_FLUSH_FALLBACK_MS) + expect(onFlush).toHaveBeenCalledTimes(1) + }) + + it('bounds waiting when a shell never enters its line editor', async () => { + gate = new PostReadyFlushGate(onFlush, vi.fn().mockResolvedValue('other')) + gate.arm(true) + await vi.advanceTimersByTimeAsync(15_500) + expect(onFlush).toHaveBeenCalledTimes(1) + expect(gate.isPending).toBe(false) + }) + it('does not flush immediately when armed', () => { gate.arm() expect(onFlush).not.toHaveBeenCalled() diff --git a/src/main/daemon/post-ready-flush-gate.ts b/src/main/daemon/post-ready-flush-gate.ts index 199bb601024..315dbf77809 100644 --- a/src/main/daemon/post-ready-flush-gate.ts +++ b/src/main/daemon/post-ready-flush-gate.ts @@ -1,22 +1,4 @@ -/** - * Defers a flush callback until after the shell has drawn its prompt and - * switched the PTY into raw mode. - * - * Why: the OSC 777 shell-ready marker fires from zsh's precmd_functions / - * bash's PROMPT_COMMAND — before the shell draws its prompt and before - * zle/readline flips the PTY into raw mode. Flushing queued input then lets - * the kernel (ECHO still on) echo the command once, and the line editor - * redraws it under the prompt — producing a visible duplicate (e.g. "claude" - * appears twice on agent launch). - * - * Strategy: after arm() is called, wait for prompt bytes plus a short delay - * for the tcsetattr() that enables raw mode. If the marker-completing scan - * already saw post-marker bytes, use that same short path immediately. - * A conservative wall-clock fallback covers ambiguous marker-only cases. - * - * Mirrors the gate in local-pty-shell-ready.ts::writeStartupCommandWhenShellReady, - * which solves the same race on the non-daemon path. - */ +import type { PtySlaveLineEditorProbe } from '../../shared/pty-slave-line-discipline-echo' export const POST_READY_FLUSH_DELAY_MS = 30 export const POST_READY_FLUSH_FALLBACK_MS = 200 @@ -26,18 +8,58 @@ export class PostReadyFlushGate { private postDataTimer: ReturnType | null = null private fallbackTimer: ReturnType | null = null - constructor(private readonly onFlush: () => void) {} + private generation = 0 + private probePending = false + private probeDeadline = 0 + + constructor( + private readonly onFlush: () => void, + private readonly lineEditorProbe?: PtySlaveLineEditorProbe + ) {} + + private async flushWhenLineEditorReady(): Promise { + const generation = this.generation + this.probePending = true + const state = await this.lineEditorProbe?.().catch(() => 'unknown') + if (generation !== this.generation) { + return + } + this.probePending = false + // Slow prompt hooks can leave cooked echo enabled long after the ready marker. + if (state === 'other' && Date.now() < this.probeDeadline) { + this.postDataTimer = setTimeout(() => { + this.postDataTimer = null + void this.flushWhenLineEditorReady() + }, 250) + return + } + this.onFlush() + } + + private flush(): void { + if (this.lineEditorProbe) { + void this.flushWhenLineEditorReady() + } else { + this.onFlush() + } + } /** True between arm() and the actual flush firing. Callers should treat * input as still-queued during this window to preserve ordering. */ get isPending(): boolean { - return this.awaitingPromptDraw || this.postDataTimer !== null || this.fallbackTimer !== null + return ( + this.probePending || + this.awaitingPromptDraw || + this.postDataTimer !== null || + this.fallbackTimer !== null + ) } /** Arm the gate after observing the shell-ready marker. Starts the * wall-clock fallback unless the marker scan already observed post-marker * bytes, in which case the short post-data settle path is enough. */ arm(postMarkerBytesObserved = false): void { + this.probeDeadline = Date.now() + 15_000 this.awaitingPromptDraw = true if (postMarkerBytesObserved) { this.notifyData() @@ -46,7 +68,7 @@ export class PostReadyFlushGate { this.fallbackTimer = setTimeout(() => { this.fallbackTimer = null this.awaitingPromptDraw = false - this.onFlush() + this.flush() }, POST_READY_FLUSH_FALLBACK_MS) } @@ -65,13 +87,15 @@ export class PostReadyFlushGate { if (this.postDataTimer === null) { this.postDataTimer = setTimeout(() => { this.postDataTimer = null - this.onFlush() + this.flush() }, POST_READY_FLUSH_DELAY_MS) } } /** Cancel any pending flush. Call on session teardown. */ clear(): void { + this.generation += 1 + this.probePending = false this.awaitingPromptDraw = false if (this.postDataTimer) { clearTimeout(this.postDataTimer) 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 5267778088c..d5dbc33246c 100644 --- a/src/main/daemon/pty-subprocess-managed-agent-env.test.ts +++ b/src/main/daemon/pty-subprocess-managed-agent-env.test.ts @@ -261,7 +261,7 @@ describe('createPtySubprocess', () => { expect(lastCall[2].env.ORCA_SHELL_FEATURES).not.toContain('ready') }) - it('keeps plain Codex startup commands on the no-marker wrapper', async () => { + it('enables readiness and shell identity for plain Codex startup', async () => { const proc = mockPtyProcess() spawnMock.mockReturnValue(proc) const platform = Object.getOwnPropertyDescriptor(process, 'platform') @@ -285,7 +285,8 @@ describe('createPtySubprocess', () => { 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).not.toContain('ready') + 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 () => { diff --git a/src/main/daemon/pty-subprocess/shell-launch-plan.ts b/src/main/daemon/pty-subprocess/shell-launch-plan.ts index ed8cd5e6a22..2f7baaba665 100644 --- a/src/main/daemon/pty-subprocess/shell-launch-plan.ts +++ b/src/main/daemon/pty-subprocess/shell-launch-plan.ts @@ -32,7 +32,6 @@ import { recognizeAgentProcessFromCommandLine, type RecognizedAgentProcess } from '../../../shared/agent-process-recognition' -import { shouldUseShellReadyStartupDelivery } from '../../../shared/codex-startup-delivery' import { ORCA_HERMES_STARTUP_QUERY_ENV } from '../../../shared/hermes-startup-query' import { WINDOWS_GIT_BASH_SHELL } from '../../../shared/windows-terminal-shell' import { getShellLaunchConfig, resolvePtyShellPath } from '../shell-ready' @@ -60,7 +59,6 @@ export function createPtyShellLaunchPlan( let startupCommandDeliveredInShellArgs = false let windowsFallbackAttempts: WindowsShellSpawnAttempt[] = [] const startupAgentRecognition = recognizeAgentProcessFromCommandLine(opts.command) - const isCodexStartupCommand = startupAgentRecognition?.agent === 'codex' const requestedCwd = opts.cwd || resolveSafePtyDefaultCwd() if (opts.command && startupAgentRecognition) { assertSafeAgentStartupCwd(requestedCwd, opts.command) @@ -190,13 +188,8 @@ export function createPtyShellLaunchPlan( `[daemon/pty] Preferred shell "${preferredShellPath}" is unavailable, fell back to "${shellPath}"` ) } - const waitsForShellReady = - Boolean(opts.command) && - (!isCodexStartupCommand || - shouldUseShellReadyStartupDelivery({ - command: opts.command as string, - startupCommandDelivery: opts.startupCommandDelivery - })) + // PTY input must wait for the line editor, including plain Codex launches. + const waitsForShellReady = Boolean(opts.command) delete env.ORCA_SHELL_FEATURES const shellLaunch = getShellLaunchConfig( shellPath, diff --git a/src/main/daemon/session-shell-ready-barrier.ts b/src/main/daemon/session-shell-ready-barrier.ts index e7562e23224..55847519e6a 100644 --- a/src/main/daemon/session-shell-ready-barrier.ts +++ b/src/main/daemon/session-shell-ready-barrier.ts @@ -1,3 +1,4 @@ +import { createPtySlaveLineEditorProbe } from '../../shared/pty-slave-line-discipline-echo' import { installDeviceAttributesResponder, STARTUP_DA1_RESPONSE @@ -19,8 +20,6 @@ import { basename } from 'node:path' import type { ShellReadyState } from './types' const SHELL_READY_TIMEOUT_MS = 15_000 -// Why: Codex skips marker-gated command delivery; this only bounds older daemon/local paths that still report shell-ready for Codex. -export const CODEX_SHELL_READY_TIMEOUT_MS = 300 export type SessionShellReadyBarrierDeps = { sessionId: string @@ -69,7 +68,10 @@ export class SessionShellReadyBarrier { this._state = 'unsupported' } - this.postReadyFlushGate = new PostReadyFlushGate(() => this.flushPreReadyQueue()) + this.postReadyFlushGate = new PostReadyFlushGate( + () => this.flushPreReadyQueue(), + createPtySlaveLineEditorProbe(deps.subprocess.slavePath) + ) } get state(): ShellReadyState {