diff --git a/src/main/daemon/hibernation-cold-restore-repro.test.ts b/src/main/daemon/hibernation-cold-restore-repro.test.ts index 561c7de26..c8e8ea232 100644 --- a/src/main/daemon/hibernation-cold-restore-repro.test.ts +++ b/src/main/daemon/hibernation-cold-restore-repro.test.ts @@ -5,6 +5,7 @@ import { mkdtempSync, rmSync } from 'node:fs' import { HistoryManager } from './history-manager' import { HistoryReader } from './history-reader' import { HeadlessEmulator } from './headless-emulator' +import { POST_REPLAY_MODE_RESET } from '../../shared/terminal-mode-reset-profiles' // Reproduces the "blank pane after agent hibernation" bug: alt-screen TUI snapshots have scrollbackAnsi='', so the adapter's // `if (scrollback)` gate (daemon-pty-adapter.ts:230) dropped the cold-restore payload and repainted blank despite an intact snapshotAnsi. @@ -95,9 +96,6 @@ describe('agent hibernation cold-restore (alt-screen TUI)', () => { expect(adapterScrollback).not.toBeNull() // Must end in the normal buffer (no alt-screen re-entry) so it won't fight the agent's own repaint when resume relaunches it. - // POST_REPLAY_MODE_RESET copied literally from renderer layout-serialization.ts (main-process test can't import renderer) — keep in sync. - const POST_REPLAY_MODE_RESET = - '\x1b[0 q\x1b[<99u\x1b[=0u\x1b[?25h\x1b[?9l\x1b[?1000l\x1b[?1002l\x1b[?1003l\x1b[?1006l\x1b[?1016l\x1b[?1004l\x1b[?2004l' const fresh = new HeadlessEmulator({ cols: 80, rows: 24 }) fresh.writeSync('\x1b[2J\x1b[3J\x1b[H') fresh.writeSync(adapterScrollback as string) diff --git a/src/main/daemon/pty-subprocess.ts b/src/main/daemon/pty-subprocess.ts index 851eef218..ca8527fb1 100644 --- a/src/main/daemon/pty-subprocess.ts +++ b/src/main/daemon/pty-subprocess.ts @@ -71,6 +71,7 @@ import { expandWindowsPathEnvironmentVariables } from '../../shared/windows-environment-expansion' import { forceKillPosixPtyProcessGroups } from '../pty/posix-pty-process-groups' +import { readPtySlavePath } from '../../shared/pty-slave-line-discipline-echo' const PANE_IDENTITY_ENV_KEYS = [ 'ORCA_PANE_KEY', @@ -1039,9 +1040,11 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl } }) + const slavePath = readPtySlavePath(proc) return { pid: proc.pid, shellPath, + ...(slavePath ? { slavePath } : {}), ...(startupCommandDeliveredInShellArgs ? { startupCommandDeliveredInShellArgs: true } : {}), getForegroundProcess: () => { // Why: node-pty's `.process` reports the live foreground name but reads a recycled pid on a reaped pty, so bail when dead. diff --git a/src/main/daemon/repro-12101-mouse-tracking-survives-agent-death.test.ts b/src/main/daemon/repro-12101-mouse-tracking-survives-agent-death.test.ts new file mode 100644 index 000000000..0e99e0029 --- /dev/null +++ b/src/main/daemon/repro-12101-mouse-tracking-survives-agent-death.test.ts @@ -0,0 +1,274 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { mkdtempSync, rmSync } from 'node:fs' +import { Terminal } from '@xterm/headless' +import { Session } from './session' +import { HistoryManager } from './history-manager' +import { HistoryReader } from './history-reader' +import { getRecoveredHistorySeedSegments } from './terminal-history-seed-segments' +import { iterateTerminalHistorySeedChunks } from './terminal-history-seed-chunks' + +// Repro for #12101: a TUI that armed DECSET mouse tracking (Claude Code, vim, +// htop) is force-killed and never emits the matching DECRST. Nothing in the +// daemon substitutes the reset, so `TerminalModes.mouseTracking` is latched into +// the on-disk checkpoint and then re-derived — from the DEAD process's own bytes — +// into every subsequent fresh emulator. The revived pane runs a bare shell that +// never armed mouse reporting, yet its snapshot re-arms it, so pointer motion +// echoes literal SGR reports (^[[<35;col;rowM) into the prompt. +// +// This drives the REAL Session / HeadlessEmulator / TerminalMouseModeMirror / +// buildRehydrateSequences / HistoryManager / HistoryReader, with a fake +// subprocess standing in for node-pty (a real PTY can't be SIGKILLed +// deterministically mid-DECSET in vitest). The kill goes through Session.kill()'s +// real teardown; only killWithDescendantSweep is stubbed so the test never +// signals a real pid. + +const killWithDescendantSweepMock = vi.hoisted(() => vi.fn()) +vi.mock('../pty-descendant-termination', () => ({ + killWithDescendantSweep: killWithDescendantSweepMock +})) + +// The variant `reattachReplayResetSequence` picks when the pane still looks like a +// live agent, which is exactly the stale state a SIGKILLed agent leaves behind: it +// deliberately OMITS RESET_MOUSE_REPORTING to keep real TUI scroll gestures alive. +import { POST_REPLAY_LIVE_AGENT_REATTACH_RESET } from '../../shared/terminal-mode-reset-profiles' + +const ANY_MOTION_TRACKING_ON = '\x1b[?1003h' +const SGR_ENCODING_ON = '\x1b[?1006h' +const ANY_MOTION_TRACKING_OFF = '\x1b[?1003l' +const SGR_ENCODING_OFF = '\x1b[?1006l' + +function createFakeSubprocess(foregroundProcess: string) { + let onData: ((data: string) => void) | null = null + let onExit: ((code: number) => void) | null = null + const written: string[] = [] + const signals: string[] = [] + return { + written, + signals, + forceKilled: false, + pid: 4242, + getForegroundProcess: () => foregroundProcess, + write: (data: string) => void written.push(data), + resize: () => {}, + kill: () => {}, + forceKill(this: { forceKilled: boolean }) { + this.forceKilled = true + }, + signal: (sig: string) => void signals.push(sig), + onData: (cb: (data: string) => void) => void (onData = cb), + onExit: (cb: (code: number) => void) => void (onExit = cb), + dispose: () => {}, + /** PTY output — same channel node-pty uses, so Session's real ingest runs. */ + emit: (data: string) => onData?.(data), + /** SIGKILL reaped: exit fires with no DECRST ever sent. */ + simulateKilledExit: () => onExit?.(-1) + } +} + +/** Session.emitSubprocessOutput awaits xterm's async parse; poll until the + * emulator has committed the bytes so snapshots are taken on a settled stream. */ +async function waitForEmulatorParse(session: Session): Promise { + for (let i = 0; i < 200; i += 1) { + await new Promise((resolve) => setTimeout(resolve, 1)) + if (session.getSnapshot()?.snapshotAnsi.includes('$ ')) { + return + } + } + throw new Error('setup: emulator never parsed the agent output') +} + +describe('#12101 mouse tracking survives the death of the process that armed it', () => { + let dir: string + const sessionId = 'repo-1::/Users/dev/feature-branch' + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'repro-12101-')) + killWithDescendantSweepMock.mockReset() + }) + + afterEach(() => { + rmSync(dir, { recursive: true, force: true }) + vi.useRealTimers() + }) + + it('re-arms mouse reporting in the fresh shell that replaces a force-killed agent', async () => { + const manager = new HistoryManager(dir) + const reader = new HistoryReader(dir) + + // 1. A live agent TUI arms any-motion tracking + SGR encoding. + const agentPty = createFakeSubprocess('claude') + const agent = new Session({ + sessionId, + cols: 80, + rows: 24, + subprocess: agentPty, + launchAgent: 'claude', + shellReadySupported: false + }) + agentPty.emit(`${ANY_MOTION_TRACKING_ON}${SGR_ENCODING_ON}`) + agentPty.emit('user@host ~ % claude\r\nclaude> analyzing...\r\nuser@host ~ $ ') + await waitForEmulatorParse(agent) + + const armed = agent.getSnapshot() + expect(armed?.modes.mouseTracking).toBe(true) + expect(armed?.modes.mouseTrackingMode).toBe('any') + expect(armed?.rehydrateSequences).toContain(ANY_MOTION_TRACKING_ON) + + // 2. Sleep/hibernation takes its final teardown checkpoint while the agent + // is still alive with mouse armed (daemon-pty-adapter's {final,teardown}). + await manager.openSession(sessionId, { cwd: '/Users/dev/feature-branch', cols: 80, rows: 24 }) + await manager.checkpoint(sessionId, armed!) + + // 3. Force-kill. Real Session teardown; the child is reaped without ever + // emitting DECRST, and nothing writes one on its behalf. + agent.kill() + agentPty.simulateKilledExit() + agent.dispose() // TerminalHost.reapSession + expect(killWithDescendantSweepMock).toHaveBeenCalled() + const teardownBytes = agentPty.written.join('') + expect(teardownBytes).not.toContain(ANY_MOTION_TRACKING_OFF) + expect(teardownBytes).not.toContain(SGR_ENCODING_OFF) + + // 4. Wake: the session is still cold-restore eligible (killed with + // keepHistory, so endedAt stays null) and a FRESH shell takes its place, + // seeded with the recovered history exactly as daemon-server does. + const restoreInfo = await reader.detectColdRestore(sessionId) + expect(restoreInfo).not.toBeNull() + const seedChunks = [ + ...iterateTerminalHistorySeedChunks(getRecoveredHistorySeedSegments(restoreInfo!)) + ] + + const shellPty = createFakeSubprocess('zsh') + const shell = new Session({ + sessionId, + cols: 80, + rows: 24, + subprocess: shellPty, + shellReadySupported: false, + historySeedChunks: seedChunks + }) + const revived = shell.getSnapshot() + try { + // #12101: the replacement shell's OWN state says mouse tracking is on, + // so every snapshot it serves — reattach, checkpoint, mobile — re-arms it. + // Soft so one run reports both re-arming channels, not just the first. + expect.soft(revived?.modes.mouseTracking).toBe(false) + expect.soft(revived?.modes.mouseTrackingMode).toBe('none') + expect.soft(revived?.rehydrateSequences).toBe('') + // Independent second channel: SerializeAddon's own mode trailer re-emits + // the DECSET from xterm's mouseTrackingMode, so dropping rehydrate alone + // would not disarm the pane. + expect.soft(revived?.snapshotAnsi).not.toContain(ANY_MOTION_TRACKING_ON) + } finally { + shell.dispose() + } + }) + + it('leaves the reattached renderer xterm armed against a dead agent (SGR reports into the prompt)', async () => { + const manager = new HistoryManager(dir) + const reader = new HistoryReader(dir) + + const agentPty = createFakeSubprocess('claude') + const agent = new Session({ + sessionId, + cols: 80, + rows: 24, + subprocess: agentPty, + launchAgent: 'claude', + shellReadySupported: false + }) + agentPty.emit(`${ANY_MOTION_TRACKING_ON}${SGR_ENCODING_ON}`) + agentPty.emit('user@host ~ $ ') + await waitForEmulatorParse(agent) + + await manager.openSession(sessionId, { cwd: '/Users/dev/feature-branch', cols: 80, rows: 24 }) + await manager.checkpoint(sessionId, agent.getSnapshot()!) + agent.kill() + agentPty.simulateKilledExit() + agent.dispose() + + const restoreInfo = await reader.detectColdRestore(sessionId) + const shellPty = createFakeSubprocess('zsh') + const shell = new Session({ + sessionId, + cols: 80, + rows: 24, + subprocess: shellPty, + shellReadySupported: false, + historySeedChunks: [ + ...iterateTerminalHistorySeedChunks(getRecoveredHistorySeedSegments(restoreInfo!)) + ] + }) + const revived = shell.getSnapshot()! + + // Reattach paint into a REAL renderer xterm, using the weakest profile in the + // family — the one a stale agent title used to select here. The renderer now + // forces the fresh-shell reset on a cold restore, so this pins the independent + // half: the seed alone must leave the revived session unarmed, because the + // daemon emulator's own state is what mobile and every other consumer read. + const term = new Terminal({ cols: 80, rows: 24, allowProposedApi: true }) + try { + await new Promise((resolve) => term.write('\x1b[2J\x1b[3J\x1b[H', resolve)) + await new Promise((resolve) => + term.write(revived.rehydrateSequences + revived.snapshotAnsi, resolve) + ) + await new Promise((resolve) => + term.write(POST_REPLAY_LIVE_AGENT_REATTACH_RESET, resolve) + ) + // The observable symptom: a bare `zsh` pane whose xterm reports pointer + // motion, so every mouse move types ^[[<35;col;rowM at the prompt. + expect(term.modes.mouseTrackingMode).toBe('none') + } finally { + term.dispose() + shell.dispose() + } + }) + + it('control: a TUI that DID emit DECRST before exiting restores a clean shell', async () => { + const manager = new HistoryManager(dir) + const reader = new HistoryReader(dir) + + const agentPty = createFakeSubprocess('vim') + const agent = new Session({ + sessionId, + cols: 80, + rows: 24, + subprocess: agentPty, + shellReadySupported: false + }) + agentPty.emit(`${ANY_MOTION_TRACKING_ON}${SGR_ENCODING_ON}`) + agentPty.emit('user@host ~ $ ') + await waitForEmulatorParse(agent) + agentPty.emit(`${ANY_MOTION_TRACKING_OFF}${SGR_ENCODING_OFF}`) + for (let i = 0; i < 50 && agent.getSnapshot()?.modes.mouseTracking !== false; i += 1) { + await new Promise((resolve) => setTimeout(resolve, 1)) + } + + await manager.openSession(sessionId, { cwd: '/Users/dev/feature-branch', cols: 80, rows: 24 }) + await manager.checkpoint(sessionId, agent.getSnapshot()!) + agent.kill() + agentPty.simulateKilledExit() + agent.dispose() + + const restoreInfo = await reader.detectColdRestore(sessionId) + const shellPty = createFakeSubprocess('zsh') + const shell = new Session({ + sessionId, + cols: 80, + rows: 24, + subprocess: shellPty, + shellReadySupported: false, + historySeedChunks: [ + ...iterateTerminalHistorySeedChunks(getRecoveredHistorySeedSegments(restoreInfo!)) + ] + }) + try { + expect(shell.getSnapshot()?.modes.mouseTracking).toBe(false) + expect(shell.getSnapshot()?.rehydrateSequences).toBe('') + } finally { + shell.dispose() + } + }) +}) diff --git a/src/main/daemon/repro-7329-remote-snapshot-corruption.test.ts b/src/main/daemon/repro-7329-remote-snapshot-corruption.test.ts index 8f8256519..d42fa33ed 100644 --- a/src/main/daemon/repro-7329-remote-snapshot-corruption.test.ts +++ b/src/main/daemon/repro-7329-remote-snapshot-corruption.test.ts @@ -1,6 +1,8 @@ import { describe, expect, it } from 'vitest' import { Terminal } from '@xterm/headless' import { HeadlessEmulator } from './headless-emulator' +// The reattach path the remote onSnapshot uses. +import { POST_REPLAY_REATTACH_RESET } from '../../shared/terminal-mode-reset-profiles' // Repro for #7329: "remote server + terminal" — typing gets escape sequences // injected/wrapped around it and follow-up commands are corrupted. @@ -11,12 +13,6 @@ import { HeadlessEmulator } from './headless-emulator' // This test drives the REAL daemon serializer and a REAL renderer-side xterm to // see what the user's terminal ends up looking like after a subscribe/reattach. -const RESET_TERMINAL_CURSOR_STYLE = '\x1b[0 q' -const RESET_KITTY_KEYBOARD_PROTOCOL = '\x1b[<99u\x1b[=0u' -const RESET_MOUSE_REPORTING = '\x1b[?9l\x1b[?1000l\x1b[?1002l\x1b[?1003l\x1b[?1006l\x1b[?1016l' -// Verbatim from layout-serialization.ts (the reattach path the remote onSnapshot uses). -const POST_REPLAY_REATTACH_RESET = `${RESET_TERMINAL_CURSOR_STYLE}${RESET_KITTY_KEYBOARD_PROTOCOL}\x1b[?25h${RESET_MOUSE_REPORTING}\x1b[?1004l` - function writeXterm(term: Terminal, data: string): Promise { return new Promise((resolve) => term.write(data, resolve)) } diff --git a/src/main/daemon/session.ts b/src/main/daemon/session.ts index 7d96484ed..d9c5553d3 100644 --- a/src/main/daemon/session.ts +++ b/src/main/daemon/session.ts @@ -26,6 +26,7 @@ import type { TerminalSnapshot } from './types' import type { PtyOwnerBackend } from '../../shared/pty-owner-backend' +import { createPtySlaveEchoProbe } from '../../shared/pty-slave-line-discipline-echo' const SHELL_READY_TIMEOUT_MS = 15_000 // Why: Codex skips marker-gated command delivery; this only bounds older daemon/local paths that still report shell-ready for Codex. @@ -53,6 +54,9 @@ export type SubprocessHandle = { /** Shell the subprocess actually spawned, after fallbacks. The host reconciles the caller's shell-ready * assumption against it so a fallback shell without a ready marker never gates startup commands. */ shellPath?: string + /** Slave device path, so startup replies can read the line discipline's ECHO bit before + * writing. Absent on handles with no POSIX slave to read (ConPTY, tests). */ + slavePath?: string write(data: string): void resize(cols: number, rows: number): void /** Stop reading the PTY fd (node-pty pause()) so a flooding child blocks on write. Optional: @@ -164,11 +168,13 @@ export class Session { } this.postReadyFlushGate = new PostReadyFlushGate(() => this.flushPreReadyQueue()) + const echoProbe = createPtySlaveEchoProbe(this.subprocess.slavePath) this.startupIngress = new PtyStartupIngress({ ...(opts.startupIngress ? { intent: opts.startupIngress } : {}), ...(opts.ownerBackend ? { ownerBackend: opts.ownerBackend } : {}), write: (data) => this.subprocess.write(data), - onEmission: (emission) => this.emitSubprocessOutput(emission) + onEmission: (emission) => this.emitSubprocessOutput(emission), + ...(echoProbe ? { echoProbe } : {}) }) this.subprocess.onData((data) => this.handleSubprocessData(data)) this.subprocess.onExit((code) => this.handleSubprocessExit(code)) diff --git a/src/main/daemon/terminal-history-seed-segments.test.ts b/src/main/daemon/terminal-history-seed-segments.test.ts new file mode 100644 index 000000000..7852a7eac --- /dev/null +++ b/src/main/daemon/terminal-history-seed-segments.test.ts @@ -0,0 +1,119 @@ +import { describe, expect, it } from 'vitest' +import { HeadlessEmulator } from './headless-emulator' +import { buildRehydrateSequences } from './terminal-mode-rehydrate-sequences' +import { getRecoveredHistorySeedSegments } from './terminal-history-seed-segments' +import { COLD_RESTORE_SEED_MODE_RESET } from '../../shared/terminal-mode-reset-profiles' +import type { ColdRestoreInfo } from './terminal-history-cold-restore-info' +import type { TerminalModes } from './types' + +const ARMED_MODES: TerminalModes = { + bracketedPaste: false, + mouseTracking: true, + mouseTrackingMode: 'any', + sgrMouseMode: true, + applicationCursor: false, + alternateScreen: false +} + +// Why import rather than restate: the exact bytes are pinned in +// terminal-mode-reset-profiles.test.ts; this suite pins placement within the seed. +const MOUSE_OFF = COLD_RESTORE_SEED_MODE_RESET + +function restoreInfo(overrides: Partial = {}): ColdRestoreInfo { + return { + snapshotAnsi: 'user@host ~ $ \x1b[?1003h', + scrollbackAnsi: 'user@host ~ $ ', + rehydrateSequences: buildRehydrateSequences(ARMED_MODES), + cwd: '/w', + cols: 80, + rows: 24, + modes: ARMED_MODES, + ...overrides + } +} + +describe('getRecoveredHistorySeedSegments', () => { + it('disarms mouse reporting after the snapshot but before the torn escape tail', () => { + const segments = getRecoveredHistorySeedSegments( + restoreInfo({ pendingEscapeTailAnsi: '\x1b[3' }) + ) + expect(segments).toEqual([ + '\x1b[?1003h\x1b[?1006h', + 'user@host ~ $ \x1b[?1003h', + MOUSE_OFF, + '\x1b[3' + ]) + }) + + it('disarms mouse reporting on the alt-screen normal-buffer branch too', () => { + expect( + getRecoveredHistorySeedSegments( + restoreInfo({ modes: { ...ARMED_MODES, alternateScreen: true } }) + ) + ).toEqual(['user@host ~ $ ', MOUSE_OFF]) + }) + + it('stays empty when there is no recovered normal buffer', () => { + expect( + getRecoveredHistorySeedSegments( + restoreInfo({ + modes: { ...ARMED_MODES, alternateScreen: true }, + scrollbackAnsi: '', + snapshotAnsi: '' + }) + ) + ).toEqual([]) + }) + + it('keeps the empty "nothing to recover" sentinel on the normal-screen branch', () => { + // Why: daemon-pty-adapter keys the probe-race kill+respawn and the history + // re-anchor on `length === 0`, so the reset must never be the only segment. + expect( + getRecoveredHistorySeedSegments( + restoreInfo({ + modes: { ...ARMED_MODES, mouseTracking: false, mouseTrackingMode: 'none' }, + scrollbackAnsi: '', + snapshotAnsi: '', + rehydrateSequences: '' + }) + ) + ).toEqual([]) + }) + + it('keeps a torn escape last when it is the only recovered data', () => { + expect( + getRecoveredHistorySeedSegments( + restoreInfo({ + scrollbackAnsi: '', + snapshotAnsi: '', + rehydrateSequences: '', + pendingEscapeTailAnsi: '\x1b[3' + }) + ) + ).toEqual([MOUSE_OFF, '\x1b[3']) + }) + + it('leaves the revived emulator unarmed while preserving scrollback (#12101)', () => { + const emulator = new HeadlessEmulator({ cols: 80, rows: 24 }) + try { + for (const segment of getRecoveredHistorySeedSegments(restoreInfo())) { + expect(emulator.writeSync(segment)).toBe(true) + } + const snapshot = emulator.getSnapshot() + expect(snapshot.modes.mouseTracking).toBe(false) + expect(snapshot.modes.mouseTrackingMode).toBe('none') + expect(snapshot.modes.sgrMouseMode).toBe(false) + expect(snapshot.rehydrateSequences).toBe('') + expect(snapshot.snapshotAnsi).not.toContain('\x1b[?1003h') + expect(snapshot.snapshotAnsi).toContain('user@host ~ $') + } finally { + emulator.dispose() + } + }) + + it('does not touch the live-session reattach payload (mobile scroll gestures)', () => { + // Why: only recovery seeding knows the arming TUI is dead; live reattach + // snapshots must keep re-arming or an alt-screen TUI loses scroll forever. + expect(buildRehydrateSequences(ARMED_MODES)).toBe('\x1b[?1003h\x1b[?1006h') + }) +}) diff --git a/src/main/daemon/terminal-history-seed-segments.ts b/src/main/daemon/terminal-history-seed-segments.ts index 78732e819..670cef152 100644 --- a/src/main/daemon/terminal-history-seed-segments.ts +++ b/src/main/daemon/terminal-history-seed-segments.ts @@ -1,13 +1,29 @@ import type { ColdRestoreInfo } from './terminal-history-cold-restore-info' +import { COLD_RESTORE_SEED_MODE_RESET } from '../../shared/terminal-mode-reset-profiles' + +// Why the reset belongs in the seed and not only at replay: the recovered stream +// re-arms mouse reporting from two independent sources (rehydrateSequences AND +// SerializeAddon's own mode trailer inside snapshotAnsi), and the seed is what +// feeds the daemon's emulator — so without it every downstream consumer that +// re-serializes from that emulator, including mobile, inherits the dead TUI's +// modes no matter which profile the desktop renderer applies. See +// COLD_RESTORE_SEED_MODE_RESET for the precondition and the choice of bits. export function getRecoveredHistorySeedSegments(restoreInfo: ColdRestoreInfo): readonly string[] { if (restoreInfo.modes.alternateScreen) { const normalBuffer = restoreInfo.scrollbackAnsi || restoreInfo.snapshotAnsi - return normalBuffer ? [normalBuffer] : [] + return normalBuffer ? [normalBuffer, COLD_RESTORE_SEED_MODE_RESET] : [] } - return [ - restoreInfo.rehydrateSequences, - restoreInfo.snapshotAnsi, - ...(restoreInfo.pendingEscapeTailAnsi ? [restoreInfo.pendingEscapeTailAnsi] : []) - ].filter((segment) => segment.length > 0) + const recovered = [restoreInfo.rehydrateSequences, restoreInfo.snapshotAnsi].filter( + (segment) => segment.length > 0 + ) + const escapeTail = restoreInfo.pendingEscapeTailAnsi + // Why: an empty list is daemon-pty-adapter's "nothing to recover" sentinel (it gates + // the probe-race respawn and the history re-anchor), so the reset must never pad it. + if (recovered.length === 0 && !escapeTail) { + return [] + } + // Why after the snapshot: it must undo the snapshot's own mode trailer, and + // pendingEscapeTailAnsi is a torn escape that has to stay at the very end. + return [...recovered, COLD_RESTORE_SEED_MODE_RESET, ...(escapeTail ? [escapeTail] : [])] } diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index 3f60ec667..257250a98 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -12179,9 +12179,12 @@ describe('registerPtyHandlers', () => { const sourceData = '\x1b]10;?\x1b\\\x1b]11;?\x1b\\ready' mockProc.emitData(sourceData) + // Why: the reply leaves the query's own turn so a still-cooked tty cannot + // echo it back as text instead of delivering it to the agent (#12112). + expect(mockProc.proc.write).not.toHaveBeenCalled() + vi.advanceTimersByTime(2) expect(mockProc.proc.write).toHaveBeenCalledWith('\x1b]10;rgb:eeee/eeee/eeee\x1b\\') expect(mockProc.proc.write).toHaveBeenCalledWith('\x1b]11;rgb:1111/1111/1111\x1b\\') - vi.advanceTimersByTime(2) expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:data', { id: spawnResult.id, data: 'ready', @@ -12216,9 +12219,11 @@ describe('registerPtyHandlers', () => { const sourceData = '\x1b]10;?;?\x1b\\ready' mockProc.emitData(sourceData) + // Why: both slots of a duplicate-slot query leave the query's own turn too (#12112). + expect(mockProc.proc.write).not.toHaveBeenCalled() + vi.advanceTimersByTime(2) expect(mockProc.proc.write).toHaveBeenCalledWith('\x1b]10;rgb:eeee/eeee/eeee\x1b\\') expect(mockProc.proc.write).toHaveBeenCalledWith('\x1b]11;rgb:1111/1111/1111\x1b\\') - vi.advanceTimersByTime(2) expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:data', { id: spawnResult.id, data: 'ready', diff --git a/src/main/providers/local-pty-provider.ts b/src/main/providers/local-pty-provider.ts index 5e8385bed..d43158482 100644 --- a/src/main/providers/local-pty-provider.ts +++ b/src/main/providers/local-pty-provider.ts @@ -66,6 +66,10 @@ import { PhysicalExitTracker } from '../../shared/physical-exit-tracker' import { mergeGitConfigEnvProtocol } from '../../shared/git-credential-prompt-env' import { PtyStartupIngress, type PtyIngressEmission } from '../../shared/pty-startup-ingress' import { resolvePtyOwnerBackend } from '../../shared/pty-owner-backend' +import { + createPtySlaveEchoProbe, + readPtySlavePath +} from '../../shared/pty-slave-line-discipline-echo' import { expandWindowsEnvironmentVariables, expandWindowsPathEnvironmentVariables @@ -922,6 +926,7 @@ export class LocalPtyProvider implements IPtyProvider { ) } } + const startupEchoProbe = createPtySlaveEchoProbe(readPtySlavePath(proc)) const startupIngress = new PtyStartupIngress({ ...(args.startupIngress ? { intent: args.startupIngress } : {}), ownerBackend: resolvePtyOwnerBackend({ @@ -930,7 +935,8 @@ export class LocalPtyProvider implements IPtyProvider { wslDistro: spawnedWslDistro }), write: (data) => proc.write(data), - onEmission: emitIngressData + onEmission: emitIngressData, + ...(startupEchoProbe ? { echoProbe: startupEchoProbe } : {}) }) startupIngressByPty.set(id, startupIngress) diff --git a/src/relay/pty-handler.ts b/src/relay/pty-handler.ts index 13267b584..6e8116158 100644 --- a/src/relay/pty-handler.ts +++ b/src/relay/pty-handler.ts @@ -69,6 +69,7 @@ import { isAgentSessionSurfaceBinding, type AgentSessionOwnerBinding } from '../shared/agent-session-host-authority' +import { createPtySlaveEchoProbe, readPtySlavePath } from '../shared/pty-slave-line-discipline-echo' // Why: only Linux compiles node-pty (no prebuilt), so the build-tools remedy is a closable setup gap // there and wrong advice anywhere node-pty ships one. The relay only sees an unloadable binding, never @@ -689,11 +690,13 @@ export class PtyHandler { : {} ) } + const echoProbe = createPtySlaveEchoProbe(readPtySlavePath(managed.pty)) managed.startupIngress ??= new PtyStartupIngress({ ...(managed.startupIngressIntent ? { intent: managed.startupIngressIntent } : {}), ownerBackend: managed.ownerBackend, write: (data) => managed.pty.write(data), - onEmission: emitIngressData + onEmission: emitIngressData, + ...(echoProbe ? { echoProbe } : {}) }) managed.pty.onData((data: string) => { const startup = managed.startupCommand diff --git a/src/renderer/src/components/terminal-pane/issue-12112-agent-pane-startup-color-reply-leak.repro.test.ts b/src/renderer/src/components/terminal-pane/issue-12112-agent-pane-startup-color-reply-leak.repro.test.ts new file mode 100644 index 000000000..d93f29ba7 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/issue-12112-agent-pane-startup-color-reply-leak.repro.test.ts @@ -0,0 +1,284 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { Terminal } from '@xterm/headless' +import { PtyStartupIngress } from '../../../../shared/pty-startup-ingress' +import { isTuiAgent } from '../../../../shared/tui-agent-config' +import { installTerminalCapabilityReplyHandlers } from './terminal-capability-replies' +import { createIpcPtyTransport } from './pty-transport' +import type { PtyTransport } from './pty-transport-types' + +// Regression for #12112: a NEW opencode agent tab on Linux prints the literal +// `10;rgb:ffff/ffff/ffff` / `11;rgb:2828/2c2c/3434` text and opencode never +// initializes, while a plain Ctrl+T tab running the same program is clean. +// +// The divergence is the main-side PtyStartupIngress: only agent panes arm it +// (terminal-startup-color-query-replies.ts), and on `posix-pty` it answers the +// query synchronously inside node-pty's onData — before the querying program +// has finished entering raw mode — with no echo suppression. The suppression +// exists but is gated on `ownerBackend === 'windows-conpty'` +// (pty-startup-ingress.ts:26-29, 247-252), so the cooked-mode echo of Orca's +// own reply is forwarded to the renderer verbatim and rendered as text. + +// opencode/OpenTUI's unconditional startup burst (BEL-terminated, not ST). +const OPENCODE_STARTUP_QUERY_BURST = '\x1b]10;?\x07\x1b]11;?\x07\x1b]4;0;?\x07' +// Orca's One Dark terminal theme — the values that appear in the leaked text. +const ORCA_TERMINAL_THEME = { foreground: '#ffffff', background: '#282c34' } +const OSC10_REPLY = '\x1b]10;rgb:ffff/ffff/ffff\x1b\\' +const OSC11_REPLY = '\x1b]11;rgb:2828/2c2c/3434\x1b\\' +const LEAKED_COLOR_REPLY_TEXT = /\d\d;rgb:[0-9a-f]{4}\// + +const PTY_ID = 'pty-12112' +let dispatchPtyData: (payload: { id: string; data: string }) => void = () => {} + +/** + * bash/readline echo projection: `\e]` is an unbound binding, so readline eats + * ESC + `]` and beeps, then self-inserts the rest; the ST is eaten the same way. + * Verified against a real `bash --norc -i` behind node-pty, and it reproduces + * the reported string exactly once rendered by xterm. + */ +function readlineEchoOf(reply: string): string { + return reply.replaceAll('\x1b]', '\x07').replaceAll('\x1b\\', '') +} + +type StartupTty = { + /** A writer (main ingress or renderer) pushes bytes at the PTY master. */ + writeToPty: (data: string) => void + /** Bytes the querying program actually read in raw mode. */ + programInput: () => string + onPtyOutput: (sink: (data: string) => void) => void + emitStartupBurst: () => void +} + +/** + * Models the single fact that decides this bug: the tty is still line + * disciplined (shell prompt / program mid-`tcsetattr`) during the turn that + * carries the startup burst, and raw immediately after. A writer answering + * synchronously inside that turn is echoed; one answering a turn later reaches + * the program. Both arms of this test share this exact model, so the divergence + * comes from Orca's code, not from the harness. + */ +function createStartupTty(): StartupTty { + let raw = false + let received = '' + let sink: (data: string) => void = () => {} + return { + writeToPty: (data) => { + if (raw) { + received += data + return + } + sink(readlineEchoOf(data)) + }, + programInput: () => received, + onPtyOutput: (next) => { + sink = next + }, + emitStartupBurst: () => { + sink(OPENCODE_STARTUP_QUERY_BURST) + // Why microtask: opencode finishes entering raw mode essentially at once, + // but not inside its writer's synchronous callback. + queueMicrotask(() => { + raw = true + }) + } + } +} + +function stubPtyApi(tty: StartupTty): void { + vi.stubGlobal('window', { + api: { + pty: { + spawn: vi.fn(async () => ({ id: PTY_ID })), + write: vi.fn((_id: string, data: string) => tty.writeToPty(data)), + resize: vi.fn(), + kill: vi.fn(async () => {}), + claimViewport: vi.fn(), + onData: (cb: (payload: { id: string; data: string }) => void) => { + dispatchPtyData = cb + return () => {} + }, + onReplay: () => () => {}, + onExit: () => () => {} + } + } + }) +} + +type RendererPane = { + transport: PtyTransport + deliver: (data: string) => void + renderedText: () => string + dispose: () => void +} + +/** Real xterm + real capability-reply handlers + the real local IPC transport. */ +async function createRendererPane(): Promise { + const terminal = new Terminal({ cols: 80, rows: 24, allowProposedApi: true }) + terminal.options.theme = { ...ORCA_TERMINAL_THEME } + + const transport = createIpcPtyTransport({ worktreeId: 'wt-1', tabId: 'tab-1', leafId: 'pane:1' }) + await transport.connect({ + url: '', + cols: 80, + rows: 24, + callbacks: { onData: (data) => terminal.write(data) } + }) + + const handlers = installTerminalCapabilityReplyHandlers({ + terminal: terminal as never, + parser: terminal.parser, + // Matches pty-connection.ts: replies go out via sendInputImmediate. + sendInput: (data) => transport.sendInputImmediate(data), + isReplaying: () => false + }) + + return { + transport, + deliver: (data) => dispatchPtyData({ id: PTY_ID, data }), + renderedText: () => { + const lines: string[] = [] + for (let row = 0; row < terminal.rows; row += 1) { + lines.push(terminal.buffer.active.getLine(row)?.translateToString(true) ?? '') + } + return lines.join('\n').trim() + }, + dispose: () => { + handlers.dispose() + terminal.dispose() + } + } +} + +async function settleUntil(condition: () => boolean, timeoutMs = 500): Promise { + const deadline = Date.now() + timeoutMs + while (!condition() && Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, 1)) + } + expect(condition()).toBe(true) +} + +describe('#12112 opencode startup OSC 10/11 replies on the local path', () => { + afterEach(() => { + vi.useRealTimers() + vi.unstubAllGlobals() + }) + + it('plain terminal tab consumes the replies and renders nothing', async () => { + const tty = createStartupTty() + stubPtyApi(tty) + const pane = await createRendererPane() + // A Ctrl+T tab has no launch agent, so shouldReplyToStartupTerminalColorQueries + // (terminal-startup-color-query-replies.ts) is false, main arms no startup + // ingress, and the pane's own xterm is the sole responder. + expect(isTuiAgent(undefined)).toBe(false) + tty.onPtyOutput(pane.deliver) + + try { + tty.emitStartupBurst() + await settleUntil(() => tty.programInput().includes(OSC11_REPLY)) + + expect(pane.renderedText()).not.toMatch(LEAKED_COLOR_REPLY_TEXT) + expect(tty.programInput()).toContain(OSC10_REPLY) + expect(tty.programInput()).toContain(OSC11_REPLY) + } finally { + pane.dispose() + } + }) + + it('agent pane consumes the replies and renders nothing', async () => { + const tty = createStartupTty() + stubPtyApi(tty) + const pane = await createRendererPane() + + // opencode is a TUI agent, so main arms spawnOptions.startupIngress for this + // pane and only this pane (pty.ts:4024, terminal-startup-color-query-replies.ts). + expect(isTuiAgent('opencode')).toBe(true) + const ingress = new PtyStartupIngress({ + intent: { colors: ORCA_TERMINAL_THEME, deadlineMs: 5_000 }, + ownerBackend: 'posix-pty', + write: (data) => tty.writeToPty(data), + onEmission: (emission) => pane.deliver(emission.data) + }) + tty.onPtyOutput((data) => ingress.accept(data)) + + try { + tty.emitStartupBurst() + await settleUntil(() => tty.programInput().includes(OSC11_REPLY)) + + expect(pane.renderedText()).not.toMatch(LEAKED_COLOR_REPLY_TEXT) + expect(tty.programInput()).toContain(OSC10_REPLY) + expect(tty.programInput()).toContain(OSC11_REPLY) + } finally { + ingress.drainAndClose() + pane.dispose() + } + }) + + it('renders no reply text when the tty echo is coalesced with program output', () => { + // The reported topology: the agent is launched by writing `opencode\n` into an + // interactive shell, so bash's echo of Orca's reply shares a read with the shell's + // own echo and the agent's first frame. It is never at the head of a chunk, and a + // read carrying no echo at all comes first. + vi.useFakeTimers() + const echoLayouts = [ + (replies: readonly string[]) => + `opencode\r\n\x1b[2Jloading${replies.map(readlineEchoOf).join('')}\r\n$ `, + // Both slots answered, but the agent draws between the two echoes. + (replies: readonly string[]) => + replies.map((reply) => `${readlineEchoOf(reply)}FRAME\r\n`).join('') + ] + + for (const [index, layout] of echoLayouts.entries()) { + const emitted: string[] = [] + const writes: string[] = [] + const ingress = new PtyStartupIngress({ + intent: { colors: ORCA_TERMINAL_THEME, deadlineMs: 5_000 }, + ownerBackend: 'posix-pty', + write: (data) => writes.push(data), + onEmission: (emission) => emitted.push(emission.data) + }) + ingress.accept(OPENCODE_STARTUP_QUERY_BURST) + vi.advanceTimersByTime(0) + expect(writes, `layout ${index}`).toEqual([OSC10_REPLY, OSC11_REPLY]) + + ingress.accept(layout(writes)) + ingress.drainAndClose() + + expect(emitted.join(''), `layout ${index}`).not.toMatch(LEAKED_COLOR_REPLY_TEXT) + } + }) + + it('suppresses its own cooked echo on POSIX and ConPTY', () => { + // No xterm, no transport: the asymmetry alone, with each backend's observed + // cooked echo handed straight back the way its line discipline would. + vi.useFakeTimers() + const forward = (ownerBackend: 'posix-pty' | 'windows-conpty') => { + const echoOf = + ownerBackend === 'windows-conpty' + ? (reply: string): string => reply.replaceAll('\x1b', '') + : readlineEchoOf + const emitted: string[] = [] + const writes: string[] = [] + const ingress = new PtyStartupIngress({ + intent: { colors: ORCA_TERMINAL_THEME, deadlineMs: 5_000 }, + ownerBackend, + write: (data) => { + writes.push(data) + ingress.accept(echoOf(data)) + }, + onEmission: (emission) => emitted.push(emission.data) + }) + ingress.accept(OPENCODE_STARTUP_QUERY_BURST) + // Why: the posix write is deferred, so draining first would make this arm + // assert on a stream where no reply was ever sent. + vi.advanceTimersByTime(0) + ingress.drainAndClose() + return { visible: emitted.join(''), writes } + } + + for (const ownerBackend of ['windows-conpty', 'posix-pty'] as const) { + const { visible, writes } = forward(ownerBackend) + expect(writes, ownerBackend).toEqual([OSC10_REPLY, OSC11_REPLY]) + expect(visible, ownerBackend).not.toMatch(LEAKED_COLOR_REPLY_TEXT) + } + }) +}) diff --git a/src/renderer/src/components/terminal-pane/layout-serialization.test.ts b/src/renderer/src/components/terminal-pane/layout-serialization.test.ts index 797719697..626168ef2 100644 --- a/src/renderer/src/components/terminal-pane/layout-serialization.test.ts +++ b/src/renderer/src/components/terminal-pane/layout-serialization.test.ts @@ -33,13 +33,15 @@ beforeAll(() => { }) import { - buildFontFamily, buildPostReplayLiveAgentReattachReset, POST_REPLAY_LIVE_AGENT_REATTACH_RESET, POST_REPLAY_MODE_RESET, replayPayloadEndsWithCursorHidden, RESET_KITTY_KEYBOARD_PROTOCOL, - RESET_TERMINAL_CURSOR_STYLE, + RESET_TERMINAL_CURSOR_STYLE +} from '../../../../shared/terminal-mode-reset-profiles' +import { + buildFontFamily, restoreScrollbackBuffers, serializePaneTree, serializeTerminalLayout, diff --git a/src/renderer/src/components/terminal-pane/layout-serialization.ts b/src/renderer/src/components/terminal-pane/layout-serialization.ts index 6fb2df027..2f8d0fea2 100644 --- a/src/renderer/src/components/terminal-pane/layout-serialization.ts +++ b/src/renderer/src/components/terminal-pane/layout-serialization.ts @@ -4,6 +4,7 @@ import type { TerminalPaneSplitDirection } from '../../../../shared/types' import { isTerminalLeafId } from '../../../../shared/stable-pane-id' +import { POST_REPLAY_MODE_RESET } from '../../../../shared/terminal-mode-reset-profiles' import type { PaneManager } from '@/lib/pane-manager/pane-manager' import { replayIntoTerminal, type ReplayingPanesRef } from './replay-guard' import type { RestoredViewportBlankingPanesRef } from './terminal-restored-viewport' @@ -27,40 +28,6 @@ export const EMPTY_LAYOUT: TerminalLayoutSnapshot = { expandedLeafId: null } -// Why: SerializeAddon replays mode bits assuming reattach to a live TUI, but Orca restores against a fresh shell with none, so stale bits (e.g. focus reporting rings the bell on click) must be reset. -export const RESET_TERMINAL_CURSOR_STYLE = '\x1b[0 q' -export const RESET_KITTY_KEYBOARD_PROTOCOL = '\x1b[<99u\x1b[=0u' -// Every mouse mode the daemon can re-arm from a snapshot: protocols 9/1000/1002/1003 + SGR encodings 1006/1016. -export const RESET_MOUSE_REPORTING = - '\x1b[?9l\x1b[?1000l\x1b[?1002l\x1b[?1003l\x1b[?1006l\x1b[?1016l' - -export const POST_REPLAY_MODE_RESET = `${RESET_TERMINAL_CURSOR_STYLE}${RESET_KITTY_KEYBOARD_PROTOCOL}\x1b[?25h${RESET_MOUSE_REPORTING}\x1b[?1004l\x1b[?2004l` - -// Why: same-session live replay; keep cursor/focus cleanup but preserve Kitty flags the running TUI relies on. -export const POST_REPLAY_LIVE_SNAPSHOT_RESET = `${RESET_TERMINAL_CURSOR_STYLE}\x1b[?25h\x1b[?1004l` - -// Why: daemon reattach hits a live session, so skip the full reset; still clear cursor/focus/mouse/Kitty bits harmful to a plain shell after a bad TUI exit — safe for live TUIs since the post-reattach SIGWINCH repaints the cursor. -export const POST_REPLAY_REATTACH_RESET = `${RESET_TERMINAL_CURSOR_STYLE}${RESET_KITTY_KEYBOARD_PROTOCOL}\x1b[?25h${RESET_MOUSE_REPORTING}\x1b[?1004l` - -// Why: a live agent owns focus reporting; resetting ?1004h suppresses the focus-in it needs to re-anchor its cursor (IME). -export const POST_REPLAY_LIVE_AGENT_REATTACH_RESET = `${RESET_TERMINAL_CURSOR_STYLE}${RESET_KITTY_KEYBOARD_PROTOCOL}\x1b[?25h` - -// Why: DECTCEM applies in emission order, so the payload's last ?25l/?25h is the cursor state the TUI left. -export function replayPayloadEndsWithCursorHidden(payload: string): boolean { - const hideIndex = payload.lastIndexOf('\x1b[?25l') - return hideIndex !== -1 && hideIndex > payload.lastIndexOf('\x1b[?25h') -} - -// Why: some agents hide the real cursor and draw their own, so preserve the payload's final visibility (pty-connection re-shows it if the agent was actually a dead TUI). -export function buildPostReplayLiveAgentReattachReset(payload: string): string { - return replayPayloadEndsWithCursorHidden(payload) - ? `${RESET_TERMINAL_CURSOR_STYLE}${RESET_KITTY_KEYBOARD_PROTOCOL}` - : POST_REPLAY_LIVE_AGENT_REATTACH_RESET -} - -// Why: a live agent owns cursor/focus here; forcing ?25h/?1004l breaks a parked agent that only arms ?1004h at startup. -export const POST_REPLAY_LIVE_AGENT_SNAPSHOT_RESET = RESET_TERMINAL_CURSOR_STYLE - // Cross-platform monospace chain: browsers skip fonts absent on the current OS, so listing all is safe. // Nerd Fonts come last to cover PUA glyphs (U+E000–U+F8FF) from OMP/Powerline that standard monospace fonts lack. const FALLBACK_FONTS = [ diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index 701dd4858..8bb448ed1 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -10,7 +10,7 @@ import { POST_REPLAY_REATTACH_RESET, RESET_KITTY_KEYBOARD_PROTOCOL, RESET_TERMINAL_CURSOR_STYLE -} from './layout-serialization' +} from '../../../../shared/terminal-mode-reset-profiles' import { buildFreshShellViewportBlankingSequence } from './terminal-restored-viewport' import { DEFAULT_DA1_RESPONSE } from './terminal-capability-replies' import { TERMINAL_PASTE_DIRECT_MAX_BYTES } from './terminal-paste-coordinator' @@ -9030,6 +9030,103 @@ describe('connectPanePty', () => { }) }) + it('ignores the stale agent signal on a cold restore and applies the fresh-shell reset', async () => { + // Why: pane status and title are persisted, so after a cold restore they still + // describe the process that died and make the pane look agent-owned. Preserving + // "its" modes arms mouse/focus/paste reporting against the replacement shell, + // which then prints the reports as junk at the prompt (#12101). + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('tab-pty') + transport.connect.mockImplementation(async ({ sessionId }: { sessionId?: string }) => { + if (sessionId) { + return { + id: sessionId, + snapshot: '\x1b[?1003h\x1b[?1006h\x1b[?2004huser@host ~ $ ', + coldRestore: { scrollback: 'cold-payload', cwd: '/tmp/wt-1' } + } + } + return null + }) + transportFactoryQueue.push(transport) + setReattachPaneTitle('Cursor Agent') + + const pane = createPane(1) + const textarea = {} as HTMLTextAreaElement + configureTerminalFocusMode(pane, textarea) + await withMockedDocumentActiveElement(textarea, async () => { + const manager = createManager(1) + const deps = createDeps({ + restoredLeafId: LEAF_1, + restoredPtyIdByLeafId: { [LEAF_1]: 'tab-pty' } + }) + + connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks(20) + + const writes = (pane.terminal.write as ReturnType).mock.calls.map( + ([data]) => data as string + ) + expect(writes).toContain(POST_REPLAY_MODE_RESET) + expect(writes).not.toContain(POST_REPLAY_LIVE_AGENT_REATTACH_RESET) + }) + }) + + it('applies the fresh-shell reset when a spawn is answered with a cold-restore reattach', async () => { + // Why: main can answer a *spawn* with an adopted session, so the reattach handler + // is reachable by a second door that skips the restored-session path entirely. The + // cold-restore signal has to survive that door too, or #12101 returns on it. + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('tab-pty') + let activePtyId = 'tab-pty' + transport.getPtyId.mockImplementation(() => activePtyId) + transport.connect.mockImplementation(async ({ sessionId }: { sessionId?: string }) => { + if (sessionId) { + throw new Error('restored session is gone') + } + // Main answered the spawn by adopting a durable session instead. + activePtyId = 'adopted-pty' + return { + id: 'adopted-pty', + isReattach: true, + // Keep this snapshot free of ?25l: the live agent reset is built from the + // payload and only equals the constant negated below when the cursor is left + // visible. Ending it hidden would quietly retire that assertion. + snapshot: '\x1b[?1003h\x1b[?1006h\x1b[?2004huser@host ~ $ ', + coldRestore: { scrollback: 'cold-payload', cwd: '/tmp/wt-1' } + } + }) + transportFactoryQueue.push(transport) + setReattachPaneTitle('Cursor Agent') + + const pane = createPane(1) + const textarea = {} as HTMLTextAreaElement + configureTerminalFocusMode(pane, textarea) + await withMockedDocumentActiveElement(textarea, async () => { + const manager = createManager(1) + const deps = createDeps({ + restoredLeafId: LEAF_1, + restoredPtyIdByLeafId: { [LEAF_1]: 'tab-pty' } + }) + + connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks(30) + + expect(transport.connect).toHaveBeenCalledTimes(2) + expect(transport.connect.mock.calls[0]?.[0]?.sessionId).toBe('tab-pty') + expect(transport.connect.mock.calls[1]?.[0]?.sessionId).toBeUndefined() + const writes = (pane.terminal.write as ReturnType).mock.calls.map( + ([data]) => data as string + ) + const output = writes.join('') + const snapshotIndex = output.indexOf('\x1b[?1003h\x1b[?1006h\x1b[?2004huser@host ~ $ ') + const resetIndex = output.indexOf(POST_REPLAY_MODE_RESET) + expect(snapshotIndex).toBeGreaterThanOrEqual(0) + expect(resetIndex).toBeGreaterThan(snapshotIndex) + expect(writes).toContain(POST_REPLAY_MODE_RESET) + expect(writes).not.toContain(POST_REPLAY_LIVE_AGENT_REATTACH_RESET) + }) + }) + it('keeps ?25h in the live agent reattach reset when the snapshot leaves the cursor visible', async () => { const { connectPanePty } = await import('./pty-connection') const transport = createMockTransport('tab-pty') diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index b5172cd89..fdace14f0 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -123,7 +123,7 @@ import { POST_REPLAY_REATTACH_RESET, RESET_KITTY_KEYBOARD_PROTOCOL, RESET_TERMINAL_CURSOR_STYLE -} from './layout-serialization' +} from '../../../../shared/terminal-mode-reset-profiles' import { buildFreshShellViewportBlankingSequence } from './terminal-restored-viewport' import { createShellReadyMarkerScanState, scanForShellReadyMarker } from './shell-ready-marker-scan' import { shouldUseShellReadyStartupDelivery } from '../../../../shared/codex-startup-delivery' @@ -3012,7 +3012,7 @@ export function connectPanePty( // rings the bell. This is specific to terminals with cross-restart // persistence (as we have); our fix is to reset 1004 and friends after // scrollback replay so the mode state matches the fresh shell - // underneath. See POST_REPLAY_MODE_RESET in layout-serialization.ts. + // underneath. See POST_REPLAY_MODE_RESET in shared/terminal-mode-reset-profiles.ts. const onBell = (): void => { // Why: restored Claude Code sessions have been observed to emit a real // standalone BEL some time after daemon snapshot reattach, even when Orca @@ -5492,7 +5492,15 @@ export function connectPanePty( }) } - const reattachReplayResetSequence = (payload: string): string => { + const reattachReplayResetSequence = (payload: string, ownerProcessEnded = false): string => { + // Why a cold restore overrides the agent signal: liveness is read from the + // pane's status and title, both of which are persisted, so after a cold + // restore they describe the process that died. Preserving "its" modes arms + // mouse, focus and paste reporting against the fresh shell that replaces it, + // which then prints the reports as junk at the prompt (#12101). + if (ownerProcessEnded) { + return POST_REPLAY_MODE_RESET + } return shouldPreserveAgentReattachModes() ? buildPostReplayLiveAgentReattachReset(payload) : POST_REPLAY_REATTACH_RESET @@ -7936,8 +7944,10 @@ export function connectPanePty( // Why: re-arm the kitty keyboard mirror from the snapshot preamble so Option chords keep their encoding after a window reload. kittyKeyboardModes.scanReplay(connectResult.snapshot) writeReplayData(connectResult.snapshot) - // Snapshot reattach keeps a live session, so drop only renderer-owned state instead of the broader mode reset. - writeReplayData(reattachReplayResetSequence(connectResult.snapshot)) + // Snapshot reattach keeps a live session, so drop only renderer-owned state instead of the broader mode reset — unless this is a cold restore, whose owner is gone. + writeReplayData( + reattachReplayResetSequence(connectResult.snapshot, Boolean(connectResult.coldRestore)) + ) if (connectResult.pendingEscapeTailAnsi) { // Why last: re-arm the dangling mid-escape after the reset (whose ESC would abort it) so the live continuation completes it (#7329). writeReplayData(connectResult.pendingEscapeTailAnsi) @@ -7990,7 +8000,9 @@ export function connectPanePty( for (const replayChunk of buildMainModelSnapshotReplayWrites(modelSnapshot)) { writeReplayData(replayChunk) } - writeReplayData(reattachReplayResetSequence(modelData)) + writeReplayData( + reattachReplayResetSequence(modelData, Boolean(connectResult?.coldRestore)) + ) if (modelSnapshot.pendingEscapeTailAnsi) { // Why last: re-arm the dangling mid-escape after the reset so the live continuation completes it (#7329). writeReplayData(modelSnapshot.pendingEscapeTailAnsi) @@ -8009,7 +8021,9 @@ export function connectPanePty( // Why: raw relay replay may contain the app's own kitty pushes; re-arm with set semantics so redelivery can't grow the stack. kittyKeyboardModes.scanReplay(connectResult.replay) writeReplayData(connectResult.replay) - writeReplayData(reattachReplayResetSequence(connectResult.replay)) + writeReplayData( + reattachReplayResetSequence(connectResult.replay, Boolean(connectResult.coldRestore)) + ) sendFocusedReattachFocusInAfterReplay(ptyId, attemptGeneration) if (connectResult.coldRestore) { if (!isRemoteRuntimePtyId(ptyId)) { diff --git a/src/renderer/src/components/terminal-pane/terminal-replay-cursor-state.test.ts b/src/renderer/src/components/terminal-pane/terminal-replay-cursor-state.test.ts index 9e9275520..3072b98da 100644 --- a/src/renderer/src/components/terminal-pane/terminal-replay-cursor-state.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-replay-cursor-state.test.ts @@ -7,7 +7,7 @@ import { POST_REPLAY_REATTACH_RESET, RESET_KITTY_KEYBOARD_PROTOCOL, RESET_TERMINAL_CURSOR_STYLE -} from './layout-serialization' +} from '../../../../shared/terminal-mode-reset-profiles' const OLD_REATTACH_RESET_WITHOUT_CURSOR_STYLE = '\x1b[?25h\x1b[?1004l' diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts index 113fd52a5..23a7475f5 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts @@ -65,10 +65,10 @@ import { resolveTerminalFontWeights } from '../../../../shared/terminal-fonts' import { buildFontFamily, normalizeTerminalLayoutSnapshot, - RESET_KITTY_KEYBOARD_PROTOCOL, replayTerminalLayout, restoreScrollbackBuffers } from './layout-serialization' +import { RESET_KITTY_KEYBOARD_PROTOCOL } from '../../../../shared/terminal-mode-reset-profiles' import { resolveTerminalLayoutActiveLeafId } from './terminal-layout-leaf-ids' import { makePaneKey } from '../../../../shared/stable-pane-id' import { applyExpandedLayoutTo, restoreExpandedLayoutFrom } from './expand-collapse' diff --git a/src/shared/pty-slave-line-discipline-echo.test.ts b/src/shared/pty-slave-line-discipline-echo.test.ts new file mode 100644 index 000000000..a18899571 --- /dev/null +++ b/src/shared/pty-slave-line-discipline-echo.test.ts @@ -0,0 +1,119 @@ +import { describe, expect, it, vi, beforeEach } from 'vitest' + +const execFileMock = vi.hoisted(() => vi.fn()) +vi.mock('node:child_process', () => ({ execFile: execFileMock })) + +import { createPtySlaveEchoProbe, readPtySlavePath } from './pty-slave-line-discipline-echo' + +/** Replies to the next stty call with the given output, or an error when `output` is null. */ +function answerStty(output: string | null): void { + execFileMock.mockImplementationOnce((_cmd, _args, _opts, cb) => { + cb(output === null ? new Error('stty: no such file') : null, output ?? '', '') + }) +} + +const COOKED = 'speed 38400 baud;\nlflags: icanon isig iexten echo echoe echok echoctl\n' +const RAW = 'speed 38400 baud;\nlflags: -icanon -isig -iexten -echo -echoe -echok -echoctl\n' + +beforeEach(() => { + execFileMock.mockReset() +}) + +describe('readPtySlavePath', () => { + it('reads node-pty ptsName and rejects every shape that is not a usable path', () => { + expect(readPtySlavePath({ ptsName: '/dev/ttys048' })).toBe('/dev/ttys048') + // A ConPTY terminal has no ptsName at all, and an empty one names no device. + expect(readPtySlavePath({})).toBeUndefined() + expect(readPtySlavePath({ ptsName: '' })).toBeUndefined() + expect(readPtySlavePath({ ptsName: 12 })).toBeUndefined() + expect(readPtySlavePath(undefined)).toBeUndefined() + expect(readPtySlavePath(null)).toBeUndefined() + }) +}) + +describe('createPtySlaveEchoProbe', () => { + it('has no probe to offer when there is no POSIX slave to read', () => { + expect(createPtySlaveEchoProbe('/dev/ttys048', 'win32')).toBeUndefined() + expect(createPtySlaveEchoProbe(undefined, 'darwin')).toBeUndefined() + }) + + it('reads the ECHO bit off the slave', async () => { + const probe = createPtySlaveEchoProbe('/dev/ttys048', 'darwin') + answerStty(COOKED) + await expect(probe?.()).resolves.toBe('echoing') + answerStty(RAW) + await expect(probe?.()).resolves.toBe('quiet') + }) + + it('does not read `echoctl` or `echoe` as the ECHO bit', async () => { + const probe = createPtySlaveEchoProbe('/dev/ttys048', 'darwin') + // Why: a substring match on "echo" reports echoing for a raw tty that merely keeps + // echoctl set, which is the exact tty the write must not be held back for. + answerStty('lflags: -icanon -echo echoe echok echoctl echoke\n') + await expect(probe?.()).resolves.toBe('quiet') + }) + + it('reports unknown rather than quiet when the slave cannot be read', async () => { + const probe = createPtySlaveEchoProbe('/dev/ttys048', 'darwin') + answerStty(null) + await expect(probe?.()).resolves.toBe('unknown') + }) + + it('reports unknown when the output carries no echo flag at all', async () => { + const probe = createPtySlaveEchoProbe('/dev/ttys048', 'darwin') + answerStty('speed 38400 baud;\n') + await expect(probe?.()).resolves.toBe('unknown') + }) + + it('stops spawning stty once it has failed, but keeps re-reading a live slave', async () => { + const probe = createPtySlaveEchoProbe('/dev/ttys048', 'darwin') + answerStty(null) + await probe?.() + await probe?.() + await probe?.() + expect(execFileMock).toHaveBeenCalledTimes(1) + + // The bit itself is what changes, so a working probe is never cached. + const live = createPtySlaveEchoProbe('/dev/ttys048', 'darwin') + answerStty(COOKED) + await expect(live?.()).resolves.toBe('echoing') + answerStty(RAW) + await expect(live?.()).resolves.toBe('quiet') + expect(execFileMock).toHaveBeenCalledTimes(3) + }) + + it('keeps probing after a transient failure and only latches a permanent one', async () => { + const probe = createPtySlaveEchoProbe('/dev/ttys048', 'darwin') + // Why: a multi-pane restore forks these in a burst, so EAGAIN and the timeout kill + // are contention — condemning the pty to guessing for its whole life on one of + // those is the failure mode, not the protection. + for (const transient of [ + Object.assign(new Error('spawn EAGAIN'), { code: 'EAGAIN' }), + Object.assign(new Error('killed'), { killed: true }), + Object.assign(new Error('too many files'), { code: 'EMFILE' }) + ]) { + execFileMock.mockImplementationOnce((_c, _a, _o, cb) => cb(transient, '', '')) + await expect(probe?.()).resolves.toBe('unknown') + } + execFileMock.mockImplementationOnce((_c, _a, _o, cb) => cb(null, RAW, '')) + await expect(probe?.()).resolves.toBe('quiet') + expect(execFileMock).toHaveBeenCalledTimes(4) + + // A non-zero exit means the device is gone or was never a tty: permanent. + execFileMock.mockImplementationOnce((_c, _a, _o, cb) => + cb(Object.assign(new Error('not a tty'), { code: 1 }), '', '') + ) + await expect(probe?.()).resolves.toBe('unknown') + await expect(probe?.()).resolves.toBe('unknown') + expect(execFileMock).toHaveBeenCalledTimes(5) + }) + + it('passes the device with the flag its own platform understands', async () => { + answerStty(RAW) + await createPtySlaveEchoProbe('/dev/ttys048', 'darwin')?.() + expect(execFileMock.mock.calls[0]?.[1]).toEqual(['-a', '-f', '/dev/ttys048']) + answerStty(RAW) + await createPtySlaveEchoProbe('/dev/pts/3', 'linux')?.() + expect(execFileMock.mock.calls[1]?.[1]).toEqual(['-a', '-F', '/dev/pts/3']) + }) +}) diff --git a/src/shared/pty-slave-line-discipline-echo.ts b/src/shared/pty-slave-line-discipline-echo.ts new file mode 100644 index 000000000..c54cd61e9 --- /dev/null +++ b/src/shared/pty-slave-line-discipline-echo.ts @@ -0,0 +1,102 @@ +import { execFile, type ExecFileException } from 'node:child_process' + +// Why this exists: a startup color reply is written to the PTY master, and a POSIX +// line discipline in ECHO copies it straight back out as visible junk (#12112). +// Whether that will happen is readable state on the slave, not something that has to +// be inferred from the bytes that come back — so Orca asks instead of guessing. + +/** `unknown` means "could not be determined", never "assume quiet". */ +export type PtySlaveLineDisciplineEcho = 'echoing' | 'quiet' | 'unknown' + +export type PtySlaveEchoProbe = () => Promise + +const STTY_TIMEOUT_MS = 2_000 +// `stty -a` prints the lflags as a space-separated list where a disabled flag is +// prefixed with `-`, so `echo` and `-echo` are the two tokens that matter. +const ECHO_FLAG = /(?:^|\s)(-?)echo(?:\s|$)/ + +function sttyArgs(ptsName: string, platform: NodeJS.Platform): readonly string[] { + // BSD/macOS take `-f`; Linux (GNU coreutils) takes `-F`. + return platform === 'darwin' || platform.includes('bsd') + ? ['-a', '-f', ptsName] + : ['-a', '-F', ptsName] +} + +function parseEchoFlag(sttyOutput: string): PtySlaveLineDisciplineEcho { + const match = ECHO_FLAG.exec(sttyOutput) + if (!match) { + return 'unknown' + } + return match[1] === '-' ? 'quiet' : 'echoing' +} + +type SttyProbeResult = { state: PtySlaveLineDisciplineEcho; permanent: boolean } + +/** + * A spawn that never ran (`stty` absent) or a device that answered non-zero (reaped, + * not a tty) will answer the same way forever. A kill by the timeout, or a fork that + * failed for want of a resource, is contention — the very thing a multi-pane restore + * produces — and must not condemn the pty to guessing for the rest of its life. + */ +function isPermanentSttyFailure(error: ExecFileException): boolean { + if (error.killed || error.signal) { + return false + } + return error.code !== 'EAGAIN' && error.code !== 'EMFILE' && error.code !== 'ENFILE' +} + +function runStty(ptsName: string, platform: NodeJS.Platform): Promise { + return new Promise((resolve) => { + execFile( + 'stty', + sttyArgs(ptsName, platform), + { timeout: STTY_TIMEOUT_MS, windowsHide: true }, + (error, stdout) => { + resolve( + error + ? { state: 'unknown', permanent: isPermanentSttyFailure(error) } + : { state: parseEchoFlag(stdout), permanent: false } + ) + } + ) + }) +} + +/** + * node-pty's UnixTerminal carries the slave device path, but its public typings do not + * declare it and the Windows terminal has no such field — so read it defensively. + */ +export function readPtySlavePath(pty: unknown): string | undefined { + const candidate = (pty as { ptsName?: unknown } | null | undefined)?.ptsName + return typeof candidate === 'string' && candidate.length > 0 ? candidate : undefined +} + +/** + * Probe for whether the slave would echo a write to the master right now. + * + * Returns undefined when the platform has no line discipline to read: ConPTY and + * wsl.exe do not echo a master write at all, so a caller with no probe is correct to + * write immediately rather than degraded. A probe that exists but answers `unknown` + * is the degraded case, and callers must not read that as `quiet`. + */ +export function createPtySlaveEchoProbe( + ptsName: string | undefined, + platform: NodeJS.Platform = process.platform +): PtySlaveEchoProbe | undefined { + if (platform === 'win32' || !ptsName) { + return undefined + } + // Why latch: `stty` missing or the slave already reaped is a permanent condition for + // this pty, and the caller polls — without this a dead probe respawns a process per + // attempt. A successful probe is never cached, because the bit is what changes, and a + // transient failure is not latched at all (see isPermanentSttyFailure). + let unavailable = false + return async () => { + if (unavailable) { + return 'unknown' + } + const result = await runStty(ptsName, platform) + unavailable = result.permanent + return result.state + } +} diff --git a/src/shared/pty-startup-ingress-contract.ts b/src/shared/pty-startup-ingress-contract.ts index c8e865faa..461c6d6c8 100644 --- a/src/shared/pty-startup-ingress-contract.ts +++ b/src/shared/pty-startup-ingress-contract.ts @@ -1,5 +1,6 @@ import type { PtyOwnerBackend } from './pty-owner-backend' import type { PtyStartupIngressIntent } from './pty-startup-ingress-intent' +import type { PtySlaveEchoProbe } from './pty-slave-line-discipline-echo' export type PtyIngressEmission = { data: string @@ -13,6 +14,12 @@ export type PtyStartupIngressOptions = { ownerBackend?: PtyOwnerBackend write: (data: string) => void onEmission: (emission: PtyIngressEmission) => void + /** + * Reports whether the slave would echo a reply written to the master. When present, + * the reply waits for `quiet` instead of relying on echo-shape recognition. Absent + * on backends with no line discipline to read (ConPTY, wsl.exe). + */ + echoProbe?: PtySlaveEchoProbe } export type PtyIngressSourceSpan = { @@ -27,6 +34,7 @@ export type PtyStartupIngressOperation = | { kind: 'snapshot' } | { kind: 'teardown' } | { kind: 'expire' } + | { kind: 'release-echo' } export function slicePtyIngressSourceSpan( span: PtyIngressSourceSpan, diff --git a/src/shared/pty-startup-ingress.test.ts b/src/shared/pty-startup-ingress.test.ts index a6ec33a37..c45d546d5 100644 --- a/src/shared/pty-startup-ingress.test.ts +++ b/src/shared/pty-startup-ingress.test.ts @@ -4,10 +4,28 @@ import { parsePtyStartupIngressIntent, type PtyIngressEmission } from './pty-startup-ingress' +import type { + PtySlaveEchoProbe, + PtySlaveLineDisciplineEcho +} from './pty-slave-line-discipline-echo' const COLORS = { foreground: '#2e3434', background: '#ffffff' } +const FOREGROUND_REPLY = '\x1b]10;rgb:2e2e/3434/3434\x1b\\' +const BACKGROUND_REPLY = '\x1b]11;rgb:ffff/ffff/ffff\x1b\\' +// The two echo shapes a cooked POSIX tty produces for a written reply: ECHOCTL +// caret forms, and readline eating `ESC ]` / ST while self-inserting the rest. +const POSIX_COOKED_ECHOES = [ + (reply: string): string => reply.replaceAll('\x1b', '^['), + (reply: string): string => reply.replaceAll('\x1b]', '\x07').replaceAll('\x1b\\', '') +] -function createHarness(options: { projection?: boolean; nested?: (data: string) => void } = {}) { +function createHarness( + options: { + projection?: boolean + nested?: (data: string) => void + echoProbe?: PtySlaveEchoProbe + } = {} +) { const emissions: PtyIngressEmission[] = [] let ingress!: PtyStartupIngress const writes: string[] = [] @@ -17,6 +35,7 @@ function createHarness(options: { projection?: boolean; nested?: (data: string) deadlineMs: 5_000 }, ...(options.projection ? { ownerBackend: 'windows-conpty' as const } : {}), + ...(options.echoProbe ? { echoProbe: options.echoProbe } : {}), write: (data) => { writes.push(data) options.nested?.(data) @@ -26,6 +45,19 @@ function createHarness(options: { projection?: boolean; nested?: (data: string) return { ingress, writes, emissions } } +/** Probe that answers from a script, repeating its last answer once exhausted. */ +function scriptedEchoProbe(...states: PtySlaveLineDisciplineEcho[]) { + let index = 0 + const probe: PtySlaveEchoProbe & { calls: number } = Object.assign( + async () => { + probe.calls += 1 + return states[Math.min(index++, states.length - 1)] ?? 'unknown' + }, + { calls: 0 } + ) + return probe +} + function visible(emissions: readonly PtyIngressEmission[]): string { return emissions.map((emission) => emission.data).join('') } @@ -42,12 +74,17 @@ describe('PtyStartupIngress', () => { expect(parsePtyStartupIngressIntent({ ...intent, deadlineMs: 30_001 })).toBeUndefined() }) - it('recognizes BEL/ST queries at every split and emits canonical replies', () => { + it('recognizes BEL/ST queries at every split and defers canonical replies', () => { + vi.useFakeTimers() const query = '\x1b]10;?\x07\x1b]11;?\x1b\\' for (let split = 0; split <= query.length; split += 1) { const { ingress, writes, emissions } = createHarness() ingress.accept(query.slice(0, split)) ingress.accept(query.slice(split)) + // Why: answering inside the query's own turn beats the querying program's + // tcsetattr, so a cooked tty echoes the reply as text instead (#12112). + expect(writes, `split ${split}`).toEqual([]) + vi.advanceTimersByTime(0) ingress.drainAndClose() expect(visible(emissions), `split ${split}`).toBe('') expect(writes, `split ${split}`).toEqual([ @@ -116,6 +153,7 @@ describe('PtyStartupIngress', () => { }) it('serializes a synchronous nested provider callback after the consumed query span', () => { + vi.useFakeTimers() const emissions: PtyIngressEmission[] = [] let ingress!: PtyStartupIngress ingress = new PtyStartupIngress({ @@ -124,6 +162,7 @@ describe('PtyStartupIngress', () => { onEmission: (emission) => emissions.push(emission) }) ingress.accept('before\x1b]10;?\x07after') + vi.advanceTimersByTime(0) ingress.drainAndClose() expect(emissions.map(({ data, transformed }) => ({ data, transformed }))).toEqual([ { data: 'before', transformed: false }, @@ -260,6 +299,515 @@ describe('PtyStartupIngress', () => { expect(visible(emissions)).toBe(input) }) + it('swallows a cooked POSIX echo of its own reply without re-sending it', () => { + // Why never re-send: POSIX ECHO copies the reply to the master but leaves it in + // the slave input queue, so the program still reads it; a second write would + // arrive on its stdin as unsolicited input once it is raw. + vi.useFakeTimers() + for (const echoOf of POSIX_COOKED_ECHOES) { + const writes: string[] = [] + const emissions: PtyIngressEmission[] = [] + let ingress!: PtyStartupIngress + ingress = new PtyStartupIngress({ + intent: { colors: COLORS, deadlineMs: 5_000 }, + ownerBackend: 'posix-pty', + write: (data) => { + writes.push(data) + ingress.accept(echoOf(data)) + }, + onEmission: (emission) => emissions.push(emission) + }) + + ingress.accept('\x1b]10;?\x07') + vi.advanceTimersByTime(0) + expect(writes).toEqual([FOREGROUND_REPLY]) + expect(visible(emissions)).toBe('') + + vi.advanceTimersByTime(5_000) + expect(writes).toEqual([FOREGROUND_REPLY]) + expect(visible(emissions)).toBe('') + ingress.drainAndClose() + } + }) + + it('swallows a cooked POSIX echo coalesced behind earlier program output', () => { + // Why this shape: an agent pane is launched by writing a command into an interactive + // shell, so the tty echo of Orca's reply never arrives at the head of a read (#12112). + vi.useFakeTimers() + for (const echoOf of POSIX_COOKED_ECHOES) { + const replies: string[] = [] + const emissions: PtyIngressEmission[] = [] + const ingress = new PtyStartupIngress({ + intent: { colors: COLORS, deadlineMs: 5_000 }, + ownerBackend: 'posix-pty', + write: (data) => replies.push(data), + onEmission: (emission) => emissions.push(emission) + }) + + ingress.accept('\x1b]10;?\x07\x1b]11;?\x07') + vi.advanceTimersByTime(0) + expect(replies).toHaveLength(2) + + // A read with no echo in it must not retire the projections either. + ingress.accept('booting...\r\n') + ingress.accept(`\x1b[2JFRAME${replies.map((reply) => echoOf(reply)).join('')}`) + ingress.drainAndClose() + + expect(visible(emissions)).toBe('booting...\r\n\x1b[2JFRAME') + } + }) + + it('answers both slots when the deferred write lands between two reads of the burst', () => { + // Why between: a pty read boundary is a macrotask, so the deferred reply is written + // while the rest of the burst is still unread. A `\x07` head-of-echo guess taken then + // steals the OSC 11 terminator, leaving the slot unanswered and its bytes emitted + // after the BEL — which parks xterm in an OSC that never terminates. + vi.useFakeTimers() + const burst = '\x1b]10;?\x07\x1b]11;?\x07' + for (let split = 0; split <= burst.length; split += 1) { + const { ingress, writes, emissions } = createHarness() + ingress.accept(burst.slice(0, split)) + vi.advanceTimersByTime(0) + ingress.accept(burst.slice(split)) + vi.advanceTimersByTime(0) + ingress.drainAndClose() + + expect(writes, `split ${split}`).toEqual([FOREGROUND_REPLY, BACKGROUND_REPLY]) + expect(visible(emissions), `split ${split}`).toBe('') + } + }) + + it('keeps raw ranges disjoint when an echo lands on a retained torn query', () => { + vi.useFakeTimers() + const { ingress, writes, emissions } = createHarness() + ingress.accept('\x1b]10;?\x07\x1b]11;?') + vi.advanceTimersByTime(0) + ingress.accept(`${writes[0]?.replaceAll('\x1b', '^[')}tail`) + const accepted = ingress.drainAndClose() + + expect(visible(emissions)).toBe('\x1b]11;?tail') + // Why exact ranges: a candidate carried across the suppressed echo re-emits its own + // bytes on a span whose end no longer matches its data, so ranges start to overlap. + expect(emissions.map((item) => [item.rawStartSeq, item.rawEndSeq])).toEqual([ + [0, 7], + [7, 13], + [13, 40], + [40, accepted] + ]) + }) + + it('releases a partial echo hold long before the startup deadline', () => { + vi.useFakeTimers() + const { ingress, writes, emissions } = createHarness() + ingress.accept('\x1b]10;?\x07') + vi.advanceTimersByTime(0) + expect(writes).toEqual([FOREGROUND_REPLY]) + + // Why a range and not the exact hold: what matters is that the guess outlasts + // relay jitter yet still resolves without the deadline's help. Pinning the exact + // value would fail on any honest retune while teaching the retuner nothing. + const RELAY_JITTER_MS = 400 + const WELL_BELOW_DEADLINE_MS = 1_500 + + // A lone BEL is the head of the readline echo projection, so it is held. + ingress.accept('\x07') + expect(visible(emissions)).toBe('') + vi.advanceTimersByTime(RELAY_JITTER_MS) + expect(visible(emissions)).toBe('') + vi.advanceTimersByTime(WELL_BELOW_DEADLINE_MS - RELAY_JITTER_MS) + + expect(visible(emissions)).toBe('\x07') + ingress.drainAndClose() + }) + + it('still swallows the echo of a reply the startup deadline raced', () => { + vi.useFakeTimers() + const writes: string[] = [] + const emissions: PtyIngressEmission[] = [] + const ingress = new PtyStartupIngress({ + intent: { colors: COLORS, deadlineMs: 5_000 }, + ownerBackend: 'posix-pty', + write: (data) => writes.push(data), + onEmission: (emission) => emissions.push(emission) + }) + + vi.advanceTimersByTime(4_999) + ingress.accept('\x1b]10;?\x07') + // The deferred write flushes at 4_999, then the deadline expires at 5_000. + vi.advanceTimersByTime(2) + expect(writes).toEqual([FOREGROUND_REPLY]) + + ingress.accept(FOREGROUND_REPLY.replaceAll('\x1b', '^[')) + ingress.drainAndClose() + expect(visible(emissions)).toBe('') + }) + + it('writes a reply the startup deadline raced instead of dropping it', () => { + // Why: the query span was already consumed, so nobody downstream can answer it. + vi.useFakeTimers() + const writes: string[] = [] + const emissions: PtyIngressEmission[] = [] + const ingress = new PtyStartupIngress({ + intent: { colors: COLORS, deadlineMs: 5_000 }, + ownerBackend: 'posix-pty', + write: (data) => writes.push(data), + onEmission: (emission) => emissions.push(emission) + }) + + vi.advanceTimersByTime(4_999) + ingress.accept('\x1b]10;?\x07') + expect(writes).toEqual([]) + vi.advanceTimersByTime(1) + + expect(visible(emissions)).toBe('') + expect(writes).toEqual([FOREGROUND_REPLY]) + ingress.drainAndClose() + }) + + it('keeps the synchronous write for ConPTY-hosted wsl.exe panes', () => { + // Why: a Windows-hosted pty must be answered before conhost's own responder. + const writes: string[] = [] + const ingress = new PtyStartupIngress({ + intent: { colors: COLORS, deadlineMs: 5_000 }, + ownerBackend: 'windows-wsl', + write: (data) => writes.push(data), + onEmission: () => {} + }) + + ingress.accept('\x1b]10;?\x07') + expect(writes).toEqual([FOREGROUND_REPLY]) + ingress.drainAndClose() + }) + + it('swallows a coalesced POSIX echo torn at every byte boundary', () => { + // Why a prefix matters: recognition used to hold a split echo only when it began + // at offset 0, so a single byte of program output ahead of it made every torn + // boundary leak the reply verbatim — the exact #12112 symptom the fix targets. + vi.useFakeTimers() + for (const echoOf of POSIX_COOKED_ECHOES) { + const echo = echoOf(FOREGROUND_REPLY) + for (let split = 1; split < echo.length; split += 1) { + const writes: string[] = [] + const emissions: PtyIngressEmission[] = [] + const ingress = new PtyStartupIngress({ + intent: { colors: COLORS, deadlineMs: 5_000 }, + ownerBackend: 'posix-pty', + write: (data) => writes.push(data), + onEmission: (emission) => emissions.push(emission) + }) + + ingress.accept('\x1b]10;?\x07') + vi.advanceTimersByTime(0) + expect(writes).toEqual([FOREGROUND_REPLY]) + + ingress.accept(`FRAME${echo.slice(0, split)}`) + ingress.accept(echo.slice(split)) + ingress.drainAndClose() + + expect(visible(emissions)).toBe('FRAME') + } + } + }) + + it('still recognizes an echo that arrives behind an enormous splash frame', () => { + // Why: the search budget must not be spent by one large frame, retiring the + // projection while the echo is still in flight behind it. + vi.useFakeTimers() + const writes: string[] = [] + const emissions: PtyIngressEmission[] = [] + const ingress = new PtyStartupIngress({ + intent: { colors: COLORS, deadlineMs: 5_000 }, + ownerBackend: 'posix-pty', + write: (data) => writes.push(data), + onEmission: (emission) => emissions.push(emission) + }) + + ingress.accept('\x1b]10;?\x07') + vi.advanceTimersByTime(0) + expect(writes).toEqual([FOREGROUND_REPLY]) + + // One enormous splash frame must not retire the projection. + ingress.accept('x'.repeat(64_000)) + ingress.accept(FOREGROUND_REPLY.replaceAll('\x1b', '^[')) + ingress.drainAndClose() + + expect(visible(emissions)).toBe('x'.repeat(64_000)) + }) + + it('stops shadowing the stream once the projection outlives its search budget', () => { + vi.useFakeTimers() + const writes: string[] = [] + const emissions: PtyIngressEmission[] = [] + const ingress = new PtyStartupIngress({ + intent: { colors: COLORS, deadlineMs: 5_000 }, + ownerBackend: 'posix-pty', + write: (data) => writes.push(data), + onEmission: (emission) => emissions.push(emission) + }) + + ingress.accept('\x1b]10;?\x07') + vi.advanceTimersByTime(0) + const printed = 'tick\r\n'.repeat(50_000) + ingress.accept(printed) + + // The echo never came, so a later exact collision is ordinary output again. + const collision = FOREGROUND_REPLY.replaceAll('\x1b', '^[') + ingress.accept(collision) + ingress.drainAndClose() + expect(visible(emissions)).toBe(`${printed}${collision}`) + }) + + it('bounds echo suppression to a few hundred bytes past the startup deadline', () => { + // Why: reset() keeps a raced reply recognizable, but an unbounded projection would + // keep deleting matching spans out of ordinary output for the rest of the session. + vi.useFakeTimers() + const writes: string[] = [] + const emissions: PtyIngressEmission[] = [] + const ingress = new PtyStartupIngress({ + intent: { colors: COLORS, deadlineMs: 5_000 }, + ownerBackend: 'posix-pty', + write: (data) => writes.push(data), + onEmission: (emission) => emissions.push(emission) + }) + + vi.advanceTimersByTime(4_999) + ingress.accept('\x1b]10;?\x07') + vi.advanceTimersByTime(2) + expect(writes).toEqual([FOREGROUND_REPLY]) + + const printed = 'a\r\n'.repeat(200) + ingress.accept(printed) + const collision = FOREGROUND_REPLY.replaceAll('\x1b', '^[') + ingress.accept(collision) + ingress.drainAndClose() + + expect(visible(emissions)).toBe(`${printed}${collision}`) + }) + + it('drops its answered claim when a deferred write fails so a retry falls through', () => { + // Why: the deferred write already reported success, so the first query was consumed + // on its behalf. Without the rollback the slot stays claimed forever and no + // downstream color authority ever sees the query either. + vi.useFakeTimers() + let failWrites = true + const emissions: PtyIngressEmission[] = [] + const writes: string[] = [] + const ingress = new PtyStartupIngress({ + intent: { colors: COLORS, deadlineMs: 5_000 }, + ownerBackend: 'posix-pty', + write: (data) => { + if (failWrites) { + throw new Error('EIO') + } + writes.push(data) + }, + onEmission: (emission) => emissions.push(emission) + }) + + ingress.accept('\x1b]10;?\x07') + vi.advanceTimersByTime(0) + expect(writes).toEqual([]) + + failWrites = false + ingress.accept('\x1b]10;?\x07') + vi.advanceTimersByTime(0) + ingress.drainAndClose() + expect(writes).toEqual([FOREGROUND_REPLY]) + }) + + it('ages a projection out even when every read ends mid-candidate', () => { + // Why: the read budget is charged once per read at the entry point, so a stream + // whose every read ends on a candidate byte still retires a projection that never + // lands. Charging only on reads that fall through left it alive forever. + vi.useFakeTimers() + const writes: string[] = [] + const emissions: PtyIngressEmission[] = [] + const ingress = new PtyStartupIngress({ + intent: { colors: COLORS, deadlineMs: 5_000 }, + ownerBackend: 'posix-pty', + write: (data) => writes.push(data), + onEmission: (emission) => emissions.push(emission) + }) + + ingress.accept('\x1b]10;?\x07') + vi.advanceTimersByTime(0) + expect(writes).toEqual([FOREGROUND_REPLY]) + + // A trailing `^` is a strict prefix of the caret projection, so every one of these + // reads returns holding a candidate. + let printed = '' + for (let read = 0; read < 8; read += 1) { + const chunk = `${'line of output\r\n'.repeat(4_000)}^` + printed += chunk + ingress.accept(chunk) + } + + const collision = FOREGROUND_REPLY.replaceAll('\x1b', '^[') + ingress.accept(collision) + ingress.drainAndClose() + expect(visible(emissions)).toBe(`${printed}${collision}`) + }) + + it('swallows an echo no matter how finely the tty chunks it', () => { + // Why: an SSH relay or a slow drain delivers the echo a few bytes at a time. A + // per-read budget was spent inside the echo itself, so the leak came back for any + // chunking finer than the budget. + vi.useFakeTimers() + const echo = FOREGROUND_REPLY.replaceAll('\x1b', '^[') + for (const chunkSize of [1, 2, 3, 5, 13]) { + const writes: string[] = [] + const emissions: PtyIngressEmission[] = [] + const ingress = new PtyStartupIngress({ + intent: { colors: COLORS, deadlineMs: 5_000 }, + ownerBackend: 'posix-pty', + write: (data) => writes.push(data), + onEmission: (emission) => emissions.push(emission) + }) + + ingress.accept('\x1b]10;?\x07') + vi.advanceTimersByTime(0) + expect(writes).toEqual([FOREGROUND_REPLY]) + for (let at = 0; at < echo.length; at += chunkSize) { + ingress.accept(echo.slice(at, at + chunkSize)) + } + ingress.drainAndClose() + + expect({ chunkSize, visible: visible(emissions) }).toEqual({ chunkSize, visible: '' }) + } + }) + + it('lets every downstream barrier cut a partial echo hold short', () => { + // Why pinned: the hold window is only affordable because it is not what bounds + // the wait — these are. If one stopped releasing, the window would become a + // real stall rather than a bet on the next read. + vi.useFakeTimers() + const cutShort: Record void> = { + snapshotBarrier: (ingress) => ingress.snapshotBarrier(), + drainAndClose: (ingress) => ingress.drainAndClose(), + startupDeadline: () => vi.advanceTimersByTime(5_000) + } + for (const [name, cut] of Object.entries(cutShort)) { + const { ingress, writes, emissions } = createHarness() + ingress.accept('\x1b]10;?\x07') + vi.advanceTimersByTime(0) + expect(writes).toEqual([FOREGROUND_REPLY]) + + // A lone BEL heads the readline projection, so it is held rather than shown. + ingress.accept('\x07') + expect({ name, held: visible(emissions) }).toEqual({ name, held: '' }) + cut(ingress) + + expect({ name, released: visible(emissions) }).toEqual({ name, released: '\x07' }) + } + }) + + it('keeps swallowing an echo split across the query-authority handoff', () => { + // Why the asymmetry with snapshotBarrier is deliberate: closing query authority + // hands off who may answer, but the reply is already on the wire and its echo is + // still Orca's to swallow. Cutting the hold here would show its first half. + vi.useFakeTimers() + const { ingress, writes, emissions } = createHarness() + const echo = FOREGROUND_REPLY.replaceAll('\x1b', '^[') + + ingress.accept('\x1b]10;?\x07') + vi.advanceTimersByTime(0) + expect(writes).toEqual([FOREGROUND_REPLY]) + ingress.accept(echo.slice(0, 10)) + ingress.closeQueryAuthority() + ingress.accept(echo.slice(10)) + ingress.drainAndClose() + + expect(visible(emissions)).toBe('') + }) + + it('swallows an echo whose halves straddle a relay-sized stall', () => { + // Why: an expired hold releases raw, so a hold shorter than real inter-chunk + // jitter reinstates the leak on exactly the links Orca has to work over. + vi.useFakeTimers() + const echo = FOREGROUND_REPLY.replaceAll('\x1b', '^[') + for (const gapMs of [50, 200, 400]) { + const { ingress, writes, emissions } = createHarness() + ingress.accept('\x1b]10;?\x07') + vi.advanceTimersByTime(0) + expect(writes).toEqual([FOREGROUND_REPLY]) + + ingress.accept(echo.slice(0, 10)) + vi.advanceTimersByTime(gapMs) + ingress.accept(echo.slice(10)) + ingress.drainAndClose() + + expect({ gapMs, visible: visible(emissions) }).toEqual({ gapMs, visible: '' }) + } + }) + + it('swallows an echo that arrives behind a query torn on an earlier read', () => { + // Why: the tty can tear the program's second query and start echoing the first + // reply in the same read. Refusing to hold while a query is pending leaked the + // whole echo, because the prefix that would have completed that query was never + // emitted first. + vi.useFakeTimers() + const { ingress, writes, emissions } = createHarness() + const echo = FOREGROUND_REPLY.replaceAll('\x1b', '^[') + + ingress.accept('\x1b]10;?\x07') + vi.advanceTimersByTime(0) + expect(writes).toEqual([FOREGROUND_REPLY]) + ingress.accept('\x1b]11;') + ingress.accept(`?\x07${echo.slice(0, 8)}`) + ingress.accept(echo.slice(8)) + vi.advanceTimersByTime(0) + ingress.drainAndClose() + + expect(visible(emissions)).toBe('') + // The torn query is still answered: it is the prefix that completes it. + expect(writes).toEqual([FOREGROUND_REPLY, BACKGROUND_REPLY]) + }) + + it('drops a torn query the next read disproves instead of the echo behind it', () => { + // Why: with the echo starting at offset 0 there is no prefix to complete the torn + // candidate, so preferring it fed the echo to the raw path and printed both. The + // candidate is not a color query at all once the echo's first byte lands. + vi.useFakeTimers() + const { ingress, writes, emissions } = createHarness() + const echo = FOREGROUND_REPLY.replaceAll('\x1b', '^[') + + ingress.accept('\x1b]10;?\x07') + vi.advanceTimersByTime(0) + ingress.accept('\x1b]11;') + ingress.accept(echo.slice(0, 8)) + ingress.accept(echo.slice(8)) + vi.advanceTimersByTime(0) + ingress.drainAndClose() + + // Only the program's own bytes survive; the echo is gone rather than trailing them. + expect(visible(emissions)).toBe('\x1b]11;') + expect(writes).toEqual([FOREGROUND_REPLY]) + }) + + it('keeps a landed reply claimed when the sibling query write fails', () => { + // Why: ConPTY writes inside the query's own turn, so one span can land slot 10 and + // lose slot 11. Forgetting every claim would answer 10 a second time, and a + // duplicate reply corrupts a parser already mid-read. + const writes: string[] = [] + const emissions: PtyIngressEmission[] = [] + const ingress = new PtyStartupIngress({ + intent: { colors: COLORS, deadlineMs: 5_000 }, + ownerBackend: 'windows-conpty', + write: (data) => { + if (data === BACKGROUND_REPLY) { + throw new Error('EIO') + } + writes.push(data) + }, + onEmission: (emission) => emissions.push(emission) + }) + + ingress.accept('\x1b]10;?\x07\x1b]11;?\x07') + ingress.accept('\x1b]10;?\x07\x1b]11;?\x07') + ingress.drainAndClose() + expect(writes).toEqual([FOREGROUND_REPLY]) + }) + it('ignores callbacks after teardown without recreating the raw sequence domain', () => { const { ingress, emissions } = createHarness({ projection: true }) ingress.accept('\x1b]10;?\x07') @@ -269,4 +817,140 @@ describe('PtyStartupIngress', () => { expect(ingress.acceptedRawSequence).toBe(closedAt) expect(visible(emissions)).toBe(']10;rgb:2e2e/') }) + + it('withholds the reply while the slave would echo it, then writes once it is quiet', async () => { + vi.useFakeTimers() + const echoProbe = scriptedEchoProbe('echoing', 'echoing', 'quiet') + const { ingress, writes } = createHarness({ echoProbe }) + ingress.accept('\x1b]10;?\x07') + await vi.advanceTimersByTimeAsync(0) + // Nothing may go out while the line discipline is still cooked: that write is the + // one that comes straight back as visible junk (#12112). + expect(writes).toEqual([]) + await vi.advanceTimersByTimeAsync(20) + expect(writes).toEqual([]) + await vi.advanceTimersByTimeAsync(20) + expect(writes).toEqual([FOREGROUND_REPLY]) + expect(echoProbe.calls).toBe(3) + ingress.drainAndClose() + }) + + it('retires only the kernel caret projection once the probe proves ECHO is clear', async () => { + vi.useFakeTimers() + const { ingress, writes, emissions } = createHarness({ + echoProbe: scriptedEchoProbe('quiet') + }) + ingress.accept('\x1b]10;?\x07') + await vi.advanceTimersByTimeAsync(0) + expect(writes).toEqual([FOREGROUND_REPLY]) + // A cleared ECHO bit proves the kernel cannot produce the caret form, so output + // that merely resembles it is ordinary program output and must survive. + const caret = POSIX_COOKED_ECHOES[0]?.(FOREGROUND_REPLY) ?? '' + ingress.accept(caret) + ingress.drainAndClose() + expect(visible(emissions)).toBe(caret) + }) + + it('still suppresses the readline echo on a slave the probe called quiet', async () => { + vi.useFakeTimers() + const { ingress, writes, emissions } = createHarness({ + echoProbe: scriptedEchoProbe('quiet') + }) + ingress.accept('\x1b]10;?\x07') + await vi.advanceTimersByTimeAsync(0) + expect(writes).toEqual([FOREGROUND_REPLY]) + // Why: readline echoes a master write in software with the tty already raw and + // ECHO off, so `quiet` is no evidence at all about this shape. Verified on a live + // pty: at a bash prompt the probe reports quiet and readline still emits it. + ingress.accept(POSIX_COOKED_ECHOES[1]?.(FOREGROUND_REPLY) ?? '') + ingress.drainAndClose() + expect(visible(emissions)).toBe('') + }) + + it('falls back to recognizing echo shapes when the probe cannot answer', async () => { + vi.useFakeTimers() + const { ingress, writes, emissions } = createHarness({ + echoProbe: scriptedEchoProbe('unknown') + }) + ingress.accept('\x1b]10;?\x07') + await vi.advanceTimersByTimeAsync(0) + expect(writes).toEqual([FOREGROUND_REPLY]) + // `unknown` is not evidence of quiet, so the guess stays armed and swallows the echo. + ingress.accept(POSIX_COOKED_ECHOES[0]?.(FOREGROUND_REPLY) ?? '') + ingress.drainAndClose() + expect(visible(emissions)).toBe('') + }) + + it('falls back immediately when the echo probe rejects', async () => { + vi.useFakeTimers() + const echoProbe: PtySlaveEchoProbe = async () => { + throw new Error('probe failed') + } + const { ingress, writes, emissions } = createHarness({ echoProbe }) + ingress.accept('\x1b]10;?\x07') + + await vi.advanceTimersByTimeAsync(0) + + expect(writes).toEqual([FOREGROUND_REPLY]) + ingress.accept(POSIX_COOKED_ECHOES[0]?.(FOREGROUND_REPLY) ?? '') + ingress.drainAndClose() + expect(visible(emissions)).toBe('') + }) + + it('stops polling a tty that never leaves cooked mode and answers it anyway', async () => { + vi.useFakeTimers() + const echoProbe = scriptedEchoProbe('echoing') + const { ingress, writes, emissions } = createHarness({ echoProbe }) + ingress.accept('\x1b]10;?\x07') + await vi.advanceTimersByTimeAsync(1_000) + // Waiting past this point only delays a reply that will echo whenever it is sent, + // so the reply goes out with the shape guess armed rather than being dropped. + expect(writes).toEqual([FOREGROUND_REPLY]) + // Bounded in wall-clock, not in probes: under fork contention each probe takes + // longer and the budget buys fewer of them, instead of the wait growing. + expect(echoProbe.calls).toBeLessThanOrEqual(11) + ingress.accept(POSIX_COOKED_ECHOES[0]?.(FOREGROUND_REPLY) ?? '') + ingress.drainAndClose() + expect(visible(emissions)).toBe('') + }) + + it('gives a later query its own probe budget, not the first query remainder', async () => { + vi.useFakeTimers() + const echoProbe = scriptedEchoProbe('echoing') + const { ingress, writes } = createHarness({ echoProbe }) + ingress.accept('\x1b]10;?\x07') + await vi.advanceTimersByTimeAsync(1_000) + expect(writes).toEqual([FOREGROUND_REPLY]) + const spentOnFirst = echoProbe.calls + // Why: OSC 10 and OSC 11 routinely arrive more than a budget apart over SSH. A + // counter carried across them would send the second reply out entirely unprobed. + ingress.accept('\x1b]11;?\x07') + await vi.advanceTimersByTimeAsync(1_000) + expect(writes).toEqual([FOREGROUND_REPLY, BACKGROUND_REPLY]) + expect(echoProbe.calls).toBeGreaterThan(spentOnFirst) + ingress.drainAndClose() + }) + + it('answers a still-pending reply when the startup deadline expires mid-poll', async () => { + vi.useFakeTimers() + const { ingress, writes } = createHarness({ echoProbe: scriptedEchoProbe('echoing') }) + ingress.accept('\x1b]10;?\x07') + await vi.advanceTimersByTimeAsync(60) + expect(writes).toEqual([]) + // The deadline is the outer bound: a reply held by a cooked tty still gets sent + // rather than dropped, because the querying program is blocked on it. + await vi.advanceTimersByTimeAsync(5_000) + expect(writes).toEqual([FOREGROUND_REPLY]) + ingress.drainAndClose() + }) + + it('drops a held reply on teardown instead of writing to a dead pty', async () => { + vi.useFakeTimers() + const { ingress, writes } = createHarness({ echoProbe: scriptedEchoProbe('echoing') }) + ingress.accept('\x1b]10;?\x07') + await vi.advanceTimersByTimeAsync(20) + ingress.drainAndClose() + await vi.advanceTimersByTimeAsync(1_000) + expect(writes).toEqual([]) + }) }) diff --git a/src/shared/pty-startup-ingress.ts b/src/shared/pty-startup-ingress.ts index 0675eb92a..edddd73dd 100644 --- a/src/shared/pty-startup-ingress.ts +++ b/src/shared/pty-startup-ingress.ts @@ -5,6 +5,7 @@ import { } from './terminal-osc-color-reply' import type { PtyStartupIngressIntent } from './pty-startup-ingress-intent' import type { PtyOwnerBackend } from './pty-owner-backend' +import { PtyStartupReplyDelivery } from './pty-startup-reply-delivery' import { combinePtyIngressSourceSpans, slicePtyIngressSourceSpan, @@ -22,11 +23,13 @@ export type { PtyStartupIngressIntent } from './pty-startup-ingress-intent' export type { PtyIngressEmission, PtyStartupIngressOptions } from './pty-startup-ingress-contract' const MAX_QUERY_CANDIDATE_CHARS = 64 - -function projectedWindowsConptyReply(reply: string): string { - // Why: the native provider harness observes ConPTY's cooked echo with ESC removed. - return reply.replaceAll('\x1b', '') -} +// Why this long: a torn echo whose halves straddle this window is released raw, so +// anything under relay jitter reinstates the leak (#12112). Almost nothing is risked +// by waiting, because the timer is rarely what ends a hold — the next read is, and +// the startup deadline and snapshot barrier both cap the wait independently. The +// exposure is at most one projection's worth of echo-shaped bytes on an already idle +// pane, which is why the guess is allowed to be slow rather than tight. +const ECHO_CONTINUATION_HOLD_MS = 500 /** * Serialized source-side startup classifier. Its raw sequence begins after @@ -35,23 +38,23 @@ function projectedWindowsConptyReply(reply: string): string { export class PtyStartupIngress { private readonly intent: PtyStartupIngressIntent | undefined private readonly ownerBackend: PtyOwnerBackend - private readonly writeProvider: (data: string) => void + private readonly delivery: PtyStartupReplyDelivery private readonly onEmission: (emission: PtyIngressEmission) => void private readonly operations: PtyStartupIngressOperation[] = [] private readonly answeredSlots = new Set() - private readonly expectedEchoes: string[] = [] private processing = false private closed = false private queryOpen: boolean private rawHighWater = 0 private queryPending: PtyIngressSourceSpan | null = null private echoPending: PtyIngressSourceSpan | null = null + private echoHoldTimer: ReturnType | null = null private deadlineTimer: ReturnType | null = null constructor(options: PtyStartupIngressOptions) { this.intent = options.intent this.ownerBackend = options.ownerBackend ?? 'posix-pty' - this.writeProvider = options.write + this.delivery = new PtyStartupReplyDelivery(this.ownerBackend, options.write, options.echoProbe) this.onEmission = options.onEmission this.queryOpen = options.intent !== undefined if (options.intent) { @@ -121,61 +124,110 @@ export class PtyStartupIngress { case 'close-query': if (this.ownerBackend !== 'windows-conpty') { 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 + // Orca's to swallow. Releasing here would show the first half of an echo + // split across the boundary and orphan the second. this.releaseQueryPending() } // Why: ConPTY cannot safely transfer color-query authority to a downstream view. return case 'expire': this.queryOpen = false - this.releaseEchoPending() - if (this.ownerBackend !== 'windows-conpty') { - this.releaseQueryPending() - } - this.expectedEchoes.length = 0 + this.releasePendingInSourceOrder(false) + this.delivery.reset() this.clearDeadline() return case 'snapshot': - this.releaseSnapshotPending() + case 'release-echo': + this.releasePendingInSourceOrder(false) return case 'teardown': this.queryOpen = false - this.releaseAllPending() - this.expectedEchoes.length = 0 + this.releasePendingInSourceOrder(true) + this.delivery.close() this.clearDeadline() this.closed = true } } + /** + * One PTY read. The charge is in `finally` because every path below can return + * early: charging after the match gives a real echo the whole read it arrives in, + * and charging unconditionally means a projection that never lands still ages out + * on the reads that end mid-candidate rather than shadowing the rest of the session. + * It charges the read, never the held-bytes-plus-read span, so a tail that waits + * across several reads is not billed again on each one. + */ private processEchoSpan(span: PtyIngressSourceSpan): void { - let input = combinePtyIngressSourceSpans(this.echoPending, span) - this.echoPending = null + try { + this.classifyRead(span) + } finally { + this.delivery.chargeEchoSearch(span.data.length) + } + } - while (this.expectedEchoes.length > 0) { - const expected = this.expectedEchoes[0] - const compared = Math.min(input.data.length, expected.length) - let matching = 0 - while (matching < compared && input.data[matching] === expected[matching]) { - matching += 1 - } - if (matching < compared) { - this.expectedEchoes.shift() - this.processQuerySpan(input) - return - } - if (input.data.length < expected.length) { - this.echoPending = input - return - } + private classifyRead(span: PtyIngressSourceSpan): void { + let input = combinePtyIngressSourceSpans(this.takeEchoPending(), span) - this.expectedEchoes.shift() - this.emit(slicePtyIngressSourceSpan(input, 0, expected.length), true, '') - input = slicePtyIngressSourceSpan(input, expected.length) - if (input.data.length === 0) { - return + while (this.delivery.hasExpectedEcho && input.data.length > 0) { + const match = this.delivery.matchEcho(input.data) + if (match.kind !== 'complete') { + // Why hold from the match rather than only at offset 0: the tty coalesces its + // echo with whatever the shell printed around it, so a split echo almost + // always arrives behind other bytes. Those bytes are emitted now and only the + // candidate tail waits, so recognition survives a split at any boundary + // without stalling real output. + if (match.kind === 'partial') { + const tail = slicePtyIngressSourceSpan(input, match.offset) + if (match.offset > 0) { + this.processQuerySpan(slicePtyIngressSourceSpan(input, 0, match.offset)) + } + // A still-torn query outranks the echo only while it can still become one: + // the tail may open with the BEL that terminates it, since the readline + // projection starts with one. Re-parsing it against the tail is what tells + // the two apart — a candidate the tail *disproves* is ordinary output that + // would otherwise absorb the echo behind it and dump both raw (#12112). + // + // `partial` counts as viable, not just `match`: the terminator can arrive a + // read later, and demoting it would emit a bare ESC and leave a real query + // unanswered until the program's own timeout. On ConPTY that costs an echo, + // because the ESC-stripped projection shares the `]10;` prefix with a real + // query and so keeps re-parsing as `partial` — a hang is the worse of the two. + if (this.queryPending) { + const resolved = combinePtyIngressSourceSpans(this.queryPending, tail) + if (parseTerminalOscColorQuery(resolved.data, 0).kind !== 'none') { + this.processQuerySpan(tail) + return + } + // Unconditional, unlike `releasePendingInSourceOrder`, which withholds a + // ConPTY candidate: that one releases candidates still *undetermined*, + // and on ConPTY an undetermined candidate may be a query it is meant to + // suppress. Here the candidate and the tail together parse as `none`, so + // whatever the candidate is, the bytes behind it are not its body — which + // is what makes it safe to stop holding the echo hostage to it. + this.releaseQueryPending() + } + this.echoPending = tail + this.armEchoHold() + return + } + break } + if (match.offset > 0) { + this.processQuerySpan(slicePtyIngressSourceSpan(input, 0, match.offset)) + } + // Why release first: a retained torn candidate cannot straddle the suppressed + // range without desynchronizing its raw sequence arithmetic. + this.releaseQueryPending() + const echoEnd = match.offset + match.length + this.emit(slicePtyIngressSourceSpan(input, match.offset, echoEnd), true, '') + input = slicePtyIngressSourceSpan(input, echoEnd) } - this.processQuerySpan(input) + if (input.data.length > 0) { + this.processQuerySpan(input) + } } private processQuerySpan(span: PtyIngressSourceSpan): void { @@ -244,22 +296,15 @@ export class PtyStartupIngress { return wroteAny } this.answeredSlots.add(slot) - const projected = - this.ownerBackend === 'windows-conpty' ? projectedWindowsConptyReply(reply) : null - if (projected) { - // Why: register before write because node-pty can synchronously re-enter onData. - this.expectedEchoes.push(projected) - } - try { - this.writeProvider(reply) - wroteAny = true - } catch { + // Why per slot: the replies to one query are written independently, so a + // deferred write that fails after reporting success invalidates only its own + // claim. Dropping every claim would let a slot that did land be answered a + // second time, and a duplicate reply corrupts a parser already mid-read. + if (!this.delivery.answer(reply, () => this.answeredSlots.delete(slot))) { this.answeredSlots.delete(slot) - if (projected) { - this.expectedEchoes.pop() - } return wroteAny } + wroteAny = true } if (this.answeredSlots.has(10) && this.answeredSlots.has(11)) { @@ -277,28 +322,41 @@ export class PtyStartupIngress { this.emit(pending, false) } - private releaseAllPending(): void { - this.releaseEchoPending() - this.releaseQueryPending() - } - - private releaseEchoPending(): void { - if (!this.echoPending) { - return - } - const pending = this.echoPending - this.echoPending = null - this.emit(pending, false) - } - - private releaseSnapshotPending(): void { - if (this.echoPending) { - this.expectedEchoes.shift() - this.releaseEchoPending() - } - if (this.ownerBackend !== 'windows-conpty') { + /** + * Why this order: were both ever live, queryPending would hold the earlier source + * bytes. `classifyRead` only ever arms one — it either keeps a viable query and + * returns, or releases a disproven one before holding the echo — so this is defense + * against a future second arming site, not a live inversion. + */ + private releasePendingInSourceOrder(includeConptyQuery: boolean): void { + if (includeConptyQuery || this.ownerBackend !== 'windows-conpty') { this.releaseQueryPending() } + const pending = this.takeEchoPending() + if (pending) { + this.emit(pending, false) + } + } + + private takeEchoPending(): PtyIngressSourceSpan | null { + const pending = this.echoPending + this.echoPending = null + if (this.echoHoldTimer) { + clearTimeout(this.echoHoldTimer) + this.echoHoldTimer = null + } + return pending + } + + private armEchoHold(): void { + if (this.echoHoldTimer) { + return + } + this.echoHoldTimer = setTimeout( + () => this.enqueue({ kind: 'release-echo' }), + ECHO_CONTINUATION_HOLD_MS + ) + this.echoHoldTimer.unref?.() } private emit(span: PtyIngressSourceSpan, transformed: boolean, data = span.data): void { diff --git a/src/shared/pty-startup-reply-delivery.ts b/src/shared/pty-startup-reply-delivery.ts new file mode 100644 index 000000000..876fc26a0 --- /dev/null +++ b/src/shared/pty-startup-reply-delivery.ts @@ -0,0 +1,349 @@ +import type { PtyOwnerBackend } from './pty-owner-backend' +import type { PtySlaveEchoProbe } from './pty-slave-line-discipline-echo' + +// Why this module exists: a startup color reply is written to the PTY master, so +// whatever line discipline sits between Orca and the querying program can echo it +// straight back out as ordinary output (#12112). ConPTY echoes it with the ESC +// bytes stripped; a POSIX tty echoes it while the querying program is still cooked. +// A program that queries before clearing ECHO loses that race if Orca answers +// inside the query's own turn, so on POSIX the write waits until the slave's ECHO +// bit is observably clear, and recognized echo shapes cover what remains. +// +// Deliberately NO re-send on a matched echo: ECHO copies bytes to the master +// without consuming them from the slave's input queue, so a program that arms raw +// mode with TCSANOW/TCSADRAIN (libuv's setRawMode, hence Node-based agents) still +// receives the reply, and re-writing would duplicate it in stdin. A TCSAFLUSH +// switcher does discard it; that case is left to the query timeout, because a +// duplicate reply corrupts a parser that is already mid-read. +// +// Why not PostReadyFlushGate's settle-and-fallback shape, which solves this same +// "don't write while ECHO is on" race for shell startup input: that gate defers +// bytes nothing is waiting on, so it can wait for the stream to go observably +// quiet. A color reply is different — the querying program is blocked on it and +// times out — so the wait here is bounded by a budget and always ends in a write. +// +// There are TWO echo sources and they are independent, which is the thing to hold onto +// when reading the rest of this file: +// +// 1. The kernel line discipline, when ECHO is set. Readable state — the probe asks +// the slave directly, and waiting for it to clear removes this echo outright. +// 2. The foreground line editor, in software. readline echoes a master write as if +// it were typed *while the tty is raw with ECHO off*, so the probe's verdict says +// nothing about it. Verified on a live pty: at a bash prompt the probe reports +// `quiet` and readline still emits `BEL 10;rgb:2e2e/3434/3434`. +// +// So `quiet` is proof about (1) only. It gates the withholding and retires the caret +// projection, and must never be read as "no suppression needed" — that reintroduces +// #12112 at a shell prompt, which is the foreground for most of an agent pane's +// startup window. The projections below stay armed for (2) on every path. + +export type PtyStartupReplyEchoMatch = + | { kind: 'complete'; offset: number; length: number } + | { kind: 'partial'; offset: number } + | { kind: 'none' } + +// 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 +// per-read budget is then spent inside the echo itself. What actually bounds a live +// projection is the startup deadline; this is only a backstop against a pathological +// pre-deadline stream, so it is set well above any splash an echo could arrive behind. +const ECHO_SEARCH_BUDGET_BYTES = 256 * 1024 +// Why far tighter past the deadline: a reply still on the wire at expiry deserves the +// read or two its echo takes, but nothing beyond it — see reset(). +const ECHO_POST_DEADLINE_BUDGET_BYTES = 512 +// Why this tight: the querying program is blocked on the reply, so every interval is +// latency it pays. A raw-mode switch lands within a turn or two of the query, and the +// probe is a subprocess — this is the smallest interval that does not spin on it. +const ECHO_POLL_INTERVAL_MS = 20 +// Why a budget at all: the startup deadline runs to 30s, which at this interval is a +// four-figure count of probe subprocesses. It is also the wrong bound — a tty still +// cooked this long after its own query never leaves cooked mode, and waiting on it only +// delays a reply that will echo whenever it is sent. +// +// Why wall-clock rather than a probe count: each probe is a subprocess, so a multi-pane +// restore serializes them on fork — a count-based cap measured ~26ms per probe across +// 30 panes, turning a nominal 200ms into ~8s of withholding and blowing past every +// query timeout at once. A deadline spends fewer probes under load instead of taking +// longer, which is the direction that fails safe: measured flat at ~210ms of withholding +// from 1 to 100 concurrent panes, with probe spawns plateauing rather than scaling. +// +// This bounds when a probe is STARTED, not one already in flight, so the hard bound is +// this plus STTY_TIMEOUT_MS — still inside the startup deadline that reset() enforces. +const ECHO_POLL_BUDGET_MS = 200 + +type ExpectedEcho = { projections: readonly string[]; remainingBytes: number } +type PendingWrite = { reply: string; onFailed: (() => void) | undefined } + +/** Only a POSIX tty both echoes the reply and still delivers a deferred write. */ +function defersWrite(ownerBackend: PtyOwnerBackend): boolean { + return ownerBackend === 'posix-pty' +} + +function replyEchoProjections( + reply: string, + ownerBackend: PtyOwnerBackend, + kernelEchoImpossible: boolean +): readonly string[] { + if (ownerBackend === 'windows-conpty') { + // Why: ConPTY's projection is the documented, deterministic ESC-stripped form. + return [reply.replaceAll('\x1b', '')] + } + if (!defersWrite(ownerBackend)) { + // wsl.exe is ConPTY-hosted but its echo shape is unverified; suppress nothing. + return [] + } + // What makes both shapes below safe to match on is that neither starts with ESC, so + // no query can share a prefix with them. The verbatim echo of a `stty -echoctl` tty + // is deliberately NOT projected for exactly that reason: it is byte-identical to the + // reply, so a bare trailing ESC — how any read can end — is a strict prefix of it. + // That read would be held as an echo candidate, and an expired hold releases its + // bytes raw, past the query parser, so a query torn at its own ESC is never answered. + return [ + // ECHOCTL (default cooked tty) renders each control byte as its caret form. This is + // the ONE projection the probe can retire, because it is the kernel's echo and a + // cleared ECHO bit is proof it cannot happen. + ...(kernelEchoImpossible ? [] : [reply.replaceAll('\x1b', '^[')]), + // readline: `ESC ]` is an unbound binding, so it is eaten (with a bell) and the + // remainder self-inserts; the ST is eaten the same way. Software echo — survives + // `quiet`, because readline does this with the tty already raw and ECHO off. + // + // This buys display cleanliness ONLY. The bytes self-inserted into readline's edit + // buffer are still there, so a user who then presses Enter runs them: `bash: 10: + // command not found`, with nothing on screen to explain it. Not fixable by + // suppressing harder — undoing it means writing a kill-line into someone's prompt. + reply.replaceAll('\x1b]', '\x07').replaceAll('\x1b\\', '') + ] +} + +/** Earliest offset whose suffix of `data` is a strict prefix of `projection`, else -1. */ +function suffixPrefixOffset(projection: string, data: string): number { + for ( + let offset = Math.max(0, data.length - projection.length + 1); + offset < data.length; + offset += 1 + ) { + if (projection.startsWith(data.slice(offset))) { + return offset + } + } + return -1 +} + +// Why search the whole span: the tty coalesces its echo with whatever the shell and the +// program wrote around it, so anchoring at offset 0 recognizes almost no real echo. +function locateEcho(projections: readonly string[], data: string): PtyStartupReplyEchoMatch { + let complete: { offset: number; length: number } | null = null + let partialOffset = -1 + for (const projection of projections) { + const at = data.indexOf(projection) + if (at !== -1) { + if (!complete || at < complete.offset) { + complete = { offset: at, length: projection.length } + } + continue + } + const suffix = suffixPrefixOffset(projection, data) + if (suffix !== -1 && (partialOffset === -1 || suffix < partialOffset)) { + partialOffset = suffix + } + } + if (complete) { + return { kind: 'complete', ...complete } + } + return partialOffset === -1 ? { kind: 'none' } : { kind: 'partial', offset: partialOffset } +} + +function isBetterEchoMatch( + candidate: PtyStartupReplyEchoMatch, + best: PtyStartupReplyEchoMatch +): boolean { + if (candidate.kind === 'none') { + return false + } + if (best.kind === 'none') { + return true + } + if (candidate.kind !== best.kind) { + return candidate.kind === 'complete' + } + return candidate.offset < best.offset +} + +/** Owns when a startup color reply is written and how its own echo is recognized. */ +export class PtyStartupReplyDelivery { + private readonly expectedEchoes: ExpectedEcho[] = [] + private readonly pendingWrites: PendingWrite[] = [] + private writeTimer: ReturnType | null = null + private echoPollDeadline = 0 + private closed = false + + constructor( + private readonly ownerBackend: PtyOwnerBackend, + private readonly writeProvider: (data: string) => void, + private readonly echoProbe?: PtySlaveEchoProbe + ) {} + + get hasExpectedEcho(): boolean { + return this.expectedEchoes.length > 0 + } + + /** + * True once the reply has been written or accepted for a later write. + * + * `onFailed` fires only for the second case: a deferred write reports success + * before it happens, so the caller's bookkeeping for THIS reply is a lie if the + * write later throws. Scoped per reply because the replies to one query are + * written independently — one failing says nothing about the ones that landed. + */ + answer(reply: string, onFailed?: () => void): boolean { + if (this.closed) { + return false + } + if (!defersWrite(this.ownerBackend)) { + // Why: ConPTY answers the query itself unless Orca beats it in this turn. + return this.writeReply(reply) + } + // A fresh queue starts a fresh budget, so a second query arriving after the first + // one exhausted its own still gets probed rather than going straight to guessing. + if (this.pendingWrites.length === 0) { + this.echoPollDeadline = Date.now() + ECHO_POLL_BUDGET_MS + } + this.pendingWrites.push({ reply, onFailed }) + this.armWriteTimer() + return true + } + + /** Recognizes any written reply's echo anywhere in the span, earliest match first. */ + matchEcho(data: string): PtyStartupReplyEchoMatch { + let best: PtyStartupReplyEchoMatch = { kind: 'none' } + let bestIndex = -1 + for (const [index, expected] of this.expectedEchoes.entries()) { + const match = locateEcho(expected.projections, data) + if (isBetterEchoMatch(match, best)) { + best = match + bestIndex = index + } + } + if (best.kind === 'complete') { + this.expectedEchoes.splice(bestIndex, 1) + return best + } + return best + } + + /** + * Bytes that went by without completing an echo. Charged by the caller once per PTY + * read rather than per `matchEcho` call, which runs several times over one span. + */ + chargeEchoSearch(byteCount: number): void { + for (let index = this.expectedEchoes.length - 1; index >= 0; index -= 1) { + const expected = this.expectedEchoes[index] + if (!expected) { + continue + } + expected.remainingBytes -= byteCount + if (expected.remainingBytes <= 0) { + this.expectedEchoes.splice(index, 1) + } + } + } + + /** + * Startup window closed. Replies already on the wire stay recognizable, but only + * across the next few hundred bytes: an unbounded projection would keep deleting + * matching spans out of ordinary output for the rest of the session. + */ + reset(): void { + this.flushPendingWrites() + for (const expected of this.expectedEchoes) { + expected.remainingBytes = Math.min(expected.remainingBytes, ECHO_POST_DEADLINE_BUDGET_BYTES) + } + } + + /** Teardown: the pty is gone, so an unwritten reply has nowhere left to go. */ + close(): void { + this.closed = true + this.clearWriteTimer() + this.pendingWrites.length = 0 + this.expectedEchoes.length = 0 + } + + private armWriteTimer(delayMs = 0): void { + if (this.writeTimer) { + return + } + this.writeTimer = setTimeout(() => this.attemptPendingWrites(), delayMs) + this.writeTimer.unref?.() + } + + /** + * Why poll rather than write on the first turn: one deferred turn cannot prove the + * querying program left cooked mode, and the leak happens precisely because Orca + * answered before it got there. Waiting costs the program nothing it is not already + * spending — it is blocked on this reply either way. + */ + private attemptPendingWrites(): void { + this.clearWriteTimer() + if (this.closed || this.pendingWrites.length === 0) { + return + } + if (!this.echoProbe || Date.now() >= this.echoPollDeadline) { + this.flushPendingWrites() + return + } + void this.echoProbe() + .catch(() => 'unknown' as const) + .then((state) => { + if (this.closed || this.pendingWrites.length === 0) { + return + } + if (state === 'echoing') { + this.armWriteTimer(ECHO_POLL_INTERVAL_MS) + return + } + // `quiet` retires the kernel caret projection; `unknown` keeps both shapes. + this.flushPendingWrites(state === 'quiet') + }) + } + + private flushPendingWrites(kernelEchoImpossible = false): void { + this.clearWriteTimer() + for (const pending of this.pendingWrites.splice(0)) { + this.writeReply(pending.reply, pending.onFailed, kernelEchoImpossible) + } + } + + private clearWriteTimer(): void { + if (!this.writeTimer) { + return + } + clearTimeout(this.writeTimer) + this.writeTimer = null + } + + private writeReply(reply: string, onFailed?: () => void, kernelEchoImpossible = false): boolean { + if (this.closed) { + return false + } + const projections = replyEchoProjections(reply, this.ownerBackend, kernelEchoImpossible) + // Why: register before write because node-pty can synchronously re-enter onData. + const expected: ExpectedEcho | null = + projections.length > 0 ? { projections, remainingBytes: ECHO_SEARCH_BUDGET_BYTES } : null + if (expected) { + this.expectedEchoes.push(expected) + } + try { + this.writeProvider(reply) + return true + } catch { + // Why splice by identity, not pop: the write above can re-enter onData and + // retire a different projection, so the last slot is not necessarily ours. + const index = expected ? this.expectedEchoes.indexOf(expected) : -1 + if (index !== -1) { + this.expectedEchoes.splice(index, 1) + } + onFailed?.() + return false + } + } +} diff --git a/src/shared/terminal-mode-reset-profiles.test.ts b/src/shared/terminal-mode-reset-profiles.test.ts new file mode 100644 index 000000000..bddcd3dbe --- /dev/null +++ b/src/shared/terminal-mode-reset-profiles.test.ts @@ -0,0 +1,82 @@ +import { describe, expect, it } from 'vitest' +import { + COLD_RESTORE_SEED_MODE_RESET, + POST_REPLAY_LIVE_AGENT_REATTACH_RESET, + POST_REPLAY_LIVE_AGENT_SNAPSHOT_RESET, + POST_REPLAY_LIVE_SNAPSHOT_RESET, + POST_REPLAY_MODE_RESET, + POST_REPLAY_REATTACH_RESET, + RESET_MOUSE_REPORTING, + buildPostReplayLiveAgentReattachReset, + replayPayloadEndsWithCursorHidden +} from './terminal-mode-reset-profiles' + +// Why literal expectations: consumers import these constants, so only a byte-level +// assertion here can catch a profile silently losing a mode it is meant to clear. +describe('terminal mode reset profiles', () => { + it('clears every mouse protocol and encoding a snapshot can re-arm', () => { + expect(RESET_MOUSE_REPORTING).toBe( + '\x1b[?9l\x1b[?1000l\x1b[?1002l\x1b[?1003l\x1b[?1006l\x1b[?1016l' + ) + }) + + it('pins the fresh-shell profile', () => { + expect(POST_REPLAY_MODE_RESET).toBe( + '\x1b[0 q\x1b[<99u\x1b[=0u\x1b[?25h\x1b[?9l\x1b[?1000l\x1b[?1002l\x1b[?1003l\x1b[?1006l\x1b[?1016l\x1b[?1004l\x1b[?2004l' + ) + }) + + it('pins the daemon-reattach profile, which keeps bracketed paste', () => { + expect(POST_REPLAY_REATTACH_RESET).toBe( + '\x1b[0 q\x1b[<99u\x1b[=0u\x1b[?25h\x1b[?9l\x1b[?1000l\x1b[?1002l\x1b[?1003l\x1b[?1006l\x1b[?1016l\x1b[?1004l' + ) + expect(POST_REPLAY_REATTACH_RESET).not.toContain('\x1b[?2004l') + }) + + // Why: #12101 — a cold-restored seed re-arms mouse reporting for a dead TUI. + it('disarms mouse reporting on the cold-restore seed', () => { + expect(COLD_RESTORE_SEED_MODE_RESET).toBe(RESET_MOUSE_REPORTING) + }) + + // Why: the seed also feeds the daemon emulator and is re-serialized from it, so + // re-entering alt screen or resetting the cursor there would fight the renderer. + it('keeps the cold-restore seed free of cursor, kitty and alt-screen bytes', () => { + for (const forbidden of ['\x1b[0 q', '\x1b[<99u', '\x1b[?25h', '\x1b[?1049']) { + expect(COLD_RESTORE_SEED_MODE_RESET).not.toContain(forbidden) + } + }) + + // Why byte equality and not just `not.toContain`: a profile that lost every mode + // would satisfy an absence assertion perfectly, so these two — whose only other + // coverage asserts they were passed through unchanged — need a literal here. + it('pins the live-snapshot and live-agent profiles', () => { + expect(POST_REPLAY_LIVE_SNAPSHOT_RESET).toBe('\x1b[0 q\x1b[?25h\x1b[?1004l') + expect(POST_REPLAY_LIVE_AGENT_REATTACH_RESET).toBe('\x1b[0 q\x1b[<99u\x1b[=0u\x1b[?25h') + expect(POST_REPLAY_LIVE_AGENT_SNAPSHOT_RESET).toBe('\x1b[0 q') + }) + + it('leaves a live agent its focus reporting and bracketed paste', () => { + for (const profile of [ + POST_REPLAY_LIVE_AGENT_REATTACH_RESET, + POST_REPLAY_LIVE_AGENT_SNAPSHOT_RESET, + POST_REPLAY_LIVE_SNAPSHOT_RESET + ]) { + expect(profile).not.toContain('\x1b[?1000l') + expect(profile).not.toContain('\x1b[?2004l') + } + expect(POST_REPLAY_LIVE_AGENT_REATTACH_RESET).not.toContain('\x1b[?1004l') + }) + + describe('live-agent cursor preservation', () => { + it('detects a payload that ends cursor-hidden', () => { + expect(replayPayloadEndsWithCursorHidden('a\x1b[?25hb\x1b[?25lc')).toBe(true) + expect(replayPayloadEndsWithCursorHidden('a\x1b[?25lb\x1b[?25hc')).toBe(false) + expect(replayPayloadEndsWithCursorHidden('no modes here')).toBe(false) + }) + + it('omits the cursor-show when the agent left its cursor hidden', () => { + expect(buildPostReplayLiveAgentReattachReset('x\x1b[?25l')).not.toContain('\x1b[?25h') + expect(buildPostReplayLiveAgentReattachReset('x\x1b[?25h')).toContain('\x1b[?25h') + }) + }) +}) diff --git a/src/shared/terminal-mode-reset-profiles.ts b/src/shared/terminal-mode-reset-profiles.ts new file mode 100644 index 000000000..70a01fcd4 --- /dev/null +++ b/src/shared/terminal-mode-reset-profiles.ts @@ -0,0 +1,41 @@ +// Why this module is shared: these profiles describe a terminal-protocol +// contract, not a renderer concern. Both the renderer (replaying a snapshot +// into an xterm) and the daemon (seeding a cold-restored session) must clear +// the same mode bits, and duplicating the literals drifted them apart (#12101). + +// Why: SerializeAddon replays mode bits assuming reattach to a live TUI, but Orca restores against a fresh shell with none, so stale bits (e.g. focus reporting rings the bell on click) must be reset. +export const RESET_TERMINAL_CURSOR_STYLE = '\x1b[0 q' +export const RESET_KITTY_KEYBOARD_PROTOCOL = '\x1b[<99u\x1b[=0u' +// Every mouse mode the daemon can re-arm from a snapshot: protocols 9/1000/1002/1003 + SGR encodings 1006/1016. +export const RESET_MOUSE_REPORTING = + '\x1b[?9l\x1b[?1000l\x1b[?1002l\x1b[?1003l\x1b[?1006l\x1b[?1016l' + +export const POST_REPLAY_MODE_RESET = `${RESET_TERMINAL_CURSOR_STYLE}${RESET_KITTY_KEYBOARD_PROTOCOL}\x1b[?25h${RESET_MOUSE_REPORTING}\x1b[?1004l\x1b[?2004l` + +// Why: same-session live replay; keep cursor/focus cleanup but preserve Kitty flags the running TUI relies on. +export const POST_REPLAY_LIVE_SNAPSHOT_RESET = `${RESET_TERMINAL_CURSOR_STYLE}\x1b[?25h\x1b[?1004l` + +// Why: daemon reattach hits a live session, so skip the full reset; still clear cursor/focus/mouse/Kitty bits harmful to a plain shell after a bad TUI exit — safe for live TUIs since the post-reattach SIGWINCH repaints the cursor. +export const POST_REPLAY_REATTACH_RESET = `${RESET_TERMINAL_CURSOR_STYLE}${RESET_KITTY_KEYBOARD_PROTOCOL}\x1b[?25h${RESET_MOUSE_REPORTING}\x1b[?1004l` + +// Why: a live agent owns focus reporting; resetting ?1004h suppresses the focus-in it needs to re-anchor its cursor (IME). +export const POST_REPLAY_LIVE_AGENT_REATTACH_RESET = `${RESET_TERMINAL_CURSOR_STYLE}${RESET_KITTY_KEYBOARD_PROTOCOL}\x1b[?25h` + +// Why: a live agent owns cursor/focus here; forcing ?25h/?1004l breaks a parked agent that only arms ?1004h at startup. +export const POST_REPLAY_LIVE_AGENT_SNAPSHOT_RESET = RESET_TERMINAL_CURSOR_STYLE + +/** Dead-TUI bytes feed a fresh shell; clear mouse modes here and renderer-owned modes later. */ +export const COLD_RESTORE_SEED_MODE_RESET = RESET_MOUSE_REPORTING + +// Why: DECTCEM applies in emission order, so the payload's last ?25l/?25h is the cursor state the TUI left. +export function replayPayloadEndsWithCursorHidden(payload: string): boolean { + const hideIndex = payload.lastIndexOf('\x1b[?25l') + return hideIndex !== -1 && hideIndex > payload.lastIndexOf('\x1b[?25h') +} + +// Why: some agents hide the real cursor and draw their own, so preserve the payload's final visibility (pty-connection re-shows it if the agent was actually a dead TUI). +export function buildPostReplayLiveAgentReattachReset(payload: string): string { + return replayPayloadEndsWithCursorHidden(payload) + ? `${RESET_TERMINAL_CURSOR_STYLE}${RESET_KITTY_KEYBOARD_PROTOCOL}` + : POST_REPLAY_LIVE_AGENT_REATTACH_RESET +} diff --git a/src/shared/terminal-restore-parity-fixture.ts b/src/shared/terminal-restore-parity-fixture.ts index 884eadb14..b05b450b9 100644 --- a/src/shared/terminal-restore-parity-fixture.ts +++ b/src/shared/terminal-restore-parity-fixture.ts @@ -176,9 +176,7 @@ export function normalBufferStylesTrimmed(terminal: Terminal): string[] { export const SNAPSHOT_REPLAY_PREAMBLE_NORMAL = '\x1b[2J\x1b[3J\x1b[H' export const SNAPSHOT_REPLAY_PREAMBLE_ALT = '\x1b[0m\x1b[?1049h\x1b[2J\x1b[H' -// Twin of POST_REPLAY_LIVE_SNAPSHOT_RESET (layout-serialization.ts) — the -// renderer suite pins equality against the real constant so drift fails fast. -export const POST_REPLAY_LIVE_SNAPSHOT_RESET_PARITY = '\x1b[0 q\x1b[?25h\x1b[?1004l' +export { POST_REPLAY_LIVE_SNAPSHOT_RESET as POST_REPLAY_LIVE_SNAPSHOT_RESET_PARITY } from './terminal-mode-reset-profiles' export type ParityMainSnapshot = { data: string diff --git a/tests/e2e/terminal-attention.spec.ts b/tests/e2e/terminal-attention.spec.ts index 43767d848..22e334f53 100644 --- a/tests/e2e/terminal-attention.spec.ts +++ b/tests/e2e/terminal-attention.spec.ts @@ -12,7 +12,7 @@ import { waitForSessionReady } from './helpers/store' import { getRendererTitleLog, installRendererTitleLog } from './helpers/terminal-title-log' -import { POST_REPLAY_MODE_RESET } from '../../src/renderer/src/components/terminal-pane/layout-serialization' +import { POST_REPLAY_MODE_RESET } from '../../src/shared/terminal-mode-reset-profiles' import { waitForPtyShellEcho } from './terminal-pty-readiness' test.describe.configure({ mode: 'serial' }) @@ -353,7 +353,7 @@ test.describe('Terminal attention', () => { // even though the underlying shell is fresh. Pane clicks then emit // `\e[I` / `\e[O` into zsh, which rings the bell as unbound-key input. // - // POST_REPLAY_MODE_RESET (in layout-serialization.ts) clears these mode + // POST_REPLAY_MODE_RESET (in shared/terminal-mode-reset-profiles.ts) clears these mode // bits after every scrollback replay so the mode state matches the fresh // shell. This test pins that fix: after writing a DECSET 1004 byte into // the terminal, focus events should NOT be emitted back to the PTY.