diff --git a/.dev/docs/mcp-sdk-v2-changes.md b/.dev/docs/mcp-sdk-v2-changes.md new file mode 100644 index 000000000..2e81bb032 --- /dev/null +++ b/.dev/docs/mcp-sdk-v2-changes.md @@ -0,0 +1,133 @@ +# MCP SDK v2 and revision `2026-07-28`: what changed + +`apps/mcp-server` moved from `@modelcontextprotocol/sdk@1` to `@modelcontextprotocol/{server,node}@2` +and now speaks protocol revision `2026-07-28` only. The tool, resource and prompt surface is +unchanged. What changed is everything underneath it: the revision removes protocol sessions, the +`initialize` handshake, and the server-to-client request channel, which between them were most of +what the old transport and the confirmation flow were built on. + +This is the short read. The decisions, the alternatives rejected, and the probe results that +corrected several of them are in the [upgrade plan](./mcp-sdk-v2-upgrade-plan.md). + +## One revision, no fallback + +The endpoint serves `2026-07-28` and refuses everything else. A 2025-era client gets an +unsupported-protocol-version error rather than a degraded session. + +This is not conservatism. Serving both eras was investigated and does not work here: the SDK's +legacy shim, which is what would have carried confirmation to an old client, consults capabilities +declared at `initialize`, and per-request stateless serving never sees one. The 2025 path would have +given us four working tools and an `execute_query` that cannot ask for confirmation, which is worse +than a clean refusal. + +The catch for consumers is that **every SDK client negotiates 2025 by default**. A host must opt into +modern negotiation explicitly or it will be turned away. The MCP Inspector needs +`"protocolEra": "modern"` in its server entry for the same reason. + +## Sessions are gone + +There is no `Mcp-Session-Id`, no GET stream, no DELETE, and no `Last-Event-ID` resumption. Each +request is served independently by a fresh `McpServer` built for it. + +Two files went with them: `http/app.ts`, which existed to manage the session map, and +`utils/inMemoryEventStore.ts`, which existed to replay events into a resumed stream. Nothing needs +reaping at shutdown any more. + +Two things that used to be session properties are now per request. Client capabilities arrive in each +request's `_meta` envelope, so a server reads them per call rather than once at connect. Server +instructions, which used to ride the `initialize` response, are now part of `server/discover`. + +The cost is that a handler holds nothing between requests. That matters most for the confirmation +flow below. + +## Express is gone + +`createMcpHandler` returns a web-standard `{ fetch, close }`, and `toNodeHandler` from +`@modelcontextprotocol/node` mounts it on plain `node:http`. Express contributed one thing we +actually used, `express.json({ limit })`, and `http/requestBody.ts` replaces it. + +Host and Origin validation come from the SDK as `node:http` guards, so nothing security-relevant was +hand-rolled to make this work. Dropping Express removed its whole transitive tree from the image and +the audit surface. + +One deliberate difference from the SDK: binding a routable interface with no Host allowlist **fails +at startup** here, where the SDK only warns. That configuration is exactly what an operator reaches +for when moving from a laptop into a container, and a warning reads as noise. + +## Confirmation is two requests now + +`execute_query` shows the user the generated GraphQL query and asks them to confirm before it runs. +That used to be a server-initiated `elicitInput()` call the handler awaited. The revision removes the +server-to-client request channel entirely, so servers can no longer ask anything mid-handler. + +Instead the handler returns an `input_required` result and the call ends. The client fulfils the +embedded request and re-invokes the tool with the answer attached. The handler runs twice per +confirmed query, from the top both times: introspection re-fetched, validation re-run, query rebuilt. + +Two consequences worth knowing: + +- **Introspection traffic doubled.** A confirmed query costs four Arranger round trips where it cost + two. Caching introspection is now load-bearing rather than an optimization, and is tracked in + `.dev/tech-debt.md`. +- **A client that cannot elicit is refused**, not served unconfirmed. With 2025-era serving gone, + that was the last remaining route to running a query nobody approved. Confirm-before-execute is an + invariant of the tool now rather than a best effort, which is what the server instructions and the + `query_arranger` prompt always claimed it was. + +## The approval is bound to the query + +Because nothing carries between the two rounds, the second round rebuilds the query from arguments +the client re-sends. Left alone, that means an agent could show one query for confirmation and +execute a different one under the same approval. + +The revision's answer is `requestState`: opaque server state the client echoes back. It travels +through the client, so it returns as attacker-controlled input, and the spec makes integrity +protection a MUST wherever it influences business logic. The SDK verifies nothing by default. + +We seal a SHA-256 digest of the built query, its variables and its endpoint into `requestState`, +signed with HMAC, and compare it on re-entry. The digest covers what actually runs rather than the +tool arguments, which also catches a subtler case: introspection is re-fetched on round two, so a +catalogue reconfigured between rounds could build a different query from identical arguments. + +Three refusals fall out of this, all of them errors rather than fresh confirmation requests, because +re-asking would hand a caller an unlimited retry loop against the gate: + +- a `requestState` that fails verification, refused by the SDK seam before the tool is entered; +- a digest that does not match the query this call built; +- an answer carrying no `requestState` at all, refused exactly like a mismatch. Nothing forces a + client to echo it, so comparing only when present would make the whole binding opt-out. + +`MCP_REQUEST_STATE_SECRET` is the signing key. Unset, the server generates one per process, which is +correct at a single replica and fails across several. + +## Results carry freshness hints + +Six results are cacheable on this revision and must carry `ttlMs` and `cacheScope`: +`tools/list`, `prompts/list`, `resources/list`, `resources/templates/list`, `resources/read` and +`server/discover`. The SDK default of `{ ttlMs: 0, cacheScope: 'private' }` is compliant and tells +every client to cache nothing. + +We publish two groups. Results that change only when the server is redeployed get an hour and +`public`. Results that track Arranger's catalogue configuration get a minute and `private`. +`resources/list` is in the second group despite the name, because the catalogue resource is a +template whose listing asks Arranger which catalogues exist. + +The scope split is worth understanding before anyone widens it. `private` does not mean "do not +cache", it means "do not share": a client still caches, partitioned by principal, which is where the +benefit is. `public` would only add sharing across principals, and would become a leak the day this +server forwards auth, since catalogue introspection already has a per-caller-filtered mode we do not +read yet. + +These are client-side hints on our results. They do nothing for the introspection traffic in front of +Arranger. + +## New environment variables + +| Variable | Why it appeared | +| -------------------------- | ------------------------------------------------------------------------------------------------------- | +| `MCP_ALLOWED_HOSTS` | DNS rebinding protection. Required whenever `MCP_HOST` is not loopback, or the server exits at startup. | +| `MCP_ALLOWED_ORIGINS` | Browser origins allowed to call the server. An empty list is a live check, not a disabled one. | +| `MCP_MAX_BODY_BYTES` | Replaces the `100kb` cap `express.json()` used to apply. | +| `MCP_REQUEST_STATE_SECRET` | Signs query confirmations. Optional at one replica, required across several. | + +Full descriptions are in [`apps/mcp-server/README.md`](../../apps/mcp-server/README.md#environment-variables). diff --git a/.dev/docs/mcp-sdk-v2-upgrade-plan.md b/.dev/docs/mcp-sdk-v2-upgrade-plan.md new file mode 100644 index 000000000..774fc8cb9 --- /dev/null +++ b/.dev/docs/mcp-sdk-v2-upgrade-plan.md @@ -0,0 +1,988 @@ +# MCP SDK v2 and spec revision `2026-07-28` upgrade + +**Status: complete.** All five commits landed on 2026-09-04 and 2026-09-05, each recorded in an "as +built" section below. Decisions 1, 3, 7 and 8 settled 2026-09-03, 5 on 2026-09-04 and 6 on +2026-09-05; 2 and 4 are moot as a consequence. One item is handed off rather than done: the +amendments the [MCP platform testing plan](./mcp-platform-testing.md) needs, listed at the end, +belong to that document's owner. + +This is the implementation record, kept for the reasoning behind each decision and for the probe +results that corrected it. For a short read on what changed and why, see +[MCP SDK v2 changes](./mcp-sdk-v2-changes.md). + +**Verified 2026-09-03** by installing `@modelcontextprotocol/{server,express,client}@2.0.0` and +reading the shipped type declarations. Every API named below exists as described. Corrections to the +earlier "Crossing the Era Boundary" review are marked inline. + +**Goal.** `apps/mcp-server` serves revision `2026-07-28` on SDK v2, with today's surface intact: five +tools (`list_catalogues`, `get_sqon_schema`, `get_catalogue_fields`, `build_sqon`, `execute_query`), +three resources (`arranger_server_introspection`, `arranger_sqon_schema`, +`arranger_catalogue_fields`), one prompt (`query_arranger`), and the confirm-before-execute flow the +prompt describes. + +--- + +## Why this cannot be incremental + +We serve `2025-11-25`, the ceiling of `@modelcontextprotocol/sdk@1.29.0` and of 1.30.0, the final v1 +release. The target revision removes the `initialize` handshake and protocol-level sessions, which +are the two things `http/app.ts` is built around, and no v1 release implements it. + +What we do **not** have to build is backwards compatibility, and per decision 3 we do not serve it +either: the endpoint is `legacy: 'reject'`, modern-only. + +**An earlier draft of this section was wrong** and the error is worth keeping visible, because it +was the reason legacy serving looked cheap. It claimed the SDK's legacy shim fulfils multi-round-trip +results for old clients by issuing real server-to-client requests and re-entering the handler. It +does, but **not under `legacy: 'stateless'`**: the shim consults the `initialize`-declared client +capabilities, and per-request stateless serving never sees an `initialize`. Measured, not inferred +(see decision 3). The shim and stateless legacy serving are mutually exclusive, so `'stateless'` +would have bought us four working tools and an `execute_query` that cannot confirm. + +--- + +## Committing: one PR, five commits + +**Do not commit this as one change.** Phase A alone touches every file in `src/mcp/` and deletes two +others; folding the confirmation rewrite into that makes the security-relevant diff unreviewable. + +**Do not split it across PRs either.** Phases A and B are not independently shippable: after A the +confirmation flow is disabled, which is a functional regression nobody should be able to deploy. One +PR, five commits, reviewable in order. + +| # | Commit | Green? | Notes | +| --- | -------------------------------------- | -------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------- | +| 1 | Swap packages and rewire the transport | **done 2026-09-04**: 302 unit, 84 integration, 2 skipped | also carried the three transport-hardening items decision 1 obliges | +| 2 | Rebuild confirmation as MRTR | **done 2026-09-04**: 313 unit, 87 integration, 0 skipped | the behavioural rewrite; one era to assert, not two | +| 3 | Integrity-protect `requestState` | **done 2026-09-04**: 331 unit, 87 integration, 0 skipped | security; separate so it is reviewed on its own | +| 4 | Configure what the revision added | **done 2026-09-05**: 340 unit, 87 integration, 0 skipped | cache hints, `serverInfo`, contract tests; the `x-mcp-header` item was declined, not deferred | +| 5 | Docs and inspector | **done 2026-09-05** | README, `CHANGELOG.md`, `mcp-inspector.json`; the testing-plan amendments are handed off, not applied here | + +Commit 1 is the only one that is deliberately feature-incomplete. Say so in its message. + +--- + +## Phase A, commit 1: swap packages and rewire the transport + +### Decision 1, SETTLED 2026-09-03: drop Express, serve `handler.fetch` on `node:http` + +**Decided.** `apps/mcp-server` takes no Express dependency. `createMcpHandler` returns a +**web-standard** `{ fetch, close, notify, bus }`; a small fetch-shaped router wraps it, and +`toNodeHandler` from `@modelcontextprotocol/node` mounts that router on `node:http`. Host and Origin +validation come from the same package, as plain `node:http` guards. + +Two premises in the original three-way comparison were wrong. Both were checked against the +installed packages rather than reasoned from. + +- **Hono is not a new dependency.** `@modelcontextprotocol/sdk@1.29.0` already depends on + `hono@4.12.23` and `@hono/node-server@1.19.14`, so both are in this workspace today. Taking + `@modelcontextprotocol/node` keeps them; it does not add them. `hono` is an _optional_ peer of that + package, and the only runtime import is `getRequestListener` from `@hono/node-server`, whose main + entry never imports `hono` (only its `serve-static` submodule does, which the adapter never + touches). So `hono` is installed and never loaded. +- **Dropping Express loses nothing security-relevant.** `@modelcontextprotocol/node@2` exports + `hostHeaderValidation`, `originValidation`, `localhostHostValidation` and + `localhostOriginValidation` as plain `node:http` guards: `(req, res) => boolean`, having already + answered `403` when false. `@modelcontextprotocol/server@2` carries the runtime-neutral core that + every framework adapter wraps: `validateHostHeader`, `validateOriginHeader`, `requireBearerAuth`, + `verifyBearerToken`, `bearerAuthChallengeResponse`, `oauthMetadataResponse`, + `createRequestStateCodec`. The `originValidation` docstring names `@modelcontextprotocol/node` + alongside the express, hono and fastify adapters, so it is a first-class adapter, not a fallback. + +Fresh-install footprint, measured: + +| Wiring | packages | size | +| ----------------------------------------- | -------- | ------- | +| today, `@modelcontextprotocol/sdk@1.29.0` | 94 | 26M | +| `server` + `express` adapter + express 5 | 74 | 19M | +| **`server` + `node`, chosen** | **6** | **18M** | +| `server` only, hand-written bridge | 3 | 15M | + +Express alone pulls 64 transitive packages, which is where its advisory history lives +(`path-to-regexp`, `qs`, `body-parser`, `send`, `cookie`). `@hono/node-server` pulls zero. + +The hand-written bridge was rejected as a false economy. `toNodeHandler` is about sixty lines, but +its careful parts are abort-on-close, SSE write backpressure via `drain`, and `content-length` +recomputation for pre-parsed bodies. That is the exact bug class a streaming transport cannot afford +to own, in a package that is already in the tree. + +Two things Express was assumed to provide and does not: `createMcpExpressApp` never calls `cors()` +(the `cors` dependency is used only inside `mcpAuthMetadataRouter`, for the OAuth well-knowns), and +`mcpAuthMetadataRouter` has a one-call fetch-layer equivalent in `oauthMetadataResponse`, which +already emits permissive CORS, `405` with `Allow`, and `204` preflight. + +**Correction to the earlier review, retained:** it showed `createMcpExpressApp` and `toNodeHandler` +used together as though the Express adapter bridged the handler. It does not. +`@modelcontextprotocol/express@2` exports `createMcpExpressApp` plus auth and host-validation +middleware (`hostHeaderValidation`, `localhostOriginValidation`, `requireBearerAuth`, and so on); the +bridge is only in `@modelcontextprotocol/node`. + +#### The shape + +A health probe and the OAuth well-knowns are the usual reason to keep a router, and both are likely +here. Both compose at the fetch layer, so the whole app stays one `http.createServer`: + +```ts +const router = { + fetch: async (request: Request, options?: McpHandlerRequestOptions): Promise => { + const { pathname } = new URL(request.url); + if (pathname === HEALTH_PATH) return Response.json({ status: 'ok' }); + return oauthMetadata(request) ?? handler.fetch(request, options); + }, +}; + +const serve = toNodeHandler(router, { onerror: (error) => logger.error({ error }, 'MCP adapter error') }); + +http.createServer(async (req, res) => { + if (!validateHost(req, res)) return; + if (!validateOrigin(req, res)) return; + const body = await readCappedJsonBody(req, res, config.mcp.maxBodyBytes); + if (body === REJECTED) return; + await serve(req, res, body); +}).listen(port, host); +``` + +`toNodeHandler` takes any `{ fetch }` structurally, and the type is documented as staying structural +so hand-wired compositions work, which makes the router a supported use rather than a workaround. +`/health` sits outside `handler.fetch` deliberately: a readiness probe should still answer while the +handler is closing. + +#### Later: when auth lands, still out of scope for this PR + +Recorded because it was the main objection to dropping Express, and it does not survive contact with +v2. + +v1 shipped a full authorization server behind Express: `mcpAuthRouter`, `OAuthServerProvider`, and +handlers for `authorize`, `token`, `register` and `revoke`, all Express-only and all using +`express-rate-limit`. **v2 ships none of it.** `@modelcontextprotocol/express@2` exports only +`createMcpExpressApp`, the host and origin guards, `requireBearerAuth`, `mcpAuthMetadataRouter` and +`getOAuthProtectedResourceMetadataUrl`. Being an authorization server is a separate service now, on +either path. + +For the resource-server side, Express's `requireBearerAuth` is documented as "the Express adapter +over the runtime-neutral core in `@modelcontextprotocol/server`", and `BearerAuthMiddlewareOptions` +is a type alias for `BearerAuthOptions`. Same capability, both paths: + +| Express | `node:http` + fetch | From | +| -------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | +| `requireBearerAuth(opts)` sets `req.auth` | `requireBearerAuth(opts)(request)` returns `AuthInfo \| Response`, or `verifyBearerToken(header, opts)` plus `bearerAuthChallengeResponse(err, opts)` | `server` | +| `mcpAuthMetadataRouter(opts)`, with `cors()` | `oauthMetadataResponse(request, opts)` returns `Response \| undefined` | `server` | +| `getOAuthProtectedResourceMetadataUrl` | same function | both | + +The handoff into the handler is identical. `toNodeHandler` forwards `req.auth` as `handler.fetch`'s +`authInfo` (`NodeIncomingMessageLike.auth` is declared as validated info "attached by upstream +middleware"), and it reaches tool handlers as `ctx.http?.authInfo`. So our wiring assigns `req.auth` +exactly as Express middleware would. + +One advantage the composed path gains: `verifyBearerToken` takes a raw `Authorization` header string, +so auth can run **before** the body is read. + +```ts +try { + req.auth = await verifyBearerToken(req.headers.authorization, bearerOptions); +} catch (error) { + return writeResponse(res, bearerAuthChallengeResponse(error, bearerOptions)); +} +``` + +`createMcpExpressApp` cannot do that: it mounts `express.json()` first and app-wide, so an +unauthenticated caller still gets the full body limit buffered before `requireBearerAuth` runs. + +When auth does land, configure the bearer gate and the OAuth metadata endpoint as a pair. +`bearerAuthChallengeResponse` advertises `resource_metadata` in its `WWW-Authenticate` challenge only +when `resourceMetadataUrl` is set, and that is what lets an unauthenticated client discover the +authorization server from the `401`. + +#### Why this stays reversible + +v2 ships no rate limiting, and `express-rate-limit` has no `node:http` drop-in. If per-token or +per-IP limits become a requirement inside the app rather than at the gateway, Express comes back, and +it costs about ten lines: `toNodeHandler` returns a function that is already valid Express +middleware, and its docstring states that a function third argument (Express's `next`) is ignored and +never treated as a body. + +```ts +app.use(rateLimit({ ... })); +app.use(express.json({ limit })); +app.post(config.mcp.path, toNodeHandler(router)); +``` + +The router, the guards, the auth gate and every tool file are untouched by that change. Re-adding +Express when a concrete middleware need appears is cheaper than carrying 64 transitive packages +against a hypothetical one. + +### Decision 2, MOOT: which Express major, if Express stays + +Closed by decision 1: Express does not come back. This app stops being gated on the root `overrides` +pin of `@types/express` to `4.17.25`, which now constrains only `apps/search-server` and +`modules/graphql-router`. That is what the tech-debt entry from `c18a5a2e` predicted would resolve +here. Drop `@types/express` from `apps/mcp-server`'s `devDependencies`. + +### Decision 3, SETTLED 2026-09-03: serve one revision only, `legacy: 'reject'` + +**Decided.** `createMcpHandler(factory, { legacy: 'reject' })`. The endpoint serves `2026-07-28` and +nothing else. This supersedes the narrower question the decision started as (signing off on lost +legacy resumability); the sign-off now covers dropping 2025-era serving entirely, which is a larger +commitment and still someone's to own. + +**Why it got bigger.** The premise that legacy clients keep the confirm-before-execute flow is false +in the configuration we would have shipped. Measured end to end: a tool returning `inputRequired(...)` +behind `legacy: 'stateless'`, called by a client that declares `elicitation` and answers it, gets + +```text +{"content":[{"type":"text","text":"Cannot request input 'confirm' (elicitation/create): the client +on this 2025-era connection did not declare the required capability (no client capabilities are +available on this connection - per-request legacy serving cannot receive server-to-client +requests)"}],"isError":true} +``` + +while the same server answers a client pinned to `2026-07-28` correctly. The SDK states the mechanism +in the `Server` internals: "Per-request instances that never saw an initialize (stateless legacy) +hold nothing, so gates refuse there." **The legacy shim and stateless legacy serving are mutually +exclusive**, and getting the shim would mean a sessionful legacy wiring, which this decision already +rejected as both SDK majors in one process. + +Everything else does work on the legacy path. The full non-confirming surface was checked: + +| | legacy client | modern pinned client | +| ----------------------------------------------------------------------------- | ---------------------- | -------------------- | +| `tools/list`, `tools/call`, `resources/list`, `resources/read`, `prompts/get` | OK | OK | +| server instructions | delivered | delivered | +| `GET` / `DELETE` on the endpoint | 405 | 405 | +| multi-round-trip input | **refused, `isError`** | works | + +So the real choice was between these, not between resumable and non-resumable: + +| | Legacy clients get | Confirmation | Cost | +| --------------------------------------------------------- | ----------------------------------------------- | ------------------------ | ----------------------------------------- | +| A. `'stateless'`, skip confirmation when unsupported | four tools plus **unconfirmed** `execute_query` | **downgrade-defeatable** | shim testing stays, decision 4 stays live | +| B. `'stateless'`, refuse `execute_query` when unsupported | four tools plus a fifth that always errors | enforced | confusing surface, two eras to test | +| **C. `'reject'` (chosen)** | a `-32022` naming `supported: ["2026-07-28"]` | enforced, one path | only opted-in clients connect | +| D. C behind a config flag | operator's choice | mode-dependent | ships a documented downgrade path | + +Option A is the one to name and reject explicitly: it turns confirm-before-execute into a control any +client removes by connecting with an older handshake, while the prompt and the docs keep claiming it +exists. Option D was rejected for the same reason in weaker form. This server is consumed by our own +client and host application, so what legacy serving buys is reach we do not need, and what it costs +is a security property we do. + +**What `'reject'` answers.** Legacy-classified requests get a clean, discoverable error rather than a +mystery failure; legacy-classified notifications are acknowledged `202` and dropped. + +```text +{"jsonrpc":"2.0","error":{"code":-32022,"message":"Unsupported protocol version: 2025-11-25", + "data":{"supported":["2026-07-28"],"requested":"2025-11-25"}},"id":0} HTTP 400 +``` + +**The catch that applies either way.** `@modelcontextprotocol/client@2` defaults to +`versionNegotiation.mode: 'legacy'`: "absent (or `mode: 'legacy'`), `connect()` runs the plain 2025 +sequence, byte-identical to today's behavior (no probe, no new headers). Opt into `'auto'` or pin to +talk to a 2026-07-28 server." Nothing speaks modern by accident, including our own integration +tests, which must set `versionNegotiation: { mode: { pin: '2026-07-28' } }` or they will silently +cover the wrong era. Our host application needs the same opt-in. + +Also still true, and now simply a consequence rather than the decision: GET and DELETE return 405, +there is no session resumption or SSE replay, and `InMemoryEventStore` (which our own source +annotates as not production-suitable) is deleted. + +### The work + +- Run `npx @modelcontextprotocol/codemod@latest v1-to-v2`, then review every hunk. It does the + mechanical import and API renames, and none of the three hardening items below. +- `package.json`: drop `@modelcontextprotocol/sdk` and the `@types/express` devDependency; add + `@modelcontextprotocol/server@^2` and `@modelcontextprotocol/node@^2`. + `@modelcontextprotocol/core` arrives transitively (`server` pins it at exactly `2.0.0`) and need + not be declared. `zod` is already `^4.2.0`. +- **Delete** `src/http/app.ts` and `src/utils/inMemoryEventStore.ts`, with the `transports` map, + `sessionHandler`, and `closeAllSessions`. +- `src/server.ts`: build the handler from a per-request factory over the existing + `createMcpServer(deps)` with `{ legacy: 'reject' }` (decision 3), wrap it in the fetch router, and + mount it with `toNodeHandler` on `http.createServer`. Shutdown becomes `await handler.close()` + followed by closing the http server. +- `src/mcp/*.ts`: import paths, `extra` becomes `ctx`, and raw `inputSchema` shapes wrap in + `zod.object()`. `ResourceTemplate` and `registerResource` keep their shape. +- **Temporarily disable confirmation** in `execute_query` so the suite can run. Commit 2 restores it. +- `integration-tests/mcp-server`: client moves to `@modelcontextprotocol/client@2`, **and must set + `versionNegotiation: { mode: { pin: '2026-07-28' } }`**. That client defaults to `mode: 'legacy'`, + so without the pin every test would connect 2025-era and be rejected outright by decision 3's + endpoint. Pin rather than `'auto'`: `'auto'` falls back silently, which is exactly the failure the + suite exists to catch. `startMcpServer.ts` is built on `createHttpApp`'s + `{ app, closeAllSessions }` return, which no longer exists. It already returns an `http.Server`, so + it takes the new wiring's `{ httpServer, close }` instead; a smaller change than it looks. + +#### What decision 1 obliges, each worth its own test + +**1. A request body cap.** `createMcpHandler` has no body-size option, and `toWebRequest` reads the +request stream to completion with no limit, so `express.json()`'s implicit `100kb` cap disappears +along with Express. Read with a byte counter, answer `413` past the limit, `JSON.parse`, and hand the +parsed object to `toNodeHandler` as `parsedBody` (it re-serializes and fixes `content-length`, the +same path Express takes today). New env var, defaulted above our largest legitimate `execute_query` +payload. + +**2. `MCP_ALLOWED_HOSTS` and `MCP_ALLOWED_ORIGINS`.** This fixes a live defect, not a migration +artifact. `createHttpApp` calls `createMcpExpressApp()` with no options, so `host` defaults to +`'127.0.0.1'` and `localhostHostValidation()` is applied, while the process binds every interface. +Verified against the installed v1 SDK: + +```text +Host: 127.0.0.1:59791 -> 200 {"ok":true} +Host: arranger-mcp:59791 -> 403 {"error":{"code":-32000,"message":"Invalid Host: arranger-mcp"}} +Host: mcp.example.org -> 403 {"error":{"code":-32000,"message":"Invalid Host: mcp.example.org"}} +``` + +The `mcp-server` stage of `docker/Dockerfile.jenkins` exposes 3100, so a peer container reaching it +as `arranger-mcp:3100` gets a `403` today. Deployment topology is undecided (proxied, direct, or +both), so the app should **fail fast at startup** when it binds a routable interface with no +allowlist configured, with an explicit `MCP_ALLOWED_HOSTS=*` opt-out for "a gateway owns this". That +is stricter than the SDK's `console.warn`, and it cannot regress an existing deployment because no +non-localhost deployment works today. Operator-facing, so it belongs in `CHANGELOG.md`. + +Leave `MCP_ALLOWED_ORIGINS` unset meaning an empty allowlist, not a disabled check. +`originValidation([])` passes requests carrying no `Origin` (every non-browser MCP client) and +rejects any browser origin, which is the right default for a server with no browser clients and +needs no special-casing. + +**3. `server.ts` honouring `config.mcp.host`.** It calls `app.listen(port)` today and ignores the +configured host, so `MCP_HOST` is decorative. On `node:http` we pass both, which also means the host +we validate against and the interface we bind derive from the same config. + +**Not in scope, file as tech debt:** `docker/Dockerfile.jenkins` copies the entire hoisted +`node_modules` into the `mcp-server` stage, so express, apollo and the graphql router ship in that +image whatever this app's manifest says. Trimming the manifest shrinks what we declare and audit; it +will not shrink the image until that stage installs per-workspace. + +**Done when:** the existing integration suite passes with confirmation disabled, and the body cap, +the allowlist fail-fast, and a non-localhost `Host` reaching the MCP endpoint are each pinned by a +test. + +### Commit 1 as built, 2026-09-04 + +Built, reviewed and staged. 31 files: 24 modified, 2 deleted, 4 new, plus the lockfile. Green at +302 unit tests, 84 integration tests, 2 skipped. Not committed at time of writing. + +**Where it differed from the plan above.** + +- **No fetch router.** The plan sketched a `{ fetch }` wrapper hosting `/health` and the OAuth + well-knowns alongside `handler.fetch`. Neither is decided, so a router would have been an empty + wrapper; `toNodeHandler(handler)` mounts directly and the router stays a one-line insertion point. +- **`ping` is gone from this revision**, and the SDK refuses it client-side before the wire, so the + integration suite's liveness probe became `server/discover`. +- **Prompt argument errors changed shape.** v1 quoted the failing path as `"goal"`; v2 renders it + `query_arranger: goal: Too small`. The assertion now pins the prompt-name-then-path shape rather + than Zod's issue wording. +- **`@modelcontextprotocol/client@2` was added as a devDependency of `apps/mcp-server`** for the + transport positive control. Dev-only, so `npm ci --omit=dev` keeps it out of the image. +- **`.env.schema` binds `0.0.0.0` with explicit allowlists** rather than binding loopback, so the + template models the deployment shape (bind broadly, allowlist explicitly) instead of a local + special case that contradicted the documented default. + +**Choices accepted in review that remain revisitable.** None of these are settled decisions; they +are judgment calls with a live counter-argument, recorded so they are not mistaken for consensus. + +- **The startup fail-fast is stricter than the SDK**, which only `console.warn`s on a routable bind + with no allowlist. Accepted because a missing env var then surfaces as a pod that will not start, + and nothing can be deployed unprotected. The counter stands: it turns an optional protection into + a required variable, and "forgot to add the new ingress hostname" reproduces the same `403` this + commit fixed, relocated rather than removed. One `superRefine` block to soften. +- **`MCP_MAX_BODY_BYTES` defaults to `102_400`, which is preserved behaviour rather than a measured + ceiling.** An earlier draft used 1 MiB, loosening the enforced limit tenfold on no evidence; that + was reverted deliberately. Raise it only against a real payload, such as an `execute_query` SQON + filtering on a very large identifier set. +- **`MCP_ALLOWED_ORIGINS` is the weakest of the three new variables.** There are no browser clients, + and leaving it unset already behaves correctly. Kept because Origin is the other half of the DNS + rebinding defense and costs little, but it is the first thing to drop if the surface is judged too + wide. + +**Test gaps left open knowingly.** None block commit 1; recorded so they are not rediscovered as +surprises. + +- Nothing asserts that an oversized body is **cut off mid-stream** rather than fully buffered. That + is what `req.destroy()` is for, and an implementation that buffered everything and then answered + `413` would pass every test written. +- No config edge cases: whitespace-only allowlists, `MCP_ALLOWED_HOSTS='*,foo'` (`*` wins, which is + undocumented), `MCP_HOST=::`. +- Shutdown is smoke-covered only. Nothing asserts in-flight request behaviour or that `close()` is + idempotent. +- `readCappedJsonBody` has no direct unit test; it is reached only through the server. + +**Deliberately still commit 5.** `README.md` continues to say v1.x and to list `http/app.ts` and +`utils/inMemoryEventStore.ts` in its folder structure; `CHANGELOG.md` and `mcp-inspector.json` are +untouched. Only the README's environment-variable table moved, because commit 1 introduced the +variables it documents. + +**Pre-existing, not fixed.** `integration-tests/mcp-server/tsconfig.json` sets no `strict`, so +discriminated unions do not narrow there and it already failed on `main`. Nothing runs it. New code +was written to be clean under it regardless. + +### What commit 2 inherits + +- `confirmExecution()` in `executeQueryTool.ts` is a stub returning `true`, and the call site is + intact (`const confirmed = confirmExecution(); if (!confirmed) { ... }`), so the decline path and + its `executed: false` result shape are still there to rebuild against. +- `integration-tests/mcp-server/test/executeQuery.ts` tests 14 and 15 are `test.skip` with their + assertions unchanged. They describe the behaviour the rewrite has to reproduce. +- `connectElicitingClient` in that file already registers its handler by method name + (`'elicitation/create'`) and already carries the modern pin, so only the assertions move. +- Decision 8 governs the remaining branch: a modern client that does not declare `elicitation` must + be **refused**, not served unconfirmed. The old skip-when-unsupported behaviour does not survive. + +--- + +## Phase B, commit 2: rebuild confirmation as MRTR + +`confirmExecution` calls `server.server.elicitInput()` at +[`executeQueryTool.ts`](../../apps/mcp-server/src/mcp/executeQueryTool.ts) +and awaits the answer. Servers can no longer initiate requests. The handler instead returns an +`InputRequiredResult`, the call ends, and the client re-invokes the tool with the answer attached. + +Verified API: `inputRequired` is an exported builder; `acceptedContent(responses, key, schema?)` +reads the reply, and its schema-aware overload validates against any Standard Schema, so a Zod object +works and the untrusted client value arrives typed. + +Three consequences: + +- **The handler must survive re-entry.** On retry it re-runs from the top: introspection fetched + again, validation again, GraphQL query rebuilt. A confirmed query therefore costs four Arranger + round trips where it cost two, so introspection caching becomes load-bearing rather than an + optimization. **No commit in this plan owns it**, because it is a server-side concern that predates + this migration and is only amplified by it; it is tracked in `.dev/tech-debt.md` under + `apps/mcp-server` instead. +- **Nothing carries over except `requestState`.** Anything the second call needs is encoded there or + re-derived from the re-sent arguments. +- **The capability check moves** from `server.server.getClientCapabilities()` at + [`executeQueryTool.ts`](../../apps/mcp-server/src/mcp/executeQueryTool.ts) + to `ctx.mcpReq.envelope?.clientCapabilities`. The rule holds: MUST NOT send an input request to a + client that did not declare `elicitation`. What changes is what we do about it: see decision 8. + +**Decision 8, SETTLED 2026-09-03: `execute_query` refuses a client that cannot elicit.** The +skip-when-unsupported branch does **not** survive. With legacy serving gone (decision 3), a modern +client that omits `elicitation` from its request envelope is the only remaining way to reach +`execute_query` without confirmation, and silently executing there would leave the same +downgrade-shaped hole that decision 3 closed, just narrower. The tool returns an error telling the +client it must support elicitation. That makes confirm-before-execute an invariant of the tool rather +than a best effort, which is what `SERVER_INSTRUCTIONS`, the `query_arranger` prompt, and the README +all already describe it as. + +**Decision 4 is moot**, closed by decision 3. `ServerOptions.inputRequired.maxRounds` (default 8) is +documented as "handler re-entries per originating request **before the shim fails**", so it only ever +governed the legacy shim, which `legacy: 'reject'` never runs. Note the identically-named client-side +knob is a different thing and still live: `ClientOptions.inputRequired.maxRounds` (default 10) caps +the auto-fulfilment driver's rounds and belongs to our host application's configuration, not this +server's. + +**Done when:** confirmation works on a modern client, asserted once, and a modern client that does +not declare `elicitation` is refused rather than served, asserted separately. + +### Commit 2 as built, 2026-09-04 + +Built and green: 313 unit tests (up from 302), 87 integration tests, nothing skipped. Tests 14 and 15 +are restored and a new test 22 pins the refusal. + +**Correction to this plan.** `ctx.mcpReq.envelope?.clientCapabilities` does not exist. The envelope +carries the reserved keys verbatim, so the capability read is +`envelope['io.modelcontextprotocol/clientCapabilities']`, via the exported +`CLIENT_CAPABILITIES_META_KEY`. `RequestMetaEnvelope` is typed as an open object, so the value is +narrowed in user land rather than typed. Measured with a probe, not read off the declarations. + +**`inputResponse()` carries the flow, not `acceptedContent()` alone.** `acceptedContent` returns +`undefined` for a missing key, a decline, a cancel, and content that fails the schema, which makes +the first round indistinguishable from a refusal. `inputResponse()` returns a discriminated view +(`missing` / `elicit` with an `action`), so the handler asks on `missing`, refuses on anything that +is not an accepted `confirm: true`, and validates the accepted content with the schema-aware +`acceptedContent` overload. + +**No `requestState`, deliberately.** The approval is not yet bound to what was approved: re-entry +rebuilds the query from the arguments the client re-sends, so an agent could show one query for +confirmation and re-enter with different ones. That is commit 3's job and is flagged in +`resolveConfirmation`'s doc comment. Splitting it this way is deliberate rather than incremental: an +unauthenticated digest is worthless, since the integrity comes from the HMAC, so the digest and the +codec have to land together. + +**The shared integration client now approves.** Decision 8 would otherwise refuse every +`execute_query` test, since that client declared no capabilities. `connectApprovingClient` declares +`elicitation` and auto-accepts, which has the useful side effect that every `execute_query` test now +exercises the two-round exchange rather than only tests 14 and 15. + +**Unit coverage added** in `executeQueryTool.test.ts`, driving the registered handler with a stubbed +Arranger client and a synthetic context. It pins the states the integration suite cannot reach +because a well-behaved client never sends them: an accepted answer whose `confirm` is not a boolean, +an accepted answer with no content, and a response shape the SDK could not read (which must re-ask +rather than count as approval or refusal). It also asserts the refusal costs no Arranger round trip. + +--- + +## Phase B, commit 3: integrity-protect `requestState` + +Separate commit because it is the security-relevant change and should be reviewed as one. + +`requestState` travels through the client and returns as attacker-controlled input. Today the query +the user approves is the query we run, because it never leaves the process. Encode the built GraphQL +query into `requestState` and execute what comes back, and a tampered value executes something the +user never saw. The spec makes integrity protection a **MUST** where the state influences +authorization, resource access, or business logic, and the SDK provides no default verification. + +### Decision 5, SETTLED 2026-09-04: what goes into `requestState` + +**Verified 2026-09-04** against `createRequestStateCodec`, by probe rather than by docstring. The +codec is richer than this plan assumed, and three of the four fields it recommended do not belong in +the payload at all. + +| Probe | Result | +| ------------------------------------------------------- | ------------------- | +| valid, same bind | returns the payload | +| tampered mac, tampered payload | throws `mac` | +| different principal, absent principal, different method | throws `bind` | +| garbage | throws `malformed` | +| 3s past `ttlSeconds: 1` | throws `expired` | + +A caution for whoever re-runs this: `exp` has one-second granularity, so a 1.5s wait against +`ttlSeconds: 1` can still land on the mint second and read as a **false** "expiry does not work". +Leave a margin of several seconds. + +**What the codec already does, so we do not:** + +- **Expiry is `ttlSeconds`, not a payload field.** Default 600 seconds, which we keep. No env var: + a one-line change if a confirmation window ever needs tuning. +- **The principal belongs in `bind`, not the payload.** `bind` is evaluated at mint and again at + verify, and is stored as a domain-separated HMAC tag, so the identifier never reaches the wire. +- **The payload is signed, not encrypted.** Decoded straight off the wire in the probe: + `{"p":{"digest":"..."},"exp":1788563528,"b":"mU1qn..."}`. Nothing secret may go in it. + +**The constraint this plan did not anticipate:** `bind(ctx)` receives a `ServerContext`, which has no +access to the tool arguments. The approved-query digest therefore cannot live in `bind` and be +compared by the SDK. It goes in the payload, and our handler does the comparison. + +**Settled shape.** Payload is `{ digest }` and nothing else. + +- **The digest covers the built query, its variables and the endpoint**, not the tool arguments. + That is what actually runs and what the confirmation message displayed. It also catches drift we + would otherwise miss: introspection is re-fetched on round two, so a catalogue reconfigured between + rounds could build a different query from identical arguments and run it under the old approval. +- **`bind` is** `` ctx => `${ctx.mcpReq.method}\0${ctx.http?.authInfo?.clientId ?? ''}` ``, the SDK's + documented shape. With auth out of scope the principal is always empty, so today it only separates + methods. It starts separating principals the moment auth lands, with no code change here. +- **Key:** `MCP_REQUEST_STATE_SECRET` when set (the codec requires at least 32 bytes and throws a + `RangeError` below that), otherwise a random 32-byte per-process key plus a startup warning naming + the consequence. Single replica today, so a per-process key is correct and costs no configuration; + it is secure, just not horizontally scalable. Under more than one replica, round one mints on pod A + and round two verifies on pod B, which fails `mac` and surfaces as an intermittent `-32602` that + reads like a bug. A process restart also invalidates in-flight confirmations, and the user simply + re-confirms. +- **On digest mismatch: refuse** with an error naming the mismatch, rather than re-asking. Re-asking + would hand a caller an unlimited retry loop against the confirmation gate. + +**A hole to close explicitly.** An answer may arrive with `requestState` **absent**: nothing forces a +client to echo it. If the handler only compares when the state is present, the whole binding is +opt-out and this commit achieves nothing. Once an answer is present, a missing `requestState` must be +refused exactly like a mismatched one. + +Plain `===` for the digest comparison. The digest is not a secret, since it is readable on the wire, +so a constant-time compare buys nothing; the codec already uses constant-time comparison where it +matters, for the mac and the bind tag. + +### How it wires up + +- **Construct the codec once at startup**, in `startServer`, and pass it into `createMcpServer` as a + dependency alongside `config` and `client`. **Not inside the factory.** The factory builds one + `McpServer` per HTTP request and each round is a separate request, so a codec built there mints + round one under one per-process key and verifies round two under another, failing `mac` on every + confirmation. It only bites when `MCP_REQUEST_STATE_SECRET` is unset, which is the local dev path, + so it presents as "the feature is broken" rather than "the wiring is wrong". This is the single + most expensive mistake available in this commit. +- **`codec.verify` passes straight through** as `ServerOptions.requestState.verify` on the + `McpServer` constructor, beside `instructions`. The seam runs it before the handler on every round + whose echoed `requestState` is a string, and any throw becomes the frozen `-32602` + `"Invalid or expired requestState"`. The thrown reason (`malformed` / `mac` / `expired` / `bind`) + reaches the server's `onerror` only and never the wire, so operators can tell the cases apart and + clients cannot. +- **Minting:** `await codec.mint({ digest }, ctx)`, returned as + `inputRequired({ requestState, inputRequests })`. `ctx` is required because a `bind` is configured. +- **Reading:** `ctx.mcpReq.requestState<{ digest: string }>()`. The handler never calls `verify` + itself; the seam already did, and the accessor yields the decoded payload precisely because the + hook resolved with it. +- **`resolveConfirmation` changes shape.** Commit 2 left it as `(ctx, message) => Confirmation`. It + now also needs the digest of the query being confirmed, to mint on the first round and compare on + the second, so the mismatch and missing-state refusals sit beside the existing decline path rather + than in the caller. +- **Serialize the digest input deterministically.** `JSON.stringify` over + `{ endpoint, query, variables }` is insertion-ordered and stable within a build, which is + sufficient. The note exists so nobody substitutes a serializer that reorders keys and silently + breaks every re-entry. + +### The new environment variable + +`MCP_REQUEST_STATE_SECRET` follows the precedent commit 1 set for `MCP_ALLOWED_HOSTS`: a new variable +updates `.env.schema` and the README environment-variable table **in the same commit**, not in +commit 5. The README description should carry the reasoning that lives here, in particular that +leaving it unset is fine at one replica and breaks confirmations across several. + +**Done when:** a tampered `requestState` is refused rather than executed, an answer arriving with no +`requestState` is refused, and a query rebuilt differently from the one approved is refused. Each +pinned by a test. The unit suite is the place for all three, since none of them need Elasticsearch. + +### Commit 3 as built, 2026-09-04 + +Built and green: 331 unit tests (up from 313), 87 integration tests, nothing skipped. All three +done-when cases are pinned, and the plan's shape survived: payload is `{ digest }`, `bind` is the +documented method-plus-principal shape, the codec is built in `startServer`, and `codec.verify` is +passed straight through as `ServerOptions.requestState.verify`. + +**`ctx.http.authInfo` does exist**, contrary to what a reading of `ServerContext` alone suggests. +`ServerContext` intersects two `http` shapes, and `authInfo` is on the one declared in `BaseContext`, +so the documented `bind` expression type-checks unchanged. + +**A test-only export was avoided, and the wiring is asserted end to end instead.** Nothing public +reads back `ServerOptions.requestState.verify` (`Server._requestStateVerify` is private), so the +seam test drives the real HTTP endpoint with `fetch` and asserts a `requestState` altered in transit +is answered `-32602` with `data.reason: 'invalid_request_state'` before `execute_query` is entered. + +**An in-memory transport cannot reach the multi-round-trip flow at all**, which cost an hour to find +and is worth recording. A `Server` on `InMemoryTransport.createLinkedPair()` serves the 2025 era +regardless of what the request's `_meta` envelope claims: the negotiated revision is set by the HTTP +entry from the `Mcp-Protocol-Version` header, and `setNegotiatedProtocolVersion` is internal to +`@modelcontextprotocol/core` rather than exported. Every 2026-era unit test therefore goes over +`startMcpHttpServer` or drives the registered handler directly. + +**Driving `tools/call` by hand needs three headers, not one.** `Mcp-Protocol-Version` classifies the +era, and `Mcp-Method` and `Mcp-Name` are required of every modern call: without them the request is +answered `-32020` as a headers-versus-body mismatch, before any handler runs. This is SEP-2243 +enforcement at the HTTP entry, and it is easy to mistake for a malformed body. + +**The handler is fail-closed even if the seam hook is ever dropped.** With no `verify` configured the +accessor yields the raw wire string, whose `digest` is `undefined`, so an altered state is refused by +the handler's own check rather than executed. The hook still earns its place: it is what enforces +expiry and the binding, neither of which the handler can see. + +**`SERVER_INSTRUCTIONS` was corrected here rather than deferred to commit 5.** It still told the model +that a client without elicitation gets no prompt, describing the branch decision 8 replaced with a +refusal in commit 2, and it now also states that an approval covers one exact query. The +`query_arranger` prompt needed no change: its confirmation language is about the model's own +conversational step, not about elicitation. + +--- + +## Phase C, commit 4: configure what the revision added + +- **Cache hints.** `cacheHints?: Partial>` on the `McpServer` + constructor. The cacheable set is closed and verified: `tools/list`, `prompts/list`, + `resources/list`, `resources/templates/list`, `resources/read`, `server/discover`. `CacheHint` is + `{ ttlMs?, cacheScope? }`, and **invalid values throw a `RangeError` at construction**, so a wrong + value fails fast rather than at request time. The SDK default is `{ ttlMs: 0, cacheScope: 'private' +}`: compliant, but it throws the feature away. + + **Decision 6, SETTLED 2026-09-05: the hint values.** + + | Method | `ttlMs` | `cacheScope` | + | -------------------------- | ----------- | ------------ | + | `tools/list` | `3_600_000` | `public` | + | `prompts/list` | `3_600_000` | `public` | + | `resources/templates/list` | `3_600_000` | `public` | + | `server/discover` | `3_600_000` | `public` | + | `resources/list` | `60_000` | `private` | + | `resources/read` | `60_000` | `private` | + + **Verified 2026-09-05** by probe, as this section demanded, and the plan's own grouping was one of + the things the probe corrected. + + | Probe | Result | + | ----------------------------------------------------------------------- | --------------------------------------------------------------------------- | + | `ttlMs` of `-1`, `1.5`, `Infinity`, `MAX_SAFE_INTEGER + 1` | `RangeError` at construction | + | `ttlMs` of `0` or `MAX_SAFE_INTEGER` | accepted | + | `cacheScope` other than `public` / `private` | `RangeError` at construction | + | a method key that is not cacheable, e.g. `tools/call` | **accepted silently, and does nothing** | + | configured hint on each of the six cacheable methods | reaches the wire verbatim | + | per-resource `cacheHint` against the server-level `resources/read` hint | overrides **field by field**, keeping the unset field from the server level | + | hint returned by the handler on the result | beats both | + + **`ttlMs: 0` means do not cache**, not no expiry. The client SDK is explicit that a + `resources/read` result whose resolved TTL is at most zero is not stored at all, so the SDK default + really does throw the feature away. + + **A typo in a method key is invisible**, which decides how commit 4 tests this: assert the values + on the wire, never on the configuration object. + + **`resources/list` is not static per build**, contrary to the grouping this section previously + carried. `arranger_catalogue_fields` is a `ResourceTemplate` whose `list` callback calls + `client.getServerIntrospection()`, and the callback was measured running on every `resources/list`, + so the result enumerates whatever catalogues Arranger currently reports. It tracks Arranger's + configuration exactly as `resources/read` does and takes the same values. The other four are + genuinely build-static: tool and prompt descriptions are literals, with every live introspection + call inside a handler rather than in a registration, and `server/discover` carries + `SERVER_INSTRUCTIONS` plus capabilities. + + **The scope is split rather than `public` for both**, which is a change from what this section + proposed. The fact that makes it cheap: **`private` does not mean "do not cache", it means "do not + share"**. The client still caches, partitioned by principal, so the benefit we actually want, one + agent re-reading catalogue fields repeatedly inside a session, survives intact. `public` would only + add cross-principal sharing at a shared gateway, which is speculative value for a single-tenant + unauthenticated deployment. + + Against that, the risk is concrete rather than hypothetical: catalogue introspection already + carries `meta.authFiltered`, so Arranger has a per-caller-filtered mode today. We neither read it + nor forward auth headers, so we see one fixed view; the day either changes, `public` on those two + is a cross-tenant leak. The spec is explicit that `cacheScope` is not access control and must never + be the thing keeping one tenant's data from another, so the version of this decision that needs + revisiting when auth lands is the version worth avoiding. The four build-static methods can never + be per-caller, being literals compiled into the process, so `public` there is unconditionally safe + and stays safe. + + **On the numbers.** An hour for build-static is a horizon where being wrong is cheap: a client + holding a stale tool list across a redeploy gets a clean error, and the client SDK has an + evict-refetch-retry path for tool schema drift specifically. Serving per request means + `listChanged` cannot reach anyone, so TTL expiry is the only correction mechanism, which argues + against much longer. Development staleness is answered by the Inspector's `refresh`, not by + shortening the production hint. A minute for the pair that tracks Arranger means a catalogue change + is visible without restarting this server, while still collapsing the burst of reads one agent + session makes. + + **Deferred, deliberately:** `arranger_sqon_schema` tracks Arranger's build rather than its + catalogue configuration, so a longer per-resource `cacheHint` would fit it. It is one line and a + real difference in volatility, but it adds a third ttl for a resource read once per session. Leave + it out of commit 4. + +- **`serverInfo.version`** is hardcoded `'0.0.0-dev'` in `server.ts` and now appears on **every** + result. Note the release process pins `main` at that placeholder, so the fix is to read the field, + not to hardcode a different string. **Settled 2026-09-05**, verified against Node 24 and `tsc`: + + ```ts + import packageJson from '../package.json' with { type: 'json' }; + new McpServer({ name: 'arranger-mcp-server', version: packageJson.version }, { ... }); + ``` + + - **`with`, never `assert`.** `assert { type: 'json' }` was removed in Node 22 and is a hard + `SyntaxError` on 24. It nonetheless appears to work here, because `tsx` strips it before Node + sees it, so it would pass `npm start` and `npm test` and fail the moment anything ran the file + through Node directly. + - **No tsconfig change.** `module: nodenext` implies `resolveJsonModule`, which `tsc --showConfig` + confirms is already on, and the import is typed rather than `any`: assigning `packageJson.version` + to a `number` fails with `TS2322`. An earlier note in this plan claiming the flag was needed was + wrong. + - **`../package.json`, not `../../`.** Resolved from `src/server.ts`, one level up is the app's own + manifest; two would reach `apps/package.json`. + - **Not `process.env.npm_package_version`**, which is populated only when the process is launched + through an npm script and is `undefined` under Docker or a process manager. + - The relative path assumes the app keeps running from source, which it does today (`start` is + `tsx ./src/index.ts`, with no build step). A `dist/` would change the depth. + +- **`server/discover`** is SDK-provided but MUST be implemented. Add a contract test that it reports + our capabilities and instructions. `SERVER_INSTRUCTIONS` stops being an `initialize` field and + becomes part of this result. +- **Deterministic `tools/list` ordering** is a SHOULD. Our registration order is fixed and the + catalogue filter is process-wide rather than per-client, so we already comply; pin it with a test. +- **Optional, and now DECLINED rather than deferred.** Annotating `catalogueId` with `x-mcp-header` + would let a gateway route on it without parsing bodies, but it is **not additive**, which this + plan assumed it was. Measured 2026-09-05: once a property carries the annotation, + `validateMcpParamHeaders` runs pre-dispatch and rejects any call whose body carries that property + without the matching `Mcp-Param-*` header, with `-32020` and HTTP `400`. It is a requirement on + callers, not a hint. Every client that does not mirror parameters into headers would break, in + exchange for a routing capability no deployment currently wants. Revisit only alongside a gateway + that needs it. + +**The test harness already exists.** `src/server.test.ts`, added in commit 3, drives the real HTTP +endpoint with `fetch` and is what the `server/discover` contract test, the `tools/list` ordering test +and the cache-hint wire assertions all need. Reuse it rather than building a second one, and note +that a hand-driven modern call needs `Mcp-Protocol-Version`, `Mcp-Method` and `Mcp-Name` headers or +it is answered `-32020` before any handler runs. + +**Done when:** `server/discover` reports our capabilities and instructions, `tools/list` order is +pinned, `serverInfo.version` comes from `package.json` rather than a literal, and the configured +cache hints appear on a cacheable result. Each pinned by a test in the unit suite. + +### Commit 4 as built, 2026-09-05 + +Built and green: 340 unit tests (up from 331), 87 integration tests, nothing skipped. The settled +hint values, the manifest read and the two contract tests all landed as specified. + +**The silent-typo hole closed at compile time rather than only in a test.** `CacheableResultMethod` +is not exported, but `ServerOptions` is, so typing the constant as +`NonNullable` gets the closed key union anyway and excess-property +checking rejects a non-cacheable key: `'tools/call'` now fails with `TS2353` rather than being +accepted and ignored. The wire assertions stay, because the type proves the SDK accepted the hint +and not that a client receives it. + +**The cache-hint test restates the six values rather than importing the constant.** Comparing the +wire against the same object that configures it would pass whatever the values drifted to. A +mutation check confirms the six fail when `cacheHints` is removed from the constructor. + +**The version test is weaker than it reads, deliberately and with the limit written down.** `main` +pins the manifest at `0.0.0-dev`, so today it cannot distinguish a manifest read from a literal that +matches; the same mutation check saw it pass with the literal restored. It bites on a release build, +which is the case that matters, and it also asserts `serverInfo` is stamped on more than one result +type, which is the reason the version stopped being cosmetic. + +**`capabilities.prompts` is advertised**, which an early probe suggested it might not be: the probe +had registered no prompt. Against the real surface `server/discover` reports tools, resources and +prompts. + +--- + +## Phase C, commit 5: docs + +- `README.md` states v1.x explicitly and says nothing about which revision it serves; it now needs + both, plus the requirement that a consumer opt into modern negotiation. Its folder-structure block + still lists `http/app.ts` and `utils/inMemoryEventStore.ts`, deleted in commit 1, and still omits + everything added since: `http/server.ts`, `http/requestBody.ts`, and the test files beside them. + Its `execute_query` row should state that a client which cannot elicit is refused (decision 8). + **The environment-variable table is not commit 5's job**: commits 1 and 3 each update it as they + introduce their variables, so by the time this commit runs it should already be current, and the + work here is to check that rather than to write it. +- **Decision 7, SETTLED 2026-09-03:** a compatible Inspector exists, so unpin rather than drop. + `@modelcontextprotocol/inspector@2.5.0` is current (`latest`; `v1-latest` is `1.0.2`) and is built + on `@modelcontextprotocol/{client,server,core}@2.0.0`. Move the `inspect` script in + `apps/mcp-server/package.json` from `@modelcontextprotocol/inspector@1` to `@2`, and add + `"protocolEra": "modern"` to the server entry in `mcp-inspector.json`: the Inspector maps that + setting through `eraToVersionNegotiation` and, like the client library, defaults it to + `{ mode: "legacy" }`, so without it the Inspector connects 2025-era and decision 3's endpoint + refuses it. +- `CHANGELOG.md`: **2025-era clients are no longer served at all** (the headline, decision 3), the + transport change, the confirmation flow becoming two requests, `execute_query` refusing clients + that cannot elicit (decision 8), the approved query becoming integrity-bound with the optional + `MCP_REQUEST_STATE_SECRET` behind it (decision 5), the new required `MCP_ALLOWED_HOSTS` (with its + startup fail-fast) and the `403`-on-any-non-localhost-`Host` fix are all operator-facing. Say plainly that any + consumer must opt into modern negotiation, since no SDK client does so by default. +- `.dev/docs/mcp-platform-testing.md`: see below. + +**Done when:** nothing in `apps/mcp-server` still describes the v1 SDK or the 2025 era, the +folder-structure block matches the tree, `npm run mcp-server:inspect` connects, and `CHANGELOG.md` +names every operator-facing change from commits 1 to 4. + +### Commit 5 as built, 2026-09-05 + +**Decision 7 re-verified against the shipped Inspector 2.5.0** rather than taken on the earlier +reading: `protocolEra` is read off the per-server `mcpServers` entry, `"modern"` maps to +`{ mode: { pin: "2026-07-28" } }`, and the source comments that an absent era "defaults to legacy". +So the setting is required, as decision 7 said. The `inspect` script was already on `@2` from +commit 1, leaving only the config key. + +**The folder block names sources only**, with one line saying tests are co-located, rather than +listing a dozen `*.test.ts` entries as this section's wording implied. Enumerating them would have +doubled the block to restate a convention. + +**`npm run mcp-server:inspect` connects**, confirmed by hand against a live Arranger on 2026-09-05. +It launches an interactive browser UI, so this is the one done-when condition no test covers. + +**`.dev/docs/mcp-platform-testing.md` was deliberately not touched**, per the knock-on section below: +it is owned elsewhere, and its five amendments belong to that owner rather than to a unilateral edit +from this commit. That handoff is the only outstanding item in this plan. + +--- + +## Knock-on: the MCP platform testing plan + +Three of its pinned dimensions use vocabulary the revision removes. Hand these to that document's +owner rather than editing it unilaterally. + +- **The surface hash** is defined as the `initialize` instructions plus the serialized lists. + `initialize` is gone; `server/discover` carries instructions, capabilities, and supported versions + in one place, and is a better hash source than what the plan describes. +- **Declared capabilities are no longer a session property.** They arrive per request, so the harness + pins a per-request envelope and can legitimately vary it call by call. +- **L1's elicitation round trip** becomes a two-request exchange with an opaque state token between. + Both halves are plain request/response, which makes it a better L1 test, and the assertion shape + changes. Asserted once, not twice: decision 3 leaves one era, and the legacy shim was never + reachable from a stateless endpoint anyway. +- **Era pinning is now a harness precondition.** Every SDK client defaults to 2025-era negotiation, + so the harness must pin `2026-07-28` explicitly or it will measure a server that refuses it. +- **Cache hints are a new L1 surface**, since `ttlMs` and `cacheScope` are part of what we publish. + +--- + +## What the revision changes, in full + +Nine breaking changes; five land on our code. + +| Change | Kind | Hits | Response | +| ----------------------------------------------------------------------------------------- | ---------- | -------------------------------------------- | ----------------------------------------------------- | +| Protocol sessions and `Mcp-Session-Id` removed. No GET stream, DELETE, or `Last-Event-ID` | gone | `http/app.ts`, `utils/inMemoryEventStore.ts` | both deleted | +| Server-initiated requests removed; elicitation rides MRTR | gone | `mcp/executeQueryTool.ts` | phase B | +| `initialize` / `notifications/initialized` removed; every request carries `_meta` | shift | `server.ts`, `mcp/instructions.ts` | SDK-handled | +| `server/discover` MUST be implemented | new | | SDK-provided; add a contract test | +| `ttlMs` / `cacheScope` required on cacheable results | new | `server.ts` | decision 6 | +| All results carry `resultType` | shift | | SDK-handled at the codec | +| `Mcp-Method` / `Mcp-Name` headers required, `-32020` on mismatch | new | | SDK-handled | +| `resources/subscribe` and GET stream become `subscriptions/listen` | shift | | free: we advertise no subscriptions | +| `ping`, `logging/setLevel`, `notifications/roots/list_changed` removed | gone | | unused; Pino already writes to process streams | +| Resource-not-found moves `-32002` to `-32602` | shift | `http/app.ts` | SDK-handled; our `-32000` goes with the sessions code | +| List results must not vary per connection; `tools/list` SHOULD be ordered | already ok | `mcp/tools.ts` | pin with a test | + +**Not a concern:** the `Task` / `TaskStatus` / `ListTasks` exports in `server@2` are marked +`@deprecated 2025-11-25 wire vocabulary with no SDK runtime; kept importable for interoperability +only`. They are not new work. + +--- + +## Decisions, collected + +| # | Decision | Blocks | Status | +| --- | ---------------------------------------------------------------- | -------------------- | ----------------------------------------------------------------------------------------------------------------------------------------- | +| 1 | How the handler reaches Express, or whether Express stays at all | commit 1, everything | **settled 2026-09-03: Express dropped, `toNodeHandler` on `node:http`** | +| 2 | Which Express major, if it stays | commit 1 | **moot, closed by decision 1** | +| 3 | Whether to serve 2025-era clients at all | commit 1, commit 2 | **settled 2026-09-03: `legacy: 'reject'`, one revision only** | +| 4 | Re-entry budget and its exhaustion message | commit 2 | **moot, closed by decision 3** | +| 5 | What goes into `requestState` | commit 3 | **settled 2026-09-04: `{ digest }` of the built query, bind on method plus principal, per-process key by default** | +| 6 | Cache hint values and scopes | commit 4 | **settled 2026-09-05: one hour and `public` for the four build-static methods, one minute and `private` for the two that track Arranger** | +| 7 | Inspector pin, if no compatible release exists | commit 5 | **settled 2026-09-03: unpin to `@2`, set `protocolEra: "modern"`** | +| 8 | `execute_query` when a modern client cannot elicit | commit 2 | **settled 2026-09-03: refuse the call** | + +Every commit is built and every decision is settled. + +Separately from the decisions above, commit 1's review left three judgment calls that are accepted +but revisitable rather than settled: the startup fail-fast being stricter than the SDK, the body-cap +default preserving `100kb` rather than being measured, and whether `MCP_ALLOWED_ORIGINS` earns its +place. They are recorded in the as-built section, not here, because they are not gates on anything. + +Decision 3 grew in scope while being settled, from "sign off on lost resumability" to "serve one +revision only". Whoever owns the sign-off should be told it changed, and why: the legacy path could +not have carried confirm-before-execute regardless, so the choice was never between full and degraded +legacy support. + +--- + +## Provenance + +Package facts, API names, option shapes, and the cacheable-method list were read on 2026-09-03 from +the shipped `.d.mts` declarations of `@modelcontextprotocol/server@2.0.0` and +`@modelcontextprotocol/express@2.0.0`, installed into a scratch project. Revision semantics come from +the "Crossing the Era Boundary" spec review, with its two errors corrected above: the Express bridge +does not live in the Express adapter, and Tasks are not new work. + +Decision 1 was settled the same day against additional evidence, all reproducible: + +- `@modelcontextprotocol/node@2.0.0` and `@modelcontextprotocol/client@2.0.0` added to the same + scratch project, and their `dist/index.d.mts` and `dist/index.mjs` read directly. `toNodeHandler` + does not call `getRequestListener`; the `@hono/node-server` import is module-level and its main + entry does not import `hono`. +- `npm ls hono` and `npm ls express --workspaces` in this repo, showing `hono@4.12.23` and + `@hono/node-server@1.19.14` already present under `@modelcontextprotocol/sdk@1.29.0`. +- Four clean-room installs (`sdk@1.29.0`; `server`+`node`; `server`+`express`+express 5; `server` + alone) for the package-count and size table. +- The `Host` header behaviour probed against the installed v1 SDK with raw `node:http` requests. + Note that `fetch` silently drops a `Host` header, so a fetch-based probe reports a false pass. +- v1's `dist/esm/server/auth/` compared against v2's exports, showing `mcpAuthRouter`, + `OAuthServerProvider` and the `authorize`/`token`/`register`/`revoke` handlers are gone in v2. + +Two later rounds of evidence, both by probe against a running server rather than by reading +declarations, after the declarations proved unreliable: + +- **2026-09-04, commit 2.** A tool dumping `ctx.mcpReq` across both rounds, which is how the envelope + correction was found: the reserved `io.modelcontextprotocol/*` keys arrive verbatim rather than + lifted to friendly names. The same probe confirmed the handler re-runs from the top on re-entry, + and that a client declaring no `elicitation` makes the SDK itself refuse an `inputRequired` return. +- **2026-09-04, decision 5.** `createRequestStateCodec` exercised directly for every failure mode + (`mac`, `bind`, `expired`, `malformed`), plus end to end through `ServerOptions.requestState.verify` + to confirm the seam decodes before the handler and `ctx.mcpReq.requestState()` yields the payload. + The wire value was base64url-decoded to confirm the payload is signed but readable. + +Decision 3 was settled the same day by experiment rather than by reading, which is why it overturned +the plan's own premise. Both scripts live in the scratch project and are worth re-running against any +future SDK release: + +- A `createMcpHandler` server with one `inputRequired(...)` tool, served at `legacy: 'stateless'` and + again at `legacy: 'reject'`, probed by two `@modelcontextprotocol/client@2` instances: one at its + default negotiation, one at `{ pin: '2026-07-28' }`. This produced the refusal message and the + `-32022` payload quoted in decision 3. +- The same server with a plain tool, a resource and a prompt, probed both ways, which produced the + works/does-not-work table. `GET` and `DELETE` were probed directly for the 405s. +- `@modelcontextprotocol/inspector@2.5.0` unpacked and grepped, confirming it is built on the v2 SDK + and that its `protocolEra` server setting maps through `eraToVersionNegotiation`, defaulting to + `{ mode: "legacy" }`. diff --git a/.dev/sessions/2026-09-04T220158.md b/.dev/sessions/2026-09-04T220158.md new file mode 100644 index 000000000..04622e6e7 --- /dev/null +++ b/.dev/sessions/2026-09-04T220158.md @@ -0,0 +1,8 @@ +Built commit 3 of the MCP SDK v2 migration: `execute_query`'s confirmation is now bound to the query it approved, rather than to nothing. + +- **The digest covers the built query, not the tool arguments.** `requestState` carries a SHA-256 of `{ endpoint, query, variables }`, which is what the confirmation message displayed and what actually runs. Digesting the arguments instead would have missed drift the arguments cannot show: introspection is fetched again on the second round, so a catalogue reconfigured between rounds can build a different query from identical input and run it under the old approval. +- **Both refusal paths are refusals, not re-asks.** A mismatched digest and an absent `requestState` are answered with an error rather than a fresh confirmation request. Re-asking would hand a caller an unlimited retry loop against the gate. The absent case is the load-bearing one: nothing in the protocol forces a client to echo the state, so comparing only when it happens to be present would have made the entire binding opt-out at the caller's discretion. +- **The codec is built once per process and passed into the server factory.** The factory runs per HTTP request and a confirmation spans two of them, so a codec built there would mint round one under one key and verify round two under another. That only bites when no secret is configured, which is the local development path, so it would have presented as "the feature is broken" rather than "the wiring is wrong". A unit test pins the shape of it: two codecs built with no secret reject each other's state. +- **`MCP_REQUEST_STATE_SECRET` is optional on purpose.** Unset, the server signs with a key generated for the process, which is secure at one replica and costs no configuration; it fails across several, and across a restart, so the startup warning names that consequence rather than the absence. Zod refuses anything shorter than 32 bytes at startup, where it reads as a misconfigured variable, instead of letting the codec throw a `RangeError` from inside server startup. +- **The seam test goes over real HTTP rather than an in-memory transport.** A linked transport pair serves the 2025 era: the negotiated revision is set by the HTTP entry from the `Mcp-Protocol-Version` header, and the helper that sets it is not publicly exported, so a call over `InMemoryTransport` never reaches the multi-round-trip flow at all. Driving the endpoint with `fetch` also allows sending a `requestState` altered in transit, which no well-behaved client would. +- **`SERVER_INSTRUCTIONS` had been wrong since commit 2 and was corrected here.** It still told the model that a client without elicitation support gets no prompt, so intent should be confirmed in conversation, which described the branch decision 8 replaced with a refusal. Now it states that there is no path on which the query runs unseen, and adds what commit 3 makes true: an approval covers one exact query, so changing the arguments between the question and the answer is refused rather than executed. The `query_arranger` prompt needed no change; its confirmation language is about the model's own conversational step, not about elicitation. diff --git a/.dev/sessions/2026-09-05T000000.md b/.dev/sessions/2026-09-05T000000.md new file mode 100644 index 000000000..cca17d991 --- /dev/null +++ b/.dev/sessions/2026-09-05T000000.md @@ -0,0 +1,27 @@ +Settled the last open decision on the MCP SDK v2 migration and cleared two obsolete tech-debt entries the migration had already invalidated. + +- **Decision 6 (cache hint values) settled by probe rather than by reasoning, and the probe corrected the plan twice.** `resources/list` had been filed as static per build, but `arranger_catalogue_fields` is a `ResourceTemplate` whose `list` callback calls `getServerIntrospection()`, measured running on every `resources/list`, so its result tracks Arranger's catalogue inventory and takes the same short ttl as `resources/read`. Settled at one hour and `public` for the four genuinely build-static methods, one minute and `private` for the two that track Arranger. +- **The scope split turned on a fact that makes it nearly free.** `private` does not mean "do not cache", it means "do not share": the client still caches, partitioned by principal, so intra-session reuse survives intact and only cross-principal sharing at a shared gateway is given up. Against that, catalogue introspection already carries `meta.authFiltered`, so Arranger has a per-caller-filtered mode today; `public` on those two would become a cross-tenant leak the moment we forward auth, and the spec is explicit that `cacheScope` is not access control. Choosing the version that needs no revisiting beat choosing the one with a note attached. +- **A cache-hint key that is not cacheable is accepted silently and does nothing**, which decides how commit 4 tests this: assert the values on the wire, never on the configuration object. `ttlMs: 0` was also confirmed to mean "do not cache" rather than "no expiry", so the SDK default really does throw the feature away. +- **`assert { type: 'json' }` passes here only because a transpiler hides it.** It was removed in Node 22 and is a hard `SyntaxError` on 24, but `tsx` strips it before Node sees it, so a JSON import written that way would survive `npm start` and `npm test` and break the moment anything ran the file through Node directly. `with { type: 'json' }` is the current spelling and needs no tsconfig change: `module: nodenext` already implies `resolveJsonModule`, which corrects an earlier claim in the plan that the flag would have to be added. +- **The introspection-caching thread was orphaned and now has an owner.** Commit 2 doubled the Arranger round trips a confirmed query costs, from two to four, and the plan noted that caching had become load-bearing without any commit picking it up. It is a server-side concern that predates the migration, so it went to `.dev/tech-debt.md` under `apps/mcp-server` with a pointer to the roadmap entry that already frames the ttl-versus-schema-hash choice, rather than being pulled into this PR. +- **Two `apps/mcp-server` tech-debt entries were removed as obsolete rather than fixed.** Both named files commit 1 deleted (`utils/inMemoryEventStore.ts` and `http/app.ts`), and per-request stateless serving removed the session map the second described. Recorded because the pattern generalizes: a migration that deletes a file silently invalidates every tech-debt entry pointing at it, and nothing in the workflow surfaces that. + +Implemented Phase C, commit 4: cache hints, the manifest-read `serverInfo.version`, and the two contract tests the revision asks for. + +- **The silently-ignored cache-hint key is now a compile error, not just a test.** `CacheableResultMethod` is unexported, but `ServerOptions` is, so typing the constant as `NonNullable` picks up the closed key union and excess-property checking rejects a non-cacheable key with `TS2353`. The wire assertions still earn their place: the type proves the SDK accepted a hint, not that a client receives one. +- **The cache-hint test restates the six values instead of importing the constant that configures them**, since comparing the wire against its own source would pass whatever the values drifted to. A mutation run confirmed all six fail when `cacheHints` is removed from the constructor. +- **The `serverInfo.version` test is weaker than it reads and says so in place.** The manifest is pinned at `0.0.0-dev` on `main`, so it cannot currently tell a manifest read from a matching literal; the same mutation run saw it pass against the literal. It bites on a release build, which is the case that matters, and it was widened to assert `serverInfo` is stamped on more than one result type. +- **The optional `x-mcp-header` item was declined rather than deferred, on evidence.** The plan had it as additive. It is not: once a property carries the annotation, `validateMcpParamHeaders` rejects pre-dispatch any call whose body carries that property without the matching `Mcp-Param-*` header, with `-32020` and HTTP `400`. Measured both ways, including a wrong guess at the header encoding. It is a requirement on callers in exchange for a gateway capability no deployment wants, so it should not be picked up later on the belief that it is free. + +Implemented Phase C, commit 5: the docs, the changelog and the Inspector config. + +- **`protocolEra: "modern"` was re-verified against the shipped Inspector 2.5.0 rather than trusted from the plan.** It is read off the per-server `mcpServers` entry, maps to `{ mode: { pin: "2026-07-28" } }`, and the source states that an absent era defaults to legacy, so without it the Inspector connects 2025-era and our endpoint refuses it. +- **`npm run mcp-server:inspect` connects**, confirmed by hand against a live Arranger. It launches an interactive browser UI, so it stays the one done-when condition no test covers. +- **The folder block names sources only, with one line noting tests are co-located.** The plan implied listing the test files; that would have doubled the block to restate a convention. +- **`.dev/docs/mcp-platform-testing.md` was left alone on purpose.** Its five amendments belong in step 9's handoff, since that document is owned elsewhere, and that handoff is now the only outstanding item in the migration plan. + +Moved the upgrade plan into the repo and wrote a short companion doc. + +- **The plan became `.dev/docs/mcp-sdk-v2-upgrade-plan.md`**, retitled, marked complete, and cut free of the parent migration's step numbering, which meant nothing outside the drafts folder. Two links resolved through a machine-specific path and now resolve repo-relative, which is the class of thing that only surfaces when a working document crosses into a committed one. +- **`.dev/docs/mcp-sdk-v2-changes.md` is the short read**: the concepts and the decisions a developer needs, without the alternatives rejected or the probe evidence. The plan stays the record for those, and each points at the other. diff --git a/.dev/tech-debt.md b/.dev/tech-debt.md index f7199775c..1d0eb6609 100644 --- a/.dev/tech-debt.md +++ b/.dev/tech-debt.md @@ -239,23 +239,14 @@ Two consumers need updating in the same pass: `dev:check` (which just calls `tes ## apps/mcp-server -### `InMemoryEventStore` is not suitable for production - -**File:** `apps/mcp-server/src/utils/inMemoryEventStore.ts` -**Severity:** medium (data reliability: state is lost on restart; no session resumability for clients) -**Kind:** placeholder / incomplete implementation -**Issue:** The `InMemoryEventStore` is copied verbatim from the MCP TypeScript SDK examples and is explicitly documented as intended for examples and testing, not production. It stores SSE event history in a `Map` in process memory, so all session state is lost on any restart or crash, and there is no mechanism for clients to replay missed events across server restarts. -**Fix:** Replace with a persistent store (e.g. Redis, a database-backed event log) before any production deployment. The `EventStore` interface from `@modelcontextprotocol/sdk/server/streamableHttp` is already the right abstraction; only the implementation needs to change. -**Standalone:** yes; swap the implementation behind the existing `EventStore` interface; no changes to `app.ts` or the MCP server wiring - -### MCP session map does not evict abandoned sessions - -**File:** `apps/mcp-server/src/http/app.ts` -**Severity:** low (memory leak under adversarial or high-traffic conditions) -**Kind:** resource management -**Issue:** The `transports` map in `createHttpApp` is cleaned up when a client sends `DELETE` (via `onclose`) or on graceful shutdown. If a client disconnects without sending `DELETE` (network drop, crash), the transport entry persists for the lifetime of the process. For a low-traffic introspection server this is unlikely to matter in practice, but under adversarial conditions or bursty usage the map grows without bound. -**Fix:** Track a `lastSeenAt` timestamp per transport entry and update it on every request that resolves an existing session. Run a `setInterval` sweep (e.g. every 5 minutes) to close and evict sessions idle beyond a configurable TTL (e.g. 30 minutes). The sweep should call `transport.close()` before deleting the entry to ensure clean teardown. -**Standalone:** yes; self-contained change to `app.ts`; no protocol or API surface changes +### Arranger introspection is re-fetched on every call, with no cache and no sharing between callers + +**File:** `apps/mcp-server/src/arranger/client.ts`; `apps/mcp-server/src/mcp/executeQueryTool.ts`, `tools.ts`, `resources.ts` +**Severity:** medium (upstream load amplification; grew rather than appeared with the MCP SDK v2 migration) +**Kind:** missing optimization +**Issue:** `ArrangerClient` holds no cache, so every consumer that needs schema information asks Arranger for it again. `execute_query` alone calls `getServerIntrospection()` and `getCatalogueIntrospection()` on each entry, and protocol revision `2026-07-28` made confirmation a two-request exchange in which the handler re-runs from the top, so a single confirmed query now costs four introspection round trips where it cost two. Nothing is shared between the tools, the resources and `execute_query` even inside one request, and the payloads are identical for every caller today. The MCP server exists to sit in front of Arranger, which makes this its most direct cost. +**Fix:** Cache introspection responses in `ArrangerClient` keyed by endpoint. Read [roadmap: query result caching](roadmap.md#query-result-caching-research) first rather than reinventing the choice it already frames: a short TTL of 30 to 60 seconds is that entry's option (b) and is self-contained, while tying entries to an ES/OS schema hash is option (a), is more correct, and depends on the ETag/schema-hash invalidation signal still open under [roadmap: MCP integration readiness](roadmap.md#mcp-integration-readiness). Note that the protocol's own `cacheHints` do not help here: those are client-side freshness hints on our results, not a cache in front of Arranger. +**Standalone:** yes for the TTL version, which needs no Arranger change; the schema-hash version does not ### `NUMERIC_AGGREGATION_TYPES` in queryBuilder duplicates `esToAggTypesMap` from `modules/types` diff --git a/CHANGELOG.md b/CHANGELOG.md index 0701eb4e0..473b97965 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,10 @@ This file covers high-level release notes for the Arranger project as a whole. W - **`execute_query`'s `sqon` argument is now enforced as required**: omitting it fails schema validation instead of reaching the handler, since Zod 4 treats an `unknown()` key as required. The tool already documented it as required, so this closes a gap rather than changing intent, and the error text is unchanged: it still names the fix and the empty-root-SQON form. +- **The MCP server serves protocol revision `2026-07-28` only, and refuses 2025-era clients**: it now runs on v2 of the MCP TypeScript SDK. Every SDK client negotiates the 2025 era by default, so a consumer must opt into modern negotiation explicitly or the endpoint refuses the connection with an unsupported-protocol-version error. The MCP Inspector needs `"protocolEra": "modern"` in its server entry. Serving both eras was not an option: the 2025 path cannot carry `execute_query`'s confirmation step. +- **`MCP_ALLOWED_HOSTS` is required whenever `MCP_HOST` is not loopback**: the server exits at startup rather than bind a routable interface without DNS rebinding protection. Set it to the hostnames clients use, or to `*` if an upstream gateway validates `Host`. Related fix: a non-localhost `Host` header was previously accepted; it is now refused with `403`. +- **`execute_query` refuses a client that does not declare the `elicitation` capability**: the generated query must be confirmed before it runs, and a client that cannot present that request is now refused rather than served unconfirmed. + - **MCP tool input schemas no longer advertise `additionalProperties: false`**: Zod 4 omits it where Zod 3 emitted it. Unrecognized properties are still stripped at runtime, so this loosens what `tools/list` advertises, not what the server accepts. Clients doing strict-mode function calling against the advertised schema should re-check. - **SQON nesting is now capped**: `SqonSchema` rejects anything deeper than `SQON_MAX_DEPTH` (128 in raw JSON nesting, 62 nested combinations) instead of throwing an uncaught `RangeError` out of `safeParse`. Real queries nest 2 to 4 levels, so no realistic query comes close. `SQON_MAX_DEPTH` and `checkSqonDepth()` are exported so callers can apply a stricter limit of their own. @@ -81,6 +85,12 @@ See [docs/reference/08-Migration/v3.1.md](docs/reference/08-Migration/v3.1.md) f - **`execute_query` addresses fields whose raw names GraphQL can't use as identifiers**, matching the server-side support noted under [Server](#server) above. `fields`, `sort`, and `aggregationFields` take names exactly as `get_catalogue_fields` reports them, including hyphens and leading digits. Results are keyed by the names the generated GraphQL schema uses, which are not always the same string: `donor-info.age-at-diagnosis` comes back as `donor_info { age_at_diagnosis }` under `hits` and as `donor_info__age_at_diagnosis` under `aggregations`. Field names inside a `sqon` are never rewritten in either direction, since a SQON travels as a query variable rather than as part of the query document. See [docs/mcp-server.md](docs/mcp-server.md). +- **Express is no longer a dependency**: the MCP handler is served on plain `node:http`, removing Express and its transitive packages from the image and its audit surface. Host and Origin validation and the request body size cap are unchanged in behaviour. +- **Confirming a query is now two requests rather than one**: revision `2026-07-28` removes the server-to-client request channel, so `execute_query` returns an `input_required` result and the client re-invokes the tool with the answer attached. Hosts using an SDK client need no change; a hand-rolled client must echo `requestState` verbatim on the retry. +- **The confirmed query is bound to the query that runs**: the approval carries a signed digest of the built GraphQL document, its variables and its endpoint, so a call that rebuilds a different query under the same approval is refused. New optional `MCP_REQUEST_STATE_SECRET`; unset, the server signs with a key generated for the process, which is correct at a single replica and fails across several. See [apps/mcp-server/README.md](apps/mcp-server/README.md#environment-variables). +- **New `MCP_ALLOWED_ORIGINS` and `MCP_MAX_BODY_BYTES`**: browser origins allowed to call the server, and the largest request body accepted (default `102_400`, preserving the limit Express applied before the transport moved to plain `node:http`). +- **Cacheable results now publish freshness hints**: `tools/list`, `prompts/list`, `resources/list`, `resources/templates/list`, `resources/read` and `server/discover` carry `ttlMs` and `cacheScope`, so a client caches them instead of re-fetching. Results that track Arranger's catalogue configuration are scoped `private` and expire in a minute; the rest are `public` for an hour. `serverInfo.version`, which this revision stamps on every result, now comes from the package manifest. + ### Charts (`@overture-stack/arranger-charts`) The charts module was introduced in this release cycle as a new package. diff --git a/apps/mcp-server/.env.schema b/apps/mcp-server/.env.schema index 6525e2843..a4a3fdde8 100644 --- a/apps/mcp-server/.env.schema +++ b/apps/mcp-server/.env.schema @@ -5,5 +5,9 @@ ARRANGER_REQUEST_TIMEOUT_MS=10_000 MCP_HOST=0.0.0.0 MCP_PORT=3100 MCP_PATH=/mcp +MCP_ALLOWED_HOSTS=localhost,127.0.0.1 +MCP_ALLOWED_ORIGINS=localhost,127.0.0.1 +MCP_REQUEST_STATE_SECRET= +MCP_MAX_BODY_BYTES=102_400 LOG_LEVEL=info diff --git a/apps/mcp-server/README.md b/apps/mcp-server/README.md index b15e96244..5c98359c8 100644 --- a/apps/mcp-server/README.md +++ b/apps/mcp-server/README.md @@ -2,24 +2,26 @@ This app is an MCP server that learns how to talk to Arranger by consuming Arranger's introspection endpoints. -The current scaffold implements the Streamable HTTP MCP transport using **v1.x** of the official [MCP TypeScript SDK](https://github.com/modelcontextprotocol/typescript-sdk/tree/v1.x). +It serves the Streamable HTTP transport on **v2** of the official [MCP TypeScript SDK](https://github.com/modelcontextprotocol/typescript-sdk), speaking protocol revision **`2026-07-28`**. ## Tools The server registers five tools that cover the full query lifecycle: -| Tool | Purpose | -| ---------------------- | -------------------------------------------------------------------------------------------------------------------------------------- | -| `list_catalogues` | Returns the catalogues the connected Arranger exposes. | -| `get_sqon_schema` | Returns a compact SQON quick reference (grammar, operators, worked examples) plus the full machine-readable SQON JSON Schema. | -| `get_catalogue_fields` | Returns field introspection for one catalogue: each field's type, display name, unit, description, and valid operators. | -| `build_sqon` | Builds a validated SQON from plain field, operator, and value clauses, with a plain-English summary. Builds only; it executes nothing. | -| `execute_query` | Builds, confirms, and executes a SQON-filtered query against a catalogue and returns the matching records. | +| Tool | Purpose | +| ---------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `list_catalogues` | Returns the catalogues the connected Arranger exposes. | +| `get_sqon_schema` | Returns a compact SQON quick reference (grammar, operators, worked examples) plus the full machine-readable SQON JSON Schema. | +| `get_catalogue_fields` | Returns field introspection for one catalogue: each field's type, display name, unit, description, and valid operators. | +| `build_sqon` | Builds a validated SQON from plain field, operator, and value clauses, with a plain-English summary. Builds only; it executes nothing. | +| `execute_query` | Builds, confirms, and executes a SQON-filtered query against a catalogue and returns the matching records. Requires a client that supports elicitation, and refuses one that does not, since the query must be confirmed before it runs. | The intended call order is `list_catalogues` → `get_catalogue_fields` → `build_sqon` → `execute_query`, which is what `SERVER_INSTRUCTIONS` and the `query_arranger` prompt both describe. `build_sqon` covers every operator `modules/sqon` implements: the single-field operators (`in`, `not-in`, `some-not-in`, `all`, `gt`, `gte`, `lt`, `lte`, `between`) with `fieldName`, and `wildcard` text search across several fields with `fieldNames`. Mixed combinators and the planned `fuzzy` operator are not supported, so those still need a hand-written `sqon` passed to `execute_query`. ## Folder Structure +Tests are co-located (`*.test.ts` beside the file they cover) and omitted below. + ```text src/ ├── arranger/ @@ -32,19 +34,21 @@ src/ │ ├── types.ts # response types for introspection payloads │ └── validation.ts # validates the connection to Arranger ├── http/ -│ └── app.ts # MCP express app with Streamable HTTP transport +│ ├── requestBody.ts # reads and size-caps the request body +│ └── server.ts # serves the MCP handler on node:http, with Host and Origin guards ├── mcp/ │ ├── buildSqonTool.ts # build SQON tool +│ ├── cacheHints.ts # freshness hints published on cacheable results │ ├── executeQueryTool.ts # execute query tool -│ ├── instructions.ts # server instructions sent in the initialize response +│ ├── instructions.ts # server instructions, returned by server/discover │ ├── prompts.ts # registers MCP prompts +│ ├── requestState.ts # signs and verifies execute_query's confirmation state │ ├── resources.ts # registers MCP resources │ ├── sqonCheatSheet.ts # compact SQON reference, returned by get_sqon_schema │ └── tools.ts # registers MCP tools ├── utils/ │ ├── config.ts # env/config parsing │ ├── errors.ts # error handling utilities -│ ├── inMemoryEventStore.ts # in-memory storage util for dev │ └── logger.ts # pino logger wrapper ├── index.ts # entrypoint for the application └── server.ts # creates the MCP server @@ -101,7 +105,7 @@ Configuration of this application is done by providing [environment variables](# > [!WARNING] > If **required** environment variables are not available or misconfigured at run time, the application will shut down immediately. -An example environment variables file is located at [`.env.schema`](./.env.schema). This example file lists all available configuration variables and is prepopulated with default values that should work to run the application locally. You can copy the contents of this file to populate a `.env`: +An example environment variables file is located at [`.env.schema`](./.env.schema). This example file lists all available configuration variables, prepopulated so the application runs locally as-is. It sets `MCP_ALLOWED_HOSTS` explicitly because the default `MCP_HOST` of `0.0.0.0` binds every interface, which requires an allowlist; see the table below. You can copy the contents of this file to populate a `.env`: ```bash # from apps/mcp-server @@ -110,15 +114,19 @@ cp .env.schema .env ### Environment Variables -| Name | Description | Type | Required | Default | -| ----------------------------- | ----------------------------------------------------------------------- | -------- | ------------ | ----------------------- | -| `ARRANGER_BASE_URL` | URL for the Arranger Server | `string` | **Required** | `http://localhost:5050` | -| `ARRANGER_CATALOGUES` | Comma-separated list of Arranger catalogues to expose to the MCP Server | `string` | **Required** | `server` | -| `ARRANGER_REQUEST_TIMEOUT_MS` | Timeout for requests to Arranger | `number` | Optional | `10_000` | -| `MCP_HOST` | Host URL for the MCP server | `string` | Optional | `0.0.0.0` | -| `MCP_PORT` | Port the MCP Server will listen for requests on | `number` | Optional | `3100` | -| `MCP_PATH` | Endpoint for the MCP Streamable HTTP transport | `string` | Optional | `/mcp` | -| `LOG_LEVEL` | Pino [log level](https://getpino.io/#/docs/api?id=level-1) | `string` | Optional | `info` | +| Name | Description | Type | Required | Default | +| ----------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | ------------ | ------------------------------------------------------- | +| `ARRANGER_BASE_URL` | URL for the Arranger Server | `string` | **Required** | `http://localhost:5050` | +| `ARRANGER_CATALOGUES` | Comma-separated list of Arranger catalogues to expose to the MCP Server | `string` | **Required** | `server` | +| `ARRANGER_REQUEST_TIMEOUT_MS` | Timeout for requests to Arranger | `number` | Optional | `10_000` | +| `MCP_HOST` | Interface the MCP Server binds to. A loopback value (`127.0.0.1`, `localhost`, `::1`) defaults both allowlists below to the localhost hostnames; any other value requires `MCP_ALLOWED_HOSTS`. | `string` | Optional | `0.0.0.0` | +| `MCP_PORT` | Port the MCP Server will listen for requests on | `number` | Optional | `3100` | +| `MCP_PATH` | Endpoint for the MCP Streamable HTTP transport | `string` | Optional | `/mcp` | +| `MCP_ALLOWED_HOSTS` | Comma-separated hostnames clients use to reach this server (e.g. `arranger-mcp,mcp.example.org`), matched against the `Host` header for DNS rebinding protection. **Required whenever `MCP_HOST` is not loopback**: the server exits at startup rather than bind a routable interface unguarded. Set it to `*` only when an upstream gateway validates `Host` on your behalf. | `string` | Conditional | localhost hostnames on a loopback bind | +| `MCP_ALLOWED_ORIGINS` | Comma-separated browser origin hostnames allowed to call this server. An empty list is still a live check, not a disabled one: a request carrying no `Origin` header (every non-browser MCP client) passes, and any browser origin is refused. | `string` | Optional | localhost hostnames on a loopback bind, otherwise empty | +| `MCP_REQUEST_STATE_SECRET` | HMAC key the server signs `execute_query` confirmations with, so the query a user approves is the query that runs. Must be at least 32 bytes. Unset, the server generates one per process, which is correct at a single replica: in-flight confirmations do not survive a restart, and every confirmation fails across multiple instances. Set a shared value when running more than one. | `string` | Optional | a key generated per process | +| `MCP_MAX_BODY_BYTES` | Largest request body accepted, in bytes; anything above it is refused with `413`. The default preserves the `100kb` limit `express.json()` applied before this app served MCP on plain `node:http`, which the MCP SDK does not replace. Raise it if a legitimate payload is found to exceed it, for example an `execute_query` SQON filtering on a very large set of identifiers. | `number` | Optional | `102_400` (100kb) | +| `LOG_LEVEL` | Pino [log level](https://getpino.io/#/docs/api?id=level-1) | `string` | Optional | `info` | ## Testing diff --git a/apps/mcp-server/mcp-inspector.json b/apps/mcp-server/mcp-inspector.json index 4f15a1575..d0f39aed9 100644 --- a/apps/mcp-server/mcp-inspector.json +++ b/apps/mcp-server/mcp-inspector.json @@ -2,7 +2,8 @@ "mcpServers": { "mcp-server": { "type": "streamable-http", - "url": "http://127.0.0.1:3100/mcp" + "url": "http://127.0.0.1:3100/mcp", + "protocolEra": "modern" } } } diff --git a/apps/mcp-server/package.json b/apps/mcp-server/package.json index e64aad2d3..ede705c65 100644 --- a/apps/mcp-server/package.json +++ b/apps/mcp-server/package.json @@ -19,10 +19,11 @@ "dev": "NODE_ENV=development tsx watch --include './src/**/*' ./src/index.ts", "start": "tsx ./src/index.ts", "test": "tsx --test --experimental-test-module-mocks", - "inspect": "npx @modelcontextprotocol/inspector@1 --config ./mcp-inspector.json --server mcp-server" + "inspect": "npx @modelcontextprotocol/inspector@2 --config ./mcp-inspector.json --server mcp-server" }, "dependencies": { - "@modelcontextprotocol/sdk": "^1.29.0", + "@modelcontextprotocol/node": "^2.0.0", + "@modelcontextprotocol/server": "^2.0.0", "@overture-stack/arranger-types": "file:../../modules/types", "@overture-stack/sqon": "file:../../modules/sqon", "dotenv": "^16.6.1", @@ -32,8 +33,8 @@ "zod": "^4.2.0" }, "devDependencies": { + "@modelcontextprotocol/client": "^2.0.0", "@tsconfig/node22": "^22.0.5", - "@types/express": "^4.17.14", "@types/node": "^25.6.2", "typescript": "^5.8.3" }, diff --git a/apps/mcp-server/src/arranger/validation.test.ts b/apps/mcp-server/src/arranger/validation.test.ts index 99851466d..2420309fa 100644 --- a/apps/mcp-server/src/arranger/validation.test.ts +++ b/apps/mcp-server/src/arranger/validation.test.ts @@ -18,6 +18,10 @@ const mockConfig = (catalogues: string[] = ['catalogue-a']): ArrangerMcpConfig = host: '0.0.0.0', port: 3100, path: '/mcp', + allowedHosts: ['arranger-mcp'], + allowedOrigins: [], + requestStateSecret: undefined, + maxBodyBytes: 102_400, }, }); diff --git a/apps/mcp-server/src/http/app.ts b/apps/mcp-server/src/http/app.ts deleted file mode 100644 index 9ba307fa6..000000000 --- a/apps/mcp-server/src/http/app.ts +++ /dev/null @@ -1,102 +0,0 @@ -import { randomUUID } from 'node:crypto'; - -import { createMcpExpressApp } from '@modelcontextprotocol/sdk/server/express'; -import { type McpServer } from '@modelcontextprotocol/sdk/server/mcp'; -import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp'; -import { isInitializeRequest } from '@modelcontextprotocol/sdk/types'; -import { type Express, type Request, type Response } from 'express'; - -import { type ArrangerMcpConfig } from '#utils/config.js'; -import { InMemoryEventStore } from '#utils/inMemoryEventStore.js'; -import logger from '#utils/logger.js'; - -export type McpHttpApp = { - app: Express; - closeAllSessions: () => Promise; -}; - -// This code was adapted from the official MCP Server "Streamable HTTP" example: -// https://github.com/modelcontextprotocol/typescript-sdk/blob/v1.x/src/examples/server/simpleStreamableHttp.ts -export const createHttpApp = (config: ArrangerMcpConfig, serverFactory: () => McpServer): McpHttpApp => { - const transports: Record = {}; - - const postHandler = async (req: Request, res: Response) => { - const sessionId = req.headers['mcp-session-id'] as string | undefined; - try { - let transport: StreamableHTTPServerTransport; - if (sessionId && transports[sessionId]) { - transport = transports[sessionId]; - } else if (!sessionId && isInitializeRequest(req.body)) { - transport = new StreamableHTTPServerTransport({ - sessionIdGenerator: () => randomUUID(), - eventStore: new InMemoryEventStore(), - onsessioninitialized: (sid) => { - logger.info(`Session initialized with ID: ${sid}`); - transports[sid] = transport; - }, - }); - transport.onclose = () => { - const sid = transport.sessionId; - if (sid && transports[sid]) { - logger.info(`Transport closed for session ${sid}, removing from transports map`); - // eslint-disable-next-line @typescript-eslint/no-dynamic-delete - delete transports[sid]; - } - }; - - await serverFactory().connect(transport); - await transport.handleRequest(req, res, req.body); - return; - } else { - res.status(400).json({ - jsonrpc: '2.0', - error: { code: -32000, message: 'Bad Request: No valid session ID provided' }, - id: null, - }); - return; - } - await transport.handleRequest(req, res, req.body); - } catch (error) { - logger.error({ error }, 'Error handling MCP POST'); - if (!res.headersSent) { - res.status(500).json({ - jsonrpc: '2.0', - error: { code: -32603, message: 'Internal server error' }, - id: null, - }); - } - } - }; - - const sessionHandler = async (req: Request, res: Response) => { - const sessionId = req.headers['mcp-session-id'] as string | undefined; - if (!sessionId || !transports[sessionId]) { - res.status(400).send('Invalid or missing session ID'); - return; - } - await transports[sessionId].handleRequest(req, res); - }; - - const app = createMcpExpressApp(); - const { - mcp: { path }, - } = config; - app.post(path, postHandler); - app.get(path, sessionHandler); - app.delete(path, sessionHandler); - - const closeAllSessions = async () => { - for (const sessionId of Object.keys(transports)) { - try { - logger.debug(`Closing transport for session ${sessionId}`); - await transports[sessionId].close(); - } catch (error) { - logger.error({ error, sessionId }, 'Error closing transport'); - } - // eslint-disable-next-line @typescript-eslint/no-dynamic-delete - delete transports[sessionId]; - } - }; - - return { app, closeAllSessions }; -}; diff --git a/apps/mcp-server/src/http/requestBody.ts b/apps/mcp-server/src/http/requestBody.ts new file mode 100644 index 000000000..bbdf88330 --- /dev/null +++ b/apps/mcp-server/src/http/requestBody.ts @@ -0,0 +1,86 @@ +import { type IncomingMessage } from 'node:http'; + +/** Why a body was refused, and the answer to send. */ +export type RequestBodyRefusal = { status: number; code: number; message: string }; + +/** + * Outcome of reading a request body. + * + * `refusal` is set when the request must be answered rather than served. Otherwise `body` is the + * parsed JSON value to hand the MCP handler, or `undefined` when there was nothing to parse (a + * body-less method, or an empty payload). `undefined` is the same value `toNodeHandler` treats as + * "no pre-parsed body", and it is safe to pass after the stream has been drained: the adapter finds + * an exhausted stream and reads nothing. + */ +export type RequestBodyResult = { body?: unknown; refusal?: RequestBodyRefusal }; + +/** JSON-RPC parse error, the answer for a body that is not valid JSON. */ +const PARSE_ERROR = -32700; + +/** Applied to a body over the configured ceiling. There is no JSON-RPC code for "too large". */ +const PAYLOAD_TOO_LARGE = -32600; + +/** Methods that carry no request body, so there is nothing to read or cap. */ +const BODY_LESS_METHODS = ['GET', 'HEAD']; + +const tooLarge = (maxBytes: number): RequestBodyResult => ({ + refusal: { + status: 413, + code: PAYLOAD_TOO_LARGE, + message: `Request body exceeds the ${maxBytes} byte limit (MCP_MAX_BODY_BYTES).`, + }, +}); + +/** + * Reads a request body into memory under a byte ceiling and parses it as JSON. + * + * This exists because the v2 SDK does not cap request bodies: `createMcpHandler` has no body-size + * option, and the Node adapter's `toWebRequest` reads the stream to completion. Serving on + * `node:http` rather than Express means `express.json({ limit })` is no longer doing this for us, so + * an unauthenticated caller could otherwise make the process buffer without bound. + * + * The `content-length` check is an early out only. It is advisory and absent under chunked transfer + * encoding, so the running byte count is the check that actually enforces the ceiling. + * + * @param req - The incoming request. Its stream is drained unless the method carries no body. + * @param maxBytes - Ceiling on the body, in bytes. + * @returns The parsed body to forward, or the refusal to answer with. + */ +export const readCappedJsonBody = async (req: IncomingMessage, maxBytes: number): Promise => { + if (BODY_LESS_METHODS.includes((req.method ?? 'GET').toUpperCase())) { + return {}; + } + + const declaredLength = Number(req.headers['content-length']); + if (Number.isFinite(declaredLength) && declaredLength > maxBytes) { + return tooLarge(maxBytes); + } + + const chunks: Buffer[] = []; + let received = 0; + for await (const chunk of req) { + const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk as string); + received += buffer.byteLength; + if (received > maxBytes) { + // Stop reading rather than finish draining: the point is to not hold the payload. + req.destroy(); + return tooLarge(maxBytes); + } + chunks.push(buffer); + } + + if (received === 0) { + return {}; + } + + try { + return { body: JSON.parse(Buffer.concat(chunks).toString('utf8')) }; + } catch { + // Answered here rather than forwarded, because the handler would classify an unparseable + // body as 2025-era traffic and reject it with an unsupported-protocol-version error, which + // tells the caller nothing about what was actually wrong. + return { + refusal: { status: 400, code: PARSE_ERROR, message: 'Request body is not valid JSON.' }, + }; + } +}; diff --git a/apps/mcp-server/src/http/server.test.ts b/apps/mcp-server/src/http/server.test.ts new file mode 100644 index 000000000..cfe73fa90 --- /dev/null +++ b/apps/mcp-server/src/http/server.test.ts @@ -0,0 +1,325 @@ +import assert from 'node:assert/strict'; +import http from 'node:http'; +import { type AddressInfo } from 'node:net'; +import { after, before, suite, test } from 'node:test'; + +import { Client, StreamableHTTPClientTransport } from '@modelcontextprotocol/client'; +import { McpServer } from '@modelcontextprotocol/server'; +import { z as zod } from 'zod'; + +import { startMcpHttpServer, type McpHttpServer } from '#http/server.js'; +import { type ArrangerMcpConfig } from '#utils/config.js'; + +const MAX_BODY_BYTES = 1024; + +const baseConfig = (mcp: Partial): ArrangerMcpConfig => ({ + arrangerBaseUrl: 'https://arranger.test', + catalogues: ['participants'], + requestTimeoutMs: 1000, + mcp: { + host: '127.0.0.1', + port: 0, + path: '/mcp', + allowedHosts: ['arranger-mcp'], + allowedOrigins: [], + requestStateSecret: undefined, + maxBodyBytes: MAX_BODY_BYTES, + ...mcp, + }, +}); + +/** A server with no registered surface: these tests assert transport behaviour, not tool behaviour. */ +const emptyServerFactory = () => new McpServer({ name: 'transport-test', version: '0.0.0-test' }); + +type Response = { status: number; body: string }; + +/** + * Sends a request with full control over the `Host` header, which `fetch` silently drops. A + * fetch-based probe reports a false pass against Host validation, which is how the 403 this suite + * pins went unnoticed. + * + * The default `Host` carries a port, because that is what a real client sends. The guard compares + * `new URL('http://' + header).hostname`, so the port has to be stripped for the allowlist to match + * at all; sending a bare hostname here would leave that stripping unexercised. + */ +const request = ( + port: number, + { + method = 'POST', + path = '/mcp', + headers = {}, + body, + omitContentLength = false, + }: { + method?: string; + path?: string; + headers?: Record; + body?: string | Buffer; + /** Sends the body with chunked transfer encoding, so its size is not declared up front. */ + omitContentLength?: boolean; + }, +): Promise => + new Promise((resolve, reject) => { + const payload = typeof body === 'string' ? Buffer.from(body) : body; + const req = http.request( + { + host: '127.0.0.1', + port, + path, + method, + headers: { + host: `arranger-mcp:${port}`, + 'content-type': 'application/json', + ...(payload && !omitContentLength ? { 'content-length': String(payload.byteLength) } : {}), + ...headers, + }, + }, + (res) => { + let collected = ''; + res.on('data', (chunk) => (collected += chunk)); + res.on('end', () => resolve({ status: res.statusCode ?? 0, body: collected })); + }, + ); + req.on('error', reject); + req.end(payload); + }); + +/** A 2025-era `initialize`: no per-request `_meta` envelope, so the handler classifies it legacy. */ +const legacyInitialize = JSON.stringify({ + jsonrpc: '2.0', + id: 1, + method: 'initialize', + params: { + protocolVersion: '2025-11-25', + capabilities: {}, + clientInfo: { name: 'probe', version: '0.0.0' }, + }, +}); + +suite('startMcpHttpServer', () => { + let server: McpHttpServer; + let port: number; + + before(async () => { + server = await startMcpHttpServer(baseConfig({}), emptyServerFactory); + port = (server.httpServer.address() as AddressInfo).port; + }); + + after(async () => { + await server.close(); + }); + + suite('DNS rebinding guards', () => { + test('serves a request whose Host header is in the allowlist', async () => { + const { status } = await request(port, { body: legacyInitialize }); + + // Reaching the handler at all is the assertion: it answers, rather than the guard refusing. + assert.equal(status, 400); + }); + + test('serves a Host header carrying no port', async () => { + const { status } = await request(port, { headers: { host: 'arranger-mcp' }, body: legacyInitialize }); + + assert.notEqual(status, 403); + }); + + test('refuses a Host header outside the allowlist', async () => { + const { status, body } = await request(port, { + headers: { host: 'evil.example.com' }, + body: legacyInitialize, + }); + + assert.equal(status, 403); + assert.match(body, /Invalid Host/); + }); + + test('refuses a request with no Host header', async () => { + // `http.request` supplies one unless it is explicitly emptied. + const { status } = await request(port, { headers: { host: '' }, body: legacyInitialize }); + + assert.equal(status, 403); + }); + + test('refuses a browser Origin when the allowlist is empty', async () => { + const { status, body } = await request(port, { + headers: { origin: 'https://portal.example.com' }, + body: legacyInitialize, + }); + + assert.equal(status, 403); + assert.match(body, /Origin/i); + }); + + test('allows a request carrying no Origin, which is every non-browser MCP client', async () => { + const { status } = await request(port, { body: legacyInitialize }); + + assert.notEqual(status, 403); + }); + }); + + suite('request body cap', () => { + test('refuses a body over the cap declared by content-length', async () => { + const { status, body } = await request(port, { body: 'x'.repeat(MAX_BODY_BYTES + 1) }); + + assert.equal(status, 413); + assert.match(body, /exceeds the 1024 byte limit/); + }); + + // The content-length check is only an early out. Chunked encoding declares no size, so this + // is the case the running byte count has to catch on its own. + test('refuses an oversized body that hides its size with chunked encoding', async () => { + const { status } = await request(port, { + body: 'x'.repeat(MAX_BODY_BYTES + 1), + omitContentLength: true, + }); + + assert.equal(status, 413); + }); + + // The check is `received > maxBytes`, so a body of exactly the cap is served. Pinned because + // nothing else would catch that becoming `>=`. + test('serves a body of exactly the cap', async () => { + const envelope = { jsonrpc: '2.0', id: 1, method: 'initialize', pad: '' }; + const padding = 'a'.repeat(MAX_BODY_BYTES - Buffer.byteLength(JSON.stringify(envelope))); + const payload = JSON.stringify({ ...envelope, pad: padding }); + assert.equal(Buffer.byteLength(payload), MAX_BODY_BYTES, 'test payload must sit exactly on the cap'); + + const { status } = await request(port, { body: payload }); + + assert.notEqual(status, 413); + }); + + test('serves a body just under the cap', async () => { + const envelope = { jsonrpc: '2.0', id: 1, method: 'initialize', pad: '' }; + const padding = 'a'.repeat(MAX_BODY_BYTES - JSON.stringify(envelope).length); + const payload = JSON.stringify({ ...envelope, pad: padding }); + assert.ok(Buffer.byteLength(payload) <= MAX_BODY_BYTES, 'test payload must fit under the cap'); + + const { status } = await request(port, { body: payload }); + + assert.notEqual(status, 413); + }); + }); + + suite('malformed input', () => { + test('answers a body that is not valid JSON with a parse error', async () => { + const { status, body } = await request(port, { body: '{ not json' }); + + assert.equal(status, 400); + assert.equal(JSON.parse(body).error.code, -32700); + }); + }); + + suite('routing', () => { + test('answers a path other than the configured MCP endpoint with 404', async () => { + const { status, body } = await request(port, { path: '/not-mcp', body: legacyInitialize }); + + assert.equal(status, 404); + assert.match(body, /The MCP endpoint is \/mcp/); + }); + + test('ignores a query string when matching the endpoint', async () => { + const { status } = await request(port, { path: '/mcp?trace=1', body: legacyInitialize }); + + assert.notEqual(status, 404); + }); + }); + + suite('protocol era', () => { + test('refuses a 2025-era request, naming the revision this endpoint serves', async () => { + const { status, body } = await request(port, { body: legacyInitialize }); + const { error } = JSON.parse(body); + + assert.equal(status, 400); + assert.equal(error.code, -32022); + assert.deepEqual(error.data.supported, ['2026-07-28']); + }); + + test('answers a 2025-era session GET without opening a stream', async () => { + const { status } = await request(port, { method: 'GET', headers: { accept: 'text/event-stream' } }); + + assert.equal(status, 405); + }); + }); +}); + +/** + * The rest of this file asserts what the transport refuses. This suite asserts that it serves: + * guards passed, body read and parsed, the web-standard handler bridged back onto `node:http`, and a + * real answer returned. Without it the file would pass against a server that refuses everything. + * + * It uses a real client rather than a hand-built request because a `2026-07-28` call carries a + * per-request `_meta` envelope that is not worth reproducing by hand. The client is a devDependency, + * so it does not reach the published image. + */ +suite('startMcpHttpServer serving a modern client', () => { + let server: McpHttpServer; + let client: Client; + + before(async () => { + server = await startMcpHttpServer( + // The client dials 127.0.0.1, so that is the hostname its `Host` header carries. + baseConfig({ allowedHosts: ['127.0.0.1'] }), + () => { + const mcpServer = new McpServer({ name: 'transport-test', version: '0.0.0-test' }); + mcpServer.registerTool( + 'echo', + { description: 'Returns what it was given.', inputSchema: zod.object({ value: zod.string() }) }, + ({ value }) => ({ content: [{ type: 'text', text: `echoed ${value}` }] }), + ); + return mcpServer; + }, + ); + + const { port } = server.httpServer.address() as AddressInfo; + client = new Client( + { name: 'transport-test-client', version: '0.0.0-test' }, + // Pinned, not 'auto': 'auto' falls back to the 2025 handshake, which this endpoint + // refuses, and the fallback is silent. + { versionNegotiation: { mode: { pin: '2026-07-28' } } }, + ); + await client.connect(new StreamableHTTPClientTransport(new URL(`http://127.0.0.1:${port}/mcp`))); + }); + + after(async () => { + await client.close(); + await server.close(); + }); + + test('completes the connect handshake and reports the server it reached', () => { + assert.equal(client.getServerVersion()?.name, 'transport-test'); + }); + + test('serves tools/list', async () => { + const { tools } = await client.listTools(); + + assert.deepEqual( + tools.map(({ name }) => name), + ['echo'], + ); + }); + + test('serves tools/call, round-tripping arguments and result', async () => { + const result = await client.callTool({ name: 'echo', arguments: { value: 'hello' } }); + + assert.deepEqual(result.content, [{ type: 'text', text: 'echoed hello' }]); + }); +}); + +suite('startMcpHttpServer with Host validation delegated', () => { + test('serves any Host header when MCP_ALLOWED_HOSTS is "*"', async () => { + const server = await startMcpHttpServer(baseConfig({ allowedHosts: 'any' }), emptyServerFactory); + const port = (server.httpServer.address() as AddressInfo).port; + + try { + const { status } = await request(port, { + headers: { host: 'anything.example.com' }, + body: legacyInitialize, + }); + + assert.notEqual(status, 403); + } finally { + await server.close(); + } + }); +}); diff --git a/apps/mcp-server/src/http/server.ts b/apps/mcp-server/src/http/server.ts new file mode 100644 index 000000000..a9ac9cf41 --- /dev/null +++ b/apps/mcp-server/src/http/server.ts @@ -0,0 +1,127 @@ +import http, { type IncomingMessage, type Server, type ServerResponse } from 'node:http'; + +import { hostHeaderValidation, originValidation, toNodeHandler } from '@modelcontextprotocol/node'; +import { createMcpHandler, type McpServerFactory } from '@modelcontextprotocol/server'; + +import { readCappedJsonBody } from '#http/requestBody.js'; +import { type ArrangerMcpConfig } from '#utils/config.js'; +import logger from '#utils/logger.js'; + +export type McpHttpServer = { + httpServer: Server; + /** Tears down the MCP handler and then stops accepting connections. */ + close: () => Promise; +}; + +/** Guards answer the request themselves when they refuse, and report whether serving may continue. */ +type RequestGuard = (req: IncomingMessage, res: ServerResponse) => boolean; + +const writeJsonRpcError = (res: ServerResponse, status: number, code: number, message: string): void => { + res.writeHead(status, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ jsonrpc: '2.0', error: { code, message }, id: null })); +}; + +/** + * Builds the DNS rebinding guards from configuration. + * + * The Host guard is omitted only for `MCP_ALLOWED_HOSTS=*`, which is the operator asserting that an + * upstream gateway validates the header. The Origin guard is always installed: an empty allowlist is + * a live check that passes requests carrying no `Origin` (every non-browser MCP client) and rejects + * any browser origin. + */ +const createGuards = ({ allowedHosts, allowedOrigins }: ArrangerMcpConfig['mcp']): RequestGuard[] => { + const guards: RequestGuard[] = []; + if (allowedHosts === 'any') { + logger.warn('MCP_ALLOWED_HOSTS is "*": Host header validation is delegated to an upstream gateway.'); + } else { + guards.push(hostHeaderValidation(allowedHosts)); + } + guards.push(originValidation(allowedOrigins)); + return guards; +}; + +/** + * Starts the MCP server over Streamable HTTP on plain `node:http`. + * + * `createMcpHandler` returns a web-standard `{ fetch, close, notify, bus }`, and `toNodeHandler` + * bridges it to `(req, res, parsedBody)`. There is no framework in between: the SDK ships the Host + * and Origin guards as `node:http` guards, and the only thing Express was contributing was + * `express.json({ limit })`, which `readCappedJsonBody` replaces. + * + * `legacy: 'reject'` serves protocol revision `2026-07-28` only. A 2025-era client is answered with + * an unsupported-protocol-version error naming the revision this endpoint speaks, rather than served + * a degraded session: per-request legacy serving cannot receive server-to-client requests, so + * `execute_query` could not obtain its confirmation on that path. + * + * @param config - Validated server configuration. + * @param serverFactory - Produces a fresh `McpServer` for each request the handler serves. + * @returns The listening server and a shutdown function. + */ +export const startMcpHttpServer = async ( + config: ArrangerMcpConfig, + serverFactory: McpServerFactory, +): Promise => { + const { host, port, path, maxBodyBytes } = config.mcp; + + const handler = createMcpHandler(serverFactory, { + legacy: 'reject', + // Reporting only: the handler has already answered. The common case here is a 2025-era + // client being turned away, which is expected traffic rather than a fault of ours, so this + // is a warning. Genuine handler failures surface in the response either way. + onerror: (err) => logger.warn({ err }, 'MCP handler rejected or reported a request'), + }); + const serve = toNodeHandler(handler, { + onerror: (err) => logger.error({ err }, 'MCP transport adapter error'), + }); + const guards = createGuards(config.mcp); + + const httpServer = http.createServer((req, res) => { + void (async () => { + try { + // Before anything reads the body: a refused request should never cost us the payload. + for (const guard of guards) { + if (!guard(req, res)) { + return; + } + } + + const { pathname } = new URL(req.url ?? '/', `http://${host}`); + if (pathname !== path) { + writeJsonRpcError(res, 404, -32601, `Not found. The MCP endpoint is ${path}.`); + return; + } + + const { body, refusal } = await readCappedJsonBody(req, maxBodyBytes); + if (refusal) { + writeJsonRpcError(res, refusal.status, refusal.code, refusal.message); + return; + } + + await serve(req, res, body); + } catch (error) { + logger.error({ err: error }, 'Unhandled error serving MCP request'); + if (!res.headersSent) { + writeJsonRpcError(res, 500, -32603, 'Internal server error'); + } + res.end(); + } + })(); + }); + + await new Promise((resolve, reject) => { + httpServer.once('error', reject); + httpServer.listen(port, host, () => { + httpServer.removeListener('error', reject); + resolve(); + }); + }); + + const close = async () => { + await handler.close(); + await new Promise((resolve, reject) => { + httpServer.close((error) => (error ? reject(error) : resolve())); + }); + }; + + return { httpServer, close }; +}; diff --git a/apps/mcp-server/src/mcp/buildSqonTool.test.ts b/apps/mcp-server/src/mcp/buildSqonTool.test.ts index 047b0d4e4..c037aa220 100644 --- a/apps/mcp-server/src/mcp/buildSqonTool.test.ts +++ b/apps/mcp-server/src/mcp/buildSqonTool.test.ts @@ -1,21 +1,35 @@ import assert from 'node:assert/strict'; import { suite, test } from 'node:test'; -import { type McpServer } from '@modelcontextprotocol/sdk/server/mcp'; +import { type McpServer } from '@modelcontextprotocol/server'; import { getSqonFieldOperatorDetails } from '@overture-stack/sqon'; -import { z as zod } from 'zod'; +import type { z as zod } from 'zod'; import { ArrangerRequestError, type ArrangerClient } from '#arranger/client.js'; import { BUILD_SQON_OPERATORS, describeOperators, registerBuildSqonTool } from '#mcp/buildSqonTool.js'; +import { createConfirmationCodec } from '#mcp/requestState.js'; import { type ArrangerMcpConfig } from '#utils/config.js'; +/** Fixed HMAC key so the confirmation codec is deterministic and does not warn about a per-process one. */ +const TEST_SIGNING_KEY = 'arranger-mcp-test-request-state-signing-key'; + const config: ArrangerMcpConfig = { arrangerBaseUrl: 'https://arranger.test', catalogues: ['participants', 'files'], requestTimeoutMs: 10_000, - mcp: { host: '0.0.0.0', port: 3100, path: '/mcp' }, + mcp: { + host: '0.0.0.0', + port: 3100, + path: '/mcp', + allowedHosts: ['arranger-mcp'], + allowedOrigins: [], + requestStateSecret: TEST_SIGNING_KEY, + maxBodyBytes: 102_400, + }, }; +const requestStateCodec = createConfirmationCodec(config); + const introspection = { catalogId: 'participants', documentType: 'participant', @@ -46,8 +60,9 @@ type CapturedTool = { name: string; config: { description: string; - // Not `ZodRawShape` / `ZodTypeAny`: both live in Zod 4's compat shim. - inputSchema: Record; + // A `ZodObject` rather than a raw shape: SDK v2 deprecates the raw-shape overloads of + // `registerTool`, so the tool passes a wrapped schema and this parses with it directly. + inputSchema: zod.ZodObject>; outputSchema: zod.ZodType; title: string; }; @@ -80,7 +95,7 @@ const captureTool = (client: ArrangerClient): CapturedTool => { }, }; - registerBuildSqonTool(server as unknown as McpServer, { client, config }); + registerBuildSqonTool(server as unknown as McpServer, { client, config, requestStateCodec }); const tool = registered[0]; if (!tool) { @@ -92,7 +107,7 @@ const captureTool = (client: ArrangerClient): CapturedTool => { /** Parses input through the registered input schema, exactly as the SDK does, then runs the handler. */ const invoke = async (client: ArrangerClient, input: Record): Promise => { const tool = captureTool(client); - return tool.handler(zod.object(tool.config.inputSchema).parse(input) as Record); + return tool.handler(tool.config.inputSchema.parse(input) as Record); }; const buildSqon = async (input: Record, client: ArrangerClient = healthyClient) => { @@ -215,7 +230,7 @@ suite('build_sqon registration', () => { }); suite('build_sqon input schema', () => { - const schema = zod.object(captureTool(healthyClient).config.inputSchema); + const schema = captureTool(healthyClient).config.inputSchema; const parse = (input: Record) => schema.safeParse(input); const oneClause = (clause: Record) => ({ catalogueId: 'participants', diff --git a/apps/mcp-server/src/mcp/buildSqonTool.ts b/apps/mcp-server/src/mcp/buildSqonTool.ts index 34445eb96..fd959f84c 100644 --- a/apps/mcp-server/src/mcp/buildSqonTool.ts +++ b/apps/mcp-server/src/mcp/buildSqonTool.ts @@ -1,4 +1,4 @@ -import { type McpServer } from '@modelcontextprotocol/sdk/server/mcp'; +import { type McpServer } from '@modelcontextprotocol/server'; import { addFilterClause, getSqonFieldOperatorDetails, @@ -146,7 +146,7 @@ const clauseSchema = () => }), ]); -const inputSchema = { +const inputSchema = zod.object({ catalogueId: zod .string() .min(1) @@ -170,7 +170,7 @@ const inputSchema = { .describe( 'The "sqon" from an earlier build_sqon response, to add conditions to a query that already ran. Pass it back unchanged. Omit it when starting a new query.', ), -}; +}); const outputSchema = zod.object({ // Left opaque: a full SqonSchema description would bloat every tools/list response. diff --git a/apps/mcp-server/src/mcp/cacheHints.ts b/apps/mcp-server/src/mcp/cacheHints.ts new file mode 100644 index 000000000..bbaf6d6df --- /dev/null +++ b/apps/mcp-server/src/mcp/cacheHints.ts @@ -0,0 +1,29 @@ +import { type ServerOptions } from '@modelcontextprotocol/server'; + +/** Freshness for results that change only when this server is redeployed. */ +const BUILD_STATIC_TTL_MS = 3_600_000; + +/** Freshness for results that track Arranger's catalogue configuration rather than our build. */ +const ARRANGER_CONFIG_TTL_MS = 60_000; + +/** + * Freshness hints published on the six cacheable results (protocol revision `2026-07-28`). The SDK + * default of `{ ttlMs: 0, cacheScope: 'private' }` tells every client to cache nothing, so leaving + * this unset throws the feature away. + * + * `resources/list` tracks Arranger rather than the build despite the name: the catalogue resource is + * a template whose `list` callback asks Arranger which catalogues exist. + * + * Typed against `ServerOptions` so a key that is not cacheable fails to compile; the SDK ignores one + * rather than rejecting it. + * + * `private` means "do not share", not "do not cache". + */ +export const RESULT_CACHE_HINTS: NonNullable = { + 'tools/list': { ttlMs: BUILD_STATIC_TTL_MS, cacheScope: 'public' }, + 'prompts/list': { ttlMs: BUILD_STATIC_TTL_MS, cacheScope: 'public' }, + 'resources/templates/list': { ttlMs: BUILD_STATIC_TTL_MS, cacheScope: 'public' }, + 'server/discover': { ttlMs: BUILD_STATIC_TTL_MS, cacheScope: 'public' }, + 'resources/list': { ttlMs: ARRANGER_CONFIG_TTL_MS, cacheScope: 'private' }, + 'resources/read': { ttlMs: ARRANGER_CONFIG_TTL_MS, cacheScope: 'private' }, +}; diff --git a/apps/mcp-server/src/mcp/executeQueryTool.test.ts b/apps/mcp-server/src/mcp/executeQueryTool.test.ts new file mode 100644 index 000000000..dda228f71 --- /dev/null +++ b/apps/mcp-server/src/mcp/executeQueryTool.test.ts @@ -0,0 +1,304 @@ +import assert from 'node:assert/strict'; +import { suite, test } from 'node:test'; + +import { CLIENT_CAPABILITIES_META_KEY, type McpServer, type ServerContext } from '@modelcontextprotocol/server'; + +import { type ArrangerClient } from '#arranger/client.js'; +import { registerExecuteQueryTool } from '#mcp/executeQueryTool.js'; +import { createConfirmationCodec, type ConfirmationState } from '#mcp/requestState.js'; +import { type ArrangerMcpConfig } from '#utils/config.js'; + +/** Fixed HMAC key so the confirmation codec is deterministic and does not warn about a per-process one. */ +const TEST_SIGNING_KEY = 'arranger-mcp-test-request-state-signing-key'; + +const config: ArrangerMcpConfig = { + arrangerBaseUrl: 'https://arranger.test', + catalogues: ['participants'], + requestTimeoutMs: 10_000, + mcp: { + host: '0.0.0.0', + port: 3100, + path: '/mcp', + allowedHosts: ['arranger-mcp'], + allowedOrigins: [], + requestStateSecret: TEST_SIGNING_KEY, + maxBodyBytes: 102_400, + }, +}; + +// One codec for the whole suite, as `startServer` builds one for the whole process: a codec built +// per round would mint under one key and verify under another, and nothing would ever confirm. +const requestStateCodec = createConfirmationCodec(config); + +const serverIntrospection = { + catalogCount: 1, + catalogs: { + participants: { + documentType: 'participant', + paths: { fields: '/fields', graphql: '/graphql', introspection: '/introspection/participants' }, + }, + }, + mode: 'single', + sqonSchemaPath: '/introspection/sqon', +}; + +const catalogueIntrospection = { + catalogId: 'participants', + documentType: 'participant', + generatedAt: '2026-01-01T00:00:00.000Z', + meta: { authFiltered: false }, + operators: { keyword: ['in', 'not-in', 'some-not-in', 'all', 'filter'] }, + fields: { study: { displayName: 'Study', isArray: false, type: 'keyword' } }, +}; + +const EMPTY_ROOT_SQON = { op: 'and', content: [] }; + +type ToolResult = { + content?: { type: string; text: string }[]; + structuredContent?: Record; + isError?: boolean; + /** Present only on the `input_required` return, which is not a `CallToolResult`. */ + resultType?: string; + inputRequests?: Record; + /** The sealed confirmation state the client echoes back on the next round. */ + requestState?: string; +}; + +/** + * Counts the Arranger calls a run makes, so a test can assert that a refusal costs none. `executeQuery` + * answers with whatever root field the built query selected, rather than a hardcoded name, so the + * stub does not have to reproduce the document-type sanitization. + */ +const createStubClient = () => { + const calls = { introspection: 0, executed: 0 }; + const client = { + getServerIntrospection: () => { + calls.introspection += 1; + return Promise.resolve(serverIntrospection); + }, + getCatalogueIntrospection: () => { + calls.introspection += 1; + return Promise.resolve(catalogueIntrospection); + }, + getSqonIntrospection: () => Promise.reject(new Error('execute_query should not call getSqonIntrospection')), + executeQuery: (_endpoint: string, request: { rootFieldName: string }) => { + calls.executed += 1; + return Promise.resolve({ data: { [request.rootFieldName]: { hits: { total: 1, edges: [] } } } }); + }, + } as unknown as ArrangerClient; + return { client, calls }; +}; + +/** Captures the handler `registerExecuteQueryTool` registers, so it can be driven directly. */ +const captureHandler = (client: ArrangerClient) => { + let handler: ((args: Record, ctx: ServerContext) => Promise) | undefined; + const server = { + registerTool: (_name: string, _toolConfig: unknown, registered: unknown) => { + handler = registered as typeof handler; + }, + }; + + registerExecuteQueryTool(server as unknown as McpServer, { client, config, requestStateCodec }); + + if (!handler) { + throw new Error('registerExecuteQueryTool registered no handler'); + } + return handler; +}; + +type ContextOptions = { + elicitation?: boolean; + inputResponses?: Record; + requestState?: ConfirmationState; +}; + +/** + * Builds the request context the handler reads: the per-request capability envelope, any answer + * carried by a retried call, and the confirmation state that answer echoed back. Only the fields + * `execute_query` touches are populated. + * + * `requestState` is an accessor returning the decoded payload rather than the wire string, because + * that is what the server seam hands a handler once the configured verify hook has resolved with it. + * `method` is populated because the codec's binding reads it. + */ +const createContext = ({ elicitation = true, inputResponses, requestState }: ContextOptions = {}): ServerContext => + ({ + mcpReq: { + method: 'tools/call', + envelope: { [CLIENT_CAPABILITIES_META_KEY]: elicitation ? { elicitation: {} } : {} }, + inputResponses, + requestState: () => requestState, + }, + }) as unknown as ServerContext; + +/** The arguments every round uses unless a test is deliberately changing the query between rounds. */ +const queryArgs = { catalogueId: 'participants', sqon: EMPTY_ROOT_SQON, fields: ['study'] }; + +const run = async (contextOptions: ContextOptions = {}, args: Record = queryArgs) => { + const { client, calls } = createStubClient(); + const handler = captureHandler(client); + const ctx = createContext(contextOptions); + const result = await handler(args, ctx); + return { result, calls, ctx }; +}; + +/** + * The state a first round mints for `args`, decoded the way the seam decodes it before re-entry. + * + * Verifying it here is not incidental: it is the same call `ServerOptions.requestState.verify` makes + * at the seam, so a value that failed would fail there too and never reach the handler. + */ +const mintedStateFor = async (args: Record): Promise => { + const { result, ctx } = await run({}, args); + assert.ok(result.requestState, 'expected the first round to mint a requestState'); + return requestStateCodec.verify(result.requestState, ctx); +}; + +/** + * Drives a whole confirmation: a first round asks, and a second answers with `response` while + * echoing back the state the first minted, which is what a client does and what the seam then hands + * the handler. + * @param approvedArgs - Arguments the approved query was built from, when they differ from the ones + * the answering round re-sends. Modelling an agent that shows one query and re-enters with another. + * @param omitRequestState - Answers without echoing any state, which nothing in the protocol forces + * a client to do. + */ +const answered = async ( + response: unknown, + { + args = queryArgs, + approvedArgs = args, + omitRequestState = false, + }: { args?: Record; approvedArgs?: Record; omitRequestState?: boolean } = {}, +) => { + const requestState = omitRequestState ? undefined : await mintedStateFor(approvedArgs); + return run({ inputResponses: { confirm: response }, requestState }, args); +}; + +suite('execute_query confirmation', () => { + suite('a client that cannot elicit', () => { + // The refusal is the whole point of the branch: with 2025-era serving gone this is the only + // remaining route to running a query nobody approved. + test('is refused rather than served unconfirmed', async () => { + const { result } = await run({ elicitation: false }); + + assert.equal(result.isError, true); + assert.match(result.content?.[0]?.text ?? '', /elicitation/); + }); + + test('is refused before any Arranger request is made', async () => { + const { calls } = await run({ elicitation: false }); + + assert.equal(calls.introspection, 0, 'expected the refusal to cost no introspection round trip'); + assert.equal(calls.executed, 0); + }); + }); + + suite('the first round', () => { + test('asks for confirmation instead of executing', async () => { + const { result, calls } = await run(); + + assert.equal(result.resultType, 'input_required'); + assert.equal(calls.executed, 0, 'expected nothing to run before the user answered'); + }); + + test('shows the query and the catalogue it will run against', async () => { + const { result } = await run(); + const message = result.inputRequests?.confirm?.params.message ?? ''; + + assert.match(message, /participants/); + assert.match(message, /ArrangerMcpExecuteQuery/); + }); + }); + + suite('the answer', () => { + test('executes the query when the user accepts', async () => { + const { result, calls } = await answered({ action: 'accept', content: { confirm: true } }); + + assert.equal(calls.executed, 1); + assert.equal(result.structuredContent?.executed, true); + }); + + test('does not execute when the user declines', async () => { + const { result, calls } = await answered({ action: 'decline' }); + + assert.equal(calls.executed, 0); + assert.equal(result.structuredContent?.executed, false); + assert.match(String(result.structuredContent?.message), /declined/); + }); + + test('does not execute when the user cancels', async () => { + const { calls } = await answered({ action: 'cancel' }); + + assert.equal(calls.executed, 0); + }); + + // The remaining cases are attacker-controlled input rather than anything a well-behaved + // client sends, which is why they are pinned here and not in the integration suite. + test('does not execute when the answer is accepted but withholds confirmation', async () => { + const { calls } = await answered({ action: 'accept', content: { confirm: false } }); + + assert.equal(calls.executed, 0); + }); + + test('does not execute when the accepted content fails the schema', async () => { + const { calls } = await answered({ action: 'accept', content: { confirm: 'yes' } }); + + assert.equal(calls.executed, 0, 'a non-boolean confirm must not be read as approval'); + }); + + test('does not execute when the accepted content is missing entirely', async () => { + const { calls } = await answered({ action: 'accept' }); + + assert.equal(calls.executed, 0); + }); + + // A response the SDK cannot read arrives as absent, so the request is re-issued rather than + // treated as either an approval or a refusal. + test('re-asks when the answer is of a shape the SDK could not read', async () => { + const { result, calls } = await answered({ method: 'elicitation/create', result: { confirm: true } }); + + assert.equal(result.resultType, 'input_required'); + assert.equal(calls.executed, 0); + }); + }); + + // What ties an approval to the query it approved. Without it the second round rebuilds the query + // from arguments the client re-sends, so an agent could show one query for confirmation and + // execute another under the same answer. + suite('the binding between the answer and the query', () => { + const accept = { action: 'accept', content: { confirm: true } }; + + test('the question travels with sealed state', async () => { + const { result } = await run(); + + assert.ok(result.requestState, 'expected the confirmation request to carry state'); + }); + + test('the state is minted for the query that was shown, not for the call', async () => { + const shown = await mintedStateFor(queryArgs); + const other = await mintedStateFor({ ...queryArgs, first: 5 }); + + assert.notEqual(shown.digest, other.digest, 'two different queries must not share one approval'); + }); + + test('refuses an answer that echoes no state at all', async () => { + const { result, calls } = await answered(accept, { omitRequestState: true }); + + assert.equal(calls.executed, 0, 'an approval tied to no query is not an approval of this one'); + assert.equal(result.isError, true); + assert.match(result.content?.[0]?.text ?? '', /requestState/); + }); + + test('refuses an approval that was minted for a different query', async () => { + const { result, calls } = await answered(accept, { approvedArgs: { ...queryArgs, first: 5 } }); + + assert.equal(calls.executed, 0, 'approval covers one exact query, not whatever the retry rebuilds'); + assert.equal(result.isError, true); + assert.match(result.content?.[0]?.text ?? '', /confirmed/); + // Refused rather than re-asked: asking again would hand a caller an unlimited retry loop + // against the confirmation gate. + assert.notEqual(result.resultType, 'input_required'); + }); + }); +}); diff --git a/apps/mcp-server/src/mcp/executeQueryTool.ts b/apps/mcp-server/src/mcp/executeQueryTool.ts index 9b3c3f19a..1390dceff 100644 --- a/apps/mcp-server/src/mcp/executeQueryTool.ts +++ b/apps/mcp-server/src/mcp/executeQueryTool.ts @@ -1,4 +1,14 @@ -import { type McpServer } from '@modelcontextprotocol/sdk/server/mcp'; +import { + acceptedContent, + CLIENT_CAPABILITIES_META_KEY, + inputRequired, + inputResponse, + type ClientCapabilities, + type InputRequiredResult, + type McpServer, + type RequestStateCodec, + type ServerContext, +} from '@modelcontextprotocol/server'; import { z as zod } from 'zod'; import { @@ -17,6 +27,7 @@ import { SQON_REQUIRED_MESSAGE, } from '#arranger/queryValidation.js'; import { catalogueIntrospectionSchema, serverIntrospectionSchema } from '#arranger/types.js'; +import { digestApprovedQuery, type ConfirmationState } from '#mcp/requestState.js'; import { type McpServerDeps } from '#server.js'; import { describeExecutionError, formatGraphQLError } from '#utils/errors.js'; @@ -27,6 +38,38 @@ const MAX_OFFSET = 10_000; const OPERATION_NAME = 'ArrangerMcpExecuteQuery'; +/** + * Identifier the confirmation request is filed under, and read back by on re-entry. It is the + * server's own key, not a protocol name, so it only has to be stable within this tool. + */ +const CONFIRMATION_KEY = 'confirm'; + +/** Shape the client's answer must satisfy before it is treated as an approval. */ +const confirmationSchema = zod.object({ confirm: zod.boolean() }); + +/** + * Refusal when an answer arrives without the state that was minted with the question. + * + * Nothing in the protocol forces a client to echo `requestState`, so an absent value has to be + * refused exactly like a tampered one. Comparing only when it happens to be present would make the + * whole binding opt-out at the caller's discretion, which is the same hole it exists to close. + */ +const UNBOUND_STATE_MESSAGE = + 'Query execution was refused: the confirmation answer did not carry back the requestState this server ' + + 'minted alongside the question, so the approval cannot be tied to any particular query. Call execute_query ' + + 'again and echo requestState verbatim on the retry.'; + +/** + * Refusal when the approved query and the rebuilt one differ. + * + * Refused rather than re-asked: re-asking would hand a caller an unlimited retry loop against the + * confirmation gate. + */ +const DIGEST_MISMATCH_MESSAGE = + 'Query execution was refused: the query built on this call is not the query that was confirmed. An approval ' + + 'covers one exact GraphQL document, its variables and its endpoint, and this call produced different ones. ' + + 'Call execute_query again to review and confirm the query you actually want to run.'; + const sortInputSchema = zod.object({ fieldName: zod.string().min(1).describe('Dot-notation field name to sort by (e.g. "donor.age_at_diagnosis").'), order: zod.enum(['asc', 'desc']).optional(), @@ -34,7 +77,7 @@ const sortInputSchema = zod.object({ missing: zod.enum(['first', 'last']).optional(), }); -const inputSchema = { +const inputSchema = zod.object({ catalogueId: zod.string().min(1).describe('Catalogue identifier from the Arranger /introspection payload.'), // Zod 4 makes an `unknown()` key required, so a missing `sqon` fails here rather than in the // handler. `.nonoptional()` carries the guidance across; the default is an unhelpful @@ -91,7 +134,7 @@ const inputSchema = { .describe( 'Whether an aggregation is narrowed by filters on its own field (default false, matching multi-select facet behaviour).', ), -}; +}); const outputSchema = zod.object({ catalogueId: zod.string(), @@ -177,45 +220,106 @@ const validateRequest = ({ }; /** - * Asks the user to review and confirm the generated GraphQL request before it is executed, - * using MCP elicitation. When the connected client does not advertise the elicitation - * capability, confirmation is skipped and the query proceeds; the executed request is - * always echoed in the tool response for transparency. - * @returns `true` when execution may proceed, `false` when the user declined or cancelled. + * Whether the client that sent this request declared the `elicitation` capability. + * + * Protocol revision `2026-07-28` carries client capabilities per request rather than per session, + * in the reserved `_meta` envelope. The SDK surfaces that envelope with its + * `io.modelcontextprotocol/*` keys intact and types it as an open object, so the value is narrowed + * here rather than typed. */ -const confirmExecution = async ({ - server, - catalogueId, - endpoint, - query, - variables, -}: { - server: McpServer; - catalogueId: string; - endpoint: string; - query: string; - variables: Record; -}): Promise => { - if (!server.server.getClientCapabilities()?.elicitation) { - return true; - } +const clientCanElicit = (ctx: ServerContext): boolean => { + const envelope = ctx.mcpReq.envelope as Record | undefined; + const capabilities = envelope?.[CLIENT_CAPABILITIES_META_KEY] as ClientCapabilities | undefined; + return capabilities?.elicitation !== undefined; +}; + +/** What the confirmation exchange has resolved to for this round of the call. */ +type Confirmation = + /** The user approved this query. */ + | { status: 'confirmed' } + /** The user declined or cancelled, or answered with something that is not an approval. */ + | { status: 'declined' } + /** An answer arrived, but nothing ties it to the query this call built. */ + | { status: 'unbound'; message: string } + /** Nothing has been asked yet: return this and wait to be re-entered with the answer. */ + | { status: 'pending'; result: InputRequiredResult }; + +/** + * Resolves the user's confirmation for the query that is about to run. + * + * Revision `2026-07-28` removed the server-to-client request channel, so a server can no longer ask + * and await an answer. It returns an `input_required` result instead, the call ends, and the client + * re-invokes the tool with the answer attached. The handler therefore runs twice per confirmed + * query, and this is what tells the two rounds apart. + * + * An answer the SDK could not read (the wrapped shape some peers emit) arrives as `missing`, so the + * request is re-issued rather than failed. The client's own round cap is what stops that repeating. + * + * **The approval is bound to what was approved.** The query is rebuilt from arguments the client + * re-sends, so without a binding an agent could show one query for confirmation and re-enter with + * different ones. Round one seals a digest of the built query into `requestState`; round two only + * counts as an approval when the state comes back carrying that same digest. The signature is what + * makes the digest worth comparing, and the server seam has already verified it by the time this + * runs, so a forged or expired value never reaches here at all. + * + * @param ctx - Request context, carrying any answer from a previous round and its verified state. + * @param codec - Seals the digest for the round trip and is verified back at the seam. + * @param digest - Digest of the query this call built, minted on the first round and compared on the second. + * @param message - The confirmation prompt shown to the user. + */ +const resolveConfirmation = async ( + ctx: ServerContext, + { codec, digest, message }: { codec: RequestStateCodec; digest: string; message: string }, +): Promise => { + const answer = inputResponse(ctx.mcpReq.inputResponses, CONFIRMATION_KEY); - const confirmation = await server.server.elicitInput({ - message: `About to execute this GraphQL query against Arranger catalogue "${catalogueId}" (POST ${endpoint}):\n\n${query}\n\nVariables:\n${JSON.stringify(variables, null, 2)}`, - requestedSchema: { - type: 'object', - properties: { - confirm: { - type: 'boolean', - title: 'Execute this query?', - description: 'Review the query and variables above, then confirm to run it against Arranger.', + if (answer.kind === 'missing') { + return { + status: 'pending', + result: inputRequired({ + requestState: await codec.mint({ digest }, ctx), + inputRequests: { + [CONFIRMATION_KEY]: inputRequired.elicit({ + message, + requestedSchema: { + type: 'object', + properties: { + confirm: { + type: 'boolean', + title: 'Execute this query?', + description: + 'Review the query and variables above, then confirm to run it against Arranger.', + }, + }, + required: ['confirm'], + }, + }), }, - }, - required: ['confirm'], - }, - }); + }), + }; + } + + // Checked before the answer itself: an approval that is tied to no query, or to a different one, + // is not an approval of this one whatever it says. + const state = ctx.mcpReq.requestState(); + if (typeof state?.digest !== 'string') { + return { status: 'unbound', message: UNBOUND_STATE_MESSAGE }; + } + // Plain `===`: the digest is readable on the wire and so is not a secret, which is also why a + // constant-time compare would buy nothing. The codec already compares the parts that are secret, + // the MAC and the bind tag, in constant time. + if (state.digest !== digest) { + return { status: 'unbound', message: DIGEST_MISMATCH_MESSAGE }; + } + + if (answer.kind !== 'elicit' || answer.action !== 'accept') { + return { status: 'declined' }; + } - return confirmation.action === 'accept' && confirmation.content?.confirm === true; + // Validated rather than read: this is attacker-controlled client input, and content failing the + // schema comes back `undefined`, which is treated the same as withholding approval. + const content = acceptedContent(ctx.mcpReq.inputResponses, CONFIRMATION_KEY, confirmationSchema); + return content?.confirm === true ? { status: 'confirmed' } : { status: 'declined' }; }; /** The slice of an Arranger GraphQL response the execute_query tool compacts for the LLM. */ @@ -232,7 +336,7 @@ type ArrangerQueryData = { * GraphQL query against one Arranger catalogue, returning a compact result without the * GraphQL `edges`/`node` nesting. */ -export const registerExecuteQueryTool = (server: McpServer, { client }: McpServerDeps): void => { +export const registerExecuteQueryTool = (server: McpServer, { client, requestStateCodec }: McpServerDeps): void => { server.registerTool( 'execute_query', { @@ -245,22 +349,37 @@ export const registerExecuteQueryTool = (server: McpServer, { client }: McpServe '3. use build_sqon to construct a valid SQON filter and pass the resulting SQON unchanged as input for this tool. ' + 'DO NOT guess field names, you MUST call get_catalogue_fields. ' + 'DO NOT construct "sqon" without calling build_sqon. ' + - 'The user is asked to review and confirm the generated GraphQL query before it runs (when the client supports elicitation).', + 'The user is asked to review and confirm the generated GraphQL query before it runs, so this tool requires a client that supports elicitation and refuses one that does not.', inputSchema, outputSchema, }, - async ({ - catalogueId, - sqon, - queryType = 'hits', - fields = [], - first = DEFAULT_FIRST, - offset = DEFAULT_OFFSET, - sort, - aggregationFields = [], - includeMissing = true, - aggregationsFilterThemselves = false, - }) => { + async ( + { + catalogueId, + sqon, + queryType = 'hits', + fields = [], + first = DEFAULT_FIRST, + offset = DEFAULT_OFFSET, + sort, + aggregationFields = [], + includeMissing = true, + aggregationsFilterThemselves = false, + }, + ctx, + ) => { + // Refused up front rather than executed unconfirmed. With 2025-era serving gone, a client + // that cannot elicit is the only remaining route to running a query nobody approved, and + // treating it as "skip the confirmation" would make the gate opt-out at the caller's + // discretion. Checked before any Arranger call, since the answer cannot change. + if (!clientCanElicit(ctx)) { + return errorResult( + 'execute_query requires a client that supports elicitation, because the generated query must be ' + + 'confirmed before it runs, and this client did not declare the "elicitation" capability. ' + + 'Reconnect with elicitation support, or use build_sqon to inspect the filter without executing it.', + ); + } + try { const serverIntrospection = serverIntrospectionSchema.parse(await client.getServerIntrospection()); const catalogue = serverIntrospection.catalogs[catalogueId]; @@ -310,14 +429,20 @@ export const registerExecuteQueryTool = (server: McpServer, { client }: McpServe operationName: OPERATION_NAME, }); - const confirmed = await confirmExecution({ - server, - catalogueId, - endpoint, - query: request.query, - variables: request.variables, + // Re-entry re-runs everything above: introspection is fetched again and the query + // rebuilt, because only the confirmation digest carries over between rounds. + const confirmation = await resolveConfirmation(ctx, { + codec: requestStateCodec, + digest: digestApprovedQuery({ endpoint, query: request.query, variables: request.variables }), + message: `About to execute this GraphQL query against Arranger catalogue "${catalogueId}" (POST ${endpoint}):\n\n${request.query}\n\nVariables:\n${JSON.stringify(request.variables, null, 2)}`, }); - if (!confirmed) { + if (confirmation.status === 'pending') { + return confirmation.result; + } + if (confirmation.status === 'unbound') { + return errorResult(confirmation.message); + } + if (confirmation.status === 'declined') { return successResult({ catalogueId, documentType, diff --git a/apps/mcp-server/src/mcp/instructions.ts b/apps/mcp-server/src/mcp/instructions.ts index 2e26e5109..884bf94da 100644 --- a/apps/mcp-server/src/mcp/instructions.ts +++ b/apps/mcp-server/src/mcp/instructions.ts @@ -1,18 +1,19 @@ /** - * Server-level instructions returned in the MCP `initialize` response. Clients typically fold this - * text into the model's system prompt, so it is the only guidance that reaches the model before it - * decides which tool to call first. Tool descriptions arrive with the tool list and are read once a - * tool is already under consideration; this text is what establishes the discovery-before-query + * Server-level instructions returned in the MCP `server/discover` result. Clients typically fold + * this text into the model's system prompt, so it is the only guidance that reaches the model before + * it decides which tool to call first. Tool descriptions arrive with the tool list and are read once + * a tool is already under consideration; this text is what establishes the discovery-before-query * rule in the first place. * - * It is deliberately static. Instructions are fixed at construction time (`createMcpServer` is - * synchronous, one call per session in `createHttpApp`), so naming the live catalogues here would - * mean an Arranger round trip per session, and would undercut the rule it exists to state: the - * model should read catalogue names from `list_catalogues`, not from a prefix that may be stale. + * It is deliberately static. Instructions are fixed at construction time and `createMcpServer` is + * synchronous, so naming the live catalogues here would mean an Arranger round trip every time the + * server is constructed. Protocol revision `2026-07-28` has no sessions and the handler builds one + * instance per request, so that is a round trip per request rather than per connection. It would + * also undercut the rule this text exists to state: the model should read catalogue names from + * `list_catalogues`, not from a prefix that may be stale. * - * Keep it short. It is billed on every session, and it competes for attention with the host's own - * system prompt. Anything longer or more procedural belongs in the `query_arranger` prompt, which - * is opt-in per turn. + * Keep it short. It competes for attention with the host's own system prompt. Anything longer or + * more procedural belongs in the `query_arranger` prompt, which is opt-in per turn. */ export const SERVER_INSTRUCTIONS = `This server exposes the data catalogues of one Overture Arranger instance, so you can search and summarize the records in them: filtered document searches (hits), per-field summaries (aggregations), or both. @@ -38,4 +39,5 @@ If the goal maps to more than one plausible query, ask which was meant instead o ## Notes - \`execute_query\` validates the SQON, every field name, and every operator against the catalogue before anything reaches Arranger, and reports the specific problem when it rejects a call. Treat a rejection as a signal to re-read the field metadata, not to resend the same shape. -- Where the client supports elicitation, the user is shown the generated GraphQL query and asked to confirm before it runs. Where it does not, nothing prompts them, so confirm intent in conversation before executing.`; +- \`execute_query\` always shows the user the generated GraphQL query and asks them to confirm before it runs. A client that cannot present that request is refused rather than served unconfirmed, so there is no path on which the query runs unseen. +- An approval covers one exact query. Answering the confirmation re-invokes \`execute_query\` with the same arguments; changing them between the question and the answer produces a different query than the one approved, and the call is refused.`; diff --git a/apps/mcp-server/src/mcp/prompts.ts b/apps/mcp-server/src/mcp/prompts.ts index d795d5c8f..ad16cdf2e 100644 --- a/apps/mcp-server/src/mcp/prompts.ts +++ b/apps/mcp-server/src/mcp/prompts.ts @@ -1,4 +1,4 @@ -import { type McpServer } from '@modelcontextprotocol/sdk/server/mcp'; +import { type McpServer } from '@modelcontextprotocol/server'; import { z as zod } from 'zod'; import { type ArrangerServerIntrospection } from '#arranger/types.js'; @@ -123,9 +123,9 @@ export const registerPrompts = (server: McpServer, { client }: McpServerDeps): v 'Translates a natural language research goal into a validated SQON query. ' + 'Loads the live schema, classifies the question, and requires explicit researcher ' + 'confirmation before any data is retrieved.', - argsSchema: { + argsSchema: zod.object({ goal: zod.string().min(1).describe('Natural language description of the data the researcher wants.'), - }, + }), }, async ({ goal }) => { const introspection = await client.getServerIntrospection(); diff --git a/apps/mcp-server/src/mcp/requestState.test.ts b/apps/mcp-server/src/mcp/requestState.test.ts new file mode 100644 index 000000000..5605f71f3 --- /dev/null +++ b/apps/mcp-server/src/mcp/requestState.test.ts @@ -0,0 +1,106 @@ +import assert from 'node:assert/strict'; +import { suite, test } from 'node:test'; + +import { type ServerContext } from '@modelcontextprotocol/server'; + +import { createConfirmationCodec, digestApprovedQuery, type ApprovedQuery } from '#mcp/requestState.js'; +import { type ArrangerMcpConfig } from '#utils/config.js'; + +/** Fixed HMAC key so a codec built here is deterministic and does not warn about a per-process one. */ +const TEST_SIGNING_KEY = 'arranger-mcp-test-request-state-signing-key'; + +const config = (requestStateSecret: string | undefined): ArrangerMcpConfig => + ({ mcp: { requestStateSecret } }) as ArrangerMcpConfig; + +/** Only the field the binding reads is populated; nothing else in the context reaches the codec. */ +const context = (method = 'tools/call'): ServerContext => ({ mcpReq: { method } }) as unknown as ServerContext; + +const approvedQuery: ApprovedQuery = { + endpoint: '/graphql', + query: 'query ArrangerMcpExecuteQuery($filters: JSON) {\n\tparticipant {\n\t\thits {\n\t\t\ttotal\n\t\t}\n\t}\n}', + variables: { filters: { op: 'and', content: [] } }, +}; + +suite('digestApprovedQuery', () => { + test('is stable for the same query', () => { + assert.equal(digestApprovedQuery(approvedQuery), digestApprovedQuery({ ...approvedQuery })); + }); + + // Each part is covered because each part changes what runs: the document and its variables decide + // the query, and the endpoint decides which catalogue it runs against. + test('changes when the document changes', () => { + const altered = { ...approvedQuery, query: approvedQuery.query.replace('total', 'total\n\t\t\tedges') }; + + assert.notEqual(digestApprovedQuery(approvedQuery), digestApprovedQuery(altered)); + }); + + test('changes when the variables change', () => { + const altered = { ...approvedQuery, variables: { filters: { op: 'or', content: [] } } }; + + assert.notEqual(digestApprovedQuery(approvedQuery), digestApprovedQuery(altered)); + }); + + test('changes when the endpoint changes', () => { + const altered = { ...approvedQuery, endpoint: '/files/graphql' }; + + assert.notEqual(digestApprovedQuery(approvedQuery), digestApprovedQuery(altered)); + }); +}); + +suite('createConfirmationCodec', () => { + test('verifies back the digest it sealed', async () => { + const codec = createConfirmationCodec(config(TEST_SIGNING_KEY)); + const digest = digestApprovedQuery(approvedQuery); + + const state = await codec.mint({ digest }, context()); + + assert.deepEqual(await codec.verify(state, context()), { digest }); + }); + + // The whole reason the digest is worth comparing: without a signature a caller could simply state + // the digest of whatever query it wanted to run. + test('refuses a state whose signature does not cover it', async () => { + const codec = createConfirmationCodec(config(TEST_SIGNING_KEY)); + const state = await codec.mint({ digest: 'approved' }, context()); + + const [prefix, body] = state.split('.'); + const forgedBody = Buffer.from(JSON.stringify({ p: { digest: 'substituted' }, exp: 2 ** 40 })).toString( + 'base64url', + ); + const forged = `${prefix}.${forgedBody}.${state.slice(state.lastIndexOf('.') + 1)}`; + + assert.notEqual(body, forgedBody); + await assert.rejects(codec.verify(forged, context()), /mac/); + }); + + test('refuses a state minted for a different method', async () => { + const codec = createConfirmationCodec(config(TEST_SIGNING_KEY)); + const state = await codec.mint({ digest: 'approved' }, context('tools/call')); + + await assert.rejects(codec.verify(state, context('prompts/get')), /bind/); + }); + + test('refuses a value that was never minted', async () => { + const codec = createConfirmationCodec(config(TEST_SIGNING_KEY)); + + await assert.rejects(codec.verify('not-a-request-state', context()), /malformed/); + }); + + // Pins the shape of the mistake the wiring is arranged to avoid: `createMcpServer` runs per HTTP + // request and a confirmation spans two of them, so a codec built there would be a different codec + // on each round. With no secret configured, which is the local development path, that is not a + // degraded flow but a broken one. + test('does not share a per-process key with another codec', async () => { + const state = await createConfirmationCodec(config(undefined)).mint({ digest: 'approved' }, context()); + + await assert.rejects(createConfirmationCodec(config(undefined)).verify(state, context()), /mac/); + }); + + test('shares a configured secret with another codec', async () => { + const state = await createConfirmationCodec(config(TEST_SIGNING_KEY)).mint({ digest: 'approved' }, context()); + + assert.deepEqual(await createConfirmationCodec(config(TEST_SIGNING_KEY)).verify(state, context()), { + digest: 'approved', + }); + }); +}); diff --git a/apps/mcp-server/src/mcp/requestState.ts b/apps/mcp-server/src/mcp/requestState.ts new file mode 100644 index 000000000..1d72e0810 --- /dev/null +++ b/apps/mcp-server/src/mcp/requestState.ts @@ -0,0 +1,87 @@ +import { createHash, randomBytes } from 'node:crypto'; + +import { createRequestStateCodec, type RequestStateCodec, type ServerContext } from '@modelcontextprotocol/server'; + +import { type ArrangerMcpConfig } from '#utils/config.js'; +import { createLogger } from '#utils/logger.js'; + +const logger = createLogger('RequestState'); + +/** Entropy in the fallback key, matching the 32-byte minimum the codec enforces. */ +const FALLBACK_KEY_BYTES = 32; + +/** + * How long a minted confirmation stays answerable. This is the codec's own default, passed + * explicitly so the window is visible here rather than inherited silently. + */ +const CONFIRMATION_TTL_SECONDS = 600; + +/** The GraphQL request a confirmation message displayed, in the form the digest covers. */ +export type ApprovedQuery = { + endpoint: string; + query: string; + variables: Record; +}; + +/** + * What `execute_query` seals into `requestState`, and all that belongs there. The codec signs rather + * than encrypts, so anyone holding the wire value can read this; the expiry and the principal + * binding live outside the payload. + */ +export type ConfirmationState = { digest: string }; + +/** + * Digest of the query a confirmation message displayed. + * + * It covers the built document, its variables and the endpoint rather than the tool arguments, which + * is both what the user reviewed and what catches drift the arguments cannot show: introspection is + * fetched again on the second round, so a reconfigured catalogue could build a different query from + * identical arguments. + * + * `JSON.stringify` is insertion-ordered and both rounds build this object the same way, so the + * serialization is stable. A serializer that reordered keys would break every re-entry. + */ +export const digestApprovedQuery = ({ endpoint, query, variables }: ApprovedQuery): string => + createHash('sha256').update(JSON.stringify({ endpoint, query, variables })).digest('hex'); + +/** + * The configured secret, or a key generated for this process. + * + * A per-process key is the intended default at a single replica, so its absence is a warning rather + * than a failure. It is secure but not shared, which is what the warning names: elsewhere round two + * verifies under a different key than round one minted with, and fails as a forgery. + */ +const resolveKey = (secret: string | undefined): string | Uint8Array => { + if (secret !== undefined) { + return secret; + } + logger.warn( + 'MCP_REQUEST_STATE_SECRET is not set: query confirmations are signed with a key generated for this ' + + 'process. That is the intended default at a single replica. The key is not shared, so confirmations ' + + 'issued before a restart stop being answerable, and every confirmation fails across multiple replicas. ' + + 'Set MCP_REQUEST_STATE_SECRET when running more than one.', + ); + return randomBytes(FALLBACK_KEY_BYTES); +}; + +/** + * Creates the codec that integrity-protects `execute_query`'s confirmation state. + * + * `requestState` travels out through the client and returns as attacker-controlled input, so the + * digest is signed rather than merely carried. `verify` is installed at the server seam, which + * refuses a forged, expired or wrongly bound value before any handler runs. + * + * **Build this once per process, never inside the server factory.** That factory runs per HTTP + * request and a confirmation spans two of them, so a codec built there mints round one under one key + * and verifies round two under another. Unset secret is the local development path, where that makes + * every confirmation fail. + */ +export const createConfirmationCodec = (config: ArrangerMcpConfig): RequestStateCodec => + createRequestStateCodec({ + key: resolveKey(config.mcp.requestStateSecret), + ttlSeconds: CONFIRMATION_TTL_SECONDS, + // The SDK's documented binding. With authentication out of scope the principal is always + // empty, so today this only stops state minted for one method being replayed against another; + // it starts separating principals the moment auth lands, with no change needed here. + bind: (ctx: ServerContext) => `${ctx.mcpReq.method}\0${ctx.http?.authInfo?.clientId ?? ''}`, + }); diff --git a/apps/mcp-server/src/mcp/resources.ts b/apps/mcp-server/src/mcp/resources.ts index 18f62d2cb..4db92179c 100644 --- a/apps/mcp-server/src/mcp/resources.ts +++ b/apps/mcp-server/src/mcp/resources.ts @@ -1,4 +1,4 @@ -import { ResourceTemplate, type McpServer } from '@modelcontextprotocol/sdk/server/mcp'; +import { ResourceTemplate, type McpServer } from '@modelcontextprotocol/server'; import { type McpServerDeps } from '#server.js'; diff --git a/apps/mcp-server/src/mcp/tools.test.ts b/apps/mcp-server/src/mcp/tools.test.ts index 0485c2830..c4d7815d5 100644 --- a/apps/mcp-server/src/mcp/tools.test.ts +++ b/apps/mcp-server/src/mcp/tools.test.ts @@ -1,23 +1,37 @@ import assert from 'node:assert/strict'; import { suite, test } from 'node:test'; -import { type McpServer } from '@modelcontextprotocol/sdk/server/mcp'; -import { type ZodType } from 'zod'; +import { type McpServer } from '@modelcontextprotocol/server'; +import { type ZodObject, type ZodType } from 'zod'; import { type ArrangerClient } from '#arranger/client.js'; import { SERVER_INSTRUCTIONS } from '#mcp/instructions.js'; import { registerPrompts } from '#mcp/prompts.js'; +import { createConfirmationCodec } from '#mcp/requestState.js'; import { SQON_CHEAT_SHEET } from '#mcp/sqonCheatSheet.js'; import { registerTools } from '#mcp/tools.js'; import { type ArrangerMcpConfig } from '#utils/config.js'; +/** Fixed HMAC key so the confirmation codec is deterministic and does not warn about a per-process one. */ +const TEST_SIGNING_KEY = 'arranger-mcp-test-request-state-signing-key'; + const config: ArrangerMcpConfig = { arrangerBaseUrl: 'https://arranger.test', catalogues: ['participants'], requestTimeoutMs: 10_000, - mcp: { host: '0.0.0.0', port: 3100, path: '/mcp' }, + mcp: { + host: '0.0.0.0', + port: 3100, + path: '/mcp', + allowedHosts: ['arranger-mcp'], + allowedOrigins: [], + requestStateSecret: TEST_SIGNING_KEY, + maxBodyBytes: 102_400, + }, }; +const requestStateCodec = createConfirmationCodec(config); + const serverIntrospection = { catalogCount: 1, catalogs: { @@ -36,8 +50,9 @@ const client = { type RegisteredTool = { name: string; - // Not `ZodRawShape`: on Zod 4 its values are the core `$ZodType`, which has no `.description`. - config: { description?: string; inputSchema?: Record; title?: string }; + // A `ZodObject` rather than a raw shape: SDK v2 deprecates the raw-shape overloads of + // `registerTool`, so every tool now passes a wrapped schema and reads fields off `.shape`. + config: { description?: string; inputSchema?: ZodObject>; title?: string }; }; const registerAllTools = (): RegisteredTool[] => { @@ -47,7 +62,7 @@ const registerAllTools = (): RegisteredTool[] => { tools.push({ name, config: toolConfig } as RegisteredTool); }, }; - registerTools(server as unknown as McpServer, { client, config }); + registerTools(server as unknown as McpServer, { client, config, requestStateCodec }); return tools; }; @@ -58,7 +73,7 @@ const renderQueryArrangerPrompt = async (): Promise => { prompts.push({ name, callback } as (typeof prompts)[number]); }, }; - registerPrompts(server as unknown as McpServer, { client, config }); + registerPrompts(server as unknown as McpServer, { client, config, requestStateCodec }); const prompt = prompts[0]; if (!prompt) { @@ -111,7 +126,7 @@ suite('execute_query guidance', () => { }); test('tells the caller where the sqon argument comes from', () => { - const sqon = executeQuery().config.inputSchema?.sqon; + const sqon = executeQuery().config.inputSchema?.shape.sqon; assert.ok(sqon?.description?.includes('build_sqon')); }); }); diff --git a/apps/mcp-server/src/mcp/tools.ts b/apps/mcp-server/src/mcp/tools.ts index bf83f3815..ff36994b1 100644 --- a/apps/mcp-server/src/mcp/tools.ts +++ b/apps/mcp-server/src/mcp/tools.ts @@ -1,4 +1,4 @@ -import { type McpServer } from '@modelcontextprotocol/sdk/server/mcp'; +import { type McpServer } from '@modelcontextprotocol/server'; import { z as zod } from 'zod'; import { @@ -56,12 +56,12 @@ export const registerTools = (server: McpServer, deps: McpServerDeps): void => { title: 'Get Catalogue Fields', description: 'Return field introspection for one catalogue. `operators` maps each field type to its valid SQON operators. `fields` lists each field with its `type`, `displayName`, optional `unit`, and optional `description`.', - inputSchema: { + inputSchema: zod.object({ catalogueId: zod .string() .min(1) .describe('Catalogue identifier from the Arranger /introspection payload.'), - }, + }), outputSchema: catalogueIntrospectionSchema, }, async ({ catalogueId }) => { diff --git a/apps/mcp-server/src/server.test.ts b/apps/mcp-server/src/server.test.ts new file mode 100644 index 000000000..62ec72134 --- /dev/null +++ b/apps/mcp-server/src/server.test.ts @@ -0,0 +1,275 @@ +import assert from 'node:assert/strict'; +import { type AddressInfo } from 'node:net'; +import { suite, test } from 'node:test'; + +import { type ArrangerClient } from '#arranger/client.js'; +import { startMcpHttpServer } from '#http/server.js'; +import { SERVER_INSTRUCTIONS } from '#mcp/instructions.js'; +import { createConfirmationCodec } from '#mcp/requestState.js'; +import { createMcpServer } from '#server.js'; +import { type ArrangerMcpConfig } from '#utils/config.js'; + +import packageJson from '../package.json' with { type: 'json' }; + +/** Fixed HMAC key so the confirmation codec is deterministic and does not warn about a per-process one. */ +const TEST_SIGNING_KEY = 'arranger-mcp-test-request-state-signing-key'; + +/** The revision this endpoint serves, and the only one it accepts. */ +const PROTOCOL_REVISION = '2026-07-28'; + +const config: ArrangerMcpConfig = { + arrangerBaseUrl: 'https://arranger.test', + catalogues: ['participants'], + requestTimeoutMs: 10_000, + mcp: { + host: '127.0.0.1', + port: 0, + path: '/mcp', + allowedHosts: ['127.0.0.1'], + allowedOrigins: [], + requestStateSecret: TEST_SIGNING_KEY, + maxBodyBytes: 102_400, + }, +}; + +const serverIntrospection = { + catalogCount: 1, + catalogs: { + participants: { + documentType: 'participant', + paths: { fields: '/fields', graphql: '/graphql', introspection: '/introspection/participants' }, + }, + }, + mode: 'single', + sqonSchemaPath: '/introspection/sqon', +}; + +const catalogueIntrospection = { + catalogId: 'participants', + documentType: 'participant', + generatedAt: '2026-01-01T00:00:00.000Z', + meta: { authFiltered: false }, + operators: { keyword: ['in', 'not-in', 'some-not-in', 'all', 'filter'] }, + fields: { study: { displayName: 'Study', isArray: false, type: 'keyword' } }, +}; + +const executeQueryArguments = { catalogueId: 'participants', sqon: { op: 'and', content: [] }, fields: ['study'] }; + +/** The accepted answer a client attaches once the user has approved the query. */ +const APPROVED = { confirm: { action: 'accept', content: { confirm: true } } }; + +/** The tools the documented workflow walks, in the order `registerTools` registers them. */ +const TOOL_ORDER = ['list_catalogues', 'get_sqon_schema', 'get_catalogue_fields', 'build_sqon', 'execute_query']; + +/** + * The freshness hints every cacheable result must carry, spelled out rather than read from + * `RESULT_CACHE_HINTS`. Restating them is the point: comparing the wire against the same constant + * that configures it would pass whatever the values happened to become. + */ +const EXPECTED_CACHE_HINTS = [ + { method: 'tools/list', ttlMs: 3_600_000, cacheScope: 'public' }, + { method: 'prompts/list', ttlMs: 3_600_000, cacheScope: 'public' }, + { method: 'resources/templates/list', ttlMs: 3_600_000, cacheScope: 'public' }, + { method: 'server/discover', ttlMs: 3_600_000, cacheScope: 'public' }, + { method: 'resources/list', ttlMs: 60_000, cacheScope: 'private' }, + { + method: 'resources/read', + name: 'arranger://introspection/server', + params: { uri: 'arranger://introspection/server' }, + ttlMs: 60_000, + cacheScope: 'private', + }, +] as const; + +type CallResponse = { + result?: { + resultType?: string; + requestState?: string; + structuredContent?: { executed?: boolean }; + ttlMs?: number; + cacheScope?: string; + instructions?: string; + capabilities?: Record; + tools?: { name: string }[]; + _meta?: Record; + }; + error?: { code: number; data?: { reason?: string } }; +}; + +/** + * The whole server as `startServer` assembles it, reached over its real HTTP endpoint. + * + * Driven with `fetch` rather than through the SDK client because these tests need to send material a + * well-behaved client never would: the point is what the server does with a `requestState` somebody + * has altered in transit. + * + * The stubbed Arranger client records the queries it was asked to run, which is what a refusal has + * to leave empty. + */ +const startTestServer = async () => { + const executed: string[] = []; + const client = { + getServerIntrospection: () => Promise.resolve(serverIntrospection), + getCatalogueIntrospection: () => Promise.resolve(catalogueIntrospection), + executeQuery: (_endpoint: string, request: { query: string; rootFieldName: string }) => { + executed.push(request.query); + return Promise.resolve({ data: { [request.rootFieldName]: { hits: { total: 1, edges: [] } } } }); + }, + } as unknown as ArrangerClient; + + const requestStateCodec = createConfirmationCodec(config); + const { httpServer, close } = await startMcpHttpServer(config, () => + createMcpServer({ config, client, requestStateCodec }), + ); + const { port } = httpServer.address() as AddressInfo; + + /** + * Sends one JSON-RPC request. `name` fills the `Mcp-Name` header, which the entry requires + * whenever the body carries a `params.name` or `params.uri`. + */ + const call = async ( + method: string, + { name, params = {} }: { name?: string; params?: Record } = {}, + ): Promise => { + const response = await fetch(`http://127.0.0.1:${port}${config.mcp.path}`, { + method: 'POST', + headers: { + 'content-type': 'application/json', + accept: 'application/json, text/event-stream', + // The revision header is what classifies the call as modern; the method and name + // headers are required of every modern call and refused as a mismatch when absent. + 'mcp-protocol-version': PROTOCOL_REVISION, + 'mcp-method': method, + ...(name ? { 'mcp-name': name } : {}), + }, + body: JSON.stringify({ + jsonrpc: '2.0', + id: 1, + method, + params: { + ...params, + _meta: { + 'io.modelcontextprotocol/protocolVersion': PROTOCOL_REVISION, + 'io.modelcontextprotocol/clientCapabilities': { elicitation: {} }, + }, + }, + }), + }); + return response.json() as Promise; + }; + + /** Calls `execute_query`, adding whatever multi-round-trip material the round is testing. */ + const callExecuteQuery = (params: Record = {}): Promise => + call('tools/call', { + name: 'execute_query', + params: { name: 'execute_query', arguments: executeQueryArguments, ...params }, + }); + + return { call, callExecuteQuery, executed, close }; +}; + +suite('createMcpServer request state', () => { + test('a confirmed query runs when the state minted with the question comes back', async () => { + const { callExecuteQuery, executed, close } = await startTestServer(); + try { + const asked = await callExecuteQuery(); + assert.equal(asked.result?.resultType, 'input_required'); + assert.ok(asked.result?.requestState, 'expected the confirmation request to carry sealed state'); + + const answered = await callExecuteQuery({ + inputResponses: APPROVED, + requestState: asked.result.requestState, + }); + + assert.equal(answered.result?.structuredContent?.executed, true); + assert.equal(executed.length, 1); + } finally { + await close(); + } + }); + + // The signature is what makes the digest inside the state worth comparing: unsigned, a caller + // could simply claim the digest of whatever query it wanted to run. Verification is installed at + // the seam, so an altered value is answered before `execute_query` is entered at all and none of + // the tool's own checks come into it. + test('a state altered in transit is refused before the tool runs', async () => { + const { callExecuteQuery, executed, close } = await startTestServer(); + try { + const minted = (await callExecuteQuery()).result?.requestState ?? ''; + const altered = minted.slice(0, -4) + (minted.endsWith('AAAA') ? 'BBBB' : 'AAAA'); + assert.notEqual(altered, minted); + + const refused = await callExecuteQuery({ inputResponses: APPROVED, requestState: altered }); + + assert.equal(refused.error?.code, -32602); + assert.equal(refused.error?.data?.reason, 'invalid_request_state'); + assert.equal(executed.length, 0, 'a refused round must cost Arranger nothing'); + } finally { + await close(); + } + }); +}); + +suite('createMcpServer served surface', () => { + test('server/discover reports the instructions and capabilities', async () => { + const { call, close } = await startTestServer(); + try { + const { result } = await call('server/discover'); + + assert.equal(result?.instructions, SERVER_INSTRUCTIONS); + assert.ok(result?.capabilities?.tools, 'expected the tools capability to be advertised'); + assert.ok(result?.capabilities?.resources); + assert.ok(result?.capabilities?.prompts); + } finally { + await close(); + } + }); + + test('tools/list is ordered by registration', async () => { + const { call, close } = await startTestServer(); + try { + const { result } = await call('tools/list'); + + assert.deepEqual( + result?.tools?.map((tool) => tool.name), + TOOL_ORDER, + ); + } finally { + await close(); + } + }); + + test('every result carries the identity from the package manifest', async () => { + const { call, close } = await startTestServer(); + try { + const [listed, discovered] = await Promise.all([call('tools/list'), call('server/discover')]); + + for (const { result } of [listed, discovered]) { + const serverInfo = result?._meta?.['io.modelcontextprotocol/serverInfo']; + assert.equal(serverInfo?.name, 'arranger-mcp-server'); + assert.equal(serverInfo?.version, packageJson.version); + } + } finally { + await close(); + } + }); +}); + +// Asserted on the wire rather than on the configuration object. The type now rejects a method that +// is not cacheable, but nothing checks that a hint the SDK accepted actually reaches a client. +suite('createMcpServer cache hints', () => { + for (const { method, ttlMs, cacheScope, ...rest } of EXPECTED_CACHE_HINTS) { + const { name, params } = rest as { name?: string; params?: Record }; + test(`${method} is cacheable for ${ttlMs}ms, ${cacheScope}`, async () => { + const { call, close } = await startTestServer(); + try { + const { result } = await call(method, { name, params }); + + assert.equal(result?.ttlMs, ttlMs); + assert.equal(result?.cacheScope, cacheScope); + } finally { + await close(); + } + }); + } +}); diff --git a/apps/mcp-server/src/server.ts b/apps/mcp-server/src/server.ts index d6914b336..749a7a51f 100644 --- a/apps/mcp-server/src/server.ts +++ b/apps/mcp-server/src/server.ts @@ -1,24 +1,36 @@ -import { McpServer } from '@modelcontextprotocol/sdk/server/mcp'; +import { McpServer, type RequestStateCodec } from '@modelcontextprotocol/server'; import { createArrangerClient, type ArrangerClient } from '#arranger/client.js'; import { validateArrangerConnection } from '#arranger/validation.js'; -import { createHttpApp } from '#http/app.js'; +import { startMcpHttpServer } from '#http/server.js'; +import { RESULT_CACHE_HINTS } from '#mcp/cacheHints.js'; import { SERVER_INSTRUCTIONS } from '#mcp/instructions.js'; import { registerPrompts } from '#mcp/prompts.js'; +import { createConfirmationCodec, type ConfirmationState } from '#mcp/requestState.js'; import { registerResources } from '#mcp/resources.js'; import { registerTools } from '#mcp/tools.js'; import { createArrangerMcpConfig, type ArrangerMcpConfig } from '#utils/config.js'; import logger from '#utils/logger.js'; +import packageJson from '../package.json' with { type: 'json' }; + export type McpServerDeps = { config: ArrangerMcpConfig; client: ArrangerClient; + /** Signs and verifies `execute_query`'s confirmation state. Built once per process, not per request. */ + requestStateCodec: RequestStateCodec; }; export const createMcpServer = (deps: McpServerDeps): McpServer => { const server = new McpServer( - { name: 'arranger-mcp-server', version: '0.0.0-dev' }, - { instructions: SERVER_INSTRUCTIONS }, + { name: 'arranger-mcp-server', version: packageJson.version }, + { + instructions: SERVER_INSTRUCTIONS, + cacheHints: RESULT_CACHE_HINTS, + // Refuses a forged, expired or wrongly bound state before any handler runs, and decodes a + // good one for `ctx.mcpReq.requestState()` to read. + requestState: { verify: deps.requestStateCodec.verify }, + }, ); registerResources(server, deps); registerTools(server, deps); @@ -31,13 +43,13 @@ export const startServer = async (): Promise => { const client = createArrangerClient(config); await validateArrangerConnection(config, client); - const deps: McpServerDeps = { config, client }; - const { app, closeAllSessions } = createHttpApp(config, () => createMcpServer(deps)); + const deps: McpServerDeps = { config, client, requestStateCodec: createConfirmationCodec(config) }; + // One instance per request: the handler serves each request independently, so nothing is held + // between them and there is no session map to reap on shutdown. + const { close } = await startMcpHttpServer(config, () => createMcpServer(deps)); const { host, port, path } = config.mcp; - app.listen(port, () => { - logger.info(`MCP server running at http://${host}:${port}${path}`); - }); + logger.info(`MCP server running at http://${host}:${port}${path}`); const gracefulShutdown = async (signal: string) => { logger.info(`Received ${signal}, initiating graceful shutdown...`); @@ -51,7 +63,7 @@ export const startServer = async (): Promise => { hardShutdownTimeout.unref(); // Allow process to exit if this is the only thing left try { - await closeAllSessions(); + await close(); logger.info('Graceful shutdown complete, exiting now.'); process.exit(0); } catch (error) { diff --git a/apps/mcp-server/src/utils/config.test.ts b/apps/mcp-server/src/utils/config.test.ts index 71ed70070..738e7e388 100644 --- a/apps/mcp-server/src/utils/config.test.ts +++ b/apps/mcp-server/src/utils/config.test.ts @@ -8,9 +8,16 @@ const ENV_KEYS = [ 'MCP_HOST', 'MCP_PORT', 'MCP_PATH', + 'MCP_ALLOWED_HOSTS', + 'MCP_ALLOWED_ORIGINS', + 'MCP_REQUEST_STATE_SECRET', + 'MCP_MAX_BODY_BYTES', 'LOG_LEVEL', ] as const; +/** What an unset allowlist resolves to on a loopback bind, matching the SDK's own localhost guards. */ +const LOCALHOST_ALLOWED = ['localhost', '127.0.0.1', '[::1]']; + // Redefining ArrangerMcpConfig type to avoid importing from config.ts before the logger module is mocked type ArrangerMcpConfig = { arrangerBaseUrl: string; @@ -20,6 +27,10 @@ type ArrangerMcpConfig = { host: string; port: number; path: string; + allowedHosts: 'any' | string[]; + allowedOrigins: string[]; + requestStateSecret: string | undefined; + maxBodyBytes: number; }; }; @@ -109,6 +120,10 @@ suite('createArrangerMcpConfig', () => { MCP_HOST: '127.0.0.1', MCP_PORT: '4200', MCP_PATH: '/custom-mcp', + MCP_ALLOWED_HOSTS: 'mcp.example.com, arranger-mcp', + MCP_ALLOWED_ORIGINS: 'portal.example.com', + MCP_REQUEST_STATE_SECRET: 'a-thirty-two-byte-or-longer-signing-key', + MCP_MAX_BODY_BYTES: '2_097_152', LOG_LEVEL: 'debug', }); @@ -122,14 +137,19 @@ suite('createArrangerMcpConfig', () => { host: '127.0.0.1', port: 4200, path: '/custom-mcp', + allowedHosts: ['mcp.example.com', 'arranger-mcp'], + allowedOrigins: ['portal.example.com'], + requestStateSecret: 'a-thirty-two-byte-or-longer-signing-key', + maxBodyBytes: 2_097_152, }, }); }); - test('builds config with defaults for optional variables when only required variables are provided', () => { + test('builds config with defaults for optional variables on a loopback bind', () => { setEnv({ ARRANGER_BASE_URL: 'http://localhost:5050', ARRANGER_CATALOGUES: 'catalogue-a', + MCP_HOST: '127.0.0.1', }); const config = createArrangerMcpConfig(); @@ -139,9 +159,13 @@ suite('createArrangerMcpConfig', () => { catalogues: ['catalogue-a'], requestTimeoutMs: 10_000, mcp: { - host: '0.0.0.0', + host: '127.0.0.1', port: 3100, path: '/mcp', + allowedHosts: LOCALHOST_ALLOWED, + allowedOrigins: LOCALHOST_ALLOWED, + requestStateSecret: undefined, + maxBodyBytes: 102_400, }, }); }); @@ -150,6 +174,7 @@ suite('createArrangerMcpConfig', () => { setEnv({ ARRANGER_BASE_URL: 'https://arranger.example.com/', ARRANGER_CATALOGUES: 'catalogue-a', + MCP_HOST: '127.0.0.1', }); const config = createArrangerMcpConfig(); @@ -157,10 +182,26 @@ suite('createArrangerMcpConfig', () => { assert.strictEqual(config.arrangerBaseUrl, 'https://arranger.example.com'); }); + // The variable ships listed but blank in `.env.schema`, so blank has to mean "unset" rather + // than "a zero-length signing key". + test('reads an empty MCP_REQUEST_STATE_SECRET as unset', () => { + setEnv({ + ARRANGER_BASE_URL: 'https://arranger.example.com', + ARRANGER_CATALOGUES: 'catalogue-a', + MCP_HOST: '127.0.0.1', + MCP_REQUEST_STATE_SECRET: '', + }); + + const config = createArrangerMcpConfig(); + + assert.strictEqual(config.mcp.requestStateSecret, undefined); + }); + test('filters empty entries from ARRANGER_CATALOGUES', () => { setEnv({ ARRANGER_BASE_URL: 'https://arranger.example.com', ARRANGER_CATALOGUES: 'catalogue-a,, catalogue-b, ,catalogue-c,', + MCP_HOST: '127.0.0.1', }); const config = createArrangerMcpConfig(); @@ -286,5 +327,116 @@ suite('createArrangerMcpConfig', () => { assert.strictEqual(exitCode, 1); assert.match(errorLogs.join(''), /LOG_LEVEL must be one of: trace, debug, info, warn, error, fatal/); }); + + // Refused here rather than at the codec, which throws a RangeError from inside server startup + // where it reads as a crash rather than as a misconfigured variable. + test('exits when MCP_REQUEST_STATE_SECRET is shorter than the codec accepts', () => { + setEnv({ + ARRANGER_BASE_URL: 'https://arranger.example.com', + ARRANGER_CATALOGUES: 'catalogue-a', + MCP_HOST: '127.0.0.1', + MCP_REQUEST_STATE_SECRET: 'too-short', + }); + + assert.throws(() => createArrangerMcpConfig(), /__process_exit__/); + assert.strictEqual(exitCode, 1); + assert.match(errorLogs.join(''), /MCP_REQUEST_STATE_SECRET must be at least 32 bytes/); + }); + + test('exits when MCP_MAX_BODY_BYTES is not a number', () => { + setEnv({ + ARRANGER_BASE_URL: 'https://arranger.example.com', + ARRANGER_CATALOGUES: 'catalogue-a', + MCP_HOST: '127.0.0.1', + MCP_MAX_BODY_BYTES: 'not-a-number', + }); + + assert.throws(() => createArrangerMcpConfig(), /__process_exit__/); + assert.strictEqual(exitCode, 1); + assert.match(errorLogs.join(''), /MCP_MAX_BODY_BYTES must be a valid number/); + }); + + test('exits when MCP_MAX_BODY_BYTES is not positive', () => { + setEnv({ + ARRANGER_BASE_URL: 'https://arranger.example.com', + ARRANGER_CATALOGUES: 'catalogue-a', + MCP_HOST: '127.0.0.1', + MCP_MAX_BODY_BYTES: '0', + }); + + assert.throws(() => createArrangerMcpConfig(), /__process_exit__/); + assert.strictEqual(exitCode, 1); + assert.match(errorLogs.join(''), /MCP_MAX_BODY_BYTES must be a positive number/); + }); + }); + + // The server binds every interface by default, and the SDK only warns about that. A warning is + // the wrong volume for a DNS rebinding exposure that appears exactly when someone moves from a + // laptop to a container, so configuration refuses to resolve instead. + suite('Host allowlist safety', () => { + const requiredEnv = { + ARRANGER_BASE_URL: 'https://arranger.example.com', + ARRANGER_CATALOGUES: 'catalogue-a', + }; + + test('exits when the default bind is used without MCP_ALLOWED_HOSTS', () => { + setEnv(requiredEnv); + + assert.throws(() => createArrangerMcpConfig(), /__process_exit__/); + assert.strictEqual(exitCode, 1); + assert.match(errorLogs.join(''), /MCP_HOST is "0\.0\.0\.0".*MCP_ALLOWED_HOSTS is not set/); + }); + + test('exits when a routable bind is used without MCP_ALLOWED_HOSTS', () => { + setEnv({ ...requiredEnv, MCP_HOST: '10.1.2.3' }); + + assert.throws(() => createArrangerMcpConfig(), /__process_exit__/); + assert.strictEqual(exitCode, 1); + assert.match(errorLogs.join(''), /MCP_HOST is "10\.1\.2\.3"/); + }); + + test('names the escape hatch in the failure message', () => { + setEnv(requiredEnv); + + assert.throws(() => createArrangerMcpConfig(), /__process_exit__/); + assert.match(errorLogs.join(''), /MCP_ALLOWED_HOSTS=\*/); + }); + + for (const host of ['127.0.0.1', 'localhost', '::1']) { + test(`resolves the localhost allowlists for a ${host} bind with no allowlist set`, () => { + setEnv({ ...requiredEnv, MCP_HOST: host }); + + const { mcp } = createArrangerMcpConfig(); + + assert.deepStrictEqual(mcp.allowedHosts, LOCALHOST_ALLOWED); + assert.deepStrictEqual(mcp.allowedOrigins, LOCALHOST_ALLOWED); + }); + } + + test('accepts a routable bind once MCP_ALLOWED_HOSTS names the hostnames', () => { + setEnv({ ...requiredEnv, MCP_HOST: '0.0.0.0', MCP_ALLOWED_HOSTS: 'arranger-mcp, mcp.example.org' }); + + const { mcp } = createArrangerMcpConfig(); + + assert.deepStrictEqual(mcp.allowedHosts, ['arranger-mcp', 'mcp.example.org']); + }); + + test('treats MCP_ALLOWED_HOSTS=* as delegating Host validation to a gateway', () => { + setEnv({ ...requiredEnv, MCP_HOST: '0.0.0.0', MCP_ALLOWED_HOSTS: '*' }); + + const { mcp } = createArrangerMcpConfig(); + + assert.strictEqual(mcp.allowedHosts, 'any'); + }); + + // An unset value is an empty allowlist rather than a disabled check: the Origin guard passes + // requests carrying no `Origin`, which is every non-browser MCP client, and rejects the rest. + test('leaves MCP_ALLOWED_ORIGINS empty on a routable bind when it is not set', () => { + setEnv({ ...requiredEnv, MCP_HOST: '0.0.0.0', MCP_ALLOWED_HOSTS: 'arranger-mcp' }); + + const { mcp } = createArrangerMcpConfig(); + + assert.deepStrictEqual(mcp.allowedOrigins, []); + }); }); }); diff --git a/apps/mcp-server/src/utils/config.ts b/apps/mcp-server/src/utils/config.ts index f5491315e..e188d19db 100644 --- a/apps/mcp-server/src/utils/config.ts +++ b/apps/mcp-server/src/utils/config.ts @@ -4,6 +4,31 @@ import { createLogger } from '#utils/logger.js'; const DEFAULT_REQUEST_TIMEOUT_MS = 10_000; +/** + * Ceiling on a request body, preserving the `100kb` that `express.json()` applied before this app + * served MCP on plain `node:http` (which does not automatically enforce a limit on its own). + */ +const DEFAULT_MAX_BODY_BYTES = 102_400; + +/** Bind addresses that are only reachable from this host, so a Host allowlist is not required. */ +const LOCALHOST_HOSTNAMES = ['127.0.0.1', 'localhost', '::1']; + +/** + * Hostnames the SDK's own localhost guards allow. `[::1]` is bracketed because both guards compare + * against `new URL(...).hostname`, which brackets IPv6 literals. + */ +const LOCALHOST_ALLOWED_HOSTNAMES = ['localhost', '127.0.0.1', '[::1]']; + +/** `MCP_ALLOWED_HOSTS` value meaning "an upstream gateway validates the Host header, do not". */ +const ALLOW_ANY_HOST = '*'; + +/** + * Shortest `MCP_REQUEST_STATE_SECRET` the HMAC codec accepts, below which it throws a `RangeError`. + * Counted in UTF-8 bytes because that is what the codec counts, and that is not the same as + * characters once the value leaves ASCII: an accented letter is two bytes, an emoji four. + */ +const MIN_REQUEST_STATE_SECRET_BYTES = 32; + const logger = createLogger('Config'); /** @@ -19,22 +44,29 @@ const logger = createLogger('Config'); const trimTrailingSlash = (value: string) => value.replace(/\/+$/, ''); /** - * Convert a comma-separated string of catalogue names into an array of trimmed strings, filtering out any empty values. - * @param cataloguesString - A comma-separated string of catalogue names. - * @returns An array of trimmed catalogue names. + * Convert a comma-separated string into an array of trimmed strings, filtering out any empty values. + * @param value - A comma-separated string. + * @returns An array of trimmed entries. * @example * ```ts - * parseCatalogueList('catalogue1,catalogue2,catalogue3') // returns ['catalogue1', 'catalogue2', 'catalogue3'] - * parseCatalogueList('catalogue1,, catalogue2, ,catalogue3,') // returns ['catalogue1', 'catalogue2', 'catalogue3'] + * parseCommaSeparatedList('a,b,c') // returns ['a', 'b', 'c'] + * parseCommaSeparatedList('a,, b, ,c,') // returns ['a', 'b', 'c'] * ``` */ -const parseCatalogueList = (cataloguesString: string): string[] => { - return cataloguesString +const parseCommaSeparatedList = (value: string): string[] => { + return value .split(',') - .map((catalogue) => catalogue.trim()) + .map((entry) => entry.trim()) .filter(Boolean); }; +/** + * Strips underscores from a numeric environment variable so large values can be written in a + * human-friendly form (`102_400`), leaving non-string input untouched for Zod to coerce. + */ +const stripNumericSeparators = (value: unknown): unknown => + typeof value === 'string' ? value.replace(/_/g, '') : value; + /** * Zod schema for validating and parsing environment variables for the Arranger MCP server configuration. * This schema ensures that all required values are present and correctly formatted, and provides default values @@ -56,15 +88,9 @@ const envSchema = zod.object({ error: 'ARRANGER_CATALOGUES is required and must be a comma-separated list of catalogue names', }) .min(1, 'ARRANGER_CATALOGUES is required and cannot be empty') - .transform(parseCatalogueList), + .transform(parseCommaSeparatedList), ARRANGER_REQUEST_TIMEOUT_MS: zod.preprocess( - (value) => { - if (typeof value === 'string') { - // Remove underscores to allow for more human-friendly large numbers (e.g., "10_000" instead of "10000") - return value.replace(/_/g, ''); - } - return value; - }, + stripNumericSeparators, zod.coerce .number({ error: 'ARRANGER_REQUEST_TIMEOUT_MS must be a valid number', @@ -77,6 +103,29 @@ const envSchema = zod.object({ MCP_HOST: zod.string().optional().default('0.0.0.0'), MCP_PORT: zod.coerce.number().int().positive().max(65535, 'MCP_PORT cannot exceed 65535').optional().default(3100), MCP_PATH: zod.string().optional().default('/mcp'), + MCP_ALLOWED_HOSTS: zod.string().optional().default(''), + MCP_ALLOWED_ORIGINS: zod.string().optional().default(''), + // An empty value reads as unset rather than as a too-short key: `.env.schema` lists the variable + // blank, and blank is the supported single-replica default. + MCP_REQUEST_STATE_SECRET: zod.preprocess( + (value) => (value === '' ? undefined : value), + zod + .string() + .refine( + (value) => Buffer.byteLength(value, 'utf8') >= MIN_REQUEST_STATE_SECRET_BYTES, + `MCP_REQUEST_STATE_SECRET must be at least ${MIN_REQUEST_STATE_SECRET_BYTES} bytes`, + ) + .optional(), + ), + MCP_MAX_BODY_BYTES: zod.preprocess( + stripNumericSeparators, + zod.coerce + .number({ error: 'MCP_MAX_BODY_BYTES must be a valid number' }) + .int('MCP_MAX_BODY_BYTES must be an integer') + .positive('MCP_MAX_BODY_BYTES must be a positive number') + .optional() + .default(DEFAULT_MAX_BODY_BYTES), + ), LOG_LEVEL: zod .enum(['trace', 'debug', 'info', 'warn', 'error', 'fatal'], { error: 'LOG_LEVEL must be one of: trace, debug, info, warn, error, fatal', @@ -85,20 +134,76 @@ const envSchema = zod.object({ .default('info'), }); +/** + * Resolves `MCP_ALLOWED_HOSTS` into the list the Host guard is built from. + * + * An unset value on a loopback bind resolves to the localhost hostnames rather than to an empty + * list, matching what the SDK's own adapters do for a localhost bind. On a routable bind it + * resolves to an empty list, which the refinement below then refuses, so an empty list never + * reaches the Host guard. + * @returns `'any'` when Host validation is delegated to an upstream gateway, otherwise the allowed + * hostnames. + */ +const resolveAllowedHosts = (rawValue: string, host: string): 'any' | string[] => { + const allowedHosts = parseCommaSeparatedList(rawValue); + if (allowedHosts.includes(ALLOW_ANY_HOST)) { + return 'any'; + } + if (allowedHosts.length > 0) { + return allowedHosts; + } + return LOCALHOST_HOSTNAMES.includes(host) ? LOCALHOST_ALLOWED_HOSTNAMES : []; +}; + +/** + * Resolves `MCP_ALLOWED_ORIGINS` into the list the Origin guard is built from. + * + * An empty list is a live check rather than a disabled one: the guard passes requests carrying no + * `Origin` (which is every non-browser MCP client) and rejects any browser origin. As with hosts, an + * unset value on a loopback bind resolves to the localhost origins so browser-based tooling works + * against a local server. + */ +const resolveAllowedOrigins = (rawValue: string, host: string): string[] => { + const allowedOrigins = parseCommaSeparatedList(rawValue); + if (allowedOrigins.length > 0) { + return allowedOrigins; + } + return LOCALHOST_HOSTNAMES.includes(host) ? LOCALHOST_ALLOWED_HOSTNAMES : []; +}; + /** * Zod schema for the Arranger MCP server configuration, derived from `envSchema`. * Transforms the validated env vars into a structured config object. */ -const ArrangerMcpConfig = envSchema.transform((data) => ({ - arrangerBaseUrl: data.ARRANGER_BASE_URL, - catalogues: data.ARRANGER_CATALOGUES, - requestTimeoutMs: data.ARRANGER_REQUEST_TIMEOUT_MS, - mcp: { - host: data.MCP_HOST, - port: data.MCP_PORT, - path: data.MCP_PATH, - }, -})); +const ArrangerMcpConfig = envSchema + .transform((data) => ({ + arrangerBaseUrl: data.ARRANGER_BASE_URL, + catalogues: data.ARRANGER_CATALOGUES, + requestTimeoutMs: data.ARRANGER_REQUEST_TIMEOUT_MS, + mcp: { + host: data.MCP_HOST, + port: data.MCP_PORT, + path: data.MCP_PATH, + allowedHosts: resolveAllowedHosts(data.MCP_ALLOWED_HOSTS, data.MCP_HOST), + allowedOrigins: resolveAllowedOrigins(data.MCP_ALLOWED_ORIGINS, data.MCP_HOST), + requestStateSecret: data.MCP_REQUEST_STATE_SECRET, + maxBodyBytes: data.MCP_MAX_BODY_BYTES, + }, + })) + // Refuse to start rather than warn. Binding a routable interface with no Host allowlist leaves + // the server open to DNS rebinding, and it is exactly the configuration an operator reaches for + // when moving from a laptop into a container, so a warning would be read as noise. + .superRefine(({ mcp }, ctx) => { + if (mcp.allowedHosts === 'any' || mcp.allowedHosts.length > 0 || LOCALHOST_HOSTNAMES.includes(mcp.host)) { + return; + } + ctx.addIssue( + `MCP_HOST is "${mcp.host}", which is reachable from outside this machine, but MCP_ALLOWED_HOSTS is not set. ` + + 'Set MCP_ALLOWED_HOSTS to the hostname(s) clients use to reach this server ' + + '(for example "arranger-mcp,mcp.example.org"), or set MCP_ALLOWED_HOSTS=* if an upstream gateway ' + + 'validates the Host header. Binding a routable interface without either is a DNS rebinding risk.', + ); + }); export type ArrangerMcpConfig = zod.infer; /** diff --git a/apps/mcp-server/src/utils/inMemoryEventStore.ts b/apps/mcp-server/src/utils/inMemoryEventStore.ts deleted file mode 100644 index 3f847ba3a..000000000 --- a/apps/mcp-server/src/utils/inMemoryEventStore.ts +++ /dev/null @@ -1,81 +0,0 @@ -// In-memory event store implementation from the MCP TypeScript SDK examples: -// https://github.com/modelcontextprotocol/typescript-sdk/blob/v1.x/src/examples/shared/inMemoryEventStore.ts -// TODO: Replace with a persistent storage solution for production use -import { type EventStore } from '@modelcontextprotocol/sdk/server/streamableHttp'; -import { type JSONRPCMessage } from '@modelcontextprotocol/sdk/types'; - -/** - * Simple in-memory implementation of the EventStore interface for resumability - * This is primarily intended for examples and testing, not for production use - * where a persistent storage solution would be more appropriate. - */ -export class InMemoryEventStore implements EventStore { - private events = new Map(); - - /** - * Generates a unique event ID for a given stream ID - */ - private generateEventId(streamId: string): string { - return `${streamId}_${Date.now()}_${Math.random().toString(36).substring(2, 10)}`; - } - - /** - * Extracts the stream ID from an event ID - */ - private getStreamIdFromEventId(eventId: string): string { - const parts = eventId.split('_'); - return parts.length > 0 ? parts[0] : ''; - } - - /** - * Stores an event with a generated event ID - * Implements EventStore.storeEvent - */ - async storeEvent(streamId: string, message: JSONRPCMessage): Promise { - const eventId = this.generateEventId(streamId); - this.events.set(eventId, { streamId, message }); - return eventId; - } - - /** - * Replays events that occurred after a specific event ID - * Implements EventStore.replayEventsAfter - */ - async replayEventsAfter( - lastEventId: string, - { send }: { send: (eventId: string, message: JSONRPCMessage) => Promise }, - ): Promise { - if (!lastEventId || !this.events.has(lastEventId)) { - return ''; - } - - // Extract the stream ID from the event ID - const streamId = this.getStreamIdFromEventId(lastEventId); - if (!streamId) { - return ''; - } - - let foundLastEvent = false; - - // Sort events by eventId for chronological ordering - const sortedEvents = [...this.events.entries()].sort((a, b) => a[0].localeCompare(b[0])); - - for (const [eventId, { streamId: eventStreamId, message }] of sortedEvents) { - // Only include events from the same stream - if (eventStreamId !== streamId) { - continue; - } - - // Start sending events after we find the lastEventId - if (eventId === lastEventId) { - foundLastEvent = true; - continue; - } - - if (foundLastEvent) { - await send(eventId, message); - } - } - return streamId; - } -} diff --git a/integration-tests/mcp-server/package.json b/integration-tests/mcp-server/package.json index 97ef0f9b4..009dab298 100644 --- a/integration-tests/mcp-server/package.json +++ b/integration-tests/mcp-server/package.json @@ -2,7 +2,7 @@ "name": "integration-tests-mcp-server", "dependencies": { "@elastic/elasticsearch": "^7.17.14", - "@modelcontextprotocol/sdk": "^1.29.0", + "@modelcontextprotocol/client": "^2.0.0", "@overture-stack/arranger-graphql-router": "file:../../modules/graphql-router", "@overture-stack/arranger-types": "file:../../modules/types", "dotenv": "^16.6.1" diff --git a/integration-tests/mcp-server/test/buildSqon.ts b/integration-tests/mcp-server/test/buildSqon.ts index 113ddde92..898b1f79e 100644 --- a/integration-tests/mcp-server/test/buildSqon.ts +++ b/integration-tests/mcp-server/test/buildSqon.ts @@ -1,7 +1,7 @@ import assert from 'node:assert/strict'; import { test } from 'node:test'; -import { type Client } from '@modelcontextprotocol/sdk/client'; +import { type Client } from '@modelcontextprotocol/client'; export type BuildSqonEnv = { getClient: () => Client; diff --git a/integration-tests/mcp-server/test/executeQuery.ts b/integration-tests/mcp-server/test/executeQuery.ts index cc2c4b11c..24975e950 100644 --- a/integration-tests/mcp-server/test/executeQuery.ts +++ b/integration-tests/mcp-server/test/executeQuery.ts @@ -1,9 +1,9 @@ import assert from 'node:assert/strict'; import { test } from 'node:test'; -import { Client } from '@modelcontextprotocol/sdk/client'; -import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp'; -import { ElicitRequestSchema, type ElicitRequest, type ElicitResult } from '@modelcontextprotocol/sdk/types'; +import type { Client, ElicitRequest, ElicitResult } from '@modelcontextprotocol/client'; + +import { connectMcpClient } from './mcpClient.js'; export type ExecuteQueryEnv = { getClient: () => Client; @@ -53,22 +53,25 @@ const getErrorText = (result: Awaited>): string = }; /** - * Connects a second MCP client that advertises the elicitation capability, so the - * execute_query tool's user-confirmation path runs (the shared suite client does not - * advertise elicitation, so every other test takes the skip-confirmation path). + * Connects a client that answers the confirmation with whatever the test decides. + * + * The shared suite client always approves, so a test that needs a decline, or that wants to read the + * prompt it was shown, brings its own client and its own handler. + * + * @param serverUrl - The MCP endpoint to connect to. + * @param handleElicit - Answers the confirmation request the tool emits. + * @returns The connected client. */ -const connectElicitingClient = async ( +const connectElicitingClient = ( serverUrl: string, handleElicit: (request: ElicitRequest) => ElicitResult, -): Promise => { - const elicitingClient = new Client( - { name: 'arranger-mcp-server-integration-tests-eliciting', version: '0.0.0-test' }, +): Promise => + connectMcpClient( + serverUrl, + 'arranger-mcp-server-integration-tests-eliciting', { capabilities: { elicitation: {} } }, + (client) => client.setRequestHandler('elicitation/create', async (request) => handleElicit(request)), ); - elicitingClient.setRequestHandler(ElicitRequestSchema, async (request) => handleElicit(request)); - await elicitingClient.connect(new StreamableHTTPClientTransport(new URL(serverUrl))); - return elicitingClient; -}; // Dataset reference (test/assets/catalogue_a.data.json): // a-001 age 34 Alive | a-002 age 51 Deceased | a-003 age 62 Alive | a-004 age 8 Unknown | a-005 age 45 Deceased @@ -283,6 +286,10 @@ export default ({ getClient, getServerUrl }: ExecuteQueryEnv) => { assert.match(text, /requires at least one entry in aggregationFields/); }); + // Confirmation is now a two-request exchange: the tool returns `input_required`, the client + // fulfils the embedded elicitation and re-invokes with the answer attached. The client driver + // does that automatically, so `callTool` still resolves with the final result and these + // assertions are unchanged from the pre-migration flow. test('14.declining the elicitation confirmation skips execution', async () => { const elicitMessages: string[] = []; const elicitingClient = await connectElicitingClient(getServerUrl(), (request) => { @@ -467,4 +474,27 @@ export default ({ getClient, getServerUrl }: ExecuteQueryEnv) => { assert.equal(structured.total, 1); assert.deepEqual(structured.hits, [{ sample_id: 'b-002', ca19_9_level: 37.5 }]); }); + + // With 2025-era serving gone, a client that cannot elicit is the only remaining route to running + // a query nobody approved. Refusing is what makes confirm-before-execute an invariant of the tool + // rather than something the caller can opt out of by omitting a capability. + test('22.refuses a client that did not declare elicitation, rather than executing unconfirmed', async () => { + const silentClient = await connectMcpClient(getServerUrl(), 'arranger-mcp-server-integration-tests-no-elicit'); + + try { + const result = await callExecuteQuery(silentClient, { + catalogueId: 'catalogue-a', + sqon: EMPTY_ROOT_SQON, + fields: ['analysis_id'], + }); + const text = getErrorText(result); + + assert.match(text, /elicitation/); + // Points at the tool that inspects a filter without running it, so the refusal is + // actionable rather than a dead end. + assert.match(text, /build_sqon/); + } finally { + await silentClient.close(); + } + }); }; diff --git a/integration-tests/mcp-server/test/index.test.ts b/integration-tests/mcp-server/test/index.test.ts index ab9a09130..697777c03 100644 --- a/integration-tests/mcp-server/test/index.test.ts +++ b/integration-tests/mcp-server/test/index.test.ts @@ -1,8 +1,7 @@ import { after, before, suite } from 'node:test'; import path from 'path'; -import { Client } from '@modelcontextprotocol/sdk/client'; -import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp'; +import { type Client } from '@modelcontextprotocol/client'; import { stringToNumber } from '@overture-stack/arranger-types/tools'; import dotenv from 'dotenv'; @@ -17,6 +16,7 @@ import catalogueBData from './assets/catalogue_b.data.json' with { type: 'json' import catalogueBMappings from './assets/catalogue_b.mappings.json' with { type: 'json' }; import buildSqon from './buildSqon.js'; import executeQuery from './executeQuery.js'; +import { connectApprovingClient } from './mcpClient.js'; import readPrompts from './readPrompts.js'; import readResources from './readResources.js'; import readTools from './readTools.js'; @@ -210,6 +210,14 @@ suite('integration-tests/mcp-server', { concurrency: false }, () => { host: '127.0.0.1', port: mcpPort, path: '/mcp', + // Spelled out rather than derived: these are what a loopback bind resolves to, + // and stating them keeps the suite honest if that defaulting ever changes. + allowedHosts: ['localhost', '127.0.0.1', '[::1]'], + allowedOrigins: ['localhost', '127.0.0.1', '[::1]'], + // Left unset, which is the single-replica default: the suite runs one server, and + // exercising the per-process key is exercising what an operator gets by default. + requestStateSecret: undefined, + maxBodyBytes: 102_400, }, }); } catch (err) { @@ -224,9 +232,7 @@ suite('integration-tests/mcp-server', { concurrency: false }, () => { console.error('\n------------------------------------'); console.log('Connecting MCP Client over Streamable HTTP\n'); - const mcpClient = new Client({ name: 'arranger-mcp-server-integration-tests', version: '0.0.0-test' }); - const transport = new StreamableHTTPClientTransport(new URL(mcpServer.url)); - await mcpClient.connect(transport); + const mcpClient = await connectApprovingClient(mcpServer.url, 'arranger-mcp-server-integration-tests'); context.mcpClient = mcpClient; context.mcpServerUrl = mcpServer.url; } catch (err) { diff --git a/integration-tests/mcp-server/test/mcpClient.ts b/integration-tests/mcp-server/test/mcpClient.ts new file mode 100644 index 000000000..f6f2f3f1b --- /dev/null +++ b/integration-tests/mcp-server/test/mcpClient.ts @@ -0,0 +1,58 @@ +import { Client, StreamableHTTPClientTransport, type ClientOptions } from '@modelcontextprotocol/client'; + +/** + * Pins every test client to the protocol revision this server serves. + * + * `@modelcontextprotocol/client` defaults to `mode: 'legacy'`, so a client built without this + * connects with the 2025-era `initialize` handshake. The server is `legacy: 'reject'`, so that + * connection is refused outright, and this suite would be asserting nothing about the era we ship. + * + * Pinned rather than `'auto'` on purpose: `'auto'` falls back to the legacy handshake when the probe + * is inconclusive, which is exactly the silent downgrade these tests exist to catch. + */ +export const MODERN_PROTOCOL_REVISION = '2026-07-28'; + +const versionNegotiation = { mode: { pin: MODERN_PROTOCOL_REVISION } } as const; + +/** + * Connects an MCP client to the server over Streamable HTTP, pinned to the modern protocol era. + * + * @param serverUrl - The MCP endpoint to connect to. + * @param name - Client name reported to the server, so a failure names the client that caused it. + * @param options - Extra client options, e.g. the capabilities a test needs to advertise. + * @param beforeConnect - Runs against the constructed client before it connects. Anything the client + * has to be able to answer from its first exchange, a request handler in particular, belongs here + * rather than on the returned client. + * @returns The connected client. + */ +export const connectMcpClient = async ( + serverUrl: string, + name: string, + options: Omit = {}, + beforeConnect?: (client: Client) => void, +): Promise => { + const client = new Client({ name, version: '0.0.0-test' }, { ...options, versionNegotiation }); + beforeConnect?.(client); + await client.connect(new StreamableHTTPClientTransport(new URL(serverUrl))); + return client; +}; + +/** + * Connects a client that behaves like a host supporting confirm-before-execute, and always approves. + * + * `execute_query` refuses a client that does not declare `elicitation`, so the shared suite client + * has to declare it or every query test would be answered with that refusal. Auto-fulfilment is on + * by default, so the client answers the embedded request and retries the call itself, which is why + * `callTool` resolves with the final result rather than the intermediate `input_required`. + * + * @param serverUrl - The MCP endpoint to connect to. + * @param name - Client name reported to the server, so a failure names the client that caused it. + * @returns The connected client, approving every confirmation it is asked for. + */ +export const connectApprovingClient = (serverUrl: string, name: string): Promise => + connectMcpClient(serverUrl, name, { capabilities: { elicitation: {} } }, (client) => { + client.setRequestHandler('elicitation/create', async () => ({ + action: 'accept', + content: { confirm: true }, + })); + }); diff --git a/integration-tests/mcp-server/test/readPrompts.ts b/integration-tests/mcp-server/test/readPrompts.ts index 0f93bd20a..62a4b9fc2 100644 --- a/integration-tests/mcp-server/test/readPrompts.ts +++ b/integration-tests/mcp-server/test/readPrompts.ts @@ -1,7 +1,7 @@ import assert from 'node:assert/strict'; import { test } from 'node:test'; -import { type Client } from '@modelcontextprotocol/sdk/client'; +import { type Client } from '@modelcontextprotocol/client'; export type PromptEnv = { getClient: () => Client; @@ -156,12 +156,15 @@ export default ({ getClient, configuredCatalogues, expectedDocumentTypes }: Prom ); // An empty string does reach the field, so this one can be pinned to `goal` specifically. + // SDK v2 renders the failing path unquoted and directly after the prompt name + // (`... query_arranger: goal: Too small`), where v1 quoted it as `"goal"`. Matching the + // prompt-name-then-path shape pins the same thing without coupling to Zod's issue wording. await assert.rejects( () => getClient().getPrompt({ name: 'query_arranger', arguments: { goal: '' } }), (error: unknown) => { const message = error instanceof Error ? error.message : String(error); assert.match(message, /Invalid arguments for prompt query_arranger/); - assert.match(message, /"goal"/, 'expected the failing argument path to name goal'); + assert.match(message, /query_arranger: goal\b/, 'expected the failing argument path to name goal'); return true; }, 'expected an empty goal to be rejected by the min(1) constraint', diff --git a/integration-tests/mcp-server/test/readResources.ts b/integration-tests/mcp-server/test/readResources.ts index ee5a952ae..e1386fd44 100644 --- a/integration-tests/mcp-server/test/readResources.ts +++ b/integration-tests/mcp-server/test/readResources.ts @@ -1,7 +1,7 @@ import assert from 'node:assert/strict'; import { test } from 'node:test'; -import { type Client } from '@modelcontextprotocol/sdk/client'; +import { type Client } from '@modelcontextprotocol/client'; export type ResourceEnv = { getClient: () => Client; diff --git a/integration-tests/mcp-server/test/readTools.ts b/integration-tests/mcp-server/test/readTools.ts index 875b5bad1..c15ec215d 100644 --- a/integration-tests/mcp-server/test/readTools.ts +++ b/integration-tests/mcp-server/test/readTools.ts @@ -1,7 +1,7 @@ import assert from 'node:assert/strict'; import { test } from 'node:test'; -import { type Client } from '@modelcontextprotocol/sdk/client'; +import { type Client } from '@modelcontextprotocol/client'; export type ToolEnv = { getClient: () => Client; diff --git a/integration-tests/mcp-server/test/spinupActive.ts b/integration-tests/mcp-server/test/spinupActive.ts index 1efe746a1..f011af5d4 100644 --- a/integration-tests/mcp-server/test/spinupActive.ts +++ b/integration-tests/mcp-server/test/spinupActive.ts @@ -1,7 +1,7 @@ import assert from 'node:assert/strict'; import { test } from 'node:test'; -import { type Client } from '@modelcontextprotocol/sdk/client'; +import { type Client } from '@modelcontextprotocol/client'; import { SERVER_INSTRUCTIONS } from '../../../apps/mcp-server/src/mcp/instructions.js'; @@ -19,8 +19,11 @@ export type SpinupEnv = { * by the server are visible to a client. */ export default ({ getClient, configuredCatalogues }: SpinupEnv) => { - test('1.responds to a ping over the MCP transport', async () => { - await assert.doesNotReject(getClient().ping()); + // `ping` was removed by protocol revision 2026-07-28, and the SDK refuses it locally before it + // reaches the wire. `server/discover` replaces it as the request every modern client makes at + // connect, so it is the reachability probe now. + test('1.responds to server/discover over the MCP transport', async () => { + await assert.doesNotReject(getClient().discover()); }); test('2.reports server name and version after initialization', async () => { diff --git a/integration-tests/mcp-server/test/startMcpServer.ts b/integration-tests/mcp-server/test/startMcpServer.ts index a15ebe235..501b77c73 100644 --- a/integration-tests/mcp-server/test/startMcpServer.ts +++ b/integration-tests/mcp-server/test/startMcpServer.ts @@ -2,7 +2,8 @@ import { type Server } from 'http'; import { createArrangerClient } from '../../../apps/mcp-server/src/arranger/client.js'; import { validateArrangerConnection } from '../../../apps/mcp-server/src/arranger/validation.js'; -import { createHttpApp } from '../../../apps/mcp-server/src/http/app.js'; +import { startMcpHttpServer } from '../../../apps/mcp-server/src/http/server.js'; +import { createConfirmationCodec } from '../../../apps/mcp-server/src/mcp/requestState.js'; import { createMcpServer } from '../../../apps/mcp-server/src/server.js'; import type { ArrangerMcpConfig } from '../../../apps/mcp-server/src/utils/config.js'; @@ -28,28 +29,20 @@ export const startMcpServerForTest = async (config: ArrangerMcpConfig): Promise< await validateArrangerConnection(config, introspectionClient); - const { app, closeAllSessions } = createHttpApp(config, () => - createMcpServer({ config, client: introspectionClient }), + // Built here rather than in the factory, as `startServer` does: the factory runs per request and + // a query confirmation spans two of them, so a codec built there would verify round two under a + // different key than it minted round one with. + const requestStateCodec = createConfirmationCodec(config); + const { httpServer, close } = await startMcpHttpServer(config, () => + createMcpServer({ config, client: introspectionClient, requestStateCodec }), ); const { host, port, path } = config.mcp; - const httpServer = await new Promise((resolve, reject) => { - const server = app.listen(port, host, () => resolve(server)); - server.once('error', reject); - }); - - const shutdown = async () => { - await closeAllSessions(); - await new Promise((resolve, reject) => { - httpServer.close((err) => (err ? reject(err) : resolve())); - }); - }; - return { config, httpServer, url: `http://${host === '0.0.0.0' ? '127.0.0.1' : host}:${port}${path}`, - shutdown, + shutdown: close, }; }; diff --git a/package-lock.json b/package-lock.json index 4ff793af5..4118099e4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -48,7 +48,8 @@ "name": "@overture-stack/arranger-mcp-server", "version": "0.0.0-dev", "dependencies": { - "@modelcontextprotocol/sdk": "^1.29.0", + "@modelcontextprotocol/node": "^2.0.0", + "@modelcontextprotocol/server": "^2.0.0", "@overture-stack/arranger-types": "file:../../modules/types", "@overture-stack/sqon": "file:../../modules/sqon", "dotenv": "^16.6.1", @@ -58,8 +59,8 @@ "zod": "^4.2.0" }, "devDependencies": { + "@modelcontextprotocol/client": "^2.0.0", "@tsconfig/node22": "^22.0.5", - "@types/express": "^4.17.14", "@types/node": "^25.6.2", "typescript": "^5.8.3" } @@ -147,7 +148,7 @@ "name": "integration-tests-mcp-server", "dependencies": { "@elastic/elasticsearch": "^7.17.14", - "@modelcontextprotocol/sdk": "^1.29.0", + "@modelcontextprotocol/client": "^2.0.0", "@overture-stack/arranger-graphql-router": "file:../../modules/graphql-router", "@overture-stack/arranger-types": "file:../../modules/types", "dotenv": "^16.6.1" @@ -5446,106 +5447,25 @@ "url": "https://github.com/sponsors/epoberezkin" } }, - "node_modules/@modelcontextprotocol/sdk": { - "version": "1.29.0", - "resolved": "https://registry.npmjs.org/@modelcontextprotocol/sdk/-/sdk-1.29.0.tgz", - "integrity": "sha512-zo37mZA9hJWpULgkRpowewez1y6ML5GsXJPY8FI0tBBCd77HEvza4jDqRKOXgHNn867PVGCyTdzqpz0izu5ZjQ==", + "node_modules/@modelcontextprotocol/client": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/@modelcontextprotocol/client/-/client-2.0.0.tgz", + "integrity": "sha512-8f1OghQ2rjzIOfqgUCP+8GiUWqRs89njoWLNqAe8kWmDePv3s1fZXseej+QXemssEuuOvLLmLO/kqM3IQHtISw==", "license": "MIT", "dependencies": { - "@hono/node-server": "^1.19.9", - "ajv": "^8.17.1", - "ajv-formats": "^3.0.1", - "content-type": "^1.0.5", - "cors": "^2.8.5", + "@modelcontextprotocol/core": "2.0.0", "cross-spawn": "^7.0.5", "eventsource": "^3.0.2", "eventsource-parser": "^3.0.0", - "express": "^5.2.1", - "express-rate-limit": "^8.2.1", - "hono": "^4.11.4", "jose": "^6.1.3", - "json-schema-typed": "^8.0.2", "pkce-challenge": "^5.0.0", - "raw-body": "^3.0.0", - "zod": "^3.25 || ^4.0", - "zod-to-json-schema": "^3.25.1" - }, - "engines": { - "node": ">=18" - }, - "peerDependencies": { - "@cfworker/json-schema": "^4.1.1", - "zod": "^3.25 || ^4.0" - }, - "peerDependenciesMeta": { - "@cfworker/json-schema": { - "optional": true - }, - "zod": { - "optional": false - } - } - }, - "node_modules/@modelcontextprotocol/sdk/node_modules/accepts": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/accepts/-/accepts-2.0.0.tgz", - "integrity": "sha512-5cvg6CtKwfgdmVqY1WIiXKc3Q1bkRqGLi+2W/6ao+6Y7gu/RCwRuAhGEzh5B4KlszSuTLgZYuqFqo5bImjNKng==", - "license": "MIT", - "dependencies": { - "mime-types": "^3.0.0", - "negotiator": "^1.0.0" - }, - "engines": { - "node": ">= 0.6" - } - }, - "node_modules/@modelcontextprotocol/sdk/node_modules/body-parser": { - "version": "2.2.2", - "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-2.2.2.tgz", - "integrity": "sha512-oP5VkATKlNwcgvxi0vM0p/D3n2C3EReYVX+DNYs5TjZFn/oQt2j+4sVJtSMr18pdRr8wjTcBl6LoV+FUwzPmNA==", - "license": "MIT", - "dependencies": { - "bytes": "^3.1.2", - "content-type": "^1.0.5", - "debug": "^4.4.3", - "http-errors": "^2.0.0", - "iconv-lite": "^0.7.0", - "on-finished": "^2.4.1", - "qs": "^6.14.1", - "raw-body": "^3.0.1", - "type-is": "^2.0.1" - }, - "engines": { - "node": ">=18" - }, - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/express" - } - }, - "node_modules/@modelcontextprotocol/sdk/node_modules/content-disposition": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/content-disposition/-/content-disposition-1.1.0.tgz", - "integrity": "sha512-5jRCH9Z/+DRP7rkvY83B+yGIGX96OYdJmzngqnw2SBSxqCFPd0w2km3s5iawpGX8krnwSGmF0FW5Nhr0Hfai3g==", - "license": "MIT", - "engines": { - "node": ">=18" + "zod": "^4.2.0" }, - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/express" - } - }, - "node_modules/@modelcontextprotocol/sdk/node_modules/cookie-signature": { - "version": "1.2.2", - "resolved": "https://registry.npmjs.org/cookie-signature/-/cookie-signature-1.2.2.tgz", - "integrity": "sha512-D76uU73ulSXrD1UXF4KE2TMxVVwhsnCgfAyTg9k8P6KGZjlXKrOLe4dJQKI3Bxi5wjesZoFXJWElNWBjPZMbhg==", - "license": "MIT", "engines": { - "node": ">=6.6.0" + "node": ">=20" } }, - "node_modules/@modelcontextprotocol/sdk/node_modules/eventsource": { + "node_modules/@modelcontextprotocol/client/node_modules/eventsource": { "version": "3.0.7", "resolved": "https://registry.npmjs.org/eventsource/-/eventsource-3.0.7.tgz", "integrity": "sha512-CRT1WTyuQoD771GW56XEZFQ/ZoSfWid1alKGDYMmkt2yl8UXrVR4pspqWNEcqKvVIzg6PAltWjxcSSPrboA4iA==", @@ -5557,239 +5477,50 @@ "node": ">=18.0.0" } }, - "node_modules/@modelcontextprotocol/sdk/node_modules/express": { - "version": "5.2.1", - "resolved": "https://registry.npmjs.org/express/-/express-5.2.1.tgz", - "integrity": "sha512-hIS4idWWai69NezIdRt2xFVofaF4j+6INOpJlVOLDO8zXGpUVEVzIYk12UUi2JzjEzWL3IOAxcTubgz9Po0yXw==", - "license": "MIT", - "dependencies": { - "accepts": "^2.0.0", - "body-parser": "^2.2.1", - "content-disposition": "^1.0.0", - "content-type": "^1.0.5", - "cookie": "^0.7.1", - "cookie-signature": "^1.2.1", - "debug": "^4.4.0", - "depd": "^2.0.0", - "encodeurl": "^2.0.0", - "escape-html": "^1.0.3", - "etag": "^1.8.1", - "finalhandler": "^2.1.0", - "fresh": "^2.0.0", - "http-errors": "^2.0.0", - "merge-descriptors": "^2.0.0", - "mime-types": "^3.0.0", - "on-finished": "^2.4.1", - "once": "^1.4.0", - "parseurl": "^1.3.3", - "proxy-addr": "^2.0.7", - "qs": "^6.14.0", - "range-parser": "^1.2.1", - "router": "^2.2.0", - "send": "^1.1.0", - "serve-static": "^2.2.0", - "statuses": "^2.0.1", - "type-is": "^2.0.1", - "vary": "^1.1.2" - }, - "engines": { - "node": ">= 18" - }, - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/express" - } - }, - "node_modules/@modelcontextprotocol/sdk/node_modules/finalhandler": { - "version": "2.1.1", - "resolved": "https://registry.npmjs.org/finalhandler/-/finalhandler-2.1.1.tgz", - "integrity": "sha512-S8KoZgRZN+a5rNwqTxlZZePjT/4cnm0ROV70LedRHZ0p8u9fRID0hJUZQpkKLzro8LfmC8sx23bY6tVNxv8pQA==", - "license": "MIT", - "dependencies": { - "debug": "^4.4.0", - "encodeurl": "^2.0.0", - "escape-html": "^1.0.3", - "on-finished": "^2.4.1", - "parseurl": "^1.3.3", - "statuses": "^2.0.1" - }, - "engines": { - "node": ">= 18.0.0" - }, - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/express" - } - }, - "node_modules/@modelcontextprotocol/sdk/node_modules/fresh": { + "node_modules/@modelcontextprotocol/core": { "version": "2.0.0", - "resolved": "https://registry.npmjs.org/fresh/-/fresh-2.0.0.tgz", - "integrity": "sha512-Rx/WycZ60HOaqLKAi6cHRKKI7zxWbJ31MhntmtwMoaTeF7XFH9hhBp8vITaMidfljRQ6eYWCKkaTK+ykVJHP2A==", - "license": "MIT", - "engines": { - "node": ">= 0.8" - } - }, - "node_modules/@modelcontextprotocol/sdk/node_modules/iconv-lite": { - "version": "0.7.2", - "resolved": "https://registry.npmjs.org/iconv-lite/-/iconv-lite-0.7.2.tgz", - "integrity": "sha512-im9DjEDQ55s9fL4EYzOAv0yMqmMBSZp6G0VvFyTMPKWxiSBHUj9NW/qqLmXUwXrrM7AvqSlTCfvqRb0cM8yYqw==", + "resolved": "https://registry.npmjs.org/@modelcontextprotocol/core/-/core-2.0.0.tgz", + "integrity": "sha512-pJCEwGG7Lfr/+PQp9ZTwKXNeO5wzbfKL7H3MYpCorM4oFBoQrdjnBgEoqG+RjhsvS1FKrDbKux+M1HhlnGWqcA==", "license": "MIT", "dependencies": { - "safer-buffer": ">= 2.1.2 < 3.0.0" + "zod": "^4.2.0" }, "engines": { - "node": ">=0.10.0" - }, - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/express" - } - }, - "node_modules/@modelcontextprotocol/sdk/node_modules/media-typer": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/media-typer/-/media-typer-1.1.0.tgz", - "integrity": "sha512-aisnrDP4GNe06UcKFnV5bfMNPBUw4jsLGaWwWfnH3v02GnBuXX2MCVn5RbrWo0j3pczUilYblq7fQ7Nw2t5XKw==", - "license": "MIT", - "engines": { - "node": ">= 0.8" + "node": ">=20" } }, - "node_modules/@modelcontextprotocol/sdk/node_modules/merge-descriptors": { + "node_modules/@modelcontextprotocol/node": { "version": "2.0.0", - "resolved": "https://registry.npmjs.org/merge-descriptors/-/merge-descriptors-2.0.0.tgz", - "integrity": "sha512-Snk314V5ayFLhp3fkUREub6WtjBfPdCPY1Ln8/8munuLuiYhsABgBVWsozAG+MWMbVEvcdcpbi9R7ww22l9Q3g==", - "license": "MIT", - "engines": { - "node": ">=18" - }, - "funding": { - "url": "https://github.com/sponsors/sindresorhus" - } - }, - "node_modules/@modelcontextprotocol/sdk/node_modules/mime-db": { - "version": "1.54.0", - "resolved": "https://registry.npmjs.org/mime-db/-/mime-db-1.54.0.tgz", - "integrity": "sha512-aU5EJuIN2WDemCcAp2vFBfp/m4EAhWJnUNSSw0ixs7/kXbd6Pg64EmwJkNdFhB8aWt1sH2CTXrLxo/iAGV3oPQ==", - "license": "MIT", - "engines": { - "node": ">= 0.6" - } - }, - "node_modules/@modelcontextprotocol/sdk/node_modules/mime-types": { - "version": "3.0.2", - "resolved": "https://registry.npmjs.org/mime-types/-/mime-types-3.0.2.tgz", - "integrity": "sha512-Lbgzdk0h4juoQ9fCKXW4by0UJqj+nOOrI9MJ1sSj4nI8aI2eo1qmvQEie4VD1glsS250n15LsWsYtCugiStS5A==", + "resolved": "https://registry.npmjs.org/@modelcontextprotocol/node/-/node-2.0.0.tgz", + "integrity": "sha512-Y4hAC2XdGDUdDOCbLDOCA4+aL3NUldjsOWlDL/YwpAxrPhRm1xHd7lZ+mLacvZ9t3PaH28wgNoaLQGrIk1P2pg==", "license": "MIT", "dependencies": { - "mime-db": "^1.54.0" + "@hono/node-server": "^1.19.9" }, "engines": { - "node": ">=18" - }, - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/express" - } - }, - "node_modules/@modelcontextprotocol/sdk/node_modules/negotiator": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/negotiator/-/negotiator-1.0.0.tgz", - "integrity": "sha512-8Ofs/AUQh8MaEcrlq5xOX0CQ9ypTF5dl78mjlMNfOK08fzpgTHQRQPBxcPlEtIw0yRpws+Zo/3r+5WRby7u3Gg==", - "license": "MIT", - "engines": { - "node": ">= 0.6" - } - }, - "node_modules/@modelcontextprotocol/sdk/node_modules/raw-body": { - "version": "3.0.2", - "resolved": "https://registry.npmjs.org/raw-body/-/raw-body-3.0.2.tgz", - "integrity": "sha512-K5zQjDllxWkf7Z5xJdV0/B0WTNqx6vxG70zJE4N0kBs4LovmEYWJzQGxC9bS9RAKu3bgM40lrd5zoLJ12MQ5BA==", - "license": "MIT", - "dependencies": { - "bytes": "~3.1.2", - "http-errors": "~2.0.1", - "iconv-lite": "~0.7.0", - "unpipe": "~1.0.0" - }, - "engines": { - "node": ">= 0.10" - } - }, - "node_modules/@modelcontextprotocol/sdk/node_modules/send": { - "version": "1.2.1", - "resolved": "https://registry.npmjs.org/send/-/send-1.2.1.tgz", - "integrity": "sha512-1gnZf7DFcoIcajTjTwjwuDjzuz4PPcY2StKPlsGAQ1+YH20IRVrBaXSWmdjowTJ6u8Rc01PoYOGHXfP1mYcZNQ==", - "license": "MIT", - "dependencies": { - "debug": "^4.4.3", - "encodeurl": "^2.0.0", - "escape-html": "^1.0.3", - "etag": "^1.8.1", - "fresh": "^2.0.0", - "http-errors": "^2.0.1", - "mime-types": "^3.0.2", - "ms": "^2.1.3", - "on-finished": "^2.4.1", - "range-parser": "^1.2.1", - "statuses": "^2.0.2" - }, - "engines": { - "node": ">= 18" - }, - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/express" - } - }, - "node_modules/@modelcontextprotocol/sdk/node_modules/serve-static": { - "version": "2.2.1", - "resolved": "https://registry.npmjs.org/serve-static/-/serve-static-2.2.1.tgz", - "integrity": "sha512-xRXBn0pPqQTVQiC8wyQrKs2MOlX24zQ0POGaj0kultvoOCstBQM5yvOhAVSUwOMjQtTvsPWoNCHfPGwaaQJhTw==", - "license": "MIT", - "dependencies": { - "encodeurl": "^2.0.0", - "escape-html": "^1.0.3", - "parseurl": "^1.3.3", - "send": "^1.2.0" + "node": ">=20" }, - "engines": { - "node": ">= 18" + "peerDependencies": { + "@modelcontextprotocol/server": "^2.0.0", + "hono": "^4.11.4" }, - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/express" + "peerDependenciesMeta": { + "hono": { + "optional": true + } } }, - "node_modules/@modelcontextprotocol/sdk/node_modules/type-is": { - "version": "2.1.0", - "resolved": "https://registry.npmjs.org/type-is/-/type-is-2.1.0.tgz", - "integrity": "sha512-faYHw0anBbc/kWF3zFTEnxSFOAGUX9GFbOBthvDdLsIlEoWOFOtS0zgCiQYwIskL9iGXZL3kAXD8OoZ4GmMATA==", + "node_modules/@modelcontextprotocol/server": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/@modelcontextprotocol/server/-/server-2.0.0.tgz", + "integrity": "sha512-YhHWdHfpFMQfd0prsEnxKeS3Qz3ytIGmsS0sth4KDjnacIT7hxk6hXHkJ9KysxlkvTM+WZAtQbbcUhdoP4Hvtw==", "license": "MIT", "dependencies": { - "content-type": "^2.0.0", - "media-typer": "^1.1.0", - "mime-types": "^3.0.0" - }, - "engines": { - "node": ">= 18" + "@modelcontextprotocol/core": "2.0.0", + "zod": "^4.2.0" }, - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/express" - } - }, - "node_modules/@modelcontextprotocol/sdk/node_modules/type-is/node_modules/content-type": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/content-type/-/content-type-2.0.0.tgz", - "integrity": "sha512-j/O/d7GcZCyNl7/hwZAb606rzqkyvaDctLmckbxLzHvFBzTJHuGEdodATcP3yIRoDrLHkIATJuvzbFlp/ki2cQ==", - "license": "MIT", "engines": { - "node": ">=18" - }, - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/express" + "node": ">=20" } }, "node_modules/@napi-rs/wasm-runtime": { @@ -9597,6 +9328,7 @@ "version": "8.20.0", "resolved": "https://registry.npmjs.org/ajv/-/ajv-8.20.0.tgz", "integrity": "sha512-Thbli+OlOj+iMPYFBVBfJ3OmCAnaSyNn4M1vz9T6Gka5Jt9ba/HIR56joy65tY6kx/FCF5VXNB819Y7/GUrBGA==", + "dev": true, "license": "MIT", "dependencies": { "fast-deep-equal": "^3.1.3", @@ -9628,6 +9360,7 @@ "version": "3.0.1", "resolved": "https://registry.npmjs.org/ajv-formats/-/ajv-formats-3.0.1.tgz", "integrity": "sha512-8iUql50EUR+uUcdRQ3HDqa6EVyo3docL8g5WJ3FNcWmu62IbkGUue/pEyLBW8VGKKucTPgqeks4fIU1DA4yowQ==", + "dev": true, "license": "MIT", "dependencies": { "ajv": "^8.0.0" @@ -17624,24 +17357,6 @@ "url": "https://opencollective.com/express" } }, - "node_modules/express-rate-limit": { - "version": "8.5.2", - "resolved": "https://registry.npmjs.org/express-rate-limit/-/express-rate-limit-8.5.2.tgz", - "integrity": "sha512-5Kb34ipNX694DH48vN9irak1Qx30nb0PLYHXfJgw4YEjiC3ZEmZJhwOp+VfiCYwFzvFTdB9QkArYS5kXa2cx2A==", - "license": "MIT", - "dependencies": { - "ip-address": "^10.2.0" - }, - "engines": { - "node": ">= 16" - }, - "funding": { - "url": "https://github.com/sponsors/express-rate-limit" - }, - "peerDependencies": { - "express": ">= 4.11" - } - }, "node_modules/express/node_modules/debug": { "version": "2.6.9", "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.9.tgz", @@ -17803,6 +17518,7 @@ "version": "3.1.3", "resolved": "https://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-3.1.3.tgz", "integrity": "sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q==", + "dev": true, "license": "MIT" }, "node_modules/fast-glob": { @@ -17852,6 +17568,7 @@ "version": "3.1.2", "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.2.tgz", "integrity": "sha512-rVjf7ArG3LTk+FS6Yw81V1DLuZl1bRbNrev6Tmd/9RaroeeRRJhAt7jg/6YFxbvAQXUCavSoZhPPj6oOx+5KjQ==", + "dev": true, "funding": [ { "type": "github", @@ -19439,6 +19156,7 @@ "resolved": "https://registry.npmjs.org/hono/-/hono-4.12.23.tgz", "integrity": "sha512-eIaZ9qDgu7XV0pxOCrg7/WhnQ6Ivm22UcxhXx/A3dcbqbbYgBEkc6e/J/s7j2tS96zoB0S9VBdLwQNCWwUo4LA==", "license": "MIT", + "peer": true, "engines": { "node": ">=16.9.0" } @@ -20137,15 +19855,6 @@ "node": ">=0.10.0" } }, - "node_modules/ip-address": { - "version": "10.2.0", - "resolved": "https://registry.npmjs.org/ip-address/-/ip-address-10.2.0.tgz", - "integrity": "sha512-/+S6j4E9AHvW9SWMSEY9Xfy66O5PWvVEJ08O0y5JGyEKQpojb0K0GKpz/v5HJ/G0vi3D2sjGK78119oXZeE0qA==", - "license": "MIT", - "engines": { - "node": ">= 12" - } - }, "node_modules/ipaddr.js": { "version": "1.9.1", "resolved": "https://registry.npmjs.org/ipaddr.js/-/ipaddr.js-1.9.1.tgz", @@ -20644,12 +20353,6 @@ "node": ">=0.10.0" } }, - "node_modules/is-promise": { - "version": "4.0.0", - "resolved": "https://registry.npmjs.org/is-promise/-/is-promise-4.0.0.tgz", - "integrity": "sha512-hvpoI6korhJMnej285dSg6nu1+e6uxs7zG3BYAm5byqDsgJNWwxzM6z6iZiAgQR4TJ30JmBTOwqZUw3WlyH3AQ==", - "license": "MIT" - }, "node_modules/is-regex": { "version": "1.2.1", "resolved": "https://registry.npmjs.org/is-regex/-/is-regex-1.2.1.tgz", @@ -23009,14 +22712,9 @@ "version": "1.0.0", "resolved": "https://registry.npmjs.org/json-schema-traverse/-/json-schema-traverse-1.0.0.tgz", "integrity": "sha512-NM8/P9n3XjXhIZn1lLhkFaACTOURQXjWhV4BA/RnOv8xvgqtqpAX9IO4mRQxSx1Rlo4tqzeqb0sOlruaOy3dug==", + "dev": true, "license": "MIT" }, - "node_modules/json-schema-typed": { - "version": "8.0.2", - "resolved": "https://registry.npmjs.org/json-schema-typed/-/json-schema-typed-8.0.2.tgz", - "integrity": "sha512-fQhoXdcvc3V28x7C7BMs4P5+kNlgUURe2jmUT1T//oBRMDrqy1QPelJimwZGo7Hg9VPV3EQV5Bnq4hbFy2vetA==", - "license": "BSD-2-Clause" - }, "node_modules/json-stable-stringify-without-jsonify": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/json-stable-stringify-without-jsonify/-/json-stable-stringify-without-jsonify-1.0.1.tgz", @@ -31173,6 +30871,7 @@ "version": "2.0.2", "resolved": "https://registry.npmjs.org/require-from-string/-/require-from-string-2.0.2.tgz", "integrity": "sha512-Xf0nWe6RseziFMu+Ap9biiUbmplq6S9/p+7w7YXP/JBHhrUDDUhwa+vANyubuqfZWTveU//DYVGsDG7RKL/vEw==", + "dev": true, "license": "MIT", "engines": { "node": ">=0.10.0" @@ -31476,32 +31175,6 @@ "dev": true, "license": "MIT" }, - "node_modules/router": { - "version": "2.2.0", - "resolved": "https://registry.npmjs.org/router/-/router-2.2.0.tgz", - "integrity": "sha512-nLTrUKm2UyiL7rlhapu/Zl45FwNgkZGaCpZbIHajDYgwlJCOzLSk+cIPAnsEqV955GjILJnKbdQC1nVPz+gAYQ==", - "license": "MIT", - "dependencies": { - "debug": "^4.4.0", - "depd": "^2.0.0", - "is-promise": "^4.0.0", - "parseurl": "^1.3.3", - "path-to-regexp": "^8.0.0" - }, - "engines": { - "node": ">= 18" - } - }, - "node_modules/router/node_modules/path-to-regexp": { - "version": "8.4.2", - "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-8.4.2.tgz", - "integrity": "sha512-qRcuIdP69NPm4qbACK+aDogI5CBDMi1jKe0ry5rSQJz8JVLsC7jV8XpiJjGRLLol3N+R5ihGYcrPLTno6pAdBA==", - "license": "MIT", - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/express" - } - }, "node_modules/run-async": { "version": "2.4.1", "resolved": "https://registry.npmjs.org/run-async/-/run-async-2.4.1.tgz", @@ -37743,15 +37416,6 @@ "funding": { "url": "https://github.com/sponsors/colinhacks" } - }, - "node_modules/zod-to-json-schema": { - "version": "3.25.2", - "resolved": "https://registry.npmjs.org/zod-to-json-schema/-/zod-to-json-schema-3.25.2.tgz", - "integrity": "sha512-O/PgfnpT1xKSDeQYSCfRI5Gy3hPf91mKVDuYLUHZJMiDFptvP41MSnWofm8dnCm0256ZNfZIM7DSzuSMAFnjHA==", - "license": "ISC", - "peerDependencies": { - "zod": "^3.25.28 || ^4" - } } } }