mirror of
https://github.com/stablyai/orca.git
synced 2026-10-12 16:03:03 +00:00
fix: wait for terminal line editor before agent startup input
This commit is contained in:
@@ -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`)
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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<ColdRestoreInfo | null>
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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<typeof setTimeout> | null = null
|
||||
private fallbackTimer: ReturnType<typeof setTimeout> | 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<void> {
|
||||
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)
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user