Uh oh!
There was an error while loading. Please reload this page.
Implementation of SEP-986: Specify Format for Tool Names - #551
Conversation
Adds validation for MCP tool naming conventions as specified in SEP-986. Ensures Rust SDK enforces standardized tool name formats, provides clear errors for invalid names, and improves consistency across implementations. Signed-off-by: tanish111 <tanishdesai37@gmail.com>
Update doctest examples to use the correct module path rmcp::handler::server::tool_name_validation instead of rmcp::model::tool_name_validation. Signed-off-by: tanish111 <tanishdesai37@gmail.com>
Uh oh!
There was an error while loading. Please reload this page.
alexhancock
left a comment
There was a problem hiding this comment.
LGTM other than the one comment. I also prefer to remove most of the comments for code that is already clear.
Uh oh!
There was an error while loading. Please reload this page.
Prevent empty warnings array from triggering warning output. Signed-off-by: tanish111 <tanishdesai37@gmail.com>
Signed-off-by: tanish111 <tanishdesai37@gmail.com>
Signed-off-by: tanish111 <tanishdesai37@gmail.com>
| pub fn add_route(&mut self, item: ToolRoute<S>) { | ||
| self.map.insert(item.attr.name.clone(), item); | ||
| let new_name = &item.attr.name; | ||
| // Validate tool name according to SEP specification |
There was a problem hiding this comment.
I'd remove these comments as it already seems clear based on method name
Signed-off-by: tanish111 <tanishdesai37@gmail.com>
| } | ||
| /// Validates a tool name according to the SEP specification. | ||
| pub fn validate_tool_name(name: &str) -> ToolNameValidationResult { |
There was a problem hiding this comment.
these don't need to be pub right?
only validate_and_warn_tool_name
There was a problem hiding this comment.
@alexhancock I kept it pub because I wanted to keep it consistent with TS SDK
There was a problem hiding this comment.
Given that the tool name validation is only for the purpose of logging warnings right now, I think we should keep the other things private
| /// Result of tool name validation containing validation status and warnings. | ||
| #[derive(Debug, Clone, PartialEq, Eq)] | ||
| pub struct ToolNameValidationResult { |
There was a problem hiding this comment.
@alexhancock should I change this to private as well as It's not used anywhere outside this file?(and all its fields and functions)
There was a problem hiding this comment.
yes. only things which are used outside the module should be private IMO. easier to expose things later if we want vs take things private.
There was a problem hiding this comment.
yes. only things which are used outside the module should be private IMO. easier to expose things later if we want vs take things private.
you mean only things which are used outside the module should be public
There was a problem hiding this comment.
yes sorry typoed. it looks good to me now.
Made ToolNameValidationResult and its functions and fields private. Made validate_tool_name and issue_tool_name_warning private. Signed-off-by: tanish111 <tanishdesai37@gmail.com>
Uh oh!
There was an error while loading. Please reload this page.
…tprotocol#551) * feat: implement SEP-986 tool name validation and error reporting Adds validation for MCP tool naming conventions as specified in SEP-986. Ensures Rust SDK enforces standardized tool name formats, provides clear errors for invalid names, and improves consistency across implementations. Signed-off-by: tanish111 <tanishdesai37@gmail.com> * fix(doctests): correct import paths in tool_name_validation Update doctest examples to use the correct module path rmcp::handler::server::tool_name_validation instead of rmcp::model::tool_name_validation. Signed-off-by: tanish111 <tanishdesai37@gmail.com> * fix: only warn when warnings exist Prevent empty warnings array from triggering warning output. Signed-off-by: tanish111 <tanishdesai37@gmail.com> * fix: remove internal check Signed-off-by: tanish111 <tanishdesai37@gmail.com> * refactor: remove doc comments from validation functions Signed-off-by: tanish111 <tanishdesai37@gmail.com> * refactor: remove doc comments from add_route functions Signed-off-by: tanish111 <tanishdesai37@gmail.com> * refactor: make tool name validation helpers private Made ToolNameValidationResult and its functions and fields private. Made validate_tool_name and issue_tool_name_warning private. Signed-off-by: tanish111 <tanishdesai37@gmail.com> --------- Signed-off-by: tanish111 <tanishdesai37@gmail.com>
Motivation and Context
modelcontextprotocol/modelcontextprotocol/issues/986
How Has This Been Tested?
Manual testing in examples/servers: added name to existing #[tool] attributes in common/counter.rs with valid and invalid names. Rebuilding and running showed expected warnings. Unit tests (11) cover validation scenarios; integration tests are planned.
Breaking Changes
None, but console warnings may appear for SDK users who have names that do not satisfy the SEP.
Types of changes
Checklist
Additional context
All design decisions, names, and other aspects follow the TypeScript implementation for consistency across SDKs.
Related issue #518