mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-12 23:12:23 -06:00
e42add1b77
* feat(coordinator): coordinator workstream kind — phase 1
Adds a new ``kind="coordinator"`` workstream that runs inside the
``turnstone-console`` process (first ChatSession hosted on the console)
with a dedicated tool set for spawning and driving child workstreams.
Supersedes the external ``turnstone-coordinator`` MCP side-car for new
installs; the extension is marked deprecated in
``examples/mcp-cluster-ops/README.md`` but still works on 1.4-and-earlier
clusters.
Phase 1 ships: the workstream class, 6 lifecycle tools, console hosting,
9 HTTP endpoints, per-user audit attribution, and a one-pane web UI at
``/coordinator/{ws_id}``. Node/skill discovery tools, task-list tool,
tree-view UI, and routing-proxy audit middleware follow in a later PR.
## Schema
Migration 039 adds ``kind`` / ``parent_ws_id`` columns + indexes to
``workstreams``. Both SQLite and PostgreSQL backends take the new
kwargs on ``register_workstream``; empty-string ``parent_ws_id``
normalises to ``NULL`` at the storage edge. PostgreSQL uses
``INSERT ... ON CONFLICT DO NOTHING`` to match SQLite's ``OR IGNORE``
and close a pre-existing SELECT-then-INSERT TOCTOU window.
``list_workstreams`` gains optional ``parent_ws_id`` / ``kind`` filters;
new ``get_workstream(ws_id)`` returns the full row (the existing
``get_workstream_metadata`` stays untouched for back-compat).
## Core session + kind routing
- ``ChatSession.__init__`` accepts ``kind`` / ``parent_ws_id`` /
``coord_client``. On ``kind="coordinator"`` it swaps
``_tools = COORDINATOR_TOOLS`` and zeros sub-agent tool lists.
- ``Workstream`` dataclass extended with ``user_id`` / ``kind`` /
``parent_ws_id``. Both ``WorkstreamManager`` and the new
``CoordinatorManager`` use the same type — no parallel hierarchy.
- ``_SessionFactory`` Protocol + server / cli factory closures thread
the new kwargs. ``POST /v1/api/workstreams/new`` rejects
``kind != "interactive"`` with 400; ``POST
/v1/api/workstreams/{ws_id}/open`` refuses coordinator rows so a
server node can't accidentally rehydrate one.
## Coordinator tool set
Six tools (``spawn``, ``inspect``, ``send``, ``close``, ``delete``,
``list_workstreams``) with a ``coordinator: true`` metadata flag,
scoped to coordinator-kind sessions only. ``inspect`` and ``list`` are
auto-approved reads; the four mutators need approval. ``list`` returns
``{"children": [...], "truncated": bool}`` so the model can detect
post-filter under-fill and paginate.
## CoordinatorClient (in-process, sync)
Mutating ops HTTP-POST to the console's own ``/v1/api/route/*`` on the
local bind URL so every existing middleware (auth, rate-limit) runs.
Read ops hit ``storage.list_workstreams`` / ``get_workstream`` /
``load_messages`` directly — the routing proxy doesn't expose
list/inspect paths. URL paths are a validated constant table (avoids
an httpx ``base_url``-merge trap). A new
``/v1/api/route/workstreams/delete`` proxy handler joins the existing
route-proxy endpoints.
## Per-session coordinator JWT
``CoordinatorTokenManager`` mints short-lived JWTs with ``sub=<real
user>`` (attribution preserved), ``src="coordinator"``,
``aud="turnstone-console"``, ``coord_ws_id=<ws>`` custom claim.
``_proxy_auth_headers`` preserves ``src`` + ``coord_ws_id`` across the
upstream re-mint so server-side middleware sees coordinator-origin,
not ``console-proxy``. ``AuthResult.extra_claims`` carries
non-reserved claims through validate→remint; ``create_jwt``'s
reserved-claim set (now including ``nbf`` / ``jti``) is symmetric with
``validate_jwt``.
## Console hosts the ChatSession
- New ConfigStore settings: ``coordinator.model_alias`` (required),
``reasoning_effort``, ``max_active`` (default 5),
``session_jwt_ttl_seconds``.
- Console lifespan builds a ``ModelRegistry`` +
``CoordinatorManager``. Missing / unresolvable alias returns **503**
with remediation text — never 500.
- ``CoordinatorManager``: placeholder-slot reservation under lock,
rollback on factory failure, per-ws_id rehydration lock to serialise
concurrent lazy-opens, ``max_active`` enforced via ``close_idle``
eviction semantics.
- ``ConsoleCoordinatorUI`` is a thin ``SessionUI`` implementation — no
global broadcast, no per-node metrics, shared
``_APPROVAL_WAIT_TIMEOUT`` constant across approval + plan paths.
- No eager startup rehydration: persisted coordinator rows load lazily
on first ``GET /v1/api/coordinator/{ws_id}``.
## Console coordinator API
Nine endpoints under ``/v1/api/coordinator/*`` gated by ``approve``
scope + new **``admin.coordinator``** permission (added to
``_VALID_PERMISSIONS``; not in any builtin role — operators opt in
explicitly). Ownership failures return **404, not 403** and use
strict equality so empty-owner rows don't leak across tenants.
Correlation-id masking on every factory-raising path
(``coordinator_create`` + ``coordinator_detail`` lazy rehydrate) — no
stack traces to the client.
## Audit attribution
Three console-side events (``coordinator.create`` / ``.close`` /
``.cancel``) with the real creator's ``user_id`` plus
``detail={coord_ws_id, src="coordinator"}``. No schema migration
required. Per-tool-call audit across the routing proxy is deferred
(needs either a ``source`` column on ``audit_events`` or
``record_audit`` calls wired into the route-proxy handlers).
## Web UI (``/coordinator/{ws_id}``)
One-pane chat served by the console. Reuses ``shared_static``
(``base.css``, ``auth.js``, ``theme.js``, ``toast.js``, ``utils.js``,
``kb.js``) and the server UI's ``renderer.js`` pipeline (KaTeX, Mermaid,
highlight.js already bundled).
- SSE to ``/v1/api/coordinator/{ws_id}/events`` with exponential-
backoff reconnect; status line carries a leading glyph
(● / ○ / ⚠) so state isn't conveyed by colour alone.
- Renders content, reasoning (dimmed italic
``.role-reasoning``), tool_result, approve_request, intent_verdict,
output_warning.
- Child ws_id references auto-wrap to
``/node/{node_id}/?ws_id={child}`` links — both ids regex-validated
before interpolation, everything else HTML-escaped.
- Non-modal approval bar (``role="region"``) with a batch header
("Approve N tool calls"), initial focus on the approve button,
buttons disabled during the in-flight POST, red-bordered deny.
``aria-live`` flips to ``off`` during streaming.
- "New coordinator" button on the dashboard header — permission-gated
on the UI side, matching the backend 403.
- Mobile composer capped under ``@media (max-width: 700px)``.
## Tests
~120 new tests across 8 files: workstream-kind storage + dataclass
semantics, CoordinatorClient URL map + token minting + storage reads +
truncation signalling, tool prepare/exec dispatch and approval gating,
CoordinatorManager create / rollback / eviction / lazy rehydration +
concurrency, HTTP endpoint auth + 404-on-ownership + 503-on-misconfig,
proxy-auth ``src`` preservation, full lifecycle end-to-end, coordinator
page HTML-injection guard. ``test_tools_schema.py`` widened to 25
tools (19 existing + 6 coordinator).
Verification: ``ruff check`` clean, ``mypy turnstone`` clean
(156 files), ``pytest`` 4054 passed (5 pre-existing failures unrelated
to this change — confirmed against ``main``).
* polish(coordinator): address PR review + CI + tool-namespace isolation
CI:
- `ruff format`: two files reformatted, matches the in-repo pre-commit config.
- `wheel-completeness`: add `turnstone/console/static/coordinator/*.html` +
`*.js` to the hatch wheel-include list. Without this the coordinator UI
was missing from published wheels.
- `test (3.11/3.12/3.13)` + `test-postgres`: three `TestExecReadImage`
tests were masking a real bug — my 6 new tool JSONs pushed tool count
19→25, crossing the default `tool_search.auto` threshold (20), which
made `ChatSession.__init__` construct a `ToolSearchManager` and cache
`_cached_capabilities` during init. Tests that later patched
`session._provider.get_capabilities` saw the cached value instead.
Root-cause fix: the tool-search threshold code path now reads
capabilities through `_resolve_capabilities(...)` directly — no cache
populate — so the patch takes.
Tool-namespace isolation (bigger fix than CI symptoms suggested):
- `TOOLS` was the union of all loaded tool JSONs including the 6 new
coordinator tools. Interactive sessions were getting coordinator
tools in their function-calling surface (which is nonsense — they
require a console-hosted `coord_client`), and coordinator sessions
counted against the interactive tool-search threshold. Fix:
- New `INTERACTIVE_TOOLS` / `INTERACTIVE_TOOL_NAMES` in
`turnstone/core/tools.py` exclude anything with `coordinator: true`
metadata. `TOOLS` stays as the union for schema introspection +
eval catalog.
- `ChatSession.__init__` selects tool set by kind: coordinator gets
fixed `COORDINATOR_TOOLS` (no MCP merge, no listeners registered);
interactive gets `INTERACTIVE_TOOLS` (+ MCP if configured).
Coordinators are meta-orchestrators that spawn child workstreams;
MCP tools / resources / prompts live on the children, not on the
coordinator's own surface.
- `_on_mcp_tools_changed` no-ops for coordinator sessions
(defence-in-depth in case listeners were registered).
- `always_on_names` on `ToolSearchManager` is now the set of builtin
tools actually present in the session (kind-aware) rather than the
full `BUILTIN_TOOL_NAMES` frozenset.
- `turnstone/eval.py` uses `INTERACTIVE_TOOLS` (coordinator tools
aren't in scope for the eval harness which tests interactive agent
behaviour).
- Regression tests in `tests/test_workstream_kind.py`:
- `INTERACTIVE_TOOLS ∩ COORDINATOR_TOOLS == ∅` and their union is
`TOOLS`.
- Interactive `ChatSession._tools` does not include any
coordinator tool name.
- Coordinator `ChatSession._tools` contains `spawn_workstream` but
not `bash` / `edit_file` / `memory`; sub-agent lists are empty.
- Coordinator `ChatSession` with an MCP client attached does NOT
merge MCP tools and does NOT register any MCP listeners.
PR review findings:
- **#10 / #11** (Copilot): coordinator UI claimed to reuse the server
renderer pipeline but loaded none of its JS. Mirrored
`turnstone/ui/static/renderer.js` into
`turnstone/console/static/coordinator/renderer.js` (flagged in-file
as a cleanup candidate to promote into `shared_static/`), added
`katex.min.js` / `highlight.min.js` / `renderer.js` script tags to
`coordinator/index.html`. `coordinator.js` now buffers raw markdown
via `textContent` during streaming, then swaps to `renderMarkdown` +
`postRenderMarkdown` on `stream_end`.
- **#7** (Copilot): N+1 query pattern in
`CoordinatorClient.list_children()` — per-row `storage.get_workstream`
just to read `skill_id`. Pushed `skill_id` + `skill_version` into
the `list_workstreams` SELECT projection on both backends; the
client reads them from `row._mapping` directly. New
`test_list_children_skill_filter_avoids_n_plus_one` pins the
behaviour (asserts `storage.get_workstream` call count is 0).
- **#8 / #9** (Copilot): `spawn_workstream` tool JSON said "if empty,
the workstream is created idle" but the prepare method rejected
empty and the field was marked required. Resolved by allowing
empty end-to-end: removed from `required`, prepare builds a
"spawn idle workstream" header + empty preview when empty,
updated `test_spawn_prepare_allows_empty_initial_message`.
- **#1–#5** (github-code-quality): five asserts with side-effecting
method calls in `test_coordinator_manager.py` (`mgr.close`,
`mgr.open`, `mgr.create` in a dead `_c = ...`). Extracted each
call to a local variable so `python -O` can't strip the side
effect.
Verification:
- `ruff check turnstone tests` clean.
- `mypy turnstone` clean (156 source files).
- `pytest -m "not live"` — 4063 passed, 3 deselected (live-backend
tests), 0 failed. The 3 image tests that were failing on this
branch now pass; wheel + lint both green locally.
* polish(coordinator): address Copilot re-review findings
Two findings from the re-review of #368 after the first polish commit.
**user_id wired into `mgr.create()` at the server handlers.** Phase 1
added ``user_id`` to the ``Workstream`` dataclass and
``WorkstreamManager.create()`` signature, but the two call sites in
``turnstone/server.py`` forgot to pass the authenticated caller
through. Result: interactive workstreams created via
``POST /v1/api/workstreams/new`` (including coordinator-spawned
children, which route through this handler) were landing with blank
``user_id``, defeating ownership-based access control on subsequent
sends / approvals / closes (``_require_ws_access`` treats blank
owners as legacy/allowed). Two changes:
- ``server.py:create_workstream`` forwards ``user_id=uid`` — the same
``uid`` already resolved from the auth result (with trusted-service
forwarding preserved).
- ``server.py:open_workstream`` prefers the persisted owner on the
workstream row over the rehydrating caller so reloading someone
else's workstream doesn't silently re-parent it. Falls back to
the authenticated caller when the stored row has no owner
recorded (pre-phase-1 rows).
Regression test in ``tests/test_workstream.py`` pins
``WorkstreamManager.create(user_id=X)`` → ``ws.user_id == X`` so the
manager seam can't regress silently on a future refactor.
**Malformed-JSON recovery allowlist expanded for coordinator args.**
``_prepare_tool()`` has a two-stage salvage path for models that
emit malformed JSON: a regex-extract (fallback 1) and a bare-string
→ primary_key wrap (fallback 2). The fallback-1 key list didn't
include coordinator argument names, so a slightly malformed
``spawn_workstream`` / ``send_to_workstream`` / etc. call would
hard-fail instead of salvaging into a minimal-args dict for retry.
Added ``ws_id`` / ``message`` / ``initial_message`` / ``parent_ws_id``
to the allowlist (kept alphabetised) so the coordinator tools get
the same model-self-correction behaviour as the interactive tools.
Fallback 2 already covers the ``ws_id``-primary-key tools via
``PRIMARY_KEY_MAP``; the regex path matters when the model emits
``{"ws_id": "abc", "message": "..."}`` with a trailing syntax error.
Verification: ``ruff check`` clean, ``mypy turnstone`` clean
(156 source files), ``pytest -m "not live"`` → 4065 passed, 3
deselected (live-backend), 0 failed.
* fix(coordinator): address ultrareview findings on coordinator workstream kind
Security
- Cross-tenant leak: CoordinatorClient.inspect/list_children now constrain
to the coordinator's own ws_id + direct children; an LLM coerced via
prompt injection can no longer exfiltrate other tenants' workstreams.
- Empty-owner short-circuit bypass: strict equality at coordinator.py
ownership gate and at the storage-fallback branch in coordinator_history;
orphan/system-owned coordinator rows can no longer be rehydrated by
arbitrary holders of admin.coordinator (DoS + history disclosure vector).
- Closed coordinators no longer silently resurrect on subsequent GET —
the Close button is now actually durable across URL revisits and tab
refreshes; rows with state in {closed, deleted} refuse rehydration.
Correctness
- ChatSession.close() now releases the CoordinatorClient httpx.Client
pool; previously every closed/evicted coordinator dropped a connection
pool on the floor until non-deterministic GC.
- open_workstream rehydration now forwards parent_ws_id + kind, so
coordinator-spawned children survive node restart / idle eviction
with their parent link intact instead of becoming silent orphans.
- list_children truncated flag now signals whenever the SQL fetch hit
the page cap (previously permanently False in the no-filter case,
causing confident-but-incomplete summaries from the coordinator).
- ConsoleCoordinatorUI.approve_tools: per-tool auto-approve now checks
auto_approve_tools independently of the blanket auto_approve flag,
so 'Always approve this tool' actually works on the next invocation.
Concurrency
- _spawn_worker no longer falls through to start a second concurrent
worker thread on the same ChatSession when queue.Full fires; instead
send() returns False and the endpoint surfaces HTTP 429.
- _open_locks entries are now refcounted under self._lock and only
popped when the last waiter releases — eliminates the race where a
rehydration-failure path lets two threads serialize on different lock
instances for the same ws_id and trip the "already tracked" guard.
Tests: +6 regression cases covering closed-coordinator refusal,
empty-owner non-admin refusal, queue.Full no-duplicate-worker,
inspect/list_children cross-tenant rejection, and truncated semantics.
333 lines
11 KiB
Python
333 lines
11 KiB
Python
"""Tests for Phase A schema additions: ``kind`` + ``parent_ws_id`` on workstreams.
|
|
|
|
Covers:
|
|
|
|
- ``register_workstream`` persists the two new columns.
|
|
- ``get_workstream`` returns the full row including the new fields.
|
|
- ``list_workstreams`` filters on ``kind`` and ``parent_ws_id`` correctly.
|
|
- ``parent_ws_id`` empty-string normalization at the storage edge.
|
|
- Defaults remain ``"interactive"`` / ``NULL`` when not specified.
|
|
- ``Workstream`` dataclass exposes ``kind`` / ``parent_ws_id`` / ``user_id``
|
|
with safe defaults.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import pytest
|
|
|
|
from turnstone.core.storage._sqlite import SQLiteBackend
|
|
from turnstone.core.workstream import Workstream
|
|
|
|
|
|
@pytest.fixture
|
|
def storage(tmp_path):
|
|
return SQLiteBackend(str(tmp_path / "test.db"))
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# register_workstream / get_workstream
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_register_defaults_to_interactive_no_parent(storage):
|
|
storage.register_workstream("ws-a")
|
|
row = storage.get_workstream("ws-a")
|
|
assert row is not None
|
|
assert row["kind"] == "interactive"
|
|
assert row["parent_ws_id"] is None
|
|
|
|
|
|
def test_register_coordinator_kind_and_parent(storage):
|
|
storage.register_workstream("ws-coord", node_id="console", user_id="user-1", kind="coordinator")
|
|
storage.register_workstream(
|
|
"ws-child",
|
|
node_id="node-a",
|
|
user_id="user-1",
|
|
kind="interactive",
|
|
parent_ws_id="ws-coord",
|
|
)
|
|
|
|
coord = storage.get_workstream("ws-coord")
|
|
child = storage.get_workstream("ws-child")
|
|
|
|
assert coord is not None and child is not None
|
|
assert coord["kind"] == "coordinator"
|
|
assert coord["parent_ws_id"] is None
|
|
assert coord["user_id"] == "user-1"
|
|
|
|
assert child["kind"] == "interactive"
|
|
assert child["parent_ws_id"] == "ws-coord"
|
|
assert child["user_id"] == "user-1"
|
|
|
|
|
|
def test_register_normalizes_empty_parent_to_null(storage):
|
|
"""Empty-string parent_ws_id must be persisted as NULL so
|
|
``WHERE parent_ws_id IS NULL`` filters stay correct."""
|
|
storage.register_workstream("ws-a", parent_ws_id="")
|
|
row = storage.get_workstream("ws-a")
|
|
assert row is not None
|
|
assert row["parent_ws_id"] is None
|
|
|
|
|
|
def test_get_workstream_missing_returns_none(storage):
|
|
assert storage.get_workstream("nonexistent") is None
|
|
|
|
|
|
def test_get_workstream_includes_all_fields(storage):
|
|
storage.register_workstream(
|
|
"ws-full",
|
|
node_id="n1",
|
|
user_id="u1",
|
|
alias="alias-1",
|
|
title="Title 1",
|
|
name="name-1",
|
|
state="idle",
|
|
skill_id="skill-x",
|
|
skill_version=3,
|
|
kind="interactive",
|
|
parent_ws_id="parent-x",
|
|
)
|
|
row = storage.get_workstream("ws-full")
|
|
assert row is not None
|
|
for expected in (
|
|
"ws_id",
|
|
"node_id",
|
|
"user_id",
|
|
"alias",
|
|
"title",
|
|
"name",
|
|
"state",
|
|
"skill_id",
|
|
"skill_version",
|
|
"kind",
|
|
"parent_ws_id",
|
|
"created",
|
|
"updated",
|
|
):
|
|
assert expected in row
|
|
assert row["skill_version"] == 3
|
|
assert row["parent_ws_id"] == "parent-x"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# list_workstreams filter params
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_list_workstreams_no_filters_unchanged(storage):
|
|
storage.register_workstream("ws-a")
|
|
storage.register_workstream("ws-b")
|
|
rows = storage.list_workstreams()
|
|
assert len(rows) == 2
|
|
|
|
|
|
def test_list_workstreams_filter_by_kind(storage):
|
|
storage.register_workstream("ws-int-1")
|
|
storage.register_workstream("ws-int-2")
|
|
storage.register_workstream("ws-coord", kind="coordinator")
|
|
|
|
interactive = storage.list_workstreams(kind="interactive")
|
|
coord = storage.list_workstreams(kind="coordinator")
|
|
|
|
assert {r[0] for r in interactive} == {"ws-int-1", "ws-int-2"}
|
|
assert {r[0] for r in coord} == {"ws-coord"}
|
|
|
|
|
|
def test_list_workstreams_filter_by_parent(storage):
|
|
storage.register_workstream("ws-coord", kind="coordinator")
|
|
storage.register_workstream("child-1", parent_ws_id="ws-coord")
|
|
storage.register_workstream("child-2", parent_ws_id="ws-coord")
|
|
storage.register_workstream("other-1") # no parent
|
|
|
|
children = storage.list_workstreams(parent_ws_id="ws-coord")
|
|
assert {r[0] for r in children} == {"child-1", "child-2"}
|
|
|
|
|
|
def test_list_workstreams_combined_filters(storage):
|
|
storage.register_workstream("ws-coord", kind="coordinator")
|
|
storage.register_workstream("child-1", parent_ws_id="ws-coord")
|
|
storage.register_workstream("child-coord", parent_ws_id="ws-coord", kind="coordinator")
|
|
|
|
# Children of ws-coord that are themselves interactive.
|
|
rows = storage.list_workstreams(parent_ws_id="ws-coord", kind="interactive")
|
|
assert {r[0] for r in rows} == {"child-1"}
|
|
|
|
|
|
def test_list_workstreams_node_id_filter_still_works(storage):
|
|
"""The existing ``node_id`` filter keeps working after the signature change."""
|
|
storage.register_workstream("ws-a", node_id="node-1")
|
|
storage.register_workstream("ws-b", node_id="node-2")
|
|
rows = storage.list_workstreams(node_id="node-1")
|
|
assert {r[0] for r in rows} == {"ws-a"}
|
|
|
|
|
|
def test_list_workstreams_returns_kind_and_parent_columns(storage):
|
|
storage.register_workstream("ws-coord", kind="coordinator")
|
|
storage.register_workstream("child-1", parent_ws_id="ws-coord")
|
|
rows = storage.list_workstreams()
|
|
by_id = {r[0]: r for r in rows}
|
|
# Columns: ws_id, node_id, name, state, created, updated, kind, parent_ws_id
|
|
coord_row = by_id["ws-coord"]
|
|
child_row = by_id["child-1"]
|
|
assert coord_row[6] == "coordinator"
|
|
assert coord_row[7] is None
|
|
assert child_row[6] == "interactive"
|
|
assert child_row[7] == "ws-coord"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Workstream dataclass field additions
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_workstream_dataclass_defaults():
|
|
ws = Workstream()
|
|
assert ws.user_id == ""
|
|
assert ws.kind == "interactive"
|
|
assert ws.parent_ws_id is None
|
|
|
|
|
|
def test_workstream_dataclass_accepts_coordinator_kind():
|
|
ws = Workstream(kind="coordinator", user_id="user-1")
|
|
assert ws.kind == "coordinator"
|
|
assert ws.user_id == "user-1"
|
|
assert ws.parent_ws_id is None
|
|
|
|
|
|
def test_workstream_dataclass_accepts_parent():
|
|
ws = Workstream(parent_ws_id="parent-x")
|
|
assert ws.parent_ws_id == "parent-x"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Tool-namespace isolation between kinds
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_interactive_and_coordinator_tool_sets_are_disjoint():
|
|
"""Interactive sessions must not see coordinator tools and vice versa.
|
|
|
|
Regression guard for the latent threshold bug where coordinator tools
|
|
counted against the interactive session's tool-search threshold, and
|
|
a future reader might naively expose ``TOOLS`` (the union) to an
|
|
interactive session.
|
|
"""
|
|
from turnstone.core.tools import COORDINATOR_TOOLS, INTERACTIVE_TOOLS, TOOLS
|
|
|
|
interactive_names = {t["function"]["name"] for t in INTERACTIVE_TOOLS}
|
|
coord_names = {t["function"]["name"] for t in COORDINATOR_TOOLS}
|
|
|
|
# No overlap.
|
|
assert interactive_names.isdisjoint(coord_names), (
|
|
f"interactive ∩ coordinator tools should be empty, got {interactive_names & coord_names}"
|
|
)
|
|
# Coordinator set is non-empty (spawn/inspect/send/close/delete/list).
|
|
assert coord_names, "expected at least one coordinator tool"
|
|
# Union covers every loaded tool (no tool is in neither set).
|
|
all_names = {t["function"]["name"] for t in TOOLS}
|
|
assert interactive_names | coord_names == all_names
|
|
|
|
|
|
def test_chatsession_interactive_kind_excludes_coordinator_tools(tmp_db):
|
|
"""An interactive ``ChatSession`` does not surface coordinator tools."""
|
|
from unittest.mock import MagicMock
|
|
|
|
from turnstone.core.session import ChatSession
|
|
|
|
class _NullUI:
|
|
def __getattr__(self, _name):
|
|
return lambda *a, **kw: None
|
|
|
|
sess = ChatSession(
|
|
client=MagicMock(),
|
|
model="test-model",
|
|
ui=_NullUI(),
|
|
instructions=None,
|
|
temperature=0.5,
|
|
max_tokens=4096,
|
|
tool_timeout=30,
|
|
)
|
|
names = {t["function"]["name"] for t in sess._tools}
|
|
# None of the coordinator-only names should be in the interactive
|
|
# session's tool set.
|
|
for coord_name in (
|
|
"spawn_workstream",
|
|
"inspect_workstream",
|
|
"send_to_workstream",
|
|
"close_workstream",
|
|
"delete_workstream",
|
|
"list_workstreams",
|
|
):
|
|
assert coord_name not in names, f"{coord_name} leaked into interactive session tools"
|
|
|
|
|
|
def test_chatsession_coordinator_kind_excludes_interactive_tools(tmp_db):
|
|
"""A coordinator ``ChatSession`` sees only coordinator tools."""
|
|
from unittest.mock import MagicMock
|
|
|
|
from turnstone.core.session import ChatSession
|
|
|
|
class _NullUI:
|
|
def __getattr__(self, _name):
|
|
return lambda *a, **kw: None
|
|
|
|
sess = ChatSession(
|
|
client=MagicMock(),
|
|
model="test-model",
|
|
ui=_NullUI(),
|
|
instructions=None,
|
|
temperature=0.5,
|
|
max_tokens=4096,
|
|
tool_timeout=30,
|
|
kind="coordinator",
|
|
)
|
|
names = {t["function"]["name"] for t in sess._tools}
|
|
# Coordinator tools present, interactive tools absent.
|
|
assert "spawn_workstream" in names
|
|
assert "bash" not in names
|
|
assert "edit_file" not in names
|
|
assert "memory" not in names
|
|
# Sub-agent tool lists are zeroed for coordinators.
|
|
assert sess._task_tools == []
|
|
assert sess._agent_tools == []
|
|
|
|
|
|
def test_chatsession_coordinator_kind_does_not_merge_mcp_tools(tmp_db):
|
|
"""Coordinator ChatSession ignores any attached MCP client tool surface.
|
|
|
|
Coordinators are meta-orchestrators that spawn child workstreams;
|
|
MCP tools live on the children. Giving the coordinator direct MCP
|
|
access defeats the child-spawning pattern.
|
|
"""
|
|
from unittest.mock import MagicMock
|
|
|
|
from turnstone.core.session import ChatSession
|
|
|
|
class _NullUI:
|
|
def __getattr__(self, _name):
|
|
return lambda *a, **kw: None
|
|
|
|
mcp_client = MagicMock()
|
|
mcp_client.get_tools.return_value = [
|
|
{"type": "function", "function": {"name": "mcp__foo__bar", "parameters": {}}}
|
|
]
|
|
sess = ChatSession(
|
|
client=MagicMock(),
|
|
model="test-model",
|
|
ui=_NullUI(),
|
|
instructions=None,
|
|
temperature=0.5,
|
|
max_tokens=4096,
|
|
tool_timeout=30,
|
|
kind="coordinator",
|
|
mcp_client=mcp_client,
|
|
)
|
|
names = {t["function"]["name"] for t in sess._tools}
|
|
# No MCP tools in the coordinator surface.
|
|
assert "mcp__foo__bar" not in names
|
|
# And no MCP listeners were registered (defence-in-depth: MCP tool
|
|
# refreshes can't mutate the coordinator's fixed tool set).
|
|
mcp_client.add_listener.assert_not_called()
|
|
mcp_client.add_resource_listener.assert_not_called()
|
|
mcp_client.add_prompt_listener.assert_not_called()
|