Fix/mcp sse accept routing

include/MCPServer.h

@@ -284,14 +284,16 @@ class MCPServer {
     /* Starts the HTTP listener. Register all tools first: doing so afterwards
      * would race the request handlers against mutations of the tool registry.
      * Call only once WiFi is connected — setupMDNS() reads WiFi.localIP() to
-     * publish the endpoint TXT record. Returns false if the server could not
-     * be created; a worker-task failure is not fatal (tool calls then run
-     * inline) and still returns true. */
+     * publish the endpoint TXT record. Returns false if the server or its
+     * endpoint handler could not be created; a worker-task failure is not fatal
+     * (tool calls then run inline) and still returns true. */
     bool begin();
 
 private:
-    void setupWebServer();
+    // False only when the endpoint handler could not be allocated.
+    bool setupWebServer();
     void setupMDNS();
+    void handleEndpointRequest(AsyncWebServerRequest* request);
     void handlePostComplete(AsyncWebServerRequest* request);
     void handleJsonBody(AsyncWebServerRequest* request, const char* body);
     void deferToolCall(AsyncWebServerRequest* request, MCPRequest&& mcpRequest);

library.json

@@ -1,6 +1,6 @@
 {
     "name": "ESP32-MCPServer",
-    "version": "0.2.0",
+    "version": "0.2.1",
     "description": "Model Context Protocol (MCP) Server library for ESP32. Implements MCP server functionality over HTTP/JSON-RPC, with tool calls executed on a worker task so blocking handlers never stall the async TCP task.",
     "keywords": "mcp, server, esp32, async, llm, ai",
     "repository": {

src/MCPServer.cpp

@@ -9,6 +9,7 @@
 #include <cctype>
 #include <cstdlib>
 #include <cstring>
+#include <functional>
 #include <new>
 #include <utility>
 
@@ -92,6 +93,52 @@ class JobRef {
     HttpToolJob* job_;
 };
 
+/* Routes the MCP endpoint by path alone.
+ *
+ * server->on() cannot be used here. MCP's Streamable HTTP transport requires
+ * every client POST to carry `Accept: application/json, text/event-stream`, and
+ * ESPAsyncWebServer reads any Accept containing text/event-stream as an SSE
+ * subscription: it marks the request RCT_EVENT, after which
+ * AsyncCallbackWebHandler::canHandle() declines it (that check insists on
+ * isHTTP()) and the request falls through to the catch-all. The result is a 404
+ * for every spec-conformant MCP client, on both POST and the GET stream probe,
+ * while a hand-written curl without the header works fine.
+ *
+ * Matching on the path and dispatching on the method ourselves keeps the
+ * connection type out of the routing decision. Nothing downstream consults it:
+ * RCT_EVENT only ever gates handler selection. */
+class MCPEndpointHandler : public AsyncWebHandler {
+public:
+    using RequestFn = std::function<void(AsyncWebServerRequest*)>;
+    using BodyFn = std::function<void(AsyncWebServerRequest*, uint8_t*, size_t, size_t, size_t)>;
+
+    MCPEndpointHandler(const char* uri, RequestFn onRequest, BodyFn onBody)
+        : uri_(uri), onRequest_(std::move(onRequest)), onBody_(std::move(onBody)) {}
+
+    bool canHandle(AsyncWebServerRequest* request) const override {
+        return request->url() == uri_;
+    }
+
+    void handleRequest(AsyncWebServerRequest* request) override {
+        onRequest_(request);
+    }
+
+    void handleBody(AsyncWebServerRequest* request, uint8_t* data, size_t len, size_t index,
+                    size_t total) override {
+        onBody_(request, data, len, index, total);
+    }
+
+    // A "trivial" handler never has the request body delivered to it.
+    bool isRequestHandlerTrivial() const override {
+        return false;
+    }
+
+private:
+    String uri_;
+    RequestFn onRequest_;
+    BodyFn onBody_;
+};
+
 /* Appending sink for serializeJson. ArduinoJson's std::string writer clears the
  * destination in its constructor, so emitting several documents into one buffer
  * needs a writer that only ever appends; the default Writer template picks this
@@ -145,7 +192,10 @@ bool MCPServer::begin() {
 
     // Best-effort: on failure tools/call degrades to inline execution.
     startWorker();
-    setupWebServer();
+    if (!setupWebServer()) {
+        stopWorker();  // nothing will ever feed it
+        return false;
+    }
     setupMDNS();
     started = true;
     return true;
@@ -241,12 +291,12 @@ void MCPServer::workerEntry(void* ctx) {
 // HTTP transport
 // ---------------------------------------------------------------------------
 
-void MCPServer::setupWebServer() {
+bool MCPServer::setupWebServer() {
     /* The body callback only accumulates; the response is always sent from the
-     * onRequest callback, which runs once the request is complete — including
+     * request callback, which runs once the request is complete — including
      * when there is no body at all. */
-    server->on(
-        "/mcp", HTTP_POST, [this](AsyncWebServerRequest* request) { handlePostComplete(request); }, NULL,
+    auto* endpoint = new (std::nothrow) MCPEndpointHandler(
+        "/mcp", [this](AsyncWebServerRequest* request) { handleEndpointRequest(request); },
         [this](AsyncWebServerRequest* request, uint8_t* data, size_t len, size_t index, size_t total) {
             BodyBuffer* body = static_cast<BodyBuffer*>(request->_tempObject);
             if (!body) {
@@ -281,28 +331,10 @@ void MCPServer::setupWebServer() {
             memcpy(body->data + index, data, len);
             body->received = index + len;
         });
-
-    server->on("/mcp", HTTP_DELETE, [this](AsyncWebServerRequest* request) {
-        if (!validateOriginHeader(request)) {
-            sendJSONRPCError(request, 403, ErrorCode::INVALID_REQUEST, "Forbidden Origin");
-            return;
-        }
-        AsyncWebServerResponse* response =
-            request->beginResponse(405, "application/json", "{\"error\":\"Method Not Allowed\"}");
-        response->addHeader("Allow", "POST");
-        request->send(response);
-    });
-
-    server->on("/mcp", HTTP_GET, [this](AsyncWebServerRequest* request) {
-        if (!validateOriginHeader(request)) {
-            sendJSONRPCError(request, 403, ErrorCode::INVALID_REQUEST, "Forbidden Origin");
-            return;
-        }
-        AsyncWebServerResponse* response =
-            request->beginResponse(405, "application/json", "{\"error\":\"Method Not Allowed\"}");
-        response->addHeader("Allow", "POST");
-        request->send(response);
-    });
+    if (!endpoint) {
+        return false;
+    }
+    server->addHandler(endpoint);  // the server owns it from here
 
     server->onNotFound([this](AsyncWebServerRequest* request) {
         if (!validateOriginHeader(request)) {
@@ -313,6 +345,27 @@ void MCPServer::setupWebServer() {
     });
 
     server->begin();
+    return true;
+}
+
+/* Every method reaching /mcp lands here, since the endpoint handler matches on
+ * path alone. POST carries the JSON-RPC traffic; everything else — including the
+ * GET the MCP client uses to probe for a server-initiated SSE stream, which this
+ * server does not offer — is answered with the 405 the spec prescribes. */
+void MCPServer::handleEndpointRequest(AsyncWebServerRequest* request) {
+    if (request->method() == HTTP_POST) {
+        handlePostComplete(request);
+        return;
+    }
+
+    if (!validateOriginHeader(request)) {
+        sendJSONRPCError(request, 403, ErrorCode::INVALID_REQUEST, "Forbidden Origin");
+        return;
+    }
+    AsyncWebServerResponse* response =
+        request->beginResponse(405, "application/json", "{\"error\":\"Method Not Allowed\"}");
+    response->addHeader("Allow", "POST");
+    request->send(response);
 }
 
 std::string MCPServer::generateUUID() {

test/mock/ESPAsyncWebServer.h

@@ -7,8 +7,12 @@
  *    like the real library — so an aborted upload exercises the same teardown
  *    path (a C++ object stored there would leak / corrupt the heap).
  *  - Header lookup is case-insensitive.
- *  - Handlers are registered per (uri, method); tests fetch them via
- *    mock_async_web::lastServer() and drive body/request callbacks manually.
+ *  - The MCP endpoint is registered as an AsyncWebHandler that matches on path
+ *    alone (the real library would refuse a server->on() route for any request
+ *    whose Accept carries text/event-stream). findRoute() probes the registered
+ *    handlers with a synthetic request the same way AsyncWebServer::_attachHandler
+ *    does, so tests still drive body/request callbacks per (uri, method) via
+ *    mock_async_web::lastServer().
  *  - Chunked/deferred responses mirror AsyncChunkedResponse: send() keeps the
  *    response object alive and the filler is pulled via pumpChunked() (the
  *    test-side stand-in for the ack/poll cycle) until it returns 0; a filler
@@ -24,7 +28,9 @@
 #include <cstdlib>
 #include <cstring>
 #include <functional>
+#include <list>
 #include <map>
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -111,6 +117,16 @@ class AsyncWebServerRequest {
 
     uint8_t version() const { return version_; }
 
+    const String& url() const { return url_; }
+
+    int method() const { return method_; }
+
+    /* The real library downgrades any request whose Accept carries
+     * text/event-stream to RCT_EVENT, and AsyncCallbackWebHandler::canHandle()
+     * then declines it. MCP clients send exactly that header on every POST, so
+     * routing has to be exercised with it modelled. */
+    bool isHTTP() const { return !eventStreamAccept_; }
+
     AsyncWebServerResponse* beginResponse(int code, const String& contentType, const String& body) {
         return new AsyncWebServerResponse(code, contentType, body);
     }
@@ -172,6 +188,9 @@ class AsyncWebServerRequest {
 
     void setHeader(const char* name, const char* value) {
         headers_.emplace(lower(name), AsyncWebHeader(value));
+        if (lower(name) == "accept" && lower(value).find("text/event-stream") != std::string::npos) {
+            eventStreamAccept_ = true;
+        }
     }
 
     void setContentLength(size_t length) { contentLength_ = length; }
@@ -180,6 +199,10 @@ class AsyncWebServerRequest {
 
     void setVersion(uint8_t version) { version_ = version; }
 
+    void setUrl(const char* url) { url_ = url; }
+
+    void setMethod(int method) { method_ = method; }
+
     int responseCount = 0;
     int lastCode = -1;
     std::string lastContentType;
@@ -206,12 +229,38 @@ class AsyncWebServerRequest {
     std::map<std::string, AsyncWebHeader> headers_;
     size_t contentLength_ = 0;
     uint8_t version_ = 1;
+    String url_ = String("/mcp");
+    int method_ = HTTP_POST;
+    bool eventStreamAccept_ = false;
     String contentType_ = String("application/json");
     AsyncWebServerResponse* _pendingResponse = nullptr;
     std::string _chunkBody;
     size_t _chunkIndex = 0;
 };
 
+/* Mirrors the subset of the real base class that MCPServer overrides. The
+ * default canHandle() declines, matching the library. */
+class AsyncWebHandler {
+public:
+    virtual ~AsyncWebHandler() = default;
+
+    virtual bool canHandle(AsyncWebServerRequest* request) const {
+        (void)request;
+        return false;
+    }
+    virtual void handleRequest(AsyncWebServerRequest* request) { (void)request; }
+    virtual void handleBody(AsyncWebServerRequest* request, uint8_t* data, size_t len, size_t index,
+                            size_t total) {
+        (void)request;
+        (void)data;
+        (void)len;
+        (void)index;
+        (void)total;
+    }
+    // The real server skips body delivery entirely for a trivial handler.
+    virtual bool isRequestHandlerTrivial() const { return true; }
+};
+
 using ArRequestHandlerFunction = std::function<void(AsyncWebServerRequest*)>;
 using ArUploadHandlerFunction =
     std::function<void(AsyncWebServerRequest*, const String&, size_t, uint8_t*, size_t, bool)>;
@@ -257,21 +306,62 @@ class AsyncWebServer {
         routes.push_back({uri, method, std::move(onRequest), std::move(onBody)});
     }
 
+    // Takes ownership, like the real AsyncWebServer (a unique_ptr list).
+    AsyncWebHandler& addHandler(AsyncWebHandler* handler) {
+        handlers.emplace_back(handler);
+        return *handlers.back();
+    }
+
     void onNotFound(ArRequestHandlerFunction handler) { notFound = std::move(handler); }
 
     void begin() { started = true; }
 
-    const Route* findRoute(const char* uri, int method) const {
+    /* Resolves a request the way AsyncWebServer::_attachHandler does: explicit
+     * on() routes first — which, like AsyncCallbackWebHandler::canHandle(),
+     * decline anything that is not plain HTTP — then the registered handlers,
+     * which decide for themselves. A matched handler is surfaced as a
+     * forwarding Route so tests see one shape either way; a nullptr return is
+     * the catch-all (onNotFound) taking over. */
+    const Route* findRoute(AsyncWebServerRequest* request) const {
         for (const auto& route : routes) {
-            if (route.uri == uri && (route.method & method) != 0) {
+            if (request->isHTTP() && route.uri == request->url().c_str() &&
+                (route.method & request->method()) != 0) {
                 return &route;
             }
         }
+
+        for (const auto& handler : handlers) {
+            if (!handler->canHandle(request)) {
+                continue;
+            }
+            AsyncWebHandler* target = handler.get();
+            // std::list: the returned pointer must survive later lookups.
+            handlerRoutes_.push_back(
+                Route{request->url().c_str(), request->method(),
+                      [target](AsyncWebServerRequest* r) { target->handleRequest(r); },
+                      target->isRequestHandlerTrivial()
+                          ? ArBodyHandlerFunction()
+                          : [target](AsyncWebServerRequest* r, uint8_t* data, size_t len, size_t index,
+                                     size_t total) { target->handleBody(r, data, len, index, total); }});
+            return &handlerRoutes_.back();
+        }
         return nullptr;
     }
 
+    // Convenience for tests that only care about (uri, method) routing.
+    const Route* findRoute(const char* uri, int method) const {
+        AsyncWebServerRequest probe;
+        probe.setUrl(uri);
+        probe.setMethod(method);
+        return findRoute(&probe);
+    }
+
     uint16_t port;
     bool started = false;
     std::vector<Route> routes;
+    std::vector<std::unique_ptr<AsyncWebHandler>> handlers;
     ArRequestHandlerFunction notFound;
+
+private:
+    mutable std::list<Route> handlerRoutes_;
 };