From 82f643d34fbf5578f347b83f727b44f415328846 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Thu, 9 Jul 2026 22:01:31 -0400 Subject: [PATCH] Prune unbound terminal layout leaves at mount so lost pane closes can't resurrect blank panes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pane close (or newborn-PTY failure) tears down the leaf's PTY and its ptyIdsByLeafId binding, but the persisted layout root can retain the leaf when the post-close persist loses to an unmount/park boundary. Remounting that snapshot materializes a pane with no terminal behind it, and a binding-less pane is unreachable by dead-session reconcile (no PTY id to prove dead) — it stays blank forever and every subsequent layout persist re-captures it. Collapse such leaves at the mount seam instead: a leaf with no PTY binding and no captured scrollback beside bound siblings is a dead remnant. Pruning is skipped when no leaf is bound (app-restart restores drop the in-session pty map) and for host-authoritative layouts (web client / remote-runtime), where leaves may legitimately precede their PTY ids. Co-authored-by: Orca --- ...terminal-layout-unbound-leaf-prune.test.ts | 162 ++++++++++++++++++ .../terminal-layout-unbound-leaf-prune.ts | 107 ++++++++++++ .../use-terminal-pane-lifecycle.ts | 19 ++ 3 files changed, 288 insertions(+) create mode 100644 src/renderer/src/components/terminal-pane/terminal-layout-unbound-leaf-prune.test.ts create mode 100644 src/renderer/src/components/terminal-pane/terminal-layout-unbound-leaf-prune.ts diff --git a/src/renderer/src/components/terminal-pane/terminal-layout-unbound-leaf-prune.test.ts b/src/renderer/src/components/terminal-pane/terminal-layout-unbound-leaf-prune.test.ts new file mode 100644 index 00000000000..829e5a78898 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-layout-unbound-leaf-prune.test.ts @@ -0,0 +1,162 @@ +import { describe, expect, it } from 'vitest' +import type { TerminalLayoutSnapshot, TerminalPaneLayoutNode } from '../../../../shared/types' +import { pruneUnboundTerminalLayoutLeaves } from './terminal-layout-unbound-leaf-prune' + +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 LEAF_4 = '44444444-4444-4444-8444-444444444444' + +function leaf(leafId: string): TerminalPaneLayoutNode { + return { type: 'leaf', leafId } +} + +function split( + first: TerminalPaneLayoutNode, + second: TerminalPaneLayoutNode, + ratio?: number +): TerminalPaneLayoutNode { + return { type: 'split', direction: 'vertical', first, second, ...(ratio ? { ratio } : {}) } +} + +// The field incident shape: split(split(agent, teammate), setup) where the +// setup pane's PTY and binding were torn down but the persisted root kept the +// leaf, so a remount materialized a permanently blank pane. +function ghostIncidentLayout(): TerminalLayoutSnapshot { + return { + root: split(split(leaf(LEAF_1), leaf(LEAF_2)), leaf(LEAF_3), 0.548), + activeLeafId: LEAF_1, + expandedLeafId: null, + ptyIdsByLeafId: { + [LEAF_1]: 'pty-agent', + [LEAF_2]: 'pty-teammate' + }, + titlesByLeafId: { + [LEAF_1]: 'agent', + [LEAF_3]: 'Setup' + } + } +} + +describe('pruneUnboundTerminalLayoutLeaves', () => { + it('collapses an unbound leaf beside bound siblings (field incident shape)', () => { + const { snapshot, changed } = pruneUnboundTerminalLayoutLeaves(ghostIncidentLayout()) + expect(changed).toBe(true) + expect(snapshot.root).toEqual(split(leaf(LEAF_1), leaf(LEAF_2))) + expect(snapshot.activeLeafId).toBe(LEAF_1) + expect(snapshot.ptyIdsByLeafId).toEqual({ + [LEAF_1]: 'pty-agent', + [LEAF_2]: 'pty-teammate' + }) + expect(snapshot.titlesByLeafId).toEqual({ [LEAF_1]: 'agent' }) + }) + + it('promotes the surviving sibling of a pruned nested split', () => { + const { snapshot, changed } = pruneUnboundTerminalLayoutLeaves({ + root: split(leaf(LEAF_1), split(leaf(LEAF_2), leaf(LEAF_3)), 0.7), + activeLeafId: LEAF_1, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF_1]: 'pty-1', [LEAF_3]: 'pty-3' } + }) + expect(changed).toBe(true) + expect(snapshot.root).toEqual(split(leaf(LEAF_1), leaf(LEAF_3), 0.7)) + }) + + it('prunes multiple unbound leaves in one pass', () => { + const { snapshot, changed } = pruneUnboundTerminalLayoutLeaves({ + root: split(split(leaf(LEAF_1), leaf(LEAF_2)), split(leaf(LEAF_3), leaf(LEAF_4))), + activeLeafId: LEAF_2, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF_2]: 'pty-2' } + }) + expect(changed).toBe(true) + expect(snapshot.root).toEqual(leaf(LEAF_2)) + expect(snapshot.activeLeafId).toBe(LEAF_2) + }) + + it('repairs activeLeafId and expandedLeafId when they pointed at a pruned leaf', () => { + const { snapshot } = pruneUnboundTerminalLayoutLeaves({ + root: split(leaf(LEAF_1), leaf(LEAF_2)), + activeLeafId: LEAF_2, + expandedLeafId: LEAF_2, + ptyIdsByLeafId: { [LEAF_1]: 'pty-1' } + }) + expect(snapshot.root).toEqual(leaf(LEAF_1)) + expect(snapshot.activeLeafId).toBe(LEAF_1) + expect(snapshot.expandedLeafId).toBeNull() + }) + + it('keeps an unbound leaf that has captured scrollback buffers', () => { + const layout: TerminalLayoutSnapshot = { + root: split(leaf(LEAF_1), leaf(LEAF_2)), + activeLeafId: LEAF_1, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF_1]: 'pty-1' }, + buffersByLeafId: { [LEAF_2]: 'saved output' } + } + const { snapshot, changed } = pruneUnboundTerminalLayoutLeaves(layout) + expect(changed).toBe(false) + expect(snapshot).toBe(layout) + }) + + it('keeps an unbound leaf that has a durable scrollback ref', () => { + const { changed } = pruneUnboundTerminalLayoutLeaves({ + root: split(leaf(LEAF_1), leaf(LEAF_2)), + activeLeafId: LEAF_1, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF_1]: 'pty-1' }, + scrollbackRefsByLeafId: { [LEAF_2]: 'ref-2' } + }) + expect(changed).toBe(false) + }) + + it('never prunes when no leaf is bound (app-restart restore drops the pty map)', () => { + const layout: TerminalLayoutSnapshot = { + root: split(leaf(LEAF_1), leaf(LEAF_2)), + activeLeafId: LEAF_1, + expandedLeafId: null + } + const { snapshot, changed } = pruneUnboundTerminalLayoutLeaves(layout) + expect(changed).toBe(false) + expect(snapshot).toBe(layout) + }) + + it('ignores pty map entries for leaves that are not in the tree', () => { + // A binding for a leaf outside root must not count as "bound tree". + const { changed } = pruneUnboundTerminalLayoutLeaves({ + root: split(leaf(LEAF_1), leaf(LEAF_2)), + activeLeafId: LEAF_1, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF_3]: 'pty-elsewhere' } + }) + expect(changed).toBe(false) + }) + + it('leaves single-leaf and rootless snapshots untouched', () => { + const singleLeaf: TerminalLayoutSnapshot = { + root: leaf(LEAF_1), + activeLeafId: LEAF_1, + expandedLeafId: null, + ptyIdsByLeafId: {} + } + expect(pruneUnboundTerminalLayoutLeaves(singleLeaf).changed).toBe(false) + const rootless: TerminalLayoutSnapshot = { + root: null, + activeLeafId: null, + expandedLeafId: null + } + expect(pruneUnboundTerminalLayoutLeaves(rootless).changed).toBe(false) + }) + + it('returns the input snapshot unchanged when every leaf is bound', () => { + const layout: TerminalLayoutSnapshot = { + root: split(leaf(LEAF_1), leaf(LEAF_2)), + activeLeafId: LEAF_1, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF_1]: 'pty-1', [LEAF_2]: 'pty-2' } + } + const { snapshot, changed } = pruneUnboundTerminalLayoutLeaves(layout) + expect(changed).toBe(false) + expect(snapshot).toBe(layout) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-layout-unbound-leaf-prune.ts b/src/renderer/src/components/terminal-pane/terminal-layout-unbound-leaf-prune.ts new file mode 100644 index 00000000000..a91546062c5 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-layout-unbound-leaf-prune.ts @@ -0,0 +1,107 @@ +import type { TerminalLayoutSnapshot, TerminalPaneLayoutNode } from '../../../../shared/types' +import { + collectLeafIdsInOrder, + resolveTerminalLayoutActiveLeafId +} from './terminal-layout-leaf-ids' + +/** + * Drops layout leaves that have no PTY binding and no captured scrollback from + * an in-session layout snapshot, collapsing their splits so the sibling + * subtree takes the space. + * + * Why: a pane close (or a newborn-PTY failure) can tear down a leaf's PTY and + * its `ptyIdsByLeafId` binding while a stale `root` still holds the leaf — + * e.g. when the post-close layout persist loses to an unmount/park boundary. + * Remounting such a snapshot materializes a pane for a terminal that no longer + * exists, and a binding-less pane is unreachable by dead-session reconcile + * (no PTY id to prove dead), so it stays blank forever. + * + * Pruning only applies when at least one leaf in the tree is still bound: + * `ptyIdsByLeafId` is in-session state (dropped across app restart), so an + * all-unbound tree is a restart restore — those leaves respawn or replay + * scrollback and must be preserved. Callers must also skip host-authoritative + * layouts (web client / remote-runtime tabs), where the host snapshot may + * legitimately deliver leaves before their PTY ids. + */ +export function pruneUnboundTerminalLayoutLeaves(snapshot: TerminalLayoutSnapshot): { + snapshot: TerminalLayoutSnapshot + changed: boolean +} { + const root = snapshot.root + if (!root || root.type === 'leaf') { + return { snapshot, changed: false } + } + const ptyIdsByLeafId = snapshot.ptyIdsByLeafId ?? {} + const leafIds = collectLeafIdsInOrder(root) + const hasBoundLeaf = leafIds.some((leafId) => ptyIdsByLeafId[leafId]) + if (!hasBoundLeaf) { + return { snapshot, changed: false } + } + + const isPrunable = (leafId: string): boolean => + !ptyIdsByLeafId[leafId] && + !snapshot.buffersByLeafId?.[leafId] && + !snapshot.scrollbackRefsByLeafId?.[leafId] + + const pruneNode = (node: TerminalPaneLayoutNode): TerminalPaneLayoutNode | null => { + if (node.type === 'leaf') { + return isPrunable(node.leafId) ? null : node + } + const first = pruneNode(node.first) + const second = pruneNode(node.second) + if (first && second) { + return first === node.first && second === node.second ? node : { ...node, first, second } + } + return first ?? second + } + + const prunedRoot = pruneNode(root) + if (prunedRoot === root || prunedRoot === null) { + return { snapshot, changed: false } + } + + const remainingLeafIds = new Set(collectLeafIdsInOrder(prunedRoot)) + const filterToRemaining = ( + record: Record | undefined + ): Record | undefined => { + if (!record) { + return undefined + } + const next = Object.fromEntries( + Object.entries(record).filter(([leafId]) => remainingLeafIds.has(leafId)) + ) + return Object.keys(next).length > 0 ? next : undefined + } + + const nextPtyIdsByLeafId = filterToRemaining(snapshot.ptyIdsByLeafId) + const nextBuffersByLeafId = filterToRemaining(snapshot.buffersByLeafId) + const nextScrollbackRefsByLeafId = filterToRemaining(snapshot.scrollbackRefsByLeafId) + const nextTitlesByLeafId = filterToRemaining(snapshot.titlesByLeafId) + const { + ptyIdsByLeafId: _ptyIdsByLeafId, + buffersByLeafId: _buffersByLeafId, + scrollbackRefsByLeafId: _scrollbackRefsByLeafId, + titlesByLeafId: _titlesByLeafId, + ...snapshotWithoutLeafRecords + } = snapshot + return { + snapshot: { + ...snapshotWithoutLeafRecords, + root: prunedRoot, + activeLeafId: resolveTerminalLayoutActiveLeafId({ + root: prunedRoot, + activeLeafId: snapshot.activeLeafId, + ptyIdsByLeafId: nextPtyIdsByLeafId + }), + expandedLeafId: + snapshot.expandedLeafId && remainingLeafIds.has(snapshot.expandedLeafId) + ? snapshot.expandedLeafId + : null, + ...(nextPtyIdsByLeafId ? { ptyIdsByLeafId: nextPtyIdsByLeafId } : {}), + ...(nextBuffersByLeafId ? { buffersByLeafId: nextBuffersByLeafId } : {}), + ...(nextScrollbackRefsByLeafId ? { scrollbackRefsByLeafId: nextScrollbackRefsByLeafId } : {}), + ...(nextTitlesByLeafId ? { titlesByLeafId: nextTitlesByLeafId } : {}) + }, + changed: true + } +} diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts index 4f9d93ed934..7b8f5d44fde 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts @@ -52,6 +52,8 @@ import { restoreScrollbackBuffers } from './layout-serialization' import { resolveTerminalLayoutActiveLeafId } from './terminal-layout-leaf-ids' +import { pruneUnboundTerminalLayoutLeaves } from './terminal-layout-unbound-leaf-prune' +import { isHostAuthoritativeLayout } from './terminal-live-layout-reconciliation' import { makePaneKey } from '../../../../shared/stable-pane-id' import { applyExpandedLayoutTo, restoreExpandedLayoutFrom } from './expand-collapse' import { @@ -717,6 +719,23 @@ export function useTerminalPaneLifecycle({ initialLayoutRef.current = normalizedInitialLayout.snapshot useAppStore.getState().setTabLayout(tabId, normalizedInitialLayout.snapshot) } + // Why: a leaf with no PTY binding and no scrollback beside bound siblings is + // a dead remnant (its close/exit teardown lost the layout collapse), and a + // materialized binding-less pane stays blank forever — dead-session + // reconcile has no PTY id to prove it dead. Host-authoritative layouts may + // deliver leaves before their PTY ids, so they are never pruned. + if ( + !isHostAuthoritativeLayout({ + isWebClient: !!(globalThis as { __ORCA_WEB_CLIENT__?: boolean }).__ORCA_WEB_CLIENT__, + ptyIdsByLeafId: initialLayoutRef.current.ptyIdsByLeafId + }) + ) { + const prunedInitialLayout = pruneUnboundTerminalLayoutLeaves(initialLayoutRef.current) + if (prunedInitialLayout.changed) { + initialLayoutRef.current = prunedInitialLayout.snapshot + useAppStore.getState().setTabLayout(tabId, prunedInitialLayout.snapshot) + } + } const initialLayoutHadBuffers = Boolean(initialLayoutRef.current.buffersByLeafId) const hydratedInitialScrollback = hydrateTerminalScrollbackRefs(initialLayoutRef.current) if (hydratedInitialScrollback.hydrated) {