v2 backwards compat: specTypeSchema exported - synchronous StandardSchemaV1 - #2047
Conversation
|
| Name | Type |
|---|---|
| @modelcontextprotocol/client | Minor |
| @modelcontextprotocol/server | Minor |
| @modelcontextprotocol/express | Major |
| @modelcontextprotocol/fastify | Major |
| @modelcontextprotocol/hono | Major |
| @modelcontextprotocol/node | Major |
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
@modelcontextprotocol/client
@modelcontextprotocol/server
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
…wing Replace intersection-based StandardSchemaV1Sync with interface-extends approach so TypeScript properly narrows validate() return type. Drop async/await from examples and migration docs to match the synchronous API this PR introduces.
There was a problem hiding this comment.
Thanks for addressing the earlier feedback — LGTM. The two remaining inline notes are non-blocking polish (vestigial test casts and the InferInput/InferOutput namespace aliases); fine to take or leave.
Extended reasoning...
Overview
This PR adds a StandardSchemaV1Sync interface (a strict subtype of StandardSchemaV1 whose validate is guaranteed to return synchronously) and retypes specTypeSchemas to use it. After my first-round review on commit 34bccc9, commit 5bdaf6c updated the changeset prose, the .examples.ts snippet, the synced JSDoc @example, and both migration docs to drop await and reference StandardSchemaV1Sync — both prior nits are fully addressed.
Security risks
None. This is a type-level narrowing with zero runtime change: _specTypeSchemas is still populated with the same Zod schemas and frozen the same way. No auth, transport, parsing, or wire behavior is touched.
Level of scrutiny
Low–medium. The only question is soundness of the narrowing, and it holds: StandardSchemaV1Sync extends StandardSchemaV1 and only narrows the return type of validate (covariant position), so every specTypeSchemas entry remains assignable wherever StandardSchemaV1 is expected. The runtime claim is also valid — every entry is a plain Zod spec schema with no async refinements, so ~standard.validate always returns synchronously and the as SchemaRecord cast is sound. The new public type export has a concrete in-tree consumer (SchemaRecord) and is now documented in the changeset and migration guides, satisfying the repo's "every new export is intentional" bar.
Other factors
The two new inline nits (leftover as { issues?: ... } casts in specTypeSchema.test.ts, and the missing InferInput/InferOutput aliases in the StandardSchemaV1Sync namespace) are consistency/cleanup items that don't affect correctness, the published API shape, or backwards compatibility. They can be folded in here or taken as a follow-up.
…rInput/InferOutput aliases
- Remove unnecessary `as { issues?: ... }` casts in specTypeSchema.test.ts
that were workarounds for the old async validate return type. The bare
`result.issues` access now serves as compile-time regression coverage
for the StandardSchemaV1Sync narrowing.
- Add InferInput/InferOutput convenience aliases to the StandardSchemaV1Sync
namespace, matching the pattern of all sibling Standard* namespaces.
|
I'm a bit confused. If we dont |
Yes, we can However, imagine the migration path. Someone is having a non-async method. All of a sudden they have to turn it into an Since we know that all of our existing schemas are It will be important though to not add any async stuff like |
There was a problem hiding this comment.
LGTM — thanks for the JSDoc tightening in db32ed5; all earlier nits are addressed and the type narrowing looks correct.
Extended reasoning...
Overview
This PR introduces StandardSchemaV1Sync, a strict subtype of StandardSchemaV1 whose validate returns Result<Output> synchronously (no | Promise<...> branch), and retypes specTypeSchemas entries to use it. Touches: util/standardSchema.ts (new interface + namespace following the established 4-sibling pattern, with InferInput/InferOutput aliases), types/specTypeSchema.ts (SchemaRecord narrowed), exports/public/index.ts (new type export, re-exported via export * in client/server barrels), plus docs/examples/tests/changeset updated to drop await and reference the sync type.
Security risks
None. This is a pure type-level change with zero runtime delta — no new code paths, no input handling, no auth/transport surface.
Level of scrutiny
Low–medium. The only substantive question is the soundness of the pre-existing as SchemaRecord cast now that it narrows to a sync-only signature. I verified in earlier review rounds that none of the backing schemas in schemas.ts / shared/auth.ts use async refinements/transforms, and the JSDoc now documents that invariant explicitly (db32ed5). The new export is minimal, follows the file's established Standard* interface+namespace pattern, and is needed so consumers can name the type of specTypeSchemas entries. StandardSchemaV1Sync extends StandardSchemaV1, so this is non-breaking.
Other factors
I raised five nits across three review rounds (stale docs/examples, stale changeset prose, vestigial test casts, missing InferInput/InferOutput aliases, over-broad JSDoc premise) — all were addressed in 5bdaf6c, 348d3a2, and db32ed5. The test file now accesses .issues without casts, giving type-level regression coverage for the sync guarantee. The feature being amended (specTypeSchemas) is itself unreleased (pre-mode changeset not yet consumed), so editing its changeset in place was the right call. A reviewer question about "why not just await everywhere" was answered with the migration-path rationale (avoiding forced async coloring of consumer call stacks). No bugs found by the bug-hunting system.
2 participants
Add
StandardSchemaV1Synctype sospecTypeSchemasentries expose synchronousvalidateMotivation and Context
specTypeSchemasexposes every MCP spec schema as aStandardSchemaV1. TheStandardSchemaV1interface definesvalidateas returningResult<Output> | Promise<Result<Output>>, because the Standard Schema spec supports async validation libraries. However, every entry inspecTypeSchemasis backed by Zod, which always validates synchronously -- thePromisevariant is never returned at runtime.This causes a real problem for consumers: calling
specTypeSchemas.X['~standard'].validate(v)and then accessing.issuesor.valueon the result produces a type error, because TypeScript thinks the result might be aPromise.How Has This Been Tested?
pnpm typecheck:all-- all packages typecheck cleanlypnpm test:all-- all 1532 tests pass across all packagesBreaking Changes
No.
StandardSchemaV1SyncextendsStandardSchemaV1, sospecTypeSchemasentries remain assignable anywhere aStandardSchemaV1is expected. The change only narrows the return type ofvalidatefromResult | Promise<Result>toResult.Types of changes
Checklist
Additional context
Three files changed (types only, no runtime changes):
packages/core/src/util/standardSchema.ts-- definesStandardSchemaV1Sync, which extendsStandardSchemaV1but narrowsvalidateto returnStandardSchemaV1.Result<Output>(noPromisevariant)packages/core/src/types/specTypeSchema.ts--SchemaRecordnow usesStandardSchemaV1Syncinstead ofStandardSchemaV1packages/core/src/exports/public/index.ts-- exportsStandardSchemaV1Syncso consumers can reference the typeAPIs that accept user-provided schemas (e.g.
McpServer.tool()) still useStandardSchemaV1/StandardSchemaWithJSON, so async validation libraries remain fully supported.