fix(mcp): strip null values from MCP tool call arguments by krissding · Pull Request #96755 · openclaw/openclaw

krissding · 2026-06-25T11:25:07Z

Summary

Some MCP servers (e.g. awslabs eks-mcp-server) distinguish between null and absent for optional parameters, causing tool calls that pass null for optional fields to fail
Fix: strip top-level null values from tool call arguments before sending to the MCP server, so { insight_id: null } becomes {} — matching the "not provided" semantics of JSON Schema optional parameters

Test plan

Verify MCP tool calls with null optional arguments succeed (same as when the field is absent)
Verify non-null arguments are unaffected
Verify nested null values are preserved (only top-level stripping)

🤖 Generated with Claude Code

Some MCP servers (e.g. awslabs eks-mcp-server) distinguish between null and absent for optional parameters. When the LLM passes null for an optional field, treat it as "not provided" by stripping top-level null values from arguments before calling the MCP server. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

clawsweeper · 2026-06-25T11:26:10Z

Codex review: needs real behavior proof before merge. Reviewed June 25, 2026, 7:37 AM ET / 11:37 UTC.

Summary
The PR adds a session MCP runtime helper that drops top-level null-valued tool arguments before calling the MCP SDK client.

PR surface: Source +10. Total +10 across 1 file.

Reproducibility: no. high-confidence live reproduction was run in this review. Source inspection and reporter evidence show a plausible null-to-empty-string path, but there is no real awslabs EKS MCP run or raw wire capture here.

Review metrics: 2 noteworthy metrics.

MCP argument contract: 1 dispatch boundary changed. The PR changes null-versus-absent semantics for all session MCP tool calls, which maintainers should settle before merge.
Open candidate fixes: 3 open. Multiple PRs target the same linked MCP bug, so maintainers should choose one canonical fix path.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #96716
Summary: This PR and sibling PRs are candidate fixes for the canonical MCP optional-null argument issue.

Members:

canonical: [Bug]: MCP tool call with an optional argument set to null fails via OpenClaw but succeeds via other MCP clients #96716 - The PR body uses closing syntax for the linked MCP null-argument bug and the issue contains the reproduction details.
same_root_cause: fix(mcp): strip null optional arguments before MCP tool calls #96718 - This sibling PR is an open candidate fix for the same linked issue but has unresolved proof and compatibility blockers.
same_root_cause: fix(agents): strip null-valued optional args from MCP tool calls #96729 - This sibling PR targets the same linked MCP null-argument bug but still requires maintainer review for broad null omission and real MCP proof.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

[P1] Add redacted after-fix MCP server output, logs, terminal output, or a recording showing the affected call succeeds.
Narrow the implementation so explicit nulls are preserved unless the tool schema proves omission is safe.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR body has unchecked test-plan items but no redacted terminal output, logs, recording, or linked artifact showing a real MCP call succeeds after the change. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

[P1] Merging as written can silently omit meaningful explicit JSON null arguments for every session MCP tool call.
[P1] The likely null-to-empty-string conversion happens before this patched boundary, so filtering only value !== null may not fix the reported empty-string failure path.
[P1] There are multiple open candidate PRs for the same linked issue, so maintainers need one canonical fix path rather than parallel broad-null patches.

Maintainer options:

Narrow the null omission rule (recommended)
Move the repair to validation/coercion or omit nulls only when the MCP tool schema proves the field is optional and null-as-absent is intended.
Define broad null omission as contract
Maintainers can intentionally decide that session MCP dispatch never sends top-level null values, but that compatibility-affecting contract should be explicit and tested first.
Pause for real server proof
Keep the candidate PRs paused until redacted real MCP output shows where the empty string appears and which layer fixes it.

Next step before merge

[P1] The remaining blocker is a maintainer-visible MCP argument contract decision plus contributor real-server proof, not a safe automated repair on this PR branch.

Security
Cleared: The diff adds no dependencies, scripts, workflow changes, lockfile changes, permissions, or secret-handling paths.

Review findings

[P1] Fix the pre-dispatch null coercion path — src/agents/agent-bundle-mcp-runtime.ts:912
[P1] Preserve schema-meaningful null MCP arguments — src/agents/agent-bundle-mcp-runtime.ts:912

Review details

Best possible solution:

Preserve explicit JSON nulls by default and fix validation/coercion, or omit null only for schema-proven optional fields with focused regression coverage and redacted real MCP proof.

Do we have a high-confidence way to reproduce the issue?

No high-confidence live reproduction was run in this review. Source inspection and reporter evidence show a plausible null-to-empty-string path, but there is no real awslabs EKS MCP run or raw wire capture here.

Is this the best way to solve the issue?

No. Unconditional top-level null stripping is too broad and likely too late for the reported anyOf [string, null] case because validation can coerce null to an empty string before this boundary.

Full review comments:

[P1] Fix the pre-dispatch null coercion path — src/agents/agent-bundle-mcp-runtime.ts:912
callTool receives arguments after prepareToolCall has already run validation. For the reported anyOf [{type:"string"},{type:"null"}] shape, current coercion can convert null to "" before this filter runs, so the reported empty-string failure can still be forwarded.
Confidence: 0.9
[P1] Preserve schema-meaningful null MCP arguments — src/agents/agent-bundle-mcp-runtime.ts:912
This drops every top-level null regardless of the MCP tool schema. The SDK accepts unknown argument values and existing schema tests allow required nullable properties, so required or meaningful explicit nulls would be silently omitted.
Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 4ecb45bf7729.

Label changes

Label justifications:

P2: This is a normal-priority MCP behavior bugfix PR with limited blast radius but real impact for affected tool calls.
merge-risk: 🚨 compatibility: The diff can silently omit explicit JSON null arguments for every MCP tool call, including tools where null is a meaningful value.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body has unchecked test-plan items but no redacted terminal output, logs, recording, or linked artifact showing a real MCP call succeeds after the change. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

PR surface:

Source +10. Total +10 across 1 file.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	11	1	+10
Tests	0	0	0	0
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	1	11	1	+10

What I checked:

PR diff: The changed call site sends stripNullValues(input) for every record-shaped MCP tool input before session.client.callTool(...). (src/agents/agent-bundle-mcp-runtime.ts:912, df236047039b)
Current validation order: Current main validates prepared tool arguments before execution, so the MCP runtime receives post-validation input rather than the model's raw argument object. (packages/agent-core/src/agent-loop.ts:872, 4ecb45bf7729)
Null-to-empty-string path: Plain JSON-schema coercion currently converts null to an empty string for string schemas, and union coercion returns the first branch that validates after coercion. (packages/llm-core/src/validation.ts:148, 4ecb45bf7729)
Schema-meaningful nulls: Existing MCP-adjacent schema tests accept required nullable values, showing explicit null can be valid data rather than always equivalent to omission. (src/agents/agent-bundle-mcp-runtime.test.ts:317, 4ecb45bf7729)
MCP SDK contract: The pinned MCP TypeScript SDK is v1.29.0, whose tools/call params define arguments as an optional record of unknown values and forward params through tools/call. (package.json:1965, 4ecb45bf7729)
Related work: GitHub search shows three open PRs targeting the same optional-null MCP issue, so this PR is one candidate fix rather than a superseded or implemented item.

Likely related people:

jalehman: Git blame and GitHub commit metadata tie the current MCP runtime forwarding path, bundle-MCP materialization path, and JSON-schema validation coercion path to the same current-main commit. (role: recent area contributor; confidence: medium; commits: 0cdb050bac19; files: src/agents/agent-bundle-mcp-runtime.ts, src/agents/agent-bundle-mcp-materialize.ts, packages/llm-core/src/validation.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

vincentkoc · 2026-06-27T15:39:49Z

Thanks for the fix. This is now covered by the landed #97212 / commit 361869e.

Evidence: shared issue(s): #96716.

Closing #96755 as a duplicate.

openclaw-barnacle Bot added agents Agent runtime and tooling size: XS triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 25, 2026

vincentkoc added duplicate This issue or pull request already exists close:duplicate Closed as duplicate dedupe:child Duplicate issue/PR child in dedupe cluster labels Jun 27, 2026

vincentkoc closed this Jun 27, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(mcp): strip null values from MCP tool call arguments#96755

fix(mcp): strip null values from MCP tool call arguments#96755
krissding wants to merge 1 commit into
openclaw:mainfrom
krissding:fix/issue-96716-Bug-MCP-tool-call-with-an-opti

krissding commented Jun 25, 2026

Uh oh!

clawsweeper Bot commented Jun 25, 2026 •

edited

Loading

Uh oh!

vincentkoc commented Jun 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

krissding commented Jun 25, 2026

Summary

Test plan

Uh oh!

clawsweeper Bot commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vincentkoc commented Jun 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Jun 25, 2026 •

edited

Loading