mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
Upgrade rmcp to 3.0.0 (#36001)
## What changed - Update the Rust MCP SDK from `3.0.0-beta.3` to `3.0.0` and adapt to its renamed metadata and server discovery types. - Accept discovery responses without server identity metadata, using the configured server name as a fallback, and rely on the SDK's native support for namespaced server identity metadata. - Preserve typed OAuth HTTP errors so transport failures, cross-origin redirects, and transient HTTP responses are reported instead of being treated as anonymous access. ## Testing - Cover modern discovery with namespaced or missing server identity over HTTP, SSE, and stdio. - Cover OAuth discovery error propagation for transport failures, redirects, and transient status codes. GitOrigin-RevId: 12c1e45136cca89ce4fb15986c2b5df14608682a
This commit is contained in:
Generated
+3
-2
File diff suppressed because one or more lines are too long
Generated
+11
-5
@@ -1252,6 +1252,12 @@ version = "0.22.1"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6"
|
||||
|
||||
[[package]]
|
||||
name = "base64"
|
||||
version = "0.23.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "b25655df2c3cdd83c5e5b293b88acd880332b2ddadd7c30ac43144fdc0033da9"
|
||||
|
||||
[[package]]
|
||||
name = "base64-simd"
|
||||
version = "0.8.0"
|
||||
@@ -11557,12 +11563,12 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "rmcp"
|
||||
version = "3.0.0-beta.3"
|
||||
version = "3.0.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "e13a8472324b3d6c74f092cc5d07b2707b128196b020ed7c3c2d52bede542680"
|
||||
checksum = "fcd2b6dd3b18129368955f32661a7718969e8c152c7d8866434c09cf15a512e0"
|
||||
dependencies = [
|
||||
"async-trait",
|
||||
"base64 0.22.1",
|
||||
"base64 0.23.0",
|
||||
"bytes",
|
||||
"chrono",
|
||||
"futures",
|
||||
@@ -11592,9 +11598,9 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "rmcp-macros"
|
||||
version = "3.0.0-beta.3"
|
||||
version = "3.0.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "faa1de06ecdc489a8a03ca84f8491111c1e37acd8c69a815e253123039619d72"
|
||||
checksum = "e1aa4b9345795260a43fc23d6d05e096407c8b953903f673af7c4404b49fb2d6"
|
||||
dependencies = [
|
||||
"darling 0.23.0",
|
||||
"proc-macro2",
|
||||
|
||||
+1
-1
@@ -381,7 +381,7 @@ rcgen = { version = "0.14.7", default-features = false, features = [
|
||||
regex = "1.12.3"
|
||||
regex-lite = "0.1.8"
|
||||
reqwest = { version = "0.12", features = ["cookies"] }
|
||||
rmcp = { version = "=3.0.0-beta.3", default-features = false }
|
||||
rmcp = { version = "=3.0.0", default-features = false }
|
||||
runfiles = { git = "https://github.com/dzbarsky/rules_rust", rev = "b56cbaa8465e74127f1ea216f813cd377295ad81" }
|
||||
rustls = { version = "0.23", default-features = false, features = [
|
||||
"aws_lc_rs",
|
||||
|
||||
@@ -41,7 +41,7 @@ use pretty_assertions::assert_eq;
|
||||
use rmcp::handler::server::ServerHandler;
|
||||
use rmcp::model::JsonObject;
|
||||
use rmcp::model::ListToolsResult;
|
||||
use rmcp::model::Meta;
|
||||
use rmcp::model::MetaObject;
|
||||
use rmcp::model::ServerCapabilities;
|
||||
use rmcp::model::ServerInfo;
|
||||
use rmcp::model::Tool;
|
||||
@@ -1761,7 +1761,7 @@ pub(super) fn connector_tool(connector_id: &str, connector_name: &str) -> Result
|
||||
);
|
||||
tool.annotations = Some(ToolAnnotations::new().read_only(true));
|
||||
|
||||
let mut meta = Meta::new();
|
||||
let mut meta = MetaObject::new();
|
||||
meta.0
|
||||
.insert("connector_id".to_string(), json!(connector_id));
|
||||
meta.0
|
||||
|
||||
@@ -41,7 +41,7 @@ use rmcp::model::ElicitResult;
|
||||
use rmcp::model::ElicitationAction;
|
||||
use rmcp::model::ElicitationSchema;
|
||||
use rmcp::model::ListResourcesResult;
|
||||
use rmcp::model::Meta;
|
||||
use rmcp::model::MetaObject;
|
||||
use rmcp::model::PaginatedRequestParams;
|
||||
use rmcp::model::PrimitiveSchemaDefinition;
|
||||
use rmcp::model::ProtocolVersion;
|
||||
@@ -924,8 +924,8 @@ fn skill_resource(
|
||||
.with_meta(skill_resource_meta(plugin_name, skill_name))
|
||||
}
|
||||
|
||||
fn skill_resource_meta(plugin_name: &str, skill_name: &str) -> Meta {
|
||||
Meta(serde_json::Map::from_iter([
|
||||
fn skill_resource_meta(plugin_name: &str, skill_name: &str) -> MetaObject {
|
||||
MetaObject(serde_json::Map::from_iter([
|
||||
("plugin_name".to_string(), json!(plugin_name)),
|
||||
("skill_name".to_string(), json!(skill_name)),
|
||||
]))
|
||||
|
||||
@@ -53,7 +53,7 @@ use rmcp::model::InitializeRequestParams;
|
||||
use rmcp::model::InitializeResult;
|
||||
use rmcp::model::JsonObject;
|
||||
use rmcp::model::ListToolsResult;
|
||||
use rmcp::model::Meta;
|
||||
use rmcp::model::MetaObject;
|
||||
use rmcp::model::PrimitiveSchemaDefinition;
|
||||
use rmcp::model::ServerCapabilities;
|
||||
use rmcp::model::ServerInfo;
|
||||
@@ -693,7 +693,7 @@ impl ServerHandler for ElicitationAppsMcpServer {
|
||||
);
|
||||
tool.annotations = Some(ToolAnnotations::new().read_only(true));
|
||||
|
||||
let mut meta = Meta::new();
|
||||
let mut meta = MetaObject::new();
|
||||
meta.0
|
||||
.insert("connector_id".to_string(), json!(CONNECTOR_ID));
|
||||
meta.0
|
||||
|
||||
@@ -47,7 +47,7 @@ use rmcp::model::ElicitationAction;
|
||||
use rmcp::model::ElicitationSchema;
|
||||
use rmcp::model::JsonObject;
|
||||
use rmcp::model::ListToolsResult;
|
||||
use rmcp::model::Meta;
|
||||
use rmcp::model::MetaObject;
|
||||
use rmcp::model::PrimitiveSchemaDefinition;
|
||||
use rmcp::model::ServerCapabilities;
|
||||
use rmcp::model::ServerInfo;
|
||||
@@ -633,7 +633,7 @@ impl ServerHandler for ToolAppsMcpServer {
|
||||
.and_then(|value| value.as_str())
|
||||
.unwrap_or_default();
|
||||
|
||||
let mut meta = Meta::new();
|
||||
let mut meta = MetaObject::new();
|
||||
meta.0.insert("calledBy".to_string(), json!("mcp-app"));
|
||||
|
||||
if message == LARGE_RESPONSE_MESSAGE {
|
||||
|
||||
@@ -38,7 +38,7 @@ use pretty_assertions::assert_eq;
|
||||
use rmcp::handler::server::ServerHandler;
|
||||
use rmcp::model::JsonObject;
|
||||
use rmcp::model::ListToolsResult;
|
||||
use rmcp::model::Meta;
|
||||
use rmcp::model::MetaObject;
|
||||
use rmcp::model::ServerCapabilities;
|
||||
use rmcp::model::ServerInfo;
|
||||
use rmcp::model::Tool;
|
||||
@@ -2337,7 +2337,7 @@ fn connector_tool(connector_id: &str, connector_name: &str) -> Result<Tool> {
|
||||
);
|
||||
tool.annotations = Some(ToolAnnotations::new().read_only(true));
|
||||
|
||||
let mut meta = Meta::new();
|
||||
let mut meta = MetaObject::new();
|
||||
meta.0
|
||||
.insert("connector_id".to_string(), json!(connector_id));
|
||||
meta.0
|
||||
|
||||
@@ -165,6 +165,7 @@ async fn list_discovers_local_oauth_server_through_environment_proxy() -> Result
|
||||
Err(error) => return Err(error.into()),
|
||||
}
|
||||
};
|
||||
stream.set_nonblocking(false)?;
|
||||
stream.set_read_timeout(Some(Duration::from_secs(5)))?;
|
||||
let mut request = Vec::new();
|
||||
let mut buffer = [0_u8; 1024];
|
||||
|
||||
@@ -3,7 +3,7 @@ use std::sync::Arc;
|
||||
|
||||
use pretty_assertions::assert_eq;
|
||||
use rmcp::model::JsonObject;
|
||||
use rmcp::model::Meta;
|
||||
use rmcp::model::MetaObject;
|
||||
use rmcp::model::Tool;
|
||||
|
||||
use super::*;
|
||||
@@ -71,7 +71,7 @@ fn prepare_openai_file_params_for_model_masks_file_params() {
|
||||
.expect("object")
|
||||
.clone(),
|
||||
);
|
||||
tool.meta = Some(Meta(
|
||||
tool.meta = Some(MetaObject(
|
||||
serde_json::json!({
|
||||
"openai/fileParams": ["file", "files"]
|
||||
})
|
||||
@@ -219,7 +219,7 @@ fn prepare_openai_file_params_for_model_derives_supported_optional_fields() {
|
||||
.clone(),
|
||||
),
|
||||
);
|
||||
tool.meta = Some(Meta(
|
||||
tool.meta = Some(MetaObject(
|
||||
serde_json::json!({
|
||||
"openai/fileParams": [
|
||||
"photoshop_image",
|
||||
|
||||
@@ -898,7 +898,8 @@ async fn start_server_task(
|
||||
)
|
||||
.await
|
||||
.map_err(StartupOutcomeError::from)?;
|
||||
let server_info = mcp_server_info_from_implementation(initialize_result.server_info);
|
||||
let server_info =
|
||||
mcp_server_info_from_implementation(&server_name, initialize_result.server_info);
|
||||
let shared_tools = match (codex_apps_tools_cache_context.as_ref(), fetch_ticket) {
|
||||
(Some(cache_context), Some(fetch_ticket)) => cache_context.publish_if_newest_accepted(
|
||||
fetch_ticket,
|
||||
@@ -954,7 +955,11 @@ fn mcp_initialize_request_params(
|
||||
.with_protocol_version(ProtocolVersion::V_2025_06_18)
|
||||
}
|
||||
|
||||
fn mcp_server_info_from_implementation(server_info: Implementation) -> McpServerInfo {
|
||||
fn mcp_server_info_from_implementation(
|
||||
server_name: &str,
|
||||
server_info: Option<Implementation>,
|
||||
) -> McpServerInfo {
|
||||
let server_info = server_info.unwrap_or_else(|| Implementation::new(server_name, ""));
|
||||
McpServerInfo {
|
||||
name: server_info.name,
|
||||
title: server_info.title,
|
||||
@@ -1085,7 +1090,7 @@ mod tests {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
use rmcp::model::JsonObject;
|
||||
use rmcp::model::Meta;
|
||||
use rmcp::model::MetaObject;
|
||||
use rmcp::transport::auth::AuthError;
|
||||
|
||||
#[test]
|
||||
@@ -1098,6 +1103,44 @@ mod tests {
|
||||
assert!(error.is_authentication_required());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn missing_server_implementation_uses_configured_server_name() {
|
||||
assert_eq!(
|
||||
mcp_server_info_from_implementation("configured-server", /*server_info*/ None),
|
||||
McpServerInfo {
|
||||
name: "configured-server".to_string(),
|
||||
title: None,
|
||||
version: String::new(),
|
||||
description: None,
|
||||
icons: None,
|
||||
website_url: None,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn advertised_server_implementation_takes_precedence_over_configured_name() {
|
||||
assert_eq!(
|
||||
mcp_server_info_from_implementation(
|
||||
"configured-server",
|
||||
Some(
|
||||
Implementation::new("advertised-server", "1.2.3")
|
||||
.with_title("Advertised server")
|
||||
.with_description("Advertised description")
|
||||
.with_website_url("https://example.com"),
|
||||
),
|
||||
),
|
||||
McpServerInfo {
|
||||
name: "advertised-server".to_string(),
|
||||
title: Some("Advertised server".to_string()),
|
||||
version: "1.2.3".to_string(),
|
||||
description: Some("Advertised description".to_string()),
|
||||
icons: None,
|
||||
website_url: Some("https://example.com".to_string()),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mcp_initialize_advertises_openai_form_only_when_supported() {
|
||||
let unsupported = mcp_initialize_request_params(
|
||||
@@ -1125,7 +1168,7 @@ mod tests {
|
||||
"test tool",
|
||||
Arc::new(JsonObject::default()),
|
||||
)
|
||||
.with_meta(Meta(
|
||||
.with_meta(MetaObject(
|
||||
serde_json::json!({
|
||||
"connector_id": "connector_gmail",
|
||||
"connector_name": "Gmail",
|
||||
|
||||
@@ -17,7 +17,7 @@ use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
|
||||
use codex_mcp::ToolInfo;
|
||||
use pretty_assertions::assert_eq;
|
||||
use rmcp::model::JsonObject;
|
||||
use rmcp::model::Meta;
|
||||
use rmcp::model::MetaObject;
|
||||
use rmcp::model::Tool;
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashSet;
|
||||
@@ -150,7 +150,7 @@ fn accessible_connectors_from_mcp_tools_carries_plugin_display_names() {
|
||||
#[test]
|
||||
fn synthetic_links_are_exposed_to_the_agent_but_not_accessible_in_app_list() {
|
||||
let mut synthetic_tool = codex_app_tool("gmail_batch_read_email", "gmail", Some("Gmail"), &[]);
|
||||
synthetic_tool.tool.meta = Some(Meta(
|
||||
synthetic_tool.tool.meta = Some(MetaObject(
|
||||
serde_json::json!({
|
||||
"resource_name": "gmail.batch_read_email",
|
||||
"_codex_apps": {
|
||||
|
||||
@@ -7,7 +7,7 @@ use codex_tools::ToolExposure;
|
||||
use codex_tools::ToolName;
|
||||
use pretty_assertions::assert_eq;
|
||||
use rmcp::model::JsonObject;
|
||||
use rmcp::model::Meta;
|
||||
use rmcp::model::MetaObject;
|
||||
use rmcp::model::Tool;
|
||||
|
||||
use super::*;
|
||||
@@ -98,7 +98,7 @@ fn runtimes_by_name(runtimes: &[Arc<dyn CoreToolRuntime>]) -> HashMap<ToolName,
|
||||
}
|
||||
|
||||
fn with_visibility(mut tool: ToolInfo, visibility: &[&str]) -> ToolInfo {
|
||||
tool.tool.meta = Some(Meta(
|
||||
tool.tool.meta = Some(MetaObject(
|
||||
serde_json::json!({ "ui": { "visibility": visibility } })
|
||||
.as_object()
|
||||
.expect("metadata object")
|
||||
|
||||
@@ -348,6 +348,18 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
fn assert_recorded_discovery_failure(discovery: Result<Option<StreamableHttpOAuthDiscovery>>) {
|
||||
let error = discovery.expect_err("the recording HTTP client rejects OAuth discovery");
|
||||
assert!(
|
||||
matches!(
|
||||
error.downcast_ref::<AuthError>(),
|
||||
Some(AuthError::MetadataError(reason))
|
||||
if reason.contains("expected discovery request failure")
|
||||
),
|
||||
"OAuth discovery must preserve the executor transport failure: {error:#}"
|
||||
);
|
||||
}
|
||||
|
||||
async fn spawn_oauth_discovery_server(metadata: serde_json::Value) -> TestServer {
|
||||
let listener = tokio::net::TcpListener::bind("127.0.0.1:0")
|
||||
.await
|
||||
@@ -517,6 +529,7 @@ mod tests {
|
||||
#[tokio::test]
|
||||
async fn oauth_discovery_does_not_follow_cross_origin_redirects() {
|
||||
let redirect_target = MockServer::start().await;
|
||||
let redirect_url = format!("{}/redirect-target", redirect_target.uri());
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/redirect-target"))
|
||||
.and(header("x-api-key", "sensitive-key"))
|
||||
@@ -529,15 +542,14 @@ mod tests {
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/mcp"))
|
||||
.and(header("x-api-key", "sensitive-key"))
|
||||
.respond_with(ResponseTemplate::new(302).insert_header(
|
||||
"location",
|
||||
format!("{}/redirect-target", redirect_target.uri()),
|
||||
))
|
||||
.respond_with(
|
||||
ResponseTemplate::new(302).insert_header("location", redirect_url.clone()),
|
||||
)
|
||||
.expect(1)
|
||||
.mount(&resource_server)
|
||||
.await;
|
||||
|
||||
let discovery = discover_streamable_http_oauth(
|
||||
let error = discover_streamable_http_oauth(
|
||||
&format!("{}/mcp", resource_server.uri()),
|
||||
Some(HashMap::from([(
|
||||
"x-api-key".to_string(),
|
||||
@@ -548,13 +560,57 @@ mod tests {
|
||||
OAuthDiscoveryTimeout::LOCAL,
|
||||
)
|
||||
.await
|
||||
.expect("discovery should complete without following the redirect");
|
||||
.expect_err("cross-origin OAuth discovery redirects must be rejected");
|
||||
|
||||
assert_eq!(discovery, None);
|
||||
assert!(
|
||||
matches!(
|
||||
error.downcast_ref::<AuthError>(),
|
||||
Some(AuthError::MetadataError(reason))
|
||||
if reason.contains("OAuth discovery redirect to non-same-origin URL rejected")
|
||||
&& reason.contains(&redirect_url)
|
||||
),
|
||||
"OAuth discovery must preserve the cross-origin redirect rejection: {error:#}"
|
||||
);
|
||||
redirect_target.verify().await;
|
||||
resource_server.verify().await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn oauth_discovery_preserves_transient_http_errors() {
|
||||
for status in [
|
||||
StatusCode::REQUEST_TIMEOUT,
|
||||
StatusCode::TOO_EARLY,
|
||||
StatusCode::TOO_MANY_REQUESTS,
|
||||
] {
|
||||
let server = MockServer::start().await;
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/mcp"))
|
||||
.respond_with(ResponseTemplate::new(status.as_u16()))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let error = discover_streamable_http_oauth(
|
||||
&format!("{}/mcp", server.uri()),
|
||||
/*http_headers*/ None,
|
||||
/*env_http_headers*/ None,
|
||||
test_http_client(),
|
||||
OAuthDiscoveryTimeout::LOCAL,
|
||||
)
|
||||
.await
|
||||
.expect_err("transient OAuth discovery failures must not become anonymous access");
|
||||
|
||||
assert!(
|
||||
matches!(
|
||||
error.downcast_ref::<AuthError>(),
|
||||
Some(AuthError::MetadataError(reason)) if reason.contains(status.as_str())
|
||||
),
|
||||
"OAuth discovery must preserve HTTP {status}: {error:#}"
|
||||
);
|
||||
server.verify().await;
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn discover_streamable_http_oauth_returns_normalized_scopes() {
|
||||
let server = spawn_oauth_discovery_server(serde_json::json!({
|
||||
@@ -594,7 +650,7 @@ mod tests {
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(matches!(discovery, Ok(None)));
|
||||
assert_recorded_discovery_failure(discovery);
|
||||
assert_eq!(
|
||||
*http_client
|
||||
.timeout_ms
|
||||
@@ -620,7 +676,7 @@ mod tests {
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(matches!(discovery, Ok(None)));
|
||||
assert_recorded_discovery_failure(discovery);
|
||||
assert_eq!(
|
||||
*http_client
|
||||
.timeout_ms
|
||||
@@ -646,7 +702,7 @@ mod tests {
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(matches!(discovery, Ok(None)));
|
||||
assert_recorded_discovery_failure(discovery);
|
||||
let headers = http_client
|
||||
.headers
|
||||
.lock()
|
||||
|
||||
@@ -20,7 +20,7 @@ use rmcp::model::JsonObject;
|
||||
use rmcp::model::ListResourceTemplatesResult;
|
||||
use rmcp::model::ListResourcesResult;
|
||||
use rmcp::model::ListToolsResult;
|
||||
use rmcp::model::Meta;
|
||||
use rmcp::model::MetaObject;
|
||||
use rmcp::model::PaginatedRequestParams;
|
||||
use rmcp::model::ReadResourceRequestParams;
|
||||
use rmcp::model::ReadResourceResult;
|
||||
@@ -93,7 +93,7 @@ impl TestToolServer {
|
||||
Arc::new(thread_hint_schema),
|
||||
);
|
||||
thread_hint_tool.annotations = Some(ToolAnnotations::new().read_only(true));
|
||||
let mut thread_hint_meta = Meta::new();
|
||||
let mut thread_hint_meta = MetaObject::new();
|
||||
thread_hint_meta.insert("ui".to_string(), json!({ "visibility": [] }));
|
||||
thread_hint_tool.meta = Some(thread_hint_meta);
|
||||
|
||||
@@ -524,7 +524,7 @@ impl ServerHandler for TestToolServer {
|
||||
&& let Some(cwd) = tools.iter_mut().find(|tool| tool.name == "cwd")
|
||||
{
|
||||
cwd.meta
|
||||
.get_or_insert_with(Meta::new)
|
||||
.get_or_insert_with(MetaObject::new)
|
||||
.insert("ui".to_string(), json!({ "visibility": ["app"] }));
|
||||
}
|
||||
Ok(ListToolsResult::with_all_items(tools))
|
||||
@@ -637,7 +637,7 @@ impl ServerHandler for TestToolServer {
|
||||
Ok(Self::structured_result(structured_content))
|
||||
}
|
||||
"encrypted_output" => {
|
||||
let mut meta = Meta::new();
|
||||
let mut meta = MetaObject::new();
|
||||
meta.insert("codex/encryptedContent".to_string(), json!(true));
|
||||
let mut result = CallToolResult::success(vec![
|
||||
rmcp::model::ContentBlock::text("Lookup completed"),
|
||||
@@ -729,7 +729,7 @@ impl TestToolServer {
|
||||
content.push(rmcp::model::ContentBlock::image(valid_data_b64, mime_type));
|
||||
}
|
||||
ImageScenario::ImageOnlyOriginalDetail => {
|
||||
let mut meta = rmcp::model::Meta::new();
|
||||
let mut meta = MetaObject::new();
|
||||
meta.insert(
|
||||
"codex/imageDetail".to_string(),
|
||||
serde_json::json!("original"),
|
||||
|
||||
@@ -1,38 +1,24 @@
|
||||
//! Compatibility decoding for server messages whose modern wire shapes are
|
||||
//! not yet represented correctly by rmcp's untagged result union.
|
||||
//! Compatibility decoding for multi-round-trip tool results that rmcp's
|
||||
//! untagged result union does not yet represent correctly.
|
||||
|
||||
use rmcp::model::DiscoverResult;
|
||||
use rmcp::model::InputRequiredResult;
|
||||
use rmcp::model::JsonRpcResponse;
|
||||
use rmcp::model::ServerJsonRpcMessage;
|
||||
use rmcp::model::ServerResult;
|
||||
use serde_json::Value;
|
||||
|
||||
// Remove this compatibility decoder once rmcp accepts namespaced server identity:
|
||||
// https://github.com/modelcontextprotocol/rust-sdk/issues/1039
|
||||
const SERVER_INFO_METADATA_KEY: &str = "io.modelcontextprotocol/serverInfo";
|
||||
|
||||
/// Decode at the transport boundary, before rmcp's untagged `ServerResult`
|
||||
/// deserializer can mistake a modern result for a completed tool call.
|
||||
pub(crate) fn deserialize_incoming_jsonrpc_message(
|
||||
bytes: &[u8],
|
||||
) -> serde_json::Result<ServerJsonRpcMessage> {
|
||||
let mut message: Value = serde_json::from_slice(bytes)?;
|
||||
let changed_discovery = normalize_discovery_server_info(&mut message);
|
||||
let message: Value = serde_json::from_slice(bytes)?;
|
||||
|
||||
let Some(result) = message.get("result") else {
|
||||
return serde_json::from_value(message);
|
||||
};
|
||||
|
||||
if changed_discovery {
|
||||
let response: JsonRpcResponse<DiscoverResult> = serde_json::from_value(message)?;
|
||||
return Ok(ServerJsonRpcMessage::response(
|
||||
ServerResult::DiscoverResult(response.result),
|
||||
response.id,
|
||||
));
|
||||
}
|
||||
|
||||
if result.get("resultType").and_then(Value::as_str) == Some("input_required") {
|
||||
if message
|
||||
.pointer("/result/resultType")
|
||||
.and_then(Value::as_str)
|
||||
== Some("input_required")
|
||||
{
|
||||
let response: JsonRpcResponse<InputRequiredResult> = serde_json::from_value(message)?;
|
||||
return Ok(ServerJsonRpcMessage::response(
|
||||
ServerResult::InputRequiredResult(response.result),
|
||||
@@ -52,48 +38,19 @@ pub(crate) fn normalize_sse_jsonrpc_message(payload: &str, modern_session: bool)
|
||||
}
|
||||
|
||||
let mut message: Value = serde_json::from_str(payload).ok()?;
|
||||
let changed_discovery = normalize_discovery_server_info(&mut message);
|
||||
let changed_mrtr = if message
|
||||
if message
|
||||
.pointer("/result/resultType")
|
||||
.and_then(Value::as_str)
|
||||
== Some("input_required")
|
||||
!= Some("input_required")
|
||||
{
|
||||
message
|
||||
.get_mut("result")
|
||||
.and_then(Value::as_object_mut)
|
||||
.is_some_and(|result| result.remove("_meta").is_some())
|
||||
} else {
|
||||
false
|
||||
};
|
||||
|
||||
if changed_discovery || changed_mrtr {
|
||||
serde_json::to_string(&message).ok()
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
fn normalize_discovery_server_info(message: &mut Value) -> bool {
|
||||
let Some(result) = message.get_mut("result").and_then(Value::as_object_mut) else {
|
||||
return false;
|
||||
};
|
||||
if result.contains_key("serverInfo")
|
||||
|| !result.contains_key("supportedVersions")
|
||||
|| !result.contains_key("capabilities")
|
||||
{
|
||||
return false;
|
||||
return None;
|
||||
}
|
||||
|
||||
let Some(server_info) = result
|
||||
.get("_meta")
|
||||
.and_then(Value::as_object)
|
||||
.and_then(|meta| meta.get(SERVER_INFO_METADATA_KEY))
|
||||
.cloned()
|
||||
else {
|
||||
return false;
|
||||
};
|
||||
result.insert("serverInfo".to_owned(), server_info);
|
||||
true
|
||||
message
|
||||
.get_mut("result")
|
||||
.and_then(Value::as_object_mut)?
|
||||
.remove("_meta")?;
|
||||
serde_json::to_string(&message).ok()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -35,7 +35,10 @@ fn discovery_accepts_metadata_namespaced_server_identity() {
|
||||
let ServerResult::DiscoverResult(result) = response.result else {
|
||||
panic!("metadata-only identity must not become a completed tool result");
|
||||
};
|
||||
assert_eq!(result.server_info.name, "modern-server");
|
||||
assert_eq!(
|
||||
result.server_info().map(|server_info| server_info.name),
|
||||
Some("modern-server".to_owned())
|
||||
);
|
||||
assert_eq!(
|
||||
result.meta.and_then(|meta| meta.get("retained").cloned()),
|
||||
Some(json!("metadata"))
|
||||
@@ -93,7 +96,7 @@ fn input_required_discriminator_wins_over_tool_result_metadata() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sse_discovery_promotes_metadata_namespaced_identity() {
|
||||
fn sse_discovery_preserves_metadata_namespaced_identity_without_normalization() {
|
||||
let message = json!({
|
||||
"jsonrpc": "2.0",
|
||||
"id": 1,
|
||||
@@ -111,11 +114,23 @@ fn sse_discovery_promotes_metadata_namespaced_identity() {
|
||||
},
|
||||
},
|
||||
});
|
||||
let normalized =
|
||||
normalize_sse_jsonrpc_message(&message.to_string(), /*modern_session*/ true)
|
||||
.expect("metadata-only SSE discovery must be normalized");
|
||||
let normalized: serde_json::Value = serde_json::from_str(&normalized).unwrap();
|
||||
assert_eq!(normalized["result"]["serverInfo"]["name"], "sse-server");
|
||||
assert_eq!(
|
||||
normalize_sse_jsonrpc_message(&message.to_string(), /*modern_session*/ true),
|
||||
None
|
||||
);
|
||||
|
||||
let decoded = deserialize_incoming_jsonrpc_message(&serde_json::to_vec(&message).unwrap())
|
||||
.expect("rmcp must natively decode metadata-only SSE discovery");
|
||||
let JsonRpcMessage::Response(response) = decoded else {
|
||||
panic!("expected a discovery response");
|
||||
};
|
||||
let ServerResult::DiscoverResult(result) = response.result else {
|
||||
panic!("metadata-only identity must decode as discovery without normalization");
|
||||
};
|
||||
assert_eq!(
|
||||
result.server_info().map(|server_info| server_info.name),
|
||||
Some("sse-server".to_owned())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -21,6 +21,20 @@ use crate::auth_status::OAuthDiscoveryTimeout;
|
||||
const MAX_OAUTH_HTTP_RESPONSE_BODY_BYTES: usize = 1024 * 1024;
|
||||
static NEXT_OAUTH_REQUEST_ID: AtomicU64 = AtomicU64::new(0);
|
||||
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
enum OAuthHttpClientAdapterError {
|
||||
#[error("unsupported OAuth HTTP redirect policy")]
|
||||
UnsupportedRedirectPolicy,
|
||||
#[error("OAuth HTTP response body exceeds {maximum_bytes} bytes")]
|
||||
ResponseBodyTooLarge { maximum_bytes: usize },
|
||||
}
|
||||
|
||||
fn oauth_http_client_error(
|
||||
error: impl std::error::Error + Send + Sync + 'static,
|
||||
) -> OAuthHttpClientError {
|
||||
Box::new(error)
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
pub(crate) struct OAuthHttpClientAdapter {
|
||||
http_client: Arc<dyn HttpClient>,
|
||||
@@ -59,8 +73,8 @@ impl OAuthHttpClientAdapter {
|
||||
OAuthHttpRedirectPolicy::Follow => HttpRedirectPolicy::Follow,
|
||||
OAuthHttpRedirectPolicy::Stop => HttpRedirectPolicy::Stop,
|
||||
_ => {
|
||||
return Err(OAuthHttpClientError::new(
|
||||
"unsupported OAuth HTTP redirect policy",
|
||||
return Err(oauth_http_client_error(
|
||||
OAuthHttpClientAdapterError::UnsupportedRedirectPolicy,
|
||||
));
|
||||
}
|
||||
};
|
||||
@@ -76,10 +90,7 @@ impl OAuthHttpClientAdapter {
|
||||
.map(|(name, value)| {
|
||||
Ok(HttpHeader {
|
||||
name: name.as_str().to_string(),
|
||||
value: value
|
||||
.to_str()
|
||||
.map_err(|error| OAuthHttpClientError::new(error.to_string()))?
|
||||
.to_string(),
|
||||
value: value.to_str().map_err(oauth_http_client_error)?.to_string(),
|
||||
})
|
||||
})
|
||||
.collect::<Result<Vec<_>, OAuthHttpClientError>>()?;
|
||||
@@ -108,17 +119,15 @@ impl OAuthHttpClientAdapter {
|
||||
stream_response: true,
|
||||
})
|
||||
.await
|
||||
.map_err(|error| OAuthHttpClientError::new(error.to_string()))?;
|
||||
.map_err(oauth_http_client_error)?;
|
||||
let mut body = Vec::new();
|
||||
while let Some(chunk) = body_stream
|
||||
.recv()
|
||||
.await
|
||||
.map_err(|error| OAuthHttpClientError::new(error.to_string()))?
|
||||
{
|
||||
while let Some(chunk) = body_stream.recv().await.map_err(oauth_http_client_error)? {
|
||||
if chunk.len() > MAX_OAUTH_HTTP_RESPONSE_BODY_BYTES - body.len() {
|
||||
return Err(OAuthHttpClientError::new(format!(
|
||||
"OAuth HTTP response body exceeds {MAX_OAUTH_HTTP_RESPONSE_BODY_BYTES} bytes"
|
||||
)));
|
||||
return Err(oauth_http_client_error(
|
||||
OAuthHttpClientAdapterError::ResponseBodyTooLarge {
|
||||
maximum_bytes: MAX_OAUTH_HTTP_RESPONSE_BODY_BYTES,
|
||||
},
|
||||
));
|
||||
}
|
||||
body.extend_from_slice(&chunk);
|
||||
}
|
||||
@@ -126,9 +135,7 @@ impl OAuthHttpClientAdapter {
|
||||
for header in response.headers {
|
||||
builder = builder.header(header.name, header.value);
|
||||
}
|
||||
builder
|
||||
.body(body)
|
||||
.map_err(|error| OAuthHttpClientError::new(error.to_string()))
|
||||
builder.body(body).map_err(oauth_http_client_error)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -33,7 +33,6 @@ use rmcp::model::ElicitResult;
|
||||
use rmcp::model::ElicitationAction;
|
||||
use rmcp::model::Extensions;
|
||||
use rmcp::model::InitializeRequestParams;
|
||||
use rmcp::model::InitializeResult;
|
||||
use rmcp::model::ListResourceTemplatesResult;
|
||||
use rmcp::model::ListResourcesResult;
|
||||
use rmcp::model::ListToolsResult;
|
||||
@@ -44,6 +43,7 @@ use rmcp::model::ReadResourceResult;
|
||||
use rmcp::model::RequestId;
|
||||
use rmcp::model::RequestMetaObject;
|
||||
use rmcp::model::RequestParamsMeta;
|
||||
use rmcp::model::ServerPeerInfo;
|
||||
use rmcp::model::ServerResult;
|
||||
use rmcp::model::Tool;
|
||||
use rmcp::service::ClientCacheConfig;
|
||||
@@ -515,7 +515,7 @@ impl RmcpClient {
|
||||
params: InitializeRequestParams,
|
||||
timeout: Option<Duration>,
|
||||
send_elicitation: SendElicitation,
|
||||
) -> Result<InitializeResult> {
|
||||
) -> Result<ServerPeerInfo> {
|
||||
let client_service = ElicitationClientService::new(
|
||||
params.clone(),
|
||||
send_elicitation,
|
||||
@@ -629,7 +629,7 @@ impl RmcpClient {
|
||||
})
|
||||
}
|
||||
|
||||
fn meta_string(meta: Option<&rmcp::model::Meta>, key: &str) -> Option<String> {
|
||||
fn meta_string(meta: Option<&rmcp::model::MetaObject>, key: &str) -> Option<String> {
|
||||
meta.and_then(|meta| meta.get(key))
|
||||
.and_then(Value::as_str)
|
||||
.map(str::trim)
|
||||
|
||||
@@ -17,8 +17,9 @@ use pretty_assertions::assert_eq;
|
||||
use rmcp::model::ClientCapabilities;
|
||||
use rmcp::model::Implementation;
|
||||
use rmcp::model::InitializeRequestParams;
|
||||
use rmcp::model::InitializeResult;
|
||||
use rmcp::model::ProtocolVersion;
|
||||
use rmcp::model::ServerCapabilities;
|
||||
use rmcp::model::ServerPeerInfo;
|
||||
use serde_json::Value;
|
||||
use serde_json::json;
|
||||
use wiremock::Mock;
|
||||
@@ -57,7 +58,7 @@ async fn create_client(server: &MockServer, mode: McpProtocolMode) -> anyhow::Re
|
||||
.await
|
||||
}
|
||||
|
||||
async fn initialize_client(client: &RmcpClient) -> anyhow::Result<InitializeResult> {
|
||||
async fn initialize_client(client: &RmcpClient) -> anyhow::Result<ServerPeerInfo> {
|
||||
client
|
||||
.initialize(
|
||||
initialize_params(),
|
||||
@@ -88,7 +89,12 @@ fn modern_discover_result(request: &Value) -> Value {
|
||||
"resultType": "complete",
|
||||
"supportedVersions": [MODERN_VERSION],
|
||||
"capabilities": {"tools": {}},
|
||||
"serverInfo": {"name": "modern-test", "version": "1.0.0"},
|
||||
"_meta": {
|
||||
"io.modelcontextprotocol/serverInfo": {
|
||||
"name": "modern-test",
|
||||
"version": "1.0.0",
|
||||
},
|
||||
},
|
||||
"ttlMs": 0,
|
||||
"cacheScope": "private",
|
||||
},
|
||||
@@ -277,6 +283,30 @@ async fn legacy_mcp_requests_preserve_existing_redirect_behavior() -> anyhow::Re
|
||||
|
||||
#[tokio::test]
|
||||
async fn modern_discovery_accepts_metadata_namespaced_server_identity() -> anyhow::Result<()> {
|
||||
let server = MockServer::start().await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/mcp"))
|
||||
.respond_with(|request: &Request| {
|
||||
let body: Value = request.body_json().expect("valid JSON-RPC request");
|
||||
assert_eq!(body["method"], "server/discover");
|
||||
modern_discover_response(&body)
|
||||
})
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let client = create_client(&server, McpProtocolMode::V20260728).await?;
|
||||
let server_info = initialize_client(&client).await?;
|
||||
assert_eq!(
|
||||
server_info.server_info,
|
||||
Some(Implementation::new("modern-test", "1.0.0"))
|
||||
);
|
||||
client.shutdown().await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn modern_discovery_accepts_missing_server_identity() -> anyhow::Result<()> {
|
||||
let server = MockServer::start().await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/mcp"))
|
||||
@@ -284,14 +314,10 @@ async fn modern_discovery_accepts_metadata_namespaced_server_identity() -> anyho
|
||||
let body: Value = request.body_json().expect("valid JSON-RPC request");
|
||||
assert_eq!(body["method"], "server/discover");
|
||||
let mut response = modern_discover_result(&body);
|
||||
let server_info = response["result"]
|
||||
response["result"]
|
||||
.as_object_mut()
|
||||
.expect("discovery object")
|
||||
.remove("serverInfo")
|
||||
.expect("top-level identity");
|
||||
response["result"]["_meta"] = json!({
|
||||
"io.modelcontextprotocol/serverInfo": server_info,
|
||||
});
|
||||
.remove("_meta");
|
||||
ResponseTemplate::new(200).set_body_json(response)
|
||||
})
|
||||
.expect(1)
|
||||
@@ -299,7 +325,14 @@ async fn modern_discovery_accepts_metadata_namespaced_server_identity() -> anyho
|
||||
.await;
|
||||
|
||||
let client = create_client(&server, McpProtocolMode::V20260728).await?;
|
||||
initialize_client(&client).await?;
|
||||
let server_info = initialize_client(&client).await?;
|
||||
assert_eq!(
|
||||
server_info,
|
||||
ServerPeerInfo::new(
|
||||
ProtocolVersion::V_2026_07_28,
|
||||
ServerCapabilities::builder().enable_tools().build(),
|
||||
)
|
||||
);
|
||||
client.shutdown().await;
|
||||
Ok(())
|
||||
}
|
||||
@@ -337,15 +370,7 @@ async fn modern_discovery_accepts_metadata_namespaced_server_identity_over_sse()
|
||||
.respond_with(|request: &Request| {
|
||||
let body: Value = request.body_json().expect("valid JSON-RPC request");
|
||||
assert_eq!(body["method"], "server/discover");
|
||||
let mut response = modern_discover_result(&body);
|
||||
let server_info = response["result"]
|
||||
.as_object_mut()
|
||||
.expect("discovery object")
|
||||
.remove("serverInfo")
|
||||
.expect("top-level identity");
|
||||
response["result"]["_meta"] = json!({
|
||||
"io.modelcontextprotocol/serverInfo": server_info,
|
||||
});
|
||||
let response = modern_discover_result(&body);
|
||||
ResponseTemplate::new(200).set_body_raw(
|
||||
format!("event: message\ndata: {response}\n\n"),
|
||||
"text/event-stream; charset=utf-8",
|
||||
|
||||
@@ -41,7 +41,12 @@ fn discover_response(body: &Value) -> ResponseTemplate {
|
||||
"resultType": "complete",
|
||||
"supportedVersions": [MODERN_VERSION],
|
||||
"capabilities": {"tools": {}, "resources": {}},
|
||||
"serverInfo": {"name": "mrtr-test", "version": "1.0.0"},
|
||||
"_meta": {
|
||||
"io.modelcontextprotocol/serverInfo": {
|
||||
"name": "mrtr-test",
|
||||
"version": "1.0.0",
|
||||
},
|
||||
},
|
||||
"ttlMs": 0,
|
||||
"cacheScope": "private",
|
||||
},
|
||||
|
||||
@@ -128,12 +128,15 @@ async fn exercise_stdio_server(
|
||||
};
|
||||
assert_eq!(initialized.protocol_version, expected_version);
|
||||
assert_eq!(
|
||||
initialized.server_info.name,
|
||||
if legacy_session {
|
||||
initialized
|
||||
.server_info
|
||||
.as_ref()
|
||||
.map(|server_info| server_info.name.as_str()),
|
||||
Some(if legacy_session {
|
||||
"legacy-stdio-test"
|
||||
} else {
|
||||
"strict-stdio-test"
|
||||
}
|
||||
})
|
||||
);
|
||||
let tools = client
|
||||
.list_tools(/*params*/ None, Some(Duration::from_secs(5)))
|
||||
|
||||
Reference in New Issue
Block a user