Open
server: support MCP stdio#26062
Conversation
Member
* server-mcp: harden transport and wire up the tool integration Builds on the transport/manager architecture (server_mcp_transport + server_pipe) with the hardening and integration the draft did not yet have. Hardening: * Reader and stderr pumps are polled (running-aware) instead of blocking on a read that only ends at EOF. subprocess_terminate() SIGKILLs only the direct child, so a grandchild the MCP server spawned that inherited the pipe would otherwise keep the write end open and hang teardown (both warmup shutdown at startup and process shutdown). The writer is likewise non-blocking + polled. * Windows: resolve the command through PATHEXT so "npx" (npm ships npx.cmd, never npx.exe) spawns, matching POSIX's PATH search; and enumerate the parent environment as UTF-8 (GetEnvironmentStringsW) instead of the active code page. * server_pipe gains an opt-in max_size (default unbounded, so the router's streaming use is unchanged); the MCP reply queue uses it so a server that streams unsolicited notifications between requests cannot grow it without bound. Integration: * --mcp-servers-config / --mcp-servers-json flags; enabling MCP restricts default CORS to localhost, same as --tools. * MCP tools are exposed through /tools (and chat-completions) as <server>_<tool>, skipping names that collide with a built-in or another MCP tool. * Manager lifecycle wired into llama_server(): warmup at start, shutdown() from the signal handler before the HTTP server drains, blocking teardown in clean_up(). * SIGPIPE ignored so a child dying mid-write yields EPIPE rather than killing us. Assisted-By: Claude Opus 4.8 <noreply@anthropic.com> * server-mcp: add MCP test suite with grandchild deadlock regression test 21 tests over the /tools endpoint: tool discovery/invocation, timeouts, crash recovery and respawn cooldown, warmup partial failure, malformed and batched notification+response output, tool-definition shape, and prompt shutdown during a slow call. The last test spawns an MCP server that leaves a grandchild inheriting its stdout/stderr and asserts the server both starts and stops promptly. Verified it fails (5s SIGKILL fallback on a deadlocked reader-join) when the pump is made to ignore the running flag, and passes with the polled reader. Assisted-By: Claude Opus 4.8 <noreply@anthropic.com> * clean up * clean up 2 * even stricter life cycle * nits * nits 2 --------- Co-authored-by: Xuan Son Nguyen <son@huggingface.co>
ngxson
commented
Jul 24, 2026
| } | ||
|
|
||
| // note: this is guaranteed to out-live ctx_http and tools | ||
| server_mcp mcp_mgr; |
Collaborator
Author
There was a problem hiding this comment.
note that the life cycle is very strict now:
- server_mcp out live everything else
- tool uses server_mcp, so on-going tool requests is safe while waiting for server_mcp to terminate
- ctx_http out live tool, so req/res objects are safe to access before tool is destroyed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Support MCP with stdio transport; server accepts MCP JSON definition, spawns and manage subprocess. MCP tools are exposed as server tools.
Supersede #25736
Depends on #26061
Additional information
The core implementation is split into 4 separate classes:
server_mcp_server_config: data class, parsed version of mcp.json config fileserver_mcp_transport: manage high-level JSON-RPC messagesserver_mcp_stdio: manage low-level pipes and subprocess (no notions of json here, just byte streams)server_mcp: manage all instances ofserver_mcp_transportThe behavior was designed to match #25736 , but the underlay architecture is different, notably:
server_mcpmanages everything, even the life-cycle of transports and subprocessessubprocess.hshared_ptr--> stricter life-cycle definition. For example:shutdown()is blocking, no pointers are leaked after going out-of-scope, never have detached threadsTODO:
server-tools.cppRequirements
server_pipe,subprocess.h, while introducing design constraints like no redundantshared_ptroratomicsDespite the stated constraints, the model still made some bad assumptions about whether to use
shared_ptr/atomic. During the development, I had to point out these and refine the design at each steps.