From 60bd646478dcfab6c296b7e4765254eea787ffa9 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Mon, 19 Oct 2026 08:58:51 -0700 Subject: [PATCH 1/4] feat(mcp): add opt-in streamable HTTP transport (#4955) Add an explicit Streamable HTTP entry for environment/gateway hosts while keeping stdio registrations and default behavior. Preserve the shared provider/session owner across DELETE and replacement; fence and drain tool/completion work before handover or teardown. Bound HTTP input and plain-response admission, cancel refused bodies, join whole exchanges and asynchronous response cleanup before provider hooks, gate startup readiness, and reject queued HTTP/1.1 pipelining while retaining sequential keep-alive. Validation: 8 static/build tasks, 83 adapter tests and registry metadata pass. A real installed gh binary produces seven equivalent CLI/stdio/HTTP reports with DELETE/replacement and clean signal shutdown. Broader local native-provider/ownership failures reproduce on clean main and are documented with limitations in the PR. Co-authored-by: Edgar Arakelyan Co-authored-by: gensokyo-dev <342580529+gensokyo-dev@users.noreply.github.com> --- docs/architecture.mermaid | 6 +- docs/cli.md | 26 + docs/mcp-contracts.md | 93 +- .../agent-environments/http-transport/plan.md | 32 + .../agent-environments/http-transport/spec.md | 79 + .../http-transport/tasks.md | 11 + .../http-transport/verification.md | 297 +++ scripts/rea.mjs | 35 +- src/config/mcpHttpEndpoint.ts | 68 + src/main.ts | 120 +- src/main/mcpHttpArguments.ts | 104 + src/main/messages.ts | 2 + src/main/processShutdown.ts | 99 +- src/main/shutdown.ts | 10 +- src/main/transport.ts | 25 +- src/main/types.ts | 24 +- src/server/EvidenceMcpServer.ts | 4 +- src/server/RequestTrackingMcpServer.test.ts | 88 + src/server/RequestTrackingMcpServer.ts | 92 + src/server/http/NodeHttpRequestOwner.ts | 180 ++ src/server/http/nodeHttpBridge.test.ts | 283 +++ src/server/http/nodeHttpBridge.ts | 228 ++ src/server/http/serveStreamableHttp.test.ts | 1907 +++++++++++++++++ src/server/http/serveStreamableHttp.ts | 697 ++++++ src/server/http/streamableHttpListenError.ts | 10 + src/server/registerPrompts.ts | 16 +- src/server/shutdownRegistration.test.ts | 135 +- tests/boundary/cli/cliStartupImports.test.ts | 9 +- tests/boundary/cli/dispatcher.test.ts | 20 +- tests/boundary/mcp/mainErrors.test.ts | 51 +- tests/boundary/mcp/mcpHttpOptions.test.ts | 118 + tests/boundary/mcp/promptMcp.test.ts | 100 +- tests/boundary/mcp/streamableHttpMcp.test.ts | 85 + .../boundary/process/mcpHttpShutdown.test.ts | 54 + tests/support/http/rawRequest.ts | 69 + tests/support/mcp/streamableHttpProcess.ts | 91 + 36 files changed, 5211 insertions(+), 57 deletions(-) create mode 100644 docs/specs/agent-environments/http-transport/plan.md create mode 100644 docs/specs/agent-environments/http-transport/spec.md create mode 100644 docs/specs/agent-environments/http-transport/tasks.md create mode 100644 docs/specs/agent-environments/http-transport/verification.md create mode 100644 src/config/mcpHttpEndpoint.ts create mode 100644 src/main/mcpHttpArguments.ts create mode 100644 src/server/RequestTrackingMcpServer.test.ts create mode 100644 src/server/RequestTrackingMcpServer.ts create mode 100644 src/server/http/NodeHttpRequestOwner.ts create mode 100644 src/server/http/nodeHttpBridge.test.ts create mode 100644 src/server/http/nodeHttpBridge.ts create mode 100644 src/server/http/serveStreamableHttp.test.ts create mode 100644 src/server/http/serveStreamableHttp.ts create mode 100644 src/server/http/streamableHttpListenError.ts create mode 100644 tests/boundary/mcp/mcpHttpOptions.test.ts create mode 100644 tests/boundary/mcp/streamableHttpMcp.test.ts create mode 100644 tests/boundary/process/mcpHttpShutdown.test.ts create mode 100644 tests/support/http/rawRequest.ts create mode 100644 tests/support/mcp/streamableHttpProcess.ts 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 }; +}; From 4af71a45f9c740b292b0b454ba11bcc0494874b8 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:03:34 +0800 Subject: [PATCH 2/4] fix(process): reconcile native lifetime and selective Linux UID facts Settle absent and zombie candidates before retaining baseline or token-read failures. Preserve fail-closed behavior for live and unknown generations, and stop retrying exited candidates. Bind Linux effective UID to stat-bracketed status reads only for unreadable-token candidates, preserving partial identity facts and avoiding full-table UID enrichment. --- src/process/LinuxProcessStatus.test.ts | 21 +++ src/process/LinuxProcessStatus.ts | 19 +++ src/process/ProcessOwnership.identity.test.ts | 21 ++- .../ProcessOwnership.procIdentity.test.ts | 148 +++++++++++++++++- .../ProcessOwnership.sweepExit.test.ts | 74 +++++++++ src/process/ProcessOwnership.ts | 63 ++++---- src/process/ProcessOwnershipObservation.ts | 112 ++++++++++--- 7 files changed, 394 insertions(+), 64 deletions(-) create mode 100644 src/process/LinuxProcessStatus.test.ts create mode 100644 src/process/LinuxProcessStatus.ts create mode 100644 src/process/ProcessOwnership.sweepExit.test.ts diff --git a/src/process/LinuxProcessStatus.test.ts b/src/process/LinuxProcessStatus.test.ts new file mode 100644 index 0000000000..46f7688447 --- /dev/null +++ b/src/process/LinuxProcessStatus.test.ts @@ -0,0 +1,21 @@ +import { expect, it } from "vitest"; + +import { parseLinuxProcessEffectiveUid } from "./LinuxProcessStatus.js"; + +it("parses the effective UID from the single native status Uid row", () => { + expect( + parseLinuxProcessEffectiveUid( + "Name:\tworker\nUid:\t1000\t1001\t1002\t1003\nGid:\t20\t20\t20\t20\n", + ), + ).toBe(1001); +}); + +it.each([ + "Name:\tworker\n", + "Uid:\t1000\t1001\t1002\n", + "Uid:\t1000\t1001\t1002\t1003\nUid:\t1000\t1001\t1002\t1003\n", + "Uid:\t1000\t-1\t1002\t1003\n", + "Uid:\t1000\t9007199254740992\t1002\t1003\n", +])("does not infer an effective UID from malformed status: %s", (status) => { + expect(parseLinuxProcessEffectiveUid(status)).toBeUndefined(); +}); diff --git a/src/process/LinuxProcessStatus.ts b/src/process/LinuxProcessStatus.ts new file mode 100644 index 0000000000..60d8f2809e --- /dev/null +++ b/src/process/LinuxProcessStatus.ts @@ -0,0 +1,19 @@ +/** Effective UID parsed from one Linux `/proc//status` record. */ +export const parseLinuxProcessEffectiveUid = ( + value: string, +): number | undefined => { + let effectiveUid: number | undefined; + for (const line of value.split("\n")) { + if (!line.startsWith("Uid:")) continue; + if (effectiveUid !== undefined) return undefined; + const match = /^Uid:\s+([0-9]+)\s+([0-9]+)\s+([0-9]+)\s+([0-9]+)\s*$/u.exec( + line, + ); + if (match === null) return undefined; + const values = match.slice(1).map(Number); + if (values.some((uid) => !Number.isSafeInteger(uid) || uid < 0)) + return undefined; + effectiveUid = values[1]; + } + return effectiveUid; +}; diff --git a/src/process/ProcessOwnership.identity.test.ts b/src/process/ProcessOwnership.identity.test.ts index 1d518fda2c..5b5c69f988 100644 --- a/src/process/ProcessOwnership.identity.test.ts +++ b/src/process/ProcessOwnership.identity.test.ts @@ -32,7 +32,11 @@ describe("Linux process start identities", () => { state: "S", command: process.execPath, } satisfies ProcessTableEntry; - const identities = await host.processIdentities?.([observedProcess]); + const identities = await host.processIdentities?.( + [observedProcess], + undefined, + new Set([observedProcess.pid]), + ); expect(identities?.get(observedProcess.pid)).toMatchObject({ state: "readable", identity: expect.stringMatching(/^linux-starttime:\d+$/u), @@ -43,8 +47,9 @@ describe("Linux process start identities", () => { processGroupId: expect.any(Number), }, }); - expect(identities?.get(observedProcess.pid)).not.toHaveProperty( + expect(identities?.get(observedProcess.pid)).toHaveProperty( "effectiveUid", + process.geteuid!(), ); const baseline = await host.captureBaseline?.(); expect(baseline).toContainEqual({ @@ -110,9 +115,11 @@ it("validates and cleans an exact-token owner without a capture baseline", async expect(signalGroup).toHaveBeenCalledWith(process.processGroupId, "SIGKILL"); }); -it.skipIf(typeof process.getuid !== "function")( - "uses a same-generation native UID instead of the process-table UID for unreadable tokens", - async () => { +it + .skipIf(typeof process.getuid !== "function") + .each(["EINVAL", "EACCES: permission denied"])( + "uses a same-generation native UID instead of the process-table UID for unreadable tokens: %s", + async (diagnostic) => { const callerUid = process.getuid!(); const foreignUid = callerUid === 0 ? 1 : 0; const verify = async (rowUid: number, effectiveUid: number) => { @@ -132,7 +139,7 @@ it.skipIf(typeof process.getuid !== "function")( runTokens: () => Promise.resolve( new Map([ - [906, { state: "unavailable", reason: "EINVAL" } as const], + [906, { state: "unavailable", reason: diagnostic } as const], ]), ), processIdentities: (entries) => @@ -161,7 +168,7 @@ it.skipIf(typeof process.getuid !== "function")( }); await expect(verify(callerUid, foreignUid)).resolves.toMatchObject({ cleaned: true, - unverified: [{ pid: 906, diagnostic: "EINVAL" }], + unverified: [{ pid: 906, diagnostic }], }); }, ); diff --git a/src/process/ProcessOwnership.procIdentity.test.ts b/src/process/ProcessOwnership.procIdentity.test.ts index 6b2eaa3eb6..fff53db724 100644 --- a/src/process/ProcessOwnership.procIdentity.test.ts +++ b/src/process/ProcessOwnership.procIdentity.test.ts @@ -9,8 +9,14 @@ vi.mock("node:fs/promises", async (importOriginal) => { return { ...original, readFile: filesystem.readFile }; }); -import { createSystemProcessOwnershipHost } from "./ProcessOwnershipObservation.js"; -import type { ProcessTableEntry } from "./ProcessOwnership.js"; +import { + createSystemProcessOwnershipHost, + readProcessOwnershipFacts, +} from "./ProcessOwnershipObservation.js"; +import type { + ProcessOwnershipHost, + ProcessTableEntry, +} from "./ProcessOwnership.js"; const entry: ProcessTableEntry = { pid: 42, @@ -23,6 +29,144 @@ const entry: ProcessTableEntry = { const filesystemError = (code: string): NodeJS.ErrnoException => Object.assign(new Error(code), { code }); +const procStat = ( + startTime: number, + parentPid = 1, + processGroupId = 42, + pid = 42, +) => { + const fields = ["S", String(parentPid), String(processGroupId), "42"]; + while (fields.length < 19) fields.push("0"); + fields.push(String(startTime)); + return `${String(pid)} (fixture) ${fields.join(" ")}`; +}; + +it("binds Linux effective UID to the same native process generation", async () => { + const host = createSystemProcessOwnershipHost("linux", {}); + filesystem.readFile + .mockResolvedValueOnce(procStat(100, 10, 42)) + .mockResolvedValueOnce("Name:\tfixture\nUid:\t1000\t1001\t1002\t1003\n") + .mockResolvedValueOnce(procStat(100, 11, 43)); + + await expect( + host.processIdentities?.([entry], undefined, new Set([entry.pid])), + ).resolves.toEqual( + new Map([ + [ + 42, + { + state: "readable", + identity: "linux-starttime:100", + effectiveUid: 1001, + lifecycle: { state: "observed", lifecycle: "non-zombie" }, + relation: { + state: "observed", + parentPid: 11, + processGroupId: 43, + }, + }, + ], + ]), + ); + expect(filesystem.readFile.mock.calls.map(([path]) => path)).toEqual([ + "/proc/42/stat", + "/proc/42/status", + "/proc/42/stat", + ]); +}); + +it("keeps native identity when status UID is unavailable", async () => { + const host = createSystemProcessOwnershipHost("linux", {}); + filesystem.readFile + .mockResolvedValueOnce(procStat(100)) + .mockRejectedValueOnce(filesystemError("EACCES")); + + await expect( + host.processIdentities?.([entry], undefined, new Set([entry.pid])), + ).resolves.toEqual( + new Map([ + [ + 42, + { + state: "readable", + identity: "linux-starttime:100", + lifecycle: { state: "observed", lifecycle: "non-zombie" }, + relation: { + state: "observed", + parentPid: 1, + processGroupId: 42, + }, + }, + ], + ]), + ); + expect(filesystem.readFile).toHaveBeenCalledTimes(2); +}); + +it("does not attach a UID when the process generation changes around status", async () => { + const host = createSystemProcessOwnershipHost("linux", {}); + filesystem.readFile + .mockResolvedValueOnce(procStat(100)) + .mockResolvedValueOnce("Uid:\t1000\t1001\t1002\t1003\n") + .mockResolvedValueOnce(procStat(101)); + + await expect( + host.processIdentities?.([entry], undefined, new Set([entry.pid])), + ).resolves.toEqual( + new Map([ + [ + 42, + { + state: "unavailable", + reason: "process_identity_changed_during_uid_observation", + }, + ], + ]), + ); +}); + +it("reads Linux effective UID only for token-unavailable rows", async () => { + const rows = [entry, { ...entry, pid: 43, processGroupId: 43 }]; + const statReads = new Map(); + const uidStatusPids: number[] = []; + filesystem.readFile.mockImplementation(async (path) => { + const match = /^\/proc\/(\d+)\/(stat|status)$/u.exec(String(path)); + if (match === null) throw new Error(`unexpected path: ${String(path)}`); + const pid = Number(match[1]); + if (match[2] === "status") { + uidStatusPids.push(pid); + return "Uid:\t1000\t1001\t1002\t1003\n"; + } + const reads = (statReads.get(pid) ?? 0) + 1; + statReads.set(pid, reads); + return procStat(100, 1, pid, pid); + }); + const systemHost = createSystemProcessOwnershipHost("linux", {}); + const host: ProcessOwnershipHost = { + ...systemHost, + runTokens: async () => + new Map([ + [entry.pid, { state: "unavailable", reason: "EACCES" }], + [43, { state: "readable", runId: undefined }], + ]), + }; + + const facts = await readProcessOwnershipFacts(rows, host); + + expect(uidStatusPids).toEqual([entry.pid]); + expect(facts.map(({ identityAfter }) => identityAfter)).toMatchObject([ + { state: "readable", effectiveUid: 1001 }, + { state: "readable" }, + ]); + expect(facts[1]?.identityAfter).not.toHaveProperty("effectiveUid"); + expect(statReads).toEqual( + new Map([ + [entry.pid, 3], + [43, 2], + ]), + ); +}); + it("reports absence only when /proc lookup and signal-zero both report ESRCH", async () => { const host = createSystemProcessOwnershipHost("linux", {}); const signalProbe = vi.spyOn(process, "kill").mockImplementation(() => { diff --git a/src/process/ProcessOwnership.sweepExit.test.ts b/src/process/ProcessOwnership.sweepExit.test.ts new file mode 100644 index 0000000000..fb0b9b2f84 --- /dev/null +++ b/src/process/ProcessOwnership.sweepExit.test.ts @@ -0,0 +1,74 @@ +import { expect, it, vi } from "vitest"; +import { + verifyNoTokenOwnedProcesses, + type ProcessIdentityObservation, + type ProcessOwnershipHost, + type ProcessTableEntry, +} from "./ProcessOwnership.js"; +import { ProcessOwnershipEvidenceOwner } from "./ProcessOwnershipEvidence.js"; + +it.each(["absent", "zombie", "live", "unavailable"] as const)( + "settles a baseline candidate against its final native lifetime: %s", + async (lifetime) => { + const row: ProcessTableEntry = { + pid: 910, + parentPid: 1, + processGroupId: 910, + state: "S", + command: "baseline-neighbor", + }; + let tokenRead = false; + const identity = (): ProcessIdentityObservation => { + if (lifetime === "absent") + return { state: "absent", reason: "not-in-snapshot" }; + if (lifetime === "unavailable") + return { state: "unavailable", reason: "native identity denied" }; + return { + state: "readable", + identity: "new-generation", + lifecycle: { + state: "observed", + lifecycle: + tokenRead && lifetime === "zombie" ? "zombie" : "non-zombie", + }, + relation: { state: "observed", parentPid: 1, processGroupId: 910 }, + }; + }; + const signalGroup = vi.fn(); + const host: ProcessOwnershipHost = { + platform: "darwin", + listProcesses: async () => [row], + processIdentities: async (members) => + new Map(members.map(({ pid }) => [pid, identity()])), + runTokens: async (members) => { + tokenRead = true; + return new Map( + members.map(({ pid }) => [ + pid, + { state: "unavailable" as const, reason: "EACCES" }, + ]), + ); + }, + environment: async () => ({}), + signalGroup, + }; + const result = await verifyNoTokenOwnedProcesses( + "run", + host, + [{ pid: row.pid, identity: "baseline-generation" }], + { + leaderPid: 100, + processGroupId: 100, + evidenceOwner: new ProcessOwnershipEvidenceOwner(), + }, + ); + if (lifetime === "absent" || lifetime === "zombie") + expect(result).toEqual({ cleaned: true, signaled: false }); + else + expect(result).toMatchObject({ + cleaned: false, + failures: [{ pid: row.pid }], + }); + expect(signalGroup).not.toHaveBeenCalled(); + }, +); diff --git a/src/process/ProcessOwnership.ts b/src/process/ProcessOwnership.ts index 3cc3d6ee22..307b20220c 100644 --- a/src/process/ProcessOwnership.ts +++ b/src/process/ProcessOwnership.ts @@ -179,6 +179,8 @@ export interface ProcessOwnershipHost { processIdentities?( processes: readonly ProcessTableEntry[], signal?: AbortSignal, + /** Linux UID enrichment is selective; omitted means UID remains unknown. */ + effectiveUidPids?: ReadonlySet, ): Promise>; /** Snapshot identities before launch, honoring startup cancellation. */ captureBaseline?(signal?: AbortSignal): Promise; @@ -965,6 +967,14 @@ const sanitizedTokenReadFailure = (diagnostic: string | undefined): string => { return "other_unavailable"; }; +const processIdentityHasExited = ( + identity: ProcessIdentityObservation, +): boolean => + identity.state === "absent" || + (identity.state === "readable" && + identity.lifecycle?.state === "observed" && + identity.lifecycle.lifecycle === "zombie"); + const processOwnershipFailures = async ( members: readonly ProcessTableEntry[], runId: string, @@ -975,13 +985,7 @@ const processOwnershipFailures = async ( const facts = await readProcessOwnershipFacts(members, host); const failures: ProcessOwnershipValidationFailure[] = []; for (const fact of facts) { - if (fact.identityAfter.state === "absent") continue; - if ( - fact.identityAfter.state === "readable" && - fact.identityAfter.lifecycle?.state === "observed" && - fact.identityAfter.lifecycle.lifecycle === "zombie" - ) - continue; + if (processIdentityHasExited(fact.identityAfter)) continue; const relation = fact.identityAfter.state === "readable" && fact.identityAfter.relation?.state === "observed" @@ -1289,14 +1293,17 @@ const scanWithOwnershipEvidence = async ( observationRows, host, ); - const baselineIdentityFailures: ProcessOwnershipValidationFailure[] = []; + const baselineIdentityFailures = new Map< + number, + ProcessOwnershipValidationFailure + >(); const candidates = observationRows.filter((process) => { if (retainedPids.has(process.pid)) return true; if (!baseline.has(process.pid)) return true; const before = baseline.get(process.pid); const now = firstIdentities.get(process.pid); if (before === null || now?.state !== "readable") { - baselineIdentityFailures.push({ + baselineIdentityFailures.set(process.pid, { pid: process.pid, reason: "process-identity-unavailable", diagnostic: "process identity was unavailable during token validation", @@ -1320,21 +1327,18 @@ const scanWithOwnershipEvidence = async ( }, ); let facts = initialFacts; - for ( - let attempt = 0; - attempt < TOKEN_READ_RETRY_ATTEMPTS && - facts.some(({ token }) => token.state === "unavailable"); - attempt += 1 - ) { + for (let attempt = 0; attempt < TOKEN_READ_RETRY_ATTEMPTS; attempt += 1) { + const retryRows = facts + .filter( + ({ token, identityAfter }) => + token.state === "unavailable" && + !processIdentityHasExited(identityAfter), + ) + .map(({ process }) => process); + if (retryRows.length === 0) break; await new Promise((resolve) => setTimeout(resolve, TOKEN_READ_RETRY_DELAY_MS), ); - const pendingPids = new Set( - facts - .filter(({ token }) => token.state === "unavailable") - .map(({ process }) => process.pid), - ); - const retryRows = candidates.filter(({ pid }) => pendingPids.has(pid)); const retries = await readProcessOwnershipFacts(retryRows, host); const retryByPid = new Map(retries.map((fact) => [fact.process.pid, fact])); facts = facts.map((fact) => retryByPid.get(fact.process.pid) ?? fact); @@ -1356,24 +1360,26 @@ const scanWithOwnershipEvidence = async ( const retainedByPid = new Map( retained.map((candidate) => [candidate.pid, candidate]), ); - const failures: ProcessOwnershipValidationFailure[] = [ - ...baselineIdentityFailures, - ]; + const failures: ProcessOwnershipValidationFailure[] = []; const unverified: OwnershipSweepUnverifiedProcess[] = []; const owned: ProcessTableEntry[] = []; const identities = new Map(); for (const fact of facts) { + // The table row and baseline predate the final native observation. An + // exited candidate needs no ownership proof or signal, even when its + // token became unreadable while exiting. Unknown native state stays a + // cleanup failure; table omission alone is never proof of exit. + if (processIdentityHasExited(fact.identityAfter)) continue; + const baselineFailure = baselineIdentityFailures.get(fact.process.pid); + if (baselineFailure !== undefined) failures.push(baselineFailure); if (fact.state === "replaced") failures.push({ pid: fact.process.pid, reason: "process-identity-unavailable", diagnostic: "process identity changed during token validation", }); - else if ( - fact.state === "unavailable" && - fact.identityAfter.state !== "absent" - ) + else if (fact.state === "unavailable") failures.push({ pid: fact.process.pid, reason: "process-identity-unavailable", @@ -1392,7 +1398,6 @@ const scanWithOwnershipEvidence = async ( reason: "run-token-mismatch", }); if (fact.token.state !== "unavailable") continue; - if (fact.identityAfter.state === "absent") continue; const relationBefore = fact.identityBefore.state === "readable" ? fact.identityBefore.relation diff --git a/src/process/ProcessOwnershipObservation.ts b/src/process/ProcessOwnershipObservation.ts index 592833855d..01439da94f 100644 --- a/src/process/ProcessOwnershipObservation.ts +++ b/src/process/ProcessOwnershipObservation.ts @@ -1,5 +1,6 @@ import { readFile } from "node:fs/promises"; import { parseLinuxProcessStat } from "./LinuxProcessStat.js"; +import { parseLinuxProcessEffectiveUid } from "./LinuxProcessStatus.js"; import { AnalysisCapabilityUnavailableError } from "../domain/analysisErrorCore.js"; import { launcherIdentityFailure } from "./ProcessOwnershipIdentity.js"; @@ -498,6 +499,7 @@ const readIdentityFacts = async ( host: ProcessOwnershipHost, phase: "before" | "after", signal?: AbortSignal, + effectiveUidPids?: ReadonlySet, ): Promise> => { signal?.throwIfAborted(); if (host.processIdentities === undefined) @@ -511,10 +513,11 @@ const readIdentityFacts = async ( ]), ); try { - const observations = - signal === undefined - ? await host.processIdentities(processes) - : await host.processIdentities(processes, signal); + const observations = await host.processIdentities( + processes, + signal, + effectiveUidPids, + ); signal?.throwIfAborted(); return new Map( processes.map(({ pid }) => [pid, identityFact(observations, pid, phase)]), @@ -583,11 +586,17 @@ export const readProcessOwnershipFacts = async ( } signal?.throwIfAborted(); + const effectiveUidPids = new Set( + processes + .filter(({ pid }) => tokens.get(pid)?.state !== "readable") + .map(({ pid }) => pid), + ); const identityAfter = await readIdentityFacts( processes, host, "after", signal, + effectiveUidPids, ); signal?.throwIfAborted(); const facts = processes.map((process): ProcessOwnershipFact => { @@ -699,7 +708,7 @@ export const createSystemProcessOwnershipHost = ( }; const processIdentities: NonNullable< ProcessOwnershipHost["processIdentities"] - > = async (processes, signal) => { + > = async (processes, signal, effectiveUidPids) => { if (processes.length === 0) return new Map(); if (platform === "linux") return new Map( @@ -716,27 +725,78 @@ export const createSystemProcessOwnershipHost = ( ...(signal === undefined ? {} : { signal }), }); const parsed = parseLinuxProcessStat(stat); - return [ - pid, - parsed === undefined || parsed.pid !== pid - ? ({ - state: "unavailable", - reason: "malformed_proc_stat", - } as const) - : ({ - state: "readable", - identity: `linux-starttime:${parsed.startTime}`, - lifecycle: { - state: "observed", - lifecycle: parsed.lifecycle, - }, - relation: { - state: "observed", - parentPid: parsed.parentPid, - processGroupId: parsed.processGroupId, - }, - } as const), - ] as const; + if (parsed === undefined || parsed.pid !== pid) + return [ + pid, + { + state: "unavailable", + reason: "malformed_proc_stat", + } as const, + ] as const; + + const readableIdentity = ( + facts: typeof parsed, + effectiveUid?: number, + ) => ({ + state: "readable" as const, + identity: `linux-starttime:${facts.startTime}`, + ...(effectiveUid === undefined ? {} : { effectiveUid }), + lifecycle: { + state: "observed" as const, + lifecycle: facts.lifecycle, + }, + relation: { + state: "observed" as const, + parentPid: facts.parentPid, + processGroupId: facts.processGroupId, + }, + }); + + if (!effectiveUidPids?.has(pid)) + return [pid, readableIdentity(parsed)] as const; + + let status: string; + try { + status = await readFile(`/proc/${String(pid)}/status`, { + encoding: "utf8", + ...(signal === undefined ? {} : { signal }), + }); + } catch (cause: unknown) { + if (isExpectedAbort(cause, signal)) throw cause; + return [pid, readableIdentity(parsed)] as const; + } + signal?.throwIfAborted(); + const effectiveUid = parseLinuxProcessEffectiveUid(status); + if (effectiveUid === undefined) + return [pid, readableIdentity(parsed)] as const; + + let afterStatus: string; + try { + afterStatus = await readFile(`/proc/${String(pid)}/stat`, { + encoding: "utf8", + ...(signal === undefined ? {} : { signal }), + }); + } catch (cause: unknown) { + if (isExpectedAbort(cause, signal)) throw cause; + return [pid, readableIdentity(parsed)] as const; + } + signal?.throwIfAborted(); + const verified = parseLinuxProcessStat(afterStatus); + if ( + verified !== undefined && + verified.pid === pid && + verified.startTime !== parsed.startTime + ) + return [ + pid, + { + state: "unavailable", + reason: "process_identity_changed_during_uid_observation", + } as const, + ] as const; + if (verified === undefined || verified.pid !== pid) + return [pid, readableIdentity(parsed)] as const; + return [pid, readableIdentity(verified, effectiveUid)] as const; } catch (cause: unknown) { if (isExpectedAbort(cause, signal)) throw cause; if ( From 0949313d8fa222dfd48d33828e19924d55775981 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:03:45 +0800 Subject: [PATCH 3/4] fix(schema): compose portable stack-safe base64 constraints Replace the lookahead-based pattern with separate allOf/not JSON Schema rules for quartet length, alphabet, padding and canonical pad bits. Share the runtime predicates with HAR and padded base64 parsing. Validate converted advertised leaves with the MCP SDK validator, preserve permissive versus canonical semantics, and exercise every pattern near the 32 MiB encoded-input budget in all JavaScript modes. --- src/browser/history/HarCapture.ts | 4 +- src/domain/go/goBinary.ts | 10 +-- src/domain/native/binaryLayout.ts | 4 +- src/domain/native/recordedCrash.ts | 4 +- src/domain/paddedBase64.ts | 5 +- src/domain/stringPatterns.test.ts | 80 ++++++++++++++----- src/domain/stringPatterns.ts | 48 ++++++++--- .../mcp/advertisedPatternsVMode.test.ts | 66 +++++++++++++++ 8 files changed, 174 insertions(+), 47 deletions(-) diff --git a/src/browser/history/HarCapture.ts b/src/browser/history/HarCapture.ts index 9e164e92d9..ec55644d0e 100644 --- a/src/browser/history/HarCapture.ts +++ b/src/browser/history/HarCapture.ts @@ -2,7 +2,7 @@ import { createHash } from "node:crypto"; import { LosslessNumber } from "lossless-json"; import { parseCaptureJson } from "./CaptureJson.js"; import type { JsonValue } from "../../domain/jsonValue.js"; -import { CANONICAL_BASE64_PATTERN } from "../../domain/stringPatterns.js"; +import { isCanonicalBase64 } from "../../domain/stringPatterns.js"; import { WEB_NETWORK_CAPTURE_LIMITS, type WebNetworkCapture, @@ -265,7 +265,7 @@ const integerLexeme = (literal: string): boolean => { const decodeHarBase64 = (value: string, pointer: string): Buffer => { const compact = value.replace(/[\t\n\r ]/g, ""); - if (compact.length % 4 !== 0 || !CANONICAL_BASE64_PATTERN.test(compact)) + if (!isCanonicalBase64(compact)) throw new CaptureFormatError( "format", "HAR content declares base64 but its text is not valid canonical base64 (ASCII whitespace is allowed).", diff --git a/src/domain/go/goBinary.ts b/src/domain/go/goBinary.ts index 8714159c38..550d7a8348 100644 --- a/src/domain/go/goBinary.ts +++ b/src/domain/go/goBinary.ts @@ -1,7 +1,7 @@ import { z } from "zod"; import { digestSchema } from "../digests.js"; import { localPathStringSchema } from "../localPath.js"; -import { CANONICAL_BASE64_PATTERN } from "../stringPatterns.js"; +import { canonicalBase64Schema } from "../stringPatterns.js"; const sourceRangeSchema = z.strictObject({ offset: z.number().int().nonnegative(), @@ -25,9 +25,7 @@ export const goModuleMetadataSchema = z.strictObject({ dependencies: z.array(goModuleSchema), settings: z.array(z.strictObject({ key: z.string(), value: z.string() })), unparsed_lines: z.array(z.string()), - unparsed_line_bytes_base64: z.array( - z.string().regex(CANONICAL_BASE64_PATTERN), - ), + unparsed_line_bytes_base64: z.array(canonicalBase64Schema), complete: z.boolean(), }); @@ -43,9 +41,9 @@ export const goBuildInfoSchema = z.strictObject({ header_offset: z.number().int().nonnegative(), encoding: z.enum(["inline", "pointer"]), go_version: z.string().nullable(), - go_version_bytes_base64: z.string().regex(CANONICAL_BASE64_PATTERN), + go_version_bytes_base64: canonicalBase64Schema, module_text: z.string().nullable(), - module_bytes_base64: z.string().regex(CANONICAL_BASE64_PATTERN), + module_bytes_base64: canonicalBase64Schema, version_location: sourceRangeSchema, module_location: sourceRangeSchema, module: goModuleMetadataSchema, diff --git a/src/domain/native/binaryLayout.ts b/src/domain/native/binaryLayout.ts index cc37376dd9..c1a694cc2d 100644 --- a/src/domain/native/binaryLayout.ts +++ b/src/domain/native/binaryLayout.ts @@ -2,13 +2,13 @@ import { z } from "zod"; import { canBorrowParsedJsonSnapshot } from "../immutableJson.js"; import { localPathStringSchema } from "../localPath.js"; -import { CANONICAL_BASE64_PATTERN } from "../stringPatterns.js"; +import { canonicalBase64Schema } from "../stringPatterns.js"; const unsignedHex = z.string().regex(/^0x(?:0|[1-9a-f][0-9a-f]{0,15})$/); const index = z.number().int().nonnegative(); const scalar = z.union([z.string(), z.number().int()]); const range = z.strictObject({ offset: unsignedHex, bytes: unsignedHex }); -const canonicalBase64 = z.string().regex(CANONICAL_BASE64_PATTERN); +const canonicalBase64 = canonicalBase64Schema; const name = z.strictObject({ display: z.string(), bytes_base64: canonicalBase64.nullable(), diff --git a/src/domain/native/recordedCrash.ts b/src/domain/native/recordedCrash.ts index 8bd04cac14..0163a85329 100644 --- a/src/domain/native/recordedCrash.ts +++ b/src/domain/native/recordedCrash.ts @@ -2,13 +2,13 @@ import { z } from "zod"; import type { BinaryArchitecture } from "../binaryTargetTypes.js"; import { localPathStringSchema } from "../localPath.js"; -import { CANONICAL_BASE64_PATTERN } from "../stringPatterns.js"; +import { canonicalBase64Schema } from "../stringPatterns.js"; import { recordedNoteCoverageIssues } from "./recordedCrashNoteCoverage.js"; const hex = z.string().regex(/^0x(?:0|[1-9a-f][0-9a-f]{0,15})$/); const index = z.number().int().nonnegative(); const signed32 = z.number().int().min(-2147483648).max(2147483647); -const base64 = z.string().regex(CANONICAL_BASE64_PATTERN); +const base64 = canonicalBase64Schema; const range = z.strictObject({ offset: hex, bytes: hex }); const diagnostics = z.strictObject({ stdout: z.string(), diff --git a/src/domain/paddedBase64.ts b/src/domain/paddedBase64.ts index 395bdf5b8e..b4402f6b5e 100644 --- a/src/domain/paddedBase64.ts +++ b/src/domain/paddedBase64.ts @@ -1,9 +1,8 @@ -import { PADDED_BASE64_PATTERN } from "./stringPatterns.js"; +import { isPaddedBase64 } from "./stringPatterns.js"; /** Validate padded syntax without imposing canonical zero pad bits. */ export const paddedBase64ByteLength = (value: string): number | undefined => { - if (value.length % 4 !== 0 || !PADDED_BASE64_PATTERN.test(value)) - return undefined; + if (!isPaddedBase64(value)) return undefined; const padding = value.endsWith("==") ? 2 : value.endsWith("=") ? 1 : 0; return (value.length / 4) * 3 - padding; }; diff --git a/src/domain/stringPatterns.test.ts b/src/domain/stringPatterns.test.ts index 7537742bd3..4410ad7b95 100644 --- a/src/domain/stringPatterns.test.ts +++ b/src/domain/stringPatterns.test.ts @@ -1,10 +1,16 @@ import { expect, it } from "vitest"; +import { z } from "zod"; import { - CANONICAL_BASE64_PATTERN, + BASE64_QUARTET_LENGTH_PATTERN, + INVALID_BASE64_CHARACTER_PATTERN, + INVALID_BASE64_PADDING_PATTERN, + INVALID_CANONICAL_BASE64_PAD_BITS_PATTERN, IDENTIFIER_PATTERN, - PADDED_BASE64_PATTERN, STABLE_IDENTIFIER_PATTERN, + canonicalBase64Schema, + isCanonicalBase64, + isPaddedBase64, } from "./stringPatterns.js"; it("preserves identifier grammars under Unicode and Unicode-set compilation", () => { @@ -43,7 +49,7 @@ it("preserves identifier grammars under Unicode and Unicode-set compilation", () } }); -it("preserves canonical base64 padding and pad-bit validation", () => { +it("preserves canonical and permissive padded base64 validation", () => { const previous = /^(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/][AQgw]==|[A-Za-z0-9+/]{2}[AEIMQUYcgkosw048]=)?$/; const samples = [ @@ -71,28 +77,62 @@ it("preserves canonical base64 padding and pad-bit validation", () => { for (const first of alphabet) for (const second of alphabet) samples.push(`${first}${second}==`, `A${first}${second}=`); - for (const flag of ["u", "v"]) { - const projected = new RegExp(CANONICAL_BASE64_PATTERN.source, flag); - for (const sample of samples) - expect(projected.test(sample), JSON.stringify({ flag, sample })).toBe( - previous.test(sample), - ); + for (const sample of samples) { + expect(isCanonicalBase64(sample), JSON.stringify({ sample })).toBe( + previous.test(sample), + ); + expect(isPaddedBase64(sample), JSON.stringify({ sample })).toBe( + sample.length % 4 === 0 && /^[A-Za-z0-9+/]*(?:==|=)?$/.test(sample), + ); } }); -it("validates large canonical base64 under JSON Schema Unicode modes", () => { - // Near the 32 MiB capture input budget: a quartet expansion alone still - // overflows at this size, especially in Unicode-set client validators. +it("advertises every portable base64 constraint and keeps large inputs stack-safe", () => { + const jsonSchema = z.toJSONSchema(canonicalBase64Schema, { + target: "draft-2020-12", + }); + expect(jsonSchema).toMatchObject({ + allOf: expect.arrayContaining([ + { pattern: BASE64_QUARTET_LENGTH_PATTERN.source }, + { not: { pattern: INVALID_BASE64_CHARACTER_PATTERN.source } }, + { not: { pattern: INVALID_BASE64_PADDING_PATTERN.source } }, + { not: { pattern: INVALID_CANONICAL_BASE64_PAD_BITS_PATTERN.source } }, + ]), + }); + const body = Buffer.alloc(24 * 1024 * 1024 - 3, 0x81); const encoded = body.toString("base64"); for (const flag of ["", "u", "v"]) { - const pattern = new RegExp(CANONICAL_BASE64_PATTERN.source, flag); - expect(pattern.test(encoded), flag).toBe(true); - expect(pattern.test(`${encoded.slice(0, -1)}!`), flag).toBe(false); - expect(pattern.test(`${encoded.slice(0, -3)}R==`), flag).toBe(false); - expect(pattern.test(`${encoded}A`), flag).toBe(false); - const padded = new RegExp(PADDED_BASE64_PATTERN.source, flag); - expect(padded.test(encoded), flag).toBe(true); - expect(padded.test(`${encoded.slice(0, -1)}!`), flag).toBe(false); + expect( + new RegExp(BASE64_QUARTET_LENGTH_PATTERN.source, flag).test(encoded), + flag, + ).toBe(true); + expect( + new RegExp(INVALID_BASE64_CHARACTER_PATTERN.source, flag).test(encoded), + flag, + ).toBe(false); + expect( + new RegExp(INVALID_BASE64_PADDING_PATTERN.source, flag).test(encoded), + flag, + ).toBe(false); + expect( + new RegExp(INVALID_CANONICAL_BASE64_PAD_BITS_PATTERN.source, flag).test( + encoded, + ), + flag, + ).toBe(false); + expect( + new RegExp(INVALID_BASE64_CHARACTER_PATTERN.source, flag).test( + `${encoded.slice(0, -1)}!`, + ), + flag, + ).toBe(true); + expect( + new RegExp(INVALID_CANONICAL_BASE64_PAD_BITS_PATTERN.source, flag).test( + `${encoded.slice(0, -3)}R==`, + ), + flag, + ).toBe(true); } + expect(isCanonicalBase64(encoded)).toBe(true); }); diff --git a/src/domain/stringPatterns.ts b/src/domain/stringPatterns.ts index ab8994f17f..718cc1d1d4 100644 --- a/src/domain/stringPatterns.ts +++ b/src/domain/stringPatterns.ts @@ -1,20 +1,44 @@ +import { z } from "zod"; + /** Portable ASCII identifier starting with a letter. */ export const IDENTIFIER_PATTERN = /^[A-Za-z][A-Za-z0-9._\x2d]*$/u; /** Stable evidence identifier, also admitting namespace and path separators. */ export const STABLE_IDENTIFIER_PATTERN = /^[A-Za-z][A-Za-z0-9._:\x2f\x2d]*$/u; -// Repeated alphabet quartets exhaust V8's regexp stack on large bodies, -// including under JSON Schema's Unicode-set mode. Check alphabet and terminal -// padding separately, leaving an unconstrained fixed-width loop for length. -const PADDED_BASE64_PREFIX = String.raw`^(?!.*[^A-Za-z0-9+\x2f=])(?!.*=[^=])(?!.*===)`; +// These rules are separate JSON Schema constraints. A single RE2-compatible +// regex for alphabet and quartet validity requires a repeated alphabet group; +// V8's Unicode-set engine overflows its regexp stack on that group near the +// provider's 32 MiB encoded-input budget. Wildcard quartets and literal bad +// token probes stay stack-safe at that size and compose exactly with `allOf`. +export const BASE64_QUARTET_LENGTH_PATTERN = /^(?:....)*$/; +export const INVALID_BASE64_CHARACTER_PATTERN = /[^A-Za-z0-9+\x2f=]/; +export const INVALID_BASE64_PADDING_PATTERN = /(?:=[^=]|===)/; +export const INVALID_CANONICAL_BASE64_PAD_BITS_PATTERN = + /(?:[^AQgw]==$|[BCDFGHJKLNOPRSTVWXZabdefhijlmnpqrtuvxyz1235679+\x2f]=$)/; -/** Padded base64 syntax, allowing nonzero pad bits and the empty encoding. */ -export const PADDED_BASE64_PATTERN = new RegExp( - `${PADDED_BASE64_PREFIX}(?:....)*$`, -); +export const isPaddedBase64 = (value: string): boolean => + value.length % 4 === 0 && + !INVALID_BASE64_CHARACTER_PATTERN.test(value) && + !INVALID_BASE64_PADDING_PATTERN.test(value); -/** Canonical padded base64, including zero pad bits and the empty encoding. */ -export const CANONICAL_BASE64_PATTERN = new RegExp( - `${PADDED_BASE64_PREFIX}(?!.*[^AQgw]==$)(?!.*[^AEIMQUYcgkosw048=]=$)(?:....)*$`, -); +export const isCanonicalBase64 = (value: string): boolean => + isPaddedBase64(value) && + !INVALID_CANONICAL_BASE64_PAD_BITS_PATTERN.test(value); + +const paddedBase64SchemaRules = [ + { pattern: BASE64_QUARTET_LENGTH_PATTERN.source }, + { not: { pattern: INVALID_BASE64_CHARACTER_PATTERN.source } }, + { not: { pattern: INVALID_BASE64_PADDING_PATTERN.source } }, +]; + +/** Canonical padded base64 with zero pad bits, including the empty encoding. */ +export const canonicalBase64Schema = z + .string() + .refine(isCanonicalBase64, "Expected canonical padded base64") + .meta({ + allOf: [ + ...paddedBase64SchemaRules, + { not: { pattern: INVALID_CANONICAL_BASE64_PAD_BITS_PATTERN.source } }, + ], + }); diff --git a/tests/boundary/mcp/advertisedPatternsVMode.test.ts b/tests/boundary/mcp/advertisedPatternsVMode.test.ts index b48fe27aa0..1291d79d9a 100644 --- a/tests/boundary/mcp/advertisedPatternsVMode.test.ts +++ b/tests/boundary/mcp/advertisedPatternsVMode.test.ts @@ -1,7 +1,14 @@ import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; +import { AjvJsonSchemaValidator } from "@modelcontextprotocol/client/validators/ajv"; import { expect, it, onTestFinished } from "vitest"; import { createServer } from "../../../src/server/createServer.js"; +import { + BASE64_QUARTET_LENGTH_PATTERN, + INVALID_BASE64_CHARACTER_PATTERN, + INVALID_BASE64_PADDING_PATTERN, + INVALID_CANONICAL_BASE64_PAD_BITS_PATTERN, +} from "../../../src/domain/stringPatterns.js"; import { createTestBinarySession } from "../../fixtures/binarySession.js"; interface AdvertisedPattern { @@ -10,6 +17,33 @@ interface AdvertisedPattern { readonly pattern: string; } +const collectCanonicalBase64Leaves = ( + value: unknown, + leaves: Record[], +): void => { + if (value === null || typeof value !== "object") return; + if (Array.isArray(value)) { + value.forEach((entry) => collectCanonicalBase64Leaves(entry, leaves)); + return; + } + const record = value as Record; + const allOf = record.allOf; + if ( + Array.isArray(allOf) && + allOf.some( + (entry) => + entry !== null && + typeof entry === "object" && + "pattern" in entry && + entry.pattern === BASE64_QUARTET_LENGTH_PATTERN.source, + ) + ) + leaves.push(record); + Object.values(record).forEach((entry) => + collectCanonicalBase64Leaves(entry, leaves), + ); +}; + const collectPatterns = ( value: unknown, where: string, @@ -48,12 +82,29 @@ it("advertises portable NUL escapes and patterns that compile in all JS modes", const advertised = (await client.listTools()).tools; const patterns: AdvertisedPattern[] = []; + const canonicalBase64Leaves: Record[] = []; for (const tool of advertised) { collectPatterns(tool.inputSchema, "inputSchema", tool.name, patterns); + collectCanonicalBase64Leaves(tool.inputSchema, canonicalBase64Leaves); if (tool.outputSchema !== undefined) collectPatterns(tool.outputSchema, "outputSchema", tool.name, patterns); + if (tool.outputSchema !== undefined) + collectCanonicalBase64Leaves(tool.outputSchema, canonicalBase64Leaves); } + for (const pattern of [ + BASE64_QUARTET_LENGTH_PATTERN, + INVALID_BASE64_CHARACTER_PATTERN, + INVALID_BASE64_PADDING_PATTERN, + INVALID_CANONICAL_BASE64_PAD_BITS_PATTERN, + ]) + expect( + patterns.some(({ pattern: advertisedPattern }) => + advertisedPattern.includes(pattern.source), + ), + pattern.source, + ).toBe(true); + // Annex B and `u` mode accept an unescaped `-`, `/` or `[` inside a character // class, so REA's own schema validation cannot observe the defect. `v` mode // rejects them, and a client that compiles advertised patterns rejects the @@ -87,4 +138,19 @@ it("advertises portable NUL escapes and patterns that compile in all JS modes", ), ).toEqual([]); expect(patterns.some(({ pattern }) => pattern.includes("\\x00"))).toBe(true); + + expect(canonicalBase64Leaves.length).toBeGreaterThan(0); + const sdkValidator = new AjvJsonSchemaValidator(); + const validators = canonicalBase64Leaves.map((schema) => + sdkValidator.getValidator(schema), + ); + for (const value of ["", "AA==", "AQID", "AB==", "AAA", "AA$=", "AA\n=="]) + expect( + validators.map((validate) => validate(value).valid), + JSON.stringify(value), + ).toEqual( + canonicalBase64Leaves.map( + () => value === "" || value === "AA==" || value === "AQID", + ), + ); }); From aacf740a047078e79c71d528e8e188afe2ba1aff Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:03:54 +0800 Subject: [PATCH 4/4] fix(ci): correlate ownership trace threads and preserve setup failures Follow CLONE_THREAD edges across strace -ff files when excluding the ownership process's stat/status reads. Keep provider child processes outside the allowance and document the same-process attribution limit. Validate setup result array shapes and preserve the original failed-plan JSON instead of masking it with a diagnostic TypeError. --- scripts/verify-package-setup-plan.d.mts | 8 + scripts/verify-package-setup-plan.mjs | 23 +++ scripts/verify-package-setup.mjs | 25 ++- .../recorded-crash/trace-ownership.d.mts | 6 + .../verify/recorded-crash/trace-ownership.mjs | 127 +++++++++++++++ scripts/verify/recorded-crash/trace.mjs | 50 ++---- .../packageSetupPlanDiagnostics.test.ts | 39 +++++ .../recordedCrashTraceClassifier.test.ts | 144 ++++++++++++++++++ 8 files changed, 383 insertions(+), 39 deletions(-) create mode 100644 scripts/verify-package-setup-plan.d.mts create mode 100644 scripts/verify-package-setup-plan.mjs create mode 100644 scripts/verify/recorded-crash/trace-ownership.d.mts create mode 100644 scripts/verify/recorded-crash/trace-ownership.mjs create mode 100644 tests/boundary/filesystem/packageSetupPlanDiagnostics.test.ts create mode 100644 tests/boundary/filesystem/recordedCrashTraceClassifier.test.ts diff --git a/scripts/verify-package-setup-plan.d.mts b/scripts/verify-package-setup-plan.d.mts new file mode 100644 index 0000000000..08fe24aed8 --- /dev/null +++ b/scripts/verify-package-setup-plan.d.mts @@ -0,0 +1,8 @@ +/** Preserve an unexpected JSON result while describing the failed plan check. */ +export function setupPlanFailureMessage(options: { + readonly planned: unknown; + readonly plannedExecution: { readonly status: number | null }; + readonly plannedClaudeConfig: string; + readonly plannedCodexConfig: string; + readonly plannedCursorConfig: string; +}): string; diff --git a/scripts/verify-package-setup-plan.mjs b/scripts/verify-package-setup-plan.mjs new file mode 100644 index 0000000000..4734607367 --- /dev/null +++ b/scripts/verify-package-setup-plan.mjs @@ -0,0 +1,23 @@ +/** Describe a failed package setup-plan check without masking its result shape. */ +export const setupPlanFailureMessage = ({ + planned, + plannedExecution, + plannedClaudeConfig, + plannedCodexConfig, + plannedCursorConfig, +}) => { + const actionKinds = (actions) => + Array.isArray(actions) + ? actions.map((action) => action?.kind ?? action) + : actions; + return `packaged setup plan was not read-only: ${JSON.stringify({ + status: planned?.status, + exitCode: plannedExecution.status, + plannedKinds: actionKinds(planned?.plannedActions), + appliedKinds: actionKinds(planned?.appliedActions), + result: planned, + claudeConfig: plannedClaudeConfig, + codexConfig: plannedCodexConfig, + cursorConfig: plannedCursorConfig, + })}`; +}; diff --git a/scripts/verify-package-setup.mjs b/scripts/verify-package-setup.mjs index ad23548cb3..e44c4f5c88 100644 --- a/scripts/verify-package-setup.mjs +++ b/scripts/verify-package-setup.mjs @@ -21,6 +21,7 @@ import { } from "./lib/verify-package-core.mjs"; import { parse as parseJsonc } from "jsonc-parser"; import { skillReferenceIssues } from "./lib/docs-facts.mjs"; +import { setupPlanFailureMessage } from "./verify-package-setup-plan.mjs"; export const terminalModesRestored = (termios) => { if (typeof termios !== "string") return false; @@ -250,23 +251,37 @@ const verifySetupPlan = async ({ ); const planned = json(plannedExecution.stdout); if (supportedSetupHost) { + const plannedActions = Array.isArray(planned?.plannedActions) + ? planned.plannedActions + : undefined; + const appliedActions = Array.isArray(planned?.appliedActions) + ? planned.appliedActions + : undefined; const plannedClaudeConfig = await readFile(claudeConfig, "utf8"); const plannedCodexConfig = await readFile(codexTarget, "utf8"); const plannedCursorConfig = await readFile(cursorConfig, "utf8"); const plannedOpenCodeConfig = await readFile(opencodeConfig, "utf8"); if ( - planned.status !== "planned" || + planned?.status !== "planned" || plannedExecution.status !== 0 || - planned.appliedActions.length !== 0 || - !planned.plannedActions.some(({ kind }) => kind === "configure_client") || - !planned.plannedActions.some(({ kind }) => kind === "install_skill") || + plannedActions === undefined || + appliedActions === undefined || + appliedActions.length !== 0 || + !plannedActions.some((action) => action?.kind === "configure_client") || + !plannedActions.some((action) => action?.kind === "install_skill") || plannedClaudeConfig !== '{"existing":true}\n' || plannedCodexConfig !== CODEX_ORIGINAL || plannedCursorConfig !== '{"existing":true}\n' || plannedOpenCodeConfig !== opencodeOriginal ) throw new Error( - `packaged setup plan was not read-only: ${JSON.stringify({ status: planned.status, exitCode: plannedExecution.status, plannedKinds: planned.plannedActions.map(({ kind }) => kind), appliedKinds: planned.appliedActions.map(({ kind }) => kind), claudeConfig: plannedClaudeConfig, codexConfig: plannedCodexConfig, cursorConfig: plannedCursorConfig })}`, + setupPlanFailureMessage({ + planned, + plannedExecution, + plannedClaudeConfig, + plannedCodexConfig, + plannedCursorConfig, + }), ); } return { planned }; diff --git a/scripts/verify/recorded-crash/trace-ownership.d.mts b/scripts/verify/recorded-crash/trace-ownership.d.mts new file mode 100644 index 0000000000..0daeba3737 --- /dev/null +++ b/scripts/verify/recorded-crash/trace-ownership.d.mts @@ -0,0 +1,6 @@ +/** Assert the documented procfs allowance over strace -ff process/thread files. */ +export function assertHistoricalPidReadsAreOwnershipOnly(options: { + readonly traces: readonly { readonly pid: number; readonly text: string }[]; + readonly sentinelPid: number; + readonly psExecutables: ReadonlySet; +}): string[]; diff --git a/scripts/verify/recorded-crash/trace-ownership.mjs b/scripts/verify/recorded-crash/trace-ownership.mjs new file mode 100644 index 0000000000..0267dcdaa8 --- /dev/null +++ b/scripts/verify/recorded-crash/trace-ownership.mjs @@ -0,0 +1,127 @@ +import assert from "node:assert/strict"; + +const ownershipPsArguments = + '["ps", "-axo", "pid=,ppid=,pgid=,uid=,stat=,command="]'; + +/** + * Allow only the procfs paths used by the process-ownership scan: ps may + * inspect its process-table fields, and its caller's thread group may read + * native identity and UID records. With strace -ff, libuv worker threads get + * separate files; follow CLONE_THREAD edges, never ordinary child-process + * edges. strace cannot distinguish the purpose of a stat/status read inside + * the same Node thread group, so this is deliberately limited to those two + * files; provider Python/GDB children remain outside the permit. + */ +export const assertHistoricalPidReadsAreOwnershipOnly = ({ + traces, + sentinelPid, + psExecutables, +}) => { + const processGroups = new Map(); + const findGroup = (pid) => { + const parent = processGroups.get(pid); + if (parent === undefined) { + processGroups.set(pid, pid); + return pid; + } + if (parent === pid) return pid; + const root = findGroup(parent); + processGroups.set(pid, root); + return root; + }; + const joinThreadGroup = (left, right) => { + const leftRoot = findGroup(left); + const rightRoot = findGroup(right); + if (leftRoot !== rightRoot) processGroups.set(rightRoot, leftRoot); + }; + + const cloneEvents = []; + for (const trace of traces) { + let unfinished; + for (const rawLine of trace.text.split("\n")) { + const line = rawLine.trimStart(); + const start = /^(clone3?|fork|vfork)\((.*)$/u.exec( + line, + ); + if (start !== null) { + unfinished = { name: start[1], text: start[2] }; + continue; + } + const resumed = /^<\.\.\. (clone3?|fork|vfork) resumed>(.*)$/u.exec(line); + if (resumed !== null && resumed[1] === unfinished?.name) { + cloneEvents.push({ + parentPid: trace.pid, + text: `${unfinished.text}${resumed[2]}`, + }); + unfinished = undefined; + continue; + } + const complete = + /^(clone3?|fork|vfork)\((.*\)) = (\d+)(?:<[^>]*>)?$/u.exec(line); + if (complete !== null) + cloneEvents.push({ + parentPid: trace.pid, + childPid: Number(complete[3]), + text: complete[2], + }); + } + } + + for (const event of cloneEvents) { + const childPid = + event.childPid ?? /\) = (\d+)(?:<[^>]*>)?$/u.exec(event.text)?.[1]; + if (childPid === undefined) continue; + event.childPid = Number(childPid); + if (/\bCLONE_THREAD\b/u.test(event.text)) + joinThreadGroup(event.parentPid, event.childPid); + } + + const psPids = new Set(); + for (const trace of traces) { + const successfulPsExec = trace.text.split("\n").some((line) => { + const executable = /^execve\("([^"]+)"/u.exec(line)?.[1]; + return ( + executable !== undefined && + psExecutables.has(executable) && + line.includes(ownershipPsArguments) && + line.endsWith(" = 0") + ); + }); + if (successfulPsExec) psPids.add(trace.pid); + } + + const psParentGroups = new Set( + cloneEvents + .filter(({ childPid }) => childPid !== undefined && psPids.has(childPid)) + .map(({ parentPid }) => findGroup(parentPid)), + ); + + const ownershipReads = []; + const pidPrefix = `/proc/${sentinelPid}/`; + for (const trace of traces) { + const isPsProcess = psPids.has(trace.pid); + const isOwnershipProcessThread = psParentGroups.has(findGroup(trace.pid)); + const pidReads = trace.text + .split("\n") + .filter((line) => line.includes(pidPrefix)); + for (const line of pidReads) { + const psAllowedPath = new RegExp( + `/proc/${sentinelPid}/(?:stat|status|cmdline|wchan|environ)"`, + "u", + ).test(line); + const ownershipThreadPath = new RegExp( + `/proc/${sentinelPid}/(?:stat|status)"`, + "u", + ).test(line); + const psRead = isPsProcess && psAllowedPath; + const ownershipThreadRead = + isOwnershipProcessThread && ownershipThreadPath; + assert.ok( + psRead || ownershipThreadRead, + `Historical PID caused a provider lookup in trace.${trace.pid}: ${line}`, + ); + ownershipReads.push(line); + } + } + return ownershipReads; +}; diff --git a/scripts/verify/recorded-crash/trace.mjs b/scripts/verify/recorded-crash/trace.mjs index 7cf5835087..44ef4f9632 100644 --- a/scripts/verify/recorded-crash/trace.mjs +++ b/scripts/verify/recorded-crash/trace.mjs @@ -3,6 +3,7 @@ import { readdir, readFile, realpath } from "node:fs/promises"; import { fileURLToPath } from "node:url"; import { isAbsolute, join, resolve } from "node:path"; import { runOwnedCommand } from "../../../dist/process/OwnedCommand.js"; +import { assertHistoricalPidReadsAreOwnershipOnly } from "./trace-ownership.mjs"; /** Check attempted syscalls, including failed attempts, on a recorded PID that collides with an owned live sentinel. */ export async function verifyCoreInspectionTrace({ @@ -68,11 +69,22 @@ export async function verifyCoreInspectionTrace({ { timeoutMs: 90_000, diagnosticBytes: 1024 * 1024 }, ); const executions = []; - const ownershipReads = []; let gdbObserved = false; - for (const file of await readdir(root)) { - if (!file.startsWith(`inspection-${mode}.trace.`)) continue; - const text = await readFile(join(root, file), "utf8"); + const traces = await Promise.all( + (await readdir(root)) + .filter((file) => file.startsWith(`inspection-${mode}.trace.`)) + .map(async (file) => ({ + file, + pid: Number(file.slice(`inspection-${mode}.trace.`.length)), + text: await readFile(join(root, file), "utf8"), + })), + ); + const ownershipReads = assertHistoricalPidReadsAreOwnershipOnly({ + traces, + sentinelPid: sentinel.pid, + psExecutables: psPaths, + }); + for (const { file, text } of traces) { // Keep the source executable present: GDB 16 otherwise silently hides a // core-only violation when its automatic executable lookup finds nothing. const recordedExecutable = JSON.stringify(join(root, "fixture-program")); @@ -80,36 +92,6 @@ export async function verifyCoreInspectionTrace({ !text.split("\n").some((line) => line.includes(recordedExecutable)), `Inspection attempted access to the recorded executable: ${file}`, ); - const ownershipTrace = text - .split("\n") - .some( - (line) => - /^execve\("\/(?:usr\/)?bin\/ps",/u.test(line) && - line.includes( - '["ps", "-axo", "pid=,ppid=,pgid=,uid=,stat=,command="]', - ) && - line.endsWith(" = 0"), - ); - const pidReads = text - .split("\n") - .filter((line) => line.includes(`/proc/${sentinel.pid}/`)); - if (ownershipTrace) { - for (const line of pidReads) { - assert.ok( - new RegExp( - `/proc/${sentinel.pid}/(?:stat|status|cmdline|wchan|environ)"`, - "u", - ).test(line), - `Unexpected ownership observation of the sentinel: ${line}`, - ); - ownershipReads.push(line); - } - } else - assert.deepEqual( - pidReads, - [], - `Historical PID caused a provider lookup in ${file}: ${pidReads.join("\n")}`, - ); assert.ok( !/\bptrace\(/u.test(text), `Inspection attempted ptrace: ${file}`, diff --git a/tests/boundary/filesystem/packageSetupPlanDiagnostics.test.ts b/tests/boundary/filesystem/packageSetupPlanDiagnostics.test.ts new file mode 100644 index 0000000000..7de7c1d420 --- /dev/null +++ b/tests/boundary/filesystem/packageSetupPlanDiagnostics.test.ts @@ -0,0 +1,39 @@ +import { expect, it } from "vitest"; + +import { setupPlanFailureMessage } from "../../../scripts/verify-package-setup-plan.mjs"; + +it("preserves setup failure details when an error result has no action arrays", () => { + const result = { + status: "needs_human", + remediation: "Review the host setup requirements.", + }; + const message = setupPlanFailureMessage({ + planned: result, + plannedExecution: { status: 1 }, + plannedClaudeConfig: "unchanged", + plannedCodexConfig: "unchanged", + plannedCursorConfig: "unchanged", + }); + + expect(message).toContain("packaged setup plan was not read-only"); + expect(message).toContain('"status":"needs_human"'); + expect(message).toContain("Review the host setup requirements."); + expect(message).not.toContain("TypeError"); +}); + +it("still formats action kinds for a structured setup result", () => { + const message = setupPlanFailureMessage({ + planned: { + status: "planned", + plannedActions: [{ kind: "configure_client" }], + appliedActions: [], + }, + plannedExecution: { status: 0 }, + plannedClaudeConfig: "unchanged", + plannedCodexConfig: "unchanged", + plannedCursorConfig: "unchanged", + }); + + expect(message).toContain('"plannedKinds":["configure_client"]'); + expect(message).toContain('"appliedKinds":[]'); +}); diff --git a/tests/boundary/filesystem/recordedCrashTraceClassifier.test.ts b/tests/boundary/filesystem/recordedCrashTraceClassifier.test.ts new file mode 100644 index 0000000000..f03fbd5ba1 --- /dev/null +++ b/tests/boundary/filesystem/recordedCrashTraceClassifier.test.ts @@ -0,0 +1,144 @@ +import { expect, it } from "vitest"; + +import { assertHistoricalPidReadsAreOwnershipOnly } from "../../../scripts/verify/recorded-crash/trace-ownership.mjs"; + +const psExec = + 'execve("/usr/bin/ps", ["ps", "-axo", "pid=,ppid=,pgid=,uid=,stat=,command="], 0x7fff) = 0'; + +it("correlates the ps child trace with its parent identity read", () => { + const ownershipReads = assertHistoricalPidReadsAreOwnershipOnly({ + sentinelPid: 4008, + psExecutables: new Set(["/usr/bin/ps"]), + traces: [ + { + pid: 1200, + text: [ + "vfork() = 1201", + "clone3({flags=CLONE_VM|CLONE_SIGHAND|CLONE_THREAD}, 88) = 1202", + 'openat(AT_FDCWD, "/proc/4008/stat", O_RDONLY|O_CLOEXEC) = 21', + ].join("\n"), + }, + { + pid: 1202, + text: [ + 'openat(AT_FDCWD, "/proc/4008/stat", O_RDONLY|O_CLOEXEC) = 21', + 'openat(AT_FDCWD, "/proc/4008/status", O_RDONLY|O_CLOEXEC) = 22', + ].join("\n"), + }, + { + pid: 1201, + text: [ + psExec, + 'openat(AT_FDCWD, "/proc/4008/status", O_RDONLY) = 3', + 'openat(AT_FDCWD, "/proc/4008/cmdline", O_RDONLY) = 4', + ].join("\n"), + }, + ], + }); + + expect(ownershipReads).toHaveLength(5); +}); + +it("joins unfinished and resumed CLONE_THREAD records", () => { + const ownershipReads = assertHistoricalPidReadsAreOwnershipOnly({ + sentinelPid: 4008, + psExecutables: new Set(["/usr/bin/ps"]), + traces: [ + { + pid: 1200, + text: [ + "vfork() = 1201", + "clone3({flags=CLONE_VM|CLONE_SIGHAND|CLONE_THREAD, child_tid=0x3 ", + "<... clone3 resumed>) = 1202", + ].join("\n"), + }, + { + pid: 1202, + text: 'openat(AT_FDCWD, "/proc/4008/stat", O_RDONLY) = 3', + }, + { pid: 1201, text: psExec }, + ], + }); + + expect(ownershipReads).toHaveLength(1); +}); + +it("rejects provider-style reads outside the ps child and stat-only caller", () => { + const traces = [ + { + pid: 1200, + text: [ + "vfork() = 1201", + 'openat(AT_FDCWD, "/proc/4008/cmdline", O_RDONLY) = 21', + ].join("\n"), + }, + { pid: 1201, text: psExec }, + ]; + + expect(() => + assertHistoricalPidReadsAreOwnershipOnly({ + traces, + sentinelPid: 4008, + psExecutables: new Set(["/usr/bin/ps"]), + }), + ).toThrow("Historical PID caused a provider lookup"); +}); + +it("does not trust a ps command with different arguments or failed exec", () => { + for (const text of [ + 'execve("/usr/bin/ps", ["ps", "-ef"], 0x7fff) = 0', + psExec.replace(" = 0", " = -1 EACCES"), + ]) + expect(() => + assertHistoricalPidReadsAreOwnershipOnly({ + sentinelPid: 4008, + psExecutables: new Set(["/usr/bin/ps"]), + traces: [ + { + pid: 1200, + text: `vfork() = 1201\nopenat(AT_FDCWD, "/proc/4008/stat", O_RDONLY) = 3`, + }, + { pid: 1201, text }, + ], + }), + ).toThrow("Historical PID caused a provider lookup"); +}); + +it("allows only known procfs fields inside the exact ps child", () => { + expect(() => + assertHistoricalPidReadsAreOwnershipOnly({ + sentinelPid: 4008, + psExecutables: new Set(["/usr/bin/ps"]), + traces: [ + { pid: 1200, text: "vfork() = 1201" }, + { + pid: 1201, + text: `${psExec}\nopenat(AT_FDCWD, "/proc/4008/exe", O_RDONLY) = 3`, + }, + ], + }), + ).toThrow("Historical PID caused a provider lookup"); +}); + +it("does not grant the ownership process thread permit to a forked child", () => { + for (const field of ["stat", "status"]) + expect(() => + assertHistoricalPidReadsAreOwnershipOnly({ + sentinelPid: 4008, + psExecutables: new Set(["/usr/bin/ps"]), + traces: [ + { + pid: 1200, + text: ["vfork() = 1201", "clone3({flags=SIGCHLD}, 88) = 1203"].join( + "\n", + ), + }, + { pid: 1201, text: psExec }, + { + pid: 1203, + text: `openat(AT_FDCWD, "/proc/4008/${field}", O_RDONLY) = 3`, + }, + ], + }), + ).toThrow("Historical PID caused a provider lookup"); +});