From facec97821badfef13125303bfd5a60976390f61 Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Mon, 2 Jun 2025 14:45:53 +0000 Subject: [PATCH] internal/mcp: add a test for complete mapping of CallToolParams fields Add a test to verify that the toWireParams helper maps all fields of CallToolParams. Change-Id: I81c28791734d60c793c76809c6b93d27e78c3c36 Reviewed-on: https://go-review.googlesource.com/c/tools/+/677975 LUCI-TryBot-Result: Go LUCI Reviewed-by: Jonathan Amsterdam --- mcp/client.go | 17 +++++++++++++---- mcp/client_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/mcp/client.go b/mcp/client.go index c866b8c..0b1e583 100644 --- a/mcp/client.go +++ b/mcp/client.go @@ -265,17 +265,26 @@ func (cs *ClientSession) CallTool(ctx context.Context, params *CallToolParams[js // CallTool is a helper to call a tool with any argument type. It returns an // error if params.Arguments fails to marshal to JSON. func CallTool[TArgs any](ctx context.Context, cs *ClientSession, params *CallToolParams[TArgs]) (*CallToolResult, error) { + wireParams, err := toWireParams(params) + if err != nil { + return nil, err + } + return cs.CallTool(ctx, wireParams) +} + +func toWireParams[TArgs any](params *CallToolParams[TArgs]) (*CallToolParams[json.RawMessage], error) { data, err := json.Marshal(params.Arguments) if err != nil { return nil, fmt.Errorf("failed to marshal arguments: %v", err) } - // TODO(rfindley): write a test that guarantees this copying is total. - wireParams := &CallToolParams[json.RawMessage]{ + // The field mapping here must be kept up to date with the CallToolParams. + // This is partially enforced by TestToWireParams, which verifies that all + // comparable fields are mapped. + return &CallToolParams[json.RawMessage]{ Meta: params.Meta, Name: params.Name, Arguments: data, - } - return cs.CallTool(ctx, wireParams) + }, nil } func (cs *ClientSession) SetLevel(ctx context.Context, params *SetLevelParams) error { diff --git a/mcp/client_test.go b/mcp/client_test.go index 9d24a92..218fafa 100644 --- a/mcp/client_test.go +++ b/mcp/client_test.go @@ -7,6 +7,7 @@ package mcp import ( "context" "fmt" + "reflect" "testing" "github.com/google/go-cmp/cmp" @@ -199,3 +200,28 @@ func TestClientPaginateVariousPageSizes(t *testing.T) { }) } } + +func TestToWireParams(t *testing.T) { + // This test verifies that toWireParams maps all fields. + // The Meta and Arguments fields are not comparable, so can't be checked by + // this simple test. However, this test will fail if new fields are added, + // and not handled by toWireParams. + params := &CallToolParams[struct{}]{ + Name: "tool", + } + wireParams, err := toWireParams(params) + if err != nil { + t.Fatal(err) + } + v := reflect.ValueOf(wireParams).Elem() + for i := range v.Type().NumField() { + f := v.Type().Field(i) + if f.Name == "Meta" || f.Name == "Arguments" { + continue // not comparable + } + fv := v.Field(i) + if fv.Interface() == reflect.Zero(f.Type).Interface() { + t.Fatalf("toWireParams: unmapped field %q", f.Name) + } + } +}