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 6 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 1af9ed2 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: |
felixweinberger
force-pushed
the
fweinberger/v2-bc-register-rawshape
branch
from
182ec53 to
9b606ab
Compare
felixweinberger
force-pushed
the
fweinberger/v2-bc-register-rawshape
branch
2 times, most recently
from
2972af1 to
a1bf55a
Compare
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
…wrap with z.object)
felixweinberger
force-pushed
the
fweinberger/v2-bc-register-rawshape
branch
from
a1bf55a to
27e4ddf
Compare
|
@claude review |
|
@claude review |
…tputSchema raw-shape test
|
@claude review |
| /** @deprecated Wrap with `z.object({...})` instead. Raw-shape form: `inputSchema`/`outputSchema` may be a plain `{ field: z.string() }` record; it is auto-wrapped with `z.object()`. */ | ||
| registerTool<InputArgs extends ZodRawShape, OutputArgs extends ZodRawShape | StandardSchemaWithJSON | undefined = undefined>( | ||
| name: string, | ||
| config: { | ||
| title?: string; | ||
| description?: string; | ||
| inputSchema?: InputArgs; | ||
| outputSchema?: OutputArgs; | ||
| annotations?: ToolAnnotations; | ||
| _meta?: Record<string, unknown>; | ||
| }, | ||
| cb: LegacyToolCallback<InputArgs> | ||
| ): RegisteredTool; |
There was a problem hiding this comment.
🟡 nit: the overloads aren't fully orthogonal — inputSchema: z.object({...}) paired with outputSchema: { result: z.string() } (wrapped input + raw output) matches neither overload and yields TS2769, even though the reverse combo (raw input + wrapped output) type-checks via overload 2 and the runtime handles both via normalizeRawShapeSchema. Low-impact since the @deprecated guidance is to wrap anyway, but if you want the changeset's "outputSchema accepts a raw shape" claim to hold unconditionally, widen overload 1's OutputArgs to StandardSchemaWithJSON | ZodRawShape.
Extended reasoning...
What the gap is
registerTool now has two public overload signatures:
inputSchema constraint |
outputSchema constraint |
|
|---|---|---|
| Overload 1 (native) | InputArgs extends StandardSchemaWithJSON | undefined |
OutputArgs extends StandardSchemaWithJSON |
Overload 2 (@deprecated raw-shape) |
InputArgs extends ZodRawShape |
OutputArgs extends ZodRawShape | StandardSchemaWithJSON | undefined |
The four input×output combinations resolve as:
- wrapped + wrapped → overload 1 ✓
- raw + raw → overload 2 ✓
- raw + wrapped → overload 2 ✓ (its
OutputArgsincludesStandardSchemaWithJSON) - wrapped + raw → no match ✗
The last cell falls through both: overload 1 rejects because { result: z.string() } has no '~standard' property and so is not a StandardSchemaWithJSON; overload 2 rejects because a ZodObject instance is not assignable to ZodRawShape = Record<string, z.ZodType> — its own members (_def, shape, parse, ~standard, …) are not z.ZodType, so it fails the index-signature constraint.
Why the runtime would handle it
registerTool's implementation signature accepts StandardSchemaWithJSON | ZodRawShape for both fields and calls normalizeRawShapeSchema on each independently (mcp.ts:914-915). normalizeRawShapeSchema checks isZodRawShape per-argument, so a wrapped inputSchema passes through unchanged while a raw outputSchema gets wrapped — there is no runtime coupling between the two. Only the public overload surface couples them.
Step-by-step proof (verified with tsc)
server.registerTool(
'mixed',
{ inputSchema: z.object({ x: z.number() }), outputSchema: { result: z.string() } },
async ({ x }) => ({ content: [{ type: 'text', text: String(x) }], structuredContent: { result: String(x) } })
);- Overload 1: TS infers
OutputArgs = { result: ZodString }, checks{ result: ZodString } extends StandardSchemaWithJSON→ fails (no'~standard'). Error: Property '"~standard"' is missing in type '{ result: ZodString }'. - Overload 2: TS infers
InputArgs = ZodObject<{x: ZodNumber}, $strip>, checks againstRecord<string, z.ZodType>→ fails (e.g._zod: $ZodObjectInternalsis not assignable toz.ZodType). Error: Index signature for type 'string' is missing / …is not assignable to type 'Record<string, ZodType>'. - Net result: TS2769: No overload matches this call.
Swapping the two — inputSchema: { x: z.number() }, outputSchema: z.object({ result: z.string() }) — compiles cleanly via overload 2, confirming the asymmetry.
Impact
Low. Pure v1 code (both raw) and pure v2 code (both wrapped) both type-check; this only bites someone who has wrapped inputSchema but not outputSchema — an unusual half-migrated state — and the compile error itself points at outputSchema lacking '~standard', which directly suggests wrapping it. The @deprecated JSDoc on the raw-shape overload already steers users that way. The only reason to mention it is that the changeset says raw shapes are accepted "for inputSchema/outputSchema/argsSchema" without noting that a raw outputSchema is only accepted alongside a raw inputSchema.
Fix (optional)
Widen overload 1 to OutputArgs extends StandardSchemaWithJSON | ZodRawShape (mirroring overload 2's output constraint). The callback type ToolCallback<InputArgs> doesn't reference OutputArgs, so no inference change is needed. Alternatively, leave as-is and treat the asymmetry as an intentional nudge toward wrapping.
| export type ZodRawShape = Record<string, z.ZodType>; | ||
|
|
||
| /** Infers the parsed-output type of a {@linkcode ZodRawShape}. */ | ||
| export type InferRawShape<S extends ZodRawShape> = { [K in keyof S]: z.output<S[K]> }; |
There was a problem hiding this comment.
🟡 nit: InferRawShape<S> = { [K in keyof S]: z.output<S[K]> } makes every key required, so { a: z.string().optional() } infers as { a: string | undefined } instead of { a?: string | undefined } — diverging from both v1 (which used z.objectOutputType) and from what the runtime z.object(shape).parse() actually returns. Consider type InferRawShape<S extends ZodRawShape> = z.output<z.ZodObject<S>> so optional/default fields get the ? modifier the same way they did in v1.
Extended reasoning...
What the bug is
InferRawShape is defined as a homogeneous mapped type:
export type InferRawShape<S extends ZodRawShape> = { [K in keyof S]: z.output<S[K]> };A mapped type over keyof S keeps every key required — it just maps the value type. So for S = { a: z.ZodOptional<z.ZodString> }, z.output<S['a']> is string | undefined, and the result is { a: string | undefined }: the key is present, the value may be undefined.
But the runtime path wraps the shape with z.object(shape) (normalizeRawShapeSchema, standardSchema.ts:173), and z.object({ a: z.string().optional() }).parse({}) returns {} — the key is absent. Zod's own output type for that object is { a?: string | undefined } (Zod applies the ? modifier via its internal addQuestionMarks helper for ZodOptional/ZodDefault fields). v1's callback args type used z.objectOutputType<Args, z.ZodTypeAny>, which produced exactly that optional-key form.
So LegacyToolCallback<{a: ZodOptional<ZodString>}> now types args as { a: string | undefined } where v1 typed it as { a?: string | undefined }, and where the actual value passed at runtime is {} or { a: 'x' }.
Step-by-step proof
Take const shape = { a: z.string().optional() }:
ZodRawShapeis satisfied —z.ZodOptional<z.ZodString>extendsz.ZodType.InferRawShape<typeof shape>expands to{ a: z.output<ZodOptional<ZodString>> }={ a: string | undefined }. Keyais required.- At runtime,
normalizeRawShapeSchema(shape)returnsz.object({ a: z.string().optional() }). - On
tools/callwitharguments: {},validateStandardSchemacalls.parse({})→ returns{}(noakey). This value is handed to the callback asargs. - So the callback's static type says
'a' in argsis alwaystrue, but the runtime value has'a' in args === false. - v1's type for the same shape was
z.objectOutputType<{a: ZodOptional<ZodString>}, ZodTypeAny>={ a?: string | undefined }— key optional, matching runtime.
Why nothing else catches it
The only thing between the raw shape and the callback's args parameter type is InferRawShape. The runtime side (z.object(shape)) is correct; only the homegrown mapped type drops the optionality marker. z.output<S[K]> per-field cannot recover it because optionality is a property of the key in the object type, not of the field's standalone output type.
Impact
Low — hence nit. For the dominant pattern async ({ a }) => ..., destructuring yields a: string | undefined either way, and under default TS settings (exactOptionalPropertyTypes: false) { a?: string | undefined } and { a: string | undefined } are mutually assignable, so a v1-typed callback still passes by contravariance. The divergence is only observable for:
'a' in args/Object.keys(args)checks (the type says the key is always there; runtime says it may not be),- projects with
exactOptionalPropertyTypes: true, where the two types are not mutually assignable, .default()fields, where v1/z.objectinfer{ a: T }(required, non-undefined) but this mapped type infers{ a: T }too — actually.default()is fine here sincez.output<ZodDefault<X>>is non-undefined and the key being required matches; the issue is specifically.optional().
This is a @deprecated v1-compat shim, so the bar is "match v1's types", and it doesn't quite.
Fix
One-liner — let Zod do the inference it already knows how to do:
export type InferRawShape<S extends ZodRawShape> = z.output<z.ZodObject<S>>;(or equivalently z.infer<z.ZodObject<S>>). This applies Zod's own optional-key logic, so { a: z.string().optional() } → { a?: string | undefined }, exactly matching v1's z.objectOutputType and the runtime wrapped-schema output.
…spy; cover normalizeRawShapeSchema passthrough/undefined
felixweinberger
force-pushed
the
fweinberger/v2-bc-register-rawshape
branch
from
688b282 to
1af9ed2
Compare
|
@claude review |
There was a problem hiding this comment.
1af9ed2 (test-only) looks good and the two open nits on the overload asymmetry and InferRawShape optionality are non-blocking from my side — deferring to a human for sign-off on the new public API surface (the @deprecated overloads and the ZodRawShape/InferRawShape/Legacy*Callback exports).
Extended reasoning...
Overview
This PR adds v1-compat raw-Zod-shape support to registerTool/registerPrompt: a new normalizeRawShapeSchema helper in @modelcontextprotocol/core that detects { field: z.string() } records and wraps them with z.object(), plus @deprecated overloads on McpServer.registerTool/registerPrompt and four new exported types (ZodRawShape, InferRawShape, LegacyToolCallback, LegacyPromptCallback). Across five review rounds the substantive issues — empty-shape crash, non-Zod-field over-acceptance, changeset/JSDoc inconsistency, missing outputSchema coverage — were all addressed. The latest commit (1af9ed2) is test-only: drops the now-vestigial console.warn spies, adds passthrough/undefined unit tests for normalizeRawShapeSchema, and adds an end-to-end tools/call test. Two 🟡 nits remain open (wrapped-input + raw-output overload gap; InferRawShape not applying ? to optional keys); both are explicitly low-impact and the author appears to have intentionally left them.
Security risks
None identified. The change is purely a schema-normalization shim on the registration path; no auth, transport, or untrusted-input handling is touched. The auto-wrap only fires on values that pass a Zod-specific structural check (_def / vendor === 'zod'), so there's no new injection or prototype-pollution surface.
Level of scrutiny
Moderate-to-high. The runtime change is small and well-tested, but this grows the public API of McpServer — the primary user-facing class — with new overloads and exported types, and REVIEW.md's guiding principles put the burden of proof on additions and ask that every new export be intentional. The PR is part of a coordinated 22-PR v2-bc series with its own reviewer guide and OSS-repo validation, which is exactly the kind of context a human maintainer tracking the series should sign off on rather than a per-PR bot.
Other factors
- The new
Legacy*types are exported frommcp.tsbut (per the author's earlier reply) intentionally not re-exported frompackages/server/src/index.ts; a human should confirm that's the desired surface. - Migration docs (
docs/migration.md/migration-SKILL.md) still say raw shapes are unsupported; the author has batched those updates into #1910 — reasonable, but another reason a human should be in the loop on the series. - Test coverage is now solid: unit tests for the detector/normalizer (incl. empty shape, non-Zod rejection, passthrough, undefined) and integration tests for inputSchema/outputSchema/argsSchema plus an e2e
tools/call.
1 participant
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