refactor(specTypeSchema): drop as unknown as casts; add allowlist drift guard#1993
Conversation
…dRecord exports Retyping the staging record from `Record<string, z.ZodTypeAny>` to `Record<string, StandardSchemaV1>` gives TS overlap with `SchemaRecord` (every `SchemaRecord[K]` is a `StandardSchemaV1`, so the cast direction is a permitted narrowing). The guard record already overlapped because `(v) => v is T` is a subtype of `(v) => boolean`. Both exports are now single `as` casts inside `Object.freeze()` instead of double-casts.
…exports Compares `Object.keys(isSpecType)` (protocol entries only) against the PascalCase `*Schema` exports of schemas.ts minus the documented internal helpers. A new schema added to schemas.ts now fails this test until it is either added to SPEC_SCHEMA_KEYS or to INTERNAL_HELPER_SCHEMAS.
|
@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.
LGTM — type-annotation cleanup with no runtime change, plus a drift-guard test that passes against current schemas.ts.
Extended reasoning...
Overview
Two-part follow-up to #1887 review feedback:
specTypeSchema.ts(4 lines): retypes_specTypeSchemasasRecord<string, StandardSchemaV1>andregister's param asz.ZodType, so the freeze-site casts become single-stepas SchemaRecord/as GuardRecordinstead ofas unknown as. The intermediate record's value type is now a supertype of the target mapped type's values, which is what makes the direct cast legal.specTypeSchema.test.ts: adds a drift-guard test assertingObject.keys(isSpecType)(minus auth entries) equals every PascalCase*Schemaexport fromschemas.tsminus the 5 documented internal helpers.
Security risks
None. No I/O, no auth, no transport code — pure type annotations and a test. No new exports or public-API surface change.
Level of scrutiny
Low. The src change is type-annotation-only; register() still calls schema.safeParse(v) on a z.ZodType, and Zod v4 types implement StandardSchemaV1, so the assignment into the record is sound. The exported specTypeSchemas / isSpecType values and their public types (SchemaRecord / GuardRecord) are unchanged — only the path the compiler takes to accept the cast differs. Runtime behavior is byte-identical.
Other factors
- Ran
viteston the test file: 18/18 pass including the new drift guard, confirmingSPEC_SCHEMA_KEYSis currently in sync withschemas.ts. - Verified
schemas.tsexports noOAuth*/OpenId*names, so theactual-side filter (!k.startsWith('OAuth') && !k.startsWith('OpenId')) cannot accidentally drop a protocol schema and the two sides of the comparison are drawn from disjoint sources as intended. - The
INTERNAL_HELPER_SCHEMASlist in the test mirrors the exclusion comment inspecTypeSchema.tsexactly (5 entries). - No changeset is appropriate: internal refactor + test only, no consumer-visible change.
2 participants
Follow-ups from #1887 review (#1887 (review)):
_specTypeSchemasasRecord<string, StandardSchemaV1>so the freeze-site cast is a singleas SchemaRecordinstead ofas unknown as. Same for_isSpecType/GuardRecord.Object.keys(isSpecType)(minus auth schemas) must equal every PascalCase*Schemaexport fromschemas.tsminus the 5 documented internal helpers. A new schema added toschemas.tswill fail this test until it is allowlisted.Breaking Changes
None.
Types of changes