server: support MCP stdio

[ngxson]

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 file
• server_mcp_transport: manage high-level JSON-RPC messages
• server_mcp_stdio: manage low-level pipes and subprocess (no notions of json here, just byte streams)
• server_mcp: manage all instances of server_mcp_transport

The behavior was designed to match #25736 , but the underlay architecture is different, notably:

• server_mcp manages everything, even the life-cycle of transports and subprocesses
• Separation of byte-level stream and JSON-RPC stream
• Subprocess handling is done by subprocess.h
• Limited uses of shared_ptr --> stricter life-cycle definition. For example: shutdown() is blocking, no pointers are leaked after going out-of-scope, never have detached threads

TODO:

• merge vendor: update subprocess.h #26061
• add arg and handle it
• add handling in server-tools.cpp
• add tests

Requirements

• I have read and agree with the contributing guidelines
• AI usage disclosure: yes, steps below:
  1. I ask it to study the behavior of server : add experimental MCP (stdio) server support #25736 (but not to study the code, the goal was to reimpl it from 0 with a clean approach)
  2. I stated existing components that we can reuse: server_pipe, subprocess.h, while introducing design constraints like no redundant shared_ptr or atomics
  3. I ask it to generate the header file first, then I manually review it
  4. Then, I ask it to implement one component at a time; only when I validate the design, that I allow it to move onto the next one

Despite 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.

[pwilkin]

@ngxson added a PR (#26075) integrating your architecture with the tests and server integration + fixes for issues that turned up during my branch's testing.

[ngxson]

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

[ngxson]

@ggml-gh-bot review

[ggml-gh-bot]

Automated code review

I have enough context. Here is my review.

---

Code review: MCP server integration (PR #26062)

This PR adds a backend MCP integration: --mcp-servers-config/--mcp-servers-json, a new server-mcp.{cpp,h} subsystem that spawns MCP child processes and speaks JSON-RPC over stdio, and wires the discovered tools into /tools. It also refactors the existing pipe_t into a shared server_pipe. The engineering is mostly careful (polled non-blocking I/O to avoid grandchild-pipe deadlock, bounded stderr tail, id correlation, spawn cooldown, warmup, and a lifetime order where mcp_mgr outlives ctx_http/tools), but I have one scope/approach question and one real concurrency bug, plus a handful of smaller issues.

Checklists run: Scope/quick-reject, Security, Approach-and-design (new subsystem), Server, General.

Blocking

(point 1) Scope/approach needs maintainer sign-off and a tracking issue. This adds a large new subsystem (server-mcp.cpp is ~810 lines, per-server process + 3 threads + hand-rolled NDJSON framing and JSON-RPC). AGENTS.md calls out that invasive changes adding whole subsystems should be discussed first. tools/server/README-dev.md lists MCP under Frontend in-scope ("Agentic features, example: MCP") and there is already a frontend approach (--ui-mcp-proxy). This PR instead implements a backend that spawns arbitrary local processes via stdio and exposes them through /tools. The "disabled by default" security note in README-dev is satisfied (only active when the new flags are passed), which is good, but confirm with maintainers that backend stdio-spawning is the intended direction versus the documented frontend model, and that there is a linked issue. Also note --agent enables builtin tools + the UI MCP proxy but deliberately does not enable this, which is consistent but worth stating explicitly in the --agent help.

(point 2) Data race on server_mcp_transport::last_error. server_mcp_stdio::diagnostics() reads last_error without holding rpc_mutex (tools/server/server-mcp.cpp:507-508), while ensure_init and list_tools write last_error under rpc_mutex (:251, :275). server_mcp::get_or_create calls diagnostics() on a transport that another thread can concurrently be using in send_rpc: thread A grabs a live transport and blocks in send_rpc; the child dies so running clears; thread B's get_or_create sees !is_alive() and calls diagnostics() (reads last_error) while A's ensure_init is writing last_error after its send_rpc returns. last_error is a std::string (heap-allocated for these messages, so not SSO-safe), so this is a TSAN-detectable data race and UB, with a narrow but real window. Fix: take rpc_mutex in diagnostics() around the last_error read (lock order is safe: nothing takes rpc_mutex then server_mcp::mutex). The err_tail half is already correctly guarded by err_mu.

Will slow the review

(point 3) /tools response shape is now inconsistent and undocumented. server_mcp_transport::call_tool (tools/server/server-mcp.cpp ~line 300) returns the raw MCP result object ({"content":[{"type":"text","text":"..."}]}) on success and the raw JSON-RPC error object ({"error":{"code":...,"message":...}}) on RPC failure. Builtin tools return {"plain_text_response":"..."} on success and {"error":"<string>"} on failure, and server_mcp::call_tool itself returns {"error":"MCP server unavailable: ..."} (string) for the unavailable/init-failure cases. So a /tools caller now gets four different shapes depending on tool type and failure mode. tools/server/README-dev.md "API for tools" only documents plain_text_response and {"error":"<string>"}. Consider normalizing MCP results to plain_text_response (concatenate result.content[].text) and converting RPC object-errors to the documented string-error form, so /tools keeps one contract. At minimum, update README-dev to document the MCP result and error-object shapes.

(point 4) Document the new type value. server_tool::to_json() now emits type() (tools/server/server-tools.cpp:27), which returns "mcp" for MCP tools. README-dev still says type is "always be "builtin" for now". Update that line.

Nits

(point 5) tools/server/tests/unit/test_mcp_servers.py:716 uses an em dash (—) in an assertion message; use ASCII - per the project's ASCII-only rule.

(point 6) common/common.h:672-673 declares mcp_servers_config/mcp_servers_json without = "" while the neighboring router string fields use = ""; match the surrounding style.

(point 7) server_mcp_transport::ensure_init (tools/server/server-mcp.cpp:257-259) ignores the return of to_server.write(notif.dump()) and sets initialized = true even if the notifications/initialized write was dropped (writer closed). A strict MCP server rejects calls before that notification; consider checking the return and not marking initialized on failure.

(point 8) mcp_write_all on Windows (tools/server/server-mcp.cpp:537) ignores the return of SetNamedPipeHandleState(..., PIPE_NOWAIT, ...). PIPE_NOWAIT on anonymous pipes is not guaranteed to succeed; if it fails the stdin handle stays blocking and the "polled/non-blocking so teardown never hangs" assumption is violated (a full pipe + non-reading child would block the writer thread until teardown kills the child). At least log/handle the failure so the behavior is explicit.

(point 9) from_server.max_size = 65536 (tools/server/server-mcp.cpp:446) bounds the queue by item count, not bytes; with max_line = 8 MB per item the comment "bound the reply queue" is only weakly true. Since MCP servers are operator-trusted this is low severity, but consider a smaller item cap or a byte cap to match the comment's intent.

(point 10) server_mcp::get_or_create (tools/server/server-mcp.cpp:794) holds server_mcp::mutex across fresh->start(), which spawns a subprocess and3 threads; this blocks lookups for all servers during a slow spawn. Consider dropping the lock for the spawn and rechecking the map.

(point 11) server_mcp::start() warmup (tools/server/server-mcp.cpp ~line 730) is sequential and each server is capped at MCP_WARMUP_TIMEOUT_SECONDS = 10s; with N servers, readiness can stall for N*10s. Worth a note or parallelization.

(point 12) Test hygiene in test_mcp_servers.py: test_mcp_empty_tool_list writes a fixture into the source tree (fixtures/_empty_mcp_server.py) and unlinks in finally (leaks on interrupt), and its startswith("empty:") check is vacuous because the implementation uses "empty_"; test_mcp_tools_not_listed_when_not_configured checks ":" in name for the same reason. The module docstring says tools use <server>:<tool> naming but the code uses <server>_<tool>. Tighten the assertions and prefer tempfile for generated fixtures.

(point 13) extern char ** environ; (tools/server/server-mcp.cpp:21) is the common workaround but is fragile across toolchains; fine for now, just be aware some strict POSIX modes need _GNU_SOURCE for the declaration to match <unistd.h>.

Things that look right (no change needed)

• The pipe_t -> server_pipe refactor is clean: no stale pipe_t references remain, and the added should_stop && should_stop() null-guard is a safe generalization (existing callers pass non-null).
• Command execution is operator-controlled (CLI/config only, not HTTP-client-supplied), args are passed as an argv array (no shell injection), env is merged, and the feature is off by default; the localhost-CORS default when MCP is enabled is a good security default.
• Declaration order of mcp_mgr before ctx_server/ctx_http/tools, combined with ctx_http.thread.join() (which joins all httplib worker threads via the pool's shutdown()) happening before local destruction, makes the shutdown/lifetime ordering sound: mcp_mgr.shutdown() flips the cancel flag before the worker join, so in-flight calls bail promptly and tools/mcp_mgr are not destroyed under live handlers.
• <sheredom/subprocess.h> is already vendored and used by mtmd-helper.cpp/server-models.cpp, so no new dependency is introduced.

This review was generated automatically by pi coding agent using zai-org/GLM-5.2. It may contain mistakes. Maintainers make the final call.

[ngxson]

need your final review @pwilkin @ServeurpersoCom

[ServeurpersoCom]

I get 4 issues:

Every spawned child leaks a zombie that accumulates for the process lifetime, because join_pumps() never calls subprocess_join().

One tool call timeout kills a healthy child, because server_pipe::read() closes the pipe for good as soon as should_stop fires.

The disconnect cancellation in server_mcp_tool::invoke() is unreachable, because support_stream is false so find_tool() rejects the streaming path and st is always null.

An MCP server that exits on stdin EOF never sees it, so it can only be killed. That last one comes from vendor/sheredom/subprocess.h creating its pipes without O_CLOEXEC, which should be fixed upstream.

Opus 5 patch for the first three:

diff --git a/tools/server/server-common.h b/tools/server/server-common.h
index 16674cb30..6ef797ebb 100644
--- a/tools/server/server-common.h
+++ b/tools/server/server-common.h
@@ -407,7 +407,9 @@ struct server_pipe {
         cv.notify_all();
     }

-    bool read(T & output, const std::function<bool()> & should_stop) {
+    // close_on_stop = true: should_stop means the reader is gone for good, so the writer is told the pipe is broken.
+    // close_on_stop = false: should_stop is a per-read deadline and further reads still come, so the pipe stays usable.
+    bool read(T & output, const std::function<bool()> & should_stop, bool close_on_stop = true) {
         std::unique_lock<std::mutex> lk(mutex);
         constexpr auto poll_interval = std::chrono::milliseconds(500);
         while (true) {
@@ -420,8 +422,10 @@ struct server_pipe {
                 return false; // clean EOF
             }
             if (should_stop && should_stop()) { // a null should_stop means "never stop"
-                close_read(); // signal broken pipe to writer
-                return false; // cancelled / reader no longer alive
+                if (close_on_stop) {
+                    close_read(); // signal broken pipe to writer
+                }
+                return false; // cancelled / deadline reached
             }
             cv.wait_for(lk, poll_interval);
         }
diff --git a/tools/server/server-mcp.cpp b/tools/server/server-mcp.cpp
index 905759f1f..e8ecbeda6 100644
--- a/tools/server/server-mcp.cpp
+++ b/tools/server/server-mcp.cpp
@@ -223,7 +223,8 @@ json server_mcp_transport::send_rpc(const json & request, const std::function<bo
     };

     std::string frame;
-    while (from_server.read(frame, stop)) {
+    // `stop` is this request's deadline, not the end of the session: a later reply is skipped on id mismatch, so the transport keeps serving the next call
+    while (from_server.read(frame, stop, false)) {
         json reply;
         try {
             reply = json::parse(frame);
@@ -660,6 +661,7 @@ void server_mcp_stdio::join_pumps() {
     if (reader.joinable()) reader.join();
     if (errlog.joinable()) errlog.join();

+    subprocess_join(&proc->sp, nullptr); // reap the child: destroy() never waits, so the pid would stay a zombie for the process lifetime
     subprocess_destroy(&proc->sp); // safe now: no thread touches the FILE* anymore
     proc.reset();
 }
diff --git a/tools/server/server-tools.cpp b/tools/server/server-tools.cpp
index f5de1735f..fcfddb3cf 100644
--- a/tools/server/server-tools.cpp
+++ b/tools/server/server-tools.cpp
@@ -1140,13 +1140,8 @@ struct server_mcp_tool : server_tool {
         };
     }

-    json invoke(json params, server_tool::stream * st) const override {
-        // pass the caller's liveness through so a disconnect cancels the in-flight RPC
-        std::function<bool()> should_stop = nullptr;
-        if (st) {
-            should_stop = [st]() { return !st->alive(); };
-        }
-        return mcp_mgr.call_tool(server_name, tool_name, params, should_stop);
+    json invoke(json params, server_tool::stream *) const override {
+        return mcp_mgr.call_tool(server_name, tool_name, params);
     }
 };

Windows still needs to be tested.

[pwilkin]

My adversarial review session brought up this:

C1 — drop-oldest can evict a pending response: Confirmed in server-common.h that server_pipe::write() does while (queue.size() >= max_size) queue.pop(); — std::queue::pop() removes the front (verified against cppreference), and read() also consumes from the front. Since the reader thread pushes both notifications and responses into the same from_server FIFO (no filtering), and send_rpc is the sole consumer, a sustained notification burst that keeps the queue saturated can evict a pending response id=N right as it reaches the front, causing a spurious timeout despite a valid server reply.
C2 — per-call timeout permanently poisons the transport: send_rpc's stop() (deadline/cancel) is passed into read(), which calls close_read() on stop. close_read() sets reader_closed=true permanently (grep confirms it's never reset in tools/server). The still-running reader then dies on the next write, running goes false, and get_or_create respawns a fresh transport with a new handshake. A slow-but-valid call destroys the JSON-RPC session.

[ServeurpersoCom]

C2 is the same bug as the timeout teardown fixed in the patch above: the per-request deadline no longer closes the pipe, and a late reply is skipped on id mismatch.

C1 reproduced, but it takes a deliberately optimized flooder. A server that serializes each notification cannot outpace the consumer: a reply sandwiched between 300k notifications still arrives in about 1s. Blasting pre-serialized notifications with raw 1MB writes does lose the reply and the call spuriously times out at the deadline (reproduced at 200k notifications around the reply).

With the patch the damage is limited to that one call: the transport survives, the next call succeeds, and memory stays bounded, which is what drop-oldest is there for. The principled fix would be backpressure instead of drop-oldest, a blocking writer when the queue is full so the child stalls on its own pipe and nothing is lost.

[ServeurpersoCom]

Proposed C++ changes: e6de1ec

Python nits: 74a08e8

I check Windows

[ngxson]

thanks, both points should be addressed now

[ServeurpersoCom]

Windows native test: the vendored subprocess.h argument quoting breaks on a quoted argument ending in a backslash. An MCP arg C:\un dossier\ (space, trailing backslash) reaches the child merged with every following argument into a single one: C:\un dossier" C:\controle\ MARKER_END. The builder only doubles a backslash when the next source character is a quote, so the trailing backslash ends up escaping the closing quote it generates. Same quoting class I fixed in #25736, should be fixed upstream in sheredom/subprocess.h.

On the other hand, the implementation on the llama.cpp side is flawless. Shall we submit a PR to subprocess.h? :D

[ServeurpersoCom]

LGTM, the limitations are on the subprocess.h side

[ServeurpersoCom]

I'm going to apply the subprocess.h patch to do an adversarial test, and we'll also be able to use it, plus, it'll be ready for a PR.

[ngxson]

Yes I think that can be a valid upstream fix, feel free to push it upstream and we will sync in a follow up PR

[ServeurpersoCom]

86b7a53

Success :

Upstream issue sheredom/subprocess.h#100
PR sheredom/subprocess.h#101

[pwilkin]

Let me do one big adversarial just to be sure and then we can merge.

[ServeurpersoCom]

My usual Node.js tools work perfectly over stdio :)

[pwilkin]

Heeeere iiiiit cooooooomes!

Adversarial review — MCP stdio integration (open findings)

Branch: xsn/server_mcp_stdio
Reviewed at: 33f3ec1b6 ("server: make MCP test fixtures JSON-RPC 2.0 compliant")
Base: origin/master @ 2cfc7670e
Date: 2026-07-25

Scope: the 12 branch-only commits vs origin/master — tools/server/server-mcp.{cpp,h},
the server_pipe move into server-common.h, the server-tools.cpp MCP tool adapter,
the two new CLI args, and the test fixtures. The --load-mode and subprocess.h vendor
changes that show up against a stale local master are upstream, not this branch.

Method: built the branch and drove a live llama-server in router mode against six
purpose-built MCP fixtures (dying-on-call, slow/parameterized-sleep, paginated,
hostile-names, non-text-content, grandchild-pipe-holder). Every finding marked
reproduced was observed on a running server, not inferred from reading.

This document lists open findings only. Three earlier findings — a per-call timeout
permanently tearing down the transport, a zombie leaked per spawn, and an unreachable
disconnect-cancellation branch — were fixed in 08a384bd7 and re-verified fixed at this
tip; they are not repeated here.

---

Summary

#	Severity	Finding
1	Critical	Spawn cooldown never engages → unbounded respawn
2	High	tools/list pagination ignored
3	High	Non-text results silently discarded
4	High	No tool-name validation
5	High	All calls to one server serialize, with no cancellation path
6	Medium	Timed-out calls keep executing; no notifications/cancelled
7	Medium	diagnostics() empty in the common death case
8	Medium	from_server bounded by count, not bytes
9	Medium	notify_all outside mutex; diagnostics() under manager lock
10	Medium	Every server spawned twice; discovery is one-shot
11	Medium	permission_write hard-coded false; type:"mcp" unconsumed
12	Medium	--mcp-servers-json exposes secrets via ps
13	Low	Five vacuous test assertions

---

Critical

1. Spawn cooldown never engages → unbounded respawn

dead_servers is written only when start() fails (server-mcp.cpp:825). A server that
spawns fine but dies during the RPC takes the is_alive() eviction path instead, which
has no cooldown at all. MCP_COOLDOWN_SECONDS is therefore dead for the most common
failure mode.

Reproduced at 33f3ec1b6 — 12 requests to a fixture that sys.exit(0)s on tools/call:

call1..call12  http=200  t≈0.016s each
spawns total: 13        <- one fresh process per request

A client can drive process creation at HTTP request rate. Each spawn used to leak a PID as
well; that leak is now fixed, but the storm itself is not.

Fix direction: record a cooldown entry on the !is_alive() eviction path too, not
just on spawn failure. Consider distinguishing "died mid-RPC" (likely to recur) from
"clean EOF after idle" (safe to respawn immediately).

---

High

2. tools/list pagination ignored

list_tools (server-mcp.cpp:292-312) never sends params.cursor and never reads
nextCursor. The only cursor tokens in the file are parse_cursor_format —
the Cursor-compatible config format, unrelated.

Reproduced: a fixture returning 5 tools plus "nextCursor":"page2" yields exactly 5;
page2_tool is silently unreachable. Tools on servers with paginated listings can never
be called.

3. Non-text results silently discarded

mcp_result_to_response (server-mcp.cpp:196) concatenates only type=="text" parts.

Reproduced:

image-only result       -> {"plain_text_response":""}
structuredContent-only  -> {"plain_text_response":""}

No error, no marker. The model is told the tool succeeded and returned nothing — a
silent-wrong-answer path. structuredContent (MCP 2025-06-18) is dropped entirely.

Related: MCP_PROTOCOL_VERSION is pinned to "2024-11-05" and the server's returned
protocolVersion is never validated — a fixture answering 2025-06-18 was accepted
without comment.

4. No tool-name validation

<server>_<tool> is passed straight into function.name with no sanitization or length
cap. Reproduced — both of these are advertised verbatim in GET /tools:

'h_has spaces/and.dots'   len=21
'h_xxxx…xxxx'             len=82

Both violate the ^[a-zA-Z0-9_-]{1,64}$ contract every OAI-compatible client enforces.
They round-trip fine through llama.cpp's own /tools POST (literal string match), so the
bundled UI works and the breakage only shows up for third-party clients and for grammar
generation from the tool schema.

Also order-dependent: the <server>_<tool> flattening means server a_b tool c collides
with server a tool b_c; the collision is detected and skipped, but which one survives
depends on map iteration order.

5. All calls to one server serialize, with no cancellation path

rpc_mutex is held across the entire blocking RPC (server-mcp.cpp:318). Reproduced —
four concurrent calls to a 5 s tool:

req t=5.02s  /  10.02s  /  15.02s  /  20.02s

Queued callers block in std::lock_guard, which cannot observe should_stop, so each
pins an HTTP worker thread for up to timeout_ms regardless of client state. With the
default 30 s timeout, N concurrent requests to one slow server occupy N worker threads
for up to N×30 s.

MCP tools set support_stream = false, so POST /tools with stream:true is rejected
with 404 and invoke is only ever reached on the non-streaming path. There is therefore
no cancellation at all: a client that disconnects mid-call leaves both the worker thread
and the child running to completion. Confirmed live — killing the client at t=1 s left the
child running the full 5 s.

The id-correlation machinery needed to multiplex concurrent in-flight requests over one
stdio pair is already implemented in send_rpc — it is simply not used for concurrency.

---

Medium

6. Timed-out calls keep executing; no notifications/cancelled

Fixture stderr shows EXECUTING id=2 secs=5.0 running to completion after the client
already received {"error":"request timed out"}. The MCP spec's notifications/cancelled
is never sent. For non-idempotent tools (write, send, commit) a timeout means "may or may
not have happened", with no way for the caller to find out.

7. diagnostics() empty in the common death case

call_tool never writes last_error — only ensure_init and list_tools do. Every
eviction logs:

W srv get_or_creat: MCP 'hf' is no longer alive:

with nothing after the colon. The err_tail stderr-capture machinery also comes up empty
here because errlog_loop shares the running flag with the reader and exits as soon as
stdout hits EOF — dropping exactly the buffered stderr that would explain the death.

8. from_server bounded by count, not bytes

from_server.max_size = 65536 (server-mcp.cpp:466) against an 8 MB per-line cap in
mcp_pump_ndjson — a theoretical worst case around 512 GB. The drop-oldest policy
(server-common.h:437) can also evict a pending reply under sustained notification
pressure. The bound was added for exactly this hazard; it is expressed in the wrong unit.

9. notify_all outside mutex; diagnostics() under the manager lock

close_write/close_read (server-common.h:405) notify without holding the mutex, so a
lost wakeup parks a reader for the full 500 ms poll interval while it holds rpc_mutex.
get_or_create calls diagnostics() — which takes rpc_mutex — while holding the
server_mcp::mutex, so that stall blocks dispatch for every other configured MCP server.
Modest in magnitude, but it is a genuine cross-server priority inversion.

10. Every server spawned twice; discovery is one-shot

server_mcp::start (server-mcp.cpp:744-760) spawns each server, lists its tools, and
shuts it down; the first real call then spawns it again. Servers with expensive or
side-effecting startup pay twice.

Discovery never repeats: a server that misses warmup has its tools permanently absent
from registry with no retry, and a server whose tool set changes at runtime is never
re-read. Warmup is also serial with a 10 s per-server cap, so N unresponsive servers add
up to N×10 s to startup.

11. permission_write hard-coded false; type:"mcp" unconsumed

server_mcp_tool sets permission_write = false for every MCP tool. A filesystem MCP
server's write_file is advertised as read-only.

Not currently exploitable: fetchBuiltinTools (tools.svelte.ts:474) discards type and
permissions wholesale and files every /tools entry as ToolSource.BUILTIN, and nothing
in the UI reads permissions.write. But the field is part of the documented /tools
contract in README-dev.md, so any consumer that honours it is misled.

Conversely the new type:"mcp" value is emitted correctly and documented, but nothing
consumes it — mcp.d.ts:289 still types it as ToolSource.BUILTIN, so MCP tools appear
in the UI grouped under "Built-in tools".

Worth noting alongside: an MCP server controls its own tool description, which is fed
straight to the model. That is inherent to MCP, but combined with a hard-coded
write=false the UI has no signal at all to gate on.

12. --mcp-servers-json exposes secrets via ps

MCP configs routinely carry API keys in env. Passed on the command line they are
world-readable in /proc/<pid>/cmdline. The --mcp-servers-config file form and the
LLAMA_ARG_MCP_SERVERS_JSON env var both avoid this; the help text should steer there.

Related: when env is non-empty, mcp_build_env merges overrides over the full parent
environment, so the child inherits every secret in llama-server's environment. This matches
Cursor's semantics, but there is no way to request a clean environment.

Minor usability trap: valid JSON without an mcpServers key logs
MCP config: no servers found in JSON and then leaves /tools returning
403 this feature is disabled — observed live while writing these tests.

---

Low — test suite

13. Five vacuous assertions

33f3ec1b6 corrected the module docstring from <server>:<tool> to <server>_<tool>,
but not the assertions that encoded the wrong separator. All five still pass against a
completely broken implementation:

Line	Assertion	Why it can never fail
199	[t for t in tools if ":" in get_tool_name(t)]	names use _, never :
533	get_tool_name(t).startswith("empty:")	same
141	t.get("name", "").startswith("nonexistent_")	to_json() emits tool, not name
228	assert res.status_code in (200, 500)	admits both outcomes
106	assert "plain_text_response" in body or ...	checks presence, never value

Line 106 is why finding #3 passes CI: a tool returning {"plain_text_response":""} for
all non-text content is indistinguishable from success.

Not covered at all: respawn rate limiting (#1), pagination (#2), non-text content (#3),
tool-name validity (#4), concurrency latency and client-disconnect behaviour (#5).

Two mechanical issues:

• test_mcp_empty_tool_list writes _empty_mcp_server.py into the repo's
  tests/fixtures/ at runtime and unlinks it in finally — stranded in the source
  tree if the test crashes hard.
• Every test hardcodes server_port = 8085 against a global server, so the file cannot
  run in parallel. test_mcp_config_file_errors also binds a local server, shadowing
  the global.

Suite status at 33f3ec1b6: 21 passed in 24.7 s. Worth flagging that this is a weaker
signal than it looks — the two critical bugs fixed in 08a384bd7 were both green under
this suite before the fix, and the assertions that would have caught them are the same
ones still tautological above.

---

Suggested order of work

1. Merging tensors of larger models #1 — the only critical; move the cooldown onto the eviction path.
2. Add missing headers for memcpy and assert #3, Repetition penalty #4 — small, self-contained, and both produce silently wrong output today.
3. Windows VS2022 Build - Returning nonsense #2 — a while (nextCursor) loop in list_tools.
4. [Q] Memory Requirements for Different Model Sizes #13 — fix the five assertions before anything else lands, so the next regression
   is actually caught.
5. Suppress output that isn't from the model #5 — the one real design change: id-multiplexed concurrency over the single stdio
   pair, using the correlation logic that already exists.

[ServeurpersoCom]

I'm try the lightest possible patch to test it out, including only critical fixes and maintaining the existing compromises.
For UI, other PR

[pwilkin]

Yeah, most of those are fortunately non-blockers or even out-of-scope (the secrets-in-inline-mcp-config one). But I figured out I might as well test Opus 5 - interestingly, unlike Opus 4.8 it didn't launch an agent swarm even though I set it to Ultracode.

[ServeurpersoCom]

Look this, there is four fixes : eb0105d

Python : 9687dc8

Intentionally left unaddressed: # 5 (concurrency multiplexed by ID; this is a design change: Son's call)

[ServeurpersoCom]

Fixed

• Spawn cooldown now also covers a child that dies right after spawning: a server dying on every call respawns once per cooldown instead of once per request, a long-lived child still respawns immediately
• tools/list pagination followed, with a page cap against infinite cursors
• Non-text tool results marked instead of silently returning an empty string, structured-only results return their JSON payload
• Tool names sanitized to the ^[a-zA-Z0-9_-]{1,64}$ contract, original name kept for the MCP round-trip
• Five test assertions that could never fail now bind: type-based MCP filters, exact result values, and a deterministic two-call fail_once contract

Not fixed, on purpose

• Per-server serialization and the missing cancellation/notifications/cancelled: changing to id-multiplexed concurrency is a design decision, not a review patch
• Bounding the reply queue in bytes instead of items, and the CV wakeup on the eviction path: same queue design discussion
• Double spawn at startup: cost of the warmup-then-close design, works as intended
• Empty diagnostics on eviction and full parent env inheritance: worth follow-ups, not blockers
• permission_write and the mcp type on the WebUI side: frontend work, separate PR

Tested

• Respawn storm: 12 rapid calls to a die-on-call server, 1 spawn instead of 13
• Aged child killed, immediate respawn in 14ms
• Two-page tool discovery with the page-2 tool callable end to end
• Image-only, structured-only, and mixed content results
• Hostile tool names with spaces, dots, and 80+ characters
• Full pytest suite green

[pwilkin]

This is good enough for me, I think we can address any further issues in a followup.

[ngxson]

I had the same concern about the "unbounded respawn" during development, but let's see if it will actually be a problem in normal usage. What I wanted initially was the have respawn serialized, but that's a bit complicated.

Merging this as-is, feel free to push follow-up fixes. Thanks again for the review.

[ggerganov]

A minimal example of using this functionality would be helpful - it's not obvious how to use it.

[ServeurpersoCom]

A minimal example of using this functionality would be helpful - it's not obvious how to use it.

Sure, let me give a quick example. In short, llama-server can fork a process that communicates via JSON-RPC, effectively turning it into a built-in tool. Even a plain script can be an MCP stdio server, I'll put together a small self-contained example that shows how to use it safely.

[ServeurpersoCom]

Here is a minimal example. An MCP stdio server is just a process reading JSON-RPC lines on stdin and writing replies on stdout, so a plain POSIX shell script is enough, no SDK, no dependencies:

#!/bin/sh
# minimal MCP stdio server in POSIX shell
# protocol: one JSON-RPC message per line (NDJSON) on stdin/stdout, logs go to stderr

while IFS= read -r line; do
    id=$(printf '%s' "$line" | sed -En 's/.*"id":([0-9]+).*/\1/p')
    method=$(printf '%s' "$line" | sed -En 's/.*"method":"([^"]*)".*/\1/p')
    case "$method" in
        initialize)
            printf '{"jsonrpc":"2.0","id":%s,"result":{"protocolVersion":"2024-11-05","capabilities":{"tools":{}},"serverInfo":{"name":"echo-sh","version":"1.0"}}}\n' "$id"
            ;;
        tools/list)
            printf '{"jsonrpc":"2.0","id":%s,"result":{"tools":[{"name":"echo","description":"Echo the input text back","inputSchema":{"type":"object","properties":{"text":{"type":"string"}},"required":["text"]}}]}}\n' "$id"
            ;;
        tools/call)
            # the value is already JSON-escaped on the wire, so it can be re-embedded as-is
            text=$(printf '%s' "$line" | sed -En 's/.*"text":"((\\.|[^"\\])*)".*/\1/p')
            printf '{"jsonrpc":"2.0","id":%s,"result":{"content":[{"type":"text","text":"%s"}]}}\n' "$id" "$text"
            ;;
        *)
            # notifications have no id and expect no reply
            ;;
    esac
done

Save it as mcp-echo.sh, chmod +x it, then declare it in a Cursor-compatible config file mcp.json:

{
  "mcpServers": {
    "sh": { "command": "/path/to/mcp-echo.sh" }
  }
}

llama-server -m model.gguf --mcp-servers-config mcp.json

The server spawns the script at startup, does the MCP handshake, and exposes the tool as sh_echo next to the built-in server tools: it shows up in the WebUI tools list and on GET /tools, and the model can call it like any other tool. You can also poke it directly:

$ curl -s http://127.0.0.1:8080/tools -H "Content-Type: application/json" \
    -d '{"tool":"sh_echo","params":{"text":"it works"}}'
{"plain_text_response":"it works"}

Since it is just stdio, the same skeleton wraps any local command. For example replacing the echo line with pmset -g batt gives the model live battery status on a Mac with no bash/system access and no C++ rebuild needed.

Beyond the toy example, the point is that any existing software, in any language, can be safely wired into llama-server this way: the process boundary is the sandbox, and only the tools you declare are exposed to the model.

[ServeurpersoCom]

It also works on Windows, using a PowerShell script as an MCP server. That's what I tested to track down the issue with subprocess.h.
And remember what I used to do with the browser a while back? This is exactly what I needed to make that shareable. You can also control smart home devices; it's a feature that makes llama.cpp incredibly hackable for enthusiasts, without them having to write C++ code, offering an easier way to interface with the outside world (no need to host a server at home like I do).

[rickgonzalez]

For your records - if useful. We picked this one as an interesting and meaningful PR to run a scan on. This is the Lenzon explainer that we also made a video of during testing of our open source tool - https://www.lenzon.ai/viewer/cms3bz2jn000cpdiqikyr5k7s?voice=google-chirp3