Fix mcp stdio process handling - #17
Closed
ServeurpersoCom wants to merge 4 commits into
Closed
Conversation
Adds support for connecting llama-server to external MCP (Model Context Protocol) servers over stdio, exposing their tools through the existing /tools endpoint alongside the built-in ones. Servers are configured in the Cursor-compatible JSON format, via either --mcp-servers-config <path> or --mcp-servers-json <inline>. At startup each configured server is spawned once to discover its tools, then shut down; instances are spawned lazily on first tool call and kept for the lifetime of the server process. MCP instances are global rather than per-slot. The /tools endpoint has no slot binding, so keying instances by slot would tie external process lifetime to an unrelated inference slot's release, tearing down stateful MCP servers on every completion. The feature is off by default and marked experimental: enabling it lets the server spawn arbitrary local processes, so it should not be used in untrusted environments. Implementation notes: * NDJSON framing buffers complete lines across reads, so a notification and a response arriving in one pipe packet are both parsed rather than the second being stranded until the request times out. * Shutdown is signalled through a lock-free atomic shared by the manager and its instances, set from the signal handler before the HTTP server drains. Setting it during cleanup would be too late: cleanup cannot run until in-flight /tools handlers return, which is what the blocked RPC prevents. * Tool names are exposed as <server>_<tool> and skipped on collision with an existing tool, so an MCP server cannot shadow a built-in one. Tests cover tool discovery and invocation, timeouts, crash recovery and respawn cooldown, warmup failures, malformed output, batched notification+response writes, tool definition shape, and prompt shutdown during a slow tool call. Assisted-By: Claude Opus 4.8 <noreply@anthropic.com>
- server-mcp.h used HANDLE without including windows.h, breaking every TU that includes the header (server-mcp.cpp, server-tools.cpp). Move the platform-specific process_handle definition into server-mcp.cpp and only forward-declare it in the header, matching the existing server_subproc pattern in server-models.h. This keeps windows.h out of the header. - server-mcp.cpp used kill()/SIGTERM/SIGKILL without <signal.h>. glibc pulls it in transitively, macOS does not. Include it explicitly, along with <errno.h> and <sys/types.h> for errno and pid_t. - Zero-initialize STARTUPINFOA/PROCESS_INFORMATION to silence -Wmissing-field-initializers. Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
AGENTS.md asks for ASCII only: replace emdash with '-' and unicode arrow with '->' in comments added by the MCP changes. Comments only, no functional change. Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- prevent POSIX MCP children from inheriting pipes from other instances - fix Windows argument quoting for paths containing spaces and backslashes - keep Windows child stderr separate from the JSON-RPC stdout pipe
Author
|
Superseded by #18 due to an incorrect base branch. |
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
server: fix cross-platform MCP stdio process handling
For ggml-org#25736
Additional information
Requirements