fix(validation): preserve null in anyOf unions instead of coercing to empty string (fixes #96716) by zw-xysk · Pull Request #97212 · openclaw/openclaw

zw-xysk · 2026-06-27T07:50:30Z

What Problem This Solves

MCP tool calls with optional null arguments fail via OpenClaw but succeed via other MCP clients. Root cause: anyOf [{type: string}, {type: null}] coerces null to "" via the string branch before the null branch is tried.

Changes Made

packages/llm-core/src/validation.ts: When value is null, check if any union member accepts null directly before falling through to coercion.
packages/llm-core/src/validation.test.ts: Added regression test.

Evidence

Unit tests: node scripts/run-vitest.mjs run packages/llm-core/src/validation.test.ts — 3/3 passed

Real behavior proof

A real HTTP MCP server received insight_id: null (preserved, not coerced to ""):

✅ REAL PROOF: MCP server received insight_id=null (not empty string)

✅ Null values in anyOf [string, null] preserved as null
✅ Normal string values unchanged
✅ Non-nullable coercion unchanged (null → "" for plain strings)
✅ Numeric coercion preserved

What was not tested: Live DeepSeek API call (fix is in the shared validation layer).

AI-assisted

… empty string Fixes openclaw#96716

zw-xysk · 2026-06-27T07:50:43Z

@clawsweeper re-review

clawsweeper · 2026-06-27T07:50:45Z

🦞🧹
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-27T07:51:27Z

Codex review: needs maintainer review before merge. Reviewed June 27, 2026, 11:01 AM ET / 15:01 UTC.

Summary
The PR changes llm-core validation so null in nullable anyOf/oneOf schemas is accepted before primitive coercion and adds a regression test for the MCP optional-null case.

PR surface: Source +278, Tests +1141, Docs +11, Other +12. Total +1442 across 39 files.

Reproducibility: yes. Source inspection shows current main can coerce null to an empty string when a nullable anyOf tries the string branch first, matching the reporter's MCP symptom; I did not run the remote awslabs EKS MCP server.

Review metrics: 2 noteworthy metrics.

Actual merge-result surface: 2 files changed in merge commit cf58519. The live file list shows stale-base noise, so reviewers should judge the effective merge from the two llm-core files.
Shared validator surface: 1 exported validation path changed. validateToolArguments is used by agent-core and re-exported through the plugin SDK, so nullable-union semantics affect tool and plugin callers.

Stored data model
Persistent data-model change detected: database schema: scripts/lib/official-external-plugin-catalog.json, database schema: src/plugins/official-external-plugin-catalog.test.ts, database schema: src/plugins/official-external-plugin-catalog.ts, migration/backfill/repair: extensions/codex/src/migration/plan.ts, migration/backfill/repair: extensions/codex/src/migration/provider.test.ts, persistent cache schema: extensions/nostr/src/nostr-bus.integration.test.ts, and 4 more. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #96716
Summary: This PR is a focused candidate fix for the canonical MCP optional-null argument bug, while sibling PRs target the same symptom through broader MCP-boundary or earlier validation-only approaches.

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 this issue, and the issue describes the exact optional-null MCP argument failure.
same_root_cause: fix(agents): strip null-valued optional args from MCP tool calls #96729 - This open PR includes validation-layer handling plus MCP-boundary null filtering for the same optional-null MCP failure.
partial_overlap: fix(mcp): strip null values from MCP tool call arguments #96755 - This open PR addresses the same symptom by stripping top-level null MCP arguments at the runtime boundary.
superseded: fix(mcp): strip null optional arguments before MCP tool calls #96718 - This closed unmerged PR is an earlier MCP-boundary attempt for the same issue.
superseded: fix(validation): preserve null in anyOf unions instead of coercing to empty string (fixes #96716) #97039 - This closed unmerged PR is an earlier validation-layer attempt for the same issue.

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

[P2] Choose this PR or a sibling PR as the canonical fix path before merge.

Risk before merge

[P1] The patch intentionally changes shared validation behavior for schemas that explicitly allow null; tools or plugins that accidentally relied on legacy empty-string coercion could receive different arguments after merge.
[P1] Multiple open sibling PRs target the same canonical issue, so maintainers should choose one fix path before landing duplicate behavior.
[P1] The GitHub PR file list is noisy, but the generated merge commit shows the effective merge result is only the two llm-core files.

Maintainer options:

Accept Nullable-Union Preservation (recommended)
Merge this PR after exact-head checks if maintainers accept that schema-declared null now remains null instead of using legacy primitive coercion.
Add Adjacent Union Coverage
Before merge, add focused oneOf or type-array nullable tests if maintainers want extra assurance for sibling nullable-schema shapes.
Choose A Different Canonical PR
Pause this PR if maintainers prefer the broader MCP-boundary approach in an open sibling PR instead of the shared validation-layer fix.

Next step before merge

Maintainers need to accept the shared validation compatibility change and choose this PR or a sibling as the canonical fix; no automated repair is needed.

Security
Cleared: The effective merge result changes only shared validation logic and a colocated test, with no dependency, workflow, secret, install, or publishing surface.

Review details

Best possible solution:

Land this focused validation-layer fix as the canonical path if maintainers accept nullable-union semantics, then close or supersede the sibling MCP null-argument PRs.

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

Yes. Source inspection shows current main can coerce null to an empty string when a nullable anyOf tries the string branch first, matching the reporter's MCP symptom; I did not run the remote awslabs EKS MCP server.

Is this the best way to solve the issue?

Yes. The validation-layer fix is the narrowest maintainable solution for schema-declared nullable unions; broad MCP-boundary null stripping would risk erasing meaningful null values.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 4010b81a77f4.

Label changes

Label justifications:

P2: This is a normal-priority MCP/tool validation bug fix with clear affected behavior and limited blast radius.
merge-risk: 🚨 compatibility: The patch intentionally changes shared validation output for schemas that explicitly accept null.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied after-fix live output from a real HTTP MCP server receiving insight_id as null, plus focused regression-test output.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied after-fix live output from a real HTTP MCP server receiving insight_id as null, plus focused regression-test output.

Evidence reviewed

PR surface:

Source +278, Tests +1141, Docs +11, Other +12. Total +1442 across 39 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	20	323	45	+278
Tests	15	1143	2	+1141
Docs	2	19	8	+11
Config	0	0	0	0
Generated	0	0	0	0
Other	2	23	11	+12
Total	39	1508	66	+1442

What I checked:

Repository policy read: Root AGENTS.md was read fully, along with relevant scoped guides for extensions, docs, scripts, plugin SDK, plugins, and Telegram maintainer notes; the shared-validation compatibility guidance affected this review. (AGENTS.md:1, 4010b81a77f4)
Current main coercion behavior: Current main can coerce null to an empty string through the string primitive coercion path. (packages/llm-core/src/validation.ts:148, 4010b81a77f4)
Current main union behavior: Current main tries each anyOf/oneOf member after coercion, so anyOf [{type:string},{type:null}] can return the string branch before the null branch. (packages/llm-core/src/validation.ts:207, 4010b81a77f4)
PR validation change: The PR head adds a null-first direct validator check before falling back to primitive coercion in coerceWithUnionSchema. (packages/llm-core/src/validation.ts:207, 79ecfc23928b)
PR regression test: The added test asserts insight_id remains null for an anyOf string/null schema while the required cluster_name still validates. (packages/llm-core/src/validation.test.ts:43, 79ecfc23928b)
Actual merge result: The generated merge commit changes only packages/llm-core/src/validation.ts and packages/llm-core/src/validation.test.ts; the live PR file list contains stale-base noise outside the effective merge result. (cf585194b0a3)

Likely related people:

steipete: GitHub path history ties the current packages/llm-core validation files and plugin SDK re-export surface to the LLM core extraction and later validation-file touches. (role: introduced validation surface and recent area contributor; confidence: high; commits: aa0d6e1bca55, 4df95d3c3fd6; files: packages/llm-core/src/validation.ts, packages/llm-core/src/validation.test.ts, src/plugin-sdk/llm.ts)
vincentkoc: Recent agent-core history shows review/coauthor activity near the tool validation call path, and this PR branch includes follow-up commits from the same account. (role: recent adjacent contributor; confidence: medium; commits: 7fe287b0d30d, 19fcd76d1550, 79ecfc23928b; files: packages/agent-core/src/agent-loop.ts, packages/llm-core/src/validation.ts, packages/llm-core/src/validation.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.

… empty string

vincentkoc · 2026-06-27T15:25:47Z

Merged via squash.

Prepared head SHA: 79ecfc23928b4f6fcc21b01becefb37127e43bd1
Landed commit: 361869e43422e7f194abf584f5a86bf7e8b6e80c

… empty string (fixes openclaw#96716) (openclaw#97212) * fix(validation): preserve null in anyOf unions instead of coercing to empty string Fixes openclaw#96716 * fix(validation): preserve null in anyOf unions instead of coercing to empty string * fix(validation): preserve null in anyOf unions instead of coercing to empty string --------- Co-authored-by: Vincent Koc <[email protected]>

fix(validation): preserve null in anyOf unions instead of coercing to…

ef363c9

… empty string Fixes openclaw#96716

openclaw-barnacle Bot added the size: XS label Jun 27, 2026

fix(validation): preserve null in anyOf unions instead of coercing to…

19fcd76

… empty string

fix(validation): preserve null in anyOf unions instead of coercing to…

79ecfc2

… empty string

vincentkoc merged commit 361869e into openclaw:main Jun 27, 2026
145 of 146 checks passed

This was referenced Jun 27, 2026

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

Closed

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

Closed

github-actions Bot mentioned this pull request Jun 27, 2026

📡 Upstream Digest — 2026-06-27 16:59 UTC curtismercier/openclaw-mods#1160

Open

This was referenced Jul 6, 2026

fix(agents): project cron tool schemas for providers #88878

Closed

fix(agents): project nullable tool schemas for providers #88880

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(validation): preserve null in anyOf unions instead of coercing to empty string (fixes #96716)#97212

fix(validation): preserve null in anyOf unions instead of coercing to empty string (fixes #96716)#97212
vincentkoc merged 3 commits into
openclaw:mainfrom
zw-xysk:fix/issue-96716-mcp-null-arg-v4

zw-xysk commented Jun 27, 2026

Uh oh!

zw-xysk commented Jun 27, 2026

Uh oh!

clawsweeper Bot commented Jun 27, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 27, 2026 •

edited

Loading

Uh oh!

Uh oh!

vincentkoc commented Jun 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

zw-xysk commented Jun 27, 2026

What Problem This Solves

Changes Made

Evidence

Real behavior proof

Uh oh!

zw-xysk commented Jun 27, 2026

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

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 27, 2026 •

edited

Loading

clawsweeper Bot commented Jun 27, 2026 •

edited

Loading