Skip to content

Progress callbacks run concurrently, out of order, and after call_tool() returns #3747

Open

Description

@dhruvindave007

Initial Checks

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

  1. added
    bugSomething isn't working
    v2Affects the v2 line (2.x on main)
    on Oct 23, 2026
  2. dhruvindave007 commented on Oct 26, 2026

    @dhruvindave007
    Author

    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:

    • one ordered progress queue per pending request, consumed by a single task, so callbacks for a request never run concurrently and keep arrival order
    • send_raw_request drains that queue before returning the result, on both the success and error-response paths
    • on caller cancellation it does not wait for callbacks; it closes the queue and returns immediately, since a cancelled caller shouldn't be blocked by a slow callback
    • a regression test whose callback actually awaits, run over the in-memory, SSE and streamable HTTP transports

    On the cancellation question above: I think not waiting is right there, matching how cancellation already abandons the response.

  3. Ben-Home commented on Oct 29, 2026

    @Ben-Home
  4. dhruvindave007 commented on Oct 29, 2026

    @dhruvindave007
    Author

    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. DirectDispatcher awaits 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_progress contains 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:

    ['progress 1.0', 'caller cancelled', 'progress 2.0', 'progress 3.0', 'progress 4.0', 'progress 5.0']
    

    So on cancellation the fix will drop anything still queued, and only a callback that is already running finishes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingv2Affects the v2 line (2.x on main)

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions