From 569e864edcfad4d269eb429f5a516b8e8a32332d Mon Sep 17 00:00:00 2001 From: "Vit.Bogachev" Date: Sat, 17 Oct 2026 10:52:39 +0300 Subject: [PATCH 1/4] fix(server): answer a known method with invalid params with -32602 A request that names a method the SDK defines but whose params do not deserialize fell through the untagged ClientRequest to CustomRequest, and the default on_custom_request answered -32601 with the method name: the client was told the server has no tools/call when the problem was its own arguments. Before calling on_custom_request, the server now checks whether the method is one ClientRequest defines. If it is, it reads the request again as that method's type to recover the error and answers -32602 with it, for example "invalid params for tools/call: invalid type: integer `7`, expected a string". Unknown methods still reach on_custom_request. Co-Authored-By: Claude Opus 5.5 --- crates/rmcp/src/handler/server.rs | 14 ++++-- crates/rmcp/src/model.rs | 53 ++++++++++++++++++++ crates/rmcp/tests/test_custom_request.rs | 63 ++++++++++++++++++++++++ 3 files changed, 126 insertions(+), 4 deletions(-) diff --git a/crates/rmcp/src/handler/server.rs b/crates/rmcp/src/handler/server.rs index 4abe9ccb4..2d33ee024 100644 --- a/crates/rmcp/src/handler/server.rs +++ b/crates/rmcp/src/handler/server.rs @@ -220,10 +220,16 @@ 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. + if 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..20fe32279 100644 --- a/crates/rmcp/src/model.rs +++ b/crates/rmcp/src/model.rs @@ -4815,6 +4815,59 @@ 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. + 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(()) +} From 0c01e98c230267c12c3b201ec78bce125c3680be Mon Sep 17 00:00:00 2001 From: "Vit.Bogachev" Date: Sat, 17 Oct 2026 18:18:49 +0300 Subject: [PATCH 2/4] fix(server): keep methods this lifecycle does not serve as -32601 The conformance suite's server-stateless scenario (SEP-2575) expects initialize, logging/setLevel, resources/subscribe and resources/unsubscribe sent on the 2026-07-28 lifecycle to be answered -32601 with HTTP 404. Those requests reach CustomRequest too, so the previous commit turned them into -32602. The check now leaves alone the methods the typed arms already answer as not found for this lifecycle: those four plus ping on the 2026-07-28 lifecycle, and subscriptions/listen on the legacy one. known_method_params_error is compiled only with the server feature, so `cargo clippy --lib --no-default-features` no longer flags it unused. Conformance server suites pass locally: 2025-11-25 83/83, 2026-07-28 153/153. Co-Authored-By: Claude Opus 5.5 --- crates/rmcp/src/handler/server.rs | 17 ++++++++++++++++- crates/rmcp/src/model.rs | 1 + 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/crates/rmcp/src/handler/server.rs b/crates/rmcp/src/handler/server.rs index 2d33ee024..28173db2f 100644 --- a/crates/rmcp/src/handler/server.rs +++ b/crates/rmcp/src/handler/server.rs @@ -223,7 +223,22 @@ impl Service for H { 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. - if let Some(reason) = ClientRequest::known_method_params_error(&request) { + // 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) diff --git a/crates/rmcp/src/model.rs b/crates/rmcp/src/model.rs index 20fe32279..1d5856734 100644 --- a/crates/rmcp/src/model.rs +++ b/crates/rmcp/src/model.rs @@ -4825,6 +4825,7 @@ impl ClientRequest { /// 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())); From 201a516ed33f51a92279eb46cb2f71bdcb25304a Mon Sep 17 00:00:00 2001 From: "Vit.Bogachev" Date: Mon, 19 Oct 2026 12:25:28 +0300 Subject: [PATCH 3/4] fix(server): tell invalid params from unknown methods at deserialization Rework after review on #1375. Instead of a second method table consulted before `on_custom_request`, the distinction is kept where it is lost: - `RequestMethod` trait: every `Request*` carries `M::VALUE`. - `ClientRequest` deserializes by `method` (`ts_union!` `#[request]` arm): the body is buffered once, only the named variant is parsed, a serde error becomes the `#[doc(hidden)]` `InvalidParams` variant that keeps method, params, error and `_meta`; `CustomRequest` is only for methods the SDK does not define. Serializing `InvalidParams` yields the custom request it came from. - One `validate_method_served` pre-check in `handle_request` replaces the per-arm lifecycle gates and `validate_tasks_capability` calls, and also refuses `initialize` and `logging/setLevel` with well-formed params on the 2026-07-28 lifecycle. - Before the handshake a malformed `initialize` is answered -32602 and the server keeps waiting. Tests: deserialization by method, inline-lifecycle and legacy-lifecycle cases (removed methods stay -32601 with fitting and unfitting params), malformed initialize before the handshake, and the #1347 rows. Co-Authored-By: Claude Fable 5.1 --- crates/rmcp/src/handler/server.rs | 228 +++++++++-------- crates/rmcp/src/model.rs | 231 +++++++++++++----- crates/rmcp/src/model/meta.rs | 1 + crates/rmcp/src/service/server.rs | 17 ++ crates/rmcp/tests/test_custom_request.rs | 68 ++++++ crates/rmcp/tests/test_deserialization.rs | 80 ++++++ .../tests/test_stateless_server_requests.rs | 129 ++++++++++ 7 files changed, 587 insertions(+), 167 deletions(-) diff --git a/crates/rmcp/src/handler/server.rs b/crates/rmcp/src/handler/server.rs index 28173db2f..2876a713d 100644 --- a/crates/rmcp/src/handler/server.rs +++ b/crates/rmcp/src/handler/server.rs @@ -28,12 +28,17 @@ pub mod wrapper; /// the request's `_meta` per-request capabilities or, for session-mode /// peers, at `initialize` time), the spec requires `-32021` Missing /// Required Client Capability with the required capability in `data`. -fn validate_tasks_capability( +fn validate_tasks_capability( handler: &H, + method: &str, context: &RequestContext, ) -> Result<(), McpError> { if !handler.get_info().capabilities.supports_tasks() { - return Err(McpError::method_not_found::()); + return Err(McpError::new( + ErrorCode::METHOD_NOT_FOUND, + method.to_owned(), + None, + )); } let client_declared = context .client_capabilities() @@ -47,6 +52,56 @@ fn validate_tasks_capability( } } +/// Decides, in one place and before dispatch, whether `method` is served on +/// this lifecycle (SEP-2575) and, for the task methods, whether both sides +/// declared the tasks capability. +/// +/// The 2026-07-28 lifecycle has no handshake, so `initialize`, `ping`, +/// `logging/setLevel`, `resources/subscribe` and `resources/unsubscribe` are +/// not found there; the legacy lifecycle has no `subscriptions/listen`. These +/// answer `-32601` whatever the params carried, so a removed method with +/// malformed params is still "not found" rather than "invalid params". +/// +/// `initialize` is judged by `inline_session` rather than `legacy_request`: +/// its own params supply the fallback protocol version while the handshake +/// is still open, so only a session already on the inline lifecycle, or a +/// request that declares 2026-07-28 in its `_meta`, has no `initialize`. +fn validate_method_served( + handler: &H, + method: &str, + legacy_request: bool, + inline_session: bool, + context: &RequestContext, +) -> Result<(), McpError> { + let not_served: &[&str] = if legacy_request { + &[SubscriptionsListenRequestMethod::VALUE] + } else { + &[ + PingRequestMethod::VALUE, + SetLevelRequestMethod::VALUE, + SubscribeRequestMethod::VALUE, + UnsubscribeRequestMethod::VALUE, + ] + }; + if not_served.contains(&method) || (inline_session && method == InitializeResultMethod::VALUE) { + return Err(McpError::new( + ErrorCode::METHOD_NOT_FOUND, + method.to_owned(), + None, + )); + } + if [ + GetTaskMethod::VALUE, + UpdateTaskMethod::VALUE, + CancelTaskMethod::VALUE, + ] + .contains(&method) + { + validate_tasks_capability(handler, method, context)?; + } + Ok(()) +} + impl Service for H { async fn handle_request( &self, @@ -100,6 +155,17 @@ impl Service for H { } let legacy_request = uses_legacy_lifecycle(protocol_version.as_ref(), requires_request_metadata); + let inline_session = context.peer.request_metadata_required() + || requested_version + .as_ref() + .is_some_and(|version| version.as_str() >= ProtocolVersion::V_2026_07_28.as_str()); + validate_method_served( + self, + request.method(), + legacy_request, + inline_session, + &context, + )?; let result = match request { ClientRequest::InitializeRequest(request) => self .initialize(request.params, context) @@ -110,11 +176,7 @@ impl Service for H { .await .map(ServerResult::DiscoverResult), ClientRequest::PingRequest(_request) => { - if !legacy_request { - Err(McpError::method_not_found::()) - } else { - self.ping(context).await.map(ServerResult::empty) - } + self.ping(context).await.map(ServerResult::empty) } ClientRequest::CompleteRequest(request) => self .complete(request.params, context) @@ -145,61 +207,45 @@ impl Service for H { .await .map(ServerResult::from), ClientRequest::SubscriptionsListenRequest(request) => { - if legacy_request { - Err(McpError::method_not_found::()) - } else { - let requested = request.params.notifications; - let Some(candidate) = self.accepted_subscription_filter(&requested) else { - return Err( - McpError::method_not_found::(), - ); - }; - let server_info = self.get_info(); - let advertised = requested.supported_by(&server_info.capabilities); - let handler_accepted = requested.intersection(&candidate); - let accepted = handler_accepted.intersection(&advertised); - if accepted != handler_accepted { - tracing::debug!( - requested_resource_count = requested - .resource_subscriptions - .as_ref() - .map_or(0, Vec::len), - accepted_resource_count = - accepted.resource_subscriptions.as_ref().map_or(0, Vec::len), - "subscription filter reduced to advertised server capabilities" - ); - } - let server_implementation = server_info.server_info; - let subscription_id = context.id.clone(); - let subscription = - SubscriptionContext::establish(context, requested, accepted).await?; - // The 2026-07-28 schema defines a final result for graceful - // server teardown; explicit stdio cancellation remains a notification. - self.listen(subscription).await.map(|()| { - let mut result = SubscriptionsListenResult::complete(subscription_id); - result.meta.set_server_info(server_implementation); - ServerResult::SubscriptionsListenResult(result) - }) - } - } - ClientRequest::SubscribeRequest(request) => { - if !legacy_request { - Err(McpError::method_not_found::()) - } else { - self.subscribe(request.params, context) - .await - .map(ServerResult::empty) - } - } - ClientRequest::UnsubscribeRequest(request) => { - if !legacy_request { - Err(McpError::method_not_found::()) - } else { - self.unsubscribe(request.params, context) - .await - .map(ServerResult::empty) + let requested = request.params.notifications; + let Some(candidate) = self.accepted_subscription_filter(&requested) else { + return Err(McpError::method_not_found::()); + }; + let server_info = self.get_info(); + let advertised = requested.supported_by(&server_info.capabilities); + let handler_accepted = requested.intersection(&candidate); + let accepted = handler_accepted.intersection(&advertised); + if accepted != handler_accepted { + tracing::debug!( + requested_resource_count = requested + .resource_subscriptions + .as_ref() + .map_or(0, Vec::len), + accepted_resource_count = + accepted.resource_subscriptions.as_ref().map_or(0, Vec::len), + "subscription filter reduced to advertised server capabilities" + ); } - } + let server_implementation = server_info.server_info; + let subscription_id = context.id.clone(); + let subscription = + SubscriptionContext::establish(context, requested, accepted).await?; + // The 2026-07-28 schema defines a final result for graceful + // server teardown; explicit stdio cancellation remains a notification. + self.listen(subscription).await.map(|()| { + let mut result = SubscriptionsListenResult::complete(subscription_id); + result.meta.set_server_info(server_implementation); + ServerResult::SubscriptionsListenResult(result) + }) + } + ClientRequest::SubscribeRequest(request) => self + .subscribe(request.params, context) + .await + .map(ServerResult::empty), + ClientRequest::UnsubscribeRequest(request) => self + .unsubscribe(request.params, context) + .await + .map(ServerResult::empty), ClientRequest::CallToolRequest(request) => { let client_declared_tasks = context .client_capabilities() @@ -220,49 +266,25 @@ impl Service for H { .list_tools(request.params, context) .await .map(ServerResult::ListToolsResult), - 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) - .await - .map(ServerResult::GetTaskResult) - } - ClientRequest::UpdateTaskRequest(request) => { - validate_tasks_capability::(self, &context)?; - self.update_task(request.params, context) - .await - .map(ServerResult::task_ack) - } - ClientRequest::CancelTaskRequest(request) => { - validate_tasks_capability::(self, &context)?; - self.cancel_task(request.params, context) - .await - .map(ServerResult::task_ack) + ClientRequest::CustomRequest(request) => self + .on_custom_request(request, context) + .await + .map(ServerResult::CustomResult), + ClientRequest::InvalidParams(request) => { + Err(McpError::invalid_params(request.to_string(), None)) } + ClientRequest::GetTaskRequest(request) => self + .get_task(request.params, context) + .await + .map(ServerResult::GetTaskResult), + ClientRequest::UpdateTaskRequest(request) => self + .update_task(request.params, context) + .await + .map(ServerResult::task_ack), + ClientRequest::CancelTaskRequest(request) => self + .cancel_task(request.params, context) + .await + .map(ServerResult::task_ack), }; let result = result.and_then(|mut result| { if matches!(result, ServerResult::InputRequiredResult(_)) && !is_2026_07_28_or_later { diff --git a/crates/rmcp/src/model.rs b/crates/rmcp/src/model.rs index 1d5856734..67d393743 100644 --- a/crates/rmcp/src/model.rs +++ b/crates/rmcp/src/model.rs @@ -79,6 +79,24 @@ pub trait ConstString: Default { Self::VALUE } } + +/// The wire method name of a typed request, read off its method marker, so a +/// union of requests can be dispatched on `method` without a hand-kept table. +pub(crate) trait RequestMethod { + const METHOD: &'static str; +} + +impl RequestMethod for Request { + const METHOD: &'static str = M::VALUE; +} + +impl RequestMethod for RequestOptionalParam { + const METHOD: &'static str = M::VALUE; +} + +impl RequestMethod for RequestNoParam { + const METHOD: &'static str = M::VALUE; +} #[macro_export] macro_rules! const_string { ($name:ident = $value:literal) => { @@ -1214,6 +1232,67 @@ impl CustomRequest { } } +/// A request for a method this SDK defines whose `params` did not deserialize +/// as that method's type. +/// +/// Only the `Deserialize` of a `#[request]` union builds one, so that a peer's +/// mistake in the arguments is answered with `-32602` Invalid params instead +/// of falling through to the custom-request path and `-32601` Method not +/// found. It keeps the request's `_meta` in `extensions`, so lifecycle checks +/// still see the protocol version the request declared. Not part of the +/// public API. +#[doc(hidden)] +#[derive(Debug, Clone)] +pub struct InvalidParamsRequest { + method: String, + params: Option, + error: String, + extensions: Extensions, +} + +impl InvalidParamsRequest { + fn new(body: &Value, error: serde_json::Error) -> Result { + let CustomRequest { + method, + params, + extensions, + } = CustomRequest::deserialize(body)?; + Ok(Self { + method, + params, + error: error.to_string(), + extensions, + }) + } + + /// The method the request named. + pub(crate) fn method(&self) -> &str { + &self.method + } +} + +/// Why the params did not fit, worded as the error message for the peer. +impl std::fmt::Display for InvalidParamsRequest { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "invalid params for {}: {}", self.method, self.error) + } +} + +impl Serialize for InvalidParamsRequest { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + // On the wire it is the request as it was received. + CustomRequest { + method: self.method.clone(), + params: self.params.clone(), + extensions: self.extensions.clone(), + } + .serialize(serializer) + } +} + const_string!(InitializeResultMethod = "initialize"); /// # Initialization /// This request is sent from the client to the server when it first connects, asking it to begin initialization. @@ -4742,27 +4821,45 @@ pub type TaskStatusNotification = // MESSAGE TYPE UNIONS // ============================================================================= +/// Declares a `#[serde(untagged)]` union of message types with a `From` impl +/// for each variant. +/// +/// `#[request]` marks a union of requests. Its variants are read by `method` +/// rather than tried in order: only the variant whose method matches is +/// deserialized; a known method whose `params` do not fit that variant becomes +/// the hidden `InvalidParams` variant, with the error kept; and only an unknown +/// method reaches the catch-all, which must be the last variant and be named +/// `CustomRequest`. macro_rules! ts_union { ( export type $U:ident = $($rest:tt)* ) => { - ts_union!(@declare $U { $($rest)* }); + ts_union!(@declare untagged $U { $($rest)* }); ts_union!(@impl_from $U { $($rest)* }); }; - (@declare $U:ident { $($variant:tt)* }) => { - ts_union!(@declare_variant $U { } {$($variant)*} ); + ( + #[request] + export type $U:ident = + $($rest:tt)* + ) => { + ts_union!(@declare request $U { $($rest)* }); + ts_union!(@impl_from $U { $($rest)* }); + ts_union!(@impl_deserialize_by_method $U { $($rest)* }); + }; + (@declare $mode:tt $U:ident { $($variant:tt)* }) => { + ts_union!(@declare_variant $mode $U { } {$($variant)*} ); }; - (@declare_variant $U:ident { $($declared:tt)* } {$(|)? box $V:ident $($rest:tt)*}) => { - ts_union!(@declare_variant $U { $($declared)* $V(Box<$V>), } {$($rest)*}); + (@declare_variant $mode:tt $U:ident { $($declared:tt)* } {$(|)? box $V:ident $($rest:tt)*}) => { + ts_union!(@declare_variant $mode $U { $($declared)* $V(Box<$V>), } {$($rest)*}); }; - (@declare_variant $U:ident { $($declared:tt)* } {$(|)? $V:ident $($rest:tt)*}) => { - ts_union!(@declare_variant $U { $($declared)* $V($V), } {$($rest)*}); + (@declare_variant $mode:tt $U:ident { $($declared:tt)* } {$(|)? $V:ident $($rest:tt)*}) => { + ts_union!(@declare_variant $mode $U { $($declared)* $V($V), } {$($rest)*}); }; - (@declare_variant $U:ident { $($declared:tt)* } { ; }) => { - ts_union!(@declare_end $U { $($declared)* } ); + (@declare_variant $mode:tt $U:ident { $($declared:tt)* } { ; }) => { + ts_union!(@declare_end $mode $U { $($declared)* } ); }; - (@declare_end $U:ident { $($declared:tt)* }) => { + (@declare_end untagged $U:ident { $($declared:tt)* }) => { #[derive(Debug, Serialize, Deserialize, Clone)] #[serde(untagged)] #[allow(clippy::large_enum_variant)] @@ -4772,6 +4869,64 @@ macro_rules! ts_union { $($declared)* } }; + (@declare_end request $U:ident { $($declared:tt)* }) => { + #[derive(Debug, Serialize, Clone)] + #[serde(untagged)] + #[allow(clippy::large_enum_variant)] + #[non_exhaustive] + #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] + pub enum $U { + $($declared)* + /// A request for a method this SDK defines whose `params` did not + /// deserialize as that method's type. Only `Deserialize` builds + /// it; the receiving side answers it with `-32602` Invalid params. + #[doc(hidden)] + #[cfg_attr(feature = "schemars", schemars(skip))] + InvalidParams(InvalidParamsRequest), + } + }; + (@impl_deserialize_by_method $U:ident { $($variant:tt)* }) => { + impl<'de> Deserialize<'de> for $U { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + // The body is buffered once; `method` picks the one variant to read. + let body = Value::Object(JsonObject::deserialize(deserializer)?); + if let Some(method) = body.get("method").and_then(Value::as_str) { + ts_union!(@dispatch $U body method { $($variant)* }); + } + CustomRequest::deserialize(&body) + .map($U::CustomRequest) + .map_err(serde::de::Error::custom) + } + } + }; + (@dispatch $U:ident $body:ident $method:ident { $(|)? CustomRequest $($rest:tt)* }) => {}; + (@dispatch $U:ident $body:ident $method:ident { $(|)? box $V:ident $($rest:tt)* }) => { + if $method == <$V as RequestMethod>::METHOD { + return Ok(match <$V as Deserialize>::deserialize(&$body) { + Ok(request) => $U::$V(Box::new(request)), + Err(error) => $U::InvalidParams( + InvalidParamsRequest::new(&$body, error).map_err(serde::de::Error::custom)?, + ), + }); + } + ts_union!(@dispatch $U $body $method { $($rest)* }); + }; + (@dispatch $U:ident $body:ident $method:ident { $(|)? $V:ident $($rest:tt)* }) => { + if $method == <$V as RequestMethod>::METHOD { + return Ok(match <$V as Deserialize>::deserialize(&$body) { + Ok(request) => $U::$V(request), + Err(error) => $U::InvalidParams( + InvalidParamsRequest::new(&$body, error).map_err(serde::de::Error::custom)?, + ), + }); + } + ts_union!(@dispatch $U $body $method { $($rest)* }); + }; + (@dispatch $U:ident $body:ident $method:ident { ; }) => {}; + (@dispatch $U:ident $body:ident $method:ident { }) => {}; (@impl_from $U: ident {$(|)? box $V:ident $($rest:tt)*}) => { impl From<$V> for $U { fn from(value: $V) -> Self { @@ -4793,6 +4948,7 @@ macro_rules! ts_union { } ts_union!( + #[request] export type ClientRequest = | PingRequest | InitializeRequest @@ -4815,60 +4971,6 @@ 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 { @@ -4891,6 +4993,7 @@ impl ClientRequest { ClientRequest::UpdateTaskRequest(r) => r.method.as_str(), ClientRequest::CancelTaskRequest(r) => r.method.as_str(), ClientRequest::CustomRequest(r) => r.method.as_str(), + ClientRequest::InvalidParams(r) => r.method(), } } } diff --git a/crates/rmcp/src/model/meta.rs b/crates/rmcp/src/model/meta.rs index 0a7121e4a..54d87213b 100644 --- a/crates/rmcp/src/model/meta.rs +++ b/crates/rmcp/src/model/meta.rs @@ -191,6 +191,7 @@ variant_extension! { GetTaskRequest UpdateTaskRequest CancelTaskRequest + InvalidParams } } diff --git a/crates/rmcp/src/service/server.rs b/crates/rmcp/src/service/server.rs index 8cb707d97..a095c02ba 100644 --- a/crates/rmcp/src/service/server.rs +++ b/crates/rmcp/src/service/server.rs @@ -598,6 +598,23 @@ where let id = req.id; let mut request = match req.request { ClientRequest::InitializeRequest(request) => break (request, id), + ClientRequest::InvalidParams(invalid) => { + // A known method whose params did not deserialize: + // answer it and keep waiting for the handshake. + transport + .send(ServerJsonRpcMessage::error( + ErrorData::invalid_params(invalid.to_string(), None), + Some(id), + )) + .await + .map_err(|error| { + ServerInitializeError::transport::( + error, + "sending pre-init invalid params response", + ) + })?; + continue; + } request => request, }; let missing_metadata = request diff --git a/crates/rmcp/tests/test_custom_request.rs b/crates/rmcp/tests/test_custom_request.rs index 841a512c3..44f33b928 100644 --- a/crates/rmcp/tests/test_custom_request.rs +++ b/crates/rmcp/tests/test_custom_request.rs @@ -251,3 +251,71 @@ async fn test_known_method_with_invalid_params_is_invalid_params() -> anyhow::Re client.cancel().await?; Ok(()) } + +/// The legacy lifecycle does not serve `subscriptions/listen`, so a malformed +/// one keeps `-32601` Method not found, while a malformed request for a +/// method this lifecycle does serve is `-32602` on the same connection (#1347). +#[tokio::test] +async fn test_legacy_lifecycle_keeps_unserved_method_not_found() -> 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 mut info = rmcp::model::ClientConfig::default(); + info.protocol_version = rmcp::model::ProtocolVersion::V_2025_11_25; + let client = info.serve(client_transport).await?; + assert_eq!( + client.peer_info().map(|info| info.protocol_version.clone()), + Some(rmcp::model::ProtocolVersion::V_2025_11_25), + "the handshake should settle on the legacy lifecycle" + ); + + for (method, params, want) in [ + ( + "subscriptions/listen", + json!({ "notifications": 7 }), + rmcp::model::ErrorCode::METHOD_NOT_FOUND, + ), + ( + "tools/call", + json!({ "name": 7, "arguments": {} }), + rmcp::model::ErrorCode::INVALID_PARAMS, + ), + ] { + let result = client + .send_request(ClientRequest::CustomRequest(CustomRequest::new( + method, + Some(params.clone()), + ))) + .await; + match result { + Err(rmcp::ServiceError::McpError(error)) => { + assert_eq!(error.code, want, "{method} {params}: {error:?}"); + assert!( + error.message.contains(method), + "{method}: message {:?} does not name the method", + error.message + ); + } + other => panic!("{method} {params}: expected code {want:?}, got {other:?}"), + } + } + assert!( + payload.lock().await.is_none(), + "a known method reached on_custom_request" + ); + + client.cancel().await?; + Ok(()) +} diff --git a/crates/rmcp/tests/test_deserialization.rs b/crates/rmcp/tests/test_deserialization.rs index 1f2191864..defdf377f 100644 --- a/crates/rmcp/tests/test_deserialization.rs +++ b/crates/rmcp/tests/test_deserialization.rs @@ -456,3 +456,83 @@ mod request_id { assert!(matches!(message, JsonRpcMessage::Notification(_))); } } + +/// `ClientRequest` is read by `method`: the variant whose method matches is the +/// one deserialized, a known method whose params do not fit becomes a request +/// the server answers with Invalid params, and only an unknown method is a +/// custom request (#1347). +mod client_request_by_method { + use rmcp::model::{ + ClientJsonRpcMessage, ClientRequest, GetMeta, JsonRpcMessage, ProtocolVersion, + }; + use serde_json::json; + + fn parse(mut message: serde_json::Value) -> ClientRequest { + message["jsonrpc"] = json!("2.0"); + message["id"] = json!(1); + match serde_json::from_value::(message).expect("a client message") { + JsonRpcMessage::Request(request) => request.request, + other => panic!("expected a request, got {other:?}"), + } + } + + #[test] + fn fitting_params_give_the_typed_variant() { + let request = + parse(json!({"method": "tools/call", "params": {"name": "echo", "arguments": {}}})); + assert!( + matches!(request, ClientRequest::CallToolRequest(_)), + "{request:?}" + ); + let request = parse(json!({"method": "tools/list"})); + assert!( + matches!(request, ClientRequest::ListToolsRequest(_)), + "{request:?}" + ); + let request = parse(json!({"method": "ping"})); + assert!( + matches!(request, ClientRequest::PingRequest(_)), + "{request:?}" + ); + } + + #[test] + fn unfitting_params_are_not_a_custom_request_and_keep_meta() { + let request = parse(json!({ + "method": "tools/call", + "params": { + "name": 7, + "arguments": {}, + "_meta": {"io.modelcontextprotocol/protocolVersion": "2026-07-28"} + } + })); + assert!( + matches!(request, ClientRequest::InvalidParams(_)), + "{request:?}" + ); + assert_eq!(request.method(), "tools/call"); + assert_eq!( + request.get_meta().protocol_version(), + Some(ProtocolVersion::V_2026_07_28) + ); + } + + #[test] + fn missing_params_on_a_method_that_needs_them_is_invalid_params() { + let request = parse(json!({"method": "prompts/get"})); + assert!( + matches!(request, ClientRequest::InvalidParams(_)), + "{request:?}" + ); + } + + #[test] + fn unknown_method_is_a_custom_request_with_its_params() { + let request = parse(json!({"method": "fixture/unknown", "params": {"name": 7}})); + let ClientRequest::CustomRequest(custom) = request else { + panic!("expected a custom request, got {request:?}"); + }; + assert_eq!(custom.method, "fixture/unknown"); + assert_eq!(custom.params, Some(json!({"name": 7}))); + } +} diff --git a/crates/rmcp/tests/test_stateless_server_requests.rs b/crates/rmcp/tests/test_stateless_server_requests.rs index abec4d3df..4479f11e1 100644 --- a/crates/rmcp/tests/test_stateless_server_requests.rs +++ b/crates/rmcp/tests/test_stateless_server_requests.rs @@ -325,3 +325,132 @@ async fn stateless_server_first_request_handler_can_send_notifications() { .await .expect("cancel server"); } + +/// SEP-2575 and #1347 together: on the 2026-07-28 lifecycle a handshake method +/// is answered Method not found whether or not its params fit, while a method +/// that exists but whose params do not fit is answered Invalid params. +#[tokio::test] +async fn inline_lifecycle_tells_removed_methods_from_invalid_params() { + let (server_transport, client_transport) = tokio::io::duplex(4096); + let server_task = tokio::spawn(async move { + StatelessServer + .serve(server_transport) + .await + .expect("server should start") + }); + let mut client = IntoTransport::::into_transport(client_transport); + + let mut discover = DiscoverRequest::new(DiscoverRequestParams {}); + discover.extensions.insert(complete_meta()); + client + .send(ClientJsonRpcMessage::request( + ClientRequest::DiscoverRequest(discover), + RequestId::Number(1), + )) + .await + .expect("send discover"); + assert!(matches!( + client.receive().await, + Some(ServerJsonRpcMessage::Response(_)) + )); + let running = server_task.await.expect("server task"); + + let meta = serde_json::to_value(complete_meta()).expect("meta"); + let cases = [ + // Removed on this lifecycle: not found even with params that fit. + ( + "logging/setLevel", + serde_json::json!({"level": "debug"}), + ErrorCode::METHOD_NOT_FOUND, + ), + ("ping", serde_json::json!({}), ErrorCode::METHOD_NOT_FOUND), + ( + "resources/subscribe", + serde_json::json!({"uri": "file:///a"}), + ErrorCode::METHOD_NOT_FOUND, + ), + ( + "initialize", + serde_json::json!({"protocolVersion": "2025-11-25", "capabilities": {}, "clientInfo": {"name": "c", "version": "1"}}), + ErrorCode::METHOD_NOT_FOUND, + ), + // Removed, and the params do not fit either: still not found. + ( + "logging/setLevel", + serde_json::json!({"level": 7}), + ErrorCode::METHOD_NOT_FOUND, + ), + // Served on this lifecycle, params do not fit: invalid params. + ( + "tools/call", + serde_json::json!({"name": 7, "arguments": {}}), + ErrorCode::INVALID_PARAMS, + ), + ( + "prompts/get", + serde_json::json!({"name": 7}), + ErrorCode::INVALID_PARAMS, + ), + ]; + for (i, (method, mut params, code)) in cases.into_iter().enumerate() { + params["_meta"] = meta.clone(); + let message: ClientJsonRpcMessage = serde_json::from_value(serde_json::json!({ + "jsonrpc": "2.0", "id": i as i64 + 2, "method": method, "params": params + })) + .expect("client message"); + client.send(message).await.expect("send request"); + let Some(ServerJsonRpcMessage::Error(error)) = client.receive().await else { + panic!("{method}: expected an error"); + }; + assert_eq!(error.error.code, code, "{method}: {}", error.error.message); + assert!( + error.error.message.contains(method), + "{method}: message {:?} does not name the method", + error.error.message + ); + } + + running.cancel().await.expect("cancel server"); +} + +/// Before the handshake, an `initialize` whose params do not fit is answered +/// Invalid params and the server keeps waiting; a proper one still completes it. +#[tokio::test] +async fn malformed_initialize_before_handshake_is_invalid_params() { + let (server_transport, client_transport) = tokio::io::duplex(4096); + let server_task = tokio::spawn(async move { + StatelessServer + .serve(server_transport) + .await + .expect("server should start") + }); + let mut client = IntoTransport::::into_transport(client_transport); + + let bad: ClientJsonRpcMessage = serde_json::from_value(serde_json::json!({ + "jsonrpc": "2.0", "id": 1, "method": "initialize", "params": {"protocolVersion": 7} + })) + .expect("client message"); + client.send(bad).await.expect("send malformed initialize"); + let Some(ServerJsonRpcMessage::Error(error)) = client.receive().await else { + panic!("expected invalid params"); + }; + assert_eq!(error.error.code, ErrorCode::INVALID_PARAMS); + assert!( + error.error.message.contains("initialize"), + "{}", + error.error.message + ); + + let good: ClientJsonRpcMessage = serde_json::from_value(serde_json::json!({ + "jsonrpc": "2.0", "id": 2, "method": "initialize", + "params": {"protocolVersion": "2025-11-25", "capabilities": {}, "clientInfo": {"name": "c", "version": "1"}} + })) + .expect("client message"); + client.send(good).await.expect("send initialize"); + assert!(matches!( + client.receive().await, + Some(ServerJsonRpcMessage::Response(_)) + )); + let running = server_task.await.expect("server task"); + running.cancel().await.expect("cancel server"); +} From 4b8df138e45b5f76a430a1964a33fad6d8b7b14d Mon Sep 17 00:00:00 2001 From: "Vit.Bogachev" Date: Mon, 19 Oct 2026 20:19:22 +0300 Subject: [PATCH 4/4] fix(model): keep the derived name of ClientRequest::deserialize's type parameter The hand-written `Deserialize` named its type parameter `D`, where `#[derive(Deserialize)]` had named it `__D`. The signature is the same, but the Public API Check compares the rendered item, so it read as a changed public item and the job failed on a minor release. Co-Authored-By: Claude Opus 5 --- crates/rmcp/src/model.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/rmcp/src/model.rs b/crates/rmcp/src/model.rs index 67d393743..221c52cb9 100644 --- a/crates/rmcp/src/model.rs +++ b/crates/rmcp/src/model.rs @@ -4887,9 +4887,12 @@ macro_rules! ts_union { }; (@impl_deserialize_by_method $U:ident { $($variant:tt)* }) => { impl<'de> Deserialize<'de> for $U { - fn deserialize(deserializer: D) -> Result + // `__D` rather than `D`: `#[derive(Deserialize)]` named it that, and + // the public-API check compares the rendered signature, so a plain + // `D` reads as a changed public item (CI "Public API Check"). + fn deserialize<__D>(deserializer: __D) -> Result where - D: serde::Deserializer<'de>, + __D: serde::Deserializer<'de>, { // The body is buffered once; `method` picks the one variant to read. let body = Value::Object(JsonObject::deserialize(deserializer)?);