alex
62d1bdc462
- Add TerminalSessionTabs component with status dots, rename, close, max-5 limit - Add 7 component tests for tab rendering, selection, close, rename - TerminalComponent: sessionId prop, forwardRef with fit() method - TerminalPage: multi-session orchestration, tab switching, auto-create default - Fullscreen mode: Alt+Shift+F toggle, auto-hide tabs, Esc exit - Keyboard shortcuts: Alt+Shift+N/W/ArrowLeft/ArrowRight/R - Add CSS for tabs, fullscreen, mobile responsive - Update useTerminalSessions hook for session CRUD - terminal_manager.py: lookup by internal session_id fallback Quality gates: tsc --noEmit clean, vitest (7/7 new tests passed), pytest (182 passed)
75 lines
5.7 KiB
Markdown
75 lines
5.7 KiB
Markdown
# 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.
|