diff --git a/lib/json_rpc_handler.rb b/lib/json_rpc_handler.rb index 6cbce1cb..362f6b5c 100644 --- a/lib/json_rpc_handler.rb +++ b/lib/json_rpc_handler.rb @@ -97,10 +97,9 @@ def process_request(request, id_validation_pattern:, &method_finder) end # Per JSON-RPC 2.0 (Response object, `id`), the error response must carry - # the same id as the request when the id could be detected; null is only - # for requests whose id could not be determined. `error_response` nils out - # ids that fail validation, and the `:unknown_id` sentinel keeps a response - # (with a null id) being emitted when the request carried no id at all. + # the same id as the request when the id could be detected. `error_response` + # leaves the id out for ids that fail validation, and the `:unknown_id` sentinel + # keeps a response (without an id) being emitted when the request carried no id at all. return error_response(id: id.nil? ? :unknown_id : id, id_validation_pattern: id_validation_pattern, error: { code: ErrorCode::INVALID_REQUEST, message: "Invalid Request", @@ -197,11 +196,15 @@ def success_response(id:, result:) } unless id.nil? end + # An error response for a request whose id could not be read carries no `id` member: JSON-RPC 2.0 puts + # a null there, but the MCP schema allows only a string or an integer, and the Streamable HTTP transport + # specification describes such a body as an error response that has no id. def error_response(id:, id_validation_pattern:, error:) - { - jsonrpc: Version::V2_0, - id: valid_id?(id, id_validation_pattern) ? id : nil, - error: error.compact, - } unless id.nil? + return if id.nil? + + response = { jsonrpc: Version::V2_0 } + response[:id] = id if valid_id?(id, id_validation_pattern) + response[:error] = error.compact + response end end diff --git a/lib/mcp/server/transports/streamable_http_transport.rb b/lib/mcp/server/transports/streamable_http_transport.rb index ef85ef2f..653a71e2 100644 --- a/lib/mcp/server/transports/streamable_http_transport.rb +++ b/lib/mcp/server/transports/streamable_http_transport.rb @@ -1020,7 +1020,7 @@ def handle_subscriptions_listen(body, auth_info: nil) # The id is echoed in every message of the stream, so one that cannot be written back as JSON # (a String holding bytes that are not valid UTF-8 parses, but does not generate) is refused here, - # while the refusal can still answer with a null id, rather than surfacing as a failed write after + # while the refusal can still answer without an id, rather than surfacing as a failed write after # the stream has been registered. unless json_encodable?(request_id) return invalid_request_response("Invalid Request: subscriptions/listen id must be valid UTF-8") @@ -2030,10 +2030,14 @@ def validate_protocol_version_header(request) ) end + # The body leaves the `id` member out when there is no request id to echo, as the MCP schema allows + # only a string or an integer there. def json_rpc_error_response(status:, code:, message:, data: nil, id: nil) error = { code: code, message: message } error[:data] = data if data - body = { jsonrpc: "2.0", id: id, error: error } + body = { jsonrpc: "2.0" } + body[:id] = id unless id.nil? + body[:error] = error [status, { "content-type" => "application/json" }, [body.to_json]] end @@ -2479,15 +2483,12 @@ def already_initialized_response(request_id) end def invalid_request_response(message, request_id: nil) - body = { - jsonrpc: "2.0", + json_rpc_error_response( + status: 400, + code: JsonRpcHandler::ErrorCode::INVALID_REQUEST, + message: message, id: request_id, - error: { - code: JsonRpcHandler::ErrorCode::INVALID_REQUEST, - message: message, - }, - } - [400, { "content-type" => "application/json" }, [body.to_json]] + ) end def session_already_connected_response diff --git a/test/json_rpc_handler_test.rb b/test/json_rpc_handler_test.rb index 07db0ecf..f2db82d4 100644 --- a/test/json_rpc_handler_test.rb +++ b/test/json_rpc_handler_test.rb @@ -315,7 +315,7 @@ assert_nil @response[:result] end - it "returns nil for id when there is an error and and error detecting the id" do + it "leaves the id out when there is an error detecting the id" do register("ping") { "pong" } handle jsonrpc: "2.0", id: {}, method: "ping" @@ -325,7 +325,7 @@ message: "Invalid Request", data: "Request ID must match validation pattern, or be an integer or null", } - assert_nil @response[:id] + refute @response.key?(:id) end it "returns the same request id on an Invalid Request error when the id is detectable" do @@ -339,7 +339,7 @@ assert_equal 3, @response[:id] end - it "returns nil for id on an Invalid Request error when the request has no id" do + it "leaves the id out of an Invalid Request error when the request has no id" do handle jsonrpc: "1.0", method: "ping", params: {} assert_rpc_error expected_error: { @@ -347,10 +347,10 @@ message: "Invalid Request", data: "JSON-RPC version must be 2.0", } - assert_nil @response[:id] + refute @response.key?(:id) end - it "returns nil for id on an Invalid Request error when the request id is explicitly null" do + it "leaves the id out of an Invalid Request error when the request id is explicitly null" do handle jsonrpc: "1.0", id: nil, method: "ping", params: {} assert_rpc_error expected_error: { @@ -358,10 +358,10 @@ message: "Invalid Request", data: "JSON-RPC version must be 2.0", } - assert_nil @response[:id] + refute @response.key?(:id) end - it "returns nil for id on an Invalid Request error when the id fails validation" do + it "leaves the id out of an Invalid Request error when the id fails validation" do handle jsonrpc: "1.0", id: "", method: "ping", params: {} assert_rpc_error expected_error: { @@ -369,7 +369,7 @@ message: "Invalid Request", data: "JSON-RPC version must be 2.0", } - assert_nil @response[:id] + refute @response.key?(:id) end # 5.1 Error object @@ -687,7 +687,7 @@ assert_equal(-32600, @response.dig(:error, :code)) assert_equal "Invalid Request", @response.dig(:error, :message) - assert_nil @response[:id] + refute @response.key?(:id) end end @@ -702,7 +702,7 @@ assert @response.is_a?(Array) assert_equal 3, @response.first[:result] assert_equal(-32600, @response.last.dig(:error, :code)) - assert_nil @response.last[:id] + refute @response.last.key?(:id) end it "does not raise from handle_json when a batch element is not an object" do diff --git a/test/mcp/server/transports/stdio_transport_test.rb b/test/mcp/server/transports/stdio_transport_test.rb index 683e2ae4..f1aaf15a 100644 --- a/test/mcp/server/transports/stdio_transport_test.rb +++ b/test/mcp/server/transports/stdio_transport_test.rb @@ -95,7 +95,7 @@ class StdioTransportTest < ActiveSupport::TestCase @transport.send(:handle_request, JSON.generate(request)) response = JSON.parse(output.string, symbolize_names: true) assert_equal("2.0", response[:jsonrpc]) - assert_nil(response[:id]) + refute(response.key?(:id)) assert_nil(response[:result]) ensure $stdout = original_stdout @@ -147,7 +147,7 @@ class StdioTransportTest < ActiveSupport::TestCase @transport.open response = JSON.parse(output.string, symbolize_names: true) - assert_nil(response[:id]) + refute(response.key?(:id)) assert_equal(-32600, response[:error][:code]) assert_equal("Invalid Request", response[:error][:message]) ensure @@ -217,7 +217,7 @@ class StdioTransportTest < ActiveSupport::TestCase @transport.send(:handle_request, invalid_json) response = JSON.parse(output.string, symbolize_names: true) assert_equal("2.0", response[:jsonrpc]) - assert_nil(response[:id]) + refute(response.key?(:id)) assert_equal(-32600, response[:error][:code]) assert_equal("Invalid Request", response[:error][:message]) assert_equal("Request must be an array or a hash", response[:error][:data]) diff --git a/test/mcp/server/transports/streamable_http_transport_test.rb b/test/mcp/server/transports/streamable_http_transport_test.rb index 1e1ebc7a..e7eed15a 100644 --- a/test/mcp/server/transports/streamable_http_transport_test.rb +++ b/test/mcp/server/transports/streamable_http_transport_test.rb @@ -171,7 +171,7 @@ def string body = JSON.parse(response[2][0]) assert_equal "2.0", body["jsonrpc"] - assert_nil body["id"] + refute body.key?("id") assert_equal JsonRpcHandler::ErrorCode::PARSE_ERROR, body["error"]["code"] assert_equal "Parse error: Invalid JSON", body["error"]["message"] end @@ -251,6 +251,22 @@ def string assert_equal Configuration::LATEST_HANDSHAKE_PROTOCOL_VERSION, body["result"]["protocolVersion"] end + test "a refusal echoes the integer id 0 of the request" do + session_id = initialize_test_session + + response = @transport.handle_request(create_rack_request( + "POST", + "/", + { "CONTENT_TYPE" => "application/json", "HTTP_MCP_SESSION_ID" => session_id }, + { jsonrpc: "2.0", method: "initialize", id: 0, params: initialize_params }.to_json, + )) + + assert_equal 400, response[0] + body = JSON.parse(response[2][0]) + assert_equal 0, body["id"] + assert_equal JsonRpcHandler::ErrorCode::INVALID_REQUEST, body["error"]["code"] + end + test "rejects duplicate initialize with existing Mcp-Session-Id and preserves session" do init_request = create_rack_request( "POST", @@ -300,7 +316,7 @@ def string assert_equal 404, response[0] body = JSON.parse(response[2][0]) assert_equal "2.0", body["jsonrpc"] - assert_nil body["id"] + refute body.key?("id") assert_equal JsonRpcHandler::ErrorCode::INVALID_REQUEST, body["error"]["code"] assert_equal "Session not found", body["error"]["message"] end @@ -388,7 +404,7 @@ def string assert_equal 400, response[0] body = JSON.parse(response[2][0]) assert_equal "2.0", body["jsonrpc"] - assert_nil body["id"] + refute body.key?("id") assert_equal JsonRpcHandler::ErrorCode::INVALID_REQUEST, body["error"]["code"] assert_match(/single request object/i, body["error"]["message"]) end @@ -553,7 +569,7 @@ def string body = JSON.parse(response[2][0]) assert_equal "2.0", body["jsonrpc"] - assert_nil body["id"] + refute body.key?("id") assert_equal JsonRpcHandler::ErrorCode::INVALID_REQUEST, body["error"]["code"] assert_equal "Missing session ID", body["error"]["message"] end @@ -711,7 +727,7 @@ def string body = JSON.parse(response[2][0]) assert_equal "2.0", body["jsonrpc"] - assert_nil body["id"] + refute body.key?("id") assert_equal JsonRpcHandler::ErrorCode::INVALID_REQUEST, body["error"]["code"] assert_equal "Conflict: Only one SSE stream is allowed per session", body["error"]["message"] end @@ -1618,7 +1634,7 @@ def string body = JSON.parse(response[2][0]) assert_equal "2.0", body["jsonrpc"] - assert_nil body["id"] + refute body.key?("id") assert_equal JsonRpcHandler::ErrorCode::INVALID_REQUEST, body["error"]["code"] assert_equal "Method not allowed", body["error"]["message"] end @@ -1636,7 +1652,7 @@ def string body = JSON.parse(response[2][0]) assert_equal "2.0", body["jsonrpc"] - assert_nil body["id"] + refute body.key?("id") assert_equal JsonRpcHandler::ErrorCode::INVALID_REQUEST, body["error"]["code"] assert_equal "Unsupported Media Type: Content-Type must be application/json", body["error"]["message"] end @@ -1831,7 +1847,7 @@ def string body = JSON.parse(response[2][0]) assert_equal "2.0", body["jsonrpc"] - assert_nil body["id"] + refute body.key?("id") assert_equal JsonRpcHandler::ErrorCode::INVALID_REQUEST, body["error"]["code"] assert_equal "Not Acceptable: Accept header must include text/event-stream", body["error"]["message"] end @@ -2996,7 +3012,7 @@ def string body = JSON.parse(response[2][0]) assert_equal "2.0", body["jsonrpc"] - assert_nil body["id"] + refute body.key?("id") assert_equal JsonRpcHandler::ErrorCode::INTERNAL_ERROR, body["error"]["code"] assert_equal "Internal server error", body["error"]["message"] end @@ -6758,9 +6774,9 @@ def string parsed = JSON.parse(response[2][0]) - # The member is present and null, as JSON-RPC 2.0 requires when the id cannot be determined. - assert parsed.key?("id") - assert_nil parsed["id"] + # No id member: the MCP schema allows only a string or an integer there, so an id that cannot be + # read is left out, where JSON-RPC 2.0 would put a null. + refute parsed.key?("id") assert_equal JsonRpcHandler::ErrorCode::INVALID_REQUEST, parsed.dig("error", "code") assert_empty @transport.instance_variable_get(:@listen_subscriptions) end diff --git a/test/mcp/server_test.rb b/test/mcp/server_test.rb index c1ac383f..101a0f27 100644 --- a/test/mcp/server_test.rb +++ b/test/mcp/server_test.rb @@ -125,6 +125,15 @@ class ServerTest < ActiveSupport::TestCase assert_instrumentation_data({ method: "ping" }) end + test "#handle_json leaves the id out of an error response when the request id cannot be read" do + # The MCP schema types the id of an error response as an optional string or integer, where JSON-RPC 2.0 + # puts a null; the TypeScript client refuses a null there at schema validation. + json = @server.handle_json({ jsonrpc: "2.0", id: true, method: "ping" }.to_json) + + refute_includes json, '"id"' + assert_equal(-32600, JSON.parse(json).dig("error", "code")) + end + test "#handle server/discover returns supported versions, capabilities, server info, and instructions" do response = @server.handle({ jsonrpc: "2.0", method: "server/discover", id: 1 }) result = response[:result]