diff --git a/tests/test_app_js.py b/tests/test_app_js.py
index d040a6b0..cef8e386 100644
--- a/tests/test_app_js.py
+++ b/tests/test_app_js.py
@@ -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
diff --git a/tests/test_console_session_factory.py b/tests/test_console_session_factory.py
index 444f210e..8dc1e63b 100644
--- a/tests/test_console_session_factory.py
+++ b/tests/test_console_session_factory.py
@@ -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
diff --git a/tests/test_coordinator_tools.py b/tests/test_coordinator_tools.py
index 6426c826..a3a9c9e2 100644
--- a/tests/test_coordinator_tools.py
+++ b/tests/test_coordinator_tools.py
@@ -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 == []
diff --git a/tests/test_mcp_admin_api.py b/tests/test_mcp_admin_api.py
index 010c4edb..a3cc0cc6 100644
--- a/tests/test_mcp_admin_api.py
+++ b/tests/test_mcp_admin_api.py
@@ -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)
diff --git a/tests/test_mcp_client.py b/tests/test_mcp_client.py
index 73a4d45c..96cc01fb 100644
--- a/tests/test_mcp_client.py
+++ b/tests/test_mcp_client.py
@@ -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
diff --git a/tests/test_shell_js.py b/tests/test_shell_js.py
index 5b446cf8..a9492a71 100644
--- a/tests/test_shell_js.py
+++ b/tests/test_shell_js.py
@@ -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``
diff --git a/tests/test_tools_schema.py b/tests/test_tools_schema.py
index 89d055cc..bbf41272 100644
--- a/tests/test_tools_schema.py
+++ b/tests/test_tools_schema.py
@@ -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):
diff --git a/tests/test_workstream_kind.py b/tests/test_workstream_kind.py
index c033332b..b6c3eca6 100644
--- a/tests/test_workstream_kind.py
+++ b/tests/test_workstream_kind.py
@@ -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"
+ )
diff --git a/turnstone/console/coordinator_adapter.py b/turnstone/console/coordinator_adapter.py
index f5fc328a..27aa55a7 100644
--- a/turnstone/console/coordinator_adapter.py
+++ b/turnstone/console/coordinator_adapter.py
@@ -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
diff --git a/turnstone/console/server.py b/turnstone/console/server.py
index 263327b0..79b560a4 100644
--- a/turnstone/console/server.py
+++ b/turnstone/console/server.py
@@ -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,
+ )
# ---------------------------------------------------------------------------
diff --git a/turnstone/console/session_factory.py b/turnstone/console/session_factory.py
index 74af1b13..5fb0298a 100644
--- a/turnstone/console/session_factory.py
+++ b/turnstone/console/session_factory.py
@@ -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,
diff --git a/turnstone/console/static/admin.js b/turnstone/console/static/admin.js
index 60fe1427..a898ad91 100644
--- a/turnstone/console/static/admin.js
+++ b/turnstone/console/static/admin.js
@@ -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);
diff --git a/turnstone/console/static/app.js b/turnstone/console/static/app.js
index 802df3b4..85bb167e 100644
--- a/turnstone/console/static/app.js
+++ b/turnstone/console/static/app.js
@@ -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
diff --git a/turnstone/console/static/coordinator/coord-chrome.css b/turnstone/console/static/coordinator/coord-chrome.css
index f96334fd..cbe163e5 100644
--- a/turnstone/console/static/coordinator/coord-chrome.css
+++ b/turnstone/console/static/coordinator/coord-chrome.css
@@ -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 2–4 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;
+ }
+}
diff --git a/turnstone/console/static/coordinator/coordinator.js b/turnstone/console/static/coordinator/coordinator.js
index 6a592c97..d2744a4a 100644
--- a/turnstone/console/static/coordinator/coordinator.js
+++ b/turnstone/console/static/coordinator/coordinator.js
@@ -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");
diff --git a/turnstone/console/static/coordinator/index.html b/turnstone/console/static/coordinator/index.html
index 751a7e28..ba4d7d17 100644
--- a/turnstone/console/static/coordinator/index.html
+++ b/turnstone/console/static/coordinator/index.html
@@ -18,6 +18,7 @@
+
diff --git a/turnstone/console/static/index.html b/turnstone/console/static/index.html
index 63ab2a1d..3388cf49 100644
--- a/turnstone/console/static/index.html
+++ b/turnstone/console/static/index.html
@@ -25,6 +25,7 @@
+
diff --git a/turnstone/core/mcp_client.py b/turnstone/core/mcp_client.py
index f2b972f2..2a3d60da 100644
--- a/turnstone/core/mcp_client.py
+++ b/turnstone/core/mcp_client.py
@@ -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)
diff --git a/turnstone/core/mcp_utils.py b/turnstone/core/mcp_utils.py
new file mode 100644
index 00000000..c8b87377
--- /dev/null
+++ b/turnstone/core/mcp_utils.py
@@ -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))
diff --git a/turnstone/core/session.py b/turnstone/core/session.py
index 38bf7363..b1d66408 100644
--- a/turnstone/core/session.py
+++ b/turnstone/core/session.py
@@ -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,
diff --git a/turnstone/server.py b/turnstone/server.py
index e6f9003c..22be883a 100644
--- a/turnstone/server.py
+++ b/turnstone/server.py
@@ -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:
diff --git a/turnstone/shared_static/interactive.css b/turnstone/shared_static/interactive.css
index 509546b9..cba4604e 100644
--- a/turnstone/shared_static/interactive.css
+++ b/turnstone/shared_static/interactive.css
@@ -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). */
diff --git a/turnstone/shared_static/interactive.js b/turnstone/shared_static/interactive.js
index 4da5987c..a618f887 100644
--- a/turnstone/shared_static/interactive.js
+++ b/turnstone/shared_static/interactive.js
@@ -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.
diff --git a/turnstone/shared_static/mcp_error.css b/turnstone/shared_static/mcp_error.css
new file mode 100644
index 00000000..d234048e
--- /dev/null
+++ b/turnstone/shared_static/mcp_error.css
@@ -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 className→rule 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
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;
+}
diff --git a/turnstone/shared_static/mcp_error.js b/turnstone/shared_static/mcp_error.js
new file mode 100644
index 00000000..b33c0817
--- /dev/null
+++ b/turnstone/shared_static/mcp_error.js
@@ -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;
+}
diff --git a/turnstone/shared_static/redact_credentials.js b/turnstone/shared_static/redact_credentials.js
index dffd01c8..32fc3120 100644
--- a/turnstone/shared_static/redact_credentials.js
+++ b/turnstone/shared_static/redact_credentials.js
@@ -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));
+}
diff --git a/turnstone/tools/read_resource.json b/turnstone/tools/read_resource.json
index e23fcd24..458d7b13 100644
--- a/turnstone/tools/read_resource.json
+++ b/turnstone/tools/read_resource.json
@@ -11,6 +11,8 @@
},
"required": ["uri"]
},
+ "coordinator": true,
+ "interactive": true,
"task_agent": true,
"auto_approve": false,
"primary_key": "uri"
diff --git a/turnstone/tools/use_prompt.json b/turnstone/tools/use_prompt.json
index d250ee7d..7cdd9128 100644
--- a/turnstone/tools/use_prompt.json
+++ b/turnstone/tools/use_prompt.json
@@ -16,6 +16,8 @@
},
"required": ["name"]
},
+ "coordinator": true,
+ "interactive": true,
"task_agent": true,
"auto_approve": false,
"primary_key": "name"
diff --git a/turnstone/ui/static/index.html b/turnstone/ui/static/index.html
index 7e046584..063ad99b 100644
--- a/turnstone/ui/static/index.html
+++ b/turnstone/ui/static/index.html
@@ -22,6 +22,7 @@
+