fix(terminal): apply the running-process close confirmation to every tab close path (#10142) (#12272)
* test(repro): demonstrate #10142 tab X close bypasses running-process confirmation Unit repro: closeTerminalTab (the X-button/middle-click entry) never consults inspectRuntimeTerminalProcess and drops a tab with a live child. E2E repro: Cmd+W shows 'Stop running command?' for a tab running sleep 300; cancelling then clicking the tab X closes it silently. Co-authored-by: Orca <help@stably.ai> * fix(terminal): confirm running-process close on every tab close path (#10142) The tab-strip X button, middle-click and the tab context menu closed a terminal with a live child process without asking, while Cmd+W raised "Stop running command?" for the same tab. The probe lived only in TerminalPane's pane-level close handler; every mouse entry point reaches closeTerminalTab(), which guarded pinned tabs and nothing else. Move the decision into closeTerminalTab, above the web-runtime branch so paired/remote host-backed tabs are covered too, and give the last-pane keyboard close back to it instead of probing twice: - running-terminal-close-guard.ts probes every live PTY of the tab and fails open on a rejected probe or a stale remote handle, matching what Cmd+W already did. No live PTY ids => fully synchronous close, so idle, parked and hibernated tabs keep today's behavior. - shouldConfirmRunningTerminalClose keeps lifecycle echoes, bulk closes, CLI/RPC closes and the post-confirmation re-entry off the modal path. - A standalone confirm store drives RunningTerminalCloseDialog, which reuses the existing CloseTerminalDialog (no new user-visible strings). The request carries the tab label because a tab-strip close can target a tab the user is not looking at, and dedupes by tab id. - TerminalPane.handleRequestClosePane now delegates the last pane to closeTerminalTab. Its transport ptyId is nullable by design, so the old path silently skipped the prompt mid-reattach; the pane keeps its own probe only for closing one pane of a split. - Agent panes win the dialog copy when a split has both an agent and a plain command busy, instead of depending on PTY spawn order. - Tab-group closeItem ran leaveWorktreeIfEmpty synchronously after a close that can now defer; it moves to onClosed and still honors skipEmptyCheck. Co-authored-by: Orca <help@stably.ai> * fix(terminal): close the running-process confirmation gaps on every path (#10142) Follow-up hardening on the tab-close confirmation, from review of the first pass: - A pinned tab with `confirmClosePinnedTab` off never got the running-process prompt on any path, including Cmd+W, which is a regression against the old pane-level behavior: the pinned branch short-circuited on pinned-ness alone and re-entered with `force`, which the running guard excludes. The pin prompt now supersedes only when it will actually appear; with the setting off the close falls through to the running guard. - The probe chain had no `.catch`, so a throw in the decision (a copy-kind lookup on a tab id makePaneKey rejects, a store subscriber) left the tab silently unclosed with no user feedback. It now fails open, as the pane path it replaced did. - A wedged remote inspect RPC could leave the X button looking dead for its full 15s timeout. The probe is now bounded; every close path shares the bound, so keyboard and mouse still behave identically. - The agent-vs-command copy had two resolvers on exactly the keyboard/mouse seam this issue is about. terminal-close-copy-kind.ts is now the single policy; TerminalPane and the tab-strip guard both call it. - The running queue is async while the pinned queue is synchronous, so both could be pending at once and stack two modal overlays. The running dialog now waits for a visible pinned confirmation. - Deduping a repeat close request dropped the second caller's callbacks; it now folds them in, so both closes resolve from one prompt. Ticking "don't ask again" also drains queued prompts instead of showing one the user just opted out of, and a queued prompt no longer inherits the previous tab's tick. closeTerminalTab drops its private pinned predicate for the shared isUnifiedTabPinned, whose only consumer the previous commit had removed. * test(e2e): wait for `sleep` to own the terminal before closing it (#10142) The running-process close specs polled `hasChildProcesses` to decide the tab was busy, but macOS starts the shell under `login`, so an initialising terminal already reports a child before `sleep 300` runs. Both specs could therefore press close against a shell that never started the command: the probe correctly saw an idle terminal and closed without asking, and the adjudicated repro failed against a correct fix. Wait for `foregroundProcess === 'sleep'` instead. Assertions are unchanged, and the repro still fails at the pre-fix baseline (a92d8e0b0d). * fix(terminal): ask instead of closing when the close probe times out (#10142) Round-1 review follow-ups. - The 4s probe bound closed the tab outright, but `inspectRuntimeTerminalProcess` gives remote runtimes a 15s RPC timeout: any probe taking 4-15s silently killed a running remote command that Cmd+W used to prompt about, and the pane path now delegates its last-pane close to this guard. An unanswered probe is unknown, not idle, so the timeout raises the confirmation with every pty treated as a candidate. Failing open still applies to an *answered* probe (rejection, stale remote handle), which is the pre-existing pane behavior. - The split-pane Cmd+W probe had no bound at all; it now shares the same one, so the two paths give the same answer to the same question. - The renamed regression spec dropped its repro scaffolding: the hardcoded /tmp screenshot directory (also a cross-platform path violation) and the title/docblock that still described the bug as open. * fix(terminal): adopt the double-activation guard and layout pty lookup from #10167 (#10142) Cross-referenced against @innocarpe's #10167, which solved the same issue. Two things it got right that this branch did not: - Queue actions now hold off for 350ms after a queued request replaces the visible one, matching the sibling pinned-tab confirmation. Without it the second click of a double-click aimed at one tab lands on the next tab's prompt and kills a running process the user never saw asked about — the exact bug class this PR exists to close. - The pty lookup unions the layout bindings with ptyIdsByTabId. A mounting pane is bound into the layout before the liveness map catches up, and the store's own teardown collector unions both for that reason, so reading only the map let a close slip through that window with no prompt. Also replaces the render-time ref write that failed React Doctor's "Ref mutated during render" rule: the queued-request checkbox reset now goes through CloseTerminalDialog's existing subject-change reset instead of remounting via key, which keeps the exit animation on one element. Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Neil
GitHub
parent
086a5d6495
commit
ba83a71e30
|
|
@ -207,6 +207,7 @@ import { showTerminalShortcutCaptureNotification } from '@/lib/terminal-shortcut
|
|||
import { resolveMountedLazyModalIds, type LazyModalId } from './lazy-modal-mount-state'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import PinnedTabCloseDialog from './components/terminal-pane/PinnedTabCloseDialog'
|
||||
import RunningTerminalCloseDialog from './components/terminal-pane/RunningTerminalCloseDialog'
|
||||
import WorktreeBaseFallbackDialog from './components/WorktreeBaseFallbackDialog'
|
||||
import { useOsc52ClipboardDefaultOnNotice } from './components/terminal-pane/osc52-clipboard-default-on-notice'
|
||||
import {
|
||||
|
|
@ -2789,6 +2790,7 @@ function App(): React.JSX.Element {
|
|||
<SkillFreshnessNudge />
|
||||
<WorktreeBaseFallbackDialog />
|
||||
<PinnedTabCloseDialog />
|
||||
<RunningTerminalCloseDialog />
|
||||
{/* Why: Electron's drag-region hit-test is DOM-order-based (ignores z-index); render last so WindowControls stay clickable. */}
|
||||
{hasCustomTitleBar && <WindowControls />}
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -1797,7 +1797,8 @@ function Terminal(): React.JSX.Element | null {
|
|||
) {
|
||||
if (unifiedTab.contentType === 'terminal') {
|
||||
// Why: paired-host bulk close must revoke renderer resume and hook authority, not just remove the host session tab.
|
||||
closeTerminalTab(unifiedTab.entityId)
|
||||
// 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,
|
||||
|
|
|
|||
|
|
@ -235,10 +235,12 @@ export function useTabGroupWorkspaceModel({
|
|||
worktreeId
|
||||
)
|
||||
if (item.contentType === 'terminal') {
|
||||
closeTerminalTab(item.entityId)
|
||||
if (!opts?.skipEmptyCheck) {
|
||||
leaveWorktreeIfEmpty()
|
||||
}
|
||||
// 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') {
|
||||
|
|
@ -296,7 +298,8 @@ export function useTabGroupWorkspaceModel({
|
|||
)
|
||||
if (item.contentType === 'terminal' && isWebRuntimeSessionActive(runtimeEnvironmentId)) {
|
||||
// Why: revoke local resume + hook authority before the host removes its canonical tab.
|
||||
closeTerminalTab(item.entityId)
|
||||
// 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') {
|
||||
|
|
|
|||
|
|
@ -17,17 +17,26 @@ export type CloseTerminalDialogCopyKind = 'command' | 'agent'
|
|||
export default function CloseTerminalDialog({
|
||||
open,
|
||||
copyKind = 'command',
|
||||
tabLabel,
|
||||
subjectKey,
|
||||
onCancel,
|
||||
onConfirm
|
||||
}: {
|
||||
open: boolean
|
||||
copyKind?: CloseTerminalDialogCopyKind
|
||||
/** 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
|
||||
/** 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
|
||||
onCancel: () => void
|
||||
onConfirm: (dontAskAgain: boolean) => void
|
||||
}): React.JSX.Element {
|
||||
const checkboxId = useId()
|
||||
const [dontAskAgain, setDontAskAgain] = useState(false)
|
||||
const [previousOpen, setPreviousOpen] = useState(open)
|
||||
const [previousSubjectKey, setPreviousSubjectKey] = useState(subjectKey)
|
||||
|
||||
// Why: each reopen represents a fresh confirmation, so clear the old choice
|
||||
// during render rather than briefly painting it while the dialog opens.
|
||||
|
|
@ -38,7 +47,18 @@ export default function CloseTerminalDialog({
|
|||
}
|
||||
}
|
||||
|
||||
// Why: a queued confirmation swaps the subject without ever closing the dialog, so the
|
||||
// reopen reset above never fires. Ignore the swap to undefined as the dialog closes —
|
||||
// clearing the tick mid-exit-animation would be visible for no reason.
|
||||
if (subjectKey !== previousSubjectKey) {
|
||||
setPreviousSubjectKey(subjectKey)
|
||||
if (subjectKey !== undefined) {
|
||||
setDontAskAgain(false)
|
||||
}
|
||||
}
|
||||
|
||||
const isAgent = copyKind === 'agent'
|
||||
const trimmedTabLabel = tabLabel?.trim()
|
||||
|
||||
return (
|
||||
<Dialog
|
||||
|
|
@ -74,6 +94,11 @@ export default function CloseTerminalDialog({
|
|||
)}
|
||||
</DialogDescription>
|
||||
</DialogHeader>
|
||||
{trimmedTabLabel ? (
|
||||
<p className="truncate text-xs font-medium text-foreground" title={trimmedTabLabel}>
|
||||
{trimmedTabLabel}
|
||||
</p>
|
||||
) : null}
|
||||
<div className="flex items-center gap-2">
|
||||
<Checkbox
|
||||
id={checkboxId}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,247 @@
|
|||
// @vitest-environment happy-dom
|
||||
|
||||
import { act } from 'react'
|
||||
import { createRoot, type Root } from 'react-dom/client'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { useAppStore } from '@/store'
|
||||
import type { AppState } from '@/store/types'
|
||||
import {
|
||||
useRunningTerminalCloseConfirmStore,
|
||||
type RunningTerminalCloseConfirmRequest
|
||||
} from '@/store/running-terminal-close-confirm'
|
||||
import RunningTerminalCloseDialog from './RunningTerminalCloseDialog'
|
||||
|
||||
const initialState = useAppStore.getInitialState()
|
||||
const mountedRoots: Root[] = []
|
||||
|
||||
// The store holds off any action for 350 ms after a queued request replaces the visible
|
||||
// one, so these tests drive a clock instead of racing it.
|
||||
let clock = 1_000
|
||||
|
||||
function advancePastGuard(): void {
|
||||
clock += 400
|
||||
}
|
||||
|
||||
async function renderDialog(
|
||||
request: Partial<RunningTerminalCloseConfirmRequest> & { onConfirm: () => void },
|
||||
updateSettings: AppState['updateSettings']
|
||||
): Promise<void> {
|
||||
useAppStore.setState({ updateSettings })
|
||||
useRunningTerminalCloseConfirmStore.getState().requestRunningTerminalCloseConfirm({
|
||||
terminalTabId: 'tab-1',
|
||||
tabLabel: 'dev server',
|
||||
copyKind: 'command',
|
||||
...request
|
||||
})
|
||||
|
||||
const container = document.createElement('div')
|
||||
document.body.appendChild(container)
|
||||
const root = createRoot(container)
|
||||
mountedRoots.push(root)
|
||||
|
||||
await act(async () => {
|
||||
root.render(<RunningTerminalCloseDialog />)
|
||||
})
|
||||
}
|
||||
|
||||
function getButton(label: string): HTMLButtonElement {
|
||||
const button = [...document.body.querySelectorAll<HTMLButtonElement>('button')].find(
|
||||
(candidate) => candidate.textContent === label
|
||||
)
|
||||
if (!button) {
|
||||
throw new Error(`Button not found: ${label}`)
|
||||
}
|
||||
return button
|
||||
}
|
||||
|
||||
function getCheckbox(): HTMLButtonElement {
|
||||
const checkbox = document.body.querySelector<HTMLButtonElement>('[role="checkbox"]')
|
||||
if (!checkbox) {
|
||||
throw new Error('Checkbox not found')
|
||||
}
|
||||
return checkbox
|
||||
}
|
||||
|
||||
describe('RunningTerminalCloseDialog', () => {
|
||||
beforeEach(() => {
|
||||
useAppStore.setState(initialState, true)
|
||||
// Monotonic across tests: the store is a singleton, so winding the clock back would
|
||||
// leave a previous test's guard deadline in the future and block every action.
|
||||
clock += 10_000
|
||||
vi.spyOn(Date, 'now').mockImplementation(() => clock)
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
while (useRunningTerminalCloseConfirmStore.getState().runningTerminalCloseConfirm !== null) {
|
||||
advancePastGuard()
|
||||
useRunningTerminalCloseConfirmStore.getState().dismissRunningTerminalClose()
|
||||
}
|
||||
vi.mocked(Date.now).mockRestore()
|
||||
await act(async () => {
|
||||
for (const root of mountedRoots.splice(0)) {
|
||||
root.unmount()
|
||||
}
|
||||
})
|
||||
document.body.innerHTML = ''
|
||||
useAppStore.setState(initialState, true)
|
||||
})
|
||||
|
||||
it('names the tab so a background close is not ambiguous', async () => {
|
||||
const onConfirm = vi.fn()
|
||||
const updateSettings = vi.fn().mockResolvedValue(undefined)
|
||||
|
||||
await renderDialog({ onConfirm }, updateSettings)
|
||||
|
||||
expect(document.body.textContent).toContain('Stop running command?')
|
||||
expect(document.body.textContent).toContain('dev server')
|
||||
|
||||
await act(async () => {
|
||||
getButton('Stop and Close').click()
|
||||
})
|
||||
|
||||
expect(updateSettings).not.toHaveBeenCalled()
|
||||
expect(onConfirm).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('uses the agent copy for an agent pane', async () => {
|
||||
const updateSettings = vi.fn().mockResolvedValue(undefined)
|
||||
|
||||
await renderDialog({ onConfirm: vi.fn(), copyKind: 'agent' }, updateSettings)
|
||||
|
||||
expect(document.body.textContent).toContain('Stop this agent?')
|
||||
expect(getButton('Stop Agent')).toBeTruthy()
|
||||
})
|
||||
|
||||
it('persists the opt-out when "don\'t ask again" is checked', async () => {
|
||||
const onConfirm = vi.fn()
|
||||
const updateSettings = vi.fn().mockResolvedValue(undefined)
|
||||
|
||||
await renderDialog({ onConfirm }, updateSettings)
|
||||
|
||||
await act(async () => {
|
||||
getCheckbox().click()
|
||||
})
|
||||
await act(async () => {
|
||||
getButton('Stop and Close').click()
|
||||
})
|
||||
|
||||
expect(updateSettings).toHaveBeenCalledWith({
|
||||
skipCloseTerminalWithRunningProcessConfirm: true
|
||||
})
|
||||
expect(onConfirm).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
// Why: this queue opens after an async probe while the pinned queue opens synchronously,
|
||||
// so both can be pending at once. Two modal overlays + focus traps is the bug.
|
||||
it('waits for a visible pinned confirmation instead of stacking a second modal', async () => {
|
||||
const updateSettings = vi.fn().mockResolvedValue(undefined)
|
||||
useAppStore.setState({
|
||||
pinnedTabCloseConfirm: { tabLabel: 'pinned tab', onConfirm: vi.fn() }
|
||||
})
|
||||
|
||||
await renderDialog({ onConfirm: vi.fn() }, updateSettings)
|
||||
|
||||
expect(document.body.textContent).not.toContain('Stop running command?')
|
||||
|
||||
await act(async () => {
|
||||
useAppStore.setState({ pinnedTabCloseConfirm: null })
|
||||
})
|
||||
|
||||
expect(document.body.textContent).toContain('Stop running command?')
|
||||
})
|
||||
|
||||
it('does not carry the opt-out tick over to the next queued tab', async () => {
|
||||
const updateSettings = vi.fn().mockResolvedValue(undefined)
|
||||
const nextOnConfirm = vi.fn()
|
||||
|
||||
await renderDialog({ onConfirm: vi.fn(), onCancel: vi.fn() }, updateSettings)
|
||||
await act(async () => {
|
||||
useRunningTerminalCloseConfirmStore.getState().requestRunningTerminalCloseConfirm({
|
||||
terminalTabId: 'tab-2',
|
||||
tabLabel: 'build watcher',
|
||||
copyKind: 'command',
|
||||
onConfirm: nextOnConfirm
|
||||
})
|
||||
})
|
||||
|
||||
await act(async () => {
|
||||
getCheckbox().click()
|
||||
})
|
||||
await act(async () => {
|
||||
getButton('Cancel').click()
|
||||
})
|
||||
|
||||
expect(document.body.textContent).toContain('build watcher')
|
||||
expect(getCheckbox().getAttribute('data-state')).toBe('unchecked')
|
||||
|
||||
advancePastGuard()
|
||||
await act(async () => {
|
||||
getButton('Stop and Close').click()
|
||||
})
|
||||
|
||||
expect(updateSettings).not.toHaveBeenCalled()
|
||||
expect(nextOnConfirm).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('drops a queued prompt once the user opts out of asking again', async () => {
|
||||
const updateSettings = vi.fn().mockResolvedValue(undefined)
|
||||
const onConfirm = vi.fn()
|
||||
const nextOnConfirm = vi.fn()
|
||||
|
||||
await renderDialog({ onConfirm }, updateSettings)
|
||||
await act(async () => {
|
||||
useRunningTerminalCloseConfirmStore.getState().requestRunningTerminalCloseConfirm({
|
||||
terminalTabId: 'tab-2',
|
||||
tabLabel: 'build watcher',
|
||||
copyKind: 'command',
|
||||
onConfirm: nextOnConfirm
|
||||
})
|
||||
})
|
||||
|
||||
await act(async () => {
|
||||
getCheckbox().click()
|
||||
})
|
||||
await act(async () => {
|
||||
getButton('Stop and Close').click()
|
||||
})
|
||||
|
||||
expect(updateSettings).toHaveBeenCalledWith({
|
||||
skipCloseTerminalWithRunningProcessConfirm: true
|
||||
})
|
||||
expect(onConfirm).toHaveBeenCalledTimes(1)
|
||||
expect(nextOnConfirm).toHaveBeenCalledTimes(1)
|
||||
expect(document.body.textContent).not.toContain('build watcher')
|
||||
})
|
||||
|
||||
it('cancels without closing and shows the next queued tab', async () => {
|
||||
const onConfirm = vi.fn()
|
||||
const onCancel = vi.fn()
|
||||
const nextOnConfirm = vi.fn()
|
||||
const updateSettings = vi.fn().mockResolvedValue(undefined)
|
||||
|
||||
await renderDialog({ onConfirm, onCancel }, updateSettings)
|
||||
await act(async () => {
|
||||
useRunningTerminalCloseConfirmStore.getState().requestRunningTerminalCloseConfirm({
|
||||
terminalTabId: 'tab-2',
|
||||
tabLabel: 'build watcher',
|
||||
copyKind: 'command',
|
||||
onConfirm: nextOnConfirm
|
||||
})
|
||||
})
|
||||
|
||||
await act(async () => {
|
||||
getButton('Cancel').click()
|
||||
})
|
||||
|
||||
expect(onCancel).toHaveBeenCalledTimes(1)
|
||||
expect(onConfirm).not.toHaveBeenCalled()
|
||||
expect(document.body.textContent).toContain('build watcher')
|
||||
|
||||
advancePastGuard()
|
||||
await act(async () => {
|
||||
getButton('Stop and Close').click()
|
||||
})
|
||||
|
||||
expect(nextOnConfirm).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,45 @@
|
|||
import { useAppStore } from '@/store'
|
||||
import { useRunningTerminalCloseConfirmStore } from '@/store/running-terminal-close-confirm'
|
||||
import CloseTerminalDialog from './CloseTerminalDialog'
|
||||
|
||||
/** Hosts the running-process close confirmation for tab-level closes (tab-strip X,
|
||||
* middle-click, tab menu, tab groups, floating panel) so they share the prompt Cmd+W
|
||||
* already raised. Store-driven, like PinnedTabCloseDialog, because those closes run
|
||||
* outside any pane's React tree. */
|
||||
export default function RunningTerminalCloseDialog(): React.JSX.Element {
|
||||
const request = useRunningTerminalCloseConfirmStore((state) => state.runningTerminalCloseConfirm)
|
||||
const confirmClose = useRunningTerminalCloseConfirmStore(
|
||||
(state) => state.confirmRunningTerminalClose
|
||||
)
|
||||
const confirmAllCloses = useRunningTerminalCloseConfirmStore(
|
||||
(state) => state.confirmAllRunningTerminalCloses
|
||||
)
|
||||
const dismissClose = useRunningTerminalCloseConfirmStore(
|
||||
(state) => state.dismissRunningTerminalClose
|
||||
)
|
||||
const updateSettings = useAppStore((state) => state.updateSettings)
|
||||
// Why: this queue is async (it opens after a probe) while the pinned queue is synchronous,
|
||||
// so both can be pending at once. Wait rather than stack two modal overlays and focus traps.
|
||||
const pinnedRequest = useAppStore((state) => state.pinnedTabCloseConfirm)
|
||||
|
||||
return (
|
||||
<CloseTerminalDialog
|
||||
open={request !== null && pinnedRequest === null}
|
||||
copyKind={request?.copyKind ?? 'command'}
|
||||
{...(request?.tabLabel ? { tabLabel: request.tabLabel } : {})}
|
||||
// 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 } : {})}
|
||||
onCancel={dismissClose}
|
||||
onConfirm={(dontAskAgain) => {
|
||||
if (dontAskAgain) {
|
||||
void updateSettings({ skipCloseTerminalWithRunningProcessConfirm: true })
|
||||
// Why: the user just opted out of this prompt; a queued one must not still appear.
|
||||
confirmAllCloses()
|
||||
return
|
||||
}
|
||||
confirmClose()
|
||||
}}
|
||||
/>
|
||||
)
|
||||
}
|
||||
|
|
@ -14,7 +14,6 @@ import { createPortal } from 'react-dom'
|
|||
import type { CSSProperties } from 'react'
|
||||
import type { IDisposable } from '@xterm/xterm'
|
||||
import { useAppStore } from '../../store'
|
||||
import { isUnifiedTabPinned } from '@/store/pinned-tab-close-guard'
|
||||
import { useLinkRoutingPreferenceDialog } from '@/components/link-routing-preference-dialog'
|
||||
import { DaemonActionDialog, useDaemonActions } from '@/components/shared/useDaemonActions'
|
||||
import {
|
||||
|
|
@ -64,6 +63,8 @@ import type { MacOptionAsAlt } from './terminal-shortcut-policy'
|
|||
import { useEffectiveMacOptionAsAlt } from '@/lib/keyboard-layout/use-effective-mac-option-as-alt'
|
||||
import { useTerminalFontZoom } from './useTerminalFontZoom'
|
||||
import CloseTerminalDialog, { type CloseTerminalDialogCopyKind } from './CloseTerminalDialog'
|
||||
import { resolveLeafCloseCopyKind } from '../terminal/terminal-close-copy-kind'
|
||||
import { RUNNING_CLOSE_PROBE_TIMEOUT_MS } from '../terminal/running-terminal-close-guard'
|
||||
import CodexRestartChip from '../CodexRestartChip'
|
||||
import { MobileDriverOverlay } from './MobileDriverOverlay'
|
||||
import { stripSshReconnectOwnedErrorLines, TerminalErrorToast } from './TerminalErrorToast'
|
||||
|
|
@ -1288,30 +1289,21 @@ function TerminalPane(
|
|||
)
|
||||
|
||||
// Cmd+W confirms before killing a shell with a running child (e.g. npm run dev); idle prompts close immediately, and Ctrl+D bypasses by design.
|
||||
// Why: the agent-vs-command rule is shared with the tab-strip prompt so the two close paths cannot word the same close differently (#10142).
|
||||
const getCloseDialogCopyKind = useCallback(
|
||||
(paneId: number): CloseTerminalDialogCopyKind => {
|
||||
const leafId = managerRef.current?.getLeafId(paneId)
|
||||
if (!leafId) {
|
||||
return 'command'
|
||||
}
|
||||
const agentType =
|
||||
useAppStore.getState().agentStatusByPaneKey[makePaneKey(tabId, leafId)]?.agentType
|
||||
return agentType && agentType !== 'unknown' ? 'agent' : 'command'
|
||||
},
|
||||
(paneId: number): CloseTerminalDialogCopyKind =>
|
||||
resolveLeafCloseCopyKind(tabId, managerRef.current?.getLeafId(paneId)),
|
||||
[tabId]
|
||||
)
|
||||
|
||||
const handleRequestClosePane = useCallback(
|
||||
(paneId: number) => {
|
||||
// Why: closing the last pane of a pinned tab prefers the pin dialog over the running-process prompt; non-pinned tabs keep the process prompt.
|
||||
const isLastPane = (managerRef.current?.getPanes().length ?? 0) <= 1
|
||||
if (isLastPane) {
|
||||
const state = useAppStore.getState()
|
||||
const confirmPinned = state.settings?.confirmClosePinnedTab ?? true
|
||||
if (confirmPinned && isUnifiedTabPinned(state, worktreeId, tabId)) {
|
||||
executeClosePane(paneId)
|
||||
return
|
||||
}
|
||||
// Why: the last pane closes the whole tab, and closeTerminalTab owns both the pinned
|
||||
// and running-process guards. Probing here too would double-prompt, and its nullable
|
||||
// transport ptyId would silently skip the prompt the mouse paths now get (#10142).
|
||||
if ((managerRef.current?.getPanes().length ?? 0) <= 1) {
|
||||
executeClosePane(paneId)
|
||||
return
|
||||
}
|
||||
const transport = paneTransportsRef.current.get(paneId)
|
||||
const ptyId = transport?.getPtyId()
|
||||
|
|
@ -1320,18 +1312,40 @@ function TerminalPane(
|
|||
return
|
||||
}
|
||||
const settings = useAppStore.getState().settings
|
||||
// Why: same bound as the whole-tab guard, so a wedged remote probe never leaves Cmd+W
|
||||
// looking dead for the full 15s RPC timeout; unanswered means ask, not close (#10142).
|
||||
let decided = false
|
||||
const decide = (act: () => void): void => {
|
||||
if (decided) {
|
||||
return
|
||||
}
|
||||
decided = true
|
||||
act()
|
||||
}
|
||||
const confirmClose = (): void =>
|
||||
setPendingCloseConfirmation({ paneId, copyKind: getCloseDialogCopyKind(paneId) })
|
||||
const probeTimeout = setTimeout(() => decide(confirmClose), RUNNING_CLOSE_PROBE_TIMEOUT_MS)
|
||||
void inspectRuntimeTerminalProcess(settings, ptyId)
|
||||
.then((process) => {
|
||||
if (!process.hasChildProcesses || settings?.skipCloseTerminalWithRunningProcessConfirm) {
|
||||
executeClosePane(paneId)
|
||||
} else {
|
||||
setPendingCloseConfirmation({ paneId, copyKind: getCloseDialogCopyKind(paneId) })
|
||||
}
|
||||
clearTimeout(probeTimeout)
|
||||
decide(() => {
|
||||
if (
|
||||
!process.hasChildProcesses ||
|
||||
settings?.skipCloseTerminalWithRunningProcessConfirm
|
||||
) {
|
||||
executeClosePane(paneId)
|
||||
} else {
|
||||
confirmClose()
|
||||
}
|
||||
})
|
||||
})
|
||||
// Why: if the child-process probe rejects (wedged IPC, legacy provider), close anyway — Cmd+W doing nothing is worse than closing a pane with a child.
|
||||
.catch(() => executeClosePane(paneId))
|
||||
.catch(() => {
|
||||
clearTimeout(probeTimeout)
|
||||
decide(() => executeClosePane(paneId))
|
||||
})
|
||||
},
|
||||
[executeClosePane, tabId, worktreeId, getCloseDialogCopyKind]
|
||||
[executeClosePane, getCloseDialogCopyKind]
|
||||
)
|
||||
|
||||
useImperativeHandle(
|
||||
|
|
|
|||
|
|
@ -1664,7 +1664,8 @@ export function useTerminalPaneLifecycle({
|
|||
manager: mgr,
|
||||
getPtyIdForLeaf: (leafId) =>
|
||||
useAppStore.getState().terminalLayoutsByTabId[tabId]?.ptyIdsByLeafId?.[leafId],
|
||||
closeTab: () => closeTerminalTab(tabId),
|
||||
// Why: CLI-driven pane close; its caller is answered immediately and cannot wait on a modal.
|
||||
closeTab: () => closeTerminalTab(tabId, { skipRunningProcessConfirm: true }),
|
||||
closeTabPreservingPty: () => {
|
||||
const store = useAppStore.getState()
|
||||
if (detail.retireSurface && detail.leafId) {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,409 @@
|
|||
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 {
|
||||
guardRunningTerminalClose,
|
||||
shouldConfirmRunningTerminalClose,
|
||||
RUNNING_CLOSE_PROBE_TIMEOUT_MS
|
||||
} from './running-terminal-close-guard'
|
||||
|
||||
const LEAF_A = '11111111-1111-4111-8111-111111111111'
|
||||
const LEAF_B = '22222222-2222-4222-8222-222222222222'
|
||||
|
||||
function setState(overrides: Record<string, unknown> = {}): void {
|
||||
getStateMock.mockReturnValue({
|
||||
settings: { activeRuntimeEnvironmentId: null },
|
||||
ptyIdsByTabId: { 'tab-1': ['pty-a'] },
|
||||
terminalLayoutsByTabId: { 'tab-1': { ptyIdsByLeafId: { [LEAF_A]: 'pty-a' } } },
|
||||
agentStatusByPaneKey: {},
|
||||
...overrides
|
||||
})
|
||||
}
|
||||
|
||||
function guard(onClose = vi.fn(), onCancel?: () => void): void {
|
||||
guardRunningTerminalClose({
|
||||
terminalTabId: 'tab-1',
|
||||
tabLabel: 'npm run dev',
|
||||
onClose,
|
||||
...(onCancel ? { onCancel } : {})
|
||||
})
|
||||
}
|
||||
|
||||
function visibleRequest() {
|
||||
return useRunningTerminalCloseConfirmStore.getState().runningTerminalCloseConfirm
|
||||
}
|
||||
|
||||
async function settleProbe(): Promise<void> {
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
}
|
||||
|
||||
describe('shouldConfirmRunningTerminalClose', () => {
|
||||
it('confirms a plain interactive close', () => {
|
||||
expect(shouldConfirmRunningTerminalClose(undefined)).toBe(true)
|
||||
expect(shouldConfirmRunningTerminalClose({})).toBe(true)
|
||||
expect(shouldConfirmRunningTerminalClose({ reason: 'user' })).toBe(true)
|
||||
expect(shouldConfirmRunningTerminalClose({ hostCloseReason: 'user' })).toBe(true)
|
||||
})
|
||||
|
||||
it.each([
|
||||
['post-confirmation re-entry', { force: true }],
|
||||
['non-interactive CLI reject', { rejectPinned: true }],
|
||||
['bulk/CLI opt-out', { skipRunningProcessConfirm: true }],
|
||||
['pty lifecycle echo', { reason: 'pty-exit' as const }],
|
||||
['cleanup teardown', { reason: 'cleanup' as const }],
|
||||
['host-only pty lifecycle echo', { hostCloseReason: 'pty-exit' as const }],
|
||||
['pty-scoped lifecycle close', { lifecyclePtyId: 'pty-a' }]
|
||||
])('never prompts for a %s', (_label, options) => {
|
||||
expect(shouldConfirmRunningTerminalClose(options)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('guardRunningTerminalClose', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
setState()
|
||||
inspectRuntimeTerminalProcessMock.mockResolvedValue({
|
||||
foregroundProcess: 'sleep',
|
||||
hasChildProcesses: true
|
||||
})
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
const store = useRunningTerminalCloseConfirmStore.getState()
|
||||
while (visibleRequest()) {
|
||||
store.dismissRunningTerminalClose()
|
||||
}
|
||||
})
|
||||
|
||||
it('closes synchronously and never probes when the tab has no live pty', () => {
|
||||
setState({ ptyIdsByTabId: {}, terminalLayoutsByTabId: {} })
|
||||
const onClose = vi.fn()
|
||||
|
||||
guard(onClose)
|
||||
|
||||
expect(onClose).toHaveBeenCalledTimes(1)
|
||||
expect(inspectRuntimeTerminalProcessMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
// Why: a mounting pane is bound into the layout before ptyIdsByTabId catches up. Reading
|
||||
// only the liveness map would let a close slip through that window with no prompt.
|
||||
it('prompts for a pane the layout has bound but the liveness map has not', async () => {
|
||||
setState({ ptyIdsByTabId: { 'tab-1': [] } })
|
||||
const onClose = vi.fn()
|
||||
|
||||
guard(onClose)
|
||||
await settleProbe()
|
||||
|
||||
expect(inspectRuntimeTerminalProcessMock).toHaveBeenCalledWith(expect.anything(), 'pty-a')
|
||||
expect(onClose).not.toHaveBeenCalled()
|
||||
expect(visibleRequest()).toMatchObject({ terminalTabId: 'tab-1' })
|
||||
})
|
||||
|
||||
it('probes each pty once when the map and the layout name the same one', async () => {
|
||||
guard()
|
||||
await settleProbe()
|
||||
|
||||
expect(inspectRuntimeTerminalProcessMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('closes without probing when the user turned the prompt off', () => {
|
||||
setState({
|
||||
settings: {
|
||||
activeRuntimeEnvironmentId: null,
|
||||
skipCloseTerminalWithRunningProcessConfirm: true
|
||||
}
|
||||
})
|
||||
const onClose = vi.fn()
|
||||
|
||||
guard(onClose)
|
||||
|
||||
expect(onClose).toHaveBeenCalledTimes(1)
|
||||
expect(inspectRuntimeTerminalProcessMock).not.toHaveBeenCalled()
|
||||
expect(visibleRequest()).toBeNull()
|
||||
})
|
||||
|
||||
it('closes an idle terminal without a prompt', async () => {
|
||||
inspectRuntimeTerminalProcessMock.mockResolvedValue({
|
||||
foregroundProcess: 'zsh',
|
||||
hasChildProcesses: false
|
||||
})
|
||||
const onClose = vi.fn()
|
||||
|
||||
guard(onClose)
|
||||
await settleProbe()
|
||||
|
||||
expect(onClose).toHaveBeenCalledTimes(1)
|
||||
expect(visibleRequest()).toBeNull()
|
||||
})
|
||||
|
||||
it('defers a busy terminal behind a confirmation that carries the tab label', async () => {
|
||||
const onClose = vi.fn()
|
||||
|
||||
guard(onClose)
|
||||
expect(onClose).not.toHaveBeenCalled()
|
||||
await settleProbe()
|
||||
|
||||
expect(onClose).not.toHaveBeenCalled()
|
||||
expect(visibleRequest()).toMatchObject({
|
||||
terminalTabId: 'tab-1',
|
||||
tabLabel: 'npm run dev',
|
||||
copyKind: 'command'
|
||||
})
|
||||
|
||||
useRunningTerminalCloseConfirmStore.getState().confirmRunningTerminalClose()
|
||||
expect(onClose).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('runs onCancel and keeps the tab when the confirmation is dismissed', async () => {
|
||||
const onClose = vi.fn()
|
||||
const onCancel = vi.fn()
|
||||
|
||||
guard(onClose, onCancel)
|
||||
await settleProbe()
|
||||
useRunningTerminalCloseConfirmStore.getState().dismissRunningTerminalClose()
|
||||
|
||||
expect(onCancel).toHaveBeenCalledTimes(1)
|
||||
expect(onClose).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('fails open and closes when the probe rejects (wedged relay / legacy provider)', async () => {
|
||||
inspectRuntimeTerminalProcessMock.mockRejectedValue(new Error('rpc_timeout'))
|
||||
const onClose = vi.fn()
|
||||
|
||||
guard(onClose)
|
||||
await settleProbe()
|
||||
|
||||
expect(onClose).toHaveBeenCalledTimes(1)
|
||||
expect(visibleRequest()).toBeNull()
|
||||
})
|
||||
|
||||
it('fails open when a remote handle reports the inspection as unavailable', async () => {
|
||||
inspectRuntimeTerminalProcessMock.mockResolvedValue({
|
||||
foregroundProcess: null,
|
||||
hasChildProcesses: true,
|
||||
unavailable: true
|
||||
})
|
||||
const onClose = vi.fn()
|
||||
|
||||
guard(onClose)
|
||||
await settleProbe()
|
||||
|
||||
expect(onClose).toHaveBeenCalledTimes(1)
|
||||
expect(visibleRequest()).toBeNull()
|
||||
})
|
||||
|
||||
it('prompts once for a split tab where only the second pane is busy', async () => {
|
||||
setState({
|
||||
ptyIdsByTabId: { 'tab-1': ['pty-a', 'pty-b'] },
|
||||
terminalLayoutsByTabId: {
|
||||
'tab-1': { ptyIdsByLeafId: { [LEAF_A]: 'pty-a', [LEAF_B]: 'pty-b' } }
|
||||
},
|
||||
agentStatusByPaneKey: { [`tab-1:${LEAF_B}`]: { agentType: 'claude' } }
|
||||
})
|
||||
inspectRuntimeTerminalProcessMock.mockImplementation(async (_settings, ptyId: string) => ({
|
||||
foregroundProcess: ptyId === 'pty-b' ? 'claude' : 'zsh',
|
||||
hasChildProcesses: ptyId === 'pty-b'
|
||||
}))
|
||||
const onClose = vi.fn()
|
||||
|
||||
guard(onClose)
|
||||
await settleProbe()
|
||||
|
||||
expect(inspectRuntimeTerminalProcessMock).toHaveBeenCalledTimes(2)
|
||||
expect(visibleRequest()).toMatchObject({ terminalTabId: 'tab-1', copyKind: 'agent' })
|
||||
expect(onClose).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not use an idle sibling pane to pick the agent copy', async () => {
|
||||
setState({
|
||||
ptyIdsByTabId: { 'tab-1': ['pty-a', 'pty-b'] },
|
||||
terminalLayoutsByTabId: {
|
||||
'tab-1': { ptyIdsByLeafId: { [LEAF_A]: 'pty-a', [LEAF_B]: 'pty-b' } }
|
||||
},
|
||||
agentStatusByPaneKey: { [`tab-1:${LEAF_B}`]: { agentType: 'claude' } }
|
||||
})
|
||||
inspectRuntimeTerminalProcessMock.mockImplementation(async (_settings, ptyId: string) => ({
|
||||
foregroundProcess: ptyId === 'pty-a' ? 'npm' : 'zsh',
|
||||
hasChildProcesses: ptyId === 'pty-a'
|
||||
}))
|
||||
|
||||
guard()
|
||||
await settleProbe()
|
||||
|
||||
expect(visibleRequest()?.copyKind).toBe('command')
|
||||
})
|
||||
|
||||
it('prefers the agent copy regardless of pty spawn order when both panes are busy', async () => {
|
||||
setState({
|
||||
ptyIdsByTabId: { 'tab-1': ['pty-a', 'pty-b'] },
|
||||
terminalLayoutsByTabId: {
|
||||
'tab-1': { ptyIdsByLeafId: { [LEAF_A]: 'pty-a', [LEAF_B]: 'pty-b' } }
|
||||
},
|
||||
agentStatusByPaneKey: { [`tab-1:${LEAF_B}`]: { agentType: 'codex' } }
|
||||
})
|
||||
|
||||
guard()
|
||||
await settleProbe()
|
||||
|
||||
expect(visibleRequest()?.copyKind).toBe('agent')
|
||||
})
|
||||
|
||||
it('ignores a pane whose agent status is unknown', async () => {
|
||||
setState({ agentStatusByPaneKey: { [`tab-1:${LEAF_A}`]: { agentType: 'unknown' } } })
|
||||
|
||||
guard()
|
||||
await settleProbe()
|
||||
|
||||
expect(visibleRequest()?.copyKind).toBe('command')
|
||||
})
|
||||
|
||||
it('survives legacy layout leaf ids that are not stable pane uuids', async () => {
|
||||
setState({ terminalLayoutsByTabId: { 'tab-1': { ptyIdsByLeafId: { leaf: 'pty-a' } } } })
|
||||
const onClose = vi.fn()
|
||||
|
||||
guard(onClose)
|
||||
await settleProbe()
|
||||
|
||||
expect(visibleRequest()).toMatchObject({ copyKind: 'command' })
|
||||
expect(onClose).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
// Why: makePaneKey throws on a tab id containing ':'; the dialog must never be the
|
||||
// reason a close silently stops happening.
|
||||
it('closes rather than wedging when the copy-kind lookup throws', async () => {
|
||||
getStateMock.mockReturnValue({
|
||||
settings: { activeRuntimeEnvironmentId: null },
|
||||
ptyIdsByTabId: { 'tab:1': ['pty-a'] },
|
||||
terminalLayoutsByTabId: { 'tab:1': { ptyIdsByLeafId: { [LEAF_A]: 'pty-a' } } },
|
||||
agentStatusByPaneKey: {}
|
||||
})
|
||||
const onClose = vi.fn()
|
||||
|
||||
guardRunningTerminalClose({ terminalTabId: 'tab:1', tabLabel: 'weird', onClose })
|
||||
await settleProbe()
|
||||
|
||||
expect(visibleRequest()).toMatchObject({ terminalTabId: 'tab:1', copyKind: 'command' })
|
||||
expect(onClose).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('closes rather than wedging when raising the confirmation throws', async () => {
|
||||
const requestSpy = vi
|
||||
.spyOn(useRunningTerminalCloseConfirmStore.getState(), 'requestRunningTerminalCloseConfirm')
|
||||
.mockImplementation(() => {
|
||||
throw new Error('subscriber blew up')
|
||||
})
|
||||
const onClose = vi.fn()
|
||||
|
||||
guard(onClose)
|
||||
await settleProbe()
|
||||
requestSpy.mockRestore()
|
||||
|
||||
expect(onClose).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
// Why: a remote inspect can take its full 15s RPC timeout. Closing at 4s would kill a
|
||||
// running remote command with no prompt — the exact failure this guard exists to stop.
|
||||
it('prompts instead of closing when a wedged remote probe never settles', async () => {
|
||||
vi.useFakeTimers()
|
||||
inspectRuntimeTerminalProcessMock.mockReturnValue(new Promise(() => {}))
|
||||
const onClose = vi.fn()
|
||||
|
||||
guard(onClose)
|
||||
expect(onClose).not.toHaveBeenCalled()
|
||||
|
||||
vi.advanceTimersByTime(RUNNING_CLOSE_PROBE_TIMEOUT_MS)
|
||||
vi.useRealTimers()
|
||||
|
||||
expect(onClose).not.toHaveBeenCalled()
|
||||
expect(visibleRequest()).toMatchObject({ terminalTabId: 'tab-1', tabLabel: 'npm run dev' })
|
||||
|
||||
useRunningTerminalCloseConfirmStore.getState().confirmRunningTerminalClose()
|
||||
expect(onClose).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('treats every pane as a candidate when picking the copy for a timed-out probe', async () => {
|
||||
setState({
|
||||
ptyIdsByTabId: { 'tab-1': ['pty-a', 'pty-b'] },
|
||||
terminalLayoutsByTabId: {
|
||||
'tab-1': { ptyIdsByLeafId: { [LEAF_A]: 'pty-a', [LEAF_B]: 'pty-b' } }
|
||||
},
|
||||
agentStatusByPaneKey: { [`tab-1:${LEAF_B}`]: { agentType: 'claude' } }
|
||||
})
|
||||
vi.useFakeTimers()
|
||||
inspectRuntimeTerminalProcessMock.mockReturnValue(new Promise(() => {}))
|
||||
|
||||
guard()
|
||||
vi.advanceTimersByTime(RUNNING_CLOSE_PROBE_TIMEOUT_MS)
|
||||
vi.useRealTimers()
|
||||
|
||||
expect(visibleRequest()?.copyKind).toBe('agent')
|
||||
})
|
||||
|
||||
it('closes rather than wedging when the timed-out prompt throws', async () => {
|
||||
const requestSpy = vi
|
||||
.spyOn(useRunningTerminalCloseConfirmStore.getState(), 'requestRunningTerminalCloseConfirm')
|
||||
.mockImplementation(() => {
|
||||
throw new Error('subscriber blew up')
|
||||
})
|
||||
vi.useFakeTimers()
|
||||
inspectRuntimeTerminalProcessMock.mockReturnValue(new Promise(() => {}))
|
||||
const onClose = vi.fn()
|
||||
|
||||
guard(onClose)
|
||||
vi.advanceTimersByTime(RUNNING_CLOSE_PROBE_TIMEOUT_MS)
|
||||
vi.useRealTimers()
|
||||
requestSpy.mockRestore()
|
||||
|
||||
expect(onClose).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('ignores a slow probe that resolves after the timeout already prompted', async () => {
|
||||
vi.useFakeTimers()
|
||||
inspectRuntimeTerminalProcessMock.mockResolvedValue({
|
||||
foregroundProcess: 'sleep',
|
||||
hasChildProcesses: true
|
||||
})
|
||||
const onClose = vi.fn()
|
||||
|
||||
guard(onClose)
|
||||
vi.advanceTimersByTime(RUNNING_CLOSE_PROBE_TIMEOUT_MS)
|
||||
vi.useRealTimers()
|
||||
await settleProbe()
|
||||
|
||||
expect(onClose).not.toHaveBeenCalled()
|
||||
useRunningTerminalCloseConfirmStore.getState().confirmRunningTerminalClose()
|
||||
expect(onClose).toHaveBeenCalledTimes(1)
|
||||
expect(visibleRequest()).toBeNull()
|
||||
})
|
||||
|
||||
// Why: an SSH drop zeroes ptyIdsByTabId while the layout still names the pane. The stale
|
||||
// binding is probed, that probe fails on the dead link, and the close falls open — so a
|
||||
// reconnecting tab stays closable instead of being blocked behind a prompt for a pty
|
||||
// nobody can reach. Documented so the behavior is a decision, not an accident.
|
||||
it('closes a reconnecting ssh tab whose pty ids were already zeroed', async () => {
|
||||
setState({ ptyIdsByTabId: { 'tab-1': [] } })
|
||||
inspectRuntimeTerminalProcessMock.mockRejectedValue(new Error('ssh_disconnected'))
|
||||
const onClose = vi.fn()
|
||||
|
||||
guard(onClose)
|
||||
await settleProbe()
|
||||
|
||||
expect(onClose).toHaveBeenCalledTimes(1)
|
||||
expect(visibleRequest()).toBeNull()
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,154 @@
|
|||
import { useAppStore } from '@/store'
|
||||
import { inspectRuntimeTerminalProcess } from '@/runtime/runtime-terminal-inspection'
|
||||
import { useRunningTerminalCloseConfirmStore } from '@/store/running-terminal-close-confirm'
|
||||
import type { TerminalTabCloseReason } from '@/store/slices/terminal-tab-retirement'
|
||||
import type { AppState } from '@/store/types'
|
||||
import { resolveBusyPtyCloseCopyKind } from './terminal-close-copy-kind'
|
||||
|
||||
export type RunningTerminalCloseGuardOptions = {
|
||||
force?: boolean
|
||||
rejectPinned?: boolean
|
||||
reason?: TerminalTabCloseReason
|
||||
hostCloseReason?: TerminalTabCloseReason
|
||||
lifecyclePtyId?: string
|
||||
skipRunningProcessConfirm?: boolean
|
||||
}
|
||||
|
||||
/** Upper bound on how long a close may wait on the probe before it asks instead. A remote
|
||||
* inspect RPC can hang for its full 15s timeout, and an X button that looks dead for 15s
|
||||
* is the same class of bug as one that never asks — but an unanswered probe is not
|
||||
* evidence of an idle shell, so the timeout raises the prompt rather than killing a
|
||||
* possibly-running remote command (#10142). */
|
||||
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. */
|
||||
export function shouldConfirmRunningTerminalClose(
|
||||
options?: RunningTerminalCloseGuardOptions
|
||||
): boolean {
|
||||
if (options?.force === true || options?.rejectPinned === true) {
|
||||
return false
|
||||
}
|
||||
if (options?.skipRunningProcessConfirm === true || options?.lifecyclePtyId !== undefined) {
|
||||
return false
|
||||
}
|
||||
const isUserReason = (reason: TerminalTabCloseReason | undefined): boolean =>
|
||||
reason === undefined || reason === 'user'
|
||||
return isUserReason(options?.reason) && isUserReason(options?.hostCloseReason)
|
||||
}
|
||||
|
||||
/** Every PTY the tab could still own. `ptyIdsByTabId` is the liveness map the rest of the
|
||||
* app reads, but a mounting pane is bound into the layout before the map catches up, and
|
||||
* 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(
|
||||
state: Pick<AppState, 'ptyIdsByTabId' | 'terminalLayoutsByTabId'>,
|
||||
terminalTabId: string
|
||||
): string[] {
|
||||
const ptyIds = new Set<string>()
|
||||
for (const ptyId of state.ptyIdsByTabId?.[terminalTabId] ?? []) {
|
||||
if (ptyId) {
|
||||
ptyIds.add(ptyId)
|
||||
}
|
||||
}
|
||||
const ptyIdsByLeafId = state.terminalLayoutsByTabId?.[terminalTabId]?.ptyIdsByLeafId ?? {}
|
||||
for (const ptyId of Object.values(ptyIdsByLeafId)) {
|
||||
if (typeof ptyId === 'string' && ptyId) {
|
||||
ptyIds.add(ptyId)
|
||||
}
|
||||
}
|
||||
return [...ptyIds]
|
||||
}
|
||||
|
||||
/**
|
||||
* Routes an interactive terminal-tab close through the running-process confirmation.
|
||||
* Closes immediately when nothing is running, so idle tabs keep today's behavior.
|
||||
*/
|
||||
export function guardRunningTerminalClose(params: {
|
||||
terminalTabId: string
|
||||
tabLabel: string
|
||||
onClose: () => void
|
||||
onCancel?: () => void
|
||||
}): void {
|
||||
const { terminalTabId, tabLabel, onClose, onCancel } = params
|
||||
const state = useAppStore.getState()
|
||||
const settings = state.settings
|
||||
const ptyIds = collectTabPtyIds(state, terminalTabId)
|
||||
// Why: no PTY at all means there is nothing to probe (parked/hibernated tab, or a
|
||||
// teardown that already cleared both maps), and the opt-out setting means the answer is
|
||||
// already known. Both keep the close fully synchronous.
|
||||
if (ptyIds.length === 0 || settings?.skipCloseTerminalWithRunningProcessConfirm === true) {
|
||||
onClose()
|
||||
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 closeNow = (): void => {
|
||||
if (decided) {
|
||||
return
|
||||
}
|
||||
decided = true
|
||||
onClose()
|
||||
}
|
||||
const confirmClose = (busyPtyIds: readonly string[]): void => {
|
||||
if (decided) {
|
||||
return
|
||||
}
|
||||
const copyKind = resolveBusyPtyCloseCopyKind(terminalTabId, busyPtyIds)
|
||||
useRunningTerminalCloseConfirmStore.getState().requestRunningTerminalCloseConfirm({
|
||||
terminalTabId,
|
||||
tabLabel,
|
||||
copyKind,
|
||||
onConfirm: onClose,
|
||||
...(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 probeTimeout = setTimeout(() => {
|
||||
try {
|
||||
// Why: a probe that has not answered yet is unknown, not idle. Ask, treating every pty
|
||||
// as a candidate, so a degraded relay costs a click instead of a killed remote command.
|
||||
confirmClose(ptyIds)
|
||||
} catch {
|
||||
closeNow()
|
||||
}
|
||||
}, RUNNING_CLOSE_PROBE_TIMEOUT_MS)
|
||||
|
||||
void Promise.allSettled(ptyIds.map((ptyId) => inspectRuntimeTerminalProcess(settings, ptyId)))
|
||||
.then((results) => {
|
||||
clearTimeout(probeTimeout)
|
||||
if (decided) {
|
||||
return
|
||||
}
|
||||
// Why: fail open on an *answered* probe, matching the Cmd+W pane path — a rejection
|
||||
// (wedged relay, legacy provider) or a stale remote handle is not evidence of a live
|
||||
// child, and a close button that silently does nothing is worse than closing a busy tab.
|
||||
const busyPtyIds = ptyIds.filter((_, index) => {
|
||||
const result = results[index]
|
||||
return (
|
||||
result?.status === 'fulfilled' &&
|
||||
result.value.hasChildProcesses &&
|
||||
result.value.unavailable !== true
|
||||
)
|
||||
})
|
||||
if (busyPtyIds.length === 0) {
|
||||
closeNow()
|
||||
return
|
||||
}
|
||||
confirmClose(busyPtyIds)
|
||||
})
|
||||
// Why: allSettled never rejects, so this only fires when the decision above throws (a
|
||||
// copy-kind lookup, a store subscriber). Without it the tab would silently never close
|
||||
// and the user would get no feedback at all; the pane path it replaced had this catch.
|
||||
.catch(() => {
|
||||
clearTimeout(probeTimeout)
|
||||
closeNow()
|
||||
})
|
||||
}
|
||||
|
|
@ -0,0 +1,131 @@
|
|||
/**
|
||||
* Regression for #10142: the tab X button / middle-click used to bypass the
|
||||
* running-process close confirmation that Cmd+W enforces. Assertions unchanged
|
||||
* from the adjudicated repro.
|
||||
*
|
||||
* Mouse close path: SortableTab (X onClick / onAuxClick button===1) -> onClose
|
||||
* -> Terminal.tsx handleCloseTab -> closeTerminalTab() -> running-process guard.
|
||||
* Keyboard path: Cmd+W -> TerminalPane.handleRequestClosePane -> inspectRuntimeTerminalProcess
|
||||
* (split panes) or closeTerminalTab's guard (last pane) -> CloseTerminalDialog.
|
||||
*/
|
||||
import { readFileSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const {
|
||||
requestPinnedTabCloseConfirmMock,
|
||||
getStateMock,
|
||||
inspectRuntimeTerminalProcessMock,
|
||||
isWebRuntimeSessionActiveMock,
|
||||
resolveHostSessionTabIdForWebSessionTabMock
|
||||
} = vi.hoisted(() => ({
|
||||
requestPinnedTabCloseConfirmMock: vi.fn(),
|
||||
getStateMock: vi.fn(),
|
||||
inspectRuntimeTerminalProcessMock: vi.fn(),
|
||||
isWebRuntimeSessionActiveMock: vi.fn(() => false),
|
||||
resolveHostSessionTabIdForWebSessionTabMock: vi.fn<() => string | null>(() => null)
|
||||
}))
|
||||
|
||||
vi.mock('@/store', () => ({
|
||||
useAppStore: { getState: getStateMock }
|
||||
}))
|
||||
|
||||
vi.mock('@/runtime/web-runtime-session', () => ({
|
||||
activateWebRuntimeSessionTab: vi.fn(),
|
||||
closeWebRuntimeSessionTab: vi.fn(),
|
||||
createWebRuntimeSessionTerminal: vi.fn(),
|
||||
isWebRuntimeSessionActive: isWebRuntimeSessionActiveMock,
|
||||
isWebTerminalSurfaceTabId: vi.fn(() => false),
|
||||
toHostSessionTabId: vi.fn((tabId: string) => tabId)
|
||||
}))
|
||||
|
||||
vi.mock('@/runtime/web-session-tabs-sync', () => ({
|
||||
getLatestWebSessionTabsPublicationEpoch: vi.fn(() => 'epoch-1'),
|
||||
resolveHostSessionTabIdForWebSessionTab: resolveHostSessionTabIdForWebSessionTabMock
|
||||
}))
|
||||
|
||||
vi.mock('@/runtime/runtime-terminal-inspection', () => ({
|
||||
inspectRuntimeTerminalProcess: inspectRuntimeTerminalProcessMock
|
||||
}))
|
||||
|
||||
import { closeTerminalTab } from './terminal-tab-actions'
|
||||
|
||||
// A non-pinned terminal tab whose PTY has a live child process (e.g. `sleep 300`).
|
||||
function stateWithBusyTerminalTab(closeTab: () => void): Record<string, unknown> {
|
||||
return {
|
||||
settings: { activeRuntimeEnvironmentId: null, confirmClosePinnedTab: true },
|
||||
tabsByWorktree: { 'wt-1': [{ id: 'tab-busy' }, { id: 'tab-other' }] },
|
||||
unifiedTabsByWorktree: {
|
||||
'wt-1': [
|
||||
{ id: 'tab-busy', entityId: 'tab-busy', contentType: 'terminal', isPinned: false },
|
||||
{ id: 'tab-other', entityId: 'tab-other', contentType: 'terminal', isPinned: false }
|
||||
]
|
||||
},
|
||||
ptyIdsByTabId: { 'tab-busy': ['pty-busy'] },
|
||||
terminalLayoutsByTabId: { 'tab-busy': { ptyIdsByLeafId: { leaf: 'pty-busy' } } },
|
||||
agentStatusByPaneKey: {},
|
||||
openFiles: [],
|
||||
browserTabsByWorktree: {},
|
||||
activeWorktreeId: 'wt-1',
|
||||
activeTabId: 'tab-busy',
|
||||
closeTab,
|
||||
requestPinnedTabCloseConfirm: requestPinnedTabCloseConfirmMock,
|
||||
setActiveTab: vi.fn(),
|
||||
setActiveFile: vi.fn(),
|
||||
setActiveTabType: vi.fn(),
|
||||
setActiveBrowserTab: vi.fn(),
|
||||
setActiveWorktree: vi.fn()
|
||||
}
|
||||
}
|
||||
|
||||
describe('#10142 close confirmation policy is the same for keyboard and mouse', () => {
|
||||
const closeTab = vi.fn()
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
getStateMock.mockReturnValue(stateWithBusyTerminalTab(closeTab))
|
||||
isWebRuntimeSessionActiveMock.mockReturnValue(false)
|
||||
resolveHostSessionTabIdForWebSessionTabMock.mockReturnValue(null)
|
||||
inspectRuntimeTerminalProcessMock.mockResolvedValue({
|
||||
foregroundProcess: 'sleep',
|
||||
hasChildProcesses: true
|
||||
})
|
||||
})
|
||||
|
||||
// Control: the keyboard entry point does probe for running children.
|
||||
it('keyboard Cmd+W path probes for running child processes before closing', () => {
|
||||
const source = readFileSync(join(__dirname, '../terminal-pane/TerminalPane.tsx'), 'utf8')
|
||||
const handler = source.slice(source.indexOf('const handleRequestClosePane'))
|
||||
expect(handler.slice(0, handler.indexOf('useImperativeHandle'))).toContain(
|
||||
'inspectRuntimeTerminalProcess'
|
||||
)
|
||||
})
|
||||
|
||||
// Control: the harness does observe a guard when one exists — pinning blocks the same mouse close.
|
||||
it('mouse close routes a pinned tab through its confirmation guard', () => {
|
||||
const state = stateWithBusyTerminalTab(closeTab)
|
||||
;(state.unifiedTabsByWorktree as Record<string, { isPinned: boolean }[]>)[
|
||||
'wt-1'
|
||||
]![0]!.isPinned = true
|
||||
getStateMock.mockReturnValue(state)
|
||||
|
||||
closeTerminalTab('tab-busy')
|
||||
|
||||
expect(requestPinnedTabCloseConfirmMock).toHaveBeenCalled()
|
||||
expect(closeTab).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('mouse close (X button / middle-click) consults the running-process probe', async () => {
|
||||
closeTerminalTab('tab-busy')
|
||||
await Promise.resolve()
|
||||
|
||||
expect(inspectRuntimeTerminalProcessMock).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('mouse close (X button / middle-click) does not drop a busy tab without confirmation', async () => {
|
||||
closeTerminalTab('tab-busy')
|
||||
await Promise.resolve()
|
||||
|
||||
expect(closeTab).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,67 @@
|
|||
/**
|
||||
* The keyboard (pane-scoped) and tab-strip (pty-scoped) close prompts must word the same
|
||||
* close the same way, so both resolve their copy through this module (#10142).
|
||||
*/
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { getStateMock } = vi.hoisted(() => ({ getStateMock: vi.fn() }))
|
||||
|
||||
vi.mock('@/store', () => ({ useAppStore: { getState: getStateMock } }))
|
||||
|
||||
import { resolveBusyPtyCloseCopyKind, resolveLeafCloseCopyKind } from './terminal-close-copy-kind'
|
||||
|
||||
const LEAF_A = '11111111-1111-4111-8111-111111111111'
|
||||
const LEAF_B = '22222222-2222-4222-8222-222222222222'
|
||||
|
||||
function setState(overrides: Record<string, unknown> = {}): void {
|
||||
getStateMock.mockReturnValue({
|
||||
terminalLayoutsByTabId: {
|
||||
'tab-1': { ptyIdsByLeafId: { [LEAF_A]: 'pty-a', [LEAF_B]: 'pty-b' } }
|
||||
},
|
||||
agentStatusByPaneKey: { [`tab-1:${LEAF_B}`]: { agentType: 'claude' } },
|
||||
...overrides
|
||||
})
|
||||
}
|
||||
|
||||
describe('terminal close copy kind', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
setState()
|
||||
})
|
||||
|
||||
it('agrees between the pane lookup and the busy-pty lookup for the same pane', () => {
|
||||
expect(resolveLeafCloseCopyKind('tab-1', LEAF_B)).toBe('agent')
|
||||
expect(resolveBusyPtyCloseCopyKind('tab-1', ['pty-b'])).toBe('agent')
|
||||
|
||||
expect(resolveLeafCloseCopyKind('tab-1', LEAF_A)).toBe('command')
|
||||
expect(resolveBusyPtyCloseCopyKind('tab-1', ['pty-a'])).toBe('command')
|
||||
})
|
||||
|
||||
it('prefers the agent copy when a split has both busy', () => {
|
||||
expect(resolveBusyPtyCloseCopyKind('tab-1', ['pty-a', 'pty-b'])).toBe('agent')
|
||||
})
|
||||
|
||||
it.each([
|
||||
['a missing leaf id', undefined],
|
||||
['a null leaf id', null],
|
||||
['a legacy non-uuid leaf id', 'leaf']
|
||||
])('falls back to the command copy for %s', (_label, leafId) => {
|
||||
expect(resolveLeafCloseCopyKind('tab-1', leafId)).toBe('command')
|
||||
})
|
||||
|
||||
it('never throws on a tab id that makePaneKey would reject', () => {
|
||||
setState({
|
||||
terminalLayoutsByTabId: { 'tab:1': { ptyIdsByLeafId: { [LEAF_B]: 'pty-b' } } }
|
||||
})
|
||||
|
||||
expect(resolveLeafCloseCopyKind('tab:1', LEAF_B)).toBe('command')
|
||||
expect(resolveBusyPtyCloseCopyKind('tab:1', ['pty-b'])).toBe('command')
|
||||
})
|
||||
|
||||
it('ignores an unknown agent type', () => {
|
||||
setState({ agentStatusByPaneKey: { [`tab-1:${LEAF_B}`]: { agentType: 'unknown' } } })
|
||||
|
||||
expect(resolveLeafCloseCopyKind('tab-1', LEAF_B)).toBe('command')
|
||||
expect(resolveBusyPtyCloseCopyKind('tab-1', ['pty-b'])).toBe('command')
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,38 @@
|
|||
import { useAppStore } from '@/store'
|
||||
import type { CloseTerminalDialogCopyKind } from '../terminal-pane/CloseTerminalDialog'
|
||||
import { isTerminalLeafId, makePaneKey } from '../../../../shared/stable-pane-id'
|
||||
|
||||
/**
|
||||
* Single source of truth for "is this pane an agent?" in the close confirmation, so the
|
||||
* keyboard (pane-scoped) and tab-strip (pty-scoped) prompts cannot word the same close
|
||||
* differently. Each caller resolves its own leaf id; only the policy is shared.
|
||||
*/
|
||||
export function resolveLeafCloseCopyKind(
|
||||
tabId: string,
|
||||
leafId: string | null | undefined
|
||||
): CloseTerminalDialogCopyKind {
|
||||
// Why: legacy layouts and mid-attach panes carry non-UUID or missing leaf ids, and
|
||||
// makePaneKey throws on those — a dialog must never be the thing that breaks a close.
|
||||
if (!leafId || !isTerminalLeafId(leafId) || !tabId || tabId.includes(':')) {
|
||||
return 'command'
|
||||
}
|
||||
const agentStatusByPaneKey = useAppStore.getState().agentStatusByPaneKey ?? {}
|
||||
const agentType = agentStatusByPaneKey[makePaneKey(tabId, leafId)]?.agentType
|
||||
return agentType && agentType !== 'unknown' ? 'agent' : 'command'
|
||||
}
|
||||
|
||||
/** Copy for a whole-tab close, given the PTYs that reported a live child. Agent panes win
|
||||
* in a mixed split: stopping an agent mid-task is the costlier surprise. */
|
||||
export function resolveBusyPtyCloseCopyKind(
|
||||
tabId: string,
|
||||
busyPtyIds: readonly string[]
|
||||
): CloseTerminalDialogCopyKind {
|
||||
const ptyIdsByLeafId =
|
||||
useAppStore.getState().terminalLayoutsByTabId?.[tabId]?.ptyIdsByLeafId ?? {}
|
||||
for (const [leafId, ptyId] of Object.entries(ptyIdsByLeafId)) {
|
||||
if (busyPtyIds.includes(ptyId) && resolveLeafCloseCopyKind(tabId, leafId) === 'agent') {
|
||||
return 'agent'
|
||||
}
|
||||
}
|
||||
return 'command'
|
||||
}
|
||||
|
|
@ -9,11 +9,20 @@ import {
|
|||
resolveHostSessionTabIdForWebSessionTab
|
||||
} from '@/runtime/web-session-tabs-sync'
|
||||
import { resolveTerminalWorktreeRoute } from '@/lib/terminal-worktree-route'
|
||||
import { guardPinnedTabClose, resolvePinnedTabLabel } from '@/store/pinned-tab-close-guard'
|
||||
import {
|
||||
guardPinnedTabClose,
|
||||
isUnifiedTabPinned,
|
||||
resolvePinnedTabLabel,
|
||||
shouldConfirmPinnedTabClose
|
||||
} from '@/store/pinned-tab-close-guard'
|
||||
import type {
|
||||
TerminalTabCloseReason,
|
||||
TerminalTabRetirementPlan
|
||||
} from '@/store/slices/terminal-tab-retirement'
|
||||
import {
|
||||
guardRunningTerminalClose,
|
||||
shouldConfirmRunningTerminalClose
|
||||
} from './running-terminal-close-guard'
|
||||
import { closeLocalTerminalTabState } from './close-local-terminal-tab-state'
|
||||
import { getTerminalIncarnationHandle } from './terminal-close-incarnation'
|
||||
import {
|
||||
|
|
@ -25,20 +34,6 @@ import {
|
|||
export type { PrecomputedTerminalCloseState } from './terminal-close-target'
|
||||
export { closeOtherTerminalTabs, closeTerminalTabsToRight } from './terminal-tab-bulk-actions'
|
||||
|
||||
type TerminalTabActionState = ReturnType<typeof useAppStore.getState>
|
||||
|
||||
function isPinnedVisibleTab(
|
||||
state: TerminalTabActionState,
|
||||
worktreeId: string,
|
||||
visibleId: string
|
||||
): boolean {
|
||||
return (
|
||||
(state.unifiedTabsByWorktree?.[worktreeId] ?? []).some(
|
||||
(tab) => (tab.id === visibleId || tab.entityId === visibleId) && tab.isPinned
|
||||
) ?? false
|
||||
)
|
||||
}
|
||||
|
||||
export function closeTerminalTab(
|
||||
tabId: string,
|
||||
options?: {
|
||||
|
|
@ -51,6 +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. */
|
||||
skipRunningProcessConfirm?: boolean
|
||||
captureRecentlyClosed?: boolean
|
||||
localPtyTeardownOwnedExternally?: boolean
|
||||
precomputedRetirementPlan?: TerminalTabRetirementPlan
|
||||
|
|
@ -82,7 +80,7 @@ export function closeTerminalTab(
|
|||
if (
|
||||
options?.reason !== 'pty-exit' &&
|
||||
!options?.force &&
|
||||
isPinnedVisibleTab(state, owningWorktreeId, terminalTabId)
|
||||
isUnifiedTabPinned(state, owningWorktreeId, terminalTabId)
|
||||
) {
|
||||
// Why: background lifecycle callers cannot safely wait on a modal whose
|
||||
// owner may be unattended; reject pinned tabs without bypassing the guard.
|
||||
|
|
@ -90,10 +88,30 @@ export function closeTerminalTab(
|
|||
options.onCancel?.()
|
||||
return
|
||||
}
|
||||
guardPinnedTabClose({
|
||||
isPinned: true,
|
||||
// Why: the pin prompt supersedes the running-process one only when it actually
|
||||
// appears. With `confirmClosePinnedTab` off it says nothing, so fall through and let
|
||||
// a busy pinned tab still get asked — Cmd+W did exactly that before #10142.
|
||||
if (shouldConfirmPinnedTabClose(state)) {
|
||||
guardPinnedTabClose({
|
||||
isPinned: true,
|
||||
tabLabel: resolvePinnedTabLabel(state, owningWorktreeId, terminalTabId),
|
||||
onClose: () => closeTerminalTab(tabId, { ...options, force: true }),
|
||||
...(options?.onCancel ? { onCancel: options.onCancel } : {})
|
||||
})
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
// Why: the X button, middle-click and the tab menu used to skip the running-process
|
||||
// prompt that Cmd+W enforced (#10142). Guarding here — above the web-runtime branch so
|
||||
// host-backed tabs are covered too — gives every close path one shared policy.
|
||||
if (shouldConfirmRunningTerminalClose(options)) {
|
||||
guardRunningTerminalClose({
|
||||
terminalTabId,
|
||||
tabLabel: resolvePinnedTabLabel(state, owningWorktreeId, terminalTabId),
|
||||
onClose: () => closeTerminalTab(tabId, { ...options, force: true }),
|
||||
// Why: re-enter instead of continuing inline so pinned/route/precomputed state is
|
||||
// re-validated against fresh state after an arbitrarily long dialog.
|
||||
onClose: () => closeTerminalTab(tabId, { ...options, skipRunningProcessConfirm: true }),
|
||||
...(options?.onCancel ? { onCancel: options.onCancel } : {})
|
||||
})
|
||||
return
|
||||
|
|
|
|||
|
|
@ -0,0 +1,263 @@
|
|||
/**
|
||||
* Which `closeTerminalTab` callers reach the running-process confirmation (#10142).
|
||||
* Kept out of terminal-tab-actions.test.ts, which is already at its max-lines budget.
|
||||
*/
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const {
|
||||
closeWebRuntimeSessionTabMock,
|
||||
getStateMock,
|
||||
inspectRuntimeTerminalProcessMock,
|
||||
isWebRuntimeSessionActiveMock,
|
||||
requestPinnedTabCloseConfirmMock,
|
||||
resolveHostSessionTabIdForWebSessionTabMock
|
||||
} = vi.hoisted(() => ({
|
||||
closeWebRuntimeSessionTabMock: vi.fn(),
|
||||
getStateMock: vi.fn(),
|
||||
inspectRuntimeTerminalProcessMock: vi.fn(),
|
||||
isWebRuntimeSessionActiveMock: vi.fn(() => false),
|
||||
requestPinnedTabCloseConfirmMock: vi.fn(),
|
||||
resolveHostSessionTabIdForWebSessionTabMock: vi.fn<() => string | null>(() => null)
|
||||
}))
|
||||
|
||||
vi.mock('@/store', () => ({
|
||||
useAppStore: { getState: getStateMock }
|
||||
}))
|
||||
|
||||
vi.mock('@/runtime/web-runtime-session', () => ({
|
||||
activateWebRuntimeSessionTab: vi.fn(),
|
||||
closeWebRuntimeSessionTab: closeWebRuntimeSessionTabMock,
|
||||
createWebRuntimeSessionTerminal: vi.fn(),
|
||||
isWebRuntimeSessionActive: isWebRuntimeSessionActiveMock,
|
||||
isWebTerminalSurfaceTabId: vi.fn(() => false),
|
||||
toHostSessionTabId: vi.fn((tabId: string) => tabId)
|
||||
}))
|
||||
|
||||
vi.mock('@/runtime/web-session-tabs-sync', () => ({
|
||||
getLatestWebSessionTabsPublicationEpoch: vi.fn(() => 'epoch-1'),
|
||||
resolveHostSessionTabIdForWebSessionTab: resolveHostSessionTabIdForWebSessionTabMock
|
||||
}))
|
||||
|
||||
vi.mock('@/runtime/runtime-terminal-inspection', () => ({
|
||||
inspectRuntimeTerminalProcess: inspectRuntimeTerminalProcessMock
|
||||
}))
|
||||
|
||||
import { useRunningTerminalCloseConfirmStore } from '@/store/running-terminal-close-confirm'
|
||||
import { closeTerminalTab } from './terminal-tab-actions'
|
||||
|
||||
const LEAF = '33333333-3333-4333-8333-333333333333'
|
||||
|
||||
const closeTab = vi.fn()
|
||||
|
||||
function busyTabState(overrides: Record<string, unknown> = {}): Record<string, unknown> {
|
||||
return {
|
||||
settings: { activeRuntimeEnvironmentId: null, confirmClosePinnedTab: true },
|
||||
tabsByWorktree: { 'wt-1': [{ id: 'tab-busy' }, { id: 'tab-other' }] },
|
||||
unifiedTabsByWorktree: {
|
||||
'wt-1': [
|
||||
{
|
||||
id: 'tab-busy',
|
||||
entityId: 'tab-busy',
|
||||
contentType: 'terminal',
|
||||
isPinned: false,
|
||||
label: 'dev server'
|
||||
},
|
||||
{ id: 'tab-other', entityId: 'tab-other', contentType: 'terminal', isPinned: false }
|
||||
]
|
||||
},
|
||||
ptyIdsByTabId: { 'tab-busy': ['pty-busy'] },
|
||||
terminalLayoutsByTabId: { 'tab-busy': { ptyIdsByLeafId: { [LEAF]: 'pty-busy' } } },
|
||||
agentStatusByPaneKey: {},
|
||||
openFiles: [],
|
||||
browserTabsByWorktree: {},
|
||||
activeWorktreeId: 'wt-1',
|
||||
activeTabId: 'tab-busy',
|
||||
closeTab,
|
||||
requestPinnedTabCloseConfirm: requestPinnedTabCloseConfirmMock,
|
||||
setActiveTab: vi.fn(),
|
||||
setActiveFile: vi.fn(),
|
||||
setActiveTabType: vi.fn(),
|
||||
setActiveBrowserTab: vi.fn(),
|
||||
setActiveWorktree: vi.fn(),
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
|
||||
function visibleRequest() {
|
||||
return useRunningTerminalCloseConfirmStore.getState().runningTerminalCloseConfirm
|
||||
}
|
||||
|
||||
async function settleProbe(): Promise<void> {
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
}
|
||||
|
||||
describe('closeTerminalTab running-process confirmation', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
getStateMock.mockReturnValue(busyTabState())
|
||||
isWebRuntimeSessionActiveMock.mockReturnValue(false)
|
||||
resolveHostSessionTabIdForWebSessionTabMock.mockReturnValue(null)
|
||||
inspectRuntimeTerminalProcessMock.mockResolvedValue({
|
||||
foregroundProcess: 'npm',
|
||||
hasChildProcesses: true
|
||||
})
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
const store = useRunningTerminalCloseConfirmStore.getState()
|
||||
while (visibleRequest()) {
|
||||
store.dismissRunningTerminalClose()
|
||||
}
|
||||
})
|
||||
|
||||
it('closes a busy tab only after the confirmation is accepted', async () => {
|
||||
closeTerminalTab('tab-busy')
|
||||
await settleProbe()
|
||||
|
||||
expect(closeTab).not.toHaveBeenCalled()
|
||||
expect(visibleRequest()).toMatchObject({
|
||||
terminalTabId: 'tab-busy',
|
||||
tabLabel: 'dev server',
|
||||
copyKind: 'command'
|
||||
})
|
||||
|
||||
useRunningTerminalCloseConfirmStore.getState().confirmRunningTerminalClose()
|
||||
|
||||
expect(closeTab).toHaveBeenCalledWith('tab-busy')
|
||||
})
|
||||
|
||||
it('keeps the tab and runs onCancel when the confirmation is dismissed', async () => {
|
||||
const onCancel = vi.fn()
|
||||
const onClosed = vi.fn()
|
||||
|
||||
closeTerminalTab('tab-busy', { onCancel, onClosed })
|
||||
await settleProbe()
|
||||
useRunningTerminalCloseConfirmStore.getState().dismissRunningTerminalClose()
|
||||
|
||||
expect(onCancel).toHaveBeenCalledTimes(1)
|
||||
expect(onClosed).not.toHaveBeenCalled()
|
||||
expect(closeTab).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('still reports the close through onClosed after the confirmation', async () => {
|
||||
const onClosed = vi.fn()
|
||||
|
||||
closeTerminalTab('tab-busy', { onClosed })
|
||||
await settleProbe()
|
||||
useRunningTerminalCloseConfirmStore.getState().confirmRunningTerminalClose()
|
||||
|
||||
expect(onClosed).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('stays fully synchronous for a tab with no live pty', () => {
|
||||
getStateMock.mockReturnValue(busyTabState({ ptyIdsByTabId: {}, terminalLayoutsByTabId: {} }))
|
||||
|
||||
closeTerminalTab('tab-busy')
|
||||
|
||||
expect(inspectRuntimeTerminalProcessMock).not.toHaveBeenCalled()
|
||||
expect(closeTab).toHaveBeenCalledWith('tab-busy')
|
||||
})
|
||||
|
||||
it.each([
|
||||
['force (post-confirmation re-entry)', { force: true }],
|
||||
['rejectPinned (CLI tab close request)', { rejectPinned: true }],
|
||||
['skipRunningProcessConfirm (bulk / CLI)', { skipRunningProcessConfirm: true }],
|
||||
['reason pty-exit', { reason: 'pty-exit' as const }],
|
||||
['reason cleanup', { reason: 'cleanup' as const }],
|
||||
['hostCloseReason pty-exit', { hostCloseReason: 'pty-exit' as const }],
|
||||
['lifecyclePtyId', { lifecyclePtyId: 'pty-busy' }]
|
||||
])('never probes for a close carrying %s', (_label, options) => {
|
||||
closeTerminalTab('tab-busy', options)
|
||||
|
||||
expect(inspectRuntimeTerminalProcessMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('shows only the pinned dialog for a pinned busy tab, and its confirm does not probe', () => {
|
||||
const state = busyTabState()
|
||||
;(state.unifiedTabsByWorktree as Record<string, { isPinned: boolean }[]>)[
|
||||
'wt-1'
|
||||
]![0]!.isPinned = true
|
||||
getStateMock.mockReturnValue(state)
|
||||
|
||||
closeTerminalTab('tab-busy')
|
||||
|
||||
expect(requestPinnedTabCloseConfirmMock).toHaveBeenCalledTimes(1)
|
||||
expect(inspectRuntimeTerminalProcessMock).not.toHaveBeenCalled()
|
||||
|
||||
const onConfirm = requestPinnedTabCloseConfirmMock.mock.calls[0]![0].onConfirm as () => void
|
||||
onConfirm()
|
||||
|
||||
expect(inspectRuntimeTerminalProcessMock).not.toHaveBeenCalled()
|
||||
expect(closeTab).toHaveBeenCalledWith('tab-busy')
|
||||
})
|
||||
|
||||
// Why: the pin prompt supersedes this one only when it actually appears. With the pin
|
||||
// confirmation off it says nothing, and the running command must still be announced.
|
||||
it('still asks about a running command on a pinned tab when pin confirmation is off', async () => {
|
||||
const state = busyTabState({
|
||||
settings: { activeRuntimeEnvironmentId: null, confirmClosePinnedTab: false }
|
||||
})
|
||||
;(state.unifiedTabsByWorktree as Record<string, { isPinned: boolean }[]>)[
|
||||
'wt-1'
|
||||
]![0]!.isPinned = true
|
||||
getStateMock.mockReturnValue(state)
|
||||
|
||||
closeTerminalTab('tab-busy')
|
||||
await settleProbe()
|
||||
|
||||
expect(requestPinnedTabCloseConfirmMock).not.toHaveBeenCalled()
|
||||
expect(closeTab).not.toHaveBeenCalled()
|
||||
expect(visibleRequest()).toMatchObject({ terminalTabId: 'tab-busy' })
|
||||
|
||||
useRunningTerminalCloseConfirmStore.getState().confirmRunningTerminalClose()
|
||||
|
||||
expect(closeTab).toHaveBeenCalledWith('tab-busy')
|
||||
})
|
||||
|
||||
it('rejects a pinned tab for a CLI close even when pin confirmation is off', () => {
|
||||
const state = busyTabState({
|
||||
settings: { activeRuntimeEnvironmentId: null, confirmClosePinnedTab: false }
|
||||
})
|
||||
;(state.unifiedTabsByWorktree as Record<string, { isPinned: boolean }[]>)[
|
||||
'wt-1'
|
||||
]![0]!.isPinned = true
|
||||
getStateMock.mockReturnValue(state)
|
||||
const onCancel = vi.fn()
|
||||
|
||||
closeTerminalTab('tab-busy', { rejectPinned: true, onCancel })
|
||||
|
||||
expect(onCancel).toHaveBeenCalledTimes(1)
|
||||
expect(inspectRuntimeTerminalProcessMock).not.toHaveBeenCalled()
|
||||
expect(closeTab).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('confirms before telling a paired host to close its busy tab', async () => {
|
||||
isWebRuntimeSessionActiveMock.mockReturnValue(true)
|
||||
getStateMock.mockReturnValue(
|
||||
busyTabState({ settings: { activeRuntimeEnvironmentId: 'web-runtime' } })
|
||||
)
|
||||
|
||||
closeTerminalTab('tab-busy')
|
||||
await settleProbe()
|
||||
|
||||
expect(closeWebRuntimeSessionTabMock).not.toHaveBeenCalled()
|
||||
expect(closeTab).not.toHaveBeenCalled()
|
||||
|
||||
useRunningTerminalCloseConfirmStore.getState().confirmRunningTerminalClose()
|
||||
|
||||
expect(closeWebRuntimeSessionTabMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('picks the agent copy for a busy agent pane', async () => {
|
||||
getStateMock.mockReturnValue(
|
||||
busyTabState({ agentStatusByPaneKey: { [`tab-busy:${LEAF}`]: { agentType: 'claude' } } })
|
||||
)
|
||||
|
||||
closeTerminalTab('tab-busy')
|
||||
await settleProbe()
|
||||
|
||||
expect(visibleRequest()?.copyKind).toBe('agent')
|
||||
})
|
||||
})
|
||||
|
|
@ -3382,7 +3382,10 @@ describe('useIpcEvents browser tab close routing', () => {
|
|||
|
||||
closeTerminalListenerRef.current?.({ tabId: 'terminal-1' })
|
||||
|
||||
expect(closeTerminalTabMock).toHaveBeenCalledWith('terminal-1')
|
||||
// The CLI/RPC caller is answered immediately, so this close must never raise a modal.
|
||||
expect(closeTerminalTabMock).toHaveBeenCalledWith('terminal-1', {
|
||||
skipRunningProcessConfirm: true
|
||||
})
|
||||
})
|
||||
|
||||
it('acknowledges whole-tab close only after the fresh session is durably persisted', async () => {
|
||||
|
|
|
|||
|
|
@ -1965,7 +1965,8 @@ export function useIpcEvents(): void {
|
|||
const detail: CloseTerminalPaneDetail = { tabId, paneRuntimeId }
|
||||
window.dispatchEvent(new CustomEvent(CLOSE_TERMINAL_PANE_EVENT, { detail }))
|
||||
} else {
|
||||
closeTerminalTab(tabId)
|
||||
// Why: the CLI/RPC caller is answered immediately, so it cannot wait on a modal.
|
||||
closeTerminalTab(tabId, { skipRunningProcessConfirm: true })
|
||||
}
|
||||
})
|
||||
)
|
||||
|
|
|
|||
|
|
@ -23,6 +23,13 @@ export function isUnifiedTabPinned(state: AppState, worktreeId: string, tabId: s
|
|||
)
|
||||
}
|
||||
|
||||
/** Whether a pinned close will actually raise the pin dialog. Callers that let the pin
|
||||
* prompt supersede another confirmation must know this: with the setting off the pin
|
||||
* has nothing to say, so it must not swallow the other prompt (#10142). */
|
||||
export function shouldConfirmPinnedTabClose(state: AppState): boolean {
|
||||
return state.settings?.confirmClosePinnedTab ?? true
|
||||
}
|
||||
|
||||
/** Routes a pinned-tab close attempt through the confirmation dialog when the
|
||||
* setting is on. Non-pinned tabs (and pinned tabs when the setting is off)
|
||||
* close immediately. Keeping every close path behind this single helper is why
|
||||
|
|
@ -40,8 +47,7 @@ export function guardPinnedTabClose(params: {
|
|||
}
|
||||
|
||||
const state = useAppStore.getState()
|
||||
const shouldConfirm = state.settings?.confirmClosePinnedTab ?? true
|
||||
if (!shouldConfirm) {
|
||||
if (!shouldConfirmPinnedTabClose(state)) {
|
||||
onClose()
|
||||
return
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,226 @@
|
|||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { useRunningTerminalCloseConfirmStore } from './running-terminal-close-confirm'
|
||||
|
||||
// The store rejects a second action within 350 ms of revealing a queued request, so these
|
||||
// tests drive a clock rather than letting wall time decide whether an action lands.
|
||||
let clock = 1_000
|
||||
|
||||
function advancePastGuard(): void {
|
||||
clock += 400
|
||||
}
|
||||
|
||||
function drainRequests(): void {
|
||||
const store = useRunningTerminalCloseConfirmStore.getState()
|
||||
while (useRunningTerminalCloseConfirmStore.getState().runningTerminalCloseConfirm) {
|
||||
advancePastGuard()
|
||||
store.dismissRunningTerminalClose()
|
||||
}
|
||||
}
|
||||
|
||||
function request(terminalTabId: string, onConfirm: () => void = vi.fn(), onCancel?: () => void) {
|
||||
return {
|
||||
terminalTabId,
|
||||
tabLabel: terminalTabId,
|
||||
copyKind: 'command' as const,
|
||||
onConfirm,
|
||||
...(onCancel ? { onCancel } : {})
|
||||
}
|
||||
}
|
||||
|
||||
describe('running terminal close confirmation store', () => {
|
||||
beforeEach(() => {
|
||||
// Monotonic across tests: the store is a singleton, so winding the clock back would
|
||||
// leave a previous test's guard deadline in the future and block every action.
|
||||
clock += 10_000
|
||||
vi.spyOn(Date, 'now').mockImplementation(() => clock)
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
drainRequests()
|
||||
vi.mocked(Date.now).mockRestore()
|
||||
})
|
||||
|
||||
it('shows the first request and queues the next one', () => {
|
||||
const first = vi.fn()
|
||||
const second = vi.fn()
|
||||
const store = useRunningTerminalCloseConfirmStore.getState()
|
||||
|
||||
store.requestRunningTerminalCloseConfirm(request('tab-1', first))
|
||||
store.requestRunningTerminalCloseConfirm(request('tab-2', second))
|
||||
|
||||
expect(
|
||||
useRunningTerminalCloseConfirmStore.getState().runningTerminalCloseConfirm?.terminalTabId
|
||||
).toBe('tab-1')
|
||||
|
||||
store.confirmRunningTerminalClose()
|
||||
|
||||
expect(first).toHaveBeenCalledTimes(1)
|
||||
expect(second).not.toHaveBeenCalled()
|
||||
expect(
|
||||
useRunningTerminalCloseConfirmStore.getState().runningTerminalCloseConfirm?.terminalTabId
|
||||
).toBe('tab-2')
|
||||
})
|
||||
|
||||
it('shows one prompt for a repeat request but still resolves both closes', () => {
|
||||
const first = vi.fn()
|
||||
const duplicate = vi.fn()
|
||||
const store = useRunningTerminalCloseConfirmStore.getState()
|
||||
|
||||
store.requestRunningTerminalCloseConfirm(request('tab-1', first))
|
||||
store.requestRunningTerminalCloseConfirm(request('tab-1', duplicate))
|
||||
store.confirmRunningTerminalClose()
|
||||
|
||||
expect(first).toHaveBeenCalledTimes(1)
|
||||
expect(duplicate).toHaveBeenCalledTimes(1)
|
||||
expect(useRunningTerminalCloseConfirmStore.getState().runningTerminalCloseConfirm).toBeNull()
|
||||
})
|
||||
|
||||
it('cancels both callers when a folded repeat request is dismissed', () => {
|
||||
const store = useRunningTerminalCloseConfirmStore.getState()
|
||||
const firstCancel = vi.fn()
|
||||
const duplicateCancel = vi.fn()
|
||||
|
||||
store.requestRunningTerminalCloseConfirm(request('tab-1', vi.fn(), firstCancel))
|
||||
store.requestRunningTerminalCloseConfirm(request('tab-1', vi.fn(), duplicateCancel))
|
||||
store.dismissRunningTerminalClose()
|
||||
|
||||
expect(firstCancel).toHaveBeenCalledTimes(1)
|
||||
expect(duplicateCancel).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('folds a repeat request into the one already waiting in the queue', () => {
|
||||
const store = useRunningTerminalCloseConfirmStore.getState()
|
||||
const queued = vi.fn()
|
||||
const duplicate = vi.fn()
|
||||
|
||||
store.requestRunningTerminalCloseConfirm(request('tab-1'))
|
||||
store.requestRunningTerminalCloseConfirm(request('tab-2', queued))
|
||||
store.requestRunningTerminalCloseConfirm(request('tab-2', duplicate))
|
||||
|
||||
store.confirmRunningTerminalClose()
|
||||
advancePastGuard()
|
||||
store.confirmRunningTerminalClose()
|
||||
|
||||
expect(queued).toHaveBeenCalledTimes(1)
|
||||
expect(duplicate).toHaveBeenCalledTimes(1)
|
||||
expect(useRunningTerminalCloseConfirmStore.getState().runningTerminalCloseConfirm).toBeNull()
|
||||
})
|
||||
|
||||
it('confirms every pending request once the user opts out of the prompt', () => {
|
||||
const store = useRunningTerminalCloseConfirmStore.getState()
|
||||
const visible = vi.fn()
|
||||
const queued = vi.fn()
|
||||
|
||||
store.requestRunningTerminalCloseConfirm(request('tab-1', visible))
|
||||
store.requestRunningTerminalCloseConfirm(request('tab-2', queued))
|
||||
store.confirmAllRunningTerminalCloses()
|
||||
|
||||
expect(visible).toHaveBeenCalledTimes(1)
|
||||
expect(queued).toHaveBeenCalledTimes(1)
|
||||
expect(useRunningTerminalCloseConfirmStore.getState().runningTerminalCloseConfirm).toBeNull()
|
||||
})
|
||||
|
||||
it('runs onCancel on dismiss and never the close', () => {
|
||||
const onConfirm = vi.fn()
|
||||
const onCancel = vi.fn()
|
||||
const store = useRunningTerminalCloseConfirmStore.getState()
|
||||
|
||||
store.requestRunningTerminalCloseConfirm(request('tab-1', onConfirm, onCancel))
|
||||
store.dismissRunningTerminalClose()
|
||||
|
||||
expect(onCancel).toHaveBeenCalledTimes(1)
|
||||
expect(onConfirm).not.toHaveBeenCalled()
|
||||
expect(useRunningTerminalCloseConfirmStore.getState().runningTerminalCloseConfirm).toBeNull()
|
||||
})
|
||||
|
||||
it('is inert when there is nothing to confirm', () => {
|
||||
const store = useRunningTerminalCloseConfirmStore.getState()
|
||||
|
||||
expect(() => store.confirmRunningTerminalClose()).not.toThrow()
|
||||
expect(() => store.dismissRunningTerminalClose()).not.toThrow()
|
||||
})
|
||||
|
||||
it('lets a re-entrant close from onConfirm queue behind the next request', () => {
|
||||
const store = useRunningTerminalCloseConfirmStore.getState()
|
||||
const reentrant = vi.fn()
|
||||
const second = vi.fn()
|
||||
|
||||
store.requestRunningTerminalCloseConfirm(
|
||||
request('tab-1', () => {
|
||||
// Why: the real onConfirm re-enters closeTerminalTab, which may request again.
|
||||
store.requestRunningTerminalCloseConfirm(request('tab-3', reentrant))
|
||||
})
|
||||
)
|
||||
store.requestRunningTerminalCloseConfirm(request('tab-2', second))
|
||||
store.confirmRunningTerminalClose()
|
||||
|
||||
expect(
|
||||
useRunningTerminalCloseConfirmStore.getState().runningTerminalCloseConfirm?.terminalTabId
|
||||
).toBe('tab-2')
|
||||
|
||||
advancePastGuard()
|
||||
store.confirmRunningTerminalClose()
|
||||
expect(second).toHaveBeenCalledTimes(1)
|
||||
expect(
|
||||
useRunningTerminalCloseConfirmStore.getState().runningTerminalCloseConfirm?.terminalTabId
|
||||
).toBe('tab-3')
|
||||
|
||||
advancePastGuard()
|
||||
store.confirmRunningTerminalClose()
|
||||
expect(reentrant).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
// Why: a queued request replaces the visible one in place. Without this window, the second
|
||||
// click of a double-click aimed at one tab lands on the next tab's prompt and kills a
|
||||
// running process the user never saw asked about — the bug class this PR exists to close.
|
||||
it('ignores a second action that lands on the freshly revealed request', () => {
|
||||
const store = useRunningTerminalCloseConfirmStore.getState()
|
||||
const first = vi.fn()
|
||||
const second = vi.fn()
|
||||
|
||||
store.requestRunningTerminalCloseConfirm(request('tab-1', first))
|
||||
store.requestRunningTerminalCloseConfirm(request('tab-2', second))
|
||||
|
||||
store.confirmRunningTerminalClose()
|
||||
store.confirmRunningTerminalClose()
|
||||
|
||||
expect(first).toHaveBeenCalledTimes(1)
|
||||
expect(second).not.toHaveBeenCalled()
|
||||
expect(
|
||||
useRunningTerminalCloseConfirmStore.getState().runningTerminalCloseConfirm?.terminalTabId
|
||||
).toBe('tab-2')
|
||||
|
||||
advancePastGuard()
|
||||
store.confirmRunningTerminalClose()
|
||||
expect(second).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('also holds off a dismiss that lands on the freshly revealed request', () => {
|
||||
const store = useRunningTerminalCloseConfirmStore.getState()
|
||||
const secondCancel = vi.fn()
|
||||
|
||||
store.requestRunningTerminalCloseConfirm(request('tab-1'))
|
||||
store.requestRunningTerminalCloseConfirm(request('tab-2', vi.fn(), secondCancel))
|
||||
|
||||
store.confirmRunningTerminalClose()
|
||||
store.dismissRunningTerminalClose()
|
||||
|
||||
expect(secondCancel).not.toHaveBeenCalled()
|
||||
expect(
|
||||
useRunningTerminalCloseConfirmStore.getState().runningTerminalCloseConfirm?.terminalTabId
|
||||
).toBe('tab-2')
|
||||
})
|
||||
|
||||
// Why: emptying the queue leaves nothing to mis-click, so the opt-out must not be delayed.
|
||||
it('arms no guard when the last request leaves the queue empty', () => {
|
||||
const store = useRunningTerminalCloseConfirmStore.getState()
|
||||
const onConfirm = vi.fn()
|
||||
|
||||
store.requestRunningTerminalCloseConfirm(request('tab-1'))
|
||||
store.confirmRunningTerminalClose()
|
||||
store.requestRunningTerminalCloseConfirm(request('tab-2', onConfirm))
|
||||
store.confirmRunningTerminalClose()
|
||||
|
||||
expect(onConfirm).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,132 @@
|
|||
import { create } from 'zustand'
|
||||
import type { CloseTerminalDialogCopyKind } from '@/components/terminal-pane/CloseTerminalDialog'
|
||||
|
||||
/** A pending confirmation for closing a terminal tab whose shell still has a
|
||||
* running child process. `onConfirm` performs the original close. */
|
||||
export type RunningTerminalCloseConfirmRequest = {
|
||||
terminalTabId: string
|
||||
tabLabel: string
|
||||
copyKind: CloseTerminalDialogCopyKind
|
||||
onConfirm: () => void
|
||||
onCancel?: () => void
|
||||
}
|
||||
|
||||
export type RunningTerminalCloseConfirmState = {
|
||||
runningTerminalCloseConfirm: RunningTerminalCloseConfirmRequest | null
|
||||
requestRunningTerminalCloseConfirm: (request: RunningTerminalCloseConfirmRequest) => void
|
||||
confirmRunningTerminalClose: () => void
|
||||
/** Accepts the visible request and every queued one. Used when the user ticks "don't ask
|
||||
* again": a prompt they just opted out of must not still be waiting behind this one. */
|
||||
confirmAllRunningTerminalCloses: () => void
|
||||
dismissRunningTerminalClose: () => void
|
||||
}
|
||||
|
||||
/** Folds a duplicate request for the same tab into the pending one instead of dropping it,
|
||||
* so two surfaces closing the same tab both get their callback. */
|
||||
function mergeRequests(
|
||||
pending: RunningTerminalCloseConfirmRequest,
|
||||
duplicate: RunningTerminalCloseConfirmRequest
|
||||
): RunningTerminalCloseConfirmRequest {
|
||||
return {
|
||||
...pending,
|
||||
onConfirm: () => {
|
||||
pending.onConfirm()
|
||||
duplicate.onConfirm()
|
||||
},
|
||||
onCancel: () => {
|
||||
pending.onCancel?.()
|
||||
duplicate.onCancel?.()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Why a standalone store instead of an AppState slice (which is what the sibling
|
||||
// pinned-tab confirmation uses): the request is raised from closeTerminalTab, a plain
|
||||
// module whose unit fixtures build partial app-state objects, so dispatching through
|
||||
// useAppStore.getState() would throw there. Nothing outside the dialog reads this state,
|
||||
// so the AppState coupling would buy nothing.
|
||||
export const useRunningTerminalCloseConfirmStore = create<RunningTerminalCloseConfirmState>()((
|
||||
set,
|
||||
get
|
||||
) => {
|
||||
const queuedRequests: RunningTerminalCloseConfirmRequest[] = []
|
||||
// Why: a queued request replaces the visible one in place, so a double-click or held
|
||||
// Enter meant for the tab the user was looking at would land on the next tab's prompt and
|
||||
// kill a second running process unseen. Matches the sibling pinned-tab confirmation.
|
||||
const INTER_REQUEST_ACTION_GUARD_MS = 350
|
||||
let nextRequestActionAllowedAt = 0
|
||||
|
||||
/** Reveals the next queued request, and reports whether one took the visible slot. */
|
||||
const advanceRequest = (): boolean => {
|
||||
const next = queuedRequests.shift() ?? null
|
||||
set({ runningTerminalCloseConfirm: next })
|
||||
return next !== null
|
||||
}
|
||||
|
||||
const guardNextAction = (revealedNextRequest: boolean): void => {
|
||||
if (revealedNextRequest) {
|
||||
nextRequestActionAllowedAt = Date.now() + INTER_REQUEST_ACTION_GUARD_MS
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
runningTerminalCloseConfirm: null,
|
||||
|
||||
requestRunningTerminalCloseConfirm: (request) => {
|
||||
const visible = get().runningTerminalCloseConfirm
|
||||
// Why: the probe is async, so a second click on the same tab arrives before the
|
||||
// dialog opens. One prompt, but both closes still resolve.
|
||||
if (visible?.terminalTabId === request.terminalTabId) {
|
||||
set({ runningTerminalCloseConfirm: mergeRequests(visible, request) })
|
||||
return
|
||||
}
|
||||
const queuedIndex = queuedRequests.findIndex(
|
||||
(queued) => queued.terminalTabId === request.terminalTabId
|
||||
)
|
||||
if (queuedIndex >= 0) {
|
||||
queuedRequests[queuedIndex] = mergeRequests(queuedRequests[queuedIndex]!, request)
|
||||
return
|
||||
}
|
||||
if (visible) {
|
||||
// Why: closing two busy tabs in quick succession must not strand the second
|
||||
// tab's close callback behind a replaced request.
|
||||
queuedRequests.push(request)
|
||||
return
|
||||
}
|
||||
set({ runningTerminalCloseConfirm: request })
|
||||
},
|
||||
|
||||
confirmRunningTerminalClose: () => {
|
||||
const request = get().runningTerminalCloseConfirm
|
||||
if (!request || Date.now() < nextRequestActionAllowedAt) {
|
||||
return
|
||||
}
|
||||
// Why: advance before running onConfirm so a re-entrant close queues behind the
|
||||
// next real request instead of seeing the stale one.
|
||||
guardNextAction(advanceRequest())
|
||||
request.onConfirm()
|
||||
},
|
||||
|
||||
confirmAllRunningTerminalCloses: () => {
|
||||
if (Date.now() < nextRequestActionAllowedAt) {
|
||||
return
|
||||
}
|
||||
const pending = [get().runningTerminalCloseConfirm, ...queuedRequests.splice(0)]
|
||||
set({ runningTerminalCloseConfirm: null })
|
||||
// No guard to arm: the queue is empty, so there is no next prompt to mis-click.
|
||||
for (const request of pending) {
|
||||
request?.onConfirm()
|
||||
}
|
||||
},
|
||||
|
||||
dismissRunningTerminalClose: () => {
|
||||
const request = get().runningTerminalCloseConfirm
|
||||
if (!request || Date.now() < nextRequestActionAllowedAt) {
|
||||
return
|
||||
}
|
||||
guardNextAction(advanceRequest())
|
||||
// Why: callers such as the tab-group model resume their own cleanup on cancel.
|
||||
request.onCancel?.()
|
||||
}
|
||||
}
|
||||
})
|
||||
|
|
@ -0,0 +1,101 @@
|
|||
/**
|
||||
* #10142 follow-ups to the X-button regression: middle-click prompts too, confirming
|
||||
* actually closes, and Cmd+W raises exactly one dialog (the pane path delegates the
|
||||
* last-pane close to closeTerminalTab instead of probing a second time).
|
||||
*/
|
||||
import { test, expect } from './helpers/orca-app'
|
||||
import type { Page } from '@stablyai/playwright-test'
|
||||
import {
|
||||
waitForSessionReady,
|
||||
waitForActiveWorktree,
|
||||
getActiveTabId,
|
||||
ensureTerminalVisible
|
||||
} from './helpers/store'
|
||||
import {
|
||||
execInTerminal,
|
||||
focusActiveTerminalInput,
|
||||
waitForActivePanePtyId,
|
||||
waitForActiveTerminalManager,
|
||||
waitForPaneCount,
|
||||
waitForTerminalOutput
|
||||
} from './helpers/terminal'
|
||||
|
||||
const SORTABLE_TAB = '[data-testid="sortable-tab"]'
|
||||
|
||||
function closeDialogTitle(page: Page) {
|
||||
return page.getByText(/Stop running command\?|Stop this agent\?/)
|
||||
}
|
||||
|
||||
async function startBusyTerminal(page: Page): Promise<string> {
|
||||
await waitForSessionReady(page)
|
||||
await waitForActiveWorktree(page)
|
||||
await ensureTerminalVisible(page)
|
||||
const hasPaneManager = await waitForActiveTerminalManager(page, 30_000)
|
||||
.then(() => true)
|
||||
.catch(() => false)
|
||||
test.skip(!hasPaneManager, 'Electron automation never mounted the live TerminalPane manager.')
|
||||
await waitForPaneCount(page, 1, 30_000)
|
||||
|
||||
const ptyId = await waitForActivePanePtyId(page)
|
||||
await execInTerminal(page, ptyId, 'echo close-confirm-ready')
|
||||
await waitForTerminalOutput(page, 'close-confirm-ready', 20_000)
|
||||
await execInTerminal(page, ptyId, 'sleep 300')
|
||||
// Why: `hasChildProcesses` is already true while macOS's `login` wrapper starts the
|
||||
// shell, so wait for `sleep` itself or the close legitimately sees an idle terminal.
|
||||
await expect
|
||||
.poll(
|
||||
async () =>
|
||||
(await page.evaluate((id) => window.api.pty.inspectProcess(id), ptyId)).foregroundProcess,
|
||||
{ timeout: 20_000, message: 'sleep 300 never became the foreground process' }
|
||||
)
|
||||
.toBe('sleep')
|
||||
return (await getActiveTabId(page))!
|
||||
}
|
||||
|
||||
test.describe.configure({ mode: 'serial' })
|
||||
|
||||
test('middle-clicking a busy tab prompts, and cancelling keeps the tab', async ({ orcaPage }) => {
|
||||
test.setTimeout(120_000)
|
||||
const busyTabId = await startBusyTerminal(orcaPage)
|
||||
const busyTab = orcaPage.locator(`${SORTABLE_TAB}[data-tab-id="${busyTabId}"]`).first()
|
||||
const tabsBefore = await orcaPage.locator(SORTABLE_TAB).count()
|
||||
|
||||
await busyTab.click({ button: 'middle' })
|
||||
|
||||
await expect(closeDialogTitle(orcaPage)).toBeVisible({ timeout: 15_000 })
|
||||
await orcaPage.getByRole('button', { name: /^Cancel$/ }).click()
|
||||
await expect(closeDialogTitle(orcaPage)).toBeHidden()
|
||||
await expect(busyTab).toBeVisible()
|
||||
expect(await orcaPage.locator(SORTABLE_TAB).count()).toBe(tabsBefore)
|
||||
})
|
||||
|
||||
test('confirming the X-button prompt closes the busy tab', async ({ orcaPage }) => {
|
||||
test.setTimeout(120_000)
|
||||
const busyTabId = await startBusyTerminal(orcaPage)
|
||||
const busyTab = orcaPage.locator(`${SORTABLE_TAB}[data-tab-id="${busyTabId}"]`).first()
|
||||
|
||||
await busyTab.hover()
|
||||
await busyTab.getByRole('button', { name: /^Close tab /i }).click()
|
||||
await expect(closeDialogTitle(orcaPage)).toBeVisible({ timeout: 15_000 })
|
||||
await orcaPage.getByRole('button', { name: /^Stop and Close$/ }).click()
|
||||
|
||||
await expect(busyTab).toHaveCount(0, { timeout: 15_000 })
|
||||
await expect(closeDialogTitle(orcaPage)).toBeHidden()
|
||||
})
|
||||
|
||||
test('Cmd+W on a busy single-pane tab raises exactly one dialog', async ({ orcaPage }) => {
|
||||
test.setTimeout(120_000)
|
||||
const busyTabId = await startBusyTerminal(orcaPage)
|
||||
const busyTab = orcaPage.locator(`${SORTABLE_TAB}[data-tab-id="${busyTabId}"]`).first()
|
||||
|
||||
await focusActiveTerminalInput(orcaPage)
|
||||
await orcaPage.keyboard.press(process.platform === 'darwin' ? 'Meta+w' : 'Control+w')
|
||||
await expect(closeDialogTitle(orcaPage)).toBeVisible({ timeout: 15_000 })
|
||||
await orcaPage.getByRole('button', { name: /^Stop and Close$/ }).click()
|
||||
|
||||
await expect(busyTab).toHaveCount(0, { timeout: 15_000 })
|
||||
// Why: the pane used to probe and prompt on its own before delegating to
|
||||
// closeTerminalTab, which now prompts too — a second dialog would mean a double prompt.
|
||||
await orcaPage.waitForTimeout(1_500)
|
||||
await expect(closeDialogTitle(orcaPage)).toBeHidden()
|
||||
})
|
||||
|
|
@ -0,0 +1,90 @@
|
|||
/**
|
||||
* Regression for #10142: keyboard and mouse enforce the same running-process close
|
||||
* confirmation. Both halves run against one tab with a live `sleep 300` child:
|
||||
* 1. Cmd/Ctrl+W -> "Stop running command?" dialog (cancelled, tab survives).
|
||||
* 2. X click -> the same dialog, and the tab is still there behind it.
|
||||
*/
|
||||
import { test, expect } from './helpers/orca-app'
|
||||
import type { Page } from '@stablyai/playwright-test'
|
||||
import {
|
||||
waitForSessionReady,
|
||||
waitForActiveWorktree,
|
||||
getActiveTabId,
|
||||
ensureTerminalVisible
|
||||
} from './helpers/store'
|
||||
import {
|
||||
execInTerminal,
|
||||
focusActiveTerminalInput,
|
||||
waitForActivePanePtyId,
|
||||
waitForActiveTerminalManager,
|
||||
waitForPaneCount,
|
||||
waitForTerminalOutput
|
||||
} from './helpers/terminal'
|
||||
|
||||
const SORTABLE_TAB = '[data-testid="sortable-tab"]'
|
||||
|
||||
function countRenderedTabs(page: Page): Promise<number> {
|
||||
return page.locator(SORTABLE_TAB).count()
|
||||
}
|
||||
|
||||
function closeDialogTitle(page: Page) {
|
||||
return page.getByText(/Stop running command\?|Stop this agent\?/)
|
||||
}
|
||||
|
||||
test.describe.configure({ mode: 'serial' })
|
||||
|
||||
test('the tab X button applies the same running-process confirmation as Cmd+W', async ({
|
||||
orcaPage
|
||||
}) => {
|
||||
test.setTimeout(120_000)
|
||||
await waitForSessionReady(orcaPage)
|
||||
await waitForActiveWorktree(orcaPage)
|
||||
await ensureTerminalVisible(orcaPage)
|
||||
const hasPaneManager = await waitForActiveTerminalManager(orcaPage, 30_000)
|
||||
.then(() => true)
|
||||
.catch(() => false)
|
||||
test.skip(!hasPaneManager, 'Electron automation never mounted the live TerminalPane manager.')
|
||||
await waitForPaneCount(orcaPage, 1, 30_000)
|
||||
|
||||
const ptyId = await waitForActivePanePtyId(orcaPage)
|
||||
await execInTerminal(orcaPage, ptyId, 'echo repro-10142-ready')
|
||||
await waitForTerminalOutput(orcaPage, 'repro-10142-ready', 20_000)
|
||||
await execInTerminal(orcaPage, ptyId, 'sleep 300')
|
||||
// Only press close once `sleep` is the foreground process; otherwise the probe
|
||||
// legitimately sees an idle shell and closing is correct. `hasChildProcesses` alone is
|
||||
// not enough: macOS spawns the shell under `login`, so a still-initialising terminal
|
||||
// reports a child before `sleep 300` has run.
|
||||
await expect
|
||||
.poll(
|
||||
async () =>
|
||||
(await orcaPage.evaluate((id) => window.api.pty.inspectProcess(id), ptyId))
|
||||
.foregroundProcess,
|
||||
{ timeout: 20_000, message: 'sleep 300 never became the foreground process' }
|
||||
)
|
||||
.toBe('sleep')
|
||||
|
||||
const busyTabId = (await getActiveTabId(orcaPage))!
|
||||
const busyTab = orcaPage.locator(`${SORTABLE_TAB}[data-tab-id="${busyTabId}"]`).first()
|
||||
|
||||
// 1. Keyboard close prompts.
|
||||
await focusActiveTerminalInput(orcaPage)
|
||||
await orcaPage.keyboard.press(process.platform === 'darwin' ? 'Meta+w' : 'Control+w')
|
||||
await expect(closeDialogTitle(orcaPage)).toBeVisible({ timeout: 15_000 })
|
||||
await orcaPage.getByRole('button', { name: /^Cancel$/ }).click()
|
||||
await expect(closeDialogTitle(orcaPage)).toBeHidden()
|
||||
await expect(busyTab).toBeVisible()
|
||||
const tabsBefore = await countRenderedTabs(orcaPage)
|
||||
|
||||
// 2. Same tab, same running child, mouse close.
|
||||
await busyTab.hover()
|
||||
await busyTab.getByRole('button', { name: /^Close tab /i }).click()
|
||||
await orcaPage.waitForTimeout(1_500)
|
||||
|
||||
expect(
|
||||
{
|
||||
confirmDialogVisible: await closeDialogTitle(orcaPage).isVisible(),
|
||||
tabStillPresent: (await countRenderedTabs(orcaPage)) === tabsBefore
|
||||
},
|
||||
'X-button close must apply the same running-process confirmation as Cmd+W'
|
||||
).toEqual({ confirmDialogVisible: true, tabStillPresent: true })
|
||||
})
|
||||
Loading…
Reference in New Issue