feat(compat): registerTool/registerPrompt accept raw Zod shape, auto-wrap with z.object()#1901
feat(compat): registerTool/registerPrompt accept raw Zod shape, auto-wrap with z.object()#1901felixweinberger wants to merge 4 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 9576f20 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@modelcontextprotocol/client
@modelcontextprotocol/server
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
182ec53 to
9b606ab
Compare
2972af1 to
a1bf55a
Compare
…wrap with z.object)
a1bf55a to
27e4ddf
Compare
|
@claude review |
|
@claude review |
| export function isZodRawShape(obj: unknown): obj is Record<string, StandardSchemaV1> { | ||
| if (typeof obj !== 'object' || obj === null) return false; | ||
| if (isStandardSchema(obj)) return false; | ||
| // [].every() is true, so an empty object is a valid raw shape (matches v1). | ||
| return Object.values(obj).every(v => isStandardSchema(v) || (typeof v === 'object' && v !== null && '_def' in v)); | ||
| } |
There was a problem hiding this comment.
🟡 The raw-shape detector and types advertise broader Standard Schema support than the implementation can deliver: ZodRawShape = Record<string, StandardSchemaWithJSON> and isZodRawShape's isStandardSchema(v) branch both accept ArkType/Valibot fields, but z.object() only works with actual Zod types — so { a: type('string') } type-checks, passes the guard, gets wrapped, and then throws on tools/list/tools/call. Since this is a Zod-only v1-compat shim, drop the isStandardSchema(v) disjunct (the '_def' in v check covers Zod v3 and v4) and tighten the JSDoc/type to say Zod-only.
Extended reasoning...
What the bug is
Three artifacts in this PR advertise that the raw-shape overload accepts any Standard Schema field, not just Zod:
| Artifact | Location | Says |
|---|---|---|
| Public type | mcp.ts — export type ZodRawShape = Record<string, StandardSchemaWithJSON> |
Any StandardSchemaWithJSON field is accepted (ArkType, Valibot, …) |
| Runtime guard | standardSchema.ts:152 — .every(v => isStandardSchema(v) || …) |
Any value with ~standard.validate passes |
| JSDoc | standardSchema.ts:142-143 — "Zod (or other Standard Schema) field schemas" |
Explicitly documents non-Zod support |
But the implementation at standardSchema.ts:168 does z.object(schema as z.ZodRawShape), and Zod v4's z.object() requires every shape value to be a ZodType — at parse time and JSON-schema-generation time it walks Zod internals (._zod / ._def) on each field. The as z.ZodRawShape cast hides a real type incompatibility that the runtime guard does not enforce.
Code path that triggers it
import { type } from 'arktype';
server.registerTool('x', { inputSchema: { a: type('string') } }, async ({ a }) => …);- Type-checks:
type('string')implementsStandardSchemaWithJSON, so{ a: type('string') }satisfiesZodRawShape = Record<string, StandardSchemaWithJSON>and resolves to the new@deprecatedoverload. normalizeRawShapeSchema→isZodRawShape({a: type('string')}): not itself a StandardSchema;Object.values→[type('string')];isStandardSchema(type('string'))→true→ guard returnstrue.- Wrapped as
z.object({ a: type('string') } as z.ZodRawShape)— constructs without throwing. - Stored on the registered tool as
inputSchema. tools/list→standardSchemaToJsonSchema(inputSchema, 'input')→ Zod's~standard.jsonSchema.input()walks the shape and reads.defon the ArkType field →TypeError: Cannot read properties of undefined (reading 'def').tools/call→validateStandardSchema→ Zod's~standard.validate()→Invalid element at key 'a': expected a Zod schema.
(One verifier reproduced both errors empirically against the repo's zod/v4.)
Why existing code doesn't prevent it
The only gate between "user passed a raw shape" and "hand it to z.object()" is isZodRawShape, and its predicate is isStandardSchema(v) || (… && '_def' in v). The first disjunct is satisfied by every Standard Schema implementation, so the guard admits exactly the inputs that z.object() cannot handle. The cast on the next line silences the compiler.
Impact
A type-checks-but-crashes footgun on a public overload. Practical likelihood is low: this is a @deprecated v1-compat shim and v1 was Zod-only, so the target audience (v1 migrators) only has Zod fields; ArkType/Valibot users would naturally pass type({...}) / v.object({...}) (a full StandardSchema object), which hits the non-deprecated overload and bypasses auto-wrap entirely. But the type signature, the runtime check, and the JSDoc all actively advertise the broken path.
Step-by-step proof
isStandardSchema(arktypeString)→ has['~standard'].validate→true.isZodRawShape({a: arktypeString})→ not null/object ✓, not itself StandardSchema ✓,[arktypeString].every(v => isStandardSchema(v) || …)→true.normalizeRawShapeSchemareturnsz.object({a: arktypeString} as z.ZodRawShape).z.objectconstructs lazily; no error yet.standardSchemaToJsonSchema(wrapped, 'input')→ Zod iterates shape, dereferencesarktypeString._zod.def(or equivalent) →undefined→ throws.
Fix
One-line tightening — drop the isStandardSchema(v) disjunct so the guard requires Zod's _def marker (present on both Zod v3 and v4 schemas; Zod v4 also has ~standard, so Zod fields still pass):
return Object.values(obj).every(v => typeof v === 'object' && v \!== null && '_def' in v);And update the JSDoc to drop "(or other Standard Schema)". Optionally narrow export type ZodRawShape to something Zod-specific (or at least note in its JSDoc that fields must be Zod schemas because they're handed to z.object()).
Part of the v2 backwards-compatibility series — see reviewer guide.
v2 requires StandardSchema objects (e.g.
z.object({...})) forinputSchema. v1 accepted raw shapes{x: z.string()}. This auto-wraps raw shapesMotivation and Context
v2 requires StandardSchema objects (e.g.
z.object({...})) forinputSchema. v1 accepted raw shapes{x: z.string()}. This auto-wraps raw shapesv1 vs v2 pattern & evidence
v1 pattern:
`registerTool('x', {inputSchema: {a: z.string()}}, cb)`v2-native:
`registerTool('x', {inputSchema: z.object({a: z.string()})}, cb)`Evidence: ~70% of typical server migration LOC was wrapping shapes. Took multiple OSS repos to zero.
How Has This Been Tested?
v2-bc-integrationvalidation branchpnpm typecheck:all && pnpm lint:all && pnpm test:allgreenBreaking Changes
None — additive
@deprecatedshim.Types of changes
Checklist
Additional context
Stacks on: C1