feat(core): add parse/safeParse methods to specTypeSchemas entries#2241
feat(core): add parse/safeParse methods to specTypeSchemas entries#2241felixweinberger wants to merge 7 commits into
Conversation
🦋 Changeset detectedLatest commit: a7c5390 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/codemod
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🔴
packages/core/src/types/specTypeSchema.ts:340-346— Design question to resolve before implementation review:parseSpecType/safeParseSpecType(plusSpecTypeValidationErrorandSafeParseSpecTypeResult) are ~5-line wrappers over the already-publicspecTypeSchemas[name]['~standard'].validate()that only remap{ issues, value }into v1'sparse/safeParseshapes — a migrating consumer can write the identical helper in userland with zero SDK changes, and the only call sites added in this PR are tests and.examples.tssnippets. Per the repo conventions (Minimalism, "Burden of proof is on addition", "One way to do things", "helpers users can write themselves belong in a cookbook, not the SDK"), consider documenting the small helper in the migration guide instead of growing the public surface of core/client/server with a third and fourth parallel validation entry point.Extended reasoning...
What this is. This PR adds four new public symbols to
core/public(and therefore to the public API of@modelcontextprotocol/clientand@modelcontextprotocol/server):parseSpecType,safeParseSpecType,SpecTypeValidationError, andSafeParseSpecTypeResult. Functionally, both helpers are thin adapters over the existing public registry: they callspecTypeSchemas[name]['~standard'].validate(value)and remap the Standard Schema result into v1'sparse(throw on failure) /safeParse({ success, data | issues }) shapes (packages/core/src/types/specTypeSchema.ts:373-378and:397-400). The rest of the addition (formatIssues, the error class) exists only to support that remap.Why this is a design-level concern rather than an implementation bug. The repo's review conventions (REVIEW.md) put "Design justification" first and say to "Kill at the highest level" — and three checklist items in the API surface section apply directly:
- "Helpers users can write themselves belong in a cookbook, not the SDK." Every building block the helpers need —
specTypeSchemas,SpecTypeName,SpecTypes,StandardSchemaV1— is already exported fromcore/public. A consumer migrating from v1 can write the identical, fully type-inferred helper in their own codebase today. - "New abstractions have at least one concrete callsite in the PR." The only callers added here are
packages/core/test/types/specTypeSchema.test.ts,packages/server/test/server/parseSpecTypeWire.test.ts, and the.examples.tsdoc-snippet file. No SDK runtime code consumes the helpers; the cited consumer is an external host-application migration that isn't part of this repo. - "One way to do things — improving an existing API beats adding a parallel one." After this PR there are four parallel ways to runtime-validate a spec type:
isSpecType.X(v),specTypeSchemas.X['~standard'].validate(v),parseSpecType('X', v), andsafeParseSpecType('X', v). Notably, the v2 direction so far has been the opposite —parseSchemaAsyncwas removed in favor ofvalidateStandardSchema, andvalidateStandardSchema/standardSchemaToJsonSchemawere deliberately not added tocore/public.
Concrete demonstration that this is userland-writable. The full replacement, written against only already-public exports:
import { specTypeSchemas, type SpecTypeName, type SpecTypes } from '@modelcontextprotocol/client'; function parseSpecType<K extends SpecTypeName>(name: K, value: unknown): SpecTypes[K] { const r = specTypeSchemas[name]['~standard'].validate(value); if (r.issues) throw new Error(`Invalid ${name}: ${r.issues.map(i => i.message).join('; ')}`); return r.value; }
Step-by-step: (1)
specTypeSchemasis already public and keyed bySpecTypeName, so the typo-safety and inference benefits the PR description cites are available to userland code identically; (2) the['~standard'].validate()call is synchronous (StandardSchemaV1Sync), so the no-awaitguarantee also holds in userland; (3) the remap from{ issues, value }to throw/{ success, data }is the entire body of both new functions — there is no SDK-internal state, schema knowledge, or invariant the helper depends on that a consumer can't reach.Why the existing surface doesn't prevent this.
core/publicis the curated boundary, and CLAUDE.md § Public API Exports stresses that adding a symbol there is intentional and that it propagates to both client and server packages. Nothing stops the addition mechanically — which is exactly why REVIEW.md places the burden of proof on the addition: "Removing something from the public API is far harder than not adding it."Impact. This is not a correctness bug — the helpers work and are well-tested. The cost is permanent public surface across three packages (two functions, one error class, one result type), a fourth parallel validation idiom that future docs, examples, and support questions have to account for, and a precedent for shipping v1-shaped compatibility shims in v2 core rather than in the migration guide.
Suggested resolution. Resolve the design question with maintainers before reviewing the implementation details. The lighter-weight alternative that fully serves the stated motivation (v1→v2 migration ergonomics): keep the
docs/migration.md/docs/migration-SKILL.mdupdates from this PR but show the ~6-line userland helper there (and/or in a cookbook/examples entry) instead of exporting it from the SDK. If maintainers do want the SDK to own the v1-shaped entry points, the remaining open question becomes whether two shapes (parse+safeParse) are needed or whether one (e.g.safeParseSpecTypeonly) suffices — but that's downstream of the surface-area decision. - "Helpers users can write themselves belong in a cookbook, not the SDK." Every building block the helpers need —
| export interface SpecTypeSchema<Input = unknown, Output = Input> extends StandardSchemaV1Sync<Input, Output> { | ||
| /** | ||
| * Validates `value` and returns the parsed output, throwing | ||
| * {@linkcode SpecTypeValidationError} on failure. | ||
| * | ||
| * This is the direct replacement for v1's `<TypeName>Schema.parse(value)`. Validation is | ||
| * synchronous, so no `await` is needed. | ||
| * | ||
| * @example | ||
| * ```ts source="./specTypeSchema.examples.ts#specTypeSchemas_parse" | ||
| * const result = specTypeSchemas.CallToolResult.parse(untrusted); | ||
| * // result is CallToolResult; throws SpecTypeValidationError on invalid input | ||
| * ``` | ||
| */ | ||
| parse(value: unknown): Output; |
There was a problem hiding this comment.
🔴 Design question to settle before merge: parse/safeParse (plus the SpecTypeSchema, SafeParseSpecTypeResult, and SpecTypeValidationError exports they require) have no SDK-internal consumer in this PR — the only call sites are tests, docs, and the examples file — and each method is a ~5-line wrapper a consumer could write over the already-public ['~standard'].validate. This permanently widens the public API of the client and server packages with a third parallel way to validate a spec type (alongside isSpecType and ['~standard'].validate); the migration ergonomics could also be served by a cookbook/migration-guide snippet or by the codemod emitting the remap, so it's worth an explicit maintainer decision that the surface is wanted.
Extended reasoning...
What the concern is. This PR adds two methods (parse, safeParse) to every one of the ~150 specTypeSchemas entries and three new symbols to the curated public barrel in packages/core/src/exports/public/index.ts (SpecTypeSchema, SafeParseSpecTypeResult, SpecTypeValidationError), which both @modelcontextprotocol/client and @modelcontextprotocol/server re-export as their root API. Once published, these are permanent public surface that is hard to remove. The repo's review conventions put design justification first, place the burden of proof on addition, and say helpers users can write themselves belong in a cookbook rather than the SDK — and this addition sits squarely in that category.
No SDK-internal consumer. Grepping packages/ for specTypeSchemas.<X>.parse / .safeParse shows the only call sites introduced by this PR are the new unit tests (packages/core/test/types/specTypeSchema.test.ts), the wire-level test (packages/server/test/server/specTypeSchemaMethodsWire.test.ts), the examples file, and JSDoc/migration docs. Nothing in the SDK runtime consumes the methods — protocol.ts keeps its own validation path (SdkError with SdkErrorCode.InvalidResult). The convention that new abstractions have at least one concrete call site in the PR is therefore satisfied only by tests/docs, not by SDK code.
The wrapper is trivially userland-writable — step-by-step proof. A consumer who wants exactly this ergonomics today can write, against the existing public API:
import { specTypeSchemas, type SpecTypeName } from '@modelcontextprotocol/client';
function parseSpec<K extends SpecTypeName>(name: K, value: unknown) {
const r = specTypeSchemas[name]['~standard'].validate(value);
if (r.issues) throw new Error(`Invalid ${name}: ${r.issues.map(i => i.message).join('; ')}`);
return r.value;
}That is functionally what register() in specTypeSchema.ts now does per entry (call standard().validate, throw or wrap into { success, data | issues }). Compare with what the SDK gains from owning it: nothing internal calls it, and the v1-lookalike is imperfect anyway — the failure branch is .issues, not v1's .error, so migrated catch/else blocks still need a manual touch and the 'pure one-line rename' claim only fully holds on the success path.
Why this conflicts with the stated direction. v2 deliberately removed the exported Zod schemas and standardized on the Standard Schema interface; this re-introduces a Zod-shaped parse/safeParse surface and creates a third parallel way to validate the same value (isSpecType.X(v), specTypeSchemas.X['~standard'].validate(v), and now specTypeSchemas.X.parse(v)), in tension with 'one way to do things'. The migration-time pain described in the PR (a dozen call sites across one large host-app migration, with easy-to-invert .issues/.success remaps) is real, but it is a migration-time cost that could be paid once by a migration-guide/cookbook snippet showing the ~5-line helper above, or by the codemod emitting the remap directly — without permanently widening the API of three packages.
Mitigating context, and why this is still worth raising. The PR offers concrete real-world evidence (the production host-app migration) and notes the entry-method shape came from earlier review feedback, so maintainers may well decide the ergonomics are worth the surface. But that should be an explicit decision: this is the highest-level question about the PR (does the feature belong in the SDK at all?), and per the repo's review-ordering conventions it should be settled before implementation details. Note this is distinct from the existing inline comment about SpecTypeValidationError's placement in the error taxonomy — that comment assumes the methods stay and asks how the error class should be shaped; this one asks whether the methods and their supporting exports belong in the SDK at all.
How to resolve. Either (a) drop the methods and instead add the ~5-line helper to the migration guide/cookbook (and/or have the codemod emit the ['~standard'].validate remap or the helper), keeping the public surface as-is; or (b) keep the feature with an explicit maintainer sign-off that the added permanent surface is justified by migration ergonomics — in which case the related comments about SpecTypeValidationError's taxonomy placement and the changeset coverage should be addressed as part of accepting it.
| If a `*Schema` constant was used for **runtime validation** (not just as a `request()` argument), the `specTypeSchemas.<TypeName>` entries carry `parse`/`safeParse` methods — a mechanical rename: | ||
|
|
||
| | v1 pattern | v2 replacement | | ||
| | -------------------------------------------------- | -------------------------------------------------------------------------------------- | | ||
| | `<TypeName>Schema.parse(value)` | `specTypeSchemas.<TypeName>.parse(value)` (returns the parsed value; throws `SpecTypeValidationError` with `.issues`) | | ||
| | `<TypeName>Schema.safeParse(value)` | `specTypeSchemas.<TypeName>.safeParse(value)` (`{ success: true, data }` \| `{ success: false, issues }`) | | ||
| | `CallToolResultSchema.safeParse(value).success` | `isSpecType.CallToolResult(value)` | | ||
| | `<TypeName>Schema.safeParse(value).success` | `isSpecType.<TypeName>(value)` | | ||
| | `<TypeName>Schema.parse(value)` | `specTypeSchemas.<TypeName>['~standard'].validate(value)` (returns a `Result` synchronously, not the value) | | ||
| | Passing `<TypeName>Schema` as a validator argument | `specTypeSchemas.<TypeName>` (a `StandardSchemaV1Sync<In, Out>`) | | ||
| | Passing `<TypeName>Schema` as a validator argument | `specTypeSchemas.<TypeName>` (a `StandardSchemaV1Sync<In, Out>` with `parse`/`safeParse`) | | ||
|
|
There was a problem hiding this comment.
🔴 The v1→v2 codemod's spec-schema transform (packages/codemod/src/migrations/v1-to-v2/transforms/specSchemaAccess.ts, untouched by this PR) still emits user-facing diagnostics stating that .parse()/.safeParse() are "not available in v2" and that a "Manual rewrite required" — statements that become factually wrong once this PR lands, so the published @modelcontextprotocol/codemod will directly contradict the SDK and this migration table. Even if retargeting the codemod's rewrite output to emit .parse/.safeParse stays a follow-up, the "not available" diagnostic prose should be corrected in the same change so the migration tooling doesn't steer users away from the API this PR adds.
Extended reasoning...
What the bug is. This PR adds .parse()/.safeParse() methods to every specTypeSchemas entry and rewrites both migration guides to present them as the primary replacement for v1's <TypeName>Schema.parse()/.safeParse() call sites. But the v1→v2 codemod's spec-schema transform — packages/codemod/src/migrations/v1-to-v2/transforms/specSchemaAccess.ts, not touched by this PR — still emits user-facing diagnostics that flatly contradict the new API:
- Lines 107–108:
${localName}.safeParse() not available in v2. Use isSpecType.${typeName}(value) for boolean validation, or specTypeSchemas.${typeName}['~standard'].validate(value) for full result. - Lines 120–121:
${localName}.parse() not available in v2. Use isSpecType... or ['~standard'].validate(value) and check for issues. - Lines 137, 162, 188:
Replaced ${localName} with specTypeSchemas.${typeName}. Note: typed as StandardSchemaV1, not ZodType — Zod methods like .safeParse()/.parse() are not available.(line 137 adds "Manual rewrite required.")
In addition, rewriteCapturedSafeParse (lines ~249–331) still rewrites captured safeParse call sites into the verbose ['~standard'].validate remap (.success → .issues === undefined, .data → .value) — exactly the error-prone pattern this PR's description says it was written to eliminate.
The code path that triggers it. A user runs the published @modelcontextprotocol/codemod (the pkg-pr-new bot shows it is built and published from this same monorepo and this PR) on v1 code containing CallToolResultSchema.parse(value) or OAuthTokensSchema.safeParse(value). The transform replaces the identifier with specTypeSchemas.CallToolResult and then prints a warning telling the user that .parse()/.safeParse() "are not available" and that a manual rewrite to ['~standard'].validate is required.
Step-by-step proof. (1) This PR's register() in packages/core/src/types/specTypeSchema.ts attaches parse/safeParse to every specTypeSchemas entry, so specTypeSchemas.CallToolResult.parse(value) works. (2) The codemod's value-position rewrite already turns CallToolResultSchema into specTypeSchemas.CallToolResult, so the remaining trailing .parse(value) in the user's code now compiles and runs correctly with no further edits. (3) Yet the diagnostic emitted at lines 137/162/188 for that exact rewrite says the Zod-style methods "are not available" and (at 137) that a "Manual rewrite required" — directing the user to undo a working call site and replace it with the ['~standard'].validate remap that docs/migration.md and docs/migration-SKILL.md (updated in this PR) now demote to "the underlying mechanism". (4) The lines 107–108/120–121 diagnostics for the bare-constant case likewise recommend only isSpecType or the ['~standard'].validate remap, never mentioning the one-line rename this PR documents as the primary path.
Why nothing prevents it. Nothing in this PR touches the codemod package, and there is no test or lint coupling the codemod's diagnostic strings to the SDK's actual API surface, so the contradiction ships silently. The PR description's "Follow-up opportunity" only covers the enhancement of emitting .parse/.safeParse rewrites directly; it does not address that the existing diagnostic prose becomes affirmatively false the moment this merges.
Impact. The official migration tool will contradict the official migration docs and the SDK's behavior in the same release: users who trust the codemod output will hand-rewrite working call sites into the verbose, inversion-prone ['~standard'].validate pattern this PR exists to remove, and users who read the docs will see the codemod calling the documented API nonexistent. No runtime breakage, but published, user-facing guidance becomes incorrect.
How to fix. At minimum, in this same change update the diagnostic strings in specSchemaAccess.ts (lines 107–108, 120–121, 137, 162, 188) to say the entries now carry .parse()/.safeParse() (e.g. "the rewritten specTypeSchemas.X entry supports .parse()/.safeParse() directly; .parseAsync() is not needed — validation is synchronous"). The fuller follow-up — having the transform emit .parse/.safeParse rewrites and retiring rewriteCapturedSafeParse's ['~standard'].validate remap — can land separately as planned.
There was a problem hiding this comment.
Thanks for adding the note in docs/migration-SKILL.md (commit b8deb0b) acknowledging that the codemod currently rewrites these call sites to the ['~standard'].validate form — that resolves the docs-side inconsistency for readers of the migration guide.
However, the core issue is still open: the codemod's own diagnostic strings in packages/codemod/src/migrations/v1-to-v2/transforms/specSchemaAccess.ts remain unchanged and become factually wrong once this PR lands:
- Lines 107–108 and 120–121 still tell users
.safeParse()/.parse()are "not available in v2" and point only toisSpecTypeor the['~standard'].validateremap. - Lines 137, 162, 188 still say "Zod methods like .safeParse()/.parse() are not available" (line 137 adds "Manual rewrite required").
A user running the published @modelcontextprotocol/codemod (built from this same monorepo) sees those messages without ever reading the migration guide, so the official tooling will still steer them away from the API this PR adds — and away from the docs' recommended form. The doc note even calls the validate form merely "the lower-level" equivalent, while the codemod calls the recommended form nonexistent.
The retargeting of the codemod's rewrite output (emitting .parse/.safeParse directly, retiring rewriteCapturedSafeParse's remap) can certainly stay a follow-up as planned — but the diagnostic prose itself is a small string-only change that would keep the published codemod from contradicting the SDK in the same release. E.g. update those messages to say the rewritten specTypeSchemas.X entry now supports .parse()/.safeParse() directly (with .parseAsync() unnecessary since validation is synchronous).
| server.registerTool( | ||
| 'add', | ||
| { description: 'adds two numbers', inputSchema: { a: z.number(), b: z.number() } }, | ||
| async ({ a, b }) => ({ | ||
| content: [{ type: 'text' as const, text: String(a + b) }], | ||
| structuredContent: undefined | ||
| }) | ||
| ); |
There was a problem hiding this comment.
🟡 The new wire test registers its tool with the deprecated raw-shape form inputSchema: { a: z.number(), b: z.number() } instead of the current API inputSchema: z.object({ a: z.number(), b: z.number() }) that this PR's own migration-guide edits tell users to write. Since this is a brand-new test unrelated to back-compat (the raw-shape overload is otherwise only exercised intentionally in mcp.compat.test.ts), wrapping with z.object() keeps it modeling the current API and ensures it survives eventual removal of the deprecated overload.
Extended reasoning...
What the issue is. The new wire-level test packages/server/test/server/specTypeSchemaMethodsWire.test.ts (added by this PR) registers its tool at line 16 with inputSchema: { a: z.number(), b: z.number() } — the raw Zod-shape form. In v2 this overload of registerTool is explicitly marked deprecated at packages/server/src/server/mcp.ts:879: "@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()."
Why it's inconsistent with the rest of the PR and the test suite. The migration guides edited in this very PR (docs/migration.md, docs/migration-SKILL.md) state that "v2 requires schema objects implementing Standard Schema — raw shapes like { name: z.string() } are no longer supported" and instruct users to wrap with z.object(). Across packages/server/test, the raw-shape form is only used in mcp.compat.test.ts, whose tests are explicitly titled around accepting a raw shape and auto-wrapping it — i.e. it exists precisely to exercise the deprecated back-compat overload. Every other server test (e.g. streamableHttp.test.ts) uses inputSchema: z.object({...}). So a brand-new test that has nothing to do with back-compat ends up modeling the very pattern the PR's own docs tell users not to write.
Step-by-step proof. (1) mcp.ts:879 declares the raw-ZodRawShape overload of registerTool as @deprecated. (2) The new test at specTypeSchemaMethodsWire.test.ts:16 passes inputSchema: { a: z.number(), b: z.number() }, which is a plain record of Zod fields, so TypeScript resolves the call to that deprecated overload. (3) A grep of packages/server/test for raw-shape inputSchema usage returns only mcp.compat.test.ts (4 intentional sites) and this new file — confirming the convention that non-compat tests use the wrapped form. (4) The deprecated overload still auto-wraps with z.object(), so the test compiles and passes today; nothing breaks at runtime.
Why nothing catches it. The deprecated overload is still present and functional, so there is no compile error, no lint failure, and no test failure — the only signal is the @deprecated JSDoc, which doesn't fail CI.
Impact. No functional impact today. The cost is consistency and future-proofing: the new test models the discouraged pattern in a file whose whole purpose is to demonstrate the recommended v1→v2 call shapes, and it would stop compiling if/when the deprecated raw-shape overload is removed (at which point someone would have to touch an unrelated wire test as part of that cleanup).
How to fix. One-line change in the new test:
{ description: 'adds two numbers', inputSchema: z.object({ a: z.number(), b: z.number() }) },The z import is already present, and the handler's destructured ({ a, b }) arguments are unaffected.
| `isCallToolResult(value)` still works, but `isSpecType` covers every spec type by name. | ||
| All validation is synchronous (`StandardSchemaV1Sync`) — never add `await`. `isCallToolResult(value)` still works, but `isSpecType` covers every spec type by name. | ||
|
|
||
| Note: the v1→v2 codemod currently rewrites these call sites to the lower-level `specTypeSchemas.<TypeName>['~standard'].validate(value)` form. That form is equivalent and stays supported, but `parse`/`safeParse` is the recommended form when writing or revising call sites by hand; a codemod update to emit the methods directly is planned. |
There was a problem hiding this comment.
🟡 The note at the end of section 11 says the v1→v2 codemod "currently rewrites these call sites to the lower-level specTypeSchemas.<TypeName>['~standard'].validate(value) form", but the codemod only rewrites the captured-variable safeParse pattern — bare <TypeName>Schema.parse(value) call sites (the first row of the table directly above) and non-captured safeParse calls only get a diagnostic warning and are left untouched. Consider narrowing the sentence (e.g. "rewrites captured safeParse call sites to … and emits warnings for parse() call sites") or updating the codemod in the same change.
Extended reasoning...
What the prose claims vs. what the codemod does. The new note added at docs/migration-SKILL.md:487 states that "the v1→v2 codemod currently rewrites these call sites to the lower-level specTypeSchemas.<TypeName>['~standard'].validate(value) form." The phrase "these call sites" refers to the table directly above (lines 477–483), whose first two rows are <TypeName>Schema.parse(value) and <TypeName>Schema.safeParse(value). Checking the codemod's actual transform in packages/codemod/src/migrations/v1-to-v2/transforms/specSchemaAccess.ts, that claim only holds for one of the patterns in the table.
What the codemod actually rewrites. The transform handles four shapes: (1) XSchema.safeParse(v).success is rewritten to isSpecType.X(v) (lines 83–92); (2) the captured-variable pattern const x = XSchema.safeParse(v) is rewritten to specTypeSchemas.X['~standard'].validate(v) via rewriteCapturedSafeParse (lines 95–100, ~249); (3) a non-captured XSchema.safeParse(v) gets only a diagnostic warning and return false — no rewrite (lines 103–112); and (4) XSchema.parse(v) is explicitly "diagnostic only" — the comment at line 114 says so — pushing a warning ("parse() not available in v2 …") and returning false, leaving the call site untouched (lines 114–125).
Step-by-step proof. Take v1 code containing const result = CallToolResultSchema.parse(value) — the exact pattern in the first row of the table. (1) The codemod's reference walk reaches isParsePattern(ref) at line 115. (2) It pushes the warning \"CallToolResultSchema.parse() not available in v2. Use isSpecType.CallToolResult(value) … or specTypeSchemas.CallToolResult['~standard'].validate(value) …\" and returns false. (3) The source line is not modified at all — it is neither rewritten to ['~standard'].validate(value) (as the new doc sentence claims) nor to .parse(value). The only pattern that genuinely ends up as ['~standard'].validate is the captured safeParse row, and the .success row becomes isSpecType.X(v), not validate.
Why this matters. The SKILL doc is explicitly written for mechanical application by an LLM or a migrating user. The note exists to set expectations about what codemod output will look like versus what to write by hand; as written it tells readers the codemod has already converted their parse() call sites into a working (if verbose) form, when in reality those call sites are left as broken v1 code accompanied only by a warning. A reader trusting the note may assume no manual follow-up is needed for parse() sites after running the codemod. Nothing in the PR or in CI couples this prose to the codemod's behavior, so the inaccuracy ships silently.
Relationship to the existing review comment. This is distinct from the earlier inline comment on this file about the codemod's own diagnostic strings becoming stale ("parse() not available in v2" wording, fixed in the codemod package). That comment asks for the codemod's diagnostics to be updated; this one is about the accuracy of the sentence this PR newly adds to the migration guide. Fixing one does not fix the other.
How to fix. Narrow the sentence to match reality, e.g.: "Note: the v1→v2 codemod currently rewrites captured safeParse call sites to the lower-level specTypeSchemas.<TypeName>['~standard'].validate(value) form and emits warnings for parse() (and non-captured safeParse) call sites; parse/safeParse is the recommended form when writing or revising call sites by hand, and a codemod update to emit the methods directly is planned." Alternatively, update the codemod in the same change so the original sentence becomes true.
v2 removed the exported Zod schemas, so v1 call sites using
SomeTypeSchema.parse(value) / .safeParse(value) currently migrate to
specTypeSchemas.X['~standard'].validate(value) plus a manual result
remap (.success -> .issues === undefined, .data -> .value), which is
verbose and easy to get wrong. These helpers restore the familiar
parse/safeParse shapes on top of the existing spec type registry:
- parseSpecType(name, value) returns the parsed value or throws
SpecTypeValidationError (message summarizes issues; .issues holds
the structured failures, mirroring ZodError usage)
- safeParseSpecType(name, value) returns { success: true, data } |
{ success: false, issues }
Both are synchronous (the backing schemas are StandardSchemaV1Sync)
and keyed by SpecTypeName for autocomplete and typo checking. Also
updates both migration guides to recommend them as the primary
replacement for v1 schema parse/safeParse call sites.
Drives a real McpServer over a linked InMemoryTransport and validates the raw tools/list and tools/call response payloads with parseSpecType and safeParseSpecType, matching the v1 pattern of parsing results received from a live server rather than hand-built fixtures. Includes a negative case proving a real wire payload is rejected when validated as a different spec type.
Review feedback on the string-keyed parseSpecType(name, value) helpers:
property access fits the specTypeSchemas/isSpecType family better and
avoids name-string arguments entirely. Each registry entry is now an
SDK-owned frozen wrapper exposing the entry's Standard Schema interface
plus v1-shaped parse/safeParse methods, so v1 call sites migrate with a
one-line rename: CallToolResultSchema.parse(v) becomes
specTypeSchemas.CallToolResult.parse(v).
parse throws SpecTypeValidationError (unchanged); safeParse returns the
same { success, data | issues } union as before. The top-level
parseSpecType/safeParseSpecType functions are removed (never released).
~standard is delegated lazily to the backing schema, preserving the
library's lazy initialization and the documented validate mechanism.
…kages in changeset
a7c5390 to
81335d9
Compare
Add v1-style
.parse()/.safeParse()methods to everyspecTypeSchemasentry, so v1 runtime-validation call sites migrate with a one-line rename.Motivation and Context
Backwards-compatibility gap this fixes: v1 exported the protocol Zod schemas, and runtime validation with
Schema.parse(value)/Schema.safeParse(value)was a documented, widely-used pattern. v2 removed the schema exports, and the current migration path is:Every call site needs the same mechanical remap (
.success→.issues === undefined,.data→.value, hand-rolled throw), and the result-shape inversion is easy to get wrong silently (e.g.if (r.issues)vsif (!r.success)confusion). While migrating a large production MCP host application from v1 to v2, this remap was needed at a dozen call sites (OAuth metadata/token validation, JSON-RPC message validation in custom transports,_metapayload checks) and several initially landed inverted or with defensiveawaits because the synchronous-validation guarantee is easy to miss.With this change, the
specTypeSchemasentries themselves carry the familiar methods, so migration is a pure rename:.parse(value)returns the parsed value; on failure throwsSpecTypeValidationError— anSdkErrorsubclass with the new codeSdkErrorCode.InvalidSpecType, so it composes with genericinstanceof SdkErrorhandling — whose message summarizes the failures and whose.issuescarries the structured issues (playing the roleZodError.issuesdid in v1 catch blocks)..safeParse(value)returns{ success: true, data } | { success: false, issues }, so migrated call sites keep their.success/.datacontrol flow unchanged.Both are synchronous (the backing schemas validate synchronously — no
awaitneeded), and each entry remains a Standard Schema:['~standard'].validatekeeps working and stays the documented underlying mechanism. The entries are SDK-owned frozen wrappers, so the internal validation library's error types never cross the public boundary.How Has This Been Tested?
packages/core/test/types/specTypeSchema.test.ts: valid/invalid inputs for both methods, schema-default application (proving the parsed output is returned, not the input),SpecTypeValidationErrorname/message/.specType/.issues, the error being anSdkErrorwithSdkErrorCode.InvalidSpecType(caught by genericinstanceof SdkErrorhandlers), the thrown error being the SDK class (not the internal library's), OAuth-record coverage, agreement between.parseand the entry's own['~standard'].validate, type-level inference (expectTypeOf), discriminant narrowing, sync (non-Promise) results, frozen entries, and compile-time rejection of unknown names.packages/server/test/server/specTypeSchemaMethodsWire.test.ts: anMcpServerwith a registered tool is driven over a realInMemoryTransport(initialize → tools/list → tools/call), and the raw JSON-RPCresultpayloads taken off the wire are validated withspecTypeSchemas.X.parse/.safeParse— the exact v1Schema.parse(response.result)call-site pattern this replaces — including a negative case validating the same wire payload against a different spec type.sync:snippetsclean.Breaking Changes
None — purely additive. The entries' declared type widens from
StandardSchemaV1SynctoSpecTypeSchema(aStandardSchemaV1Syncwith the two methods); existing['~standard'].validatecall sites are unaffected.Types of changes
Checklist
Additional context
parseSpecType(name, value)helpers) to methods on the schema entries, based on review feedback: property access matches thespecTypeSchemas/isSpecTypefamily and avoids name-string arguments entirely.parse/safeParsecall sites, with['~standard'].validatekept documented as the underlying mechanism, and state explicitly that validation is synchronous..parse/.safeParsedirectly once this lands, instead of rewriting call sites to the['~standard'].validateremap.SpecTypeValidationErrorexposes.specTypeand.issuesso catch blocks that previously inspectedZodError.issueshave a structured equivalent. Per review feedback it extendsSdkError(mirroringSdkHttpError) rather than adding a new error root, and carries its extras in.datatyped asSpecTypeValidationErrorData.