diff --git a/src/renderer/src/components/terminal/terminal-tab-actions-unrouted-close.test.ts b/src/renderer/src/components/terminal/terminal-tab-actions-unrouted-close.test.ts new file mode 100644 index 00000000000..1c0b4b07801 --- /dev/null +++ b/src/renderer/src/components/terminal/terminal-tab-actions-unrouted-close.test.ts @@ -0,0 +1,85 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { toRemoteRuntimePtyId } from '../../../../shared/remote-runtime-pty-id' + +const { closeWebRuntimeSessionTabMock, getStateMock, isWebRuntimeSessionActiveMock } = vi.hoisted( + () => ({ + closeWebRuntimeSessionTabMock: vi.fn(), + getStateMock: vi.fn(), + isWebRuntimeSessionActiveMock: vi.fn() + }) +) + +vi.mock('@/store', () => ({ useAppStore: { getState: getStateMock } })) + +vi.mock('@/runtime/web-runtime-session', () => ({ + closeWebRuntimeSessionTab: closeWebRuntimeSessionTabMock, + isWebRuntimeSessionActive: isWebRuntimeSessionActiveMock, + toHostSessionTabId: (tabId: string) => tabId +})) + +vi.mock('@/runtime/web-session-tabs-sync', () => ({ + getLatestWebSessionTabsPublicationEpoch: () => null, + resolveHostSessionTabIdForWebSessionTab: () => null +})) + +import { closeTerminalTab } from './terminal-tab-actions' + +const WORKTREE_ID = 'repo-1::/srv/app' + +/** The worktree catalog has not hydrated yet, so the worktree itself names no host (#11308). */ +function unhydratedState(ptyId: string | null, closeTab = vi.fn()) { + return { + settings: { activeRuntimeEnvironmentId: null }, + repos: [], + worktreesByRepo: {}, + detectedWorktreesByRepo: {}, + tabsByWorktree: { [WORKTREE_ID]: [{ id: 'tab-1', ptyId, worktreeId: WORKTREE_ID }] }, + unifiedTabsByWorktree: {}, + ptyIdsByTabId: {}, + terminalLayoutsByTabId: {}, + lastKnownRelayPtyIdByTabId: {}, + deferredSshSessionIdsByTabId: {}, + pendingReconnectPtyIdByTabId: {}, + activeWorktreeId: null, + activeTabId: null, + openFiles: [], + closeTab, + setActiveTab: vi.fn() + } +} + +describe('closeTerminalTab before the worktree catalog names a host', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('sends the close to the host its PTY id names', () => { + isWebRuntimeSessionActiveMock.mockImplementation((id: string | null) => id === 'env-1') + const closeTab = vi.fn() + getStateMock.mockReturnValue(unhydratedState(toRemoteRuntimePtyId('h-1', 'env-1'), closeTab)) + const onCancel = vi.fn() + + closeTerminalTab('tab-1', { onCancel }) + + expect(onCancel).not.toHaveBeenCalled() + expect(closeWebRuntimeSessionTabMock).toHaveBeenCalledWith({ + worktreeId: WORKTREE_ID, + tabId: 'tab-1', + environmentId: 'env-1', + reason: 'user' + }) + }) + + it('still closes the tab locally when nothing names its host', () => { + isWebRuntimeSessionActiveMock.mockReturnValue(false) + const closeTab = vi.fn() + getStateMock.mockReturnValue(unhydratedState(null, closeTab)) + const onClosed = vi.fn() + + closeTerminalTab('tab-1', { onClosed }) + + expect(closeTab).toHaveBeenCalledWith('tab-1') + expect(onClosed).toHaveBeenCalled() + expect(closeWebRuntimeSessionTabMock).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/components/terminal/terminal-tab-actions.ts b/src/renderer/src/components/terminal/terminal-tab-actions.ts index 3013b921f70..489bbf01178 100644 --- a/src/renderer/src/components/terminal/terminal-tab-actions.ts +++ b/src/renderer/src/components/terminal/terminal-tab-actions.ts @@ -8,7 +8,7 @@ import { getLatestWebSessionTabsPublicationEpoch, resolveHostSessionTabIdForWebSessionTab } from '@/runtime/web-session-tabs-sync' -import { resolveTerminalWorktreeRoute } from '@/lib/terminal-worktree-route' +import { resolveTerminalCloseRoute } from '@/lib/terminal-close-route' import { guardPinnedTabClose, isUnifiedTabPinned, @@ -82,11 +82,9 @@ export function closeTerminalTab( return } const { worktreeId: owningWorktreeId, terminalTabId } = target - const worktreeRoute = resolveTerminalWorktreeRoute(state, owningWorktreeId) - if (!worktreeRoute) { - options?.onCancel?.() - return - } + // Why no early return: an unrouted close still prunes locally and reaches main's close record; + // teardown of a PTY whose host is unproven fails closed on its own. + const worktreeRoute = resolveTerminalCloseRoute(state, owningWorktreeId, terminalTabId) // Why: a pinned tab routes through the confirmation guard instead of closing // outright. `force` is the post-confirmation re-entry, which skips the guard. @@ -130,7 +128,7 @@ export function closeTerminalTab( return } - const runtimeEnvironmentId = worktreeRoute.runtimeEnvironmentId + const runtimeEnvironmentId = worktreeRoute?.runtimeEnvironmentId ?? null if (runtimeEnvironmentId && isWebRuntimeSessionActive(runtimeEnvironmentId)) { if (options?.reason === 'pty-exit') { // Why: stream exit is not host-tab closure; the HUB snapshot decides whether reconnect restores or removes this tab. diff --git a/src/renderer/src/lib/terminal-close-route.ts b/src/renderer/src/lib/terminal-close-route.ts new file mode 100644 index 00000000000..0232951a823 --- /dev/null +++ b/src/renderer/src/lib/terminal-close-route.ts @@ -0,0 +1,45 @@ +import { parseExecutionHostId } from '../../../shared/execution-host' +import { getPtyExecutionHost } from '../../../shared/terminal-execution-host' +import type { AppState } from '@/store/types' +import { collectPtyIdsForTab } from '@/store/slices/terminal-tab-retirement' +import { resolveTerminalWorktreeRoute, type TerminalWorktreeRoute } from './terminal-worktree-route' + +function routeForHost(hostId: string | null | undefined): TerminalWorktreeRoute | null { + const host = parseExecutionHostId(hostId) + if (host?.kind === 'runtime') { + return { runtimeEnvironmentId: host.environmentId } + } + return host ? { runtimeEnvironmentId: null } : null +} + +/** + * Which host a terminal tab's close goes to: the worktree catalog's route, else the one host the + * tab's own PTY ids name, else the host recorded on its unified tab. Null when nothing names one, + * e.g. before the catalog hydrates (#11308); per-PTY teardown still fails closed then. + */ +export function resolveTerminalCloseRoute( + state: AppState, + worktreeId: string, + terminalTabId: string +): TerminalWorktreeRoute | null { + const catalogRoute = resolveTerminalWorktreeRoute(state, worktreeId) + if (catalogRoute) { + return catalogRoute + } + const row = state.tabsByWorktree?.[worktreeId]?.find((tab) => tab.id === terminalTabId) + const ptyHosts = new Set( + collectPtyIdsForTab(state, terminalTabId, row?.ptyId ?? null) + .map(getPtyExecutionHost) + .filter((host) => host !== null) + ) + if (ptyHosts.size > 0) { + // Why: an id that names no host ('foreign') or two hosts proves only that it is not local. + const [host] = ptyHosts + return ptyHosts.size === 1 && host !== 'foreign' ? routeForHost(host) : null + } + const unified = state.unifiedTabsByWorktree?.[worktreeId]?.find( + (tab) => + tab.contentType === 'terminal' && (tab.entityId === terminalTabId || tab.id === terminalTabId) + ) + return routeForHost(unified?.executionHostId) +} diff --git a/src/renderer/src/store/slices/tabs-close-dangling-group.test.ts b/src/renderer/src/store/slices/tabs-close-dangling-group.test.ts new file mode 100644 index 00000000000..626b090ffd7 --- /dev/null +++ b/src/renderer/src/store/slices/tabs-close-dangling-group.test.ts @@ -0,0 +1,36 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { createTabsSliceMockApi } from './tabs-slice-test-harness' +import { createTestStore } from './store-test-helpers' + +vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } })) + +createTabsSliceMockApi() + +const WT = 'repo1::/tmp/feature' + +describe('closeUnifiedTab with a missing group record (#11308)', () => { + let store: ReturnType + + beforeEach(() => { + store = createTestStore() + }) + + it.each(['terminal', 'editor'] as const)('still removes a %s tab', (contentType) => { + const kept = store.getState().createUnifiedTab(WT, contentType, { id: `${contentType}-kept` }) + const closing = store.getState().createUnifiedTab(WT, contentType, { id: `${contentType}-x` }) + store.setState({ groupsByWorktree: { [WT]: [] } }) + + const result = store.getState().closeUnifiedTab(closing.id) + + expect(result).toEqual({ closedTabId: closing.id, wasLastTab: false, worktreeId: WT }) + expect(store.getState().unifiedTabsByWorktree[WT]?.map((tab) => tab.id)).toEqual([kept.id]) + }) + + it('reports the last tab when the worktree has no other tab', () => { + const only = store.getState().createUnifiedTab(WT, 'editor', { id: 'editor-only' }) + store.setState({ groupsByWorktree: { [WT]: [] } }) + + expect(store.getState().closeUnifiedTab(only.id)?.wasLastTab).toBe(true) + expect(store.getState().unifiedTabsByWorktree[WT]).toEqual([]) + }) +}) diff --git a/src/renderer/src/store/slices/tabs/tabs-close-actions.ts b/src/renderer/src/store/slices/tabs/tabs-close-actions.ts index 6af491e7a37..56ed2fb6623 100644 --- a/src/renderer/src/store/slices/tabs/tabs-close-actions.ts +++ b/src/renderer/src/store/slices/tabs/tabs-close-actions.ts @@ -34,12 +34,12 @@ export function createTabsCloseActions( } const { tab, worktreeId } = found const group = findGroupForTab(state.groupsByWorktree, worktreeId, tab.groupId) - if (!group) { - return null - } + // Why: a tab whose group record vanished must still close; its order is the worktree's tabs. + const groupOrder = + group?.tabOrder ?? (state.unifiedTabsByWorktree[worktreeId] ?? []).map((item) => item.id) if (tab.contentType === 'terminal' && !opts?.terminalRetirementHandled) { - const dedupedGroupOrder = dedupeTabOrder(group.tabOrder) + const dedupedGroupOrder = dedupeTabOrder(groupOrder) const wasLastTab = dedupeTabOrder(dedupedGroupOrder.filter((id) => id !== tabId)).length === 0 // Why: unified-only hydrated tabs still own provider sessions without a legacy row, so retire every terminal close by entity id. @@ -47,7 +47,7 @@ export function createTabsCloseActions( return { closedTabId: tabId, wasLastTab, worktreeId } } - const dedupedGroupOrder = dedupeTabOrder(group.tabOrder) + const dedupedGroupOrder = dedupeTabOrder(groupOrder) const remainingOrder = dedupeTabOrder(dedupedGroupOrder.filter((id) => id !== tabId)) const wasLastTab = remainingOrder.length === 0 if (tab.contentType === 'agent-session') { @@ -78,13 +78,13 @@ export function createTabsCloseActions( } // Why: on closing the active tab, walk the MRU stack to the previously-active tab; pickNextActiveTab falls back to the neighbor. const nextActiveTabId = - group.activeTabId === tabId + group?.activeTabId === tabId ? wasLastTab ? null : pickNextActiveTab(dedupedGroupOrder, group.recentTabIds, tabId) - : group.activeTabId + : (group?.activeTabId ?? null) const nextRecentTabIds = sanitizeRecentTabIds( - (group.recentTabIds ?? []).filter((id) => id !== tabId), + (group?.recentTabIds ?? []).filter((id) => id !== tabId), remainingOrder ) const terminalEntityId = tab.contentType === 'terminal' ? tab.entityId : null @@ -100,7 +100,7 @@ export function createTabsCloseActions( delete nextUnreadTerminalTabs[terminalEntityId] } let nextGroups = (current.groupsByWorktree[worktreeId] ?? []).map((candidate) => - candidate.id === group.id + candidate.id === group?.id ? { ...candidate, activeTabId: nextActiveTabId, @@ -111,7 +111,7 @@ export function createTabsCloseActions( ) let nextLayoutByWorktree = current.layoutByWorktree let nextActiveGroupIdByWorktree = current.activeGroupIdByWorktree - if (wasLastTab && current.layoutByWorktree[worktreeId] && nextGroups.length > 1) { + if (group && wasLastTab && current.layoutByWorktree[worktreeId] && nextGroups.length > 1) { nextGroups = nextGroups.filter((candidate) => candidate.id !== group.id) const collapsedState = collapseGroupLayout( current.layoutByWorktree, diff --git a/src/renderer/src/store/slices/terminal-tab-retirement.ts b/src/renderer/src/store/slices/terminal-tab-retirement.ts index 0276541b7da..0310c65ba7d 100644 --- a/src/renderer/src/store/slices/terminal-tab-retirement.ts +++ b/src/renderer/src/store/slices/terminal-tab-retirement.ts @@ -63,7 +63,7 @@ export function getTerminalPtyOwnershipIdentity( return JSON.stringify(['runtime', environmentId, remote.handle]) } -function collectPtyIdsForTab( +export function collectPtyIdsForTab( state: TerminalTabRetirementState, tabId: string, rowPtyId: string | null | undefined