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..041acc7cf --- /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 | Promise; +}) { + return ( + + + + Close terminal? + + {`This stops the “${terminalName}” shell and any command it's running.`} + + + + Cancel + { + event.preventDefault(); + void 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..8df71fbda 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 { 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"; import { MobilePillButton } from "./mobile-pill-button"; import { MobilePickerSheet } from "./mobile-picker-sheet"; import { useMobileTerminalsContext } from "./mobile-terminals-context"; @@ -76,44 +68,10 @@ 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; + taskId: string | null; terminals: Terminal[]; terminalTabValue: string; removeTerminal: (id: string) => void; @@ -123,6 +81,7 @@ type CloseHandlerArgs = { function useTerminalCloseHandler({ sessionId, environmentId, + taskId, terminals, terminalTabValue, removeTerminal, @@ -130,42 +89,41 @@ 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, taskId ?? undefined); + 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, + taskId, + 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({ @@ -177,16 +135,19 @@ const MobileTerminalsList = memo(function MobileTerminalsList({ }) { const { terminals, terminalTabValue, addTerminal, removeTerminal, environmentId } = useMobileTerminalsContext(); - const { shells } = useUserShells(environmentId); const setRightPanelActiveTab = useAppStore((s) => s.setRightPanelActiveTab); - const { pendingClose, setPendingClose, handleConfirmClose } = useTerminalCloseHandler({ - sessionId, - environmentId, - terminals, - terminalTabValue, - removeTerminal, - setRightPanelActiveTab, - }); + const taskId = useAppStore((s) => s.tasks?.activeTaskId ?? null); + const { shells } = useUserShells(environmentId, taskId); + const { pendingClose, setPendingClose, handleConfirmClose, closeTerminal } = + useTerminalCloseHandler({ + sessionId, + environmentId, + taskId, + terminals, + terminalTabValue, + removeTerminal, + setRightPanelActiveTab, + }); const isShellRunning = useCallback( (id: string) => shells.find((s) => s.terminalId === id)?.running ?? false, @@ -202,8 +163,18 @@ const MobileTerminalsList = memo(function MobileTerminalsList({ ); const handleAskClose = useCallback( - (terminal: Terminal) => setPendingClose(terminal), - [setPendingClose], + (terminal: Terminal) => { + const needsConfirm = shouldConfirmTerminalClose(terminal.id, { + type: terminal.type, + kind: terminal.kind, + }); + if (needsConfirm) { + setPendingClose(terminal); + return; + } + void closeTerminal(terminal); + }, + [closeTerminal, setPendingClose], ); if (!sessionId) { @@ -247,7 +218,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..5b813f327 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-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 35dbc472a..1d5b9b788 100644 --- a/apps/web/components/task/task-right-panel.tsx +++ b/apps/web/components/task/task-right-panel.tsx @@ -3,25 +3,27 @@ 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"; 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 { 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"; @@ -184,6 +186,52 @@ 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; +}) { + const [pendingClose, setPendingClose] = useState(null); + + const handleAskCloseTab = useCallback( + (event: MouseEvent, terminalId: string) => { + const terminal = terminals.find((t) => t.id === terminalId); + const needsConfirm = + !!terminal && + shouldConfirmTerminalClose(terminalId, { + type: terminal.type, + kind: terminal.kind, + }); + if (needsConfirm) { + event.preventDefault(); + event.stopPropagation(); + setPendingClose(terminal); + return; + } + handleCloseTab(event, terminalId); + }, + [terminals, handleCloseTab], + ); + + const handleConfirmClose = useCallback(async () => { + if (!pendingClose) return; + await destroyTerminal(pendingClose.id); + setPendingClose(null); + }, [pendingClose, destroyTerminal]); + + return { pendingClose, setPendingClose, handleAskCloseTab, handleConfirmClose }; +} + type CollapsedRightPanelProps = { topPanel: ReactNode; tabs: SessionTab[]; @@ -331,18 +379,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 +404,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,113 +433,97 @@ 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 ( - - ); -}); -/** Commands tab content showing repository scripts */ -function CommandsTabContent({ - scripts, - onRunCommand, -}: { - scripts: RepositoryScript[]; - onRunCommand: (script: RepositoryScript) => void; -}) { - return ( - - -
- {scripts.map((script) => ( - - ))} -
- - - ); + return { + ...terminalsApi, + rightLayoutKey, + rightLayout, + onRightLayoutChange, + isBottomCollapsed, + setIsBottomCollapsed, + environmentId, + scripts, + tabs, + handleTabChange, + closeConfirm, + }; } -/** 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; -}) { +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 ( <> - {terminals.map((terminal) => ( - - - {terminal.type === "dev-server" ? ( - - ) : ( - - )} - - - ))} + + { + if (!open) setPendingClose(null); + }} + onConfirm={handleConfirmClose} + /> ); -} +}); export { TaskRightPanel }; diff --git a/apps/web/components/task/terminal-tab.tsx b/apps/web/components/task/terminal-tab.tsx index 3988882fc..ea2d6a33a 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 { shouldConfirmTerminalClose } from "@/lib/terminal/terminal-busy-registry"; +import { CloseTerminalConfirmDialog } from "./close-terminal-confirm-dialog"; import { markTerminalPanelTerminateClose } from "./dockview-layout-setup"; /** @@ -72,14 +74,67 @@ 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 useTerminalTabClose({ + terminalId, + taskID, + stampedEnv, + shell, + closable, + panelId, + closePanel, +}: { + terminalId: string; + taskID: string | null; + stampedEnv: string | undefined; + shell: { kind?: 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); + setConfirmClose(false); + 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, + }) + ) { + 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 { shell, isOrdinary, seq, showBadge, displayName } = useTerminalTabState( + const { shell, isOrdinary, seq, showBadge, displayName, closable } = useTerminalTabState( stampedEnv, terminalId, props.api.title ?? "Terminal", @@ -93,6 +148,16 @@ export function TerminalTab(props: IDockviewPanelHeaderProps) { }, [props.api, displayName]); const [isRenaming, setIsRenaming] = 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,35 +172,53 @@ export function TerminalTab(props: IDockviewPanelHeaderProps) { 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(); + }} + /> + + { + if (!open) setConfirmClose(false); }} + onConfirm={() => void destroyAndClosePanel()} /> - + ); } @@ -175,6 +258,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 +270,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.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/components/task/use-terminal-busy-tracking.ts b/apps/web/components/task/use-terminal-busy-tracking.ts new file mode 100644 index 000000000..d92118e0f --- /dev/null +++ b/apps/web/components/task/use-terminal-busy-tracking.ts @@ -0,0 +1,47 @@ +import { useEffect } from "react"; +import type { Terminal } from "@xterm/xterm"; +import type { IDisposable } 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, + terminalReady: boolean, +): void { + useEffect(() => { + if (!enabled || !terminalId || !terminalReady) return; + + 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 () => { + disposed = true; + if (raf) cancelAnimationFrame(raf); + inputSub?.dispose(); + outputSub?.dispose(); + clearTerminalBusy(terminalId); + }; + // xtermRef object is stable; included to satisfy exhaustive-deps. + }, [enabled, terminalId, terminalReady, 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..2e906b80a 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, @@ -187,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. */ @@ -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..c434a6489 --- /dev/null +++ b/apps/web/lib/terminal/terminal-busy-registry.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, it, beforeEach } from "vitest"; +import type { Terminal } from "@xterm/xterm"; +import { + isTerminalBusy, + markTerminalInput, + markTerminalOutput, + resetTerminalBusyRegistry, + shouldConfirmTerminalClose, +} 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(() => { + resetTerminalBusyRegistry(); + }); + + 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); + }); + + 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("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("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("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); + }); + + 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("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", () => { + 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 new file mode 100644 index 000000000..daea75c0c --- /dev/null +++ b/apps/web/lib/terminal/terminal-busy-registry.ts @@ -0,0 +1,68 @@ +import type { Terminal } from "@xterm/xterm"; +import { stripAnsi } from "@/lib/utils/ansi"; + +/** + * 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 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; + type?: 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 (!busyByTerminalId.get(terminalId)) return; + 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); +} + +/** 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(); + // 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); +}