From 2ba06282621c1898e6685e11c64ea9b3e338464a Mon Sep 17 00:00:00 2001 From: advancedresearcharray Date: Tue, 9 Jun 2026 17:01:50 +0000 Subject: [PATCH] fix: accept omitted tool arguments for zero-param tools like get_me When MCP clients omit the arguments field on tools/call, the typed tool wrapper received nil RawMessage and failed JSON unmarshaling before the handler ran. Coerce nil or empty arguments to {} so zero-parameter tools such as get_me work as documented. Closes #2587 --- pkg/github/context_tools_test.go | 42 ++++++++++++ pkg/github/helper_test.go | 10 +++ pkg/inventory/server_tool.go | 26 ++++++-- pkg/inventory/server_tool_test.go | 107 ++++++++++++++++++++++++++++++ 4 files changed, 178 insertions(+), 7 deletions(-) diff --git a/pkg/github/context_tools_test.go b/pkg/github/context_tools_test.go index 65c4741a4b..ce6b2baf47 100644 --- a/pkg/github/context_tools_test.go +++ b/pkg/github/context_tools_test.go @@ -139,6 +139,48 @@ func Test_GetMe(t *testing.T) { } } +func Test_GetMe_OmittedArguments(t *testing.T) { + t.Parallel() + + serverTool := GetMe(translations.NullTranslationHelper) + + mockUser := &github.User{ + Login: github.Ptr("testuser"), + Name: github.Ptr("Test User"), + HTMLURL: github.Ptr("https://github.com/testuser"), + CreatedAt: &github.Timestamp{Time: time.Now()}, + } + mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetUser: mockResponse(t, http.StatusOK, mockUser), + }) + deps := BaseDeps{Client: mustNewGHClient(t, mockedClient), Obsv: stubExporters()} + handler := serverTool.Handler(deps) + + tests := []struct { + name string + arguments json.RawMessage + }{ + {name: "nil arguments", arguments: nil}, + {name: "empty byte slice", arguments: json.RawMessage{}}, + {name: "explicit empty object", arguments: json.RawMessage(`{}`)}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + request := createMCPRequestWithRawArguments(tc.arguments) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + + textContent := getTextResult(t, result) + var returnedUser MinimalUser + err = json.Unmarshal([]byte(textContent.Text), &returnedUser) + require.NoError(t, err) + assert.Equal(t, "testuser", returnedUser.Login) + }) + } +} + func Test_GetMe_IFC_FeatureFlag(t *testing.T) { t.Parallel() diff --git a/pkg/github/helper_test.go b/pkg/github/helper_test.go index f6737c5df0..5a77de0758 100644 --- a/pkg/github/helper_test.go +++ b/pkg/github/helper_test.go @@ -334,6 +334,16 @@ func createMCPRequest(args any) mcp.CallToolRequest { } } +// createMCPRequestWithRawArguments creates a CallToolRequest with the given raw JSON arguments. +// Use nil or an empty slice to simulate clients that omit the arguments field. +func createMCPRequestWithRawArguments(args json.RawMessage) mcp.CallToolRequest { + return mcp.CallToolRequest{ + Params: &mcp.CallToolParamsRaw{ + Arguments: args, + }, + } +} + // Well-known MCP client names used in tests. const ( ClientNameVSCodeInsiders = "Visual Studio Code - Insiders" diff --git a/pkg/inventory/server_tool.go b/pkg/inventory/server_tool.go index d25458253f..ef76c1493f 100644 --- a/pkg/inventory/server_tool.go +++ b/pkg/inventory/server_tool.go @@ -1,6 +1,7 @@ package inventory import ( + "bytes" "context" "encoding/json" "fmt" @@ -210,13 +211,15 @@ func NewServerToolWithContextHandler[In any, Out any](tool mcp.Tool, toolset Too HandlerFunc: func(_ any) mcp.ToolHandler { return func(ctx context.Context, req *mcp.CallToolRequest) (*mcp.CallToolResult, error) { var arguments In - if err := json.Unmarshal(req.Params.Arguments, &arguments); err != nil { - return &mcp.CallToolResult{ - Content: []mcp.Content{ - &mcp.TextContent{Text: fmt.Sprintf("invalid arguments: %s", err)}, - }, - IsError: true, - }, nil + args := req.Params.Arguments + if len(args) == 0 { + args = json.RawMessage(`{}`) + } + if bytes.Equal(bytes.TrimSpace(args), []byte("null")) { + return invalidArgumentsResult("arguments must be a JSON object"), nil + } + if err := json.Unmarshal(args, &arguments); err != nil { + return invalidArgumentsResult(err.Error()), nil } resp, _, err := handler(ctx, req, arguments) return resp, err @@ -225,6 +228,15 @@ func NewServerToolWithContextHandler[In any, Out any](tool mcp.Tool, toolset Too } } +func invalidArgumentsResult(message string) *mcp.CallToolResult { + return &mcp.CallToolResult{ + Content: []mcp.Content{ + &mcp.TextContent{Text: fmt.Sprintf("invalid arguments: %s", message)}, + }, + IsError: true, + } +} + // NewServerTool creates a ServerTool with a raw handler that receives deps via context. // This is the preferred constructor for tools that use mcp.ToolHandler directly because // it doesn't create closures at registration time, which is critical for performance in diff --git a/pkg/inventory/server_tool_test.go b/pkg/inventory/server_tool_test.go index c6d2a6fdd8..5b5aecfd64 100644 --- a/pkg/inventory/server_tool_test.go +++ b/pkg/inventory/server_tool_test.go @@ -81,6 +81,113 @@ func TestNewServerToolWithContextHandler_ValidArguments_Succeeds(t *testing.T) { assert.Equal(t, "success: octocat/hello-world", textContent.Text) } +func TestNewServerToolWithContextHandler_ArgumentObjects(t *testing.T) { + type emptyArgs struct{} + + handlerCalled := false + tool := NewServerToolWithContextHandler( + mcp.Tool{Name: "zero_arg_tool"}, + testToolsetMetadata("test"), + func(_ context.Context, _ *mcp.CallToolRequest, _ emptyArgs) (*mcp.CallToolResult, any, error) { + handlerCalled = true + return &mcp.CallToolResult{ + Content: []mcp.Content{&mcp.TextContent{Text: "ok"}}, + }, nil, nil + }, + ) + handler := tool.HandlerFunc(nil) + + tests := []struct { + name string + arguments json.RawMessage + wantError bool + wantMessage string + wantHandler bool + }{ + { + name: "nil arguments", + arguments: nil, + wantHandler: true, + }, + { + name: "zero-length arguments", + arguments: json.RawMessage{}, + wantHandler: true, + }, + { + name: "empty object", + arguments: json.RawMessage(`{}`), + wantHandler: true, + }, + { + name: "null", + arguments: json.RawMessage(`null`), + wantError: true, + wantMessage: "arguments must be a JSON object", + }, + { + name: "malformed JSON", + arguments: json.RawMessage(`{not valid json`), + wantError: true, + wantMessage: "invalid character", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + handlerCalled = false + result, err := handler(context.Background(), &mcp.CallToolRequest{ + Params: &mcp.CallToolParamsRaw{ + Name: "zero_arg_tool", + Arguments: tt.arguments, + }, + }) + + require.NoError(t, err) + require.NotNil(t, result) + assert.Equal(t, tt.wantError, result.IsError) + assert.Equal(t, tt.wantHandler, handlerCalled) + if tt.wantMessage != "" { + textContent, ok := result.Content[0].(*mcp.TextContent) + require.True(t, ok) + assert.Contains(t, textContent.Text, tt.wantMessage) + } + }) + } +} + +func TestNewServerToolWithContextHandler_OmittedArgumentsReachRequiredParameterValidation(t *testing.T) { + tool := NewServerToolWithContextHandler( + mcp.Tool{Name: "parameterized_tool"}, + testToolsetMetadata("test"), + func(_ context.Context, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + if _, ok := args["owner"]; !ok { + return &mcp.CallToolResult{ + Content: []mcp.Content{&mcp.TextContent{Text: "missing required parameter: owner"}}, + IsError: true, + }, nil, nil + } + return &mcp.CallToolResult{ + Content: []mcp.Content{&mcp.TextContent{Text: "ok"}}, + }, nil, nil + }, + ) + + result, err := tool.HandlerFunc(nil)(context.Background(), &mcp.CallToolRequest{ + Params: &mcp.CallToolParamsRaw{ + Name: "parameterized_tool", + Arguments: nil, + }, + }) + + require.NoError(t, err) + require.NotNil(t, result) + require.True(t, result.IsError) + textContent, ok := result.Content[0].(*mcp.TextContent) + require.True(t, ok) + assert.Contains(t, textContent.Text, "missing required parameter: owner") +} + func TestServerToolRegisterFuncAppliesMiddleware(t *testing.T) { tool := NewServerTool( mcp.Tool{