-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(compat): add /server/zod-schemas subpath re-exporting *Schema constants #1906
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| --- | ||
| '@modelcontextprotocol/server': patch | ||
| --- | ||
|
|
||
| Add `@modelcontextprotocol/server/zod-schemas` subpath for v1 compatibility | ||
|
|
||
| Re-exports the `*Schema` Zod constants (e.g. `CallToolRequestSchema`, `JSONRPCMessageSchema`) and the `getRequestSchema(method)` / `getResultSchema(method)` / `getNotificationSchema(method)` lookup helpers, so v1 code that imported schemas from `@modelcontextprotocol/sdk/types.js` can be pointed at a single subpath. These are Zod schemas; their TS type may change with internal Zod upgrades. Prefer `specTypeSchemas` / `isSpecType` (#1887) for runtime validation. | ||
|
|
||
| The `@modelcontextprotocol/sdk` meta-package (#1913 in this series) re-exports this subpath at `sdk/types.js`, so v1 imports work unchanged once both PRs land. The `specTypeSchemas` / `isSpecType` references in the deprecation guidance are added by #1887. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| /** | ||
| * v1-compat re-export of all protocol Zod schemas. | ||
| * | ||
| * Prefer `specTypeSchemas` / `isSpecType` (from `@modelcontextprotocol/server`) | ||
| * for runtime validation. These are Zod schemas; their TS type may change with | ||
| * internal Zod upgrades. | ||
| * | ||
| * @deprecated Use `specTypeSchemas` / `isSpecType` for runtime validation. | ||
| * @packageDocumentation | ||
| */ | ||
|
felixweinberger marked this conversation as resolved.
|
||
|
|
||
| /** @deprecated Use `specTypeSchemas` / `isSpecType` for runtime validation. */ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One more from a deeper pass (nit, non-blocking): the Separately — bughunter independently flagged the |
||
| export * from '@modelcontextprotocol/core/schemas'; | ||
|
|
||
| /** @deprecated Use `specTypeSchemas` / `isSpecType` for runtime validation. */ | ||
| export { | ||
| IdJagTokenExchangeResponseSchema, | ||
| OAuthClientInformationFullSchema, | ||
| OAuthClientInformationSchema, | ||
| OAuthClientMetadataSchema, | ||
| OAuthClientRegistrationErrorSchema, | ||
| OAuthErrorResponseSchema, | ||
| OAuthMetadataSchema, | ||
| OAuthProtectedResourceMetadataSchema, | ||
| OAuthTokenRevocationRequestSchema, | ||
| OAuthTokensSchema, | ||
| OpenIdProviderDiscoveryMetadataSchema, | ||
| OpenIdProviderMetadataSchema, | ||
| OptionalSafeUrlSchema, | ||
| SafeUrlSchema | ||
| } from '@modelcontextprotocol/core'; | ||
|
Check warning on line 31 in packages/server/src/zodSchemas.ts
|
||
|
Comment on lines
+15
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 The Extended reasoning...What's wrong
/** @deprecated Use `specTypeSchemas` / `isSpecType` for runtime validation. */
export {
IdJagTokenExchangeResponseSchema,
OAuthClientInformationFullSchema,
...
OAuthTokensSchema,
OpenIdProviderDiscoveryMetadataSchema,
...
SafeUrlSchema
} from '@modelcontextprotocol/core';On line 12 the pointer is correct — the wildcard re-export pulls from Why
|
||
| export { | ||
| /** @deprecated Use {@linkcode JSONRPCErrorResponseSchema}. */ | ||
| JSONRPCErrorResponseSchema as JSONRPCErrorSchema, | ||
| /** @deprecated Use {@linkcode ResourceTemplateReferenceSchema}. */ | ||
| ResourceTemplateReferenceSchema as ResourceReferenceSchema | ||
| } from '@modelcontextprotocol/core/schemas'; | ||
|
Check failure on line 37 in packages/server/src/zodSchemas.ts
|
||
|
felixweinberger marked this conversation as resolved.
Comment on lines
+32
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 The Extended reasoning...What changed semanticallyIn v1 ( In v2 ( How the shim leaks the wider schema
Why nothing catches itThe compat test ( Step-by-step proof
ImpactThis is the PR's stated target use case ("runtime validation at HTTP boundaries… custom transports"), and the changeset promises "v1 imports work unchanged." Same-name/silently-wider-acceptance is the worst failure mode for that promise: unlike a missing export, nothing fails — error responses pass validation as success and downstream code reads FixAdd to the alias block at lines 32-37: export {
/** @deprecated Use {@linkcode JSONRPCErrorResponseSchema}. */
JSONRPCErrorResponseSchema as JSONRPCErrorSchema,
/** @deprecated Use {@linkcode ResourceTemplateReferenceSchema}. */
ResourceTemplateReferenceSchema as ResourceReferenceSchema,
/** @deprecated Use {@linkcode JSONRPCResultResponseSchema}. v2's `JSONRPCResponseSchema` now matches both result and error responses. */
JSONRPCResultResponseSchema as JSONRPCResponseSchema
} from '@modelcontextprotocol/core/schemas';And add
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Checked this against v1.25.2, v1.26.0, and v1.29.0 — The guard The two 🟡 below (migration.md:896 stale "All Zod schemas" line, and the OAuth-block |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| import * as z from 'zod/v4'; | ||
|
|
||
| // Compat: the deprecated `/zod-schemas` subpath re-exports the internal Zod | ||
| // schema constants for v1 source compatibility. This test asserts the import | ||
| // resolves and the values are usable Zod schemas at runtime. | ||
| import { | ||
| CallToolRequestSchema, | ||
| getResultSchema, | ||
| JSONRPCMessageSchema, | ||
| ListToolsResultSchema | ||
| } from '@modelcontextprotocol/server/zod-schemas'; | ||
|
|
||
| describe('@modelcontextprotocol/server/zod-schemas (compat subpath)', () => { | ||
| it('re-exports Zod schema constants from core', () => { | ||
| expect(CallToolRequestSchema).toBeInstanceOf(z.ZodType); | ||
| expect(JSONRPCMessageSchema).toBeInstanceOf(z.ZodType); | ||
| expect(ListToolsResultSchema).toBeInstanceOf(z.ZodType); | ||
| }); | ||
|
|
||
| it('re-exports the get*Schema lookup helpers', () => { | ||
| expect(getResultSchema('tools/call')).toBeInstanceOf(z.ZodType); | ||
| }); | ||
|
|
||
| it('schemas parse valid spec values', () => { | ||
| const parsed = CallToolRequestSchema.parse({ | ||
| method: 'tools/call', | ||
| params: { name: 'echo', arguments: {} } | ||
| }); | ||
| expect(parsed.method).toBe('tools/call'); | ||
| expect(parsed.params.name).toBe('echo'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 This PR rewrote line 503 (and migration-SKILL.md:101) to clarify that Zod schema constants are not at the package roots and live at
@modelcontextprotocol/server/zod-schemas— but the same two files still say the opposite further down: migration.md:896 ("All Zod schemas and type definitions fromtypes.ts" under Unchanged APIs — only the import paths changed) and migration-SKILL.md:206-207 ("Unchanged APIs … all Zod schemas"). A reader following those sections will mapsdk/types.js→ package root per the §3 table and hit TS2305. Suggest dropping "Zod schemas and" from migration.md:896 (or pointing at the/zod-schemassubpath), and dropping "all Zod schemas," from migration-SKILL.md:207.Extended reasoning...
What's stale
This PR's purpose is to document where Zod schema constants live in v2. It rewrote two lines to that end:
CallToolRequestSchema) are not re-exported from the package roots; … import them from@modelcontextprotocol/server/zod-schemas."@modelcontextprotocol/server/zod-schemassubpath."But each file has a second occurrence of the same claim that the PR didn't touch:
## Unchanged APIswhose lead-in is "The following APIs are unchanged between v1 and v2 (only the import paths changed)", the bullet still reads: "All Zod schemas and type definitions fromtypes.ts(except the aliases listed above)".StdioServerTransport, all Zod schemas, all callback return types."Both leftover passages list Zod schemas alongside
Client,McpServer,StdioServerTransport, etc. — every other item in those lists moved to a package root. The §3 import-mapping table for@modelcontextprotocol/sdk/types.jsalso points at the package root. So a reader applying "Zod schemas → unchanged, only import path changed → use the §3 mapping" lands on@modelcontextprotocol/server, which is exactly the misconception line 503 was rewritten to dispel.Why nothing catches it
Both files are prose markdown; no link-checker or symbol-checker validates API names mentioned in them. CI is green. The author updated one occurrence in each file (responding to earlier bot review of line 503) but missed the second occurrence further down — a classic §Completeness leftover ("grep for surviving instances of the old form").
Step-by-step proof (migration.md)
docs/migration.mdtop-to-bottom and reaches## Unchanged APIs(line 889).types.ts" alongsideClient, transports, andStdioServerTransport.sdk/types.js→@modelcontextprotocol/clientor@modelcontextprotocol/server).import { CallToolRequestSchema } from '@modelcontextprotocol/server'.packages/server/src/index.tsdoes not re-exportCallToolRequestSchema(onlyzodSchemas.tsdoes, behind the./zod-schemassubpath added in this PR) → TS2305 "Module has no exported member 'CallToolRequestSchema'".import { CallToolRequestSchema } from '@modelcontextprotocol/server'and get a type error") and that the PR fixed at 503 — line 896 is the surviving instance.The same walk-through applies to migration-SKILL.md, which is designed to be loaded into an LLM for mechanical migration; an automated rewrite that pattern-matches "all Zod schemas → only import path changed → apply §3 table" will produce the broken import.
Addressing the refutation (migration-SKILL.md only)
One verifier argued that for migration-SKILL.md specifically, the PR resolved rather than created a contradiction: pre-PR line 101 said schemas were "no longer part of the public API", which flatly contradicted line 207's "all Zod schemas [unchanged]"; post-PR line 101 says they're at
/zod-schemas, so line 207's "(only import paths changed)" is now literally true.That's fair as far as it goes — line 207 is no longer a hard contradiction in the SKILL file. But it remains misleading in context: every other item in that comma-separated list (
Client,McpServer, transports,StdioServerTransport, callback return types) maps to a package root via §3, and line 207 names no path, so the only mapping a mechanical reader can apply is the §3sdk/types.js→ package-root one. Line 101 corrects this if read, but an LLM scanning §5's summary list may not back-reference. Importantly, the refuter explicitly agreed that the migration.md:896 case is a genuine leftover ("both lines were previously consistent (and wrong), and the PR's edit to 503 left 896 stale"). Since the PR should fix migration.md:896 anyway, tidying the parallel migration-SKILL.md:207 in the same sweep is essentially free and keeps the two docs aligned.Impact
Documentation consistency only — no runtime or build impact. Worst case is a confused migrating user (or LLM-driven migration) trying the package root, getting TS2305, and having to re-read §5. Hence nit.
Fix
- All type definitions fromtypes.ts(except the aliases listed above); Zod schema constants moved to@modelcontextprotocol/server/zod-schemas`` — or simply drop "Zod schemas and"./zod-schemassubpath)".