Skip to content

fix(sequentialthinking): restore nextThoughtNeeded in the advertised inputSchema required array - #4652

Open
AmirF194 wants to merge 1 commit into
modelcontextprotocol:mainfrom
AmirF194:fix/4651-nextthoughtneeded-required-drift
Open

fix(sequentialthinking): restore nextThoughtNeeded in the advertised inputSchema required array#4652
AmirF194 wants to merge 1 commit into
modelcontextprotocol:mainfrom
AmirF194:fix/4651-nextthoughtneeded-required-drift

Conversation

@AmirF194

Copy link
Copy Markdown

Root cause

Commit 1cdf806d (#3533) wrapped nextThoughtNeeded in a z.preprocess()-based coercedBoolean to fix a real footgun (the string "false" coercing to true). zod's toJSONSchema(..., { io: "input" }) treats a z.preprocess()-wrapped field's input type as unknown, so it silently drops that field from the emitted required array, even though the field carries no .optional(). The advertised schema then says nextThoughtNeeded is optional while the runtime validator still requires it, so any client that builds its call arguments from the declared schema gets -32602 Invalid params.

Invariant

The advertised inputSchema.required array and the runtime validator agree on which fields are mandatory for sequentialthinking.

Fix

Rebuild coercedBoolean as a .transform() on an explicit z.union([z.boolean(), z.string()]) instead of z.preprocess(). The union gives toJSONSchema a concrete input type to report, so nextThoughtNeeded stays in required, while parse behavior (including the case-insensitive string coercion #3533 added) is unchanged.

Verification

  • Docker (node:22-slim), clean checkout of 76d64c8 (current main): a live stdio MCP session (initializetools/listtools/call) built its call arguments strictly from the advertised required array. Fails on main with MCP error -32602: ... expected boolean, received undefined at nextThoughtNeeded; passes on this branch, and tools/list now reports required: ["thought","nextThoughtNeeded","thoughtNumber","totalThoughts"].
  • Same session, sent nextThoughtNeeded: "FALSE" (the case fix(sequential-thinking): use z.coerce for number and boolean params #3533 fixed): still coerces to false on this branch, so the original footgun stays fixed.
  • npm run build (tsc) and npm test (vitest, 14/14) both green on this branch.
  • What I did not verify: this repo's test suite only covers lib.ts (SequentialThinkingServer), not index.ts (0% coverage before and after this change, per npm test's own coverage report), because index.ts calls runServer() unconditionally at module load, so it isn't imported by the existing tests. I verified the fix with a live protocol session instead of adding a unit test, rather than invent a new import-safe seam (exporting the schema, guarding the runServer() call) that has no precedent anywhere else in this repo's ~30 other reference servers. Happy to add that seam if you'd like a permanent regression test here.

Fixes #4651

…inputSchema required array

commit 1cdf806 (modelcontextprotocol#3533) wrapped nextThoughtNeeded in a z.preprocess-based
coercedBoolean to fix a real footgun (string "false" coercing to true).
zod's toJSONSchema(..., { io: "input" }) treats a z.preprocess()-wrapped
field's input type as unknown, so it silently drops that field from the
emitted required array, even though it carries no .optional(). A client
that builds its call arguments from the advertised schema then omits
nextThoughtNeeded and gets a -32602 Invalid params from the runtime
validator, which still requires it.

Rebuild coercedBoolean as a transform on an explicit z.union([z.boolean(),
z.string()]) instead of z.preprocess. The union gives toJSONSchema a
concrete input type to report, so nextThoughtNeeded stays in required,
while parse behavior (including the case-insensitive string coercion
modelcontextprotocol#3533 added) is unchanged.

Fixes modelcontextprotocol#4651
Copilot AI balanced review requested due to automatic review settings August 17, 2026 18:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@AmirF194

Copy link
Copy Markdown
Author

#4655 (AbhiPra24, opened 71 minutes after this one) fixes the same nextThoughtNeeded/required drift with the same mechanism (drop z.preprocess for a union that keeps a real Zod type, so toJSONSchema stops dropping the field), plus a broader scope (also fixes #4575's version drift) and a full integration test suite this PR doesn't have. One behavioral difference worth flagging either way: this PR rejects a malformed nextThoughtNeeded string (anything besides "true"/"false") with a validation error, #4655's fallback silently coerces it to false. Happy to stand down in favor of #4655 if that's the one you'd rather take forward.

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.

sequential-thinking: nextThoughtNeeded missing from inputSchema.required but required at runtime (regression from #3533)

2 participants