From c4460f8820245a3944684abbd15fe1ed6e429467 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 14 Apr 2026 16:23:01 -0700 Subject: [PATCH] fix: harden terminal pane lifecycle for split groups (#641) --- docs/split-groups-rollout-pr2.md | 24 ++ src/renderer/src/components/Terminal.tsx | 6 + .../components/terminal-pane/TerminalPane.tsx | 87 ++++++- .../layout-serialization.test.ts | 49 +++- .../terminal-pane/layout-serialization.ts | 44 ++++ .../terminal-pane/pty-connection.test.ts | 233 ++++++++++++++++++ .../terminal-pane/pty-connection.ts | 142 +++++++++-- .../terminal-pane/pty-dispatcher.ts | 5 +- .../terminal-pane/pty-transport.test.ts | 93 +++++++ .../components/terminal-pane/pty-transport.ts | 26 ++ .../use-terminal-pane-global-effects.ts | 42 ++-- .../use-terminal-pane-lifecycle.ts | 40 ++- .../src/lib/pane-manager/pane-manager.ts | 8 +- .../store/slices/terminals-hydration.test.ts | 121 +++++++++ src/renderer/src/store/slices/terminals.ts | 10 +- vitest.config.ts | 3 + 16 files changed, 879 insertions(+), 54 deletions(-) create mode 100644 docs/split-groups-rollout-pr2.md create mode 100644 src/renderer/src/components/terminal-pane/pty-connection.test.ts create mode 100644 src/renderer/src/store/slices/terminals-hydration.test.ts create mode 100644 vitest.config.ts diff --git a/docs/split-groups-rollout-pr2.md b/docs/split-groups-rollout-pr2.md new file mode 100644 index 00000000..fb295edb --- /dev/null +++ b/docs/split-groups-rollout-pr2.md @@ -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 diff --git a/src/renderer/src/components/Terminal.tsx b/src/renderer/src/components/Terminal.tsx index 507e595a..e02e84e0 100644 --- a/src/renderer/src/components/Terminal.tsx +++ b/src/renderer/src/components/Terminal.tsx @@ -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)} /> diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index 9c7b3672..7114d142 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -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>(new Map()) const isActiveRef = useRef(isActive) isActiveRef.current = isActive + const isVisibleRef = useRef(isVisible) + isVisibleRef.current = isVisible const [expandedPaneId, setExpandedPaneId] = useState(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( diff --git a/src/renderer/src/components/terminal-pane/layout-serialization.test.ts b/src/renderer/src/components/terminal-pane/layout-serialization.test.ts index 682f08ac..ea4e04be 100644 --- a/src/renderer/src/components/terminal-pane/layout-serialization.test.ts +++ b/src/renderer/src/components/terminal-pane/layout-serialization.test.ts @@ -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).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']) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/layout-serialization.ts b/src/renderer/src/components/terminal-pane/layout-serialization.ts index e0d9d3db..873e234e 100644 --- a/src/renderer/src/components/terminal-pane/layout-serialization.ts +++ b/src/renderer/src/components/terminal-pane/layout-serialization.ts @@ -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, + 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 / diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts new file mode 100644 index 00000000..cbbb0ab2 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -0,0 +1,233 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +type StoreState = { + tabsByWorktree: Record + worktreesByRepo: Record + repos: { id: string; connectionId?: string | null }[] + cacheTimerByKey: Record + settings: { promptCacheTimerEnabled?: boolean } | null +} + +type MockTransport = { + attach: ReturnType + connect: ReturnType + sendInput: ReturnType + resize: ReturnType + getPtyId: ReturnType +} + +const scheduleRuntimeGraphSync = vi.fn() +const shouldSeedCacheTimerOnInitialTitle = vi.fn(() => false) + +let mockStoreState: StoreState +let transportFactoryQueue: MockTransport[] = [] +let createdTransportOptions: Record[] = [] + +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) => { + 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 = {}) { + 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') + }) +}) diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 5f45647c..b31f9d58 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -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>() + type PtyConnectionDeps = { tabId: string worktreeId: string cwd?: string startup?: { command: string; env?: Record } | null + restoredLeafId?: string | null + restoredPtyIdByLeafId?: Record paneTransportsRef: React.RefObject> pendingWritesRef: React.RefObject> isActiveRef: React.RefObject + isVisibleRef: React.RefObject 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() } diff --git a/src/renderer/src/components/terminal-pane/pty-dispatcher.ts b/src/renderer/src/components/terminal-pane/pty-dispatcher.ts index 6888281c..1226ec3a 100644 --- a/src/renderer/src/components/terminal-pane/pty-dispatcher.ts +++ b/src/renderer/src/components/terminal-pane/pty-dispatcher.ts @@ -119,7 +119,7 @@ export type PtyTransport = { onError?: (message: string, errors?: string[]) => void onExit?: (code: number) => void } - }) => void | Promise + }) => void | Promise /** 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 } diff --git a/src/renderer/src/components/terminal-pane/pty-transport.test.ts b/src/renderer/src/components/terminal-pane/pty-transport.test.ts index 0d71fc28..b3fa1ba4 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.test.ts @@ -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() + }) }) diff --git a/src/renderer/src/components/terminal-pane/pty-transport.ts b/src/renderer/src/components/terminal-pane/pty-transport.ts index ec647972..a65c1cba 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.ts @@ -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 diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts index 2859d1df..8baed82a 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts @@ -12,25 +12,29 @@ import type { PtyTransport } from './pty-transport' type UseTerminalPaneGlobalEffectsArgs = { tabId: string isActive: boolean + isVisible: boolean managerRef: React.RefObject containerRef: React.RefObject paneTransportsRef: React.RefObject> pendingWritesRef: React.RefObject> isActiveRef: React.RefObject + isVisibleRef: React.RefObject 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 100–500 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 100–500 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) => { 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 e527d185..f2660b06 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 @@ -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> pendingWritesRef: React.RefObject> isActiveRef: React.RefObject + isVisibleRef: React.RefObject 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() diff --git a/src/renderer/src/lib/pane-manager/pane-manager.ts b/src/renderer/src/lib/pane-manager/pane-manager.ts index 1326dd5a..bce1b593 100644 --- a/src/renderer/src/lib/pane-manager/pane-manager.ts +++ b/src/renderer/src/lib/pane-manager/pane-manager.ts @@ -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 }) } }, diff --git a/src/renderer/src/store/slices/terminals-hydration.test.ts b/src/renderer/src/store/slices/terminals-hydration.test.ts new file mode 100644 index 00000000..d3043fdc --- /dev/null +++ b/src/renderer/src/store/slices/terminals-hydration.test.ts @@ -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' } + }) + }) +}) diff --git a/src/renderer/src/store/slices/terminals.ts b/src/renderer/src/store/slices/terminals.ts index 268a4a6d..864b4926 100644 --- a/src/renderer/src/store/slices/terminals.ts +++ b/src/renderer/src/store/slices/terminals.ts @@ -882,8 +882,16 @@ export const createTerminalSlice: StateCreator .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 + }) ) } }) diff --git a/vitest.config.ts b/vitest.config.ts new file mode 100644 index 00000000..614b53e4 --- /dev/null +++ b/vitest.config.ts @@ -0,0 +1,3 @@ +import config from './config/vitest.config' + +export default config