[comparison] specTypeSchema/isSpecType as string-arg functions#1979
[comparison] specTypeSchema/isSpecType as string-arg functions#1979felixweinberger wants to merge 16 commits intofweinberger/spec-type-predicatesfrom
Conversation
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.
…cTypeSchema()/isSpecType()
- Explicit auth-schema allowlist (excludes SafeUrl/OptionalSafeUrl/IdJagTokenExchangeResponse helpers) - Guard predicate types value as schema input (z.input), not output, since safeParse only proves input shape - SchemaRecord typed as StandardSchemaV1<In, Out> so validate() output is the spec type - JSDoc/migration examples await validate() (Result | Promise<Result>); drop stale setCustom* refs
…-checked @example snippets - Replace import-* derivation with explicit SPEC_SCHEMA_KEYS tuple (150 entries with a matching public type in types.ts). Excludes internal helper schemas (ListChangedOptionsBase, BaseRequestParams, NotificationsParams, Client/ServerTasksCapability, ResourceTemplate name mismatch). - Add test asserting internals are not in SpecTypeName. - Bump changeset patch -> minor (5 new public exports). - Move @example code into specTypeSchema.examples.ts and source via sync:snippets, per repo convention.
…emas with defaults The previous 'narrows the value type' test asserted against SpecTypes['Implementation'] (the output type), which only passed because Implementation has no defaults. Clarified that case and added a CallToolResult case that proves the documented input-type narrowing: content is optional in the narrowed type, and the narrowed type is not CallToolResult.
specTypeSchemas.X is typed as StandardSchemaV1, which only exposes ['~standard'].validate. The .parse() method exists at runtime (Zod) but is not on the public type, so the parenthetical produced a TS error if followed.
ResourceTemplate is a first-class spec type with a public type export (ResourceTemplateType). It was excluded under the assumption that the name collides with the server package's ResourceTemplate class, but isSpecType.ResourceTemplate is a record property, not a top-level export, so no collision occurs. Including it makes the migration docs' 'every named type in the MCP spec' claim accurate. Corrected the allowlist comment (ResourceTemplate is not an internal helper) and added type-level and runtime tests.
…ype describe block
JSONValueSchema/JSONObjectSchema/JSONArraySchema were typed as z.ZodType<T> (output only). Zod v4's ZodType<O, I> defaults I to unknown, so z.input<typeof JSONValueSchema> was unknown and isSpecType.JSONValue was (v: unknown) => v is unknown, a no-op predicate. Annotating as z.ZodType<T, T> fixes the input type at the source (these schemas have no defaults/transforms, so input equals output). Test asserts the narrowing.
…arg function form
Replaces the Record-of-predicates exports with two generic functions:
isSpecType('TypeName', value) and specTypeSchema('TypeName').
Drops the register() helper, the per-key closure creation (161 closures
at module init), the SchemaRecord/GuardRecord types, and both
Object.freeze(...) as unknown as casts. The single remaining cast is a
narrow Record<SpecTypeName, z.ZodType> at construction so indexing by
SpecTypeName is non-undefined under noUncheckedIndexedAccess.
Trade-off: arr.filter needs an arrow wrapper (mixed.filter(v =>
isSpecType('ContentBlock', v))). TypeScript's inferred type predicates
still narrow the result to ContentBlock[].
|
| 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}
There was a problem hiding this comment.
No issues spotted in the implementation, but since this PR exists specifically for humans to compare two API shapes (record-keyed vs string-arg) and pick one, I'll leave the design call to a maintainer.
Extended reasoning...
Overview
This PR reshapes the not-yet-released isSpecType / specTypeSchemas API from #1887 into a string-arg function form: isSpecType('TypeName', value) and specTypeSchema('TypeName'). It touches the implementation in packages/core/src/types/specTypeSchema.ts, the public export barrel, the examples file, the test file, both migration docs, and the changeset. The diff targets the #1887 branch, so what's shown is purely the API-shape delta.
Security risks
None. This is a type-predicate / schema-lookup helper over an internal static table of Zod schemas; no I/O, auth, or untrusted-input handling is affected.
Level of scrutiny
The code change itself is small and mechanically correct — the overload signatures, the allSchemas population loop, the docs, tests, and changeset are all internally consistent, and the bug-hunting pass found nothing. However, the PR's stated purpose is "for side-by-side comparison" of two candidate public-API shapes. Per the repo's guiding principles (burden of proof on addition, one way to do things, kill at the highest level), choosing between record-property access and string-arg function is a design decision that a maintainer should make explicitly, not something to rubber-stamp.
Other factors
- The function form loses the standalone-callback ergonomics (
arr.filter(isSpecType.ContentBlock)→arr.filter(v => isSpecType('ContentBlock', v))), which the diff acknowledges by removing the eslint-disable and updating the docs/tests. Whether that trade-off is worth the simpler surface is the question this PR is asking. - The export rename (
specTypeSchemas→specTypeSchema) is a public-API change relative to #1887; whichever shape lands needs to be the one the team commits to before release. - Test coverage mirrors the previous shape and still asserts compile-time rejection of unknown/internal names and correct narrowing, so correctness parity is demonstrated.
felixweinberger
force-pushed
the
fweinberger/spec-type-predicates
branch
from
42062b3 to
7e199ae
Compare
1 participant
Alternative API shape for #1887 using
isSpecType('TypeName', value)andspecTypeSchema('TypeName')instead of Record property access. For side-by-side comparison; targets the #1887 branch so the diff shows only the API-shape delta.