Skip to content

fix(server): preserve explicit tool listChanged capability - #2625

Open
LizunovSergey wants to merge 3 commits into
modelcontextprotocol:v1.xfrom
LizunovSergey:fix/v1-listchanged-capability
Open

LizunovSergey wants to merge 3 commits into
modelcontextprotocol:v1.xfrom
LizunovSergey:fix/v1-listchanged-capability

Conversation

@LizunovSergey

@LizunovSergey LizunovSergey commented Aug 8, 2026 •

Copy link
Copy Markdown

Summary

  • preserve an explicitly configured tools.listChanged: false value when the first tool is registered
  • expose the server's current capabilities so McpServer can merge its default without overwriting caller configuration
  • keep the existing listChanged: true default when no value is provided
  • skip notifications/tools/list_changed in McpServer.sendToolListChanged() when the declared tools.listChanged is false, so registration, update and removal do not send the notification the server advertises as unsupported

Root cause

McpServer.setToolRequestHandlers() unconditionally registered tools.listChanged: true, and the capability merge allowed that internal default to overwrite the constructor-provided value. The v2 server already reads the current capability before applying the default; this brings the same tool behavior to v1.x.

Closes #2622.

Validation

  • npx vitest run test/server/mcp.test.ts (252 passed, including a wire-level test that tools.listChanged: false sends no notification, in both Zod variants)
  • npm run typecheck
  • npm run lint

AI assistance disclosure: Codex was used to help investigate and implement this change. I reviewed the diff and validated the code and tests locally.

@changeset-bot

changeset-bot Bot commented Aug 8, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: c879eb4

The changes in this PR will be included in the next version bump.

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

@pkg-pr-new

pkg-pr-new Bot commented Aug 8, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@modelcontextprotocol/sdk@2625

commit: c879eb4

@LizunovSergey
LizunovSergey marked this pull request as ready for review August 8, 2026 19:02
@LizunovSergey
LizunovSergey requested a review from a team as a code owner August 8, 2026 19:02
@claude claude Bot added the v1 Issues / PRs related to v1.x label Aug 18, 2026
@LizunovSergey
LizunovSergey force-pushed the fix/v1-listchanged-capability branch from 15cf0fa to 0ef1663 Compare August 25, 2026 21:51
@foma-agent

Copy link
Copy Markdown

Exact head 3176d340829d6670dd9439883a11cec58c651ad1 gets the handshake half right, but still sends the notification it advertises as unsupported.

I added a wire-level test in a detached worktree. The PR's two capability tests pass for both Zod versions (4 passed). The new test is 2 failed (Zod v3/v4): after client.getServerCapabilities()?.tools?.listChanged === false, calling mcpServer.sendToolListChanged() delivers notifications/tools/list_changed once.

npm test -- test/server/mcp.test.ts -t 'should suppress sendToolListChanged notifications when tools.listChanged is explicitly false'
Test Files  1 failed (1)
Tests       2 failed | 236 skipped (238)
Received: { method: "notifications/tools/list_changed" }
Number of calls: 1

The write at src/server/mcp.ts:134 only changes the initialize response. McpServer.sendToolListChanged() at src/server/mcp.ts:1198 checks isConnected() and then calls the low-level sender unconditionally. Registration/update/removal use that same helper, so they inherit the mismatch.

Please gate the public helper on the declared tools listChanged value and keep the wire test. I made no production change.

@LizunovSergey
LizunovSergey force-pushed the fix/v1-listchanged-capability branch from 3176d34 to c879eb4 Compare November 2, 2026 20:42
@LizunovSergey

Copy link
Copy Markdown
Author

Confirmed: on 3176d34, sendToolListChanged() delivered notifications/tools/list_changed once after the client saw tools.listChanged === false.

McpServer.sendToolListChanged() now skips the send when the declared tools.listChanged is false, so registration, update and removal stay silent too. I added your wire test (it runs in both Zod variants) and rebased onto current v1.x. The low-level Server assertion still checks only that tools exists, the same as for resources and prompts, so a direct server.sendToolListChanged() call is unchanged.

@foma-agent

Copy link
Copy Markdown

McpServer.sendToolListChanged() now suppresses correctly, but the same high-level instance still exposes a public sender that crosses the wire.

On exact head c879eb4fc331b136020d1ebf2b64d71ab56c6a51, I added one disposable test after the new wrapper/automatic-mutation test. It connects with tools.listChanged: false, confirms the client saw false, then calls await mcpServer.server.sendToolListChanged().

npm test -- test/server/mcp.test.ts -t 'should suppress underlying Server tool list notifications when tools.listChanged is explicitly false'
Test Files  1 failed (1)
Tests       2 failed | 252 skipped (254)
Expected length: 0
Received length: 1

McpServer.server is public, and Server.assertNotificationCapability() checks only that tools exists. Preserving false therefore makes the high-level instance advertise that tool-list notifications are unsupported while its exposed Server can still send one.

Please gate that underlying public path as well, or otherwise prevent it from contradicting the handshake. I made no production change; the disposable test is removed and the worktree is clean.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v1 Issues / PRs related to v1.x

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants