From c51d955f0cabf19fc308f7efc3197752cfde42bb Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 11 Jul 2026 03:46:04 -0700 Subject: [PATCH] fix: hide completed Pi agent row after closing its terminal tab (#6584) * fix: sweep completed Pi agent row when its terminal tab is closed A 'done' Pi agent entry lingers in agentStatusByPaneKey after its terminal tab is closed: its paneKey's tabId no longer matches any open tab, so dropAgentStatusByTabPrefix never sweeps it. The sidebar selector getLiveEntriesByWorktree used an unconditional fallback (tabIdToWorktreeId.get(parsed.tabId) ?? entry.worktreeId), added in #4371 to surface early child-agent rows before their tab reaches the renderer. That fallback re-attributed the orphaned 'done' entry to the worktree, so buildWorktreeAgentRows rendered it as a stale live 'Done' row forever (Codex/Claude tear down cleanly because their pane/tab keying matches the closed tab). Restrict the worktree-attribution fallback to non-'done' entries so completed rows whose tab is gone are dropped, while still surfacing active child rows whose tab has not yet reached the renderer. Supersedes #5914 (adopted and re-verified). Fixes #5913 Co-authored-by: Alberto Cuervo * fix: drop completed Pi orphan during tab close * fix: skip retention suppressor for completed-orphan keys during tab close A completed-orphan key's tab is already gone, so buildRetainedAgentsSyncSnapshot never snapshots it and no live->gone transition ever fires to consume a planted retention suppressor. Planting one leaked a permanent retentionSuppressedPaneKeys entry per swept orphan. Plant suppressors only for prefix-matched keys. Co-authored-by: Orca --------- Co-authored-by: Alberto Cuervo Co-authored-by: Orca --- .../worktree-agent-row-selectors.test.ts | 21 ++++++ .../sidebar/worktree-agent-row-selectors.ts | 4 +- .../store/slices/agent-status-drop.test.ts | 68 ++++++++++++++++++- src/renderer/src/store/slices/agent-status.ts | 65 +++++++++++++++--- src/renderer/src/store/slices/terminals.ts | 17 ++++- 5 files changed, 161 insertions(+), 14 deletions(-) diff --git a/src/renderer/src/components/sidebar/worktree-agent-row-selectors.test.ts b/src/renderer/src/components/sidebar/worktree-agent-row-selectors.test.ts index c3d33e7f0..a40b68400 100644 --- a/src/renderer/src/components/sidebar/worktree-agent-row-selectors.test.ts +++ b/src/renderer/src/components/sidebar/worktree-agent-row-selectors.test.ts @@ -150,6 +150,27 @@ describe('selectLiveAgentStatusEntriesForWorktree', () => { expect(selectLiveAgentStatusEntriesForWorktree(state, 'wt-1')).toEqual([childEntry]) }) + + it('does not use worktree attribution for a completed row whose tab is gone', () => { + const closedEntry = makeEntry(PANE_KEY_1, 1000, { + state: 'done', + worktreeId: 'wt-1', + tabId: 'tab-1', + agentType: 'pi' + }) + const state = { + tabsByWorktree: { + 'wt-1': [makeTab('tab-live')] + }, + agentStatusByPaneKey: { + [PANE_KEY_1]: closedEntry + }, + migrationUnsupportedByPtyId: {}, + retainedAgentsByPaneKey: {} + } + + expect(selectLiveAgentStatusEntriesForWorktree(state, 'wt-1')).toEqual([]) + }) }) describe('selectRuntimeAgentOrchestrationForWorktree', () => { diff --git a/src/renderer/src/components/sidebar/worktree-agent-row-selectors.ts b/src/renderer/src/components/sidebar/worktree-agent-row-selectors.ts index 8fe9290ba..567454f45 100644 --- a/src/renderer/src/components/sidebar/worktree-agent-row-selectors.ts +++ b/src/renderer/src/components/sidebar/worktree-agent-row-selectors.ts @@ -97,7 +97,9 @@ function getLiveEntriesByWorktree(state: WorktreeAgentRowsState): Map { expect(s.sortEpoch).toBe(sortEpochBefore + 1) }) + it('closeTab drops completed worktree-attributed orphan rows', () => { + vi.useFakeTimers() + const store = createTestStore() + store.setState({ + tabsByWorktree: { + 'wt-1': [ + makeTab({ id: 'tab-closed', worktreeId: 'wt-1' }), + makeTab({ id: 'tab-live', worktreeId: 'wt-1' }) + ], + 'wt-2': [] + } + }) + store + .getState() + .setAgentStatus('tab-closed:0', { state: 'done', prompt: 'closed', agentType: 'pi' }) + store + .getState() + .setAgentStatus( + 'tab-orphan:0', + { state: 'done', prompt: 'orphan', agentType: 'pi' }, + undefined, + undefined, + { worktreeId: 'wt-1' } + ) + store + .getState() + .setAgentStatus( + 'tab-active-child:0', + { state: 'working', prompt: 'active child', agentType: 'pi' }, + undefined, + undefined, + { worktreeId: 'wt-1' } + ) + store + .getState() + .setAgentStatus( + 'tab-live:0', + { state: 'done', prompt: 'open tab', agentType: 'pi' }, + undefined, + undefined, + { worktreeId: 'wt-1' } + ) + store + .getState() + .setAgentStatus( + 'tab-other-orphan:0', + { state: 'done', prompt: 'other worktree', agentType: 'pi' }, + undefined, + undefined, + { worktreeId: 'wt-2' } + ) + + store.getState().closeTab('tab-closed') + + const s = store.getState() + expect(s.tabsByWorktree['wt-1']?.some((tab) => tab.id === 'tab-closed')).toBe(false) + expect(s.agentStatusByPaneKey['tab-closed:0']).toBeUndefined() + expect(s.agentStatusByPaneKey['tab-orphan:0']).toBeUndefined() + // No suppressor for the orphan: its tab is already gone, so retention sync + // never re-surfaces it and a suppressor would leak permanently. + expect(s.retentionSuppressedPaneKeys['tab-orphan:0']).toBeUndefined() + expect(s.agentStatusByPaneKey['tab-active-child:0']).toBeDefined() + expect(s.agentStatusByPaneKey['tab-live:0']).toBeDefined() + expect(s.agentStatusByPaneKey['tab-other-orphan:0']).toBeDefined() + }) + it('on a paneKey with neither live nor retained entry: no-op (same state reference, no epoch bumps)', () => { vi.useFakeTimers() const store = createTestStore() diff --git a/src/renderer/src/store/slices/agent-status.ts b/src/renderer/src/store/slices/agent-status.ts index 05f0c2039..99ae087f5 100644 --- a/src/renderer/src/store/slices/agent-status.ts +++ b/src/renderer/src/store/slices/agent-status.ts @@ -64,6 +64,10 @@ type DropHibernatedAgentPaneOptions = { retainedCompletionEvidence?: readonly RetainedAgentEntry[] } +type DropAgentStatusByTabPrefixOptions = { + worktreeId?: string +} + type AgentLaunchConfigRegistrationMetadata = { agentType?: AgentType launchToken?: string @@ -175,7 +179,10 @@ export type AgentStatusSlice = { /** Remove all entries under a tab AND suppress re-retention for each. * Used on tab close — the user is tearing down the whole tab, so any * remaining agent rows (live or retained) must not reappear. */ - dropAgentStatusByTabPrefix: (tabIdPrefix: string) => void + dropAgentStatusByTabPrefix: ( + tabIdPrefix: string, + opts?: DropAgentStatusByTabPrefixOptions + ) => void /** Remove one automatically hibernated completed-agent pane while preserving * sibling live/retained rows in the same worktree. */ @@ -307,6 +314,29 @@ function getLeafIdFromPaneKey(paneKey: string): string | null { return leafId.length > 0 ? leafId : null } +function findCompletedOrphanPaneKeysForTabClose( + state: AppState, + worktreeId: string | undefined, + prefix: string +): string[] { + if (!worktreeId) { + return [] + } + const openTabIds = new Set((state.tabsByWorktree[worktreeId] ?? []).map((tab) => tab.id)) + const paneKeys: string[] = [] + for (const [paneKey, entry] of Object.entries(state.agentStatusByPaneKey)) { + if (paneKey.startsWith(prefix) || entry.state !== 'done' || entry.worktreeId !== worktreeId) { + continue + } + const tabId = getTabIdFromPaneKey(paneKey) + if (!tabId || openTabIds.has(tabId)) { + continue + } + paneKeys.push(paneKey) + } + return paneKeys +} + function isRecentlyClosedAgentStatusTab( closedTabs: Record, tabId: string | null @@ -1810,16 +1840,25 @@ export const createAgentStatusSlice: StateCreator { + dropAgentStatusByTabPrefix: (tabIdPrefix, opts) => { const prefix = `${tabIdPrefix}:` let hadLive = false set((s) => { - const liveKeys = Object.keys(s.agentStatusByPaneKey).filter((k) => k.startsWith(prefix)) - const launchConfigKeys = Object.keys(s.agentLaunchConfigByPaneKey).filter((k) => - k.startsWith(prefix) + const completedOrphanKeys = findCompletedOrphanPaneKeysForTabClose( + s, + opts?.worktreeId, + prefix ) - const retainedKeys = Object.keys(s.retainedAgentsByPaneKey).filter((k) => - k.startsWith(prefix) + const completedOrphanKeySet = new Set(completedOrphanKeys) + const liveKeys = [ + ...Object.keys(s.agentStatusByPaneKey).filter((k) => k.startsWith(prefix)), + ...completedOrphanKeys + ] + const launchConfigKeys = Object.keys(s.agentLaunchConfigByPaneKey).filter( + (k) => k.startsWith(prefix) || completedOrphanKeySet.has(k) + ) + const retainedKeys = Object.keys(s.retainedAgentsByPaneKey).filter( + (k) => k.startsWith(prefix) || completedOrphanKeySet.has(k) ) const migrationUnsupported = pruneMigrationUnsupportedEntries( s.migrationUnsupportedByPtyId, @@ -1829,7 +1868,9 @@ export const createAgentStatusSlice: StateCreator k.startsWith(prefix)) + const ackKeys = Object.keys(nextAck).filter( + (k) => k.startsWith(prefix) || completedOrphanKeySet.has(k) + ) if (ackKeys.length > 0) { nextAck = { ...nextAck } for (const k of ackKeys) { @@ -1890,7 +1931,13 @@ export const createAgentStatusSlice: StateCreator !(k in s.retentionSuppressedPaneKeys)) + // + // Skip completed-orphan keys: their tab is already gone, so retention + // sync never snapshots them and no live→gone transition ever fires to + // consume the suppressor — planting one would leak permanently. + const suppressorAdds = liveKeys.filter( + (k) => !completedOrphanKeySet.has(k) && !(k in s.retentionSuppressedPaneKeys) + ) let nextRetentionSuppressedPaneKeys = s.retentionSuppressedPaneKeys if (suppressorAdds.length > 0) { nextRetentionSuppressedPaneKeys = { ...s.retentionSuppressedPaneKeys } diff --git a/src/renderer/src/store/slices/terminals.ts b/src/renderer/src/store/slices/terminals.ts index 514f5b01e..a674e3a11 100644 --- a/src/renderer/src/store/slices/terminals.ts +++ b/src/renderer/src/store/slices/terminals.ts @@ -1188,13 +1188,18 @@ export const createTerminalSlice: StateCreator }, closeTab: (tabId, opts) => { + let closingWorktreeId: string | null = null set((s) => { const next = { ...s.tabsByWorktree } let closingPtyId: string | null = null for (const wId of Object.keys(next)) { const before = next[wId] - if (!closingPtyId) { - closingPtyId = before.find((t) => t.id === tabId)?.ptyId ?? null + const closingTab = before.find((t) => t.id === tabId) + if (closingTab) { + closingWorktreeId = wId + if (!closingPtyId) { + closingPtyId = closingTab.ptyId ?? null + } } const after = before.filter((t) => t.id !== tabId) if (after.length !== before.length) { @@ -1345,7 +1350,13 @@ export const createTerminalSlice: StateCreator // too. Use dropAgentStatusByTabPrefix (not removeAgentStatusByTabPrefix) // so retention suppressors are planted: a live→gone transition inside the // same frame as the tab close cannot re-snapshot a row we just dropped. - get().dropAgentStatusByTabPrefix(tabId) + // Why: Pi can leave a completed row attributed to the worktree but keyed + // under an already-missing tab id; pass the worktree to sweep only that + // completed orphan while preserving active pre-render child rows. + get().dropAgentStatusByTabPrefix( + tabId, + closingWorktreeId ? { worktreeId: closingWorktreeId } : undefined + ) // Why: retired pane keys never recur, so stranded foreground entries would // accumulate for the renderer's whole lifetime. get().clearPaneForegroundAgentByTabPrefix(tabId)