diff --git a/DESIGN.md b/DESIGN.md index deada4413e2..399701e01cf 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -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 diff --git a/src/renderer/src/components/Terminal.tsx b/src/renderer/src/components/Terminal.tsx index 931bdd47b0a..bd8e39d4301 100644 --- a/src/renderer/src/components/Terminal.tsx +++ b/src/renderer/src/components/Terminal.tsx @@ -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() + 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) { diff --git a/src/renderer/src/components/terminal-pane/terminal-hidden-scrollback-demotion.test.ts b/src/renderer/src/components/terminal-pane/terminal-hidden-scrollback-demotion.test.ts index 47c6cb1e6c2..caa261c9722 100644 --- a/src/renderer/src/components/terminal-pane/terminal-hidden-scrollback-demotion.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-hidden-scrollback-demotion.test.ts @@ -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) diff --git a/src/renderer/src/components/terminal-pane/terminal-hidden-scrollback-demotion.ts b/src/renderer/src/components/terminal-pane/terminal-hidden-scrollback-demotion.ts index 6977cf2f919..b587cbf23d5 100644 --- a/src/renderer/src/components/terminal-pane/terminal-hidden-scrollback-demotion.ts +++ b/src/renderer/src/components/terminal-pane/terminal-hidden-scrollback-demotion.ts @@ -28,6 +28,16 @@ export function setScrollbackDemotedTerminalWorktrees(next: ReadonlySet) } } +/** + * 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) } diff --git a/src/renderer/src/components/terminal-pane/terminal-hidden-worktree-retention.test.ts b/src/renderer/src/components/terminal-pane/terminal-hidden-worktree-retention.test.ts index 3f3e986f589..1b51b9672cb 100644 --- a/src/renderer/src/components/terminal-pane/terminal-hidden-worktree-retention.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-hidden-worktree-retention.test.ts @@ -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) }) }) diff --git a/src/renderer/src/components/terminal-pane/terminal-hidden-worktree-retention.ts b/src/renderer/src/components/terminal-pane/terminal-hidden-worktree-retention.ts index a9f799c07a2..e2d24cd1bba 100644 --- a/src/renderer/src/components/terminal-pane/terminal-hidden-worktree-retention.ts +++ b/src/renderer/src/components/terminal-pane/terminal-hidden-worktree-retention.ts @@ -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, +// 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 } diff --git a/src/renderer/src/components/terminal-pane/terminal-parked-tab-watchers.test.ts b/src/renderer/src/components/terminal-pane/terminal-parked-tab-watchers.test.ts index b67645133de..7aeb20e2cc7 100644 --- a/src/renderer/src/components/terminal-pane/terminal-parked-tab-watchers.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-parked-tab-watchers.test.ts @@ -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', () => { diff --git a/src/renderer/src/components/terminal-pane/terminal-parked-tab-watchers.ts b/src/renderer/src/components/terminal-pane/terminal-parked-tab-watchers.ts index bdde930c6b2..2ec82a0ee52 100644 --- a/src/renderer/src/components/terminal-pane/terminal-parked-tab-watchers.ts +++ b/src/renderer/src/components/terminal-pane/terminal-parked-tab-watchers.ts @@ -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 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 2a179357091..874f0c01a84 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 @@ -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'