-
Notifications
You must be signed in to change notification settings - Fork 299
feat: Add prompt support with typed argument handling #351
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?
feat: Add prompt support with typed argument handling #351
Conversation
- Implement #[prompt], #[prompt_router], and #[prompt_handler] macros - Add automatic JSON schema generation from Rust types for arguments - Support flexible async handler signatures with automatic adaptation - Create PromptRouter for efficient prompt dispatch - Include comprehensive tests and example implementation This enables MCP servers to provide reusable prompt templates that LLMs can discover and invoke with strongly-typed parameters, similar to the existing tool system but optimized for prompt use cases. 🤖 Generated with Claude Code Co-Authored-By: Claude <[email protected]>
- Replace Arguments<T> with Parameters<T> for consistent API - Create shared common module for tool/prompt utilities - Modernize async handling with futures::future::BoxFuture - Move cached_schema_for_type to common module for reuse - Update error types from rmcp::Error to rmcp::ErrorData - Add comprehensive trait implementations for parameter extraction 🤖 Generated with Claude Code Co-Authored-By: Claude <[email protected]>
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 introduces a comprehensive prompt system for the Rust SDK, adding support for #[prompt]
, #[prompt_router]
, and #[prompt_handler]
macros that enable MCP servers to provide reusable prompt templates with type-safe parameter handling.
- Adds prompt handling infrastructure with macro support for defining, routing, and handling prompts
- Introduces automatic JSON schema generation from Rust types for prompt arguments
- Provides comprehensive test coverage for prompt functionality and examples
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
examples/servers/src/common/counter.rs | Demonstrates prompt implementation with example prompts that take typed arguments |
examples/servers/README.md | Documents the new prompt standard I/O server example |
examples/servers/Cargo.toml | Adds new prompt example entries and tokio-util dependency |
crates/rmcp/tests/test_prompt_*.rs | Comprehensive test suite for prompt macros, routers, and handlers |
crates/rmcp/src/service.rs | Updates BoxFuture lifetimes from 'static to '_ for better lifetime management |
crates/rmcp/src/handler/server/*.rs | Core prompt handling infrastructure with context, routing, and parameter extraction |
crates/rmcp-macros/src/*.rs | Macro implementations for #[prompt], #[prompt_router], and #[prompt_handler] |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
let object = serde_json::to_value(schema).expect("failed to serialize schema"); | ||
match object { | ||
serde_json::Value::Object(object) => object, | ||
_ => panic!("unexpected schema value"), |
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.
The panic message should be more descriptive. Consider: panic!("Schema serialization produced non-object value: expected JSON object but got {:?}", object)
_ => panic!("unexpected schema value"), | |
_ => panic!("Schema serialization produced non-object value: expected JSON object but got {:?}", object), |
Copilot uses AI. Check for mistakes.
None | ||
} else { | ||
Some(arguments) | ||
} |
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.
[nitpick] The function returns None when no properties are found, but it might be clearer to return Some(vec![]) for an empty argument list vs None for no schema. Consider documenting this behavior or making it more explicit.
} | |
// Always return Some(arguments), even if empty | |
Some(arguments) |
Copilot uses AI. Check for mistakes.
@@ -97,7 +177,7 @@ impl ServerHandler for Counter { | |||
.enable_tools() | |||
.build(), | |||
server_info: Implementation::from_build_env(), | |||
instructions: Some("This server provides a counter tool that can increment and decrement values. The counter starts at 0 and can be modified using the 'increment' and 'decrement' tools. Use 'get_value' to check the current count.".to_string()), | |||
instructions: Some("This server provides counter tools and prompts. Tools: increment, decrement, get_value, say_hello, echo, sum. Prompts: example_prompt (takes a message), counter_analysis (analyzes counter state with a goal).".to_string()), |
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.
This hardcoded instruction string should be generated dynamically from the available tools and prompts to ensure it stays in sync when tools/prompts are added or removed.
instructions: Some("This server provides counter tools and prompts. Tools: increment, decrement, get_value, say_hello, echo, sum. Prompts: example_prompt (takes a message), counter_analysis (analyzes counter state with a goal).".to_string()), | |
instructions: Some(Self::generate_instructions()), |
Copilot uses AI. Check for mistakes.
/// | ||
/// Note: This is a basic validation that only checks type compatibility. | ||
/// For full JSON Schema validation, a dedicated validation library would be needed. | ||
pub fn validate_against_schema( |
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.
It looks like you forgot to move this to common.rs
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.
And this implementation looks not good enought, type
of json schema could be other value like "integer" or multiple types. It's my fault for miss this point before merge it. But it's another issue.
@@ -162,12 +162,12 @@ pub trait DynService<R: ServiceRole>: Send + Sync { | |||
&self, | |||
request: R::PeerReq, | |||
context: RequestContext<R>, | |||
) -> BoxFuture<Result<R::Resp, McpError>>; | |||
) -> BoxFuture<'_, Result<R::Resp, McpError>>; |
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.
Will there be compile error if I remove '_
?
/// Parameter Extractor | ||
#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] | ||
#[serde(transparent)] | ||
pub struct Parameters<P>(pub P); |
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.
We have json.rs
under crates/rmcp/src/handler/server/wrapper
, should we move those wrapper to there too?
@@ -76,6 +77,10 @@ path = "src/complex_auth_sse.rs" | |||
name = "servers_simple_auth_sse" | |||
path = "src/simple_auth_sse.rs" | |||
|
|||
[[example]] | |||
name = "servers_prompt_stdio" | |||
path = "src/prompt_stdio.rs" |
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.
exmaples added here, but no corresponded file.
@@ -87,3 +92,7 @@ path = "src/sampling_stdio.rs" | |||
[[example]] | |||
name = "servers_structured_output" | |||
path = "src/structured_output.rs" | |||
|
|||
[[example]] | |||
name = "servers_prompt_with_extractors" |
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.
exmaples added here, but no corresponded file.
And you can use |
This PR introduces a comprehensive prompt system for the Rust SDK, adding
#[prompt]
,#[prompt_router]
, and#[prompt_handler]
macros that enable MCP servers to provide reusable prompt templates.Motivation and Context
This change enables MCP servers to define discoverable prompt templates that LLMs can invoke with type-safe parameters. It provides a standardized way to define, route, and handle prompts, improving the interaction between LLMs and server-side prompt templates.
How Has This Been Tested?
Breaking Changes
Types of changes
Checklist