From 876838551d3d6dfbf44699c67cdc6b4d2d2197ab Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 25 Aug 2026 00:33:29 -0700 Subject: [PATCH] feat(terminal): ask before bulk-closing tabs with running work Close Others / Close Tabs To The Right / Close Tabs To The Left / Close split pane closed busy terminals silently: every bulk path opted out of the running-process prompt because one modal per busy tab would have been a modal storm. Add a bulk guard that probes every terminal in the set in parallel and raises a single aggregated confirmation naming each tab that reported a live child. Idle sets stay synchronous, so nothing changes when there is nothing running. Cancel abandons the whole close rather than leaving a partial, un-undoable result; an unanswered probe asks instead of closing, matching the single-tab guard's fail-safe. --- src/renderer/src/components/Terminal.tsx | 114 ++++---- .../FloatingTerminalPanel.tsx | 76 ++++-- .../useTabGroupCloseScopeCommands.ts | 35 ++- ...abGroupTabCloseCommands.bulk-close.test.ts | 138 ++++++++++ .../tab-group/useTabGroupTabCloseCommands.ts | 106 ++++---- .../CloseTerminalDialog.test.tsx | 45 ++++ .../terminal-pane/CloseTerminalDialog.tsx | 97 +++++-- .../RunningTerminalCloseDialog.tsx | 1 + .../bulk-terminal-close-guard.test.ts | 254 ++++++++++++++++++ .../terminal/bulk-terminal-close-guard.ts | 181 +++++++++++++ .../terminal/running-terminal-close-guard.ts | 7 +- .../terminal/terminal-tab-actions.ts | 5 +- .../terminal/terminal-tab-bulk-actions.ts | 106 +++++--- src/renderer/src/i18n/locales/en.json | 11 + .../store/running-terminal-close-confirm.ts | 4 + 15 files changed, 976 insertions(+), 204 deletions(-) create mode 100644 src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.bulk-close.test.ts create mode 100644 src/renderer/src/components/terminal/bulk-terminal-close-guard.test.ts create mode 100644 src/renderer/src/components/terminal/bulk-terminal-close-guard.ts diff --git a/src/renderer/src/components/Terminal.tsx b/src/renderer/src/components/Terminal.tsx index 49bc620914a..e03e3c07ef5 100644 --- a/src/renderer/src/components/Terminal.tsx +++ b/src/renderer/src/components/Terminal.tsx @@ -182,6 +182,10 @@ import { showTerminalShortcutCaptureNotification } from '@/lib/terminal-shortcut import { useContextualTour } from './contextual-tours/use-contextual-tour' import { openTabBarEntry, type TabCreateEntryArgs } from './tab-bar/tab-create-entry-action' import { closeTerminalTab } from './terminal/terminal-tab-actions' +import { + collectBulkTerminalTabIds, + guardBulkTerminalClose +} from './terminal/bulk-terminal-close-guard' import { translate } from '@/i18n/i18n' import { getRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner' import { getResolvedExecutionHostIdForWorktree } from '@/lib/resolved-worktree-execution-host' @@ -1835,61 +1839,69 @@ function Terminal(): React.JSX.Element | null { if (!activeWorktreeId) { return } - const state = useAppStore.getState() - const dirtyFileIds: string[] = [] - for (const id of tabIds) { - const unifiedTab = (state.unifiedTabsByWorktree[activeWorktreeId] ?? []).find( - (candidate) => candidate.id === id || candidate.entityId === id - ) - if (unifiedTab?.isPinned) { - continue - } - const runtimeEnvironmentId = getActiveWorktreeRuntimeEnvironmentId(activeWorktreeId) - if ( - isWebRuntimeSessionActive(runtimeEnvironmentId) && - (unifiedTab?.contentType === 'terminal' || - (unifiedTab?.contentType === 'browser' && - browserWorkspaceHasRemoteOwner(state, unifiedTab.entityId, runtimeEnvironmentId))) - ) { - if (unifiedTab.contentType === 'terminal') { - // Why: paired-host bulk close must revoke renderer resume and hook authority, not just remove the host session tab. - // No running-process prompt: "Close Others" over N busy tabs would be a modal storm. - closeTerminalTab(unifiedTab.entityId, { skipRunningProcessConfirm: true }) - } else { - void closeWebRuntimeSessionTab({ - worktreeId: activeWorktreeId, - tabId: unifiedTab.id, - environmentId: runtimeEnvironmentId, - reason: 'user' - }) - } - continue - } - if ((state.tabsByWorktree[activeWorktreeId] ?? []).some((tab) => tab.id === id)) { - closeTab(id) - } else if ( - state.openFiles.some((file) => file.worktreeId === activeWorktreeId && file.id === id) - ) { - const file = state.openFiles.find((candidate) => candidate.id === id) - if (file?.isDirty) { - dirtyFileIds.push(id) + const worktreeId = activeWorktreeId + // Why: re-read state inside the proceed closure — the aggregated running-process + // prompt can sit open arbitrarily long before this actually runs. + const performClose = (): void => { + const state = useAppStore.getState() + const dirtyFileIds: string[] = [] + for (const id of tabIds) { + const unifiedTab = (state.unifiedTabsByWorktree[worktreeId] ?? []).find( + (candidate) => candidate.id === id || candidate.entityId === id + ) + if (unifiedTab?.isPinned) { continue } - closeFile(id) - } else if ( - (state.browserTabsByWorktree[activeWorktreeId] ?? []).some((tab) => tab.id === id) - ) { - destroyWorkspaceWebviews(state.browserPagesByWorkspace, id) - closeBrowserTab(id) - } else if (unifiedTab?.contentType === 'simulator') { - // Why: simulator tabs live only in the unified-tab store, so the - // entity-store checks above never match them. - state.closeUnifiedTab(unifiedTab.id) + const runtimeEnvironmentId = getActiveWorktreeRuntimeEnvironmentId(worktreeId) + if ( + isWebRuntimeSessionActive(runtimeEnvironmentId) && + (unifiedTab?.contentType === 'terminal' || + (unifiedTab?.contentType === 'browser' && + browserWorkspaceHasRemoteOwner(state, unifiedTab.entityId, runtimeEnvironmentId))) + ) { + if (unifiedTab.contentType === 'terminal') { + // Why: paired-host bulk close must revoke renderer resume and hook authority, not just remove the host session tab. + // The single aggregated prompt already ran above, so skip the per-tab one. + closeTerminalTab(unifiedTab.entityId, { skipRunningProcessConfirm: true }) + } else { + void closeWebRuntimeSessionTab({ + worktreeId, + tabId: unifiedTab.id, + environmentId: runtimeEnvironmentId, + reason: 'user' + }) + } + continue + } + if ((state.tabsByWorktree[worktreeId] ?? []).some((tab) => tab.id === id)) { + closeTab(id) + } else if ( + state.openFiles.some((file) => file.worktreeId === worktreeId && file.id === id) + ) { + const file = state.openFiles.find((candidate) => candidate.id === id) + if (file?.isDirty) { + dirtyFileIds.push(id) + continue + } + closeFile(id) + } else if ((state.browserTabsByWorktree[worktreeId] ?? []).some((tab) => tab.id === id)) { + destroyWorkspaceWebviews(state.browserPagesByWorkspace, id) + closeBrowserTab(id) + } else if (unifiedTab?.contentType === 'simulator') { + // Why: simulator tabs live only in the unified-tab store, so the + // entity-store checks above never match them. + state.closeUnifiedTab(unifiedTab.id) + } + } + if (dirtyFileIds.length > 0) { + queueEditorCloseRequests(dirtyFileIds) } } - if (dirtyFileIds.length > 0) { - queueEditorCloseRequests(dirtyFileIds) - } + guardBulkTerminalClose({ + worktreeId, + terminalTabIds: collectBulkTerminalTabIds(useAppStore.getState(), worktreeId, tabIds), + onProceed: performClose + }) }, [activeWorktreeId, closeBrowserTab, closeFile, closeTab, queueEditorCloseRequests] ) diff --git a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx index f37a6968c36..0866e5ea431 100644 --- a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx +++ b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx @@ -59,6 +59,10 @@ import { type FloatingWorkspaceGuestSelectIndexDetail } from '@/lib/floating-workspace-guest-bridge' import { closeTerminalTab } from '@/components/terminal/terminal-tab-actions' +import { + collectBulkTerminalTabIds, + guardBulkTerminalClose +} from '@/components/terminal/bulk-terminal-close-guard' import { guardPinnedTabClose, resolvePinnedTabLabel } from '@/store/pinned-tab-close-guard' import { extractIpcErrorMessage } from '@/lib/ipc-error' import { getShortcutPlatform } from '@/lib/shortcut-platform' @@ -879,39 +883,55 @@ export function FloatingTerminalPanel({ const closeFloatingItems = useCallback( (visibleIds: string[]) => { - const state = useAppStore.getState() - const currentGroupTabs = activeGroup - ? (state.unifiedTabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? []).filter( - (tab) => tab.groupId === activeGroup.id - ) - : (state.unifiedTabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? []) - const items = visibleIds - .map((visibleId) => resolveGroupTabFromVisibleId(currentGroupTabs, visibleId)) - .filter((item): item is Tab => item !== null && !item.isPinned) - if (items.length === 0) { + const resolveItems = (): Tab[] => { + const latest = useAppStore.getState() + const currentGroupTabs = activeGroup + ? (latest.unifiedTabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? []).filter( + (tab) => tab.groupId === activeGroup.id + ) + : (latest.unifiedTabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? []) + return visibleIds + .map((visibleId) => resolveGroupTabFromVisibleId(currentGroupTabs, visibleId)) + .filter((item): item is Tab => item !== null && !item.isPinned) + } + if (resolveItems().length === 0) { return } - const dirtyEditorFileIds: string[] = [] - for (const item of items) { - if (item.contentType === 'terminal') { - closeTab(item.entityId, { reason: 'cleanup' }) - } else if (item.contentType === 'browser') { - destroyWorkspaceWebviews(state.browserPagesByWorkspace, item.entityId) - closeBrowserTab(item.entityId) - } else if (item.contentType === 'simulator') { - closeUnifiedTab(item.id) - } else { - const file = state.openFiles.find((candidate) => candidate.id === item.entityId) - if (file?.isDirty) { - dirtyEditorFileIds.push(item.entityId) - continue + // Why: re-resolve inside the proceed closure — the aggregated running-process prompt + // can sit open arbitrarily long before the close actually runs. + const performClose = (): void => { + const state = useAppStore.getState() + const dirtyEditorFileIds: string[] = [] + for (const item of resolveItems()) { + if (item.contentType === 'terminal') { + closeTab(item.entityId, { reason: 'cleanup' }) + } else if (item.contentType === 'browser') { + destroyWorkspaceWebviews(state.browserPagesByWorkspace, item.entityId) + closeBrowserTab(item.entityId) + } else if (item.contentType === 'simulator') { + closeUnifiedTab(item.id) + } else { + const file = state.openFiles.find((candidate) => candidate.id === item.entityId) + if (file?.isDirty) { + dirtyEditorFileIds.push(item.entityId) + continue + } + closeFile(item.entityId) } - closeFile(item.entityId) + } + if (dirtyEditorFileIds.length > 0) { + queueEditorCloseRequests(dirtyEditorFileIds) } } - if (dirtyEditorFileIds.length > 0) { - queueEditorCloseRequests(dirtyEditorFileIds) - } + guardBulkTerminalClose({ + worktreeId: FLOATING_TERMINAL_WORKTREE_ID, + terminalTabIds: collectBulkTerminalTabIds( + useAppStore.getState(), + FLOATING_TERMINAL_WORKTREE_ID, + visibleIds + ), + onProceed: performClose + }) }, [activeGroup, closeBrowserTab, closeFile, closeTab, closeUnifiedTab, queueEditorCloseRequests] ) diff --git a/src/renderer/src/components/tab-group/useTabGroupCloseScopeCommands.ts b/src/renderer/src/components/tab-group/useTabGroupCloseScopeCommands.ts index a2f18a74dae..5ed6c171014 100644 --- a/src/renderer/src/components/tab-group/useTabGroupCloseScopeCommands.ts +++ b/src/renderer/src/components/tab-group/useTabGroupCloseScopeCommands.ts @@ -1,6 +1,10 @@ import { useCallback } from 'react' import type { Tab, TabGroup } from '../../../../shared/tab-types' import { useAppStore } from '../../store' +import { + collectBulkTerminalTabIds, + guardBulkTerminalClose +} from '../terminal/bulk-terminal-close-guard' export function useTabGroupCloseScopeCommands({ groupId, @@ -15,22 +19,33 @@ export function useTabGroupCloseScopeCommands({ worktreeId: string group: TabGroup | null groupTabs: Tab[] - closeItem: (itemId: string, opts?: { skipEmptyCheck?: boolean }) => void + closeItem: ( + itemId: string, + opts?: { skipEmptyCheck?: boolean; skipRunningProcessConfirm?: boolean } + ) => void closeMany: (itemIds: string[]) => void leaveWorktreeIfEmpty: () => void }) { const closeEmptyGroup = useAppStore((state) => state.closeEmptyGroup) const closeGroup = useCallback(() => { - const items = [...(useAppStore.getState().unifiedTabsByWorktree[worktreeId] ?? [])].filter( - (item) => item.groupId === groupId - ) - for (const item of items) { - closeItem(item.id, { skipEmptyCheck: true }) - } - // Why: closing tabs doesn't remove the group shell; empty split groups are layout state, collapse the placeholder pane here. - closeEmptyGroup(worktreeId, groupId) - leaveWorktreeIfEmpty() + const itemIds = (useAppStore.getState().unifiedTabsByWorktree[worktreeId] ?? []) + .filter((item) => item.groupId === groupId) + .map((item) => item.id) + // Why: one aggregated running-process prompt for the whole pane, matching the + // Close Others / to-the-side actions, instead of a modal per busy tab. + guardBulkTerminalClose({ + worktreeId, + terminalTabIds: collectBulkTerminalTabIds(useAppStore.getState(), worktreeId, itemIds), + onProceed: () => { + for (const itemId of itemIds) { + closeItem(itemId, { skipEmptyCheck: true, skipRunningProcessConfirm: true }) + } + // Why: closing tabs doesn't remove the group shell; empty split groups are layout state, collapse the placeholder pane here. + closeEmptyGroup(worktreeId, groupId) + leaveWorktreeIfEmpty() + } + }) }, [closeEmptyGroup, closeItem, groupId, leaveWorktreeIfEmpty, worktreeId]) const closeAllEditorTabsInGroup = useCallback(() => { diff --git a/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.bulk-close.test.ts b/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.bulk-close.test.ts new file mode 100644 index 00000000000..054df0a3e16 --- /dev/null +++ b/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.bulk-close.test.ts @@ -0,0 +1,138 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { Tab } from '../../../../shared/tab-types' + +const mocks = vi.hoisted(() => ({ + closeBrowserTab: vi.fn(), + closeFile: vi.fn(), + closeTab: vi.fn(), + closeTerminalTab: vi.fn(), + closeUnifiedTab: vi.fn(), + inspectRuntimeTerminalProcess: vi.fn(), + setActiveWorktree: vi.fn() +})) + +const storeBox = vi.hoisted(() => ({ state: {} as Record })) + +vi.mock('react', async () => { + const actual = await vi.importActual('react') // eslint-disable-line @typescript-eslint/consistent-type-imports -- vi.importActual requires inline import() + return { ...actual, useCallback: (callback: T) => callback } +}) + +vi.mock('../../store', () => { + const useAppStore = Object.assign( + (selector: (state: Record) => unknown) => selector(storeBox.state), + { getState: () => storeBox.state } + ) + return { useAppStore } +}) + +vi.mock('@/runtime/runtime-terminal-inspection', () => ({ + inspectRuntimeTerminalProcess: mocks.inspectRuntimeTerminalProcess +})) + +vi.mock('../../runtime/web-runtime-session', () => ({ + closeWebRuntimeSessionTab: vi.fn(), + isWebRuntimeSessionActive: () => false +})) + +vi.mock('../../store/slices/browser-webview-cleanup', () => ({ + destroyWorkspaceWebviews: vi.fn() +})) + +vi.mock('../editor/editor-autosave', () => ({ requestEditorFileClose: vi.fn() })) + +vi.mock('@/lib/worktree-runtime-owner', () => ({ + getRuntimeEnvironmentIdForWorktree: () => null +})) + +vi.mock('@/runtime/remote-browser-tab-ownership', () => ({ + browserWorkspaceHasRemoteOwner: () => false +})) + +vi.mock('../terminal/terminal-tab-actions', () => ({ closeTerminalTab: mocks.closeTerminalTab })) + +import { useRunningTerminalCloseConfirmStore } from '@/store/running-terminal-close-confirm' +import { useTabGroupTabCloseCommands } from './useTabGroupTabCloseCommands' + +const GROUP_TABS = [ + { id: 'unified-a', entityId: 'tab-a', contentType: 'terminal', label: 'npm run dev' }, + { id: 'unified-b', entityId: 'tab-b', contentType: 'terminal', label: 'pytest' } +] as unknown as Tab[] + +// Why: `useCallback` is stubbed to identity above, so the hook is a plain factory here. +function useCloseCommands() { + return useTabGroupTabCloseCommands({ worktreeId: 'wt-1', groupTabs: GROUP_TABS }) +} + +async function settleProbe(): Promise { + await Promise.resolve() + await Promise.resolve() + await Promise.resolve() +} + +describe('useTabGroupTabCloseCommands closeMany', () => { + beforeEach(() => { + vi.clearAllMocks() + storeBox.state = { + settings: {}, + ptyIdsByTabId: { 'tab-a': ['pty-a'], 'tab-b': ['pty-b'] }, + terminalLayoutsByTabId: {}, + unifiedTabsByWorktree: { 'wt-1': GROUP_TABS }, + tabsByWorktree: { 'wt-1': [{ id: 'tab-a' }, { id: 'tab-b' }] }, + openFiles: [], + browserPagesByWorkspace: {}, + agentStatusByPaneKey: {}, + activeWorktreeId: 'wt-1', + closeUnifiedTab: mocks.closeUnifiedTab, + closeTab: mocks.closeTab, + closeFile: mocks.closeFile, + closeBrowserTab: mocks.closeBrowserTab, + setActiveWorktree: mocks.setActiveWorktree, + reconcileWorktreeTabModel: () => ({ renderableTabCount: 1 }) + } + useRunningTerminalCloseConfirmStore.setState({ runningTerminalCloseConfirm: null }) + }) + + it('closes straight through when no tab in the set is busy', async () => { + mocks.inspectRuntimeTerminalProcess.mockResolvedValue({ + hasChildProcesses: false, + unavailable: false + }) + + useCloseCommands().closeMany(['unified-a', 'unified-b']) + await settleProbe() + + expect(mocks.closeTab).toHaveBeenCalledTimes(2) + expect(useRunningTerminalCloseConfirmStore.getState().runningTerminalCloseConfirm).toBeNull() + }) + + it('holds every close behind one prompt when tabs are busy, then runs them on confirm', async () => { + mocks.inspectRuntimeTerminalProcess.mockResolvedValue({ + hasChildProcesses: true, + unavailable: false + }) + + useCloseCommands().closeMany(['unified-a', 'unified-b']) + await settleProbe() + + expect(mocks.closeTab).not.toHaveBeenCalled() + const request = useRunningTerminalCloseConfirmStore.getState().runningTerminalCloseConfirm + expect(request?.busyTabLabels).toEqual(['npm run dev', 'pytest']) + + useRunningTerminalCloseConfirmStore.getState().confirmRunningTerminalClose() + expect(mocks.closeTab).toHaveBeenCalledTimes(2) + }) + + it('closes nothing when the prompt is cancelled', async () => { + mocks.inspectRuntimeTerminalProcess.mockResolvedValue({ + hasChildProcesses: true, + unavailable: false + }) + + useCloseCommands().closeMany(['unified-a', 'unified-b']) + await settleProbe() + useRunningTerminalCloseConfirmStore.getState().dismissRunningTerminalClose() + + expect(mocks.closeTab).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.ts b/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.ts index 9b6e85c2fcc..56f7054ca3f 100644 --- a/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.ts +++ b/src/renderer/src/components/tab-group/useTabGroupTabCloseCommands.ts @@ -8,6 +8,10 @@ import { isWebRuntimeSessionActive } from '../../runtime/web-runtime-session' import { closeTerminalTab } from '../terminal/terminal-tab-actions' +import { + collectBulkTerminalTabIds, + guardBulkTerminalClose +} from '../terminal/bulk-terminal-close-guard' import { getRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner' import { browserWorkspaceHasRemoteOwner } from '@/runtime/remote-browser-tab-ownership' @@ -62,7 +66,7 @@ export function useTabGroupTabCloseCommands({ }, [setActiveWorktree, worktreeId]) const closeItem = useCallback( - (itemId: string, opts?: { skipEmptyCheck?: boolean }) => { + (itemId: string, opts?: { skipEmptyCheck?: boolean; skipRunningProcessConfirm?: boolean }) => { const item = groupTabs.find((candidate) => candidate.id === itemId) if (!item) { return @@ -77,10 +81,11 @@ export function useTabGroupTabCloseCommands({ 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 } - ) + closeTerminalTab(item.entityId, { + ...(opts?.skipEmptyCheck ? {} : { onClosed: leaveWorktreeIfEmpty }), + // Why: a whole-group close already asked once for the whole set. + ...(opts?.skipRunningProcessConfirm ? { skipRunningProcessConfirm: true } : {}) + }) return } if (item.contentType === 'browser') { @@ -127,52 +132,61 @@ export function useTabGroupTabCloseCommands({ 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 === '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') { - // Why: see closeItem — host-close a remote-owned browser or pageless host-mirror; always remove the visible tab. - const browserState = useAppStore.getState() - const hasLocalPages = - (browserState.browserPagesByWorkspace[item.entityId] ?? []).length > 0 - const shouldCloseOnHost = - isWebRuntimeSessionActive(runtimeEnvironmentId) && - (browserWorkspaceHasRemoteOwner(browserState, item.entityId, runtimeEnvironmentId) || - !hasLocalPages) - if (shouldCloseOnHost) { - void closeWebRuntimeSessionTab({ - worktreeId, - tabId: item.id, - environmentId: runtimeEnvironmentId, - reason: 'user' - }) + // Why: re-resolve inside the proceed closure — the aggregated running-process prompt + // can sit open arbitrarily long before the close actually runs. + const performClose = (): void => { + for (const itemId of itemIds) { + const item = groupTabs.find((candidate) => candidate.id === itemId) + if (!item || item.isPinned) { + continue } - destroyWorkspaceWebviews(browserState.browserPagesByWorkspace, item.entityId) - closeBrowserTab(item.entityId) - closeUnifiedTab(item.id) - } else if (item.contentType === 'terminal') { - closeTab(item.entityId) - } else if (item.contentType === 'simulator') { - closeUnifiedTab(item.id) - } else { - const canCloseTab = closeEditorIfUnreferenced(item.entityId, item.id) - if (canCloseTab) { + const runtimeEnvironmentId = getRuntimeEnvironmentIdForWorktree( + useAppStore.getState(), + worktreeId + ) + if (item.contentType === 'terminal' && isWebRuntimeSessionActive(runtimeEnvironmentId)) { + // Why: revoke local resume + hook authority before the host removes its canonical tab. + // The single aggregated prompt already ran, so skip the per-tab one. + closeTerminalTab(item.entityId, { skipRunningProcessConfirm: true }) + continue + } + if (item.contentType === 'browser') { + // Why: see closeItem — host-close a remote-owned browser or pageless host-mirror; always remove the visible tab. + const browserState = useAppStore.getState() + const hasLocalPages = + (browserState.browserPagesByWorkspace[item.entityId] ?? []).length > 0 + const shouldCloseOnHost = + isWebRuntimeSessionActive(runtimeEnvironmentId) && + (browserWorkspaceHasRemoteOwner(browserState, item.entityId, runtimeEnvironmentId) || + !hasLocalPages) + if (shouldCloseOnHost) { + void closeWebRuntimeSessionTab({ + worktreeId, + tabId: item.id, + environmentId: runtimeEnvironmentId, + reason: 'user' + }) + } + destroyWorkspaceWebviews(browserState.browserPagesByWorkspace, item.entityId) + closeBrowserTab(item.entityId) closeUnifiedTab(item.id) + } else if (item.contentType === 'terminal') { + closeTab(item.entityId) + } else if (item.contentType === 'simulator') { + closeUnifiedTab(item.id) + } else { + const canCloseTab = closeEditorIfUnreferenced(item.entityId, item.id) + if (canCloseTab) { + closeUnifiedTab(item.id) + } } } } + guardBulkTerminalClose({ + worktreeId, + terminalTabIds: collectBulkTerminalTabIds(useAppStore.getState(), worktreeId, itemIds), + onProceed: performClose + }) }, [closeBrowserTab, closeEditorIfUnreferenced, closeTab, closeUnifiedTab, groupTabs, worktreeId] ) diff --git a/src/renderer/src/components/terminal-pane/CloseTerminalDialog.test.tsx b/src/renderer/src/components/terminal-pane/CloseTerminalDialog.test.tsx index d2b8efd3efa..8862a5eaffa 100644 --- a/src/renderer/src/components/terminal-pane/CloseTerminalDialog.test.tsx +++ b/src/renderer/src/components/terminal-pane/CloseTerminalDialog.test.tsx @@ -9,6 +9,7 @@ const mountedRoots: Root[] = [] async function renderDialog(props: { copyKind?: 'command' | 'agent' + busyTabLabels?: readonly string[] onConfirm: (dontAskAgain: boolean) => void onCancel?: () => void }): Promise { @@ -22,6 +23,7 @@ async function renderDialog(props: { @@ -89,6 +91,49 @@ describe('CloseTerminalDialog', () => { expect(onConfirm).toHaveBeenCalledWith(true) }) + it('counts and names every busy tab for a bulk close', async () => { + const onConfirm = vi.fn() + + await renderDialog({ + copyKind: 'command', + busyTabLabels: ['npm run dev', 'pytest', 'tail -f log'], + onConfirm + }) + + expect(document.body.textContent).toContain('Stop 3 running commands?') + expect(document.body.textContent).toContain( + 'Closing these tabs will stop the commands running inside them.' + ) + for (const label of ['npm run dev', 'pytest', 'tail -f log']) { + expect(document.body.textContent).toContain(label) + } + + await act(async () => { + clickButton('Stop and Close') + }) + + expect(onConfirm).toHaveBeenCalledWith(false) + }) + + it('caps the named tabs and counts the remainder', async () => { + await renderDialog({ + busyTabLabels: ['one', 'two', 'three', 'four', 'five', 'six', 'seven'], + onConfirm: vi.fn() + }) + + expect(document.body.textContent).toContain('Stop 7 running commands?') + expect(document.body.textContent).toContain('five') + expect(document.body.textContent).not.toContain('six') + expect(document.body.textContent).toContain('+2 more') + }) + + it('keeps the single-tab copy when only one tab in the bulk set is busy', async () => { + await renderDialog({ copyKind: 'agent', busyTabLabels: ['claude'], onConfirm: vi.fn() }) + + expect(document.body.textContent).toContain('Stop this agent?') + expect(document.body.textContent).not.toContain('Stop 1 running') + }) + it('resets the skip preference when the dialog closes and reopens', async () => { const onConfirm = vi.fn() const onCancel = vi.fn() diff --git a/src/renderer/src/components/terminal-pane/CloseTerminalDialog.tsx b/src/renderer/src/components/terminal-pane/CloseTerminalDialog.tsx index c4da5244457..e07f3b016a3 100644 --- a/src/renderer/src/components/terminal-pane/CloseTerminalDialog.tsx +++ b/src/renderer/src/components/terminal-pane/CloseTerminalDialog.tsx @@ -14,10 +14,15 @@ import { translate } from '@/i18n/i18n' export type CloseTerminalDialogCopyKind = 'command' | 'agent' +/** Beyond this the list stops naming tabs and counts the rest, so the compact confirmation + * never grows into a scrolling wall for a 20-tab "Close Others". */ +const MAX_LISTED_BUSY_TAB_LABELS = 5 + export default function CloseTerminalDialog({ open, copyKind = 'command', tabLabel, + busyTabLabels, subjectKey, onCancel, onConfirm @@ -27,6 +32,9 @@ export default function CloseTerminalDialog({ /** Names the tab when the prompt can target a tab the user is not looking at * (tab-strip X, middle-click). Omitted for the focused-pane keyboard path. */ tabLabel?: string + /** Names every busy tab when one prompt covers a bulk close ("Close Others" and the + * to-the-side variants). More than one switches the copy to the counted form. */ + busyTabLabels?: readonly string[] /** Identifies what is being closed, for hosts that reuse one open dialog across a queue * of confirmations. Changing it clears the previous subject's "don't ask again" tick. */ subjectKey?: string @@ -58,7 +66,12 @@ export default function CloseTerminalDialog({ } const isAgent = copyKind === 'agent' - const trimmedTabLabel = tabLabel?.trim() + const trimmedBusyTabLabels = (busyTabLabels ?? []).map((label) => label.trim()).filter(Boolean) + const busyTabCount = trimmedBusyTabLabels.length + const isBulk = busyTabCount > 1 + const listedBusyTabLabels = trimmedBusyTabLabels.slice(0, MAX_LISTED_BUSY_TAB_LABELS) + const unlistedBusyTabCount = busyTabCount - listedBusyTabLabels.length + const trimmedTabLabel = isBulk ? undefined : tabLabel?.trim() return ( - {isAgent - ? translate( - 'auto.components.terminal.pane.CloseTerminalDialog.stop_agent_title', - 'Stop this agent?' - ) - : translate( - 'auto.components.terminal.pane.CloseTerminalDialog.stop_command_title', - 'Stop running command?' - )} + {isBulk + ? isAgent + ? translate( + 'components.terminal.pane.CloseTerminalDialog.stopAgentsTitle', + 'Stop {{count}} running agents?', + { count: busyTabCount } + ) + : translate( + 'components.terminal.pane.CloseTerminalDialog.stopCommandsTitle', + 'Stop {{count}} running commands?', + { count: busyTabCount } + ) + : isAgent + ? translate( + 'auto.components.terminal.pane.CloseTerminalDialog.stop_agent_title', + 'Stop this agent?' + ) + : translate( + 'auto.components.terminal.pane.CloseTerminalDialog.stop_command_title', + 'Stop running command?' + )} - {isAgent - ? translate( - 'auto.components.terminal.pane.CloseTerminalDialog.stop_agent_description', - "Closing this terminal will stop the agent's current work." - ) - : translate( - 'auto.components.terminal.pane.CloseTerminalDialog.stop_command_description', - 'Closing this terminal will stop the command running inside it.' - )} + {isBulk + ? isAgent + ? translate( + 'components.terminal.pane.CloseTerminalDialog.stopAgentsDescription', + "Closing these tabs will stop the agents' current work." + ) + : translate( + 'components.terminal.pane.CloseTerminalDialog.stopCommandsDescription', + 'Closing these tabs will stop the commands running inside them.' + ) + : isAgent + ? translate( + 'auto.components.terminal.pane.CloseTerminalDialog.stop_agent_description', + "Closing this terminal will stop the agent's current work." + ) + : translate( + 'auto.components.terminal.pane.CloseTerminalDialog.stop_command_description', + 'Closing this terminal will stop the command running inside it.' + )} {trimmedTabLabel ? ( @@ -99,6 +134,28 @@ export default function CloseTerminalDialog({ {trimmedTabLabel}

) : null} + {isBulk ? ( +
    + {listedBusyTabLabels.map((label, index) => ( +
  • + {label} +
    • + ))} + {unlistedBusyTabCount > 0 ? ( +
  • + {translate( + 'components.terminal.pane.CloseTerminalDialog.moreBusyTabs', + '+{{count}} more', + { count: unlistedBusyTabCount } + )} +
    • + ) : null} +
+ ) : null}
onConfirm(dontAskAgain)} > - {isAgent + {isAgent && !isBulk ? translate( 'auto.components.terminal.pane.CloseTerminalDialog.stop_agent_confirm', 'Stop Agent' diff --git a/src/renderer/src/components/terminal-pane/RunningTerminalCloseDialog.tsx b/src/renderer/src/components/terminal-pane/RunningTerminalCloseDialog.tsx index cf93fa77207..e30010f664c 100644 --- a/src/renderer/src/components/terminal-pane/RunningTerminalCloseDialog.tsx +++ b/src/renderer/src/components/terminal-pane/RunningTerminalCloseDialog.tsx @@ -27,6 +27,7 @@ export default function RunningTerminalCloseDialog(): React.JSX.Element { open={request !== null && pinnedRequest === null} copyKind={request?.copyKind ?? 'command'} {...(request?.tabLabel ? { tabLabel: request.tabLabel } : {})} + {...(request?.busyTabLabels ? { busyTabLabels: request.busyTabLabels } : {})} // Why: a queued request swaps tabs in the already-open dialog, so the reopen reset // never runs; naming the subject is what clears the previous tab's opt-out tick. {...(request ? { subjectKey: request.terminalTabId } : {})} diff --git a/src/renderer/src/components/terminal/bulk-terminal-close-guard.test.ts b/src/renderer/src/components/terminal/bulk-terminal-close-guard.test.ts new file mode 100644 index 00000000000..244a716e41f --- /dev/null +++ b/src/renderer/src/components/terminal/bulk-terminal-close-guard.test.ts @@ -0,0 +1,254 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { getStateMock, inspectRuntimeTerminalProcessMock } = vi.hoisted(() => ({ + getStateMock: vi.fn(), + inspectRuntimeTerminalProcessMock: vi.fn() +})) + +vi.mock('@/store', () => ({ + useAppStore: { getState: getStateMock } +})) + +vi.mock('@/runtime/runtime-terminal-inspection', () => ({ + inspectRuntimeTerminalProcess: inspectRuntimeTerminalProcessMock +})) + +import { useRunningTerminalCloseConfirmStore } from '@/store/running-terminal-close-confirm' +import { RUNNING_CLOSE_PROBE_TIMEOUT_MS } from './running-terminal-close-guard' +import { collectBulkTerminalTabIds, guardBulkTerminalClose } from './bulk-terminal-close-guard' + +const LEAF_A = '11111111-1111-4111-8111-111111111111' +const LEAF_B = '22222222-2222-4222-8222-222222222222' + +function setState(overrides: Record = {}): void { + getStateMock.mockReturnValue({ + settings: { activeRuntimeEnvironmentId: null }, + ptyIdsByTabId: { 'tab-a': ['pty-a'], 'tab-b': ['pty-b'] }, + terminalLayoutsByTabId: { + 'tab-a': { ptyIdsByLeafId: { [LEAF_A]: 'pty-a' } }, + 'tab-b': { ptyIdsByLeafId: { [LEAF_B]: 'pty-b' } } + }, + unifiedTabsByWorktree: { + 'wt-1': [ + { id: 'unified-a', entityId: 'tab-a', contentType: 'terminal', label: 'npm run dev' }, + { id: 'unified-b', entityId: 'tab-b', contentType: 'terminal', label: 'pytest' } + ] + }, + tabsByWorktree: { 'wt-1': [{ id: 'tab-a' }, { id: 'tab-b' }] }, + agentStatusByPaneKey: {}, + ...overrides + }) +} + +function guard(onProceed = vi.fn(), onCancel?: () => void): void { + guardBulkTerminalClose({ + worktreeId: 'wt-1', + terminalTabIds: ['tab-a', 'tab-b'], + onProceed, + ...(onCancel ? { onCancel } : {}) + }) +} + +function visibleRequest() { + return useRunningTerminalCloseConfirmStore.getState().runningTerminalCloseConfirm +} + +async function settleProbe(): Promise { + await Promise.resolve() + await Promise.resolve() + await Promise.resolve() +} + +describe('collectBulkTerminalTabIds', () => { + beforeEach(() => { + vi.clearAllMocks() + setState() + }) + + it('accepts unified ids and entity ids, and skips non-terminal and pinned tabs', () => { + setState({ + unifiedTabsByWorktree: { + 'wt-1': [ + { id: 'unified-a', entityId: 'tab-a', contentType: 'terminal' }, + { id: 'unified-b', entityId: 'tab-b', contentType: 'terminal', isPinned: true }, + { id: 'unified-c', entityId: 'file-c', contentType: 'editor' } + ] + } + }) + + expect( + collectBulkTerminalTabIds(getStateMock(), 'wt-1', ['unified-a', 'tab-b', 'unified-c']) + ).toEqual(['tab-a']) + }) + + it('falls back to the runtime tab store for a terminal with no unified row yet', () => { + setState({ unifiedTabsByWorktree: { 'wt-1': [] } }) + + expect(collectBulkTerminalTabIds(getStateMock(), 'wt-1', ['tab-a', 'unknown'])).toEqual([ + 'tab-a' + ]) + }) +}) + +describe('guardBulkTerminalClose', () => { + beforeEach(() => { + vi.clearAllMocks() + setState() + inspectRuntimeTerminalProcessMock.mockResolvedValue({ + hasChildProcesses: false, + unavailable: false + }) + useRunningTerminalCloseConfirmStore.setState({ runningTerminalCloseConfirm: null }) + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it('proceeds synchronously when no tab in the set has a live pty', () => { + setState({ ptyIdsByTabId: {}, terminalLayoutsByTabId: {} }) + const onProceed = vi.fn() + + guard(onProceed) + + expect(onProceed).toHaveBeenCalledTimes(1) + expect(inspectRuntimeTerminalProcessMock).not.toHaveBeenCalled() + expect(visibleRequest()).toBeNull() + }) + + it('proceeds synchronously when the user opted out of the prompt', () => { + setState({ + settings: { + activeRuntimeEnvironmentId: null, + skipCloseTerminalWithRunningProcessConfirm: true + } + }) + const onProceed = vi.fn() + + guard(onProceed) + + expect(onProceed).toHaveBeenCalledTimes(1) + expect(inspectRuntimeTerminalProcessMock).not.toHaveBeenCalled() + }) + + it('proceeds without asking when every probe reports an idle shell', async () => { + const onProceed = vi.fn() + + guard(onProceed) + await settleProbe() + + expect(inspectRuntimeTerminalProcessMock).toHaveBeenCalledTimes(2) + expect(onProceed).toHaveBeenCalledTimes(1) + expect(visibleRequest()).toBeNull() + }) + + it('raises one aggregated prompt naming every busy tab', async () => { + inspectRuntimeTerminalProcessMock.mockResolvedValue({ + hasChildProcesses: true, + unavailable: false + }) + const onProceed = vi.fn() + + guard(onProceed) + await settleProbe() + + expect(onProceed).not.toHaveBeenCalled() + const request = visibleRequest() + expect(request?.busyTabLabels).toEqual(['npm run dev', 'pytest']) + expect(request?.terminalTabId).toBe('bulk:tab-a,tab-b') + expect(request?.copyKind).toBe('command') + + useRunningTerminalCloseConfirmStore.getState().confirmRunningTerminalClose() + expect(onProceed).toHaveBeenCalledTimes(1) + }) + + it('lists only the busy tabs, keeping strip order', async () => { + inspectRuntimeTerminalProcessMock.mockImplementation((_settings, ptyId: string) => + Promise.resolve({ hasChildProcesses: ptyId === 'pty-b', unavailable: false }) + ) + + guard() + await settleProbe() + + const request = visibleRequest() + expect(request?.busyTabLabels).toEqual(['pytest']) + // Why: a lone busy tab keys on its real id so it merges with that tab's own prompt. + expect(request?.terminalTabId).toBe('tab-b') + }) + + it('cancelling abandons the whole bulk close', async () => { + inspectRuntimeTerminalProcessMock.mockResolvedValue({ + hasChildProcesses: true, + unavailable: false + }) + const onProceed = vi.fn() + const onCancel = vi.fn() + + guard(onProceed, onCancel) + await settleProbe() + useRunningTerminalCloseConfirmStore.getState().dismissRunningTerminalClose() + + expect(onProceed).not.toHaveBeenCalled() + expect(onCancel).toHaveBeenCalledTimes(1) + }) + + it('words the prompt for agents when any busy pane is one', async () => { + setState({ + agentStatusByPaneKey: { [`tab-b:${LEAF_B}`]: { agentType: 'claude' } } + }) + inspectRuntimeTerminalProcessMock.mockResolvedValue({ + hasChildProcesses: true, + unavailable: false + }) + + guard() + await settleProbe() + + expect(visibleRequest()?.copyKind).toBe('agent') + }) + + it('proceeds on an answered probe that could not inspect the shell', async () => { + inspectRuntimeTerminalProcessMock.mockResolvedValue({ + hasChildProcesses: true, + unavailable: true + }) + const onProceed = vi.fn() + + guard(onProceed) + await settleProbe() + + expect(onProceed).toHaveBeenCalledTimes(1) + expect(visibleRequest()).toBeNull() + }) + + it('asks rather than closes when the probe never answers', async () => { + vi.useFakeTimers() + inspectRuntimeTerminalProcessMock.mockReturnValue(new Promise(() => {})) + const onProceed = vi.fn() + + guard(onProceed) + vi.advanceTimersByTime(RUNNING_CLOSE_PROBE_TIMEOUT_MS) + + expect(onProceed).not.toHaveBeenCalled() + expect(visibleRequest()?.busyTabLabels).toEqual(['npm run dev', 'pytest']) + }) + + it('merges a repeated bulk close into the open prompt instead of stacking one', async () => { + inspectRuntimeTerminalProcessMock.mockResolvedValue({ + hasChildProcesses: true, + unavailable: false + }) + const first = vi.fn() + const second = vi.fn() + + guard(first) + await settleProbe() + guard(second) + await settleProbe() + + useRunningTerminalCloseConfirmStore.getState().confirmRunningTerminalClose() + expect(first).toHaveBeenCalledTimes(1) + expect(second).toHaveBeenCalledTimes(1) + expect(visibleRequest()).toBeNull() + }) +}) diff --git a/src/renderer/src/components/terminal/bulk-terminal-close-guard.ts b/src/renderer/src/components/terminal/bulk-terminal-close-guard.ts new file mode 100644 index 00000000000..bcf4641abac --- /dev/null +++ b/src/renderer/src/components/terminal/bulk-terminal-close-guard.ts @@ -0,0 +1,181 @@ +import { useAppStore } from '@/store' +import { inspectRuntimeTerminalProcess } from '@/runtime/runtime-terminal-inspection' +import { resolvePinnedTabLabel } from '@/store/pinned-tab-close-guard' +import { useRunningTerminalCloseConfirmStore } from '@/store/running-terminal-close-confirm' +import type { AppState } from '@/store/types' +import type { CloseTerminalDialogCopyKind } from '../terminal-pane/CloseTerminalDialog' +import { collectTabPtyIds, RUNNING_CLOSE_PROBE_TIMEOUT_MS } from './running-terminal-close-guard' +import { resolveBusyPtyCloseCopyKind } from './terminal-close-copy-kind' + +/** A terminal tab in the bulk set, with the PTYs it could still own. */ +type BulkCloseCandidate = { terminalTabId: string; ptyIds: string[] } + +/** + * Terminal tab ids (entity ids) inside a mixed tab-strip id list, skipping the pinned tabs + * a bulk close leaves alone. Accepts either unified-tab ids or entity ids because the tab + * strip emits entity ids for terminals and unified ids for editors. + */ +export function collectBulkTerminalTabIds( + state: Pick, + worktreeId: string, + visibleIds: readonly string[] +): string[] { + const unifiedTabs = state.unifiedTabsByWorktree?.[worktreeId] ?? [] + const terminalTabIds: string[] = [] + for (const visibleId of visibleIds) { + const unifiedTab = unifiedTabs.find( + (candidate) => candidate.id === visibleId || candidate.entityId === visibleId + ) + if (unifiedTab) { + if (unifiedTab.contentType === 'terminal' && unifiedTab.isPinned !== true) { + terminalTabIds.push(unifiedTab.entityId) + } + continue + } + // Why: agent quick-launch can briefly leave a terminal in the runtime store before its + // unified row lands; missing it here would silently drop the prompt for a busy tab. + if ((state.tabsByWorktree?.[worktreeId] ?? []).some((tab) => tab.id === visibleId)) { + terminalTabIds.push(visibleId) + } + } + return terminalTabIds +} + +/** Stable dedupe key so a double-fired bulk close merges into one prompt. A lone busy tab + * keys on its real id, so it also merges with that tab's own single-close prompt. */ +function resolveRequestKey(busyTabIds: readonly string[]): string { + return busyTabIds.length === 1 ? busyTabIds[0]! : `bulk:${[...busyTabIds].sort().join(',')}` +} + +/** + * Routes "Close Others" / "Close Tabs To The Right" / "Close Tabs To The Left" through one + * aggregated running-process confirmation instead of the modal storm that made these paths + * opt out of the prompt entirely. Proceeds synchronously when no tab in the set has a live + * PTY, so idle bulk closes keep today's behavior. + * + * Cancel abandons the whole bulk close rather than closing the idle subset: a partial, + * un-undoable close is a worse surprise than doing nothing and letting the user retry. + */ +export function guardBulkTerminalClose(params: { + worktreeId: string + terminalTabIds: readonly string[] + onProceed: () => void + onCancel?: () => void +}): void { + const { worktreeId, terminalTabIds, onProceed, onCancel } = params + const state = useAppStore.getState() + const settings = state.settings + const candidates: BulkCloseCandidate[] = [] + for (const terminalTabId of new Set(terminalTabIds)) { + const ptyIds = collectTabPtyIds(state, terminalTabId) + if (ptyIds.length > 0) { + candidates.push({ terminalTabId, ptyIds }) + } + } + // Why: nothing to probe (parked/hibernated tabs, or a set with no terminals at all) and + // the opt-out setting both mean the answer is known, so the close stays synchronous. + if (candidates.length === 0 || settings?.skipCloseTerminalWithRunningProcessConfirm === true) { + onProceed() + return + } + + // Why: the timeout, the probe result and the error path race to decide this close, so the + // first one to land owns it instead of trusting those races to stay mutually exclusive. + let decided = false + const proceedNow = (): void => { + if (decided) { + return + } + decided = true + onProceed() + } + const confirmClose = (busy: readonly BulkCloseCandidate[]): void => { + if (decided) { + return + } + const latest = useAppStore.getState() + const busyTabLabels = busy.map((candidate) => + resolvePinnedTabLabel(latest, worktreeId, candidate.terminalTabId) + ) + // Why: an agent anywhere in the set wins the wording, matching the single-tab prompt's + // mixed-split rule — stopping an agent mid-task is the costlier surprise. + const copyKind: CloseTerminalDialogCopyKind = busy.some( + (candidate) => + resolveBusyPtyCloseCopyKind(candidate.terminalTabId, candidate.ptyIds) === 'agent' + ) + ? 'agent' + : 'command' + useRunningTerminalCloseConfirmStore.getState().requestRunningTerminalCloseConfirm({ + terminalTabId: resolveRequestKey(busy.map((candidate) => candidate.terminalTabId)), + tabLabel: busyTabLabels[0] ?? '', + busyTabLabels, + copyKind, + onConfirm: onProceed, + ...(onCancel ? { onCancel } : {}) + }) + // Why: only once the prompt is actually up — if either call above throws, the close must + // still be free to fall through and happen. + decided = true + } + + const probes = candidates.flatMap((candidate) => + candidate.ptyIds.map((ptyId) => ({ candidate, ptyId })) + ) + const probeTimeout = setTimeout(() => { + try { + // Why: a probe that has not answered yet is unknown, not idle. Ask about every + // candidate so a degraded relay costs a click instead of killed remote work. + confirmClose(candidates) + } catch { + proceedNow() + } + }, RUNNING_CLOSE_PROBE_TIMEOUT_MS) + + void Promise.allSettled(probes.map(({ ptyId }) => inspectRuntimeTerminalProcess(settings, ptyId))) + .then((results) => { + clearTimeout(probeTimeout) + if (decided) { + return + } + // Why: fail open on an *answered* probe, matching the single-tab guard — a rejection + // or a stale remote handle is not evidence of a live child, and a menu action that + // silently does nothing is worse than closing a busy tab. + const busyPtyIdsByTabId = new Map() + results.forEach((result, index) => { + if ( + result.status !== 'fulfilled' || + !result.value.hasChildProcesses || + result.value.unavailable === true + ) { + return + } + const probe = probes[index]! + const busyPtyIds = busyPtyIdsByTabId.get(probe.candidate.terminalTabId) + if (busyPtyIds) { + busyPtyIds.push(probe.ptyId) + } else { + busyPtyIdsByTabId.set(probe.candidate.terminalTabId, [probe.ptyId]) + } + }) + if (busyPtyIdsByTabId.size === 0) { + proceedNow() + return + } + // Why: walk `candidates` rather than the map so the prompt lists tabs in strip order. + confirmClose( + candidates + .filter((candidate) => busyPtyIdsByTabId.has(candidate.terminalTabId)) + .map((candidate) => ({ + terminalTabId: candidate.terminalTabId, + ptyIds: busyPtyIdsByTabId.get(candidate.terminalTabId)! + })) + ) + }) + // Why: allSettled never rejects, so this only fires when the decision above throws (a + // label lookup, a store subscriber). Without it the bulk close would silently never + // happen and the user would get no feedback at all. + .catch(() => { + clearTimeout(probeTimeout) + proceedNow() + }) +} diff --git a/src/renderer/src/components/terminal/running-terminal-close-guard.ts b/src/renderer/src/components/terminal/running-terminal-close-guard.ts index 73f63fc85f7..6cd3136d3f0 100644 --- a/src/renderer/src/components/terminal/running-terminal-close-guard.ts +++ b/src/renderer/src/components/terminal/running-terminal-close-guard.ts @@ -22,8 +22,9 @@ export type RunningTerminalCloseGuardOptions = { export const RUNNING_CLOSE_PROBE_TIMEOUT_MS = 4_000 /** Whether this close is an interactive user action that should stop and ask before - * killing a live child process. Lifecycle echoes, bulk closes, CLI/RPC closes and the - * post-confirmation re-entry are all excluded. */ + * killing a live child process. Lifecycle echoes, CLI/RPC closes and the + * post-confirmation re-entry are all excluded, as are bulk closes — those raise one + * aggregated prompt up front (see bulk-terminal-close-guard) and opt each tab out here. */ export function shouldConfirmRunningTerminalClose( options?: RunningTerminalCloseGuardOptions ): boolean { @@ -43,7 +44,7 @@ export function shouldConfirmRunningTerminalClose( * the store's own teardown collector unions both for exactly that reason — reading only * the map would let a close slip through the window with no prompt. A stale id costs * nothing: its probe fails and the guard falls open. */ -function collectTabPtyIds( +export function collectTabPtyIds( state: Pick, terminalTabId: string ): string[] { diff --git a/src/renderer/src/components/terminal/terminal-tab-actions.ts b/src/renderer/src/components/terminal/terminal-tab-actions.ts index 8216696d217..9aca98b1bde 100644 --- a/src/renderer/src/components/terminal/terminal-tab-actions.ts +++ b/src/renderer/src/components/terminal/terminal-tab-actions.ts @@ -46,8 +46,9 @@ export function closeTerminalTab( hostCloseReason?: TerminalTabCloseReason /** PTY whose lifecycle event initiated the host close. */ lifecyclePtyId?: string - /** Set by callers that must never raise a modal (bulk closes, CLI/RPC, lifecycle - * fallbacks) and by the re-entry that runs once the user confirmed. */ + /** Set by callers that must never raise a modal here (CLI/RPC, lifecycle fallbacks, + * and bulk closes that already asked once for the whole set) and by the re-entry + * that runs once the user confirmed. */ skipRunningProcessConfirm?: boolean captureRecentlyClosed?: boolean localPtyTeardownOwnedExternally?: boolean diff --git a/src/renderer/src/components/terminal/terminal-tab-bulk-actions.ts b/src/renderer/src/components/terminal/terminal-tab-bulk-actions.ts index 3bca59bd476..a78bf6381b1 100644 --- a/src/renderer/src/components/terminal/terminal-tab-bulk-actions.ts +++ b/src/renderer/src/components/terminal/terminal-tab-bulk-actions.ts @@ -7,6 +7,7 @@ import { closeWebRuntimeSessionTab, isWebRuntimeSessionActive } from '@/runtime/ import { useAppStore } from '@/store' import { reconcileTabOrder } from '../tab-bar/reconcile-order' import { closeLocalTerminalTabState } from './close-local-terminal-tab-state' +import { collectBulkTerminalTabIds, guardBulkTerminalClose } from './bulk-terminal-close-guard' const EDITOR_TAB_CONTENT_TYPES = new Set([ 'editor', @@ -38,29 +39,39 @@ export function closeOtherTerminalTabs(tabId: string, activeWorktreeId: string | return } const currentTabs = state.tabsByWorktree[activeWorktreeId] ?? [] - state.setActiveTab(tabId) - const runtimeEnvironmentId = resolveTerminalWorktreeRoute( - state, - activeWorktreeId - )?.runtimeEnvironmentId - const closeHostTerminalTabs = isWebRuntimeSessionActive(runtimeEnvironmentId) - for (const tab of currentTabs) { - if (tab.id === tabId || isPinnedVisibleTab(state, activeWorktreeId, tab.id)) { - continue - } - if (closeHostTerminalTabs) { - // Why: prune the mirror immediately, then close on its authoritative host so snapshots converge. - closeLocalTerminalTabState(tab.id, { remoteCloseOwnedByHost: true }) - void closeWebRuntimeSessionTab({ - worktreeId: activeWorktreeId, - tabId: tab.id, - environmentId: runtimeEnvironmentId, - reason: 'user' - }) - } else { - state.closeTab(tab.id) + const closableIds = currentTabs + .map((tab) => tab.id) + .filter((id) => id !== tabId && !isPinnedVisibleTab(state, activeWorktreeId, id)) + // Why: activate and close only once the aggregated running-process prompt is answered, so + // cancelling leaves both the tab strip and the running work untouched. + const performClose = (): void => { + const latest = useAppStore.getState() + latest.setActiveTab(tabId) + const runtimeEnvironmentId = resolveTerminalWorktreeRoute( + latest, + activeWorktreeId + )?.runtimeEnvironmentId + const closeHostTerminalTabs = isWebRuntimeSessionActive(runtimeEnvironmentId) + for (const id of closableIds) { + if (closeHostTerminalTabs) { + // Why: prune the mirror immediately, then close on its authoritative host so snapshots converge. + closeLocalTerminalTabState(id, { remoteCloseOwnedByHost: true }) + void closeWebRuntimeSessionTab({ + worktreeId: activeWorktreeId, + tabId: id, + environmentId: runtimeEnvironmentId, + reason: 'user' + }) + } else { + latest.closeTab(id) + } } } + guardBulkTerminalClose({ + worktreeId: activeWorktreeId, + terminalTabIds: collectBulkTerminalTabIds(state, activeWorktreeId, closableIds), + onProceed: performClose + }) } export function closeTerminalTabsToRight(tabId: string, activeWorktreeId: string | null): void { @@ -91,30 +102,37 @@ export function closeTerminalTabsToRight(tabId: string, activeWorktreeId: string if (index === -1) { return } - for (const id of orderedIds.slice(index + 1)) { - if (isPinnedVisibleTab(state, activeWorktreeId, id)) { - continue - } - if (terminalIdSet.has(id)) { - if (closeHostTerminalTabs) { - // Why: prune the mirror immediately, then close on its authoritative host so snapshots converge. - closeLocalTerminalTabState(id, { remoteCloseOwnedByHost: true }) - void closeWebRuntimeSessionTab({ - worktreeId: activeWorktreeId, - tabId: id, - environmentId: runtimeEnvironmentId, - reason: 'user' - }) - } else { - state.closeTab(id) + const closableIds = orderedIds + .slice(index + 1) + .filter((id) => !isPinnedVisibleTab(state, activeWorktreeId, id)) + const performClose = (): void => { + for (const id of closableIds) { + if (terminalIdSet.has(id)) { + if (closeHostTerminalTabs) { + // Why: prune the mirror immediately, then close on its authoritative host so snapshots converge. + closeLocalTerminalTabState(id, { remoteCloseOwnedByHost: true }) + void closeWebRuntimeSessionTab({ + worktreeId: activeWorktreeId, + tabId: id, + environmentId: runtimeEnvironmentId, + reason: 'user' + }) + } else { + useAppStore.getState().closeTab(id) + } + continue + } + const unifiedTab = ( + useAppStore.getState().unifiedTabsByWorktree?.[activeWorktreeId] ?? [] + ).find((tab) => tab.entityId === id && EDITOR_TAB_CONTENT_TYPES.has(tab.contentType)) + if (!unifiedTab?.isPinned) { + useAppStore.getState().closeFile(id) } - continue - } - const unifiedTab = (state.unifiedTabsByWorktree?.[activeWorktreeId] ?? []).find( - (tab) => tab.entityId === id && EDITOR_TAB_CONTENT_TYPES.has(tab.contentType) - ) - if (!unifiedTab?.isPinned) { - useAppStore.getState().closeFile(id) } } + guardBulkTerminalClose({ + worktreeId: activeWorktreeId, + terminalTabIds: closableIds.filter((id) => terminalIdSet.has(id)), + onProceed: performClose + }) } diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 7f559834060..6cb9f12c1e4 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -16295,6 +16295,17 @@ "days": "{{value0}}d", "underOneMinute": "<1m" } + }, + "terminal": { + "pane": { + "CloseTerminalDialog": { + "stopAgentsTitle": "Stop {{count}} running agents?", + "stopCommandsTitle": "Stop {{count}} running commands?", + "stopAgentsDescription": "Closing these tabs will stop the agents' current work.", + "stopCommandsDescription": "Closing these tabs will stop the commands running inside them.", + "moreBusyTabs": "+{{count}} more" + } + } } }, "dashboardPopout": { diff --git a/src/renderer/src/store/running-terminal-close-confirm.ts b/src/renderer/src/store/running-terminal-close-confirm.ts index 88db03a5d38..04254de9eb3 100644 --- a/src/renderer/src/store/running-terminal-close-confirm.ts +++ b/src/renderer/src/store/running-terminal-close-confirm.ts @@ -4,8 +4,12 @@ import type { CloseTerminalDialogCopyKind } from '@/components/terminal-pane/Clo /** A pending confirmation for closing a terminal tab whose shell still has a * running child process. `onConfirm` performs the original close. */ export type RunningTerminalCloseConfirmRequest = { + /** Also the queue dedupe key, so a bulk close passes a synthetic id covering its set. */ terminalTabId: string tabLabel: string + /** Present when one prompt covers a bulk close; names every tab that reported a live + * child so the counted copy can list them. */ + busyTabLabels?: readonly string[] copyKind: CloseTerminalDialogCopyKind onConfirm: () => void onCancel?: () => void