feat(node): restore SSEServerTransport under /node/sse, @deprecated#1909
feat(node): restore SSEServerTransport under /node/sse, @deprecated#1909felixweinberger wants to merge 2 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 2a95520 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
@modelcontextprotocol/client
@modelcontextprotocol/server
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
felixweinberger
force-pushed
the
fweinberger/v2-bc-node-sse-restore
branch
2 times, most recently
from
9624cc3 to
5d751a4
Compare
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
felixweinberger
force-pushed
the
fweinberger/v2-bc-node-sse-restore
branch
from
5d751a4 to
2d16182
Compare
|
@claude review |
felixweinberger
force-pushed
the
fweinberger/v2-bc-node-sse-restore
branch
from
2d16182 to
1188b1a
Compare
|
@claude review |
…node/sse Ports the v1 SSEServerTransport to v2 as a deprecated compat shim under a dedicated /sse subpath. The implementation is a frozen copy of the v1 class with imports adapted to the v2 package layout (parseJSONRPCMessage, MessageExtraInfo.request as Web Request). Constructing the transport emits a one-time deprecation warning pointing to NodeStreamableHTTPServerTransport. The class and options interface are both marked @deprecated and slated for removal in v3. Adds content-type and raw-body as runtime deps (matching v1 body parsing).
felixweinberger
force-pushed
the
fweinberger/v2-bc-node-sse-restore
branch
from
1188b1a to
9287528
Compare
|
@claude review |
There was a problem hiding this comment.
All prior feedback addressed (docs, changeset wording, section heading, test-only export removed) — no further issues from me. Deferring to a human for the final sign-off since this adds a new public entry point (./sse), two runtime deps, and a ~260-line transport, which is a BC-series design call worth a maintainer ack.
Extended reasoning...
Overview
Restores the v1 SSEServerTransport as a deprecated compat shim under @modelcontextprotocol/node/sse. Touches: new packages/middleware/node/src/sse.ts (~260 LoC, frozen v1 port + a small toWebRequest adapter), new ./sse subpath export in package.json + tsdown.config.ts, two new runtime deps (content-type, raw-body), a vitest integration test, a changeset, and doc updates across docs/migration.md, docs/migration-SKILL.md, and docs/faq.md.
Review history
Four prior rounds. All actionable items are now resolved: migration/FAQ prose updated to reflect the shim; the leaked _resetSSEDeprecationWarning export was removed (runtime warning dropped entirely); the changeset wording was corrected (no more "without code changes" / non-existent meta-package); and the §8 heading was renamed in 2a95520. The two declined items (close() double-onclose, unguarded new URL on raw Host header) were declined on the explicit "frozen v1 port — not hardening beyond v1" basis, which is consistent with the PR's stated scope and the linked BC-series reviewer guide.
Security risks
The toWebRequest Host-header parse can throw on malformed-but-valid header values, potentially yielding an unhandled rejection — flagged previously, author confirmed v1 has identical behavior at the same position, so this is not a regression. DNS-rebinding protection is opt-in (matches v1). No new auth/crypto surface.
Level of scrutiny
Moderate-to-high: this is new public API surface and new dependencies, and it reverses a prior v2 removal decision. The implementation itself is low-risk (battle-tested v1 code), but the decision to ship it belongs to a maintainer per REVIEW.md's "burden of proof is on addition" principle.
Other factors
Part of the documented v2-bc series with a reviewer guide; integration-tested against 5 OSS repos per the description; vitest coverage for the happy path. Bug-hunting system found nothing this round. I'm not approving solely because the scope (new entry point + deps + design reversal) exceeds the "simple/mechanical" bar, not because of any outstanding concern.
…esVersions with exports Addresses bhosmer-ant review on #1898: - packages/middleware/node: add typesVersions for the /sse subpath (forward-ref to #1909, same pattern as the stdio/zod-schemas forward-refs in client/server) - All 6 touched package.jsons: move types/typesVersions to sit immediately after exports so subpath additions update one contiguous block
1 participant
Part of the v2 backwards-compatibility series — see reviewer guide.
v2 removed
SSEServerTransport. Proxy/bridge use-cases (stdio→SSE→browser) still need it; the spec still documents SSE. Restored as@deprecatedv1 copy.Motivation and Context
v2 removed
SSEServerTransport. Proxy/bridge use-cases (stdio→SSE→browser) still need it; the spec still documents SSE. Restored as@deprecatedv1 copy.v1 vs v2 pattern & evidence
v1 pattern:
`import { SSEServerTransport } from '@modelcontextprotocol/sdk/server/sse.js'`v2-native:
Evidence: Hit in real-repo testing (proxy/bridge pattern).
How Has This Been Tested?
v2-bc-integrationvalidation branchpnpm typecheck:all && pnpm lint:all && pnpm test:allgreenBreaking Changes
None — additive
@deprecatedshim. Removed in v3.Types of changes
Checklist
Additional context
Stacks on: none