Skip to content

fix: auto-send notifications/cancelled on anyio task cancellation#2479

Open
cubaseuser123 wants to merge 1 commit intomodelcontextprotocol:mainfrom
cubaseuser123:fix/1410-auto-cancel-notification
Open

fix: auto-send notifications/cancelled on anyio task cancellation#2479
cubaseuser123 wants to merge 1 commit intomodelcontextprotocol:mainfrom
cubaseuser123:fix/1410-auto-cancel-notification

Conversation

@cubaseuser123
Copy link
Copy Markdown

@cubaseuser123 cubaseuser123 commented Apr 20, 2026

Fixes #1410.

Motivation and Context

The friction here is pretty specific: when you use high-level request wrappers like client.call_tool(), the generated request_id stays hidden internally inside send_request. So if you need to abort that task from the outside using standard Python async patterns (like anyio.fail_after or CancelScope.cancel()), the client aborts locally, but it has no way to tell the server. The server continues executing an orphaned request that the client has already stopped waiting for.

So I moved the cancellation hook into BaseSession.send_request. The idea is simple: intercept the anyio cancellation before it bubbles up, read the request_id we just generated, and automatically dispatch a notifications/cancelled straight to the peer.

How Has This Been Tested?

I made sure it runs cleanly end-to-end. Added test_anyio_cancel_scope_sends_cancelled_notification to tests/server/test_cancel_handling.py which:

  • Kicks off call_tool
  • Triggers an outer CancelScope.cancel()
  • Asserts the server handler actually got aborted

(Side note: Verified locally against the full regression suite using anyio 4.10.0 on Windows.)

Breaking Changes

None.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

A few quick implementation details:

  • It uses a request_sent flag: Cancels are only dispatched if the request actually hit the wire (so we don't spam the server for requests that failed on _write_stream.send()).
  • It's shielded: The dispatch runs inside with anyio.CancelScope(shield=True) so it survives the very cancellation it's reacting to.
  • It has a deadlock timeout: Added with anyio.move_on_after(2.0) inside the shield. If the transport buffer is completely full, we don't want to hang the user's cancellation handler forever.
  • It's bidirectional: Since this lives in BaseSession, it automatically works for both client→server and server→client directions.

Happy to tweak the timeout value or the notification reason string if there's a preference. Let me know if I should squash anything.

Just a heads-up for reviewers — the CI failures here are pre-existing and not related to the changes in this PR.

The pyright hook in .pre-commit-config.yaml runs against the whole repo. Currently, the upstream main branch (commit 3d7b311) has 21 Pyright errors located entirely in src/mcp/os/win32/utilities.py.

Because pre-commit is configured with fail_fast: true, it aborts early and cascades all 20 test matrix jobs (checks / test) to fail with exit code 2 before Pytest even gets a chance to run. (I noticed this is happening on other recent PRs like #2475 as well).

I have verified the test suite locally using parallel workers (pytest -n auto):

  • 1170 passed, 101 skipped, 1 xfailed, exit 0.
  • uv run pyright src/mcp/shared/session.py returns 0 errors.

Happy to rebase to trigger a clean run once the upstream Pyright issue on main is resolved, or if a maintainer wants to merge this as-is!

Closes modelcontextprotocol#1410.

Users of high-level APIs like client.call_tool() have no access to the
internal 
equest_id, making it impossible to manually send a cancel
notification when a task is aborted. The server would keep processing a
request that nobody is waiting for.

This fix intercepts �nyio task/scope cancellation inside send_request
and automatically dispatches
otifications/cancelled to the peer before
re-raising. The send is shielded from the cancellation scope and guarded
by a 2-second timeout to prevent deadlock if the transport buffer is full.

A 
equest_sent flag ensures we only notify the server if the request
actually hit the wire — avoiding spurious cancels for requests that were
never received.

This mirrors the pattern used by the TypeScript SDK with AbortSignal and
applies to both client→server and server→client request directions since
the change lives in BaseSession.
@cubaseuser123 cubaseuser123 force-pushed the fix/1410-auto-cancel-notification branch from 8f4ea8c to ed89a6a Compare April 20, 2026 09:23
@christianjohnmorral18
Copy link
Copy Markdown

The request_id visibility issue is a real friction point for anyone writing custom cancellation logic on top of the high-level wrappers. Having the SDK handle notifications/cancelled automatically on anyio cancellation makes the cancel path much more reliable — previously you had to inject your own cleanup hook at the task group level to get any notification out. One question: for servers that don't handle notifications/cancelled, is there a risk the auto-notification triggers a confusing error log on their side?

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.

Cancelling a Call Tool Request from Client

2 participants