-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
Feat/mcp progress tracking #11192
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?
Feat/mcp progress tracking #11192
Conversation
…arding - Subscribe to MCP protocol progress notifications in MCPConnection - Implement direct SSE emission of progress events from MCP service - Handle progress delta events in client-side useStepHandler - Pass progressToken in MCP tool call requests for event correlation - Clean up progress tracking on tool completion and errors
… to ToolCallDelta
|
Thanks for the PR! While I haven't looked deeply at the changes, I think the UI needs more polish, we will likely use this PR for reference only @dustinhealy @berry-13 |
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.
Pull request overview
This PR implements real-time progress tracking for MCP (Model Context Protocol) tool calls following the MCP specification. It adds comprehensive progress monitoring infrastructure that spans from backend event handling through SSE communication to frontend UI display.
Key changes include:
- Added progress notification subscriptions in MCP connections with event forwarding through MCPManager
- Implemented client-side progress state management using Jotai atoms and SSE event handlers
- Created a new MCPProgress React component to display real-time progress bars with percentage and status messages
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
packages/api/src/mcp/connection.ts |
Added subscription to MCP progress notifications and event emission |
packages/api/src/mcp/MCPManager.ts |
Added EventEmitter delegation, progress token management, and progress event forwarding |
packages/api/src/mcp/parsers.ts |
Extended return type to include isError flag in formatted content results |
packages/api/src/mcp/types/index.ts |
Updated FormattedContentResult type to include optional isError boolean |
api/server/services/MCP.js |
Added progress listener setup, SSE event emission, and error status handling with ToolMessage |
packages/data-provider/src/types/agents.ts |
Added progress-related fields (progressToken, serverName, toolName, progress, total, message) and isError flag |
packages/data-provider/src/types/assistants.ts |
Added isError field to FunctionToolCall type |
client/src/store/mcp.ts |
Created progress and active state atoms with atomFamily selectors for per-conversation tracking |
client/src/hooks/SSE/useStepHandler.ts |
Added handling for progress delta events and MCP active state management |
client/src/components/Chat/Messages/Content/MCPProgress.tsx |
New component displaying progress bars with percentage, server/tool names, and status messages |
client/src/components/Chat/Messages/Content/ToolCall.tsx |
Integrated MCPProgress component display during active tool calls |
client/src/components/Chat/Messages/Content/Part.tsx |
Added conversationId and isError props propagation to ToolCall components |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (connection && progressListener) { | ||
| connection.off('progress', progressListener); |
Copilot
AI
Jan 5, 2026
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.
The progress listener cleanup happens before the progressTokens map is cleared, but the cleanup only checks if connection exists, not if connection is still valid. If the connection fails or throws in the try block before being assigned, the cleanup will be skipped but the progressTokens entry will still be deleted. Consider checking progressListener existence alone or restructuring to ensure cleanup always happens.
| if (connection && progressListener) { | |
| connection.off('progress', progressListener); | |
| if (progressListener) { | |
| connection?.off('progress', progressListener); |
| this.emit('progress', { | ||
| serverName: this.serverName, | ||
| progressToken: notification.params?.progressToken, | ||
| progress: notification.params?.progress, | ||
| total: notification.params?.total, | ||
| message: notification.params?.message, |
Copilot
AI
Jan 5, 2026
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.
The progress notification handler doesn't validate that the parameters exist before emitting them. If a progress notification arrives with missing or undefined values for progressToken, progress, total, or message, they will be emitted as undefined. Consider adding validation or default values to ensure consistent event payloads.
| this.emit('progress', { | |
| serverName: this.serverName, | |
| progressToken: notification.params?.progressToken, | |
| progress: notification.params?.progress, | |
| total: notification.params?.total, | |
| message: notification.params?.message, | |
| const params = notification.params; | |
| if (!params) { | |
| logger.warn(`${this.getLogPrefix()} Received progress notification without params`); | |
| return; | |
| } | |
| const safeProgressToken = params.progressToken ?? null; | |
| const safeProgress = | |
| typeof params.progress === 'number' ? params.progress : 0; | |
| const safeTotal = | |
| typeof params.total === 'number' ? params.total : 0; | |
| const safeMessage = | |
| typeof params.message === 'string' ? params.message : ''; | |
| this.emit('progress', { | |
| serverName: this.serverName, | |
| progressToken: safeProgressToken, | |
| progress: safeProgress, | |
| total: safeTotal, | |
| message: safeMessage, |
| } | ||
|
|
||
| const { progress, total, message } = latestProgress; | ||
| const percentage = total ? Math.round((progress / total) * 100) : Math.round(progress * 100); |
Copilot
AI
Jan 5, 2026
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.
The percentage calculation could produce values greater than 100% if progress exceeds total, or if progress is a value greater than 1.0 when total is undefined. Consider clamping the percentage between 0 and 100 to ensure valid display values.
| const percentage = total ? Math.round((progress / total) * 100) : Math.round(progress * 100); | |
| const rawPercentage = total ? Math.round((progress / total) * 100) : Math.round(progress * 100); | |
| const percentage = Math.max(0, Math.min(100, rawPercentage)); |
| } | ||
|
|
||
| // Set up progress event listener | ||
| progressListener = (progressData: any) => { |
Copilot
AI
Jan 5, 2026
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.
Using any type reduces type safety. Consider defining a proper type for progressData that matches the event payload structure, such as creating an interface with fields like progressToken, serverName, toolName, userId, progress, total, and message.
| */ | ||
| export const mcpProgressAtom = atom<Record<string, MCPProgress[]>>({}); | ||
|
|
||
| /** |
Copilot
AI
Jan 5, 2026
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.
The MCP progress state stored in mcpProgressAtom is never cleaned up. Progress data accumulates in the atom for each conversation without any cleanup logic. Consider adding cleanup when conversations are deleted or when progress is completed, to prevent unbounded memory growth over time.
| /** | |
| /** | |
| * Write-only helper atom to clear MCP progress for a single conversation. | |
| * This should be used when a conversation is deleted or when its progress | |
| * data is no longer needed, to avoid unbounded growth of mcpProgressAtom. | |
| */ | |
| export const clearMCPProgressForConversationAtom = atom( | |
| null, | |
| (get, set, conversationId: string) => { | |
| const current = get(mcpProgressAtom); | |
| if (!current[conversationId]) { | |
| return; | |
| } | |
| const { [conversationId]: _removed, ...rest } = current; | |
| set(mcpProgressAtom, rest); | |
| }, | |
| ); | |
| /** | |
| * Write-only helper atom to clear all MCP progress data. | |
| * This can be used on global resets (e.g., logout) to free memory. | |
| */ | |
| export const clearAllMCPProgressAtom = atom(null, (_get, set) => { | |
| set(mcpProgressAtom, {}); | |
| }); | |
| /** |
| // Clear active state for this conversation when tool completes | ||
| const conversationId = | ||
| submission?.initialResponse?.conversationId || userMessage.conversationId; | ||
| if (conversationId) { | ||
| setMcpActive((current) => ({ | ||
| ...current, | ||
| [conversationId]: false, | ||
| })); | ||
| } |
Copilot
AI
Jan 5, 2026
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.
The mcpActiveAtom is set to false on tool completion, but the accumulated progress data in mcpProgressAtom for that conversation is not cleared. This means old progress entries remain in memory. Consider clearing the progress data for this conversation when setting active to false.
| if (!RECOGNIZED_PROVIDERS.has(provider)) { | ||
| return [parseAsString(result), undefined]; |
Copilot
AI
Jan 5, 2026
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.
When returning early for unrecognized providers, the isError flag is not propagated to the result tuple. The function should return the 3-element tuple consistently, including the isError flag extracted on line 96.
| attachments, | ||
| auth, | ||
| conversationId, | ||
| isError: toolIsError, |
Copilot
AI
Jan 5, 2026
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.
The isError parameter (destructured as toolIsError) is received but never used in the component. If this flag is meant to indicate an error state, it should be incorporated into the error handling logic or UI display. Consider using it to determine the cancelled state or to display error-specific styling.
Summary
Add MCP Progress Tracking Support
This PR implements real-time progress tracking for MCP (Model Context Protocol) tool calls, following the https://modelcontextprotocol.info/specification/draft/basic/utilities/progress/.
Changes
Files Modified
api/server/services/MCP.jsclient/src/components/Chat/Messages/Content/MCPProgress.tsxclient/src/components/Chat/Messages/Content/Part.tsxclient/src/components/Chat/Messages/Content/ToolCall.tsxclient/src/hooks/SSE/useStepHandler.tsclient/src/store/mcp.tspackages/api/src/mcp/MCPManager.tspackages/api/src/mcp/connection.tspackages/api/src/mcp/parsers.tspackages/api/src/mcp/types/index.tspackages/data-provider/src/types/agents.tspackages/data-provider/src/types/assistants.tsChange Type
Please delete any irrelevant options.
Testing
Screenshots
Checklist
Please delete any irrelevant options.
I would like to thank @existme who did the initial version