From 1724ae8e8be23311a0c8b3c8b86bf587ee052888 Mon Sep 17 00:00:00 2001 From: Carlos Florencio Date: Sun, 31 May 2026 19:14:30 +0100 Subject: [PATCH 1/9] fix(web): destroy terminal shells on tab close with busy confirmation Closing a terminal tab now stops the PTY and removes the shell instead of parking it in the background. A confirmation dialog appears when a command looks like it is still running. Co-authored-by: Cursor --- .../task/close-terminal-confirm-dialog.tsx | 55 ++++++ .../components/task/dockview-layout-setup.ts | 22 +-- .../task/mobile/mobile-terminals-section.tsx | 137 +++++--------- .../components/task/passthrough-terminal.tsx | 2 + apps/web/components/task/task-right-panel.tsx | 170 ++++++++++++++---- apps/web/components/task/terminal-tab.tsx | 134 ++++++++++---- .../task/use-terminal-busy-tracking.ts | 29 +++ .../terminal/terminal-dockview-ui.spec.ts | 46 +---- .../hooks/domains/session/use-terminals.ts | 49 ++--- .../terminal/terminal-busy-registry.test.ts | 49 +++++ .../lib/terminal/terminal-busy-registry.ts | 41 +++++ 11 files changed, 479 insertions(+), 255 deletions(-) create mode 100644 apps/web/components/task/close-terminal-confirm-dialog.tsx create mode 100644 apps/web/components/task/use-terminal-busy-tracking.ts create mode 100644 apps/web/lib/terminal/terminal-busy-registry.test.ts create mode 100644 apps/web/lib/terminal/terminal-busy-registry.ts diff --git a/apps/web/components/task/close-terminal-confirm-dialog.tsx b/apps/web/components/task/close-terminal-confirm-dialog.tsx new file mode 100644 index 000000000..1b5caf2a5 --- /dev/null +++ b/apps/web/components/task/close-terminal-confirm-dialog.tsx @@ -0,0 +1,55 @@ +"use client"; + +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from "@kandev/ui/alert-dialog"; + +/** + * Shared "Close terminal?" confirmation. Rendered before a destroy-on-close + * when the terminal looks busy (a command is running) or is a script terminal. + * Used by the dockview tab, the right-panel strip, and the mobile picker so + * all three close paths warn consistently. + */ +export function CloseTerminalConfirmDialog({ + open, + terminalName, + onOpenChange, + onConfirm, +}: { + open: boolean; + terminalName: string; + onOpenChange: (open: boolean) => void; + onConfirm: () => void; +}) { + return ( + + + + Close terminal? + + {`This stops the “${terminalName}” shell and any command it's running.`} + + + + Cancel + { + onOpenChange(false); + onConfirm(); + }} + className="cursor-pointer bg-destructive text-destructive-foreground hover:bg-destructive/90" + > + Close terminal + + + + + ); +} diff --git a/apps/web/components/task/dockview-layout-setup.ts b/apps/web/components/task/dockview-layout-setup.ts index bc8fc87a3..35377e3a3 100644 --- a/apps/web/components/task/dockview-layout-setup.ts +++ b/apps/web/components/task/dockview-layout-setup.ts @@ -14,7 +14,7 @@ import { import { setEnvLayout, setGlobalSidebarWidth } from "@/lib/local-storage"; import { panelPortalManager } from "@/lib/layout/panel-portal-manager"; import { stopVscode } from "@/lib/api/domains/vscode-api"; -import { parkUserShell, stopUserShell } from "@/lib/api/domains/user-shell-api"; +import { stopUserShell } from "@/lib/api/domains/user-shell-api"; import { createDebugLogger, IS_DEBUG } from "@/lib/debug/log"; import { snapshotColumnWidths, formatWidthsSnapshot } from "@/lib/state/dockview-widths-debug"; import { enforcePinnedTargets, setSashDragging } from "@/lib/state/dockview-pinned-enforce"; @@ -347,8 +347,8 @@ function resolveSessionForEntry( return match?.[0] ?? active; } -/** Tab close → ordinary terminals park (PTY + DB row survive, reappear in - * the "+" menu); scripts/bottom-panel/legacy passthrough still destroy. */ +/** Tab close → destroy the shell (PTY stopped, DB row removed). When the tab + * component already destroyed the shell it marks the panel id so we skip. */ function handleTerminalPanelClosed( appStore: StoreApi, panelId: string, @@ -364,19 +364,9 @@ function handleTerminalPanelClosed( const fallbackEnv = active ? (state.environmentIdBySessionId[active] ?? null) : null; const envForTerminal = stampedEnv || fallbackEnv; if (!envForTerminal) return; - const shell = state.userShells.byEnvironmentId[envForTerminal]?.find( - (s) => s.terminalId === terminalId, - ); - if (shell?.kind === "ordinary") { - parkUserShell(terminalId, stampedTaskID).then( - () => state.updateUserShell(envForTerminal, terminalId, { state: "parked" }), - (err: unknown) => console.error("park terminal on tab close:", err), - ); - } else { - stopUserShell(envForTerminal, terminalId, stampedTaskID).catch((err: unknown) => - console.warn("stop terminal on tab close:", err), - ); - } + stopUserShell(envForTerminal, terminalId, stampedTaskID) + .then(() => state.removeUserShell(envForTerminal, terminalId)) + .catch((err: unknown) => console.warn("stop terminal on tab close:", err)); } export function setupPortalCleanup( diff --git a/apps/web/components/task/mobile/mobile-terminals-section.tsx b/apps/web/components/task/mobile/mobile-terminals-section.tsx index 986e37e87..159351674 100644 --- a/apps/web/components/task/mobile/mobile-terminals-section.tsx +++ b/apps/web/components/task/mobile/mobile-terminals-section.tsx @@ -3,20 +3,12 @@ import { memo, useCallback, useState } from "react"; import { IconPlus, IconTerminal2, IconX } from "@tabler/icons-react"; import { Button } from "@kandev/ui/button"; -import { - AlertDialog, - AlertDialogAction, - AlertDialogCancel, - AlertDialogContent, - AlertDialogDescription, - AlertDialogFooter, - AlertDialogHeader, - AlertDialogTitle, -} from "@kandev/ui/alert-dialog"; import { useAppStore } from "@/components/state-provider"; import { stopUserShell } from "@/lib/api/domains/user-shell-api"; +import { isTerminalBusy } from "@/lib/terminal/terminal-busy-registry"; import { useUserShells } from "@/hooks/domains/session/use-user-shells"; import { releaseAutoCreatedEnvironment } from "@/hooks/domains/session/use-mobile-terminals"; +import { CloseTerminalConfirmDialog } from "../close-terminal-confirm-dialog"; import { MobilePillButton } from "./mobile-pill-button"; import { MobilePickerSheet } from "./mobile-picker-sheet"; import { useMobileTerminalsContext } from "./mobile-terminals-context"; @@ -76,41 +68,6 @@ function TerminalRow({ ); } -function CloseTerminalConfirmDialog({ - terminal, - onOpenChange, - onConfirm, -}: { - terminal: Terminal | null; - onOpenChange: (open: boolean) => void; - onConfirm: () => void; -}) { - return ( - - - - Close terminal? - - {`This stops the “${terminal?.label ?? ""}” shell and any process it's running.`} - - - - Cancel - { - onOpenChange(false); - onConfirm(); - }} - className="cursor-pointer bg-destructive text-destructive-foreground hover:bg-destructive/90" - > - Close terminal - - - - - ); -} - type CloseHandlerArgs = { sessionId: string | null; environmentId: string | null; @@ -130,42 +87,33 @@ function useTerminalCloseHandler({ }: CloseHandlerArgs) { const [pendingClose, setPendingClose] = useState(null); - const handleConfirmClose = useCallback(async () => { - const t = pendingClose; - if (!t || !sessionId) return; - try { - // Stop the remote shell first; only mutate UI on success so a failed - // stop doesn't orphan the shell server-side while the picker hides it. - if (environmentId) await stopUserShell(environmentId, t.id); - // If the closed terminal was active, advance the active tab to a - // remaining one so the picker doesn't leave terminalTabValue pointing - // at a deleted id (which would render every row as inactive). - if (terminalTabValue === t.id) { - const next = terminals.find((row) => row.id !== t.id); - if (next) setRightPanelActiveTab(sessionId, next.id); - } - removeTerminal(t.id); - // If this was the last terminal, release the auto-create guard so the - // pane recreates a default shell on next render instead of getting - // stuck on the "Starting terminal…" placeholder forever. - if (environmentId && terminals.length <= 1) { - releaseAutoCreatedEnvironment(environmentId); + const closeTerminal = useCallback( + async (t: Terminal) => { + if (!sessionId) return; + try { + if (environmentId) await stopUserShell(environmentId, t.id); + if (terminalTabValue === t.id) { + const next = terminals.find((row) => row.id !== t.id); + if (next) setRightPanelActiveTab(sessionId, next.id); + } + removeTerminal(t.id); + if (environmentId && terminals.length <= 1) { + releaseAutoCreatedEnvironment(environmentId); + } + setPendingClose(null); + } catch (err) { + console.error("Failed to stop terminal:", err); } - setPendingClose(null); - } catch (err) { - console.error("Failed to stop terminal:", err); - } - }, [ - pendingClose, - sessionId, - environmentId, - terminals, - terminalTabValue, - removeTerminal, - setRightPanelActiveTab, - ]); + }, + [sessionId, environmentId, terminals, terminalTabValue, removeTerminal, setRightPanelActiveTab], + ); - return { pendingClose, setPendingClose, handleConfirmClose }; + const handleConfirmClose = useCallback(async () => { + if (!pendingClose) return; + await closeTerminal(pendingClose); + }, [pendingClose, closeTerminal]); + + return { pendingClose, setPendingClose, handleConfirmClose, closeTerminal }; } const MobileTerminalsList = memo(function MobileTerminalsList({ @@ -179,14 +127,15 @@ const MobileTerminalsList = memo(function MobileTerminalsList({ useMobileTerminalsContext(); const { shells } = useUserShells(environmentId); const setRightPanelActiveTab = useAppStore((s) => s.setRightPanelActiveTab); - const { pendingClose, setPendingClose, handleConfirmClose } = useTerminalCloseHandler({ - sessionId, - environmentId, - terminals, - terminalTabValue, - removeTerminal, - setRightPanelActiveTab, - }); + const { pendingClose, setPendingClose, handleConfirmClose, closeTerminal } = + useTerminalCloseHandler({ + sessionId, + environmentId, + terminals, + terminalTabValue, + removeTerminal, + setRightPanelActiveTab, + }); const isShellRunning = useCallback( (id: string) => shells.find((s) => s.terminalId === id)?.running ?? false, @@ -202,8 +151,15 @@ const MobileTerminalsList = memo(function MobileTerminalsList({ ); const handleAskClose = useCallback( - (terminal: Terminal) => setPendingClose(terminal), - [setPendingClose], + (terminal: Terminal) => { + const needsConfirm = isTerminalBusy(terminal.id) || terminal.type === "script"; + if (needsConfirm) { + setPendingClose(terminal); + return; + } + void closeTerminal(terminal); + }, + [closeTerminal, setPendingClose], ); if (!sessionId) { @@ -247,7 +203,8 @@ const MobileTerminalsList = memo(function MobileTerminalsList({ ))} { if (!open) setPendingClose(null); }} diff --git a/apps/web/components/task/passthrough-terminal.tsx b/apps/web/components/task/passthrough-terminal.tsx index 2ee57586f..d4029eca0 100644 --- a/apps/web/components/task/passthrough-terminal.tsx +++ b/apps/web/components/task/passthrough-terminal.tsx @@ -27,6 +27,7 @@ import { useEnvironmentSessionId } from "@/hooks/use-environment-session-id"; import { useTerminalSearch } from "./use-terminal-search"; import { TerminalSearchBar } from "./terminal-search-bar"; import { usePanelSearch } from "@/hooks/use-panel-search"; +import { useTerminalBusyTracking } from "./use-terminal-busy-tracking"; type BaseProps = { autoFocus?: boolean; @@ -232,6 +233,7 @@ export function PassthroughTerminal(props: PassthroughTerminalProps) { keyboardShortcutsRef, onFindInPanelRef, }); + useTerminalBusyTracking(terminalId, xtermRef, mode === "shell" && isTerminalReady); useTouchScroll({ terminalRef: refs.terminalRef, diff --git a/apps/web/components/task/task-right-panel.tsx b/apps/web/components/task/task-right-panel.tsx index 35dbc472a..9b03e9175 100644 --- a/apps/web/components/task/task-right-panel.tsx +++ b/apps/web/components/task/task-right-panel.tsx @@ -21,7 +21,9 @@ import { useDefaultLayout } from "@/lib/layout/use-default-layout"; import { SessionTabs, type SessionTab } from "@/components/session-tabs"; import { useRepositoryScripts } from "@/hooks/domains/workspace/use-repository-scripts"; import { useTerminals } from "@/hooks/domains/session/use-terminals"; +import { isTerminalBusy } from "@/lib/terminal/terminal-busy-registry"; import { ParkedTerminalsMenu } from "@/components/task/parked-terminals-menu"; +import { CloseTerminalConfirmDialog } from "@/components/task/close-terminal-confirm-dialog"; import type { RepositoryScript } from "@/lib/types/http"; import type { Terminal } from "@/hooks/domains/session/use-terminals"; @@ -184,6 +186,47 @@ function useRightPanelTabs({ return { tabs, handleTabChange }; } +/** + * Gate the X-button close behind a confirm when the terminal looks busy or is + * a script terminal — mirrors the dockview tab and mobile picker so every + * close path warns before it destroys a running shell. Idle shells still close + * immediately via the underlying `handleCloseTab`. + */ +function useConfirmableTerminalClose({ + terminals, + handleCloseTab, + destroyTerminal, +}: { + terminals: Terminal[]; + handleCloseTab: (event: MouseEvent, terminalId: string) => void; + destroyTerminal: (id: string) => Promise | void; +}) { + const [pendingClose, setPendingClose] = useState(null); + + const handleAskCloseTab = useCallback( + (event: MouseEvent, terminalId: string) => { + const terminal = terminals.find((t) => t.id === terminalId); + const needsConfirm = !!terminal && (terminal.type === "script" || isTerminalBusy(terminalId)); + if (needsConfirm) { + event.preventDefault(); + event.stopPropagation(); + setPendingClose(terminal); + return; + } + handleCloseTab(event, terminalId); + }, + [terminals, handleCloseTab], + ); + + const handleConfirmClose = useCallback(() => { + if (!pendingClose) return; + void destroyTerminal(pendingClose.id); + setPendingClose(null); + }, [pendingClose, destroyTerminal]); + + return { pendingClose, setPendingClose, handleAskCloseTab, handleConfirmClose }; +} + type CollapsedRightPanelProps = { topPanel: ReactNode; tabs: SessionTab[]; @@ -331,18 +374,17 @@ function RightPanelContent({ ); } -const TaskRightPanel = memo(function TaskRightPanel({ - topPanel, - sessionId = null, - repositoryId = null, +function useTaskRightPanel({ + sessionId, + repositoryId, initialScripts = [], initialTerminals, -}: TaskRightPanelProps) { - const rightPanelIds = ["top", "bottom"]; +}: Required> & + Pick) { const rightLayoutKey = "task-layout-right-v2"; const { defaultLayout: rightLayout, onLayoutChanged: onRightLayoutChange } = useDefaultLayout({ id: rightLayoutKey, - panelIds: rightPanelIds, + panelIds: ["top", "bottom"], baseLayout: DEFAULT_RIGHT_LAYOUT, }); @@ -357,22 +399,15 @@ const TaskRightPanel = memo(function TaskRightPanel({ const closeLayoutPreview = useLayoutStore((state) => state.closePreview); // Use the terminals hook — env-keyed for shell ops, session-keyed for tab UX + const terminalsApi = useTerminals({ sessionId, environmentId, initialTerminals }); const { terminals, - parkedTerminals, activeTab, - terminalTabValue, - addTerminal, handleCloseDevTab: baseHandleCloseDevTab, handleCloseTab, - handleRunCommand, renameTerminal, - resumeTerminal, destroyTerminal, - isStoppingDev, - devProcessId, - devOutput, - } = useTerminals({ sessionId, environmentId, initialTerminals }); + } = terminalsApi; // Wrap handleCloseDevTab to also close the layout preview const handleCloseDevTab = useCallback( @@ -393,39 +428,96 @@ const TaskRightPanel = memo(function TaskRightPanel({ isBottomCollapsed, setRightPanelActiveTab, }); + const closeConfirm = useConfirmableTerminalClose({ terminals, handleCloseTab, destroyTerminal }); const { tabs, handleTabChange } = useRightPanelTabs({ hasScripts, terminals, handleCloseDevTab, - handleCloseTab, + handleCloseTab: closeConfirm.handleAskCloseTab, renameTerminal, destroyTerminal, sessionId, setRightPanelActiveTab, }); + + return { + ...terminalsApi, + rightLayoutKey, + rightLayout, + onRightLayoutChange, + isBottomCollapsed, + setIsBottomCollapsed, + environmentId, + scripts, + tabs, + handleTabChange, + closeConfirm, + }; +} + +const TaskRightPanel = memo(function TaskRightPanel({ + topPanel, + sessionId = null, + repositoryId = null, + initialScripts = [], + initialTerminals, +}: TaskRightPanelProps) { + const { + terminals, + parkedTerminals, + terminalTabValue, + addTerminal, + handleRunCommand, + resumeTerminal, + destroyTerminal, + isStoppingDev, + devProcessId, + devOutput, + rightLayoutKey, + rightLayout, + onRightLayoutChange, + isBottomCollapsed, + setIsBottomCollapsed, + environmentId, + scripts, + tabs, + handleTabChange, + closeConfirm, + } = useTaskRightPanel({ sessionId, repositoryId, initialScripts, initialTerminals }); + const { pendingClose, setPendingClose, handleConfirmClose } = closeConfirm; return ( - + <> + + { + if (!open) setPendingClose(null); + }} + onConfirm={handleConfirmClose} + /> + ); }); diff --git a/apps/web/components/task/terminal-tab.tsx b/apps/web/components/task/terminal-tab.tsx index 3988882fc..5b2dcbb98 100644 --- a/apps/web/components/task/terminal-tab.tsx +++ b/apps/web/components/task/terminal-tab.tsx @@ -11,6 +11,8 @@ import { } from "@kandev/ui/context-menu"; import { useAppStore } from "@/components/state-provider"; import { destroyUserShell, renameUserShell } from "@/lib/api/domains/user-shell-api"; +import { isTerminalBusy } from "@/lib/terminal/terminal-busy-registry"; +import { CloseTerminalConfirmDialog } from "./close-terminal-confirm-dialog"; import { markTerminalPanelTerminateClose } from "./dockview-layout-setup"; /** @@ -72,14 +74,21 @@ function useTerminalTabState(stampedEnv: string | undefined, terminalId: string, const seq = shell?.seq; const showBadge = isOrdinary && ordinaryCount > 1 && typeof seq === "number"; const displayName = pickDisplayName(shell, apiTitle); - return { shell, isOrdinary, seq, showBadge, displayName }; + const closable = shell?.closable ?? true; + return { shell, isOrdinary, seq, showBadge, displayName, closable }; +} + +function shouldConfirmTerminalClose(terminalId: string, shell: { initialCommand?: string } | null) { + if (shell?.initialCommand) return true; + return isTerminalBusy(terminalId); } export function TerminalTab(props: IDockviewPanelHeaderProps) { const { terminalId, taskID: stampedTaskID, environmentId: stampedEnv } = extractParams(props); const activeTaskID = useAppStore((s) => s.tasks?.activeTaskId ?? null); const taskID = stampedTaskID ?? activeTaskID ?? null; - const { shell, isOrdinary, seq, showBadge, displayName } = useTerminalTabState( + const removeUserShellStore = useAppStore((s) => s.removeUserShell); + const { shell, isOrdinary, seq, showBadge, displayName, closable } = useTerminalTabState( stampedEnv, terminalId, props.api.title ?? "Terminal", @@ -93,6 +102,7 @@ export function TerminalTab(props: IDockviewPanelHeaderProps) { }, [props.api, displayName]); const [isRenaming, setIsRenaming] = useState(false); + const [confirmClose, setConfirmClose] = useState(false); const handleCommitRename = useRenameCommitter({ isOrdinary, stampedEnv, @@ -102,40 +112,80 @@ export function TerminalTab(props: IDockviewPanelHeaderProps) { onDone: () => setIsRenaming(false), }); + const destroyAndClosePanel = useCallback(async () => { + if (!stampedEnv) { + props.api.close(); + return; + } + try { + await destroyUserShell(stampedEnv, terminalId, taskID ?? undefined); + removeUserShellStore(stampedEnv, terminalId); + markTerminalPanelTerminateClose(props.api.id); + props.api.close(); + } catch (error) { + console.error("close terminal:", error); + } + }, [stampedEnv, terminalId, taskID, removeUserShellStore, props.api]); + + const handleCloseTab = useCallback(() => { + if (!closable) return; + if (shouldConfirmTerminalClose(terminalId, shell)) { + setConfirmClose(true); + return; + } + void destroyAndClosePanel(); + }, [closable, terminalId, shell, destroyAndClosePanel]); + const renameInitial = shell?.customName && shell.customName !== "" ? shell.customName : displayName; const seqBadgeForInput = showBadge && typeof seq === "number" ? seq : null; return ( - - - {isRenaming ? ( - setIsRenaming(false)} - /> - ) : ( - - )} - - setIsRenaming(true)} - onClosePanel={() => props.api.close()} - onTerminatePanel={() => { - markTerminalPanelTerminateClose(props.api.id); - props.api.close(); - }} + <> + + + {isRenaming ? ( + setIsRenaming(false)} + /> + ) : ( + + )} + + setIsRenaming(true)} + onClosePanel={() => props.api.close()} + onTerminatePanel={() => { + markTerminalPanelTerminateClose(props.api.id); + props.api.close(); + }} + /> + + void destroyAndClosePanel()} /> - + ); } @@ -175,6 +225,9 @@ function useRenameCommitter({ function TerminalTabBody({ showBadge, seq, + closable, + terminalId, + onCloseTab, // `displayName` is computed in the parent but consumed via the // api.setTitle effect — drop it here so it doesn't leak into the // DOM via the {...props} spread below (React warning otherwise). @@ -184,10 +237,23 @@ function TerminalTabBody({ showBadge: boolean; seq: number | undefined; displayName: string; + closable: boolean; + onCloseTab: () => void; + terminalId: string; }) { void _displayName; + const tabContentRef = useRef(null); + + useEffect(() => { + if (!closable) return; + const closeAction = tabContentRef.current?.querySelector(".dv-default-tab-action"); + if (!closeAction) return; + closeAction.setAttribute("data-testid", `terminal-tab-close-${terminalId}`); + return () => closeAction.removeAttribute("data-testid"); + }, [closable, terminalId, props.api.id]); + return ( -
+
{showBadge && ( )} - +
); } diff --git a/apps/web/components/task/use-terminal-busy-tracking.ts b/apps/web/components/task/use-terminal-busy-tracking.ts new file mode 100644 index 000000000..176e18af9 --- /dev/null +++ b/apps/web/components/task/use-terminal-busy-tracking.ts @@ -0,0 +1,29 @@ +import { useEffect } from "react"; +import type { Terminal } from "@xterm/xterm"; +import { + clearTerminalBusy, + markTerminalInput, + markTerminalOutput, +} from "@/lib/terminal/terminal-busy-registry"; + +/** Wire xterm input/output hooks so close handlers can detect running commands. */ +export function useTerminalBusyTracking( + terminalId: string | undefined, + xtermRef: React.MutableRefObject, + enabled: boolean, +): void { + useEffect(() => { + if (!enabled || !terminalId) return; + const terminal = xtermRef.current; + if (!terminal) return; + + const inputSub = terminal.onData((data) => markTerminalInput(terminalId, data)); + const outputSub = terminal.onWriteParsed(() => markTerminalOutput(terminalId, terminal)); + + return () => { + inputSub.dispose(); + outputSub.dispose(); + clearTerminalBusy(terminalId); + }; + }, [enabled, terminalId, xtermRef]); +} diff --git a/apps/web/e2e/tests/terminal/terminal-dockview-ui.spec.ts b/apps/web/e2e/tests/terminal/terminal-dockview-ui.spec.ts index f738aaa2c..fe20468f9 100644 --- a/apps/web/e2e/tests/terminal/terminal-dockview-ui.spec.ts +++ b/apps/web/e2e/tests/terminal/terminal-dockview-ui.spec.ts @@ -112,16 +112,10 @@ test.describe("Terminals — dockview UI", () => { /** * Regression: closing a terminal tab via dockview's X button must - * PARK the terminal (preserving the PTY + DB row), not destroy it. - * After reload, the closed terminal must surface in the "+" → Terminals - * menu with a `parked` pill AND its original seq, so the user can - * resume it. - * - * Before the fix this fails because dockview-layout-setup.onDidRemovePanel - * calls stopUserShell (alias for destroyUserShell) — the DB row is - * gone, no "parked" row exists to find. + * destroy the shell (PTY stopped, DB row removed), not park it. + * After reload the closed terminal must NOT surface in the "+" menu. */ - test("closing a terminal parks it and the row shows up after reload with parked pill", async ({ + test("closing a terminal destroys it and the row does not return after reload", async ({ testPage, apiClient, seedData, @@ -132,15 +126,10 @@ test.describe("Terminals — dockview UI", () => { await session.clickTab("Terminal"); await session.expectTerminalConnected(); - // Add a second terminal so we have a non-default one to close. await clickNewTerminalInPlusMenu(testPage, session); await expect(testPage.getByTestId("terminal-tab-seq-1")).toBeVisible({ timeout: 10_000 }); await expect(testPage.getByTestId("terminal-tab-seq-2")).toBeVisible({ timeout: 5_000 }); - // Close the seq=2 tab specifically. `.last()` is unreliable when - // terminals span multiple groups (default lives in the right-bottom, - // user-created lands in the center). Walk from the seq=2 badge up - // to its wrapping div and grab that subtree's close button. const seq2Close = testPage .getByTestId("terminal-tab-seq-2") .locator("..") @@ -148,18 +137,12 @@ test.describe("Terminals — dockview UI", () => { await seq2Close.click(); await expect(testPage.getByTestId("terminal-tab-seq-2")).toHaveCount(0, { timeout: 5_000 }); - // Layout-save is 300ms-debounced. Wait past the debounce so the - // saved JSON reflects the closed/parked terminal — otherwise the - // saved layout still has it and the reload re-opens it. await testPage.waitForTimeout(800); - // Reload — only the open default terminal should restore as a - // panel; the parked one lives in the reopen menu. await testPage.reload(); await session.waitForLoad(); await session.clickTab("Terminal"); - // The open default terminal should still render as a visible tab. const terminalContent = testPage.locator(".dv-default-tab-content").filter({ hasText: /^Terminal$/, }); @@ -170,28 +153,16 @@ test.describe("Terminals — dockview UI", () => { }) .toBeGreaterThanOrEqual(1); - // Open "+" menu — the parked second terminal must appear under - // Terminals with its seq=2 marker AND the `parked` pill. await session.addPanelButton().click(); const terminalSection = testPage .locator('[role="menu"]') .getByText("Terminals", { exact: true }); await expect(terminalSection).toBeVisible({ timeout: 10_000 }); - // The seq=2 row must be present — proves the close was a park, not - // a destroy. The row no longer carries a distinct "parked" pill; - // its mere presence after a destroy-on-close handler would have - // wiped the DB row is the regression guard. const reopenRowsWithSeq2 = testPage .locator('[data-testid^="reopen-terminal-"]') .filter({ has: testPage.getByTestId("reopen-terminal-seq-2") }); - await expect - .poll(() => reopenRowsWithSeq2.count(), { - timeout: 10_000, - message: - "parked terminal #2 should appear in reopen menu — if missing, close-handler destroyed instead of parked", - }) - .toBe(1); + await expect(reopenRowsWithSeq2).toHaveCount(0, { timeout: 5_000 }); }); /** @@ -261,14 +232,7 @@ test.describe("Terminals — dockview UI", () => { await clickNewTerminalInPlusMenu(testPage, session); await expect(testPage.getByTestId("terminal-tab-seq-2")).toBeVisible({ timeout: 10_000 }); - // Park the seq=2 tab by closing it (last in tab order). - const terminalCloseButtons = testPage - .locator(".dv-default-tab") - .filter({ hasText: /^Terminal/ }) - .locator(".dv-default-tab-action"); - await terminalCloseButtons.last().click(); - - // Open the "+" menu and confirm-destroy the seq=2 row. + // Destroy the still-open seq=2 row from the "+" menu without closing its tab first. await session.addPanelButton().click(); const row = testPage .locator('[data-testid^="reopen-terminal-"]') diff --git a/apps/web/hooks/domains/session/use-terminals.ts b/apps/web/hooks/domains/session/use-terminals.ts index 9d7cc5a54..d9add1506 100644 --- a/apps/web/hooks/domains/session/use-terminals.ts +++ b/apps/web/hooks/domains/session/use-terminals.ts @@ -6,7 +6,6 @@ import { useAppStore, useAppStoreApi } from "@/components/state-provider"; import { stopProcess } from "@/lib/api"; import { destroyUserShell, - parkUserShell, resumeUserShell, renameUserShell, createUserShell, @@ -273,58 +272,36 @@ function useCloseDevTab({ type CloseTabOpts = { environmentId: string | null; taskID: string | null; - terminals: Terminal[]; removeTerminal: (id: string) => void; removeUserShellStore: (environmentId: string, terminalId: string) => void; - updateUserShell: ( - environmentId: string, - terminalId: string, - patch: { state?: "open" | "parked" }, - ) => void; }; /** - * X-button close. For ordinary terminals this PARKS the tab (PTY keeps - * running, user can resume from the "Parked terminals" submenu). For - * scripts and any non-ordinary terminal it falls back to destroy. - * - * The local tab is removed only AFTER the backend call resolves — that - * way a transient failure (network, backend 500) leaves the tab on the - * strip rather than disappearing into thin air. The next `user_shell.list` - * poll then reflects whatever state the backend actually settled on. + * X-button close. Destroys the shell (PTY stopped, DB row removed). The local + * tab is removed only AFTER the backend call resolves — that way a transient + * failure (network, backend 500) leaves the tab on the strip rather than + * disappearing into thin air. The next `user_shell.list` poll then reflects + * whatever state the backend actually settled on. */ function useCloseTab({ environmentId, taskID, - terminals, removeTerminal, removeUserShellStore, - updateUserShell, }: CloseTabOpts) { return useCallback( (event: MouseEvent, terminalId: string) => { event.preventDefault(); event.stopPropagation(); if (!environmentId) return; - const term = terminals.find((t) => t.id === terminalId); - const isOrdinaryTab = term?.kind === "ordinary"; - if (isOrdinaryTab) { - parkUserShell(terminalId, taskID ?? undefined) - .then(() => { - updateUserShell(environmentId, terminalId, { state: "parked" }); - removeTerminal(terminalId); - }) - .catch((error) => console.error("Failed to park terminal:", error)); - } else { - destroyUserShell(environmentId, terminalId, taskID ?? undefined) - .then(() => { - removeUserShellStore(environmentId, terminalId); - removeTerminal(terminalId); - }) - .catch((error) => console.error("Failed to destroy terminal:", error)); - } + destroyUserShell(environmentId, terminalId, taskID ?? undefined) + .then(() => { + removeUserShellStore(environmentId, terminalId); + removeTerminal(terminalId); + }) + .catch((error) => console.error("Failed to destroy terminal:", error)); }, - [environmentId, taskID, terminals, removeTerminal, removeUserShellStore, updateUserShell], + [environmentId, taskID, removeTerminal, removeUserShellStore], ); } @@ -478,10 +455,8 @@ function useTerminalActions({ const handleCloseTab = useCloseTab({ environmentId, taskID, - terminals, removeTerminal, removeUserShellStore, - updateUserShell, }); const { renameTerminal, resumeTerminal, destroyTerminal } = useManagedTerminalActions({ diff --git a/apps/web/lib/terminal/terminal-busy-registry.test.ts b/apps/web/lib/terminal/terminal-busy-registry.test.ts new file mode 100644 index 000000000..486a7496c --- /dev/null +++ b/apps/web/lib/terminal/terminal-busy-registry.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it, beforeEach } from "vitest"; +import type { Terminal } from "@xterm/xterm"; +import { + clearTerminalBusy, + isTerminalBusy, + markTerminalInput, + markTerminalOutput, +} from "./terminal-busy-registry"; + +function mockTerminal(lineText: string): Terminal { + return { + buffer: { + active: { + baseY: 0, + cursorY: 0, + getLine: () => ({ + translateToString: () => lineText, + }), + }, + }, + } as unknown as Terminal; +} + +describe("terminal-busy-registry", () => { + beforeEach(() => { + clearTerminalBusy("term-1"); + }); + + it("starts idle", () => { + expect(isTerminalBusy("term-1")).toBe(false); + }); + + it("marks busy after Enter", () => { + markTerminalInput("term-1", "sleep 10\r"); + expect(isTerminalBusy("term-1")).toBe(true); + }); + + it("clears busy when the buffer returns to a shell prompt", () => { + markTerminalInput("term-1", "echo hi\r"); + markTerminalOutput("term-1", mockTerminal("user@host:~/proj$ ")); + expect(isTerminalBusy("term-1")).toBe(false); + }); + + it("stays busy while output has no prompt tail", () => { + markTerminalInput("term-1", "make dev\r"); + markTerminalOutput("term-1", mockTerminal("DEBUG logger/logger.go:42")); + expect(isTerminalBusy("term-1")).toBe(true); + }); +}); diff --git a/apps/web/lib/terminal/terminal-busy-registry.ts b/apps/web/lib/terminal/terminal-busy-registry.ts new file mode 100644 index 000000000..da946910a --- /dev/null +++ b/apps/web/lib/terminal/terminal-busy-registry.ts @@ -0,0 +1,41 @@ +import type { Terminal } from "@xterm/xterm"; +import { stripAnsi } from "@/lib/utils/ansi"; + +/** Tracks whether a user shell likely has a foreground command running. */ +const busyByTerminalId = new Map(); + +// A line "looks like a prompt" when its last non-space glyph is a shell +// sigil ($ # > %) that is either preceded by whitespace (bare prompts like +// "$ ") or sits at the end of a path/host-ish token containing @, ~ or / +// (e.g. "user@host:~/proj$", "/usr/bin%"). Requiring the @/~/ anchor for +// attached sigils keeps progress lines like "Building... 50%" or +// "logger.go:42" from being misread as a returned prompt. +const PROMPT_TAIL = /(?:(?:^|\s)|[\w.@~:/-]*[@~/][\w.@~:/-]*)[$#>%]\s*$/; + +export function markTerminalInput(terminalId: string, data: string): void { + if (data.includes("\r") || data.includes("\n")) { + busyByTerminalId.set(terminalId, true); + } +} + +export function markTerminalOutput(terminalId: string, terminal: Terminal): void { + if (bufferLooksAtPrompt(terminal)) { + busyByTerminalId.set(terminalId, false); + } +} + +export function isTerminalBusy(terminalId: string): boolean { + return busyByTerminalId.get(terminalId) ?? false; +} + +export function clearTerminalBusy(terminalId: string): void { + busyByTerminalId.delete(terminalId); +} + +function bufferLooksAtPrompt(terminal: Terminal): boolean { + const buf = terminal.buffer.active; + const line = buf.getLine(buf.baseY + buf.cursorY)?.translateToString(true) ?? ""; + const trimmed = stripAnsi(line).trimEnd(); + if (trimmed === "") return true; + return PROMPT_TAIL.test(trimmed); +} From e1a08ac7ebf013b4e890a535de2a06d1240d5b2c Mon Sep 17 00:00:00 2001 From: Carlos Florencio Date: Sun, 31 May 2026 19:24:29 +0100 Subject: [PATCH 2/9] fix(web): address PR review feedback for terminal close Thread taskId through mobile destroy, unify script/busy confirm gating, fix blank-line busy false-positive, and harden busy-tracking attach. Co-authored-by: Cursor --- .../task/mobile/mobile-terminals-section.tsx | 25 +++++++++++--- .../components/task/passthrough-terminal.tsx | 2 +- apps/web/components/task/task-right-panel.tsx | 8 +++-- apps/web/components/task/terminal-tab.tsx | 9 ++--- .../task/use-terminal-busy-tracking.ts | 34 ++++++++++++++----- .../terminal/terminal-busy-registry.test.ts | 21 ++++++++++-- .../lib/terminal/terminal-busy-registry.ts | 32 +++++++++++++++-- 7 files changed, 103 insertions(+), 28 deletions(-) diff --git a/apps/web/components/task/mobile/mobile-terminals-section.tsx b/apps/web/components/task/mobile/mobile-terminals-section.tsx index 159351674..ec27551e0 100644 --- a/apps/web/components/task/mobile/mobile-terminals-section.tsx +++ b/apps/web/components/task/mobile/mobile-terminals-section.tsx @@ -5,7 +5,7 @@ import { IconPlus, IconTerminal2, IconX } from "@tabler/icons-react"; import { Button } from "@kandev/ui/button"; import { useAppStore } from "@/components/state-provider"; import { stopUserShell } from "@/lib/api/domains/user-shell-api"; -import { isTerminalBusy } from "@/lib/terminal/terminal-busy-registry"; +import { shouldConfirmTerminalClose } from "@/lib/terminal/terminal-busy-registry"; import { useUserShells } from "@/hooks/domains/session/use-user-shells"; import { releaseAutoCreatedEnvironment } from "@/hooks/domains/session/use-mobile-terminals"; import { CloseTerminalConfirmDialog } from "../close-terminal-confirm-dialog"; @@ -71,6 +71,7 @@ function TerminalRow({ type CloseHandlerArgs = { sessionId: string | null; environmentId: string | null; + taskId: string | null; terminals: Terminal[]; terminalTabValue: string; removeTerminal: (id: string) => void; @@ -80,6 +81,7 @@ type CloseHandlerArgs = { function useTerminalCloseHandler({ sessionId, environmentId, + taskId, terminals, terminalTabValue, removeTerminal, @@ -91,7 +93,7 @@ function useTerminalCloseHandler({ async (t: Terminal) => { if (!sessionId) return; try { - if (environmentId) await stopUserShell(environmentId, t.id); + if (environmentId) await stopUserShell(environmentId, t.id, taskId ?? undefined); if (terminalTabValue === t.id) { const next = terminals.find((row) => row.id !== t.id); if (next) setRightPanelActiveTab(sessionId, next.id); @@ -105,7 +107,15 @@ function useTerminalCloseHandler({ console.error("Failed to stop terminal:", err); } }, - [sessionId, environmentId, terminals, terminalTabValue, removeTerminal, setRightPanelActiveTab], + [ + sessionId, + environmentId, + taskId, + terminals, + terminalTabValue, + removeTerminal, + setRightPanelActiveTab, + ], ); const handleConfirmClose = useCallback(async () => { @@ -127,10 +137,12 @@ const MobileTerminalsList = memo(function MobileTerminalsList({ useMobileTerminalsContext(); const { shells } = useUserShells(environmentId); const setRightPanelActiveTab = useAppStore((s) => s.setRightPanelActiveTab); + const taskId = useAppStore((s) => s.tasks?.activeTaskId ?? null); const { pendingClose, setPendingClose, handleConfirmClose, closeTerminal } = useTerminalCloseHandler({ sessionId, environmentId, + taskId, terminals, terminalTabValue, removeTerminal, @@ -152,7 +164,10 @@ const MobileTerminalsList = memo(function MobileTerminalsList({ const handleAskClose = useCallback( (terminal: Terminal) => { - const needsConfirm = isTerminalBusy(terminal.id) || terminal.type === "script"; + const needsConfirm = shouldConfirmTerminalClose(terminal.id, { + type: terminal.type, + kind: terminal.kind, + }); if (needsConfirm) { setPendingClose(terminal); return; @@ -204,7 +219,7 @@ const MobileTerminalsList = memo(function MobileTerminalsList({
{ if (!open) setPendingClose(null); }} diff --git a/apps/web/components/task/passthrough-terminal.tsx b/apps/web/components/task/passthrough-terminal.tsx index d4029eca0..5b813f327 100644 --- a/apps/web/components/task/passthrough-terminal.tsx +++ b/apps/web/components/task/passthrough-terminal.tsx @@ -233,7 +233,7 @@ export function PassthroughTerminal(props: PassthroughTerminalProps) { keyboardShortcutsRef, onFindInPanelRef, }); - useTerminalBusyTracking(terminalId, xtermRef, mode === "shell" && isTerminalReady); + useTerminalBusyTracking(terminalId, xtermRef, mode === "shell", isTerminalReady); useTouchScroll({ terminalRef: refs.terminalRef, diff --git a/apps/web/components/task/task-right-panel.tsx b/apps/web/components/task/task-right-panel.tsx index 9b03e9175..37890fd33 100644 --- a/apps/web/components/task/task-right-panel.tsx +++ b/apps/web/components/task/task-right-panel.tsx @@ -21,7 +21,7 @@ import { useDefaultLayout } from "@/lib/layout/use-default-layout"; import { SessionTabs, type SessionTab } from "@/components/session-tabs"; import { useRepositoryScripts } from "@/hooks/domains/workspace/use-repository-scripts"; import { useTerminals } from "@/hooks/domains/session/use-terminals"; -import { isTerminalBusy } from "@/lib/terminal/terminal-busy-registry"; +import { shouldConfirmTerminalClose } from "@/lib/terminal/terminal-busy-registry"; import { ParkedTerminalsMenu } from "@/components/task/parked-terminals-menu"; import { CloseTerminalConfirmDialog } from "@/components/task/close-terminal-confirm-dialog"; import type { RepositoryScript } from "@/lib/types/http"; @@ -206,7 +206,9 @@ function useConfirmableTerminalClose({ const handleAskCloseTab = useCallback( (event: MouseEvent, terminalId: string) => { const terminal = terminals.find((t) => t.id === terminalId); - const needsConfirm = !!terminal && (terminal.type === "script" || isTerminalBusy(terminalId)); + const needsConfirm = + !!terminal && + shouldConfirmTerminalClose(terminalId, { type: terminal.type, kind: terminal.kind }); if (needsConfirm) { event.preventDefault(); event.stopPropagation(); @@ -511,7 +513,7 @@ const TaskRightPanel = memo(function TaskRightPanel({ /> { if (!open) setPendingClose(null); }} diff --git a/apps/web/components/task/terminal-tab.tsx b/apps/web/components/task/terminal-tab.tsx index 5b2dcbb98..4ff716e83 100644 --- a/apps/web/components/task/terminal-tab.tsx +++ b/apps/web/components/task/terminal-tab.tsx @@ -11,7 +11,7 @@ import { } from "@kandev/ui/context-menu"; import { useAppStore } from "@/components/state-provider"; import { destroyUserShell, renameUserShell } from "@/lib/api/domains/user-shell-api"; -import { isTerminalBusy } from "@/lib/terminal/terminal-busy-registry"; +import { shouldConfirmTerminalClose } from "@/lib/terminal/terminal-busy-registry"; import { CloseTerminalConfirmDialog } from "./close-terminal-confirm-dialog"; import { markTerminalPanelTerminateClose } from "./dockview-layout-setup"; @@ -78,11 +78,6 @@ function useTerminalTabState(stampedEnv: string | undefined, terminalId: string, return { shell, isOrdinary, seq, showBadge, displayName, closable }; } -function shouldConfirmTerminalClose(terminalId: string, shell: { initialCommand?: string } | null) { - if (shell?.initialCommand) return true; - return isTerminalBusy(terminalId); -} - export function TerminalTab(props: IDockviewPanelHeaderProps) { const { terminalId, taskID: stampedTaskID, environmentId: stampedEnv } = extractParams(props); const activeTaskID = useAppStore((s) => s.tasks?.activeTaskId ?? null); @@ -129,7 +124,7 @@ export function TerminalTab(props: IDockviewPanelHeaderProps) { const handleCloseTab = useCallback(() => { if (!closable) return; - if (shouldConfirmTerminalClose(terminalId, shell)) { + if (shouldConfirmTerminalClose(terminalId, { kind: shell?.kind })) { setConfirmClose(true); return; } diff --git a/apps/web/components/task/use-terminal-busy-tracking.ts b/apps/web/components/task/use-terminal-busy-tracking.ts index 176e18af9..d92118e0f 100644 --- a/apps/web/components/task/use-terminal-busy-tracking.ts +++ b/apps/web/components/task/use-terminal-busy-tracking.ts @@ -1,5 +1,6 @@ import { useEffect } from "react"; import type { Terminal } from "@xterm/xterm"; +import type { IDisposable } from "@xterm/xterm"; import { clearTerminalBusy, markTerminalInput, @@ -11,19 +12,36 @@ export function useTerminalBusyTracking( terminalId: string | undefined, xtermRef: React.MutableRefObject, enabled: boolean, + terminalReady: boolean, ): void { useEffect(() => { - if (!enabled || !terminalId) return; - const terminal = xtermRef.current; - if (!terminal) return; + if (!enabled || !terminalId || !terminalReady) return; - const inputSub = terminal.onData((data) => markTerminalInput(terminalId, data)); - const outputSub = terminal.onWriteParsed(() => markTerminalOutput(terminalId, terminal)); + let disposed = false; + let inputSub: IDisposable | undefined; + let outputSub: IDisposable | undefined; + let raf = 0; + + const attach = () => { + if (disposed) return; + const terminal = xtermRef.current; + if (!terminal) { + raf = requestAnimationFrame(attach); + return; + } + inputSub = terminal.onData((data) => markTerminalInput(terminalId, data)); + outputSub = terminal.onWriteParsed(() => markTerminalOutput(terminalId, terminal)); + }; + + attach(); return () => { - inputSub.dispose(); - outputSub.dispose(); + disposed = true; + if (raf) cancelAnimationFrame(raf); + inputSub?.dispose(); + outputSub?.dispose(); clearTerminalBusy(terminalId); }; - }, [enabled, terminalId, xtermRef]); + // xtermRef object is stable; included to satisfy exhaustive-deps. + }, [enabled, terminalId, terminalReady, xtermRef]); } diff --git a/apps/web/lib/terminal/terminal-busy-registry.test.ts b/apps/web/lib/terminal/terminal-busy-registry.test.ts index 486a7496c..1a36ed391 100644 --- a/apps/web/lib/terminal/terminal-busy-registry.test.ts +++ b/apps/web/lib/terminal/terminal-busy-registry.test.ts @@ -1,10 +1,11 @@ import { describe, expect, it, beforeEach } from "vitest"; import type { Terminal } from "@xterm/xterm"; import { - clearTerminalBusy, isTerminalBusy, markTerminalInput, markTerminalOutput, + resetTerminalBusyRegistry, + shouldConfirmTerminalClose, } from "./terminal-busy-registry"; function mockTerminal(lineText: string): Terminal { @@ -23,7 +24,7 @@ function mockTerminal(lineText: string): Terminal { describe("terminal-busy-registry", () => { beforeEach(() => { - clearTerminalBusy("term-1"); + resetTerminalBusyRegistry(); }); it("starts idle", () => { @@ -46,4 +47,20 @@ describe("terminal-busy-registry", () => { markTerminalOutput("term-1", mockTerminal("DEBUG logger/logger.go:42")); expect(isTerminalBusy("term-1")).toBe(true); }); + + it("stays busy when cursor is on a blank line before prompt redraw", () => { + markTerminalInput("term-1", "make\r"); + markTerminalOutput("term-1", mockTerminal("")); + expect(isTerminalBusy("term-1")).toBe(true); + }); + + it("requires confirm for script terminals regardless of busy state", () => { + expect(shouldConfirmTerminalClose("term-1", { type: "script" })).toBe(true); + expect(shouldConfirmTerminalClose("term-1", { kind: "script" })).toBe(true); + }); + + it("requires confirm when busy for ordinary terminals", () => { + markTerminalInput("term-1", "npm install\r"); + expect(shouldConfirmTerminalClose("term-1", { kind: "ordinary" })).toBe(true); + }); }); diff --git a/apps/web/lib/terminal/terminal-busy-registry.ts b/apps/web/lib/terminal/terminal-busy-registry.ts index da946910a..8fd3b912f 100644 --- a/apps/web/lib/terminal/terminal-busy-registry.ts +++ b/apps/web/lib/terminal/terminal-busy-registry.ts @@ -1,7 +1,10 @@ import type { Terminal } from "@xterm/xterm"; import { stripAnsi } from "@/lib/utils/ansi"; -/** Tracks whether a user shell likely has a foreground command running. */ +/** + * Client-only busy tracking for user shells. Mutated from xterm hooks in the + * browser; guarded so accidental SSR imports never share state across requests. + */ const busyByTerminalId = new Map(); // A line "looks like a prompt" when its last non-space glyph is a shell @@ -12,13 +15,31 @@ const busyByTerminalId = new Map(); // "logger.go:42" from being misread as a returned prompt. const PROMPT_TAIL = /(?:(?:^|\s)|[\w.@~:/-]*[@~/][\w.@~:/-]*)[$#>%]\s*$/; +export type TerminalCloseConfirmOpts = { + kind?: string; + type?: string; + initialCommand?: string; +}; + +/** True when closing should warn because a script/non-ordinary shell or a busy command is active. */ +export function shouldConfirmTerminalClose( + terminalId: string, + opts: TerminalCloseConfirmOpts, +): boolean { + if (opts.type === "script" || opts.kind === "script") return true; + if (opts.kind && opts.kind !== "ordinary") return true; + return isTerminalBusy(terminalId); +} + export function markTerminalInput(terminalId: string, data: string): void { + if (typeof window === "undefined") return; if (data.includes("\r") || data.includes("\n")) { busyByTerminalId.set(terminalId, true); } } export function markTerminalOutput(terminalId: string, terminal: Terminal): void { + if (typeof window === "undefined") return; if (bufferLooksAtPrompt(terminal)) { busyByTerminalId.set(terminalId, false); } @@ -32,10 +53,17 @@ export function clearTerminalBusy(terminalId: string): void { busyByTerminalId.delete(terminalId); } +/** Test-only: flush all busy state between cases. */ +export function resetTerminalBusyRegistry(): void { + busyByTerminalId.clear(); +} + function bufferLooksAtPrompt(terminal: Terminal): boolean { const buf = terminal.buffer.active; const line = buf.getLine(buf.baseY + buf.cursorY)?.translateToString(true) ?? ""; const trimmed = stripAnsi(line).trimEnd(); - if (trimmed === "") return true; + // Blank lines appear briefly between command output and prompt redraw — do + // not treat them as "back at the shell". + if (trimmed === "") return false; return PROMPT_TAIL.test(trimmed); } From 138aba5a9e02718f4a882e652facb810beb928f5 Mon Sep 17 00:00:00 2001 From: Carlos Florencio Date: Sun, 31 May 2026 19:42:30 +0100 Subject: [PATCH 3/9] fix(web): address second-pass PR review on terminal close Use initialCommand in confirm gating, pass taskId to useUserShells on mobile, and add use-terminal-busy-tracking hook tests. Co-authored-by: Cursor --- .../task/mobile/mobile-terminals-section.tsx | 2 +- apps/web/components/task/terminal-tab.tsx | 88 ++++++++++----- .../task/use-terminal-busy-tracking.test.ts | 100 ++++++++++++++++++ .../terminal/terminal-busy-registry.test.ts | 4 + .../lib/terminal/terminal-busy-registry.ts | 1 + 5 files changed, 168 insertions(+), 27 deletions(-) create mode 100644 apps/web/components/task/use-terminal-busy-tracking.test.ts diff --git a/apps/web/components/task/mobile/mobile-terminals-section.tsx b/apps/web/components/task/mobile/mobile-terminals-section.tsx index ec27551e0..8df71fbda 100644 --- a/apps/web/components/task/mobile/mobile-terminals-section.tsx +++ b/apps/web/components/task/mobile/mobile-terminals-section.tsx @@ -135,9 +135,9 @@ const MobileTerminalsList = memo(function MobileTerminalsList({ }) { const { terminals, terminalTabValue, addTerminal, removeTerminal, environmentId } = useMobileTerminalsContext(); - const { shells } = useUserShells(environmentId); const setRightPanelActiveTab = useAppStore((s) => s.setRightPanelActiveTab); const taskId = useAppStore((s) => s.tasks?.activeTaskId ?? null); + const { shells } = useUserShells(environmentId, taskId); const { pendingClose, setPendingClose, handleConfirmClose, closeTerminal } = useTerminalCloseHandler({ sessionId, diff --git a/apps/web/components/task/terminal-tab.tsx b/apps/web/components/task/terminal-tab.tsx index 4ff716e83..edede9ec6 100644 --- a/apps/web/components/task/terminal-tab.tsx +++ b/apps/web/components/task/terminal-tab.tsx @@ -78,11 +78,62 @@ function useTerminalTabState(stampedEnv: string | undefined, terminalId: string, return { shell, isOrdinary, seq, showBadge, displayName, closable }; } +function useTerminalTabClose({ + terminalId, + taskID, + stampedEnv, + shell, + closable, + panelId, + closePanel, +}: { + terminalId: string; + taskID: string | null; + stampedEnv: string | undefined; + shell: { kind?: string; initialCommand?: string } | null; + closable: boolean; + panelId: string; + closePanel: () => void; +}) { + const removeUserShellStore = useAppStore((s) => s.removeUserShell); + const [confirmClose, setConfirmClose] = useState(false); + + const destroyAndClosePanel = useCallback(async () => { + if (!stampedEnv) { + closePanel(); + return; + } + try { + await destroyUserShell(stampedEnv, terminalId, taskID ?? undefined); + removeUserShellStore(stampedEnv, terminalId); + markTerminalPanelTerminateClose(panelId); + closePanel(); + } catch (error) { + console.error("close terminal:", error); + } + }, [stampedEnv, terminalId, taskID, removeUserShellStore, panelId, closePanel]); + + const handleCloseTab = useCallback(() => { + if (!closable) return; + if ( + shouldConfirmTerminalClose(terminalId, { + kind: shell?.kind, + initialCommand: shell?.initialCommand, + }) + ) { + setConfirmClose(true); + return; + } + void destroyAndClosePanel(); + }, [closable, terminalId, shell, destroyAndClosePanel]); + + return { confirmClose, setConfirmClose, handleCloseTab, destroyAndClosePanel }; +} + export function TerminalTab(props: IDockviewPanelHeaderProps) { const { terminalId, taskID: stampedTaskID, environmentId: stampedEnv } = extractParams(props); const activeTaskID = useAppStore((s) => s.tasks?.activeTaskId ?? null); const taskID = stampedTaskID ?? activeTaskID ?? null; - const removeUserShellStore = useAppStore((s) => s.removeUserShell); const { shell, isOrdinary, seq, showBadge, displayName, closable } = useTerminalTabState( stampedEnv, terminalId, @@ -97,7 +148,16 @@ export function TerminalTab(props: IDockviewPanelHeaderProps) { }, [props.api, displayName]); const [isRenaming, setIsRenaming] = useState(false); - const [confirmClose, setConfirmClose] = useState(false); + const { confirmClose, setConfirmClose, handleCloseTab, destroyAndClosePanel } = + useTerminalTabClose({ + terminalId, + taskID, + stampedEnv, + shell, + closable, + panelId: props.api.id, + closePanel: () => props.api.close(), + }); const handleCommitRename = useRenameCommitter({ isOrdinary, stampedEnv, @@ -107,30 +167,6 @@ export function TerminalTab(props: IDockviewPanelHeaderProps) { onDone: () => setIsRenaming(false), }); - const destroyAndClosePanel = useCallback(async () => { - if (!stampedEnv) { - props.api.close(); - return; - } - try { - await destroyUserShell(stampedEnv, terminalId, taskID ?? undefined); - removeUserShellStore(stampedEnv, terminalId); - markTerminalPanelTerminateClose(props.api.id); - props.api.close(); - } catch (error) { - console.error("close terminal:", error); - } - }, [stampedEnv, terminalId, taskID, removeUserShellStore, props.api]); - - const handleCloseTab = useCallback(() => { - if (!closable) return; - if (shouldConfirmTerminalClose(terminalId, { kind: shell?.kind })) { - setConfirmClose(true); - return; - } - void destroyAndClosePanel(); - }, [closable, terminalId, shell, destroyAndClosePanel]); - const renameInitial = shell?.customName && shell.customName !== "" ? shell.customName : displayName; const seqBadgeForInput = showBadge && typeof seq === "number" ? seq : null; diff --git a/apps/web/components/task/use-terminal-busy-tracking.test.ts b/apps/web/components/task/use-terminal-busy-tracking.test.ts new file mode 100644 index 000000000..35e1a7893 --- /dev/null +++ b/apps/web/components/task/use-terminal-busy-tracking.test.ts @@ -0,0 +1,100 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { renderHook } from "@testing-library/react"; +import type { Terminal } from "@xterm/xterm"; +import type { IDisposable } from "@xterm/xterm"; + +const registryMocks = vi.hoisted(() => ({ + markTerminalInput: vi.fn(), + markTerminalOutput: vi.fn(), + clearTerminalBusy: vi.fn(), +})); + +vi.mock("@/lib/terminal/terminal-busy-registry", () => registryMocks); + +import { useTerminalBusyTracking } from "./use-terminal-busy-tracking"; + +function makeDisposable(): IDisposable { + return { dispose: vi.fn() }; +} + +function makeTerminal(): Terminal { + return { + onData: vi.fn(() => makeDisposable()), + onWriteParsed: vi.fn(() => makeDisposable()), + } as unknown as Terminal; +} + +describe("useTerminalBusyTracking", () => { + beforeEach(() => { + vi.stubGlobal( + "requestAnimationFrame", + vi.fn((cb: FrameRequestCallback) => { + cb(0); + return 1; + }), + ); + vi.stubGlobal("cancelAnimationFrame", vi.fn()); + registryMocks.markTerminalInput.mockReset(); + registryMocks.markTerminalOutput.mockReset(); + registryMocks.clearTerminalBusy.mockReset(); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("registers subscriptions when enabled and terminal is ready", () => { + const terminal = makeTerminal(); + const xtermRef = { current: terminal }; + + renderHook(() => useTerminalBusyTracking("term-1", xtermRef, true, true)); + + expect(terminal.onData).toHaveBeenCalledOnce(); + expect(terminal.onWriteParsed).toHaveBeenCalledOnce(); + }); + + it("is a no-op when disabled", () => { + const terminal = makeTerminal(); + const xtermRef = { current: terminal }; + + renderHook(() => useTerminalBusyTracking("term-1", xtermRef, false, true)); + + expect(terminal.onData).not.toHaveBeenCalled(); + expect(registryMocks.clearTerminalBusy).not.toHaveBeenCalled(); + }); + + it("retries attach via requestAnimationFrame when xtermRef is initially null", () => { + vi.stubGlobal( + "requestAnimationFrame", + vi.fn(() => 42), + ); + const terminal = makeTerminal(); + const xtermRef: { current: Terminal | null } = { current: null }; + + renderHook(() => useTerminalBusyTracking("term-1", xtermRef, true, true)); + expect(terminal.onData).not.toHaveBeenCalled(); + + xtermRef.current = terminal; + const rafCb = vi.mocked(requestAnimationFrame).mock.calls[0]?.[0]; + rafCb?.(0); + + expect(terminal.onData).toHaveBeenCalledOnce(); + expect(terminal.onWriteParsed).toHaveBeenCalledOnce(); + }); + + it("cleans up subscriptions and clears busy state on unmount", () => { + const terminal = makeTerminal(); + const inputSub = makeDisposable(); + const outputSub = makeDisposable(); + vi.mocked(terminal.onData).mockReturnValue(inputSub); + vi.mocked(terminal.onWriteParsed).mockReturnValue(outputSub); + const xtermRef = { current: terminal }; + + const { unmount } = renderHook(() => useTerminalBusyTracking("term-1", xtermRef, true, true)); + unmount(); + + expect(inputSub.dispose).toHaveBeenCalledOnce(); + expect(outputSub.dispose).toHaveBeenCalledOnce(); + expect(registryMocks.clearTerminalBusy).toHaveBeenCalledWith("term-1"); + }); +}); diff --git a/apps/web/lib/terminal/terminal-busy-registry.test.ts b/apps/web/lib/terminal/terminal-busy-registry.test.ts index 1a36ed391..8b677b1bf 100644 --- a/apps/web/lib/terminal/terminal-busy-registry.test.ts +++ b/apps/web/lib/terminal/terminal-busy-registry.test.ts @@ -59,6 +59,10 @@ describe("terminal-busy-registry", () => { expect(shouldConfirmTerminalClose("term-1", { kind: "script" })).toBe(true); }); + it("requires confirm when initialCommand is set on legacy shells", () => { + expect(shouldConfirmTerminalClose("term-1", { initialCommand: "npm run dev" })).toBe(true); + }); + it("requires confirm when busy for ordinary terminals", () => { markTerminalInput("term-1", "npm install\r"); expect(shouldConfirmTerminalClose("term-1", { kind: "ordinary" })).toBe(true); diff --git a/apps/web/lib/terminal/terminal-busy-registry.ts b/apps/web/lib/terminal/terminal-busy-registry.ts index 8fd3b912f..de71e0fb3 100644 --- a/apps/web/lib/terminal/terminal-busy-registry.ts +++ b/apps/web/lib/terminal/terminal-busy-registry.ts @@ -28,6 +28,7 @@ export function shouldConfirmTerminalClose( ): boolean { if (opts.type === "script" || opts.kind === "script") return true; if (opts.kind && opts.kind !== "ordinary") return true; + if (opts.initialCommand) return true; return isTerminalBusy(terminalId); } From 1682bb13b1e0066349718da691ae401467394e79 Mon Sep 17 00:00:00 2001 From: Carlos Florencio Date: Sun, 31 May 2026 20:43:09 +0100 Subject: [PATCH 4/9] fix(web): address third-pass PR review on terminal close Keep confirm dialog open until destroy settles, update stale park doc, and extract right-panel tab contents to stay under file line cap. Co-authored-by: Cursor --- .../task/task-right-panel-tab-contents.tsx | 82 +++++++++++++++++ apps/web/components/task/task-right-panel.tsx | 89 ++----------------- .../hooks/domains/session/use-terminals.ts | 2 +- 3 files changed, 91 insertions(+), 82 deletions(-) create mode 100644 apps/web/components/task/task-right-panel-tab-contents.tsx diff --git a/apps/web/components/task/task-right-panel-tab-contents.tsx b/apps/web/components/task/task-right-panel-tab-contents.tsx new file mode 100644 index 000000000..f05306290 --- /dev/null +++ b/apps/web/components/task/task-right-panel-tab-contents.tsx @@ -0,0 +1,82 @@ +"use client"; + +import { Badge } from "@kandev/ui/badge"; +import { SessionPanelContent } from "@kandev/ui/pannel-session"; +import { TabsContent } from "@kandev/ui/tabs"; +import { ShellTerminal } from "@/components/task/shell-terminal"; +import { PassthroughTerminal } from "@/components/task/passthrough-terminal"; +import type { RepositoryScript } from "@/lib/types/http"; +import type { Terminal } from "@/hooks/domains/session/use-terminals"; + +/** Commands tab content showing repository scripts */ +export function CommandsTabContent({ + scripts, + onRunCommand, +}: { + scripts: RepositoryScript[]; + onRunCommand: (script: RepositoryScript) => void; +}) { + return ( + + +
+ {scripts.map((script) => ( + + ))} +
+ + + ); +} + +/** Terminal tab contents (dev-server and shell terminals) */ +export function TerminalTabContents({ + terminals, + environmentId, + devProcessId, + devOutput, + isStoppingDev, +}: { + terminals: Terminal[]; + environmentId: string | null; + devProcessId: string | null | undefined; + devOutput: string | undefined; + isStoppingDev: boolean; +}) { + return ( + <> + {terminals.map((terminal) => ( + + + {terminal.type === "dev-server" ? ( + + ) : ( + + )} + + + ))} + + ); +} diff --git a/apps/web/components/task/task-right-panel.tsx b/apps/web/components/task/task-right-panel.tsx index 37890fd33..dab5cf6f7 100644 --- a/apps/web/components/task/task-right-panel.tsx +++ b/apps/web/components/task/task-right-panel.tsx @@ -3,18 +3,14 @@ import type { ReactNode, MouseEvent } from "react"; import type { Layout } from "react-resizable-panels"; import { memo, useEffect, useCallback, useState, useMemo } from "react"; -import { Badge } from "@kandev/ui/badge"; -import { SessionPanel, SessionPanelContent } from "@kandev/ui/pannel-session"; +import { SessionPanel } from "@kandev/ui/pannel-session"; import { Group, Panel } from "react-resizable-panels"; -import { TabsContent } from "@kandev/ui/tabs"; import { getLocalStorage, setLocalStorage, getSessionStorage, setSessionStorage, } from "@/lib/local-storage"; -import { ShellTerminal } from "@/components/task/shell-terminal"; -import { PassthroughTerminal } from "@/components/task/passthrough-terminal"; import { useAppStore } from "@/components/state-provider"; import { useLayoutStore } from "@/lib/state/layout-store"; import { useDefaultLayout } from "@/lib/layout/use-default-layout"; @@ -24,6 +20,10 @@ import { useTerminals } from "@/hooks/domains/session/use-terminals"; import { shouldConfirmTerminalClose } from "@/lib/terminal/terminal-busy-registry"; import { ParkedTerminalsMenu } from "@/components/task/parked-terminals-menu"; import { CloseTerminalConfirmDialog } from "@/components/task/close-terminal-confirm-dialog"; +import { + CommandsTabContent, + TerminalTabContents, +} from "@/components/task/task-right-panel-tab-contents"; import type { RepositoryScript } from "@/lib/types/http"; import type { Terminal } from "@/hooks/domains/session/use-terminals"; @@ -199,7 +199,7 @@ function useConfirmableTerminalClose({ }: { terminals: Terminal[]; handleCloseTab: (event: MouseEvent, terminalId: string) => void; - destroyTerminal: (id: string) => Promise | void; + destroyTerminal: (id: string) => Promise; }) { const [pendingClose, setPendingClose] = useState(null); @@ -220,9 +220,9 @@ function useConfirmableTerminalClose({ [terminals, handleCloseTab], ); - const handleConfirmClose = useCallback(() => { + const handleConfirmClose = useCallback(async () => { if (!pendingClose) return; - void destroyTerminal(pendingClose.id); + await destroyTerminal(pendingClose.id); setPendingClose(null); }, [pendingClose, destroyTerminal]); @@ -523,77 +523,4 @@ const TaskRightPanel = memo(function TaskRightPanel({ ); }); -/** Commands tab content showing repository scripts */ -function CommandsTabContent({ - scripts, - onRunCommand, -}: { - scripts: RepositoryScript[]; - onRunCommand: (script: RepositoryScript) => void; -}) { - return ( - - -
- {scripts.map((script) => ( - - ))} -
- - - ); -} - -/** Terminal tab contents (dev-server and shell terminals) */ -function TerminalTabContents({ - terminals, - environmentId, - devProcessId, - devOutput, - isStoppingDev, -}: { - terminals: Terminal[]; - environmentId: string | null; - devProcessId: string | null | undefined; - devOutput: string | undefined; - isStoppingDev: boolean; -}) { - return ( - <> - {terminals.map((terminal) => ( - - - {terminal.type === "dev-server" ? ( - - ) : ( - - )} - - - ))} - - ); -} - export { TaskRightPanel }; diff --git a/apps/web/hooks/domains/session/use-terminals.ts b/apps/web/hooks/domains/session/use-terminals.ts index d9add1506..2e906b80a 100644 --- a/apps/web/hooks/domains/session/use-terminals.ts +++ b/apps/web/hooks/domains/session/use-terminals.ts @@ -186,7 +186,7 @@ type RemoveTerminalOpts = { /** * Source-of-truth getter for the currently active tab. The handler reads * this at call-time rather than capturing the value in the closure so an - * async close (`parkUserShell(...).then(removeTerminal)`) that resolves + * async close (`destroyUserShell(...).then(removeTerminal)`) that resolves * after the user switches tabs doesn't clobber the new selection with a * stale fallback shift. */ From ee84b72c0ed59eb86de14e74ef48af24675827da Mon Sep 17 00:00:00 2001 From: Carlos Florencio Date: Sun, 31 May 2026 20:58:43 +0100 Subject: [PATCH 5/9] fix(web): thread initialCommand through terminal close confirm Extend Terminal with initialCommand from UserShellInfo so right-panel and mobile close paths confirm before destroying legacy script shells. Co-authored-by: Cursor --- .../web/components/task/mobile/mobile-terminals-section.tsx | 1 + apps/web/components/task/task-right-panel.tsx | 6 +++++- apps/web/hooks/domains/session/use-terminals-build.ts | 1 + apps/web/hooks/domains/session/use-terminals-types.ts | 1 + 4 files changed, 8 insertions(+), 1 deletion(-) diff --git a/apps/web/components/task/mobile/mobile-terminals-section.tsx b/apps/web/components/task/mobile/mobile-terminals-section.tsx index 8df71fbda..f3a9e5861 100644 --- a/apps/web/components/task/mobile/mobile-terminals-section.tsx +++ b/apps/web/components/task/mobile/mobile-terminals-section.tsx @@ -167,6 +167,7 @@ const MobileTerminalsList = memo(function MobileTerminalsList({ const needsConfirm = shouldConfirmTerminalClose(terminal.id, { type: terminal.type, kind: terminal.kind, + initialCommand: terminal.initialCommand, }); if (needsConfirm) { setPendingClose(terminal); diff --git a/apps/web/components/task/task-right-panel.tsx b/apps/web/components/task/task-right-panel.tsx index dab5cf6f7..8755888a9 100644 --- a/apps/web/components/task/task-right-panel.tsx +++ b/apps/web/components/task/task-right-panel.tsx @@ -208,7 +208,11 @@ function useConfirmableTerminalClose({ const terminal = terminals.find((t) => t.id === terminalId); const needsConfirm = !!terminal && - shouldConfirmTerminalClose(terminalId, { type: terminal.type, kind: terminal.kind }); + shouldConfirmTerminalClose(terminalId, { + type: terminal.type, + kind: terminal.kind, + initialCommand: terminal.initialCommand, + }); if (needsConfirm) { event.preventDefault(); event.stopPropagation(); diff --git a/apps/web/hooks/domains/session/use-terminals-build.ts b/apps/web/hooks/domains/session/use-terminals-build.ts index 523c9810e..85f2d5f61 100644 --- a/apps/web/hooks/domains/session/use-terminals-build.ts +++ b/apps/web/hooks/domains/session/use-terminals-build.ts @@ -37,6 +37,7 @@ export function shellToTerminal(shell: UserShellInfo): Terminal { customName: shell.customName, state: shell.state, ptyStatus: shell.ptyStatus, + initialCommand: shell.initialCommand, }; } diff --git a/apps/web/hooks/domains/session/use-terminals-types.ts b/apps/web/hooks/domains/session/use-terminals-types.ts index 0c9732b6d..b5e877302 100644 --- a/apps/web/hooks/domains/session/use-terminals-types.ts +++ b/apps/web/hooks/domains/session/use-terminals-types.ts @@ -22,4 +22,5 @@ export type Terminal = { customName?: string | null; state?: UserShellState; ptyStatus?: UserShellPTYStatus; + initialCommand?: string; }; From 2f82c70d5b134b307d5e1bb44648b31a13fea4c9 Mon Sep 17 00:00:00 2001 From: Carlos Florencio Date: Sun, 31 May 2026 21:08:38 +0100 Subject: [PATCH 6/9] fix(web): use shell kind only for terminal close confirm Drop initialCommand from confirm gating so ordinary shells with cd metadata close idle without a dialog; short-circuit busy output checks. Co-authored-by: Cursor --- .../components/task/mobile/mobile-terminals-section.tsx | 1 - apps/web/components/task/task-right-panel.tsx | 1 - apps/web/components/task/terminal-tab.tsx | 3 +-- apps/web/hooks/domains/session/use-terminals-build.ts | 1 - apps/web/hooks/domains/session/use-terminals-types.ts | 1 - apps/web/lib/terminal/terminal-busy-registry.test.ts | 9 +++++++-- apps/web/lib/terminal/terminal-busy-registry.ts | 3 +-- 7 files changed, 9 insertions(+), 10 deletions(-) diff --git a/apps/web/components/task/mobile/mobile-terminals-section.tsx b/apps/web/components/task/mobile/mobile-terminals-section.tsx index f3a9e5861..8df71fbda 100644 --- a/apps/web/components/task/mobile/mobile-terminals-section.tsx +++ b/apps/web/components/task/mobile/mobile-terminals-section.tsx @@ -167,7 +167,6 @@ const MobileTerminalsList = memo(function MobileTerminalsList({ const needsConfirm = shouldConfirmTerminalClose(terminal.id, { type: terminal.type, kind: terminal.kind, - initialCommand: terminal.initialCommand, }); if (needsConfirm) { setPendingClose(terminal); diff --git a/apps/web/components/task/task-right-panel.tsx b/apps/web/components/task/task-right-panel.tsx index 8755888a9..1d5b9b788 100644 --- a/apps/web/components/task/task-right-panel.tsx +++ b/apps/web/components/task/task-right-panel.tsx @@ -211,7 +211,6 @@ function useConfirmableTerminalClose({ shouldConfirmTerminalClose(terminalId, { type: terminal.type, kind: terminal.kind, - initialCommand: terminal.initialCommand, }); if (needsConfirm) { event.preventDefault(); diff --git a/apps/web/components/task/terminal-tab.tsx b/apps/web/components/task/terminal-tab.tsx index edede9ec6..04c25a9f4 100644 --- a/apps/web/components/task/terminal-tab.tsx +++ b/apps/web/components/task/terminal-tab.tsx @@ -90,7 +90,7 @@ function useTerminalTabClose({ terminalId: string; taskID: string | null; stampedEnv: string | undefined; - shell: { kind?: string; initialCommand?: string } | null; + shell: { kind?: string } | null; closable: boolean; panelId: string; closePanel: () => void; @@ -118,7 +118,6 @@ function useTerminalTabClose({ if ( shouldConfirmTerminalClose(terminalId, { kind: shell?.kind, - initialCommand: shell?.initialCommand, }) ) { setConfirmClose(true); diff --git a/apps/web/hooks/domains/session/use-terminals-build.ts b/apps/web/hooks/domains/session/use-terminals-build.ts index 85f2d5f61..523c9810e 100644 --- a/apps/web/hooks/domains/session/use-terminals-build.ts +++ b/apps/web/hooks/domains/session/use-terminals-build.ts @@ -37,7 +37,6 @@ export function shellToTerminal(shell: UserShellInfo): Terminal { customName: shell.customName, state: shell.state, ptyStatus: shell.ptyStatus, - initialCommand: shell.initialCommand, }; } diff --git a/apps/web/hooks/domains/session/use-terminals-types.ts b/apps/web/hooks/domains/session/use-terminals-types.ts index b5e877302..0c9732b6d 100644 --- a/apps/web/hooks/domains/session/use-terminals-types.ts +++ b/apps/web/hooks/domains/session/use-terminals-types.ts @@ -22,5 +22,4 @@ export type Terminal = { customName?: string | null; state?: UserShellState; ptyStatus?: UserShellPTYStatus; - initialCommand?: string; }; diff --git a/apps/web/lib/terminal/terminal-busy-registry.test.ts b/apps/web/lib/terminal/terminal-busy-registry.test.ts index 8b677b1bf..15d2d9e4e 100644 --- a/apps/web/lib/terminal/terminal-busy-registry.test.ts +++ b/apps/web/lib/terminal/terminal-busy-registry.test.ts @@ -54,13 +54,18 @@ describe("terminal-busy-registry", () => { expect(isTerminalBusy("term-1")).toBe(true); }); + it("ignores output when terminal is already idle", () => { + markTerminalOutput("term-1", mockTerminal("user@host:~/proj$ ")); + expect(isTerminalBusy("term-1")).toBe(false); + }); + it("requires confirm for script terminals regardless of busy state", () => { expect(shouldConfirmTerminalClose("term-1", { type: "script" })).toBe(true); expect(shouldConfirmTerminalClose("term-1", { kind: "script" })).toBe(true); }); - it("requires confirm when initialCommand is set on legacy shells", () => { - expect(shouldConfirmTerminalClose("term-1", { initialCommand: "npm run dev" })).toBe(true); + it("does not require confirm for idle ordinary terminals with initialCommand metadata", () => { + expect(shouldConfirmTerminalClose("term-1", { kind: "ordinary", type: "shell" })).toBe(false); }); it("requires confirm when busy for ordinary terminals", () => { diff --git a/apps/web/lib/terminal/terminal-busy-registry.ts b/apps/web/lib/terminal/terminal-busy-registry.ts index de71e0fb3..6785fb76e 100644 --- a/apps/web/lib/terminal/terminal-busy-registry.ts +++ b/apps/web/lib/terminal/terminal-busy-registry.ts @@ -18,7 +18,6 @@ const PROMPT_TAIL = /(?:(?:^|\s)|[\w.@~:/-]*[@~/][\w.@~:/-]*)[$#>%]\s*$/; export type TerminalCloseConfirmOpts = { kind?: string; type?: string; - initialCommand?: string; }; /** True when closing should warn because a script/non-ordinary shell or a busy command is active. */ @@ -28,7 +27,6 @@ export function shouldConfirmTerminalClose( ): boolean { if (opts.type === "script" || opts.kind === "script") return true; if (opts.kind && opts.kind !== "ordinary") return true; - if (opts.initialCommand) return true; return isTerminalBusy(terminalId); } @@ -41,6 +39,7 @@ export function markTerminalInput(terminalId: string, data: string): void { export function markTerminalOutput(terminalId: string, terminal: Terminal): void { if (typeof window === "undefined") return; + if (!busyByTerminalId.get(terminalId)) return; if (bufferLooksAtPrompt(terminal)) { busyByTerminalId.set(terminalId, false); } From 97e5998b4b02d7ad5a508d30bdcce72802173c4e Mon Sep 17 00:00:00 2001 From: Carlos Florencio Date: Sun, 31 May 2026 21:49:39 +0100 Subject: [PATCH 7/9] fix(web): keep close dialog open until destroy settles Prevent AlertDialogAction from closing early; exclude bash heredoc PS2 prompt from idle detection so busy shells still warn on close. Co-authored-by: Cursor --- .../task/close-terminal-confirm-dialog.tsx | 8 ++++---- apps/web/components/task/terminal-tab.tsx | 5 ++++- apps/web/lib/terminal/terminal-busy-registry.test.ts | 12 ++++++++++++ apps/web/lib/terminal/terminal-busy-registry.ts | 11 +++++------ 4 files changed, 25 insertions(+), 11 deletions(-) diff --git a/apps/web/components/task/close-terminal-confirm-dialog.tsx b/apps/web/components/task/close-terminal-confirm-dialog.tsx index 1b5caf2a5..041acc7cf 100644 --- a/apps/web/components/task/close-terminal-confirm-dialog.tsx +++ b/apps/web/components/task/close-terminal-confirm-dialog.tsx @@ -26,7 +26,7 @@ export function CloseTerminalConfirmDialog({ open: boolean; terminalName: string; onOpenChange: (open: boolean) => void; - onConfirm: () => void; + onConfirm: () => void | Promise; }) { return ( @@ -40,9 +40,9 @@ export function CloseTerminalConfirmDialog({ Cancel { - onOpenChange(false); - onConfirm(); + onClick={(event) => { + event.preventDefault(); + void onConfirm(); }} className="cursor-pointer bg-destructive text-destructive-foreground hover:bg-destructive/90" > diff --git a/apps/web/components/task/terminal-tab.tsx b/apps/web/components/task/terminal-tab.tsx index 04c25a9f4..ea2d6a33a 100644 --- a/apps/web/components/task/terminal-tab.tsx +++ b/apps/web/components/task/terminal-tab.tsx @@ -107,6 +107,7 @@ function useTerminalTabClose({ await destroyUserShell(stampedEnv, terminalId, taskID ?? undefined); removeUserShellStore(stampedEnv, terminalId); markTerminalPanelTerminateClose(panelId); + setConfirmClose(false); closePanel(); } catch (error) { console.error("close terminal:", error); @@ -212,7 +213,9 @@ export function TerminalTab(props: IDockviewPanelHeaderProps) { { + if (!open) setConfirmClose(false); + }} onConfirm={() => void destroyAndClosePanel()} /> diff --git a/apps/web/lib/terminal/terminal-busy-registry.test.ts b/apps/web/lib/terminal/terminal-busy-registry.test.ts index 15d2d9e4e..51eb99e56 100644 --- a/apps/web/lib/terminal/terminal-busy-registry.test.ts +++ b/apps/web/lib/terminal/terminal-busy-registry.test.ts @@ -54,6 +54,18 @@ describe("terminal-busy-registry", () => { expect(isTerminalBusy("term-1")).toBe(true); }); + it("stays busy on bash heredoc continuation prompt", () => { + markTerminalInput("term-1", "cat << EOF\r"); + markTerminalOutput("term-1", mockTerminal("> ")); + expect(isTerminalBusy("term-1")).toBe(true); + }); + + it("clears busy on bare dollar prompt at line start", () => { + markTerminalInput("term-1", "echo hi\r"); + markTerminalOutput("term-1", mockTerminal("$ ")); + expect(isTerminalBusy("term-1")).toBe(false); + }); + it("ignores output when terminal is already idle", () => { markTerminalOutput("term-1", mockTerminal("user@host:~/proj$ ")); expect(isTerminalBusy("term-1")).toBe(false); diff --git a/apps/web/lib/terminal/terminal-busy-registry.ts b/apps/web/lib/terminal/terminal-busy-registry.ts index 6785fb76e..b772d7fad 100644 --- a/apps/web/lib/terminal/terminal-busy-registry.ts +++ b/apps/web/lib/terminal/terminal-busy-registry.ts @@ -8,12 +8,11 @@ import { stripAnsi } from "@/lib/utils/ansi"; const busyByTerminalId = new Map(); // A line "looks like a prompt" when its last non-space glyph is a shell -// sigil ($ # > %) that is either preceded by whitespace (bare prompts like -// "$ ") or sits at the end of a path/host-ish token containing @, ~ or / -// (e.g. "user@host:~/proj$", "/usr/bin%"). Requiring the @/~/ anchor for -// attached sigils keeps progress lines like "Building... 50%" or -// "logger.go:42" from being misread as a returned prompt. -const PROMPT_TAIL = /(?:(?:^|\s)|[\w.@~:/-]*[@~/][\w.@~:/-]*)[$#>%]\s*$/; +// sigil ($ # > %) that is either preceded by whitespace at line start for +// bare prompts like "$ " / "# " / "% " (but not "> ", which is bash PS2 / +// heredoc continuation), or sits at the end of a path/host-ish token +// containing @, ~ or / (e.g. "user@host:~/proj$", "/usr/bin%"). +const PROMPT_TAIL = /^(?:[$#%]\s*|[\w.@~:/-]*[@~/][\w.@~:/-]*[$#>%]\s*)$/; export type TerminalCloseConfirmOpts = { kind?: string; From b180179c6296776574e69c262483725b6c76eef3 Mon Sep 17 00:00:00 2001 From: Carlos Florencio Date: Sun, 31 May 2026 22:03:02 +0100 Subject: [PATCH 8/9] fix(web): recognize prefixed shell prompts in busy detection Allow optional leading context before prompt sigils so virtualenv and fish-style PS1 strings clear busy state without reopening heredoc gaps. Co-authored-by: Cursor --- .../terminal/terminal-busy-registry.test.ts | 18 ++++++++++++++++++ .../web/lib/terminal/terminal-busy-registry.ts | 11 +++++------ 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/apps/web/lib/terminal/terminal-busy-registry.test.ts b/apps/web/lib/terminal/terminal-busy-registry.test.ts index 51eb99e56..84b405383 100644 --- a/apps/web/lib/terminal/terminal-busy-registry.test.ts +++ b/apps/web/lib/terminal/terminal-busy-registry.test.ts @@ -66,6 +66,24 @@ describe("terminal-busy-registry", () => { expect(isTerminalBusy("term-1")).toBe(false); }); + it("clears busy on virtualenv-prefixed prompts", () => { + markTerminalInput("term-1", "echo hi\r"); + markTerminalOutput("term-1", mockTerminal("(venv) $ ")); + expect(isTerminalBusy("term-1")).toBe(false); + }); + + it("clears busy on fish-style bracket prompts", () => { + markTerminalInput("term-1", "echo hi\r"); + markTerminalOutput("term-1", mockTerminal("[user@host ~/proj]$ ")); + expect(isTerminalBusy("term-1")).toBe(false); + }); + + it("clears busy when the prompt has leading whitespace before the sigil", () => { + markTerminalInput("term-1", "echo hi\r"); + markTerminalOutput("term-1", mockTerminal(" $ ")); + expect(isTerminalBusy("term-1")).toBe(false); + }); + it("ignores output when terminal is already idle", () => { markTerminalOutput("term-1", mockTerminal("user@host:~/proj$ ")); expect(isTerminalBusy("term-1")).toBe(false); diff --git a/apps/web/lib/terminal/terminal-busy-registry.ts b/apps/web/lib/terminal/terminal-busy-registry.ts index b772d7fad..cea9d016d 100644 --- a/apps/web/lib/terminal/terminal-busy-registry.ts +++ b/apps/web/lib/terminal/terminal-busy-registry.ts @@ -7,12 +7,11 @@ import { stripAnsi } from "@/lib/utils/ansi"; */ const busyByTerminalId = new Map(); -// A line "looks like a prompt" when its last non-space glyph is a shell -// sigil ($ # > %) that is either preceded by whitespace at line start for -// bare prompts like "$ " / "# " / "% " (but not "> ", which is bash PS2 / -// heredoc continuation), or sits at the end of a path/host-ish token -// containing @, ~ or / (e.g. "user@host:~/proj$", "/usr/bin%"). -const PROMPT_TAIL = /^(?:[$#%]\s*|[\w.@~:/-]*[@~/][\w.@~:/-]*[$#>%]\s*)$/; +// A line "looks like a prompt" when it ends with a shell sigil ($ # > %) +// optionally preceded by arbitrary prefix text (virtualenv markers, fish +// brackets, etc.) separated by whitespace. Bare `> ` at line start is +// excluded because `>` requires a path token with @, ~, or / before it. +const PROMPT_TAIL = /^(?:.*\s)?(?:[$#%]|[\w.@~:/-]*[@~/][\w.@~:/-]*\]?[$#>%])\s*$/; export type TerminalCloseConfirmOpts = { kind?: string; From d25aae02fe6641661904bc1a69f8812ddf8d9494 Mon Sep 17 00:00:00 2001 From: Carlos Florencio Date: Sun, 31 May 2026 22:20:28 +0100 Subject: [PATCH 9/9] fix(web): tighten prompt regex bare-sigil matching Only treat $/#/% as prompts at line start or after env markers, not when arbitrary output ends with a sigil after whitespace. Co-authored-by: Cursor --- apps/web/lib/terminal/terminal-busy-registry.test.ts | 6 ++++++ apps/web/lib/terminal/terminal-busy-registry.ts | 11 ++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/apps/web/lib/terminal/terminal-busy-registry.test.ts b/apps/web/lib/terminal/terminal-busy-registry.test.ts index 84b405383..c434a6489 100644 --- a/apps/web/lib/terminal/terminal-busy-registry.test.ts +++ b/apps/web/lib/terminal/terminal-busy-registry.test.ts @@ -84,6 +84,12 @@ describe("terminal-busy-registry", () => { expect(isTerminalBusy("term-1")).toBe(false); }); + it("stays busy when command output ends with a bare sigil after text", () => { + markTerminalInput("term-1", "grep pattern\r"); + markTerminalOutput("term-1", mockTerminal("foo $")); + expect(isTerminalBusy("term-1")).toBe(true); + }); + it("ignores output when terminal is already idle", () => { markTerminalOutput("term-1", mockTerminal("user@host:~/proj$ ")); expect(isTerminalBusy("term-1")).toBe(false); diff --git a/apps/web/lib/terminal/terminal-busy-registry.ts b/apps/web/lib/terminal/terminal-busy-registry.ts index cea9d016d..daea75c0c 100644 --- a/apps/web/lib/terminal/terminal-busy-registry.ts +++ b/apps/web/lib/terminal/terminal-busy-registry.ts @@ -7,11 +7,12 @@ import { stripAnsi } from "@/lib/utils/ansi"; */ const busyByTerminalId = new Map(); -// A line "looks like a prompt" when it ends with a shell sigil ($ # > %) -// optionally preceded by arbitrary prefix text (virtualenv markers, fish -// brackets, etc.) separated by whitespace. Bare `> ` at line start is -// excluded because `>` requires a path token with @, ~, or / before it. -const PROMPT_TAIL = /^(?:.*\s)?(?:[$#%]|[\w.@~:/-]*[@~/][\w.@~:/-]*\]?[$#>%])\s*$/; +// A line "looks like a prompt" when it ends with a shell sigil ($ # > %). +// Bare sigils match only at line start (optional whitespace or a parenthesized +// env marker like "(venv) "). Path-attached sigils require @, ~, or / and +// allow optional prefix text (fish brackets, etc.). Bare `> ` is excluded. +const PROMPT_TAIL = + /^(?:(?:\([^)\n]*\)\s+|\s*)[$#%]|(?:.*\s)?[\w.@~:/-]*[@~/][\w.@~:/-]*\]?[$#>%])\s*$/; export type TerminalCloseConfirmOpts = { kind?: string;