fix(stream): only warn about MCP servers we configured
The unconnected-server warning fired on everything `init` listed, which includes whatever the host's own claude config carries. On the pi that was seven unrelated connectors parked at `pending` / `needs-auth` forever — seven bogus lines on every turn, burying the one line the warning exists to surface. Scope it to the servers passed in `mcp_servers`. Those are the ones we have an expectation about; the rest are the host's business.
This commit is contained in:
@@ -576,6 +576,7 @@ class ClaudeCodeBackend:
|
||||
proc,
|
||||
include_meta_user=self._opts.include_meta_user,
|
||||
include_partial_messages=self._opts.include_partial_messages,
|
||||
expected_mcp_servers=tuple(self._opts.mcp_servers or ()),
|
||||
on_parse_error=self._on_parse_error,
|
||||
)
|
||||
await tm.start()
|
||||
|
||||
@@ -547,6 +547,7 @@ class StreamTurnManager:
|
||||
*,
|
||||
include_meta_user: bool = False,
|
||||
include_partial_messages: bool = False,
|
||||
expected_mcp_servers: Iterable[str] = (),
|
||||
on_parse_error: Callable[[MessageParseError, dict[str, Any]], None]
|
||||
| None = None,
|
||||
owns_proc: bool = True,
|
||||
@@ -554,6 +555,7 @@ class StreamTurnManager:
|
||||
self._proc = proc
|
||||
self._include_meta_user = include_meta_user
|
||||
self._include_partial_messages = include_partial_messages
|
||||
self._expected_mcp = frozenset(expected_mcp_servers)
|
||||
self._on_parse_error = on_parse_error
|
||||
self._owns_proc = owns_proc
|
||||
self._started = False
|
||||
@@ -749,7 +751,7 @@ class StreamTurnManager:
|
||||
|
||||
if rtype == "system" and record.get("subtype") in _BOOKKEEPING_SUBTYPES:
|
||||
if record.get("subtype") == "init":
|
||||
_warn_on_unconnected_mcp(record, sid)
|
||||
_warn_on_unconnected_mcp(record, sid, self._expected_mcp)
|
||||
continue
|
||||
|
||||
try:
|
||||
@@ -835,8 +837,10 @@ class StreamTurnManager:
|
||||
await self.aclose()
|
||||
|
||||
|
||||
def _warn_on_unconnected_mcp(record: Mapping[str, Any], session_id: str) -> None:
|
||||
"""Log the MCP servers this turn is starting without.
|
||||
def _warn_on_unconnected_mcp(
|
||||
record: Mapping[str, Any], session_id: str, expected: frozenset[str]
|
||||
) -> None:
|
||||
"""Log the MCP servers *we asked for* that this turn is starting without.
|
||||
|
||||
A turn that begins while a server is still ``pending`` runs without
|
||||
that server's tools — silently, from the model's point of view: it
|
||||
@@ -844,19 +848,31 @@ def _warn_on_unconnected_mcp(record: Mapping[str, Any], session_id: str) -> None
|
||||
the failure :meth:`StreamTurnManager.warmup` exists to prevent, so
|
||||
when it happens anyway it should be one grep away rather than a
|
||||
mystery about why the agent "forgot" it had a tool.
|
||||
|
||||
Scoped to `expected` deliberately. The CLI also loads servers from
|
||||
the host's own claude config, and on a box where somebody once
|
||||
logged in interactively that can be a fistful of unrelated
|
||||
connectors sitting at ``pending`` or ``needs-auth`` forever —
|
||||
observed in prod: seven of them, which would have meant seven bogus
|
||||
warnings on every single turn, burying the one line that matters.
|
||||
We only have expectations about servers we configured.
|
||||
"""
|
||||
if not expected:
|
||||
return
|
||||
servers = record.get("mcp_servers")
|
||||
if not isinstance(servers, list):
|
||||
return
|
||||
stragglers = [
|
||||
f"{s.get('name')}:{s.get('status')}"
|
||||
for s in servers
|
||||
if isinstance(s, Mapping) and s.get("status") not in ("connected", None)
|
||||
if isinstance(s, Mapping)
|
||||
and s.get("name") in expected
|
||||
and s.get("status") not in ("connected", None)
|
||||
]
|
||||
if stragglers:
|
||||
_log.warning(
|
||||
"session_id=%s turn starting without %d MCP server(s): %s — "
|
||||
"their tools are unavailable for this turn",
|
||||
"session_id=%s turn starting without %d configured MCP server(s): "
|
||||
"%s — their tools are unavailable for this turn",
|
||||
session_id,
|
||||
len(stragglers),
|
||||
", ".join(stragglers),
|
||||
|
||||
+34
-1
@@ -371,7 +371,7 @@ async def test_unconnected_mcp_servers_are_logged(
|
||||
_result(),
|
||||
]
|
||||
)
|
||||
tm = StreamTurnManager(proc) # type: ignore[arg-type]
|
||||
tm = StreamTurnManager(proc, expected_mcp_servers=("telegram", "firefly")) # type: ignore[arg-type]
|
||||
await tm.start()
|
||||
with caplog.at_level(logging.WARNING, logger="claude_code_api.stream"):
|
||||
await _drain(tm)
|
||||
@@ -379,6 +379,39 @@ async def test_unconnected_mcp_servers_are_logged(
|
||||
assert "firefly" not in caplog.text
|
||||
|
||||
|
||||
async def test_unexpected_mcp_servers_are_not_warned_about(caplog: Any) -> None:
|
||||
"""Servers we didn't configure are the host's business, not ours.
|
||||
|
||||
A box where somebody logged in interactively can carry a pile of
|
||||
unrelated connectors parked at ``needs-auth`` forever. Warning about
|
||||
those every turn would bury the one line that matters.
|
||||
"""
|
||||
import logging
|
||||
|
||||
proc = FakeStreamProcess(
|
||||
[
|
||||
{
|
||||
"type": "system",
|
||||
"subtype": "init",
|
||||
"session_id": "s",
|
||||
"mcp_servers": [
|
||||
{"name": "telegram", "status": "connected"},
|
||||
{"name": "claude.ai Gmail", "status": "needs-auth"},
|
||||
{"name": "claude.ai Drive", "status": "pending"},
|
||||
],
|
||||
},
|
||||
_assistant({"type": "text", "text": "x"}),
|
||||
_result(),
|
||||
]
|
||||
)
|
||||
tm = StreamTurnManager(proc, expected_mcp_servers=("telegram",)) # type: ignore[arg-type]
|
||||
await tm.start()
|
||||
with caplog.at_level(logging.WARNING, logger="claude_code_api.stream"):
|
||||
await _drain(tm)
|
||||
assert "claude.ai" not in caplog.text
|
||||
assert "MCP server" not in caplog.text
|
||||
|
||||
|
||||
# --- failures ------------------------------------------------------------
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user