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
This commit is contained in:
commit
49d6d7f254
4 changed files with 133 additions and 0 deletions
|
|
@ -47,6 +47,10 @@ module MCP
|
||||||
end
|
end
|
||||||
|
|
||||||
class << self
|
class << self
|
||||||
|
def notification?(method)
|
||||||
|
method.is_a?(String) && method.start_with?("notifications/")
|
||||||
|
end
|
||||||
|
|
||||||
def ensure_capability!(method, capabilities)
|
def ensure_capability!(method, capabilities)
|
||||||
case method
|
case method
|
||||||
when PROMPTS_GET, PROMPTS_LIST
|
when PROMPTS_GET, PROMPTS_LIST
|
||||||
|
|
|
||||||
|
|
@ -430,6 +430,13 @@ module MCP
|
||||||
end
|
end
|
||||||
|
|
||||||
def handle_request(request, method, session: nil, related_request_id: nil)
|
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)
|
# `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 intentionally bypasses the `@handlers` lookup, capability check, in-flight registry,
|
||||||
# and rescue blocks below.
|
# and rescue blocks below.
|
||||||
|
|
|
||||||
|
|
@ -80,6 +80,38 @@ module MCP
|
||||||
assert_equal({}, body["result"])
|
assert_equal({}, body["result"])
|
||||||
end
|
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
|
test "handles POST request with invalid JSON" do
|
||||||
request = create_rack_request(
|
request = create_rack_request(
|
||||||
"POST",
|
"POST",
|
||||||
|
|
|
||||||
|
|
@ -144,6 +144,96 @@ module MCP
|
||||||
assert_nil response
|
assert_nil response
|
||||||
end
|
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
|
test "duplicate cancellation for the same in-flight request is idempotent" do
|
||||||
@server.define_tool(name: "slow_dup") do |server_context:|
|
@server.define_tool(name: "slow_dup") do |server_context:|
|
||||||
20.times do
|
20.times do
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue