headquarter/openspec/changes/multi-session-terminal-ux/apply-pr1.md

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.