Skip to content
Merged
89 changes: 89 additions & 0 deletions src/main/runtime/orca-runtime.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10783,6 +10783,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(() => ({
Expand Down
25 changes: 24 additions & 1 deletion src/main/runtime/orca-runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7960,14 +7960,23 @@ export class OrcaRuntimeService {
return null
}
const parentTabId = firstSurface.parentTabId
const activeLeafId =
const requestedActiveLeafId =
firstSurface.parentLayout?.activeLeafId ??
surfaces.find((surface) => surface.isActive)?.leafId ??
firstSurface.leafId
const root = firstSurface.parentLayout?.root ?? {
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
Expand All @@ -7980,6 +7989,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,
Expand Down
44 changes: 34 additions & 10 deletions src/renderer/src/components/terminal-pane/TerminalPane.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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])

Expand Down Expand Up @@ -1302,6 +1326,7 @@ export default function TerminalPane({
},
[
clearCodexRestartNotice,
clearExitedPanePtyLayoutBinding,
clearRuntimePaneTitle,
clearTabPtyId,
cwd,
Expand All @@ -1317,7 +1342,6 @@ export default function TerminalPane({
setCacheTimerStartedAt,
setRuntimePaneTitle,
suppressPtyExit,
clearExitedPanePtyLayoutBinding,
syncPanePtyLayoutBinding,
tabId,
updateTabPtyId,
Expand Down
73 changes: 63 additions & 10 deletions src/renderer/src/components/terminal-pane/pty-connection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -352,19 +352,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
})
}
}

Expand Down Expand Up @@ -855,10 +865,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)
Expand All @@ -871,6 +901,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 () => {
Expand Down Expand Up @@ -4152,6 +4183,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()
Expand Down
40 changes: 40 additions & 0 deletions src/renderer/src/components/terminal-pane/pty-connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1304,6 +1304,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
})
}
}

// Why: the transport's own exit handler (pty-transport.ts) normally makes
// onExit run-at-most-once by clearing connected/ptyId + unregistering BEFORE
// calling it. reconcileIfSessionDead drives onExit directly (bypassing that),
Expand All @@ -1317,6 +1346,16 @@ export function connectPanePty(
if (handledExitPtyId === ptyId) {
return
}
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.
handledExitPtyId = ptyId
deps.clearTabPtyId(deps.tabId, ptyId)
deps.consumeSuppressedPtyExit(ptyId)
scheduleRuntimeGraphSync()
return
}
handledExitPtyId = ptyId
agentCompletionCoordinator.dispose()
clearPanePtyFitBinding()
Expand Down Expand Up @@ -1361,6 +1400,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)
Expand Down
Loading
Loading