From 1b3d75c7b9cf515032fb3a8d8d9852f8a8cf2ce4 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 31 Aug 2026 04:34:08 -0700 Subject: [PATCH] fix(terminal): reject stale pane transport callbacks --- .../pty-connection-pty-exit-teardown.test.ts | 87 ++++++++++++++++++- .../pane-pty-visibility-bind.ts | 12 ++- .../pty-connection/reattach-result-handler.ts | 6 ++ 3 files changed, 102 insertions(+), 3 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/pty-connection-pty-exit-teardown.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-pty-exit-teardown.test.ts index 23e521bd0eb..10d9bb95193 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-pty-exit-teardown.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-pty-exit-teardown.test.ts @@ -735,7 +735,11 @@ describe('connectPanePty', () => { // The fixture dependency is intentionally lightweight, so mirror the live // tab/layout commit that the real store performs atomically on replacement. mockStoreState.tabsByWorktree['wt-1'][0]!.ptyId = 'terminal-reconnected' - mockStoreState.terminalLayoutsByTabId['tab-1']!.ptyIdsByLeafId![LEAF_1] = 'terminal-reconnected' + const replacementLayout = mockStoreState.terminalLayoutsByTabId?.['tab-1'] + if (!replacementLayout) { + throw new Error('test fixture missing terminal layout') + } + replacementLayout.ptyIdsByLeafId![LEAF_1] = 'terminal-reconnected' onPtySpawn?.('terminal-old') @@ -743,6 +747,87 @@ describe('connectPanePty', () => { expect(deps.syncPanePtyLayoutBinding).not.toHaveBeenLastCalledWith(1, 'terminal-old') }) + it('ignores a late split-pane spawn callback when the tab identity belongs to pane one', async () => { + const { connectPanePty } = await import('./pty-connection') + let transportPtyId = 'terminal-old' + const transport = createMockTransport(transportPtyId) + transport.getPtyId = vi.fn(() => transportPtyId) + transportFactoryQueue.push(transport) + const manager = createManager(2, 2) + const deps = createDeps() + const pane = createPane(2) + mockStoreState.terminalLayoutsByTabId = { + 'tab-1': { + root: { + type: 'split', + direction: 'horizontal', + first: { type: 'leaf', leafId: LEAF_1 }, + second: { type: 'leaf', leafId: LEAF_2 }, + ratio: 0.5 + }, + activeLeafId: LEAF_2, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF_1]: 'tab-pty', [LEAF_2]: 'terminal-old' } + } + } + + connectPanePty(pane as never, manager as never, deps as never) + const onPtySpawn = createdTransportOptions[0]?.onPtySpawn as + | ((ptyId: string) => void) + | undefined + const onPtyRebind = createdTransportOptions[0]?.onPtyRebind as + | ((ptyId: string, replacedPtyId: string) => void) + | undefined + expect(onPtySpawn).toBeTypeOf('function') + expect(onPtyRebind).toBeTypeOf('function') + + onPtySpawn?.('terminal-old') + transportPtyId = 'terminal-reconnected' + onPtyRebind?.('terminal-reconnected', 'terminal-old') + + // The tab-level PTY is pane one's fallback; pane two is represented by its leaf binding. + expect(mockStoreState.tabsByWorktree['wt-1'][0]?.ptyId).toBe('tab-pty') + expect(mockStoreState.terminalLayoutsByTabId?.['tab-1']?.ptyIdsByLeafId?.[LEAF_2]).toBe( + 'terminal-reconnected' + ) + + onPtySpawn?.('terminal-old') + + expect(pane.container.dataset.ptyId).toBe('terminal-reconnected') + expect(deps.syncPanePtyLayoutBinding).not.toHaveBeenLastCalledWith(2, 'terminal-old') + }) + + it('accepts a fresh spawn when the persisted pane still names the retired PTY', async () => { + const { connectPanePty } = await import('./pty-connection') + let transportPtyId = 'terminal-new' + const transport = createMockTransport(transportPtyId) + transport.getPtyId = vi.fn(() => transportPtyId) + transportFactoryQueue.push(transport) + const manager = createManager(1) + const deps = createDeps() + const pane = createPane(1) + mockStoreState.tabsByWorktree['wt-1'] = [{ id: 'tab-1', ptyId: 'terminal-old' }] + const terminalLayoutsByTabId = + mockStoreState.terminalLayoutsByTabId ?? (mockStoreState.terminalLayoutsByTabId = {}) + terminalLayoutsByTabId['tab-1'] = { + root: { type: 'leaf', leafId: LEAF_1 }, + activeLeafId: LEAF_1, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF_1]: 'terminal-old' } + } + + connectPanePty(pane as never, manager as never, deps as never) + const onPtySpawn = createdTransportOptions[0]?.onPtySpawn as + | ((ptyId: string) => void) + | undefined + expect(onPtySpawn).toBeTypeOf('function') + + onPtySpawn?.('terminal-new') + + expect(pane.container.dataset.ptyId).toBe('terminal-new') + expect(deps.syncPanePtyLayoutBinding).toHaveBeenLastCalledWith(1, 'terminal-new') + }) + it('closes a split pane when an established PTY exits after output', async () => { const { connectPanePty } = await import('./pty-connection') const capturedDataCallback: { current: ((data: string) => void) | null } = { current: null } diff --git a/src/renderer/src/components/terminal-pane/pty-connection/pane-pty-visibility-bind.ts b/src/renderer/src/components/terminal-pane/pty-connection/pane-pty-visibility-bind.ts index ef6f3062c94..f905a87f42e 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/pane-pty-visibility-bind.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/pane-pty-visibility-bind.ts @@ -49,12 +49,20 @@ export function installPanePtyVisibilityBind(session: ConnectPanePtySession): vo const tabPtyId = Object.values(state.tabsByWorktree) .flat() .find((tab) => tab.id === session.deps.tabId)?.ptyId + const activePtyId = session.activePanePtyBinding + const isCurrentPaneTransport = + session.deps.paneTransportsRef.current.get(session.pane.id) === session.transport + const isStaleTransportBinding = + !isCurrentPaneTransport || (activePtyId !== null && session.transport.getPtyId() !== ptyId) + const isInitialCurrentTransportBinding = isCurrentPaneTransport && activePtyId === null if ( - shouldIgnoreStalePanePtyLayoutBinding({ + isStaleTransportBinding || + (shouldIgnoreStalePanePtyLayoutBinding({ existingPtyId, nextPtyId: ptyId, tabPtyId - }) + }) && + !isInitialCurrentTransportBinding) ) { return } diff --git a/src/renderer/src/components/terminal-pane/pty-connection/reattach-result-handler.ts b/src/renderer/src/components/terminal-pane/pty-connection/reattach-result-handler.ts index 6ab9dc0769b..22ca145cada 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/reattach-result-handler.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/reattach-result-handler.ts @@ -23,6 +23,8 @@ type ReattachResultSession = ReattachPayloadSession & Pick< ConnectPanePtySession, | 'agentCompletionCoordinator' + | 'activePanePtyBinding' + | 'activePanePtyBindingBoundAt' | 'authoritativeReattachGeneration' | 'capturedDirectSshRetryPtyAccepted' | 'cacheKey' @@ -168,6 +170,10 @@ export function bindHandleReattachResult(sessionBag: ConnectPanePtySession): voi return false } session.setPanePtyFitBinding(ptyId) + // Keep the session-local identity in step with the transport before any + // queued spawn callback can arrive during replay. + session.activePanePtyBinding = ptyId + session.activePanePtyBindingBoundAt = performance.now() session.reportPanePtyVisibility(ptyId, session.deps.isVisibleRef.current) session.registerSideEffectFactConsumerForPty(ptyId) session.syncHiddenRendererPtyDelivery()