Skip to content

Commit ca5b0ff

Browse files
its-mashMohammod Al Amin Ashikclaude
authored
feat: Streamable HTTP transport with SSE notifications and E2E tests (#61)
Signed-off-by: Mohammod Al Amin Ashik <maa.ashik00@gmail.com> Co-authored-by: Mohammod Al Amin Ashik <alamin.ashik@sitecore.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 96795b0 commit ca5b0ff

18 files changed

Lines changed: 2695 additions & 84 deletions

File tree

Cargo.lock

Lines changed: 242 additions & 33 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ os_pipe = "1"
5858

5959
# MCP Protocol
6060
# NOTE: Never use local path dependency - E:\one-mcp\rust-sdk is for source lookup only
61-
rmcp = { version = "0.14.0", features = [
61+
rmcp = { version = "0.15.0", features = [
6262
"client",
6363
"server",
6464
"transport-io",
@@ -84,4 +84,9 @@ lto = true
8484
codegen-units = 1
8585
strip = true
8686

87+
# Temporary patch: fixes SSE channel replacement bug (notifications lost on reconnect)
88+
# Remove once upstream merges https://github.com/modelcontextprotocol/rust-sdk/pull/660
89+
[patch.crates-io]
90+
rmcp = { git = "https://github.com/ion-ash/rust-sdk.git", branch = "fix/sse-channel-replacement-conflict" }
91+
8792

crates/mcpmux-gateway/src/consumers/mcp_notifier.rs

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ impl MCPNotifier {
415415
feature_set_id = %feature_set_id,
416416
"[MCPNotifier] 📨 GrantIssued - notifying all clients in space"
417417
);
418-
self.notify_all_list_changed(space_id).await;
418+
self.notify_all_list_changed(space_id, true).await;
419419
}
420420

421421
DomainEvent::GrantRevoked {
@@ -429,7 +429,7 @@ impl MCPNotifier {
429429
feature_set_id = %feature_set_id,
430430
"[MCPNotifier] 📨 GrantRevoked - notifying all clients in space"
431431
);
432-
self.notify_all_list_changed(space_id).await;
432+
self.notify_all_list_changed(space_id, true).await;
433433
}
434434

435435
DomainEvent::ClientGrantsUpdated {
@@ -443,7 +443,7 @@ impl MCPNotifier {
443443
feature_sets = feature_set_ids.len(),
444444
"[MCPNotifier] 📨 ClientGrantsUpdated - notifying all clients in space"
445445
);
446-
self.notify_all_list_changed(space_id).await;
446+
self.notify_all_list_changed(space_id, true).await;
447447
}
448448

449449
DomainEvent::FeatureSetMembersChanged {
@@ -456,7 +456,7 @@ impl MCPNotifier {
456456
feature_set_id = %feature_set_id,
457457
"[MCPNotifier] 📨 FeatureSetMembersChanged - notifying all clients in space"
458458
);
459-
self.notify_all_list_changed(space_id).await;
459+
self.notify_all_list_changed(space_id, true).await;
460460
}
461461

462462
// ============ Backend Server Notifications (Pass-through with Throttling) ============
@@ -523,7 +523,7 @@ impl MCPNotifier {
523523
status = ?status,
524524
"[MCPNotifier] ServerStatusChanged (Disconnected) - notifying clients to clear features"
525525
);
526-
self.notify_all_list_changed(space_id).await;
526+
self.notify_all_list_changed(space_id, false).await;
527527
} else {
528528
debug!(
529529
server_id = %server_id,
@@ -549,7 +549,7 @@ impl MCPNotifier {
549549
removed = removed.len(),
550550
"[MCPNotifier] ServerFeaturesRefreshed"
551551
);
552-
self.notify_all_list_changed(space_id).await;
552+
self.notify_all_list_changed(space_id, false).await;
553553
}
554554

555555
// Other events that affect MCP capabilities are handled above
@@ -571,8 +571,13 @@ impl MCPNotifier {
571571
/// **Important**: This method handles throttling at the batch level and marks
572572
/// all individual notification types as sent, preventing double-notifications
573573
/// when individual DomainEvent::ToolsChanged/etc. events arrive shortly after.
574-
async fn notify_all_list_changed(&self, space_id: Uuid) {
575-
// 1. Content-Based Deduping
574+
///
575+
/// **`force` parameter**: When `true`, skips content-based hash dedup. Used for
576+
/// grant-related events where the total features in the space haven't changed but
577+
/// the *effective* features visible to clients have (due to grant/feature set changes).
578+
/// The hash is computed from all features in the space, so it can't detect grant changes.
579+
async fn notify_all_list_changed(&self, space_id: Uuid, force: bool) {
580+
// 1. Content-Based Deduping (skipped when force=true)
576581
let tools_hash = self
577582
.calculate_feature_hash(space_id, FeatureType::Tool)
578583
.await;
@@ -583,23 +588,27 @@ impl MCPNotifier {
583588
.calculate_feature_hash(space_id, FeatureType::Resource)
584589
.await;
585590

586-
let any_changed = {
587-
let hashes = self.state_hashes.read();
588-
let t_changed = hashes
589-
.get(&(space_id, NotificationType::Tools))
590-
.is_none_or(|&h| h != tools_hash);
591-
let p_changed = hashes
592-
.get(&(space_id, NotificationType::Prompts))
593-
.is_none_or(|&h| h != prompts_hash);
594-
let r_changed = hashes
595-
.get(&(space_id, NotificationType::Resources))
596-
.is_none_or(|&h| h != resources_hash);
597-
t_changed || p_changed || r_changed
598-
};
599-
600-
if !any_changed {
601-
debug!(space_id = %space_id, "[MCPNotifier] 🛑 Batch content unchanged, skipping");
602-
return;
591+
if !force {
592+
let any_changed = {
593+
let hashes = self.state_hashes.read();
594+
let t_changed = hashes
595+
.get(&(space_id, NotificationType::Tools))
596+
.is_none_or(|&h| h != tools_hash);
597+
let p_changed = hashes
598+
.get(&(space_id, NotificationType::Prompts))
599+
.is_none_or(|&h| h != prompts_hash);
600+
let r_changed = hashes
601+
.get(&(space_id, NotificationType::Resources))
602+
.is_none_or(|&h| h != resources_hash);
603+
t_changed || p_changed || r_changed
604+
};
605+
606+
if !any_changed {
607+
debug!(space_id = %space_id, "[MCPNotifier] 🛑 Batch content unchanged, skipping");
608+
return;
609+
}
610+
} else {
611+
info!(space_id = %space_id, "[MCPNotifier] 🔓 Force-sending (grant/feature set change, bypassing hash dedup)");
603612
}
604613

605614
let now = Instant::now();

crates/mcpmux-gateway/src/mcp/handler.rs

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -102,14 +102,14 @@ impl ServerHandler for McpMuxGatewayHandler {
102102
protocol_version: Default::default(),
103103
capabilities: ServerCapabilities::builder()
104104
.enable_tools_with(ToolsCapability {
105-
list_changed: Some(false), // Stateless mode - no notifications
105+
list_changed: Some(true),
106106
})
107107
.enable_prompts_with(PromptsCapability {
108-
list_changed: Some(false), // Stateless mode - no notifications
108+
list_changed: Some(true),
109109
})
110110
.enable_resources_with(ResourcesCapability {
111111
subscribe: Some(false),
112-
list_changed: Some(false), // Stateless mode - no notifications
112+
list_changed: Some(true),
113113
})
114114
.build(),
115115
server_info: Implementation {
@@ -150,19 +150,34 @@ impl ServerHandler for McpMuxGatewayHandler {
150150
}
151151

152152
async fn on_initialized(&self, context: NotificationContext<RoleServer>) {
153-
// Silently process - entry already logged in oauth_middleware
154-
let _oauth_ctx = match self.get_oauth_context(&context.extensions) {
153+
let oauth_ctx = match self.get_oauth_context(&context.extensions) {
155154
Ok(ctx) => ctx,
156155
Err(e) => {
157-
warn!("Failed to extract OAuth context: {}", e);
156+
warn!("Failed to extract OAuth context on_initialized: {}", e);
158157
return;
159158
}
160159
};
161160

162-
// In stateless mode:
163-
// - No session tracking
164-
// - No notification registration
165-
// - Each request is independent
161+
// Register peer with MCPNotifier for list_changed notification delivery
162+
let peer = std::sync::Arc::new(context.peer);
163+
self.notification_bridge
164+
.register_peer(oauth_ctx.client_id.clone(), peer);
165+
166+
// Mark the client stream as active immediately - RMCP's session transport
167+
// handles SSE streaming and message caching internally
168+
self.notification_bridge
169+
.mark_client_stream_active(&oauth_ctx.client_id);
170+
171+
// Pre-populate feature hashes to prevent spurious first notifications
172+
self.notification_bridge
173+
.prime_hashes_for_space(oauth_ctx.space_id)
174+
.await;
175+
176+
info!(
177+
client_id = %oauth_ctx.client_id,
178+
space_id = %oauth_ctx.space_id,
179+
"Client initialized - peer registered for notifications"
180+
);
166181
}
167182

168183
async fn list_tools(

crates/mcpmux-gateway/src/pool/credential_store.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,7 @@ impl CredentialStore for DatabaseCredentialStore {
207207
Some(StoredCredentials {
208208
client_id: reg.client_id,
209209
token_response: Some(token_response),
210+
granted_scopes: Vec::new(),
210211
})
211212
}
212213
(Some(reg), None) => {
@@ -217,6 +218,7 @@ impl CredentialStore for DatabaseCredentialStore {
217218
Some(StoredCredentials {
218219
client_id: reg.client_id,
219220
token_response: None,
221+
granted_scopes: Vec::new(),
220222
})
221223
}
222224
(None, Some(access)) => {
@@ -228,6 +230,7 @@ impl CredentialStore for DatabaseCredentialStore {
228230
Some(StoredCredentials {
229231
client_id: String::new(),
230232
token_response: Some(token_response),
233+
granted_scopes: Vec::new(),
231234
})
232235
}
233236
(None, None) => {
@@ -584,6 +587,7 @@ mod tests {
584587
let credentials = StoredCredentials {
585588
client_id: "new-client-id".to_string(),
586589
token_response: Some(token_response),
590+
granted_scopes: Vec::new(),
587591
};
588592

589593
store.save(credentials).await.unwrap();
@@ -641,6 +645,7 @@ mod tests {
641645
let credentials = StoredCredentials {
642646
client_id: "client-id".to_string(),
643647
token_response: Some(token_response),
648+
granted_scopes: Vec::new(),
644649
};
645650

646651
store.save(credentials).await.unwrap();

crates/mcpmux-gateway/src/pool/instance.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ impl McpClientHandler {
4646
title: Some("McpMux Gateway".to_string()),
4747
icons: None,
4848
website_url: None,
49+
..Default::default()
4950
},
5051
meta: None,
5152
},

crates/mcpmux-gateway/src/pool/oauth_utils.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ pub fn convert_from_stored_metadata(stored: &StoredOAuthMetadata) -> Authorizati
110110
scopes_supported: stored.scopes_supported.clone(),
111111
response_types_supported: stored.response_types_supported.clone(),
112112
additional_fields: stored.additional_fields.clone(),
113+
..Default::default()
113114
}
114115
}
115116

crates/mcpmux-gateway/src/server/logging_middleware.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ fn redact_headers_compact(headers: &axum::http::HeaderMap) -> String {
4545
| "user-agent"
4646
| "mcp-session-id"
4747
| "mcp-protocol-version"
48+
| "last-event-id"
4849
)
4950
})
5051
.map(|(name, value)| {
@@ -208,7 +209,23 @@ pub async fn http_logging_middleware(request: Request, next: Next) -> Result<Res
208209
let response = next.run(request).await;
209210
let status = response.status().as_u16();
210211

211-
// Extract response body to log it
212+
// Check if this is a streaming response (SSE) — NEVER buffer these.
213+
// SSE responses have Content-Type: text/event-stream and are infinite
214+
// streams. Calling body.collect() would block forever, preventing VS Code
215+
// from receiving any SSE events (notifications, keep-alive pings).
216+
let is_streaming = response
217+
.headers()
218+
.get("content-type")
219+
.and_then(|v| v.to_str().ok())
220+
.is_some_and(|ct| ct.contains("text/event-stream"));
221+
222+
if is_streaming {
223+
// For SSE streams, log entry only and pass through without touching body
224+
RequestSpan::log_exit(&ctx, status, None);
225+
return Ok(response);
226+
}
227+
228+
// For non-streaming responses, capture body for logging
212229
let (parts, body) = response.into_parts();
213230
let body_bytes = match body.collect().await {
214231
Ok(collected) => collected.to_bytes(),

crates/mcpmux-gateway/src/server/mod.rs

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -240,29 +240,27 @@ impl GatewayServer {
240240
let handler =
241241
McpMuxGatewayHandler::new(Arc::new(self.services.clone()), notification_bridge.clone());
242242

243-
// Create STATELESS MCP service
244-
// stateful_mode: false means:
245-
// - No Mcp-Session-Id header
246-
// - GET/DELETE return 405 automatically (no notification streams)
247-
// - Each POST is independent
248-
// - Avoids the stream management issues that caused connection loops
249-
// Trade-off: Cannot send list_changed notifications
243+
// Create STATEFUL MCP service (full Streamable HTTP per spec 2025-11-25)
244+
// stateful_mode: true means:
245+
// - Mcp-Session-Id header for session management
246+
// - GET endpoint for SSE streams (server-initiated notifications)
247+
// - DELETE endpoint for session termination
248+
// - list_changed notifications delivered via SSE
250249
let mcp_service = StreamableHttpService::new(
251250
move || {
252-
debug!("[Gateway] Creating handler instance for MCP request");
251+
debug!("[Gateway] Creating handler instance for MCP session");
253252
Ok(handler.clone())
254253
},
255254
LocalSessionManager::default().into(),
256255
StreamableHttpServerConfig {
257-
stateful_mode: false,
256+
stateful_mode: true,
258257
sse_keep_alive: Some(std::time::Duration::from_secs(30)),
259-
sse_retry: None,
258+
sse_retry: Some(std::time::Duration::from_secs(3)),
260259
cancellation_token: CancellationToken::new(),
261260
},
262261
);
263262

264263
// Wrap MCP service with OAuth middleware
265-
// In stateless mode, no session healing needed - rmcp handles 405 for GET/DELETE
266264
let mcp_routes =
267265
Router::new()
268266
.nest_service("/mcp", mcp_service)

crates/mcpmux-mcp/src/transports.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ impl McpClientHandler {
9393
title: Some("McpMux Gateway".to_string()),
9494
icons: None,
9595
website_url: None,
96+
..Default::default()
9697
},
9798
meta: None,
9899
},

0 commit comments

Comments
 (0)