fix: memoize terminal eviction exemptions on layout leaf PTYs

Splits add leaf panes to the layout store without changing the tabs
array. A memo keyed only on tabs misses this change, leaving new panes
unexempted for unmount. Include layout leaf PTYs in the exemption memo
key so it recalculates when splits occur or PTYs are re-minted.
This commit is contained in:
Jinjing
2026-07-29 00:20:27 -07:00
parent 9661f8e712
commit 8d49ffb69e
7 changed files with 140 additions and 7 deletions
+3
View File
@@ -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<string>()
for (const worktreeId of forceParkedWorktreeIds) {
@@ -17,7 +17,10 @@ type CommandCodeDoneSettleExecutor = (normalizedPrompt: string) => void
const executorByPaneKey = new Map<string, CommandCodeDoneSettleExecutor>()
const timerByPaneKey = new Map<string, ReturnType<typeof setTimeout>>()
/** 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
@@ -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'
}
@@ -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<string, string>) {
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}=`
)
})
})
@@ -60,3 +60,28 @@ export function selectEvictionExemptTerminalTabIds(
}
return exemptTabIds
}
type EvictionExemptTabLayoutState = {
terminalLayoutsByTabId: Record<string, { ptyIdsByLeafId?: Record<string, string> } | 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('|')
}
@@ -6,7 +6,8 @@ import type { TerminalTab } from '../../../../shared/types'
const mocks = vi.hoisted(() => ({
storeState: {
pendingStartupByTabId: {} as Record<string, unknown>,
settings: {} as Record<string, unknown>
settings: {} as Record<string, unknown>,
terminalLayoutsByTabId: {} as Record<string, { ptyIdsByLeafId?: Record<string, string> }>
},
exemptTabIds: new Set<string>(),
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', () => {
@@ -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