diff --git a/docs/get-started/real-host.md b/docs/get-started/real-host.md index 159c1f56f7..17e0e045e8 100644 --- a/docs/get-started/real-host.md +++ b/docs/get-started/real-host.md @@ -165,7 +165,7 @@ Once that command sits and waits, what's left is almost always one of three thin * **A relative path.** The host launches your server from *its* working directory, not the one you registered from. `server.py` where `/absolute/path/to/server.py` is needed is the single most common failure. If the host can't find `uv` either, that path has to be absolute too. * **The host is still running its old config.** Hosts read their config at launch. Claude Desktop in particular has to be *fully quit* (not just its window closed) and reopened before an edit to `claude_desktop_config.json` takes effect. -* **Something reached stdout.** On stdio, stdout *is* the protocol. One stray `print()` and the host reads a corrupt message and drops the connection. Log with the `logging` module, which writes to stderr. **[Logging](../handlers/logging.md)** has the whole story. +* **Something reached stdout before serving began.** On stdio, stdout *is* the protocol. The SDK diverts stray output to stderr while serving, but anything that reaches stdout before then -- a wrapper script echoing, an import-time `print()` in an unbuffered process -- hands the host a corrupt message and it drops the connection. Log with the `logging` module, which writes to stderr. **[Logging](../handlers/logging.md)** has the whole story. Claude Desktop keeps a log per server: `mcp-server-.log` is your server's stderr, next to `mcp.log` for connections, under `~/Library/Logs/Claude` on macOS and `%APPDATA%\Claude\logs` on Windows. diff --git a/docs/handlers/logging.md b/docs/handlers/logging.md index 945aa60d5e..c53aa79e47 100644 --- a/docs/handlers/logging.md +++ b/docs/handlers/logging.md @@ -33,8 +33,9 @@ For a **stdio** server this question matters more than usual. The host launched The standard library already does the right thing: log output goes to `sys.stderr` by default. Your `logger.info(...)` lines land in the terminal (or wherever the host collects the subprocess's stderr), and the protocol stream stays clean. !!! tip - Never `print()` in a stdio server. `print` writes to **stdout**, and stdout *is* the wire: one stray - line and the client is trying to parse it as JSON-RPC. + Don't `print()` in a stdio server. `print` writes to **stdout**, and stdout belongs to the protocol: + while serving, the SDK diverts stray stdout to stderr so it can't corrupt the wire, but that leaves + your line interleaved raw among the log output -- no level, no logger name, no way to filter it. `logger.debug("got here")` is the same one line of effort and goes to the right place. @@ -72,7 +73,7 @@ went to standard error: the terminal, not the wire. * The MCP protocol's logging capability is deprecated by the 2026-07-28 spec and not replaced. Don't build on it. * `logger = logging.getLogger(__name__)` at module level, `logger.info(...)` in the tool. That's the whole pattern. * Log output never reaches the model. Only the value you `return` does. -* Standard error is yours; stdout belongs to the protocol. Never `print()` in a stdio server. +* Standard error is yours; stdout belongs to the protocol. The SDK diverts a stray `print()` to stderr while serving, but it arrives unlabeled -- use `logging`. * `MCPServer(..., log_level="DEBUG")` sets the level, and a logging configuration you made first is left alone. Telling connected clients that something on your server changed (the tool list, a resource) is **[Subscriptions](subscriptions.md)**. diff --git a/docs/migration.md b/docs/migration.md index 876608db70..24ca788930 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -1855,6 +1855,41 @@ group (spawned with `start_new_session=True`); the `getpgid()` lookup and the per-process terminate/kill fallback are gone. The win32 utilities logger is now named `mcp.os.win32.utilities` (was `client.stdio.win32`). +### `stdio_server` keeps the protocol streams on private descriptors + +While serving on the process's real stdin and stdout, the stdio server transport now +duplicates each protocol pipe to a private descriptor and points the standard +descriptors — with their Windows standard handles — away from the wire, restoring +both when the transport exits: fd 0 reads the null device, and fd 1 writes to stderr +(the null device if stderr is unusable). Subprocesses started by handler code +therefore inherit the diversions instead of the protocol pipes, and a stray +`print()` lands on stderr rather than the wire. (The claim is best-effort: in the +rare process whose descriptor table cannot be rearranged, the transport serves in +place, exactly as v1 did.) + +In v1 a child inheriting the pipes could consume protocol bytes or corrupt the +outgoing stream with its own output, and on Windows it hung inside interpreter +startup — behind the transport's pending read on the shared pipe +([CPython gh-78961](https://github.com/python/cpython/issues/78961)) — until +the next request arrived: the +[#671](https://github.com/modelcontextprotocol/python-sdk/issues/671) hang, for +which passing `stdin=subprocess.DEVNULL` on every spawn (and capturing the child's +stdout) was the required workaround. On v2 the workaround is no longer needed, for +any spawn API, redirected or not. + +To migrate: nothing, unless handler code used the standard streams directly during a +stdio session. Reading `sys.stdin` (or calling `input()`) now sees end-of-file +instead of racing the transport for protocol bytes; there was never a meaningful +value to read there. `print()` and other `sys.stdout` writes reach stderr instead of +corrupting the wire — code that deliberately wrote protocol frames to `sys.stdout` +must send them through the transport's write stream instead. A child that streams +a lot of output to its inherited stdout now streams it into the client's stderr +channel; capture output you don't want in the client's logs. Likewise, anything +that read ahead from `sys.stdin` before the server started keeps those bytes; they +no longer reach the transport (they never reliably did). Passing an explicit `stdin=`/`stdout=` stream to +`stdio_server(...)` skips the descriptor changes for that stream, as does any +environment where the sys stream is not backed by the process's real descriptor. + ### WebSocket transport removed The WebSocket transport has been removed: `mcp.client.websocket.websocket_client`, `mcp.server.websocket.websocket_server`, and the `ws` optional dependency extra (`mcp[ws]`) no longer exist. WebSocket was never part of the MCP specification. Use the streamable HTTP transport instead (`mcp.client.streamable_http.streamable_http_client` on the client, `streamable_http_app()` on the server), which supports bidirectional communication with server-to-client streaming over standard HTTP. diff --git a/docs/run/index.md b/docs/run/index.md index f92090fee4..7757fc888a 100644 --- a/docs/run/index.md +++ b/docs/run/index.md @@ -39,31 +39,7 @@ python server.py Nothing prints, and it doesn't return. It is waiting on stdin for a host to speak first. -That also means stdout **is the wire**. A stray `print()` corrupts the stream; the `logging` module writes to stderr and is the right tool. That story is in **[Logging](../handlers/logging.md)**. - -On Windows, the same rule applies to child processes your tools start. A child -that inherits the stdio server's stdin can block behind the server's protocol -reader. If your tool starts a subprocess and you do not intend to feed it input, -redirect the child's stdin: - -```python -import asyncio -import subprocess -import sys - - -async def run_script() -> tuple[bytes, bytes]: - process = await asyncio.create_subprocess_exec( - sys.executable, - "script.py", - stdin=subprocess.DEVNULL, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - ) - return await process.communicate() -``` - -The matching troubleshooting entry is **[My stdio tool hangs when it starts a subprocess on Windows](../troubleshooting.md#my-stdio-tool-hangs-when-it-starts-a-subprocess-on-windows)**. +That also means stdout **is the wire**. While serving, the SDK moves the wire to a private descriptor and diverts stray output -- a `print()`, a subprocess writing to its inherited stdout -- to stderr, where it can't corrupt the stream. Output that reaches stdout *before* serving begins (a wrapper script echoing, an unbuffered import-time print) still lands on the wire. For output you actually want, the `logging` module is the right tool. That story is in **[Logging](../handlers/logging.md)**. ### Try it diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 1279448359..e6613a2900 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -137,45 +137,10 @@ There is no error string for this, which is exactly why it is hard to search. Th * **Is the tool on the `mcp` the host is running?** A second `MCPServer(...)` in another module is a different, empty server. Check which object the host's command actually imports. * **Did two tools share a name?** Then one of them is gone. Look for `Tool already exists:` in the server log. * **Is the host's list stale?** Adding a tool after startup only reaches clients that handle `notifications/tools/list_changed`. Restarting the host is the blunt fix. -* **Did something write to `stdout`?** On a stdio transport, stdout *is* the protocol: one stray `print()` and the host drops the connection, which some hosts render as a server with nothing in it. Log with the `logging` module instead. The rest of the host-side checklist is on **[Connect to a real host](get-started/real-host.md)**. +* **Did something write to `stdout` before the server started serving?** While serving, the SDK diverts stray stdout to stderr (best-effort: an environment that replaces `sys.stdout`, or merges stderr into stdout on Windows, is served as-is), but output flushed to stdout earlier -- a wrapper script echoing, an import-time `print()` in an unbuffered process -- lands on the protocol stream, and one junk line can make the host drop the connection, which some hosts render as a server with nothing in it. Log with the `logging` module instead. The rest of the host-side checklist is on **[Connect to a real host](get-started/real-host.md)**. An "invalid" tool name is *not* on that list: a non-conforming name logs a warning but the tool is registered and listed anyway. -## My stdio tool hangs when it starts a subprocess on Windows - -Your server is running over `stdio`, and a tool starts another process with -`asyncio.create_subprocess_exec`, `asyncio.create_subprocess_shell`, or -`subprocess.Popen`. The tool call never returns on Windows, while the same code -works over an HTTP transport. - -The child inherited the server's stdin. In a stdio server, stdin is the protocol -pipe and the server is already waiting on it for the next JSON-RPC message. A -Python child process on Windows can block during startup when it inherits that -same pipe. - -If you do not intend to send input to the child, redirect its stdin: - -```python -import asyncio -import subprocess -import sys - - -async def run_script() -> tuple[bytes, bytes]: - process = await asyncio.create_subprocess_exec( - sys.executable, - "script.py", - stdin=subprocess.DEVNULL, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - ) - return await process.communicate() -``` - -Use the same idea with `subprocess.Popen(..., stdin=subprocess.DEVNULL)`. Also -capture or redirect the child's stdout. The stdio server's stdout is the MCP -wire, so a child that writes there can corrupt the connection. - ## `MCPError: Server returned an error response` The server refused the HTTP request outright, with a body that is not JSON-RPC, so the python `Client` has nothing better to show you than this stand-in. diff --git a/src/mcp/os/posix/utilities.py b/src/mcp/os/posix/utilities.py index d15be17194..999fa3b757 100644 --- a/src/mcp/os/posix/utilities.py +++ b/src/mcp/os/posix/utilities.py @@ -1,8 +1,9 @@ -"""POSIX-specific functionality for stdio client operations.""" +"""POSIX-specific functionality for stdio transport operations.""" import logging import os import signal +import sys from contextlib import suppress import anyio @@ -10,6 +11,21 @@ logger = logging.getLogger(__name__) + +def same_open_file(fd_a: int, fd_b: int) -> bool: + """Whether two descriptors refer to the same open file. + + False on Windows - anonymous pipe handles carry no identity to compare - + and False when either descriptor cannot be interrogated. + """ + if sys.platform == "win32": + return False + try: + return os.path.sameopenfile(fd_a, fd_b) + except OSError: + return False + + # How often to probe for surviving group members between SIGTERM and SIGKILL. _GROUP_POLL_INTERVAL = 0.01 diff --git a/src/mcp/os/win32/utilities.py b/src/mcp/os/win32/utilities.py index 1cc867d4fa..39df0de050 100644 --- a/src/mcp/os/win32/utilities.py +++ b/src/mcp/os/win32/utilities.py @@ -1,4 +1,4 @@ -"""Windows-specific functionality for stdio client operations.""" +"""Windows-specific functionality for stdio transport operations.""" import logging import shutil @@ -20,14 +20,39 @@ import pywintypes import win32api import win32con + import win32file import win32job else: # Type stubs for non-Windows platforms win32api = None win32con = None + win32file = None win32job = None pywintypes = None + +def rebind_std_handle_to_fd(fd: int) -> None: + """Points the Win32 standard-handle slot for fd 0, 1, or 2 at fd's current OS handle. + + os.dup2 updates only the C runtime's descriptor table; anything that resolves + GetStdHandle — subprocess standard-handle inheritance, native code — reads the + Win32 slot, so after retargeting a standard descriptor the slot must be + repointed too. + + Raises: + OSError: The slot could not be set; callers treat descriptor + rearrangement as best-effort and fall back on failure. + """ + if sys.platform != "win32" or not win32api or not win32file or not pywintypes: + return + std_ids = {0: win32api.STD_INPUT_HANDLE, 1: win32api.STD_OUTPUT_HANDLE, 2: win32api.STD_ERROR_HANDLE} + try: + win32api.SetStdHandle(std_ids[fd], win32file._get_osfhandle(fd)) + except pywintypes.error as exc: + # Normalized so callers' OSError-based best-effort handling covers it. + raise OSError(f"SetStdHandle failed for fd {fd}") from exc + + # How often FallbackProcess polls the underlying Popen for exit. _EXIT_POLL_INTERVAL = 0.01 diff --git a/src/mcp/server/stdio.py b/src/mcp/server/stdio.py index 876d256ddb..ad430309fa 100644 --- a/src/mcp/server/stdio.py +++ b/src/mcp/server/stdio.py @@ -17,61 +17,235 @@ async def run_server(): ``` """ +import os import sys -from contextlib import asynccontextmanager +import threading +from collections.abc import Callable +from contextlib import asynccontextmanager, suppress from io import TextIOWrapper +from typing import BinaryIO, Literal, TextIO import anyio import anyio.lowlevel import mcp_types as types +from mcp.os.posix.utilities import same_open_file +from mcp.os.win32.utilities import rebind_std_handle_to_fd from mcp.shared._context_streams import create_context_streams from mcp.shared.message import SessionMessage +# Descriptors a transport in this process currently has pointed away from +# their protocol pipes. A second concurrent stdio_server() must not claim an +# fd again: it would duplicate the diversion target instead of the protocol +# pipe, and its restore would clobber the first transport's. The lock makes +# the check-and-claim atomic for embedders running transports on threads. +_claimed: set[int] = set() +_claimed_lock = threading.Lock() + + +def _is_backed_by_fd(stream: TextIO, fd: int) -> bool: + """Whether stream is a text wrapper over the process's real descriptor fd.""" + try: + return stream.buffer.fileno() == fd + except (AttributeError, OSError, ValueError): + # In-memory or injected streams (tests, embedders) have no usable + # descriptor; io.UnsupportedOperation is a subclass of OSError/ValueError. + return False + + +def _dup_above_std(fd: int) -> int: + """Duplicates fd onto a descriptor above the standard range (fd > 2). + + os.dup hands out the lowest free slot, so in a process started with a + standard descriptor closed the private wire duplicate would itself become + fd 0, 1, or 2 - handed to children as a standard stream, and, when it + lands on fd 2, silently made the target of the stdout diversion. + + Raises: + OSError: propagated from os.dup; nothing is leaked into the standard + range - every duplicate made so far is closed first. + """ + duplicates = [os.dup(fd)] + try: + while duplicates[-1] <= 2: + duplicates.append(os.dup(fd)) + except OSError: + for duplicate in duplicates: + os.close(duplicate) + raise + for below_std in duplicates[:-1]: + os.close(below_std) + return duplicates[-1] + + +def _open_stdin_diversion() -> int: + """What fd 0 reads while claimed: the null device, so readers see EOF.""" + return os.open(os.devnull, os.O_RDONLY) + + +def _open_stdout_diversion() -> int: + """What fd 1 receives while claimed: stderr, where stray output is at least + visible in the client's logs, or the null device when stderr is unusable + or is itself the wire (stderr merged into stdout, the 2>&1 launch shape - + detectable on POSIX; on Windows such a merge keeps its v1 behavior).""" + if not same_open_file(2, 1): + try: + return os.dup(2) + except OSError: + pass + return os.open(os.devnull, os.O_WRONLY) + + +def _claim_fd( + fd: int, stream: TextIO, mode: Literal["rb", "wb"], open_diversion: Callable[[], int] +) -> tuple[BinaryIO, Callable[[], None] | None]: + """Returns the binary stream the transport uses for the protocol, and its undo. + + When stream is the process's real standard stream, moves the protocol pipe + to a private descriptor and points fd (and, on Windows, the matching + standard handle) at the diversion for the transport's lifetime, so handler + code and its child processes touch the diversion instead of the protocol + pipe; stdio_server's docstring describes the resulting behavior. + + The claim is best-effort: when the descriptors cannot be rearranged the + transport serves the sys stream's buffer in place, exactly as before + isolation existed, and when a transport in this process already claimed + fd it serves a fresh buffered view of wherever fd currently points. In + both cases the undo callback is None. A stream with no real descriptor + is served through its own buffer. + """ + if not _is_backed_by_fd(stream, fd): + return stream.buffer, None + # Claimed before touching the descriptor table so a second transport + # entering mid-claim serves in place instead of duplicating a half-moved + # descriptor. + with _claimed_lock: + already_claimed = fd in _claimed + if not already_claimed: + _claimed.add(fd) + if already_claimed: + # An enclosing transport owns the claim. Serve wherever fd currently + # points - the diversion - through a fresh view rather than the sys + # stream's buffer: its cached seekability describes the pre-claim + # target, and interrogating it can fail on the retargeted descriptor. + return open(fd, mode, closefd=False), None + private_fd = None + try: + private_fd = _dup_above_std(fd) + diversion_fd = open_diversion() + try: + os.dup2(diversion_fd, fd) + finally: + os.close(diversion_fd) + if sys.platform == "win32": # pragma: no cover + rebind_std_handle_to_fd(fd) + except OSError: + with _claimed_lock: + _claimed.discard(fd) + if private_fd is not None: + # A completed dup2 is undone; an untouched fd is re-pointed at + # the same pipe it already holds, which is harmless. + _restore_fd(fd, private_fd) + os.close(private_fd) + # fd still holds the protocol pipe, so the sys stream's buffer is + # target-consistent: serve it in place, exactly as v1 did - shared + # write ordering, no new descriptors to allocate in a process whose + # descriptor table is already failing. + return stream.buffer, None + + def restore() -> None: + # Flush first: text buffered in the sys stream during the claim (a + # stray print() while stdout is claimed) drains to the diversion, not + # to the restored protocol pipe. + with suppress(OSError, ValueError): + stream.flush() + _restore_fd(fd, private_fd) + with _claimed_lock: + _claimed.discard(fd) + + # closefd=False: an I/O call may sit blocked on this descriptor in a + # worker thread past the transport's lifetime, so garbage collection of + # the wrapper must never close (and free for reuse) the fd under it. The + # private descriptor is deliberately left open for the same reason - one + # descriptor per stream per session, restore() only points fd back at it. + return os.fdopen(private_fd, mode, closefd=False), restore + + +def _restore_fd(fd: int, private_fd: int) -> None: + """Points fd back at the protocol stream the transport claimed. + + Best-effort: a failure must never mask whatever ended the transport, so it + is swallowed rather than raised out of stdio_server's finally. + """ + with suppress(OSError): + os.dup2(private_fd, fd) + if sys.platform == "win32": # pragma: no cover + rebind_std_handle_to_fd(fd) + @asynccontextmanager async def stdio_server(stdin: anyio.AsyncFile[str] | None = None, stdout: anyio.AsyncFile[str] | None = None): """Server transport for stdio: this communicates with an MCP client by reading from the current process' stdin and writing to stdout. + + While serving on the process's real stdin and stdout, the transport claims + them: each protocol pipe moves to a private descriptor, fd 0 (with the + Windows standard input handle) reads the null device, and fd 1 (with the + standard output handle) is diverted to stderr — the null device if stderr + is unusable. Handler code and the child processes it spawns can therefore + neither consume protocol bytes nor corrupt the outgoing stream: reads see + end-of-file, and stray writes (a `print()`, a child's inherited stdout) + land on stderr. Both descriptors are restored when the context exits. + Passing an explicit stream skips the claim for that side. """ # Purposely not using context managers for these, as we don't want to close # standard process handles. Encoding of stdin/stdout as text streams on # python is platform-dependent (Windows is particularly problematic), so we # re-wrap the underlying binary stream to ensure UTF-8. - if not stdin: - stdin = anyio.wrap_file(TextIOWrapper(sys.stdin.buffer, encoding="utf-8", errors="replace")) - if not stdout: - stdout = anyio.wrap_file(TextIOWrapper(sys.stdout.buffer, encoding="utf-8")) + restore_stdin: Callable[[], None] | None = None + restore_stdout: Callable[[], None] | None = None + try: + if not stdin: + stdin_buffer, restore_stdin = _claim_fd(0, sys.stdin, "rb", _open_stdin_diversion) + stdin = anyio.wrap_file(TextIOWrapper(stdin_buffer, encoding="utf-8", errors="replace")) + if not stdout: + stdout_buffer, restore_stdout = _claim_fd(1, sys.stdout, "wb", _open_stdout_diversion) + stdout = anyio.wrap_file(TextIOWrapper(stdout_buffer, encoding="utf-8")) - read_stream_writer, read_stream = create_context_streams[SessionMessage | Exception](0) - write_stream, write_stream_reader = create_context_streams[SessionMessage](0) + read_stream_writer, read_stream = create_context_streams[SessionMessage | Exception](0) + write_stream, write_stream_reader = create_context_streams[SessionMessage](0) - async def stdin_reader(): - try: - async with read_stream_writer: - async for line in stdin: - try: - message = types.jsonrpc_message_adapter.validate_json(line, by_name=False) - except Exception as exc: - await read_stream_writer.send(exc) - continue - - session_message = SessionMessage(message) - await read_stream_writer.send(session_message) - except anyio.ClosedResourceError: # pragma: no cover - await anyio.lowlevel.checkpoint() - - async def stdout_writer(): - try: - async with write_stream_reader: - async for session_message in write_stream_reader: - json = session_message.message.model_dump_json(by_alias=True, exclude_unset=True) - await stdout.write(json + "\n") - await stdout.flush() - except anyio.ClosedResourceError: # pragma: no cover - await anyio.lowlevel.checkpoint() - - async with anyio.create_task_group() as tg: - tg.start_soon(stdin_reader) - tg.start_soon(stdout_writer) - yield read_stream, write_stream + async def stdin_reader(): + try: + async with read_stream_writer: + async for line in stdin: + try: + message = types.jsonrpc_message_adapter.validate_json(line, by_name=False) + except Exception as exc: + await read_stream_writer.send(exc) + continue + + session_message = SessionMessage(message) + await read_stream_writer.send(session_message) + except anyio.ClosedResourceError: # pragma: no cover + await anyio.lowlevel.checkpoint() + + async def stdout_writer(): + try: + async with write_stream_reader: + async for session_message in write_stream_reader: + json = session_message.message.model_dump_json(by_alias=True, exclude_unset=True) + await stdout.write(json + "\n") + await stdout.flush() + except anyio.ClosedResourceError: # pragma: no cover + await anyio.lowlevel.checkpoint() + + async with anyio.create_task_group() as tg: + tg.start_soon(stdin_reader) + tg.start_soon(stdout_writer) + yield read_stream, write_stream + finally: + if restore_stdout is not None: + restore_stdout() + if restore_stdin is not None: + restore_stdin() diff --git a/tests/interaction/_requirements.py b/tests/interaction/_requirements.py index 752c17aa4f..98e0116f6b 100644 --- a/tests/interaction/_requirements.py +++ b/tests/interaction/_requirements.py @@ -3916,9 +3916,12 @@ def __post_init__(self) -> None: note="Only observable over stdio: stdin/stdout purity is stdio-specific.", divergence=Divergence( note=( - "stdio_server's own writes satisfy this, but it does not redirect or guard sys.stdout: " - "handler code that calls print() writes directly to the protocol stream and corrupts the " - "framing. The spec MUST is satisfied only as long as application code behaves." + "While serving, stdio_server moves the wire to private descriptors and diverts fd 0/1, so " + "handler code and its child processes can neither read protocol bytes nor write into the " + "stream (pinned by tests/server/test_stdio.py). Remaining gaps: output flushed to stdout " + "before the transport enters can still precede the first frame; the claim is best-effort " + "and skipped for explicitly injected streams; and a stderr merged into stdout (2>&1) is " + "detected and neutralized only on POSIX - Windows pipe handles carry no identity." ), ), ), diff --git a/tests/interaction/transports/test_stdio.py b/tests/interaction/transports/test_stdio.py index dbb7de3459..2c34f5a0bf 100644 --- a/tests/interaction/transports/test_stdio.py +++ b/tests/interaction/transports/test_stdio.py @@ -113,7 +113,8 @@ async def test_stdio_server_writes_one_jsonrpc_message_per_line() -> None: """Every `stdio_server` write is one valid JSON-RPC message on its own line. Each line is newline-terminated with payload newlines JSON-escaped. This proves the - transport's own framing; it does not guard `sys.stdout` against handler code (see the + transport's own framing over injected streams; the descriptor-level guard that keeps + handler code off the wire is pinned by tests/server/test_stdio.py (see the narrowed divergence on `transport:stdio:stream-purity`). """ captured = io.StringIO() diff --git a/tests/server/test_stdio.py b/tests/server/test_stdio.py index 218e34d5ac..73d39797fa 100644 --- a/tests/server/test_stdio.py +++ b/tests/server/test_stdio.py @@ -1,11 +1,13 @@ import io +import os import sys import threading -from collections.abc import AsyncIterator -from contextlib import asynccontextmanager +from collections.abc import AsyncIterator, Iterator +from contextlib import asynccontextmanager, contextmanager from io import TextIOWrapper import anyio +import anyio.to_thread import pytest from mcp_types import ( CLIENT_CAPABILITIES_META_KEY, @@ -105,6 +107,597 @@ async def test_stdio_server_invalid_utf8(monkeypatch: pytest.MonkeyPatch) -> Non assert second.message == valid +@contextmanager +def _pipe_planted_on_fd0(monkeypatch: pytest.MonkeyPatch) -> Iterator[tuple[int, int]]: + """Plants a fresh pipe on the process's fd 0 and rebinds sys.stdin over it. + + Yields the pipe's (read_fd, write_fd). The caller owns the write end and + must close it exactly once (every test does, to EOF the transport); the + helper closes only what it created and still owns, so no descriptor is + ever closed twice - a second close lands on a recycled fd number and + destroys whatever unrelated file now lives there (pytest's capture files, + in practice). Captures the real os.dup2 up front so teardown restores + fd 0 even for tests that monkeypatch descriptor calls. + """ + real_dup2 = os.dup2 + in_r, in_w = os.pipe() + saved0 = os.dup(0) + stdin_double = TextIOWrapper(open(0, "rb", closefd=False), encoding="utf-8") + try: + os.dup2(in_r, 0) + monkeypatch.setattr(sys, "stdin", stdin_double) + yield in_r, in_w + finally: + # Closed while its descriptor is still valid, so a later garbage + # collection never does I/O on a recycled fd. + stdin_double.close() + real_dup2(saved0, 0) + os.close(saved0) + os.close(in_r) + + +@contextmanager +def _pipe_planted_on_fd1(monkeypatch: pytest.MonkeyPatch) -> Iterator[tuple[int, int]]: + """Plants a fresh pipe on the process's fd 1 and rebinds sys.stdout over it. + + Yields the pipe's (read_fd, write_fd); the test observes the wire through + the read end. The helper owns every descriptor it creates and closes each + exactly once (see _pipe_planted_on_fd0 for why double-closing is + destructive), closing the sys.stdout double first, while its descriptor is + still valid. Captures the real os.dup2 up front so teardown restores fd 1 + even for tests that monkeypatch descriptor calls. + """ + real_dup2 = os.dup2 + out_r, out_w = os.pipe() + saved1 = os.dup(1) + stdout_double = TextIOWrapper(open(1, "wb", closefd=False), encoding="utf-8") + try: + os.dup2(out_w, 1) + monkeypatch.setattr(sys, "stdout", stdout_double) + yield out_r, out_w + finally: + stdout_double.close() + real_dup2(saved1, 1) + os.close(saved1) + os.close(out_r) + os.close(out_w) + + +@contextmanager +def _pipe_planted_on_fd2() -> Iterator[int]: + """Plants a fresh pipe on the process's fd 2 to observe the stdout diversion. + + Yields the pipe's read end. Same exactly-once ownership rules as the other + planting helpers; nothing rebinds sys.stderr because the transport never + touches it - the diversion duplicates fd 2 directly. + """ + real_dup2 = os.dup2 + err_r, err_w = os.pipe() + saved2 = os.dup(2) + try: + os.dup2(err_w, 2) + yield err_r + finally: + real_dup2(saved2, 2) + os.close(saved2) + os.close(err_r) + os.close(err_w) + + +def _frame(message: JSONRPCRequest | JSONRPCResponse) -> bytes: + """One JSON-RPC message as the newline-terminated wire line the transport reads.""" + return (message.model_dump_json(by_alias=True, exclude_none=True) + "\n").encode() + + +async def _read_from(fd: int) -> bytes: + """One os.read from fd, in a worker thread. + + A regression can leave the target pipe empty, and a blocked read on the + loop thread would outlive fail_after; abandoning turns that into a red + TimeoutError instead. + """ + return await anyio.to_thread.run_sync(os.read, fd, 65536, abandon_on_cancel=True) + + +@pytest.mark.anyio +async def test_stdio_server_takes_stdin_off_the_descriptor_table_while_serving( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """On the real process stdin, the transport claims the protocol pipe and releases it on exit. + + SDK-defined behavior: while serving, fd 0 is the null device, so a child + process that inherits it cannot consume protocol bytes and, on Windows, + cannot hang at interpreter startup behind the transport's pending read + (CPython gh-78961). The protocol still flows over the original pipe, and on + exit fd 0 points back at it. Raw pipes are the subject here: the property + under test is what the process's descriptor table looks like while serving. + """ + with _pipe_planted_on_fd0(monkeypatch) as (in_r, in_w): + out_r, out_w = os.pipe() # captures responses via the sys.stdout double + stdout_double = TextIOWrapper(open(out_w, "wb", closefd=False), encoding="utf-8") + try: + monkeypatch.setattr(sys, "stdout", stdout_double) + + request = JSONRPCRequest(jsonrpc="2.0", id=1, method="ping") + response = JSONRPCResponse(jsonrpc="2.0", id=1, result={}) + with anyio.fail_after(5): + async with stdio_server() as (read_stream, write_stream): + async with read_stream: + # fd 0 is the null device: instant EOF instead of protocol bytes. In a + # worker thread because a regression would leave fd 0 on the (empty) + # protocol pipe, and a blocked read on the loop thread would outlive + # fail_after; abandoning turns that into a red TimeoutError instead. + assert await anyio.to_thread.run_sync(os.read, 0, 1, abandon_on_cancel=True) == b"" + + # The protocol still flows over the original pipe. + os.write(in_w, _frame(request)) + received = await read_stream.receive() + assert isinstance(received, SessionMessage) + assert received.message == request + + await write_stream.send(SessionMessage(response)) + # Worker thread + abandon for the same reason as the fd 0 read above. + line = await _read_from(out_r) + assert jsonrpc_message_adapter.validate_json(line.decode().strip()) == response + + os.close(in_w) # EOF lets the reader finish so the context can exit + await write_stream.aclose() + + # Restored: fd 0 is a handle to the protocol pipe again, not the null + # device. samestat degrades to a trivially-true comparison for pipes on + # Windows (st_dev/st_ino are 0 there); the POSIX legs carry the assertion. + assert os.path.sameopenfile(0, in_r) + finally: + stdout_double.close() + os.close(out_r) + os.close(out_w) + + +@pytest.mark.anyio +async def test_a_nested_stdio_server_does_not_clobber_the_first_transports_claim( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Only the first default-stream transport claims stdin; a nested one serves in place. + + SDK-defined behavior: a second stdio_server() entered while the first is + serving must not re-claim fd 0 (it would duplicate the null device and its + restore would clobber the first transport's). The inner transport reads the + in-place stdin - the null device while the outer one serves - and sees + immediate EOF; after both exit, fd 0 is back on the protocol pipe. + """ + with _pipe_planted_on_fd0(monkeypatch) as (in_r, in_w): + monkeypatch.setattr(sys, "stdout", TextIOWrapper(io.BytesIO(), encoding="utf-8")) + + request = JSONRPCRequest(jsonrpc="2.0", id=1, method="ping") + with anyio.fail_after(5): + async with stdio_server() as (outer_read, outer_write): + async with outer_read: + async with stdio_server() as (inner_read, inner_write): + async with inner_read: + with pytest.raises(anyio.EndOfStream): + await inner_read.receive() + await inner_write.aclose() + + # The outer transport still owns the real pipe. + os.write(in_w, _frame(request)) + received = await outer_read.receive() + assert isinstance(received, SessionMessage) + assert received.message == request + + os.close(in_w) + await outer_write.aclose() + + assert os.path.sameopenfile(0, in_r) + + +@pytest.mark.anyio +@pytest.mark.parametrize("failing_call", ["dup", "dup2"]) +async def test_stdio_server_reads_stdin_in_place_when_descriptor_isolation_fails( + failing_call: str, monkeypatch: pytest.MonkeyPatch +) -> None: + """A descriptor-table failure while claiming stdin degrades to reading sys.stdin in place. + + SDK-defined behavior: isolation is best-effort; when duplicating fd 0 or + retargeting it fails, the transport serves over the original stdin exactly + as it did before isolation existed - observable as fd 0 still being the + protocol pipe while serving. The dup2 variant fails after the private + duplicate exists, covering its cleanup path. + """ + request = JSONRPCRequest(jsonrpc="2.0", id=1, method="ping") + with _pipe_planted_on_fd0(monkeypatch) as (in_r, in_w): + os.write(in_w, _frame(request)) + os.close(in_w) # EOF after the one frame lets the reader finish + monkeypatch.setattr(sys, "stdout", TextIOWrapper(io.BytesIO(), encoding="utf-8")) + + # Injections fire exactly once, at the transport's own call, then pass + # through: pytest's capture machinery also calls os.dup/os.dup2 at + # phase transitions, and a still-armed injector detonating there + # corrupts capture for every later test in the process. + if failing_call == "dup": + real_dup = os.dup + armed = [True] + + def failing_dup(fd: int) -> int: + if fd == 0 and armed[0]: + armed[0] = False + raise OSError("injected descriptor failure") + return real_dup(fd) + + monkeypatch.setattr(os, "dup", failing_dup) + else: + real_dup2 = os.dup2 + armed = [True] + + def failing_dup2(fd: int, fd2: int, inheritable: bool = True) -> int: + if armed[0]: + armed[0] = False + raise OSError("injected descriptor failure") + return real_dup2(fd, fd2, inheritable) + + monkeypatch.setattr(os, "dup2", failing_dup2) + + with anyio.fail_after(5): + async with stdio_server() as (read_stream, write_stream): # pragma: no branch + async with read_stream: # pragma: no branch + # Isolation was skipped: fd 0 is still the protocol pipe, + # not the null device. (samestat degrades to trivially-true + # for pipes on Windows; the POSIX legs carry the assertion.) + assert os.path.sameopenfile(0, in_r) + # The spent injector passes calls through untouched. + os.close(os.dup(0)) + received = await read_stream.receive() + assert isinstance(received, SessionMessage) + assert received.message == request + await write_stream.aclose() + + +@pytest.mark.anyio +async def test_stdio_server_exits_cleanly_when_the_stdin_restore_fails( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A failed fd 0 restore on exit is swallowed, not raised. + + SDK-defined behavior: the restore in stdio_server's finally must never mask + the exception (or clean exit) that ended the transport. The context exits + normally; the only trace is fd 0 remaining on the null device. + """ + request = JSONRPCRequest(jsonrpc="2.0", id=1, method="ping") + with _pipe_planted_on_fd0(monkeypatch) as (_, in_w): + os.write(in_w, _frame(request)) + os.close(in_w) + monkeypatch.setattr(sys, "stdout", TextIOWrapper(io.BytesIO(), encoding="utf-8")) + + # The claim's dup2 (first call) must succeed and only the restore's + # dup2 (second call) fails; later calls pass through because pytest's + # capture machinery also uses os.dup2 between test phases. + real_dup2 = os.dup2 + dup2_calls: list[tuple[int, int]] = [] + + def flaky_dup2(fd: int, fd2: int, inheritable: bool = True) -> int: + dup2_calls.append((fd, fd2)) + if len(dup2_calls) == 2: + raise OSError("injected restore failure") + return real_dup2(fd, fd2, inheritable) + + monkeypatch.setattr(os, "dup2", flaky_dup2) + + with anyio.fail_after(5): + async with stdio_server() as (read_stream, write_stream): # pragma: no branch + async with read_stream: # pragma: no branch + received = await read_stream.receive() + assert isinstance(received, SessionMessage) + assert received.message == request + await write_stream.aclose() + + # The restore was attempted (second dup2) and its failure swallowed: + # the context exited cleanly with fd 0 left on the null device. + assert dup2_calls[1] == (dup2_calls[1][0], 0) + devnull_probe = os.open(os.devnull, os.O_RDONLY) + try: + assert os.path.sameopenfile(0, devnull_probe) + finally: + os.close(devnull_probe) + + +@pytest.mark.anyio +async def test_stdio_server_takes_stdout_off_the_descriptor_table_while_serving( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """On the real process stdout, the transport claims the wire and diverts fd 1 to stderr. + + SDK-defined behavior: while serving, fd 1 points at stderr, so a stray + write - a print(), or a child process inheriting stdout - lands in the + client's log instead of corrupting the JSON-RPC stream. The transport's + own frames still flow over the original pipe, and on exit fd 1 points + back at it. Raw pipes are the subject here: the property under test is + what the process's descriptor table looks like while serving. + """ + response = JSONRPCResponse(jsonrpc="2.0", id=1, result={}) + with _pipe_planted_on_fd1(monkeypatch) as (out_r, out_w), _pipe_planted_on_fd2() as err_r: + with anyio.fail_after(5): + async with stdio_server(stdin=anyio.AsyncFile(io.StringIO())) as (read_stream, write_stream): + read_stream.close() + # A raw write to fd 1 - what a child process inheriting + # stdout does - lands on stderr, not the wire. + os.write(1, b"stray child output\n") + assert await _read_from(err_r) == b"stray child output\n" + + # sys.stdout follows fd 1 to the diversion as well: a stray + # print() in handler code cannot reach the wire. The text + # layer writes os.linesep, hence CRLF on Windows. + print("stray print", flush=True) + assert await _read_from(err_r) == b"stray print" + os.linesep.encode() + + # The transport's frames flow over the original pipe, unpolluted. + await write_stream.send(SessionMessage(response)) + line = await _read_from(out_r) + assert jsonrpc_message_adapter.validate_json(line.decode().strip()) == response + + await write_stream.aclose() + + # Restored: fd 1 is a handle to the protocol pipe again, not the + # diversion. (samestat degrades to trivially-true for pipes on + # Windows; the POSIX legs carry the assertion.) + assert os.path.sameopenfile(1, out_w) + + +@pytest.mark.anyio +async def test_a_nested_stdio_server_does_not_clobber_the_first_transports_stdout_claim( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Only the first default-stream transport claims stdout; a nested one writes in place. + + SDK-defined behavior: a second stdio_server() entered while the first is + serving must not re-claim fd 1 (it would duplicate the diversion and its + restore would clobber the first transport's), and must not write into the + outer transport's wire. The inner transport writes the in-place stdout - + wherever fd 1 currently points, here the stderr diversion, which is where + its frames observably land - and after both exit, fd 1 is back on the + protocol pipe. + """ + outer_response = JSONRPCResponse(jsonrpc="2.0", id=1, result={}) + inner_response = JSONRPCResponse(jsonrpc="2.0", id=2, result={}) + with _pipe_planted_on_fd1(monkeypatch) as (out_r, out_w), _pipe_planted_on_fd2() as err_r: + with anyio.fail_after(5): + async with stdio_server(stdin=anyio.AsyncFile(io.StringIO())) as (outer_read, outer_write): + outer_read.close() + async with stdio_server(stdin=anyio.AsyncFile(io.StringIO())) as (inner_read, inner_write): + inner_read.close() + await inner_write.send(SessionMessage(inner_response)) + line = await _read_from(err_r) + assert jsonrpc_message_adapter.validate_json(line.decode().strip()) == inner_response + await inner_write.aclose() + + # The outer transport still owns the real pipe. + await outer_write.send(SessionMessage(outer_response)) + line = await _read_from(out_r) + assert jsonrpc_message_adapter.validate_json(line.decode().strip()) == outer_response + await outer_write.aclose() + + assert os.path.sameopenfile(1, out_w) + + +@pytest.mark.anyio +async def test_stdio_server_diverts_stdout_to_the_null_device_when_stderr_is_unusable( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """When stderr cannot be duplicated, fd 1 is diverted to the null device instead. + + SDK-defined behavior: the diversion target is stderr so stray output stays + visible, but a process whose fd 2 is unusable still gets its stdout + claimed - pointed at the null device - and the wire stays pure. + """ + response = JSONRPCResponse(jsonrpc="2.0", id=1, result={}) + with _pipe_planted_on_fd1(monkeypatch) as (out_r, out_w): + # Fires exactly once, at the diversion's own dup of fd 2, then passes + # through: pytest's capture machinery also calls os.dup at phase + # transitions, and a still-armed injector detonating there corrupts + # capture for every later test in the process. + real_dup = os.dup + armed = [True] + + def failing_dup(fd: int) -> int: + if fd == 2 and armed[0]: + armed[0] = False + raise OSError("injected stderr failure") + return real_dup(fd) + + monkeypatch.setattr(os, "dup", failing_dup) + + with anyio.fail_after(5): + async with stdio_server(stdin=anyio.AsyncFile(io.StringIO())) as (read_stream, write_stream): + read_stream.close() + devnull_probe = os.open(os.devnull, os.O_WRONLY) + try: + assert os.path.sameopenfile(1, devnull_probe) + finally: + os.close(devnull_probe) + + # Stray output is discarded rather than blocking or reaching + # the wire; the transport's frames still flow over the pipe. + os.write(1, b"discarded\n") + await write_stream.send(SessionMessage(response)) + line = await _read_from(out_r) + assert jsonrpc_message_adapter.validate_json(line.decode().strip()) == response + await write_stream.aclose() + + assert os.path.sameopenfile(1, out_w) + + +@pytest.mark.anyio +async def test_the_claim_survives_a_process_started_with_stderr_closed( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A free standard slot never captures the private wire duplicate. + + SDK-defined behavior: in a process started with fd 2 closed, os.dup hands + out slot 2 - without the guard, the private stdout duplicate would become + the process's "stderr" and the diversion would then point fd 1 straight + back at the wire. The claim must keep its own descriptors above the + standard range: stray writes are discarded (stderr is unusable, so the + diversion is the null device), the wire stays pure, and fd 2 is left as + it was found - closed. + """ + response = JSONRPCResponse(jsonrpc="2.0", id=1, result={}) + with _pipe_planted_on_fd1(monkeypatch) as (out_r, out_w): + saved2 = os.dup(2) + os.close(2) # the freed slot is what os.dup would hand out next + try: + with anyio.fail_after(5): + async with stdio_server(stdin=anyio.AsyncFile(io.StringIO())) as (read_stream, write_stream): + read_stream.close() + devnull_probe = os.open(os.devnull, os.O_WRONLY) + try: + assert os.path.sameopenfile(1, devnull_probe) + finally: + os.close(devnull_probe) + + os.write(1, b"discarded\n") + await write_stream.send(SessionMessage(response)) + line = await _read_from(out_r) + assert jsonrpc_message_adapter.validate_json(line.decode().strip()) == response + await write_stream.aclose() + + assert os.path.sameopenfile(1, out_w) + # No claim descriptor was left occupying the closed stderr slot. + with pytest.raises(OSError): + os.fstat(2) + finally: + os.dup2(saved2, 2) + os.close(saved2) + + +@pytest.mark.anyio +async def test_a_duplication_failure_midway_through_the_claim_leaks_nothing_into_the_standard_range( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A dup failure while skipping the standard range closes what was already made. + + SDK-defined behavior: with fd 2 free, the claim's first duplicate lands on + slot 2 and a second duplication is needed; when that one fails, the claim + degrades to serving in place and the slot-2 duplicate is closed rather + than left masquerading as the process's stderr. + """ + request = JSONRPCRequest(jsonrpc="2.0", id=1, method="ping") + with _pipe_planted_on_fd0(monkeypatch) as (in_r, in_w): + os.write(in_w, _frame(request)) + os.close(in_w) + monkeypatch.setattr(sys, "stdout", TextIOWrapper(io.BytesIO(), encoding="utf-8")) + + saved2 = os.dup(2) + os.close(2) + # Fires on the second dup of fd 0 - the escape from the standard + # range - then passes through (see the one-shot injector rationale in + # the descriptor-isolation failure test above). + real_dup = os.dup + dup_calls: list[int] = [] + + def failing_second_dup(fd: int) -> int: + if fd == 0: + dup_calls.append(fd) + if len(dup_calls) == 2: + raise OSError("injected descriptor failure") + return real_dup(fd) + + monkeypatch.setattr(os, "dup", failing_second_dup) + try: + with anyio.fail_after(5): + async with stdio_server() as (read_stream, write_stream): # pragma: no branch + async with read_stream: # pragma: no branch + # Isolation degraded to serving in place. + assert os.path.sameopenfile(0, in_r) + received = await read_stream.receive() + assert isinstance(received, SessionMessage) + assert received.message == request + await write_stream.aclose() + + # The injector leaves non-stdin descriptors untouched. + os.close(os.dup(in_r)) + # The slot-2 duplicate made before the failure was closed again. + with pytest.raises(OSError): + os.fstat(2) + finally: + real_dup2 = os.dup2 + real_dup2(saved2, 2) + os.close(saved2) + + +@pytest.mark.anyio +async def test_stdout_is_diverted_to_the_null_device_when_stderr_is_detected_as_the_wire( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A stderr that aliases stdout is never used as the diversion target. + + SDK-defined behavior: when stderr refers to the same open file as stdout + (a `2>&1` launch), diverting stray output "to stderr" would write straight + back into the wire, so the diversion falls back to the null device. The + detection itself is platform-dependent (see same_open_file), so it is + forced here; the POSIX twin below exercises the real detection. + """ + response = JSONRPCResponse(jsonrpc="2.0", id=1, result={}) + with _pipe_planted_on_fd1(monkeypatch) as (out_r, out_w): + + def merged(fd_a: int, fd_b: int) -> bool: + return True + + monkeypatch.setattr("mcp.server.stdio.same_open_file", merged) + + with anyio.fail_after(5): + async with stdio_server(stdin=anyio.AsyncFile(io.StringIO())) as (read_stream, write_stream): + read_stream.close() + devnull_probe = os.open(os.devnull, os.O_WRONLY) + try: + assert os.path.sameopenfile(1, devnull_probe) + finally: + os.close(devnull_probe) + + os.write(1, b"discarded\n") + await write_stream.send(SessionMessage(response)) + line = await _read_from(out_r) + assert jsonrpc_message_adapter.validate_json(line.decode().strip()) == response + await write_stream.aclose() + + assert os.path.sameopenfile(1, out_w) + + +@pytest.mark.anyio +@pytest.mark.skipif(sys.platform == "win32", reason="pipe identity is only comparable on POSIX") +# lax no cover: the per-job 100% coverage gate also runs on Windows, where this test is skipped. +async def test_a_stderr_merged_into_stdout_is_really_detected_on_posix( # pragma: lax no cover + monkeypatch: pytest.MonkeyPatch, +) -> None: + """With fd 2 genuinely pointing at the wire pipe, the diversion is the null device. + + The unmocked twin of the detection test above: fd 2 is planted on the same + pipe as fd 1 and the real same-open-file comparison must notice. + """ + response = JSONRPCResponse(jsonrpc="2.0", id=1, result={}) + with _pipe_planted_on_fd1(monkeypatch) as (out_r, out_w): + real_dup2 = os.dup2 + saved2 = os.dup(2) + os.dup2(out_w, 2) # stderr now IS the wire, as 2>&1 leaves it + try: + with anyio.fail_after(5): + async with stdio_server(stdin=anyio.AsyncFile(io.StringIO())) as (read_stream, write_stream): + read_stream.close() + devnull_probe = os.open(os.devnull, os.O_WRONLY) + try: + assert os.path.sameopenfile(1, devnull_probe) + finally: + os.close(devnull_probe) + + os.write(1, b"discarded\n") + await write_stream.send(SessionMessage(response)) + line = await _read_from(out_r) + assert jsonrpc_message_adapter.validate_json(line.decode().strip()) == response + await write_stream.aclose() + + assert os.path.sameopenfile(1, out_w) + finally: + real_dup2(saved2, 2) + os.close(saved2) + + class _GatedStdin(io.RawIOBase): """Raw stdin double: serves its frames, then blocks until released before EOF. diff --git a/tests/transports/stdio/test_lifecycle.py b/tests/transports/stdio/test_lifecycle.py index 8a370c10f6..21b3f5f5c2 100644 --- a/tests/transports/stdio/test_lifecycle.py +++ b/tests/transports/stdio/test_lifecycle.py @@ -15,12 +15,15 @@ import threading from contextlib import AsyncExitStack from pathlib import Path +from textwrap import dedent import anyio import anyio.abc import pytest +from mcp_types import TextContent from mcp.client import stdio +from mcp.client.client import Client from mcp.client.stdio import StdioServerParameters, stdio_client from mcp.os.win32.utilities import FallbackProcess from tests.transports.stdio._liveness import ( @@ -274,3 +277,49 @@ async def test_fallback_process_wait_is_cancellable_while_the_child_lives() -> N popen.wait() popen.stdin.close() popen.stdout.close() + + +@pytest.mark.anyio +async def test_a_tool_spawned_childs_stdout_writes_never_reach_the_wire(tmp_path: Path) -> None: + """A child writing to its inherited stdout pollutes the server's stderr, never the protocol. + + Regression for the stdout half of `stdio_server`'s descriptor isolation: + pre-isolation, a tool-spawned child that printed to its inherited stdout + wrote straight into the JSON-RPC stream ahead of the tool response. The + junk line arriving on the server's stderr is the whole assertion: fd 1 has + exactly one target, so stderr delivery proves the wire never saw it (the + byte-level wire-purity twin is in tests/server/test_stdio.py). + """ + server = dedent( + """ + import subprocess, sys + from mcp.server import MCPServer + + mcp = MCPServer("noisy-spawner") + + @mcp.tool() + def run_noisy_child() -> str: + # No redirection: the child inherits the server's stdout. + proc = subprocess.run([sys.executable, "-c", "print('this is not json')"], timeout=20) + return str(proc.returncode) + + mcp.run() + """ + ) + + with (tmp_path / "server-stderr.txt").open("w+") as errlog: + transport = stdio_client(StdioServerParameters(command=sys.executable, args=["-c", server]), errlog=errlog) + # Three interpreter cold starts (the server also imports the SDK) on a + # loaded runner; a regressed Windows leg hangs rather than corrupts, so + # the bound only has to beat "never". + with anyio.fail_after(40): + async with Client(transport) as client: + result = await client.call_tool("run_noisy_child") + errlog.seek(0) + server_stderr = errlog.read() + + content = result.content[0] + assert isinstance(content, TextContent) + assert content.text == "0" + # Diverted to the server's stderr instead of landing on the wire. + assert "this is not json" in server_stderr diff --git a/tests/transports/stdio/test_windows.py b/tests/transports/stdio/test_windows.py index 2d4eeac826..621e367ad0 100644 --- a/tests/transports/stdio/test_windows.py +++ b/tests/transports/stdio/test_windows.py @@ -15,12 +15,14 @@ import sys from contextlib import AsyncExitStack from pathlib import Path +from textwrap import dedent import anyio import anyio.abc import pytest -from mcp_types import JSONRPCRequest, JSONRPCResponse +from mcp_types import JSONRPCRequest, JSONRPCResponse, TextContent +from mcp.client.client import Client from mcp.client.stdio import StdioServerParameters, stdio_client from mcp.os.win32.utilities import FallbackProcess from mcp.shared.message import SessionMessage @@ -238,3 +240,56 @@ async def test_a_native_server_emitting_crlf_line_endings_round_trips_messages() # here instead of a parsed message. assert isinstance(received, SessionMessage) assert received.message == JSONRPCResponse(jsonrpc="2.0", id=1, result={}) + + +async def test_a_tool_spawned_python_child_with_default_stdin_completes_promptly() -> None: # pragma: no cover + """A tool that runs a Python subprocess without redirecting stdin returns promptly. + + Regression for #671 (SDK-defined isolation behavior): before `stdio_server` + pointed fd 0 at the null device, such a child inherited the protocol stdin + pipe and blocked inside interpreter startup behind the transport's pending + read (CPython gh-78961) until the next inbound message arrived - so these + calls, with no follow-up traffic, hung until the timeout. Covers both + reported shapes: output piped with stdin defaulted, and no redirection at + all (the console subsystem still propagates the standard handles). + """ + server = dedent( + """ + import subprocess, sys + from mcp.server import MCPServer + + mcp = MCPServer("spawner") + + @mcp.tool() + def run_child() -> str: + proc = subprocess.run([sys.executable, "-c", "print('ok')"], capture_output=True, timeout=20) + return proc.stdout.decode().strip() + + @mcp.tool() + def run_child_bare() -> str: + # No redirection at all: Windows still hands a console child the + # parent's standard handles, so pre-isolation this hung too. The + # child prints nothing, keeping this test pinned on the hang; the + # noisy-child shape is owned by test_lifecycle.py. + proc = subprocess.run([sys.executable, "-c", "pass"], timeout=20) + return str(proc.returncode) + + mcp.run() + """ + ) + transport = stdio_client(StdioServerParameters(command=sys.executable, args=["-c", server])) + + # Four interpreter cold starts on a loaded runner (the server also imports + # the SDK); healthy runs take ~3s. A regression hangs forever, so the bound + # only has to beat "never". + with anyio.fail_after(40.0): + async with Client(transport) as client: + result = await client.call_tool("run_child") + bare = await client.call_tool("run_child_bare") + + content = result.content[0] + assert isinstance(content, TextContent) + assert content.text == "ok" + bare_content = bare.content[0] + assert isinstance(bare_content, TextContent) + assert bare_content.text == "0"