From 4c5ba3bc71e329452fb42bce2a95ff16f3b23608 Mon Sep 17 00:00:00 2001 From: Golchic Date: Thu, 27 Aug 2026 16:29:02 +0900 Subject: [PATCH] fix(sequentialthinking): keep nextThoughtNeeded in inputSchema required The `coercedBoolean` helper used `z.preprocess(fn, z.boolean())` to accept string "true"/"false" from clients. With Zod v4, `z.preprocess` widens the input type to `unknown`, so the SDK's JSON Schema generation (io: "input") drops the field from `required` even though it is not `.optional()`. Clients that build arguments from the advertised schema then omit `nextThoughtNeeded` and get `-32602 Input validation error`, which contradicts the schema. Regression from #3533. Reimplement `coercedBoolean` as a union of `z.boolean()` and a string transform. The union keeps a concrete input type, so required fields stay in `required`, while still coercing "true"/"false" (case-insensitively) and now rejecting other strings with a clearer message. - Move `coercedBoolean` into lib.ts and export it so it can be unit-tested - Declare zod as an explicit dependency (already imported directly; matches the everything server) - Add schema.test.ts: round-trips a probe tool through the SDK and asserts a required coercedBoolean field stays in `required` while an optional one does not, plus value-coercion cases Fixes #4651 --- package-lock.json | 3 +- .../__tests__/schema.test.ts | 57 +++++++++++++++++++ src/sequentialthinking/index.ts | 12 +--- src/sequentialthinking/lib.ts | 21 +++++++ src/sequentialthinking/package.json | 3 +- 5 files changed, 83 insertions(+), 13 deletions(-) create mode 100644 src/sequentialthinking/__tests__/schema.test.ts diff --git a/package-lock.json b/package-lock.json index 1845571736..9a1fa9dec5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3906,7 +3906,8 @@ "dependencies": { "@modelcontextprotocol/sdk": "^1.30.0", "chalk": "^5.3.0", - "yargs": "^17.7.2" + "yargs": "^17.7.2", + "zod": "^4.0.0" }, "bin": { "mcp-server-sequential-thinking": "dist/index.js" diff --git a/src/sequentialthinking/__tests__/schema.test.ts b/src/sequentialthinking/__tests__/schema.test.ts new file mode 100644 index 0000000000..7f3adc6ee3 --- /dev/null +++ b/src/sequentialthinking/__tests__/schema.test.ts @@ -0,0 +1,57 @@ +import { describe, it, expect } from 'vitest'; +import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'; +import { coercedBoolean } from '../lib.js'; + +describe('coercedBoolean - value coercion', () => { + it('accepts real booleans unchanged', () => { + expect(coercedBoolean.parse(true)).toBe(true); + expect(coercedBoolean.parse(false)).toBe(false); + }); + + it('coerces the strings "true"/"false" (case-insensitive)', () => { + expect(coercedBoolean.parse('true')).toBe(true); + expect(coercedBoolean.parse('false')).toBe(false); + expect(coercedBoolean.parse('TRUE')).toBe(true); + expect(coercedBoolean.parse('False')).toBe(false); + }); + + it('rejects other values instead of silently passing them through', () => { + // Regression guard: a naive coercion that returns the raw value would let + // the truthy string "false" through as `true`; these must all fail. + expect(coercedBoolean.safeParse('yes').success).toBe(false); + expect(coercedBoolean.safeParse('').success).toBe(false); + expect(coercedBoolean.safeParse(1).success).toBe(false); + expect(coercedBoolean.safeParse(null).success).toBe(false); + }); +}); + +describe('coercedBoolean - JSON Schema generation (issue #4651)', () => { + async function toolInputSchema(inputSchema: Record) { + const server = new McpServer({ name: 'test', version: '0.0.0' }); + server.registerTool( + 'probe', + { inputSchema: inputSchema as never }, + async () => ({ content: [] }), + ); + + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: 'test-client', version: '0.0.0' }); + await Promise.all([server.connect(serverTransport), client.connect(clientTransport)]); + + const { tools } = await client.listTools(); + await client.close(); + return tools.find((t) => t.name === 'probe')!.inputSchema; + } + + it('keeps a non-optional coercedBoolean field in `required`', async () => { + const schema = await toolInputSchema({ flag: coercedBoolean }); + expect(schema.required).toContain('flag'); + }); + + it('omits an .optional() coercedBoolean field from `required`', async () => { + const schema = await toolInputSchema({ flag: coercedBoolean.optional() }); + expect(schema.required ?? []).not.toContain('flag'); + }); +}); diff --git a/src/sequentialthinking/index.ts b/src/sequentialthinking/index.ts index 217845bb3d..2505dad8a2 100644 --- a/src/sequentialthinking/index.ts +++ b/src/sequentialthinking/index.ts @@ -3,17 +3,7 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; import { z } from "zod"; -import { SequentialThinkingServer } from './lib.js'; - -/** Safe boolean coercion that correctly handles string "false" */ -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; -}, z.boolean()); +import { SequentialThinkingServer, coercedBoolean } from './lib.js'; const server = new McpServer({ name: "sequential-thinking-server", diff --git a/src/sequentialthinking/lib.ts b/src/sequentialthinking/lib.ts index 31a1098644..a8673780c0 100644 --- a/src/sequentialthinking/lib.ts +++ b/src/sequentialthinking/lib.ts @@ -1,4 +1,25 @@ import chalk from 'chalk'; +import { z } from 'zod'; + +/** + * Boolean coercion that also accepts the strings "true"/"false" (case-insensitive), + * which some MCP clients send for boolean arguments. + * + * Implemented as a union rather than `z.preprocess(fn, z.boolean())`: `z.preprocess` + * widens the *input* type to `unknown`, so JSON Schema generation (`io: "input"`) + * treats the field as optional and drops it from `required` even when it is not + * `.optional()`. A union keeps a concrete input type, so required fields stay required. + */ +export const coercedBoolean = z.union([ + z.boolean(), + z.string().transform((val, ctx) => { + const normalized = val.toLowerCase(); + if (normalized === 'true') return true; + if (normalized === 'false') return false; + ctx.addIssue({ code: 'custom', message: `Expected "true" or "false", received "${val}"` }); + return z.NEVER; + }), +]); export interface ThoughtData { thought: string; diff --git a/src/sequentialthinking/package.json b/src/sequentialthinking/package.json index 03fbb2b361..94dbeb43d5 100644 --- a/src/sequentialthinking/package.json +++ b/src/sequentialthinking/package.json @@ -27,7 +27,8 @@ "dependencies": { "@modelcontextprotocol/sdk": "^1.30.0", "chalk": "^5.3.0", - "yargs": "^17.7.2" + "yargs": "^17.7.2", + "zod": "^4.0.0" }, "devDependencies": { "@types/node": "^22",