diff --git a/CHANGELOG.md b/CHANGELOG.md index abc7825..3741ac8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,11 @@ All notable changes to this project are documented here. This project adheres to - `fetchWithRetry` rethrew `lastError`, typed `unknown`, so a non-`Error` rejection reached callers as something they could not read `.message` off. ### Added +- **Streamable HTTP transport**, alongside stdio. `MCP_HTTP_PORT=4320 node dist/http-server.js`. stdio remains the default and is untouched — every existing client is configured to spawn it. This is what a hosted instance needs, because a reverse proxy cannot front a process that talks over stdin/stdout. + + Each session gets its own server instance, built by the `createServer()` factory. Idle sessions are reaped (`MCP_SESSION_TTL_MS`, 30 min) and concurrency is capped (`MCP_MAX_SESSIONS`, 256), so a client that never sends `DELETE` cannot accumulate servers until the process dies. + + `MCP_HTTP_HOST` defaults to `127.0.0.1`, which also enables the SDK's DNS-rebinding protection. That default is deliberate: the Reactome origin has already been taken down once by crawlers on public `/ContentService/exporter/*` URLs, and an MCP endpoint is the same shape of risk and worse per request. - **A cap on how much text one tool may return** (`MAX_TOOL_RESPONSE_CHARS`, default 40,000; override by env). Every tool result is spent from the caller's context window, and the size of several of these is driven by the ID asked about rather than by anything the tool decides: `reactome_events_hierarchy` rendered ~86 KB (~22,000 tokens) in a single call and `reactome_query` on Metabolism ~60 KB. The cap is applied once, in the wrapper every tool handler already passes through, rather than in 56 places — and it announces the cut rather than truncating silently, because a model that cannot see it was truncated reports the partial answer as the whole one. - **Spec Kit.** `.specify/` with a constitution written from failures this repository actually had, and `specs/` for design decisions. Spec 001 records what was found about response shapes; spec 002 states the transport and hosting question rather than answering it. The Spec Kit skills under `.claude/skills/` are tracked deliberately — people clone this repository and point an agent at it. - **A live sweep.** `npm run sweep` calls all 53 tools against the live services. It checks for `undefined`, `[object Object]` and empty bodies, and — because marker-grepping cannot see a field that was dropped cleanly — asserts expected content for 16 tools whose arguments are known to return data. Runs weekly and on demand, not in CI, since a red run there can mean Reactome changed rather than this repository did. diff --git a/README.md b/README.md index 6bb34dc..8d6d7b7 100644 --- a/README.md +++ b/README.md @@ -259,6 +259,53 @@ Only registered when `NEO4J_URI` is set. Designed for curators running the [`rea | `reactome://top-pathways/{species}` | Top-level pathways for a species | | `reactome://events-hierarchy/{species}` | Full event hierarchy for a species | +## Transports + +**stdio** is the default and the one every existing client uses: + +```bash +node dist/index.js +``` + +**Streamable HTTP** is for a hosted instance, because a reverse proxy cannot +front a process that talks over stdin/stdout: + +```bash +MCP_HTTP_PORT=4320 node dist/http-server.js +# or: MCP_HTTP_PORT=4320 npm run start:http +``` + +| variable | default | | +|---|---|---| +| `MCP_HTTP_PORT` | *(unset)* | required to serve HTTP | +| `MCP_HTTP_HOST` | `127.0.0.1` | see below before changing | +| `MCP_SESSION_TTL_MS` | `1800000` | idle session reaped after 30 min | +| `MCP_MAX_SESSIONS` | `256` | concurrent session ceiling | + +Endpoints: `POST /mcp` (initialize, then requests), `GET /mcp` (server stream), +`DELETE /mcp` (end session), `GET /health`. + +Each session gets its **own** server instance, so two clients cannot interleave +on shared state. Idle sessions are reaped and the session count is capped, so a +client that never sends `DELETE` cannot accumulate servers until the process +dies. + +### Why it binds to loopback + +`MCP_HTTP_HOST` defaults to `127.0.0.1`, and that is a deliberate default rather +than a placeholder. The Reactome site already learned this the expensive way: +crawlers on the public `/ContentService/exporter/*` URLs exhausted Tomcat's heap +and took the origin down, which is why the sibling render service on that box +binds loopback only and is reached through the site's own origin. + +An MCP endpoint is the same shape of risk and worse per request — +`reactome_analyze_identifiers` submits a real job to the Analysis Service. Put +it behind something that rate-limits before binding it anywhere else. + +When the host is a loopback address the SDK also turns on DNS-rebinding +protection, which is what stops a page in someone's browser from driving a +server bound to their own machine. Binding to `0.0.0.0` turns that off. + ## Development ```bash diff --git a/package-lock.json b/package-lock.json index 006eb20..ee6e752 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,6 +10,7 @@ "license": "Apache-2.0", "dependencies": { "@modelcontextprotocol/sdk": "^1.30.0", + "express": "^5.2.1", "neo4j-driver": "^6.2.0", "zod": "^4.6.5" }, @@ -18,6 +19,7 @@ }, "devDependencies": { "@eslint/js": "^10.0.1", + "@types/express": "^5.0.6", "@types/node": "^26.5.1", "@vitest/coverage-v8": "^5.0.0", "eslint": "^10.10.0", @@ -721,6 +723,17 @@ "license": "MIT", "peer": true }, + "node_modules/@types/body-parser": { + "version": "1.19.6", + "resolved": "https://registry.npmjs.org/@types/body-parser/-/body-parser-1.19.6.tgz", + "integrity": "sha512-HLFeCYgz89uk22N5Qg3dvGvsv46B8GLvKKo1zKG4NybA8U2DiEO3w9lqGg29t/tfLRJpJ6iQxnVw4OnB7MoM9g==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/connect": "*", + "@types/node": "*" + } + }, "node_modules/@types/chai": { "version": "5.2.3", "resolved": "https://registry.npmjs.org/@types/chai/-/chai-5.2.3.tgz", @@ -732,6 +745,16 @@ "assertion-error": "^2.0.1" } }, + "node_modules/@types/connect": { + "version": "3.4.38", + "resolved": "https://registry.npmjs.org/@types/connect/-/connect-3.4.38.tgz", + "integrity": "sha512-K6uROf1LD88uDQqJCktA4yzL1YYAK6NgfsI0v/mTgyPKWsX1CnJ0XPSDhViejru1GcRkLWb8RlzFYJRqGUbaug==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/node": "*" + } + }, "node_modules/@types/deep-eql": { "version": "4.0.2", "resolved": "https://registry.npmjs.org/@types/deep-eql/-/deep-eql-4.0.2.tgz", @@ -753,6 +776,38 @@ "dev": true, "license": "MIT" }, + "node_modules/@types/express": { + "version": "5.0.6", + "resolved": "https://registry.npmjs.org/@types/express/-/express-5.0.6.tgz", + "integrity": "sha512-sKYVuV7Sv9fbPIt/442koC7+IIwK5olP1KWeD88e/idgoJqDm3JV/YUiPwkoKK92ylff2MGxSz1CSjsXelx0YA==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/body-parser": "*", + "@types/express-serve-static-core": "^5.0.0", + "@types/serve-static": "^2" + } + }, + "node_modules/@types/express-serve-static-core": { + "version": "5.1.3", + "resolved": "https://registry.npmjs.org/@types/express-serve-static-core/-/express-serve-static-core-5.1.3.tgz", + "integrity": "sha512-dPfW8NFiOF4wOHc7+N/QSxlY9cfSsenewGbAz8C8U/MULPd/YZ27LvJUIlzaXie7e6Ove9YunJGgC9tbHD2cKw==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/node": "*", + "@types/qs": "*", + "@types/range-parser": "*", + "@types/send": "*" + } + }, + "node_modules/@types/http-errors": { + "version": "2.0.5", + "resolved": "https://registry.npmjs.org/@types/http-errors/-/http-errors-2.0.5.tgz", + "integrity": "sha512-r8Tayk8HJnX0FztbZN7oVqGccWgw98T/0neJphO91KkmOzug1KkofZURD4UaD5uH8AqcFLfdPErnBod0u71/qg==", + "dev": true, + "license": "MIT" + }, "node_modules/@types/json-schema": { "version": "7.0.15", "resolved": "https://registry.npmjs.org/@types/json-schema/-/json-schema-7.0.15.tgz", @@ -770,6 +825,41 @@ "undici-types": "~8.9.0" } }, + "node_modules/@types/qs": { + "version": "6.15.1", + "resolved": "https://registry.npmjs.org/@types/qs/-/qs-6.15.1.tgz", + "integrity": "sha512-GZHUBZR9hckSUhrxmp1nG6NwdpM9fCunJwyThLW1X3AyHgd9IlHb6VANpQQqDr2o/qQp6McZ3y/IA2rVzKzSbw==", + "dev": true, + "license": "MIT" + }, + "node_modules/@types/range-parser": { + "version": "1.2.7", + "resolved": "https://registry.npmjs.org/@types/range-parser/-/range-parser-1.2.7.tgz", + "integrity": "sha512-hKormJbkJqzQGhziax5PItDUTMAM9uE2XXQmM37dyd4hVM+5aVl7oVxMVUiVQn2oCQFN/LKCZdvSM0pFRqbSmQ==", + "dev": true, + "license": "MIT" + }, + "node_modules/@types/send": { + "version": "1.2.1", + "resolved": "https://registry.npmjs.org/@types/send/-/send-1.2.1.tgz", + "integrity": "sha512-arsCikDvlU99zl1g69TcAB3mzZPpxgw0UQnaHeC1Nwb015xp8bknZv5rIfri9xTOcMuaVgvabfIRA7PSZVuZIQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/node": "*" + } + }, + "node_modules/@types/serve-static": { + "version": "2.2.0", + "resolved": "https://registry.npmjs.org/@types/serve-static/-/serve-static-2.2.0.tgz", + "integrity": "sha512-8mam4H1NHLtu7nmtalF7eyBH14QyOASmcxHhSfEoRyr0nP/YdoesEtU+uSRvMe96TW/HPTtkoKqQLl53N7UXMQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/http-errors": "*", + "@types/node": "*" + } + }, "node_modules/@typescript-eslint/scope-manager": { "version": "8.70.0", "resolved": "https://registry.npmjs.org/@typescript-eslint/scope-manager/-/scope-manager-8.70.0.tgz", diff --git a/package.json b/package.json index 7cdea6c..e574dee 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,8 @@ "type": "module", "main": "dist/index.js", "bin": { - "reactome-mcp": "dist/index.js" + "reactome-mcp": "dist/index.js", + "reactome-mcp-http": "dist/http-server.js" }, "files": [ "dist/", @@ -30,7 +31,8 @@ "test:coverage": "vitest run --coverage", "sweep": "node scripts/sweep-live.mjs", "check": "npm run lint && npm run format:check && npm run typecheck && npm run typecheck:lint-compiler && npm run build && npm test", - "typecheck:lint-compiler": "node node_modules/typescript/bin/tsc --noEmit -p tsconfig.eslint.json" + "typecheck:lint-compiler": "node node_modules/typescript/bin/tsc --noEmit -p tsconfig.eslint.json", + "start:http": "node dist/http-server.js" }, "keywords": [ "mcp", @@ -43,11 +45,13 @@ "license": "Apache-2.0", "dependencies": { "@modelcontextprotocol/sdk": "^1.30.0", + "express": "^5.2.1", "neo4j-driver": "^6.2.0", "zod": "^4.6.5" }, "devDependencies": { "@eslint/js": "^10.0.1", + "@types/express": "^5.0.6", "@types/node": "^26.5.1", "@vitest/coverage-v8": "^5.0.0", "eslint": "^10.10.0", diff --git a/specs/002-transport-and-hosting/spec.md b/specs/002-transport-and-hosting/spec.md index a7e1693..b53d1b3 100644 --- a/specs/002-transport-and-hosting/spec.md +++ b/specs/002-transport-and-hosting/spec.md @@ -1,6 +1,6 @@ # 002 — Transports and hosting -**Status:** open — the design question is stated here, not settled +**Status:** transport settled 2026-09-14; where it runs is still open **Date:** 2026-09-14 **Constitution:** Principles IV, V @@ -30,11 +30,46 @@ can only make the calls a browser can. A test asserts the gate holds. **Analysis runs in the Analysis Service.** The server submits identifiers, holds the token, and formats the reply. +## Settled 2026-09-14: the transport exists + +Streamable HTTP ships alongside stdio. stdio stays the default and is unchanged. + + - `MCP_HTTP_PORT=4320 node dist/http-server.js` + - `POST /mcp`, `GET /mcp`, `DELETE /mcp`, `GET /health` + - one server per session, built by `createServer()` + - idle sessions reaped at 30 min, concurrency capped at 256 + - binds `127.0.0.1` unless told otherwise, which also turns on the SDK's + DNS-rebinding protection + +This was the blocker for everything below: a reverse proxy cannot front a +process that speaks over stdin/stdout, so there was previously nothing to host. + +### The brief for whoever adds it to the website repo + +That repository already runs this exact pattern. `render` is a sibling Node +service in the same compose file, bound loopback-only, reached through the +site's own origin; `serve-prod.js` reads the proxy table from `proxy.conf.js`, +so beta and the dev server both proxy it the same way; and +`deploy/apache/beta-chat-proxy.conf` shows how a service gets a path on the +beta vhost. + +So the work is three small things, not a design exercise: + + 1. a compose service running `dist/http-server.js` with `MCP_HTTP_PORT` set, + published on `127.0.0.1` only -- copy what `render` does + 2. an entry in `proxy.conf.js` so the origin forwards a path to it + 3. an Apache stanza on beta, modelled on `beta-chat-proxy.conf` + +**Loopback only to begin with.** Not because public access is wrong, but +because it is a separate decision that needs rate limiting attached, and the +comments on the render service record why: crawlers on the public +`/ContentService/exporter/*` URLs exhausted Tomcat's heap and took the origin +down. An MCP endpoint is the same shape of risk and worse per request -- +`reactome_analyze_identifiers` submits a real job to the Analysis Service. + ## What is open -1. **Who adds Streamable HTTP, and when.** The SDK provides - `StreamableHTTPServerTransport`. The work is small; the operational - commitment is not. +1. ~~**Who adds Streamable HTTP, and when.**~~ *Settled: it is in, see above.* 2. **Where a hosted instance runs.** Spinning it up alongside the Angular website has been raised. That would put it behind infrastructure that already @@ -42,7 +77,9 @@ the token, and formats the reply. 3. **Whether it is public.** A public endpoint needs rate limiting, abuse handling, and an answer for what happens when Reactome's own services are - slow — this server would become a new way to load them. + slow — this server would become a new way to load them. The default binding + makes not-public the path of least resistance, which is the right way round + for a decision of this shape. 4. **npm publishing.** Deferred by decision, to be settled in one pass with the website and the other Reactome repositories rather than piecemeal. diff --git a/src/config.ts b/src/config.ts index da83915..5887603 100644 --- a/src/config.ts +++ b/src/config.ts @@ -79,3 +79,29 @@ export const MAX_TOOL_RESPONSE_CHARS = parsePositiveInt( process.env.MAX_TOOL_RESPONSE_CHARS, 40_000 ); + +/** + * HTTP transport. Off unless a port is set -- stdio stays the default, because + * that is what every existing user's client is configured for. + * + * The host defaults to loopback deliberately. The Reactome site already learned + * this lesson the expensive way: crawlers on the public `/ContentService/ + * exporter/*` URLs exhausted Tomcat's heap and took the origin down, which is + * why the sibling render service on that box binds 127.0.0.1 only and is + * reached through the site's own origin. An MCP endpoint is the same shape of + * risk and worse per request -- `reactome_analyze_identifiers` submits a real + * job to the Analysis Service. + * + * Binding elsewhere is possible and deliberate: set MCP_HTTP_HOST. Do that + * behind something that rate-limits. + */ +export const MCP_HTTP_PORT = process.env.MCP_HTTP_PORT + ? parsePositiveInt(process.env.MCP_HTTP_PORT, 0) + : undefined; +export const MCP_HTTP_HOST = process.env.MCP_HTTP_HOST ?? "127.0.0.1"; + +/** How long an idle session is kept before its server is torn down. */ +export const MCP_SESSION_TTL_MS = parsePositiveInt(process.env.MCP_SESSION_TTL_MS, 30 * 60_000); + +/** Ceiling on concurrent sessions, so a client loop cannot exhaust memory. */ +export const MCP_MAX_SESSIONS = parsePositiveInt(process.env.MCP_MAX_SESSIONS, 256); diff --git a/src/http-server.ts b/src/http-server.ts new file mode 100644 index 0000000..e1ea0f2 --- /dev/null +++ b/src/http-server.ts @@ -0,0 +1,43 @@ +#!/usr/bin/env node +/** + * HTTP entrypoint. `src/index.ts` remains the stdio one and is unchanged -- + * every existing user has a client configured to spawn it, and that must keep + * working exactly as before. + * + * MCP_HTTP_PORT=4320 node dist/http-server.js + * + * Binds 127.0.0.1 unless MCP_HTTP_HOST says otherwise. See config.ts for why + * that default is what it is. + */ +import { startHttpServer } from "./http.js"; +import { logger } from "./logger.js"; +import { MCP_HTTP_PORT, MCP_HTTP_HOST, NEO4J_URI } from "./config.js"; +import { fetchGraphSchema } from "./graph/schema.js"; + +const port = MCP_HTTP_PORT; + +if (!port) { + logger.error("MCP_HTTP_PORT is not set", { + hint: "Set MCP_HTTP_PORT to serve over HTTP, or run dist/index.js for stdio.", + }); + process.exit(1); +} + +startHttpServer(port, MCP_HTTP_HOST) + .then(() => { + // Same warm-up the stdio entrypoint does: apoc.meta.schema() samples 3M + // nodes and takes 15-30s, so the first caller should not pay for it. + if (NEO4J_URI) { + fetchGraphSchema().catch((err: unknown) => { + logger.warn("graph schema prefetch failed; will retry on first use", { + error: err instanceof Error ? err.message : String(err), + }); + }); + } + }) + .catch((error: unknown) => { + logger.error("fatal error starting http server", { + error: error instanceof Error ? error.message : String(error), + }); + process.exit(1); + }); diff --git a/src/http.ts b/src/http.ts new file mode 100644 index 0000000..1455de2 --- /dev/null +++ b/src/http.ts @@ -0,0 +1,206 @@ +import { randomUUID } from "node:crypto"; +import type { Server } from "node:http"; +import type { Request, Response } from "express"; +import express from "express"; +import { createMcpExpressApp } from "@modelcontextprotocol/sdk/server/express.js"; +import { StreamableHTTPServerTransport } from "@modelcontextprotocol/sdk/server/streamableHttp.js"; +import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import { createServer, SERVER_NAME, SERVER_VERSION } from "./server.js"; +import { logger } from "./logger.js"; +import { + MCP_HTTP_HOST, + MCP_MAX_SESSIONS, + MCP_SESSION_TTL_MS, + CONTENT_SERVICE_URL, + ANALYSIS_SERVICE_URL, + NEO4J_URI, +} from "./config.js"; + +interface Session { + transport: StreamableHTTPServerTransport; + server: McpServer; + lastSeen: number; +} + +/** + * Serve MCP over Streamable HTTP. + * + * stdio remains the default and is untouched: every existing user has a client + * configured to spawn this process. This is the transport a hosted instance + * needs, because a reverse proxy cannot front a process that talks over + * stdin/stdout. + * + * Each session gets its **own** server instance. Sharing one across sessions + * would mean one client's in-flight request and another's could interleave on + * shared per-connection state; `createServer()` exists precisely so that + * building one per session is cheap. + */ +export function startHttpServer(port: number, host: string = MCP_HTTP_HOST): Promise { + // Defaults to 127.0.0.1 and turns on DNS-rebinding protection for localhost + // hosts, which is what stops a web page in the user's browser from driving + // this server. + const app = createMcpExpressApp({ host }); + app.use(express.json({ limit: "4mb" })); + + const sessions = new Map(); + + const closeSession = (sessionId: string, why: string) => { + const session = sessions.get(sessionId); + if (!session) return; + sessions.delete(sessionId); + logger.info("mcp session closed", { sessionId, why, open: sessions.size }); + void Promise.resolve(session.transport.close()).catch(() => { + // Already gone; nothing useful left to do. + }); + }; + + // Sessions that are never closed by the client would otherwise accumulate a + // server each until the process dies. + const reaper = setInterval(() => { + const cutoff = Date.now() - MCP_SESSION_TTL_MS; + for (const [id, session] of sessions) { + if (session.lastSeen < cutoff) closeSession(id, "idle"); + } + }, 60_000); + reaper.unref(); + + const existing = (req: Request): Session | undefined => { + const id = req.header("mcp-session-id"); + if (!id) return undefined; + const session = sessions.get(id); + if (session) session.lastSeen = Date.now(); + return session; + }; + + app.post("/mcp", (req: Request, res: Response) => { + void (async () => { + const session = existing(req); + if (session) { + await session.transport.handleRequest(req, res, req.body); + return; + } + + // No session header: this must be an initialize request, which is the + // only thing allowed to create one. + const body: unknown = req.body; + const isInitialize = + typeof body === "object" && + body !== null && + (body as { method?: unknown }).method === "initialize"; + + if (!isInitialize) { + res.status(400).json({ + jsonrpc: "2.0", + error: { code: -32000, message: "No valid session. Send initialize first." }, + id: null, + }); + return; + } + + if (sessions.size >= MCP_MAX_SESSIONS) { + // Refuse rather than let a client loop exhaust memory. 503 tells a + // proxy this is load, not a malformed request. + logger.warn("mcp session limit reached", { open: sessions.size }); + res.status(503).json({ + jsonrpc: "2.0", + error: { code: -32000, message: "Server at session capacity. Retry shortly." }, + id: null, + }); + return; + } + + const transport = new StreamableHTTPServerTransport({ + sessionIdGenerator: () => randomUUID(), + onsessioninitialized: (sessionId: string) => { + sessions.set(sessionId, { transport, server, lastSeen: Date.now() }); + logger.info("mcp session opened", { sessionId, open: sessions.size }); + }, + onsessionclosed: (sessionId: string) => closeSession(sessionId, "client closed"), + }); + + const server = createServer(); + transport.onclose = () => { + if (transport.sessionId) closeSession(transport.sessionId, "transport closed"); + }; + + await server.connect(transport); + await transport.handleRequest(req, res, req.body); + })().catch((error: unknown) => { + logger.error("mcp post failed", { + error: error instanceof Error ? error.message : String(error), + }); + if (!res.headersSent) { + res.status(500).json({ + jsonrpc: "2.0", + error: { code: -32603, message: "Internal server error" }, + id: null, + }); + } + }); + }); + + // GET opens the server-to-client stream; DELETE ends the session. Both + // require an established session. + const requireSession = (req: Request, res: Response): Session | undefined => { + const session = existing(req); + if (!session) { + res.status(400).json({ + jsonrpc: "2.0", + error: { code: -32000, message: "Unknown or missing mcp-session-id" }, + id: null, + }); + return undefined; + } + return session; + }; + + app.get("/mcp", (req: Request, res: Response) => { + const session = requireSession(req, res); + if (!session) return; + void session.transport.handleRequest(req, res); + }); + + app.delete("/mcp", (req: Request, res: Response) => { + const session = requireSession(req, res); + if (!session) return; + void session.transport.handleRequest(req, res); + }); + + // For whatever fronts this. Deliberately says nothing about Reactome's own + // health: this endpoint reports that the process is up, not that the + // Content Service is. + app.get("/health", (_req: Request, res: Response) => { + res.json({ + status: "ok", + name: SERVER_NAME, + version: SERVER_VERSION, + sessions: sessions.size, + contentService: CONTENT_SERVICE_URL, + analysisService: ANALYSIS_SERVICE_URL, + neo4jEnabled: Boolean(NEO4J_URI), + }); + }); + + return new Promise((resolve, reject) => { + const http = app.listen(port, host, () => { + logger.info("reactome mcp http server listening", { + host, + port, + contentService: CONTENT_SERVICE_URL, + analysisService: ANALYSIS_SERVICE_URL, + neo4jEnabled: Boolean(NEO4J_URI), + }); + resolve(http); + }); + + http.on("error", reject); + + const shutdown = () => { + clearInterval(reaper); + for (const id of [...sessions.keys()]) closeSession(id, "shutdown"); + http.close(); + }; + process.once("SIGINT", shutdown); + process.once("SIGTERM", shutdown); + }); +} diff --git a/tests/http-transport.test.ts b/tests/http-transport.test.ts new file mode 100644 index 0000000..943d06d --- /dev/null +++ b/tests/http-transport.test.ts @@ -0,0 +1,195 @@ +/** + * Streamable HTTP transport. + * + * stdio cannot be put behind a reverse proxy, so a hosted instance needs this. + * These tests run a real server on an ephemeral port and speak real HTTP to it + * -- the session lifecycle is the part worth testing, and a mock of it would be + * testing the mock. + */ +import { describe, it, expect, beforeAll, afterAll } from "vitest"; +import { request as httpRequest, type Server } from "node:http"; +import type { AddressInfo } from "node:net"; +import { startHttpServer } from "../src/http.js"; + +const INIT = { + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: { + protocolVersion: "2024-11-05", + capabilities: {}, + clientInfo: { name: "vitest", version: "0" }, + }, +}; + +const JSON_HEADERS = { + "Content-Type": "application/json", + Accept: "application/json, text/event-stream", +}; + +describe("streamable http transport", () => { + let server: Server; + let base: string; + + beforeAll(async () => { + // Port 0: let the OS pick, so the suite cannot collide with whatever else + // is listening on this machine. One already did. + server = await startHttpServer(0, "127.0.0.1"); + const { port } = server.address() as AddressInfo; + base = `http://127.0.0.1:${port}`; + }); + + afterAll(() => { + server.close(); + }); + + async function openSession(): Promise { + const res = await fetch(`${base}/mcp`, { + method: "POST", + headers: JSON_HEADERS, + body: JSON.stringify(INIT), + }); + const sessionId = res.headers.get("mcp-session-id"); + if (!sessionId) throw new Error(`no session id; status ${res.status}`); + await fetch(`${base}/mcp`, { + method: "POST", + headers: { ...JSON_HEADERS, "mcp-session-id": sessionId }, + body: JSON.stringify({ jsonrpc: "2.0", method: "notifications/initialized" }), + }); + return sessionId; + } + + async function sessionCount(): Promise { + const res = await fetch(`${base}/health`); + const body = (await res.json()) as { sessions: number }; + return body.sessions; + } + + it("reports its own health without claiming Reactome is healthy", async () => { + const res = await fetch(`${base}/health`); + expect(res.status).toBe(200); + + const body = (await res.json()) as Record; + expect(body.status).toBe("ok"); + expect(body.name).toBe("reactome"); + // This says the process is up. It deliberately does not probe the Content + // Service, so a green health check never implies Reactome is well. + expect(body).toHaveProperty("contentService"); + }); + + it("issues a session id on initialize", async () => { + const res = await fetch(`${base}/mcp`, { + method: "POST", + headers: JSON_HEADERS, + body: JSON.stringify(INIT), + }); + + expect(res.status).toBe(200); + expect(res.headers.get("mcp-session-id")).toBeTruthy(); + }); + + it("refuses a request that carries no session", async () => { + const res = await fetch(`${base}/mcp`, { + method: "POST", + headers: JSON_HEADERS, + body: JSON.stringify({ jsonrpc: "2.0", id: 2, method: "tools/list" }), + }); + + // Only initialize may create a session; anything else without one is a + // client error, not a reason to make a new one. + expect(res.status).toBe(400); + }); + + it("refuses an unknown session id", async () => { + const res = await fetch(`${base}/mcp`, { + method: "GET", + headers: { Accept: "text/event-stream", "mcp-session-id": "not-a-real-session" }, + }); + + expect(res.status).toBe(400); + }); + + it("serves the full tool list over HTTP", async () => { + const sessionId = await openSession(); + const res = await fetch(`${base}/mcp`, { + method: "POST", + headers: { ...JSON_HEADERS, "mcp-session-id": sessionId }, + body: JSON.stringify({ jsonrpc: "2.0", id: 3, method: "tools/list" }), + }); + + const text = await res.text(); + const names = [...text.matchAll(/"name":"(reactome_[a-z_0-9]+)"/g)].map(m => m[1]); + + // The same surface stdio serves -- one registration path, two transports. + expect(new Set(names).size).toBeGreaterThan(40); + expect(names).toContain("reactome_search"); + }); + + it("gives each session its own server", async () => { + const before = await sessionCount(); + const a = await openSession(); + const b = await openSession(); + + expect(a).not.toBe(b); + expect(await sessionCount()).toBe(before + 2); + }); + + it("tears a session down on DELETE", async () => { + const sessionId = await openSession(); + const before = await sessionCount(); + + const res = await fetch(`${base}/mcp`, { + method: "DELETE", + headers: { "mcp-session-id": sessionId }, + }); + expect(res.status).toBeLessThan(400); + + expect(await sessionCount()).toBe(before - 1); + }); + + it("rejects a forged Host header", async () => { + // DNS rebinding protection: without it a page in the user's browser could + // drive a server bound to their loopback. + // + // Sent with node:http rather than fetch. `Host` is a forbidden header name, + // so fetch drops an override silently -- the first version of this test + // passed a Host it was never actually sending, and would have passed with + // the protection turned off. + const { port } = server.address() as AddressInfo; + const status = await new Promise((resolve, reject) => { + const req = httpRequest( + { + host: "127.0.0.1", + port, + path: "/health", + method: "GET", + headers: { Host: "evil.example.com" }, + }, + res => { + res.resume(); + resolve(res.statusCode ?? 0); + } + ); + req.on("error", reject); + req.end(); + }); + + expect(status).toBe(403); + }); + + it("accepts the real Host header", async () => { + // The mirror of the test above: proof the 403 is about the forged value, + // not about node:http requests in general. + const { port } = server.address() as AddressInfo; + const status = await new Promise((resolve, reject) => { + const req = httpRequest({ host: "127.0.0.1", port, path: "/health", method: "GET" }, res => { + res.resume(); + resolve(res.statusCode ?? 0); + }); + req.on("error", reject); + req.end(); + }); + + expect(status).toBe(200); + }); +});