diff --git a/src/renderer/src/hooks/agent-hook-completion-notifications.test.ts b/src/renderer/src/hooks/agent-hook-completion-notifications.test.ts index 3fa220ffaff..75487c039b9 100644 --- a/src/renderer/src/hooks/agent-hook-completion-notifications.test.ts +++ b/src/renderer/src/hooks/agent-hook-completion-notifications.test.ts @@ -824,6 +824,37 @@ describe('agent hook completion notifications', () => { expect(_getAgentHookCompletionNotificationCoordinatorCountForTest()).toBe(3) }) + it('gates cosmetic store updates but still prunes after a pane closes', async () => { + const { + _getAgentHookCompletionNotificationCoordinatorCountForTest, + observeAgentHookCompletionForNotification, + syncAgentHookCompletionNotificationsForStoreUpdate + } = await import('./agent-hook-completion-notifications') + + observeAgentHookCompletionForNotification({ + paneKey, + worktreeId: 'wt-1', + payload: hookStatus('working') + }) + + const beforeCosmeticUpdate = { ...mockStoreState } + mockStoreState.tabsByWorktree = { + 'wt-1': [{ id: 'tab-1', ptyId: 'pty-1' }] + } + expect( + syncAgentHookCompletionNotificationsForStoreUpdate(mockStoreState, beforeCosmeticUpdate) + ).toBe(false) + expect(_getAgentHookCompletionNotificationCoordinatorCountForTest()).toBe(1) + + const beforeClose = { ...mockStoreState } + mockStoreState.tabsByWorktree = { 'wt-1': [] } + mockStoreState.ptyIdsByTabId = {} + expect(syncAgentHookCompletionNotificationsForStoreUpdate(mockStoreState, beforeClose)).toBe( + true + ) + expect(_getAgentHookCompletionNotificationCoordinatorCountForTest()).toBe(0) + }) + it('skips tab scans until a pane-liveness slice changes', async () => { seedManyLivePanes() const { diff --git a/src/renderer/src/hooks/agent-hook-completion-notifications.ts b/src/renderer/src/hooks/agent-hook-completion-notifications.ts index 0154d5d0d4a..125c0994514 100644 --- a/src/renderer/src/hooks/agent-hook-completion-notifications.ts +++ b/src/renderer/src/hooks/agent-hook-completion-notifications.ts @@ -10,6 +10,10 @@ import { dispatchTerminalNotification } from '@/components/terminal-pane/use-not import { collectLeafIdsInOrder } from '@/components/terminal-pane/layout-serialization' import { createCodexAutoApprovalHookCompletionSuppressor } from '@/components/terminal-pane/codex-auto-approval-notification-suppression' import { dispatchAgentHookTerminalLifecycle } from '@/components/terminal-pane/agent-hook-terminal-lifecycle' +import { + shouldSyncAgentHookCompletionForStoreUpdate, + type AgentHookCompletionStoreSnapshot +} from './agent-hook-completion-store-sync' type CoordinatorEntry = { worktreeId: string @@ -19,7 +23,8 @@ type CoordinatorEntry = { type StoreSnapshot = ReturnType type WorktreeTab = NonNullable[string][number] // Why: a paneKey resolves to a tab by id. Prebuilding this index once per prune -// pass avoids re-flattening tabsByWorktree per coordinator (O(coordinators x tabs)). +// pass avoids re-flattening tabsByWorktree per coordinator (O(coordinators x +// tabs)) when a liveness or notification-setting update requires a prune. type TabIndex = ReadonlyMap type PaneCoordinatorLivenessSnapshot = Pick< StoreSnapshot, @@ -119,6 +124,19 @@ export function syncAgentHookCompletionNotificationSettings(): boolean { return enabled } +export function syncAgentHookCompletionNotificationsForStoreUpdate( + current: AgentHookCompletionStoreSnapshot, + previous: AgentHookCompletionStoreSnapshot +): boolean { + // Why: Zustand also publishes high-rate title/status writes that cannot make + // module-scoped completion coordinators stale. + if (!shouldSyncAgentHookCompletionForStoreUpdate(current, previous)) { + return false + } + syncAgentHookCompletionNotificationSettings() + return true +} + function getPtyIdForPaneKey(paneKey: string): string | null { const parsed = parsePaneKey(paneKey) if (!parsed) { diff --git a/src/renderer/src/hooks/agent-hook-completion-store-sync.test.ts b/src/renderer/src/hooks/agent-hook-completion-store-sync.test.ts new file mode 100644 index 00000000000..de1f22eca11 --- /dev/null +++ b/src/renderer/src/hooks/agent-hook-completion-store-sync.test.ts @@ -0,0 +1,190 @@ +import { describe, expect, it } from 'vitest' +import { + _measureAgentHookCompletionStoreSyncForTest, + shouldSyncAgentHookCompletionForStoreUpdate, + type AgentHookCompletionStoreSnapshot +} from './agent-hook-completion-store-sync' + +function createState( + overrides: Partial = {} +): AgentHookCompletionStoreSnapshot { + return { + settings: { + experimentalTerminalAttention: false, + notifications: { enabled: true, agentTaskComplete: true } + }, + tabsByWorktree: { + 'wt-1': [{ id: 'tab-1', ptyId: 'pty-1', title: 'Terminal 1' }] + }, + ptyIdsByTabId: { 'tab-1': ['pty-1'] }, + terminalLayoutsByTabId: {}, + suppressedPtyExitIds: {}, + ...overrides + } +} + +describe('agent hook completion store sync', () => { + it('ignores unrelated and title-only writes at accumulated-workspace scale', () => { + const tabCount = 100 + const coordinatorCount = 100 + const updateCount = 600 + const tabsByWorktree = Object.fromEntries( + Array.from({ length: tabCount }, (_, index) => [ + `wt-${index}`, + [{ id: `tab-${index}`, ptyId: `pty-${index}`, title: `Terminal ${index}` }] + ]) + ) + let previous = createState({ tabsByWorktree }) + let syncPasses = 0 + let tabVisits = 0 + + for (let index = 0; index < updateCount; index += 1) { + const current = { ...previous } + const measurement = _measureAgentHookCompletionStoreSyncForTest(current, previous) + syncPasses += Number(measurement.shouldSync) + tabVisits += measurement.tabVisits + previous = current + } + + for (let index = 0; index < updateCount; index += 1) { + const targetTabs = previous.tabsByWorktree['wt-99'] + if (!targetTabs) { + throw new Error('Expected title-update fixture tab') + } + const current = createState({ + ...previous, + tabsByWorktree: { + ...previous.tabsByWorktree, + 'wt-99': targetTabs.map((tab) => ({ ...tab, title: `Agent frame ${index}` })) + } + }) + const measurement = _measureAgentHookCompletionStoreSyncForTest(current, previous) + syncPasses += Number(measurement.shouldSync) + tabVisits += measurement.tabVisits + previous = current + } + + const previousFullPassCost = { + tabVisits: updateCount * 2 * tabCount, + coordinatorVisits: updateCount * 2 * coordinatorCount + } + const gatedCost = { + tabVisits, + coordinatorVisits: syncPasses * coordinatorCount + } + expect(previousFullPassCost).toEqual({ tabVisits: 120_000, coordinatorVisits: 120_000 }) + expect(gatedCost).toEqual({ tabVisits: 600, coordinatorVisits: 0 }) + }) + + it('keeps every coordinator-liveness input reactive', () => { + const previous = createState() + const titleOnly = createState({ + ...previous, + tabsByWorktree: { + 'wt-1': [{ id: 'tab-1', ptyId: 'pty-1', title: 'Codex working' }] + } + }) + expect(shouldSyncAgentHookCompletionForStoreUpdate(titleOnly, previous)).toBe(false) + + const tabCreated = createState({ + ...previous, + tabsByWorktree: { + ...previous.tabsByWorktree, + 'wt-2': [{ id: 'tab-2', ptyId: null }] + } + }) + expect(shouldSyncAgentHookCompletionForStoreUpdate(tabCreated, previous)).toBe(true) + + const tabRemoved = createState({ ...previous, tabsByWorktree: {} }) + expect(shouldSyncAgentHookCompletionForStoreUpdate(tabRemoved, previous)).toBe(true) + + const tabPtyChanged = createState({ + ...previous, + tabsByWorktree: { 'wt-1': [{ id: 'tab-1', ptyId: 'pty-2' }] } + }) + expect(shouldSyncAgentHookCompletionForStoreUpdate(tabPtyChanged, previous)).toBe(true) + + expect( + shouldSyncAgentHookCompletionForStoreUpdate( + createState({ ...previous, ptyIdsByTabId: { 'tab-1': ['pty-2'] } }), + previous + ) + ).toBe(true) + expect( + shouldSyncAgentHookCompletionForStoreUpdate( + createState({ ...previous, terminalLayoutsByTabId: { 'tab-1': { root: null } } }), + previous + ) + ).toBe(true) + expect( + shouldSyncAgentHookCompletionForStoreUpdate( + createState({ ...previous, suppressedPtyExitIds: { 'pty-1': true } }), + previous + ) + ).toBe(true) + + const trackingDisabled = createState({ + ...previous, + settings: { + experimentalTerminalAttention: false, + notifications: { enabled: false, agentTaskComplete: false } + } + }) + expect(shouldSyncAgentHookCompletionForStoreUpdate(trackingDisabled, previous)).toBe(true) + }) + + it('treats tab order and duplicate-id worktree precedence as liveness inputs', () => { + const previous = createState({ + tabsByWorktree: { + 'wt-1': [{ id: 'tab-shared', ptyId: 'pty-first' }], + 'wt-2': [{ id: 'tab-shared', ptyId: 'pty-second' }] + } + }) + const reorderedWorktrees = createState({ + ...previous, + tabsByWorktree: { + 'wt-2': previous.tabsByWorktree['wt-2'] ?? [], + 'wt-1': previous.tabsByWorktree['wt-1'] ?? [] + } + }) + expect(shouldSyncAgentHookCompletionForStoreUpdate(reorderedWorktrees, previous)).toBe(true) + + const twoTabPrevious = createState({ + tabsByWorktree: { + 'wt-1': [ + { id: 'tab-1', ptyId: 'pty-1' }, + { id: 'tab-2', ptyId: 'pty-2' } + ] + } + }) + const reorderedTabs = createState({ + ...twoTabPrevious, + tabsByWorktree: { + 'wt-1': [ + { id: 'tab-2', ptyId: 'pty-2' }, + { id: 'tab-1', ptyId: 'pty-1' } + ] + } + }) + expect(shouldSyncAgentHookCompletionForStoreUpdate(reorderedTabs, twoTabPrevious)).toBe(true) + }) + + it('compares effective tracking state instead of unrelated settings identity', () => { + const previous = createState({ + settings: { + experimentalTerminalAttention: true, + notifications: { enabled: false, agentTaskComplete: false } + } + }) + const stillTrackedByNotifications = createState({ + ...previous, + settings: { + experimentalTerminalAttention: false, + notifications: { enabled: true, agentTaskComplete: true } + } + }) + expect(shouldSyncAgentHookCompletionForStoreUpdate(stillTrackedByNotifications, previous)).toBe( + false + ) + }) +}) diff --git a/src/renderer/src/hooks/agent-hook-completion-store-sync.ts b/src/renderer/src/hooks/agent-hook-completion-store-sync.ts new file mode 100644 index 00000000000..e900c5b0c5d --- /dev/null +++ b/src/renderer/src/hooks/agent-hook-completion-store-sync.ts @@ -0,0 +1,112 @@ +type CompletionNotificationSettings = { + readonly enabled?: boolean + readonly agentTaskComplete?: boolean +} + +type CompletionStoreSettings = { + readonly notifications?: CompletionNotificationSettings + readonly experimentalTerminalAttention?: boolean +} + +type CompletionTerminalTab = { + readonly id: string + readonly ptyId?: string | null + readonly title?: string +} + +export type AgentHookCompletionStoreSnapshot = { + readonly settings: CompletionStoreSettings | null + readonly tabsByWorktree: Readonly> + readonly ptyIdsByTabId: Readonly> + readonly terminalLayoutsByTabId: Readonly> + readonly suppressedPtyExitIds: Readonly> +} + +type TabVisit = () => void + +function isTrackingEnabled(state: AgentHookCompletionStoreSnapshot): boolean { + const notifications = state.settings?.notifications + const notificationEnabled = + notifications?.enabled !== false && notifications?.agentTaskComplete !== false + return notificationEnabled || state.settings?.experimentalTerminalAttention === true +} + +function terminalTabLivenessMatches( + current: AgentHookCompletionStoreSnapshot['tabsByWorktree'], + previous: AgentHookCompletionStoreSnapshot['tabsByWorktree'], + visitTab?: TabVisit +): boolean { + if (current === previous) { + return true + } + + const currentWorktreeIds = Object.keys(current) + const previousWorktreeIds = Object.keys(previous) + if (currentWorktreeIds.length !== previousWorktreeIds.length) { + return false + } + + for (const [worktreeIndex, worktreeId] of currentWorktreeIds.entries()) { + // Why: duplicate tab ids use first-worktree-wins lookup semantics, so a + // worktree-key reorder is a liveness change even when every array is reused. + if (previousWorktreeIds[worktreeIndex] !== worktreeId) { + return false + } + const currentTabs = current[worktreeId] + const previousTabs = previous[worktreeId] + if (currentTabs === previousTabs) { + continue + } + if (!currentTabs || !previousTabs || currentTabs.length !== previousTabs.length) { + return false + } + for (const [tabIndex, currentTab] of currentTabs.entries()) { + visitTab?.() + const previousTab = previousTabs[tabIndex] + if ( + !previousTab || + currentTab.id !== previousTab.id || + currentTab.ptyId !== previousTab.ptyId + ) { + return false + } + } + } + return true +} + +function shouldSync( + current: AgentHookCompletionStoreSnapshot, + previous: AgentHookCompletionStoreSnapshot, + visitTab?: TabVisit +): boolean { + if (isTrackingEnabled(current) !== isTrackingEnabled(previous)) { + return true + } + if ( + current.ptyIdsByTabId !== previous.ptyIdsByTabId || + current.terminalLayoutsByTabId !== previous.terminalLayoutsByTabId || + current.suppressedPtyExitIds !== previous.suppressedPtyExitIds + ) { + return true + } + return !terminalTabLivenessMatches(current.tabsByWorktree, previous.tabsByWorktree, visitTab) +} + +export function shouldSyncAgentHookCompletionForStoreUpdate( + current: AgentHookCompletionStoreSnapshot, + previous: AgentHookCompletionStoreSnapshot +): boolean { + return shouldSync(current, previous) +} + +export function _measureAgentHookCompletionStoreSyncForTest( + current: AgentHookCompletionStoreSnapshot, + previous: AgentHookCompletionStoreSnapshot +): { shouldSync: boolean; tabVisits: number } { + let tabVisits = 0 + const requiresSync = shouldSync(current, previous, () => { + tabVisits += 1 + }) + return { shouldSync: requiresSync, tabVisits } +} diff --git a/src/renderer/src/hooks/useIpcEvents.test.ts b/src/renderer/src/hooks/useIpcEvents.test.ts index 7abf34a6d3e..28d2aa993dd 100644 --- a/src/renderer/src/hooks/useIpcEvents.test.ts +++ b/src/renderer/src/hooks/useIpcEvents.test.ts @@ -4299,7 +4299,7 @@ describe('useIpcEvents agent status snapshot integration', () => { stateStartedAt: number } type StoreLike = Record - type StoreSubscribeListener = (state: StoreLike) => void + type StoreSubscribeListener = (state: StoreLike, previousState: StoreLike) => void type MobileFitEvent = { ptyId: string mode: 'mobile-fit' | 'desktop-fit' @@ -4799,6 +4799,7 @@ describe('useIpcEvents agent status snapshot integration', () => { expect(setAgentStatus).not.toHaveBeenCalled() expect(getSnapshot).not.toHaveBeenCalled() + const previousStoreState = { ...storeState } storeState.workspaceSessionReady = true storeState.tabsByWorktree = { 'wt-1': [{ id: 'tab-future', ptyId: 'pty-1', worktreeId: 'wt-1', title: 'Future Tab' }] @@ -4813,7 +4814,7 @@ describe('useIpcEvents agent status snapshot integration', () => { if (typeof subscribeListenerRef.current !== 'function') { throw new Error('Expected useAppStore.subscribe listener to be registered') } - subscribeListenerRef.current(storeState) + subscribeListenerRef.current(storeState, previousStoreState) await Promise.resolve() expect(setAgentStatus).toHaveBeenCalledTimes(1) @@ -4937,7 +4938,7 @@ describe('useIpcEvents agent status snapshot integration', () => { vi.doMock('./agent-hook-completion-notifications', () => ({ observeAgentHookCompletionForNotification, resetAgentHookCompletionNotificationCoordinators: vi.fn(), - syncAgentHookCompletionNotificationSettings: vi.fn() + syncAgentHookCompletionNotificationsForStoreUpdate: vi.fn() })) stubAuxiliaryModules() vi.stubGlobal( @@ -5014,7 +5015,7 @@ describe('useIpcEvents agent status snapshot integration', () => { vi.doMock('./agent-hook-completion-notifications', () => ({ observeAgentHookCompletionForNotification, resetAgentHookCompletionNotificationCoordinators: vi.fn(), - syncAgentHookCompletionNotificationSettings: vi.fn() + syncAgentHookCompletionNotificationsForStoreUpdate: vi.fn() })) stubAuxiliaryModules() vi.stubGlobal( @@ -5169,7 +5170,7 @@ describe('useIpcEvents agent status snapshot integration', () => { vi.doMock('./agent-hook-completion-notifications', () => ({ observeAgentHookCompletionForNotification, resetAgentHookCompletionNotificationCoordinators: vi.fn(), - syncAgentHookCompletionNotificationSettings: vi.fn() + syncAgentHookCompletionNotificationsForStoreUpdate: vi.fn() })) stubAuxiliaryModules() vi.stubGlobal( @@ -5978,6 +5979,7 @@ describe('useIpcEvents agent status snapshot integration', () => { reason: 'unknown_tab_id' }) + const previousStoreState = { ...storeState } storeState.terminalLayoutsByTabId = { 'tab-future': { root: { type: 'leaf', leafId: FUTURE_LEAF_ID }, @@ -5988,7 +5990,7 @@ describe('useIpcEvents agent status snapshot integration', () => { if (typeof subscribeListenerRef.current !== 'function') { throw new Error('Expected useAppStore.subscribe listener to be registered') } - subscribeListenerRef.current(storeState) + subscribeListenerRef.current(storeState, previousStoreState) expect(setAgentStatus).toHaveBeenCalledTimes(2) expect(setAgentStatus).toHaveBeenNthCalledWith( diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index 219f8dca783..2eb7f545d2b 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -116,7 +116,7 @@ import { import { observeAgentHookCompletionForNotification, resetAgentHookCompletionNotificationCoordinators, - syncAgentHookCompletionNotificationSettings + syncAgentHookCompletionNotificationsForStoreUpdate } from './agent-hook-completion-notifications' import { shouldSuppressCodexAutoApprovalStatus } from '@/components/terminal-pane/codex-auto-approval-notification-suppression' import { showTerminalShortcutCaptureNotification } from '@/lib/terminal-shortcut-capture-notification' @@ -3212,10 +3212,10 @@ export function useIpcEvents(): void { // renderer state. requestAgentStatusSnapshotIfReady() unsubs.push( - useAppStore.subscribe(() => { + useAppStore.subscribe((state, previousState) => { requestAgentStatusSnapshotIfReady() flushPendingAgentStatuses() - syncAgentHookCompletionNotificationSettings() + syncAgentHookCompletionNotificationsForStoreUpdate(state, previousState) }) )