From cd69443166afd39e2666505fc76fb83f0bf0a226 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 15 Oct 2026 23:26:20 -0700 Subject: [PATCH] perf(ssh): a parked web-mirror reveal reads its pane alongside the activation (#29472) * perf(ssh): a web-mirror reveal reads its pane alongside the activation, not after it * fix(ssh): an early pane read never erases an incarnation the activation learned --- ...y-transport-mirror-resolve-overlap.test.ts | 179 ++++++++++++++++++ .../remote-runtime-pty-transport.ts | 63 ++++-- 2 files changed, 222 insertions(+), 20 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-mirror-resolve-overlap.test.ts diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-mirror-resolve-overlap.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-mirror-resolve-overlap.test.ts new file mode 100644 index 00000000000..31d00164fbe --- /dev/null +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-mirror-resolve-overlap.test.ts @@ -0,0 +1,179 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + createRemoteRuntimeTransportMocks, + type MultiplexSubscriptionCallbacks +} from './remote-runtime-pty-transport-test-harness' + +// A parked web-mirror reveal used to wait for session.tabs.activate before sending +// terminal.resolvePane, a whole extra round trip before Subscribe. The pane read has no +// data dependency on the activation, so it now overlaps it; a pending surface that only +// the activation materializes is still re-read afterwards. + +vi.mock('@/store', () => ({ useAppStore: { getState: () => ({}) } })) + +let subscriptionCallbacks: MultiplexSubscriptionCallbacks = null +let resolvedPaneHandle = 'terminal-1' + +const { runtimeCall, subscriptionSendBinary, resetRemoteRuntimeTransport } = + createRemoteRuntimeTransportMocks({ + getCallbacks: () => subscriptionCallbacks, + setCallbacks: (callbacks) => { + subscriptionCallbacks = callbacks + }, + getResolvedPaneHandle: () => resolvedPaneHandle, + setResolvedPaneHandle: (handle) => { + resolvedPaneHandle = handle + } + }) + +function calledMethods(): string[] { + return runtimeCall.mock.calls.map(([request]) => request.method) +} + +describe('web mirror reveal: resolvePane overlaps the activation', () => { + beforeEach(() => { + resetRemoteRuntimeTransport() + resolvedPaneHandle = 'terminal-1' + }) + + async function attachMirror() { + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'web-terminal-host-tab-1', + leafId: 'pane:1' + }) + return transport.connect({ + url: '', + sessionId: 'remote:env-1@@terminal-1', + cols: 100, + rows: 30, + callbacks: {} + }) + } + + it('sends resolvePane before the activation answers, and reads it once', async () => { + const defaultCall = runtimeCall.getMockImplementation()! + let answerActivate: () => void = () => {} + runtimeCall.mockImplementation((request: { method: string }) => { + if (request.method === 'session.tabs.activate') { + return new Promise((resolve) => { + answerActivate = () => resolve(defaultCall(request)) + }) + } + if (request.method === 'terminal.resolvePane') { + return Promise.resolve(defaultCall(request)).then((response) => ({ + ...response, + result: { + terminal: { ...response.result.terminal, incarnationId: 'incarnation-resolved' } + } + })) + } + return defaultCall(request) + }) + + const attached = attachMirror() + await vi.waitFor(() => expect(calledMethods()).toContain('terminal.resolvePane'), { + timeout: 10_000 + }) + // The activation has not answered, yet the pane read is already on the wire. + expect(calledMethods()).toContain('session.tabs.activate') + expect(subscriptionSendBinary).not.toHaveBeenCalled() + + answerActivate() + await expect(attached).resolves.toMatchObject({ + id: 'remote:env-1@@terminal-1', + incarnationId: 'incarnation-resolved' + }) + expect(calledMethods().filter((method) => method === 'terminal.resolvePane')).toHaveLength(1) + }) + + it('re-reads the pane after an activation that materialized a different handle', async () => { + const defaultCall = runtimeCall.getMockImplementation()! + let activated = false + let earlyReadAnswered: () => void = () => {} + const earlyRead = new Promise((resolve) => { + earlyReadAnswered = resolve + }) + runtimeCall.mockImplementation(async (request: { method: string }) => { + if (request.method === 'session.tabs.activate') { + await earlyRead + // The surface was pending; activating it publishes the real PTY handle. + resolvedPaneHandle = 'terminal-2' + activated = true + return defaultCall(request) + } + if (request.method === 'terminal.resolvePane' && !activated) { + queueMicrotask(earlyReadAnswered) + return { ok: false, error: { code: 'runtime_error', message: 'terminal_not_found' } } + } + if (request.method === 'terminal.resolvePane') { + const response = await defaultCall(request) + return { + ...response, + result: { + terminal: { ...response.result.terminal, incarnationId: 'incarnation-after-activate' } + } + } + } + return defaultCall(request) + }) + + await expect(attachMirror()).resolves.toMatchObject({ + id: 'remote:env-1@@terminal-2', + incarnationId: 'incarnation-after-activate' + }) + expect(calledMethods().slice(0, 3)).toEqual([ + 'terminal.resolvePane', + 'session.tabs.activate', + 'terminal.resolvePane' + ]) + }) + + it('re-reads a pane answered before the activation learned its incarnation', async () => { + const defaultCall = runtimeCall.getMockImplementation()! + let activated = false + let earlyReadAnswered: () => void = () => {} + const earlyRead = new Promise((resolve) => { + earlyReadAnswered = resolve + }) + runtimeCall.mockImplementation(async (request: { method: string }) => { + if (request.method === 'session.tabs.activate') { + await earlyRead + activated = true + const response = await defaultCall(request) + return { + ...response, + result: { + ...response.result, + tabs: response.result.tabs.map((tab: object) => ({ + ...tab, + incarnationId: 'incarnation-learned' + })) + } + } + } + if (request.method === 'terminal.resolvePane') { + const response = await defaultCall(request) + if (!activated) { + queueMicrotask(earlyReadAnswered) + return response + } + return { + ...response, + result: { + terminal: { ...response.result.terminal, incarnationId: 'incarnation-learned' } + } + } + } + return defaultCall(request) + }) + + // The same handle answered early without an incarnation must not erase the learned one. + await expect(attachMirror()).resolves.toMatchObject({ + id: 'remote:env-1@@terminal-1', + incarnationId: 'incarnation-learned' + }) + expect(calledMethods().filter((method) => method === 'terminal.resolvePane')).toHaveLength(2) + }) +}) 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 2e7da5b0073..63e641e2af7 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 @@ -1064,6 +1064,27 @@ export function createRemoteRuntimePtyTransport( } } + async function resolveHostSessionPane( + hostTabId: string + ): Promise { + if (!leafId || !worktreeId || resolvePaneUnavailable) { + return null + } + try { + const resolved = await callRuntime<{ terminal: RuntimeTerminalResolvePane }>( + 'terminal.resolvePane', + { paneKey: `${hostTabId}:${leafId}`, worktreeId } + ) + // Why: wire data; an answer without a pane must read as unresolved, not throw past this catch. + return resolved.terminal ?? null + } catch (error) { + if (error instanceof RuntimeRpcCallError && error.code === 'method_not_found') { + resolvePaneUnavailable = true + } + return null + } + } + async function attachHostSessionMirror( options: { cols?: number; rows?: number }, notifySpawn = true, @@ -1079,6 +1100,8 @@ export function createRemoteRuntimePtyTransport( (expectedLifecycleEpoch === undefined || expectedLifecycleEpoch === lifecycleEpoch) const hostTabId = toHostSessionTabId(tabId) const waitOutcome = { boundedWaitEnded: false } + // Why overlap: resolvePane is a read keyed only by the pane, so it need not wait an activation round trip. + const earlyPaneResolution = resolveHostSessionPane(hostTabId) const hostHandle = await waitForHostSessionHandleWithRecovery(hostTabId, isCurrent, waitOutcome) if (!isCurrent()) { return undefined @@ -1112,26 +1135,26 @@ export function createRemoteRuntimePtyTransport( return undefined } - if (leafId && worktreeId && !resolvePaneUnavailable) { - try { - const resolved = await callRuntime<{ terminal: RuntimeTerminalResolvePane }>( - 'terminal.resolvePane', - { paneKey: `${hostTabId}:${leafId}`, worktreeId } - ) - const terminal = resolved.terminal - if ( - terminal.handle === hostHandle && - terminal.tabId === hostTabId && - terminal.leafId === leafId && - (!terminal.worktreeId || terminal.worktreeId === worktreeId) - ) { - adoptExecutionMetadata(terminal) - } - } catch (error) { - if (error instanceof RuntimeRpcCallError && error.code === 'method_not_found') { - resolvePaneUnavailable = true - } - } + const matchesHostHandle = ( + terminal: RuntimeTerminalResolvePane | null + ): terminal is RuntimeTerminalResolvePane => + terminal !== null && + terminal.handle === hostHandle && + terminal.tabId === hostTabId && + terminal.leafId === leafId && + (!terminal.worktreeId || terminal.worktreeId === worktreeId) + let resolvedPane = await earlyPaneResolution + const activationIncarnationId = authoritativePtyIncarnationId + if ( + !matchesHostHandle(resolvedPane) || + (activationIncarnationId !== null && + (resolvedPane.incarnationId ?? null) !== activationIncarnationId) + ) { + // Why: activation can materialize the surface or learn its incarnation, so an earlier read may be stale. + resolvedPane = await resolveHostSessionPane(hostTabId) + } + if (matchesHostHandle(resolvedPane)) { + adoptExecutionMetadata(resolvedPane) } if (!isCurrent() || recovery.currentPhase === 'disconnected') {