fix(terminal): clear sleeping-agent records for invalid-tab-id rows

Rows with invalid tab IDs are dropped like subsumed rows but were not getting their sleeping-agent records cleaned up. Extract common test mocks into a bootstrap file to reduce duplication.
This commit is contained in:
Jinjing
2026-08-08 17:38:42 -07:00
parent f13bde2b1a
commit 3d9a544867
5 changed files with 125 additions and 45 deletions
@@ -0,0 +1,21 @@
import { vi } from 'vitest'
// Why: import this before the store modules — session hydration reaches for the preload API and the
// runtime/PTY singletons, which don't exist under vitest.
vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } }))
vi.mock('@/runtime/sync-runtime-graph', () => ({
scheduleRuntimeGraphSync: vi.fn()
}))
vi.mock('@/components/terminal-pane/pty-transport', () => ({
registerEagerPtyBuffer: vi.fn(),
ensurePtyDispatcher: vi.fn()
}))
const apiProxy = (): unknown =>
new Proxy(() => undefined, {
get: (_target, prop) => (prop === 'then' ? undefined : apiProxy()),
apply: () => Promise.resolve(null)
})
// @ts-expect-error -- mocked browser preload API
globalThis.window = { api: apiProxy() }
@@ -38,6 +38,8 @@ export type WorkspaceTerminalRowHydration = {
releasedPtyIdsByTabId: Map<string, Set<string>>
/** Rows dropped as pure canonical duplicates; they are never retired, so callers clean up after them. */
subsumedTabIds: string[]
/** Rows dropped for an unusable tab id; like subsumed rows they need caller-side cleanup. */
invalidTabIds: string[]
/** The canonical row that inherited each subsumed row's PTYs, so pointers at it can follow. */
canonicalTabIdBySubsumedTabId: Map<string, string>
}
@@ -51,11 +53,13 @@ export function hydrateWorkspaceTerminalRows(
const canonical = readCanonicalTerminals(session, worktreeId, rows)
const releasedPtyIdsByTabId = new Map<string, Set<string>>()
const subsumedTabIds: string[] = []
const invalidTabIds: string[] = []
const canonicalTabIdBySubsumedTabId = new Map<string, string>()
const retained: TerminalTab[] = []
for (const row of rows) {
// Why: old web-client mirrors could persist host surface ids with "::"; makePaneKey reserves ":" as its separator.
if (!isValidTerminalTabId(row.id)) {
invalidTabIds.push(row.id)
continue
}
const claim = options.rowsFromRemoteSnapshot
@@ -77,6 +81,7 @@ export function hydrateWorkspaceTerminalRows(
.map((row, index) => restoreCanonicalMetadata(row, index, canonical)),
releasedPtyIdsByTabId,
subsumedTabIds,
invalidTabIds,
canonicalTabIdBySubsumedTabId
}
}
@@ -1,27 +1,10 @@
import { describe, expect, it, vi } from 'vitest'
import './terminal-hydration-store-test-bootstrap'
import { describe, expect, it } from 'vitest'
import type { Tab, TerminalTab, WorkspaceSessionState } from '../../../../shared/types'
import { getDefaultWorkspaceSession } from '../../../../shared/constants'
import { buildWorkspaceSessionPayload } from '@/lib/workspace-session'
import { createTestStore, makeLayout, makeTab, makeWorktree, seedStore } from './store-test-helpers'
vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } }))
vi.mock('@/runtime/sync-runtime-graph', () => ({
scheduleRuntimeGraphSync: vi.fn()
}))
vi.mock('@/components/terminal-pane/pty-transport', () => ({
registerEagerPtyBuffer: vi.fn(),
ensurePtyDispatcher: vi.fn()
}))
const apiProxy = (): unknown =>
new Proxy(() => undefined, {
get: (_target, prop) => (prop === 'then' ? undefined : apiProxy()),
apply: () => Promise.resolve(null)
})
// @ts-expect-error -- mocked browser preload API
globalThis.window = { api: apiProxy() }
const WORKTREE_ID = 'repo1::/wt-1'
function makeCanonicalUnifiedTab(entityId: string, sortOrder: number): Tab {
@@ -140,16 +123,17 @@ describe('hydrateWorkspaceSession canonical PTY overlap', () => {
'split-tab'
])
expect(state.pendingReconnectPtyIdByTabId['split-tab']).toBe(soloPtyId)
expect(
Object.values(state.terminalLayoutsByTabId['split-tab']?.ptyIdsByLeafId ?? {})
).toContain(soloPtyId)
// Why: the split row keeps its own PTY and gives up the one the canonical row owns.
expect(Object.values(state.terminalLayoutsByTabId['split-tab']?.ptyIdsByLeafId ?? {})).toEqual([
soloPtyId
])
expect(persisted.tabsByWorktree[WORKTREE_ID]?.map((tab) => tab.id)).toEqual([
'canonical-tab',
'split-tab'
])
expect(
Object.values(persisted.terminalLayoutsByTabId['split-tab']?.ptyIdsByLeafId ?? {})
).toContain(soloPtyId)
).toEqual([soloPtyId])
})
it('ignores a canonical row’s stale leaf binding when scoring another row’s live PTY', () => {
@@ -1,27 +1,11 @@
import { describe, expect, it, vi } from 'vitest'
import './terminal-hydration-store-test-bootstrap'
import { describe, expect, it } from 'vitest'
import type { SleepingAgentSessionRecord } from '../../../../shared/agent-session-resume'
import type { WorkspaceSessionState } from '../../../../shared/types'
import { getDefaultWorkspaceSession } from '../../../../shared/constants'
import { buildWorkspaceSessionPayload } from '@/lib/workspace-session'
import { createTestStore, makeLayout, makeTab, makeWorktree, seedStore } from './store-test-helpers'
vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } }))
vi.mock('@/runtime/sync-runtime-graph', () => ({
scheduleRuntimeGraphSync: vi.fn()
}))
vi.mock('@/components/terminal-pane/pty-transport', () => ({
registerEagerPtyBuffer: vi.fn(),
ensurePtyDispatcher: vi.fn()
}))
const apiProxy = (): unknown =>
new Proxy(() => undefined, {
get: (_target, prop) => (prop === 'then' ? undefined : apiProxy()),
apply: () => Promise.resolve(null)
})
// @ts-expect-error -- mocked browser preload API
globalThis.window = { api: apiProxy() }
describe('hydrateWorkspaceSession canonical terminal rows', () => {
it('drops only legacy rows that duplicate canonical PTY ownership', () => {
const store = createTestStore()
@@ -128,4 +112,90 @@ describe('hydrateWorkspaceSession canonical terminal rows', () => {
expect(state.activeTabId).toBe('canonical-tab')
expect(state.activeTabIdByWorktree).toEqual({ [worktreeId]: 'canonical-tab' })
})
it('clears sleeping-agent records for subsumed and invalid-id rows only', () => {
const store = createTestStore()
const worktreeId = 'repo1::/wt-1'
const sharedPtyId = 'daemon-session-1'
const recoveryPtyId = 'daemon-session-2'
const invalidTabId = 'host-tab::11111111-1111-4111-8111-111111111111'
const leafId = '22222222-2222-4222-8222-222222222222'
seedStore(store, {
worktreesByRepo: {
repo1: [makeWorktree({ id: worktreeId, repoId: 'repo1', path: '/wt-1' })]
}
})
const makeSleepingRecord = (tabId: string): SleepingAgentSessionRecord => ({
paneKey: `${tabId}:${leafId}`,
tabId,
worktreeId,
agent: 'codex',
providerSession: { key: 'session_id', id: `session-${tabId}` },
prompt: 'continue',
state: 'working',
capturedAt: 1,
updatedAt: 1
})
const session: WorkspaceSessionState = {
...getDefaultWorkspaceSession(),
activeRepoId: 'repo1',
activeWorktreeId: worktreeId,
activeWorktreeIdsOnShutdown: [worktreeId],
tabsByWorktree: {
[worktreeId]: [
makeTab({ id: 'canonical-tab', worktreeId, ptyId: sharedPtyId }),
makeTab({ id: 'stale-tab', worktreeId, ptyId: sharedPtyId }),
makeTab({ id: invalidTabId, worktreeId, ptyId: null }),
makeTab({ id: 'recovery-tab', worktreeId, ptyId: recoveryPtyId })
]
},
terminalLayoutsByTabId: {
'canonical-tab': { ...makeLayout(), ptyIdsByLeafId: { 'canonical-leaf': sharedPtyId } },
'stale-tab': { ...makeLayout(), ptyIdsByLeafId: { 'stale-leaf': sharedPtyId } },
'recovery-tab': { ...makeLayout(), ptyIdsByLeafId: { 'recovery-leaf': recoveryPtyId } }
},
sleepingAgentSessionsByPaneKey: {
[`stale-tab:${leafId}`]: makeSleepingRecord('stale-tab'),
[`${invalidTabId}:${leafId}`]: makeSleepingRecord(invalidTabId),
[`recovery-tab:${leafId}`]: makeSleepingRecord('recovery-tab')
},
unifiedTabs: {
[worktreeId]: [
{
id: 'canonical-unified-tab',
entityId: 'canonical-tab',
groupId: 'group-1',
worktreeId,
contentType: 'terminal',
label: 'Grok',
customLabel: null,
color: null,
sortOrder: 0,
createdAt: 1
}
]
},
tabGroups: {
[worktreeId]: [
{
id: 'group-1',
worktreeId,
activeTabId: 'canonical-unified-tab',
tabOrder: ['canonical-unified-tab']
}
]
}
}
store.getState().hydrateWorkspaceSession(session)
const state = store.getState()
expect(state.tabsByWorktree[worktreeId]?.map((tab) => tab.id)).toEqual([
'canonical-tab',
'recovery-tab'
])
// Why: both dropped classes keep a valid worktreeId, so only the per-tab sweep can evict them.
expect(Object.keys(state.sleepingAgentSessionsByPaneKey)).toEqual([`recovery-tab:${leafId}`])
})
})
+3 -3
View File
@@ -3982,10 +3982,10 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
validWorktreeIds.has(record.worktreeId)
)
)
// Why: a subsumed row is dropped rather than retired, so nothing else clears the sleeping
// records keyed to it and the workspace keeps a pane nobody can wake.
// Why: a dropped row is never retired, so nothing else clears the sleeping records keyed to
// it — its worktree is still valid — and the workspace keeps a pane nobody can wake.
for (const [, hydration] of rowHydrationByWorktree) {
for (const tabId of hydration.subsumedTabIds) {
for (const tabId of [...hydration.subsumedTabIds, ...hydration.invalidTabIds]) {
sleepingAgentSessionsByPaneKey = removeSleepingAgentSessionsForTab(
sleepingAgentSessionsByPaneKey,
tabId