fix(terminal): resolve the eviction exemption per pane, not per tab (C1 review #8)

isEvictionExemptTerminalTab read only tab.ptyId — the FIRST leaf's pty —
while the coverage veto that makes a worktree a retention candidate walks
every pane. A split tab whose second leaf held an unrestorable pty therefore
failed coverage (→ force-park target) yet looked exempt-free, so force-park
unmounted it and orphaned the live shell. The exemption now resolves panes
through the same resolveParkedTerminalPaneCandidates, keeping tab.ptyId in
the union for the no-layout/no-capture case.

Also from the same review round:
- force-park's capture passes includeLocalBuffers:false like every other
  shutdownBufferCaptures caller; it was serializing up to 512KB/pane of
  scrollback into the store inside a fix meant to bound renderer heap.
- Terminal.tsx unmount resets the scrollback-demotion registry — module
  state with no reset path, read by a pane effect that runs before the host
  effect that would clear it, so a stale verdict trimmed restore replays.
- memoize watcher coverage per tab within the parking pass; the retention
  candidates re-asked it for every mounted worktree, not just the parked few.
This commit is contained in:
Jinjing
2026-07-28 13:21:53 -07:00
parent ae19f890a4
commit 310da683b4
9 changed files with 186 additions and 41 deletions
+24 -9
View File
@@ -136,10 +136,16 @@ only new state is the verdict itself.
hidden un-parkable worktree never force-parks — one warm slot is the
deliberate floor (slice C bounds it instead, §2.3).
- **Eviction exemption is per-TAB, not per-worktree.** An eviction-exempt
tab (`isEvictionExemptTerminalTab`: a live local pty a remount could not
reattach — daemon-fail-open separator-less ids AND ptys minted under
another worktreeId; null/SSH/remote ids are not exempt) no longer vetoes
its worktree. The worktree force-parks while exempt tabs keep their
tab (`isEvictionExemptTerminalTab`: ANY of its pane ptys is a live local
one a remount could not reattach — daemon-fail-open separator-less ids AND
ptys minted under another worktreeId; null/SSH/remote ids are not exempt)
no longer vetoes its worktree. The exemption resolves panes through the
same `resolveParkedTerminalPaneCandidates` the coverage veto uses, so the
two agree on split tabs: reading only `tab.ptyId` (the FIRST leaf's pty)
would let a split tab whose second leaf holds the unrestorable pty fail
coverage — making its worktree a retention candidate — while looking
exempt-free, and force-park would orphan that live shell.
The worktree force-parks while exempt tabs keep their
mounted panes via a per-tab exclusion mirroring the Activity-portal
pattern (legacy watcher sync + render in `Terminal.tsx`, and
`useTerminalTabColdParking` via the `isForceParked` prop). Exempt panes
@@ -147,10 +153,13 @@ only new state is the verdict itself.
coverage where the transport exists. Ordinary parking eligibility is
untouched: a worktree with an exempt tab still cannot ordinary-park,
because eligibility requires every tab restorable.
- Best-effort capture at force-park: the existing 512 KB
`shutdownBufferCaptures` serialization runs once per force-park episode
before the unmount render, so snapshot-less classes still reveal
last-known content cold-restore-style.
- Best-effort capture at force-park: the existing `shutdownBufferCaptures`
serialization runs once per force-park episode before the unmount render,
so snapshot-less classes still reveal last-known content
cold-restore-style. It passes `includeLocalBuffers: false` like every other
caller, so only relay/remote-host worktrees (no other scrollback source)
keep bytes — a heap fix must not plant 512 KB/pane of retained strings for
local worktrees that already have the daemon snapshot.
- Damping: the verdict follows every existing convention — computed in the
worktree-parking effect, excluded from its own deps, set-equality guards
returning the previous reference, strictly-positive recheck timers (the
@@ -182,7 +191,13 @@ Demotion applies through each mounted pane's lifecycle hook
(`useTerminalWorktreeScrollbackDemoted` →
`applyTerminalScrollbackRowsToMountedPanes`) — an xterm option update only,
no remount/replay/refit/SIGWINCH. Trimmed history is gone by design; reveal
restores the configured cap for future output. Measured reclaim at 50k-row
restores the configured cap for future output. The verdict registry is
module state, so `Terminal.tsx`'s unmount cleanup calls
`resetTerminalScrollbackDemotion()` beside `disposeAllParkedTerminalWatchers()`
— nothing would recompute a stale verdict, and React runs a pane's scrollback
effect BEFORE the host effect that would clear it, so a pane remounting
against a stale demotion would have its restore replay trimmed to 1 000 rows
unrecoverably. Measured reclaim at 50k-row
settings: 19.4 → 1.3 MB V8 heap and 114.7 → 2.7 MB ArrayBuffers per pane.
Membership is time-monotone for fixed inputs (unit-tested): the force-parked
and past-TTL sets only grow with time and the last-active pick is
+42 -11
View File
@@ -36,7 +36,13 @@ import {
import { isIntentionalAppRestartInProgress } from '@/lib/updater-beforeunload'
import { preventUnloadAndScheduleShutdownCheckpointReset } from '@/lib/shutdown-checkpoint-guard'
import EditorAutosaveController from './editor/EditorAutosaveController'
import type { Tab, TabContentType, TabGroupLayoutNode, TuiAgent } from '../../../shared/types'
import type {
Tab,
TabContentType,
TabGroupLayoutNode,
TerminalTab,
TuiAgent
} from '../../../shared/types'
import { hasFeatureInteraction } from '../../../shared/feature-interactions'
import BrowserPane from './browser-pane/BrowserPane'
import BrowserPaneOverlayLayer from './browser-pane/BrowserPaneOverlayLayer'
@@ -86,18 +92,21 @@ import {
} from './terminal-pane/terminal-hidden-view-parking'
import {
TERMINAL_HIDDEN_WORKTREE_RETENTION_TTL_MS,
isEvictionExemptTerminalTab,
selectRetentionForceParkedTerminalWorktrees,
selectScrollbackDemotedTerminalWorktrees,
type TerminalWorktreeRetentionCandidate
} from './terminal-pane/terminal-hidden-worktree-retention'
import { setScrollbackDemotedTerminalWorktrees } from './terminal-pane/terminal-hidden-scrollback-demotion'
import {
resetTerminalScrollbackDemotion,
setScrollbackDemotedTerminalWorktrees
} from './terminal-pane/terminal-hidden-scrollback-demotion'
import { shutdownBufferCaptures } from './terminal-pane/shutdown-buffer-captures'
import { getTerminalParkingPolicyOverrides } from './terminal-pane/terminal-parking-e2e-overrides'
import { useColdParkedTerminalPresentation } from './terminal-pane/use-cold-parked-terminal-presentation'
import {
canWatcherCoverParkedTerminalTab,
disposeAllParkedTerminalWatchers,
isEvictionExemptTerminalTab,
pruneParkedTerminalWatchers,
shouldDeferParkedPtyExitTabClose,
syncParkedTerminalTabWatchers
@@ -918,10 +927,23 @@ function Terminal(): React.JSX.Element | null {
restorePolicy: { sshParkingEnabled: terminalSshParkingEnabled },
...overrides
})
// Why memoized: the retention pass below re-asks coverage for EVERY mounted
// worktree's tabs, not just the parked few — each call re-reads the store and
// can walk the layout tree, so cache per tab for this pass.
const watcherCoverageByTabId = new Map<string, boolean>()
const worktreeTabsAreWatcherCovered = (worktreeId: string, tabs: TerminalTab[]): boolean =>
tabs.every((tab) => {
const cached = watcherCoverageByTabId.get(tab.id)
if (cached !== undefined) {
return cached
}
const covered = canWatcherCoverParkedTerminalTab(worktreeId, tab)
watcherCoverageByTabId.set(tab.id, covered)
return covered
})
// Why: a worktree with any watcher-uncoverable tab must never park, or it goes silent for bells/titles/completions (sank the first parking attempt).
for (const worktreeId of Array.from(nextParkedTerminalWorktreeIds)) {
const tabs = tabsByWorktree[worktreeId] ?? []
if (!tabs.every((tab) => canWatcherCoverParkedTerminalTab(worktreeId, tab))) {
if (!worktreeTabsAreWatcherCovered(worktreeId, tabsByWorktree[worktreeId] ?? [])) {
nextParkedTerminalWorktreeIds.delete(worktreeId)
}
}
@@ -950,8 +972,7 @@ function Terminal(): React.JSX.Element | null {
shouldMeasureHiddenWorktree: candidate.shouldMeasureHiddenWorktree,
hasActivityTerminalPortal: candidate.hasActivityTerminalPortal,
ordinaryParkingCovers:
parkEligible &&
tabs.every((tab) => canWatcherCoverParkedTerminalTab(candidate.worktreeId, tab)),
parkEligible && worktreeTabsAreWatcherCovered(candidate.worktreeId, tabs),
hasEvictionExemptTab: tabs.some((tab) =>
isEvictionExemptTerminalTab(tab, candidate.worktreeId)
),
@@ -980,9 +1001,13 @@ function Terminal(): React.JSX.Element | null {
for (const worktreeId of forceParkedWorktreeIds) {
if (!capturedForceParked.has(worktreeId)) {
capturedForceParked.add(worktreeId)
// Why: serialize buffers while the panes still exist (same registry sleep uses), so classes with no snapshot source still reveal last-known content.
// Why: serialize buffers while the panes still exist (same registry sleep
// uses), so classes with no snapshot source still reveal last-known
// content. includeLocalBuffers:false is required here, not optional — a
// heap fix must not plant 512KB/pane of scrollback strings in the store
// for local worktrees that already have the daemon snapshot.
for (const tab of tabsByWorktree[worktreeId] ?? []) {
shutdownBufferCaptures.get(tab.id)?.()
shutdownBufferCaptures.get(tab.id)?.({ includeLocalBuffers: false })
}
}
nextParkedTerminalWorktreeIds.add(worktreeId)
@@ -1295,8 +1320,14 @@ function Terminal(): React.JSX.Element | null {
workspaceSessionReady,
workspaceSurfaces
])
// Why: on host unmount no reconciliation effect runs again, so dispose every remaining parked watcher here.
useEffect(() => () => disposeAllParkedTerminalWatchers(), [])
// Why: on host unmount no reconciliation effect runs again, so dispose every remaining parked watcher and drop the demotion verdicts nothing would recompute.
useEffect(
() => () => {
disposeAllParkedTerminalWatchers()
resetTerminalScrollbackDemotion()
},
[]
)
// Auto-create first tab when worktree activates
useEffect(() => {
if (!workspaceSessionReady) {
@@ -3,6 +3,7 @@ import {
TERMINAL_DEMOTED_SCROLLBACK_ROWS,
demotedTerminalScrollbackRows,
isTerminalWorktreeScrollbackDemoted,
resetTerminalScrollbackDemotion,
setScrollbackDemotedTerminalWorktrees,
subscribeTerminalScrollbackDemotion
} from './terminal-hidden-scrollback-demotion'
@@ -32,6 +33,21 @@ describe('setScrollbackDemotedTerminalWorktrees', () => {
})
})
describe('resetTerminalScrollbackDemotion', () => {
// Why: module state outlives the host, so a stale verdict would demote a pane
// that remounts before any effect recomputes it.
it('clears every verdict and notifies subscribers', () => {
setScrollbackDemotedTerminalWorktrees(new Set(['wt-1', 'wt-2']))
const listener = vi.fn()
const unsubscribe = subscribeTerminalScrollbackDemotion(listener)
resetTerminalScrollbackDemotion()
expect(isTerminalWorktreeScrollbackDemoted('wt-1')).toBe(false)
expect(isTerminalWorktreeScrollbackDemoted('wt-2')).toBe(false)
expect(listener).toHaveBeenCalledTimes(1)
unsubscribe()
})
})
describe('demotedTerminalScrollbackRows', () => {
it('never exceeds the configured rows', () => {
expect(demotedTerminalScrollbackRows(50_000)).toBe(TERMINAL_DEMOTED_SCROLLBACK_ROWS)
@@ -28,6 +28,16 @@ export function setScrollbackDemotedTerminalWorktrees(next: ReadonlySet<string>)
}
}
/**
* Clear every verdict — the demotion host is gone, so nothing recomputes them.
* Without this a pane remounting later reads a stale demotion (React runs the
* pane's scrollback effect BEFORE the host effect that would clear it) and
* xterm trims its restore replay to 1 000 rows, unrecoverably.
*/
export function resetTerminalScrollbackDemotion(): void {
setScrollbackDemotedTerminalWorktrees(new Set())
}
export function isTerminalWorktreeScrollbackDemoted(worktreeId: string): boolean {
return demotedWorktreeIds.has(worktreeId)
}
@@ -2,27 +2,25 @@ import { describe, expect, it } from 'vitest'
import { TERMINAL_WORKTREE_PARK_DELAY_MS } from './terminal-hidden-view-parking'
import {
TERMINAL_HIDDEN_WORKTREE_RETENTION_TTL_MS,
isEvictionExemptTerminalTab,
isEvictionExemptTerminalPty,
selectRetentionForceParkedTerminalWorktrees,
selectScrollbackDemotedTerminalWorktrees,
type TerminalWorktreeRetentionCandidate
} from './terminal-hidden-worktree-retention'
describe('isEvictionExemptTerminalTab', () => {
describe('isEvictionExemptTerminalPty', () => {
const worktreeId = 'repo::/worktree'
it('exempts only live local ptys a remount could not reattach', () => {
expect(isEvictionExemptTerminalTab({ ptyId: 'pty-local-detached' }, worktreeId)).toBe(true)
expect(isEvictionExemptTerminalTab({ ptyId: 'other::wt@@session-1' }, worktreeId)).toBe(true)
expect(isEvictionExemptTerminalPty('pty-local-detached', worktreeId)).toBe(true)
expect(isEvictionExemptTerminalPty('other::wt@@session-1', worktreeId)).toBe(true)
})
it('never exempts snapshot-backed, SSH, remote-runtime, or unbound tabs', () => {
expect(isEvictionExemptTerminalTab({ ptyId: `${worktreeId}@@session-1` }, worktreeId)).toBe(
false
)
expect(isEvictionExemptTerminalTab({ ptyId: 'ssh:conn-1@@pty-1' }, worktreeId)).toBe(false)
expect(isEvictionExemptTerminalTab({ ptyId: 'remote:env-1@@t-1' }, worktreeId)).toBe(false)
expect(isEvictionExemptTerminalTab({ ptyId: null }, worktreeId)).toBe(false)
it('never exempts snapshot-backed, SSH, remote-runtime, or unbound ptys', () => {
expect(isEvictionExemptTerminalPty(`${worktreeId}@@session-1`, worktreeId)).toBe(false)
expect(isEvictionExemptTerminalPty('ssh:conn-1@@pty-1', worktreeId)).toBe(false)
expect(isEvictionExemptTerminalPty('remote:env-1@@t-1', worktreeId)).toBe(false)
expect(isEvictionExemptTerminalPty(null, worktreeId)).toBe(false)
})
})
@@ -1,6 +1,5 @@
import { isRemoteRuntimePtyId } from '@/runtime/runtime-terminal-inspection'
import { parseAppSshPtyId } from '../../../../shared/ssh-pty-id'
import type { TerminalTab } from '../../../../shared/types'
import {
TERMINAL_WORKTREE_COLD_PARK_DELAY_MS,
isSnapshotBackedTerminalPty,
@@ -18,16 +17,17 @@ import {
export const TERMINAL_HIDDEN_WORKTREE_RETENTION_LIMIT = 12
export const TERMINAL_HIDDEN_WORKTREE_RETENTION_TTL_MS = 45 * 60_000
// Why: an eviction-exempt tab holds a live local pty a remount could not
// reattach (daemon-fail-open separator-less ids, ptys minted under another
// worktree) — a fresh spawn would orphan the live shell. The TAB keeps its
// mounted pane when its worktree force-parks (per-tab exclusion, mirroring
// Activity portals) and relies on scrollback demotion instead.
export function isEvictionExemptTerminalTab(
tab: Pick<TerminalTab, 'ptyId'>,
// Why: an eviction-exempt pty is a live local one a remount could not reattach
// (daemon-fail-open separator-less ids, ptys minted under another worktree) — a
// fresh spawn would orphan the live shell. Its TAB keeps its mounted pane when
// the worktree force-parks (per-tab exclusion, mirroring Activity portals) and
// relies on scrollback demotion instead. Per-PTY, not per-tab: the coverage
// veto that makes a worktree a retention candidate walks every split pane, so
// the exemption must too (see isEvictionExemptTerminalTab).
export function isEvictionExemptTerminalPty(
ptyId: string | null | undefined,
worktreeId: string
): boolean {
const ptyId = tab.ptyId
if (!ptyId || isRemoteRuntimePtyId(ptyId) || parseAppSshPtyId(ptyId)) {
return false
}
@@ -99,6 +99,7 @@ import {
disposeParkedTerminalWatchersForWorktree,
fallbackParkedPaneCandidates,
getParkedTerminalWatcherTabIds,
isEvictionExemptTerminalTab,
pruneParkedTerminalWatchers,
shouldDeferParkedPtyExitTabClose,
syncParkedTerminalTabWatchers
@@ -780,6 +781,58 @@ describe('terminal-parked-tab-watchers', () => {
)
})
})
describe('isEvictionExemptTerminalTab', () => {
// Why these pair with coverage: the same split tab that fails coverage (so
// force-park targets its worktree) must be exempt, or force-park unmounts
// the very live pty the exemption exists to protect.
it('exempts a split tab whose SECOND pane holds the unrestorable pty', () => {
capturePanes([
{ ptyId: PTY_ID, paneId: 1, leafId: LEAF_ID, drivesTabTitle: true },
{ ptyId: 'other::wt@@session-9', paneId: 2, leafId: SECOND_LEAF_ID, drivesTabTitle: false }
])
const tab = { id: TAB_ID, ptyId: PTY_ID }
expect(canWatcherCoverParkedTerminalTab(WORKTREE_ID, tab)).toBe(false)
expect(isEvictionExemptTerminalTab(tab, WORKTREE_ID)).toBe(true)
})
it('exempts a split tab whose second leaf pty comes from the layout fallback', () => {
mockStoreState.terminalLayoutsByTabId[TAB_ID] = {
root: {
type: 'split',
direction: 'row',
first: { type: 'leaf', leafId: LEAF_ID },
second: { type: 'leaf', leafId: SECOND_LEAF_ID }
},
activeLeafId: LEAF_ID,
expandedLeafId: null,
ptyIdsByLeafId: { [LEAF_ID]: PTY_ID, [SECOND_LEAF_ID]: 'pty-local-detached' }
}
expect(isEvictionExemptTerminalTab({ id: TAB_ID, ptyId: PTY_ID }, WORKTREE_ID)).toBe(true)
})
it('does not exempt a split tab whose panes are all snapshot-backed', () => {
capturePanes([
{ ptyId: PTY_ID, paneId: 1, leafId: LEAF_ID, drivesTabTitle: true },
{ ptyId: SECOND_PTY_ID, paneId: 2, leafId: SECOND_LEAF_ID, drivesTabTitle: false }
])
expect(isEvictionExemptTerminalTab({ id: TAB_ID, ptyId: PTY_ID }, WORKTREE_ID)).toBe(false)
})
it('exempts on tab.ptyId alone when no panes resolve', () => {
expect(
isEvictionExemptTerminalTab({ id: TAB_ID, ptyId: 'pty-local-detached' }, WORKTREE_ID)
).toBe(true)
})
it('never exempts remote-runtime or SSH panes', () => {
capturePanes([
{ ptyId: 'remote:env-1@@t-1', paneId: 1, leafId: LEAF_ID, drivesTabTitle: true },
{ ptyId: 'ssh:conn-1@@pty-1', paneId: 2, leafId: SECOND_LEAF_ID, drivesTabTitle: false }
])
expect(isEvictionExemptTerminalTab({ id: TAB_ID, ptyId: null }, WORKTREE_ID)).toBe(false)
})
})
})
describe('fallbackParkedPaneCandidates', () => {
@@ -21,6 +21,7 @@ import {
isParkRestorableTerminalPty,
type TerminalParkRestorePolicy
} from './terminal-hidden-view-parking'
import { isEvictionExemptTerminalPty } from './terminal-hidden-worktree-retention'
import {
resolveTabTitleAfterPaneClose,
shouldClearLaunchAgentForClosedPane
@@ -106,6 +107,27 @@ function parkRestorePolicyFromState(state: {
return { sshParkingEnabled: state.settings?.terminalSshViewParking !== false }
}
/**
* Whether force-park must keep this tab's panes mounted: ANY of its pane PTYs is
* one a remount could not reattach. Resolved from the same pane candidates as
* canWatcherCoverParkedTerminalTab — tab.ptyId is only the first leaf's PTY, so
* a split tab whose SECOND leaf holds the unrestorable PTY fails coverage (and
* so becomes a retention candidate) yet would otherwise look exempt-free and
* unmount, orphaning that live shell. tab.ptyId stays in the union in case pane
* resolution misses it (no layout, no capture).
*/
export function isEvictionExemptTerminalTab(
tab: ParkableTerminalTabModel,
worktreeId: string
): boolean {
if (isEvictionExemptTerminalPty(tab.ptyId, worktreeId)) {
return true
}
return resolveParkedTerminalPaneCandidates(tab, useAppStore.getState()).some((pane) =>
isEvictionExemptTerminalPty(pane.ptyId, worktreeId)
)
}
/**
* Whether parked byte watchers can fully cover this tab's PTYs (every candidate
* has a park-restorable PTY on a valid leaf). Hosts must refuse to park a tab
@@ -22,11 +22,11 @@ import {
recordParkVerdictFlips,
type ParkVerdictFlipRecord
} from './terminal-park-verdict-flip-telemetry'
import { isEvictionExemptTerminalTab } from './terminal-hidden-worktree-retention'
import { getTerminalParkingPolicyOverrides } from './terminal-parking-e2e-overrides'
import {
canWatcherCoverParkedTerminalTab,
disposeParkedTerminalWatchersForWorktree,
isEvictionExemptTerminalTab,
syncParkedTerminalTabWatchers
} from './terminal-parked-tab-watchers'