fix(server,client): non-SEP draft spec conformance (eager list handlers, pagination docs, path-sanitization note)#2269
Open
mattzcarey wants to merge 1 commit into
Open
Conversation
…rs, pagination docs, path-sanitization note) - McpServer eagerly installs tools/resources/prompts handlers for capabilities declared in ServerOptions.capabilities, so declared-but-empty capabilities answer their list methods with empty results instead of -32601 (draft spec MUST) - Fix pagination doc examples to loop while (cursor !== undefined) so an empty-string cursor is not treated as end-of-results (draft spec MUST NOT) - Add path-sanitization security note to docs/server.md resources section - Add regression tests for deterministic tools/list ordering (already compliant; insertion order) - Completion 100-cap + hasMore already implemented and covered by e2e tests Closes #2202
🦋 Changeset detectedLatest commit: f1377e9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
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.
Implements the non-SEP changes from the 2025-11-25 → draft spec diff tracked in #2202. Per-item breakdown:
(a) Deterministic
tools/listordering — already compliant, tests addedMcpServerlists tools viaObject.entries(this._registeredTools), which is stable insertion order — no code change needed. Added regression tests (identical order across repeatedtools/listcalls, and stability acrossdisable()/enable()toggles, including that re-enabling restores insertion position rather than appending) plus a short comment on the list handler documenting the guarantee.(b) List responses when capability declared — the only behavioral change
Previously,
McpServerinstalled list/call handlers lazily on firstregisterTool/registerResource/registerPrompt, so a server constructed withcapabilities: { tools: {} }and zero registrations answeredtools/listwith-32601(Method not found). The constructor now eagerly installs handlers for every primitive capability declared inServerOptions.capabilities:tools→tools/listreturns[];tools/callfor an unknown tool returns-32602resources→resources/list/resources/templates/listreturn[];resources/readreturns resource-not-foundprompts→prompts/listreturns[];prompts/getreturns-32602Undeclared capabilities with no registrations still return
-32601, and lazy installation on first registration is unchanged for servers that don't declare capabilities up front. Low-levelServerusers remain responsible for their own handlers — documented onServerOptions.capabilities.Happy to split this item into its own PR if you'd prefer to keep behavioral and docs-only changes separate.
(c) Empty-string cursor — doc examples fixed
The SDK has no runtime pagination loop, but the pagination examples in
client.examples.ts(and the JSDoc onlistTools/listPrompts/listResourcesgenerated from them viasync:snippets) used} while (cursor);, which stops on an empty-string cursor. Fixed towhile (cursor !== undefined)and re-ranpnpm sync:snippets.(d) Resource path sanitization — docs only
The SDK serves no files itself; added a security note to the Resources section of
docs/server.mdabout sanitizing user-influenced paths (traversal sequences, encoded forms, symlink escapes).(e) Completion value caps — already compliant, already tested
createCompletionResultinmcp.tsalready slices to 100 and setshasMore/total, andtest/e2e/scenarios/completion.test.ts(requirementcompletion:result-shape, 150-item fixture) already asserts the 100-cap,total, andhasMore: true. No change needed.Notes
packages/server/src/server/mcp.tsnear regions feat(core,server,client): implement SEP-2106 (tool schemas conform to JSON Schema 2020-12) #2249 (SEP-2106) also modifies; edits here are deliberately minimal and self-contained, but this PR may need a trivial rebase after feat(core,server,client): implement SEP-2106 (tool schemas conform to JSON Schema 2020-12) #2249 merges (or vice versa).MCPAgentalways registers primitives before connect, BUT agents has the same empty-string-cursor consumer bug inpackages/agents/src/mcp/client-connection.ts(~L562-617,while (result.nextCursor)) — an agents-side fix issue should be filed.Validation
pnpm build:all,pnpm typecheck:all,pnpm lint:all(includessync:snippets --check): passtest/integration/test/server/declaredCapabilities.test.ts)@modelcontextprotocol/serverand@modelcontextprotocol/clientCloses #2202