Skip to content

Conversation

@changkun
Copy link

@changkun changkun commented Jan 25, 2026

Fixes #8221

Summary

This PR adds real-time progress tracking for MCP tool calls, allowing users to see progress updates while long-running tools execute.

Data Flow:

  MCP Server -> MCPConnection -> MCPManager -> SSE Events -> useSSE -> ToolCall Component                                                                                                                                                                                                                            
       ↓              ↓              ↓            ↓           ↓           ↓                                                                                                                                                                                                                                     
    Progress    Token-based     Progress      progress   Progress    Progress                                                                                                                                                                                                                                   
  Notifications  Validation     Callback      event      Map/Atom    Bar UI 

Change Type

Please delete any irrelevant options.

  • New feature (non-breaking change which adds functionality)

Testing

This test communicates with a MCP server and invokes tool progress which sends progress notification, below is a sample implementation:

	mcp.AddTool(s, &mcp.Tool{
		Name:        "progress",
		Description: "The tool to track the progress of the current task.",
	}, func(ctx context.Context, req *mcp.CallToolRequest, in any) (*mcp.CallToolResult, any, error) {

		for i := 0; i < 100; i++ {
			time.Sleep(100 * time.Millisecond)
			progress := float64(i) / 100

			_ = req.Session.NotifyProgress(ctx, &mcp.ProgressNotificationParams{
				ProgressToken: req.Params.GetProgressToken(),
				Progress:      progress,
				Total:         100,
				Message:       fmt.Sprintf("Progress: %d%%", i),
			}) // Ignore error for non-critical notifications
		}

		b, err := json.Marshal(struct {
			Progress float64 `json:"progress"`
			Total    float64 `json:"total"`
			Message  string  `json:"message"`
		}{
			Progress: 100,
			Total:    100,
			Message:  "Task completed",
		})
		if err != nil {
			return nil, nil, fmt.Errorf("unable to marshal JSON: %w", err)
		}
		return &mcp.CallToolResult{Content: []mcp.Content{&mcp.TextContent{Text: string(b)}},}, nil, nil
}
Screen.Recording.2026-01-25.at.10.39.01.mov

Test Configuration:

Checklist

  • My code adheres to this project's style guidelines
  • I have performed a self-review of my own code
  • I have commented in any complex areas of my code

@changkun changkun marked this pull request as ready for review January 25, 2026 11:59
@danny-avila danny-avila changed the base branch from dev to dev-staging January 26, 2026 14:59
@danny-avila danny-avila requested a review from Copilot January 26, 2026 15:00
@danny-avila
Copy link
Owner

@changkun thanks for the PR!

Changed base branch to dev-staging since this is reserved for changes post-v0.8.2-release.

Looks like some tests are failing and there are merge conflicts. Also started a copilot review and would be great if you can address any comments if it returns any (I'll also check validity of those AI comments).

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds end-to-end support for MCP progress notifications so the UI can show live progress updates during long-running MCP tool calls (Fixes #8221).

Changes:

  • Introduces progress-token generation/tracking in MCPConnection and passes the token via tools/call _meta.
  • Forwards MCP progress notifications from the API to the client over SSE as a new progress event.
  • Stores progress per toolCallId on the client and surfaces progress text in the ToolCall UI.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
packages/api/src/utils/events.ts Adds safer SSE response writes and introduces sendProgress SSE event.
packages/api/src/mcp/types/index.ts Adds shared progress-related types (ProgressToken, state interfaces).
packages/api/src/mcp/connection.ts Subscribes to MCP progress notifications, throttles, tracks token state, emits progress events.
packages/api/src/mcp/MCPManager.ts Generates/registers progress tokens, forwards progress events via onProgress, cleans up listeners/tokens.
client/src/store/progress.ts Adds Jotai state for mapping toolCallId -> progress.
client/src/hooks/SSE/useSSE.ts Listens for progress SSE events and updates the progress map.
client/src/components/Chat/Messages/Content/ToolCall.tsx Displays MCP progress message/values in ToolCall “in-progress” text.
client/src/components/Chat/Messages/Content/Part.tsx Passes toolCallId down to ToolCall for correct progress correlation.
api/server/services/MCP.js Forwards MCP progress updates from MCPManager to the browser via SSE.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +528 to +530
try {
logger.info(`${this.getLogPrefix()} Received progress notification:`, notification.params);
const { progressToken, progress, total, message } = notification.params;
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Progress notifications can arrive many times per second; logging every notification at info will spam logs and increase CPU/IO. Consider downgrading this to debug (or sampling), especially since you already throttle emission separately.

Copilot uses AI. Check for mistakes.
Comment on lines +253 to +257
logger.info(`${logPrefix}[${toolName}] Progress event received:`, {
progressToken: data.progressToken,
progress: data.progress,
total: data.total,
message: data.message,
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logging every progress update at info will flood logs for long-running tools (potentially multiple times per second). Consider switching this per-update log to debug (or removing it) and keeping info for coarse lifecycle events only.

Copilot uses AI. Check for mistakes.
Comment on lines +523 to +527
private subscribeToProgress(): void {
try {
this.client.setNotificationHandler(
ProgressNotificationSchema,
async (notification) => {
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Progress token tracking/throttling is new behavior but there are no tests covering it (token registration, unknown-token filtering, monotonic validation, throttling, cleanup). There is already an MCP test suite under src/mcp/__tests__; consider adding coverage for these cases to prevent regressions.

Copilot uses AI. Check for mistakes.
Comment on lines +123 to +127
sse.addEventListener('progress', (e: MessageEvent) => {
try {
const data = JSON.parse(e.data);
const { progress, total, message, toolCallId } = data;

Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Progress events are only processed in this non-resumable SSE hook. When resumable streams are enabled, this hook is inactive (submission is null), so MCP progress won’t be displayed. Progress needs to be propagated/handled in the resumable stream path as well.

Copilot uses AI. Check for mistakes.
Comment on lines +506 to +511
// Forward progress events to client via SSE, including tool call ID for matching
logger.debug(`[MCP][${serverName}][${toolName}] Sending progress to client:`, progressData);
sendProgress(res, {
...progressData,
toolCallId: toolCall.id, // Include tool call ID so frontend can match progress to specific tool call
});
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onProgress always forwards via sendProgress(res, ...), but in resumable mode (streamId set) the active SSE connection is /chat/stream/:streamId and this response may not be the streaming one. Progress updates will be dropped unless they’re routed through the resumable stream transport (e.g., GenerationJobManager.emitChunk(streamId, ...)).

Suggested change
// Forward progress events to client via SSE, including tool call ID for matching
logger.debug(`[MCP][${serverName}][${toolName}] Sending progress to client:`, progressData);
sendProgress(res, {
...progressData,
toolCallId: toolCall.id, // Include tool call ID so frontend can match progress to specific tool call
});
// Forward progress events to client, including tool call ID for matching
logger.debug(`[MCP][${serverName}][${toolName}] Sending progress to client:`, progressData);
const payload = {
...progressData,
toolCallId: toolCall.id, // Include tool call ID so frontend can match progress to specific tool call
};
if (streamId) {
// In resumable mode, route progress through the resumable stream transport
GenerationJobManager.emitChunk(streamId, payload);
return;
}
// In non-resumable mode, send progress directly via SSE on this response
sendProgress(res, payload);

Copilot uses AI. Check for mistakes.
Comment on lines +507 to +509
logger.debug(`[MCP][${serverName}][${toolName}] Sending progress to client:`, progressData);
sendProgress(res, {
...progressData,
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This forwards progressToken to the browser (...progressData). The token is generated using userId/serverName in MCPConnection.generateProgressToken(), so this can expose internal user identifiers unnecessarily. Since the UI matches progress using toolCallId, consider omitting progressToken from the SSE payload (and/or stop embedding userId in the token).

Suggested change
logger.debug(`[MCP][${serverName}][${toolName}] Sending progress to client:`, progressData);
sendProgress(res, {
...progressData,
// Strip progressToken to avoid exposing internal identifiers derived from userId/serverName.
const { progressToken, ...safeProgressData } = progressData || {};
logger.debug(
`[MCP][${serverName}][${toolName}] Sending progress to client:`,
safeProgressData,
);
sendProgress(res, {
...safeProgressData,

Copilot uses AI. Check for mistakes.
Comment on lines +246 to +248
progressToken = connection.generateProgressToken();
connection.registerProgressToken(progressToken);
logger.info(`${logPrefix}[${toolName}] Progress token generated and registered: ${progressToken}`);
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Progress token generation is logged at info. Since progress is expected to be high-frequency, consider downgrading token/progress logs to debug (or sampling) to avoid log spam and excess IO.

Copilot uses AI. Check for mistakes.
Comment on lines +147 to +151
const progress = useMemo(() => {
if (hasOutput) {
return 1; // Tool completed
}
return simulatedProgress;
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The progress bar value is always driven by simulatedProgress until output exists, so it won’t reflect real MCP {progress,total} updates. Consider computing the bar value from mcpProgress when total is present (e.g., progress/total, clamped to [0,1]) and falling back to simulated progress otherwise.

Copilot uses AI. Check for mistakes.
Comment on lines +17 to +18
} catch {
// Client may have disconnected - log but don't crash
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says “log but don't crash”, but the catch block doesn’t actually log anything. Either add logging here (at an appropriate level to avoid spamming) or adjust the comment to match the behavior.

Suggested change
} catch {
// Client may have disconnected - log but don't crash
} catch (error) {
// Client may have disconnected - log but don't crash
console.warn('safeWrite: failed to write to response; client may have disconnected.', error);

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants