Skip to content

fix(server): remove initialized notification gate to support Streamable HTTP #788

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DaleSeo merged 2 commits into from
Apr 9, 2026
+78 −54
Merged

fix(server): remove initialized notification gate to support Streamable HTTP #788

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
277a6cc
fix(server): remove initialized notification gate to support Streamab…
Apr 3, 2026
aa8b5e6
fix(server): keep ExpectedInitializedNotification as deprecated
Apr 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,5 @@ __pycache__/
# and can be added to the global gitignore or merged into this file. For a more nuclear
# option (not recommended) you can uncomment the following to ignore the entire idea folder.
#.idea/
node_modules/
.DS_Store
53 changes: 10 additions & 43 deletions crates/rmcp/src/service/server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ pub enum ServerInitializeError {
#[error("expect initialized request, but received: {0:?}")]
ExpectedInitializeRequest(Option<ClientJsonRpcMessage>),

#[deprecated(
since = "1.4.0",
note = "The server no longer gates on the initialized notification. This variant is never constructed and will be removed in a future major release."
)]
#[error("expect initialized notification, but received: {0:?}")]
ExpectedInitializedNotification(Option<ClientJsonRpcMessage>),
Copy link
Copy Markdown
Member

@DaleSeo DaleSeo Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since ServerInitializeError is public, removing its variant is a breaking change. Since the behavior changed intentionally, I suggest keeping this variant as deprecated. This way, we can maintain compatibility and avoid bumping the major version in the next release.

Here's the output from the semver checks:

$ cargo semver-checks check-release
    Building rmcp v1.3.0 (current)
       Built [  17.264s] (current)
     Parsing rmcp v1.3.0 (current)
      Parsed [   0.069s] (current)
    Building rmcp v1.3.0 (baseline)
       Built [   3.207s] (baseline)
     Parsing rmcp v1.3.0 (baseline)
      Parsed [   0.066s] (baseline)
    Checking rmcp v1.3.0 -> v1.3.0 (no change; assume patch)
     Checked [   0.082s] 221 checks: 220 pass, 1 fail, 0 warn, 24 skip

--- failure enum_variant_missing: pub enum variant removed or renamed ---

Description:
A publicly-visible enum has at least one variant that is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.46.0/src/lints/enum_variant_missing.ron

Failed in:
  variant ServerInitializeError::ExpectedInitializedNotification, previously in file /private/var/folders/v2/5j2zr9qs7c76ph0yxn53d_9r0000gp/T/cursor-sandbox-cache/ef50fef5e15a857ee43162e39176828f/cargo-target/semver-checks/git-8e22aa2de28df5a285eed87c11cd89bf15fa90d3/257a621cff1beb7eafa5cd4a260f415999422844/crates/rmcp/src/service/server.rs:57

     Summary semver requires new major version: 1 major and 0 minor checks failed
    Finished [  22.930s] rmcp

Copy link
Copy Markdown
Contributor Author

@anara123 anara123 Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review, good catch. I reverted this variant.

Copy link
Copy Markdown
Member

@DaleSeo DaleSeo Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing this, @anara123! Now I see it will bump the minor version. 👍

$ cargo semver-checks check-release
    Building rmcp v1.3.0 (current)
       Built [   3.137s] (current)
     Parsing rmcp v1.3.0 (current)
      Parsed [   0.072s] (current)
    Building rmcp v1.3.0 (baseline)
       Built [   2.607s] (baseline)
     Parsing rmcp v1.3.0 (baseline)
      Parsed [   0.067s] (baseline)
    Checking rmcp v1.3.0 -> v1.3.0 (no change; assume patch)
     Checked [   0.074s] 221 checks: 220 pass, 1 fail, 0 warn, 24 skip

--- failure enum_variant_marked_deprecated: enum variant #[deprecated] added ---

Description:
An enum variant is now #[deprecated]. Downstream crates will get a compiler warning when using this variant.
        ref: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-deprecated-attribute
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.46.0/src/lints/enum_variant_marked_deprecated.ron

Failed in:
  variant rmcp::service::ServerInitializeError::ExpectedInitializedNotification in /Users/dale.seo/.cursor/worktrees/rust-sdk/review-pr-788-qd1/crates/rmcp/src/service/server.rs:61

     Summary semver requires new minor version: 0 major and 1 minor checks failed
    Finished [   9.655s] rmcp


Expand Down Expand Up @@ -243,49 +247,12 @@ where
ServerInitializeError::transport::<T>(error, "sending initialize response")
})?;

// Wait for initialized notification. The MCP spec permits logging/setLevel and ping
// before initialized; VS Code sends setLevel immediately after the initialize response.
let notification = loop {
let msg = expect_next_message(&mut transport, "initialize notification").await?;
match msg {
ClientJsonRpcMessage::Notification(n)
if matches!(
n.notification,
ClientNotification::InitializedNotification(_)
) =>
{
break n.notification;
}
ClientJsonRpcMessage::Request(req)
if matches!(
req.request,
ClientRequest::SetLevelRequest(_) | ClientRequest::PingRequest(_)
) =>
{
transport
.send(ServerJsonRpcMessage::response(
ServerResult::EmptyResult(EmptyResult {}),
req.id,
))
.await
.map_err(|error| {
ServerInitializeError::transport::<T>(error, "sending pre-init response")
})?;
}
other => {
return Err(ServerInitializeError::ExpectedInitializedNotification(
Some(other),
));
}
}
};
let context = NotificationContext {
meta: notification.get_meta().clone(),
extensions: notification.extensions().clone(),
peer: peer.clone(),
};
let _ = service.handle_notification(notification, context).await;
// Continue processing service
// Enter the main service loop immediately after sending InitializeResult.
// The initialized notification will be handled as a regular notification by serve_inner.
// This matches the TypeScript SDK behavior: no init gate, no waiting for initialized.
// Streamable HTTP has no ordering guarantee between POSTs, and the MCP spec uses
// SHOULD NOT (not MUST NOT) for pre-initialized messages, so any request arriving
// before initialized is processed normally.
Ok(serve_inner(service, transport, peer, peer_rx, ct))
}

Expand Down
77 changes: 66 additions & 11 deletions crates/rmcp/tests/test_server_initialization.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use common::handlers::TestServer;
use rmcp::{
ServiceExt,
model::{ClientJsonRpcMessage, ServerJsonRpcMessage, ServerResult},
service::ServerInitializeError,
transport::{IntoTransport, Transport},
};

Expand Down Expand Up @@ -54,7 +53,7 @@ async fn do_initialize(client: &mut impl Transport<rmcp::RoleClient>) {
let _response = client.receive().await.unwrap();
}

// Server responds with EmptyResult to setLevel received before initialized.
// Server handles setLevel sent before initialized notification (processed by serve_inner).
#[tokio::test]
async fn server_init_set_level_response_is_empty_result() {
let (server_transport, client_transport) = tokio::io::duplex(4096);
Expand All @@ -64,7 +63,14 @@ async fn server_init_set_level_response_is_empty_result() {
do_initialize(&mut client).await;
client.send(set_level_request(2)).await.unwrap();

let response = client.receive().await.unwrap();
// The handler may send logging notifications before the response;
// skip notifications to find the EmptyResult response.
let response = loop {
let msg = client.receive().await.unwrap();
if matches!(msg, ServerJsonRpcMessage::Response(_)) {
break msg;
}
};
assert!(
matches!(
response,
Expand All @@ -85,7 +91,13 @@ async fn server_init_succeeds_after_set_level_before_initialized() {

do_initialize(&mut client).await;
client.send(set_level_request(2)).await.unwrap();
let _response = client.receive().await.unwrap();
// Skip notifications until we get the response
loop {
let msg = client.receive().await.unwrap();
if matches!(msg, ServerJsonRpcMessage::Response(_)) {
break;
}
}
client.send(initialized_notification()).await.unwrap();

let result = server_handle.await.unwrap();
Expand Down Expand Up @@ -179,23 +191,66 @@ async fn server_init_succeeds_after_ping_before_initialized() {
result.unwrap().cancel().await.unwrap();
}

// Server returns ExpectedInitializedNotification for any other message before initialized.
// Server buffers tools/list sent before initialized and processes it after initialization.
#[tokio::test]
async fn server_init_rejects_unexpected_message_before_initialized() {
async fn server_init_buffers_request_before_initialized() {
let (server_transport, client_transport) = tokio::io::duplex(4096);
let server_handle =
tokio::spawn(async move { TestServer::new().serve(server_transport).await });
let mut client = IntoTransport::<rmcp::RoleClient, _, _>::into_transport(client_transport);

do_initialize(&mut client).await;
// Send tools/list before initialized notification
client.send(list_tools_request(2)).await.unwrap();
// Now send initialized notification
client.send(initialized_notification()).await.unwrap();

// The buffered tools/list should be processed — expect a response
let response = client.receive().await.unwrap();
assert!(
matches!(response, ServerJsonRpcMessage::Response(_)),
"expected response for buffered tools/list, got: {response:?}"
);

let result = server_handle.await.unwrap();
assert!(
matches!(
result,
Err(ServerInitializeError::ExpectedInitializedNotification(_))
),
"expected ExpectedInitializedNotification error"
result.is_ok(),
"server should initialize successfully when buffering pre-init messages"
);
result.unwrap().cancel().await.unwrap();
}

// Server buffers multiple requests before initialized and processes them in order.
#[tokio::test]
async fn server_init_buffers_multiple_requests_before_initialized() {
let (server_transport, client_transport) = tokio::io::duplex(4096);
let server_handle =
tokio::spawn(async move { TestServer::new().serve(server_transport).await });
let mut client = IntoTransport::<rmcp::RoleClient, _, _>::into_transport(client_transport);

do_initialize(&mut client).await;
// Send two requests before initialized
client.send(list_tools_request(2)).await.unwrap();
client.send(ping_request(3)).await.unwrap();
// Now send initialized notification
client.send(initialized_notification()).await.unwrap();

// Both buffered messages should get responses
let response1 = client.receive().await.unwrap();
let response2 = client.receive().await.unwrap();
assert!(
matches!(response1, ServerJsonRpcMessage::Response(_)),
"expected response for first buffered message, got: {response1:?}"
);
assert!(
matches!(response2, ServerJsonRpcMessage::Response(_)),
"expected response for second buffered message, got: {response2:?}"
);

let result = server_handle.await.unwrap();
assert!(
result.is_ok(),
"server should initialize successfully with multiple buffered messages"
);
result.unwrap().cancel().await.unwrap();
}
Loading