fix(mcp): strip null optional arguments before MCP tool calls by zw-xysk · Pull Request #96718 · openclaw/openclaw

zw-xysk · 2026-06-25T09:00:36Z

What Problem This Solves

MCP tool calls with optional arguments set to JSON null fail via OpenClaw but succeed via other MCP clients. The MCP server distinguishes null from absent/undefined, so sending {"insight_id": null} triggers a different code path than omitting insight_id entirely.

Evidence

Fix

src/agents/agent-bundle-mcp-materialize.ts: strip null values from tool call input before passing to callTool().

Terminal output

========================================
  PR #96718 — MCP Null Argument Strip
========================================
  Original: {"category":null,"cluster_name":"testenv","insight_id":null}
  Cleaned:  {"cluster_name":"testenv"}
  insight_id present: false
  category present:   false
  Status: PASS
========================================

What was not tested

End-to-end with a real MCP server. The fix is a targeted null-strip at the boundary between OpenClaw's tool call dispatch and the MCP runtime.

Changes

1 file, +9/-1. Lockfile unchanged.

MCP tool calls with optional arguments set to JSON null are transmitted differently than absent/undefined arguments by some MCP runtimes, causing servers to reject the call. Strip null values before passing input to callTool so null optional args behave the same as omitted ones. Fixes openclaw#96716 Co-Authored-By: Claude (via Anthropic API) <[email protected]>

zw-xysk · 2026-06-25T09:00:38Z

@clawsweeper re-review

clawsweeper · 2026-06-25T09:00:42Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

clawsweeper · 2026-06-25T09:09:42Z

Codex review: needs real behavior proof before merge. Reviewed June 26, 2026, 9:59 AM ET / 13:59 UTC.

Summary
The branch adds schema-aware filtering of top-level null MCP tool arguments before materialized bundle-MCP callTool, adds materialization tests, and makes a small test config-path cleanup.

PR surface: Source +69, Tests +78. Total +147 across 3 files.

Reproducibility: no. high-confidence live reproduction was run in this review. Source inspection shows current validation can convert null to "" before MCP execution, and the reporter supplied release logs for the affected path.

Review metrics: 2 noteworthy metrics.

MCP argument contract: 1 dispatch boundary changed. The PR changes null-versus-absent semantics for every materialized bundle-MCP tool call.
Canonical fix candidates: 3 open PRs. Multiple open branches target the same MCP optional-null bug, so maintainers need one canonical landing path.

Stored data model
Persistent data-model change detected: serialized state: src/agents/agent-bundle-mcp-tools.materialize.test.ts, serialized state: src/agents/session-tool-result-guard.tool-result-persist-hook.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #96716
Summary: This PR is one candidate fix for the canonical MCP optional-null argument bug; two sibling PRs target the same root cause but remain open and review-blocked.

Members:

canonical: [Bug]: MCP tool call with an optional argument set to null fails via OpenClaw but succeeds via other MCP clients #96716 - The issue contains the reported release version, MCP server details, reproduction steps, expected behavior, and logs for optional-null MCP arguments.
same_root_cause: fix(agents): strip null-valued optional args from MCP tool calls #96729 - This open sibling PR targets the same canonical issue and changes both llm-core validation and bundle-MCP null argument handling, but it still has proof and validation-order review blockers.
same_root_cause: fix(mcp): strip null values from MCP tool call arguments #96755 - This open sibling PR uses closing syntax for the same canonical issue and changes MCP null argument handling, but it is broader and still review-blocked.

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 stronger 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] Move or add the fix at the validation/pre-validation path so nullable anyOf nulls cannot become empty strings before MCP dispatch.
Use normalized tool schemas for nullability decisions, including $ref and OpenAPI nullable cases.
[P1] Add redacted configured OpenClaw MCP output, logs, terminal output, or a recording showing the affected call succeeds.

Proof guidance:

[P1] Needs stronger real behavior proof before merge: The PR body only shows synthetic terminal transformation output and explicitly says no end-to-end real MCP server test was run; it needs redacted real MCP output, logs, terminal output, or a recording before merge. 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] The execute-time filter can miss the reported case because validation can already convert a nullable anyOf null to an empty string before execute runs.
[P1] The nullability helper reads the raw MCP inputSchema; schemas using local $ref or other normalization-only shapes can have schema-meaningful nulls dropped.
[P1] The PR body proof is synthetic terminal output and explicitly says no end-to-end real MCP server test was run.
[P1] Three open PRs target the same canonical MCP optional-null issue, so maintainers need one landing path before retiring siblings.

Maintainer options:

Fix Validation Order Before Merge (recommended)
Move MCP null normalization to a pre-validation boundary or fix union validation so reported nulls cannot become empty strings before callTool, and use normalized schemas for nullability.
Narrow To Nullable Preservation
If maintainers only want the reported anyOf [string, null] repair, remove broad null omission and keep the change focused on preserving schema-nullable nulls.
Pause For Canonical Path
Keep this PR paused while maintainers choose between the open candidate PRs for the canonical MCP optional-null issue.

Next step before merge

[P1] Manual review is needed because the PR has insufficient contributor real-behavior proof, compatibility-sensitive argument semantics, and sibling candidate PRs for the same canonical issue.

Security
Cleared: The diff changes TypeScript source and tests only, with no dependency, lockfile, workflow, permission, download, publishing, or secret-handling changes.

Review findings

[P1] Fix null coercion before MCP execution — src/agents/agent-bundle-mcp-materialize.ts:471
[P2] Use normalized schemas for nullability checks — src/agents/agent-bundle-mcp-materialize.ts:471

Review details

Best possible solution:

Land one canonical fix that preserves schema-declared nullable nulls, performs any null-as-absent cleanup before validation with normalized schemas, and includes redacted configured OpenClaw 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 shows current validation can convert null to "" before MCP execution, and the reporter supplied release logs for the affected path.

Is this the best way to solve the issue?

No. The branch is a plausible partial fix, but it filters after validation and uses raw schema data; the safer fix is pre-validation or validation-layer normalization with normalized-schema nullability and real MCP proof.

Full review comments:

[P1] Fix null coercion before MCP execution — src/agents/agent-bundle-mcp-materialize.ts:471
In real agent runs, validateToolArguments runs before this execute callback. For the reported anyOf [{ type: "string" }, { type: "null" }] shape, current validation can coerce null to "" before this filter sees the input, so the failing MCP call can still be forwarded as an empty string.
Confidence: 0.91
[P2] Use normalized schemas for nullability checks — src/agents/agent-bundle-mcp-materialize.ts:471
This checks the raw MCP inputSchema, while the model-facing tool schema is normalized elsewhere to resolve $ref and OpenAPI nullable forms. A nullable field hidden behind a local $ref can be treated as non-nullable here and have a meaningful explicit null silently omitted.
Confidence: 0.82

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 1cd6f81a46ae.

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 PR changes MCP tool argument semantics by omitting top-level null values based on schema classification before dispatch.
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 stronger real behavior proof before merge: The PR body only shows synthetic terminal transformation output and explicitly says no end-to-end real MCP server test was run; it needs redacted real MCP output, logs, terminal output, or a recording before merge. 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 +69, Tests +78. Total +147 across 3 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	71	2	+69
Tests	2	88	10	+78
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	3	159	12	+147

What I checked:

Repository policy read: Root AGENTS.md and scoped src/agents/AGENTS.md were read fully; the review applied the proof-gate, dependency-contract, and compatibility-sensitive MCP/runtime guidance. (AGENTS.md:1, 1cd6f81a46ae)
PR filters inside execute: At PR head, null stripping happens inside the materialized tool execute callback immediately before runtime.callTool, so it sees post-validation input in real agent runs. (src/agents/agent-bundle-mcp-materialize.ts:471, 49fe87dda602)
Real caller validates first: Current main prepares and validates tool arguments before calling tool.execute, then passes validatedArgs into the changed execute callback. (packages/agent-core/src/agent-loop.ts:872, 1cd6f81a46ae)
Current validation can erase reported null: Current main coerces null to an empty string for string schemas, and union coercion returns the first branch that validates after coercion, matching the reported anyOf [{string},{null}] failure shape. (packages/llm-core/src/validation.ts:148, 1cd6f81a46ae)
PR test bypasses real validation order: The new MCP materialization test calls runtime.tools[0].execute directly with raw nulls, so it does not prove the agent-loop validation path that can convert null before execute. (src/agents/agent-bundle-mcp-tools.materialize.test.ts:568, 49fe87dda602)
MCP SDK argument contract: The pinned MCP TypeScript SDK v1.29.0 defines tools/call arguments as an optional record of unknown values and Client.callTool forwards supplied params through tools/call, so OpenClaw owns this normalization decision. (package.json:1965, 1cd6f81a46ae)

Likely related people:

Ayaan Zaidi: git blame attributes the current bundle-MCP materialization boundary, MCP runtime callTool forwarding, and llm-core union coercion code in this checkout to the same commit. (role: current-main area contributor; confidence: medium; commits: 8bc069f76f62; files: src/agents/agent-bundle-mcp-materialize.ts, src/agents/agent-bundle-mcp-runtime.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.

zw-xysk · 2026-06-25T11:08:56Z

@clawsweeper re-review

clawsweeper · 2026-06-25T11:08:58Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Improve the MCP null-argument fix to be schema-aware: - Only strip null values for fields NOT declared as nullable in the tool's input schema (handles type:null, type:[...,null], anyOf/oneOf with null variant, nullable:true) - Preserves null for schema-meaningful nulls (anyOf [string, null]) This approach is based on competitor PR openclaw#96729 (maweibin) which correctly handles the distinction between nullable and non-nullable fields at the MCP boundary. Fixes openclaw#96716 Signed-off-by: 赵旺0668001248 <[email protected]>

zw-xysk · 2026-06-26T13:50:53Z

@clawsweeper re-review

clawsweeper · 2026-06-26T13:50:56Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

openclaw-barnacle Bot added agents Agent runtime and tooling size: XS labels Jun 25, 2026

clawsweeper Bot mentioned this pull request Jun 25, 2026

[Bug]: MCP tool call with an optional argument set to null fails via OpenClaw but succeeds via other MCP clients #96716

Closed

fix(test): resolve string|undefined type error in writeFileSync call

5f6b53b

chore: trigger CI for updated PR body proof

c63de8c

clawsweeper Bot mentioned this pull request Jun 25, 2026

fix(agents): strip null-valued optional args from MCP tool calls #96729

Closed

clawsweeper Bot mentioned this pull request Jun 25, 2026

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

Closed

3 tasks

openclaw-barnacle Bot added size: S and removed size: XS labels Jun 26, 2026

zw-xysk closed this Jun 26, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(mcp): strip null optional arguments before MCP tool calls#96718

fix(mcp): strip null optional arguments before MCP tool calls#96718
zw-xysk wants to merge 4 commits into
openclaw:mainfrom
zw-xysk:fix/issue-96716-mcp-tool-null-arg

zw-xysk commented Jun 25, 2026 •

edited

Loading

Uh oh!

zw-xysk commented Jun 25, 2026

Uh oh!

clawsweeper Bot commented Jun 25, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 25, 2026 •

edited

Loading

Uh oh!

zw-xysk commented Jun 25, 2026

Uh oh!

clawsweeper Bot commented Jun 25, 2026 •

edited

Loading

Uh oh!

zw-xysk commented Jun 26, 2026

Uh oh!

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

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

Uh oh!

What Problem This Solves

Evidence

Fix

Terminal output

What was not tested

Changes

Uh oh!

zw-xysk commented Jun 25, 2026

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!

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

Uh oh!

Uh oh!

zw-xysk commented Jun 25, 2026

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!

zw-xysk commented Jun 26, 2026

Uh oh!

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

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

zw-xysk commented Jun 25, 2026 •

edited

Loading

clawsweeper Bot commented Jun 25, 2026 •

edited

Loading

clawsweeper Bot commented Jun 25, 2026 •

edited

Loading

clawsweeper Bot commented Jun 25, 2026 •

edited

Loading

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading