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 00000000000..6614f3fe79b --- /dev/null +++ b/src/main/daemon/post-ready-flush-gate.test.ts @@ -0,0 +1,86 @@ +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() + }) +}) 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 00000000000..a78442bdf2b --- /dev/null +++ b/src/main/daemon/post-ready-flush-gate.ts @@ -0,0 +1,75 @@ +/** + * 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) {} + + /** 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 a664653e21a..1fc7307778b 100644 --- a/src/main/daemon/session.test.ts +++ b/src/main/daemon/session.test.ts @@ -1,4 +1,3 @@ -/* oxlint-disable max-lines */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { Session } from './session' import type { SessionState, ShellReadyState } from './types' @@ -187,29 +186,20 @@ describe('Session', () => { expect(subprocess.written).toEqual([]) }) - // Why: the shell-ready marker fires from precmd/PROMPT_COMMAND, before - // the shell draws its prompt and before readline flips the PTY into raw - // mode. Flushing then causes the kernel (ECHO on) to echo the command - // once, and readline redraws it under the prompt — producing "claude - // claude" on agent launch. The flush must defer until after prompt draw. - it('defers flush until after prompt draw + short delay', () => { + // 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; here we + // just verify Session wires the gate in (no write on the raw marker). + it('defers flush past the shell-ready marker to avoid kernel ECHO duplication', () => { createSession({ shellReadySupported: true }) - session.write('pre-ready input') + session.write('claude\n') subprocess.simulateData('\x1b]777;orca-shell-ready\x07') expect(session.shellState).toBe('ready' satisfies ShellReadyState) expect(subprocess.written).toEqual([]) subprocess.simulateData('\r\nuser@host $ ') vi.advanceTimersByTime(30) - expect(subprocess.written).toEqual(['pre-ready input']) - }) - - it('flushes via fallback timer if the prompt arrived in the marker chunk', () => { - createSession({ shellReadySupported: true }) - session.write('claude\n') - subprocess.simulateData('\x1b]777;orca-shell-ready\x07user@host $ ') - expect(subprocess.written).toEqual([]) - vi.advanceTimersByTime(50) expect(subprocess.written).toEqual(['claude\n']) }) diff --git a/src/main/daemon/session.ts b/src/main/daemon/session.ts index 8f32b7cffc7..7ed9bf17019 100644 --- a/src/main/daemon/session.ts +++ b/src/main/daemon/session.ts @@ -1,25 +1,11 @@ -/* eslint-disable max-lines -- Why: post-ready flush gating (see POST_READY_FLUSH_DELAY_MS) -adds ~25 lines of timer/state wiring across handleSubprocessData, transitionToReady, -and cleanup paths. Splitting it out of Session would scatter tightly coupled shell -lifecycle logic across files without a cleaner ownership seam. */ 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 const KILL_TIMEOUT_MS = 5_000 const SHELL_READY_MARKER = '\x1b]777;orca-shell-ready\x07' -// Why: the OSC 777 marker fires from precmd/PROMPT_COMMAND, before the shell -// draws its prompt and before zle/readline flips the PTY into raw mode. -// Flushing the pre-ready queue 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" twice when launching an agent). Wait for -// the next data chunk (the prompt draw) plus a short delay for the -// tcsetattr() that enables raw mode, with a wall-clock fallback for cases -// where the prompt arrived in the same chunk as the marker. Matches the gate -// in local-pty-shell-ready.ts::writeStartupCommandWhenShellReady. -const POST_READY_FLUSH_DELAY_MS = 30 -const POST_READY_FLUSH_FALLBACK_MS = 50 export type SubprocessHandle = { pid: number @@ -61,10 +47,7 @@ export class Session { private markerBuffer = '' private shellReadyTimer: ReturnType | null = null private killTimer: ReturnType | null = null - // See POST_READY_FLUSH_DELAY_MS for why the flush is deferred. - private postReadyFlushPending = false - private postReadyFlushTimer: ReturnType | null = null - private postReadyFallbackTimer: ReturnType | null = null + private postReadyFlushGate: PostReadyFlushGate constructor(opts: SessionOptions) { this.sessionId = opts.sessionId @@ -89,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)) } @@ -228,7 +212,7 @@ export class Session { clearTimeout(this.killTimer) this.killTimer = null } - this.clearPostReadyTimers() + this.postReadyFlushGate.clear() this.attachedClients = [] this.preReadyStdinQueue = [] @@ -249,20 +233,8 @@ export class Session { if (this._shellState === 'pending') { this.scanForShellMarker(data) - } else if (this.postReadyFlushPending) { - // Why: prompt draw arrived. Swap the wall-clock fallback for the short - // post-data delay so readline has time to enable raw mode first. - this.postReadyFlushPending = false - if (this.postReadyFallbackTimer) { - clearTimeout(this.postReadyFallbackTimer) - this.postReadyFallbackTimer = null - } - if (this.postReadyFlushTimer === null) { - this.postReadyFlushTimer = setTimeout(() => { - this.postReadyFlushTimer = null - this.flushPreReadyQueue() - }, POST_READY_FLUSH_DELAY_MS) - } + } else { + this.postReadyFlushGate.notifyData() } // Broadcast to attached clients @@ -287,7 +259,7 @@ export class Session { clearTimeout(this.shellReadyTimer) this.shellReadyTimer = null } - this.clearPostReadyTimers() + this.postReadyFlushGate.clear() for (const client of this.attachedClients) { client.onExit(code) @@ -320,13 +292,7 @@ export class Session { if (this.preReadyStdinQueue.length === 0) { return } - // See POST_READY_FLUSH_DELAY_MS. - this.postReadyFlushPending = true - this.postReadyFallbackTimer = setTimeout(() => { - this.postReadyFallbackTimer = null - this.postReadyFlushPending = false - this.flushPreReadyQueue() - }, POST_READY_FLUSH_FALLBACK_MS) + this.postReadyFlushGate.arm() } private onShellReadyTimeout(): void { @@ -339,7 +305,6 @@ export class Session { } private flushPreReadyQueue(): void { - this.postReadyFlushPending = false const queued = this.preReadyStdinQueue this.preReadyStdinQueue = [] for (const data of queued) { @@ -347,18 +312,6 @@ export class Session { } } - private clearPostReadyTimers(): void { - this.postReadyFlushPending = false - if (this.postReadyFlushTimer) { - clearTimeout(this.postReadyFlushTimer) - this.postReadyFlushTimer = null - } - if (this.postReadyFallbackTimer) { - clearTimeout(this.postReadyFallbackTimer) - this.postReadyFallbackTimer = null - } - } - private forceDispose(): void { if (this._state === 'exited') { return @@ -377,7 +330,7 @@ export class Session { clearTimeout(this.killTimer) this.killTimer = null } - this.clearPostReadyTimers() + this.postReadyFlushGate.clear() const clients = this.attachedClients this.attachedClients = []