Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/node-transport-v1-aliases.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@modelcontextprotocol/node': patch
---

Add v1-compat re-exports: `StreamableHTTPServerTransport` (alias for `NodeStreamableHTTPServerTransport`) and the `EventStore` / `EventId` / `StreamId` types, so v1 imports from `@modelcontextprotocol/sdk/server/streamableHttp.js` map cleanly onto `@modelcontextprotocol/node`.
7 changes: 7 additions & 0 deletions packages/middleware/node/src/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,8 @@
export * from './streamableHttp.js';

// v1-compat re-exports.
export {
/** v1 name. The v2-preferred name is {@linkcode NodeStreamableHTTPServerTransport}. */
NodeStreamableHTTPServerTransport as StreamableHTTPServerTransport
} from './streamableHttp.js';

Check warning on line 7 in packages/middleware/node/src/index.ts

View check run for this annotation

Claude / Claude Code Review

PR description claims '@deprecated shim' but code has no @deprecated tag

This supersedes the earlier inline comment, which now misquotes the diff after the force-push — the current code has **no** `@deprecated` tag (line 5 reads only "v1 name. The v2-preferred name is…"), no "keep until v3" comment, and no `eslint-disable` in the test. That's internally consistent with the existing `StreamableHTTPServerTransportOptions` alias, but it contradicts the PR description's "additive `@deprecated` shim. Removed in v3" framing: consumers of the v1 name get zero deprecation si
Comment on lines +4 to +7
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 This supersedes the earlier inline comment, which now misquotes the diff after the force-push — the current code has no @deprecated tag (line 5 reads only "v1 name. The v2-preferred name is…"), no "keep until v3" comment, and no eslint-disable in the test. That's internally consistent with the existing StreamableHTTPServerTransportOptions alias, but it contradicts the PR description's "additive @deprecated shim. Removed in v3" framing: consumers of the v1 name get zero deprecation signal at any level. Either restore @deprecated via the export const + export type pattern so it survives dts bundling, or drop the "Removed in v3" claim from the description/changeset to match the code.

Extended reasoning...

What changed since the last review

The earlier claude[bot] inline comment on this file quotes source that no longer exists in the PR. It references:

  • /** @deprecated Use {@linkcode NodeStreamableHTTPServerTransport}. Removed in v3. */
  • // v1-compat re-exports — keep until v3.
  • an eslint-disable @typescript-eslint/no-deprecated line in compat.test.ts

None of those appear in the current single-commit diff (e5af3b3, force-pushed over the earlier 3d48f5e per the pkg-pr-new comment). The current state is:

// v1-compat re-exports.
export {
    /** v1 name. The v2-preferred name is {@linkcode NodeStreamableHTTPServerTransport}. */
    NodeStreamableHTTPServerTransport as StreamableHTTPServerTransport
} from './streamableHttp.js';

and compat.test.ts imports StreamableHTTPServerTransport with no lint suppression. grep -r '@deprecated' packages/middleware/ returns zero hits. So the previous comment's premise ("@deprecated is present in source but stripped by the bundler") is no longer accurate — there is now no @deprecated anywhere, source or dist.

The remaining mismatch

The PR description's Breaking Changes section still says:

None — additive @deprecated shim. Removed in v3.

That no longer describes the code. What ships is a plain alias with an informational JSDoc hint (and per the previous review's build evidence, even that hint is dropped from dist/index.d.mts because tsdown flattens export specifiers). A consumer who writes import { StreamableHTTPServerTransport } from '@modelcontextprotocol/node' gets no IDE strikethrough and no @typescript-eslint/no-deprecated warning.

Step-by-step

  1. Line 5 of packages/middleware/node/src/index.ts contains the JSDoc /** v1 name. The v2-preferred name is {@linkcode NodeStreamableHTTPServerTransport}. */ — no @deprecated token.
  2. compat.test.ts line 3 imports StreamableHTTPServerTransport directly, with no // eslint-disable-next-line @typescript-eslint/no-deprecated. Lint passes, confirming the symbol is not marked deprecated even at source level.
  3. pnpm --filter @modelcontextprotocol/node builddist/index.d.mts emits the alias in a flattened export { … NodeStreamableHTTPServerTransport as StreamableHTTPServerTransport … } line with no JSDoc at all (same bundler behaviour the previous comment documented).
  4. The ~1,700 v1 call sites cited in the PR description compile cleanly with no migration nudge. If the name is actually removed in v3 as the description states, they break with no prior warning.

Why this might be intentional

The pre-existing StreamableHTTPServerTransportOptions alias at streamableHttp.ts:22 also carries no @deprecated — just a "for backward compatibility" note. So dropping @deprecated here is consistent with prior art in this package. The changeset file (.changeset/node-transport-v1-aliases.md) also doesn't claim deprecation, so what's published to npm is self-consistent. The only thing out of sync is the PR description.

Two valid resolutions

(a) If v3 removal is real, restore the deprecation in a form that survives dts bundling:

import { NodeStreamableHTTPServerTransport } from './streamableHttp.js';
/** @deprecated Use {@linkcode NodeStreamableHTTPServerTransport}. Removed in v3. */
export const StreamableHTTPServerTransport = NodeStreamableHTTPServerTransport;
/** @deprecated Use {@linkcode NodeStreamableHTTPServerTransport}. Removed in v3. */
export type StreamableHTTPServerTransport = NodeStreamableHTTPServerTransport;

(and add the eslint-disable back to the test, and consider doing the same for StreamableHTTPServerTransportOptions).

(b) If the alias is meant to be a stable, non-deprecated convenience export (matching the Options precedent), just drop "@deprecated shim. Removed in v3" from the PR description so the record matches what ships.

export type { EventId, EventStore, StreamId } from '@modelcontextprotocol/server';
Comment thread
claude[bot] marked this conversation as resolved.
19 changes: 19 additions & 0 deletions packages/middleware/node/test/compat.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { describe, expect, expectTypeOf, it } from 'vitest';

import { NodeStreamableHTTPServerTransport, StreamableHTTPServerTransport } from '../src/index.js';
import type { EventId, EventStore, StreamId } from '../src/index.js';

describe('v1 compat exports from @modelcontextprotocol/node', () => {
it('StreamableHTTPServerTransport aliases NodeStreamableHTTPServerTransport', () => {
expect(StreamableHTTPServerTransport).toBe(NodeStreamableHTTPServerTransport);
expectTypeOf<StreamableHTTPServerTransport>().toEqualTypeOf<NodeStreamableHTTPServerTransport>();
});

it('re-exports EventStore / EventId / StreamId types', () => {
// Type-level assertions: these compile only if the types are exported.
expectTypeOf<EventId>().toBeString();
expectTypeOf<StreamId>().toBeString();
expectTypeOf<EventStore>().toHaveProperty('storeEvent');
expectTypeOf<EventStore>().toHaveProperty('replayEventsAfter');
});
});
Loading