Merge pull request #435 from koic/fix_result_null_for_id_bearing_notification

Fix an incorrect `result: null` response to an id-bearing notification message

Author: koic

lib/mcp/methods.rb

@@ -47,6 +47,10 @@ def initialize(method, capability)
     end
 
     class << self
+      def notification?(method)
+        method.is_a?(String) && method.start_with?("notifications/")
+      end
+
       def ensure_capability!(method, capabilities)
         case method
         when PROMPTS_GET, PROMPTS_LIST

lib/mcp/server.rb

@@ -430,6 +430,13 @@ def schema_contains_ref?(schema)
     end
 
     def handle_request(request, method, session: nil, related_request_id: nil)
+      # A well-formed notification carries no JSON-RPC id and receives no response.
+      # If a client erroneously sends a notification-only method with an id, the message
+      # is framed as a request; since notification methods have no request handler,
+      # returning `nil` here makes `JsonRpcHandler` report "Method not found", matching
+      # the TypeScript and Python SDKs rather than emitting a spurious `result: null`.
+      return if Methods.notification?(method) && !related_request_id.nil?
+
       # `notifications/cancelled` is dispatched directly: it is a notification (no JSON-RPC id)
       # and intentionally bypasses the `@handlers` lookup, capability check, in-flight registry,
       # and rescue blocks below.

test/mcp/server/transports/streamable_http_transport_test.rb

@@ -80,6 +80,38 @@ def string
           assert_equal({}, body["result"])
         end
 
+        test "id-bearing notification message is rejected with Method not found" do
+          init_request = create_rack_request(
+            "POST",
+            "/",
+            { "CONTENT_TYPE" => "application/json" },
+            { jsonrpc: "2.0", method: "initialize", id: "init" }.to_json,
+          )
+          init_response = @transport.handle_request(init_request)
+          session_id = init_response[1]["Mcp-Session-Id"]
+
+          request = create_rack_request(
+            "POST",
+            "/",
+            {
+              "CONTENT_TYPE" => "application/json",
+              "HTTP_MCP_SESSION_ID" => session_id,
+            },
+            { jsonrpc: "2.0", method: "notifications/cancelled", id: "123" }.to_json,
+          )
+
+          response = @transport.handle_request(request)
+          assert_equal 200, response[0]
+
+          io = StringIO.new
+          response[2].call(io)
+          body = JSON.parse(io.string.match(/^data: (.+)$/)[1])
+
+          assert_equal "123", body["id"]
+          refute body.key?("result")
+          assert_equal JsonRpcHandler::ErrorCode::METHOD_NOT_FOUND, body["error"]["code"]
+        end
+
         test "handles POST request with invalid JSON" do
           request = create_rack_request(
             "POST",

test/mcp/server_cancellation_test.rb

@@ -144,6 +144,96 @@ def handle_request(request); end
       assert_nil response
     end
 
+    test "id-bearing notifications/cancelled is rejected with Method not found, not result: null" do
+      response = @session.handle(
+        jsonrpc: "2.0",
+        id: 1001,
+        method: Methods::NOTIFICATIONS_CANCELLED,
+      )
+
+      assert_equal 1001, response[:id]
+      refute response.key?(:result)
+      assert_equal JsonRpcHandler::ErrorCode::METHOD_NOT_FOUND, response.dig(:error, :code)
+    end
+
+    test "id-bearing notifications/initialized is rejected with Method not found" do
+      response = @session.handle(
+        jsonrpc: "2.0",
+        id: 1002,
+        method: Methods::NOTIFICATIONS_INITIALIZED,
+      )
+
+      assert_equal 1002, response[:id]
+      refute response.key?(:result)
+      assert_equal JsonRpcHandler::ErrorCode::METHOD_NOT_FOUND, response.dig(:error, :code)
+    end
+
+    test "id-bearing notifications/progress is rejected with Method not found" do
+      response = @session.handle(
+        jsonrpc: "2.0",
+        id: 1003,
+        method: Methods::NOTIFICATIONS_PROGRESS,
+        params: { progressToken: "tok", progress: 1 },
+      )
+
+      assert_equal 1003, response[:id]
+      refute response.key?(:result)
+      assert_equal JsonRpcHandler::ErrorCode::METHOD_NOT_FOUND, response.dig(:error, :code)
+    end
+
+    test "well-formed notifications/initialized still receives no response" do
+      response = @session.handle(
+        jsonrpc: "2.0",
+        method: Methods::NOTIFICATIONS_INITIALIZED,
+      )
+
+      assert_nil response
+    end
+
+    test "id-bearing notification mixed into a batch yields only its Method not found error" do
+      responses = @session.handle([
+        { jsonrpc: "2.0", method: Methods::NOTIFICATIONS_CANCELLED, params: { requestId: "x" } },
+        { jsonrpc: "2.0", id: 2001, method: Methods::NOTIFICATIONS_CANCELLED },
+        { jsonrpc: "2.0", id: 2002, method: Methods::PING },
+      ])
+
+      cancelled_error = responses.find { |response| response[:id] == 2001 }
+      assert_equal JsonRpcHandler::ErrorCode::METHOD_NOT_FOUND, cancelled_error.dig(:error, :code)
+
+      ping_response = responses.find { |response| response[:id] == 2002 }
+      assert_equal({}, ping_response[:result])
+
+      # The well-formed notification carries no id and contributes no response.
+      assert_equal 2, responses.size
+    end
+
+    test "id-bearing custom notifications/* method is rejected with Method not found" do
+      called = false
+      @server.define_custom_method(method_name: "notifications/custom") { called = true }
+
+      response = @session.handle(
+        jsonrpc: "2.0",
+        id: 3001,
+        method: "notifications/custom",
+      )
+
+      assert_equal JsonRpcHandler::ErrorCode::METHOD_NOT_FOUND, response.dig(:error, :code)
+      refute called
+    end
+
+    test "well-formed custom notifications/* method is dispatched without a response" do
+      called = false
+      @server.define_custom_method(method_name: "notifications/custom") { called = true }
+
+      response = @session.handle(
+        jsonrpc: "2.0",
+        method: "notifications/custom",
+      )
+
+      assert_nil response
+      assert called
+    end
+
     test "duplicate cancellation for the same in-flight request is idempotent" do
       @server.define_tool(name: "slow_dup") do |server_context:|
         20.times do