Skip to content

[comparison] specTypeSchema/isSpecType as string-arg functions#1979

Open
felixweinberger wants to merge 16 commits intofweinberger/spec-type-predicatesfrom
fweinberger/spec-type-predicates-fn
Open

[comparison] specTypeSchema/isSpecType as string-arg functions#1979
felixweinberger wants to merge 16 commits intofweinberger/spec-type-predicatesfrom
fweinberger/spec-type-predicates-fn

Conversation

@felixweinberger
Copy link
Copy Markdown
Contributor

Alternative API shape for #1887 using isSpecType('TypeName', value) and specTypeSchema('TypeName') instead of Record property access. For side-by-side comparison; targets the #1887 branch so the diff shows only the API-shape delta.

felixweinberger and others added 16 commits April 27, 2026 12:37
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.
- 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.
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[].
@felixweinberger felixweinberger requested a review from a team as a code owner April 29, 2026 15:36
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 29, 2026

⚠️ No Changeset found

Latest commit: 8f3d23b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

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

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

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Apr 29, 2026

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@1979

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@1979

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@1979

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@1979

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@1979

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@1979

commit: 8f3d23b

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.

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 (specTypeSchemasspecTypeSchema) 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 felixweinberger force-pushed the fweinberger/spec-type-predicates branch from 42062b3 to 7e199ae Compare April 29, 2026 16:46
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