-
Notifications
You must be signed in to change notification settings - Fork 401
refactor: refactor tool macros and router implementation #261
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
Conversation
- Updated the `#[tool(tool_box)]` macro to `#[tool_router]` across various modules for consistency. - Enhanced the `Calculator`, `Counter`, and `GenericService` structs to utilize `ToolRouter` for handling tool calls. - Introduced `Parameters` struct for better parameter handling in tool functions. - Added new methods for listing tools and calling tools in server handlers. - Improved test cases to reflect changes in tool routing and parameter handling. - Updated documentation and examples to align with the new router structure.
|
@jokemanfire It's ready for review now |
…an up unused code in server handler
|
We can collect docs like this: // extract doc line from attribute
fn extract_doc_line(existing_docs: Option<String>, attr: &syn::Attribute) -> Option<String> {
if !attr.path().is_ident("doc") {
return None;
}
let syn::Meta::NameValue(name_value) = &attr.meta else {
return None;
};
let syn::Expr::Lit(expr_lit) = &name_value.value else {
return None;
};
let syn::Lit::Str(lit_str) = &expr_lit.lit else {
return None;
};
let content = lit_str.value().trim().to_string();
match (existing_docs, content) {
(Some(mut existing_docs), content) if !content.is_empty() => {
existing_docs.push('\n');
existing_docs.push_str(&content);
Some(existing_docs)
}
(Some(existing_docs), _) => Some(existing_docs),
(None, content) if !content.is_empty() => Some(content),
_ => None,
}
}and fn_item.attrs.iter().fold(None, extract_doc_line)And check again please. |
|
I will give a feedback tonight. |
|
|
||
| ```rust ignore | ||
| #[tool(tool_box)] | ||
| #[tool_router] |
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 like tool_router 's expand is in func 'new' , The struct must have 'ToolRouter', I think it should be write in this readme
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.
Not always, a static router is also ok. I will add more examples.
| _request: Option<rmcp::model::PaginatedRequestParam>, | ||
| _context: rmcp::service::RequestContext<rmcp::RoleServer>, | ||
| ) -> Result<ListToolsResult, rmcp::Error> { | ||
| let items = self.tool_router.list_all(); |
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 marco 'tool_handler' will expand the call_tool and list_tools func , why we need clarify it again?
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 will use #[tool_handler] as much as possible, but remain one place to explain what it looks like after expanded
|
The tool_handler marco looks like not really using? |
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 refactors the tool macros and router implementation to simplify parameter handling, remove attributes on function parameters, and replace the deprecated tool_box macro with the new tool_router and tool_handler macros.
- Updated macro usage and renaming (tool_box → tool_router/tool_handler).
- Introduced constructor methods (e.g. Calculator::new()) in multiple examples.
- Modified function signatures to use a wrapping Parameters type for tool functions.
Reviewed Changes
Copilot reviewed 31 out of 33 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| justfile | Added basic formatting and lint/fix tasks. |
| examples/wasi/src/lib.rs | Updated Calculator instantiation using Calculator::new(). |
| examples/wasi/src/calculator.rs | Refactored Calculator, updated parameter extraction and return types. |
| examples/transport/src/{websocket,unix_socket,tcp,http_upgrade}.rs | Updated server instantiations to use Calculator::new(). |
| examples/transport/src/common/calculator.rs | Refactored calculator module with new macros and added capabilities. |
| examples/servers/src/common/{generic_service,counter,calculator}.rs | Updated tool function signatures and macro usage. |
| crates/rmcp/tests/* | Updated tests to align with new macro signatures. |
| crates/rmcp/src/{model,handler,handler/server/tool.rs,handler/server/router/tool.rs,handler/server/router.rs,handler/server.rs,README.md} | Updated internal macros and API usage throughout the codebase. |
| crates/rmcp-macros/* | Updated macro implementations and documentation for new tools. |
Comments suppressed due to low confidence (2)
examples/wasi/src/calculator.rs:50
- The 'sub' tool now returns a Json-wrapped i32 while 'sum' returns a String. Consider unifying the return types for consistency unless the difference is intentional.
fn sub(&self, Parameters(SubRequest { a, b }): Parameters<SubRequest>) -> Json<i32> {
examples/servers/src/common/counter.rs:73
- In the 'echo' function, the JSON object is converted to a string before being wrapped in Content::text. Verify that this conversion is the intended design, as it may limit downstream processing of the original JSON structure.
fn echo(&self, Parameters(object): Parameters<JsonObject>) -> Result<CallToolResult, McpError> {
|
@jokemanfire check again please |
Motivation and Context
Refactor tool macros:
How Has This Been Tested?
Breaking Changes
Now we need to use a combination of three macros:
tool
This macro is used to mark a function as a tool handler.
This will generate a function that return the attribute of this tool, with type
rmcp::model::Tool.Usage
nameStringdescriptionStringinput_schemaExprParameters<T>annotationsToolAnnotationsAttributeNone.Example
tool_router
This macro is used to generate a tool router based on functions marked with
#[rmcp::tool]in an implementation block.It creates a function that returns a
ToolRouterinstance.Usage
routerIdenttool_router.visVisibilityExample
Or specify the visibility and router name:
tool_handler
This macro will generate the handler for
tool_callandlist_toolsmethods in the implementation block, by using an existingToolRouterinstance.Usage
routerExprToolRouterinstance. Defaults toself.tool_router.Example
or using a custom router expression:
Types of changes
Checklist
Additional context
This also can be a template for possible prompt handler in the future. And also can be a base of implementation of
#[derive(ServerHandler)]