diff --git a/common/arg.cpp b/common/arg.cpp index a287b907d490..9753441313a7 100644 --- a/common/arg.cpp +++ b/common/arg.cpp @@ -850,8 +850,9 @@ static bool common_params_parse_ex(int argc, char ** argv, common_params_context params.kv_overrides.back().key[0] = 0; } - if (!params.server_tools.empty() && !params.cors_origins_explicit) { - LOG_WRN("server tools are enabled, using localhost as default CORS origin (change via --cors-origins)\n"); + const bool mcp_enabled = !params.mcp_servers_config.empty() || !params.mcp_servers_json.empty(); + if ((!params.server_tools.empty() || mcp_enabled) && !params.cors_origins_explicit) { + LOG_WRN("server tools or MCP servers are enabled, using localhost as default CORS origin (change via --cors-origins)\n"); params.cors_origins = "localhost"; } @@ -3261,6 +3262,22 @@ common_params_context common_params_parser_init(common_params & params, llama_ex params.server_tools = parse_csv_row(value); } ).set_examples({LLAMA_EXAMPLE_SERVER}).set_env("LLAMA_ARG_TOOLS")); + add_opt(common_arg( + {"--mcp-servers-config"}, "PATH", + "experimental: path to JSON file with MCP server definitions (Cursor-compatible format) - do not enable in untrusted environments (default: none)\n" + "note: for security reasons, this will limit --cors-origins to localhost by default", + [](common_params & params, const std::string & value) { + params.mcp_servers_config = value; + } + ).set_examples({LLAMA_EXAMPLE_SERVER}).set_env("LLAMA_ARG_MCP_SERVERS_CONFIG")); + add_opt(common_arg( + {"--mcp-servers-json"}, "JSON", + "experimental: inline JSON with MCP server definitions (Cursor-compatible format) - do not enable in untrusted environments (default: none)\n" + "note: for security reasons, this will limit --cors-origins to localhost by default", + [](common_params & params, const std::string & value) { + params.mcp_servers_json = value; + } + ).set_examples({LLAMA_EXAMPLE_SERVER}).set_env("LLAMA_ARG_MCP_SERVERS_JSON")); add_opt(common_arg( {"-ag", "--agent"}, {"-no-ag", "--no-agent"}, diff --git a/common/common.h b/common/common.h index cb1827f5dd24..e7a0e2c29bcb 100644 --- a/common/common.h +++ b/common/common.h @@ -668,6 +668,10 @@ struct common_params { // enable built-in tools std::vector server_tools; + // MCP server configs (Cursor-compatible JSON) + std::string mcp_servers_config; // path to JSON file with MCP server definitions + std::string mcp_servers_json; // inline JSON with MCP server definitions + // router server configs std::string models_dir = ""; // directory containing models for the router server std::string models_preset = ""; // directory containing model presets for the router server diff --git a/tools/server/server-common.h b/tools/server/server-common.h index 74780b96e2e6..4969fe73cbb2 100644 --- a/tools/server/server-common.h +++ b/tools/server/server-common.h @@ -393,6 +393,10 @@ struct server_pipe { std::atomic writer_closed{false}; std::atomic reader_closed{false}; + // 0 = unbounded (default) + // > 0, write() drops the oldest item once the queue is full + size_t max_size = 0; + void close_write() { writer_closed.store(true, std::memory_order_relaxed); cv.notify_all(); @@ -428,6 +432,11 @@ struct server_pipe { if (reader_closed.load()) { return false; // broken pipe } + if (max_size > 0) { + while (queue.size() >= max_size) { + queue.pop(); // drop oldest to stay bounded + } + } queue.push(std::move(data)); cv.notify_one(); return true; diff --git a/tools/server/server-mcp.cpp b/tools/server/server-mcp.cpp index bf209c3e3a36..22b64040f257 100644 --- a/tools/server/server-mcp.cpp +++ b/tools/server/server-mcp.cpp @@ -2,30 +2,113 @@ #include +#include +#include #include -#include #include +#include +#include +#include #if defined(_WIN32) +# include # include #else +# include +# include +# include +# include extern char ** environ; #endif -// -// server_mcp_server_config -// - -std::vector server_mcp_server_config::parse_from_file(const std::string & path) { - std::ifstream f(path); +// read NDJSON lines from a child pipe, calling on_line per line until `running` clears, EOF/error, or on_line returns false. +// polled, not blocking: a grandchild can inherit the pipe's write end and hold it open (terminate() kills only the direct child), so a blocking read would hang teardown on an EOF that never comes. +static void mcp_pump_ndjson(FILE * f, std::atomic & running, + const std::function & on_line) { if (!f) { - throw std::runtime_error("failed to open MCP config file: " + path); + return; + } + const int poll_ms = 50; +#if defined(_WIN32) + HANDLE h = (HANDLE) _get_osfhandle(_fileno(f)); +#else + int fd = fileno(f); + int fl = fcntl(fd, F_GETFL, 0); + if (fl >= 0) { + fcntl(fd, F_SETFL, fl | O_NONBLOCK); + } +#endif + std::string buf; + char chunk[4096]; + while (running.load()) { + size_t n = 0; +#if defined(_WIN32) + DWORD avail = 0; + if (!PeekNamedPipe(h, NULL, 0, NULL, &avail, NULL)) { + break; // pipe broken / child gone + } + if (avail == 0) { + std::this_thread::sleep_for(std::chrono::milliseconds(poll_ms)); + continue; + } + DWORD to_read = avail < (DWORD) sizeof(chunk) ? avail : (DWORD) sizeof(chunk); + DWORD got = 0; + if (!ReadFile(h, chunk, to_read, &got, NULL) || got == 0) { + break; + } + n = (size_t) got; +#else + struct pollfd pfd; + pfd.fd = fd; + pfd.events = POLLIN; + pfd.revents = 0; + int pr = poll(&pfd, 1, poll_ms); + if (pr < 0) { + if (errno == EINTR) { + continue; + } + break; + } + if (pr == 0) { + continue; // timeout -> re-check running + } + if (pfd.revents & (POLLERR | POLLNVAL)) { + break; + } + ssize_t r = read(fd, chunk, sizeof(chunk)); + if (r < 0) { + if (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK) { + continue; + } + break; + } + if (r == 0) { + break; // EOF: child (and any pipe writers) closed the stream + } + n = (size_t) r; +#endif + buf.append(chunk, n); + size_t pos; + while ((pos = buf.find('\n')) != std::string::npos) { + std::string line = buf.substr(0, pos); + buf.erase(0, pos + 1); + if (!line.empty() && line.back() == '\r') { + line.pop_back(); + } + if (line.empty()) { + continue; + } + if (!on_line(std::move(line))) { + return; + } + } } - json j; - f >> j; - return parse_cursor_format(j); } +// +// server_mcp_server_config +// + std::vector server_mcp_server_config::parse_from_json(const std::string & json_str) { return parse_cursor_format(json::parse(json_str)); } @@ -107,8 +190,7 @@ json server_mcp_transport::send_rpc(const json & request, const std::function std::string { + DWORD n = SearchPathW(NULL, wcmd.c_str(), ext, cap, buf, NULL); + return (n > 0 && n < cap) ? windows_wide_to_utf8(buf, (int) n) : std::string(); + }; + + std::string found = search(NULL); // exact path / already-extensioned / .exe on PATH + if (!found.empty()) { + return found; + } + + std::wstring pathext; + DWORD need = GetEnvironmentVariableW(L"PATHEXT", NULL, 0); + if (need > 0) { + pathext.resize(need); + DWORD got = GetEnvironmentVariableW(L"PATHEXT", &pathext[0], need); + pathext.resize(got); + } + if (pathext.empty()) { + pathext = L".COM;.EXE;.BAT;.CMD"; + } + for (size_t start = 0; start <= pathext.size();) { + size_t sep = pathext.find(L';', start); + std::wstring ext = pathext.substr(start, sep == std::wstring::npos ? std::wstring::npos : sep - start); + if (!ext.empty()) { + found = search(ext.c_str()); + if (!found.empty()) { + return found; + } + } + if (sep == std::wstring::npos) { + break; + } + start = sep + 1; + } + return command; // give up and let subprocess.h report the spawn error +#else + return command; +#endif // _WIN32 +} + static std::vector mcp_parent_env() { std::vector env; #if defined(_WIN32) - LPCH block = GetEnvironmentStringsA(); + LPWCH block = GetEnvironmentStringsW(); if (block) { - for (LPCH e = block; *e; e += strlen(e) + 1) { - env.emplace_back(e); + for (LPWCH e = block; *e; e += wcslen(e) + 1) { + env.emplace_back(windows_wide_to_utf8(e, -1)); } - FreeEnvironmentStringsA(block); + FreeEnvironmentStringsW(block); } #else if (environ) { @@ -266,6 +424,8 @@ static std::vector mcp_build_env(const std::map argv_s; - argv_s.push_back(config.command); + argv_s.push_back(mcp_resolve_command(config.command)); argv_s.insert(argv_s.end(), config.args.begin(), config.args.end()); int options = subprocess_option_no_window | subprocess_option_search_user_path; @@ -327,33 +487,77 @@ bool server_mcp_stdio::is_alive() const { } void server_mcp_stdio::reader_loop() { - std::string buf; - char chunk[4096]; - for (;;) { - size_t n = fread(chunk, 1, sizeof(chunk), proc->out); - if (n == 0) { - break; // EOF or error - } - buf.append(chunk, n); + mcp_pump_ndjson(proc->out, running, [this](std::string && line) { + return from_server.write(std::move(line)); // false => consumer gone, stop + }); + running.store(false); + to_server.close_write(); // stop the writer + from_server.close_write(); // EOF to any waiting caller +} - size_t pos; - while ((pos = buf.find('\n')) != std::string::npos) { - std::string line = buf.substr(0, pos); - buf.erase(0, pos + 1); - if (!line.empty() && line.back() == '\r') { - line.pop_back(); +// write all of `data` to child stdin, non-blocking and polled so teardown never hangs (a grandchild can hold the read end of a full pipe open). returns false on error/close/shutdown. +static bool mcp_write_all(FILE * f, const std::string & data, std::atomic & running) { + if (!f) { + return false; + } + size_t total = 0; +#if defined(_WIN32) + HANDLE h = (HANDLE) _get_osfhandle(_fileno(f)); + DWORD nowait = PIPE_NOWAIT; + SetNamedPipeHandleState(h, &nowait, NULL, NULL); + while (total < data.size() && running.load()) { + DWORD written = 0; + BOOL ok = WriteFile(h, data.data() + total, (DWORD) (data.size() - total), &written, NULL); + if (ok && written > 0) { + total += written; + continue; + } + if (!ok) { + DWORD err = GetLastError(); + if (err != ERROR_NO_DATA && err != ERROR_PIPE_BUSY) { + return false; } - if (line.empty()) { + } + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + } +#else + int fd = fileno(f); + int fl = fcntl(fd, F_GETFL, 0); + if (fl >= 0) { + fcntl(fd, F_SETFL, fl | O_NONBLOCK); + } + while (total < data.size() && running.load()) { + ssize_t n = write(fd, data.data() + total, data.size() - total); + if (n > 0) { + total += (size_t) n; + continue; + } + if (n == 0) { + return false; + } + if (errno == EINTR) { + continue; + } + if (errno != EAGAIN && errno != EWOULDBLOCK) { + return false; + } + struct pollfd pfd; + pfd.fd = fd; + pfd.events = POLLOUT; + pfd.revents = 0; + int pr = poll(&pfd, 1, 50); + if (pr < 0) { + if (errno == EINTR) { continue; } - if (!from_server.write(std::move(line))) { - return; // consumer gone - } + return false; + } + if (pfd.revents & (POLLERR | POLLNVAL | POLLHUP)) { + return false; } } - running.store(false); - to_server.close_write(); // stop the writer - from_server.close_write(); // EOF to any waiting caller +#endif + return total == data.size(); } void server_mcp_stdio::writer_loop() { @@ -361,8 +565,8 @@ void server_mcp_stdio::writer_loop() { std::string msg; while (to_server.read(msg, should_stop)) { msg.push_back('\n'); - if (fwrite(msg.data(), 1, msg.size(), proc->in) != msg.size() || fflush(proc->in) != 0) { - break; // child gone + if (!mcp_write_all(proc->in, msg, running)) { + break; // child gone or shutting down } } running.store(false); @@ -372,35 +576,18 @@ void server_mcp_stdio::writer_loop() { void server_mcp_stdio::errlog_loop() { static constexpr size_t ERR_TAIL_MAX = 4096; - std::string buf; - char chunk[4096]; - for (;;) { - size_t n = fread(chunk, 1, sizeof(chunk), proc->err); - if (n == 0) { - break; + // drain stderr (an undrained pipe blocks the child): + // log it, and keep a bounded tail for reporting when the server dies + mcp_pump_ndjson(proc->err, running, [this](std::string && line) { + SRV_DBG("MCP '%s' stderr: %s\n", name.c_str(), line.c_str()); + std::lock_guard lk(err_mu); + err_tail += line; + err_tail += '\n'; + if (err_tail.size() > ERR_TAIL_MAX) { + err_tail.erase(0, err_tail.size() - ERR_TAIL_MAX); } - buf.append(chunk, n); - - size_t pos; - while ((pos = buf.find('\n')) != std::string::npos) { - std::string line = buf.substr(0, pos); - buf.erase(0, pos + 1); - if (!line.empty() && line.back() == '\r') { - line.pop_back(); - } - if (line.empty()) { - continue; - } - SRV_DBG("MCP '%s' stderr: %s\n", name.c_str(), line.c_str()); - - std::lock_guard lk(err_mu); - err_tail += line; - err_tail += '\n'; - if (err_tail.size() > ERR_TAIL_MAX) { - err_tail.erase(0, err_tail.size() - ERR_TAIL_MAX); - } - } - } + return true; + }); } void server_mcp_stdio::join_pumps() { @@ -428,9 +615,6 @@ void server_mcp_stdio::join_pumps() { static constexpr int MCP_COOLDOWN_SECONDS = 5; -server_mcp::server_mcp(std::vector configs) - : configs(std::move(configs)) {} - server_mcp::~server_mcp() { shutdown(); @@ -464,7 +648,35 @@ const server_mcp_server_config * server_mcp::find_config(const std::string & nam return nullptr; } -void server_mcp::start() { +void server_mcp::start(const common_params & params) { + auto append = [this](const std::string & json_str) { + try { + auto parsed = server_mcp_server_config::parse_from_json(json_str); + if (parsed.empty()) { + SRV_WRN("%s", "MCP config: no servers found in JSON\n"); + } + configs.insert(configs.end(), std::make_move_iterator(parsed.begin()), std::make_move_iterator(parsed.end())); + } catch (const std::exception & e) { + throw std::runtime_error(std::string("failed to parse MCP config JSON: ") + e.what()); + } + }; + if (!params.mcp_servers_config.empty()) { + std::ifstream f = fs_open_ifstream(params.mcp_servers_config, std::ios::in); + if (!f) { + throw std::runtime_error("failed to open MCP config file: " + params.mcp_servers_config); + } + std::stringstream ss; + ss << f.rdbuf(); + append(ss.str()); + } + if (!params.mcp_servers_json.empty()) { + append(params.mcp_servers_json); + } + + if (configs.empty()) { + return; + } + auto should_stop = [this]() { return stopping.load(); }; std::vector discovered; diff --git a/tools/server/server-mcp.h b/tools/server/server-mcp.h index f5488bf31a81..ca4f5abc02c1 100644 --- a/tools/server/server-mcp.h +++ b/tools/server/server-mcp.h @@ -24,8 +24,7 @@ struct server_mcp_server_config { std::string cwd; int timeout_ms = 30000; // per-tool-call timeout - // from_file/from_json throw on I/O or parse errors; a missing "mcpServers" yields an empty list, and entries without a "command" are skipped - static std::vector parse_from_file(const std::string & path); + // throw on parse errors; missing "mcpServers" yields an empty list; entries without a "command" are skipped static std::vector parse_from_json(const std::string & json_str); static std::vector parse_cursor_format(const json & j); }; @@ -44,8 +43,8 @@ struct server_mcp_tool_def { // caller --send_rpc--> to_server --[writer]--> framing --> server // caller <--send_rpc-- from_server <--[reader]-- framing <-- server // -// Each queue item is one complete serialized JSON message -// A subclass owns the byte I/O and framing (stdio: NDJSON); the base owns json (parse/dump) and the JSON-RPC session (handshake, id correlation). +// each queue item is one complete serialized JSON message. +// subclass owns byte I/O and framing; base owns JSON and the JSON-RPC session (handshake, id correlation). // struct server_mcp_transport { @@ -70,7 +69,7 @@ struct server_mcp_transport { const std::function & should_stop); protected: - // per-transport, not shared: send_rpc() holds it across the wait for a reply, so a shared lock would stall every server behind one slow call all members below are touched only under it + // per-transport: send_rpc() holds it across the reply wait, so sharing it would stall every server behind one slow call. guards all members below. std::mutex rpc_mutex; uint64_t next_id = 1; // reset to 1 per (re)spawn bool initialized = false; @@ -79,11 +78,11 @@ struct server_mcp_transport { // both assume rpc_mutex is already held by the public caller bool ensure_init(const std::function & should_stop); // initialize handshake, once - json send_rpc(const json & request, const std::function & should_stop); // returns the reply, or an {"error": ...} object + json send_rpc(const json & request, const std::function & should_stop); // returns the reply or an {"error": ...} }; // -// server_mcp_stdio: child process, NDJSON JSON-RPC over stdio (stderr inherited) +// server_mcp_stdio: child process, NDJSON JSON-RPC over stdio (stderr drained to the debug log) // struct server_mcp_stdio : server_mcp_transport { @@ -120,32 +119,40 @@ struct server_mcp_stdio : server_mcp_transport { // // server_mcp -// manager lives inside main_server(); declare it before the HTTP context so it outlives every /tools handler. +// declare before the HTTP context so it outlives every /tools handler. // class server_mcp { public: - explicit server_mcp(std::vector configs); + server_mcp() = default; ~server_mcp(); - // spawn each server once, list its tools, shut it down. failures are logged, not fatal. - void start(); + // parse the MCP config from params (file and/or inline JSON), + // then spawn each server once, list its tools, and shut it down + // throws on config parse errors; spawn failures are logged. + void start(const common_params & params); + + // true until start() has parsed at least one server from the config + bool empty() const { return configs.empty(); } std::vector list_tools() const; - // lazily (re)spawns the transport. returns the MCP result or an {"error": ...} object. should_stop is OR-ed with the manager's cancel flag. + // lazily (re)spawns the transport. returns the MCP result or an {"error": ...}. should_stop is OR-ed with the manager's cancel flag. json call_tool(const std::string & server_name, const std::string & tool_name, const json & arguments, const std::function & should_stop = nullptr); // flip the cancel flag so in-flight calls return; blocking teardown is in the destructor. call before the HTTP server drains. + // note: multiple calls are idempotent void shutdown(); private: std::vector configs; mutable std::mutex mutex; // guards transports, dead_servers, registry + + // shared_ptr: call_tool() hands a transport to the caller and drops the lock for the blocking RPC, so a concurrent evict/respawn must not destroy it mid-call std::map> transports; std::map dead_servers; // spawn-failure cooldown std::vector registry; diff --git a/tools/server/server-tools.cpp b/tools/server/server-tools.cpp index 9eb57abaea15..f5de1735fd11 100644 --- a/tools/server/server-tools.cpp +++ b/tools/server/server-tools.cpp @@ -9,10 +9,10 @@ #include #include #include -#include #include #include #include +#include namespace fs = std::filesystem; @@ -24,7 +24,7 @@ json server_tool::to_json() const { return { {"display_name", display_name}, {"tool", name}, - {"type", "builtin"}, + {"type", type()}, {"permissions", json{ {"write", permission_write} }}, @@ -1102,6 +1102,54 @@ struct server_tools_res : server_http_res { } }; +// +// server_mcp_tool: exposes one tool from a running MCP server as a server_tool. +// +struct server_mcp_tool : server_tool { + std::string server_name; + std::string tool_name; + server_mcp_tool_def def; + server_mcp & mcp_mgr; + + server_mcp_tool(server_mcp_tool_def d, server_mcp & mgr) + : server_name(d.server_name) + , tool_name(d.name) + , def(std::move(d)) + , mcp_mgr(mgr) + { + name = server_name + "_" + tool_name; + display_name = name; + permission_write = false; + support_stream = false; + } + + std::string type() const override { return "mcp"; } + + json get_definition() const override { + json schema = def.input_schema; + if (schema.is_null() || !schema.is_object()) { + schema = json::object(); + } + return { + {"type", "function"}, + {"function", { + {"name", name}, + {"description", def.description}, + {"parameters", schema}, + }}, + }; + } + + 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 should_stop = nullptr; + if (st) { + should_stop = [st]() { return !st->alive(); }; + } + return mcp_mgr.call_tool(server_name, tool_name, params, should_stop); + } +}; + static server_tool & find_tool(std::vector> & tools, const std::string & name, bool require_stream) { for (auto & t : tools) { if (t->name == name) { @@ -1130,7 +1178,8 @@ static std::vector> build_tools() { return tools; } -void server_tools::setup(const std::vector & enabled_tools) { +void server_tools::setup(const std::vector & enabled_tools, + server_mcp & mcp_mgr) { if (!enabled_tools.empty()) { std::unordered_set enabled_set(enabled_tools.begin(), enabled_tools.end()); auto all_tools = build_tools(); @@ -1161,6 +1210,29 @@ void server_tools::setup(const std::vector & enabled_tools) { } } + // append MCP tools, skipping any that collide with a built-in or another MCP tool of the same "_" name + if (!mcp_mgr.empty()) { + std::unordered_set seen_names; + for (auto & t : tools) { + seen_names.insert(t->name); + } + size_t n_added = 0; + for (const auto & def : mcp_mgr.list_tools()) { + std::string mcp_name = def.server_name + "_" + def.name; + if (seen_names.count(mcp_name)) { + SRV_WRN("MCP tool \"%s\" from server \"%s\" collides with an existing tool, skipping\n", + mcp_name.c_str(), def.server_name.c_str()); + continue; + } + seen_names.insert(mcp_name); + tools.push_back(std::make_unique(def, mcp_mgr)); + n_added++; + } + if (n_added > 0) { + SRV_INF("Added %zu MCP tools\n", n_added); + } + } + handle_get = [this](const server_http_req &) -> server_http_res_ptr { auto res = std::make_unique(); try { diff --git a/tools/server/server-tools.h b/tools/server/server-tools.h index 6f6528f484f8..601399ee9392 100644 --- a/tools/server/server-tools.h +++ b/tools/server/server-tools.h @@ -3,9 +3,11 @@ #include "server-common.h" #include "server-http.h" #include "server-queue.h" +#include "server-mcp.h" #include #include +#include struct server_tool { std::string name; @@ -15,6 +17,7 @@ struct server_tool { virtual ~server_tool() = default; virtual json get_definition() const = 0; + virtual std::string type() const { return "builtin"; } struct stream { server_response & qr; @@ -34,7 +37,8 @@ struct server_tools { server_response queue_res; std::atomic res_id{0}; - void setup(const std::vector & enabled_tools); + void setup(const std::vector & enabled_tools, + server_mcp & mcp_mgr); server_http_context::handler_t handle_get; server_http_context::handler_t handle_post; diff --git a/tools/server/server.cpp b/tools/server/server.cpp index 20effbb14851..b6fef99e8747 100644 --- a/tools/server/server.cpp +++ b/tools/server/server.cpp @@ -88,6 +88,11 @@ static server_http_context::handler_t ex_wrapper(server_http_context::handler_t int llama_server(int argc, char ** argv) { std::setlocale(LC_NUMERIC, "C"); +#ifndef _WIN32 + // Ignore SIGPIPE so the server does not crash if an MCP child exits while we are writing to its stdin + signal(SIGPIPE, SIG_IGN); +#endif + // own arguments required by this example common_params params; @@ -157,6 +162,9 @@ int llama_server(common_params & params, int argc, char ** argv) { params.model_alias.insert(model_name); } + // note: this is guaranteed to out-live ctx_http and tools + server_mcp mcp_mgr; + // struct that contains llama context and inference server_context ctx_server; @@ -326,17 +334,28 @@ int llama_server(common_params & params, int argc, char ** argv) { ctx_http.post("/cors-proxy", ex_wrapper(res_403)); } - // EXPERIMENTAL built-in tools - if (!params.server_tools.empty()) { + try { + mcp_mgr.start(params); + } catch (const std::exception & e) { + SRV_ERR("MCP starting failed: %s\n", e.what()); + return 1; + } + + if (!params.server_tools.empty() || !mcp_mgr.empty()) { try { - tools.setup(params.server_tools); + tools.setup(params.server_tools, mcp_mgr); } catch (const std::exception & e) { SRV_ERR("tools setup failed: %s\n", e.what()); return 1; } ctx_http.get ("/tools", ex_wrapper(tools.handle_get)); ctx_http.post("/tools", ex_wrapper(tools.handle_post)); - warn_names.push_back("built-in tools (experimental)"); + if (!params.server_tools.empty()) { + warn_names.push_back("built-in tools (experimental)"); + } + if (!mcp_mgr.empty()) { + warn_names.push_back("MCP servers (experimental)"); + } } else { ctx_http.get ("/tools", ex_wrapper(res_403)); ctx_http.post("/tools", ex_wrapper(res_403)); @@ -378,7 +397,7 @@ int llama_server(common_params & params, int argc, char ** argv) { if (is_router_server) { SRV_INF("%s", "starting server in router mode. models will be automatically loaded on-demand\n"); - clean_up = [&models_routes]() { + clean_up = [&models_routes, &mcp_mgr]() { SRV_INF("%s: cleaning up before exit...\n", __func__); // stop the session GC first, it finalizes live sessions and wakes pending readers server_stream_session_manager_stop(); @@ -386,6 +405,7 @@ int llama_server(common_params & params, int argc, char ** argv) { models_routes->stopping.store(true); // maybe redundant, but just to be safe models_routes->models.unload_all(); } + mcp_mgr.shutdown(); llama_backend_free(); }; @@ -401,17 +421,19 @@ int llama_server(common_params & params, int argc, char ** argv) { // important to disconnect any SSE clients models_routes->stopping.store(true); } + mcp_mgr.shutdown(); ctx_http.stop(); }; } else { // setup clean up function, to be called before exit - clean_up = [&ctx_http, &ctx_server]() { + clean_up = [&ctx_http, &ctx_server, &mcp_mgr]() { SRV_INF("%s: cleaning up before exit...\n", __func__); // stop the session GC first, it finalizes live sessions and wakes pending readers server_stream_session_manager_stop(); ctx_http.stop(); ctx_server.terminate(); + mcp_mgr.shutdown(); llama_backend_free(); }; @@ -444,6 +466,7 @@ int llama_server(common_params & params, int argc, char ** argv) { SRV_INF("%s", "model loaded\n"); shutdown_handler = [&](int) { + mcp_mgr.shutdown(); // this will unblock start_loop() ctx_server.terminate(); }; diff --git a/tools/server/tests/fixtures/mcp_burst_server.py b/tools/server/tests/fixtures/mcp_burst_server.py new file mode 100644 index 000000000000..fd66b6973713 --- /dev/null +++ b/tools/server/tests/fixtures/mcp_burst_server.py @@ -0,0 +1,124 @@ +#!/usr/bin/env python3 +""" +Minimal MCP server that writes notification + response in a single write() with no flush. +This reproduces the buffering bug where read_message() can strand the response. +""" +import json +import sys +import os + +TOOLS = [ + { + "name": "echo", + "description": "Echo back the input message", + "inputSchema": { + "type": "object", + "properties": { + "message": {"type": "string"} + }, + "required": ["message"] + } + } +] + +def handle_initialize(params, req_id): + return { + "jsonrpc": "2.0", + "id": req_id, + "result": { + "protocolVersion": "2024-11-05", + "capabilities": {"tools": {}}, + "serverInfo": {"name": "burst-test", "version": "1.0"} + } + } + +def handle_tools_list(params, req_id): + return { + "jsonrpc": "2.0", + "id": req_id, + "result": {"tools": TOOLS} + } + +def handle_tools_call(params, req_id): + tool_name = params.get("name") + arguments = params.get("arguments", {}) + + if tool_name == "echo": + message = arguments.get("message", "") + notif = { + "jsonrpc": "2.0", + "method": "notifications/progress", + "params": {"progress": 50, "total": 100} + } + response = { + "jsonrpc": "2.0", + "id": req_id, + "result": { + "content": [{"type": "text", "text": f"echo: {message}"}] + } + } + # Single os.write() call: both lines land in one pipe packet atomically. + # This is the key difference from mcp_malformed_server.py which flushes between writes. + data = (json.dumps(notif) + "\n" + json.dumps(response) + "\n").encode("utf-8") + os.write(sys.stdout.fileno(), data) + return None # already written + else: + response = { + "jsonrpc": "2.0", + "id": req_id, + "error": {"code": -32601, "message": f"Unknown tool: {tool_name}"} + } + return response + +HANDLERS = { + "initialize": handle_initialize, + "tools/list": handle_tools_list, + "tools/call": handle_tools_call, +} + +# notifications have no id and don't expect a response +NOTIFICATION_HANDLERS = { + "notifications/initialized": lambda params, req_id: None, +} + +def main(): + # Use line-buffered text mode for regular responses, but the burst write + # uses os.write() directly to guarantee a single kernel write(). + sys.stdout = os.fdopen(sys.stdout.fileno(), "w", buffering=1) + sys.stderr = os.fdopen(sys.stderr.fileno(), "w", buffering=1) + + for line in sys.stdin: + line = line.strip() + if not line: + continue + try: + request = json.loads(line) + except json.JSONDecodeError: + continue + + method = request.get("method") + req_id = request.get("id") + params = request.get("params", {}) + + # Check notification handlers first (no response expected) + if not req_id and method in NOTIFICATION_HANDLERS: + NOTIFICATION_HANDLERS[method](params, req_id) + continue + + handler = HANDLERS.get(method) + if handler: + response = handler(params, req_id) + if response is not None: + sys.stdout.write(json.dumps(response) + "\n") + sys.stdout.flush() + else: + response = { + "jsonrpc": "2.0", + "id": req_id, + "error": {"code": -32601, "message": f"Method not found: {method}"} + } + sys.stdout.write(json.dumps(response) + "\n") + sys.stdout.flush() + +if __name__ == "__main__": + main() diff --git a/tools/server/tests/fixtures/mcp_crash_server.py b/tools/server/tests/fixtures/mcp_crash_server.py new file mode 100644 index 000000000000..7f389c6bc0a3 --- /dev/null +++ b/tools/server/tests/fixtures/mcp_crash_server.py @@ -0,0 +1,114 @@ +#!/usr/bin/env python3 +""" +MCP server that crashes after receiving a specific tool call. +""" +import json +import sys +import os + +def handle_initialize(params, req_id): + return { + "jsonrpc": "2.0", + "id": req_id, + "result": { + "protocolVersion": "2024-11-05", + "capabilities": {"tools": {}}, + "serverInfo": {"name": "crash-test", "version": "1.0"} + } + } + +def handle_tools_list(params, req_id): + return { + "jsonrpc": "2.0", + "id": req_id, + "result": { + "tools": [ + { + "name": "echo", + "description": "Echo back the input message", + "inputSchema": { + "type": "object", + "properties": { + "message": {"type": "string"} + } + } + }, + { + "name": "crash", + "description": "Crash the server", + "inputSchema": { + "type": "object", + "properties": {} + } + } + ] + } + } + +def handle_tools_call(params, req_id): + tool_name = params.get("name") + arguments = params.get("arguments", {}) + + if tool_name == "echo": + message = arguments.get("message", "") + return { + "jsonrpc": "2.0", + "id": req_id, + "result": { + "content": [{"type": "text", "text": f"echo: {message}"}] + } + } + elif tool_name == "crash": + # Send a partial response then exit + sys.stdout.write(json.dumps({"jsonrpc": "2.0", "id": req_id, "result": {"content": [{"type": "text", "text": "crashing..."}]}}) + "\n") + sys.stdout.flush() + os._exit(1) + else: + return { + "jsonrpc": "2.0", + "id": req_id, + "error": {"code": -32601, "message": f"Unknown tool: {tool_name}"} + } + +HANDLERS = { + "initialize": handle_initialize, + "tools/list": handle_tools_list, + "tools/call": handle_tools_call, +} + +def main(): + sys.stdout = os.fdopen(sys.stdout.fileno(), "w", buffering=1) + sys.stderr = os.fdopen(sys.stderr.fileno(), "w", buffering=1) + + for line in sys.stdin: + line = line.strip() + if not line: + continue + try: + request = json.loads(line) + except json.JSONDecodeError: + continue + + method = request.get("method") + req_id = request.get("id") + params = request.get("params", {}) + + # JSON-RPC 2.0: a message without an id is a notification and must not receive a response + if req_id is None: + continue + + handler = HANDLERS.get(method) + if handler: + response = handler(params, req_id) + else: + response = { + "jsonrpc": "2.0", + "id": req_id, + "error": {"code": -32601, "message": f"Method not found: {method}"} + } + + sys.stdout.write(json.dumps(response) + "\n") + sys.stdout.flush() + +if __name__ == "__main__": + main() diff --git a/tools/server/tests/fixtures/mcp_echo_server.py b/tools/server/tests/fixtures/mcp_echo_server.py new file mode 100755 index 000000000000..7acfb358881a --- /dev/null +++ b/tools/server/tests/fixtures/mcp_echo_server.py @@ -0,0 +1,164 @@ +#!/usr/bin/env python3 +""" +Minimal MCP server for testing. +Implements JSON-RPC 2.0 over stdio (line-delimited JSON). +""" +import json +import sys +import os + +# Ensure we use python3 from the current environment +if sys.platform == "win32": + # On Windows, we need to use the same python interpreter + pass + +TOOLS = [ + { + "name": "echo", + "description": "Echo back the input message", + "inputSchema": { + "type": "object", + "properties": { + "message": {"type": "string", "description": "Message to echo"} + }, + "required": ["message"] + } + }, + { + "name": "add", + "description": "Add two numbers", + "inputSchema": { + "type": "object", + "properties": { + "a": {"type": "number"}, + "b": {"type": "number"} + }, + "required": ["a", "b"] + } + }, + { + "name": "fail_once", + "description": "Fails on first call, succeeds on subsequent calls", + "inputSchema": { + "type": "object", + "properties": {} + } + } +] + +_state = {"fail_once_called": False} + +def handle_initialize(params, req_id): + return { + "jsonrpc": "2.0", + "id": req_id, + "result": { + "protocolVersion": "2024-11-05", + "capabilities": {"tools": {}}, + "serverInfo": {"name": "echo-test", "version": "1.0"} + } + } + +def handle_tools_list(params, req_id): + return { + "jsonrpc": "2.0", + "id": req_id, + "result": {"tools": TOOLS} + } + +def handle_tools_call(params, req_id): + tool_name = params.get("name") + arguments = params.get("arguments", {}) + + if tool_name == "echo": + message = arguments.get("message", "") + return { + "jsonrpc": "2.0", + "id": req_id, + "result": { + "content": [{"type": "text", "text": f"echo: {message}"}] + } + } + elif tool_name == "add": + a = arguments.get("a", 0) + b = arguments.get("b", 0) + return { + "jsonrpc": "2.0", + "id": req_id, + "result": { + "content": [{"type": "text", "text": str(a + b)}] + } + } + elif tool_name == "fail_once": + if not _state["fail_once_called"]: + _state["fail_once_called"] = True + return { + "jsonrpc": "2.0", + "id": req_id, + "error": {"code": -32000, "message": "transient error"} + } + return { + "jsonrpc": "2.0", + "id": req_id, + "result": { + "content": [{"type": "text", "text": "ok"}] + } + } + else: + return { + "jsonrpc": "2.0", + "id": req_id, + "error": {"code": -32602, "message": f"Unknown tool: {tool_name}"} + } + +def handle_ping(params, req_id): + return { + "jsonrpc": "2.0", + "id": req_id, + "result": {} + } + +HANDLERS = { + "initialize": handle_initialize, + "tools/list": handle_tools_list, + "tools/call": handle_tools_call, + "ping": handle_ping, +} + +def main(): + # Use unbuffered output + sys.stdout = os.fdopen(sys.stdout.fileno(), "w", buffering=1) + sys.stderr = os.fdopen(sys.stderr.fileno(), "w", buffering=1) + + for line in sys.stdin: + line = line.strip() + if not line: + continue + try: + request = json.loads(line) + except json.JSONDecodeError: + continue + + method = request.get("method") + req_id = request.get("id") + params = request.get("params", {}) + + # JSON-RPC 2.0: a message without an id is a notification and must not receive a response + if req_id is None: + continue + + handler = HANDLERS.get(method) + if handler: + response = handler(params, req_id) + else: + response = { + "jsonrpc": "2.0", + "id": req_id, + "error": {"code": -32601, "message": f"Method not found: {method}"} + } + + sys.stdout.write(json.dumps(response) + "\n") + sys.stdout.flush() + +if __name__ == "__main__": + main() diff --git a/tools/server/tests/fixtures/mcp_grandchild_server.py b/tools/server/tests/fixtures/mcp_grandchild_server.py new file mode 100644 index 000000000000..9defa904d61f --- /dev/null +++ b/tools/server/tests/fixtures/mcp_grandchild_server.py @@ -0,0 +1,100 @@ +#!/usr/bin/env python3 +""" +MCP server (NDJSON JSON-RPC over stdio) that spawns a long-lived grandchild which inherits +this process's stdin/stdout/stderr and keeps them open. + +This reproduces the reader-teardown deadlock: killing the direct MCP child (SIGKILL, which is +all subprocess_terminate() does) does NOT close the stdout/stderr pipe write ends, because the +grandchild still holds them. A server that reads those pipes with a blocking read would then +wait forever for an EOF that never arrives, hanging teardown (both warmup shutdown at startup +and process shutdown). The polled, running-aware reader must exit regardless. +""" +import json +import os +import subprocess +import sys + +# Spawn a grandchild that inherits our std handles (fds 0/1/2 = the MCP pipes) and lives well +# past any teardown in the tests. We do NOT redirect its stdio, so it keeps the pipe write ends +# open even after this process is killed. +subprocess.Popen([sys.executable, "-c", "import time; time.sleep(30)"]) + +TOOLS = [ + { + "name": "echo", + "description": "Echo back the input message", + "inputSchema": { + "type": "object", + "properties": {"message": {"type": "string", "description": "Message to echo"}}, + "required": ["message"], + }, + } +] + + +def handle_initialize(params, req_id): + return { + "jsonrpc": "2.0", + "id": req_id, + "result": { + "protocolVersion": "2024-11-05", + "capabilities": {"tools": {}}, + "serverInfo": {"name": "grandchild-test", "version": "1.0"}, + }, + } + + +def handle_tools_list(params, req_id): + return {"jsonrpc": "2.0", "id": req_id, "result": {"tools": TOOLS}} + + +def handle_tools_call(params, req_id): + if params.get("name") == "echo": + message = params.get("arguments", {}).get("message", "") + return { + "jsonrpc": "2.0", + "id": req_id, + "result": {"content": [{"type": "text", "text": f"echo: {message}"}]}, + } + return {"jsonrpc": "2.0", "id": req_id, "error": {"code": -32601, "message": "Unknown tool"}} + + +HANDLERS = { + "initialize": handle_initialize, + "tools/list": handle_tools_list, + "tools/call": handle_tools_call, +} + + +def main(): + sys.stdout = os.fdopen(sys.stdout.fileno(), "w", buffering=1) + sys.stderr = os.fdopen(sys.stderr.fileno(), "w", buffering=1) + + for line in sys.stdin: + line = line.strip() + if not line: + continue + try: + request = json.loads(line) + except json.JSONDecodeError: + continue + + method = request.get("method") + req_id = request.get("id") + params = request.get("params", {}) + + if req_id is None: + continue # notification, no response + + handler = HANDLERS.get(method) + if handler: + response = handler(params, req_id) + else: + response = {"jsonrpc": "2.0", "id": req_id, "error": {"code": -32601, "message": f"Method not found: {method}"}} + + sys.stdout.write(json.dumps(response) + "\n") + sys.stdout.flush() + + +if __name__ == "__main__": + main() diff --git a/tools/server/tests/fixtures/mcp_malformed_server.py b/tools/server/tests/fixtures/mcp_malformed_server.py new file mode 100644 index 000000000000..365595867b86 --- /dev/null +++ b/tools/server/tests/fixtures/mcp_malformed_server.py @@ -0,0 +1,109 @@ +#!/usr/bin/env python3 +""" +MCP server that sends malformed responses and notifications during requests. +""" +import json +import sys +import os + +def handle_initialize(params, req_id): + return { + "jsonrpc": "2.0", + "id": req_id, + "result": { + "protocolVersion": "2024-11-05", + "capabilities": {"tools": {}}, + "serverInfo": {"name": "malformed-test", "version": "1.0"} + } + } + +def handle_tools_list(params, req_id): + return { + "jsonrpc": "2.0", + "id": req_id, + "result": { + "tools": [ + { + "name": "echo", + "description": "Echo back the input message", + "inputSchema": { + "type": "object", + "properties": { + "message": {"type": "string"} + } + } + } + ] + } + } + +def handle_tools_call(params, req_id): + tool_name = params.get("name") + arguments = params.get("arguments", {}) + + if tool_name == "echo": + message = arguments.get("message", "") + # Send a notification first (no id field) + notif = { + "jsonrpc": "2.0", + "method": "notifications/progress", + "params": {"progress": 50, "total": 100} + } + sys.stdout.write(json.dumps(notif) + "\n") + sys.stdout.flush() + # Then send the actual response + return { + "jsonrpc": "2.0", + "id": req_id, + "result": { + "content": [{"type": "text", "text": f"echo: {message}"}] + } + } + else: + return { + "jsonrpc": "2.0", + "id": req_id, + "error": {"code": -32601, "message": f"Unknown tool: {tool_name}"} + } + +HANDLERS = { + "initialize": handle_initialize, + "tools/list": handle_tools_list, + "tools/call": handle_tools_call, +} + +def main(): + sys.stdout = os.fdopen(sys.stdout.fileno(), "w", buffering=1) + sys.stderr = os.fdopen(sys.stderr.fileno(), "w", buffering=1) + + for line in sys.stdin: + line = line.strip() + if not line: + continue + try: + request = json.loads(line) + except json.JSONDecodeError: + # Send malformed JSON response + sys.stdout.write("THIS IS NOT JSON\n") + sys.stdout.flush() + continue + + method = request.get("method") + req_id = request.get("id") + params = request.get("params", {}) + + handler = HANDLERS.get(method) + if handler: + response = handler(params, req_id) + else: + response = { + "jsonrpc": "2.0", + "id": req_id, + "error": {"code": -32601, "message": f"Method not found: {method}"} + } + + sys.stdout.write(json.dumps(response) + "\n") + sys.stdout.flush() + +if __name__ == "__main__": + main() diff --git a/tools/server/tests/fixtures/mcp_slow_server.py b/tools/server/tests/fixtures/mcp_slow_server.py new file mode 100644 index 000000000000..6b7f7184e527 --- /dev/null +++ b/tools/server/tests/fixtures/mcp_slow_server.py @@ -0,0 +1,132 @@ +#!/usr/bin/env python3 +""" +MCP server that sleeps before responding, for timeout testing. +""" +import json +import sys +import os +import time +import argparse + +TOOLS = [ + { + "name": "sleep", + "description": "Sleep for a given number of seconds", + "inputSchema": { + "type": "object", + "properties": { + "seconds": {"type": "number", "description": "Seconds to sleep"} + }, + "required": ["seconds"] + } + } +] + +def handle_initialize(params, req_id): + return { + "jsonrpc": "2.0", + "id": req_id, + "result": { + "protocolVersion": "2024-11-05", + "capabilities": {"tools": {}}, + "serverInfo": {"name": "slow-test", "version": "1.0"} + } + } + +def handle_tools_list(params, req_id): + return { + "jsonrpc": "2.0", + "id": req_id, + "result": {"tools": TOOLS} + } + +def handle_tools_call(params, req_id): + tool_name = params.get("name") + arguments = params.get("arguments", {}) + + if tool_name == "sleep": + seconds = arguments.get("seconds", 1) + time.sleep(seconds) + return { + "jsonrpc": "2.0", + "id": req_id, + "result": { + "content": [{"type": "text", "text": f"slept {seconds}s"}] + } + } + else: + return { + "jsonrpc": "2.0", + "id": req_id, + "error": {"code": -32601, "message": f"Unknown tool: {tool_name}"} + } + +HANDLERS = { + "initialize": handle_initialize, + "tools/list": handle_tools_list, + "tools/call": handle_tools_call, +} + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument("--delay", type=float, default=5.0, help="Delay in seconds for sleep tool") + args = parser.parse_args() + + # Override the sleep duration + global handle_tools_call + def handle_tools_call(params, req_id): + tool_name = params.get("name") + arguments = params.get("arguments", {}) + + if tool_name == "sleep": + seconds = arguments.get("seconds", args.delay) + time.sleep(seconds) + return { + "jsonrpc": "2.0", + "id": req_id, + "result": { + "content": [{"type": "text", "text": f"slept {seconds}s"}] + } + } + else: + return { + "jsonrpc": "2.0", + "id": req_id, + "error": {"code": -32601, "message": f"Unknown tool: {tool_name}"} + } + + sys.stdout = os.fdopen(sys.stdout.fileno(), "w", buffering=1) + sys.stderr = os.fdopen(sys.stderr.fileno(), "w", buffering=1) + + for line in sys.stdin: + line = line.strip() + if not line: + continue + try: + request = json.loads(line) + except json.JSONDecodeError: + continue + + method = request.get("method") + req_id = request.get("id") + params = request.get("params", {}) + + # JSON-RPC 2.0: a message without an id is a notification and must not receive a response + if req_id is None: + continue + + handler = HANDLERS.get(method) + if handler: + response = handler(params, req_id) + else: + response = { + "jsonrpc": "2.0", + "id": req_id, + "error": {"code": -32601, "message": f"Method not found: {method}"} + } + + sys.stdout.write(json.dumps(response) + "\n") + sys.stdout.flush() + +if __name__ == "__main__": + main() diff --git a/tools/server/tests/unit/test_mcp_servers.py b/tools/server/tests/unit/test_mcp_servers.py new file mode 100644 index 000000000000..de4aa36bcf47 --- /dev/null +++ b/tools/server/tests/unit/test_mcp_servers.py @@ -0,0 +1,718 @@ +#!/usr/bin/env python3 +""" +Tests for MCP server integration via the /tools endpoint. + +Invariants verified: +1. MCP tools appear in /tools listing when configured +2. MCP tools use : naming +3. MCP tools can be invoked and return correct results +4. Misconfigured MCP servers do not crash the server +5. Multiple MCP servers can be configured simultaneously +6. Warmup populates the tool list at startup +""" +import json +import os +import sys +import tempfile +import time + +import pytest + +from utils import * + +# Path to the test MCP server fixture +FIXTURES_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), "..", "fixtures") +MCP_ECHO_SERVER = os.path.join(FIXTURES_DIR, "mcp_echo_server.py") + +server: ServerProcess + + +def _mcp_config_json(servers: dict) -> str: + """Create a JSON config string for --mcp-servers-json.""" + return json.dumps({"mcpServers": servers}) + + +def _start_server_with_mcp(mcp_json: str, **kwargs) -> ServerProcess: + """Helper to start a router server with MCP config.""" + srv = ServerPreset.router() + srv.server_tools = "all" + srv.no_ui = True + srv.server_port = 8085 # avoid conflict with load_all() which uses 8080 + srv.mcp_servers_json = mcp_json + for k, v in kwargs.items(): + setattr(srv, k, v) + srv.start() + return srv + + +def test_mcp_tools_listed_in_tools_endpoint(): + """MCP tools should appear in GET /tools with server:tool naming.""" + global server + mcp_json = _mcp_config_json({ + "echo": { + "command": sys.executable, + "args": [MCP_ECHO_SERVER], + } + }) + server = _start_server_with_mcp(mcp_json) + + try: + res = server.make_request("GET", "/tools") + assert res.status_code == 200, res.body + + tools = res.body + assert isinstance(tools, list), f"Expected list, got {type(tools)}" + + # Find MCP tools - name is in "tool" field or definition.function.name + def get_tool_name(t): + return t.get("tool", "") or t.get("definition", {}).get("function", {}).get("name", "") + + mcp_tools = [t for t in tools if get_tool_name(t).startswith("echo_")] + assert len(mcp_tools) >= 2, f"Expected at least 2 echo_ tools, got {len(mcp_tools)}: {mcp_tools}" + + tool_names = {get_tool_name(t) for t in mcp_tools} + assert "echo_echo" in tool_names + assert "echo_add" in tool_names + + # Verify tool structure + echo_tool = next(t for t in mcp_tools if get_tool_name(t) == "echo_echo") + assert "description" in echo_tool or "definition" in echo_tool + finally: + server.stop() + + +def test_mcp_tool_invocation(): + """MCP tools should be callable via POST /tools and return correct results.""" + global server + mcp_json = _mcp_config_json({ + "echo": { + "command": sys.executable, + "args": [MCP_ECHO_SERVER], + } + }) + server = _start_server_with_mcp(mcp_json) + + try: + # Call echo_echo + res = server.make_request("POST", "/tools", data={ + "tool": "echo_echo", + "params": {"message": "hello world"} + }) + assert res.status_code == 200, res.body + body = res.body + assert "error" not in body, body + # The result format depends on the tool implementation + # For MCP tools, it should contain the tool result + assert "plain_text_response" in body or "result" in body or "content" in body, body + + # Call echo_add + res = server.make_request("POST", "/tools", data={ + "tool": "echo_add", + "params": {"a": 3, "b": 5} + }) + assert res.status_code == 200, res.body + body = res.body + assert "error" not in body, body + finally: + server.stop() + + +def test_mcp_bad_command_does_not_crash(): + """A misconfigured MCP server should not crash the llama-server.""" + global server + mcp_json = _mcp_config_json({ + "nonexistent": { + "command": "this_executable_does_not_exist_12345", + "args": [], + } + }) + server = _start_server_with_mcp(mcp_json) + + try: + # Server should still be healthy + res = server.make_request("GET", "/health") + assert res.status_code == 200, res.body + + # Builtin tools should still work + res = server.make_request("GET", "/tools") + assert res.status_code == 200, res.body + tools = res.body + # Should have builtin tools but no MCP tools from the bad server + mcp_tools = [t for t in tools if t.get("name", "").startswith("nonexistent_")] + assert len(mcp_tools) == 0, f"Expected no nonexistent_ tools, got {mcp_tools}" + finally: + server.stop() + + +def test_mcp_multiple_servers(): + """Multiple MCP servers can be configured simultaneously.""" + global server + mcp_json = _mcp_config_json({ + "echo": { + "command": sys.executable, + "args": [MCP_ECHO_SERVER], + }, + "echo2": { + "command": sys.executable, + "args": [MCP_ECHO_SERVER], + } + }) + server = _start_server_with_mcp(mcp_json) + + try: + res = server.make_request("GET", "/tools") + assert res.status_code == 200, res.body + + tools = res.body + + def get_tool_name(t): + return t.get("tool", "") or t.get("definition", {}).get("function", {}).get("name", "") + + echo_tools = [t for t in tools if get_tool_name(t).startswith("echo_")] + echo2_tools = [t for t in tools if get_tool_name(t).startswith("echo2_")] + + assert len(echo_tools) >= 2, f"Expected echo_ tools, got {echo_tools}" + assert len(echo2_tools) >= 2, f"Expected echo2_ tools, got {echo2_tools}" + finally: + server.stop() + + +def test_mcp_tools_not_listed_when_not_configured(): + """Without MCP config, no MCP tools should appear.""" + global server + server = ServerPreset.router() + server.server_tools = "all" + server.no_ui = True + server.server_port = 8085 + server.start() + + try: + res = server.make_request("GET", "/tools") + assert res.status_code == 200, res.body + + tools = res.body + + def get_tool_name(t): + return t.get("tool", "") or t.get("definition", {}).get("function", {}).get("name", "") + + # Should only have builtin tools, no server: prefixed tools + mcp_tools = [t for t in tools if ":" in get_tool_name(t)] + assert len(mcp_tools) == 0, f"Expected no MCP tools, got {mcp_tools}" + finally: + server.stop() + + +def test_mcp_fail_once_tool_eventual_success(): + """Test that a tool that fails once eventually succeeds (tests instance respawn).""" + global server + mcp_json = _mcp_config_json({ + "echo": { + "command": sys.executable, + "args": [MCP_ECHO_SERVER], + } + }) + server = _start_server_with_mcp(mcp_json) + + try: + # First call should succeed (warmup already spawned and shut down the instance, + # but the first actual tool call will spawn a fresh instance) + res = server.make_request("POST", "/tools", data={ + "tool": "echo_fail_once", + "params": {} + }) + # It might fail on first call if the warmup instance was shut down + # and a new instance is spawned. The fail_once state is per-process, + # so a fresh process will fail once then succeed. + # Actually, warmup spawns, lists, then shuts down. So the first tool call + # spawns a new process which will fail once. + assert res.status_code in (200, 500), res.body + finally: + server.stop() + + +def test_mcp_tools_via_json_config_file(): + """Test that --mcp-servers-config (file) works as well as --mcp-servers-json.""" + global server + config = { + "mcpServers": { + "echo": { + "command": sys.executable, + "args": [MCP_ECHO_SERVER], + } + } + } + + with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f: + json.dump(config, f) + config_path = f.name + + try: + server = ServerPreset.router() + server.server_tools = "all" + server.no_ui = True + server.server_port = 8085 + server.mcp_servers_config = config_path + server.start() + + res = server.make_request("GET", "/tools") + assert res.status_code == 200, res.body + + tools = res.body + + def get_tool_name(t): + return t.get("tool", "") or t.get("definition", {}).get("function", {}).get("name", "") + + mcp_tools = [t for t in tools if get_tool_name(t).startswith("echo_")] + assert len(mcp_tools) >= 2, f"Expected echo_ tools, got {mcp_tools}" + finally: + os.unlink(config_path) + server.stop() + + +def test_mcp_tools_slot_independent(): + """MCP tools should work without any slot concept; /tools is slot-independent.""" + global server + mcp_json = _mcp_config_json({ + "echo": { + "command": sys.executable, + "args": [MCP_ECHO_SERVER], + } + }) + server = _start_server_with_mcp(mcp_json) + + try: + # Call /tools without any slot binding - should succeed + res = server.make_request("POST", "/tools", data={ + "tool": "echo_echo", + "params": {"message": "hello"} + }) + assert res.status_code == 200, res.body + body = res.body + assert "error" not in body, body + finally: + server.stop() + + +def test_mcp_concurrent_tool_calls(): + """Concurrent POST /tools to same MCP server should all succeed.""" + global server + mcp_json = _mcp_config_json({ + "echo": { + "command": sys.executable, + "args": [MCP_ECHO_SERVER], + } + }) + server = _start_server_with_mcp(mcp_json) + + try: + def call_tool(): + return server.make_request("POST", "/tools", data={ + "tool": "echo_echo", + "params": {"message": "hi"} + }) + + with ThreadPoolExecutor(max_workers=10) as executor: + futures = [executor.submit(call_tool) for _ in range(10)] + results = [f.result() for f in futures] + + for res in results: + assert res.status_code == 200, res.body + assert "error" not in res.body, res.body + finally: + server.stop() + + +def test_mcp_tool_timeout(): + """Tool call should timeout if MCP server is too slow.""" + global server + MCP_SLOW_SERVER = os.path.join(FIXTURES_DIR, "mcp_slow_server.py") + mcp_json = _mcp_config_json({ + "slow": { + "command": sys.executable, + "args": [MCP_SLOW_SERVER, "--delay", "5"], + "timeout_ms": 500 + } + }) + server = _start_server_with_mcp(mcp_json) + + try: + res = server.make_request("POST", "/tools", data={ + "tool": "slow_sleep", + "params": {"seconds": 5} + }) + assert res.status_code == 200, res.body + body = res.body + assert "error" in body, body + finally: + server.stop() + + +def test_mcp_warmup_partial_failure(): + """Good server's tools should appear even if bad server fails warmup.""" + global server + mcp_json = _mcp_config_json({ + "good": { + "command": sys.executable, + "args": [MCP_ECHO_SERVER], + }, + "bad": { + "command": "nonexistent", + "args": [] + } + }) + server = _start_server_with_mcp(mcp_json) + + try: + res = server.make_request("GET", "/tools") + assert res.status_code == 200, res.body + tools = res.body + + def get_tool_name(t): + return t.get("tool", "") or t.get("definition", {}).get("function", {}).get("name", "") + + # good server tools should be present + assert any("good_" in get_tool_name(t) for t in tools), f"Expected good: tools in {tools}" + finally: + server.stop() + + +def test_mcp_notification_during_request(): + """Notification during request should not be returned as response.""" + global server + MCP_MALFORMED_SERVER = os.path.join(FIXTURES_DIR, "mcp_malformed_server.py") + mcp_json = _mcp_config_json({ + "notifying": { + "command": sys.executable, + "args": [MCP_MALFORMED_SERVER], + } + }) + server = _start_server_with_mcp(mcp_json) + + try: + res = server.make_request("POST", "/tools", data={ + "tool": "notifying_echo", + "params": {"message": "hi"} + }) + assert res.status_code == 200, res.body + body = res.body + assert "error" not in body, body + finally: + server.stop() + + +def test_mcp_instance_respawn_after_crash(): + """Tool call after process crash should respawn and succeed.""" + global server + MCP_CRASH_SERVER = os.path.join(FIXTURES_DIR, "mcp_crash_server.py") + mcp_json = _mcp_config_json({ + "crash": { + "command": sys.executable, + "args": [MCP_CRASH_SERVER], + } + }) + server = _start_server_with_mcp(mcp_json) + + try: + # First call succeeds + res1 = server.make_request("POST", "/tools", data={ + "tool": "crash_echo", + "params": {"message": "hi"} + }) + assert res1.status_code == 200, res1.body + assert "error" not in res1.body, res1.body + + # Second call should also succeed (respawned instance) + res2 = server.make_request("POST", "/tools", data={ + "tool": "crash_echo", + "params": {"message": "hi2"} + }) + assert res2.status_code == 200, res2.body + assert "error" not in res2.body, res2.body + finally: + server.stop() + + + + +def test_mcp_fail_once_eventual_success_verified(): + """Verify that fail_once tool eventually succeeds after respawn.""" + global server + mcp_json = _mcp_config_json({ + "echo": { + "command": sys.executable, + "args": [MCP_ECHO_SERVER], + } + }) + server = _start_server_with_mcp(mcp_json) + + try: + # First call may fail (fresh process) + res1 = server.make_request("POST", "/tools", data={ + "tool": "echo_fail_once", + "params": {} + }) + # Second call should succeed + res2 = server.make_request("POST", "/tools", data={ + "tool": "echo_fail_once", + "params": {} + }) + assert res2.status_code == 200, res2.body + assert "error" not in res2.body, res2.body + finally: + server.stop() + + +def test_mcp_config_file_errors(): + """Invalid JSON config and missing file should cause server to fail to start.""" + # Invalid JSON - server should fail to start + server = ServerPreset.router() + server.server_tools = "all" + server.no_ui = True + server.server_port = 8085 + server.mcp_servers_json = "not valid json" + try: + server.start() + assert False, "Server should not have started with invalid MCP JSON config" + except RuntimeError: + pass # Expected: server process dies due to bad config + + # Missing file - server should fail to start + server = ServerPreset.router() + server.server_tools = "all" + server.no_ui = True + server.server_port = 8085 + server.mcp_servers_config = "/nonexistent/path.json" + try: + server.start() + assert False, "Server should not have started with missing config file" + except RuntimeError: + pass # Expected: server process dies due to missing config + + +def test_mcp_empty_tool_list(): + """MCP server reporting zero tools should result in empty tool list.""" + global server + # Create a minimal server that returns empty tools list + empty_server = os.path.join(FIXTURES_DIR, "_empty_mcp_server.py") + with open(empty_server, "w") as f: + f.write('''#!/usr/bin/env python3 +import json, sys, os +def main(): + sys.stdout = os.fdopen(sys.stdout.fileno(), "w", buffering=1) + for line in sys.stdin: + line = line.strip() + if not line: continue + try: request = json.loads(line) + except: continue + method = request.get("method") + req_id = request.get("id") + if method == "initialize": + resp = {"jsonrpc": "2.0", "id": req_id, "result": {"protocolVersion": "2024-11-05", "capabilities": {"tools": {}}, "serverInfo": {"name": "empty", "version": "1.0"}}} + elif method == "tools/list": + resp = {"jsonrpc": "2.0", "id": req_id, "result": {"tools": []}} + else: + resp = {"jsonrpc": "2.0", "id": req_id, "error": {"code": -32601, "message": "Method not found"}} + sys.stdout.write(json.dumps(resp) + "\\n") + sys.stdout.flush() +if __name__ == "__main__": + main() +''') + try: + mcp_json = _mcp_config_json({ + "empty": { + "command": sys.executable, + "args": [empty_server], + } + }) + server = _start_server_with_mcp(mcp_json) + res = server.make_request("GET", "/tools") + assert res.status_code == 200, res.body + tools = res.body + def get_tool_name(t): + return t.get("tool", "") or t.get("definition", {}).get("function", {}).get("name", "") + mcp_tools = [t for t in tools if get_tool_name(t).startswith("empty:")] + assert len(mcp_tools) == 0, f"Expected no empty: tools, got {mcp_tools}" + finally: + os.unlink(empty_server) + server.stop() + + +def test_mcp_rapid_succession_calls(): + """Many rapid calls should increment next_id correctly and correlate responses.""" + global server + mcp_json = _mcp_config_json({ + "echo": { + "command": sys.executable, + "args": [MCP_ECHO_SERVER], + } + }) + server = _start_server_with_mcp(mcp_json) + + try: + for i in range(20): + res = server.make_request("POST", "/tools", data={ + "tool": "echo_echo", + "params": {"message": f"msg{i}"} + }) + assert res.status_code == 200, res.body + assert "error" not in res.body, res.body + finally: + server.stop() + + +def test_mcp_notification_burst(): + """Notification + response in a single write() with no flush should not strand the response.""" + global server + MCP_BURST_SERVER = os.path.join(FIXTURES_DIR, "mcp_burst_server.py") + mcp_json = _mcp_config_json({ + "burst": { + "command": sys.executable, + "args": [MCP_BURST_SERVER], + } + }) + server = _start_server_with_mcp(mcp_json) + + try: + res = server.make_request("POST", "/tools", data={ + "tool": "burst_echo", + "params": {"message": "burst test"} + }) + assert res.status_code == 200, res.body + body = res.body + assert "error" not in body, body + finally: + server.stop() + + +def test_mcp_tool_definition_shape_via_chat_completions(): + """MCP tool definitions returned by GET /tools should have the correct shape for chat/completions.""" + global server + mcp_json = _mcp_config_json({ + "echo": { + "command": sys.executable, + "args": [MCP_ECHO_SERVER], + } + }) + server = _start_server_with_mcp(mcp_json) + + try: + # Get MCP tool definitions + res = server.make_request("GET", "/tools") + assert res.status_code == 200, res.body + tools = res.body + + def get_tool_name(t): + return t.get("tool", "") or t.get("definition", {}).get("function", {}).get("name", "") + + echo_tools = [t for t in tools if get_tool_name(t).startswith("echo_")] + assert len(echo_tools) >= 2, f"Expected echo_ tools, got {echo_tools}" + + echo_tool = next(t for t in echo_tools if get_tool_name(t) == "echo_echo") + definition = echo_tool.get("definition", echo_tool) + + # Verify the definition has the standard function-calling shape + assert definition.get("type") == "function", f"Expected type=function, got {definition.get('type')}" + func = definition.get("function", {}) + assert "name" in func, "Missing function.name" + assert "description" in func, "Missing function.description" + assert "parameters" in func, f"Missing function.parameters, got keys: {list(func.keys())}" + params = func["parameters"] + assert params.get("type") == "object", f"Expected parameters.type=object, got {params.get('type')}" + assert "properties" in params, "Missing parameters.properties" + finally: + server.stop() + + +def test_mcp_slow_tool_call_slot_release(): + """A slow tool call should not stall server shutdown for the full I/O timeout.""" + global server + MCP_SLOW_SERVER = os.path.join(FIXTURES_DIR, "mcp_slow_server.py") + mcp_json = _mcp_config_json({ + "slow": { + "command": sys.executable, + "args": [MCP_SLOW_SERVER, "--delay", "10"], + "timeout_ms": 30000 + } + }) + server = _start_server_with_mcp(mcp_json) + + try: + # Start a slow tool call in a background thread + def slow_call(): + return server.make_request("POST", "/tools", data={ + "tool": "slow_sleep", + "params": {"seconds": 10} + }) + + with ThreadPoolExecutor(max_workers=1) as executor: + future = executor.submit(slow_call) + + # Wait a moment for the call to start + time.sleep(2) + + # Stop the server while the tool call is in progress. + # With global MCP instances, close_all() is called explicitly at shutdown + # (not from slot release), so shutdown should complete promptly. + start_time = time.time() + server.stop() + elapsed = time.time() - start_time + + # The server should stop quickly, not wait for the full 30s I/O timeout. + # With the terminating flag, send_rpc() bails out within one select() + # slice (~50ms). This threshold MUST stay below the 5s force-kill + # fallback in ServerProcess.stop(): without the flag, shutdown stalls + # on the instance mutex and only completes when stop() sends SIGKILL + # at ~5s -- which any threshold above 5 would still accept. + assert elapsed < 3, f"Server stop took {elapsed:.1f}s, expected < 3s" + + # Wait for the future to complete (it will get an error response or timeout) + try: + res = future.result(timeout=5) + # If we got a response, it should be an error since the server stopped + if hasattr(res, 'status_code'): + assert res.status_code in (200, 500, 502, 503, 504), f"Unexpected status: {res.status_code}" + except Exception: + # Thread may have raised due to connection error - that's acceptable + pass + finally: + server.stop() + + +def test_mcp_grandchild_holding_pipes_does_not_deadlock(): + """An MCP server that leaves a grandchild inheriting its stdout/stderr must not deadlock + teardown. + + subprocess_terminate() only SIGKILLs the direct MCP child, so the inherited pipe write ends + stay open and a blocking read on them would never see EOF. That hung both warmup shutdown + (the server would never reach "ready") and process shutdown. The polled, running-aware reader + must exit regardless, so the server both starts and stops promptly here. + """ + global server + MCP_GRANDCHILD_SERVER = os.path.join(FIXTURES_DIR, "mcp_grandchild_server.py") + mcp_json = _mcp_config_json({ + "gc": { + "command": sys.executable, + "args": [MCP_GRANDCHILD_SERVER], + } + }) + + # If warmup teardown deadlocked, the server would never become ready and start() would time out. + server = _start_server_with_mcp(mcp_json) + + try: + # invoking the tool spawns a live transport whose reader thread holds the inherited pipe + res = server.make_request("POST", "/tools", data={ + "tool": "gc_echo", + "params": {"message": "hello"} + }) + assert res.status_code == 200, res.body + assert "error" not in res.body, res.body + + # shutdown must be prompt: a deadlocked reader-join would stall until the 5s SIGKILL + # fallback in ServerProcess.stop(), so the threshold has to stay below that + start = time.time() + server.stop() + elapsed = time.time() - start + assert elapsed < 3, f"server shutdown took {elapsed:.1f}s (expected < 3s) — teardown likely deadlocked" + finally: + server.stop() diff --git a/tools/server/tests/utils.py b/tools/server/tests/utils.py index 5d5c873ac4cc..ae56bc70a15a 100644 --- a/tools/server/tests/utils.py +++ b/tools/server/tests/utils.py @@ -115,6 +115,8 @@ class ServerProcess: backend_sampling: bool = False gcp_compat: bool = False server_tools: str | None = None + mcp_servers_config: str | None = None + mcp_servers_json: str | None = None cors_origins: str | None = None # session variables @@ -265,6 +267,10 @@ def start(self, timeout_seconds: int = DEFAULT_HTTP_TIMEOUT) -> None: server_args.append("--ui-mcp-proxy") if self.server_tools: server_args.extend(["--tools", self.server_tools]) + if self.mcp_servers_config: + server_args.extend(["--mcp-servers-config", self.mcp_servers_config]) + if self.mcp_servers_json: + server_args.extend(["--mcp-servers-json", self.mcp_servers_json]) if self.backend_sampling: server_args.append("--backend_sampling") if self.gcp_compat: