fix(terminal): reject stale pane transport callbacks

This commit is contained in:
Neil
2026-08-31 05:43:12 -07:00
parent 963d0cfe73
commit 1b3d75c7b9
3 changed files with 102 additions and 3 deletions
@@ -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 }
@@ -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
}
@@ -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()