Merge branch 'feat/tool-definition-manifest' into dev
Conflicts resolved: - models/__init__.py: kept both TerminalSessionModel (from dev) and ToolDefinitionManifest (from feature branch) - alembic migration: kept full migration (already applied to DB) - openspec/config.yaml: kept full config with SDD settings
This commit is contained in:
Alex Blank
@@ -0,0 +1,3 @@
|
||||
name: multi-session-terminal-ux
|
||||
status: exploring
|
||||
started_at: 2026-05-28
|
||||
@@ -0,0 +1,74 @@
|
||||
# Apply Report: PR 1 – Database + Backend Core for Multi-Session Terminal UX
|
||||
|
||||
## Summary
|
||||
|
||||
Implemented the database schema, Alembic migration, TerminalManager multi-session core, and TerminalSession name/status tracking for the multi-session terminal UX feature. All changes are backward-compatible with the existing single-session `/terminal` WebSocket endpoint.
|
||||
|
||||
### Key Changes
|
||||
|
||||
1. **Database Schema** – Added `terminal_sessions` table with `UUIDPrimaryKeyMixin` + `TimestampMixin`, storing `instance_id`, `name`, `status`, `last_activity_at`, and `closed_at`.
|
||||
2. **Alembic Migration** – Created migration `2026_05_28_add_terminal_sessions` (down-revision from `20260527_160017_add_pi_agent`).
|
||||
3. **TerminalSession** – Added `name` (auto-generated as "Session N"), `status` field (`active`/`resetting`/`closed`), and updated `reset()`/`close()` to set status appropriately.
|
||||
4. **TerminalManager** – Migrated `_sessions` dict from `dict[str, TerminalSession]` to `dict[tuple[str, str], TerminalSession]`. Added `create_session()`, `get_session()`, `get_sessions_for_instance()`, `close_session()`, and updated `reset_session()` to accept an optional `session_id`. Preserved `get_or_create_session()` for backward compatibility (uses `"default"` session_id). Idle cleanup now operates on composite keys and fires DB status updates asynchronously.
|
||||
5. **Tests** – Created 7 unit tests covering session creation, max-5 enforcement, filtering, close/removal, WebSocket isolation, default session keying, and idle cleanup DB updates.
|
||||
|
||||
## Files Created
|
||||
|
||||
- `apps/api/src/models/terminal_session.py`
|
||||
- `apps/api/alembic/versions/2026_05_28_add_terminal_sessions_table.py`
|
||||
- `apps/api/tests/services/test_terminal_manager_multi.py`
|
||||
|
||||
## Files Modified
|
||||
|
||||
- `apps/api/src/models/__init__.py` – Imported `TerminalSessionModel`
|
||||
- `apps/api/src/main.py` – Imported `TerminalSessionModel` for Alembic model discovery
|
||||
- `apps/api/src/services/terminal_manager.py` – Full refactor to composite-key session management with DB fire-and-forget helpers
|
||||
- `apps/api/src/services/terminal_session.py` – Added `name`, `status`, `_instance_counters`, and status transitions
|
||||
|
||||
## Test Results
|
||||
|
||||
### New Tests (7/7 passed)
|
||||
|
||||
```
|
||||
$ cd apps/api && python -m pytest tests/services/test_terminal_manager_multi.py -v
|
||||
|
||||
tests/services/test_terminal_manager_multi.py::test_create_session_increases_count PASSED
|
||||
tests/services/test_terminal_manager_multi.py::test_create_session_enforces_max_5 PASSED
|
||||
tests/services/test_terminal_manager_multi.py::test_get_sessions_for_instance_filters_by_instance PASSED
|
||||
tests/services/test_terminal_manager_multi.py::test_close_session_removes_from_dict PASSED
|
||||
tests/services/test_terminal_manager_multi.py::test_attach_websocket_only_closes_same_session PASSED
|
||||
tests/services/test_terminal_manager_multi.py::test_default_session_keyed_separately PASSED
|
||||
tests/services/test_terminal_manager_multi.py::test_idle_cleanup_updates_db_status PASSED
|
||||
|
||||
======================== 7 passed, 4 warnings in 0.11s =========================
|
||||
```
|
||||
|
||||
### Full Suite (no regressions)
|
||||
|
||||
```
|
||||
$ cd apps/api && python -m pytest tests/ -q
|
||||
|
||||
51 failed, 174 passed, 6 warnings in 15.96s
|
||||
```
|
||||
|
||||
- **Baseline failures**: 51 (pre-existing, unchanged by this PR)
|
||||
- **New passes**: +7 (from `test_terminal_manager_multi.py`)
|
||||
- **No new failures introduced**
|
||||
|
||||
## Deviations from Design
|
||||
|
||||
1. **Duplicate `created_at` column** – The design spec and its Alembic snippet listed `created_at` twice (once explicitly, once from `TimestampMixin`). I removed the explicit `created_at` from the model and migration, relying on `TimestampMixin` which provides `server_default=func.now()`.
|
||||
2. **DB write implementation** – The design showed DB writes inside `TerminalManager` but didn't specify the exact async pattern. I implemented them as `asyncio.create_task`-wrapped coroutines using `SessionLocal()` so they are non-blocking. Unit tests mock `_mark_closed_in_db` and `_insert_db_session_row` to verify calls without needing a live DB.
|
||||
3. **`get_or_create_session` auto-name** – The design said default session should count toward the 5-session limit. The current implementation does count it, but `get_or_create_session` creates the default session outside the `create_session` path (to preserve backward compat). Future REST endpoints can enforce the limit at the API layer before calling either path.
|
||||
|
||||
## Blockers / Risks
|
||||
|
||||
- **Global singleton test isolation** – `TerminalManager` is still a global singleton (`terminal_manager = TerminalManager()`). The unit tests create fresh instances via the `manager` fixture, but integration tests that import the global may need care to reset state between tests.
|
||||
- **DB fire-and-forget in tests** – The aiosqlite background thread emits `RuntimeError: Event loop is closed` warnings when the test event loop tears down before the fire-and-forget DB task completes. This is harmless in tests but worth monitoring.
|
||||
- **Migration head** – The migration chains from `20260527_160017_add_pi_agent`. If a new migration lands on `dev` before this PR merges, the `down_revision` must be updated.
|
||||
|
||||
## Next Recommended Action
|
||||
|
||||
1. **Task 5 (WebSocket endpoint + REST API)** – Implement the new `/ws/tool-instances/{instance_id}/terminal/{session_id}` WebSocket route and the REST endpoints (`GET/POST/DELETE .../terminal/sessions`) in `apps/api/src/api/terminal.py`. Extract the shared auth/validation/I/O loop into `_handle_terminal_websocket()` as specified in the design.
|
||||
2. **Run migration in a staging environment** – Verify `alembic upgrade head` applies cleanly and `downgrade` reverses without data loss.
|
||||
3. **Integration tests for WebSocket multi-session** – Create `apps/api/tests/api/test_terminal_ws_multi.py` to validate concurrent session isolation and the default-session alias.
|
||||
@@ -0,0 +1,40 @@
|
||||
# PR 3: Frontend Multi-Session Terminal UI
|
||||
|
||||
## Summary
|
||||
Implemented the frontend UI for multi-session terminal support: tabbed session management, fullscreen mode, keyboard shortcuts, and mobile integration.
|
||||
|
||||
## Files Created
|
||||
- `apps/web/src/components/terminal-session-tabs.tsx` — Tab bar component with rename, close, status dots, overflow scroll
|
||||
- `apps/web/src/components/terminal-session-tabs.test.tsx` — 7 passing component tests
|
||||
|
||||
## Files Modified
|
||||
- `apps/web/src/components/terminal.tsx` — Added `sessionId` prop, `TerminalRef` with `fit()`, `forwardRef` wrapper
|
||||
- `apps/web/src/pages/terminal.tsx` — Multi-session orchestration with tabs, fullscreen, keyboard shortcuts
|
||||
- `apps/web/src/hooks/use-terminal-sessions.ts` — Hook for session CRUD + state management
|
||||
- `apps/web/src/api/terminal.ts` — API client for terminal session endpoints
|
||||
- `apps/web/src/styles.css` — Terminal tab styles, fullscreen mode, mobile responsive
|
||||
- `apps/api/src/services/terminal_manager.py` — Added lookup by internal session_id fallback
|
||||
|
||||
## Acceptance Criteria
|
||||
- [x] TerminalComponent accepts optional sessionId prop
|
||||
- [x] WS URL includes sessionId when provided
|
||||
- [x] TerminalSessionTabs renders sessions with status dots
|
||||
- [x] Double-click to rename, click × to close (with confirm)
|
||||
- [x] New session (+) button, disabled at 5 sessions
|
||||
- [x] Tab switching updates active terminal, calls fit()
|
||||
- [x] Fullscreen toggle (Alt+Shift+F), exit via Esc
|
||||
- [x] Keyboard shortcuts: Alt+Shift+N (new), W (close), ←/→ (navigate), R (reset)
|
||||
- [x] Auto-creates default session if none exist
|
||||
- [x] Closing last session auto-creates new default
|
||||
- [x] Mobile: tabs in compact strip, same keyboard shortcuts
|
||||
- [x] No browser shortcuts overridden (uses Alt+Shift, not Ctrl+Shift)
|
||||
|
||||
## Quality Gates
|
||||
- TypeScript typecheck: ✅ clean
|
||||
- Frontend tests: ✅ 7/7 terminal-session-tabs tests passing
|
||||
- Backend tests: ✅ 182 passed, 51 pre-existing failures (no regressions)
|
||||
- Lint: ✅ 0 errors
|
||||
|
||||
## Blockers / Deviations
|
||||
- MobileTerminalWrapper was not fully integrated with session tabs due to complexity. Mobile path uses inline tab rendering instead.
|
||||
- This is acceptable for MVP; full mobile integration can be refined in follow-up.
|
||||
@@ -0,0 +1,867 @@
|
||||
# SDD Design: Multi-Session Terminal UX
|
||||
|
||||
## Architecture Overview
|
||||
|
||||
The multi-session terminal extends the existing persistent-session foundation to support up to 5 concurrent terminal sessions per tool instance. The architecture uses a **hybrid storage model**: active PTY processes and WebSocket routing live in-memory (performance-critical path), while session metadata (name, status, timestamps) persists in a new `terminal_sessions` database table.
|
||||
|
||||
### High-Level Flow
|
||||
|
||||
```
|
||||
┌─────────────────────────────────────────────────────────────────────────────┐
|
||||
│ Frontend (React) │
|
||||
│ ┌──────────────────┐ ┌──────────────────┐ ┌──────────────────┐ │
|
||||
│ │ TerminalSession │ │ TerminalSession │ │ TerminalSession │ ... │
|
||||
│ │ Tabs (Desktop) │ │ Tabs (Mobile) │ │ FullscreenMgr │ │
|
||||
│ └────────┬─────────┘ └────────┬─────────┘ └────────┬─────────┘ │
|
||||
│ │ │ │ │
|
||||
│ ┌────────▼──────────────────────▼──────────────────────▼─────────┐ │
|
||||
│ │ TerminalSessionManager │ │
|
||||
│ │ (React state: sessions[], activeSessionId) │ │
|
||||
│ └────────┬──────────────────────┬──────────────────────┬─────────┘ │
|
||||
│ │ │ │ │
|
||||
│ ┌────────▼─────────┐ ┌────────▼─────────┐ ┌────────▼─────────┐ │
|
||||
│ │ TerminalComponent│ │ TerminalComponent│ │ TerminalComponent│ ... │
|
||||
│ │ (xterm.js + WS) │ │ (xterm.js + WS) │ │ (xterm.js + WS) │ │
|
||||
│ └────────┬─────────┘ └────────┬─────────┘ └────────┬─────────┘ │
|
||||
└───────────┼─────────────────────┼─────────────────────┼────────────────────┘
|
||||
│ │ │
|
||||
▼ ▼ ▼
|
||||
┌─────────────────────────────────────────────────────────────┐
|
||||
│ FastAPI Backend │
|
||||
│ ┌──────────────────┐ ┌──────────────────┐ ┌────────────┐ │
|
||||
│ │ /terminal │ │ /terminal/{sid} │ │ REST /ses- │ │
|
||||
│ │ (default alias) │ │ (specific sess) │ │ sions │ │
|
||||
│ └────────┬─────────┘ └────────┬─────────┘ └─────┬──────┘ │
|
||||
│ │ │ │ │
|
||||
│ ┌────────▼──────────────────────▼────────────────────▼─────┐ │
|
||||
│ │ TerminalManager │ │
|
||||
│ │ dict[(instance_id, session_id)] → TerminalSession │ │
|
||||
│ └────────┬──────────────────────┬──────────────────────────┘ │
|
||||
│ │ │ │
|
||||
│ ┌────────▼─────────┐ ┌────────▼─────────┐ │
|
||||
│ │ TerminalSession │ │ TerminalSession │ ... │
|
||||
│ │ (PTY + docker │ │ (PTY + docker │ │
|
||||
│ │ exec process) │ │ exec process) │ │
|
||||
│ └────────┬─────────┘ └────────┬─────────┘ │
|
||||
│ │ │ │
|
||||
│ ┌────────▼──────────────────────▼───────────────────────────┐│
|
||||
│ │ TerminalSessionModel (DB) ││
|
||||
│ │ instance_id | name | status | created_at | closed_at ││
|
||||
│ └───────────────────────────────────────────────────────────┘│
|
||||
└───────────────────────────────────────────────────────────────┘
|
||||
```
|
||||
|
||||
### Key Principles
|
||||
|
||||
- **One WebSocket per session**: Each `TerminalComponent` opens its own WebSocket to its specific `session_id`. Inactive sessions keep their WebSocket open to preserve scrollback and real-time output.
|
||||
- **Max 5 sessions per instance**: Enforced in `TerminalManager.create_session()` and validated in the REST endpoint.
|
||||
- **Default session alias**: `/ws/tool-instances/{instance_id}/terminal` maps to the single legacy session (or the first/only active session) for backward compatibility.
|
||||
- **Tab-only UI**: No split panes for MVP. Sessions are presented as tabs on desktop and as a scrollable tab strip integrated into the mobile header area.
|
||||
|
||||
---
|
||||
|
||||
## Backend Design
|
||||
|
||||
### 1. TerminalManager Changes
|
||||
|
||||
**File**: `apps/api/src/services/terminal_manager.py`
|
||||
|
||||
#### Session Key Change
|
||||
|
||||
```python
|
||||
# BEFORE
|
||||
self._sessions: dict[str, TerminalSession] = {} # keyed by instance_id
|
||||
|
||||
# AFTER
|
||||
self._sessions: dict[tuple[str, str], TerminalSession] = {} # keyed by (instance_id, session_id)
|
||||
```
|
||||
|
||||
#### New / Modified Methods
|
||||
|
||||
| Method | Signature | Behavior |
|
||||
|--------|-----------|----------|
|
||||
| `create_session` | `(instance_id, container_id, startup_command=None, name=None) → TerminalSession` | Creates a new `TerminalSession`, starts it, stores under `(instance_id, session_id)`, and inserts a `TerminalSessionModel` DB row. Enforces max 5 sessions. |
|
||||
| `get_or_create_session` | *(preserved)* | **Backward-compat only.** Returns existing default session or creates one with `session_id="default"`. Called by the legacy `/terminal` WebSocket endpoint. |
|
||||
| `get_session` | `(instance_id, session_id) → TerminalSession \| None` | Lookup by composite key. |
|
||||
| `get_sessions_for_instance` | `(instance_id) → list[TerminalSession]` | Returns all in-memory sessions for an instance. |
|
||||
| `close_session` | `(instance_id, session_id) → None` | Kills the PTY process, removes from `_sessions`, updates DB row `status=closed`, `closed_at=now()`. |
|
||||
| `reset_session` | *(modified)* | Now accepts an optional `session_id`. If omitted, resets the default session. |
|
||||
| `attach_websocket` | *(preserved)* | **Critical fix**: The "close existing WebSockets" logic must only close sockets **within the same `(instance_id, session_id)`**. Previously it closed all sockets for the instance. |
|
||||
|
||||
#### Default Session Behavior
|
||||
|
||||
- The first time a client hits `/ws/.../terminal` (no `session_id`), `TerminalManager` checks if a "default" session exists under key `(instance_id, "default")`.
|
||||
- If none exists, it creates one (same as `get_or_create_session`).
|
||||
- The default session counts toward the 5-session limit.
|
||||
|
||||
#### Idle Cleanup
|
||||
|
||||
```python
|
||||
async def _cleanup_idle_sessions(self) -> None:
|
||||
idle_keys = []
|
||||
for (instance_id, session_id), session in list(self._sessions.items()):
|
||||
if session.is_idle():
|
||||
idle_keys.append((instance_id, session_id))
|
||||
for key in idle_keys:
|
||||
session = self._sessions.pop(key, None)
|
||||
if session:
|
||||
await session.close()
|
||||
# Update DB status
|
||||
await self._mark_closed_in_db(key[1])
|
||||
```
|
||||
|
||||
### 2. TerminalSession Changes
|
||||
|
||||
**File**: `apps/api/src/services/terminal_session.py`
|
||||
|
||||
#### New Fields
|
||||
|
||||
```python
|
||||
class TerminalSession:
|
||||
# ... existing fields ...
|
||||
|
||||
def __init__(self, session_id: str, instance_id: uuid.UUID, container_id: str,
|
||||
startup_command: str | None = None, name: str | None = None) -> None:
|
||||
# ... existing init ...
|
||||
self.name = name or f"Session {self._next_session_number(instance_id)}"
|
||||
self.status: str = "active" # active, resetting, closed
|
||||
```
|
||||
|
||||
The `name` field is runtime-only in `TerminalSession`. Renames update the DB via REST, then the frontend uses the new name on next mount or via a lightweight WS status broadcast (optional optimization).
|
||||
|
||||
#### Status Tracking
|
||||
|
||||
- `active`: Normal operation.
|
||||
- `resetting`: Transient during `reset()` — cleared after new process starts.
|
||||
- `closed`: Set after `close()` is called.
|
||||
|
||||
### 3. WebSocket Endpoint Changes
|
||||
|
||||
**File**: `apps/api/src/api/terminal.py`
|
||||
|
||||
#### New Route (Specific Session)
|
||||
|
||||
```python
|
||||
@router.websocket("/ws/tool-instances/{instance_id}/terminal/{session_id}")
|
||||
async def terminal_websocket_specific(
|
||||
websocket: WebSocket,
|
||||
instance_id: str,
|
||||
session_id: str,
|
||||
db_session: AsyncSession = Depends(get_db_session),
|
||||
) -> None:
|
||||
...
|
||||
```
|
||||
|
||||
#### Backward-Compatible Route (Default Session)
|
||||
|
||||
```python
|
||||
@router.websocket("/ws/tool-instances/{instance_id}/terminal")
|
||||
async def terminal_websocket_default(
|
||||
websocket: WebSocket,
|
||||
instance_id: str,
|
||||
db_session: AsyncSession = Depends(get_db_session),
|
||||
) -> None:
|
||||
# Identical auth/validation logic
|
||||
# Calls terminal_manager.get_or_create_session(...) # uses "default" session_id
|
||||
# Rest of the loop is identical to specific-session endpoint
|
||||
...
|
||||
```
|
||||
|
||||
#### Refactoring
|
||||
|
||||
Both endpoints share the same auth/validation and I/O loop logic. Extract a common coroutine:
|
||||
|
||||
```python
|
||||
async def _handle_terminal_websocket(
|
||||
websocket: WebSocket,
|
||||
instance_id: str,
|
||||
session_id: str | None, # None means default
|
||||
db_session: AsyncSession,
|
||||
) -> None:
|
||||
# Shared: auth, instance lookup, tool_type fetch, session fetch/create,
|
||||
# attach_websocket, read/write/heartbeat loops, detach_websocket
|
||||
```
|
||||
|
||||
#### Control Messages (Unchanged)
|
||||
|
||||
The WebSocket control message protocol is unchanged:
|
||||
|
||||
- `{"type": "resize", "cols": 80, "rows": 24}`
|
||||
- `{"type": "reset"}` — resets the **current** session only
|
||||
|
||||
### 4. Database Schema
|
||||
|
||||
**File**: `apps/api/src/models/terminal_session.py` (new)
|
||||
|
||||
```python
|
||||
import uuid
|
||||
from datetime import datetime
|
||||
|
||||
from sqlalchemy import DateTime, ForeignKey, String
|
||||
from sqlalchemy import Uuid as UUID
|
||||
from sqlalchemy.orm import Mapped, mapped_column
|
||||
|
||||
from src.models.base import Base, TimestampMixin, UUIDPrimaryKeyMixin
|
||||
|
||||
|
||||
class TerminalSessionModel(UUIDPrimaryKeyMixin, TimestampMixin, Base):
|
||||
__tablename__ = "terminal_sessions"
|
||||
|
||||
instance_id: Mapped[uuid.UUID] = mapped_column(
|
||||
UUID(),
|
||||
ForeignKey("tool_instances.id", ondelete="CASCADE"),
|
||||
nullable=False,
|
||||
index=True,
|
||||
)
|
||||
name: Mapped[str | None] = mapped_column(String(255), nullable=True)
|
||||
status: Mapped[str] = mapped_column(
|
||||
String(50),
|
||||
nullable=False,
|
||||
default="active",
|
||||
)
|
||||
created_at: Mapped[datetime] = mapped_column(
|
||||
DateTime(timezone=True),
|
||||
nullable=False,
|
||||
)
|
||||
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,
|
||||
)
|
||||
```
|
||||
|
||||
#### Rationale
|
||||
|
||||
- `instance_id` is indexed because lookups by instance are frequent (listing sessions, cleanup).
|
||||
- `name` is nullable; auto-generated names are stored here so they survive page reloads.
|
||||
- `status` tracks `active` vs `closed`. The `TerminalManager` updates `last_activity_at` whenever a WebSocket attaches/detaches or I/O occurs.
|
||||
- On API restart, in-memory sessions are lost, but `terminal_sessions` rows remain as metadata history. A future enhancement could resurrect sessions, but that is out of scope.
|
||||
|
||||
### 5. Alembic Migration
|
||||
|
||||
**File**: `apps/api/src/alembic/versions/XXXX_add_terminal_sessions_table.py`
|
||||
|
||||
```python
|
||||
"""Add terminal_sessions table."""
|
||||
|
||||
from alembic import op
|
||||
import sqlalchemy as sa
|
||||
|
||||
# revision identifiers, used by Alembic.
|
||||
revision = "<generated>"
|
||||
down_revision = "<previous>"
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
op.create_table(
|
||||
"terminal_sessions",
|
||||
sa.Column("id", sa.UUID(), nullable=False),
|
||||
sa.Column("instance_id", sa.UUID(), nullable=False),
|
||||
sa.Column("name", sa.String(length=255), nullable=True),
|
||||
sa.Column("status", sa.String(length=50), nullable=False),
|
||||
sa.Column("created_at", sa.DateTime(timezone=True), nullable=False),
|
||||
sa.Column("last_activity_at", sa.DateTime(timezone=True), nullable=True),
|
||||
sa.Column("closed_at", sa.DateTime(timezone=True), nullable=True),
|
||||
sa.Column("created_at", sa.DateTime(timezone=True), nullable=False), # TimestampMixin
|
||||
sa.Column("updated_at", sa.DateTime(timezone=True), nullable=False), # TimestampMixin
|
||||
sa.ForeignKeyConstraint(["instance_id"], ["tool_instances.id"], ondelete="CASCADE"),
|
||||
sa.PrimaryKeyConstraint("id"),
|
||||
)
|
||||
op.create_index(op.f("ix_terminal_sessions_instance_id"), "terminal_sessions", ["instance_id"], unique=False)
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
op.drop_index(op.f("ix_terminal_sessions_instance_id"), table_name="terminal_sessions")
|
||||
op.drop_table("terminal_sessions")
|
||||
```
|
||||
|
||||
### 6. REST API Additions
|
||||
|
||||
**File**: `apps/api/src/api/terminal.py` (same file as WebSocket endpoint)
|
||||
|
||||
All new endpoints follow the existing URL pattern: `/projects/{project_id}/repositories/{repo_id}/instances/{instance_id}/terminal/sessions`.
|
||||
|
||||
#### Endpoints
|
||||
|
||||
| Method | Path | Description |
|
||||
|--------|------|-------------|
|
||||
| `GET` | `.../instances/{instance_id}/terminal/sessions` | List sessions for an instance. Returns metadata from DB + live `has_websockets` flag by querying `TerminalManager`. |
|
||||
| `POST` | `.../instances/{instance_id}/terminal/sessions` | Create a new session. Optional body: `{ "name": "Custom Name" }`. Returns `{ session_id, name, status, created_at }`. Enforces max 5. |
|
||||
| `DELETE` | `.../instances/{instance_id}/terminal/sessions/{session_id}` | Close a specific session. Kills PTY, updates DB. Returns `{ status: "closed" }`. |
|
||||
| `POST` | `.../instances/{instance_id}/terminal/sessions/{session_id}/reset` | Reset a specific session (kill + recreate). Returns `{ session_id, name, status }`. |
|
||||
| `POST` | `.../instances/{instance_id}/terminal/sessions/{session_id}/rename` | Rename a session. Body: `{ "name": "New Name" }`. Updates DB; name reflected on next session list fetch. |
|
||||
|
||||
#### Existing Endpoint Preservation
|
||||
|
||||
| Method | Path | Behavior |
|
||||
|--------|------|----------|
|
||||
| `POST` | `.../instances/{instance_id}/terminal/reset` | **Preserved as alias.** Resets the default session (same as `POST .../sessions/default/reset`). |
|
||||
|
||||
#### Response Schema (List Sessions)
|
||||
|
||||
```json
|
||||
{
|
||||
"sessions": [
|
||||
{
|
||||
"id": "uuid",
|
||||
"name": "Session 1",
|
||||
"status": "active",
|
||||
"has_websockets": true,
|
||||
"created_at": "2026-05-28T10:00:00Z",
|
||||
"last_activity_at": "2026-05-28T10:05:00Z"
|
||||
}
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Frontend Design
|
||||
|
||||
### 1. Session Tabs Component (`TerminalSessionTabs`)
|
||||
|
||||
**File**: `apps/web/src/components/terminal-session-tabs.tsx`
|
||||
|
||||
#### Props
|
||||
|
||||
```typescript
|
||||
interface TerminalSessionTabsProps {
|
||||
sessions: TerminalSessionInfo[];
|
||||
activeSessionId: string;
|
||||
onSelect: (sessionId: string) => void;
|
||||
onClose: (sessionId: string) => void;
|
||||
onCreate: () => void;
|
||||
onRename: (sessionId: string, newName: string) => void;
|
||||
isMobile?: boolean;
|
||||
}
|
||||
|
||||
interface TerminalSessionInfo {
|
||||
id: string;
|
||||
name: string;
|
||||
status: "connecting" | "connected" | "disconnected" | "error" | "resetting";
|
||||
}
|
||||
```
|
||||
|
||||
#### Desktop Behavior
|
||||
|
||||
- Horizontal tab strip positioned **above** the terminal container.
|
||||
- Each tab shows: session name, status dot (colored), close button (×) visible on hover/active.
|
||||
- Overflow: horizontal scroll with subtle fade indicator.
|
||||
- **New session button (+)**: Fixed at the right end of the tab strip. Disabled when 5 sessions exist.
|
||||
- **Double-click to rename**: Inline `<input>` replaces tab text. `Enter` to confirm, `Escape` to cancel. Blur confirms.
|
||||
- **Close confirmation**: For sessions with an active process and WebSocket, show a lightweight inline confirm tooltip (not a full modal) to avoid friction.
|
||||
|
||||
#### Mobile Behavior
|
||||
|
||||
- Tab strip is integrated into the existing auto-hide chrome.
|
||||
- `MobileTerminalHeader` gains a `sessionTabs` render prop or child area below the title row.
|
||||
- Tabs are compact (icon + truncated name + ×). Horizontal swipe scrolls.
|
||||
- New session (+) is the rightmost item.
|
||||
- The tab strip shares the auto-hide behavior with the header (tapping the terminal toggles visibility).
|
||||
|
||||
### 2. Modified `TerminalPage`
|
||||
|
||||
**File**: `apps/web/src/pages/terminal.tsx`
|
||||
|
||||
#### State Management
|
||||
|
||||
```typescript
|
||||
interface TerminalPageState {
|
||||
sessions: TerminalSessionInfo[];
|
||||
activeSessionId: string | null;
|
||||
isFullscreen: boolean;
|
||||
isLoading: boolean;
|
||||
}
|
||||
```
|
||||
|
||||
#### Session Lifecycle
|
||||
|
||||
1. **Mount**: `useEffect` calls `GET .../terminal/sessions`. If no sessions exist, auto-creates one via `POST`.
|
||||
2. **Active session**: Only one tab is visually active. **All `TerminalComponent` instances remain mounted** but inactive ones use CSS `display: none` to preserve xterm.js scrollback and WebSocket connections.
|
||||
3. **Switch tabs**: Updates `activeSessionId`. The newly active tab's `TerminalComponent` triggers `fitAddon.fit()` via a ref callback after becoming visible (using a `useEffect` on visibility).
|
||||
|
||||
#### Render Structure
|
||||
|
||||
```tsx
|
||||
<section className={`terminal-page ${isFullscreen ? "fullscreen" : ""}`}>
|
||||
{!isFullscreen && (
|
||||
<div className="terminal-page-header">...</div>
|
||||
)}
|
||||
|
||||
<TerminalSessionTabs
|
||||
sessions={sessions}
|
||||
activeSessionId={activeSessionId}
|
||||
onSelect={setActiveSessionId}
|
||||
onClose={handleCloseSession}
|
||||
onCreate={handleCreateSession}
|
||||
onRename={handleRenameSession}
|
||||
/>
|
||||
|
||||
<div className="terminal-sessions-container">
|
||||
{sessions.map((s) => (
|
||||
<div
|
||||
key={s.id}
|
||||
className={s.id === activeSessionId ? "active" : "hidden"}
|
||||
>
|
||||
<TerminalComponent
|
||||
instanceId={instanceId}
|
||||
sessionId={s.id} // NEW PROP
|
||||
onClose={() => handleCloseSession(s.id)}
|
||||
isMobile={isMobile}
|
||||
// ... other props
|
||||
/>
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
</section>
|
||||
```
|
||||
|
||||
### 3. Modified `TerminalComponent`
|
||||
|
||||
**File**: `apps/web/src/components/terminal.tsx`
|
||||
|
||||
#### New Props
|
||||
|
||||
```typescript
|
||||
interface TerminalProps {
|
||||
instanceId: string;
|
||||
sessionId?: string; // NEW: omitted → uses default session (backward compat)
|
||||
// ... existing props
|
||||
}
|
||||
```
|
||||
|
||||
#### WebSocket URL
|
||||
|
||||
```typescript
|
||||
const wsPath = sessionId
|
||||
? `/ws/tool-instances/${instanceId}/terminal/${sessionId}`
|
||||
: `/ws/tool-instances/${instanceId}/terminal`;
|
||||
```
|
||||
|
||||
#### Reset Semantics Update
|
||||
|
||||
The component's reset button now sends `{"type": "reset"}` to its own session. The `SessionRef` loop in the backend handles resetting that specific session. After reset, the backend sends `{"type": "status", "status": "connected"}` with the new session object, and the frontend clears the terminal.
|
||||
|
||||
#### Fullscreen Awareness
|
||||
|
||||
When `TerminalPage` enters fullscreen, it passes `isFullscreen` down (via context or prop drilling). `TerminalComponent` adjusts its container height to `100vh` (minus tab strip if visible in fullscreen).
|
||||
|
||||
### 4. Mobile Integration
|
||||
|
||||
**File**: `apps/web/src/components/mobile-terminal-wrapper.tsx`
|
||||
|
||||
#### Changes
|
||||
|
||||
- Accepts `sessions`, `activeSessionId`, and tab callbacks as props from `TerminalPage`.
|
||||
- Renders `TerminalSessionTabs` between `MobileTerminalHeader` and the terminal content area.
|
||||
- The tab strip auto-hides along with the header (`useAutoHide`).
|
||||
- `MobileTerminalHeader` title is updated to show `activeSession.name` instead of generic "Terminal".
|
||||
- Fullscreen on mobile: hides the header, tab strip, and special-keys strip. A tap in the bottom-right corner (or swipe from edge) reveals the tab strip temporarily.
|
||||
|
||||
### 5. Fullscreen Mode
|
||||
|
||||
**Trigger**: UI button (maximize icon in header) or `Ctrl+Shift+F`.
|
||||
|
||||
#### Desktop Fullscreen
|
||||
|
||||
- `TerminalPage` adds `.fullscreen` class.
|
||||
- Header and page chrome are hidden (`display: none`).
|
||||
- Tab strip remains visible as a minimal overlay (semi-transparent, auto-hides after 3s of inactivity, reappears on mouse move).
|
||||
- Terminal container fills viewport.
|
||||
- Exit: `Esc` key or click exit-fullscreen button.
|
||||
|
||||
#### Mobile Fullscreen
|
||||
|
||||
- Same as desktop but also hides `SpecialKeysStrip` and `SpecialKeysPanel`.
|
||||
- A small floating handle at the bottom center reveals the tab strip and special keys on tap.
|
||||
|
||||
### 6. Keyboard Shortcuts
|
||||
|
||||
**Constraint**: Do not override browser defaults. All shortcuts use combinations that are either unassigned or safe in major browsers.
|
||||
|
||||
| Shortcut | Action | Browser Conflict? |
|
||||
|----------|--------|-------------------|
|
||||
| `Ctrl+Shift+F` | Toggle fullscreen | None major |
|
||||
| `Alt+Shift+N` | New session | None major |
|
||||
| `Alt+Shift+W` | Close current session | None major |
|
||||
| `Alt+Shift+←` / `Alt+Shift+→` | Previous / next session | None major |
|
||||
| `Alt+Shift+R` | Reset current session | None major |
|
||||
|
||||
All actions are also accessible via UI buttons. Shortcuts are registered in `TerminalPage` via a `useEffect` on `keydown` with `event.preventDefault()` only for the specific combos above.
|
||||
|
||||
### 7. Session State Management
|
||||
|
||||
**File**: `apps/web/src/hooks/use-terminal-sessions.ts` (new hook)
|
||||
|
||||
```typescript
|
||||
export function useTerminalSessions(instanceId: string) {
|
||||
const [sessions, setSessions] = useState<TerminalSessionInfo[]>([]);
|
||||
const [activeSessionId, setActiveSessionId] = useState<string | null>(null);
|
||||
|
||||
const createSession = useCallback(async (name?: string) => { ... }, [instanceId]);
|
||||
const closeSession = useCallback(async (sessionId: string) => { ... }, [instanceId]);
|
||||
const renameSession = useCallback(async (sessionId: string, name: string) => { ... }, [instanceId]);
|
||||
const resetSession = useCallback(async (sessionId: string) => { ... }, [instanceId]);
|
||||
|
||||
// Initial load
|
||||
useEffect(() => {
|
||||
loadSessions().then((sess) => {
|
||||
if (sess.length === 0) {
|
||||
createSession().then((s) => setActiveSessionId(s.id));
|
||||
} else {
|
||||
setSessions(sess);
|
||||
setActiveSessionId(sess[0].id);
|
||||
}
|
||||
});
|
||||
}, [instanceId]);
|
||||
|
||||
return { sessions, activeSessionId, setActiveSessionId, createSession, closeSession, renameSession, resetSession };
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Data Flow
|
||||
|
||||
### 1. Create New Session
|
||||
|
||||
```
|
||||
User clicks [+] tab
|
||||
│
|
||||
▼
|
||||
Frontend: POST /instances/{id}/terminal/sessions { name?: "Session 3" }
|
||||
│
|
||||
▼
|
||||
Backend:
|
||||
1. Auth + validate instance running
|
||||
2. Check session count < 5
|
||||
3. TerminalManager.create_session()
|
||||
- Generates UUID session_id
|
||||
- Starts docker exec PTY
|
||||
- Inserts TerminalSessionModel row
|
||||
4. Returns { session_id, name, status, created_at }
|
||||
│
|
||||
▼
|
||||
Frontend:
|
||||
1. Append session to sessions[]
|
||||
2. setActiveSessionId(newId)
|
||||
3. React renders new <TerminalComponent> with sessionId prop
|
||||
4. Component opens WS to /terminal/{session_id}
|
||||
5. Backend attaches WS, replays buffer
|
||||
```
|
||||
|
||||
### 2. Switch Between Sessions
|
||||
|
||||
```
|
||||
User clicks tab "Session 2"
|
||||
│
|
||||
▼
|
||||
Frontend: setActiveSessionId("session-2-uuid")
|
||||
│
|
||||
▼
|
||||
React re-renders:
|
||||
- Session 1 container → className="hidden" (display: none)
|
||||
- Session 2 container → className="active" (display: block)
|
||||
│
|
||||
▼
|
||||
Session 2 useEffect (on visibility change):
|
||||
- Calls fitAddon.fit()
|
||||
- Sends resize message over its existing WS
|
||||
│
|
||||
▼
|
||||
(Backend: no operation needed. Both WS connections remain open.)
|
||||
```
|
||||
|
||||
### 3. Close Session
|
||||
|
||||
```
|
||||
User clicks [×] on "Session 2"
|
||||
│
|
||||
▼
|
||||
Frontend: confirm() or inline tooltip
|
||||
│
|
||||
▼
|
||||
Frontend: DELETE /instances/{id}/terminal/sessions/{session_id}
|
||||
│
|
||||
▼
|
||||
Backend:
|
||||
1. Auth
|
||||
2. TerminalManager.close_session(instance_id, session_id)
|
||||
- Kills docker exec process
|
||||
- Removes from _sessions dict
|
||||
- Updates DB: status=closed, closed_at=now()
|
||||
3. Returns { status: "closed" }
|
||||
│
|
||||
▼
|
||||
Frontend:
|
||||
1. Remove session from sessions[]
|
||||
2. Unmount <TerminalComponent> (WS closes with code 1000)
|
||||
3. If closed session was active, setActiveSessionId to another session (or create one if none left)
|
||||
```
|
||||
|
||||
### 4. Reconnect to Existing Session
|
||||
|
||||
```
|
||||
User reloads page
|
||||
│
|
||||
▼
|
||||
Frontend: GET /instances/{id}/terminal/sessions
|
||||
│
|
||||
▼
|
||||
Backend: Returns all DB rows with status != "closed"
|
||||
│
|
||||
▼
|
||||
Frontend: Populate sessions[]. For each session, render <TerminalComponent>.
|
||||
│
|
||||
▼
|
||||
Each TerminalComponent opens its WS:
|
||||
WS URL: /ws/tool-instances/{id}/terminal/{session_id}
|
||||
│
|
||||
▼
|
||||
Backend:
|
||||
1. Auth
|
||||
2. TerminalManager.get_session(instance_id, session_id)
|
||||
- If found in-memory: attach_websocket, replay buffer
|
||||
- If not found in-memory (API restarted): WS closes with code 4004 "Session not found"
|
||||
(Frontend handles by showing "Session expired" with option to reset/recreate.)
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Contracts
|
||||
|
||||
### WebSocket Protocol
|
||||
|
||||
#### Connection URLs
|
||||
|
||||
| URL | Purpose |
|
||||
|-----|---------|
|
||||
| `/ws/tool-instances/{instance_id}/terminal` | Default session (backward compatible). Creates/attaches to the single legacy session. |
|
||||
| `/ws/tool-instances/{instance_id}/terminal/{session_id}` | Specific session. Attaches to an existing session or fails if not found. |
|
||||
|
||||
#### Client → Server Messages
|
||||
|
||||
| Type | Payload | Purpose |
|
||||
|------|---------|---------|
|
||||
| `resize` | `{ cols: number, rows: number }` | Resize PTY |
|
||||
| `reset` | `{}` | Kill and restart the **current** session's shell |
|
||||
| `pong` | `{}` | Heartbeat response |
|
||||
|
||||
#### Server → Client Messages
|
||||
|
||||
| Type | Payload | Purpose |
|
||||
|------|---------|---------|
|
||||
| (binary) | `bytes` | PTY output |
|
||||
| `status` | `{ status: "connected" \| "resetting" }` | Lifecycle status |
|
||||
| `ping` | `{}` | Heartbeat |
|
||||
|
||||
### REST API Contract
|
||||
|
||||
#### `GET /projects/{pid}/repositories/{rid}/instances/{iid}/terminal/sessions`
|
||||
|
||||
**Response 200:**
|
||||
```json
|
||||
{
|
||||
"sessions": [
|
||||
{
|
||||
"id": "uuid",
|
||||
"name": "Session 1",
|
||||
"status": "active",
|
||||
"has_websockets": true,
|
||||
"created_at": "2026-05-28T10:00:00Z",
|
||||
"last_activity_at": "2026-05-28T10:05:00Z"
|
||||
}
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
#### `POST /projects/{pid}/repositories/{rid}/instances/{iid}/terminal/sessions`
|
||||
|
||||
**Request body:**
|
||||
```json
|
||||
{ "name": "Optional Custom Name" }
|
||||
```
|
||||
|
||||
**Response 201:**
|
||||
```json
|
||||
{
|
||||
"id": "uuid",
|
||||
"name": "Session 2",
|
||||
"status": "active",
|
||||
"created_at": "2026-05-28T10:00:00Z"
|
||||
}
|
||||
```
|
||||
|
||||
**Response 409:** (max sessions reached)
|
||||
```json
|
||||
{ "detail": "Maximum of 5 terminal sessions reached for this instance" }
|
||||
```
|
||||
|
||||
#### `DELETE /projects/{pid}/repositories/{rid}/instances/{iid}/terminal/sessions/{sid}`
|
||||
|
||||
**Response 200:**
|
||||
```json
|
||||
{ "status": "closed", "session_id": "uuid" }
|
||||
```
|
||||
|
||||
#### `POST /projects/{pid}/repositories/{rid}/instances/{iid}/terminal/sessions/{sid}/reset`
|
||||
|
||||
**Response 200:**
|
||||
```json
|
||||
{
|
||||
"id": "uuid",
|
||||
"name": "Session 1",
|
||||
"status": "active"
|
||||
}
|
||||
```
|
||||
|
||||
#### `POST /projects/{pid}/repositories/{rid}/instances/{iid}/terminal/sessions/{sid}/rename`
|
||||
|
||||
**Request body:**
|
||||
```json
|
||||
{ "name": "New Name" }
|
||||
```
|
||||
|
||||
**Response 200:**
|
||||
```json
|
||||
{ "id": "uuid", "name": "New Name" }
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Testing Strategy
|
||||
|
||||
### Unit Tests
|
||||
|
||||
**Backend**: `apps/api/tests/services/test_terminal_manager.py`
|
||||
|
||||
| Test | Scenario |
|
||||
|------|----------|
|
||||
| `test_create_session_increases_count` | Creating sessions increments the per-instance count |
|
||||
| `test_create_session_enforces_max_5` | 6th creation raises `MaxSessionsExceededError` |
|
||||
| `test_get_sessions_for_instance` | Returns only sessions for the requested instance |
|
||||
| `test_close_session_removes_from_dict` | `close_session` removes key from `_sessions` |
|
||||
| `test_attach_websocket_only_closes_same_session` | Attaching to session A does not close websockets on session B |
|
||||
| `test_default_session_keyed_separately` | Default session uses `"default"` session_id and does not collide with named sessions |
|
||||
| `test_idle_cleanup_updates_db` | Idle cleanup calls DB update with `status=closed` |
|
||||
|
||||
**Frontend**: `apps/web/src/components/terminal-session-tabs.test.tsx`
|
||||
|
||||
| Test | Scenario |
|
||||
|------|----------|
|
||||
| `test_renders_all_tabs` | Renders one tab per session |
|
||||
| `test_click_tab_selects_session` | Clicking a tab calls `onSelect` with correct ID |
|
||||
| `test_close_button_calls_onClose` | Clicking × calls `onClose` |
|
||||
| `test_double_click_enables_rename` | Double-click shows input; Enter commits |
|
||||
| `test_plus_disabled_at_max_sessions` | `+` button is disabled when 5 sessions exist |
|
||||
|
||||
### Integration Tests
|
||||
|
||||
**Backend**: `apps/api/tests/api/test_terminal_ws.py`
|
||||
|
||||
| Test | Scenario |
|
||||
|------|----------|
|
||||
| `test_specific_session_websocket` | Connect to `/terminal/{session_id}`, verify output |
|
||||
| `test_default_session_alias` | Connect to `/terminal`, verify it creates/uses default session |
|
||||
| `test_concurrent_sessions_isolated` | Two WS connections to different session_ids receive independent output |
|
||||
| `test_reset_control_message_scoped` | `{"type":"reset"}` only resets the current session |
|
||||
| `test_list_sessions_returns_live_and_db` | `GET /sessions` reflects both in-memory state and DB rows |
|
||||
|
||||
**Frontend**: `apps/web/src/pages/terminal.test.tsx` (or E2E)
|
||||
|
||||
| Test | Scenario |
|
||||
|------|----------|
|
||||
| `test_create_session_adds_tab` | Clicking + creates a new tab and switches to it |
|
||||
| `test_switch_tab_preserves_scrollback` | Switching back to a previous tab shows prior output |
|
||||
| `test_close_last_session_creates_default` | Closing the final session auto-creates a new default session |
|
||||
| `test_fullscreen_toggle` | `Ctrl+Shift+F` toggles fullscreen class |
|
||||
|
||||
---
|
||||
|
||||
## Rollout Plan
|
||||
|
||||
### Phase 1: Database (Zero-Downtime)
|
||||
|
||||
1. Run Alembic migration to create `terminal_sessions` table.
|
||||
2. No code reads from or writes to this table yet. Existing sessions remain purely in-memory.
|
||||
3. **Rollback**: Alembic downgrade removes table (no data loss risk since table is empty).
|
||||
|
||||
### Phase 2: Backend API (Backward Compatible)
|
||||
|
||||
1. Deploy updated `TerminalManager` with composite key `_sessions`.
|
||||
2. Deploy updated `TerminalSession` with `name` support.
|
||||
3. Deploy new WebSocket route `/terminal/{session_id}` and preserve `/terminal` alias.
|
||||
4. Deploy new REST endpoints (`GET/POST/DELETE .../sessions`).
|
||||
5. Update DB writes on session lifecycle (create, close, activity update).
|
||||
6. **Rollback**: Revert code. Old `/terminal` endpoint continues to work. New `/terminal/{session_id}` returns 404, but no clients call it yet.
|
||||
|
||||
### Phase 3: Frontend (Feature Flag Optional)
|
||||
|
||||
1. Deploy new components (`TerminalSessionTabs`, `useTerminalSessions`).
|
||||
2. Update `TerminalPage` and `MobileTerminalWrapper`.
|
||||
3. Update `TerminalComponent` to accept optional `sessionId` prop.
|
||||
4. If a feature flag is used, enable multi-session UI for beta users first.
|
||||
5. **Rollback**: Revert frontend. Users see the old single-session UI. Backend `/terminal` alias continues to serve them.
|
||||
|
||||
### Phase 4: Deprecation & Cleanup (Follow-Up Task)
|
||||
|
||||
1. Monitor usage of the legacy `/terminal` WebSocket endpoint and `POST .../terminal/reset` REST endpoint.
|
||||
2. After 2-4 weeks of stable multi-session usage:
|
||||
- Mark legacy endpoints as deprecated in OpenAPI docs.
|
||||
- Update frontend to always use `/terminal/{session_id}` (never rely on default alias).
|
||||
3. In a future release, remove the default alias if desired (not required for correctness).
|
||||
|
||||
### Backward Compatibility Strategy
|
||||
|
||||
| Layer | Compat Mechanism |
|
||||
|-------|-----------------|
|
||||
| WebSocket | `/terminal` remains default-session alias forever (or until explicit deprecation). Old clients continue to work. |
|
||||
| REST API | Existing `POST .../terminal/reset` preserved as alias. No breaking changes to response shape. |
|
||||
| Frontend | `sessionId` prop on `TerminalComponent` is optional. Omitting it uses the default session path. |
|
||||
| DB | New table is additive only. No changes to `tool_instances` schema. |
|
||||
|
||||
---
|
||||
|
||||
## Files to Create / Modify
|
||||
|
||||
### New Files
|
||||
|
||||
| File | Description |
|
||||
|------|-------------|
|
||||
| `apps/api/src/models/terminal_session.py` | SQLAlchemy `TerminalSessionModel` |
|
||||
| `apps/api/src/alembic/versions/XXXX_add_terminal_sessions_table.py` | Alembic migration |
|
||||
| `apps/web/src/components/terminal-session-tabs.tsx` | Tab bar UI (desktop + mobile) |
|
||||
| `apps/web/src/hooks/use-terminal-sessions.ts` | Session CRUD + state hook |
|
||||
| `apps/web/src/components/terminal-session-tabs.test.tsx` | Unit tests |
|
||||
| `apps/api/tests/services/test_terminal_manager_multi.py` | TerminalManager multi-session tests |
|
||||
| `apps/api/tests/api/test_terminal_ws_multi.py` | WS integration tests |
|
||||
|
||||
### Modified Files
|
||||
|
||||
| File | Changes |
|
||||
|------|---------|
|
||||
| `apps/api/src/services/terminal_manager.py` | Composite key dict, new CRUD methods, max session limit, DB integration |
|
||||
| `apps/api/src/services/terminal_session.py` | Add `name` field, status tracking |
|
||||
| `apps/api/src/api/terminal.py` | New WS route, REST endpoints, shared handler coroutine |
|
||||
| `apps/api/src/main.py` | Import new model (if needed for Alembic autogenerate) |
|
||||
| `apps/web/src/components/terminal.tsx` | Accept `sessionId` prop, use it in WS URL |
|
||||
| `apps/web/src/pages/terminal.tsx` | Multi-session orchestration, tabs, fullscreen |
|
||||
| `apps/web/src/components/mobile-terminal-wrapper.tsx` | Integrate tabs, pass session state |
|
||||
| `apps/web/src/components/mobile-terminal-header.tsx` | Show active session name |
|
||||
| `apps/web/src/api/sessions.ts` (or new `terminal.ts`) | REST client functions for session CRUD |
|
||||
|
||||
---
|
||||
|
||||
## Risks & Mitigations
|
||||
|
||||
| Risk | Likelihood | Impact | Mitigation |
|
||||
|------|------------|--------|------------|
|
||||
| Resource exhaustion from 5× docker exec per instance | Medium | High | Max 5 enforced. Idle timeout (30 min) still applies per session. |
|
||||
| Mobile UX degraded by tab bar + special keys strip | Medium | Medium | Auto-hide shared between tabs and header. Minimal tab design. |
|
||||
| Concurrent WS policy closes wrong session's sockets | Medium | High | Unit test explicitly: attach to session A must not affect session B's websockets. |
|
||||
| DB writes on hot path (activity tracking) | Low | Medium | `last_activity_at` updates are non-blocking fire-and-forget asyncio tasks. No await on commit. |
|
||||
| Frontend performance with 5 mounted xterm.js instances | Low | Medium | Max 5 sessions. Inactive terminals are `display: none` (not unmounted). GPU acceleration in xterm.js handles this well. |
|
||||
| Default session alias ambiguity | Low | Low | Document that `/terminal` maps to `"default"` session. Future deprecation can migrate default to explicit ID. |
|
||||
@@ -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.
|
||||
@@ -0,0 +1,33 @@
|
||||
## Why
|
||||
|
||||
Currently, each tool instance (e.g., pi-agent, code-server) supports exactly one terminal session. Users who want to run multiple concurrent tasks (e.g., a long-running build in one pane, an editor in another, and a shell for quick commands) must open multiple tool instances or use tmux/screen inside a single session. This is inefficient and confusing.
|
||||
|
||||
Additionally, the web terminal lacks basic usability features found in modern terminal emulators: fullscreen mode, detachable panes, session tabs, and keyboard shortcuts for common actions.
|
||||
|
||||
## What Changes
|
||||
|
||||
- **Backend**: Allow multiple `TerminalSession` objects per `ToolInstance`, each with a unique `session_id`
|
||||
- **Backend**: Update `TerminalManager` to track and route multiple sessions per instance
|
||||
- **Backend**: Update terminal WebSocket protocol to include `session_id` in connection URL or message
|
||||
- **Frontend**: Add session tabs/management UI (create new session, switch between sessions, close sessions)
|
||||
- **Frontend**: Add fullscreen mode for the terminal
|
||||
- **Frontend**: Add keyboard shortcuts for session management (Ctrl+Shift+N new session, etc.)
|
||||
- **Frontend**: Session list panel showing active sessions per instance
|
||||
|
||||
## Capabilities
|
||||
|
||||
### New Capabilities
|
||||
- `multi-session-terminal`: Multiple independent terminal sessions per tool instance
|
||||
- `terminal-fullscreen`: Fullscreen terminal mode
|
||||
- `terminal-session-management`: Create, switch, rename, and close terminal sessions
|
||||
|
||||
### Modified Capabilities
|
||||
- `tool-terminal`: Extend WebSocket protocol and UI to support multiple sessions per instance
|
||||
- `terminal-session-lifecycle`: Session creation, naming, and cleanup for multi-session model
|
||||
|
||||
## Impact
|
||||
|
||||
- Backend: `TerminalManager`, `TerminalSession`, `api/terminal.py`, database schema (session tracking)
|
||||
- Frontend: `TerminalComponent`, `terminal.tsx`, new `TerminalSessionTabs`, `TerminalSessionManager`
|
||||
- Protocol: WebSocket message format changes (add session_id field)
|
||||
- Database: New or extended table to track terminal sessions per instance
|
||||
@@ -0,0 +1,404 @@
|
||||
# SDD Tasks: Multi-Session Terminal UX
|
||||
|
||||
## Review Workload Forecast
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| Estimated changed lines | ~1,400–1,600 (new ~900, modified ~600–700) |
|
||||
| 400-line budget risk | High |
|
||||
| Chained PRs recommended | Yes |
|
||||
| Suggested split | PR 1: DB + Backend Core → PR 2: Backend API + Tests → PR 3: Frontend + Tests |
|
||||
| Delivery strategy | auto-chain |
|
||||
| Chain strategy | stacked-to-main |
|
||||
|
||||
```text
|
||||
Decision needed before apply: Yes
|
||||
Chained PRs recommended: Yes
|
||||
Chain strategy: stacked-to-main
|
||||
400-line budget risk: High
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task Overview
|
||||
|
||||
| # | Task | PR | Est. Lines | Dependencies |
|
||||
|---|------|-----|------------|--------------|
|
||||
| 1 | Database schema and Alembic migration | 1 | ~80 | None |
|
||||
| 2 | TerminalManager multi-session core | 1 | ~250 | Task 1 |
|
||||
| 3 | TerminalSession name and status fields | 1 | ~40 | Task 2 |
|
||||
| 4 | WebSocket routing and backward-compat alias | 2 | ~200 | Task 2 |
|
||||
| 5 | REST endpoints for session CRUD | 2 | ~180 | Task 2, 4 |
|
||||
| 6 | Frontend API client and `useTerminalSessions` hook | 3 | ~180 | Task 5 |
|
||||
| 7 | `TerminalComponent` `sessionId` support | 3 | ~100 | Task 4, 6 |
|
||||
| 8 | `TerminalSessionTabs` UI component | 3 | ~220 | Task 6 |
|
||||
| 9 | `TerminalPage` multi-session orchestration and fullscreen | 3 | ~200 | Task 7, 8 |
|
||||
| 10 | Mobile terminal integration | 3 | ~100 | Task 8, 9 |
|
||||
| 11 | Backend integration tests | 2 | ~250 | Task 4, 5 |
|
||||
| 12 | Frontend component tests | 3 | ~150 | Task 8, 9, 10 |
|
||||
|
||||
---
|
||||
|
||||
## PR 1: Database + Backend Core
|
||||
|
||||
### Task 1: Database Schema and Alembic Migration
|
||||
|
||||
**Scope**: Create the `terminal_sessions` metadata table and corresponding Alembic migration.
|
||||
|
||||
**Files to create**:
|
||||
- `apps/api/src/models/terminal_session.py`
|
||||
- `apps/api/alembic/versions/XXXX_add_terminal_sessions_table.py`
|
||||
|
||||
**Files to modify**:
|
||||
- `apps/api/src/main.py` — import new model so Alembic autogenerate discovers it
|
||||
|
||||
**Acceptance Criteria**:
|
||||
- `TerminalSessionModel` extends `Base`, `UUIDPrimaryKeyMixin`, `TimestampMixin`
|
||||
- Columns: `instance_id` (UUID, FK `tool_instances.id` ON DELETE CASCADE, indexed), `name` (String 255, nullable), `status` (String 50, default `"active"`), `created_at` (DateTime TZ, non-nullable), `last_activity_at` (DateTime TZ, nullable), `closed_at` (DateTime TZ, nullable)
|
||||
- Migration is reversible (`downgrade` drops table + index)
|
||||
- `make migrate` applies successfully in local dev
|
||||
|
||||
**Testing (TDD)**:
|
||||
- RED: Write a migration metadata test asserting the new table exists in `Base.metadata` and has expected columns
|
||||
- GREEN: Create model and migration
|
||||
- Run `pytest tests/integration/test_models.py` or equivalent to verify table registration
|
||||
|
||||
---
|
||||
|
||||
### Task 2: TerminalManager Multi-Session Core
|
||||
|
||||
**Scope**: Refactor `TerminalManager` to support up to 5 concurrent sessions per instance using composite keys.
|
||||
|
||||
**Files to modify**:
|
||||
- `apps/api/src/services/terminal_manager.py`
|
||||
|
||||
**Acceptance Criteria**:
|
||||
- `self._sessions` keyed by `(instance_id: str, session_id: str)`
|
||||
- `create_session(instance_id, container_id, startup_command=None, name=None)`:
|
||||
- Generates UUID `session_id`
|
||||
- Enforces max 5 active sessions per instance (raise `MaxSessionsExceededError` / HTTP 409)
|
||||
- Inserts `TerminalSessionModel` DB row (fire-and-forget async task acceptable)
|
||||
- Returns `TerminalSession`
|
||||
- `get_or_create_session(instance_id, container_id, ...)` preserved for backward compatibility; uses `"default"` session_id
|
||||
- `get_session(instance_id, session_id)` returns session or `None`
|
||||
- `get_sessions_for_instance(instance_id)` returns list of in-memory sessions
|
||||
- `close_session(instance_id, session_id)`: kills PTY, removes from `_sessions`, updates DB `status=closed`, `closed_at=now()`
|
||||
- `reset_session(instance_id, container_id, session_id=None)`: if `session_id` omitted, resets `"default"` session
|
||||
- `attach_websocket` only closes existing WebSockets **within the same `(instance_id, session_id)`**
|
||||
- `_cleanup_idle_sessions` uses composite keys and updates DB status on cleanup
|
||||
- Idle timeout (30 min) and buffer replay behavior preserved
|
||||
|
||||
**Testing (TDD)**:
|
||||
- RED: Create `apps/api/tests/services/test_terminal_manager_multi.py` with tests:
|
||||
- `test_create_session_increases_count`
|
||||
- `test_create_session_enforces_max_5`
|
||||
- `test_get_sessions_for_instance_filters_by_instance`
|
||||
- `test_close_session_removes_from_dict_and_updates_db`
|
||||
- `test_attach_websocket_only_closes_same_session`
|
||||
- `test_default_session_keyed_separately`
|
||||
- `test_idle_cleanup_updates_db_status`
|
||||
- GREEN: Implement `TerminalManager` changes
|
||||
- Run `make test-unit`
|
||||
|
||||
---
|
||||
|
||||
### Task 3: TerminalSession Name and Status Fields
|
||||
|
||||
**Scope**: Add runtime `name` and `status` tracking to `TerminalSession`.
|
||||
|
||||
**Files to modify**:
|
||||
- `apps/api/src/services/terminal_session.py`
|
||||
|
||||
**Acceptance Criteria**:
|
||||
- `__init__` accepts optional `name`; auto-generates `"Session N"` if omitted (N = per-instance counter)
|
||||
- `self.name` stored as runtime attribute
|
||||
- `self.status` enum-like string: `"active"`, `"resetting"`, `"closed"`
|
||||
- `reset()` sets `status="resetting"` during transition, `"active"` after restart
|
||||
- `close()` sets `status="closed"`
|
||||
- No breaking changes to existing `TerminalSession` behavior
|
||||
|
||||
**Testing (TDD)**:
|
||||
- RED: Extend `test_terminal_manager_multi.py` or add `test_terminal_session_name_and_status.py` covering auto-naming, status transitions, and reset/close side effects
|
||||
- GREEN: Implement fields and transitions
|
||||
- Run `make test-unit`
|
||||
|
||||
---
|
||||
|
||||
## PR 2: Backend API + Tests
|
||||
|
||||
### Task 4: WebSocket Routing and Backward-Compat Alias
|
||||
|
||||
**Scope**: Add session-scoped WebSocket route, extract shared handler, preserve legacy alias.
|
||||
|
||||
**Files to modify**:
|
||||
- `apps/api/src/api/terminal.py`
|
||||
|
||||
**Files to create**:
|
||||
- `apps/api/tests/api/test_terminal_ws_multi.py`
|
||||
|
||||
**Acceptance Criteria**:
|
||||
- New route: `@router.websocket("/ws/tool-instances/{instance_id}/terminal/{session_id}")`
|
||||
- Existing route `@router.websocket("/ws/tool-instances/{instance_id}/terminal")` preserved; calls `get_or_create_session(...)` for `"default"` session
|
||||
- Extract `async def _handle_terminal_websocket(websocket, instance_id, session_id, db_session)` containing shared auth/validation/I/O loop logic
|
||||
- Both routes call `_handle_terminal_websocket`
|
||||
- Auth/validation logic unchanged (cookie-based, ownership check, running status)
|
||||
- `reset` control message scoped to the current session only (via `SessionRef` update)
|
||||
- On unknown `session_id`, close WS with code `4004` "Session not found"
|
||||
|
||||
**Testing (TDD)**:
|
||||
- RED: Write `test_terminal_ws_multi.py`:
|
||||
- `test_specific_session_websocket_connects`
|
||||
- `test_default_session_alias_creates_default`
|
||||
- `test_concurrent_sessions_isolated_output`
|
||||
- `test_reset_control_message_scoped_to_session`
|
||||
- `test_unknown_session_id_returns_4004`
|
||||
- GREEN: Implement routes and shared handler
|
||||
- Run `pytest tests/api/test_terminal_ws_multi.py`
|
||||
|
||||
---
|
||||
|
||||
### Task 5: REST Endpoints for Session CRUD
|
||||
|
||||
**Scope**: Add REST endpoints for listing, creating, closing, resetting, and renaming sessions.
|
||||
|
||||
**Files to modify**:
|
||||
- `apps/api/src/api/terminal.py`
|
||||
|
||||
**Acceptance Criteria**:
|
||||
- `GET /projects/{pid}/repositories/{rid}/instances/{iid}/terminal/sessions`
|
||||
- Returns `{ sessions: [...] }` with `id`, `name`, `status`, `has_websockets`, `created_at`, `last_activity_at`
|
||||
- `has_websockets` queried live from `TerminalManager`
|
||||
- `POST .../terminal/sessions` — body `{ name?: string }`
|
||||
- Returns `201` with `{ id, name, status, created_at }`
|
||||
- Returns `409` if max 5 reached
|
||||
- `DELETE .../terminal/sessions/{sid}` — returns `{ status: "closed", session_id }`
|
||||
- `POST .../terminal/sessions/{sid}/reset` — returns `{ id, name, status }`
|
||||
- `POST .../terminal/sessions/{sid}/rename` — body `{ name: string }`, returns `{ id, name }`
|
||||
- Existing `POST .../terminal/reset` preserved as alias for default session reset
|
||||
- All endpoints validate auth, ownership, and running instance status
|
||||
|
||||
**Testing (TDD)**:
|
||||
- RED: Add integration tests in `test_terminal_ws_multi.py` or new `test_terminal_rest.py`:
|
||||
- `test_list_sessions_returns_db_and_live_state`
|
||||
- `test_create_session_201`
|
||||
- `test_create_session_409_at_max`
|
||||
- `test_close_session_200`
|
||||
- `test_reset_session_200`
|
||||
- `test_rename_session_200`
|
||||
- `test_legacy_reset_alias_still_works`
|
||||
- GREEN: Implement endpoints
|
||||
- Run `make test-integration`
|
||||
|
||||
---
|
||||
|
||||
### Task 6: Frontend API Client and `useTerminalSessions` Hook
|
||||
|
||||
**Scope**: Add frontend REST client functions and the central session state hook.
|
||||
|
||||
**Files to create**:
|
||||
- `apps/web/src/api/terminal.ts` (new file for terminal-specific API calls)
|
||||
- `apps/web/src/hooks/use-terminal-sessions.ts`
|
||||
|
||||
**Files to modify**:
|
||||
- `apps/web/src/api/sessions.ts` — optional, or keep terminal API separate
|
||||
|
||||
**Acceptance Criteria**:
|
||||
- API functions: `listTerminalSessions`, `createTerminalSession`, `closeTerminalSession`, `resetTerminalSession`, `renameTerminalSession`
|
||||
- `useTerminalSessions(instanceId: string)` hook:
|
||||
- Loads sessions on mount; auto-creates one if list is empty
|
||||
- Exposes `sessions`, `activeSessionId`, `setActiveSessionId`
|
||||
- Exposes `createSession`, `closeSession`, `renameSession`, `resetSession` with optimistic UI updates
|
||||
- Handles 409 errors (max sessions) gracefully
|
||||
- Refetches after reset/rename to stay in sync
|
||||
|
||||
**Testing (TDD)**:
|
||||
- RED: Write hook unit tests mocking API client:
|
||||
- `test_loads_sessions_on_mount`
|
||||
- `test_auto_creates_session_if_empty`
|
||||
- `test_close_session_removes_from_state`
|
||||
- `test_create_session_enforces_max_5_error`
|
||||
- GREEN: Implement hook and API client
|
||||
- Run `cd apps/web && npm test`
|
||||
|
||||
---
|
||||
|
||||
## PR 3: Frontend + Tests
|
||||
|
||||
### Task 7: `TerminalComponent` `sessionId` Support
|
||||
|
||||
**Scope**: Update `TerminalComponent` to accept an optional `sessionId` and route WS accordingly.
|
||||
|
||||
**Files to modify**:
|
||||
- `apps/web/src/components/terminal.tsx`
|
||||
|
||||
**Acceptance Criteria**:
|
||||
- New optional prop `sessionId?: string`
|
||||
- WS URL constructed as:
|
||||
- `/ws/tool-instances/{instanceId}/terminal/{sessionId}` if `sessionId` provided
|
||||
- `/ws/tool-instances/{instanceId}/terminal` if omitted (backward compat)
|
||||
- Reset button sends `{"type": "reset"}` to the correct session's WS
|
||||
- Component still supports all existing props and mobile behavior
|
||||
- `onTerminalReady` callback still works; parent can differentiate sessions by key
|
||||
|
||||
**Testing (TDD)**:
|
||||
- RED: Add/update `terminal.test.tsx` (or similar) to assert WS URL includes `sessionId` when provided
|
||||
- GREEN: Implement prop and URL logic
|
||||
- Run `cd apps/web && npm test`
|
||||
|
||||
---
|
||||
|
||||
### Task 8: `TerminalSessionTabs` UI Component
|
||||
|
||||
**Scope**: Build the tab bar for desktop and mobile.
|
||||
|
||||
**Files to create**:
|
||||
- `apps/web/src/components/terminal-session-tabs.tsx`
|
||||
- `apps/web/src/components/terminal-session-tabs.test.tsx`
|
||||
|
||||
**Acceptance Criteria**:
|
||||
- Props interface: `sessions`, `activeSessionId`, `onSelect`, `onClose`, `onCreate`, `onRename`, `isMobile?`
|
||||
- Desktop: horizontal tab strip above terminal, overflow scroll with fade indicator
|
||||
- Mobile: compact tabs integrated into auto-hide chrome, horizontal swipe scroll
|
||||
- Each tab shows: name, status dot (connecting/connected/disconnected/error), close button (×) on hover/active
|
||||
- Double-click to rename: inline `<input>`, `Enter` to confirm, `Escape` to cancel, blur confirms
|
||||
- New session button (+) at right end; disabled when 5 sessions exist
|
||||
- Close confirmation: lightweight inline confirm tooltip (not modal)
|
||||
- Accessible: `role="tablist"`, `role="tab"`, keyboard navigation
|
||||
|
||||
**Testing (TDD)**:
|
||||
- RED: Write `terminal-session-tabs.test.tsx`:
|
||||
- `test_renders_all_tabs`
|
||||
- `test_click_tab_calls_onSelect`
|
||||
- `test_close_button_calls_onClose`
|
||||
- `test_double_click_enables_rename`
|
||||
- `test_plus_disabled_at_max_sessions`
|
||||
- `test_status_dot_reflects_connection_state`
|
||||
- GREEN: Implement component
|
||||
- Run `cd apps/web && npm test`
|
||||
|
||||
---
|
||||
|
||||
### Task 9: `TerminalPage` Multi-Session Orchestration, Fullscreen, and Shortcuts
|
||||
|
||||
**Scope**: Rewrite `TerminalPage` to manage multiple mounted terminals, fullscreen mode, and keyboard shortcuts.
|
||||
|
||||
**Files to modify**:
|
||||
- `apps/web/src/pages/terminal.tsx`
|
||||
|
||||
**Acceptance Criteria**:
|
||||
- Uses `useTerminalSessions` hook
|
||||
- Renders `<TerminalSessionTabs />` above terminal area
|
||||
- Renders one `<TerminalComponent />` per session; inactive sessions hidden via `display: none` (preserves scrollback and WS)
|
||||
- On tab switch, active terminal calls `fitAddon.fit()` via ref + `useEffect` on visibility
|
||||
- Fullscreen toggle:
|
||||
- `Ctrl+Shift+F` toggles `.fullscreen` class
|
||||
- Desktop: hides page header; tab strip becomes minimal overlay (auto-hides after 3s, reappears on mouse move)
|
||||
- Mobile: hides header, tab strip, special keys; floating handle reveals chrome
|
||||
- Exit via `Esc` or UI button
|
||||
- Keyboard shortcuts (registered in `useEffect` on `keydown`):
|
||||
- `Alt+Shift+N` — new session
|
||||
- `Alt+Shift+W` — close current session
|
||||
- `Alt+Shift+←` / `Alt+Shift+→` — prev/next session
|
||||
- `Alt+Shift+R` — reset current session
|
||||
- All use `preventDefault()` only for the exact combo; no browser overrides
|
||||
- Closing last session auto-creates a new default session
|
||||
|
||||
**Testing (TDD)**:
|
||||
- RED: Add `terminal-page.test.tsx`:
|
||||
- `test_creates_default_session_on_empty_load`
|
||||
- `test_switching_tabs_hides_inactive_terminals`
|
||||
- `test_fullscreen_toggle_adds_class`
|
||||
- `test_keyboard_shortcut_creates_session`
|
||||
- `test_close_last_session_auto_creates_default`
|
||||
- GREEN: Implement page orchestration
|
||||
- Run `cd apps/web && npm test`
|
||||
|
||||
---
|
||||
|
||||
### Task 10: Mobile Terminal Integration
|
||||
|
||||
**Scope**: Integrate session tabs into mobile terminal wrapper and update header.
|
||||
|
||||
**Files to modify**:
|
||||
- `apps/web/src/components/mobile-terminal-wrapper.tsx`
|
||||
- `apps/web/src/components/mobile-terminal-header.tsx`
|
||||
|
||||
**Acceptance Criteria**:
|
||||
- `MobileTerminalWrapper` accepts session-related props from `TerminalPage` and passes them to `TerminalSessionTabs`
|
||||
- `MobileTerminalHeader` displays `activeSession.name` instead of generic `"Terminal"`
|
||||
- Tab strip shares `useAutoHide` behavior with header (tapping terminal toggles visibility)
|
||||
- Special keys strip remains functional; no z-index conflicts with tabs
|
||||
- Fullscreen on mobile correctly hides/shows all chrome layers
|
||||
|
||||
**Testing (TDD)**:
|
||||
- RED: Add/update mobile wrapper tests:
|
||||
- `test_renders_session_tabs`
|
||||
- `test_header_shows_session_name`
|
||||
- `test_auto_hide_applies_to_tabs`
|
||||
- GREEN: Implement mobile integration
|
||||
- Run `cd apps/web && npm test`
|
||||
|
||||
---
|
||||
|
||||
### Task 11: Backend Integration Tests
|
||||
|
||||
**Scope**: Complete backend test coverage for multi-session WebSocket and REST behavior.
|
||||
|
||||
**Files to create / modify**:
|
||||
- `apps/api/tests/services/test_terminal_manager_multi.py` (finalize)
|
||||
- `apps/api/tests/api/test_terminal_ws_multi.py` (finalize)
|
||||
|
||||
**Acceptance Criteria**:
|
||||
- All tests from Tasks 2, 4, 5 pass
|
||||
- Additional integration tests:
|
||||
- `test_list_sessions_after_api_restart_shows_db_metadata` (simulates restart by clearing in-memory dict)
|
||||
- `test_two_websockets_on_same_session_receive_same_output`
|
||||
- `test_idle_cleanup_per_session_not_global`
|
||||
- `make test` passes (unit + integration)
|
||||
|
||||
**Testing (TDD)**:
|
||||
- These are the GREEN/TRIANGULATE phases for earlier backend tasks; ensure coverage is comprehensive
|
||||
|
||||
---
|
||||
|
||||
### Task 12: Frontend Component Tests
|
||||
|
||||
**Scope**: Finalize frontend test coverage for tabs, page, and hook.
|
||||
|
||||
**Files to create / modify**:
|
||||
- `apps/web/src/components/terminal-session-tabs.test.tsx` (finalize)
|
||||
- `apps/web/src/hooks/use-terminal-sessions.test.ts` (new, if not created earlier)
|
||||
- `apps/web/src/pages/terminal.test.tsx` (new)
|
||||
|
||||
**Acceptance Criteria**:
|
||||
- Tab component tests cover rendering, selection, close, rename, and max-session disable
|
||||
- Hook tests cover load, create, close, error handling
|
||||
- Page tests cover session lifecycle, fullscreen, and keyboard shortcuts
|
||||
- `cd apps/web && npm test` passes
|
||||
|
||||
**Testing (TDD)**:
|
||||
- Finalize RED→GREEN→TRIANGULATE for all frontend tasks
|
||||
|
||||
---
|
||||
|
||||
## Risks and Mitigations
|
||||
|
||||
| Risk | Likelihood | Impact | Mitigation |
|
||||
|------|------------|--------|------------|
|
||||
| Resource exhaustion (5× docker exec per instance) | Medium | High | Max 5 enforced in `create_session`. Idle timeout (30 min) applies per session. |
|
||||
| Mobile UX degraded by tab bar + special keys strip | Medium | Medium | Auto-hide shared between tabs and header. Compact tab design. Overflow scroll. |
|
||||
| Concurrent WS policy closes wrong session's sockets | Medium | High | Explicit unit test: `attach_websocket` must only affect same `(instance_id, session_id)`. |
|
||||
| DB writes on hot path (activity tracking) | Low | Medium | `last_activity_at` updates are fire-and-forget async tasks; do not block I/O loop. |
|
||||
| Frontend performance with 5 mounted xterm.js instances | Low | Medium | Max 5 sessions. Inactive terminals use `display: none` (not unmounted). xterm.js GPU acceleration handles this. |
|
||||
| Default session alias ambiguity | Low | Low | Document that `/terminal` maps to `"default"`. Future deprecation can migrate to explicit IDs. |
|
||||
| Browser shortcut conflicts | Low | Medium | Use `Alt+Shift+*` instead of `Ctrl+Shift+W/N`. Only `preventDefault()` on exact matching combos. |
|
||||
|
||||
---
|
||||
|
||||
## Rollback Plan
|
||||
|
||||
- **PR 1 rollback**: Alembic downgrade removes `terminal_sessions` table. Old `TerminalManager` code is fully replaced, so reverting PR 1 requires reverting all subsequent PRs.
|
||||
- **PR 2 rollback**: Revert API changes. Legacy `/terminal` WS route and `POST .../terminal/reset` continue to work; new `/terminal/{session_id}` returns 404 but no clients call it until PR 3 is deployed.
|
||||
- **PR 3 rollback**: Revert frontend. Users see old single-session UI. Backend `/terminal` alias continues to serve them.
|
||||
|
||||
Because PRs are stacked, rolling back PR 2 or PR 1 requires rolling back all dependent PRs above it.
|
||||
Reference in New Issue
Block a user