fix(mcp): give stdio servers the environment, and let builtins reconnect

Two defects that combine to make the built-in browser MCP server
unusable, each of which hides the other.

`_connect_stdio` built `env={**os.environ, **env} if env else None`.
`None` does not mean "inherit the parent environment" — the MCP SDK
substitutes a minimal default one. Callers that pass an env inherit
everything, which is why the Python builtins work: they pass
`builtin_python_env(base_dir)`. The NPX browser server passes nothing,
so it loses the entire container environment, including
PLAYWRIGHT_BROWSERS_PATH. It then looks for browsers in the default
cache and reports `Browser "firefox" is not installed` — with the
browser sitting one directory away, which is what makes this so hard to
place.

Second, a stdio session can disappear without the process dying: the
teardown races across asyncio tasks. `call_tool` returned early on a
missing session, and the existing recovery only ran when a call raised
— which presupposes a session. A missing one was therefore terminal,
even though reconnecting would have fixed it. Reconnection is now
attempted in that case too.

`_reconnect_builtin` also excluded the browser outright. It tested
membership against `_BUILTIN_SERVERS`, the Python-server dict, while
`is_builtin()` counts the NPX servers as builtins as well — so the one
server most likely to need a restart was the one that could never get
one. It now handles both kinds.

Together these mean a browser server that dropped mid-session stayed
down for the rest of the process lifetime, and a fresh one started
without the environment it needs. Verified on Docker/Windows: the
server reconnects on demand and reports its 30 tools, and
`browser_navigate` returns exit_code=0 against a real URL.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Matthieu 2026-07-26 21:22:07 +02:00
parent d8a2059df8
commit 32bfcb162f

View file

@ -190,7 +190,15 @@ class McpManager:
server_params = StdioServerParameters(
command=command,
args=args,
env={**os.environ, **env} if env else None,
# `None` is not "inherit the parent environment" — the MCP SDK
# substitutes a minimal default one. Callers that pass an env
# (the Python builtins, via builtin_python_env) inherit
# everything; callers that pass none lose the whole container
# environment. The NPX browser server is in the second group,
# so it loses PLAYWRIGHT_BROWSERS_PATH and then reports
# `Browser "firefox" is not installed` with the browser sitting
# one directory away.
env={**os.environ, **(env or {})},
)
stack = AsyncExitStack()
@ -477,6 +485,14 @@ class McpManager:
tool_name = parts[2]
session = self._sessions.get(server_id)
if not session and self.is_builtin(server_id):
# A stdio session can disappear without the process dying — the
# teardown races across asyncio tasks. The recovery below only runs
# when a call raises, which presupposes a session, so a missing one
# was terminal even though reconnecting would have fixed it.
logger.warning(f"No session for builtin {server_id}; attempting reconnect")
if await self._reconnect_builtin(server_id):
session = self._sessions.get(server_id)
if not session:
return {"error": f"MCP server not connected: {server_id}", "exit_code": 1}
@ -537,7 +553,30 @@ class McpManager:
async def _reconnect_builtin(self, server_id: str) -> bool:
"""Tear down and reconnect a crashed builtin MCP server."""
import sys
from src.builtin_mcp import _BUILTIN_SERVERS, builtin_python_env
from src.builtin_mcp import (
_BUILTIN_SERVERS, _BUILTIN_NPX_SERVERS, _find_npx, builtin_python_env,
)
# NPX-backed builtins (the browser) are builtins too — is_builtin()
# says so — but this membership test only knew about the Python ones,
# so the browser could never be reconnected.
if server_id in _BUILTIN_NPX_SERVERS:
cfg = _BUILTIN_NPX_SERVERS[server_id]
await self.disconnect_server(server_id)
try:
ok = await self.connect_server(
server_id=server_id,
name=cfg["name"],
transport="stdio",
command=_find_npx(),
args=cfg["args"],
)
if ok:
logger.info(f"Reconnected builtin MCP server: {cfg['name']}")
return ok
except Exception as e:
logger.error(f"Failed to reconnect builtin MCP server {cfg['name']}: {e}")
return False
if server_id not in _BUILTIN_SERVERS:
return False