diff --git a/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.structured-session.test.ts b/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.structured-session.test.ts index d1feb923255..39f7d134223 100644 --- a/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.structured-session.test.ts +++ b/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.structured-session.test.ts @@ -24,12 +24,16 @@ const store = vi.hoisted(() => ({ reconcileWorktreeTabModel: vi.fn(() => ({ renderableTabCount: 1 })), setActiveWorktree: mocks.setActiveWorktree, tabsByWorktree: {}, - unifiedTabsByWorktree: {} + unifiedTabsByWorktree: {} as Record })) vi.mock('react', async () => { const actual = await vi.importActual('react') - return { ...actual, useCallback: (callback: T) => callback } + return { + ...actual, + useCallback: (callback: T) => callback, + useMemo: (factory: () => T) => factory() + } }) vi.mock('../../store', () => ({ @@ -105,6 +109,7 @@ const AGENT_TAB = { beforeEach(() => { vi.clearAllMocks() + store.unifiedTabsByWorktree = { 'wt-1': [AGENT_TAB] } mocks.closeStructuredAgentSession.mockResolvedValue('closed') mocks.callRuntimeRpc.mockResolvedValue({ ok: true }) }) diff --git a/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.test.tsx b/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.test.tsx index b40edd5f773..bbe972f0a6a 100644 --- a/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.test.tsx +++ b/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.test.tsx @@ -245,3 +245,45 @@ describe('closing a browser workspace owned by more than one runtime environment expect(mocks.destroyWorkspaceWebviews).not.toHaveBeenCalled() }) }) + +describe('shared tab close policies', () => { + it('queues the dirty editor close without removing its tab or file', () => { + const editor = { + ...BROWSER_TAB, + id: 'editor-tab', + entityId: 'dirty-file', + contentType: 'editor' + } as Tab + useAppStore.setState({ + unifiedTabsByWorktree: { 'worktree-a': [editor] }, + openFiles: [{ id: 'dirty-file', isDirty: true, worktreeId: 'worktree-a' }] + } as never) + renderHook(() => + useTabGroupTabCloseCommands({ worktreeId: 'worktree-a', groupTabs: [editor] }) + ).result.current.closeItem(editor.id) + expect(mocks.requestEditorFileClose).toHaveBeenCalledWith('dirty-file') + expect(closeUnifiedTab).not.toHaveBeenCalled() + expect(useAppStore.getState().closeFile).not.toHaveBeenCalled() + }) + + it('shares terminal teardown while skipping running-process prompts only for bulk closes', () => { + const terminal = { + ...BROWSER_TAB, + id: 'terminal-tab', + entityId: 'terminal-entity', + contentType: 'terminal' + } as Tab + useAppStore.setState({ unifiedTabsByWorktree: { 'worktree-a': [terminal] } }) + const { result } = renderHook(() => + useTabGroupTabCloseCommands({ worktreeId: 'worktree-a', groupTabs: [terminal] }) + ) + result.current.closeItem(terminal.id) + expect(mocks.closeTerminalTab).toHaveBeenLastCalledWith('terminal-entity', { + onClosed: expect.any(Function) + }) + result.current.closeMany([terminal.id]) + expect(mocks.closeTerminalTab).toHaveBeenLastCalledWith('terminal-entity', { + skipRunningProcessConfirm: true + }) + }) +}) diff --git a/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.ts b/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.ts index a5a9a65455d..9f9d1de837a 100644 --- a/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.ts +++ b/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.ts @@ -1,28 +1,7 @@ -import { useCallback } from 'react' -import { toast } from 'sonner' +import { useMemo } from 'react' import type { Tab } from '../../../../shared/tab-types' -import { useAppStore } from '../../store' -import { destroyWorkspaceWebviews } from '../../store/slices/browser-webview-cleanup' -import { requestEditorFileClose } from '../editor/editor-autosave' -import { isWebRuntimeSessionActive } from '../../runtime/web-runtime-session' -import { closeTerminalTab } from '../terminal/terminal-tab-actions' -import { getRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner' -import { closeBrowserWorkspaceTabOnHosts } from '@/runtime/browser-workspace-tab-close' -import { callRuntimeRpc, getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client' -import { closeStructuredAgentSession } from '@/runtime/structured-agent-session-close' -import { cancelStructuredAgentLaunch } from '@/lib/structured-agent-session-launch' -import { toRuntimeWorktreeSelector } from '@/runtime/runtime-worktree-selector' -import { translate } from '@/i18n/i18n' - -function reportStructuredSessionCloseError(error: unknown): void { - toast.error( - translate( - 'components.native-chat.structuredSessionCloseFailed', - 'Could not close this chat session' - ), - { description: error instanceof Error ? error.message : String(error) } - ) -} +import { dispatchWorkspaceTabCommand } from '@/lib/workspace-tab-commands' +import { createWorkspaceTabCloseCommands } from './workspace-tab-close-commands' export function useTabGroupTabCloseCommands({ worktreeId, @@ -31,211 +10,27 @@ export function useTabGroupTabCloseCommands({ worktreeId: string groupTabs: Tab[] }) { - const closeUnifiedTab = useAppStore((state) => state.closeUnifiedTab) - const closeFile = useAppStore((state) => state.closeFile) - const closeBrowserTab = useAppStore((state) => state.closeBrowserTab) - const setActiveWorktree = useAppStore((state) => state.setActiveWorktree) - - const closeEditorIfUnreferenced = useCallback( - (entityId: string, closingTabId: string) => { - const otherReference = (useAppStore.getState().unifiedTabsByWorktree[worktreeId] ?? []).some( - (item) => - item.id !== closingTabId && - item.entityId === entityId && - (item.contentType === 'editor' || - item.contentType === 'diff' || - item.contentType === 'conflict-review' || - item.contentType === 'check-details') - ) - if (!otherReference) { - const file = useAppStore.getState().openFiles.find((candidate) => candidate.id === entityId) - if (file?.isDirty) { - // Why: route through Terminal.tsx so the unsaved-confirmation save/discard queue stays centralized across all close paths. - requestEditorFileClose(entityId) - return false - } - closeFile(entityId) - } - return true - }, - [closeFile, worktreeId] - ) - - const leaveWorktreeIfEmpty = useCallback(() => { - const state = useAppStore.getState() - if (state.activeWorktreeId !== worktreeId) { - return - } - // Why: split-group closes bypass legacy Terminal.tsx; deselect the emptied worktree here or the window goes blank instead of landing. - const { renderableTabCount } = state.reconcileWorktreeTabModel(worktreeId) - if (renderableTabCount === 0) { - setActiveWorktree(null) - } - }, [setActiveWorktree, worktreeId]) - - const closeBrowserItem = useCallback( - (item: Tab, focusedEnvironmentId: string | null | undefined) => { - const state = useAppStore.getState() - const plan = closeBrowserWorkspaceTabOnHosts({ - state, - worktreeId, - workspaceId: item.entityId, - visibleTabId: item.id, - focusedEnvironmentId - }) - // Why: both teardown sites take the reason. closeBrowserTab usually removes the visible tab - // itself, but when it cannot find it the bare call below is the one that runs — and a cleanup - // close that lands there without these options hands the user an empty-worktree landing. - const cleanupOptions = - plan.localCloseReason === 'cleanup' - ? { preserveWorktreeSelection: true, recordInteraction: false } - : undefined - if (plan.closesLocally) { - // Why before the teardown: closeBrowserTab announces the MRU page selection, and a guest - // torn down first leaves the fallback picking registration order instead (#16306). - closeBrowserTab( - item.entityId, - plan.localCloseReason ? { reason: plan.localCloseReason } : undefined - ) - destroyWorkspaceWebviews(state.browserPagesByWorkspace, item.entityId) - } - if (plan.removesVisibleTab) { - closeUnifiedTab(item.id, cleanupOptions) - } - return plan - }, - [closeBrowserTab, closeUnifiedTab, worktreeId] - ) - - const closeItem = useCallback( - (itemId: string, opts?: { skipEmptyCheck?: boolean }) => { - const item = groupTabs.find((candidate) => candidate.id === itemId) - if (!item) { - return - } - if (item.isPinned) { - return - } - const runtimeEnvironmentId = getRuntimeEnvironmentIdForWorktree( - useAppStore.getState(), - worktreeId - ) - if (item.contentType === 'agent-session') { - cancelStructuredAgentLaunch(worktreeId, item.entityId) - // Why: the structured session lives on the host, so the local tab close must also - // retire the host's canonical row or it reappears on the next sync. - // Cancel a still-reconciling create before closing its owner; otherwise a missing - // post-create snapshot is mistaken for an unknown outcome and retried after close. - cancelStructuredAgentLaunch(worktreeId, item.entityId) - const target = getActiveRuntimeTarget({ - activeRuntimeEnvironmentId: runtimeEnvironmentId + return useMemo( + () => ({ + closeItem: (tabId: string, opts?: { skipEmptyCheck?: boolean }) => { + dispatchWorkspaceTabCommand({ + type: 'close', + target: { kind: 'tab', worktreeId, tabId }, + ...opts }) - void closeStructuredAgentSession(target, item.entityId) - .then(() => - callRuntimeRpc(target, 'session.tabs.close', { - worktree: toRuntimeWorktreeSelector(worktreeId), - tabId: `agent-session:${item.entityId}`, - reason: 'user' - }) - ) - .then(() => { - closeUnifiedTab(item.id) - if (!opts?.skipEmptyCheck) { - leaveWorktreeIfEmpty() - } + }, + closeMany: (tabIds: string[]) => { + for (const tabId of tabIds) { + dispatchWorkspaceTabCommand({ + type: 'close', + target: { kind: 'tab', worktreeId, tabId }, + bulk: true }) - .catch(reportStructuredSessionCloseError) - return - } - if (item.contentType === 'terminal') { - // Why: closeTerminalTab can defer behind a pin / running-process dialog, so the - // empty check has to run on the actual close — never on cancel. - closeTerminalTab( - item.entityId, - opts?.skipEmptyCheck ? undefined : { onClosed: leaveWorktreeIfEmpty } - ) - return - } - if (item.contentType === 'browser') { - const plan = closeBrowserItem(item, runtimeEnvironmentId) - // Why: the empty check below answers "the user emptied this worktree". Unwinding a create - // that never finished is not that — it must leave the selection as the click found it. - if (!plan.closesLocally || plan.localCloseReason === 'cleanup') { - return } - } else if (item.contentType === 'simulator') { - closeUnifiedTab(item.id) - } else { - const canCloseTab = closeEditorIfUnreferenced(item.entityId, item.id) - if (!canCloseTab) { - return - } - closeUnifiedTab(item.id) - } - if (!opts?.skipEmptyCheck) { - leaveWorktreeIfEmpty() - } - }, - [ - closeBrowserItem, - closeEditorIfUnreferenced, - closeUnifiedTab, - groupTabs, - leaveWorktreeIfEmpty, - worktreeId - ] + }, + leaveWorktreeIfEmpty: createWorkspaceTabCloseCommands({ worktreeId, groupTabs }) + .leaveWorktreeIfEmpty + }), + [worktreeId, groupTabs] ) - - const closeMany = useCallback( - (itemIds: string[]) => { - for (const itemId of itemIds) { - const item = groupTabs.find((candidate) => candidate.id === itemId) - if (!item || item.isPinned) { - continue - } - const runtimeEnvironmentId = getRuntimeEnvironmentIdForWorktree( - useAppStore.getState(), - worktreeId - ) - if (item.contentType === 'agent-session') { - cancelStructuredAgentLaunch(worktreeId, item.entityId) - const target = getActiveRuntimeTarget({ - activeRuntimeEnvironmentId: runtimeEnvironmentId - }) - void closeStructuredAgentSession(target, item.entityId) - .then(() => - callRuntimeRpc(target, 'session.tabs.close', { - worktree: toRuntimeWorktreeSelector(worktreeId), - tabId: `agent-session:${item.entityId}`, - reason: 'user' - }) - ) - .then(() => closeUnifiedTab(item.id)) - .catch(reportStructuredSessionCloseError) - continue - } - if (item.contentType === 'terminal' && isWebRuntimeSessionActive(runtimeEnvironmentId)) { - // Why: revoke local resume + hook authority before the host removes its canonical tab. - // No running-process prompt: a bulk close of N busy tabs would be a modal storm. - closeTerminalTab(item.entityId, { skipRunningProcessConfirm: true }) - continue - } - if (item.contentType === 'browser') { - closeBrowserItem(item, runtimeEnvironmentId) - } else if (item.contentType === 'terminal') { - closeTerminalTab(item.entityId, { skipRunningProcessConfirm: true }) - } else if (item.contentType === 'simulator') { - closeUnifiedTab(item.id) - } else { - const canCloseTab = closeEditorIfUnreferenced(item.entityId, item.id) - if (canCloseTab) { - closeUnifiedTab(item.id) - } - } - } - }, - [closeBrowserItem, closeEditorIfUnreferenced, closeUnifiedTab, groupTabs, worktreeId] - ) - - return { closeItem, closeMany, leaveWorktreeIfEmpty } } diff --git a/src/renderer/src/components/tab-group/workspace-tab-close-commands.ts b/src/renderer/src/components/tab-group/workspace-tab-close-commands.ts new file mode 100644 index 00000000000..d7d0b24b91e --- /dev/null +++ b/src/renderer/src/components/tab-group/workspace-tab-close-commands.ts @@ -0,0 +1,138 @@ +import { toast } from 'sonner' +import type { Tab } from '../../../../shared/tab-types' +import { useAppStore } from '../../store' +import { requestEditorFileClose } from '../editor/editor-autosave' +import { closeTerminalTab } from '../terminal/terminal-tab-actions' +import { getRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner' +import { closeWorkspaceBrowserTab } from '@/lib/workspace-browser-tab-close' +import { callRuntimeRpc, getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client' +import { withLocalSessionTabCloseOwner } from '@/runtime/local-session-tab-close-owner' +import { closeStructuredAgentSession } from '@/runtime/structured-agent-session-close' +import { cancelStructuredAgentLaunch } from '@/lib/structured-agent-session-launch' +import { toRuntimeWorktreeSelector } from '@/runtime/runtime-worktree-selector' +import { translate } from '@/i18n/i18n' + +function reportStructuredSessionCloseError(error: unknown): void { + toast.error( + translate( + 'components.native-chat.structuredSessionCloseFailed', + 'Could not close this chat session' + ), + { description: error instanceof Error ? error.message : String(error) } + ) +} + +export function createWorkspaceTabCloseCommands({ + worktreeId, + groupTabs +}: { + worktreeId: string + groupTabs: Tab[] +}) { + const { closeUnifiedTab, closeFile, setActiveWorktree } = useAppStore.getState() + + const closeEditorIfUnreferenced = (entityId: string, closingTabId: string) => { + const otherReference = (useAppStore.getState().unifiedTabsByWorktree[worktreeId] ?? []).some( + (item) => + item.id !== closingTabId && + item.entityId === entityId && + (item.contentType === 'editor' || + item.contentType === 'diff' || + item.contentType === 'conflict-review' || + item.contentType === 'check-details') + ) + if (!otherReference) { + const file = useAppStore.getState().openFiles.find((candidate) => candidate.id === entityId) + if (file?.isDirty) { + // Why: route through Terminal.tsx so the unsaved-confirmation save/discard queue stays centralized across all close paths. + requestEditorFileClose(entityId) + return false + } + closeFile(entityId) + } + return true + } + + const leaveWorktreeIfEmpty = () => { + const state = useAppStore.getState() + if (state.activeWorktreeId !== worktreeId) { + return + } + // Why: split-group closes bypass legacy Terminal.tsx; deselect the emptied worktree here or the window goes blank instead of landing. + const { renderableTabCount } = state.reconcileWorktreeTabModel(worktreeId) + if (renderableTabCount === 0) { + setActiveWorktree(null) + } + } + + const closeItem = ( + itemId: string, + opts?: { skipEmptyCheck?: boolean; skipRunningProcessConfirm?: boolean } + ) => { + const item = groupTabs.find((candidate) => candidate.id === itemId) + if (!item) { + return + } + const runtimeEnvironmentId = getRuntimeEnvironmentIdForWorktree( + useAppStore.getState(), + worktreeId + ) + if (item.contentType === 'agent-session') { + // Cancel pending creation and retire the host session before removing its tab. + cancelStructuredAgentLaunch(worktreeId, item.entityId) + const target = getActiveRuntimeTarget({ + activeRuntimeEnvironmentId: runtimeEnvironmentId + }) + void closeStructuredAgentSession(target, item.entityId) + .then(() => { + const closeHostTab = () => + callRuntimeRpc(target, 'session.tabs.close', { + worktree: toRuntimeWorktreeSelector(worktreeId), + tabId: `agent-session:${item.entityId}`, + reason: 'user' + }) + return target.kind === 'local' + ? withLocalSessionTabCloseOwner(worktreeId, item.id, closeHostTab) + : closeHostTab() + }) + .then(() => { + closeUnifiedTab(item.id) + if (!opts?.skipEmptyCheck) { + leaveWorktreeIfEmpty() + } + }) + .catch(reportStructuredSessionCloseError) + return + } + if (item.contentType === 'terminal') { + // Why: closeTerminalTab can defer behind a pin / running-process dialog, so the + // empty check has to run on the actual close — never on cancel. + closeTerminalTab(item.entityId, { + ...(opts?.skipRunningProcessConfirm ? { skipRunningProcessConfirm: true } : {}), + ...(!opts?.skipEmptyCheck ? { onClosed: leaveWorktreeIfEmpty } : {}) + }) + return + } + if (item.contentType === 'browser') { + const plan = closeWorkspaceBrowserTab(worktreeId, item.entityId, item.id) + // Why: the empty check below answers "the user emptied this worktree". Unwinding a create + // that never finished is not that — it must leave the selection as the click found it. + if (!plan.closesLocally || plan.localCloseReason === 'cleanup') { + return + } + } else if (item.contentType === 'simulator') { + closeUnifiedTab(item.id) + } else { + const canCloseTab = closeEditorIfUnreferenced(item.entityId, item.id) + if (!canCloseTab) { + return + } + closeUnifiedTab(item.id) + } + if (!opts?.skipEmptyCheck) { + leaveWorktreeIfEmpty() + } + } + + return { closeItem, leaveWorktreeIfEmpty } +} diff --git a/src/renderer/src/components/terminal-workspace-keydown.test.ts b/src/renderer/src/components/terminal-workspace-keydown.test.ts index 015b9708a11..c26a5f70c7d 100644 --- a/src/renderer/src/components/terminal-workspace-keydown.test.ts +++ b/src/renderer/src/components/terminal-workspace-keydown.test.ts @@ -1,6 +1,10 @@ // @vitest-environment happy-dom import { beforeEach, describe, expect, it, vi } from 'vitest' +import { handleSwitchTabAcrossAllTypes } from '../hooks/ipc-tab-switch' +import { switchFloatingWorkspaceTab } from '@/lib/floating-workspace-terminal-actions' +import { dispatchWorkspaceTabCommand } from '@/lib/workspace-tab-commands' +import type { Tab } from '../../../shared/tab-types' import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants' import { ORCA_EDITOR_REQUEST_CMD_SAVE_EVENT, @@ -11,6 +15,10 @@ import type { TerminalActivationController } from './use-terminal-activation-act const mocks = vi.hoisted(() => ({ state: {} as Record, + closeTerminalTab: vi.fn(), + closeStructuredAgentSession: vi.fn(async () => 'closed'), + callRuntimeRpc: vi.fn(async () => ({ ok: true })), + cancelStructuredAgentLaunch: vi.fn(), floatingFocused: false, targetInsideFloatingPanel: false })) @@ -27,6 +35,7 @@ vi.mock('@/lib/floating-workspace-terminal-actions', () => ({ createFloatingWorkspaceMarkdownTab: vi.fn(), createFloatingWorkspaceTerminalTab: vi.fn(), handleEmptyFloatingWorkspacePanelCloseShortcut: () => false, + isEmptyFloatingWorkspacePanelVisible: () => false, isEventTargetInsideFloatingWorkspacePanel: () => mocks.targetInsideFloatingPanel, isFloatingWorkspacePanelFocused: () => mocks.floatingFocused, switchFloatingWorkspaceTab: vi.fn() @@ -38,6 +47,26 @@ vi.mock('./terminal-agent-tab-shortcut', () => ({ resolveTerminalAgentTabShortcut: () => ({ actionId: null, agent: null }) })) +vi.mock('./terminal/terminal-tab-actions', () => ({ closeTerminalTab: mocks.closeTerminalTab })) +vi.mock('@/runtime/structured-agent-session-close', () => ({ + closeStructuredAgentSession: mocks.closeStructuredAgentSession +})) +vi.mock('@/runtime/runtime-rpc-client', () => ({ + getActiveRuntimeTarget: () => ({ kind: 'local' }), + callRuntimeRpc: mocks.callRuntimeRpc +})) +vi.mock('@/lib/structured-agent-session-launch', () => ({ + cancelStructuredAgentLaunch: mocks.cancelStructuredAgentLaunch +})) +vi.mock('@/lib/worktree-runtime-owner', () => ({ getRuntimeEnvironmentIdForWorktree: () => null })) +vi.mock('@/runtime/runtime-worktree-selector', () => ({ + toRuntimeWorktreeSelector: (id: string) => `id:${id}` +})) +vi.mock('@/runtime/browser-workspace-tab-close', () => ({ + closeBrowserWorkspaceTabOnHosts: () => ({ closesLocally: true, removesVisibleTab: true }) +})) +vi.mock('../store/slices/browser-webview-cleanup', () => ({ destroyWorkspaceWebviews: vi.fn() })) + const controller = { activeWorktreeId: 'repo-1::/repo/worktree', handleCloseAllFiles: vi.fn(), @@ -102,3 +131,209 @@ describe('handleTerminalWorkspaceKeyDown editor.save', () => { expect(pressCmdS()).toEqual([]) }) }) + +describe('tab.close uses the unified active tab', () => { + const worktreeId = controller.activeWorktreeId! + let tab: Tab + const closeUnifiedTab = vi.fn() + const closeFile = vi.fn() + const closeBrowserTab = vi.fn() + + beforeEach(() => { + vi.clearAllMocks() + mocks.floatingFocused = false + mocks.targetInsideFloatingPanel = false + tab = { + id: 'chat-tab', + entityId: 'chat-session', + worktreeId, + groupId: 'right-group', + contentType: 'agent-session', + label: 'Chat', + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + mocks.state = { + activeView: 'terminal', + activeWorktreeId: worktreeId, + // The old terminal mirror can lag behind focus in a split group. + activeTabType: 'terminal', + activeTabId: 'other-terminal', + activeFileId: 'other-file', + activeBrowserTabId: 'other-browser', + getActiveTab: () => tab, + unifiedTabsByWorktree: { [worktreeId]: [tab] }, + openFiles: [], + browserPagesByWorkspace: {}, + closeUnifiedTab, + closeFile, + closeBrowserTab, + reconcileWorktreeTabModel: () => ({ renderableTabCount: 1 }), + requestPinnedTabCloseConfirm: vi.fn(), + setActiveWorktree: vi.fn() + } + }) + + function close(platform: NodeJS.Platform = 'darwin', terminalFocus = false): KeyboardEvent { + const target = document.createElement('textarea') + if (terminalFocus) { + target.classList.add('xterm-helper-textarea') + } + const event = new KeyboardEvent('keydown', { + key: 'w', + metaKey: platform === 'darwin', + ctrlKey: platform !== 'darwin', + cancelable: true + }) + Object.defineProperty(event, 'target', { value: target }) + handleTerminalWorkspaceKeyDown(event, controller, platform) + return event + } + + it.each(['darwin', 'win32', 'linux'] as const)( + 'closes native chat from its composer on %s', + async (platform) => { + expect(close(platform).defaultPrevented).toBe(true) + await vi.waitFor(() => expect(closeUnifiedTab).toHaveBeenCalledWith(tab.id)) + expect(mocks.closeStructuredAgentSession).toHaveBeenCalledWith( + { kind: 'local' }, + 'chat-session' + ) + expect(mocks.callRuntimeRpc).toHaveBeenCalledWith({ kind: 'local' }, 'session.tabs.close', { + worktree: `id:${worktreeId}`, + tabId: 'agent-session:chat-session', + reason: 'user' + }) + expect(mocks.cancelStructuredAgentLaunch).toHaveBeenCalledTimes(1) + expect(mocks.closeTerminalTab).not.toHaveBeenCalled() + } + ) + + it.each(['editor', 'diff', 'conflict-review', 'check-details', 'browser', 'simulator'] as const)( + 'closes the focused %s tab through the same command', + (contentType) => { + tab.contentType = contentType + close() + expect(closeUnifiedTab).toHaveBeenCalledWith( + tab.id, + ...(contentType === 'browser' ? [undefined] : []) + ) + expect(controller.handleCloseFile).not.toHaveBeenCalled() + expect(controller.handleCloseBrowserTab).not.toHaveBeenCalled() + } + ) + + it('leaves a focused terminal split pane to its pane close handler', () => { + tab.contentType = 'terminal' + expect(close('darwin', true).defaultPrevented).toBe(false) + expect(mocks.closeTerminalTab).not.toHaveBeenCalled() + }) + + it('closes a terminal tab from its native chat overlay through terminal teardown', () => { + tab.contentType = 'terminal' + tab.viewMode = 'chat' + close() + expect(mocks.closeTerminalTab).toHaveBeenCalledWith(tab.entityId, { + onClosed: expect.any(Function) + }) + }) + + it('waits for pinned-tab confirmation before closing', async () => { + tab.isPinned = true + close() + expect(mocks.closeStructuredAgentSession).not.toHaveBeenCalled() + expect(closeUnifiedTab).not.toHaveBeenCalled() + const request = vi.mocked(mocks.state.requestPinnedTabCloseConfirm as ReturnType) + .mock.calls[0][0] + request.onConfirm() + await vi.waitFor(() => expect(closeUnifiedTab).toHaveBeenCalledWith(tab.id)) + }) + + it('does not close the main workspace tab while the floating panel owns focus', () => { + mocks.floatingFocused = true + expect(close().defaultPrevented).toBe(false) + expect(mocks.closeStructuredAgentSession).not.toHaveBeenCalled() + }) + + it('preserves pinned tabs during a bulk close without prompting', () => { + tab.isPinned = true + dispatchWorkspaceTabCommand({ + type: 'close', + target: { kind: 'tab', worktreeId, tabId: tab.id }, + bulk: true + }) + expect(mocks.state.requestPinnedTabCloseConfirm).not.toHaveBeenCalled() + expect(mocks.closeStructuredAgentSession).not.toHaveBeenCalled() + }) + + it('does not substitute the ambient tab for a stale explicit target', () => { + expect( + dispatchWorkspaceTabCommand({ + type: 'close', + target: { kind: 'tab', worktreeId, tabId: 'removed' } + }) + ).toBe(false) + expect(mocks.closeStructuredAgentSession).not.toHaveBeenCalled() + }) + + it('does not close another group from an empty focused split', () => { + mocks.state.getActiveTab = () => null + mocks.state.activeGroupIdByWorktree = { [worktreeId]: 'empty-group' } + mocks.state.groupsByWorktree = { [worktreeId]: [{ id: 'empty-group', activeTabId: null }] } + tab.contentType = 'terminal' + mocks.state.activeTabId = tab.entityId + expect(close().defaultPrevented).toBe(false) + expect(mocks.closeTerminalTab).not.toHaveBeenCalled() + }) + + it('closes only the focused copy of an editor shared between split groups', () => { + tab.contentType = 'editor' + mocks.state.unifiedTabsByWorktree = { + [worktreeId]: [tab, { ...tab, id: 'left-copy', groupId: 'left-group' }] + } + close() + expect(closeUnifiedTab).toHaveBeenCalledWith(tab.id) + expect(closeFile).not.toHaveBeenCalled() + }) +}) + +describe('shared tab navigation routing', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.floatingFocused = false + mocks.targetInsideFloatingPanel = false + mocks.state = { activeWorktreeId: controller.activeWorktreeId } + }) + + function nextTab(): void { + handleTerminalWorkspaceKeyDown( + new KeyboardEvent('keydown', { + key: ']', + metaKey: true, + shiftKey: true, + cancelable: true + }), + controller, + 'darwin' + ) + } + + it('sends the keyboard and IPC navigation intent to the same workspace operation', () => { + nextTab() + dispatchWorkspaceTabCommand({ type: 'switch', direction: 1, scope: 'all-types' }) + expect(handleSwitchTabAcrossAllTypes).toHaveBeenCalledTimes(2) + expect(handleSwitchTabAcrossAllTypes).toHaveBeenLastCalledWith(1) + expect(switchFloatingWorkspaceTab).not.toHaveBeenCalled() + }) + + it('routes both entry points to the floating panel when it owns focus', () => { + mocks.floatingFocused = true + nextTab() + dispatchWorkspaceTabCommand({ type: 'switch', direction: 1, scope: 'all-types' }) + expect(switchFloatingWorkspaceTab).toHaveBeenCalledTimes(2) + expect(switchFloatingWorkspaceTab).toHaveBeenLastCalledWith(mocks.state, 1, 'all-types') + expect(handleSwitchTabAcrossAllTypes).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/components/terminal-workspace-keydown.ts b/src/renderer/src/components/terminal-workspace-keydown.ts index dd6e1e70878..72deb34f6b4 100644 --- a/src/renderer/src/components/terminal-workspace-keydown.ts +++ b/src/renderer/src/components/terminal-workspace-keydown.ts @@ -1,22 +1,16 @@ import { toast } from 'sonner' +import { dispatchWorkspaceTabCommand } from '@/lib/workspace-tab-commands' import type { KeybindingActionId } from '../../../shared/keybindings' import { keybindingMatchesAction } from '../../../shared/keybindings' import { matchesRecentTabSwitcherChord } from '../../../shared/window-shortcut-policy' import { useAppStore } from '../store' -import { - handleSwitchRecentTab, - handleSwitchTab, - handleSwitchTabAcrossAllTypes, - handleSwitchTerminalTab -} from '../hooks/ipc-tab-switch' import { createFloatingWorkspaceBrowserTab, createFloatingWorkspaceMarkdownTab, createFloatingWorkspaceTerminalTab, handleEmptyFloatingWorkspacePanelCloseShortcut, isEventTargetInsideFloatingWorkspacePanel, - isFloatingWorkspacePanelFocused, - switchFloatingWorkspaceTab + isFloatingWorkspacePanelFocused } from '@/lib/floating-workspace-terminal-actions' import { showTerminalShortcutCaptureNotification } from '@/lib/terminal-shortcut-capture-notification' import { @@ -38,8 +32,6 @@ export function handleTerminalWorkspaceKeyDown( const { activeWorktreeId, handleCloseAllFiles, - handleCloseBrowserTab, - handleCloseFile, handleNewAgentTab, handleNewBrowserTab, handleNewFile, @@ -180,16 +172,9 @@ export function handleTerminalWorkspaceKeyDown( if (floatingPanelOwnsEvent) { return } - const state = useAppStore.getState() - if (state.activeTabType === 'terminal' && context === 'terminal') { - return - } - event.preventDefault() - notifyTerminalCapture('tab.close') - if (state.activeTabType === 'editor' && state.activeFileId) { - handleCloseFile(state.activeFileId) - } else if (state.activeTabType === 'browser' && state.activeBrowserTabId) { - handleCloseBrowserTab(state.activeBrowserTabId) + if (dispatchWorkspaceTabCommand({ type: 'close', context })) { + event.preventDefault() + notifyTerminalCapture('tab.close') } return } @@ -211,7 +196,7 @@ export function handleTerminalWorkspaceKeyDown( event.preventDefault() event.stopPropagation() event.stopImmediatePropagation() - handleSwitchRecentTab() + dispatchWorkspaceTabCommand({ type: 'previous-recent' }) return } const switchSameTypeDirection = matchShortcut('tab.nextSameType') @@ -237,17 +222,11 @@ export function handleTerminalWorkspaceKeyDown( ? 'tab.nextSameType' : 'tab.previousSameType' ) - if (floatingWorkspaceFocused) { - switchFloatingWorkspaceTab( - useAppStore.getState(), - switchAllTypesDirection ?? switchSameTypeDirection ?? 1, - switchAllTypesDirection !== null ? 'all-types' : 'same-type' - ) - } else if (switchAllTypesDirection !== null) { - handleSwitchTabAcrossAllTypes(switchAllTypesDirection) - } else { - handleSwitchTab(switchSameTypeDirection ?? 1) - } + dispatchWorkspaceTabCommand({ + type: 'switch', + direction: switchAllTypesDirection ?? switchSameTypeDirection ?? 1, + scope: switchAllTypesDirection !== null ? 'all-types' : 'same-type' + }) } const terminalTabDirection = matchShortcut('tab.nextTerminal') ? 1 @@ -258,10 +237,10 @@ export function handleTerminalWorkspaceKeyDown( event.preventDefault() event.stopPropagation() event.stopImmediatePropagation() - if (floatingWorkspaceFocused) { - switchFloatingWorkspaceTab(useAppStore.getState(), terminalTabDirection, 'terminal') - } else { - handleSwitchTerminalTab(terminalTabDirection) - } + dispatchWorkspaceTabCommand({ + type: 'switch', + direction: terminalTabDirection, + scope: 'terminal' + }) } } diff --git a/src/renderer/src/components/use-terminal-close-actions.ts b/src/renderer/src/components/use-terminal-close-actions.ts index 4ade6ce88a6..3df2a6601a3 100644 --- a/src/renderer/src/components/use-terminal-close-actions.ts +++ b/src/renderer/src/components/use-terminal-close-actions.ts @@ -3,95 +3,21 @@ import { useAppStore } from '../store' import { isProvenProcessExit } from '../../../shared/terminal-exit-cause' import { closeTerminalTab } from './terminal/terminal-tab-actions' import { shouldDeferParkedPtyExitTabClose } from './terminal-pane/terminal-parked-tab-watchers' -import { destroyWorkspaceWebviews } from '../store/slices/browser-webview-cleanup' -import { closeBrowserWorkspaceTabOnHosts } from '@/runtime/browser-workspace-tab-close' -import { - getActiveWorktreeRuntimeEnvironmentId, - isPinnedVisibleTab -} from './terminal-workspace-model' +import { dispatchWorkspaceTabCommand } from '@/lib/workspace-tab-commands' import type { TerminalCreateController } from './use-terminal-create-actions' export function useTerminalCloseActions(controller: TerminalCreateController) { - const { - closeBrowserTab, - consumeSuppressedPtyExit, - setActiveFile, - setActiveTab, - setActiveTabType, - setActiveWorktree - } = controller + const { consumeSuppressedPtyExit } = controller const handleCloseTab = useCallback((tabId: string) => { closeTerminalTab(tabId) }, []) - const handleCloseBrowserTab = useCallback( - (tabId: string) => { - const state = useAppStore.getState() - const owningWorktreeEntry = Object.entries(state.browserTabsByWorktree).find( - ([, worktreeTabs]) => worktreeTabs.some((tab) => tab.id === tabId) - ) - const owningWorktreeId = owningWorktreeEntry?.[0] ?? null - if (!owningWorktreeId) { - return - } - if (isPinnedVisibleTab(state, owningWorktreeId, tabId)) { - return - } - const plan = closeBrowserWorkspaceTabOnHosts({ - state, - worktreeId: owningWorktreeId, - workspaceId: tabId, - visibleTabId: tabId, - focusedEnvironmentId: getActiveWorktreeRuntimeEnvironmentId(owningWorktreeId) - }) - if (!plan.closesLocally) { - if (plan.removesVisibleTab) { - const mirroredTab = (state.unifiedTabsByWorktree[owningWorktreeId] ?? []).find( - (candidate) => candidate.contentType === 'browser' && candidate.entityId === tabId - ) - if (mirroredTab) { - state.closeUnifiedTab(mirroredTab.id) - } - } - return - } - const closeOptions = plan.localCloseReason ? { reason: plan.localCloseReason } : undefined - const currentTabs = state.browserTabsByWorktree[owningWorktreeId] ?? [] - if (currentTabs.length <= 1) { - const hasUnifiedEntry = Object.values(state.unifiedTabsByWorktree).some((tabs) => - tabs.some((tab) => tab.contentType === 'browser' && tab.entityId === tabId) - ) - closeBrowserTab(tabId, closeOptions) - // closeBrowserTab announces the MRU target before guest teardown can trigger bridge fallback. - destroyWorkspaceWebviews(state.browserPagesByWorkspace, tabId) - // Why: the fallback below answers "the user emptied this worktree". Unwinding a create - // that never finished is not that, so it must leave the selection as the click found it. - if (plan.localCloseReason === 'cleanup') { - return - } - if (!hasUnifiedEntry && state.activeWorktreeId === owningWorktreeId) { - const worktreeFile = state.openFiles.find((file) => file.worktreeId === owningWorktreeId) - if (worktreeFile) { - setActiveFile(worktreeFile.id) - setActiveTabType('editor') - } else { - const terminalTab = (state.tabsByWorktree[owningWorktreeId] ?? [])[0] - if (terminalTab) { - setActiveTab(terminalTab.id) - setActiveTabType('terminal') - } else { - setActiveWorktree(null) - } - } - } - return - } - closeBrowserTab(tabId, closeOptions) - // closeBrowserTab announces the MRU target before guest teardown can trigger bridge fallback. - destroyWorkspaceWebviews(state.browserPagesByWorkspace, tabId) - }, - [closeBrowserTab, setActiveFile, setActiveTab, setActiveTabType, setActiveWorktree] - ) + const handleCloseBrowserTab = useCallback((tabId: string) => { + dispatchWorkspaceTabCommand({ + type: 'close', + target: { kind: 'browser-source', sourceId: tabId } + }) + }, []) const handlePtyExit = useCallback( (tabId: string, ptyId: string, exitCode?: number) => { diff --git a/src/renderer/src/components/use-terminal-keyboard-shortcuts.ts b/src/renderer/src/components/use-terminal-keyboard-shortcuts.ts index cbd621575d5..d7cbff68b13 100644 --- a/src/renderer/src/components/use-terminal-keyboard-shortcuts.ts +++ b/src/renderer/src/components/use-terminal-keyboard-shortcuts.ts @@ -5,11 +5,7 @@ import type { TerminalActivationController } from './use-terminal-activation-act export function useTerminalKeyboardShortcuts(controller: TerminalActivationController): void { const { activeWorktreeId, - closeBrowserTab, handleCloseAllFiles, - handleCloseBrowserTab, - handleCloseFile, - handleCloseTab, handleNewAgentTab, handleNewBrowserTab, handleNewFile, @@ -42,10 +38,6 @@ export function useTerminalKeyboardShortcuts(controller: TerminalActivationContr handleNewFile, handleNewTab, handleNewAgentTab, - handleCloseTab, - handleCloseBrowserTab, - closeBrowserTab, - handleCloseFile, handleCloseAllFiles, keybindings, mobileEmulatorEnabled, diff --git a/src/renderer/src/hooks/ipc-events-close-routing-test-harness.ts b/src/renderer/src/hooks/ipc-events-close-routing-test-harness.ts index 69cc9383556..72e9683382c 100644 --- a/src/renderer/src/hooks/ipc-events-close-routing-test-harness.ts +++ b/src/renderer/src/hooks/ipc-events-close-routing-test-harness.ts @@ -53,6 +53,9 @@ export async function useIpcEventsForCloseRouting({ respondTerminalTabClose?: ReturnType persistWorkspaceSession?: ReturnType }): Promise { + if (typeof HTMLElement === 'undefined') { + vi.stubGlobal('HTMLElement', class {}) + } vi.doMock('react', async () => { const actual = await vi.importActual('react') return { @@ -67,6 +70,9 @@ export async function useIpcEventsForCloseRouting({ useAppStore: { subscribe: vi.fn(() => () => {}), getState: () => ({ + getActiveTab: () => null, + closeUnifiedTab: vi.fn(), + reconcileWorktreeTabModel: () => ({ renderableTabCount: 1 }), setUpdateStatus: vi.fn(), fetchRepos: vi.fn(), fetchWorktrees: vi.fn(), diff --git a/src/renderer/src/hooks/ipc-events/session-tab-ipc-bridge.ts b/src/renderer/src/hooks/ipc-events/session-tab-ipc-bridge.ts index b0b284fbd79..47464072034 100644 --- a/src/renderer/src/hooks/ipc-events/session-tab-ipc-bridge.ts +++ b/src/renderer/src/hooks/ipc-events/session-tab-ipc-bridge.ts @@ -1,3 +1,4 @@ +import { isLocalSessionTabCloseOwned } from '@/runtime/local-session-tab-close-owner' import { closeMobileSessionTabInStore } from '@/runtime/mobile-session-tab-close' import { SESSION_TAB_CLOSE_CANCELED_ERROR, @@ -16,6 +17,9 @@ import { resolveBrowserSessionTabTarget } from './browser-session-tab-target' export function registerSessionTabIpcBridge(unsubs: (() => void)[]): void { unsubs.push( window.api.ui.onCloseSessionTab(({ tabId, worktreeId }) => { + if (isLocalSessionTabCloseOwned(worktreeId, tabId)) { + return + } const store = useAppStore.getState() const browserTarget = resolveBrowserSessionTabTarget(store, worktreeId, tabId) if (browserTarget) { @@ -80,6 +84,14 @@ export function registerSessionTabIpcBridge(unsubs: (() => void)[]): void { respond(error instanceof Error ? error.message : SESSION_TAB_CLOSE_FAILED_ERROR) } } + if (isLocalSessionTabCloseOwned(worktreeId, tabId)) { + respond( + expiresAt !== undefined && Date.now() >= expiresAt + ? SESSION_TAB_CLOSE_TIMEOUT_ERROR + : undefined + ) + return + } const visibleId = browserTarget?.workspaceId ?? tabId cancelConfirmation = guardPinnedTabClose({ isPinned: isUnifiedTabPinned(store, worktreeId, visibleId), diff --git a/src/renderer/src/hooks/ipc-events/tab-lifecycle-ipc-bridge.ts b/src/renderer/src/hooks/ipc-events/tab-lifecycle-ipc-bridge.ts index bf60e9dc0a1..1661f1906db 100644 --- a/src/renderer/src/hooks/ipc-events/tab-lifecycle-ipc-bridge.ts +++ b/src/renderer/src/hooks/ipc-events/tab-lifecycle-ipc-bridge.ts @@ -4,20 +4,11 @@ import { createWebRuntimeSessionTerminal, isWebRuntimeSessionActive } from '@/runtime/web-runtime-session' -import { closeBrowserWorkspaceTabOnHosts } from '@/runtime/browser-workspace-tab-close' -import { destroyWorkspaceWebviews } from '@/store/slices/browser-webview-cleanup' -import { - guardPinnedTabClose, - isUnifiedTabPinned, - resolvePinnedTabLabel -} from '../../store/pinned-tab-close-guard' -import { TOGGLE_FLOATING_TERMINAL_EVENT } from '@/lib/floating-terminal' +import { dispatchWorkspaceTabCommand } from '@/lib/workspace-tab-commands' import { createFloatingWorkspaceTerminalTab, - isEmptyFloatingWorkspacePanelVisible, isFloatingWorkspacePanelFocused, - resolveFloatingWorkspaceBrowserWorkspaceId, - switchFloatingWorkspaceTab + resolveFloatingWorkspaceBrowserWorkspaceId } from '@/lib/floating-workspace-terminal-actions' import { dispatchFloatingWorkspaceGuestClose, @@ -25,13 +16,6 @@ import { } from '@/lib/floating-workspace-guest-bridge' import { useAppStore } from '../../store' -import { resolveBrowserWorkspaceOwner } from '../../lib/browser-workspace-source-resolution' -import { - handleSwitchRecentTab, - handleSwitchTab, - handleSwitchTabAcrossAllTypes, - handleSwitchTerminalTab -} from '../ipc-tab-switch' function getWorktreeRuntimeEnvironmentId(worktreeId: string | null | undefined): string | null { return getRuntimeEnvironmentIdForWorktree(useAppStore.getState(), worktreeId) } @@ -88,73 +72,12 @@ export function registerTabLifecycleIpcBridge(unsubs: (() => void)[]): void { unsubs.push( window.api.ui.onCloseActiveTab((payload) => { - // Why: the empty-panel toggle is the ambient fallback only. A guest-originated close names a - // main-workspace target, so an open-but-empty floating panel must not swallow it. - if (!payload?.sourceId && isEmptyFloatingWorkspacePanelVisible()) { - window.dispatchEvent(new Event(TOGGLE_FLOATING_TERMINAL_EVENT)) - return - } - const store = useAppStore.getState() - // Why: a guest-originated close names its own page; the activeTabType mirror goes stale in - // split layouts (guest focus never reaches the group's focus-capture), so trust the source id. - const explicitTarget = payload?.sourceId - ? resolveBrowserWorkspaceOwner(store, payload.sourceId) - : null - if (payload?.sourceId && !explicitTarget) { - // Stale id (guest closed between keydown and IPC) = no-op, never the ambient fallback. - return - } - if (explicitTarget || (store.activeTabType === 'browser' && store.activeBrowserTabId)) { - const tabId = explicitTarget?.workspaceId ?? store.activeBrowserTabId - const worktreeId = explicitTarget?.worktreeId ?? store.activeWorktreeId - if (!tabId) { - return - } - const closeActiveBrowserTab = (): void => { - const currentStore = useAppStore.getState() - if (!worktreeId) { - currentStore.closeBrowserTab(tabId) - return - } - // Why: the menu's Close Tab used to decide ownership itself — "runtime connected means - // the host owns it" — which fires an inert close at a local-only or still-staged tab. - // The shared plan is the one authority on who tears a browser workspace down. - const plan = closeBrowserWorkspaceTabOnHosts({ - state: currentStore, - worktreeId, - workspaceId: tabId, - visibleTabId: tabId, - focusedEnvironmentId: getRuntimeEnvironmentIdForWorktree(currentStore, worktreeId) - }) - if (plan.closesLocally) { - // Why before the teardown: closeBrowserTab announces the MRU page selection, and a - // guest torn down first leaves the fallback picking registration order (#16306). - currentStore.closeBrowserTab( - tabId, - plan.localCloseReason ? { reason: plan.localCloseReason } : undefined - ) - destroyWorkspaceWebviews(currentStore.browserPagesByWorkspace, tabId) - return - } - if (plan.removesVisibleTab) { - const mirroredTab = (currentStore.unifiedTabsByWorktree[worktreeId] ?? []).find( - (candidate) => candidate.contentType === 'browser' && candidate.entityId === tabId - ) - if (mirroredTab) { - currentStore.closeUnifiedTab(mirroredTab.id) - } - } - } - if (worktreeId && isUnifiedTabPinned(store, worktreeId, tabId)) { - guardPinnedTabClose({ - isPinned: true, - tabLabel: resolvePinnedTabLabel(store, worktreeId, tabId), - onClose: closeActiveBrowserTab - }) - return - } - closeActiveBrowserTab() - } + dispatchWorkspaceTabCommand({ + type: 'close', + ...(payload?.sourceId + ? { target: { kind: 'browser-source', sourceId: payload.sourceId } as const } + : {}) + }) }) ) @@ -181,33 +104,16 @@ export function registerTabLifecycleIpcBridge(unsubs: (() => void)[]): void { unsubs.push( window.api.ui.onSwitchTab((direction) => { - const store = useAppStore.getState() - if (isFloatingWorkspacePanelFocused()) { - switchFloatingWorkspaceTab(store, direction, 'same-type') - return - } - handleSwitchTab(direction) - }) - ) - unsubs.push( + dispatchWorkspaceTabCommand({ type: 'switch', direction, scope: 'same-type' }) + }), window.api.ui.onSwitchTabAcrossAllTypes((direction) => { - const store = useAppStore.getState() - if (isFloatingWorkspacePanelFocused()) { - switchFloatingWorkspaceTab(store, direction, 'all-types') - return - } - handleSwitchTabAcrossAllTypes(direction) - }) - ) - unsubs.push(window.api.ui.onSwitchRecentTab(handleSwitchRecentTab)) - unsubs.push( + dispatchWorkspaceTabCommand({ type: 'switch', direction, scope: 'all-types' }) + }), + window.api.ui.onSwitchRecentTab(() => { + dispatchWorkspaceTabCommand({ type: 'previous-recent' }) + }), window.api.ui.onSwitchTerminalTab((direction) => { - const store = useAppStore.getState() - if (isFloatingWorkspacePanelFocused()) { - switchFloatingWorkspaceTab(store, direction, 'terminal') - return - } - handleSwitchTerminalTab(direction) + dispatchWorkspaceTabCommand({ type: 'switch', direction, scope: 'terminal' }) }) ) } diff --git a/src/renderer/src/hooks/useIpcEvents-close-routing-active-browser-tab.test.ts b/src/renderer/src/hooks/useIpcEvents-close-routing-active-browser-tab.test.ts index 4225e309427..d0eee5d5d2f 100644 --- a/src/renderer/src/hooks/useIpcEvents-close-routing-active-browser-tab.test.ts +++ b/src/renderer/src/hooks/useIpcEvents-close-routing-active-browser-tab.test.ts @@ -94,7 +94,7 @@ describe('useIpcEvents Close Tab on the active browser tab', () => { expect(closeWebRuntimeSessionTab).toHaveBeenCalledWith({ worktreeId: 'wt-1', - tabId: 'workspace-1', + tabId: 'unified-1', environmentId: 'env-a', reason: 'user' }) @@ -146,11 +146,11 @@ describe('useIpcEvents Close Tab on the active browser tab', () => { expect(closeWebRuntimeSessionTab).toHaveBeenCalledWith({ worktreeId: 'wt-1', - tabId: 'workspace-1', + tabId: 'unified-1', environmentId: 'env-a', reason: 'user' }) - expect(closeUnifiedTab).toHaveBeenCalledWith('unified-1') + expect(closeUnifiedTab).toHaveBeenCalledWith('unified-1', undefined) expect(closeBrowserTab).not.toHaveBeenCalled() }) @@ -181,7 +181,7 @@ describe('useIpcEvents Close Tab on the active browser tab', () => { requireListener(listenerRef)() expect(closeWebRuntimeSessionTab).not.toHaveBeenCalled() - expect(closeUnifiedTab).not.toHaveBeenCalled() + expect(closeUnifiedTab).toHaveBeenCalledWith('unified-1', undefined) expect(closeBrowserTab).toHaveBeenCalledWith('workspace-1', undefined) }) diff --git a/src/renderer/src/hooks/useIpcEvents-session-tab-close-request.test.ts b/src/renderer/src/hooks/useIpcEvents-session-tab-close-request.test.ts index ffd6b36d28b..c9905a3d408 100644 --- a/src/renderer/src/hooks/useIpcEvents-session-tab-close-request.test.ts +++ b/src/renderer/src/hooks/useIpcEvents-session-tab-close-request.test.ts @@ -44,6 +44,39 @@ describe('useIpcEvents session tab close requests', () => { expect(respondSessionTabClose).toHaveBeenCalledWith({ requestId: 'close-session-tab' }) }) + it('acknowledges an initiating command without re-prompting or removing its tab early', async () => { + const listenerRef: { current: SessionTabCloseRequestListener | null } = { current: null } + const closeUnifiedTab = vi.fn() + const respondSessionTabClose = vi.fn() + const requestPinnedTabCloseConfirm = vi.fn() + await useIpcEventsForCloseRouting({ + sessionTabCloseRequestListenerRef: listenerRef, + respondSessionTabClose, + getState: () => ({ + closeUnifiedTab, + requestPinnedTabCloseConfirm, + browserTabsByWorktree: {}, + openFiles: [], + unifiedTabsByWorktree: { + 'wt-1': [ + { id: 'chat-tab', entityId: 'session-1', contentType: 'agent-session', isPinned: true } + ] + } + }) + }) + const { withLocalSessionTabCloseOwner } = + await import('@/runtime/local-session-tab-close-owner') + await withLocalSessionTabCloseOwner('wt-1', 'chat-tab', async () => { + listenerRef.current?.({ requestId: 'owned-close', worktreeId: 'wt-1', tabId: 'chat-tab' }) + expect(respondSessionTabClose).toHaveBeenCalledWith({ requestId: 'owned-close' }) + expect(closeUnifiedTab).not.toHaveBeenCalled() + expect(requestPinnedTabCloseConfirm).not.toHaveBeenCalled() + }) + listenerRef.current?.({ requestId: 'independent-close', worktreeId: 'wt-1', tabId: 'chat-tab' }) + expect(requestPinnedTabCloseConfirm).toHaveBeenCalledOnce() + expect(respondSessionTabClose).not.toHaveBeenCalledWith({ requestId: 'independent-close' }) + }) + it('rejects a pinned browser close when confirmation is canceled', async () => { const listenerRef: { current: SessionTabCloseRequestListener | null } = { current: null } const closeBrowserTab = vi.fn() diff --git a/src/renderer/src/lib/workspace-browser-tab-close.ts b/src/renderer/src/lib/workspace-browser-tab-close.ts new file mode 100644 index 00000000000..3f6e0138248 --- /dev/null +++ b/src/renderer/src/lib/workspace-browser-tab-close.ts @@ -0,0 +1,33 @@ +import { useAppStore } from '@/store' +import { destroyWorkspaceWebviews } from '@/store/slices/browser-webview-cleanup' +import { getRuntimeEnvironmentIdForWorktree } from './worktree-runtime-owner' +import { closeBrowserWorkspaceTabOnHosts } from '@/runtime/browser-workspace-tab-close' + +export function closeWorkspaceBrowserTab(worktreeId: string, workspaceId: string, tabId?: string) { + const state = useAppStore.getState() + const { closeBrowserTab, closeUnifiedTab } = state + const plan = closeBrowserWorkspaceTabOnHosts({ + state, + worktreeId, + workspaceId, + visibleTabId: tabId ?? workspaceId, + focusedEnvironmentId: getRuntimeEnvironmentIdForWorktree(state, worktreeId) + }) + // Cleanup closes must preserve workspace selection at both teardown sites. + const cleanupOptions = + plan.localCloseReason === 'cleanup' + ? { preserveWorktreeSelection: true, recordInteraction: false } + : undefined + if (plan.closesLocally) { + // Announce the MRU page selection before guest teardown triggers focus fallback. + closeBrowserTab( + workspaceId, + plan.localCloseReason ? { reason: plan.localCloseReason } : undefined + ) + destroyWorkspaceWebviews(state.browserPagesByWorkspace, workspaceId) + } + if (plan.removesVisibleTab && tabId) { + closeUnifiedTab(tabId, cleanupOptions) + } + return plan +} diff --git a/src/renderer/src/lib/workspace-tab-commands.ts b/src/renderer/src/lib/workspace-tab-commands.ts new file mode 100644 index 00000000000..5000a5ba9a0 --- /dev/null +++ b/src/renderer/src/lib/workspace-tab-commands.ts @@ -0,0 +1,183 @@ +import type { KeybindingContext } from '../../../shared/keybindings' +import { toVisibleTabType, type Tab } from '../../../shared/tab-types' +import { useAppStore } from '@/store' +import { guardPinnedTabClose, resolvePinnedTabLabel } from '@/store/pinned-tab-close-guard' +import { createWorkspaceTabCloseCommands } from '@/components/tab-group/workspace-tab-close-commands' +import { + handleSwitchRecentTab, + handleSwitchTab, + handleSwitchTabAcrossAllTypes, + handleSwitchTerminalTab +} from '@/hooks/ipc-tab-switch' +import { + isEmptyFloatingWorkspacePanelVisible, + isFloatingWorkspacePanelFocused, + switchFloatingWorkspaceTab +} from './floating-workspace-terminal-actions' +import { TOGGLE_FLOATING_TERMINAL_EVENT } from './floating-terminal' +import { closeWorkspaceBrowserTab } from './workspace-browser-tab-close' +import { resolveBrowserWorkspaceOwner } from './browser-workspace-source-resolution' + +export type WorkspaceTabTarget = + | { kind: 'tab'; worktreeId: string; tabId: string } + | { kind: 'browser-source'; sourceId: string } + +export type WorkspaceTabCommand = + | { + type: 'close' + target?: WorkspaceTabTarget + context?: KeybindingContext + skipEmptyCheck?: boolean + bulk?: boolean + } + | { type: 'switch'; direction: number; scope: 'same-type' | 'all-types' | 'terminal' } + | { type: 'previous-recent' } + +type TabState = ReturnType + +function hasFocusedGroup(state: TabState, worktreeId: string): boolean { + const groupId = state.activeGroupIdByWorktree?.[worktreeId] + return (state.groupsByWorktree?.[worktreeId] ?? []).some((group) => group.id === groupId) +} + +function resolveActiveTab(state: TabState, worktreeId: string): Tab | null { + const activeTab = state.getActiveTab(worktreeId) + if (activeTab) { + return activeTab + } + if (hasFocusedGroup(state, worktreeId)) { + return null + } + // Hydration can publish the backing selection before its group selection. + const entityId = + state.activeTabType === 'browser' + ? state.activeBrowserTabId + : state.activeTabType === 'editor' + ? state.activeFileId + : state.activeTabId + return ( + (state.unifiedTabsByWorktree[worktreeId] ?? []).find( + (tab) => + toVisibleTabType(tab.contentType) === state.activeTabType && tab.entityId === entityId + ) ?? null + ) +} + +function resolveCloseTarget( + state: TabState, + target?: WorkspaceTabTarget +): { worktreeId: string; tab: Tab | null; browserWorkspaceId?: string } | null { + if (target?.kind === 'tab') { + const tab = (state.unifiedTabsByWorktree[target.worktreeId] ?? []).find( + (tab) => tab.id === target.tabId + ) + return tab ? { worktreeId: target.worktreeId, tab } : null + } + if (target?.kind === 'browser-source') { + const owner = resolveBrowserWorkspaceOwner(state, target.sourceId) + if (!owner) { + return null + } + const tab = (state.unifiedTabsByWorktree[owner.worktreeId] ?? []).find( + (tab) => tab.contentType === 'browser' && tab.entityId === owner.workspaceId + ) + return { worktreeId: owner.worktreeId, tab: tab ?? null, browserWorkspaceId: owner.workspaceId } + } + if (!state.activeWorktreeId) { + return null + } + const tab = resolveActiveTab(state, state.activeWorktreeId) + if (tab) { + return { worktreeId: state.activeWorktreeId, tab } + } + if ( + !hasFocusedGroup(state, state.activeWorktreeId) && + state.activeTabType === 'browser' && + state.activeBrowserTabId + ) { + return resolveCloseTarget(state, { kind: 'browser-source', sourceId: state.activeBrowserTabId }) + } + return null +} + +/** Input adapters describe intent; targeting and tab operations live here. */ +export function dispatchWorkspaceTabCommand(command: WorkspaceTabCommand): boolean { + const state = useAppStore.getState() + if (command.type === 'close') { + if (!command.target) { + if (isEmptyFloatingWorkspacePanelVisible()) { + window.dispatchEvent(new Event(TOGGLE_FLOATING_TERMINAL_EVENT)) + return true + } + if (isFloatingWorkspacePanelFocused()) { + return false + } + } + const target = resolveCloseTarget(state, command.target) + if (!target) { + return false + } + if (!target.tab) { + if (!target.browserWorkspaceId) { + return false + } + const plan = closeWorkspaceBrowserTab(target.worktreeId, target.browserWorkspaceId) + if ( + plan.closesLocally && + plan.localCloseReason !== 'cleanup' && + !command.skipEmptyCheck && + !command.bulk + ) { + createWorkspaceTabCloseCommands({ + worktreeId: target.worktreeId, + groupTabs: [] + }).leaveWorktreeIfEmpty() + } + return true + } + const tab = target.tab + if (command.context === 'terminal' && tab.contentType === 'terminal') { + return false + } + const commands = createWorkspaceTabCloseCommands({ + worktreeId: target.worktreeId, + groupTabs: state.unifiedTabsByWorktree[target.worktreeId] ?? [] + }) + if ((command.bulk || command.skipEmptyCheck) && tab.isPinned) { + return true + } + const close = () => + commands.closeItem(tab.id, { + skipEmptyCheck: command.bulk || command.skipEmptyCheck, + skipRunningProcessConfirm: command.bulk + }) + if (tab.contentType === 'terminal' || command.bulk) { + close() + } else { + guardPinnedTabClose({ + isPinned: tab.isPinned === true, + tabLabel: resolvePinnedTabLabel(state, target.worktreeId, tab.id), + onClose: close + }) + } + return true + } + if (command.type === 'previous-recent') { + if (isFloatingWorkspacePanelFocused()) { + return false + } + return handleSwitchRecentTab() + } + if (isFloatingWorkspacePanelFocused()) { + switchFloatingWorkspaceTab(state, command.direction, command.scope) + return true + } + switch (command.scope) { + case 'same-type': + return handleSwitchTab(command.direction) + case 'all-types': + return handleSwitchTabAcrossAllTypes(command.direction) + case 'terminal': + return handleSwitchTerminalTab(command.direction) + } +} diff --git a/src/renderer/src/runtime/local-session-tab-close-owner.test.ts b/src/renderer/src/runtime/local-session-tab-close-owner.test.ts new file mode 100644 index 00000000000..4e4b8d84bc7 --- /dev/null +++ b/src/renderer/src/runtime/local-session-tab-close-owner.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from 'vitest' +import { + isLocalSessionTabCloseOwned, + withLocalSessionTabCloseOwner +} from './local-session-tab-close-owner' + +function deferred(): { promise: Promise; resolve: () => void } { + let resolve!: () => void + const promise = new Promise((done) => { + resolve = done + }) + return { promise, resolve } +} + +describe('local session tab close ownership', () => { + it('scopes ownership to the exact workspace and tab until host acknowledgement', async () => { + const host = deferred() + const closing = withLocalSessionTabCloseOwner('folder:one', 'chat', () => host.promise) + expect(isLocalSessionTabCloseOwned('folder:one', 'chat')).toBe(true) + expect(isLocalSessionTabCloseOwned('folder:two', 'chat')).toBe(false) + expect(isLocalSessionTabCloseOwned('folder:one', 'other-chat')).toBe(false) + host.resolve() + await closing + expect(isLocalSessionTabCloseOwned('folder:one', 'chat')).toBe(false) + }) + + it('releases ownership on host failure so a later close must be authorized again', async () => { + await expect( + withLocalSessionTabCloseOwner('wt', 'chat', async () => { + throw new Error('host unavailable') + }) + ).rejects.toThrow('host unavailable') + expect(isLocalSessionTabCloseOwned('wt', 'chat')).toBe(false) + }) + + it('keeps overlapping handoffs owned until both have settled', async () => { + const first = deferred() + const second = deferred() + const firstClose = withLocalSessionTabCloseOwner('wt', 'chat', () => first.promise) + const secondClose = withLocalSessionTabCloseOwner('wt', 'chat', () => second.promise) + first.resolve() + await firstClose + expect(isLocalSessionTabCloseOwned('wt', 'chat')).toBe(true) + second.resolve() + await secondClose + expect(isLocalSessionTabCloseOwned('wt', 'chat')).toBe(false) + }) +}) diff --git a/src/renderer/src/runtime/local-session-tab-close-owner.ts b/src/renderer/src/runtime/local-session-tab-close-owner.ts new file mode 100644 index 00000000000..193af3920a8 --- /dev/null +++ b/src/renderer/src/runtime/local-session-tab-close-owner.ts @@ -0,0 +1,29 @@ +const owners = new Map() + +function closeKey(worktreeId: string, tabId: string): string { + return JSON.stringify([worktreeId, tabId]) +} + +export function isLocalSessionTabCloseOwned(worktreeId: string, tabId: string): boolean { + return owners.has(closeKey(worktreeId, tabId)) +} + +/** The initiating command owns UI removal; its local host relay only acknowledges the handoff. */ +export async function withLocalSessionTabCloseOwner( + worktreeId: string, + tabId: string, + close: () => Promise +): Promise { + const key = closeKey(worktreeId, tabId) + owners.set(key, (owners.get(key) ?? 0) + 1) + try { + return await close() + } finally { + const remaining = (owners.get(key) ?? 1) - 1 + if (remaining === 0) { + owners.delete(key) + } else { + owners.set(key, remaining) + } + } +}