From bf16e06ca760a4f2f81d69656b1c4529be440d69 Mon Sep 17 00:00:00 2001 From: Dale Seo <5466341+DaleSeo@users.noreply.github.com> Date: Tue, 28 Jul 2026 09:54:22 -0400 Subject: [PATCH] fix: gate client handler bounds for local --- crates/rmcp/Cargo.toml | 5 + crates/rmcp/src/handler/client.rs | 365 +++++++++--------- .../rmcp/tests/test_local_client_handler.rs | 16 + 3 files changed, 210 insertions(+), 176 deletions(-) create mode 100644 crates/rmcp/tests/test_local_client_handler.rs diff --git a/crates/rmcp/Cargo.toml b/crates/rmcp/Cargo.toml index 5d2bea792..44c064418 100644 --- a/crates/rmcp/Cargo.toml +++ b/crates/rmcp/Cargo.toml @@ -238,6 +238,11 @@ required-features = [ ] path = "tests/test_with_js.rs" +[[test]] +name = "test_local_client_handler" +required-features = ["client", "local"] +path = "tests/test_local_client_handler.rs" + [[test]] name = "test_notification" required-features = ["server", "client"] diff --git a/crates/rmcp/src/handler/client.rs b/crates/rmcp/src/handler/client.rs index d61070b61..532ca7ed2 100644 --- a/crates/rmcp/src/handler/client.rs +++ b/crates/rmcp/src/handler/client.rs @@ -85,189 +85,202 @@ impl Service for H { } } -#[allow(unused_variables)] -pub trait ClientHandler: Sized + Send + Sync + 'static { - fn ping( - &self, - context: RequestContext, - ) -> impl Future> + MaybeSendFuture + '_ { - std::future::ready(Ok(())) - } +macro_rules! client_handler_methods { + () => { + fn ping( + &self, + context: RequestContext, + ) -> impl Future> + MaybeSendFuture + '_ { + std::future::ready(Ok(())) + } - fn create_message( - &self, - params: CreateMessageRequestParams, - context: RequestContext, - ) -> impl Future> + MaybeSendFuture + '_ { - std::future::ready(Err( - McpError::method_not_found::(), - )) - } + fn create_message( + &self, + params: CreateMessageRequestParams, + context: RequestContext, + ) -> impl Future> + MaybeSendFuture + '_ { + std::future::ready(Err( + McpError::method_not_found::(), + )) + } - fn list_roots( - &self, - context: RequestContext, - ) -> impl Future> + MaybeSendFuture + '_ { - std::future::ready(Ok(ListRootsResult::default())) - } + fn list_roots( + &self, + context: RequestContext, + ) -> impl Future> + MaybeSendFuture + '_ { + std::future::ready(Ok(ListRootsResult::default())) + } - /// Handle an elicitation request from a server asking for user input. - /// - /// This method is called when a server needs interactive input from the user - /// during tool execution. Implementations should present the message to the user, - /// collect their input according to the requested schema, and return the result. - /// - /// # Arguments - /// * `request` - The elicitation request with message and schema - /// * `context` - The request context - /// - /// # Returns - /// The user's response including action (accept/decline/cancel) and optional data - /// - /// # Default Behavior - /// The default implementation automatically declines all elicitation requests. - /// Real clients should override this to provide user interaction. - /// - /// # Example - /// ```rust,ignore - /// use rmcp::model::ElicitRequestParams; - /// use rmcp::{ - /// model::ErrorData as McpError, - /// model::*, - /// service::{NotificationContext, RequestContext, RoleClient, Service, ServiceRole}, - /// }; - /// use rmcp::ClientHandler; - /// - /// impl ClientHandler for MyClient { - /// async fn create_elicitation( - /// &self, - /// request: ElicitRequestParams, - /// context: RequestContext, - /// ) -> Result { - /// match request { - /// ElicitRequestParams::FormElicitationParam {meta, message, requested_schema,} => { - /// // Display message to user and collect input according to requested_schema - /// let user_input = get_user_input(message, requested_schema).await?; - /// Ok(ElicitResult { - /// action: ElicitationAction::Accept, - /// content: Some(user_input), - /// meta: None, - /// }) - /// } - /// ElicitRequestParams::UrlElicitationParam {meta, message, url, elicitation_id,} => { - /// // Open URL in browser for user to complete elicitation - /// open_url_in_browser(url).await?; - /// Ok(ElicitResult { - /// action: ElicitationAction::Accept, - /// content: None, - /// meta: None, - /// }) - /// } - /// } - /// } - /// } - /// ``` - fn create_elicitation( - &self, - request: ElicitRequestParams, - context: RequestContext, - ) -> impl Future> + MaybeSendFuture + '_ { - // Default implementation declines all requests - real clients should override this - let _ = (request, context); - std::future::ready(Ok(ElicitResult { - action: ElicitationAction::Decline, - content: None, - meta: None, - })) - } + /// Handle an elicitation request from a server asking for user input. + /// + /// This method is called when a server needs interactive input from the user + /// during tool execution. Implementations should present the message to the user, + /// collect their input according to the requested schema, and return the result. + /// + /// # Arguments + /// * `request` - The elicitation request with message and schema + /// * `context` - The request context + /// + /// # Returns + /// The user's response including action (accept/decline/cancel) and optional data + /// + /// # Default Behavior + /// The default implementation automatically declines all elicitation requests. + /// Real clients should override this to provide user interaction. + /// + /// # Example + /// ```rust,ignore + /// use rmcp::model::ElicitRequestParams; + /// use rmcp::{ + /// model::ErrorData as McpError, + /// model::*, + /// service::{NotificationContext, RequestContext, RoleClient, Service, ServiceRole}, + /// }; + /// use rmcp::ClientHandler; + /// + /// impl ClientHandler for MyClient { + /// async fn create_elicitation( + /// &self, + /// request: ElicitRequestParams, + /// context: RequestContext, + /// ) -> Result { + /// match request { + /// ElicitRequestParams::FormElicitationParam {meta, message, requested_schema,} => { + /// // Display message to user and collect input according to requested_schema + /// let user_input = get_user_input(message, requested_schema).await?; + /// Ok(ElicitResult { + /// action: ElicitationAction::Accept, + /// content: Some(user_input), + /// meta: None, + /// }) + /// } + /// ElicitRequestParams::UrlElicitationParam {meta, message, url, elicitation_id,} => { + /// // Open URL in browser for user to complete elicitation + /// open_url_in_browser(url).await?; + /// Ok(ElicitResult { + /// action: ElicitationAction::Accept, + /// content: None, + /// meta: None, + /// }) + /// } + /// } + /// } + /// } + /// ``` + fn create_elicitation( + &self, + request: ElicitRequestParams, + context: RequestContext, + ) -> impl Future> + MaybeSendFuture + '_ { + // Default implementation declines all requests - real clients should override this + let _ = (request, context); + std::future::ready(Ok(ElicitResult { + action: ElicitationAction::Decline, + content: None, + meta: None, + })) + } - fn on_custom_request( - &self, - request: CustomRequest, - context: RequestContext, - ) -> impl Future> + MaybeSendFuture + '_ { - let CustomRequest { method, .. } = request; - let _ = context; - std::future::ready(Err(McpError::new( - ErrorCode::METHOD_NOT_FOUND, - method, - None, - ))) - } + fn on_custom_request( + &self, + request: CustomRequest, + context: RequestContext, + ) -> impl Future> + MaybeSendFuture + '_ { + let CustomRequest { method, .. } = request; + let _ = context; + std::future::ready(Err(McpError::new( + ErrorCode::METHOD_NOT_FOUND, + method, + None, + ))) + } - fn on_cancelled( - &self, - params: CancelledNotificationParam, - context: NotificationContext, - ) -> impl Future + MaybeSendFuture + '_ { - std::future::ready(()) - } - fn on_progress( - &self, - params: ProgressNotificationParam, - context: NotificationContext, - ) -> impl Future + MaybeSendFuture + '_ { - std::future::ready(()) - } - fn on_logging_message( - &self, - params: LoggingMessageNotificationParam, - context: NotificationContext, - ) -> impl Future + MaybeSendFuture + '_ { - std::future::ready(()) - } - fn on_resource_updated( - &self, - params: ResourceUpdatedNotificationParam, - context: NotificationContext, - ) -> impl Future + MaybeSendFuture + '_ { - std::future::ready(()) - } - fn on_resource_list_changed( - &self, - context: NotificationContext, - ) -> impl Future + MaybeSendFuture + '_ { - std::future::ready(()) - } - fn on_tool_list_changed( - &self, - context: NotificationContext, - ) -> impl Future + MaybeSendFuture + '_ { - std::future::ready(()) - } - fn on_prompt_list_changed( - &self, - context: NotificationContext, - ) -> impl Future + MaybeSendFuture + '_ { - std::future::ready(()) - } - fn on_subscriptions_acknowledged( - &self, - params: SubscriptionsAcknowledgedNotificationParams, - context: NotificationContext, - ) -> impl Future + MaybeSendFuture + '_ { - std::future::ready(()) - } + fn on_cancelled( + &self, + params: CancelledNotificationParam, + context: NotificationContext, + ) -> impl Future + MaybeSendFuture + '_ { + std::future::ready(()) + } + fn on_progress( + &self, + params: ProgressNotificationParam, + context: NotificationContext, + ) -> impl Future + MaybeSendFuture + '_ { + std::future::ready(()) + } + fn on_logging_message( + &self, + params: LoggingMessageNotificationParam, + context: NotificationContext, + ) -> impl Future + MaybeSendFuture + '_ { + std::future::ready(()) + } + fn on_resource_updated( + &self, + params: ResourceUpdatedNotificationParam, + context: NotificationContext, + ) -> impl Future + MaybeSendFuture + '_ { + std::future::ready(()) + } + fn on_resource_list_changed( + &self, + context: NotificationContext, + ) -> impl Future + MaybeSendFuture + '_ { + std::future::ready(()) + } + fn on_tool_list_changed( + &self, + context: NotificationContext, + ) -> impl Future + MaybeSendFuture + '_ { + std::future::ready(()) + } + fn on_prompt_list_changed( + &self, + context: NotificationContext, + ) -> impl Future + MaybeSendFuture + '_ { + std::future::ready(()) + } + fn on_subscriptions_acknowledged( + &self, + params: SubscriptionsAcknowledgedNotificationParams, + context: NotificationContext, + ) -> impl Future + MaybeSendFuture + '_ { + std::future::ready(()) + } - fn on_task_status( - &self, - params: TaskStatusNotificationParams, - context: NotificationContext, - ) -> impl Future + MaybeSendFuture + '_ { - std::future::ready(()) - } - fn on_custom_notification( - &self, - notification: CustomNotification, - context: NotificationContext, - ) -> impl Future + MaybeSendFuture + '_ { - let _ = (notification, context); - std::future::ready(()) - } + fn on_task_status( + &self, + params: TaskStatusNotificationParams, + context: NotificationContext, + ) -> impl Future + MaybeSendFuture + '_ { + std::future::ready(()) + } + fn on_custom_notification( + &self, + notification: CustomNotification, + context: NotificationContext, + ) -> impl Future + MaybeSendFuture + '_ { + let _ = (notification, context); + std::future::ready(()) + } - fn get_info(&self) -> ClientInfo { - ClientInfo::default() - } + fn get_info(&self) -> ClientInfo { + ClientInfo::default() + } + }; +} + +#[allow(unused_variables)] +#[cfg(not(feature = "local"))] +pub trait ClientHandler: Sized + Send + Sync + 'static { + client_handler_methods!(); +} + +#[allow(unused_variables)] +#[cfg(feature = "local")] +pub trait ClientHandler: Sized + 'static { + client_handler_methods!(); } /// Do nothing, with default client info. diff --git a/crates/rmcp/tests/test_local_client_handler.rs b/crates/rmcp/tests/test_local_client_handler.rs new file mode 100644 index 000000000..e85d3d8c9 --- /dev/null +++ b/crates/rmcp/tests/test_local_client_handler.rs @@ -0,0 +1,16 @@ +use std::rc::Rc; + +use rmcp::ClientHandler; + +struct LocalClientHandler { + _state: Rc<()>, +} + +impl ClientHandler for LocalClientHandler {} + +#[test] +fn client_handler_accepts_non_send_sync_state_with_local_feature() { + fn assert_client_handler() {} + + assert_client_handler::(); +}