diff --git a/src/renderer/src/components/Terminal.tsx b/src/renderer/src/components/Terminal.tsx index 6632ec63d35..d8b424bc5bf 100644 --- a/src/renderer/src/components/Terminal.tsx +++ b/src/renderer/src/components/Terminal.tsx @@ -1031,6 +1031,9 @@ function Terminal(): React.JSX.Element | null { capturedForceParked.delete(id) } } + // Why getState and not a dep: this read happens in the same pass that + // unmounts the panes, so it is the current catalog; re-running on a later + // repos change could not re-capture buffers whose panes are already gone. const repos = useAppStore.getState().repos const nextEvictionExemptTabIds = new Set() for (const worktreeId of forceParkedWorktreeIds) { diff --git a/src/renderer/src/components/terminal-pane/command-code-done-settle.ts b/src/renderer/src/components/terminal-pane/command-code-done-settle.ts index f7c207fb7e7..69a0082df2d 100644 --- a/src/renderer/src/components/terminal-pane/command-code-done-settle.ts +++ b/src/renderer/src/components/terminal-pane/command-code-done-settle.ts @@ -17,7 +17,10 @@ type CommandCodeDoneSettleExecutor = (normalizedPrompt: string) => void const executorByPaneKey = new Map() const timerByPaneKey = new Map>() -/** Registers the current writer for this pane; returns its release. */ +/** Registers the current writer for this pane; returns its release. + * Last registrant wins by design: park and reveal each hand the pane to a new + * owner while the predecessor is still registered, so refusing the overwrite + * would leave the row with an owner that can no longer write it. */ export function setCommandCodeDoneSettleExecutor( paneKey: string, execute: CommandCodeDoneSettleExecutor diff --git a/src/renderer/src/components/terminal-pane/ssh-reattach-model-restore.ts b/src/renderer/src/components/terminal-pane/ssh-reattach-model-restore.ts index 1bcaa1ebf6a..c641332758c 100644 --- a/src/renderer/src/components/terminal-pane/ssh-reattach-model-restore.ts +++ b/src/renderer/src/components/terminal-pane/ssh-reattach-model-restore.ts @@ -54,6 +54,9 @@ export function decideSshReattachPaintSource(args: { } // Why the escape tail is excluded: a dangling mid-escape is not content — a // model holding only one paints a blank pane, which this gate forbids. + // Accepted cost: a session that wrote only control-sequence preamble before + // parking always falls back to relay; the gate cannot tell it apart from a + // model that never received content, and relay replays that preamble anyway. const contentLength = (args.snapshot.scrollbackAnsi?.length ?? 0) + args.snapshot.data.length return contentLength === 0 ? 'relay-replay' : 'main-model-snapshot' } diff --git a/src/renderer/src/components/terminal-pane/terminal-eviction-exempt-tabs.test.ts b/src/renderer/src/components/terminal-pane/terminal-eviction-exempt-tabs.test.ts new file mode 100644 index 00000000000..0560057b3dd --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-eviction-exempt-tabs.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it, vi } from 'vitest' + +vi.mock('@/store', () => ({ + useAppStore: { getState: () => ({ terminalLayoutsByTabId: {}, runtimePaneTitlesByTabId: {} }) } +})) + +import { selectEvictionExemptTerminalTabLayoutKey } from './terminal-eviction-exempt-tabs' + +const TAB_ID = 'tab-1' +const LEAF_ID = '11111111-1111-4111-8111-111111111111' +const SECOND_LEAF_ID = '22222222-2222-4222-8222-222222222222' +const PTY_ID = 'repo::/worktree@@session-1' +const TABS = [{ id: TAB_ID, ptyId: PTY_ID }] + +function layoutState(ptyIdsByLeafId: Record) { + return { terminalLayoutsByTabId: { [TAB_ID]: { ptyIdsByLeafId } } } +} + +describe('selectEvictionExemptTerminalTabLayoutKey', () => { + // Why: the exempt memo is keyed on the tabs array, which a split leaves + // untouched — a key blind to layout PTYs would unmount the new live pane. + it('changes when a split adds a leaf pty', () => { + const single = layoutState({ [LEAF_ID]: PTY_ID }) + expect(selectEvictionExemptTerminalTabLayoutKey(single, TABS)).toBe( + selectEvictionExemptTerminalTabLayoutKey(single, TABS) + ) + expect( + selectEvictionExemptTerminalTabLayoutKey( + layoutState({ [LEAF_ID]: PTY_ID, [SECOND_LEAF_ID]: 'pty-local-detached' }), + TABS + ) + ).not.toBe(selectEvictionExemptTerminalTabLayoutKey(single, TABS)) + }) + + it('changes when a leaf pty is re-minted under the same layout', () => { + expect( + selectEvictionExemptTerminalTabLayoutKey( + layoutState({ [LEAF_ID]: 'pty-local-detached' }), + TABS + ) + ).not.toBe(selectEvictionExemptTerminalTabLayoutKey(layoutState({ [LEAF_ID]: PTY_ID }), TABS)) + }) + + it('tolerates tabs with no persisted layout', () => { + expect(selectEvictionExemptTerminalTabLayoutKey({ terminalLayoutsByTabId: {} }, TABS)).toBe( + `${TAB_ID}=` + ) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-eviction-exempt-tabs.ts b/src/renderer/src/components/terminal-pane/terminal-eviction-exempt-tabs.ts index 242cab9bf65..3a11fab51fd 100644 --- a/src/renderer/src/components/terminal-pane/terminal-eviction-exempt-tabs.ts +++ b/src/renderer/src/components/terminal-pane/terminal-eviction-exempt-tabs.ts @@ -60,3 +60,28 @@ export function selectEvictionExemptTerminalTabIds( } return exemptTabIds } + +type EvictionExemptTabLayoutState = { + terminalLayoutsByTabId: Record } | undefined> +} + +/** + * Memo key over the store half of an exemption verdict: a split add or a + * re-minted leaf pty changes the layout's pane PTYs without changing the tabs + * array, and a memo keyed on tabs alone would keep serving an exempt set that + * misses the new pane — force-park would unmount it and orphan a live shell. + */ +export function selectEvictionExemptTerminalTabLayoutKey( + state: EvictionExemptTabLayoutState, + tabs: readonly ParkableTerminalTabModel[] +): string { + return tabs + .map((tab) => { + const ptyIdsByLeafId = state.terminalLayoutsByTabId[tab.id]?.ptyIdsByLeafId ?? {} + const leafPtys = Object.entries(ptyIdsByLeafId) + .map(([leafId, ptyId]) => `${leafId}:${ptyId}`) + .join(',') + return `${tab.id}=${leafPtys}` + }) + .join('|') +} diff --git a/src/renderer/src/components/terminal-pane/use-terminal-tab-cold-parking.test.ts b/src/renderer/src/components/terminal-pane/use-terminal-tab-cold-parking.test.ts index af7168ccb69..24ae41a7725 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-tab-cold-parking.test.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-tab-cold-parking.test.ts @@ -6,7 +6,8 @@ import type { TerminalTab } from '../../../../shared/types' const mocks = vi.hoisted(() => ({ storeState: { pendingStartupByTabId: {} as Record, - settings: {} as Record + settings: {} as Record, + terminalLayoutsByTabId: {} as Record }> }, exemptTabIds: new Set(), exemptSelectCalls: 0 @@ -20,7 +21,16 @@ vi.mock('./terminal-eviction-exempt-tabs', () => ({ selectEvictionExemptTerminalTabIds: (_worktreeId: string, tabs: readonly { id: string }[]) => { mocks.exemptSelectCalls += 1 return new Set(tabs.filter((tab) => mocks.exemptTabIds.has(tab.id)).map((tab) => tab.id)) - } + }, + selectEvictionExemptTerminalTabLayoutKey: ( + state: typeof mocks.storeState, + tabs: readonly { id: string }[] + ) => + tabs + .map( + (tab) => `${tab.id}=${JSON.stringify(state.terminalLayoutsByTabId[tab.id]?.ptyIdsByLeafId)}` + ) + .join('|') })) vi.mock('./terminal-parked-tab-watchers', () => ({ @@ -64,6 +74,7 @@ describe('useTerminalTabColdParking measure-clock contract', () => { vi.useRealTimers() mocks.exemptTabIds = new Set() mocks.exemptSelectCalls = 0 + mocks.storeState.terminalLayoutsByTabId = {} }) // Why: the worktree layer preserves hiddenSince through a background-measure @@ -154,6 +165,34 @@ describe('useTerminalTabColdParking measure-clock contract', () => { expect(result.current.parkedTerminalTabIds).toEqual(new Set(['tab-1', 'tab-2'])) }) + // Why: a split lands in the layout store, not in terminalTabs — a memo keyed + // on the tabs alone would keep serving a set that misses the new pane and + // unmount the live shell it should have exempted. + it('re-resolves exemptions when only the layout PTYs change', () => { + const stableArgs = { ...hookArgs(false), coldParkTerminalPanes: true, isForceParked: true } + const { result, rerender } = renderHook( + (args: typeof stableArgs) => useTerminalTabColdParking(args), + { initialProps: stableArgs } + ) + expect(result.current.parkedTerminalTabIds).toEqual(new Set(['tab-1', 'tab-2'])) + + // Same tabs, same verdict: nothing the memo can see has moved yet. + mocks.exemptTabIds = new Set(['tab-1']) + act(() => { + rerender(stableArgs) + }) + expect(result.current.parkedTerminalTabIds).toEqual(new Set(['tab-1', 'tab-2'])) + + // The split's leaf pty lands in the layout store and re-keys the memo. + mocks.storeState.terminalLayoutsByTabId = { + 'tab-1': { ptyIdsByLeafId: { 'leaf-2': 'pty-local-detached' } } + } + act(() => { + rerender(stableArgs) + }) + expect(result.current.parkedTerminalTabIds).toEqual(new Set(['tab-2'])) + }) + // Why: resolving an exemption re-reads the store and walks the layout tree per // tab, so an unrelated re-render must not repeat that work. it('resolves eviction exemptions once per force-park input change', () => { diff --git a/src/renderer/src/components/terminal-pane/use-terminal-tab-cold-parking.ts b/src/renderer/src/components/terminal-pane/use-terminal-tab-cold-parking.ts index 55db274b6ad..cbf128e7877 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-tab-cold-parking.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-tab-cold-parking.ts @@ -24,7 +24,10 @@ import { type ParkVerdictFlipRecord } from './terminal-park-verdict-flip-telemetry' import { getTerminalParkingPolicyOverrides } from './terminal-parking-e2e-overrides' -import { selectEvictionExemptTerminalTabIds } from './terminal-eviction-exempt-tabs' +import { + selectEvictionExemptTerminalTabIds, + selectEvictionExemptTerminalTabLayoutKey +} from './terminal-eviction-exempt-tabs' import { canWatcherCoverParkedTerminalTab, disposeParkedTerminalWatchersForWorktree, @@ -246,13 +249,21 @@ export function useTerminalTabColdParking(args: { worktreeId ]) + // Why subscribed: the exemption also reads layout leaf PTYs, which change + // without a terminalTabs change (split added, pty re-minted); gated on + // isForceParked so only force-parked worktrees build the key per store change. + const evictionExemptLayoutKey = useAppStore((state) => + isForceParked ? selectEvictionExemptTerminalTabLayoutKey(state, terminalTabs) : '' + ) // Why memoized: resolving an exemption re-reads the store and walks the - // layout tree per tab, so recompute only when the force-park verdict or the - // tabs change — not on every assignment/park-set change below. + // layout tree per tab, so recompute only when the force-park verdict, the + // tabs, or their layout PTYs change — not on every assignment/park-set change + // below. const evictionExemptTerminalTabIds = useMemo( () => isForceParked ? selectEvictionExemptTerminalTabIds(worktreeId, terminalTabs) : EMPTY_TAB_IDS, - [isForceParked, terminalTabs, worktreeId] + // eslint-disable-next-line react-hooks/exhaustive-deps -- the layout key encodes the store fields the selector re-reads internally. + [evictionExemptLayoutKey, isForceParked, terminalTabs, worktreeId] ) // Why: the rendered park verdict — worktree-level park (prop from