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
2026-11-01 01:27:35 +00:00 · 2026-10-15 23:26:20 -07:00
parent 0c8303c210
commit cd69443166
2 changed files with 222 additions and 20 deletions
@@ -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<void>((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<void>((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)
+  })
+})
@@ -1064,6 +1064,27 @@ export function createRemoteRuntimePtyTransport(
    }
  }

+  async function resolveHostSessionPane(
+    hostTabId: string
+  ): Promise<RuntimeTerminalResolvePane | null> {
+    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') {