From 1184db7590f31e1f2a3371856b6d17831917c7a1 Mon Sep 17 00:00:00 2001 From: thomas Date: Wed, 22 Apr 2026 16:20:16 +0200 Subject: [PATCH 01/29] Refactor ROADMAP.md for clarity and remove outdated sections --- docs/ROADMAP.md | 30 ++++-------------------------- 1 file changed, 4 insertions(+), 26 deletions(-) diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index e6beb61c63..8ca362348b 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -2,43 +2,21 @@ -# 7.2.0 - -- Move SOAP samples from tutorials/transformations to ../soap TB - -# 7.1.1 - -# Improvements -- Move URL template evaluation after the request flow has been processed. Before expressions in the target were evaluated before the request flow was processed. -- In a target/url with an expression like "a: ${propery.a} b: ${property.b}" the evaluation result of ${} is now URL encoded. - -# Features -- urlEncode(), pathSeg() functions of SpEL and Groovy - -# 7.X - PRIO 1: - Proxy Server Configuration Sample - explains how to configure a proxy server - HotReload for YAML -- Support for multiple apis.yaml in one folder - - idea: load apis.yaml last. - pattern *.apis.yaml - Register JSON Schema for YAML at: https://www.schemastore.org TB - create test asserting that connection reuse via proxy works TP - Central description of Membrane Languages, Cheat Sheets, links to their docs. TP - Central description of MEMBRANE_* environment variables - Like MEMBRANE_HOME... - - @coderabbitai look through the code base for usages of these variables and suggest documentation -- Idea: Multiple api.yaml files - - Membrane reads all *.apis.yaml files in the conf folder or the current working directory - the apis.yaml is read last and therefore overwrites previous definitions - e.g. fruitshop.apis.yaml, dlp.apis.yaml, apis.yaml with global and matches all - - - + - @coderabbitai look through the code base for usages of these variables and suggest documentation PRIO 2: +- jsonRPCProtection: + - maxBatchSize = 0,1 -> No Batch, n = n-Batches + - allow/block list for methods - Fix maven central publish job - Tutorial: Replace httpbin and catfact TB - use @MCElement(collapsed=true) for suitable classes From cd06f7fd028873a6ce236f5bfadc8250f994d013 Mon Sep 17 00:00:00 2001 From: thomas Date: Thu, 23 Apr 2026 11:20:38 +0200 Subject: [PATCH 02/29] feat: add JSON-RPC request, response, and MCP initialization support --- .../core/exchangestore/ClientStatistics.java | 10 +- .../interceptor/mcp/MembraneMCPServer.java | 188 ++++++++ .../membrane/core/jsonrpc/JSONRPCRequest.java | 237 ++++++++++ .../core/jsonrpc/JSONRPCResponse.java | 421 ++++++++++++++++++ .../membrane/core/mcp/MCPInitialize.java | 140 ++++++ .../core/mcp/MCPInitializeResponse.java | 221 +++++++++ .../membrane/core/mcp/MCPInitialized.java | 40 ++ .../membrane/core/mcp/MCPNotification.java | 51 +++ .../predic8/membrane/core/mcp/MCPRequest.java | 62 +++ .../membrane/core/mcp/MCPResponse.java | 93 ++++ .../membrane/core/mcp/MCPToolsCall.java | 100 +++++ .../core/mcp/MCPToolsCallResponse.java | 336 ++++++++++++++ .../membrane/core/mcp/MCPToolsList.java | 69 +++ .../core/mcp/MCPToolsListResponse.java | 248 +++++++++++ docs/ROADMAP.md | 2 + 15 files changed, 2213 insertions(+), 5 deletions(-) create mode 100644 core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java create mode 100644 core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCRequest.java create mode 100644 core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCResponse.java create mode 100644 core/src/main/java/com/predic8/membrane/core/mcp/MCPInitialize.java create mode 100644 core/src/main/java/com/predic8/membrane/core/mcp/MCPInitializeResponse.java create mode 100644 core/src/main/java/com/predic8/membrane/core/mcp/MCPInitialized.java create mode 100644 core/src/main/java/com/predic8/membrane/core/mcp/MCPNotification.java create mode 100644 core/src/main/java/com/predic8/membrane/core/mcp/MCPRequest.java create mode 100644 core/src/main/java/com/predic8/membrane/core/mcp/MCPResponse.java create mode 100644 core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsCall.java create mode 100644 core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsCallResponse.java create mode 100644 core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsList.java create mode 100644 core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsListResponse.java diff --git a/core/src/main/java/com/predic8/membrane/core/exchangestore/ClientStatistics.java b/core/src/main/java/com/predic8/membrane/core/exchangestore/ClientStatistics.java index b6fbccc3fd..8ada7f27ba 100644 --- a/core/src/main/java/com/predic8/membrane/core/exchangestore/ClientStatistics.java +++ b/core/src/main/java/com/predic8/membrane/core/exchangestore/ClientStatistics.java @@ -16,14 +16,14 @@ public interface ClientStatistics { - public int getCount(); + int getCount(); - public String getClient(); + String getClient(); - public long getMinDuration(); + long getMinDuration(); - public long getMaxDuration(); + long getMaxDuration(); - public long getAvgDuration(); + long getAvgDuration(); } \ No newline at end of file diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java new file mode 100644 index 0000000000..5f658b1164 --- /dev/null +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java @@ -0,0 +1,188 @@ +package com.predic8.membrane.core.interceptor.mcp; + +import com.predic8.membrane.annot.Constants; +import com.predic8.membrane.annot.MCElement; +import com.predic8.membrane.core.exchange.Exchange; +import com.predic8.membrane.core.http.Response; +import com.predic8.membrane.core.interceptor.AbstractInterceptor; +import com.predic8.membrane.core.interceptor.Outcome; +import com.predic8.membrane.core.jsonrpc.JSONRPCRequest; +import com.predic8.membrane.core.mcp.*; +import com.predic8.membrane.core.openapi.serviceproxy.APIProxy; +import com.predic8.membrane.core.proxies.SOAPProxy; +import com.predic8.membrane.core.proxies.ServiceProxy; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; + +import static com.predic8.membrane.core.http.MimeType.APPLICATION_JSON; +import static com.predic8.membrane.core.http.Response.ok; +import static com.predic8.membrane.core.interceptor.Outcome.RETURN; + +/** + * @description MCP Server for Membrane. It allows to query Membrane's internal state and operation from an LLM + * Ask the LLM questions like: + * - What APIs are deployed? + * - Is the Membrane instance healthy? + * - Give me a summary about the requests + */ +@MCElement(name = "membraneMCPServer") +public class MembraneMCPServer extends AbstractInterceptor { + + private static final Logger log = LoggerFactory.getLogger(MembraneMCPServer.class); + + @Override + public Outcome handleRequest(Exchange exc) { + + try { + var request = JSONRPCRequest.parse(exc.getRequest().getBodyAsStreamDecoded()); + + MCPResponse mcpResponse = null; + Response response; + if (request.getMethod().equals("initialize")) { + mcpResponse = initialize(exc, request); + } else if (request.getMethod().equals("notifications/initialized")) { + // Do nothing + } else if (request.getMethod().equals("tools/list")) { + mcpResponse = toolsList(exc, request); + } else if (request.getMethod().equals("tools/call")) { + mcpResponse = toolsCall(exc, request); + } else { + System.out.println("Unknown MCP Request: " + request); + } + if (mcpResponse == null) { + response = Response.noContent().build(); + } else { + response = ok().contentType(APPLICATION_JSON).body(mcpResponse.toJson()).build(); + } + exc.setResponse(response); + return RETURN; + + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + private MCPResponse toolsCall(Exchange exc, JSONRPCRequest request) throws IOException { + var req = MCPToolsCall.from(request); + + log.debug("Received MCP tools call: {}", req); + + if (req.getName().equals("listProxies")) { + return listProxies(req); + } else if (req.getName().equals("getExchanges")) { + return getExchanges(req); + } else if (req.getName().equals("getStatistics")) { + getRouter().getStatistics(); + } else { + log.info("Unknown tools call: " + req.getName()); + } + + return null; + } + + private MCPToolsCallResponse getExchanges(MCPToolsCall req) { + var exchangesRes = getRouter().getExchangeStore().getAllExchangesAsList().stream().map(e -> { + + if (e.getResponse() == null) + return null; + + var exc = new HashMap(); + exc.put("id", e.getId()); + var request = new HashMap(); + var response = new HashMap(); + + request.put("method", e.getRequest().getMethod()); + request.put("path", e.getRequest().getUri()); + request.put("body", e.getRequest().getBodyAsStringDecoded()); + request.put("headers",e.getRequest().getHeader()); + + exc.put("request", request); + exc.put("response", response); + return exc; + }).filter(Objects::nonNull).toList(); + + return MCPToolsCallResponse.from(req).withJson(Map.of("exchanges", exchangesRes)); + } + + private MCPToolsCallResponse listProxies(MCPToolsCall req) { + var proxies = getRouter().getRuleManager().getRules(); + + var proxiesDesc = proxies.stream().map(p -> { + var proxy = new HashMap(); + proxy.put("name", p.getName()); + + String type; + switch (p) { + case APIProxy ap -> { + type = "API"; + // proxy.put("openapi", ap.getOpenapi()); + } + case ServiceProxy s -> { + type = "serviceProxy"; + } + case SOAPProxy sp -> { + type = "soapProxy"; + proxy.put("wsdl", sp.getWsdl()); + proxy.put("serviceName", sp.getServiceName()); + } + default -> { + type = "unknown"; + } + } + + var interceptors = p.getFlow().stream().map(i -> { + Map interceptor = new HashMap(); + interceptor.put("name", i.getDisplayName()); + return interceptor; + }).toList(); + + proxy.put("statistics", getRouter().getExchangeStore().getStatistics(p.getKey())); + + proxy.put("interceptors", interceptors); + proxy.put("type", type); + proxy.put("rule", p.getKey().toString()); + return proxy; + }).toList(); + + return MCPToolsCallResponse.from(req).withJson(Map.of("proxies", proxiesDesc)); + } + + private MCPResponse toolsList(Exchange exc, JSONRPCRequest request) throws IOException { + log.debug("Tools list"); + var req = MCPToolsList.from(request); + var resp = MCPToolsListResponse.from(req) + .withTool(new MCPToolsListResponse.Tool( + "listProxies", + "Lists all the proxies, e.g. API, soapProxy", Map.of("type", "object"))) + .withTool(new MCPToolsListResponse.Tool("getExchanges", "Gets the last 100 HTTP exchanges", Map.of("type", "object"))); + +// Map.of("type", "object", +// "properties", Map.of("query", Map.of("type", "string")), +// "required", List.of("query")))); + return resp; + } + + private MCPResponse initialize(Exchange exc, JSONRPCRequest request) throws IOException { + var initialize = new MCPInitialize(request); + + log.debug("initialize: " + initialize); + + var response = new MCPInitializeResponse(initialize); + + var capabilities = new HashMap(); + capabilities.put("tools", Map.of("lastExchanges", false)); + response.withCapabilities(capabilities) + .withServerInfo("Membrane", Constants.VERSION); + return response; + } + + @Override + public String getDisplayName() { + return "Membrane MCP Server"; + } +} diff --git a/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCRequest.java b/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCRequest.java new file mode 100644 index 0000000000..b86b497b81 --- /dev/null +++ b/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCRequest.java @@ -0,0 +1,237 @@ +package com.predic8.membrane.core.jsonrpc; + +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.util.List; +import java.util.Map; +import java.util.Objects; + +/** + * Represents a JSON-RPC 2.0 request as defined by https://www.jsonrpc.org/specification. + * + *

The {@code id} member may be a {@link String}, a {@link Number}, or {@code null}. + * A request without an {@code id} is a notification and the server MUST NOT reply.

+ * + *

The {@code params} member is structured: either a JSON Array (positional, see + * {@link #getParamsList()}) or a JSON Object (named, see {@link #getParamsMap()}).

+ */ +@JsonInclude(JsonInclude.Include.NON_NULL) +public class JSONRPCRequest { + + public static final String JSONRPC_VERSION = "2.0"; + + private static final ObjectMapper OM = new ObjectMapper(); + private static final TypeReference> MAP_TYPE = new TypeReference<>() {}; + private static final TypeReference> LIST_TYPE = new TypeReference<>() {}; + + @JsonProperty("jsonrpc") + private String jsonrpc = JSONRPC_VERSION; + + /** May be String, Number, or null. */ + @JsonProperty("id") + private Object id; + + @JsonProperty("method") + private String method; + + /** Populated when params is a JSON object; mutually exclusive with {@link #paramsList}. */ + @JsonIgnore + private Map paramsMap; + + /** Populated when params is a JSON array; mutually exclusive with {@link #paramsMap}. */ + @JsonIgnore + private List paramsList; + + public JSONRPCRequest() {} + + public JSONRPCRequest(Object id, String method, Map paramsMap) { + this.id = id; + this.method = Objects.requireNonNull(method, "method must not be null"); + this.paramsMap = paramsMap; + } + + public JSONRPCRequest(Object id, String method, List paramsList) { + this.id = id; + this.method = Objects.requireNonNull(method, "method must not be null"); + this.paramsList = paramsList; + } + + // ---------- Parsing ---------- + + public static JSONRPCRequest parse(InputStream is) throws IOException { + Objects.requireNonNull(is, "input stream must not be null"); + return fromNode(OM.readTree(is)); + } + + public static JSONRPCRequest parse(String json) throws IOException { + Objects.requireNonNull(json, "json must not be null"); + return fromNode(OM.readTree(json)); + } + + private static JSONRPCRequest fromNode(JsonNode root) throws IOException { + if (root == null || !root.isObject()) { + throw new IOException("Invalid JSON-RPC request: expected JSON object"); + } + + JSONRPCRequest req = new JSONRPCRequest(); + + JsonNode versionNode = root.get("jsonrpc"); + req.jsonrpc = versionNode != null && !versionNode.isNull() ? versionNode.asText() : null; + if (!JSONRPC_VERSION.equals(req.jsonrpc)) { + throw new IOException("Unsupported or missing jsonrpc version: " + req.jsonrpc); + } + + JsonNode methodNode = root.get("method"); + if (methodNode == null || !methodNode.isTextual()) { + throw new IOException("Invalid JSON-RPC request: 'method' must be a string"); + } + req.method = methodNode.asText(); + + JsonNode idNode = root.get("id"); + if (idNode != null && !idNode.isNull()) { + if (idNode.isTextual()) { + req.id = idNode.asText(); + } else if (idNode.isIntegralNumber()) { + req.id = idNode.longValue(); + } else if (idNode.isNumber()) { + req.id = idNode.numberValue(); + } else { + throw new IOException("Invalid JSON-RPC request: 'id' must be string, number, or null"); + } + } + + JsonNode paramsNode = root.get("params"); + if (paramsNode != null && !paramsNode.isNull()) { + if (paramsNode.isArray()) { + req.paramsList = OM.convertValue(paramsNode, LIST_TYPE); + } else if (paramsNode.isObject()) { + req.paramsMap = OM.convertValue(paramsNode, MAP_TYPE); + } else { + throw new IOException("Invalid JSON-RPC request: 'params' must be array or object"); + } + } + + return req; + } + + // ---------- Serialization ---------- + + /** Returns the params field for serialization (object, array, or null). */ + @JsonProperty("params") + @JsonInclude(JsonInclude.Include.NON_NULL) + public Object getParams() { + if (paramsMap != null) return paramsMap; + return paramsList; + } + + public String toJson() throws IOException { + return OM.writeValueAsString(this); + } + + public void writeTo(OutputStream os) throws IOException { + OM.writeValue(os, this); + } + + // ---------- Convenience ---------- + + @JsonIgnore + public boolean isNotification() { + return id == null; + } + + @JsonIgnore + public boolean hasNamedParams() { + return paramsMap != null; + } + + @JsonIgnore + public boolean hasPositionalParams() { + return paramsList != null; + } + + // ---------- Getters / setters ---------- + + public String getJsonrpc() { + return jsonrpc; + } + + public void setJsonrpc(String jsonrpc) { + this.jsonrpc = jsonrpc; + } + + public Object getId() { + return id; + } + + /** Convenience accessor preserving backwards compatibility — returns the id as String, or null. */ + @JsonIgnore + public String getIdAsString() { + return id == null ? null : id.toString(); + } + + public void setId(Object id) { + this.id = id; + } + + public String getMethod() { + return method; + } + + public void setMethod(String method) { + this.method = method; + } + + public Map getParamsMap() { + return paramsMap; + } + + public void setParamsMap(Map paramsMap) { + this.paramsMap = paramsMap; + if (paramsMap != null) this.paramsList = null; + } + + public List getParamsList() { + return paramsList; + } + + public void setParamsList(List paramsList) { + this.paramsList = paramsList; + if (paramsList != null) this.paramsMap = null; + } + + // ---------- Object overrides ---------- + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof JSONRPCRequest that)) return false; + return Objects.equals(jsonrpc, that.jsonrpc) + && Objects.equals(id, that.id) + && Objects.equals(method, that.method) + && Objects.equals(paramsMap, that.paramsMap) + && Objects.equals(paramsList, that.paramsList); + } + + @Override + public int hashCode() { + return Objects.hash(jsonrpc, id, method, paramsMap, paramsList); + } + + @Override + public String toString() { + return "JSONRPCRequest{" + + "jsonrpc='" + jsonrpc + '\'' + + ", id=" + id + + ", method='" + method + '\'' + + ", params=" + (paramsMap != null ? paramsMap : paramsList) + + '}'; + } +} \ No newline at end of file diff --git a/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCResponse.java b/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCResponse.java new file mode 100644 index 0000000000..91deebbaf2 --- /dev/null +++ b/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCResponse.java @@ -0,0 +1,421 @@ +package com.predic8.membrane.core.jsonrpc; + +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonPropertyOrder; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.util.Objects; + +/** + * Represents a JSON-RPC 2.0 response as defined by https://www.jsonrpc.org/specification. + * + *

A response is either a success or an error — the two are + * mutually exclusive:

+ *
    + *
  • Success: {@code result} is present, {@code error} is absent.
    • + *
  • Error: {@code error} is present, {@code result} is absent.
    • + *
+ * + *

Wire format (success):

+ * 
{@code
+ * { "jsonrpc": "2.0", "id": 1, "result": { ... } }
+ * }
+ * + *

Wire format (error):

+ * 
{@code
+ * { "jsonrpc": "2.0", "id": 1, "error": { "code": -32600, "message": "...", "data": ... } }
+ * }
+ * + *

The {@code id} member mirrors the {@code id} of the originating request. + * It MUST be {@code null} if the id could not be determined (e.g. parse error).

+ * + *

Use the static factories {@link #success(Object, Object)} and + * {@link #error(Object, int, String)} (or {@link #error(Object, int, String, Object)}) + * to construct instances. Use {@link #from(JSONRPCRequest, Object)} to build a + * success response that echoes the id of an existing {@link JSONRPCRequest}.

+ * + *

Standard error codes are provided as constants, e.g. {@link #ERR_PARSE_ERROR}.

+ */ +@JsonInclude(JsonInclude.Include.NON_NULL) +@JsonPropertyOrder({"jsonrpc", "id", "result", "error"}) +public class JSONRPCResponse { + + /** JSON-RPC protocol version — always {@code "2.0"}. */ + public static final String JSONRPC_VERSION = JSONRPCRequest.JSONRPC_VERSION; + + // ---------- Standard error codes (JSON-RPC 2.0 spec, section 5.1) ---------- + + /** Invalid JSON was received by the server. */ + public static final int ERR_PARSE_ERROR = -32700; + /** The JSON sent is not a valid Request object. */ + public static final int ERR_INVALID_REQUEST = -32600; + /** The method does not exist / is not available. */ + public static final int ERR_METHOD_NOT_FOUND = -32601; + /** Invalid method parameter(s). */ + public static final int ERR_INVALID_PARAMS = -32602; + /** Internal JSON-RPC error. */ + public static final int ERR_INTERNAL_ERROR = -32603; + + private static final ObjectMapper OM = new ObjectMapper(); + + @JsonProperty("jsonrpc") + private String jsonrpc = JSONRPC_VERSION; + + /** May be String, Number, or null. Mirrors the id of the originating request. */ + @JsonProperty("id") + private Object id; + + /** + * Present on success responses; absent (null) on error responses. + * + *

Note: per spec, {@code result} may legitimately be JSON {@code null}. + * In that case serialisation will omit the field due to {@code @JsonInclude(NON_NULL)}. + * If an explicit JSON {@code null} result must be preserved on the wire, wrap the + * value before passing it to {@link #success(Object, Object)}.

+ */ + @JsonProperty("result") + private Object result; + + /** Present on error responses; absent (null) on success responses. */ + @JsonProperty("error") + private Error error; + + // ---------- Constructors ---------- + + /** No-arg constructor for Jackson deserialization. */ + public JSONRPCResponse() {} + + private JSONRPCResponse(Object id, Object result, Error error) { + this.id = id; + this.result = result; + this.error = error; + } + + // ---------- Static factories ---------- + + /** + * Creates a success response with the given {@code id} and {@code result}. + * + * @param id the id echoed from the request (String, Number, or null) + * @param result the result value (any JSON-serialisable object, including null) + */ + public static JSONRPCResponse success(Object id, Object result) { + return new JSONRPCResponse(id, result, null); + } + + /** + * Creates a success response that echoes the id of the originating {@link JSONRPCRequest}. + * + * @param request the request to reply to + * @param result the result value + */ + public static JSONRPCResponse from(JSONRPCRequest request, Object result) { + Objects.requireNonNull(request, "request must not be null"); + return new JSONRPCResponse(request.getId(), result, null); + } + + /** + * Creates an error response with the given {@code id}, error {@code code}, and {@code message}. + * + * @param id the id echoed from the request, or null if the id could not be determined + * @param code the JSON-RPC error code (see {@code ERR_*} constants) + * @param message a human-readable error description + */ + public static JSONRPCResponse error(Object id, int code, String message) { + return new JSONRPCResponse(id, null, new Error(code, message, null)); + } + + /** + * Creates an error response with optional additional {@code data}. + * + * @param id the id echoed from the request, or null if the id could not be determined + * @param code the JSON-RPC error code (see {@code ERR_*} constants) + * @param message a human-readable error description + * @param data optional additional error information (any JSON-serialisable object) + */ + public static JSONRPCResponse error(Object id, int code, String message, Object data) { + return new JSONRPCResponse(id, null, new Error(code, message, data)); + } + + /** + * Creates an error response that echoes the id of the originating {@link JSONRPCRequest}. + * + * @param request the request to reply to + * @param code the JSON-RPC error code (see {@code ERR_*} constants) + * @param message a human-readable error description + */ + public static JSONRPCResponse errorFor(JSONRPCRequest request, int code, String message) { + Objects.requireNonNull(request, "request must not be null"); + return new JSONRPCResponse(request.getId(), null, new Error(code, message, null)); + } + + // ---------- Parsing ---------- + + /** + * Parses a JSON-RPC 2.0 response from the given {@link InputStream}. + * + * @throws IOException if the JSON is malformed or violates the JSON-RPC 2.0 structure + */ + public static JSONRPCResponse parse(InputStream is) throws IOException { + Objects.requireNonNull(is, "input stream must not be null"); + return fromNode(OM.readTree(is)); + } + + /** + * Parses a JSON-RPC 2.0 response from the given JSON string. + * + * @throws IOException if the JSON is malformed or violates the JSON-RPC 2.0 structure + */ + public static JSONRPCResponse parse(String json) throws IOException { + Objects.requireNonNull(json, "json must not be null"); + return fromNode(OM.readTree(json)); + } + + private static JSONRPCResponse fromNode(JsonNode root) throws IOException { + if (root == null || !root.isObject()) { + throw new IOException("Invalid JSON-RPC response: expected JSON object"); + } + + JSONRPCResponse resp = new JSONRPCResponse(); + + JsonNode versionNode = root.get("jsonrpc"); + resp.jsonrpc = versionNode != null && !versionNode.isNull() ? versionNode.asText() : null; + if (!JSONRPC_VERSION.equals(resp.jsonrpc)) { + throw new IOException("Unsupported or missing jsonrpc version: " + resp.jsonrpc); + } + + JsonNode idNode = root.get("id"); + if (idNode != null && !idNode.isNull()) { + if (idNode.isTextual()) { + resp.id = idNode.asText(); + } else if (idNode.isIntegralNumber()) { + resp.id = idNode.longValue(); + } else if (idNode.isNumber()) { + resp.id = idNode.numberValue(); + } else { + throw new IOException("Invalid JSON-RPC response: 'id' must be string, number, or null"); + } + } + + boolean hasResult = root.has("result"); + boolean hasError = root.has("error"); + + if (hasResult && hasError) { + throw new IOException("Invalid JSON-RPC response: 'result' and 'error' are mutually exclusive"); + } + if (!hasResult && !hasError) { + throw new IOException("Invalid JSON-RPC response: either 'result' or 'error' must be present"); + } + + if (hasResult) { + JsonNode resultNode = root.get("result"); + resp.result = resultNode.isNull() ? null : OM.convertValue(resultNode, Object.class); + } else { + JsonNode errorNode = root.get("error"); + if (!errorNode.isObject()) { + throw new IOException("Invalid JSON-RPC response: 'error' must be a JSON object"); + } + resp.error = Error.fromNode(errorNode); + } + + return resp; + } + + // ---------- Serialization ---------- + + public String toJson() throws IOException { + return OM.writeValueAsString(this); + } + + public void writeTo(OutputStream os) throws IOException { + OM.writeValue(os, this); + } + + // ---------- Convenience ---------- + + /** Returns {@code true} if this is a success response (no {@code error} member). */ + @JsonIgnore + public boolean isSuccess() { + return error == null; + } + + /** Returns {@code true} if this is an error response. */ + @JsonIgnore + public boolean isError() { + return error != null; + } + + /** Convenience accessor — returns the id as String, or null. */ + @JsonIgnore + public String getIdAsString() { + return id == null ? null : id.toString(); + } + + // ---------- Getters / setters ---------- + + public String getJsonrpc() { + return jsonrpc; + } + + public void setJsonrpc(String jsonrpc) { + this.jsonrpc = jsonrpc; + } + + public Object getId() { + return id; + } + + public void setId(Object id) { + this.id = id; + } + + public Object getResult() { + return result; + } + + public void setResult(Object result) { + this.result = result; + if (result != null) this.error = null; + } + + public Error getError() { + return error; + } + + public void setError(Error error) { + this.error = error; + if (error != null) this.result = null; + } + + // ---------- Object overrides ---------- + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof JSONRPCResponse that)) return false; + return Objects.equals(jsonrpc, that.jsonrpc) + && Objects.equals(id, that.id) + && Objects.equals(result, that.result) + && Objects.equals(error, that.error); + } + + @Override + public int hashCode() { + return Objects.hash(jsonrpc, id, result, error); + } + + @Override + public String toString() { + return "JSONRPCResponse{" + + "jsonrpc='" + jsonrpc + '\'' + + ", id=" + id + + (error == null ? ", result=" + result : ", error=" + error) + + '}'; + } + + // ---------- Nested type: Error ---------- + + /** + * Represents the JSON-RPC 2.0 {@code error} member. + * + *

Wire format:

+ * 
{@code
+     * { "code": -32600, "message": "Invalid Request", "data": { ... } }
+     * }
+ */ + @JsonInclude(JsonInclude.Include.NON_NULL) + @JsonPropertyOrder({"code", "message", "data"}) + public static final class Error { + + @JsonProperty("code") + private int code; + + @JsonProperty("message") + private String message; + + /** Optional additional information about the error (any JSON-serialisable object). */ + @JsonProperty("data") + private Object data; + + /** No-arg constructor for Jackson deserialization. */ + public Error() {} + + public Error(int code, String message) { + this.code = code; + this.message = Objects.requireNonNull(message, "message must not be null"); + } + + public Error(int code, String message, Object data) { + this.code = code; + this.message = Objects.requireNonNull(message, "message must not be null"); + this.data = data; + } + + private static Error fromNode(JsonNode node) throws IOException { + JsonNode codeNode = node.get("code"); + if (codeNode == null || !codeNode.isIntegralNumber()) { + throw new IOException("Invalid JSON-RPC error: 'code' must be an integer"); + } + JsonNode messageNode = node.get("message"); + if (messageNode == null || !messageNode.isTextual()) { + throw new IOException("Invalid JSON-RPC error: 'message' must be a string"); + } + Object data = null; + JsonNode dataNode = node.get("data"); + if (dataNode != null && !dataNode.isNull()) { + data = new ObjectMapper().convertValue(dataNode, Object.class); + } + return new Error(codeNode.intValue(), messageNode.asText(), data); + } + + public int getCode() { + return code; + } + + public void setCode(int code) { + this.code = code; + } + + public String getMessage() { + return message; + } + + public void setMessage(String message) { + this.message = message; + } + + public Object getData() { + return data; + } + + public void setData(Object data) { + this.data = data; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof Error that)) return false; + return code == that.code + && Objects.equals(message, that.message) + && Objects.equals(data, that.data); + } + + @Override + public int hashCode() { + return Objects.hash(code, message, data); + } + + @Override + public String toString() { + return "Error{code=" + code + ", message='" + message + "'" + + (data != null ? ", data=" + data : "") + "}"; + } + } +} diff --git a/core/src/main/java/com/predic8/membrane/core/mcp/MCPInitialize.java b/core/src/main/java/com/predic8/membrane/core/mcp/MCPInitialize.java new file mode 100644 index 0000000000..46e8228275 --- /dev/null +++ b/core/src/main/java/com/predic8/membrane/core/mcp/MCPInitialize.java @@ -0,0 +1,140 @@ +package com.predic8.membrane.core.mcp; + +import com.predic8.membrane.core.jsonrpc.JSONRPCRequest; + +import java.util.Collections; +import java.util.Map; +import java.util.Objects; + +/** + * Typed view of an MCP {@code initialize} request. + * + *

The MCP {@code initialize} method has the following params shape:

+ * 
{@code
+ * {
+ *   "protocolVersion": "2024-11-05",
+ *   "capabilities":   { ... },
+ *   "clientInfo":     { "name": "...", "version": "..." }
+ * }
+ * }
+ * + *

An instance is created from a {@link JSONRPCRequest} via {@link #MCPInitialize(JSONRPCRequest)} + * or {@link #from(JSONRPCRequest)}. The constructor validates the JSON-RPC method name and + * the structure of the {@code params} object.

+ */ +public class MCPInitialize extends MCPRequest { + + public static final String METHOD = "initialize"; + + private final String protocolVersion; + private final Map capabilities; + private final ClientInfo clientInfo; + + public MCPInitialize(JSONRPCRequest request) { + super(request, METHOD); + + if (!request.hasNamedParams()) { + throw new IllegalArgumentException( + "MCP 'initialize' requires a params object (named parameters)"); + } + + Map params = request.getParamsMap(); + this.protocolVersion = requireString(params, "protocolVersion"); + this.capabilities = asMap(params.get("capabilities")); + this.clientInfo = ClientInfo.from(asMap(params.get("clientInfo"))); + } + + /** Static factory equivalent to {@link #MCPInitialize(JSONRPCRequest)}. */ + public static MCPInitialize from(JSONRPCRequest request) { + return new MCPInitialize(request); + } + + // ---------- Accessors ---------- + + public String getProtocolVersion() { + return protocolVersion; + } + + public Map getCapabilities() { + return capabilities; + } + + public ClientInfo getClientInfo() { + return clientInfo; + } + + // ---------- Helpers ---------- + + private static String requireString(Map params, String key) { + Object v = params.get(key); + if (!(v instanceof String s) || s.isBlank()) { + throw new IllegalArgumentException( + "MCP 'initialize' params: '" + key + "' must be a non-empty string"); + } + return s; + } + + @SuppressWarnings("unchecked") + private static Map asMap(Object value) { + if (value == null) return Collections.emptyMap(); + if (value instanceof Map m) return (Map) m; + throw new IllegalArgumentException("Expected JSON object, got: " + value.getClass().getSimpleName()); + } + + @Override + public String toString() { + return "MCPInitialize{" + + "id=" + getId() + + ", protocolVersion='" + protocolVersion + '\'' + + ", capabilities=" + capabilities + + ", clientInfo=" + clientInfo + + '}'; + } + + // ---------- Nested types ---------- + + /** Identification of the connecting MCP client. */ + public static final class ClientInfo { + private final String name; + private final String version; + + public ClientInfo(String name, String version) { + this.name = name; + this.version = version; + } + + static ClientInfo from(Map map) { + if (map == null || map.isEmpty()) return new ClientInfo(null, null); + Object name = map.get("name"); + Object version = map.get("version"); + return new ClientInfo( + name == null ? null : name.toString(), + version == null ? null : version.toString()); + } + + public String getName() { + return name; + } + + public String getVersion() { + return version; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof ClientInfo that)) return false; + return Objects.equals(name, that.name) && Objects.equals(version, that.version); + } + + @Override + public int hashCode() { + return Objects.hash(name, version); + } + + @Override + public String toString() { + return "ClientInfo{name='" + name + "', version='" + version + "'}"; + } + } +} diff --git a/core/src/main/java/com/predic8/membrane/core/mcp/MCPInitializeResponse.java b/core/src/main/java/com/predic8/membrane/core/mcp/MCPInitializeResponse.java new file mode 100644 index 0000000000..4f8d9317c4 --- /dev/null +++ b/core/src/main/java/com/predic8/membrane/core/mcp/MCPInitializeResponse.java @@ -0,0 +1,221 @@ +package com.predic8.membrane.core.mcp; + +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonPropertyOrder; +import com.predic8.membrane.core.jsonrpc.JSONRPCRequest; + +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Objects; + +/** + * Typed response for the MCP {@code initialize} method. + * + *

Extends {@link MCPResponse} so that all JSON-RPC 2.0 envelope concerns + * ({@code jsonrpc}, {@code id}, serialization) are handled by the base class.

+ * + *

Wire format:

+ * 
{@code
+ * {
+ *   "jsonrpc": "2.0",
+ *   "id":      ,
+ *   "result": {
+ *     "protocolVersion": "2024-11-05",
+ *     "capabilities":   { ... },
+ *     "serverInfo":     { "name": "...", "version": "..." },
+ *     "instructions":   "optional free-form text"
+ *   }
+ * }
+ * }
+ */ +public class MCPInitializeResponse extends MCPResponse { + + // ---------- Constructors ---------- + + /** Creates a response with no id and an empty {@link Result}. */ + public MCPInitializeResponse() { + super(null, new Result()); + } + + /** + * Creates a response with an explicit {@code id} and {@link Result}. + * + * @param id the id to echo (String, Number, or null) + * @param result the MCP result payload; must not be null + */ + public MCPInitializeResponse(Object id, Result result) { + super(id, result); + } + + /** + * Creates a response from an already-parsed {@link MCPInitialize} request. + * Echoes the request's {@code id} and pre-populates {@code protocolVersion}. + */ + public MCPInitializeResponse(MCPInitialize request) { + super(request.getId(), initialResult(request.getProtocolVersion())); + } + + /** + * Creates a response directly from a raw {@link JSONRPCRequest}. + * Validates the method name and echoes the {@code id}. + * + * @throws IllegalArgumentException if the method is not {@code initialize} or params are missing + */ + public MCPInitializeResponse(JSONRPCRequest request) { + this(MCPInitialize.from(request)); + } + + /** Static factory equivalent to {@link #MCPInitializeResponse(JSONRPCRequest)}. */ + public static MCPInitializeResponse from(JSONRPCRequest request) { + return new MCPInitializeResponse(request); + } + + /** Static factory equivalent to {@link #MCPInitializeResponse(MCPInitialize)}. */ + public static MCPInitializeResponse from(MCPInitialize request) { + return new MCPInitializeResponse(request); + } + + private static Result initialResult(String protocolVersion) { + Result r = new Result(); + r.setProtocolVersion(protocolVersion); + return r; + } + + // ---------- Builder-style helpers ---------- + + public MCPInitializeResponse withProtocolVersion(String protocolVersion) { + getResult().setProtocolVersion(protocolVersion); + return this; + } + + public MCPInitializeResponse withServerInfo(String name, String version) { + getResult().setServerInfo(new ServerInfo(name, version)); + return this; + } + + public MCPInitializeResponse withCapabilities(Map capabilities) { + getResult().setCapabilities(capabilities); + return this; + } + + public MCPInitializeResponse withCapability(String key, Object value) { + Result result = getResult(); + if (result.capabilities == null) { + result.capabilities = new LinkedHashMap<>(); + } + result.capabilities.put(key, value); + return this; + } + + public MCPInitializeResponse withInstructions(String instructions) { + getResult().setInstructions(instructions); + return this; + } + + @Override + public String toString() { + return "MCPInitializeResponse{id=" + getId() + ", result=" + getResult() + "}"; + } + + // ---------- Nested types ---------- + + @JsonInclude(JsonInclude.Include.NON_NULL) + @JsonPropertyOrder({"protocolVersion", "capabilities", "serverInfo", "instructions"}) + public static final class Result { + + @JsonProperty("protocolVersion") + private String protocolVersion; + + @JsonProperty("capabilities") + private Map capabilities = new LinkedHashMap<>(); + + @JsonProperty("serverInfo") + private ServerInfo serverInfo; + + @JsonProperty("instructions") + private String instructions; + + public Result() {} + + public Result(String protocolVersion, Map capabilities, ServerInfo serverInfo) { + this.protocolVersion = protocolVersion; + this.capabilities = capabilities != null ? capabilities : new LinkedHashMap<>(); + this.serverInfo = serverInfo; + } + + public String getProtocolVersion() { return protocolVersion; } + public void setProtocolVersion(String protocolVersion) { this.protocolVersion = protocolVersion; } + + public Map getCapabilities() { return capabilities; } + public void setCapabilities(Map capabilities) { this.capabilities = capabilities; } + + public ServerInfo getServerInfo() { return serverInfo; } + public void setServerInfo(ServerInfo serverInfo) { this.serverInfo = serverInfo; } + + public String getInstructions() { return instructions; } + public void setInstructions(String instructions) { this.instructions = instructions; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof Result that)) return false; + return Objects.equals(protocolVersion, that.protocolVersion) + && Objects.equals(capabilities, that.capabilities) + && Objects.equals(serverInfo, that.serverInfo) + && Objects.equals(instructions, that.instructions); + } + + @Override + public int hashCode() { + return Objects.hash(protocolVersion, capabilities, serverInfo, instructions); + } + + @Override + public String toString() { + return "Result{protocolVersion='" + protocolVersion + "', capabilities=" + capabilities + + ", serverInfo=" + serverInfo + ", instructions='" + instructions + "'}"; + } + } + + @JsonInclude(JsonInclude.Include.NON_NULL) + @JsonPropertyOrder({"name", "version"}) + public static final class ServerInfo { + + @JsonProperty("name") + private String name; + + @JsonProperty("version") + private String version; + + public ServerInfo() {} + + public ServerInfo(String name, String version) { + this.name = name; + this.version = version; + } + + public String getName() { return name; } + public void setName(String name) { this.name = name; } + + public String getVersion() { return version; } + public void setVersion(String version) { this.version = version; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof ServerInfo that)) return false; + return Objects.equals(name, that.name) && Objects.equals(version, that.version); + } + + @Override + public int hashCode() { + return Objects.hash(name, version); + } + + @Override + public String toString() { + return "ServerInfo{name='" + name + "', version='" + version + "'}"; + } + } +} diff --git a/core/src/main/java/com/predic8/membrane/core/mcp/MCPInitialized.java b/core/src/main/java/com/predic8/membrane/core/mcp/MCPInitialized.java new file mode 100644 index 0000000000..27d63ee716 --- /dev/null +++ b/core/src/main/java/com/predic8/membrane/core/mcp/MCPInitialized.java @@ -0,0 +1,40 @@ +package com.predic8.membrane.core.mcp; + +import com.predic8.membrane.core.jsonrpc.JSONRPCRequest; + +/** + * Typed view of the MCP {@code notifications/initialized} notification. + * + *

After the server has responded to {@code initialize}, the client sends this + * notification to signal that it is ready to begin normal operations. The server + * MUST NOT send any requests to the client before receiving this notification.

+ * + *

Per JSON-RPC 2.0, notifications carry no {@code id} and require no response.

+ * + *

Wire format:

+ * 
{@code
+ * {
+ *   "jsonrpc": "2.0",
+ *   "method":  "notifications/initialized",
+ *   "params":  {}
+ * }
+ * }
+ */ +public class MCPInitialized extends MCPNotification { + + public static final String METHOD = "notifications/initialized"; + + public MCPInitialized(JSONRPCRequest request) { + super(request, METHOD); + } + + /** Static factory equivalent to {@link #MCPInitialized(JSONRPCRequest)}. */ + public static MCPInitialized from(JSONRPCRequest request) { + return new MCPInitialized(request); + } + + @Override + public String toString() { + return "MCPInitialized{method='" + METHOD + "'}"; + } +} diff --git a/core/src/main/java/com/predic8/membrane/core/mcp/MCPNotification.java b/core/src/main/java/com/predic8/membrane/core/mcp/MCPNotification.java new file mode 100644 index 0000000000..eeefa8e421 --- /dev/null +++ b/core/src/main/java/com/predic8/membrane/core/mcp/MCPNotification.java @@ -0,0 +1,51 @@ +package com.predic8.membrane.core.mcp; + +import com.predic8.membrane.core.jsonrpc.JSONRPCRequest; + +import java.util.Objects; + +/** + * Abstract base class for all MCP notifications (JSON-RPC messages without an + * {@code id} that require no response). + * + *

Subclasses declare their {@code METHOD} constant and may extract optional + * notification-specific parameters in their own constructor:

+ * 
{@code
+ * public class MCPInitialized extends MCPNotification {
+ *     public static final String METHOD = "notifications/initialized";
+ *
+ *     public MCPInitialized(JSONRPCRequest request) {
+ *         super(request, METHOD);
+ *     }
+ * }
+ * }
+ * + * @see MCPRequest for requests that carry an {@code id} and expect a response + */ +public abstract class MCPNotification { + + /** + * Validates the JSON-RPC request as a proper notification. + * + * @param request the raw JSON-RPC 2.0 request + * @param expectedMethod the MCP method this class handles (e.g. {@code "notifications/initialized"}) + * @throws IllegalArgumentException if the method name does not match or the + * request carries an {@code id} (making it a + * request rather than a notification) + */ + protected MCPNotification(JSONRPCRequest request, String expectedMethod) { + Objects.requireNonNull(request, "request must not be null"); + Objects.requireNonNull(expectedMethod, "expectedMethod must not be null"); + + if (!expectedMethod.equals(request.getMethod())) { + throw new IllegalArgumentException( + "Expected JSON-RPC method '" + expectedMethod + + "' but got '" + request.getMethod() + "'"); + } + if (!request.isNotification()) { + throw new IllegalArgumentException( + "'" + expectedMethod + "' must be a notification (no 'id')" + + ", but id was: " + request.getId()); + } + } +} diff --git a/core/src/main/java/com/predic8/membrane/core/mcp/MCPRequest.java b/core/src/main/java/com/predic8/membrane/core/mcp/MCPRequest.java new file mode 100644 index 0000000000..be10c1542c --- /dev/null +++ b/core/src/main/java/com/predic8/membrane/core/mcp/MCPRequest.java @@ -0,0 +1,62 @@ +package com.predic8.membrane.core.mcp; + +import com.predic8.membrane.core.jsonrpc.JSONRPCRequest; + +import java.util.Objects; + +/** + * Abstract base class for all MCP requests (JSON-RPC calls that carry an {@code id} + * and expect a response). + * + *

Subclasses declare their {@code METHOD} constant and extract any + * method-specific parameters in their own constructor:

+ * 
{@code
+ * public class MCPToolsList extends MCPRequest {
+ *     public static final String METHOD = "tools/list";
+ *
+ *     public MCPToolsList(JSONRPCRequest request) {
+ *         super(request, METHOD);
+ *         // extract params...
+ *     }
+ * }
+ * }
+ * + * @see MCPNotification for notifications (no {@code id}, no response) + * @see MCPResponse for the corresponding response side + */ +public abstract class MCPRequest { + + private final Object id; + + /** + * Validates the JSON-RPC request and extracts the {@code id}. + * + * @param request the raw JSON-RPC 2.0 request + * @param expectedMethod the MCP method this class handles (e.g. {@code "tools/list"}) + * @throws IllegalArgumentException if the method name does not match + */ + protected MCPRequest(JSONRPCRequest request, String expectedMethod) { + Objects.requireNonNull(request, "request must not be null"); + Objects.requireNonNull(expectedMethod, "expectedMethod must not be null"); + + if (!expectedMethod.equals(request.getMethod())) { + throw new IllegalArgumentException( + "Expected JSON-RPC method '" + expectedMethod + + "' but got '" + request.getMethod() + "'"); + } + + this.id = request.getId(); + } + + // ---------- Accessors ---------- + + /** Returns the JSON-RPC {@code id} of this request (String, Number, or null). */ + public Object getId() { + return id; + } + + /** Convenience accessor — returns the id as String, or null. */ + public String getIdAsString() { + return id == null ? null : id.toString(); + } +} diff --git a/core/src/main/java/com/predic8/membrane/core/mcp/MCPResponse.java b/core/src/main/java/com/predic8/membrane/core/mcp/MCPResponse.java new file mode 100644 index 0000000000..f9a27bd1d6 --- /dev/null +++ b/core/src/main/java/com/predic8/membrane/core/mcp/MCPResponse.java @@ -0,0 +1,93 @@ +package com.predic8.membrane.core.mcp; + +import com.predic8.membrane.core.jsonrpc.JSONRPCResponse; + +import java.io.IOException; +import java.io.OutputStream; +import java.util.Objects; + +/** + * Abstract generic base class for all MCP responses. + * + *

Wraps a {@link JSONRPCResponse} and delegates all JSON-RPC 2.0 envelope + * concerns ({@code jsonrpc}, {@code id}, serialization) to it. Subclasses only + * need to own their MCP-specific {@code Result} type {@code R}.

+ * + *

Typical subclass:

+ * 
{@code
+ * public class MCPToolsListResponse extends MCPResponse {
+ *
+ *     public MCPToolsListResponse(MCPToolsList request) {
+ *         super(request.getId(), new Result());
+ *     }
+ *
+ *     public MCPToolsListResponse withTool(Tool tool) {
+ *         getResult().getTools().add(tool);
+ *         return this;
+ *     }
+ *
+ *     // ... nested Result and Tool classes
+ * }
+ * }
+ * + * @param the MCP-specific result type carried in the JSON-RPC {@code result} field + * + * @see MCPRequest for the request side + * @see JSONRPCResponse for the underlying envelope + */ +public abstract class MCPResponse { + + /** The JSON-RPC 2.0 envelope — owns jsonrpc, id, and result serialization. */ + protected final JSONRPCResponse rpcResponse; + + /** + * Creates a success response with the given {@code id} and initial {@code result}. + * + * @param id the id to echo from the request (String, Number, or null) + * @param result the MCP-specific result payload; must not be null + */ + protected MCPResponse(Object id, R result) { + Objects.requireNonNull(result, "result must not be null"); + this.rpcResponse = JSONRPCResponse.success(id, result); + } + + // ---------- Serialization — delegates to JSONRPCResponse ---------- + + /** Serializes the full JSON-RPC 2.0 envelope + MCP result to a JSON string. */ + public String toJson() throws IOException { + return rpcResponse.toJson(); + } + + /** Writes the full JSON-RPC 2.0 envelope + MCP result to the given stream. */ + public void writeTo(OutputStream os) throws IOException { + rpcResponse.writeTo(os); + } + + /** Exposes the underlying {@link JSONRPCResponse} for lower-level access if needed. */ + public JSONRPCResponse toRpcResponse() { + return rpcResponse; + } + + // ---------- Accessors — delegate to JSONRPCResponse ---------- + + public String getJsonrpc() { + return rpcResponse.getJsonrpc(); + } + + public Object getId() { + return rpcResponse.getId(); + } + + public void setId(Object id) { + rpcResponse.setId(id); + } + + @SuppressWarnings("unchecked") + public R getResult() { + return (R) rpcResponse.getResult(); + } + + public void setResult(R result) { + rpcResponse.setResult(result); + } +} diff --git a/core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsCall.java b/core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsCall.java new file mode 100644 index 0000000000..5bbf8b1a3a --- /dev/null +++ b/core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsCall.java @@ -0,0 +1,100 @@ +package com.predic8.membrane.core.mcp; + +import com.predic8.membrane.core.jsonrpc.JSONRPCRequest; + +import java.util.Collections; +import java.util.Map; + +/** + * Typed view of an MCP {@code tools/call} request. + * + *

Wire format:

+ * 
{@code
+ * {
+ *   "jsonrpc": "2.0",
+ *   "id":      2,
+ *   "method":  "tools/call",
+ *   "params": {
+ *     "name":      "listProxies",
+ *     "arguments": { "query": "..." }
+ *   }
+ * }
+ * }
+ * + *

{@code arguments} is optional and defaults to an empty map if absent.

+ */ +public class MCPToolsCall extends MCPRequest { + + public static final String METHOD = "tools/call"; + + /** The name of the tool to invoke — matches {@code Tool.name} from {@code tools/list}. */ + private final String name; + + /** The tool's input arguments, keyed by parameter name. Empty map if none were provided. */ + private final Map arguments; + + public MCPToolsCall(JSONRPCRequest request) { + super(request, METHOD); + + if (!request.hasNamedParams()) { + throw new IllegalArgumentException( + "MCP 'tools/call' requires a params object (named parameters)"); + } + + Map params = request.getParamsMap(); + this.name = requireString(params, "name"); + this.arguments = extractArguments(params); + } + + /** Static factory equivalent to {@link #MCPToolsCall(JSONRPCRequest)}. */ + public static MCPToolsCall from(JSONRPCRequest request) { + return new MCPToolsCall(request); + } + + // ---------- Helpers ---------- + + private static String requireString(Map params, String key) { + Object v = params.get(key); + if (!(v instanceof String s) || s.isBlank()) { + throw new IllegalArgumentException( + "MCP 'tools/call' params: '" + key + "' must be a non-empty string"); + } + return s; + } + + @SuppressWarnings("unchecked") + private static Map extractArguments(Map params) { + Object args = params.get("arguments"); + if (args == null) return Collections.emptyMap(); + if (args instanceof Map m) return (Map) m; + throw new IllegalArgumentException( + "MCP 'tools/call' params: 'arguments' must be a JSON object"); + } + + // ---------- Accessors ---------- + + /** Returns the name of the tool to invoke. */ + public String getName() { + return name; + } + + /** Returns the tool's input arguments, or an empty map if none were provided. */ + public Map getArguments() { + return arguments; + } + + /** Convenience: returns a single argument by key, or {@code null} if absent. */ + public Object getArgument(String key) { + return arguments.get(key); + } + + /** Returns {@code true} if the caller provided at least one argument. */ + public boolean hasArguments() { + return !arguments.isEmpty(); + } + + @Override + public String toString() { + return "MCPToolsCall{id=" + getId() + ", name='" + name + "', arguments=" + arguments + "}"; + } +} diff --git a/core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsCallResponse.java b/core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsCallResponse.java new file mode 100644 index 0000000000..026b032375 --- /dev/null +++ b/core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsCallResponse.java @@ -0,0 +1,336 @@ +package com.predic8.membrane.core.mcp; + +import com.fasterxml.jackson.annotation.*; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.predic8.membrane.core.jsonrpc.JSONRPCRequest; + +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; + +/** + * Typed response for the MCP {@code tools/call} method. + * + *

Wire format (success):

+ * 
{@code
+ * {
+ *   "jsonrpc": "2.0",
+ *   "id": 2,
+ *   "result": {
+ *     "content": [
+ *       { "type": "text", "text": "Found 3 proxies: ..." }
+ *     ],
+ *     "isError": false
+ *   }
+ * }
+ * }
+ * + *

Wire format (tool-level error — note: this is still a JSON-RPC success + * response; the tool itself signals failure via {@code isError: true}):

+ * 
{@code
+ * {
+ *   "jsonrpc": "2.0",
+ *   "id": 2,
+ *   "result": {
+ *     "content": [
+ *       { "type": "text", "text": "Tool 'listProxies' failed: ..." }
+ *     ],
+ *     "isError": true
+ *   }
+ * }
+ * }
+ * + *

Usage:

+ * 
{@code
+ * MCPToolsCallResponse.from(call)
+ *         .withText("Found 3 proxies: api, soap, rest");
+ *
+ * // Tool-level error:
+ * MCPToolsCallResponse.toolError(call, "Unknown tool: " + call.getName());
+ * }
+ */ +public class MCPToolsCallResponse extends MCPResponse { + + private static final ObjectMapper OM = new ObjectMapper(); + + // ---------- Constructors ---------- + + public MCPToolsCallResponse() { + super(null, new Result()); + } + + public MCPToolsCallResponse(Object id, Result result) { + super(id, result); + } + + public MCPToolsCallResponse(MCPToolsCall request) { + super(request.getId(), new Result()); + } + + public MCPToolsCallResponse(JSONRPCRequest request) { + this(MCPToolsCall.from(request)); + } + + // ---------- Static factories ---------- + + public static MCPToolsCallResponse from(JSONRPCRequest request) { + return new MCPToolsCallResponse(request); + } + + public static MCPToolsCallResponse from(MCPToolsCall request) { + return new MCPToolsCallResponse(request); + } + + /** + * Creates a tool-level error response ({@code isError: true}). + * Note: this is still a valid JSON-RPC success response — the tool itself signals failure. + */ + public static MCPToolsCallResponse toolError(MCPToolsCall request, String message) { + return new MCPToolsCallResponse(request) + .withText(message) + .withIsError(true); + } + + // ---------- Builder-style helpers ---------- + + /** Appends a plain text content item. */ + public MCPToolsCallResponse withText(String text) { + getResult().content.add(new TextContent(text)); + return this; + } + + /** Appends a base64-encoded image content item. */ + public MCPToolsCallResponse withImage(String base64Data, String mimeType) { + getResult().content.add(new ImageContent(base64Data, mimeType)); + return this; + } + + /** Appends a resource content item. */ + public MCPToolsCallResponse withResource(String uri, String text) { + getResult().content.add(new ResourceContent(uri, text)); + return this; + } + + /** + * Serializes {@code data} to a JSON string and appends it as a {@link TextContent} item. + * + *

This is the simplest way to return structured data — the LLM receives valid + * JSON text it can reason about directly:

+ * 
{@code
+     * .withJson(List.of(Map.of("name", "api", "port", 8080),
+     *                   Map.of("name", "soap", "port", 8443)))
+     * // → { "type": "text", "text": "[{\"name\":\"api\",\"port\":8080}, ...]" }
+     * }
+ * + * @throws RuntimeException if {@code data} cannot be serialized to JSON + */ + public MCPToolsCallResponse withJson(Object data) { + try { + getResult().content.add(new TextContent(OM.writeValueAsString(data))); + } catch (Exception e) { + throw new RuntimeException("Failed to serialize data to JSON", e); + } + return this; + } + + /** + * Serializes {@code data} to a JSON string and appends it as a {@link ResourceContent} + * with {@code mimeType: "application/json"} and the given {@code uri}. + * + *

Prefer this over {@link #withJson(Object)} when the data represents a + * named resource that clients may want to reference by URI:

+ * 
{@code
+     * .withJsonResource("membrane://proxies", proxiesList)
+     * // → { "type": "resource", "resource": { "uri": "membrane://proxies",
+     * //       "mimeType": "application/json", "text": "[...]" } }
+     * }
+ * + * @throws RuntimeException if {@code data} cannot be serialized to JSON + */ + public MCPToolsCallResponse withJsonResource(String uri, Object data) { + try { + String json = OM.writeValueAsString(data); + ResourceContent.Resource resource = new ResourceContent.Resource(uri, json); + resource.setMimeType("application/json"); + getResult().content.add(new ResourceContent(resource)); + } catch (Exception e) { + throw new RuntimeException("Failed to serialize data to JSON", e); + } + return this; + } + + /** Appends an arbitrary content item. */ + public MCPToolsCallResponse withContent(Content content) { + Objects.requireNonNull(content, "content must not be null"); + getResult().content.add(content); + return this; + } + + /** Marks the response as a tool-level error. */ + public MCPToolsCallResponse withIsError(boolean isError) { + getResult().setIsError(isError); + return this; + } + + @Override + public String toString() { + return "MCPToolsCallResponse{id=" + getId() + ", result=" + getResult() + "}"; + } + + // ---------- Nested types ---------- + + @JsonInclude(JsonInclude.Include.NON_NULL) + @JsonPropertyOrder({"content", "isError"}) + public static final class Result { + + @JsonProperty("content") + private List content = new ArrayList<>(); + + /** {@code true} if the tool itself encountered an error (distinct from a JSON-RPC error). */ + @JsonProperty("isError") + private Boolean isError; + + public Result() {} + + public List getContent() { return content; } + public void setContent(List content) { this.content = content; } + + public Boolean getIsError() { return isError; } + public void setIsError(Boolean isError) { this.isError = isError; } + + @Override + public String toString() { + return "Result{content=" + content + ", isError=" + isError + "}"; + } + } + + // ---------- Content types ---------- + + /** + * Base class for all MCP content items. + * Jackson uses the {@code type} field to deserialize the correct subtype. + */ + @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "type") + @JsonSubTypes({ + @JsonSubTypes.Type(value = TextContent.class, name = "text"), + @JsonSubTypes.Type(value = ImageContent.class, name = "image"), + @JsonSubTypes.Type(value = ResourceContent.class, name = "resource") + }) + @JsonInclude(JsonInclude.Include.NON_NULL) + public abstract static class Content { + public abstract String getType(); + } + + /** A plain-text content item. */ + @JsonPropertyOrder({"type", "text"}) + public static final class TextContent extends Content { + + @JsonProperty("text") + private String text; + + public TextContent() {} + + public TextContent(String text) { + this.text = Objects.requireNonNull(text, "text must not be null"); + } + + @Override + public String getType() { return "text"; } + + public String getText() { return text; } + public void setText(String text) { this.text = text; } + + @Override + public String toString() { return "TextContent{text='" + text + "'}"; } + } + + /** A base64-encoded image content item. */ + @JsonPropertyOrder({"type", "data", "mimeType"}) + public static final class ImageContent extends Content { + + @JsonProperty("data") + private String data; + + @JsonProperty("mimeType") + private String mimeType; + + public ImageContent() {} + + public ImageContent(String data, String mimeType) { + this.data = Objects.requireNonNull(data, "data must not be null"); + this.mimeType = Objects.requireNonNull(mimeType, "mimeType must not be null"); + } + + @Override + public String getType() { return "image"; } + + public String getData() { return data; } + public void setData(String data) { this.data = data; } + + public String getMimeType() { return mimeType; } + public void setMimeType(String mimeType) { this.mimeType = mimeType; } + + @Override + public String toString() { return "ImageContent{mimeType='" + mimeType + "'}"; } + } + + /** A resource reference content item (URI + optional inline text or blob). */ + @JsonPropertyOrder({"type", "resource"}) + public static final class ResourceContent extends Content { + + @JsonProperty("resource") + private Resource resource; + + public ResourceContent() {} + + public ResourceContent(String uri, String text) { + this.resource = new Resource(uri, text); + } + + public ResourceContent(Resource resource) { + this.resource = Objects.requireNonNull(resource, "resource must not be null"); + } + + @Override + public String getType() { return "resource"; } + + public Resource getResource() { return resource; } + public void setResource(Resource resource) { this.resource = resource; } + + @Override + public String toString() { return "ResourceContent{resource=" + resource + "}"; } + + @JsonInclude(JsonInclude.Include.NON_NULL) + @JsonPropertyOrder({"uri", "mimeType", "text"}) + public static final class Resource { + + @JsonProperty("uri") + private String uri; + + @JsonProperty("mimeType") + private String mimeType; + + @JsonProperty("text") + private String text; + + public Resource() {} + + public Resource(String uri, String text) { + this.uri = Objects.requireNonNull(uri, "uri must not be null"); + this.text = text; + } + + public String getUri() { return uri; } + public void setUri(String uri) { this.uri = uri; } + + public String getMimeType() { return mimeType; } + public void setMimeType(String mimeType) { this.mimeType = mimeType; } + + public String getText() { return text; } + public void setText(String text) { this.text = text; } + + @Override + public String toString() { return "Resource{uri='" + uri + "'}"; } + } + } +} diff --git a/core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsList.java b/core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsList.java new file mode 100644 index 0000000000..c0beba054c --- /dev/null +++ b/core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsList.java @@ -0,0 +1,69 @@ +package com.predic8.membrane.core.mcp; + +import com.predic8.membrane.core.jsonrpc.JSONRPCRequest; + +/** + * Typed view of an MCP {@code tools/list} request. + * + *

The client sends this request to retrieve all tools the server exposes. + * The response is a {@link MCPToolsListResponse}.

+ * + *

Wire format:

+ * 
{@code
+ * {
+ *   "jsonrpc": "2.0",
+ *   "id":      1,
+ *   "method":  "tools/list",
+ *   "params":  { "cursor": "optional-pagination-cursor" }
+ * }
+ * }
+ * + *

{@code params} is optional. If present, {@code cursor} is an opaque string + * returned by a previous {@code tools/list} response as {@code nextCursor} and + * signals that the client wants the next page of results.

+ */ +public class MCPToolsList extends MCPRequest { + + public static final String METHOD = "tools/list"; + + /** + * Opaque pagination cursor, or {@code null} if the client requests the first page. + * Echoed from {@code params.cursor} of the incoming request. + */ + private final String cursor; + + public MCPToolsList(JSONRPCRequest request) { + super(request, METHOD); + this.cursor = extractCursor(request); + } + + /** Static factory equivalent to {@link #MCPToolsList(JSONRPCRequest)}. */ + public static MCPToolsList from(JSONRPCRequest request) { + return new MCPToolsList(request); + } + + // ---------- Helpers ---------- + + private static String extractCursor(JSONRPCRequest request) { + if (!request.hasNamedParams()) return null; + Object cursor = request.getParamsMap().get("cursor"); + return cursor instanceof String s ? s : null; + } + + // ---------- Accessors ---------- + + /** Returns the pagination cursor, or {@code null} for the first page. */ + public String getCursor() { + return cursor; + } + + /** Returns {@code true} if this request is for a continuation page. */ + public boolean hasCursor() { + return cursor != null; + } + + @Override + public String toString() { + return "MCPToolsList{id=" + getId() + ", cursor=" + cursor + "}"; + } +} diff --git a/core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsListResponse.java b/core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsListResponse.java new file mode 100644 index 0000000000..21ae4c041f --- /dev/null +++ b/core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsListResponse.java @@ -0,0 +1,248 @@ +package com.predic8.membrane.core.mcp; + +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonPropertyOrder; +import com.predic8.membrane.core.jsonrpc.JSONRPCRequest; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Objects; + +/** + * Typed response for the MCP {@code tools/list} method. + * + *

Extends {@link MCPResponse} so that all JSON-RPC 2.0 envelope concerns + * ({@code jsonrpc}, {@code id}, serialization) are handled by the base class.

+ * + *

Wire format:

+ * 
{@code
+ * {
+ *   "jsonrpc": "2.0",
+ *   "id": 1,
+ *   "result": {
+ *     "tools": [
+ *       {
+ *         "name":        "my_tool",
+ *         "description": "Does something useful",
+ *         "inputSchema": {
+ *           "type": "object",
+ *           "properties": { "param": { "type": "string" } },
+ *           "required":   ["param"]
+ *         }
+ *       }
+ *     ],
+ *     "nextCursor": "optional-opaque-cursor"
+ *   }
+ * }
+ * }
+ * + *

Usage:

+ * 
{@code
+ * MCPToolsListResponse resp = MCPToolsListResponse.from(toolsListRequest)
+ *         .withTool(new MCPToolsListResponse.Tool(
+ *                 "my_tool",
+ *                 "Does something useful",
+ *                 Map.of("type", "object",
+ *                        "properties", Map.of("param", Map.of("type", "string")),
+ *                        "required", List.of("param"))));
+ * resp.writeTo(outputStream);
+ * }
+ */ +public class MCPToolsListResponse extends MCPResponse { + + // ---------- Constructors ---------- + + /** Creates a response with no id and an empty {@link Result}. */ + public MCPToolsListResponse() { + super(null, new Result()); + } + + /** + * Creates a response with an explicit {@code id} and {@link Result}. + * + * @param id the id to echo (String, Number, or null) + * @param result the MCP result payload; must not be null + */ + public MCPToolsListResponse(Object id, Result result) { + super(id, result); + } + + /** + * Creates a response from an already-parsed {@link MCPToolsList} request. + * Echoes the request's {@code id}. + */ + public MCPToolsListResponse(MCPToolsList request) { + super(request.getId(), new Result()); + } + + /** + * Creates a response directly from a raw {@link JSONRPCRequest}. + * Validates the method name and echoes the {@code id}. + * + * @throws IllegalArgumentException if the method is not {@code tools/list} + */ + public MCPToolsListResponse(JSONRPCRequest request) { + this(MCPToolsList.from(request)); + } + + /** Static factory equivalent to {@link #MCPToolsListResponse(JSONRPCRequest)}. */ + public static MCPToolsListResponse from(JSONRPCRequest request) { + return new MCPToolsListResponse(request); + } + + /** Static factory equivalent to {@link #MCPToolsListResponse(MCPToolsList)}. */ + public static MCPToolsListResponse from(MCPToolsList request) { + return new MCPToolsListResponse(request); + } + + // ---------- Builder-style helpers ---------- + + /** Adds a single tool to the result. */ + public MCPToolsListResponse withTool(Tool tool) { + Objects.requireNonNull(tool, "tool must not be null"); + getResult().tools.add(tool); + return this; + } + + /** Adds multiple tools to the result. */ + public MCPToolsListResponse withTools(List tools) { + Objects.requireNonNull(tools, "tools must not be null"); + getResult().tools.addAll(tools); + return this; + } + + /** + * Sets the {@code nextCursor} for pagination. + * Pass {@code null} (or omit) to indicate this is the last page. + */ + public MCPToolsListResponse withNextCursor(String nextCursor) { + getResult().setNextCursor(nextCursor); + return this; + } + + @Override + public String toString() { + return "MCPToolsListResponse{id=" + getId() + ", result=" + getResult() + "}"; + } + + // ---------- Nested types ---------- + + @JsonInclude(JsonInclude.Include.NON_NULL) + @JsonPropertyOrder({"tools", "nextCursor"}) + public static final class Result { + + @JsonProperty("tools") + private List tools = new ArrayList<>(); + + /** + * Opaque pagination cursor for the next page, or {@code null} if this is the last page. + * The client passes this value back as {@code params.cursor} in the next {@code tools/list}. + */ + @JsonProperty("nextCursor") + private String nextCursor; + + public Result() {} + + public Result(List tools, String nextCursor) { + this.tools = tools != null ? tools : new ArrayList<>(); + this.nextCursor = nextCursor; + } + + public List getTools() { return tools; } + public void setTools(List tools) { this.tools = tools; } + + public String getNextCursor() { return nextCursor; } + public void setNextCursor(String nextCursor) { this.nextCursor = nextCursor; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof Result that)) return false; + return Objects.equals(tools, that.tools) + && Objects.equals(nextCursor, that.nextCursor); + } + + @Override + public int hashCode() { + return Objects.hash(tools, nextCursor); + } + + @Override + public String toString() { + return "Result{tools=" + tools + ", nextCursor=" + nextCursor + "}"; + } + } + + /** + * Describes a single MCP tool. + * + *

The {@code inputSchema} is a JSON Schema object (type {@code "object"}) + * describing the tool's parameters. Clients use it to validate arguments before + * calling {@code tools/call}.

+ */ + @JsonInclude(JsonInclude.Include.NON_NULL) + @JsonPropertyOrder({"name", "description", "inputSchema"}) + public static final class Tool { + + @JsonProperty("name") + private String name; + + @JsonProperty("description") + private String description; + + /** + * JSON Schema describing the tool's input parameters. + * Must be an object schema, e.g.: + * 
{@code
+         * Map.of("type", "object",
+         *        "properties", Map.of("query", Map.of("type", "string")),
+         *        "required", List.of("query"))
+         * }
+ */ + @JsonProperty("inputSchema") + private Map inputSchema; + + public Tool() {} + + /** + * @param name unique tool identifier (required) + * @param description human-readable description shown to the LLM (optional) + * @param inputSchema JSON Schema object for the tool's parameters (required) + */ + public Tool(String name, String description, Map inputSchema) { + this.name = Objects.requireNonNull(name, "name must not be null"); + this.description = description; + this.inputSchema = inputSchema; + } + + public String getName() { return name; } + public void setName(String name) { this.name = name; } + + public String getDescription() { return description; } + public void setDescription(String description) { this.description = description; } + + public Map getInputSchema() { return inputSchema; } + public void setInputSchema(Map inputSchema) { this.inputSchema = inputSchema; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof Tool that)) return false; + return Objects.equals(name, that.name) + && Objects.equals(description, that.description) + && Objects.equals(inputSchema, that.inputSchema); + } + + @Override + public int hashCode() { + return Objects.hash(name, description, inputSchema); + } + + @Override + public String toString() { + return "Tool{name='" + name + "', description='" + description + "'}"; + } + } +} diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 8ca362348b..342d007873 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -14,6 +14,8 @@ PRIO 1: - @coderabbitai look through the code base for usages of these variables and suggest documentation PRIO 2: +- Remove MemoryExchangeStore + - It was used only by Membrane Monitor - jsonRPCProtection: - maxBatchSize = 0,1 -> No Batch, n = n-Batches - allow/block list for methods From 2c126634f31568ae364811e3d6dd9c4a9bb65d93 Mon Sep 17 00:00:00 2001 From: thomas Date: Thu, 23 Apr 2026 11:34:43 +0200 Subject: [PATCH 03/29] refactor: streamline MCP request processing and response generation --- .../interceptor/mcp/MembraneMCPServer.java | 208 +++++++++--------- 1 file changed, 109 insertions(+), 99 deletions(-) diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java index 5f658b1164..070eac2394 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java @@ -1,7 +1,7 @@ package com.predic8.membrane.core.interceptor.mcp; -import com.predic8.membrane.annot.Constants; import com.predic8.membrane.annot.MCElement; +import com.predic8.membrane.core.exchange.AbstractExchange; import com.predic8.membrane.core.exchange.Exchange; import com.predic8.membrane.core.http.Response; import com.predic8.membrane.core.interceptor.AbstractInterceptor; @@ -9,8 +9,11 @@ import com.predic8.membrane.core.jsonrpc.JSONRPCRequest; import com.predic8.membrane.core.mcp.*; import com.predic8.membrane.core.openapi.serviceproxy.APIProxy; +import com.predic8.membrane.core.proxies.Proxy; import com.predic8.membrane.core.proxies.SOAPProxy; import com.predic8.membrane.core.proxies.ServiceProxy; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -19,6 +22,7 @@ import java.util.Map; import java.util.Objects; +import static com.predic8.membrane.annot.Constants.VERSION; import static com.predic8.membrane.core.http.MimeType.APPLICATION_JSON; import static com.predic8.membrane.core.http.Response.ok; import static com.predic8.membrane.core.interceptor.Outcome.RETURN; @@ -37,29 +41,8 @@ public class MembraneMCPServer extends AbstractInterceptor { @Override public Outcome handleRequest(Exchange exc) { - try { - var request = JSONRPCRequest.parse(exc.getRequest().getBodyAsStreamDecoded()); - - MCPResponse mcpResponse = null; - Response response; - if (request.getMethod().equals("initialize")) { - mcpResponse = initialize(exc, request); - } else if (request.getMethod().equals("notifications/initialized")) { - // Do nothing - } else if (request.getMethod().equals("tools/list")) { - mcpResponse = toolsList(exc, request); - } else if (request.getMethod().equals("tools/call")) { - mcpResponse = toolsCall(exc, request); - } else { - System.out.println("Unknown MCP Request: " + request); - } - if (mcpResponse == null) { - response = Response.noContent().build(); - } else { - response = ok().contentType(APPLICATION_JSON).body(mcpResponse.toJson()).build(); - } - exc.setResponse(response); + exc.setResponse(createResponse(processMCPRequest(JSONRPCRequest.parse(exc.getRequest().getBodyAsStreamDecoded())))); return RETURN; } catch (IOException e) { @@ -67,122 +50,149 @@ public Outcome handleRequest(Exchange exc) { } } - private MCPResponse toolsCall(Exchange exc, JSONRPCRequest request) throws IOException { + private static Response createResponse(MCPResponse mcpResponse) throws IOException { + if (mcpResponse == null) { + return Response.noContent().build(); + } + return ok().contentType(APPLICATION_JSON).body(mcpResponse.toJson()).build(); + } + + private @Nullable MCPResponse processMCPRequest(JSONRPCRequest request) throws IOException { + switch (request.getMethod()) { + case "initialize" -> { + return initialize(request); + } + case "notifications/initialized" -> { + log.debug("MCP Client is ready"); + return null; + } + case "tools/list" -> { + return toolsList(request); + } + case "tools/call" -> { + return toolsCall(request); + } + default -> log.info("Unknown MCP Request: {}",request); + } + return null; + } + + private MCPToolsCallResponse toolsCall(JSONRPCRequest request) { var req = MCPToolsCall.from(request); log.debug("Received MCP tools call: {}", req); - if (req.getName().equals("listProxies")) { - return listProxies(req); - } else if (req.getName().equals("getExchanges")) { - return getExchanges(req); - } else if (req.getName().equals("getStatistics")) { - getRouter().getStatistics(); - } else { - log.info("Unknown tools call: " + req.getName()); + switch (req.getName()) { + case "listProxies" -> { + return listProxies(req); + } + case "getExchanges" -> { + return getExchanges(req); + } + case "getStatistics" -> getRouter().getStatistics(); + default -> log.info("Unknown tools call: " + req.getName()); } return null; } private MCPToolsCallResponse getExchanges(MCPToolsCall req) { - var exchangesRes = getRouter().getExchangeStore().getAllExchangesAsList().stream().map(e -> { - - if (e.getResponse() == null) - return null; + return MCPToolsCallResponse.from(req) + .withJson(Map.of("exchanges", getRouter().getExchangeStore() + .getAllExchangesAsList().stream() + .map(MembraneMCPServer::getExchangeDescription) + .filter(Objects::nonNull).toList())); + } - var exc = new HashMap(); - exc.put("id", e.getId()); - var request = new HashMap(); - var response = new HashMap(); + private static @Nullable HashMap getExchangeDescription(AbstractExchange e) { + if (e.getResponse() == null) + return null; - request.put("method", e.getRequest().getMethod()); - request.put("path", e.getRequest().getUri()); - request.put("body", e.getRequest().getBodyAsStringDecoded()); - request.put("headers",e.getRequest().getHeader()); + var exc = new HashMap(); + exc.put("id", e.getId()); + var request = new HashMap(); + var response = new HashMap(); - exc.put("request", request); - exc.put("response", response); - return exc; - }).filter(Objects::nonNull).toList(); + request.put("method", e.getRequest().getMethod()); + request.put("path", e.getRequest().getUri()); + request.put("body", e.getRequest().getBodyAsStringDecoded()); + request.put("headers", e.getRequest().getHeader()); - return MCPToolsCallResponse.from(req).withJson(Map.of("exchanges", exchangesRes)); + exc.put("request", request); + exc.put("response", response); + return exc; } private MCPToolsCallResponse listProxies(MCPToolsCall req) { - var proxies = getRouter().getRuleManager().getRules(); - - var proxiesDesc = proxies.stream().map(p -> { - var proxy = new HashMap(); - proxy.put("name", p.getName()); - - String type; - switch (p) { - case APIProxy ap -> { - type = "API"; - // proxy.put("openapi", ap.getOpenapi()); - } - case ServiceProxy s -> { - type = "serviceProxy"; - } - case SOAPProxy sp -> { - type = "soapProxy"; - proxy.put("wsdl", sp.getWsdl()); - proxy.put("serviceName", sp.getServiceName()); - } - default -> { - type = "unknown"; - } - } + return MCPToolsCallResponse.from(req) + .withJson(Map.of("proxies", getRouter().getRuleManager().getRules().stream().map(this::getProxyDescription).toList())); + } - var interceptors = p.getFlow().stream().map(i -> { - Map interceptor = new HashMap(); - interceptor.put("name", i.getDisplayName()); - return interceptor; - }).toList(); + private @NotNull HashMap getProxyDescription(Proxy p) { + var proxy = new HashMap(); + proxy.put("name", p.getName()); - proxy.put("statistics", getRouter().getExchangeStore().getStatistics(p.getKey())); + String type; + switch (p) { + case APIProxy ap -> { + type = "API"; + // proxy.put("openapi", ap.getOpenapi()); + } + case ServiceProxy s -> { + type = "serviceProxy"; + } + case SOAPProxy sp -> { + type = "soapProxy"; + proxy.put("wsdl", sp.getWsdl()); + proxy.put("serviceName", sp.getServiceName()); + } + default -> { + type = "unknown"; + } + } - proxy.put("interceptors", interceptors); - proxy.put("type", type); - proxy.put("rule", p.getKey().toString()); - return proxy; + var interceptors = p.getFlow().stream().map(i -> { + Map interceptor = new HashMap<>(); + interceptor.put("name", i.getDisplayName()); + return interceptor; }).toList(); - return MCPToolsCallResponse.from(req).withJson(Map.of("proxies", proxiesDesc)); + proxy.put("statistics", getRouter().getExchangeStore().getStatistics(p.getKey())); + + proxy.put("interceptors", interceptors); + proxy.put("type", type); + proxy.put("rule", p.getKey().toString()); + return proxy; } - private MCPResponse toolsList(Exchange exc, JSONRPCRequest request) throws IOException { + private MCPToolsListResponse toolsList(JSONRPCRequest request) throws IOException { log.debug("Tools list"); - var req = MCPToolsList.from(request); - var resp = MCPToolsListResponse.from(req) + return MCPToolsListResponse.from(MCPToolsList.from(request)) .withTool(new MCPToolsListResponse.Tool( "listProxies", "Lists all the proxies, e.g. API, soapProxy", Map.of("type", "object"))) .withTool(new MCPToolsListResponse.Tool("getExchanges", "Gets the last 100 HTTP exchanges", Map.of("type", "object"))); -// Map.of("type", "object", + // Map.of("type", "object", // "properties", Map.of("query", Map.of("type", "string")), // "required", List.of("query")))); - return resp; - } - - private MCPResponse initialize(Exchange exc, JSONRPCRequest request) throws IOException { - var initialize = new MCPInitialize(request); - log.debug("initialize: " + initialize); + } - var response = new MCPInitializeResponse(initialize); + private MCPInitializeResponse initialize(JSONRPCRequest request) throws IOException { + return new MCPInitializeResponse(new MCPInitialize(request)) + .withCapabilities(getCapabilities()) + .withServerInfo("Membrane", VERSION); + } + private static @NotNull HashMap getCapabilities() { var capabilities = new HashMap(); capabilities.put("tools", Map.of("lastExchanges", false)); - response.withCapabilities(capabilities) - .withServerInfo("Membrane", Constants.VERSION); - return response; + return capabilities; } @Override public String getDisplayName() { return "Membrane MCP Server"; } -} +} \ No newline at end of file From eee7cc963d2e91b03efdc330b9ed4014faab77b3 Mon Sep 17 00:00:00 2001 From: thomas Date: Thu, 23 Apr 2026 11:35:25 +0200 Subject: [PATCH 04/29] refactor: remove redundant IOException declaration in toolsList method --- .../membrane/core/interceptor/mcp/MembraneMCPServer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java index 070eac2394..f66913b902 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java @@ -165,7 +165,7 @@ private MCPToolsCallResponse listProxies(MCPToolsCall req) { return proxy; } - private MCPToolsListResponse toolsList(JSONRPCRequest request) throws IOException { + private MCPToolsListResponse toolsList(JSONRPCRequest request) { log.debug("Tools list"); return MCPToolsListResponse.from(MCPToolsList.from(request)) .withTool(new MCPToolsListResponse.Tool( From dfed9b9793d3b56093e08987a363311a605dc907 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Mon, 27 Apr 2026 11:37:50 +0200 Subject: [PATCH 05/29] Enhance MCP server error handling with detailed response generation --- .../interceptor/mcp/MembraneMCPServer.java | 43 +++++++++++++++++-- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java index f66913b902..642584f447 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java @@ -1,5 +1,6 @@ package com.predic8.membrane.core.interceptor.mcp; +import com.fasterxml.jackson.core.JsonProcessingException; import com.predic8.membrane.annot.MCElement; import com.predic8.membrane.core.exchange.AbstractExchange; import com.predic8.membrane.core.exchange.Exchange; @@ -7,6 +8,7 @@ import com.predic8.membrane.core.interceptor.AbstractInterceptor; import com.predic8.membrane.core.interceptor.Outcome; import com.predic8.membrane.core.jsonrpc.JSONRPCRequest; +import com.predic8.membrane.core.jsonrpc.JSONRPCResponse; import com.predic8.membrane.core.mcp.*; import com.predic8.membrane.core.openapi.serviceproxy.APIProxy; import com.predic8.membrane.core.proxies.Proxy; @@ -26,6 +28,7 @@ import static com.predic8.membrane.core.http.MimeType.APPLICATION_JSON; import static com.predic8.membrane.core.http.Response.ok; import static com.predic8.membrane.core.interceptor.Outcome.RETURN; +import static com.predic8.membrane.core.jsonrpc.JSONRPCResponse.*; /** * @description MCP Server for Membrane. It allows to query Membrane's internal state and operation from an LLM @@ -42,7 +45,28 @@ public class MembraneMCPServer extends AbstractInterceptor { @Override public Outcome handleRequest(Exchange exc) { try { - exc.setResponse(createResponse(processMCPRequest(JSONRPCRequest.parse(exc.getRequest().getBodyAsStreamDecoded())))); + JSONRPCRequest request; + try { + request = JSONRPCRequest.parse(exc.getRequest().getBodyAsStreamDecoded()); + } catch (JsonProcessingException e) { + exc.setResponse(createResponse(createErrorResponse(exc, null, ERR_PARSE_ERROR, "Parse error", e))); + return RETURN; + } catch (IOException e) { + exc.setResponse(createResponse(createErrorResponse(exc, null, ERR_INVALID_REQUEST, "Invalid Request", e))); + return RETURN; + } + + MCPResponse mcpResponse; + try { + mcpResponse = processMCPRequest(request); + } catch (IllegalArgumentException e) { + exc.setResponse(createResponse(createErrorResponse(exc, request, JSONRPCResponse.ERR_INVALID_PARAMS, "Invalid params", e))); + return RETURN; + } catch (Exception e) { + exc.setResponse(createResponse(createErrorResponse(exc, request, ERR_INTERNAL_ERROR, "Internal error", e))); + return RETURN; + } + exc.setResponse(createResponse(mcpResponse)); return RETURN; } catch (IOException e) { @@ -50,11 +74,24 @@ public Outcome handleRequest(Exchange exc) { } } + private JSONRPCResponse createErrorResponse(Exchange exc, @Nullable JSONRPCRequest request, int code, String message, Exception e) { + if (code == ERR_INTERNAL_ERROR) { + log.warn("Failed to handle MCP request {} {}.", exc.getRequest().getMethod(), exc.getRequest().getUri(), e); + } else { + log.info("Rejected MCP request {} {}: {}", exc.getRequest().getMethod(), exc.getRequest().getUri(), e.getMessage()); + } + return JSONRPCResponse.error(request == null ? null : request.getId(), code, message, e.getMessage()); + } + private static Response createResponse(MCPResponse mcpResponse) throws IOException { if (mcpResponse == null) { return Response.noContent().build(); } - return ok().contentType(APPLICATION_JSON).body(mcpResponse.toJson()).build(); + return createResponse(mcpResponse.toRpcResponse()); + } + + private static Response createResponse(JSONRPCResponse rpcResponse) throws IOException { + return ok().contentType(APPLICATION_JSON).body(rpcResponse.toJson()).build(); } private @Nullable MCPResponse processMCPRequest(JSONRPCRequest request) throws IOException { @@ -195,4 +232,4 @@ private MCPInitializeResponse initialize(JSONRPCRequest request) throws IOExcept public String getDisplayName() { return "Membrane MCP Server"; } -} \ No newline at end of file +} From f1b99b65234f110a5250a464b7719ca0a624d038 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Mon, 27 Apr 2026 11:43:48 +0200 Subject: [PATCH 06/29] Refactor MCP server to replace MCPResponse with JSONRPCResponse for consistent handling --- .../interceptor/mcp/MembraneMCPServer.java | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java index 642584f447..2d0a6ccef6 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java @@ -56,9 +56,9 @@ public Outcome handleRequest(Exchange exc) { return RETURN; } - MCPResponse mcpResponse; + JSONRPCResponse rpcResponse; try { - mcpResponse = processMCPRequest(request); + rpcResponse = processMCPRequest(request); } catch (IllegalArgumentException e) { exc.setResponse(createResponse(createErrorResponse(exc, request, JSONRPCResponse.ERR_INVALID_PARAMS, "Invalid params", e))); return RETURN; @@ -66,7 +66,7 @@ public Outcome handleRequest(Exchange exc) { exc.setResponse(createResponse(createErrorResponse(exc, request, ERR_INTERNAL_ERROR, "Internal error", e))); return RETURN; } - exc.setResponse(createResponse(mcpResponse)); + exc.setResponse(createResponse(rpcResponse)); return RETURN; } catch (IOException e) { @@ -80,7 +80,7 @@ private JSONRPCResponse createErrorResponse(Exchange exc, @Nullable JSONRPCReque } else { log.info("Rejected MCP request {} {}: {}", exc.getRequest().getMethod(), exc.getRequest().getUri(), e.getMessage()); } - return JSONRPCResponse.error(request == null ? null : request.getId(), code, message, e.getMessage()); + return error(request == null ? null : request.getId(), code, message, e.getMessage()); } private static Response createResponse(MCPResponse mcpResponse) throws IOException { @@ -90,26 +90,35 @@ private static Response createResponse(MCPResponse mcpResponse) throws IOExce return createResponse(mcpResponse.toRpcResponse()); } - private static Response createResponse(JSONRPCResponse rpcResponse) throws IOException { + private static Response createResponse(@Nullable JSONRPCResponse rpcResponse) throws IOException { + if (rpcResponse == null) { + return Response.noContent().build(); + } return ok().contentType(APPLICATION_JSON).body(rpcResponse.toJson()).build(); } - private @Nullable MCPResponse processMCPRequest(JSONRPCRequest request) throws IOException { + private @Nullable JSONRPCResponse processMCPRequest(JSONRPCRequest request) throws IOException { switch (request.getMethod()) { case "initialize" -> { - return initialize(request); + return initialize(request).toRpcResponse(); } case "notifications/initialized" -> { log.debug("MCP Client is ready"); return null; } case "tools/list" -> { - return toolsList(request); + return toolsList(request).toRpcResponse(); } case "tools/call" -> { - return toolsCall(request); + var response = toolsCall(request); + return response == null ? null : response.toRpcResponse(); + } + default -> { + log.info("Unknown MCP Request: {}", request); + if (request.getId() != null) { + return error(request.getId(), ERR_METHOD_NOT_FOUND, "Method not found"); + } } - default -> log.info("Unknown MCP Request: {}",request); } return null; } From 0ccdddee5e3bd8d3f33167c2bf2bdc8a59d00438 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Mon, 27 Apr 2026 11:51:34 +0200 Subject: [PATCH 07/29] Add support for fetching Membrane runtime statistics in MCP server --- .../core/interceptor/mcp/MembraneMCPServer.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java index 2d0a6ccef6..dc987b6375 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java @@ -135,13 +135,20 @@ private MCPToolsCallResponse toolsCall(JSONRPCRequest request) { case "getExchanges" -> { return getExchanges(req); } - case "getStatistics" -> getRouter().getStatistics(); + case "getStatistics" -> { + return getStatistics(req); + } default -> log.info("Unknown tools call: " + req.getName()); } return null; } + private MCPToolsCallResponse getStatistics(MCPToolsCall req) { + return MCPToolsCallResponse.from(req) + .withJson(getRouter().getStatistics()); + } + private MCPToolsCallResponse getExchanges(MCPToolsCall req) { return MCPToolsCallResponse.from(req) .withJson(Map.of("exchanges", getRouter().getExchangeStore() @@ -217,7 +224,8 @@ private MCPToolsListResponse toolsList(JSONRPCRequest request) { .withTool(new MCPToolsListResponse.Tool( "listProxies", "Lists all the proxies, e.g. API, soapProxy", Map.of("type", "object"))) - .withTool(new MCPToolsListResponse.Tool("getExchanges", "Gets the last 100 HTTP exchanges", Map.of("type", "object"))); + .withTool(new MCPToolsListResponse.Tool("getExchanges", "Gets the last 100 HTTP exchanges", Map.of("type", "object"))) + .withTool(new MCPToolsListResponse.Tool("getStatistics", "Gets Membrane runtime statistics", Map.of("type", "object"))); // Map.of("type", "object", // "properties", Map.of("query", Map.of("type", "string")), From 4d7b3d3157592dd1dda186546de61f0b48267fb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Mon, 27 Apr 2026 12:14:06 +0200 Subject: [PATCH 08/29] Limit MCP server exchange retrieval to the last 100 entries --- .../membrane/core/interceptor/mcp/MembraneMCPServer.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java index dc987b6375..c901f899a7 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java @@ -41,6 +41,7 @@ public class MembraneMCPServer extends AbstractInterceptor { private static final Logger log = LoggerFactory.getLogger(MembraneMCPServer.class); + private static final int MAX_EXCHANGES = 100; @Override public Outcome handleRequest(Exchange exc) { @@ -150,9 +151,11 @@ private MCPToolsCallResponse getStatistics(MCPToolsCall req) { } private MCPToolsCallResponse getExchanges(MCPToolsCall req) { + var exchanges = getRouter().getExchangeStore().getAllExchangesAsList(); + int start = Math.max(0, exchanges.size() - MAX_EXCHANGES); + return MCPToolsCallResponse.from(req) - .withJson(Map.of("exchanges", getRouter().getExchangeStore() - .getAllExchangesAsList().stream() + .withJson(Map.of("exchanges", exchanges.subList(start, exchanges.size()).stream() .map(MembraneMCPServer::getExchangeDescription) .filter(Objects::nonNull).toList())); } From 5de11e4e9c8853a8895a86a49c76499222b9d8f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Mon, 27 Apr 2026 12:41:57 +0200 Subject: [PATCH 09/29] Refactor JSON-RPC request handling to distinguish between missing and null IDs, improving MCP server error handling and response generation. --- .../interceptor/mcp/MembraneMCPServer.java | 2 +- .../membrane/core/jsonrpc/JSONRPCRequest.java | 86 +++++++++++++++---- 2 files changed, 70 insertions(+), 18 deletions(-) diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java index c901f899a7..eae1db306f 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java @@ -116,7 +116,7 @@ private static Response createResponse(@Nullable JSONRPCResponse rpcResponse) th } default -> { log.info("Unknown MCP Request: {}", request); - if (request.getId() != null) { + if (!request.isNotification()) { return error(request.getId(), ERR_METHOD_NOT_FOUND, "Method not found"); } } diff --git a/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCRequest.java b/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCRequest.java index b86b497b81..2128a5fb64 100644 --- a/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCRequest.java +++ b/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCRequest.java @@ -3,9 +3,11 @@ import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonSetter; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; import java.io.IOException; import java.io.InputStream; @@ -36,9 +38,12 @@ public class JSONRPCRequest { private String jsonrpc = JSONRPC_VERSION; /** May be String, Number, or null. */ - @JsonProperty("id") private Object id; + /** Distinguishes a missing {@code id} from an explicit {@code "id": null}. */ + @JsonIgnore + private boolean idPresent; + @JsonProperty("method") private String method; @@ -53,13 +58,21 @@ public class JSONRPCRequest { public JSONRPCRequest() {} public JSONRPCRequest(Object id, String method, Map paramsMap) { - this.id = id; + this(id, true, method, paramsMap); + } + + public JSONRPCRequest(Object id, String method, List paramsList) { + this(id, true, method, paramsList); + } + + public JSONRPCRequest(Object id, boolean idPresent, String method, Map paramsMap) { + setId(id, idPresent); this.method = Objects.requireNonNull(method, "method must not be null"); this.paramsMap = paramsMap; } - public JSONRPCRequest(Object id, String method, List paramsList) { - this.id = id; + public JSONRPCRequest(Object id, boolean idPresent, String method, List paramsList) { + setId(id, idPresent); this.method = Objects.requireNonNull(method, "method must not be null"); this.paramsList = paramsList; } @@ -95,14 +108,16 @@ private static JSONRPCRequest fromNode(JsonNode root) throws IOException { } req.method = methodNode.asText(); - JsonNode idNode = root.get("id"); - if (idNode != null && !idNode.isNull()) { - if (idNode.isTextual()) { - req.id = idNode.asText(); + if (root.has("id")) { + JsonNode idNode = root.get("id"); + if (idNode == null || idNode.isNull()) { + req.setId(null, true); + } else if (idNode.isTextual()) { + req.setId(idNode.asText(), true); } else if (idNode.isIntegralNumber()) { - req.id = idNode.longValue(); + req.setId(idNode.longValue(), true); } else if (idNode.isNumber()) { - req.id = idNode.numberValue(); + req.setId(idNode.numberValue(), true); } else { throw new IOException("Invalid JSON-RPC request: 'id' must be string, number, or null"); } @@ -133,18 +148,43 @@ public Object getParams() { } public String toJson() throws IOException { - return OM.writeValueAsString(this); + return OM.writeValueAsString(toNode()); } public void writeTo(OutputStream os) throws IOException { - OM.writeValue(os, this); + OM.writeValue(os, toNode()); + } + + private ObjectNode toNode() { + ObjectNode root = OM.createObjectNode(); + + if (jsonrpc != null) { + root.put("jsonrpc", jsonrpc); + } + if (idPresent) { + if (id == null) { + root.putNull("id"); + } else { + root.set("id", OM.valueToTree(id)); + } + } + if (method != null) { + root.put("method", method); + } + if (paramsMap != null) { + root.set("params", OM.valueToTree(paramsMap)); + } else if (paramsList != null) { + root.set("params", OM.valueToTree(paramsList)); + } + + return root; } // ---------- Convenience ---------- @JsonIgnore public boolean isNotification() { - return id == null; + return !idPresent; } @JsonIgnore @@ -171,14 +211,25 @@ public Object getId() { return id; } + @JsonIgnore + public boolean isIdPresent() { + return idPresent; + } + /** Convenience accessor preserving backwards compatibility — returns the id as String, or null. */ @JsonIgnore public String getIdAsString() { return id == null ? null : id.toString(); } + @JsonSetter("id") public void setId(Object id) { - this.id = id; + setId(id, true); + } + + public void setId(Object id, boolean present) { + this.id = present ? id : null; + this.idPresent = present; } public String getMethod() { @@ -214,6 +265,7 @@ public boolean equals(Object o) { if (this == o) return true; if (!(o instanceof JSONRPCRequest that)) return false; return Objects.equals(jsonrpc, that.jsonrpc) + && idPresent == that.idPresent && Objects.equals(id, that.id) && Objects.equals(method, that.method) && Objects.equals(paramsMap, that.paramsMap) @@ -222,16 +274,16 @@ public boolean equals(Object o) { @Override public int hashCode() { - return Objects.hash(jsonrpc, id, method, paramsMap, paramsList); + return Objects.hash(jsonrpc, id, idPresent, method, paramsMap, paramsList); } @Override public String toString() { return "JSONRPCRequest{" + "jsonrpc='" + jsonrpc + '\'' + - ", id=" + id + + ", id=" + (idPresent ? id : "") + ", method='" + method + '\'' + ", params=" + (paramsMap != null ? paramsMap : paramsList) + '}'; } -} \ No newline at end of file +} From afaee6d0227b41c18d863a0d6501e6c8dfe81517 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Mon, 27 Apr 2026 15:13:53 +0200 Subject: [PATCH 10/29] Add ResponseKind to JSONRPCResponse for explicit result/error distinction and customize serialization --- .../core/jsonrpc/JSONRPCResponse.java | 93 +++++++++++++++---- 1 file changed, 73 insertions(+), 20 deletions(-) diff --git a/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCResponse.java b/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCResponse.java index 91deebbaf2..cc5eaca1ca 100644 --- a/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCResponse.java +++ b/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCResponse.java @@ -1,17 +1,24 @@ package com.predic8.membrane.core.jsonrpc; +import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonPropertyOrder; +import com.fasterxml.jackson.databind.SerializerProvider; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; +import com.fasterxml.jackson.databind.ser.std.StdSerializer; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.util.Objects; +import static com.predic8.membrane.core.jsonrpc.JSONRPCResponse.ResponseKind.ERROR; +import static com.predic8.membrane.core.jsonrpc.JSONRPCResponse.ResponseKind.SUCCESS; + /** * Represents a JSON-RPC 2.0 response as defined by https://www.jsonrpc.org/specification. * @@ -42,8 +49,8 @@ * *

Standard error codes are provided as constants, e.g. {@link #ERR_PARSE_ERROR}.

*/ -@JsonInclude(JsonInclude.Include.NON_NULL) @JsonPropertyOrder({"jsonrpc", "id", "result", "error"}) +@JsonSerialize(using = JSONRPCResponse.Serializer.class) public class JSONRPCResponse { /** JSON-RPC protocol version — always {@code "2.0"}. */ @@ -73,11 +80,6 @@ public class JSONRPCResponse { /** * Present on success responses; absent (null) on error responses. - * - *

Note: per spec, {@code result} may legitimately be JSON {@code null}. - * In that case serialisation will omit the field due to {@code @JsonInclude(NON_NULL)}. - * If an explicit JSON {@code null} result must be preserved on the wire, wrap the - * value before passing it to {@link #success(Object, Object)}.

*/ @JsonProperty("result") private Object result; @@ -86,15 +88,20 @@ public class JSONRPCResponse { @JsonProperty("error") private Error error; + /** Distinguishes between a success response carrying {@code result} and an error response carrying {@code error}. */ + @JsonIgnore + private ResponseKind responseKind; + // ---------- Constructors ---------- /** No-arg constructor for Jackson deserialization. */ public JSONRPCResponse() {} - private JSONRPCResponse(Object id, Object result, Error error) { + private JSONRPCResponse(Object id, Object result, Error error, ResponseKind responseKind) { this.id = id; this.result = result; this.error = error; + this.responseKind = responseKind; } // ---------- Static factories ---------- @@ -106,7 +113,7 @@ private JSONRPCResponse(Object id, Object result, Error error) { * @param result the result value (any JSON-serialisable object, including null) */ public static JSONRPCResponse success(Object id, Object result) { - return new JSONRPCResponse(id, result, null); + return new JSONRPCResponse(id, result, null, SUCCESS); } /** @@ -117,7 +124,7 @@ public static JSONRPCResponse success(Object id, Object result) { */ public static JSONRPCResponse from(JSONRPCRequest request, Object result) { Objects.requireNonNull(request, "request must not be null"); - return new JSONRPCResponse(request.getId(), result, null); + return new JSONRPCResponse(request.getId(), result, null, SUCCESS); } /** @@ -128,7 +135,7 @@ public static JSONRPCResponse from(JSONRPCRequest request, Object result) { * @param message a human-readable error description */ public static JSONRPCResponse error(Object id, int code, String message) { - return new JSONRPCResponse(id, null, new Error(code, message, null)); + return new JSONRPCResponse(id, null, new Error(code, message, null), ERROR); } /** @@ -140,7 +147,7 @@ public static JSONRPCResponse error(Object id, int code, String message) { * @param data optional additional error information (any JSON-serialisable object) */ public static JSONRPCResponse error(Object id, int code, String message, Object data) { - return new JSONRPCResponse(id, null, new Error(code, message, data)); + return new JSONRPCResponse(id, null, new Error(code, message, data), ERROR); } /** @@ -152,7 +159,7 @@ public static JSONRPCResponse error(Object id, int code, String message, Object */ public static JSONRPCResponse errorFor(JSONRPCRequest request, int code, String message) { Objects.requireNonNull(request, "request must not be null"); - return new JSONRPCResponse(request.getId(), null, new Error(code, message, null)); + return new JSONRPCResponse(request.getId(), null, new Error(code, message, null), ERROR); } // ---------- Parsing ---------- @@ -215,13 +222,13 @@ private static JSONRPCResponse fromNode(JsonNode root) throws IOException { if (hasResult) { JsonNode resultNode = root.get("result"); - resp.result = resultNode.isNull() ? null : OM.convertValue(resultNode, Object.class); + resp.setResult(resultNode.isNull() ? null : OM.convertValue(resultNode, Object.class)); } else { JsonNode errorNode = root.get("error"); if (!errorNode.isObject()) { throw new IOException("Invalid JSON-RPC response: 'error' must be a JSON object"); } - resp.error = Error.fromNode(errorNode); + resp.setError(Error.fromNode(errorNode)); } return resp; @@ -242,13 +249,21 @@ public void writeTo(OutputStream os) throws IOException { /** Returns {@code true} if this is a success response (no {@code error} member). */ @JsonIgnore public boolean isSuccess() { - return error == null; + return getResponseKind() == SUCCESS; } /** Returns {@code true} if this is an error response. */ @JsonIgnore public boolean isError() { - return error != null; + return getResponseKind() == ERROR; + } + + @JsonIgnore + public ResponseKind getResponseKind() { + if (responseKind != null) { + return responseKind; + } + return error != null ? ERROR : SUCCESS; } /** Convenience accessor — returns the id as String, or null. */ @@ -281,7 +296,8 @@ public Object getResult() { public void setResult(Object result) { this.result = result; - if (result != null) this.error = null; + this.error = null; + this.responseKind = SUCCESS; } public Error getError() { @@ -290,7 +306,8 @@ public Error getError() { public void setError(Error error) { this.error = error; - if (error != null) this.result = null; + this.result = null; + this.responseKind = ERROR; } // ---------- Object overrides ---------- @@ -302,12 +319,13 @@ public boolean equals(Object o) { return Objects.equals(jsonrpc, that.jsonrpc) && Objects.equals(id, that.id) && Objects.equals(result, that.result) - && Objects.equals(error, that.error); + && Objects.equals(error, that.error) + && getResponseKind() == that.getResponseKind(); } @Override public int hashCode() { - return Objects.hash(jsonrpc, id, result, error); + return Objects.hash(jsonrpc, id, result, error, getResponseKind()); } @Override @@ -319,6 +337,41 @@ public String toString() { '}'; } + public enum ResponseKind { + SUCCESS, + ERROR + } + + static final class Serializer extends StdSerializer { + + Serializer() { + super(JSONRPCResponse.class); + } + + @Override + public void serialize(JSONRPCResponse value, JsonGenerator gen, SerializerProvider provider) throws IOException { + gen.writeStartObject(); + gen.writeStringField("jsonrpc", value.jsonrpc); + + if (value.id == null) { + gen.writeNullField("id"); + } else { + gen.writeFieldName("id"); + provider.defaultSerializeValue(value.id, gen); + } + + if (value.getResponseKind() == SUCCESS) { + gen.writeFieldName("result"); + provider.defaultSerializeValue(value.result, gen); + } else { + gen.writeFieldName("error"); + provider.defaultSerializeValue(value.error, gen); + } + + gen.writeEndObject(); + } + } + // ---------- Nested type: Error ---------- /** From 602d186a93a58fcbdf7834eec838bf3222b08e09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Mon, 27 Apr 2026 15:25:13 +0200 Subject: [PATCH 11/29] Improve JSON-RPC validation by rejecting notifications in MCPRequest parsing --- .../main/java/com/predic8/membrane/core/mcp/MCPRequest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/src/main/java/com/predic8/membrane/core/mcp/MCPRequest.java b/core/src/main/java/com/predic8/membrane/core/mcp/MCPRequest.java index be10c1542c..d3745d49a1 100644 --- a/core/src/main/java/com/predic8/membrane/core/mcp/MCPRequest.java +++ b/core/src/main/java/com/predic8/membrane/core/mcp/MCPRequest.java @@ -44,6 +44,10 @@ protected MCPRequest(JSONRPCRequest request, String expectedMethod) { "Expected JSON-RPC method '" + expectedMethod + "' but got '" + request.getMethod() + "'"); } + if (request.isNotification()) { + throw new IllegalArgumentException( + "'" + expectedMethod + "' must be a request with an 'id', not a notification"); + } this.id = request.getId(); } From d56dc7d9720624c0f9aa213ef03fd898541f9505 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Tue, 28 Apr 2026 07:44:19 +0200 Subject: [PATCH 12/29] Rename "lastExchanges" capability to "listChanged" in MCP server --- .../membrane/core/interceptor/mcp/MembraneMCPServer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java index eae1db306f..9bca3270a0 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java @@ -244,7 +244,7 @@ private MCPInitializeResponse initialize(JSONRPCRequest request) throws IOExcept private static @NotNull HashMap getCapabilities() { var capabilities = new HashMap(); - capabilities.put("tools", Map.of("lastExchanges", false)); + capabilities.put("tools", Map.of("listChanged", false)); return capabilities; } From 5ab2c303f2b376bc30f6bcc26e744cbe69861e03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Tue, 28 Apr 2026 07:48:32 +0200 Subject: [PATCH 13/29] Replace `Objects.requireNonNull` with static import `requireNonNull` for cleanup and consistency --- .../membrane/core/mcp/MCPToolsListResponse.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsListResponse.java b/core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsListResponse.java index 21ae4c041f..ad78539949 100644 --- a/core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsListResponse.java +++ b/core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsListResponse.java @@ -10,6 +10,8 @@ import java.util.Map; import java.util.Objects; +import static java.util.Objects.requireNonNull; + /** * Typed response for the MCP {@code tools/list} method. * @@ -101,14 +103,14 @@ public static MCPToolsListResponse from(MCPToolsList request) { /** Adds a single tool to the result. */ public MCPToolsListResponse withTool(Tool tool) { - Objects.requireNonNull(tool, "tool must not be null"); + requireNonNull(tool, "tool must not be null"); getResult().tools.add(tool); return this; } /** Adds multiple tools to the result. */ public MCPToolsListResponse withTools(List tools) { - Objects.requireNonNull(tools, "tools must not be null"); + requireNonNull(tools, "tools must not be null"); getResult().tools.addAll(tools); return this; } @@ -212,19 +214,19 @@ public Tool() {} * @param inputSchema JSON Schema object for the tool's parameters (required) */ public Tool(String name, String description, Map inputSchema) { - this.name = Objects.requireNonNull(name, "name must not be null"); + this.name = requireNonNull(name, "name must not be null"); this.description = description; - this.inputSchema = inputSchema; + this.inputSchema = requireNonNull(inputSchema, "inputSchema must not be null"); } public String getName() { return name; } - public void setName(String name) { this.name = name; } + public void setName(String name) { this.name = requireNonNull(name, "name must not be null"); } public String getDescription() { return description; } public void setDescription(String description) { this.description = description; } public Map getInputSchema() { return inputSchema; } - public void setInputSchema(Map inputSchema) { this.inputSchema = inputSchema; } + public void setInputSchema(Map inputSchema) { this.inputSchema = requireNonNull(inputSchema, "inputSchema must not be null"); } @Override public boolean equals(Object o) { From f465969f4751e2021cf87c45ba5765f64a24c824 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Tue, 28 Apr 2026 07:51:26 +0200 Subject: [PATCH 14/29] Remove unnecessary `IOException` declarations from MCP server methods for cleanup --- .gitignore | 1 + .../membrane/core/interceptor/mcp/MembraneMCPServer.java | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 59b28ca2b9..33d0b2cce0 100644 --- a/.gitignore +++ b/.gitignore @@ -129,3 +129,4 @@ maven-plugin/target/surefire/ /core/derby.log /distribution/conf/apis.yaml /distribution/conf/membrane.log +/.codex diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java index 9bca3270a0..876dc833d9 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java @@ -98,7 +98,7 @@ private static Response createResponse(@Nullable JSONRPCResponse rpcResponse) th return ok().contentType(APPLICATION_JSON).body(rpcResponse.toJson()).build(); } - private @Nullable JSONRPCResponse processMCPRequest(JSONRPCRequest request) throws IOException { + private @Nullable JSONRPCResponse processMCPRequest(JSONRPCRequest request) { switch (request.getMethod()) { case "initialize" -> { return initialize(request).toRpcResponse(); @@ -236,7 +236,7 @@ private MCPToolsListResponse toolsList(JSONRPCRequest request) { } - private MCPInitializeResponse initialize(JSONRPCRequest request) throws IOException { + private MCPInitializeResponse initialize(JSONRPCRequest request) { return new MCPInitializeResponse(new MCPInitialize(request)) .withCapabilities(getCapabilities()) .withServerInfo("Membrane", VERSION); From ce5099cd56a5d0fb0ec4f3536d4605a1541cc039 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Tue, 28 Apr 2026 07:55:14 +0200 Subject: [PATCH 15/29] Add response details (status, body, headers) to MCP server exchange serialization --- .../membrane/core/interceptor/mcp/MembraneMCPServer.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java index 876dc833d9..29e53b6ecb 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java @@ -174,6 +174,10 @@ private MCPToolsCallResponse getExchanges(MCPToolsCall req) { request.put("body", e.getRequest().getBodyAsStringDecoded()); request.put("headers", e.getRequest().getHeader()); + response.put("status", e.getResponse().getStatusCode()); + response.put("body", e.getResponse().getBodyAsStringDecoded()); + response.put("headers", e.getResponse().getHeader()); + exc.put("request", request); exc.put("response", response); return exc; From 9a0440f2d708cd39e2ae11a0855a9aeb18348daf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Tue, 28 Apr 2026 07:56:51 +0200 Subject: [PATCH 16/29] Reorder case blocks in `MembraneMCPServer` for consistency --- .../membrane/core/interceptor/mcp/MembraneMCPServer.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java index 29e53b6ecb..e7b6a71c28 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java @@ -198,14 +198,14 @@ private MCPToolsCallResponse listProxies(MCPToolsCall req) { type = "API"; // proxy.put("openapi", ap.getOpenapi()); } - case ServiceProxy s -> { - type = "serviceProxy"; - } case SOAPProxy sp -> { type = "soapProxy"; proxy.put("wsdl", sp.getWsdl()); proxy.put("serviceName", sp.getServiceName()); } + case ServiceProxy s -> { + type = "serviceProxy"; + } default -> { type = "unknown"; } From 819ed81f70df0d5f79ae2c7302865e1768a87c7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Wed, 29 Apr 2026 07:54:47 +0200 Subject: [PATCH 17/29] Refactor error response handling in `MembraneMCPServer` to improve clarity and logging for notifications --- .../interceptor/mcp/MembraneMCPServer.java | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java index e7b6a71c28..b2d77804fb 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java @@ -61,10 +61,10 @@ public Outcome handleRequest(Exchange exc) { try { rpcResponse = processMCPRequest(request); } catch (IllegalArgumentException e) { - exc.setResponse(createResponse(createErrorResponse(exc, request, JSONRPCResponse.ERR_INVALID_PARAMS, "Invalid params", e))); + exc.setResponse(createResponse(createProcessingErrorResponse(exc, request, JSONRPCResponse.ERR_INVALID_PARAMS, "Invalid params", e))); return RETURN; } catch (Exception e) { - exc.setResponse(createResponse(createErrorResponse(exc, request, ERR_INTERNAL_ERROR, "Internal error", e))); + exc.setResponse(createResponse(createProcessingErrorResponse(exc, request, ERR_INTERNAL_ERROR, "Internal error", e))); return RETURN; } exc.setResponse(createResponse(rpcResponse)); @@ -84,11 +84,16 @@ private JSONRPCResponse createErrorResponse(Exchange exc, @Nullable JSONRPCReque return error(request == null ? null : request.getId(), code, message, e.getMessage()); } - private static Response createResponse(MCPResponse mcpResponse) throws IOException { - if (mcpResponse == null) { - return Response.noContent().build(); + private @Nullable JSONRPCResponse createProcessingErrorResponse(Exchange exc, JSONRPCRequest request, int code, String message, Exception e) { + if (request.isNotification()) { + if (code == ERR_INTERNAL_ERROR) { + log.warn("Failed to handle MCP notification {} {}.", exc.getRequest().getMethod(), exc.getRequest().getUri(), e); + } else { + log.info("Rejected MCP notification {} {}: {}", exc.getRequest().getMethod(), exc.getRequest().getUri(), e.getMessage()); + } + return null; } - return createResponse(mcpResponse.toRpcResponse()); + return createErrorResponse(exc, request, code, message, e); } private static Response createResponse(@Nullable JSONRPCResponse rpcResponse) throws IOException { @@ -104,6 +109,7 @@ private static Response createResponse(@Nullable JSONRPCResponse rpcResponse) th return initialize(request).toRpcResponse(); } case "notifications/initialized" -> { + MCPInitialized.from(request); log.debug("MCP Client is ready"); return null; } From 7fa6d37acc704fc170f20ab9071606746040118c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Wed, 29 Apr 2026 08:52:21 +0200 Subject: [PATCH 18/29] Refactor JSON-RPC utilities to improve validation, normalization, and serialization handling --- .../membrane/core/jsonrpc/JSONRPCRequest.java | 123 +++++---- .../core/jsonrpc/JSONRPCResponse.java | 245 +++++++----------- .../membrane/core/jsonrpc/JSONRPCUtil.java | 72 +++++ 3 files changed, 233 insertions(+), 207 deletions(-) create mode 100644 core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCUtil.java diff --git a/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCRequest.java b/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCRequest.java index 2128a5fb64..65f3b3b86b 100644 --- a/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCRequest.java +++ b/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCRequest.java @@ -4,10 +4,14 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonSetter; +import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.SerializerProvider; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; import com.fasterxml.jackson.databind.node.ObjectNode; +import com.fasterxml.jackson.databind.ser.std.StdSerializer; import java.io.IOException; import java.io.InputStream; @@ -16,16 +20,21 @@ import java.util.Map; import java.util.Objects; +import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_NULL; +import static com.predic8.membrane.core.jsonrpc.JSONRPCUtil.*; +import static java.util.Objects.requireNonNull; + /** * Represents a JSON-RPC 2.0 request as defined by https://www.jsonrpc.org/specification. * - *

The {@code id} member may be a {@link String}, a {@link Number}, or {@code null}. + *

The {@code id} member may be a {@link String}, an Integer {@link Integer}, or {@code null}. * A request without an {@code id} is a notification and the server MUST NOT reply.

* *

The {@code params} member is structured: either a JSON Array (positional, see * {@link #getParamsList()}) or a JSON Object (named, see {@link #getParamsMap()}).

*/ -@JsonInclude(JsonInclude.Include.NON_NULL) +@JsonInclude(NON_NULL) +@JsonSerialize(using = JSONRPCRequest.Serializer.class) public class JSONRPCRequest { public static final String JSONRPC_VERSION = "2.0"; @@ -37,7 +46,7 @@ public class JSONRPCRequest { @JsonProperty("jsonrpc") private String jsonrpc = JSONRPC_VERSION; - /** May be String, Number, or null. */ + /** May be String, integral Number, or null. */ private Object id; /** Distinguishes a missing {@code id} from an explicit {@code "id": null}. */ @@ -67,68 +76,52 @@ public JSONRPCRequest(Object id, String method, List paramsList) { public JSONRPCRequest(Object id, boolean idPresent, String method, Map paramsMap) { setId(id, idPresent); - this.method = Objects.requireNonNull(method, "method must not be null"); - this.paramsMap = paramsMap; + setMethod(method); + setParamsMap(paramsMap); } public JSONRPCRequest(Object id, boolean idPresent, String method, List paramsList) { setId(id, idPresent); - this.method = Objects.requireNonNull(method, "method must not be null"); - this.paramsList = paramsList; + setMethod(method); + setParamsList(paramsList); } - // ---------- Parsing ---------- - public static JSONRPCRequest parse(InputStream is) throws IOException { - Objects.requireNonNull(is, "input stream must not be null"); + requireNonNull(is, "input stream must not be null"); return fromNode(OM.readTree(is)); } public static JSONRPCRequest parse(String json) throws IOException { - Objects.requireNonNull(json, "json must not be null"); + requireNonNull(json, "json must not be null"); return fromNode(OM.readTree(json)); } private static JSONRPCRequest fromNode(JsonNode root) throws IOException { - if (root == null || !root.isObject()) { - throw new IOException("Invalid JSON-RPC request: expected JSON object"); - } + if (root == null || !root.isObject()) throw new IOException("Invalid JSON-RPC request: expected JSON object"); JSONRPCRequest req = new JSONRPCRequest(); - JsonNode versionNode = root.get("jsonrpc"); - req.jsonrpc = versionNode != null && !versionNode.isNull() ? versionNode.asText() : null; - if (!JSONRPC_VERSION.equals(req.jsonrpc)) { - throw new IOException("Unsupported or missing jsonrpc version: " + req.jsonrpc); - } + req.jsonrpc = parseVersion(root, "request"); JsonNode methodNode = root.get("method"); - if (methodNode == null || !methodNode.isTextual()) { - throw new IOException("Invalid JSON-RPC request: 'method' must be a string"); + if (methodNode == null || !methodNode.isTextual()) throw new IOException("Invalid JSON-RPC request: 'method' must be a string"); + + try { + req.setMethod(methodNode.asText()); + } catch (IllegalArgumentException e) { + throw new IOException("Invalid JSON-RPC request: " + e.getMessage(), e); } - req.method = methodNode.asText(); if (root.has("id")) { - JsonNode idNode = root.get("id"); - if (idNode == null || idNode.isNull()) { - req.setId(null, true); - } else if (idNode.isTextual()) { - req.setId(idNode.asText(), true); - } else if (idNode.isIntegralNumber()) { - req.setId(idNode.longValue(), true); - } else if (idNode.isNumber()) { - req.setId(idNode.numberValue(), true); - } else { - throw new IOException("Invalid JSON-RPC request: 'id' must be string, number, or null"); - } + req.setId(JSONRPCUtil.parseId(root.get("id"), "request"), true); } JsonNode paramsNode = root.get("params"); if (paramsNode != null && !paramsNode.isNull()) { if (paramsNode.isArray()) { - req.paramsList = OM.convertValue(paramsNode, LIST_TYPE); + req.setParamsList(OM.convertValue(paramsNode, LIST_TYPE)); } else if (paramsNode.isObject()) { - req.paramsMap = OM.convertValue(paramsNode, MAP_TYPE); + req.setParamsMap(OM.convertValue(paramsNode, MAP_TYPE)); } else { throw new IOException("Invalid JSON-RPC request: 'params' must be array or object"); } @@ -137,25 +130,21 @@ private static JSONRPCRequest fromNode(JsonNode root) throws IOException { return req; } - // ---------- Serialization ---------- - /** Returns the params field for serialization (object, array, or null). */ @JsonProperty("params") - @JsonInclude(JsonInclude.Include.NON_NULL) + @JsonInclude(NON_NULL) public Object getParams() { if (paramsMap != null) return paramsMap; return paramsList; } public String toJson() throws IOException { - return OM.writeValueAsString(toNode()); - } - - public void writeTo(OutputStream os) throws IOException { - OM.writeValue(os, toNode()); + return OM.writeValueAsString(this); } private ObjectNode toNode() { + validate(); + ObjectNode root = OM.createObjectNode(); if (jsonrpc != null) { @@ -180,30 +169,23 @@ private ObjectNode toNode() { return root; } - // ---------- Convenience ---------- - @JsonIgnore public boolean isNotification() { return !idPresent; } + @SuppressWarnings("BooleanMethodIsAlwaysInverted") @JsonIgnore public boolean hasNamedParams() { return paramsMap != null; } - @JsonIgnore - public boolean hasPositionalParams() { - return paramsList != null; - } - - // ---------- Getters / setters ---------- - public String getJsonrpc() { return jsonrpc; } public void setJsonrpc(String jsonrpc) { + validateVersion(jsonrpc); this.jsonrpc = jsonrpc; } @@ -216,20 +198,14 @@ public boolean isIdPresent() { return idPresent; } - /** Convenience accessor preserving backwards compatibility — returns the id as String, or null. */ - @JsonIgnore - public String getIdAsString() { - return id == null ? null : id.toString(); - } - @JsonSetter("id") public void setId(Object id) { setId(id, true); } public void setId(Object id, boolean present) { - this.id = present ? id : null; this.idPresent = present; + this.id = present ? normalizeId(id) : null; } public String getMethod() { @@ -237,7 +213,7 @@ public String getMethod() { } public void setMethod(String method) { - this.method = method; + this.method = normalizeMethod(method); } public Map getParamsMap() { @@ -258,7 +234,18 @@ public void setParamsList(List paramsList) { if (paramsList != null) this.paramsMap = null; } - // ---------- Object overrides ---------- + private void validate() { + validateVersion(jsonrpc); + normalizeMethod(method); + if (paramsMap != null && paramsList != null) { + throw new IllegalStateException("paramsMap and paramsList are mutually exclusive"); + } + if (idPresent) { + id = normalizeId(id); + } else if (id != null) { + throw new IllegalStateException("id must be null when id is absent"); + } + } @Override public boolean equals(Object o) { @@ -286,4 +273,16 @@ public String toString() { ", params=" + (paramsMap != null ? paramsMap : paramsList) + '}'; } + + static final class Serializer extends StdSerializer { + + Serializer() { + super(JSONRPCRequest.class); + } + + @Override + public void serialize(JSONRPCRequest value, JsonGenerator gen, SerializerProvider provider) throws IOException { + provider.defaultSerializeValue(value.toNode(), gen); + } + } } diff --git a/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCResponse.java b/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCResponse.java index cc5eaca1ca..ebe1a2513e 100644 --- a/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCResponse.java +++ b/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCResponse.java @@ -5,10 +5,11 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonPropertyOrder; -import com.fasterxml.jackson.databind.SerializerProvider; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.SerializerProvider; import com.fasterxml.jackson.databind.annotation.JsonSerialize; +import com.fasterxml.jackson.databind.node.ObjectNode; import com.fasterxml.jackson.databind.ser.std.StdSerializer; import java.io.IOException; @@ -16,38 +17,18 @@ import java.io.OutputStream; import java.util.Objects; +import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_NULL; import static com.predic8.membrane.core.jsonrpc.JSONRPCResponse.ResponseKind.ERROR; import static com.predic8.membrane.core.jsonrpc.JSONRPCResponse.ResponseKind.SUCCESS; +import static com.predic8.membrane.core.jsonrpc.JSONRPCUtil.*; +import static java.util.Objects.requireNonNull; /** * Represents a JSON-RPC 2.0 response as defined by https://www.jsonrpc.org/specification. * - *

A response is either a success or an error — the two are - * mutually exclusive:

- *
    - *
  • Success: {@code result} is present, {@code error} is absent.
    • - *
  • Error: {@code error} is present, {@code result} is absent.
    • - *
- * - *

Wire format (success):

- * 
{@code
- * { "jsonrpc": "2.0", "id": 1, "result": { ... } }
- * }
- * - *

Wire format (error):

- * 
{@code
- * { "jsonrpc": "2.0", "id": 1, "error": { "code": -32600, "message": "...", "data": ... } }
- * }
- * - *

The {@code id} member mirrors the {@code id} of the originating request. - * It MUST be {@code null} if the id could not be determined (e.g. parse error).

- * - *

Use the static factories {@link #success(Object, Object)} and - * {@link #error(Object, int, String)} (or {@link #error(Object, int, String, Object)}) - * to construct instances. Use {@link #from(JSONRPCRequest, Object)} to build a - * success response that echoes the id of an existing {@link JSONRPCRequest}.

- * - *

Standard error codes are provided as constants, e.g. {@link #ERR_PARSE_ERROR}.

+ *

A response is either a success carrying {@code result} or an error carrying + * {@code error}. The {@code id} mirrors the originating request and is + * {@code null} when the request id could not be determined.

*/ @JsonPropertyOrder({"jsonrpc", "id", "result", "error"}) @JsonSerialize(using = JSONRPCResponse.Serializer.class) @@ -86,26 +67,22 @@ public class JSONRPCResponse { /** Present on error responses; absent (null) on success responses. */ @JsonProperty("error") - private Error error; + private JSONRPCError error; - /** Distinguishes between a success response carrying {@code result} and an error response carrying {@code error}. */ @JsonIgnore private ResponseKind responseKind; - // ---------- Constructors ---------- - - /** No-arg constructor for Jackson deserialization. */ public JSONRPCResponse() {} - private JSONRPCResponse(Object id, Object result, Error error, ResponseKind responseKind) { - this.id = id; - this.result = result; - this.error = error; - this.responseKind = responseKind; + private JSONRPCResponse(Object id, Object result, JSONRPCError error, ResponseKind responseKind) { + setId(id); + if (responseKind == SUCCESS) { + setResult(result); + return; + } + setError(error); } - // ---------- Static factories ---------- - /** * Creates a success response with the given {@code id} and {@code result}. * @@ -123,8 +100,7 @@ public static JSONRPCResponse success(Object id, Object result) { * @param result the result value */ public static JSONRPCResponse from(JSONRPCRequest request, Object result) { - Objects.requireNonNull(request, "request must not be null"); - return new JSONRPCResponse(request.getId(), result, null, SUCCESS); + return new JSONRPCResponse(requireResponseId(request), result, null, SUCCESS); } /** @@ -135,7 +111,7 @@ public static JSONRPCResponse from(JSONRPCRequest request, Object result) { * @param message a human-readable error description */ public static JSONRPCResponse error(Object id, int code, String message) { - return new JSONRPCResponse(id, null, new Error(code, message, null), ERROR); + return new JSONRPCResponse(id, null, new JSONRPCError(code, message, null), ERROR); } /** @@ -147,22 +123,9 @@ public static JSONRPCResponse error(Object id, int code, String message) { * @param data optional additional error information (any JSON-serialisable object) */ public static JSONRPCResponse error(Object id, int code, String message, Object data) { - return new JSONRPCResponse(id, null, new Error(code, message, data), ERROR); + return new JSONRPCResponse(id, null, new JSONRPCError(code, message, data), ERROR); } - /** - * Creates an error response that echoes the id of the originating {@link JSONRPCRequest}. - * - * @param request the request to reply to - * @param code the JSON-RPC error code (see {@code ERR_*} constants) - * @param message a human-readable error description - */ - public static JSONRPCResponse errorFor(JSONRPCRequest request, int code, String message) { - Objects.requireNonNull(request, "request must not be null"); - return new JSONRPCResponse(request.getId(), null, new Error(code, message, null), ERROR); - } - - // ---------- Parsing ---------- /** * Parses a JSON-RPC 2.0 response from the given {@link InputStream}. @@ -170,7 +133,7 @@ public static JSONRPCResponse errorFor(JSONRPCRequest request, int code, String * @throws IOException if the JSON is malformed or violates the JSON-RPC 2.0 structure */ public static JSONRPCResponse parse(InputStream is) throws IOException { - Objects.requireNonNull(is, "input stream must not be null"); + requireNonNull(is, "input stream must not be null"); return fromNode(OM.readTree(is)); } @@ -180,7 +143,7 @@ public static JSONRPCResponse parse(InputStream is) throws IOException { * @throws IOException if the JSON is malformed or violates the JSON-RPC 2.0 structure */ public static JSONRPCResponse parse(String json) throws IOException { - Objects.requireNonNull(json, "json must not be null"); + requireNonNull(json, "json must not be null"); return fromNode(OM.readTree(json)); } @@ -191,24 +154,14 @@ private static JSONRPCResponse fromNode(JsonNode root) throws IOException { JSONRPCResponse resp = new JSONRPCResponse(); - JsonNode versionNode = root.get("jsonrpc"); - resp.jsonrpc = versionNode != null && !versionNode.isNull() ? versionNode.asText() : null; - if (!JSONRPC_VERSION.equals(resp.jsonrpc)) { - throw new IOException("Unsupported or missing jsonrpc version: " + resp.jsonrpc); + resp.jsonrpc = parseVersion(root, "response"); + + if (!root.has("id")) { + throw new IOException("Invalid JSON-RPC response: 'id' is required"); } JsonNode idNode = root.get("id"); - if (idNode != null && !idNode.isNull()) { - if (idNode.isTextual()) { - resp.id = idNode.asText(); - } else if (idNode.isIntegralNumber()) { - resp.id = idNode.longValue(); - } else if (idNode.isNumber()) { - resp.id = idNode.numberValue(); - } else { - throw new IOException("Invalid JSON-RPC response: 'id' must be string, number, or null"); - } - } + resp.setId(parseId(idNode, "response")); boolean hasResult = root.has("result"); boolean hasError = root.has("error"); @@ -228,31 +181,25 @@ private static JSONRPCResponse fromNode(JsonNode root) throws IOException { if (!errorNode.isObject()) { throw new IOException("Invalid JSON-RPC response: 'error' must be a JSON object"); } - resp.setError(Error.fromNode(errorNode)); + resp.setError(JSONRPCError.fromNode(errorNode)); } return resp; } - // ---------- Serialization ---------- - public String toJson() throws IOException { - return OM.writeValueAsString(this); + return OM.writeValueAsString(toNode()); } public void writeTo(OutputStream os) throws IOException { - OM.writeValue(os, this); + OM.writeValue(os, toNode()); } - // ---------- Convenience ---------- - - /** Returns {@code true} if this is a success response (no {@code error} member). */ @JsonIgnore public boolean isSuccess() { return getResponseKind() == SUCCESS; } - /** Returns {@code true} if this is an error response. */ @JsonIgnore public boolean isError() { return getResponseKind() == ERROR; @@ -263,22 +210,18 @@ public ResponseKind getResponseKind() { if (responseKind != null) { return responseKind; } - return error != null ? ERROR : SUCCESS; - } - - /** Convenience accessor — returns the id as String, or null. */ - @JsonIgnore - public String getIdAsString() { - return id == null ? null : id.toString(); + if (error != null) { + return ERROR; + } + throw new IllegalStateException("response kind is undefined until either result or error is set"); } - // ---------- Getters / setters ---------- - public String getJsonrpc() { return jsonrpc; } public void setJsonrpc(String jsonrpc) { + validateVersion(jsonrpc); this.jsonrpc = jsonrpc; } @@ -287,7 +230,7 @@ public Object getId() { } public void setId(Object id) { - this.id = id; + this.id = normalizeId(id); } public Object getResult() { @@ -300,16 +243,63 @@ public void setResult(Object result) { this.responseKind = SUCCESS; } - public Error getError() { - return error; - } - - public void setError(Error error) { - this.error = error; + public void setError(JSONRPCError error) { + this.error = requireNonNull(error, "error must not be null"); this.result = null; this.responseKind = ERROR; } + private ObjectNode toNode() { + validate(); + + ObjectNode root = OM.createObjectNode(); + root.put("jsonrpc", jsonrpc); + + if (id == null) { + root.putNull("id"); + } else { + root.set("id", OM.valueToTree(id)); + } + + if (responseKind == SUCCESS) { + root.set("result", OM.valueToTree(result)); + } else { + root.set("error", OM.valueToTree(error)); + } + + return root; + } + + private void validate() { + validateVersion(jsonrpc); + id = normalizeId(id); + + ResponseKind kind = responseKind != null ? responseKind : (error != null ? ERROR : null); + if (kind == null) { + throw new IllegalStateException("response kind is undefined until either result or error is set"); + } + if (kind == SUCCESS) { + if (error != null) { + throw new IllegalStateException("success response must not contain error"); + } + } else { + if (error == null) { + throw new IllegalStateException("error response must contain error"); + } + if (result != null) { + throw new IllegalStateException("error response must not contain result"); + } + } + } + + private static Object requireResponseId(JSONRPCRequest request) { + requireNonNull(request, "request must not be null"); + if (request.isNotification()) { + throw new IllegalArgumentException("cannot create a JSON-RPC response for a notification"); + } + return request.getId(); + } + // ---------- Object overrides ---------- @Override @@ -350,41 +340,13 @@ static final class Serializer extends StdSerializer { @Override public void serialize(JSONRPCResponse value, JsonGenerator gen, SerializerProvider provider) throws IOException { - gen.writeStartObject(); - gen.writeStringField("jsonrpc", value.jsonrpc); - - if (value.id == null) { - gen.writeNullField("id"); - } else { - gen.writeFieldName("id"); - provider.defaultSerializeValue(value.id, gen); - } - - if (value.getResponseKind() == SUCCESS) { - gen.writeFieldName("result"); - provider.defaultSerializeValue(value.result, gen); - } else { - gen.writeFieldName("error"); - provider.defaultSerializeValue(value.error, gen); - } - - gen.writeEndObject(); + provider.defaultSerializeValue(value.toNode(), gen); } } - // ---------- Nested type: Error ---------- - - /** - * Represents the JSON-RPC 2.0 {@code error} member. - * - *

Wire format:

- * 
{@code
-     * { "code": -32600, "message": "Invalid Request", "data": { ... } }
-     * }
- */ - @JsonInclude(JsonInclude.Include.NON_NULL) + @JsonInclude(NON_NULL) @JsonPropertyOrder({"code", "message", "data"}) - public static final class Error { + public static class JSONRPCError { @JsonProperty("code") private int code; @@ -396,21 +358,13 @@ public static final class Error { @JsonProperty("data") private Object data; - /** No-arg constructor for Jackson deserialization. */ - public Error() {} - - public Error(int code, String message) { + public JSONRPCError(int code, String message, Object data) { this.code = code; - this.message = Objects.requireNonNull(message, "message must not be null"); - } - - public Error(int code, String message, Object data) { - this.code = code; - this.message = Objects.requireNonNull(message, "message must not be null"); + setMessage(message); this.data = data; } - private static Error fromNode(JsonNode node) throws IOException { + private static JSONRPCError fromNode(JsonNode node) throws IOException { JsonNode codeNode = node.get("code"); if (codeNode == null || !codeNode.isIntegralNumber()) { throw new IOException("Invalid JSON-RPC error: 'code' must be an integer"); @@ -422,9 +376,9 @@ private static Error fromNode(JsonNode node) throws IOException { Object data = null; JsonNode dataNode = node.get("data"); if (dataNode != null && !dataNode.isNull()) { - data = new ObjectMapper().convertValue(dataNode, Object.class); + data = OM.convertValue(dataNode, Object.class); } - return new Error(codeNode.intValue(), messageNode.asText(), data); + return new JSONRPCError(codeNode.intValue(), messageNode.asText(), data); } public int getCode() { @@ -440,7 +394,7 @@ public String getMessage() { } public void setMessage(String message) { - this.message = message; + this.message = requireNonNull(message, "message must not be null"); } public Object getData() { @@ -454,7 +408,7 @@ public void setData(Object data) { @Override public boolean equals(Object o) { if (this == o) return true; - if (!(o instanceof Error that)) return false; + if (!(o instanceof JSONRPCError that)) return false; return code == that.code && Objects.equals(message, that.message) && Objects.equals(data, that.data); @@ -467,8 +421,9 @@ public int hashCode() { @Override public String toString() { - return "Error{code=" + code + ", message='" + message + "'" + + return "JSONRPCError{code=" + code + ", message='" + message + "'" + (data != null ? ", data=" + data : "") + "}"; } } + } diff --git a/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCUtil.java b/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCUtil.java new file mode 100644 index 0000000000..158fa9e087 --- /dev/null +++ b/core/src/main/java/com/predic8/membrane/core/jsonrpc/JSONRPCUtil.java @@ -0,0 +1,72 @@ +package com.predic8.membrane.core.jsonrpc; + +import com.fasterxml.jackson.databind.JsonNode; + +import java.io.IOException; +import java.math.BigInteger; + +import static com.predic8.membrane.core.jsonrpc.JSONRPCRequest.JSONRPC_VERSION; + +final class JSONRPCUtil { + + private JSONRPCUtil() { + } + + static String parseVersion(JsonNode root, String messageType) throws IOException { + JsonNode versionNode = root.get("jsonrpc"); + String version = versionNode != null && !versionNode.isNull() ? versionNode.asText() : null; + if (!JSONRPC_VERSION.equals(version)) { + throw new IOException("Unsupported or missing jsonrpc version in " + messageType + ": " + version); + } + return version; + } + + static void validateVersion(String jsonrpc) { + if (!JSONRPC_VERSION.equals(jsonrpc)) { + throw new IllegalArgumentException("jsonrpc must be '" + JSONRPC_VERSION + "'"); + } + } + + static String normalizeMethod(String method) { + if (method == null) { + throw new IllegalArgumentException("method must not be null"); + } + if (method.isBlank()) { + throw new IllegalArgumentException("method must not be blank"); + } + return method; + } + + static Object parseId(JsonNode idNode, String messageType) throws IOException { + if (idNode == null || idNode.isNull()) { + return null; + } + if (idNode.isTextual()) { + return idNode.asText(); + } + if (idNode.isIntegralNumber() && idNode.canConvertToLong()) { + return idNode.longValue(); + } + throw new IOException("Invalid JSON-RPC " + messageType + ": 'id' must be string, integer, or null"); + } + + static Object normalizeId(Object id) { + if (id == null) { + return null; + } + if (id instanceof String) { + return id; + } + if (id instanceof Byte || id instanceof Short || id instanceof Integer || id instanceof Long) { + return ((Number) id).longValue(); + } + if (id instanceof BigInteger bigInteger) { + try { + return bigInteger.longValueExact(); + } catch (ArithmeticException e) { + throw new IllegalArgumentException("id is out of long range", e); + } + } + throw new IllegalArgumentException("id must be String, Integer, or null"); + } +} From 22bb7924431d43d48330ad486713620a8916aa2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Wed, 29 Apr 2026 10:12:07 +0200 Subject: [PATCH 19/29] Add unit tests for `JSONRPCRequest` and `JSONRPCResponse` to validate parsing, normalization, and error handling --- .../core/jsonrpc/JSONRPCRequestTest.java | 180 +++++++++++++++++ .../core/jsonrpc/JSONRPCResponseTest.java | 181 ++++++++++++++++++ 2 files changed, 361 insertions(+) create mode 100644 core/src/test/java/com/predic8/membrane/core/jsonrpc/JSONRPCRequestTest.java create mode 100644 core/src/test/java/com/predic8/membrane/core/jsonrpc/JSONRPCResponseTest.java diff --git a/core/src/test/java/com/predic8/membrane/core/jsonrpc/JSONRPCRequestTest.java b/core/src/test/java/com/predic8/membrane/core/jsonrpc/JSONRPCRequestTest.java new file mode 100644 index 0000000000..935ae66c12 --- /dev/null +++ b/core/src/test/java/com/predic8/membrane/core/jsonrpc/JSONRPCRequestTest.java @@ -0,0 +1,180 @@ +package com.predic8.membrane.core.jsonrpc; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.math.BigInteger; +import java.nio.charset.StandardCharsets; +import java.util.List; +import java.util.Map; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.params.provider.Arguments.of; + +class JSONRPCRequestTest { + + @ParameterizedTest(name = "{0}") + @MethodSource("validRequests") + void parsesValidRequests(String name, String json, JSONRPCRequest expected) throws IOException { + JSONRPCRequest parsed = JSONRPCRequest.parse(json); + + assertEquals(expected, parsed); + assertEquals(expected.getParams(), parsed.getParams()); + assertEquals(expected.isNotification(), parsed.isNotification()); + assertEquals(expected.hasNamedParams(), parsed.hasNamedParams()); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("requestsForRoundTrip") + void roundTripsViaStringAndInputStream(String name, JSONRPCRequest request) throws IOException { + String json = request.toJson(); + + assertEquals(request, JSONRPCRequest.parse(json)); + assertEquals(request, JSONRPCRequest.parse(input(json))); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("normalizedIds") + void normalizesSupportedIds(String name, Object rawId, Object expectedId) { + JSONRPCRequest request = new JSONRPCRequest(rawId, "sum", List.of()); + + assertEquals(expectedId, request.getId()); + assertTrue(request.isIdPresent()); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("invalidIds") + void rejectsUnsupportedIds(String name, Object rawId, String messageFragment) { + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> new JSONRPCRequest(rawId, "sum", List.of()) + ); + + assertMessageContains(exception, messageFragment); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("invalidRequests") + void rejectsInvalidRequests(String name, String json, String messageFragment) { + IOException exception = assertThrows(IOException.class, () -> JSONRPCRequest.parse(json)); + + assertMessageContains(exception, messageFragment); + } + + @Test + void switchingParamsRepresentationClearsTheOtherOne() { + JSONRPCRequest request = new JSONRPCRequest("req-1", "tools/call", Map.of("name", "echo")); + + request.setParamsList(List.of("echo", Map.of("value", 1))); + + assertNull(request.getParamsMap()); + assertEquals(List.of("echo", Map.of("value", 1)), request.getParamsList()); + + request.setParamsMap(Map.of("name", "echo")); + + assertEquals(Map.of("name", "echo"), request.getParamsMap()); + assertNull(request.getParamsList()); + } + + private static Stream validRequests() { + return Stream.of( + of( + "named params request", + """ + {"jsonrpc":"2.0","id":"req-1","method":"tools/list","params":{"cursor":"abc","limit":10}} + """, + new JSONRPCRequest("req-1", "tools/list", Map.of("cursor", "abc", "limit", 10)) + ), + of( + "positional params request", + """ + {"jsonrpc":"2.0","id":7,"method":"sum","params":[1,2,3]} + """, + new JSONRPCRequest(7, "sum", List.of(1, 2, 3)) + ), + of( + "notification without id", + """ + {"jsonrpc":"2.0","method":"notifications/initialized"} + """, + notification("notifications/initialized") + ), + of( + "request with explicit null id", + """ + {"jsonrpc":"2.0","id":null,"method":"shutdown"} + """, + requestWithNullId("shutdown") + ) + ); + } + + private static Stream requestsForRoundTrip() { + return Stream.of( + of("named params round trip", new JSONRPCRequest("req-1", "tools/list", Map.of("cursor", "abc", "limit", 10))), + of("positional params round trip", new JSONRPCRequest(7, "sum", List.of(1, 2, 3))), + of("notification round trip", notification("notifications/initialized")), + of("explicit null id round trip", new JSONRPCRequest(null, true, "shutdown", (List) null)) + ); + } + + private static Stream normalizedIds() { + return Stream.of( + of("string id stays string", "req-1", "req-1"), + of("integer id becomes long", 7, 7L), + of("short id becomes long", (short) 3, 3L), + of("big integer in range becomes long", BigInteger.valueOf(11), 11L), + of("null id stays null", null, null) + ); + } + + private static Stream invalidIds() { + return Stream.of( + of("decimal id", 1.5d, "id must be String, Integer, or null"), + of("out of range big integer id", BigInteger.valueOf(Long.MAX_VALUE).add(BigInteger.ONE), "id is out of long range"), + of("arbitrary object id", new Object(), "id must be String, Integer, or null") + ); + } + + private static Stream invalidRequests() { + return Stream.of( + of("root must be object", "[]", "expected JSON object"), + of("jsonrpc version is required", """ + {"method":"sum"} + """, "Unsupported or missing jsonrpc version in request"), + of("method must be textual", """ + {"jsonrpc":"2.0","method":1} + """, "'method' must be a string"), + of("method must not be blank", """ + {"jsonrpc":"2.0","method":" "} + """, "method must not be blank"), + of("id must be integral when numeric", """ + {"jsonrpc":"2.0","id":1.5,"method":"sum"} + """, "'id' must be string, integer, or null"), + of("params must be object or array", """ + {"jsonrpc":"2.0","id":"req-1","method":"sum","params":true} + """, "'params' must be array or object") + ); + } + + private static JSONRPCRequest notification(String method) { + return new JSONRPCRequest(null, false, method, (Map) null); + } + + private static JSONRPCRequest requestWithNullId(String method) { + return new JSONRPCRequest(null, true, method, (Map) null); + } + + private static ByteArrayInputStream input(String json) { + return new ByteArrayInputStream(json.getBytes(StandardCharsets.UTF_8)); + } + + private static void assertMessageContains(Throwable exception, String messageFragment) { + assertTrue(exception.getMessage().contains(messageFragment)); + } +} diff --git a/core/src/test/java/com/predic8/membrane/core/jsonrpc/JSONRPCResponseTest.java b/core/src/test/java/com/predic8/membrane/core/jsonrpc/JSONRPCResponseTest.java new file mode 100644 index 0000000000..97eed97b58 --- /dev/null +++ b/core/src/test/java/com/predic8/membrane/core/jsonrpc/JSONRPCResponseTest.java @@ -0,0 +1,181 @@ +package com.predic8.membrane.core.jsonrpc; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.math.BigInteger; +import java.nio.charset.StandardCharsets; +import java.util.List; +import java.util.Map; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.params.provider.Arguments.of; + +class JSONRPCResponseTest { + + @ParameterizedTest(name = "{0}") + @MethodSource("validResponses") + void parsesValidResponses(String name, String json, JSONRPCResponse expected) throws IOException { + JSONRPCResponse parsed = JSONRPCResponse.parse(json); + + assertEquals(expected, parsed); + assertEquals(expected.isSuccess(), parsed.isSuccess()); + assertEquals(expected.isError(), parsed.isError()); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("responsesForRoundTrip") + void roundTripsViaStringAndOutputStream(String name, JSONRPCResponse response) throws IOException { + assertEquals(response, JSONRPCResponse.parse(response.toJson())); + + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + response.writeTo(outputStream); + + assertEquals(response, JSONRPCResponse.parse(new ByteArrayInputStream(outputStream.toByteArray()))); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("requestsWithResponseIds") + void createsSuccessResponseFromRequest(String name, JSONRPCRequest request, Object expectedId) { + JSONRPCResponse response = JSONRPCResponse.from(request, Map.of("ok", true)); + + assertTrue(response.isSuccess()); + assertEquals(expectedId, response.getId()); + assertEquals(Map.of("ok", true), response.getResult()); + } + + @Test + void rejectsCreatingResponseForNotification() { + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> JSONRPCResponse.from(notification("notifications/initialized"), Map.of("ok", true)) + ); + + assertMessageContains(exception, "cannot create a JSON-RPC response for a notification"); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("invalidIds") + void rejectsUnsupportedIds(String name, Object rawId, String messageFragment) { + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> JSONRPCResponse.success(rawId, "ok") + ); + + assertMessageContains(exception, messageFragment); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("invalidResponses") + void rejectsInvalidResponses(String name, String json, String messageFragment) { + IOException exception = assertThrows(IOException.class, () -> JSONRPCResponse.parse(json)); + + assertMessageContains(exception, messageFragment); + } + + @Test + void responseKindIsUndefinedUntilResultOrErrorIsSet() { + JSONRPCResponse response = new JSONRPCResponse(); + + IllegalStateException kindException = assertThrows(IllegalStateException.class, response::getResponseKind); + IllegalStateException serializationException = assertThrows(IllegalStateException.class, response::toJson); + + assertMessageContains(kindException, "response kind is undefined"); + assertMessageContains(serializationException, "response kind is undefined"); + } + + @Test + void parseSupportsInputStream() throws IOException { + JSONRPCResponse parsed = JSONRPCResponse.parse(input(""" + {"jsonrpc":"2.0","id":"req-1","result":{"ok":true}} + """)); + + assertEquals(JSONRPCResponse.success("req-1", Map.of("ok", true)), parsed); + } + + private static Stream validResponses() { + return Stream.of( + of("success response", """ + {"jsonrpc":"2.0","id":"req-1","result":{"tools":["echo"]}} + """, JSONRPCResponse.success("req-1", Map.of("tools", List.of("echo")))), + of("success response with null result", """ + {"jsonrpc":"2.0","id":7,"result":null} + """, JSONRPCResponse.success(7, null)), + of("error response with data", """ + {"jsonrpc":"2.0","id":null,"error":{"code":-32602,"message":"Invalid params","data":{"field":"cursor"}}} + """, JSONRPCResponse.error(null, JSONRPCResponse.ERR_INVALID_PARAMS, "Invalid params", Map.of("field", "cursor"))) + ); + } + + private static Stream responsesForRoundTrip() { + return Stream.of( + of("success round trip", JSONRPCResponse.success("req-1", Map.of("tools", List.of("echo")))), + of("success with null result round trip", JSONRPCResponse.success(7, null)), + of("error round trip", JSONRPCResponse.error(null, JSONRPCResponse.ERR_INVALID_PARAMS, "Invalid params", Map.of("field", "cursor"))) + ); + } + + private static Stream requestsWithResponseIds() { + return Stream.of( + of("string id is echoed", new JSONRPCRequest("req-1", "tools/list", Map.of("cursor", "abc")), "req-1"), + of("numeric id is normalized and echoed", new JSONRPCRequest(7, "sum", List.of(1, 2)), 7L), + of("explicit null id is echoed", new JSONRPCRequest(null, true, "shutdown", (Map) null), null) + ); + } + + private static Stream invalidIds() { + return Stream.of( + of("decimal id", 1.5d, "id must be String, Integer, or null"), + of("out of range big integer id", BigInteger.valueOf(Long.MAX_VALUE).add(BigInteger.ONE), "id is out of long range"), + of("arbitrary object id", new Object(), "id must be String, Integer, or null") + ); + } + + private static Stream invalidResponses() { + return Stream.of( + of("root must be object", "[]", "expected JSON object"), + of("jsonrpc version is required", """ + {"id":"req-1","result":1} + """, "Unsupported or missing jsonrpc version in response"), + of("id is required", """ + {"jsonrpc":"2.0","result":1} + """, "'id' is required"), + of("result and error are mutually exclusive", """ + {"jsonrpc":"2.0","id":"req-1","result":1,"error":{"code":-32603,"message":"boom"}} + """, "'result' and 'error' are mutually exclusive"), + of("either result or error must be present", """ + {"jsonrpc":"2.0","id":"req-1"} + """, "either 'result' or 'error' must be present"), + of("error must be object", """ + {"jsonrpc":"2.0","id":"req-1","error":true} + """, "'error' must be a JSON object"), + of("id must be integral when numeric", """ + {"jsonrpc":"2.0","id":1.5,"result":1} + """, "'id' must be string, integer, or null"), + of("error code must be integer", """ + {"jsonrpc":"2.0","id":"req-1","error":{"code":"x","message":"boom"}} + """, "'code' must be an integer"), + of("error message must be string", """ + {"jsonrpc":"2.0","id":"req-1","error":{"code":-32603,"message":1}} + """, "'message' must be a string") + ); + } + + private static JSONRPCRequest notification(String method) { + return new JSONRPCRequest(null, false, method, (Map) null); + } + + private static ByteArrayInputStream input(String json) { + return new ByteArrayInputStream(json.getBytes(StandardCharsets.UTF_8)); + } + + private static void assertMessageContains(Throwable exception, String messageFragment) { + assertTrue(exception.getMessage().contains(messageFragment)); + } +} From fe04d8fe4dcd692826226f2f76237a55e7eecd6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Thu, 30 Apr 2026 09:38:59 +0200 Subject: [PATCH 20/29] Introduce MCP tool registration and utilities, including `McpToolRegistry`, `McpToolDefinition`, and `McpToolHandler`. Minor cleanup and refactoring in MCP initialization and request handling. --- .../interceptor/mcp/McpToolDefinition.java | 16 +++++++++++ .../core/interceptor/mcp/McpToolHandler.java | 12 +++++++++ .../core/interceptor/mcp/McpToolRegistry.java | 25 +++++++++++++++++ .../membrane/core/mcp/MCPInitialize.java | 27 +++++++------------ .../core/mcp/MCPInitializeResponse.java | 10 ++----- .../predic8/membrane/core/mcp/MCPPing.java | 16 +++++++++++ .../predic8/membrane/core/mcp/MCPRequest.java | 6 +++-- 7 files changed, 84 insertions(+), 28 deletions(-) create mode 100644 core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpToolDefinition.java create mode 100644 core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpToolHandler.java create mode 100644 core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpToolRegistry.java create mode 100644 core/src/main/java/com/predic8/membrane/core/mcp/MCPPing.java diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpToolDefinition.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpToolDefinition.java new file mode 100644 index 0000000000..49d14cbb4f --- /dev/null +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpToolDefinition.java @@ -0,0 +1,16 @@ +package com.predic8.membrane.core.interceptor.mcp; + +import com.predic8.membrane.core.mcp.MCPToolsListResponse; + +import java.util.Map; + +public record McpToolDefinition( + String name, + String description, + Map inputSchema, + McpToolHandler handler +) { + public MCPToolsListResponse.Tool toTool() { + return new MCPToolsListResponse.Tool(name, description, inputSchema); + } +} diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpToolHandler.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpToolHandler.java new file mode 100644 index 0000000000..b8bcf4acf3 --- /dev/null +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpToolHandler.java @@ -0,0 +1,12 @@ +package com.predic8.membrane.core.interceptor.mcp; + +import com.predic8.membrane.core.exchange.Exchange; +import com.predic8.membrane.core.mcp.MCPToolsCall; +import com.predic8.membrane.core.mcp.MCPToolsCallResponse; + +@FunctionalInterface +public interface McpToolHandler { + + MCPToolsCallResponse handle(MCPToolsCall call, Exchange exc) throws Exception; + +} diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpToolRegistry.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpToolRegistry.java new file mode 100644 index 0000000000..1dde3ef18a --- /dev/null +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpToolRegistry.java @@ -0,0 +1,25 @@ +package com.predic8.membrane.core.interceptor.mcp; + +import java.util.Collection; +import java.util.LinkedHashMap; +import java.util.Map; + +public final class McpToolRegistry { + + private final Map tools = new LinkedHashMap<>(); + + public McpToolRegistry register(McpToolDefinition definition) { + if (tools.putIfAbsent(definition.name(), definition) != null) { + throw new IllegalArgumentException("Duplicate MCP tool registration: " + definition.name()); + } + return this; + } + + public McpToolDefinition find(String name) { + return tools.get(name); + } + + public Collection list() { + return tools.values(); + } +} diff --git a/core/src/main/java/com/predic8/membrane/core/mcp/MCPInitialize.java b/core/src/main/java/com/predic8/membrane/core/mcp/MCPInitialize.java index 46e8228275..a8d94ca230 100644 --- a/core/src/main/java/com/predic8/membrane/core/mcp/MCPInitialize.java +++ b/core/src/main/java/com/predic8/membrane/core/mcp/MCPInitialize.java @@ -2,7 +2,6 @@ import com.predic8.membrane.core.jsonrpc.JSONRPCRequest; -import java.util.Collections; import java.util.Map; import java.util.Objects; @@ -40,8 +39,8 @@ public MCPInitialize(JSONRPCRequest request) { Map params = request.getParamsMap(); this.protocolVersion = requireString(params, "protocolVersion"); - this.capabilities = asMap(params.get("capabilities")); - this.clientInfo = ClientInfo.from(asMap(params.get("clientInfo"))); + this.capabilities = asMap(params, "capabilities"); + this.clientInfo = ClientInfo.from(asMap(params, "clientInfo")); } /** Static factory equivalent to {@link #MCPInitialize(JSONRPCRequest)}. */ @@ -49,8 +48,6 @@ public static MCPInitialize from(JSONRPCRequest request) { return new MCPInitialize(request); } - // ---------- Accessors ---------- - public String getProtocolVersion() { return protocolVersion; } @@ -63,7 +60,6 @@ public ClientInfo getClientInfo() { return clientInfo; } - // ---------- Helpers ---------- private static String requireString(Map params, String key) { Object v = params.get(key); @@ -75,10 +71,12 @@ private static String requireString(Map params, String key) { } @SuppressWarnings("unchecked") - private static Map asMap(Object value) { - if (value == null) return Collections.emptyMap(); - if (value instanceof Map m) return (Map) m; - throw new IllegalArgumentException("Expected JSON object, got: " + value.getClass().getSimpleName()); + private static Map asMap(Map params, String key) { + Object value = params.get(key); + if (value instanceof Map m) { + return (Map) m; + } + throw new IllegalArgumentException("MCP 'initialize' params: '" + key + "' must be a JSON object"); } @Override @@ -91,8 +89,6 @@ public String toString() { '}'; } - // ---------- Nested types ---------- - /** Identification of the connecting MCP client. */ public static final class ClientInfo { private final String name; @@ -104,12 +100,7 @@ public ClientInfo(String name, String version) { } static ClientInfo from(Map map) { - if (map == null || map.isEmpty()) return new ClientInfo(null, null); - Object name = map.get("name"); - Object version = map.get("version"); - return new ClientInfo( - name == null ? null : name.toString(), - version == null ? null : version.toString()); + return new ClientInfo(requireString(map, "name"),requireString(map, "version")); } public String getName() { diff --git a/core/src/main/java/com/predic8/membrane/core/mcp/MCPInitializeResponse.java b/core/src/main/java/com/predic8/membrane/core/mcp/MCPInitializeResponse.java index 4f8d9317c4..c3227bae7a 100644 --- a/core/src/main/java/com/predic8/membrane/core/mcp/MCPInitializeResponse.java +++ b/core/src/main/java/com/predic8/membrane/core/mcp/MCPInitializeResponse.java @@ -50,10 +50,10 @@ public MCPInitializeResponse(Object id, Result result) { /** * Creates a response from an already-parsed {@link MCPInitialize} request. - * Echoes the request's {@code id} and pre-populates {@code protocolVersion}. + * Echoes the request's {@code id}. */ public MCPInitializeResponse(MCPInitialize request) { - super(request.getId(), initialResult(request.getProtocolVersion())); + super(request.getId(), new Result()); } /** @@ -76,12 +76,6 @@ public static MCPInitializeResponse from(MCPInitialize request) { return new MCPInitializeResponse(request); } - private static Result initialResult(String protocolVersion) { - Result r = new Result(); - r.setProtocolVersion(protocolVersion); - return r; - } - // ---------- Builder-style helpers ---------- public MCPInitializeResponse withProtocolVersion(String protocolVersion) { diff --git a/core/src/main/java/com/predic8/membrane/core/mcp/MCPPing.java b/core/src/main/java/com/predic8/membrane/core/mcp/MCPPing.java new file mode 100644 index 0000000000..b7aa3c8ce1 --- /dev/null +++ b/core/src/main/java/com/predic8/membrane/core/mcp/MCPPing.java @@ -0,0 +1,16 @@ +package com.predic8.membrane.core.mcp; + +import com.predic8.membrane.core.jsonrpc.JSONRPCRequest; + +public class MCPPing extends MCPRequest { + + public static final String METHOD = "ping"; + + public MCPPing(JSONRPCRequest request) { + super(request, METHOD); + } + + public static MCPPing from(JSONRPCRequest request) { + return new MCPPing(request); + } +} diff --git a/core/src/main/java/com/predic8/membrane/core/mcp/MCPRequest.java b/core/src/main/java/com/predic8/membrane/core/mcp/MCPRequest.java index d3745d49a1..1d47fcf638 100644 --- a/core/src/main/java/com/predic8/membrane/core/mcp/MCPRequest.java +++ b/core/src/main/java/com/predic8/membrane/core/mcp/MCPRequest.java @@ -4,6 +4,8 @@ import java.util.Objects; +import static java.util.Objects.requireNonNull; + /** * Abstract base class for all MCP requests (JSON-RPC calls that carry an {@code id} * and expect a response). @@ -36,8 +38,8 @@ public abstract class MCPRequest { * @throws IllegalArgumentException if the method name does not match */ protected MCPRequest(JSONRPCRequest request, String expectedMethod) { - Objects.requireNonNull(request, "request must not be null"); - Objects.requireNonNull(expectedMethod, "expectedMethod must not be null"); + requireNonNull(request, "request must not be null"); + requireNonNull(expectedMethod, "expectedMethod must not be null"); if (!expectedMethod.equals(request.getMethod())) { throw new IllegalArgumentException( From 8b397b6e3bbcc5f3432518bcbe730e2581fd916f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Thu, 30 Apr 2026 09:53:04 +0200 Subject: [PATCH 21/29] Add `McpPayloadSanitizer` and `McpSessionContext` utilities for MCP payload sanitation and session state management --- .../interceptor/mcp/McpPayloadSanitizer.java | 75 +++++++++++++++++++ .../interceptor/mcp/McpSessionContext.java | 62 +++++++++++++++ 2 files changed, 137 insertions(+) create mode 100644 core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpPayloadSanitizer.java create mode 100644 core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpSessionContext.java diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpPayloadSanitizer.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpPayloadSanitizer.java new file mode 100644 index 0000000000..02591b5443 --- /dev/null +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpPayloadSanitizer.java @@ -0,0 +1,75 @@ +package com.predic8.membrane.core.interceptor.mcp; + +import com.predic8.membrane.core.http.Header; +import com.predic8.membrane.core.http.HeaderField; +import com.predic8.membrane.core.http.Message; +import com.predic8.membrane.core.http.MimeType; + +import java.util.LinkedHashMap; +import java.util.Locale; +import java.util.Map; +import java.util.Set; + +import static com.predic8.membrane.core.http.Header.*; +import static com.predic8.membrane.core.http.Header.PROXY_AUTHORIZATION; +import static java.util.Locale.ROOT; + +public final class McpPayloadSanitizer { + + private static final Set SENSITIVE_HEADERS = Set.of( + AUTHORIZATION.toLowerCase(ROOT), + COOKIE.toLowerCase(ROOT), + SET_COOKIE.toLowerCase(ROOT), + PROXY_AUTHORIZATION.toLowerCase(ROOT) + ); + + private static final String REDACTED = ""; + private static final String BINARY_BODY_OMITTED = ""; + private static final String BODY_UNAVAILABLE = ""; + private static final int MAX_BODY_LENGTH = 8 * 1024; + + public Map sanitizeHeaders(Header header) { + var sanitized = new LinkedHashMap(); + if (header == null) { + return sanitized; + } + + for (HeaderField field : header.getAllHeaderFields()) { + String name = field.getHeaderName().toString(); + + sanitized.merge(name, redactIfSensitive(field, name), (previous, current) -> previous + ", " + current); + } + + return sanitized; + } + + private static String redactIfSensitive(HeaderField field, String name) { + return SENSITIVE_HEADERS.contains(name.toLowerCase(ROOT)) ? REDACTED : field.getValue(); + } + + public String sanitizeBody(Message message) { + if (message == null) { + return BODY_UNAVAILABLE; + } + + try { + if (message.isBodyEmpty()) { + return ""; + } + + // TODO: keep this? + String contentType = message.getHeader().getContentType(); + if (contentType != null && !(MimeType.isText(contentType) || MimeType.isJson(contentType) || MimeType.isXML(contentType))) { + return BINARY_BODY_OMITTED; + } + + String body = message.getBodyAsStringDecoded(); + if (body.length() <= MAX_BODY_LENGTH) { + return body; + } + return body.substring(0, MAX_BODY_LENGTH) + "... "; + } catch (Exception e) { + return BODY_UNAVAILABLE; + } + } +} diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpSessionContext.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpSessionContext.java new file mode 100644 index 0000000000..615e22d543 --- /dev/null +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpSessionContext.java @@ -0,0 +1,62 @@ +package com.predic8.membrane.core.interceptor.mcp; + +import com.predic8.membrane.core.mcp.MCPInitialize.ClientInfo; + +import static com.predic8.membrane.core.interceptor.mcp.McpSessionContext.McpSessionState.*; + +public final class McpSessionContext { + + private McpSessionState state = NEW; + + // TODO: only if we want to support multiple versions (maybe in the future?) + private String negotiatedProtocolVersion; + private ClientInfo clientInfo; + + public synchronized McpSessionState getState() { + return state; + } + + public synchronized boolean initialize(String protocolVersion, ClientInfo clientInfo) { + if (state != NEW) { + return false; + } + negotiatedProtocolVersion = protocolVersion; + this.clientInfo = clientInfo; + state = INITIALIZED; + return true; + } + + public synchronized boolean markReady() { + if (state != INITIALIZED) { + return false; + } + state = READY; + return true; + } + + @SuppressWarnings("BooleanMethodIsAlwaysInverted") + public synchronized boolean isIn(McpSessionState... states) { + for (McpSessionState candidate : states) { + if (state == candidate) { + return true; + } + } + return false; + } + + public synchronized String getNegotiatedProtocolVersion() { + return negotiatedProtocolVersion; + } + + public synchronized ClientInfo getClientInfo() { + return clientInfo; + } + + public enum McpSessionState { + NEW, + INITIALIZED, + READY, + CLOSED + } + +} From e29eb1206c998874a7162c22cac373a6d4e5c1e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Thu, 30 Apr 2026 09:53:39 +0200 Subject: [PATCH 22/29] Remove unused imports in `McpPayloadSanitizer` for cleanup --- .../membrane/core/interceptor/mcp/McpPayloadSanitizer.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpPayloadSanitizer.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpPayloadSanitizer.java index 02591b5443..addf83e551 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpPayloadSanitizer.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/McpPayloadSanitizer.java @@ -6,12 +6,10 @@ import com.predic8.membrane.core.http.MimeType; import java.util.LinkedHashMap; -import java.util.Locale; import java.util.Map; import java.util.Set; import static com.predic8.membrane.core.http.Header.*; -import static com.predic8.membrane.core.http.Header.PROXY_AUTHORIZATION; import static java.util.Locale.ROOT; public final class McpPayloadSanitizer { From 6132374cd7184c7598394e622b0973070f7e2102 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Thu, 30 Apr 2026 09:57:12 +0200 Subject: [PATCH 23/29] Refactor and streamline MCP server request handling: add tool registration, session management, and improved error handling. --- .../interceptor/mcp/MembraneMCPServer.java | 454 +++++++++++------- 1 file changed, 292 insertions(+), 162 deletions(-) diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java index b2d77804fb..be897fe1fc 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java @@ -7,6 +7,7 @@ import com.predic8.membrane.core.http.Response; import com.predic8.membrane.core.interceptor.AbstractInterceptor; import com.predic8.membrane.core.interceptor.Outcome; +import com.predic8.membrane.core.interceptor.mcp.McpSessionContext.McpSessionState; import com.predic8.membrane.core.jsonrpc.JSONRPCRequest; import com.predic8.membrane.core.jsonrpc.JSONRPCResponse; import com.predic8.membrane.core.mcp.*; @@ -14,24 +15,23 @@ import com.predic8.membrane.core.proxies.Proxy; import com.predic8.membrane.core.proxies.SOAPProxy; import com.predic8.membrane.core.proxies.ServiceProxy; -import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.IOException; -import java.util.HashMap; -import java.util.Map; -import java.util.Objects; +import java.util.*; import static com.predic8.membrane.annot.Constants.VERSION; import static com.predic8.membrane.core.http.MimeType.APPLICATION_JSON; -import static com.predic8.membrane.core.http.Response.ok; +import static com.predic8.membrane.core.http.Response.*; import static com.predic8.membrane.core.interceptor.Outcome.RETURN; +import static com.predic8.membrane.core.interceptor.mcp.McpSessionContext.McpSessionState.*; +import static com.predic8.membrane.core.jsonrpc.JSONRPCRequest.parse; import static com.predic8.membrane.core.jsonrpc.JSONRPCResponse.*; /** - * @description MCP Server for Membrane. It allows to query Membrane's internal state and operation from an LLM + * @description MCP Server for Membrane. It allows querying Membrane's internal state and operation from an LLM * Ask the LLM questions like: * - What APIs are deployed? * - Is the Membrane instance healthy? @@ -40,226 +40,356 @@ @MCElement(name = "membraneMCPServer") public class MembraneMCPServer extends AbstractInterceptor { + static final String SUPPORTED_PROTOCOL_VERSION = "2025-03-26"; + private static final Logger log = LoggerFactory.getLogger(MembraneMCPServer.class); + private static final int MAX_EXCHANGES = 100; + private static final Map EMPTY_OBJECT_SCHEMA = Map.of( + "type", "object", + "properties", Map.of(), + "additionalProperties", false + ); + private static final Map GET_EXCHANGES_SCHEMA = Map.of( + "type", "object", + "properties", Map.of( + "limit", Map.of("type", "integer", "minimum", 1, "maximum", MAX_EXCHANGES), + "includeBodies", Map.of("type", "boolean") + ), + "additionalProperties", false + ); + + private final McpSessionContext sessionContext = new McpSessionContext(); + private final McpPayloadSanitizer payloadSanitizer = new McpPayloadSanitizer(); + private final McpToolRegistry toolRegistry = buildToolRegistry(); + @Override public Outcome handleRequest(Exchange exc) { try { - JSONRPCRequest request; - try { - request = JSONRPCRequest.parse(exc.getRequest().getBodyAsStreamDecoded()); - } catch (JsonProcessingException e) { - exc.setResponse(createResponse(createErrorResponse(exc, null, ERR_PARSE_ERROR, "Parse error", e))); - return RETURN; - } catch (IOException e) { - exc.setResponse(createResponse(createErrorResponse(exc, null, ERR_INVALID_REQUEST, "Invalid Request", e))); - return RETURN; - } - - JSONRPCResponse rpcResponse; - try { - rpcResponse = processMCPRequest(request); - } catch (IllegalArgumentException e) { - exc.setResponse(createResponse(createProcessingErrorResponse(exc, request, JSONRPCResponse.ERR_INVALID_PARAMS, "Invalid params", e))); - return RETURN; - } catch (Exception e) { - exc.setResponse(createResponse(createProcessingErrorResponse(exc, request, ERR_INTERNAL_ERROR, "Internal error", e))); - return RETURN; - } - exc.setResponse(createResponse(rpcResponse)); + exc.setResponse(handleHttpRequest(exc)); return RETURN; - } catch (IOException e) { throw new RuntimeException(e); } } - private JSONRPCResponse createErrorResponse(Exchange exc, @Nullable JSONRPCRequest request, int code, String message, Exception e) { - if (code == ERR_INTERNAL_ERROR) { - log.warn("Failed to handle MCP request {} {}.", exc.getRequest().getMethod(), exc.getRequest().getUri(), e); - } else { - log.info("Rejected MCP request {} {}: {}", exc.getRequest().getMethod(), exc.getRequest().getUri(), e.getMessage()); + private Response handleHttpRequest(Exchange exc) throws IOException { + if (!exc.getRequest().isPOSTRequest()) { + return methodNotAllowed().build(); + } + + JSONRPCRequest request; + try { + request = parse(exc.getRequest().getBodyAsStreamDecoded()); + } catch (JsonProcessingException e) { + return createResponse(badRequest(exc, null, ERR_PARSE_ERROR, "Parse error", e)); + } catch (IOException e) { + return createResponse(badRequest(exc, null, ERR_INVALID_REQUEST, "Invalid request", e)); + } + + try { + return createResponse(processMcpRequest(request, exc)); + } catch (IllegalArgumentException e) { + return createResponse(badRequest(exc, request, ERR_INVALID_PARAMS, "Invalid params", e)); + } catch (Exception e) { + return createResponse(internalError(exc, request, e)); } - return error(request == null ? null : request.getId(), code, message, e.getMessage()); } - private @Nullable JSONRPCResponse createProcessingErrorResponse(Exchange exc, JSONRPCRequest request, int code, String message, Exception e) { - if (request.isNotification()) { - if (code == ERR_INTERNAL_ERROR) { - log.warn("Failed to handle MCP notification {} {}.", exc.getRequest().getMethod(), exc.getRequest().getUri(), e); - } else { - log.info("Rejected MCP notification {} {}: {}", exc.getRequest().getMethod(), exc.getRequest().getUri(), e.getMessage()); - } - return null; + private McpHttpResult processMcpRequest(JSONRPCRequest request, Exchange exc) throws Exception { + return switch (request.getMethod()) { + case MCPInitialize.METHOD -> processInitialize(request); + case MCPInitialized.METHOD -> processInitializedNotification(request); + case MCPPing.METHOD -> processPing(request); + case MCPToolsList.METHOD -> processToolsList(request); + case MCPToolsCall.METHOD -> processToolsCall(request, exc); + default -> protocolError(request, ERR_METHOD_NOT_FOUND, "Method not found"); + }; + } + + private McpHttpResult processInitialize(JSONRPCRequest request) { + MCPInitialize initialize = MCPInitialize.from(request); + if (!SUPPORTED_PROTOCOL_VERSION.equals(initialize.getProtocolVersion())) { + return protocolError( + request, + ERR_INVALID_PARAMS, + "Unsupported protocol version: " + initialize.getProtocolVersion() + ); } - return createErrorResponse(exc, request, code, message, e); + if (!sessionContext.initialize(SUPPORTED_PROTOCOL_VERSION, initialize.getClientInfo())) { + return protocolError(request, ERR_INVALID_REQUEST, "'initialize' must be the first MCP request"); + } + + return httpOk(new MCPInitializeResponse(initialize) + .withProtocolVersion(SUPPORTED_PROTOCOL_VERSION) + .withCapabilities(getCapabilities()) + .withServerInfo("Membrane", VERSION) + .toRpcResponse()); } - private static Response createResponse(@Nullable JSONRPCResponse rpcResponse) throws IOException { - if (rpcResponse == null) { - return Response.noContent().build(); + private McpHttpResult processInitializedNotification(JSONRPCRequest request) { + MCPInitialized.from(request); + if (!sessionContext.markReady()) { + return protocolError( + request, + ERR_INVALID_REQUEST, + "'notifications/initialized' is only allowed after a successful 'initialize'" + ); } - return ok().contentType(APPLICATION_JSON).body(rpcResponse.toJson()).build(); + log.debug("MCP client is ready"); + return acceptedNotification(); } - private @Nullable JSONRPCResponse processMCPRequest(JSONRPCRequest request) { - switch (request.getMethod()) { - case "initialize" -> { - return initialize(request).toRpcResponse(); - } - case "notifications/initialized" -> { - MCPInitialized.from(request); - log.debug("MCP Client is ready"); - return null; - } - case "tools/list" -> { - return toolsList(request).toRpcResponse(); - } - case "tools/call" -> { - var response = toolsCall(request); - return response == null ? null : response.toRpcResponse(); - } - default -> { - log.info("Unknown MCP Request: {}", request); - if (!request.isNotification()) { - return error(request.getId(), ERR_METHOD_NOT_FOUND, "Method not found"); - } - } + private McpHttpResult processPing(JSONRPCRequest request) { + MCPPing.from(request); + if (!sessionContext.isIn(INITIALIZED, READY)) { + return protocolError(request, ERR_INVALID_REQUEST, "'ping' is only allowed after 'initialize'"); } - return null; + return httpOk(success(request.getId(), Map.of())); } - private MCPToolsCallResponse toolsCall(JSONRPCRequest request) { - var req = MCPToolsCall.from(request); + private McpHttpResult processToolsList(JSONRPCRequest request) { + MCPToolsList toolsList = MCPToolsList.from(request); + if (!sessionContext.isIn(READY)) { + return protocolError(request, ERR_INVALID_REQUEST, "'tools/list' requires a completed MCP handshake"); + } - log.debug("Received MCP tools call: {}", req); + return httpOk(MCPToolsListResponse.from(toolsList) + .withTools(toolRegistry.list().stream().map(McpToolDefinition::toTool).toList()) + .toRpcResponse()); + } - switch (req.getName()) { - case "listProxies" -> { - return listProxies(req); - } - case "getExchanges" -> { - return getExchanges(req); - } - case "getStatistics" -> { - return getStatistics(req); - } - default -> log.info("Unknown tools call: " + req.getName()); + private McpHttpResult processToolsCall(JSONRPCRequest request, Exchange exc) throws Exception { + MCPToolsCall call = MCPToolsCall.from(request); + if (!sessionContext.isIn(READY)) { + return protocolError(request, ERR_INVALID_REQUEST, "'tools/call' requires a completed MCP handshake"); + } + + McpToolDefinition tool = toolRegistry.find(call.getName()); + if (tool == null) { + return protocolError(request, ERR_INVALID_PARAMS, "Unknown tool: " + call.getName()); } - return null; + try { + return httpOk(tool.handler().handle(call, exc).toRpcResponse()); + } catch (IllegalArgumentException e) { + return httpOk(MCPToolsCallResponse.toolError(call, e.getMessage()).toRpcResponse()); + } } - private MCPToolsCallResponse getStatistics(MCPToolsCall req) { - return MCPToolsCallResponse.from(req) + private McpToolRegistry buildToolRegistry() { + return new McpToolRegistry() + .register(new McpToolDefinition( + "listProxies", + "Lists configured proxies and selected runtime metadata", + EMPTY_OBJECT_SCHEMA, + this::listProxies + )) + .register(new McpToolDefinition( + "getExchanges", + "Gets recent HTTP exchanges with sanitized headers and optional bodies", + GET_EXCHANGES_SCHEMA, + this::getExchanges + )) + .register(new McpToolDefinition( + "getStatistics", + "Gets Membrane runtime statistics", + EMPTY_OBJECT_SCHEMA, + this::getStatistics + )); + } + + private MCPToolsCallResponse listProxies(MCPToolsCall call, Exchange exc) { + rejectUnexpectedArguments(call, Set.of()); + return MCPToolsCallResponse.from(call) + .withJson(Map.of( + "proxies", + getRouter().getRuleManager().getRules().stream() + .map(this::getProxyDescription) + .toList() + )); + } + + private MCPToolsCallResponse getStatistics(MCPToolsCall call, Exchange exc) { + rejectUnexpectedArguments(call, Set.of()); + return MCPToolsCallResponse.from(call) .withJson(getRouter().getStatistics()); } - private MCPToolsCallResponse getExchanges(MCPToolsCall req) { - var exchanges = getRouter().getExchangeStore().getAllExchangesAsList(); - int start = Math.max(0, exchanges.size() - MAX_EXCHANGES); + private MCPToolsCallResponse getExchanges(MCPToolsCall call, Exchange exc) { + rejectUnexpectedArguments(call, Set.of("limit", "includeBodies")); + + int limit = getOptionalIntArgument(call, "limit", MAX_EXCHANGES, 1, MAX_EXCHANGES); + boolean includeBodies = getOptionalBooleanArgument(call, "includeBodies", false); + + List exchanges = Optional.ofNullable(getRouter().getExchangeStore().getAllExchangesAsList()) + .orElse(List.of()); + int start = Math.max(0, exchanges.size() - limit); - return MCPToolsCallResponse.from(req) - .withJson(Map.of("exchanges", exchanges.subList(start, exchanges.size()).stream() - .map(MembraneMCPServer::getExchangeDescription) - .filter(Objects::nonNull).toList())); + return MCPToolsCallResponse.from(call) + .withJson(Map.of( + "exchanges", + exchanges.subList(start, exchanges.size()).stream() + .map(exchange -> describeExchange(exchange, includeBodies)) + .filter(Objects::nonNull) + .toList() + )); } - private static @Nullable HashMap getExchangeDescription(AbstractExchange e) { - if (e.getResponse() == null) + private @Nullable Map describeExchange(AbstractExchange exchange, boolean includeBodies) { + if (exchange.getResponse() == null) { return null; + } - var exc = new HashMap(); - exc.put("id", e.getId()); - var request = new HashMap(); - var response = new HashMap(); - - request.put("method", e.getRequest().getMethod()); - request.put("path", e.getRequest().getUri()); - request.put("body", e.getRequest().getBodyAsStringDecoded()); - request.put("headers", e.getRequest().getHeader()); + var description = new LinkedHashMap(); + description.put("id", exchange.getId()); - response.put("status", e.getResponse().getStatusCode()); - response.put("body", e.getResponse().getBodyAsStringDecoded()); - response.put("headers", e.getResponse().getHeader()); + var request = new LinkedHashMap(); + request.put("method", exchange.getRequest().getMethod()); + request.put("path", exchange.getRequest().getUri()); + request.put("headers", payloadSanitizer.sanitizeHeaders(exchange.getRequest().getHeader())); + if (includeBodies) { + request.put("body", payloadSanitizer.sanitizeBody(exchange.getRequest())); + } - exc.put("request", request); - exc.put("response", response); - return exc; - } + var response = new LinkedHashMap(); + response.put("status", exchange.getResponse().getStatusCode()); + response.put("headers", payloadSanitizer.sanitizeHeaders(exchange.getResponse().getHeader())); + if (includeBodies) { + response.put("body", payloadSanitizer.sanitizeBody(exchange.getResponse())); + } - private MCPToolsCallResponse listProxies(MCPToolsCall req) { - return MCPToolsCallResponse.from(req) - .withJson(Map.of("proxies", getRouter().getRuleManager().getRules().stream().map(this::getProxyDescription).toList())); + description.put("request", request); + description.put("response", response); + return description; } - private @NotNull HashMap getProxyDescription(Proxy p) { - var proxy = new HashMap(); - proxy.put("name", p.getName()); + private Map getProxyDescription(Proxy proxy) { + var description = new LinkedHashMap(); + description.put("name", proxy.getName()); String type; - switch (p) { - case APIProxy ap -> { - type = "API"; - // proxy.put("openapi", ap.getOpenapi()); - } - case SOAPProxy sp -> { + switch (proxy) { + case APIProxy ignored -> type = "API"; + case SOAPProxy soapProxy -> { type = "soapProxy"; - proxy.put("wsdl", sp.getWsdl()); - proxy.put("serviceName", sp.getServiceName()); - } - case ServiceProxy s -> { - type = "serviceProxy"; + description.put("wsdl", soapProxy.getWsdl()); + description.put("serviceName", soapProxy.getServiceName()); } - default -> { - type = "unknown"; + case ServiceProxy ignored -> type = "serviceProxy"; + default -> type = "unknown"; + } + + description.put("type", type); + description.put("rule", proxy.getKey().toString()); + description.put("interceptors", proxy.getFlow().stream() + .map(interceptor -> Map.of("name", interceptor.getDisplayName())) + .toList()); + description.put("statistics", getRouter().getExchangeStore().getStatistics(proxy.getKey())); + return description; + } + + private static int getOptionalIntArgument(MCPToolsCall call, String name, int defaultValue, int minimum, int maximum) { + Object value = call.getArgument(name); + if (value == null) { + return defaultValue; + } + if (!(value instanceof Number number) || number.doubleValue() != Math.rint(number.doubleValue())) { + throw new IllegalArgumentException("Tool argument '" + name + "' must be an integer"); + } + int parsed = number.intValue(); + if (parsed < minimum || parsed > maximum) { + throw new IllegalArgumentException( + "Tool argument '" + name + "' must be between " + minimum + " and " + maximum + ); + } + return parsed; + } + + private static boolean getOptionalBooleanArgument(MCPToolsCall call, String name, boolean defaultValue) { + Object value = call.getArgument(name); + if (value == null) { + return defaultValue; + } + if (value instanceof Boolean bool) { + return bool; + } + throw new IllegalArgumentException("Tool argument '" + name + "' must be a boolean"); + } + + private static void rejectUnexpectedArguments(MCPToolsCall call, Set allowed) { + for (String argumentName : call.getArguments().keySet()) { + if (!allowed.contains(argumentName)) { + throw new IllegalArgumentException("Unexpected tool argument: " + argumentName); } } + } - var interceptors = p.getFlow().stream().map(i -> { - Map interceptor = new HashMap<>(); - interceptor.put("name", i.getDisplayName()); - return interceptor; - }).toList(); + private McpHttpResult badRequest(Exchange exc, @Nullable JSONRPCRequest request, int code, String message, Exception exception) { + log.info("Rejected MCP request {} {}: {}", exc.getRequest().getMethod(), exc.getRequest().getUri(), exception.getMessage()); + return new McpHttpResult(400, error(responseId(request), code, message, exception.getMessage())); + } - proxy.put("statistics", getRouter().getExchangeStore().getStatistics(p.getKey())); + private McpHttpResult internalError(Exchange exc, @Nullable JSONRPCRequest request, Exception exception) { + log.warn("Failed to handle MCP request {} {}.", exc.getRequest().getMethod(), exc.getRequest().getUri(), exception); + return new McpHttpResult( + request != null && request.isNotification() ? 500 : 200, + error(responseId(request), ERR_INTERNAL_ERROR, "Internal error", exception.getMessage()) + ); + } - proxy.put("interceptors", interceptors); - proxy.put("type", type); - proxy.put("rule", p.getKey().toString()); - return proxy; + private McpHttpResult protocolError(JSONRPCRequest request, int code, String message) { + return protocolError(request, code, message, null); } - private MCPToolsListResponse toolsList(JSONRPCRequest request) { - log.debug("Tools list"); - return MCPToolsListResponse.from(MCPToolsList.from(request)) - .withTool(new MCPToolsListResponse.Tool( - "listProxies", - "Lists all the proxies, e.g. API, soapProxy", Map.of("type", "object"))) - .withTool(new MCPToolsListResponse.Tool("getExchanges", "Gets the last 100 HTTP exchanges", Map.of("type", "object"))) - .withTool(new MCPToolsListResponse.Tool("getStatistics", "Gets Membrane runtime statistics", Map.of("type", "object"))); + private McpHttpResult protocolError(JSONRPCRequest request, int code, String message, @Nullable Object data) { + return new McpHttpResult( + request.isNotification() ? 400 : 200, + data == null ? error(responseId(request), code, message) : error(responseId(request), code, message, data) + ); + } - // Map.of("type", "object", -// "properties", Map.of("query", Map.of("type", "string")), -// "required", List.of("query")))); + private static Object responseId(@Nullable JSONRPCRequest request) { + if (request == null || request.isNotification()) { + return null; + } + return request.getId(); + } + private static McpHttpResult httpOk(JSONRPCResponse response) { + return new McpHttpResult(200, response); } - private MCPInitializeResponse initialize(JSONRPCRequest request) { - return new MCPInitializeResponse(new MCPInitialize(request)) - .withCapabilities(getCapabilities()) - .withServerInfo("Membrane", VERSION); + private static McpHttpResult acceptedNotification() { + return new McpHttpResult(202, null); } - private static @NotNull HashMap getCapabilities() { - var capabilities = new HashMap(); - capabilities.put("tools", Map.of("listChanged", false)); - return capabilities; + private static Response createResponse(McpHttpResult result) throws IOException { + if (result.body() == null) { + if (result.status() == 202) { + return accepted().build(); + } + return statusCode(result.status()).bodyEmpty().build(); + } + return statusCode(result.status()) + .contentType(APPLICATION_JSON) + .body(result.body().toJson()) + .build(); + } + + private static Map getCapabilities() { + return Map.of("tools", Map.of("listChanged", false)); } @Override public String getDisplayName() { return "Membrane MCP Server"; } + + record McpHttpResult( + int status, + @Nullable JSONRPCResponse body + ) { + } } From bc522ffb9c8493b02680e97b36f51e72ca3088be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Thu, 30 Apr 2026 10:03:37 +0200 Subject: [PATCH 24/29] Refactor `MembraneMCPServer`: improve HTTP 405 response handling and clean up imports --- .../core/interceptor/mcp/MembraneMCPServer.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java index be897fe1fc..909fe35f89 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java @@ -7,7 +7,6 @@ import com.predic8.membrane.core.http.Response; import com.predic8.membrane.core.interceptor.AbstractInterceptor; import com.predic8.membrane.core.interceptor.Outcome; -import com.predic8.membrane.core.interceptor.mcp.McpSessionContext.McpSessionState; import com.predic8.membrane.core.jsonrpc.JSONRPCRequest; import com.predic8.membrane.core.jsonrpc.JSONRPCResponse; import com.predic8.membrane.core.mcp.*; @@ -24,9 +23,12 @@ import static com.predic8.membrane.annot.Constants.VERSION; import static com.predic8.membrane.core.http.MimeType.APPLICATION_JSON; -import static com.predic8.membrane.core.http.Response.*; +import static com.predic8.membrane.core.http.Request.METHOD_POST; +import static com.predic8.membrane.core.http.Response.accepted; +import static com.predic8.membrane.core.http.Response.statusCode; import static com.predic8.membrane.core.interceptor.Outcome.RETURN; -import static com.predic8.membrane.core.interceptor.mcp.McpSessionContext.McpSessionState.*; +import static com.predic8.membrane.core.interceptor.mcp.McpSessionContext.McpSessionState.INITIALIZED; +import static com.predic8.membrane.core.interceptor.mcp.McpSessionContext.McpSessionState.READY; import static com.predic8.membrane.core.jsonrpc.JSONRPCRequest.parse; import static com.predic8.membrane.core.jsonrpc.JSONRPCResponse.*; @@ -76,7 +78,10 @@ public Outcome handleRequest(Exchange exc) { private Response handleHttpRequest(Exchange exc) throws IOException { if (!exc.getRequest().isPOSTRequest()) { - return methodNotAllowed().build(); + return statusCode(405) + .header("Allow", METHOD_POST) + .bodyEmpty() + .build(); } JSONRPCRequest request; From b953c177decbb619f4a2788d1674540ec4bd2e27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Thu, 30 Apr 2026 10:16:43 +0200 Subject: [PATCH 25/29] Add unit tests for `MembraneMCPServer` and `MCPInitialize` to validate lifecycle handling, error scenarios, and MCP tool behaviors --- .../mcp/MembraneMCPServerTest.java | 238 ++++++++++++++++++ .../membrane/core/mcp/MCPInitializeTest.java | 73 ++++++ 2 files changed, 311 insertions(+) create mode 100644 core/src/test/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServerTest.java create mode 100644 core/src/test/java/com/predic8/membrane/core/mcp/MCPInitializeTest.java diff --git a/core/src/test/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServerTest.java b/core/src/test/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServerTest.java new file mode 100644 index 0000000000..f21e485c06 --- /dev/null +++ b/core/src/test/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServerTest.java @@ -0,0 +1,238 @@ +package com.predic8.membrane.core.interceptor.mcp; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.predic8.membrane.core.exchange.AbstractExchange; +import com.predic8.membrane.core.exchange.Exchange; +import com.predic8.membrane.core.exchangestore.ForgetfulExchangeStore; +import com.predic8.membrane.core.http.Request; +import com.predic8.membrane.core.http.Response; +import com.predic8.membrane.core.interceptor.Outcome; +import com.predic8.membrane.core.jsonrpc.JSONRPCRequest; +import com.predic8.membrane.core.mcp.MCPInitialize; +import com.predic8.membrane.core.mcp.MCPInitialized; +import com.predic8.membrane.core.mcp.MCPPing; +import com.predic8.membrane.core.mcp.MCPToolsCall; +import com.predic8.membrane.core.mcp.MCPToolsList; +import com.predic8.membrane.core.router.TestRouter; +import org.junit.jupiter.api.Test; + +import java.net.URISyntaxException; +import java.util.List; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class MembraneMCPServerTest { + + private static final ObjectMapper OM = new ObjectMapper(); + + @Test + void toolsListBeforeInitializeIsRejected() throws Exception { + Exchange exc = invoke(newServer(), request(1, MCPToolsList.METHOD, Map.of())); + + assertEquals(200, exc.getResponse().getStatusCode()); + assertEquals(-32600, responseJson(exc).path("error").path("code").asInt()); + } + + @Test + void initializedNotificationBeforeInitializeIsRejected() throws Exception { + Exchange exc = invoke(newServer(), notification(MCPInitialized.METHOD, Map.of())); + + assertEquals(400, exc.getResponse().getStatusCode()); + assertEquals(-32600, responseJson(exc).path("error").path("code").asInt()); + } + + @Test + void initializePingInitializedAndToolsListFollowTheLifecycle() throws Exception { + MembraneMCPServer server = newServer(); + + Exchange initialize = invoke(server, initializeRequest("2025-03-26")); + assertEquals(200, initialize.getResponse().getStatusCode()); + JsonNode initializeJson = responseJson(initialize); + assertEquals("2025-03-26", initializeJson.path("result").path("protocolVersion").asText()); + + Exchange ping = invoke(server, request(2, MCPPing.METHOD, Map.of())); + assertEquals(200, ping.getResponse().getStatusCode()); + assertTrue(responseJson(ping).path("result").isObject()); + assertEquals(0, responseJson(ping).path("result").size()); + + Exchange initialized = invoke(server, notification(MCPInitialized.METHOD, Map.of())); + assertEquals(202, initialized.getResponse().getStatusCode()); + assertEquals("", initialized.getResponse().getBodyAsStringDecoded()); + + Exchange toolsList = invoke(server, request(3, MCPToolsList.METHOD, Map.of())); + JsonNode tools = responseJson(toolsList).path("result").path("tools"); + assertEquals(3, tools.size()); + assertEquals("listProxies", tools.get(0).path("name").asText()); + assertEquals("getExchanges", tools.get(1).path("name").asText()); + assertTrue(tools.get(1).path("inputSchema").path("properties").has("limit")); + assertFalse(tools.get(1).path("inputSchema").path("additionalProperties").asBoolean(true)); + } + + @Test + void unknownToolReturnsJsonRpcError() throws Exception { + MembraneMCPServer server = readyServer(); + + Exchange exc = invoke(server, request( + 2, + MCPToolsCall.METHOD, + Map.of("name", "doesNotExist", "arguments", Map.of()) + )); + + assertEquals(200, exc.getResponse().getStatusCode()); + assertEquals(-32602, responseJson(exc).path("error").path("code").asInt()); + } + + @Test + void toolValidationErrorsStayInsideToolResult() throws Exception { + MembraneMCPServer server = readyServer(); + + Exchange exc = invoke(server, request( + 2, + MCPToolsCall.METHOD, + Map.of("name", "getExchanges", "arguments", Map.of("limit", 0)) + )); + + assertEquals(200, exc.getResponse().getStatusCode()); + JsonNode response = responseJson(exc); + assertTrue(response.path("error").isMissingNode()); + assertTrue(response.path("result").path("isError").asBoolean()); + } + + @Test + void getWithoutSseReturns405AndAllowPost() throws Exception { + MembraneMCPServer server = newServer(); + Exchange exc = Request.get("http://localhost/mcp").buildExchange(); + + assertEquals(Outcome.RETURN, server.handleRequest(exc)); + assertEquals(405, exc.getResponse().getStatusCode()); + assertEquals(Request.METHOD_POST, exc.getResponse().getHeader().getFirstValue("Allow")); + } + + @Test + void unsupportedProtocolVersionIsRejected() throws Exception { + Exchange exc = invoke(newServer(), initializeRequest("2024-11-05")); + + assertEquals(200, exc.getResponse().getStatusCode()); + assertEquals(-32602, responseJson(exc).path("error").path("code").asInt()); + } + + @Test + void getExchangesRedactsSensitiveHeadersAndOmitsBinaryBodies() throws Exception { + MembraneMCPServer server = newServer(new StubExchangeStore(List.of(sampleExchange()))); + readyHandshake(server); + + Exchange exc = invoke(server, request( + 2, + MCPToolsCall.METHOD, + Map.of("name", "getExchanges", "arguments", Map.of("limit", 1, "includeBodies", true)) + )); + + JsonNode payload = toolPayload(exc); + JsonNode exchange = payload.path("exchanges").get(0); + + assertEquals("", exchange.path("request").path("headers").path("Authorization").asText()); + assertEquals("", exchange.path("request").path("headers").path("Cookie").asText()); + assertEquals("", exchange.path("response").path("headers").path("Set-Cookie").asText()); + assertEquals("", exchange.path("response").path("body").asText()); + } + + private static MembraneMCPServer newServer() { + return newServer(new ForgetfulExchangeStore()); + } + + private static MembraneMCPServer newServer(ForgetfulExchangeStore exchangeStore) { + TestRouter router = new TestRouter(); + router.setExchangeStore(exchangeStore); + router.init(); + + MembraneMCPServer server = new MembraneMCPServer(); + server.init(router); + return server; + } + + private static MembraneMCPServer readyServer() throws Exception { + MembraneMCPServer server = newServer(); + readyHandshake(server); + return server; + } + + private static void readyHandshake(MembraneMCPServer server) throws Exception { + invoke(server, initializeRequest("2025-03-26")); + invoke(server, notification(MCPInitialized.METHOD, Map.of())); + } + + private static JSONRPCRequest initializeRequest(String protocolVersion) { + return request( + 1, + MCPInitialize.METHOD, + Map.of( + "protocolVersion", protocolVersion, + "capabilities", Map.of(), + "clientInfo", Map.of("name", "test-client", "version", "1.0.0") + ) + ); + } + + private static JSONRPCRequest request(Object id, String method, Map params) { + return new JSONRPCRequest(id, method, params); + } + + private static JSONRPCRequest notification(String method, Map params) { + return new JSONRPCRequest(null, false, method, params); + } + + private static Exchange invoke(MembraneMCPServer server, JSONRPCRequest request) throws Exception { + Exchange exc = Request.post("http://localhost/mcp") + .json(request.toJson()) + .buildExchange(); + + assertEquals(Outcome.RETURN, server.handleRequest(exc)); + return exc; + } + + private static JsonNode responseJson(Exchange exc) throws Exception { + return OM.readTree(exc.getResponse().getBodyAsStringDecoded()); + } + + private static JsonNode toolPayload(Exchange exc) throws Exception { + JsonNode textNode = responseJson(exc) + .path("result") + .path("content") + .get(0) + .path("text"); + return OM.readTree(textNode.asText()); + } + + private static Exchange sampleExchange() throws URISyntaxException { + Exchange exc = new Exchange(null); + exc.setRequest(Request.post("http://localhost/internal") + .contentType("application/json") + .header("Authorization", "Bearer super-secret") + .header("Cookie", "session=top-secret") + .body("{\"secret\":true}") + .build()); + exc.setResponse(Response.ok() + .contentType("image/png") + .header("Set-Cookie", "session=top-secret") + .body(new byte[]{1, 2, 3, 4}) + .build()); + return exc; + } + + private static final class StubExchangeStore extends ForgetfulExchangeStore { + private final List exchanges; + + private StubExchangeStore(List exchanges) { + this.exchanges = exchanges; + } + + @Override + public List getAllExchangesAsList() { + return exchanges; + } + } +} diff --git a/core/src/test/java/com/predic8/membrane/core/mcp/MCPInitializeTest.java b/core/src/test/java/com/predic8/membrane/core/mcp/MCPInitializeTest.java new file mode 100644 index 0000000000..02d6dadce4 --- /dev/null +++ b/core/src/test/java/com/predic8/membrane/core/mcp/MCPInitializeTest.java @@ -0,0 +1,73 @@ +package com.predic8.membrane.core.mcp; + +import com.predic8.membrane.core.jsonrpc.JSONRPCRequest; +import org.junit.jupiter.api.Test; + +import java.util.Map; + +import static com.predic8.membrane.core.mcp.MCPInitialize.METHOD; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class MCPInitializeTest { + + @Test + void parsesRequiredInitializeFields() { + MCPInitialize initialize = new MCPInitialize(new JSONRPCRequest( + 1, + METHOD, + Map.of( + "protocolVersion", "2025-03-26", + "capabilities", Map.of("roots", Map.of()), + "clientInfo", Map.of("name", "test-client", "version", "1.0.0") + ) + )); + + assertEquals("2025-03-26", initialize.getProtocolVersion()); + assertEquals("test-client", initialize.getClientInfo().getName()); + assertEquals("1.0.0", initialize.getClientInfo().getVersion()); + } + + @Test + void rejectsMissingCapabilities() { + JSONRPCRequest request = new JSONRPCRequest( + 1, + METHOD, + Map.of( + "protocolVersion", "2025-03-26", + "clientInfo", Map.of("name", "test-client", "version", "1.0.0") + ) + ); + + assertThrows(IllegalArgumentException.class, () -> new MCPInitialize(request)); + } + + @Test + void rejectsMissingClientInfo() { + JSONRPCRequest request = new JSONRPCRequest( + 1, + METHOD, + Map.of( + "protocolVersion", "2025-03-26", + "capabilities", Map.of() + ) + ); + + assertThrows(IllegalArgumentException.class, () -> new MCPInitialize(request)); + } + + @Test + void rejectsBlankClientVersion() { + JSONRPCRequest request = new JSONRPCRequest( + 1, + METHOD, + Map.of( + "protocolVersion", "2025-03-26", + "capabilities", Map.of(), + "clientInfo", Map.of("name", "test-client", "version", " ") + ) + ); + + assertThrows(IllegalArgumentException.class, () -> new MCPInitialize(request)); + } +} From ab38255806f513194e7c15142c28d51daf2b761e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Thu, 30 Apr 2026 10:30:03 +0200 Subject: [PATCH 26/29] Refactor `MembraneMCPServer` error handling: introduce `InvalidToolArgumentsException`, improve protocol version validation, and enhance test coverage. --- .../interceptor/mcp/MembraneMCPServer.java | 23 ++-- .../mcp/MembraneMCPServerTest.java | 100 +++++++++++++----- 2 files changed, 87 insertions(+), 36 deletions(-) diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java index 909fe35f89..2c0cfd85b9 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java @@ -115,13 +115,6 @@ private McpHttpResult processMcpRequest(JSONRPCRequest request, Exchange exc) th private McpHttpResult processInitialize(JSONRPCRequest request) { MCPInitialize initialize = MCPInitialize.from(request); - if (!SUPPORTED_PROTOCOL_VERSION.equals(initialize.getProtocolVersion())) { - return protocolError( - request, - ERR_INVALID_PARAMS, - "Unsupported protocol version: " + initialize.getProtocolVersion() - ); - } if (!sessionContext.initialize(SUPPORTED_PROTOCOL_VERSION, initialize.getClientInfo())) { return protocolError(request, ERR_INVALID_REQUEST, "'initialize' must be the first MCP request"); } @@ -178,6 +171,8 @@ private McpHttpResult processToolsCall(JSONRPCRequest request, Exchange exc) thr try { return httpOk(tool.handler().handle(call, exc).toRpcResponse()); + } catch (InvalidToolArgumentsException e) { + return protocolError(request, ERR_INVALID_PARAMS, e.getMessage()); } catch (IllegalArgumentException e) { return httpOk(MCPToolsCallResponse.toolError(call, e.getMessage()).toRpcResponse()); } @@ -301,11 +296,11 @@ private static int getOptionalIntArgument(MCPToolsCall call, String name, int de return defaultValue; } if (!(value instanceof Number number) || number.doubleValue() != Math.rint(number.doubleValue())) { - throw new IllegalArgumentException("Tool argument '" + name + "' must be an integer"); + throw new InvalidToolArgumentsException("Tool argument '" + name + "' must be an integer"); } int parsed = number.intValue(); if (parsed < minimum || parsed > maximum) { - throw new IllegalArgumentException( + throw new InvalidToolArgumentsException( "Tool argument '" + name + "' must be between " + minimum + " and " + maximum ); } @@ -320,13 +315,13 @@ private static boolean getOptionalBooleanArgument(MCPToolsCall call, String name if (value instanceof Boolean bool) { return bool; } - throw new IllegalArgumentException("Tool argument '" + name + "' must be a boolean"); + throw new InvalidToolArgumentsException("Tool argument '" + name + "' must be a boolean"); } private static void rejectUnexpectedArguments(MCPToolsCall call, Set allowed) { for (String argumentName : call.getArguments().keySet()) { if (!allowed.contains(argumentName)) { - throw new IllegalArgumentException("Unexpected tool argument: " + argumentName); + throw new InvalidToolArgumentsException("Unexpected tool argument: " + argumentName); } } } @@ -397,4 +392,10 @@ record McpHttpResult( @Nullable JSONRPCResponse body ) { } + + private static final class InvalidToolArgumentsException extends IllegalArgumentException { + private InvalidToolArgumentsException(String message) { + super(message); + } + } } diff --git a/core/src/test/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServerTest.java b/core/src/test/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServerTest.java index f21e485c06..e0692ca2ef 100644 --- a/core/src/test/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServerTest.java +++ b/core/src/test/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServerTest.java @@ -9,25 +9,22 @@ import com.predic8.membrane.core.http.Response; import com.predic8.membrane.core.interceptor.Outcome; import com.predic8.membrane.core.jsonrpc.JSONRPCRequest; -import com.predic8.membrane.core.mcp.MCPInitialize; -import com.predic8.membrane.core.mcp.MCPInitialized; -import com.predic8.membrane.core.mcp.MCPPing; -import com.predic8.membrane.core.mcp.MCPToolsCall; -import com.predic8.membrane.core.mcp.MCPToolsList; +import com.predic8.membrane.core.mcp.*; import com.predic8.membrane.core.router.TestRouter; import org.junit.jupiter.api.Test; import java.net.URISyntaxException; import java.util.List; import java.util.Map; +import java.util.TreeMap; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; class MembraneMCPServerTest { private static final ObjectMapper OM = new ObjectMapper(); + private static final String POST_ACCEPT_HEADER = "application/json, text/event-stream"; + private static final String GET_ACCEPT_HEADER = "text/event-stream"; @Test void toolsListBeforeInitializeIsRejected() throws Exception { @@ -42,9 +39,32 @@ void initializedNotificationBeforeInitializeIsRejected() throws Exception { Exchange exc = invoke(newServer(), notification(MCPInitialized.METHOD, Map.of())); assertEquals(400, exc.getResponse().getStatusCode()); + if (!exc.getResponse().isBodyEmpty()) { + assertEquals(-32600, responseJson(exc).path("error").path("code").asInt()); + } + } + + @Test + void pingBeforeInitializeIsRejected() throws Exception { + Exchange exc = invoke(newServer(), request(1, MCPPing.METHOD, Map.of())); + + assertEquals(200, exc.getResponse().getStatusCode()); assertEquals(-32600, responseJson(exc).path("error").path("code").asInt()); } + @Test + void toolsListAfterInitializeButBeforeInitializedIsRejected() throws Exception { + MembraneMCPServer server = newServer(); + + Exchange initialize = invoke(server, initializeRequest("2025-03-26")); + assertEquals(200, initialize.getResponse().getStatusCode()); + assertEquals(MembraneMCPServer.SUPPORTED_PROTOCOL_VERSION, responseJson(initialize).path("result").path("protocolVersion").asText()); + + Exchange toolsList = invoke(server, request(2, MCPToolsList.METHOD, Map.of())); + assertEquals(200, toolsList.getResponse().getStatusCode()); + assertEquals(-32600, responseJson(toolsList).path("error").path("code").asInt()); + } + @Test void initializePingInitializedAndToolsListFollowTheLifecycle() throws Exception { MembraneMCPServer server = newServer(); @@ -52,7 +72,7 @@ void initializePingInitializedAndToolsListFollowTheLifecycle() throws Exception Exchange initialize = invoke(server, initializeRequest("2025-03-26")); assertEquals(200, initialize.getResponse().getStatusCode()); JsonNode initializeJson = responseJson(initialize); - assertEquals("2025-03-26", initializeJson.path("result").path("protocolVersion").asText()); + assertEquals(MembraneMCPServer.SUPPORTED_PROTOCOL_VERSION, initializeJson.path("result").path("protocolVersion").asText()); Exchange ping = invoke(server, request(2, MCPPing.METHOD, Map.of())); assertEquals(200, ping.getResponse().getStatusCode()); @@ -65,11 +85,13 @@ void initializePingInitializedAndToolsListFollowTheLifecycle() throws Exception Exchange toolsList = invoke(server, request(3, MCPToolsList.METHOD, Map.of())); JsonNode tools = responseJson(toolsList).path("result").path("tools"); - assertEquals(3, tools.size()); - assertEquals("listProxies", tools.get(0).path("name").asText()); - assertEquals("getExchanges", tools.get(1).path("name").asText()); - assertTrue(tools.get(1).path("inputSchema").path("properties").has("limit")); - assertFalse(tools.get(1).path("inputSchema").path("additionalProperties").asBoolean(true)); + JsonNode getExchangesTool = findToolByName(tools, "getExchanges"); + + assertNotNull(findToolByName(tools, "listProxies")); + assertNotNull(getExchangesTool); + assertNotNull(findToolByName(tools, "getStatistics")); + assertTrue(getExchangesTool.path("inputSchema").path("properties").has("limit")); + assertFalse(getExchangesTool.path("inputSchema").path("additionalProperties").asBoolean(true)); } @Test @@ -87,7 +109,7 @@ void unknownToolReturnsJsonRpcError() throws Exception { } @Test - void toolValidationErrorsStayInsideToolResult() throws Exception { + void invalidToolArgumentsReturnJsonRpcError() throws Exception { MembraneMCPServer server = readyServer(); Exchange exc = invoke(server, request( @@ -98,14 +120,15 @@ void toolValidationErrorsStayInsideToolResult() throws Exception { assertEquals(200, exc.getResponse().getStatusCode()); JsonNode response = responseJson(exc); - assertTrue(response.path("error").isMissingNode()); - assertTrue(response.path("result").path("isError").asBoolean()); + assertEquals(-32602, response.path("error").path("code").asInt()); } @Test void getWithoutSseReturns405AndAllowPost() throws Exception { MembraneMCPServer server = newServer(); - Exchange exc = Request.get("http://localhost/mcp").buildExchange(); + Exchange exc = Request.get("http://localhost/mcp") + .header("Accept", GET_ACCEPT_HEADER) + .buildExchange(); assertEquals(Outcome.RETURN, server.handleRequest(exc)); assertEquals(405, exc.getResponse().getStatusCode()); @@ -113,11 +136,13 @@ void getWithoutSseReturns405AndAllowPost() throws Exception { } @Test - void unsupportedProtocolVersionIsRejected() throws Exception { + void unsupportedProtocolVersionNegotiatesServerVersion() throws Exception { Exchange exc = invoke(newServer(), initializeRequest("2024-11-05")); assertEquals(200, exc.getResponse().getStatusCode()); - assertEquals(-32602, responseJson(exc).path("error").path("code").asInt()); + JsonNode response = responseJson(exc); + assertTrue(response.path("error").isMissingNode()); + assertEquals(MembraneMCPServer.SUPPORTED_PROTOCOL_VERSION, response.path("result").path("protocolVersion").asText()); } @Test @@ -133,10 +158,12 @@ void getExchangesRedactsSensitiveHeadersAndOmitsBinaryBodies() throws Exception JsonNode payload = toolPayload(exc); JsonNode exchange = payload.path("exchanges").get(0); + Map requestHeaders = normalizeHeaders(exchange.path("request").path("headers")); + Map responseHeaders = normalizeHeaders(exchange.path("response").path("headers")); - assertEquals("", exchange.path("request").path("headers").path("Authorization").asText()); - assertEquals("", exchange.path("request").path("headers").path("Cookie").asText()); - assertEquals("", exchange.path("response").path("headers").path("Set-Cookie").asText()); + assertEquals("", requestHeaders.get("authorization")); + assertEquals("", requestHeaders.get("cookie")); + assertEquals("", responseHeaders.get("set-cookie")); assertEquals("", exchange.path("response").path("body").asText()); } @@ -161,8 +188,13 @@ private static MembraneMCPServer readyServer() throws Exception { } private static void readyHandshake(MembraneMCPServer server) throws Exception { - invoke(server, initializeRequest("2025-03-26")); - invoke(server, notification(MCPInitialized.METHOD, Map.of())); + Exchange initialize = invoke(server, initializeRequest("2025-03-26")); + assertEquals(200, initialize.getResponse().getStatusCode()); + assertEquals(MembraneMCPServer.SUPPORTED_PROTOCOL_VERSION, responseJson(initialize).path("result").path("protocolVersion").asText()); + + Exchange initialized = invoke(server, notification(MCPInitialized.METHOD, Map.of())); + assertEquals(202, initialized.getResponse().getStatusCode()); + assertEquals("", initialized.getResponse().getBodyAsStringDecoded()); } private static JSONRPCRequest initializeRequest(String protocolVersion) { @@ -187,6 +219,7 @@ private static JSONRPCRequest notification(String method, Map pa private static Exchange invoke(MembraneMCPServer server, JSONRPCRequest request) throws Exception { Exchange exc = Request.post("http://localhost/mcp") + .header("Accept", POST_ACCEPT_HEADER) .json(request.toJson()) .buildExchange(); @@ -207,6 +240,23 @@ private static JsonNode toolPayload(Exchange exc) throws Exception { return OM.readTree(textNode.asText()); } + private static JsonNode findToolByName(JsonNode tools, String name) { + for (JsonNode tool : tools) { + if (name.equals(tool.path("name").asText())) { + return tool; + } + } + return null; + } + + private static Map normalizeHeaders(JsonNode headersNode) { + Map headers = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); + for (Map.Entry entry : headersNode.properties()) { + headers.put(entry.getKey().toLowerCase(), entry.getValue().asText()); + } + return headers; + } + private static Exchange sampleExchange() throws URISyntaxException { Exchange exc = new Exchange(null); exc.setRequest(Request.post("http://localhost/internal") From 32803b350b12fb5046849cf096fd53d49af34066 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Mon, 4 May 2026 13:49:35 +0200 Subject: [PATCH 27/29] Update `toString` implementation in `MCPToolsCall` to display argument keys instead of full argument details. --- .../main/java/com/predic8/membrane/core/mcp/MCPToolsCall.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsCall.java b/core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsCall.java index 5bbf8b1a3a..1724b5022e 100644 --- a/core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsCall.java +++ b/core/src/main/java/com/predic8/membrane/core/mcp/MCPToolsCall.java @@ -95,6 +95,6 @@ public boolean hasArguments() { @Override public String toString() { - return "MCPToolsCall{id=" + getId() + ", name='" + name + "', arguments=" + arguments + "}"; + return "MCPToolsCall{id=" + getId() + ", name='" + name + "', argumentKeys=" + arguments.keySet() + "}"; } } From 69e6fb676926a650337f6f293dca283d8a53e840 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Mon, 4 May 2026 13:57:27 +0200 Subject: [PATCH 28/29] Extract utility methods from `MembraneMCPServer` to new `MCPUtil` class for improved modularity and code reuse. --- .../core/interceptor/mcp/MCPUtil.java | 114 +++++++++++++++++ .../interceptor/mcp/MembraneMCPServer.java | 117 ++---------------- 2 files changed, 125 insertions(+), 106 deletions(-) create mode 100644 core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MCPUtil.java diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MCPUtil.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MCPUtil.java new file mode 100644 index 0000000000..68b324a9e8 --- /dev/null +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MCPUtil.java @@ -0,0 +1,114 @@ +package com.predic8.membrane.core.interceptor.mcp; + +import com.predic8.membrane.core.exchange.AbstractExchange; +import com.predic8.membrane.core.mcp.MCPToolsCall; +import com.predic8.membrane.core.openapi.serviceproxy.APIProxy; +import com.predic8.membrane.core.proxies.Proxy; +import com.predic8.membrane.core.proxies.SOAPProxy; +import com.predic8.membrane.core.proxies.ServiceProxy; +import org.jetbrains.annotations.Nullable; + +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Set; + +public final class MCPUtil { + + private MCPUtil() { + } + + public static Map describeProxy(Proxy proxy, Object statistics) { + var description = new LinkedHashMap(); + description.put("name", proxy.getName()); + + String type; + switch (proxy) { + case APIProxy ignored -> type = "API"; + case SOAPProxy soapProxy -> { + type = "soapProxy"; + description.put("wsdl", soapProxy.getWsdl()); + description.put("serviceName", soapProxy.getServiceName()); + } + case ServiceProxy ignored -> type = "serviceProxy"; + default -> type = "unknown"; + } + + description.put("type", type); + description.put("rule", proxy.getKey().toString()); + description.put("interceptors", proxy.getFlow().stream() + .map(interceptor -> Map.of("name", interceptor.getDisplayName())) + .toList()); + description.put("statistics", statistics); + return description; + } + + public static @Nullable Map describeExchange(AbstractExchange exchange, boolean includeBodies, McpPayloadSanitizer payloadSanitizer) { + if (exchange.getResponse() == null) { + return null; + } + + var description = new LinkedHashMap(); + description.put("id", exchange.getId()); + + var request = new LinkedHashMap(); + request.put("method", exchange.getRequest().getMethod()); + request.put("path", exchange.getRequest().getUri()); + request.put("headers", payloadSanitizer.sanitizeHeaders(exchange.getRequest().getHeader())); + if (includeBodies) { + request.put("body", payloadSanitizer.sanitizeBody(exchange.getRequest())); + } + + var response = new LinkedHashMap(); + response.put("status", exchange.getResponse().getStatusCode()); + response.put("headers", payloadSanitizer.sanitizeHeaders(exchange.getResponse().getHeader())); + if (includeBodies) { + response.put("body", payloadSanitizer.sanitizeBody(exchange.getResponse())); + } + + description.put("request", request); + description.put("response", response); + return description; + } + + public static int getOptionalIntArgument(MCPToolsCall call, String name, int defaultValue, int minimum, int maximum) { + Object value = call.getArgument(name); + if (value == null) { + return defaultValue; + } + if (!(value instanceof Number number) || number.doubleValue() != Math.rint(number.doubleValue())) { + throw new InvalidToolArgumentsException("Tool argument '" + name + "' must be an integer"); + } + int parsed = number.intValue(); + if (parsed < minimum || parsed > maximum) { + throw new InvalidToolArgumentsException( + "Tool argument '" + name + "' must be between " + minimum + " and " + maximum + ); + } + return parsed; + } + + public static boolean getOptionalBooleanArgument(MCPToolsCall call, String name, boolean defaultValue) { + Object value = call.getArgument(name); + if (value == null) { + return defaultValue; + } + if (value instanceof Boolean bool) { + return bool; + } + throw new InvalidToolArgumentsException("Tool argument '" + name + "' must be a boolean"); + } + + public static void rejectUnexpectedArguments(MCPToolsCall call, Set allowed) { + for (String argumentName : call.getArguments().keySet()) { + if (!allowed.contains(argumentName)) { + throw new InvalidToolArgumentsException("Unexpected tool argument: " + argumentName); + } + } + } + + public static final class InvalidToolArgumentsException extends IllegalArgumentException { + private InvalidToolArgumentsException(String message) { + super(message); + } + } +} diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java index 2c0cfd85b9..a4cf2f7ff1 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java @@ -7,13 +7,10 @@ import com.predic8.membrane.core.http.Response; import com.predic8.membrane.core.interceptor.AbstractInterceptor; import com.predic8.membrane.core.interceptor.Outcome; +import com.predic8.membrane.core.interceptor.mcp.MCPUtil.InvalidToolArgumentsException; import com.predic8.membrane.core.jsonrpc.JSONRPCRequest; import com.predic8.membrane.core.jsonrpc.JSONRPCResponse; import com.predic8.membrane.core.mcp.*; -import com.predic8.membrane.core.openapi.serviceproxy.APIProxy; -import com.predic8.membrane.core.proxies.Proxy; -import com.predic8.membrane.core.proxies.SOAPProxy; -import com.predic8.membrane.core.proxies.ServiceProxy; import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -201,27 +198,30 @@ private McpToolRegistry buildToolRegistry() { } private MCPToolsCallResponse listProxies(MCPToolsCall call, Exchange exc) { - rejectUnexpectedArguments(call, Set.of()); + MCPUtil.rejectUnexpectedArguments(call, Set.of()); return MCPToolsCallResponse.from(call) .withJson(Map.of( "proxies", getRouter().getRuleManager().getRules().stream() - .map(this::getProxyDescription) + .map(proxy -> MCPUtil.describeProxy( + proxy, + getRouter().getExchangeStore().getStatistics(proxy.getKey()) + )) .toList() )); } private MCPToolsCallResponse getStatistics(MCPToolsCall call, Exchange exc) { - rejectUnexpectedArguments(call, Set.of()); + MCPUtil.rejectUnexpectedArguments(call, Set.of()); return MCPToolsCallResponse.from(call) .withJson(getRouter().getStatistics()); } private MCPToolsCallResponse getExchanges(MCPToolsCall call, Exchange exc) { - rejectUnexpectedArguments(call, Set.of("limit", "includeBodies")); + MCPUtil.rejectUnexpectedArguments(call, Set.of("limit", "includeBodies")); - int limit = getOptionalIntArgument(call, "limit", MAX_EXCHANGES, 1, MAX_EXCHANGES); - boolean includeBodies = getOptionalBooleanArgument(call, "includeBodies", false); + int limit = MCPUtil.getOptionalIntArgument(call, "limit", MAX_EXCHANGES, 1, MAX_EXCHANGES); + boolean includeBodies = MCPUtil.getOptionalBooleanArgument(call, "includeBodies", false); List exchanges = Optional.ofNullable(getRouter().getExchangeStore().getAllExchangesAsList()) .orElse(List.of()); @@ -231,101 +231,12 @@ private MCPToolsCallResponse getExchanges(MCPToolsCall call, Exchange exc) { .withJson(Map.of( "exchanges", exchanges.subList(start, exchanges.size()).stream() - .map(exchange -> describeExchange(exchange, includeBodies)) + .map(exchange -> MCPUtil.describeExchange(exchange, includeBodies, payloadSanitizer)) .filter(Objects::nonNull) .toList() )); } - private @Nullable Map describeExchange(AbstractExchange exchange, boolean includeBodies) { - if (exchange.getResponse() == null) { - return null; - } - - var description = new LinkedHashMap(); - description.put("id", exchange.getId()); - - var request = new LinkedHashMap(); - request.put("method", exchange.getRequest().getMethod()); - request.put("path", exchange.getRequest().getUri()); - request.put("headers", payloadSanitizer.sanitizeHeaders(exchange.getRequest().getHeader())); - if (includeBodies) { - request.put("body", payloadSanitizer.sanitizeBody(exchange.getRequest())); - } - - var response = new LinkedHashMap(); - response.put("status", exchange.getResponse().getStatusCode()); - response.put("headers", payloadSanitizer.sanitizeHeaders(exchange.getResponse().getHeader())); - if (includeBodies) { - response.put("body", payloadSanitizer.sanitizeBody(exchange.getResponse())); - } - - description.put("request", request); - description.put("response", response); - return description; - } - - private Map getProxyDescription(Proxy proxy) { - var description = new LinkedHashMap(); - description.put("name", proxy.getName()); - - String type; - switch (proxy) { - case APIProxy ignored -> type = "API"; - case SOAPProxy soapProxy -> { - type = "soapProxy"; - description.put("wsdl", soapProxy.getWsdl()); - description.put("serviceName", soapProxy.getServiceName()); - } - case ServiceProxy ignored -> type = "serviceProxy"; - default -> type = "unknown"; - } - - description.put("type", type); - description.put("rule", proxy.getKey().toString()); - description.put("interceptors", proxy.getFlow().stream() - .map(interceptor -> Map.of("name", interceptor.getDisplayName())) - .toList()); - description.put("statistics", getRouter().getExchangeStore().getStatistics(proxy.getKey())); - return description; - } - - private static int getOptionalIntArgument(MCPToolsCall call, String name, int defaultValue, int minimum, int maximum) { - Object value = call.getArgument(name); - if (value == null) { - return defaultValue; - } - if (!(value instanceof Number number) || number.doubleValue() != Math.rint(number.doubleValue())) { - throw new InvalidToolArgumentsException("Tool argument '" + name + "' must be an integer"); - } - int parsed = number.intValue(); - if (parsed < minimum || parsed > maximum) { - throw new InvalidToolArgumentsException( - "Tool argument '" + name + "' must be between " + minimum + " and " + maximum - ); - } - return parsed; - } - - private static boolean getOptionalBooleanArgument(MCPToolsCall call, String name, boolean defaultValue) { - Object value = call.getArgument(name); - if (value == null) { - return defaultValue; - } - if (value instanceof Boolean bool) { - return bool; - } - throw new InvalidToolArgumentsException("Tool argument '" + name + "' must be a boolean"); - } - - private static void rejectUnexpectedArguments(MCPToolsCall call, Set allowed) { - for (String argumentName : call.getArguments().keySet()) { - if (!allowed.contains(argumentName)) { - throw new InvalidToolArgumentsException("Unexpected tool argument: " + argumentName); - } - } - } - private McpHttpResult badRequest(Exchange exc, @Nullable JSONRPCRequest request, int code, String message, Exception exception) { log.info("Rejected MCP request {} {}: {}", exc.getRequest().getMethod(), exc.getRequest().getUri(), exception.getMessage()); return new McpHttpResult(400, error(responseId(request), code, message, exception.getMessage())); @@ -392,10 +303,4 @@ record McpHttpResult( @Nullable JSONRPCResponse body ) { } - - private static final class InvalidToolArgumentsException extends IllegalArgumentException { - private InvalidToolArgumentsException(String message) { - super(message); - } - } } From a911b22b2f362d09525d69891a0976a5dcacf7bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6rdes?= Date: Mon, 4 May 2026 16:22:17 +0200 Subject: [PATCH 29/29] Introduce session management in `MembraneMCPServer`: add `SESSION_HEADER` handling, session-based request processing, and enhanced error handling. Update corresponding unit tests for validation. --- .../interceptor/mcp/MembraneMCPServer.java | 102 ++++++++++++++---- .../mcp/MembraneMCPServerTest.java | 85 +++++++++++---- 2 files changed, 146 insertions(+), 41 deletions(-) diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java index a4cf2f7ff1..f249b6c20f 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServer.java @@ -17,6 +17,7 @@ import java.io.IOException; import java.util.*; +import java.util.concurrent.ConcurrentHashMap; import static com.predic8.membrane.annot.Constants.VERSION; import static com.predic8.membrane.core.http.MimeType.APPLICATION_JSON; @@ -40,6 +41,7 @@ public class MembraneMCPServer extends AbstractInterceptor { static final String SUPPORTED_PROTOCOL_VERSION = "2025-03-26"; + static final String SESSION_HEADER = "Mcp-Session-Id"; private static final Logger log = LoggerFactory.getLogger(MembraneMCPServer.class); @@ -59,7 +61,7 @@ public class MembraneMCPServer extends AbstractInterceptor { "additionalProperties", false ); - private final McpSessionContext sessionContext = new McpSessionContext(); + private final Map sessionContexts = new ConcurrentHashMap<>(); private final McpPayloadSanitizer payloadSanitizer = new McpPayloadSanitizer(); private final McpToolRegistry toolRegistry = buildToolRegistry(); @@ -101,30 +103,41 @@ private Response handleHttpRequest(Exchange exc) throws IOException { private McpHttpResult processMcpRequest(JSONRPCRequest request, Exchange exc) throws Exception { return switch (request.getMethod()) { - case MCPInitialize.METHOD -> processInitialize(request); - case MCPInitialized.METHOD -> processInitializedNotification(request); - case MCPPing.METHOD -> processPing(request); - case MCPToolsList.METHOD -> processToolsList(request); + case MCPInitialize.METHOD -> processInitialize(request, exc); + case MCPInitialized.METHOD -> processInitializedNotification(request, exc); + case MCPPing.METHOD -> processPing(request, exc); + case MCPToolsList.METHOD -> processToolsList(request, exc); case MCPToolsCall.METHOD -> processToolsCall(request, exc); default -> protocolError(request, ERR_METHOD_NOT_FOUND, "Method not found"); }; } - private McpHttpResult processInitialize(JSONRPCRequest request) { + private McpHttpResult processInitialize(JSONRPCRequest request, Exchange exc) { MCPInitialize initialize = MCPInitialize.from(request); + if (getSessionId(exc) != null) { + return protocolError(request, ERR_INVALID_REQUEST, "'initialize' must not include '" + SESSION_HEADER + "'"); + } + + String sessionId = UUID.randomUUID().toString(); + McpSessionContext sessionContext = new McpSessionContext(); if (!sessionContext.initialize(SUPPORTED_PROTOCOL_VERSION, initialize.getClientInfo())) { return protocolError(request, ERR_INVALID_REQUEST, "'initialize' must be the first MCP request"); } + sessionContexts.put(sessionId, sessionContext); return httpOk(new MCPInitializeResponse(initialize) .withProtocolVersion(SUPPORTED_PROTOCOL_VERSION) .withCapabilities(getCapabilities()) .withServerInfo("Membrane", VERSION) - .toRpcResponse()); + .toRpcResponse(), Map.of(SESSION_HEADER, sessionId)); } - private McpHttpResult processInitializedNotification(JSONRPCRequest request) { + private McpHttpResult processInitializedNotification(JSONRPCRequest request, Exchange exc) { MCPInitialized.from(request); + McpSessionContext sessionContext = requireSession(request, exc); + if (sessionContext == null) { + return missingOrInvalidSession(request, exc); + } if (!sessionContext.markReady()) { return protocolError( request, @@ -136,16 +149,24 @@ private McpHttpResult processInitializedNotification(JSONRPCRequest request) { return acceptedNotification(); } - private McpHttpResult processPing(JSONRPCRequest request) { + private McpHttpResult processPing(JSONRPCRequest request, Exchange exc) { MCPPing.from(request); + McpSessionContext sessionContext = requireSession(request, exc); + if (sessionContext == null) { + return missingOrInvalidSession(request, exc); + } if (!sessionContext.isIn(INITIALIZED, READY)) { return protocolError(request, ERR_INVALID_REQUEST, "'ping' is only allowed after 'initialize'"); } return httpOk(success(request.getId(), Map.of())); } - private McpHttpResult processToolsList(JSONRPCRequest request) { + private McpHttpResult processToolsList(JSONRPCRequest request, Exchange exc) { MCPToolsList toolsList = MCPToolsList.from(request); + McpSessionContext sessionContext = requireSession(request, exc); + if (sessionContext == null) { + return missingOrInvalidSession(request, exc); + } if (!sessionContext.isIn(READY)) { return protocolError(request, ERR_INVALID_REQUEST, "'tools/list' requires a completed MCP handshake"); } @@ -157,6 +178,10 @@ private McpHttpResult processToolsList(JSONRPCRequest request) { private McpHttpResult processToolsCall(JSONRPCRequest request, Exchange exc) throws Exception { MCPToolsCall call = MCPToolsCall.from(request); + McpSessionContext sessionContext = requireSession(request, exc); + if (sessionContext == null) { + return missingOrInvalidSession(request, exc); + } if (!sessionContext.isIn(READY)) { return protocolError(request, ERR_INVALID_REQUEST, "'tools/call' requires a completed MCP handshake"); } @@ -239,14 +264,15 @@ private MCPToolsCallResponse getExchanges(MCPToolsCall call, Exchange exc) { private McpHttpResult badRequest(Exchange exc, @Nullable JSONRPCRequest request, int code, String message, Exception exception) { log.info("Rejected MCP request {} {}: {}", exc.getRequest().getMethod(), exc.getRequest().getUri(), exception.getMessage()); - return new McpHttpResult(400, error(responseId(request), code, message, exception.getMessage())); + return new McpHttpResult(400, error(responseId(request), code, message, exception.getMessage()), Map.of()); } private McpHttpResult internalError(Exchange exc, @Nullable JSONRPCRequest request, Exception exception) { log.warn("Failed to handle MCP request {} {}.", exc.getRequest().getMethod(), exc.getRequest().getUri(), exception); return new McpHttpResult( request != null && request.isNotification() ? 500 : 200, - error(responseId(request), ERR_INTERNAL_ERROR, "Internal error", exception.getMessage()) + error(responseId(request), ERR_INTERNAL_ERROR, "Internal error", exception.getMessage()), + Map.of() ); } @@ -257,10 +283,39 @@ private McpHttpResult protocolError(JSONRPCRequest request, int code, String mes private McpHttpResult protocolError(JSONRPCRequest request, int code, String message, @Nullable Object data) { return new McpHttpResult( request.isNotification() ? 400 : 200, - data == null ? error(responseId(request), code, message) : error(responseId(request), code, message, data) + data == null ? error(responseId(request), code, message) : error(responseId(request), code, message, data), + Map.of() + ); + } + + private @Nullable McpSessionContext requireSession(JSONRPCRequest request, Exchange exc) { + String sessionId = getSessionId(exc); + if (sessionId == null) { + return null; + } + return sessionContexts.get(sessionId); + } + + private McpHttpResult missingOrInvalidSession(JSONRPCRequest request, Exchange exc) { + String sessionId = getSessionId(exc); + if (sessionId == null) { + return new McpHttpResult( + 400, + error(responseId(request), ERR_INVALID_REQUEST, "'" + SESSION_HEADER + "' header is required"), + Map.of() + ); + } + return new McpHttpResult( + 404, + error(responseId(request), ERR_INVALID_REQUEST, "Unknown MCP session"), + Map.of() ); } + private @Nullable String getSessionId(Exchange exc) { + return exc.getRequest().getHeader().getFirstValue(SESSION_HEADER); + } + private static Object responseId(@Nullable JSONRPCRequest request) { if (request == null || request.isNotification()) { return null; @@ -269,11 +324,15 @@ private static Object responseId(@Nullable JSONRPCRequest request) { } private static McpHttpResult httpOk(JSONRPCResponse response) { - return new McpHttpResult(200, response); + return httpOk(response, Map.of()); + } + + private static McpHttpResult httpOk(JSONRPCResponse response, Map headers) { + return new McpHttpResult(200, response, headers); } private static McpHttpResult acceptedNotification() { - return new McpHttpResult(202, null); + return new McpHttpResult(202, null, Map.of()); } private static Response createResponse(McpHttpResult result) throws IOException { @@ -283,10 +342,12 @@ private static Response createResponse(McpHttpResult result) throws IOException } return statusCode(result.status()).bodyEmpty().build(); } - return statusCode(result.status()) - .contentType(APPLICATION_JSON) - .body(result.body().toJson()) - .build(); + Response.ResponseBuilder builder = statusCode(result.status()) + .contentType(APPLICATION_JSON); + for (Map.Entry header : result.headers().entrySet()) { + builder.header(header.getKey(), header.getValue()); + } + return builder.body(result.body().toJson()).build(); } private static Map getCapabilities() { @@ -300,7 +361,8 @@ public String getDisplayName() { record McpHttpResult( int status, - @Nullable JSONRPCResponse body + @Nullable JSONRPCResponse body, + Map headers ) { } } diff --git a/core/src/test/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServerTest.java b/core/src/test/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServerTest.java index e0692ca2ef..33fa0184f4 100644 --- a/core/src/test/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServerTest.java +++ b/core/src/test/java/com/predic8/membrane/core/interceptor/mcp/MembraneMCPServerTest.java @@ -30,7 +30,7 @@ class MembraneMCPServerTest { void toolsListBeforeInitializeIsRejected() throws Exception { Exchange exc = invoke(newServer(), request(1, MCPToolsList.METHOD, Map.of())); - assertEquals(200, exc.getResponse().getStatusCode()); + assertEquals(400, exc.getResponse().getStatusCode()); assertEquals(-32600, responseJson(exc).path("error").path("code").asInt()); } @@ -48,7 +48,7 @@ void initializedNotificationBeforeInitializeIsRejected() throws Exception { void pingBeforeInitializeIsRejected() throws Exception { Exchange exc = invoke(newServer(), request(1, MCPPing.METHOD, Map.of())); - assertEquals(200, exc.getResponse().getStatusCode()); + assertEquals(400, exc.getResponse().getStatusCode()); assertEquals(-32600, responseJson(exc).path("error").path("code").asInt()); } @@ -59,8 +59,9 @@ void toolsListAfterInitializeButBeforeInitializedIsRejected() throws Exception { Exchange initialize = invoke(server, initializeRequest("2025-03-26")); assertEquals(200, initialize.getResponse().getStatusCode()); assertEquals(MembraneMCPServer.SUPPORTED_PROTOCOL_VERSION, responseJson(initialize).path("result").path("protocolVersion").asText()); + String sessionId = sessionId(initialize); - Exchange toolsList = invoke(server, request(2, MCPToolsList.METHOD, Map.of())); + Exchange toolsList = invoke(server, request(2, MCPToolsList.METHOD, Map.of()), sessionId); assertEquals(200, toolsList.getResponse().getStatusCode()); assertEquals(-32600, responseJson(toolsList).path("error").path("code").asInt()); } @@ -73,17 +74,18 @@ void initializePingInitializedAndToolsListFollowTheLifecycle() throws Exception assertEquals(200, initialize.getResponse().getStatusCode()); JsonNode initializeJson = responseJson(initialize); assertEquals(MembraneMCPServer.SUPPORTED_PROTOCOL_VERSION, initializeJson.path("result").path("protocolVersion").asText()); + String sessionId = sessionId(initialize); - Exchange ping = invoke(server, request(2, MCPPing.METHOD, Map.of())); + Exchange ping = invoke(server, request(2, MCPPing.METHOD, Map.of()), sessionId); assertEquals(200, ping.getResponse().getStatusCode()); assertTrue(responseJson(ping).path("result").isObject()); assertEquals(0, responseJson(ping).path("result").size()); - Exchange initialized = invoke(server, notification(MCPInitialized.METHOD, Map.of())); + Exchange initialized = invoke(server, notification(MCPInitialized.METHOD, Map.of()), sessionId); assertEquals(202, initialized.getResponse().getStatusCode()); assertEquals("", initialized.getResponse().getBodyAsStringDecoded()); - Exchange toolsList = invoke(server, request(3, MCPToolsList.METHOD, Map.of())); + Exchange toolsList = invoke(server, request(3, MCPToolsList.METHOD, Map.of()), sessionId); JsonNode tools = responseJson(toolsList).path("result").path("tools"); JsonNode getExchangesTool = findToolByName(tools, "getExchanges"); @@ -94,15 +96,39 @@ void initializePingInitializedAndToolsListFollowTheLifecycle() throws Exception assertFalse(getExchangesTool.path("inputSchema").path("additionalProperties").asBoolean(true)); } + @Test + void initializeCreatesIndependentSessions() throws Exception { + MembraneMCPServer server = newServer(); + + Exchange firstInitialize = invoke(server, initializeRequest("2025-03-26")); + Exchange secondInitialize = invoke(server, initializeRequest("2025-03-26")); + + assertEquals(200, firstInitialize.getResponse().getStatusCode()); + assertEquals(200, secondInitialize.getResponse().getStatusCode()); + assertNotEquals(sessionId(firstInitialize), sessionId(secondInitialize)); + } + + @Test + void toolsListWithoutSessionHeaderIsRejectedEvenAfterInitialize() throws Exception { + MembraneMCPServer server = newServer(); + + Exchange initialize = invoke(server, initializeRequest("2025-03-26")); + assertEquals(200, initialize.getResponse().getStatusCode()); + + Exchange toolsList = invoke(server, request(2, MCPToolsList.METHOD, Map.of())); + assertEquals(400, toolsList.getResponse().getStatusCode()); + assertEquals(-32600, responseJson(toolsList).path("error").path("code").asInt()); + } + @Test void unknownToolReturnsJsonRpcError() throws Exception { - MembraneMCPServer server = readyServer(); + ReadyServer server = readyServer(); - Exchange exc = invoke(server, request( + Exchange exc = invoke(server.server(), request( 2, MCPToolsCall.METHOD, Map.of("name", "doesNotExist", "arguments", Map.of()) - )); + ), server.sessionId()); assertEquals(200, exc.getResponse().getStatusCode()); assertEquals(-32602, responseJson(exc).path("error").path("code").asInt()); @@ -110,13 +136,13 @@ void unknownToolReturnsJsonRpcError() throws Exception { @Test void invalidToolArgumentsReturnJsonRpcError() throws Exception { - MembraneMCPServer server = readyServer(); + ReadyServer server = readyServer(); - Exchange exc = invoke(server, request( + Exchange exc = invoke(server.server(), request( 2, MCPToolsCall.METHOD, Map.of("name", "getExchanges", "arguments", Map.of("limit", 0)) - )); + ), server.sessionId()); assertEquals(200, exc.getResponse().getStatusCode()); JsonNode response = responseJson(exc); @@ -148,13 +174,13 @@ void unsupportedProtocolVersionNegotiatesServerVersion() throws Exception { @Test void getExchangesRedactsSensitiveHeadersAndOmitsBinaryBodies() throws Exception { MembraneMCPServer server = newServer(new StubExchangeStore(List.of(sampleExchange()))); - readyHandshake(server); + String sessionId = readyHandshake(server); Exchange exc = invoke(server, request( 2, MCPToolsCall.METHOD, Map.of("name", "getExchanges", "arguments", Map.of("limit", 1, "includeBodies", true)) - )); + ), sessionId); JsonNode payload = toolPayload(exc); JsonNode exchange = payload.path("exchanges").get(0); @@ -181,20 +207,22 @@ private static MembraneMCPServer newServer(ForgetfulExchangeStore exchangeStore) return server; } - private static MembraneMCPServer readyServer() throws Exception { + private static ReadyServer readyServer() throws Exception { MembraneMCPServer server = newServer(); - readyHandshake(server); - return server; + return new ReadyServer(server, readyHandshake(server)); } - private static void readyHandshake(MembraneMCPServer server) throws Exception { + private static String readyHandshake(MembraneMCPServer server) throws Exception { Exchange initialize = invoke(server, initializeRequest("2025-03-26")); assertEquals(200, initialize.getResponse().getStatusCode()); assertEquals(MembraneMCPServer.SUPPORTED_PROTOCOL_VERSION, responseJson(initialize).path("result").path("protocolVersion").asText()); + String sessionId = sessionId(initialize); + assertNotNull(sessionId); - Exchange initialized = invoke(server, notification(MCPInitialized.METHOD, Map.of())); + Exchange initialized = invoke(server, notification(MCPInitialized.METHOD, Map.of()), sessionId); assertEquals(202, initialized.getResponse().getStatusCode()); assertEquals("", initialized.getResponse().getBodyAsStringDecoded()); + return sessionId; } private static JSONRPCRequest initializeRequest(String protocolVersion) { @@ -218,8 +246,16 @@ private static JSONRPCRequest notification(String method, Map pa } private static Exchange invoke(MembraneMCPServer server, JSONRPCRequest request) throws Exception { - Exchange exc = Request.post("http://localhost/mcp") - .header("Accept", POST_ACCEPT_HEADER) + return invoke(server, request, null); + } + + private static Exchange invoke(MembraneMCPServer server, JSONRPCRequest request, String sessionId) throws Exception { + Request.Builder builder = Request.post("http://localhost/mcp") + .header("Accept", POST_ACCEPT_HEADER); + if (sessionId != null) { + builder.header(MembraneMCPServer.SESSION_HEADER, sessionId); + } + Exchange exc = builder .json(request.toJson()) .buildExchange(); @@ -227,6 +263,10 @@ private static Exchange invoke(MembraneMCPServer server, JSONRPCRequest request) return exc; } + private static String sessionId(Exchange exc) { + return exc.getResponse().getHeader().getFirstValue(MembraneMCPServer.SESSION_HEADER); + } + private static JsonNode responseJson(Exchange exc) throws Exception { return OM.readTree(exc.getResponse().getBodyAsStringDecoded()); } @@ -285,4 +325,7 @@ public List getAllExchangesAsList() { return exchanges; } } + + private record ReadyServer(MembraneMCPServer server, String sessionId) { + } }