diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 3aa0b1f5688..baabaf806f6 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -10708,6 +10708,95 @@ describe('OrcaRuntimeService', () => { ]) }) + it('selects a visible active pane when terminal visual layout prunes a stale leaf', async () => { + const runtime = new OrcaRuntimeService(store) + const parentLayout: TerminalLayoutSnapshot = { + root: { + type: 'split', + direction: 'vertical', + first: { type: 'leaf', leafId: 'pane:1' }, + second: { type: 'leaf', leafId: 'pane:2' } + }, + activeLeafId: 'pane:1', + expandedLeafId: null, + ptyIdsByLeafId: { 'pane:2': 'pty-2' } + } + runtime.attachWindow(1) + runtime.syncWindowGraph(1, { + tabs: [ + { + tabId: 'tab-1', + worktreeId: TEST_WORKTREE_ID, + title: 'Split terminal', + activeLeafId: 'pane:1', + layout: null + } + ], + leaves: [ + { + tabId: 'tab-1', + worktreeId: TEST_WORKTREE_ID, + leafId: 'pane:2', + paneRuntimeId: 2, + ptyId: 'pty-2', + paneTitle: 'Live pane' + } + ], + mobileSessionTabs: [ + { + worktree: TEST_WORKTREE_ID, + publicationEpoch: 'epoch-1', + snapshotVersion: 1, + activeGroupId: null, + activeTabId: 'tab-1::pane:1', + activeTabType: 'terminal', + tabs: [ + { + type: 'terminal', + id: 'tab-1::pane:1', + parentTabId: 'tab-1', + leafId: 'pane:1', + title: 'Stale pane', + parentLayout, + isActive: true + }, + { + type: 'terminal', + id: 'tab-1::pane:2', + parentTabId: 'tab-1', + leafId: 'pane:2', + title: 'Live pane', + parentLayout, + isActive: false + } + ] + } + ] + }) + + const result = await runtime.listTerminals(`id:${TEST_WORKTREE_ID}`) + + expect(result.visualLayouts).toMatchObject([ + { + worktreeId: TEST_WORKTREE_ID, + root: { + type: 'group', + tabs: [ + { + tabId: 'tab-1', + activeLeafId: 'pane:2', + panes: { + type: 'terminal', + leafId: 'pane:2', + active: true + } + } + ] + } + } + ]) + }) + it('omits stale browser session tabs that no longer have live webContents', async () => { const runtime = new OrcaRuntimeService(store) const tabList = vi.fn(() => ({ diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 4c421345834..b72a56816f3 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -7942,7 +7942,7 @@ export class OrcaRuntimeService { return null } const parentTabId = firstSurface.parentTabId - const activeLeafId = + const requestedActiveLeafId = firstSurface.parentLayout?.activeLeafId ?? surfaces.find((surface) => surface.isActive)?.leafId ?? firstSurface.leafId @@ -7950,6 +7950,15 @@ export class OrcaRuntimeService { type: 'leaf' as const, leafId: firstSurface.leafId } + const visibleLeafIds = this.collectVisibleTerminalLeafIds(root, parentTabId, summariesByLeafKey) + if (visibleLeafIds.length === 0) { + return null + } + const activeLeafId = + (requestedActiveLeafId && visibleLeafIds.includes(requestedActiveLeafId) + ? requestedActiveLeafId + : surfaces.find((surface) => surface.isActive && visibleLeafIds.includes(surface.leafId)) + ?.leafId) ?? visibleLeafIds[0]! const panes = this.buildTerminalVisualPane(root, parentTabId, activeLeafId, summariesByLeafKey) if (!panes) { return null @@ -7962,6 +7971,20 @@ export class OrcaRuntimeService { } } + private collectVisibleTerminalLeafIds( + node: TerminalPaneLayoutNode, + tabId: string, + summariesByLeafKey: ReadonlyMap + ): string[] { + if (node.type === 'leaf') { + return summariesByLeafKey.has(this.getLeafKey(tabId, node.leafId)) ? [node.leafId] : [] + } + return [ + ...this.collectVisibleTerminalLeafIds(node.first, tabId, summariesByLeafKey), + ...this.collectVisibleTerminalLeafIds(node.second, tabId, summariesByLeafKey) + ] + } + private buildTerminalVisualPane( node: TerminalPaneLayoutNode, tabId: string, diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index b8e8cf1ab84..2d8745e71e9 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -840,8 +840,8 @@ export default function TerminalPane({ persistLayoutSnapshot() }, [paneCount, paneTitles, persistLayoutSnapshot, terminalTab]) - const syncPanePtyLayoutBinding = useCallback( - (paneId: number, ptyId: string | null): void => { + const writePanePtyLayoutBinding = useCallback( + (paneId: number, ptyId: string | null, repairActiveLeafOnClear: boolean): void => { const existingLayout = useAppStore.getState().terminalLayoutsByTabId[tabId] ?? EMPTY_LAYOUT const { ptyIdsByLeafId: _existingPtyIdsByLeafId, ...layoutWithoutPtyBindings } = existingLayout @@ -868,14 +868,36 @@ export default function TerminalPane({ const nextBindings = { ...existingBindings } delete nextBindings[leafId] - setTabLayout(tabId, { + const nextLayout = { ...layoutWithoutPtyBindings, ...(Object.keys(nextBindings).length > 0 ? { ptyIdsByLeafId: nextBindings } : {}) - }) + } + if ( + repairActiveLeafOnClear && + existingLayout.activeLeafId === leafId && + Object.keys(nextBindings).length > 0 + ) { + // Why: an active pane that lost its PTY would keep swallowing input if + // sibling bound panes are available; replacement/restart bookkeeping + // opts out so focus stays with the pane about to receive a fresh PTY. + nextLayout.activeLeafId = resolveTerminalLayoutActiveLeafId({ + root: nextLayout.root, + activeLeafId: nextLayout.activeLeafId, + ptyIdsByLeafId: nextBindings + }) + } + setTabLayout(tabId, nextLayout) }, [setTabLayout, tabId] ) + const syncPanePtyLayoutBinding = useCallback( + (paneId: number, ptyId: string | null): void => { + writePanePtyLayoutBinding(paneId, ptyId, false) + }, + [writePanePtyLayoutBinding] + ) + const clearExitedPanePtyLayoutBinding = useCallback( (paneId: number, exitedPtyId: string): void => { const existingLayout = useAppStore.getState().terminalLayoutsByTabId[tabId] ?? EMPTY_LAYOUT @@ -1152,11 +1174,13 @@ export default function TerminalPane({ persistLayoutSnapshot() } - if (restoredLayout.activeLeafId) { - const activePaneId = manager.getNumericIdForLeaf(restoredLayout.activeLeafId) - if (activePaneId !== null) { - manager.setActivePane(activePaneId, { focus: isActive }) - } + const activePaneId = restoredLayout.activeLeafId + ? manager.getNumericIdForLeaf(restoredLayout.activeLeafId) + : null + const fallbackActivePaneId = manager.getActivePane()?.id ?? manager.getPanes()[0]?.id ?? null + const nextActivePaneId = activePaneId ?? fallbackActivePaneId + if (nextActivePaneId !== null) { + manager.setActivePane(nextActivePaneId, { focus: isActive }) } }, [isActive, paneCount, persistLayoutSnapshot, restoredLayout]) @@ -1302,6 +1326,7 @@ export default function TerminalPane({ }, [ clearCodexRestartNotice, + clearExitedPanePtyLayoutBinding, clearRuntimePaneTitle, clearTabPtyId, cwd, 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 ac1c52a4174..128b1c50f57 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -4090,6 +4090,28 @@ describe('connectPanePty', () => { expect(mockStoreState.clearAgentLaunchConfig).toHaveBeenCalledWith(paneKey) }) + it('ignores a late exit from a transport that no longer owns the pane', async () => { + const { connectPanePty } = await import('./pty-connection') + const oldTransport = createMockTransport('old-pty') + const replacementTransport = createMockTransport('new-pty') + transportFactoryQueue.push(oldTransport) + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps() + + connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks() + deps.paneTransportsRef.current.set(pane.id, replacementTransport) + + const onPtyExit = createdTransportOptions[0]?.onPtyExit as ((ptyId: string) => void) | undefined + onPtyExit?.('old-pty') + + expect(deps.syncPanePtyLayoutBinding).not.toHaveBeenCalledWith(1, null) + expect(deps.clearTabPtyId).toHaveBeenCalledWith('tab-1', 'old-pty') + expect(deps.consumeSuppressedPtyExit).toHaveBeenCalledWith('old-pty') + expect(manager.closePane).not.toHaveBeenCalled() + }) + it('clears launch config when an agent startup spawn produces no PTY', async () => { const { connectPanePty } = await import('./pty-connection') const transport = createMockTransport() diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index e6930cfbd9d..b507a816d90 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -1294,6 +1294,15 @@ export function connectPanePty( }) const onExit = (ptyId: string): void => { + const currentPaneTransport = deps.paneTransportsRef.current.get(pane.id) + if (currentPaneTransport && currentPaneTransport !== transport) { + // Why: an old transport can deliver a late exit after this pane has + // rebound to a replacement PTY; only clear ownership for the exited id. + deps.clearTabPtyId(deps.tabId, ptyId) + deps.consumeSuppressedPtyExit(ptyId) + scheduleRuntimeGraphSync() + return + } agentCompletionCoordinator.dispose() clearPanePtyFitBinding() const isSuppressedExit = deps.consumeSuppressedPtyExit(ptyId) diff --git a/src/renderer/src/components/terminal-pane/terminal-layout-leaf-ids.test.ts b/src/renderer/src/components/terminal-pane/terminal-layout-leaf-ids.test.ts index 9be2f7d8e8d..5f05376fd58 100644 --- a/src/renderer/src/components/terminal-pane/terminal-layout-leaf-ids.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-layout-leaf-ids.test.ts @@ -1,13 +1,15 @@ import { describe, expect, it } from 'vitest' -import type { TerminalPaneLayoutNode } from '../../../../shared/types' +import type { TerminalLayoutSnapshot, TerminalPaneLayoutNode } from '../../../../shared/types' import { normalizeTerminalLayoutSnapshot, + resolvePtyBoundActiveLeafId, resolveTerminalLayoutActiveLeafId } from './terminal-layout-leaf-ids' const LEAF_1 = '11111111-1111-4111-8111-111111111111' const LEAF_2 = '22222222-2222-4222-8222-222222222222' const LEAF_3 = '33333333-3333-4333-8333-333333333333' +const MISSING_LEAF = '99999999-9999-4999-8999-999999999999' function split(firstLeafId: string, secondLeafId: string): TerminalPaneLayoutNode { return { @@ -18,6 +20,29 @@ function split(firstLeafId: string, secondLeafId: string): TerminalPaneLayoutNod } } +function splitLayout(): TerminalLayoutSnapshot { + return { + root: { + type: 'split', + direction: 'vertical', + first: { type: 'leaf', leafId: LEAF_1 }, + second: { + type: 'split', + direction: 'horizontal', + first: { type: 'leaf', leafId: LEAF_2 }, + second: { type: 'leaf', leafId: LEAF_3 } + } + }, + activeLeafId: LEAF_2, + expandedLeafId: LEAF_3, + ptyIdsByLeafId: { + [LEAF_1]: 'pty-1', + [LEAF_2]: 'pty-2', + [LEAF_3]: 'pty-3' + } + } +} + describe('resolveTerminalLayoutActiveLeafId', () => { it('keeps the active leaf when it is still PTY-bound', () => { expect( @@ -128,4 +153,88 @@ describe('normalizeTerminalLayoutSnapshot active leaf repair', () => { expect(result.snapshot.activeLeafId).toBe(LEAF_2) expect(result.snapshot.ptyIdsByLeafId).toEqual({ [LEAF_2]: 'pty-2' }) }) + + it('repairs stale active leaf ids to the first bound leaf in the root layout', () => { + const layout = splitLayout() + layout.activeLeafId = MISSING_LEAF + + const normalized = normalizeTerminalLayoutSnapshot(layout) + + expect(normalized.changed).toBe(true) + expect(normalized.snapshot.activeLeafId).toBe(LEAF_1) + expect(normalized.snapshot.expandedLeafId).toBe(LEAF_3) + expect(normalized.snapshot.ptyIdsByLeafId).toEqual(layout.ptyIdsByLeafId) + }) + + it('clears stale expanded leaf ids without changing a valid active leaf', () => { + const layout = splitLayout() + layout.expandedLeafId = MISSING_LEAF + + const normalized = normalizeTerminalLayoutSnapshot(layout) + + expect(normalized.changed).toBe(true) + expect(normalized.snapshot.activeLeafId).toBe(LEAF_2) + expect(normalized.snapshot.expandedLeafId).toBeNull() + }) + + it('preserves valid active and expanded leaf ids', () => { + const layout = splitLayout() + + const normalized = normalizeTerminalLayoutSnapshot(layout) + + expect(normalized.changed).toBe(false) + expect(normalized.snapshot.activeLeafId).toBe(LEAF_2) + expect(normalized.snapshot.expandedLeafId).toBe(LEAF_3) + }) +}) + +describe('resolvePtyBoundActiveLeafId', () => { + it('preserves the active leaf when it still has a PTY binding', () => { + const layout = splitLayout() + + const activeLeafId = resolvePtyBoundActiveLeafId({ + root: layout.root, + activeLeafId: LEAF_2, + ptyIdsByLeafId: layout.ptyIdsByLeafId + }) + + expect(activeLeafId).toBe(LEAF_2) + }) + + it('moves active selection to the first bound layout leaf when the active PTY is gone', () => { + const layout = splitLayout() + + const activeLeafId = resolvePtyBoundActiveLeafId({ + root: layout.root, + activeLeafId: LEAF_2, + ptyIdsByLeafId: { + [LEAF_1]: 'pty-1', + [LEAF_3]: 'pty-3' + } + }) + + expect(activeLeafId).toBe(LEAF_1) + }) + + it('falls back to the current active leaf when no PTY bindings exist', () => { + const layout = splitLayout() + + const activeLeafId = resolvePtyBoundActiveLeafId({ + root: layout.root, + activeLeafId: LEAF_2, + ptyIdsByLeafId: undefined + }) + + expect(activeLeafId).toBe(LEAF_2) + }) + + it('uses a binding key when there is no root layout to inspect', () => { + const activeLeafId = resolvePtyBoundActiveLeafId({ + root: null, + activeLeafId: null, + ptyIdsByLeafId: { [LEAF_3]: 'pty-3' } + }) + + expect(activeLeafId).toBe(LEAF_3) + }) }) diff --git a/src/renderer/src/components/terminal-pane/terminal-layout-leaf-ids.ts b/src/renderer/src/components/terminal-pane/terminal-layout-leaf-ids.ts index 38e857bd261..81e4efd6af5 100644 --- a/src/renderer/src/components/terminal-pane/terminal-layout-leaf-ids.ts +++ b/src/renderer/src/components/terminal-pane/terminal-layout-leaf-ids.ts @@ -114,6 +114,16 @@ export function resolveTerminalLayoutActiveLeafId(opts: { return activeLeafId && leafIdSet.has(activeLeafId) ? activeLeafId : leafIds[0] } +function getRemappedLeafId( + leafId: string | null | undefined, + rewrite: LeafIdRewrite +): string | null { + if (!leafId || rewrite.duplicatedInputLeafIds.has(leafId)) { + return null + } + return rewrite.nextLeafIdByInputLeafId.get(leafId) ?? null +} + export function normalizeTerminalLayoutSnapshot( snapshot: TerminalLayoutSnapshot | null | undefined ): { snapshot: TerminalLayoutSnapshot; changed: boolean } { @@ -146,27 +156,27 @@ export function normalizeTerminalLayoutSnapshot( nextLeafIdByInputLeafId.set(leafId, mintStablePaneId()) } } - if (!changed) { - const activeLeafId = resolveTerminalLayoutActiveLeafId({ - root: snapshot.root, - activeLeafId: snapshot.activeLeafId, - ptyIdsByLeafId: snapshot.ptyIdsByLeafId - }) - if (activeLeafId !== snapshot.activeLeafId) { - return { snapshot: { ...snapshot, activeLeafId }, changed: true } - } + const inputLeafIds = new Set(counts.keys()) + const activeLeafId = resolveTerminalLayoutActiveLeafId({ + root: snapshot.root, + activeLeafId: snapshot.activeLeafId, + ptyIdsByLeafId: snapshot.ptyIdsByLeafId + }) + const expandedLeafId = + snapshot.expandedLeafId && inputLeafIds.has(snapshot.expandedLeafId) + ? snapshot.expandedLeafId + : null + const selectionChanged = + activeLeafId !== snapshot.activeLeafId || expandedLeafId !== snapshot.expandedLeafId + if (!changed && !selectionChanged) { return { snapshot, changed: false } } const rewrite: LeafIdRewrite = { nextLeafIdByInputLeafId, duplicatedInputLeafIds } - const root = cloneLayoutWithLeafRewrite(snapshot.root, rewrite) - const remappedActiveLeafId = - snapshot.activeLeafId && !duplicatedInputLeafIds.has(snapshot.activeLeafId) - ? (nextLeafIdByInputLeafId.get(snapshot.activeLeafId) ?? null) - : firstLeafId(root) - const expandedLeafId = - snapshot.expandedLeafId && !duplicatedInputLeafIds.has(snapshot.expandedLeafId) - ? (nextLeafIdByInputLeafId.get(snapshot.expandedLeafId) ?? null) - : null + const root = changed ? cloneLayoutWithLeafRewrite(snapshot.root, rewrite) : snapshot.root + // Why: split panes can be restored after a leaf was closed elsewhere; stale + // selection ids must not strand focus on a missing pane. + const remappedActiveLeafId = getRemappedLeafId(activeLeafId, rewrite) ?? firstLeafId(root) + const remappedExpandedLeafId = getRemappedLeafId(expandedLeafId, rewrite) const ptyIdsByLeafId = remapLeafRecord(snapshot.ptyIdsByLeafId, rewrite) const buffersByLeafId = remapLeafRecord(snapshot.buffersByLeafId, rewrite) const scrollbackRefsByLeafId = remapLeafRecord(snapshot.scrollbackRefsByLeafId, rewrite) @@ -187,7 +197,7 @@ export function normalizeTerminalLayoutSnapshot( activeLeafId: remappedActiveLeafId, ptyIdsByLeafId }), - expandedLeafId, + expandedLeafId: remappedExpandedLeafId, ...(ptyIdsByLeafId ? { ptyIdsByLeafId } : {}), ...(buffersByLeafId ? { buffersByLeafId } : {}), ...(scrollbackRefsByLeafId ? { scrollbackRefsByLeafId } : {}), @@ -207,6 +217,21 @@ export function collectLeafIdsInOrder(node: TerminalPaneLayoutNode | null | unde return [...collectLeafIdsInOrder(node.first), ...collectLeafIdsInOrder(node.second)] } +export function resolvePtyBoundActiveLeafId(args: { + root: TerminalPaneLayoutNode | null | undefined + activeLeafId: string | null | undefined + ptyIdsByLeafId: Record | null | undefined +}): string | null { + if (!args.root) { + return Object.keys(args.ptyIdsByLeafId ?? {})[0] ?? args.activeLeafId ?? null + } + return resolveTerminalLayoutActiveLeafId({ + root: args.root, + activeLeafId: args.activeLeafId, + ptyIdsByLeafId: args.ptyIdsByLeafId ?? undefined + }) +} + export function getLeftmostLeafId(node: TerminalPaneLayoutNode): string { return node.type === 'leaf' ? node.leafId : getLeftmostLeafId(node.first) } 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 19ca26f723b..d3e81348487 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 @@ -53,7 +53,7 @@ function mockRootForPane(paneId: number, leafId: string = LEAF_ID): HTMLDivEleme return new MockHTMLElement({ firstElementChild: pane }) as unknown as HTMLDivElement } -function mockRootForSplit(firstPaneId: number, secondPaneId: number): HTMLDivElement { +function mockRootForSplit(firstPaneId = 1, secondPaneId = 2): HTMLDivElement { const first = new MockHTMLElement({ classList: ['pane'], dataset: { paneId: String(firstPaneId), leafId: LEAF_ID }, @@ -328,4 +328,42 @@ describe('captureTerminalShutdownLayout', () => { [LEAF_ID_2]: 'pty-second' }) }) + + it('does not persist a no-PTY pane as active when another split pane is bound', 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: undefined + }) + + 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 99d636e9037..f38dbd47648 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 @@ -132,6 +132,8 @@ export function captureTerminalShutdownLayout({ currentLeafIds }) 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. layout.activeLeafId = resolveTerminalLayoutActiveLeafId({ root: layout.root, activeLeafId: layout.activeLeafId,