feat(core): add specTypeSchema() for runtime validation of any spec type#1887
feat(core): add specTypeSchema() for runtime validation of any spec type#1887felixweinberger wants to merge 3 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 98091e7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@modelcontextprotocol/client
@modelcontextprotocol/server
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — clean, additive, follows established patterns.
Extended reasoning...
Overview
This PR adds five new type-guard functions (, , , , ) to packages/core/src/types/guards.ts, exports them via the public API barrel, and adds 29 unit tests. It also corrects a pre-existing issue where guards.test.ts lived under src/types/ and was never executed by vitest (which only scans test/**).
Security risks
None. These are pure read-only type predicates that delegate to Zod's safeParse — no side effects, no I/O, no auth-sensitive paths.
Level of scrutiny
Low. Each new guard is a single-line lambda following the exact same pattern as the existing isCallToolResult and other guards already present in the file. The public-index export list is alphabetized correctly. The test coverage is thorough (valid inputs, missing required fields, non-object primitives).
Other factors
The only reported issue is a nit: CLAUDE.md's co-location guideline contradicts the vitest include pattern. This is a pre-existing doc inconsistency and is already flagged as an inline comment on the PR — it does not affect the correctness of the code changes. No outstanding human-reviewer comments exist. The changeset correctly marks downstream packages as patch releases.
felixweinberger
changed the title
felixweinberger
force-pushed
the
fweinberger/spec-type-predicates
branch
2 times, most recently
from
1219736 to
06882c8
Compare
Adds specTypeSchema(name) returning a StandardSchemaV1 validator for any named MCP spec type, and isSpecType(name, value) as a boolean predicate. The SpecTypeName union and SpecTypes map are derived from the internal Zod schemas, so they cover every spec type with no curation. Replaces the earlier curated-5-predicates approach with a single keyed entrypoint that does not require a new export each time an extension embeds a different spec type. Also: moves guards.test.ts to test/ (vitest include is test/**/*.test.ts; the file was previously dead) and corrects CLAUDE.md test-location guidance.
specTypeSchema()/isSpecType() now also cover the OAuth/OpenID types from shared/auth.ts (OAuthTokens, OAuthMetadata, OAuthClientMetadata, etc.), addressing the standalone-validation use noted in #1680 for auth implementers.
felixweinberger
force-pushed
the
fweinberger/spec-type-predicates
branch
from
a08445c to
98091e7
Compare
|
@claude review |
| The `name` argument is typed as `SpecTypeName` — a literal union of every named type in the MCP spec — so you get autocomplete and a compile error on typos. `specTypeSchema()` returns a `StandardSchemaV1<T>`, which composes with any Standard-Schema-aware library and is accepted | ||
| by `setCustomRequestHandler`/`sendCustomRequest`. The pre-existing `isCallToolResult(value)` guard still works. |
There was a problem hiding this comment.
🔴 The new prose says the StandardSchemaV1 returned by specTypeSchema() "is accepted by setCustomRequestHandler/sendCustomRequest", but neither symbol exists anywhere in the SDK — repo-wide grep finds them only in this file. Users following the migration guide will search for these methods and find nothing; the clause should be removed or replaced with the actual API names.
Extended reasoning...
What the bug is and how it manifests
This PR adds a sentence to docs/migration.md (lines 467–468) stating that specTypeSchema() returns a StandardSchemaV1<T> which "is accepted by setCustomRequestHandler/sendCustomRequest". Neither setCustomRequestHandler nor sendCustomRequest exists anywhere in the SDK. A repo-wide grep for both names returns exactly one hit: this line in docs/migration.md itself. There is no source file, no public export, no test, and no other documentation that defines or references these methods.
Why existing code doesn't prevent it
Markdown prose is not type-checked, and there is no link-checker or symbol-existence lint for backtick-quoted identifiers in docs. The companion docs/migration-SKILL.md was updated by the same PR but does not mention these names — only migration.md does — which further suggests this is a typo or a leftover reference to an API that was never shipped (perhaps the author meant the existing setRequestHandler/request/ctx.mcpReq.send family, or a planned-but-unlanded custom-method API).
Impact
The migration guide is the canonical document users consult when porting v1 code. A user who reads "the schema is accepted by setCustomRequestHandler/sendCustomRequest" will reasonably try server.setCustomRequestHandler(...) or client.sendCustomRequest(...), get a TypeScript compile error / "is not a function" runtime error, search the repo, and find nothing. This is exactly the failure mode the repo's review guidance calls out: "prose that promises behavior the code no longer ships misleads consumers." Because this sentence is brand-new in this PR (not pre-existing), it should be fixed before merge.
Step-by-step proof
rg -n 'setCustomRequestHandler|sendCustomRequest'over the whole repo → single match:docs/migration.md:468.rg -n 'CustomRequest|customRequest' packages/→ only matches are a test-localcustomRequestHandler?: ...config field inpackages/middleware/node/test/streamableHttp.test.ts(an internal test fixture option, not a public API and not the names referenced).packages/core/src/exports/public/index.ts(the curated public surface) exports no such symbols.- The PR diff itself adds only
isSpecType,specTypeSchema,SpecTypeName,SpecTypes, and theStandardSchemaV1type — nothing named*CustomRequest*. - Conclusion: the documentation references API that does not exist in this PR or anywhere in the codebase.
How to fix it
Either drop the trailing clause entirely — "...which composes with any Standard-Schema-aware library." already conveys the useful point — or, if the intent was to reference real SDK entry points that accept a StandardSchemaV1, replace the names with the actual exported methods (e.g., the inputSchema/outputSchema/argsSchema slots on registerTool/registerPrompt, or whatever custom-method API the author had in mind once it actually lands).
| const specTypeSchemas: Record<string, z.ZodTypeAny> = {}; | ||
| for (const source of [schemas, authSchemas]) { | ||
| for (const [key, value] of Object.entries(source)) { | ||
| if (key.endsWith('Schema') && value !== null && typeof value === 'object') { | ||
| specTypeSchemas[key.slice(0, -'Schema'.length)] = value as z.ZodTypeAny; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🔴 The wildcard import * as authSchemas plus the endsWith('Schema') filter pulls SafeUrlSchema and OptionalSafeUrlSchema (internal URL-validation refinement helpers, not OAuth spec types) and IdJagTokenExchangeResponseSchema into the map, so 'SafeUrl', 'OptionalSafeUrl', and 'IdJagTokenExchangeResponse' become members of the public SpecTypeName literal union. None of these have a corresponding type in the curated core/public exports, and once shipped removing them is a breaking change — exclude them (e.g. explicit pick-list or Omit<> on the auth import) so SpecTypeName only covers intentional spec types.
Extended reasoning...
What the bug is. specTypeSchema.ts builds the public SpecTypeName union and the runtime specTypeSchemas map by reflecting over every export of shared/auth.ts whose name ends in Schema and whose value looks like a Standard Schema object. But shared/auth.ts exports three schemas that are not part of the curated public OAuth surface:
SafeUrlSchema(auth.ts:6) — az.url().superRefine(...)helper that rejectsjavascript:URLs. It is a building block used inside other schemas, not a named OAuth/MCP type.OptionalSafeUrlSchema(auth.ts:174) —SafeUrlSchema.optional().or(z.literal('').transform(...)). Same story.IdJagTokenExchangeResponseSchema(auth.ts:149) — has a siblingtype IdJagTokenExchangeResponse, but that type is deliberately not re-exported fromexports/public/index.ts(lines 19-33 list the 13 intentional OAuth types and omit it).
All three satisfy both the type-level filter (K extends ${string}Schema`` + { readonly '~standard': unknown }) and the runtime filter (`key.endsWith('Schema') && typeof value === 'object'`), so they end up in the map and in the literal union.
Code path. At specTypeSchema.ts:3 the wildcard import * as authSchemas from '../shared/auth.js' brings in every named export. At :7 SchemaModule = typeof schemas & typeof authSchemas merges them into the keyspace. At :12-18 SchemaKey keeps any key ending in Schema whose value has '~standard' — Zod v4 schemas all have this. At :25 SpecTypeName = StripSchemaSuffix<SchemaKey> therefore includes 'SafeUrl' | 'OptionalSafeUrl' | 'IdJagTokenExchangeResponse'. At :36-43 the runtime loop populates specTypeSchemas['SafeUrl'] etc., so specTypeSchema('SafeUrl') and isSpecType('SafeUrl', x) are valid public API calls at both type and runtime level.
Why nothing prevents it. The only gate is the Schema suffix + '~standard' brand, which every Zod schema in the file passes. There is no allowlist tying SpecTypeName to the curated exports/public/index.ts type list, and the test at specTypeSchema.test.ts:70-77 only asserts that representative names are included, not that internal helpers are excluded.
Impact. SpecTypeName is exported from core/public (public/index.ts:139) and the JSDoc promises it is "every named type in the SDK's protocol and OAuth schemas". SafeUrl/OptionalSafeUrl are not named protocol or OAuth types — they have no TS type counterpart anywhere. Shipping them means isSpecType('SafeUrl', x) autocompletes and type-checks for consumers; later removing them narrows a published literal union, which is a breaking change. This is exactly the "every new export is intentional" / "removing public API is far harder than not adding it" hazard the repo's review guidelines flag.
Step-by-step proof.
shared/auth.ts:6exportsconst SafeUrlSchema = z.url().superRefine(...)— a Zod schema, hence has'~standard'.specTypeSchema.ts:3doesimport * as authSchemas from '../shared/auth.js', soauthSchemas.SafeUrlSchemais in scope.- Type-level:
'SafeUrlSchema' extends${string}Schema`` ✓;typeof SafeUrlSchema extends { readonly '~standard': unknown }✓ → `'SafeUrlSchema'` ∈ `SchemaKey` → `'SafeUrl'` ∈ `SpecTypeName`. - Runtime: the
forloop seeskey='SafeUrlSchema',valueis an object, sospecTypeSchemas['SafeUrl'] = SafeUrlSchema. - A consumer writes
isSpecType('SafeUrl', userInput)— compiles, runs, returnstruefor any safe URL string.SpecTypes['SafeUrl']resolves tostring. exports/public/index.tsdoes not export aSafeUrltype, soSpecTypes['SafeUrl']has no named-type counterpart — the map and the curated surface diverge.
Fix. Replace the wildcard auth import with an explicit pick of the intended OAuth metadata schemas (mirroring the type list at public/index.ts:19-33), or add an Exclude<..., 'SafeUrl' | 'OptionalSafeUrl' | 'IdJagTokenExchangeResponse'> on SchemaKey plus a matching runtime skip-set in the loop. A negative test (expectTypeOf<'SafeUrl'>().not.toMatchTypeOf<SpecTypeName>()) would lock this in.
| * @example | ||
| * ```ts | ||
| * const schema = specTypeSchema('CallToolResult'); | ||
| * const result = schema['~standard'].validate(untrusted); | ||
| * if (result.issues === undefined) { | ||
| * // result.value is CallToolResult | ||
| * } | ||
| * ``` | ||
| */ |
There was a problem hiding this comment.
🔴 The ['~standard'].validate() examples in the JSDoc, docs/migration.md, and docs/migration-SKILL.md don't typecheck and are semantically misleading: validate() returns Result<T> | Promise<Result<T>>, so accessing .issues without await is a TS error (the test file works around this with as casts), and it is not a drop-in replacement for Zod's .parse() (which throws and returns T synchronously). Suggest const result = await schema['~standard'].validate(value) in the JSDoc/migration.md examples, and rewording the SKILL table row for .parse() to show the unwrap-and-check-issues pattern (or point to validateStandardSchema).
Extended reasoning...
What the bug is and how it manifests
StandardSchemaV1.Props.validate is typed at packages/core/src/util/standardSchema.ts:37 as:
readonly validate: (value: unknown, options?) => Result<Output> | Promise<Result<Output>>;The new documentation introduced by this PR shows usage that does not match this signature in three places:
packages/core/src/types/specTypeSchema.ts:60-63(JSDoc@example):const result = schema['~standard'].validate(untrusted); if (result.issues === undefined) { ... }. BecauseresultisResult<T> | Promise<Result<T>>andPromisehas noissuesproperty,result.issuesis a TypeScript compile error.docs/migration.md(the "v2: or get the StandardSchemaV1 validator" block): same pattern —const result = schema['~standard'].validate(value)withoutawait.docs/migration-SKILL.mdsection 11 table: maps<TypeName>Schema.parse(value)→specTypeSchema('<TypeName>')['~standard'].validate(value). These are not equivalent: Zod's.parse()is synchronous, returnsTdirectly, and throws on invalid input;['~standard'].validate()returns a{value}|{issues}wrapper (possibly wrapped in a Promise) and never throws.
Why existing code doesn't prevent it
Per CLAUDE.md, @example snippets are meant to be type-checked via companion .examples.ts files, but no specTypeSchema.examples.ts exists, so the broken example slips through. The test file confirms the typecheck problem indirectly — packages/core/test/types/specTypeSchema.test.ts:12,18,27,32 all need (result as { issues?: unknown }).issues casts to compile, which is exactly the workaround a user copy-pasting the JSDoc would have to discover on their own.
Impact
- Users copy-pasting the JSDoc or migration.md example get
Property 'issues' does not exist on type 'Promise<Result<...>>'. migration-SKILL.mdis explicitly described in CLAUDE.md as "LLM-optimized mapping tables for mechanical migration". An LLM or codemod applying the.parse()row mechanically would convertconst x: T = FooSchema.parse(v)(throws on invalid) intoconst x = specTypeSchema('Foo')['~standard'].validate(v)— the call site that expectedTnow holds aResult|Promisewrapper, and validation failures are silently swallowed instead of thrown. The pre-PR row ("UseisCallToolResult(value)then cast") was honest about the semantic difference; the new row regresses that.
Addressing the refutation
One verifier flagged the SKILL-table issue as a duplicate of the JSDoc-typecheck issue. They share the same root cause (documenting validate() as if it behaves like sync .parse()), which is why they're reported together here as one comment covering all three locations rather than separate findings.
Step-by-step proof
- User reads the JSDoc
@exampleatspecTypeSchema.ts:58-64and pastes into a.tsfile:const schema = specTypeSchema('CallToolResult'); const result = schema['~standard'].validate(untrusted); if (result.issues === undefined) { /* ... */ }
tscresolvesresulttoStandardSchemaV1.Result<CallToolResult> | Promise<StandardSchemaV1.Result<CallToolResult>>(perstandardSchema.ts:37).result.issuesfails: Property 'issues' does not exist on type 'Promise<Result>'.- Separately, a migration following the SKILL table replaces
const tok = OAuthTokensSchema.parse(json)withconst tok = specTypeSchema('OAuthTokens')['~standard'].validate(json).tokis now aResult|Promisewrapper, notOAuthTokens; downstreamtok.access_tokenno longer typechecks, and an invalid payload no longer throws.
How to fix
- JSDoc + migration.md: change to
const result = await schema['~standard'].validate(untrusted);(and mark the example async), or recommendvalidateStandardSchema(schema, value). - migration-SKILL.md row: replace with something semantically equivalent to
.parse(), e.g.const r = await specTypeSchema('<TypeName>')['~standard'].validate(value); if (r.issues) throw ...; r.value— or simply note "useisSpecTypethen narrow/throw" as the previous version did.
1 participant
Part of the v2 backwards-compatibility series — see reviewer guide.
Adds
specTypeSchema(name)/isSpecType(name, value)for runtime validation of any spec type, including OAuth metadata schemas. The v2-native replacement for importing Zod*Schemaconstants directly.Motivation and Context
v2 stopped exporting Zod schema constants. Consumers validating HTTP boundary data (OAuth token responses, OIDC discovery, custom transport frames) lost runtime validators.
specTypeSchema('OAuthTokens')returns the schema;isSpecType('OAuthTokens', x)is a type predicate.How Has This Been Tested?
pnpm typecheck:all && pnpm test:allgreenBreaking Changes
None — additive.
Types of changes
Checklist
Additional context
Stacks on: none. The companion
/server/zod-schemassubpath PR re-exports the raw constants for consumers who need the Zod object directly.