feat(coordinator): MCP tool surface for coordinator sessions (#725)

Coordinator-kind workstreams get the same MCP surface as interactive
sessions — tools, resources, and prompts (read_resource/use_prompt go
dual-kind) — gated per-persona exactly like interactive, with no
separate feature flag.

The console hosts its manager with node parity end to end: boot calls
create_mcp_client inline (same catalog resolution: DB rows, then
mcp.config_path, then this host's config.toml), the admin reload
fan-out lazily constructs and reconciles it under a lock (the node's
unlocked equivalent is #873), per-server refresh/reconnect and the
admin MCP status view cover it under the collector's console
pseudo-node id, and shutdown follows LIFO teardown. Sessions read the
live manager through a per-construction getter — the console
counterpart of the node factory's mcp_ref[0] read; client presence is
the session-level contract, and the kind-aware tool assembly runs the
same listener/prime/rebind skeleton as interactive. bind_acting_user
re-scopes listeners and per-user pools, which is security-critical for
multi-sender coordinators.

The wire-safety status projections move verbatim to core/mcp_utils so
both hosts present one schema (node endpoint bodies byte-identical);
the console's per-server action classification is a pinned COPY of the
node endpoints', with a parity test driving both sides across the
outcome matrix that fails if either drifts.

The shared MCP error card (consent / re-consent / forbidden / operator)
moves to mcp_error.js + mcp_error.css, linked by all three card hosts
and pinned by className→rule and host→link parity tests; the module
joins the whole-file sink-scan and var-ratchet lists. Reload reporting
is honest about the console entry: excluded from the unreached-node
warning's list and denominator, and the toast claims "+ console" only
for a real reconcile, with an explicit note on failure.

The pending-consent badge (#874's console half) ships too: the console
defines the same onConsentDetected seam the node dashboard exposes —
lighting up the shared pane host's existing bridge for hosted
interactive panes — and the coordinator pane threads its card's
detections through the single MCP-error helper. The badge rides the
Admin > MCP Servers rail row, hydrates at boot from the Phase 9
pending-consent endpoint the console already serves, re-syncs to DB
truth when the operator views the MCP panel, and the rail-less
standalone page carries a status-bar chip instead. A coordinator that
hits a consent wall unattended now has a persistent, glanceable signal.

Pre-existing bugs fixed along the way: create_mcp_client returned None
on pool-only installs, leaving any host managerless after restart until
the next admin MCP write; admin_import_mcp_config never scheduled the
reload fan-out (stale catalogs after import); the admin settings UI
rendered the coordinator settings section unordered and unlabeled.
Follow-ups: #873 (node reload double-construct race); #874 narrows to
the admin-MCP-view per-server indicator.
This commit is contained in:
Patrick Buckley
2026-07-20 01:46:43 -07:00
parent 5ae963cb49
commit 984a10307e
29 changed files with 2208 additions and 476 deletions
+162 -14
View File
@@ -19,6 +19,7 @@ import pytest
_APP_JS = Path(__file__).resolve().parent.parent / "turnstone/ui/static/app.js"
_INTERACTIVE_JS = Path(__file__).resolve().parent.parent / "turnstone/shared_static/interactive.js"
_MCP_ERROR_JS = Path(__file__).resolve().parent.parent / "turnstone/shared_static/mcp_error.js"
_SHELL_JS = Path(__file__).resolve().parent.parent / "turnstone/shared_static/shell.js"
_REDACT_CREDENTIALS_JS = (
Path(__file__).resolve().parent.parent / "turnstone/shared_static/redact_credentials.js"
@@ -433,20 +434,27 @@ _UNSAFE_CODE_SINK_RE = re.compile(
def test_phase8_mcp_error_helpers_defined() -> None:
"""``tryParseMcpError`` (envelope detector) + ``buildMcpErrorEmbed``
(interactive consent / forbidden / operator card) moved into the shared
interactive module with the Pane. The consent-badge state
(``_pendingConsentServers`` / ``_onConsentDetected``) stays in the
standalone shell — it drives the rail's Manage-row badge — and the pane
reaches it through the ``host.onConsentDetected`` seam. The shared host
bridges that seam to the standalone via ``window.TS_APP.onConsentDetected``
(undefined on the console, so it stays a no-op there). Pin both halves and
(consent / forbidden / operator card) live in the shared ``mcp_error.js``
module — lifted out of interactive.js by #725 so BOTH conversation
surfaces render the same card: the interactive pane AND the coordinator
pane (whose sessions carry the same MCP surface, persona-gated).
The consent-badge state (``_pendingConsentServers`` /
``_onConsentDetected``) stays in the standalone shell — it drives the
rail's Manage-row badge — and the pane reaches it through the
``host.onConsentDetected`` seam. The shared host bridges that seam to the
standalone via ``window.TS_APP.onConsentDetected`` (undefined on the
console, so it stays a no-op there). Pin the module, both consumers, and
the bridge."""
inter = _INTERACTIVE_JS.read_text(encoding="utf-8")
assert "function tryParseMcpError" in inter
assert "function buildMcpErrorEmbed" in inter
mod = _MCP_ERROR_JS.read_text(encoding="utf-8")
assert "function tryParseMcpError" in mod
assert "function buildMcpErrorEmbed" in mod
# The actionable branch surfaces consent via the THREADED callback, not a
# direct shell call — that decoupling is what lets the console no-op it.
assert "if (onConsent) onConsent(err.server)" in inter
assert "if (onConsent) onConsent(err.server)" in mod
inter = _INTERACTIVE_JS.read_text(encoding="utf-8")
assert 'from "./mcp_error.js"' in inter, (
"the interactive pane must consume the shared MCP error module"
)
assert "onConsentDetected(s)" in inter, (
"the pane must notify consent through host.onConsentDetected"
)
@@ -455,6 +463,15 @@ def test_phase8_mcp_error_helpers_defined() -> None:
assert "window.TS_APP.onConsentDetected(server)" in inter, (
"the shared interactive host must bridge onConsentDetected to the TS_APP seam"
)
# The coordinator pane is the second consumer (#725): a coordinator MCP
# dispatch hitting consent-required must render the card, not raw JSON.
coord = _COORD_JS.read_text(encoding="utf-8")
assert '"/shared/mcp_error.js"' in coord, (
"the coordinator pane must import the shared MCP error module (#725)"
)
assert "tryParseMcpError(" in coord and "buildMcpErrorEmbed(" in coord, (
"the coordinator pane must dispatch structured MCP errors to the shared card"
)
app = _APP_JS.read_text(encoding="utf-8")
assert "_pendingConsentServers" in app
assert "function _onConsentDetected" in app
@@ -570,6 +587,7 @@ _UNSAFE_CODE_SINK_LINT_TARGETS = [
("turnstone/shared_static/utils.js", _UTILS_JS),
("turnstone/shared_static/auth.js", _AUTH_JS),
("turnstone/shared_static/kb.js", _KB_JS),
("turnstone/shared_static/mcp_error.js", _MCP_ERROR_JS),
("turnstone/console/static/coordinator/coordinator.js", _COORD_JS),
("turnstone/console/static/admin.js", _CONSOLE_ADMIN_JS),
("turnstone/console/static/governance.js", _CONSOLE_GOVERNANCE_JS),
@@ -860,7 +878,7 @@ def test_phase8_xss_safe_render_in_build_mcp_error_embed() -> None:
scopes list. The card builder uses createElement + textContent
throughout so a script-tag server name renders harmlessly. Pin
the absence of the unsafe-write inside ``buildMcpErrorEmbed``."""
body = _INTERACTIVE_JS.read_text(encoding="utf-8")
body = _MCP_ERROR_JS.read_text(encoding="utf-8")
start = body.index("function buildMcpErrorEmbed(")
# Bound to the function body — find its closing brace at column 0.
rest = body[start:]
@@ -883,7 +901,7 @@ def test_mcp_error_button_gated_on_consent_url_not_code_alone() -> None:
obo rows) produced a button that dead-ended in a 'no consent URL' toast.
The button must render only when a valid per-server consent URL is present —
obo errors show the card's honest detail text without a broken affordance."""
body = _INTERACTIVE_JS.read_text(encoding="utf-8")
body = _MCP_ERROR_JS.read_text(encoding="utf-8")
start = body.index("function buildMcpErrorEmbed(")
rest = body[start:]
end_match = re.search(r"\n}\n", rest)
@@ -939,7 +957,7 @@ def test_phase8_consent_url_prefix_check_in_click_handler() -> None:
string and the ``startsWith`` form so a future refactor can't
silently weaken the guard.
"""
body = _INTERACTIVE_JS.read_text(encoding="utf-8")
body = _MCP_ERROR_JS.read_text(encoding="utf-8")
# Bound the search to the click handler region (between the
# ``buildMcpErrorEmbed`` function and the next top-level helper) to
# avoid false positives from unrelated string occurrences.
@@ -2879,3 +2897,133 @@ def test_projects_personas_keep_public_surface_on_factory() -> None:
"onPersonasChange",
):
assert f"function {name}(" in persona, f"personas.js must still export {name}"
_MCP_ERROR_CSS = Path(__file__).resolve().parent.parent / "turnstone/shared_static/mcp_error.css"
# The categories _mcpErrorCategory can return that carry their own CSS —
# buildMcpErrorEmbed's interpolated `"mcp-error-" + category` class expands
# to these plus "actionable", which is deliberately unstyled (the base card
# look IS the actionable look: accent icon + Connect button).
_MCP_ERROR_STYLED_CATEGORIES = ("operator", "transient", "forbidden")
def test_mcp_error_css_owns_every_card_class() -> None:
"""Every class mcp_error.js assigns has a rule in mcp_error.css.
The dual-static reach class (#725 review round 1: the standalone
coordinator page rendered the consent card bare because the card's
rules lived only in a host sheet it never linked). The sheet pairs
with the module and every card host links it; this pin makes the
invariant structural — add a class to the card and this fails until
mcp_error.css owns it."""
js = _MCP_ERROR_JS.read_text(encoding="utf-8")
css = _MCP_ERROR_CSS.read_text(encoding="utf-8")
tokens: set[str] = set()
for assignment in re.findall(r'className\s*=\s*"([^"]+)"', js):
for tok in assignment.split():
if tok.endswith("-"):
# Interpolated category suffix ("mcp-error-" + category).
tokens.update(tok + cat for cat in _MCP_ERROR_STYLED_CATEGORIES)
else:
tokens.add(tok)
assert "mcp-error-card" in tokens, "class extractor found nothing — regex rotted?"
missing = sorted(t for t in tokens if ("." + t) not in css)
assert not missing, f"mcp_error.css lacks rules for: {missing}"
def test_mcp_error_css_linked_by_every_card_host() -> None:
"""Every page that renders the MCP error card links its sheet — the
other half of the reach invariant (a future host that imports
mcp_error.js without the stylesheet regresses to bare markup)."""
root = Path(__file__).resolve().parent.parent
hosts = (
root / "turnstone/console/static/coordinator/index.html",
root / "turnstone/console/static/index.html",
root / "turnstone/ui/static/index.html",
)
for host in hosts:
html = host.read_text(encoding="utf-8")
assert "/shared/mcp_error.css" in html, f"{host.name} must link /shared/mcp_error.css"
def test_console_consent_badge_seam() -> None:
"""#874 badge half, console side: the console defines the same
TS_APP.onConsentDetected seam the node dashboard does (which the shared
pane host bridges interactive panes' detections to), hydrates from the
Phase 9 persistence endpoint at boot, and badges the Admin > MCP
Servers rail row through the shell bridge."""
js = _CONSOLE_APP_JS.read_text(encoding="utf-8")
assert "window.TS_APP.onConsentDetected" in js
assert '"/v1/api/mcp/oauth/pending"' in js
assert 'setRowBadge("mcp"' in js
assert "window.TS_APP.syncConsentBadge" in js
def test_admin_mcp_panel_resyncs_consent_badge() -> None:
"""The admin MCP panel re-syncs the badge to DB truth AFTER a
successful render (node Connections-flow semantics) — and only then:
a failed load keeps the pending signal. The order is asserted inside
loadAdminMcp's body (a whole-file check would bind to unrelated
earlier .catch( sites and false-pass a move into the failure path)."""
js = _CONSOLE_ADMIN_JS.read_text(encoding="utf-8")
body = js[js.index("function loadAdminMcp(") :]
body = body[: body.index("\nfunction ")]
assert (
body.index("_renderMcpServers(") < body.index("syncConsentBadge") < body.index(".catch(")
), "syncConsentBadge must sit in loadAdminMcp's success path, after the render"
def test_coordinator_pane_threads_consent_detection() -> None:
"""#874 badge half, coordinator side: the single MCP-error helper
passes the consent callback to the card builder (both result paths get
detection), forwarding to the L-shell seam and the standalone
status-bar chip."""
js = _COORD_JS.read_text(encoding="utf-8")
assert "buildMcpErrorEmbed(mcpErr, output, _notifyConsentDetected)" in js
assert "onConsentDetected" in js
assert "coord-sb-consent" in js
# The chip is standalone-only chrome; the L-shell pane relies on the
# rail badge.
assert "opts.standalone" in js
def test_consent_badge_visibility_resync_pins() -> None:
"""#874 self-heal: both consent surfaces re-pull server truth on the
visibility-show edge (the consent popup is noopener — no cross-window
channel exists), with merge-on-success semantics: the mirrors diff
against a preFetch snapshot so a detection landing mid-fetch survives
and a failed fetch never blanks a possibly-valid warning. A blind
clear()-then-refill would clobber mid-fetch adds — the preFetch
marker is the pin against that regression."""
app = _CONSOLE_APP_JS.read_text(encoding="utf-8")
assert 'document.addEventListener("visibilitychange"' in app
assert "_resyncPendingConsents" in app
assert app.count("preFetch") >= 2
# Single-flight: overlapping fetches can resolve out of order and no
# ordering guard covers every interleaving (stale clobber, failure
# suppression, phantom re-adds) — exclusion plus one queued rerun is
# the pinned shape, with a bounded flight so a stalled fetch cannot
# wedge the gate shut. The timeout signal is feature-detected (old
# runtimes, per the codebase's AbortController guards) so it can
# never throw the gate wedged.
assert "_resyncInFlight" in app
assert "_resyncQueued" in app
assert app.count("AbortSignal.timeout") >= 2 # guard + use
coord = _COORD_JS.read_text(encoding="utf-8")
assert 'document.addEventListener("visibilitychange"' in coord
assert coord.count("preFetch") >= 2
assert "hydrateInFlight" in coord
assert "hydrateQueued" in coord
assert coord.count("AbortSignal.timeout") >= 2 # guard + use
def test_reload_toast_console_phrasing_pins() -> None:
"""Reload-toast truth table (#725): a node-less install reads
'Reload sent to console' instead of '0 node(s) + console'; a failed
console entry appends its explicit note; and failed beats reconciled
if a malformed entry ever carries both shapes."""
js = _CONSOLE_ADMIN_JS.read_text(encoding="utf-8")
assert '"Reload sent to console"' in js
assert '"; console reload failed"' in js
assert "!consoleFailed &&" in js
+88
View File
@@ -217,3 +217,91 @@ def test_whitespace_only_coord_alias_falls_through() -> None:
)
_invoke(factory)
assert registry.captured_alias == "registry-default"
# ---------------------------------------------------------------------------
# Coordinator MCP gate (#725) — flag × getter matrix, resolved per construction
# ---------------------------------------------------------------------------
def _capture_chatsession_kwargs(
*,
settings: dict[str, Any],
mcp_client_getter: Any = None,
getter_passed: bool = True,
) -> Any:
"""Run the factory through to a (patched) ChatSession and return the
captured construction kwargs. ChatSession's own contract is covered
elsewhere; the unit under test here is the factory's MCP gate."""
from unittest.mock import patch
from tests._coord_test_helpers import _fake_registry
extra: dict[str, Any] = {}
if getter_passed:
extra["mcp_client_getter"] = mcp_client_getter
factory = build_console_session_factory(
registry=_fake_registry(),
config_store=_FakeConfigStore(dict(settings)), # type: ignore[arg-type]
node_id="console",
coord_client_factory=lambda ws_id, uid: MagicMock(),
**extra,
)
ui = MagicMock()
ui._user_id = ""
with patch("turnstone.console.session_factory.ChatSession") as cs:
factory(ui, ws_id="w1")
assert cs.call_count == 1
return cs.call_args.kwargs
def test_mcp_getter_passes_live_manager_unconditionally() -> None:
"""Node parity: the factory passes the live console manager to every
coordinator session (the console counterpart of the node factory's
mcp_ref[0] read) — whether MCP tools surface is the persona's call,
exactly as for interactive sessions."""
manager = MagicMock()
got = _capture_chatsession_kwargs(settings={}, mcp_client_getter=lambda: manager)
assert got["mcp_client"] is manager
def test_mcp_getter_none_manager_passes_none() -> None:
"""Nothing configured (create_mcp_client returned None): the session
gets None, not a crash."""
got = _capture_chatsession_kwargs(settings={}, mcp_client_getter=lambda: None)
assert got["mcp_client"] is None
def test_mcp_no_getter_is_backward_compatible() -> None:
got = _capture_chatsession_kwargs(settings={}, getter_passed=False)
assert got["mcp_client"] is None
def test_mcp_getter_resolved_per_construction() -> None:
"""The getter is consulted at EVERY construction — a manager
(re)constructed by the console ensure-helper after factory build must
reach the next session. An instance captured at factory-build time
fails this row."""
from unittest.mock import patch
from tests._coord_test_helpers import _fake_registry
holder: dict[str, Any] = {"mgr": None}
factory = build_console_session_factory(
registry=_fake_registry(),
config_store=_FakeConfigStore({}), # type: ignore[arg-type]
node_id="console",
coord_client_factory=lambda ws_id, uid: MagicMock(),
mcp_client_getter=lambda: holder["mgr"],
)
ui = MagicMock()
ui._user_id = ""
with patch("turnstone.console.session_factory.ChatSession") as cs:
factory(ui, ws_id="w1")
first = cs.call_args.kwargs["mcp_client"]
manager = MagicMock()
holder["mgr"] = manager # the ensure-helper lazily constructed it
factory(ui, ws_id="w2")
second = cs.call_args.kwargs["mcp_client"]
assert first is None
assert second is manager
+9
View File
@@ -162,6 +162,15 @@ def test_coordinator_session_uses_coordinator_tools(coord_session):
# surfacing to a human channel without spawning a child purely
# to ship the message. Routing is session-kind-agnostic.
"notify",
# ``read_resource``/``use_prompt`` joined in 1.8 (#725) — the
# coordinator MCP surface covers tools, resources, AND prompts.
# They sit in the BASE list unconditionally (like their
# interactive siblings), but the WIRE strips them when no client
# is attached or the per-user catalog counts are zero — the
# _without_tool gate in _get_active_tools, pinned by the wire
# matrix in test_workstream_kind.py.
"read_resource",
"use_prompt",
}
# Sub-agent tool set is zeroed on coordinator sessions.
assert sess._task_tools == []
+454 -11
View File
@@ -8,7 +8,7 @@ import logging
import uuid
from types import SimpleNamespace
from typing import TYPE_CHECKING, Any
from unittest.mock import AsyncMock, MagicMock, patch
from unittest.mock import ANY, AsyncMock, MagicMock, patch
import httpx
import pytest
@@ -24,9 +24,12 @@ if TYPE_CHECKING:
from turnstone.console.server import (
_collect_mcp_status,
_console_mcp_action_outcome,
_ensure_console_mcp_client,
_notify_nodes_mcp_reconnect_one,
_notify_nodes_mcp_refresh_one,
_notify_nodes_mcp_reload,
_schedule_mcp_reload,
admin_create_mcp_server,
admin_delete_mcp_server,
admin_get_mcp_server,
@@ -1769,6 +1772,40 @@ class TestImportMcpConfig:
assert r.status_code == 400
assert "config" in r.json()["error"].lower()
def test_import_notifies_nodes_when_rows_change(self, client):
"""Import creates enabled rows, so it must fan the reload out like
its CRUD siblings — a stale-catalog window otherwise opens on
every node (and the console's flag+rows-free ensure never fires)."""
with patch(
"turnstone.console.server._notify_nodes_mcp_reload",
new_callable=AsyncMock,
return_value={},
) as notify:
r = client.post(
"/v1/api/admin/mcp-servers/import",
json={"config": {"mcpServers": {"imp-new": {"command": "node", "args": []}}}},
)
assert r.status_code == 200
assert "imp-new" in r.json()["imported"]
notify.assert_awaited_once_with(ANY)
def test_import_all_skipped_schedules_nothing(self, client):
"""A 200 whose every name already existed changed zero rows —
matching the audit record's `if imported:` gate, no fan-out."""
_create_server(client, name="already-there")
with patch(
"turnstone.console.server._notify_nodes_mcp_reload",
new_callable=AsyncMock,
return_value={},
) as notify:
r = client.post(
"/v1/api/admin/mcp-servers/import",
json={"config": {"mcpServers": {"already-there": {"command": "node"}}}},
)
assert r.status_code == 200
assert r.json()["imported"] == []
notify.assert_not_awaited()
def test_import_no_mcp_servers_key(self, client):
r = client.post(
"/v1/api/admin/mcp-servers/import",
@@ -1812,9 +1849,18 @@ def _fake_request(*nodes: dict[str, Any], proxy_client: Any = None) -> MagicMock
collector = MagicMock()
collector.get_nodes.return_value = (list(nodes), len(nodes))
collector.get_all_nodes.side_effect = lambda: collector.get_nodes.return_value[0]
# Real pseudo-node id + no storage: the reload fan-out also runs the
# console's own MCP ensure-helper (#725), which reports under this key
# and deterministically skips when auth_storage is absent.
collector.CONSOLE_PSEUDO_NODE_ID = "console"
req = MagicMock()
req.state.auth_result = None
req.app.state.collector = collector
req.app.state.auth_storage = None
# Pinned None (MagicMock would auto-create a truthy attribute): the
# action/status console arms key on manager PRESENCE — cells that
# want the arm set req.app.state.mcp_client explicitly.
req.app.state.mcp_client = None
req.app.state.jwt_secret = ""
req.app.state.proxy_client = proxy_client or AsyncMock()
req.app.state.proxy_token_mgr = None
@@ -1899,6 +1945,48 @@ class TestCollectMcpStatus:
result = await _collect_mcp_status(req)
assert result == {"n1": {"s1": {"status": "ok"}}}
@pytest.mark.anyio
async def test_console_manager_reports_projected(self):
"""Console arm (#725): the console's rows ride the read-scope
projection node rows get — has_error present; command/url/verbose
error absent."""
req = _fake_request()
mgr = MagicMock()
mgr.get_all_server_status.return_value = {
"srv": {
"connected": False,
"tools": 0,
"resources": 0,
"prompts": 0,
"error": "ConnectError: https://internal.example",
"transport": "http",
"command": "",
"url": "https://internal.example",
"circuit_open": True,
"consecutive_failures": 2,
}
}
req.app.state.mcp_client = mgr
result = await _collect_mcp_status(req)
row = result["console"]["srv"]
assert row["has_error"] is True
assert "error" not in row
assert "command" not in row
assert "url" not in row
# Admin path mirror of internal_mcp_status: cross-user aggregate.
mgr.get_all_server_status.assert_called_once_with(ANY, aggregate=True)
@pytest.mark.anyio
async def test_console_read_failure_omits_key(self):
"""Failure mirrors _fetch's None contract: the console key is
omitted, never an error payload in a status map."""
req = _fake_request()
mgr = MagicMock()
mgr.get_all_server_status.side_effect = RuntimeError("boom")
req.app.state.mcp_client = mgr
result = await _collect_mcp_status(req)
assert result == {}
class TestNotifyNodesMcpReload:
@pytest.mark.anyio
@@ -1910,7 +1998,10 @@ class TestNotifyNodesMcpReload:
proxy_client=client,
)
result = await _notify_nodes_mcp_reload(req)
assert result == {"n1": {"reloaded": 3}}
assert result == {
"n1": {"reloaded": 3},
"console": {"skipped": "storage not initialized"},
}
@pytest.mark.anyio
async def test_skips_nodes_without_url(self):
@@ -1920,7 +2011,7 @@ class TestNotifyNodesMcpReload:
proxy_client=client,
)
result = await _notify_nodes_mcp_reload(req)
assert result == {}
assert result == {"console": {"skipped": "storage not initialized"}}
client.post.assert_not_called()
@pytest.mark.anyio
@@ -1962,7 +2053,10 @@ class TestNotifyNodesMcpReload:
async def test_empty_cluster(self):
req = _fake_request()
result = await _notify_nodes_mcp_reload(req)
assert result == {}
# No nodes reached, but the console's own ensure-helper always
# reports — the operator reload view shows the console row even
# on a node-less install.
assert result == {"console": {"skipped": "storage not initialized"}}
@pytest.mark.anyio
async def test_multiple_nodes_mixed(self):
@@ -1980,6 +2074,90 @@ class TestNotifyNodesMcpReload:
assert result["n1"] == {"reloaded": 2}
assert "error" in result["n2"]
@pytest.mark.anyio
async def test_console_ensure_failure_isolated(self):
"""A console-ensure crash must not abandon collected node results —
the exception is caught INSIDE the gathered coroutine (the gather
runs return_exceptions=False). Raise-injection is load-bearing:
under _fake_request auth_storage is None, so the real helper
returns a skip dict and can never raise — without the patch this
except arm is unreachable."""
client = AsyncMock()
client.post.return_value = _mock_resp(200, {"reloaded": 1})
req = _fake_request(
{"node_id": "n1", "server_url": "http://n1:8000"},
proxy_client=client,
)
with patch(
"turnstone.console.server._ensure_console_mcp_client",
side_effect=Exception("boom"),
):
result = await _notify_nodes_mcp_reload(req)
assert result["n1"] == {"reloaded": 1}
assert "error" in result["console"]
assert "boom" in result["console"]["error"]
class TestScheduleMcpReloadAccounting:
"""_schedule_mcp_reload._run's unreached-node accounting: the console's
pseudo-node entry must never count as an unreached NODE — not in the
warning's list, not in its denominator."""
@pytest.mark.anyio
async def test_console_error_alone_fires_no_node_warning(self, caplog):
"""A console-only failure (e.g. a node-less install) must not log
'did not reach ... node(s)' — its production site already warned
with console wording."""
req = _fake_request()
with (
patch(
"turnstone.console.server._notify_nodes_mcp_reload",
AsyncMock(return_value={"console": {"error": "ensure blew up"}}),
),
caplog.at_level(logging.WARNING, logger="turnstone.console.server"),
):
await _schedule_mcp_reload(req)()
assert "did not reach" not in caplog.text
@pytest.mark.anyio
async def test_console_excluded_from_list_and_denominator(self, caplog):
"""Mixed results: the unreached warning covers nodes only — the
console error appears in neither the list nor the '%d of %d'
denominator (pre-fix this logged '2 of 3 node(s)')."""
req = _fake_request()
results = {
"console": {"error": "console-local"},
"n1": {"error": "timeout"},
"n2": {"reloaded": 1},
}
with (
patch(
"turnstone.console.server._notify_nodes_mcp_reload",
AsyncMock(return_value=results),
),
caplog.at_level(logging.WARNING, logger="turnstone.console.server"),
):
await _schedule_mcp_reload(req)()
[rec] = [r for r in caplog.records if "did not reach" in r.getMessage()]
msg = rec.getMessage()
assert "1 of 2 node(s)" in msg
assert msg.rstrip().endswith("n1")
@pytest.mark.anyio
async def test_systemic_failure_warns_node_remediation(self, caplog):
"""An infra fault before the gather logs the node-facing
remediation prose."""
req = _fake_request()
with (
patch(
"turnstone.console.server._notify_nodes_mcp_reload",
AsyncMock(side_effect=RuntimeError("infra down")),
),
caplog.at_level(logging.WARNING, logger="turnstone.console.server"),
):
await _schedule_mcp_reload(req)()
assert "nodes may serve a stale MCP catalog" in caplog.text
# ---------------------------------------------------------------------------
# Console reload endpoint: POST /v1/api/admin/mcp-servers/reload
@@ -2079,7 +2257,7 @@ class TestMcpWriteAutoReload:
return_value={},
) as notify:
_create_server(client, name="auto-reload-create")
notify.assert_awaited_once()
notify.assert_awaited_once_with(ANY)
def test_update_notifies_nodes(self, client: TestClient) -> None:
sid = _create_server(client, name="auto-reload-update")["server_id"]
@@ -2303,7 +2481,10 @@ class TestNotifyNodesMcpRefreshOne:
proxy_client=client,
)
result = await _notify_nodes_mcp_refresh_one(req, "srv")
assert result == {"n1": {"status": "ok"}}
assert result == {
"n1": {"status": "ok"},
"console": {"skipped": "console MCP manager not running"},
}
# Verify the URL used the safe-encoded name segment
call_args = client.post.call_args
assert call_args[0][0].endswith("/v1/api/_internal/mcp-refresh/srv")
@@ -2316,7 +2497,7 @@ class TestNotifyNodesMcpRefreshOne:
proxy_client=client,
)
result = await _notify_nodes_mcp_refresh_one(req, "srv")
assert result == {}
assert result == {"console": {"skipped": "console MCP manager not running"}}
client.post.assert_not_called()
@pytest.mark.anyio
@@ -2352,7 +2533,7 @@ class TestNotifyNodesMcpRefreshOne:
async def test_empty_cluster(self):
req = _fake_request()
result = await _notify_nodes_mcp_refresh_one(req, "srv")
assert result == {}
assert result == {"console": {"skipped": "console MCP manager not running"}}
class TestNotifyNodesMcpReconnectOne:
@@ -2365,7 +2546,10 @@ class TestNotifyNodesMcpReconnectOne:
proxy_client=client,
)
result = await _notify_nodes_mcp_reconnect_one(req, "srv")
assert result == {"n1": {"status": "ok"}}
assert result == {
"n1": {"status": "ok"},
"console": {"skipped": "console MCP manager not running"},
}
call_args = client.post.call_args
assert call_args[0][0].endswith("/v1/api/_internal/mcp-reconnect/srv")
@@ -2377,7 +2561,7 @@ class TestNotifyNodesMcpReconnectOne:
proxy_client=client,
)
result = await _notify_nodes_mcp_reconnect_one(req, "srv")
assert result == {}
assert result == {"console": {"skipped": "console MCP manager not running"}}
client.post.assert_not_called()
@pytest.mark.anyio
@@ -2413,7 +2597,176 @@ class TestNotifyNodesMcpReconnectOne:
async def test_empty_cluster(self):
req = _fake_request()
result = await _notify_nodes_mcp_reconnect_one(req, "srv")
assert result == {}
assert result == {"console": {"skipped": "console MCP manager not running"}}
class TestMcpActionConsoleArm:
"""The action fan-out's console arm (#725): presence-gated,
membership pre-checked via the public status API, self-caught, keyed
under the console pseudo-node id."""
@pytest.mark.anyio
async def test_known_name_runs_outcome(self):
client = AsyncMock()
client.post.return_value = _mock_resp(200, {"status": "ok"})
req = _fake_request(
{"node_id": "n1", "server_url": "http://n1:8000"},
proxy_client=client,
)
mgr = MagicMock()
mgr.get_all_server_status.return_value = {"srv": {}}
req.app.state.mcp_client = mgr
with patch(
"turnstone.console.server._console_mcp_action_outcome",
return_value={"status": "ok", "server": {"connected": True}},
) as outcome:
result = await _notify_nodes_mcp_refresh_one(req, "srv")
outcome.assert_called_once_with(mgr, "refresh", "srv")
assert result["console"] == {"status": "ok", "server": {"connected": True}}
assert result["n1"] == {"status": "ok"}
@pytest.mark.anyio
async def test_unknown_name_skips(self):
"""Names only nodes know (e.g. a node's config-file servers) must
skip, never error through reconnect_sync's unknown-server arm."""
req = _fake_request()
mgr = MagicMock()
mgr.get_all_server_status.return_value = {"other": {}}
req.app.state.mcp_client = mgr
result = await _notify_nodes_mcp_reconnect_one(req, "srv")
assert result["console"] == {"skipped": "not in console catalog"}
mgr.reconnect_sync.assert_not_called()
@pytest.mark.anyio
async def test_console_failure_isolated(self):
"""A console-arm crash must not abandon collected node results —
raise-injection is load-bearing: the arm's own paths return
dicts, never raise."""
client = AsyncMock()
client.post.return_value = _mock_resp(200, {"status": "ok"})
req = _fake_request(
{"node_id": "n1", "server_url": "http://n1:8000"},
proxy_client=client,
)
mgr = MagicMock()
mgr.get_all_server_status.side_effect = RuntimeError("boom")
req.app.state.mcp_client = mgr
result = await _notify_nodes_mcp_refresh_one(req, "srv")
assert result["n1"] == {"status": "ok"}
assert "boom" in result["console"]["error"]
class TestConsoleActionOutcomeParity:
"""_console_mcp_action_outcome is a pinned COPY of the node
endpoints' outcome classification (internal_mcp_refresh_one /
internal_mcp_reconnect_one). Drive BOTH sides with
identically-configured managers across the outcome matrix and assert
the payloads are EQUAL — this is the cell that fails if either side
drifts."""
_CLEAN_STATUS: dict[str, Any] = {
"connected": True,
"tools": 3,
"resources": 0,
"prompts": 1,
"error": "",
"transport": "stdio",
"command": "secret",
"url": "",
"circuit_open": False,
"consecutive_failures": 0,
}
_ERROR_STATUS: dict[str, Any] = {
**_CLEAN_STATUS,
"connected": False,
"error": "Refresh failed: connection refused",
"circuit_open": True,
}
def _node_json(self, storage: Any, mgr: Any, action: str) -> dict[str, Any]:
app = Starlette(
routes=_routes_with_internal(),
middleware=[Middleware(_InjectAuthMiddleware)],
)
app.state.auth_storage = storage
app.state.mcp_client = mgr
c = TestClient(app, raise_server_exceptions=False)
return c.post(f"/v1/api/_internal/mcp-{action}/srv").json()
def _refresh_mgr(self, *, status: dict[str, Any], outcome: Any, raises: Any) -> MagicMock:
mgr = MagicMock()
if raises is not None:
mgr.refresh_sync.side_effect = raises
else:
mgr.refresh_sync.return_value = {"srv": None}
mgr.get_server_status.return_value = dict(status)
mgr.last_refresh_outcome.return_value = outcome
return mgr
@pytest.mark.parametrize(
("status", "outcome", "raises"),
[
pytest.param(_CLEAN_STATUS, None, None, id="ok"),
pytest.param(_ERROR_STATUS, None, None, id="per-server-error"),
pytest.param(_CLEAN_STATUS, "skipped", None, id="skipped"),
pytest.param(_ERROR_STATUS, "skipped", None, id="error-beats-skip"),
pytest.param(
_CLEAN_STATUS, None, RuntimeError("stdio /etc/shadow blew up"), id="raise"
),
],
)
def test_refresh_matrix_matches(
self, storage: SQLiteBackend, status: dict[str, Any], outcome: Any, raises: Any
) -> None:
node = self._node_json(
storage, self._refresh_mgr(status=status, outcome=outcome, raises=raises), "refresh"
)
console = _console_mcp_action_outcome(
self._refresh_mgr(status=status, outcome=outcome, raises=raises), "refresh", "srv"
)
assert node == console
def _reconnect_mgr(self, *, result: Any, raises: Any) -> MagicMock:
mgr = MagicMock()
if raises is not None:
mgr.reconnect_sync.side_effect = raises
else:
mgr.reconnect_sync.return_value = result
mgr.get_server_status.return_value = dict(self._CLEAN_STATUS)
return mgr
@pytest.mark.parametrize(
("result", "raises"),
[
pytest.param(
{"connected": True, "tools": 3, "resources": 0, "prompts": 1, "error": ""},
None,
id="ok",
),
pytest.param(
{
"connected": False,
"tools": 0,
"resources": 0,
"prompts": 0,
"error": "unknown server",
},
None,
id="error-dict",
),
pytest.param(None, RuntimeError("boom"), id="raise"),
],
)
def test_reconnect_matrix_matches(
self, storage: SQLiteBackend, result: Any, raises: Any
) -> None:
node = self._node_json(
storage, self._reconnect_mgr(result=result, raises=raises), "reconnect"
)
console = _console_mcp_action_outcome(
self._reconnect_mgr(result=result, raises=raises), "reconnect", "srv"
)
assert node == console
# ---------------------------------------------------------------------------
@@ -2864,3 +3217,93 @@ class TestInternalMcpStatusEndpoint:
user_call = _aggregate_arg(_InjectAuthNoMcpMiddleware)
assert user_call.kwargs.get("aggregate") is False
# ---------------------------------------------------------------------------
# _ensure_console_mcp_client (#725) — the ONE locked construct/reconcile path
# ---------------------------------------------------------------------------
class TestEnsureConsoleMcpClient:
"""Matrix for the console's MCP ensure-helper: the ONE post-boot
lazy-construct/reconcile path, shared by the reload fan-out (all
admin-write producers) and the operator POST /reload."""
def _app(self, *, manager: Any = None, config_path: Any = None) -> Any:
import types
state = types.SimpleNamespace()
state.auth_storage = MagicMock()
cs = MagicMock()
cs.get.side_effect = lambda k, d=None: config_path if k == "mcp.config_path" else d
state.config_store = cs
if manager is not None:
state.mcp_client = manager
return types.SimpleNamespace(state=state)
def test_reconcile_arm_runs_with_existing_manager(self):
"""Keep-surface semantics: an existing manager keeps receiving
catalog updates — running coordinators track admin edits exactly
like node sessions do."""
mgr = MagicMock()
mgr.reconcile_sync.return_value = {"added": [], "removed": ["s"], "updated": []}
app = self._app(manager=mgr)
out = _ensure_console_mcp_client(app)
mgr.reconcile_sync.assert_called_once_with(app.state.auth_storage)
assert out == {"added": [], "removed": ["s"], "updated": []}
def test_construct_arm_uses_create_mcp_client(self):
"""Node parity pin: construction goes through create_mcp_client —
the node's constructor and catalog resolution (DB →
mcp.config_path → config.toml) — with the manager stored on
app.state and then reconciled."""
from unittest.mock import patch
with patch("turnstone.core.mcp_client.create_mcp_client") as create:
app = self._app(config_path="/etc/turnstone/mcp.json")
out = _ensure_console_mcp_client(app)
create.assert_called_once_with(
"/etc/turnstone/mcp.json", storage=app.state.auth_storage
)
inst = create.return_value
assert app.state.mcp_client is inst
inst.reconcile_sync.assert_called_once_with(app.state.auth_storage)
assert out is inst.reconcile_sync.return_value
def test_nothing_configured_skips(self):
"""create_mcp_client returning None (no DB rows, no file config)
is a skip, not an error — and nothing is stored on app.state."""
from unittest.mock import patch
with patch("turnstone.core.mcp_client.create_mcp_client", return_value=None):
app = self._app()
out = _ensure_console_mcp_client(app)
assert out == {"skipped": "no MCP servers configured"}
assert getattr(app.state, "mcp_client", None) is None
def test_concurrent_triggers_construct_exactly_once(self):
"""Two rapid triggers with no manager built must construct exactly
ONE manager — the module lock serializes them; the loser of an
unserialized race would leak its mcp-loop thread and connections
(the node's internal_mcp_reload has this latent race, #873; the
console must not)."""
import time
from concurrent.futures import ThreadPoolExecutor
from unittest.mock import patch
constructed: list[Any] = []
def _slow_create(config_path: Any = None, *, storage: Any = None) -> Any:
time.sleep(0.05)
mgr = MagicMock()
mgr.reconcile_sync.return_value = {"added": [], "removed": [], "updated": []}
constructed.append(mgr)
return mgr
with patch("turnstone.core.mcp_client.create_mcp_client", _slow_create):
app = self._app()
with ThreadPoolExecutor(max_workers=2) as ex:
results = list(ex.map(lambda _: _ensure_console_mcp_client(app), range(2)))
assert len(constructed) == 1, "double-construct: the ensure lock failed"
assert app.state.mcp_client is constructed[0]
assert all(r == {"added": [], "removed": [], "updated": []} for r in results)
+36
View File
@@ -965,6 +965,42 @@ class TestCreateMcpClient:
result = create_mcp_client()
assert result is None
def test_returns_none_with_storage_but_no_rows(self):
"""Empty DB + empty file config still yields no manager."""
from turnstone.core.mcp_client import create_mcp_client
storage = MagicMock()
storage.list_mcp_servers.return_value = []
with patch("turnstone.core.mcp_client.load_mcp_config", return_value={}):
assert create_mcp_client(storage=storage) is None
def test_pool_only_rows_construct_manager(self):
"""Pool-backed rows alone must construct an empty-config manager.
oauth_user/oauth_obo rows are stripped from the static config
(_db_servers_to_config), so load_mcp_config returns {} — but the
host still needs a running manager for per-user pools to form.
Returning None here left a pool-only install managerless after
every restart until the next admin MCP write or reload fan-out."""
from turnstone.core.mcp_client import create_mcp_client
storage = MagicMock()
storage.list_mcp_servers.return_value = [
{"name": "g", "auth_type": "oauth_user"},
{"name": "o", "auth_type": "oauth_obo"},
]
with patch("turnstone.core.mcp_client.MCPClientManager") as cls:
result = create_mcp_client(storage=storage)
cls.assert_called_once_with({})
inst = cls.return_value
assert result is inst
inst.start.assert_called_once_with()
# Pool-name caches still populated so per-turn auth_type lookups
# and pool priming see the rows.
assert inst._oauth_user_server_names == {"g"}
assert inst._obo_server_names == {"o"}
assert inst._db_managed == set()
# ---------------------------------------------------------------------------
# Tool refresh — _rebuild_tools, _refresh_server, listeners
+2
View File
@@ -51,6 +51,7 @@ _ESM_BUNDLES = [
_SHARED / "conversation.js",
_SHARED / "preview.js",
_SHARED / "redact_credentials.js",
_SHARED / "mcp_error.js",
]
# Sink scan: everything except renderer.js — the one sanctioned HTML-string
@@ -72,6 +73,7 @@ _ESM_NO_VAR_BUNDLES = [
_SHARED / "conversation.js",
_SHARED / "preview.js",
_SHARED / "redact_credentials.js",
_SHARED / "mcp_error.js",
]
# The same unsafe DOM-write / dynamic-code sink set that ``test_app_js.py``
+12 -2
View File
@@ -60,7 +60,8 @@ class TestToolsMetadata:
"""Validate the metadata extracted from JSON files."""
def test_tool_count(self):
# 19 interactive tools + 12 coordinator-only tools.
# 31 tool files total: 12 coordinator-only + the rest interactive,
# with memory/skills/notify/read_resource/use_prompt dual-kind.
assert len(TOOLS) == 31
def test_task_agent_tools_count(self):
@@ -69,7 +70,7 @@ class TestToolsMetadata:
def test_coordinator_tools_count(self):
from turnstone.core.tools import COORDINATOR_TOOLS
assert len(COORDINATOR_TOOLS) == 15
assert len(COORDINATOR_TOOLS) == 17
assert {t["function"]["name"] for t in COORDINATOR_TOOLS} == {
"spawn_workstream",
"spawn_batch",
@@ -99,6 +100,15 @@ class TestToolsMetadata:
# failed, phase done) without spawning a child purely to
# ship a message. Routing logic is session-kind-agnostic.
"notify",
# ``read_resource``/``use_prompt`` are dual-kind (#725):
# coordinators get the full MCP surface — tools, resources,
# prompts — persona-gated like every session. The catalog
# blocks in the system prompt and the name-keyed dispatch
# light up together with these schemas; both tools keep
# auto_approve:false, so coordinator calls prompt like any
# other MCP-backed action.
"read_resource",
"use_prompt",
}
def test_auto_approve_sets_match(self):
+259 -42
View File
@@ -283,8 +283,10 @@ def test_interactive_and_coordinator_tool_sets_overlap_only_on_dual_kind():
# ``model.skills.write`` permission, and ``load`` errors on coord
# sessions where it doesn't apply. ``notify`` joined in 1.6.0 so
# coords can post status updates at narrative beats without spawning
# a child purely to ship a message.
dual_kind = {"memory", "skills", "notify"}
# a child purely to ship a message. ``read_resource``/``use_prompt``
# joined in 1.8 (#725): coordinators get the full MCP surface —
# tools, resources, prompts — persona-gated like every session.
dual_kind = {"memory", "skills", "notify", "read_resource", "use_prompt"}
overlap = interactive_names & coord_names
assert overlap == dual_kind, (
@@ -304,14 +306,10 @@ def test_chatsession_interactive_kind_excludes_coordinator_tools(tmp_db):
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(),
ui=_null_ui(),
instructions=None,
temperature=0.5,
max_tokens=4096,
@@ -345,25 +343,7 @@ def test_chatsession_coordinator_kind_excludes_interactive_tools(tmp_db):
edit_file, ...) stay out those operate on the local node and
have no meaningful semantics from the console.
"""
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,
user_id="user-1", # the constructor refuses anonymous coordinators
kind="coordinator",
)
sess = _make_coordinator(mcp_client=None)
names = {t["function"]["name"] for t in sess._tools}
# Coordinator tools present, IC-only tools absent.
assert "spawn_workstream" in names
@@ -375,29 +355,33 @@ def test_chatsession_coordinator_kind_excludes_interactive_tools(tmp_db):
assert sess._task_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
def _null_ui():
class _NullUI:
def __getattr__(self, _name):
return lambda *a, **kw: None
return _NullUI()
def _mcp_client_mock():
from unittest.mock import MagicMock
mcp_client = MagicMock()
mcp_client.get_tools.return_value = [
{"type": "function", "function": {"name": "mcp__foo__bar", "parameters": {}}}
]
sess = ChatSession(
return mcp_client
def _make_coordinator(mcp_client, persona_snapshot=None):
from unittest.mock import MagicMock
from turnstone.core.session import ChatSession
return ChatSession(
client=MagicMock(),
model="test-model",
ui=_NullUI(),
ui=_null_ui(),
instructions=None,
temperature=0.5,
max_tokens=4096,
@@ -405,12 +389,245 @@ def test_chatsession_coordinator_kind_does_not_merge_mcp_tools(tmp_db):
user_id="user-1", # the constructor refuses anonymous coordinators
kind="coordinator",
mcp_client=mcp_client,
persona_snapshot=persona_snapshot,
)
def test_chatsession_coordinator_mcp_keyed_on_client_presence(tmp_db):
"""Coordinator MCP is keyed on CLIENT PRESENCE (#725).
The console session factory the only coordinator producer (the
CLI is blocked by the anonymous-coordinator guard; nodes reject
non-interactive kinds at create and at session-load) passes the
live console manager unconditionally; the persona MCP toggle
governs the surface, like interactive. Session-level contract: a
coordinator constructed WITH an mcp_client treats MCP as enabled and
runs the same listener/prime skeleton as interactive over the
COORDINATOR_TOOLS base; one constructed WITHOUT keeps the fixed
builtin surface.
"""
from unittest.mock import patch
# Cell (a): no client → today's fixed invariant, verbatim.
sess = _make_coordinator(mcp_client=None)
names = {t["function"]["name"] for t in sess._tools}
assert "mcp__foo__bar" not in names
assert "spawn_workstream" in names
assert sess._task_tools == []
# Cell (b): client present → merged surface + ALL THREE listeners +
# the creator's pools primed (full parity with interactive).
mcp_client = _mcp_client_mock()
with patch("turnstone.core.session.try_prime_user_pools") as prime:
sess = _make_coordinator(mcp_client=mcp_client)
names = {t["function"]["name"] for t in sess._tools}
assert "mcp__foo__bar" in names
assert "spawn_workstream" in names # base is additive, never replaced
assert sess._task_tools == [] # coordinators have no task-agent lane
mcp_client.add_listener.assert_called_once()
assert mcp_client.add_listener.call_args.kwargs.get("user_id") == "user-1"
mcp_client.add_resource_listener.assert_called_once()
mcp_client.add_prompt_listener.assert_called_once()
prime.assert_called_once()
assert prime.call_args.args[1] == "user-1"
def test_chatsession_coordinator_persona_mcp_off_wins(tmp_db):
"""A coordinator persona with mcp=False gates the surface off even
when the factory passed a live client same precedence as
interactive (the 1762 persona gate composes upstream of the kind
branch), and ``_mcp_gated_off`` records that a real client was
withheld so resume() refuses to adopt an MCP-on stamp."""
from turnstone.core.personas import PersonaSnapshot
mcp_client = _mcp_client_mock()
snap = PersonaSnapshot(name="p", prompt="x", tools=None, mcp=False, memory=True)
sess = _make_coordinator(mcp_client=mcp_client, persona_snapshot=snap)
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()
assert sess._mcp_gated_off is True
def test_chatsession_coordinator_drop_mcp_surface_resets_to_coordinator_tools(tmp_db):
"""The resume()-adopts-MCP-off-stamp path (_drop_mcp_surface) on a
COORDINATOR resets to COORDINATOR_TOOLS never the interactive lanes
and removes all three listeners under the tracked registration
identity. Direct cell for the docstring's both-kinds claim; every
sibling coordinator MCP transition has one."""
from unittest.mock import patch
mcp_client = _mcp_client_mock()
with patch("turnstone.core.session.try_prime_user_pools"):
sess = _make_coordinator(mcp_client=mcp_client)
assert "mcp__foo__bar" in {t["function"]["name"] for t in sess._tools}
sess._drop_mcp_surface()
names = {t["function"]["name"] for t in sess._tools}
assert "mcp__foo__bar" not in names
assert "spawn_workstream" in names
assert sess._task_tools == []
assert sess._mcp_client is None
mcp_client.remove_listener.assert_called_once()
assert mcp_client.remove_listener.call_args.kwargs.get("user_id") == "user-1"
mcp_client.remove_resource_listener.assert_called_once()
mcp_client.remove_prompt_listener.assert_called_once()
def test_chatsession_coordinator_drop_mcp_surface_after_rebind_uses_tracked_id(tmp_db):
"""After bind_acting_user re-scopes the listener registrations to a
NEW operator, a subsequent surface drop must remove them under the
tracked _mcp_listener_user_id (the rebound identity) removal keyed
on the owner id would leave the rebound registrations leaking."""
from unittest.mock import patch
mcp_client = _mcp_client_mock()
with patch("turnstone.core.session.try_prime_user_pools"):
sess = _make_coordinator(mcp_client=mcp_client)
sess.bind_acting_user("user-2")
assert sess._mcp_listener_user_id == "user-2"
sess._drop_mcp_surface()
assert mcp_client.remove_listener.call_args.kwargs.get("user_id") == "user-2"
assert mcp_client.remove_resource_listener.call_args.kwargs.get("user_id") == "user-2"
assert mcp_client.remove_prompt_listener.call_args.kwargs.get("user_id") == "user-2"
def test_coordinator_catalog_change_rebuilds_merged_tools(tmp_db):
"""A registered coordinator's tool listener rebuilds the merged set on
catalog change the surface tracks admin edits and reconnects instead
of freezing at construction (the reversed early-return)."""
mcp_client = _mcp_client_mock()
sess = _make_coordinator(mcp_client=mcp_client)
cb = mcp_client.add_listener.call_args.args[0]
mcp_client.get_tools.return_value = [
{"type": "function", "function": {"name": "mcp__foo__baz", "parameters": {}}}
]
cb()
names = {t["function"]["name"] for t in sess._tools}
assert "mcp__foo__baz" in names
assert "mcp__foo__bar" not in names # fresh merge, not accretion
assert "spawn_workstream" in names # base survives every rebuild
assert sess._task_tools == []
def test_coordinator_construction_race_converges_post_snapshot_change(tmp_db):
"""A catalog change landing AFTER the authoritative read must be
converged by the end-of-construction recheck. The seq bump rides
get_tools' FIRST call so it lands strictly after the constructor's
snapshot a bump at add_listener time would fold into the snapshot
and pass whether or not the recheck runs for coordinators."""
from unittest.mock import MagicMock
mcp_client = MagicMock()
v1 = [{"type": "function", "function": {"name": "mcp__foo__v1", "parameters": {}}}]
v2 = [{"type": "function", "function": {"name": "mcp__foo__v2", "parameters": {}}}]
calls = {"n": 0}
def _get_tools(user_id=None):
calls["n"] += 1
if calls["n"] == 1:
# Simulate a notification landing between the authoritative
# read and the end of tool setup: advance the bound session's
# change counter (the listener callback's __self__).
cb = mcp_client.add_listener.call_args.args[0]
cb.__self__._mcp_tools_change_seq += 1
return v1
return v2
mcp_client.get_tools.side_effect = _get_tools
sess = _make_coordinator(mcp_client=mcp_client)
names = {t["function"]["name"] for t in sess._tools}
assert "mcp__foo__v2" in names, "post-snapshot catalog change was lost for a coordinator"
def test_coordinator_bind_acting_user_rescopes_listeners_and_primes(tmp_db):
"""Coordinators are multi-sender: a fresh turn from operator B must
re-scope the MCP listeners to B and prime B's pools, so B dispatches
against B's catalog — never A's (#725, security-critical cell)."""
from unittest.mock import patch
mcp_client = _mcp_client_mock()
with patch("turnstone.core.session.try_prime_user_pools") as prime:
sess = _make_coordinator(mcp_client=mcp_client)
sess.bind_acting_user("user-2")
assert mcp_client.remove_listener.call_args.kwargs.get("user_id") == "user-1"
assert mcp_client.add_listener.call_args.kwargs.get("user_id") == "user-2"
assert mcp_client.remove_resource_listener.call_args.kwargs.get("user_id") == "user-1"
assert mcp_client.add_resource_listener.call_args.kwargs.get("user_id") == "user-2"
assert mcp_client.remove_prompt_listener.call_args.kwargs.get("user_id") == "user-1"
assert mcp_client.add_prompt_listener.call_args.kwargs.get("user_id") == "user-2"
assert prime.call_args.args[1] == "user-2"
assert sess._mcp_effective_user_id == "user-2"
def test_coordinator_close_removes_listeners_owner_only(tmp_db):
"""Exit-path cell (i): closing an owner-only coordinator removes all
three listeners under the owner identity."""
mcp_client = _mcp_client_mock()
sess = _make_coordinator(mcp_client=mcp_client)
sess.close()
assert mcp_client.remove_listener.call_args.kwargs.get("user_id") == "user-1"
assert mcp_client.remove_resource_listener.call_args.kwargs.get("user_id") == "user-1"
assert mcp_client.remove_prompt_listener.call_args.kwargs.get("user_id") == "user-1"
def test_coordinator_close_after_rebind_removes_under_new_identity(tmp_db):
"""Exit-path cell (ii), the wrong-field regression pin: after operator
B rebinds, eviction/close must remove listeners under B's uid (the
tracked ``_mcp_listener_user_id``), not the owner's ``_mcp_user_id`` —
owner-only closes cannot distinguish the two fields (both equal the
owner at construction)."""
from unittest.mock import patch
mcp_client = _mcp_client_mock()
with patch("turnstone.core.session.try_prime_user_pools"):
sess = _make_coordinator(mcp_client=mcp_client)
sess.bind_acting_user("user-2")
sess.close()
assert mcp_client.remove_listener.call_args.kwargs.get("user_id") == "user-2"
assert mcp_client.remove_resource_listener.call_args.kwargs.get("user_id") == "user-2"
assert mcp_client.remove_prompt_listener.call_args.kwargs.get("user_id") == "user-2"
def test_coordinator_read_resource_use_prompt_on_wire_and_approval(tmp_db):
"""Dual-kind read_resource/use_prompt (#725) — the WIRE matrix via
_get_active_tools(): present with a client exposing nonzero
resource/prompt counts; STRIPPED without a client; STRIPPED with a
client whose counts are zero (the _without_tool gate keys on client
presence AND per-user catalog count). Base-list membership is
asserted separately both tools sit in COORDINATOR_TOOLS
unconditionally. Approval: needs_approval preserved
(auto_approve:false)."""
mcp_client = _mcp_client_mock()
# Explicit integers: a bare MagicMock return is truthy, which would
# let the present-cell pass without the count gate ever working.
mcp_client.resource_count_for_user.return_value = 2
mcp_client.prompt_count_for_user.return_value = 1
sess = _make_coordinator(mcp_client=mcp_client)
assert {"read_resource", "use_prompt"} <= {t["function"]["name"] for t in sess._tools}
wire = {t["function"]["name"] for t in sess._get_active_tools()}
assert {"read_resource", "use_prompt"} <= wire
item = sess._prepare_read_resource("c1", {"uri": "res://x"})
assert item.get("needs_approval") is True
bare = _make_coordinator(mcp_client=None)
assert {"read_resource", "use_prompt"} <= {t["function"]["name"] for t in bare._tools}
bare_wire = {t["function"]["name"] for t in bare._get_active_tools()}
assert not ({"read_resource", "use_prompt"} & bare_wire), (
"client-less coordinator must not advertise MCP catalog tools on the wire"
)
zero = _mcp_client_mock()
zero.resource_count_for_user.return_value = 0
zero.prompt_count_for_user.return_value = 0
sess_zero = _make_coordinator(mcp_client=zero)
zero_wire = {t["function"]["name"] for t in sess_zero._get_active_tools()}
assert not ({"read_resource", "use_prompt"} & zero_wire), (
"zero-count catalogs must strip read_resource/use_prompt from the wire"
)
+4 -2
View File
@@ -294,8 +294,10 @@ class CoordinatorAdapter:
prefix (``/high``, ``/urgent``, etc.) by :meth:`ChatSession.queue_message`.
``acting_user_id`` is the authenticated sender. On a fresh turn it is
bound as the coordinator's acting user (so per-participant MCP creds and
the state_change acting-user signal work once coordinator MCP lands);
bound as the coordinator's acting user — with coordinator MCP enabled
(#725) this re-scopes the session's MCP listeners and primes the
sender's per-user pools, so each participant dispatches against their
OWN credentials, and it drives the state_change acting-user signal;
on a mid-turn interjection it is passed to ``queue_message``, which
rejects a DIFFERENT participant (:class:`CrossUserInterjectionError`)
the same cross-user protection the interactive surface has. Empty on
+265 -8
View File
@@ -5121,6 +5121,9 @@ def _bootstrap_coord_subsystem(
config_store=config_store,
node_id="console",
coord_client_factory=_coord_client_factory,
# Getter, not the instance: the console MCP ensure-helper can
# (re)construct the manager after this bootstrap (#725).
mcp_client_getter=lambda: getattr(app.state, "mcp_client", None),
)
coord_adapter = CoordinatorAdapter(
collector=app.state.collector,
@@ -5579,6 +5582,28 @@ async def _lifespan(app: Starlette) -> AsyncGenerator[None, None]:
# ``asyncio.to_thread`` invocation.
await asyncio.to_thread(_load_and_bootstrap_coord_subsystem, app, storage, config_store)
# Console-hosted MCP client manager for coordinator workstreams
# (#725) — node-boot parity: same constructor, same catalog
# resolution (DB → mcp.config_path → config.toml), same bounded
# inline connect wait, None when nothing is configured. Later
# convergence rides the reload fan-out (_ensure_console_mcp_client).
app.state.mcp_client = None
if storage and config_store:
from turnstone.core.mcp_client import create_mcp_client
try:
app.state.mcp_client = await asyncio.to_thread(
create_mcp_client,
config_store.get("mcp.config_path") or None,
storage=storage,
)
except Exception:
log.warning(
"console MCP manager boot failed — coordinators get no MCP "
"until the next admin MCP write or reload",
exc_info=True,
)
yield
# Shutdown
if _console_heartbeat_task is not None:
@@ -5633,6 +5658,20 @@ async def _lifespan(app: Starlette) -> AsyncGenerator[None, None]:
log.debug("console.coord_ui_refs_reset_failed", exc_info=True)
await app.state.proxy_sse_client.aclose()
await app.state.proxy_client.aclose()
# Console-hosted MCP manager: coordinator sessions stay LOADED at
# console shutdown (there is no session-close loop here, unlike the
# node), so their MCP listeners are intentionally still registered
# when the manager dies — acceptable because the process is exiting
# and the manager's listener fan-out swallows per-callback
# exceptions. shutdown() blocks (mcp-loop join + transport
# teardown), so offload like coord_state_writer above. Ordered
# before close_mcp_oauth_state per LIFO teardown.
_console_mcp_shutdown = getattr(app.state, "mcp_client", None)
if _console_mcp_shutdown is not None:
try:
await asyncio.to_thread(_console_mcp_shutdown.shutdown)
except Exception:
log.debug("console.mcp_client_shutdown_failed", exc_info=True)
# Close in reverse order of initialization (mcp_oauth → mcp_crypto →
# oidc) per LIFO teardown discipline.
from turnstone.core.mcp_oauth import close_mcp_oauth_state
@@ -10233,7 +10272,15 @@ def _mcp_server_to_detail(
async def _collect_mcp_status(
request: Request,
) -> dict[str, dict[str, dict[str, Any]]]:
"""Query all nodes for MCP status. Returns {node_id: {server_name: status}}."""
"""Query all nodes — and the console's own manager — for MCP status.
Returns {node_id: {server_name: status}}. The console's manager
reports under the collector's console pseudo-node id (#725): the
admin MCP view must show the surface coordinators actually dispatch
through. Its rows carry the same read-scope projection node rows
get (``has_error``, no command/url/verbose error); on failure the
console key is omitted, matching ``_fetch``'s None contract.
"""
collector: ClusterCollector = request.app.state.collector
nodes = collector.get_all_nodes()
client: httpx.AsyncClient = request.app.state.proxy_client
@@ -10258,7 +10305,31 @@ async def _collect_mcp_status(
log.debug("Failed to fetch MCP status from node %s", node_id, exc_info=True)
return node_id, None
results = await asyncio.gather(*[_fetch(n) for n in nodes])
async def _console() -> tuple[str, dict[str, dict[str, Any]] | None]:
# Class constant, not the collector instance: several callers
# exercise this collector with minimal stubs, and the pseudo-node
# key must not depend on runtime state they never provided.
console_id = ClusterCollector.CONSOLE_PSEUDO_NODE_ID
mgr = getattr(request.app.state, "mcp_client", None)
if mgr is None:
return console_id, None
from turnstone.core.mcp_utils import strip_server_status_for_read
# Mirrors internal_mcp_status's admin path: cross-user aggregate
# view scoped to the requesting admin.
uid = _auth_user_id(request)
def _status() -> dict[str, dict[str, Any]]:
all_status = mgr.get_all_server_status(uid, aggregate=True)
return {n: strip_server_status_for_read(s) for n, s in all_status.items()}
try:
return console_id, await asyncio.to_thread(_status)
except Exception:
log.debug("console MCP status read failed", exc_info=True)
return console_id, None
results = await asyncio.gather(*[_fetch(n) for n in nodes], _console())
return {nid: servers for nid, servers in results if servers is not None}
@@ -10951,6 +11022,60 @@ async def admin_delete_mcp_server(request: Request) -> JSONResponse:
return JSONResponse({"status": "ok"}, background=_schedule_mcp_reload(request))
# Guards concurrent construction of the console's own MCP client manager
# (lifespan boot racing an admin-write fan-out, or two rapid fan-outs
# while ``app.state.mcp_client`` is still None). Two winners would each
# start an mcp-loop daemon thread and a static connection set; the second
# assignment clobbers the first ref and the loser leaks both forever.
# The node's ``internal_mcp_reload`` lazy-construct carries exactly this
# latent race (turnstone/server.py internal_mcp_reload) — the console
# does not replicate it. Same shape as ``_COORD_BOOTSTRAP_LOCK``.
_CONSOLE_MCP_ENSURE_LOCK = threading.Lock()
def _ensure_console_mcp_client(app: Any) -> dict[str, Any]:
"""Reconcile — and lazily construct — the console's own MCP client manager.
The console hosts an ``MCPClientManager`` so coordinator-kind
workstreams get an MCP tool surface (#725). Node parity throughout:
construction goes through ``create_mcp_client`` the same catalog
resolution as a node host (DB rows, then ``mcp.config_path``, then
this host's config.toml; ``None`` when nothing is configured) — and
the reconcile arm runs whenever a manager exists, so running
coordinators track admin edits exactly like node sessions do.
This is the ONE lazy-construct/reconcile path for every post-boot
trigger the admin-write reload fan-out and the operator
``POST /reload`` mirroring the node's ``internal_mcp_reload``.
Unlike the node's arm, the body holds a lock: two concurrent
triggers on a managerless console must not double-construct (the
node's unlocked equivalent is issue #873).
Plain SYNC function: construction connects to static servers and
``reconcile_sync`` performs bounded sync waits, so every caller
(all async) MUST invoke via ``asyncio.to_thread``.
Returns a reconcile-shaped dict (``added``/``removed``/``updated``,
or ``skipped``) so the operator reload view reports the console
alongside the nodes.
"""
with _CONSOLE_MCP_ENSURE_LOCK:
storage = getattr(app.state, "auth_storage", None)
if storage is None:
return {"skipped": "storage not initialized"}
mgr = getattr(app.state, "mcp_client", None)
if mgr is None:
from turnstone.core.mcp_client import create_mcp_client
cs = getattr(app.state, "config_store", None)
cfg_path = cs.get("mcp.config_path") if cs is not None else None
mgr = create_mcp_client(cfg_path or None, storage=storage)
if mgr is None:
return {"skipped": "no MCP servers configured"}
app.state.mcp_client = mgr
return mgr.reconcile_sync(storage)
async def _notify_nodes_mcp_reload(request: Request) -> dict[str, Any]:
"""Tell all nodes to re-read the mcp_servers DB table and reconcile.
@@ -10986,7 +11111,33 @@ async def _notify_nodes_mcp_reload(request: Request) -> dict[str, Any]:
log.debug("Failed to notify node %s for MCP reload", node_id, exc_info=True)
return node_id, {"error": str(exc)}
results = await asyncio.gather(*[_notify(n) for n in nodes])
async def _console() -> tuple[str, Any]:
# The console hosts its own MCP manager for coordinator sessions
# (#725) — ensure/reconcile it in the SAME gather as the node
# fan-out so EVERY producer (post-write background hooks and the
# operator POST /reload alike) covers it, and so the console's
# (up to ~30s) construct/connect work OVERLAPS the node HTTP
# window instead of stacking after it on the operator-inline
# reload path. Keyed under the collector's console pseudo-node
# id: get_nodes hides that id from fan-out enumeration, so the
# key can neither collide with a real node nor self-HTTP.
# Deliberately OUTSIDE ``sem`` — that semaphore paces node POSTs,
# and a ~30s worker-thread hold would starve a slot. Exceptions
# are caught HERE (the gather runs return_exceptions=False): a
# console failure must not abandon collected node results. Class
# constant for the key, like the sibling arms: stub collectors
# may not carry the instance attr.
console_id = ClusterCollector.CONSOLE_PSEUDO_NODE_ID
try:
return (
console_id,
await asyncio.to_thread(_ensure_console_mcp_client, request.app),
)
except Exception as exc:
log.warning("console MCP self-reconcile failed", exc_info=True)
return console_id, {"error": str(exc)}
results = await asyncio.gather(*[_notify(n) for n in nodes], _console())
return {nid: data for nid, data in results if data is not None}
@@ -11018,23 +11169,97 @@ def _schedule_mcp_reload(request: Request) -> BackgroundTask:
exc_info=True,
)
return
# The console's ensure entry rides the same results dict but is
# NOT a node: its failures are already logged with console
# wording at their production sites, and counting it here would
# both warn with node-remediation prose for a console-local
# failure and inflate the denominator (a node-less install with
# a console error would log "1 of 1 node(s)"). Class constant,
# not request.app.state.collector: the accounting must not
# depend on runtime state the (possibly patched) fan-out never
# touched.
console_id = ClusterCollector.CONSOLE_PSEUDO_NODE_ID
node_results = {nid: data for nid, data in results.items() if nid != console_id}
unreached = sorted(
nid for nid, data in results.items() if isinstance(data, dict) and "error" in data
nid for nid, data in node_results.items() if isinstance(data, dict) and "error" in data
)
if unreached:
log.warning(
"auto mcp-reload did not reach %d of %d node(s) after admin write "
"(stale MCP catalog until next reload): %s",
len(unreached),
len(results),
len(node_results),
", ".join(unreached),
)
return BackgroundTask(_run)
def _console_mcp_action_outcome(mgr: Any, action: str, name: str) -> dict[str, Any]:
"""Run one per-server MCP action against the console's own manager.
COPY of the node endpoints' outcome classification
(``internal_mcp_refresh_one`` / ``internal_mcp_reconnect_one`` in
turnstone/server.py) minus the HTTP mapping deliberately a copy,
not an extraction, so the node endpoint bodies stay untouched; the
equivalence is pinned by TestConsoleActionOutcomeParity in
tests/test_mcp_admin_api.py, which drives both sides with identical
managers and fails if either drifts. See the node endpoints for the
full rationale comments (authoritative-outcome re-check;
error-beats-skip).
Sync + blocking (``refresh_sync``/``reconnect_sync`` park on manager
locks up to their caller timeouts) call via ``asyncio.to_thread``.
"""
from turnstone.core.mcp_utils import public_server_status
if action == "refresh":
try:
mgr.refresh_sync(server_name=name)
except Exception as exc:
log.warning("console MCP refresh failed for %s: %s", name, exc)
return {"status": "error", "error": "refresh failed"}
status = public_server_status(mgr, name)
if status.get("error"):
log.warning("console MCP refresh reported error for %s: %s", name, status["error"])
return {"status": "error", "error": "refresh failed", "server": status}
if mgr.last_refresh_outcome(name) == "skipped":
return {"status": "skipped", "server": status}
return {"status": "ok", "server": status}
try:
result = mgr.reconnect_sync(name)
except Exception as exc:
log.warning("console MCP reconnect failed for %s: %s", name, exc)
return {"status": "error", "error": "reconnect failed"}
if result.get("error"):
log.warning(
"console MCP reconnect reported error for %s: %s",
name,
result.get("error", ""),
)
return {
"status": "error",
"error": "reconnect failed",
"server": public_server_status(mgr, name),
}
return {"status": "ok", "server": public_server_status(mgr, name)}
async def _notify_nodes_mcp_action(request: Request, action: str, name: str) -> dict[str, Any]:
"""Tell all nodes to perform a per-server MCP action.
"""Tell all nodes — and the console's own manager — to perform a per-server MCP action.
Console arm (#725): the console's manager serves coordinator
dispatch, so the operator's per-server recovery actions must reach
it like any node's — otherwise reconnecting a wedged server heals
every node while the surface coordinators actually dispatch through
stays wedged until a console restart. The arm runs the pinned copy
of the node outcome classification (see
``_console_mcp_action_outcome``), keyed under the collector's
console pseudo-node id, gated on manager PRESENCE (nothing here
reads settings a live manager must stay operable). Names the
console's catalog never carried (e.g. servers from a node's own
config file) report skipped, not error.
*action* is the suffix of the internal endpoint currently
``"refresh"`` or ``"reconnect"``. Each node is hit at
@@ -11073,7 +11298,32 @@ async def _notify_nodes_mcp_action(request: Request, action: str, name: str) ->
)
return node_id, {"error": str(exc)}
results = await asyncio.gather(*[_notify(n) for n in nodes])
def _console_action() -> dict[str, Any]:
# Sync body for the worker thread.
mgr = getattr(request.app.state, "mcp_client", None)
if mgr is None:
return {"skipped": "console MCP manager not running"}
# Membership via the public status API (covers static AND
# pool-backed names): a name only nodes know must skip, not
# error through reconnect_sync's "unknown server" arm.
if name not in mgr.get_all_server_status(None, aggregate=True):
return {"skipped": "not in console catalog"}
return _console_mcp_action_outcome(mgr, action, name)
async def _console() -> tuple[str, Any]:
# Gather sibling OUTSIDE ``sem`` (that semaphore paces node
# POSTs; a long worker-thread hold would starve a slot),
# self-caught so a console failure never abandons collected node
# results — mirroring the reload fan-out's console arm. Class
# constant for the key: stub collectors may not carry the attr.
console_id = ClusterCollector.CONSOLE_PSEUDO_NODE_ID
try:
return console_id, await asyncio.to_thread(_console_action)
except Exception as exc:
log.warning("console MCP %s of %s failed", action, name, exc_info=True)
return console_id, {"error": str(exc)}
results = await asyncio.gather(*[_notify(n) for n in nodes], _console())
return {nid: data for nid, data in results if data is not None}
@@ -11354,7 +11604,14 @@ async def admin_import_mcp_config(request: Request) -> JSONResponse:
ip,
)
return JSONResponse({"imported": imported, "skipped": skipped, "errors": errors})
# Conditional like the audit record above: import is the one write
# whose 200 can mean zero row changes (every name already present) —
# fan out only when rows actually changed, matching the CRUD
# siblings' newly-written-row semantics.
return JSONResponse(
{"imported": imported, "skipped": skipped, "errors": errors},
background=_schedule_mcp_reload(request) if imported else None,
)
# ---------------------------------------------------------------------------
+15 -1
View File
@@ -33,6 +33,7 @@ if TYPE_CHECKING:
from turnstone.console.coordinator_client import CoordinatorClient
from turnstone.core.config_store import ConfigStore
from turnstone.core.mcp_client import MCPClientManager
from turnstone.core.model_registry import ModelRegistry
from turnstone.core.personas import PersonaSnapshot
from turnstone.core.session import SessionUI
@@ -46,12 +47,18 @@ def build_console_session_factory(
config_store: ConfigStore,
node_id: str,
coord_client_factory: Callable[[str, str], CoordinatorClient],
mcp_client_getter: Callable[[], MCPClientManager | None] | None = None,
) -> Callable[..., ChatSession]:
"""Return a session factory that builds coordinator-kind ChatSessions.
The factory signature matches :class:`turnstone.core.workstream._SessionFactory`.
``coord_client_factory`` is called at session-create time with
``(ws_id, user_id)`` and returns a prepared :class:`CoordinatorClient`.
``mcp_client_getter`` returns the console's CURRENT MCP client manager
(or ``None``) a getter rather than an instance because the console's
ensure-helper can (re)construct the manager after this factory is
built; it is consulted per session construction (#725), the console
counterpart of the node factory's ``mcp_ref[0]`` read.
Only ``kind="coordinator"`` is supported here the console doesn't
host interactive workstreams. The factory rejects any other kind
@@ -138,6 +145,13 @@ def build_console_session_factory(
live_memory_config = _build_memory_config()
live_judge_config = _build_judge_config()
# Coordinator MCP surface (#725): resolved per construction so the
# session sees the CURRENT manager — the console ensure-helper can
# (re)construct it after this factory was built. Passed
# unconditionally, exactly like the node session factory reads
# mcp_ref[0]: whether MCP tools actually surface is the persona's
# call, same as interactive (#725).
live_mcp_client = mcp_client_getter() if mcp_client_getter is not None else None
# NOTE: do not pre-resolve ``live_judge_config.model`` against the
# registry here. ``IntentJudge.__init__`` does a richer resolution
# that also picks up the alias's *provider + client*; rewriting
@@ -198,7 +212,7 @@ def build_console_session_factory(
auto_compact_pct=config_store.get("session.auto_compact_pct"),
agent_max_turns=config_store.get("tools.agent_max_turns"),
tool_truncation=config_store.get("tools.truncation"),
mcp_client=None, # console doesn't host MCP today
mcp_client=live_mcp_client,
registry=registry,
model_alias=effective_alias,
health_registry=None,
+46 -1
View File
@@ -3747,6 +3747,7 @@ const _settingsSectionOrder = [
"tools",
"server",
"cluster",
"coordinator",
"channels",
"mcp",
"ratelimit",
@@ -3763,6 +3764,7 @@ function _settingsSectionLabel(section) {
tools: "Tools",
server: "Server",
cluster: "Cluster",
coordinator: "Coordinator",
channels: "Channels",
audio: "Voice",
mcp: "MCP",
@@ -4670,6 +4672,15 @@ function loadAdminMcp() {
.then(function (data) {
_mcpServers = data.servers || [];
_renderMcpServers(_mcpServers);
// Re-sync the rail's pending-consent badge AFTER the table renders
// — the operator has now seen current state (#874). A failed load
// (the catch below) keeps the pending signal instead.
if (
window.TS_APP &&
typeof window.TS_APP.syncConsentBadge === "function"
) {
window.TS_APP.syncConsentBadge();
}
})
.catch(function () {
setSafeHtml(
@@ -5472,9 +5483,43 @@ function reloadMcpNodes() {
totalAdded += (nr.added || []).length;
totalRemoved += (nr.removed || []).length;
}
let msg = "Reload sent to " + nodeIds.length + " node(s)";
// The results map includes the console's own MCP reconcile under
// the "console" pseudo-node key (coordinator MCP, #725) — count it
// separately so the operator-facing tally stays honest, and only
// claim "+ console" when the console actually RECONCILED: a
// skipped entry (nothing configured) says nothing, and an error
// entry gets an explicit failure note instead of riding the
// success phrasing. failed beats reconciled if a malformed entry
// ever carries both shapes. (The added/removed tally above can
// only include console counts when the entry is reconcile-shaped —
// exactly when "+ console" is claimed — so attribution stays
// honest.)
const consoleEntry = Object.prototype.hasOwnProperty.call(
results,
"console",
)
? results.console
: null;
const consoleFailed =
consoleEntry !== null && consoleEntry.error !== undefined;
const consoleReconciled =
!consoleFailed &&
consoleEntry !== null &&
(consoleEntry.added !== undefined ||
consoleEntry.removed !== undefined ||
consoleEntry.updated !== undefined);
const nodeCount =
consoleEntry !== null ? nodeIds.length - 1 : nodeIds.length;
let msg =
nodeCount === 0 && consoleReconciled
? "Reload sent to console"
: "Reload sent to " +
nodeCount +
" node(s)" +
(consoleReconciled ? " + console" : "");
if (totalAdded) msg += ", +" + totalAdded + " added";
if (totalRemoved) msg += ", -" + totalRemoved + " removed";
if (consoleFailed) msg += "; console reload failed";
showToast(msg);
_clearMcpSyncPending();
setTimeout(loadAdminMcp, 1500);
+142
View File
@@ -2356,6 +2356,139 @@ window.TS_APP.resolveInteractiveNode = function (wsId, hintNodeId) {
return { error: "Failed to open this session." };
});
};
// === MCP consent badge (console L-shell) ====================================
// Mirror of the node dashboard's pending-consent subsystem (ui/static/app.js
// §12), driving the rail's Admin > MCP Servers row badge through the shell
// bridge. The pending set is USER-scoped server truth — the Phase 9
// mcp_oauth_pending table, which the console serves at
// /v1/api/mcp/oauth/pending — hydrated at boot and fed live by hosted panes
// through the window.TS_APP.onConsentDetected seam (the shared pane host
// bridges an interactive pane's detections here, and the coordinator pane
// forwards its error card's onConsent; both light up now that the seam
// exists on the console). admin.js re-syncs after the operator has SEEN
// the MCP panel render (same semantics as the node's Connections flow).
const _pendingConsentServers = new Set();
function _refreshConsentBadge() {
const shell = window.TS_SHELL;
if (!shell || typeof shell.setRowBadge !== "function") return;
const n = _pendingConsentServers.size;
const label =
n === 0
? ""
: n + " MCP server" + (n === 1 ? "" : "s") + " awaiting consent";
shell.setRowBadge("mcp", n, label);
}
function _onConsentDetected(server) {
if (typeof server === "string" && server) {
_pendingConsentServers.add(server);
_refreshConsentBadge();
}
}
function loadPendingConsents() {
authFetch("/v1/api/mcp/oauth/pending")
.then(function (r) {
if (!r.ok) return null;
return r.json();
})
.then(function (data) {
if (!data || !Array.isArray(data.servers)) return;
for (let i = 0; i < data.servers.length; i++) {
const row = data.servers[i];
if (row && typeof row.server_name === "string") {
_pendingConsentServers.add(row.server_name);
}
}
_refreshConsentBadge();
})
.catch(function () {
// Endpoint failures must not block console boot.
});
}
// Live MCP-consent notifications from a hosted pane — the seam the shared
// pane host bridges to (and the coordinator pane's card forwards to).
window.TS_APP.onConsentDetected = _onConsentDetected;
// Badge re-sync to DB truth after the operator has seen the admin MCP
// panel render: clear-then-rehydrate, the node Connections-flow semantics
// (a FAILED panel load keeps the pending signal instead). The repaint
// between clear and rehydrate keeps the badge↔set invariant even when the
// rehydrate fetch fails — matching the node's _clearConsentBadge.
window.TS_APP.syncConsentBadge = function () {
_pendingConsentServers.clear();
_refreshConsentBadge();
loadPendingConsents();
};
// Visibility-show resync (#874): the consent popup opens noopener — no
// cross-window channel exists — so the only reliable signal that consent
// completed is the user returning to this tab. Merge-on-success:
// entries the server no longer lists are dropped ONLY if they predate
// this fetch (the preFetch snapshot) — a detection arriving through
// _onConsentDetected while the fetch is in flight survives, since its
// DB row may postdate the server's read. A failed fetch changes
// nothing, so a possibly-valid warning is never blanked (unlike
// syncConsentBadge above, whose blind clear is justified by the
// operator having just SEEN the MCP panel).
let _resyncInFlight = false;
let _resyncQueued = false;
function _resyncPendingConsents() {
// Single-flight: overlapping fetches can resolve out of order, and
// every guard short of exclusion re-admits some interleaving (a stale
// read clobbering a newer one, a failed fetch suppressing a valid
// one, an older read's additions surviving a newer read's deletes).
// One flight at a time makes the whole class structurally impossible;
// an edge firing mid-flight queues exactly one rerun so the freshest
// truth still lands.
if (_resyncInFlight) {
_resyncQueued = true;
return;
}
// Bounded flight: a stalled fetch would otherwise hold the gate shut
// forever (the .finally below never runs, freezing self-heal); on
// timeout the chain rejects → .catch → .finally clears the gate and
// drains any queued rerun. Feature-detected like the codebase's
// AbortController guards — old runtimes just run unbounded, as before
// — and computed BEFORE the gate is set so no throw can wedge it.
const signal =
typeof AbortSignal !== "undefined" && AbortSignal.timeout
? AbortSignal.timeout(10000)
: undefined;
_resyncInFlight = true;
const preFetch = new Set(_pendingConsentServers);
authFetch("/v1/api/mcp/oauth/pending", { signal: signal })
.then(function (r) {
return r.ok ? r.json() : null;
})
.then(function (data) {
if (!data || !Array.isArray(data.servers)) return;
const fetched = new Set();
for (let i = 0; i < data.servers.length; i++) {
const row = data.servers[i];
if (row && typeof row.server_name === "string") {
fetched.add(row.server_name);
}
}
preFetch.forEach(function (s) {
if (!fetched.has(s)) _pendingConsentServers.delete(s);
});
fetched.forEach(function (s) {
_pendingConsentServers.add(s);
});
_refreshConsentBadge();
})
.catch(function () {})
.finally(function () {
_resyncInFlight = false;
if (_resyncQueued) {
_resyncQueued = false;
_resyncPendingConsents();
}
});
}
window.TS_APP.boot = function () {
history.replaceState({ view: "home" }, "");
_initSavedCoordTable(); // substrate modules have evaluated by boot time
@@ -2365,6 +2498,15 @@ window.TS_APP.boot = function () {
// pipeline (#9); the console pseudo-node carries coordinator
// ws_created / ws_closed / cluster_state events.
loadOverview();
// Hydrate the MCP pending-consent badge (#874): a coordinator or
// scheduled run that hit mcp_consent_required while nobody watched left
// a DB row — the badge is how the operator finds out. Cheap no-op on
// installs with no user-scoped MCP servers; failures are silent.
loadPendingConsents();
// Self-heal the badge after the consent popup (see _resyncPendingConsents).
document.addEventListener("visibilitychange", function () {
if (document.visibilityState === "visible") _resyncPendingConsents();
});
_ensureHomeComposerInit();
// Refresh the coord button visibility once auth.js has populated
// sessionStorage from the initial whoami. window.permissionsReady
@@ -556,3 +556,28 @@
max-height: none;
}
}
/* Standalone-page pending-consent chip (#874) the rail-less page's
counterpart of the L-shell rail badge. Rides the status bar next to
the token/tool/turn chips; hidden at zero via [hidden]. */
.ws-sb-consent {
color: var(--yellow);
font-weight: 500;
}
/* Below ~440px the sentence-length chip is the only wrappable element in
the nowrap status bar and fragments across 24 lines; let it drop
intact onto its own full line instead. Scoped to bars carrying the
chip (ws-sb-has-consent is set at chip mount) so the L-shell pane's
bar keeps its single-line behavior. Plain nowrap without the wrap
rules would be worse: the bar is overflow:hidden, so the warning
would clip out of existence. */
@media (max-width: 700px) {
.ws-status-bar.ws-sb-has-consent {
flex-wrap: wrap;
}
.ws-status-bar.ws-sb-has-consent .ws-sb-consent {
flex-basis: 100%;
white-space: nowrap;
}
}
@@ -44,6 +44,7 @@ import {
indexLabel,
} from "/shared/conversation.js";
import { redactCredentials } from "/shared/redact_credentials.js";
import { tryParseMcpError, buildMcpErrorEmbed } from "/shared/mcp_error.js";
import {
createQueueController,
parsePriority,
@@ -59,6 +60,145 @@ import {
degradedCooldownStep,
} from "/shared/sse_overflow.js";
// Standalone-page pending-consent chip (#874): the rail-less coordinator
// page's counterpart of the L-shell rail badge. The pending set is
// USER-scoped server truth (the Phase 9 mcp_oauth_pending table), not
// per-workstream state — the wording says "awaiting consent", never "this
// workstream". Inert until mount() (the L-shell pane never mounts it; the
// rail badge carries the signal there).
const _consentChip = (function () {
const pending = new Set();
let chipEl = null;
function paint() {
if (!chipEl) return;
const n = pending.size;
chipEl.hidden = n === 0;
if (n > 0) {
// Glyph is aria-hidden so screen readers announce only the plain
// sentence (an aria-label on a role-less span is ignored by
// several of them, which then voice the raw glyph).
chipEl.textContent = "";
const glyph = document.createElement("span");
glyph.setAttribute("aria-hidden", "true");
glyph.textContent = "⚠ ";
chipEl.appendChild(glyph);
chipEl.appendChild(
document.createTextNode(
n + " MCP server" + (n === 1 ? "" : "s") + " awaiting consent",
),
);
}
}
let hydrateInFlight = false;
let hydrateQueued = false;
function hydrate() {
// Merge-on-success: entries the server no longer lists are dropped
// ONLY if they predate this fetch (the preFetch snapshot) — a
// detection add()ed while the fetch was in flight survives, since
// its DB row may postdate the server's read. A failed fetch
// changes nothing, so a possibly-valid warning is never blanked.
// Single-flight: overlapping hydrates (mount + rapid visibility
// edges) can resolve out of order, and every guard short of
// exclusion re-admits some interleaving; one flight at a time makes
// the class structurally impossible, and an edge firing mid-flight
// queues exactly one rerun so the freshest truth still lands.
if (hydrateInFlight) {
hydrateQueued = true;
return;
}
// Bounded flight: a stalled fetch would otherwise hold the gate
// shut forever (the .finally below never runs, freezing self-heal);
// on timeout the chain rejects → .catch → .finally clears the gate
// and drains any queued rerun. Feature-detected like the
// codebase's AbortController guards — old runtimes just run
// unbounded, as before — and computed BEFORE the gate is set so no
// throw can wedge it (or escape mount()).
const signal =
typeof AbortSignal !== "undefined" && AbortSignal.timeout
? AbortSignal.timeout(10000)
: undefined;
hydrateInFlight = true;
const preFetch = new Set(pending);
authFetch("/v1/api/mcp/oauth/pending", { signal: signal })
.then(function (r) {
return r.ok ? r.json() : null;
})
.then(function (data) {
if (!data || !Array.isArray(data.servers)) return;
const fetched = new Set();
for (let i = 0; i < data.servers.length; i++) {
const row = data.servers[i];
if (row && typeof row.server_name === "string") {
fetched.add(row.server_name);
}
}
preFetch.forEach(function (s) {
if (!fetched.has(s)) pending.delete(s);
});
fetched.forEach(function (s) {
pending.add(s);
});
paint();
})
.catch(function () {})
.finally(function () {
hydrateInFlight = false;
if (hydrateQueued) {
hydrateQueued = false;
hydrate();
}
});
}
return {
mount: function (elRef) {
chipEl = elRef;
paint();
// Hydrate so a consent wall hit while nobody watched still
// surfaces here...
hydrate();
// ...and self-heal after the popup: the consent flow opens
// noopener (no cross-window channel exists), and the user lands
// back on this tab right after finishing there — so re-pull server
// truth on every visibility-show edge. mount() runs once per
// page, so this registers exactly one listener.
document.addEventListener("visibilitychange", function () {
if (document.visibilityState === "visible") hydrate();
});
},
add: function (server) {
if (typeof server === "string" && server) {
pending.add(server);
paint();
}
},
};
})();
// Consent-detection fan-out for the card's onConsent hook (#874): in the
// L-shell the console app exposes the same window.TS_APP.onConsentDetected
// seam the node dashboard does (driving the rail's MCP-row badge); the
// standalone page mounts the status-bar chip instead. Both are no-ops
// when their surface is absent.
function _notifyConsentDetected(server) {
const app = window.TS_APP;
if (app && typeof app.onConsentDetected === "function") {
app.onConsentDetected(server);
}
_consentChip.add(server);
}
// Shared MCP-error detection for both result paths (#725): parse + build
// ONLY — wrapper concerns stay with each call site (the live-row path
// adds the conv-row-result marker classes; the orphan path appends into
// .msg-body). Consent threading lands here once, for both (#874).
function _tryMcpErrorBlock(isError, output) {
if (!isError) return null;
const mcpErr = tryParseMcpError(output);
return mcpErr
? buildMcpErrorEmbed(mcpErr, output, _notifyConsentDetected)
: null;
}
function buildCoordChrome(root, opts) {
opts = opts || {};
root.classList.add("coord-chrome-root");
@@ -233,6 +373,23 @@ function createCoordinatorPane(root, wsId, opts) {
}
buildCoordChrome(root, opts);
if (opts && opts.standalone) {
// Rail-less page: mount the pending-consent chip in the status bar —
// the persistent signal the L-shell gets from the rail badge (#874).
const sb = root.querySelector("#coord-status-bar");
if (sb) {
// Marker class scopes the narrow-width wrap rules to bars that
// actually carry the chip (the L-shell pane's bar never does).
sb.classList.add("ws-sb-has-consent");
const chip = document.createElement("span");
chip.id = "coord-sb-consent";
chip.className = "ws-sb-consent";
chip.hidden = true;
sb.appendChild(chip);
_consentChip.mount(chip);
}
}
const messagesEl = root.querySelector("#coord-messages");
const coordMain = root.querySelector("#coord-main");
const composerMount = root.querySelector("#coord-composer-mount");
@@ -1005,6 +1162,17 @@ function createCoordinatorPane(root, wsId, opts) {
_scheduleScroll();
return entry.row;
}
// Orphan result (no live row — replay edge): still render the MCP
// error card rather than the raw envelope (#725).
const orphanCard = _tryMcpErrorBlock(isError, output);
if (orphanCard) {
const el = appendMsg("error", "", {
label: "error · " + (name || "tool"),
callId: callId,
});
el.querySelector(".msg-body").appendChild(orphanCard);
return el;
}
const html = renderToolOutput(output);
const el = appendMsg(isError ? "error" : "tool", html, {
label: (isError ? "error · " : "") + (name || "tool"),
@@ -1321,7 +1489,16 @@ function createCoordinatorPane(root, wsId, opts) {
const batch = row.closest(".conv-batch");
if (batch) batch.classList.add("conv-batch--error");
}
const block = buildConvResult(output, { isError });
// Structured MCP error envelope (consent / re-consent / forbidden /
// operator) renders as the shared card instead of raw JSON — the same
// dispatch the interactive pane does (#725). The card's Connect popup
// works here unchanged: the console hosts /v1/api/mcp/oauth/start and
// consent_url is relative. The conv-row-result marker keeps the
// replace-existing logic above and _refreshRowStatus working when the
// result is the card.
let block = _tryMcpErrorBlock(isError, output);
if (block) block.classList.add("conv-row-result");
if (!block) block = buildConvResult(output, { isError });
// Marker class so _refreshRowStatus can preserve the row's .error state
// across upgrade-in-place when the error came from a tool_result.
if (isError) block.classList.add("conv-row-result--error");
@@ -18,6 +18,7 @@
<link rel="stylesheet" href="/shared/ui-base.css" />
<link rel="stylesheet" href="/shared/chat.css" />
<link rel="stylesheet" href="/shared/conversation.css" />
<link rel="stylesheet" href="/shared/mcp_error.css" />
<link rel="stylesheet" href="/shared/katex-0.18.1/katex.min.css" />
<link rel="stylesheet" href="/static/style.css" />
<link rel="stylesheet" href="/static/coordinator/coordinator.css" />
+1
View File
@@ -25,6 +25,7 @@
<link rel="stylesheet" href="/static/coordinator/coordinator.css" />
<link rel="stylesheet" href="/static/coordinator/coord-chrome.css" />
<link rel="stylesheet" href="/shared/interactive.css" />
<link rel="stylesheet" href="/shared/mcp_error.css" />
<link rel="stylesheet" href="/shared/preview.css" />
<link rel="stylesheet" href="/shared/hatch.css" />
</head>
+10 -2
View File
@@ -9088,7 +9088,15 @@ def create_mcp_client(
) -> MCPClientManager | None:
"""Create and start an MCP client manager.
Returns *None* if no servers are configured.
Returns *None* if nothing is configured no static servers from any
source AND no pool-backed (``oauth_user``/``oauth_obo``) DB rows.
Pool-backed rows alone construct an empty-config manager: their
connections form lazily per user, so they contribute nothing to the
static *servers* dict, but the host still needs a running manager
for those pools to form on. (Returning None here for pool-only
installs left the host managerless after every restart no pools,
no MCP until the next admin MCP write or reload fan-out happened
to lazy-construct one.)
"""
# Check DB first to know which servers are DB-managed
db_names: set[str] = set()
@@ -9107,7 +9115,7 @@ def create_mcp_client(
log.warning("Failed to load DB-managed MCP servers", exc_info=True)
servers = load_mcp_config(config_path, storage=storage)
if not servers:
if not servers and not oauth_user_names and not obo_names:
return None
mgr = MCPClientManager(servers)
+66
View File
@@ -0,0 +1,66 @@
"""Wire-safety projections for MCP server status dicts.
Shared by the node's internal MCP endpoints (turnstone/server.py) and
the console's coordinator-facing MCP arms (#725) so the two hosts
present one status schema over the wire. Pure functions over plain
dicts anything stateful (outcome classification, HTTP mapping,
logging) stays with the callers.
"""
from __future__ import annotations
from typing import Any
_SERVER_STATUS_PUBLIC_KEYS: tuple[str, ...] = (
"connected",
"tools",
"resources",
"prompts",
"error",
"transport",
"circuit_open",
"consecutive_failures",
)
_READ_STATUS_PUBLIC_KEYS: tuple[str, ...] = tuple(
k for k in _SERVER_STATUS_PUBLIC_KEYS if k != "error"
)
def strip_server_status(full: dict[str, Any]) -> dict[str, Any]:
"""Project a status dict to the approve-scope public-safe key set.
The full status dict embeds ``command`` (stdio argv) and ``url``
(remote MCP endpoint) which are admin-only context. Approve-scoped
callers (refresh/reconnect) get the verbose ``error`` text so an
operator triaging a failure sees the underlying exception.
Read-scope callers must use :func:`strip_server_status_for_read`
instead error strings can carry stdio binary paths
(``FileNotFoundError: ... '/usr/local/bin/...'``) or internal MCP
URLs (``httpx.ConnectError: ... 'https://internal/...'``) and
those are equivalent to leaking ``command``/``url``.
"""
return {k: full[k] for k in _SERVER_STATUS_PUBLIC_KEYS if k in full}
def strip_server_status_for_read(full: dict[str, Any]) -> dict[str, Any]:
"""Project a status dict for read-scope callers.
Drops the verbose ``error`` text and replaces it with a coarse
``has_error: bool`` so dashboards can light up a failure indicator
without leaking the underlying exception detail.
"""
out = {k: full[k] for k in _READ_STATUS_PUBLIC_KEYS if k in full}
out["has_error"] = bool(full.get("error"))
return out
def public_server_status(mcp_mgr: Any, name: str) -> dict[str, Any]:
"""Strip ``command``/``url`` from ``get_server_status`` before returning over the wire."""
# aggregate=True: the operator refresh/reconnect endpoints are approve-scoped
# cluster actions with no single requesting user, so oauth_user servers report
# the any-user warm-pool view (matching the admin console) rather than the
# per-user default — which, with user_id=None, would render a warm, in-use
# server as disconnected/empty right after a successful refresh.
return strip_server_status(mcp_mgr.get_server_status(name, aggregate=True))
+69 -51
View File
@@ -1438,7 +1438,10 @@ class ChatSession:
self.model = model
# Coordinator plumbing: populated by the console's session factory
# only — ``kind == COORDINATOR`` sessions run COORDINATOR_TOOLS
# and dispatch tool execs through ``coord_client``.
# (plus a merged MCP surface when the factory passes an
# ``mcp_client``, #725) and dispatch their BUILTIN tool execs
# through ``coord_client``; MCP tools dispatch in-process via
# ``_prepare_mcp_tool``/``call_tool_sync`` like every other kind.
self._kind = kind
self._parent_ws_id = parent_ws_id if parent_ws_id else None
self._coord_client: Any = coord_client
@@ -1768,17 +1771,16 @@ class ChatSession:
self._mcp_refresh_cb: Any = None # Callable | None (avoid import)
self._mcp_resource_cb: Any = None
self._mcp_prompt_cb: Any = None
# Tool-set selection is kind-aware:
# * coordinator — fixed COORDINATOR_TOOLS, no MCP surface.
# Coordinators are meta-orchestrators that spawn child
# workstreams; MCP tools / resources / prompts live on the
# children. Giving the coordinator direct MCP access
# defeats the child-spawning pattern, so we don't merge
# MCP tools and don't register MCP listeners either.
# * interactive + mcp — INTERACTIVE_TOOLS mcp tools; MCP
# listeners register so tool/resource/prompt refreshes flow
# through to this session.
# * interactive (no mcp) — INTERACTIVE_TOOLS.
# Tool-set selection is kind-aware in the BASE only (see
# _set_session_tools):
# * coordinator — COORDINATOR_TOOLS base; with an mcp_client
# (the console factory passes the live console manager,
# #725) the MCP surface merges additively and the SAME
# listener/prime skeleton as interactive runs below. Client
# presence IS the session-level contract — the console
# counterpart of a node session holding mcp_ref[0].
# * interactive — INTERACTIVE_TOOLS mcp tools when a client
# is present; plain INTERACTIVE_TOOLS otherwise.
# Unconditional — every session kind carries the counter, so its
# existence never encodes "MCP was wired at construction" (a
# late-bound client or a directly-invoked callback must not
@@ -1789,13 +1791,7 @@ class ChatSession:
# value at the authoritative merged read, compared once at the
# end of tool setup. Only ``_mcp_tools_change_seq`` lives on.
mcp_tools_seq_at_read = 0
if kind == WorkstreamKind.COORDINATOR:
# No _set_interactive_tools/_apply_cwd_notes: COORDINATOR_TOOLS
# holds no cwd-dependent tool (no fs/shell), so there is nothing
# to render.
self._tools = list(COORDINATOR_TOOLS)
self._task_tools: list[dict[str, Any]] = []
elif self._mcp_client:
if self._mcp_client:
# ``self._mcp_client`` (not the raw kwarg) so an MCP-off persona
# falls through to the no-MCP branch below.
# Constants first — the attributes must exist for a listener
@@ -1803,7 +1799,7 @@ class ChatSession:
# merged read runs after the registrations below. (Also warms
# the get_workspace_dir config cache on the main thread, before
# any background-thread rebuild can race the first load.)
self._set_interactive_tools([])
self._set_session_tools([])
# Register for tool-change notifications from MCP servers.
# ``user_id`` is the listener identity component — pool-only
# changes for OTHER users must not fire this callback.
@@ -1832,7 +1828,7 @@ class ChatSession:
# the tool-search construction below reading mixed state.
mcp_tools_seq_at_read = self._mcp_tools_change_seq
mcp_tools = self._mcp_client.get_tools(user_id=self._mcp_user_id)
self._set_interactive_tools(mcp_tools)
self._set_session_tools(mcp_tools)
# Proactively warm this user's per-user OAuth (oauth_user) pools so
# their tools are present without a manual reconnect (e.g. after a
# reboot/upgrade, or right after consent). Fire-and-forget — the
@@ -1841,7 +1837,7 @@ class ChatSession:
# consented oauth_user servers.
try_prime_user_pools(self._mcp_client, self._mcp_user_id, context="session-start")
else:
self._set_interactive_tools([])
self._set_session_tools([])
# Inject the live alias list into the task_agent tool
# description so the calling LLM sees its `model` parameter options.
# Replaces affected tool dicts with deep copies — module-level
@@ -1893,11 +1889,7 @@ class ChatSession:
# dependency now exists, so re-running the full callback is
# safe — it rebuilds the merged lists, rendered descriptions,
# and search index from the CURRENT maps.
if (
self._kind != WorkstreamKind.COORDINATOR
and self._mcp_client
and self._mcp_tools_change_seq != mcp_tools_seq_at_read
):
if self._mcp_client and self._mcp_tools_change_seq != mcp_tools_seq_at_read:
self._on_mcp_tools_changed()
# Skill: explicit name overrides is_default skills. ``skill_arguments``
# carries the spec's $ARGUMENTS payload — set at create/load time,
@@ -2587,10 +2579,6 @@ class ChatSession:
"""
if not self._mcp_client:
return
# Coordinator sessions don't consume MCP tools — the tool set
# is fixed at COORDINATOR_TOOLS. Ignore MCP server changes.
if self._kind == WorkstreamKind.COORDINATOR:
return
# Monotonic change marker: the constructor snapshots this around
# its authoritative post-registration read and re-runs this
# callback if it advanced — otherwise a notification landing
@@ -2603,7 +2591,7 @@ class ChatSession:
# regardless; ``user_id=None`` would silently drop pool tools
# that the LLM is allowed to call.
mcp_tools = self._mcp_client.get_tools(user_id=self._mcp_effective_user_id)
self._set_interactive_tools(mcp_tools)
self._set_session_tools(mcp_tools)
self._render_agent_tool_descriptions()
self._rebuild_tool_search()
@@ -2656,24 +2644,38 @@ class ChatSession:
parts.append(entry)
return "Available personas: " + "; ".join(parts) + "."
def _set_interactive_tools(self, mcp_tools: list[dict[str, Any]]) -> None:
"""Build both interactive tool lanes from pristine bases + MCP catalog.
def _set_session_tools(self, mcp_tools: list[dict[str, Any]]) -> None:
"""Build this session's tool lanes from pristine bases + MCP catalog.
THE single assignment path for every fresh interactive build of
``self._tools`` AND ``self._task_tools`` each lane routes through
``_apply_cwd_notes`` here, so a future rebuild site cannot silently
drop the working-dir/workspace notes by assigning from the module
constants directly. Pass ``[]`` when there is no MCP surface
(construction pre-read, MCP-off, disconnect): ``merge_mcp_tools``
with an empty list is just a fresh copy of the builtin base.
THE single assignment path for every fresh build of ``self._tools``
AND ``self._task_tools`` kind-aware in the BASE only, so every
rebuild site (construction pre-read, authoritative read, catalog
change, MCP drop) and any future one is kind-correct by
construction rather than by remembering a guard:
* coordinator ``merge_mcp_tools(COORDINATOR_TOOLS, mcp_tools)``;
no cwd notes (no cwd-dependent tool in the base, and MCP tools
never carry one) and no task-agent lane.
* interactive both lanes route through ``_apply_cwd_notes`` so a
rebuild cannot silently drop the working-dir/workspace notes by
assigning from the module constants directly.
Pass ``[]`` when there is no MCP surface (construction pre-read,
MCP-off, disconnect): ``merge_mcp_tools`` with an empty list is a
fresh copy of the builtin base, so the no-client and drop paths
reproduce the fixed kind base verbatim.
"""
if self._kind == WorkstreamKind.COORDINATOR:
self._tools = merge_mcp_tools(COORDINATOR_TOOLS, mcp_tools)
self._task_tools: list[dict[str, Any]] = []
return
self._tools = self._apply_cwd_notes(merge_mcp_tools(INTERACTIVE_TOOLS, mcp_tools))
self._task_tools = self._apply_cwd_notes(merge_mcp_tools(TASK_AGENT_TOOLS, mcp_tools))
def _apply_cwd_notes(self, tools: list[dict[str, Any]]) -> list[dict[str, Any]]:
"""Render per-process working-dir/workspace notes into fs-tool descriptions.
Reached via ``_set_interactive_tools``, which routes BOTH lanes
Reached via ``_set_session_tools`` (interactive lane), which routes BOTH lanes
(``self._tools`` and the separate ``self._task_tools`` sub-agent
list) through here at every fresh interactive build. Applied at
assignment time, so the copy cost is paid per rebuild (construction,
@@ -2685,8 +2687,9 @@ class ChatSession:
``_render_agent_tool_descriptions`` re-derive is NOT a fresh build
and must not re-apply: it deep-copies only the persona/model
param-description tools, passing fs tools (and these notes) through
untouched. Coordinator builds skip all of this COORDINATOR_TOOLS
holds no cwd-dependent tool.
untouched. Coordinator builds never reach here their
``_set_session_tools`` lane skips notes (COORDINATOR_TOOLS holds no
cwd-dependent tool, and MCP tools never carry one).
``os.getcwd()`` is what a cwd-less Popen inherits (spawn_group_leader)
and what relative file-tool paths resolve against, so the note names
@@ -3154,8 +3157,10 @@ class ChatSession:
adopting an MCP-off stamp. Deregisters the three listeners (same
``_mcp_listener_user_id`` identity rule as ``close()``), drops the
client reference, and resets both toolsets to the builtin lists.
Only reachable on interactive sessions coordinators never hold a
client. The caller rebuilds tool search and recomposes the prompt.
Reachable on BOTH kinds (#725): a coordinator resume() can adopt an
MCP-off stamp too the kind-aware ``_set_session_tools`` resets it
to COORDINATOR_TOOLS, never the interactive lanes. The caller
rebuilds tool search and recomposes the prompt.
"""
if self._mcp_client is None:
return
@@ -3175,7 +3180,7 @@ class ChatSession:
)
self._mcp_prompt_cb = None
self._mcp_client = None
self._set_interactive_tools([])
self._set_session_tools([])
self._render_agent_tool_descriptions()
def _handle_mcp_refresh(self, arg: str) -> None:
@@ -5919,7 +5924,13 @@ class ChatSession:
return
self._acting_user_id = user_id
mcp = self._mcp_client
if not mcp or self._kind == WorkstreamKind.COORDINATOR:
# Coordinators participate fully (#725): they are multi-sender by
# design (any admin.coordinator operator with project visibility
# may drive one — ownership is not enforced), so an acting-user
# change MUST re-scope the listeners and prime the new user's
# pools below; otherwise operator B would dispatch against
# operator A's primed pool catalog.
if not mcp:
return
old_listener_uid = self._mcp_listener_user_id
new_listener_uid: str | None = self._mcp_effective_user_id
@@ -5939,10 +5950,17 @@ class ChatSession:
# state under the new identity NOW — the prime above completes
# asynchronously and only notifies on catalog changes, while
# already-warm pool entries for this user produce no
# notification at all.
# notification at all. ONE _init_system_messages() covers both
# the resource and prompt catalogs: the _on_mcp_resources_changed
# / _on_mcp_prompts_changed wrappers are pure passthroughs to it
# (they exist for the manager's separate notification channels,
# which still fire them independently), and it rebuilds the full
# system message copy-on-write — calling it twice per handoff was
# a redundant list_prompt_policies() read + compose per rebind.
# The persona-catalog read inside the tools rebuild stays as-is:
# sender-independent but handoff-frequency, not worth memoizing.
self._on_mcp_tools_changed()
self._on_mcp_resources_changed()
self._on_mcp_prompts_changed()
self._init_system_messages()
def send(
self,
+10 -53
View File
@@ -57,6 +57,12 @@ from turnstone.core.auth import (
)
from turnstone.core.idle_nudge_watcher import wake_workstream_if_pending
from turnstone.core.log import get_logger
from turnstone.core.mcp_utils import (
public_server_status as _public_server_status,
)
from turnstone.core.mcp_utils import (
strip_server_status_for_read as _strip_server_status_for_read,
)
from turnstone.core.metrics import metrics as _metrics
from turnstone.core.model_turn import resolve_effort_setting, resolve_temperature_setting
from turnstone.core.ratelimit import resolve_client_ip
@@ -3725,59 +3731,10 @@ def internal_mcp_reload(request: Request) -> JSONResponse:
return JSONResponse({"status": "ok", **result})
_SERVER_STATUS_PUBLIC_KEYS: tuple[str, ...] = (
"connected",
"tools",
"resources",
"prompts",
"error",
"transport",
"circuit_open",
"consecutive_failures",
)
_READ_STATUS_PUBLIC_KEYS: tuple[str, ...] = tuple(
k for k in _SERVER_STATUS_PUBLIC_KEYS if k != "error"
)
def _strip_server_status(full: dict[str, Any]) -> dict[str, Any]:
"""Project a status dict to the approve-scope public-safe key set.
The full status dict embeds ``command`` (stdio argv) and ``url``
(remote MCP endpoint) which are admin-only context. Approve-scoped
callers (refresh/reconnect) get the verbose ``error`` text so an
operator triaging a failure sees the underlying exception.
Read-scope callers must use :func:`_strip_server_status_for_read`
instead error strings can carry stdio binary paths
(``FileNotFoundError: ... '/usr/local/bin/...'``) or internal MCP
URLs (``httpx.ConnectError: ... 'https://internal/...'``) and
those are equivalent to leaking ``command``/``url``.
"""
return {k: full[k] for k in _SERVER_STATUS_PUBLIC_KEYS if k in full}
def _strip_server_status_for_read(full: dict[str, Any]) -> dict[str, Any]:
"""Project a status dict for read-scope callers.
Drops the verbose ``error`` text and replaces it with a coarse
``has_error: bool`` so dashboards can light up a failure indicator
without leaking the underlying exception detail.
"""
out = {k: full[k] for k in _READ_STATUS_PUBLIC_KEYS if k in full}
out["has_error"] = bool(full.get("error"))
return out
def _public_server_status(mcp_mgr: Any, name: str) -> dict[str, Any]:
"""Strip ``command``/``url`` from ``get_server_status`` before returning over the wire."""
# aggregate=True: the operator refresh/reconnect endpoints are approve-scoped
# cluster actions with no single requesting user, so oauth_user servers report
# the any-user warm-pool view (matching the admin console) rather than the
# per-user default — which, with user_id=None, would render a warm, in-use
# server as disconnected/empty right after a successful refresh.
return _strip_server_status(mcp_mgr.get_server_status(name, aggregate=True))
# Wire-safety status projections live in core/mcp_utils (#725: the
# console's coordinator-facing arms present the same schema); imported
# at the top of this module under the established private names so
# every endpoint body below stays byte-identical.
def internal_mcp_status(request: Request) -> JSONResponse:
+3 -107
View File
@@ -351,110 +351,6 @@ audio.media-player {
padding-top: 14px;
}
/* ==========================================================================
MCP error embed (consent / scope / forbidden / operator)
========================================================================== */
.mcp-error-card {
border-top: 1px solid var(--border);
background: var(--code-bg);
padding: 12px;
display: grid;
grid-template-columns: auto 1fr;
gap: 12px;
align-items: start;
font-family: var(--font-ui);
font-size: 13px;
}
.mcp-error-icon {
font-size: 20px;
line-height: 1;
color: var(--accent);
}
.mcp-error-card.mcp-error-forbidden .mcp-error-icon,
.mcp-error-card.mcp-error-operator .mcp-error-icon {
color: var(--red);
}
.mcp-error-card.mcp-error-transient .mcp-error-icon {
color: var(--yellow);
}
.mcp-error-body {
display: flex;
flex-direction: column;
gap: 6px;
}
.mcp-error-title {
font-weight: 600;
color: var(--fg);
}
.mcp-error-detail {
color: var(--fg-dim);
}
.mcp-error-server,
.mcp-error-scopes {
font-size: 12px;
color: var(--fg-dim);
}
.mcp-error-server code {
font-family: var(--font-mono);
font-size: 12px;
color: var(--fg);
}
.mcp-scope-pill {
display: inline-block;
background: var(--code-bg);
border: 1px solid var(--border);
border-radius: 10px;
padding: 2px 8px;
margin-right: 4px;
font-family: var(--font-mono);
font-size: 11px;
color: var(--fg);
}
.mcp-error-action-btn {
margin-top: 6px;
padding: 6px 12px;
background: var(--accent);
color: var(--bg);
border: none;
border-radius: 4px;
font-family: var(--font-ui);
font-size: 13px;
font-weight: 500;
cursor: pointer;
align-self: start;
}
.mcp-error-action-btn:hover {
filter: brightness(1.1);
}
.mcp-error-card details {
grid-column: 1 / -1;
margin-top: 6px;
}
.mcp-error-card details summary {
cursor: pointer;
font-size: 11px;
color: var(--fg-dim);
padding: 4px 0;
}
.mcp-error-card details pre {
margin: 4px 0 0 0;
font-family: var(--font-mono);
font-size: 11px;
color: var(--fg-dim);
white-space: pre-wrap;
word-break: break-all;
}
/* MCP error embed rules live in shared_static/mcp_error.css the sheet
pairs with mcp_error.js and is linked by every card host (dual-static
rule: shared visual rules live in shared_static, next to their JS). */
+7 -181
View File
@@ -39,7 +39,8 @@ import {
batchKicker,
indexLabel,
} from "./conversation.js";
import { redactCredentials } from "./redact_credentials.js";
import { redactCredentials, tryPrettyJson } from "./redact_credentials.js";
import { tryParseMcpError, buildMcpErrorEmbed } from "./mcp_error.js";
import { authFetch } from "./auth.js";
import { showToast } from "./toast.js";
import { Composer } from "./composer.js";
@@ -3991,16 +3992,6 @@ function _formatRuntime(item) {
return h > 0 ? h + "h " + m + "m" : m + "m";
}
function _tryPrettyJson(text) {
let obj;
try {
obj = JSON.parse(text);
} catch (e) {
return null;
}
return redactCredentials(JSON.stringify(obj, null, 2));
}
// ---------------------------------------------------------------------------
// HLS lazy-loader + click-to-play (lifted from the standalone app.js so
// console-hosted panes activate media too). Follows the mermaid.js
@@ -4296,40 +4287,6 @@ function buildMediaResultsList(results, totalCount) {
return container;
}
function _mcpErrorCategory(code) {
if (code === "mcp_consent_required" || code === "mcp_insufficient_scope") {
return "actionable";
}
if (
code === "mcp_token_undecryptable_key_unknown" ||
code === "mcp_oauth_url_insecure"
) {
return "operator";
}
if (code === "mcp_refresh_unavailable") {
// Soft, retryable state — a transient refresh failure, not a hard denial.
return "transient";
}
// Default for any other mcp_*_forbidden / unrecognised mcp_ code.
return "forbidden";
}
function _mcpErrorTitle(err) {
switch (err.code) {
case "mcp_consent_required":
return "Consent required";
case "mcp_insufficient_scope":
return "Re-consent required (insufficient scope)";
case "mcp_token_undecryptable_key_unknown":
case "mcp_oauth_url_insecure":
return "Operator action required";
case "mcp_refresh_unavailable":
return "Temporarily unavailable";
default:
return "Forbidden";
}
}
// ---------------------------------------------------------------------------
// Conversational rendering helpers — tool output, media embeds, MCP-error
// cards. Used only by the Pane. The approval-card builders (rows, verdict,
@@ -4459,7 +4416,7 @@ function renderToolOutput(stripped, isError) {
const out = document.createElement("div");
out.className = "tool-output" + (isError ? " tool-output-error" : "");
if (!isError) {
const pretty = _tryPrettyJson(stripped);
const pretty = tryPrettyJson(stripped);
if (pretty) {
out.textContent = pretty;
return out;
@@ -4500,147 +4457,16 @@ function buildMediaEmbed(media, rawJson) {
// Collapsed raw JSON for inspection (with redacted API keys)
const raw = document.createElement("div");
raw.className = "tool-output";
raw.textContent = _tryPrettyJson(rawJson) || redactCredentials(rawJson);
raw.textContent = tryPrettyJson(rawJson) || redactCredentials(rawJson);
makeCollapsible(raw);
wrapper.appendChild(raw);
return wrapper;
}
function tryParseMcpError(text) {
let obj;
try {
obj = JSON.parse(text);
} catch (e) {
return null;
}
if (!obj || typeof obj !== "object") return null;
const err = obj.error;
if (!err || typeof err !== "object") return null;
if (typeof err.code !== "string" || err.code.indexOf("mcp_") !== 0)
return null;
return err;
}
function buildMcpErrorEmbed(err, rawJson, onConsent) {
const category = _mcpErrorCategory(err.code);
const wrapper = document.createElement("div");
wrapper.className = "mcp-error-card mcp-error-" + category;
const icon = document.createElement("div");
icon.className = "mcp-error-icon";
icon.setAttribute("aria-hidden", "true");
icon.textContent = "⚠";
wrapper.appendChild(icon);
const body = document.createElement("div");
body.className = "mcp-error-body";
const title = document.createElement("div");
title.className = "mcp-error-title";
title.textContent = _mcpErrorTitle(err);
body.appendChild(title);
if (err.detail) {
const detail = document.createElement("div");
detail.className = "mcp-error-detail";
detail.textContent = String(err.detail);
body.appendChild(detail);
}
if (err.server) {
const serverLine = document.createElement("div");
serverLine.className = "mcp-error-server";
serverLine.appendChild(document.createTextNode("server: "));
const serverCode = document.createElement("code");
serverCode.textContent = String(err.server);
serverLine.appendChild(serverCode);
body.appendChild(serverLine);
}
if (Array.isArray(err.scopes_required) && err.scopes_required.length) {
const scopesLine = document.createElement("div");
scopesLine.className = "mcp-error-scopes";
scopesLine.appendChild(document.createTextNode("scopes: "));
for (let i = 0; i < err.scopes_required.length; i++) {
const pill = document.createElement("span");
pill.className = "mcp-scope-pill";
pill.textContent = String(err.scopes_required[i]);
scopesLine.appendChild(pill);
}
body.appendChild(scopesLine);
}
// Render the Connect / Re-consent button only when the dispatcher actually
// supplied a per-server consent URL. An "actionable"-category code with no
// consent_url means there is no per-server consent flow for this server —
// sign-in passthrough (oauth_obo) mints from the user's Turnstone sign-in and
// is deliberately absent from the Settings connections list and rejected by
// /start — so a button here would dead-end ("no consent URL; open Settings"
// pointing at a panel with nothing to connect). In that case the honest
// remedy is the detail text (sign in again / ask your administrator), so we
// show the card without a broken affordance.
//
// This never wrongly hides a needed button for oauth_user: the backend
// invariant is that _build_consent_url returns a /v1/api/mcp/oauth/start URL
// for EVERY oauth_user row and None only for non-oauth_user auth types, so an
// oauth_user actionable error always carries a valid consent_url and always
// renders its button. The removed click-time "open Settings" fallback guarded
// a producer path that that invariant makes unreachable.
const consentUrl = err.consent_url;
const hasConsentAffordance =
typeof consentUrl === "string" &&
consentUrl.startsWith("/v1/api/mcp/oauth/start");
if (category === "actionable" && hasConsentAffordance) {
const btn = document.createElement("button");
btn.type = "button";
btn.className = "mcp-error-action-btn";
const serverLabel = err.server ? String(err.server) : "server";
btn.textContent =
err.code === "mcp_insufficient_scope"
? "Re-consent with new scopes →"
: "Connect to " + serverLabel + " →";
btn.setAttribute(
"aria-label",
err.code === "mcp_insufficient_scope"
? "Re-consent with new scopes for " + serverLabel
: "Connect to " + serverLabel,
);
btn.addEventListener("click", function () {
// Defence-in-depth: the render gate already proved the prefix, but
// re-check at click time — a non-prefix value would indicate producer
// drift or a compromised dispatcher, and window.open("javascript:...")
// would be catastrophic. Never rely on the producer-side guarantee alone.
if (!consentUrl.startsWith("/v1/api/mcp/oauth/start")) {
showToast("Invalid consent URL");
return;
}
const sep = consentUrl.indexOf("?") >= 0 ? "&" : "?";
const url =
consentUrl +
sep +
"return_url=" +
encodeURIComponent(window.location.href);
window.open(url, "_blank", "noopener");
});
body.appendChild(btn);
if (onConsent) onConsent(err.server);
}
wrapper.appendChild(body);
const details = document.createElement("details");
const summary = document.createElement("summary");
summary.textContent = "raw payload";
details.appendChild(summary);
const pre = document.createElement("pre");
pre.className = "tool-output";
pre.textContent = _tryPrettyJson(rawJson) || redactCredentials(rawJson);
details.appendChild(pre);
wrapper.appendChild(details);
return wrapper;
}
// tryParseMcpError / buildMcpErrorEmbed moved to the shared mcp_error.js
// module (#725) so the coordinator pane renders the same consent /
// forbidden / operator card — imported at the top of this file.
// ---------------------------------------------------------------------------
// createInteractivePane — the console L-shell factory.
+140
View File
@@ -0,0 +1,140 @@
/* ==========================================================================
MCP error embed (consent / scope / forbidden / operator) pairs with
mcp_error.js and owns EVERY class that module emits, so any page that
renders the card gets its styling from this one sheet. Linked by all
three card hosts: the standalone coordinator page, the console L-shell,
and the node interactive page. Tokens resolve via shared base.css,
which every host links. (Reach pinned by the classNamerule parity
test in tests/test_app_js.py.)
========================================================================== */
.mcp-error-card {
border-top: 1px solid var(--border);
background: var(--code-bg);
padding: 12px;
display: grid;
grid-template-columns: auto 1fr;
gap: 12px;
align-items: start;
font-family: var(--font-ui);
font-size: 13px;
}
.mcp-error-icon {
font-size: 20px;
line-height: 1;
color: var(--accent);
}
.mcp-error-card.mcp-error-forbidden .mcp-error-icon,
.mcp-error-card.mcp-error-operator .mcp-error-icon {
color: var(--red);
}
.mcp-error-card.mcp-error-transient .mcp-error-icon {
color: var(--yellow);
}
/* .mcp-error-actionable deliberately has no rule: the base card look IS
the actionable look (accent icon + Connect button). */
.mcp-error-body {
display: flex;
flex-direction: column;
gap: 6px;
}
.mcp-error-title {
font-weight: 600;
color: var(--fg);
}
.mcp-error-detail {
color: var(--fg-dim);
}
.mcp-error-server,
.mcp-error-scopes {
font-size: 12px;
color: var(--fg-dim);
}
.mcp-error-server code {
font-family: var(--font-mono);
font-size: 12px;
color: var(--fg);
}
.mcp-scope-pill {
display: inline-block;
/* Panel-on-code-bg like the raw-payload box below: the card itself is
--code-bg, so a --code-bg pill had zero fill contrast and the
0.06/0.08-alpha --border read as a bare hairline (below the 0.15-α
chip rule) --panel + solid --hair lift it off the card in both
themes. */
background: var(--panel);
border: 1px solid var(--hair);
border-radius: 10px;
padding: 2px 8px;
margin-right: 4px;
font-family: var(--font-mono);
font-size: 11px;
color: var(--fg);
}
.mcp-error-action-btn {
margin-top: 6px;
padding: 6px 12px;
background: var(--accent);
color: var(--bg);
border: none;
border-radius: 4px;
font-family: var(--font-ui);
font-size: 13px;
font-weight: 500;
cursor: pointer;
align-self: start;
}
.mcp-error-action-btn:hover {
filter: brightness(1.1);
}
.mcp-error-card details {
grid-column: 1 / -1;
margin-top: 6px;
}
.mcp-error-card details summary {
cursor: pointer;
font-size: 11px;
color: var(--fg-dim);
padding: 4px 0;
}
.mcp-error-card details pre {
margin: 4px 0 0 0;
font-family: var(--font-mono);
font-size: 11px;
color: var(--fg-dim);
white-space: pre-wrap;
word-break: break-all;
}
/* The card's raw-payload <pre> carries className "tool-output" so it
matches the hosts' generic tool-output look but pages without those
generic rules (the standalone coordinator page) must not render it
bare. This rule carries ONLY the box properties the generic
.tool-output rules supply (interactive.css:122 padding/max-height/
overflow; :330 panel surface/border/radius): the typography properties
(font, size, color, wrap) stay with `.mcp-error-card details pre`
above, which out-specifies the generic rules today carrying them
here too would flip that contest on interactive hosts and shift
rendering. */
.mcp-error-card .tool-output {
padding: 8px 12px;
background: var(--panel);
border: 1px solid var(--hair);
border-radius: var(--r-sm);
max-height: 300px;
overflow-y: auto;
}
+184
View File
@@ -0,0 +1,184 @@
// ---------------------------------------------------------------------------
// Structured MCP error envelope — shared detector + card builder.
//
// Lifted from interactive.js (#725) so BOTH conversation surfaces render the
// same consent / re-consent / forbidden / operator card: the interactive pane
// and the coordinator pane (whose sessions carry the same MCP surface,
// persona-gated like interactive). The card is self-contained — the Connect /
// Re-consent button opens the relative /v1/api/mcp/oauth/start popup, which
// both hosts serve — and the optional onConsent callback is a notification
// hook only (the interactive pane threads it to the standalone consent badge
// through host.onConsentDetected; the coordinator pane omits it).
// ---------------------------------------------------------------------------
import { redactCredentials, tryPrettyJson } from "./redact_credentials.js";
import { showToast } from "./toast.js";
function _mcpErrorCategory(code) {
if (code === "mcp_consent_required" || code === "mcp_insufficient_scope") {
return "actionable";
}
if (
code === "mcp_token_undecryptable_key_unknown" ||
code === "mcp_oauth_url_insecure"
) {
return "operator";
}
if (code === "mcp_refresh_unavailable") {
// Soft, retryable state — a transient refresh failure, not a hard denial.
return "transient";
}
// Default for any other mcp_*_forbidden / unrecognised mcp_ code.
return "forbidden";
}
function _mcpErrorTitle(err) {
switch (err.code) {
case "mcp_consent_required":
return "Consent required";
case "mcp_insufficient_scope":
return "Re-consent required (insufficient scope)";
case "mcp_token_undecryptable_key_unknown":
case "mcp_oauth_url_insecure":
return "Operator action required";
case "mcp_refresh_unavailable":
return "Temporarily unavailable";
default:
return "Forbidden";
}
}
export function tryParseMcpError(text) {
let obj;
try {
obj = JSON.parse(text);
} catch (e) {
return null;
}
if (!obj || typeof obj !== "object") return null;
const err = obj.error;
if (!err || typeof err !== "object") return null;
if (typeof err.code !== "string" || err.code.indexOf("mcp_") !== 0)
return null;
return err;
}
export function buildMcpErrorEmbed(err, rawJson, onConsent) {
const category = _mcpErrorCategory(err.code);
const wrapper = document.createElement("div");
wrapper.className = "mcp-error-card mcp-error-" + category;
const icon = document.createElement("div");
icon.className = "mcp-error-icon";
icon.setAttribute("aria-hidden", "true");
icon.textContent = "⚠";
wrapper.appendChild(icon);
const body = document.createElement("div");
body.className = "mcp-error-body";
const title = document.createElement("div");
title.className = "mcp-error-title";
title.textContent = _mcpErrorTitle(err);
body.appendChild(title);
if (err.detail) {
const detail = document.createElement("div");
detail.className = "mcp-error-detail";
detail.textContent = String(err.detail);
body.appendChild(detail);
}
if (err.server) {
const serverLine = document.createElement("div");
serverLine.className = "mcp-error-server";
serverLine.appendChild(document.createTextNode("server: "));
const serverCode = document.createElement("code");
serverCode.textContent = String(err.server);
serverLine.appendChild(serverCode);
body.appendChild(serverLine);
}
if (Array.isArray(err.scopes_required) && err.scopes_required.length) {
const scopesLine = document.createElement("div");
scopesLine.className = "mcp-error-scopes";
scopesLine.appendChild(document.createTextNode("scopes: "));
for (let i = 0; i < err.scopes_required.length; i++) {
const pill = document.createElement("span");
pill.className = "mcp-scope-pill";
pill.textContent = String(err.scopes_required[i]);
scopesLine.appendChild(pill);
}
body.appendChild(scopesLine);
}
// Render the Connect / Re-consent button only when the dispatcher actually
// supplied a per-server consent URL. An "actionable"-category code with no
// consent_url means there is no per-server consent flow for this server —
// sign-in passthrough (oauth_obo) mints from the user's Turnstone sign-in and
// is deliberately absent from the Settings connections list and rejected by
// /start — so a button here would dead-end ("no consent URL; open Settings"
// pointing at a panel with nothing to connect). In that case the honest
// remedy is the detail text (sign in again / ask your administrator), so we
// show the card without a broken affordance.
//
// This never wrongly hides a needed button for oauth_user: the backend
// invariant is that _build_consent_url returns a /v1/api/mcp/oauth/start URL
// for EVERY oauth_user row and None only for non-oauth_user auth types, so an
// oauth_user actionable error always carries a valid consent_url and always
// renders its button. The removed click-time "open Settings" fallback guarded
// a producer path that that invariant makes unreachable.
const consentUrl = err.consent_url;
const hasConsentAffordance =
typeof consentUrl === "string" &&
consentUrl.startsWith("/v1/api/mcp/oauth/start");
if (category === "actionable" && hasConsentAffordance) {
const btn = document.createElement("button");
btn.type = "button";
btn.className = "mcp-error-action-btn";
const serverLabel = err.server ? String(err.server) : "server";
btn.textContent =
err.code === "mcp_insufficient_scope"
? "Re-consent with new scopes →"
: "Connect to " + serverLabel + " →";
btn.setAttribute(
"aria-label",
err.code === "mcp_insufficient_scope"
? "Re-consent with new scopes for " + serverLabel
: "Connect to " + serverLabel,
);
btn.addEventListener("click", function () {
// Defence-in-depth: the render gate already proved the prefix, but
// re-check at click time — a non-prefix value would indicate producer
// drift or a compromised dispatcher, and window.open("javascript:...")
// would be catastrophic. Never rely on the producer-side guarantee alone.
if (!consentUrl.startsWith("/v1/api/mcp/oauth/start")) {
showToast("Invalid consent URL");
return;
}
const sep = consentUrl.indexOf("?") >= 0 ? "&" : "?";
const url =
consentUrl +
sep +
"return_url=" +
encodeURIComponent(window.location.href);
window.open(url, "_blank", "noopener");
});
body.appendChild(btn);
if (onConsent) onConsent(err.server);
}
wrapper.appendChild(body);
const details = document.createElement("details");
const summary = document.createElement("summary");
summary.textContent = "raw payload";
details.appendChild(summary);
const pre = document.createElement("pre");
pre.className = "tool-output";
pre.textContent = tryPrettyJson(rawJson) || redactCredentials(rawJson);
details.appendChild(pre);
wrapper.appendChild(details);
return wrapper;
}
@@ -211,3 +211,18 @@ export function redactCredentials(text) {
return result;
}
// Pretty-print JSON WITH redaction — the sanctioned way to render a raw
// JSON payload for display (interactive.js raw-payload views, the shared
// MCP error card). Lives here, next to the redaction it wraps, so a
// future caller cannot pretty-print without redacting. Returns null for
// non-JSON input; callers fall back to redactCredentials(text) directly.
export function tryPrettyJson(text) {
let obj;
try {
obj = JSON.parse(text);
} catch (e) {
return null;
}
return redactCredentials(JSON.stringify(obj, null, 2));
}
+2
View File
@@ -11,6 +11,8 @@
},
"required": ["uri"]
},
"coordinator": true,
"interactive": true,
"task_agent": true,
"auto_approve": false,
"primary_key": "uri"
+2
View File
@@ -16,6 +16,8 @@
},
"required": ["name"]
},
"coordinator": true,
"interactive": true,
"task_agent": true,
"auto_approve": false,
"primary_key": "name"
+1
View File
@@ -22,6 +22,7 @@
<link rel="stylesheet" href="/static/style.css" />
<link rel="stylesheet" href="/shared/shell.css" />
<link rel="stylesheet" href="/shared/interactive.css" />
<link rel="stylesheet" href="/shared/mcp_error.css" />
<link rel="stylesheet" href="/shared/preview.css" />
<link rel="stylesheet" href="/shared/hatch.css" />
<link rel="stylesheet" href="/shared/katex-0.18.1/katex.min.css" />