Bundle automatic validator defaults in client/server shims #2088
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| --- | ||
| '@modelcontextprotocol/core': minor | ||
| '@modelcontextprotocol/client': patch | ||
| '@modelcontextprotocol/server': patch | ||
| --- | ||
|
|
||
| Make validator backends symmetrical in core and bundle automatic defaults in client/server runtime shims. | ||
|
|
||
| Core no longer re-exports concrete validator providers as runtime values from the root/public barrels. AJV/AJV formats and `@cfworker/json-schema` are optional peer backends behind explicit core validator provider subpaths, used internally by client/server shims. | ||
|
|
||
| Client/server continue to select defaults automatically: Node shims use AJV, while browser/workerd shims use `@cfworker/json-schema`. Those backends are bundled into the shim chunks that select them, so users do not need to install validator packages or import explicit validators for default behavior. Advanced users can still pass their own `jsonSchemaValidator` implementation. | ||
|
Check warning on line 11 in .changeset/workerd-shim-vendors-cfworker.md
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| import type { JSONRPCMessage, JsonSchemaType, JsonSchemaValidatorResult, jsonSchemaValidator } from '@modelcontextprotocol/core'; | ||
| import { InMemoryTransport, LATEST_PROTOCOL_VERSION } from '@modelcontextprotocol/core'; | ||
| import { Client } from '../../src/client/client.js'; | ||
| import { fromJsonSchema } from '../../src/fromJsonSchema.js'; | ||
|
|
||
| class RecordingValidator implements jsonSchemaValidator { | ||
| schemas: JsonSchemaType[] = []; | ||
| values: unknown[] = []; | ||
|
|
||
| getValidator<T>(schema: JsonSchemaType) { | ||
| this.schemas.push(schema); | ||
| return (value: unknown): JsonSchemaValidatorResult<T> => { | ||
| this.values.push(value); | ||
| return { valid: true, data: value as T, errorMessage: undefined }; | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| async function connectInitializedClient(client: Client) { | ||
| const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); | ||
| serverTransport.onmessage = async message => { | ||
| if ('method' in message && 'id' in message && message.method === 'initialize') { | ||
| await serverTransport.send({ | ||
| jsonrpc: '2.0', | ||
| id: message.id, | ||
| result: { | ||
| protocolVersion: LATEST_PROTOCOL_VERSION, | ||
| capabilities: { tools: {} }, | ||
| serverInfo: { name: 'test-server', version: '1.0.0' } | ||
| } | ||
| }); | ||
| } else if ('method' in message && 'id' in message && message.method === 'tools/list') { | ||
| await serverTransport.send({ | ||
| jsonrpc: '2.0', | ||
| id: message.id, | ||
| result: { | ||
| tools: [ | ||
| { | ||
| name: 'structured-tool', | ||
| description: 'A tool with structured output', | ||
| inputSchema: { type: 'object' }, | ||
| outputSchema: { | ||
| type: 'object', | ||
| properties: { count: { type: 'number' } }, | ||
| required: ['count'] | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| } satisfies JSONRPCMessage); | ||
| } | ||
| }; | ||
|
|
||
| await Promise.all([client.connect(clientTransport), serverTransport.start()]); | ||
| return { clientTransport, serverTransport }; | ||
| } | ||
|
|
||
| describe('client JSON Schema validator overrides', () => { | ||
| test('Client constructor uses a custom validator for tool output schema caching', async () => { | ||
| const validator = new RecordingValidator(); | ||
| const client = new Client( | ||
| { name: 'test-client', version: '1.0.0' }, | ||
| { | ||
| capabilities: {}, | ||
| jsonSchemaValidator: validator | ||
| } | ||
| ); | ||
| const { clientTransport, serverTransport } = await connectInitializedClient(client); | ||
|
|
||
| await expect(client.listTools()).resolves.toMatchObject({ | ||
| tools: [ | ||
| { | ||
| name: 'structured-tool', | ||
| outputSchema: { | ||
| type: 'object', | ||
| properties: { count: { type: 'number' } }, | ||
| required: ['count'] | ||
| } | ||
| } | ||
| ] | ||
| }); | ||
|
|
||
| expect(validator.schemas).toEqual([ | ||
| { | ||
| type: 'object', | ||
| properties: { count: { type: 'number' } }, | ||
| required: ['count'] | ||
| } | ||
| ]); | ||
|
|
||
| await client.close(); | ||
| await clientTransport.close(); | ||
| await serverTransport.close(); | ||
| }); | ||
|
|
||
| test('fromJsonSchema uses an explicitly supplied custom validator', async () => { | ||
| const validator = new RecordingValidator(); | ||
| const schema: JsonSchemaType = { | ||
| type: 'object', | ||
| properties: { name: { type: 'string' } }, | ||
| required: ['name'] | ||
| }; | ||
|
|
||
| const standardSchema = fromJsonSchema<{ name: string }>(schema, validator); | ||
| expect(standardSchema['~standard'].validate({ name: 123 })).toEqual({ value: { name: 123 } }); | ||
|
|
||
| expect(validator.schemas).toEqual([schema]); | ||
| expect(validator.values).toEqual([{ name: 123 }]); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 nit: this changeset describes the core barrel change and the shim bundling, but never says that the
./validators/cf-workersubpath export was removed from@modelcontextprotocol/{client,server}— line 11's "do not need to … import explicit validators" reads as a convenience, not a removal. Worth one explicit sentence here, especially since the open comment onpackages/server/package.json:31suggests deleting.changeset/cfworker-out-of-barrel.md, which would leave this as the only changeset covering the subpath and the removal entirely undocumented in the generated CHANGELOG.Extended reasoning...
What the issue is
This PR removes a public subpath export from two published packages:
./validators/cf-workeris deleted from theexportsandtypesVersionsmaps inpackages/{client,server}/package.json, bothsrc/validators/cfWorker.tsre-export files are deleted, and the entry is dropped from bothtsdown.config.tsarrays. The previously-documented patternimport { CfWorkerJsonSchemaValidator } from '@modelcontextprotocol/server/validators/cf-worker'now resolves toERR_PACKAGE_PATH_NOT_EXPORTED.The PR's own changeset,
.changeset/workerd-shim-vendors-cfworker.md, describes two things: (a) core no longer re-exporting concrete validator providers as runtime values from its barrels, and (b) client/server bundling default backends into their shim chunks. Line 11 says "users do not need to install validator packages or import explicit validators for default behavior" — but that frames explicit imports as no longer necessary, not as no longer possible. Nowhere does the changeset state that the./validators/cf-workersubpath was removed from client/server.Why this isn't covered by existing comments
The still-open inline comment on
packages/server/package.json:31is about other prose that still references the removed subpath (.changeset/cfworker-out-of-barrel.md,ajvProvider.ts:36's@see,.changeset/support-standard-json-schema.md). This finding is the inverse: the PR's own new changeset fails to announce the removal it introduces.The two interact: that open comment's first suggested fix is "delete
cfworker-out-of-barrel.md(its purpose is subsumed byworkerd-shim-vendors-cfworker.md)". If the author follows that suggestion as-is,workerd-shim-vendors-cfworker.mdbecomes the only changeset touching this area — and it doesn't mention the subpath removal, so the generated CHANGELOG would contain no record that@modelcontextprotocol/{client,server}/validators/cf-workerever went away.The resolved comment on
exports/public/index.ts:145did raise the patch-vs-breaking concern, but for the earlierexport typechange. The author's resolution expanded the breaking surface (it removed the subpath entirely instead of adding a parallel one) without revisiting the changeset prose. The bump-level concern itself is muted by2.0.0-alpha.2pre-release status, so the actionable residue is just the missing prose.Step-by-step proof
@modelcontextprotocol/server@2.0.0-alpha.2hasimport { CfWorkerJsonSchemaValidator } from '@modelcontextprotocol/server/validators/cf-worker'— the exact pattern the pre-PRdocs/migration.mdanddocs/migration-SKILL.mddocumented.packages/server/package.jsonno longer has"./validators/cf-worker"inexports; Node throwsERR_PACKAGE_PATH_NOT_EXPORTED.packages/server/CHANGELOG.mdfor that version. The entry fromworkerd-shim-vendors-cfworker.mdreads: "Client/server continue to select defaults automatically … users do not need to install validator packages or import explicit validators for default behavior. Advanced users can still pass their ownjsonSchemaValidatorimplementation.".changeset/cfworker-out-of-barrel.mdis also deleted per the open comment's suggestion, there is no CHANGELOG entry mentioning/validators/cf-workerat all.Impact
Doc-only; no runtime effect. The PR description and updated migration guides do call this out ("Public surface: explicit concrete validator subpaths are removed from client/server"), so the gap is specifically in the changeset → CHANGELOG path. Filed as a nit given the alpha pre-release context and that three related doc nits are already open — but it's a one-line addition while the changeset is being touched anyway.
How to fix
Add one sentence to
.changeset/workerd-shim-vendors-cfworker.md, e.g. after line 11:(If
.changeset/cfworker-out-of-barrel.mdis kept and rewritten per the other open comment instead of deleted, that would also cover it — either location works as long as one changeset states the removal.)