From f5b65c7f1a3ec6af48c87c725745e3a24d3515dd Mon Sep 17 00:00:00 2001 From: Patrick Schmitt <45056826+patschmittdev@users.noreply.github.com> Date: Sat, 12 Sep 2026 15:47:29 -0400 Subject: [PATCH] fix: preserve strict transport declarations on v1.x Allow explicitly undefined transport callbacks and session IDs to match the Node HTTP accessors while preserving the generic message callback. Express the existing header input domain without the DOM-only HeadersInit alias or widening Node header values. Add SDK-only ESM and CommonJS public consumers, declaration roundtrips, precise negative type cases, and focused runtime regressions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 37126330-b171-4157-975e-35a6e42f2d18 --- CONTRIBUTING.md | 14 ++ scripts/fixtures/transport-types/consumer.cts | 37 +++++ scripts/fixtures/transport-types/consumer.mts | 64 ++++++++ .../fixtures/transport-types/downstream.cts | 13 ++ .../fixtures/transport-types/downstream.mts | 10 ++ scripts/test-transport-types.ts | 150 ++++++++++++++++++ src/shared/transport.ts | 10 +- test/server/streamableHttpCallbacks.test.ts | 39 +++++ test/shared/transport.test.ts | 22 +++ 9 files changed, 354 insertions(+), 5 deletions(-) create mode 100644 scripts/fixtures/transport-types/consumer.cts create mode 100644 scripts/fixtures/transport-types/consumer.mts create mode 100644 scripts/fixtures/transport-types/downstream.cts create mode 100644 scripts/fixtures/transport-types/downstream.mts create mode 100644 scripts/test-transport-types.ts create mode 100644 test/server/streamableHttpCallbacks.test.ts create mode 100644 test/shared/transport.test.ts diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ace9542dbf..a99025fb74 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -26,6 +26,20 @@ We welcome contributions to the Model Context Protocol TypeScript SDK! This docu - Keep changes focused and atomic - Provide a clear description of changes +## Strict Transport Declaration Checks + +Build with `npm run build`, then install the packed SDK in an isolated consumer directory. Run the generic public-export checks with the desired TypeScript CLI: + +```sh +npx tsx scripts/test-transport-types.ts /path/to/consumer /path/to/typescript/bin/tsc +``` + +The consumer directory must contain the installed SDK and `@types/node`. The checks use `strict`, `exactOptionalPropertyTypes`, `noUncheckedIndexedAccess`, `verbatimModuleSyntax`, and `skipLibCheck: false` with ES2022 and NodeNext. They check Node-only and DOM library +configurations, ESM and CJS public exports, callback clearing, invalid callback/header types, and declaration emission followed by a second consumer with no access to the first consumer's source. + +`Transport` callbacks may be omitted or explicitly cleared with `undefined`, matching the Node HTTP transport accessors. Its session ID may also be absent or `undefined`. `normalizeHeaders` uses the standard `Headers`, tuple-array, and string-record forms directly so Node +consumers do not need the DOM-only `HeadersInit` alias. + ## Running Examples - Start the server: `npm run server` diff --git a/scripts/fixtures/transport-types/consumer.cts b/scripts/fixtures/transport-types/consumer.cts new file mode 100644 index 0000000000..42fb34ac13 --- /dev/null +++ b/scripts/fixtures/transport-types/consumer.cts @@ -0,0 +1,37 @@ +import Http = require('@modelcontextprotocol/sdk/server/streamableHttp.js'); +import ServerModule = require('@modelcontextprotocol/sdk/server/index.js'); +import TransportModule = require('@modelcontextprotocol/sdk/shared/transport.js'); +import type { Transport } from '@modelcontextprotocol/sdk/shared/transport.js'; +import type { IncomingMessage, ServerResponse } from 'node:http'; + +function createTransport(): Http.StreamableHTTPServerTransport { + return new Http.StreamableHTTPServerTransport({ sessionIdGenerator: () => 'strict-transport-test' }); +} + +async function connect(server: ServerModule.Server, transport: Http.StreamableHTTPServerTransport): Promise { + await server.connect(transport); +} + +function handle(transport: Http.StreamableHTTPServerTransport, request: IncomingMessage, response: ServerResponse): Promise { + return transport.handleRequest(request, response); +} + +function configure(transport: Transport): void { + transport.onclose = () => {}; + transport.onerror = error => { + error.message.toUpperCase(); + }; + transport.onmessage = (message, extra) => { + message.jsonrpc.toUpperCase(); + extra?.authInfo?.token.toUpperCase(); + }; + transport.onclose = undefined; + transport.onerror = undefined; + transport.onmessage = undefined; +} + +function headers(input: Parameters[0]): Record { + return TransportModule.normalizeHeaders(input); +} + +export = { createTransport, connect, handle, configure, headers }; diff --git a/scripts/fixtures/transport-types/consumer.mts b/scripts/fixtures/transport-types/consumer.mts new file mode 100644 index 0000000000..ea60ed655d --- /dev/null +++ b/scripts/fixtures/transport-types/consumer.mts @@ -0,0 +1,64 @@ +import { Server } from '@modelcontextprotocol/sdk/server/index.js'; +import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js'; +import { normalizeHeaders } from '@modelcontextprotocol/sdk/shared/transport.js'; +import type { Transport } from '@modelcontextprotocol/sdk/shared/transport.js'; +import type { JSONRPCMessage, MessageExtraInfo } from '@modelcontextprotocol/sdk/types.js'; +import type { IncomingMessage, ServerResponse } from 'node:http'; + +export function createTransport(): StreamableHTTPServerTransport { + return new StreamableHTTPServerTransport({ sessionIdGenerator: () => 'strict-transport-test' }); +} + +export async function connect(server: Server, transport: StreamableHTTPServerTransport): Promise { + await server.connect(transport); +} + +export function handle(transport: StreamableHTTPServerTransport, request: IncomingMessage, response: ServerResponse): Promise { + return transport.handleRequest(request, response); +} + +export function configure(transport: Transport): void { + transport.onclose = () => {}; + transport.onerror = (error: Error) => { + error.message.toUpperCase(); + }; + transport.onmessage = (message: T, extra?: MessageExtraInfo) => { + const sameMessage: T = message; + sameMessage.jsonrpc.toUpperCase(); + extra?.authInfo?.token.toUpperCase(); + }; + transport.onclose = undefined; + transport.onerror = undefined; + transport.onmessage = undefined; +} + +export function configureNode(transport: StreamableHTTPServerTransport): void { + configure(transport); + transport.onclose = undefined; + transport.onerror = undefined; + transport.onmessage = undefined; + const sessionId: string | undefined = transport.sessionId; + sessionId?.toUpperCase(); +} + +export const withoutCallbacks: Transport = { + async start() {}, + async send() {}, + async close() {} +}; + +export const clearedCallbacks: Transport = { + ...withoutCallbacks, + onclose: undefined, + onerror: undefined, + onmessage: undefined, + sessionId: undefined +}; + +export function headers(input: Parameters[0]): Record { + normalizeHeaders(new Headers({ authorization: 'example' })); + normalizeHeaders([['authorization', 'example']]); + normalizeHeaders({ authorization: 'example' }); + normalizeHeaders(undefined); + return normalizeHeaders(input); +} diff --git a/scripts/fixtures/transport-types/downstream.cts b/scripts/fixtures/transport-types/downstream.cts new file mode 100644 index 0000000000..c221204f25 --- /dev/null +++ b/scripts/fixtures/transport-types/downstream.cts @@ -0,0 +1,13 @@ +import ServerModule = require('@modelcontextprotocol/sdk/server/index.js'); +import consumer = require('./consumer.cjs'); +import type { Transport } from '@modelcontextprotocol/sdk/shared/transport.js'; + +async function check(): Promise { + const transport = consumer.createTransport(); + const contract: Transport = transport; + consumer.configure(contract); + consumer.headers({ authorization: 'example' }); + await consumer.connect(new ServerModule.Server({ name: 'declaration-consumer', version: '0' }), transport); +} + +export = check; diff --git a/scripts/fixtures/transport-types/downstream.mts b/scripts/fixtures/transport-types/downstream.mts new file mode 100644 index 0000000000..64086ec668 --- /dev/null +++ b/scripts/fixtures/transport-types/downstream.mts @@ -0,0 +1,10 @@ +import { Server } from '@modelcontextprotocol/sdk/server/index.js'; +import type { Transport } from '@modelcontextprotocol/sdk/shared/transport.js'; +import { configure, configureNode, connect, createTransport, headers } from './consumer.mjs'; + +const transport = createTransport(); +const contract: Transport = transport; +configure(contract); +configureNode(transport); +headers({ authorization: 'example' }); +await connect(new Server({ name: 'declaration-consumer', version: '0' }), transport); diff --git a/scripts/test-transport-types.ts b/scripts/test-transport-types.ts new file mode 100644 index 0000000000..6d4c966ff3 --- /dev/null +++ b/scripts/test-transport-types.ts @@ -0,0 +1,150 @@ +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { copyFileSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const [profileArgument, compilerArgument] = process.argv.slice(2); +assert(profileArgument && compilerArgument, 'Usage: tsx scripts/test-transport-types.ts '); +const profile = resolve(profileArgument); +const compiler = resolve(compilerArgument); +assert(existsSync(compiler), `Compiler does not exist: ${compiler}`); +const packageDirectory = join(profile, 'node_modules', '@modelcontextprotocol', 'sdk'); +assert(existsSync(join(packageDirectory, 'dist', 'esm', 'server', 'streamableHttp.d.ts')), 'Install the SDK package in the profile first'); +const fixtures = fileURLToPath(new URL('./fixtures/transport-types/', import.meta.url)); + +const negativeCases = [ + { source: 'contract.onclose = 1;', code: 2322 }, + { source: 'contract.onclose = (required: string) => {};', code: 2322 }, + { source: 'contract.onerror = (error: string) => {};', code: 2322 }, + { source: 'contract.onmessage = (message: string) => {};', code: 2322 }, + { source: 'contract.onmessage = (message: JSONRPCMessage, extra: string) => {};', code: 2322 }, + { source: 'contract.onmessage = (message: JSONRPCRequest) => {};', code: 2322 }, + { source: 'contract.onclose = null;', code: 2322 }, + { source: 'contract.onerror = null;', code: 2322 }, + { source: 'contract.onmessage = null;', code: 2322 }, + { source: 'contract.onerror!();', code: 2554 }, + { source: 'contract.onmessage!();', code: 2554 }, + { source: 'contract.onerror!("not-an-error");', code: 2345 }, + { source: 'contract.onmessage!({ jsonrpc: "invalid", method: "ping" });', code: 2322 }, + { source: 'transport.onerror = (error: string) => {};', code: 2322 }, + { source: 'transport.onmessage = (message: string) => {};', code: 2322 }, + { source: 'transport.onclose = null;', code: 2322 }, + { source: 'normalizeHeaders(42);', code: 2345 }, + { source: 'normalizeHeaders({ authorization: ["example"] });', code: 2345 }, + { source: 'normalizeHeaders([["name"]]);', code: 2322 } +]; + +function compile(directory: string): { status: number | null; output: string } { + const result = spawnSync(process.execPath, [compiler, '--project', join(directory, 'tsconfig.json'), '--pretty', 'false'], { + encoding: 'utf8', + timeout: 120_000 + }); + if (result.error) throw result.error; + return { status: result.status, output: result.stdout + result.stderr }; +} + +function checkConsumers(mode: 'esm' | 'cjs', lib: string[]): void { + const extension = mode === 'esm' ? 'mts' : 'cts'; + const httpImport = + mode === 'esm' + ? "import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js';" + : "import Http = require('@modelcontextprotocol/sdk/server/streamableHttp.js'); const { StreamableHTTPServerTransport } = Http;"; + const headersImport = + mode === 'esm' + ? "import { normalizeHeaders } from '@modelcontextprotocol/sdk/shared/transport.js';" + : "import TransportModule = require('@modelcontextprotocol/sdk/shared/transport.js'); const { normalizeHeaders } = TransportModule;"; + const directory = mkdtempSync(join(profile, 'transport-types-')); + try { + const compilerOptions = { + target: 'ES2022', + module: 'NodeNext', + moduleResolution: 'NodeNext', + lib, + types: ['node'], + strict: true, + noImplicitAny: true, + exactOptionalPropertyTypes: true, + noUncheckedIndexedAccess: true, + verbatimModuleSyntax: true, + skipLibCheck: false, + declaration: true, + emitDeclarationOnly: true, + outDir: './declarations' + }; + const config = join(directory, 'tsconfig.json'); + const files = [`consumer.${extension}`]; + if (lib.includes('DOM')) { + files.push('headers-dom.mts'); + writeFileSync( + join(directory, 'headers-dom.mts'), + [ + "import { normalizeHeaders } from '@modelcontextprotocol/sdk/shared/transport.js';", + 'type Input = Parameters[0];', + 'type Original = HeadersInit | undefined;', + 'type Equal = (() => T extends A ? 1 : 2) extends (() => T extends B ? 1 : 2) ? true : false;', + 'const unchangedDomain: Equal = true;', + 'export { unchangedDomain };' + ].join('\n') + ); + } + writeFileSync(config, JSON.stringify({ compilerOptions, files })); + copyFileSync(join(fixtures, `consumer.${extension}`), join(directory, `consumer.${extension}`)); + let result = compile(directory); + assert.equal(result.status, 0, `${lib.join(',')}: public consumer failed\n${result.output}`); + const declaration = join(directory, 'declarations', `consumer.d.${extension}`); + assert(existsSync(declaration), 'Consumer declaration was not emitted'); + assert(readFileSync(declaration, 'utf8').includes('@modelcontextprotocol/sdk/server/streamableHttp.js')); + + rmSync(join(directory, `consumer.${extension}`)); + const downstream = join(directory, 'downstream'); + mkdirSync(downstream); + copyFileSync(declaration, join(downstream, `consumer.d.${extension}`)); + copyFileSync(join(fixtures, `downstream.${extension}`), join(downstream, `downstream.${extension}`)); + writeFileSync( + config, + JSON.stringify({ compilerOptions: { ...compilerOptions, noEmit: true }, files: [`downstream/downstream.${extension}`] }) + ); + result = compile(directory); + assert.equal(result.status, 0, `${lib.join(',')}: declaration-only downstream failed\n${result.output}`); + assert(!existsSync(join(directory, `consumer.${extension}`)), 'First consumer source must not be available'); + + const negativeFiles = negativeCases.map((_, index) => `negative-${index}.${extension}`); + writeFileSync(config, JSON.stringify({ compilerOptions: { ...compilerOptions, noEmit: true }, files: negativeFiles })); + for (const [index, { source }] of negativeCases.entries()) { + writeFileSync( + join(directory, `negative-${index}.${extension}`), + [ + httpImport, + headersImport, + "import type { Transport } from '@modelcontextprotocol/sdk/shared/transport.js';", + "import type { JSONRPCMessage, JSONRPCRequest } from '@modelcontextprotocol/sdk/types.js';", + 'const transport = new StreamableHTTPServerTransport();', + 'const contract: Transport = transport;', + source + ].join('\n') + ); + } + result = compile(directory); + assert.notEqual(result.status, 0, 'Invalid callback/header types were accepted'); + assert.equal([...result.output.matchAll(/error TS\d+:/g)].length, negativeCases.length, result.output); + for (const [index, { source, code }] of negativeCases.entries()) { + assert.match( + result.output, + new RegExp(`negative-${index}\\.${extension}\\(7,\\d+\\): error TS${code}:`), + `Expected diagnostic at the invalid expression: ${source}\n${result.output}` + ); + } + console.log( + `${process.version}: ${mode}: ${lib.join(',')}: public consumer, declaration roundtrip, ${negativeCases.length} negative cases passed` + ); + } finally { + rmSync(directory, { recursive: true, force: true }); + } +} + +for (const mode of ['esm', 'cjs'] as const) { + for (const lib of [['ES2022'], ['ES2022', 'DOM', 'DOM.Iterable']]) { + checkConsumers(mode, lib); + } +} diff --git a/src/shared/transport.ts b/src/shared/transport.ts index f9b21bed32..2d757a455c 100644 --- a/src/shared/transport.ts +++ b/src/shared/transport.ts @@ -6,7 +6,7 @@ export type FetchLike = (url: string | URL, init?: RequestInit) => Promise for manipulation. * Handles Headers objects, arrays of tuples, and plain objects. */ -export function normalizeHeaders(headers: HeadersInit | undefined): Record { +export function normalizeHeaders(headers: Headers | [string, string][] | Record | undefined): Record { if (!headers) return {}; if (headers instanceof Headers) { @@ -98,14 +98,14 @@ export interface Transport { * * This should be invoked when close() is called as well. */ - onclose?: () => void; + onclose?: (() => void) | undefined; /** * Callback for when an error occurs. * * Note that errors are not necessarily fatal; they are used for reporting any kind of exceptional condition out of band. */ - onerror?: (error: Error) => void; + onerror?: ((error: Error) => void) | undefined; /** * Callback for when a message (request or response) is received over the connection. @@ -114,12 +114,12 @@ export interface Transport { * * The requestInfo can be used to get the original request information (headers, etc.) */ - onmessage?: (message: T, extra?: MessageExtraInfo) => void; + onmessage?: ((message: T, extra?: MessageExtraInfo) => void) | undefined; /** * The session ID generated for this connection. */ - sessionId?: string; + sessionId?: string | undefined; /** * Sets the protocol version used for the connection (called when the initialize response is received). diff --git a/test/server/streamableHttpCallbacks.test.ts b/test/server/streamableHttpCallbacks.test.ts new file mode 100644 index 0000000000..7ebd1dd703 --- /dev/null +++ b/test/server/streamableHttpCallbacks.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it, vi } from 'vitest'; +import { StreamableHTTPServerTransport } from '../../src/server/streamableHttp.js'; +import type { Transport } from '../../src/shared/transport.js'; + +describe('StreamableHTTPServerTransport callbacks', () => { + it('allows callbacks to be installed and cleared through the transport interface', async () => { + const nodeTransport = new StreamableHTTPServerTransport({ sessionIdGenerator: undefined }); + const transport: Transport = nodeTransport; + const onclose = vi.fn(); + const onerror = vi.fn(); + const onmessage = vi.fn(); + + try { + expect(transport.sessionId).toBeUndefined(); + expect(transport.onclose).toBeUndefined(); + expect(transport.onerror).toBeUndefined(); + expect(transport.onmessage).toBeUndefined(); + + transport.onclose = onclose; + transport.onerror = onerror; + transport.onmessage = onmessage; + expect(nodeTransport.onclose).toBe(onclose); + expect(nodeTransport.onerror).toBe(onerror); + expect(nodeTransport.onmessage).toBe(onmessage); + + transport.onclose = undefined; + transport.onerror = undefined; + transport.onmessage = undefined; + expect(nodeTransport.onclose).toBeUndefined(); + expect(nodeTransport.onerror).toBeUndefined(); + expect(nodeTransport.onmessage).toBeUndefined(); + } finally { + await transport.close(); + } + expect(onclose).not.toHaveBeenCalled(); + expect(onerror).not.toHaveBeenCalled(); + expect(onmessage).not.toHaveBeenCalled(); + }); +}); diff --git a/test/shared/transport.test.ts b/test/shared/transport.test.ts new file mode 100644 index 0000000000..dcd9abc645 --- /dev/null +++ b/test/shared/transport.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from 'vitest'; +import { normalizeHeaders } from '../../src/shared/transport.js'; + +describe('normalizeHeaders', () => { + it('accepts undefined', () => { + expect(normalizeHeaders(undefined)).toEqual({}); + }); + + it('accepts standard Headers', () => { + expect(normalizeHeaders(new Headers({ authorization: 'example' }))).toEqual({ authorization: 'example' }); + }); + + it('accepts tuples', () => { + expect(normalizeHeaders([['authorization', 'example']])).toEqual({ authorization: 'example' }); + }); + + it('copies string-valued records', () => { + const headers = { authorization: 'example' }; + expect(normalizeHeaders(headers)).toEqual(headers); + expect(normalizeHeaders(headers)).not.toBe(headers); + }); +});