fix(client): keep resultType through decode so skills/directory results validate - #2791
Draft
tiagovilasboas wants to merge 4 commits into
Conversation
decodeResult() already checks resultType === "complete" and strips it as part of complete-result lifting. Caller schemas that still model the 2026 wire envelope (Inspector skills/list, skills/get, and resources/directory/read) then reject the lifted object because resultType is gone. Retry the caller/registry schema with the already-validated discriminator restored only when the lifted object fails, so post-lift schemas that omit the field keep working unchanged. Fixes modelcontextprotocol#2789 Co-authored-by: Tiago Vilas Boas <tcarvalhovb@gmail.com>
…sults Regression coverage for modelcontextprotocol#2789: skills/list, skills/get, and resources/directory/read must succeed when the server sends a resultType: "complete" payload and the caller schema still requires that discriminator (the Inspector Modern* envelopes). Co-authored-by: Tiago Vilas Boas <tcarvalhovb@gmail.com>
🦋 Changeset detectedLatest commit: 2326041 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Keep eslint import/no-duplicates and Prettier happy on the modelcontextprotocol#2789 validation helper and its protocol-level regression. Co-authored-by: Tiago Vilas Boas <tcarvalhovb@gmail.com>
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/core
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
Record the modelcontextprotocol#2789 Client.request() skills/directory validation fix as a patch on @modelcontextprotocol/client. Co-authored-by: Tiago Vilas Boas <tcarvalhovb@gmail.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Problem
Client.request()rejects every spec-conformingskills/list,skills/get, andresources/directory/readresult with:even when the server sent
resultType: "complete".decodeResult()validates the 2026 discriminator and strips it as part of complete-result lifting. Caller schemas that still model the wire envelope (InspectorModernListSkillsResultSchema,ModernGetSkillEnvelopeSchema,ModernDirectoryReadResultSchema) then re-requireresultType: "complete"against the already-lifted object. No server payload can pass.Core list methods (
tools/list,resources/list, …) are unaffected because their post-lift schemas omitresultType.Solution
Keep codec complete-result lifting intact. After the lifted object fails the caller/registry schema, retry once with the already-validated
resultType: "complete"restored. Post-lift schemas that omit the field (core list methods, strictEmptyResult) still succeed on the first pass.Test plan
skills/listaccepts a spec-conforming{ resultType: "complete", ttlMs, cacheScope, skills }payloadskills/getaccepts a spec-conforming{ resultType: "complete", skill }payloadresources/directory/readaccepts a spec-conforming{ resultType: "complete", resources }payloadskills/listbody still fails after the discriminator is restoredpnpm --filter @modelcontextprotocol/core-internal exec vitest run test/shared/extensionResultType.test.ts test/shared/rawResultTypeFirst.test.ts— 15/15 passedpnpm --filter @modelcontextprotocol/client exec vitest run test/client/skillsDirectoryResultType.test.ts— 3/3 passed@modelcontextprotocol/core-internalsuite — 1461 passed@modelcontextprotocol/clientsuite — 890 passedFixes #2789