diff --git a/src/main/daemon/post-ready-flush-gate.test.ts b/src/main/daemon/post-ready-flush-gate.test.ts new file mode 100644 index 000000000..49379a53b --- /dev/null +++ b/src/main/daemon/post-ready-flush-gate.test.ts @@ -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 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) + }) +}) diff --git a/src/main/daemon/post-ready-flush-gate.ts b/src/main/daemon/post-ready-flush-gate.ts new file mode 100644 index 000000000..2e15b1840 --- /dev/null +++ b/src/main/daemon/post-ready-flush-gate.ts @@ -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 | null = null + private fallbackTimer: ReturnType | 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 + } + } +} diff --git a/src/main/daemon/session.test.ts b/src/main/daemon/session.test.ts index c15d1107e..5191f77fd 100644 --- a/src/main/daemon/session.test.ts +++ b/src/main/daemon/session.test.ts @@ -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', () => { diff --git a/src/main/daemon/session.ts b/src/main/daemon/session.ts index e01627ff3..4c398f5a2 100644 --- a/src/main/daemon/session.ts +++ b/src/main/daemon/session.ts @@ -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 | null = null private killTimer: ReturnType | 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 = [] diff --git a/src/main/daemon/terminal-host.test.ts b/src/main/daemon/terminal-host.test.ts index d611ce838..3dcf673dc 100644 --- a/src/main/daemon/terminal-host.test.ts +++ b/src/main/daemon/terminal-host.test.ts @@ -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') }) })