-
Notifications
You must be signed in to change notification settings - Fork 280
Description
This issue summarizes some internal feedback we received about tool binding APIs. As a result of this feedback (and since handler APIs is changing in v0.3.0 anyway due to #243), we've been evaluating making some or all of the changes described below this week.
The feedback was this:
- There must be a way to get a
ToolHandlerfrom aToolHandlerFor[In, Out]. - Inside of a tool handler, there should be a way to access the non-generic
CallToolParams. - For similar reasons, there should be a way to access the non-parameterized parts of
ServerRequest.
In essence, the feedback was that generics make it hard to implement aspect-oriented handling of requests, at various stages in the handling pass. Also, generics can be hard to parse visually. I think this feedback is probably right, though we may not act on all suggestions.
Notably, this is more similar to the original design, where typed tool handlers simply accepted an input struct, but we evolved toward *CallToolParamsFor[T] and then *ServerRequest[*CallToolParamsFor[T]] in #243. The feedback was essentially that this evolution toward generic structs is problematic.
Here are the changes that we're considering:
- Add a function to get the *Tool and ToolHandler that are internally created by
mcp.AddTool[In, Out]. - Move the
InandOuttypes to function parameters onToolHandlerFor[In, Out], rather than parameterizingCallToolParams. - Move ServerRequest into an embedded field of feature-specific request types. (This is incidentally similar to mcp-go, which would be a nice alignment).
type CallToolRequest struct {
ServerRequest
Params *CallToolParams
}
type ToolHandler func(context.Context, *CallToolRequest) (*CallToolResult, error)
type ToolHandlerFor[In, Out any] func(context.Context, *CallToolRequest, In) (*CallToolResult, Out, error)
func ToolFor[In, Out any](*Tool, ToolHandlerFor[In, Out]) (*Tool, ToolHandler) // this name could be improved
// The common usage is s.AddTool(ToolFor(tool, handler))
func (*Server) AddTool(*Tool, ToolHandler)We like this, but still see a few problems / unanswered questions
- What is the type of CallToolParams.Arguments?
any,map[string]any, andjson.RawMessageall have problems, if this type is shared between client and server.CallToolParamsForsolved these problems. - The name
ToolForis not great. - Do we still want the
mcp.AddTool(s, tool, handler)helper, if it is just a wrapper arounds.AddTool(ToolFor(tool, handler))?Server.AddToolis more discoverable in documentation.
Any feedback on this change is welcome.