From b350d3f6b7936233126e0b852722ab71952e50fd Mon Sep 17 00:00:00 2001 From: dimakis Date: Thu, 25 Jun 2026 12:29:43 +0100 Subject: [PATCH 1/7] =?UTF-8?q?feat(terminal):=20add=20interactive=20shell?= =?UTF-8?q?=20terminal=20=E2=80=94=20Phase=201?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Server: node-pty based terminal manager (spawn, I/O, resize, cleanup) - Protocol: terminal_create/input/resize/destroy WS message schemas - Server: WS handlers wired into v2 dispatcher - Server: GET /api/terminals REST endpoint - Frontend: xterm.js Terminal component with mobile keyboard toolbar - Frontend: TerminalView page with PTY connection via useTerminal hook - Frontend: Route (/terminal, /terminal/:sessionId) + nav integration - Mobile: touch-friendly toolbar (Esc, Tab, Ctrl, arrows, pipe, slash, tilde) - Desktop: toolbar auto-hidden, Terminal in sidebar nav Co-Authored-By: Claude Opus 4.6 --- frontend/src/App.tsx | 17 +++ frontend/src/components/DesktopNav.tsx | 1 + frontend/src/components/MobileShell.tsx | 2 +- frontend/src/components/TabBar.tsx | 13 +- frontend/src/components/Terminal.tsx | 165 ++++++++++++++++++++ frontend/src/hooks/useTerminal.ts | 170 +++++++++++++++++++++ frontend/src/main.tsx | 1 + frontend/src/pages/TerminalView.tsx | 60 ++++++++ frontend/src/styles/terminal.css | 132 ++++++++++++++++ frontend/src/types/ws-messages.ts | 36 ++++- packages/protocol/src/index.ts | 4 + packages/protocol/src/ws-schemas-v2.ts | 31 ++++ server/app.ts | 8 + server/terminal-manager.ts | 191 ++++++++++++++++++++++++ server/ws-handler-v2.ts | 151 +++++++++++++++++++ 15 files changed, 979 insertions(+), 3 deletions(-) create mode 100644 frontend/src/components/Terminal.tsx create mode 100644 frontend/src/hooks/useTerminal.ts create mode 100644 frontend/src/pages/TerminalView.tsx create mode 100644 frontend/src/styles/terminal.css create mode 100644 server/terminal-manager.ts diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 0280d010..27b0defe 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -13,6 +13,7 @@ import { CalendarView } from './pages/CalendarView'; import { TodoView } from './pages/TodoView'; import { TodoDetailView } from './pages/TodoDetailView'; import { TaskBoard } from './pages/TaskBoard'; +import { TerminalView } from './pages/TerminalView'; import { ErrorBoundary } from './components/ErrorBoundary'; import { MobileShell } from './components/MobileShell'; import { DesktopShell } from './components/DesktopShell'; @@ -168,6 +169,22 @@ export function App() { } /> + + + + } + /> + + + + } + /> diff --git a/frontend/src/components/DesktopNav.tsx b/frontend/src/components/DesktopNav.tsx index 08d0dec7..22134864 100644 --- a/frontend/src/components/DesktopNav.tsx +++ b/frontend/src/components/DesktopNav.tsx @@ -19,6 +19,7 @@ export function DesktopNav() { }, { label: 'Calendar', path: '/calendar', match: (p) => p.startsWith('/calendar') }, { label: 'Files', path: '/files', match: (p) => p.startsWith('/files') }, + { label: 'Terminal', path: '/terminal', match: (p) => p.startsWith('/terminal') }, ]; return ( diff --git a/frontend/src/components/MobileShell.tsx b/frontend/src/components/MobileShell.tsx index 06e8c306..7b54d36a 100644 --- a/frontend/src/components/MobileShell.tsx +++ b/frontend/src/components/MobileShell.tsx @@ -2,7 +2,7 @@ import { useLocation } from 'react-router-dom'; import { useIsDesktop } from '../hooks/useMediaQuery'; import { TabBar } from './TabBar'; -const HIDE_TAB_BAR = ['/login', '/chat']; +const HIDE_TAB_BAR = ['/login', '/chat', '/terminal']; function shouldHideTabBar(pathname: string): boolean { return HIDE_TAB_BAR.some((p) => pathname === p || pathname.startsWith(p + '/')); diff --git a/frontend/src/components/TabBar.tsx b/frontend/src/components/TabBar.tsx index 44c63821..2940a97d 100644 --- a/frontend/src/components/TabBar.tsx +++ b/frontend/src/components/TabBar.tsx @@ -39,7 +39,9 @@ export function TabBar() { }, ]; - const isMoreActive = ['/tasks', '/files'].some((p) => location.pathname.startsWith(p)); + const isMoreActive = ['/tasks', '/files', '/terminal'].some((p) => + location.pathname.startsWith(p), + ); return ( <> @@ -64,6 +66,15 @@ export function TabBar() { > Files +
+ ))} +
+ + ); +} diff --git a/frontend/src/hooks/useTerminal.ts b/frontend/src/hooks/useTerminal.ts new file mode 100644 index 00000000..ae188935 --- /dev/null +++ b/frontend/src/hooks/useTerminal.ts @@ -0,0 +1,170 @@ +/** + * useTerminal — manages a WebSocket connection to the terminal backend. + * + * Shares the same WS endpoint as chat (/ws/chat) but sends terminal-specific + * message types (terminal_create, terminal_input, terminal_resize, terminal_destroy). + * The v2 protocol dispatcher routes these to the terminal handlers. + */ + +import { useRef, useCallback, useEffect, useState } from 'react'; +import { getWsChatUrl } from '../lib/api-fetch'; + +export interface TerminalState { + terminalId: string | null; + connected: boolean; + exited: boolean; + exitCode?: number; +} + +interface UseTerminalOptions { + sessionId: string; + cols?: number; + rows?: number; + onData: (data: string) => void; + onExit?: (exitCode: number, signal?: number) => void; + onError?: (error: string) => void; +} + +export function useTerminal({ + sessionId, + cols = 80, + rows = 24, + onData, + onExit, + onError, +}: UseTerminalOptions) { + const wsRef = useRef(null); + const terminalIdRef = useRef(null); + const [state, setState] = useState({ + terminalId: null, + connected: false, + exited: false, + }); + + // Store latest callbacks in refs to avoid reconnect churn + const onDataRef = useRef(onData); + const onExitRef = useRef(onExit); + const onErrorRef = useRef(onError); + onDataRef.current = onData; + onExitRef.current = onExit; + onErrorRef.current = onError; + + const send = useCallback((msg: Record) => { + const ws = wsRef.current; + if (ws?.readyState === WebSocket.OPEN) { + ws.send(JSON.stringify(msg)); + } + }, []); + + const connect = useCallback(() => { + if (wsRef.current?.readyState === WebSocket.OPEN) return; + + const ws = new WebSocket(getWsChatUrl()); + wsRef.current = ws; + + ws.onopen = () => { + // v2 handshake + ws.send(JSON.stringify({ type: 'hello', protocolVersion: 2 })); + }; + + ws.onmessage = (event) => { + let msg: Record; + try { + msg = JSON.parse(event.data as string); + } catch { + return; + } + + switch (msg.type) { + case 'welcome': + // Handshake complete — create terminal + send({ + type: 'terminal_create', + sessionId, + cols, + rows, + }); + setState((s) => ({ ...s, connected: true })); + break; + + case 'terminal_created': + terminalIdRef.current = msg.terminalId as string; + setState((s) => ({ + ...s, + terminalId: msg.terminalId as string, + })); + break; + + case 'terminal_output': + if (msg.terminalId === terminalIdRef.current) { + onDataRef.current(msg.data as string); + } + break; + + case 'terminal_exit': + if (msg.terminalId === terminalIdRef.current) { + setState((s) => ({ + ...s, + exited: true, + exitCode: msg.exitCode as number, + })); + onExitRef.current?.(msg.exitCode as number, msg.signal as number | undefined); + } + break; + + case 'terminal_error': + onErrorRef.current?.(msg.error as string); + break; + } + }; + + ws.onclose = () => { + setState((s) => ({ ...s, connected: false })); + }; + }, [sessionId, cols, rows, send]); + + const writeInput = useCallback( + (data: string) => { + if (!terminalIdRef.current) return; + send({ type: 'terminal_input', terminalId: terminalIdRef.current, data }); + }, + [send], + ); + + const resize = useCallback( + (newCols: number, newRows: number) => { + if (!terminalIdRef.current) return; + send({ + type: 'terminal_resize', + terminalId: terminalIdRef.current, + cols: newCols, + rows: newRows, + }); + }, + [send], + ); + + const destroy = useCallback(() => { + if (terminalIdRef.current) { + send({ type: 'terminal_destroy', terminalId: terminalIdRef.current }); + } + wsRef.current?.close(); + wsRef.current = null; + terminalIdRef.current = null; + setState({ terminalId: null, connected: false, exited: false }); + }, [send]); + + // Cleanup on unmount + useEffect(() => { + return () => { + if (terminalIdRef.current && wsRef.current?.readyState === WebSocket.OPEN) { + wsRef.current.send( + JSON.stringify({ type: 'terminal_destroy', terminalId: terminalIdRef.current }), + ); + } + wsRef.current?.close(); + }; + }, []); + + return { state, connect, writeInput, resize, destroy }; +} diff --git a/frontend/src/main.tsx b/frontend/src/main.tsx index be203163..3edda088 100644 --- a/frontend/src/main.tsx +++ b/frontend/src/main.tsx @@ -9,6 +9,7 @@ import './styles/global.css'; import './styles/code-block.css'; import './styles/calendar.css'; import './styles/desktop.css'; +import './styles/terminal.css'; initTheme(); diff --git a/frontend/src/pages/TerminalView.tsx b/frontend/src/pages/TerminalView.tsx new file mode 100644 index 00000000..bab4b7b2 --- /dev/null +++ b/frontend/src/pages/TerminalView.tsx @@ -0,0 +1,60 @@ +/** + * TerminalView — full-page interactive shell terminal. + * + * Creates a PTY on the server scoped to the current session's worktree, + * renders it via xterm.js, and provides a mobile-friendly key toolbar. + */ + +import { useRef, useEffect, useCallback } from 'react'; +import { useNavigate, useParams } from 'react-router-dom'; +import { Terminal } from '../components/Terminal'; +import { useTerminal } from '../hooks/useTerminal'; + +export function TerminalView() { + const { sessionId } = useParams<{ sessionId: string }>(); + const navigate = useNavigate(); + const termRef = useRef<{ write: (data: string) => void } | null>(null); + + const resolvedSessionId = sessionId || 'default'; + + const { state, connect, writeInput, resize, destroy } = useTerminal({ + sessionId: resolvedSessionId, + onData: useCallback((data: string) => { + termRef.current?.write(data); + }, []), + onExit: useCallback((_exitCode: number) => { + termRef.current?.write('\r\n\x1b[90m[Process exited — press any key to restart]\x1b[0m\r\n'); + }, []), + onError: useCallback((error: string) => { + termRef.current?.write(`\r\n\x1b[31mError: ${error}\x1b[0m\r\n`); + }, []), + }); + + // Connect on mount + useEffect(() => { + connect(); + return () => destroy(); + }, [connect, destroy]); + + return ( +
+
+ + + Terminal + {state.connected && !state.exited && ( + + )} + {state.exited && (exited)} + {!state.connected && !state.exited && ( + (connecting...) + )} + +
+
+ +
+ ); +} diff --git a/frontend/src/styles/terminal.css b/frontend/src/styles/terminal.css new file mode 100644 index 00000000..9ded624c --- /dev/null +++ b/frontend/src/styles/terminal.css @@ -0,0 +1,132 @@ +/* Terminal view + component styles */ + +.terminal-view { + display: flex; + flex-direction: column; + height: 100dvh; + background: #1a1a2e; + color: #e0e0e0; +} + +.terminal-header { + display: flex; + align-items: center; + padding: 8px 12px; + background: #16162a; + border-bottom: 1px solid #2a2a4e; + flex-shrink: 0; + /* Safe area for iOS notch */ + padding-top: max(8px, env(safe-area-inset-top)); +} + +.terminal-back-btn { + background: none; + border: none; + color: var(--accent, #74c0fc); + font-size: 16px; + cursor: pointer; + padding: 4px 8px; + margin-right: 8px; +} + +.terminal-title { + font-size: 15px; + font-weight: 600; + display: flex; + align-items: center; + gap: 6px; +} + +.terminal-header-spacer { + flex: 1; +} + +.terminal-status { + width: 8px; + height: 8px; + border-radius: 50%; + display: inline-block; +} + +.terminal-status--live { + background: #51cf66; +} + +.terminal-status-text { + font-size: 12px; + font-weight: 400; + color: #888; +} + +/* Terminal container — fills remaining space */ +.terminal-container { + display: flex; + flex-direction: column; + flex: 1; + min-height: 0; +} + +.terminal-xterm { + flex: 1; + min-height: 0; + padding: 4px; +} + +/* Make xterm fill its container */ +.terminal-xterm .xterm { + height: 100%; +} + +.terminal-xterm .xterm-viewport { + overflow-y: auto !important; +} + +/* Mobile keyboard toolbar */ +.terminal-toolbar { + display: flex; + gap: 4px; + padding: 6px 8px; + padding-bottom: max(6px, env(safe-area-inset-bottom)); + background: #16162a; + border-top: 1px solid #2a2a4e; + overflow-x: auto; + flex-shrink: 0; + -webkit-overflow-scrolling: touch; +} + +.terminal-toolbar-key { + background: #2a2a4e; + border: 1px solid #3a3a5e; + border-radius: 6px; + color: #e0e0e0; + font-size: 13px; + font-family: inherit; + padding: 8px 12px; + min-width: 40px; + cursor: pointer; + user-select: none; + -webkit-user-select: none; + touch-action: manipulation; + flex-shrink: 0; +} + +.terminal-toolbar-key:active { + background: #3a3a5e; +} + +.terminal-toolbar-key--active { + background: var(--accent, #74c0fc); + color: #1a1a2e; + border-color: var(--accent, #74c0fc); +} + +/* Desktop: hide toolbar (real keyboard available) */ +@media (min-width: 768px) { + .terminal-toolbar { + display: none; + } + + .terminal-xterm { + padding: 8px; + } +} diff --git a/frontend/src/types/ws-messages.ts b/frontend/src/types/ws-messages.ts index 05ae4b3e..013da20d 100644 --- a/frontend/src/types/ws-messages.ts +++ b/frontend/src/types/ws-messages.ts @@ -218,6 +218,36 @@ interface TaskDeletedMsg { taskId: string; } +// ─── Terminal messages (server → client) ─────────────────────────────────── + +export interface TerminalCreatedMsg { + type: 'terminal_created'; + terminalId: string; + sessionId: string; + pid: number; + cols: number; + rows: number; +} + +export interface TerminalOutputMsg { + type: 'terminal_output'; + terminalId: string; + data: string; +} + +export interface TerminalExitMsg { + type: 'terminal_exit'; + terminalId: string; + exitCode: number; + signal?: number; +} + +export interface TerminalErrorMsg { + type: 'terminal_error'; + terminalId?: string; + error: string; +} + export type ServerMessage = | ClientIdMsg | ReattachedMsg @@ -260,7 +290,11 @@ export type ServerMessage = | SubagentBlockEndMsg | SubagentToolResultMsg | SubagentEndMsg - | SubagentCancelledMsg; + | SubagentCancelledMsg + | TerminalCreatedMsg + | TerminalOutputMsg + | TerminalExitMsg + | TerminalErrorMsg; export interface ProgressStartMsg { type: 'progress_start'; diff --git a/packages/protocol/src/index.ts b/packages/protocol/src/index.ts index 7c1040f7..101ee53c 100644 --- a/packages/protocol/src/index.ts +++ b/packages/protocol/src/index.ts @@ -106,6 +106,10 @@ export { V2StopMessage, V2PermissionResponseMessage, V2SetModeMessage, + TerminalCreateMessage, + TerminalInputMessage, + TerminalResizeMessage, + TerminalDestroyMessage, IncomingWsMessageV2, } from './ws-schemas-v2.js'; diff --git a/packages/protocol/src/ws-schemas-v2.ts b/packages/protocol/src/ws-schemas-v2.ts index 8fc07d35..7e635ac8 100644 --- a/packages/protocol/src/ws-schemas-v2.ts +++ b/packages/protocol/src/ws-schemas-v2.ts @@ -122,6 +122,33 @@ export const V2SetModeMessage = z.object({ mode: z.enum(['ask', 'agent', 'auto']), }); +// ─── Terminal messages ───────────────────────────────────────────────────── + +export const TerminalCreateMessage = z.object({ + type: z.literal('terminal_create'), + sessionId: z.string().min(1), + cols: z.number().int().min(1).optional(), + rows: z.number().int().min(1).optional(), +}); + +export const TerminalInputMessage = z.object({ + type: z.literal('terminal_input'), + terminalId: z.string().min(1), + data: z.string(), +}); + +export const TerminalResizeMessage = z.object({ + type: z.literal('terminal_resize'), + terminalId: z.string().min(1), + cols: z.number().int().min(1), + rows: z.number().int().min(1), +}); + +export const TerminalDestroyMessage = z.object({ + type: z.literal('terminal_destroy'), + terminalId: z.string().min(1), +}); + // ─── Union ────────────────────────────────────────────────────────────────── export const IncomingWsMessageV2 = z.discriminatedUnion('type', [ @@ -137,6 +164,10 @@ export const IncomingWsMessageV2 = z.discriminatedUnion('type', [ V2StopMessage, V2PermissionResponseMessage, V2SetModeMessage, + TerminalCreateMessage, + TerminalInputMessage, + TerminalResizeMessage, + TerminalDestroyMessage, ]); export type IncomingWsMessageV2 = z.infer; diff --git a/server/app.ts b/server/app.ts index 72bd0f9e..a6699b46 100644 --- a/server/app.ts +++ b/server/app.ts @@ -41,6 +41,7 @@ import { isValidInternalToken } from './internal-token.js'; import { getLocalCommit, isUpdateAvailable } from './git-version.js'; import { resolvePending } from './permissions.js'; import { createLogger } from './logger.js'; +import { listTerminals } from './terminal-manager.js'; import { handleTaskSet, handleTaskComplete, @@ -622,6 +623,13 @@ app.get('/api/service-health', (_req, res) => { res.json(healthMonitor?.getSnapshot() ?? { services: [], checkedAt: 0 }); }); +// --- Terminal API --- + +app.get('/api/terminals', (req, res) => { + const sessionId = req.query.sessionId as string | undefined; + res.json({ terminals: listTerminals(sessionId) }); +}); + // --- Task Board API --- app.get('/api/tasks', (_req, res) => { diff --git a/server/terminal-manager.ts b/server/terminal-manager.ts new file mode 100644 index 00000000..c7f5384d --- /dev/null +++ b/server/terminal-manager.ts @@ -0,0 +1,191 @@ +/** + * Terminal Manager — PTY lifecycle management for interactive shell terminals. + * + * Spawns node-pty processes, routes I/O to WebSocket connections, and handles + * resize/cleanup. Each terminal is scoped to a session and inherits the + * session's worktree cwd. + */ + +import * as pty from 'node-pty'; +import { createLogger } from './logger.js'; + +const log = createLogger('terminal'); + +export interface TerminalInfo { + id: string; + sessionId: string; + pid: number; + cols: number; + rows: number; + cwd: string; + createdAt: number; +} + +interface ManagedTerminal { + id: string; + sessionId: string; + process: pty.IPty; + cols: number; + rows: number; + cwd: string; + createdAt: number; + /** Callback to send output data to the client. */ + onData: ((data: string) => void) | null; + /** Callback when the terminal process exits. */ + onExit: ((exitCode: number, signal?: number) => void) | null; +} + +let terminalCounter = 0; + +/** Active terminals keyed by terminal ID. */ +const terminals = new Map(); + +function generateTerminalId(): string { + return `term-${Date.now()}-${++terminalCounter}`; +} + +function getDefaultShell(): string { + return process.env.SHELL || (process.platform === 'win32' ? 'powershell.exe' : '/bin/zsh'); +} + +export function createTerminal( + sessionId: string, + cwd: string, + opts?: { cols?: number; rows?: number; env?: Record }, +): TerminalInfo { + const id = generateTerminalId(); + const cols = opts?.cols ?? 80; + const rows = opts?.rows ?? 24; + const shell = getDefaultShell(); + + const proc = pty.spawn(shell, [], { + name: 'xterm-256color', + cols, + rows, + cwd, + env: { + ...process.env, + TERM: 'xterm-256color', + COLORTERM: 'truecolor', + ...opts?.env, + } as Record, + }); + + const managed: ManagedTerminal = { + id, + sessionId, + process: proc, + cols, + rows, + cwd, + createdAt: Date.now(), + onData: null, + onExit: null, + }; + + proc.onData((data) => { + managed.onData?.(data); + }); + + proc.onExit(({ exitCode, signal }) => { + log.info('terminal exited', { id, sessionId, exitCode, signal }); + managed.onExit?.(exitCode, signal); + terminals.delete(id); + }); + + terminals.set(id, managed); + log.info('terminal created', { id, sessionId, cwd, shell, pid: proc.pid }); + + return { id, sessionId, pid: proc.pid, cols, rows, cwd, createdAt: managed.createdAt }; +} + +export function writeTerminal(id: string, data: string): boolean { + const term = terminals.get(id); + if (!term) return false; + term.process.write(data); + return true; +} + +export function resizeTerminal(id: string, cols: number, rows: number): boolean { + const term = terminals.get(id); + if (!term) return false; + term.process.resize(cols, rows); + term.cols = cols; + term.rows = rows; + return true; +} + +export function destroyTerminal(id: string): boolean { + const term = terminals.get(id); + if (!term) return false; + term.process.kill(); + terminals.delete(id); + log.info('terminal destroyed', { id, sessionId: term.sessionId }); + return true; +} + +export function getTerminal(id: string): TerminalInfo | null { + const term = terminals.get(id); + if (!term) return null; + return { + id: term.id, + sessionId: term.sessionId, + pid: term.process.pid, + cols: term.cols, + rows: term.rows, + cwd: term.cwd, + createdAt: term.createdAt, + }; +} + +export function listTerminals(sessionId?: string): TerminalInfo[] { + const result: TerminalInfo[] = []; + for (const term of terminals.values()) { + if (sessionId && term.sessionId !== sessionId) continue; + result.push({ + id: term.id, + sessionId: term.sessionId, + pid: term.process.pid, + cols: term.cols, + rows: term.rows, + cwd: term.cwd, + createdAt: term.createdAt, + }); + } + return result; +} + +export function destroySessionTerminals(sessionId: string): number { + let count = 0; + for (const [id, term] of terminals.entries()) { + if (term.sessionId === sessionId) { + term.process.kill(); + terminals.delete(id); + count++; + } + } + if (count > 0) { + log.info('destroyed session terminals', { sessionId, count }); + } + return count; +} + +export function setTerminalCallbacks( + id: string, + onData: (data: string) => void, + onExit: (exitCode: number, signal?: number) => void, +): boolean { + const term = terminals.get(id); + if (!term) return false; + term.onData = onData; + term.onExit = onExit; + return true; +} + +export function clearTerminalCallbacks(id: string): boolean { + const term = terminals.get(id); + if (!term) return false; + term.onData = null; + term.onExit = null; + return true; +} diff --git a/server/ws-handler-v2.ts b/server/ws-handler-v2.ts index 32d9d5a1..821a7b14 100644 --- a/server/ws-handler-v2.ts +++ b/server/ws-handler-v2.ts @@ -24,6 +24,10 @@ import { V2InterruptMessage, V2PermissionResponseMessage, V2SetModeMessage, + TerminalCreateMessage, + TerminalInputMessage, + TerminalResizeMessage, + TerminalDestroyMessage, } from '@mitzo/protocol'; import type { z } from 'zod'; @@ -37,6 +41,10 @@ type StopMsg = z.infer; type InterruptMsg = z.infer; type PermissionMsg = z.infer; type SetModeMsg = z.infer; +type TerminalCreateMsg = z.infer; +type TerminalInputMsg = z.infer; +type TerminalResizeMsg = z.infer; +type TerminalDestroyMsg = z.infer; import { randomUUID } from 'crypto'; import { withSpan, withSpanAsync } from './tracing.js'; import { SpanStatusCode } from '@opentelemetry/api'; @@ -57,6 +65,13 @@ import { setSkillPolicy, clearSkillPolicy } from './skill-policy.js'; import { resolveSlashCommand } from './slash-commands.js'; import { buildSkillRegistry, isAllowedPath, NATIVE_COMMAND_NAMES } from './app.js'; import type { NativeCommandRegistry } from './native-commands.js'; +import { + createTerminal, + writeTerminal, + resizeTerminal, + destroyTerminal, + setTerminalCallbacks, +} from './terminal-manager.js'; import { createLogger } from './logger.js'; const log = createLogger('ws-v2'); @@ -914,6 +929,130 @@ export function handleSessionClose( ); } +// ─── Terminal handlers ────────────────────────────────────────────────────── + +export function handleTerminalCreate( + connectionId: string, + msg: TerminalCreateMsg, + ctx: V2HandlerContext, +): void { + withSpan( + 'ws.terminal_create', + { 'ws.connectionId': connectionId, 'ws.sessionId': msg.sessionId }, + () => { + const conn = ctx.connRegistry.get(connectionId); + if (!conn) return; + + // Resolve cwd from session metadata (worktree path or base repo) + const sessionMeta = ctx.eventStore.getSession(msg.sessionId); + const cwd = sessionMeta?.cwd || BASE_REPO || process.cwd(); + + try { + const info = createTerminal(msg.sessionId, cwd, { + cols: msg.cols, + rows: msg.rows, + }); + + // Wire PTY output → WS broadcast to connection + setTerminalCallbacks( + info.id, + (data) => { + conn.transport.send({ + type: 'terminal_output', + terminalId: info.id, + data, + }); + }, + (exitCode, signal) => { + conn.transport.send({ + type: 'terminal_exit', + terminalId: info.id, + exitCode, + ...(signal !== undefined ? { signal } : {}), + }); + }, + ); + + conn.transport.send({ + type: 'terminal_created', + terminalId: info.id, + sessionId: msg.sessionId, + pid: info.pid, + cols: info.cols, + rows: info.rows, + }); + + log.info('terminal created via ws', { + connectionId, + sessionId: msg.sessionId, + terminalId: info.id, + cwd, + }); + } catch (err: unknown) { + const message = err instanceof Error ? err.message : String(err); + conn.transport.send({ + type: 'terminal_error', + error: `Failed to create terminal: ${message}`, + }); + log.error('terminal create failed', { + connectionId, + sessionId: msg.sessionId, + error: message, + }); + } + }, + ); +} + +export function handleTerminalInput( + connectionId: string, + msg: TerminalInputMsg, + ctx: V2HandlerContext, +): void { + const ok = writeTerminal(msg.terminalId, msg.data); + if (!ok) { + const conn = ctx.connRegistry.get(connectionId); + conn?.transport.send({ + type: 'terminal_error', + terminalId: msg.terminalId, + error: 'Terminal not found', + }); + } +} + +export function handleTerminalResize( + connectionId: string, + msg: TerminalResizeMsg, + ctx: V2HandlerContext, +): void { + const ok = resizeTerminal(msg.terminalId, msg.cols, msg.rows); + if (!ok) { + const conn = ctx.connRegistry.get(connectionId); + conn?.transport.send({ + type: 'terminal_error', + terminalId: msg.terminalId, + error: 'Terminal not found', + }); + } +} + +export function handleTerminalDestroy( + connectionId: string, + msg: TerminalDestroyMsg, + ctx: V2HandlerContext, +): void { + const ok = destroyTerminal(msg.terminalId); + if (!ok) { + const conn = ctx.connRegistry.get(connectionId); + conn?.transport.send({ + type: 'terminal_error', + terminalId: msg.terminalId, + error: 'Terminal not found', + }); + } + log.info('terminal destroyed via ws', { connectionId, terminalId: msg.terminalId }); +} + // ─── Dispatcher ────────────────────────────────────────────────────────────── /** @@ -982,5 +1121,17 @@ export async function dispatchV2Message( case 'set_mode': handleSetModeV2(connectionId, msg, ctx); break; + case 'terminal_create': + handleTerminalCreate(connectionId, msg, ctx); + break; + case 'terminal_input': + handleTerminalInput(connectionId, msg, ctx); + break; + case 'terminal_resize': + handleTerminalResize(connectionId, msg, ctx); + break; + case 'terminal_destroy': + handleTerminalDestroy(connectionId, msg, ctx); + break; } } From 58292052f932b1d688e0ca481949fc9fb2385e11 Mon Sep 17 00:00:00 2001 From: dimakis Date: Thu, 25 Jun 2026 12:31:49 +0100 Subject: [PATCH 2/7] build(terminal): add node-pty dependency Co-Authored-By: Claude Opus 4.6 --- package-lock.json | 17 +++++++++++++++++ package.json | 1 + 2 files changed, 18 insertions(+) diff --git a/package-lock.json b/package-lock.json index 46b186ea..59b0c1cd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -33,6 +33,7 @@ "jose": "^5.9.0", "js-yaml": "^4.2.0", "nanoid": "^5.0.9", + "node-pty": "^1.1.0", "pino": "^10.3.1", "pino-loki": "^3.0.0", "pino-roll": "^4.0.0", @@ -9305,6 +9306,12 @@ "node": ">=10" } }, + "node_modules/node-addon-api": { + "version": "7.1.1", + "resolved": "https://registry.npmjs.org/node-addon-api/-/node-addon-api-7.1.1.tgz", + "integrity": "sha512-5m3bsyrjFWE1xf7nz7YXdN4udnVtXK6/Yfgn5qnahL6bCkf2yKt4k3nuTKAtT4r3IG8JNR2ncsIMdZuAzJjHQQ==", + "license": "MIT" + }, "node_modules/node-fetch": { "version": "2.7.0", "resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.7.0.tgz", @@ -9356,6 +9363,16 @@ "node": ">= 6.13.0" } }, + "node_modules/node-pty": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/node-pty/-/node-pty-1.1.0.tgz", + "integrity": "sha512-20JqtutY6JPXTUnL0ij1uad7Qe1baT46lyolh2sSENDd4sTzKZ4nmAFkeAARDKwmlLjPx6XKRlwRUxwjOy+lUg==", + "hasInstallScript": true, + "license": "MIT", + "dependencies": { + "node-addon-api": "^7.1.0" + } + }, "node_modules/node-releases": { "version": "2.0.37", "resolved": "https://registry.npmjs.org/node-releases/-/node-releases-2.0.37.tgz", diff --git a/package.json b/package.json index df732b66..7e1c056e 100644 --- a/package.json +++ b/package.json @@ -55,6 +55,7 @@ "jose": "^5.9.0", "js-yaml": "^4.2.0", "nanoid": "^5.0.9", + "node-pty": "^1.1.0", "pino": "^10.3.1", "pino-loki": "^3.0.0", "pino-roll": "^4.0.0", From 2c25d808dfa5613ff3139beadc275f0b4c19aee2 Mon Sep 17 00:00:00 2001 From: dimakis Date: Thu, 25 Jun 2026 13:35:56 +0100 Subject: [PATCH 3/7] =?UTF-8?q?fix(terminal):=20address=20Centaur=20review?= =?UTF-8?q?=20=E2=80=94=20limits,=20cleanup,=20validation,=20bounds?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add per-session (5) and global (50) terminal caps in createTerminal() - Wire destroySessionTerminals() into WS close handler to prevent PTY leaks - Add isAllowedPath() validation on resolved terminal cwd - Add .max(65536) bound on TerminalInputMessage.data schema - Fix handleTerminalDestroy to only log on success - Fix stale cols/rows closure in useTerminal connect deps (use refs) - Generate unique fallback sessionId instead of 'default' - Trim multi-line JSDoc comments to single-line Co-Authored-By: Claude Opus 4.6 --- frontend/src/components/Terminal.tsx | 8 +------- frontend/src/hooks/useTerminal.ts | 18 ++++++++---------- frontend/src/pages/TerminalView.tsx | 9 ++------- packages/protocol/src/ws-schemas-v2.ts | 2 +- server/index.ts | 4 ++++ server/terminal-manager.ts | 19 ++++++++++++------- server/ws-handler-v2.ts | 8 +++++--- 7 files changed, 33 insertions(+), 35 deletions(-) diff --git a/frontend/src/components/Terminal.tsx b/frontend/src/components/Terminal.tsx index 0c969982..9e42e33b 100644 --- a/frontend/src/components/Terminal.tsx +++ b/frontend/src/components/Terminal.tsx @@ -1,10 +1,4 @@ -/** - * Terminal — xterm.js wrapper with mobile keyboard toolbar. - * - * Renders a full-screen terminal emulator using xterm.js, connected to a - * server-side PTY via the useTerminal hook. Includes a touch-friendly toolbar - * with special keys (Tab, Ctrl, arrows, Esc) for mobile use. - */ +/** Terminal — xterm.js wrapper with mobile keyboard toolbar. */ import { useRef, useEffect, useCallback, useState } from 'react'; import { Terminal as XTerm } from '@xterm/xterm'; diff --git a/frontend/src/hooks/useTerminal.ts b/frontend/src/hooks/useTerminal.ts index ae188935..f05f3b5a 100644 --- a/frontend/src/hooks/useTerminal.ts +++ b/frontend/src/hooks/useTerminal.ts @@ -1,10 +1,4 @@ -/** - * useTerminal — manages a WebSocket connection to the terminal backend. - * - * Shares the same WS endpoint as chat (/ws/chat) but sends terminal-specific - * message types (terminal_create, terminal_input, terminal_resize, terminal_destroy). - * The v2 protocol dispatcher routes these to the terminal handlers. - */ +/** useTerminal — manages a WebSocket connection to the terminal backend. */ import { useRef, useCallback, useEffect, useState } from 'react'; import { getWsChatUrl } from '../lib/api-fetch'; @@ -45,9 +39,13 @@ export function useTerminal({ const onDataRef = useRef(onData); const onExitRef = useRef(onExit); const onErrorRef = useRef(onError); + const colsRef = useRef(cols); + const rowsRef = useRef(rows); onDataRef.current = onData; onExitRef.current = onExit; onErrorRef.current = onError; + colsRef.current = cols; + rowsRef.current = rows; const send = useCallback((msg: Record) => { const ws = wsRef.current; @@ -81,8 +79,8 @@ export function useTerminal({ send({ type: 'terminal_create', sessionId, - cols, - rows, + cols: colsRef.current, + rows: rowsRef.current, }); setState((s) => ({ ...s, connected: true })); break; @@ -121,7 +119,7 @@ export function useTerminal({ ws.onclose = () => { setState((s) => ({ ...s, connected: false })); }; - }, [sessionId, cols, rows, send]); + }, [sessionId, send]); const writeInput = useCallback( (data: string) => { diff --git a/frontend/src/pages/TerminalView.tsx b/frontend/src/pages/TerminalView.tsx index bab4b7b2..d0b37fe7 100644 --- a/frontend/src/pages/TerminalView.tsx +++ b/frontend/src/pages/TerminalView.tsx @@ -1,9 +1,4 @@ -/** - * TerminalView — full-page interactive shell terminal. - * - * Creates a PTY on the server scoped to the current session's worktree, - * renders it via xterm.js, and provides a mobile-friendly key toolbar. - */ +/** TerminalView — full-page interactive shell terminal. */ import { useRef, useEffect, useCallback } from 'react'; import { useNavigate, useParams } from 'react-router-dom'; @@ -15,7 +10,7 @@ export function TerminalView() { const navigate = useNavigate(); const termRef = useRef<{ write: (data: string) => void } | null>(null); - const resolvedSessionId = sessionId || 'default'; + const resolvedSessionId = sessionId || `terminal-${Date.now()}`; const { state, connect, writeInput, resize, destroy } = useTerminal({ sessionId: resolvedSessionId, diff --git a/packages/protocol/src/ws-schemas-v2.ts b/packages/protocol/src/ws-schemas-v2.ts index 7e635ac8..53b5acc1 100644 --- a/packages/protocol/src/ws-schemas-v2.ts +++ b/packages/protocol/src/ws-schemas-v2.ts @@ -134,7 +134,7 @@ export const TerminalCreateMessage = z.object({ export const TerminalInputMessage = z.object({ type: z.literal('terminal_input'), terminalId: z.string().min(1), - data: z.string(), + data: z.string().max(65536), }); export const TerminalResizeMessage = z.object({ diff --git a/server/index.ts b/server/index.ts index 8cb4640d..41b32c3e 100644 --- a/server/index.ts +++ b/server/index.ts @@ -91,6 +91,7 @@ import { withSpan, withSpanAsync } from './tracing.js'; import { contextFromTraceparent } from './trace-context.js'; import { SseTransport } from './sse-transport.js'; import { createChatRestRouter } from './chat-rest-handler.js'; +import { destroySessionTerminals } from './terminal-manager.js'; const log = createLogger('server'); @@ -512,6 +513,9 @@ function handleChatWsV2(ws: WebSocket, connectionId: string) { log.info('v2 disconnected', { connectionId, code, reason: reason?.toString() }); for (const sessionId of watchedSessions) { + // Clean up any PTY terminals for this session + destroySessionTerminals(sessionId); + const found = registry.findBySessionId(sessionId); if (!found) continue; diff --git a/server/terminal-manager.ts b/server/terminal-manager.ts index c7f5384d..7647c33c 100644 --- a/server/terminal-manager.ts +++ b/server/terminal-manager.ts @@ -1,10 +1,4 @@ -/** - * Terminal Manager — PTY lifecycle management for interactive shell terminals. - * - * Spawns node-pty processes, routes I/O to WebSocket connections, and handles - * resize/cleanup. Each terminal is scoped to a session and inherits the - * session's worktree cwd. - */ +/** Terminal Manager — PTY lifecycle for interactive shell terminals. */ import * as pty from 'node-pty'; import { createLogger } from './logger.js'; @@ -35,6 +29,9 @@ interface ManagedTerminal { onExit: ((exitCode: number, signal?: number) => void) | null; } +const MAX_TERMINALS_PER_SESSION = 5; +const MAX_TERMINALS_GLOBAL = 50; + let terminalCounter = 0; /** Active terminals keyed by terminal ID. */ @@ -53,6 +50,14 @@ export function createTerminal( cwd: string, opts?: { cols?: number; rows?: number; env?: Record }, ): TerminalInfo { + if (terminals.size >= MAX_TERMINALS_GLOBAL) { + throw new Error(`Global terminal limit reached (${MAX_TERMINALS_GLOBAL})`); + } + const sessionCount = [...terminals.values()].filter((t) => t.sessionId === sessionId).length; + if (sessionCount >= MAX_TERMINALS_PER_SESSION) { + throw new Error(`Session terminal limit reached (${MAX_TERMINALS_PER_SESSION})`); + } + const id = generateTerminalId(); const cols = opts?.cols ?? 80; const rows = opts?.rows ?? 24; diff --git a/server/ws-handler-v2.ts b/server/ws-handler-v2.ts index 821a7b14..fefe0b3b 100644 --- a/server/ws-handler-v2.ts +++ b/server/ws-handler-v2.ts @@ -945,7 +945,8 @@ export function handleTerminalCreate( // Resolve cwd from session metadata (worktree path or base repo) const sessionMeta = ctx.eventStore.getSession(msg.sessionId); - const cwd = sessionMeta?.cwd || BASE_REPO || process.cwd(); + const rawCwd = sessionMeta?.cwd || BASE_REPO || process.cwd(); + const cwd = isAllowedPath(rawCwd) ? rawCwd : BASE_REPO || process.cwd(); try { const info = createTerminal(msg.sessionId, cwd, { @@ -1042,7 +1043,9 @@ export function handleTerminalDestroy( ctx: V2HandlerContext, ): void { const ok = destroyTerminal(msg.terminalId); - if (!ok) { + if (ok) { + log.info('terminal destroyed via ws', { connectionId, terminalId: msg.terminalId }); + } else { const conn = ctx.connRegistry.get(connectionId); conn?.transport.send({ type: 'terminal_error', @@ -1050,7 +1053,6 @@ export function handleTerminalDestroy( error: 'Terminal not found', }); } - log.info('terminal destroyed via ws', { connectionId, terminalId: msg.terminalId }); } // ─── Dispatcher ────────────────────────────────────────────────────────────── From 4a5170ecf81e565e0b5eea8b68f73b127ba14f40 Mon Sep 17 00:00:00 2001 From: dimakis Date: Thu, 25 Jun 2026 13:52:52 +0100 Subject: [PATCH 4/7] =?UTF-8?q?fix(terminal):=20address=20second=20Centaur?= =?UTF-8?q?=20review=20=E2=80=94=20ownership,=20env,=20bounds?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Track terminals by connectionId for cleanup on WS disconnect - Add destroyConnectionTerminals() — fixes PTY leak for terminal-only WS - Whitelist safe env vars (PATH, HOME, etc.) instead of inheriting all - Add ownership verification on terminal input/resize/destroy operations - Add .max(500) bound on cols/rows in create and resize schemas - Stabilize fallback sessionId with useMemo to prevent re-render churn Co-Authored-By: Claude Opus 4.6 --- frontend/src/pages/TerminalView.tsx | 4 +- packages/protocol/src/ws-schemas-v2.ts | 8 ++-- server/index.ts | 8 ++-- server/terminal-manager.ts | 61 ++++++++++++++++++++++--- server/ws-handler-v2.ts | 62 ++++++++++++++------------ 5 files changed, 98 insertions(+), 45 deletions(-) diff --git a/frontend/src/pages/TerminalView.tsx b/frontend/src/pages/TerminalView.tsx index d0b37fe7..63503a2b 100644 --- a/frontend/src/pages/TerminalView.tsx +++ b/frontend/src/pages/TerminalView.tsx @@ -1,6 +1,6 @@ /** TerminalView — full-page interactive shell terminal. */ -import { useRef, useEffect, useCallback } from 'react'; +import { useRef, useEffect, useCallback, useMemo } from 'react'; import { useNavigate, useParams } from 'react-router-dom'; import { Terminal } from '../components/Terminal'; import { useTerminal } from '../hooks/useTerminal'; @@ -10,7 +10,7 @@ export function TerminalView() { const navigate = useNavigate(); const termRef = useRef<{ write: (data: string) => void } | null>(null); - const resolvedSessionId = sessionId || `terminal-${Date.now()}`; + const resolvedSessionId = useMemo(() => sessionId || `terminal-${Date.now()}`, [sessionId]); const { state, connect, writeInput, resize, destroy } = useTerminal({ sessionId: resolvedSessionId, diff --git a/packages/protocol/src/ws-schemas-v2.ts b/packages/protocol/src/ws-schemas-v2.ts index 53b5acc1..6d1b0c96 100644 --- a/packages/protocol/src/ws-schemas-v2.ts +++ b/packages/protocol/src/ws-schemas-v2.ts @@ -127,8 +127,8 @@ export const V2SetModeMessage = z.object({ export const TerminalCreateMessage = z.object({ type: z.literal('terminal_create'), sessionId: z.string().min(1), - cols: z.number().int().min(1).optional(), - rows: z.number().int().min(1).optional(), + cols: z.number().int().min(1).max(500).optional(), + rows: z.number().int().min(1).max(500).optional(), }); export const TerminalInputMessage = z.object({ @@ -140,8 +140,8 @@ export const TerminalInputMessage = z.object({ export const TerminalResizeMessage = z.object({ type: z.literal('terminal_resize'), terminalId: z.string().min(1), - cols: z.number().int().min(1), - rows: z.number().int().min(1), + cols: z.number().int().min(1).max(500), + rows: z.number().int().min(1).max(500), }); export const TerminalDestroyMessage = z.object({ diff --git a/server/index.ts b/server/index.ts index 41b32c3e..86d71893 100644 --- a/server/index.ts +++ b/server/index.ts @@ -91,7 +91,7 @@ import { withSpan, withSpanAsync } from './tracing.js'; import { contextFromTraceparent } from './trace-context.js'; import { SseTransport } from './sse-transport.js'; import { createChatRestRouter } from './chat-rest-handler.js'; -import { destroySessionTerminals } from './terminal-manager.js'; +import { destroyConnectionTerminals } from './terminal-manager.js'; const log = createLogger('server'); @@ -512,10 +512,10 @@ function handleChatWsV2(ws: WebSocket, connectionId: string) { transportMap.delete(ws); log.info('v2 disconnected', { connectionId, code, reason: reason?.toString() }); - for (const sessionId of watchedSessions) { - // Clean up any PTY terminals for this session - destroySessionTerminals(sessionId); + // Clean up any PTY terminals owned by this connection + destroyConnectionTerminals(connectionId); + for (const sessionId of watchedSessions) { const found = registry.findBySessionId(sessionId); if (!found) continue; diff --git a/server/terminal-manager.ts b/server/terminal-manager.ts index 7647c33c..0d0ce11d 100644 --- a/server/terminal-manager.ts +++ b/server/terminal-manager.ts @@ -18,6 +18,7 @@ export interface TerminalInfo { interface ManagedTerminal { id: string; sessionId: string; + connectionId: string; process: pty.IPty; cols: number; rows: number; @@ -29,6 +30,24 @@ interface ManagedTerminal { onExit: ((exitCode: number, signal?: number) => void) | null; } +/** Safe env vars to inherit — everything else is stripped. */ +const SAFE_ENV_KEYS = new Set([ + 'PATH', + 'HOME', + 'USER', + 'LOGNAME', + 'SHELL', + 'LANG', + 'TERM', + 'COLORTERM', + 'EDITOR', + 'VISUAL', + 'PAGER', + 'TMPDIR', + 'TZ', +]); +const SAFE_ENV_PREFIXES = ['LC_', 'XDG_']; + const MAX_TERMINALS_PER_SESSION = 5; const MAX_TERMINALS_GLOBAL = 50; @@ -45,8 +64,23 @@ function getDefaultShell(): string { return process.env.SHELL || (process.platform === 'win32' ? 'powershell.exe' : '/bin/zsh'); } +function buildSafeEnv(extra?: Record): Record { + const env: Record = {}; + for (const [key, val] of Object.entries(process.env)) { + if (val == null) continue; + if (SAFE_ENV_KEYS.has(key) || SAFE_ENV_PREFIXES.some((p) => key.startsWith(p))) { + env[key] = val; + } + } + env.TERM = 'xterm-256color'; + env.COLORTERM = 'truecolor'; + if (extra) Object.assign(env, extra); + return env; +} + export function createTerminal( sessionId: string, + connectionId: string, cwd: string, opts?: { cols?: number; rows?: number; env?: Record }, ): TerminalInfo { @@ -68,17 +102,13 @@ export function createTerminal( cols, rows, cwd, - env: { - ...process.env, - TERM: 'xterm-256color', - COLORTERM: 'truecolor', - ...opts?.env, - } as Record, + env: buildSafeEnv(opts?.env), }); const managed: ManagedTerminal = { id, sessionId, + connectionId, process: proc, cols, rows, @@ -175,6 +205,25 @@ export function destroySessionTerminals(sessionId: string): number { return count; } +export function destroyConnectionTerminals(connectionId: string): number { + let count = 0; + for (const [id, term] of terminals.entries()) { + if (term.connectionId === connectionId) { + term.process.kill(); + terminals.delete(id); + count++; + } + } + if (count > 0) { + log.info('destroyed connection terminals', { connectionId, count }); + } + return count; +} + +export function getTerminalOwner(id: string): string | null { + return terminals.get(id)?.connectionId ?? null; +} + export function setTerminalCallbacks( id: string, onData: (data: string) => void, diff --git a/server/ws-handler-v2.ts b/server/ws-handler-v2.ts index fefe0b3b..a7c28428 100644 --- a/server/ws-handler-v2.ts +++ b/server/ws-handler-v2.ts @@ -71,6 +71,7 @@ import { resizeTerminal, destroyTerminal, setTerminalCallbacks, + getTerminalOwner, } from './terminal-manager.js'; import { createLogger } from './logger.js'; @@ -949,7 +950,7 @@ export function handleTerminalCreate( const cwd = isAllowedPath(rawCwd) ? rawCwd : BASE_REPO || process.cwd(); try { - const info = createTerminal(msg.sessionId, cwd, { + const info = createTerminal(msg.sessionId, connectionId, cwd, { cols: msg.cols, rows: msg.rows, }); @@ -1005,20 +1006,38 @@ export function handleTerminalCreate( ); } -export function handleTerminalInput( +function verifyTerminalOwner( connectionId: string, - msg: TerminalInputMsg, + terminalId: string, ctx: V2HandlerContext, -): void { - const ok = writeTerminal(msg.terminalId, msg.data); - if (!ok) { - const conn = ctx.connRegistry.get(connectionId); - conn?.transport.send({ +): boolean { + const owner = getTerminalOwner(terminalId); + if (owner === null) { + ctx.connRegistry.get(connectionId)?.transport.send({ type: 'terminal_error', - terminalId: msg.terminalId, + terminalId, error: 'Terminal not found', }); + return false; + } + if (owner !== connectionId) { + ctx.connRegistry.get(connectionId)?.transport.send({ + type: 'terminal_error', + terminalId, + error: 'Not terminal owner', + }); + return false; } + return true; +} + +export function handleTerminalInput( + connectionId: string, + msg: TerminalInputMsg, + ctx: V2HandlerContext, +): void { + if (!verifyTerminalOwner(connectionId, msg.terminalId, ctx)) return; + writeTerminal(msg.terminalId, msg.data); } export function handleTerminalResize( @@ -1026,15 +1045,8 @@ export function handleTerminalResize( msg: TerminalResizeMsg, ctx: V2HandlerContext, ): void { - const ok = resizeTerminal(msg.terminalId, msg.cols, msg.rows); - if (!ok) { - const conn = ctx.connRegistry.get(connectionId); - conn?.transport.send({ - type: 'terminal_error', - terminalId: msg.terminalId, - error: 'Terminal not found', - }); - } + if (!verifyTerminalOwner(connectionId, msg.terminalId, ctx)) return; + resizeTerminal(msg.terminalId, msg.cols, msg.rows); } export function handleTerminalDestroy( @@ -1042,17 +1054,9 @@ export function handleTerminalDestroy( msg: TerminalDestroyMsg, ctx: V2HandlerContext, ): void { - const ok = destroyTerminal(msg.terminalId); - if (ok) { - log.info('terminal destroyed via ws', { connectionId, terminalId: msg.terminalId }); - } else { - const conn = ctx.connRegistry.get(connectionId); - conn?.transport.send({ - type: 'terminal_error', - terminalId: msg.terminalId, - error: 'Terminal not found', - }); - } + if (!verifyTerminalOwner(connectionId, msg.terminalId, ctx)) return; + destroyTerminal(msg.terminalId); + log.info('terminal destroyed via ws', { connectionId, terminalId: msg.terminalId }); } // ─── Dispatcher ────────────────────────────────────────────────────────────── From 56a896a23fc740cd7fe941dffc9adf4dfc652a05 Mon Sep 17 00:00:00 2001 From: dimakis Date: Thu, 2 Jul 2026 23:27:21 +0100 Subject: [PATCH 5/7] fix(terminal): address all Centaur review findings - Fix PTY output race: accept onData/onExit callbacks in createTerminal opts so they're wired before shell output fires - Add try-catch around process.kill() in all three destroy functions to handle already-exited processes without skipping cleanup - Guard useTerminal connect() against CONNECTING state to prevent orphaned WebSockets on React strict-mode double-mount - Reject terminal creation when cwd is unresolvable instead of silently falling back to process.cwd() - Use captured ws variable instead of wsRef in welcome handler to avoid stale reference if connect() is called again before welcome arrives - Remove unused setTerminalCallbacks import from ws-handler-v2 - Add comprehensive tests for terminal-manager (30 tests) and terminal WS handlers (17 tests) covering lifecycle, limits, ownership, env sanitization, error paths, and dispatch routing Co-Authored-By: Claude Opus 4.6 --- frontend/src/hooks/useTerminal.ts | 21 +- server/__tests__/terminal-manager.test.ts | 403 ++++++++++++++++++++++ server/__tests__/ws-handler-v2.test.ts | 297 +++++++++++++++- server/terminal-manager.ts | 52 ++- server/ws-handler-v2.ts | 30 +- 5 files changed, 768 insertions(+), 35 deletions(-) create mode 100644 server/__tests__/terminal-manager.test.ts diff --git a/frontend/src/hooks/useTerminal.ts b/frontend/src/hooks/useTerminal.ts index f05f3b5a..4635dac8 100644 --- a/frontend/src/hooks/useTerminal.ts +++ b/frontend/src/hooks/useTerminal.ts @@ -55,7 +55,10 @@ export function useTerminal({ }, []); const connect = useCallback(() => { - if (wsRef.current?.readyState === WebSocket.OPEN) return; + const existing = wsRef.current; + if (existing?.readyState === WebSocket.OPEN || existing?.readyState === WebSocket.CONNECTING) { + return; + } const ws = new WebSocket(getWsChatUrl()); wsRef.current = ws; @@ -75,13 +78,15 @@ export function useTerminal({ switch (msg.type) { case 'welcome': - // Handshake complete — create terminal - send({ - type: 'terminal_create', - sessionId, - cols: colsRef.current, - rows: rowsRef.current, - }); + // Handshake complete — create terminal (use captured ws, not wsRef) + ws.send( + JSON.stringify({ + type: 'terminal_create', + sessionId, + cols: colsRef.current, + rows: rowsRef.current, + }), + ); setState((s) => ({ ...s, connected: true })); break; diff --git a/server/__tests__/terminal-manager.test.ts b/server/__tests__/terminal-manager.test.ts new file mode 100644 index 00000000..494b2f8f --- /dev/null +++ b/server/__tests__/terminal-manager.test.ts @@ -0,0 +1,403 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; + +// Mock node-pty before importing terminal-manager +const mockPtyProcess = { + pid: 12345, + onData: vi.fn(), + onExit: vi.fn(), + write: vi.fn(), + resize: vi.fn(), + kill: vi.fn(), +}; + +vi.mock('node-pty', () => ({ + spawn: vi.fn(() => ({ ...mockPtyProcess })), +})); + +import * as pty from 'node-pty'; +import { + createTerminal, + writeTerminal, + resizeTerminal, + destroyTerminal, + destroySessionTerminals, + destroyConnectionTerminals, + getTerminal, + getTerminalOwner, + listTerminals, + setTerminalCallbacks, + clearTerminalCallbacks, +} from '../terminal-manager.js'; + +// Track created terminal IDs for cleanup +let createdIds: string[] = []; + +function createTestTerminal( + sessionId = 'sess-1', + connectionId = 'conn-1', + cwd = '/tmp/test', + opts?: Parameters[3], +) { + const info = createTerminal(sessionId, connectionId, cwd, opts); + createdIds.push(info.id); + return info; +} + +beforeEach(() => { + vi.clearAllMocks(); + createdIds = []; + // Reset mock to return fresh objects each call + vi.mocked(pty.spawn).mockImplementation( + () => + ({ + ...mockPtyProcess, + onData: vi.fn(), + onExit: vi.fn(), + write: vi.fn(), + resize: vi.fn(), + kill: vi.fn(), + }) as unknown as pty.IPty, + ); +}); + +afterEach(() => { + // Clean up all terminals created during the test + for (const id of createdIds) { + try { + destroyTerminal(id); + } catch { + // already destroyed + } + } + createdIds = []; +}); + +// ─── createTerminal ───────────────────────────────────────────────────────── + +describe('createTerminal', () => { + it('returns terminal info with id, pid, dimensions, cwd', () => { + const info = createTestTerminal('sess-1', 'conn-1', '/tmp/test'); + + expect(info.id).toMatch(/^term-\d+-\d+$/); + expect(info.sessionId).toBe('sess-1'); + expect(info.pid).toBe(12345); + expect(info.cols).toBe(80); + expect(info.rows).toBe(24); + expect(info.cwd).toBe('/tmp/test'); + expect(info.createdAt).toBeGreaterThan(0); + }); + + it('respects custom cols and rows', () => { + const info = createTestTerminal('sess-1', 'conn-1', '/tmp', { cols: 120, rows: 40 }); + + expect(info.cols).toBe(120); + expect(info.rows).toBe(40); + expect(pty.spawn).toHaveBeenCalledWith( + expect.any(String), + [], + expect.objectContaining({ cols: 120, rows: 40 }), + ); + }); + + it('spawns with safe environment (no process.env leak)', () => { + // Set a dangerous env var + process.env.ANTHROPIC_API_KEY = 'secret-key-123'; + createTestTerminal(); + + const spawnCall = vi.mocked(pty.spawn).mock.calls[0]; + const env = spawnCall[2].env as Record; + + expect(env.ANTHROPIC_API_KEY).toBeUndefined(); + expect(env.TERM).toBe('xterm-256color'); + expect(env.COLORTERM).toBe('truecolor'); + + delete process.env.ANTHROPIC_API_KEY; + }); + + it('passes safe env vars through (PATH, HOME, LANG, LC_*)', () => { + const origPath = process.env.PATH; + const origHome = process.env.HOME; + process.env.LC_ALL = 'en_US.UTF-8'; + + createTestTerminal(); + + const spawnCall = vi.mocked(pty.spawn).mock.calls[0]; + const env = spawnCall[2].env as Record; + + expect(env.PATH).toBe(origPath); + expect(env.HOME).toBe(origHome); + expect(env.LC_ALL).toBe('en_US.UTF-8'); + }); + + it('enforces per-session limit (5)', () => { + for (let i = 0; i < 5; i++) { + createTestTerminal('sess-limit', `conn-${i}`, '/tmp'); + } + + expect(() => createTestTerminal('sess-limit', 'conn-6', '/tmp')).toThrow( + 'Session terminal limit reached (5)', + ); + }); + + it('enforces global limit (50)', () => { + // Create 50 terminals across different sessions to avoid per-session limit + for (let i = 0; i < 50; i++) { + createTestTerminal(`sess-global-${i}`, `conn-${i}`, '/tmp'); + } + + expect(() => createTestTerminal('sess-new', 'conn-new', '/tmp')).toThrow( + 'Global terminal limit reached (50)', + ); + }); + + it('accepts onData/onExit callbacks and wires them before spawn output', () => { + const onData = vi.fn(); + const onExit = vi.fn(); + + const info = createTestTerminal('sess-1', 'conn-1', '/tmp', { onData, onExit }); + + // The managed terminal should have callbacks set before proc.onData fires + // Verify by simulating PTY output via the onData handler registered on the mock + const spawnResult = vi.mocked(pty.spawn).mock.results[0].value; + const registeredOnData = spawnResult.onData.mock.calls[0][0]; + registeredOnData('hello from shell'); + + expect(onData).toHaveBeenCalledWith('hello from shell'); + + // Simulate exit + const registeredOnExit = spawnResult.onExit.mock.calls[0][0]; + registeredOnExit({ exitCode: 0, signal: 15 }); + + expect(onExit).toHaveBeenCalledWith(0, 15); + expect(info.id).toBeTruthy(); + }); +}); + +// ─── writeTerminal ────────────────────────────────────────────────────────── + +describe('writeTerminal', () => { + it('writes data to the PTY process', () => { + const info = createTestTerminal(); + const spawnResult = vi.mocked(pty.spawn).mock.results[0].value; + + expect(writeTerminal(info.id, 'ls -la\n')).toBe(true); + expect(spawnResult.write).toHaveBeenCalledWith('ls -la\n'); + }); + + it('returns false for unknown terminal ID', () => { + expect(writeTerminal('nonexistent', 'data')).toBe(false); + }); +}); + +// ─── resizeTerminal ───────────────────────────────────────────────────────── + +describe('resizeTerminal', () => { + it('resizes the PTY process', () => { + const info = createTestTerminal(); + const spawnResult = vi.mocked(pty.spawn).mock.results[0].value; + + expect(resizeTerminal(info.id, 200, 50)).toBe(true); + expect(spawnResult.resize).toHaveBeenCalledWith(200, 50); + }); + + it('returns false for unknown terminal ID', () => { + expect(resizeTerminal('nonexistent', 80, 24)).toBe(false); + }); +}); + +// ─── destroyTerminal ──────────────────────────────────────────────────────── + +describe('destroyTerminal', () => { + it('kills the process and removes from map', () => { + const info = createTestTerminal(); + const spawnResult = vi.mocked(pty.spawn).mock.results[0].value; + + expect(destroyTerminal(info.id)).toBe(true); + expect(spawnResult.kill).toHaveBeenCalled(); + expect(getTerminal(info.id)).toBeNull(); + // Remove from cleanup list since already destroyed + createdIds = createdIds.filter((id) => id !== info.id); + }); + + it('returns false for unknown terminal ID', () => { + expect(destroyTerminal('nonexistent')).toBe(false); + }); + + it('handles kill() throwing (process already exited)', () => { + const info = createTestTerminal(); + const spawnResult = vi.mocked(pty.spawn).mock.results[0].value; + spawnResult.kill.mockImplementation(() => { + throw new Error('Process already dead'); + }); + + // Should not throw — try-catch handles it + expect(destroyTerminal(info.id)).toBe(true); + expect(getTerminal(info.id)).toBeNull(); + createdIds = createdIds.filter((id) => id !== info.id); + }); +}); + +// ─── destroySessionTerminals ──────────────────────────────────────────────── + +describe('destroySessionTerminals', () => { + it('destroys all terminals for a session', () => { + createTestTerminal('sess-a', 'conn-1', '/tmp'); + createTestTerminal('sess-a', 'conn-2', '/tmp'); + createTestTerminal('sess-b', 'conn-3', '/tmp'); + + const count = destroySessionTerminals('sess-a'); + + expect(count).toBe(2); + expect(listTerminals('sess-a')).toHaveLength(0); + expect(listTerminals('sess-b')).toHaveLength(1); + // Update cleanup list + createdIds = listTerminals().map((t) => t.id); + }); + + it('returns 0 for unknown session', () => { + expect(destroySessionTerminals('nonexistent')).toBe(0); + }); + + it('continues cleanup even if kill() throws on one terminal', () => { + createTestTerminal('sess-err', 'conn-1', '/tmp'); + createTestTerminal('sess-err', 'conn-2', '/tmp'); + + // Make the first spawned process throw on kill + const firstResult = vi.mocked(pty.spawn).mock.results[0].value; + firstResult.kill.mockImplementation(() => { + throw new Error('Process already dead'); + }); + + const count = destroySessionTerminals('sess-err'); + expect(count).toBe(2); + expect(listTerminals('sess-err')).toHaveLength(0); + createdIds = []; + }); +}); + +// ─── destroyConnectionTerminals ───────────────────────────────────────────── + +describe('destroyConnectionTerminals', () => { + it('destroys all terminals for a connection', () => { + createTestTerminal('sess-1', 'conn-target', '/tmp'); + createTestTerminal('sess-2', 'conn-target', '/tmp'); + createTestTerminal('sess-1', 'conn-other', '/tmp'); + + const count = destroyConnectionTerminals('conn-target'); + + expect(count).toBe(2); + expect(listTerminals()).toHaveLength(1); + createdIds = listTerminals().map((t) => t.id); + }); + + it('returns 0 for unknown connection', () => { + expect(destroyConnectionTerminals('nonexistent')).toBe(0); + }); + + it('continues cleanup even if kill() throws', () => { + createTestTerminal('sess-1', 'conn-err', '/tmp'); + createTestTerminal('sess-2', 'conn-err', '/tmp'); + + const firstResult = vi.mocked(pty.spawn).mock.results[0].value; + firstResult.kill.mockImplementation(() => { + throw new Error('gone'); + }); + + const count = destroyConnectionTerminals('conn-err'); + expect(count).toBe(2); + createdIds = []; + }); +}); + +// ─── getTerminal / getTerminalOwner / listTerminals ───────────────────────── + +describe('getTerminal', () => { + it('returns terminal info for valid ID', () => { + const info = createTestTerminal('sess-1', 'conn-1', '/tmp/cwd'); + const retrieved = getTerminal(info.id); + + expect(retrieved).not.toBeNull(); + expect(retrieved!.id).toBe(info.id); + expect(retrieved!.sessionId).toBe('sess-1'); + expect(retrieved!.cwd).toBe('/tmp/cwd'); + }); + + it('returns null for unknown ID', () => { + expect(getTerminal('nonexistent')).toBeNull(); + }); +}); + +describe('getTerminalOwner', () => { + it('returns connectionId for valid terminal', () => { + const info = createTestTerminal('sess-1', 'conn-owner', '/tmp'); + expect(getTerminalOwner(info.id)).toBe('conn-owner'); + }); + + it('returns null for unknown terminal', () => { + expect(getTerminalOwner('nonexistent')).toBeNull(); + }); +}); + +describe('listTerminals', () => { + it('lists all terminals when no filter', () => { + createTestTerminal('sess-1', 'conn-1', '/tmp'); + createTestTerminal('sess-2', 'conn-2', '/tmp'); + + expect(listTerminals()).toHaveLength(2); + }); + + it('filters by sessionId', () => { + createTestTerminal('sess-a', 'conn-1', '/tmp'); + createTestTerminal('sess-a', 'conn-2', '/tmp'); + createTestTerminal('sess-b', 'conn-3', '/tmp'); + + expect(listTerminals('sess-a')).toHaveLength(2); + expect(listTerminals('sess-b')).toHaveLength(1); + expect(listTerminals('sess-c')).toHaveLength(0); + }); +}); + +// ─── setTerminalCallbacks / clearTerminalCallbacks ────────────────────────── + +describe('setTerminalCallbacks', () => { + it('sets callbacks on existing terminal', () => { + const info = createTestTerminal(); + const onData = vi.fn(); + const onExit = vi.fn(); + + expect(setTerminalCallbacks(info.id, onData, onExit)).toBe(true); + + // Trigger the PTY onData handler to verify callback is wired + const spawnResult = vi.mocked(pty.spawn).mock.results[0].value; + const registeredOnData = spawnResult.onData.mock.calls[0][0]; + registeredOnData('test output'); + + expect(onData).toHaveBeenCalledWith('test output'); + }); + + it('returns false for unknown terminal', () => { + expect(setTerminalCallbacks('nonexistent', vi.fn(), vi.fn())).toBe(false); + }); +}); + +describe('clearTerminalCallbacks', () => { + it('clears callbacks on existing terminal', () => { + const onData = vi.fn(); + const info = createTestTerminal('sess-1', 'conn-1', '/tmp', { onData }); + + expect(clearTerminalCallbacks(info.id)).toBe(true); + + // Output after clearing should not reach the callback + const spawnResult = vi.mocked(pty.spawn).mock.results[0].value; + const registeredOnData = spawnResult.onData.mock.calls[0][0]; + registeredOnData('after clear'); + + // onData was called once during creation output, but clearTerminalCallbacks nulls it + // The managed.onData?.() will be a no-op since onData is null + expect(onData).not.toHaveBeenCalledWith('after clear'); + }); + + it('returns false for unknown terminal', () => { + expect(clearTerminalCallbacks('nonexistent')).toBe(false); + }); +}); diff --git a/server/__tests__/ws-handler-v2.test.ts b/server/__tests__/ws-handler-v2.test.ts index bad44d60..3465622f 100644 --- a/server/__tests__/ws-handler-v2.test.ts +++ b/server/__tests__/ws-handler-v2.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; import type { SessionTransport } from '@mitzo/harness'; import { ConnectionRegistry } from '@mitzo/harness'; import { V2SendMessage } from '@mitzo/protocol'; @@ -35,6 +35,23 @@ vi.mock('../permissions.js', () => ({ denyPendingBySession: vi.fn().mockReturnValue(0), })); +vi.mock('../terminal-manager.js', () => ({ + createTerminal: vi.fn().mockReturnValue({ + id: 'term-mock-1', + sessionId: 'sess-1', + pid: 99999, + cols: 80, + rows: 24, + cwd: '/tmp/test-repo', + createdAt: Date.now(), + }), + writeTerminal: vi.fn().mockReturnValue(true), + resizeTerminal: vi.fn().mockReturnValue(true), + destroyTerminal: vi.fn().mockReturnValue(true), + setTerminalCallbacks: vi.fn().mockReturnValue(true), + getTerminalOwner: vi.fn().mockReturnValue('conn-1'), +})); + import { startChat, interruptChat, @@ -48,6 +65,13 @@ import { import { setSkillPolicy, clearSkillPolicy } from '../skill-policy.js'; import { resolveSlashCommand } from '../slash-commands.js'; import { denyPendingBySession } from '../permissions.js'; +import { + createTerminal, + writeTerminal, + resizeTerminal, + destroyTerminal, + getTerminalOwner, +} from '../terminal-manager.js'; import { handleHello, @@ -61,6 +85,10 @@ import { handleStopV2, handlePermissionResponseV2, handleSessionSuspend, + handleTerminalCreate, + handleTerminalInput, + handleTerminalResize, + handleTerminalDestroy, isHelloHandshake, dispatchV2Message, getOwnerConnection, @@ -3340,3 +3368,270 @@ describe('detectStateMismatch', () => { expect(result.mismatch).toBe(false); }); }); + +// ─── Terminal handlers ────────────────────────────────────────────────────── + +function resetTerminalMocks() { + vi.mocked(createTerminal).mockClear(); + vi.mocked(writeTerminal).mockClear(); + vi.mocked(resizeTerminal).mockClear(); + vi.mocked(destroyTerminal).mockClear(); + vi.mocked(getTerminalOwner).mockClear(); + vi.mocked(createTerminal).mockReturnValue({ + id: 'term-mock-1', + sessionId: 'sess-1', + pid: 99999, + cols: 80, + rows: 24, + cwd: '/tmp/test-repo', + createdAt: Date.now(), + }); + vi.mocked(writeTerminal).mockReturnValue(true); + vi.mocked(resizeTerminal).mockReturnValue(true); + vi.mocked(destroyTerminal).mockReturnValue(true); + vi.mocked(getTerminalOwner).mockReturnValue('conn-1'); +} + +describe('handleTerminalCreate', () => { + beforeEach(resetTerminalMocks); + it('creates a terminal and sends terminal_created response', () => { + const ctx = createContext(); + const transport = mockTransport(); + const connId = handleHello('conn-term', transport, ctx); + + handleTerminalCreate( + connId, + { type: 'terminal_create' as const, sessionId: 'sess-1', cols: 100, rows: 40 }, + ctx, + ); + + expect(createTerminal).toHaveBeenCalledWith('sess-1', connId, expect.any(String), { + cols: 100, + rows: 40, + onData: expect.any(Function), + onExit: expect.any(Function), + }); + + const created = transport.sent.find((m) => m.type === 'terminal_created'); + expect(created).toBeDefined(); + expect(created!.terminalId).toBe('term-mock-1'); + expect(created!.pid).toBe(99999); + }); + + it('sends terminal_error when createTerminal throws', () => { + const ctx = createContext(); + const transport = mockTransport(); + const connId = handleHello('conn-err', transport, ctx); + + vi.mocked(createTerminal).mockImplementationOnce(() => { + throw new Error('Session terminal limit reached (5)'); + }); + + handleTerminalCreate(connId, { type: 'terminal_create' as const, sessionId: 'sess-1' }, ctx); + + const error = transport.sent.find((m) => m.type === 'terminal_error'); + expect(error).toBeDefined(); + expect(error!.error).toContain('Session terminal limit reached'); + }); + + it('resolves cwd from session metadata when available', () => { + const eventStore = mockEventStore(); + eventStore.getSession.mockReturnValue({ cwd: '/tmp/worktree-cwd' }); + const ctx = createContext({ + eventStore: eventStore as unknown as V2HandlerContext['eventStore'], + }); + const transport = mockTransport(); + const connId = handleHello('conn-cwd', transport, ctx); + + handleTerminalCreate(connId, { type: 'terminal_create' as const, sessionId: 'sess-1' }, ctx); + + expect(createTerminal).toHaveBeenCalledWith( + 'sess-1', + connId, + '/tmp/worktree-cwd', + expect.any(Object), + ); + }); +}); + +describe('handleTerminalInput', () => { + beforeEach(resetTerminalMocks); + + it('writes input to terminal after ownership check', () => { + const ctx = createContext(); + const transport = mockTransport(); + const connId = handleHello('conn-1', transport, ctx); + + vi.mocked(getTerminalOwner).mockReturnValue(connId); + + handleTerminalInput( + connId, + { type: 'terminal_input' as const, terminalId: 'term-mock-1', data: 'ls\n' }, + ctx, + ); + + expect(writeTerminal).toHaveBeenCalledWith('term-mock-1', 'ls\n'); + }); + + it('rejects input from non-owner connection', () => { + const ctx = createContext(); + const transport = mockTransport(); + const connId = handleHello('conn-intruder', transport, ctx); + + vi.mocked(getTerminalOwner).mockReturnValue('conn-real-owner'); + + handleTerminalInput( + connId, + { type: 'terminal_input' as const, terminalId: 'term-mock-1', data: 'rm -rf /\n' }, + ctx, + ); + + expect(writeTerminal).not.toHaveBeenCalled(); + const error = transport.sent.find((m) => m.type === 'terminal_error'); + expect(error).toBeDefined(); + expect(error!.error).toBe('Not terminal owner'); + }); + + it('sends error for unknown terminal', () => { + const ctx = createContext(); + const transport = mockTransport(); + const connId = handleHello('conn-1', transport, ctx); + + vi.mocked(getTerminalOwner).mockReturnValue(null); + + handleTerminalInput( + connId, + { type: 'terminal_input' as const, terminalId: 'term-gone', data: 'x' }, + ctx, + ); + + expect(writeTerminal).not.toHaveBeenCalled(); + const error = transport.sent.find((m) => m.type === 'terminal_error'); + expect(error!.error).toBe('Terminal not found'); + }); +}); + +describe('handleTerminalResize', () => { + beforeEach(resetTerminalMocks); + + it('resizes terminal after ownership check', () => { + const ctx = createContext(); + const transport = mockTransport(); + const connId = handleHello('conn-1', transport, ctx); + + vi.mocked(getTerminalOwner).mockReturnValue(connId); + + handleTerminalResize( + connId, + { type: 'terminal_resize' as const, terminalId: 'term-mock-1', cols: 120, rows: 40 }, + ctx, + ); + + expect(resizeTerminal).toHaveBeenCalledWith('term-mock-1', 120, 40); + }); + + it('rejects resize from non-owner', () => { + const ctx = createContext(); + const transport = mockTransport(); + const connId = handleHello('conn-other', transport, ctx); + + vi.mocked(getTerminalOwner).mockReturnValue('conn-real-owner'); + + handleTerminalResize( + connId, + { type: 'terminal_resize' as const, terminalId: 'term-mock-1', cols: 200, rows: 50 }, + ctx, + ); + + expect(resizeTerminal).not.toHaveBeenCalled(); + }); +}); + +describe('handleTerminalDestroy', () => { + beforeEach(resetTerminalMocks); + + it('destroys terminal after ownership check', () => { + const ctx = createContext(); + const transport = mockTransport(); + const connId = handleHello('conn-1', transport, ctx); + + vi.mocked(getTerminalOwner).mockReturnValue(connId); + + handleTerminalDestroy( + connId, + { type: 'terminal_destroy' as const, terminalId: 'term-mock-1' }, + ctx, + ); + + expect(destroyTerminal).toHaveBeenCalledWith('term-mock-1'); + }); + + it('rejects destroy from non-owner', () => { + const ctx = createContext(); + const transport = mockTransport(); + const connId = handleHello('conn-other', transport, ctx); + + vi.mocked(getTerminalOwner).mockReturnValue('conn-real-owner'); + + handleTerminalDestroy( + connId, + { type: 'terminal_destroy' as const, terminalId: 'term-mock-1' }, + ctx, + ); + + expect(destroyTerminal).not.toHaveBeenCalled(); + }); +}); + +describe('dispatchV2Message — terminal routing', () => { + beforeEach(resetTerminalMocks); + + it('routes terminal_create to handler', async () => { + const ctx = createContext(); + const transport = mockTransport(); + const connId = handleHello('conn-dispatch', transport, ctx); + + await dispatchV2Message( + connId, + transport, + JSON.stringify({ type: 'terminal_create', sessionId: 'sess-1', cols: 80, rows: 24 }), + ctx, + ); + + expect(createTerminal).toHaveBeenCalled(); + }); + + it('routes terminal_input to handler', async () => { + const ctx = createContext(); + const transport = mockTransport(); + const connId = handleHello('conn-dispatch', transport, ctx); + + vi.mocked(getTerminalOwner).mockReturnValue(connId); + + await dispatchV2Message( + connId, + transport, + JSON.stringify({ type: 'terminal_input', terminalId: 'term-mock-1', data: 'x' }), + ctx, + ); + + expect(writeTerminal).toHaveBeenCalled(); + }); + + it('routes terminal_destroy to handler', async () => { + const ctx = createContext(); + const transport = mockTransport(); + const connId = handleHello('conn-dispatch', transport, ctx); + + vi.mocked(getTerminalOwner).mockReturnValue(connId); + + await dispatchV2Message( + connId, + transport, + JSON.stringify({ type: 'terminal_destroy', terminalId: 'term-mock-1' }), + ctx, + ); + + expect(destroyTerminal).toHaveBeenCalled(); + }); +}); diff --git a/server/terminal-manager.ts b/server/terminal-manager.ts index 0d0ce11d..cde33f65 100644 --- a/server/terminal-manager.ts +++ b/server/terminal-manager.ts @@ -78,11 +78,19 @@ function buildSafeEnv(extra?: Record): Record { return env; } +export interface CreateTerminalOpts { + cols?: number; + rows?: number; + env?: Record; + onData?: (data: string) => void; + onExit?: (exitCode: number, signal?: number) => void; +} + export function createTerminal( sessionId: string, connectionId: string, cwd: string, - opts?: { cols?: number; rows?: number; env?: Record }, + opts?: CreateTerminalOpts, ): TerminalInfo { if (terminals.size >= MAX_TERMINALS_GLOBAL) { throw new Error(`Global terminal limit reached (${MAX_TERMINALS_GLOBAL})`); @@ -97,27 +105,29 @@ export function createTerminal( const rows = opts?.rows ?? 24; const shell = getDefaultShell(); - const proc = pty.spawn(shell, [], { - name: 'xterm-256color', - cols, - rows, - cwd, - env: buildSafeEnv(opts?.env), - }); - const managed: ManagedTerminal = { id, sessionId, connectionId, - process: proc, + process: null!, cols, rows, cwd, createdAt: Date.now(), - onData: null, - onExit: null, + onData: opts?.onData ?? null, + onExit: opts?.onExit ?? null, }; + // Wire callbacks BEFORE spawning so no output is dropped + const proc = pty.spawn(shell, [], { + name: 'xterm-256color', + cols, + rows, + cwd, + env: buildSafeEnv(opts?.env), + }); + managed.process = proc; + proc.onData((data) => { managed.onData?.(data); }); @@ -153,7 +163,11 @@ export function resizeTerminal(id: string, cols: number, rows: number): boolean export function destroyTerminal(id: string): boolean { const term = terminals.get(id); if (!term) return false; - term.process.kill(); + try { + term.process.kill(); + } catch (err) { + log.warn('kill failed (process may have already exited)', { id, error: String(err) }); + } terminals.delete(id); log.info('terminal destroyed', { id, sessionId: term.sessionId }); return true; @@ -194,7 +208,11 @@ export function destroySessionTerminals(sessionId: string): number { let count = 0; for (const [id, term] of terminals.entries()) { if (term.sessionId === sessionId) { - term.process.kill(); + try { + term.process.kill(); + } catch (err) { + log.warn('kill failed during session cleanup', { id, error: String(err) }); + } terminals.delete(id); count++; } @@ -209,7 +227,11 @@ export function destroyConnectionTerminals(connectionId: string): number { let count = 0; for (const [id, term] of terminals.entries()) { if (term.connectionId === connectionId) { - term.process.kill(); + try { + term.process.kill(); + } catch (err) { + log.warn('kill failed during connection cleanup', { id, error: String(err) }); + } terminals.delete(id); count++; } diff --git a/server/ws-handler-v2.ts b/server/ws-handler-v2.ts index a7c28428..0814ed94 100644 --- a/server/ws-handler-v2.ts +++ b/server/ws-handler-v2.ts @@ -70,7 +70,6 @@ import { writeTerminal, resizeTerminal, destroyTerminal, - setTerminalCallbacks, getTerminalOwner, } from './terminal-manager.js'; import { createLogger } from './logger.js'; @@ -946,26 +945,35 @@ export function handleTerminalCreate( // Resolve cwd from session metadata (worktree path or base repo) const sessionMeta = ctx.eventStore.getSession(msg.sessionId); - const rawCwd = sessionMeta?.cwd || BASE_REPO || process.cwd(); - const cwd = isAllowedPath(rawCwd) ? rawCwd : BASE_REPO || process.cwd(); + const rawCwd = sessionMeta?.cwd || BASE_REPO; + if (!rawCwd) { + conn.transport.send({ + type: 'terminal_error', + error: 'No working directory available for terminal', + }); + return; + } + const cwd = isAllowedPath(rawCwd) ? rawCwd : BASE_REPO; + if (!cwd) { + conn.transport.send({ + type: 'terminal_error', + error: 'Working directory not allowed', + }); + return; + } try { const info = createTerminal(msg.sessionId, connectionId, cwd, { cols: msg.cols, rows: msg.rows, - }); - - // Wire PTY output → WS broadcast to connection - setTerminalCallbacks( - info.id, - (data) => { + onData: (data) => { conn.transport.send({ type: 'terminal_output', terminalId: info.id, data, }); }, - (exitCode, signal) => { + onExit: (exitCode, signal) => { conn.transport.send({ type: 'terminal_exit', terminalId: info.id, @@ -973,7 +981,7 @@ export function handleTerminalCreate( ...(signal !== undefined ? { signal } : {}), }); }, - ); + }); conn.transport.send({ type: 'terminal_created', From b5b1ff845ef11f5a95405d7a208afbdabbb1f094 Mon Sep 17 00:00:00 2001 From: dimakis Date: Sat, 4 Jul 2026 01:31:29 +0100 Subject: [PATCH 6/7] fix(terminal): address third Centaur review findings - Destroy session terminals on session_close (was only cleaning up on WS disconnect, leaving orphaned PTYs for multi-session connections) - Use crypto.randomUUID() for terminal IDs instead of predictable Date.now()-counter pattern - Validate sessionId against EventStore before creating terminals, reject unknown/synthetic session IDs with terminal_error - Change default shell fallback from /bin/zsh to /bin/sh (POSIX standard) - Add tests: onExit auto-cleanup, session close terminal cleanup, unknown sessionId rejection, env prefix filtering (LC_*, XDG_*), extra env merging, dangerous env stripping Co-Authored-By: Claude Opus 4.6 --- server/__tests__/terminal-manager.test.ts | 85 ++++++++++++++++++++++- server/__tests__/ws-handler-v2.test.ts | 75 +++++++++++++++++++- server/terminal-manager.ts | 7 +- server/ws-handler-v2.ts | 27 ++++++- 4 files changed, 184 insertions(+), 10 deletions(-) diff --git a/server/__tests__/terminal-manager.test.ts b/server/__tests__/terminal-manager.test.ts index 494b2f8f..cc336460 100644 --- a/server/__tests__/terminal-manager.test.ts +++ b/server/__tests__/terminal-manager.test.ts @@ -78,7 +78,7 @@ describe('createTerminal', () => { it('returns terminal info with id, pid, dimensions, cwd', () => { const info = createTestTerminal('sess-1', 'conn-1', '/tmp/test'); - expect(info.id).toMatch(/^term-\d+-\d+$/); + expect(info.id).toMatch(/^term-[0-9a-f-]{36}$/); expect(info.sessionId).toBe('sess-1'); expect(info.pid).toBe(12345); expect(info.cols).toBe(80); @@ -401,3 +401,86 @@ describe('clearTerminalCallbacks', () => { expect(clearTerminalCallbacks('nonexistent')).toBe(false); }); }); + +// ─── onExit auto-cleanup ──────────────────────────────────────────────────── + +describe('onExit auto-cleanup', () => { + it('removes terminal from map when PTY process exits naturally', () => { + const info = createTestTerminal('sess-1', 'conn-1', '/tmp'); + + // Terminal should exist + expect(getTerminal(info.id)).not.toBeNull(); + + // Simulate PTY process exit via the onExit handler + const spawnResult = vi.mocked(pty.spawn).mock.results[0].value; + const registeredOnExit = spawnResult.onExit.mock.calls[0][0]; + registeredOnExit({ exitCode: 0, signal: 0 }); + + // Terminal should be auto-removed + expect(getTerminal(info.id)).toBeNull(); + expect(listTerminals('sess-1')).toHaveLength(0); + + // Remove from cleanup list since it auto-cleaned + createdIds = createdIds.filter((id) => id !== info.id); + }); +}); + +// ─── environment variable filtering ───────────────────────────────────────── + +describe('buildSafeEnv (via createTerminal)', () => { + it('passes LC_* prefixed vars through', () => { + process.env.LC_CTYPE = 'UTF-8'; + process.env.LC_MESSAGES = 'en_US.UTF-8'; + createTestTerminal(); + + const spawnCall = vi.mocked(pty.spawn).mock.calls[0]; + const env = spawnCall[2].env as Record; + + expect(env.LC_CTYPE).toBe('UTF-8'); + expect(env.LC_MESSAGES).toBe('en_US.UTF-8'); + }); + + it('passes XDG_* prefixed vars through', () => { + process.env.XDG_CONFIG_HOME = '/home/user/.config'; + process.env.XDG_DATA_HOME = '/home/user/.local/share'; + createTestTerminal(); + + const spawnCall = vi.mocked(pty.spawn).mock.calls[0]; + const env = spawnCall[2].env as Record; + + expect(env.XDG_CONFIG_HOME).toBe('/home/user/.config'); + expect(env.XDG_DATA_HOME).toBe('/home/user/.local/share'); + + delete process.env.XDG_CONFIG_HOME; + delete process.env.XDG_DATA_HOME; + }); + + it('merges extra env vars from opts', () => { + createTestTerminal('sess-1', 'conn-1', '/tmp', { + env: { CUSTOM_VAR: 'custom-value' }, + }); + + const spawnCall = vi.mocked(pty.spawn).mock.calls[0]; + const env = spawnCall[2].env as Record; + + expect(env.CUSTOM_VAR).toBe('custom-value'); + }); + + it('strips dangerous env vars (DATABASE_URL, secrets, etc.)', () => { + process.env.DATABASE_URL = 'postgres://secret'; + process.env.AWS_SECRET_ACCESS_KEY = 'aws-secret'; + process.env.JWT_SECRET = 'jwt-secret'; + createTestTerminal(); + + const spawnCall = vi.mocked(pty.spawn).mock.calls[0]; + const env = spawnCall[2].env as Record; + + expect(env.DATABASE_URL).toBeUndefined(); + expect(env.AWS_SECRET_ACCESS_KEY).toBeUndefined(); + expect(env.JWT_SECRET).toBeUndefined(); + + delete process.env.DATABASE_URL; + delete process.env.AWS_SECRET_ACCESS_KEY; + delete process.env.JWT_SECRET; + }); +}); diff --git a/server/__tests__/ws-handler-v2.test.ts b/server/__tests__/ws-handler-v2.test.ts index 3465622f..cbbbb7b2 100644 --- a/server/__tests__/ws-handler-v2.test.ts +++ b/server/__tests__/ws-handler-v2.test.ts @@ -8,6 +8,7 @@ vi.mock('../chat.js', () => ({ sendToChat: vi.fn(), interruptChat: vi.fn(), stopChat: vi.fn(), + closeSessionByUser: vi.fn(), isActive: vi.fn().mockReturnValue(false), reattachChat: vi.fn().mockReturnValue(true), rekeyChat: vi.fn().mockReturnValue(true), @@ -48,6 +49,7 @@ vi.mock('../terminal-manager.js', () => ({ writeTerminal: vi.fn().mockReturnValue(true), resizeTerminal: vi.fn().mockReturnValue(true), destroyTerminal: vi.fn().mockReturnValue(true), + destroySessionTerminals: vi.fn().mockReturnValue(0), setTerminalCallbacks: vi.fn().mockReturnValue(true), getTerminalOwner: vi.fn().mockReturnValue('conn-1'), })); @@ -70,6 +72,7 @@ import { writeTerminal, resizeTerminal, destroyTerminal, + destroySessionTerminals, getTerminalOwner, } from '../terminal-manager.js'; @@ -85,6 +88,7 @@ import { handleStopV2, handlePermissionResponseV2, handleSessionSuspend, + handleSessionClose, handleTerminalCreate, handleTerminalInput, handleTerminalResize, @@ -3376,7 +3380,9 @@ function resetTerminalMocks() { vi.mocked(writeTerminal).mockClear(); vi.mocked(resizeTerminal).mockClear(); vi.mocked(destroyTerminal).mockClear(); + vi.mocked(destroySessionTerminals).mockClear(); vi.mocked(getTerminalOwner).mockClear(); + vi.mocked(destroySessionTerminals).mockReturnValue(0); vi.mocked(createTerminal).mockReturnValue({ id: 'term-mock-1', sessionId: 'sess-1', @@ -3395,7 +3401,11 @@ function resetTerminalMocks() { describe('handleTerminalCreate', () => { beforeEach(resetTerminalMocks); it('creates a terminal and sends terminal_created response', () => { - const ctx = createContext(); + const eventStore = mockEventStore(); + eventStore.getSession.mockReturnValue({ cwd: '/tmp/test-repo' }); + const ctx = createContext({ + eventStore: eventStore as unknown as V2HandlerContext['eventStore'], + }); const transport = mockTransport(); const connId = handleHello('conn-term', transport, ctx); @@ -3419,7 +3429,11 @@ describe('handleTerminalCreate', () => { }); it('sends terminal_error when createTerminal throws', () => { - const ctx = createContext(); + const eventStore = mockEventStore(); + eventStore.getSession.mockReturnValue({ cwd: '/tmp/test-repo' }); + const ctx = createContext({ + eventStore: eventStore as unknown as V2HandlerContext['eventStore'], + }); const transport = mockTransport(); const connId = handleHello('conn-err', transport, ctx); @@ -3587,7 +3601,11 @@ describe('dispatchV2Message — terminal routing', () => { beforeEach(resetTerminalMocks); it('routes terminal_create to handler', async () => { - const ctx = createContext(); + const eventStore = mockEventStore(); + eventStore.getSession.mockReturnValue({ cwd: '/tmp/test-repo' }); + const ctx = createContext({ + eventStore: eventStore as unknown as V2HandlerContext['eventStore'], + }); const transport = mockTransport(); const connId = handleHello('conn-dispatch', transport, ctx); @@ -3635,3 +3653,54 @@ describe('dispatchV2Message — terminal routing', () => { expect(destroyTerminal).toHaveBeenCalled(); }); }); + +describe('handleTerminalCreate — sessionId validation', () => { + beforeEach(resetTerminalMocks); + + it('rejects terminal creation for unknown sessionId', () => { + const eventStore = mockEventStore(); + eventStore.getSession.mockReturnValue(null); + const ctx = createContext({ + eventStore: eventStore as unknown as V2HandlerContext['eventStore'], + }); + const transport = mockTransport(); + const connId = handleHello('conn-unknown-sess', transport, ctx); + + handleTerminalCreate( + connId, + { type: 'terminal_create' as const, sessionId: 'fake-session-123' }, + ctx, + ); + + expect(createTerminal).not.toHaveBeenCalled(); + const error = transport.sent.find((m) => m.type === 'terminal_error'); + expect(error).toBeDefined(); + expect(error!.error).toContain('Unknown session'); + }); +}); + +describe('handleSessionClose — terminal cleanup', () => { + beforeEach(resetTerminalMocks); + + it('destroys session terminals when session is closed', () => { + const sessionRegistry = mockSessionRegistry(); + sessionRegistry.findBySessionId.mockReturnValue({ + clientId: 'conn-close:sess-close', + session: {}, + }); + const ctx = createContext({ + sessionRegistry: sessionRegistry as unknown as V2HandlerContext['sessionRegistry'], + }); + const transport = mockTransport(); + const connId = handleHello('conn-close', transport, ctx); + + vi.mocked(destroySessionTerminals).mockReturnValue(2); + + handleSessionClose(connId, { type: 'session_close' as const, sessionId: 'sess-close' }, ctx); + + expect(destroySessionTerminals).toHaveBeenCalledWith('sess-close'); + const ack = transport.sent.find((m) => m.type === 'session_close_ack'); + expect(ack).toBeDefined(); + expect(ack!.accepted).toBe(true); + }); +}); diff --git a/server/terminal-manager.ts b/server/terminal-manager.ts index cde33f65..409f29a8 100644 --- a/server/terminal-manager.ts +++ b/server/terminal-manager.ts @@ -1,6 +1,7 @@ /** Terminal Manager — PTY lifecycle for interactive shell terminals. */ import * as pty from 'node-pty'; +import { randomUUID } from 'crypto'; import { createLogger } from './logger.js'; const log = createLogger('terminal'); @@ -51,17 +52,15 @@ const SAFE_ENV_PREFIXES = ['LC_', 'XDG_']; const MAX_TERMINALS_PER_SESSION = 5; const MAX_TERMINALS_GLOBAL = 50; -let terminalCounter = 0; - /** Active terminals keyed by terminal ID. */ const terminals = new Map(); function generateTerminalId(): string { - return `term-${Date.now()}-${++terminalCounter}`; + return `term-${randomUUID()}`; } function getDefaultShell(): string { - return process.env.SHELL || (process.platform === 'win32' ? 'powershell.exe' : '/bin/zsh'); + return process.env.SHELL || (process.platform === 'win32' ? 'powershell.exe' : '/bin/sh'); } function buildSafeEnv(extra?: Record): Record { diff --git a/server/ws-handler-v2.ts b/server/ws-handler-v2.ts index 0814ed94..53a5bc19 100644 --- a/server/ws-handler-v2.ts +++ b/server/ws-handler-v2.ts @@ -70,6 +70,7 @@ import { writeTerminal, resizeTerminal, destroyTerminal, + destroySessionTerminals, getTerminalOwner, } from './terminal-manager.js'; import { createLogger } from './logger.js'; @@ -913,6 +914,15 @@ export function handleSessionClose( return; } + // Clean up any terminals associated with this session + const destroyed = destroySessionTerminals(msg.sessionId); + if (destroyed > 0) { + log.info('destroyed terminals on session close', { + sessionId: msg.sessionId, + count: destroyed, + }); + } + closeSessionByUser(found.clientId); log.info('session close initiated by user', { connectionId, @@ -943,9 +953,22 @@ export function handleTerminalCreate( const conn = ctx.connRegistry.get(connectionId); if (!conn) return; - // Resolve cwd from session metadata (worktree path or base repo) + // Validate that sessionId refers to a known session const sessionMeta = ctx.eventStore.getSession(msg.sessionId); - const rawCwd = sessionMeta?.cwd || BASE_REPO; + if (!sessionMeta) { + log.warn('terminal create: unknown sessionId', { + connectionId, + sessionId: msg.sessionId, + }); + conn.transport.send({ + type: 'terminal_error', + error: 'Unknown session — cannot create terminal', + }); + return; + } + + // Resolve cwd from session metadata (worktree path or base repo) + const rawCwd = sessionMeta.cwd || BASE_REPO; if (!rawCwd) { conn.transport.send({ type: 'terminal_error', From 78699feaf444b4b754d0e8c7f16ab9aaf218f0e5 Mon Sep 17 00:00:00 2001 From: dimakis Date: Sat, 4 Jul 2026 10:06:32 +0100 Subject: [PATCH 7/7] fix(terminal): allow standalone terminals without a real session Standalone terminals (navigating to /terminal without a chat session) are the primary use case. Session lookup is now optional: if a sessionId maps to a real session, use its worktree cwd; otherwise fall back to BASE_REPO. Never reject for missing sessions. Co-Authored-By: Claude Opus 4.6 --- server/__tests__/ws-handler-v2.test.ts | 20 ++++++++++++-------- server/ws-handler-v2.ts | 18 +++--------------- 2 files changed, 15 insertions(+), 23 deletions(-) diff --git a/server/__tests__/ws-handler-v2.test.ts b/server/__tests__/ws-handler-v2.test.ts index cbbbb7b2..e547c57c 100644 --- a/server/__tests__/ws-handler-v2.test.ts +++ b/server/__tests__/ws-handler-v2.test.ts @@ -3654,28 +3654,32 @@ describe('dispatchV2Message — terminal routing', () => { }); }); -describe('handleTerminalCreate — sessionId validation', () => { +describe('handleTerminalCreate — standalone terminals', () => { beforeEach(resetTerminalMocks); - it('rejects terminal creation for unknown sessionId', () => { + it('creates a standalone terminal with BASE_REPO cwd when sessionId has no real session', () => { const eventStore = mockEventStore(); eventStore.getSession.mockReturnValue(null); const ctx = createContext({ eventStore: eventStore as unknown as V2HandlerContext['eventStore'], }); const transport = mockTransport(); - const connId = handleHello('conn-unknown-sess', transport, ctx); + const connId = handleHello('conn-standalone', transport, ctx); handleTerminalCreate( connId, - { type: 'terminal_create' as const, sessionId: 'fake-session-123' }, + { type: 'terminal_create' as const, sessionId: 'terminal-standalone' }, ctx, ); - expect(createTerminal).not.toHaveBeenCalled(); - const error = transport.sent.find((m) => m.type === 'terminal_error'); - expect(error).toBeDefined(); - expect(error!.error).toContain('Unknown session'); + expect(createTerminal).toHaveBeenCalledWith( + 'terminal-standalone', + connId, + '/tmp/test-repo', + expect.any(Object), + ); + const created = transport.sent.find((m) => m.type === 'terminal_created'); + expect(created).toBeDefined(); }); }); diff --git a/server/ws-handler-v2.ts b/server/ws-handler-v2.ts index 53a5bc19..244c7be5 100644 --- a/server/ws-handler-v2.ts +++ b/server/ws-handler-v2.ts @@ -953,22 +953,10 @@ export function handleTerminalCreate( const conn = ctx.connRegistry.get(connectionId); if (!conn) return; - // Validate that sessionId refers to a known session + // Resolve cwd: use session worktree if available, otherwise BASE_REPO. + // Standalone terminals (no real session) are a valid use case. const sessionMeta = ctx.eventStore.getSession(msg.sessionId); - if (!sessionMeta) { - log.warn('terminal create: unknown sessionId', { - connectionId, - sessionId: msg.sessionId, - }); - conn.transport.send({ - type: 'terminal_error', - error: 'Unknown session — cannot create terminal', - }); - return; - } - - // Resolve cwd from session metadata (worktree path or base repo) - const rawCwd = sessionMeta.cwd || BASE_REPO; + const rawCwd = sessionMeta?.cwd || BASE_REPO; if (!rawCwd) { conn.transport.send({ type: 'terminal_error',