Skip to content

Commit 8dd872a

Browse files
authored
fix(mcp): tool call without arguments does not panic (#849) (#859)
Handle the case where an MCP client omits the "arguments" field from a tools/call JSON-RPC request. The MCP spec defines arguments as optional (arguments?: { [key: string]: unknown }), so clients that omit it are spec-compliant. This is an edge case since most mainstream clients always send "arguments": {}, but custom or minimal clients may not. The fix: - GoSdkToolCallParamsToToolCallRequest now skips json.Unmarshal when Arguments is nil, returning a valid ToolCallRequest instead of an error. - toolCallLoggingMiddleware now checks the error instead of discarding it, preventing a nil pointer dereference on the log line. Also adds test.McpRawPost, a reusable helper for sending raw JSON-RPC requests to MCP HTTP endpoints, bypassing the go-sdk client normalization. Fixes #849 Signed-off-by: Marc Nuri <marc@marcnuri.com>
1 parent c8c94dd commit 8dd872a

4 files changed

Lines changed: 58 additions & 4 deletions

File tree

internal/test/mcp.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@ package test
33
import (
44
"context"
55
"encoding/json"
6+
"fmt"
67
"net/http"
78
"net/http/httptest"
9+
"strings"
810
"sync"
911
"testing"
1012
"time"
@@ -250,6 +252,36 @@ func (m *McpClient) CallTool(name string, args map[string]any) (*mcp.CallToolRes
250252
})
251253
}
252254

255+
// CallToolRaw sends a raw JSON-RPC tools/call request bypassing the go-sdk client.
256+
// This allows sending requests exactly as a non-go-sdk MCP client would, without
257+
// the go-sdk's automatic normalization (e.g., the go-sdk always adds "arguments": {}
258+
// even when nil).
259+
// The jsonParams is the raw JSON for the "params" field of the JSON-RPC request.
260+
func (m *McpClient) CallToolRaw(t *testing.T, jsonParams string) *http.Response {
261+
t.Helper()
262+
body := fmt.Sprintf(`{"jsonrpc":"2.0","id":99,"method":"tools/call","params":%s}`, jsonParams)
263+
return McpRawPost(t, m.testServer.URL+"/mcp", m.Session.ID(), body)
264+
}
265+
266+
// McpRawPost sends a raw JSON-RPC request to an MCP HTTP endpoint.
267+
// This is useful for testing MCP protocol edge cases that can't be reproduced through
268+
// the go-sdk client due to its automatic normalization (e.g., always adding "arguments": {},
269+
// always setting clientInfo).
270+
// The jsonBody should be a complete JSON-RPC message.
271+
func McpRawPost(t *testing.T, endpoint, sessionID, jsonBody string) *http.Response {
272+
t.Helper()
273+
req, err := http.NewRequest(http.MethodPost, endpoint, strings.NewReader(jsonBody))
274+
require.NoError(t, err)
275+
req.Header.Set("Content-Type", "application/json")
276+
req.Header.Set("Accept", "application/json, text/event-stream")
277+
if sessionID != "" {
278+
req.Header.Set("Mcp-Session-Id", sessionID)
279+
}
280+
resp, err := http.DefaultClient.Do(req)
281+
require.NoError(t, err)
282+
return resp
283+
}
284+
253285
// ListTools helper function to list available tools
254286
func (m *McpClient) ListTools() (*mcp.ListToolsResult, error) {
255287
return m.Session.ListTools(m.ctx, &mcp.ListToolsParams{})

pkg/mcp/gosdk.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,10 @@ func GoSdkToolCallRequestToToolCallRequest(request *mcp.CallToolRequest) (*ToolC
8181

8282
func GoSdkToolCallParamsToToolCallRequest(toolCallParams *mcp.CallToolParamsRaw) (*ToolCallRequest, error) {
8383
var arguments map[string]any
84-
if err := json.Unmarshal(toolCallParams.Arguments, &arguments); err != nil {
85-
return nil, fmt.Errorf("failed to unmarshal tool call arguments: %w", err)
84+
if len(toolCallParams.Arguments) > 0 {
85+
if err := json.Unmarshal(toolCallParams.Arguments, &arguments); err != nil {
86+
return nil, fmt.Errorf("failed to unmarshal tool call arguments: %w", err)
87+
}
8688
}
8789
return &ToolCallRequest{
8890
Name: toolCallParams.Name,

pkg/mcp/middleware.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,10 @@ func toolCallLoggingMiddleware(next mcp.MethodHandler) mcp.MethodHandler {
7070
return func(ctx context.Context, method string, req mcp.Request) (mcp.Result, error) {
7171
switch params := req.GetParams().(type) {
7272
case *mcp.CallToolParamsRaw:
73-
toolCallRequest, _ := GoSdkToolCallParamsToToolCallRequest(params)
74-
klog.V(5).Infof("mcp tool call: %s(%v)", toolCallRequest.Name, toolCallRequest.GetArguments())
73+
toolCallRequest, err := GoSdkToolCallParamsToToolCallRequest(params)
74+
if err == nil {
75+
klog.V(5).Infof("mcp tool call: %s(%v)", toolCallRequest.Name, toolCallRequest.GetArguments())
76+
}
7577
if req.GetExtra() != nil && req.GetExtra().Header != nil {
7678
buffer := bytes.NewBuffer(make([]byte, 0))
7779
if err := req.GetExtra().Header.WriteSubset(buffer, map[string]bool{"Authorization": true, "authorization": true}); err == nil {

pkg/mcp/namespaces_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,24 @@ func (s *NamespacesSuite) TestNamespacesList() {
4646
})
4747
}
4848

49+
// TestNamespacesListWithoutArguments verifies that namespaces_list can be called when the
50+
// MCP client omits the "arguments" field from the tools/call JSON-RPC request.
51+
// The MCP spec defines arguments as optional (arguments?: { [key: string]: unknown }),
52+
// so spec-compliant clients may omit it entirely for tools with no required parameters.
53+
// This is an edge case because most mainstream MCP clients (Claude Desktop, Cursor, etc.)
54+
// always send "arguments": {}, but custom or minimal clients may not.
55+
// https://github.com/containers/kubernetes-mcp-server/issues/849
56+
func (s *NamespacesSuite) TestNamespacesListWithoutArguments() {
57+
s.InitMcpClient()
58+
s.Run("namespaces_list without arguments does not panic", func() {
59+
// Send a raw JSON-RPC request without the "arguments" field, bypassing
60+
// the go-sdk client which always normalizes nil arguments to {}.
61+
resp := s.CallToolRaw(s.T(), `{"name":"namespaces_list"}`)
62+
defer func() { _ = resp.Body.Close() }()
63+
s.Equal(200, resp.StatusCode)
64+
})
65+
}
66+
4967
func (s *NamespacesSuite) TestNamespacesListDenied() {
5068
s.Require().NoError(toml.Unmarshal([]byte(`
5169
denied_resources = [ { version = "v1", kind = "Namespace" } ]

0 commit comments

Comments
 (0)