fix(core): await the notification send so a failed send is never briefly unhandled

[claude]

Requested by Felix Weinberger · Slack thread

Problem

Sending a notification on a closed connection (e.g. the transport closes during the legacy initialize handshake, so _legacyHandshake sends notifications/initialized with no transport) produces a briefly unhandled promise rejection in addition to the rejection the caller receives. Node's tracker is silent about it; workerd reports unhandledrejection followed by rejectionhandled, which shows up as noise in Vitest runs on Cloudflare Workers.

Cause

Protocol.notification() is async and did return this._notificationViaCodec(...). The funnel throws Not connected synchronously, so it hands back an already-rejected promise. Returning that promise (rather than awaiting it) resolves the outer promise through the thenable job, which only attaches a handler to the inner promise one microtask later. For that one microtask the inner rejection has no handler.

Fix

return await this._notificationViaCodec(...) in packages/core-internal/src/shared/protocol.ts. await attaches its reaction synchronously, so the inner promise is handled in the same microtask it rejects in. One-word source change; connect() still rejects with the same error (SdkError / NotConnected), and _legacyHandshake is untouched.

Verification

• packages/core-internal/test/shared/notificationSendRejection.test.ts — when not connected, the inner send rejection is handled in the same microtask (no thenable-job hop): substitutes the funnel with a natively rejected promise whose then property counts reads. With a bare return, the thenable job reads then (1 read, i.e. the inner promise sat rejected and unhandled for a microtask); with return await it is never read. Fails on main (expected 1 to be +0), passes with the fix.
• packages/server/test/server/notificationSendRejection.test.ts — same observation for the server-side send via Server.sendLoggingMessage(); fails on main the same way, passes with the fix.
• packages/client/test/client/legacyHandshakeCloseAfterInitialize.test.ts — end-to-end trigger: a transport that answers initialize then closes in the same tick. Pins that connect() rejects with SdkError code NotConnected, only initialize went out, and no unhandledRejection reaches the process (passes on both main and this branch, since Node does not report the gap).
• Full core-internal / client / server suites compared against unmodified main: 1457→1459, 895→896, 521→523 passing; no existing test changed outcome. pnpm typecheck:all and pnpm lint:all clean.

Fixes #2864

🤖 Generated with Claude Code

https://claude.ai/code/session_012VRbFCp41otcScXE1YY3es

---

Generated by Claude Code

[changeset-bot]

🦋 Changeset detected

Latest commit: 607eb84

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages

Name	Type
@modelcontextprotocol/client	Patch
@modelcontextprotocol/server	Patch
@modelcontextprotocol/codemod	Patch
@modelcontextprotocol/core	Patch
@modelcontextprotocol/server-legacy	Patch
@modelcontextprotocol/core-internal	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[pkg-pr-new]

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@2885

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@2885

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@2885

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@2885

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@2885

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@2885

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@2885

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@2885

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@2885

commit: 607eb84

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked the other outbound paths in protocol.ts for the same one-microtask gap: request() (line 1271) and the related-send closures in _onrequest (line 1034) are not async and hand the funnel's promise back directly, so they have no thenable-job hop and are not affected.

Extended reasoning...

The production change is a single return await in Protocol.notification() in packages/core-internal/src/shared/protocol.ts, plus three new tests and a changeset; it touches no auth, crypto, or data-exposure surface. Inline findings already cover the sibling async send* wrappers in server.ts/client.ts that still return without await and a misleading test comment, so the note here only records the request and related-send paths that were examined and ruled out.

2 optional notes from this repository's REVIEW.md or CLAUDE.md checks were not posted as comments, over this review's limit for such notes; they are on this commit's check card.

[claude]

Code review found no issues

No high-confidence issues detected in this change.