mirror of
https://github.com/stablyai/orca.git
synced 2026-10-17 00:03:08 +00:00
fix(store): purge pane-scoped agent/unread state on external worktree removal
buildWorktreePurgeState is the bulk worktree-removal reducer reached by the
authoritative-scan reconcile (CLI `git worktree remove`, another Orca window,
SSH), remove-project, and the hydration stale-purge. Unlike the single in-app
removeWorktree path — which runs shutdownWorktreeTerminals /
dropAgentStatusByWorktree / clearPaneForegroundAgentByWorktree — it never ran
terminal teardown, so it left every pane-scoped map untouched:
agentStatusByPaneKey, agentLaunchConfigByPaneKey, acknowledgedAgentsByPaneKey,
paneForegroundAgentByPaneKey, sleepingAgentSessionsByPaneKey,
unreadTerminalPanes, unreadAgentCompletionPanes, lastTerminalInputAtByPaneKey
(all keyed by `${tabId}:${leafId}`) and unreadTerminalTabs (keyed by tabId).
Result: one orphaned entry per agent pane of every externally-removed worktree,
retained for the whole renderer session (monotonic growth), plus a phantom +1
on the unread dock badge. This is a direct parallel to leaks the maintainers
already fixed in this same function (browserAnnotationsByPageId, editorCursorLine
— each carrying a 'the bulk reconcile path missed them' comment).
Fix: evict these maps by tab-id / `${tabId}:` prefix in the reducer, matching
the in-app teardown. retainedAgentsByPaneKey and runtimeAgentOrchestrationByPaneKey
are intentionally left out — both self-heal (pruneRetainedAgents on a
worktreesByRepo change; the runtime map is replaced wholesale each sync).
Leak-regression test drives purgeWorktreeTerminalState and asserts every map is
emptied for the removed worktree while a sibling worktree's state is preserved.
Reverting the fix fails both cases.
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -0,0 +1,139 @@
|
||||
/**
|
||||
* Memory-leak regression: pane-scoped agent-status, unread, and terminal-input
|
||||
* maps must be purged when a worktree is removed via the BULK path.
|
||||
*
|
||||
* These maps are keyed by `${tabId}:${leafId}` (or by tabId for
|
||||
* `unreadTerminalTabs`). The single `removeWorktree` path clears them via
|
||||
* `shutdownWorktreeTerminals` / `dropAgentStatusByWorktree` /
|
||||
* `clearPaneForegroundAgentByWorktree`, but the bulk `purgeWorktreeTerminalState`
|
||||
* reducer (used by the authoritative-scan reconcile, remove-project, and the
|
||||
* hydration stale-purge) never ran terminal teardown — so an externally-removed
|
||||
* worktree (CLI `git worktree remove`, another Orca window, SSH, or a removed
|
||||
* project) orphaned one entry per agent pane for the lifetime of the renderer
|
||||
* session, plus a phantom unread dock badge.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
import type { AgentStatusEntry } from '../../../../shared/agent-status-types'
|
||||
import type { AppState } from '../types'
|
||||
import type * as AgentStatusModule from '@/lib/agent-status'
|
||||
|
||||
vi.mock('sonner', () => ({
|
||||
toast: { info: vi.fn(), success: vi.fn(), error: vi.fn(), warning: vi.fn() }
|
||||
}))
|
||||
|
||||
vi.mock('@/components/terminal-pane/pty-dispatcher', () => ({
|
||||
restorePtyDataHandlersAfterFailedShutdown: vi.fn(),
|
||||
unregisterPtyDataHandlers: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/agent-status', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof AgentStatusModule>()
|
||||
return { ...actual, detectAgentStatusFromTitle: vi.fn().mockReturnValue(null) }
|
||||
})
|
||||
|
||||
const mockApi = {
|
||||
worktrees: {
|
||||
list: vi.fn().mockResolvedValue([]),
|
||||
remove: vi.fn().mockResolvedValue(undefined),
|
||||
forceDeletePreservedBranch: vi.fn().mockResolvedValue({ deleted: true }),
|
||||
updateMeta: vi.fn().mockResolvedValue({})
|
||||
},
|
||||
pty: { kill: vi.fn().mockResolvedValue(undefined) },
|
||||
runtimeEnvironments: { call: vi.fn().mockResolvedValue({ ok: true, result: {} }) }
|
||||
}
|
||||
|
||||
// @ts-expect-error -- minimal window.api stub for the store under test
|
||||
globalThis.window = { api: mockApi }
|
||||
|
||||
import { createTestStore, seedStore, makeWorktree, makeTab } from './store-test-helpers'
|
||||
|
||||
const WT = 'repo1::/path/wt1'
|
||||
const TAB = 'tab-1'
|
||||
const PANE = `${TAB}:leaf-a`
|
||||
|
||||
function liveEntry(paneKey: string): AgentStatusEntry {
|
||||
return {
|
||||
state: 'working',
|
||||
prompt: 'p',
|
||||
updatedAt: 0,
|
||||
stateStartedAt: 0,
|
||||
paneKey,
|
||||
stateHistory: []
|
||||
}
|
||||
}
|
||||
|
||||
function seedPaneState(store: ReturnType<typeof createTestStore>): void {
|
||||
seedStore(store, {
|
||||
worktreesByRepo: {
|
||||
repo1: [makeWorktree({ id: WT, repoId: 'repo1', path: '/path/wt1' })]
|
||||
},
|
||||
tabsByWorktree: { [WT]: [makeTab({ id: TAB, worktreeId: WT })] },
|
||||
agentStatusByPaneKey: { [PANE]: liveEntry(PANE) },
|
||||
agentLaunchConfigByPaneKey: {
|
||||
[PANE]: { launchConfig: {}, registeredAt: 0, identity: {} }
|
||||
} as unknown as AppState['agentLaunchConfigByPaneKey'],
|
||||
acknowledgedAgentsByPaneKey: { [PANE]: 1 },
|
||||
paneForegroundAgentByPaneKey: { [PANE]: { agent: null, shellForeground: true } },
|
||||
sleepingAgentSessionsByPaneKey: {
|
||||
[PANE]: {}
|
||||
} as unknown as AppState['sleepingAgentSessionsByPaneKey'],
|
||||
unreadTerminalTabs: { [TAB]: true },
|
||||
unreadTerminalPanes: { [PANE]: true },
|
||||
unreadAgentCompletionPanes: { [PANE]: true },
|
||||
lastTerminalInputAtByPaneKey: { [PANE]: 123 }
|
||||
})
|
||||
}
|
||||
|
||||
describe('bulk worktree purge evicts pane-scoped agent/unread/input maps (leak regression)', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
it('purgeWorktreeTerminalState drops every pane-scoped map for the removed worktree', () => {
|
||||
const store = createTestStore()
|
||||
seedPaneState(store)
|
||||
|
||||
store.getState().purgeWorktreeTerminalState([WT])
|
||||
|
||||
const s = store.getState()
|
||||
expect(s.agentStatusByPaneKey[PANE]).toBeUndefined()
|
||||
expect(s.agentLaunchConfigByPaneKey[PANE]).toBeUndefined()
|
||||
expect(s.acknowledgedAgentsByPaneKey[PANE]).toBeUndefined()
|
||||
expect(s.paneForegroundAgentByPaneKey[PANE]).toBeUndefined()
|
||||
expect(s.sleepingAgentSessionsByPaneKey[PANE]).toBeUndefined()
|
||||
expect(s.unreadTerminalTabs[TAB]).toBeUndefined()
|
||||
expect(s.unreadTerminalPanes[PANE]).toBeUndefined()
|
||||
expect(s.unreadAgentCompletionPanes[PANE]).toBeUndefined()
|
||||
expect(s.lastTerminalInputAtByPaneKey[PANE]).toBeUndefined()
|
||||
})
|
||||
|
||||
it('keeps pane-scoped state for worktrees that are NOT removed', () => {
|
||||
const store = createTestStore()
|
||||
const OTHER = 'repo1::/path/wt2'
|
||||
const OTHER_TAB = 'tab-2'
|
||||
const OTHER_PANE = `${OTHER_TAB}:leaf-b`
|
||||
seedStore(store, {
|
||||
worktreesByRepo: {
|
||||
repo1: [
|
||||
makeWorktree({ id: WT, repoId: 'repo1', path: '/path/wt1' }),
|
||||
makeWorktree({ id: OTHER, repoId: 'repo1', path: '/path/wt2' })
|
||||
]
|
||||
},
|
||||
tabsByWorktree: {
|
||||
[WT]: [makeTab({ id: TAB, worktreeId: WT })],
|
||||
[OTHER]: [makeTab({ id: OTHER_TAB, worktreeId: OTHER })]
|
||||
},
|
||||
agentStatusByPaneKey: { [PANE]: liveEntry(PANE), [OTHER_PANE]: liveEntry(OTHER_PANE) },
|
||||
unreadTerminalTabs: { [TAB]: true, [OTHER_TAB]: true },
|
||||
lastTerminalInputAtByPaneKey: { [PANE]: 1, [OTHER_PANE]: 2 }
|
||||
})
|
||||
|
||||
store.getState().purgeWorktreeTerminalState([WT])
|
||||
|
||||
const s = store.getState()
|
||||
expect(s.agentStatusByPaneKey[PANE]).toBeUndefined()
|
||||
expect(s.agentStatusByPaneKey[OTHER_PANE]).toBeDefined()
|
||||
expect(s.unreadTerminalTabs[OTHER_TAB]).toBe(true)
|
||||
expect(s.lastTerminalInputAtByPaneKey[OTHER_PANE]).toBe(2)
|
||||
})
|
||||
})
|
||||
@@ -1903,6 +1903,25 @@ function buildWorktreePurgeState(s: AppState, worktreeIds: string[]): Partial<Ap
|
||||
}
|
||||
return changed ? out : obj
|
||||
}
|
||||
// Pane-scoped maps are keyed by `${tabId}:${leafId}` (stable-pane-id); tabId
|
||||
// never contains ":", so the segment before the first ":" is the owning tab.
|
||||
const omitByPaneKeyTabPrefix = <T>(obj: Record<string, T>): Record<string, T> => {
|
||||
// Null-tolerant like omitByTabId: some worktree-isolation callers omit these
|
||||
// slices entirely, and the production store always initializes them to {}.
|
||||
if (!obj) {
|
||||
return obj
|
||||
}
|
||||
let changed = false
|
||||
const out = { ...obj }
|
||||
for (const paneKey of Object.keys(obj)) {
|
||||
const sep = paneKey.indexOf(':')
|
||||
if (sep > 0 && doomedTabIds.has(paneKey.slice(0, sep))) {
|
||||
delete out[paneKey]
|
||||
changed = true
|
||||
}
|
||||
}
|
||||
return changed ? out : obj
|
||||
}
|
||||
const omitByBrowserWorkspaceId = <T>(obj: Record<string, T>): Record<string, T> => {
|
||||
let changed = false
|
||||
const out = { ...obj }
|
||||
@@ -1972,6 +1991,25 @@ function buildWorktreePurgeState(s: AppState, worktreeIds: string[]): Partial<Ap
|
||||
ptyIdsByTabId: omitByTabId(s.ptyIdsByTabId),
|
||||
runtimePaneTitlesByTabId: omitByTabId(s.runtimePaneTitlesByTabId),
|
||||
automaticAgentResumeClaimsByTabId: omitByTabId(s.automaticAgentResumeClaimsByTabId),
|
||||
// Why: these tab/pane-scoped agent-status, unread, and input maps are only
|
||||
// cleared on the single removeWorktree path (via shutdownWorktreeTerminals /
|
||||
// dropAgentStatusByWorktree / clearPaneForegroundAgentByWorktree, which read
|
||||
// tabsByWorktree while it is still populated). The bulk reconcile / remove-
|
||||
// project / hydration-stale paths that reach this reducer never run terminal
|
||||
// teardown, so without this they orphan an entry per agent pane of every
|
||||
// externally-removed worktree for the session (plus a phantom unread dock
|
||||
// badge). retainedAgentsByPaneKey and runtimeAgentOrchestrationByPaneKey are
|
||||
// intentionally omitted here — both self-heal (pruneRetainedAgents on a
|
||||
// worktreesByRepo change; the runtime map is replaced wholesale each sync).
|
||||
agentStatusByPaneKey: omitByPaneKeyTabPrefix(s.agentStatusByPaneKey),
|
||||
agentLaunchConfigByPaneKey: omitByPaneKeyTabPrefix(s.agentLaunchConfigByPaneKey),
|
||||
acknowledgedAgentsByPaneKey: omitByPaneKeyTabPrefix(s.acknowledgedAgentsByPaneKey),
|
||||
paneForegroundAgentByPaneKey: omitByPaneKeyTabPrefix(s.paneForegroundAgentByPaneKey),
|
||||
sleepingAgentSessionsByPaneKey: omitByPaneKeyTabPrefix(s.sleepingAgentSessionsByPaneKey),
|
||||
unreadTerminalTabs: omitByTabId(s.unreadTerminalTabs),
|
||||
unreadTerminalPanes: omitByPaneKeyTabPrefix(s.unreadTerminalPanes),
|
||||
unreadAgentCompletionPanes: omitByPaneKeyTabPrefix(s.unreadAgentCompletionPanes),
|
||||
lastTerminalInputAtByPaneKey: omitByPaneKeyTabPrefix(s.lastTerminalInputAtByPaneKey),
|
||||
// Delete state
|
||||
deleteStateByWorktreeId: omitByWorktree(s.deleteStateByWorktreeId),
|
||||
baseStatusByWorktreeId: omitByWorktree(s.baseStatusByWorktreeId),
|
||||
|
||||
Reference in New Issue
Block a user