ewilliams28/turnstone
1
0
Fork 0
mirror of https://github.com/turnstonelabs/turnstone.git synced 2026-08-12 23:12:23 -06:00
Code Issues Packages Projects Releases Wiki Activity
Files
stable/1.5
turnstone/tests/test_interactive_adapter.py
T
Patrick Buckley 48c9ad2a40 refactor(core): split SessionKindAdapter Protocol into construction +… (#412) 
* refactor(core): split SessionKindAdapter Protocol into construction + emission (Stage 2 P3)

The single ``SessionKindAdapter`` Protocol that ``SessionManager``
takes is split into two:

* ``SessionKindAdapter`` — kind / build_ui / build_session /
  cleanup_ui. Required for every kind. The shared lifecycle
  manager always delegates here for construction + cleanup.
* ``SessionEventEmitter`` — emit_created / emit_state /
  emit_rehydrated / emit_closed. **Optional**, wired through a new
  ``event_emitter: SessionEventEmitter | None = None`` kwarg on
  ``SessionManager``. Reserved for future kinds whose lifecycle
  transitions don't fan out anywhere; both production kinds wire
  one today.

Both production adapters implement both Protocols. The interactive
lifespan (``server.py``) and console lifespan
(``console/server.py``) pass their adapter as both ``adapter`` and
``event_emitter`` — production behaviour is unchanged. Six lifecycle
sites in ``SessionManager`` (create / open eviction / open rehydrate /
close / set_state / close_idle / _reserve_and_install_locked unwind)
now call ``self._event_emitter.emit_*(...)`` guarded by
``if self._event_emitter is not None``.

InteractiveAdapter asymmetry preserved + documented:

* ``emit_closed`` stays load-bearing — it's the **sole** transport
  path for ``ws_closed`` onto the process-wide global SSE queue
  (Stage 1 consolidated emission from the create handler here so
  there's exactly one emission point; ``name`` powers the
  frontend's eviction toast).
* ``emit_created`` / ``emit_state`` / ``emit_rehydrated`` are
  documented no-op stubs (``del ws[, state]``). Those events fire
  from out-of-band paths — the create HTTP handler enqueues
  ``ws_created`` directly onto ``global_queue`` *after* attachment
  validation (so a rejected upload doesn't surface a phantom
  create→close pair); ``WebUI._broadcast_state`` emits the full
  ``ws_state`` payload (tokens + context_ratio + activity) via the
  ``SessionUI.on_state_change`` callback chain. The stubs exist
  solely to satisfy ``SessionEventEmitter`` Protocol so the
  adapter can be wired as the manager's ``event_emitter`` for the
  ``emit_closed`` path. Each stub has a 1-line inline rationale to
  match the in-repo convention (``coordinator_adapter.py:210``).

Test scaffolding:

* ``tests/test_session_manager.py`` — ``_make_manager`` and
  ``_make_with_writer`` wire ``FakeAdapter`` as both ``adapter``
  and ``event_emitter`` for production parity; the standalone
  ``test_create_uses_configured_node_id`` does the same.
  ``FakeAdapter.emit_rehydrated`` now records as
  ``_Event("rehydrated", ...)`` rather than conflating with
  ``"created"``, and ``test_open_resurrects_closed_state`` asserts
  against ``events_of("rehydrated")`` so a regression where the
  manager fires the wrong call on the open path actually fails.
* ``tests/_coord_test_helpers.py`` and
  ``tests/test_coordinator_end_to_end.py`` — wire
  ``CoordinatorAdapter`` as both args.
* Six interactive test fixtures (``test_skills.py``,
  ``test_prompt_templates_runtime.py`` x2, ``test_model_registry.py``,
  ``test_server_authz.py``, ``test_server_attachments_on_create.py``)
  — wire ``event_emitter=adapter`` so they match the production
  wiring, removing the footgun where a future contributor adds a
  ``gq.get_nowait()`` assertion and silently loses the only
  ``ws_closed`` transport.
* ``tests/test_interactive_adapter.py`` — drops the three
  tautological no-op-emit_* tests (``test_emit_created_is_noop``,
  ``test_emit_state_is_noop``, ``test_emit_rehydrated_is_noop``);
  keeps the four ``emit_closed`` tests (real behaviour).

Lint + mypy clean. 4475 tests passing.

* docs(core): correct SessionKindAdapter + SessionEventEmitter docstrings to match implementation

Two Copilot review threads on PR #412 caught the same real
discrepancy: my P3 docstrings on ``SessionKindAdapter`` and
``SessionEventEmitter`` described an *intent* — "interactive
doesn't implement ``SessionEventEmitter``; the manager skips emit
calls when no emitter is wired" — that doesn't match the actual
wiring. ``InteractiveAdapter`` does implement both Protocols and
``server.py`` does pass it as ``event_emitter``; only the three
no-op stubs (``emit_created`` / ``emit_state`` / ``emit_rehydrated``)
are dead, while ``emit_closed`` is load-bearing.

Updated both docstrings to:

* State that both production adapters implement both Protocols.
* Explain the asymmetry is in *which* emit methods carry real
  bodies (coord: 4; interactive: 1, with 3 documented stubs because
  the out-of-band paths — create handler ``ws_created`` after
  attachment validation, ``WebUI._broadcast_state`` carrying the
  richer ``ws_state`` payload — fire those events).
* Clarify the ``if self._event_emitter is not None`` guard exists
  for the kwarg-omitted case (tests that don't care about events,
  reserved for future kinds whose transitions don't fan out
  anywhere).

Docstring-only change. Lint + mypy clean; the 75 tests in
test_session_manager + test_interactive_adapter + test_coordinator_adapter
pass.

Resolves the two Copilot review threads on PR #412 (commits
PRRC_kwDORcMomM67VyPD, PRRC_kwDORcMomM67VyPI).
2026-04-25 22:52:39 -07:00

249 lines
8.5 KiB
Python
Raw Permalink Blame History

"""Tests for InteractiveAdapter.
Focus: the ``emit_closed`` transport contract (sole path for
``ws_closed`` onto the process-wide queue) and ``cleanup_ui``
behavior (unblock pending events, broadcast ``ws_closed`` to per-UI
listeners, cancel + close session). The SessionManager-level tests
in ``test_session_manager.py`` cover the adapter-agnostic lifecycle.
The other three :class:`SessionEventEmitter` methods
(``emit_created`` / ``emit_state`` / ``emit_rehydrated``) are
documented no-op stubs — ``ws_created`` is fired by the create HTTP
handler after attachment validation, and ``ws_state`` is fired by
``WebUI._broadcast_state`` with the full payload. No-op assertions
on those methods would be tautological given the class docstring,
so they're not retested here.
"""
from __future__ import annotations
import queue
import threading
from typing import Any
from unittest.mock import MagicMock
from turnstone.core.adapters.interactive_adapter import InteractiveAdapter
from turnstone.core.workstream import Workstream, WorkstreamKind
class _StubUI:
"""Stub matching the subset of WebUI the adapter touches."""
def __init__(self) -> None:
self._approval_event = threading.Event()
self._approval_result: tuple[bool, str | None] = (True, "initial")
self._plan_event = threading.Event()
self._plan_result: str = "accept"
self._fg_event = threading.Event()
self._listeners_lock = threading.Lock()
self._listeners: list[queue.Queue[dict[str, Any]]] = []
class _StubSession:
def __init__(self) -> None:
self.cancelled = False
self.closed = False
self.model = "gpt-5"
self.model_alias = "default"
def cancel(self) -> None:
self.cancelled = True
def close(self) -> None:
self.closed = True
def _make_adapter(
*,
ui_factory: Any = None,
session_factory: Any = None,
) -> tuple[InteractiveAdapter, queue.Queue[dict[str, Any]]]:
gq: queue.Queue[dict[str, Any]] = queue.Queue(maxsize=100)
adapter = InteractiveAdapter(
global_queue=gq,
ui_factory=ui_factory or (lambda ws: _StubUI()),
session_factory=session_factory or (lambda *a, **kw: _StubSession()),
)
return adapter, gq
def _make_ws(**overrides: Any) -> Workstream:
ws = Workstream(id="ws-1", name="hello")
ws.kind = WorkstreamKind.INTERACTIVE
ws.user_id = "u1"
ws.ui = _StubUI()
ws.session = _StubSession()
for k, v in overrides.items():
setattr(ws, k, v)
return ws
# ---------------------------------------------------------------------------
# Transport — emit_closed (the only emit_* with real behavior on interactive;
# emit_created / emit_state / emit_rehydrated are documented no-op stubs)
# ---------------------------------------------------------------------------
def test_emit_closed_defaults_to_closed_reason() -> None:
adapter, gq = _make_adapter()
adapter.emit_closed("ws-1", name="my-ws")
event = gq.get_nowait()
assert event == {
"type": "ws_closed",
"ws_id": "ws-1",
"reason": "closed",
"name": "my-ws",
}
def test_emit_closed_propagates_evicted_reason_and_name() -> None:
adapter, gq = _make_adapter()
adapter.emit_closed("ws-1", reason="evicted", name="my-ws")
event = gq.get_nowait()
assert event["reason"] == "evicted"
assert event["name"] == "my-ws"
def test_emit_closed_default_name_is_empty_string() -> None:
adapter, gq = _make_adapter()
adapter.emit_closed("ws-1")
assert gq.get_nowait()["name"] == ""
def test_emit_swallows_queue_full_without_raising() -> None:
gq: queue.Queue[dict[str, Any]] = queue.Queue(maxsize=1)
gq.put({"type": "filler"})
adapter = InteractiveAdapter(
global_queue=gq,
ui_factory=lambda ws: _StubUI(),
session_factory=lambda *a, **kw: _StubSession(),
)
adapter.emit_closed("ws-1") # must not raise even though queue is full
assert gq.qsize() == 1 # nothing added on a full queue
# ---------------------------------------------------------------------------
# cleanup_ui
# ---------------------------------------------------------------------------
def test_cleanup_ui_unblocks_pending_approval_plan_fg_events() -> None:
adapter, _ = _make_adapter()
ws = _make_ws()
# Simulate pending events
ws.ui._approval_event.clear() # type: ignore[attr-defined]
ws.ui._plan_event.clear() # type: ignore[attr-defined]
ws.ui._fg_event.clear() # type: ignore[attr-defined]
adapter.cleanup_ui(ws)
assert ws.ui._approval_event.is_set() # type: ignore[attr-defined]
assert ws.ui._plan_event.is_set() # type: ignore[attr-defined]
assert ws.ui._fg_event.is_set() # type: ignore[attr-defined]
# Approval result flipped to "deny" so the waiter sees a sensible value.
assert ws.ui._approval_result == (False, None) # type: ignore[attr-defined]
assert ws.ui._plan_result == "reject" # type: ignore[attr-defined]
def test_cleanup_ui_broadcasts_ws_closed_to_listener_queues() -> None:
adapter, _ = _make_adapter()
ws = _make_ws()
lq1: queue.Queue[dict[str, Any]] = queue.Queue(maxsize=10)
lq2: queue.Queue[dict[str, Any]] = queue.Queue(maxsize=10)
ws.ui._listeners.extend([lq1, lq2]) # type: ignore[attr-defined]
adapter.cleanup_ui(ws)
assert lq1.get_nowait() == {"type": "ws_closed"}
assert lq2.get_nowait() == {"type": "ws_closed"}
# Listeners cleared so subsequent events don't fan out to dead generators.
assert ws.ui._listeners == [] # type: ignore[attr-defined]
def test_cleanup_ui_broadcast_evicts_stale_head_when_listener_queue_full() -> None:
"""Per the old _cleanup_ui fallback: when a listener queue is full,
drop the oldest event and put ws_closed. Ensures an unresponsive
browser tab doesn't block close."""
adapter, _ = _make_adapter()
ws = _make_ws()
lq: queue.Queue[dict[str, Any]] = queue.Queue(maxsize=1)
lq.put_nowait({"type": "stale"})
ws.ui._listeners.append(lq) # type: ignore[attr-defined]
adapter.cleanup_ui(ws)
assert lq.get_nowait() == {"type": "ws_closed"}
assert lq.empty()
def test_cleanup_ui_cancels_and_closes_session() -> None:
adapter, _ = _make_adapter()
ws = _make_ws()
adapter.cleanup_ui(ws)
assert ws.session.cancelled is True # type: ignore[attr-defined]
assert ws.session.closed is True # type: ignore[attr-defined]
def test_cleanup_ui_tolerates_missing_session_and_ui() -> None:
"""A placeholder workstream whose session build failed may arrive
at cleanup_ui with session=None or ui=None. Must not crash."""
adapter, _ = _make_adapter()
ws = _make_ws()
ws.session = None
ws.ui = None
adapter.cleanup_ui(ws) # no crash
def test_cleanup_ui_tolerates_stub_ui_without_events() -> None:
"""A stub UI missing _approval_event / etc. (test scaffolding
code) must not crash cleanup_ui — the hasattr guards matter."""
adapter, _ = _make_adapter()
ws = _make_ws()
ws.ui = MagicMock(spec=[]) # empty spec — attribute accesses miss
adapter.cleanup_ui(ws) # no crash
# ---------------------------------------------------------------------------
# Construction passthrough
# ---------------------------------------------------------------------------
def test_build_ui_delegates_to_ui_factory() -> None:
captured_ws: list[Workstream] = []
def _ui_factory(ws: Workstream) -> Any:
captured_ws.append(ws)
return _StubUI()
adapter, _ = _make_adapter(ui_factory=_ui_factory)
ws = _make_ws()
result = adapter.build_ui(ws)
assert captured_ws == [ws]
assert isinstance(result, _StubUI)
def test_build_session_forwards_all_kwargs_to_session_factory() -> None:
captured: dict[str, Any] = {}
def _sf(ui: Any, model: str | None, ws_id: str, **kwargs: Any) -> Any:
captured["ui"] = ui
captured["model"] = model
captured["ws_id"] = ws_id
captured.update(kwargs)
return _StubSession()
adapter, _ = _make_adapter(session_factory=_sf)
ws = _make_ws()
adapter.build_session(
ws, skill="coder", model="gpt-5", client_type="web", judge_model="gpt-4.1"
)
assert captured["ui"] is ws.ui
assert captured["model"] == "gpt-5"
assert captured["ws_id"] == ws.id
assert captured["skill"] == "coder"
assert captured["client_type"] == "web"
assert captured["kind"] == WorkstreamKind.INTERACTIVE
assert captured["parent_ws_id"] is None
# Kind-specific passthrough — interactive session_factory accepts judge_model.
assert captured["judge_model"] == "gpt-4.1"
Reference in New Issue View Git Blame Copy Permalink