From 3fc500fe099c62d8127bef4fe5523af22d20faf5 Mon Sep 17 00:00:00 2001 From: Neil Date: Sat, 24 Oct 2026 15:10:19 -0700 Subject: [PATCH] fix(terminal): answer queries in output a pausing remote view discards unsent A chunk ingested while the view was still receiving yields its queries to that view. If the view then pauses, the host drops the chunk from the output batcher, or clears it from the ack backlog, before the view ever parses it. Nothing answered those queries. The pause now carves the query bytes out of what it discards, with the same extraction the hidden-pane gate uses, and hands them to main's existing responder. The responder answers only while no other view is receiving that terminal's output. Refs #32944 Co-authored-by: Ben Grynhaus --- src/main/ipc/pty/delivery/pending.ts | 10 +----- .../terminal-multiplex-paused-output-drop.ts | 19 +++++++++- .../terminal-multiplex-slot-frames.ts | 10 +++++- ...erminal-multiplex-stream-initialization.ts | 4 +++ ...minal-multiplex-paused-output-drop.test.ts | 36 +++++++++++++++---- .../rpc/terminal-multiplex-test-harness.ts | 1 + .../runtime/terminal-query-responder.test.ts | 21 +++++++++++ .../output/headless-terminal-hydration.ts | 18 ++++++++++ src/shared/terminal-reply-query-extraction.ts | 9 +++++ 9 files changed, 110 insertions(+), 18 deletions(-) diff --git a/src/main/ipc/pty/delivery/pending.ts b/src/main/ipc/pty/delivery/pending.ts index 305605667cc..3e4dba8a914 100644 --- a/src/main/ipc/pty/delivery/pending.ts +++ b/src/main/ipc/pty/delivery/pending.ts @@ -1,4 +1,4 @@ -import { extractHiddenStartupRendererQueryData } from '../../../../shared/terminal-reply-query-extraction' +import { extractDroppedPtyQueryBytes } from '../../../../shared/terminal-reply-query-extraction' import { INITIAL_MODE_2031_REPLY_SCAN_STATE, scanMode2031ReplyDecision, @@ -20,14 +20,6 @@ export function pendingDataCapChars(session: PtyIpcSession): number { return terminalOutputBacklogCapChars(session.getSettings?.().terminalScrollbackRows) } -export function extractDroppedPtyQueryBytes(data: string): string { - if (!data.includes('\x1b')) { - return '' - } - const extracted = extractHiddenStartupRendererQueryData(data, '') - return extracted.statelessQueryData + extracted.statefulQueryData + extracted.oscColorQueryData -} - export function scanDroppedMode2031Data( data: string, previous: Mode2031ReplyScanState diff --git a/src/main/runtime/rpc/methods/terminal/terminal-multiplex-paused-output-drop.ts b/src/main/runtime/rpc/methods/terminal/terminal-multiplex-paused-output-drop.ts index c485e48d244..0c1af4ead27 100644 --- a/src/main/runtime/rpc/methods/terminal/terminal-multiplex-paused-output-drop.ts +++ b/src/main/runtime/rpc/methods/terminal/terminal-multiplex-paused-output-drop.ts @@ -1,7 +1,24 @@ -import { TerminalStreamOpcode } from '../../../../../shared/terminal-stream-protocol' +import { + TerminalStreamOpcode, + decodeTerminalStreamJson, + decodeTerminalStreamText +} from '../../../../../shared/terminal-stream-protocol' +import type { TerminalOutputFrameChunk } from '../../terminal-output-frame-chunks' import type { TerminalMultiplexConnection } from './terminal-multiplex-connection' import type { TerminalMultiplexStream } from './terminal-stream-types' +/** The text of queued output frames a pause discards unsent. */ +export function pausedDiscardedOutputText(chunks: readonly TerminalOutputFrameChunk[]): string { + return chunks + .map((chunk) => + chunk.opcode === TerminalStreamOpcode.OutputSpan + ? (decodeTerminalStreamJson<{ data?: unknown }>(chunk.bytes)?.data ?? '') + : decodeTerminalStreamText(chunk.bytes) + ) + .filter((text): text is string => typeof text === 'string') + .join('') +} + /** Tells a negotiated client that this pause discarded output, so its reveal must restore. */ export function notePausedOutputDropped( state: Pick, diff --git a/src/main/runtime/rpc/methods/terminal/terminal-multiplex-slot-frames.ts b/src/main/runtime/rpc/methods/terminal/terminal-multiplex-slot-frames.ts index ae7b9327863..8cd6c19abe1 100644 --- a/src/main/runtime/rpc/methods/terminal/terminal-multiplex-slot-frames.ts +++ b/src/main/runtime/rpc/methods/terminal/terminal-multiplex-slot-frames.ts @@ -18,7 +18,10 @@ import { serializeBudgetedRequestedSnapshot } from './terminal-snapshot-publication' import { updateViewportForClient } from './terminal-viewport-update' -import { notePausedOutputDropped } from './terminal-multiplex-paused-output-drop' +import { + notePausedOutputDropped, + pausedDiscardedOutputText +} from './terminal-multiplex-paused-output-drop' import { isMultiplexStreamAttached, type MultiplexSnapshotRequest, @@ -84,6 +87,11 @@ export function installMultiplexSlotFrames(state: TerminalMultiplexConnection): stream.pausedOutputDropNoticeSent = false const discardsQueuedOutput = stream.ackPendingOutput.length > 0 || stream.ackPendingOutputOverflowed + // Why: main yielded these queued bytes to this view at ingestion; it will never parse them now. + runtime.answerQueriesInDiscardedRemoteOutput( + stream.ptyId, + pausedDiscardedOutputText(stream.ackPendingOutput) + ) // Why: batched bytes flush into the paused gate, which reports them as dropped. stream.outputBatcher.flush() stream.ackPendingOutput = [] diff --git a/src/main/runtime/rpc/methods/terminal/terminal-multiplex-stream-initialization.ts b/src/main/runtime/rpc/methods/terminal/terminal-multiplex-stream-initialization.ts index a9a30d872ed..fc6e13a01d3 100644 --- a/src/main/runtime/rpc/methods/terminal/terminal-multiplex-stream-initialization.ts +++ b/src/main/runtime/rpc/methods/terminal/terminal-multiplex-stream-initialization.ts @@ -61,6 +61,10 @@ export async function initializeMultiplexStream( lastResizeCols: undefined, resizeGeneration: 0, outputBatcher: createTerminalOutputBatcher((data, meta) => { + // Why: a pause flushes bytes main yielded to this view at ingestion into the paused gate. + if (stream.outputPaused) { + runtime.answerQueriesInDiscardedRemoteOutput(ptyId, data) + } if (meta?.cwd !== undefined) { state.sendFrame( request.streamId, diff --git a/src/main/runtime/rpc/terminal-multiplex-paused-output-drop.test.ts b/src/main/runtime/rpc/terminal-multiplex-paused-output-drop.test.ts index f162ded5efd..1b7230c83aa 100644 --- a/src/main/runtime/rpc/terminal-multiplex-paused-output-drop.test.ts +++ b/src/main/runtime/rpc/terminal-multiplex-paused-output-drop.test.ts @@ -12,6 +12,7 @@ import { RuntimeTerminalViewSubscribers } from '../runtime-terminal-view-subscri async function subscribePausable(capabilities: Record) { const listeners: ((data: string, meta?: RuntimeTerminalDataMeta) => void)[] = [] + const answerQueriesInDiscardedRemoteOutput = vi.fn() const viewSubscribers = new RuntimeTerminalViewSubscribers({ notifyPresenceChanged: () => {}, hasMobileSubscribers: () => false, @@ -24,7 +25,8 @@ async function subscribePausable(capabilities: Record) { return vi.fn() }), registerRemoteTerminalViewSubscriber: (ptyId, isOutputPaused) => - viewSubscribers.registerRemote(ptyId, isOutputPaused) + viewSubscribers.registerRemote(ptyId, isOutputPaused), + answerQueriesInDiscardedRemoteOutput }) await vi.waitFor(() => expect(harness.handlers.has(0)).toBe(true)) harness.handlers.get(0)?.( @@ -73,13 +75,15 @@ async function subscribePausable(capabilities: Record) { harness.registry.cleanupSubscription('terminal-multiplex:conn-desktop-first-paint') await harness.dispatchPromise } - const answersQueries = (): boolean => viewSubscribers.hasRemoteReceivingOutput('pty-1') + const remoteViewHoldsAuthority = (): boolean => viewSubscribers.hasRemoteReceivingOutput('pty-1') return { subscribed: subscribedEvent(), setPaused, emitOutput, dropNotices, - answersQueries, + remoteViewHoldsAuthority, + discardedOutputSalvaged: () => + answerQueriesInDiscardedRemoteOutput.mock.calls.map(([, data]) => data).join(''), close } } @@ -119,14 +123,32 @@ describe('terminal multiplex paused-output drop notice', () => { // #32944: the dropped output can carry a query, so main must answer it while this view is paused. it('yields query authority while paused and takes it back on resume and close', async () => { const host = await subscribePausable({ ackOutput: 1, outputPause: 1, outputPauseDropNotice: 1 }) - expect(host.answersQueries()).toBe(true) + expect(host.remoteViewHoldsAuthority()).toBe(true) host.setPaused(true) - expect(host.answersQueries()).toBe(false) + expect(host.remoteViewHoldsAuthority()).toBe(false) host.setPaused(false) - expect(host.answersQueries()).toBe(true) + expect(host.remoteViewHoldsAuthority()).toBe(true) host.setPaused(true) await host.close() - expect(host.answersQueries()).toBe(false) + expect(host.remoteViewHoldsAuthority()).toBe(false) + }) + + // #32944: main yielded these bytes to the view at ingestion, so a pause that discards them must hand their queries back. + it('hands the queries of batched output a pause discards to main', async () => { + const host = await subscribePausable({ ackOutput: 1, outputPause: 1, outputPauseDropNotice: 1 }) + host.emitOutput('batched\x1b[c') + host.setPaused(true) + expect(host.discardedOutputSalvaged()).toBe('batched\x1b[c') + await host.close() + }) + + it('hands the queries of output waiting on the ack window to main when a pause clears it', async () => { + const host = await subscribePausable({ ackOutput: 1, outputPause: 1, outputPauseDropNotice: 1 }) + // Past the 512 KiB initial window, so the query's frame waits unsent for an ack. + host.emitOutput(`${'x'.repeat(600 * 1024)}\x1b[c`) + host.setPaused(true) + expect(host.discardedOutputSalvaged().endsWith('\x1b[c')).toBe(true) + await host.close() }) it('never sends the frame to a client that did not negotiate it', async () => { diff --git a/src/main/runtime/rpc/terminal-multiplex-test-harness.ts b/src/main/runtime/rpc/terminal-multiplex-test-harness.ts index c85e962d198..0e74ffb9b75 100644 --- a/src/main/runtime/rpc/terminal-multiplex-test-harness.ts +++ b/src/main/runtime/rpc/terminal-multiplex-test-harness.ts @@ -29,6 +29,7 @@ export function stubRuntime(overrides: Partial = {}): OrcaRu // Why: every multiplex stream registers as a remote view subscriber for // Phase-5 query-authority suppression (terminal-query-authority.md). registerRemoteTerminalViewSubscriber: () => () => {}, + answerQueriesInDiscardedRemoteOutput: vi.fn(), // Why: the multiplex subscribe path resolves handles via // resolveLiveLeafForHandle (#7718). Default to a live pty so tests that // only stub the legacy resolveLeafForHandle still bind; tests that need a diff --git a/src/main/runtime/terminal-query-responder.test.ts b/src/main/runtime/terminal-query-responder.test.ts index 3fa44178a14..90fb36a9074 100644 --- a/src/main/runtime/terminal-query-responder.test.ts +++ b/src/main/runtime/terminal-query-responder.test.ts @@ -290,6 +290,27 @@ describe('reply ownership matrix', () => { expect(replies).toHaveLength(2) }) + it('answers the queries of output a pausing view discards, once and only while no view receives', async () => { + const { runtime, replies } = createResponderRuntime() + markHiddenRendererPty('pty-discard') + let paused = false + runtime.registerRemoteTerminalViewSubscriber('pty-discard', () => paused) + runtime.onPtyData('pty-discard', 'prompt\x1b[c', Date.now()) + await settle(runtime, 'pty-discard') + expect(replies).toEqual([]) + + paused = true + runtime.answerQueriesInDiscardedRemoteOutput('pty-discard', 'prompt\x1b[c') + await settle(runtime, 'pty-discard') + expect(replies.map((reply) => reply.data)).toEqual(['\x1b[?1;2c']) + + const releaseLive = runtime.registerRemoteTerminalViewSubscriber('pty-discard', () => false) + runtime.answerQueriesInDiscardedRemoteOutput('pty-discard', '\x1b[c') + await settle(runtime, 'pty-discard') + expect(replies).toHaveLength(1) + releaseLive() + }) + it('keeps raw preview presence separate from terminal query authority', async () => { const { runtime, replies } = createResponderRuntime() markHiddenRendererPty('pty-preview') diff --git a/src/main/runtime/terminal/output/headless-terminal-hydration.ts b/src/main/runtime/terminal/output/headless-terminal-hydration.ts index 3ca2e17dc17..6b913571aff 100644 --- a/src/main/runtime/terminal/output/headless-terminal-hydration.ts +++ b/src/main/runtime/terminal/output/headless-terminal-hydration.ts @@ -9,6 +9,7 @@ import { normalizeTerminalTitle } from '../../../../shared/agent-detection' import { shouldModelAnswerHiddenPtyQueries } from '../../terminal-model-query-authority' +import { extractDroppedPtyQueryBytes } from '../../../../shared/terminal-reply-query-extraction' export abstract class OrcaRuntimeWithMaybeHydrateHeadlessFromRenderer extends OrcaRuntimeWithSerializeMainTerminalBuffer { protected abstract getOrCreateHeadlessTerminal(ptyId: string): RuntimeHeadlessTerminal @@ -174,6 +175,23 @@ export abstract class OrcaRuntimeWithMaybeHydrateHeadlessFromRenderer extends Or }) } + /** Output a remote view discards unread when it pauses was yielded to that view at ingestion, + * so its queries reach no xterm; answer them here unless another view still receives it. */ + answerQueriesInDiscardedRemoteOutput(ptyId: string, data: string): void { + const queries = extractDroppedPtyQueryBytes(data) + const state = this.headlessTerminals.get(ptyId) + if (!queries || !state || !this.shouldAnswerQueriesForLiveChunk(ptyId)) { + return + } + state.writeChain = state.writeChain + .then(async () => { + if (this.headlessTerminals.get(ptyId) === state) { + await state.emulator.write(queries, { forwardQueryReplies: true }) + } + }) + .catch(() => {}) + } + protected trackHeadlessTerminalData( ptyId: string, data: string, diff --git a/src/shared/terminal-reply-query-extraction.ts b/src/shared/terminal-reply-query-extraction.ts index 0ff95b943aa..762cf2833be 100644 --- a/src/shared/terminal-reply-query-extraction.ts +++ b/src/shared/terminal-reply-query-extraction.ts @@ -158,3 +158,12 @@ export function isStatelessRendererReplyCsiQuery(sequence: string): boolean { export function isStatefulRendererReplyCsiQuery(sequence: string): boolean { return sequence === '\x1b[6n' || (sequence.startsWith('\x1b[?') && sequence.endsWith('$p')) } + +/** Only the query bytes of output a view will never parse, so a responder can still answer them. */ +export function extractDroppedPtyQueryBytes(data: string): string { + if (!data.includes('\x1b')) { + return '' + } + const extracted = extractHiddenStartupRendererQueryData(data, '') + return extracted.statelessQueryData + extracted.statefulQueryData + extracted.oscColorQueryData +}