-
Notifications
You must be signed in to change notification settings - Fork 181
MCP Notifications Support for Live Tool Updates #12384
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
Conversation
|
|
||
| // Read from notificationStream and yield events until the tool is done. | ||
| let toolDone = false; | ||
| while (!toolDone) { |
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.
you rely on the timeout above to exit the loop if the tool never ends ?
is the toolPromise resolved when the timeout is reached or does it raise an error or something ?
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.
If the tools never ends, it works like before. If timeout is reached, then it raises an McpError that will be caught by the surrounding try/catch block.
Here is the Promise.race() is used to either yield the notification or the result based on the first one that resolves.
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.
I understand but if you have an error, how does the while(toolDone) behave ?
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.
There are two scenarios here:
- Looking at the return type of
callToolit returns an object withisError. This one will properly exit the loop from thetoolPromise.then(() => MCP_TOOL_DONE_EVENT_NAME),, callToolcan also throw an error like it does when the timeout is reached. Here is what will happen:
toolPromisewill reject with a timeout error- The Promise.race() will reject with that same error
- The error will propagate out of the while loop
- The outer try/catch will catch the error
|
|
||
| // If the tool completed, break from the loop and stop reading notifications. | ||
| if (notificationOrDone === MCP_TOOL_DONE_EVENT_NAME) { | ||
| toolDone = true; |
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.
i find it a bit convoluted to have the tool promise to turn into a string to turn into a boolean. Can't you make it a bit more simple ?
Fraggle
left a comment
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.
My main concerns is the handling of the potential infinite loop.
* MCP Notifications Foundations * Add notifications in image_generation and reasoning tools * ✨ * ✂️ * Address comments from review * 👕 * ✨ * Fix types + tentative fix linter * Fix TS peer dependencies * :exploded_brain: * 🙈 * ✨
Description
The Model Context Protocol was designed to facilitate tool interactions in a standardized way. However, our current implementation only displays results once a tool has completed execution, which can lead to a degraded user experience, especially for long-running tools that may take several minutes to complete.
Previously, with Dust Apps, we could stream reasoning tokens as they were generated, providing immediate feedback to users. The introduction of tools changed this paradigm, creating a gap in user feedback during tool execution.
Implementation
This PR introduces foundational support for MCP notifications, enabling server-to-client intermediate updates during tool execution. We leverage the
notifications/progressmethod from the MCP specification, which provides a structured way to communicate progress updates.The notification system requires:
We temporary hack the
progressTokenused to ensure only active requests can get progress notification, since it's not yet available in theToolCallback. The PR was recently merged and the SDK has not been published yet. This logic will be removed once it's available (PR modelcontextprotocol/typescript-sdk#328).For example, an image generation tool can now provide:
Dependencies
We've updated to the latest MCP SDK which includes proper notification support and fixes the ESM issues reported in modelcontextprotocol/typescript-sdk#217.
Future Work
While this PR establishes the basic infrastructure, the current implementation prioritizes functionality over type safety. A follow-up PR will introduce proper TypeScript types and wrappers around the notification system.
The decision to use a subset of our output types rather than the complete
MCPToolResultContentTypeallows tools to send partial results during execution, which is essential for progressive updates. This approach maintains flexibility while ensuring consistent structure in notifications.Some refactoring is required around the
AgentMessagecomponent to handle notification as part of the streaming message.Tests
Risk
Deploy Plan