Skip to content

Commit bce78aa

Browse files
feat(session): wire PTYTransport into /session/ws (§1.5)
/api/session/ws now runs the new transport stack — no more agent_session_manager, no legacy {type:"start"} frame. The route binds the already-acquired active.ActiveSession to the WS and pumps transport events both ways under asyncio.TaskGroup + except*. Why: §1.5 of the agent-session-transport plan. Closes the wiring gap that left session/transport.py, session/transports/pty.py, and session/active.py fully implemented but unreachable from the web UI since PR-B-1..B-4. Route contract (authoritative for frontend §1.7): - Preconditions: active session must already be acquired. WS closes 1008 pre-accept when current() is None. - Origin guard: localhost/127.0.0.1 any port by default, extend via STUDYLOOP_ALLOWED_ORIGINS env var (plan Blocker B1). - Session-id match: ?study_session_id must equal active.study_session_id or close 1008 (pattern from terminal_proxy.py:134). - Inbound JSON: input/resize/stop control frames. - Outbound: binary for OutputBytes, text JSON for lifecycle events. - finally: always await active.release() — guarantees a clean singleton on disconnect. Scope cuts (deferred to §1.5b, separate PR): - POST /session/start still runs legacy tmux+ttyd flow. Re-pointing it at active.acquire(config, PTYTransport) is the remaining §1.5 work; user deferred because it needs Playwright-driven iteration. - Frontend WS feature flag (§1.9) becomes a browser concern rather than server concern — the WS route no longer serves ttyd at all; iframe path stays behind /terminal/ in terminal_proxy.py. Legacy test cleanup: 2 stale tests removed from test_web_live_session.py (test_live_session_websocket_streams_events_and_accepts_input and test_live_session_websocket_rejects_acp_until_handshake_is_implemented) — they exercised the now-replaced protocol. test_session_runtime.py's unit tests of the legacy manager still pass; session_runtime/ stays on disk until §1.5b also lands. Tests: 1980 passed, 145 deselected. +8 new in test_web_session_ws.py (origin guard, session-id mismatch, no-active-session rejection, Started/Output/Stopped pump, input/resize/stop inbound frames, release-on-disconnect). Ruff clean, format clean. Plan: docs/plans/2026-05-09-refactor-agent-session-transport-plan.md Amendment #5 appended with the full route contract for §1.7 frontend. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 96cb851 commit bce78aa

3 files changed

Lines changed: 430 additions & 136 deletions

File tree

packages/studyloop/src/studyloop/web/routes/session.py

Lines changed: 154 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -576,79 +576,170 @@ def get_session_options() -> dict[str, list[dict]]:
576576
}
577577

578578

579+
_WS_CLOSE_POLICY = 1008 # RFC 6455 Policy Violation
580+
581+
582+
def _allowed_ws_origins() -> set[str]:
583+
"""Return the allow-list for Origin on the live session WS.
584+
585+
Localhost only by default — this is a single-user tool. Expand via
586+
``STUDYLOOP_ALLOWED_ORIGINS`` (comma-separated) if the operator fronts
587+
the app behind a reverse proxy. See plan Blocker B1 (CSRF via cross-
588+
origin WS upgrade; RFC 6455 has no same-origin enforcement and CORS
589+
does not apply to WS).
590+
"""
591+
import os
592+
593+
defaults = {
594+
"http://127.0.0.1:8788",
595+
"http://localhost:8788",
596+
"http://127.0.0.1",
597+
"http://localhost",
598+
}
599+
extra = os.environ.get("STUDYLOOP_ALLOWED_ORIGINS", "").strip()
600+
if extra:
601+
return defaults | {o.strip() for o in extra.split(",") if o.strip()}
602+
return defaults
603+
604+
605+
def _origin_allowed(origin: str) -> bool:
606+
if not origin:
607+
return False
608+
allowed = _allowed_ws_origins()
609+
if origin in allowed:
610+
return True
611+
# Allow any port on localhost / 127.0.0.1 (dev server port may vary).
612+
for prefix in ("http://127.0.0.1", "http://localhost"):
613+
if origin.startswith(prefix + ":") or origin == prefix:
614+
return True
615+
return False
616+
617+
579618
@router.websocket("/session/ws")
580619
async def live_session_socket(websocket: WebSocket) -> None:
581-
"""Bidirectional live agent session socket.
582-
583-
Client messages:
584-
- {"type": "start", "topic": "...", "energy": 5, "agent": "codex", "transport": "pty"}
585-
- {"type": "input", "text": "..."}
586-
- {"type": "stop"}
620+
"""Bidirectional live agent session socket (plan §1.5).
621+
622+
The route binds an already-acquired ``active.ActiveSession`` to the
623+
WS client. Sessions are acquired via ``POST /api/session/start``
624+
(which calls ``active.acquire(config, PTYTransport)``); the WS then
625+
streams transport events out and pumps control frames in.
626+
627+
Inbound JSON control frames:
628+
- ``{"type": "input", "data": "..."}`` → ``transport.send_input``
629+
- ``{"type": "resize", "cols": N, "rows": N}`` → ``transport.resize``
630+
- ``{"type": "stop"}`` → ``transport.cancel``
631+
632+
Outbound framing:
633+
- ``OutputBytes.data`` → **binary** frame (verbatim PTY bytes)
634+
- ``Started``/``Stopped``/``TransportError``/``AgentMessage`` → text
635+
JSON frames (``{"type": ..., ...}``)
636+
637+
Close codes:
638+
- 1008 (Policy Violation) if Origin is disallowed, if no session is
639+
active, or if ``?study_session_id`` does not match the active one.
640+
- 1000 normal close on ``stop`` frame or transport-emitted ``Stopped``.
587641
"""
588-
await websocket.accept()
589-
manager = websocket.app.state.agent_session_manager
590-
session_id: str | None = None
591-
event_task: asyncio.Task[None] | None = None
642+
from studyloop.session import active as session_active
643+
from studyloop.session.transport import (
644+
AgentMessage,
645+
OutputBytes,
646+
Started,
647+
Stopped,
648+
TransportError,
649+
)
592650

593-
async def forward_events(events) -> None: # type: ignore[no-untyped-def]
594-
async for event in events:
595-
await websocket.send_json(event.as_dict())
651+
# --- Pre-accept guards -----------------------------------------------
596652

597-
try:
598-
while True:
599-
message = await websocket.receive_json()
600-
message_type = message.get("type")
601-
if message_type == "start":
602-
requested_transport = str(message.get("transport") or "pty")
603-
if requested_transport == "acp":
604-
await websocket.send_json(
605-
{
606-
"type": "error",
607-
"data": {
608-
"message": (
609-
"ACP transport is scaffolded but not enabled yet. "
610-
"Use Browser terminal for Gemini or Kiro."
611-
)
612-
},
613-
}
614-
)
615-
continue
616-
if session_id is not None:
617-
await websocket.send_json(
618-
{"type": "error", "data": {"message": "Session already started"}}
619-
)
620-
continue
621-
session_id, events = await manager.start_session(
622-
topic=str(message.get("topic") or "Study Session"),
623-
energy=int(message.get("energy") or 5),
624-
agent=message.get("agent"),
625-
transport=requested_transport,
653+
# Origin check (plan Blocker B1). Must happen before accept() — the
654+
# handshake has not yet completed, so close-without-accept sends an
655+
# HTTP 403 response per Starlette semantics, which the client sees as
656+
# a failed upgrade. After accept() we can only send a WS close frame.
657+
origin = websocket.headers.get("origin", "")
658+
if not _origin_allowed(origin):
659+
logger.warning("WS /session/ws rejected: disallowed origin=%r", origin)
660+
await websocket.close(code=_WS_CLOSE_POLICY)
661+
return
662+
663+
requested = websocket.query_params.get("study_session_id")
664+
current = await session_active.current()
665+
if current is None:
666+
await websocket.close(code=_WS_CLOSE_POLICY)
667+
return
668+
if requested and requested != current.study_session_id:
669+
logger.warning(
670+
"WS /session/ws rejected: requested=%r active=%r",
671+
requested,
672+
current.study_session_id,
673+
)
674+
await websocket.close(code=_WS_CLOSE_POLICY)
675+
return
676+
677+
await websocket.accept()
678+
transport = current.transport
679+
680+
async def pty_to_ws() -> None:
681+
"""Pump transport events → WS frames until the session ends."""
682+
async for event in transport.events():
683+
if isinstance(event, OutputBytes):
684+
await websocket.send_bytes(event.data)
685+
elif isinstance(event, Started):
686+
await websocket.send_json({"type": "started", "agent": event.agent})
687+
elif isinstance(event, Stopped):
688+
await websocket.send_json(
689+
{
690+
"type": "stopped",
691+
"returncode": event.returncode,
692+
"reason": event.reason,
693+
}
626694
)
627-
event_task = asyncio.create_task(forward_events(events))
628-
elif message_type == "input":
629-
if session_id is None:
630-
await websocket.send_json(
631-
{"type": "error", "data": {"message": "No active session"}}
632-
)
633-
continue
634-
await manager.send(session_id, str(message.get("text") or ""))
635-
elif message_type == "stop":
636-
if session_id:
637-
await manager.stop(session_id)
638-
session_id = None
639-
await websocket.close()
640-
return
641-
else:
695+
return # Stopped is terminal — stop pumping.
696+
elif isinstance(event, TransportError):
697+
await websocket.send_json({"type": "transport_error", "message": event.message})
698+
elif isinstance(event, AgentMessage):
642699
await websocket.send_json(
643-
{"type": "error", "data": {"message": "Unknown message type"}}
700+
{"type": "agent_message", "kind": event.kind, "payload": event.payload}
644701
)
645-
except WebSocketDisconnect:
702+
703+
async def ws_to_pty() -> None:
704+
"""Read WS control frames and forward to transport."""
705+
while True:
706+
frame = await websocket.receive_json()
707+
ftype = frame.get("type")
708+
if ftype == "input":
709+
data = frame.get("data", "")
710+
if isinstance(data, str):
711+
await transport.send_input(data.encode("utf-8"))
712+
elif ftype == "resize":
713+
try:
714+
cols = int(frame.get("cols", 80))
715+
rows = int(frame.get("rows", 24))
716+
except (TypeError, ValueError):
717+
continue
718+
await transport.resize(cols, rows)
719+
elif ftype == "stop":
720+
await transport.cancel()
721+
return
722+
# Silently drop unknown frame types — no error channel needed.
723+
724+
# --- Pump with TaskGroup (plan Blocker B5) ---------------------------
725+
#
726+
# TaskGroup raises ExceptionGroup on any child exception; the WS
727+
# disconnect paths come up as ExceptionGroup[WebSocketDisconnect,
728+
# ConnectionClosedOK, ...]. ``except*`` unpacks cleanly without
729+
# reaching for .exceptions.
730+
731+
try:
732+
async with asyncio.TaskGroup() as tg:
733+
tg.create_task(pty_to_ws(), name="ws-pty-to-ws")
734+
tg.create_task(ws_to_pty(), name="ws-ws-to-pty")
735+
except* WebSocketDisconnect:
646736
pass
737+
except* OSError as eg:
738+
logger.error("PTY I/O error on /session/ws: %s", eg.exceptions)
739+
except* Exception as eg: # pragma: no cover — defensive
740+
logger.exception("unexpected error on /session/ws: %s", eg.exceptions)
647741
finally:
648-
if event_task:
649-
event_task.cancel()
650-
if session_id:
651-
await manager.stop(session_id)
742+
await session_active.release()
652743

653744

654745
@router.post("/session/end")
Lines changed: 8 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,58 +1,26 @@
1-
"""Tests for web live-session WebSocket and picker endpoints."""
1+
"""Tests for web session-picker endpoints.
2+
3+
Prior to §1.5 wiring this file also held WS route tests exercising the
4+
legacy ``{type: "start", ...}`` protocol and ``agent_session_manager``.
5+
Those were removed when the WS route migrated to the
6+
``active.acquire`` + ``PTYTransport`` flow; current WS coverage lives in
7+
``test_web_session_ws.py``.
8+
"""
29

310
from __future__ import annotations
411

512
from typing import TYPE_CHECKING
613

714
from fastapi.testclient import TestClient
815

9-
from studyloop.session_runtime import SessionEvent
1016
from studyloop.web.app import create_app
1117

1218
if TYPE_CHECKING:
13-
from collections.abc import AsyncIterator
1419
from pathlib import Path
1520

1621
import pytest
1722

1823

19-
class FakeManager:
20-
"""Minimal manager used by the WebSocket route tests."""
21-
22-
def __init__(self) -> None:
23-
self.sent: list[str] = []
24-
self.stopped: list[str] = []
25-
26-
async def start_session(
27-
self,
28-
*,
29-
topic: str,
30-
energy: int,
31-
agent: str | None = None,
32-
transport: str = "pty",
33-
) -> tuple[str, AsyncIterator[SessionEvent]]:
34-
async def events() -> AsyncIterator[SessionEvent]:
35-
yield SessionEvent(
36-
"started",
37-
"fake-session",
38-
{
39-
"topic": topic,
40-
"energy": energy,
41-
"agent": agent or "shell",
42-
"transport": transport,
43-
},
44-
)
45-
yield SessionEvent("output", "fake-session", {"text": "ready"})
46-
47-
return "fake-session", events()
48-
49-
async def send(self, session_id: str, text: str) -> None:
50-
self.sent.append(f"{session_id}:{text}")
51-
52-
async def stop(self, session_id: str) -> None:
53-
self.stopped.append(session_id)
54-
55-
5624
def test_session_options_returns_course_hierarchy(
5725
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
5826
) -> None:
@@ -83,36 +51,3 @@ def __init__(self) -> None:
8351
assert "agents" in body
8452
assert all(agent["recommended_transport"] == "ttyd" for agent in body["agents"])
8553
assert all(agent["acp_ready"] is False for agent in body["agents"])
86-
87-
88-
def test_live_session_websocket_streams_events_and_accepts_input() -> None:
89-
app = create_app()
90-
manager = FakeManager()
91-
app.state.agent_session_manager = manager
92-
client = TestClient(app)
93-
94-
with client.websocket_connect("/api/session/ws") as websocket:
95-
websocket.send_json({"type": "start", "topic": "Python", "energy": 6, "transport": "pty"})
96-
assert websocket.receive_json()["type"] == "started"
97-
assert websocket.receive_json()["type"] == "output"
98-
websocket.send_json({"type": "input", "text": "what is a decorator?"})
99-
websocket.send_json({"type": "stop"})
100-
101-
assert manager.sent == ["fake-session:what is a decorator?"]
102-
assert manager.stopped == ["fake-session"]
103-
104-
105-
def test_live_session_websocket_rejects_acp_until_handshake_is_implemented() -> None:
106-
app = create_app()
107-
manager = FakeManager()
108-
app.state.agent_session_manager = manager
109-
client = TestClient(app)
110-
111-
with client.websocket_connect("/api/session/ws") as websocket:
112-
websocket.send_json({"type": "start", "topic": "Python", "energy": 6, "transport": "acp"})
113-
payload = websocket.receive_json()
114-
websocket.close()
115-
116-
assert payload["type"] == "error"
117-
assert "ACP transport is scaffolded" in payload["data"]["message"]
118-
assert manager.sent == []

0 commit comments

Comments
 (0)