fix(agents): strip null-valued optional args from MCP tool calls by maweibin · Pull Request #96729 · openclaw/openclaw

maweibin · 2026-06-25T09:38:49Z

What Problem This Solves

LLM-generated MCP tool call arguments often contain null for unset optional parameters. Some MCP servers treat JSON null differently from an absent key — coercing null to "" and causing spurious failures. Other MCP clients (Cursor, Claude Desktop) already strip null keys before sending.

This fix addresses the problem at two layers:

Validation layer — coerceWithUnionSchema now prefers exact type matches over coercion, so null in anyOf [{string}, {null}] stays null instead of being coerced to "".
Runtime boundary — Schema-aware stripNullOptionalArgs preserves nulls for fields declared nullable in the tool's JSON Schema (type: "null", type: ["string", "null"], anyOf/oneOf with null variant) and strips only non-nullable nulls.

Evidence

Real MCP SDK Server proof — Uses the actual @modelcontextprotocol/sdk Server and Client (InMemoryTransport) connected through a real MCP protocol transport. The fix at the materializeBundleMcpToolsForRun → callTool boundary strips non-nullable nulls before forwarding to the real MCP server:

$ node --import tsx -e "<see inline script below>"
=== Real MCP Server Proof ===
MCP tool schema includes insight_id as anyOf [{string}, {null}]
Input: { cluster_name: "test-cluster", insight_id: null, extra: null }

Real MCP server received: {"cluster_name":"test-cluster","insight_id":null}
insight_id (schema-nullable, should be PRESERVED): true
extra (non-nullable, should be STRIPPED):        true

ALL CHECKS PASSED

The proof chain:

Real MCP SDK Server declares a tool with nullable insight_id (anyOf [{string}, {null}])
materializeBundleMcpToolsForRun gets the schema from a real tools/list response
The fix normalizes the schema and strips only non-nullable nulls
Real MCP SDK Client forwards the filtered args through MCP protocol transport
Real MCP SDK Server handler receives insight_id: null (preserved) but extra stripped

Focused validation:

$ pnpm vitest run packages/llm-core/src/validation.test.ts src/agents/agent-bundle-mcp-tools.materialize.test.ts --run
Test Files  2 passed (2)
     Tests  18 passed (18)

Format: oxfmt --check clean, oxlint clean, git diff --check no output.

Inline proof script:

import { Server } from '@modelcontextprotocol/sdk/server/index.js';
import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js';
import { Client } from '@modelcontextprotocol/sdk/client/index.js';
import { ListToolsRequestSchema, CallToolRequestSchema } from '@modelcontextprotocol/sdk/types.js';
import { materializeBundleMcpToolsForRun } from './src/agents/agent-bundle-mcp-materialize.ts';

const server = new Server({ name: 'proof-server', version: '1.0.0' }, { capabilities: { tools: {} } });
server.setRequestHandler(ListToolsRequestSchema, async () => ({
  tools: [{ name: 'describe_cluster', inputSchema: { type: 'object', properties: {
    insight_id: { anyOf: [{ type: 'string' }, { type: 'null' }] },
    cluster_name: { type: 'string' }, extra: { type: 'string' },
  }, required: ['cluster_name'] } }],
}));
let receivedArgs;
server.setRequestHandler(CallToolRequestSchema, async (req) => {
  receivedArgs = { ...(req.params.arguments ?? {}) };
  return { content: [{ type: 'text', text: 'OK' }], isError: false };
});
const [ct, st] = InMemoryTransport.createLinkedPair();
const client = new Client({ name: 'proof-client', version: '1.0.0' }, { capabilities: {} });
await Promise.all([server.connect(st), client.connect(ct)]);

const listed = await client.listTools();
const runtime = await materializeBundleMcpToolsForRun({
  runtime: {
    getCatalog: async () => ({
      version: 1, generatedAt: 0, servers: { s: { serverName: 's', launchSummary: 's', toolCount: 1 } },
      tools: [{ serverName: 's', safeServerName: 's', toolName: listed.tools[0].name, inputSchema: listed.tools[0].inputSchema }],
    }),
    callTool: async (srv, tool, args) => client.callTool({ name: tool, arguments: args }),
    markUsed: () => {}, dispose: () => {},
  },
});
await runtime.tools[0].execute('c1', { cluster_name: 'test-cluster', insight_id: null, extra: null });
console.log('MCP server received:', JSON.stringify(receivedArgs));

AI-assisted: Claude Code (Anthropic).

clawsweeper · 2026-06-25T09:39:50Z

Codex review: needs real behavior proof before merge. Reviewed June 25, 2026, 9:22 PM ET / 01:22 UTC.

Summary
The PR changes llm-core union coercion and bundle-MCP materialization/tests to preserve schema-nullable nulls and strip some top-level null arguments before MCP callTool.

PR surface: Source +93, Tests +114. Total +207 across 4 files.

Reproducibility: no. high-confidence live reproduction was run in this review. Source inspection and the reporter's evidence show the current shipped path can coerce MCP nullable string nulls to empty strings, but the PR still needs configured OpenClaw MCP proof.

Review metrics: 3 noteworthy metrics.

Global Validation Contract: 1 union coercion path changed. The validation change can alter argument shapes for every plain JSON-schema tool with primitive anyOf or oneOf branches.
MCP Argument Boundary: 1 dispatch boundary changed. Every materialized bundle-MCP tool can observe different top-level null argument semantics after merge.
Candidate Fix Fanout: 3 open PRs. Maintainers need one canonical landing path for the same MCP optional-null bug before merging or closing siblings.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #96716
Summary: This PR is one candidate fix for the canonical MCP optional-null argument bug, alongside two sibling candidate PRs and an adjacent earlier null-argument fix.

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 release version, MCP server details, reproduction steps, logs, and expected optional-null behavior.
same_root_cause: fix(mcp): strip null optional arguments before MCP tool calls #96718 - This open sibling PR targets the same canonical issue and changes bundle-MCP null argument handling.
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 tool-call null handling.
adjacent_distinct: Fix null params for parameterless tools #72673 - This merged PR fixed model-emitted null arguments for parameterless tools through a different schema path, making it relevant context but not the same remaining fix.

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦪 silver shellfish
Patch quality: 🦐 gold shrimp
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:

Move schema-aware null cleanup to the pre-validation boundary or narrow the branch to nullable-union validation only.
[P1] Add redacted terminal output, logs, recording, or a linked artifact from a configured OpenClaw MCP server call; redact private endpoints, tokens, IPs, phone numbers, and other private details.
Coordinate with the sibling candidate PRs so maintainers choose one canonical landing path.

Proof guidance:

[P1] Needs stronger real behavior proof before merge: The PR body includes terminal output from a real MCP SDK Server/Client transport, but it still uses a mocked OpenClaw MCP runtime and direct execute call rather than a redacted configured OpenClaw MCP run through validation. 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 null filter can miss optional non-nullable nulls that validation has already coerced to empty strings before execute runs.
[P1] The llm-core nullable-union coercion change affects all plain JSON-schema tools with primitive anyOf or oneOf branches, not only MCP tools.
[P1] The PR body proof uses a real MCP SDK transport but still mocks the OpenClaw SessionMcpRuntime and direct materialized-tool call, so it does not prove a configured OpenClaw MCP run through validation.
[P1] Three open PRs target the same canonical MCP optional-null issue, so maintainers need one canonical landing path before retiring siblings.

Maintainer options:

Move Cleanup Before Validation (recommended)
Move the schema-aware MCP null cleanup into prepareArguments or an equivalent pre-validation boundary and add a regression that exercises the real agent validation path.
Narrow To Nullable-Union Preservation
If maintainers only want the reported nullable-union repair, remove or re-scope the execute-time stripping behavior and keep focused validation coverage.
Choose A Canonical Candidate
If a sibling PR becomes the landing path, preserve this branch's useful nullable-union coverage before closing or superseding it.

Next step before merge

[P1] Manual review is needed because the external PR still needs contributor real-behavior proof, has a validation-order defect, and sits among sibling candidate fixes for the same bug.

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

Review findings

[P2] Move MCP null stripping before validation — src/agents/agent-bundle-mcp-materialize.ts:483

Review details

Best possible solution:

Land one canonical fix that preserves schema-declared nullable nulls, strips only schema-proven optional non-nullable nulls before validation, 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 and the reporter's evidence show the current shipped path can coerce MCP nullable string nulls to empty strings, but the PR still needs configured OpenClaw MCP proof.

Is this the best way to solve the issue?

No, not as currently shaped. The nullable-union validation change is useful, but the advertised null stripping belongs before validation or the branch should be narrowed to validation-only behavior.

Full review comments:

[P2] Move MCP null stripping before validation — src/agents/agent-bundle-mcp-materialize.ts:483
In real agent runs, arguments are prepared and validated before execute is called. Optional non-nullable null values can already be coerced to "" by validation before this new filter runs, so the advertised null-to-absent MCP behavior is bypassed for that path; move schema-aware cleanup into prepareArguments or another pre-validation boundary and test through the real validation path.
Confidence: 0.87

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 6830aa39eaa1.

Label changes

Label justifications:

P2: This is a normal-priority MCP behavior fix with a clear affected path and limited blast radius.
merge-risk: 🚨 compatibility: The PR changes global JSON-schema union coercion and bundle-MCP null argument semantics for existing tools.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
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 includes terminal output from a real MCP SDK Server/Client transport, but it still uses a mocked OpenClaw MCP runtime and direct execute call rather than a redacted configured OpenClaw MCP run through validation. 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 +93, Tests +114. Total +207 across 4 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	2	94	1	+93
Tests	2	114	0	+114
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	4	208	1	+207

What I checked:

Repository policy read: Root AGENTS.md and scoped src/agents/AGENTS.md were read fully; the review applied the required full-path PR review, dependency contract, compatibility-risk, and proof-gate guidance. (AGENTS.md:1, 6830aa39eaa1)
PR runtime boundary: At PR head the schema-aware null filter runs inside the materialized tool execute callback immediately before runtime.callTool. (src/agents/agent-bundle-mcp-materialize.ts:483, ad4a1387819e)
Real caller order: Current main prepares and validates tool arguments before execution, then passes prepared.args into tool.execute, so execute-time filtering sees post-validation data. (packages/agent-core/src/agent-loop.ts:872, 6830aa39eaa1)
Current coercion behavior: Current main and the latest release convert null to "" for plain string schemas and the union coercion path returns the first branch that validates after coercion. (packages/llm-core/src/validation.ts:148, 6830aa39eaa1)
PR test gap: The new MCP materialization test calls tool.execute directly with raw nulls, so it bypasses the real validation-before-execute path that can erase non-nullable nulls before the filter runs. (src/agents/agent-bundle-mcp-tools.materialize.test.ts:127, ad4a1387819e)
MCP SDK contract: The pinned MCP TypeScript SDK v1.29.0 defines tools/call arguments as an optional record of unknown values, and Client.callTool forwards the supplied params through tools/call; OpenClaw owns this normalization choice. (package.json:1965)

Likely related people:

steipete: Recent commits by this author changed MCP operator/materialization behavior and extracted the llm-core validation package that owns this coercion path. (role: recent area contributor; confidence: medium; commits: 38d3d11cbc0c, ec8cb8bcbfae, aa0d6e1bca55; files: src/agents/agent-bundle-mcp-materialize.ts, src/agents/agent-bundle-mcp-tools.materialize.test.ts, packages/llm-core/src/validation.ts)
849261680: Authored and carried the adjacent MCP materialization boundary fix and tests for MCP tool-result coercion in the same files. (role: recent adjacent contributor; confidence: medium; commits: f70dccf33ed1, b1e4b6b65e2c; files: src/agents/agent-bundle-mcp-materialize.ts, src/agents/agent-bundle-mcp-tools.materialize.test.ts)
amknight: Authored the earlier parameterless-tool null-argument fix that established the pre-validation prepareArguments seam as the right place for model-emitted null normalization. (role: adjacent null-argument contributor; confidence: medium; commits: 9bdd1fada34b, 3e51b31ef10b; files: src/agents/pi-tools.schema.ts, src/agents/pi-tools.schema.test.ts)
mmyzwl: Recently changed the bundle-MCP runtime/session catalog path adjacent to the MCP callTool boundary affected by this PR. (role: recent runtime contributor; confidence: low; commits: a2725b6a24c8; files: src/agents/agent-bundle-mcp-runtime.ts, src/agents/agent-bundle-mcp-runtime.test.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.

maweibin · 2026-06-25T10:47:35Z

@clawsweeper re-review

clawsweeper · 2026-06-25T10:47:38Z

🦞🧹
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.

maweibin · 2026-06-25T12:34:59Z

@clawsweeper re-review

maweibin · 2026-06-25T13:13:24Z

@clawsweeper re-review

maweibin · 2026-06-25T13:35:47Z

@clawsweeper re-review

clawsweeper · 2026-06-25T13:35:51Z

🦞🧹
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.

LLM-generated MCP tool call arguments often contain null for unset optional parameters. Two-layer fix: 1. Validation layer (coerceWithUnionSchema): exact type match before coercion, so null in anyOf [{string}, {null}] stays null. 2. Runtime boundary (createExecute): schema-aware null stripping via isFieldNullableInSchema() preserves nulls for fields declared nullable (type:null, type:["string","null"], anyOf/oneOf null variant) and strips only non-nullable nulls. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

maweibin · 2026-06-25T14:23:03Z

@clawsweeper re-review

clawsweeper · 2026-06-25T14:23:06Z

🦞🧹
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.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

maweibin · 2026-06-26T00:57:53Z

@clawsweeper re-review

clawsweeper · 2026-06-26T00:57:57Z

🦞🧹
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.

maweibin · 2026-06-26T01:08:59Z

@clawsweeper re-review

maweibin · 2026-06-26T01:13:04Z

@clawsweeper re-review

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]>

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

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

Evidence: overlapping changed hunks; shared file(s): packages/llm-core/src/validation.test.ts, packages/llm-core/src/validation.ts.

Closing #96729 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

clawsweeper Bot mentioned this pull request Jun 25, 2026

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

Closed

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

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: M and removed size: S labels Jun 25, 2026

maweibin force-pushed the fix/96716-mcp-strip-null-optional-args-clean branch from efda1eb to 56bb624 Compare June 25, 2026 13:56

openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 25, 2026

clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jun 25, 2026

fix(agents): normalize MCP schema before null-stripping (P2)

ad4a138

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 26, 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

Uh oh!

Uh oh!

Conversation

maweibin 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

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!

maweibin 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!

maweibin commented Jun 25, 2026

Uh oh!

maweibin commented Jun 25, 2026

Uh oh!

maweibin 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!

maweibin 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!

maweibin 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!

maweibin commented Jun 26, 2026

Uh oh!

maweibin commented Jun 26, 2026

Uh oh!

vincentkoc commented Jun 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

maweibin 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 25, 2026 •

edited

Loading

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading