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/recorded-crashes.md b/docs/recorded-crashes.md index 1dbc1c2f55..117b592b5f 100644 --- a/docs/recorded-crashes.md +++ b/docs/recorded-crashes.md @@ -105,11 +105,13 @@ GDB/Python/pwndbg/strace. Fixture generation explicitly launches its owned test program; subsequent inspection only reads the resulting core. Real acceptance checks recorded threads, high registers, missing/malformed notes, opaque owners, CLI/MCP parity, optional debugger context and historical PID collision with an -owned live sentinel. Observed Node processes and their traced threads may read -that live process's `stat` for the shared ownership inventory; debugger and -decoder processes cannot use this exception. A second recording uses a valid +owned live sentinel. An observed Node thread group must launch the exact ownership +`ps` inventory before it can read that live process's `stat` or `status` for native +identity/UID facts. Only traced `CLONE_THREAD` members share this allowance; +exec-replaced threads, ordinary children, debugger and decoder processes cannot +use it. A second recording uses a valid historical PID above the host's assignable PID range and rejects resolved lookups, -including Node `stat` reads, to detect historical-PID-driven access separately +including Node `stat`/`status` reads, to detect historical-PID-driven access separately from live inventory. Descriptor paths come from strace's `-yy` decoding; exact PID-directory opens and relative accesses through those handles are rejected. Failed FD-relative pathname attempts are checked across syscall variants. 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/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..c265488fac --- /dev/null +++ b/scripts/verify/recorded-crash/trace-ownership.d.mts @@ -0,0 +1,17 @@ +export interface OwnershipTraceExecution { + readonly pid: number; + readonly path: string; + readonly line: string; + readonly succeeded: boolean; + readonly ownershipInventory: boolean; +} +/** Parsed topology and execution facts for one complete strace observation. */ +export function collectOwnershipTraceFacts(options: { + readonly traces: readonly { readonly pid: number; readonly text: string }[]; + readonly nodeExecutable: string; + readonly psExecutables: ReadonlySet; +}): { + readonly ownershipThreads: ReadonlySet; + readonly psProcesses: ReadonlySet; + readonly executions: readonly OwnershipTraceExecution[]; +}; diff --git a/scripts/verify/recorded-crash/trace-ownership.mjs b/scripts/verify/recorded-crash/trace-ownership.mjs new file mode 100644 index 0000000000..2787ebe41d --- /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="]'; + +/** Parse execution/thread facts once; only an observed Node ps caller owns inventory reads. */ +export const collectOwnershipTraceFacts = ({ + traces, + nodeExecutable, + 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; + const observedChild = Number(childPid); + // A child-side clone return of zero is not a parent-to-child edge. + if (!Number.isSafeInteger(observedChild) || observedChild <= 0) continue; + event.childPid = observedChild; + if (/\bCLONE_THREAD\b/u.test(event.text)) + joinThreadGroup(event.parentPid, event.childPid); + } + + const executions = []; + for (const trace of traces) { + for (const line of trace.text.split("\n")) { + if (!/execve(?:at)?\(/u.test(line)) continue; + const match = /^execve\("([^"]+)"/u.exec(line); + assert.notEqual(match, null, `Unresolved executable identity: ${line}`); + assert.ok( + / = (?:0|-1 [A-Z]+)/u.test(line), + `Incomplete execution observation: ${line}`, + ); + executions.push({ + pid: trace.pid, + path: match[1], + line, + succeeded: line.endsWith(" = 0"), + ownershipInventory: + psExecutables.has(match[1]) && line.includes(ownershipPsArguments), + }); + } + } + const psProcesses = new Set( + executions + .filter( + ({ succeeded, ownershipInventory }) => succeeded && ownershipInventory, + ) + .map(({ pid }) => pid), + ); + const psParentGroups = new Set( + cloneEvents + .filter(({ childPid }) => psProcesses.has(childPid)) + .map(({ parentPid }) => findGroup(parentPid)), + ); + const nodeGroups = new Set( + executions + .filter(({ path, succeeded }) => succeeded && path === nodeExecutable) + .map(({ pid }) => findGroup(pid)), + ); + const replaced = new Set( + executions + .filter(({ path, succeeded }) => succeeded && path !== nodeExecutable) + .map(({ pid }) => pid), + ); + const ownershipThreads = new Set( + traces + .filter( + ({ pid }) => + nodeGroups.has(findGroup(pid)) && + psParentGroups.has(findGroup(pid)) && + !replaced.has(pid), + ) + .map(({ pid }) => pid), + ); + return { ownershipThreads, psProcesses, executions }; +}; diff --git a/scripts/verify/recorded-crash/trace-policy.d.mts b/scripts/verify/recorded-crash/trace-policy.d.mts index 15c4d96fb2..1f39deb219 100644 --- a/scripts/verify/recorded-crash/trace-policy.d.mts +++ b/scripts/verify/recorded-crash/trace-policy.d.mts @@ -7,14 +7,10 @@ export const historicalPidTraceContract: { readonly pid_syscalls: readonly string[]; readonly unverified: readonly string[]; }; -export function nodeInventoryThreads( - traces: readonly SyscallTrace[], - executable: string, -): ReadonlySet; export function assertHistoricalPidAccess(input: { readonly trace: SyscallTrace; readonly historicalPid: number; - readonly nodeThreads: ReadonlySet; + readonly ownershipThreads: ReadonlySet; readonly liveCollision: boolean; readonly psInventory: boolean; }): readonly string[]; diff --git a/scripts/verify/recorded-crash/trace-policy.mjs b/scripts/verify/recorded-crash/trace-policy.mjs index a4f39584a7..07fb5ee24a 100644 --- a/scripts/verify/recorded-crash/trace-policy.mjs +++ b/scripts/verify/recorded-crash/trace-policy.mjs @@ -1,52 +1,6 @@ import assert from "node:assert/strict"; import { posix } from "node:path"; -/** Identify REA's Node processes and their observed CLONE_THREAD children. */ -export function nodeInventoryThreads(traces, executable) { - const executesForeign = (text) => - text - .split("\n") - .some( - (line) => - /^execve\(/u.test(line) && - line.endsWith(" = 0") && - !line.startsWith(`execve(${JSON.stringify(executable)},`), - ); - const byPid = new Map(traces.map((trace) => [trace.pid, trace])); - const nodes = new Set( - traces - .filter( - ({ text }) => - !executesForeign(text) && - text - .split("\n") - .some( - (line) => - line.startsWith(`execve(${JSON.stringify(executable)},`) && - line.endsWith(" = 0"), - ), - ) - .map(({ pid }) => pid), - ); - for (const pid of nodes) { - let pendingThreadClone = false; - for (const line of byPid.get(pid)?.text.split("\n") ?? []) { - if (!/^(?:clone3?\(|<\.\.\. clone3? resumed>)/u.test(line)) continue; - if (/^clone3?\(/u.test(line)) - pendingThreadClone = /\bCLONE_THREAD\b/u.test(line); - const child = / = (\d+)$/u.exec(line); - if (pendingThreadClone && child !== null) { - const childPid = Number(child[1]); - const trace = byPid.get(childPid); - if (trace !== undefined && !executesForeign(trace.text)) - nodes.add(childPid); - } - if (!line.endsWith("")) pendingThreadClone = false; - } - } - return nodes; -} - // This owner supplies both tracer selection and target interpretation. Kinds // describe syscall argument semantics, never catalog positions or output PIDs. const pidTargetKinds = new Map([ @@ -176,7 +130,7 @@ const readsMetadata = (line, root, names) => { export function assertHistoricalPidAccess({ trace, historicalPid, - nodeThreads, + ownershipThreads, liveCollision, psInventory, }) { @@ -216,8 +170,9 @@ export function assertHistoricalPidAccess({ !referencesHistoricalPath(line) ) continue; - const nodeStat = - nodeThreads.has(trace.pid) && readsMetadata(line, root, ["stat"]); + const ownershipMetadata = + ownershipThreads.has(trace.pid) && + readsMetadata(line, root, ["stat", "status"]); const psMetadata = psInventory && (readsMetadata(line, root, [ @@ -236,7 +191,7 @@ export function assertHistoricalPidAccess({ liveCollision && target !== "historical" && target !== "unresolved" && - (nodeStat || psMetadata), + (ownershipMetadata || psMetadata), `${target === "unresolved" ? "Unresolved process target in" : "Historical PID caused a"} non-inventory lookup in trace ${trace.pid}: ${line}`, ); accesses.push(line); diff --git a/scripts/verify/recorded-crash/trace.mjs b/scripts/verify/recorded-crash/trace.mjs index 9bb23fe9a6..34735737b6 100644 --- a/scripts/verify/recorded-crash/trace.mjs +++ b/scripts/verify/recorded-crash/trace.mjs @@ -7,8 +7,8 @@ import { parseEvidence } from "../../../dist/domain/evidence.js"; import { assertHistoricalPidAccess, historicalPidTraceContract, - nodeInventoryThreads, } from "./trace-policy.mjs"; +import { collectOwnershipTraceFacts } 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({ @@ -100,11 +100,19 @@ export async function verifyCoreInspectionTrace({ // Retain only execution/thread evidence across files, not all raw traces. const inventory = text .split("\n") - .filter((line) => /^(?:execve|clone|<\.\.\. clone)/u.test(line)) + .filter((line) => + /^(?:execve|clone|fork|vfork|<\.\.\. (?:clone|fork|vfork))/u.test( + line, + ), + ) .join("\n"); traces.push({ pid, text: inventory, file }); } - const nodeThreads = nodeInventoryThreads(traces, process.execPath); + const ownershipFacts = collectOwnershipTraceFacts({ + traces, + nodeExecutable: process.execPath, + psExecutables: psPaths, + }); for (const trace of traces) { const { file } = trace; const text = await readFile(join(root, file), "utf8"); @@ -115,23 +123,13 @@ 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"), - ); ownershipReads.push( ...assertHistoricalPidAccess({ trace: { pid: trace.pid, text }, historicalPid, - nodeThreads, + ownershipThreads: ownershipFacts.ownershipThreads, liveCollision: historicalPid === sentinel.pid, - psInventory: ownershipTrace, + psInventory: ownershipFacts.psProcesses.has(trace.pid), }), ); assert.ok( @@ -148,48 +146,40 @@ export async function verifyCoreInspectionTrace({ ), `Inspection attempted an Internet socket: ${file}`, ); - for (const line of text.split("\n")) { - if (!/execve(?:at)?\(/u.test(line)) continue; - const match = /^execve\("([^"]+)"/u.exec(line); - assert.notEqual(match, null, `Unresolved executable identity: ${line}`); - assert.ok( - / = (?:0|-1 [A-Z]+)/u.test(line), - `Incomplete execution observation: ${line}`, + } + for (const execution of ownershipFacts.executions) { + const { line } = execution; + const executable = await realpath(execution.path).catch( + () => execution.path, + ); + const ownership = execution.ownershipInventory; + const iconv = + iconvPaths.has(execution.path) && line.includes('["iconv", "-l"]'); + const version = + gitPaths.has(execution.path) && + line.includes( + "[" + + [ + "git", + "--git-dir", + join(plugin, ".git"), + "rev-parse", + "--short", + "HEAD", + ] + .map((argument) => JSON.stringify(argument)) + .join(", ") + + "]", ); - const executable = await realpath(match[1]).catch(() => match[1]); - const ownership = - psPaths.has(match[1]) && - line.includes( - '["ps", "-axo", "pid=,ppid=,pgid=,uid=,stat=,command="]', - ); - const iconv = - iconvPaths.has(match[1]) && line.includes('["iconv", "-l"]'); - const version = - gitPaths.has(match[1]) && - line.includes( - "[" + - [ - "git", - "--git-dir", - join(plugin, ".git"), - "rev-parse", - "--short", - "HEAD", - ] - .map((argument) => JSON.stringify(argument)) - .join(", ") + - "]", - ); - assert.ok( - [process.execPath, python, gdb].includes(executable) || - ownership || - iconv || - version, - `Unexpected attempted host execution: ${line}`, - ); - if (executable === gdb) gdbObserved = true; - executions.push(executable); - } + assert.ok( + [process.execPath, python, gdb].includes(executable) || + ownership || + iconv || + version, + `Unexpected attempted host execution: ${line}`, + ); + if (executable === gdb) gdbObserved = true; + executions.push(executable); } assert.ok(gdbObserved, "Trace must include the actual core-only GDB stage"); assert.ok(executions.includes(python)); 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/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/domain/base64Schema.test.ts b/src/domain/base64Schema.test.ts deleted file mode 100644 index d7c2e4a155..0000000000 --- a/src/domain/base64Schema.test.ts +++ /dev/null @@ -1,71 +0,0 @@ -import { Ajv2020 } from "ajv/dist/2020.js"; -import { expect, it } from "vitest"; -import { z } from "zod"; - -import { canonicalBase64Schema } from "./base64Schema.js"; -import { CANONICAL_BASE64_PATTERN } from "./stringPatterns.js"; - -it("keeps runtime and projected canonical base64 contracts equivalent", () => { - const samples = [ - "", - "AA==", - "AAA=", - "////", - "AB==", - "AAB=", - "A", - "AA", - "AAA", - "====", - "AA==\n", - "AA==\r\n", - "AA==\u2028", - "AA==\u2029", - "AA\nAA", - "AA=\n=", - "AA-_", - "a b c", - 42, - null, - ]; - for (const io of ["input", "output"] as const) { - const projected = z.toJSONSchema(canonicalBase64Schema, { io }); - const validate = new Ajv2020({ strict: true }).compile(projected); - expect(JSON.stringify(projected)).not.toMatch(/\(\? { - const encoded = Buffer.alloc(24 * 1024 * 1024 - 3, 0x81).toString("base64"); - const projected = z.toJSONSchema(canonicalBase64Schema); - const validate = new Ajv2020({ strict: true }).compile(projected); - const samples = [ - { value: encoded, expected: true }, - { value: `${encoded.slice(0, -1)}!`, expected: false }, - { value: `${encoded.slice(0, -3)}R==`, expected: false }, - { value: `${encoded}A`, expected: false }, - ]; - for (const { value, expected } of samples) { - expect(canonicalBase64Schema.safeParse(value).success).toBe(expected); - expect(validate(value)).toBe(expected); - for (const flag of ["", "u", "v"]) { - const positive = new RegExp(projected.pattern!, flag); - const negative = new RegExp( - (projected.not as { pattern: string }).pattern, - flag, - ); - expect(positive.test(value) && !negative.test(value), flag).toBe( - expected, - ); - } - } -}); diff --git a/src/domain/base64Schema.ts b/src/domain/base64Schema.ts deleted file mode 100644 index 0bab4bdd82..0000000000 --- a/src/domain/base64Schema.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { z } from "zod"; - -import { - BASE64_QUARTETS_PATTERN, - CANONICAL_BASE64_INVALID_PATTERN, -} from "./stringPatterns.js"; - -/** Exact canonical base64 using stack-bounded, portable JSON Schema patterns. */ -export const canonicalBase64Schema = z - .string() - .regex(BASE64_QUARTETS_PATTERN) - .refine((value) => !CANONICAL_BASE64_INVALID_PATTERN.test(value), { - message: "Expected canonical padded base64", - }) - // JSON Schema negation expresses the same rejection without look-around. - .meta({ not: { pattern: CANONICAL_BASE64_INVALID_PATTERN.source } }); diff --git a/src/domain/go/goBinary.ts b/src/domain/go/goBinary.ts index fae20f85b5..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 { canonicalBase64Schema } from "../base64Schema.js"; +import { canonicalBase64Schema } from "../stringPatterns.js"; const sourceRangeSchema = z.strictObject({ offset: z.number().int().nonnegative(), diff --git a/src/domain/native/binaryLayout.ts b/src/domain/native/binaryLayout.ts index 767ecd73dd..c1a694cc2d 100644 --- a/src/domain/native/binaryLayout.ts +++ b/src/domain/native/binaryLayout.ts @@ -2,15 +2,16 @@ import { z } from "zod"; import { canBorrowParsedJsonSnapshot } from "../immutableJson.js"; import { localPathStringSchema } from "../localPath.js"; -import { canonicalBase64Schema } from "../base64Schema.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 = canonicalBase64Schema; const name = z.strictObject({ display: z.string(), - bytes_base64: canonicalBase64Schema.nullable(), + bytes_base64: canonicalBase64.nullable(), location: range.nullable(), unknown_reason: z.string().nullable(), }); @@ -150,7 +151,7 @@ const binaryLayoutObjectSchema = z.strictObject({ z.strictObject({ section_index: index, location: range, - encoded_bytes_base64: canonicalBase64Schema, + encoded_bytes_base64: canonicalBase64, entries: z.array( z.strictObject({ decoded_index: index, reported_offset: unsignedHex }), ), diff --git a/src/domain/native/recordedCrash.ts b/src/domain/native/recordedCrash.ts index 7c9e42c6c9..0163a85329 100644 --- a/src/domain/native/recordedCrash.ts +++ b/src/domain/native/recordedCrash.ts @@ -2,12 +2,13 @@ import { z } from "zod"; import type { BinaryArchitecture } from "../binaryTargetTypes.js"; import { localPathStringSchema } from "../localPath.js"; -import { canonicalBase64Schema } from "../base64Schema.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 = canonicalBase64Schema; const range = z.strictObject({ offset: hex, bytes: hex }); const diagnostics = z.strictObject({ stdout: z.string(), @@ -221,17 +222,17 @@ const recordedCrashObjectSchema = z.strictObject({ type: z.union([z.string(), index]), location: range, owner_location: range, - owner_bytes_base64: canonicalBase64Schema, + owner_bytes_base64: base64, owner_display: z.string().nullable(), descriptor_location: range, - descriptor_bytes_base64: canonicalBase64Schema, + descriptor_bytes_base64: base64, }), ), note_padding: z.array( z.strictObject({ segment_index: index, location: range, - bytes_base64: canonicalBase64Schema, + bytes_base64: base64, }), ), threads: z.array( 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..4dd634d7a7 100644 --- a/src/domain/stringPatterns.test.ts +++ b/src/domain/stringPatterns.test.ts @@ -1,10 +1,17 @@ +import { Ajv2020 } from "ajv/dist/2020.js"; 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 +50,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 +78,79 @@ 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); +}); + +it("validates complete projected canonical schemas at the encoded-input budget", () => { + const projected = z.toJSONSchema(canonicalBase64Schema, { + target: "draft-2020-12", + }); + const validate = new Ajv2020({ strict: true }).compile(projected); + const encoded = Buffer.alloc(24 * 1024 * 1024 - 3, 0x81).toString("base64"); + for (const { value, expected } of [ + { value: encoded, expected: true }, + { value: `${encoded.slice(0, -1)}!`, expected: false }, + { value: `${encoded.slice(0, -3)}R==`, expected: false }, + { value: `${encoded}A`, expected: false }, + ]) { + expect(canonicalBase64Schema.safeParse(value).success).toBe(expected); + expect(validate(value)).toBe(expected); } }); diff --git a/src/domain/stringPatterns.ts b/src/domain/stringPatterns.ts index 0a195724d3..718cc1d1d4 100644 --- a/src/domain/stringPatterns.ts +++ b/src/domain/stringPatterns.ts @@ -1,29 +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, -// especially under JSON Schema's Unicode-set mode. Check invalid content -// separately from an unconstrained fixed-width loop for length. -export const BASE64_QUARTETS_PATTERN = /^(?:....)*$/; -const PADDED_BASE64_INVALID_PATTERN = /[^A-Za-z0-9+\x2f=]|=[^=]|===/; -export const CANONICAL_BASE64_INVALID_PATTERN = new RegExp( - `${PADDED_BASE64_INVALID_PATTERN.source}|[^AQgw]==$|[^AEIMQUYcgkosw048=]=$`, -); +// 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]=$)/; -const excludingInvalidBase64 = (invalid: RegExp): RegExp => - new RegExp( - `^(?!.*(?:${invalid.source}))${BASE64_QUARTETS_PATTERN.source.slice(1)}`, - ); +export const isPaddedBase64 = (value: string): boolean => + value.length % 4 === 0 && + !INVALID_BASE64_CHARACTER_PATTERN.test(value) && + !INVALID_BASE64_PADDING_PATTERN.test(value); -/** Padded base64 syntax, allowing nonzero pad bits and the empty encoding. */ -export const PADDED_BASE64_PATTERN = excludingInvalidBase64( - PADDED_BASE64_INVALID_PATTERN, -); +export const isCanonicalBase64 = (value: string): boolean => + isPaddedBase64(value) && + !INVALID_CANONICAL_BASE64_PAD_BITS_PATTERN.test(value); -/** Canonical padded base64, including zero pad bits and the empty encoding. */ -export const CANONICAL_BASE64_PATTERN = excludingInvalidBase64( - CANONICAL_BASE64_INVALID_PATTERN, -); +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/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/process/LinuxProcessStat.test.ts b/src/process/LinuxProcessStat.test.ts index d8544528bb..d922714e1c 100644 --- a/src/process/LinuxProcessStat.test.ts +++ b/src/process/LinuxProcessStat.test.ts @@ -7,6 +7,7 @@ const statRecord = ( readonly pid?: string; readonly command?: string; readonly state?: string; + readonly flags?: string; readonly parentPid?: string; readonly processGroupId?: string; readonly sessionId?: string; @@ -20,6 +21,7 @@ const statRecord = ( options.sessionId ?? "19", ]; while (fields.length < 19) fields.push("0"); + fields[6] = options.flags ?? "0"; fields.push(options.startTime ?? "987654"); return `${options.pid ?? "42"} (${options.command ?? "worker"}) ${fields.join(" ")}`; }; @@ -56,3 +58,38 @@ it("distinguishes zombie lifecycle and rejects malformed or missing identity fie undefined, ); }); + +it.each(["R", "S", "D"])( + "retains PF_EXITING separately from terminal state %s", + (state) => { + expect( + parseLinuxProcessStat(statRecord({ state, flags: "4194316" }))?.lifecycle, + ).toBe("exiting"); + expect( + parseLinuxProcessStat(statRecord({ state, flags: "4194312" }))?.lifecycle, + ).toBe("non-zombie"); + }, +); + +it.each(["Z", "X", "x"])( + "recognizes terminal native state %s independently of flags", + (state) => { + expect( + parseLinuxProcessStat(statRecord({ state, flags: "invalid" }))?.lifecycle, + ).toBe(state === "Z" ? "zombie" : "dead"); + }, +); + +it.each(["invalid", "-1", "4.1", "9007199254740992"])( + "preserves identity and relations with unknown flags %s", + (flags) => { + expect(parseLinuxProcessStat(statRecord({ flags }))).toEqual({ + pid: 42, + startTime: "987654", + lifecycle: undefined, + parentPid: 17, + processGroupId: 18, + sessionId: 19, + }); + }, +); diff --git a/src/process/LinuxProcessStat.ts b/src/process/LinuxProcessStat.ts index 93e56647c1..d255b74936 100644 --- a/src/process/LinuxProcessStat.ts +++ b/src/process/LinuxProcessStat.ts @@ -1,8 +1,10 @@ +import type { ProcessLifecycle } from "./ProcessLifecycle.js"; + /** Native process facts parsed from one Linux `/proc//stat` record. */ export interface LinuxProcessStat { readonly pid: number; readonly startTime: string; - readonly lifecycle: "non-zombie" | "zombie"; + readonly lifecycle: ProcessLifecycle | undefined; readonly parentPid: number; readonly processGroupId: number; readonly sessionId: number; @@ -58,10 +60,28 @@ export const parseLinuxProcessStat = ( ) return undefined; + // Field 9 is task flags. PF_EXITING can precede state Z and procfs token + // access can already fail during that interval. Neither proves terminal exit. + const flagsText = fields[6]; + const flags = + flagsText !== undefined && /^\d+$/u.test(flagsText) + ? Number(flagsText) + : Number.NaN; + const lifecycle: ProcessLifecycle | undefined = + state === "Z" + ? "zombie" + : state === "X" || state === "x" + ? "dead" + : !Number.isSafeInteger(flags) + ? undefined + : (flags & 0x00000004) !== 0 + ? "exiting" + : "non-zombie"; + return { pid, startTime, - lifecycle: state === "Z" ? "zombie" : "non-zombie", + lifecycle, parentPid, processGroupId, sessionId, 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/ProcessLifecycle.ts b/src/process/ProcessLifecycle.ts new file mode 100644 index 0000000000..025a845786 --- /dev/null +++ b/src/process/ProcessLifecycle.ts @@ -0,0 +1,30 @@ +import type { ProcessIdentityObservation } from "./ProcessOwnership.js"; + +/** Native lifecycle is independent of ownership, token readability and ps text. */ +export type ProcessLifecycle = "non-zombie" | "exiting" | "zombie" | "dead"; + +export type ProcessLifecycleObservation = + | { readonly state: "observed"; readonly lifecycle: ProcessLifecycle } + | { readonly state: "unavailable"; readonly reason: string }; + +export const PROCESS_CLEANUP_VERIFICATION_GRACE_MS = 1_000; +export const PROCESS_CLEANUP_VERIFICATION_INTERVAL_MS = 25; + +export const processIdentityHasExited = ( + identity: ProcessIdentityObservation | undefined, +): boolean => + identity?.state === "absent" || + (identity?.state === "readable" && + identity.lifecycle?.state === "observed" && + (identity.lifecycle.lifecycle === "zombie" || + identity.lifecycle.lifecycle === "dead")); + +export const processIdentityIsExiting = ( + identity: ProcessIdentityObservation | undefined, +): identity is Extract< + ProcessIdentityObservation, + { readonly state: "readable" } +> => + identity?.state === "readable" && + identity.lifecycle?.state === "observed" && + identity.lifecycle.lifecycle === "exiting"; diff --git a/src/process/ProcessOwnership.exiting.test.ts b/src/process/ProcessOwnership.exiting.test.ts new file mode 100644 index 0000000000..f9cfff3d53 --- /dev/null +++ b/src/process/ProcessOwnership.exiting.test.ts @@ -0,0 +1,178 @@ +import { expect, it, vi } from "vitest"; +import { + cleanupOwnedProcessGroup, + type ProcessIdentityObservation, + type ProcessOwnershipHost, + type ProcessTableEntry, +} from "./ProcessOwnership.js"; +import { + readProcessOwnershipFacts, + settleExitingProcessIdentities, +} from "./ProcessOwnershipObservation.js"; +import { processIdentityHasExited } from "./ProcessLifecycle.js"; + +const row: ProcessTableEntry = { + pid: 910, + parentPid: 17, + processGroupId: 910, + state: "R", + command: "[MainThread]", +}; +const identity = ( + lifecycle: "non-zombie" | "exiting" | "zombie" | "dead", + generation = "start:1", +): ProcessIdentityObservation => ({ + state: "readable", + identity: generation, + lifecycle: { state: "observed", lifecycle }, + relation: { + state: "observed", + parentPid: row.parentPid, + processGroupId: row.processGroupId, + }, +}); +const hostWith = ( + observe: () => ProcessIdentityObservation, +): ProcessOwnershipHost => ({ + listProcesses: vi.fn(async () => [row]), + environment: async () => ({}), + processIdentities: vi.fn( + async (rows) => new Map(rows.map(({ pid }) => [pid, observe()])), + ), + runTokens: vi.fn( + async (rows) => + new Map( + rows.map(({ pid }) => [ + pid, + { state: "unavailable" as const, reason: "EACCES" }, + ]), + ), + ), + signalGroup: vi.fn(), +}); + +it.each(["zombie", "dead", "absent"] as const)( + "settles exiting token reads only after a fresh native %s observation", + async (terminal) => { + let reads = 0; + const host = hostWith(() => + ++reads < 3 + ? identity("exiting") + : terminal === "absent" + ? { state: "absent", reason: "not-in-snapshot" } + : identity(terminal), + ); + const facts = await readProcessOwnershipFacts([row], host); + expect(processIdentityHasExited(facts[0]?.identityAfter)).toBe(true); + expect(facts[0]?.token).toEqual({ state: "unavailable", reason: "EACCES" }); + expect(host.processIdentities).toHaveBeenCalledTimes(3); + expect(host.runTokens).toHaveBeenCalledTimes(1); + expect(host.listProcesses).not.toHaveBeenCalled(); + expect(host.signalGroup).not.toHaveBeenCalled(); + }, +); + +it("keeps unconfirmed exit explicit at the bounded deadline", async () => { + const host = hostWith(() => identity("exiting")); + const result = await cleanupOwnedProcessGroup( + { + runId: "run", + leaderPid: row.pid, + processGroupId: row.processGroupId, + expectedCommand: "/exact/node", + }, + host, + ); + expect(result).toMatchObject({ + cleaned: false, + reason: "owned launcher exit could not be confirmed", + }); + expect(host.signalGroup).not.toHaveBeenCalled(); +}); + +it("settles launcher exit before comparing a stale command display", async () => { + let reads = 0; + const host = hostWith(() => + ++reads === 1 ? identity("exiting") : identity("zombie"), + ); + await expect( + cleanupOwnedProcessGroup( + { + runId: "run", + leaderPid: row.pid, + processGroupId: row.processGroupId, + expectedCommand: "/exact/node", + }, + host, + ), + ).resolves.toMatchObject({ cleaned: true, signaled: false }); + expect(host.signalGroup).not.toHaveBeenCalled(); +}); + +it("stops at PID reuse and preserves the changed generation", async () => { + let reads = 0; + const host = hostWith(() => + ++reads < 3 ? identity("exiting") : identity("exiting", "start:2"), + ); + const facts = await readProcessOwnershipFacts([row], host); + expect(facts[0]).toMatchObject({ + state: "replaced", + identityBefore: { identity: "start:1" }, + identityAfter: { identity: "start:2" }, + }); + expect(host.processIdentities).toHaveBeenCalledTimes(3); + expect(host.signalGroup).not.toHaveBeenCalled(); +}); + +it("preserves unknown exit evidence without inferring terminal absence", async () => { + const host = hostWith(() => ({ + state: "unavailable", + reason: "native identity denied", + })); + const result = await settleExitingProcessIdentities( + [row], + host, + new Map([[row.pid, identity("exiting")]]), + ); + expect(result.get(row.pid)).toEqual({ + state: "unavailable", + reason: "native identity denied", + }); + expect(processIdentityHasExited(result.get(row.pid))).toBe(false); +}); + +it("refreshes only selected exiting PIDs and preserves other native observations", async () => { + const other = { ...row, pid: 911 }; + const host = hostWith(() => identity("zombie")); + const initial = new Map([ + [row.pid, identity("exiting")], + [other.pid, identity("non-zombie")], + ]); + const result = await settleExitingProcessIdentities( + [row, other], + host, + initial, + ); + expect(host.processIdentities).toHaveBeenCalledExactlyOnceWith( + [row], + undefined, + undefined, + ); + expect(result.get(other.pid)).toBe(initial.get(other.pid)); + expect(host.listProcesses).not.toHaveBeenCalled(); +}); + +it("preserves caller cancellation during native exit settlement", async () => { + const abort = new AbortController(); + const reason = new Error("caller cancelled exit observation"); + const host = hostWith(() => identity("exiting")); + const pending = settleExitingProcessIdentities( + [row], + host, + new Map([[row.pid, identity("exiting")]]), + abort.signal, + ); + abort.abort(reason); + await expect(pending).rejects.toBe(reason); + expect(host.processIdentities).not.toHaveBeenCalled(); +}); diff --git a/src/process/ProcessOwnership.identity.test.ts b/src/process/ProcessOwnership.identity.test.ts index a9d83285c1..724edbba85 100644 --- a/src/process/ProcessOwnership.identity.test.ts +++ b/src/process/ProcessOwnership.identity.test.ts @@ -113,7 +113,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), @@ -124,8 +128,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({ @@ -191,9 +196,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) => { @@ -213,7 +220,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) => @@ -242,7 +249,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 888cf88996..95a80b22dc 100644 --- a/src/process/ProcessOwnership.ts +++ b/src/process/ProcessOwnership.ts @@ -7,6 +7,7 @@ import { errorMessage, readNativeProcessIdentityMap, readProcessOwnershipFacts, + settleExitingProcessIdentities, systemProcessOwnershipHost as systemHost, } from "./ProcessOwnershipObservation.js"; import { launcherIdentityFailure } from "./ProcessOwnershipIdentity.js"; @@ -15,10 +16,15 @@ import { readProcessRunTokens } from "./ProcessRunTokenObservations.js"; import { compareUnicodeCodePoints } from "../domain/unicodeCodePointOrder.js"; import { ProcessOwnershipEvidenceOwner } from "./ProcessOwnershipEvidence.js"; -const execFileAsync = promisify(execFile); +import { + PROCESS_CLEANUP_VERIFICATION_GRACE_MS, + PROCESS_CLEANUP_VERIFICATION_INTERVAL_MS, + processIdentityHasExited, + processIdentityIsExiting, + type ProcessLifecycleObservation, +} from "./ProcessLifecycle.js"; -const PROCESS_CLEANUP_VERIFICATION_GRACE_MS = 1_000; -const PROCESS_CLEANUP_VERIFICATION_INTERVAL_MS = 25; +const execFileAsync = promisify(execFile); /** Shared fields for a process group acquired by a resource owner. */ interface OwnedProcessGroupFacts { @@ -99,13 +105,7 @@ export type ProcessIdentityObservation = } | { readonly state: "unavailable"; readonly reason: string }; -/** Native lifecycle fact observed alongside (but independent of) start identity. */ -export type ProcessLifecycleObservation = - | { - readonly state: "observed"; - readonly lifecycle: "non-zombie" | "zombie"; - } - | { readonly state: "unavailable"; readonly reason: string }; +export type { ProcessLifecycleObservation } from "./ProcessLifecycle.js"; /** Parent and process-group membership from the same native identity read. */ export type ProcessNativeRelationObservation = @@ -180,6 +180,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; @@ -524,12 +526,7 @@ const readProcessRelationSnapshot = async ( const failures: ProcessOwnershipValidationFailure[] = []; for (const process of processes) { const identity = identities.get(process.pid); - if ( - identity?.state === "readable" && - identity.lifecycle?.state === "observed" && - identity.lifecycle.lifecycle === "zombie" - ) - continue; + if (processIdentityHasExited(identity)) continue; if ( identity?.state === "readable" && identity.relation?.state === "observed" @@ -552,8 +549,8 @@ const readProcessRelationSnapshot = async ( }; // A process-table read can race exit and retain the old state with an altered -// command. Only a fresh table proving that PID is no longer live can settle -// that mismatch; surviving group members still require normal token checks. +// command. Native terminal facts can settle that mismatch; exit-in-progress +// must be confirmed first. Surviving members still require normal token checks. const settleLauncherExit = async ( ownership: OwnedProcessGroup, processes: readonly ProcessTableEntry[], @@ -566,15 +563,32 @@ const settleLauncherExit = async ( if (launcher === undefined) return processes; let identityCheckedLauncher = launcher; if (host.processIdentities !== undefined) { - const identities = await readNativeProcessIdentityMap([launcher], host); + const initialIdentities = await readNativeProcessIdentityMap( + [launcher], + host, + ); + const identities = await settleExitingProcessIdentities( + [launcher], + host, + initialIdentities, + ); const identity = identities.get(launcher.pid); - if (identity?.state === "absent") - return processes.filter(({ pid }) => pid !== launcher.pid); + const initialIdentity = initialIdentities.get(launcher.pid); if ( + initialIdentity?.state === "readable" && identity?.state === "readable" && - identity.lifecycle?.state === "observed" && - identity.lifecycle.lifecycle === "zombie" + initialIdentity.identity !== identity.identity ) + return { + cleaned: false, + reason: "owned launcher identity changed while verifying exit", + }; + if (processIdentityIsExiting(identity)) + return { + cleaned: false, + reason: "owned launcher exit could not be confirmed", + }; + if (processIdentityHasExited(identity)) return processes.filter(({ pid }) => pid !== launcher.pid); if ( identity?.state !== "readable" || @@ -973,13 +987,16 @@ 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" - ) + if (processIdentityHasExited(fact.identityAfter)) continue; + if (processIdentityIsExiting(fact.identityAfter)) { + failures.push({ + pid: fact.process.pid, + reason: "process-identity-unavailable", + diagnostic: + "native process exit could not be confirmed before signal", + }); continue; + } const relation = fact.identityAfter.state === "readable" && fact.identityAfter.relation?.state === "observed" @@ -1291,9 +1308,8 @@ const scanWithOwnershipEvidence = async ( const before = baseline.get(process.pid); const now = firstIdentities.get(process.pid); if (before === null || now?.state !== "readable") { - // Baseline identity only selects rows for current validation. Let the - // owning fact pass distinguish confirmed absence, a current foreign - // generation and unresolved identity; a baseline miss is not a failure. + // The baseline selects candidates; current native facts and retained + // ownership evidence decide identity, liveness and applicability. return true; } return now.identity !== before; @@ -1310,19 +1326,17 @@ 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) && + !processIdentityIsExiting(identityAfter), + ) + .map(({ process }) => process); + if (retryRows.length === 0) break; await delay(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); @@ -1350,16 +1364,18 @@ const scanWithOwnershipEvidence = async ( 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; 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", @@ -1378,7 +1394,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/ProcessOwnershipEvidence.ts b/src/process/ProcessOwnershipEvidence.ts index 7f35d93a02..fd93b78042 100644 --- a/src/process/ProcessOwnershipEvidence.ts +++ b/src/process/ProcessOwnershipEvidence.ts @@ -1,3 +1,4 @@ +import { processIdentityHasExited } from "./ProcessLifecycle.js"; import type { ProcessIdentityObservation, ProcessNativeRelationObservation, @@ -208,9 +209,7 @@ export class ProcessOwnershipEvidenceOwner { fact.identityAfter.state === "readable" && fact.identityAfter.identity !== previous.identity) || (fact.state === "same-generation" && - fact.identityAfter.state === "readable" && - fact.identityAfter.lifecycle?.state === "observed" && - fact.identityAfter.lifecycle.lifecycle === "zombie") + processIdentityHasExited(fact.identityAfter)) ) { this.#candidates.delete(pid); retiredPids.add(pid); @@ -274,9 +273,7 @@ export class ProcessOwnershipEvidenceOwner { (retiredPids.has(pid) && fact.state !== "same-generation") || fact.state === "absent" || (fact.state === "same-generation" && - fact.identityAfter.state === "readable" && - fact.identityAfter.lifecycle?.state === "observed" && - fact.identityAfter.lifecycle.lifecycle === "zombie") + processIdentityHasExited(fact.identityAfter)) ) continue; const identity = fact.identityAfter; diff --git a/src/process/ProcessOwnershipObservation.ts b/src/process/ProcessOwnershipObservation.ts index 592833855d..ec1ae9ddb6 100644 --- a/src/process/ProcessOwnershipObservation.ts +++ b/src/process/ProcessOwnershipObservation.ts @@ -1,5 +1,14 @@ import { readFile } from "node:fs/promises"; +import { performance } from "node:perf_hooks"; +import { setTimeout as delay } from "node:timers/promises"; +import { + processIdentityHasExited, + processIdentityIsExiting, + PROCESS_CLEANUP_VERIFICATION_GRACE_MS, + PROCESS_CLEANUP_VERIFICATION_INTERVAL_MS, +} from "./ProcessLifecycle.js"; import { parseLinuxProcessStat } from "./LinuxProcessStat.js"; +import { parseLinuxProcessEffectiveUid } from "./LinuxProcessStatus.js"; import { AnalysisCapabilityUnavailableError } from "../domain/analysisErrorCore.js"; import { launcherIdentityFailure } from "./ProcessOwnershipIdentity.js"; @@ -62,11 +71,7 @@ export const observeOwnedProcessGroups = async ( if (retainedPids.has(process.pid)) return true; const identity = firstIdentities.get(process.pid); if (identity?.state === "readable") { - if ( - identity.lifecycle?.state === "observed" && - identity.lifecycle.lifecycle === "zombie" - ) - return false; + if (processIdentityHasExited(identity)) return false; if ( identity.relation?.state === "observed" && groupIds.includes(identity.relation.processGroupId) @@ -115,12 +120,7 @@ export const observeOwnedProcessGroups = async ( fact.identityAfter.relation?.state === "observed" ? fact.identityAfter.relation.processGroupId : undefined; - if ( - fact.identityAfter.state === "readable" && - fact.identityAfter.lifecycle?.state === "observed" && - fact.identityAfter.lifecycle.lifecycle === "zombie" - ) - continue; + if (processIdentityHasExited(fact.identityAfter)) continue; const candidateGroups = groupIds.filter( (groupId) => nativeGroupId === groupId || @@ -300,10 +300,7 @@ const observeNativeOwnedProcessLineage = async ( ownership, `owned launcher identity could not be read: ${launcherIdentity?.reason ?? "identity observation was omitted"}`, ); - if ( - launcherIdentity.lifecycle?.state === "observed" && - launcherIdentity.lifecycle.lifecycle === "zombie" - ) + if (processIdentityHasExited(launcherIdentity)) return unavailableLineage(ownership, "owned launcher is not live"); const nativeLauncher = processWithNativeRelation(launcher, launcherIdentity); if (nativeLauncher === undefined) @@ -317,12 +314,7 @@ const observeNativeOwnedProcessLineage = async ( const nativeProcesses = processes.flatMap((process) => { const identity = initialIdentities.get(process.pid); - if ( - identity?.state === "readable" && - identity.lifecycle?.state === "observed" && - identity.lifecycle.lifecycle === "zombie" - ) - return []; + if (processIdentityHasExited(identity)) return []; const nativeProcess = identity?.state === "readable" ? processWithNativeRelation(process, identity) @@ -370,11 +362,7 @@ const observeNativeOwnedProcessLineage = async ( ownership, `process identity changed or became unavailable during lineage validation for PID ${String(member.pid)}`, ); - if ( - fact.identityAfter.state === "readable" && - fact.identityAfter.lifecycle?.state === "observed" && - fact.identityAfter.lifecycle.lifecycle === "zombie" - ) { + if (processIdentityHasExited(fact.identityAfter)) { if (member.pid === ownership.leaderPid) return unavailableLineage( ownership, @@ -498,6 +486,7 @@ const readIdentityFacts = async ( host: ProcessOwnershipHost, phase: "before" | "after", signal?: AbortSignal, + effectiveUidPids?: ReadonlySet, ): Promise> => { signal?.throwIfAborted(); if (host.processIdentities === undefined) @@ -511,10 +500,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)]), @@ -539,6 +529,55 @@ export const readNativeProcessIdentityMap = async ( ): Promise> => readIdentityFacts(processes, host, "before", signal); +/** + * Wait only for positively observed native exit-in-progress. Never infer exit + * from EACCES or ps text, enumerate the process table, or follow a reused PID. + * A deadline, unavailable read or changed generation remains explicit evidence. + */ +export const settleExitingProcessIdentities = async ( + processes: readonly ProcessTableEntry[], + host: ProcessOwnershipHost, + initial: ReadonlyMap, + signal?: AbortSignal, +): Promise> => { + signal?.throwIfAborted(); + let pending = processes.filter(({ pid }) => + processIdentityIsExiting(initial.get(pid)), + ); + if (pending.length === 0) return initial; + const identities = new Map(initial); + const deadline = performance.now() + PROCESS_CLEANUP_VERIFICATION_GRACE_MS; + while (pending.length > 0) { + const remaining = deadline - performance.now(); + if (remaining <= 0) break; + try { + await delay( + Math.min(PROCESS_CLEANUP_VERIFICATION_INTERVAL_MS, remaining), + undefined, + signal === undefined ? undefined : { signal }, + ); + } catch (cause: unknown) { + signal?.throwIfAborted(); + throw cause; + } + const refreshed = await readNativeProcessIdentityMap(pending, host, signal); + for (const { pid } of pending) + identities.set(pid, identityFact(refreshed, pid, "after")); + pending = pending.filter(({ pid }) => { + const first = initial.get(pid); + const current = identities.get(pid); + return ( + first?.state === "readable" && + current?.state === "readable" && + processIdentityIsExiting(current) && + current.identity === first.identity + ); + }); + } + signal?.throwIfAborted(); + return identities; +}; + /** * Read identity, token, then identity for caller-provided rows. This owner * never enumerates the process table; a token is tied to a generation only @@ -583,10 +622,15 @@ export const readProcessOwnershipFacts = async ( } signal?.throwIfAborted(); - const identityAfter = await readIdentityFacts( + const effectiveUidPids = new Set( + processes + .filter(({ pid }) => tokens.get(pid)?.state !== "readable") + .map(({ pid }) => pid), + ); + const identityAfter = await settleExitingProcessIdentities( processes, host, - "after", + await readIdentityFacts(processes, host, "after", signal, effectiveUidPids), signal, ); signal?.throwIfAborted(); @@ -699,7 +743,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 +760,84 @@ 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, + 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: + facts.lifecycle === undefined + ? { + state: "unavailable" as const, + reason: "malformed_proc_flags", + } + : { + state: "observed" as const, + lifecycle: facts.lifecycle, }, - relation: { - state: "observed", - parentPid: parsed.parentPid, - processGroupId: parsed.processGroupId, - }, - } as const), - ] as const; + 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 ( 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/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..4a4e72b958 --- /dev/null +++ b/tests/boundary/filesystem/recordedCrashTraceClassifier.test.ts @@ -0,0 +1,171 @@ +import { expect, it } from "vitest"; + +import { collectOwnershipTraceFacts } from "../../../scripts/verify/recorded-crash/trace-ownership.mjs"; +import { assertHistoricalPidAccess } from "../../../scripts/verify/recorded-crash/trace-policy.mjs"; + +const node = "/fixture/node"; +const nodeExec = 'execve("/fixture/node", ["node", "rea.mjs"], 0x7fff) = 0'; +const psExec = + 'execve("/usr/bin/ps", ["ps", "-axo", "pid=,ppid=,pgid=,uid=,stat=,command="], 0x7fff) = 0'; +const parent = (...events: readonly string[]) => ({ + pid: 1200, + text: [nodeExec, ...events].join("\n"), +}); +const check = ( + traces: readonly { readonly pid: number; readonly text: string }[], +) => { + const facts = collectOwnershipTraceFacts({ + traces, + nodeExecutable: node, + psExecutables: new Set(["/usr/bin/ps"]), + }); + return traces.flatMap((trace) => + assertHistoricalPidAccess({ + trace, + historicalPid: 4008, + ownershipThreads: facts.ownershipThreads, + liveCollision: true, + psInventory: facts.psProcesses.has(trace.pid), + }), + ); +}; + +it("correlates the ps child trace with its parent identity read", () => { + expect( + check([ + parent( + "vfork() = 1201", + "clone3({flags=CLONE_VM|CLONE_SIGHAND|CLONE_THREAD}, 88) = 1202", + 'openat(AT_FDCWD, "/proc/4008/stat", O_RDONLY|O_CLOEXEC) = 21', + ), + { + 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"), + }, + ]), + ).toHaveLength(5); +}); + +it("joins unfinished and resumed CLONE_THREAD records", () => { + expect( + check([ + parent( + "vfork() = 1201", + "clone3({flags=CLONE_VM|CLONE_SIGHAND|CLONE_THREAD, child_tid=0x3 ", + "<... clone3 resumed>) = 1202", + ), + { pid: 1202, text: 'openat(AT_FDCWD, "/proc/4008/stat", O_RDONLY) = 3' }, + { pid: 1201, text: psExec }, + ]), + ).toHaveLength(1); +}); + +it("rejects provider-style reads outside the ps child and identity-only caller", () => { + expect(() => + check([ + parent( + "vfork() = 1201", + 'openat(AT_FDCWD, "/proc/4008/cmdline", O_RDONLY) = 21', + ), + { pid: 1201, text: psExec }, + ]), + ).toThrow("non-inventory 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(() => + check([ + parent( + "vfork() = 1201", + 'openat(AT_FDCWD, "/proc/4008/stat", O_RDONLY) = 3', + ), + { pid: 1201, text }, + ]), + ).toThrow("non-inventory lookup"); +}); + +it("allows only known procfs fields inside the exact ps child", () => { + expect(() => + check([ + parent("vfork() = 1201"), + { + pid: 1201, + text: `${psExec}\nopenat(AT_FDCWD, "/proc/4008/exe", O_RDONLY) = 3`, + }, + ]), + ).toThrow("non-inventory lookup"); +}); + +it("does not grant the ownership process thread permit to a forked child", () => { + for (const field of ["stat", "status"]) + expect(() => + check([ + parent("vfork() = 1201", "clone3({flags=SIGCHLD}, 88) = 1203"), + { pid: 1201, text: psExec }, + { + pid: 1203, + text: `openat(AT_FDCWD, "/proc/4008/${field}", O_RDONLY) = 3`, + }, + ]), + ).toThrow("non-inventory lookup"); +}); + +it("requires an observed Node execution as well as the inventory ps child", () => { + expect(() => + check([ + { + pid: 1200, + text: 'vfork() = 1201\nopenat(AT_FDCWD, "/proc/4008/stat", O_RDONLY) = 3', + }, + { pid: 1201, text: psExec }, + ]), + ).toThrow("non-inventory lookup"); +}); + +it("refuses identity reads in an exec-replaced ownership thread", () => { + expect(() => + check([ + parent( + "vfork() = 1201", + "clone3({flags=CLONE_VM|CLONE_THREAD}, 88) = 1202", + ), + { pid: 1201, text: psExec }, + { + pid: 1202, + text: 'execve("/fixture/gdb", ["gdb"], 0x7fff) = 0\nopenat(AT_FDCWD, "/proc/4008/stat", O_RDONLY) = 3', + }, + ]), + ).toThrow("non-inventory lookup"); +}); + +it("does not join unrelated groups through child-side clone returns", () => { + expect(() => + check([ + parent("vfork() = 1201", "clone3({flags=CLONE_VM|CLONE_THREAD}, 88) = 0"), + { pid: 1201, text: psExec }, + { + pid: 1300, + text: [ + nodeExec, + "clone3({flags=CLONE_VM|CLONE_THREAD}, 88) = 0", + 'openat(AT_FDCWD, "/proc/4008/stat", O_RDONLY) = 3', + ].join("\n"), + }, + ]), + ).toThrow("non-inventory lookup"); +}); 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", + ), + ); }); 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/jadxIntegration.test.ts b/tests/boundary/process/jadxIntegration.test.ts index e620fb13c8..3e452f8679 100644 --- a/tests/boundary/process/jadxIntegration.test.ts +++ b/tests/boundary/process/jadxIntegration.test.ts @@ -917,9 +917,21 @@ it("retains typed memory failure through failed cleanup and restores it after re "heap-failure", () => fails, ); + const originalRootClose = PrivateRuntimeRoot.prototype.close; + const workspaceCloseCause = new Error("fixture workspace close failure"); + let workspaceCloseFails = true; + let workspaceCloseAttempts = 0; const rootClose = vi .spyOn(PrivateRuntimeRoot.prototype, "close") - .mockRejectedValueOnce(new Error("fixture workspace close failure")); + .mockImplementation(function (this: PrivateRuntimeRoot) { + // Independent native helpers own their own roots. Keep the acquired + // Android workspace's failure active until the test explicitly clears it. + if (this.path === launches[0]?.cwd) { + workspaceCloseAttempts += 1; + if (workspaceCloseFails) return Promise.reject(workspaceCloseCause); + } + return originalRootClose.call(this); + }); try { const result = await service.execute("inspect_android_package", { path: apk, @@ -935,11 +947,14 @@ it("retains typed memory failure through failed cleanup and restores it after re }); const workspace = launches[0]?.cwd; expect(failure.cleanupResources).toContain(workspace); + expect(workspaceCloseAttempts).toBe(0); fails = false; await expect(provider.close()).rejects.toMatchObject({ cleanupIncomplete: true, }); expect(failure.cleanupResources).toEqual([workspace]); + expect(workspaceCloseAttempts).toBeGreaterThan(0); + workspaceCloseFails = false; await provider.close(); expect(failure.cleanupIncomplete).toBe(false); expect(failure.cleanupResources).toEqual([]); @@ -955,6 +970,7 @@ it("retains typed memory failure through failed cleanup and restores it after re await verifyCleanup(launches); } finally { rootClose.mockRestore(); + workspaceCloseFails = false; fails = false; await provider.close(); } 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/boundary/process/recordedCrashTracePolicy.test.ts b/tests/boundary/process/recordedCrashTracePolicy.test.ts index 2e317253e0..69db11fc48 100644 --- a/tests/boundary/process/recordedCrashTracePolicy.test.ts +++ b/tests/boundary/process/recordedCrashTracePolicy.test.ts @@ -1,35 +1,18 @@ import { expect, it } from "vitest"; -import { - assertHistoricalPidAccess, - nodeInventoryThreads, -} from "../../../scripts/verify/recorded-crash/trace-policy.mjs"; +import { assertHistoricalPidAccess } from "../../../scripts/verify/recorded-crash/trace-policy.mjs"; -const node = "/fixture/node"; const stat = 'openat(AT_FDCWD, "/proc/900/stat", O_RDONLY|O_CLOEXEC) = 22'; -const traces = [ - { - pid: 100, - text: [ - 'execve("/fixture/node", ["node", "rea.mjs"], 0x1 /* 3 vars */) = 0', - "clone3({flags=CLONE_VM|CLONE_THREAD|CLONE_SIGHAND}, 88) = 101", - "clone(child_stack=NULL, flags=SIGCHLD) = 102", - ].join("\n"), - }, - { pid: 101, text: stat }, - { pid: 102, text: stat }, -]; const check = (pid: number, text: string, liveCollision = true) => assertHistoricalPidAccess({ trace: { pid, text }, historicalPid: 900, - nodeThreads: nodeInventoryThreads(traces, node), + ownershipThreads: new Set([100, 101]), liveCollision, psInventory: false, }); -it("allows a live collision's stat read only in an observed Node process or thread", () => { - expect([...nodeInventoryThreads(traces, node)]).toEqual([100, 101]); +it("allows stat reads only in the supplied ownership thread set", () => { expect(check(100, stat)).toEqual([stat]); expect(check(101, stat)).toEqual([stat]); expect(() => check(102, stat)).toThrow("non-inventory lookup"); @@ -51,7 +34,7 @@ it("permits procps UID metadata without granting a PID directory handle", () => const input = { trace: { pid: 102, text: metadata }, historicalPid: 900, - nodeThreads: new Set(), + ownershipThreads: new Set(), liveCollision: true, psInventory: true, }; @@ -119,7 +102,7 @@ it("keeps descriptor reuse and distinct PIDs separate and refuses ps writes", () assertHistoricalPidAccess({ trace: { pid: 102, text: stat.replace("O_RDONLY", "O_WRONLY") }, historicalPid: 900, - nodeThreads: new Set(), + ownershipThreads: new Set(), liveCollision: true, psInventory: true, }), @@ -152,22 +135,6 @@ it("keeps native priority UID selectors and distinct PIDs separate", () => { expect(check(101, "getpriority(PRIO_PROCESS, 9000) = 20")).toEqual([]); }); -it("keeps exec-replaced threads foreign and follows acknowledged unfinished thread clones", () => { - const observed = [ - { - pid: 100, - text: [ - traces[0]!.text, - "clone3({flags=CLONE_VM|CLONE_THREAD|CLONE_SIGHAND}, 88 ", - "<... clone3 resumed>) = 103", - ].join("\n"), - }, - { pid: 101, text: 'execve("/fixture/gdb", ["gdb"], 0x1) = 0\n' + stat }, - { pid: 103, text: stat }, - ]; - expect([...nodeInventoryThreads(observed, node)]).toEqual([100, 103]); -}); - it.each([ String.raw`openat(AT_FDCWD, "/fixture/\345\267\245\345\205\267", O_RDONLY) = -1 ENOENT (No such file or directory)`, String.raw`openat(5, "\345\267\245\345\205\267", O_RDONLY) = -1 ENOENT (No such file or directory)`, 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 }; +};