Merge pull request #3379 from lahuman/fix/3324-windows-mcp-stdio
fix(mcp): avoid WinError 193 for Windows stdio launchers Co-authored-by: lahuman <6156679+lahuman@users.noreply.github.com>
This commit is contained in:
@@ -1,6 +1,8 @@
|
||||
"""MCP client: connects to MCP servers and wraps their tools as native nanobot tools."""
|
||||
|
||||
import asyncio
|
||||
import os
|
||||
import shutil
|
||||
from contextlib import AsyncExitStack
|
||||
from typing import Any
|
||||
|
||||
@@ -24,12 +26,50 @@ _TRANSIENT_EXC_NAMES: frozenset[str] = frozenset((
|
||||
"ConnectionError",
|
||||
))
|
||||
|
||||
_WINDOWS_SHELL_LAUNCHERS: frozenset[str] = frozenset(("npx", "npm", "pnpm", "yarn", "bunx"))
|
||||
|
||||
|
||||
def _is_transient(exc: BaseException) -> bool:
|
||||
"""Check if an exception looks like a transient connection error."""
|
||||
return type(exc).__name__ in _TRANSIENT_EXC_NAMES
|
||||
|
||||
|
||||
def _windows_command_basename(command: str) -> str:
|
||||
"""Return the lowercase basename for a Windows command or path."""
|
||||
return command.replace("\\", "/").rsplit("/", maxsplit=1)[-1].lower()
|
||||
|
||||
|
||||
def _normalize_windows_stdio_command(
|
||||
command: str,
|
||||
args: list[str] | None,
|
||||
env: dict[str, str] | None,
|
||||
) -> tuple[str, list[str], dict[str, str] | None]:
|
||||
"""Wrap Windows shell launchers so MCP stdio servers start reliably."""
|
||||
normalized_args = list(args or [])
|
||||
if os.name != "nt":
|
||||
return command, normalized_args, env
|
||||
|
||||
basename = _windows_command_basename(command)
|
||||
if basename in {"cmd", "cmd.exe", "powershell", "powershell.exe", "pwsh", "pwsh.exe"}:
|
||||
return command, normalized_args, env
|
||||
|
||||
if basename.endswith((".exe", ".com")):
|
||||
return command, normalized_args, env
|
||||
|
||||
resolved = shutil.which(command, path=(env or {}).get("PATH")) or command
|
||||
resolved_basename = _windows_command_basename(resolved)
|
||||
should_wrap = (
|
||||
basename in _WINDOWS_SHELL_LAUNCHERS
|
||||
or basename.endswith((".cmd", ".bat"))
|
||||
or resolved_basename.endswith((".cmd", ".bat"))
|
||||
)
|
||||
if not should_wrap:
|
||||
return command, normalized_args, env
|
||||
|
||||
comspec = (env or {}).get("COMSPEC") or os.environ.get("COMSPEC") or "cmd.exe"
|
||||
return comspec, ["/d", "/c", command, *normalized_args], env
|
||||
|
||||
|
||||
def _extract_nullable_branch(options: Any) -> tuple[dict[str, Any], bool] | None:
|
||||
"""Return the single non-null branch for nullable unions."""
|
||||
if not isinstance(options, list):
|
||||
@@ -416,8 +456,15 @@ async def connect_mcp_servers(
|
||||
return name, None
|
||||
|
||||
if transport_type == "stdio":
|
||||
command, args, env = _normalize_windows_stdio_command(
|
||||
cfg.command,
|
||||
cfg.args,
|
||||
cfg.env or None,
|
||||
)
|
||||
params = StdioServerParameters(
|
||||
command=cfg.command, args=cfg.args, env=cfg.env or None
|
||||
command=command,
|
||||
args=args,
|
||||
env=env,
|
||||
)
|
||||
read, write = await server_stack.enter_async_context(stdio_client(params))
|
||||
elif transport_type == "sse":
|
||||
|
||||
@@ -1,16 +1,18 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
from contextlib import AsyncExitStack, asynccontextmanager
|
||||
import sys
|
||||
from contextlib import asynccontextmanager
|
||||
from types import ModuleType, SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
import nanobot.agent.tools.mcp as mcp_mod
|
||||
from nanobot.agent.tools.mcp import (
|
||||
MCPResourceWrapper,
|
||||
MCPPromptWrapper,
|
||||
MCPResourceWrapper,
|
||||
MCPToolWrapper,
|
||||
_normalize_windows_stdio_command,
|
||||
connect_mcp_servers,
|
||||
)
|
||||
from nanobot.agent.tools.registry import ToolRegistry
|
||||
@@ -178,6 +180,99 @@ def test_wrapper_normalizes_nullable_property_anyof() -> None:
|
||||
}
|
||||
|
||||
|
||||
def test_normalize_windows_stdio_command_is_noop_off_windows(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
monkeypatch.setattr(mcp_mod.os, "name", "posix", raising=False)
|
||||
|
||||
command, args, env = _normalize_windows_stdio_command(
|
||||
"npx",
|
||||
["-y", "chrome-devtools-mcp@latest"],
|
||||
{"FOO": "bar"},
|
||||
)
|
||||
|
||||
assert command == "npx"
|
||||
assert args == ["-y", "chrome-devtools-mcp@latest"]
|
||||
assert env == {"FOO": "bar"}
|
||||
|
||||
|
||||
def test_normalize_windows_stdio_command_wraps_npx_on_windows(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
monkeypatch.setattr(mcp_mod.os, "name", "nt", raising=False)
|
||||
monkeypatch.setattr(
|
||||
mcp_mod.shutil,
|
||||
"which",
|
||||
lambda command, path=None: r"C:\Program Files\nodejs\npx.cmd",
|
||||
)
|
||||
monkeypatch.setenv("COMSPEC", r"C:\Windows\System32\cmd.exe")
|
||||
|
||||
command, args, env = _normalize_windows_stdio_command(
|
||||
"npx",
|
||||
["-y", "chrome-devtools-mcp@latest"],
|
||||
None,
|
||||
)
|
||||
|
||||
assert command == r"C:\Windows\System32\cmd.exe"
|
||||
assert args == ["/d", "/c", "npx", "-y", "chrome-devtools-mcp@latest"]
|
||||
assert env is None
|
||||
|
||||
|
||||
def test_normalize_windows_stdio_command_wraps_resolved_cmd_launcher(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
monkeypatch.setattr(mcp_mod.os, "name", "nt", raising=False)
|
||||
|
||||
def _fake_which(command: str, path: str | None = None) -> str:
|
||||
assert command == "custom-launcher"
|
||||
assert path == r"C:\Tools"
|
||||
return r"C:\Tools\custom-launcher.cmd"
|
||||
|
||||
monkeypatch.setattr(mcp_mod.shutil, "which", _fake_which)
|
||||
monkeypatch.setenv("COMSPEC", r"C:\Windows\System32\cmd.exe")
|
||||
|
||||
command, args, _env = _normalize_windows_stdio_command(
|
||||
"custom-launcher",
|
||||
["serve"],
|
||||
{"PATH": r"C:\Tools"},
|
||||
)
|
||||
|
||||
assert command == r"C:\Windows\System32\cmd.exe"
|
||||
assert args == ["/d", "/c", "custom-launcher", "serve"]
|
||||
|
||||
|
||||
def test_normalize_windows_stdio_command_keeps_real_executables_unchanged(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
monkeypatch.setattr(mcp_mod.os, "name", "nt", raising=False)
|
||||
|
||||
command, args, env = _normalize_windows_stdio_command(
|
||||
"python.exe",
|
||||
["-m", "http.server"],
|
||||
{"FOO": "bar"},
|
||||
)
|
||||
|
||||
assert command == "python.exe"
|
||||
assert args == ["-m", "http.server"]
|
||||
assert env == {"FOO": "bar"}
|
||||
|
||||
|
||||
def test_normalize_windows_stdio_command_skips_existing_shells(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
monkeypatch.setattr(mcp_mod.os, "name", "nt", raising=False)
|
||||
|
||||
command, args, env = _normalize_windows_stdio_command(
|
||||
"cmd.exe",
|
||||
["/c", "echo", "hello"],
|
||||
None,
|
||||
)
|
||||
|
||||
assert command == "cmd.exe"
|
||||
assert args == ["/c", "echo", "hello"]
|
||||
assert env is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_execute_returns_text_blocks() -> None:
|
||||
async def call_tool(_name: str, arguments: dict) -> object:
|
||||
@@ -423,6 +518,48 @@ async def test_connect_mcp_servers_one_failure_does_not_block_others(
|
||||
assert set(stacks) == {"good"}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_connect_mcp_servers_wraps_windows_stdio_launchers(
|
||||
fake_mcp_runtime: dict[str, object | None],
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
fake_mcp_runtime["session"] = _make_fake_session(["demo"])
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
@asynccontextmanager
|
||||
async def _capturing_stdio_client(params: object):
|
||||
captured["command"] = params.command
|
||||
captured["args"] = params.args
|
||||
captured["env"] = params.env
|
||||
yield object(), object()
|
||||
|
||||
monkeypatch.setattr(mcp_mod.os, "name", "nt", raising=False)
|
||||
monkeypatch.setattr(
|
||||
mcp_mod.shutil,
|
||||
"which",
|
||||
lambda command, path=None: r"C:\Program Files\nodejs\npx.cmd",
|
||||
)
|
||||
monkeypatch.setenv("COMSPEC", r"C:\Windows\System32\cmd.exe")
|
||||
monkeypatch.setattr(sys.modules["mcp.client.stdio"], "stdio_client", _capturing_stdio_client)
|
||||
|
||||
registry = ToolRegistry()
|
||||
stacks = await connect_mcp_servers(
|
||||
{
|
||||
"test": MCPServerConfig(
|
||||
command="npx",
|
||||
args=["-y", "chrome-devtools-mcp@latest"],
|
||||
)
|
||||
},
|
||||
registry,
|
||||
)
|
||||
for stack in stacks.values():
|
||||
await stack.aclose()
|
||||
|
||||
assert captured["command"] == r"C:\Windows\System32\cmd.exe"
|
||||
assert captured["args"] == ["/d", "/c", "npx", "-y", "chrome-devtools-mcp@latest"]
|
||||
assert captured["env"] is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# MCPResourceWrapper tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user