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 <Br1NKOL@users.noreply.github.com> * 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 <help@stably.ai> --------- Co-authored-by: Alberto Cuervo <Br1NKOL@users.noreply.github.com> Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Neil
GitHub
parent
25ecf2eea2
commit
c51d955f0c
|
|
@ -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', () => {
|
||||
|
|
|
|||
|
|
@ -97,7 +97,9 @@ function getLiveEntriesByWorktree(state: WorktreeAgentRowsState): Map<string, Ag
|
|||
if (!parsed) {
|
||||
continue
|
||||
}
|
||||
const worktreeId = tabIdToWorktreeId.get(parsed.tabId) ?? entry.worktreeId
|
||||
const tabWorktreeId = tabIdToWorktreeId.get(parsed.tabId)
|
||||
// Why: keep early attributed child rows, but hide completed rows once their tab is gone.
|
||||
const worktreeId = tabWorktreeId ?? (entry.state === 'done' ? undefined : entry.worktreeId)
|
||||
if (!worktreeId) {
|
||||
continue
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest'
|
|||
import type { AgentStatusEntry } from '../../../../shared/agent-status-types'
|
||||
import type { TerminalTab } from '../../../../shared/types'
|
||||
import type { RetainedAgentEntry } from './agent-status'
|
||||
import { createTestStore } from './store-test-helpers'
|
||||
import { createTestStore, makeTab } from './store-test-helpers'
|
||||
|
||||
// Why: split out from agent-status.test.ts to keep each file under the
|
||||
// repo's 300-line cap for test files. This suite covers the new
|
||||
|
|
@ -128,6 +128,72 @@ describe('dropAgentStatus + retention suppressor', () => {
|
|||
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()
|
||||
|
|
|
|||
|
|
@ -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<string, true>,
|
||||
tabId: string | null
|
||||
|
|
@ -1810,16 +1840,25 @@ export const createAgentStatusSlice: StateCreator<AppState, [], [], AgentStatusS
|
|||
}
|
||||
},
|
||||
|
||||
dropAgentStatusByTabPrefix: (tabIdPrefix) => {
|
||||
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<AppState, [], [], AgentStatusS
|
|||
// regardless of live/retained presence — ack entries are owned by
|
||||
// the pane lifecycle independently of live/retained state.
|
||||
let nextAck = s.acknowledgedAgentsByPaneKey
|
||||
const ackKeys = Object.keys(nextAck).filter((k) => 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<AppState, [], [], AgentStatusS
|
|||
// the user just tore it down. Planting suppressors is the cheap guard
|
||||
// for the common ordering; the rare inverse ordering has the same
|
||||
// bounded suppressor-leak tradeoff described in dropAgentStatus.
|
||||
const suppressorAdds = liveKeys.filter((k) => !(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 }
|
||||
|
|
|
|||
|
|
@ -1188,13 +1188,18 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
|
|||
},
|
||||
|
||||
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<AppState, [], [], TerminalSlice>
|
|||
// 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)
|
||||
|
|
|
|||
Loading…
Reference in New Issue