From e0c3ee5d35c2c2d5162c617fcb273292166a062f Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 9 Aug 2026 19:58:18 -0700 Subject: [PATCH] fix(terminal): harden parked watcher synchronization --- ...-cold-park-pre-gate-loop.react185.test.tsx | 2 + .../terminal-cold-park-verdict-loop.test.tsx | 1 + ...-watcher-synchronization.react185.test.tsx | 188 ++++++++++++++++++ ...parked-terminal-watcher-synchronization.ts | 105 +++++++++- .../use-terminal-tab-cold-parking.test.ts | 2 + .../use-terminal-tab-cold-parking.ts | 7 +- 6 files changed, 292 insertions(+), 13 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/use-parked-terminal-watcher-synchronization.react185.test.tsx diff --git a/src/renderer/src/components/terminal-pane/terminal-cold-park-pre-gate-loop.react185.test.tsx b/src/renderer/src/components/terminal-pane/terminal-cold-park-pre-gate-loop.react185.test.tsx index d689bef8dc4..902c3254557 100644 --- a/src/renderer/src/components/terminal-pane/terminal-cold-park-pre-gate-loop.react185.test.tsx +++ b/src/renderer/src/components/terminal-pane/terminal-cold-park-pre-gate-loop.react185.test.tsx @@ -33,7 +33,9 @@ vi.mock('../../store', async () => { activeGroupIdByWorktree: {} as Record, groupsByWorktree: {} as Record, pendingStartupByTabId: {} as Record, + ptyIdsByTabId: {} as Record, runtimeStatusByEnvironmentId: new Map(), + runtimePaneTitlesByTabId: {} as Record>, settings: {} as Record, sleepingAgentSessionsByPaneKey: {} as Record, tabsByWorktree: {} as Record, 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 index 97cb2db3428..9a97a10745b 100644 --- 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 @@ -31,6 +31,7 @@ vi.mock('../../store', async () => { const { create } = await import('zustand') const useAppStore = create(() => ({ pendingStartupByTabId: {} as Record, + ptyIdsByTabId: {} as Record, runtimeStatusByEnvironmentId: new Map(), settings: {} as Record, terminalLayoutsByTabId: {} as Record, diff --git a/src/renderer/src/components/terminal-pane/use-parked-terminal-watcher-synchronization.react185.test.tsx b/src/renderer/src/components/terminal-pane/use-parked-terminal-watcher-synchronization.react185.test.tsx new file mode 100644 index 00000000000..5732d745b60 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/use-parked-terminal-watcher-synchronization.react185.test.tsx @@ -0,0 +1,188 @@ +/** @vitest-environment happy-dom */ +import { act, StrictMode, useEffect } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { TerminalLayoutSnapshot, TerminalTab } from '../../../../shared/types' + +;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + +const WORKTREE_ID = 'repo::/parked-watcher-sync' +const TAB_ID = 'tab-1' +const FIRST_LEAF_ID = '11111111-1111-4111-8111-111111111111' +const SECOND_LEAF_ID = '22222222-2222-4222-8222-222222222222' +const FIRST_PTY_ID = `${WORKTREE_ID}@@session-1` +const OLD_SECOND_PTY_ID = `${WORKTREE_ID}@@session-2` +const NEW_SECOND_PTY_ID = `${WORKTREE_ID}@@session-3` + +const harness = vi.hoisted(() => ({ + syncCalls: 0, + disposeCalls: 0, + watchedPtyIds: new Set() +})) + +vi.mock('../../store', async () => { + const { create } = await import('zustand') + const useAppStore = create(() => ({ + ptyIdsByTabId: {} as Record, + runtimePaneTitlesByTabId: {} as Record>, + terminalLayoutsByTabId: {} as Record + })) + return { useAppStore } +}) + +vi.mock('./terminal-parked-tab-watchers', async () => { + const { useAppStore } = await import('../../store') + return { + disposeParkedTerminalWatchersForWorktree: () => { + harness.disposeCalls += 1 + harness.watchedPtyIds.clear() + }, + syncParkedTerminalTabWatchers: (args: { + tabs: readonly TerminalTab[] + parkedTabIds: ReadonlySet + }) => { + harness.syncCalls += 1 + const state = useAppStore.getState() + harness.watchedPtyIds.clear() + for (const tab of args.tabs) { + if (!args.parkedTabIds.has(tab.id)) { + continue + } + const layoutPtyIds = Object.values( + state.terminalLayoutsByTabId[tab.id]?.ptyIdsByLeafId ?? {} + ) + for (const ptyId of layoutPtyIds.length > 0 ? layoutPtyIds : [tab.ptyId]) { + if (ptyId) { + harness.watchedPtyIds.add(ptyId) + } + } + } + } + } +}) + +import { useAppStore } from '../../store' +import { disposeParkedTerminalWatchersForWorktree } from './terminal-parked-tab-watchers' +import { + captureParkedTerminalPaneCandidates, + capturedPanesByTabId +} from './terminal-parked-watcher-registry' +import { useParkedTerminalWatcherSynchronization } from './use-parked-terminal-watcher-synchronization' + +const terminalTabs = [{ id: TAB_ID, ptyId: FIRST_PTY_ID }] as TerminalTab[] +const parkedTabIds = new Set([TAB_ID]) + +function splitLayout(secondPtyId: string): TerminalLayoutSnapshot { + return { + root: { + type: 'split', + direction: 'vertical', + first: { type: 'leaf', leafId: FIRST_LEAF_ID }, + second: { type: 'leaf', leafId: SECOND_LEAF_ID } + }, + activeLeafId: FIRST_LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { + [FIRST_LEAF_ID]: FIRST_PTY_ID, + [SECOND_LEAF_ID]: secondPtyId + } + } +} + +function WatcherSynchronizationHarness(): null { + useParkedTerminalWatcherSynchronization({ + worktreeId: WORKTREE_ID, + terminalTabs, + inputsKey: JSON.stringify([[TAB_ID, FIRST_PTY_ID, null]]), + assignmentsKey: JSON.stringify([[TAB_ID, 'group-1', false]]), + parkedTabIds + }) + // Models the pre-fix sibling disposal effect that StrictMode replays after synchronization. + useEffect(() => () => disposeParkedTerminalWatchersForWorktree(WORKTREE_ID), []) + return null +} + +describe('parked terminal watcher synchronization', () => { + let container: HTMLDivElement + let root: Root | undefined + + beforeEach(() => { + harness.syncCalls = 0 + harness.disposeCalls = 0 + harness.watchedPtyIds.clear() + useAppStore.setState({ + ptyIdsByTabId: { [TAB_ID]: [FIRST_PTY_ID, OLD_SECOND_PTY_ID] }, + runtimePaneTitlesByTabId: { [TAB_ID]: { 1: 'active', 2: 'split' } }, + terminalLayoutsByTabId: { [TAB_ID]: splitLayout(OLD_SECOND_PTY_ID) } + }) + container = document.createElement('div') + document.body.appendChild(container) + }) + + afterEach(() => { + act(() => root?.unmount()) + root = undefined + capturedPanesByTabId.delete(TAB_ID) + container.remove() + }) + + it('reinstates parked watchers after StrictMode effect replay cleanup', () => { + root = createRoot(container) + + act(() => { + root?.render( + + + + ) + }) + + expect(harness.disposeCalls).toBeGreaterThanOrEqual(1) + expect(harness.syncCalls).toBe(2) + expect(harness.watchedPtyIds).toEqual(new Set([FIRST_PTY_ID, OLD_SECOND_PTY_ID])) + }) + + it('resynchronizes a parked split when only its detached PTY and layout change', () => { + root = createRoot(container) + act(() => { + root?.render() + }) + expect(harness.watchedPtyIds).toContain(OLD_SECOND_PTY_ID) + + act(() => { + useAppStore.setState({ + ptyIdsByTabId: { [TAB_ID]: [FIRST_PTY_ID, NEW_SECOND_PTY_ID] }, + terminalLayoutsByTabId: { [TAB_ID]: splitLayout(NEW_SECOND_PTY_ID) } + }) + harness.watchedPtyIds.delete(OLD_SECOND_PTY_ID) + }) + + expect(harness.syncCalls).toBe(2) + expect(harness.watchedPtyIds).toEqual(new Set([FIRST_PTY_ID, NEW_SECOND_PTY_ID])) + + act(() => { + useAppStore.setState({ + runtimePaneTitlesByTabId: { [TAB_ID]: { 1: '⠋ active', 2: 'new split title' } } + }) + }) + expect(harness.syncCalls).toBe(2) + + act(() => { + useAppStore.setState({ + terminalLayoutsByTabId: { + [TAB_ID]: { ...splitLayout(NEW_SECOND_PTY_ID), activeLeafId: SECOND_LEAF_ID } + } + }) + }) + expect(harness.syncCalls).toBe(3) + + captureParkedTerminalPaneCandidates(TAB_ID, WORKTREE_ID, [ + { ptyId: FIRST_PTY_ID, paneId: 10, leafId: FIRST_LEAF_ID, drivesTabTitle: false }, + { ptyId: NEW_SECOND_PTY_ID, paneId: 11, leafId: SECOND_LEAF_ID, drivesTabTitle: true } + ]) + act(() => { + root?.render() + }) + expect(harness.syncCalls).toBe(4) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/use-parked-terminal-watcher-synchronization.ts b/src/renderer/src/components/terminal-pane/use-parked-terminal-watcher-synchronization.ts index 91987c63be1..42c37627bed 100644 --- a/src/renderer/src/components/terminal-pane/use-parked-terminal-watcher-synchronization.ts +++ b/src/renderer/src/components/terminal-pane/use-parked-terminal-watcher-synchronization.ts @@ -1,12 +1,27 @@ import { useEffect, useRef } from 'react' -import type { TerminalTab } from '../../../../shared/types' -import { syncParkedTerminalTabWatchers } from './terminal-parked-tab-watchers' +import { useShallow } from 'zustand/react/shallow' +import type { TerminalLayoutSnapshot, TerminalTab } from '../../../../shared/types' +import { useAppStore } from '../../store' +import { + disposeParkedTerminalWatchersForWorktree, + syncParkedTerminalTabWatchers +} from './terminal-parked-tab-watchers' +import { capturedPanesByTabId } from './terminal-parked-watcher-registry' type TerminalParkingAssignment = { groupId: string isActiveInGroup: boolean } +const EMPTY_PTY_IDS: readonly string[] = [] + +type WatcherReconciliationStoreInputs = ( + | readonly string[] + | TerminalLayoutSnapshot + | string + | null +)[] + export function getTerminalParkingInputsKey(terminalTabs: readonly TerminalTab[]): string { return JSON.stringify(terminalTabs.map((tab) => [tab.id, tab.ptyId, tab.pendingActivationSpawn])) } @@ -29,16 +44,51 @@ function getWatcherSynchronizationKey(args: { assignmentsKey: string parkedTabIds: ReadonlySet activationDeferredMountTabIds?: ReadonlySet | null + reconciliationKey: string }): string { return JSON.stringify([ args.worktreeId, args.inputsKey, args.assignmentsKey, Array.from(args.parkedTabIds), - Array.from(args.activationDeferredMountTabIds ?? []).sort() + Array.from(args.activationDeferredMountTabIds ?? []).sort(), + args.reconciliationKey ]) } +function getCapturedPaneKey(terminalTabs: readonly TerminalTab[]): unknown[] { + return terminalTabs.map((tab) => { + const capture = capturedPanesByTabId.get(tab.id) + return [ + tab.id, + capture?.worktreeId ?? null, + capture?.panes.map((pane) => [pane.ptyId, pane.paneId, pane.leafId, pane.drivesTabTitle]) ?? + [] + ] + }) +} + +function getWatcherReconciliationKey( + terminalTabs: readonly TerminalTab[], + inputs: WatcherReconciliationStoreInputs +): string { + const tabs = terminalTabs.map((tab, index) => { + const offset = index * 3 + const ptyIds = inputs[offset] as readonly string[] + const layout = inputs[offset + 1] as TerminalLayoutSnapshot | null + const titleSlotKey = inputs[offset + 2] as string + return [ + tab.id, + ptyIds, + layout?.root ?? null, + layout?.activeLeafId ?? null, + layout?.ptyIdsByLeafId ?? null, + titleSlotKey + ] + }) + return JSON.stringify([tabs, getCapturedPaneKey(terminalTabs)]) +} + export function useParkedTerminalWatcherSynchronization(args: { worktreeId: string terminalTabs: readonly TerminalTab[] @@ -47,15 +97,39 @@ export function useParkedTerminalWatcherSynchronization(args: { parkedTabIds: ReadonlySet activationDeferredMountTabIds?: ReadonlySet | null }): void { - const { worktreeId, terminalTabs, parkedTabIds, activationDeferredMountTabIds } = args - const synchronizationKey = getWatcherSynchronizationKey(args) + const { + worktreeId, + terminalTabs, + inputsKey, + assignmentsKey, + parkedTabIds, + activationDeferredMountTabIds + } = args + const reconciliationStoreInputs = useAppStore( + useShallow((state) => + terminalTabs.flatMap((tab) => [ + state.ptyIdsByTabId[tab.id] ?? EMPTY_PTY_IDS, + state.terminalLayoutsByTabId[tab.id] ?? null, + Object.keys(state.runtimePaneTitlesByTabId[tab.id] ?? {}).join(',') + ]) + ) + ) as WatcherReconciliationStoreInputs + const reconciliationKey = getWatcherReconciliationKey(terminalTabs, reconciliationStoreInputs) + const synchronizationKey = getWatcherSynchronizationKey({ ...args, reconciliationKey }) const synchronizationKeyRef = useRef(null) + useEffect( + () => () => { + synchronizationKeyRef.current = null + disposeParkedTerminalWatchersForWorktree(worktreeId) + }, + [worktreeId] + ) + useEffect(() => { if (synchronizationKeyRef.current === synchronizationKey) { return } - synchronizationKeyRef.current = synchronizationKey syncParkedTerminalTabWatchers({ worktreeId, tabs: terminalTabs, @@ -63,5 +137,22 @@ export function useParkedTerminalWatcherSynchronization(args: { // Why: activation-deferred tabs have no prior pane-owned title slot. restoreTitleOnStartTabIds: activationDeferredMountTabIds ?? undefined }) - }, [activationDeferredMountTabIds, parkedTabIds, synchronizationKey, terminalTabs, worktreeId]) + synchronizationKeyRef.current = getWatcherSynchronizationKey({ + worktreeId, + inputsKey, + assignmentsKey, + parkedTabIds, + activationDeferredMountTabIds, + reconciliationKey: getWatcherReconciliationKey(terminalTabs, reconciliationStoreInputs) + }) + }, [ + activationDeferredMountTabIds, + assignmentsKey, + inputsKey, + parkedTabIds, + reconciliationStoreInputs, + synchronizationKey, + terminalTabs, + worktreeId + ]) } 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 888f82237ee..2e1c97c5450 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,9 @@ import type { TerminalTab } from '../../../../shared/types' const mocks = vi.hoisted(() => ({ storeState: { pendingStartupByTabId: {} as Record, + ptyIdsByTabId: {} as Record, runtimeStatusByEnvironmentId: new Map(), + runtimePaneTitlesByTabId: {} as Record>, settings: {} as Record, terminalLayoutsByTabId: {} as Record }>, sleepingAgentSessionsByPaneKey: {} as Record< 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 98b2680c5a8..f33023c9499 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 @@ -31,10 +31,7 @@ import { selectEvictionExemptTerminalTabLayoutKey } from './terminal-eviction-exempt-tabs' import { selectSleepingRecordParkExemptTabIds } from './sleeping-record-park-exemption' -import { - canWatcherCoverParkedTerminalTab, - disposeParkedTerminalWatchersForWorktree -} from './terminal-parked-tab-watchers' +import { canWatcherCoverParkedTerminalTab } from './terminal-parked-tab-watchers' import { getTerminalParkingAssignmentsKey, getTerminalParkingInputsKey, @@ -379,7 +376,5 @@ export function useTerminalTabColdParking(args: { activationDeferredMountTabIds }) - useEffect(() => () => disposeParkedTerminalWatchersForWorktree(worktreeId), [worktreeId]) - return parkedTerminalTabIds }