Skip to content

test: add compile-time key-parity assertions for spec type checks#1652

Merged
felixweinberger merged 11 commits intomodelcontextprotocol:mainfrom
rechedev9:test/spec-type-key-parity-assertions
Mar 30, 2026
Merged

test: add compile-time key-parity assertions for spec type checks#1652
felixweinberger merged 11 commits intomodelcontextprotocol:mainfrom
rechedev9:test/spec-type-key-parity-assertions

Conversation

@rechedev9
Copy link
Copy Markdown
Contributor

Summary

  • Adds 151 compile-time key-parity assertions to spec.types.test.ts that catch missing/extra .optional() fields between SDK and spec types
  • Introduces 3 utility types (KnownKeys<T>, AssertExactKeys<A,B>, Assert<T>) that produce descriptive compile errors on key-set mismatches
  • Documents 7 genuine SDK/spec key mismatches with @ts-expect-error annotations (serving as a living drift inventory)
  • Covers 151 of 174 sdkTypeChecks entries; 23 exclusions documented (union types and primitive aliases)

Motivation

Closes #1298.

The existing mutual-assignability checks (sdk = spec; spec = sdk) pass even when one type has optional keys that the other lacks, because TypeScript structural subtyping treats optional properties as compatible in both directions. This means a Zod schema missing .optional() on a field — or having .optional() when the spec requires the field — goes undetected.

The new key-parity layer compares the exact set of named property keys between each SDK/spec type pair at compile time, with zero runtime cost.

Test plan

  • pnpm typecheck:all passes (key-parity assertions validated at compile time)
  • pnpm test:all — 734 tests pass (440 core + 37 server + 257 client)
  • All 7 @ts-expect-error annotations verified as necessary
  • Verified that removing .optional() from a schema correctly triggers a compile error from the new assertions

Add 151 compile-time key-parity assertions to spec.types.test.ts that
verify SDK and spec types have exactly the same set of named property
keys. This catches missing .optional() parameters that the existing
mutual-assignability checks cannot detect due to TypeScript structural
subtyping.

Introduces three utility types:
- KnownKeys<T>: strips index signatures, keeps named keys only
- AssertExactKeys<A, B>: resolves to true on match, descriptive error
  type on mismatch
- Assert<T extends true>: constraint that forces compile error

Coverage: 151 of 174 sdkTypeChecks entries. 23 exclusions documented
(15 union types, 8 primitive aliases).

Discovered 7 genuine SDK/spec key mismatches, documented with
@ts-expect-error annotations as a living drift inventory.

Closes modelcontextprotocol#1298
@rechedev9 rechedev9 requested a review from a team as a code owner March 9, 2026 22:23
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 9, 2026

⚠️ No Changeset found

Latest commit: bec0c76

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 no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

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 Mar 9, 2026

Open in StackBlitz

@modelcontextprotocol/client

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

@modelcontextprotocol/server

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

@modelcontextprotocol/express

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

@modelcontextprotocol/hono

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

@modelcontextprotocol/node

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

commit: bec0c76

@felixweinberger
Copy link
Copy Markdown
Contributor

@claude review

Copy link
Copy Markdown
Contributor

@felixweinberger felixweinberger left a comment

Choose a reason for hiding this comment

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

Thanks for this.

One ask before merging: could you add a completeness guard that ties the _K_ assertions back to sdkTypeChecks? Something like a parallel runtime array:

const keyParityChecked = ['RequestParams', 'NotificationParams', /* ... */] as const;

that the existing should have comprehensive compatibility tests check also validates against (minus the 23 documented exclusions). Otherwise if someone adds a new entry to sdkTypeChecks but forgets the _K_ line, nothing catches the gap.

Happy to merge once that's in.

Ensures every sdkTypeChecks entry (minus excluded union/primitive types)
has a corresponding _K_ compile-time assertion. The checked set is
derived at test time by parsing _K_ type aliases from the file itself,
so adding a new _K_ assertion automatically satisfies the guard.
@rechedev9
Copy link
Copy Markdown
Contributor Author

Added the completeness guard requested in review.

What: A new runtime test (should have key-parity assertions for all non-excluded compatibility tests) that fails when a sdkTypeChecks entry exists without a corresponding type _K_<Name> assertion.

How it works: Instead of maintaining a hardcoded list of checked types, it reads the test file itself at test time and extracts _K_ suffixes via regex — same pattern as the existing extractExportedTypes. This means:

  • Adding a new _K_ assertion automatically satisfies the guard
  • No manual bookkeeping required
  • 23 excluded types (union types + primitive aliases) are listed in KEY_PARITY_EXCLUDED

Verified:

  • Typecheck passes
  • All 441 core tests pass (6 in spec.types.test.ts)
  • Removing a _K_ assertion correctly causes the new test to fail

@felixweinberger
Copy link
Copy Markdown
Contributor

@claude review

@felixweinberger felixweinberger enabled auto-merge (squash) March 30, 2026 20:19
@felixweinberger felixweinberger merged commit a39a9eb into modelcontextprotocol:main Mar 30, 2026
11 checks passed
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.

Make spec type tests catch .optional() missing args too

2 participants