Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the tool registration logic to centralize read-only mode checks. Instead of wrapping tool registration calls with explicit if !a.options.ReadOnly conditionals scattered throughout the code, the PR moves this logic into the addTool helper function, which uses annotation comparison to determine whether a tool should be registered.
Changes:
- Centralized read-only mode checking in the
addToolfunction by comparing annotations - Changed annotation variables from pointers to values for comparison
- Removed repetitive
if !a.options.ReadOnlyconditionals from tool registration code
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| func addTool[In, Out any](server *mcp.Server, name string, description string, annotations *mcp.ToolAnnotations, handler mcp.ToolHandlerFor[In, Out]) { | ||
| func addTool[In, Out any](a *App, server *mcp.Server, name string, description string, annotations mcp.ToolAnnotations, handler mcp.ToolHandlerFor[In, Out]) { | ||
| if a.options.ReadOnly && annotations != readOnlyAnnotation { |
There was a problem hiding this comment.
Struct comparison with pointer fields is fragile. While this works because package-level annotation variables are used consistently, it compares pointer addresses rather than values. A more robust approach would be to check the annotation fields directly (e.g., checking if ReadOnlyHint is true) or add a method to ToolAnnotations to determine if it's read-only. This would make the intent clearer and prevent issues if annotations are created differently in the future.
| if a.options.ReadOnly && annotations != readOnlyAnnotation { | |
| if a.options.ReadOnly && !annotations.ReadOnlyHint { |
2 participants
No description provided.