fix(terminal): deduplicate rows with correct orphaned PTY handling

- Extract row hydration to terminal-session-row-hydration.ts
- Distinguish mounted vs orphaned PTY bindings when scoring ownership
- Strip shared PTYs from retained rows to prevent duplicate reconnection
- Follow dropped row focus to canonical twin; clean up orphaned agent sessions
This commit is contained in:
Jinjing
2026-08-07 15:00:35 -07:00
parent d34d389850
commit 539249bfa0
4 changed files with 350 additions and 78 deletions
@@ -0,0 +1,217 @@
import type {
TerminalLayoutSnapshot,
TerminalTab,
WorkspaceSessionState
} from '../../../../shared/types'
import type { AiVaultSessionTitle } from '../../../../shared/ai-vault-session-title'
import { isValidTerminalTabId } from '../../../../shared/terminal-tab-id'
import { collectLeafIdsInOrder } from '@/components/terminal-pane/terminal-layout-leaf-ids'
import { clearTransientTerminalState } from './terminal-helpers'
/**
* A persisted session describes the same terminals twice: as canonical unified tabs and as
* legacy per-worktree terminal rows. Hydration keeps every row that still owns a PTY nothing
* else can reattach to, drops rows the canonical mount fully subsumes, and strips the shared
* PTYs off rows it only partially subsumes.
*/
type CanonicalTerminals = {
/** Terminal ids the unified tab model owns in this workspace. */
tabIds: Set<string>
/** PTYs already claimed by canonical rows that survive hydration, keyed to their claimant. */
tabIdByPtyId: Map<string, string>
quickCommandLabelByTabId: Map<string, string>
aiVaultTitleByTabId: Map<string, AiVaultSessionTitle>
}
export type HydrateWorkspaceTerminalRowsOptions = {
/**
* Rows arrived from a remote snapshot. `unifiedTabs` is not on the remote wire, so the session's
* canonical list still describes the local client and cannot arbitrate ownership of these rows.
*/
rowsFromRemoteSnapshot?: boolean
}
export type WorkspaceTerminalRowHydration = {
rows: TerminalTab[]
/** PTYs a retained row must give up because a canonical row already owns them. */
releasedPtyIdsByTabId: Map<string, Set<string>>
/** Rows dropped as pure canonical duplicates; they are never retired, so callers clean up after them. */
subsumedTabIds: string[]
/** The canonical row that inherited each subsumed row's PTYs, so pointers at it can follow. */
canonicalTabIdBySubsumedTabId: Map<string, string>
}
export function hydrateWorkspaceTerminalRows(
session: WorkspaceSessionState,
worktreeId: string,
rows: readonly TerminalTab[],
options: HydrateWorkspaceTerminalRowsOptions = {}
): WorkspaceTerminalRowHydration {
const canonical = readCanonicalTerminals(session, worktreeId, rows)
const releasedPtyIdsByTabId = new Map<string, Set<string>>()
const subsumedTabIds: 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)) {
continue
}
const claim = options.rowsFromRemoteSnapshot
? RETAINED_UNCLAIMED
: resolveCanonicalPtyClaim(session, row, canonical)
if (claim.kind === 'subsumed') {
subsumedTabIds.push(row.id)
canonicalTabIdBySubsumedTabId.set(row.id, claim.canonicalTabId)
continue
}
if (claim.releasedPtyIds.size > 0) {
releasedPtyIdsByTabId.set(row.id, claim.releasedPtyIds)
}
retained.push(row)
}
return {
rows: retained
.sort((a, b) => a.sortOrder - b.sortOrder || a.createdAt - b.createdAt)
.map((row, index) => restoreCanonicalMetadata(row, index, canonical)),
releasedPtyIdsByTabId,
subsumedTabIds,
canonicalTabIdBySubsumedTabId
}
}
/** Drop the PTYs a canonical row owns so the retained row records only its own panes. */
export function releaseTerminalLayoutPtyIds(
layout: TerminalLayoutSnapshot,
releasedPtyIds: ReadonlySet<string>
): TerminalLayoutSnapshot {
const ptyIdsByLeafId = layout.ptyIdsByLeafId
if (!ptyIdsByLeafId) {
return layout
}
const kept = Object.entries(ptyIdsByLeafId).filter(([, ptyId]) => !releasedPtyIds.has(ptyId))
if (kept.length === Object.keys(ptyIdsByLeafId).length) {
return layout
}
return { ...layout, ptyIdsByLeafId: Object.fromEntries(kept) }
}
function readCanonicalTerminals(
session: WorkspaceSessionState,
worktreeId: string,
rows: readonly TerminalTab[]
): CanonicalTerminals {
const canonicalTabs = (session.unifiedTabs?.[worktreeId] ?? []).filter(
(tab) => tab.contentType === 'terminal'
)
const tabIds = new Set(canonicalTabs.map((tab) => tab.entityId))
// Why: only rows that survive the id check can claim PTY ownership; a dropped invalid-id mirror must not evict the valid row sharing its PTY.
const tabIdByPtyId = new Map(
rows
.filter((row) => tabIds.has(row.id) && isValidTerminalTabId(row.id))
.flatMap((row) =>
readPersistedTerminalPtyIds(session, row).owned.map((ptyId) => [ptyId, row.id] as const)
)
)
return {
tabIds,
tabIdByPtyId,
quickCommandLabelByTabId: new Map(
canonicalTabs.flatMap((tab) =>
tab.quickCommandLabel?.trim() ? [[tab.entityId, tab.quickCommandLabel.trim()]] : []
)
),
aiVaultTitleByTabId: new Map(
canonicalTabs.flatMap((tab) => (tab.aiVaultTitle ? [[tab.entityId, tab.aiVaultTitle]] : []))
)
}
}
type CanonicalPtyClaim =
| { kind: 'subsumed'; canonicalTabId: string }
| { kind: 'retained'; releasedPtyIds: Set<string> }
const RETAINED_UNCLAIMED: CanonicalPtyClaim = { kind: 'retained', releasedPtyIds: new Set() }
/** Canonical mounts win PTY ownership over stale legacy duplicates of the same terminal. */
function resolveCanonicalPtyClaim(
session: WorkspaceSessionState,
row: TerminalTab,
canonical: CanonicalTerminals
): CanonicalPtyClaim {
if (canonical.tabIds.has(row.id)) {
return RETAINED_UNCLAIMED
}
const { owned, orphaned } = readPersistedTerminalPtyIds(session, row)
const claimed = owned.filter((ptyId) => canonical.tabIdByPtyId.has(ptyId))
// Why: an unclaimed row has no canonical twin, which also keeps a PTY-less row — it duplicates
// nothing — out of the fully-claimed branch.
const canonicalTabId = claimed.length > 0 ? canonical.tabIdByPtyId.get(claimed[0]) : undefined
if (canonicalTabId && claimed.length === owned.length) {
return { kind: 'subsumed', canonicalTabId }
}
// Why: a split row with an independent pane owns a PTY nothing else can reattach to, but keeping the
// shared PTY too would leave two recorded owners and make ownership resolution ambiguous (#10486).
// Stale unmounted bindings go too, else reconnect republishes the canonical PTY under this row.
return {
kind: 'retained',
releasedPtyIds: new Set([
...claimed,
...orphaned.filter((ptyId) => canonical.tabIdByPtyId.has(ptyId))
])
}
}
function restoreCanonicalMetadata(
row: TerminalTab,
index: number,
canonical: CanonicalTerminals
): TerminalTab {
const quickCommandLabel =
row.quickCommandLabel?.trim() || canonical.quickCommandLabelByTabId.get(row.id)
const aiVaultTitle = row.aiVaultTitle ?? canonical.aiVaultTitleByTabId.get(row.id)
return {
...clearTransientTerminalState(row, index),
...(quickCommandLabel ? { quickCommandLabel } : {}),
...(aiVaultTitle ? { aiVaultTitle } : {}),
sortOrder: index,
// Why: suppress restored mounts so only real activity updates Recent.
pendingActivationSpawn: true
}
}
type PersistedTerminalPtyIds = {
/** PTYs a live pane or the tab itself still reattaches to — the only ones that prove ownership. */
owned: string[]
/** PTYs stranded in `ptyIdsByLeafId` by panes that already left the tree; they reattach nothing. */
orphaned: string[]
}
function readPersistedTerminalPtyIds(
session: WorkspaceSessionState,
tab: TerminalTab
): PersistedTerminalPtyIds {
const layout = session.terminalLayoutsByTabId[tab.id]
// Why: ptyIdsByLeafId is merged but never pruned, and hydration reads it before
// normalizeTerminalLayoutSnapshot runs, so unmounted leaves still carry dead bindings here.
// Rootless layouts bind their sole pane off-tree, so treat every entry as mounted.
const mountedLeafIds = layout?.root ? new Set(collectLeafIdsInOrder(layout.root)) : null
const mounted: string[] = []
const unmounted: string[] = []
for (const [leafId, ptyId] of Object.entries(layout?.ptyIdsByLeafId ?? {})) {
if (!ptyId) {
continue
}
;(!mountedLeafIds || mountedLeafIds.has(leafId) ? mounted : unmounted).push(ptyId)
}
const owned = new Set(
[tab.ptyId, session.remoteSessionIdsByTabId?.[tab.id], ...mounted].filter(
(ptyId): ptyId is string => Boolean(ptyId)
)
)
return {
owned: [...owned],
orphaned: [...new Set(unmounted)].filter((ptyId) => !owned.has(ptyId))
}
}
@@ -152,6 +152,68 @@ describe('hydrateWorkspaceSession canonical PTY overlap', () => {
).toContain(soloPtyId)
})
it('ignores a canonical row’s stale leaf binding when scoring another row’s live PTY', () => {
const canonicalPtyId = 'daemon-canonical'
const livePtyId = 'daemon-live'
const session = makeSession({
tabs: [
makeTab({ id: 'canonical-tab', worktreeId: WORKTREE_ID, ptyId: canonicalPtyId }),
makeTab({ id: 'live-tab', worktreeId: WORKTREE_ID, ptyId: livePtyId, sortOrder: 1 })
],
layouts: {
// 'ghost-leaf' left the tree but its binding was never pruned, so it must not claim livePtyId.
'canonical-tab': {
root: { type: 'leaf', leafId: 'canonical-leaf' },
activeLeafId: 'canonical-leaf',
expandedLeafId: null,
ptyIdsByLeafId: { 'canonical-leaf': canonicalPtyId, 'ghost-leaf': livePtyId }
},
'live-tab': { ...makeLayout(), ptyIdsByLeafId: { 'live-leaf': livePtyId } }
},
canonicalEntityIds: ['canonical-tab']
})
const state = hydrate(session).getState()
expect(state.tabsByWorktree[WORKTREE_ID]?.map((tab) => tab.id)).toEqual([
'canonical-tab',
'live-tab'
])
expect(state.pendingReconnectPtyIdByTabId['live-tab']).toBe(livePtyId)
})
it('strips a retained row’s stale binding to a PTY the canonical row owns', () => {
const sharedPtyId = 'daemon-shared'
const soloPtyId = 'daemon-solo'
const session = makeSession({
tabs: [
makeTab({ id: 'canonical-tab', worktreeId: WORKTREE_ID, ptyId: sharedPtyId }),
makeTab({ id: 'legacy-tab', worktreeId: WORKTREE_ID, ptyId: soloPtyId, sortOrder: 1 })
],
layouts: {
'canonical-tab': { ...makeLayout(), ptyIdsByLeafId: { 'canonical-leaf': sharedPtyId } },
'legacy-tab': {
root: { type: 'leaf', leafId: 'leaf-a' },
activeLeafId: 'leaf-a',
expandedLeafId: null,
ptyIdsByLeafId: { 'leaf-a': soloPtyId, 'ghost-leaf': sharedPtyId }
}
},
canonicalEntityIds: ['canonical-tab']
})
const state = hydrate(session).getState()
expect(state.tabsByWorktree[WORKTREE_ID]?.map((tab) => tab.id)).toEqual([
'canonical-tab',
'legacy-tab'
])
// Why: reconnect publishes every recorded leaf PTY, so a leftover binding would re-duplicate ownership.
expect(Object.values(state.terminalLayoutsByTabId['legacy-tab']?.ptyIdsByLeafId ?? {})).toEqual(
[soloPtyId]
)
})
it('retains a non-canonical row that owns no PTY at all', () => {
const sharedPtyId = 'daemon-shared'
const session = makeSession({
@@ -124,7 +124,8 @@ describe('hydrateWorkspaceSession canonical terminal rows', () => {
'canonical-tab': sharedPtyId,
'recovery-tab': recoveryPtyId
})
expect(state.activeTabId).toBeNull()
expect(state.activeTabIdByWorktree).toEqual({})
// The canonical row inherited the dropped row's PTY, so focus follows it instead of resetting.
expect(state.activeTabId).toBe('canonical-tab')
expect(state.activeTabIdByWorktree).toEqual({ [worktreeId]: 'canonical-tab' })
})
})
+68 -76
View File
@@ -36,7 +36,7 @@ import {
parseLegacyNumericPaneKey,
parsePaneKey
} from '../../../../shared/stable-pane-id'
import { isValidHostTerminalTabId, isValidTerminalTabId } from '../../../../shared/terminal-tab-id'
import { isValidHostTerminalTabId } from '../../../../shared/terminal-tab-id'
import { buildByIdIndex, buildWorktreeByIdIndex } from './worktree-by-id-index'
import { resolveActiveTabOwnerWorktreeId } from './active-tab-owner-worktree'
import { isSameCodexRestartNoticeAccount } from './codex-restart-notice-account-identity'
@@ -58,6 +58,10 @@ import { forgetForegroundTerminalTabs } from '@/lib/foreground-terminal-tabs'
import { terminalLayoutEqual } from '@/lib/terminal-layout-equality'
import { forgetAgentStartupDeliveriesForTabs } from '@/lib/agent-startup-delivery-guards'
import { clearTransientTerminalState, emptyLayoutSnapshot } from './terminal-helpers'
import {
hydrateWorkspaceTerminalRows,
releaseTerminalLayoutPtyIds
} from './terminal-session-row-hydration'
import {
getRecentlyClosedTabPosition,
pushClosedTerminalTabSnapshot,
@@ -492,17 +496,6 @@ function uniquePtyIds(ptyIds: readonly (string | null | undefined)[]): string[]
return [...new Set(ptyIds.filter((ptyId): ptyId is string => Boolean(ptyId)))]
}
function collectPersistedTerminalPtyIds(
session: WorkspaceSessionState,
tab: TerminalTab
): string[] {
return uniquePtyIds([
tab.ptyId,
session.remoteSessionIdsByTabId?.[tab.id],
...Object.values(session.terminalLayoutsByTabId[tab.id]?.ptyIdsByLeafId ?? {})
])
}
function resolvePrimaryLayoutPtyId(layout: TerminalLayoutSnapshot): string | null {
const ptyIdsByLeafId = layout.ptyIdsByLeafId ?? {}
const activePtyId = layout.activeLeafId ? ptyIdsByLeafId[layout.activeLeafId] : undefined
@@ -3949,77 +3942,56 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
validWorktreeIds.add(folderWorkspaceKey(workspace.id))
}
addAdditionalValidWorkspaceKeys(validWorktreeIds, options)
// Why: suppress restored mounts so only real activity updates Recent.
const tabsByWorktree: Record<string, TerminalTab[]> = Object.fromEntries(
Object.entries(session.tabsByWorktree)
.filter(([worktreeId]) => validWorktreeIds.has(worktreeId))
.map(([worktreeId, tabs]) => {
const canonicalTerminalIds = new Set(
(session.unifiedTabs?.[worktreeId] ?? []).flatMap((tab) =>
tab.contentType === 'terminal' ? [tab.entityId] : []
)
)
// Why: only rows that survive the id check can claim PTY ownership; a dropped invalid-id mirror must not evict the valid row sharing its PTY.
const canonicalPtyIds = new Set(
tabs
.filter((tab) => canonicalTerminalIds.has(tab.id) && isValidTerminalTabId(tab.id))
.flatMap((tab) => collectPersistedTerminalPtyIds(session, tab))
)
const quickCommandLabelByTerminalId = new Map(
(session.unifiedTabs?.[worktreeId] ?? [])
.filter((tab) => tab.contentType === 'terminal' && tab.quickCommandLabel?.trim())
.map((tab) => [tab.entityId, tab.quickCommandLabel!.trim()])
)
const aiVaultTitleByTerminalId = new Map(
(session.unifiedTabs?.[worktreeId] ?? [])
.filter((tab) => tab.contentType === 'terminal' && tab.aiVaultTitle)
.map((tab) => [tab.entityId, tab.aiVaultTitle!])
)
return [
// Why: rows for these keys came off the remote wire, which carries no unifiedTabs, so the
// session's canonical list describes a different snapshot and must not arbitrate their PTYs.
const remoteSnapshotWorkspaceKeys = new Set(
options?.directSshAuthority ? (options.replaceWorkspaceKeys ?? []) : []
)
const rowHydrationByWorktree = Object.entries(session.tabsByWorktree)
.filter(([worktreeId]) => validWorktreeIds.has(worktreeId))
.map(
([worktreeId, tabs]) =>
[
worktreeId,
[...tabs]
.filter((tab) => {
// Why: canonical mounts win PTY ownership over stale legacy duplicates.
// Why: old web-client mirrors could persist host surface ids with "::"; makePaneKey reserves ":" as its separator.
const persistedPtyIds = collectPersistedTerminalPtyIds(session, tab)
// Why: drop only rows the canonical mount fully subsumes; a split tab with one independent pane still owns a PTY nothing else can reattach.
// Why: the length guard keeps `every` from swallowing PTY-less rows, which own nothing to duplicate.
const isPureDuplicate =
persistedPtyIds.length > 0 &&
persistedPtyIds.every((ptyId) => canonicalPtyIds.has(ptyId))
return (
(canonicalTerminalIds.has(tab.id) || !isPureDuplicate) &&
isValidTerminalTabId(tab.id)
)
})
.sort((a, b) => a.sortOrder - b.sortOrder || a.createdAt - b.createdAt)
.map((tab, index) => {
const quickCommandLabel =
tab.quickCommandLabel?.trim() || quickCommandLabelByTerminalId.get(tab.id)
const aiVaultTitle = tab.aiVaultTitle ?? aiVaultTitleByTerminalId.get(tab.id)
return {
...clearTransientTerminalState(tab, index),
...(quickCommandLabel ? { quickCommandLabel } : {}),
...(aiVaultTitle ? { aiVaultTitle } : {}),
sortOrder: index,
pendingActivationSpawn: true
}
})
]
})
hydrateWorkspaceTerminalRows(session, worktreeId, tabs, {
rowsFromRemoteSnapshot: remoteSnapshotWorkspaceKeys.has(worktreeId)
})
] as const
)
const tabsByWorktree: Record<string, TerminalTab[]> = Object.fromEntries(
rowHydrationByWorktree
.map(([worktreeId, hydration]) => [worktreeId, hydration.rows] as const)
.filter(([, tabs]) => tabs.length > 0)
)
const releasedPtyIdsByTabId = new Map<string, Set<string>>(
rowHydrationByWorktree.flatMap(([, hydration]) => [...hydration.releasedPtyIdsByTabId])
)
const canonicalTabIdBySubsumedTabId = new Map<string, string>(
rowHydrationByWorktree.flatMap(([, hydration]) => [
...hydration.canonicalTabIdBySubsumedTabId
])
)
const validTabIds = new Set(
Object.values(tabsByWorktree)
.flat()
.map((tab) => tab.id)
)
const sleepingAgentSessionsByPaneKey = Object.fromEntries(
let sleepingAgentSessionsByPaneKey = Object.fromEntries(
Object.entries(session.sleepingAgentSessionsByPaneKey ?? {}).filter(([, record]) =>
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.
for (const [, hydration] of rowHydrationByWorktree) {
for (const tabId of hydration.subsumedTabIds) {
sleepingAgentSessionsByPaneKey = removeSleepingAgentSessionsForTab(
sleepingAgentSessionsByPaneKey,
tabId
)
}
}
const fallbackActiveWorktreeId =
!session.activeWorktreeId && session.activeRepoId && knownRepoIds.has(session.activeRepoId)
? (runtimeSessionPlaceholders.worktreesByRepo[session.activeRepoId]?.find(
@@ -4047,8 +4019,13 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
activeWorkspaceKey && session.activeWorkspaceExecutionHostId
? session.activeWorkspaceExecutionHostId
: null
// Why: follow a subsumed row to the canonical twin that inherited its PTY, else the app
// restarts with no active terminal even though the same session is still mounted.
const restoredActiveTabId = session.activeTabId
? (canonicalTabIdBySubsumedTabId.get(session.activeTabId) ?? session.activeTabId)
: null
const activeTabId =
session.activeTabId && validTabIds.has(session.activeTabId) ? session.activeTabId : null
restoredActiveTabId && validTabIds.has(restoredActiveTabId) ? restoredActiveTabId : null
const activeRepoId =
session.activeRepoId &&
runtimeSessionPlaceholders.repos.some((repo) => repo.id === session.activeRepoId)
@@ -4092,7 +4069,12 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
}
const rawTabs = session.tabsByWorktree[worktreeId] ?? []
for (const tab of rawTabs) {
if (tab.ptyId && validTabIds.has(tab.id)) {
// Why: a released PTY belongs to the canonical row now; reattaching it here would restore the duplicate ownership.
if (
tab.ptyId &&
validTabIds.has(tab.id) &&
!releasedPtyIdsByTabId.get(tab.id)?.has(tab.ptyId)
) {
pendingReconnectPtyIdByTabId[tab.id] = tab.ptyId
}
}
@@ -4100,7 +4082,7 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
// Why: remote PTY reattach uses the relay's pty.attach RPC, not the local daemon; the loop above skips SSH repos, so no overlap.
for (const [tabId, sessionId] of Object.entries(remoteSessionIds)) {
if (validTabIds.has(tabId)) {
if (validTabIds.has(tabId) && !releasedPtyIdsByTabId.get(tabId)?.has(sessionId)) {
pendingReconnectPtyIdByTabId[tabId] = sessionId
}
}
@@ -4109,8 +4091,14 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
let activeTabIdByWorktree: Record<string, string | null> = {}
if (session.activeTabIdByWorktree) {
for (const [wId, tabId] of Object.entries(session.activeTabIdByWorktree)) {
if (validWorktreeIds.has(wId) && tabId && validTabIds.has(tabId)) {
activeTabIdByWorktree[wId] = tabId
if (!validWorktreeIds.has(wId) || !tabId) {
continue
}
// Why: a subsumed row's canonical twin holds the same PTY, so follow the pointer there
// instead of forgetting which terminal the workspace last focused.
const restored = validTabIds.has(tabId) ? tabId : canonicalTabIdBySubsumedTabId.get(tabId)
if (restored && validTabIds.has(restored)) {
activeTabIdByWorktree[wId] = restored
}
}
} else {
@@ -4209,7 +4197,11 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
terminalLayoutsByTabId: Object.fromEntries(
Object.entries(session.terminalLayoutsByTabId)
.filter(([tabId]) => validTabIds.has(tabId))
.map(([tabId, layout]) => {
.map(([tabId, persisted]) => {
const releasedPtyIds = releasedPtyIdsByTabId.get(tabId)
const layout = releasedPtyIds
? releaseTerminalLayoutPtyIds(persisted, releasedPtyIds)
: persisted
// Why: old sessions can contain renderer-local pane:1-style leaf ids; normalize before runtime/mobile surfaces read them.
const normalization = normalizeTerminalLayoutSnapshot(layout)
const normalized = normalization.snapshot