Skip to content

fix: handle nil Params in ParseToolArguments#2172

Merged
lpcox merged 1 commit intomainfrom
fix/nil-params-parse-tool-arguments
Mar 19, 2026
Merged

fix: handle nil Params in ParseToolArguments#2172
lpcox merged 1 commit intomainfrom
fix/nil-params-parse-tool-arguments

Conversation

@lpcox
Copy link
Collaborator

@lpcox lpcox commented Mar 19, 2026

Problem

CallToolRequest is ServerRequest[*CallToolParamsRaw]Params is a pointer type that defaults to nil when not explicitly initialized. ParseToolArguments accessed req.Params.Arguments without checking for nil, causing a nil pointer dereference panic.

Fix

  • Added nil check for req.Params in ParseToolArguments() before accessing req.Params.Arguments
  • Fixed test cases to properly initialize Params via struct literals instead of assigning to a nil pointer field
  • Added a dedicated nil params returns empty map test case

Testing

All tests pass (make agent-finished ✅).

CallToolRequest.Params is *CallToolParamsRaw (a pointer type), so it can
be nil when a request is constructed without explicit initialization.
ParseToolArguments now checks req.Params != nil before accessing
req.Params.Arguments to prevent nil pointer dereference.

Also fixes test cases to properly initialize Params via struct literals
and adds a dedicated nil-params test case.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 19, 2026 17:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a nil-pointer panic in MCP tool argument parsing by safely handling CallToolRequest.Params being nil, and updates/adds tests to reflect correct request construction.

Changes:

  • Guarded ParseToolArguments() against req.Params == nil before accessing Arguments.
  • Updated tests to initialize CallToolRequest.Params via struct literals instead of mutating a nil pointer field.
  • Added a dedicated test case for “nil params returns empty map”.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
internal/mcp/tool_result.go Adds a nil check for req.Params in ParseToolArguments to prevent panics.
internal/mcp/tool_result_test.go Adjusts request construction in tests and adds coverage for nil Params.
Comments suppressed due to low confidence (1)

internal/mcp/tool_result.go:117

  • ParseToolArguments can still return a nil map when Arguments is present but set to JSON null (json.Unmarshal succeeds and leaves toolArgs nil). This is inconsistent with the “no arguments → empty map” behavior and with callers that may assume a non-nil args object; consider normalizing toolArgs to an empty map after unmarshal when it’s nil (and ideally add a test for arguments: null).
func ParseToolArguments(req *sdk.CallToolRequest) (map[string]interface{}, error) {
	var toolArgs map[string]interface{}
	if req.Params != nil && req.Params.Arguments != nil {
		if err := json.Unmarshal(req.Params.Arguments, &toolArgs); err != nil {
			return nil, fmt.Errorf("failed to parse arguments: %w", err)
		}
	} else {
		// No arguments provided, use empty map
		toolArgs = make(map[string]interface{})
	}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants