Files
headquarter/openspec/changes/multi-session-terminal-ux/tasks.md
T
alex 62d1bdc462 feat: multi-session terminal frontend UI + tests (PR 3)
- Add TerminalSessionTabs component with status dots, rename, close, max-5 limit
- Add 7 component tests for tab rendering, selection, close, rename
- TerminalComponent: sessionId prop, forwardRef with fit() method
- TerminalPage: multi-session orchestration, tab switching, auto-create default
- Fullscreen mode: Alt+Shift+F toggle, auto-hide tabs, Esc exit
- Keyboard shortcuts: Alt+Shift+N/W/ArrowLeft/ArrowRight/R
- Add CSS for tabs, fullscreen, mobile responsive
- Update useTerminalSessions hook for session CRUD
- terminal_manager.py: lookup by internal session_id fallback

Quality gates: tsc --noEmit clean, vitest (7/7 new tests passed), pytest (182 passed)
2026-05-28 13:35:45 +02:00

405 lines
17 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# SDD Tasks: Multi-Session Terminal UX
## Review Workload Forecast
| Field | Value |
|-------|-------|
| Estimated changed lines | ~1,4001,600 (new ~900, modified ~600700) |
| 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.