diff --git a/src/renderer/src/components/terminal-pane/terminal-cold-park-recheck-deadlines.test.ts b/src/renderer/src/components/terminal-pane/terminal-cold-park-recheck-deadlines.test.ts index b5692be08b1..51a0716c13b 100644 --- a/src/renderer/src/components/terminal-pane/terminal-cold-park-recheck-deadlines.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-cold-park-recheck-deadlines.test.ts @@ -169,4 +169,45 @@ describe('getTerminalTabColdParkRecheckDelayMs', () => { }) ).toBe(250) }) + + // Why: a flip-damped tab is long past every hysteresis deadline, so the pin + // expiry is the only wakeup left — drop it and the tab never re-parks. + it('wakes at the flip-damping pin deadline when all ordinary deadlines have passed', () => { + expect( + getTerminalTabColdParkRecheckDelayMs({ + parkingEnabled: true, + hiddenSinceMs: 1_000, + nowMs: 1_000_000, + coldParkDelayMs: 100, + hotRetainMs: 1_000, + parkVerdictPinUntilMs: 1_060_000 + }) + ).toBe(60_000) + }) + + // Why: the earliest pending deadline wins — a pin must not delay a nearer + // cool-down wakeup, and vice versa. + it('takes the nearest of the pin and cool-down deadlines', () => { + const base = { + parkingEnabled: true, + hiddenSinceMs: 1_000, + nowMs: 2_500, + coldParkDelayMs: 100, + hotRetainMs: 1_000 + } + expect( + getTerminalTabColdParkRecheckDelayMs({ + ...base, + parkCooldownUntilMs: 2_750, + parkVerdictPinUntilMs: 62_500 + }) + ).toBe(250) + expect( + getTerminalTabColdParkRecheckDelayMs({ + ...base, + parkCooldownUntilMs: 62_500, + parkVerdictPinUntilMs: 2_750 + }) + ).toBe(250) + }) }) diff --git a/src/renderer/src/components/terminal-pane/terminal-cold-park-recheck-deadlines.ts b/src/renderer/src/components/terminal-pane/terminal-cold-park-recheck-deadlines.ts index ba106054c87..603cf2f34b4 100644 --- a/src/renderer/src/components/terminal-pane/terminal-cold-park-recheck-deadlines.ts +++ b/src/renderer/src/components/terminal-pane/terminal-cold-park-recheck-deadlines.ts @@ -18,6 +18,7 @@ function nextColdParkDeadlineDelayMs(args: { hotRetainMs: number retentionTtlMs?: number parkCooldownUntilMs?: number | null + parkVerdictPinUntilMs?: number | null }): number | null { if (!args.parkingEnabled || args.hiddenSinceMs === null) { return null @@ -27,7 +28,9 @@ function nextColdParkDeadlineDelayMs(args: { args.hiddenSinceMs + args.hotRetainMs, ...(args.retentionTtlMs !== undefined ? [args.hiddenSinceMs + args.retentionTtlMs] : []), // Why: the cool-down holds past-deadline candidates out of the parked set; without this wakeup nothing re-parks them. - ...(args.parkCooldownUntilMs != null ? [args.parkCooldownUntilMs] : []) + ...(args.parkCooldownUntilMs != null ? [args.parkCooldownUntilMs] : []), + // Why: same for the flip-damping pin — and it is worse, because the pin's job is to stop the churn that would otherwise re-run the verdict effect. + ...(args.parkVerdictPinUntilMs != null ? [args.parkVerdictPinUntilMs] : []) ].filter((deadlineMs) => deadlineMs > args.nowMs) return pendingDeadlines.length === 0 ? null : Math.min(...pendingDeadlines) - args.nowMs } @@ -60,6 +63,8 @@ export function getTerminalTabColdParkRecheckDelayMs(args: { coldParkDelayMs?: number hotRetainMs?: number parkCooldownUntilMs?: number | null + /** Provided only for tabs damped out of the parked set by flip churn. */ + parkVerdictPinUntilMs?: number | null }): number | null { return nextColdParkDeadlineDelayMs({ parkingEnabled: args.parkingEnabled, @@ -67,6 +72,7 @@ export function getTerminalTabColdParkRecheckDelayMs(args: { nowMs: args.nowMs, coldParkDelayMs: args.coldParkDelayMs ?? TERMINAL_TAB_COLD_PARK_DELAY_MS, hotRetainMs: args.hotRetainMs ?? TERMINAL_TAB_HOT_RETAIN_MS, - parkCooldownUntilMs: args.parkCooldownUntilMs + parkCooldownUntilMs: args.parkCooldownUntilMs, + parkVerdictPinUntilMs: args.parkVerdictPinUntilMs }) } diff --git a/src/renderer/src/components/terminal-pane/terminal-cold-park-verdict-loop.test.tsx b/src/renderer/src/components/terminal-pane/terminal-cold-park-verdict-loop.test.tsx new file mode 100644 index 00000000000..ad69325cb02 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-cold-park-verdict-loop.test.tsx @@ -0,0 +1,245 @@ +/** @vitest-environment happy-dom */ +/** + * Cold-park verdict oscillation (crash cluster: React #185 in the terminal overlay). + * + * Why this shape: canWatcherCoverParkedTerminalTab is re-derived from store and + * registry state that mounting/unmounting the very pane the verdict controls + * rewrites, and the parked watchers write the tab model back. The pane stand-in + * models that edge — mounted pane grants coverage, parked pane withdraws it, + * and either transition re-mints the tab array the cold-park effect depends on. + * + * Why the store-write chain is parameterised: React bails on *commits*, not on + * flips, so a damping threshold expressed in flips is only safe if it holds at + * the real cycle's commits-per-flip cost (pane-connect updateTabPtyId, the + * watcher-sync writes, the overlay's own subscriber renders). The chain models + * that cost so the derived burst limit stays regression-tested. + */ +import { act, useEffect, useRef, useState } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { TerminalTab } from '../../../../shared/types' + +;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + +const park = vi.hoisted(() => ({ + worktreeId: 'repo::/wt-park-loop', + /** Whether the byte watchers could cover the tab if it parked right now. */ + coverage: true +})) + +vi.mock('../../store', async () => { + const { create } = await import('zustand') + const useAppStore = create(() => ({ + pendingStartupByTabId: {} as Record, + runtimeStatusByEnvironmentId: new Map(), + settings: {} as Record, + terminalLayoutsByTabId: {} as Record, + runtimePaneTitlesByTabId: {} as Record, + tabsByWorktree: {} as Record + })) + return { useAppStore } +}) + +vi.mock('./terminal-parked-tab-watchers', () => ({ + canWatcherCoverParkedTerminalTab: () => park.coverage, + disposeParkedTerminalWatchersForWorktree: () => {}, + resolveParkedTerminalPaneCandidates: () => [], + syncParkedTerminalTabWatchers: () => {} +})) + +vi.mock('./terminal-parking-e2e-overrides', () => ({ + getTerminalParkingPolicyOverrides: () => ({ coldParkDelayMs: 0, hotRetainMs: 0 }) +})) + +import { useAppStore } from '../../store' +import { + TERMINAL_TAB_PARK_FLIP_BURST_LIMIT, + TERMINAL_TAB_PARK_FLIP_COMMIT_COST +} from './terminal-park-verdict-flip-telemetry' +import { useTerminalTabColdParking } from './use-terminal-tab-cold-parking' + +/** The most recently hidden tab holds the last-active retain exemption. */ +const EXEMPT_TAB_ID = 'tab-a' +const PARKABLE_TAB_ID = 'tab-b' +/** Stops an unfixed loop from hanging the run if React ever raises its bail. */ +const RENDER_HARD_STOP = 500 +/** +1: the pin lands in the passive effect observing the burst flip, so the + * verdict settles one flip after damping engages. */ +const SETTLED_FLIP_BUDGET = TERMINAL_TAB_PARK_FLIP_BURST_LIMIT + 1 +const EMPTY_ASSIGNMENTS = new Map() +const EMPTY_PORTALS: never[] = [] + +type ParkingStoreState = { tabsByWorktree: Record } + +const parkingStore = useAppStore as unknown as { + setState: (partial: (state: ParkingStoreState) => Partial) => void +} + +function terminalTab(id: string): TerminalTab { + return { id, ptyId: `${park.worktreeId}@@session-${id}`, title: id } as TerminalTab +} + +let tabModelRevision = 0 + +// Models the store writes a pane mount/unmount produces — updateTabPtyId on +// connect, parked-watcher title writes — both of which re-mint the tab array. +function rewriteTabModel(tabId: string): void { + tabModelRevision += 1 + const revision = tabModelRevision + parkingStore.setState((state) => ({ + tabsByWorktree: { + ...state.tabsByWorktree, + [park.worktreeId]: (state.tabsByWorktree[park.worktreeId] ?? []).map((tab) => + tab.id === tabId ? { ...tab, title: `${tab.id}-${revision}` } : tab + ) + } + })) +} + +/** Store writes a single park transition costs; each lands in its own commit. */ +let storeCommitsPerParkTransition = 1 +let paneMountCount = 0 + +/** The pane whose mount/unmount the park verdict authorizes. */ +function TerminalPaneStandIn(): null { + useEffect(() => { + paneMountCount += 1 + }, []) + return null +} + +/** + * The store writes a park transition drags behind it — pane-connect + * updateTabPtyId, then the watcher-sync writes that each only see the tab model + * the previous write produced. Coverage is re-derived from the last of them, so + * the verdict cannot flip until the whole chain has committed. Why staged + * rather than batched: nested commits are what React counts against + * NESTED_UPDATE_LIMIT, so this is the per-flip cost the damping must beat. + */ +function ParkTransitionStoreWrites({ parked }: { parked: boolean }): null { + const pendingWritesRef = useRef(storeCommitsPerParkTransition) + const lastParkedRef = useRef(parked) + const [writeTick, setWriteTick] = useState(0) + useEffect(() => { + if (lastParkedRef.current !== parked) { + lastParkedRef.current = parked + pendingWritesRef.current = storeCommitsPerParkTransition + } + if (pendingWritesRef.current <= 0) { + return + } + pendingWritesRef.current -= 1 + if (pendingWritesRef.current === 0) { + // A mounted pane grants byte-watcher coverage; a parked one withdraws it. + park.coverage = !parked + } + rewriteTabModel(PARKABLE_TAB_ID) + setWriteTick((tick) => tick + 1) + }, [parked, writeTick]) + return null +} + +let hostRenderCount = 0 +let parkVerdictFlipCount = 0 +let lastParkVerdict = false + +function OverlayHost(): React.JSX.Element | null { + hostRenderCount += 1 + const terminalTabs = useAppStore( + (state) => (state as ParkingStoreState).tabsByWorktree[park.worktreeId] + ) as TerminalTab[] + const parkedTerminalTabIds = useTerminalTabColdParking({ + worktreeId: park.worktreeId, + terminalTabs, + assignments: EMPTY_ASSIGNMENTS, + isWorktreeActive: false, + coldParkTerminalPanes: false, + shouldMeasureHiddenWorktree: false, + activityTerminalPortals: EMPTY_PORTALS, + activationDeferredMountTabIds: null + }) + const parked = parkedTerminalTabIds.has(PARKABLE_TAB_ID) + if (parked !== lastParkVerdict) { + parkVerdictFlipCount += 1 + } + lastParkVerdict = parked + if (hostRenderCount > RENDER_HARD_STOP) { + return null + } + return ( + <> + {parked ? null : } + + + ) +} + +function renderOverlayHost(root: Root): unknown { + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) + let thrown: unknown = null + try { + act(() => { + root.render() + }) + } catch (error) { + thrown = error + } + consoleError.mockRestore() + return thrown +} + +describe('cold-park verdict oscillation', () => { + let container: HTMLDivElement + let root: Root + + beforeEach(() => { + park.coverage = true + hostRenderCount = 0 + parkVerdictFlipCount = 0 + lastParkVerdict = false + tabModelRevision = 0 + paneMountCount = 0 + storeCommitsPerParkTransition = 1 + parkingStore.setState(() => ({ + tabsByWorktree: { + [park.worktreeId]: [terminalTab(EXEMPT_TAB_ID), terminalTab(PARKABLE_TAB_ID)] + } + })) + container = document.createElement('div') + document.body.appendChild(container) + }) + + afterEach(() => { + act(() => { + root.unmount() + }) + container.remove() + }) + + it('settles the park verdict when watcher coverage tracks the pane it unmounts', () => { + root = createRoot(container) + const thrown = renderOverlayHost(root) + + // Damping must land inside the burst window and settle on the safe side: + // the pane stays mounted, so bells/titles/completions never go silent. + expect(thrown).toBeNull() + expect(parkVerdictFlipCount).toBeLessThanOrEqual(SETTLED_FLIP_BUDGET) + expect(lastParkVerdict).toBe(false) + expect(paneMountCount).toBeGreaterThan(0) + }) + + // Why: the burst limit is derived from React's 50-commit bail divided by an + // assumed worst-case commits-per-flip. Running the loop at exactly that cost + // is what keeps the derivation honest — a limit tuned on a one-write-per-flip + // model pins only after React has already thrown #185. + it('settles at the assumed worst-case commit cost per park transition', () => { + storeCommitsPerParkTransition = TERMINAL_TAB_PARK_FLIP_COMMIT_COST + root = createRoot(container) + const thrown = renderOverlayHost(root) + + expect(thrown).toBeNull() + expect(parkVerdictFlipCount).toBeLessThanOrEqual(SETTLED_FLIP_BUDGET) + expect(lastParkVerdict).toBe(false) + expect(paneMountCount).toBeGreaterThan(0) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-park-verdict-flip-telemetry.test.ts b/src/renderer/src/components/terminal-pane/terminal-park-verdict-flip-telemetry.test.ts index 8f53bf39c5e..4a245c2c021 100644 --- a/src/renderer/src/components/terminal-pane/terminal-park-verdict-flip-telemetry.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-park-verdict-flip-telemetry.test.ts @@ -1,7 +1,11 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { + TERMINAL_TAB_PARK_FLIP_BURST_LIMIT, + TERMINAL_TAB_PARK_FLIP_BURST_WINDOW_MS, + TERMINAL_TAB_PARK_FLIP_COMMIT_COST, TERMINAL_TAB_PARK_FLIP_NOTICE_LIMIT, TERMINAL_TAB_PARK_FLIP_WINDOW_MS, + getParkVerdictUnparkPinUntilMs, recordParkVerdictFlips, type ParkVerdictFlipRecord } from './terminal-park-verdict-flip-telemetry' @@ -12,6 +16,8 @@ vi.mock('@/lib/crash-breadcrumb-recorder', () => ({ })) const TAB = 'tab-1' +/** Slower than the burst window, so only the notice limit can fire. */ +const SLOW_CHURN_STEP_MS = TERMINAL_TAB_PARK_FLIP_BURST_WINDOW_MS * 4 function observe(args: { records: Map @@ -31,6 +37,16 @@ beforeEach(() => { recordBreadcrumb.mockClear() }) +// Why asserted: the whole point of the burst trigger is that it is derived from +// React's 50-commit bail, not copied from the breadcrumb notice limit. If the +// two ever converge again the damping stops firing before React throws #185. +describe('burst damping threshold', () => { + it('stays under React NESTED_UPDATE_LIMIT at the assumed commits-per-flip cost', () => { + expect(TERMINAL_TAB_PARK_FLIP_BURST_LIMIT * TERMINAL_TAB_PARK_FLIP_COMMIT_COST).toBeLessThan(50) + expect(TERMINAL_TAB_PARK_FLIP_BURST_LIMIT).toBeLessThan(TERMINAL_TAB_PARK_FLIP_NOTICE_LIMIT) + }) +}) + describe('recordParkVerdictFlips', () => { it('stays silent for a stable verdict', () => { const records = new Map() @@ -42,7 +58,7 @@ describe('recordParkVerdictFlips', () => { expect(records.get(TAB)?.flips).toBe(0) }) - it('emits one breadcrumb per window once the verdict churns', () => { + it('emits one burst breadcrumb once the verdict churns at render cadence', () => { const records = new Map() for (let i = 0; i < 40; i += 1) { observe({ records, parked: i % 2 === 0, nowMs: 1_000 + i * 10 }) @@ -51,34 +67,61 @@ describe('recordParkVerdictFlips', () => { expect(recordBreadcrumb).toHaveBeenCalledTimes(1) expect(recordBreadcrumb).toHaveBeenCalledWith( 'terminal_park_verdict_churn', - expect.objectContaining({ tabId: TAB, flips: TERMINAL_TAB_PARK_FLIP_NOTICE_LIMIT }) + expect.objectContaining({ + tabId: TAB, + trigger: 'burst', + flips: TERMINAL_TAB_PARK_FLIP_BURST_LIMIT, + pinnedForMs: TERMINAL_TAB_PARK_FLIP_WINDOW_MS + }) ) }) - // Why: flips saturates at the notice limit, so elapsedMs is the only field - // that tells a runaway loop apart from slow churn — assert both ends. - it('reports elapsedMs so a tight loop is distinguishable from slow churn', () => { + // Why: the two triggers answer different questions — 'burst' means damping + // engaged before React could bail, 'window' means churn too slow to loop. + it('separates a damped burst from slow churn', () => { const tightRecords = new Map() for (let i = 0; i < 40; i += 1) { observe({ records: tightRecords, parked: i % 2 === 0, nowMs: 1_000 + i }) } expect(recordBreadcrumb).toHaveBeenCalledWith( 'terminal_park_verdict_churn', - expect.objectContaining({ flips: TERMINAL_TAB_PARK_FLIP_NOTICE_LIMIT, elapsedMs: 12 }) + expect.objectContaining({ + trigger: 'burst', + flips: TERMINAL_TAB_PARK_FLIP_BURST_LIMIT, + elapsedMs: TERMINAL_TAB_PARK_FLIP_BURST_LIMIT, + windowMs: TERMINAL_TAB_PARK_FLIP_BURST_WINDOW_MS + }) ) recordBreadcrumb.mockClear() const slowRecords = new Map() - for (let i = 0; i < 13; i += 1) { - observe({ records: slowRecords, parked: i % 2 === 0, nowMs: 1_000 + i * 4_000 }) + for (let i = 0; i < TERMINAL_TAB_PARK_FLIP_NOTICE_LIMIT + 1; i += 1) { + observe({ records: slowRecords, parked: i % 2 === 0, nowMs: 1_000 + i * SLOW_CHURN_STEP_MS }) } + expect(recordBreadcrumb).toHaveBeenCalledTimes(1) expect(recordBreadcrumb).toHaveBeenCalledWith( 'terminal_park_verdict_churn', - expect.objectContaining({ flips: TERMINAL_TAB_PARK_FLIP_NOTICE_LIMIT, elapsedMs: 48_000 }) + expect.objectContaining({ + trigger: 'window', + flips: TERMINAL_TAB_PARK_FLIP_NOTICE_LIMIT, + elapsedMs: TERMINAL_TAB_PARK_FLIP_NOTICE_LIMIT * SLOW_CHURN_STEP_MS, + windowMs: TERMINAL_TAB_PARK_FLIP_WINDOW_MS + }) ) }) + // Why: slow churn must not be damped — parking it out for a minute would cost + // a mounted pane's memory for a verdict that was never near React's bail. + it('does not pin churn spread past the burst window', () => { + const records = new Map() + for (let i = 0; i < TERMINAL_TAB_PARK_FLIP_NOTICE_LIMIT + 1; i += 1) { + observe({ records, parked: i % 2 === 0, nowMs: 1_000 + i * SLOW_CHURN_STEP_MS }) + } + + expect(records.get(TAB)?.pinnedUntilMs ?? null).toBeNull() + }) + it('re-arms after the window elapses', () => { const records = new Map() for (let i = 0; i < 40; i += 1) { @@ -116,23 +159,27 @@ describe('recordParkVerdictFlips', () => { expect(records.get(TAB)?.flips).toBe(1) }) - it('honours flipWindowMs and noticeLimit overrides', () => { + it('honours the window, notice, burst-window and burst-limit overrides', () => { const records = new Map() for (let i = 0; i < 10; i += 1) { recordParkVerdictFlips({ records, liveTabIds: new Set([TAB]), nextParkedTabIds: i % 2 === 0 ? new Set([TAB]) : new Set(), - nowMs: 1_000 + i, - flipWindowMs: 500, - noticeLimit: 3 + nowMs: 1_000 + i * 100, + flipWindowMs: 5_000, + noticeLimit: 3, + burstWindowMs: 10, + burstLimit: 2 }) } + // Why 'window': the 100ms step outruns the 10ms burst window, so the burst + // counter resets on every flip and only the notice limit can fire. expect(recordBreadcrumb).toHaveBeenCalledTimes(1) expect(recordBreadcrumb).toHaveBeenCalledWith( 'terminal_park_verdict_churn', - expect.objectContaining({ flips: 3, windowMs: 500 }) + expect.objectContaining({ trigger: 'window', flips: 3, windowMs: 5_000 }) ) }) @@ -171,3 +218,52 @@ describe('recordParkVerdictFlips', () => { expect(records.size).toBe(0) }) }) + +describe('getParkVerdictUnparkPinUntilMs', () => { + function churnToBurst(records: Map, startMs = 1_000): number { + for (let i = 0; i < 40; i += 1) { + observe({ records, parked: i % 2 === 0, nowMs: startMs + i * 10 }) + } + // The first observation only seeds the record, so flip N lands one step later. + return startMs + TERMINAL_TAB_PARK_FLIP_BURST_LIMIT * 10 + } + + it('does not pin a stable verdict', () => { + const records = new Map() + observe({ records, parked: true, nowMs: 1_000 }) + + expect(getParkVerdictUnparkPinUntilMs({ records, tabId: TAB, nowMs: 2_000 })).toBeNull() + expect(getParkVerdictUnparkPinUntilMs({ records, tabId: 'missing', nowMs: 2_000 })).toBeNull() + }) + + // Why the deadline and not a boolean: the caller has to schedule a recheck at + // it, or the pin never lifts once it has stopped the churn that woke the + // verdict effect. + it('reports the pin deadline one window out, then re-arms', () => { + const records = new Map() + const pinnedAtMs = churnToBurst(records) + const pinUntilMs = pinnedAtMs + TERMINAL_TAB_PARK_FLIP_WINDOW_MS + + expect(getParkVerdictUnparkPinUntilMs({ records, tabId: TAB, nowMs: pinnedAtMs + 1 })).toBe( + pinUntilMs + ) + expect(getParkVerdictUnparkPinUntilMs({ records, tabId: TAB, nowMs: pinUntilMs })).toBeNull() + expect(records.get(TAB)?.flips).toBe(0) + expect(records.get(TAB)?.burstFlips).toBe(0) + + recordBreadcrumb.mockClear() + churnToBurst(records, pinUntilMs) + expect(recordBreadcrumb).toHaveBeenCalledTimes(1) + expect(getParkVerdictUnparkPinUntilMs({ records, tabId: TAB, nowMs: pinUntilMs + 1 })).not.toBe( + null + ) + }) + + // Why: a backwards clock jump must release the pin, not strand it for a window. + it('releases the pin when the clock jumps backwards', () => { + const records = new Map() + churnToBurst(records) + + expect(getParkVerdictUnparkPinUntilMs({ records, tabId: TAB, nowMs: 5 })).toBeNull() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-park-verdict-flip-telemetry.ts b/src/renderer/src/components/terminal-pane/terminal-park-verdict-flip-telemetry.ts index f54477c90f0..d394032dc3d 100644 --- a/src/renderer/src/components/terminal-pane/terminal-park-verdict-flip-telemetry.ts +++ b/src/renderer/src/components/terminal-pane/terminal-park-verdict-flip-telemetry.ts @@ -1,36 +1,111 @@ /** - * Cold-park verdict flip telemetry (observation only — changes no verdict). + * Cold-park verdict flip telemetry and damping. * - * Why: crash cluster C5 (React #185 in TerminalPaneOverlayLayer) points at a - * setState loop in the cold-parking passive effect, but no park-flip loop has - * been reproduced, and the capture-coverage oscillator that looked like the - * cause provably self-terminates (the unmount capture is never cleared on - * remount, so coverage pins rather than alternates). This records whether the - * rendered park verdict actually churns in the field. Deliberately no damping: - * guarding a loop nobody has shown can run would add a park delay and a - * retention path for no demonstrated benefit. + * Why: crash cluster C5 (React #185 in TerminalPaneOverlayLayer) is a park-flip + * loop — the veto (canWatcherCoverParkedTerminalTab) is re-derived from store + * and registry state that mounting/unmounting the very pane the verdict + * controls rewrites, so the verdict can alternate at render cadence. Field + * bundles carry 12 flips in as little as 348ms, which falsified the original + * "deliberately no damping" stance, so the record is a policy input too: a tab + * whose verdict bursts pins to NOT-parked for one window (the safe side — a + * mounted pane never goes silent for bells, titles or completions). + * + * Why two thresholds: React bails on *commits*, not on flips, and one flip in + * the real cycle costs many commits (pane-connect updateTabPtyId, the + * watcher-sync writes, the overlay's own subscriber renders). The damping + * trigger is therefore derived from React's commit budget and is far tighter + * than the breadcrumb notice limit, which exists only to keep the crumb rare. * * Reading the signal: breadcrumbs also emit a durable `renderer.breadcrumb` - * trace span, so this is queryable without waiting for a crash bundle. `flips` - * always equals the notice limit — `elapsedMs` is what separates a render loop - * from slow churn. Silence only rules out churn at effect cadence; a same-tick - * cascade can crash before the limit accumulates. If it fires, start with the - * candidate selection in terminal-hidden-view-parking.ts. + * trace span, so this is queryable without waiting for a crash bundle. + * `trigger: 'burst'` is the damped render loop (and always pins); `'window'` is + * slow churn that never got near React's bail. A still-churning tab re-pins + * once per window, so a repeating crumb means the oscillator is still live. */ import { recordRendererCrashBreadcrumb } from '@/lib/crash-breadcrumb-recorder' export const TERMINAL_TAB_PARK_FLIP_WINDOW_MS = 60_000 -/** Flips per window that no sane park policy should reach. */ +/** Flips per window that no sane park policy should reach. Breadcrumb only. */ export const TERMINAL_TAB_PARK_FLIP_NOTICE_LIMIT = 12 +/** react-dom 19.2.x NESTED_UPDATE_LIMIT — the commit budget a loop must beat. */ +const REACT_NESTED_UPDATE_LIMIT = 50 +/** + * Commits the pinned verdict still costs to settle after damping engages — the + * pin lands in the passive effect that observes the burst, so the verdict + * settles one flip later. Measured at 5 by terminal-cold-park-verdict-loop. + */ +const PARK_PIN_SETTLE_COMMITS = 6 +/** + * Commits one verdict flip costs, worst case: the pane mount/unmount, the + * pane-connect updateTabPtyId, the parked-watcher writes (clearTabLaunchAgent, + * setTabLayout, updateTabTitle) and the overlay's zustand subscriber renders. + * Raising this tightens the burst limit automatically — never hand-tune both. + */ +export const TERMINAL_TAB_PARK_FLIP_COMMIT_COST = 12 +/** + * Flips that must pin before React throws #185. Deliberately NOT the notice + * limit: at the commit cost above, 12 flips is ~144 commits, so a notice-limit + * pin never fires in the field — React bails long before it. + */ +export const TERMINAL_TAB_PARK_FLIP_BURST_LIMIT = Math.max( + 2, + Math.floor( + (REACT_NESTED_UPDATE_LIMIT - PARK_PIN_SETTLE_COMMITS) / TERMINAL_TAB_PARK_FLIP_COMMIT_COST + ) +) +/** + * Burst horizon. Only a render-cadence loop reaches the burst limit inside it: + * an honest park needs the 30s cold-park hysteresis in one direction, so + * sub-second park/unpark round trips are always the pathology. + */ +export const TERMINAL_TAB_PARK_FLIP_BURST_WINDOW_MS = 1_000 + export type ParkVerdictFlipRecord = { parked: boolean windowStartMs: number flips: number notified: boolean + burstStartMs: number + burstFlips: number + /** Set when a flip burst engaged damping; the verdict stays unparked until then. */ + pinnedUntilMs?: number | null } -/** Records park-verdict churn per tab; emits one breadcrumb per window. */ +function resetFlipWindows(record: ParkVerdictFlipRecord, nowMs: number): void { + record.windowStartMs = nowMs + record.flips = 0 + record.notified = false + record.burstStartMs = nowMs + record.burstFlips = 0 + record.pinnedUntilMs = null +} + +/** + * When this tab's NOT-parked damping pin expires, or null when it is unpinned. + * Callers must schedule a recheck at the returned deadline: the pin's whole job + * is to stop the churn that would otherwise re-run the verdict effect, so + * without that wakeup nothing ever re-parks the tab. Expiry reopens the window + * so a still-churning tab re-pins (and re-breadcrumbs) instead of pinning + * forever on one old burst. + */ +export function getParkVerdictUnparkPinUntilMs(args: { + records: Map + tabId: string + nowMs: number +}): number | null { + const record = args.records.get(args.tabId) + if (record?.pinnedUntilMs == null) { + return null + } + if (args.nowMs >= record.pinnedUntilMs || args.nowMs < record.windowStartMs) { + resetFlipWindows(record, args.nowMs) + return null + } + return record.pinnedUntilMs +} + +/** Records park-verdict churn per tab; damps bursts and breadcrumbs the rest. */ export function recordParkVerdictFlips(args: { records: Map liveTabIds: ReadonlySet @@ -38,6 +113,8 @@ export function recordParkVerdictFlips(args: { nowMs: number flipWindowMs?: number noticeLimit?: number + burstWindowMs?: number + burstLimit?: number }): void { const { records, @@ -45,7 +122,9 @@ export function recordParkVerdictFlips(args: { nextParkedTabIds, nowMs, flipWindowMs = TERMINAL_TAB_PARK_FLIP_WINDOW_MS, - noticeLimit = TERMINAL_TAB_PARK_FLIP_NOTICE_LIMIT + noticeLimit = TERMINAL_TAB_PARK_FLIP_NOTICE_LIMIT, + burstWindowMs = TERMINAL_TAB_PARK_FLIP_BURST_WINDOW_MS, + burstLimit = TERMINAL_TAB_PARK_FLIP_BURST_LIMIT } = args for (const tabId of Array.from(records.keys())) { @@ -59,7 +138,15 @@ export function recordParkVerdictFlips(args: { const record = records.get(tabId) if (!record) { - records.set(tabId, { parked, windowStartMs: nowMs, flips: 0, notified: false }) + records.set(tabId, { + parked, + windowStartMs: nowMs, + flips: 0, + notified: false, + burstStartMs: nowMs, + burstFlips: 0, + pinnedUntilMs: null + }) continue } if (parked === record.parked) { @@ -70,20 +157,40 @@ export function recordParkVerdictFlips(args: { // elapsed value as a fresh window rather than trusting the delta. const elapsedMs = nowMs - record.windowStartMs if (elapsedMs >= flipWindowMs || elapsedMs < 0) { - record.windowStartMs = nowMs - record.flips = 0 - record.notified = false + resetFlipWindows(record, nowMs) + } + const burstElapsedMs = nowMs - record.burstStartMs + if (burstElapsedMs >= burstWindowMs || burstElapsedMs < 0) { + record.burstStartMs = nowMs + record.burstFlips = 0 } record.parked = parked record.flips += 1 + record.burstFlips += 1 - if (!record.notified && record.flips >= noticeLimit) { - record.notified = true - // Why: flips is always exactly noticeLimit here, so elapsedMs is the only - // field that separates a render loop from slow benign churn. + if (record.pinnedUntilMs == null && record.burstFlips >= burstLimit) { + record.pinnedUntilMs = nowMs + flipWindowMs recordRendererCrashBreadcrumb('terminal_park_verdict_churn', { tabId, + trigger: 'burst', + flips: record.burstFlips, + elapsedMs: nowMs - record.burstStartMs, + windowMs: burstWindowMs, + pinnedForMs: flipWindowMs + }) + continue + } + // Why pinnedUntilMs gates this: the burst crumb already reported the same + // window, so a second crumb would only double the volume the notice limit + // exists to keep down. + if (record.pinnedUntilMs == null && !record.notified && record.flips >= noticeLimit) { + record.notified = true + // Why: flips is always exactly noticeLimit here, so elapsedMs is the only + // field that separates slow churn from a burst the damping already caught. + recordRendererCrashBreadcrumb('terminal_park_verdict_churn', { + tabId, + trigger: 'window', flips: record.flips, elapsedMs: nowMs - record.windowStartMs, windowMs: flipWindowMs 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 7138edb37fc..e39f5cdf088 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 @@ -11,7 +11,9 @@ const mocks = vi.hoisted(() => ({ terminalLayoutsByTabId: {} as Record }> }, exemptTabIds: new Set(), - exemptSelectCalls: 0 + exemptSelectCalls: 0, + /** Toggled to churn the park verdict the way the crash cluster does. */ + watcherCoverage: true })) vi.mock('../../store', () => ({ @@ -35,15 +37,23 @@ vi.mock('./terminal-eviction-exempt-tabs', () => ({ })) vi.mock('./terminal-parked-tab-watchers', () => ({ - canWatcherCoverParkedTerminalTab: () => true, + canWatcherCoverParkedTerminalTab: () => mocks.watcherCoverage, disposeParkedTerminalWatchersForWorktree: vi.fn(), syncParkedTerminalTabWatchers: vi.fn() })) +vi.mock('@/lib/crash-breadcrumb-recorder', () => ({ + recordRendererCrashBreadcrumb: vi.fn() +})) + import { TERMINAL_TAB_COLD_PARK_DELAY_MS, TERMINAL_TAB_HOT_RETAIN_MS } from './terminal-hidden-view-parking' +import { + TERMINAL_TAB_PARK_FLIP_BURST_LIMIT, + TERMINAL_TAB_PARK_FLIP_WINDOW_MS +} from './terminal-park-verdict-flip-telemetry' import { useTerminalTabColdParking } from './use-terminal-tab-cold-parking' const WORKTREE_ID = 'wt-1' @@ -75,6 +85,7 @@ describe('useTerminalTabColdParking measure-clock contract', () => { vi.useRealTimers() mocks.exemptTabIds = new Set() mocks.exemptSelectCalls = 0 + mocks.watcherCoverage = true mocks.storeState.terminalLayoutsByTabId = {} mocks.storeState.runtimeStatusByEnvironmentId = new Map() }) @@ -106,6 +117,41 @@ describe('useTerminalTabColdParking measure-clock contract', () => { } }) + // Why: the flip-damping pin removes the tab from the parked set, and every + // hysteresis deadline of a long-hidden tab is already past — so the pin + // deadline is the only wakeup that can ever re-park it. Without scheduling + // it, damping silently becomes a permanent unpark: the pane (~4-5MB) stays + // mounted for the life of the window, which is the renderer-OOM cluster. + it('re-parks a damped tab once the pin expires with no other store change', () => { + const { result, rerender } = renderHook( + (args: ReturnType) => useTerminalTabColdParking(args), + { initialProps: hookArgs(false) } + ) + act(() => { + vi.advanceTimersByTime(TERMINAL_TAB_HOT_RETAIN_MS + 1) + }) + expect(result.current).toEqual(new Set(['tab-2'])) + + // Churn the coverage veto at render cadence — no clock advance, so every + // flip lands inside the burst window and damping engages. + for (let flip = 0; flip <= TERMINAL_TAB_PARK_FLIP_BURST_LIMIT + 1; flip += 1) { + mocks.watcherCoverage = flip % 2 === 1 + act(() => { + rerender(hookArgs(false)) + }) + } + mocks.watcherCoverage = true + act(() => { + rerender(hookArgs(false)) + }) + expect(result.current.size).toBe(0) + + act(() => { + vi.advanceTimersByTime(TERMINAL_TAB_PARK_FLIP_WINDOW_MS) + }) + expect(result.current).toEqual(new Set(['tab-2'])) + }) + // Why: the worktree layer preserves hiddenSince through a background-measure // window; the tab layer must share that contract (no clock reset) while a // post-measure cool-down prevents the instant re-park thrash. 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 c339f00fe0a..f639c3d00ce 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 @@ -21,6 +21,7 @@ import { type TerminalTabColdParkCandidate } from './terminal-hidden-view-parking' import { + getParkVerdictUnparkPinUntilMs, recordParkVerdictFlips, type ParkVerdictFlipRecord } from './terminal-park-verdict-flip-telemetry' @@ -204,9 +205,24 @@ export function useTerminalTabColdParking(args: { // Why: a tab the byte watchers cannot cover (no capture, no layout // snapshot, legacy leaf ids) must never park — it would go silent for // bells/titles/completions, the failure that sank the first attempt. + // Coverage is re-derived from state the authorized unmount rewrites, so a + // churning verdict is damped to NOT-parked (a mounted pane never goes + // silent) instead of looping React into #185. + const parkVerdictPinUntilMsByTabId = new Map() for (const terminalTab of terminalTabs) { + if (!nextColdParkedTerminalTabIds.has(terminalTab.id)) { + continue + } + const parkVerdictPinUntilMs = getParkVerdictUnparkPinUntilMs({ + records: parkVerdictRecordsRef.current, + tabId: terminalTab.id, + nowMs + }) + if (parkVerdictPinUntilMs !== null) { + parkVerdictPinUntilMsByTabId.set(terminalTab.id, parkVerdictPinUntilMs) + } if ( - nextColdParkedTerminalTabIds.has(terminalTab.id) && + parkVerdictPinUntilMs !== null || !canWatcherCoverParkedTerminalTab(worktreeId, terminalTab) ) { nextColdParkedTerminalTabIds.delete(terminalTab.id) @@ -230,6 +246,10 @@ export function useTerminalTabColdParking(args: { parkingEnabled: terminalParkingEnabled, hiddenSinceMs: candidate.hiddenSinceMs, parkCooldownUntilMs: measureParkCooldownUntilRef.current, + // Why: a pinned tab is past every hysteresis deadline, so the pin + // expiry is often the only pending wakeup left — and the pin exists + // precisely to stop the churn that would otherwise re-run this effect. + parkVerdictPinUntilMs: parkVerdictPinUntilMsByTabId.get(candidate.id) ?? null, nowMs, ...overrides })