mcp-go
mcp-go copied to clipboard
Extract Params from CallToolRequest into a Named Struct
Currently, CallToolRequest uses an anonymous inline struct for the Params field, which makes it cumbersome to initialize in tests or client code. This is particularly painful when mocking or setting up test cases, as it requires verbose and brittle inline struct literals.
Example
In this test, the anonymous struct makes the code harder to read and reuse:
request := mcp.CallToolRequest{
Params: struct {
Name string `json:"name"`
Arguments map[string]interface{} `json:"arguments,omitempty"`
Meta *struct {
ProgressToken mcp.ProgressToken `json:"progressToken,omitempty"`
} `json:"_meta,omitempty"`
}{
Arguments: map[string]interface{}{
"all_namespaces": true,
},
},
}
This verbosity could be avoided if Params (and optionally _meta) were defined as named types.
Suggested Improvement
Refactor CallToolRequest to use a named ToolParameters struct:
type ToolParameters struct {
Name string `json:"name"`
Arguments map[string]interface{} `json:"arguments,omitempty"`
Meta *ToolParametersMeta `json:"_meta,omitempty"`
}
type ToolParametersMeta struct {
ProgressToken ProgressToken `json:"progressToken,omitempty"`
}
type CallToolRequest struct {
Request
Params ToolParameters `json:"params"`
}
This would greatly improve testability, readability, and ease of use.
This issue with anonymous inline structs isn't limited to CallToolRequest. It exists in many other types under types.go, including:
InitializeRequest, Request, JSONRPCError, CancelledNotification, ServerCapabilities, ProgressNotification, PaginatedRequest, ReadResourceRequest, SubscribeRequest, UnsubscribeRequest, ResourceUpdatedNotification, SetLevelRequest, LoggingMessageNotification, CreateMessageRequest, CompleteRequest.
Having inline anonymous structs in all these cases makes it unnecessarily verbose in client code. It also affects reusability.
Would be happy to help with a refactor PR if there's interest.
Looks like part of this issue i.e Meta anonymous struct was addressed by this PR. Thanks to @anuraaga.
Yeah I extracted a type there mostly to be able to implement the custom JSON marshaling. But I agree with this issue for all the other types too that generally avoiding anonymous structs across the board will make things easier to use. Admittedly a particular minor use case but it was very hard to write transport middleware. For anyone interested in Go trivia, the json tags are required for it to work 😂
// SendRequest implements transport.Interface.
func (c *contextTransport) SendRequest(ctx context.Context, request transport.JSONRPCRequest) (*transport.JSONRPCResponse, error) {
if params, ok := request.Params.(struct {
Name string `json:"name"`
Arguments map[string]any `json:"arguments,omitempty"`
Meta *mcp.Meta `json:"_meta,omitempty"`
}); ok {
meta := params.Meta
if meta == nil {
meta = &mcp.Meta{}
params.Meta = meta
}
if meta.AdditionalFields == nil {
meta.AdditionalFields = make(map[string]any)
}
otel.GetTextMapPropagator().Inject(ctx, additionalFieldsCarrier{meta.AdditionalFields})
request.Params = params
}
return c.delegate.SendRequest(ctx, request)
}
Some fixes were made here: https://github.com/mark3labs/mcp-go/pull/333
Ideally all anonymous structs must be changed.