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

zw-xysk · 2026-06-26T14:06:05Z

What Problem This Solves

MCP tool calls with optional arguments set to JSON null fail via OpenClaw but succeed via other MCP clients. Root cause: when a JSON schema uses anyOf [{type: string}, {type: null}], the union coercion (coerceWithUnionSchema) tries the string branch first, which coerces null to "" (a valid string), and returns that before ever trying the {type: null} branch. The MCP server receives "" instead of null, triggering a different code path.

Why This Change Was Made

coerceWithUnionSchema iterated schemas in order and applied coerceWithJsonSchema before validating. For anyOf [{type: string}, {type: null}] with value null, the first schema coerces null → "" and the validator passes — so the null branch is never tried.

The fix is narrowly scoped: when value is null, check if any union member accepts null directly (type: "null") before falling through to coercion. This only adds behavior for the null case, without affecting other union types like number | string.

Changes Made

packages/llm-core/src/validation.ts: Added null-first check in coerceWithUnionSchema.
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

Schema: insight_id: anyOf [{type: string}, {type: null}] (from issue #96716)

Input:      {"insight_id":null,"cluster_name":"testenv-cluster"}
Validated:  {"insight_id":null,"cluster_name":"testenv-cluster"}
✅ insight_id preserved as null — MCP server receives null, not empty string

Before fix (simulated): null would be coerced to "" via the string branch, causing the MCP server to error.

After fix: null matches {type: null} directly and is preserved.

✅ Null values in anyOf [string, null] schemas preserved as null
✅ Normal string values unchanged
✅ Non-nullable type coercion (number, integer) still works
✅ Only affects null values — no impact on other union types

What was not tested: End-to-end with a real remote MCP server (the fix is in the shared validation layer — all tool calls benefit from it).

AI-assisted

clawsweeper · 2026-06-26T14:07:05Z

Codex review: needs real behavior proof before merge. Reviewed June 26, 2026, 10:43 AM ET / 14:43 UTC.

Summary
This PR changes llm-core plain JSON-schema union coercion to preserve null for nullable anyOf/oneOf members and adds a regression test for validateToolArguments.

PR surface: Source +15, Tests +25. Total +40 across 2 files.

Reproducibility: yes. source-reproducible: current main validates tool arguments before execution and can coerce null through the first validating string branch before a nullable union branch is considered. I did not run a configured awslabs EKS MCP server or capture a wire payload.

Review metrics: 2 noteworthy metrics.

Shared Union Coercion Path: 1 helper changed. The helper is used by plain JSON-schema tool argument validation beyond the reported MCP tool, so compatibility impact is broader than one server.
Candidate Fix Fanout: 2 open siblings, 1 closed unmerged sibling. Multiple branches target the same canonical MCP optional-null bug with different semantics, so maintainers should select one canonical path.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #96716
Summary: This PR is one candidate fix for the canonical MCP optional-null argument issue; sibling PRs target the same root cause with broader or different null-stripping behavior, and none has merged.

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, MCP server details, reproduction steps, logs, and expected nullable argument behavior.
same_root_cause: fix(agents): strip null-valued optional args from MCP tool calls #96729 - This open sibling PR targets the same optional-null MCP argument problem with validation plus schema-aware MCP dispatch behavior.
same_root_cause: fix(mcp): strip null values from MCP tool call arguments #96755 - This open sibling PR uses closing syntax for the same issue and strips top-level null MCP tool-call arguments.
same_root_cause: fix(mcp): strip null optional arguments before MCP tool calls #96718 - This closed unmerged sibling PR targeted the same optional-null MCP argument behavior and remains useful history.

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

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
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] Add redacted terminal output, logs, recording, or a linked artifact from a configured OpenClaw MCP server call showing the nullable argument is not sent as an empty string.
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 validation-layer terminal-style output, but it does not show a configured OpenClaw MCP call proving the affected server receives null instead of an empty string. 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 PR changes shared plain JSON-schema union coercion, so existing tools with nullable anyOf or oneOf primitive branches may now receive null instead of a coerced primitive value after upgrade.
[P1] The PR body includes copied validation-layer terminal output, but not a redacted configured OpenClaw MCP call proving the affected server receives null instead of an empty string.
[P1] Two open sibling PRs still target the same canonical MCP optional-null issue with broader or different semantics, so maintainers need one landing path rather than competing fixes.

Maintainer options:

Require MCP Proof Then Land Narrow Validation Fix (recommended)
Have the contributor add redacted configured OpenClaw MCP output showing the nullable argument reaches the server as null, then merge this narrow validation fix if maintainers accept validation-only scope.
Choose A Broader Sibling Candidate
If maintainers want optional non-nullable null stripping in addition to nullable-null preservation, choose the better-proven sibling as the canonical landing path and retire the others afterward.
Pause Parallel Candidates
Keep this PR and sibling branches paused until maintainers choose the single MCP null-handling contract they want to ship.

Next step before merge

[P1] Manual review remains because proof, compatibility acceptance, and canonical selection among sibling fixes are maintainer/contributor actions rather than a narrow automated code repair.

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

Review details

Best possible solution:

Land one canonical fix that preserves schema-declared nullable nulls on the real OpenClaw MCP path, backed by focused regression coverage and redacted configured MCP proof.

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

Yes, source-reproducible: current main validates tool arguments before execution and can coerce null through the first validating string branch before a nullable union branch is considered. I did not run a configured awslabs EKS MCP server or capture a wire payload.

Is this the best way to solve the issue?

Yes for the narrowed validation bug: preserving schema-declared null in the shared validation layer is cleaner than broad MCP null stripping. It is not merge-ready until maintainers accept the compatibility semantics, pick the canonical sibling path, and the contributor adds real MCP proof.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 1cd6f81a46ae.

Label changes

Label changes:

add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
remove rating: 🧂 unranked krab: Current PR rating is rating: 🦪 silver shellfish, so this older rating label is no longer current.

Label justifications:

P2: This is a normal-priority MCP/tool validation bug fix with a clear source path and limited blast radius, not a core outage.
merge-risk: 🚨 compatibility: The PR changes shared JSON-schema nullable-union coercion semantics, which can alter existing tool arguments on upgrade.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
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 validation-layer terminal-style output, but it does not show a configured OpenClaw MCP call proving the affected server receives null instead of an empty string. 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 +15, Tests +25. Total +40 across 2 files.

View PR surface stats

Area	Files	Added	Net
Source	1	15	+15
Tests	1	25	+25
Docs	0	0	0
Config	0	0	0
Generated	0	0	0
Other	0	0	0
Total	2	40	+40

What I checked:

Current main nullable-union coercion path: Current main coerces null to an empty string for a string schema, and coerceWithUnionSchema returns the first union branch that validates after coercion, matching the reported anyOf [{type:"string"},{type:"null"}] failure mode. (packages/llm-core/src/validation.ts:148, 1cd6f81a46ae)
Real agent validation order: The agent loop prepares and validates tool arguments before executing the tool, so a validation-layer change is on the real tool-call path used before MCP dispatch. (packages/agent-core/src/agent-loop.ts:872, 1cd6f81a46ae)
MCP dispatch boundary: Materialized bundle-MCP tools forward validated input to runtime.callTool, and the runtime passes record-shaped input as MCP SDK arguments. (src/agents/agent-bundle-mcp-materialize.ts:404, 1cd6f81a46ae)
PR head fix shape: The PR adds a value === null pass that returns original null when a union member includes type: "null" and validates it before earlier primitive branches can coerce null. (packages/llm-core/src/validation.ts:207, c3b2539ad844)
Focused regression coverage: The PR adds a validateToolArguments regression test for an anyOf [{ type: "string" }, { type: "null" }] property and expects insight_id to remain null. (packages/llm-core/src/validation.test.ts:43, c3b2539ad844)
MCP dependency contract: The pinned MCP TypeScript SDK v1.29.0 defines tools/call arguments as an optional record of unknown values and forwards supplied params through tools/call, so OpenClaw owns this normalization choice. (package.json:1965, 1cd6f81a46ae)

Likely related people:

obviyus: Local blame maps the current primitive coercion and union-coercion lines to commit 8bc069f; the commit is broad, so this is a routing signal rather than sole ownership. (role: current checkout blame signal; confidence: medium; commits: 8bc069f76f62; files: packages/llm-core/src/validation.ts)
steipete: GitHub path history shows recent llm-core package extraction and multiple MCP materialization/operator commits near the affected validation and MCP tool-call path. (role: recent area contributor; confidence: medium; commits: aa0d6e1bca55, 38d3d11cbc0c, 99ce71ddbbdb; files: packages/llm-core/src/validation.ts, src/agents/agent-bundle-mcp-materialize.ts, src/agents/agent-bundle-mcp-runtime.ts)
849261680: Recent merged work changed bundle-MCP materialization and tests for MCP tool-result coercion in the same runtime boundary area. (role: recent adjacent contributor; confidence: medium; commits: b1e4b6b65e2c; files: src/agents/agent-bundle-mcp-materialize.ts, src/agents/agent-bundle-mcp-tools.materialize.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 Fixes openclaw#96716

zw-xysk · 2026-06-26T14:35:34Z

@clawsweeper re-review

clawsweeper · 2026-06-26T14:35:37Z

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

zw-xysk · 2026-06-26T16:27:07Z

@clawsweeper re-review

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

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

c3b2539

… empty string Fixes openclaw#96716

zw-xysk force-pushed the fix/issue-96716-mcp-null-arg-validation branch from 35af6af to c3b2539 Compare June 26, 2026 14:25

openclaw-barnacle Bot added size: XS and removed agents Agent runtime and tooling size: S labels Jun 26, 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 26, 2026

zw-xysk closed this Jun 26, 2026

clawsweeper Bot mentioned this pull request Jun 27, 2026

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

Merged

1 task

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)#97039

fix(validation): preserve null in anyOf unions instead of coercing to empty string (fixes #96716)#97039
zw-xysk wants to merge 1 commit into
openclaw:mainfrom
zw-xysk:fix/issue-96716-mcp-null-arg-validation

zw-xysk commented Jun 26, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading

Uh oh!

zw-xysk commented Jun 26, 2026

Uh oh!

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading

Uh oh!

zw-xysk commented Jun 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

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

Uh oh!

What Problem This Solves

Why This Change Was Made

Changes Made

Evidence

Real behavior proof

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!

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!

zw-xysk commented Jun 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

zw-xysk commented Jun 26, 2026 •

edited

Loading

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading