diff --git a/src/main/daemon/daemon-pty-adapter.ts b/src/main/daemon/daemon-pty-adapter.ts index 56a775a8c99..648870abe1c 100644 --- a/src/main/daemon/daemon-pty-adapter.ts +++ b/src/main/daemon/daemon-pty-adapter.ts @@ -316,7 +316,13 @@ export class DaemonPtyAdapter implements IPtyProvider { snapshotCols: result.snapshot.cols, snapshotRows: result.snapshot.rows, isReattach: true, - isAlternateScreen: isAltScreen + isAlternateScreen: isAltScreen, + // Why: carry the mid-escape tail so the renderer can write it after the + // reattach reset — without it the local daemon reattach path renders a + // split escape's continuation literally, unlike the remote path (#7329). + ...(result.snapshot.pendingEscapeTailAnsi + ? { pendingEscapeTailAnsi: result.snapshot.pendingEscapeTailAnsi } + : {}) } } diff --git a/src/main/daemon/headless-emulator.ts b/src/main/daemon/headless-emulator.ts index fb0a9c190d4..74381d3d543 100644 --- a/src/main/daemon/headless-emulator.ts +++ b/src/main/daemon/headless-emulator.ts @@ -3,6 +3,7 @@ import { Terminal } from '@xterm/headless' import { SerializeAddon } from '@xterm/addon-serialize' import { Unicode11Addon } from '@xterm/addon-unicode11' import { activateOrcaTerminalUnicodeProvider } from '../../shared/terminal-unicode-provider' +import { advancePartialEscapeTail } from '../../shared/terminal-partial-escape-tail' import { extractLastOscTitle } from '../../shared/agent-detection' import { collectHeadlessOscLinkRanges } from './headless-osc-link-ranges' import { extractOscScanTail, scanOsc7Uris } from './osc7-uri-extraction' @@ -36,6 +37,13 @@ export class HeadlessEmulator { private oscScanTail = '' private privateModes = new TerminalPrivateModeTracker() private restoredOscLinks: TerminalOscLinkRange[] = [] + // Why: a PTY read can end mid-escape-sequence — those bytes live in xterm's + // parser, not the screen buffer, so serialize() drops them and the next + // chunk's continuation renders literally after a remote snapshot restore + // (#7329). Track the unparsed trailing partial at ingest (committed after + // xterm parses the same bytes, like the private-mode mirror) and ship it in + // the snapshot so the restorer can complete the sequence. + private partialEscapeTail = '' private disposed = false private readonly pathFlavor?: 'posix' | 'win32' private readonly remotePosixFileUriAuthority: boolean @@ -89,6 +97,7 @@ export class HeadlessEmulator { // Why: snapshots combine serialized xterm state with mirrored mouse // modes. Commit the mirror only after xterm has parsed the same bytes. this.privateModes.scan(data) + this.partialEscapeTail = advancePartialEscapeTail(this.partialEscapeTail, data) resolve() }) }) @@ -115,6 +124,7 @@ export class HeadlessEmulator { // PTY bursts; queued headless writes can snapshot half-cleared TUI rows. writeSync.call((this.terminal as TerminalWithSynchronousWrite)._core, data) this.privateModes.scan(data) + this.partialEscapeTail = advancePartialEscapeTail(this.partialEscapeTail, data) return true } @@ -164,7 +174,13 @@ export class HeadlessEmulator { cols: this.terminal.cols, rows: this.terminal.rows, scrollbackLines: this.terminal.buffer.normal.length - this.terminal.rows, - lastTitle: this.lastTitle ?? undefined + lastTitle: this.lastTitle ?? undefined, + // Why: written LAST by the restorer (after any reset) so the next live + // chunk completes this dangling sequence instead of rendering it literally + // (#7329). Its bytes are already counted by the snapshot seq. + ...(this.partialEscapeTail.length > 0 + ? { pendingEscapeTailAnsi: this.partialEscapeTail } + : {}) } } diff --git a/src/main/daemon/repro-7329-remote-snapshot-corruption.test.ts b/src/main/daemon/repro-7329-remote-snapshot-corruption.test.ts new file mode 100644 index 00000000000..34a0e095ee1 --- /dev/null +++ b/src/main/daemon/repro-7329-remote-snapshot-corruption.test.ts @@ -0,0 +1,157 @@ +import { describe, expect, it } from 'vitest' +import { Terminal } from '@xterm/headless' +import { HeadlessEmulator } from './headless-emulator' + +// Repro for #7329: "remote server + terminal" — typing gets escape sequences +// injected/wrapped around it and follow-up commands are corrupted. +// +// The remote-server path serializes terminal state on the daemon via +// HeadlessEmulator.getSnapshot() (SerializeAddon + buildRehydrateSequences) and +// replays it into the renderer xterm, then applies POST_REPLAY_REATTACH_RESET. +// 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' +// 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\x1b[?1004l` + +function writeXterm(term: Terminal, data: string): Promise { + return new Promise((resolve) => term.write(data, resolve)) +} + +async function emulatorWrite(emu: HeadlessEmulator, data: string): Promise { + await emu.write(data) +} + +/** Replay a daemon snapshot into a renderer xterm exactly like the remote + * onSnapshot → replayDataCallback path: clear, write rehydrate+snapshot, + * then the reattach mode reset. */ +async function replayRemoteSnapshot( + term: Terminal, + snapshot: { rehydrateSequences: string; snapshotAnsi: string; pendingEscapeTailAnsi?: string } +): Promise { + await writeXterm(term, '\x1b[2J\x1b[3J\x1b[H') + await writeXterm(term, snapshot.rehydrateSequences + snapshot.snapshotAnsi) + await writeXterm(term, POST_REPLAY_REATTACH_RESET) + // The fix: the restorer writes the pending mid-escape tail LAST, after the + // reset (mirrors drainReplayDataQueue in pty-connection.ts). + if (snapshot.pendingEscapeTailAnsi) { + await writeXterm(term, snapshot.pendingEscapeTailAnsi) + } +} + +function renderVisible(term: Terminal): string { + const buf = term.buffer.active + const lines: string[] = [] + for (let y = 0; y < term.rows; y += 1) { + lines.push(buf.getLine(buf.viewportY + y)?.translateToString(true) ?? '') + } + return lines.join('\n').replace(/\s+$/g, '') +} + +describe('#7329 remote-server snapshot corruption', () => { + it('leaves bracketed-paste + mouse modes armed after a reattach snapshot', async () => { + const emu = new HeadlessEmulator({ cols: 80, rows: 24 }) + const term = new Terminal({ cols: 80, rows: 24, allowProposedApi: true }) + try { + // A live remote shell that armed bracketed paste (bash 4.4+/readline + // default) and vt200+SGR mouse (a TUI the user just exited). + await emulatorWrite(emu, '\x1b[?2004h\x1b[?1000h\x1b[?1006h') + await emulatorWrite(emu, 'user@host:~$ ') + + const snapshot = emu.getSnapshot({ scrollbackRows: 0 }) + // The daemon snapshot re-arms the modes it observed. + expect(snapshot.rehydrateSequences).toContain('\x1b[?2004h') + expect(snapshot.modes.bracketedPaste).toBe(true) + expect(snapshot.modes.mouseTracking).toBe(true) + + await replayRemoteSnapshot(term, snapshot) + + // After the remote reattach reset, the renderer xterm is STILL in + // bracketed-paste + mouse mode — even though POST_REPLAY_REATTACH_RESET + // ran. Cold restore's POST_REPLAY_MODE_RESET would have cleared these. + expect(term.modes.bracketedPasteMode).toBe(true) // leak + expect(term.modes.mouseTrackingMode).not.toBe('none') // leak + } finally { + emu.dispose() + term.dispose() + } + }) + + it('drops a mid-escape tail so continuation bytes render literally after reattach', async () => { + const emu = new HeadlessEmulator({ cols: 80, rows: 24 }) + const term = new Terminal({ cols: 80, rows: 24, allowProposedApi: true }) + try { + // A remote PTY read that ends mid-escape: the shell was about to paint a + // colored prompt but the read boundary split the SGR sequence. The next + // read carries the continuation. The daemon serializes BETWEEN the two + // reads (e.g. the client subscribes/reattaches right then). + await emulatorWrite(emu, 'first line\r\n') + await emulatorWrite(emu, '\x1b[3') // <-- partial SGR: "\x1b[38;5;...m" started + + const snapshot = emu.getSnapshot({ scrollbackRows: 0 }) + // The serializer still cannot put the dangling "\x1b[3" in the screen ANSI + // (it lives in the parser)... + expect(snapshot.snapshotAnsi).not.toContain('\x1b[3') + // ...but the emulator now ships it as a separate pending-escape tail. + expect(snapshot.pendingEscapeTailAnsi).toBe('\x1b[3') + + await replayRemoteSnapshot(term, snapshot) + + // The continuation of the split escape arrives as the next live chunk. + await writeXterm(term, '8;5;196mred$ yes\r\n') + + const visible = renderVisible(term) + // FIXED: the tail was replayed after the reset, so the continuation + // "8;5;196m" completes the SGR escape and is consumed — not rendered + // literally. The visible text is just the prompt + typed command. + expect(visible).not.toContain('8;5;196m') + expect(visible).toContain('red$ yes') + } finally { + emu.dispose() + term.dispose() + } + }) + + it('does not eat the continuation byte when the tail and continuation are contiguous', async () => { + // The safety property the fix relies on (mirrors the snapshot-seq accounting + // in orca-runtime/getOutputAfterSnapshotSeq): the snapshot seq counts the + // tail bytes, so the FIRST live chunk after the snapshot is the exact + // continuation and completes the dangling sequence with no eaten byte. + const emu = new HeadlessEmulator({ cols: 80, rows: 24 }) + const term = new Terminal({ cols: 80, rows: 24, allowProposedApi: true }) + try { + await emulatorWrite(emu, '\x1b[38;5;') // dangling: params so far + const snapshot = emu.getSnapshot({ scrollbackRows: 0 }) + expect(snapshot.pendingEscapeTailAnsi).toBe('\x1b[38;5;') + await replayRemoteSnapshot(term, snapshot) + // Continuation completes the SGR then prints a visible token. + await writeXterm(term, '82mHELLO') + const visible = renderVisible(term) + // No byte of "82m" leaks; the token renders whole (color applied). + expect(visible).toBe('HELLO') + } finally { + emu.dispose() + term.dispose() + } + }) + + it('documents the residual edge: an idle-death tail eats the next output byte', async () => { + // KNOWN, ACCEPTED trade-off: if a mid-escape read is the LAST thing the + // process emits (it dies/idles and never sends the continuation), the tail + // sits armed in the parser and the NEXT unrelated output loses its first + // byte. This is strictly narrower than the bug it fixes (which garbled every + // real split escape), and pre-fix such a stream simply dropped the partial. + const term = new Terminal({ cols: 80, rows: 24, allowProposedApi: true }) + try { + await writeXterm(term, 'prompt$ ') + await writeXterm(term, '\x1b[3') // armed tail, no continuation ever comes + await writeXterm(term, 'yes\r\n') // unrelated later output + // The 'y' is absorbed as a CSI parameter of the dangling sequence. + expect(renderVisible(term)).toBe('prompt$ es') + } finally { + term.dispose() + } + }) +}) diff --git a/src/main/daemon/types.ts b/src/main/daemon/types.ts index a38cc21a425..10d27f2c8e0 100644 --- a/src/main/daemon/types.ts +++ b/src/main/daemon/types.ts @@ -25,6 +25,12 @@ export type TerminalSnapshot = { scrollbackAnsi: string oscLinks?: TerminalOscLinkRange[] rehydrateSequences: string + /** The trailing partial escape sequence left unparsed in the emulator when a + * PTY read ended mid-escape. serialize() cannot carry it (it lives in the + * parser, not the buffer), so the restorer must write it LAST — after any + * post-snapshot reset — so the next live chunk's continuation completes the + * sequence instead of rendering literally (#7329). */ + pendingEscapeTailAnsi?: string cwd: string | null modes: TerminalModes cols: number diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 79739fd27ac..f63b43719f0 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -2614,6 +2614,7 @@ export function registerPtyHandlers( seq?: number source?: 'headless' | 'renderer' alternateScreen?: boolean + pendingEscapeTailAnsi?: string } | null> => { if (!runtime || typeof args?.id !== 'string' || args.id.length === 0) { return null diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 52d248170ca..ba2b4d946aa 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -5707,6 +5707,7 @@ export class OrcaRuntimeService { source?: 'headless' | 'renderer' oscLinks?: TerminalOscLinkRange[] alternateScreen?: boolean + pendingEscapeTailAnsi?: string } | null> { return this.serializeTerminalBufferFromAvailableState(ptyId, opts) } @@ -5741,6 +5742,7 @@ export class OrcaRuntimeService { source?: 'headless' | 'renderer' oscLinks?: TerminalOscLinkRange[] alternateScreen?: boolean + pendingEscapeTailAnsi?: string } | null> { const headlessSnapshot = await this.serializeHeadlessTerminalBuffer(ptyId, { ...opts, @@ -6021,6 +6023,7 @@ export class OrcaRuntimeService { source?: 'headless' | 'renderer' oscLinks?: TerminalOscLinkRange[] alternateScreen?: boolean + pendingEscapeTailAnsi?: string } | null> { const headlessSnapshot = await this.serializeHeadlessTerminalBuffer(ptyId, opts) if (headlessSnapshot) { @@ -6138,6 +6141,10 @@ export class OrcaRuntimeService { source?: 'headless' oscLinks?: TerminalOscLinkRange[] alternateScreen?: boolean + // Why: dangling mid-escape tail the restorer must write LAST, after any + // reset, so the next live chunk completes it instead of rendering it + // literally (#7329). + pendingEscapeTailAnsi?: string } | null> { const state = this.headlessTerminals.get(ptyId) if (!state) { @@ -6166,6 +6173,9 @@ export class OrcaRuntimeService { seq: state.outputSequence, source: 'headless', oscLinks: snapshot.oscLinks, + ...(snapshot.pendingEscapeTailAnsi + ? { pendingEscapeTailAnsi: snapshot.pendingEscapeTailAnsi } + : {}), // Why: lets the renderer skip the destructive scrollback clear when // restoring an alt-screen snapshot — clearing wipes xterm's own // history that the TUI relies on for scroll-up after a tab return. diff --git a/src/main/runtime/rpc/methods/terminal.ts b/src/main/runtime/rpc/methods/terminal.ts index 48b1bc793b2..4ebe781495f 100644 --- a/src/main/runtime/rpc/methods/terminal.ts +++ b/src/main/runtime/rpc/methods/terminal.ts @@ -55,6 +55,7 @@ type SnapshotFrameOptions = { truncatedByByteBudget?: boolean source?: 'headless' | 'renderer' oscLinks?: TerminalOscLinkRange[] + pendingEscapeTailAnsi?: string } type SerializedSnapshot = { @@ -67,6 +68,7 @@ type SerializedSnapshot = { oscLinks?: TerminalOscLinkRange[] scrollbackRows: number truncatedByByteBudget: boolean + pendingEscapeTailAnsi?: string } | null type TerminalViewportClient = { @@ -466,6 +468,7 @@ function sendSnapshotFrames( cwd: options.cwd, source: options.source, oscLinks: options.oscLinks, + pendingEscapeTailAnsi: options.pendingEscapeTailAnsi, truncated: options.truncated === true, truncatedByByteBudget: options.truncatedByByteBudget === true }) @@ -1398,6 +1401,7 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ cwd: serialized?.cwd, source: serialized?.source, oscLinks: serialized?.oscLinks, + pendingEscapeTailAnsi: serialized?.pendingEscapeTailAnsi, truncated: false, truncatedByByteBudget: serialized?.truncatedByByteBudget, data: serialized?.data ?? '' @@ -1602,6 +1606,7 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ truncatedByByteBudget: serialized?.truncatedByByteBudget, source: serialized?.source, oscLinks: serialized?.oscLinks, + pendingEscapeTailAnsi: serialized?.pendingEscapeTailAnsi, data: serialized?.data ?? (read.tail.length > 0 ? `${read.tail.join('\r\n')}\r\n` : '') }) // Why: baseline for resize re-stream gating; the client already diff --git a/src/main/runtime/rpc/terminal-multiplex-escape-tail.test.ts b/src/main/runtime/rpc/terminal-multiplex-escape-tail.test.ts new file mode 100644 index 00000000000..ab255856216 --- /dev/null +++ b/src/main/runtime/rpc/terminal-multiplex-escape-tail.test.ts @@ -0,0 +1,125 @@ +import { describe, expect, it, vi } from 'vitest' +import { RpcDispatcher } from './dispatcher' +import type { RpcRequest } from './core' +import type { OrcaRuntimeService } from '../orca-runtime' +import { TERMINAL_METHODS } from './methods/terminal' +import type { RuntimeTerminalWait } from '../../../shared/runtime-types' +import { + TerminalStreamOpcode, + decodeTerminalStreamFrame, + decodeTerminalStreamJson, + encodeTerminalStreamFrame, + encodeTerminalStreamJson +} from '../../../shared/terminal-stream-protocol' + +// Transport-level regression for #7329: the daemon's mid-escape tail +// (pendingEscapeTailAnsi) must survive the terminal.multiplex wire so the +// renderer can replay it after the reset. Without threading it through the +// SnapshotStart JSON frame, the tail is lost and the next live chunk renders +// literally ("colors/garbage around what I type"). + +function stubRuntime(overrides: Partial = {}): OrcaRuntimeService { + return { + getRuntimeId: () => 'test-runtime', + ...overrides + } as OrcaRuntimeService +} + +function makeRequest(method: string, params?: unknown): RpcRequest { + return { id: 'req-1', authToken: 'tok', method, params } +} + +describe('terminal.multiplex pending-escape-tail threading (#7329)', () => { + it('carries the daemon pendingEscapeTailAnsi through the SnapshotStart frame', async () => { + vi.useFakeTimers() + try { + const messages: string[] = [] + const binaryFrames: Uint8Array[] = [] + const handlers = new Map< + number, + (frame: NonNullable>) => void + >() + const cleanups = new Map void>() + const runtime = stubRuntime({ + resolveLeafForHandle: vi.fn().mockReturnValue({ ptyId: 'pty-1' }), + readTerminal: vi.fn().mockResolvedValue({ tail: [], truncated: false }), + serializeTerminalBuffer: vi.fn().mockResolvedValue({ + data: 'user@host:~$ ', + cols: 80, + rows: 24, + // The dangling partial the emulator could not serialize. + pendingEscapeTailAnsi: '\x1b[3' + }), + getTerminalSize: vi.fn().mockReturnValue({ cols: 80, rows: 24 }), + getMobileDisplayMode: vi.fn().mockReturnValue('auto'), + getLayout: vi.fn().mockReturnValue({ seq: 1 }), + subscribeToTerminalData: vi.fn().mockReturnValue(vi.fn()), + subscribeToTerminalResize: vi.fn().mockReturnValue(vi.fn()), + subscribeToFitOverrideChanges: vi.fn().mockReturnValue(vi.fn()), + subscribeToDriverChanges: vi.fn().mockReturnValue(vi.fn()), + getTerminalFitOverride: vi.fn().mockReturnValue(null), + getDriver: vi.fn().mockReturnValue({ kind: 'idle' }), + registerSubscriptionCleanup: vi.fn((id: string, cleanup: () => void) => { + cleanups.set(id, cleanup) + }), + cleanupSubscription: vi.fn((id: string) => { + const cleanup = cleanups.get(id) + cleanups.delete(id) + cleanup?.() + }), + waitForTerminal: vi.fn(() => new Promise(() => {})), + updateDesktopViewport: vi.fn().mockResolvedValue(true) + }) + const dispatcher = new RpcDispatcher({ runtime, methods: TERMINAL_METHODS }) + + const dispatchPromise = dispatcher.dispatchStreaming( + makeRequest('terminal.multiplex', {}), + (msg) => messages.push(msg), + { + connectionId: 'conn-1', + sendBinary: (bytes) => binaryFrames.push(bytes), + registerBinaryStreamHandler: (streamId, handler) => { + handlers.set(streamId, handler) + return () => handlers.delete(streamId) + } + } + ) + + await vi.runOnlyPendingTimersAsync() + expect(messages.some((msg) => JSON.parse(msg).result?.type === 'ready')).toBe(true) + + handlers.get(0)?.( + decodeTerminalStreamFrame( + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.Subscribe, + streamId: 0, + seq: 1, + payload: encodeTerminalStreamJson({ + streamId: 5, + terminal: 'terminal-1', + client: { id: 'desktop-1', type: 'desktop' } + }) + }) + )! + ) + + // Drive the subscribe promise chain (readTerminal/serialize awaits + the + // output-batcher flush timer) to completion. + for (let i = 0; i < 5; i += 1) { + await vi.runOnlyPendingTimersAsync() + } + + const snapshotStart = binaryFrames + .map((frame) => decodeTerminalStreamFrame(frame)) + .find((frame) => frame?.opcode === TerminalStreamOpcode.SnapshotStart)! + expect(decodeTerminalStreamJson(snapshotStart.payload)).toMatchObject({ + pendingEscapeTailAnsi: '\x1b[3' + }) + + runtime.cleanupSubscription('terminal-multiplex:conn-1') + await dispatchPromise + } finally { + vi.useRealTimers() + } + }) +}) diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 184e0aea648..10687c01a29 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -1174,6 +1174,7 @@ export type PreloadApi = { seq?: number source?: 'headless' | 'renderer' alternateScreen?: boolean + pendingEscapeTailAnsi?: string } | null> getRendererDeliveryDebugSnapshot: () => Promise<{ pendingPtyCount: number diff --git a/src/preload/index.ts b/src/preload/index.ts index f0ef9cf7e34..4a24404831a 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -850,6 +850,7 @@ const api = { seq?: number source?: 'headless' | 'renderer' alternateScreen?: boolean + pendingEscapeTailAnsi?: string } | null> => ipcRenderer.invoke('pty:getMainBufferSnapshot', { id, opts }), getRendererDeliveryDebugSnapshot: (): Promise<{ 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 78edfeeb596..9b43d897171 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -205,6 +205,7 @@ type MockTransport = { ) => unknown } sendInput: ReturnType + sendInputImmediate: ReturnType sendInputAccepted?: ReturnType resize: ReturnType getPtyId: ReturnType @@ -353,6 +354,9 @@ function createMockTransport(initialPtyId: string | null = null): MockTransport serializeBuffer: undefined } as MockTransport const sendInput = transport.sendInput as unknown as (data: string) => boolean + // Why: query replies now route through sendInputImmediate; delegate to the + // same spy so assertions on reply delivery still observe them (#7329). + transport.sendInputImmediate = vi.fn((data: string) => sendInput(data)) transport.sendInputAccepted = vi.fn(async (data: string) => sendInput(data)) return transport } @@ -5247,6 +5251,189 @@ describe('connectPanePty', () => { ) }) + it('resizes the pane to the snapshot grid before replaying daemon snapshot bytes (bug #7279)', async () => { + // Why: the daemon serializes soft-wrapped lines as continuous text. Replaying + // that at the pane's current column count rewraps rows one cell early/late. + // The reattach path must resize xterm to the snapshot's grid before writing + // the snapshot bytes, so a remote pane whose size drifted from the daemon's + // grid still repaints the exact host layout. + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + transport.connect.mockImplementation(async ({ sessionId }: { sessionId?: string }) => { + if (sessionId) { + return { + id: sessionId, + snapshot: '\x1b[?1004hrestored snapshot', + snapshotCols: 80, + snapshotRows: 24 + } + } + return null + }) + transportFactoryQueue.push(transport) + mockStoreState = { + ...mockStoreState, + tabsByWorktree: { + 'wt-1': [{ id: 'tab-1', ptyId: 'tab-pty' }] + } + } as StoreState + + // createPane opens the pane at 120x40, deliberately different from the + // daemon snapshot's 80x24 grid. + const pane = createPane(1) + 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) + + // Pane was resized to the snapshot grid before the snapshot bytes landed. + expect(pane.terminal.resize).toHaveBeenCalledWith(80, 24) + const resizeToSnapshotCall = pane.terminal.resize.mock.invocationCallOrder.find( + (_order, index) => { + const [cols, rows] = pane.terminal.resize.mock.calls[index] + return cols === 80 && rows === 24 + } + ) + const snapshotWriteCall = pane.terminal.write.mock.invocationCallOrder.find( + (_order, index) => pane.terminal.write.mock.calls[index][0] === '\x1b[?1004hrestored snapshot' + ) + expect(resizeToSnapshotCall).toBeDefined() + expect(snapshotWriteCall).toBeDefined() + expect(resizeToSnapshotCall as number).toBeLessThan(snapshotWriteCall as number) + }) + + it('writes the daemon pendingEscapeTailAnsi after the reset on local reattach (#7329)', async () => { + // Why: the mid-escape tail must be re-armed LAST — after the reattach reset, + // whose ESC would abort it — so the racing live continuation completes it + // instead of rendering literally. Covers the local daemon reattach path, + // which previously dropped the field the remote path already honored. + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + transport.connect.mockImplementation(async ({ sessionId }: { sessionId?: string }) => { + if (sessionId) { + return { + id: sessionId, + snapshot: 'restored snapshot', + snapshotCols: 80, + snapshotRows: 24, + pendingEscapeTailAnsi: '\x1b[3' + } + } + return null + }) + transportFactoryQueue.push(transport) + mockStoreState = { + ...mockStoreState, + tabsByWorktree: { 'wt-1': [{ id: 'tab-1', ptyId: 'tab-pty' }] } + } as StoreState + + const pane = createPane(1) + 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 tailWriteCall = pane.terminal.write.mock.invocationCallOrder.find( + (_order, index) => pane.terminal.write.mock.calls[index][0] === '\x1b[3' + ) + const resetWriteCall = pane.terminal.write.mock.invocationCallOrder.find((_order, index) => + String(pane.terminal.write.mock.calls[index][0]).includes(POST_REPLAY_REATTACH_RESET) + ) + expect(tailWriteCall).toBeDefined() + expect(resetWriteCall).toBeDefined() + // The dangling tail is written AFTER the reset. + expect(resetWriteCall as number).toBeLessThan(tailWriteCall as number) + }) + + it('routes native onData query replies through sendInputImmediate, typed input through sendInput (#7329)', async () => { + // Why this test: the mock transport delegates sendInputImmediate to the + // sendInput spy, so reply-delivery assertions elsewhere cannot tell the two + // apart — reverting the onData isTerminalQueryReply branch used to pass the + // whole suite. This pins the routing decision itself. + const { connectPanePty } = await import('./pty-connection') + enableActiveRuntimeEnvironment() + const pane = createPane(1) + const transport = createMockTransport('remote:web-env-1@@pty-7329') + transportFactoryQueue.push(transport) + const manager = createManager(1, 1) + const deps = createDeps() + + connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks() + + // xterm answers CSI 6n natively by emitting a CPR through onData, mixed + // with keystrokes. It must take the immediate path (skips the remote 8ms + // input debounce that corrupted it). + sendTerminalInputThroughPane(pane, '\x1b[3;1R') + expect(transport.sendInputImmediate).toHaveBeenCalledWith('\x1b[3;1R') + + // Ordinary typed input must stay on the debounced path — never immediate. + transport.sendInputImmediate.mockClear() + sendTerminalInputThroughPane(pane, 'yes') + sendTerminalInputThroughPane(pane, '\x1b[A') // arrow-key auto-repeat stays batched + expect(transport.sendInput).toHaveBeenCalledWith('yes') + expect(transport.sendInput).toHaveBeenCalledWith('\x1b[A') + expect(transport.sendInputImmediate).not.toHaveBeenCalled() + }) + + it('writes the onReplayData pendingEscapeTailAnsi meta last, after the replayed bytes (#7329)', async () => { + // Why this test: the remote snapshot path delivers the daemon tail through + // transport callbacks.onReplayData meta into drainReplayDataQueue. That + // consumer (and the replayDataCallback meta threading before it) had no + // failing test — severing the meta pass-through kept the suite green. + const { connectPanePty } = await import('./pty-connection') + enableActiveRuntimeEnvironment() + const pane = createPane(1) + const writes: string[] = [] + pane.terminal.write = vi.fn((data: string, callback?: () => void) => { + writes.push(data) + callback?.() + }) as typeof pane.terminal.write + const transport = createMockTransport('remote:web-env-1@@pty-7329-tail') + const replayCallback: { + current: + | (( + data: string, + meta?: { clearBeforeReplay?: boolean; pendingEscapeTailAnsi?: string } + ) => void) + | null + } = { current: null } + transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + replayCallback.current = callbacks.onReplayData ?? null + return { id: 'remote:web-env-1@@pty-7329-tail', replay: '' } + }) + transportFactoryQueue.push(transport) + const manager = createManager(1, 1) + const deps = createDeps() + + connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks(20) + expect(replayCallback.current).toBeTypeOf('function') + + replayCallback.current?.('remote snapshot bytes', { + clearBeforeReplay: false, + pendingEscapeTailAnsi: '\x1b[3' + }) + await flushAsyncTicks(20) + + const snapshotIndex = writes.indexOf('remote snapshot bytes') + const tailIndex = writes.lastIndexOf('\x1b[3') + expect(snapshotIndex).toBeGreaterThanOrEqual(0) + expect(tailIndex).toBeGreaterThanOrEqual(0) + // The dangling tail is re-armed after the snapshot (and any reset), so the + // next live chunk's continuation completes it instead of rendering literally. + expect(tailIndex).toBeGreaterThan(snapshotIndex) + expect(writes.slice(tailIndex + 1)).toEqual([]) + }) + it('preserves live modes and injects focus-in after focused agent reattach', async () => { const { connectPanePty } = await import('./pty-connection') const transport = createMockTransport() diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 0e7da9d9493..2a6a968bf2f 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -11,6 +11,7 @@ import { getWorktreeMapFromState } from '@/store/selectors' import { parseWorkspaceKey } from '../../../../shared/workspace-scope' import { createTerminalZeroDimensionsMessage } from '../../../../shared/terminal-zero-dimensions-diagnostic' import { parseTerminalOscColorQuery } from '../../../../shared/terminal-osc-color-reply' +import { isTerminalQueryReply } from '../../../../shared/terminal-query-reply' import type { PtyBufferSnapshot, PtyConnectResult } from './pty-transport' import { createIpcPtyTransport } from './pty-transport' import { createRemoteRuntimePtyTransport } from './remote-runtime-pty-transport' @@ -2847,13 +2848,16 @@ export function connectPanePty( const terminalCapabilityRepliesDisposable = installTerminalCapabilityReplyHandlers({ terminal: pane.terminal, parser: pane.terminal.parser, - sendInput: (data) => transport.sendInput(data), + // Why: OSC 10/11 + DA1 replies must beat the querying program's raw-mode + // read window; the remote transport's input debounce would corrupt them + // (#7329), so send immediately. + sendInput: (data) => transport.sendInputImmediate(data), isReplaying: () => isPaneReplaying(deps.replayingPanesRef, pane.id), ...(isNativeWindowsConpty ? { da1Response: CONPTY_DA1_RESPONSE } : {}) }) const respondToTerminalPixelSizeQueries = createTerminalPixelSizeQueryResponder( pane.terminal, - (data) => transport.sendInput(data) + (data) => transport.sendInputImmediate(data) ) const onDataDisposable = pane.terminal.onData((data) => { @@ -2891,6 +2895,19 @@ export function connectPanePty( clearPendingTerminalInputIntent() return } + // Why: xterm answers CPR/DSR/DA queries natively through this same onData + // stream (mixed with keystrokes). Those replies are latency-critical — a + // querying program reads them in raw mode with a short timeout — so send + // them immediately, skipping the remote input debounce that would corrupt + // them (#7329). They are not user input, so they bypass intent inference and + // activity recording below. No pending-intent guard: the only intents are + // plain-escape (`\x1b`) and ctrl-c (`\x03`), neither of which can satisfy + // isTerminalQueryReply (it requires length >= 3 and a full reply grammar), + // so a real keystroke never reaches this branch. + if (isTerminalQueryReply(data)) { + transport.sendInputImmediate(data) + return + } const intent = pendingTerminalInputIntent // Why: real xterm can deliver the terminal byte even when our DOM keydown // listener missed the press. Exact Ctrl+C/Escape bytes are still safe to @@ -4040,13 +4057,14 @@ export function connectPanePty( type PendingReplayData = { data: string clearBeforeReplay: boolean + pendingEscapeTailAnsi?: string } let pendingReplayData: PendingReplayData | null = null let replayDrainQueued = false const drainReplayDataQueue = async (): Promise => { while (pendingReplayData !== null) { - const { data, clearBeforeReplay } = pendingReplayData + const { data, clearBeforeReplay, pendingEscapeTailAnsi } = pendingReplayData pendingReplayData = null // Relay replay buffers may overlap with content already rendered in // xterm. Local eager replay decides this earlier so metadata-only frames @@ -4064,6 +4082,14 @@ export function connectPanePty( await writeReplayDataAsync(reattachReplayResetSequence()) sendFocusedReattachFocusInAfterReplay() } + // Why: the daemon could not serialize a PTY read that ended mid-escape, + // so the emulator shipped the dangling partial separately. Write it LAST + // — after the reset, whose ESC would otherwise abort it — so the next + // live chunk completes the sequence instead of rendering literally + // (#7329). Guarded so a later ESC cannot leave the parser wedged. + if (pendingEscapeTailAnsi) { + await writeReplayDataAsync(pendingEscapeTailAnsi) + } if (disposed) { pendingReplayData = null return @@ -4074,10 +4100,14 @@ export function connectPanePty( manager.rebuildPaneWebgl(pane.id) } } - const replayDataCallback = (data: string, meta: { clearBeforeReplay?: boolean } = {}): void => { + const replayDataCallback = ( + data: string, + meta: { clearBeforeReplay?: boolean; pendingEscapeTailAnsi?: string } = {} + ): void => { pendingReplayData = { data, - clearBeforeReplay: meta.clearBeforeReplay !== false + clearBeforeReplay: meta.clearBeforeReplay !== false, + ...(meta.pendingEscapeTailAnsi ? { pendingEscapeTailAnsi: meta.pendingEscapeTailAnsi } : {}) } if (replayDrainQueued) { return @@ -4090,7 +4120,10 @@ export function connectPanePty( replayDrainQueued = false if (pendingReplayData !== null) { replayDataCallback(pendingReplayData.data, { - clearBeforeReplay: pendingReplayData.clearBeforeReplay + clearBeforeReplay: pendingReplayData.clearBeforeReplay, + ...(pendingReplayData.pendingEscapeTailAnsi + ? { pendingEscapeTailAnsi: pendingReplayData.pendingEscapeTailAnsi } + : {}) }) } }) @@ -4182,7 +4215,9 @@ export function connectPanePty( // mode 2031 out-of-band so TUIs still render the snapshot with the same // theme-dependent styling they would have used in a visible pane. deps.paneMode2031Ref.current.set(pane.id, true) - transport.sendInput(mode2031SequenceFor(mode)) + // Why: a query reply — send immediately so the remote input debounce + // cannot delay it past the querying program's read window (#7329). + transport.sendInputImmediate(mode2031SequenceFor(mode)) deps.paneLastThemeModeRef.current.set(pane.id, mode) recordHiddenMode2031Reply() } @@ -4472,9 +4507,10 @@ export function connectPanePty( hiddenStartupRendererQueryPending = extracted.pending if (extracted.oscColorQueryData) { // Why: Codex's startup palette probe has a 100 ms budget. Answer - // hidden color queries directly so renderer scheduling cannot miss it. + // hidden color queries directly and immediately so neither renderer + // scheduling nor the remote input debounce (#7329) can miss it. sendTerminalOscColorQueryReplies(extracted.oscColorQueryData, pane.terminal, (reply) => - transport.sendInput(reply) + transport.sendInputImmediate(reply) ) } if (extracted.statelessQueryData) { @@ -4949,6 +4985,7 @@ export function connectPanePty( rows: number seq?: number alternateScreen?: boolean + pendingEscapeTailAnsi?: string }): void { const scrollIntent = captureTerminalWriteScrollIntent(pane.terminal) const colsBeforeReplay = pane.terminal.cols @@ -4989,6 +5026,13 @@ export function connectPanePty( } writeReplayData(snapshot.data) writeReplayData(POST_REPLAY_LIVE_SNAPSHOT_RESET) + if (snapshot.pendingEscapeTailAnsi) { + // Why last: the snapshot was serialized while the emulator sat mid-escape; + // re-arm the dangling sequence as the FINAL replay write (any later ESC — + // including the reset above — would abort it) so the racing live tail's + // continuation completes it instead of rendering literally (#7329). + writeReplayData(snapshot.pendingEscapeTailAnsi) + } hiddenRendererStateDirty = false recordRendererOrderedSeq(snapshot) resetHiddenRendererRiskState() @@ -5250,7 +5294,9 @@ export function connectPanePty( sendTerminalOscColorQueryReplies( pendingForegroundQuery.oscColorQueryData, pane.terminal, - (reply) => transport.sendInput(reply) + // Why: OSC color reply — immediate so the remote debounce cannot delay + // it past the querying program's read window (#7329). + (reply) => transport.sendInputImmediate(reply) ) } const restoreAppliesToCurrentPty = @@ -5381,12 +5427,45 @@ export function connectPanePty( // and only the freshest source belongs on screen. if (connectResult?.snapshot) { rememberReattachPayloadAgentSignal(connectResult.snapshot, { fullScreenReplay: true }) + // Why: the daemon serializes its grid with soft-wrapped lines as + // continuous text. Replaying that at a different column count rewraps + // rows one cell early/late (bug #7279). Replay at the snapshot's own + // dimensions first; safeFit below fits the pane back and resizes the + // remote PTY. Suppress the xterm->PTY forward so this layout-only + // resize does not SIGWINCH the live remote TUI. Mirrors + // applyMainBufferSnapshot. + const snapshotCols = connectResult.snapshotCols + const snapshotRows = connectResult.snapshotRows + const hasSnapshotDimensions = + typeof snapshotCols === 'number' && + typeof snapshotRows === 'number' && + Number.isFinite(snapshotCols) && + Number.isFinite(snapshotRows) && + snapshotCols > 0 && + snapshotRows > 0 + if ( + hasSnapshotDimensions && + (pane.terminal.cols !== snapshotCols || pane.terminal.rows !== snapshotRows) + ) { + suppressSnapshotReplayPtyResize = true + try { + pane.terminal.resize(snapshotCols, snapshotRows) + } finally { + suppressSnapshotReplayPtyResize = false + } + } writeReplayData('\x1b[2J\x1b[3J\x1b[H') writeReplayData(connectResult.snapshot) // Snapshot reattach keeps a live session, so avoid the broader mode // reset. We only drop renderer-owned state that should not leak from // replay bytes into the restored renderer terminal. writeReplayData(reattachReplayResetSequence()) + if (connectResult.pendingEscapeTailAnsi) { + // Why last: re-arm the daemon's dangling mid-escape sequence AFTER the + // reset (whose ESC would abort it) so the racing live continuation + // completes it instead of rendering literally (#7329). + writeReplayData(connectResult.pendingEscapeTailAnsi) + } sendFocusedReattachFocusInAfterReplay() if (connectResult.coldRestore) { // Snapshot superseded the cold-restore payload — ack it so the diff --git a/src/renderer/src/components/terminal-pane/pty-transport-types.ts b/src/renderer/src/components/terminal-pane/pty-transport-types.ts index f1e3b1b67b8..312945ed3f6 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-types.ts @@ -18,6 +18,11 @@ export type PtyBufferSnapshot = { * scrollback lives in xterm and a clear destroys scroll-up after a tab * return. Mirrors the attach-time guard in pty-transport.ts. */ alternateScreen?: boolean + /** Trailing partial escape sequence the source emulator held mid-parse when + * the snapshot was taken. The restorer writes it LAST (after the reset) so a + * racing live continuation completes it instead of rendering literally + * (#7329). */ + pendingEscapeTailAnsi?: string } export type LocalPtySessionMetadata = { cwd?: string; shellOverride?: string } @@ -32,13 +37,20 @@ export type PtyConnectResult = { sessionExpired?: boolean coldRestore?: { scrollback: string; cwd: string } replay?: string + /** Trailing partial escape the daemon emulator held mid-parse; the reattach + * replay writes it LAST (after the reset) so a racing live continuation + * completes it instead of rendering literally (#7329). */ + pendingEscapeTailAnsi?: string } type PtyCallbacks = { onConnect?: () => void onDisconnect?: () => void onData?: (data: string, meta?: PtyDataMeta) => void - onReplayData?: (data: string, meta?: { clearBeforeReplay?: boolean }) => void + onReplayData?: ( + data: string, + meta?: { clearBeforeReplay?: boolean; pendingEscapeTailAnsi?: string } + ) => void onStatus?: (shell: string) => void onError?: (message: string, errors?: string[]) => void onExit?: (code: number) => void @@ -67,6 +79,13 @@ export type PtyTransport = { }) => void disconnect: () => void sendInput: (data: string) => boolean + // Why: latency-critical terminal query replies (CPR/DSR/DA/OSC color/pixel + // size) must skip input coalescing — a querying program reads them in raw + // mode with a short timeout, so a debounced reply lands on the shell prompt + // and corrupts input (#7329). Local transports already write promptly, so + // this is `sendInput` for them; the remote transport flushes pending input + // (preserving order) and sends the reply immediately. + sendInputImmediate: (data: string) => boolean sendInputAccepted?: (data: string) => Promise resize: ( cols: number, diff --git a/src/renderer/src/components/terminal-pane/pty-transport.test.ts b/src/renderer/src/components/terminal-pane/pty-transport.test.ts index eb526d201f4..dc0096d899d 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.test.ts @@ -1026,6 +1026,51 @@ describe('createIpcPtyTransport', () => { }) }) + it('threads the daemon pendingEscapeTailAnsi through the reattach connect result (#7329)', async () => { + // Why: the local daemon ships the mid-escape tail on the spawn/reattach + // result; dropping it here silently regressed the local half of #7329 + // (the consumer test injects at the transport boundary, so only this + // asserts the IPC threading). + const { createIpcPtyTransport } = await import('./pty-transport') + const spawnMock = vi.fn().mockResolvedValue({ + id: 'pty-reattach-tail', + isReattach: true, + snapshot: 'snapshot data', + snapshotCols: 80, + snapshotRows: 24, + pendingEscapeTailAnsi: '\x1b[3' + }) + + ;(globalThis as { window: typeof window }).window = { + ...originalWindow, + api: { + ...originalWindow?.api, + pty: { + ...originalWindow?.api?.pty, + spawn: spawnMock, + write: vi.fn(), + resize: vi.fn(), + kill: vi.fn(), + onData: vi.fn(() => () => {}), + onReplay: vi.fn(() => () => {}), + onExit: vi.fn(() => () => {}) + } + } + } as unknown as typeof window + + const transport = createIpcPtyTransport() + const result = await transport.connect({ + url: '', + sessionId: 'pty-reattach-tail', + callbacks: {} + }) + + expect(result).toMatchObject({ + id: 'pty-reattach-tail', + pendingEscapeTailAnsi: '\x1b[3' + }) + }) + it('kills a PTY that finishes spawning after the transport was destroyed', async () => { const { createIpcPtyTransport } = await import('./pty-transport') const spawnControls: { resolve: ((value: { id: string }) => void) | null } = { resolve: null } diff --git a/src/renderer/src/components/terminal-pane/pty-transport.ts b/src/renderer/src/components/terminal-pane/pty-transport.ts index c85138cc7be..6e6c80a3932 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.ts @@ -85,6 +85,10 @@ type ProcessPtyOutputOptions = { replayingBufferedData?: boolean suppressAttentionEvents?: boolean clearBeforeReplay?: boolean + // Why: a mid-escape tail the daemon could not serialize. The replay consumer + // must write it LAST, after the post-replay reset, so the next live chunk + // completes it instead of rendering literally (#7329). + pendingEscapeTailAnsi?: string } type PendingPtySideEffect = { @@ -402,8 +406,16 @@ export function createPtyOutputProcessor({ // session into the live store. The parser still consumes the bytes so they // do not leak into xterm, we just suppress the callback. if (options.replayingBufferedData && callbacks.onReplayData) { - if (options.clearBeforeReplay === false) { - callbacks.onReplayData(data, { clearBeforeReplay: false }) + const replayMeta = { + ...(options.clearBeforeReplay === false ? { clearBeforeReplay: false } : {}), + ...(options.pendingEscapeTailAnsi + ? { pendingEscapeTailAnsi: options.pendingEscapeTailAnsi } + : {}) + } + // Why: preserve the bare-data call shape when there is no replay metadata, + // so eager-buffer replay (which passes neither) is unchanged. + if (Object.keys(replayMeta).length > 0) { + callbacks.onReplayData(data, replayMeta) } else { callbacks.onReplayData(data) } @@ -671,7 +683,8 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra isAlternateScreen: spawnResult.isAlternateScreen, sessionExpired: spawnResult.sessionExpired, coldRestore: spawnResult.coldRestore, - replay: spawnResult.replay + replay: spawnResult.replay, + pendingEscapeTailAnsi: spawnResult.pendingEscapeTailAnsi } satisfies PtyConnectResult } if (spawnResult.launchConfig) { @@ -849,6 +862,17 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra return inputWriteQueue.enqueue(ptyId, data) }, + // Why: the local write queue already drains a lone item in the same turn + // (no wall-clock debounce), so query replies are prompt without special + // handling. Kept as a distinct method so callers express intent and the + // remote transport can override with its flush-then-send behavior (#7329). + sendInputImmediate(data: string): boolean { + if (!connected || !ptyId) { + return false + } + return inputWriteQueue.enqueue(ptyId, data) + }, + ...(connectionId ? {} : { diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-batching.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-batching.ts index 8e1ee1b0ab5..48a37e4b9d7 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-batching.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-batching.ts @@ -8,6 +8,7 @@ import { export type RemoteRuntimePtyBatcher = { push: (data: string) => boolean + hasPendingValidation: () => boolean drain: () => Promise takePending: () => string flush: () => void @@ -145,6 +146,10 @@ export function createRemoteRuntimePtyTextBatcher( enqueueValidatedInput(data, tooLarge) return true }, + // Why: earlier input can be mid async byte-length validation and not yet in + // `pending`. `takePending()` cannot see it, so callers that must preserve + // byte order (sendInputImmediate) check this before bypassing the queue. + hasPendingValidation: (): boolean => validationTail !== null, drain, takePending, flush, diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-query-reply-immediate.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-query-reply-immediate.test.ts new file mode 100644 index 00000000000..8060b32b7c1 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-query-reply-immediate.test.ts @@ -0,0 +1,142 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { CLIPBOARD_TEXT_MEASURE_YIELD_CODE_UNITS } from '../../../../shared/clipboard-text' + +// Regression for #7329: terminal query replies must NOT sit behind the remote +// input debounce (REMOTE_TERMINAL_INPUT_FLUSH_MS). transport.sendInputImmediate +// sends without arming the 8ms timer, so a reply beats the querying program's +// raw-mode read window; transport.sendInput stays debounced for typed input. + +describe('remote transport sendInputImmediate (#7329)', () => { + const runtimeCall = vi.fn() + const runtimeSubscribe = vi.fn() + const subscriptionSendBinary = vi.fn() + let subscriptionCallbacks: { + onResponse: (response: unknown) => void + onBinary?: (bytes: Uint8Array) => void + onError?: (error: { code: string; message: string }) => void + onClose?: () => void + } | null = null + + beforeEach(() => { + vi.resetModules() + vi.doUnmock('../../runtime/remote-runtime-terminal-multiplexer') + vi.clearAllMocks() + subscriptionCallbacks = null + subscriptionSendBinary.mockReset() + runtimeCall.mockResolvedValue({ ok: true, result: { terminal: { handle: 'terminal-1' } } }) + runtimeSubscribe.mockImplementation( + async (_args: unknown, callbacks: typeof subscriptionCallbacks) => { + subscriptionCallbacks = callbacks + return { unsubscribe: vi.fn(), sendBinary: subscriptionSendBinary } + } + ) + vi.stubGlobal('window', { + api: { + runtimeEnvironments: { call: runtimeCall, subscribe: runtimeSubscribe } + } + }) + }) + + function terminalSendCalls(): unknown[] { + return runtimeCall.mock.calls + .map((call) => call[0] as { method?: string; params?: { text?: string } }) + .filter((args) => args.method === 'terminal.send') + } + + it('sends a query reply immediately, but debounces typed input by 8ms', async () => { + vi.useFakeTimers() + try { + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: 'pane:1' + }) + transport.attach({ + existingPtyId: 'remote:env-1@@terminal-1', + cols: 80, + rows: 24, + callbacks: {} + }) + + // Typed input: debounced — nothing sent before the 8ms flush. + expect(transport.sendInput('yes')).toBe(true) + expect(terminalSendCalls()).toEqual([]) + + // Query reply (OSC 11 background color): sent immediately, no timer. + expect(transport.sendInputImmediate('\x1b]11;rgb:2828/2c2c/3434\x1b\\')).toBe(true) + await Promise.resolve() + + // The immediate send flushed pending typed input ahead of the reply + // (preserving byte order) in a single send, without advancing timers. + const sends = terminalSendCalls() as { params: { text: string } }[] + expect(sends).toHaveLength(1) + expect(sends[0]?.params.text).toBe('yes\x1b]11;rgb:2828/2c2c/3434\x1b\\') + } finally { + vi.useRealTimers() + } + }) + + it('sends an immediate reply even with no pending typed input', async () => { + vi.useFakeTimers() + try { + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: 'pane:1' + }) + transport.attach({ + existingPtyId: 'remote:env-1@@terminal-1', + cols: 80, + rows: 24, + callbacks: {} + }) + + expect(transport.sendInputImmediate('\x1b[3;1R')).toBe(true) // CPR reply + await Promise.resolve() + + const sends = terminalSendCalls() as { params: { text: string } }[] + expect(sends).toHaveLength(1) + expect(sends[0]?.params.text).toBe('\x1b[3;1R') + } finally { + vi.useRealTimers() + } + }) + + it('does not reorder a query reply ahead of a large paste still in async validation', async () => { + // #7736 review: a paste over the deferred-measurement threshold sits in the + // batcher's async validationTail (not in `pending`). sendInputImmediate must + // not send the reply ahead of it and reorder bytes on the wire. + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: 'pane:1' + }) + transport.attach({ + existingPtyId: 'remote:env-1@@terminal-1', + cols: 80, + rows: 24, + callbacks: {} + }) + + // A paste above CLIPBOARD_TEXT_MEASURE_YIELD_CODE_UNITS forces the async + // validation path, so its bytes are captured in validationTail, not pending. + const paste = 'p'.repeat(CLIPBOARD_TEXT_MEASURE_YIELD_CODE_UNITS + 1) + expect(transport.sendInput(paste)).toBe(true) + // Immediately (validation still pending) a TUI emits a CPR reply. + expect(transport.sendInputImmediate('\x1b[3;1R')).toBe(true) + + // Wait until the reply is actually flushed instead of sleeping a fixed + // interval: the reply legitimately trails the paste's async validation plus + // one debounce tick, and a fixed sleep raced that chain under CI load. + const combined = (): string => + (terminalSendCalls() as { params: { text: string } }[]).map((s) => s.params.text).join('') + await expect.poll(combined, { timeout: 5000, interval: 5 }).toContain('\x1b[3;1R') + + // The paste bytes must come before the reply — no reordering. + expect(combined().indexOf('p')).toBeLessThan(combined().indexOf('\x1b[3;1R')) + expect(combined()).toContain(`${paste}\x1b[3;1R`) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-snapshot-escape-tail.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-snapshot-escape-tail.test.ts new file mode 100644 index 00000000000..8d701469775 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-snapshot-escape-tail.test.ts @@ -0,0 +1,116 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + TerminalStreamOpcode, + decodeTerminalStreamFrame, + decodeTerminalStreamJson, + encodeTerminalStreamFrame, + encodeTerminalStreamJson, + encodeTerminalStreamText +} from '../../../../shared/terminal-stream-protocol' + +// Client-side wire regression for #7329: the daemon's pendingEscapeTailAnsi +// rides the SnapshotStart JSON frame. This drives REAL binary frames through +// the REAL multiplexer (decodeSnapshotInfo → onSnapshot meta) into the REAL +// transport (processData → onReplayData meta). The server-side counterpart is +// terminal-multiplex-escape-tail.test.ts; without this test the client half of +// the chain could silently drop the field and every suite stayed green. + +describe('remote transport snapshot escape-tail threading (#7329)', () => { + const runtimeCall = vi.fn() + const runtimeSubscribe = vi.fn() + const subscriptionSendBinary = vi.fn() + let subscriptionCallbacks: { + onResponse: (response: unknown) => void + onBinary?: (bytes: Uint8Array) => void + onError?: (error: { code: string; message: string }) => void + onClose?: () => void + } | null = null + + beforeEach(() => { + vi.resetModules() + vi.doUnmock('../../runtime/remote-runtime-terminal-multiplexer') + vi.clearAllMocks() + subscriptionCallbacks = null + subscriptionSendBinary.mockReset() + runtimeCall.mockResolvedValue({ ok: true, result: { terminal: { handle: 'terminal-1' } } }) + runtimeSubscribe.mockImplementation( + async (_args: unknown, callbacks: typeof subscriptionCallbacks) => { + subscriptionCallbacks = callbacks + return { unsubscribe: vi.fn(), sendBinary: subscriptionSendBinary } + } + ) + vi.stubGlobal('window', { + api: { + runtimeEnvironments: { call: runtimeCall, subscribe: runtimeSubscribe } + } + }) + }) + + it('delivers the SnapshotStart pendingEscapeTailAnsi to onReplayData meta', async () => { + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: 'pane:1' + }) + const onReplayData = vi.fn() + transport.attach({ + existingPtyId: 'remote:env-1@@terminal-1', + cols: 80, + rows: 24, + callbacks: { onReplayData } + }) + + // Complete the multiplexer handshake so the terminal stream subscribes. + await expect.poll(() => subscriptionCallbacks !== null, { timeout: 5000 }).toBe(true) + subscriptionCallbacks?.onResponse({ ok: true, result: { type: 'ready' } }) + + // The client allocates the terminal's stream id in its Subscribe payload. + await expect + .poll(() => subscriptionSendBinary.mock.calls.length, { timeout: 5000 }) + .toBeGreaterThan(0) + const subscribeFrame = subscriptionSendBinary.mock.calls + .map((call) => decodeTerminalStreamFrame(call[0] as Uint8Array)) + .find((frame) => frame?.opcode === TerminalStreamOpcode.Subscribe) + expect(subscribeFrame).toBeDefined() + const subscribePayload = decodeTerminalStreamJson<{ streamId: number }>(subscribeFrame!.payload) + const streamId = subscribePayload!.streamId + + // Server → client: initial snapshot whose emulator sat mid-escape. + const frames = [ + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.SnapshotStart, + streamId, + seq: 0, + payload: encodeTerminalStreamJson({ + cols: 80, + rows: 24, + seq: 7, + source: 'headless', + pendingEscapeTailAnsi: '\x1b[3' + }) + }), + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.SnapshotChunk, + streamId, + seq: 0, + payload: encodeTerminalStreamText('user@host:~$ ') + }), + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.SnapshotEnd, + streamId, + seq: 0, + payload: new Uint8Array(0) + }) + ] + for (const frame of frames) { + subscriptionCallbacks?.onBinary?.(frame) + } + + await expect.poll(() => onReplayData.mock.calls.length, { timeout: 5000 }).toBeGreaterThan(0) + expect(onReplayData).toHaveBeenCalledWith( + 'user@host:~$ ', + expect.objectContaining({ pendingEscapeTailAnsi: '\x1b[3' }) + ) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts index f739ef9707f..9caf6210475 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts @@ -391,11 +391,16 @@ export function createRemoteRuntimePtyTransport( outputProcessor.processData(data, storedCallbacks, undefined, meta) } }, - onSnapshot: (data) => { - if (data && isCurrentSubscription()) { + onSnapshot: (data, meta) => { + // Why: a snapshot with no body can still carry a pending mid-escape + // tail that must be replayed so the next live chunk completes it. + if ((data || meta?.pendingEscapeTailAnsi) && isCurrentSubscription()) { outputProcessor.processData(data, storedCallbacks, { replayingBufferedData: true, - suppressAttentionEvents: true + suppressAttentionEvents: true, + ...(meta?.pendingEscapeTailAnsi + ? { pendingEscapeTailAnsi: meta.pendingEscapeTailAnsi } + : {}) }) } }, @@ -616,6 +621,49 @@ export function createRemoteRuntimePtyTransport( return inputBatcher.push(data) }, + // Why: terminal query replies (CPR/DSR/DA/OSC color/pixel size) are read by + // the querying program in raw mode with a short timeout. The 8ms input + // debounce makes the reply miss that window, so it lands on the shell prompt + // and is echoed literally / spliced into typed input (#7329). Flush any + // pending batched input first so byte order is preserved, then send the + // reply immediately without arming the debounce timer. + sendInputImmediate(data: string): boolean { + const targetHandle = handle + if (!connected || !targetHandle) { + return false + } + if (!data) { + return true + } + // Why: earlier input (e.g. a large paste) may still be in async byte-length + // validation, so it is captured in the batcher's validationTail and NOT in + // takePending(). Bypassing the queue here would send the reply ahead of it + // and reorder bytes on the wire. In that rare window, route the reply + // through the batcher's ordered queue and flush what is already validated; + // the reply lands right after the pending input once its validation + // resolves. Order correctness beats the immediacy that the debounce + // normally trades away. + if (inputBatcher.hasPendingValidation()) { + const accepted = inputBatcher.push(data) + inputBatcher.flush() + return accepted + } + const pending = inputBatcher.takePending() + const text = `${pending}${data}` + const stream = getCurrentMultiplexedStream(targetHandle) + if (stream?.sendInput(text)) { + return true + } + void callRuntime('terminal.send', { + terminal: targetHandle, + text, + client: { id: clientId, type: 'desktop' } + }).catch((error) => { + storedCallbacks.onError?.(runtimeTerminalErrorMessage(error)) + }) + return true + }, + sendInputAccepted: sendInputAcceptedToRuntime, resize(cols: number, rows: number): boolean { diff --git a/src/renderer/src/runtime/remote-runtime-terminal-multiplexer.ts b/src/renderer/src/runtime/remote-runtime-terminal-multiplexer.ts index bd2f13fc6d2..70fb729b9db 100644 --- a/src/renderer/src/runtime/remote-runtime-terminal-multiplexer.ts +++ b/src/renderer/src/runtime/remote-runtime-terminal-multiplexer.ts @@ -37,7 +37,7 @@ type TerminalMultiplexEvent = export type RemoteRuntimeMultiplexedTerminalCallbacks = { onData: (data: string, meta?: { seq?: number; rawLength?: number }) => void - onSnapshot: (data: string) => void + onSnapshot: (data: string, meta?: { pendingEscapeTailAnsi?: string }) => void onSubscribed?: () => void onEnd?: () => void onError?: (message: string) => void @@ -86,6 +86,10 @@ type RemoteRuntimeSnapshotInfo = { source?: 'headless' | 'renderer' requestId?: number truncated?: boolean + // Why: a mid-escape tail the emulator could not serialize; the transport + // must write it AFTER the replay reset so the next live chunk completes it + // instead of rendering literally (#7329). + pendingEscapeTailAnsi?: string } type RemoteRuntimeSnapshotRequest = { @@ -97,6 +101,7 @@ type RemoteRuntimeSnapshotRequest = { rows: number seq?: number source?: 'headless' | 'renderer' + pendingEscapeTailAnsi?: string } | null ) => void reject: (error: Error) => void @@ -381,11 +386,14 @@ class RemoteRuntimeTerminalMultiplexer { cols: info?.cols ?? 80, rows: info?.rows ?? 24, seq: info?.seq, - source: info?.source + source: info?.source, + pendingEscapeTailAnsi: info?.pendingEscapeTailAnsi }) clearPendingSnapshotRequest(stream) } else if (target === 'initial') { - stream.callbacks.onSnapshot(data ?? '') + stream.callbacks.onSnapshot(data ?? '', { + pendingEscapeTailAnsi: info?.pendingEscapeTailAnsi + }) } } else if (matchesPendingRequest) { pendingRequest.resolve(null) @@ -608,6 +616,7 @@ function decodeSnapshotInfo( source?: unknown requestId?: unknown truncated?: unknown + pendingEscapeTailAnsi?: unknown }>(payload) if (!raw) { return null @@ -618,7 +627,9 @@ function decodeSnapshotInfo( seq: typeof raw.seq === 'number' ? raw.seq : undefined, source: raw.source === 'headless' || raw.source === 'renderer' ? raw.source : undefined, requestId: typeof raw.requestId === 'number' ? raw.requestId : undefined, - truncated: raw.truncated === true + truncated: raw.truncated === true, + pendingEscapeTailAnsi: + typeof raw.pendingEscapeTailAnsi === 'string' ? raw.pendingEscapeTailAnsi : undefined } } diff --git a/src/shared/terminal-partial-escape-tail.test.ts b/src/shared/terminal-partial-escape-tail.test.ts new file mode 100644 index 00000000000..e0ef2095ddd --- /dev/null +++ b/src/shared/terminal-partial-escape-tail.test.ts @@ -0,0 +1,86 @@ +import { describe, expect, it } from 'vitest' +import { + advancePartialEscapeTail, + extractPartialEscapeTail, + MAX_PARTIAL_ESCAPE_TAIL_LENGTH +} from './terminal-partial-escape-tail' + +describe('extractPartialEscapeTail', () => { + it('returns empty for parser-clean streams', () => { + expect(extractPartialEscapeTail('')).toBe('') + expect(extractPartialEscapeTail('plain text no escapes')).toBe('') + expect(extractPartialEscapeTail('\x1b[38;5;196mred\x1b[0m done')).toBe('') + expect(extractPartialEscapeTail('\x1b[2J\x1b[H')).toBe('') + }) + + it('returns the dangling CSI when a chunk ends mid-sequence', () => { + expect(extractPartialEscapeTail('hello\x1b[3')).toBe('\x1b[3') + expect(extractPartialEscapeTail('a\x1b[38;5;')).toBe('\x1b[38;5;') + expect(extractPartialEscapeTail('\x1b')).toBe('\x1b') + expect(extractPartialEscapeTail('\x1b[')).toBe('\x1b[') + }) + + it('returns the dangling OSC (unterminated) sequence', () => { + // OSC 0 title with no BEL/ST terminator yet. + expect(extractPartialEscapeTail('\x1b]0;my-title')).toBe('\x1b]0;my-title') + // Terminated OSC is clean. + expect(extractPartialEscapeTail('\x1b]0;title\x07after')).toBe('') + expect(extractPartialEscapeTail('\x1b]0;title\x1b\\after')).toBe('') + }) + + it('treats a fresh ESC as aborting a pending CSI', () => { + // The second ESC starts a new (complete) sequence. + expect(extractPartialEscapeTail('\x1b[3\x1b[0m')).toBe('') + // ...and a new dangling one. + expect(extractPartialEscapeTail('\x1b[3\x1b[')).toBe('\x1b[') + }) + + it('treats CAN/SUB as aborting an in-progress escape back to ground', () => { + // CAN (0x18) / SUB (0x1a) abort the sequence in xterm's VT500 parser. + // esc state: + expect(extractPartialEscapeTail('\x1b\x18')).toBe('') // ESC CAN + expect(extractPartialEscapeTail('\x1b\x1a')).toBe('') // ESC SUB + // escIntermediate state (ESC then an intermediate byte, then CAN): + expect(extractPartialEscapeTail('\x1b \x18')).toBe('') // ESC SP CAN + expect(extractPartialEscapeTail('\x1b#\x1a')).toBe('') // ESC # SUB + // csi/osc/string already aborted — keep them green: + expect(extractPartialEscapeTail('\x1b[38;\x18')).toBe('') // CSI ... CAN + expect(extractPartialEscapeTail('\x1b]0;title\x18')).toBe('') // OSC ... CAN + // A CAN that aborts, followed by a fresh dangling sequence, tracks the new one: + expect(extractPartialEscapeTail('\x1b\x18\x1b[3')).toBe('\x1b[3') + }) + + it('is fold-safe across chunk boundaries', () => { + // extract(a + b) === extract(extract(a) + b) — the invariant ingest relies on. + const cases: [string, string][] = [ + ['first\x1b[3', '8;5;196mred'], + ['\x1b', '[0m'], + ['\x1b]0;ti', 'tle\x07'], + ['clean', '\x1b[1'], + // Fold-safety must hold across the CAN abort too. + ['\x1b', '\x18after'], + ['\x1b ', '\x18after'] + ] + for (const [a, b] of cases) { + expect(extractPartialEscapeTail(extractPartialEscapeTail(a) + b)).toBe( + extractPartialEscapeTail(a + b) + ) + } + }) +}) + +describe('advancePartialEscapeTail', () => { + it('accumulates a split sequence across chunks', () => { + let tail = '' + tail = advancePartialEscapeTail(tail, 'ls\r\n\x1b[3') + expect(tail).toBe('\x1b[3') + tail = advancePartialEscapeTail(tail, '8;5;196m') + expect(tail).toBe('') // sequence completed + }) + + it('abandons tracking (returns empty) past the cap', () => { + // An unterminated OSC longer than the cap degrades to pre-fix behavior. + const huge = `\x1b]0;${'x'.repeat(MAX_PARTIAL_ESCAPE_TAIL_LENGTH + 10)}` + expect(advancePartialEscapeTail('', huge)).toBe('') + }) +}) diff --git a/src/shared/terminal-partial-escape-tail.ts b/src/shared/terminal-partial-escape-tail.ts new file mode 100644 index 00000000000..1ee606e60f6 --- /dev/null +++ b/src/shared/terminal-partial-escape-tail.ts @@ -0,0 +1,149 @@ +// Why this module exists: a PTY read can end mid-escape-sequence. The bytes +// already handed to xterm sit inside its parser state machine, not the screen +// buffer, so a serialized snapshot cannot carry them — and the next chunk's +// continuation bytes then render as literal text after a snapshot restore +// (Bug E in notes/garble-fuzz-divergences.md). Tracking the unparsed trailing +// partial sequence at the ingest boundary lets snapshot producers append it +// after the serialized screen so the continuation completes exactly as live. + +// Mirrors the VT500 parser states that can span a chunk boundary. C0 controls +// (except ESC/CAN/SUB) execute mid-sequence without aborting it, matching +// xterm's state machine. +type ScanState = + | 'ground' + | 'esc' + | 'escIntermediate' + | 'csi' + | 'osc' + | 'oscEsc' + | 'string' + | 'stringEsc' + +const ESC = 0x1b +const CAN = 0x18 +const SUB = 0x1a +const BEL = 0x07 + +// Why a cap: OSC/DCS payloads are unbounded and an unterminated one would +// grow the tracked tail (and every snapshot) without limit. Real payloads +// (titles, cwd, hyperlinks) are far below this; beyond it we stop tracking +// and degrade to the pre-fix behavior for that pathological stream. +export const MAX_PARTIAL_ESCAPE_TAIL_LENGTH = 4096 + +/** ESC-state transition shared by the fresh-ESC and abort-reprocess paths. */ +function stateAfterEscByte(code: number): ScanState { + if (code === 0x5b) { + return 'csi' // [ + } + if (code === 0x5d) { + return 'osc' // ] + } + // P / X / ^ / _ open DCS / SOS / PM / APC — ST-terminated strings. + if (code === 0x50 || code === 0x58 || code === 0x5e || code === 0x5f) { + return 'string' + } + if (code >= 0x20 && code <= 0x2f) { + return 'escIntermediate' + } + if (code < 0x20 || code === 0x7f) { + return 'esc' // C0 executes / DEL is ignored mid-sequence; ESC via callers + } + return 'ground' // final byte — two-byte sequence (ESC 7, ESC 8, ESC c, …) +} + +/** Returns the trailing incomplete escape sequence of `stream` ('' when the + * stream ends parser-clean). Fold-safe across chunk boundaries: + * extract(a + b) === extract(extract(a) + b), which is how ingest-time + * trackers advance without keeping the whole stream. */ +export function extractPartialEscapeTail(stream: string): string { + let state: ScanState = 'ground' + let start = 0 + for (let i = 0; i < stream.length; i++) { + const code = stream.charCodeAt(i) + if (state === 'ground') { + if (code === ESC) { + start = i + state = 'esc' + } + continue + } + if ( + code === ESC && + state !== 'osc' && + state !== 'string' && + state !== 'oscEsc' && + state !== 'stringEsc' + ) { + // ESC aborts a pending ESC/CSI sequence and starts a new one. + start = i + state = 'esc' + continue + } + // CAN/SUB abort an in-progress escape sequence back to ground in every + // non-string state (xterm's VT500 parser). The csi/osc/string cases handle + // this inline below; esc/escIntermediate must too, or `ESC CAN` and + // `ESC CAN` leave a bogus tail instead of dropping to ground. + if ((code === CAN || code === SUB) && (state === 'esc' || state === 'escIntermediate')) { + state = 'ground' + continue + } + switch (state) { + case 'esc': + state = stateAfterEscByte(code) + break + case 'escIntermediate': + if (code >= 0x30 && code <= 0x7e) { + state = 'ground' + } + // 0x20–0x2f stays; other C0 executes and stays (CAN/SUB handled above). + break + case 'csi': + if (code === CAN || code === SUB) { + state = 'ground' + } else if (code >= 0x40 && code <= 0x7e) { + state = 'ground' // final byte completes the CSI + } + // params/intermediates (0x20–0x3f), C0, DEL stay in-sequence. + break + case 'osc': + if (code === BEL || code === CAN || code === SUB) { + state = 'ground' + } else if (code === ESC) { + state = 'oscEsc' + } + break + case 'oscEsc': + if (code === 0x5c) { + state = 'ground' // ESC \ = ST terminates the OSC + } else { + // The ESC aborted the OSC and opened a new sequence at i-1. + start = i - 1 + state = code === ESC ? 'esc' : stateAfterEscByte(code) + } + break + case 'string': + if (code === CAN || code === SUB) { + state = 'ground' + } else if (code === ESC) { + state = 'stringEsc' + } + break + case 'stringEsc': + if (code === 0x5c) { + state = 'ground' + } else { + start = i - 1 + state = code === ESC ? 'esc' : stateAfterEscByte(code) + } + break + } + } + return state === 'ground' ? '' : stream.slice(start) +} + +/** Ingest-time fold: advance the tracked tail with one more chunk. Returns '' + * (tracking abandoned) when the tail exceeds the cap — see the cap comment. */ +export function advancePartialEscapeTail(pendingTail: string, chunk: string): string { + const tail = extractPartialEscapeTail(pendingTail + chunk) + return tail.length > MAX_PARTIAL_ESCAPE_TAIL_LENGTH ? '' : tail +} diff --git a/src/shared/terminal-query-reply.test.ts b/src/shared/terminal-query-reply.test.ts new file mode 100644 index 00000000000..ee26abf1604 --- /dev/null +++ b/src/shared/terminal-query-reply.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, it } from 'vitest' +import { isTerminalQueryReply } from './terminal-query-reply' + +describe('isTerminalQueryReply', () => { + it('matches synthetic query replies that must be sent immediately', () => { + // CPR cursor position report (answer to CSI 6n) — the #7329 culprit. + expect(isTerminalQueryReply('\x1b[3;1R')).toBe(true) + expect(isTerminalQueryReply('\x1b[22;1R')).toBe(true) + // DSR device status. + expect(isTerminalQueryReply('\x1b[0n')).toBe(true) + // DA1/DA2/DA3 device attributes. + expect(isTerminalQueryReply('\x1b[?1;2c')).toBe(true) + expect(isTerminalQueryReply('\x1b[?61;4c')).toBe(true) + expect(isTerminalQueryReply('\x1b[>0;276;0c')).toBe(true) + // Window/cell pixel-size reports. + expect(isTerminalQueryReply('\x1b[6;16;8t')).toBe(true) + expect(isTerminalQueryReply('\x1b[4;384;640t')).toBe(true) + // DECRPM mode report — private (with ?) and ANSI (without ?). + expect(isTerminalQueryReply('\x1b[?2026;2$y')).toBe(true) + expect(isTerminalQueryReply('\x1b[4;1$y')).toBe(true) + // OSC 10/11 color responses (the #7329 culprit) — BEL and ST terminated. + expect(isTerminalQueryReply('\x1b]11;rgb:2828/2c2c/3434\x1b\\')).toBe(true) + expect(isTerminalQueryReply('\x1b]10;rgb:c0c0/c0c0/c0c0\x07')).toBe(true) + // DECXCPR extended cursor position report (answer to CSI ? 6n). + expect(isTerminalQueryReply('\x1b[?12;5R')).toBe(true) + // Text-area size in characters (answer to CSI 18t). + expect(isTerminalQueryReply('\x1b[8;24;80t')).toBe(true) + // Kitty keyboard flags report (answer to CSI ? u) — crossterm probes this + // at startup, so a debounced reply corrupts the same way CPR did. + expect(isTerminalQueryReply('\x1b[?0u')).toBe(true) + expect(isTerminalQueryReply('\x1b[?31u')).toBe(true) + // DCS DECRQSS reports (vim queries cursor style via DCS $ q) + XTVERSION. + expect(isTerminalQueryReply('\x1bP1$r2 q\x1b\\')).toBe(true) + expect(isTerminalQueryReply('\x1bP1$r0m\x1b\\')).toBe(true) + expect(isTerminalQueryReply('\x1bP0$r\x1b\\')).toBe(true) + expect(isTerminalQueryReply('\x1bP>|xterm.js(5.6.0)\x1b\\')).toBe(true) + }) + + it('documents the accepted modified-F3/CPR collision', () => { + // xterm.js encodes Shift+F3 as CSI 1;2R — byte-identical to a CPR report. + // Classified as a reply on purpose: order is still preserved (the immediate + // path flushes pending input first); see the comment in terminal-query-reply.ts. + expect(isTerminalQueryReply('\x1b[1;2R')).toBe(true) + }) + + it('does NOT match ordinary typed input or navigation sequences', () => { + // Plain text. + expect(isTerminalQueryReply('yes')).toBe(false) + expect(isTerminalQueryReply('y')).toBe(false) + expect(isTerminalQueryReply('\r')).toBe(false) + expect(isTerminalQueryReply('\x03')).toBe(false) // Ctrl-C + // Arrow keys / navigation — must stay batched (coalesced auto-repeat). + expect(isTerminalQueryReply('\x1b[A')).toBe(false) + expect(isTerminalQueryReply('\x1b[B')).toBe(false) + expect(isTerminalQueryReply('\x1b[C')).toBe(false) + expect(isTerminalQueryReply('\x1b[D')).toBe(false) + expect(isTerminalQueryReply('\x1b[H')).toBe(false) // Home + expect(isTerminalQueryReply('\x1b[F')).toBe(false) // End + // Function keys (end in ~). + expect(isTerminalQueryReply('\x1b[15~')).toBe(false) + expect(isTerminalQueryReply('\x1b[3~')).toBe(false) // Delete + // Bare Escape key. + expect(isTerminalQueryReply('\x1b')).toBe(false) + // Alt+key (including Alt+Shift+P, whose bytes prefix the DCS grammar). + expect(isTerminalQueryReply('\x1bb')).toBe(false) + expect(isTerminalQueryReply('\x1bP')).toBe(false) + // Kitty-protocol KEYSTROKES (CSI code;mods u, no "?") must stay batched. + expect(isTerminalQueryReply('\x1b[97;5u')).toBe(false) + expect(isTerminalQueryReply('\x1b[13u')).toBe(false) + // Modified F1/F2/F4 (CSI 1; P/Q/S) are keystrokes, not replies. + expect(isTerminalQueryReply('\x1b[1;2P')).toBe(false) + expect(isTerminalQueryReply('\x1b[1;2Q')).toBe(false) + expect(isTerminalQueryReply('\x1b[1;2S')).toBe(false) + // Bracketed paste markers are input framing, not replies. + expect(isTerminalQueryReply('\x1b[200~')).toBe(false) + expect(isTerminalQueryReply('\x1b[201~')).toBe(false) + // Incomplete / non-terminated OSC and DCS must not match. + expect(isTerminalQueryReply('\x1b]11;rgb:2828/2c2c/3434')).toBe(false) + expect(isTerminalQueryReply('\x1bP1$r2 q')).toBe(false) + }) +}) diff --git a/src/shared/terminal-query-reply.ts b/src/shared/terminal-query-reply.ts new file mode 100644 index 00000000000..4d009fc0d40 --- /dev/null +++ b/src/shared/terminal-query-reply.ts @@ -0,0 +1,69 @@ +// Why this module exists: xterm's public onData stream mixes real keystrokes +// with the parser's synthetic replies to terminal queries a program embedded in +// its output (CPR/DSR cursor + device-status reports, DA device attributes, +// DECRPM mode reports, window/cell pixel-size reports, OSC 10/11 color reports, +// kitty keyboard flag reports, DCS-framed DECRQSS/XTVERSION reports). +// A querying program (e.g. starship/orb) reads these replies synchronously in +// raw mode with a short timeout, so on the remote path they must NOT sit behind +// the input debounce — a late reply lands on the shell prompt in cooked mode, +// which echoes it literally and splices it into the next typed line (#7329). +// This classifier lets the transport send replies immediately while keeping +// ordinary typed input (including bursty arrow-key auto-repeat) coalesced. + +const ESC = String.fromCharCode(0x1b) + +// Built via new RegExp from \u-escaped strings so no literal control +// characters appear in the source. // Final bytes of xterm's own query-reply grammars: +// R — CPR / DECXCPR cursor position report (answer to CSI 6n / CSI ? 6n) +// n — DSR device status report (answer to CSI 5n → CSI 0n) +// c — DA1/DA2/DA3 device attributes (answer to CSI c / CSI > c / CSI = c) +// t — window/cell pixel-size + text-area-size reports (CSI 14t/16t/18t) +// y — DECRPM mode report (answer to CSI ? Ps $ p), body ends "$y" +// u — kitty keyboard flags report (answer to CSI ? u), carries "?" +/* oxlint-disable no-control-regex -- grammars match terminal ESC/BEL sequences by definition */ +// Known accepted collision: xterm.js encodes MODIFIED F3 (Shift/Ctrl/Alt+F3) as +// `CSI 1 ; R`, which is indistinguishable from a CPR report (a classic +// VT ambiguity). Such a keystroke is sent immediately instead of debounced — +// byte order is still preserved (the immediate path flushes pending input +// first), it just skips input-intent/activity bookkeeping. Harmless, so we +// keep the reply grammar complete rather than special-casing it. +const CPR_OR_DSR_RE = new RegExp('^\\u001b\\[\\??[0-9;]*[Rn]$') +const DEVICE_ATTRIBUTES_RE = new RegExp('^\\u001b\\[[?>=]?[0-9;]*c$') +// 4/6 = pixel-size reports, 8 = text-area size in characters (answer to CSI 18t). +const WINDOW_SIZE_REPORT_RE = new RegExp('^\\u001b\\[[468];[0-9]+;[0-9]+t$') +// `?` optional: private-mode reports carry it (DECRPM), ANSI-mode reports don't. +const DECRPM_RE = new RegExp('^\\u001b\\[\\??[0-9;]*\\$y$') +// Kitty keyboard protocol flags report: CSI ? flags u. The `?` distinguishes it +// from kitty-protocol *keystrokes* (CSI code;mods u), which must stay batched. +const KITTY_FLAGS_RE = new RegExp('^\\u001b\\[\\?[0-9]+u$') +// OSC color/title responses: ESC ] Ps ; body ST (ST = BEL or ESC backslash). +const OSC_RESPONSE_RE = new RegExp('^\\u001b\\][0-9]+;[^\\u0007\\u001b]*(?:\\u0007|\\u001b\\\\)$') +// DCS-framed reports xterm emits: DECRQSS "ESC P 1 $ r Pt ST" / "ESC P 0 $ r ST" +// (vim queries cursor style this way) and XTVERSION "ESC P > | text ST". +const DCS_RESPONSE_RE = new RegExp('^\\u001bP(?:[01]\\$r[^\\u001b]*|>\\|[^\\u001b]*)\\u001b\\\\$') +/* oxlint-enable no-control-regex */ + +/** + * True when `data` (from xterm.onData) is a synthetic reply the emulator + * generated in response to a query — not something the user typed. These are + * latency-critical and must bypass input coalescing on the remote transport. + * + * Conservative by design: matches only complete, well-formed reply grammars so + * ordinary keystrokes and navigation sequences (arrows CSI A/B/C/D, Home/End, + * function keys ending in ~, kitty CSI-u keystrokes) are never misclassified + * as replies — with the single documented modified-F3/CPR collision above. + */ +export function isTerminalQueryReply(data: string): boolean { + if (data.length < 3 || data[0] !== ESC) { + return false + } + return ( + CPR_OR_DSR_RE.test(data) || + DEVICE_ATTRIBUTES_RE.test(data) || + WINDOW_SIZE_REPORT_RE.test(data) || + DECRPM_RE.test(data) || + KITTY_FLAGS_RE.test(data) || + OSC_RESPONSE_RE.test(data) || + DCS_RESPONSE_RE.test(data) + ) +}