From 37937db52e05f140a319b206b646d57c19291b0d Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 7 Aug 2026 11:53:15 -0700 Subject: [PATCH] fix(terminal): answer DA1 past the shell-ready barrier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The barrier queues all inbound input until the ready marker, including the renderer's DA1 reply. A shell that withholds its first prompt until DA1 is answered — fish waits 10s — therefore never emits the marker that would release the reply it is waiting for. Measured: 10.37s to launch an agent, versus 0.35s once the reply lands. Answer DA1 from the daemon while the barrier holds, writing straight to the subprocess so the reply bypasses the queue, and consume the query so the renderer's xterm cannot also reply. Released on ready, timeout, or dispose, handing DA1 back to the renderer for steady state. Consolidates the identical DA1 handler the ConPTY override already used. --- src/main/daemon/headless-emulator.ts | 20 ++-- src/main/daemon/session.test.ts | 14 +++ src/main/daemon/session.ts | 24 ++++ ...tartup-device-attributes-responder.test.ts | 110 ++++++++++++++++++ .../startup-device-attributes-responder.ts | 40 +++++++ 5 files changed, 200 insertions(+), 8 deletions(-) create mode 100644 src/main/daemon/startup-device-attributes-responder.test.ts create mode 100644 src/main/daemon/startup-device-attributes-responder.ts diff --git a/src/main/daemon/headless-emulator.ts b/src/main/daemon/headless-emulator.ts index ca8bb6c331f..195b97a30f6 100644 --- a/src/main/daemon/headless-emulator.ts +++ b/src/main/daemon/headless-emulator.ts @@ -18,6 +18,7 @@ import { installTerminalViewAttributeResponder, type TerminalViewAttributeResponder } from './terminal-view-attribute-responder' +import { installDeviceAttributesResponder } from './startup-device-attributes-responder' import type { TerminalSnapshot, TerminalModes } from './types' import type { TerminalOscLinkRange } from '../../shared/terminal-osc-link-ranges' @@ -100,23 +101,26 @@ export class HeadlessEmulator { } } - /** ConPTY 1.22+ blocks at spawn awaiting a DA1 reply; answers `CSI ?61;4c` and consumes the query so xterm's default `?1;2c` can't double-reply. */ + /** ConPTY 1.22+ blocks at spawn awaiting a DA1 reply. See startup-device-attributes-responder. */ installConptyPrimaryDeviceAttributesOverride(): void { // Why idempotent: installed at creation and again at spawn-mark time (which can land later), so it's never stacked. if (this.conptyDa1OverrideInstalled) { return } this.conptyDa1OverrideInstalled = true - this.terminal.parser.registerCsiHandler({ final: 'c' }, (params) => { - const isPrimaryQuery = params.length === 0 || (params.length === 1 && params[0] === 0) - if (!isPrimaryQuery) { - return false - } - this.emitQueryReply(CONPTY_DA1_RESPONSE) - return true + installDeviceAttributesResponder({ + parser: this.terminal.parser, + response: CONPTY_DA1_RESPONSE, + reply: (data) => this.emitQueryReply(data) }) } + /** Why exposed: responder modules install handlers here (see the view-attribute and + * device-attributes responders); the caller owns disposal. */ + get responderParser(): Terminal['parser'] { + return this.terminal.parser + } + /** Headless core has no theme service, so OSC 4/10/11/12 and DSR ?996n answer from the renderer's pushed attributes; daemon Session must never call this. */ installViewAttributeResponder(getBaseAttributes: () => TerminalViewAttributes | null): void { if (this.viewAttributeResponder) { diff --git a/src/main/daemon/session.test.ts b/src/main/daemon/session.test.ts index bc4b45484f3..a8b46e52afd 100644 --- a/src/main/daemon/session.test.ts +++ b/src/main/daemon/session.test.ts @@ -331,6 +331,20 @@ describe('Session', () => { }) describe('shell readiness gating', () => { + // Why: the renderer's DA1 reply would be queued here, and a shell that withholds its + // first prompt until DA1 is answered never emits the marker that would release it. + it('answers DA1 past the pre-ready queue while ordinary input still waits', async () => { + createSession({ shellReadySupported: true }) + session.write('queued\n') + expect(subprocess.written).toEqual([]) + + subprocess.simulateData('\x1b[0c') + await vi.advanceTimersByTimeAsync(1) + + expect(subprocess.written).toEqual(['\x1b[?1;2c']) + expect(session.shellState).toBe('pending') + }) + // 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 diff --git a/src/main/daemon/session.ts b/src/main/daemon/session.ts index d9c5553d3e8..d8b5d3a4552 100644 --- a/src/main/daemon/session.ts +++ b/src/main/daemon/session.ts @@ -1,5 +1,9 @@ /* oxlint-disable max-lines */ import { HeadlessEmulator } from './headless-emulator' +import { + installDeviceAttributesResponder, + STARTUP_DA1_RESPONSE +} from './startup-device-attributes-responder' import { isValidPtySize, normalizePtySize } from './daemon-pty-size' import { PostReadyFlushGate } from './post-ready-flush-gate' import { @@ -116,6 +120,7 @@ export class Session { private readonly onSessionExit?: (code: number) => void private attachedClients: AttachedClient[] = [] private preReadyStdinQueue: string[] = [] + private releaseStartupDeviceAttributesResponder: (() => void) | null = null private shellReadyScanState: ShellReadyScanState | null = null private shellReadyTimer: ReturnType | null = null private killTimer: ReturnType | null = null @@ -148,6 +153,8 @@ export class Session { wslDistro: opts.wslDistro // No onData: the daemon emulator must never reply to query sequences — the renderer's xterm is // the authoritative responder and a daemon reply would race ahead and clobber it. See HeadlessEmulator. + // The one exception is DA1 while the shell-ready barrier holds (below): the renderer's reply + // would be queued behind the marker it is needed to produce, so it cannot be authoritative there. }) // Why: seed recovery must precede listener registration; shells can emit their prompt synchronously once onData subscribes. // Why the every() short-circuit is safe: writeSync only fails emulator-wide (disposed / no sync write API), so later @@ -160,6 +167,14 @@ export class Session { if (opts.shellReadySupported) { this._shellState = 'pending' this.shellReadyScanState = createShellReadyScanState() + // Why: `write` queues everything until the ready marker, including the renderer's DA1 + // reply — and a shell that withholds its first prompt until DA1 is answered (fish) then + // never emits the marker that would release it. Answer from the daemon, past the queue. + this.releaseStartupDeviceAttributesResponder = installDeviceAttributesResponder({ + parser: this.emulator.responderParser, + response: STARTUP_DA1_RESPONSE, + reply: (data) => this.subprocess.write(data) + }) this.shellReadyTimer = setTimeout(() => { this.onShellReadyTimeout() }, opts.shellReadyTimeoutMs ?? SHELL_READY_TIMEOUT_MS) @@ -514,6 +529,7 @@ export class Session { // Why: `wasTerminating` below must be read BEFORE the `_state = 'exited'` flip — it guards the // "dispose while kill() in flight" case and the invariant needs the pre-flip `_state`; do NOT move it down. + this.releaseStartupDeviceAttributes() this.releaseHeldShellReadyBytes() this.startupIngress.drainAndClose() const wasTerminating = this._isTerminating && this._state !== 'exited' @@ -704,9 +720,16 @@ export class Session { return this.startupIngress.closeQueryAuthority() } + /** Hands DA1 back to the renderer once the barrier is done, however it ended. */ + private releaseStartupDeviceAttributes(): void { + this.releaseStartupDeviceAttributesResponder?.() + this.releaseStartupDeviceAttributesResponder = null + } + private transitionToReady(postMarkerBytesObserved = false): void { this._shellState = 'ready' this.shellReadyScanState = null + this.releaseStartupDeviceAttributes() if (this.shellReadyTimer) { clearTimeout(this.shellReadyTimer) this.shellReadyTimer = null @@ -723,6 +746,7 @@ export class Session { return } this._shellState = 'timed_out' + this.releaseStartupDeviceAttributes() this.releaseHeldShellReadyBytes() this.flushPreReadyQueue() } diff --git a/src/main/daemon/startup-device-attributes-responder.test.ts b/src/main/daemon/startup-device-attributes-responder.test.ts new file mode 100644 index 00000000000..6ce59fd39d9 --- /dev/null +++ b/src/main/daemon/startup-device-attributes-responder.test.ts @@ -0,0 +1,110 @@ +import { describe, expect, it } from 'vitest' +import { + installDeviceAttributesResponder, + STARTUP_DA1_RESPONSE +} from './startup-device-attributes-responder' + +type CsiHandler = (params: number[]) => boolean + +function createFakeParser() { + const handlers: CsiHandler[] = [] + return { + registered: handlers, + parser: { + registerCsiHandler(_id: { final: string }, handler: CsiHandler) { + handlers.push(handler) + return { + dispose() { + const at = handlers.indexOf(handler) + if (at !== -1) { + handlers.splice(at, 1) + } + } + } + } + } as never + } +} + +describe('startup device attributes responder', () => { + it('answers a bare DA1 query and consumes it so the renderer cannot double-reply', () => { + const { parser, registered } = createFakeParser() + const replies: string[] = [] + + installDeviceAttributesResponder({ + parser, + response: STARTUP_DA1_RESPONSE, + reply: (d) => replies.push(d) + }) + + expect(registered[0]?.([])).toBe(true) + expect(replies).toEqual([STARTUP_DA1_RESPONSE]) + }) + + it('answers the explicit `CSI 0 c` form', () => { + const { parser, registered } = createFakeParser() + const replies: string[] = [] + + installDeviceAttributesResponder({ + parser, + response: STARTUP_DA1_RESPONSE, + reply: (d) => replies.push(d) + }) + + expect(registered[0]?.([0])).toBe(true) + expect(replies).toEqual([STARTUP_DA1_RESPONSE]) + }) + + it('answers every occurrence, because shells re-query around each prompt', () => { + const { parser, registered } = createFakeParser() + const replies: string[] = [] + + installDeviceAttributesResponder({ + parser, + response: STARTUP_DA1_RESPONSE, + reply: (d) => replies.push(d) + }) + + registered[0]?.([]) + registered[0]?.([]) + + expect(replies).toEqual([STARTUP_DA1_RESPONSE, STARTUP_DA1_RESPONSE]) + }) + + it('declines non-primary variants so they fall through to the renderer', () => { + const { parser, registered } = createFakeParser() + const replies: string[] = [] + + installDeviceAttributesResponder({ + parser, + response: STARTUP_DA1_RESPONSE, + reply: (d) => replies.push(d) + }) + + // Why: a non-zero parameter is a secondary/tertiary DA request, not DA1. + expect(registered[0]?.([1])).toBe(false) + expect(registered[0]?.([0, 1])).toBe(false) + expect(replies).toEqual([]) + }) + + it('stops answering once disposed, handing the query back to the renderer', () => { + const { parser, registered } = createFakeParser() + const replies: string[] = [] + + const release = installDeviceAttributesResponder({ + parser, + response: STARTUP_DA1_RESPONSE, + reply: (d) => replies.push(d) + }) + release() + + expect(registered).toHaveLength(0) + expect(replies).toEqual([]) + }) + + it('reports the same primary attributes the renderer would, so consuming the query is transparent', () => { + // Why pinned: the renderer's xterm answers `?1;2c` for xterm-* TERMs. Diverging here + // would silently change the capabilities a TUI sees on barrier-gated panes only. + expect(STARTUP_DA1_RESPONSE).toBe('\x1b[?1;2c') + }) +}) diff --git a/src/main/daemon/startup-device-attributes-responder.ts b/src/main/daemon/startup-device-attributes-responder.ts new file mode 100644 index 00000000000..737de5ad43b --- /dev/null +++ b/src/main/daemon/startup-device-attributes-responder.ts @@ -0,0 +1,40 @@ +/** + * Primary Device Attributes (DA1) responders for the daemon's headless emulator. + * + * Both callers answer DA1 on behalf of a renderer that cannot, and both consume + * the query so the renderer's xterm never sees it and cannot double-reply: + * + * - ConPTY 1.22+ blocks at spawn awaiting a DA1 reply. + * - The shell-ready barrier queues all inbound input until the ready marker, + * including the renderer's DA1 reply — and a shell that withholds its first + * prompt until DA1 is answered (fish waits 10s) never emits the marker that + * would release it. That caller's `reply` must write straight to the + * subprocess, past the queue, or the deadlock remains. + */ +import type { Terminal } from '@xterm/headless' + +type DeviceAttributesParser = Pick + +/** Matches what the renderer's xterm answers for xterm-* TERMs, so consuming the + * query upstream cannot change the capabilities a TUI sees. */ +export const STARTUP_DA1_RESPONSE = '\x1b[?1;2c' + +/** Returns a disposer so a caller scoped to startup can hand DA1 back to the + * renderer once its window closes. */ +export function installDeviceAttributesResponder(deps: { + parser: DeviceAttributesParser + response: string + reply: (data: string) => void +}): () => void { + const handler = deps.parser.registerCsiHandler({ final: 'c' }, (params) => { + // Why the param check: only DA1 is answered here. Secondary/tertiary variants + // carry a prefix and must fall through to the renderer. + const isPrimaryQuery = params.length === 0 || (params.length === 1 && params[0] === 0) + if (!isPrimaryQuery) { + return false + } + deps.reply(deps.response) + return true + }) + return () => handler.dispose() +}