fix: harden terminal pane lifecycle for split groups (#641)

This commit is contained in:
Brennan Benson 2026-04-14 16:23:01 -07:00 committed by GitHub
parent 92353e7233
commit c4460f8820
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
16 changed files with 879 additions and 54 deletions

View file

@ -0,0 +1,24 @@
# Split Groups PR 2: Terminal Lifecycle Hardening
This branch lands the terminal ownership and remount safety work required
before split groups can be exposed.
Scope:
- preserve PTYs across remounts
- fix pending-spawn dedupe paths
- fix split-pane PTY ownership
- keep visible-but-unfocused panes rendering correctly
What Is Actually Hooked Up In This PR:
- the existing terminal path uses the new PTY attach/detach/remount behavior
- split panes inside a terminal tab get distinct PTY ownership
- visible terminal panes continue rendering even when another pane or group has focus
What Is Not Hooked Up Yet:
- no split-group layout is rendered
- `Terminal.tsx` still uses the legacy single-surface host path
- no worktree restore/activation changes land here
Non-goals:
- no split-group UI rollout
- no worktree activation fallback changes

View file

@ -893,6 +893,12 @@ function Terminal(): React.JSX.Element | null {
worktreeId={worktree.id}
cwd={worktree.path}
isActive={isVisible && tab.id === activeTabId && activeTabType === 'terminal'}
// Why: the legacy single-surface terminal host still expects
// tab visibility to follow active-tab selection. Split-group
// surfaces pass a broader visibility signal separately, but
// this path must preserve the old one-pane-at-a-time stacking
// behavior until it is migrated off the legacy host.
isVisible={isVisible && tab.id === activeTabId && activeTabType === 'terminal'}
onPtyExit={(ptyId) => handlePtyExit(tab.id, ptyId)}
onCloseTab={() => handleCloseTab(tab.id)}
/>

View file

@ -49,7 +49,7 @@ export default function TerminalPane({
worktreeId,
cwd,
isActive,
isVisible: _isVisible,
isVisible = true,
onPtyExit,
onCloseTab
}: TerminalPaneProps): React.JSX.Element {
@ -65,6 +65,8 @@ export default function TerminalPane({
const pendingWritesRef = useRef<Map<number, string>>(new Map())
const isActiveRef = useRef(isActive)
isActiveRef.current = isActive
const isVisibleRef = useRef(isVisible)
isVisibleRef.current = isVisible
const [expandedPaneId, setExpandedPaneId] = useState<number | null>(null)
const [searchOpen, setSearchOpen] = useState(false)
@ -173,10 +175,30 @@ export default function TerminalPane({
Object.entries(existing.buffersByLeafId).filter(([id]) => currentLeafIds.has(id))
)
}
// Why: between pane creation and the deferred rAF where PTYs actually
// attach, all transports have getPtyId() === null. If persistLayoutSnapshot
// fires during that window the live-transport block below finds no entries,
// so this block preserves the *prior* snapshot's leaf→PTY mappings. Without
// it, a rapid successive remount (tab moved again before the first rAF)
// would lose the mappings and force fresh PTY spawns.
if (existing?.ptyIdsByLeafId) {
const currentLeafIds = new Set(manager.getPanes().map((p) => paneLeafId(p.id)))
layout.ptyIdsByLeafId = Object.fromEntries(
Object.entries(existing.ptyIdsByLeafId).filter(([id]) => currentLeafIds.has(id))
)
}
// Preserve pane titles — uses the live React state (via ref) rather than
// the stale Zustand value because React state reflects in-flight title
// edits that haven't been persisted yet.
const currentPanes = manager.getPanes()
const ptyEntries = currentPanes
.map(
(p) => [paneLeafId(p.id), paneTransportsRef.current.get(p.id)?.getPtyId() ?? null] as const
)
.filter((entry): entry is readonly [string, string] => entry[1] !== null)
if (ptyEntries.length > 0) {
layout.ptyIdsByLeafId = Object.fromEntries(ptyEntries)
}
const titles = paneTitlesRef.current
const titleEntries = currentPanes
.filter((p) => titles[p.id])
@ -187,6 +209,39 @@ export default function TerminalPane({
setTabLayout(tabId, layout)
}, [tabId, setTabLayout])
const syncPanePtyLayoutBinding = useCallback(
(paneId: number, ptyId: string | null): void => {
const existingLayout = useAppStore.getState().terminalLayoutsByTabId[tabId] ?? EMPTY_LAYOUT
const { ptyIdsByLeafId: _existingPtyIdsByLeafId, ...layoutWithoutPtyBindings } =
existingLayout
const existingBindings = existingLayout.ptyIdsByLeafId ?? {}
const leafId = paneLeafId(paneId)
if (ptyId) {
setTabLayout(tabId, {
...layoutWithoutPtyBindings,
// Why: PTY ownership changes happen after the synchronous layout
// snapshot on mount. Persist the live pane→PTY binding here so
// remounts attach each pane to its current shell instead of a stale
// or missing PTY id from an earlier snapshot.
ptyIdsByLeafId: {
...existingBindings,
[leafId]: ptyId
}
})
return
}
const nextBindings = { ...existingBindings }
delete nextBindings[leafId]
setTabLayout(tabId, {
...layoutWithoutPtyBindings,
...(Object.keys(nextBindings).length > 0 ? { ptyIdsByLeafId: nextBindings } : {})
})
},
[setTabLayout, tabId]
)
const {
setExpandedPane,
restoreExpandedLayout,
@ -218,10 +273,11 @@ export default function TerminalPane({
// longer exists. The closeTab path handles bulk cleanup, but closing
// a single split pane doesn't go through closeTab.
useAppStore.getState().setCacheTimerStartedAt(`${tabId}:${paneId}`, null)
syncPanePtyLayoutBinding(paneId, null)
manager.closePane(paneId)
}
},
[onCloseTab, tabId]
[onCloseTab, syncPanePtyLayoutBinding, tabId]
)
// Cmd+W handler — shows a Ghostty-style confirmation dialog when the
@ -275,6 +331,7 @@ export default function TerminalPane({
panePtyBindingsRef,
pendingWritesRef,
isActiveRef,
isVisibleRef,
onPtyExitRef,
onPtyErrorRef,
clearTabPtyId,
@ -286,6 +343,7 @@ export default function TerminalPane({
markWorktreeUnread,
dispatchNotification,
setCacheTimerStartedAt,
syncPanePtyLayoutBinding,
setTabPaneExpanded,
setTabCanExpandPane,
setExpandedPane,
@ -320,6 +378,7 @@ export default function TerminalPane({
panePtyBinding?.dispose()
panePtyBindingsRef.current.delete(paneId)
syncPanePtyLayoutBinding(paneId, null)
transport?.destroy?.()
paneTransportsRef.current.delete(paneId)
setCacheTimerStartedAt(`${tabId}:${paneId}`, null)
@ -332,6 +391,7 @@ export default function TerminalPane({
paneTransportsRef,
pendingWritesRef,
isActiveRef,
isVisibleRef,
onPtyExitRef,
onPtyErrorRef,
clearTabPtyId,
@ -342,7 +402,8 @@ export default function TerminalPane({
updateTabPtyId,
markWorktreeUnread,
dispatchNotification,
setCacheTimerStartedAt
setCacheTimerStartedAt,
syncPanePtyLayoutBinding
})
panePtyBindingsRef.current.set(paneId, newPaneBinding)
manager.setActivePane(paneId, { focus: true })
@ -358,6 +419,7 @@ export default function TerminalPane({
setCacheTimerStartedAt,
setRuntimePaneTitle,
suppressPtyExit,
syncPanePtyLayoutBinding,
tabId,
updateTabPtyId,
updateTabTitle,
@ -406,11 +468,13 @@ export default function TerminalPane({
useTerminalPaneGlobalEffects({
tabId,
isActive,
isVisible,
managerRef,
containerRef,
paneTransportsRef,
pendingWritesRef,
isActiveRef,
isVisibleRef,
toggleExpandPane
})
@ -606,6 +670,18 @@ export default function TerminalPane({
if (Object.keys(buffers).length > 0) {
layout.buffersByLeafId = buffers
}
const ptyEntries = panes
.map(
(pane) =>
[
paneLeafId(pane.id),
paneTransportsRef.current.get(pane.id)?.getPtyId() ?? null
] as const
)
.filter((entry): entry is readonly [string, string] => entry[1] !== null)
if (ptyEntries.length > 0) {
layout.ptyIdsByLeafId = Object.fromEntries(ptyEntries)
}
// Merge pane titles so the shutdown snapshot doesn't silently drop them.
// Why: the old early-return on empty buffers skipped this entirely, which
// meant titles were lost on restart when the terminal had no scrollback
@ -704,7 +780,10 @@ export default function TerminalPane({
: null
const terminalContainerStyle: CSSProperties = {
display: isActive ? 'flex' : 'none',
// Why: split groups can keep one terminal visible in an unfocused group so
// users still see its output while typing elsewhere. Hiding on `isActive`
// blanked the previously focused pane and exposed the white group body.
display: isVisible ? 'flex' : 'none',
['--orca-terminal-divider-color' as string]:
effectiveAppearance?.dividerColor ?? DEFAULT_TERMINAL_DIVIDER_DARK,
['--orca-terminal-divider-color-strong' as string]: normalizeColor(

View file

@ -1,4 +1,5 @@
import { describe, expect, it, beforeAll } from 'vitest'
import type { TerminalPaneLayoutNode } from '../../../../shared/types'
// ---------------------------------------------------------------------------
// Provide a minimal HTMLElement so `instanceof HTMLElement` passes in Node env
@ -31,18 +32,14 @@ beforeAll(() => {
;(globalThis as unknown as Record<string, unknown>).HTMLElement = MockHTMLElement
})
// Now import the module *after* HTMLElement is defined on globalThis.
// Vitest hoists imports, so we use dynamic import inside tests instead?
// Actually vitest hoists `beforeAll` too, but the global assignment happens
// before the imported module's runtime code runs since the module only uses
// HTMLElement at call-time (not at import-time). Let's verify.
import {
paneLeafId,
buildFontFamily,
serializePaneTree,
serializeTerminalLayout,
EMPTY_LAYOUT
EMPTY_LAYOUT,
collectLeafIdsInOrder,
collectLeafIdsInReplayCreationOrder
} from './layout-serialization'
// ---------------------------------------------------------------------------
@ -258,3 +255,41 @@ describe('serializeTerminalLayout', () => {
})
})
})
// ---------------------------------------------------------------------------
// collectLeafIdsInReplayCreationOrder
// ---------------------------------------------------------------------------
describe('collectLeafIdsInReplayCreationOrder', () => {
it('matches replayTerminalLayout pane creation order for nested left splits', () => {
const layout: TerminalPaneLayoutNode = {
type: 'split',
direction: 'vertical',
first: {
type: 'split',
direction: 'horizontal',
first: { type: 'leaf', leafId: 'A' },
second: { type: 'leaf', leafId: 'B' }
},
second: { type: 'leaf', leafId: 'C' }
}
expect(collectLeafIdsInOrder(layout)).toEqual(['A', 'B', 'C'])
expect(collectLeafIdsInReplayCreationOrder(layout)).toEqual(['A', 'C', 'B'])
})
it('matches replayTerminalLayout pane creation order for nested right splits', () => {
const layout: TerminalPaneLayoutNode = {
type: 'split',
direction: 'vertical',
first: { type: 'leaf', leafId: 'A' },
second: {
type: 'split',
direction: 'horizontal',
first: { type: 'leaf', leafId: 'B' },
second: { type: 'leaf', leafId: 'C' }
}
}
expect(collectLeafIdsInReplayCreationOrder(layout)).toEqual(['A', 'B', 'C'])
})
})

View file

@ -15,6 +15,50 @@ export function paneLeafId(paneId: number): string {
return `pane:${paneId}`
}
export function collectLeafIdsInOrder(node: TerminalPaneLayoutNode | null | undefined): string[] {
if (!node) {
return []
}
if (node.type === 'leaf') {
return [node.leafId]
}
return [...collectLeafIdsInOrder(node.first), ...collectLeafIdsInOrder(node.second)]
}
function getLeftmostLeafId(node: TerminalPaneLayoutNode): string {
return node.type === 'leaf' ? node.leafId : getLeftmostLeafId(node.first)
}
function collectReplayCreatedPaneLeafIds(
node: Extract<TerminalPaneLayoutNode, { type: 'split' }>,
leafIdsInReplayCreationOrder: string[]
): void {
// Why: replayTerminalLayout() creates one new pane per split and assigns it
// to the split's second subtree before recursing, so the new pane maps to
// the leftmost leaf reachable within that second subtree.
leafIdsInReplayCreationOrder.push(getLeftmostLeafId(node.second))
if (node.first.type === 'split') {
collectReplayCreatedPaneLeafIds(node.first, leafIdsInReplayCreationOrder)
}
if (node.second.type === 'split') {
collectReplayCreatedPaneLeafIds(node.second, leafIdsInReplayCreationOrder)
}
}
export function collectLeafIdsInReplayCreationOrder(
node: TerminalPaneLayoutNode | null | undefined
): string[] {
if (!node) {
return []
}
const leafIdsInReplayCreationOrder = [getLeftmostLeafId(node)]
if (node.type === 'split') {
collectReplayCreatedPaneLeafIds(node, leafIdsInReplayCreationOrder)
}
return leafIdsInReplayCreationOrder
}
// Cross-platform monospace fallback chain ensures the terminal always has a
// usable font regardless of OS. macOS-only fonts like SF Mono and Menlo are
// harmless on other platforms (the browser skips them), while Cascadia Mono /

View file

@ -0,0 +1,233 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
type StoreState = {
tabsByWorktree: Record<string, { id: string; ptyId: string | null }[]>
worktreesByRepo: Record<string, { id: string; repoId: string; path: string }[]>
repos: { id: string; connectionId?: string | null }[]
cacheTimerByKey: Record<string, number | null>
settings: { promptCacheTimerEnabled?: boolean } | null
}
type MockTransport = {
attach: ReturnType<typeof vi.fn>
connect: ReturnType<typeof vi.fn>
sendInput: ReturnType<typeof vi.fn>
resize: ReturnType<typeof vi.fn>
getPtyId: ReturnType<typeof vi.fn>
}
const scheduleRuntimeGraphSync = vi.fn()
const shouldSeedCacheTimerOnInitialTitle = vi.fn(() => false)
let mockStoreState: StoreState
let transportFactoryQueue: MockTransport[] = []
let createdTransportOptions: Record<string, unknown>[] = []
vi.mock('@/runtime/sync-runtime-graph', () => ({
scheduleRuntimeGraphSync
}))
vi.mock('@/store', () => ({
useAppStore: {
getState: () => mockStoreState
}
}))
vi.mock('@/lib/agent-status', () => ({
isGeminiTerminalTitle: vi.fn(() => false),
isClaudeAgent: vi.fn(() => false)
}))
vi.mock('./cache-timer-seeding', () => ({
shouldSeedCacheTimerOnInitialTitle
}))
vi.mock('./pty-transport', () => ({
createIpcPtyTransport: vi.fn((options: Record<string, unknown>) => {
createdTransportOptions.push(options)
const nextTransport = transportFactoryQueue.shift()
if (!nextTransport) {
throw new Error('No mock transport queued')
}
return nextTransport
})
}))
function createMockTransport(initialPtyId: string | null = null): MockTransport {
let ptyId = initialPtyId
return {
attach: vi.fn(({ existingPtyId }: { existingPtyId: string }) => {
ptyId = existingPtyId
}),
connect: vi.fn().mockImplementation(async () => {
return ptyId
}),
sendInput: vi.fn(() => true),
resize: vi.fn(() => true),
getPtyId: vi.fn(() => ptyId)
}
}
function createPane(paneId: number) {
return {
id: paneId,
terminal: {
cols: 120,
rows: 40,
write: vi.fn(),
onData: vi.fn(() => ({ dispose: vi.fn() })),
onResize: vi.fn(() => ({ dispose: vi.fn() }))
},
fitAddon: {
fit: vi.fn()
}
}
}
function createManager(paneCount = 1) {
return {
setPaneGpuRendering: vi.fn(),
getPanes: vi.fn(() => Array.from({ length: paneCount }, (_, index) => ({ id: index + 1 }))),
closePane: vi.fn()
}
}
function createDeps(overrides: Record<string, unknown> = {}) {
return {
tabId: 'tab-1',
worktreeId: 'wt-1',
cwd: '/tmp/wt-1',
startup: null,
restoredLeafId: null,
restoredPtyIdByLeafId: {},
paneTransportsRef: { current: new Map() },
pendingWritesRef: { current: new Map() },
isActiveRef: { current: true },
isVisibleRef: { current: true },
onPtyExitRef: { current: vi.fn() },
onPtyErrorRef: { current: vi.fn() },
clearTabPtyId: vi.fn(),
consumeSuppressedPtyExit: vi.fn(() => false),
updateTabTitle: vi.fn(),
setRuntimePaneTitle: vi.fn(),
clearRuntimePaneTitle: vi.fn(),
updateTabPtyId: vi.fn(),
markWorktreeUnread: vi.fn(),
dispatchNotification: vi.fn(),
setCacheTimerStartedAt: vi.fn(),
syncPanePtyLayoutBinding: vi.fn(),
...overrides
}
}
describe('connectPanePty', () => {
const originalRequestAnimationFrame = globalThis.requestAnimationFrame
const originalCancelAnimationFrame = globalThis.cancelAnimationFrame
beforeEach(() => {
vi.resetModules()
vi.clearAllMocks()
transportFactoryQueue = []
createdTransportOptions = []
mockStoreState = {
tabsByWorktree: {
'wt-1': [{ id: 'tab-1', ptyId: 'tab-pty' }]
},
worktreesByRepo: {
repo1: [{ id: 'wt-1', repoId: 'repo1', path: '/tmp/wt-1' }]
},
repos: [{ id: 'repo1', connectionId: null }],
cacheTimerByKey: {},
settings: { promptCacheTimerEnabled: true }
}
globalThis.requestAnimationFrame = vi.fn((callback: FrameRequestCallback) => {
callback(0)
return 1
})
globalThis.cancelAnimationFrame = vi.fn()
})
afterEach(() => {
if (originalRequestAnimationFrame) {
globalThis.requestAnimationFrame = originalRequestAnimationFrame
} else {
delete (globalThis as { requestAnimationFrame?: typeof requestAnimationFrame })
.requestAnimationFrame
}
if (originalCancelAnimationFrame) {
globalThis.cancelAnimationFrame = originalCancelAnimationFrame
} else {
delete (globalThis as { cancelAnimationFrame?: typeof cancelAnimationFrame })
.cancelAnimationFrame
}
})
it('reattaches a remounted split pane to its restored leaf PTY instead of the tab-level PTY', async () => {
const { connectPanePty } = await import('./pty-connection')
const transport = createMockTransport()
transportFactoryQueue.push(transport)
const pane = createPane(2)
const manager = createManager(2)
const deps = createDeps({
restoredLeafId: 'pane:2',
restoredPtyIdByLeafId: { 'pane:2': 'leaf-pty-2' }
})
connectPanePty(pane as never, manager as never, deps as never)
expect(transport.attach).toHaveBeenCalledWith(
expect.objectContaining({ existingPtyId: 'leaf-pty-2' })
)
expect(transport.connect).not.toHaveBeenCalled()
expect(deps.syncPanePtyLayoutBinding).toHaveBeenCalledWith(2, 'leaf-pty-2')
})
it('persists a restarted pane PTY id and uses it on the next remount', async () => {
const { connectPanePty } = await import('./pty-connection')
const restartedTransport = createMockTransport()
let spawnedPtyId: string | null = null
restartedTransport.connect.mockImplementation(async () => {
spawnedPtyId = 'pty-restarted'
const opts = createdTransportOptions[0]
;(opts.onPtySpawn as (ptyId: string) => void)('pty-restarted')
return 'pty-restarted'
})
transportFactoryQueue.push(restartedTransport)
const restartPane = createPane(1)
const restartManager = createManager(1)
const restartDeps = createDeps({
paneTransportsRef: { current: new Map([[99, createMockTransport('another-pane-pty')]]) }
})
connectPanePty(restartPane as never, restartManager as never, restartDeps as never)
await Promise.resolve()
expect(spawnedPtyId).toBe('pty-restarted')
expect(restartDeps.syncPanePtyLayoutBinding).toHaveBeenCalledWith(1, 'pty-restarted')
mockStoreState = {
...mockStoreState,
tabsByWorktree: {
'wt-1': [{ id: 'tab-1', ptyId: 'pty-restarted' }]
}
}
const remountTransport = createMockTransport()
transportFactoryQueue.push(remountTransport)
const remountPane = createPane(1)
const remountManager = createManager(1)
const remountDeps = createDeps({
restoredLeafId: 'pane:1',
restoredPtyIdByLeafId: { 'pane:1': 'pty-restarted' }
})
connectPanePty(remountPane as never, remountManager as never, remountDeps as never)
expect(remountTransport.attach).toHaveBeenCalledWith(
expect.objectContaining({ existingPtyId: 'pty-restarted' })
)
expect(remountDeps.syncPanePtyLayoutBinding).toHaveBeenCalledWith(1, 'pty-restarted')
})
})

View file

@ -4,17 +4,22 @@ import { isGeminiTerminalTitle, isClaudeAgent } from '@/lib/agent-status'
import { scheduleRuntimeGraphSync } from '@/runtime/sync-runtime-graph'
import { useAppStore } from '@/store'
import type { PtyTransport } from './pty-transport'
import { createIpcPtyTransport, getEagerPtyBufferHandle } from './pty-transport'
import { createIpcPtyTransport } from './pty-transport'
import { shouldSeedCacheTimerOnInitialTitle } from './cache-timer-seeding'
const pendingSpawnByTabId = new Map<string, Promise<string | null>>()
type PtyConnectionDeps = {
tabId: string
worktreeId: string
cwd?: string
startup?: { command: string; env?: Record<string, string> } | null
restoredLeafId?: string | null
restoredPtyIdByLeafId?: Record<string, string>
paneTransportsRef: React.RefObject<Map<number, PtyTransport>>
pendingWritesRef: React.RefObject<Map<number, string>>
isActiveRef: React.RefObject<boolean>
isVisibleRef: React.RefObject<boolean>
onPtyExitRef: React.RefObject<(ptyId: string) => void>
onPtyErrorRef?: React.RefObject<(paneId: number, message: string) => void>
clearTabPtyId: (tabId: string, ptyId: string) => void
@ -29,6 +34,7 @@ type PtyConnectionDeps = {
terminalTitle?: string
}) => void
setCacheTimerStartedAt: (key: string, ts: number | null) => void
syncPanePtyLayoutBinding: (paneId: number, ptyId: string | null) => void
}
export function connectPanePty(
@ -36,6 +42,8 @@ export function connectPanePty(
manager: PaneManager,
deps: PtyConnectionDeps
): IDisposable {
let disposed = false
let connectFrame: number | null = null
// Why: setup commands must only run once — in the initial pane of the tab.
// Capture and clear the startup reference synchronously so that panes
// created later by splits or layout restoration cannot re-execute the
@ -51,6 +59,7 @@ export function connectPanePty(
const cacheKey = `${deps.tabId}:${pane.id}`
const onExit = (ptyId: string): void => {
deps.syncPanePtyLayoutBinding(pane.id, null)
deps.clearRuntimePaneTitle(deps.tabId, pane.id)
deps.clearTabPtyId(deps.tabId, ptyId)
// Why: if the PTY exits abruptly (Ctrl-D, crash, shell termination) without
@ -108,6 +117,7 @@ export function connectPanePty(
}
const onPtySpawn = (ptyId: string): void => {
deps.syncPanePtyLayoutBinding(pane.id, ptyId)
deps.updateTabPtyId(deps.tabId, ptyId)
// Spawn completion is when a pane gains a concrete PTY ID. The initial
// frame-level sync often runs before that async result arrives.
@ -168,6 +178,7 @@ export function connectPanePty(
onAgentBecameWorking,
onAgentExited
})
const hasExistingPaneTransport = deps.paneTransportsRef.current.size > 0
deps.paneTransportsRef.current.set(pane.id, transport)
const onDataDisposable = pane.terminal.onData((data) => {
@ -181,7 +192,11 @@ export function connectPanePty(
// Defer PTY spawn/attach to next frame so FitAddon has time to calculate
// the correct terminal dimensions from the laid-out container.
deps.pendingWritesRef.current.set(pane.id, '')
requestAnimationFrame(() => {
connectFrame = requestAnimationFrame(() => {
connectFrame = null
if (disposed) {
return
}
try {
pane.fitAddon.fit()
} catch {
@ -210,9 +225,32 @@ export function connectPanePty(
// terminal state is preserved. This matches the MAX_BUFFER_BYTES
// constant used for serialized scrollback capture.
const MAX_PENDING_BYTES = 512 * 1024
const startFreshSpawn = (): void => {
const spawnPromise = Promise.resolve(
transport.connect({
url: '',
cols,
rows,
callbacks: {
onData: dataCallback,
onError: reportError
}
})
)
.then((spawnedPtyId) =>
typeof spawnedPtyId === 'string' ? spawnedPtyId : transport.getPtyId()
)
.catch(() => null)
.finally(() => {
if (pendingSpawnByTabId.get(deps.tabId) === spawnPromise) {
pendingSpawnByTabId.delete(deps.tabId)
}
})
pendingSpawnByTabId.set(deps.tabId, spawnPromise)
}
const dataCallback = (data: string): void => {
if (deps.isActiveRef.current) {
if (deps.isVisibleRef.current) {
pane.terminal.write(data)
} else {
const pending = deps.pendingWritesRef.current
@ -239,21 +277,41 @@ export function connectPanePty(
// The eagerly-spawned PTY could exit during the one-frame gap (e.g.,
// broken .bashrc), clearing the tab's ptyId. Reading it stale would
// cause attach() on a dead process, leaving the pane frozen.
const restoredPtyId =
deps.restoredLeafId && deps.restoredPtyIdByLeafId
? (deps.restoredPtyIdByLeafId[deps.restoredLeafId] ?? null)
: null
const existingPtyId = useAppStore
.getState()
.tabsByWorktree[deps.worktreeId]?.find((t) => t.id === deps.tabId)?.ptyId
// Why: only attach if the eager buffer handle still exists. For split-pane
// tabs, replayTerminalLayout calls connectPanePty once per pane. The first
// pane consumes the handle via attach(); subsequent panes find no handle
// and fall through to connect(), which spawns their own fresh PTYs. Without
// this guard, every split pane would try to share the same PTY ID, and the
// last one's handler would overwrite the earlier ones' in the dispatcher.
if (existingPtyId && getEagerPtyBufferHandle(existingPtyId)) {
// Why: remounting a multi-pane terminal tab (for example after closing or
// moving a split group) must preserve each pane's own live PTY. The saved
// leaf→PTY mapping takes precedence over the tab-level PTY owner.
if (restoredPtyId) {
allowInitialIdleCacheSeed = true
// Why: this tab had a PTY eagerly spawned by reconnectPersistedTerminals().
// Attach to it instead of spawning a duplicate. Startup commands are
// intentionally skipped — the PTY was already spawned with a fresh shell.
deps.syncPanePtyLayoutBinding(pane.id, restoredPtyId)
transport.attach({
existingPtyId: restoredPtyId,
cols,
rows,
callbacks: {
onData: dataCallback,
onError: reportError
}
})
} else if (existingPtyId && !hasExistingPaneTransport) {
// Why: only the first pane in a tab may reattach to the tab-level PTY.
// Additional panes created by in-tab splits need their own fresh PTYs; if
// they attach to the tab's existing ptyId, both panes end up sharing one
// session and the last-attached pane steals the live transport handlers.
// Group moves/remounts still reattach correctly because they recreate the
// whole TerminalPane with no surviving pane transports yet.
allowInitialIdleCacheSeed = true
deps.syncPanePtyLayoutBinding(pane.id, existingPtyId)
// Why: this tab already owns a PTY. Attach to it instead of spawning a
// duplicate. Startup commands are intentionally skipped — the PTY was
// already spawned with a fresh shell.
transport.attach({
existingPtyId,
cols,
@ -265,21 +323,61 @@ export function connectPanePty(
})
} else {
allowInitialIdleCacheSeed = false
transport.connect({
url: '',
cols,
rows,
callbacks: {
onData: dataCallback,
onError: reportError
}
})
const pendingSpawn = hasExistingPaneTransport
? undefined
: pendingSpawnByTabId.get(deps.tabId)
if (pendingSpawn) {
void pendingSpawn
.then((spawnedPtyId) => {
if (disposed) {
return
}
if (transport.getPtyId()) {
return
}
if (!spawnedPtyId) {
// Why: React StrictMode in dev can mount, start a spawn, then
// immediately unmount/remount the pane. If the first mount never
// produced a usable PTY ID, the remounted pane must issue its own
// spawn instead of staying attached to a completed-but-empty
// promise and rendering a dead terminal surface.
console.warn(
`Pending PTY spawn for tab ${deps.tabId} resolved without a PTY id, retrying fresh spawn`
)
startFreshSpawn()
return
}
transport.attach({
existingPtyId: spawnedPtyId,
cols,
rows,
callbacks: {
onData: dataCallback,
onError: reportError
}
})
})
.catch((err) => {
reportError(err instanceof Error ? err.message : String(err))
})
} else {
startFreshSpawn()
}
}
scheduleRuntimeGraphSync()
})
return {
dispose() {
disposed = true
if (connectFrame !== null) {
// Why: StrictMode and split-group remounts can dispose a pane binding
// before its deferred PTY attach/spawn work runs. Cancel that queued
// frame so stale bindings cannot reattach the PTY and steal the live
// handler wiring from the current pane.
cancelAnimationFrame(connectFrame)
connectFrame = null
}
onDataDisposable.dispose()
onResizeDisposable.dispose()
}

View file

@ -119,7 +119,7 @@ export type PtyTransport = {
onError?: (message: string, errors?: string[]) => void
onExit?: (code: number) => void
}
}) => void | Promise<void>
}) => void | Promise<void | string>
/** Attach to an existing PTY that was eagerly spawned during startup.
* Skips pty:spawn registers handlers and replays buffered data instead. */
attach: (options: {
@ -145,6 +145,9 @@ export type PtyTransport = {
isConnected: () => boolean
getPtyId: () => string | null
preserve?: () => void
/** Unregister PTY handlers without killing the process, so a remounted
* pane can reattach to the same running shell. */
detach?: () => void
destroy?: () => void | Promise<void>
}

View file

@ -177,4 +177,97 @@ describe('createIpcPtyTransport', () => {
})
expect(writeMock).not.toHaveBeenCalled()
})
it('kills a PTY that finishes spawning after the transport was destroyed', async () => {
const { createIpcPtyTransport } = await import('./pty-transport')
const spawnControls: { resolve: ((value: { id: string }) => void) | null } = { resolve: null }
const spawnPromise = new Promise<{ id: string }>((resolve) => {
spawnControls.resolve = resolve
})
const spawnMock = vi.fn().mockReturnValue(spawnPromise)
const killMock = vi.fn()
const onPtySpawn = vi.fn()
;(globalThis as { window: typeof window }).window = {
...originalWindow,
api: {
...originalWindow?.api,
pty: {
...originalWindow?.api?.pty,
spawn: spawnMock,
write: vi.fn(),
resize: vi.fn(),
kill: killMock,
onData: vi.fn((callback: (payload: { id: string; data: string }) => void) => {
onData = callback
return () => {}
}),
onExit: vi.fn((callback: (payload: { id: string; code: number }) => void) => {
onExit = callback
return () => {}
}),
onOpenCodeStatus: vi.fn(
(
callback: (payload: {
ptyId: string
status: 'working' | 'idle' | 'permission'
}) => void
) => {
onOpenCodeStatus = callback
return () => {}
}
)
}
}
} as unknown as typeof window
const transport = createIpcPtyTransport({ onPtySpawn })
const connectPromise = transport.connect({
url: '',
callbacks: {}
})
transport.destroy?.()
if (!spawnControls.resolve) {
throw new Error('Expected spawn resolver to be captured')
}
spawnControls.resolve({ id: 'pty-late' })
await connectPromise
expect(killMock).toHaveBeenCalledWith('pty-late')
expect(onPtySpawn).not.toHaveBeenCalled()
expect(transport.getPtyId()).toBeNull()
})
it('keeps the exit observer alive after detach so remounts do not reuse dead PTYs', async () => {
const { createIpcPtyTransport } = await import('./pty-transport')
const onPtyExit = vi.fn()
const onBell = vi.fn()
const onTitleChange = vi.fn()
const transport = createIpcPtyTransport({
onPtyExit,
onBell,
onTitleChange
})
transport.attach({
existingPtyId: 'pty-detached',
callbacks: {
onData: vi.fn(),
onDisconnect: vi.fn()
}
})
transport.detach?.()
onData?.({ id: 'pty-detached', data: '\u001b]0;Detached title\u0007\u0007' })
expect(onTitleChange).not.toHaveBeenCalled()
expect(onBell).not.toHaveBeenCalled()
onExit?.({ id: 'pty-detached', code: 0 })
expect(onPtyExit).toHaveBeenCalledWith('pty-detached')
expect(transport.getPtyId()).toBeNull()
})
})

View file

@ -71,6 +71,11 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
openCodeStatusHandlers.delete(id)
}
function unregisterPtyDataAndStatusHandlers(id: string): void {
ptyDataHandlers.delete(id)
openCodeStatusHandlers.delete(id)
}
function getSyntheticOpenCodeTitle(status: OpenCodeStatusEvent['status']): string {
const baseTitle =
lastObservedTerminalTitle && lastObservedTerminalTitle !== 'OpenCode'
@ -199,9 +204,11 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
storedCallbacks.onConnect?.()
storedCallbacks.onStatus?.('shell')
return result.id
} catch (err) {
const msg = err instanceof Error ? err.message : String(err)
storedCallbacks.onError?.(msg)
throw err
}
},
@ -267,6 +274,25 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
}
},
detach() {
if (staleTitleTimer) {
clearTimeout(staleTitleTimer)
staleTitleTimer = null
}
openCodeStatus = null
if (ptyId) {
// Why: detach() is used for in-session remounts such as moving a tab
// between split groups. Stop delivering data/title events into the
// unmounted pane immediately, but keep the PTY exit observer alive so
// a shell that dies during the remount gap can still clear stale
// tab/leaf bindings before the next pane attempts to reattach.
unregisterPtyDataAndStatusHandlers(ptyId)
}
connected = false
ptyId = null
storedCallbacks = {}
},
sendInput(data: string): boolean {
if (!connected || !ptyId) {
return false

View file

@ -12,25 +12,29 @@ import type { PtyTransport } from './pty-transport'
type UseTerminalPaneGlobalEffectsArgs = {
tabId: string
isActive: boolean
isVisible: boolean
managerRef: React.RefObject<PaneManager | null>
containerRef: React.RefObject<HTMLDivElement | null>
paneTransportsRef: React.RefObject<Map<number, PtyTransport>>
pendingWritesRef: React.RefObject<Map<number, string>>
isActiveRef: React.RefObject<boolean>
isVisibleRef: React.RefObject<boolean>
toggleExpandPane: (paneId: number) => void
}
export function useTerminalPaneGlobalEffects({
tabId,
isActive,
isVisible,
managerRef,
containerRef,
paneTransportsRef,
pendingWritesRef,
isActiveRef,
isVisibleRef,
toggleExpandPane
}: UseTerminalPaneGlobalEffectsArgs): void {
const wasActiveRef = useRef(false)
const wasVisibleRef = useRef(false)
// Why: tracks any in-progress chunked pending-write flush so the cleanup
// function can cancel it if the pane deactivates mid-flush.
@ -57,14 +61,13 @@ export function useTerminalPaneGlobalEffects({
if (!manager) {
return
}
if (isActive) {
// Why: resume WebGL immediately so the terminal shows its last-known
// state on the first painted frame. On macOS, WebGL context creation
// is ~5 ms — fast enough to feel instant. On Windows (ANGLE → D3D11)
// it can take 100500 ms, but the alternative (deferring to a rAF
// after the pending-write drain) leaves the terminal blank for multiple
// frames, which is a worse UX tradeoff.
manager.resumeRendering()
if (isVisible) {
// Why: resumeRendering() creates WebGL contexts for each pane, which
// blocks the renderer for 100500 ms per pane on Windows (ANGLE →
// D3D11). Deferring it into the rAF that runs after the pending-write
// drain lets the browser paint one frame with the DOM renderer so the
// terminal content appears immediately. WebGL takes over seamlessly
// in the next frame without a visible flash.
fitEpochRef.current++
const epoch = fitEpochRef.current
@ -110,7 +113,15 @@ export function useTerminalPaneGlobalEffects({
return
}
fitRanForEpochRef.current = epoch
fitAndFocusPanes(mgr)
// Why: suspendRendering() disposes every WebGL addon while hidden. We
// defer recreation until this post-drain rAF so the browser can paint
// one responsive frame with the DOM renderer before WebGL setup runs.
mgr.resumeRendering()
if (isActive) {
fitAndFocusPanes(mgr)
return
}
fitPanes(mgr)
}
if (entries.length === 0) {
@ -150,7 +161,7 @@ export function useTerminalPaneGlobalEffects({
drainNextChunk()
}
} else if (wasActiveRef.current) {
} else if (wasVisibleRef.current) {
// Cancel any in-progress chunked flush before suspending.
if (pendingFlushRef.current !== null) {
clearTimeout(pendingFlushRef.current)
@ -164,10 +175,11 @@ export function useTerminalPaneGlobalEffects({
}
manager.suspendRendering()
}
wasActiveRef.current = isActive
wasVisibleRef.current = isVisible
isActiveRef.current = isActive
isVisibleRef.current = isVisible
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [isActive])
}, [isActive, isVisible])
useEffect(() => {
const onToggleExpand = (event: Event): void => {
@ -215,7 +227,7 @@ export function useTerminalPaneGlobalEffects({
}, [tabId, managerRef])
useEffect(() => {
if (!isActive) {
if (!isVisible) {
return
}
const container = containerRef.current
@ -268,7 +280,7 @@ export function useTerminalPaneGlobalEffects({
}
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [isActive])
}, [isVisible])
useEffect(() => {
return window.api.ui.onFileDrop((data) => {

View file

@ -14,6 +14,7 @@ import type { GlobalSettings, TerminalLayoutSnapshot } from '../../../../shared/
import { resolveTerminalFontWeights } from '../../../../shared/terminal-fonts'
import {
buildFontFamily,
collectLeafIdsInReplayCreationOrder,
replayTerminalLayout,
restoreScrollbackBuffers
} from './layout-serialization'
@ -50,6 +51,7 @@ type UseTerminalPaneLifecycleDeps = {
panePtyBindingsRef: React.RefObject<Map<number, IDisposable>>
pendingWritesRef: React.RefObject<Map<number, string>>
isActiveRef: React.RefObject<boolean>
isVisibleRef: React.RefObject<boolean>
onPtyExitRef: React.RefObject<(ptyId: string) => void>
onPtyErrorRef?: React.RefObject<(paneId: number, message: string) => void>
clearTabPtyId: (tabId: string, ptyId: string) => void
@ -64,6 +66,7 @@ type UseTerminalPaneLifecycleDeps = {
terminalTitle?: string
}) => void
setCacheTimerStartedAt: (key: string, ts: number | null) => void
syncPanePtyLayoutBinding: (paneId: number, ptyId: string | null) => void
setTabPaneExpanded: (tabId: string, expanded: boolean) => void
setTabCanExpandPane: (tabId: string, canExpand: boolean) => void
setExpandedPane: (paneId: number | null) => void
@ -94,6 +97,7 @@ export function useTerminalPaneLifecycle({
panePtyBindingsRef,
pendingWritesRef,
isActiveRef,
isVisibleRef,
onPtyExitRef,
onPtyErrorRef,
clearTabPtyId,
@ -105,6 +109,7 @@ export function useTerminalPaneLifecycle({
markWorktreeUnread,
dispatchNotification,
setCacheTimerStartedAt,
syncPanePtyLayoutBinding,
setTabPaneExpanded,
setTabCanExpandPane,
setExpandedPane,
@ -185,6 +190,10 @@ export function useTerminalPaneLifecycle({
}
let shouldPersistLayout = false
const restoredLeafIdsInCreationOrder = collectLeafIdsInReplayCreationOrder(
initialLayoutRef.current.root
)
let restoredPaneCreateIndex = 0
const ptyDeps = {
tabId,
worktreeId,
@ -193,6 +202,7 @@ export function useTerminalPaneLifecycle({
paneTransportsRef,
pendingWritesRef,
isActiveRef,
isVisibleRef,
onPtyExitRef,
onPtyErrorRef,
clearTabPtyId,
@ -203,7 +213,9 @@ export function useTerminalPaneLifecycle({
updateTabPtyId,
markWorktreeUnread,
dispatchNotification,
setCacheTimerStartedAt
setCacheTimerStartedAt,
syncPanePtyLayoutBinding,
restoredPtyIdByLeafId: initialLayoutRef.current.ptyIdsByLeafId ?? {}
}
const unregisterRuntimeTab = registerRuntimeTerminalTab({
@ -238,7 +250,12 @@ export function useTerminalPaneLifecycle({
}
}
applyAppearance(manager)
const panePtyBinding = connectPanePty(pane, manager, ptyDeps)
const restoredLeafId = restoredLeafIdsInCreationOrder[restoredPaneCreateIndex] ?? null
restoredPaneCreateIndex += 1
const panePtyBinding = connectPanePty(pane, manager, {
...ptyDeps,
restoredLeafId
})
panePtyBindings.set(pane.id, panePtyBinding)
scheduleRuntimeGraphSync()
queueResizeAll(true)
@ -258,6 +275,7 @@ export function useTerminalPaneLifecycle({
if (transport) {
const ptyId = transport.getPtyId()
if (ptyId) {
syncPanePtyLayoutBinding(paneId, null)
clearTabPtyId(tabId, ptyId)
}
transport.destroy?.()
@ -436,6 +454,11 @@ export function useTerminalPaneLifecycle({
scheduleRuntimeGraphSync()
return () => {
const tabStillExists = Boolean(
useAppStore
.getState()
.tabsByWorktree[worktreeId]?.find((candidate) => candidate.id === tabId)
)
unregisterRuntimeTab()
if (resizeRaf !== null) {
cancelAnimationFrame(resizeRaf)
@ -446,7 +469,18 @@ export function useTerminalPaneLifecycle({
}
linkDisposables.clear()
for (const transport of paneTransports.values()) {
transport.destroy?.()
if (tabStillExists && transport.getPtyId()) {
// Why: moving a terminal tab between groups currently rehomes the
// React subtree, which unmounts this TerminalPane even though the tab
// itself is still alive. Detaching preserves the running PTY so the
// remounted pane can reattach without restarting the user's shell.
// Transports that have not attached yet still have no PTY ID; those
// must be destroyed so any in-flight spawn resolves into a killed PTY
// instead of reviving a stale binding after unmount.
transport.detach?.()
} else {
transport.destroy?.()
}
}
for (const panePtyBinding of panePtyBindings.values()) {
panePtyBinding.dispose()

View file

@ -318,7 +318,13 @@ export class PaneManager {
this.dragState,
this.getDragCallbacks(),
(paneId) => {
if (!this.destroyed && this.activePaneId !== paneId) {
if (!this.destroyed) {
// Why: split-pane layout/focus callbacks can leave the manager's
// activePaneId temporarily in sync while the browser's real focused
// xterm textarea is still on a different pane. Clicking a pane must
// always re-focus its terminal, even if the manager already thinks
// that pane is active; otherwise input can keep going to the wrong
// split after vertical/horizontal splits.
this.setActivePane(paneId, { focus: true })
}
},

View file

@ -0,0 +1,121 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } }))
vi.mock('@/lib/agent-status', () => ({
detectAgentStatusFromTitle: vi.fn().mockReturnValue(null)
}))
vi.mock('@/runtime/sync-runtime-graph', () => ({
scheduleRuntimeGraphSync: vi.fn()
}))
vi.mock('@/components/terminal-pane/pty-transport', () => ({
registerEagerPtyBuffer: vi.fn(),
ensurePtyDispatcher: vi.fn()
}))
const mockApi = {
worktrees: {
list: vi.fn().mockResolvedValue([]),
create: vi.fn().mockResolvedValue({}),
remove: vi.fn().mockResolvedValue(undefined),
updateMeta: vi.fn().mockResolvedValue({})
},
repos: {
list: vi.fn().mockResolvedValue([]),
add: vi.fn().mockResolvedValue({}),
remove: vi.fn().mockResolvedValue(undefined),
update: vi.fn().mockResolvedValue({}),
pickFolder: vi.fn().mockResolvedValue(null)
},
pty: {
kill: vi.fn().mockResolvedValue(undefined)
},
gh: {
prForBranch: vi.fn().mockResolvedValue(null),
issue: vi.fn().mockResolvedValue(null)
},
settings: {
get: vi.fn().mockResolvedValue({}),
set: vi.fn().mockResolvedValue(undefined)
},
cache: {
getGitHub: vi.fn().mockResolvedValue(null),
setGitHub: vi.fn().mockResolvedValue(undefined)
},
claudeUsage: {
getScanState: vi.fn().mockResolvedValue({
enabled: false,
isScanning: false,
lastScanStartedAt: null,
lastScanCompletedAt: null,
lastScanError: null,
hasAnyClaudeData: false
}),
setEnabled: vi.fn().mockResolvedValue({}),
refresh: vi.fn().mockResolvedValue({}),
getSummary: vi.fn().mockResolvedValue(null),
getDaily: vi.fn().mockResolvedValue([]),
getBreakdown: vi.fn().mockResolvedValue([]),
getRecentSessions: vi.fn().mockResolvedValue([])
},
codexUsage: {
getScanState: vi.fn().mockResolvedValue({
enabled: false,
isScanning: false,
lastScanStartedAt: null,
lastScanCompletedAt: null,
lastScanError: null,
hasAnyCodexData: false
}),
setEnabled: vi.fn().mockResolvedValue({}),
refresh: vi.fn().mockResolvedValue({}),
getSummary: vi.fn().mockResolvedValue(null),
getDaily: vi.fn().mockResolvedValue([]),
getBreakdown: vi.fn().mockResolvedValue([]),
getRecentSessions: vi.fn().mockResolvedValue([])
}
}
// @ts-expect-error -- mocked browser preload API
globalThis.window = { api: mockApi }
import type { WorkspaceSessionState } from '../../../../shared/types'
import { createTestStore, makeLayout, makeTab, makeWorktree, seedStore } from './store-test-helpers'
describe('hydrateWorkspaceSession', () => {
beforeEach(() => {
vi.clearAllMocks()
})
it('drops persisted ptyIdsByLeafId because restart must reconnect fresh PTYs', () => {
const store = createTestStore()
const worktreeId = 'repo1::/wt-1'
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': {
...makeLayout(),
ptyIdsByLeafId: { 'pane:1': 'stale-leaf-pty' },
buffersByLeafId: { 'pane:1': 'buffer' }
}
}
}
store.getState().hydrateWorkspaceSession(session)
expect(store.getState().terminalLayoutsByTabId['tab-1']).toEqual({
...makeLayout(),
buffersByLeafId: { 'pane:1': 'buffer' }
})
})
})

View file

@ -882,8 +882,16 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
.flat()
.map((tab) => [tab.id, []] as const)
),
// Why: leaf→PTY mappings only make sense within the current renderer
// process. App restart reconnects fresh PTYs and must not attempt to
// reattach dead process IDs from the last session snapshot.
terminalLayoutsByTabId: Object.fromEntries(
Object.entries(session.terminalLayoutsByTabId).filter(([tabId]) => validTabIds.has(tabId))
Object.entries(session.terminalLayoutsByTabId)
.filter(([tabId]) => validTabIds.has(tabId))
.map(([tabId, layout]) => {
const { ptyIdsByLeafId: _ptyIdsByLeafId, ...restartSafeLayout } = layout
return [tabId, restartSafeLayout] as const
})
)
}
})

3
vitest.config.ts Normal file
View file

@ -0,0 +1,3 @@
import config from './config/vitest.config'
export default config