From 3511f368f81cbb1df3bd259f9bfb76fac9dfd39b Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Thu, 25 Jun 2026 04:15:08 -0700 Subject: [PATCH] Prevent active terminal focus on dead panes and ignore stale PTY events - Redirect active keyboard focus to a live, PTY-backed sibling pane if the focused pane dies, fails to spawn, or is hydrated without a live connection. - Ignore stale data, replay messages, and subscription rejection errors from older PTY sessions after a transport has reconnected. - Avoid persisting stale PTY bindings and dead pane focus inside terminal shutdown layout snapshots. --- .../terminal-pane/pty-connection.test.ts | 51 ++++++++++++++---- .../terminal-pane/pty-connection.ts | 30 +++++++++++ .../remote-runtime-pty-transport.test.ts | 6 +-- .../terminal-shutdown-layout-capture.test.ts | 43 +++++++++++++++ .../terminal-shutdown-layout-capture.ts | 3 +- .../store/slices/terminals-hydration.test.ts | 52 +++++++++++++++++++ src/renderer/src/store/slices/terminals.ts | 15 +++++- 7 files changed, 183 insertions(+), 17 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index 128b1c50f57..9b55e9c8696 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -349,19 +349,29 @@ function createPane(paneId: number) { } } -function createManager(paneCount = 1) { +function createManager(paneCount = 1, initialActivePaneId: number | null = null) { + let activePaneId = initialActivePaneId + const panes = Array.from({ length: paneCount }, (_, index) => ({ + id: index + 1, + leafId: leafIdForPane(index + 1) + })) return { setPaneGpuRendering: vi.fn(), markPaneHasComplexScriptOutput: vi.fn(), rebuildPaneWebgl: vi.fn(), - getPanes: vi.fn(() => - Array.from({ length: paneCount }, (_, index) => ({ - id: index + 1, - leafId: leafIdForPane(index + 1) - })) - ), + getPanes: vi.fn(() => panes), closePane: vi.fn(), - getActivePane: vi.fn<() => { id: number } | null>(() => null) + getActivePane: vi.fn<() => { id: number; leafId?: string } | null>(() => + activePaneId === null + ? null + : (panes.find((candidate) => candidate.id === activePaneId) ?? null) + ), + getNumericIdForLeaf: vi.fn((leafId: string) => { + return panes.find((candidate) => candidate.leafId === leafId)?.id ?? null + }), + setActivePane: vi.fn((paneId: number) => { + activePaneId = paneId + }) } } @@ -851,10 +861,30 @@ describe('connectPanePty', () => { const { connectPanePty } = await import('./pty-connection') const transport = createMockTransport('pty-pane-2') transportFactoryQueue.push(transport) - const manager = createManager(2) + const manager = createManager(2, 2) const deps = createDeps({ restoredLeafId: LEAF_2, - paneTransportsRef: { current: new Map([[1, createMockTransport('pty-pane-1')]]) } + paneTransportsRef: { current: new Map([[1, createMockTransport('pty-pane-1')]]) }, + clearExitedPanePtyLayoutBinding: vi.fn(() => { + mockStoreState = { + ...mockStoreState, + terminalLayoutsByTabId: { + ...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_1, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF_1]: 'pty-pane-1' } + } + } + } + }) }) connectPanePty(createPane(2) as never, manager as never, deps as never) @@ -867,6 +897,7 @@ describe('connectPanePty', () => { expect(deps.clearTabPtyId).toHaveBeenCalledWith('tab-1', 'pty-pane-2') expect(deps.onPtyExitRef.current).not.toHaveBeenCalled() expect(manager.closePane).not.toHaveBeenCalled() + expect(manager.setActivePane).toHaveBeenCalledWith(1, { focus: true }) }) it('closes a split pane when an established PTY exits after output', async () => { diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index b507a816d90..f6a779fd758 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -1293,6 +1293,35 @@ export function connectPanePty( } }) + const focusSurvivingPtyPaneAfterKeptExit = (): void => { + if (manager.getActivePane()?.id !== pane.id) { + return + } + const hasPtyBinding = (paneId: number): boolean => + Boolean(deps.paneTransportsRef.current.get(paneId)?.getPtyId()) + const repairedActiveLeafId = + useAppStore.getState().terminalLayoutsByTabId[deps.tabId]?.activeLeafId ?? null + const repairedActivePaneId = repairedActiveLeafId + ? manager.getNumericIdForLeaf(repairedActiveLeafId) + : null + const targetPaneId = + repairedActivePaneId !== null && + repairedActivePaneId !== pane.id && + hasPtyBinding(repairedActivePaneId) + ? repairedActivePaneId + : (manager + .getPanes() + .find((candidate) => candidate.id !== pane.id && hasPtyBinding(candidate.id))?.id ?? + null) + if (targetPaneId !== null) { + // Why: when a newborn split PTY dies before output/input, the pane stays + // mounted for diagnostics; move live focus to the sibling that still owns a PTY. + manager.setActivePane(targetPaneId, { + focus: deps.isActiveRef.current && deps.isVisibleRef.current + }) + } + } + const onExit = (ptyId: string): void => { const currentPaneTransport = deps.paneTransportsRef.current.get(pane.id) if (currentPaneTransport && currentPaneTransport !== transport) { @@ -1346,6 +1375,7 @@ export function connectPanePty( ) { // Why: a freshly split pane can lose its newborn PTY during setup; keep // the split visible so the failed session does not immediately collapse. + focusSurvivingPtyPaneAfterKeptExit() return } manager.closePane(pane.id) diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts index 4d835b6dd7d..82a2369fa13 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts @@ -343,9 +343,7 @@ describe('createRemoteRuntimePtyTransport', () => { }) it('ignores stale attach subscription rejection after reattaching a newer remote terminal', async () => { - let rejectOldSubscription: (error: Error) => void = () => { - throw new Error('old subscription reject was not captured') - } + let rejectOldSubscription: ((error: Error) => void) | null = null const newStream = { streamId: 2, sendInput: vi.fn(() => true), @@ -389,7 +387,7 @@ describe('createRemoteRuntimePtyTransport', () => { }) await vi.waitFor(() => expect(subscribeTerminal).toHaveBeenCalledTimes(2)) - rejectOldSubscription(new Error('terminal_handle_stale')) + rejectOldSubscription?.(new Error('terminal_handle_stale')) await Promise.resolve() await Promise.resolve() diff --git a/src/renderer/src/components/terminal-pane/terminal-shutdown-layout-capture.test.ts b/src/renderer/src/components/terminal-pane/terminal-shutdown-layout-capture.test.ts index d3e81348487..07770304a26 100644 --- a/src/renderer/src/components/terminal-pane/terminal-shutdown-layout-capture.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-shutdown-layout-capture.test.ts @@ -366,4 +366,47 @@ describe('captureTerminalShutdownLayout', () => { expect(layout.activeLeafId).toBe(LEAF_ID_2) expect(layout.ptyIdsByLeafId).toEqual({ [LEAF_ID_2]: 'pty-2' }) }) + + it('does not preserve a stale prior PTY binding for active shutdown focus', async () => { + const { captureTerminalShutdownLayout } = await import('./terminal-shutdown-layout-capture') + const paneWithoutPty = { + id: 1, + leafId: LEAF_ID, + stablePaneId: LEAF_ID, + terminal: { options: { scrollback: 1_000 } }, + serializeAddon: { + serialize: vi.fn(() => '') + } + } + const paneWithPty = { + id: 2, + leafId: LEAF_ID_2, + stablePaneId: LEAF_ID_2, + terminal: { options: { scrollback: 1_000 } }, + serializeAddon: { + serialize: vi.fn(() => '') + } + } + const manager = { + getPanes: vi.fn(() => [paneWithoutPty, paneWithPty]), + getActivePane: vi.fn(() => paneWithoutPty) + } + + const layout = captureTerminalShutdownLayout({ + manager: manager as never, + container: mockRootForSplit(), + expandedPaneId: null, + paneTransports: new Map([[2, { getPtyId: vi.fn(() => 'pty-2') }]]), + paneTitlesByPaneId: {}, + existingLayout: { + root: null, + activeLeafId: LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF_ID]: 'stale-pty', [LEAF_ID_2]: 'pty-2' } + } + }) + + expect(layout.activeLeafId).toBe(LEAF_ID_2) + expect(layout.ptyIdsByLeafId).toEqual({ [LEAF_ID_2]: 'pty-2' }) + }) }) diff --git a/src/renderer/src/components/terminal-pane/terminal-shutdown-layout-capture.ts b/src/renderer/src/components/terminal-pane/terminal-shutdown-layout-capture.ts index f38dbd47648..c0ee1de1713 100644 --- a/src/renderer/src/components/terminal-pane/terminal-shutdown-layout-capture.ts +++ b/src/renderer/src/components/terminal-pane/terminal-shutdown-layout-capture.ts @@ -133,7 +133,8 @@ export function captureTerminalShutdownLayout({ }) const ptyIdsByLeafId = { ...preservedPtyIdsByLeafId, ...livePtyIdsByLeafId } // Why: shutdown snapshots can otherwise persist focus on a mounted pane whose - // transport was already cleared during PTY exit/reconnect cleanup. + // transport was already cleared during PTY exit/reconnect cleanup. Unlike + // scrollback, PTY bindings only preserve prior ids during a live attach gap. layout.activeLeafId = resolveTerminalLayoutActiveLeafId({ root: layout.root, activeLeafId: layout.activeLeafId, diff --git a/src/renderer/src/store/slices/terminals-hydration.test.ts b/src/renderer/src/store/slices/terminals-hydration.test.ts index 5fc2323873e..082184dfdaf 100644 --- a/src/renderer/src/store/slices/terminals-hydration.test.ts +++ b/src/renderer/src/store/slices/terminals-hydration.test.ts @@ -139,6 +139,58 @@ describe('hydrateWorkspaceSession', () => { }) }) + it('moves restored active focus from a dead split leaf to a pty-backed sibling', () => { + const store = createTestStore() + const worktreeId = 'repo1::/wt-1' + const liveLeftLeafId = '9ee09218-72a5-4e1c-b075-729e937d4e29' + const liveRightLeafId = 'f5fc66b1-ec43-404b-b7b0-a06f0db34940' + const deadActiveLeafId = 'fbf63fd9-34d6-4387-9109-562f7c02bc4c' + seedStore(store, { + worktreesByRepo: { + repo1: [makeWorktree({ id: worktreeId, repoId: 'repo1', path: '/wt-1' })] + } + }) + + const session: WorkspaceSessionState = { + activeRepoId: 'repo1', + activeWorktreeId: worktreeId, + activeTabId: 'tab-1', + tabsByWorktree: { + [worktreeId]: [makeTab({ id: 'tab-1', worktreeId, ptyId: 'old-pty' })] + }, + terminalLayoutsByTabId: { + 'tab-1': { + root: { + type: 'split', + direction: 'horizontal', + first: { type: 'leaf', leafId: liveLeftLeafId }, + second: { + type: 'split', + direction: 'vertical', + first: { type: 'leaf', leafId: liveRightLeafId }, + second: { type: 'leaf', leafId: deadActiveLeafId } + } + }, + activeLeafId: deadActiveLeafId, + expandedLeafId: null, + ptyIdsByLeafId: { + [liveLeftLeafId]: 'daemon-session-left', + [liveRightLeafId]: 'daemon-session-right' + }, + buffersByLeafId: { + [deadActiveLeafId]: 'retained scrollback' + } + } + } + } + + store.getState().hydrateWorkspaceSession(session) + + // Why: restart can preserve scrollback for an exited pane while live siblings + // reattach. Keyboard focus must land on a PTY-backed pane, not the dead leaf. + expect(store.getState().terminalLayoutsByTabId['tab-1']?.activeLeafId).toBe(liveLeftLeafId) + }) + it('hydrates floating terminal tabs even though they are not repo worktrees', () => { const store = createTestStore() const session: WorkspaceSessionState = { diff --git a/src/renderer/src/store/slices/terminals.ts b/src/renderer/src/store/slices/terminals.ts index b99b248a503..d997f1dcba3 100644 --- a/src/renderer/src/store/slices/terminals.ts +++ b/src/renderer/src/store/slices/terminals.ts @@ -44,7 +44,10 @@ import { restorePtyDataHandlersAfterFailedShutdown, unregisterPtyDataHandlers } from '@/components/terminal-pane/pty-transport' -import { normalizeTerminalLayoutSnapshot } from '@/components/terminal-pane/terminal-layout-leaf-ids' +import { + normalizeTerminalLayoutSnapshot, + resolvePtyBoundActiveLeafId +} from '@/components/terminal-pane/terminal-layout-leaf-ids' import { shutdownBufferCaptures } from '@/components/terminal-pane/shutdown-buffer-captures' import { callRuntimeRpc } from '@/runtime/runtime-rpc-client' import { parseRemoteRuntimePtyId } from '@/runtime/runtime-terminal-stream' @@ -2780,7 +2783,15 @@ export const createTerminalSlice: StateCreator const tab = Object.values(tabsByWorktree) .flat() .find((entry) => entry.id === tabId) - return [tabId, tab ? sanitizeTerminalLayoutPaneTitles(normalized, tab) : normalized] + const sanitized = tab ? sanitizeTerminalLayoutPaneTitles(normalized, tab) : normalized + const activeLeafId = sanitized.root + ? resolvePtyBoundActiveLeafId({ + root: sanitized.root, + activeLeafId: sanitized.activeLeafId, + ptyIdsByLeafId: sanitized.ptyIdsByLeafId + }) + : sanitized.activeLeafId + return [tabId, { ...sanitized, activeLeafId }] }) ) }