Skip to content

fix(sequential-thinking): use z.coerce for number and boolean params#3533

Merged
olaservo merged 2 commits intomodelcontextprotocol:mainfrom
nielskaspers:fix/issue-3428-sequential-thinking-coerce
Mar 15, 2026
Merged

fix(sequential-thinking): use z.coerce for number and boolean params#3533
olaservo merged 2 commits intomodelcontextprotocol:mainfrom
nielskaspers:fix/issue-3428-sequential-thinking-coerce

Conversation

@nielskaspers
Copy link
Contributor

Summary

LLM clients intermittently send thoughtNumber, totalThoughts, and boolean fields as strings instead of native JSON types, causing Zod validation errors. This replaces strict z.number() and z.boolean() with z.coerce.number() and z.coerce.boolean() in the inputSchema to gracefully accept both formats.

Issue

Fixes #3428

Changes

  • Replace z.number() with z.coerce.number() for thoughtNumber, totalThoughts, revisesThought, and branchFromThought
  • Replace z.boolean() with z.coerce.boolean() for nextThoughtNeeded, isRevision, and needsMoreThoughts

Testing

  • z.coerce.number() accepts both 1 and "1", coercing strings to numbers
  • z.coerce.boolean() accepts both true and "true", coercing strings to booleans
  • All .int().min(1) and .optional() constraints are preserved
  • Existing tests pass unchanged since they already use native types

LLM clients (Claude Code, Augment.AI, etc.) intermittently send
thoughtNumber, totalThoughts, and nextThoughtNeeded as strings
instead of native JSON types. Using z.coerce gracefully handles
both string and native inputs without breaking existing behavior.

Fixes modelcontextprotocol#3428
Copy link
Member

@olaservo olaservo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tackling this — the problem is real and well-documented in #3428. The z.coerce.number() changes for the four number fields are correct and safe. However, the z.coerce.boolean() changes for the three boolean fields introduce a serious bug.

Issue: z.coerce.boolean() silently converts "false" to true

z.coerce.boolean() uses JavaScript's Boolean() constructor under the hood. Since "false" is a non-empty string, Boolean("false") === true:

Input z.coerce.boolean() result Correct?
true true
false false
"true" true
"false" true

This is a well-known Zod footgun documented in zod#1672 and zod#3924.

Impact on nextThoughtNeeded: This field controls whether the thinking loop continues. If a client sends nextThoughtNeeded: "false" (meaning "done thinking"), the server would coerce it to true, telling the client to keep going — creating a runaway thought loop that never terminates. This is worse than the original validation error, which at least surfaces the type mismatch.

Suggested fix

Replace z.coerce.boolean() with a safe preprocess transform:

const coercedBoolean = z.preprocess((val) => {
  if (typeof val === "boolean") return val;
  if (typeof val === "string") {
    if (val.toLowerCase() === "true") return true;
    if (val.toLowerCase() === "false") return false;
  }
  return val; // let z.boolean() reject it
}, z.boolean());

Then use coercedBoolean / coercedBoolean.optional() in place of z.coerce.boolean() for nextThoughtNeeded, isRevision, and needsMoreThoughts.

Alternatively, if the project upgrades to Zod v4, z.stringbool() was introduced specifically for this problem.

Summary

  • ✅ Keep the z.coerce.number() changes — they are correct
  • ❌ Replace z.coerce.boolean() with a safe boolean coercion
  • 💡 Consider adding tests for string-typed inputs (a test with "false" would have caught this)

Reviewed with the assistance of Claude Code (claude-opus-4-6). The Boolean("false") === true behavior was independently verified against Zod source code and referenced GitHub issues.

…cess

z.coerce.boolean() is unsafe because Boolean("false") === true in
JavaScript. Replace with a preprocess transform that correctly handles
string "true"/"false" values. Keeps z.coerce.number() as-is since
Number("123") works correctly.
Copy link
Member

@olaservo olaservo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updated implementation addresses the boolean coercion issue perfectly. The coercedBoolean helper using z.preprocess() correctly handles "false"false (unlike z.coerce.boolean() which would produce true), while still accepting native booleans and rejecting invalid input.

Recap:

  • z.coerce.number() for the 4 number fields — correct (unchanged from v1)
  • coercedBoolean via z.preprocess() for the 3 boolean fields — now correct
  • nextThoughtNeeded: "false" will properly signal "done thinking" instead of causing a runaway loop

Clean fix for a real problem affecting multiple MCP clients. Thanks for the quick turnaround @nielskaspers!


Reviewed with the assistance of Claude Code (claude-opus-4-6). The Boolean("false") === true footgun was verified against Zod issues #1672 and #3924.

@olaservo olaservo merged commit 1cdf806 into modelcontextprotocol:main Mar 15, 2026
16 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(sequential-thinking): Use z.coerce for inputSchema to handle string-typed parameters from LLM clients

2 participants