Skip to content

feat(core): add parse/safeParse methods to specTypeSchemas entries#2241

Draft
felixweinberger wants to merge 7 commits into
mainfrom
fweinberger/spec-type-parse-helpers
Draft

feat(core): add parse/safeParse methods to specTypeSchemas entries#2241
felixweinberger wants to merge 7 commits into
mainfrom
fweinberger/spec-type-parse-helpers

Conversation

@felixweinberger
Copy link
Copy Markdown
Contributor

@felixweinberger felixweinberger commented Jun 2, 2026

Add v1-style .parse() / .safeParse() methods to every specTypeSchemas entry, 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:

// v1
const result = CallToolResultSchema.parse(value); // throws ZodError (.issues) on failure
const parsed = OAuthTokensSchema.safeParse(value);
if (parsed.success) use(parsed.data);

// v2 today
const r = specTypeSchemas.CallToolResult['~standard'].validate(value);
if (r.issues !== undefined) throw new Error(/* hand-rolled from r.issues */);
use(r.value);

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) vs if (!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, _meta payload checks) and several initially landed inverted or with defensive awaits because the synchronous-validation guarantee is easy to miss.

With this change, the specTypeSchemas entries themselves carry the familiar methods, so migration is a pure rename:

// v1
const result = CallToolResultSchema.parse(value);
const parsed = OAuthTokensSchema.safeParse(value);

// v2 with this PR
const result = specTypeSchemas.CallToolResult.parse(value);
const parsed = specTypeSchemas.OAuthTokens.safeParse(value);
  • .parse(value) returns the parsed value; on failure throws SpecTypeValidationError — an SdkError subclass with the new code SdkErrorCode.InvalidSpecType, so it composes with generic instanceof SdkError handling — whose message summarizes the failures and whose .issues carries the structured issues (playing the role ZodError.issues did in v1 catch blocks).
  • .safeParse(value) returns { success: true, data } | { success: false, issues }, so migrated call sites keep their .success/.data control flow unchanged.

Both are synchronous (the backing schemas validate synchronously — no await needed), and each entry remains a Standard Schema: ['~standard'].validate keeps 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?

  • Unit tests in 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), SpecTypeValidationError name/message/.specType/.issues, the error being an SdkError with SdkErrorCode.InvalidSpecType (caught by generic instanceof SdkError handlers), the thrown error being the SDK class (not the internal library's), OAuth-record coverage, agreement between .parse and 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.
  • Wire-level integration tests in packages/server/test/server/specTypeSchemaMethodsWire.test.ts: an McpServer with a registered tool is driven over a real InMemoryTransport (initialize → tools/list → tools/call), and the raw JSON-RPC result payloads taken off the wire are validated with specTypeSchemas.X.parse/.safeParse — the exact v1 Schema.parse(response.result) call-site pattern this replaces — including a negative case validating the same wire payload against a different spec type.
  • Full package suites pass locally: core 566, client 367, server 75. Lint, typecheck, and sync:snippets clean.
  • The equivalent call shape was used throughout a real v1→v2 migration of a large MCP host application (client + server + custom transports), which is where the ergonomics gap was identified.

Breaking Changes

None — purely additive. The entries' declared type widens from StandardSchemaV1Sync to SpecTypeSchema (a StandardSchemaV1Sync with the two methods); existing ['~standard'].validate call sites are unaffected.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

  • The API was reshaped from an earlier revision of this branch (string-keyed top-level parseSpecType(name, value) helpers) to methods on the schema entries, based on review feedback: property access matches the specTypeSchemas/isSpecType family and avoids name-string arguments entirely.
  • Both migration guides now show the one-line rename as the primary replacement for v1 parse/safeParse call sites, with ['~standard'].validate kept documented as the underlying mechanism, and state explicitly that validation is synchronous.
  • Follow-up opportunity: the codemod's spec-schema transform can emit .parse/.safeParse directly once this lands, instead of rewriting call sites to the ['~standard'].validate remap.
  • SpecTypeValidationError exposes .specType and .issues so catch blocks that previously inspected ZodError.issues have a structured equivalent. Per review feedback it extends SdkError (mirroring SdkHttpError) rather than adding a new error root, and carries its extras in .data typed as SpecTypeValidationErrorData.

@felixweinberger felixweinberger requested a review from a team as a code owner June 2, 2026 09:26
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Jun 2, 2026

🦋 Changeset detected

Latest commit: a7c5390

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
@modelcontextprotocol/client Minor
@modelcontextprotocol/server Minor
@modelcontextprotocol/express Major
@modelcontextprotocol/fastify Major
@modelcontextprotocol/hono Major
@modelcontextprotocol/node Major

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

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Jun 2, 2026

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/client@2241

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/codemod@2241

@modelcontextprotocol/server

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server@2241

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server-legacy@2241

@modelcontextprotocol/express

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/express@2241

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/fastify@2241

@modelcontextprotocol/hono

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/hono@2241

@modelcontextprotocol/node

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/node@2241

commit: 81335d9

@felixweinberger felixweinberger changed the title feat(core): add parseSpecType/safeParseSpecType validation helpers feat(core): add parse/safeParse methods to specTypeSchemas entries Jun 2, 2026
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (plus SpecTypeValidationError and SafeParseSpecTypeResult) are ~5-line wrappers over the already-public specTypeSchemas[name]['~standard'].validate() that only remap { issues, value } into v1's parse/safeParse shapes — 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.ts snippets. 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/client and @modelcontextprotocol/server): parseSpecType, safeParseSpecType, SpecTypeValidationError, and SafeParseSpecTypeResult. Functionally, both helpers are thin adapters over the existing public registry: they call specTypeSchemas[name]['~standard'].validate(value) and remap the Standard Schema result into v1's parse (throw on failure) / safeParse ({ success, data | issues }) shapes (packages/core/src/types/specTypeSchema.ts:373-378 and :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:

    1. "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 from core/public. A consumer migrating from v1 can write the identical, fully type-inferred helper in their own codebase today.
    2. "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.ts doc-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.
    3. "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), and safeParseSpecType('X', v). Notably, the v2 direction so far has been the opposite — parseSchemaAsync was removed in favor of validateStandardSchema, and validateStandardSchema/standardSchemaToJsonSchema were deliberately not added to core/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) specTypeSchemas is already public and keyed by SpecTypeName, 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-await guarantee 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/public is 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.md updates 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. safeParseSpecType only) suffices — but that's downstream of the surface-area decision.

Comment thread packages/core/src/types/specTypeSchema.ts Outdated
Comment thread .changeset/spec-type-parse-helpers.md Outdated
Comment on lines +280 to +294
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;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 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.

Comment thread docs/migration-SKILL.md
Comment on lines +474 to 483
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`) |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 to isSpecType or the ['~standard'].validate remap.
  • 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).

Comment thread PR-DESCRIPTION.md Outdated
Comment thread docs/migration.md
@felixweinberger felixweinberger marked this pull request as draft June 2, 2026 17:21
Comment on lines +14 to +21
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
})
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Comment thread docs/migration-SKILL.md
`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.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.
@felixweinberger felixweinberger force-pushed the fweinberger/spec-type-parse-helpers branch from a7c5390 to 81335d9 Compare June 2, 2026 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant