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.
This commit is contained in:
Neil
2026-04-24 15:15:31 -07:00
parent 6208cfc47b
commit 5b3134fafd
4 changed files with 177 additions and 73 deletions
@@ -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<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()
})
})
+75
View File
@@ -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<typeof setTimeout> | null = null
private fallbackTimer: ReturnType<typeof setTimeout> | 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
}
}
}
+7 -17
View File
@@ -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'])
})
+9 -56
View File
@@ -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<typeof setTimeout> | null = null
private killTimer: ReturnType<typeof setTimeout> | null = null
// See POST_READY_FLUSH_DELAY_MS for why the flush is deferred.
private postReadyFlushPending = false
private postReadyFlushTimer: ReturnType<typeof setTimeout> | null = null
private postReadyFallbackTimer: ReturnType<typeof setTimeout> | 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 = []