-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add notifications/tools/list_changed notification handling to Client
#1068
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add notifications/tools/list_changed notification handling to Client
#1068
Conversation
| // Reset tool list changed options and remove notification handler | ||
| else { | ||
| this._toolListChangedOptions = null; | ||
| this.removeNotificationHandler(ToolListChangedNotificationSchema.shape.method.value); |
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.
In this section, we should probably clearTimeout on any existing _toolListChangedDebounceTimer and set it to undefined
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.
| this.removeNotificationHandler(ToolListChangedNotificationSchema.shape.method.value); | |
| this.removeNotificationHandler(ToolListChangedNotificationSchema.shape.method.value); | |
| if (this._toolListChangedDebounceTimer) { | |
| clearTimeout(this._toolListChangedDebounceTimer); | |
| this._toolListChangedDebounceTimer) = undefined; | |
| } |
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.
Updated
|
Great work, @chipgpt! I've tested our branch on the Inspector, but I need to adapt the Inspector client code to work with our changes. I also tested it with the dynamic MCP tool. See the changes here: https://github.com/sandros06/inspector/pull/1/files This is still experimental code. What are your thoughts on it? This opens the door to a whole new class of dynamic MCP servers! See on gif: |
@sandros06 I see the pull request in your repo, but can you please summarize what you needed to change and speak to whether this PR should include such changes and why? |
I wanted to integrate it into the “inspector”, and after setting up the options for However, this same handler is used both to display messages in the message list on the inspector and for the tool |
|
Very cool to see it in action in the inspector! I would definitely like to see this implemented in the official MCP inspector (probably needs to be an optional setting in case some people prefer the current behavior where you manually relist the tools). It would be great if you could create a PR for the modelcontextprotocol/inspector repo that implements this SDK update. |
commit: |

This PR iterates on #239 which seems to have stalled. It adds a new, optional field to the
ClientOptionsthat enables a tool list changed notification handler.autoRefreshistrue, then it will reload the tools first and pass them into the callback. If reload fails thenerrorwill be set andtoolswill benull.autoRefreshisfalse, thenerrorandtoolswill always benull.Motivation and Context
This update will allow clients to utilize the dynamic aspect of MCP by reloading tools when the server sends change notifications. Addresses issue #205
How Has This Been Tested?
I have added tests for the new behavior. It has not been tested in a real application.
Breaking Changes
No breaking changes. The new field is optional.
Types of changes
Checklist
Additional context