Repository navigation
Progress callbacks run concurrently, out of order, and after call_tool() returns #3747
Description
Initial Checks
- I confirm that I'm using the newest release of my line (the latest 2.x, or the latest 1.x if I'm still on v1)
- I confirm that I searched for my issue in https://github.com/modelcontextprotocol/python-sdk/issues before opening this issue
Release line
2.x (current stable)
Description
Over any transport that goes through JSONRPCDispatcher (stdio, SSE, streamable HTTP, in-memory streams), each notifications/progress spawns its own task for the request's progress_callback, and nothing waits for it. As soon as the callback awaits anything:
- callbacks for the same request run concurrently and can finish in any order
call_tool()/send_request()can return before earlier progress callbacks have run
With the example below, a real transport prints:
['call_tool returned', 'progress 3.0', 'progress 2.0', 'progress 1.0']
The same code with Client(server) (in-process DirectDispatcher, which awaits the callback inline) prints what I'd expect:
['progress 1', 'progress 2', 'progress 3', 'call_tool returned']
v1 also delivered progress in order, before the result.
Cause: in src/mcp/shared/jsonrpc_dispatcher.py, _dispatch_notification, the notifications/progress branch calls self._spawn(_shielded_progress(pending.on_progress), ...) once per notification. The response then resolves the pending request independently of those tasks. tests/interaction/lowlevel/test_progress.py doesn't catch it because its callback never awaits.
Impact: progress bars go backwards, a final "100%" lands after the result was handled, and callbacks can touch state the caller already cleaned up. The server side has the same problem (send_request(..., progress_callback=...)).
Possible fix: deliver progress for each pending request through a single ordered queue with one consumer, and drain it before send_raw_request hands back the result. Awaiting the callback inline in the read loop would also restore ordering, but a slow callback would then block every other message on the connection.
I'm happy to send a PR with a regression test if this approach sounds right.
I used an AI assistant to help trace the cause; I reproduced and verified it myself.
Example Code
import anyio
from mcp.client import Client
from mcp.client._memory import InMemoryTransport
from mcp.server.mcpserver import Context, MCPServer
server = MCPServer("progress")
@server.tool()
async def work(ctx: Context) -> str:
for p in (1, 2, 3):
await ctx.report_progress(p, 3)
return "done"
async def main() -> None:
events: list[str] = []
async def on_progress(progress: float, total: float | None, message: str | None) -> None:
await anyio.sleep(0.1 * (4 - progress)) # e.g. updating a UI or writing a log
events.append(f"progress {progress}")
# Swap in Client(server) to see the correct order.
async with Client(InMemoryTransport(server)) as client:
await client.call_tool("work", {}, progress_callback=on_progress)
events.append("call_tool returned")
await anyio.sleep(1)
print(events)
anyio.run(main)Python & MCP Python SDK
Python 3.12.14
mcp 2.3.0 (also reproduced on main @ 2af9e30)
anyio 4.15.1
Windows 11
Activity
Jihaochang1 commentedon Oct 24, 2026
Confirming the reproduction on main @ 2af9e30, independently of the report: with the example above the events list comes out
['call_tool returned', 'progress 3.0', 'progress 2.0', 'progress 1.0']over the stream transports (in-memory legacy, SSE, streamable HTTP stateful and stateless), and only the modern in-process path (DirectDispatcher) prints the expected['progress 1.0', 'progress 2.0', 'progress 3.0', 'call_tool returned']. The existingtest_progress_during_tool_call_reaches_callback_in_ordercannot catch this because its callback never awaits.I put together a fix along the lines the report suggests, for what it's worth: each notification's task awaits its predecessor's done-event before invoking the callback, so the callbacks for one request run one at a time in arrival order, and
send_raw_requestdrains the tail of the chain before the request completes. The read loop still never blocks on a callback (inline awaiting would stall every other message on the connection), the callback stays shielded and exception-contained, progress arriving after the response is still dropped, and because the same dispatcher servescall_tool(progress_callback=...)andsend_request(progress_callback=...)the server direction is covered too. The regression test fails on 5 of the 6 transport-matrix cells before the change and passes on all 6 after; the parked PR is #3753.One design question the fix surfaces: the drain currently also runs on the error-response path (a JSON-RPC error response still waits for in-flight callbacks) but not on the caller-cancelled path, where the request propagates immediately. Happy to hear if maintainers would rather have it consistent the other way.
dhruvindave007 commentedon Oct 26, 2026
Thanks for confirming the reproduction across the transports @Jihaochang1
As the reporter I'd like to take the fix, if a maintainer is happy to assign it. My plan:
send_raw_requestdrains that queue before returning the result, on both the success and error-response pathsOn the cancellation question above: I think not waiting is right there, matching how cancellation already abandons the response.
Ben-Home commentedon Oct 29, 2026
Your diagnosis is right and the
DirectDispatchercomparison is what makes it conclusive, because it isolates the dispatcher from the callback semantics. Same code, two transports, opposite ordering. That is a defect in_dispatch_notification, not in how anyone uses progress.Three refinements on the proposed fix, since the ordering guarantee has edges that are easy to get wrong on the first pass:
Order per request, not globally. A single consumer queue is correct only if it is one queue per pending request. A connection-wide ordered queue would serialize progress from unrelated concurrent requests behind the slowest callback, which trades a correctness bug for a throughput bug. In your proposed shape the pending request already owns its callbacks, so keying the queue to that object is the natural implementation, but state it, or the first PR will use a shared queue and pass the same test.
The drain before returning the result has to be bounded. "Drain before
send_raw_requesthands back the result" is correct for ordering, and it also gives any callback the power to block the response forever, because a callback that awaits a hung dependency now sits between the caller and its result. Bound the drain, and decide explicitly what a timeout does: delivering late progress after the result is exactly the bug you are fixing, so the timeout should drop the remaining callbacks and let the result through.Check the interaction with cancellation.
_shielded_progressshields the task from cancellation, which is intentional for surviving a cancelled scope during delivery, but it also means the callbacks keep running after the caller has abandoned the request, which is the "touch state the caller already cleaned up" half of your impact statement. An ordered queue gives you a clean place to solve this: on cancellation, stop consuming the queue and let the shield coverage end, rather than shielding each callback independently.The test gap you identified is the most reusable part of this report and worth calling out in the PR: a callback that never awaits cannot observe ordering, because with no suspension point every task runs to completion inline and the failure is invisible. The regression test needs a callback that awaits an actual yield, and it should assert the full sequence
progress 1, progress 2, progress 3, resultrather than assert that all three arrived. Asserting arrival is what the current test does, and that is why it passes.One note on v1 compatibility, since you mentioned it: if restored ordering makes callbacks synchronously interleaved with the read loop, watch the path where a callback itself issues a request on the same connection. That is the case an ordered queue handles and inline awaiting deadlocks on, which is a reason to prefer the queue over the simpler inline fix.
dhruvindave007 commentedon Oct 29, 2026
Thanks @Ben-Home. Agreed on per-request queues, on asserting the full sequence with a callback that actually yields, and on the nested-request case, which is a good reason to prefer the queue over inline awaiting.
Two points where I see it differently:
Bounding the drain. I'd rather not add a timeout that drops callbacks. A slow callback only delays its own call, not the connection: the read loop keeps running, and the wait stays cancellable, so
anyio.fail_after()around the call already bounds it.DirectDispatcherawaits the callback unbounded too, and the goal here is for both paths to behave the same. A built-in bound would need either a magic number or new API, and dropping progress on timeout swaps one silent loss for another.Cancellation.
_shielded_progresscontains exceptions; it doesn't shield from cancellation. You're right about the outcome, though: I checked, and after the caller cancels, already-queued callbacks still run:So on cancellation the fix will drop anything still queued, and only a callback that is already running finishes.