fix(terminal): dedupe PTY ownership at hydration instead of dropping rows

Reland of #13060 (reverted by #13098).

The Grok pane restored at a stale width because a session can persist the
same PTY id on two terminal rows. Restoring both made two panes mirror one
PTY, and the hidden mirror's fit resized the shared PTY to its stale
dimensions.

#13060 fixed this by filtering non-canonical rows out of hydration, which
also deleted legacy rows that owned live panes of their own — a row whose
layout listed one shared leaf lost all its unshared leaves with it.

Instead, keep every row and assign each PTY id a single owner (canonical
unified-tab row first, then sortOrder/createdAt). Non-owners keep their row
and spawn a fresh PTY on activation rather than mirror-mounting one they
do not own.
This commit is contained in:
Jinjing
2026-08-08 14:22:05 -07:00
parent 3e6b93f0d2
commit f1cc636f05
4 changed files with 486 additions and 4 deletions
@@ -0,0 +1,70 @@
import { describe, expect, it } from 'vitest'
import { resolveHydrationPtyOwnership } from './terminal-hydration-pty-ownership'
const row = (
tabId: string,
ptyIds: string[],
overrides: {
isCanonical?: boolean
sortOrder?: number
createdAt?: number
} = {}
) => ({
tabId,
isCanonical: overrides.isCanonical ?? false,
sortOrder: overrides.sortOrder ?? 0,
createdAt: overrides.createdAt ?? 0,
ptyIds
})
describe('resolveHydrationPtyOwnership', () => {
it('gives the canonical row ownership of a PTY a stale row also claims', () => {
const ownership = resolveHydrationPtyOwnership([
row('stale', ['pty-1'], { sortOrder: 0 }),
row('canonical', ['pty-1'], { isCanonical: true, sortOrder: 9 })
])
expect(ownership.ownsPtyId('canonical', 'pty-1')).toBe(true)
expect(ownership.ownsPtyId('stale', 'pty-1')).toBe(false)
expect(ownership.ownsAnyPtyId('stale')).toBe(false)
})
it('keeps every row that owns a distinct PTY', () => {
const ownership = resolveHydrationPtyOwnership([
row('canonical', ['pty-1'], { isCanonical: true }),
row('legacy', ['pty-2'])
])
expect(ownership.ownsPtyId('legacy', 'pty-2')).toBe(true)
expect(ownership.ownsAnyPtyId('legacy')).toBe(true)
})
it('splits ownership per PTY so a partial overlap keeps the unshared leaf', () => {
const ownership = resolveHydrationPtyOwnership([
row('canonical', ['shared'], { isCanonical: true }),
row('legacy', ['shared', 'own'])
])
expect(ownership.ownsPtyId('legacy', 'shared')).toBe(false)
expect(ownership.ownsPtyId('legacy', 'own')).toBe(true)
expect(ownership.ownsAnyPtyId('legacy')).toBe(true)
})
it('falls back to sort order then createdAt when no row is canonical', () => {
const ownership = resolveHydrationPtyOwnership([
row('later', ['pty-1'], { sortOrder: 1 }),
row('earlier', ['pty-1'], { sortOrder: 0 })
])
expect(ownership.ownsPtyId('earlier', 'pty-1')).toBe(true)
expect(ownership.ownsPtyId('later', 'pty-1')).toBe(false)
})
it('resolves ties deterministically regardless of input order', () => {
const forward = resolveHydrationPtyOwnership([row('b', ['pty-1']), row('a', ['pty-1'])])
const reverse = resolveHydrationPtyOwnership([row('a', ['pty-1']), row('b', ['pty-1'])])
expect(forward.ownsPtyId('a', 'pty-1')).toBe(true)
expect(reverse.ownsPtyId('a', 'pty-1')).toBe(true)
})
})
@@ -0,0 +1,57 @@
/**
* Assigns each persisted PTY id a single owning terminal row at hydration time.
*
* Why: a session can persist the same PTY id on more than one terminal row (a
* stale row left behind by an interrupted close, a legacy row shadowing its
* canonical unified-tab row). Restoring both makes two panes mirror one PTY, and
* the hidden mirror's fit resizes the shared PTY to its stale dimensions — the
* visible pane then wraps at the wrong width. Rows still restore; only the
* duplicate PTY binding is dropped, so the non-owner spawns a fresh PTY instead.
*/
export type HydrationPtyOwnershipRow = {
tabId: string
/** Row is referenced by a unified tab, i.e. it is the one the user can see. */
isCanonical: boolean
sortOrder: number
createdAt: number
ptyIds: readonly string[]
}
export type HydrationPtyOwnership = {
ownsPtyId: (tabId: string, ptyId: string) => boolean
ownsAnyPtyId: (tabId: string) => boolean
}
function compareRows(a: HydrationPtyOwnershipRow, b: HydrationPtyOwnershipRow): number {
if (a.isCanonical !== b.isCanonical) {
return a.isCanonical ? -1 : 1
}
return (
a.sortOrder - b.sortOrder ||
a.createdAt - b.createdAt ||
(a.tabId < b.tabId ? -1 : a.tabId > b.tabId ? 1 : 0)
)
}
export function resolveHydrationPtyOwnership(
rows: readonly HydrationPtyOwnershipRow[]
): HydrationPtyOwnership {
const ownerTabIdByPtyId = new Map<string, string>()
const ownedPtyCountByTabId = new Map<string, number>()
for (const row of [...rows].sort(compareRows)) {
for (const ptyId of row.ptyIds) {
if (ownerTabIdByPtyId.has(ptyId)) {
continue
}
ownerTabIdByPtyId.set(ptyId, row.tabId)
ownedPtyCountByTabId.set(row.tabId, (ownedPtyCountByTabId.get(row.tabId) ?? 0) + 1)
}
}
return {
ownsPtyId: (tabId, ptyId) => ownerTabIdByPtyId.get(ptyId) === tabId,
ownsAnyPtyId: (tabId) => (ownedPtyCountByTabId.get(tabId) ?? 0) > 0
}
}
@@ -0,0 +1,308 @@
import { describe, expect, it, vi } from 'vitest'
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() }
const WORKTREE_ID = 'repo1::/wt-1'
function seedWorktree(store: ReturnType<typeof createTestStore>): void {
seedStore(store, {
worktreesByRepo: {
repo1: [makeWorktree({ id: WORKTREE_ID, repoId: 'repo1', path: '/wt-1' })]
}
})
}
function baseSession(overrides: Partial<WorkspaceSessionState>): WorkspaceSessionState {
return {
...getDefaultWorkspaceSession(),
activeRepoId: 'repo1',
activeWorktreeId: WORKTREE_ID,
activeWorktreeIdsOnShutdown: [WORKTREE_ID],
...overrides
}
}
function unifiedTerminalTab(entityId: string, label: string, sortOrder: number) {
return {
id: `${entityId}-unified`,
entityId,
groupId: 'group-1',
worktreeId: WORKTREE_ID,
contentType: 'terminal' as const,
label,
customLabel: null,
color: null,
sortOrder,
createdAt: sortOrder + 1
}
}
describe('hydrateWorkspaceSession duplicate PTY rows', () => {
it('gives the canonical Grok row sole ownership of a PTY a stale row also claims', () => {
const store = createTestStore()
seedWorktree(store)
const grokPtyId = 'daemon-session-grok'
store.getState().hydrateWorkspaceSession(
baseSession({
tabsByWorktree: {
[WORKTREE_ID]: [
makeTab({
id: 'stale-tab',
worktreeId: WORKTREE_ID,
ptyId: grokPtyId,
sortOrder: 0
}),
makeTab({
id: 'grok-tab',
worktreeId: WORKTREE_ID,
ptyId: grokPtyId,
sortOrder: 1
})
]
},
terminalLayoutsByTabId: {
'stale-tab': {
...makeLayout(),
ptyIdsByLeafId: { 'terminal:stale': grokPtyId }
},
'grok-tab': {
...makeLayout(),
ptyIdsByLeafId: { 'terminal:grok': grokPtyId }
}
},
unifiedTabs: {
[WORKTREE_ID]: [unifiedTerminalTab('grok-tab', 'Grok', 0)]
}
})
)
const state = store.getState()
// The stale row still restores — it just cannot mirror-mount (and resize) Grok's PTY.
expect(state.tabsByWorktree[WORKTREE_ID]?.map((tab) => tab.id)).toEqual([
'stale-tab',
'grok-tab'
])
expect(state.pendingReconnectPtyIdByTabId).toEqual({
'grok-tab': grokPtyId
})
expect(state.pendingReconnectTabByWorktree[WORKTREE_ID]).toEqual(['grok-tab'])
expect(state.terminalLayoutsByTabId['grok-tab']?.ptyIdsByLeafId).toEqual({
'terminal:grok': grokPtyId
})
expect(state.terminalLayoutsByTabId['stale-tab']?.ptyIdsByLeafId).toEqual({})
// Regression (#13098): the row must survive the next persist too, not just hydration.
expect(
buildWorkspaceSessionPayload(state).tabsByWorktree[WORKTREE_ID]?.map((t) => t.id)
).toEqual(['stale-tab', 'grok-tab'])
})
it('strips a duplicated relay session id from the non-canonical row only', () => {
const store = createTestStore()
seedWorktree(store)
const relayId = 'relay-session-grok'
store.getState().hydrateWorkspaceSession(
baseSession({
tabsByWorktree: {
[WORKTREE_ID]: [
makeTab({ id: 'stale-tab', worktreeId: WORKTREE_ID, sortOrder: 0 }),
makeTab({ id: 'grok-tab', worktreeId: WORKTREE_ID, sortOrder: 1 })
]
},
remoteSessionIdsByTabId: { 'stale-tab': relayId, 'grok-tab': relayId },
unifiedTabs: {
[WORKTREE_ID]: [unifiedTerminalTab('grok-tab', 'Grok', 0)]
}
})
)
const state = store.getState()
expect(state.tabsByWorktree[WORKTREE_ID]?.map((tab) => tab.id)).toEqual([
'stale-tab',
'grok-tab'
])
expect(state.pendingReconnectPtyIdByTabId).toEqual({ 'grok-tab': relayId })
expect(state.pendingReconnectTabByWorktree[WORKTREE_ID]).toEqual(['grok-tab'])
})
it('restores non-canonical legacy rows that own their own PTY (regression: #13098)', () => {
const store = createTestStore()
seedWorktree(store)
store.getState().hydrateWorkspaceSession(
baseSession({
tabsByWorktree: {
[WORKTREE_ID]: [
makeTab({
id: 'grok-tab',
worktreeId: WORKTREE_ID,
ptyId: 'pty-grok',
sortOrder: 0
}),
makeTab({
id: 'legacy-tab',
worktreeId: WORKTREE_ID,
ptyId: 'pty-legacy',
sortOrder: 1
})
]
},
terminalLayoutsByTabId: {
'grok-tab': {
...makeLayout(),
ptyIdsByLeafId: { 'terminal:grok': 'pty-grok' }
},
'legacy-tab': {
...makeLayout(),
ptyIdsByLeafId: { 'terminal:legacy': 'pty-legacy' }
}
},
// Only the Grok tab has a unified row; the legacy tab must survive anyway.
unifiedTabs: {
[WORKTREE_ID]: [unifiedTerminalTab('grok-tab', 'Grok', 0)]
}
})
)
const state = store.getState()
expect(state.tabsByWorktree[WORKTREE_ID]?.map((tab) => tab.id)).toEqual([
'grok-tab',
'legacy-tab'
])
expect(state.pendingReconnectTabByWorktree[WORKTREE_ID]).toEqual(['grok-tab', 'legacy-tab'])
expect(state.pendingReconnectPtyIdByTabId).toEqual({
'grok-tab': 'pty-grok',
'legacy-tab': 'pty-legacy'
})
expect(state.terminalLayoutsByTabId['legacy-tab']?.ptyIdsByLeafId).toEqual({
'terminal:legacy': 'pty-legacy'
})
})
it('restores every row of a legacy session that has no unified tabs at all', () => {
const store = createTestStore()
seedWorktree(store)
store.getState().hydrateWorkspaceSession(
baseSession({
tabsByWorktree: {
[WORKTREE_ID]: [
makeTab({
id: 'legacy-a',
worktreeId: WORKTREE_ID,
ptyId: 'pty-a',
sortOrder: 0
}),
makeTab({
id: 'legacy-b',
worktreeId: WORKTREE_ID,
ptyId: 'pty-b',
sortOrder: 1
})
]
},
terminalLayoutsByTabId: {
'legacy-a': {
...makeLayout(),
ptyIdsByLeafId: { 'terminal:a': 'pty-a' }
},
'legacy-b': {
...makeLayout(),
ptyIdsByLeafId: { 'terminal:b': 'pty-b' }
}
}
})
)
const state = store.getState()
expect(state.tabsByWorktree[WORKTREE_ID]?.map((tab) => tab.id)).toEqual([
'legacy-a',
'legacy-b'
])
expect(state.pendingReconnectPtyIdByTabId).toEqual({
'legacy-a': 'pty-a',
'legacy-b': 'pty-b'
})
expect(state.pendingReconnectTabByWorktree[WORKTREE_ID]).toEqual(['legacy-a', 'legacy-b'])
expect(state.terminalLayoutsByTabId['legacy-b']?.ptyIdsByLeafId).toEqual({
'terminal:b': 'pty-b'
})
})
it('keeps a split leaf that only the legacy row owns while dropping the shared one', () => {
const store = createTestStore()
seedWorktree(store)
const sharedPtyId = 'daemon-session-shared'
store.getState().hydrateWorkspaceSession(
baseSession({
tabsByWorktree: {
[WORKTREE_ID]: [
makeTab({
id: 'grok-tab',
worktreeId: WORKTREE_ID,
ptyId: sharedPtyId,
sortOrder: 0
}),
makeTab({
id: 'legacy-tab',
worktreeId: WORKTREE_ID,
sortOrder: 1
})
]
},
terminalLayoutsByTabId: {
'grok-tab': {
...makeLayout(),
ptyIdsByLeafId: { 'terminal:grok': sharedPtyId }
},
'legacy-tab': {
...makeLayout(),
ptyIdsByLeafId: {
'terminal:mirror': sharedPtyId,
'terminal:own': 'pty-own'
}
}
},
unifiedTabs: {
[WORKTREE_ID]: [unifiedTerminalTab('grok-tab', 'Grok', 0)]
}
})
)
const state = store.getState()
expect(state.tabsByWorktree[WORKTREE_ID]?.map((tab) => tab.id)).toEqual([
'grok-tab',
'legacy-tab'
])
expect(state.terminalLayoutsByTabId['legacy-tab']?.ptyIdsByLeafId).toEqual({
'terminal:own': 'pty-own'
})
expect(state.terminalLayoutsByTabId['grok-tab']?.ptyIdsByLeafId).toEqual({
'terminal:grok': sharedPtyId
})
})
})
+51 -4
View File
@@ -58,6 +58,7 @@ 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 { resolveHydrationPtyOwnership } from './terminal-hydration-pty-ownership'
import {
getRecentlyClosedTabPosition,
pushClosedTerminalTabSnapshot,
@@ -492,6 +493,17 @@ 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
@@ -3938,6 +3950,27 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
validWorktreeIds.add(folderWorkspaceKey(workspace.id))
}
addAdditionalValidWorkspaceKeys(validWorktreeIds, options)
// Why: PTY ids are globally unique, so resolve ownership across every restored workspace at once.
const ptyOwnership = resolveHydrationPtyOwnership(
Object.entries(session.tabsByWorktree)
.filter(([worktreeId]) => validWorktreeIds.has(worktreeId))
.flatMap(([worktreeId, tabs]) => {
const canonicalTerminalIds = new Set(
(session.unifiedTabs?.[worktreeId] ?? []).flatMap((tab) =>
tab.contentType === 'terminal' ? [tab.entityId] : []
)
)
return tabs
.filter((tab) => isValidTerminalTabId(tab.id))
.map((tab) => ({
tabId: tab.id,
isCanonical: canonicalTerminalIds.has(tab.id),
sortOrder: tab.sortOrder,
createdAt: tab.createdAt,
ptyIds: collectPersistedTerminalPtyIds(session, tab)
}))
})
)
// Why: suppress restored mounts so only real activity updates Recent.
const tabsByWorktree: Record<string, TerminalTab[]> = Object.fromEntries(
Object.entries(session.tabsByWorktree)
@@ -4039,7 +4072,13 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
for (const worktreeId of pendingReconnectWorktreeIds) {
const rawTabs = session.tabsByWorktree[worktreeId] ?? []
const liveTabIds = rawTabs
.filter((t) => (t.ptyId || remoteSessionIds[t.id]) && validTabIds.has(t.id))
.filter(
(t) =>
(t.ptyId || remoteSessionIds[t.id]) &&
validTabIds.has(t.id) &&
// Why: a row whose every PTY id belongs to another row has nothing to reattach to.
ptyOwnership.ownsAnyPtyId(t.id)
)
.map((t) => t.id)
if (liveTabIds.length > 0) {
pendingReconnectTabByWorktree[worktreeId] = liveTabIds
@@ -4060,7 +4099,7 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
}
const rawTabs = session.tabsByWorktree[worktreeId] ?? []
for (const tab of rawTabs) {
if (tab.ptyId && validTabIds.has(tab.id)) {
if (tab.ptyId && validTabIds.has(tab.id) && ptyOwnership.ownsPtyId(tab.id, tab.ptyId)) {
pendingReconnectPtyIdByTabId[tab.id] = tab.ptyId
}
}
@@ -4068,7 +4107,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) && ptyOwnership.ownsPtyId(tabId, sessionId)) {
pendingReconnectPtyIdByTabId[tabId] = sessionId
}
}
@@ -4191,7 +4230,15 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
)
}
const tab = tabById.get(tabId)
const sanitized = tab ? sanitizeTerminalLayoutPaneTitles(normalized, tab) : normalized
const titled = tab ? sanitizeTerminalLayoutPaneTitles(normalized, tab) : normalized
// Why: drop leaf bindings owned by another row so this layout cannot mirror-mount and resize a PTY it does not own.
const ownedLeafEntries = Object.entries(titled.ptyIdsByLeafId ?? {}).filter(
([, ptyId]) => (ptyId ? ptyOwnership.ownsPtyId(tabId, ptyId) : true)
)
const sanitized =
ownedLeafEntries.length === Object.keys(titled.ptyIdsByLeafId ?? {}).length
? titled
: { ...titled, ptyIdsByLeafId: Object.fromEntries(ownedLeafEntries) }
const activeLeafId = sanitized.root
? resolvePtyBoundActiveLeafId({
root: sanitized.root,