Resolve stale terminal focus and ignore late exits from old PTYs

- Repair the active terminal pane leaf selection when a pane's PTY exits, preventing focus from being stranded on a dead pane.
- Ensure stale, pruned, or unbound terminal leaves are not persisted as active in visual layouts or shutdown snapshots.
- Ignore late exit notifications and subscription end events from old PTY transports or remote stream handles after a reconnect or rebind.
This commit is contained in:
Jinjing
2026-06-27 21:43:18 -07:00
parent f175030d39
commit 29c8fa78cb
9 changed files with 373 additions and 31 deletions
+89
View File
@@ -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(() => ({
+24 -1
View File
@@ -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, RuntimeTerminalSummary>
): 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,
@@ -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,
@@ -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()
@@ -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)
@@ -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)
})
})
@@ -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<string, string> | 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)
}
@@ -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' })
})
})
@@ -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,