Repository navigation
Conversation
callTool already asks servers for progress but drops each notification. Forward them to the running tool's progress so clients can show what a long MCP call is doing.
There was a problem hiding this comment.
Direct MCP tools now forward progress, but MCP tools are Code Mode tools by default, and there the progress overwrites the execute call's own progress, so this needs a change before it's ready.
How I checked: the new client test passes. A temporary test through the tool registry (not committed) shows a progress notification from an MCP tool called inside execute replacing the execute call's { toolCalls } progress with { progress, total, message } until the next update. Core and the client both replace progress metadata instead of merging it, and the TUI draws the execute card from metadata.toolCalls. So during a long MCP call (the case this PR is for), the card loses its call list. Details are inline. bun test test/mcp.test.ts gives 74 pass and 1 fail; the failing test (terminates MCP descendants…) fails on the base too. typecheck:tsc passes.
Public API: no schema changes. MCP tools now emit the existing session.tool.progress event with progress/total/message metadata. The onProgress option and the McpClient.Progress type are internal to core.
Related: #49929
| name: tool.name, | ||
| args: (input ?? {}) as Record<string, unknown>, | ||
| sessionID: context.sessionID, | ||
| onProgress: (progress) => |
There was a problem hiding this comment.
When this tool runs inside Code Mode (the default for MCP tools), context belongs to the outer execute call, so context.progress({ progress, total, message }) replaces that call's { toolCalls } progress (publish-llm-event sets tool.progress = update, and client/src/solid/data.ts replaces state.metadata). I checked with a mocked onProgress call: the execute call's progress updates went {toolCalls:[running]} → {progress:1,total:2,message} → {toolCalls:[completed]}, so the execute card's call list disappears while a slow MCP call runs. The 'late notifications are ignored' guarantee also doesn't hold here: the outer execute call is still running, so a late notification from a nested call that has finished still gets published. Nested calls probably need their progress recorded per call (e.g. on the ExecuteCall entry through Code Mode's progress hooks) instead of overwriting the parent's metadata.
A tool called inside execute shares the execute call's context, so its progress replaced the call list. Record it on the call's own toolCalls row instead, and drop progress for a call that has settled.
There was a problem hiding this comment.
The new commit fixes the Code Mode problem from my earlier comment: a nested call's progress now goes on its own toolCalls row instead of replacing the execute call's progress, and progress sent after a call has finished is dropped. I re-ran my earlier temporary test. Inside execute (with and without arguments), every progress update was a call list, and the running row had progress: { progress, total, message }. A direct MCP tool still gets the bare { progress, total, message }. The related tests in packages/core pass, apart from terminates MCP descendants…, which also fails on the base. typecheck:tsc passes.
Public API: rows in the execute tool's toolCalls (both its progress and its final output) can now have an optional progress field. It's an added optional field, so existing clients keep working. MCP tools also now publish { progress, total, message } through the existing session.tool.progress event.
No further concerns.
Issue for this PR
Refs #49929 (the core half: the progress is published; drawing it on the TUI's tool card is not in this PR)
Type of change
What does this PR do?
McpClient.callToolasks servers for progress (onprogress, so the SDK sends aprogressToken) but the handler is() => {}, so everynotifications/progressa server sends is dropped.This passes an optional
onProgressthroughMcp.callToolto the client, and the MCP tool wrapper publishes each notification as the running call's progress withcontext.progress({ progress, total, message }), the same path the shell and subagent tools use. Clients then get it assession.tool.progressmetadata. The SDK callsonprogressoutside any fiber, so it runs with the tool's context.Inside Code Mode (the default for MCP tools) a nested tool shares the
executecall's context, so its progress used to replace that call's{ toolCalls }. Each nested call now gets its ownprogress, which records the update on itstoolCallsrow (a new optionalprogressfield) and drops it once that call has settled. Theexecutecall's metadata is always its call list. This also changes the existing test intool-registry.test.tsthat expected a nested{ stage: "old" }to replace the parent's progress.We run an MCP server with calls that take a few minutes (reading a multi-page design), and our client had nothing to show until the call ended.
How did you verify your code works?
test/mcp.test.ts: a server that sends two progress notifications during a tool call; the client'sonProgressgets both, in order, with total and message.test/tool-registry.test.ts: two nested calls insideexecute; every progress the parent publishes is a call list, the running call's row carries its progress, and a progress sent after the call settled publishes nothing.bun testfortest/mcp.test.ts(75),test/tool-registry.test.ts(42),test/tool-execute.test.ts,test/session-runner.test.ts,test/codemodeand packages/codemode, all passing;bun run typecheckin packages/core.Screenshots / recordings
Not a UI change.
Checklist
Added by the opencode bot (AI-generated).
Related: #49929
Public API: adds an optional
progressfield to Code ModetoolCallsrows; MCP tools publish progress viasession.tool.progress(backwards compatible)