feat(compat): add /server/zod-schemas subpath re-exporting *Schema constants#1906
feat(compat): add /server/zod-schemas subpath re-exporting *Schema constants#1906felixweinberger wants to merge 1 commit intomainfrom
Conversation
🦋 Changeset detectedLatest commit: f7e391b The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 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-zod-schemas-subpath
branch
2 times, most recently
from
8c9a388 to
ab8b2ed
Compare
felixweinberger
force-pushed
the
fweinberger/v2-bc-zod-schemas-subpath
branch
from
ab8b2ed to
54f3063
Compare
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline nits: docs/migration-SKILL.md:101 still states Zod schemas "are no longer part of the public API" and points users at type guards instead — that prose now contradicts this subpath and should mention /zod-schemas as the v1-compat option. Also worth confirming whether @modelcontextprotocol/client should get the same subpath, since v1's types.js served both sides.
Extended reasoning...
Overview
Adds a new public subpath @modelcontextprotocol/server/zod-schemas that re-exports ~150 Zod schema constants (plus OAuth/auth schemas from core) as a v1→v2 compat shim. Touches core/package.json (new ./schemas export on the private core package), server/package.json exports map, a new zodSchemas.ts entry, tsdown/tsconfig/typedoc plumbing, a compat test, and a changeset.
Security risks
None — pure re-exports of existing internal constants; no new logic, auth, or I/O paths.
Level of scrutiny
Moderate-to-high. Per REVIEW.md, "every new export is intentional" and "burden of proof is on addition" — this is a deliberate public-API-surface expansion (an entire subpath), even if framed as deprecated. A maintainer should sign off on the surface shape, especially given the export * pulls in three non-v1 helper functions (getResultSchema/getRequestSchema/getNotificationSchema) and the changeset only promises *Schema constants.
Other factors
docs/migration-SKILL.md:101anddocs/migration.md(around L447–503) currently tell users Zod schemas are gone and to use type guards /specTypeSchema; that guidance now needs a pointer to this compat subpath (REVIEW.md "flag prose that now contradicts the implementation").- v1's
@modelcontextprotocol/sdk/types.jswas consumed by client-side code too (e.g., validatingCallToolResult), but only the server package gets the subpath here — may be intentional, but worth an explicit call. - Core package is
private: trueand bundled vianoExternal, so the new./schemascore export doesn't leak externally — that part is fine. - Test coverage is minimal (instanceof + one parse) but adequate for a re-export shim.
| /** | ||
| * These are Zod schemas; their TS type may change with internal Zod upgrades. | ||
| * For runtime validation use them as `Schema.parse(value)`. | ||
| */ |
There was a problem hiding this comment.
🟡 The PR description positions this as a "@deprecated shim. Removed in v3", but the module-level JSDoc only warns about Zod type instability — there's no @deprecated tag, no mention of v3 removal, and no pointer to the v2-native alternative (specTypeSchema() from #1887). Since this file is also excluded from typedoc, the source comment is the only place a consumer can learn this subpath is a temporary compat surface — consider adding that context here.
Extended reasoning...
What's missing
The PR description and changeset both frame /zod-schemas as a deprecated v1-compat shim slated for removal in v3, with specTypeSchema() (#1887) as the v2-native replacement. But the file-level JSDoc at lines 1-4 says only:
These are Zod schemas; their TS type may change with internal Zod upgrades. For runtime validation use them as
Schema.parse(value).
There is no @deprecated tag, no mention that this is a v1-compat bridge, no v3 removal notice, and no pointer to the preferred alternative. A developer landing on this file (or hovering the module specifier in their IDE) gets zero signal that they're on a temporary surface.
Step-by-step
- A v1 consumer follows the migration guide and switches
@modelcontextprotocol/sdk/types.js→@modelcontextprotocol/server/zod-schemas. - They cmd-click into the source or read the emitted
.d.mtsto see what's available. - The header comment tells them "TS type may change with Zod upgrades" — a stability caveat, not a deprecation.
- Nothing tells them this subpath will disappear in v3, or that
specTypeSchema()is the forward path. They reasonably treat it as a stable public API. typedoc.jsonexcludes**/zodSchemas.ts, so generated docs won't carry this either — the source comment is the only documentation surface for this subpath.
Addressing the counter-arguments
- "Module-level
@deprecateddoesn't propagate to import sites" — correct, and per-export tagging is impractical givenexport *over ~150 schemas. This nit is not asking for IDE strikethrough on every symbol; it's asking that the one comment that does exist state the deprecation intent and migration path. Prose in the module JSDoc is cheap and is exactly where a reader looks. - "typedoc excludes it, so it's intentionally undocumented" — that exclusion is presumably to keep ~150 internal Zod constants out of the API reference. It strengthens rather than weakens the case: with generated docs suppressed, the source comment is the only place left to record "this is a shim, removed in v3, prefer X".
- "The PR description's '@deprecated' is colloquial, not a JSDoc promise" — fair, but the repo uses
@deprecatedJSDoc tags elsewhere (e.g. theServerclass,allowedHosts/allowedOrigins) for exactly this kind of "works now, going away later" signal, so it's established convention here. - "
JSONRPCErrorSchema's@deprecatedalready covers it" — no, that tag (line 23) is about the v1→v2 rename (JSONRPCErrorSchema→JSONRPCErrorResponseSchema) and correctly points at the new name within this module. It says nothing about the subpath itself being a shim.
Suggested fix
/**
* v1-compat re-export of the internal Zod `*Schema` constants.
*
* @deprecated This subpath is a v1 compatibility shim and will be removed in v3.
* Prefer {@link specTypeSchema} for runtime validation, or import the TS types directly.
*
* These are Zod schemas; their TS type may change with internal Zod upgrades.
* For runtime validation use them as `Schema.parse(value)`.
*/Documentation/DX only — no runtime impact, hence nit.
There was a problem hiding this comment.
Addressed in f7e391b: added @packageDocumentation block with @deprecated and specTypeSchema() pointer; per series policy we don't promise a specific removal version.
| OptionalSafeUrlSchema, | ||
| SafeUrlSchema | ||
| } from '@modelcontextprotocol/core'; | ||
| export * from '@modelcontextprotocol/core/schemas'; |
There was a problem hiding this comment.
🟡 The export * here re-exports more than *Schema constants — it also pulls in the internal lookup helpers getResultSchema, getRequestSchema, and getNotificationSchema (schemas.ts:2186/2222/2230), which were never in v1's types.js and aren't covered by the changeset's "*Schema constants" promise. Per CLAUDE.md §Public API Exports, public entry points should use explicit named exports rather than export * to keep the surface intentional; consider enumerating the schema constants explicitly (or noting in the changeset that the helpers are intentionally included).
Extended reasoning...
What's leaking. packages/core/src/types/schemas.ts exports three helper functions in addition to the *Schema Zod constants:
getResultSchema(method)— L2186getRequestSchema(method)— L2222getNotificationSchema(method)— L2230
These are method→schema lookup tables used internally (e.g. packages/server/src/experimental/tasks/server.ts via the private core barrel). They are not Zod schema constants and did not exist in v1's @modelcontextprotocol/sdk/types.js.
How it leaks. packages/server/src/zodSchemas.ts:21 does export * from '@modelcontextprotocol/core/schemas'. A wildcard re-export carries every named binding from the source module, so all three get*Schema functions become part of the public @modelcontextprotocol/server/zod-schemas subpath surface.
Step-by-step proof.
core/src/types/schemas.tshasexport function getResultSchema(...)at L2186 (and the other two at L2222/L2230).core/package.jsonnow maps./schemas→./src/types/schemas.ts, so those exports are reachable as@modelcontextprotocol/core/schemas.server/src/zodSchemas.ts:21doesexport * from '@modelcontextprotocol/core/schemas'.server/package.jsonmaps./zod-schemas→dist/zodSchemas.mjs.- Therefore a consumer can write
import { getResultSchema } from '@modelcontextprotocol/server/zod-schemas'and it will resolve and work at runtime — even though the PR/changeset only promise "*Schemaconstants" and these helpers were never part of the v1 compat surface being recreated.
Why the existing code doesn't prevent it. The explicit named-export blocks at L5–20 and L22–25 only add specific symbols; they don't constrain the wildcard. Nothing in schemas.ts marks the helpers @internal, and the typedoc.json exclusion only hides the file from docs — it doesn't change what ships in dist/zodSchemas.mjs.
Why it matters / repo guidance. CLAUDE.md §Public API Exports says: "Use explicit named exports, not export *" and "Internal helpers should stay in the core internal barrel and not be added to … package index files." The intent is that every public export is deliberate. Once these helpers are reachable from a published subpath (even a @deprecated one), they become API surface that has to be supported until the v3 removal, and the changeset description ("Re-exports the *Schema Zod constants") no longer matches what actually ships.
Impact. Low — this is API-surface hygiene on an already-deprecated compat shim, not a correctness bug. But it does (a) violate an explicit repo convention, (b) widen the compat surface beyond what v1 actually offered, and (c) create a docs/implementation mismatch with the changeset.
Fix. Replace the export * with an explicit list of the *Schema constants (matching what v1's types.js exposed), or — if the helpers are intentionally being made public — call that out in the changeset and TSDoc so it's a deliberate decision rather than an accidental leak.
There was a problem hiding this comment.
Documented the get*Schema lookup helpers in the changeset rather than excluding them: enumerating ~156 named constants to drop 3 helpers is more churn than the benefit, and the helpers serve the same v1-compat use case (look up the schema for a method string). f7e391b.
felixweinberger
force-pushed
the
fweinberger/v2-bc-zod-schemas-subpath
branch
from
54f3063 to
a860d7e
Compare
Re-exports the internal *Schema Zod constants (CallToolRequestSchema, JSONRPCMessageSchema, etc.) from a deprecated /zod-schemas subpath so v1 code that imported schemas from @modelcontextprotocol/sdk/types.js has a single drop-in target. - packages/server/src/zodSchemas.ts: re-export barrel (deprecated module) - packages/server/package.json: ./zod-schemas exports entry - packages/server/tsdown.config.ts: build entry + dts path mapping - packages/server/tsconfig.json: path mappings for core/schemas and the self-reference subpath - packages/core/package.json: internal ./schemas subpath (core is private, consumed only by sibling packages) - compat test asserting the import resolves and schemas parse The schemas remain an internal implementation detail; their Zod major version is not covered by semver. Subpath will be removed in v3.
felixweinberger
force-pushed
the
fweinberger/v2-bc-zod-schemas-subpath
branch
from
a860d7e to
f7e391b
Compare
|
Re the review-body points: @claude review |
Review in progress
|
1 participant
Part of the v2 backwards-compatibility series — see reviewer guide.
v2 stopped exporting Zod schema constants (only TS types). Consumers using them for runtime validation at HTTP boundaries (token endpoints, OIDC discovery, custom transports) lose validators. This re-exports them under a
@deprecatedsubpath.Motivation and Context
v2 stopped exporting Zod schema constants (only TS types). Consumers using them for runtime validation at HTTP boundaries (token endpoints, OIDC discovery, custom transports) lose validators. This re-exports them under a
@deprecatedsubpath.v1 vs v2 pattern & evidence
v1 pattern:
`import { OAuthTokensSchema, CallToolRequestSchema } from '@modelcontextprotocol/sdk/types.js'`v2-native:
Evidence: Hits any consumer doing runtime validation of HTTP responses or implementing a custom transport.
How Has This Been Tested?
v2-bc-integrationvalidation branchpnpm typecheck:all && pnpm lint:all && pnpm test:allgreenBreaking Changes
None — additive
@deprecatedshim. Removed in v3.Types of changes
Checklist
Additional context
Stacks on: none