diff --git a/docs/architecture.mermaid b/docs/architecture.mermaid index 94b31723e0..435460319f 100644 --- a/docs/architecture.mermaid +++ b/docs/architecture.mermaid @@ -1,8 +1,8 @@ graph TD subgraph "Entry Points" CLI["src/cli.ts + src/cli/
One-shot CLI adapter
setup, diagnostics, analysis, decompilation"] - MAIN["src/main.ts + src/main/
MCP stdio server adapter
config, session, process lifetime"] - DISPATCH["scripts/rea.mjs
Executable dispatcher
routes mcp / one-shot"] + MAIN["src/main.ts + src/main/
MCP server adapter: stdio default, streamable HTTP opt-in
config, session, process lifetime"] + DISPATCH["scripts/rea.mjs
Executable dispatcher
routes mcp / mcp --http / one-shot"] end subgraph "Production Composition" @@ -10,7 +10,7 @@ graph TD end subgraph "Adapters" - SERVER["src/server/
MCP request translation
createServer, tool registration, toolResult"] + SERVER["src/server/
MCP request translation
createServer, tool registration, toolResult
http/: streamable HTTP listener and node:http bridge"] HOPPER_BRIDGE["bridge/hopper_bridge.py
Runs inside Hopper
adapts operations to Hopper Python API"] GHIDRA_BRIDGE["bridge/ghidra/ReaGhidraBridge.java
Packaged HeadlessScript
inventory, decompiler, references, CFG"] INSTALL["src/cli/ + src/application/Setup*
Host installation adapters
macOS: verified vendor DMG to ~/Applications
Linux: verified vendor package + apt/dnf/pacman"] diff --git a/docs/cli.md b/docs/cli.md index cad2255d91..dcbd2cbb83 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -316,6 +316,28 @@ Choose the guide for the target and intended interaction: Runtime requests name the target and actions. Launched targets run with your user permissions; consult the chosen guide for host requirements and effects. +## Serve MCP over streamable HTTP + +`rea mcp` speaks stdio to the client that launched it. To serve the same MCP +catalog over streamable HTTP for one local client, such as an agent environment +runtime, start the dispatcher entry with `--http`: + +```sh +rea mcp --http --port 8765 +``` + +`--host` and `--port` default to `MCP_HOST` and `MCP_PORT` from the environment, +then to `127.0.0.1` and an ephemeral port. The bound URL is written to stderr as +one JSON log line (`"msg":"MCP transport listening"`) whatever `REA_LOG_LEVEL` +is set to; nothing is written to stdout. `rea mcp --http --help` lists the accepted options. The listener serves +one client at a time: a session ends with the client's `DELETE`, and a client +that connects while another session is open takes it over, so a client that +exited without ending its session needs no restart. The listener runs until +SIGINT or SIGTERM and does not stop when stdin closes. See +[transports](mcp-contracts.md#transports) for the session, `Host` header and +shutdown contract. Client registrations written by setup and the MCP registry +entry keep stdio. + ## Output and exit status `rea mcp doctor --json` checks production MCP startup and reports child logs in @@ -350,6 +372,10 @@ output formatting. | `1` | The operation could not complete. Structured output identifies invalid input, permissions, cancellation, timeouts or another failure when available. | | `128 + N` | Signal-derived status. An unhandled signal may have ended the process, with the shell or runtime preserving the conventional status; a command may instead have handled the signal, waited for its operation and cleanup to settle, and assigned the same status (`130` for SIGINT, `143` for SIGTERM). In the handled case, it may also have printed a structured `cancelled` result. | +`rea mcp --http` is long-running and prints no structured result. It exits +`130` on SIGINT or `143` on SIGTERM after closing the listener, the session and +its providers, and `1` when that cleanup fails. + A `cancelled` result does not by itself determine the exit status; it depends on the command's signal handling. Commands routed through the shared `withCommandCancellation` handler in the CLI set status `130` for SIGINT or diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index 3a4daed24f..011dc9826c 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -2,7 +2,8 @@ Use this reference to call REA tools and interpret their replies on one MCP connection. Start with [tool results](#tool-results), [retained Evidence](#retained-application-evidence-inputs), -and [session discovery](#identity-and-discovery). See [cancellation](#progress-and-cancellation) +and [session discovery](#identity-and-discovery). See [transports](#transports) +for the opt-in HTTP listener, [cancellation](#progress-and-cancellation) for partial-result recovery and [first-query deadlines](#ghidra-first-query-deadlines-and-recovery) when a cold provider exceeds the client's wait. @@ -443,6 +444,96 @@ inspection fails or is cancelled before a provider client exists. If no client remains, no provider run or health observation is available; follow the cleanup error's reported resources. +## Transports + +stdio is the default transport and the only one that client registrations, +`rea mcp doctor` and the MCP registry entry describe. `rea mcp --http [--host H] +[--port N]` is an opt-in alternative that serves the same catalog over the +streamable HTTP transport at `/mcp`; it replaces stdio for that process rather +than running beside it. Host and port come from the flags, then `MCP_HOST` and +`MCP_PORT`, then `127.0.0.1` and an ephemeral port. The bound URL is announced +as one JSON log line on stderr (`"msg":"MCP transport listening"` with `url`) +at every `REA_LOG_LEVEL`, because an ephemeral port is otherwise unreachable; +stdout stays unused. + +This endpoint uses plain HTTP without TLS or client authentication. Use it only +within a trusted local or isolated environment; an explicit non-loopback bind +makes the same tools and active analysis state reachable at that address. It +does not provide separate users or independent target sessions. Standard client +registrations continue to use a dedicated stdio process. + +The HTTP server is stateful and serves one client at a time on one server +instance. `initialize` returns an `Mcp-Session-Id`; `notifications/initialized` +is acknowledged with `202`; responses arrive as JSON or SSE frames; `DELETE +/mcp` with the session id ends the session with `200`. Ending a session is not +a server shutdown: the process keeps listening, the instance keeps its providers +and the shared active target, and the next client opens a fresh session against +them. A client ending or replacing a session cancels its running tool calls and +prompt completions, stops request-body reads and fences further message dispatch, +then waits up to ten seconds for request dispatch, handlers and cleanup to finish before reusing the +server. A timeout returns `503`; the old calls remain owned, and later requests +retry the drain rather than starting another session against those resources. +Requests that finish parsing or SDK dispatch after their session closed are +refused, and their response streams are cancelled. Tool callbacks whose +validation finishes after cancellation never start. +Body forwarding stops when SDK request processing ends, including early header +and size refusals, rather than remaining active until the session closes. +HTTP input/route dispatch is limited to eight concurrent requests by default, +shared by MCP and plain routes. Excess requests receive `503` with a closing +connection before their bodies are converted, read or cloned. An embedder can +select `maxConcurrentDispatches`; the SDK's per-body size limit still applies. +MCP responses with fully read or bodyless requests do not hold admission slots, +so an idle MCP SSE stream does not prevent later calls. Plain routes retain their +slots through response delivery and response-source cancellation cleanup. An +unread upload retains its slot until input finishes or either side closes, +even with a long response. Sequential keep-alive is supported; a peer pipelining +behind an unfinished response is disconnected before queued requests dispatch. +Concurrent MCP requests use separate connections. +This bounds concurrent input work, not total provider or process memory. +Provider-backed prompt completions use the same request owner and drain, so a +cancelled completion cannot keep using providers after handover or shutdown. +HTTP factories enable `RequestTrackingMcpServer` tracking so completion is tracked +even when cancellation suppresses the SDK's reply; stdio retains the SDK's +original callback behavior. Full server shutdown stops the listener and waits +for MCP work, plain-route handlers and response-source cancellation to settle +before running provider shutdown hooks or closing providers. Response producers +must finish provider work before closing their stream and expose asynchronous +disconnect cleanup through the stream's `cancel()` result. Cancellation does not +delay delivery of a HEAD reply; a caller retaining a sibling of a teed response +must consume or cancel that sibling before shutdown can finish. Failed response +cleanup remains owned, blocks further admission and prevents provider teardown. +The server continues cleanup +automatically when they finish, including after a client handover timed out. +All routes return `503` until the server factory succeeds; startup failure closes +the listener and joins its pending replies before rejecting the factory. +The newest client replaces an open session: an `initialize` that arrives +while a session is open ends that session instead of being refused, because a +client that exits without `DELETE` must not wedge the listener; the replaced +client's open streams are closed and its later requests are refused. Only an +`initialize` that carries the streamable headers (`Content-Type: +application/json` and an `Accept` naming both `application/json` and +`text/event-stream`) and passes the Host and Origin checks can replace a +session; any other sessionless POST goes to the live transport, which refuses +it without disturbing the session. Requests that carry no valid session id are +refused with `400` or `404` as the SDK transport defines. A bind is loopback by +address class, not spelling, and is classified from the address the listener +actually bound: any `127.0.0.0/8` address, `::1`, or an IPv4-mapped loopback. +Configured names, including `localhost` and its aliases, do not establish that +the bound address is local; names are added only after classifying the actual IP. +On such a bind, requests whose `Host` header is outside the SDK loopback allowlist plus +the configured name and the bound address are refused with `403`, and a browser +`Origin` outside that allowlist is refused the same way (a missing `Origin` +passes); a non-loopback bind relaxes both checks and logs the relaxation. A reply sent before a request body was read (a refused `Host` or `Origin`, an unknown route, the startup `503`) closes the connection after the reply, so an unread upload cannot keep a socket open. Request targets are taken verbatim as origin-form paths (`//host/path` is a path, not another origin); an absolute-form target, a target containing a raw fragment delimiter (`#`), or one whose path URL parsing would rewrite (dot segments such as `/public/../mcp`), is refused with `400`, and a method Fetch does not support with `405`. Percent-encoded `%23` remains valid query data. Only a lone `initialize` can replace an open session; a batch that carries one is left to the transport's refusal. Scoped +IPv6 literals (`fe80::1%eth0`) are refused before binding, because no URL can +name them. Listener errors reach the runtime's connection-loss handler only +after a successful bind; a bind failure prints the listen-failure message alone. Request bodies are bounded by the SDK's +4 MiB limit, and the response budget keeps its stdio-derived floor. The +listener ends on SIGINT or SIGTERM only, not on stdin EOF, and exits `130` or +`143` after cleanup settles. A signal that arrives while startup is still +running is recorded and acted on as soon as the listener and its providers are +owned, so startup never ends without cleanup; a second signal during startup +exits immediately. + ## Progress and cancellation REA accepts ordinary `tools/call` progress tokens. The numeric `progress` counts diff --git a/docs/specs/agent-environments/http-transport/plan.md b/docs/specs/agent-environments/http-transport/plan.md new file mode 100644 index 0000000000..c3dea3aaa1 --- /dev/null +++ b/docs/specs/agent-environments/http-transport/plan.md @@ -0,0 +1,32 @@ +# Implementation plan + +1. Widen the runtime `serve` dependency to a transport-neutral factory that may + bind asynchronously. `startMcpTransport` awaits it, hands it the server + logger, and keeps stdio as the default; the shutdown handle type loses its + stdio name. +2. Add `src/server/http/nodeHttpBridge.ts` to turn an IncomingMessage into a web + Request (headers, streamed body, abort on client close) and to pipe a web + Response back with flushed headers, and `src/server/http/serveStreamableHttp.ts` + to own one `McpServer` instance and one stateful transport per session, + ended by `DELETE` or replaced by the next client's `initialize` without + running the instance's shutdown hooks, plus plain routes, loopback Host + validation, the listening log line and idempotent close. The typed listen + error lives in its own module so the stdio entry never loads the listener. +3. Parse `--http`, `--host` and `--port` in `src/main/mcpHttpArguments.ts` from + one schema that also renders help; resolve `MCP_HOST`/`MCP_PORT` in + `src/config/mcpHttpEndpoint.ts` as a `Result` that names rejected settings + without their values. +4. Route the dispatcher to `runMcpHttpEntrypoint` when the `mcp` entry carries + `--http`; compose HTTP dependencies in `src/main.ts` with the shared + `run()` path so configuration, session, providers and shutdown stay common. +5. Register signals only for HTTP mode through `registerSignalShutdown`, a + projection of `registerProcessShutdown` without stdin that sets the signal + exit status before the idempotent shutdown runs; stdio keeps its stdin + end/close triggers. +6. Document the transport in docs/mcp-contracts.md and docs/cli.md, relabel the + architecture diagram, and cover each boundary with adapters, dispatcher, + mcp-boundary and process-boundary tests plus a Python SDK client exchange. + +No new dependency is introduced: the transport, Host validation and body bound +come from the already pinned `@modelcontextprotocol/server` root export. Tool +contracts, catalog digests, response budgets and stdio behaviour are unchanged. diff --git a/docs/specs/agent-environments/http-transport/spec.md b/docs/specs/agent-environments/http-transport/spec.md new file mode 100644 index 0000000000..e4e9135833 --- /dev/null +++ b/docs/specs/agent-environments/http-transport/spec.md @@ -0,0 +1,79 @@ +# Streamable HTTP transport for the MCP server + +Issue: https://github.com/morluto/rea/issues/4933 + +## Outcome + +`rea mcp --http [--host H] [--port N]` serves the existing MCP tool catalog over +the pinned SDK's streamable HTTP transport instead of stdio, so an agent +environment runtime or any HTTP MCP client can reach REA on a local URL. The +listener is opt-in: bare `rea mcp`, `rea mcp doctor`, `rea mcp add` and every +client registration keep stdio. + +## Requirements + +- Route `mcp --http ...` and `--mcp --http ...` to the MCP runtime in the + dispatcher without changing bare `mcp`, `mcp doctor` or `mcp add`. +- Resolve host and port from the flags, then `MCP_HOST`/`MCP_PORT` from the + environment snapshot, then `127.0.0.1` and port `0`; announce the bound URL as + one pino JSON line on stderr at every log level and leave stdout unused. +- Use `WebStandardStreamableHTTPServerTransport` in stateful mode around one + server instance built once, after the listener has bound. `DELETE` ends the session on its transport without + running the instance's shutdown hooks, so the same instance keeps its + providers and serves the next client on a fresh transport; the hooks run when + the listener closes. +- Let a new client's `initialize` replace a session that is still open, closing + the earlier transport the same way, so a client that left without `DELETE` + cannot block the listener; the replaced client's later requests are refused + (`404` while the replacing session is open). +- Bridge node:http to web Request/Response in `src/server/http/`, streaming + response chunks as they are produced and turning a client disconnect into a + request abort and body cancellation. +- Classify a loopback bind by the actual bound IP (`127.0.0.0/8`, `::1`, or + IPv4-mapped loopback), without using the configured DNS name as evidence; validate + the `Host` and `Origin` headers with the SDK loopback allowlist plus the + configured name and the bound address on such binds, and relax both for + other binds and log that relaxation. Refuse scoped IPv6 literals before + binding, close the listener if its URL cannot be formed, and forward + listener errors to `onerror` only after a successful bind. +- Let the caller own instance cleanup (`closeInstance: false`) when its factory + owner already closes every instance, as the MCP runtime does. +- Let only an `initialize` that carries the streamable headers and passes those + checks replace an open session; refuse requests once the slot is closed, join + an arming session during shutdown, and keep close stages retryable. +- End the HTTP listener on SIGINT/SIGTERM only, never on stdin EOF, and exit with + the signal status (`130`/`143`) after cleanup settles; latch a signal that + arrives during startup so it is acted on as soon as the lifecycle owner exists, + and exit at once on a second one. +- Export `serveStreamableHttp({ host, port, createMcpServer, routes, logger })` + returning `{ url, close }` so an agent environment server can add plain routes + beside `/mcp` without a second bridge. +- Keep listener failure causes out of stderr behind a fixed message while the + typed listen error carries the system cause (such as `EADDRINUSE`) into the + error-level log line. + +## Acceptance and limits + +Adapters tests cover the bridge (method, URL, headers, body, GET without body, +incremental streaming, abort and cancel) and the listener (listening line, one +factory call across three sessions, replacement of an open session by the next +`initialize`, two `initialize` requests racing to replace one session while the +instance's shutdown hook stays installed, a working tool call after `DELETE`, +the instance's shutdown hook firing only when the listener closes, plain +routes, 404, Host refusal, port release, typed bind failure naming +`EADDRINUSE`). The dispatcher test covers the new routes and keeps `mcp add +--http` with the CLI. An mcp-boundary test spawns the built CLI and drives +initialize, tools/list, `binary_session` and `DELETE` with the SDK streamable +HTTP client, then proves `binary_session` still answers in the next session and +in a session that replaced one left open; a process-boundary test asserts +`143`/`130` exits and a released port. Concurrent clients are not supported: +the newest `initialize` owns the server. The non-loopback relaxation is covered +only through its pure allowlist helper; no wide bind runs in tests. Response +budgets keep their stdio-derived floor; registrations, `rea mcp doctor` and +`server.json` stay stdio. + +## Deferred issue criteria + +Serving an environment manifest with replayed records, its environment card and +the exported bundle are later increments of #4933. This PR must reference rather +than close the issue. diff --git a/docs/specs/agent-environments/http-transport/tasks.md b/docs/specs/agent-environments/http-transport/tasks.md new file mode 100644 index 0000000000..07981bff35 --- /dev/null +++ b/docs/specs/agent-environments/http-transport/tasks.md @@ -0,0 +1,11 @@ +# Tasks + +- [x] Define single-feature requirements and implementation plan before code. +- [x] Widen the runtime serve seam and shutdown handle to transport-neutral types. +- [x] Implement the node:http bridge and the streamable HTTP listener module. +- [x] Parse flags and `MCP_HOST`/`MCP_PORT`, route the dispatcher, add signals-only shutdown. +- [x] Document the transport, CLI entry, exit status and architecture label. +- [x] Verify bridge, listener, dispatcher, SDK client, Python client and shutdown behavior. +- [x] Keep providers alive across sessions, replace a session left open, name the bind cause in the log. +- [x] Resolve the re-review: capture the shutdown hook once, one shutdown registration primitive, flags-only HTTP routing. +- [x] Run relevant repository checks and prepare a separately scoped PR. diff --git a/docs/specs/agent-environments/http-transport/verification.md b/docs/specs/agent-environments/http-transport/verification.md new file mode 100644 index 0000000000..73dd3ff34a --- /dev/null +++ b/docs/specs/agent-environments/http-transport/verification.md @@ -0,0 +1,297 @@ +# Verification + +Verified base: 18aa9af2b2c26fd114113dd39528a91224801840 (fetched main). + +The worktree was moved onto that base with the slice's changes kept +uncommitted, then reinstalled with `npm ci` (exit 0; 540 packages added, 541 +audited; 6.56 s elapsed, 3.85 s user, 8.07 s sys, 881459200 bytes maximum +resident set size). After the edits of this pass the tree was rebuilt with +`npm run build` (exit 0, three turbo tasks; 5.23 s elapsed, 13.94 s user, +2.93 s sys, 2964652032 bytes) and `npm run test:prepare` (exit 0, six tasks +with three cache hits; 1.37 s elapsed, 3.07 s user, 0.76 s sys, 422625280 +bytes); `dist/mcp/` carries the rebuilt `serveStreamableHttp-*.js` chunk beside +`main.js`. + +Static gates on the final tree: `npm run check:fast` passed (typecheck and +lint, two tasks; 4.69 s elapsed, 24.25 s user, 3.00 s sys, 4480139264 bytes). +`npx oxlint` over the files touched in this pass reported no findings. After +the last edit, including this record, `npm run format` ran over 4032 files +(0.68 s elapsed, 4.11 s user, 0.74 s sys, 227393536 bytes) and `npm run +format:check` passed (0.69 s elapsed, 4.09 s user, 0.62 s sys, 216989696 +bytes). `npx knip` exited 0 (3.80 s elapsed, 5.10 s user, 0.72 s sys, +868679680 bytes). `npm run check:ci:tasks` passed all eight tasks, seven of +them turbo cache hits from the run on the same sources (0.88 s elapsed, 4.52 s +user, 1.04 s sys, 215171072 bytes): jscpd reported 2576 clones and 3.35% +duplicated lines across 3143 files under the 3.5% threshold, lint:built 0 +errors over 3734 files, and format:check ran fresh. `npm run docs:check` +passed, including the README consistency test (5 cases; 3.27 s elapsed, +3.43 s user, 0.80 s sys, 288866304 bytes). `git diff --stat` lists 13 tracked +files, none under `src/domain/`. + +Source-group tests: `/usr/bin/time -l npx vitest run --project adapters +src/server/http src/server/shutdownRegistration.test.ts` passed 3 files, 17 +tests in 2.74 s elapsed, 3.12 s user, 0.93 s sys, 289767424 bytes. +`serveStreamableHttp.test.ts` gained the concurrent case: a slot with one open +session receives two `initialize` requests through `Promise.all`, exactly one +answers `200` with a session id and the other `400`, the instance's `onclose` +hook has fired zero times, and `slot.close()` fires it once. Run before the fix +against the same file, that case failed with the hook count `0` instead of `1` +(1 failed, 5 passed; 1.07 s elapsed, 0.84 s user, 0.21 s sys, 158187520 +bytes), which is the pinned no-op hook the review described. +`shutdownRegistration.test.ts` asserts that `registerProcessShutdown` hands +the handler `SIGINT`/`SIGTERM` by name and `undefined` for stdin end, that +omitting stdin registers no input listener, and that `registerSignalShutdown` +reports `143`/`130`. + +Built-group tests ran after `npm run test:prepare`. The targeted dispatcher +run, `/usr/bin/time -l npx vitest run --project cli-boundary +tests/boundary/cli/dispatcher.test.ts`, passed 3 tests in 1.08 s elapsed, +0.89 s user, 0.26 s sys, 153026560 bytes, now including `mcp add --http` +routed to the CLI beside `mcp --http --port 0`, `--mcp --http` and `mcp --host +127.0.0.1 --http` routed to the HTTP entry and `mcp doctor --http`, `mcp add` +and `--mcp extra` keeping their routes. + +The four full projects ran in sequence while a sibling worktree ran its own +`cli-boundary`, `mcp-boundary` and `acceptance` chain on the same host. +`/usr/bin/time -l npx vitest run --project adapters` passed 247 files, 3945 +tests with 8 skipped, in 93.77 s elapsed, 80.21 s user, 30.69 s sys, 344260608 +bytes. `/usr/bin/time -l npx vitest run --project mcp-boundary` passed 155 +files with 2 skipped, 910 tests with 7 skipped, in 167.51 s elapsed, 321.24 s +user, 51.30 s sys, 4923490304 bytes (the Vitest parent, its workers and the +spawned CLI children). `/usr/bin/time -l npx vitest run --project +process-boundary` exited 1: 56 files passed, 2 skipped and 2 failed, 460 tests +passed, 19 skipped and 7 failed, in 544.81 s elapsed, 160.89 s user, 133.49 s +sys, 1028931584 bytes. `/usr/bin/time -l npx vitest run --project +cli-boundary` exited 1: 108 files passed and 3 failed, 658 tests passed, 2 +skipped and 7 failed, in 452.34 s elapsed, 734.29 s user, 234.02 s sys, +648495104 bytes. No failing file belongs to this slice. + +The failures split into two kinds. Load: six `jadxIntegration.test.ts` cases +hit the 30 s test or 10 s hook timeout, and six `javascriptApplication*.test.ts` +cases failed with the worker's result-capacity constraint; re-run alone in this +worktree after the other chains ended, `jadxIntegration.test.ts` passed all 48 +tests and both JavaScript files passed all 20 tests (`cli-boundary` three files: +20 passed, 1 failed, 23.38 s elapsed, 32.99 s user, 10.97 s sys, 343244800 +bytes; `process-boundary` two files: 48 passed, 1 failed, 62.48 s elapsed, +21.89 s user, 17.55 s sys, 686620672 bytes). Pre-existing on main in this +environment: `closedStdout.test.ts` "stops quietly when the reader closes +during streamed JSON" exits `1` instead of `141` with 1009 bytes received and +empty stderr, and `releaseArtifactCanary.test.ts` fails parsing `npm pack +--json`, which npm 12.0.2 emits as an object keyed by package name rather than +the tuple the test expects. Both reproduce identically on the clean sibling +clone at `52b73ee857029a3ab8f5c6eeef233b295951a46c` (no tracked or untracked +changes; `closedStdout` 1 failed, 3 passed, 8.86 s elapsed, 6.98 s user, 2.29 s +sys, 348012544 bytes; the canary 1 failed, 7.91 s elapsed, 7.76 s user, 1.84 s +sys, 668729344 bytes). That clone predates the verified base by 15 commits, +none touching either test or `scripts/rea.mjs`. Because the dispatcher is on +the `closedStdout` path, its scenario was also replayed against this +worktree's built `dist/` with the `18aa9af2` `scripts/rea.mjs` and with the +edited one, three runs each: all six exited `1` with 1009 bytes received and +empty stderr. Nothing in this pass was changed for these failures. + +Cross-implementation check with the Python `mcp` 1.30.0 client (httpx 0.28.1, +Python 3.12.12) against the built `node scripts/rea.mjs mcp --http --port 0 < +/dev/null`: stderr announced +`{"transport":"streamable-http","url":"http://127.0.0.1:57093/mcp","msg":"MCP transport listening"}`. +Session A: `initialize` POST returned `200 text/event-stream` with +`mcp-session-id`; `notifications/initialized` returned `202` with no body; the +standalone GET and the `tools/list` and `tools/call` POSTs returned `200 +text/event-stream`; `tools/list` reported 207 tools including +`binary_session`; `binary_session` with `{}` returned `isError=False`; `DELETE +/mcp` returned `200`. Session B, opened after that `DELETE`, initialized with a +new session id, listed the same 207 tools, and `binary_session` again returned +`isError=False` before its own `DELETE` returned `200`. The server logged two +`MCP HTTP session opened` and two `MCP HTTP session closed by the client` +lines, SIGTERM then exited it with status `143`, and stdout stayed empty. The +methods, statuses and session-id handling of session A match the recorded +reference exchange step for step; the reference's echo server labelled its +`202` and `DELETE` replies `application/json`, where this listener sends no +content type for those bodiless replies. + +## Independent review + +An adversarial review of the first implementation reproduced that, after a +`DELETE`, the next session could initialize and list tools but every tool call +failed with a `cancelled` error because the SDK's `DELETE` handling closed the +transport and the server's `onclose` hooks ran provider teardown. It also found +that a session ended without `DELETE` blocked the listener until restart, that +the bind-failure log line omitted the system cause, a duplicated foreign-`Host` +probe, a missing trailing newline on `rea mcp --http --help`, and a static +import of the HTTP listener on the stdio path. Each finding is addressed and +covered by the adapters, mcp-boundary and Python checks in this document. + +The re-review of the rebased tree raised four points, all resolved here. Two +concurrent `initialize` POSTs could leave the no-op `onclose` hook installed, +because each session end captured the hook current at that moment and the +later `finally` restored the other call's no-op; `McpSessionSlot` now captures +the instance's hook once, after the factory returns the fully wrapped server, +and every session end restores exactly that, with the adapters case above +failing before and passing after the change. The optional stdin parameter of +`registerProcessShutdown` was dead in production; `registerSignalShutdown` is +now a projection of `registerProcessShutdown(handler, { signals })`, so one +primitive owns registration while the handler learns the signal and keeps +`130`/`143`. The dispatcher routed any `mcp` invocation containing `--http` to +the HTTP entry; it now does so only when the first token after `mcp` or +`--mcp` is a flag, so `rea mcp add --http` keeps the CLI's own diagnostic, and +the dispatcher test covers that case. The formatter hunk in +`src/domain/propertyListXml.test.ts` noted in the earlier record is gone after +the rebase, as `git diff --stat` confirms. The replacement rule stays as +documented, and `docs/mcp-contracts.md` now states it in one sentence with its +rationale so maintainers can decide on it. + +## Scope and measurement limits + +The concurrent-`initialize` case is exercised at the slot with in-memory +`Request` objects, where both calls advance in lockstep; over TCP two +connections are served in separate event-loop turns, so the race is reachable +only in-process and the listener-level behaviour remains "the newest client +owns the server, the other sees `400` or `404`". The non-loopback relaxation +path is covered only through the pure allowlist helper and the loopback smoke +runs; no wide bind was exercised. Sessions are served one at a time; +concurrent clients are not supported. The two failures that reproduce on the +clean clone are environmental (macOS with Node 22.23.1 and npm 12.0.2) and are +left for their owners; the load-induced timeouts were classified by isolated +re-runs, not by repeating the full projects on an idle host. The error-level +log line that names the bind cause was observed in earlier runs, not asserted +by `mainErrors.test.ts`. Response budgets keep the stdio-derived floor and were +not re-measured over HTTP. Windows was not run. + +## Automated review findings on the PR (2026-10-18) + +Codex and Greptile left eight inline findings on morluto/rea#4955 at +46dd8f0e; each was checked against the code and addressed in one follow-up +commit, with a regression test where the behaviour is observable: + +- A late `initialize` could arm a fresh transport after shutdown (Codex, P1): + the slot now records that it is closed, `close()` joins an arming session + before ending it, and every request after that is refused with `503`. Test: + an initialize racing `close()` gets `503`, the shutdown hook fires once, and + the next request gets `503`. +- A failed `close()` was memoized forever (both reviewers, P1): the close runs + three stages once each and a failed stage is retried by the next call. Test: + the instance's first `close()` rejects, the second call resolves and the port + is released. +- Loopback aliases skipped Host validation (both reviewers, P2 and P1): a bind + is now loopback by address class (`127.0.0.0/8`, `::1`, IPv4-mapped loopback, + `localhost` names) after the same normalization the SDK applies to the Host + header, and the allowlist adds the bind's own hostname so requests to + `127.0.0.2` or `localhost.` are still served. Test: twelve spellings. +- A sessionless POST could close the live session before any header check + (Greptile, P1): on loopback binds the route validates `Origin` with the same + allowlist as `Host`, and only a POST carrying `Content-Type: + application/json` and an `Accept` naming both `application/json` and + `text/event-stream` may replace a session; others reach the live transport, + which refuses them. Test: a `text/plain` initialize and one with a foreign + `Origin` are refused (4xx and 403) while the connected client keeps echoing. +- The bound URL disappeared at quiet log levels (Greptile, P2): the listening + line is written through a child logger pinned to `info`. Test: a `warn` + logger still receives the line with the URL. +- `rea mcp -h --http` reached the ordinary CLI (Greptile, P2): the dispatcher + predicate accepts any flag after the entry. Test: the dispatcher case. + +Checks after the change: `oxfmt --check .` clean; `tsc --noEmit` clean; +`oxlint` on the touched sources clean after extracting the staged close into +its own function; `npx knip` no findings; `npm run jscpd` 3.35 % duplicated +lines; `scan:todos` none; `verify:test-discovery` 1,583 files once; `vitest +run` over the listener and bridge adapters tests 14 passed in 2 files; after +`npm run build:cached`, `vitest run` over the dispatcher, option, streamable +HTTP, shutdown and main-error boundary tests 22 passed in 5 files. + +A second automated round on the fix (Codex, two P2 findings) was addressed in +the next commit: the replacement guard compares the media type proper, so +`application/json-patch+json` no longer counts as JSON (test: such an +initialize is refused while the connected client keeps echoing), and the +server instance is created only after the listener has bound, so a bind +failure has nothing to clean up and a factory failure releases the listener +and rethrows (test: a throwing factory rejects without announcing a URL). + +A third round (Greptile and Codex, five findings) was addressed next: the +listener's `error` forwarding to `onerror` is registered after the bind, so +`EADDRINUSE` prints only the listen-failure message (test: an occupied port +rejects without calling `onerror`); scoped IPv6 literals are refused before +binding and a URL that cannot be formed from the bound address closes the +listener (test: `fe80::1%lo0` rejects before the factory runs); loopback is +classified from the bound address with the configured name added to the +allowlist (test: `ip6-localhost` bound to `::1`, `dev.local` bound to +`127.0.0.1`); and the MCP runtime passes `closeInstance: false` so the factory +owner in `startMcpTransport` remains the single owner of instance cleanup +(test: `close` leaves the instance open until its owner closes it). + +A fourth round (Codex, two findings) was addressed next: the replacement +guard parses `Accept` into media ranges and requires exact `application/json` +and `text/event-stream` entries (test: `application/json-patch+json, +text/event-streaming` is refused while the connected client keeps echoing), +and a session whose transport shutdown rejected stays owned so the next close +retries it (test: a failing transport close rejects `close()` once, the second +call completes, the shutdown hook fires once and later requests get `503`). + +A fifth round (Codex, three findings) was addressed next: the instance's +shutdown hook now runs exactly once even when the listener closes without a +live session (test: two closes of an idle slot fire it once; an idle listener +close fires it once); the initialize probe no longer awaits cancelling its +cloned body, which is one branch of a tee and would wait for the original +request to drain (test: a 4.5 MB sessionless initialize is refused promptly +while the connected client keeps echoing); and the node:http bridge passes +repeated `Set-Cookie` values as an array instead of collapsing them (test: two +cookies from a composed route both arrive). + +A sixth round (Codex, one P1) was addressed next: signal handlers were only +installed after the asynchronous startup returned, so a SIGINT or SIGTERM +during target opening, provider loading or the HTTP bind ended the process +without cleanup. The HTTP entry now latches signals before startup, the +lifecycle owner picks the recorded signal up as soon as it registers, and a +second signal during startup exits at once (test: the latch records `143`, +exits on the second signal, hands the status over on release and leaves no +listeners behind). + +Both reviewers then noted that the latched signal's status was overwritten by +the entry's normal `0` return: the HTTP entry now owns its exit status through +`createExitStatus`, where a `0` after a recorded signal keeps the signal's +status and a real failure still wins (unit test), and a process test sends the +signal the moment the listening line appears and asserts the signal status. + +An eighth finding (Codex): a reply sent before the MCP handler read the body +left that body unread on a paused bridged stream, so a slow uploader could +hold the socket. The bridge now closes the connection after any reply whose +request body was never read (test: an unknown-route POST and a foreign-Host +POST are answered with `Connection: close`, while a keep-alive GET without a +body keeps its connection). + +A ninth round (Greptile and Codex): request conversion ran outside the guard, +so a method Fetch refuses (`TRACE`) could crash the process; the conversion is +now guarded and answered with `405` and a closed connection (test: `TRACE` +gets `405` and the next client still echoes). The unread-body decision now +comes from the incoming message's own body declaration, so a `GET` or `HEAD` +that carries a body also closes, and the regression uses a keep-alive agent +and checks that the socket actually closes after the early reply while a +bodiless `GET` keeps its socket. + +A tenth round (Codex, three findings): the takeover guard now requires a lone +`initialize`, mirroring the transport (test: a batch of `initialize` plus +`notifications/initialized` is refused while the connected client keeps +echoing); the shutdown hook records completion only after it returns, so a +hook that threw stays eligible for the next close (test: a hook that throws +once runs twice across two closes and then no more); and the bridge joins the +request target onto the listener's origin verbatim, refusing absolute-form +targets (test: `//attacker.example/card` is a 404 for that path, a plain route +sees the bound host only, and `http://attacker.example/card` gets `400`). + +An eleventh finding (Codex): URL construction canonicalized dot segments, so +`/public/../mcp` reached `/mcp`. The bridge now refuses any target whose path +differs after URL parsing, so routing always sees the path the client sent +(test: `/public/../mcp`, `/public/%2e%2e/mcp` and `/./mcp` get `400`; `/card?x=1` +still routes). + +A twelfth finding (Codex): `bodyUsed` means disturbed, not consumed, so a +composed route that read one chunk of an upload left the rest paused on a +keep-alive socket. The close decision now uses the incoming message's end of +stream (test: a route that reads one chunk of a 256 KiB upload is answered +with `Connection: close` and a closed socket; a route that reads it all keeps +the connection). + +A thirteenth round (Codex): an `Accept` range with `q=0` no longer counts toward +the takeover guard (test: `application/json;q=0, text/event-stream` is refused +while the connected client keeps echoing). The same round noted that a failed +startup leaves cleanup of the already opened session and providers to a single +attempt; that path is the pre-existing stdio startup-failure cleanup, which the +HTTP mode now reaches on `EADDRINUSE`, and is left for the maintainers to decide. diff --git a/scripts/rea.mjs b/scripts/rea.mjs index aa0659465a..326676a976 100755 --- a/scripts/rea.mjs +++ b/scripts/rea.mjs @@ -5,15 +5,22 @@ import { dirname, join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; // Route production MCP before importing Incur. Incur owns registration helpers -// such as `mcp add`, while the MCP runtime serves the stdio tool catalog. +// such as `mcp add`, while the MCP runtime serves the tool catalog over stdio +// or, with `--http`, over streamable HTTP. const args = process.argv.slice(2); const { default: packageJson } = await import("../package.json", { with: { type: "json" }, }); process.env.REA_PACKAGE_VERSION = packageJson.version; -const isMcpMode = - args.length === 1 && (args[0] === "--mcp" || args[0] === "mcp"); +const isMcpEntry = args[0] === "--mcp" || args[0] === "mcp"; +const isMcpMode = isMcpEntry && args.length === 1; const isMcpDoctorMode = args[0] === "mcp" && args[1] === "doctor"; +// Only flags may follow the entry: `mcp add --http` stays Incur's `mcp add`, +// while `mcp -h --http` reaches the HTTP entry's own help. +const isMcpHttpMode = + isMcpEntry && + (args[1]?.startsWith("-") ?? false) && + args.slice(1).includes("--http"); /** Exit status of a writer terminated by SIGPIPE (128 + 13). */ const SIGPIPE_EXIT_STATUS = 141; // Set once the reader of CLI output has closed the pipe. @@ -40,15 +47,16 @@ if ( process.env.PLAYWRIGHT_BROWSERS_PATH = join(cacheRoot, "ms-playwright"); } -const runtimeFiles = isMcpMode - ? ["dist/mcp/main.js"] - : isMcpDoctorMode - ? ["dist/mcp/main.js", "dist/cli-runtime/mcpDoctor.js"] - : [ - "dist/cli-runtime/cli.js", - "dist/cli-runtime/cliOutput.js", - "dist/cli-runtime/cliResultOutput.js", - ]; +const runtimeFiles = + isMcpMode || isMcpHttpMode + ? ["dist/mcp/main.js"] + : isMcpDoctorMode + ? ["dist/mcp/main.js", "dist/cli-runtime/mcpDoctor.js"] + : [ + "dist/cli-runtime/cli.js", + "dist/cli-runtime/cliOutput.js", + "dist/cli-runtime/cliResultOutput.js", + ]; if (!(await compiledRuntimeExists(runtimeFiles))) { process.stderr.write( @@ -58,6 +66,9 @@ if (!(await compiledRuntimeExists(runtimeFiles))) { } else if (isMcpMode) { const { runEntrypoint } = await import("../dist/mcp/main.js"); await runEntrypoint(); +} else if (isMcpHttpMode) { + const { runMcpHttpEntrypoint } = await import("../dist/mcp/main.js"); + await runMcpHttpEntrypoint(args.slice(1)); } else if (isMcpDoctorMode) { guardClosedStdout(); const { runProductionMcpDoctorCli } = diff --git a/src/config/mcpHttpEndpoint.ts b/src/config/mcpHttpEndpoint.ts new file mode 100644 index 0000000000..4604497ffb --- /dev/null +++ b/src/config/mcpHttpEndpoint.ts @@ -0,0 +1,68 @@ +import { z } from "zod"; + +import { + ConfigurationError, + type ConfigurationSettingIssue, +} from "../domain/configurationErrors.js"; +import { err, ok, type Result } from "../domain/result.js"; + +/** The loopback default keeps the listener local-only until an operator opts out. */ +export const DEFAULT_MCP_HTTP_HOST = "127.0.0.1"; +/** Port 0 asks the host for an ephemeral port, reported by the listening log line. */ +export const DEFAULT_MCP_HTTP_PORT = 0; + +/** Decimal TCP port text, shared by the `--port` flag and `MCP_PORT`. */ +export const mcpHttpPortSchema = z + .string() + .regex(/^(?:0|[1-9]\d*)$/u) + .transform(Number) + .pipe(z.number().int().min(0).max(65535)); + +const mcpHttpHostSchema = z.string().trim().min(1); + +export const MCP_HTTP_PORT_CONSTRAINT = + "MCP_PORT must be a decimal integer between 0 and 65535."; +export const MCP_HTTP_HOST_CONSTRAINT = + "MCP_HOST must be a non-empty host name or IP address."; + +/** Where the streamable HTTP listener binds. */ +export interface McpHttpEndpoint { + readonly host: string; + readonly port: number; +} + +/** Resolve the endpoint: explicit values, then MCP_HOST/MCP_PORT, then loopback defaults. */ +export const parseMcpHttpEndpoint = ( + environment: Readonly, + overrides: Partial = {}, +): Result => { + const issues: ConfigurationSettingIssue[] = []; + let host = overrides.host ?? DEFAULT_MCP_HTTP_HOST; + let port = overrides.port ?? DEFAULT_MCP_HTTP_PORT; + if (overrides.host === undefined && environment.MCP_HOST !== undefined) { + const parsed = mcpHttpHostSchema.safeParse(environment.MCP_HOST); + if (parsed.success) host = parsed.data; + else + issues.push({ + setting: "MCP_HOST", + constraint: MCP_HTTP_HOST_CONSTRAINT, + }); + } + if (overrides.port === undefined && environment.MCP_PORT !== undefined) { + const parsed = mcpHttpPortSchema.safeParse(environment.MCP_PORT); + if (parsed.success) port = parsed.data; + else + issues.push({ + setting: "MCP_PORT", + constraint: MCP_HTTP_PORT_CONSTRAINT, + }); + } + if (issues.length > 0) + return err( + new ConfigurationError( + issues.map(({ constraint }) => constraint).join(" "), + { settings: issues }, + ), + ); + return ok({ host, port }); +}; diff --git a/src/main.ts b/src/main.ts index a01bd2398e..4a10d8a1dd 100644 --- a/src/main.ts +++ b/src/main.ts @@ -21,7 +21,18 @@ import { } from "./composition/optionalObservationProviders.js"; import { createToolResultDelivery } from "./server/toolResult.js"; import { createShutdown } from "./main/shutdown.js"; -import { registerProcessShutdown } from "./main/processShutdown.js"; +import { + createExitStatus, + latchSignalShutdown, + registerProcessShutdown, + registerSignalShutdown, + type ExitStatus, + type SignalLatch, +} from "./main/processShutdown.js"; +import { + parseMcpHttpEndpoint, + type McpHttpEndpoint, +} from "./config/mcpHttpEndpoint.js"; const runtimeDependencies = (): RuntimeDependencies => ({ env: process.env, @@ -33,10 +44,57 @@ const runtimeDependencies = (): RuntimeDependencies => ({ registerShutdown: registerProcessShutdown, }); +/** Replace stdio with the streamable HTTP listener; signals alone end it, never stdin. */ +const httpRuntimeDependencies = ( + endpoint: McpHttpEndpoint, + latch: SignalLatch, + exit: ExitStatus, +): RuntimeDependencies => { + const base = runtimeDependencies(); + return { + ...base, + setExitCode: exit.set, + serve: async (factory, options) => { + // Loaded here so the default stdio entry never loads the HTTP listener. + const { serveStreamableHttp } = + await import("./server/http/serveStreamableHttp.js"); + const { RequestTrackingMcpServer } = + await import("./server/RequestTrackingMcpServer.js"); + // The transport's factory owner closes every instance it created; the listener must not race it. + return serveStreamableHttp({ + ...endpoint, + createMcpServer: async (context) => { + const server = await factory(context); + if (!(server instanceof RequestTrackingMcpServer)) + throw new TypeError("HTTP MCP servers must track tool completion"); + return server; + }, + closeInstance: false, + ...options, + }); + }, + registerShutdown: (request) => { + const unregister = registerSignalShutdown((exitStatus) => { + exit.signal(exitStatus); + request(); + }); + // A signal that arrived during startup is honoured now that the lifecycle owner exists; + // its status outlives the entry's own `0`, which startup returns right after this. + const pending = latch.release(); + if (pending !== undefined) { + exit.signal(pending); + request(); + } + return unregister; + }, + }; +}; + /** * Start the long-lived MCP adapter and install idempotent shutdown handlers. - * The adapter owns its BinarySession for the process lifetime; EOF and process - * signals close bridge resources without assuming ownership of the Hopper app. + * The adapter owns its BinarySession for the process lifetime; stdin EOF on + * the stdio transport and process signals on either transport close bridge + * resources without assuming ownership of the Hopper app. */ export const run = async ( dependencies: RuntimeDependencies = runtimeDependencies(), @@ -100,6 +158,62 @@ export const runEntrypoint = async ( } }; +/** Process streams the dispatcher entries write to. */ +export interface EntrypointIo { + readonly writeStdout: (text: string) => void; + readonly writeStderr: (text: string) => void; + readonly setExitCode: (code: number) => void; +} + +const processIo: EntrypointIo = { + writeStdout: (text) => void process.stdout.write(text), + writeStderr: (text) => void process.stderr.write(text), + setExitCode: (code) => { + process.exitCode = code; + }, +}; + +/** Run `rea mcp --http`: parse its flags, resolve MCP_HOST/MCP_PORT, then serve over HTTP. */ +export const runMcpHttpEntrypoint = async ( + arguments_: readonly string[], + io: EntrypointIo = processIo, +): Promise => { + const { parseMcpHttpArguments, renderMcpHttpHelp } = + await import("./main/mcpHttpArguments.js"); + const parsed = parseMcpHttpArguments(arguments_); + if (!parsed.ok) { + if (parsed.help) { + io.writeStdout(`${renderMcpHttpHelp()}\n`); + io.setExitCode(0); + return; + } + io.writeStderr(`${parsed.message}\n`); + io.setExitCode(1); + return; + } + const endpoint = parseMcpHttpEndpoint(snapshotEnvironment(process.env), { + ...(parsed.host === undefined ? {} : { host: parsed.host }), + ...(parsed.port === undefined ? {} : { port: parsed.port }), + }); + if (!endpoint.ok) { + io.writeStderr(`${projectAnalysisError(endpoint.error).message}\n`); + io.setExitCode(1); + return; + } + // Latched before any asynchronous startup so a signal cannot end the process without cleanup. + const latch = latchSignalShutdown(); + const exit = createExitStatus(io.setExitCode); + try { + await runEntrypoint( + () => run(httpRuntimeDependencies(endpoint.value, latch, exit)), + io.writeStderr, + exit.set, + ); + } finally { + latch.release(); + } +}; + /** Return false when the runtime's argv path is synthetic or no longer exists. */ const isDirectEntrypoint = ( entryPath: string | undefined, diff --git a/src/main/mcpHttpArguments.ts b/src/main/mcpHttpArguments.ts new file mode 100644 index 0000000000..634c58ff32 --- /dev/null +++ b/src/main/mcpHttpArguments.ts @@ -0,0 +1,104 @@ +import { Help, z } from "incur"; + +import { + MCP_HTTP_PORT_CONSTRAINT, + mcpHttpPortSchema, +} from "../config/mcpHttpEndpoint.js"; + +/** + * Canonical `rea mcp --http` options. The help renderer lists exactly this + * schema and the parser accepts exactly these flags plus the `-h` alias. + */ +const MCP_HTTP_OPTIONS = z.object({ + http: z + .boolean() + .describe("Serve the MCP catalog over streamable HTTP at /mcp, not stdio"), + host: z + .string() + .min(1) + .optional() + .describe("Listener address; default MCP_HOST, else 127.0.0.1"), + port: z + .number() + .int() + .min(0) + .max(65535) + .optional() + .describe("Listener port; default MCP_PORT, else 0 for an ephemeral port"), + help: z.boolean().optional().describe("Show help"), +}); + +const MCP_HTTP_ALIAS = { help: "h" } as const; +const MCP_HTTP_COMMAND = "rea mcp --http"; +const MCP_HTTP_DESCRIPTION = + "Serve the MCP server over streamable HTTP for one local client"; +const HELP_FLAGS: ReadonlySet = new Set(["--help", "-h"]); +const VALUED_FLAGS: ReadonlySet = new Set(["--host", "--port"]); + +/** Render help for the dispatcher entry, which accepts no Incur global options. */ +export const renderMcpHttpHelp = (): string => + Help.formatCommand(MCP_HTTP_COMMAND, { + description: MCP_HTTP_DESCRIPTION, + hideGlobalOptions: true, + options: MCP_HTTP_OPTIONS, + alias: MCP_HTTP_ALIAS, + hint: "This entry accepts only the options above. The bound URL is logged as one JSON line on stderr.", + }); + +/** Parsed `rea mcp --http` arguments; omitted values defer to the environment. */ +export type McpHttpArguments = + | { readonly ok: true; readonly host?: string; readonly port?: number } + | { readonly ok: false; readonly help: true } + | { readonly ok: false; readonly help: false; readonly message: string }; + +const invalid = (message: string): McpHttpArguments => ({ + ok: false, + help: false, + message, +}); + +/** Split `--flag=value` into its flag and inline value. */ +const splitFlag = (argument: string): readonly [string, string | undefined] => { + const separator = argument.indexOf("="); + return separator === -1 + ? [argument, undefined] + : [argument.slice(0, separator), argument.slice(separator + 1)]; +}; + +/** Parse the dispatcher arguments after `mcp`; help wins only in flag position. */ +export const parseMcpHttpArguments = ( + arguments_: readonly string[], +): McpHttpArguments => { + const values = new Map(); + let http = false; + for (let index = 0; index < arguments_.length; index += 1) { + const argument = arguments_[index] ?? ""; + if (HELP_FLAGS.has(argument)) return { ok: false, help: true }; + if (argument === "--http") { + http = true; + continue; + } + const [flag, inline] = splitFlag(argument); + if (!VALUED_FLAGS.has(flag)) + return invalid(`Unknown mcp --http argument: ${argument}`); + const value = inline ?? arguments_[index + 1]; + if (inline === undefined) index += 1; + if (value === undefined || (inline === undefined && value.startsWith("-"))) + return invalid(`Missing value for ${flag}`); + values.set(flag, value); + } + if (!http) return invalid("--host and --port require --http"); + const host = values.get("--host")?.trim(); + if (host !== undefined && host.length === 0) + return invalid("--host must be a non-empty host name or IP address"); + const portText = values.get("--port"); + const port = + portText === undefined ? undefined : mcpHttpPortSchema.safeParse(portText); + if (port !== undefined && !port.success) + return invalid(`Invalid --port: ${MCP_HTTP_PORT_CONSTRAINT}`); + return { + ok: true, + ...(host === undefined ? {} : { host }), + ...(port === undefined ? {} : { port: port.data }), + }; +}; diff --git a/src/main/messages.ts b/src/main/messages.ts index 552be71413..965ce9f28a 100644 --- a/src/main/messages.ts +++ b/src/main/messages.ts @@ -6,3 +6,5 @@ export const MCP_SHUTDOWN_FAILED = "REA could not close cleanly. End the REA process before starting it again."; export const SERVER_START_FAILED = "REA could not start. Run `rea doctor`, then restart REA from your MCP client."; +export const MCP_HTTP_LISTEN_FAILED = + "REA could not listen for MCP HTTP connections on the selected host and port. Choose another --port or stop the process using it."; diff --git a/src/main/processShutdown.ts b/src/main/processShutdown.ts index 74c51f4d6a..754b73065c 100644 --- a/src/main/processShutdown.ts +++ b/src/main/processShutdown.ts @@ -1,5 +1,11 @@ type ShutdownSignal = "SIGINT" | "SIGTERM"; +/** Exit status a signal-terminated process reports (128 + signal number). */ +const SIGNAL_EXIT_STATUS: Readonly> = { + SIGINT: 130, + SIGTERM: 143, +}; + interface ShutdownSignalSource { on(event: ShutdownSignal, listener: () => void): unknown; off(event: ShutdownSignal, listener: () => void): unknown; @@ -12,7 +18,8 @@ interface ShutdownInputSource { interface ProcessShutdownEvents { readonly signals: ShutdownSignalSource; - readonly stdin: ShutdownInputSource; + /** Absent when the transport does not own stdin, as the HTTP listener does not. */ + readonly stdin?: ShutdownInputSource; } const processShutdownEvents: ProcessShutdownEvents = { @@ -20,19 +27,91 @@ const processShutdownEvents: ProcessShutdownEvents = { stdin: process.stdin, }; +/** Receives the signal that requested shutdown; stdin end and close carry none. */ +type ShutdownHandler = (signal?: ShutdownSignal) => void; + /** Register the process events that request shutdown and return their owner. */ export const registerProcessShutdown = ( - handler: () => void, + handler: ShutdownHandler, events: ProcessShutdownEvents = processShutdownEvents, ): (() => void) => { - events.signals.on("SIGINT", handler); - events.signals.on("SIGTERM", handler); - events.stdin.on("end", handler); - events.stdin.on("close", handler); + const onInterrupt = (): void => handler("SIGINT"); + const onTerminate = (): void => handler("SIGTERM"); + const onInputEnd = (): void => handler(); + events.signals.on("SIGINT", onInterrupt); + events.signals.on("SIGTERM", onTerminate); + events.stdin?.on("end", onInputEnd); + events.stdin?.on("close", onInputEnd); return () => { - events.signals.off("SIGINT", handler); - events.signals.off("SIGTERM", handler); - events.stdin.off("end", handler); - events.stdin.off("close", handler); + events.signals.off("SIGINT", onInterrupt); + events.signals.off("SIGTERM", onTerminate); + events.stdin?.off("end", onInputEnd); + events.stdin?.off("close", onInputEnd); + }; +}; + +/** Register signals only, for a transport that does not own stdin, and report the signal's exit status. */ +export const registerSignalShutdown = ( + handler: (exitStatus: number) => void, + signals: ShutdownSignalSource = process, +): (() => void) => + registerProcessShutdown( + (signal) => { + if (signal !== undefined) handler(SIGNAL_EXIT_STATUS[signal]); + }, + { signals }, + ); + +/** A signal recorded while startup is still running, before the lifecycle owner can act on it. */ +export interface SignalLatch { + /** Stop recording; returns the exit status of the first signal seen, if any. */ + readonly release: () => number | undefined; +} + +/** Record the first SIGINT or SIGTERM during startup and exit at once on a second one, so startup never dies without its cleanup. */ +export const latchSignalShutdown = ( + signals: ShutdownSignalSource = process, + exit: (status: number) => void = (status) => process.exit(status), +): SignalLatch => { + let pending: number | undefined; + const record = (signal: ShutdownSignal) => (): void => { + if (pending === undefined) pending = SIGNAL_EXIT_STATUS[signal]; + else exit(pending); + }; + const onInterrupt = record("SIGINT"); + const onTerminate = record("SIGTERM"); + signals.on("SIGINT", onInterrupt); + signals.on("SIGTERM", onTerminate); + return { + release: () => { + signals.off("SIGINT", onInterrupt); + signals.off("SIGTERM", onTerminate); + return pending; + }, + }; +}; + +/** The exit status an entry reports: a signal's status survives the entry's normal `0` return. */ +export interface ExitStatus { + /** The entry's own result; a `0` after a recorded signal keeps the signal's status. */ + readonly set: (code: number) => void; + /** Record the exit status of a signal-requested shutdown. */ + readonly signal: (status: number) => void; +} + +export const createExitStatus = ( + setExitCode: (code: number) => void, +): ExitStatus => { + let signalStatus: number | undefined; + return { + set: (code) => { + setExitCode( + code === 0 && signalStatus !== undefined ? signalStatus : code, + ); + }, + signal: (status) => { + signalStatus = status; + setExitCode(status); + }, }; }; diff --git a/src/main/shutdown.ts b/src/main/shutdown.ts index 90d30c5753..e74ce0c894 100644 --- a/src/main/shutdown.ts +++ b/src/main/shutdown.ts @@ -1,11 +1,9 @@ -import type { StdioServerHandle } from "@modelcontextprotocol/server/stdio"; - import type { BinarySession } from "../application/binary/BinarySession.js"; import type { Logger } from "pino"; import { AnalysisError } from "../domain/analysisErrorBase.js"; import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import type { JsonValue } from "../domain/jsonValue.js"; -import type { RuntimeDependencies } from "./types.js"; +import type { McpServerHandle, RuntimeDependencies } from "./types.js"; import { MCP_SHUTDOWN_FAILED } from "./messages.js"; interface ShutdownCleanupOwner { @@ -13,8 +11,9 @@ interface ShutdownCleanupOwner { closed: boolean; } +/** Own idempotent teardown of the transport handle, providers and session for any shutdown trigger. */ export const createShutdown = (input: { - readonly handle: StdioServerHandle; + readonly handle: McpServerHandle; readonly closeAndroid?: () => Promise; readonly closeOptionalProviders?: () => Promise; readonly session: BinarySession; @@ -48,6 +47,9 @@ export const createShutdown = (input: { if (shutdownAttempt !== undefined) return shutdownAttempt; const pending = cleanupOwners.filter(({ closed }) => !closed); const attempt = (async () => { + // HTTP handlers can still own provider resources after cancellation. + // A failed drain keeps every shared owner available for a later retry. + await handle.prepareShutdown?.(); const results = await Promise.allSettled( pending.map(async (owner) => { await owner.close(); diff --git a/src/main/transport.ts b/src/main/transport.ts index c26e51a77d..3a1e7e807f 100644 --- a/src/main/transport.ts +++ b/src/main/transport.ts @@ -1,17 +1,18 @@ import { createAndroidAnalysisProvider } from "../composition/android.js"; import type { AndroidAnalysisPort } from "../application/android/AndroidAnalysisPort.js"; -import type { StdioServerHandle } from "@modelcontextprotocol/server/stdio"; import type { BinarySession } from "../application/binary/BinarySession.js"; import type { Logger } from "pino"; import { createServer } from "../server/createServer.js"; +import { StreamableHttpListenError } from "../server/http/streamableHttpListenError.js"; import type { ToolResultDelivery } from "../server/toolResult.js"; -import type { RuntimeDependencies } from "./types.js"; +import type { McpServerHandle, RuntimeDependencies } from "./types.js"; import type { OptionalProviderLoadResult } from "../application/OptionalObservationProviders.js"; import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import { MCP_CONNECTION_LOST, MCP_CONNECTION_START_FAILED, + MCP_HTTP_LISTEN_FAILED, } from "./messages.js"; interface ServerContext { @@ -26,6 +27,7 @@ type ClosableOptionalProvider = { readonly close?: () => Promise; }; +/** Start the injected transport (stdio or streamable HTTP) around one shared session. */ export const startMcpTransport = async ( dependencies: RuntimeDependencies, session: BinarySession, @@ -33,7 +35,7 @@ export const startMcpTransport = async ( ): Promise< | { readonly ok: true; - readonly handle: StdioServerHandle; + readonly handle: McpServerHandle; readonly closeAndroid: () => Promise; readonly closeOptionalProviders: () => Promise; } @@ -105,9 +107,9 @@ export const startMcpTransport = async ( }); await optionalProviderClose; }; - let handle: StdioServerHandle; + let handle: McpServerHandle; try { - handle = dependencies.serve( + handle = await dependencies.serve( () => { // The SDK can discard a discovery probe and construct a replacement server. const android = createAndroidAnalysisProvider( @@ -139,6 +141,7 @@ export const startMcpTransport = async ( serverLogger.error(MCP_CONNECTION_LOST); dependencies.writeStderr(`${MCP_CONNECTION_LOST}\n`); }, + logger: serverLogger, }, ); } catch (cause: unknown) { @@ -153,13 +156,17 @@ export const startMcpTransport = async ( closeServers(), closeOptionalProviders(), ]); + const startFailure = + cause instanceof StreamableHttpListenError + ? MCP_HTTP_LISTEN_FAILED + : MCP_CONNECTION_START_FAILED; serverLogger.error( { error: cause instanceof Error ? cause.message : String(cause), }, - MCP_CONNECTION_START_FAILED, + startFailure, ); - dependencies.writeStderr(`${MCP_CONNECTION_START_FAILED}\n`); + dependencies.writeStderr(`${startFailure}\n`); if (binaryCleanup.status === "fulfilled" && !binaryCleanup.value.ok) { const cleanupError = projectAnalysisError(binaryCleanup.value.error); serverLogger.error( @@ -191,7 +198,11 @@ export const startMcpTransport = async ( return { ok: true, handle: { + ...(handle.prepareShutdown === undefined + ? {} + : { prepareShutdown: handle.prepareShutdown }), close: async () => { + await handle.prepareShutdown?.(); const results = await Promise.allSettled([ handle.close(), closeServers(), diff --git a/src/main/types.ts b/src/main/types.ts index 8b46e25f45..72430027a6 100644 --- a/src/main/types.ts +++ b/src/main/types.ts @@ -1,8 +1,30 @@ +import type { McpServerFactory } from "@modelcontextprotocol/server"; +import type { Logger } from "pino"; + import type { OptionalProviderLoadResult } from "../application/OptionalObservationProviders.js"; +/** Teardown handle every MCP transport returns to the runtime. */ +export interface McpServerHandle { + /** Optional transport barrier before shared provider teardown. */ + readonly prepareShutdown?: () => Promise; + readonly close: () => Promise; +} + +/** Runtime inputs shared by the stdio and streamable HTTP transports. */ +export interface McpServeOptions { + readonly onerror?: (error: Error) => void; + readonly logger?: Logger; +} + +/** Start one transport around the server factory; a listener binds asynchronously. */ +export type McpServe = ( + factory: McpServerFactory, + options: McpServeOptions, +) => McpServerHandle | Promise; + export interface RuntimeDependencies { readonly env: NodeJS.ProcessEnv; - readonly serve: typeof import("@modelcontextprotocol/server/stdio").serveStdio; + readonly serve: McpServe; readonly writeStderr: (text: string) => void; readonly setExitCode: (code: number) => void; readonly registerShutdown: (handler: () => void) => () => void; diff --git a/src/server/EvidenceMcpServer.ts b/src/server/EvidenceMcpServer.ts index cbf694bd7f..33ee1f24be 100644 --- a/src/server/EvidenceMcpServer.ts +++ b/src/server/EvidenceMcpServer.ts @@ -1,7 +1,6 @@ import { AsyncLocalStorage } from "node:async_hooks"; import { - McpServer, isCallToolResult, isJSONRPCRequest, isJSONRPCNotification, @@ -40,6 +39,7 @@ import { measureMcpResultWithinBudget, TARGET_CONTENT_RESULT_META, } from "./toolResultEncoding.js"; +import { RequestTrackingMcpServer } from "./RequestTrackingMcpServer.js"; export type { CompactInputSchemaPresentation } from "../contracts/compactInputSchema.js"; @@ -104,7 +104,7 @@ export const forgetPendingMcpRequest = ( }; /** Bind oversized MCP error recovery to this server's Evidence ledger. */ -export class EvidenceMcpServer extends McpServer { +export class EvidenceMcpServer extends RequestTrackingMcpServer { readonly catalogIdentity: ReturnType; private readonly compactPresentation: | CompactInputSchemaPresentation diff --git a/src/server/RequestTrackingMcpServer.test.ts b/src/server/RequestTrackingMcpServer.test.ts new file mode 100644 index 0000000000..c1ac93955c --- /dev/null +++ b/src/server/RequestTrackingMcpServer.test.ts @@ -0,0 +1,88 @@ +import { Client } from "@modelcontextprotocol/client"; +import { InMemoryTransport } from "@modelcontextprotocol/server"; +import { expect, it } from "vitest"; +import { z } from "zod"; + +import { RequestTrackingMcpServer } from "./RequestTrackingMcpServer.js"; + +it.each([false, true])( + "preserves context-only callbacks with HTTP tracking enabled=%s", + async (tracking) => { + const server = new RequestTrackingMcpServer({ + name: "context-only", + version: "1", + }); + if (tracking) server.enableRequestTracking(); + server.registerTool("context", {}, (context) => ({ + content: [{ type: "text", text: String(context.mcpReq.signal.aborted) }], + })); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + const client = new Client({ name: "context-client", version: "1" }); + try { + await server.connect(serverTransport); + await client.connect(clientTransport); + expect((await client.callTool({ name: "context" })).content).toEqual([ + { type: "text", text: "false" }, + ]); + expect(await server.drainRequests(0)).toBe(true); + } finally { + await client.close(); + await server.close(); + } + }, +); + +it("never starts a tool whose asynchronous input validation finishes after transport cancellation", async () => { + let release!: () => void; + let entered!: () => void; + const gate = new Promise((resolve) => { + release = resolve; + }); + const validating = new Promise((resolve) => { + entered = resolve; + }); + let calls = 0; + const server = new RequestTrackingMcpServer({ + name: "validation-race", + version: "1", + }); + server.enableRequestTracking(); + server.registerTool( + "slow_validation", + { + inputSchema: z.object({ + value: z.string().refine(async () => { + entered(); + await gate; + return true; + }), + }), + }, + () => { + calls += 1; + return { content: [] }; + }, + ); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + const client = new Client({ name: "validation-client", version: "1" }); + try { + await server.connect(serverTransport); + await client.connect(clientTransport); + void client + .callTool({ name: "slow_validation", arguments: { value: "x" } }) + .catch(() => undefined); + await validating; + await server.close(); + expect(await server.drainRequests(0)).toBe(true); + release(); + await new Promise((resolve) => setImmediate(resolve)); + expect(calls).toBe(0); + expect(await server.drainRequests(0)).toBe(true); + } finally { + release(); + await client.close(); + await server.close(); + } +}); diff --git a/src/server/RequestTrackingMcpServer.ts b/src/server/RequestTrackingMcpServer.ts new file mode 100644 index 0000000000..cb9f7a17ed --- /dev/null +++ b/src/server/RequestTrackingMcpServer.ts @@ -0,0 +1,92 @@ +import { + McpServer, + type Icon, + type RegisteredTool, + type ServerContext, + type ScopeChallengeHandler, + type StandardSchemaWithJSON, + type ToolAnnotations, + type ToolCallback, +} from "@modelcontextprotocol/server"; + +/** Tracks tools automatically; other resource-using MCP callbacks use runRequest. */ +export class RequestTrackingMcpServer extends McpServer { + readonly #calls = new Set>(); + #tracking = false; + + /** HTTP sessions share resources across transports; stdio keeps the SDK's original behavior. */ + enableRequestTracking(): void { + this.#tracking = true; + } + + override registerTool< + OutputArgs extends StandardSchemaWithJSON, + InputArgs extends StandardSchemaWithJSON | undefined = undefined, + >( + name: string, + config: { + title?: string; + description?: string; + inputSchema?: InputArgs; + outputSchema?: OutputArgs; + annotations?: ToolAnnotations; + icons?: Icon[]; + scopeChallenge?: ScopeChallengeHandler; + _meta?: Record; + }, + callback: ToolCallback, + ): RegisteredTool { + // The SDK calls a schema-less callback with context alone, and a schema'd + // callback with (validated input, context). Narrow its conditional type at + // the same inputSchema discriminant used by the SDK. + const tracked = + config.inputSchema === undefined + ? (context: ServerContext) => + this.runRequest(context.mcpReq.signal, () => + (callback as ToolCallback)(context), + ) + : (args: unknown, context: ServerContext) => + this.runRequest(context.mcpReq.signal, () => + (callback as ToolCallback)(args, context), + ); + return super.registerTool( + name, + config, + tracked as ToolCallback, + ); + } + + /** Retain HTTP dispatch, tool and completion work under the HTTP owner's shared drain. */ + runRequest( + signal: AbortSignal | undefined, + callback: () => T | Promise, + ): T | Promise { + if (!this.#tracking) return callback(); + // SDK validation can finish after its transport closed. Such a callback + // must never start work against the next session's shared resources. + const call = Promise.resolve().then(() => { + signal?.throwIfAborted(); + return callback(); + }); + this.#calls.add(call); + return call.finally(() => this.#calls.delete(call)); + } + + /** Wait for running handlers, including their finally blocks; a timeout retains ownership. */ + async drainRequests(timeoutMs?: number): Promise { + if (this.#calls.size === 0) return true; + const settled = Promise.allSettled(this.#calls).then(() => true); + if (timeoutMs === undefined) return settled; + let timer: ReturnType | undefined; + try { + return await Promise.race([ + settled, + new Promise((resolve) => { + timer = setTimeout(() => resolve(false), timeoutMs); + }), + ]); + } finally { + clearTimeout(timer); + } + } +} diff --git a/src/server/http/NodeHttpRequestOwner.ts b/src/server/http/NodeHttpRequestOwner.ts new file mode 100644 index 0000000000..5563245fdc --- /dev/null +++ b/src/server/http/NodeHttpRequestOwner.ts @@ -0,0 +1,180 @@ +import type { IncomingMessage, ServerResponse } from "node:http"; + +import { writeWebResponse } from "./nodeHttpBridge.js"; +import { AnalysisResourceConstraintError } from "../../domain/analysisErrorCore.js"; + +export type HttpAdmissionLifetime = "dispatch" | "response"; + +export interface NodeHttpExchangeContext { + readonly onDispatchSettled: (state: { + readonly hasUnreadBody: boolean; + readonly admissionLifetime: HttpAdmissionLifetime; + }) => void; + readonly onBodyCleanup: (cleanup: Promise) => void; +} + +interface NodeHttpRequestTarget { + readonly serve: ( + request: IncomingMessage, + response: ServerResponse, + context: NodeHttpExchangeContext, + ) => Promise; + readonly refuse: (reason: "capacity" | "cleanup") => Response; +} + +/** Own admitted exchanges and separately joinable response-source cleanup. */ +export class NodeHttpRequestOwner { + #active = 0; + readonly #exchanges = new Set>(); + readonly #bodyCleanups = new Set>(); + readonly #cleanupFailures: unknown[] = []; + + constructor( + private readonly capacity: number, + private readonly target: NodeHttpRequestTarget, + ) { + if (!Number.isSafeInteger(capacity) || capacity < 1) + throw new RangeError( + "maxConcurrentDispatches must be a positive safe integer", + ); + } + + handle(request: IncomingMessage, response: ServerResponse): Promise { + // Node queues a pipelined response without assigning a socket until its + // predecessor finishes. Never start work or queue a refusal behind it. + if (request.socket.destroyed || response.socket === null) { + request.socket.destroy(); + return Promise.resolve(); + } + const refusal = + this.#cleanupFailures.length > 0 + ? "cleanup" + : this.#active >= this.capacity + ? "capacity" + : undefined; + if (refusal !== undefined) { + return this.#trackExchange(async () => { + try { + await writeWebResponse(this.target.refuse(refusal), response, { + closeConnection: true, + onBodyCleanup: this.#trackBodyCleanup, + }); + } catch { + response.destroy(); + } + }); + } + this.#active += 1; + let reserved = true; + let dispatched = false; + let inputDone = false; + let responseDone = false; + let holdResponse = true; + let pendingCleanups = 0; + let cleanupFailed = false; + const release = (): void => { + if (!reserved) return; + reserved = false; + this.#active -= 1; + request.off("end", inputEnded); + request.off("close", inputEnded); + response.off("close", inputEnded); + }; + const releaseIfReady = (): void => { + if ( + dispatched && + inputDone && + (!holdResponse || + (responseDone && pendingCleanups === 0 && !cleanupFailed)) + ) + release(); + }; + const inputEnded = (): void => { + inputDone = true; + releaseIfReady(); + }; + return this.#trackExchange(async () => { + if (request.socket.destroyed || response.destroyed) { + release(); + return; + } + try { + await this.target.serve(request, response, { + onBodyCleanup: (cleanup) => { + pendingCleanups += 1; + this.#trackBodyCleanup(cleanup, (succeeded) => { + pendingCleanups -= 1; + cleanupFailed ||= !succeeded; + releaseIfReady(); + }); + }, + onDispatchSettled: ({ hasUnreadBody, admissionLifetime }) => { + dispatched = true; + holdResponse = admissionLifetime === "response"; + if (!hasUnreadBody || request.destroyed || response.destroyed) { + inputEnded(); + return; + } + request.once("end", inputEnded); + request.once("close", inputEnded); + response.once("close", inputEnded); + }, + }); + } catch (cause: unknown) { + response.destroy(); + release(); + throw cause; + } finally { + responseDone = true; + releaseIfReady(); + } + }); + } + + #trackExchange(serve: () => Promise): Promise { + const exchange = Promise.resolve().then(serve); + this.#exchanges.add(exchange); + return exchange.finally(() => this.#exchanges.delete(exchange)); + } + + readonly #trackBodyCleanup = ( + cleanup: Promise, + settled?: (succeeded: boolean) => void, + ): void => { + this.#bodyCleanups.add(cleanup); + void cleanup.then( + () => { + this.#bodyCleanups.delete(cleanup); + settled?.(true); + }, + (cause: unknown) => { + this.#cleanupFailures.push(cause); + this.#bodyCleanups.delete(cleanup); + settled?.(false); + }, + ); + }; + + async drain(): Promise { + // An exchange can register cancellation after its route has returned, + // including when a late Response finds that its client already left. + while (this.#exchanges.size > 0) + await Promise.allSettled([...this.#exchanges]); + while (this.#bodyCleanups.size > 0) + await Promise.allSettled([...this.#bodyCleanups]); + if (this.#cleanupFailures.length > 0) + throw new AnalysisResourceConstraintError( + "mcp_http_response_cleanup", + "transport", + "HTTP response cleanup could not be confirmed; provider shutdown remains blocked", + { failed_cleanup_count: this.#cleanupFailures.length }, + { + cause: new AggregateError( + this.#cleanupFailures, + "Response cleanup failed", + ), + retryable: false, + }, + ); + } +} diff --git a/src/server/http/nodeHttpBridge.test.ts b/src/server/http/nodeHttpBridge.test.ts new file mode 100644 index 0000000000..82b41885cb --- /dev/null +++ b/src/server/http/nodeHttpBridge.test.ts @@ -0,0 +1,283 @@ +import { once } from "node:events"; +import { createServer, type Server } from "node:http"; +import { connect } from "node:net"; +import type { AddressInfo } from "node:net"; +import { afterEach, expect, it, vi } from "vitest"; + +import { toWebRequest, writeWebResponse } from "./nodeHttpBridge.js"; + +const servers: Server[] = []; +const encoder = new TextEncoder(); +const decoder = new TextDecoder(); + +afterEach(async () => { + for (const server of servers.splice(0)) { + server.closeAllConnections(); + await new Promise((resolve) => server.close(() => resolve(undefined))); + } +}); + +/** Serve a web handler through the bridge on an ephemeral loopback port. */ +const serve = async ( + handle: (request: Request) => Response | Promise, +): Promise => { + const server = createServer(); + servers.push(server); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + const origin = `http://127.0.0.1:${String((server.address() as AddressInfo).port)}`; + server.on("request", (request, response) => { + void (async () => { + try { + const webRequest = toWebRequest(request, response, origin); + await writeWebResponse(await handle(webRequest), response); + } catch { + response.destroy(); + } + })(); + }); + return origin; +}; + +it("ends a HEAD reply after the headers and cancels a body the handler attached", async () => { + let pulls = 0; + let cancelled = false; + const origin = await serve( + () => + new Response( + new ReadableStream({ + pull: async () => { + pulls += 1; + // Never produces a chunk, like an idle event stream. + await new Promise(() => undefined); + }, + cancel: () => { + cancelled = true; + }, + }), + { headers: { "content-type": "text/event-stream" } }, + ), + ); + const response = await fetch(`${origin}/events`, { method: "HEAD" }); + expect(response.status).toBe(200); + expect(response.headers.get("content-type")).toBe("text/event-stream"); + await vi.waitFor(() => { + expect(cancelled).toBe(true); + }); + expect(pulls).toBeLessThanOrEqual(1); +}); + +it("finishes a HEAD reply without waiting on a teed body whose sibling stays live", async () => { + const cached = new Response( + new ReadableStream({ + pull: () => new Promise(() => undefined), + }), + { headers: { "content-type": "text/event-stream" } }, + ); + let finished = false; + const server = createServer((_request, response) => { + void writeWebResponse(cached.clone(), response).then(() => { + finished = true; + }); + }); + servers.push(server); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + const { port } = server.address() as AddressInfo; + const response = await fetch(`http://127.0.0.1:${String(port)}/events`, { + method: "HEAD", + }); + expect(response.status).toBe(200); + await vi.waitFor(() => { + expect(finished).toBe(true); + }); +}); + +it("cancels the body when an asynchronous route resolves after its client left", async () => { + let cancelled = false; + let settled = false; + let received = false; + const server = createServer((_request, response) => { + received = true; + response.once("close", () => { + const idle = new Response( + new ReadableStream({ + pull: () => new Promise(() => undefined), + cancel: () => { + cancelled = true; + }, + }), + ); + const settle = (): void => { + settled = true; + }; + void writeWebResponse(idle, response).then(settle, settle); + }); + }); + servers.push(server); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + const { port } = server.address() as AddressInfo; + const socket = connect(port, "127.0.0.1"); + await once(socket, "connect"); + socket.write("GET /late HTTP/1.1\r\nHost: 127.0.0.1\r\n\r\n"); + await vi.waitFor(() => { + expect(received).toBe(true); + }); + socket.destroy(); + await vi.waitFor(() => { + expect(settled).toBe(true); + expect(cancelled).toBe(true); + }); +}); + +it("returns when a client leaves a teed streaming body whose sibling stays live", async () => { + const cached = new Response( + new ReadableStream({ + start: (controller) => { + controller.enqueue(encoder.encode("data: first\n\n")); + }, + pull: () => new Promise(() => undefined), + }), + { headers: { "content-type": "text/event-stream" } }, + ); + let settled = false; + const server = createServer((_request, response) => { + const settle = (): void => { + settled = true; + }; + void writeWebResponse(cached.clone(), response).then(settle, settle); + }); + servers.push(server); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + const { port } = server.address() as AddressInfo; + const leave = new AbortController(); + const response = await fetch(`http://127.0.0.1:${String(port)}/events`, { + signal: leave.signal, + }); + if (response.body === null) throw new Error("Expected a streamed body"); + const first = await response.body.getReader().read(); + expect(decoder.decode(first.value)).toBe("data: first\n\n"); + leave.abort(); + await vi.waitFor(() => { + expect(settled).toBe(true); + }); +}); + +it("forwards method, URL, headers and the request body as a web Request", async () => { + const seen: Request[] = []; + const origin = await serve(async (request) => { + seen.push(request); + return Response.json( + { body: await request.text() }, + { headers: { "x-reply": "bridged" } }, + ); + }); + const response = await fetch(`${origin}/mcp?page=2`, { + method: "POST", + headers: { "content-type": "application/json", "x-probe": "yes" }, + body: '{"hello":1}', + }); + expect(response.status).toBe(200); + expect(response.headers.get("x-reply")).toBe("bridged"); + await expect(response.json()).resolves.toEqual({ body: '{"hello":1}' }); + const request = seen[0]; + if (request === undefined) throw new Error("Request not bridged"); + const url = new URL(request.url); + expect(request.method).toBe("POST"); + expect(`${url.pathname}${url.search}`).toBe("/mcp?page=2"); + expect(request.headers.get("x-probe")).toBe("yes"); + expect(request.headers.get("host")).toBe(new URL(origin).host); +}); + +it("forwards GET requests without a body", async () => { + const origin = await serve((request) => + Response.json({ bodyless: request.body === null }), + ); + const response = await fetch(`${origin}/mcp`); + await expect(response.json()).resolves.toEqual({ bodyless: true }); +}); + +it("streams response chunks as the handler produces them", async () => { + let release: () => void = () => undefined; + const gate = new Promise((resolve) => { + release = resolve; + }); + const origin = await serve( + () => + new Response( + new ReadableStream({ + async start(controller) { + controller.enqueue(encoder.encode("event: message\ndata: 1\n\n")); + await gate; + controller.enqueue(encoder.encode("event: message\ndata: 2\n\n")); + controller.close(); + }, + }), + { headers: { "content-type": "text/event-stream" } }, + ), + ); + const response = await fetch(`${origin}/stream`); + expect(response.headers.get("content-type")).toBe("text/event-stream"); + const reader = response.body?.getReader(); + if (reader === undefined) throw new Error("Response body missing"); + const first = await reader.read(); + expect(decoder.decode(first.value)).toContain("data: 1"); + release(); + let rest = ""; + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + rest += decoder.decode(value); + } + expect(rest).toContain("data: 2"); +}); + +it("aborts the web Request and cancels its response body when the client leaves", async () => { + let aborted = false; + let cancelled = false; + const origin = await serve((request) => { + request.signal.addEventListener("abort", () => { + aborted = true; + }); + return new Response( + new ReadableStream({ + start(controller) { + controller.enqueue(encoder.encode(": open\n\n")); + }, + cancel() { + cancelled = true; + }, + }), + { headers: { "content-type": "text/event-stream" } }, + ); + }); + const controller = new AbortController(); + const response = await fetch(`${origin}/stream`, { + signal: controller.signal, + }); + const reader = response.body?.getReader(); + if (reader === undefined) throw new Error("Response body missing"); + await reader.read(); + controller.abort(); + await vi.waitFor(() => { + expect(aborted).toBe(true); + expect(cancelled).toBe(true); + }); +}); + +it("preserves every Set-Cookie header of a composed route", async () => { + const origin = await serve(() => { + const headers = new Headers({ "x-one": "1" }); + headers.append("set-cookie", "session=a; Path=/; HttpOnly"); + headers.append("set-cookie", "csrf=b; Path=/"); + return new Response("ok", { headers }); + }); + const response = await fetch(`${origin}/cookies`); + expect(response.headers.get("x-one")).toBe("1"); + expect(response.headers.getSetCookie()).toEqual([ + "session=a; Path=/; HttpOnly", + "csrf=b; Path=/", + ]); +}); diff --git a/src/server/http/nodeHttpBridge.ts b/src/server/http/nodeHttpBridge.ts new file mode 100644 index 0000000000..01fcacfc3e --- /dev/null +++ b/src/server/http/nodeHttpBridge.ts @@ -0,0 +1,228 @@ +import type { IncomingMessage, ServerResponse } from "node:http"; +import { finished } from "node:stream"; + +/** Fetch rejects a body on these methods, so their request streams are not forwarded. */ +const BODYLESS_METHODS: ReadonlySet = new Set(["GET", "HEAD"]); + +/** The request target was not an origin-form path; the bridge serves an origin server only. */ +export class UnsupportedRequestTargetError extends Error { + override readonly name = "UnsupportedRequestTargetError"; + constructor(target: string) { + super(`Unsupported request target ${target}`); + } +} + +/** Join an origin-form target onto the listener's origin verbatim, so `//host/path` stays a path and a path that URL parsing would rewrite (dot segments) is refused rather than routed elsewhere. */ +const requestUrl = (target: string, origin: string): URL => { + if (!target.startsWith("/") || target.includes("#")) + throw new UnsupportedRequestTargetError(target); + const url = new URL(`${origin}${target}`); + const rawPath = target.split("?")[0] ?? ""; + if (url.pathname !== rawPath) throw new UnsupportedRequestTargetError(target); + return url; +}; + +const toHeaders = (incoming: IncomingMessage["headers"]): Headers => { + const headers = new Headers(); + for (const [name, value] of Object.entries(incoming)) { + if (value === undefined) continue; + if (Array.isArray(value)) + for (const entry of value) headers.append(name, entry); + else headers.set(name, value); + } + return headers; +}; + +/** Cancelling a body stops reads; the response owner closes an unread upload after replying. */ +const requestBody = (request: IncomingMessage): ReadableStream => { + let ended = false; + let controller: ReadableStreamDefaultController; + let cleanup = (): void => undefined; + const onData = (chunk: unknown): void => { + if (ended) return; + if (!Buffer.isBuffer(chunk)) { + ended = true; + request.pause(); + request.off("data", onData); + controller.error(new TypeError("HTTP request body must be bytes")); + return; + } + controller.enqueue(new Uint8Array(chunk)); + if ((controller.desiredSize ?? 0) <= 0) request.pause(); + }; + return new ReadableStream( + { + start(target) { + controller = target; + request.pause(); + cleanup = finished( + request, + { readable: true, writable: false }, + (cause) => { + request.off("data", onData); + cleanup(); + if (ended) return; + ended = true; + if (cause) controller.error(cause); + else controller.close(); + }, + ); + request.on("data", onData); + }, + pull() { + if (!ended) request.resume(); + }, + cancel() { + ended = true; + request.pause(); + request.off("data", onData); + // Keep finished's error/close ownership until the raw request settles. + }, + }, + new ByteLengthQueuingStrategy({ + highWaterMark: request.readableHighWaterMark, + }), + ); +}; + +/** Translate a node:http request into a web Request whose signal aborts when the client leaves. */ +export const toWebRequest = ( + request: IncomingMessage, + response: ServerResponse, + origin: string, +): Request => { + const controller = new AbortController(); + response.once("close", () => { + if (!response.writableFinished) controller.abort(); + }); + const method = request.method ?? "GET"; + return new Request(requestUrl(request.url ?? "/", origin), { + method, + headers: toHeaders(request.headers), + signal: controller.signal, + ...(BODYLESS_METHODS.has(method) + ? {} + : { body: requestBody(request), duplex: "half" }), + }); +}; + +/** Header fields for writeHead; repeated Set-Cookie values survive as one array. */ +const responseHeaders = ( + headers: Headers, + closeConnection: boolean, +): Record => { + const fields: Record = {}; + for (const [name, value] of headers) + if (name !== "set-cookie") fields[name] = value; + const cookies = headers.getSetCookie(); + if (cookies.length > 0) fields["set-cookie"] = cookies; + if (closeConnection) fields.connection = "close"; + return fields; +}; + +/** Resolve on the next `drain` or `close`, removing both listeners, so a long stream does not accumulate waiters. */ +const drainOrClose = (target: ServerResponse): Promise => + new Promise((resolve) => { + if (target.destroyed) { + resolve(); + return; + } + const settle = (): void => { + target.off("drain", settle); + target.off("close", settle); + resolve(); + }; + target.once("drain", settle); + target.once("close", settle); + }); + +export interface WebResponseOptions { + readonly closeConnection?: boolean; + /** Retain cleanup independently of request completion, including pending tee cancellation. */ + readonly onBodyCleanup?: (cleanup: Promise) => void; +} + +const retainCleanup = ( + cleanup: Promise, + options: WebResponseOptions, +): void => { + options.onBodyCleanup?.(cleanup); + // The owner receives the original outcome; standalone writers stay nonblocking. + void cleanup.catch(() => undefined); +}; + +/** Copy with backpressure; request completion does not wait on source cancellation. */ +const pumpBody = async ( + reader: ReadableStreamDefaultReader, + target: ServerResponse, + cancel: () => void, +): Promise => { + let left = false; + const leave = (): void => { + if (left) return; + left = true; + cancel(); + }; + target.once("close", leave); + try { + if (target.destroyed) { + leave(); + return; + } + for ( + let next = await reader.read(); + !next.done && !left; + next = await reader.read() + ) + if (!target.write(next.value) && !left) await drainOrClose(target); + } finally { + target.off("close", leave); + } + if (!left) target.end(); +}; + +/** Stream a web Response into node:http, flushing headers and each chunk as it arrives; `closeConnection` ends the socket after this reply. A HEAD reply ends after the headers and cancels any body the route attached. */ +export const writeWebResponse = async ( + response: Response, + target: ServerResponse, + options: WebResponseOptions = {}, +): Promise => { + // One owner spans headers, pumping and completion. The pump borrows its reader, + // so every output failure cancels the same source exactly once before release. + let reader: ReadableStreamDefaultReader | undefined; + try { + reader = response.body?.getReader(); + } catch (cause: unknown) { + retainCleanup(Promise.reject(cause), options); + throw cause; + } + let cancelled = false; + const cancel = (): void => { + if (cancelled || reader === undefined) return; + cancelled = true; + retainCleanup(reader.cancel(), options); + }; + try { + if (target.destroyed) { + cancel(); + return; + } + target.writeHead( + response.status, + responseHeaders(response.headers, options.closeConnection ?? false), + ); + target.flushHeaders(); + if (reader === undefined || target.req.method === "HEAD") { + target.end(); + // A tee's sibling may remain caller-owned; delivery does not wait for its cleanup. + cancel(); + return; + } + await pumpBody(reader, target, cancel); + } catch (cause: unknown) { + cancel(); + throw cause; + } finally { + reader?.releaseLock(); + } +}; diff --git a/src/server/http/serveStreamableHttp.test.ts b/src/server/http/serveStreamableHttp.test.ts new file mode 100644 index 0000000000..9ec7af0ecd --- /dev/null +++ b/src/server/http/serveStreamableHttp.test.ts @@ -0,0 +1,1907 @@ +import { + Client, + StreamableHTTPClientTransport, +} from "@modelcontextprotocol/client"; +import { WebStandardStreamableHTTPServerTransport } from "@modelcontextprotocol/server"; +import { Agent, request as httpRequest } from "node:http"; +import { connect as connectTcp } from "node:net"; +import pino from "pino"; +import { afterEach, expect, it, vi } from "vitest"; +import { z } from "zod"; + +import { + expectForeignHostRefused, + rawHttpRequest, +} from "../../../tests/support/http/rawRequest.js"; +import { + hostHeaderAllowlist, + McpSessionSlot, + serveStreamableHttp, + type StreamableHttpServer, +} from "./serveStreamableHttp.js"; +import { StreamableHttpListenError } from "./streamableHttpListenError.js"; +import { RequestTrackingMcpServer } from "../RequestTrackingMcpServer.js"; + +const servers: StreamableHttpServer[] = []; + +const createGate = () => { + let release!: () => void; + const promise = new Promise((resolve) => { + release = resolve; + }); + return { promise, release }; +}; + +afterEach(async () => { + await Promise.all(servers.splice(0).map((server) => server.close())); +}); + +/** An echo server whose protocol onclose hook counts as a shutdown, as rea's provider cleanup does. */ +const createEchoServer = (onShutdown: () => void): RequestTrackingMcpServer => { + const server = new RequestTrackingMcpServer({ + name: "echo-fixture", + version: "1", + }); + server.registerTool( + "echo", + { description: "Echo text", inputSchema: z.object({ text: z.string() }) }, + ({ text }) => ({ content: [{ type: "text", text }] }), + ); + server.server.onclose = onShutdown; + return server; +}; + +const start = async ( + overrides: Partial[0]> = {}, +) => { + const logs: string[] = []; + const logger = pino({ level: "debug" }, { write: (line) => logs.push(line) }); + let factoryCalls = 0; + let shutdownHooks = 0; + const served = await serveStreamableHttp({ + host: "127.0.0.1", + port: 0, + createMcpServer: () => { + factoryCalls += 1; + return createEchoServer(() => { + shutdownHooks += 1; + }); + }, + logger, + ...overrides, + }); + servers.push(served); + return { + served, + messages: () => + logs.map((line) => (JSON.parse(line) as { msg: string }).msg), + entries: () => + logs.map((line) => JSON.parse(line) as Record), + factoryCalls: () => factoryCalls, + shutdownHooks: () => shutdownHooks, + }; +}; + +const connect = async (url: string) => { + const transport = new StreamableHTTPClientTransport(new URL(url)); + const client = new Client({ name: "streamable-http-adapter", version: "1" }); + await client.connect(transport); + return { transport, client }; +}; + +const echo = async (client: Client, text: string): Promise => { + const result = await client.callTool({ name: "echo", arguments: { text } }); + return result.content; +}; + +/** One keep-alive request; reports whether the server closed the socket after the reply. */ +const keepAliveRequest = ( + agent: Agent, + url: URL, + options: { + readonly method: string; + readonly headers?: Readonly>; + readonly body?: string; + }, +): Promise<{ + status: number; + headers: Record; + socketClosed: boolean; +}> => + new Promise((resolve, reject) => { + const request = httpRequest( + url, + { method: options.method, headers: { ...options.headers }, agent }, + (response) => { + const socket = response.socket; + let closed = false; + socket?.once("close", () => { + closed = true; + }); + response.resume(); + response.once("end", () => { + // Give a closing server one turn to end the socket after the reply. + setTimeout( + () => + resolve({ + status: response.statusCode ?? 0, + headers: response.headers, + socketClosed: closed || (socket?.destroyed ?? false), + }), + 50, + ); + }); + response.once("error", reject); + }, + ); + request.once("error", reject); + request.end(options.body); + }); + +/** A new client's initialize as the bridge hands it to the slot: no session id yet. */ +const initializeRequest = (): Request => + new Request("http://127.0.0.1/mcp", { + method: "POST", + headers: { + accept: "application/json, text/event-stream", + "content-type": "application/json", + }, + body: JSON.stringify({ + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: { + protocolVersion: "2025-11-25", + capabilities: {}, + clientInfo: { name: "racing-client", version: "1" }, + }, + }), + }); + +it("announces the bound URL and serves one session at a time on one live instance", async () => { + const { served, entries, messages, factoryCalls, shutdownHooks } = + await start(); + expect(new URL(served.url).pathname).toBe("/mcp"); + expect(entries()).toContainEqual( + expect.objectContaining({ + transport: "streamable-http", + url: served.url, + msg: "MCP transport listening", + }), + ); + const first = await connect(served.url); + const sessionId = first.transport.sessionId; + expect(sessionId).toEqual(expect.any(String)); + await expect(echo(first.client, "hi")).resolves.toEqual([ + { type: "text", text: "hi" }, + ]); + // A new client's initialize replaces the open session; the earlier client is refused. + const second = await connect(served.url); + expect(second.transport.sessionId).toEqual(expect.any(String)); + expect(second.transport.sessionId).not.toBe(sessionId); + await expect(echo(first.client, "stale")).rejects.toThrow(); + await expect(echo(second.client, "live")).resolves.toEqual([ + { type: "text", text: "live" }, + ]); + await first.client.close(); + // DELETE ends the session, not the server: the instance keeps serving. + await second.transport.terminateSession(); + expect(second.transport.sessionId).toBeUndefined(); + await second.client.close(); + const third = await connect(served.url); + await expect(echo(third.client, "again")).resolves.toEqual([ + { type: "text", text: "again" }, + ]); + expect(factoryCalls()).toBe(1); + expect(shutdownHooks()).toBe(0); + expect( + messages().filter((msg) => msg === "MCP HTTP session opened"), + ).toHaveLength(3); + expect(messages()).toContain("MCP HTTP session replaced by a new client"); + // Closing the listener with a session open is the shutdown the instance's hook waits for. + await served.close(); + expect(shutdownHooks()).toBe(1); + await third.client.close(); +}); + +it("keeps the instance's shutdown hook when two initialize requests race to replace an open session", async () => { + let shutdownHooks = 0; + const slot = new McpSessionSlot( + createEchoServer(() => { + shutdownHooks += 1; + }), + undefined, + ); + const opened = await slot.handle(initializeRequest()); + expect(opened.status).toBe(200); + // Both requests see the same open session and end it in lockstep. + const raced = await Promise.all([ + slot.handle(initializeRequest()), + slot.handle(initializeRequest()), + ]); + expect(raced.map((response) => response.status).sort()).toEqual([200, 400]); + const winner = raced.find((response) => response.status === 200); + expect(winner?.headers.get("mcp-session-id")).toEqual(expect.any(String)); + expect(shutdownHooks).toBe(0); + // Only the slot's own close is the shutdown the instance's hook waits for. + await slot.close(); + expect(shutdownHooks).toBe(1); +}); + +it("serves plain routes beside the MCP endpoint and answers unknown paths with 404", async () => { + const { served } = await start({ + routes: [ + { + method: "GET", + path: "/.well-known/probe", + handle: () => Response.json({ probe: true }), + }, + ], + }); + const probe = await fetch(new URL("/.well-known/probe", served.url)); + await expect(probe.json()).resolves.toEqual({ probe: true }); + const missing = await fetch(new URL("/missing", served.url)); + expect(missing.status).toBe(404); + await expect(missing.json()).resolves.toMatchObject({ + error: { code: -32601 }, + }); + const wrongMethod = await fetch(new URL("/.well-known/probe", served.url), { + method: "POST", + body: "{}", + }); + expect(wrongMethod.status).toBe(404); +}); + +it("refuses foreign Host headers on a loopback bind and frees the port on close", async () => { + const { served } = await start(); + const url = new URL(served.url); + await expectForeignHostRefused(url); + await served.close(); + await served.close(); + await expect(rawHttpRequest(url)).rejects.toMatchObject({ + code: "ECONNREFUSED", + }); +}); + +it("reports a port that cannot be bound as a typed listen error naming the cause", async () => { + const { served } = await start(); + const failed = serveStreamableHttp({ + host: "127.0.0.1", + port: Number(new URL(served.url).port), + createMcpServer: () => createEchoServer(() => undefined), + }); + await expect(failed).rejects.toBeInstanceOf(StreamableHttpListenError); + await expect(failed).rejects.toThrow(/EADDRINUSE/u); +}); + +it("keeps the SDK loopback allowlist, plus the bind's own hostname, for every loopback spelling", () => { + const sdk = ["localhost", "127.0.0.1", "[::1]"]; + expect(hostHeaderAllowlist("127.0.0.1", "127.0.0.1")).toEqual(sdk); + expect(hostHeaderAllowlist("localhost", "127.0.0.1")).toEqual(sdk); + expect(hostHeaderAllowlist("LOCALHOST", "::1")).toEqual(sdk); + expect(hostHeaderAllowlist("::1", "::1")).toEqual(sdk); + expect(hostHeaderAllowlist("0:0:0:0:0:0:0:1", "::1")).toEqual(sdk); + expect(hostHeaderAllowlist("127.0.0.2", "127.0.0.2")).toEqual([ + ...sdk, + "127.0.0.2", + ]); + expect(hostHeaderAllowlist("localhost.", "127.0.0.1")).toEqual([ + ...sdk, + "localhost.", + ]); + expect(hostHeaderAllowlist("::ffff:127.0.0.1", "::ffff:127.0.0.1")).toEqual([ + ...sdk, + "[::ffff:7f00:1]", + ]); + expect(hostHeaderAllowlist("192.0.2.10", "192.0.2.10")).toBeUndefined(); + expect(hostHeaderAllowlist("0.0.0.0", "0.0.0.0")).toBeUndefined(); + expect(hostHeaderAllowlist("::", "::")).toBeUndefined(); + expect(hostHeaderAllowlist("example.com", "192.0.2.10")).toBeUndefined(); + expect(hostHeaderAllowlist("not a host", "192.0.2.10")).toBeUndefined(); +}); + +it("refuses requests once the slot is closed and joins an initialize that is still arming", async () => { + let shutdowns = 0; + const slot = new McpSessionSlot( + createEchoServer(() => { + shutdowns += 1; + }), + undefined, + ); + const arming = slot.handle(initializeRequest()); + await slot.close(); + expect((await arming).status).toBe(503); + expect(shutdowns).toBe(1); + expect((await slot.handle(initializeRequest())).status).toBe(503); +}); + +it("retries unfinished cleanup after a close stage fails instead of replaying the failure", async () => { + let failures = 1; + const { served } = await start({ + createMcpServer: () => { + const server = createEchoServer(() => undefined); + const original = server.close.bind(server); + server.close = async () => { + if (failures > 0) { + failures -= 1; + throw new Error("provider cleanup failed"); + } + await original(); + }; + return server; + }, + }); + await expect(served.close()).rejects.toThrow("provider cleanup failed"); + await expect(served.close()).resolves.toBeUndefined(); + await expect(rawHttpRequest(new URL(served.url))).rejects.toMatchObject({ + code: "ECONNREFUSED", + }); +}); + +it("leaves a sessionless POST without the streamable headers or with a foreign Origin to refusal, keeping the session", async () => { + const { served } = await start(); + const { client } = await connect(served.url); + const url = new URL(served.url); + const body = await initializeRequest().text(); + const host = `127.0.0.1:${url.port}`; + const plain = await rawHttpRequest(url, { + method: "POST", + headers: { + host, + "content-type": "text/plain", + accept: "application/json, text/event-stream", + }, + body, + }); + expect(plain.status).toBeGreaterThanOrEqual(400); + expect(plain.status).toBeLessThan(500); + const foreign = await rawHttpRequest(url, { + method: "POST", + headers: { + host, + origin: "http://attacker.example", + "content-type": "application/json", + accept: "application/json, text/event-stream", + }, + body, + }); + expect(foreign.status).toBe(403); + expect(await echo(client, "still here")).toEqual([ + { type: "text", text: "still here" }, + ]); + await client.close(); +}); + +it("accepts a same-origin browser Origin carrying the bound port on the MCP endpoint and on routes", async () => { + const { served } = await start({ + routes: [ + { method: "GET", path: "/health", handle: () => new Response("ok") }, + ], + }); + const url = new URL(served.url); + const body = await initializeRequest().text(); + for (const name of ["127.0.0.1", "localhost"]) { + const headers = { + host: `${name}:${url.port}`, + origin: `http://${name}:${url.port}`, + }; + const route = await rawHttpRequest(new URL("/health", url), { headers }); + expect(route.status).toBe(200); + const initialize = await rawHttpRequest(url, { + method: "POST", + headers: { + ...headers, + "content-type": "application/json", + accept: "application/json, text/event-stream", + }, + body, + }); + expect(initialize.status).toBe(200); + } +}); + +it("announces the bound URL even when the log level hides info lines", async () => { + const logs: string[] = []; + const logger = pino({ level: "warn" }, { write: (line) => logs.push(line) }); + const { served } = await start({ logger }); + expect( + logs.map((line) => JSON.parse(line) as Record), + ).toContainEqual( + expect.objectContaining({ + msg: "MCP transport listening", + url: served.url, + }), + ); +}); + +it("does not let a JSON-suffixed media type replace the session", async () => { + const { served } = await start(); + const { client } = await connect(served.url); + const url = new URL(served.url); + const patch = await rawHttpRequest(url, { + method: "POST", + headers: { + host: `127.0.0.1:${url.port}`, + "content-type": "application/json-patch+json", + accept: "application/json, text/event-stream", + }, + body: await initializeRequest().text(), + }); + expect(patch.status).toBeGreaterThanOrEqual(400); + expect(patch.status).toBeLessThan(500); + expect(await echo(client, "unchanged")).toEqual([ + { type: "text", text: "unchanged" }, + ]); + await client.close(); +}); + +it("releases the listener when the server factory fails after the port is bound", async () => { + const logs: string[] = []; + const logger = pino({ level: "debug" }, { write: (line) => logs.push(line) }); + const seen: string[] = []; + const failing = serveStreamableHttp({ + host: "127.0.0.1", + port: 0, + logger, + createMcpServer: () => { + throw new Error("provider unavailable"); + }, + }); + await expect(failing).rejects.toThrow("provider unavailable"); + for (const line of logs) seen.push((JSON.parse(line) as { msg: string }).msg); + expect(seen).not.toContain("MCP transport listening"); +}); + +it("classifies loopback from the bound address and lists the configured name beside it", () => { + const sdk = ["localhost", "127.0.0.1", "[::1]"]; + expect(hostHeaderAllowlist("ip6-localhost", "::1")).toEqual([ + ...sdk, + "ip6-localhost", + ]); + expect(hostHeaderAllowlist("dev.local", "127.0.0.1")).toEqual([ + ...sdk, + "dev.local", + ]); + expect(hostHeaderAllowlist("0.0.0.0", "0.0.0.0")).toBeUndefined(); + expect(hostHeaderAllowlist("dev.local", "192.0.2.10")).toBeUndefined(); +}); + +it.each([ + ["localhost", "192.0.2.10"], + ["service.localhost", "2001:db8::10"], +])( + "does not classify configured %s as loopback when bound to %s", + (host, address) => { + expect(hostHeaderAllowlist(host, address)).toBeUndefined(); + }, +); + +it("refuses a scoped IPv6 bind before creating anything and keeps bind failures away from onerror", async () => { + let factoryCalls = 0; + const onerror = (): void => { + throw new Error("onerror must not run during startup"); + }; + await expect( + serveStreamableHttp({ + host: "fe80::1%lo0", + port: 0, + onerror, + createMcpServer: () => { + factoryCalls += 1; + return createEchoServer(() => undefined); + }, + }), + ).rejects.toBeInstanceOf(StreamableHttpListenError); + expect(factoryCalls).toBe(0); + const { served } = await start(); + await expect( + serveStreamableHttp({ + host: "127.0.0.1", + port: Number(new URL(served.url).port), + onerror, + createMcpServer: () => createEchoServer(() => undefined), + }), + ).rejects.toThrow(/EADDRINUSE/u); +}); + +it("leaves the instance to its owner when closeInstance is false", async () => { + let instanceCloses = 0; + const server = createEchoServer(() => undefined); + const original = server.close.bind(server); + server.close = async () => { + instanceCloses += 1; + await original(); + }; + const served = await serveStreamableHttp({ + host: "127.0.0.1", + port: 0, + closeInstance: false, + createMcpServer: () => server, + }); + await served.close(); + expect(instanceCloses).toBe(0); + await server.close(); + expect(instanceCloses).toBe(1); +}); + +it("requires exact Accept media ranges before a sessionless POST may replace the session", async () => { + const { served } = await start(); + const { client } = await connect(served.url); + const url = new URL(served.url); + const lookalike = await rawHttpRequest(url, { + method: "POST", + headers: { + host: `127.0.0.1:${url.port}`, + "content-type": "application/json", + accept: "application/json-patch+json, text/event-streaming", + }, + body: await initializeRequest().text(), + }); + expect(lookalike.status).toBeGreaterThanOrEqual(400); + expect(lookalike.status).toBeLessThan(500); + expect(await echo(client, "still mine")).toEqual([ + { type: "text", text: "still mine" }, + ]); + await client.close(); +}); + +it("keeps the live session when the new client left before its initialize was probed", async () => { + const slot = new McpSessionSlot( + createEchoServer(() => undefined), + undefined, + ); + const close = vi.spyOn( + WebStandardStreamableHTTPServerTransport.prototype, + "close", + ); + try { + expect((await slot.handle(initializeRequest())).status).toBe(200); + const left = new AbortController(); + left.abort(); + const abandoned = new Request(initializeRequest(), { signal: left.signal }); + expect((await slot.handle(abandoned)).status).toBe(400); + expect(close).not.toHaveBeenCalled(); + expect((await slot.handle(initializeRequest())).status).toBe(200); + expect(close).toHaveBeenCalledTimes(1); + } finally { + close.mockRestore(); + await slot.close(); + } +}); + +it("keeps a session whose transport shutdown failed so the next close retries it", async () => { + let shutdowns = 0; + const slot = new McpSessionSlot( + createEchoServer(() => { + shutdowns += 1; + }), + undefined, + ); + // The slot binds the transport's close when it arms, so the spy must exist before the first request. + const close = vi + .spyOn(WebStandardStreamableHTTPServerTransport.prototype, "close") + .mockImplementationOnce(() => Promise.reject(new Error("transport stuck"))); + try { + await slot.handle(initializeRequest()); + await expect(slot.close()).rejects.toThrow("transport stuck"); + expect(shutdowns).toBe(0); + await expect(slot.close()).resolves.toBeUndefined(); + expect(shutdowns).toBe(1); + expect((await slot.handle(initializeRequest())).status).toBe(503); + } finally { + close.mockRestore(); + } +}); + +it("runs the instance's shutdown hook exactly once when the slot closes without a live session", async () => { + let hooks = 0; + const slot = new McpSessionSlot( + createEchoServer(() => { + hooks += 1; + }), + undefined, + ); + await slot.close(); + await slot.close(); + expect(hooks).toBe(1); + const { served, shutdownHooks } = await start(); + await served.close(); + expect(shutdownHooks()).toBe(1); +}); + +it("answers an oversized sessionless POST promptly instead of stalling on the probe", async () => { + const { served } = await start(); + const { client } = await connect(served.url); + const url = new URL(served.url); + // The SDK refuses the body as soon as it exceeds the bound, so the upload may end in a broken pipe. + await rawHttpRequest(url, { + method: "POST", + headers: { + host: `127.0.0.1:${url.port}`, + "content-type": "application/json", + accept: "application/json, text/event-stream", + }, + body: `{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"pad":"${"x".repeat(4_500_000)}"}}`, + }).then( + (refused) => { + expect(refused.status).toBeGreaterThanOrEqual(400); + expect(refused.status).toBeLessThan(500); + }, + (cause: unknown) => { + expect(["EPIPE", "ECONNRESET"]).toContain( + (cause as { code?: string }).code, + ); + }, + ); + expect(await echo(client, "still served")).toEqual([ + { type: "text", text: "still served" }, + ]); + await client.close(); +}, 20_000); + +it("closes the connection after a reply sent before the request body was read, on a keep-alive client", async () => { + const { served } = await start(); + const url = new URL(served.url); + const agent = new Agent({ keepAlive: true, maxSockets: 1 }); + try { + const unread = await keepAliveRequest(agent, new URL("/nowhere", url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: '{"never":"read"}', + }); + expect(unread.status).toBe(404); + expect(unread.headers.connection).toBe("close"); + expect(unread.socketClosed).toBe(true); + // Node's parser refuses a chunked GET body itself; a declared Content-Length reaches the bridge. + const bodyOnGet = await keepAliveRequest(agent, new URL("/nowhere", url), { + method: "GET", + headers: { "content-type": "text/plain", "content-length": "15" }, + body: "a body on a GET", + }); + expect(bodyOnGet.status).toBe(404); + expect(bodyOnGet.headers.connection).toBe("close"); + expect(bodyOnGet.socketClosed).toBe(true); + const bodiless = await keepAliveRequest(agent, new URL("/nowhere", url), { + method: "GET", + }); + expect(bodiless.status).toBe(404); + expect(bodiless.headers.connection).not.toBe("close"); + expect(bodiless.socketClosed).toBe(false); + } finally { + agent.destroy(); + } +}); + +it("answers a method Fetch refuses instead of letting it escape the request guard", async () => { + const { served } = await start(); + const url = new URL(served.url); + const trace = await rawHttpRequest(url, { + method: "TRACE", + headers: { host: `127.0.0.1:${url.port}` }, + }); + expect(trace.status).toBe(405); + expect(trace.headers.connection).toBe("close"); + const { client } = await connect(served.url); + expect(await echo(client, "still alive")).toEqual([ + { type: "text", text: "still alive" }, + ]); + await client.close(); +}); + +/** One request with a caller-controlled request target, which URL-based clients normalize away. */ +const rawTargetRequest = ( + served: URL, + target: string, +): Promise<{ status: number; body: string }> => + new Promise((resolve, reject) => { + const request = httpRequest( + { + host: served.hostname, + port: Number(served.port), + path: target, + method: "GET", + agent: false, + headers: { host: `127.0.0.1:${served.port}` }, + }, + (response) => { + let body = ""; + response.setEncoding("utf8"); + response.on("data", (chunk: string) => { + body += chunk; + }); + response.on("end", () => + resolve({ status: response.statusCode ?? 0, body }), + ); + }, + ); + request.on("error", reject); + request.end(); + }); + +it("keeps a network-path request target as a path and refuses absolute-form targets", async () => { + const seen: string[] = []; + const { served } = await start({ + routes: [ + { + method: "GET", + path: "/card", + handle: (request) => { + seen.push(new URL(request.url).host); + return Response.json({ ok: true }); + }, + }, + ], + }); + const url = new URL(served.url); + const networkPath = await rawTargetRequest(url, "//attacker.example/card"); + expect(networkPath.status).toBe(404); + expect(networkPath.body).toContain( + "No route for GET //attacker.example/card", + ); + const absolute = await rawTargetRequest(url, "http://attacker.example/card"); + expect(absolute.status).toBe(400); + // Dot segments would route a different path than the one requested; they are refused, not canonicalized. + for (const target of ["/public/../mcp", "/public/%2e%2e/mcp", "/./mcp"]) + expect((await rawTargetRequest(url, target)).status).toBe(400); + expect((await rawTargetRequest(url, "/card?x=1")).status).toBe(200); + const plain = await rawTargetRequest(url, "/card"); + expect(plain.status).toBe(200); + // Both successful route calls saw only the bound host, never the attacker-selected one. + expect(seen).toEqual([`127.0.0.1:${url.port}`, `127.0.0.1:${url.port}`]); +}); + +it("refuses raw fragments without changing an encoded query value", async () => { + const handled = vi.fn((request: Request) => + Response.json({ + token: new URL(request.url).searchParams.get("token"), + url: request.url, + }), + ); + const { served } = await start({ + routes: [{ method: "GET", path: "/plain", handle: handled }], + }); + const url = new URL(served.url); + for (const target of ["/plain?token=a#b", "/plain?token=a#"]) + expect((await rawTargetRequest(url, target)).status).toBe(400); + expect(handled).not.toHaveBeenCalled(); + const valid = await rawTargetRequest(url, "/plain?token=a%23b"); + expect(valid.status).toBe(200); + expect(JSON.parse(valid.body)).toEqual({ + token: "a#b", + url: `${url.origin}/plain?token=a%23b`, + }); +}); + +it("lets only a lone initialize replace a session, not a batch that carries one", async () => { + const { served } = await start(); + const { client } = await connect(served.url); + const url = new URL(served.url); + const single = JSON.parse(await initializeRequest().text()) as unknown; + const batch = await rawHttpRequest(url, { + method: "POST", + headers: { + host: `127.0.0.1:${url.port}`, + "content-type": "application/json", + accept: "application/json, text/event-stream", + }, + body: JSON.stringify([ + single, + { jsonrpc: "2.0", method: "notifications/initialized" }, + ]), + }); + expect(batch.status).toBeGreaterThanOrEqual(400); + expect(batch.status).toBeLessThan(500); + expect(await echo(client, "not evicted")).toEqual([ + { type: "text", text: "not evicted" }, + ]); + await client.close(); +}); + +it("retries a shutdown hook that threw instead of marking it complete", async () => { + let attempts = 0; + const slot = new McpSessionSlot( + createEchoServer(() => { + attempts += 1; + if (attempts === 1) throw new Error("hook failed once"); + }), + undefined, + ); + await expect(slot.close()).rejects.toThrow("hook failed once"); + await expect(slot.close()).resolves.toBeUndefined(); + await slot.close(); + expect(attempts).toBe(2); +}); + +it("closes the connection when a route read only part of the body and keeps it after a full read", async () => { + const { served } = await start({ + routes: [ + { + method: "POST", + path: "/partial", + handle: async (request) => { + const reader = request.body?.getReader(); + await reader?.read(); + return new Response("stopped early"); + }, + }, + { + method: "POST", + path: "/whole", + handle: async (request) => + Response.json({ bytes: (await request.text()).length }), + }, + ], + }); + const url = new URL(served.url); + const agent = new Agent({ keepAlive: true, maxSockets: 1 }); + const body = "x".repeat(256 * 1024); + try { + const partial = await keepAliveRequest(agent, new URL("/partial", url), { + method: "POST", + headers: { + "content-type": "text/plain", + "content-length": String(body.length), + }, + body, + }); + expect(partial.status).toBe(200); + expect(partial.headers.connection).toBe("close"); + expect(partial.socketClosed).toBe(true); + const whole = await keepAliveRequest(agent, new URL("/whole", url), { + method: "POST", + headers: { + "content-type": "text/plain", + "content-length": String(body.length), + }, + body, + }); + expect(whole.status).toBe(200); + expect(whole.headers.connection).not.toBe("close"); + expect(whole.socketClosed).toBe(false); + } finally { + agent.destroy(); + } +}); + +it("does not let a zero-quality, quoted or SDK-refused Accept qualify a session takeover", async () => { + const { served } = await start(); + const { client } = await connect(served.url); + const url = new URL(served.url); + // The second header holds one range: the comma and `text/event-stream` sit inside a quoted parameter. + // The third is valid HTTP, but the SDK matches media types with case and would answer 406. + for (const accept of [ + "application/json;q=0, text/event-stream", + 'application/json;note=", text/event-stream;x=1"', + "Application/JSON, Text/Event-Stream", + ]) { + const refused = await rawHttpRequest(url, { + method: "POST", + headers: { + host: `127.0.0.1:${url.port}`, + "content-type": "application/json", + accept, + }, + body: await initializeRequest().text(), + }); + expect(refused.status).toBeGreaterThanOrEqual(400); + expect(refused.status).toBeLessThan(500); + } + expect(await echo(client, "kept")).toEqual([{ type: "text", text: "kept" }]); + await client.close(); +}); + +it.each(["replacement", "DELETE"])( + "waits for a cancelled tool's finally block before reusing the instance after %s", + async (ending) => { + let release!: () => void; + let entered!: () => void; + let cancelled!: () => void; + const gate = new Promise((resolve) => { + release = resolve; + }); + const started = new Promise((resolve) => { + entered = resolve; + }); + const aborted = new Promise((resolve) => { + cancelled = resolve; + }); + let state = "initial"; + let connections = 0; + const { served } = await start({ + createMcpServer: () => { + const server = createEchoServer(() => undefined); + const connectServer = server.connect.bind(server); + server.connect = async (transport) => { + connections += 1; + await connectServer(transport); + }; + server.registerTool( + "slow", + { inputSchema: z.object({}) }, + async (_args, context) => { + context.mcpReq.signal.addEventListener("abort", cancelled, { + once: true, + }); + entered(); + try { + await gate; + return { content: [] }; + } finally { + state = "old-finally"; + } + }, + ); + return server; + }, + }); + const first = await connect(served.url); + let next: Awaited> | undefined; + let replacement: ReturnType | undefined; + try { + void first.client + .callTool({ name: "slow", arguments: {} }) + .catch(() => undefined); + await started; + const deletion = + ending === "DELETE" ? first.transport.terminateSession() : undefined; + if (deletion !== undefined) await aborted; + let connected = false; + replacement = connect(served.url).then((value) => { + connected = true; + next = value; + return value; + }); + await aborted; + await new Promise((resolve) => setImmediate(resolve)); + expect(connections).toBe(1); + expect(connected).toBe(false); + expect(state).toBe("initial"); + release(); + await deletion; + const connectedNext = await replacement; + expect(state).toBe("old-finally"); + await expect(echo(connectedNext.client, "new session")).resolves.toEqual([ + { type: "text", text: "new session" }, + ]); + } finally { + release(); + await replacement?.catch(() => undefined); + await first.client.close(); + await next?.client.close(); + } + }, +); + +it.each(["ready", "failure"] as const)( + "keeps plain routes unavailable while the factory is pending before %s", + async (outcome) => { + const reserved = await start(); + const url = new URL(reserved.served.url); + await reserved.served.close(); + const factoryStarted = createGate(); + const factoryGate = createGate(); + const failure = new Error("provider unavailable"); + const handled = vi.fn(() => new Response("ready")); + const starting = serveStreamableHttp({ + host: "127.0.0.1", + port: Number(url.port), + createMcpServer: async () => { + factoryStarted.release(); + await factoryGate.promise; + if (outcome === "failure") throw failure; + return createEchoServer(() => undefined); + }, + routes: [{ method: "GET", path: "/pending", handle: handled }], + }); + const settled = starting.catch(() => undefined); + try { + await factoryStarted.promise; + expect((await fetch(new URL("/pending", url))).status).toBe(503); + expect(handled).not.toHaveBeenCalled(); + factoryGate.release(); + if (outcome === "failure") { + await expect(starting).rejects.toBe(failure); + await expect(rawHttpRequest(url)).rejects.toMatchObject({ + code: "ECONNREFUSED", + }); + } else { + await starting; + expect((await fetch(new URL("/pending", url))).status).toBe(200); + expect(handled).toHaveBeenCalledTimes(1); + } + } finally { + factoryGate.release(); + await (await settled)?.close(); + } + }, +); + +it.each(["open stream", "late response"])( + "joins response-body cancellation before provider hooks for %s", + async (timing) => { + const entered = createGate(); + const cancelling = createGate(); + const cleanup = createGate(); + const { served, shutdownHooks } = await start({ + routes: [ + { + method: "GET", + path: "/stream", + handle: async (request) => { + entered.release(); + if (timing === "late response") + await new Promise((resolve) => { + request.signal.addEventListener("abort", () => resolve(), { + once: true, + }); + }); + return new Response( + new ReadableStream({ + start(controller) { + controller.enqueue( + new TextEncoder().encode("data: ready\n\n"), + ); + }, + async cancel() { + cancelling.release(); + await cleanup.promise; + }, + }), + ); + }, + }, + ], + }); + const response = fetch(new URL("/stream", served.url)).catch( + () => undefined, + ); + await entered.promise; + if (timing === "open stream") await response; + let closed = false; + const closing = served.prepareShutdown().then(() => { + closed = true; + }); + try { + await cancelling.promise; + await new Promise((resolve) => setImmediate(resolve)); + expect(closed).toBe(false); + expect(shutdownHooks()).toBe(0); + cleanup.release(); + await closing; + expect(shutdownHooks()).toBe(1); + } finally { + cleanup.release(); + await closing; + await (await response)?.body?.cancel().catch(() => undefined); + } + }, +); + +it.each(["chunk", "headers"])( + "joins response cleanup after an output failure in %s", + async (failure) => { + const cleanup = createGate(); + let cancelled = 0; + const source = new ReadableStream({ + start(controller) { + // Node's Response accepts this malformed producer; the HTTP writer rejects it. + controller.enqueue( + failure === "chunk" + ? ({} as unknown as Uint8Array) + : new TextEncoder().encode("pending"), + ); + }, + async cancel() { + cancelled += 1; + await cleanup.promise; + }, + }); + const { served, shutdownHooks } = await start({ + maxConcurrentDispatches: 1, + routes: [ + { method: "GET", path: "/health", handle: () => new Response("ok") }, + { + method: "GET", + path: "/broken", + handle: () => + new Response( + source, + failure === "headers" + ? { headers: { "x-invalid": "\u0001" } } + : undefined, + ), + }, + ], + }); + const received = fetch(new URL("/broken", served.url)) + .then(async (response) => { + await response.arrayBuffer(); + }) + .catch(() => undefined); + let closing: Promise | undefined; + try { + await vi.waitFor(() => { + expect(cancelled).toBe(1); + }); + expect((await fetch(new URL("/health", served.url))).status).toBe(503); + let closed = false; + closing = served.prepareShutdown().then(() => { + closed = true; + }); + await new Promise((resolve) => setImmediate(resolve)); + expect(closed).toBe(false); + expect(shutdownHooks()).toBe(0); + cleanup.release(); + await closing; + expect(shutdownHooks()).toBe(1); + expect(cancelled).toBe(1); + } finally { + cleanup.release(); + await source.cancel().catch(() => undefined); + await closing; + await received; + } + }, +); + +it("keeps HEAD delivery prompt while joining a retained tee's cleanup on shutdown", async () => { + let sourceClosed = false; + const original = new Response( + new ReadableStream({ + pull: () => new Promise(() => undefined), + cancel: () => { + sourceClosed = true; + }, + }), + ); + const { served, shutdownHooks } = await start({ + routes: [ + { method: "HEAD", path: "/cached", handle: () => original.clone() }, + ], + }); + const response = await fetch(new URL("/cached", served.url), { + method: "HEAD", + }); + expect(response.status).toBe(200); + let closed = false; + const closing = served.prepareShutdown().then(() => { + closed = true; + }); + try { + await new Promise((resolve) => setImmediate(resolve)); + expect(closed).toBe(false); + expect(sourceClosed).toBe(false); + expect(shutdownHooks()).toBe(0); + await original.body?.cancel(); + await closing; + expect(sourceClosed).toBe(true); + expect(shutdownHooks()).toBe(1); + } finally { + await original.body?.cancel(); + await closing; + } +}); + +it("retains failed response cleanup and blocks provider teardown and further admission", async () => { + const failure = new Error("response finalizer failed"); + const { served, shutdownHooks } = await start({ + routes: [ + { method: "GET", path: "/health", handle: () => new Response("ok") }, + { + method: "GET", + path: "/failed", + handle: () => + new Response( + new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode("ready")); + }, + cancel: () => Promise.reject(failure), + }), + ), + }, + ], + }); + const response = await fetch(new URL("/failed", served.url)); + try { + await response.body?.cancel(); + await vi.waitFor(async () => { + expect((await fetch(new URL("/health", served.url))).status).toBe(503); + }); + await expect(served.prepareShutdown()).rejects.toMatchObject({ + _tag: "AnalysisResourceConstraintError", + reportedLimits: { failed_cleanup_count: 1 }, + cause: expect.any(AggregateError), + }); + await expect(served.close()).rejects.toMatchObject({ + reportedLimits: { failed_cleanup_count: 1 }, + }); + expect(shutdownHooks()).toBe(0); + await expect(rawHttpRequest(new URL(served.url))).rejects.toMatchObject({ + code: "ECONNREFUSED", + }); + } finally { + await served.close().catch(() => undefined); + // The deliberate finalizer failure is retained; this fixture owns no real provider. + servers.splice(servers.indexOf(served), 1); + } +}); + +it("retains plain-response admission until asynchronous cancellation settles", async () => { + const cancelling = createGate(); + const cleanup = createGate(); + const health = vi.fn(() => new Response("ok")); + const { served } = await start({ + maxConcurrentDispatches: 1, + routes: [ + { method: "GET", path: "/health", handle: health }, + { + method: "GET", + path: "/stream", + handle: () => + new Response( + new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode("ready")); + }, + async cancel() { + cancelling.release(); + await cleanup.promise; + }, + }), + ), + }, + ], + }); + const response = await fetch(new URL("/stream", served.url)); + try { + expect((await fetch(new URL("/health", served.url))).status).toBe(503); + await response.body?.cancel(); + await cancelling.promise; + expect((await fetch(new URL("/health", served.url))).status).toBe(503); + expect(health).not.toHaveBeenCalled(); + cleanup.release(); + await vi.waitFor(async () => { + expect((await fetch(new URL("/health", served.url))).status).toBe(200); + }); + } finally { + cleanup.release(); + await response.body?.cancel(); + } +}); + +it("supports sequential keep-alive requests on the same connection", async () => { + const { served } = await start({ + routes: [ + { method: "GET", path: "/health", handle: () => new Response("ok") }, + ], + }); + const agent = new Agent({ keepAlive: true, maxSockets: 1 }); + const connections = new Set(); + agent.on("free", (socket: unknown) => connections.add(socket)); + try { + for (let i = 0; i < 2; i += 1) { + const result = await keepAliveRequest( + agent, + new URL("/health", served.url), + { method: "GET" }, + ); + expect(result.status).toBe(200); + expect(result.socketClosed).toBe(false); + expect(connections.size).toBe(1); + } + } finally { + agent.destroy(); + } +}); + +it.each(["pending handler", "open response"])( + "closes a peer pipelining behind a %s without queuing refusals", + async (mode) => { + const first = createGate(); + const fallback = createGate(); + let finalized = 0; + let secondCalled = 0; + const held = async (request: Request) => { + try { + await Promise.race([ + fallback.promise, + new Promise((resolve) => { + request.signal.addEventListener("abort", () => resolve(), { + once: true, + }); + }), + ]); + } finally { + finalized += 1; + } + return new Response("done"); + }; + const health = vi.fn(() => new Response("ok")); + const { served } = await start({ + maxConcurrentDispatches: 2, + routes: [ + { + method: "GET", + path: "/first", + handle: (request) => { + first.release(); + return mode === "pending handler" + ? held(request) + : new Response( + new ReadableStream({ + start(controller) { + controller.enqueue( + new TextEncoder().encode("data: ready\n\n"), + ); + }, + }), + ); + }, + }, + { + method: "GET", + path: "/second", + handle: (request) => { + secondCalled += 1; + return held(request); + }, + }, + { method: "GET", path: "/health", handle: health }, + ], + }); + const url = new URL(served.url); + const socket = connectTcp(Number(url.port), url.hostname); + socket.on("error", () => undefined); + socket.on("data", () => undefined); + const request = (path: string) => + `GET ${path} HTTP/1.1\r\nHost: ${url.host}\r\n\r\n`; + try { + await new Promise((resolve) => { + socket.once("connect", resolve); + }); + socket.write(request("/first")); + await first.promise; + socket.write(request("/second")); + socket.write(request("/health").repeat(32)); + await vi.waitFor(() => { + expect(socket.destroyed).toBe(true); + }); + await vi.waitFor(() => { + expect(finalized).toBe(mode === "pending handler" ? 1 : 0); + }); + expect(secondCalled).toBe(0); + expect(health).not.toHaveBeenCalled(); + } finally { + fallback.release(); + socket.destroy(); + } + }, +); + +it.each(["prepareShutdown", "close"] as const)( + "waits for plain-route finalizers before provider hooks during %s", + async (method) => { + let entered!: () => void; + let finalizing!: () => void; + let release!: () => void; + const started = new Promise((resolve) => { + entered = resolve; + }); + const cleanupStarted = new Promise((resolve) => { + finalizing = resolve; + }); + const gate = new Promise((resolve) => { + release = resolve; + }); + const { served, shutdownHooks } = await start({ + routes: [ + { + method: "GET", + path: "/pending", + handle: async (request) => { + entered(); + try { + await new Promise((resolve) => { + request.signal.addEventListener("abort", () => resolve(), { + once: true, + }); + }); + } finally { + finalizing(); + await gate; + } + return new Response("done"); + }, + }, + ], + }); + const response = fetch(new URL("/pending", served.url)).catch( + () => undefined, + ); + await started; + let closed = false; + const closing = served[method]().then(() => { + closed = true; + }); + try { + await cleanupStarted; + await new Promise((resolve) => setImmediate(resolve)); + expect(closed).toBe(false); + expect(shutdownHooks()).toBe(0); + release(); + await closing; + expect(shutdownHooks()).toBe(1); + } finally { + release(); + await closing; + await response; + } + }, +); + +it("retains admission for an unread upload attached to a long response", async () => { + const { served } = await start({ + maxConcurrentDispatches: 1, + routes: [ + { method: "GET", path: "/health", handle: () => new Response("ok") }, + { + method: "POST", + path: "/stream", + handle: async (request) => { + const reader = request.body?.getReader(); + await reader?.read(); + reader?.releaseLock(); + return new Response( + new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode("data: ready\n\n")); + }, + }), + { headers: { "content-type": "text/event-stream" } }, + ); + }, + }, + ], + }); + let received = false; + const upload = httpRequest( + new URL("/stream", served.url), + { + method: "POST", + headers: { "content-length": "1024" }, + }, + (response) => { + response.once("data", () => { + received = true; + }); + response.once("error", () => undefined); + response.resume(); + }, + ); + upload.once("error", () => undefined); + upload.write("prefix"); + try { + await vi.waitFor(() => { + expect(received).toBe(true); + }); + expect((await fetch(new URL("/health", served.url))).status).toBe(503); + upload.destroy(); + await vi.waitFor(async () => { + expect((await fetch(new URL("/health", served.url))).status).toBe(200); + }); + } finally { + upload.destroy(); + } +}); + +it("bounds HTTP input dispatch globally and releases admission when an upload leaves", async () => { + const { served } = await start({ + maxConcurrentDispatches: 2, + routes: [ + { method: "GET", path: "/health", handle: () => new Response("ok") }, + ], + }); + const first = await connect(served.url); + const uploads: ReturnType[] = []; + const pending: Promise[] = []; + try { + for (let i = 0; i < 2; i += 1) { + pending.push( + new Promise((resolve) => { + const upload = httpRequest( + served.url, + { + method: "POST", + headers: { + accept: "application/json, text/event-stream", + "content-type": "application/json", + }, + }, + (response) => { + response.resume(); + response.once("end", resolve); + response.once("error", () => resolve()); + }, + ); + uploads.push(upload); + upload.once("error", () => resolve()); + upload.write('{"jsonrpc":"2.0",'); + }), + ); + } + // A plain route observes the same budget while both sessionless MCP probes + // wait for the rest of their bodies. Idle SSE connections consume no permit. + await vi.waitFor(async () => { + expect((await fetch(new URL("/health", served.url))).status).toBe(503); + }); + const refused = await rawHttpRequest(new URL(served.url), { + method: "POST", + headers: { + accept: "application/json, text/event-stream", + "content-type": "application/json", + "content-length": String(4 * 1024 * 1024), + }, + }); + expect(refused.status).toBe(503); + expect(refused.headers.connection).toBe("close"); + uploads[0]?.destroy(); + await pending[0]; + await vi.waitFor(async () => { + expect((await fetch(new URL("/health", served.url))).status).toBe(200); + }); + expect(await echo(first.client, "kept during overload")).toEqual([ + { type: "text", text: "kept during overload" }, + ]); + } finally { + for (const upload of uploads) upload.destroy(); + await Promise.all(pending); + await first.client.close(); + } +}); + +it.each([ + { status: 406, headers: { accept: "application/json" } }, + { status: 415, headers: { "content-type": "text/plain" } }, + { status: 413, headers: { "content-length": String(8 * 1024 * 1024) } }, +])( + "releases an unread body when the SDK refuses it with $status, keeping the live session", + async ({ status, headers }) => { + const slot = new McpSessionSlot( + createEchoServer(() => undefined), + undefined, + ); + const opened = await slot.handle(initializeRequest()); + const sessionId = opened.headers.get("mcp-session-id") ?? ""; + const cancelled = vi.fn(); + const body = new ReadableStream( + { cancel: cancelled }, + { highWaterMark: 0 }, + ); + const requestHeaders = new Headers({ + accept: "application/json, text/event-stream", + "content-type": "application/json", + "mcp-session-id": sessionId, + }); + for (const [name, value] of Object.entries(headers)) + if (value !== undefined) requestHeaders.set(name, value); + try { + const refused = await slot.handle( + new Request("http://127.0.0.1/mcp", { + method: "POST", + headers: requestHeaders, + body, + duplex: "half", + } as RequestInit), + ); + expect(refused.status).toBe(status); + await new Promise((resolve) => setImmediate(resolve)); + expect(cancelled).toHaveBeenCalledTimes(1); + const live = await slot.handle( + new Request("http://127.0.0.1/mcp", { + method: "POST", + headers: { + accept: "application/json, text/event-stream", + "content-type": "application/json", + "mcp-session-id": sessionId, + }, + body: JSON.stringify({ + jsonrpc: "2.0", + id: 3, + method: "tools/call", + params: { name: "echo", arguments: { text: "kept" } }, + }), + }), + ); + expect(live.status).toBe(200); + expect(live.headers.get("mcp-session-id")).toBe(sessionId); + expect(await live.text()).toContain('"text":"kept"'); + } finally { + await slot.close(); + } + }, +); + +it.each(["DELETE", "shutdown"])( + "ends a real HTTP request with an unfinished upload during %s", + async (ending) => { + const { served, shutdownHooks } = await start(); + const first = await connect(served.url); + const entered = vi.spyOn( + WebStandardStreamableHTTPServerTransport.prototype, + "handleRequest", + ); + let next: Awaited> | undefined; + let upload!: ReturnType; + const response = new Promise((resolve, reject) => { + upload = httpRequest( + served.url, + { + method: "POST", + headers: { + accept: "application/json, text/event-stream", + "content-type": "application/json", + "mcp-session-id": first.transport.sessionId ?? "", + }, + }, + (reply) => { + reply.resume(); + reply.once("end", () => resolve(reply.statusCode)); + reply.once("error", reject); + }, + ); + upload.once("error", (cause) => { + if (ending === "shutdown") resolve(undefined); + else reject(cause); + }); + upload.write('{"jsonrpc":"2.0","id":2,"method":"tools/call",'); + }); + try { + await vi.waitFor(() => { + expect( + entered.mock.calls.some( + ([request]) => + request.method === "POST" && + request.headers.get("mcp-session-id") === + first.transport.sessionId, + ), + ).toBe(true); + }); + if (ending === "DELETE") { + await first.transport.terminateSession(); + expect(await response).toBe(404); + next = await connect(served.url); + expect(await echo(next.client, "after upload")).toEqual([ + { type: "text", text: "after upload" }, + ]); + expect(shutdownHooks()).toBe(0); + } else { + await served.close(); + await response; + expect(shutdownHooks()).toBe(1); + } + } finally { + upload.destroy(); + entered.mockRestore(); + await first.client.close(); + await next?.client.close(); + } + }, +); + +it.each(["replacement", "DELETE", "shutdown"])( + "refuses an old session's tool body that finishes reading after %s", + async (ending) => { + const server = createEchoServer(() => undefined); + const called = vi.fn(() => ({ content: [] })); + server.registerTool("record", { inputSchema: z.object({}) }, called); + const slot = new McpSessionSlot(server, undefined); + const opened = await slot.handle(initializeRequest()); + const sessionId = opened.headers.get("mcp-session-id"); + expect(sessionId).toEqual(expect.any(String)); + let release!: () => void; + const gate = new Promise((resolve) => { + release = resolve; + }); + let reading!: () => void; + const readStarted = new Promise((resolve) => { + reading = resolve; + }); + const body = new ReadableStream( + { + async pull(controller) { + reading(); + await gate; + controller.enqueue( + new TextEncoder().encode( + JSON.stringify({ + jsonrpc: "2.0", + id: 2, + method: "tools/call", + params: { name: "record", arguments: {} }, + }), + ), + ); + controller.close(); + }, + }, + { highWaterMark: 0 }, + ); + const pending = slot.handle( + new Request("http://127.0.0.1/mcp", { + method: "POST", + headers: { + accept: "application/json, text/event-stream", + "content-type": "application/json", + "mcp-session-id": sessionId ?? "", + }, + body, + // Node requires duplex for streaming Web Requests. + duplex: "half", + } as RequestInit), + ); + try { + // With no stream prefetch, this proves the SDK is waiting on the old body. + await readStarted; + if (ending === "shutdown") await slot.close(); + else { + if (ending === "DELETE") { + const deleted = await slot.handle( + new Request("http://127.0.0.1/mcp", { + method: "DELETE", + headers: { "mcp-session-id": sessionId ?? "" }, + }), + ); + expect(deleted.status).toBe(200); + } + const next = await slot.handle(initializeRequest()); + expect(next.status).toBe(200); + expect(next.headers.get("mcp-session-id")).not.toBe(sessionId); + } + release(); + const refused = await pending; + expect(refused.status).toBe(404); + await expect(refused.json()).resolves.toMatchObject({ + error: { message: "Session not found" }, + }); + expect(called).not.toHaveBeenCalled(); + } finally { + release(); + await pending; + await slot.close(); + } + }, +); + +it.each(["replacement", "shutdown"])( + "does not start old-session tools across %s at sampled SDK dispatch orderings", + async (ending) => { + const late: number[] = []; + for (let offset = 0; offset < 40; offset += 1) { + let ended = false; + let connections = 0; + const server = createEchoServer(() => undefined); + const connectServer = server.connect.bind(server); + server.connect = async (transport) => { + connections += 1; + if (connections > 1) ended = true; + await connectServer(transport); + }; + server.registerTool("record", { inputSchema: z.object({}) }, () => { + if (ended) late.push(offset); + return { content: [] }; + }); + const slot = new McpSessionSlot(server, undefined); + const opened = await slot.handle(initializeRequest()); + const pending = slot.handle( + new Request("http://127.0.0.1/mcp", { + method: "POST", + headers: { + accept: "application/json, text/event-stream", + "content-type": "application/json", + "mcp-session-id": opened.headers.get("mcp-session-id") ?? "", + }, + body: JSON.stringify({ + jsonrpc: "2.0", + id: 2, + method: "tools/call", + params: { name: "record", arguments: {} }, + }), + }), + ); + try { + // Exercise public Request/SDK scheduling, without mocking dispatch or + // requiring one particular number of internal SDK await boundaries. + for (let step = 0; step < offset; step += 1) await Promise.resolve(); + if (ending === "replacement") { + expect((await slot.handle(initializeRequest())).status).toBe(200); + } else { + await slot.close(); + ended = true; + } + const response = await pending; + await response.body?.cancel(); + await new Promise((resolve) => setImmediate(resolve)); + } finally { + await slot.close(); + } + } + expect(late).toEqual([]); + }, +); + +it.each(["after timeout", "during drain"])( + "automatically completes shutdown %s when the retained handler settles", + async (shutdownTiming) => { + let release!: () => void; + let entered!: () => void; + let draining!: () => void; + const gate = new Promise((resolve) => { + release = resolve; + }); + const started = new Promise((resolve) => { + entered = resolve; + }); + const drainStarted = new Promise((resolve) => { + draining = resolve; + }); + let shutdowns = 0; + const { served } = await start({ + sessionDrainTimeoutMs: 0, + createMcpServer: () => { + const server = createEchoServer(() => { + shutdowns += 1; + }); + const drain = server.drainRequests.bind(server); + vi.spyOn(server, "drainRequests").mockImplementation((timeoutMs) => { + const result = drain(timeoutMs); + draining(); + return result; + }); + server.registerTool("slow", { inputSchema: z.object({}) }, async () => { + entered(); + await gate; + return { content: [] }; + }); + return server; + }, + }); + const first = await connect(served.url); + try { + void first.client + .callTool({ name: "slow", arguments: {} }) + .catch(() => undefined); + await started; + const replacement = fetch(served.url, { + method: "POST", + headers: { + "content-type": "application/json", + accept: "application/json, text/event-stream", + }, + body: await initializeRequest().text(), + }).catch((cause: unknown) => { + // Full shutdown closes listener sockets, including this pending handover. + if (shutdownTiming === "during drain") return undefined; + throw cause; + }); + await drainStarted; + const pendingClose = + shutdownTiming === "during drain" ? served.close() : undefined; + const refused = await replacement; + if (shutdownTiming === "after timeout") expect(refused?.status).toBe(503); + let closed = false; + const closing = (pendingClose ?? served.close()).then(() => { + closed = true; + }); + await new Promise((resolve) => setImmediate(resolve)); + expect(closed).toBe(false); + expect(shutdowns).toBe(0); + release(); + await closing; + expect(shutdowns).toBe(1); + } finally { + release(); + await first.client.close(); + } + }, +); diff --git a/src/server/http/serveStreamableHttp.ts b/src/server/http/serveStreamableHttp.ts new file mode 100644 index 0000000000..91305f8e13 --- /dev/null +++ b/src/server/http/serveStreamableHttp.ts @@ -0,0 +1,697 @@ +import { + hostHeaderValidationResponse, + isInitializeRequest, + isJsonContentType, + localhostAllowedHostnames, + originValidationResponse, + readRequestBody, + WebStandardStreamableHTTPServerTransport, + type McpServer, + type McpRequestContext, + type Server, +} from "@modelcontextprotocol/server"; +import { randomUUID } from "node:crypto"; +import { once } from "node:events"; +import { + createServer as createHttpServer, + type IncomingMessage, + type ServerResponse, +} from "node:http"; +import { isIPv4, isIPv6, type AddressInfo } from "node:net"; +import type { Logger } from "pino"; + +import { + toWebRequest, + UnsupportedRequestTargetError, + writeWebResponse, +} from "./nodeHttpBridge.js"; +import { StreamableHttpListenError } from "./streamableHttpListenError.js"; +import { + NodeHttpRequestOwner, + type HttpAdmissionLifetime, + type NodeHttpExchangeContext, +} from "./NodeHttpRequestOwner.js"; +import { RequestTrackingMcpServer } from "../RequestTrackingMcpServer.js"; +import { AnalysisResourceConstraintError } from "../../domain/analysisErrorCore.js"; + +/** MCP endpoint path shared by `rea mcp --http` and agent environment servers. */ +export const MCP_HTTP_PATH = "/mcp"; + +/** One plain HTTP route served beside the MCP endpoint as web Request to Response. */ +export interface StreamableHttpRoute { + readonly method: string; + readonly path: string; + readonly handle: (request: Request) => Response | Promise; +} + +export interface ServeStreamableHttpOptions { + readonly host: string; + readonly port: number; + /** Called once; the instance serves every session in turn on a fresh transport. */ + readonly createMcpServer: ( + context: McpRequestContext, + ) => RequestTrackingMcpServer | Promise; + /** Maximum time a client session handover waits for cancelled tool/completion handlers. */ + readonly sessionDrainTimeoutMs?: number; + /** Concurrent HTTP input/route work (default 8); unread uploads retain their permit. */ + readonly maxConcurrentDispatches?: number; + readonly routes?: readonly StreamableHttpRoute[]; + readonly logger?: Logger; + /** Reports listener failures after binding; request failures are logged instead. */ + readonly onerror?: (error: Error) => void; + /** Close the server instance as the last stage of `close`; false when the factory's owner closes it. */ + readonly closeInstance?: boolean; +} + +export interface StreamableHttpServer { + /** The bound MCP endpoint URL. */ + readonly url: string; + /** Stop incoming dispatch and drain handlers before shared provider owners close. */ + readonly prepareShutdown: () => Promise; + readonly close: () => Promise; +} + +/** The bind host as the SDK compares Host headers: lowercase, IPv6 bracketed and canonical. */ +const normalizedHostname = (host: string): string | undefined => { + try { + return new URL(`http://${isIPv6(host) ? `[${host}]` : host}`).hostname; + } catch { + return undefined; + } +}; +const isLoopbackIpv4 = (address: string): boolean => + isIPv4(address) && address.startsWith("127."); +/** Classify the native bound IP; configured DNS names are not binding evidence. */ +const isLoopbackAddress = (hostname: string): boolean => { + if (isLoopbackIpv4(hostname)) return true; + if (!hostname.startsWith("[") || !hostname.endsWith("]")) return false; + const inner = hostname.slice(1, -1); + if (inner === "::1") return true; + if (!inner.startsWith("::ffff:")) return false; + const mapped = inner.slice("::ffff:".length); + if (isLoopbackIpv4(mapped)) return true; + const hex = /^([0-9a-f]{1,4}):[0-9a-f]{1,4}$/u.exec(mapped); + return hex !== null && Number.parseInt(hex[1] ?? "0", 16) >> 8 === 0x7f; +}; +/** Classify the bound address first, then allow its address and configured name. */ +export const hostHeaderAllowlist = ( + host: string, + boundAddress: string, +): string[] | undefined => { + const bound = normalizedHostname(boundAddress); + if (bound === undefined || !isLoopbackAddress(bound)) return undefined; + const names = [normalizedHostname(host), bound]; + const known = names.filter((name): name is string => name !== undefined); + return [...new Set([...localhostAllowedHostnames(), ...known])]; +}; + +const jsonError = (status: number, code: number, message: string): Response => + Response.json( + { jsonrpc: "2.0", error: { code, message }, id: null }, + { status }, + ); + +const protocolOf = (instance: McpServer | Server): Server => + "server" in instance ? instance.server : instance; + +const describe = (cause: unknown): string => + cause instanceof Error ? cause.message : String(cause); + +const NO_SHUTDOWN_HOOK = (): void => undefined; +const SHUTTING_DOWN = (): Response => + jsonError(503, -32000, "Server shutting down"); +const SESSION_NOT_FOUND = (): Response => + jsonError(404, -32001, "Session not found"); +const DEFAULT_SESSION_DRAIN_TIMEOUT_MS = 10_000; +const DEFAULT_MAX_CONCURRENT_DISPATCHES = 8; + +class SessionDrainTimeoutError extends AnalysisResourceConstraintError { + constructor(timeoutMs: number) { + super( + "mcp_http_session", + "transport", + "Previous MCP session still has running requests; retry after they finish", + { drain_timeout_ms: timeoutMs }, + ); + } +} + +/** Split a header field on `separator` outside quoted strings, honouring backslash escapes (RFC 9110). */ +const splitUnquoted = (value: string, separator: string): string[] => { + const parts: string[] = []; + let current = ""; + let quoted = false; + let escaped = false; + for (const char of value) { + if (escaped) escaped = false; + else if (quoted && char === "\\") escaped = true; + else if (char === '"') quoted = !quoted; + else if (char === separator && !quoted) { + parts.push(current); + current = ""; + continue; + } + current += char; + } + parts.push(current); + return parts; +}; +/** The media type proper of one header value or Accept range: no parameters, lowercase. */ +const mediaType = (value: string): string => + (splitUnquoted(value, ";")[0] ?? "").trim().toLowerCase(); +/** An Accept range's quality weight; a missing or unreadable `q` counts as 1, as in RFC 9110. */ +const quality = (range: string): number => { + const q = splitUnquoted(range, ";") + .slice(1) + .map((parameter) => parameter.split("=")) + .find(([name]) => name?.trim().toLowerCase() === "q")?.[1]; + const value = q === undefined ? 1 : Number(q.trim()); + return Number.isFinite(value) ? value : 1; +}; +/** The headers the SDK requires of a streamable POST; a request without them is left to the transport's refusal. */ +const hasStreamablePostHeaders = (request: Request): boolean => { + const accept = request.headers.get("accept") ?? ""; + // Exact media ranges with a positive weight: `application/json-patch+json`, `text/event-streaming` + // and `application/json;q=0` do not count. + const accepted = new Set( + splitUnquoted(accept, ",") + .filter((range) => quality(range) > 0) + .map(mediaType), + ); + // The SDK's own checks too: its Accept test is a case-sensitive substring match, so a header it + // would refuse with 406 (such as `Application/JSON`) must not close the live session first. + return ( + accepted.has("application/json") && + accepted.has("text/event-stream") && + accept.includes("application/json") && + accept.includes("text/event-stream") && + isJsonContentType(request.headers.get("content-type")) + ); +}; + +/** Only a new client's initialize travels without a session id; peek within the SDK body bound. */ +const isInitializePost = async (request: Request): Promise => { + if ( + request.method !== "POST" || + request.headers.has("mcp-session-id") || + !hasStreamablePostHeaders(request) + ) + return false; + const probe = request.clone(); + try { + const read = await readRequestBody(probe); + if (read.tooLarge) return false; + const body: unknown = JSON.parse(read.text); + // The transport accepts only a lone initialize, so only that may evict a session. + return Array.isArray(body) + ? body.length === 1 && isInitializeRequest(body[0]) + : isInitializeRequest(body); + } catch { + return false; + } finally { + // The clone is one branch of a tee; awaiting its cancel would wait for the original to drain. + void probe.body?.cancel().catch(() => undefined); + } +}; + +/** Owns parsing and dispatch as well as the connected transport of one session. */ +class LiveSession { + #closed = false; + readonly #bodyAbort = new AbortController(); + readonly #shutdown: () => Promise; + + constructor( + readonly transport: WebStandardStreamableHTTPServerTransport, + private readonly instance: RequestTrackingMcpServer, + ) { + this.#shutdown = transport.close.bind(transport); + } + + /** Install after connect, so the SDK and Evidence adapter have bound their callbacks. */ + fenceDispatch(): void { + const dispatch = this.transport.onmessage; + this.transport.onmessage = (...args) => { + if (!this.#closed) dispatch?.(...args); + }; + } + + handle(request: Request): Promise { + if (this.#closed) return Promise.resolve(SESSION_NOT_FOUND()); + // DELETE awaits this session's close; including it in its own drain would deadlock. + if (request.method === "DELETE") + return this.transport.handleRequest(request); + return Promise.resolve( + this.instance.runRequest(undefined, async () => { + if (this.#closed) return SESSION_NOT_FOUND(); + const bodyAbort = new AbortController(); + try { + const guarded = + request.body === null + ? request + : new Request(request, { + body: request.body.pipeThrough( + new TransformStream(), + { + signal: AbortSignal.any([ + this.#bodyAbort.signal, + bodyAbort.signal, + ]), + }, + ), + duplex: "half", + } as RequestInit); + const response = await this.transport.handleRequest(guarded); + if (!this.#closed) return response; + // The SDK may have crossed an await before close and created its SSE + // response afterwards. Cancel that stream before releasing its owner. + await response.body?.cancel(); + return SESSION_NOT_FOUND(); + } finally { + // Header/size refusals can leave the SDK body unread. Its forwarding + // stream belongs to this dispatch, not the remaining session lifetime. + bodyAbort.abort(); + } + }), + ); + } + + shutdown(): Promise { + this.#closed = true; + this.#bodyAbort.abort(); + return this.#shutdown(); + } +} + +/** Owns the single connected server and the transport of its current session. */ +export class McpSessionSlot { + #live: LiveSession | undefined; + #arming: Promise | undefined; + #closed = false; + #hookRan = false; + #draining = false; + #ending: Promise | undefined; + /** The instance's own close hook, captured once; it runs exactly once, when the listener closes. */ + readonly #shutdownHook: () => void; + + constructor( + private readonly instance: RequestTrackingMcpServer, + private readonly logger: Logger | undefined, + private readonly drainTimeoutMs = DEFAULT_SESSION_DRAIN_TIMEOUT_MS, + ) { + instance.enableRequestTracking(); + const protocol = protocolOf(instance); + const captured = protocol.onclose ?? NO_SHUTDOWN_HOOK; + let running = false; + // Completion is recorded only after the hook returns, so a hook that threw stays eligible for a retry. + this.#shutdownHook = () => { + if (this.#hookRan || running) return; + running = true; + try { + captured(); + this.#hookRan = true; + } finally { + running = false; + } + }; + protocol.onclose = this.#shutdownHook; + } + + /** Route one MCP request; a new client's initialize replaces a session its predecessor left open. */ + async handle(request: Request): Promise { + try { + return await this.#handle(request); + } catch (cause: unknown) { + if (!(cause instanceof SessionDrainTimeoutError)) throw cause; + return jsonError(503, -32000, cause.message); + } + } + + async #handle(request: Request): Promise { + if (this.#closed) return SHUTTING_DOWN(); + if (this.#draining) await this.#live?.transport.close(); + if (this.#closed) return SHUTTING_DOWN(); + const open = this.#live; + if ( + open?.transport.sessionId !== undefined && + (await isInitializePost(request)) && + // A client that left while its initialize was probed would evict the session for nobody. + !request.signal.aborted + ) { + if (this.#closed) return SHUTTING_DOWN(); + this.logger?.info("MCP HTTP session replaced by a new client"); + await open.transport.close(); + } + if (this.#closed) return SHUTTING_DOWN(); + const session = await this.#current(); + // Shutdown may have closed the slot while this request was arming it. + if (this.#closed) return SHUTTING_DOWN(); + return session.handle(request); + } + + /** Cancel and drain MCP work before any shared provider shutdown hook can run. */ + async prepareShutdown(): Promise { + this.#closed = true; + await this.#arming?.catch(() => undefined); + const open = this.#live; + // The session stays owned until its shutdown succeeds, so a retry can finish it. + try { + await open?.transport.close(); + } catch (cause: unknown) { + // Shutdown may have joined a handover that already started its bounded + // drain. Retry that timeout with shutdown's completion-based wait. + if (!(cause instanceof SessionDrainTimeoutError)) throw cause; + await open?.transport.close(); + } + if (this.#live === open) this.#live = undefined; + } + + /** Finish shutdown once every consumer of the shared providers has settled. */ + async close(): Promise { + await this.prepareShutdown(); + // Without a live transport nothing fires the instance's hook; run it here, once. + this.#shutdownHook(); + } + + #current(): Promise { + if (this.#closed) + return Promise.reject(new Error("MCP HTTP session slot is closed")); + if (this.#live !== undefined) return Promise.resolve(this.#live); + this.#arming ??= this.#arm().finally(() => { + this.#arming = undefined; + }); + return this.#arming; + } + + async #arm(): Promise { + const transport = new WebStandardStreamableHTTPServerTransport({ + sessionIdGenerator: () => randomUUID(), + onsessioninitialized: () => this.logger?.info("MCP HTTP session opened"), + onsessionclosed: () => + this.logger?.info("MCP HTTP session closed by the client"), + }); + const session = new LiveSession(transport, this.instance); + // The SDK closes the transport on DELETE; that ends the session, not the server. + transport.close = () => this.#endSession(session); + await this.instance.connect(transport); + session.fenceDispatch(); + this.#live = session; + return session; + } + + /** Close one session's transport while the instance's onclose hooks wait for close(). */ + async #endSession(session: LiveSession): Promise { + if (this.#live !== session) return; + if (this.#ending !== undefined) return this.#ending; + this.#draining = true; + const ending = this.#finishSession(session).finally(() => { + if (this.#ending === ending) this.#ending = undefined; + }); + this.#ending = ending; + return ending; + } + + async #finishSession(session: LiveSession): Promise { + const protocol = protocolOf(this.instance); + protocol.onclose = NO_SHUTDOWN_HOOK; + try { + await session.shutdown(); + if ( + !(await this.instance.drainRequests( + this.#closed ? undefined : this.drainTimeoutMs, + )) + ) + throw new SessionDrainTimeoutError(this.drainTimeoutMs); + // A timed-out handler remains owned and blocks reuse until a later drain succeeds. + if (this.#live === session) this.#live = undefined; + this.#draining = false; + } finally { + protocol.onclose = this.#shutdownHook; + } + } +} + +const endpointUrl = (address: AddressInfo): string => { + const host = + address.family === "IPv6" ? `[${address.address}]` : address.address; + return `http://${host}:${String(address.port)}${MCP_HTTP_PATH}`; +}; + +interface NodeRequestTarget { + readonly route: (request: Request) => Promise<{ + readonly response: Response; + readonly admissionLifetime: HttpAdmissionLifetime; + }>; + readonly origin: () => string; + readonly logger: Logger | undefined; +} + +/** Serve one MCP server, plus plain routes, over streamable HTTP until closed. */ +export const serveStreamableHttp = async ( + options: ServeStreamableHttpOptions, +): Promise => { + const { host, port, logger, routes = [], closeInstance = true } = options; + // Scoped IPv6 literals bind but cannot be named in a URL; refuse them before anything exists. + if (host.includes("%")) + throw new StreamableHttpListenError( + new Error(`scoped IPv6 bind address ${host} is not supported`), + ); + // Loopback is classified from the address the listener binds, after it binds; the instance + // is created after that, so a bind failure leaves nothing to clean up. + let allowedHosts: string[] | undefined; + let slot: McpSessionSlot | undefined; + let origin = ""; + let stopped = false; + const route: NodeRequestTarget["route"] = async (request) => { + const local = (response: Response) => ({ + response, + admissionLifetime: "response" as const, + }); + if (stopped) return local(jsonError(503, -32000, "Server shutting down")); + // One loopback allowlist gates Host (DNS rebinding) and Origin (browser pages); a missing Origin passes. + const refused = + allowedHosts === undefined + ? undefined + : (hostHeaderValidationResponse(request, allowedHosts) ?? + originValidationResponse(request, allowedHosts)); + if (refused !== undefined) return local(refused); + if (slot === undefined) + return local(jsonError(503, -32000, "Server starting")); + const { pathname } = new URL(request.url); + if (pathname === MCP_HTTP_PATH) + return { + response: await slot.handle(request), + admissionLifetime: "dispatch", + }; + const match = routes.find( + (candidate) => + candidate.path === pathname && candidate.method === request.method, + ); + return local( + match === undefined + ? jsonError(404, -32601, `No route for ${request.method} ${pathname}`) + : await match.handle(request), + ); + }; + const httpServer = createHttpServer(); + const target: NodeRequestTarget = { route, origin: () => origin, logger }; + const requests = new NodeHttpRequestOwner( + options.maxConcurrentDispatches ?? DEFAULT_MAX_CONCURRENT_DISPATCHES, + { + serve: (request, response, context) => + serveNodeRequest(target, request, response, context), + refuse: (reason) => + jsonError( + 503, + -32000, + reason === "capacity" + ? "HTTP request capacity is full; retry after pending dispatches finish" + : "HTTP response cleanup failed; this server is not accepting new requests", + ), + }, + ); + const prepareTransportShutdown = stagedClose([ + async () => { + stopped = true; + await closeHttpServer(httpServer); + }, + () => slot?.prepareShutdown() ?? Promise.resolve(), + () => requests.drain(), + ]); + try { + httpServer.listen(port, host); + await once(httpServer, "listening"); + } catch (cause: unknown) { + throw new StreamableHttpListenError(cause); + } + const bound = httpServer.address(); + let url: string; + try { + if (bound === null || typeof bound === "string") + throw new TypeError("MCP HTTP listener has no TCP address"); + url = endpointUrl(bound); + origin = new URL(url).origin; + allowedHosts = hostHeaderAllowlist(host, bound.address); + } catch (cause: unknown) { + await prepareTransportShutdown().catch(() => undefined); + throw new StreamableHttpListenError(cause); + } + httpServer.on("request", (request, response) => { + void requests.handle(request, response).catch((cause: unknown) => { + logger?.warn( + { error: describe(cause) }, + "MCP HTTP request did not complete", + ); + response.destroy(); + }); + }); + // Bind failures already rejected the listening wait above; only later listener errors are reported. + httpServer.on("error", (error) => options.onerror?.(error)); + let instance: RequestTrackingMcpServer; + try { + instance = await options.createMcpServer({ era: "legacy" }); + } catch (cause: unknown) { + // Starting responses may still be in flight; application routes stay unavailable. + await prepareTransportShutdown().catch(() => undefined); + throw cause; + } + protocolOf(instance).onerror = (error) => + logger?.warn({ error: error.message }, "MCP HTTP request rejected"); + const live = new McpSessionSlot( + instance, + logger, + options.sessionDrainTimeoutMs, + ); + slot = live; + if (allowedHosts === undefined) + logger?.warn( + { host }, + "MCP HTTP Host header validation relaxed for a non-loopback bind", + ); + // The URL is the only way to reach an ephemeral port, so the announcement ignores the log threshold. + logger + ?.child({}, { level: "info" }) + .info({ transport: "streamable-http", url }, "MCP transport listening"); + const prepareShutdown = stagedClose([ + prepareTransportShutdown, + () => live.close(), + ]); + const close = stagedClose([ + prepareShutdown, + ...(closeInstance ? [() => instance.close()] : []), + ]); + return { url, prepareShutdown, close }; +}; + +/** Stop accepting, drop open connections and release the port. */ +const closeHttpServer = ( + httpServer: ReturnType, +): Promise => { + // close() first, as Node documents: a connection accepted before it would otherwise miss the forced-close pass. + const closed = new Promise((resolve, reject) => { + httpServer.close((error) => + error === undefined ? resolve() : reject(error), + ); + }); + httpServer.closeAllConnections(); + return closed; +}; + +/** Translate one node:http exchange through `route`, logging a failure and answering it when headers are still open. */ +const serveNodeRequest = async ( + target: NodeRequestTarget, + request: IncomingMessage, + response: ServerResponse, + context: NodeHttpExchangeContext, +): Promise => { + const { route, origin, logger } = target; + // A reply sent before an incoming body was fully consumed leaves the rest on the socket, which + // the bridged stream keeps paused; closing the connection after the reply releases it. The + // incoming message decides (end of stream, not "body disturbed"), so a partially read upload + // and a GET or HEAD that carried a body close too. + const declaresBody = + request.headers["content-length"] !== undefined || + request.headers["transfer-encoding"] !== undefined; + let webRequest: Request | undefined; + const unreadBody = (): boolean => declaresBody && !request.readableEnded; + let reported = false; + let admissionLifetime: HttpAdmissionLifetime = "response"; + const settled = (): void => { + if (reported) return; + reported = true; + context.onDispatchSettled({ + hasUnreadBody: unreadBody(), + admissionLifetime, + }); + }; + try { + try { + webRequest = toWebRequest(request, response, origin()); + } catch (cause: unknown) { + // Fetch refuses methods such as TRACE that node:http accepts, and the bridge refuses + // targets that are not origin-form paths; both are answered instead of thrown. + const target = cause instanceof UnsupportedRequestTargetError; + await writeWebResponse( + jsonError( + target ? 400 : 405, + -32600, + target + ? "Unsupported request target" + : `Unsupported method ${request.method ?? ""}`, + ), + response, + { closeConnection: true, onBodyCleanup: context.onBodyCleanup }, + ); + return; + } + let webResponse: Response; + try { + const routed = await route(webRequest); + webResponse = routed.response; + admissionLifetime = routed.admissionLifetime; + } finally { + settled(); + } + await writeWebResponse(webResponse, response, { + closeConnection: unreadBody(), + onBodyCleanup: context.onBodyCleanup, + }); + } catch (cause: unknown) { + // A client that left mid-stream is routine; a failure before headers is not. + logger?.[response.headersSent ? "debug" : "warn"]( + { error: describe(cause) }, + "MCP HTTP request did not complete", + ); + if (response.headersSent) response.destroy(); + else + await writeWebResponse( + jsonError(500, -32603, "Internal server error"), + response, + { closeConnection: unreadBody(), onBodyCleanup: context.onBodyCleanup }, + ).catch(() => undefined); + } finally { + settled(); + } +}; + +/** Run close stages in order once each; a failed stage is retried by the next call instead of replaying its rejection. */ +const stagedClose = ( + stages: readonly (() => Promise)[], +): (() => Promise) => { + const done = stages.map(() => false); + let closing: Promise | undefined; + return () => { + if (closing !== undefined) return closing; + const attempt = (async () => { + for (const [index, stage] of stages.entries()) { + if (done[index]) continue; + await stage(); + done[index] = true; + } + })(); + closing = attempt; + void attempt.catch(() => { + if (closing === attempt) closing = undefined; + }); + return attempt; + }; +}; diff --git a/src/server/http/streamableHttpListenError.ts b/src/server/http/streamableHttpListenError.ts new file mode 100644 index 0000000000..7f536f9207 --- /dev/null +++ b/src/server/http/streamableHttpListenError.ts @@ -0,0 +1,10 @@ +/** The listener could not bind before any client connected; the message names the system cause. */ +export class StreamableHttpListenError extends Error { + override readonly name = "StreamableHttpListenError"; + constructor(cause: unknown) { + super( + `MCP HTTP listener could not bind: ${cause instanceof Error ? cause.message : String(cause)}`, + { cause }, + ); + } +} diff --git a/src/server/registerPrompts.ts b/src/server/registerPrompts.ts index e0bd00768c..a20713452b 100644 --- a/src/server/registerPrompts.ts +++ b/src/server/registerPrompts.ts @@ -1,4 +1,7 @@ -import type { EvidenceMcpServer } from "./EvidenceMcpServer.js"; +import { + currentMcpRequestSignal, + type EvidenceMcpServer, +} from "./EvidenceMcpServer.js"; import { completable, type RegisteredPrompt, @@ -31,7 +34,16 @@ export const registerGuidedPrompts = ( analysis: AnalysisOperationPort, session?: BinarySessionPort, ): GuidedPromptRegistry => { - const completion = createPromptCompletionSource(analysis, session); + const source = createPromptCompletionSource(analysis, session); + const completion: PromptCompletionSource = { + async complete(kind, value, context) { + // Completions can query the same providers as tools. Cancellation alone + // does not mean their provider cleanup has finished. + return server.runRequest(currentMcpRequestSignal(), () => + source.complete(kind, value, context), + ); + }, + }; const prompts = new Map(); for (const contract of PROMPT_CONTRACTS) prompts.set(contract.name, registerPrompt(server, contract, completion)); diff --git a/src/server/shutdownRegistration.test.ts b/src/server/shutdownRegistration.test.ts index 70da2d2dcc..fdb18a1e8c 100644 --- a/src/server/shutdownRegistration.test.ts +++ b/src/server/shutdownRegistration.test.ts @@ -3,11 +3,57 @@ import { expect, it } from "vitest"; import pino from "pino"; import { createShutdown } from "../main/shutdown.js"; -import { registerProcessShutdown } from "../main/processShutdown.js"; -import type { RuntimeDependencies } from "../main/types.js"; -import type { StdioServerHandle } from "@modelcontextprotocol/server/stdio"; +import { + createExitStatus, + latchSignalShutdown, + registerProcessShutdown, + registerSignalShutdown, +} from "../main/processShutdown.js"; +import type { McpServerHandle, RuntimeDependencies } from "../main/types.js"; import { createManagedBinarySession } from "../composition/binary.js"; +it("registers only signals for a listener and reports the signal exit status", () => { + const signals = new EventEmitter(); + const statuses: number[] = []; + const unregister = registerSignalShutdown( + (status) => statuses.push(status), + signals, + ); + signals.emit("SIGTERM"); + signals.emit("SIGINT"); + expect(statuses).toEqual([143, 130]); + unregister(); + signals.emit("SIGTERM"); + expect(statuses).toEqual([143, 130]); + expect(signals.listenerCount("SIGTERM")).toBe(0); +}); + +it("names the signal that fired, passes none for stdin, and ignores stdin when the events omit it", () => { + const signals = new EventEmitter(); + const input = new EventEmitter(); + const triggers: (string | undefined)[] = []; + const withInput = registerProcessShutdown((signal) => triggers.push(signal), { + signals, + stdin: input, + }); + signals.emit("SIGINT"); + input.emit("end"); + expect(triggers).toEqual(["SIGINT", undefined]); + withInput(); + const signalsOnly = registerProcessShutdown( + (signal) => triggers.push(signal), + { signals }, + ); + signals.emit("SIGTERM"); + input.emit("end"); + input.emit("close"); + expect(triggers).toEqual(["SIGINT", undefined, "SIGTERM"]); + signalsOnly(); + signals.emit("SIGINT"); + expect(triggers).toEqual(["SIGINT", undefined, "SIGTERM"]); + expect(input.listenerCount("end")).toBe(0); +}); + const shutdownEvents = ["SIGINT", "SIGTERM", "end", "close"] as const; const flush = async (): Promise => { @@ -51,7 +97,7 @@ it.each(shutdownEvents)( }; }, }; - const handle: StdioServerHandle = { + const handle: McpServerHandle = { close: async () => { transportCloseCalls += 1; if (transportCloseCalls === 1) @@ -110,6 +156,55 @@ it.each(shutdownEvents)( }, ); +it("keeps shared provider owners open when the transport preparation barrier fails", async () => { + const events: string[] = []; + let attempts = 0; + const session = createManagedBinarySession(); + const closeSession = session.close.bind(session); + session.close = async (...args) => { + events.push("session"); + return closeSession(...args); + }; + const lifecycle = createShutdown({ + handle: { + prepareShutdown: async () => { + events.push("prepare"); + attempts += 1; + if (attempts === 1) throw new Error("tool cleanup is still running"); + }, + close: async () => { + events.push("transport"); + }, + }, + closeOptionalProviders: async () => { + events.push("provider"); + }, + session, + dependencies: { + env: {}, + serve: () => { + throw new Error("unused transport"); + }, + writeStderr: () => undefined, + setExitCode: () => undefined, + registerShutdown: () => () => undefined, + }, + serverLogger: pino({ enabled: false }), + }); + await expect(lifecycle.shutdown()).rejects.toThrow( + "tool cleanup is still running", + ); + expect(events).toEqual(["prepare"]); + await lifecycle.shutdown(); + expect(events).toEqual([ + "prepare", + "prepare", + "transport", + "provider", + "session", + ]); +}); + it("joins concurrent shutdown events and retries only failed owners", async () => { const signals = new EventEmitter(); const input = new EventEmitter(); @@ -134,7 +229,7 @@ it("joins concurrent shutdown events and retries only failed owners", async () = registerShutdown: (handler) => registerProcessShutdown(handler, { signals, stdin: input }), }; - const handle: StdioServerHandle = { + const handle: McpServerHandle = { close: async () => { transportCloseCalls += 1; if (transportCloseCalls === 1) @@ -176,3 +271,33 @@ it("joins concurrent shutdown events and retries only failed owners", async () = expect(input.listenerCount("end")).toBe(0); expect(input.listenerCount("close")).toBe(0); }); + +it("latches the first startup signal, exits at once on a second, and hands the status over on release", () => { + const signals = new EventEmitter(); + const exits: number[] = []; + const latch = latchSignalShutdown(signals, (status) => exits.push(status)); + expect(signals.listenerCount("SIGINT")).toBe(1); + signals.emit("SIGTERM"); + expect(exits).toEqual([]); + signals.emit("SIGINT"); + expect(exits).toEqual([143]); + expect(latch.release()).toBe(143); + expect(signals.listenerCount("SIGINT")).toBe(0); + expect(signals.listenerCount("SIGTERM")).toBe(0); + const quiet = latchSignalShutdown(signals, (status) => exits.push(status)); + expect(quiet.release()).toBeUndefined(); +}); + +it("keeps a signal's exit status through the entry's normal return and yields to a real failure", () => { + const codes: number[] = []; + const exit = createExitStatus((code) => codes.push(code)); + exit.set(0); + exit.signal(143); + exit.set(0); + expect(codes).toEqual([0, 143, 143]); + exit.set(1); + expect(codes.at(-1)).toBe(1); + const plain = createExitStatus((code) => codes.push(code)); + plain.set(0); + expect(codes.at(-1)).toBe(0); +}); diff --git a/tests/boundary/cli/cliStartupImports.test.ts b/tests/boundary/cli/cliStartupImports.test.ts index cc1232606f..ce500215d4 100644 --- a/tests/boundary/cli/cliStartupImports.test.ts +++ b/tests/boundary/cli/cliStartupImports.test.ts @@ -235,12 +235,16 @@ describe("CLI startup imports", () => { }); describe("MCP startup imports", () => { - it("does not load command-specific dependencies to initialize and list tools", async () => { + it("initializes and lists tools over stdio with HTTP-only environment settings present", async () => { const transport = new StdioClientTransport({ command: process.execPath, args: [`--import=${RESOLUTION_HOOK}`, "scripts/rea.mjs", "mcp"], cwd: process.cwd(), - env: { PATH: process.env.PATH ?? "" }, + env: { + PATH: process.env.PATH ?? "", + MCP_HOST: " ", + MCP_PORT: "not-a-port", + }, stderr: "pipe", }); let stderr = ""; @@ -258,6 +262,7 @@ describe("MCP startup imports", () => { await client.close(); } const resolved = resolvedMarkers(stderr); + expect(stderr).not.toContain("MCP transport listening"); expect(resolved).not.toContain("playwright-core"); expect(resolved).not.toContain("isomorphic-git"); }); diff --git a/tests/boundary/cli/dispatcher.test.ts b/tests/boundary/cli/dispatcher.test.ts index 516d33b68f..316a2cff21 100644 --- a/tests/boundary/cli/dispatcher.test.ts +++ b/tests/boundary/cli/dispatcher.test.ts @@ -56,7 +56,7 @@ describe("executable dispatcher", () => { ), workspace.write( "dist/mcp/main.js", - 'export const runEntrypoint = () => process.stdout.write(JSON.stringify({ adapter: "mcp" }));\n', + 'export const runEntrypoint = () => process.stdout.write(JSON.stringify({ adapter: "mcp" })); export const runMcpHttpEntrypoint = (args) => process.stdout.write(JSON.stringify({ adapter: "mcp-http", args }));\n', ), workspace.write( "dist/cli-runtime/cli.js", @@ -100,6 +100,24 @@ describe("executable dispatcher", () => { adapter: "cli", args: ["--mcp", "extra"], }); + for (const args of [ + ["mcp", "--http", "--port", "0"], + ["--mcp", "--http"], + ["mcp", "-h", "--http"], + ["mcp", "--host", "127.0.0.1", "--http"], + ]) + await expect(invoke(args)).resolves.toEqual({ + adapter: "mcp-http", + args: args.slice(1), + }); + await expect(invoke(["mcp", "doctor", "--http"])).resolves.toEqual({ + adapter: "mcp-doctor", + args: ["--http"], + }); + await expect(invoke(["mcp", "add", "--http"])).resolves.toEqual({ + adapter: "cli", + args: ["mcp", "add", "--http"], + }); }, ); diff --git a/tests/boundary/mcp/mainErrors.test.ts b/tests/boundary/mcp/mainErrors.test.ts index c3738e5e2d..f20512653f 100644 --- a/tests/boundary/mcp/mainErrors.test.ts +++ b/tests/boundary/mcp/mainErrors.test.ts @@ -1,10 +1,12 @@ -import type { ServeStdioOptions } from "@modelcontextprotocol/server/stdio"; import { spawnSync } from "node:child_process"; import { resolve } from "node:path"; import { pathToFileURL } from "node:url"; import { describe, expect, it } from "vitest"; import { run, runEntrypoint } from "../../../src/main.js"; +import { MCP_HTTP_LISTEN_FAILED } from "../../../src/main/messages.js"; +import type { McpServeOptions } from "../../../src/main/types.js"; +import { StreamableHttpListenError } from "../../../src/server/http/streamableHttpListenError.js"; type RuntimeDependencies = NonNullable[0]>; @@ -61,7 +63,7 @@ describe("MCP runtime errors", () => { const output: string[] = []; const shutdown: Array<() => void> = []; const shutdownUnregistrations: string[] = []; - let options: ServeStdioOptions | undefined; + let options: McpServeOptions | undefined; let closeCalls = 0; const close = (): Promise => { closeCalls += 1; @@ -176,3 +178,48 @@ describe("MCP runtime errors", () => { expect(exitCodes).toEqual([1]); }); }); + +describe("MCP runtime HTTP transport errors", () => { + it("awaits an asynchronous transport start before installing shutdown", async () => { + const shutdown: Array<() => void> = []; + let closeCalls = 0; + expect( + await run( + dependencies({ + serve: async () => ({ + close: async () => { + closeCalls += 1; + }, + }), + output: [], + shutdown, + exitCodes: [], + }), + ), + ).toBe(0); + expect(shutdown).toHaveLength(1); + shutdown[0]?.(); + await nextTurn(); + expect(closeCalls).toBe(1); + }); + + it("reports an HTTP listener that cannot bind without its cause", async () => { + const output: string[] = []; + expect( + await run( + dependencies({ + serve: () => + Promise.reject( + new StreamableHttpListenError( + new Error("SECRET EADDRINUSE 127.0.0.1:1"), + ), + ), + output, + shutdown: [], + exitCodes: [], + }), + ), + ).toBe(1); + expect(output).toEqual([`${MCP_HTTP_LISTEN_FAILED}\n`]); + }); +}); diff --git a/tests/boundary/mcp/mcpHttpOptions.test.ts b/tests/boundary/mcp/mcpHttpOptions.test.ts new file mode 100644 index 0000000000..243ef5682d --- /dev/null +++ b/tests/boundary/mcp/mcpHttpOptions.test.ts @@ -0,0 +1,118 @@ +import { describe, expect, it } from "vitest"; + +import { parseMcpHttpEndpoint } from "../../../src/config/mcpHttpEndpoint.js"; +import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; +import { runMcpHttpEntrypoint } from "../../../src/main.js"; +import { + parseMcpHttpArguments, + renderMcpHttpHelp, +} from "../../../src/main/mcpHttpArguments.js"; + +describe("rea mcp --http arguments", () => { + it("accepts --http with optional host and port in either flag spelling", () => { + expect(parseMcpHttpArguments(["--http"])).toEqual({ ok: true }); + expect( + parseMcpHttpArguments(["--http", "--host", "::1", "--port=8080"]), + ).toEqual({ ok: true, host: "::1", port: 8080 }); + expect(parseMcpHttpArguments(["--port", "0", "--http"])).toEqual({ + ok: true, + port: 0, + }); + }); + + it("answers help only in flag position and names the rejected argument", () => { + expect(parseMcpHttpArguments(["--http", "-h"])).toEqual({ + ok: false, + help: true, + }); + expect(parseMcpHttpArguments(["--http", "--host", "--help"])).toEqual({ + ok: false, + help: false, + message: "Missing value for --host", + }); + expect(parseMcpHttpArguments(["--http", "add"])).toMatchObject({ + ok: false, + message: "Unknown mcp --http argument: add", + }); + expect(parseMcpHttpArguments(["--port", "1"])).toMatchObject({ + ok: false, + message: "--host and --port require --http", + }); + for (const port of ["70000", "08", "-1", "http"]) + expect(parseMcpHttpArguments(["--http", `--port=${port}`])).toMatchObject( + { ok: false, message: expect.stringContaining("Invalid --port") }, + ); + expect(parseMcpHttpArguments(["--http", "--host", " "])).toMatchObject({ + ok: false, + message: expect.stringContaining("--host must be"), + }); + }); + + it("renders help from the option schema without Incur global options", () => { + const help = renderMcpHttpHelp(); + expect(help).toContain("rea mcp --http"); + expect(help).toContain("--host"); + expect(help).toContain("--port"); + expect(help).toContain("MCP_PORT"); + expect(help).not.toContain("--filter-output"); + }); + + it("writes help as one newline-terminated stdout block and rejections to stderr", async () => { + const io = () => { + const stdout: string[] = []; + const stderr: string[] = []; + const exitCodes: number[] = []; + return { + stdout, + stderr, + exitCodes, + writeStdout: (text: string) => void stdout.push(text), + writeStderr: (text: string) => void stderr.push(text), + setExitCode: (code: number) => void exitCodes.push(code), + }; + }; + const help = io(); + await runMcpHttpEntrypoint(["--http", "--help"], help); + expect(help.stdout).toEqual([`${renderMcpHttpHelp()}\n`]); + expect(help.stderr).toEqual([]); + expect(help.exitCodes).toEqual([0]); + const rejected = io(); + await runMcpHttpEntrypoint(["--http", "add"], rejected); + expect(rejected.stdout).toEqual([]); + expect(rejected.stderr).toEqual(["Unknown mcp --http argument: add\n"]); + expect(rejected.exitCodes).toEqual([1]); + }); +}); + +describe("MCP_HOST and MCP_PORT", () => { + it("defaults to loopback and an ephemeral port", () => { + expect(parseMcpHttpEndpoint({})).toEqual({ + ok: true, + value: { host: "127.0.0.1", port: 0 }, + }); + }); + + it("reads the environment snapshot and lets explicit values win", () => { + expect( + parseMcpHttpEndpoint({ MCP_HOST: "192.0.2.10", MCP_PORT: "8080" }), + ).toEqual({ ok: true, value: { host: "192.0.2.10", port: 8080 } }); + expect( + parseMcpHttpEndpoint( + { MCP_HOST: " ", MCP_PORT: "nope" }, + { host: "::1", port: 1 }, + ), + ).toEqual({ ok: true, value: { host: "::1", port: 1 } }); + }); + + it("rejects malformed values by setting name without echoing them", () => { + const result = parseMcpHttpEndpoint({ MCP_HOST: " ", MCP_PORT: "65536" }); + if (result.ok) throw new Error("Expected a configuration error"); + expect(result.error.settings.map(({ setting }) => setting)).toEqual([ + "MCP_HOST", + "MCP_PORT", + ]); + const projected = JSON.stringify(projectAnalysisError(result.error)); + expect(projected).toContain("MCP_PORT must be a decimal integer"); + expect(projected).not.toContain("65536"); + }); +}); diff --git a/tests/boundary/mcp/promptMcp.test.ts b/tests/boundary/mcp/promptMcp.test.ts index 9138a102dc..6ff84a3ed2 100644 --- a/tests/boundary/mcp/promptMcp.test.ts +++ b/tests/boundary/mcp/promptMcp.test.ts @@ -2,12 +2,16 @@ import { ok as resultOk } from "../../../src/domain/result.js"; import { readFile, rm, writeFile } from "node:fs/promises"; import { basename, join } from "node:path"; -import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; +import { + Client, + InMemoryTransport, + StreamableHTTPClientTransport, +} from "@modelcontextprotocol/client"; import { STDIO_DEFAULT_MAX_BUFFER_SIZE, type McpServer, } from "@modelcontextprotocol/server"; -import { afterEach, describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; @@ -39,6 +43,7 @@ import { registerGuidedPrompts } from "../../../src/server/registerPrompts.js"; import { EvidenceMcpServer } from "../../../src/server/EvidenceMcpServer.js"; import { ToolResultDelivery } from "../../../src/server/toolResult.js"; import { observed } from "../../fixtures/analysisExecution.js"; +import { serveStreamableHttp } from "../../../src/server/http/serveStreamableHttp.js"; const resources: Array<{ close(): Promise }> = []; const temporaryDirectories: string[] = []; @@ -380,6 +385,97 @@ it("guided prompt completion avoids cold startup and cancels warm lookup on clos expect((await pending).completion.values).toEqual([]); }, 10_000); +it.each(["replacement", "shutdown"])( + "waits for cancelled completion cleanup before HTTP %s", + async (ending) => { + let release!: () => void; + let entered!: () => void; + let cancelled!: () => void; + const gate = new Promise((resolve) => { + release = resolve; + }); + const started = new Promise((resolve) => { + entered = resolve; + }); + const aborted = new Promise((resolve) => { + cancelled = resolve; + }); + const server = new EvidenceMcpServer( + { name: "http-completion-drain", version: "1" }, + {}, + undefined, + new ToolResultDelivery(STDIO_DEFAULT_MAX_BUFFER_SIZE), + ); + const connections = vi.spyOn(server, "connect"); + let calls = 0; + registerGuidedPrompts(server, { + async execute(_operation, _parameters, options) { + if (++calls > 1) return observed(["fresh-document"]); + entered(); + try { + await new Promise((resolve) => { + options?.signal?.addEventListener( + "abort", + () => { + cancelled(); + resolve(); + }, + { once: true }, + ); + }); + } finally { + await gate; + } + return observed(["old-document"]); + }, + }); + const served = await serveStreamableHttp({ + host: "127.0.0.1", + port: 0, + createMcpServer: () => server, + }); + resources.push(served); + const connectHttp = async () => { + const client = new Client({ name: "completion-http", version: "1" }); + resources.push(client); + await client.connect( + new StreamableHTTPClientTransport(new URL(served.url)), + ); + return client; + }; + const first = await connectHttp(); + let next: Client | undefined; + let finishing: Promise | undefined; + try { + void complete(first, "document", "").catch(() => undefined); + await started; + let finished = false; + finishing = ( + ending === "shutdown" + ? served.close() + : connectHttp().then((value) => { + next = value; + }) + ).then(() => { + finished = true; + }); + await aborted; + await new Promise((resolve) => setImmediate(resolve)); + expect(connections).toHaveBeenCalledTimes(1); + expect(finished).toBe(false); + release(); + await finishing; + if (next !== undefined) + expect(await complete(next, "document", "fresh")).toEqual([ + "fresh-document", + ]); + } finally { + release(); + await finishing?.catch(() => undefined); + } + }, +); + describe("guided prompt completion lifecycle", () => { it("survives a cancelled completion and ignores its late result", async () => { let calls = 0; diff --git a/tests/boundary/mcp/streamableHttpMcp.test.ts b/tests/boundary/mcp/streamableHttpMcp.test.ts new file mode 100644 index 0000000000..64560b20d2 --- /dev/null +++ b/tests/boundary/mcp/streamableHttpMcp.test.ts @@ -0,0 +1,85 @@ +import { + Client, + StreamableHTTPClientTransport, +} from "@modelcontextprotocol/client"; +import { describe, expect } from "vitest"; + +import { expectForeignHostRefused } from "../../support/http/rawRequest.js"; +import { startMcpHttpProcess } from "../../support/mcp/streamableHttpProcess.js"; +import { processTest } from "../../support/process/processFixture.js"; +import { assertAdvertisedMcpContracts } from "./mcpContractHarness.js"; + +const HTTP_BOUNDARY_TIMEOUT_MS = 90_000; + +const openSession = async (url: URL, name: string) => { + const transport = new StreamableHTTPClientTransport(url); + const client = new Client({ name, version: "1" }); + await client.connect(transport); + expect(transport.sessionId).toEqual(expect.any(String)); + return { transport, client }; +}; + +/** The shared target answers with its identity only while its providers are still open. */ +const expectIdentity = async (client: Client): Promise => { + const identity = await client.callTool({ + name: "binary_session", + arguments: {}, + }); + expect(identity.isError).toBeFalsy(); + expect(identity.structuredContent).toMatchObject({ + result: { + server_identity: { package: { version: expect.any(String) } }, + }, + }); +}; + +describe("rea mcp --http", () => { + processTest( + "serves initialize, tools/list, a tool call and DELETE, then keeps serving later clients", + async ({ processes }) => { + const started = await startMcpHttpProcess(processes); + expect(started.url.hostname).toBe("127.0.0.1"); + expect(started.url.pathname).toBe("/mcp"); + let stdout = ""; + started.child.stdout.on("data", (chunk: Buffer) => { + stdout += chunk.toString("utf8"); + }); + const first = await openSession(started.url, "streamable-http-boundary"); + const { tools } = await first.client.listTools(); + assertAdvertisedMcpContracts(["binary_session"], tools); + await expectIdentity(first.client); + await first.transport.terminateSession(); + expect(first.transport.sessionId).toBeUndefined(); + await first.client.close(); + // DELETE ended the session, not the server: the next client's tools still work. + const second = await openSession(started.url, "next-session"); + await expectIdentity(second.client); + // A client that leaves without DELETE does not block the listener: the next initialize replaces it. + const third = await openSession(started.url, "replacing-session"); + expect(third.transport.sessionId).not.toBe(second.transport.sessionId); + await expectIdentity(third.client); + await expect( + second.client.callTool({ name: "binary_session", arguments: {} }), + ).rejects.toThrow(); + await second.client.close(); + await third.transport.terminateSession(); + await third.client.close(); + expect(stdout).toBe(""); + expect(started.stderr()).toContain('"transport":"streamable-http"'); + expect(started.stderr()).toContain( + "MCP HTTP session replaced by a new client", + ); + }, + HTTP_BOUNDARY_TIMEOUT_MS, + ); + + processTest( + "refuses a non-loopback Host header on the loopback bind", + async ({ processes }) => { + const { url } = await startMcpHttpProcess(processes); + const refused = await expectForeignHostRefused(url); + expect(refused.body).not.toContain("127.0.0.1"); + }, + HTTP_BOUNDARY_TIMEOUT_MS, + ); +}); diff --git a/tests/boundary/process/mcpHttpShutdown.test.ts b/tests/boundary/process/mcpHttpShutdown.test.ts new file mode 100644 index 0000000000..d9f1b62fff --- /dev/null +++ b/tests/boundary/process/mcpHttpShutdown.test.ts @@ -0,0 +1,54 @@ +import { once } from "node:events"; +import { describe, expect } from "vitest"; + +import { MCP_SHUTDOWN_FAILED } from "../../../src/main/messages.js"; +import { rawHttpRequest } from "../../support/http/rawRequest.js"; +import { startMcpHttpProcess } from "../../support/mcp/streamableHttpProcess.js"; +import { processTest } from "../../support/process/processFixture.js"; + +const SHUTDOWN_TIMEOUT_MS = 90_000; + +const shutdownSignals: readonly (readonly [NodeJS.Signals, number])[] = + process.platform === "win32" + ? [["SIGINT", 130]] + : [ + ["SIGTERM", 143], + ["SIGINT", 130], + ]; + +describe("rea mcp --http shutdown", () => { + for (const [signal, status] of shutdownSignals) + processTest( + `releases the listener and exits ${String(status)} on ${signal}`, + async ({ processes }) => { + const started = await startMcpHttpProcess(processes); + // The live transport refuses a GET that does not accept an event stream. + const before = await rawHttpRequest(started.url); + expect(before.status).toBe(406); + started.child.kill(signal); + const [code, signalCode] = await once(started.child, "close"); + expect(code).toBe(status); + expect(signalCode).toBeNull(); + expect(started.stderr()).not.toContain(MCP_SHUTDOWN_FAILED); + await expect(rawHttpRequest(started.url)).rejects.toMatchObject({ + code: "ECONNREFUSED", + }); + }, + SHUTDOWN_TIMEOUT_MS, + ); + + processTest( + "exits with the signal status when the signal lands as startup completes", + async ({ processes }) => { + const started = await startMcpHttpProcess(processes); + // No request first: the signal arrives at the edge between startup and the lifecycle owner. + const [signal, status] = shutdownSignals[0]!; + started.child.kill(signal); + const [code, signalCode] = await once(started.child, "close"); + expect(code).toBe(status); + expect(signalCode).toBeNull(); + expect(started.stderr()).not.toContain(MCP_SHUTDOWN_FAILED); + }, + SHUTDOWN_TIMEOUT_MS, + ); +}); diff --git a/tests/support/http/rawRequest.ts b/tests/support/http/rawRequest.ts new file mode 100644 index 0000000000..8f87c26547 --- /dev/null +++ b/tests/support/http/rawRequest.ts @@ -0,0 +1,69 @@ +import { request as httpRequest, type IncomingHttpHeaders } from "node:http"; + +import { expect } from "vitest"; + +/** One buffered HTTP response. */ +export interface RawHttpResponse { + readonly status: number; + readonly headers: IncomingHttpHeaders; + readonly body: string; +} + +/** Send one request with caller-controlled headers, including Host, which fetch overwrites. */ +export const rawHttpRequest = ( + url: URL, + options: { + readonly method?: string; + readonly headers?: Readonly>; + readonly body?: string; + } = {}, +): Promise => + new Promise((resolve, reject) => { + // A fresh connection per request keeps a closed listener observable as ECONNREFUSED. + const request = httpRequest( + url, + { + method: options.method ?? "GET", + headers: { ...options.headers }, + agent: false, + }, + (response) => { + let body = ""; + response.setEncoding("utf8"); + response.on("data", (chunk: string) => { + body += chunk; + }); + response.on("end", () => + resolve({ + status: response.statusCode ?? 0, + headers: response.headers, + body, + }), + ); + response.on("error", reject); + }, + ); + request.on("error", reject); + request.end(options.body); + }); + +/** Assert the MCP endpoint refuses a Host header outside the loopback allowlist. */ +export const expectForeignHostRefused = async ( + url: URL, +): Promise => { + const refused = await rawHttpRequest(url, { + method: "POST", + headers: { + host: "rea.invalid", + "content-type": "application/json", + accept: "application/json, text/event-stream", + }, + body: "{}", + }); + expect(refused.status).toBe(403); + expect(JSON.parse(refused.body)).toMatchObject({ + jsonrpc: "2.0", + error: { code: -32000 }, + }); + return refused; +}; diff --git a/tests/support/mcp/streamableHttpProcess.ts b/tests/support/mcp/streamableHttpProcess.ts new file mode 100644 index 0000000000..35d8ff8945 --- /dev/null +++ b/tests/support/mcp/streamableHttpProcess.ts @@ -0,0 +1,91 @@ +import type { ChildProcessWithoutNullStreams } from "node:child_process"; +import { resolve as resolvePath } from "node:path"; + +import type { TestProcesses } from "../process/processFixture.js"; + +const LISTENING_MESSAGE = "MCP transport listening"; +const STARTUP_TIMEOUT_MS = 30_000; + +/** A compiled `rea mcp --http` child and the URL it announced. */ +export interface McpHttpProcess { + readonly child: ChildProcessWithoutNullStreams; + readonly url: URL; + /** Everything the child has written to stderr so far. */ + readonly stderr: () => string; +} + +/** Find the bound URL in the pino JSON lines the listener writes to stderr. */ +const listeningUrl = (stderr: string): URL | undefined => { + for (const line of stderr.split("\n")) { + let entry: unknown; + try { + entry = JSON.parse(line); + } catch { + continue; + } + if ( + typeof entry === "object" && + entry !== null && + "msg" in entry && + entry.msg === LISTENING_MESSAGE && + "url" in entry && + typeof entry.url === "string" + ) + return new URL(entry.url); + } + return undefined; +}; + +/** Spawn `rea mcp --http --port 0` with stdin closed and resolve once it names its URL. */ +export const startMcpHttpProcess = async ( + processes: TestProcesses, + arguments_: readonly string[] = [], +): Promise => { + const child = processes.spawn( + process.execPath, + [ + resolvePath("scripts/rea.mjs"), + "mcp", + "--http", + "--port", + "0", + ...arguments_, + ], + { + env: { + PATH: process.env.PATH ?? "", + HOPPER_LAUNCHER_PATH: "/rea-unconfigured-deep-provider/hopper", + }, + }, + ); + // A closed stdin must not end the HTTP listener; only signals do. + child.stdin.end(); + let stderr = ""; + const url = await new Promise((resolve, reject) => { + const timer = setTimeout( + () => + reject( + new Error( + `No listening line within ${String(STARTUP_TIMEOUT_MS)} ms:\n${stderr}`, + ), + ), + STARTUP_TIMEOUT_MS, + ); + child.stderr.on("data", (chunk: Buffer) => { + stderr += chunk.toString("utf8"); + const found = listeningUrl(stderr); + if (found === undefined) return; + clearTimeout(timer); + resolve(found); + }); + child.once("exit", (code, signal) => { + clearTimeout(timer); + reject( + new Error( + `Listener exited early (${String(code)}, ${String(signal)}):\n${stderr}`, + ), + ); + }); + }); + return { child, url, stderr: () => stderr }; +};