From 85f2f32c491822da5ff90f4ccdc0b33bc8956026 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 4 Jul 2026 17:27:31 -0700 Subject: [PATCH] fix(store): purge pane-scoped agent/unread state on external worktree removal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../agent-status-worktree-purge-leak.test.ts | 139 ++++++++++++++++++ src/renderer/src/store/slices/worktrees.ts | 38 +++++ 2 files changed, 177 insertions(+) create mode 100644 src/renderer/src/store/slices/agent-status-worktree-purge-leak.test.ts diff --git a/src/renderer/src/store/slices/agent-status-worktree-purge-leak.test.ts b/src/renderer/src/store/slices/agent-status-worktree-purge-leak.test.ts new file mode 100644 index 00000000000..681c26f81c4 --- /dev/null +++ b/src/renderer/src/store/slices/agent-status-worktree-purge-leak.test.ts @@ -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() + 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): 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) + }) +}) diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index 15d44711ac5..6d5de1c57bb 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -1903,6 +1903,25 @@ function buildWorktreePurgeState(s: AppState, worktreeIds: string[]): Partial(obj: Record): Record => { + // 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 = (obj: Record): Record => { let changed = false const out = { ...obj } @@ -1972,6 +1991,25 @@ function buildWorktreePurgeState(s: AppState, worktreeIds: string[]): Partial