62d1bdc462
- 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)
257 lines
14 KiB
Markdown
257 lines
14 KiB
Markdown
# 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.
|