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 7d50b9f48de..5e0e42715ca 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 @@ -377,6 +377,20 @@ export function createRemoteRuntimePtyTransport( pendingClaimQueryReplyCount += 1 } } + // Why: clearing the claim flag without draining strands the queued bytes. + const flushPendingClaimInput = (stream: RemoteRuntimeMultiplexedTerminal): void => { + const queued = pendingClaimInput + pendingViewportClaim = false + pendingClaimInput = [] + pendingClaimQueryReplyCount = 0 + for (const segment of queued) { + stream.sendInput(segment.text) + } + for (const resolve of viewportClaimReadyWaiters) { + resolve(true) + } + viewportClaimReadyWaiters.clear() + } // Why: tab/leaf ids are shared by paired viewers; the instance suffix keeps one viewer's refresh off peer records. const clientId = `desktop:${tabId ?? 'tab'}:${leafId ?? 'leaf'}:${createBrowserUuid()}` const terminalCreateMutationId = createBrowserUuid() @@ -1338,8 +1352,8 @@ export function createRemoteRuntimePtyTransport( } const stream = getCurrentMultiplexedStream(targetHandle) if (claim ? stream?.claimViewport(cols, rows) : stream?.resize(cols, rows)) { - if (claim) { - pendingViewportClaim = false + if (claim && stream) { + flushPendingClaimInput(stream) } return } @@ -1953,17 +1967,6 @@ export function createRemoteRuntimePtyTransport( // Why: a viewport change during the subscribe round-trip hit the no-op one-shot fallback; replay the latest viewport so the PTY isn't stuck at subscribe-time size. if (pendingViewportClaim && desiredViewport) { nextStream.claimViewport(desiredViewport.cols, desiredViewport.rows) - pendingViewportClaim = false - const queuedInput = pendingClaimInput - pendingClaimInput = [] - pendingClaimQueryReplyCount = 0 - for (const segment of queuedInput) { - nextStream.sendInput(segment.text) - } - for (const resolve of viewportClaimReadyWaiters) { - resolve(true) - } - viewportClaimReadyWaiters.clear() } else if ( desiredViewport && (desiredViewport.cols !== subscribedViewport?.cols || @@ -1971,6 +1974,8 @@ export function createRemoteRuntimePtyTransport( ) { nextStream.resize(desiredViewport.cols, desiredViewport.rows) } + // Why: a live claim may already have cleared the flag, so drain on every install. + flushPendingClaimInput(nextStream) } const transport: PtyTransport = { diff --git a/src/renderer/src/runtime/remote-runtime-terminal-multiplexer.ts b/src/renderer/src/runtime/remote-runtime-terminal-multiplexer.ts index 183838fdc2f..7b0d7cb3a8c 100644 --- a/src/renderer/src/runtime/remote-runtime-terminal-multiplexer.ts +++ b/src/renderer/src/runtime/remote-runtime-terminal-multiplexer.ts @@ -464,14 +464,18 @@ class RemoteRuntimeTerminalMultiplexer { const stream: RemoteRuntimeMultiplexedTerminal = { streamId, - sendInput: (text) => this.sendInput(state, text), + sendInput: (text) => this.isRegisteredStream(state) && this.sendInput(state, text), resize: (cols, rows) => + this.isRegisteredStream(state) && this.sendFrame( streamId, TerminalStreamOpcode.Resize, encodeTerminalStreamJson({ cols, rows }) ), claimViewport: (cols, rows) => { + if (!this.isRegisteredStream(state)) { + return false + } const claimed = this.sendFrame( streamId, TerminalStreamOpcode.ClaimViewport, @@ -1181,6 +1185,11 @@ class RemoteRuntimeTerminalMultiplexer { ) } + // Why: sendFrame gates on readiness alone; a dropped handle would still report success. + private isRegisteredStream(stream: RemoteRuntimeMultiplexedTerminalState): boolean { + return this.streams.get(stream.streamId) === stream + } + private sendInput(stream: RemoteRuntimeMultiplexedTerminalState, text: string): boolean { const sent = this.sendFrame( stream.streamId, diff --git a/src/renderer/src/runtime/remote-runtime-terminal-stale-stream-frames.test.ts b/src/renderer/src/runtime/remote-runtime-terminal-stale-stream-frames.test.ts new file mode 100644 index 00000000000..ccde2d69c9d --- /dev/null +++ b/src/renderer/src/runtime/remote-runtime-terminal-stale-stream-frames.test.ts @@ -0,0 +1,87 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + TerminalStreamOpcode, + decodeTerminalStreamFrame, + decodeTerminalStreamText +} from '../../../shared/terminal-stream-protocol' +import { + getRemoteRuntimeTerminalMultiplexer, + resetRemoteRuntimeTerminalMultiplexersForTests, + type RemoteRuntimeMultiplexedTerminal +} from './remote-runtime-terminal-multiplexer' +import { replaceRuntimeEnvironmentRevisions } from './runtime-environment-revision' + +// Why: sendFrame gates on socket readiness alone, so a dropped stream handle reported success +// while the host discarded the frames for an unknown stream id. + +type SubscribeCallbacks = { + onResponse: (response: unknown) => void + onBinary?: (bytes: Uint8Array) => void + onError?: (error: { message: string }) => void + onClose?: () => void +} + +describe('remote terminal stale stream frames', () => { + let sent: Uint8Array[] + + beforeEach(() => { + vi.clearAllMocks() + resetRemoteRuntimeTerminalMultiplexersForTests() + replaceRuntimeEnvironmentRevisions([]) + sent = [] + + const subscribe = vi.fn(async (_args: unknown, callbacks: SubscribeCallbacks) => { + queueMicrotask(() => callbacks.onResponse({ ok: true, result: { type: 'ready' } })) + return { + unsubscribe: vi.fn(), + sendBinary: (bytes: Uint8Array) => { + sent.push(bytes) + } + } + }) + vi.stubGlobal('window', { api: { runtimeEnvironments: { subscribe } } }) + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + async function subscribeStream(terminal: string): Promise { + const stream = await getRemoteRuntimeTerminalMultiplexer('env-1').subscribeTerminal({ + terminal, + client: { id: 'desktop-1', type: 'desktop' }, + callbacks: { onData: () => {}, onSnapshot: () => {} } + }) + await Promise.resolve() + return stream + } + + function inputTextOnWire(): string { + return sent + .map((bytes) => decodeTerminalStreamFrame(bytes)) + .filter((frame) => frame?.opcode === TerminalStreamOpcode.Input) + .map((frame) => (frame ? decodeTerminalStreamText(frame.payload) : '')) + .join('') + } + + function opcodeCount(opcode: TerminalStreamOpcode): number { + return sent.filter((bytes) => decodeTerminalStreamFrame(bytes)?.opcode === opcode).length + } + + it('refuses input and viewport frames from a closed stream while a sibling keeps the socket live', async () => { + const parked = await subscribeStream('terminal-1') + // A sibling stream keeps the multiplexer connected, so `ready` stays true after the close. + await subscribeStream('terminal-2') + + parked.close() + sent = [] + + expect(parked.sendInput('never-delivered\r')).toBe(false) + expect(parked.claimViewport(80, 24)).toBe(false) + expect(parked.resize(80, 24)).toBe(false) + + expect(inputTextOnWire()).toBe('') + expect(opcodeCount(TerminalStreamOpcode.ClaimViewport)).toBe(0) + expect(opcodeCount(TerminalStreamOpcode.Resize)).toBe(0) + }) +})