feat(terminal): interactive shell terminal — Phase 1 by dimakis · Pull Request #394 · dimakis/mitzo

dimakis · 2026-06-25T11:58:17Z

Summary

Server-side PTY manager (server/terminal-manager.ts) — spawn, I/O, resize, cleanup via node-pty
WS protocol extension — 4 new message types (terminal_create/input/resize/destroy) with Zod schemas, wired into v2 dispatcher
xterm.js frontend — Terminal.tsx component with mobile keyboard toolbar (Esc, Tab, Ctrl, arrows, pipe, slash, tilde)
Full-page TerminalView with connection status, back navigation, and useTerminal hook managing WS lifecycle
Navigation — "Terminal" in mobile More menu + desktop sidebar, routes at /terminal and /terminal/:sessionId
REST endpoint — GET /api/terminals for listing active terminals

Architecture

Terminal I/O uses a dedicated WS connection (same /ws/chat endpoint, v2 protocol) independent of the chat connection. Each terminal spawns a node-pty process scoped to the session's worktree cwd. The mobile toolbar is hidden on desktop (real keyboard available).

Phases

Phase 1 (this PR): Core terminal — PTY, protocol, xterm.js, mobile toolbar, navigation
Phase 2: Multi-tab terminals
Phase 3: Claude integration (paste output to chat, split-view)

Test plan

Navigate to Terminal from mobile More menu and desktop sidebar
Verify shell spawns and renders in xterm.js
Type commands, verify output renders correctly (colors, cursor)
Test mobile toolbar keys (Esc, Tab, Ctrl+C, arrows)
Resize browser window, verify terminal re-fits
Navigate away and back, verify terminal cleanup
Test with active session — verify worktree cwd is used

🤖 Generated with Claude Code

- 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 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

dimakis

Centaur Review

Found 10 issue(s) (3 critical) (5 warning).

`frontend/src/hooks/useTerminal.ts`

Critical build blocker (missing getWsChatUrl export), no terminal cleanup on disconnect (PTY leak), no resource limits, and no tests — the feature needs fixes before merge.

🔴 bugs (L11): getWsChatUrl is imported from '../lib/api-fetch' but this function does not exist anywhere in the codebase. No file at frontend/src/lib/api-fetch.ts exists, and no other file exports this symbol. This will fail at build time or runtime. [fixable]
🟡 bugs (L59): The connect callback captures cols and rows in its closure (via the useCallback dependency array). If the xterm.js FitAddon resizes the terminal before the WebSocket welcome arrives, the terminal_create message will send stale initial dimensions. Consider sending a terminal_resize after terminal_created to sync. [fixable]
🟡 unsafe_assumptions (L59): No reconnection logic. If the WebSocket drops, the terminal is lost with no way to resume. Other parts of the system (MitzoConnection) handle reconnection for chat — terminal connections should at minimum attempt reconnect or notify the user clearly. [fixable]

`server/terminal-manager.ts`

Critical build blocker (missing getWsChatUrl export), no terminal cleanup on disconnect (PTY leak), no resource limits, and no tests — the feature needs fixes before merge.

🔴 unsafe_assumptions (L61): No limit on the number of terminals that can be created. An authenticated user can spawn unlimited PTY processes, exhausting server PIDs, memory, and file descriptors. Add a per-session and global terminal cap (e.g., 5 per session, 50 total). [fixable]
🔴 bugs (L158): destroySessionTerminals() is defined but never called. When a WebSocket disconnects (index.ts close handler) or a session is closed (handleSessionClose / closeSessionByUser), terminals are not cleaned up. PTY processes will be orphaned and continue running indefinitely. [fixable]
🟡 missing_tests: No tests for terminal-manager.ts. The project requires TDD per CLAUDE.md. At minimum, unit tests should cover: createTerminal, writeTerminal, resizeTerminal, destroyTerminal, destroySessionTerminals, listTerminals, and the callback wiring (setTerminalCallbacks/clearTerminalCallbacks). node-pty can be mocked. [fixable]

`server/ws-handler-v2.ts`

Critical build blocker (missing getWsChatUrl export), no terminal cleanup on disconnect (PTY leak), no resource limits, and no tests — the feature needs fixes before merge.

🟡 unsafe_assumptions (L946): Terminal cwd is resolved via sessionMeta?.cwd || BASE_REPO || process.cwd() without isAllowedPath() validation. The send handler (line 474) validates cwd with isAllowedPath() — terminal_create should do the same to prevent shells in unexpected directories. [fixable]
🔵 style (L1056): handleTerminalDestroy logs success even when the terminal was not found (the !ok branch sends an error but execution continues to the log.info on line 1056). Move the log inside a success branch. [fixable]

`packages/protocol/src/ws-schemas-v2.ts`

Critical build blocker (missing getWsChatUrl export), no terminal cleanup on disconnect (PTY leak), no resource limits, and no tests — the feature needs fixes before merge.

🟡 unsafe_assumptions (L134): TerminalInputMessage.data is z.string() with no max length. A malicious client could send arbitrarily large strings, causing memory pressure. Add .max() (e.g., 64KB) consistent with typical terminal input bounds. [fixable]

`frontend/src/components/Terminal.tsx`

Critical build blocker (missing getWsChatUrl export), no terminal cleanup on disconnect (PTY leak), no resource limits, and no tests — the feature needs fixes before merge.

🔵 style (L1): Multi-line doc comment block violates the project convention: 'default to writing no comments... never write multi-paragraph docstrings or multi-line comment blocks — one short line max.' Same applies to TerminalView.tsx (line 1) and useTerminal.ts (line 1) and terminal-manager.ts (line 1). [fixable]

dimakis · 2026-06-25T12:03:12Z

+
+import { useRef, useCallback, useEffect, useState } from 'react';
+import { getWsChatUrl } from '../lib/api-fetch';
+


🔴 bugs: getWsChatUrl is imported from '../lib/api-fetch' but this function does not exist anywhere in the codebase. No file at frontend/src/lib/api-fetch.ts exists, and no other file exports this symbol. This will fail at build time or runtime. [fixable]

dimakis · 2026-06-25T12:03:12Z

+    }
+  }, []);
+
+  const connect = useCallback(() => {


🟡 bugs: The connect callback captures cols and rows in its closure (via the useCallback dependency array). If the xterm.js FitAddon resizes the terminal before the WebSocket welcome arrives, the terminal_create message will send stale initial dimensions. Consider sending a terminal_resize after terminal_created to sync. [fixable]

dimakis · 2026-06-25T12:03:12Z

+    }
+  }, []);
+
+  const connect = useCallback(() => {


🟡 unsafe_assumptions: No reconnection logic. If the WebSocket drops, the terminal is lost with no way to resume. Other parts of the system (MitzoConnection) handle reconnection for chat — terminal connections should at minimum attempt reconnect or notify the user clearly. [fixable]

dimakis · 2026-06-25T12:03:12Z

+  const rows = opts?.rows ?? 24;
+  const shell = getDefaultShell();
+
+  const proc = pty.spawn(shell, [], {


🔴 unsafe_assumptions: No limit on the number of terminals that can be created. An authenticated user can spawn unlimited PTY processes, exhausting server PIDs, memory, and file descriptors. Add a per-session and global terminal cap (e.g., 5 per session, 50 total). [fixable]

dimakis · 2026-06-25T12:03:12Z

+  return result;
+}
+
+export function destroySessionTerminals(sessionId: string): number {


🔴 bugs: destroySessionTerminals() is defined but never called. When a WebSocket disconnects (index.ts close handler) or a session is closed (handleSessionClose / closeSessionByUser), terminals are not cleaned up. PTY processes will be orphaned and continue running indefinitely. [fixable]

dimakis · 2026-06-25T12:03:12Z

+
+      // 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();


🟡 unsafe_assumptions: Terminal cwd is resolved via sessionMeta?.cwd || BASE_REPO || process.cwd() without isAllowedPath() validation. The send handler (line 474) validates cwd with isAllowedPath() — terminal_create should do the same to prevent shells in unexpected directories. [fixable]

dimakis · 2026-06-25T12:03:12Z

+
 // ─── Dispatcher ──────────────────────────────────────────────────────────────

 /**


🔵 style: handleTerminalDestroy logs success even when the terminal was not found (the !ok branch sends an error but execution continues to the log.info on line 1056). Move the log inside a success branch. [fixable]

dimakis · 2026-06-25T12:03:12Z

+  rows: z.number().int().min(1).optional(),
+});
+
+export const TerminalInputMessage = z.object({


🟡 unsafe_assumptions: TerminalInputMessage.data is z.string() with no max length. A malicious client could send arbitrarily large strings, causing memory pressure. Add .max() (e.g., 64KB) consistent with typical terminal input bounds. [fixable]

dimakis · 2026-06-25T12:03:12Z

@@ -0,0 +1,165 @@
+/**


🔵 style: Multi-line doc comment block violates the project convention: 'default to writing no comments... never write multi-paragraph docstrings or multi-line comment blocks — one short line max.' Same applies to TerminalView.tsx (line 1) and useTerminal.ts (line 1) and terminal-manager.ts (line 1). [fixable]

dimakis

Centaur Review

Found 10 issue(s) (2 critical) (5 warning).

`server/ws-handler-v2.ts`

The terminal feature works end-to-end but has two critical gaps: PTY processes are never cleaned up on disconnect (resource leak), and the spawned shell's cwd is not validated with isAllowedPath(). No tests exist, violating the project's mandatory TDD policy.

🔴 bugs (L940): Terminal PTY processes are never cleaned up when a WebSocket connection closes. The ws.on('close') handler in index.ts detaches sessions but does not call destroySessionTerminals(). Similarly, handleSessionClose does not clean up terminals. This means PTY processes leak indefinitely — they persist until the shell naturally exits or the server restarts. The destroySessionTerminals() function exists in terminal-manager.ts but is never imported or called anywhere. [fixable]
🔴 unsafe_assumptions (L943): No path validation on the cwd used to spawn the PTY. handleSendV2 validates cwd with isAllowedPath(), but handleTerminalCreate passes sessionMeta?.cwd || BASE_REPO || process.cwd() directly to createTerminal() without validation. If a session's stored cwd is somehow set to a path outside the allowed set (e.g., /etc), the PTY spawns there. The isAllowedPath check should be applied here too. [fixable]
🟡 missing_tests: No test coverage for the four terminal WS handlers (handleTerminalCreate, handleTerminalInput, handleTerminalResize, handleTerminalDestroy). Error paths (terminal not found, create failure) and the happy path should be tested. [fixable]

`server/terminal-manager.ts`

The terminal feature works end-to-end but has two critical gaps: PTY processes are never cleaned up on disconnect (resource leak), and the spawned shell's cwd is not validated with isAllowedPath(). No tests exist, violating the project's mandatory TDD policy.

🟡 unsafe_assumptions (L69): No limit on the number of terminals a client can create. A single authenticated user could call terminal_create in a loop, spawning unlimited PTY processes and exhausting server resources. Consider adding a per-session or per-connection cap (e.g., max 5 terminals). [fixable]
🟡 missing_tests: No test coverage for terminal-manager.ts. The CLAUDE.md mandates TDD for all feature work. Core logic like createTerminal, destroyTerminal, destroySessionTerminals, listTerminals, and the callback wiring should have unit tests. The PTY can be mocked for deterministic testing. [fixable]

`frontend/src/hooks/useTerminal.ts`

The terminal feature works end-to-end but has two critical gaps: PTY processes are never cleaned up on disconnect (resource leak), and the spawned shell's cwd is not validated with isAllowedPath(). No tests exist, violating the project's mandatory TDD policy.

🟡 bugs (L62): The connect callback captures cols and rows in its closure (via useCallback deps), but these values are from the hook's initial options, not the terminal's actual dimensions after xterm.js fit(). The Terminal component calls onResize after fitting, but connect sends the stale default (80x24) in the terminal_create message. The actual fitted dimensions are only sent if resize is called later, creating a mismatch between the PTY and the rendered terminal until the first resize event. [fixable]
🔵 style: The useTerminal hook creates a separate WebSocket connection instead of multiplexing on the existing shared connection (MitzoConnection). The v2 protocol dispatcher already routes terminal messages, so a separate WS is redundant — it doubles connections per client and requires a duplicate hello/welcome handshake. Consider integrating with the existing @mitzo/client connection for consistency with the architecture described in CLAUDE.md. [fixable]

`frontend/src/pages/TerminalView.tsx`

The terminal feature works end-to-end but has two critical gaps: PTY processes are never cleaned up on disconnect (resource leak), and the spawned shell's cwd is not validated with isAllowedPath(). No tests exist, violating the project's mandatory TDD policy.

🟡 unsafe_assumptions (L18): When no sessionId URL param is provided, the code defaults to 'default'. This string is passed to terminal_create which looks up eventStore.getSession('default') — unlikely to match any real session. The PTY will fall back to BASE_REPO || process.cwd(), which works but is not explicitly documented. The /terminal route without a sessionId should either redirect to a session picker or clearly document this fallback behavior. [fixable]

`frontend/src/App.tsx`

The terminal feature works end-to-end but has two critical gaps: PTY processes are never cleaned up on disconnect (resource leak), and the spawned shell's cwd is not validated with isAllowedPath(). No tests exist, violating the project's mandatory TDD policy.

🔵 style (L172): The /terminal routes are not wrapped in <PageRoute> unlike all other protected pages (/inbox, /calendar, /todos, /tasks, /files). On desktop, the terminal view will not render inside the DesktopShell layout, losing the nav sidebar. This may be intentional for a full-screen terminal experience, but is inconsistent with the other pages. [fixable]

`frontend/src/components/Terminal.tsx`

The terminal feature works end-to-end but has two critical gaps: PTY processes are never cleaned up on disconnect (resource leak), and the spawned shell's cwd is not validated with isAllowedPath(). No tests exist, violating the project's mandatory TDD policy.

🔵 style (L1): Multi-line file-level comment block (lines 1-7) violates the project convention: 'Default to writing no comments. Never write multi-paragraph docstrings or multi-line comment blocks — one short line max.' Same applies to the comment blocks in useTerminal.ts and TerminalView.tsx. [fixable]

dimakis · 2026-06-25T12:22:15Z

+  withSpan(
+    'ws.terminal_create',
+    { 'ws.connectionId': connectionId, 'ws.sessionId': msg.sessionId },
+    () => {


🔴 bugs: Terminal PTY processes are never cleaned up when a WebSocket connection closes. The ws.on('close') handler in index.ts detaches sessions but does not call destroySessionTerminals(). Similarly, handleSessionClose does not clean up terminals. This means PTY processes leak indefinitely — they persist until the shell naturally exits or the server restarts. The destroySessionTerminals() function exists in terminal-manager.ts but is never imported or called anywhere. [fixable]

dimakis · 2026-06-25T12:22:15Z

+    () => {
+      const conn = ctx.connRegistry.get(connectionId);
+      if (!conn) return;
+


🔴 unsafe_assumptions: No path validation on the cwd used to spawn the PTY. handleSendV2 validates cwd with isAllowedPath(), but handleTerminalCreate passes sessionMeta?.cwd || BASE_REPO || process.cwd() directly to createTerminal() without validation. If a session's stored cwd is somehow set to a path outside the allowed set (e.g., /etc), the PTY spawns there. The isAllowedPath check should be applied here too. [fixable]

dimakis · 2026-06-25T12:22:15Z

+    env: {
+      ...process.env,
+      TERM: 'xterm-256color',
+      COLORTERM: 'truecolor',


🟡 unsafe_assumptions: No limit on the number of terminals a client can create. A single authenticated user could call terminal_create in a loop, spawning unlimited PTY processes and exhausting server resources. Consider adding a per-session or per-connection cap (e.g., max 5 terminals). [fixable]

dimakis · 2026-06-25T12:22:15Z

+  const connect = useCallback(() => {
+    if (wsRef.current?.readyState === WebSocket.OPEN) return;
+
+    const ws = new WebSocket(getWsChatUrl());


🟡 bugs: The connect callback captures cols and rows in its closure (via useCallback deps), but these values are from the hook's initial options, not the terminal's actual dimensions after xterm.js fit(). The Terminal component calls onResize after fitting, but connect sends the stale default (80x24) in the terminal_create message. The actual fitted dimensions are only sent if resize is called later, creating a mismatch between the PTY and the rendered terminal until the first resize event. [fixable]

dimakis · 2026-06-25T12:22:15Z

+  const navigate = useNavigate();
+  const termRef = useRef<{ write: (data: string) => void } | null>(null);
+
+  const resolvedSessionId = sessionId || 'default';


🟡 unsafe_assumptions: When no sessionId URL param is provided, the code defaults to 'default'. This string is passed to terminal_create which looks up eventStore.getSession('default') — unlikely to match any real session. The PTY will fall back to BASE_REPO || process.cwd(), which works but is not explicitly documented. The /terminal route without a sessionId should either redirect to a session picker or clearly document this fallback behavior. [fixable]

dimakis · 2026-06-25T12:22:15Z

                  </ProtectedRoute>
                }
              />
+              <Route


🔵 style: The /terminal routes are not wrapped in <PageRoute> unlike all other protected pages (/inbox, /calendar, /todos, /tasks, /files). On desktop, the terminal view will not render inside the DesktopShell layout, losing the nav sidebar. This may be intentional for a full-screen terminal experience, but is inconsistent with the other pages. [fixable]

dimakis · 2026-06-25T12:22:15Z

@@ -0,0 +1,165 @@
+/**


🔵 style: Multi-line file-level comment block (lines 1-7) violates the project convention: 'Default to writing no comments. Never write multi-paragraph docstrings or multi-line comment blocks — one short line max.' Same applies to the comment blocks in useTerminal.ts and TerminalView.tsx. [fixable]

dimakis

Centaur Review

Found 10 issue(s) (2 critical) (6 warning).

`server/terminal-manager.ts`

The terminal feature works end-to-end but has critical gaps: PTY processes are never cleaned up on disconnect (orphan leak), terminal operations lack session ownership checks (any authed user can control any terminal), and the server's full environment is passed to spawned shells. No tests exist despite the project's strict TDD policy.

🔴 bugs (L158): destroySessionTerminals() is exported but never called anywhere — not on session close, not on WS disconnect, not on server shutdown. When a WebSocket disconnects or a session is closed, the PTY processes remain alive as orphans, leaking processes and memory indefinitely. The onData callback will also keep trying to send to a dead transport. Either handleSessionClose in ws-handler-v2.ts or the WS close handler in index.ts should call destroySessionTerminals(sessionId). [fixable]
🟡 unsafe_assumptions (L61): The spawned PTY inherits the entire server process.env via ...process.env. This exposes sensitive server-side variables (API keys, JWT secrets, database credentials, ANTHROPIC_API_KEY, etc.) to the interactive shell. Consider using an explicit allowlist of safe environment variables or at minimum filtering out known secrets. [fixable]
🟡 unsafe_assumptions (L51): No limit on the number of terminals a client can create. A single authenticated user can spam terminal_create messages and spawn unlimited PTY processes, exhausting server resources (PIDs, memory, file descriptors). Add a per-session or global terminal cap. [fixable]
🟡 missing_tests: No tests for terminal-manager.ts. The project follows strict TDD per CLAUDE.md — the test file should be the first artifact. At minimum, createTerminal, writeTerminal, resizeTerminal, destroyTerminal, destroySessionTerminals, and the onExit cleanup path need test coverage. Other server modules of comparable complexity (e.g., task-store, worktree, auth) all have tests in server/__tests__/. [fixable]
🟡 bugs (L86): PTY onData fires immediately after proc.onData() is registered, but managed.onData is null until setTerminalCallbacks() is called later (after the WS response round-trip). Any output the shell produces between spawn and callback registration (e.g., shell init scripts, .zshrc output) is silently dropped. Consider buffering output until callbacks are attached. [fixable]

`server/ws-handler-v2.ts`

The terminal feature works end-to-end but has critical gaps: PTY processes are never cleaned up on disconnect (orphan leak), terminal operations lack session ownership checks (any authed user can control any terminal), and the server's full environment is passed to spawned shells. No tests exist despite the project's strict TDD policy.

🔴 unsafe_assumptions (L932): handleTerminalCreate does not verify that the requesting connection owns the session. Any authenticated client can send terminal_create with an arbitrary sessionId and get a shell in that session's worktree cwd. Compare with handleSessionClose which checks getOwnerConnection(found.clientId) !== connectionId before acting. The same ownership check is missing from handleTerminalInput, handleTerminalResize, and handleTerminalDestroy — any client can write to, resize, or kill any terminal by guessing the ID. [fixable]
🟡 bugs (L948): The cwd fallback chain sessionMeta?.cwd || BASE_REPO || process.cwd() falls through to process.cwd() when both are falsy. BASE_REPO is process.env.REPO_PATH || '' — an empty string is falsy, so if REPO_PATH is unset and the session has no cwd, the terminal spawns in the server's working directory. This is likely fine in practice (REPO_PATH is required) but the empty-string case is a silent fallback that could surprise. [fixable]

`frontend/src/pages/TerminalView.tsx`

The terminal feature works end-to-end but has critical gaps: PTY processes are never cleaned up on disconnect (orphan leak), terminal operations lack session ownership checks (any authed user can control any terminal), and the server's full environment is passed to spawned shells. No tests exist despite the project's strict TDD policy.

🟡 bugs (L35): The useEffect that calls connect() and returns destroy() depends on [connect, destroy]. But connect is memoized with [sessionId, cols, rows, send] — since cols/rows use default values (80/24), any future change to these (e.g., from a resize) would recreate connect, tearing down and reconnecting the WS. More immediately, destroy creates a new function identity each render because send is stable but destroy captures send — this is actually stable, but the connect/destroy cycle on mount/unmount could race if React strict-mode double-mounts. [fixable]

`frontend/src/components/Terminal.tsx`

The terminal feature works end-to-end but has critical gaps: PTY processes are never cleaned up on disconnect (orphan leak), terminal operations lack session ownership checks (any authed user can control any terminal), and the server's full environment is passed to spawned shells. No tests exist despite the project's strict TDD policy.

🔵 style (L1): Multi-line docstring comment block at the top of the file. CLAUDE.md style guide says: "Default to writing no comments" and "Never write multi-paragraph docstrings or multi-line comment blocks." Same applies to useTerminal.ts (line 1), TerminalView.tsx (line 1), and terminal-manager.ts (line 1). [fixable]

`frontend/src/hooks/useTerminal.ts`

The terminal feature works end-to-end but has critical gaps: PTY processes are never cleaned up on disconnect (orphan leak), terminal operations lack session ownership checks (any authed user can control any terminal), and the server's full environment is passed to spawned shells. No tests exist despite the project's strict TDD policy.

🔵 unsafe_assumptions (L59): The terminal hook opens its own dedicated WebSocket connection rather than reusing the existing multiplexed MitzoConnection WS. This means each terminal view creates a separate authenticated WS, doubling the connection count. The comment says it "shares the same WS endpoint" but it doesn't share the connection — it's a second parallel WS. Consider using the existing multiplexed connection from @mitzo/client.

dimakis · 2026-06-25T12:31:57Z

+  return result;
+}
+
+export function destroySessionTerminals(sessionId: string): number {


🔴 bugs: destroySessionTerminals() is exported but never called anywhere — not on session close, not on WS disconnect, not on server shutdown. When a WebSocket disconnects or a session is closed, the PTY processes remain alive as orphans, leaking processes and memory indefinitely. The onData callback will also keep trying to send to a dead transport. Either handleSessionClose in ws-handler-v2.ts or the WS close handler in index.ts should call destroySessionTerminals(sessionId). [fixable]

dimakis · 2026-06-25T12:31:57Z

+  const rows = opts?.rows ?? 24;
+  const shell = getDefaultShell();
+
+  const proc = pty.spawn(shell, [], {


🟡 unsafe_assumptions: The spawned PTY inherits the entire server process.env via ...process.env. This exposes sensitive server-side variables (API keys, JWT secrets, database credentials, ANTHROPIC_API_KEY, etc.) to the interactive shell. Consider using an explicit allowlist of safe environment variables or at minimum filtering out known secrets. [fixable]

dimakis · 2026-06-25T12:31:57Z

+  return process.env.SHELL || (process.platform === 'win32' ? 'powershell.exe' : '/bin/zsh');
+}
+
+export function createTerminal(


🟡 unsafe_assumptions: No limit on the number of terminals a client can create. A single authenticated user can spam terminal_create messages and spawn unlimited PTY processes, exhausting server resources (PIDs, memory, file descriptors). Add a per-session or global terminal cap. [fixable]

dimakis · 2026-06-25T12:31:57Z

+    onExit: null,
+  };
+
+  proc.onData((data) => {


🟡 bugs: PTY onData fires immediately after proc.onData() is registered, but managed.onData is null until setTerminalCallbacks() is called later (after the WS response round-trip). Any output the shell produces between spawn and callback registration (e.g., shell init scripts, .zshrc output) is silently dropped. Consider buffering output until callbacks are attached. [fixable]

dimakis · 2026-06-25T12:31:57Z


+// ─── Terminal handlers ──────────────────────────────────────────────────────
+
+export function handleTerminalCreate(


🔴 unsafe_assumptions: handleTerminalCreate does not verify that the requesting connection owns the session. Any authenticated client can send terminal_create with an arbitrary sessionId and get a shell in that session's worktree cwd. Compare with handleSessionClose which checks getOwnerConnection(found.clientId) !== connectionId before acting. The same ownership check is missing from handleTerminalInput, handleTerminalResize, and handleTerminalDestroy — any client can write to, resize, or kill any terminal by guessing the ID. [fixable]

dimakis · 2026-06-25T12:31:57Z

+      const sessionMeta = ctx.eventStore.getSession(msg.sessionId);
+      const cwd = sessionMeta?.cwd || BASE_REPO || process.cwd();
+
+      try {


🟡 bugs: The cwd fallback chain sessionMeta?.cwd || BASE_REPO || process.cwd() falls through to process.cwd() when both are falsy. BASE_REPO is process.env.REPO_PATH || '' — an empty string is falsy, so if REPO_PATH is unset and the session has no cwd, the terminal spawns in the server's working directory. This is likely fine in practice (REPO_PATH is required) but the empty-string case is a silent fallback that could surprise. [fixable]

dimakis · 2026-06-25T12:31:57Z

+
+  // Connect on mount
+  useEffect(() => {
+    connect();


🟡 bugs: The useEffect that calls connect() and returns destroy() depends on [connect, destroy]. But connect is memoized with [sessionId, cols, rows, send] — since cols/rows use default values (80/24), any future change to these (e.g., from a resize) would recreate connect, tearing down and reconnecting the WS. More immediately, destroy creates a new function identity each render because send is stable but destroy captures send — this is actually stable, but the connect/destroy cycle on mount/unmount could race if React strict-mode double-mounts. [fixable]

dimakis · 2026-06-25T12:31:57Z

@@ -0,0 +1,165 @@
+/**


🔵 style: Multi-line docstring comment block at the top of the file. CLAUDE.md style guide says: "Default to writing no comments" and "Never write multi-paragraph docstrings or multi-line comment blocks." Same applies to useTerminal.ts (line 1), TerminalView.tsx (line 1), and terminal-manager.ts (line 1). [fixable]

dimakis · 2026-06-25T12:31:57Z

+    }
+  }, []);
+
+  const connect = useCallback(() => {


🔵 unsafe_assumptions: The terminal hook opens its own dedicated WebSocket connection rather than reusing the existing multiplexed MitzoConnection WS. This means each terminal view creates a separate authenticated WS, doubling the connection count. The comment says it "shares the same WS endpoint" but it doesn't share the connection — it's a second parallel WS. Consider using the existing multiplexed connection from @mitzo/client.

…bounds - 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 <noreply@anthropic.com>

dimakis