Skip to content

fix: Strip nil arguments before forwarding MCP tool calls to upstream#176

Merged
mrsabath merged 1 commit intokagenti:mainfrom
mrsabath:fix/github-tool-null-params
Mar 16, 2026
Merged

fix: Strip nil arguments before forwarding MCP tool calls to upstream#176
mrsabath merged 1 commit intokagenti:mainfrom
mrsabath:fix/github-tool-null-params

Conversation

@mrsabath
Copy link
Copy Markdown
Contributor

Summary

  • Strip nil-valued entries from tool call arguments before forwarding to the upstream GitHub MCP server
  • OpenAI function calling sends explicit JSON null for optional parameters, but the upstream MCP server rejects nil where it expects a typed value

Problem

Every list_issues call fails with errors like:

parameter state is not of type string, is <nil>

The LLM retries with progressively more fields filled in, but each attempt fails on the next null field until retries are exhausted.

Fix

Added stripNilArguments() in CallTool() that removes nil-valued entries from the arguments map before forwarding to the upstream MCP server.

Test plan

  • Built and deployed locally to Kind cluster
  • Verified end-to-end: Alice token → inbound validation → token exchange → MCP tool call → GitHub issues returned successfully

Fixes #175

OpenAI function calling sends explicit JSON null for optional parameters.
The upstream GitHub MCP server rejects nil where it expects a typed value,
causing every list_issues call to fail with errors like
"parameter state is not of type string, is <nil>".

Strip nil-valued entries from tool call arguments before forwarding
to the upstream MCP server.

Fixes kagenti#175

Signed-off-by: Mariusz Sabath <mrsabath@gmail.com>
Copy link
Copy Markdown
Contributor

@pdettori pdettori left a comment

Choose a reason for hiding this comment

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

Clean, focused fix. stripNilArguments correctly removes JSON null entries (which become Go nil after unmarshalling) before forwarding to the upstream GitHub MCP server. The in-place map mutation via type assertion is idiomatic Go and correct here. The doc comment clearly explains the rationale.

Areas reviewed: Go
Commits: 1 commit, signed-off ✅
CI status: All 9 checks passing ✅


Suggestion (upstream.gostripNilArguments): Consider adding a small unit test for this function — it covers a non-obvious edge case (JSON null → Go nil) and would guard against regressions.

Nit (upstream.gostripNilArguments): The function only strips top-level nils. If the GitHub MCP server ever passes nested map arguments, nested nils would survive. Likely not an issue today, but worth a note in the comment if intentional.

@mrsabath mrsabath merged commit bf62346 into kagenti:main Mar 16, 2026
9 checks passed
@mrsabath mrsabath deleted the fix/github-tool-null-params branch March 16, 2026 16:37
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.

bug: GitHub tool MCP server rejects null for optional parameters

3 participants