From 2b111ec055baa3dbc75535eb0ccc7910d504be4d Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Sun, 20 Sep 2026 17:15:18 -0400 Subject: [PATCH] fix(terminal): never mount one PTY in two tabs A terminal reveal minted a second tab for a live PTY, reusing the event's leaf id and the spawn-time tab id, whenever the owning tab sat outside the event worktree's tab list: ownership only scanned that list while layouts and mounted-pty maps are keyed by tab id. Two panes then shared one PTY, the single-slot data handler starved one of them, and both forwarded their grid, so an OpenCode alt-screen TUI shredded and resize could not heal it. - ownership scans every tab with a row; a bound leaf id names its pane; ambiguity adopts an owner instead of minting; a rowless owner fails the reveal with a reported error rather than a phantom activation - hydration gives each duplicated leaf id and pty id one owning tab and withholds the losing row's pty from the reconnect plan - a second data-handler registration for one pty records a breadcrumb STA-7961 --- ...ty-session-duplicate-pane-delivery.test.ts | 92 ++++++ .../terminal-pane/ipc-pty-session-handlers.ts | 2 + .../pty-data-handler-overwrite-breadcrumb.ts | 21 ++ .../terminal-presentation-ipc-bridge.ts | 67 ++-- ...obile-terminal-reveal-tab-adoption.test.ts | 9 +- ...erminal-reveal-duplicate-leaf-mint.test.ts | 101 +++++++ ...minal-reveal-resurrects-closed-tab.test.ts | 286 ++++++++++++++++++ .../src/lib/terminal-reveal-tab-adoption.ts | 99 ++++++ .../src/lib/terminal-tab-for-pty-id.test.ts | 14 + .../src/lib/terminal-tab-for-pty-id.ts | 114 +++++-- ...rminal-duplicate-binding-reconnect.test.ts | 130 ++++++++ .../terminals/workspace-terminal-hydration.ts | 45 +-- ...terminal-layout-duplicate-bindings.test.ts | 193 ++++++++++++ ...pace-terminal-layout-duplicate-bindings.ts | 193 ++++++++++++ ...erminal-layout-plan-duplicate-leaf.test.ts | 108 +++++++ .../workspace-terminal-layout-plan.ts | 45 ++- 16 files changed, 1450 insertions(+), 69 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/ipc-pty-session-duplicate-pane-delivery.test.ts create mode 100644 src/renderer/src/components/terminal-pane/pty-data-handler-overwrite-breadcrumb.ts create mode 100644 src/renderer/src/hooks/terminal-reveal-duplicate-leaf-mint.test.ts create mode 100644 src/renderer/src/hooks/terminal-reveal-resurrects-closed-tab.test.ts create mode 100644 src/renderer/src/lib/terminal-reveal-tab-adoption.ts create mode 100644 src/renderer/src/store/terminals/workspace-terminal-duplicate-binding-reconnect.test.ts create mode 100644 src/renderer/src/store/terminals/workspace-terminal-layout-duplicate-bindings.test.ts create mode 100644 src/renderer/src/store/terminals/workspace-terminal-layout-duplicate-bindings.ts create mode 100644 src/renderer/src/store/terminals/workspace-terminal-layout-plan-duplicate-leaf.test.ts diff --git a/src/renderer/src/components/terminal-pane/ipc-pty-session-duplicate-pane-delivery.test.ts b/src/renderer/src/components/terminal-pane/ipc-pty-session-duplicate-pane-delivery.test.ts new file mode 100644 index 00000000000..b54aeda8498 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/ipc-pty-session-duplicate-pane-delivery.test.ts @@ -0,0 +1,92 @@ +// Two panes can end up bound to one ptyId when a leaf id is duplicated across tabs (STA-7961). +// Delivery is a single slot per ptyId, so the second mount evicts the first and one of the two +// panes paints nothing the shell sent. Hydration heals the duplicate; this records the ones that +// still reach the renderer, so a field crash bundle names the PTY. +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const recordRendererCrashBreadcrumb = vi.fn() +vi.mock('@/lib/crash-breadcrumb-recorder', () => ({ + recordRendererCrashBreadcrumb: (name: string, data?: unknown) => + recordRendererCrashBreadcrumb(name, data) +})) + +const PTY_ID = 'wt-1@@289ed0f2' + +describe('duplicate pane pty data delivery', () => { + let warn: ReturnType + + beforeEach(() => { + vi.resetModules() + recordRendererCrashBreadcrumb.mockReset() + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + }) + + afterEach(() => { + warn.mockRestore() + }) + + async function mountPane(): Promise<{ + registerData: (id: string) => void + unregisterData: (id: string) => void + }> { + const { createIpcPtySessionHandlers } = await import('./ipc-pty-session-handlers') + const { createPtyOutputProcessor } = await import('./pty-output-processor') + return createIpcPtySessionHandlers({ + outputProcessor: createPtyOutputProcessor({}), + getPtyId: () => PTY_ID, + getCallbacks: () => ({ onData: vi.fn() }), + getSuppressAttentionEvents: () => false, + markExited: vi.fn() + }) + } + + it('stays silent when the first pane claims the pty', async () => { + const pane = await mountPane() + + pane.registerData(PTY_ID) + + expect(recordRendererCrashBreadcrumb).not.toHaveBeenCalled() + expect(warn).not.toHaveBeenCalled() + }) + + it('reports the ptyId when a second pane overwrites the live handler', async () => { + const first = await mountPane() + const second = await mountPane() + + first.registerData(PTY_ID) + second.registerData(PTY_ID) + + expect(recordRendererCrashBreadcrumb).toHaveBeenCalledWith( + 'terminal_pty_data_handler_overwritten', + { ptyId: PTY_ID } + ) + expect(warn).toHaveBeenCalledTimes(1) + }) + + it('stays silent when a pane re-registers after releasing the pty', async () => { + const pane = await mountPane() + + pane.registerData(PTY_ID) + pane.unregisterData(PTY_ID) + pane.registerData(PTY_ID) + + expect(recordRendererCrashBreadcrumb).not.toHaveBeenCalled() + expect(warn).not.toHaveBeenCalled() + }) + + it('stays silent while a shutdown of the same pty is still pending', async () => { + const { unregisterPtyDataHandlers } = await import('./pty-shutdown-data-suspension') + const outgoing = await mountPane() + const incoming = await mountPane() + + outgoing.registerData(PTY_ID) + const snapshots = unregisterPtyDataHandlers([PTY_ID]) + incoming.registerData(PTY_ID) + + expect(recordRendererCrashBreadcrumb).not.toHaveBeenCalled() + expect(warn).not.toHaveBeenCalled() + for (const snapshot of snapshots) { + snapshot.commit() + } + }) +}) diff --git a/src/renderer/src/components/terminal-pane/ipc-pty-session-handlers.ts b/src/renderer/src/components/terminal-pane/ipc-pty-session-handlers.ts index ad225b928bb..a921aa38e1f 100644 --- a/src/renderer/src/components/terminal-pane/ipc-pty-session-handlers.ts +++ b/src/renderer/src/components/terminal-pane/ipc-pty-session-handlers.ts @@ -15,6 +15,7 @@ import { hasPreHandlerPtyExit } from './pty-pre-handler-buffer' import type { createPtyOutputProcessor } from './pty-output-processor' +import { reportOverwrittenPtyDataHandler } from './pty-data-handler-overwrite-breadcrumb' import type { IpcPtyTransportOptions, PtyTransport } from './pty-transport-types' type PtyCallbacks = Parameters[0]['callbacks'] @@ -124,6 +125,7 @@ export function createIpcPtySessionHandlers({ } } ptyReplayHandlers.set(id, replay) + reportOverwrittenPtyDataHandler(id, data) ptyDataHandlers.set(id, data) ptyWriteUnavailableHandlers.set(id, writeUnavailable) ownedDataHandlers.set(id, { data, replay, writeUnavailable }) diff --git a/src/renderer/src/components/terminal-pane/pty-data-handler-overwrite-breadcrumb.ts b/src/renderer/src/components/terminal-pane/pty-data-handler-overwrite-breadcrumb.ts new file mode 100644 index 00000000000..daf084ef75b --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-data-handler-overwrite-breadcrumb.ts @@ -0,0 +1,21 @@ +import { recordRendererCrashBreadcrumb } from '@/lib/crash-breadcrumb-recorder' +import { isPtyDataHandlerShutdownPending, ptyDataHandlers } from './pty-shutdown-data-suspension' + +type PtyDataHandler = NonNullable> + +/** + * Report a second pane claiming a PTY's only data-handler slot. + * + * Why not fan the data out instead: two panes on one PTY is the STA-7961 bug, not a mode to + * support — they also both forward their fit, so the PTY grid flips between two sizes. The one + * legitimate overlap, a remount, runs through the pending-shutdown queue, which deliberately + * leaves the outgoing handler in the map; that case is silent. + */ +export function reportOverwrittenPtyDataHandler(ptyId: string, next: PtyDataHandler): void { + const previous = ptyDataHandlers.get(ptyId) + if (!previous || previous === next || isPtyDataHandlerShutdownPending(ptyId)) { + return + } + console.warn('[pty] a second pane replaced the data handler for', ptyId) + recordRendererCrashBreadcrumb('terminal_pty_data_handler_overwritten', { ptyId }) +} diff --git a/src/renderer/src/hooks/ipc-events/terminal-presentation-ipc-bridge.ts b/src/renderer/src/hooks/ipc-events/terminal-presentation-ipc-bridge.ts index 47835ab529d..372c324d04c 100644 --- a/src/renderer/src/hooks/ipc-events/terminal-presentation-ipc-bridge.ts +++ b/src/renderer/src/hooks/ipc-events/terminal-presentation-ipc-bridge.ts @@ -1,7 +1,10 @@ import { requestBackgroundTerminalWorktreeMount } from '@/components/terminal/background-terminal-worktree-mount' import { hasRegisteredRuntimeTerminalTab } from '@/runtime/sync-runtime-graph' import { planMobileTerminalTabMount } from '@/lib/mobile-terminal-tab-mount' -import { resolveTerminalTabPtyOwnership } from '@/lib/terminal-tab-for-pty-id' +import { + findTerminalTabRow, + resolveTerminalRevealTabAdoption +} from '@/lib/terminal-reveal-tab-adoption' import { SPLIT_TERMINAL_PANE_EVENT } from '@/constants/terminal' import type { SplitTerminalPaneDetail } from '@/constants/terminal' import { singlePaneLayoutSnapshot } from '@/store/slices/terminal-helpers' @@ -54,24 +57,34 @@ export function registerTerminalPresentationIpcBridge(unsubs: (() => void)[]): v }) const shouldActivate = terminalPresentation === 'focused' const shouldSurfaceOwner = terminalPresentation !== 'background' && surfaceOwner !== false - if (shouldActivate) { - activateTerminalInitiatedWorktree(store, worktreeId) - } - const worktreeTabs = store.tabsByWorktree[worktreeId] ?? [] // Why: a split pane revealed from mobile is only bound in the persisted // layout until its pane mounts; missing it minted a duplicate tab (#10486). - const ownership = ptyId - ? resolveTerminalTabPtyOwnership( - store, + const adoption = ptyId + ? resolveTerminalRevealTabAdoption(store, { worktreeId, ptyId, - tabId !== undefined ? { preferTabId: tabId } : {} - ) - : { kind: 'none' as const } - const existingTab = - ownership.kind === 'owned' - ? worktreeTabs.find((candidate) => candidate.id === ownership.tabId) - : undefined + ...(leafId !== undefined ? { leafId } : {}), + ...(tabId !== undefined ? { hintTabId: tabId } : {}) + }) + : ({ kind: 'mint', verdict: 'none' } as const) + const adoptedRow = + adoption.kind === 'adopt' ? findTerminalTabRow(store, adoption.tabId) : null + if (adoption.kind === 'adopt' && adoptedRow === null) { + // Why: the owner's row is gone, so there is no pane to activate, and + // minting one would re-bind a leaf id its orphan layout still holds. + console.warn( + `[onCreateTerminal] rowless-owner ${adoption.tabId} for ptyId ${ptyId}; reveal failed` + ) + throw new Error(`terminal_reveal_owner_row_missing: tab ${adoption.tabId}`) + } + // Why: a layout outlives its row's membership in any one worktree list, + // so the owner's key, not the event's, is the one to surface (STA-7961). + const ownerWorktreeId = adoptedRow?.worktreeId ?? worktreeId + if (shouldActivate) { + activateTerminalInitiatedWorktree(store, ownerWorktreeId) + } + const worktreeTabs = store.tabsByWorktree[worktreeId] ?? [] + const existingTab = adoptedRow?.tab const isSplitReveal = Boolean(ptyId && tabId && leafId && splitFromLeafId) const splitTargetTab = isSplitReveal ? worktreeTabs.find((candidate) => candidate.id === tabId) @@ -80,7 +93,16 @@ export function registerTerminalPresentationIpcBridge(unsubs: (() => void)[]): v throw new Error(`Terminal tab ${tabId} not found`) } const reusedTab = existingTab ?? splitTargetTab - const tab = + if (ptyId && !reusedTab) { + // Why: the next field report needs to say which binding was missing + // when a reveal minted a second tab for an existing session. + const verdict = adoption.kind === 'mint' ? adoption.verdict : adoption.kind + console.warn( + `[onCreateTerminal] minting a tab for ptyId ${ptyId} (ownership ${verdict});` + + ` tabId hint ${tabId ?? 'none'}, leafId ${leafId ?? 'none'}` + ) + } + const tab: { id: string; title?: string } = reusedTab ?? (ptyId ? store.createTab(worktreeId, undefined, undefined, { @@ -130,8 +152,8 @@ export function registerTerminalPresentationIpcBridge(unsubs: (() => void)[]): v store.setActiveTab(tab.id) } if (shouldSurfaceOwner) { - store.revealWorktreeInSidebar(worktreeId) - focusTerminalInitiatedTab(tab.id, leafId, worktreeId) + store.revealWorktreeInSidebar(ownerWorktreeId) + focusTerminalInitiatedTab(tab.id, leafId, ownerWorktreeId) } // Why: only stamp the runtime title on fresh tabs; reused tabs may have a user customTitle it would overwrite on focus. if (title && !reusedTab) { @@ -172,7 +194,7 @@ export function registerTerminalPresentationIpcBridge(unsubs: (() => void)[]): v new CustomEvent(SPLIT_TERMINAL_PANE_EVENT, { detail: { tabId: tab.id, - worktreeId, + worktreeId: ownerWorktreeId, paneRuntimeId: -1, direction: splitDirection ?? 'horizontal', sourceLeafId: splitFromLeafId, @@ -212,14 +234,17 @@ export function registerTerminalPresentationIpcBridge(unsubs: (() => void)[]): v }) } if (ptyId && terminalPresentation === 'background') { - requestBackgroundTerminalWorktreeMount({ worktreeId, tabIds: [tab.id] }) + requestBackgroundTerminalWorktreeMount({ + worktreeId: ownerWorktreeId, + tabIds: [tab.id] + }) } if (requestId) { // Why: attest the actual binding; recovery callers compare it with their expected identity. const identity = ptyId && tabId && leafId ? verifyTerminalRevealIdentity(useAppStore.getState(), { - worktreeId, + worktreeId: ownerWorktreeId, tabId: tab.id, leafId, ptyId diff --git a/src/renderer/src/hooks/mobile-terminal-reveal-tab-adoption.test.ts b/src/renderer/src/hooks/mobile-terminal-reveal-tab-adoption.test.ts index 4f5afa434fd..8fd941fc2e6 100644 --- a/src/renderer/src/hooks/mobile-terminal-reveal-tab-adoption.test.ts +++ b/src/renderer/src/hooks/mobile-terminal-reveal-tab-adoption.test.ts @@ -211,9 +211,10 @@ describe('mobile terminal reveal tab adoption', () => { }) }) - it('replies without an error when two recorded bindings both claim the pty', async () => { - // Ambiguity is unresolvable, so the reveal still creates a tab — but it must - // not reject, because the mobile focus path awaits it with no catch. + it('adopts a recorded owner without rejecting when two bindings both claim the pty', async () => { + // Ambiguity adopts a recorded owner instead of minting a second mount + // (STA-7961); the reply still must not reject, because the mobile focus + // path awaits it with no catch. const storeState: HarnessStoreState = createHarnessStoreState({ tabsByWorktree: { [WORKTREE_ID]: [ @@ -238,7 +239,7 @@ describe('mobile terminal reveal tab adoption', () => { expect(harness.replyTerminalCreate).toHaveBeenCalledWith({ requestId: 'mobile-reveal', - tabId: 'tab-minted', + tabId: 'tab-stale-a', title: 'codex' }) }) diff --git a/src/renderer/src/hooks/terminal-reveal-duplicate-leaf-mint.test.ts b/src/renderer/src/hooks/terminal-reveal-duplicate-leaf-mint.test.ts new file mode 100644 index 00000000000..6f7cdcedcba --- /dev/null +++ b/src/renderer/src/hooks/terminal-reveal-duplicate-leaf-mint.test.ts @@ -0,0 +1,101 @@ +import { describe, expect, it } from 'vitest' +import { + createHarnessStoreState, + loadIpcEventsHarness, + type HarnessStoreState +} from './ipc-events-test-harness' + +const WORKTREE_ID = 'wt-1' +const SHARED_LEAF_ID = 'leaf-shared' +const SHARED_PTY_ID = 'pty-shared' + +/** Tabs whose persisted layout binds SHARED_LEAF_ID, whatever pty it points at. */ +function tabsBindingSharedLeaf(state: HarnessStoreState): string[] { + return Object.entries(state.terminalLayoutsByTabId) + .filter(([, layout]) => SHARED_LEAF_ID in (layout.ptyIdsByLeafId ?? {})) + .map(([tabId]) => tabId) +} + +function revealUnownedPty(harness: { + createTerminal: (request: { + requestId?: string + worktreeId: string + ptyId?: string + leafId?: string + presentation?: 'background' | 'focused' + title?: string + }) => void +}): void { + harness.createTerminal({ + requestId: 'reveal', + worktreeId: WORKTREE_ID, + ptyId: SHARED_PTY_ID, + leafId: SHARED_LEAF_ID, + presentation: 'focused', + title: 'OpenCode' + }) +} + +describe('terminal reveal must not mint a tab that reuses a bound leaf id (STA-7961)', () => { + it('leaves the leaf id bound to one tab when the owning row is filed under another workspace key', async () => { + // terminalLayoutsByTabId is keyed by tab id alone, so it still records + // tab-a's split leaf while the reveal's worktree key lists other rows. + const storeState: HarnessStoreState = createHarnessStoreState({ + tabsByWorktree: { + [WORKTREE_ID]: [{ id: 'tab-other', ptyId: 'pty-other', title: 'Terminal 3' }], + 'wt-other': [{ id: 'tab-a', ptyId: 'pty-a', title: 'Terminal 1' }] + }, + ptyIdsByTabId: {}, + terminalLayoutsByTabId: { + 'tab-a': { ptyIdsByLeafId: { 'leaf-a': 'pty-a', [SHARED_LEAF_ID]: SHARED_PTY_ID } } + } + }) + const harness = await loadIpcEventsHarness(storeState) + harness.useIpcEvents() + + revealUnownedPty(harness) + + expect(tabsBindingSharedLeaf(storeState)).toEqual(['tab-a']) + }) + + it('refuses to bind a leaf id a sibling tab in the same worktree already owns', async () => { + // The pty is unowned, so ownership resolves to none; the leaf id is not, + // and re-minting it hands two tabs the same pane identity. + const storeState: HarnessStoreState = createHarnessStoreState({ + tabsByWorktree: { + [WORKTREE_ID]: [{ id: 'tab-a', ptyId: 'pty-a', title: 'Terminal 1' }] + }, + ptyIdsByTabId: {}, + terminalLayoutsByTabId: { + 'tab-a': { ptyIdsByLeafId: { 'leaf-a': 'pty-a', [SHARED_LEAF_ID]: 'pty-stale' } } + } + }) + const harness = await loadIpcEventsHarness(storeState) + harness.useIpcEvents() + + revealUnownedPty(harness) + + expect(tabsBindingSharedLeaf(storeState)).toEqual(['tab-a']) + }) + + it('adopts the recorded owner instead of minting when the layout binds the pty but no pane is mounted', async () => { + // Control for the hibernated-pane hypothesis: a recorded layout row alone + // is enough for the resolver, so this path never reaches the mint branch. + const storeState: HarnessStoreState = createHarnessStoreState({ + tabsByWorktree: { + [WORKTREE_ID]: [{ id: 'tab-a', ptyId: 'pty-a', title: 'Terminal 1' }] + }, + ptyIdsByTabId: { 'tab-a': [] }, + terminalLayoutsByTabId: { + 'tab-a': { ptyIdsByLeafId: { 'leaf-a': 'pty-a', [SHARED_LEAF_ID]: SHARED_PTY_ID } } + } + }) + const harness = await loadIpcEventsHarness(storeState) + harness.useIpcEvents() + + revealUnownedPty(harness) + + expect(storeState.createTab).not.toHaveBeenCalled() + expect(tabsBindingSharedLeaf(storeState)).toEqual(['tab-a']) + }) +}) diff --git a/src/renderer/src/hooks/terminal-reveal-resurrects-closed-tab.test.ts b/src/renderer/src/hooks/terminal-reveal-resurrects-closed-tab.test.ts new file mode 100644 index 00000000000..ea54e2856ac --- /dev/null +++ b/src/renderer/src/hooks/terminal-reveal-resurrects-closed-tab.test.ts @@ -0,0 +1,286 @@ +// A pane moved out of its spawn tab keeps its leaf id, but the PTY's env keeps +// the tab id it was minted against. When that spawn tab is later closed, a +// reveal carrying the stale tab id re-mints it (STA-7961). +import { describe, expect, it, vi } from 'vitest' +import { addSplitLeafToLayout } from './ipc-events/terminal-command-state' +import { + createHarnessStoreState, + loadIpcEventsHarness, + type HarnessStoreState +} from './ipc-events-test-harness' +import { detachTerminalLayoutLeaf } from '@/components/terminal-pane/terminal-layout-leaf-detach' +import { + createTestStore, + makeTab, + makeWorktree, + seedStore +} from '@/store/slices/store-test-helpers' +import { resolveTerminalTabPtyOwnership } from '@/lib/terminal-tab-for-pty-id' +import type { AppState } from '@/store/types' + +vi.mock('sonner', () => ({ + toast: { info: vi.fn(), success: vi.fn(), error: vi.fn(), warning: vi.fn() } +})) +vi.mock('@/components/terminal-pane/pty-dispatcher', () => ({ + restorePtyDataHandlersAfterFailedShutdown: vi.fn(), + unregisterPtyDataHandlers: vi.fn<() => unknown[]>(() => []) +})) + +const WORKTREE_ID = 'repo1::/repo1' +const SPAWN_TAB_ID = 'tab-x' +const HOST_TAB_ID = 'tab-a' +const MOVED_LEAF_ID = '10cb5648-8a54-41c0-a6a4-ef0028d93599' +const SIBLING_LEAF_ID = '08d0d524-0d46-410b-ba08-b43c97a2b4e4' +const HOST_LEAF_ID = 'df8913c9-fd8a-420a-a7d6-17daf0ed30f0' +const MOVED_PTY_ID = 'repo1::/repo1@@289ed0f2' + +type MovedPaneState = Pick< + AppState, + 'tabsByWorktree' | 'ptyIdsByTabId' | 'terminalLayoutsByTabId' +> & { spawnTabPtyIdAfterMove: string | null | undefined } + +/** The harness models a layout as an optional root; a rootless snapshot has none. */ +function toHarnessLayouts( + layouts: AppState['terminalLayoutsByTabId'] +): HarnessStoreState['terminalLayoutsByTabId'] { + return Object.fromEntries( + Object.entries(layouts).map(([tabId, layout]) => [ + tabId, + { + ...(layout.root ? { root: layout.root } : {}), + ...(layout.ptyIdsByLeafId ? { ptyIdsByLeafId: layout.ptyIdsByLeafId } : {}) + } + ]) + ) +} + +/** + * Runs the real move and close through the store: the pane leaves tab X for + * tab A's split, then the emptied tab X is closed. + */ +function moveThenCloseSpawnTab(): MovedPaneState { + const store = createTestStore() + seedStore(store, { + worktreesByRepo: { + repo1: [makeWorktree({ id: WORKTREE_ID, repoId: 'repo1', path: '/repo1' })] + }, + tabsByWorktree: { + [WORKTREE_ID]: [ + makeTab({ id: SPAWN_TAB_ID, worktreeId: WORKTREE_ID, ptyId: MOVED_PTY_ID }), + makeTab({ id: HOST_TAB_ID, worktreeId: WORKTREE_ID, ptyId: 'pty-host', sortOrder: 1 }) + ] + }, + ptyIdsByTabId: { + [SPAWN_TAB_ID]: [MOVED_PTY_ID, 'pty-sibling'], + [HOST_TAB_ID]: ['pty-host'] + } + }) + store.getState().setTabLayout(SPAWN_TAB_ID, { + root: { + type: 'split', + direction: 'horizontal', + first: { type: 'leaf', leafId: MOVED_LEAF_ID }, + second: { type: 'leaf', leafId: SIBLING_LEAF_ID } + }, + activeLeafId: MOVED_LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { [MOVED_LEAF_ID]: MOVED_PTY_ID, [SIBLING_LEAF_ID]: 'pty-sibling' } + }) + store.getState().setTabLayout(HOST_TAB_ID, { + root: { type: 'leaf', leafId: HOST_LEAF_ID }, + activeLeafId: HOST_LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { [HOST_LEAF_ID]: 'pty-host' } + }) + + const detached = detachTerminalLayoutLeaf( + store.getState().terminalLayoutsByTabId[SPAWN_TAB_ID], + MOVED_LEAF_ID + ) + if (!detached) { + throw new Error('Expected the pane to detach from its spawn tab') + } + store.getState().setTabLayout(SPAWN_TAB_ID, detached.sourceLayout) + store + .getState() + .setTabLayout( + HOST_TAB_ID, + addSplitLeafToLayout( + store.getState().terminalLayoutsByTabId[HOST_TAB_ID], + HOST_LEAF_ID, + MOVED_LEAF_ID, + MOVED_PTY_ID, + 'horizontal', + 'OpenCode', + true + ) + ) + store.getState().syncPaneDetachPtyOwnership({ + detachedLeafId: MOVED_LEAF_ID, + detachedPtyId: MOVED_PTY_ID, + sourceLayout: detached.sourceLayout, + sourceTabId: SPAWN_TAB_ID, + targetTabId: HOST_TAB_ID + }) + + const spawnTabPtyIdAfterMove = (store.getState().tabsByWorktree[WORKTREE_ID] ?? []).find( + (tab) => tab.id === SPAWN_TAB_ID + )?.ptyId + + store.getState().closeTab(SPAWN_TAB_ID, { reason: 'user' }) + + const after = store.getState() + return { + tabsByWorktree: after.tabsByWorktree, + ptyIdsByTabId: after.ptyIdsByTabId, + terminalLayoutsByTabId: after.terminalLayoutsByTabId, + spawnTabPtyIdAfterMove + } +} + +function tabsBindingMovedLeaf(state: HarnessStoreState): string[] { + return Object.entries(state.terminalLayoutsByTabId) + .filter(([, layout]) => MOVED_LEAF_ID in (layout.ptyIdsByLeafId ?? {})) + .map(([tabId]) => tabId) +} + +describe('terminal reveal against a closed spawn tab (STA-7961)', () => { + it('drops the spawn tab pty id when the pane moves to another tab', () => { + // Recorded for the report: the move rewrites tab X's own ptyId. + expect(moveThenCloseSpawnTab().spawnTabPtyIdAfterMove).toBe('pty-sibling') + }) + + it('resolves the pty to the tab the pane moved into, not the stale hint', () => { + const moved = moveThenCloseSpawnTab() + expect( + resolveTerminalTabPtyOwnership( + { + tabsByWorktree: moved.tabsByWorktree, + terminalLayoutsByTabId: moved.terminalLayoutsByTabId, + ptyIdsByTabId: moved.ptyIdsByTabId + }, + WORKTREE_ID, + MOVED_PTY_ID, + { preferTabId: SPAWN_TAB_ID } + ) + ).toEqual({ kind: 'owned', tabId: HOST_TAB_ID }) + }) + + it('does not resurrect the closed spawn tab or duplicate its leaf id', async () => { + const moved = moveThenCloseSpawnTab() + const storeState: HarnessStoreState = createHarnessStoreState({ + tabsByWorktree: moved.tabsByWorktree, + ptyIdsByTabId: moved.ptyIdsByTabId, + terminalLayoutsByTabId: toHarnessLayouts(moved.terminalLayoutsByTabId), + createTab: vi.fn((_worktreeId, _groupId, _shell, options) => ({ + id: options?.id ?? 'tab-minted' + })) + }) + const harness = await loadIpcEventsHarness(storeState) + harness.useIpcEvents() + + harness.createTerminal({ + requestId: 'reveal', + worktreeId: WORKTREE_ID, + ptyId: MOVED_PTY_ID, + leafId: MOVED_LEAF_ID, + tabId: SPAWN_TAB_ID, + presentation: 'focused', + title: 'OpenCode' + }) + + expect(Object.keys(storeState.terminalLayoutsByTabId)).not.toContain(SPAWN_TAB_ID) + expect(tabsBindingMovedLeaf(storeState)).toEqual([HOST_TAB_ID]) + }) + + it('still adopts the host tab when the reveal beats layout hydration', async () => { + // Nothing is hydrated except the tab rows, so the host tab's own ptyId is + // the last binding standing. It is enough, and the stale hint loses. + const moved = moveThenCloseSpawnTab() + const storeState: HarnessStoreState = createHarnessStoreState({ + tabsByWorktree: moved.tabsByWorktree, + ptyIdsByTabId: {}, + terminalLayoutsByTabId: {}, + createTab: vi.fn((_worktreeId, _groupId, _shell, options) => ({ + id: options?.id ?? 'tab-minted' + })) + }) + const harness = await loadIpcEventsHarness(storeState) + harness.useIpcEvents() + + harness.createTerminal({ + requestId: 'reveal', + worktreeId: WORKTREE_ID, + ptyId: MOVED_PTY_ID, + leafId: MOVED_LEAF_ID, + tabId: SPAWN_TAB_ID, + presentation: 'focused', + title: 'OpenCode' + }) + + expect(storeState.createTab).not.toHaveBeenCalled() + expect(tabsBindingMovedLeaf(storeState)).toEqual([HOST_TAB_ID]) + }) + + it('fails the reveal instead of minting when the only owner has no tab row', async () => { + // A layout can outlive its row. Nothing is left to activate, and minting + // here would re-bind a leaf id the orphan layout still holds (STA-7961). + const storeState: HarnessStoreState = createHarnessStoreState({ + tabsByWorktree: { [WORKTREE_ID]: [] }, + ptyIdsByTabId: {}, + terminalLayoutsByTabId: { + 'tab-gone': { ptyIdsByLeafId: { [MOVED_LEAF_ID]: MOVED_PTY_ID } } + }, + createTab: vi.fn((_worktreeId, _groupId, _shell, options) => ({ + id: options?.id ?? 'tab-minted' + })) + }) + const harness = await loadIpcEventsHarness(storeState) + harness.useIpcEvents() + + harness.createTerminal({ + requestId: 'reveal', + worktreeId: WORKTREE_ID, + ptyId: MOVED_PTY_ID, + leafId: MOVED_LEAF_ID, + presentation: 'focused', + title: 'OpenCode' + }) + + expect(storeState.createTab).not.toHaveBeenCalled() + expect(tabsBindingMovedLeaf(storeState)).toEqual(['tab-gone']) + expect(harness.replyTerminalCreate).toHaveBeenCalledWith({ + requestId: 'reveal', + error: 'terminal_reveal_owner_row_missing: tab tab-gone' + }) + }) + + it('keeps the closed spawn tab retired when the host tab row is out of scope', async () => { + // Same move and close, but the reveal names a worktree whose tab list does + // not carry the host row. The stale hint becomes the minted tab's id. + const moved = moveThenCloseSpawnTab() + const storeState: HarnessStoreState = createHarnessStoreState({ + tabsByWorktree: { [WORKTREE_ID]: [] }, + ptyIdsByTabId: {}, + terminalLayoutsByTabId: toHarnessLayouts(moved.terminalLayoutsByTabId), + createTab: vi.fn((_worktreeId, _groupId, _shell, options) => ({ + id: options?.id ?? 'tab-minted' + })) + }) + const harness = await loadIpcEventsHarness(storeState) + harness.useIpcEvents() + + harness.createTerminal({ + requestId: 'reveal', + worktreeId: WORKTREE_ID, + ptyId: MOVED_PTY_ID, + leafId: MOVED_LEAF_ID, + tabId: SPAWN_TAB_ID, + presentation: 'focused', + title: 'OpenCode' + }) + + expect(Object.keys(storeState.terminalLayoutsByTabId)).not.toContain(SPAWN_TAB_ID) + expect(tabsBindingMovedLeaf(storeState)).toEqual([HOST_TAB_ID]) + }) +}) diff --git a/src/renderer/src/lib/terminal-reveal-tab-adoption.ts b/src/renderer/src/lib/terminal-reveal-tab-adoption.ts new file mode 100644 index 00000000000..1b56a9c0619 --- /dev/null +++ b/src/renderer/src/lib/terminal-reveal-tab-adoption.ts @@ -0,0 +1,99 @@ +import { collectLeafIdsInOrder } from '@/components/terminal-pane/layout-serialization' +import type { AppState } from '@/store/types' +import type { TerminalTab } from '../../../shared/terminal-tab-types' +import { + listTerminalTabPtyOwners, + resolveTerminalTabPtyOwnership, + type TerminalTabPtyOwnershipState +} from './terminal-tab-for-pty-id' + +export type TerminalRevealTabAdoption = + | { kind: 'adopt'; tabId: string; via: 'pty-owner' | 'bound-leaf' | 'ambiguity-tiebreak' } + | { kind: 'mint'; verdict: 'none' | 'ambiguous' } + +export type TerminalRevealTabRequest = { + worktreeId: string + ptyId: string + leafId?: string + /** Tab id baked into the PTY's env when it spawned; a hint, not a binding. */ + hintTabId?: string +} + +/** + * The tab whose layout already contains a leaf id. A leaf id names one pane for + * its lifetime, and layouts are keyed by tab id alone, so this answer is + * independent of which worktree list the owning row currently sits in. + */ +export function findTerminalTabIdBindingLeafId( + state: Pick, + leafId: string +): string | null { + for (const [tabId, layout] of Object.entries(state.terminalLayoutsByTabId)) { + if (layout === undefined) { + continue + } + if (layout.ptyIdsByLeafId !== undefined && leafId in layout.ptyIdsByLeafId) { + return tabId + } + if (layout.root && collectLeafIdsInOrder(layout.root).includes(leafId)) { + return tabId + } + } + return null +} + +/** Locate a tab row and the worktree key it is filed under, across every key. */ +export function findTerminalTabRow( + state: Pick, + tabId: string +): { tab: TerminalTab; worktreeId: string } | null { + for (const [worktreeId, tabs] of Object.entries(state.tabsByWorktree)) { + const tab = tabs.find((candidate) => candidate.id === tabId) + if (tab) { + return { tab, worktreeId } + } + } + return null +} + +/** + * Decide which tab a terminal reveal belongs to. Minting is the last resort: + * a second tab bound to a live PTY mounts the same session twice, and the + * single-slot data handler then starves one of the two panes (STA-7961). + */ +export function resolveTerminalRevealTabAdoption( + state: TerminalTabPtyOwnershipState, + request: TerminalRevealTabRequest +): TerminalRevealTabAdoption { + const ownership = resolveTerminalTabPtyOwnership( + state, + request.worktreeId, + request.ptyId, + request.hintTabId !== undefined ? { preferTabId: request.hintTabId } : {} + ) + if (ownership.kind === 'owned') { + return { kind: 'adopt', tabId: ownership.tabId, via: 'pty-owner' } + } + // Why: re-minting a bound leaf id hands two tabs the same pane identity even + // when the reveal's pty id is unowned or stale. + const leafOwnerTabId = request.leafId + ? findTerminalTabIdBindingLeafId(state, request.leafId) + : null + if (leafOwnerTabId !== null) { + return { kind: 'adopt', tabId: leafOwnerTabId, via: 'bound-leaf' } + } + if (ownership.kind === 'ambiguous') { + const owners = listTerminalTabPtyOwners(state, request.worktreeId, request.ptyId) + // The hint already won upstream if it named an owner, so take the strongest tier. + const fallbackTabId = owners.mounted[0] ?? owners.recorded[0] + if (fallbackTabId !== undefined) { + const candidates = [...owners.mounted, ...owners.recorded].join(', ') + console.warn( + `[onCreateTerminal] ptyId ${request.ptyId} is claimed by ${candidates};` + + ` attaching to ${fallbackTabId} rather than minting a second tab for it` + ) + return { kind: 'adopt', tabId: fallbackTabId, via: 'ambiguity-tiebreak' } + } + } + return { kind: 'mint', verdict: ownership.kind } +} diff --git a/src/renderer/src/lib/terminal-tab-for-pty-id.test.ts b/src/renderer/src/lib/terminal-tab-for-pty-id.test.ts index ed3f651a2d4..a575b33f9ad 100644 --- a/src/renderer/src/lib/terminal-tab-for-pty-id.test.ts +++ b/src/renderer/src/lib/terminal-tab-for-pty-id.test.ts @@ -205,4 +205,18 @@ describe('resolveTerminalTabPtyOwnership', () => { const s = state({ tabs: { wt: [{ id: 'tab-a', ptyId: 'wt@@2' }] } }) expect(resolveTerminalTabPtyOwnership(s, 'wt', 'wt@@1')).toEqual({ kind: 'none' }) }) + + it('does not report no owner while a layout still records the ptyId (STA-7961)', () => { + // terminalLayoutsByTabId is keyed by tab id alone, so it outlives the tab + // row's membership in this worktree list. Reporting none here is what lets + // the reveal bridge mint a second tab that re-binds the recorded leaf id. + const s = state({ + tabs: { + wt: [{ id: 'tab-other', ptyId: 'wt@@2' }], + 'wt-other': [{ id: 'tab-a', ptyId: null }] + }, + layouts: { 'tab-a': { ptyIdsByLeafId: { 'leaf-shared': 'wt@@1' } } } + }) + expect(resolveTerminalTabPtyOwnership(s, 'wt', 'wt@@1')).not.toEqual({ kind: 'none' }) + }) }) diff --git a/src/renderer/src/lib/terminal-tab-for-pty-id.ts b/src/renderer/src/lib/terminal-tab-for-pty-id.ts index 1c247e6b55e..7c293e10a27 100644 --- a/src/renderer/src/lib/terminal-tab-for-pty-id.ts +++ b/src/renderer/src/lib/terminal-tab-for-pty-id.ts @@ -10,11 +10,92 @@ export type TerminalTabPtyOwnership = | { kind: 'ambiguous' } | { kind: 'none' } +/** Tabs binding a ptyId, split by tier: a mounted pane outranks a recorded row. */ +export type TerminalTabPtyOwners = { mounted: string[]; recorded: string[] } + type TerminalTabPtyOwnershipOptions = { /** Tab id baked into the PTY's env; a fallback and tie-break, not a binding. */ preferTabId?: string } +/** + * Every tab row the store holds, the named worktree's first. + * + * Why: terminalLayoutsByTabId and ptyIdsByTabId are keyed by tab id alone, so a + * binding outlives its row's membership in any one worktree list (STA-7961). + * A key with no row anywhere is orphan state, not an owner: counting it would + * make a live PTY look surfaced when nothing can render it. + */ +function listKnownTabIds(state: TerminalTabPtyOwnershipState, worktreeId: string): string[] { + const ordered: string[] = [] + const seen = new Set() + const push = (tabId: string): void => { + if (!seen.has(tabId)) { + seen.add(tabId) + ordered.push(tabId) + } + } + for (const tab of state.tabsByWorktree[worktreeId] ?? []) { + push(tab.id) + } + for (const tabs of Object.values(state.tabsByWorktree)) { + for (const tab of tabs) { + push(tab.id) + } + } + return ordered +} + +/** tab.ptyId by tab id across every worktree list. */ +function collectTabRowPtyIds(state: TerminalTabPtyOwnershipState): Map { + const byTabId = new Map() + for (const tabs of Object.values(state.tabsByWorktree)) { + for (const tab of tabs) { + if (tab.ptyId) { + byTabId.set(tab.id, tab.ptyId) + } + } + } + return byTabId +} + +function collectOwners( + state: TerminalTabPtyOwnershipState, + tabIds: string[], + ptyId: string +): TerminalTabPtyOwners { + const rowPtyIds = collectTabRowPtyIds(state) + const mounted: string[] = [] + const recorded: string[] = [] + for (const tabId of tabIds) { + if ((state.ptyIdsByTabId[tabId] ?? []).includes(ptyId)) { + mounted.push(tabId) + continue + } + const ptyIdsByLeafId = state.terminalLayoutsByTabId[tabId]?.ptyIdsByLeafId + if ( + rowPtyIds.get(tabId) === ptyId || + (ptyIdsByLeafId !== undefined && Object.values(ptyIdsByLeafId).includes(ptyId)) + ) { + recorded.push(tabId) + } + } + return { mounted, recorded } +} + +/** Candidate owners of a ptyId across the whole store, event worktree first. */ +export function listTerminalTabPtyOwners( + state: TerminalTabPtyOwnershipState, + worktreeId: string, + ptyId: string +): TerminalTabPtyOwners { + return collectOwners(state, listKnownTabIds(state, worktreeId), ptyId) +} + +function tabRowExists(state: TerminalTabPtyOwnershipState, tabId: string): boolean { + return Object.values(state.tabsByWorktree).some((tabs) => tabs.some((tab) => tab.id === tabId)) +} + /** * Resolve which tab owns a ptyId. Every binding below is an exact match on the * id, so any of them beats the caller's tab hint: that hint is written once when @@ -27,27 +108,12 @@ export function resolveTerminalTabPtyOwnership( ptyId: string, options: TerminalTabPtyOwnershipOptions = {} ): TerminalTabPtyOwnership { - const tabs = state.tabsByWorktree[worktreeId] ?? [] - const mountedOwners: string[] = [] - const recordedOwners: string[] = [] - for (const tab of tabs) { - if ((state.ptyIdsByTabId[tab.id] ?? []).includes(ptyId)) { - mountedOwners.push(tab.id) - continue - } - const ptyIdsByLeafId = state.terminalLayoutsByTabId[tab.id]?.ptyIdsByLeafId - if ( - tab.ptyId === ptyId || - (ptyIdsByLeafId !== undefined && Object.values(ptyIdsByLeafId).includes(ptyId)) - ) { - recordedOwners.push(tab.id) - } - } + const { mounted, recorded } = listTerminalTabPtyOwners(state, worktreeId, ptyId) const preferredTabId = - options.preferTabId !== undefined && tabs.some((tab) => tab.id === options.preferTabId) + options.preferTabId !== undefined && tabRowExists(state, options.preferTabId) ? options.preferTabId : undefined - const owners = mountedOwners.length > 0 ? mountedOwners : recordedOwners + const owners = mounted.length > 0 ? mounted : recorded if (owners.length === 1) { return { kind: 'owned', tabId: owners[0]! } } @@ -63,12 +129,18 @@ export function resolveTerminalTabPtyOwnership( return preferredTabId !== undefined ? { kind: 'owned', tabId: preferredTabId } : { kind: 'none' } } -/** Resolve a synthetic mobile handle's ptyId; null when unowned or ambiguous. */ +/** + * Resolve a synthetic mobile handle's ptyId within one worktree; null when + * unowned or ambiguous. Scoped on purpose: the caller mounts the tab under the + * requested worktree, so a row filed elsewhere is not a usable answer. + */ export function resolveTerminalTabIdForPtyId( state: TerminalTabPtyOwnershipState, worktreeId: string, ptyId: string ): string | null { - const ownership = resolveTerminalTabPtyOwnership(state, worktreeId, ptyId) - return ownership.kind === 'owned' ? ownership.tabId : null + const tabIds = (state.tabsByWorktree[worktreeId] ?? []).map((tab) => tab.id) + const { mounted, recorded } = collectOwners(state, tabIds, ptyId) + const owners = mounted.length > 0 ? mounted : recorded + return owners.length === 1 ? owners[0]! : null } diff --git a/src/renderer/src/store/terminals/workspace-terminal-duplicate-binding-reconnect.test.ts b/src/renderer/src/store/terminals/workspace-terminal-duplicate-binding-reconnect.test.ts new file mode 100644 index 00000000000..df3d9d8a9ef --- /dev/null +++ b/src/renderer/src/store/terminals/workspace-terminal-duplicate-binding-reconnect.test.ts @@ -0,0 +1,130 @@ +// A losing tab has a second route back to the PTY its layout just gave up: its own row +// `ptyId`, which reconnect copies back onto the row and the pane then takes as a fallback. +// This drives the real hydration entry with the STA-7961 pair to pin both routes shut. +import { describe, expect, it, vi } from 'vitest' +import type { WorkspaceSessionState } from '../../../../shared/workspace-session-state-types' + +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() } + +import { createTestStore, makeTab, makeWorktree, seedStore } from '../slices/store-test-helpers' + +const WORKTREE_ID = 'repo1::/wt-1' +const SPLIT_TAB_ID = 'eba00a9a-17df-4152-8258-42381b48890a' +const SINGLE_TAB_ID = '881a9ee2-7143-46c8-98ac-8ffbb9cf4b2c' +const SHARED_LEAF_ID = '10cb5648-8a54-41c0-a6a4-ef0028d93599' +const OWN_LEAF_ID = 'df8913c9-fd8a-420a-a7d6-17daf0ed30f0' +const SHARED_PTY_ID = 'repo1::/wt-1@@289ed0f2' +const SPLIT_OWN_PTY_ID = 'repo1::/wt-1@@eaff6e99' + +/** The persisted pair from the report: both tabs bind the same leaf to the same PTY. */ +function duplicateLeafSession(): WorkspaceSessionState { + return { + activeRepoId: 'repo1', + activeWorktreeId: WORKTREE_ID, + activeTabId: SINGLE_TAB_ID, + activeWorktreeIdsOnShutdown: [WORKTREE_ID], + tabsByWorktree: { + [WORKTREE_ID]: [ + makeTab({ + id: SPLIT_TAB_ID, + worktreeId: WORKTREE_ID, + ptyId: SPLIT_OWN_PTY_ID, + sortOrder: 0, + createdAt: 1_789_867_969_623 + }), + makeTab({ + id: SINGLE_TAB_ID, + worktreeId: WORKTREE_ID, + ptyId: SHARED_PTY_ID, + sortOrder: 1, + createdAt: 1_789_867_969_624 + }) + ] + }, + terminalLayoutsByTabId: { + [SPLIT_TAB_ID]: { + root: { + type: 'split', + direction: 'vertical', + first: { type: 'leaf', leafId: OWN_LEAF_ID }, + second: { type: 'leaf', leafId: SHARED_LEAF_ID } + }, + activeLeafId: SHARED_LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { [OWN_LEAF_ID]: SPLIT_OWN_PTY_ID, [SHARED_LEAF_ID]: SHARED_PTY_ID } + }, + [SINGLE_TAB_ID]: { + root: { type: 'leaf', leafId: SHARED_LEAF_ID }, + activeLeafId: SHARED_LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { [SHARED_LEAF_ID]: SHARED_PTY_ID } + } + } + } +} + +function hydrate(): ReturnType['getState']> { + const store = createTestStore() + seedStore(store, { + worktreesByRepo: { + repo1: [makeWorktree({ id: WORKTREE_ID, repoId: 'repo1', path: '/wt-1' })] + } + }) + store.getState().hydrateWorkspaceSession(duplicateLeafSession()) + return store.getState() +} + +describe('hydrating the STA-7961 duplicate binding', () => { + it('leaves the shared pty bound to one tab only', () => { + const state = hydrate() + + expect(state.terminalLayoutsByTabId[SPLIT_TAB_ID]?.ptyIdsByLeafId?.[SHARED_LEAF_ID]).toBe( + SHARED_PTY_ID + ) + expect( + Object.values(state.terminalLayoutsByTabId[SINGLE_TAB_ID]?.ptyIdsByLeafId ?? {}) + ).not.toContain(SHARED_PTY_ID) + }) + + it('keeps the losing row from taking the pty back through its tab-level id', () => { + const state = hydrate() + + expect(state.pendingReconnectPtyIdByTabId[SINGLE_TAB_ID]).toBeUndefined() + expect( + state.tabsByWorktree[WORKTREE_ID]?.find((tab) => tab.id === SINGLE_TAB_ID)?.ptyId + ).not.toBe(SHARED_PTY_ID) + }) + + it('still reconnects a row whose own pty nothing else claims', () => { + const state = hydrate() + + expect(state.pendingReconnectPtyIdByTabId[SPLIT_TAB_ID]).toBe(SPLIT_OWN_PTY_ID) + }) + + it('keeps the losing tab and its pane', () => { + const state = hydrate() + + expect(state.tabsByWorktree[WORKTREE_ID]?.map((tab) => tab.id)).toEqual([ + SPLIT_TAB_ID, + SINGLE_TAB_ID + ]) + expect(state.terminalLayoutsByTabId[SINGLE_TAB_ID]?.root).toEqual({ + type: 'leaf', + leafId: SHARED_LEAF_ID + }) + }) +}) diff --git a/src/renderer/src/store/terminals/workspace-terminal-hydration.ts b/src/renderer/src/store/terminals/workspace-terminal-hydration.ts index db278e05d92..c3f1ea4aa32 100644 --- a/src/renderer/src/store/terminals/workspace-terminal-hydration.ts +++ b/src/renderer/src/store/terminals/workspace-terminal-hydration.ts @@ -109,19 +109,6 @@ export function createWorkspaceTerminalHydrationActions( runtimeSessionPlaceholders.repos.some((repo) => repo.id === session.activeRepoId) ? session.activeRepoId : null - const { - pendingReconnectPtyIdByTabId, - pendingReconnectTabByWorktree, - pendingReconnectWorktreeIds - } = buildWorkspaceTerminalReconnectPlan({ - reconnectPtyIdByRetainedTabId, - releasedPtyIdsByTabId, - repos: runtimeSessionPlaceholders.repos, - session, - validTabIds, - validWorktreeIds, - worktreesByRepo: runtimeSessionPlaceholders.worktreesByRepo - }) // Restore per-worktree active tab; validate ids when the map exists, else derive for legacy sessions. let activeTabIdByWorktree: Record = {} if (session.activeTabIdByWorktree) { @@ -163,6 +150,29 @@ export function createWorkspaceTerminalHydrationActions( // re-flattening tabsByWorktree per entry is O(tabs x layouts). const allTabs = Object.values(tabsByWorktree).flat() const tabById = buildByIdIndex(allTabs) + const terminalLayoutPlan = buildWorkspaceTerminalLayoutPlan({ + ownershipTransfersByTabId, + ownershipTransferTabIds, + releasedPtyIdsByTabId, + session, + tabById, + validTabIds + }) + // Why after the layout plan: it reports the PTYs a duplicate binding just cost a row, + // and reconnect must withhold those from the row's tab-level id too. + const { + pendingReconnectPtyIdByTabId, + pendingReconnectTabByWorktree, + pendingReconnectWorktreeIds + } = buildWorkspaceTerminalReconnectPlan({ + reconnectPtyIdByRetainedTabId, + releasedPtyIdsByTabId: terminalLayoutPlan.releasedPtyIdsByTabId, + repos: runtimeSessionPlaceholders.repos, + session, + validTabIds, + validWorktreeIds, + worktreesByRepo: runtimeSessionPlaceholders.worktreesByRepo + }) const hydrated: WorkspaceHydrationPatch = { activeRepoId, activeWorktreeId, @@ -210,14 +220,7 @@ export function createWorkspaceTerminalHydrationActions( worktreeNavHistory: activeWorktreeId ? [activeWorktreeId] : [], worktreeNavHistoryIndex: activeWorktreeId ? 0 : -1, ptyIdsByTabId: Object.fromEntries(allTabs.map((tab) => [tab.id, []] as const)), - terminalLayoutsByTabId: buildWorkspaceTerminalLayoutPlan({ - ownershipTransfersByTabId, - ownershipTransferTabIds, - releasedPtyIdsByTabId, - session, - tabById, - validTabIds - }), + terminalLayoutsByTabId: terminalLayoutPlan.layoutsByTabId, localOnlyScrollbackByTabId: Object.fromEntries( Object.entries(session.localOnlyScrollbackByTabId ?? {}).filter(([tabId]) => validTabIds.has(tabId) diff --git a/src/renderer/src/store/terminals/workspace-terminal-layout-duplicate-bindings.test.ts b/src/renderer/src/store/terminals/workspace-terminal-layout-duplicate-bindings.test.ts new file mode 100644 index 00000000000..26aeb4cefbc --- /dev/null +++ b/src/renderer/src/store/terminals/workspace-terminal-layout-duplicate-bindings.test.ts @@ -0,0 +1,193 @@ +import { describe, expect, it } from 'vitest' +import { resolveDuplicateTerminalLayoutBindings } from './workspace-terminal-layout-duplicate-bindings' +import type { TerminalLayoutSnapshot, TerminalTab } from '../../../../shared/terminal-tab-types' + +const SPLIT_TAB = 'tab-split' +const SINGLE_TAB = 'tab-single' +// Why real UUIDs: layout normalization re-mints any leaf id that is not a stable pane id. +const SHARED_LEAF = '10cb5648-8a54-41c0-a6a4-ef0028d93599' +const OTHER_LEAF = 'df8913c9-fd8a-420a-a7d6-17daf0ed30f0' +const ELSEWHERE_LEAF = '96cdf7ea-9c83-4ba5-a41b-c425955e6606' +const SHARED_PTY = 'wt-1@@shared' + +function tab(id: string, sortOrder: number, createdAt = 1_000 + sortOrder): TerminalTab { + return { + id, + ptyId: null, + worktreeId: 'wt-1', + title: 'Terminal', + customTitle: null, + color: null, + sortOrder, + createdAt + } +} + +function heal( + layoutsByTabId: Record, + tabs: TerminalTab[], + canonicalTabIds: string[] = [] +): Record { + return resolveDuplicateTerminalLayoutBindings({ + canonicalTabIds: new Set(canonicalTabIds), + layoutsByTabId, + tabById: new Map(tabs.map((row) => [row.id, row])) + }).layoutsByTabId +} + +function splitLayout(): TerminalLayoutSnapshot { + return { + root: { + type: 'split', + direction: 'vertical', + first: { type: 'leaf', leafId: OTHER_LEAF }, + second: { type: 'leaf', leafId: SHARED_LEAF } + }, + activeLeafId: SHARED_LEAF, + expandedLeafId: null, + ptyIdsByLeafId: { [OTHER_LEAF]: 'wt-1@@other', [SHARED_LEAF]: SHARED_PTY } + } +} + +function singleLeafLayout(leafId: string, ptyId: string): TerminalLayoutSnapshot { + return { + root: { type: 'leaf', leafId }, + activeLeafId: leafId, + expandedLeafId: null, + ptyIdsByLeafId: { [leafId]: ptyId } + } +} + +describe('resolveDuplicateTerminalLayoutBindings', () => { + it('leaves the single-leaf loser its pane and takes only the pty binding', () => { + const healed = heal( + { + [SPLIT_TAB]: splitLayout(), + [SINGLE_TAB]: singleLeafLayout(SHARED_LEAF, SHARED_PTY) + }, + [tab(SPLIT_TAB, 0), tab(SINGLE_TAB, 1)] + ) + + expect(healed[SPLIT_TAB].ptyIdsByLeafId?.[SHARED_LEAF]).toBe(SHARED_PTY) + expect(healed[SINGLE_TAB].root).toEqual({ type: 'leaf', leafId: SHARED_LEAF }) + expect(healed[SINGLE_TAB].ptyIdsByLeafId ?? {}).toEqual({}) + expect(healed[SINGLE_TAB].activeLeafId).toBe(SHARED_LEAF) + }) + + it('removes the duplicated leaf from a split loser and keeps its other panes', () => { + const healed = heal( + { + [SPLIT_TAB]: splitLayout(), + [SINGLE_TAB]: singleLeafLayout(SHARED_LEAF, SHARED_PTY) + }, + [tab(SINGLE_TAB, 0), tab(SPLIT_TAB, 1)] + ) + + expect(healed[SPLIT_TAB].root).toEqual({ type: 'leaf', leafId: OTHER_LEAF }) + expect(healed[SPLIT_TAB].ptyIdsByLeafId).toEqual({ [OTHER_LEAF]: 'wt-1@@other' }) + expect(healed[SPLIT_TAB].activeLeafId).toBe(OTHER_LEAF) + expect(healed[SINGLE_TAB].ptyIdsByLeafId?.[SHARED_LEAF]).toBe(SHARED_PTY) + }) + + it('unbinds a pty shared under two leaf ids and leaves both panes standing', () => { + const healed = heal( + { + [SPLIT_TAB]: splitLayout(), + [SINGLE_TAB]: singleLeafLayout(ELSEWHERE_LEAF, SHARED_PTY) + }, + [tab(SINGLE_TAB, 0), tab(SPLIT_TAB, 1)] + ) + + const owners = Object.entries(healed) + .filter(([, layout]) => Object.values(layout.ptyIdsByLeafId ?? {}).includes(SHARED_PTY)) + .map(([tabId]) => tabId) + expect(owners).toEqual([SINGLE_TAB]) + // The losing pane is real — only its mount collided — so the split keeps both leaves. + expect(healed[SPLIT_TAB].root).toEqual(splitLayout().root) + expect(healed[SPLIT_TAB].ptyIdsByLeafId).toEqual({ [OTHER_LEAF]: 'wt-1@@other' }) + expect(healed[SPLIT_TAB].activeLeafId).toBe(OTHER_LEAF) + }) + + it('lets a canonical unified tab outrank a lower sort order', () => { + const healed = heal( + { + [SPLIT_TAB]: splitLayout(), + [SINGLE_TAB]: singleLeafLayout(SHARED_LEAF, SHARED_PTY) + }, + [tab(SPLIT_TAB, 0), tab(SINGLE_TAB, 1)], + [SINGLE_TAB] + ) + + expect(healed[SINGLE_TAB].ptyIdsByLeafId?.[SHARED_LEAF]).toBe(SHARED_PTY) + expect(healed[SPLIT_TAB].root).toEqual({ type: 'leaf', leafId: OTHER_LEAF }) + }) + + it('breaks an all-equal tie on tab id, whichever order the layouts arrive in', () => { + const tabs = [tab('tab-b', 0, 1_000), tab('tab-a', 0, 1_000)] + const forward = heal( + { + 'tab-b': singleLeafLayout(SHARED_LEAF, SHARED_PTY), + 'tab-a': singleLeafLayout(SHARED_LEAF, SHARED_PTY) + }, + tabs + ) + const reverse = heal( + { + 'tab-a': singleLeafLayout(SHARED_LEAF, SHARED_PTY), + 'tab-b': singleLeafLayout(SHARED_LEAF, SHARED_PTY) + }, + tabs + ) + + for (const healed of [forward, reverse]) { + expect(healed['tab-a'].ptyIdsByLeafId?.[SHARED_LEAF]).toBe(SHARED_PTY) + expect(healed['tab-b'].ptyIdsByLeafId ?? {}).toEqual({}) + } + }) + + it('ignores a binding whose pane already left the tree', () => { + const ghost: TerminalLayoutSnapshot = { + root: null, + activeLeafId: null, + expandedLeafId: null, + ptyIdsByLeafId: { [ELSEWHERE_LEAF]: SHARED_PTY } + } + const healed = heal({ [SINGLE_TAB]: ghost, [SPLIT_TAB]: splitLayout() }, [ + tab(SINGLE_TAB, 0), + tab(SPLIT_TAB, 1) + ]) + + // The ghost reattaches nothing, so it must not outrank the pane that still mounts the pty. + expect(healed[SPLIT_TAB].ptyIdsByLeafId?.[SHARED_LEAF]).toBe(SHARED_PTY) + }) + + it('returns the same object, and surrenders nothing, when no tab collides', () => { + const layoutsByTabId = { + [SPLIT_TAB]: splitLayout(), + [SINGLE_TAB]: singleLeafLayout('08d0d524-0d46-410b-ba08-b43c97a2b4e4', 'wt-1@@own') + } + + const resolution = resolveDuplicateTerminalLayoutBindings({ + canonicalTabIds: new Set(), + layoutsByTabId, + tabById: new Map([tab(SPLIT_TAB, 0), tab(SINGLE_TAB, 1)].map((row) => [row.id, row])) + }) + + expect(resolution.layoutsByTabId).toBe(layoutsByTabId) + expect(resolution.surrenderedPtyIdsByTabId.size).toBe(0) + }) + + it('names the pty each losing tab gave up, whichever shape it lost it in', () => { + const resolution = resolveDuplicateTerminalLayoutBindings({ + canonicalTabIds: new Set(), + layoutsByTabId: { + [SPLIT_TAB]: splitLayout(), + [SINGLE_TAB]: singleLeafLayout(SHARED_LEAF, SHARED_PTY) + }, + tabById: new Map([tab(SPLIT_TAB, 0), tab(SINGLE_TAB, 1)].map((row) => [row.id, row])) + }) + + expect([...(resolution.surrenderedPtyIdsByTabId.get(SINGLE_TAB) ?? [])]).toEqual([SHARED_PTY]) + expect(resolution.surrenderedPtyIdsByTabId.has(SPLIT_TAB)).toBe(false) + }) +}) diff --git a/src/renderer/src/store/terminals/workspace-terminal-layout-duplicate-bindings.ts b/src/renderer/src/store/terminals/workspace-terminal-layout-duplicate-bindings.ts new file mode 100644 index 00000000000..0a7b863914c --- /dev/null +++ b/src/renderer/src/store/terminals/workspace-terminal-layout-duplicate-bindings.ts @@ -0,0 +1,193 @@ +/** + * Hydration self-heal for layouts that persist one leaf id — or one PTY id — under two tabs. + * + * Why: a mint bug can write the same leaf id, bound to the same PTY, into two tabs' layouts. + * On the next launch both tabs mount that PTY. Renderer data delivery is one handler slot per + * PTY id, so one pane starves, and both panes forward their fit, so the PTY grid flips between + * two sizes. One tab keeps the binding; the other cold-starts its own shell. + */ +import type { TerminalLayoutSnapshot, TerminalTab } from '../../../../shared/terminal-tab-types' +import type { WorkspaceSessionState } from '../../../../shared/workspace-session-state-types' +import { detachTerminalLayoutLeaf } from '@/components/terminal-pane/terminal-layout-leaf-detach' +import { + collectLeafIdsInOrder, + resolvePtyBoundActiveLeafId +} from '@/components/terminal-pane/terminal-layout-leaf-ids' + +/** A layout's claim on a shared id, and the leaf that carries it. */ +type LayoutBinding = { key: string; leafId: string } + +export type DuplicateTerminalLayoutBindingResolution = { + layoutsByTabId: Record + /** PTYs a losing tab just gave up, so reconnect withholds them from its row as well. */ + surrenderedPtyIdsByTabId: ReadonlyMap> +} + +type TabRanking = { + canonicalTabIds: ReadonlySet + tabById: ReadonlyMap +} + +/** Terminal ids the unified tab model owns, across every worktree the session describes. */ +export function readCanonicalTerminalTabIds(session: WorkspaceSessionState): Set { + return new Set( + Object.values(session.unifiedTabs ?? {}).flatMap((tabs) => + tabs.filter((tab) => tab.contentType === 'terminal').map((tab) => tab.entityId) + ) + ) +} + +function readLeafBindings(layout: TerminalLayoutSnapshot): LayoutBinding[] { + return collectLeafIdsInOrder(layout.root).map((leafId) => ({ key: leafId, leafId })) +} + +/** + * Why only leaves still in the tree: a binding whose pane already left reattaches nothing, so + * it cannot starve the pane that does hold the PTY. Row hydration draws the same line — it + * refuses to read a stranded binding as ownership — and counting one here would let a + * never-pruned ghost outrank a live pane. + */ +function readPtyBindings(layout: TerminalLayoutSnapshot): LayoutBinding[] { + const mountedLeafIds = new Set(collectLeafIdsInOrder(layout.root)) + return Object.entries(layout.ptyIdsByLeafId ?? {}) + .filter(([leafId]) => mountedLeafIds.has(leafId)) + .map(([leafId, ptyId]) => ({ key: ptyId, leafId })) +} + +/** Canonical first, then the tab the user ordered first, then the older row, then id order. */ +function compareOwnerTabIds(a: string, b: string, ranking: TabRanking): number { + const aCanonical = ranking.canonicalTabIds.has(a) + const bCanonical = ranking.canonicalTabIds.has(b) + if (aCanonical !== bCanonical) { + return aCanonical ? -1 : 1 + } + const aTab = ranking.tabById.get(a) + const bTab = ranking.tabById.get(b) + // Why MAX_SAFE_INTEGER: a layout with no surviving row ranks behind every row that has one. + return ( + (aTab?.sortOrder ?? Number.MAX_SAFE_INTEGER) - (bTab?.sortOrder ?? Number.MAX_SAFE_INTEGER) || + (aTab?.createdAt ?? Number.MAX_SAFE_INTEGER) - (bTab?.createdAt ?? Number.MAX_SAFE_INTEGER) || + (a < b ? -1 : a > b ? 1 : 0) + ) +} + +function dropPtyBinding(layout: TerminalLayoutSnapshot, leafId: string): TerminalLayoutSnapshot { + const ptyIdsByLeafId = layout.ptyIdsByLeafId + if (!ptyIdsByLeafId || !Object.hasOwn(ptyIdsByLeafId, leafId)) { + return layout + } + const remaining = Object.fromEntries( + Object.entries(ptyIdsByLeafId).filter(([boundLeafId]) => boundLeafId !== leafId) + ) + return { + ...layout, + ptyIdsByLeafId: remaining, + activeLeafId: resolvePtyBoundActiveLeafId({ + root: layout.root, + activeLeafId: layout.activeLeafId, + ptyIdsByLeafId: remaining + }) + } +} + +/** + * A duplicated leaf id is an identity collision, so a split tab gives up the whole pane; a + * single-leaf tab keeps its pane and cold-starts a shell rather than being left with no leaf. + */ +function surrenderLeaf(layout: TerminalLayoutSnapshot, leafId: string): TerminalLayoutSnapshot { + return detachTerminalLayoutLeaf(layout, leafId)?.sourceLayout ?? dropPtyBinding(layout, leafId) +} + +function applyDuplicateBindingPass( + layoutsByTabId: Record, + readBindings: (layout: TerminalLayoutSnapshot) => LayoutBinding[], + surrender: (layout: TerminalLayoutSnapshot, leafId: string) => TerminalLayoutSnapshot, + ranking: TabRanking, + surrenderedPtyIdsByTabId: Map> +): Record { + const claimantsByKey = new Map>() + for (const [tabId, layout] of Object.entries(layoutsByTabId)) { + for (const { key, leafId } of readBindings(layout)) { + const claimants = claimantsByKey.get(key) ?? new Map() + claimantsByKey.set(key, claimants) + if (!claimants.has(tabId)) { + claimants.set(tabId, leafId) + } + } + } + + const surrenderedLeafIdsByTabId = new Map() + for (const claimants of claimantsByKey.values()) { + if (claimants.size < 2) { + continue + } + // Why sort over reduce: the id tie-break makes the winner independent of iteration order. + const [ownerTabId] = [...claimants.keys()].sort((a, b) => compareOwnerTabIds(a, b, ranking)) + for (const [tabId, leafId] of claimants) { + if (tabId === ownerTabId) { + continue + } + const surrendered = surrenderedLeafIdsByTabId.get(tabId) ?? [] + surrenderedLeafIdsByTabId.set(tabId, surrendered) + if (!surrendered.includes(leafId)) { + surrendered.push(leafId) + } + } + } + + if (surrenderedLeafIdsByTabId.size === 0) { + return layoutsByTabId + } + const healed = { ...layoutsByTabId } + for (const [tabId, leafIds] of surrenderedLeafIdsByTabId) { + for (const leafId of leafIds) { + // Why recorded: the row's own `tab.ptyId` can name the same PTY, and reconnect would + // otherwise hand it back through that tab-level fallback after the layout let it go. + const surrenderedPtyId = healed[tabId].ptyIdsByLeafId?.[leafId] + if (surrenderedPtyId) { + const ptyIds = surrenderedPtyIdsByTabId.get(tabId) ?? new Set() + surrenderedPtyIdsByTabId.set(tabId, ptyIds) + ptyIds.add(surrenderedPtyId) + } + healed[tabId] = surrender(healed[tabId], leafId) + } + } + return healed +} + +/** + * Give every duplicated leaf id, then every duplicated PTY id, a single owning tab. + * Returns the argument itself when nothing collides. The PTY pass reads the healed + * layouts, so a leaf the first pass already surrendered is not counted twice — and it + * only unbinds, because a PTY shared under two distinct leaf ids collides on the mount, + * not on pane identity, so the losing pane is real and stays. + */ +export function resolveDuplicateTerminalLayoutBindings({ + canonicalTabIds, + layoutsByTabId, + tabById +}: { + canonicalTabIds: ReadonlySet + layoutsByTabId: Record + tabById: ReadonlyMap +}): DuplicateTerminalLayoutBindingResolution { + const ranking: TabRanking = { canonicalTabIds, tabById } + const surrenderedPtyIdsByTabId = new Map>() + const withUniqueLeafIds = applyDuplicateBindingPass( + layoutsByTabId, + readLeafBindings, + surrenderLeaf, + ranking, + surrenderedPtyIdsByTabId + ) + return { + layoutsByTabId: applyDuplicateBindingPass( + withUniqueLeafIds, + readPtyBindings, + dropPtyBinding, + ranking, + surrenderedPtyIdsByTabId + ), + surrenderedPtyIdsByTabId + } +} diff --git a/src/renderer/src/store/terminals/workspace-terminal-layout-plan-duplicate-leaf.test.ts b/src/renderer/src/store/terminals/workspace-terminal-layout-plan-duplicate-leaf.test.ts new file mode 100644 index 00000000000..37739ed6e0c --- /dev/null +++ b/src/renderer/src/store/terminals/workspace-terminal-layout-plan-duplicate-leaf.test.ts @@ -0,0 +1,108 @@ +import { describe, expect, it } from 'vitest' +import { buildWorkspaceTerminalLayoutPlan } from './workspace-terminal-layout-plan' +import type { TerminalTab } from '../../../../shared/terminal-tab-types' +import type { WorkspaceSessionState } from '../../../../shared/workspace-session-state-types' + +const WORKTREE_ID = 'wt-1' +// Ids and shapes copied verbatim from the STA-7961 orca-data.json excerpt. +const SHARED_LEAF_ID = '10cb5648-8a54-41c0-a6a4-ef0028d93599' +const SHARED_PTY_ID = 'wt-1@@289ed0f2' +const SPLIT_TAB_ID = 'eba00a9a-17df-4152-8258-42381b48890a' +const SINGLE_TAB_ID = '881a9ee2-7143-46c8-98ac-8ffbb9cf4b2c' + +function tab(id: string, sortOrder: number): TerminalTab { + return { + id, + ptyId: null, + worktreeId: WORKTREE_ID, + title: 'OpenCode', + customTitle: null, + color: null, + sortOrder, + createdAt: 1_789_867_969_623 + sortOrder + } +} + +/** The persisted pair from the report: a nested split and a single leaf, same leaf id and pty. */ +function duplicateLeafSession(): WorkspaceSessionState { + return { + activeRepoId: 'repo-1', + activeWorktreeId: WORKTREE_ID, + activeTabId: SINGLE_TAB_ID, + tabsByWorktree: { [WORKTREE_ID]: [tab(SPLIT_TAB_ID, 0), tab(SINGLE_TAB_ID, 1)] }, + terminalLayoutsByTabId: { + [SPLIT_TAB_ID]: { + root: { + type: 'split', + direction: 'vertical', + first: { + type: 'split', + direction: 'horizontal', + first: { type: 'leaf', leafId: 'df8913c9-fd8a-420a-a7d6-17daf0ed30f0' }, + second: { + type: 'split', + direction: 'horizontal', + first: { type: 'leaf', leafId: SHARED_LEAF_ID }, + second: { type: 'leaf', leafId: '08d0d524-0d46-410b-ba08-b43c97a2b4e4' } + } + }, + second: { type: 'leaf', leafId: '96cdf7ea-9c83-4ba5-a41b-c425955e6606' } + }, + activeLeafId: SHARED_LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { + 'df8913c9-fd8a-420a-a7d6-17daf0ed30f0': 'wt-1@@eaff6e99', + '96cdf7ea-9c83-4ba5-a41b-c425955e6606': 'wt-1@@e905e74a', + [SHARED_LEAF_ID]: SHARED_PTY_ID, + '08d0d524-0d46-410b-ba08-b43c97a2b4e4': 'wt-1@@8123ec24' + } + }, + [SINGLE_TAB_ID]: { + root: { type: 'leaf', leafId: SHARED_LEAF_ID }, + activeLeafId: SHARED_LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { [SHARED_LEAF_ID]: SHARED_PTY_ID } + } + } + } +} + +describe('workspace terminal layout hydration (STA-7961)', () => { + it('binds a persisted leaf id to exactly one tab', () => { + const session = duplicateLeafSession() + const tabs = session.tabsByWorktree[WORKTREE_ID] ?? [] + + const plan = buildWorkspaceTerminalLayoutPlan({ + ownershipTransfersByTabId: new Map(), + ownershipTransferTabIds: null, + releasedPtyIdsByTabId: new Map(), + session, + tabById: new Map(tabs.map((row) => [row.id, row])), + validTabIds: new Set(tabs.map((row) => row.id)) + }) + + const tabsBindingSharedLeaf = Object.entries(plan.layoutsByTabId) + .filter(([, layout]) => SHARED_LEAF_ID in (layout.ptyIdsByLeafId ?? {})) + .map(([tabId]) => tabId) + expect(tabsBindingSharedLeaf).toHaveLength(1) + }) + + it('hands the shared pty to exactly one tab', () => { + const session = duplicateLeafSession() + const tabs = session.tabsByWorktree[WORKTREE_ID] ?? [] + + const plan = buildWorkspaceTerminalLayoutPlan({ + ownershipTransfersByTabId: new Map(), + ownershipTransferTabIds: null, + releasedPtyIdsByTabId: new Map(), + session, + tabById: new Map(tabs.map((row) => [row.id, row])), + validTabIds: new Set(tabs.map((row) => row.id)) + }) + + const tabsBindingSharedPty = Object.entries(plan.layoutsByTabId) + .filter(([, layout]) => Object.values(layout.ptyIdsByLeafId ?? {}).includes(SHARED_PTY_ID)) + .map(([tabId]) => tabId) + expect(tabsBindingSharedPty).toHaveLength(1) + }) +}) diff --git a/src/renderer/src/store/terminals/workspace-terminal-layout-plan.ts b/src/renderer/src/store/terminals/workspace-terminal-layout-plan.ts index a97af259580..2818bd81837 100644 --- a/src/renderer/src/store/terminals/workspace-terminal-layout-plan.ts +++ b/src/renderer/src/store/terminals/workspace-terminal-layout-plan.ts @@ -8,6 +8,20 @@ import { import { resolveTerminalLayoutPtyOwnershipTransfers } from '@/components/terminal-pane/terminal-layout-pty-ownership' import { sanitizeTerminalLayoutPaneTitles } from '@/lib/terminal-pane-title-sanitization' import type { TerminalLayoutPtyOwnershipTransfer } from './workspace-terminal-hydration-patch' +import { + readCanonicalTerminalTabIds, + resolveDuplicateTerminalLayoutBindings +} from './workspace-terminal-layout-duplicate-bindings' + +export type WorkspaceTerminalLayoutPlan = { + layoutsByTabId: Record + /** + * Every PTY a row may no longer reattach: the ones the row plan released to a canonical + * twin, plus the ones a duplicate binding just cost it. Reconnect reads both, so a losing + * row cannot take its PTY back through `tab.ptyId`. + */ + releasedPtyIdsByTabId: ReadonlyMap> +} export function buildWorkspaceTerminalLayoutPlan({ ownershipTransfersByTabId, @@ -23,8 +37,8 @@ export function buildWorkspaceTerminalLayoutPlan({ session: WorkspaceSessionState tabById: ReadonlyMap validTabIds: ReadonlySet -}): Record { - return Object.fromEntries( +}): WorkspaceTerminalLayoutPlan { + const perTabLayouts = Object.fromEntries( Object.entries(session.terminalLayoutsByTabId) .filter(([tabId]) => validTabIds.has(tabId)) .map(([tabId, persisted]) => { @@ -55,4 +69,31 @@ export function buildWorkspaceTerminalLayoutPlan({ return [tabId, { ...sanitized, activeLeafId }] }) ) + // Why after per-tab normalization: a duplicated leaf or PTY id is only visible across tabs. + const resolved = resolveDuplicateTerminalLayoutBindings({ + canonicalTabIds: readCanonicalTerminalTabIds(session), + layoutsByTabId: perTabLayouts, + tabById + }) + return { + layoutsByTabId: resolved.layoutsByTabId, + releasedPtyIdsByTabId: mergeReleasedPtyIds( + releasedPtyIdsByTabId, + resolved.surrenderedPtyIdsByTabId + ) + } +} + +function mergeReleasedPtyIds( + released: ReadonlyMap>, + surrendered: ReadonlyMap> +): ReadonlyMap> { + if (surrendered.size === 0) { + return released + } + const merged = new Map>(released) + for (const [tabId, ptyIds] of surrendered) { + merged.set(tabId, new Set([...(released.get(tabId) ?? []), ...ptyIds])) + } + return merged }