From d15d817e92ef946dd16035ac8938ba90b84615b6 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Sun, 18 Oct 2026 20:22:31 -0700 Subject: [PATCH 1/3] fix(mcp): leave the live session alone when the new client left during the initialize probe A complete initialize still parses after its client disconnects, so the takeover closed the working transport for a client that would never receive the new session. The takeover now also requires that the request's signal has not aborted. Co-Authored-By: Claude Opus 5.5 --- src/server/http/serveStreamableHttp.test.ts | 24 +++++++++++++++++++++ src/server/http/serveStreamableHttp.ts | 4 +++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/server/http/serveStreamableHttp.test.ts b/src/server/http/serveStreamableHttp.test.ts index accba0c190..9c5b43b22a 100644 --- a/src/server/http/serveStreamableHttp.test.ts +++ b/src/server/http/serveStreamableHttp.test.ts @@ -519,6 +519,30 @@ it("requires exact Accept media ranges before a sessionless POST may replace the await client.close(); }); +it("keeps the live session when the new client left before its initialize was probed", async () => { + const slot = new McpSessionSlot( + createEchoServer(() => undefined), + undefined, + ); + const close = vi.spyOn( + WebStandardStreamableHTTPServerTransport.prototype, + "close", + ); + try { + expect((await slot.handle(initializeRequest())).status).toBe(200); + const left = new AbortController(); + left.abort(); + const abandoned = new Request(initializeRequest(), { signal: left.signal }); + expect((await slot.handle(abandoned)).status).toBe(400); + expect(close).not.toHaveBeenCalled(); + expect((await slot.handle(initializeRequest())).status).toBe(200); + expect(close).toHaveBeenCalledTimes(1); + } finally { + close.mockRestore(); + await slot.close(); + } +}); + it("keeps a session whose transport shutdown failed so the next close retries it", async () => { let shutdowns = 0; const slot = new McpSessionSlot( diff --git a/src/server/http/serveStreamableHttp.ts b/src/server/http/serveStreamableHttp.ts index 432c4ea597..b0ef27637f 100644 --- a/src/server/http/serveStreamableHttp.ts +++ b/src/server/http/serveStreamableHttp.ts @@ -232,7 +232,9 @@ export class McpSessionSlot { const open = this.#live; if ( open?.transport.sessionId !== undefined && - (await isInitializePost(request)) + (await isInitializePost(request)) && + // A client that left while its initialize was probed would evict the session for nobody. + !request.signal.aborted ) { if (this.#closed) return SHUTTING_DOWN(); this.logger?.info("MCP HTTP session replaced by a new client"); From cd7f18feb76743203b0bff1a64fe7d8cefbb8f91 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Sun, 18 Oct 2026 20:27:02 -0700 Subject: [PATCH 2/3] test(environment): stop the served environment with SIGINT on Windows Windows has no SIGTERM handler to run: Node's kill("SIGTERM") ends the child outright, so the graceful-shutdown check could never pass there. The test sends SIGINT and expects 130 on Windows, as the streamable HTTP shutdown test does. Co-Authored-By: Claude Opus 5.5 --- .../boundary/process/environmentServeShutdown.test.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/boundary/process/environmentServeShutdown.test.ts b/tests/boundary/process/environmentServeShutdown.test.ts index 039f7dee55..95299fb558 100644 --- a/tests/boundary/process/environmentServeShutdown.test.ts +++ b/tests/boundary/process/environmentServeShutdown.test.ts @@ -10,9 +10,13 @@ import { processTest } from "../../support/process/processFixture.js"; const SHUTDOWN_TIMEOUT_MS = 90_000; +// Windows has no SIGTERM handler to run: Node's kill("SIGTERM") ends the child outright there. +const [SIGNAL, STATUS]: readonly [NodeJS.Signals, number] = + process.platform === "win32" ? ["SIGINT", 130] : ["SIGTERM", 143]; + describe("rea environment-serve shutdown", () => { processTest( - "closes the listener on SIGTERM, prints one stopped document and exits 143", + `closes the listener on ${SIGNAL}, prints one stopped document and exits ${String(STATUS)}`, async ({ processes }) => { const root = await createTestTempDirectory("rea-environment-serve-stop-"); const files = await writeEnvironmentServeFiles(root); @@ -33,9 +37,9 @@ describe("rea environment-serve shutdown", () => { new URL("/.well-known/agent-env.json", started.url.origin), ); expect(card.status).toBe(200); - started.child.kill("SIGTERM"); + started.child.kill(SIGNAL); const [code, signalCode] = await once(started.child, "close"); - expect(code).toBe(143); + expect(code).toBe(STATUS); expect(signalCode).toBeNull(); expect(JSON.parse(stdout)).toMatchObject({ status: "stopped", From 124adf945cb28b53206b653a3346daec85f8457d Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Sun, 18 Oct 2026 20:27:06 -0700 Subject: [PATCH 3/3] fix(environment): keep every tool input addressable, flag unrecorded bodies and bound truncation Path parameters, query parameters and the write body share one argument object, so a query named like a path parameter or 'body' could not be supplied, and query names such as 'constructor' were dropped as already taken. One argument map, used by both the schema and the replay scorer, now renames a colliding query to query_ and a colliding body to request_body, and is built without inherited keys. Replay results gain body_recorded, so a body the capture did not keep is not passed off as a recorded null, and truncation encodes only the 256 KiB prefix instead of the whole body. Co-Authored-By: Claude Opus 5.5 --- docs/agent-environments.md | 10 ++- .../environment-serve/spec.md | 8 ++- .../environment/environmentReplay.test.ts | 50 ++++++++++++++ .../environment/environmentReplay.ts | 29 ++++---- .../environment/environmentTools.test.ts | 24 +++++++ .../environment/environmentTools.ts | 67 +++++++++++++++---- 6 files changed, 158 insertions(+), 30 deletions(-) diff --git a/docs/agent-environments.md b/docs/agent-environments.md index c88300a689..0666ce8cb7 100644 --- a/docs/agent-environments.md +++ b/docs/agent-environments.md @@ -62,6 +62,7 @@ where the answer came from: "exact_match": true, "recorded_path": "/api/users/2", "recorded_query": null, + "body_recorded": true, "truncated": false } } @@ -74,7 +75,9 @@ record and reports `exact_match: false`. `body` is parsed JSON when the recorded content type is JSON-like (`application/json`, `application/*+json`, `text/plain`, `text/markdown`) and the text parses; otherwise it is the recorded text. Bodies over 256 KiB are cut at a UTF-8 boundary and returned as -text with `truncated: true`. Request bodies passed to `POST`, `PUT` and `PATCH` +text with `truncated: true`. `body_recorded` is false when the capture kept the +response's status and content type but no body; `body` is then `null`, which +is not a recorded JSON `null`. Request bodies passed to `POST`, `PUT` and `PATCH` tools are accepted and ignored: nothing is written, and no call changes what a later call returns. Arguments the tool's input schema does not name are refused by the MCP server as invalid arguments, so a misspelled parameter never @@ -163,7 +166,10 @@ connecting to `/mcp`: `tools/list` advertises over MCP. Path parameters are required strings whose description names the segment index, the templating rule and the observed values; query parameter names are optional strings; write methods accept an -optional `body` object described by the recorded top-level key types. Every +optional `body` object described by the recorded top-level key types. Each +input keeps its own argument: a query name equal to a path parameter's name is +advertised as `query_`, and the write body as `request_body` when a query +parameter is named `body`. Every schema closes with `additionalProperties: false`, a key the echo example's schema does not carry. `extensions`, `operations` and `children_environments` are fixed to `null`, diff --git a/docs/specs/agent-environments/environment-serve/spec.md b/docs/specs/agent-environments/environment-serve/spec.md index 2a9d457e56..415e3047e5 100644 --- a/docs/specs/agent-environments/environment-serve/spec.md +++ b/docs/specs/agent-environments/environment-serve/spec.md @@ -27,7 +27,9 @@ later increment. - Build an `McpServer` from the manifest alone: each tool's input schema lists every path parameter as a required string described by its segment index, rule and observed values, every observed query name as an optional string, - and an optional `body` object for `POST`, `PUT` and `PATCH`, closed with + and an optional `body` object for `POST`, `PUT` and `PATCH` (a query name + that collides with a path parameter becomes `query_`, and the body + becomes `request_body` when a query parameter is named `body`), closed with `additionalProperties: false` so unknown arguments are refused; the description reads `METHOD template; replays retained records [ordinals]; evidence ev_…`. The served catalog stays separate from REA's own tool @@ -38,7 +40,9 @@ later increment. `content_type`, `body` (parsed JSON for JSON-like content types that parse, else text; bodies over 256 KiB cut at a UTF-8 boundary with `truncated: true`) and `replay` with `record_ordinal`, `evidence_id`, - `exact_match`, `recorded_path`, `recorded_query` and `truncated`, as both + `exact_match`, `recorded_path`, `recorded_query`, `body_recorded` (false + when the capture kept no response body, so a `null` body is not mistaken for + a recorded one) and `truncated`, as both structured content and text, with REA's existing target-content provenance marker in `_meta`. - Serve the card with exactly the echo example's keys and nesting: `name` diff --git a/src/application/environment/environmentReplay.test.ts b/src/application/environment/environmentReplay.test.ts index ec69803176..6b1ab00bc0 100644 --- a/src/application/environment/environmentReplay.test.ts +++ b/src/application/environment/environmentReplay.test.ts @@ -68,6 +68,7 @@ describe("replayEnvironmentTool selection", () => { exact_match: true, recorded_path: "/api/users/2", recorded_query: null, + body_recorded: true, truncated: false, }, }); @@ -163,7 +164,56 @@ describe("replayEnvironmentTool selection", () => { }); }); +describe("replayEnvironmentTool argument names", () => { + it("matches a query value whose name collides with a path parameter under its renamed argument", () => { + const users = tool("get_users_by_id"); + const exchange = (ordinal: number, query: string): RecordedExchange => ({ + ordinal, + path: "/api/users/2", + query, + status: 200, + contentType: "application/json", + bodyText: JSON.stringify({ query }), + }); + const colliding = { + ...users, + query_parameter_names: ["id"], + record_ordinals: [901, 902], + }; + const from: EnvironmentSource = { + ...source, + exchanges: new Map([ + [901, exchange(901, "id=3")], + [902, exchange(902, "id=4")], + ]), + }; + const result = replayEnvironmentTool(from, colliding, { + id: "2", + query_id: "4", + }); + if (!result.ok) throw result.error; + expect(result.value.replay).toMatchObject({ + record_ordinal: 902, + recorded_query: "id=4", + exact_match: true, + }); + }); +}); + describe("replayEnvironmentTool bodies", () => { + it("reports a response whose body the capture did not retain instead of passing off null as recorded", () => { + const result = replay( + "get_users_by_id", + { id: "2" }, + singleExchangeSource(null, "application/json"), + ); + expect(result.body).toBeNull(); + expect(result.replay).toMatchObject({ + body_recorded: false, + truncated: false, + }); + }); + it("returns text for non-JSON content types and for JSON-like content that does not parse", () => { expect( replay( diff --git a/src/application/environment/environmentReplay.ts b/src/application/environment/environmentReplay.ts index d035faa481..4454722bf1 100644 --- a/src/application/environment/environmentReplay.ts +++ b/src/application/environment/environmentReplay.ts @@ -5,6 +5,7 @@ import { AnalysisInputError } from "../../domain/analysisErrorCore.js"; import { jsonValueSchema, type JsonValue } from "../../domain/jsonValue.js"; import { err, ok, type Result } from "../../domain/result.js"; import { safeParseJson } from "../../domain/safeJson.js"; +import { environmentToolArguments } from "./environmentTools.js"; import { ENVIRONMENT_SERVE_OPERATION, type EnvironmentManifestTool, @@ -29,6 +30,7 @@ export interface ReplayResult { readonly exact_match: boolean; readonly recorded_path: string; readonly recorded_query: string | null; + readonly body_recorded: boolean; readonly truncated: boolean; }; } @@ -50,6 +52,7 @@ const decoded = (segment: string): string => { /** Count how many supplied path and query values the recorded request line satisfies. */ const scoreExchange = ( tool: EnvironmentManifestTool, + queryArguments: ReadonlyMap, exchange: RecordedExchange, args: ReplayArguments, ): MatchScore => { @@ -70,8 +73,8 @@ const scoreExchange = ( const query = new URLSearchParams(exchange.query ?? ""); let queryProvided = 0; let queryMatches = 0; - for (const name of tool.query_parameter_names) { - const value = args[name]; + for (const [name, argument] of queryArguments) { + const value = args[argument]; if (typeof value !== "string") continue; queryProvided += 1; if (query.getAll(name).includes(value)) queryMatches += 1; @@ -92,10 +95,11 @@ export const selectReplayExchange = ( | { readonly exchange: RecordedExchange; readonly exact: boolean } | undefined => { let best: { exchange: RecordedExchange; score: MatchScore } | undefined; + const queryArguments = environmentToolArguments(tool).query; for (const ordinal of tool.record_ordinals) { const exchange = exchanges.get(ordinal); if (exchange === undefined) continue; - const score = scoreExchange(tool, exchange, args); + const score = scoreExchange(tool, queryArguments, exchange, args); if ( best === undefined || score.pathMatches > best.score.pathMatches || @@ -109,12 +113,11 @@ export const selectReplayExchange = ( : { exchange: best.exchange, exact: best.score.exact }; }; -/** Cut text at the last UTF-8 character boundary within the byte limit. */ -const truncateUtf8 = (text: string, maxBytes: number): string => { - const bytes = Buffer.from(text, "utf8"); - let end = maxBytes; - while (end > 0 && (bytes[end]! & 0xc0) === 0x80) end -= 1; - return bytes.toString("utf8", 0, end); +/** The longest prefix of text whose UTF-8 encoding fits the byte limit, encoding only that prefix. */ +const utf8Prefix = (text: string, maxBytes: number): string => { + // encodeInto stops before the first character that does not fit, so `read` ends on a boundary. + const { read } = new TextEncoder().encodeInto(text, new Uint8Array(maxBytes)); + return text.slice(0, read); }; /** Parse a retained body as JSON when its content type is JSON-like; otherwise return the text. */ @@ -123,11 +126,8 @@ const replayBody = ( ): { readonly body: JsonValue; readonly truncated: boolean } => { const text = exchange.bodyText; if (text === null) return { body: null, truncated: false }; - if (Buffer.byteLength(text, "utf8") > REPLAY_BODY_LIMIT_BYTES) - return { - body: truncateUtf8(text, REPLAY_BODY_LIMIT_BYTES), - truncated: true, - }; + const prefix = utf8Prefix(text, REPLAY_BODY_LIMIT_BYTES); + if (prefix.length < text.length) return { body: prefix, truncated: true }; if ( exchange.contentType !== null && isJsonLike(mediaTypeEssence(exchange.contentType)) @@ -170,6 +170,7 @@ export const replayEnvironmentTool = ( exact_match: exact, recorded_path: exchange.path, recorded_query: exchange.query, + body_recorded: exchange.bodyText !== null, truncated, }, }); diff --git a/src/application/environment/environmentTools.test.ts b/src/application/environment/environmentTools.test.ts index df67efa7ad..a048db1ec5 100644 --- a/src/application/environment/environmentTools.test.ts +++ b/src/application/environment/environmentTools.test.ts @@ -92,6 +92,30 @@ describe("environmentToolDefinitions", () => { }); }); +describe("environmentToolInputSchema argument names", () => { + it("keeps every input addressable when query names collide with a path parameter, the body or inherited names", () => { + const schema = environmentToolInputSchema({ + ...tool("post_folders"), + params: tool("get_users_by_id").params, + query_parameter_names: ["id", "body", "constructor"], + }); + expect(Object.keys(schema.properties)).toEqual([ + "id", + "query_id", + "body", + "constructor", + "request_body", + ]); + expect(Object.hasOwn(schema.properties, "constructor")).toBe(true); + expect(schema.properties.query_id?.description).toBe( + "Query parameter id; optional, matched against the recorded query on replay.", + ); + expect(schema.properties.body?.type).toBe("string"); + expect(schema.properties.request_body?.type).toBe("object"); + expect(schema.required).toEqual(["id"]); + }); +}); + describe("environmentCard", () => { it("shapes the card with fixed protocol fields, the tools and null extension slots in contract key order", () => { const card = environmentCard("grafana-example", definitions); diff --git a/src/application/environment/environmentTools.ts b/src/application/environment/environmentTools.ts index e2bf8012fc..18eea3af9c 100644 --- a/src/application/environment/environmentTools.ts +++ b/src/application/environment/environmentTools.ts @@ -40,27 +40,70 @@ const bodyDescription = (tool: EnvironmentManifestTool): string => .map(([key, type]) => `${key} (${type})`) .join(", ")}.`; +/** Where each input of one tool lives in the flat argument object the schema advertises. */ +export interface EnvironmentToolArguments { + /** Query parameter name to argument name. */ + readonly query: ReadonlyMap; + /** Argument name of the write body, for write methods. */ + readonly body: string | undefined; +} + +const freeName = ( + taken: ReadonlySet, + preferred: string, + prefix: string, +): string => { + if (!taken.has(preferred)) return preferred; + let candidate = `${prefix}${preferred}`; + for (let suffix = 2; taken.has(candidate); suffix += 1) + candidate = `${prefix}${preferred}_${String(suffix)}`; + return candidate; +}; + +/** Path parameters keep their names; a query name or the write body that would collide is renamed, so every input stays addressable. */ +export const environmentToolArguments = ( + tool: EnvironmentManifestTool, +): EnvironmentToolArguments => { + const taken = new Set(tool.params.map((param) => param.name)); + const query = new Map(); + for (const name of tool.query_parameter_names) { + const argument = freeName(taken, name, "query_"); + taken.add(argument); + query.set(name, argument); + } + return { + query, + body: WRITE_METHODS.has(tool.method) + ? freeName(taken, "body", "request_") + : undefined, + }; +}; + /** Map one manifest tool to its advertised MCP input schema. */ export const environmentToolInputSchema = ( tool: EnvironmentManifestTool, ): EnvironmentToolInputSchema => { - const properties: Record = {}; + // A Map, not an object literal, so names such as `constructor` are never mistaken for taken. + const properties = new Map(); for (const param of tool.params) - properties[param.name] = { + properties.set(param.name, { type: "string", description: `Path segment ${String(param.index)} (${param.rule}); observed: ${observed(param.observed_values)}`, - }; - for (const name of tool.query_parameter_names) - if (!(name in properties)) - properties[name] = { - type: "string", - description: `Query parameter ${name}; optional, matched against the recorded query on replay.`, - }; - if (WRITE_METHODS.has(tool.method) && !("body" in properties)) - properties.body = { type: "object", description: bodyDescription(tool) }; + }); + const args = environmentToolArguments(tool); + for (const [name, argument] of args.query) + properties.set(argument, { + type: "string", + description: `Query parameter ${name}; optional, matched against the recorded query on replay.`, + }); + if (args.body !== undefined) + properties.set(args.body, { + type: "object", + description: bodyDescription(tool), + }); return { type: "object", - properties, + properties: Object.fromEntries(properties), required: tool.params.map((param) => param.name), additionalProperties: false, };