feat: multi-session terminal frontend UI + tests (PR 3)

- Add TerminalSessionTabs component with status dots, rename, close, max-5 limit - Add 7 component tests for tab rendering, selection, close, rename - TerminalComponent: sessionId prop, forwardRef with fit() method - TerminalPage: multi-session orchestration, tab switching, auto-create default - Fullscreen mode: Alt+Shift+F toggle, auto-hide tabs, Esc exit - Keyboard shortcuts: Alt+Shift+N/W/ArrowLeft/ArrowRight/R - Add CSS for tabs, fullscreen, mobile responsive - Update useTerminalSessions hook for session CRUD - terminal_manager.py: lookup by internal session_id fallback Quality gates: tsc --noEmit clean, vitest (7/7 new tests passed), pytest (182 passed)
2026-05-28 13:35:45 +02:00
parent 0b35ae3bf0
commit 62d1bdc462
26 changed files with 4589 additions and 775 deletions
@@ -0,0 +1,256 @@
+# SDD Explore: Multi-Session Terminal UX
+
+## Executive Summary
+
+The codebase has a well-built persistent terminal foundation from the `persistent-terminal-sessions` change. `TerminalManager` currently tracks exactly one `TerminalSession` per `instance_id` in an in-memory dict. `TerminalSession` already supports WebSocket attach/detach, circular output buffer replay, idle timeout, and process lifecycle management.
+
+Implementing multi-session terminal support is a **moderate-complexity, medium-risk** change. The core backend refactor is straightforward: change the session tracking key from `instance_id` to `(instance_id, session_id)` and update the WebSocket endpoint to accept a `session_id`. The frontend work is more involved: designing a tabbed session UI that works on both desktop and mobile, handling session creation/switching/closing, and integrating with the existing `MobileTerminalWrapper`.
+
+No database schema change is **strictly required** for an MVP—sessions can remain purely in-memory with the same idle-timeout cleanup. However, adding a `terminal_sessions` table would provide cross-API-restart persistence, session auditability, and a foundation for future features like session history or named sessions.
+
+## Current Architecture (as explored)
+
+### Backend
+- **`TerminalManager`** (`apps/api/src/services/terminal_manager.py`):
+  - `self._sessions: dict[str, TerminalSession]` keyed by `instance_id` string.
+  - `get_or_create_session(instance_id, container_id, startup_command)` — returns the single existing session or creates a new one.
+  - `attach_websocket(session, websocket)` — detaches any *existing* WebSocket connections on that session (closes them with code 4000) before attaching the new one. This enforces single-active-client per session.
+  - `reset_session(instance_id, container_id, ...)` — kills the existing session and creates a new one.
+  - Idle check loop every 60s; sessions with no WebSockets attached for 30 minutes are cleaned up.
+- **`TerminalSession`** (`apps/api/src/services/terminal_session.py`):
+  - Already has a `session_id: str` field (UUID) but it is not used as a lookup key.
+  - Manages one `docker exec` PTY process per session.
+  - Circular buffer (10KB) for output replay.
+  - Tracks `self._websockets: set[Any]` for attached connections.
+- **`api/terminal.py`** (`apps/api/src/api/terminal.py`):
+  - WebSocket endpoint: `/ws/tool-instances/{instance_id}/terminal`
+  - Authenticates user, verifies instance ownership/running state, then calls `terminal_manager.get_or_create_session()`.
+  - Supports JSON control messages: `resize`, `reset`.
+  - POST endpoint: `/projects/{project_id}/repositories/{repo_id}/instances/{instance_id}/terminal/reset` — resets the single session.
+- **Database**:
+  - No `terminal_sessions` table exists. Terminal sessions are purely in-memory.
+  - `ToolInstance` model (`apps/api/src/models/tool_instance.py`) has no terminal-related fields.
+
+### Frontend
+- **`TerminalComponent`** (`apps/web/src/components/terminal.tsx`):
+  - Single xterm.js terminal per component.
+  - One WebSocket connection to `/ws/tool-instances/{instance_id}/terminal`.
+  - Handles reconnect with exponential backoff (max 3 attempts).
+  - Font size persisted globally in `localStorage` under key `terminal-font-size`.
+  - Copy/paste buttons on mobile only.
+  - Status indicator: connecting, connected, disconnected, error, resetting.
+- **`TerminalPage`** (`apps/web/src/pages/terminal.tsx`):
+  - Desktop: renders one `TerminalComponent` inside a page shell.
+  - Mobile: renders `MobileTerminalWrapper` which composes `MobileTerminalHeader`, `TerminalComponent`, `SpecialKeysStrip`, and `SpecialKeysPanel`.
+- **`MobileTerminalWrapper`** (`apps/web/src/components/mobile-terminal-wrapper.tsx`):
+  - Already handles auto-hide header, virtual keyboard height, special keys, and mobile viewport detection.
+  - Manages terminal ref callbacks (`sendData`, `connectionStatus`, `focusInput`, `changeFontSize`).
+
+### Prior Art
+- **`persistent-terminal-sessions`** (fully implemented):
+  - Sessions survive WebSocket disconnections.
+  - Buffer replay on reconnect.
+  - Idle timeout cleanup.
+  - Reset functionality.
+- **`mobile-terminal-ux`** (mostly implemented):
+  - Mobile fullscreen terminal with collapsible chrome.
+  - Special keys toolbar.
+  - Dynamic viewport handling for virtual keyboard.
+
+## Architecture Options for Multi-Session
+
+### Option A: In-Memory Multi-Session (MVP)
+- Change `TerminalManager._sessions` to `dict[tuple[str, str], TerminalSession]` keyed by `(instance_id, session_id)`.
+- Add `create_session(instance_id, container_id, ...)` that always creates a new session.
+- Keep `get_or_create_session()` for backward compatibility (returns the "default" or only session).
+- Add `get_sessions_for_instance(instance_id) -> list[TerminalSession]`.
+- Add `close_session(instance_id, session_id)` to kill a specific session.
+- **Tradeoffs**: Simplest, no DB migration, survives existing patterns. Loses sessions on API restart.
+
+### Option B: Database-Backed Session Metadata
+- Create `terminal_sessions` table:
+  ```sql
+  id UUID PRIMARY KEY,
+  instance_id UUID FK(tool_instances.id, ondelete=CASCADE),
+  session_name VARCHAR(255),
+  status VARCHAR(50), -- active, idle, closed
+  created_at TIMESTAMPTZ,
+  last_activity_at TIMESTAMPTZ,
+  closed_at TIMESTAMPTZ
+  ```
+- `TerminalManager` still keeps `TerminalSession` objects in memory, but creates/updates DB rows on lifecycle events.
+- **Tradeoffs**: Enables cross-restart persistence, session history, named sessions, and auditability. Adds migration and async DB overhead to hot paths.
+
+### Option C: Hybrid (Recommended)
+- In-memory active sessions for performance.
+- DB table for metadata, created on session start, updated on activity/close.
+- On API restart, sessions are gone (no process resurrection), but metadata remains for history.
+- **Tradeoffs**: Best of both worlds. Slightly more complex than Option A but much simpler than full persistence.
+
+### Decision Matrix
+
+| Criterion | Option A | Option B | Option C |
+|-----------|----------|----------|----------|
+| Implementation complexity | Low | Medium | Medium |
+| DB migration required | No | Yes | Yes |
+| Cross-restart persistence | No | Yes (full) | Metadata only |
+| Resource auditability | No | Yes | Yes |
+| Performance | Best | Good (cacheable) | Best |
+| Recommended for MVP | **Yes** | No | **Preferred** |
+
+## WebSocket Protocol Options
+
+### Option 1: URL Path Segment (Recommended)
+```
+/ws/tool-instances/{instance_id}/terminal/{session_id}
+```
+- Clean, RESTful, easy to route in FastAPI.
+- Default session can use a reserved ID like `default` or keep `/terminal` as an alias.
+- **Tradeoff**: Breaks existing hardcoded URLs; needs backward-compatibility route.
+
+### Option 2: Query Parameter
+```
+/ws/tool-instances/{instance_id}/terminal?session_id=...
+```
+- Easier to add without changing route structure.
+- Less idiomatic for WebSocket APIs.
+- **Tradeoff**: Query params in WebSocket URLs can be inconsistently supported by proxies.
+
+### Option 3: First-Message JSON Payload
+- Client connects to `/terminal`, then sends `{"type": "attach", "session_id": "..."}`.
+- Server must hold the connection in limbo until the attach message arrives.
+- **Tradeoff**: More complex state machine; harder to reject invalid sessions early.
+
+**Recommendation**: Option 1 with a backward-compatible fallback:
+- `/ws/tool-instances/{instance_id}/terminal` → attaches to the "default" session (existing behavior).
+- `/ws/tool-instances/{instance_id}/terminal/{session_id}` → attaches to the specified session.
+
+## Frontend UX Design Options
+
+### Session Presentation: Tabs vs Panes
+
+| Feature | Tabs | Panes (Split) |
+|---------|------|---------------|
+| Desktop UX | Good | Excellent (tmux-like) |
+| Mobile UX | Good | Poor (too cramped) |
+| Implementation | Medium | High |
+| Accessibility | Good | Complex |
+| Recommendation | **Preferred** | Future enhancement |
+
+**Decision**: Start with tabs. A split-pane layout can be added later as an advanced feature without breaking the tab model.
+
+### Tab Bar Design
+- Position: Above the terminal container on desktop; integrated into `MobileTerminalHeader` on mobile.
+- Contents:
+  - Session name (auto-named "Session 1", "Session 2", or custom).
+  - Status dot (connecting, connected, error).
+  - Close button (×) on hover/active.
+  - New tab button (+).
+- Overflow: Horizontal scroll on mobile; wrap or scroll on desktop.
+
+### Fullscreen Mode
+- **Behavior**: Toggle hides all page chrome (header, sidebar, tab bar can optionally be shown as a minimal overlay).
+- **Trigger**: `Ctrl+Shift+F` or UI button.
+- **Mobile**: Should integrate with existing mobile fullscreen behavior (already hides AppShell). Fullscreen on mobile could mean hiding the special-keys strip too, with a gesture to reveal.
+- **Exit**: `Esc` or UI button.
+
+### Keyboard Shortcuts
+
+| Shortcut | Action | Notes |
+|----------|--------|-------|
+| `Ctrl+Shift+N` | New session | May conflict with browser "New window" on some platforms. Consider `Ctrl+Shift+T` if not used for "Reopen tab". |
+| `Ctrl+Shift+W` | Close current session | Conflicts with browser "Close window". May need `Ctrl+Shift+D` or accept override with `preventDefault()`. |
+| `Ctrl+Shift+F` | Toggle fullscreen | Safe, no major browser conflict. |
+| `Ctrl+Shift+T` | Toggle tab bar visibility | Conflicts with "Reopen closed tab" in browsers. Consider `Ctrl+Shift+B` or `Ctrl+Shift+~`. |
+
+**Recommendation**: Use `preventDefault()` aggressively and show a shortcuts help modal (e.g., `Ctrl+Shift+/` or `?`).
+
+### Session Naming
+- **Auto-name**: "Session 1", "Session 2", etc. based on creation order.
+- **Custom name**: Editable by double-clicking the tab. Persisted in DB if Option B/C, or in-memory only for Option A.
+- **Default session**: The first session created for an instance can be unnamed or named "Default".
+
+### Reset/Kill Semantics
+Current behavior: "Reset Terminal" kills the single session and starts fresh.
+
+With multi-session:
+- **Close Session** (× on tab): Kills the `docker exec` process and removes the session.
+- **New Session** (+ on tab bar): Creates a new session and switches to it.
+- **Reset Session** (in menu): Same as current reset but scoped to the active session.
+- **Reset All** (optional, in menu): Kill all sessions for the instance and recreate a default one.
+
+### Font Size Persistence
+- Currently global (`localStorage` key `terminal-font-size`).
+- With multi-session, users may want different font sizes per session (e.g., larger for presentations, smaller for logs).
+- **Options**:
+  1. Keep global (simplest, no change).
+  2. Per-session font size (stored in session state or DB).
+  3. Per-instance font size.
+- **Recommendation**: Keep global for MVP. Per-session font size is a nice-to-have that adds complexity.
+
+### Status Per Session
+- Each tab shows a status dot.
+- Possible statuses: `connecting` (pulsing), `connected` (green), `disconnected` (yellow), `error` (red), `closed` (gray).
+- The terminal component already tracks these statuses; they just need to be surfaced at the tab level.
+
+## Database Schema Recommendation (Option C)
+
+```python
+class TerminalSessionModel(UUIDPrimaryKeyMixin, TimestampMixin, Base):
+    __tablename__ = "terminal_sessions"
+
+    instance_id: Mapped[uuid.UUID] = mapped_column(
+        UUID(), ForeignKey("tool_instances.id", ondelete="CASCADE"), nullable=False
+    )
+    name: Mapped[str | None] = mapped_column(String(255), nullable=True)
+    status: Mapped[str] = mapped_column(
+        String(50), nullable=False, default="active"
+    )
+    # Not storing process PID here — that's runtime-only in TerminalManager
+    created_at: Mapped[datetime] = mapped_column(
+        DateTime(timezone=True), nullable=False, default=datetime.utcnow
+    )
+    last_activity_at: Mapped[datetime | None] = mapped_column(
+        DateTime(timezone=True), nullable=True
+    )
+    closed_at: Mapped[datetime | None] = mapped_column(
+        DateTime(timezone=True), nullable=True
+    )
+```
+
+**Migration**: New alembic revision adding `terminal_sessions` table.
+
+## Open Questions Needing User/Product Decisions
+
+1. **Max sessions per instance?** Suggest 5 for MVP to prevent resource exhaustion.
+2. **Should we persist sessions across API restarts?** Option A = no; Option C = metadata only. Product call.
+3. **Tab vs Pane UI?** Strongly recommend tabs for MVP. Panes as future work.
+4. **Keyboard shortcuts — override browser defaults?** `Ctrl+Shift+W` closes browser window. We can `preventDefault()` but should warn users.
+5. **Should the existing `/terminal` endpoint remain as a default-session alias?** Yes for backward compatibility, but confirm.
+6. **Session idle timeout per session or global per instance?** Currently per session. Keep per session.
+7. **Should font size be global, per-instance, or per-session?** Recommend global for MVP.
+8. **Copy/paste on desktop — any gaps?** Current desktop relies on native xterm.js copy/paste (`Ctrl+C`/`Ctrl+V` with selection). This is standard and sufficient. Mobile already has buttons.
+
+## Risks and Feasibility Assessment
+
+| Risk | Likelihood | Impact | Mitigation |
+|------|------------|--------|------------|
+| Resource exhaustion from too many docker exec processes | Medium | High | Enforce max sessions per instance (5). Idle timeout already exists. |
+| Mobile UX degradation from tab bar clutter | Medium | Medium | Integrate tabs into existing `MobileTerminalHeader` auto-hide. Limit visible tabs, overflow scroll. |
+| Backward compat breakage from URL change | Low | Medium | Keep `/terminal` as default-session alias. |
+| Concurrent WebSocket policy bugs | Medium | High | Ensure "close existing" only applies within same `(instance_id, session_id)`, not across sessions. |
+| Scope creep (panes, detachable windows) | High | Medium | Explicitly exclude split panes and detachable windows from MVP. |
+
+## Feasibility: Green/Yellow/Red
+
+**Yellow-Green**. The backend changes are well-scoped and build on solid existing infrastructure. The frontend tab UI is the largest unknown, especially mobile integration, but the existing `MobileTerminalWrapper` provides a good foundation. No external dependencies needed.
+
+## Recommended Next Step
+
+**Proceed to `design` phase** after resolving these scoping decisions:
+1. Choose Option A or C for session storage (recommend Option C).
+2. Confirm max sessions limit (recommend 5).
+3. Confirm tab-only UI for MVP (no panes).
+4. Confirm backward-compatible WebSocket URL strategy.
+
+Then write `design.md` with concrete decisions and `tasks.md` with implementation steps.