-
Notifications
You must be signed in to change notification settings - Fork 299
feat: Add MCP Elicitation support #332
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.
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
base: main
Are you sure you want to change the base?
Conversation
Adds comprehensive elicitation functionality according to MCP 2025-06-18 specification: Core Features: - ElicitationAction enum (Accept, Decline, Cancel) - CreateElicitationRequestParam and CreateElicitationResult structures - Protocol version V_2025_06_18 with elicitation methods - Full JSON-RPC integration with method constants Capabilities Integration: - ElicitationCapability with schema validation support - ClientCapabilities builder pattern integration - enable_elicitation() and enable_elicitation_schema_validation() methods Handler Support: - create_elicitation method in ClientHandler and ServerHandler traits - Integration with existing request/response union types - Async/await compatible implementation Service Layer: - Basic create_elicitation method via macro expansion - Four convenience methods for common scenarios: * elicit_confirmation() - yes/no questions * elicit_text_input() - string input with optional requirements * elicit_choice() - selection from multiple options * elicit_structured_input() - complex data via JSON Schema Comprehensive Testing: - 11 test cases covering all functionality aspects - JSON serialization/deserialization validation - MCP specification compliance verification - Error handling and edge cases - Performance benchmarks - Capabilities integration tests All tests pass and code follows project standards.
- Add new 'elicitation' feature that depends on 'client' and 'schemars' - Implement elicit<T>() method for type-safe elicitation with automatic schema generation - Remove convenience methods (elicit_confirmation, elicit_text_input, elicit_choice) - Add ElicitationError enum with detailed error variants: - Service: underlying service errors - UserDeclined: user cancelled or declined request - ParseError: response parsing failed with context - NoContent: no response content provided - Update documentation with comprehensive examples and error handling - Add comprehensive tests for typed elicitation and error handling
- Remove CreateElicitationRequest from ClientRequest - clients cannot initiate elicitation - Move elicit methods from client to server - servers now request user input - Add comprehensive direction tests verifying Server→Client→Server flow - Maintain CreateElicitationResult in ClientResult for proper responses - Update handlers to reflect correct message routing - Add elicitation feature flag for typed schema generation Fixes elicitation direction to match specification where servers request interactive user input from clients, not the reverse.
- Add supports_elicitation() method to check client capabilities - Add CapabilityNotSupported error variant to ElicitationError - Update elicit_structured_input() to check capabilities before execution - Update elicit<T>() method to check capabilities before execution - Add comprehensive tests for capability checking functionality - Tests verify that servers check client capabilities before sending elicitation requests - Ensures compliance with MCP 2025-06-18 specification requirement
I think we might want to consider implementing different request options (including timeouts) depending on the request type. For elicitation calls, they could potentially require longer timeouts and would benefit from being configurable. |
crates/rmcp/src/model.rs
Outdated
pub const V_2025_03_26: Self = Self(Cow::Borrowed("2025-03-26")); | ||
pub const V_2024_11_05: Self = Self(Cow::Borrowed("2024-11-05")); | ||
pub const LATEST: Self = Self::V_2025_03_26; | ||
pub const LATEST: Self = Self::V_2025_06_18; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't change it to 20250618 now, since we haven't implment all the new features
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! Reverted LATEST back to V_2025_03_26 in commit 923d9aa.
Sorry for late response, it looks good to me. |
CI fail , check pls |
…RoleServer - Move (supports_elicitation, elicit_structured_input, elicit) to separate impl block - Move ElicitationError definition to elicitation methods section - Keep base methods (create_message, list_roots, notify_*) in main impl block with macro - Add section comments to distinguish general and elicitation-specific methods
923d9aa
to
01dd3e8
Compare
- Remove assertions for V_2025_06_18 protocol version
According to MCP specification and PR feedback, decline and cancel actions should be handled differently: - UserDeclined: explicit user rejection (clicked "Decline", "No", etc.) - UserCancelled: dismissal without explicit choice (closed dialog, Escape, etc.) Changes: - Split ElicitationError::UserDeclined into two distinct error types - Update error handling logic to map each ElicitationAction correctly - Improve documentation with proper action semantics - Add comprehensive tests for new error types and action mapping - Update examples to demonstrate proper error handling This provides better error granularity allowing servers to handle explicit declines vs cancellations appropriately as per MCP spec.
Add ElicitationSafe trait and elicit_safe\! macro to ensure elicit<T>() methods are only used with types that generate appropriate JSON object schemas, addressing type safety concerns from PR feedback. Features: - ElicitationSafe marker trait for compile-time constraints - elicit_safe\! macro for opt-in type safety declaration - Updated elicit<T> and elicit_with_timeout<T> to require ElicitationSafe bound - Comprehensive documentation with examples and rationale - Full test coverage for new type safety features This prevents common mistakes like: - elicit::<String>() - primitives not suitable for object schemas - elicit::<Vec<i32>>() - arrays don't match client expectations Breaking change: Existing code must add elicit_safe\!(TypeName) declarations for types used with elicit methods. This is an intentional safety improvement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable to me, but I question the addition of the validate_timeout
function.
crates/rmcp/src/service/server.rs
Outdated
@@ -3,6 +3,34 @@ use std::borrow::Cow; | |||
use thiserror::Error; | |||
|
|||
use super::*; | |||
|
|||
/// Validates timeout values to prevent DoS attacks and ensure reasonable limits | |||
fn validate_timeout(timeout: Option<std::time::Duration>) -> Result<(), ServiceError> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm is this necessary? Why does it need to be limited to 1ms-5min? Values outside of that might be very uncommon but I'd think that should be left to the server developer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jamadeo You're absolutely right. Hard-coding these limits was an oversight - server developers should have control over timeout policies for their specific use cases.
I'll remove the validation and let developers implement their own timeout constraints as needed. The library shouldn't enforce arbitrary business logic decisions.
Thanks for catching this!
This reverts commit 8296242.
- Remove invalid async/await usage in doctest example - Comment out the actual usage line to show intent without compilation errors - Maintain clear documentation of the macro's purpose and usage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements comprehensive MCP Elicitation support according to the MCP 2025-06-18 specification, enabling servers to request interactive user input during tool execution with full JSON Schema validation and type-safe convenience methods.
- Added core elicitation data structures and types for the "elicitation/create" protocol
- Implemented server-side convenience methods with automatic schema generation for typed data collection
- Added client capabilities system integration with ElicitationCapability support
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
crates/rmcp/tests/test_message_schema/server_json_rpc_message_schema.json | Added JSON schema definitions for elicitation request/response structures |
crates/rmcp/tests/test_message_schema/client_json_rpc_message_schema.json | Added client-side elicitation capability and result schema definitions |
crates/rmcp/tests/test_elicitation.rs | Comprehensive test suite covering serialization, protocol compliance, and typed elicitation functionality |
crates/rmcp/src/service/server.rs | Added elicitation convenience methods, error types, and typed elicitation with timeout support |
crates/rmcp/src/model/meta.rs | Added CreateElicitationRequest to ServerRequest variant extensions |
crates/rmcp/src/model/capabilities.rs | Added ElicitationCapability struct and builder methods for capability negotiation |
crates/rmcp/src/model.rs | Added core elicitation types, protocol version, and JSON-RPC integration |
crates/rmcp/src/handler/client.rs | Added client handler method for processing elicitation requests |
crates/rmcp/Cargo.toml | Added "elicitation" feature flag and test configuration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
||
// Verify the list doesn't contain elicitation | ||
assert!(!valid_client_requests.contains(&"CreateElicitationRequest")); | ||
assert_eq!(valid_client_requests.len(), 13); // Should be 13, not 14 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard-coded magic number 13 in the assertion makes the test brittle. If new request types are added, this test will break. Consider using the actual length of the array or a more robust validation approach.
assert_eq!(valid_client_requests.len(), 13); // Should be 13, not 14 |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get the point of this testcase, it looks like some LLM generated stuff
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're absolutely right - I missed cleaning up this redundant code during refactoring. The presence of CreateElicitationRequest
in ServerRequest
is already verified by the test_elicitation_direction_server_to_client
test, which
comprehensively checks the protocol direction constraints.
This match block can be safely removed since it's purely duplicative testing. I'll clean this up.
- Remove test_elicitation_not_in_client_request (duplicated functionality) - Remove redundant ServerRequest match in test_elicitation_direction_server_to_client - Direction compliance is already verified by the remaining comprehensive test - Reduces test fragility and maintenance burden
@jamadeo Could you review again? |
I tested this out from a client perspective by adding a trivial handler in Goose + the "everything" test server, and looked good! |
Thanks , LGTM , If you can add an example about this feature, that will be greater. |
@@ -324,11 +328,68 @@ macro_rules! method { | |||
Ok(()) | |||
} | |||
}; | |||
|
|||
// Timeout-only variants (base method should be created separately with peer_req) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these two macros have same public code ,we can extract it.
- Add elicitation server example demonstrating real MCP usage - Implement greet_user tool with context.peer.elicit::<T>() API - Show type-safe elicitation with elicit_safe! macro - Include reset_name tool and MCP Inspector instructions - Update examples documentation and dependencies
@jokemanfire Added a simple elicitation example demonstrating the feature in action |
This PR implements comprehensive MCP Elicitation support according to the MCP 2025-06-18 specification, enabling servers to request interactive user input during tool execution with full JSON Schema validation and type-safe convenience methods.
Closes #304
Motivation and Context
Elicitation functionality introduced in the MCP 2025-06-18 specification. This allows servers to create interactive workflows that require user input during tool execution.
Solution: This change adds complete elicitation support, enabling:
Context: Based on MCP PR #382 which introduced elicitation to the MCP specification. This implementation ensures rust-sdk maintains full feature parity with the latest MCP standards.
How Has This Been Tested?
Comprehensive Test Suite (11 test cases):
Quality Assurance:
cargo check
- No compilation errorscargo clippy
- No linting warningscargo test
- All 11 elicitation tests passcargo fmt
- Code properly formattedScenarios Tested:
Breaking Changes
No breaking changes - This is a purely additive implementation:
Types of changes
Checklist
Additional context
Implementation Highlights:
Core Architecture:
Accept
,Decline
,Cancel
actionsCreateElicitationRequestParam
,CreateElicitationResult
Developer Experience:
elicit_structured_input()
- Complex JSON Schema validationelicit<T>()
- Typed data request with JSON auto generationCapabilities System:
enable_elicitation()
,enable_elicitation_schema_validation()
This implementation makes rust-sdk compatible with MCP 2025-06-18 and enables interactive workflows while maintaining full backward compatibility.