diff --git a/src/renderer/src/hooks/ipc-events/agent-status-ipc-bridge.ts b/src/renderer/src/hooks/ipc-events/agent-status-ipc-bridge.ts index 3a99527b1d3..26058a3e9e0 100644 --- a/src/renderer/src/hooks/ipc-events/agent-status-ipc-bridge.ts +++ b/src/renderer/src/hooks/ipc-events/agent-status-ipc-bridge.ts @@ -13,6 +13,7 @@ import type { AgentStatusBatchEvent, PendingAgentStatusEvent } from './agent-status-bridge-types' +import { shouldRetryPendingAgentStatusesAfterStoreUpdate } from './agent-status-pending-retry-gate' const PENDING_AGENT_STATUS_RETRY_MS = 100 const PENDING_AGENT_STATUS_TTL_MS = 15_000 @@ -94,8 +95,8 @@ export function registerAgentStatusIpcBridge(unsubs: (() => void)[]): AgentStatu } } finally { isFlushingAgentStatuses = false + schedulePendingAgentStatusFlush() } - schedulePendingAgentStatusFlush() } const applyAgentStatus = createAgentStatusEventApplicator({ @@ -261,7 +262,13 @@ export function registerAgentStatusIpcBridge(unsubs: (() => void)[]): AgentStatu requestAgentStatusSnapshotIfReady() const unsubscribeAgentStatusStore = useAppStore.subscribe((state, previousState) => { requestAgentStatusSnapshotIfReady() - flushPendingAgentStatuses() + // Why: the timer covers module-owned rekeys; unrelated store writes cannot change attribution and must not rebuild its routing index. + if ( + pendingAgentStatusEvents.length > 0 && + shouldRetryPendingAgentStatusesAfterStoreUpdate(state, previousState) + ) { + flushPendingAgentStatuses() + } syncAgentHookCompletionNotificationsForStoreUpdate(state, previousState) }) diff --git a/src/renderer/src/hooks/ipc-events/agent-status-pending-retry-gate.test.ts b/src/renderer/src/hooks/ipc-events/agent-status-pending-retry-gate.test.ts new file mode 100644 index 00000000000..51e8161a8fc --- /dev/null +++ b/src/renderer/src/hooks/ipc-events/agent-status-pending-retry-gate.test.ts @@ -0,0 +1,191 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { AgentStatusBatchUpdate } from '../../store/slices/agent-status' +import { + buildStoreState, + FUTURE_LEAF_ID, + FUTURE_PANE_KEY, + type AgentStatusSetData, + type StoreLike, + type StoreSubscribeListener +} from '../ipc-events-agent-status-store-test-fixtures' +import { shouldRetryPendingAgentStatusesAfterStoreUpdate } from './agent-status-pending-retry-gate' + +type RetryState = Parameters[0] + +function createRetryState(): RetryState { + return { + workspaceSessionReady: true, + tabsByWorktree: {}, + unifiedTabsByWorktree: {}, + terminalLayoutsByTabId: {}, + worktreesByRepo: {}, + repos: [], + recentlyClosedAgentStatusTabIds: {}, + recentlyRetiredAgentStatusPaneKeys: {} + } +} + +type PendingRetryHarness = { + bridge: { disposeAsyncState: () => void; unsubscribeStore: () => void } + emitPendingStatus: () => void + publish: (mutate: (state: StoreLike) => void) => void + setAgentStatuses: ReturnType + transactAgentStatuses: ReturnType +} + +async function createPendingRetryHarness(): Promise { + const subscribeListenerRef: { current: StoreSubscribeListener | null } = { current: null } + const onSetListenerRef: { current: ((data: AgentStatusSetData) => void) | null } = { + current: null + } + const setAgentStatuses = vi.fn((updates: readonly AgentStatusBatchUpdate[]) => + updates.map(() => true) + ) + const storeState = buildStoreState({ + setAgentStatuses, + workspaceSessionReady: true, + tabsByWorktree: {}, + terminalLayoutsByTabId: {}, + settings: { terminalFontSize: 13, notifications: { enabled: false } } + }) + const transactImplementation = storeState.transactAgentStatuses as (...args: unknown[]) => unknown + const transactAgentStatuses = vi.fn(transactImplementation) + storeState.transactAgentStatuses = transactAgentStatuses + + vi.doMock('../../store', () => ({ + useAppStore: { + subscribe: vi.fn((listener: StoreSubscribeListener) => { + subscribeListenerRef.current = listener + return () => { + subscribeListenerRef.current = null + } + }), + getState: () => storeState + } + })) + vi.doMock('../agent-hook-completion-notifications', () => ({ + observeAgentHookCompletionForNotification: vi.fn(), + syncAgentHookCompletionNotificationsForStoreUpdate: vi.fn() + })) + vi.stubGlobal('window', { + api: { + agentStatus: { + onSet: (listener: (data: AgentStatusSetData) => void) => { + onSetListenerRef.current = listener + return () => { + onSetListenerRef.current = null + } + } + } + } + }) + + const { registerAgentStatusIpcBridge } = await import('./agent-status-ipc-bridge') + const bridge = registerAgentStatusIpcBridge([]) + if (!subscribeListenerRef.current || !onSetListenerRef.current) { + throw new Error('Expected agent-status bridge listeners') + } + + return { + bridge, + emitPendingStatus: () => { + onSetListenerRef.current?.({ + paneKey: FUTURE_PANE_KEY, + state: 'working', + prompt: 'pending attribution', + agentType: 'claude', + receivedAt: 1_700_000_000_000, + stateStartedAt: 1_700_000_000_000 + }) + }, + publish: (mutate) => { + const previousState = { ...storeState } + mutate(storeState) + subscribeListenerRef.current?.(storeState, previousState) + }, + setAgentStatuses, + transactAgentStatuses + } +} + +describe('agent status pending retry gate', () => { + beforeEach(() => { + vi.resetModules() + vi.useFakeTimers() + vi.setSystemTime(1_700_000_100_000) + }) + + afterEach(() => { + vi.useRealTimers() + vi.unstubAllGlobals() + }) + + it('tracks every store-owned input that can resolve or retire pending attribution', () => { + const previous = createRetryState() + const changes: RetryState[] = [ + { ...previous, workspaceSessionReady: false }, + { ...previous, tabsByWorktree: {} }, + { ...previous, unifiedTabsByWorktree: {} }, + { ...previous, terminalLayoutsByTabId: {} }, + { ...previous, worktreesByRepo: {} }, + { ...previous, repos: [] }, + { ...previous, recentlyClosedAgentStatusTabIds: {} }, + { ...previous, recentlyRetiredAgentStatusPaneKeys: {} } + ] + + expect(shouldRetryPendingAgentStatusesAfterStoreUpdate({ ...previous }, previous)).toBe(false) + for (const current of changes) { + expect(shouldRetryPendingAgentStatusesAfterStoreUpdate(current, previous)).toBe(true) + } + }) + + it('skips retry transactions for unrelated publications and retries on hydration', async () => { + const harness = await createPendingRetryHarness() + harness.emitPendingStatus() + + for (let updateIndex = 0; updateIndex < 2_400; updateIndex += 1) { + harness.publish((state) => { + state.cosmeticUpdateIndex = updateIndex + }) + } + + expect(harness.transactAgentStatuses).not.toHaveBeenCalled() + + harness.publish((state) => { + state.tabsByWorktree = { + 'wt-1': [{ id: 'tab-future', ptyId: 'pty-1', worktreeId: 'wt-1', title: 'Future' }] + } + state.terminalLayoutsByTabId = { + 'tab-future': { + root: { type: 'leaf', leafId: FUTURE_LEAF_ID }, + activeLeafId: FUTURE_LEAF_ID, + expandedLeafId: null + } + } + }) + + expect(harness.transactAgentStatuses).toHaveBeenCalledTimes(1) + expect(harness.setAgentStatuses).toHaveBeenCalledWith([ + expect.objectContaining({ paneKey: FUTURE_PANE_KEY }) + ]) + vi.advanceTimersByTime(100) + expect(harness.transactAgentStatuses).toHaveBeenCalledTimes(1) + harness.bridge.unsubscribeStore() + harness.bridge.disposeAsyncState() + }) + + it('keeps the timer fallback when routing references stay unchanged', async () => { + const harness = await createPendingRetryHarness() + harness.emitPendingStatus() + harness.publish((state) => { + state.cosmeticUpdateIndex = 1 + }) + + expect(harness.transactAgentStatuses).not.toHaveBeenCalled() + vi.advanceTimersByTime(100) + expect(harness.transactAgentStatuses).toHaveBeenCalledTimes(1) + expect(harness.setAgentStatuses).toHaveBeenCalledWith([]) + harness.bridge.unsubscribeStore() + harness.bridge.disposeAsyncState() + }) +}) diff --git a/src/renderer/src/hooks/ipc-events/agent-status-pending-retry-gate.ts b/src/renderer/src/hooks/ipc-events/agent-status-pending-retry-gate.ts new file mode 100644 index 00000000000..4b8dcc36a9e --- /dev/null +++ b/src/renderer/src/hooks/ipc-events/agent-status-pending-retry-gate.ts @@ -0,0 +1,29 @@ +import type { AppState } from '../../store/types' + +type AgentStatusPendingRetryState = Pick< + AppState, + | 'workspaceSessionReady' + | 'tabsByWorktree' + | 'unifiedTabsByWorktree' + | 'terminalLayoutsByTabId' + | 'worktreesByRepo' + | 'repos' + | 'recentlyClosedAgentStatusTabIds' + | 'recentlyRetiredAgentStatusPaneKeys' +> + +export function shouldRetryPendingAgentStatusesAfterStoreUpdate( + current: AgentStatusPendingRetryState, + previous: AgentStatusPendingRetryState +): boolean { + return ( + current.workspaceSessionReady !== previous.workspaceSessionReady || + current.tabsByWorktree !== previous.tabsByWorktree || + current.unifiedTabsByWorktree !== previous.unifiedTabsByWorktree || + current.terminalLayoutsByTabId !== previous.terminalLayoutsByTabId || + current.worktreesByRepo !== previous.worktreesByRepo || + current.repos !== previous.repos || + current.recentlyClosedAgentStatusTabIds !== previous.recentlyClosedAgentStatusTabIds || + current.recentlyRetiredAgentStatusPaneKeys !== previous.recentlyRetiredAgentStatusPaneKeys + ) +} diff --git a/src/renderer/src/hooks/useIpcEvents-agent-status-queue-ordering.test.ts b/src/renderer/src/hooks/useIpcEvents-agent-status-queue-ordering.test.ts index 322e253e541..eb613f21e70 100644 --- a/src/renderer/src/hooks/useIpcEvents-agent-status-queue-ordering.test.ts +++ b/src/renderer/src/hooks/useIpcEvents-agent-status-queue-ordering.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { AgentStatusUpdate } from '../store/slices/agent-status' import { YOLO_TUI_AGENT_ARGS } from '../../../shared/tui-agent-permissions' import { @@ -22,6 +22,10 @@ describe('useIpcEvents agent status snapshot integration', () => { vi.unstubAllGlobals() }) + afterEach(() => { + vi.useRealTimers() + }) + it('preserves queued set-clear order for working removal and done retention', async () => { vi.useFakeTimers() let storeState: StoreLike @@ -163,7 +167,7 @@ describe('useIpcEvents agent status snapshot integration', () => { it('does not recurse when flushing a pending status re-enters via the store subscriber', async () => { // Repro for crash 9fc89529 (RangeError: Maximum call stack size exceeded): - // the store subscriber calls flushPendingAgentStatuses() on every update. + // the store subscriber retries pending statuses synchronously on hydration. // flush -> applyAgentStatus -> store.setAgentStatus notifies subscribers // synchronously (like Zustand) -> subscriber -> flush again while the same // event is still queued -> infinite recursion. Model setAgentStatus with a @@ -173,10 +177,10 @@ describe('useIpcEvents agent status snapshot integration', () => { current: null } let setAgentStatusCalls = 0 - const notify = (): void => { + const notify = (previousState: StoreLike = storeState): void => { const listener = subscribeListenerRef.current if (listener) { - listener(storeState, storeState) + listener(storeState, previousState) } } const storeState: StoreLike = buildStoreState({ @@ -243,6 +247,7 @@ describe('useIpcEvents agent status snapshot integration', () => { // Tab hydrates; the next store update flushes the pending event. Without the // re-entrancy guard this overflows the stack instead of applying once. + const beforeHydration = { ...storeState } storeState.tabsByWorktree = { 'wt-1': [{ id: 'tab-future', ptyId: 'pty-1', worktreeId: 'wt-1', title: 'Future Tab' }] } @@ -254,7 +259,7 @@ describe('useIpcEvents agent status snapshot integration', () => { } } - expect(() => notify()).not.toThrow() + expect(() => notify(beforeHydration)).not.toThrow() // Applied exactly once — the re-entrant flush is a no-op, not a loop. expect(setAgentStatusCalls).toBe(1) }) @@ -263,6 +268,7 @@ describe('useIpcEvents agent status snapshot integration', () => { // buffered event permanently. Before batching the queue was only replaced after the loop, // so a throw left it intact — keep that. it('keeps pending statuses queued when the retry fold throws', async () => { + vi.useFakeTimers() const subscribeListenerRef: { current: StoreSubscribeListener | null } = { current: null } const onSetListenerRef: { current: ((data: AgentStatusSetData) => void) | null } = { current: null @@ -284,8 +290,6 @@ describe('useIpcEvents agent status snapshot integration', () => { tabsByWorktree: {}, terminalLayoutsByTabId: {} }) - const notify = (): void => subscribeListenerRef.current?.(storeState, storeState) - stubReactSyncEffect() vi.doMock('../store', () => ({ useAppStore: { @@ -325,7 +329,10 @@ describe('useIpcEvents agent status snapshot integration', () => { stateStartedAt: 1_700_000_000_100 }) - // Tab hydrates, so the next store update flushes the pending event — and throws. + // The timer-owned retry throws after clearing its handle. + expect(() => vi.advanceTimersByTime(100)).toThrow('fold blew up') + + // Hydrate without publishing so only the re-armed timer can recover the event. storeState.tabsByWorktree = { 'wt-1': [{ id: 'tab-future', ptyId: 'pty-1', worktreeId: 'wt-1', title: 'Future Tab' }] } @@ -336,10 +343,8 @@ describe('useIpcEvents agent status snapshot integration', () => { expandedLeafId: null } } - expect(() => notify()).toThrow('fold blew up') - // The event must still be queued, so the next flush replays it. - notify() + vi.advanceTimersByTime(100) const replayedAfterThrow = setAgentStatuses.mock.calls .slice(1) .flatMap((call) => call[0].map((update) => update.payload.prompt)) diff --git a/src/renderer/src/hooks/useIpcEvents-agent-status-snapshot-replay.test.ts b/src/renderer/src/hooks/useIpcEvents-agent-status-snapshot-replay.test.ts index cbb4992e7f2..e26448f0b63 100644 --- a/src/renderer/src/hooks/useIpcEvents-agent-status-snapshot-replay.test.ts +++ b/src/renderer/src/hooks/useIpcEvents-agent-status-snapshot-replay.test.ts @@ -87,6 +87,7 @@ describe('useIpcEvents agent status snapshot integration', () => { expect(setAgentStatus).not.toHaveBeenCalled() + const beforeHydration = { ...storeState } Object.assign(storeState, { tabsByWorktree: { 'wt-1': [{ id: 'tab-future', ptyId: 'pty-1', worktreeId: 'wt-1', title: 'SSH Tab' }] @@ -99,7 +100,7 @@ describe('useIpcEvents agent status snapshot integration', () => { } } }) - subscribeListenerRef.current?.(storeState, storeState) + subscribeListenerRef.current?.(storeState, beforeHydration) expect(setAgentStatus).toHaveBeenCalledTimes(1) expect(setAgentStatus).toHaveBeenCalledWith( @@ -192,6 +193,7 @@ describe('useIpcEvents agent status snapshot integration', () => { expect(setAgentStatus).not.toHaveBeenCalled() + const beforeHydration = { ...storeState } Object.assign(storeState, { tabsByWorktree: { 'wt-1': [{ id: 'tab-future', ptyId: 'pty-1', worktreeId: 'wt-1', title: 'SSH Tab' }] @@ -204,7 +206,7 @@ describe('useIpcEvents agent status snapshot integration', () => { } } }) - subscribeListenerRef.current?.(storeState, storeState) + subscribeListenerRef.current?.(storeState, beforeHydration) expect(setAgentStatus).toHaveBeenCalledWith( FUTURE_PANE_KEY,