diff --git a/src/renderer/src/lib/workspace-session.ts b/src/renderer/src/lib/workspace-session.ts index 4bf57f6d82f..6fb3509d138 100644 --- a/src/renderer/src/lib/workspace-session.ts +++ b/src/renderer/src/lib/workspace-session.ts @@ -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 }) ]) diff --git a/src/renderer/src/runtime/web-session-restored-placeholder-identity.test.ts b/src/renderer/src/runtime/web-session-restored-placeholder-identity.test.ts new file mode 100644 index 00000000000..82d2308c8ee --- /dev/null +++ b/src/renderer/src/runtime/web-session-restored-placeholder-identity.test.ts @@ -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 = 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 = 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) + }) +}) diff --git a/src/renderer/src/runtime/web-session-tabs-sync/apply-preparation-base.ts b/src/renderer/src/runtime/web-session-tabs-sync/apply-preparation-base.ts index 40638148759..1f4c88bd445 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/apply-preparation-base.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync/apply-preparation-base.ts @@ -145,7 +145,8 @@ export function prepareWebSessionTabsSnapshotBase( environmentId, nextRemotePtyIds, nextMirroredTerminalIds, - exactProvisionalHandoffs + exactProvisionalHandoffs, + state.terminalLayoutsByTabId[tab.id]?.ptyIdsByLeafId ) ) : currentTerminalTabs diff --git a/src/renderer/src/runtime/web-session-tabs-sync/terminal-surfaces.ts b/src/renderer/src/runtime/web-session-tabs-sync/terminal-surfaces.ts index 65e01f513b7..fd796a455b8 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/terminal-surfaces.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync/terminal-surfaces.ts @@ -230,7 +230,8 @@ export function shouldReplaceTerminalTab( environmentId: string, nextRemotePtyIds: ReadonlySet, nextMirroredTerminalIds: ReadonlySet, - exactProvisionalHandoffs: ReadonlySet + exactProvisionalHandoffs: ReadonlySet, + persistedLeafPtyIds: Readonly> | 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 diff --git a/src/renderer/src/store/slices/terminal-session-row-hydration.ts b/src/renderer/src/store/slices/terminal-session-row-hydration.ts index eda4f369725..ea61dfe8ae5 100644 --- a/src/renderer/src/store/slices/terminal-session-row-hydration.ts +++ b/src/renderer/src/store/slices/terminal-session-row-hydration.ts @@ -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 } } diff --git a/src/renderer/src/store/terminals/terminal-pty-bindings.ts b/src/renderer/src/store/terminals/terminal-pty-bindings.ts index f78c8019e74..d7f39c70ac5 100644 --- a/src/renderer/src/store/terminals/terminal-pty-bindings.ts +++ b/src/renderer/src/store/terminals/terminal-pty-bindings.ts @@ -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 diff --git a/src/shared/terminal-tab-types.ts b/src/shared/terminal-tab-types.ts index 689aee49546..bcfcb2f7a16 100644 --- a/src/shared/terminal-tab-types.ts +++ b/src/shared/terminal-tab-types.ts @@ -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 diff --git a/src/shared/workspace-layout/workspace-layout-disk-fields.ts b/src/shared/workspace-layout/workspace-layout-disk-fields.ts index 30f47612c39..ece0eab9ef7 100644 --- a/src/shared/workspace-layout/workspace-layout-disk-fields.ts +++ b/src/shared/workspace-layout/workspace-layout-disk-fields.ts @@ -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 const ENTRY_FIELDS = {