From 0c16d31563052c2e7447892501d1fc7e00e41b3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20M=2E=20Requena=20Plens?= Date: Sat, 26 Sep 2026 02:11:52 +0200 Subject: [PATCH 1/2] fix(model): decode float fields through serde_json::Number When serde_json's arbitrary_precision feature is enabled anywhere in a build, serde replays a buffered decimal as serde_json's private number map, and a plain f32 or f64 field rejects it. Read the model's float fields through serde_json::Number, which accepts both forms. rmcp's dev-dependencies enable the feature, so the tests run with it. Refs openai/codex#38979 --- crates/rmcp/Cargo.toml | 3 + crates/rmcp/src/model.rs | 31 ++- crates/rmcp/src/model/annotated.rs | 6 +- crates/rmcp/src/model/elicitation_schema.rs | 18 +- crates/rmcp/src/model/serde_impl.rs | 204 ++++++++++++++++++++ crates/rmcp/tests/test_deserialization.rs | 126 ++++++++++++ 6 files changed, 379 insertions(+), 9 deletions(-) diff --git a/crates/rmcp/Cargo.toml b/crates/rmcp/Cargo.toml index ac698f976..772844d86 100644 --- a/crates/rmcp/Cargo.toml +++ b/crates/rmcp/Cargo.toml @@ -216,6 +216,9 @@ tracing-subscriber = { version = "0.3", features = [ ] } async-trait = "0.1" rstest = "0.27.0" +# Cargo unifies this feature into rmcp whenever any crate in a build enables it, +# so the tests run with it on (see tests/test_deserialization.rs). +serde_json = { version = "1.0", features = ["arbitrary_precision"] } [[test]] name = "test_tool_macros" required-features = ["server", "client"] diff --git a/crates/rmcp/src/model.rs b/crates/rmcp/src/model.rs index a639bfb1c..5d326c020 100644 --- a/crates/rmcp/src/model.rs +++ b/crates/rmcp/src/model.rs @@ -1592,9 +1592,14 @@ const_string!(ProgressNotificationMethod = "notifications/progress"); pub struct ProgressNotificationParam { pub progress_token: ProgressToken, /// The progress thus far. This should increase every time progress is made, even if the total is unknown. + #[serde(deserialize_with = "serde_impl::json_float::f64")] pub progress: f64, /// Total number of items to process (or total progress required), if known - #[serde(skip_serializing_if = "Option::is_none")] + #[serde( + default, + deserialize_with = "serde_impl::json_float::option_f64", + skip_serializing_if = "Option::is_none" + )] pub total: Option, /// An optional message describing the current progress. #[serde(skip_serializing_if = "Option::is_none")] @@ -2995,7 +3000,11 @@ pub struct CreateMessageRequestParams { #[serde(skip_serializing_if = "Option::is_none")] pub include_context: Option, /// Temperature for controlling randomness (0.0 to 1.0) - #[serde(skip_serializing_if = "Option::is_none")] + #[serde( + default, + deserialize_with = "serde_impl::json_float::option_f32", + skip_serializing_if = "Option::is_none" + )] pub temperature: Option, /// Maximum number of tokens to generate pub max_tokens: u32, @@ -3184,13 +3193,25 @@ pub struct ModelPreferences { #[serde(skip_serializing_if = "Option::is_none")] pub hints: Option>, /// Priority for cost optimization (0.0 to 1.0, higher = prefer cheaper models) - #[serde(skip_serializing_if = "Option::is_none")] + #[serde( + default, + deserialize_with = "serde_impl::json_float::option_f32", + skip_serializing_if = "Option::is_none" + )] pub cost_priority: Option, /// Priority for speed/latency (0.0 to 1.0, higher = prefer faster models) - #[serde(skip_serializing_if = "Option::is_none")] + #[serde( + default, + deserialize_with = "serde_impl::json_float::option_f32", + skip_serializing_if = "Option::is_none" + )] pub speed_priority: Option, /// Priority for intelligence/capability (0.0 to 1.0, higher = prefer more capable models) - #[serde(skip_serializing_if = "Option::is_none")] + #[serde( + default, + deserialize_with = "serde_impl::json_float::option_f32", + skip_serializing_if = "Option::is_none" + )] pub intelligence_priority: Option, } diff --git a/crates/rmcp/src/model/annotated.rs b/crates/rmcp/src/model/annotated.rs index ac3dbd3ed..65b31a1a3 100644 --- a/crates/rmcp/src/model/annotated.rs +++ b/crates/rmcp/src/model/annotated.rs @@ -17,7 +17,11 @@ use super::Role; pub struct Annotations { #[serde(skip_serializing_if = "Option::is_none")] pub audience: Option>, - #[serde(skip_serializing_if = "Option::is_none")] + #[serde( + default, + deserialize_with = "super::serde_impl::json_float::option_f32", + skip_serializing_if = "Option::is_none" + )] pub priority: Option, #[serde(skip_serializing_if = "Option::is_none", rename = "lastModified")] pub last_modified: Option, diff --git a/crates/rmcp/src/model/elicitation_schema.rs b/crates/rmcp/src/model/elicitation_schema.rs index 486108719..7a556e847 100644 --- a/crates/rmcp/src/model/elicitation_schema.rs +++ b/crates/rmcp/src/model/elicitation_schema.rs @@ -256,15 +256,27 @@ pub struct NumberSchema { pub description: Option>, /// Minimum value (inclusive) - #[serde(skip_serializing_if = "Option::is_none")] + #[serde( + default, + deserialize_with = "super::serde_impl::json_float::option_f64", + skip_serializing_if = "Option::is_none" + )] pub minimum: Option, /// Maximum value (inclusive) - #[serde(skip_serializing_if = "Option::is_none")] + #[serde( + default, + deserialize_with = "super::serde_impl::json_float::option_f64", + skip_serializing_if = "Option::is_none" + )] pub maximum: Option, /// Default value - #[serde(skip_serializing_if = "Option::is_none")] + #[serde( + default, + deserialize_with = "super::serde_impl::json_float::option_f64", + skip_serializing_if = "Option::is_none" + )] pub default: Option, } diff --git a/crates/rmcp/src/model/serde_impl.rs b/crates/rmcp/src/model/serde_impl.rs index bfd3b5783..2190c8229 100644 --- a/crates/rmcp/src/model/serde_impl.rs +++ b/crates/rmcp/src/model/serde_impl.rs @@ -8,6 +8,49 @@ use super::{ RequestOptionalParam, }; +/// Deserializers for the model's float fields. +/// +/// Serde buffers a value whose type it does not know yet (untagged and +/// internally tagged enums, `#[serde(flatten)]`), and every [`JsonRpcMessage`] +/// takes that path. With serde_json's `arbitrary_precision` feature, which +/// Cargo turns on for every crate in a build once any crate enables it, a +/// buffered decimal is replayed as serde_json's private number map, and a +/// plain `f32`/`f64` field rejects it (serde-rs/json#721). +/// [`serde_json::Number`] reads both forms. +/// +/// [`JsonRpcMessage`]: super::JsonRpcMessage +pub(crate) mod json_float { + use serde::{Deserialize, Deserializer, de::Error}; + use serde_json::Number; + + fn to_f64(number: Number) -> Result { + // `None` only for a value beyond `f64`, which serde_json also rejects + // without the feature. + number + .as_f64() + .ok_or_else(|| E::custom(format_args!("number out of range: {number}"))) + } + + pub(crate) fn f64<'de, D: Deserializer<'de>>(deserializer: D) -> Result { + to_f64(Number::deserialize(deserializer)?) + } + + pub(crate) fn option_f64<'de, D: Deserializer<'de>>( + deserializer: D, + ) -> Result, D::Error> { + Option::::deserialize(deserializer)? + .map(to_f64) + .transpose() + } + + pub(crate) fn option_f32<'de, D: Deserializer<'de>>( + deserializer: D, + ) -> Result, D::Error> { + // serde_json also reads an `f32` field as `f64` and narrows it with `as`. + Ok(option_f64(deserializer)?.map(|value| value as f32)) + } +} + /// Wire-side view of `params`: the `_meta` map plus the remaining fields. /// /// All metadata types are transparent wrappers over [`JsonObject`], so the @@ -848,4 +891,165 @@ mod test { let output = serde_json::to_value(&req).unwrap(); assert_eq!(input, output); } + + /// The float fields read through [`super::json_float`]. + mod json_float { + use std::fmt::Debug; + + use rstest::rstest; + use serde::{Deserialize, Serialize, de::DeserializeOwned}; + use serde_json::{Value, json}; + + use crate::model::{ + Annotations, CreateMessageRequestParams, ModelPreferences, NumberSchema, + ProgressNotificationParam, + }; + + /// Untagged, so serde buffers the input before `T` reads it, as it + /// does for every JSON-RPC message. + #[derive(Deserialize)] + #[serde(untagged)] + enum Buffered { + Inner(T), + } + + /// Decodes `text` directly and through serde's buffer, checks that both + /// agree, and that the value survives a round trip. + fn decode(text: &str) -> T + where + T: DeserializeOwned + Serialize + PartialEq + Debug, + { + let direct: T = serde_json::from_str(text).unwrap(); + let Buffered::Inner(buffered) = serde_json::from_str(text).unwrap(); + assert_eq!(direct, buffered, "{text}"); + let encoded = serde_json::to_string(&direct).unwrap(); + assert_eq!( + serde_json::from_str::(&encoded).unwrap(), + direct, + "{encoded}" + ); + direct + } + + fn rejects(text: &str) { + assert!(serde_json::from_str::(text).is_err(), "{text}"); + assert!(serde_json::from_str::>(text).is_err(), "{text}"); + } + + #[rstest] + #[case::decimal("0.6", 0.6)] + #[case::trailing_zero("0.60", 0.6)] + #[case::exponent("6e-1", 0.6)] + #[case::integer("1", 1.0)] + #[case::integral_decimal("1.0", 1.0)] + #[case::zero("0", 0.0)] + fn fields_read_every_spelling(#[case] n: &str, #[case] expected: f64) { + // An `f32` field holds the `f64` narrowed, as serde_json reads it. + let narrowed = Some(expected as f32); + + let annotations: Annotations = decode(&format!(r#"{{"priority":{n}}}"#)); + assert_eq!(annotations.priority, narrowed); + + let progress: ProgressNotificationParam = decode(&format!( + r#"{{"progressToken":1,"progress":{n},"total":{n}}}"# + )); + assert_eq!(progress.progress, expected); + assert_eq!(progress.total, Some(expected)); + + let params: CreateMessageRequestParams = decode(&format!( + r#"{{"messages":[],"maxTokens":1,"temperature":{n}}}"# + )); + assert_eq!(params.temperature, narrowed); + + let preferences: ModelPreferences = decode(&format!( + r#"{{"costPriority":{n},"speedPriority":{n},"intelligencePriority":{n}}}"# + )); + assert_eq!(preferences.cost_priority, narrowed); + assert_eq!(preferences.speed_priority, narrowed); + assert_eq!(preferences.intelligence_priority, narrowed); + + let schema: NumberSchema = decode(&format!( + r#"{{"type":"number","minimum":{n},"maximum":{n},"default":{n}}}"# + )); + assert_eq!(schema.minimum, Some(expected)); + assert_eq!(schema.maximum, Some(expected)); + assert_eq!(schema.default, Some(expected)); + } + + #[test] + fn decimal_narrows_to_the_nearest_f32() { + let annotations: Annotations = decode(r#"{"priority":0.6}"#); + assert_eq!(annotations.priority, Some(0.6_f32)); + } + + #[rstest] + #[case::missing(None)] + #[case::null(Some(Value::Null))] + fn optional_fields_read_absent_as_none(#[case] value: Option) { + // `base` with each of `keys` set to `value`, or left out. + let with = |mut base: Value, keys: &[&str]| { + if let Some(value) = &value { + for key in keys { + base[*key] = value.clone(); + } + } + base.to_string() + }; + + let annotations: Annotations = decode(&with(json!({}), &["priority"])); + assert_eq!(annotations, Annotations::default()); + + let progress: ProgressNotificationParam = decode(&with( + json!({"progressToken": 1, "progress": 0}), + &["total"], + )); + assert_eq!(progress.total, None); + + let params: CreateMessageRequestParams = decode(&with( + json!({"messages": [], "maxTokens": 1}), + &["temperature"], + )); + assert_eq!(params.temperature, None); + + let preferences: ModelPreferences = decode(&with( + json!({}), + &["costPriority", "speedPriority", "intelligencePriority"], + )); + assert_eq!(preferences, ModelPreferences::new()); + + let schema: NumberSchema = decode(&with( + json!({"type": "number"}), + &["minimum", "maximum", "default"], + )); + assert_eq!(schema, NumberSchema::new()); + } + + #[test] + fn progress_is_required() { + rejects::(r#"{"progressToken":1}"#); + rejects::(r#"{"progressToken":1,"progress":null}"#); + } + + #[rstest] + #[case::string(r#""high""#)] + #[case::out_of_range("1e400")] + fn fields_reject_anything_but_a_finite_number(#[case] n: &str) { + rejects::(&format!(r#"{{"priority":{n}}}"#)); + rejects::(&format!( + r#"{{"progressToken":1,"progress":{n}}}"# + )); + rejects::(&format!( + r#"{{"progressToken":1,"progress":0,"total":{n}}}"# + )); + rejects::(&format!( + r#"{{"messages":[],"maxTokens":1,"temperature":{n}}}"# + )); + for key in ["costPriority", "speedPriority", "intelligencePriority"] { + rejects::(&format!(r#"{{"{key}":{n}}}"#)); + } + for key in ["minimum", "maximum", "default"] { + rejects::(&format!(r#"{{"type":"number","{key}":{n}}}"#)); + } + } + } } diff --git a/crates/rmcp/tests/test_deserialization.rs b/crates/rmcp/tests/test_deserialization.rs index be457d142..bac8e180b 100644 --- a/crates/rmcp/tests/test_deserialization.rs +++ b/crates/rmcp/tests/test_deserialization.rs @@ -271,3 +271,129 @@ mod untagged_server_result { assert!(matches!(result, ServerResult::CallToolResult(_))); } } + +/// Regression tests for decimal float fields under serde_json's +/// `arbitrary_precision` feature, which rmcp's dev-dependencies enable. +/// +/// With the feature, serde replays a decimal it buffered for an untagged enum +/// as serde_json's private number map. A plain `f32`/`f64` field rejected that +/// map, so each of these messages fell through to a `Custom*` variant. They are +/// decoded from text, which is where the map comes from. +mod arbitrary_precision { + use rmcp::model::{ + ClientJsonRpcMessage, ClientNotification, ElicitRequestParams, JsonRpcMessage, + JsonRpcNotification, JsonRpcRequest, JsonRpcResponse, PrimitiveSchemaDefinition, + ServerJsonRpcMessage, ServerNotification, ServerRequest, ServerResult, + }; + + /// Decodes a server message the way a client does (`RxJsonRpcMessage`). + fn from_server(text: &str) -> ServerJsonRpcMessage { + serde_json::from_str(text).unwrap() + } + + #[test] + fn feature_is_enabled() { + // Only the feature keeps the number as written; without it this reads `0.6`. + let value: serde_json::Value = serde_json::from_str("0.60").unwrap(); + assert_eq!(value.to_string(), "0.60"); + } + + #[test] + fn call_tool_result_with_fractional_priority() { + let message = from_server( + r#"{"jsonrpc":"2.0","id":1,"result":{"content":[ + {"type":"text","text":"ok","annotations":{"priority":0.6}}]}}"#, + ); + let JsonRpcMessage::Response(JsonRpcResponse { + result: ServerResult::CallToolResult(result), + .. + }) = message + else { + panic!("expected CallToolResult, got {message:?}"); + }; + let annotations = result.content[0].as_text().unwrap().annotations.as_ref(); + assert_eq!(annotations.unwrap().priority, Some(0.6)); + } + + #[test] + fn progress_notification_with_fractional_progress() { + let text = r#"{"jsonrpc":"2.0","method":"notifications/progress", + "params":{"progressToken":"t","progress":0.5,"total":2.5}}"#; + + let message = from_server(text); + let JsonRpcMessage::Notification(JsonRpcNotification { + notification: ServerNotification::ProgressNotification(notification), + .. + }) = message + else { + panic!("expected ProgressNotification, got {message:?}"); + }; + assert_eq!(notification.params.progress, 0.5); + assert_eq!(notification.params.total, Some(2.5)); + + let message: ClientJsonRpcMessage = serde_json::from_str(text).unwrap(); + assert!( + matches!( + message, + JsonRpcMessage::Notification(JsonRpcNotification { + notification: ClientNotification::ProgressNotification(_), + .. + }) + ), + "expected ProgressNotification, got {message:?}" + ); + } + + #[test] + #[expect(deprecated, reason = "sampling is deprecated by SEP-2577")] + fn create_message_request_with_fractional_parameters() { + let message = from_server( + r#"{"jsonrpc":"2.0","id":2,"method":"sampling/createMessage","params":{ + "messages":[],"maxTokens":10,"temperature":0.7,"modelPreferences":{ + "costPriority":0.2,"speedPriority":0.5,"intelligencePriority":0.9}}}"#, + ); + let JsonRpcMessage::Request(JsonRpcRequest { + request: ServerRequest::CreateMessageRequest(request), + .. + }) = message + else { + panic!("expected CreateMessageRequest, got {message:?}"); + }; + assert_eq!(request.params.temperature, Some(0.7)); + let preferences = request.params.model_preferences.unwrap(); + assert_eq!(preferences.cost_priority, Some(0.2)); + assert_eq!(preferences.speed_priority, Some(0.5)); + assert_eq!(preferences.intelligence_priority, Some(0.9)); + } + + #[test] + fn elicit_request_with_fractional_number_schema() { + let message = from_server( + r#"{"jsonrpc":"2.0","id":3,"method":"elicitation/create","params":{ + "mode":"form","message":"How much?","requestedSchema":{"type":"object", + "properties":{"amount":{"type":"number", + "minimum":0.5,"maximum":9.5,"default":1.5}}}}}"#, + ); + let JsonRpcMessage::Request(JsonRpcRequest { + request: ServerRequest::ElicitRequest(request), + .. + }) = message + else { + panic!("expected ElicitRequest, got {message:?}"); + }; + let ElicitRequestParams::FormElicitationParams { + requested_schema, .. + } = request.params + else { + panic!("expected a form elicitation"); + }; + let Some(PrimitiveSchemaDefinition::Number(amount)) = + requested_schema.properties.get("amount") + else { + panic!("expected a number schema"); + }; + assert_eq!(amount.minimum, Some(0.5)); + assert_eq!(amount.maximum, Some(9.5)); + assert_eq!(amount.default, Some(1.5)); + } +} From 2c3266edee069d6606511c6f42c2c508b207fb70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20M=2E=20Requena=20Plens?= Date: Sat, 26 Sep 2026 21:20:55 +0200 Subject: [PATCH 2/2] docs(model): shorten the json_float module comment --- crates/rmcp/src/model/serde_impl.rs | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/crates/rmcp/src/model/serde_impl.rs b/crates/rmcp/src/model/serde_impl.rs index 2190c8229..6ce30eb99 100644 --- a/crates/rmcp/src/model/serde_impl.rs +++ b/crates/rmcp/src/model/serde_impl.rs @@ -8,17 +8,9 @@ use super::{ RequestOptionalParam, }; -/// Deserializers for the model's float fields. -/// -/// Serde buffers a value whose type it does not know yet (untagged and -/// internally tagged enums, `#[serde(flatten)]`), and every [`JsonRpcMessage`] -/// takes that path. With serde_json's `arbitrary_precision` feature, which -/// Cargo turns on for every crate in a build once any crate enables it, a -/// buffered decimal is replayed as serde_json's private number map, and a -/// plain `f32`/`f64` field rejects it (serde-rs/json#721). -/// [`serde_json::Number`] reads both forms. -/// -/// [`JsonRpcMessage`]: super::JsonRpcMessage +/// Float deserializers that also accept the number map serde_json's +/// `arbitrary_precision` feature produces for buffered (untagged/flattened) values. +/// Like JSON itself, they reject NaN and infinities. pub(crate) mod json_float { use serde::{Deserialize, Deserializer, de::Error}; use serde_json::Number;