diff --git a/crates/rmcp/src/handler/server.rs b/crates/rmcp/src/handler/server.rs index 4abe9ccb4..28173db2f 100644 --- a/crates/rmcp/src/handler/server.rs +++ b/crates/rmcp/src/handler/server.rs @@ -220,10 +220,31 @@ impl Service for H { .list_tools(request.params, context) .await .map(ServerResult::ListToolsResult), - ClientRequest::CustomRequest(request) => self - .on_custom_request(request, context) - .await - .map(ServerResult::CustomResult), + ClientRequest::CustomRequest(request) => { + // A known method only lands here when its params failed to + // deserialize; that is the client's mistake, not an unknown method. + // Methods this lifecycle does not serve stay "method not found", + // as the typed arms above answer them (SEP-2575). + let not_served: &[&str] = if legacy_request { + &[SubscriptionsListenRequestMethod::VALUE] + } else { + &[ + InitializeResultMethod::VALUE, + PingRequestMethod::VALUE, + SetLevelRequestMethod::VALUE, + SubscribeRequestMethod::VALUE, + UnsubscribeRequestMethod::VALUE, + ] + }; + if !not_served.contains(&request.method.as_str()) + && let Some(reason) = ClientRequest::known_method_params_error(&request) + { + return Err(McpError::invalid_params(reason, None)); + } + self.on_custom_request(request, context) + .await + .map(ServerResult::CustomResult) + } ClientRequest::GetTaskRequest(request) => { validate_tasks_capability::(self, &context)?; self.get_task(request.params, context) diff --git a/crates/rmcp/src/model.rs b/crates/rmcp/src/model.rs index 9dd04564e..1d5856734 100644 --- a/crates/rmcp/src/model.rs +++ b/crates/rmcp/src/model.rs @@ -4815,6 +4815,60 @@ ts_union!( | CustomRequest; ); +impl ClientRequest { + /// Why the params of `request` do not fit the method it names, when that + /// method is one this SDK defines. + /// + /// `ClientRequest` is untagged, so a request for a known method whose params + /// fail to deserialize lands in [`ClientRequest::CustomRequest`] instead of + /// its typed variant. Reading the request again as that method's type + /// recovers the error, so the server can answer `-32602` rather than treat + /// a known method as unknown. Returns `None` for a method the SDK does not + /// define. + #[cfg(feature = "server")] + pub(crate) fn known_method_params_error(request: &CustomRequest) -> Option { + let mut message = serde_json::Map::new(); + message.insert("method".into(), Value::String(request.method.clone())); + if let Some(params) = &request.params { + message.insert("params".into(), params.clone()); + } + let message = Value::Object(message); + macro_rules! check { + ($($method:ty => $request:ty),* $(,)?) => { + $( + if request.method == <$method as ConstString>::VALUE { + let error = serde_json::from_value::<$request>(message) + .err() + .map_or_else(|| "params do not match the method".to_owned(), |e| e.to_string()); + return Some(format!("invalid params for {}: {error}", request.method)); + } + )* + }; + } + check!( + PingRequestMethod => PingRequest, + InitializeResultMethod => InitializeRequest, + DiscoverRequestMethod => DiscoverRequest, + CompleteRequestMethod => CompleteRequest, + SetLevelRequestMethod => SetLevelRequest, + GetPromptRequestMethod => GetPromptRequest, + ListPromptsRequestMethod => ListPromptsRequest, + ListResourcesRequestMethod => ListResourcesRequest, + ListResourceTemplatesRequestMethod => ListResourceTemplatesRequest, + ReadResourceRequestMethod => ReadResourceRequest, + SubscriptionsListenRequestMethod => SubscriptionsListenRequest, + SubscribeRequestMethod => SubscribeRequest, + UnsubscribeRequestMethod => UnsubscribeRequest, + CallToolRequestMethod => CallToolRequest, + ListToolsRequestMethod => ListToolsRequest, + GetTaskMethod => GetTaskRequest, + UpdateTaskMethod => UpdateTaskRequest, + CancelTaskMethod => CancelTaskRequest, + ); + None + } +} + impl ClientRequest { pub fn method(&self) -> &str { match &self { diff --git a/crates/rmcp/tests/test_custom_request.rs b/crates/rmcp/tests/test_custom_request.rs index 66ee1ff99..841a512c3 100644 --- a/crates/rmcp/tests/test_custom_request.rs +++ b/crates/rmcp/tests/test_custom_request.rs @@ -188,3 +188,66 @@ async fn test_custom_server_request_reaches_client() -> anyhow::Result<()> { client.cancel().await?; Ok(()) } + +/// A request that names a method the SDK defines but whose params do not fit +/// it is answered with `-32602` Invalid params, not `-32601` Method not found, +/// and never reaches `on_custom_request` (#1347). +#[tokio::test] +async fn test_known_method_with_invalid_params_is_invalid_params() -> anyhow::Result<()> { + let (server_transport, client_transport) = tokio::io::duplex(4096); + let payload = Arc::new(Mutex::new(None)); + { + let payload = payload.clone(); + tokio::spawn(async move { + let server = CustomRequestServer { + receive_signal: Arc::new(Notify::new()), + payload, + } + .serve(server_transport) + .await?; + server.waiting().await?; + anyhow::Ok(()) + }); + } + let client = ().serve(client_transport).await?; + + for (method, params) in [ + ("tools/call", Some(json!({ "name": 7, "arguments": {} }))), + ( + "tools/call", + Some(json!({ "name": "ok", "arguments": "x" })), + ), + ("tools/call", None), + ("prompts/get", Some(json!({ "name": 7 }))), + ("resources/read", Some(json!({ "uri": 7 }))), + ] { + let result = client + .send_request(ClientRequest::CustomRequest(CustomRequest::new( + method, + params.clone(), + ))) + .await; + match result { + Err(rmcp::ServiceError::McpError(error)) => { + assert_eq!( + error.code, + rmcp::model::ErrorCode::INVALID_PARAMS, + "{method} {params:?}: {error:?}" + ); + assert!( + error.message.contains(method), + "{method} {params:?}: message {:?} does not name the method", + error.message + ); + } + other => panic!("{method} {params:?}: expected Invalid params, got {other:?}"), + } + } + assert!( + payload.lock().await.is_none(), + "a known method with invalid params reached on_custom_request" + ); + + client.cancel().await?; + Ok(()) +}