mirror of
https://github.com/stablyai/orca.git
synced 2026-09-24 16:02:41 +00:00
perf(renderer): gate pending agent-status retries (#17254)
This commit is contained in:
@@ -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)
|
||||
})
|
||||
|
||||
|
||||
@@ -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<typeof shouldRetryPendingAgentStatusesAfterStoreUpdate>[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<typeof vi.fn>
|
||||
transactAgentStatuses: ReturnType<typeof vi.fn>
|
||||
}
|
||||
|
||||
async function createPendingRetryHarness(): Promise<PendingRetryHarness> {
|
||||
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()
|
||||
})
|
||||
})
|
||||
@@ -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
|
||||
)
|
||||
}
|
||||
@@ -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))
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user