Skip to content

Conversation

@mrchantey
Copy link
Contributor

The list_tools function now accepts Option<PaginatedRequestParam> but the macro wasn't updated for generic servers. This pr fixes the error and adds the implementation to the test to avoid future regressions.

#[tool(tool_box)]
impl<DS: DataService> ServerHandler for GenericServer<DS> {}

/// error:


error[E0053]: method `list_tools` has an incompatible type for trait
  --> crates/rmcp/tests/test_tool_macros.rs:86:1
   |
86 | #[tool(tool_box)]
   | ^^^^^^^^^^^^^^^^^ expected `Option<PaginatedRequestParam>`, found `PaginatedRequestParam`
   |
   = note: expected signature `fn(&GenericServer<_>, std::option::Option<PaginatedRequestParam>, RequestContext<_>) -> _`
              found signature `fn(&GenericServer<_>, PaginatedRequestParam, RequestContext<_>) -> _`
   = note: this error originates in the attribute macro `tool` (in Nightly builds, run with -Z macro-backtrace for more info)
help: change the parameter type to match the trait
   |
86 | std::option::Option<PaginatedRequestParam>

@4t145
Copy link
Collaborator

4t145 commented May 26, 2025

You can rewrite commit history by using force push: git push -f

Copy link
Collaborator

@jokemanfire jokemanfire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait , I'm confusing the ut not cover it before?

@mrchantey
Copy link
Contributor Author

Thats right, its a bit confusing because the #[tool(tool_box)] attribute is identical for impl MyServer and impl ServerHandler for MyServer:

// existing coverage
#[tool(tool_box)]
impl<DS: DataService> GenericServer<DS> {
  ...
}
// new coverage
#[tool(tool_box)]
impl<DS: DataService> ServerHandler for GenericServer<DS> {}

@jokemanfire
Copy link
Collaborator

Thanks for this pr ,the example has some problem

@jokemanfire jokemanfire merged commit 4e8686f into modelcontextprotocol:main May 26, 2025
10 checks passed
@github-actions github-actions bot mentioned this pull request Jul 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants