Skip to content

Commit 601a804

Browse files
committed
fix: address metadata review feedback
1 parent aa17a9f commit 601a804

7 files changed

Lines changed: 142 additions & 60 deletions

crates/rmcp/src/model/meta.rs

Lines changed: 48 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -259,10 +259,12 @@ pub struct MetaObject(pub JsonObject);
259259

260260
/// Deprecated alias for [`MetaObject`].
261261
///
262-
/// Note: request and notification metadata now have dedicated types; use
263-
/// [`RequestMetaObject`] or [`NotificationMetaObject`] where those are expected.
262+
/// This is a re-export rather than a type alias so the `Meta(...)` tuple
263+
/// constructor keeps working. Request and notification metadata now have
264+
/// dedicated types; use [`RequestMetaObject`] or [`NotificationMetaObject`]
265+
/// where those are expected.
264266
#[deprecated(note = "Use MetaObject (or RequestMetaObject / NotificationMetaObject)")]
265-
pub type Meta = MetaObject;
267+
pub use self::MetaObject as Meta;
266268

267269
impl MetaObject {
268270
/// Reserved `_meta` key for the W3C Trace Context `traceparent` value (SEP-414).
@@ -390,14 +392,15 @@ impl schemars::JsonSchema for MetaObject {
390392
///
391393
/// In addition to arbitrary extension keys, requests reserve:
392394
/// - `progressToken` for progress tracking
393-
/// - `io.modelcontextprotocol/protocolVersion` (SEP-1319)
394-
/// - `io.modelcontextprotocol/clientInfo` (SEP-1319)
395-
/// - `io.modelcontextprotocol/clientCapabilities` (SEP-1319)
396-
/// - `io.modelcontextprotocol/logLevel` (SEP-1319)
395+
/// - `io.modelcontextprotocol/protocolVersion` (SEP-2575)
396+
/// - `io.modelcontextprotocol/clientInfo` (SEP-2575)
397+
/// - `io.modelcontextprotocol/clientCapabilities` (SEP-2575)
398+
/// - `io.modelcontextprotocol/logLevel` (SEP-2575)
397399
///
398400
/// The 2026-07-28 draft schema marks the protocol-version, client-info, and
399401
/// client-capabilities keys as required; earlier protocol versions do not know
400-
/// them. All keys therefore stay optional at runtime — use
402+
/// them. All keys therefore stay optional at runtime and in the generated
403+
/// (version-shared) JSON schema — use
401404
/// [`RequestMetaObject::missing_required_keys`] to validate a request against
402405
/// the negotiated protocol version.
403406
///
@@ -496,8 +499,12 @@ impl RequestMetaObject {
496499
.insert_serialized(Self::META_KEY_LOG_LEVEL, log_level);
497500
}
498501

499-
/// Return the [`Self::DRAFT_REQUIRED_KEYS`] absent from this map, if
500-
/// `protocol_version` requires them.
502+
/// Return the [`Self::DRAFT_REQUIRED_KEYS`] whose values are absent or
503+
/// invalid in this map, if `protocol_version` requires them.
504+
///
505+
/// A key counts as missing when it is not present *or* when its value does
506+
/// not decode into the expected type (e.g. a numeric `protocolVersion` or
507+
/// a string `clientInfo`), matching what the typed accessors return.
501508
///
502509
/// Protocol versions before 2026-07-28 have no required request metadata,
503510
/// so this always returns an empty list for them.
@@ -513,7 +520,7 @@ impl RequestMetaObject {
513520
/// meta.missing_required_keys(&ProtocolVersion::V_2025_11_25)
514521
/// .is_empty()
515522
/// );
516-
/// // The 2026-07-28 draft requires the SEP-1319 keys.
523+
/// // The 2026-07-28 draft requires the SEP-2575 keys.
517524
/// assert_eq!(
518525
/// meta.missing_required_keys(&ProtocolVersion::V_2026_07_28),
519526
/// RequestMetaObject::DRAFT_REQUIRED_KEYS.to_vec(),
@@ -523,10 +530,17 @@ impl RequestMetaObject {
523530
if protocol_version.as_str() < ProtocolVersion::V_2026_07_28.as_str() {
524531
return Vec::new();
525532
}
526-
Self::DRAFT_REQUIRED_KEYS
527-
.into_iter()
528-
.filter(|key| !self.0.0.contains_key(*key))
529-
.collect()
533+
let mut missing = Vec::new();
534+
if self.protocol_version().is_none() {
535+
missing.push(Self::META_KEY_PROTOCOL_VERSION);
536+
}
537+
if self.client_info().is_none() {
538+
missing.push(Self::META_KEY_CLIENT_INFO);
539+
}
540+
if self.client_capabilities().is_none() {
541+
missing.push(Self::META_KEY_CLIENT_CAPABILITIES);
542+
}
543+
missing
530544
}
531545

532546
/// Insert every entry of `other`, overwriting existing keys on conflict.
@@ -572,6 +586,11 @@ impl schemars::JsonSchema for RequestMetaObject {
572586
let client_info = generator.subschema_for::<Implementation>();
573587
let client_capabilities = generator.subschema_for::<ClientCapabilities>();
574588
let log_level = generator.subschema_for::<LoggingLevel>();
589+
// rmcp generates one schema shared by every supported protocol
590+
// version, so the keys the 2026-07-28 draft marks as required are left
591+
// optional here: a 2025-11-25 request whose `_meta` only carries
592+
// `progressToken` is valid. Draft-strict validation is available at
593+
// runtime via [`RequestMetaObject::missing_required_keys`].
575594
schemars::json_schema!({
576595
"description": "Metadata reserved by MCP on requests. Extension keys are also allowed.",
577596
"type": "object",
@@ -584,11 +603,6 @@ impl schemars::JsonSchema for RequestMetaObject {
584603
"io.modelcontextprotocol/clientCapabilities": client_capabilities,
585604
"io.modelcontextprotocol/logLevel": log_level,
586605
},
587-
"required": [
588-
"io.modelcontextprotocol/protocolVersion",
589-
"io.modelcontextprotocol/clientInfo",
590-
"io.modelcontextprotocol/clientCapabilities",
591-
],
592606
"additionalProperties": true,
593607
})
594608
}
@@ -831,6 +845,20 @@ mod tests {
831845
);
832846
}
833847

848+
#[test]
849+
fn treats_malformed_values_as_missing() {
850+
let meta: RequestMetaObject = serde_json::from_value(serde_json::json!({
851+
"io.modelcontextprotocol/protocolVersion": 123,
852+
"io.modelcontextprotocol/clientInfo": "not an implementation",
853+
"io.modelcontextprotocol/clientCapabilities": null,
854+
}))
855+
.unwrap();
856+
assert_eq!(
857+
meta.missing_required_keys(&ProtocolVersion::V_2026_07_28),
858+
RequestMetaObject::DRAFT_REQUIRED_KEYS.to_vec()
859+
);
860+
}
861+
834862
#[test]
835863
fn is_empty_when_draft_keys_are_present() {
836864
let mut meta = RequestMetaObject::new();

crates/rmcp/src/model/serde_impl.rs

Lines changed: 83 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::borrow::Cow;
33
use serde::{Deserialize, Serialize};
44

55
use super::{
6-
CustomNotification, CustomRequest, Extensions, JsonObject, Notification,
6+
CustomNotification, CustomRequest, Extensions, JsonObject, MetaObject, Notification,
77
NotificationMetaObject, NotificationNoParam, Request, RequestMetaObject, RequestNoParam,
88
RequestOptionalParam,
99
};
@@ -85,18 +85,39 @@ struct ProxyNoParam<M> {
8585
method: M,
8686
}
8787

88+
/// Combine the message-specific `_meta` map with a legacy [`MetaObject`]
89+
/// extension (inserted through the deprecated `Meta` name), so pre-3.x code
90+
/// does not silently lose metadata on the wire. On key conflicts the
91+
/// message-specific map wins.
92+
fn merge_legacy_meta<'a>(
93+
typed: Option<&'a JsonObject>,
94+
extensions: &'a Extensions,
95+
) -> Option<Cow<'a, JsonObject>> {
96+
let legacy = extensions.get::<MetaObject>().map(|meta| &meta.0);
97+
match (typed, legacy) {
98+
(Some(typed), None) => Some(Cow::Borrowed(typed)),
99+
(None, Some(legacy)) => Some(Cow::Borrowed(legacy)),
100+
(Some(typed), Some(legacy)) => {
101+
let mut merged = legacy.clone();
102+
merged.extend(typed.clone());
103+
Some(Cow::Owned(merged))
104+
}
105+
(None, None) => None,
106+
}
107+
}
108+
88109
/// Borrow the request `_meta` map from extensions, if any.
89110
fn request_meta(extensions: &Extensions) -> Option<Cow<'_, JsonObject>> {
90-
extensions
91-
.get::<RequestMetaObject>()
92-
.map(|meta| Cow::Borrowed(&meta.0.0))
111+
let typed = extensions.get::<RequestMetaObject>().map(|meta| &meta.0.0);
112+
merge_legacy_meta(typed, extensions)
93113
}
94114

95115
/// Borrow the notification `_meta` map from extensions, if any.
96116
fn notification_meta(extensions: &Extensions) -> Option<Cow<'_, JsonObject>> {
97-
extensions
117+
let typed = extensions
98118
.get::<NotificationMetaObject>()
99-
.map(|meta| Cow::Borrowed(&meta.0.0))
119+
.map(|meta| &meta.0.0);
120+
merge_legacy_meta(typed, extensions)
100121
}
101122

102123
/// Build extensions holding a typed metadata map deserialized from `params._meta`.
@@ -761,6 +782,62 @@ mod test {
761782
let _req: PingRequest = serde_json::from_value(json!({"method": "ping"})).unwrap();
762783
}
763784

785+
#[test]
786+
fn test_legacy_meta_extension_still_serializes() {
787+
// Pre-3.x code inserts `MetaObject` into extensions through the
788+
// deprecated `Meta` name; its metadata must not be silently dropped.
789+
let mut extensions = Extensions::new();
790+
let mut legacy = crate::model::MetaObject::new();
791+
legacy.insert("traceId".to_string(), json!("legacy"));
792+
extensions.insert(legacy);
793+
794+
let req = CallToolRequest {
795+
extensions,
796+
method: Default::default(),
797+
params: CallToolRequestParams {
798+
meta: None,
799+
name: "my_tool".into(),
800+
arguments: None,
801+
task: None,
802+
input_responses: None,
803+
request_state: None,
804+
},
805+
};
806+
807+
let value = serde_json::to_value(&req).unwrap();
808+
assert_eq!(value["params"]["_meta"]["traceId"], json!("legacy"));
809+
}
810+
811+
#[test]
812+
fn test_typed_meta_wins_over_legacy_extension_on_conflict() {
813+
let mut extensions = Extensions::new();
814+
let mut legacy = crate::model::MetaObject::new();
815+
legacy.insert("shared".to_string(), json!("legacy"));
816+
legacy.insert("legacy_only".to_string(), json!("kept"));
817+
extensions.insert(legacy);
818+
let mut typed = RequestMetaObject::new();
819+
typed.insert("shared".to_string(), json!("typed"));
820+
extensions.insert(typed);
821+
822+
let req = CallToolRequest {
823+
extensions,
824+
method: Default::default(),
825+
params: CallToolRequestParams {
826+
meta: None,
827+
name: "my_tool".into(),
828+
arguments: None,
829+
task: None,
830+
input_responses: None,
831+
request_state: None,
832+
},
833+
};
834+
835+
let value = serde_json::to_value(&req).unwrap();
836+
let meta = value["params"]["_meta"].as_object().unwrap();
837+
assert_eq!(meta.get("shared").unwrap(), "typed");
838+
assert_eq!(meta.get("legacy_only").unwrap(), "kept");
839+
}
840+
764841
#[test]
765842
fn test_arbitrary_meta_keys_round_trip_unchanged() {
766843
let input = json!({

crates/rmcp/tests/test_message_schema.rs

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,13 @@ mod tests {
6161
);
6262
}
6363

64-
/// The three metadata definitions must match the MCP 2026-07-28 draft
65-
/// schema exactly: `MetaObject` is an open map, `RequestMetaObject`
66-
/// reserves `progressToken` plus the SEP-1319 keys (three of which the
67-
/// draft marks required), and `NotificationMetaObject` reserves
68-
/// `io.modelcontextprotocol/subscriptionId`.
64+
/// The three metadata definitions must expose the MCP 2026-07-28 draft
65+
/// vocabulary: `MetaObject` is an open map, `RequestMetaObject` reserves
66+
/// `progressToken` plus the SEP-2575 keys, and `NotificationMetaObject`
67+
/// reserves `io.modelcontextprotocol/subscriptionId`. The keys the draft
68+
/// marks as required stay optional because rmcp generates one schema
69+
/// shared by every supported protocol version; draft-strict validation is
70+
/// a runtime concern (`RequestMetaObject::missing_required_keys`).
6971
#[test]
7072
fn test_metadata_definitions_match_draft_schema() {
7173
let settings = SchemaSettings::draft07();
@@ -96,11 +98,6 @@ mod tests {
9698
"io.modelcontextprotocol/clientCapabilities": { "$ref": "#/definitions/ClientCapabilities" },
9799
"io.modelcontextprotocol/logLevel": { "$ref": "#/definitions/LoggingLevel" },
98100
},
99-
"required": [
100-
"io.modelcontextprotocol/protocolVersion",
101-
"io.modelcontextprotocol/clientInfo",
102-
"io.modelcontextprotocol/clientCapabilities",
103-
],
104101
"additionalProperties": true,
105102
})
106103
);

crates/rmcp/tests/test_message_schema/client_json_rpc_message_schema.json

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1727,12 +1727,7 @@
17271727
"$ref": "#/definitions/ProgressToken"
17281728
}
17291729
},
1730-
"additionalProperties": true,
1731-
"required": [
1732-
"io.modelcontextprotocol/protocolVersion",
1733-
"io.modelcontextprotocol/clientInfo",
1734-
"io.modelcontextprotocol/clientCapabilities"
1735-
]
1730+
"additionalProperties": true
17361731
},
17371732
"RequestNoParam": {
17381733
"type": "object",

crates/rmcp/tests/test_message_schema/client_json_rpc_message_schema_current.json

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1727,12 +1727,7 @@
17271727
"$ref": "#/definitions/ProgressToken"
17281728
}
17291729
},
1730-
"additionalProperties": true,
1731-
"required": [
1732-
"io.modelcontextprotocol/protocolVersion",
1733-
"io.modelcontextprotocol/clientInfo",
1734-
"io.modelcontextprotocol/clientCapabilities"
1735-
]
1730+
"additionalProperties": true
17361731
},
17371732
"RequestNoParam": {
17381733
"type": "object",

crates/rmcp/tests/test_message_schema/server_json_rpc_message_schema.json

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2607,12 +2607,7 @@
26072607
"$ref": "#/definitions/ProgressToken"
26082608
}
26092609
},
2610-
"additionalProperties": true,
2611-
"required": [
2612-
"io.modelcontextprotocol/protocolVersion",
2613-
"io.modelcontextprotocol/clientInfo",
2614-
"io.modelcontextprotocol/clientCapabilities"
2615-
]
2610+
"additionalProperties": true
26162611
},
26172612
"RequestNoParam": {
26182613
"type": "object",

crates/rmcp/tests/test_message_schema/server_json_rpc_message_schema_current.json

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2607,12 +2607,7 @@
26072607
"$ref": "#/definitions/ProgressToken"
26082608
}
26092609
},
2610-
"additionalProperties": true,
2611-
"required": [
2612-
"io.modelcontextprotocol/protocolVersion",
2613-
"io.modelcontextprotocol/clientInfo",
2614-
"io.modelcontextprotocol/clientCapabilities"
2615-
]
2610+
"additionalProperties": true
26162611
},
26172612
"RequestNoParam": {
26182613
"type": "object",

0 commit comments

Comments
 (0)