fix(session-tabs): a restored terminal row is retired only by its own host PTY (#27101)

Co-authored-by: m4air <m4air@m4airs-MacBook-Air.local>
This commit is contained in:
OrcaWin
2026-10-09 19:51:58 -07:00
committed by GitHub
co-authored by m4air
parent 0969f60c8e
commit 80992a5df7
8 changed files with 130 additions and 7 deletions
+7 -1
View File
@@ -208,9 +208,15 @@ export function buildSanitizedTabsByWorktree(
Object.entries(tabsByWorktree).map(([worktreeId, tabs]) => [
worktreeId,
tabs.map((tab) => {
const { pendingActivationSpawn: _unused, recovery: _recovery, ...rest } = tab
const {
pendingActivationSpawn: _unused,
recovery: _recovery,
restoredFromSession: _restored,
...rest
} = tab
void _unused
void _recovery
void _restored
return rest
})
])
@@ -0,0 +1,103 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import type { TerminalTab } from '../../../shared/terminal-tab-types'
import { applyWebSessionTabsSnapshot } from './web-session-tabs-sync/snapshot-api'
import type { WebSessionTabsSyncState } from './web-session-tabs-sync/state'
import {
ENV,
HOST_SURFACE_ID,
LEAF_ID,
NOW,
SECOND_LEAF_ID,
THIRD_LEAF_ID,
WT,
makeSnapshot,
makeState,
resetWebSessionTabsSyncTestState
} from './web-session-tabs-sync-test-harness'
vi.mock('../store', () => ({ useAppStore: { setState: vi.fn() } }))
/**
* #25339: rows restored from the saved session carry `pendingActivationSpawn` until their pane
* mounts. Any ready host terminal used to retire every one of them, unrelated or not.
*/
function restoredRow(id: string, sortOrder: number): TerminalTab {
return {
id,
ptyId: null,
worktreeId: WT,
title: `Restored ${id}`,
customTitle: null,
color: null,
sortOrder,
createdAt: NOW - 10,
pendingActivationSpawn: true,
restoredFromSession: true
}
}
function restoredLayout(leafId: string, ptyId: string) {
return {
root: { type: 'leaf' as const, leafId },
activeLeafId: leafId,
expandedLeafId: null,
ptyIdsByLeafId: { [leafId]: ptyId }
}
}
const unrelatedReadySurface = {
type: 'terminal' as const,
id: HOST_SURFACE_ID,
title: 'host shell',
parentTabId: 'host-tab-1',
leafId: LEAF_ID,
isActive: true,
status: 'ready' as const,
terminal: 'terminal-1'
}
describe('restored placeholders retire only by their own identity', () => {
beforeEach(resetWebSessionTabsSyncTestState)
it('keeps two restored rows when the host publishes an unrelated terminal', () => {
const first = restoredRow('restored-a', 0)
const second = restoredRow('restored-b', 1)
const patch: Partial<WebSessionTabsSyncState> = applyWebSessionTabsSnapshot(
makeState({
tabsByWorktree: { [WT]: [first, second] },
terminalLayoutsByTabId: {
[first.id]: restoredLayout(SECOND_LEAF_ID, 'remote:web-env-1@@gone-a'),
[second.id]: restoredLayout(THIRD_LEAF_ID, 'remote:web-env-1@@gone-b')
}
}),
makeSnapshot([unrelatedReadySurface]),
ENV,
NOW
)
const ids = patch.tabsByWorktree?.[WT]?.map((tab) => tab.id) ?? []
expect(ids).toContain(first.id)
expect(ids).toContain(second.id)
expect(ids).toHaveLength(3)
})
it('retires a restored row once the host publishes the PTY it was bound to', () => {
const restored = restoredRow('restored-a', 0)
const patch: Partial<WebSessionTabsSyncState> = applyWebSessionTabsSnapshot(
makeState({
tabsByWorktree: { [WT]: [restored] },
terminalLayoutsByTabId: {
[restored.id]: restoredLayout(SECOND_LEAF_ID, 'remote:web-env-1@@terminal-1')
}
}),
makeSnapshot([unrelatedReadySurface]),
ENV,
NOW
)
expect(patch.tabsByWorktree?.[WT]?.map((tab) => tab.id)).not.toContain(restored.id)
expect(patch.tabsByWorktree?.[WT]).toHaveLength(1)
})
})
@@ -145,7 +145,8 @@ export function prepareWebSessionTabsSnapshotBase(
environmentId,
nextRemotePtyIds,
nextMirroredTerminalIds,
exactProvisionalHandoffs
exactProvisionalHandoffs,
state.terminalLayoutsByTabId[tab.id]?.ptyIdsByLeafId
)
)
: currentTerminalTabs
@@ -230,7 +230,8 @@ export function shouldReplaceTerminalTab(
environmentId: string,
nextRemotePtyIds: ReadonlySet<string>,
nextMirroredTerminalIds: ReadonlySet<string>,
exactProvisionalHandoffs: ReadonlySet<string>
exactProvisionalHandoffs: ReadonlySet<string>,
persistedLeafPtyIds: Readonly<Record<string, string>> | undefined
): boolean {
if (exactProvisionalHandoffs.has(tab.id)) {
// Why: agent kind is not session identity; retire only the provisional tab
@@ -242,7 +243,13 @@ export function shouldReplaceTerminalTab(
return true
}
if (tab.pendingActivationSpawn && tab.ptyId === null && nextRemotePtyIds.size > 0) {
return true
// Why: a fresh placeholder has no identity, so the host's first terminal takes it over. A
// restored row names its own PTYs, and only those may retire it (#25339).
return (
!tab.restoredFromSession ||
nextMirroredTerminalIds.has(toWebTerminalSurfaceTabId(tab.id)) ||
Object.values(persistedLeafPtyIds ?? {}).some((ptyId) => nextRemotePtyIds.has(ptyId))
)
}
if (!isRuntimeTerminalTabForEnvironment(tab, environmentId)) {
return false
@@ -209,7 +209,8 @@ function restoreCanonicalMetadata(
...(aiVaultTitle ? { aiVaultTitle } : {}),
sortOrder: index,
// Why: suppress restored mounts so only real activity updates Recent.
pendingActivationSpawn: true
pendingActivationSpawn: true,
restoredFromSession: true
}
}
@@ -100,8 +100,9 @@ export function createTerminalPtyBindingActions(
wasActivationSpawn = true
}
// Why: consume one suppression per split-pane activation callback.
const { pendingActivationSpawn: _unused, ...rest } = tab
const { pendingActivationSpawn: _unused, restoredFromSession: _restored, ...rest } = tab
void _unused
void _restored
// Why: tab.ptyId is the single-pane fallback for legacy attach; later split-pane spawns must not steal it or remount/close reattaches the tab to the wrong PTY.
const currentTabPtyId = tab.ptyId === replacementPtyId ? ptyId : tab.ptyId
const nextTabPtyId = currentTabPtyId ?? nextPtyIds[0] ?? null
+3
View File
@@ -114,6 +114,9 @@ export type TerminalTab = {
* `sortEpoch` increments. Split layouts use a numeric count because one tab
* can remount several panes. Never persisted — it is a transient handoff. */
pendingActivationSpawn?: boolean | number
/** Hydrated from the saved session, so its own persisted PTY bindings identify it; a host
* surface may retire it only by that identity. Never persisted; dropped once a PTY binds. */
restoredFromSession?: true
/** Transient recovery ledger for this tab. Never persisted — it describes a
* mounted pane's in-flight heal, and a stale one would refuse the first
* legitimate recovery after restart. Stripped exactly like
@@ -126,7 +126,8 @@ const ROW = {
// Transient handoffs, never restored; the window strips them, but main's minimal row mint
// stores `pendingActivationSpawn`, so the Loader drops it.
pendingActivationSpawn: 'unwritten',
recovery: 'unwritten'
recovery: 'unwritten',
restoredFromSession: 'unwritten'
} as const satisfies Record<keyof TerminalTab, DiskFieldWritten>
const ENTRY_FIELDS = {