Repository navigation
fix: deliver progress notifications that share a read with their response - #3023
Steeve-Crypto wants to merge 2 commits into
Conversation
…onse _onnotification deferred handlers via Promise.resolve(), while _onresponse runs synchronously and deletes the progress handler. When both messages arrive in one turn, the response won the race and the progress was dropped. Dispatch notification handlers synchronously (async rejections still go to _onerror) and add a regression test for the same-turn case. Fixes modelcontextprotocol#3018
|
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/core
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
The sync notification dispatch in modelcontextprotocol#3023 delivers progress that shares a read with its response, so the SSE progress scenarios should pass.
|
Verified the fix locally. On main, applying only the new regression test fails with 1525 passed and 1 failed in packages/core-internal—the progress callback is never called. On this branch the full suite passes with 1526 passed. The synchronous dispatch in packages/core-internal/src/shared/protocol.ts:933 closes the race by handling the notification before _onresponse deletes the progress handler. The Promise.resolve() deferral was the culprit. |
This branch has not been deployed
2 participants
Summary
Fixes #3018.
_onnotificationdeferred handlers withPromise.resolve(), while_onresponseruns synchronously and deletes the matching progress handler. When a progress notification and its response arrive in the same read/turn, the response cleared the handler before the deferred notification ran, so progress was dropped andonerrorreported an unknown token.This dispatches notification handlers synchronously (same posture as
_onresponse). Synchronous throws and async rejections still go to_onerror.Test plan
pnpm testinpackages/core-internal(1526 passed, including the new case)AI disclosure
Used AI assistance to locate the race in
_onnotificationvs_onresponse, draft the sync dispatch change and regression test, and write this PR description. I reviewed the diff and ran the tests above myself.