diff --git a/src/main/daemon/session.test.ts b/src/main/daemon/session.test.ts index ea37c9237e5..70aa7a70951 100644 --- a/src/main/daemon/session.test.ts +++ b/src/main/daemon/session.test.ts @@ -233,6 +233,53 @@ describe('Session', () => { expect(session.getSnapshot()?.snapshotAnsi).not.toContain(']10;rgb') }) + it('keeps armed POSIX startup authority across the renderer visibility handoff', () => { + createSession({ + ownerBackend: 'posix-pty', + startupIngress: { + colors: { foreground: '#2e3434', background: '#ffffff' }, + deadlineMs: 5_000 + } + }) + const onData = vi.fn() + session.attachClient({ onData, onExit: () => {} }) + session.closeStartupQueryAuthority() + + const foregroundQuery = '\x1b]10;?\x07' + const backgroundQuery = '\x1b]11;?\x07' + const foregroundReply = '\x1b]10;rgb:2e2e/3434/3434\x1b\\' + const backgroundReply = '\x1b]11;rgb:ffff/ffff/ffff\x1b\\' + const foregroundPtyEcho = foregroundReply.replaceAll('\x1b', '^[') + const backgroundPtyEcho = backgroundReply.replaceAll('\x1b', '^[') + const foregroundEcho = '\x071\b10;rgb:2e2e/3434/3434\x07' + const backgroundEcho = '\x0711;rgb:ffff/ffff/ffff\x07' + const suppressedStartupData = [ + foregroundQuery, + backgroundQuery, + foregroundPtyEcho, + backgroundPtyEcho, + foregroundEcho, + backgroundEcho + ] + for (const data of [...suppressedStartupData, 'prompt']) { + subprocess.simulateData(data) + } + + expect(subprocess.written).toEqual([foregroundReply, backgroundReply]) + let rawEndSeq = 0 + expect(onData.mock.calls).toEqual([ + ...suppressedStartupData.map((data) => { + rawEndSeq += data.length + return ['', data.length, true, rawEndSeq] + }), + ['prompt'] + ]) + + subprocess.simulateData(foregroundQuery) + expect(subprocess.written).toEqual([foregroundReply, backgroundReply]) + expect(onData).toHaveBeenLastCalledWith(foregroundQuery) + }) + it('releases a held cooked-echo prefix before taking a snapshot', () => { createSession({ ownerBackend: 'windows-conpty', diff --git a/src/main/ipc/pty-dispatcher-handshake-osc-answers.test.ts b/src/main/ipc/pty-dispatcher-handshake-osc-answers.test.ts index 157efb84d3b..b839a846e0f 100644 --- a/src/main/ipc/pty-dispatcher-handshake-osc-answers.test.ts +++ b/src/main/ipc/pty-dispatcher-handshake-osc-answers.test.ts @@ -315,7 +315,7 @@ describe('registerPtyHandlers', () => { vi.useRealTimers() } }) - it('does not answer ordinary terminal OSC color queries in main', async () => { + it('answers plain-shell startup OSC color queries before renderer batching', async () => { vi.useFakeTimers() const mockProc = createMockProc() spawnMock.mockReturnValue(mockProc.proc) @@ -331,16 +331,20 @@ describe('registerPtyHandlers', () => { background: '#111111' } })) as { id: string } + mockProc.proc.write.mockClear() mainWindow.webContents.send.mockClear() const query = '\x1b]10;?\x1b\\\x1b]11;?\x1b\\' mockProc.emitData(`${query}ready`) - expect(mockProc.proc.write).not.toHaveBeenCalled() + expect(mockProc.proc.write).toHaveBeenCalledWith('\x1b]10;rgb:eeee/eeee/eeee\x1b\\') vi.advanceTimersByTime(2) + expect(mockProc.proc.write).toHaveBeenCalledWith('\x1b]11;rgb:1111/1111/1111\x1b\\') expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:data', { id: spawnResult.id, - data: `${query}ready` + data: 'ready', + rawLength: `${query}ready`.length, + transformed: true }) } finally { vi.useRealTimers() diff --git a/src/main/ipc/terminal-startup-color-query-replies.ts b/src/main/ipc/terminal-startup-color-query-replies.ts index 6d6b8ebff42..e121e0c09e6 100644 --- a/src/main/ipc/terminal-startup-color-query-replies.ts +++ b/src/main/ipc/terminal-startup-color-query-replies.ts @@ -1,7 +1,3 @@ -import { recognizeAgentProcessFromCommandLine } from '../../shared/agent-process-recognition' -import { isTuiAgent } from '../../shared/tui-agent-config' -import { agentKindSchema } from '../../shared/telemetry-events' -import type { SleepingAgentLaunchConfig } from '../../shared/agent-session-resume' import { terminalOscColorQueryReply, type TerminalOscColorQueryReplyColors @@ -24,35 +20,8 @@ function normalizeTerminalColorQueryReplyColors( return colors } -function shouldReplyToStartupTerminalColorQueries(args: { - launchAgent?: unknown - telemetry?: { agent_kind?: unknown } | undefined - command?: string - launchConfig?: SleepingAgentLaunchConfig -}): boolean { - if (isTuiAgent(args.launchAgent)) { - return true - } - const agentKindParse = - args.telemetry?.agent_kind !== undefined - ? agentKindSchema.safeParse(args.telemetry.agent_kind) - : null - if (agentKindParse?.success && agentKindParse.data !== 'other') { - return true - } - const command = args.launchConfig?.agentCommand?.trim() || args.command?.trim() || '' - return recognizeAgentProcessFromCommandLine(command) !== null -} - export function getStartupTerminalColorQueryReplyColors(args: { - launchAgent?: unknown - telemetry?: { agent_kind?: unknown } | undefined - command?: string - launchConfig?: SleepingAgentLaunchConfig terminalColorQueryReplies?: unknown }): TerminalOscColorQueryReplyColors | null { - if (!shouldReplyToStartupTerminalColorQueries(args)) { - return null - } return normalizeTerminalColorQueryReplyColors(args.terminalColorQueryReplies) } diff --git a/src/shared/pty-startup-ingress.test.ts b/src/shared/pty-startup-ingress.test.ts index d5458725fe3..2808fef9771 100644 --- a/src/shared/pty-startup-ingress.test.ts +++ b/src/shared/pty-startup-ingress.test.ts @@ -234,22 +234,38 @@ describe('PtyStartupIngress', () => { expect(visible(malformedEmissions)).toBe('\x1b]10;not-a-query\x07') }) - it('releases a partial query immediately when source authority closes', () => { + it('leaves an unarmed partial query on the renderer path', () => { const emissions: PtyIngressEmission[] = [] const ingress = new PtyStartupIngress({ - intent: { colors: COLORS, deadlineMs: 5_000 }, ownerBackend: 'posix-pty', write: () => {}, onEmission: (emission) => emissions.push(emission) }) ingress.accept('\x1b]10;') - expect(emissions).toEqual([]) + expect(visible(emissions)).toBe('\x1b]10;') ingress.closeQueryAuthority() expect(visible(emissions)).toBe('\x1b]10;') }) + it('hands an armed WSL query to its renderer because host echo containment is unavailable', () => { + const writes: string[] = [] + const emissions: PtyIngressEmission[] = [] + const wslIngress = new PtyStartupIngress({ + intent: { colors: COLORS, deadlineMs: 5_000 }, + ownerBackend: 'windows-wsl', + write: (data) => writes.push(data), + onEmission: (emission) => emissions.push(emission) + }) + wslIngress.closeQueryAuthority() + wslIngress.accept('\x1b]10;?\x07') + + expect(writes).toEqual([]) + expect(visible(emissions)).toBe('\x1b]10;?\x07') + wslIngress.drainAndClose() + }) + it('keeps POSIX, WSL, malformed, and unrelated output unchanged', () => { const input = 'typed\x1b[A\x1b]12;?\x1b\\\x1b]10;not-a-query\x07' vi.useFakeTimers() @@ -422,18 +438,20 @@ describe('PtyStartupIngress', () => { expect(visible(emissions)).toBe('') }) - it('keeps the synchronous write for ConPTY-hosted wsl.exe panes', () => { + it('keeps source ownership before visibility for ConPTY-hosted wsl.exe panes', () => { // Why: a Windows-hosted pty must be answered before conhost's own responder. const writes: string[] = [] + const emissions: PtyIngressEmission[] = [] const ingress = new PtyStartupIngress({ intent: { colors: COLORS, deadlineMs: 5_000 }, ownerBackend: 'windows-wsl', write: (data) => writes.push(data), - onEmission: () => {} + onEmission: (emission) => emissions.push(emission) }) ingress.accept('\x1b]10;?\x07') expect(writes).toEqual([FOREGROUND_REPLY]) + expect(visible(emissions)).toBe('') ingress.drainAndClose() }) @@ -789,6 +807,24 @@ describe('PtyStartupIngress', () => { expect(visible(emissions)).toBe('') }) + it('suppresses zsh line-editor echoes from consecutive startup color replies', () => { + const { ingress, writes, emissions } = createHarness() + ingress.accept('\x1b]10;?\x07\x1b]11;?\x07') + expect(writes).toEqual([FOREGROUND_REPLY, BACKGROUND_REPLY]) + + ingress.accept(FOREGROUND_REPLY.replaceAll('\x1b', '^[')) + ingress.accept(BACKGROUND_REPLY.replaceAll('\x1b', '^[')) + + const firstZshEcho = '\x071\b10;rgb:2e2e/3434/3434\x07' + const split = firstZshEcho.indexOf('/3434', firstZshEcho.indexOf('/3434') + 1) + ingress.accept(`\rFIXTURE% ${firstZshEcho.slice(0, split)}`) + ingress.accept(firstZshEcho.slice(split)) + ingress.accept('\x07' + '11;rgb:ffff/ffff/ffff\x07') + ingress.drainAndClose() + + expect(visible(emissions)).toBe('\rFIXTURE% ') + }) + it('suppresses the kernel caret echo', () => { const { ingress, writes, emissions } = createHarness() ingress.accept('\x1b]10;?\x07') diff --git a/src/shared/pty-startup-ingress.ts b/src/shared/pty-startup-ingress.ts index 15c8a062201..d83b49e2e63 100644 --- a/src/shared/pty-startup-ingress.ts +++ b/src/shared/pty-startup-ingress.ts @@ -130,7 +130,11 @@ export class PtyStartupIngress { this.processEchoSpan(operation.chunk) return case 'close-query': - if (this.ownerBackend !== 'windows-conpty') { + // A visible renderer must not preempt an armed POSIX owner before the shell's startup probes. + if ( + this.ownerBackend !== 'windows-conpty' && + !(this.ownerBackend === 'posix-pty' && this.intent && this.queryOpen) + ) { this.queryOpen = false // Why the echo hold deliberately survives this, unlike `snapshot`: the // handoff ends query *authority*, but a reply already on the wire is still diff --git a/src/shared/pty-startup-reply-delivery.ts b/src/shared/pty-startup-reply-delivery.ts index b95cc643cdc..c917b502d5c 100644 --- a/src/shared/pty-startup-reply-delivery.ts +++ b/src/shared/pty-startup-reply-delivery.ts @@ -33,7 +33,7 @@ export type { PtyStartupReplyEchoMatch } from './pty-startup-reply-echo-shapes' // // Both are projected on every write, which is why (2) is covered at all. -type ExpectedEcho = { projections: readonly EchoProjection[]; remainingBytes: number } +type ExpectedEcho = { projections: EchoProjection[]; remainingBytes: number } // Why bytes and not reads: the echo is a fixed ~30 bytes, but nothing bounds how the tty // chunks them — an SSH relay or a slow drain delivers a few bytes at a time, and a @@ -73,7 +73,9 @@ export class PtyStartupReplyDelivery { // Why register before the write: node-pty can synchronously re-enter onData, so the // echo can arrive inside `writeProvider` itself. const expected: ExpectedEcho | null = - projections.length > 0 ? { projections, remainingBytes: ECHO_SEARCH_BUDGET_BYTES } : null + projections.length > 0 + ? { projections: [...projections], remainingBytes: ECHO_SEARCH_BUDGET_BYTES } + : null if (expected) { this.expectedEchoes.push(expected) } @@ -98,15 +100,32 @@ export class PtyStartupReplyDelivery { matchEcho(data: string): PtyStartupReplyEchoMatch { let best: PtyStartupReplyEchoMatch = { kind: 'none' } let bestIndex = -1 + const remainingLineEditorEchoes = this.expectedEchoes.filter((expected) => + expected.projections.some((projection) => projection.source === 'line-editor') + ).length for (const [index, expected] of this.expectedEchoes.entries()) { - const match = locateEcho(expected.projections, data) + // The final echo can consume zsh's trailing BEL; earlier echoes leave it for the next reply. + const match = locateEcho( + expected.projections, + data, + remainingLineEditorEchoes === 1 && + expected.projections.some((projection) => projection.source === 'line-editor') + ) if (isBetterEchoMatch(match, best)) { best = match bestIndex = index } } if (best.kind === 'complete') { - this.expectedEchoes.splice(bestIndex, 1) + const expected = this.expectedEchoes[bestIndex] + if (expected) { + expected.projections = expected.projections.filter( + (projection) => projection.source !== best.source + ) + if (expected.projections.length === 0) { + this.expectedEchoes.splice(bestIndex, 1) + } + } return best } return best diff --git a/src/shared/pty-startup-reply-echo-shapes.ts b/src/shared/pty-startup-reply-echo-shapes.ts index b232880d7dc..5eec1bbca64 100644 --- a/src/shared/pty-startup-reply-echo-shapes.ts +++ b/src/shared/pty-startup-reply-echo-shapes.ts @@ -14,10 +14,14 @@ import type { PtyOwnerBackend } from './pty-owner-backend' * the query parser, and an expired hold releases it raw — so a query torn at its own ESC * would never be answered. Such a shape is matched only when it arrives complete. */ -export type EchoProjection = { needle: string; holdPartial: boolean } +export type EchoProjection = { + needle: string + holdPartial: boolean + source: 'pty' | 'line-editor' +} export type PtyStartupReplyEchoMatch = - | { kind: 'complete'; offset: number; length: number } + | { kind: 'complete'; offset: number; length: number; source: EchoProjection['source'] } | { kind: 'partial'; offset: number } | { kind: 'none' } @@ -27,24 +31,45 @@ export function replyEchoProjections( ): readonly EchoProjection[] { if (ownerBackend === 'windows-conpty') { // Why: ConPTY's projection is the documented, deterministic ESC-stripped form. - return [{ needle: reply.replaceAll('\x1b', ''), holdPartial: true }] + return [{ needle: reply.replaceAll('\x1b', ''), holdPartial: true, source: 'pty' }] } if (ownerBackend !== 'posix-pty') { // wsl.exe is ConPTY-hosted but its echo shape is unverified; suppress nothing. return [] } + const oscPayload = + reply.startsWith('\x1b]') && reply.endsWith('\x1b\\') ? reply.slice(2, -2) : null return [ // The kernel's ECHOCTL caret form — the POSIX default. - { needle: reply.replaceAll('\x1b', '^['), holdPartial: true }, + { needle: reply.replaceAll('\x1b', '^['), holdPartial: true, source: 'pty' }, + ...(oscPayload + ? [ + // zsh ZLE rings the bell around OSC input; its first insertion also redraws the leading byte. + { + needle: `\x07${oscPayload[0]}\b${oscPayload}\x07`, + holdPartial: true, + source: 'line-editor' as const + } + ] + : []), // Readline rewrites OSC, and echoes it even while the kernel reports ECHO clear. ...(reply.includes('\x1b]') - ? [{ needle: reply.replaceAll('\x1b]', '\x07').replaceAll('\x1b\\', ''), holdPartial: true }] + ? [ + { + needle: reply.replaceAll('\x1b]', '\x07').replaceAll('\x1b\\', ''), + holdPartial: true, + source: 'line-editor' as const + } + ] + : []), + ...(oscPayload + ? [{ needle: `\x07${oscPayload}\x07`, holdPartial: true, source: 'line-editor' as const }] : []), // A `stty -echoctl` tty echoes the reply verbatim. Complete-match-only, because it // starts with ESC — see `holdPartial`. We just wrote these exact bytes, and we are // the terminal, so a child emitting the identical span in the same window is not a // case worth trading a stolen query for. - { needle: reply, holdPartial: false } + { needle: reply, holdPartial: false, source: 'pty' } ] } @@ -66,15 +91,26 @@ function suffixPrefixOffset(needle: string, data: string): number { // program wrote around it, so anchoring at offset 0 recognizes almost no real echo. export function locateEcho( projections: readonly EchoProjection[], - data: string + data: string, + preferLongestAtSameOffset = false ): PtyStartupReplyEchoMatch { - let complete: { offset: number; length: number } | null = null + let complete: { + offset: number + length: number + source: EchoProjection['source'] + } | null = null let partialOffset = -1 for (const projection of projections) { const at = data.indexOf(projection.needle) if (at !== -1) { - if (!complete || at < complete.offset) { - complete = { offset: at, length: projection.needle.length } + if ( + !complete || + at < complete.offset || + (preferLongestAtSameOffset && + at === complete.offset && + projection.needle.length > complete.length) + ) { + complete = { offset: at, length: projection.needle.length, source: projection.source } } continue }