fix(daemon): defer startup flush past shell raw-mode switch (#1060)
* fix(daemon): defer startup-command flush past shell raw-mode switch When launching an agent (e.g. `claude`) through the quick-launch menu, the command name appeared twice in the terminal — once from kernel echo, once from readline's prompt redraw. The daemon session was flushing its pre-ready stdin queue the moment the OSC 777 shell-ready marker arrived, but that marker fires from precmd_functions / PROMPT_COMMAND — before the shell draws its prompt and before zle/readline flips the PTY into raw mode. Writing while ECHO was still on produced the visible duplicate. Mirror the gating already used by the non-daemon path (local-pty-shell-ready.ts::writeStartupCommandWhenShellReady): wait for the next data chunk after the marker (the prompt draw) plus a short 30ms delay, with a 50ms wall-clock fallback for the case where the prompt arrives in the same chunk as the marker. This regression became visible after #1025 made the shell-ready wrappers persist reliably — previously the marker was often missed and the 15s fallback path fired long after the shell was already in raw mode, masking the race. * refactor(daemon): extract PostReadyFlushGate out of Session Factor the post-ready flush gating out of Session into its own class in post-ready-flush-gate.ts. Session's only responsibility is now to arm() the gate on shell-ready, notifyData() on subsequent PTY data, and clear() on teardown. Removes the max-lines oxlint-disable added in the previous commit and puts the timing behavior behind focused unit tests. No behavior change — kept as a separate refactor commit from the behavior fix for reviewability. * fix(daemon): keep queueing writes while post-ready flush gate is pending Codex review caught an ordering regression: once transitionToReady() sets _shellState to 'ready', any Session.write() that arrives during the 30–50ms gate window was being written directly to the subprocess, bypassing the still-unflushed preReadyStdinQueue. That let late input race ahead of the buffered startup command. Expose PostReadyFlushGate.isPending and continue queuing while the gate is armed so queued writes drain in their original order before any fresh input reaches the subprocess.
This commit is contained in:
parent
622b85c73e
commit
1be13d58f3
|
|
@ -0,0 +1,97 @@
|
|||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
PostReadyFlushGate,
|
||||
POST_READY_FLUSH_DELAY_MS,
|
||||
POST_READY_FLUSH_FALLBACK_MS
|
||||
} from './post-ready-flush-gate'
|
||||
|
||||
describe('PostReadyFlushGate', () => {
|
||||
let onFlush: ReturnType<typeof vi.fn<() => void>>
|
||||
let gate: PostReadyFlushGate
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
onFlush = vi.fn<() => void>()
|
||||
gate = new PostReadyFlushGate(onFlush)
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
gate.clear()
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('does not flush immediately when armed', () => {
|
||||
gate.arm()
|
||||
expect(onFlush).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('flushes via short delay after notifyData signals the prompt draw', () => {
|
||||
gate.arm()
|
||||
gate.notifyData()
|
||||
expect(onFlush).not.toHaveBeenCalled()
|
||||
|
||||
vi.advanceTimersByTime(POST_READY_FLUSH_DELAY_MS)
|
||||
expect(onFlush).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('flushes via wall-clock fallback when no notifyData arrives', () => {
|
||||
gate.arm()
|
||||
|
||||
vi.advanceTimersByTime(POST_READY_FLUSH_FALLBACK_MS)
|
||||
expect(onFlush).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('ignores notifyData before arm()', () => {
|
||||
gate.notifyData()
|
||||
vi.advanceTimersByTime(1000)
|
||||
expect(onFlush).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('notifyData after the fallback fired is a no-op', () => {
|
||||
gate.arm()
|
||||
vi.advanceTimersByTime(POST_READY_FLUSH_FALLBACK_MS)
|
||||
expect(onFlush).toHaveBeenCalledTimes(1)
|
||||
|
||||
gate.notifyData()
|
||||
vi.advanceTimersByTime(1000)
|
||||
expect(onFlush).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('only the first notifyData schedules the short-delay flush', () => {
|
||||
gate.arm()
|
||||
gate.notifyData()
|
||||
gate.notifyData()
|
||||
gate.notifyData()
|
||||
|
||||
vi.advanceTimersByTime(POST_READY_FLUSH_DELAY_MS)
|
||||
expect(onFlush).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('clear() cancels a pending fallback flush', () => {
|
||||
gate.arm()
|
||||
gate.clear()
|
||||
|
||||
vi.advanceTimersByTime(POST_READY_FLUSH_FALLBACK_MS * 2)
|
||||
expect(onFlush).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('clear() cancels a pending post-data flush', () => {
|
||||
gate.arm()
|
||||
gate.notifyData()
|
||||
gate.clear()
|
||||
|
||||
vi.advanceTimersByTime(POST_READY_FLUSH_DELAY_MS * 2)
|
||||
expect(onFlush).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('isPending is true throughout the gate window and false once flush fires', () => {
|
||||
expect(gate.isPending).toBe(false)
|
||||
gate.arm()
|
||||
expect(gate.isPending).toBe(true)
|
||||
gate.notifyData()
|
||||
expect(gate.isPending).toBe(true)
|
||||
vi.advanceTimersByTime(POST_READY_FLUSH_DELAY_MS)
|
||||
expect(gate.isPending).toBe(false)
|
||||
expect(onFlush).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,81 @@
|
|||
/**
|
||||
* 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 the next PTY data chunk (the
|
||||
* prompt draw) plus a short delay for the tcsetattr() that enables raw mode.
|
||||
* A wall-clock fallback covers the case where the prompt arrives in the same
|
||||
* chunk as the marker, so no follow-up notifyData() ever fires.
|
||||
*
|
||||
* Mirrors the gate in local-pty-shell-ready.ts::writeStartupCommandWhenShellReady,
|
||||
* which solves the same race on the non-daemon path.
|
||||
*/
|
||||
|
||||
export const POST_READY_FLUSH_DELAY_MS = 30
|
||||
export const POST_READY_FLUSH_FALLBACK_MS = 50
|
||||
|
||||
export class PostReadyFlushGate {
|
||||
private awaitingPromptDraw = false
|
||||
private postDataTimer: ReturnType<typeof setTimeout> | null = null
|
||||
private fallbackTimer: ReturnType<typeof setTimeout> | null = null
|
||||
|
||||
constructor(private readonly onFlush: () => void) {}
|
||||
|
||||
/** 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
|
||||
}
|
||||
|
||||
/** Arm the gate after observing the shell-ready marker. Starts the
|
||||
* wall-clock fallback; the flush fires when the fallback elapses or when
|
||||
* notifyData() observes a subsequent PTY data chunk. */
|
||||
arm(): void {
|
||||
this.awaitingPromptDraw = true
|
||||
this.fallbackTimer = setTimeout(() => {
|
||||
this.fallbackTimer = null
|
||||
this.awaitingPromptDraw = false
|
||||
this.onFlush()
|
||||
}, POST_READY_FLUSH_FALLBACK_MS)
|
||||
}
|
||||
|
||||
/** Report a PTY data chunk observed after arm(). The first such call swaps
|
||||
* the wall-clock fallback for the short post-data delay so readline has
|
||||
* time to enable raw mode before the flush fires. */
|
||||
notifyData(): void {
|
||||
if (!this.awaitingPromptDraw) {
|
||||
return
|
||||
}
|
||||
this.awaitingPromptDraw = false
|
||||
if (this.fallbackTimer) {
|
||||
clearTimeout(this.fallbackTimer)
|
||||
this.fallbackTimer = null
|
||||
}
|
||||
if (this.postDataTimer === null) {
|
||||
this.postDataTimer = setTimeout(() => {
|
||||
this.postDataTimer = null
|
||||
this.onFlush()
|
||||
}, POST_READY_FLUSH_DELAY_MS)
|
||||
}
|
||||
}
|
||||
|
||||
/** Cancel any pending flush. Call on session teardown. */
|
||||
clear(): void {
|
||||
this.awaitingPromptDraw = false
|
||||
if (this.postDataTimer) {
|
||||
clearTimeout(this.postDataTimer)
|
||||
this.postDataTimer = null
|
||||
}
|
||||
if (this.fallbackTimer) {
|
||||
clearTimeout(this.fallbackTimer)
|
||||
this.fallbackTimer = null
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -178,24 +178,25 @@ describe('Session', () => {
|
|||
})
|
||||
|
||||
describe('shell readiness gating', () => {
|
||||
it('buffers writes during pending state', () => {
|
||||
// Why: regression guard for "claude claude" double-echo. The marker fires
|
||||
// from precmd before readline switches the PTY into raw mode; flushing
|
||||
// then lets the kernel re-echo the command under the prompt. Detailed
|
||||
// timing behavior is covered by post-ready-flush-gate.test.ts.
|
||||
// Also checks writes that arrive during the gate window keep their order
|
||||
// — the gate continues to queue even though shellState is already 'ready'.
|
||||
it('defers flush past the shell-ready marker and preserves write order', () => {
|
||||
createSession({ shellReadySupported: true })
|
||||
expect(session.shellState).toBe('pending')
|
||||
|
||||
session.write('buffered input')
|
||||
expect(subprocess.written).toEqual([])
|
||||
})
|
||||
|
||||
it('flushes buffered writes when shell marker is detected', () => {
|
||||
createSession({ shellReadySupported: true })
|
||||
|
||||
session.write('pre-ready input')
|
||||
expect(subprocess.written).toEqual([])
|
||||
|
||||
// Simulate the shell marker arriving in PTY output
|
||||
session.write('first\n')
|
||||
subprocess.simulateData('\x1b]777;orca-shell-ready\x07')
|
||||
expect(session.shellState).toBe('ready' satisfies ShellReadyState)
|
||||
expect(subprocess.written).toEqual(['pre-ready input'])
|
||||
session.write('second\n')
|
||||
expect(subprocess.written).toEqual([])
|
||||
|
||||
subprocess.simulateData('\r\nuser@host $ ')
|
||||
vi.advanceTimersByTime(30)
|
||||
expect(subprocess.written).toEqual(['first\n', 'second\n'])
|
||||
})
|
||||
|
||||
it('transitions to timed_out after 15 seconds', () => {
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import { HeadlessEmulator } from './headless-emulator'
|
||||
import { isValidPtySize, normalizePtySize } from './daemon-pty-size'
|
||||
import { PostReadyFlushGate } from './post-ready-flush-gate'
|
||||
import type { SessionState, ShellReadyState, TerminalSnapshot } from './types'
|
||||
|
||||
const SHELL_READY_TIMEOUT_MS = 15_000
|
||||
|
|
@ -46,6 +47,7 @@ export class Session {
|
|||
private markerBuffer = ''
|
||||
private shellReadyTimer: ReturnType<typeof setTimeout> | null = null
|
||||
private killTimer: ReturnType<typeof setTimeout> | null = null
|
||||
private postReadyFlushGate: PostReadyFlushGate
|
||||
|
||||
constructor(opts: SessionOptions) {
|
||||
this.sessionId = opts.sessionId
|
||||
|
|
@ -70,6 +72,7 @@ export class Session {
|
|||
this._shellState = 'unsupported'
|
||||
}
|
||||
|
||||
this.postReadyFlushGate = new PostReadyFlushGate(() => this.flushPreReadyQueue())
|
||||
this.subprocess.onData((data) => this.handleSubprocessData(data))
|
||||
this.subprocess.onExit((code) => this.handleSubprocessExit(code))
|
||||
}
|
||||
|
|
@ -103,7 +106,11 @@ export class Session {
|
|||
return
|
||||
}
|
||||
|
||||
if (this._shellState === 'pending') {
|
||||
// Why: during the post-ready flush gate window (shellState is already
|
||||
// 'ready' but the queue hasn't flushed yet) we must keep queuing. Writing
|
||||
// directly would let fresh input race ahead of the buffered startup
|
||||
// command, changing execution order.
|
||||
if (this._shellState === 'pending' || this.postReadyFlushGate.isPending) {
|
||||
this.preReadyStdinQueue.push(data)
|
||||
return
|
||||
}
|
||||
|
|
@ -209,6 +216,7 @@ export class Session {
|
|||
clearTimeout(this.killTimer)
|
||||
this.killTimer = null
|
||||
}
|
||||
this.postReadyFlushGate.clear()
|
||||
|
||||
this.attachedClients = []
|
||||
this.preReadyStdinQueue = []
|
||||
|
|
@ -229,6 +237,8 @@ export class Session {
|
|||
|
||||
if (this._shellState === 'pending') {
|
||||
this.scanForShellMarker(data)
|
||||
} else {
|
||||
this.postReadyFlushGate.notifyData()
|
||||
}
|
||||
|
||||
// Broadcast to attached clients
|
||||
|
|
@ -253,6 +263,7 @@ export class Session {
|
|||
clearTimeout(this.shellReadyTimer)
|
||||
this.shellReadyTimer = null
|
||||
}
|
||||
this.postReadyFlushGate.clear()
|
||||
|
||||
for (const client of this.attachedClients) {
|
||||
client.onExit(code)
|
||||
|
|
@ -282,7 +293,10 @@ export class Session {
|
|||
clearTimeout(this.shellReadyTimer)
|
||||
this.shellReadyTimer = null
|
||||
}
|
||||
this.flushPreReadyQueue()
|
||||
if (this.preReadyStdinQueue.length === 0) {
|
||||
return
|
||||
}
|
||||
this.postReadyFlushGate.arm()
|
||||
}
|
||||
|
||||
private onShellReadyTimeout(): void {
|
||||
|
|
@ -320,6 +334,7 @@ export class Session {
|
|||
clearTimeout(this.killTimer)
|
||||
this.killTimer = null
|
||||
}
|
||||
this.postReadyFlushGate.clear()
|
||||
|
||||
const clients = this.attachedClients
|
||||
this.attachedClients = []
|
||||
|
|
|
|||
|
|
@ -147,7 +147,15 @@ describe('TerminalHost', () => {
|
|||
|
||||
expect(lastSubprocess.write).not.toHaveBeenCalled()
|
||||
|
||||
// Why: the marker alone no longer flushes — the kernel can still have
|
||||
// ECHO enabled when it arrives. The flush waits for the prompt draw
|
||||
// plus a short delay so readline has switched the PTY into raw mode
|
||||
// first. Otherwise the command would be visibly double-echoed.
|
||||
lastSubprocess._onDataCb?.('\x1b]777;orca-shell-ready\x07')
|
||||
expect(lastSubprocess.write).not.toHaveBeenCalled()
|
||||
|
||||
lastSubprocess._onDataCb?.('\r\nuser@host $ ')
|
||||
await new Promise((r) => setTimeout(r, 40))
|
||||
expect(lastSubprocess.write).toHaveBeenCalledWith('echo hello\n')
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Reference in New Issue