diff --git a/src/renderer/src/hooks/ipc-events-test-harness.ts b/src/renderer/src/hooks/ipc-events-test-harness.ts new file mode 100644 index 00000000000..1ce81d5e404 --- /dev/null +++ b/src/renderer/src/hooks/ipc-events-test-harness.ts @@ -0,0 +1,216 @@ +import { vi } from 'vitest' +import type * as ReactModule from 'react' +import type { TerminalPaneLayoutNode } from '../../../shared/types' + +export type CreateTerminalRequest = { + requestId?: string + worktreeId: string + command?: string + title?: string + ptyId?: string + presentation?: 'background' | 'focused' + tabId?: string + leafId?: string + splitFromLeafId?: string +} + +export type HarnessTab = { id: string; ptyId?: string | null; title?: string } + +export type HarnessStoreState = { + tabsByWorktree: Record + ptyIdsByTabId: Record + terminalLayoutsByTabId: Record< + string, + { root?: TerminalPaneLayoutNode; ptyIdsByLeafId?: Record } + > + [key: string]: unknown +} + +/** Store surface useIpcEvents touches at mount plus the terminal-reveal path. */ +export function createHarnessStoreState( + overrides: Partial & Pick +): HarnessStoreState { + return { + createTab: vi.fn(() => ({ id: 'tab-minted' })), + setActiveView: vi.fn(), + setActiveWorktree: vi.fn(), + markWorktreeVisited: vi.fn(), + recordWorktreeVisit: vi.fn(), + isNavigatingHistory: false, + setActiveTabType: vi.fn(), + setActiveTab: vi.fn(), + revealWorktreeInSidebar: vi.fn(), + setTabCustomTitle: vi.fn(), + queueTabStartupCommand: vi.fn(), + registerAgentLaunchConfig: vi.fn(), + clearAgentLaunchConfig: vi.fn(), + updateTabPtyId: vi.fn(), + setTabLayout: vi.fn(), + setTabBarOrder: vi.fn(), + clearTabPtyId: vi.fn(), + setUpdateStatus: vi.fn(), + fetchRepos: vi.fn(), + fetchWorktrees: vi.fn(), + closeModal: vi.fn(), + openModal: vi.fn(), + setActiveRepo: vi.fn(), + setIsFullScreen: vi.fn(), + updateBrowserPageState: vi.fn(), + setEditorFontZoomLevel: vi.fn(), + setRateLimitsFromPush: vi.fn(), + setSshConnectionState: vi.fn(), + setSshTargetLabels: vi.fn(), + setPortForwards: vi.fn(), + clearPortForwards: vi.fn(), + setDetectedPorts: vi.fn(), + enqueueSshCredentialRequest: vi.fn(), + removeSshCredentialRequest: vi.fn(), + ptyIdsByTabId: {}, + terminalLayoutsByTabId: {}, + folderWorkspaces: [], + projectGroups: [], + repos: [{ id: 'repo-1', connectionId: null, executionHostId: 'local' }], + worktreesByRepo: { 'repo-1': [{ id: 'wt-1', repoId: 'repo-1' }] }, + openFiles: [], + browserTabsByWorktree: {}, + tabBarOrderByWorktree: {}, + activeModal: null, + activeWorktreeId: 'wt-1', + activeView: 'terminal', + activeTabType: 'terminal', + editorFontZoomLevel: 0, + settings: { + terminalFontSize: 13, + experimentalNativeChat: false, + openAgentTabsInChatByDefault: false, + activeRuntimeEnvironmentId: undefined + }, + ...overrides + } +} + +/** Subscription no-ops for every listener useIpcEvents attaches beyond the ones under test. */ +function createApiNamespaceStub(overrides: Record = {}): Record { + return new Proxy(overrides, { + get: (target, prop: string) => (prop in target ? target[prop] : () => () => {}) + }) +} + +export type IpcEventsHarness = { + /** Call inside the test body: useIpcEvents runs its effects eagerly here. */ + useIpcEvents: () => void + createTerminal: (request: CreateTerminalRequest) => void + replyTerminalCreate: ReturnType +} + +/** + * Loads useIpcEvents against a stubbed preload API and returns a driver for the + * create-terminal IPC, so reveal/adoption behavior is asserted through the hook. + */ +export async function loadIpcEventsHarness( + storeState: HarnessStoreState +): Promise { + const replyTerminalCreate = vi.fn() + let createTerminalListener: ((request: CreateTerminalRequest) => void) | null = null + + vi.resetModules() + vi.unstubAllGlobals() + + vi.doMock('react', async () => { + const actual = await vi.importActual('react') + return { ...actual, useEffect: (effect: () => void | (() => void)) => void effect() } + }) + vi.doMock('../store', () => ({ + useAppStore: { subscribe: vi.fn(() => () => {}), getState: () => storeState } + })) + vi.doMock('@/lib/ui-zoom', () => ({ applyUIZoom: vi.fn() })) + vi.doMock('@/lib/worktree-activation', () => ({ + activateAndRevealWorktree: vi.fn(), + ensureWorktreeHasInitialTerminal: vi.fn() + })) + vi.doMock('@/components/sidebar/visible-worktrees', () => ({ getVisibleWorktreeIds: () => [] })) + vi.doMock('@/lib/floating-workspace-terminal-actions', () => ({ + createFloatingWorkspaceTerminalTab: vi.fn(), + isEmptyFloatingWorkspacePanelVisible: () => false, + isFloatingWorkspacePanelFocused: () => false + })) + vi.doMock('@/runtime/web-runtime-session', () => ({ + activateWebRuntimeSessionTab: vi.fn(), + closeWebRuntimeSessionTab: vi.fn(), + createWebRuntimeSessionBrowserTab: vi.fn().mockResolvedValue(false), + createWebRuntimeSessionTerminal: vi.fn().mockResolvedValue({ status: 'failed', message: '' }), + isWebRuntimeSessionActive: vi.fn(() => false) + })) + vi.doMock('@/lib/focus-terminal-tab-surface', () => ({ focusTerminalTabSurface: vi.fn() })) + vi.doMock('@/runtime/sync-runtime-graph', () => ({ + focusRuntimeTerminalSurface: vi.fn(() => false) + })) + vi.doMock('@/lib/activate-tab-and-focus-pane', () => ({ activateTabAndFocusPane: vi.fn() })) + + vi.stubGlobal('window', { + dispatchEvent: vi.fn(), + api: new Proxy( + { + ui: createApiNamespaceStub({ + getZoomLevel: () => 0, + consumePendingOpenSettings: () => Promise.resolve(false), + set: vi.fn(), + replyTabCreate: vi.fn(), + replyTabClose: vi.fn(), + replyTabSetProfile: vi.fn(), + replyTerminalCreate, + onCreateTerminal: (listener: (request: CreateTerminalRequest) => void) => { + createTerminalListener = listener + return () => {} + } + }), + rateLimits: { + get: () => Promise.resolve({ limits: {}, lastUpdatedAt: 0 }), + onUpdate: () => () => {} + }, + runtime: { + getTerminalFitOverrides: () => Promise.resolve([]), + getTerminalDrivers: () => Promise.resolve([]), + getBrowserDrivers: () => Promise.resolve([]), + onTerminalFitOverrideChanged: () => () => {}, + onTerminalDriverChanged: () => () => {}, + onBrowserDriverChanged: () => () => {} + }, + ssh: { + listTargets: () => Promise.resolve([]), + listPortForwards: () => Promise.resolve([]), + listDetectedPorts: () => Promise.resolve([]), + listRemovedTargetLabels: () => Promise.resolve([]), + getState: () => Promise.resolve(null), + onStateChanged: () => () => {}, + onCredentialRequest: () => () => {}, + onCredentialResolved: () => () => {}, + onPortForwardsChanged: () => () => {}, + onDetectedPortsChanged: () => () => {} + }, + updater: { + getStatus: () => Promise.resolve({ state: 'idle' }), + onStatus: () => () => {}, + onClearDismissal: () => () => {} + }, + mobile: createApiNamespaceStub({ + consumePendingUnpairedDeviceAuthFailure: () => Promise.resolve(false) + }), + remoteWorkspace: createApiNamespaceStub({ clientId: () => Promise.resolve(null) }) + } as Record, + { get: (target, prop: string) => target[prop] ?? createApiNamespaceStub() } + ) + }) + + const { useIpcEvents } = await import('./useIpcEvents') + return { + useIpcEvents, + createTerminal: (request) => { + if (typeof createTerminalListener !== 'function') { + throw new Error('Expected the create-terminal listener to be registered') + } + createTerminalListener(request) + }, + replyTerminalCreate + } +} 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 new file mode 100644 index 00000000000..30130211c62 --- /dev/null +++ b/src/renderer/src/hooks/mobile-terminal-reveal-tab-adoption.test.ts @@ -0,0 +1,237 @@ +import { describe, expect, it } from 'vitest' +import { + createHarnessStoreState, + loadIpcEventsHarness, + type HarnessStoreState +} from './ipc-events-test-harness' + +const WORKTREE_ID = 'wt-1' + +function revealSplitPaneFromMobile(harness: { + createTerminal: (request: { + requestId?: string + worktreeId: string + ptyId?: string + tabId?: string + leafId?: string + presentation?: 'background' | 'focused' + title?: string + }) => void +}): void { + harness.createTerminal({ + requestId: 'mobile-reveal', + worktreeId: WORKTREE_ID, + ptyId: 'pty-b', + tabId: 'tab-split', + leafId: 'leaf-b', + presentation: 'focused', + title: 'codex' + }) +} + +describe('mobile terminal reveal tab adoption', () => { + it('adopts the owning tab when a split pane is revealed before its panes mount (#10486)', async () => { + // Desktop has the worktree closed: no pane is mounted, so the live pty map is + // empty and only the persisted layout still records the second leaf's pty. + const storeState: HarnessStoreState = createHarnessStoreState({ + tabsByWorktree: { [WORKTREE_ID]: [{ id: 'tab-split', ptyId: 'pty-a', title: 'codex' }] }, + ptyIdsByTabId: {}, + terminalLayoutsByTabId: { + 'tab-split': { ptyIdsByLeafId: { 'leaf-a': 'pty-a', 'leaf-b': 'pty-b' } } + } + }) + const harness = await loadIpcEventsHarness(storeState) + harness.useIpcEvents() + + revealSplitPaneFromMobile(harness) + + expect(storeState.createTab).not.toHaveBeenCalled() + expect(harness.replyTerminalCreate).toHaveBeenCalledWith({ + requestId: 'mobile-reveal', + tabId: 'tab-split', + title: 'codex' + }) + }) + + it('adopts the owning tab when neither the live map nor the layout is hydrated', async () => { + // Same reveal, but layout hydration has not landed either; the pre-minted + // tabId hint is the PTY's baked-in pane key and must still be honoured. + const storeState: HarnessStoreState = createHarnessStoreState({ + tabsByWorktree: { [WORKTREE_ID]: [{ id: 'tab-split', ptyId: 'pty-a', title: 'codex' }] }, + ptyIdsByTabId: { 'tab-split': ['pty-a'] }, + terminalLayoutsByTabId: {} + }) + const harness = await loadIpcEventsHarness(storeState) + harness.useIpcEvents() + + revealSplitPaneFromMobile(harness) + + expect(storeState.createTab).not.toHaveBeenCalled() + expect(harness.replyTerminalCreate).toHaveBeenCalledWith({ + requestId: 'mobile-reveal', + tabId: 'tab-split', + title: 'codex' + }) + }) + + it('adopts the tab that records the pty when the reveal hint disagrees', async () => { + // The pane was dragged out of tab-split since the PTY's env was baked, so + // the reveal still names tab-split while tab-detached is where it now lives. + const storeState: HarnessStoreState = createHarnessStoreState({ + tabsByWorktree: { + [WORKTREE_ID]: [ + { id: 'tab-detached', ptyId: null, title: 'Terminal 1' }, + { id: 'tab-split', ptyId: 'pty-a', title: 'codex' } + ] + }, + ptyIdsByTabId: { 'tab-split': ['pty-a'] }, + terminalLayoutsByTabId: { + 'tab-detached': { ptyIdsByLeafId: { 'leaf-b': 'pty-b' } }, + 'tab-split': { ptyIdsByLeafId: { 'leaf-a': 'pty-a' } } + } + }) + const harness = await loadIpcEventsHarness(storeState) + harness.useIpcEvents() + + revealSplitPaneFromMobile(harness) + + expect(storeState.createTab).not.toHaveBeenCalled() + expect(harness.replyTerminalCreate).toHaveBeenCalledWith({ + requestId: 'mobile-reveal', + tabId: 'tab-detached', + title: 'codex' + }) + }) + + it('adopts through the persisted layout when the PTY carries no tab id', async () => { + const storeState: HarnessStoreState = createHarnessStoreState({ + tabsByWorktree: { [WORKTREE_ID]: [{ id: 'tab-split', ptyId: 'pty-a', title: 'codex' }] }, + ptyIdsByTabId: {}, + terminalLayoutsByTabId: { + 'tab-split': { ptyIdsByLeafId: { 'leaf-a': 'pty-a', 'leaf-b': 'pty-b' } } + } + }) + const harness = await loadIpcEventsHarness(storeState) + harness.useIpcEvents() + + harness.createTerminal({ + requestId: 'mobile-reveal', + worktreeId: WORKTREE_ID, + ptyId: 'pty-b', + leafId: 'leaf-b', + presentation: 'focused', + title: 'codex' + }) + + expect(storeState.createTab).not.toHaveBeenCalled() + expect(harness.replyTerminalCreate).toHaveBeenCalledWith({ + requestId: 'mobile-reveal', + tabId: 'tab-split', + title: 'codex' + }) + }) + + it('keeps a live binding authoritative over a stale layout row', async () => { + // No tabId hint: the live pty map still pins the session to its real tab. + const storeState: HarnessStoreState = createHarnessStoreState({ + tabsByWorktree: { + [WORKTREE_ID]: [ + { id: 'tab-stale', ptyId: null, title: 'Terminal 1' }, + { id: 'tab-live', ptyId: null, title: 'codex' } + ] + }, + ptyIdsByTabId: { 'tab-live': ['pty-b'] }, + terminalLayoutsByTabId: { 'tab-stale': { ptyIdsByLeafId: { 'leaf-x': 'pty-b' } } } + }) + const harness = await loadIpcEventsHarness(storeState) + harness.useIpcEvents() + + harness.createTerminal({ + requestId: 'mobile-reveal', + worktreeId: WORKTREE_ID, + ptyId: 'pty-b', + leafId: 'leaf-b', + presentation: 'focused', + title: 'codex' + }) + + expect(storeState.createTab).not.toHaveBeenCalled() + expect(harness.replyTerminalCreate).toHaveBeenCalledWith({ + requestId: 'mobile-reveal', + tabId: 'tab-live', + title: 'codex' + }) + }) + + it('leaves the hinted tab split intact when the pty lives in another tab', async () => { + // Adopting the hinted tab here would rewrite its layout with a single-pane + // snapshot, silently collapsing the user's split onto the revealed pty. + const storeState: HarnessStoreState = createHarnessStoreState({ + tabsByWorktree: { + [WORKTREE_ID]: [ + { id: 'tab-split', ptyId: null, title: 'codex' }, + { id: 'tab-detached', ptyId: null, title: 'Terminal 2' } + ] + }, + ptyIdsByTabId: {}, + terminalLayoutsByTabId: { + 'tab-split': { + root: { + type: 'split', + direction: 'horizontal', + first: { type: 'leaf', leafId: 'leaf-1' }, + second: { type: 'leaf', leafId: 'leaf-2' } + }, + ptyIdsByLeafId: { 'leaf-1': 'pty-1', 'leaf-2': 'pty-2' } + }, + 'tab-detached': { ptyIdsByLeafId: { 'leaf-b': 'pty-b' } } + } + }) + + const harness = await loadIpcEventsHarness(storeState) + harness.useIpcEvents() + + revealSplitPaneFromMobile(harness) + + expect(storeState.createTab).not.toHaveBeenCalled() + const layoutWrites = (storeState.setTabLayout as { mock: { calls: unknown[][] } }).mock.calls + expect(layoutWrites.map((call) => call[0])).not.toContain('tab-split') + expect(harness.replyTerminalCreate).toHaveBeenCalledWith({ + requestId: 'mobile-reveal', + tabId: 'tab-detached', + title: 'codex' + }) + }) + + 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. + const storeState: HarnessStoreState = createHarnessStoreState({ + tabsByWorktree: { + [WORKTREE_ID]: [ + { id: 'tab-stale-a', ptyId: 'pty-b', title: 'Terminal 1' }, + { id: 'tab-stale-b', ptyId: null, title: 'Terminal 2' } + ] + }, + ptyIdsByTabId: {}, + terminalLayoutsByTabId: { 'tab-stale-b': { ptyIdsByLeafId: { 'leaf-y': 'pty-b' } } } + }) + const harness = await loadIpcEventsHarness(storeState) + harness.useIpcEvents() + + harness.createTerminal({ + requestId: 'mobile-reveal', + worktreeId: WORKTREE_ID, + ptyId: 'pty-b', + leafId: 'leaf-b', + presentation: 'focused', + title: 'codex' + }) + + expect(harness.replyTerminalCreate).toHaveBeenCalledWith({ + requestId: 'mobile-reveal', + tabId: 'tab-minted', + title: 'codex' + }) + }) +}) diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index 68a94915300..65119eaa24e 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -15,6 +15,7 @@ import { OPEN_WORKSPACE_BOARD_EVENT } from '@/components/sidebar/useWorkspaceBoa import { SPLIT_TERMINAL_PANE_EVENT, CLOSE_TERMINAL_PANE_EVENT } from '@/constants/terminal' import { requestBackgroundTerminalWorktreeMount } from '@/components/terminal/background-terminal-worktree-mount' import { planMobileTerminalTabMount } from '@/lib/mobile-terminal-tab-mount' +import { resolveTerminalTabPtyOwnership } from '@/lib/terminal-tab-for-pty-id' import { hasRegisteredRuntimeTerminalTab, focusRuntimeTerminalSurface @@ -1411,13 +1412,20 @@ export function useIpcEvents(): void { activateTerminalInitiatedWorktree(store, worktreeId) } const worktreeTabs = store.tabsByWorktree[worktreeId] ?? [] - const existingTab = ptyId - ? worktreeTabs.find( - (candidate) => - candidate.ptyId === ptyId || - (store.ptyIdsByTabId[candidate.id] ?? []).includes(ptyId) + // 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, + worktreeId, + ptyId, + tabId !== undefined ? { preferTabId: tabId } : {} ) - : undefined + : { kind: 'none' as const } + const existingTab = + ownership.kind === 'owned' + ? worktreeTabs.find((candidate) => candidate.id === ownership.tabId) + : undefined const isSplitReveal = Boolean(ptyId && tabId && leafId && splitFromLeafId) const splitTargetTab = isSplitReveal ? worktreeTabs.find((candidate) => candidate.id === tabId) @@ -1425,18 +1433,7 @@ export function useIpcEvents(): void { if (isSplitReveal && !splitTargetTab) { throw new Error(`Terminal tab ${tabId} not found`) } - const hintedPendingTab = - ptyId && tabId && !isSplitReveal - ? worktreeTabs.find((candidate) => { - if (candidate.id !== tabId) { - return false - } - const candidatePtyIds = store.ptyIdsByTabId[candidate.id] ?? [] - return candidate.ptyId == null && candidatePtyIds.length === 0 - }) - : undefined - // Why: runtime fallback reveals a PTY for a renderer-created pending tab; adopt only when the hinted tab has no PTY yet. - const reusedTab = existingTab ?? splitTargetTab ?? hintedPendingTab + const reusedTab = existingTab ?? splitTargetTab const tab = reusedTab ?? (ptyId diff --git a/src/renderer/src/lib/mobile-terminal-tab-mount.test.ts b/src/renderer/src/lib/mobile-terminal-tab-mount.test.ts index 6529ebca17f..35bef7fec53 100644 --- a/src/renderer/src/lib/mobile-terminal-tab-mount.test.ts +++ b/src/renderer/src/lib/mobile-terminal-tab-mount.test.ts @@ -1,10 +1,9 @@ import { describe, expect, it, vi } from 'vitest' import type { AppState } from '@/store/types' import { planMobileTerminalTabMount } from './mobile-terminal-tab-mount' +import type { TerminalTabPtyOwnershipState } from './terminal-tab-for-pty-id' -type PlannerState = Pick - -function state(tabCount = 1): PlannerState { +function state(tabCount = 1): TerminalTabPtyOwnershipState { return { tabsByWorktree: { wt: Array.from({ length: tabCount }, (_, index) => ({ @@ -12,7 +11,8 @@ function state(tabCount = 1): PlannerState { ptyId: `wt@@${index}` })) } as unknown as AppState['tabsByWorktree'], - terminalLayoutsByTabId: {} + terminalLayoutsByTabId: {}, + ptyIdsByTabId: {} } } @@ -49,6 +49,22 @@ describe('planMobileTerminalTabMount', () => { expect(planMobileTerminalTabMount(s, { worktreeId: 'wt', ptyId: 'wt@@173' })).toBeNull() }) + it('mounts the tab whose pane is mounted when a stale layout row also claims the pty', () => { + const s = state(200) + s.ptyIdsByTabId['tab-173'] = ['wt@@173'] + s.terminalLayoutsByTabId['tab-199'] = { + root: null, + activeLeafId: null, + expandedLeafId: null, + ptyIdsByLeafId: { leaf: 'wt@@173' } + } + + expect(planMobileTerminalTabMount(s, { worktreeId: 'wt', ptyId: 'wt@@173' })).toEqual({ + worktreeId: 'wt', + tabIds: ['tab-173'] + }) + }) + it('does not mount a hidden worktree for a stale direct tab id', () => { const isTabMounted = vi.fn() diff --git a/src/renderer/src/lib/mobile-terminal-tab-mount.ts b/src/renderer/src/lib/mobile-terminal-tab-mount.ts index 28431cc5ad6..e4e034db28b 100644 --- a/src/renderer/src/lib/mobile-terminal-tab-mount.ts +++ b/src/renderer/src/lib/mobile-terminal-tab-mount.ts @@ -1,6 +1,8 @@ -import type { AppState } from '@/store/types' import type { BackgroundMountTerminalWorktreeDetail } from '@/constants/terminal' -import { resolveTerminalTabIdForPtyId } from './terminal-tab-for-pty-id' +import { + resolveTerminalTabIdForPtyId, + type TerminalTabPtyOwnershipState +} from './terminal-tab-for-pty-id' export type MobileTerminalTabMountRequest = { worktreeId: string @@ -14,7 +16,7 @@ type MobileTerminalTabMountOptions = { /** Why: exact-tab planning prevents a stale ptyId from mounting every saved xterm (#8597). */ export function planMobileTerminalTabMount( - state: Pick, + state: TerminalTabPtyOwnershipState, request: MobileTerminalTabMountRequest, options: MobileTerminalTabMountOptions = {} ): BackgroundMountTerminalWorktreeDetail | null { 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 4fdc698e79e..ed3f651a2d4 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 @@ -1,16 +1,21 @@ import { describe, expect, it } from 'vitest' -import { resolveTerminalTabIdForPtyId } from './terminal-tab-for-pty-id' +import { + resolveTerminalTabIdForPtyId, + resolveTerminalTabPtyOwnership, + type TerminalTabPtyOwnershipState +} from './terminal-tab-for-pty-id' import type { AppState } from '@/store/types' -type ResolverState = Pick - function state(partial: { tabs?: Record layouts?: Record }> -}): ResolverState { + livePtyIds?: Record +}): TerminalTabPtyOwnershipState { return { tabsByWorktree: (partial.tabs ?? {}) as unknown as AppState['tabsByWorktree'], - terminalLayoutsByTabId: (partial.layouts ?? {}) as unknown as AppState['terminalLayoutsByTabId'] + terminalLayoutsByTabId: (partial.layouts ?? + {}) as unknown as AppState['terminalLayoutsByTabId'], + ptyIdsByTabId: partial.livePtyIds ?? {} } } @@ -35,6 +40,14 @@ describe('resolveTerminalTabIdForPtyId', () => { expect(resolveTerminalTabIdForPtyId(s, 'wt', 'wt@@9')).toBe('tab-a') }) + it('matches a tab by a live pty binding with no saved layout yet', () => { + const s = state({ + tabs: { wt: [{ id: 'tab-a', ptyId: null }] }, + livePtyIds: { 'tab-a': ['wt@@9'] } + }) + expect(resolveTerminalTabIdForPtyId(s, 'wt', 'wt@@9')).toBe('tab-a') + }) + it('returns null when no tab owns the ptyId', () => { const s = state({ tabs: { wt: [{ id: 'tab-a', ptyId: 'wt@@1' }] } }) expect(resolveTerminalTabIdForPtyId(s, 'wt', 'wt@@nope')).toBeNull() @@ -58,3 +71,138 @@ describe('resolveTerminalTabIdForPtyId', () => { expect(resolveTerminalTabIdForPtyId(s, 'other', 'wt@@1')).toBeNull() }) }) + +describe('resolveTerminalTabPtyOwnership', () => { + const staleLayoutBesideMountedTab = state({ + tabs: { + wt: [ + { id: 'tab-stale', ptyId: null }, + { id: 'tab-mounted', ptyId: null } + ] + }, + layouts: { 'tab-stale': { ptyIdsByLeafId: { leaf1: 'wt@@1' } } }, + livePtyIds: { 'tab-mounted': ['wt@@1'] } + }) + + it('prefers a mounted pane over a stale layout row in another tab', () => { + expect(resolveTerminalTabPtyOwnership(staleLayoutBesideMountedTab, 'wt', 'wt@@1')).toEqual({ + kind: 'owned', + tabId: 'tab-mounted' + }) + }) + + it('keeps the mounted pane even when the hint names the stale tab', () => { + expect( + resolveTerminalTabPtyOwnership(staleLayoutBesideMountedTab, 'wt', 'wt@@1', { + preferTabId: 'tab-stale' + }) + ).toEqual({ kind: 'owned', tabId: 'tab-mounted' }) + }) + + it('keeps a sole wake hint over a tab id the PTY outgrew', () => { + // Why: a pane dragged to another tab moves tab.ptyId with it, but the id + // baked into the PTY env is written once at spawn and never rewritten. + const s = state({ + tabs: { + wt: [ + { id: 'tab-minted-in', ptyId: null }, + { id: 'tab-detached-to', ptyId: 'wt@@1' } + ] + } + }) + expect( + resolveTerminalTabPtyOwnership(s, 'wt', 'wt@@1', { preferTabId: 'tab-minted-in' }) + ).toEqual({ kind: 'owned', tabId: 'tab-detached-to' }) + }) + + it('keeps a sole layout row over a tab id the PTY outgrew', () => { + const s = state({ + tabs: { + wt: [ + { id: 'tab-minted-in', ptyId: null }, + { id: 'tab-detached-to', ptyId: null } + ] + }, + layouts: { 'tab-detached-to': { ptyIdsByLeafId: { leaf1: 'wt@@1' } } } + }) + expect( + resolveTerminalTabPtyOwnership(s, 'wt', 'wt@@1', { preferTabId: 'tab-minted-in' }) + ).toEqual({ kind: 'owned', tabId: 'tab-detached-to' }) + }) + + it('falls back to the pre-minted tab id when nothing records the ptyId', () => { + const s = state({ + tabs: { + wt: [ + { id: 'tab-other', ptyId: 'wt@@2' }, + { id: 'tab-hinted', ptyId: null } + ] + } + }) + expect(resolveTerminalTabPtyOwnership(s, 'wt', 'wt@@1', { preferTabId: 'tab-hinted' })).toEqual( + { kind: 'owned', tabId: 'tab-hinted' } + ) + }) + + it('ignores a hint that names no existing tab', () => { + const s = state({ + tabs: { wt: [{ id: 'tab-a', ptyId: null }] }, + layouts: { 'tab-a': { ptyIdsByLeafId: { leaf1: 'wt@@1' } } } + }) + expect(resolveTerminalTabPtyOwnership(s, 'wt', 'wt@@1', { preferTabId: 'tab-gone' })).toEqual({ + kind: 'owned', + tabId: 'tab-a' + }) + }) + + it('reports ambiguity when only recorded bindings claim the ptyId', () => { + const s = state({ + tabs: { + wt: [ + { id: 'tab-a', ptyId: 'wt@@1' }, + { id: 'tab-b', ptyId: null } + ] + }, + layouts: { 'tab-b': { ptyIdsByLeafId: { leaf2: 'wt@@1' } } } + }) + expect(resolveTerminalTabPtyOwnership(s, 'wt', 'wt@@1')).toEqual({ kind: 'ambiguous' }) + }) + + it('breaks a recorded ownership conflict with the pre-minted tab id', () => { + const s = state({ + tabs: { + wt: [ + { id: 'tab-a', ptyId: 'wt@@1' }, + { id: 'tab-b', ptyId: null } + ] + }, + layouts: { 'tab-b': { ptyIdsByLeafId: { leaf2: 'wt@@1' } } } + }) + expect(resolveTerminalTabPtyOwnership(s, 'wt', 'wt@@1', { preferTabId: 'tab-b' })).toEqual({ + kind: 'owned', + tabId: 'tab-b' + }) + }) + + it('breaks a mounted ownership conflict with the pre-minted tab id', () => { + const s = state({ + tabs: { + wt: [ + { id: 'tab-a', ptyId: null }, + { id: 'tab-b', ptyId: null } + ] + }, + livePtyIds: { 'tab-a': ['wt@@1'], 'tab-b': ['wt@@1'] } + }) + expect(resolveTerminalTabPtyOwnership(s, 'wt', 'wt@@1')).toEqual({ kind: 'ambiguous' }) + expect(resolveTerminalTabPtyOwnership(s, 'wt', 'wt@@1', { preferTabId: 'tab-b' })).toEqual({ + kind: 'owned', + tabId: 'tab-b' + }) + }) + + it('reports no owner when nothing binds the ptyId', () => { + const s = state({ tabs: { wt: [{ id: 'tab-a', ptyId: 'wt@@2' }] } }) + expect(resolveTerminalTabPtyOwnership(s, 'wt', 'wt@@1')).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 77ee3c29706..1c247e6b55e 100644 --- a/src/renderer/src/lib/terminal-tab-for-pty-id.ts +++ b/src/renderer/src/lib/terminal-tab-for-pty-id.ts @@ -1,27 +1,74 @@ import type { AppState } from '@/store/types' -/** Resolve a synthetic mobile handle's ptyId through persisted tab and split bindings. */ +export type TerminalTabPtyOwnershipState = Pick< + AppState, + 'tabsByWorktree' | 'terminalLayoutsByTabId' | 'ptyIdsByTabId' +> + +export type TerminalTabPtyOwnership = + | { kind: 'owned'; tabId: string } + | { kind: 'ambiguous' } + | { kind: 'none' } + +type TerminalTabPtyOwnershipOptions = { + /** Tab id baked into the PTY's env; a fallback and tie-break, not a binding. */ + preferTabId?: string +} + +/** + * 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 + * the PTY spawns and goes stale as soon as a pane moves between tabs. A mounted + * pane outranks a recorded one, and same-tier conflicts stay ambiguous. + */ +export function resolveTerminalTabPtyOwnership( + state: TerminalTabPtyOwnershipState, + worktreeId: string, + 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 preferredTabId = + options.preferTabId !== undefined && tabs.some((tab) => tab.id === options.preferTabId) + ? options.preferTabId + : undefined + const owners = mountedOwners.length > 0 ? mountedOwners : recordedOwners + if (owners.length === 1) { + return { kind: 'owned', tabId: owners[0]! } + } + if (owners.length > 1) { + // Why: stale duplicate ownership must not attach whichever hidden tab + // happens to appear first in persisted order. + return preferredTabId !== undefined && owners.includes(preferredTabId) + ? { kind: 'owned', tabId: preferredTabId } + : { kind: 'ambiguous' } + } + // Why: nothing records the PTY yet, so the tab it was minted against is the + // only thing left that keeps paneKey hook attribution intact (#10486). + return preferredTabId !== undefined ? { kind: 'owned', tabId: preferredTabId } : { kind: 'none' } +} + +/** Resolve a synthetic mobile handle's ptyId; null when unowned or ambiguous. */ export function resolveTerminalTabIdForPtyId( - state: Pick, + state: TerminalTabPtyOwnershipState, worktreeId: string, ptyId: string ): string | null { - const tabs = state.tabsByWorktree[worktreeId] ?? [] - let resolvedTabId: string | null = null - for (const tab of tabs) { - const ptyIdsByLeafId = state.terminalLayoutsByTabId[tab.id]?.ptyIdsByLeafId - const ownsPty = - tab.ptyId === ptyId || - (ptyIdsByLeafId !== undefined && Object.values(ptyIdsByLeafId).includes(ptyId)) - if (!ownsPty) { - continue - } - if (resolvedTabId && resolvedTabId !== tab.id) { - // Why: stale duplicate ownership must not attach whichever hidden tab - // happens to appear first in persisted order. - return null - } - resolvedTabId = tab.id - } - return resolvedTabId + const ownership = resolveTerminalTabPtyOwnership(state, worktreeId, ptyId) + return ownership.kind === 'owned' ? ownership.tabId : null }