fix: honor supported_protocol_versions when negotiating initialize (#1093)
This commit is contained in:
parent
51c8ee60e3
commit
d272389fd4
7 changed files with 246 additions and 16 deletions
|
|
@ -300,6 +300,10 @@ impl<H: ServerHandler> Service<RoleServer> for H {
|
|||
fn get_info(&self) -> <RoleServer as ServiceRole>::Info {
|
||||
self.get_info()
|
||||
}
|
||||
|
||||
fn supported_protocol_versions(&self) -> Cow<'static, [ProtocolVersion]> {
|
||||
ServerHandler::supported_protocol_versions(self)
|
||||
}
|
||||
}
|
||||
|
||||
macro_rules! server_handler_methods {
|
||||
|
|
@ -321,10 +325,17 @@ macro_rules! server_handler_methods {
|
|||
info.protocol_version = negotiate_protocol_version(
|
||||
&request.protocol_version,
|
||||
info.protocol_version,
|
||||
&self.supported_protocol_versions(),
|
||||
);
|
||||
std::future::ready(Ok(info))
|
||||
}
|
||||
/// Return the protocol versions supported by this server.
|
||||
///
|
||||
/// Defaults to every version this SDK knows. Override it to narrow the
|
||||
/// set to the revisions the server actually implements: the returned
|
||||
/// list is advertised by [`Self::discover`], bounds what `initialize`
|
||||
/// negotiation may agree to, and is what per-request versions are
|
||||
/// validated against.
|
||||
fn supported_protocol_versions(&self) -> Cow<'static, [ProtocolVersion]> {
|
||||
Cow::Borrowed(ProtocolVersion::KNOWN_VERSIONS)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
use std::sync::Arc;
|
||||
use std::{borrow::Cow, sync::Arc};
|
||||
|
||||
use prompt::{IntoPromptRoute, PromptRoute};
|
||||
use tool::{IntoToolRoute, ToolRoute};
|
||||
|
|
@ -6,7 +6,10 @@ use tool::{IntoToolRoute, ToolRoute};
|
|||
use super::ServerHandler;
|
||||
use crate::{
|
||||
RoleServer, Service,
|
||||
model::{ClientNotification, ClientRequest, ListPromptsResult, ListToolsResult, ServerResult},
|
||||
model::{
|
||||
ClientNotification, ClientRequest, ListPromptsResult, ListToolsResult, ProtocolVersion,
|
||||
ServerResult,
|
||||
},
|
||||
service::NotificationContext,
|
||||
};
|
||||
|
||||
|
|
@ -155,6 +158,10 @@ where
|
|||
.list_changed = Some(true);
|
||||
info
|
||||
}
|
||||
|
||||
fn supported_protocol_versions(&self) -> Cow<'static, [ProtocolVersion]> {
|
||||
ServerHandler::supported_protocol_versions(&self.service)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
use std::sync::OnceLock;
|
||||
use std::{borrow::Cow, sync::OnceLock};
|
||||
|
||||
use futures::FutureExt;
|
||||
#[cfg(not(feature = "local"))]
|
||||
|
|
@ -284,6 +284,19 @@ pub trait Service<R: ServiceRole>: Send + Sync + 'static {
|
|||
context: NotificationContext<R>,
|
||||
) -> impl Future<Output = Result<(), McpError>> + MaybeSendFuture + '_;
|
||||
fn get_info(&self) -> R::Info;
|
||||
/// The protocol versions this service can speak, bounding what `initialize`
|
||||
/// negotiation may agree to.
|
||||
///
|
||||
/// Servers normally override
|
||||
/// [`ServerHandler::supported_protocol_versions`] instead of this method;
|
||||
/// the blanket `Service` impl forwards to it. This method exists so the
|
||||
/// transport and handshake layers, which see only a `Service`, can read the
|
||||
/// list and avoid agreeing to a version the server cannot serve.
|
||||
///
|
||||
/// [`ServerHandler::supported_protocol_versions`]: crate::handler::server::ServerHandler::supported_protocol_versions
|
||||
fn supported_protocol_versions(&self) -> Cow<'static, [ProtocolVersion]> {
|
||||
Cow::Borrowed(ProtocolVersion::KNOWN_VERSIONS)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(feature = "local")]
|
||||
|
|
@ -299,6 +312,12 @@ pub trait Service<R: ServiceRole>: 'static {
|
|||
context: NotificationContext<R>,
|
||||
) -> impl Future<Output = Result<(), McpError>> + MaybeSendFuture + '_;
|
||||
fn get_info(&self) -> R::Info;
|
||||
/// The protocol versions this service can speak.
|
||||
///
|
||||
/// See the non-`local` variant of this trait for details.
|
||||
fn supported_protocol_versions(&self) -> Cow<'static, [ProtocolVersion]> {
|
||||
Cow::Borrowed(ProtocolVersion::KNOWN_VERSIONS)
|
||||
}
|
||||
}
|
||||
|
||||
pub trait ServiceExt<R: ServiceRole>: Service<R> + Sized {
|
||||
|
|
@ -350,6 +369,10 @@ impl<R: ServiceRole> Service<R> for Box<dyn DynService<R>> {
|
|||
fn get_info(&self) -> R::Info {
|
||||
DynService::get_info(self.as_ref())
|
||||
}
|
||||
|
||||
fn supported_protocol_versions(&self) -> Cow<'static, [ProtocolVersion]> {
|
||||
DynService::supported_protocol_versions(self.as_ref())
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(feature = "local"))]
|
||||
|
|
@ -365,6 +388,10 @@ pub trait DynService<R: ServiceRole>: Send + Sync {
|
|||
context: NotificationContext<R>,
|
||||
) -> MaybeBoxFuture<'_, Result<(), McpError>>;
|
||||
fn get_info(&self) -> R::Info;
|
||||
/// See [`Service::supported_protocol_versions`].
|
||||
fn supported_protocol_versions(&self) -> Cow<'static, [ProtocolVersion]> {
|
||||
Cow::Borrowed(ProtocolVersion::KNOWN_VERSIONS)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(feature = "local")]
|
||||
|
|
@ -380,6 +407,10 @@ pub trait DynService<R: ServiceRole> {
|
|||
context: NotificationContext<R>,
|
||||
) -> MaybeBoxFuture<'_, Result<(), McpError>>;
|
||||
fn get_info(&self) -> R::Info;
|
||||
/// See [`Service::supported_protocol_versions`].
|
||||
fn supported_protocol_versions(&self) -> Cow<'static, [ProtocolVersion]> {
|
||||
Cow::Borrowed(ProtocolVersion::KNOWN_VERSIONS)
|
||||
}
|
||||
}
|
||||
|
||||
impl<R: ServiceRole, S: Service<R>> DynService<R> for S {
|
||||
|
|
@ -400,6 +431,9 @@ impl<R: ServiceRole, S: Service<R>> DynService<R> for S {
|
|||
fn get_info(&self) -> R::Info {
|
||||
self.get_info()
|
||||
}
|
||||
fn supported_protocol_versions(&self) -> Cow<'static, [ProtocolVersion]> {
|
||||
Service::supported_protocol_versions(self)
|
||||
}
|
||||
}
|
||||
|
||||
use std::{
|
||||
|
|
|
|||
|
|
@ -460,12 +460,18 @@ where
|
|||
}
|
||||
}
|
||||
|
||||
/// Echoes the client-requested version if known; otherwise returns `server_fallback`.
|
||||
/// Echoes the client-requested version if the server supports it; otherwise
|
||||
/// returns `server_fallback`.
|
||||
///
|
||||
/// `server_supported` comes from [`Service::supported_protocol_versions`], so a
|
||||
/// server that narrows that list is never made to answer `initialize` with a
|
||||
/// version it cannot serve.
|
||||
pub(crate) fn negotiate_protocol_version(
|
||||
client_requested: &ProtocolVersion,
|
||||
server_fallback: ProtocolVersion,
|
||||
server_supported: &[ProtocolVersion],
|
||||
) -> ProtocolVersion {
|
||||
if ProtocolVersion::KNOWN_VERSIONS.contains(client_requested) {
|
||||
if server_supported.contains(client_requested) {
|
||||
client_requested.clone()
|
||||
} else {
|
||||
tracing::warn!(
|
||||
|
|
@ -578,8 +584,11 @@ where
|
|||
return Err(ServerInitializeError::InitializeFailed(e));
|
||||
}
|
||||
};
|
||||
init_response.protocol_version =
|
||||
negotiate_protocol_version(&requested_protocol_version, init_response.protocol_version);
|
||||
init_response.protocol_version = negotiate_protocol_version(
|
||||
&requested_protocol_version,
|
||||
init_response.protocol_version,
|
||||
&service.supported_protocol_versions(),
|
||||
);
|
||||
// Update peer_info so context.protocol_version() reflects the negotiated
|
||||
// version in all subsequent request handlers.
|
||||
negotiated_peer_info.protocol_version = init_response.protocol_version.clone();
|
||||
|
|
|
|||
|
|
@ -279,8 +279,11 @@ impl<S: Service<RoleServer>> Service<RoleServer> for NegotiatingStatelessHttpSer
|
|||
if let (Some(requested), ServerResult::InitializeResult(result)) =
|
||||
(requested_protocol_version, &mut response)
|
||||
{
|
||||
result.protocol_version =
|
||||
negotiate_protocol_version(&requested, result.protocol_version.clone());
|
||||
result.protocol_version = negotiate_protocol_version(
|
||||
&requested,
|
||||
result.protocol_version.clone(),
|
||||
&self.0.supported_protocol_versions(),
|
||||
);
|
||||
if let Some(peer_info) = peer.peer_info() {
|
||||
let mut peer_info = (*peer_info).clone();
|
||||
peer_info.protocol_version = result.protocol_version.clone();
|
||||
|
|
@ -301,6 +304,10 @@ impl<S: Service<RoleServer>> Service<RoleServer> for NegotiatingStatelessHttpSer
|
|||
fn get_info(&self) -> ServerInfo {
|
||||
self.0.get_info()
|
||||
}
|
||||
|
||||
fn supported_protocol_versions(&self) -> Cow<'static, [ProtocolVersion]> {
|
||||
self.0.supported_protocol_versions()
|
||||
}
|
||||
}
|
||||
|
||||
#[expect(
|
||||
|
|
|
|||
|
|
@ -4,9 +4,12 @@
|
|||
#![cfg(not(feature = "local"))]
|
||||
#![cfg(feature = "client")]
|
||||
|
||||
use std::borrow::Cow;
|
||||
|
||||
use rmcp::{
|
||||
ClientHandler, ServerHandler, ServiceExt,
|
||||
model::{ClientInfo, ProtocolVersion, ServerInfo},
|
||||
ClientHandler, ErrorData, RoleServer, ServerHandler, ServiceExt,
|
||||
model::{ClientInfo, InitializeRequestParams, InitializeResult, ProtocolVersion, ServerInfo},
|
||||
service::RequestContext,
|
||||
};
|
||||
|
||||
#[derive(Debug, Clone, Default)]
|
||||
|
|
@ -18,6 +21,52 @@ impl ServerHandler for EchoServer {
|
|||
}
|
||||
}
|
||||
|
||||
/// Every known version except `2026-07-28`, standing in for a server that has
|
||||
/// not implemented that revision.
|
||||
const NARROWED_VERSIONS: &[ProtocolVersion] = &[
|
||||
ProtocolVersion::V_2024_11_05,
|
||||
ProtocolVersion::V_2025_03_26,
|
||||
ProtocolVersion::V_2025_06_18,
|
||||
ProtocolVersion::V_2025_11_25,
|
||||
];
|
||||
|
||||
#[derive(Debug, Clone, Default)]
|
||||
struct NarrowedServer;
|
||||
|
||||
impl ServerHandler for NarrowedServer {
|
||||
fn get_info(&self) -> ServerInfo {
|
||||
ServerInfo::default()
|
||||
}
|
||||
|
||||
fn supported_protocol_versions(&self) -> Cow<'static, [ProtocolVersion]> {
|
||||
Cow::Borrowed(NARROWED_VERSIONS)
|
||||
}
|
||||
}
|
||||
|
||||
/// Narrows the supported versions *and* overrides `initialize`, so the
|
||||
/// handler's own answer never runs the default negotiation. The handshake layer
|
||||
/// must still honor the narrowed list.
|
||||
#[derive(Debug, Clone, Default)]
|
||||
struct NarrowedOverridingServer;
|
||||
|
||||
impl ServerHandler for NarrowedOverridingServer {
|
||||
fn get_info(&self) -> ServerInfo {
|
||||
ServerInfo::default()
|
||||
}
|
||||
|
||||
fn supported_protocol_versions(&self) -> Cow<'static, [ProtocolVersion]> {
|
||||
Cow::Borrowed(NARROWED_VERSIONS)
|
||||
}
|
||||
|
||||
async fn initialize(
|
||||
&self,
|
||||
_request: InitializeRequestParams,
|
||||
_context: RequestContext<RoleServer>,
|
||||
) -> Result<InitializeResult, ErrorData> {
|
||||
Ok(self.get_info())
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
struct VersionedClient {
|
||||
protocol_version: ProtocolVersion,
|
||||
|
|
@ -32,10 +81,17 @@ impl ClientHandler for VersionedClient {
|
|||
}
|
||||
|
||||
async fn negotiated_version(client_version: ProtocolVersion) -> ProtocolVersion {
|
||||
negotiated_version_with(EchoServer, client_version).await
|
||||
}
|
||||
|
||||
async fn negotiated_version_with<S: ServerHandler>(
|
||||
server: S,
|
||||
client_version: ProtocolVersion,
|
||||
) -> ProtocolVersion {
|
||||
let (server_transport, client_transport) = tokio::io::duplex(4096);
|
||||
|
||||
tokio::spawn(async move {
|
||||
let _ = EchoServer
|
||||
let _ = server
|
||||
.serve(server_transport)
|
||||
.await
|
||||
.expect("server should start")
|
||||
|
|
@ -81,3 +137,35 @@ async fn unknown_version_falls_back_to_latest() {
|
|||
"unknown version should fall back to LATEST"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn narrowed_server_still_echoes_versions_it_supports() {
|
||||
for version in NARROWED_VERSIONS {
|
||||
let negotiated = negotiated_version_with(NarrowedServer, version.clone()).await;
|
||||
assert_eq!(
|
||||
negotiated, *version,
|
||||
"supported version {version} should be echoed back"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn narrowed_server_does_not_agree_to_version_it_excludes() {
|
||||
let negotiated = negotiated_version_with(NarrowedServer, ProtocolVersion::V_2026_07_28).await;
|
||||
assert_eq!(
|
||||
negotiated,
|
||||
ProtocolVersion::V_2025_11_25,
|
||||
"a version outside supported_protocol_versions should not be echoed back"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn narrowed_server_caps_even_when_it_overrides_initialize() {
|
||||
let negotiated =
|
||||
negotiated_version_with(NarrowedOverridingServer, ProtocolVersion::V_2026_07_28).await;
|
||||
assert_eq!(
|
||||
negotiated,
|
||||
ProtocolVersion::V_2025_11_25,
|
||||
"the handshake layer should not raise the version above what the server supports"
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,8 +1,12 @@
|
|||
//! Tests for protocol version negotiation in stateless HTTP mode.
|
||||
//!
|
||||
//! Known versions are echoed back; unknown versions fall back to LATEST.
|
||||
//! Supported versions are echoed back; unknown versions, and versions outside
|
||||
//! the server's `supported_protocol_versions`, fall back to the handler's own
|
||||
//! version.
|
||||
#![cfg(not(feature = "local"))]
|
||||
|
||||
use std::borrow::Cow;
|
||||
|
||||
use rmcp::{
|
||||
ErrorData, RoleServer, ServerHandler,
|
||||
model::{
|
||||
|
|
@ -15,7 +19,7 @@ use rmcp::{
|
|||
};
|
||||
use tokio_util::sync::CancellationToken;
|
||||
|
||||
#[derive(Clone)]
|
||||
#[derive(Clone, Default)]
|
||||
struct OverridingInitialize;
|
||||
|
||||
impl ServerHandler for OverridingInitialize {
|
||||
|
|
@ -32,6 +36,38 @@ impl ServerHandler for OverridingInitialize {
|
|||
}
|
||||
}
|
||||
|
||||
/// Every known version except `2026-07-28`, standing in for a server that has
|
||||
/// not implemented that revision.
|
||||
const NARROWED_VERSIONS: &[ProtocolVersion] = &[
|
||||
ProtocolVersion::V_2024_11_05,
|
||||
ProtocolVersion::V_2025_03_26,
|
||||
ProtocolVersion::V_2025_06_18,
|
||||
ProtocolVersion::V_2025_11_25,
|
||||
];
|
||||
|
||||
/// Overrides `initialize`, so the handler-side default negotiation never runs,
|
||||
/// *and* narrows the supported versions.
|
||||
#[derive(Clone, Default)]
|
||||
struct NarrowedOverridingInitialize;
|
||||
|
||||
impl ServerHandler for NarrowedOverridingInitialize {
|
||||
fn get_info(&self) -> ServerInfo {
|
||||
ServerInfo::new(ServerCapabilities::default())
|
||||
}
|
||||
|
||||
fn supported_protocol_versions(&self) -> Cow<'static, [ProtocolVersion]> {
|
||||
Cow::Borrowed(NARROWED_VERSIONS)
|
||||
}
|
||||
|
||||
async fn initialize(
|
||||
&self,
|
||||
_request: InitializeRequestParams,
|
||||
_context: RequestContext<RoleServer>,
|
||||
) -> Result<InitializeResult, ErrorData> {
|
||||
Ok(self.get_info())
|
||||
}
|
||||
}
|
||||
|
||||
fn stateless_sse_config() -> StreamableHttpServerConfig {
|
||||
StreamableHttpServerConfig::default()
|
||||
.with_legacy_session_mode(false)
|
||||
|
|
@ -45,10 +81,16 @@ fn stateless_json_config() -> StreamableHttpServerConfig {
|
|||
|
||||
async fn spawn_server(
|
||||
config: StreamableHttpServerConfig,
|
||||
) -> (reqwest::Client, String, CancellationToken) {
|
||||
spawn_server_of::<OverridingInitialize>(config).await
|
||||
}
|
||||
|
||||
async fn spawn_server_of<H: ServerHandler + Default>(
|
||||
config: StreamableHttpServerConfig,
|
||||
) -> (reqwest::Client, String, CancellationToken) {
|
||||
let ct = config.cancellation_token.clone();
|
||||
let service: StreamableHttpService<OverridingInitialize, LocalSessionManager> =
|
||||
StreamableHttpService::new(|| Ok(OverridingInitialize), Default::default(), config);
|
||||
let service: StreamableHttpService<H, LocalSessionManager> =
|
||||
StreamableHttpService::new(|| Ok(H::default()), Default::default(), config);
|
||||
|
||||
let router = axum::Router::new().nest_service("/mcp", service);
|
||||
let tcp_listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
|
||||
|
|
@ -150,3 +192,35 @@ async fn stateless_json_init_preserves_handler_fallback_for_unknown_version() {
|
|||
|
||||
ct.cancel();
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn stateless_json_init_echoes_versions_the_server_narrowed_to() {
|
||||
let (client, url, ct) =
|
||||
spawn_server_of::<NarrowedOverridingInitialize>(stateless_json_config()).await;
|
||||
|
||||
for version in NARROWED_VERSIONS {
|
||||
let resp = post_init(&client, &url, version.as_str()).await;
|
||||
assert_eq!(
|
||||
resp["result"]["protocolVersion"],
|
||||
version.as_str(),
|
||||
"supported version {version} should be echoed back"
|
||||
);
|
||||
}
|
||||
|
||||
ct.cancel();
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn stateless_json_init_does_not_agree_to_version_outside_supported_list() {
|
||||
let (client, url, ct) =
|
||||
spawn_server_of::<NarrowedOverridingInitialize>(stateless_json_config()).await;
|
||||
|
||||
let resp = post_init(&client, &url, ProtocolVersion::V_2026_07_28.as_str()).await;
|
||||
assert_eq!(
|
||||
resp["result"]["protocolVersion"],
|
||||
ProtocolVersion::V_2025_11_25.as_str(),
|
||||
"a version outside supported_protocol_versions should not be echoed back"
|
||||
);
|
||||
|
||||
ct.cancel();
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue