From e9f1f13fd623adfe93329033015089c731676a68 Mon Sep 17 00:00:00 2001 From: gensokyo-dev Date: Mon, 19 Oct 2026 17:51:03 +0000 Subject: [PATCH 1/8] fix(environment): preserve replay numbers and cancel file loading --- docs/agent-environments.md | 17 +-- .../environment-serve/spec.md | 8 +- .../environment/environmentReplay.test.ts | 49 +++++++++ .../environment/environmentReplay.ts | 17 ++- .../environment/environmentTools.test.ts | 28 +++++ .../environment/environmentTools.ts | 24 ++++- src/cli/environmentCommands.ts | 25 +++-- src/cliJsonInput.ts | 21 +++- src/domain/safeJson.ts | 11 +- .../environment/serveEnvironment.test.ts | 46 ++++++++ tests/boundary/cli/cliStartupImports.test.ts | 14 +++ .../cliJsonInputCancellation.test.ts | 30 ++++++ .../process/environmentServeShutdown.test.ts | 102 ++++++++++++++++++ 13 files changed, 367 insertions(+), 25 deletions(-) create mode 100644 tests/boundary/filesystem/cliJsonInputCancellation.test.ts diff --git a/docs/agent-environments.md b/docs/agent-environments.md index 0c7c83197a..21faaf12c3 100644 --- a/docs/agent-environments.md +++ b/docs/agent-environments.md @@ -42,10 +42,11 @@ serve command refuses a manifest whose parent is not the supplied `--evidence` file with an `invalid_request` error naming both identifiers, so a manifest can never be served from records it did not cite. -The MCP catalog and environment card sample path observations, request-body key descriptions and cited-record -lists within a 1 KiB UTF-8 budget per list. Oversized lists report how many -whole values are shown and point to the complete list in the manifest Evidence; -this only limits descriptive context, not arguments, record selection or replay. +The MCP catalog and environment card use a 1 KiB UTF-8 budget for each +captured display sample. Observation, body-key and cited-record lists show +whole values with omission counts; long method, path and query-name displays +show a marked prefix. Complete values remain in manifest Evidence. Argument +names and replay selection keep their original identities. ## What replay means and does not mean @@ -82,8 +83,9 @@ value as written is chosen. When no record matches, the tool still answers from the tool's first cited 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/plain`, `text/markdown`) and the text parses with safe numeric conversion; otherwise +it is the recorded text. Large integer IDs and other numbers that would lose +information stay in the original JSON text. Bodies over 256 KiB are cut at a UTF-8 boundary and returned as 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`. `body_encoding` is `base64` when the capture @@ -126,6 +128,9 @@ listener serves one MCP client at a time with the same session, `Host` header and shutdown contract as [transports](mcp-contracts.md#transports) describes. It runs until SIGINT or SIGTERM, then closes the listener, prints one `{"status":"stopped", …}` document and exits `130` or `143`. +Signals during input loading cancel the readers and stop startup before binding; +that path returns a structured `cancelled` result instead of a stopped-listener +document, with the same signal exit status. Three routes are served: diff --git a/docs/specs/agent-environments/environment-serve/spec.md b/docs/specs/agent-environments/environment-serve/spec.md index bb8beee8c2..f5426900c6 100644 --- a/docs/specs/agent-environments/environment-serve/spec.md +++ b/docs/specs/agent-environments/environment-serve/spec.md @@ -21,6 +21,9 @@ later increment. require `inspect_analysis_view` Evidence of kind `environment-manifest` and complete `inspect_web_network_capture` Evidence; refuse any other pair with an `invalid_request` whose issue path names the offending file. +- Forward caller cancellation to file reads and check startup boundaries before + authentication/listener work; an already observed abort must not bind or + announce a listener. - Refuse a manifest whose `parent_evidence_id` differs from the evidence file's `evidence_id` before binding any port; Evidence authentication covers the capture's records, so no further record-coverage refusal is defined. @@ -39,12 +42,15 @@ later increment. - Bound each path-observation, body-key and cited-ordinal description sample to 1 KiB of UTF-8 whole values before joining them, with a separate omitted-count and manifest-Evidence notice. Keep complete arrays and replay semantics unchanged. +- Bound captured method, path and query-name prose to UTF-8-safe display prefixes + under the same budget, with an explicit full-value Evidence notice. Preserve + complete lookup values and argument names. - Replay by selecting, among the tool's cited records, the one whose path segments at the parameter indexes match the most supplied values, then the most of those segments equal to the value as written rather than only after percent-decoding, then the most supplied query values, keeping manifest order on ties; return `status`, - `content_type`, `body` (parsed JSON for JSON-like content types that parse, + `content_type`, `body` (parsed JSON for JSON-like content types with safe numeric conversion, 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`, `body_recorded` (false diff --git a/src/application/environment/environmentReplay.test.ts b/src/application/environment/environmentReplay.test.ts index 72720f072a..5e46f11d8a 100644 --- a/src/application/environment/environmentReplay.test.ts +++ b/src/application/environment/environmentReplay.test.ts @@ -98,6 +98,55 @@ const base64Source = (bytes: Buffer): EnvironmentSource => { }); }; +describe("replay JSON value fidelity", () => { + it.each([ + '{"id":9007199254740993}', + '[{"id":18446744073709551615}]', + '{"value":0.100000000000000005}', + '{"value":1e-1000}', + '{"value":-0}', + ])( + "retains numeric JSON as text when conversion loses its value: %s", + (text) => { + expect( + replay( + "get_users_by_id", + {}, + singleExchangeSource(text, "application/json"), + ), + ).toMatchObject({ + body: text, + replay: { + truncated: false, + body_recorded: true, + body_encoding: "utf-8", + }, + }); + }, + ); + + it("still parses equivalent ordinary numeric spellings", () => { + const text = '{"integer":1.0,"exponent":1e3,"fraction":0.1}'; + expect( + replay( + "get_users_by_id", + {}, + singleExchangeSource(text, "application/json"), + ).body, + ).toEqual({ integer: 1, exponent: 1000, fraction: 0.1 }); + }); + + it("retains own prototype-named JSON properties", () => { + const text = '{"__proto__":{"retained":true},"constructor":1}'; + const result = replay( + "get_users_by_id", + {}, + singleExchangeSource(text, "application/json"), + ); + expect(JSON.stringify(result.body)).toBe(text); + }); +}); + describe("replayEnvironmentTool selection", () => { it("replays the record whose path segments equal the supplied parameters as an exact match", () => { const result = replay("get_users_by_id", { id: "2" }); diff --git a/src/application/environment/environmentReplay.ts b/src/application/environment/environmentReplay.ts index 26c99431cd..ea5143c3aa 100644 --- a/src/application/environment/environmentReplay.ts +++ b/src/application/environment/environmentReplay.ts @@ -1,3 +1,5 @@ +import { isSafeNumber } from "lossless-json"; + import { isJsonLike } from "../../domain/analysisView/EnvironmentManifestAccumulator.js"; import { mediaTypeEssence } from "../../domain/analysisView/webNetworkRouteTemplates.js"; import type { AnalysisError } from "../../domain/analysisErrorBase.js"; @@ -160,7 +162,20 @@ const replayBody = ( exchange.contentType !== null && isJsonLike(mediaTypeEssence(exchange.contentType)) ) { - const parsed = safeParseJson(text); + const parsed = safeParseJson(text, (_key, value, context) => { + // The native parser supplies each primitive's original spelling. Share + // HAR's exact-number policy rather than accepting an already-rounded ID. + if ( + typeof value === "number" && + (context?.source === undefined || + !isSafeNumber(context.source) || + Object.is(value, -0)) + ) + throw new RangeError( + "JSON numeric conversion would lose recorded data", + ); + return value; + }); const value = parsed.ok ? jsonValueSchema.safeParse(parsed.value) : undefined; diff --git a/src/application/environment/environmentTools.test.ts b/src/application/environment/environmentTools.test.ts index 1ef60f35f7..416e9f39ea 100644 --- a/src/application/environment/environmentTools.test.ts +++ b/src/application/environment/environmentTools.test.ts @@ -94,6 +94,34 @@ describe("environmentToolDefinitions", () => { }); describe("environment catalog description budgets", () => { + it("bounds captured scalar prose while preserving complete lookup identities", () => { + const query = "query-".repeat(1000); + const expanded = { + ...tool("get_users_by_id"), + method: "METHOD".repeat(1000), + path_template: `/${"😀".repeat(1000)}`, + query_parameter_names: [query], + }; + const [definition] = environmentToolDefinitions( + { ...source.manifest, tools: [expanded] }, + source.evidenceId, + ); + expect(definition?.tool).toBe(expanded); + expect(Buffer.byteLength(definition?.description ?? "")).toBeLessThan(3500); + expect(definition?.description).toContain( + "path display truncated; full value in manifest Evidence", + ); + expect(definition?.description).not.toContain("\uFFFD"); + expect(Object.hasOwn(definition?.inputSchema.properties ?? {}, query)).toBe( + true, + ); + expect( + Buffer.byteLength( + definition?.inputSchema.properties[query]?.description ?? "", + ), + ).toBeLessThan(1400); + }); + it("bounds observation and record samples without changing the retained manifest", () => { const users = tool("get_users_by_id"); const observations = Array.from( diff --git a/src/application/environment/environmentTools.ts b/src/application/environment/environmentTools.ts index 58cc6e6981..6d5f83208c 100644 --- a/src/application/environment/environmentTools.ts +++ b/src/application/environment/environmentTools.ts @@ -32,8 +32,22 @@ export interface EnvironmentToolDefinition { readonly tool: EnvironmentManifestTool; } -/** Agent-context budget for each list sample, excluding its omission/provenance notice. */ -const DESCRIPTION_LIST_SAMPLE_BYTES = 1024; +/** Agent-context budget for each captured display sample, excluding its provenance notice. */ +const DESCRIPTION_SAMPLE_BYTES = 1024; + +/** A display prefix is explicitly separate from the complete identity held by the manifest. */ +const describeText = (text: string, label: string): string => { + if ( + text.length <= DESCRIPTION_SAMPLE_BYTES && + Buffer.byteLength(text, "utf8") <= DESCRIPTION_SAMPLE_BYTES + ) + return text; + const { read } = new TextEncoder().encodeInto( + text, + new Uint8Array(DESCRIPTION_SAMPLE_BYTES), + ); + return `${text.slice(0, read)}… (${label} display truncated; full value in manifest Evidence)`; +}; /** Sample whole values before joining; keep full lists in the manifest that owns them. */ const describeList = ( @@ -46,7 +60,7 @@ const describeList = ( for (const value of values) { const text = display(value); const remaining = - DESCRIPTION_LIST_SAMPLE_BYTES - bytes - (sample.length === 0 ? 0 : 2); + DESCRIPTION_SAMPLE_BYTES - bytes - (sample.length === 0 ? 0 : 2); // UTF-8 takes at least as many bytes as JS code units. Reject long values // without encoding or scanning their entire contents just to size a sample. if (text.length > remaining) break; @@ -123,7 +137,7 @@ export const environmentToolInputSchema = ( for (const [name, argument] of args.query) properties.set(argument, { type: "string", - description: `Query parameter ${name}; optional, matched against the recorded query on replay.`, + description: `Query parameter ${describeText(name, "query name")}; optional, matched against the recorded query on replay.`, }); if (args.body !== undefined) properties.set(args.body, { @@ -145,7 +159,7 @@ export const environmentToolDefinitions = ( ): readonly EnvironmentToolDefinition[] => manifest.tools.map((tool) => ({ name: tool.name, - description: `${REPLAY_CATALOG_INSTRUCTIONS} ${tool.method} ${tool.path_template}; replays retained records [${describeList(tool.record_ordinals)}]; evidence ${evidenceId}`, + description: `${REPLAY_CATALOG_INSTRUCTIONS} ${describeText(tool.method, "method")} ${describeText(tool.path_template, "path")}; replays retained records [${describeList(tool.record_ordinals)}]; evidence ${evidenceId}`, inputSchema: environmentToolInputSchema(tool), tool, })); diff --git a/src/cli/environmentCommands.ts b/src/cli/environmentCommands.ts index 034d1cfbd1..e0a9819c22 100644 --- a/src/cli/environmentCommands.ts +++ b/src/cli/environmentCommands.ts @@ -1,10 +1,6 @@ import { z } from "incur"; import type { Logger } from "pino"; -import { - ENVIRONMENT_SERVE_OPERATION, - loadEnvironmentSource, -} from "../application/environment/environmentSource.js"; import { CLI_COMMANDS } from "../cliCommandNames.js"; import { parseCliJsonInput } from "../cliJsonInput.js"; import { logCliCommand } from "../cliLogging.js"; @@ -14,7 +10,10 @@ import { type McpHttpEndpoint, } from "../config/mcpHttpEndpoint.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; -import { AnalysisResourceConstraintError } from "../domain/analysisErrorCore.js"; +import { + AnalysisCancelledError, + AnalysisResourceConstraintError, +} from "../domain/analysisErrorCore.js"; import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import type { JsonValue } from "../domain/jsonValue.js"; import { createLogger } from "../logger.js"; @@ -52,7 +51,7 @@ const transportError = ( cause: Error, ): AnalysisResourceConstraintError => new AnalysisResourceConstraintError( - ENVIRONMENT_SERVE_OPERATION, + CLI_COMMANDS.environmentServe, "transport", cause.message, { host: endpoint.host, port: endpoint.port }, @@ -78,6 +77,9 @@ const serveEnvironmentCommand = async ( logger: Logger, signal: AbortSignal, ): Promise => { + const cancelled = (): JsonValue => + cliError(new AnalysisCancelledError(CLI_COMMANDS.environmentServe)); + if (signal.aborted) return cancelled(); const endpoint = parseMcpHttpEndpoint(environment, { ...(input.host === undefined ? {} : { host: input.host }), ...(input.port === undefined ? {} : { port: input.port }), @@ -87,21 +89,28 @@ const serveEnvironmentCommand = async ( if (!environmentName.ok) return cliError(environmentName.error); const manifest = await parseCliJsonInput( input.manifest, - ENVIRONMENT_SERVE_OPERATION, + CLI_COMMANDS.environmentServe, + signal, ); if (!manifest.ok) return manifest.error; const evidence = await parseCliJsonInput( input.evidence, - ENVIRONMENT_SERVE_OPERATION, + CLI_COMMANDS.environmentServe, + signal, ); if (!evidence.ok) return evidence.error; + const { loadEnvironmentSource } = + await import("../application/environment/environmentSource.js"); + if (signal.aborted) return cancelled(); const source = loadEnvironmentSource(manifest.value, evidence.value); if (!source.ok) return cliError(source.error); + if (signal.aborted) return cancelled(); // Loaded here so registering the command catalog never loads the HTTP listener. const { serveEnvironment } = await import("../server/environment/serveEnvironment.js"); const { StreamableHttpListenError } = await import("../server/http/streamableHttpListenError.js"); + if (signal.aborted) return cancelled(); // A listener that fails after binding ends the command instead of serving nothing. let failListener: (error: Error) => void = () => undefined; const listenerFailed = new Promise((resolve) => { diff --git a/src/cliJsonInput.ts b/src/cliJsonInput.ts index cf1b6ea727..7583a1e4fd 100644 --- a/src/cliJsonInput.ts +++ b/src/cliJsonInput.ts @@ -4,24 +4,35 @@ import { readJsonInputFile } from "./application/JsonInputFile.js"; import { NonRegularFileReadError } from "./filesystem/RegularFile.js"; import { AnalysisAccessDeniedError, + AnalysisCancelledError, AnalysisInputError, } from "./domain/analysisErrorCore.js"; import { projectAnalysisError } from "./domain/analysisErrorProjection.js"; import type { JsonValue } from "./domain/jsonValue.js"; import { safeParseJson } from "./domain/safeJson.js"; -/** Parse inline JSON or one local JSON file for a CLI workflow. */ +/** Parse inline JSON or one local JSON file, honoring optional caller cancellation. */ export const parseCliJsonInput = async ( value: string, operation: string, + signal?: AbortSignal, ): Promise< | { readonly ok: true; readonly value: unknown } | { readonly ok: false; readonly error: JsonValue } > => { + const cancelled = () => ({ + ok: false as const, + error: { + error: "Application workflow failed", + ...projectAnalysisError(new AnalysisCancelledError(operation)), + }, + }); + if (signal?.aborted) return cancelled(); const inline = safeParseJson(value); if (inline.ok) return { ok: true, value: inline.value }; try { - const parsed = await readJsonInputFile(value, operation); + const parsed = await readJsonInputFile(value, operation, signal); + if (parsed.ok && signal?.aborted) return cancelled(); return parsed.ok ? { ok: true, value: parsed.value } : { @@ -37,6 +48,12 @@ export const parseCliJsonInput = async ( }, }; } catch (cause: unknown) { + if ( + signal?.aborted && + (cause === signal.reason || + (cause instanceof Error && cause.name === "AbortError")) + ) + return cancelled(); const systemCode = accessDeniedSystemCode(cause); if (systemCode !== undefined) return jsonAccessDeniedError(value, operation, systemCode, cause); diff --git a/src/domain/safeJson.ts b/src/domain/safeJson.ts index fc141e3748..e7a8c1bf02 100644 --- a/src/domain/safeJson.ts +++ b/src/domain/safeJson.ts @@ -4,9 +4,16 @@ export type SafeJsonParseResult = | { readonly ok: false; readonly error: string; readonly cause: unknown }; /** Parse JSON text, returning the failure reason instead of throwing. */ -export const safeParseJson = (value: string): SafeJsonParseResult => { +export const safeParseJson = ( + value: string, + reviver?: ( + key: string, + value: unknown, + context?: { readonly source?: string }, + ) => unknown, +): SafeJsonParseResult => { try { - return { ok: true, value: JSON.parse(value) as unknown }; + return { ok: true, value: JSON.parse(value, reviver) as unknown }; } catch (cause: unknown) { return { ok: false, diff --git a/src/server/environment/serveEnvironment.test.ts b/src/server/environment/serveEnvironment.test.ts index 9b8675163a..783a56cb61 100644 --- a/src/server/environment/serveEnvironment.test.ts +++ b/src/server/environment/serveEnvironment.test.ts @@ -71,6 +71,52 @@ const start = async (environmentName?: string): Promise => { return entry; }; +describe("replay value fidelity over MCP", () => { + it("preserves unsafe numeric text and prototype-named fields through the SDK", async () => { + const users = source.manifest.tools.find( + (tool) => tool.name === "get_users_by_id", + ); + const first = source.exchanges.get(users?.record_ordinals[0] ?? -1); + const second = source.exchanges.get(users?.record_ordinals[1] ?? -1); + if (first === undefined || second === undefined) + throw new Error("Missing user exchanges"); + const unsafe = '{"id":9007199254740993}'; + const proto = '{"__proto__":{"retained":true},"id":3}'; + const exchanges = new Map(source.exchanges); + exchanges.set(first.ordinal, { ...first, bodyText: unsafe }); + exchanges.set(second.ordinal, { ...second, bodyText: proto }); + const entry = await serveEnvironment({ + source: { ...source, exchanges }, + host: "127.0.0.1", + port: 0, + }); + served.push(entry); + const client = new Client({ name: "exact-replay", version: "1" }); + const transport = new StreamableHTTPClientTransport(new URL(entry.url)); + await client.connect(transport); + try { + const largeId = await client.callTool({ + name: "get_users_by_id", + arguments: { id: "2" }, + }); + expect(largeId.structuredContent).toMatchObject({ + body: unsafe, + replay: { truncated: false }, + }); + const ownKeys = await client.callTool({ + name: "get_users_by_id", + arguments: { id: "3" }, + }); + expect(JSON.stringify(ownKeys.structuredContent)).toContain( + '"__proto__":{"retained":true}', + ); + } finally { + await transport.terminateSession(); + await client.close(); + } + }); +}); + describe("environment routes", () => { it("serves the card byte-identical to the echo card shape with the manifest tools", async () => { const response = await route("GET", ENVIRONMENT_CARD_PATH).handle( diff --git a/tests/boundary/cli/cliStartupImports.test.ts b/tests/boundary/cli/cliStartupImports.test.ts index 9948cee984..50ec459dfe 100644 --- a/tests/boundary/cli/cliStartupImports.test.ts +++ b/tests/boundary/cli/cliStartupImports.test.ts @@ -114,6 +114,10 @@ const DEFERRED_IMPORTS = [ name: "reconstruction-obligation-ledger", path: "/dist/application/ReconstructionObligationLedgerService.js", }, + { + name: "environment-source", + path: "/dist/application/environment/environmentSource.js", + }, { name: "environment-server", path: "/dist/server/environment/serveEnvironment.js", @@ -165,6 +169,13 @@ const evaluate = (source: string): readonly string[] => [ describe("CLI startup imports", () => { it("detects each deferred package when its command code loads", async () => { + await expect( + resolvedPackages( + evaluate( + 'await import("./dist/application/environment/environmentSource.js");', + ), + ), + ).resolves.toContain("environment-source"); const missingBrowser = join( await createTestTempDirectory("rea-startup-browser-"), "missing-browser", @@ -195,6 +206,9 @@ describe("CLI startup imports", () => { await expect( resolvedPackages(["scripts/rea.mjs", "--help"]), ).resolves.toEqual(new Set()); + await expect( + resolvedPackages(evaluate('await import("./dist/cli.js");')), + ).resolves.toEqual(new Set()); }); it("does not load native provider composition for JavaScript application analysis", async () => { diff --git a/tests/boundary/filesystem/cliJsonInputCancellation.test.ts b/tests/boundary/filesystem/cliJsonInputCancellation.test.ts new file mode 100644 index 0000000000..6752f7da27 --- /dev/null +++ b/tests/boundary/filesystem/cliJsonInputCancellation.test.ts @@ -0,0 +1,30 @@ +import { writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { expect, it } from "vitest"; + +import { parseCliJsonInput } from "../../../src/cliJsonInput.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; + +it("reports an already-cancelled inline JSON read as cancellation", async () => { + const controller = new AbortController(); + controller.abort(); + expect( + await parseCliJsonInput('{"value":1}', "test-input", controller.signal), + ).toMatchObject({ + ok: false, + error: { code: "cancelled", details: { operation: "test-input" } }, + }); +}); + +it("cancels a pending regular-file JSON read through the owning reader", async () => { + const root = await createTestTempDirectory("rea-json-input-cancel-"); + const path = join(root, "input.json"); + await writeFile(path, '{"value":1}'); + const controller = new AbortController(); + const result = parseCliJsonInput(path, "test-input", controller.signal); + controller.abort(); + expect(await result).toMatchObject({ + ok: false, + error: { code: "cancelled", details: { operation: "test-input" } }, + }); +}); diff --git a/tests/boundary/process/environmentServeShutdown.test.ts b/tests/boundary/process/environmentServeShutdown.test.ts index 2ac4aa4089..357e5ef4c5 100644 --- a/tests/boundary/process/environmentServeShutdown.test.ts +++ b/tests/boundary/process/environmentServeShutdown.test.ts @@ -1,4 +1,6 @@ import { once } from "node:events"; +import { writeFile } from "node:fs/promises"; +import { join } from "node:path"; import { describe, expect } from "vitest"; @@ -51,3 +53,103 @@ describe("rea environment-serve shutdown", () => { SHUTDOWN_TIMEOUT_MS, ); }); + +describe.skipIf(process.platform === "win32")( + "environment startup cancellation", + () => { + processTest( + "cancels a paused Evidence read without announcing a listener", + async ({ processes }) => { + const root = await createTestTempDirectory( + "rea-environment-start-cancel-", + ); + const files = await writeEnvironmentServeFiles(root); + const preload = join(root, "pause-read.mjs"); + const marker = "REA_EVIDENCE_READ_STARTED"; + await writeFile( + preload, + ` +import fs from "node:fs/promises"; +import { syncBuiltinESMExports } from "node:module"; +const open = fs.open; +let gated = false; +fs.open = async (path, ...args) => { + const handle = await open(path, ...args); + if (path === ${JSON.stringify(files.manifestPath)}) { + const read = handle.read.bind(handle); + handle.read = async (...readArgs) => { + if (!gated) { + gated = true; + const interrupted = new Promise(resolve => process.once("SIGTERM", resolve)); + const keepAlive = setInterval(() => undefined, 1000); + process.stderr.write(${JSON.stringify(marker + "\n")}); + try { await interrupted; } finally { clearInterval(keepAlive); } + } + return read(...readArgs); + }; + } + return handle; +}; +syncBuiltinESMExports(); +`, + ); + const child = processes.spawn( + process.execPath, + [ + "--import", + preload, + "scripts/rea.mjs", + "environment-serve", + files.manifestPath, + "--evidence", + files.capturePath, + "--port", + "0", + "--json", + ], + { + env: { + PATH: process.env.PATH ?? "", + NODE_OPTIONS: "--max-old-space-size=768", + REA_LOG_LEVEL: "info", + }, + }, + ); + const closed = once(child, "close"); + child.stdin.end(); + let stdout = ""; + let stderr = ""; + child.stdout.on("data", (chunk: Buffer) => { + stdout += chunk.toString("utf8"); + }); + await new Promise((resolve, reject) => { + const timeout = setTimeout( + () => reject(new Error(`No Evidence read marker: ${stderr}`)), + 30_000, + ); + child.stderr.on("data", (chunk: Buffer) => { + stderr = (stderr + chunk.toString("utf8")).slice(-64 * 1024); + if (stderr.includes(marker)) { + clearTimeout(timeout); + resolve(); + } + }); + child.once("exit", () => { + clearTimeout(timeout); + reject(new Error(`Exited before Evidence read: ${stderr}`)); + }); + }); + child.kill("SIGTERM"); + const [code, signal] = await closed; + expect(code).toBe(143); + expect(signal).toBeNull(); + expect(JSON.parse(stdout)).toMatchObject({ + code: "cancelled", + details: { operation: "environment-serve" }, + }); + expect(stderr).not.toContain("MCP transport listening"); + }, + SHUTDOWN_TIMEOUT_MS, + ); + }, +); From 0ab4bbde31a98a8ae3bc5362152273c54dd01595 Mon Sep 17 00:00:00 2001 From: gensokyo-dev Date: Mon, 19 Oct 2026 17:56:30 +0000 Subject: [PATCH 2/8] fix(environment): bound argument and replay metadata allocations --- docs/agent-environments.md | 6 +- .../environment-serve/spec.md | 6 +- .../environment/environmentReplay.test.ts | 120 +++++------------- .../environment/environmentReplay.ts | 27 ++-- .../environmentReplayNumbers.test.ts | 65 ++++++++++ .../environment/environmentSource.ts | 25 +--- .../environment/environmentTools.test.ts | 15 +++ .../environment/environmentTools.ts | 65 ++++++---- src/domain/exactJsonNumber.ts | 8 ++ src/domain/recordedTransportUrl.test.ts | 14 ++ src/domain/recordedTransportUrl.ts | 9 +- .../environment/serveEnvironment.test.ts | 18 +++ src/server/environment/serveEnvironment.ts | 9 +- tests/fixtures/environmentServe.ts | 22 ++++ 14 files changed, 260 insertions(+), 149 deletions(-) create mode 100644 src/application/environment/environmentReplayNumbers.test.ts create mode 100644 src/domain/exactJsonNumber.ts diff --git a/docs/agent-environments.md b/docs/agent-environments.md index 08da1cee31..d2a04c7ccf 100644 --- a/docs/agent-environments.md +++ b/docs/agent-environments.md @@ -46,7 +46,7 @@ The MCP catalog and environment card use a 1 KiB UTF-8 budget for each captured display sample. Observation, body-key and cited-record lists show whole values with omission counts; long method, path and query-name displays show a marked prefix. Complete values remain in manifest Evidence. Argument -names longer than 64 characters use collision-safe query aliases; replay maps +names longer than 64 characters use collision-safe path or query aliases; replay maps those aliases to the complete captured names. ## What replay means and does not mean @@ -79,6 +79,7 @@ where the answer came from: `exact_match` is true when every supplied path parameter and query value equals the recorded one; it says nothing about parameters the caller omitted. +Recorded query parsing runs only when supplied query arguments affect selection. A path value matches a recorded segment as written or after percent-decoding; when both kinds of match are available, the record whose segment equals the value as written is chosen. `recorded_query` keeps the `?` of an explicit empty @@ -220,3 +221,6 @@ only the cited records' request lines and response text in memory. No outbound network request is made, no process is launched and nothing is written to disk. A listener that fails after binding is logged and ends the command with a `resource_constraint` error and exit `1` instead of serving nothing. + +Numeric JSON-RPC IDs that cannot be represented and returned exactly are refused +as invalid requests with a null ID; they are never echoed as a different number. diff --git a/docs/specs/agent-environments/environment-serve/spec.md b/docs/specs/agent-environments/environment-serve/spec.md index 07ab4b8a0d..d25cb2047c 100644 --- a/docs/specs/agent-environments/environment-serve/spec.md +++ b/docs/specs/agent-environments/environment-serve/spec.md @@ -45,7 +45,8 @@ later increment. manifest-Evidence notice. Keep complete arrays and replay semantics unchanged. - Bound captured method, path and query-name prose to UTF-8-safe display prefixes under the same budget, with an explicit full-value Evidence notice. Preserve - complete lookup values; long query names use collision-safe aliases. + complete lookup values; long path and query names use collision-safe aliases. +- Parse a recorded query only when supplied query arguments can affect selection. - Replay by selecting, among the tool's cited records, the one whose path segments at the parameter indexes match the most supplied values, then the most of those segments equal to the value as written rather than only after @@ -111,3 +112,6 @@ listener `error` after binding is wired but not provoked by a test. Exporting a manifest and its evidence as an agent-environment bundle with a verifier, and the hosted verification lane, are later independent increments of #4933. This PR must reference rather than close the whole issue. + +Numeric data-plane IDs that lose value through JSON number conversion are invalid +requests with null response IDs, rather than silently changed correlation values. diff --git a/src/application/environment/environmentReplay.test.ts b/src/application/environment/environmentReplay.test.ts index c7b2a34681..96bba6b1d9 100644 --- a/src/application/environment/environmentReplay.test.ts +++ b/src/application/environment/environmentReplay.test.ts @@ -7,6 +7,7 @@ import { import { environmentServeFixture, environmentServeSource, + environmentServeSourceWithBody as singleExchangeSource, } from "../../../tests/fixtures/environmentServe.js"; import { projectAnalysisError } from "../../domain/analysisErrorProjection.js"; import { inspectAnalysisView } from "../analysisView/AnalysisViewService.js"; @@ -38,24 +39,6 @@ const replay = ( return result.value; }; -/** A source whose only exchange carries the given body and content type. */ -const singleExchangeSource = ( - bodyText: string | null, - contentType: string | null, -): EnvironmentSource => { - const users = tool("get_users_by_id"); - const ordinal = users.record_ordinals[0] ?? 0; - const exchange: RecordedExchange = { - ordinal, - path: "/api/users/2", - query: null, - status: 200, - contentType, - bodyText, - }; - return { ...source, exchanges: new Map([[ordinal, exchange]]) }; -}; - /** The fixture source with /api/users/2's response replaced by HAR base64 content the decoder retained. */ const base64Source = (bytes: Buffer): EnvironmentSource => { const capture = environmentManifestCaptureFixture(); @@ -98,56 +81,38 @@ const base64Source = (bytes: Buffer): EnvironmentSource => { }); }; -describe("replay JSON value fidelity", () => { - it.each([ - '{"id":9007199254740993}', - '[{"id":18446744073709551615}]', - '{"value":0.100000000000000005}', - '{"value":1e-1000}', - '{"value":-0}', - ])( - "retains numeric JSON as text when conversion loses its value: %s", - (text) => { - expect( - replay( - "get_users_by_id", - {}, - singleExchangeSource(text, "application/json"), - ), - ).toMatchObject({ - body: text, - replay: { - truncated: false, - body_recorded: true, - body_encoding: "utf-8", - }, - }); - }, - ); - - it("still parses equivalent ordinary numeric spellings", () => { - const text = '{"integer":1.0,"exponent":1e3,"fraction":0.1}'; - expect( - replay( - "get_users_by_id", - {}, - singleExchangeSource(text, "application/json"), - ).body, - ).toEqual({ integer: 1, exponent: 1000, fraction: 0.1 }); - }); - - it("retains own prototype-named JSON properties", () => { - const text = '{"__proto__":{"retained":true},"constructor":1}'; - const result = replay( - "get_users_by_id", - {}, - singleExchangeSource(text, "application/json"), - ); - expect(JSON.stringify(result.body)).toBe(text); - }); -}); - describe("replayEnvironmentTool selection", () => { + it("does not inspect a recorded query when no query argument can affect selection", () => { + const users = tool("get_users_by_id"); + const original = source.exchanges.get(users.record_ordinals[0]!); + if (original === undefined) throw new Error("Missing fixture exchange"); + const exchange = { + ...original, + get query(): string { + throw new Error("Query must not be inspected"); + }, + }; + expect( + selectReplayExchange(users, new Map([[original.ordinal, exchange]]), { + id: "2", + }), + ).toMatchObject({ exact: true }); + }); + + it("matches a bounded path argument alias against the complete retained parameter", () => { + const users = tool("get_users_by_id"); + const long = { + ...users, + params: users.params.map((param) => ({ + ...param, + name: "long".repeat(1000), + })), + }; + expect( + selectReplayExchange(long, source.exchanges, { path_1: "3" }), + ).toMatchObject({ exchange: { path: "/api/users/3" }, exact: true }); + }); + it("replays the record whose path segments equal the supplied parameters as an exact match", () => { const result = replay("get_users_by_id", { id: "2" }); expect(result).toMatchObject({ @@ -351,27 +316,6 @@ describe("replayEnvironmentTool bodies", () => { }); }); - it("keeps a JSON body as its recorded text when parsing would change an integer", () => { - const unsafe = '{"id":9007199254740993,"name":"lamp"}'; - expect( - replay( - "get_users_by_id", - { id: "2" }, - singleExchangeSource(unsafe, "application/json"), - ).body, - ).toBe(unsafe); - expect( - replay( - "get_users_by_id", - { id: "2" }, - singleExchangeSource( - '{"id":9007199254740992,"n":1.5}', - "application/json", - ), - ).body, - ).toEqual({ id: 9007199254740992, n: 1.5 }); - }); - 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 c78bcfc216..fd415985ad 100644 --- a/src/application/environment/environmentReplay.ts +++ b/src/application/environment/environmentReplay.ts @@ -1,10 +1,10 @@ -import { isSafeNumber } from "lossless-json"; - import { isJsonLike } from "../../domain/analysisView/EnvironmentManifestAccumulator.js"; import { mediaTypeEssence } from "../../domain/analysisView/webNetworkRouteTemplates.js"; import type { AnalysisError } from "../../domain/analysisErrorBase.js"; import { AnalysisInputError } from "../../domain/analysisErrorCore.js"; +import { isExactJsonNumber } from "../../domain/exactJsonNumber.js"; import { jsonValueSchema, type JsonValue } from "../../domain/jsonValue.js"; +import { safeParseJson } from "../../domain/safeJson.js"; import { err, ok, type Result } from "../../domain/result.js"; import { environmentToolArguments } from "./environmentTools.js"; import { @@ -58,16 +58,18 @@ const decoded = (segment: string): string => { /** Count how many supplied path and query values the recorded request line satisfies. */ const scoreExchange = ( tool: EnvironmentManifestTool, + pathArguments: ReadonlyMap, queryArguments: ReadonlyMap, exchange: RecordedExchange, args: ReplayArguments, ): MatchScore => { - const segments = exchange.path.split("/").slice(1); + const segments = + tool.params.length === 0 ? [] : exchange.path.split("/").slice(1); let pathProvided = 0; let pathMatches = 0; let verbatimMatches = 0; for (const param of tool.params) { - const value = args[param.name]; + const value = args[pathArguments.get(param.name) ?? param.name]; if (typeof value !== "string") continue; pathProvided += 1; const segment = segments[param.index]; @@ -78,13 +80,14 @@ const scoreExchange = ( ) pathMatches += 1; } - const query = new URLSearchParams(exchange.query ?? ""); + let query: URLSearchParams | undefined; let queryProvided = 0; let queryMatches = 0; for (const [name, argument] of queryArguments) { const value = args[argument]; if (typeof value !== "string") continue; queryProvided += 1; + query ??= new URLSearchParams(exchange.query ?? ""); if (query.getAll(name).includes(value)) queryMatches += 1; } return { @@ -111,11 +114,17 @@ export const selectReplayExchange = ( | { readonly exchange: RecordedExchange; readonly exact: boolean } | undefined => { let best: { exchange: RecordedExchange; score: MatchScore } | undefined; - const queryArguments = environmentToolArguments(tool).query; + const argumentsMap = environmentToolArguments(tool); for (const ordinal of tool.record_ordinals) { const exchange = exchanges.get(ordinal); if (exchange === undefined) continue; - const score = scoreExchange(tool, queryArguments, exchange, args); + const score = scoreExchange( + tool, + argumentsMap.path, + argumentsMap.query, + exchange, + args, + ); if (best === undefined || outranks(score, best.score)) best = { exchange, score }; } @@ -168,9 +177,7 @@ const replayBody = ( // HAR's exact-number policy rather than accepting an already-rounded ID. if ( typeof value === "number" && - (context?.source === undefined || - !isSafeNumber(context.source) || - Object.is(value, -0)) + !isExactJsonNumber(value, context?.source) ) throw new RangeError( "JSON numeric conversion would lose recorded data", diff --git a/src/application/environment/environmentReplayNumbers.test.ts b/src/application/environment/environmentReplayNumbers.test.ts new file mode 100644 index 0000000000..4e21219b4a --- /dev/null +++ b/src/application/environment/environmentReplayNumbers.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, it } from "vitest"; +import { environmentServeSourceWithBody as singleExchangeSource } from "../../../tests/fixtures/environmentServe.js"; +import type { EnvironmentSource } from "./environmentSource.js"; +import { replayEnvironmentTool } from "./environmentReplay.js"; +const replay = ( + name: string, + args: Readonly>, + source: EnvironmentSource, +) => { + const tool = source.manifest.tools.find((tool) => tool.name === name); + if (tool === undefined) throw new Error("Missing fixture tool"); + const result = replayEnvironmentTool(source, tool, args); + if (!result.ok) throw result.error; + return result.value; +}; +describe("replay JSON value fidelity", () => { + it.each([ + '{"id":9007199254740993}', + '{"id":9007199254740993.0}', + '{"id":9.007199254740993e15}', + '[{"id":18446744073709551615}]', + '{"value":0.100000000000000005}', + '{"value":1e-1000}', + '{"value":-0}', + ])( + "retains numeric JSON as text when conversion loses its value: %s", + (text) => { + expect( + replay( + "get_users_by_id", + {}, + singleExchangeSource(text, "application/json"), + ), + ).toMatchObject({ + body: text, + replay: { + truncated: false, + body_recorded: true, + body_encoding: "utf-8", + }, + }); + }, + ); + + it("still parses equivalent ordinary numeric spellings", () => { + const text = '{"integer":1.0,"exponent":1e3,"fraction":0.1}'; + expect( + replay( + "get_users_by_id", + {}, + singleExchangeSource(text, "application/json"), + ).body, + ).toEqual({ integer: 1, exponent: 1000, fraction: 0.1 }); + }); + + it("retains own prototype-named JSON properties", () => { + const text = '{"__proto__":{"retained":true},"constructor":1}'; + const result = replay( + "get_users_by_id", + {}, + singleExchangeSource(text, "application/json"), + ); + expect(JSON.stringify(result.body)).toBe(text); + }); +}); diff --git a/src/application/environment/environmentSource.ts b/src/application/environment/environmentSource.ts index ac2942121e..1f6260add1 100644 --- a/src/application/environment/environmentSource.ts +++ b/src/application/environment/environmentSource.ts @@ -135,29 +135,6 @@ const recordedBody = ( } }; -/** WHATWG reports an explicit empty query (`/api?`) as "", like no query; the recorded URL keeps the delimiter. */ -const recordedQuery = ( - record: WebNetworkCaptureRecord, - query: string | undefined, -): string | null => { - if (query === undefined) return null; - if (query !== "") return query; - const reported = record.reported; - const request = - typeof reported === "object" && - reported !== null && - !Array.isArray(reported) - ? reported.request - : undefined; - const url = - typeof request === "object" && request !== null && !Array.isArray(request) - ? request.url - : undefined; - return typeof url === "string" && url.split("#", 1)[0]?.includes("?") === true - ? "?" - : null; -}; - const recordedExchange = ( record: WebNetworkCaptureRecord, ): RecordedExchange | undefined => { @@ -166,7 +143,7 @@ const recordedExchange = ( return { ordinal: record.ordinal, path: line.path, - query: recordedQuery(record, line.query), + query: line.query === undefined || line.query === "" ? null : line.query, status: line.status, contentType: line.rawMime === "" ? null : line.rawMime, ...recordedBody(record), diff --git a/src/application/environment/environmentTools.test.ts b/src/application/environment/environmentTools.test.ts index 0385fffa3b..3e32bb1b46 100644 --- a/src/application/environment/environmentTools.test.ts +++ b/src/application/environment/environmentTools.test.ts @@ -177,6 +177,21 @@ describe("environment catalog description budgets", () => { expect(definition?.tool.path_template).toBe(long.path_template); }); + it("advertises long path parameters with collision-safe aliases consistently", () => { + const users = tool("get_users_by_id"); + const expanded = { + ...users, + params: [ + { ...users.params[0]!, name: "long".repeat(1000) }, + { ...users.params[0]!, name: "path_1", index: 3 }, + ], + }; + const schema = environmentToolInputSchema(expanded); + expect(schema.required).toEqual(["path_1", "path_path_1"]); + expect(Object.keys(schema.properties)).toEqual(schema.required); + expect(Buffer.byteLength(JSON.stringify(schema))).toBeLessThan(4096); + }); + it("advertises an oversized query name under a short, collision-safe alias", () => { const longName = "n".repeat(100_000); const search = { diff --git a/src/application/environment/environmentTools.ts b/src/application/environment/environmentTools.ts index 5c6aba155b..ce4ebaad4d 100644 --- a/src/application/environment/environmentTools.ts +++ b/src/application/environment/environmentTools.ts @@ -51,10 +51,11 @@ const describeText = (text: string, label: string): string => { /** Sample whole values before joining; keep full lists in the manifest that owns them. */ const describeList = ( - values: readonly Value[], + values: Iterable, + count: number, display: (value: Value) => string = String, ): string => { - if (values.length === 0) return "none"; + if (count === 0) return "none"; const sample: string[] = []; let bytes = 0; for (const value of values) { @@ -70,24 +71,31 @@ const describeList = ( sample.push(text); } const listed = sample.join(", "); - return sample.length === values.length + return sample.length === count ? listed - : `${listed}${sample.length === 0 ? "" : ", "}… (${String(sample.length)}/${String(values.length)} shown; full list in manifest Evidence)`; + : `${listed}${sample.length === 0 ? "" : ", "}… (${String(sample.length)}/${String(count)} shown; full list in manifest Evidence)`; }; -/** Captured query names longer than this are advertised under a short alias; the manifest keeps the name. */ -export const MAX_QUERY_ARGUMENT_CHARS = 64; +/** Captured argument names longer than this are advertised under a short alias; the manifest keeps the name. */ +export const MAX_ARGUMENT_NAME_CHARS = 64; -const bodyDescription = (tool: EnvironmentManifestTool): string => - tool.body_shape === null - ? "Request body; the capture retained no JSON body shape for this tool." - : `Request body; recorded top-level keys: ${describeList( - Object.entries(tool.body_shape.properties), - ([key, type]) => `${key} (${type})`, - )}.`; +const bodyDescription = (tool: EnvironmentManifestTool): string => { + const properties = tool.body_shape?.properties; + if (properties === undefined) + return "Request body; the capture retained no JSON body shape for this tool."; + let count = 0; + for (const key in properties) if (Object.hasOwn(properties, key)) count += 1; + function* entries(): IterableIterator { + for (const key in properties) + if (Object.hasOwn(properties, key)) yield `${key} (${properties[key]})`; + } + return `Request body; recorded top-level keys: ${describeList(entries(), count)}.`; +}; /** Where each input of one tool lives in the flat argument object the schema advertises. */ export interface EnvironmentToolArguments { + /** Captured path parameter name to its advertised argument name. */ + readonly path: ReadonlyMap; /** Query parameter name to argument name. */ readonly query: ReadonlyMap; /** Argument name of the write body, for write methods. */ @@ -110,11 +118,21 @@ const freeName = ( export const environmentToolArguments = ( tool: EnvironmentManifestTool, ): EnvironmentToolArguments => { - const taken = new Set(tool.params.map((param) => param.name)); + const taken = new Set(); + const path = new Map(); + for (const [index, param] of tool.params.entries()) { + const preferred = + param.name.length <= MAX_ARGUMENT_NAME_CHARS + ? param.name + : `path_${String(index + 1)}`; + const argument = freeName(taken, preferred, "path_"); + taken.add(argument); + path.set(param.name, argument); + } const query = new Map(); for (const [index, name] of tool.query_parameter_names.entries()) { const preferred = - name.length <= MAX_QUERY_ARGUMENT_CHARS + name.length <= MAX_ARGUMENT_NAME_CHARS ? name : `query_${String(index + 1)}`; const argument = freeName(taken, preferred, "query_"); @@ -122,6 +140,7 @@ export const environmentToolArguments = ( query.set(name, argument); } return { + path, query, body: WRITE_METHODS.has(tool.method) ? freeName(taken, "body", "request_") @@ -135,12 +154,12 @@ export const environmentToolInputSchema = ( ): EnvironmentToolInputSchema => { // 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.set(param.name, { - type: "string", - description: `Path segment ${String(param.index)} (${param.rule}); observed: ${describeList(param.observed_values)}`, - }); const args = environmentToolArguments(tool); + for (const param of tool.params) + properties.set(args.path.get(param.name) ?? param.name, { + type: "string", + description: `Path segment ${String(param.index)} (${param.rule}); observed: ${describeList(param.observed_values, param.observed_values.length)}`, + }); for (const [name, argument] of args.query) properties.set(argument, { type: "string", @@ -154,7 +173,9 @@ export const environmentToolInputSchema = ( return { type: "object", properties: Object.fromEntries(properties), - required: tool.params.map((param) => param.name), + required: tool.params.map( + (param) => args.path.get(param.name) ?? param.name, + ), additionalProperties: false, }; }; @@ -166,7 +187,7 @@ export const environmentToolDefinitions = ( ): readonly EnvironmentToolDefinition[] => manifest.tools.map((tool) => ({ name: tool.name, - description: `${REPLAY_CATALOG_INSTRUCTIONS} ${describeText(tool.method, "method")} ${describeText(tool.path_template, "path")}; replays retained records [${describeList(tool.record_ordinals)}]; evidence ${evidenceId}`, + description: `${REPLAY_CATALOG_INSTRUCTIONS} ${describeText(tool.method, "method")} ${describeText(tool.path_template, "path")}; replays retained records [${describeList(tool.record_ordinals, tool.record_ordinals.length)}]; evidence ${evidenceId}`, inputSchema: environmentToolInputSchema(tool), tool, })); diff --git a/src/domain/exactJsonNumber.ts b/src/domain/exactJsonNumber.ts new file mode 100644 index 0000000000..411d2f8f91 --- /dev/null +++ b/src/domain/exactJsonNumber.ts @@ -0,0 +1,8 @@ +import { isSafeNumber } from "lossless-json"; + +/** Whether native JSON conversion and serialization preserve the recorded number. */ +export const isExactJsonNumber = ( + value: number, + source: string | undefined, +): boolean => + source !== undefined && isSafeNumber(source) && !Object.is(value, -0); diff --git a/src/domain/recordedTransportUrl.test.ts b/src/domain/recordedTransportUrl.test.ts index 9c7b78ea0c..ddaacfce18 100644 --- a/src/domain/recordedTransportUrl.test.ts +++ b/src/domain/recordedTransportUrl.test.ts @@ -9,3 +9,17 @@ it("recognizes an empty userinfo marker at the authority start", () => { credentials: { start: 8, end: 9 }, }); }); + +it.each([ + ["https://host/api?", "?"], + ["https://host?", "?"], + ["https://host/api?#fragment", "?"], + ["https://host/api#?", ""], + ["https://host/api", ""], + ["https://host/api?x=1", "?x=1"], +])("preserves the selected query delimiter in %s", (url, query) => { + expect(parseRecordedTransportUrl(url, { includeQuery: true }).query).toBe( + query, + ); + expect(parseRecordedTransportUrl(url)).not.toHaveProperty("query"); +}); diff --git a/src/domain/recordedTransportUrl.ts b/src/domain/recordedTransportUrl.ts index 429c0f6ca8..8f5fa1e38f 100644 --- a/src/domain/recordedTransportUrl.ts +++ b/src/domain/recordedTransportUrl.ts @@ -2,7 +2,7 @@ export interface RecordedTransportUrl { readonly origin: string | null; readonly path: string | null; - /** WHATWG query spelling, present only when explicitly selected for projection. */ + /** WHATWG query spelling, retaining an explicit empty delimiter; selected only on request. */ readonly query?: string; readonly credentials: | { readonly start: number; readonly end: number } @@ -70,8 +70,13 @@ export const parseRecordedTransportUrl = ( (special && value[authorityEnd] === "\\") ) return { origin, path: null, credentials }; + const hash = value.indexOf("#", authorityEnd); + const question = value.indexOf("?", authorityEnd); + const hasQuery = question >= 0 && (hash < 0 || question < hash); const query = - options.includeQuery === true ? { query: absolute!.search } : {}; + options.includeQuery === true + ? { query: absolute!.search || (hasQuery ? "?" : "") } + : {}; if (value[authorityEnd] !== "/") return { origin, path: "/", credentials, ...query }; let pathEnd = authorityEnd; diff --git a/src/server/environment/serveEnvironment.test.ts b/src/server/environment/serveEnvironment.test.ts index 783a56cb61..2675762a4e 100644 --- a/src/server/environment/serveEnvironment.test.ts +++ b/src/server/environment/serveEnvironment.test.ts @@ -203,6 +203,24 @@ describe("environment routes", () => { body: JSON.stringify(body), }), ); + for (const id of [ + "9007199254740993", + "9.007199254740993e15", + "0.100000000000000005", + "-0", + ]) { + const refused = await rpc.handle( + new Request(`http://127.0.0.1${ENVIRONMENT_RPC_PATH}`, { + method: "POST", + body: `{"jsonrpc":"2.0","id":${id},"method":"data/reset"}`, + }), + ); + expect(refused.status).toBe(400); + await expect(refused.json()).resolves.toMatchObject({ + id: null, + error: { code: -32600 }, + }); + } // A notification gets no reply; a batch is answered per request, skipping notifications. expect((await post({ jsonrpc: "2.0", method: "data/reset" })).status).toBe( 202, diff --git a/src/server/environment/serveEnvironment.ts b/src/server/environment/serveEnvironment.ts index bbeb41dba7..70e8e997a6 100644 --- a/src/server/environment/serveEnvironment.ts +++ b/src/server/environment/serveEnvironment.ts @@ -20,6 +20,7 @@ import { type EnvironmentToolDefinition, } from "../../application/environment/environmentTools.js"; import { projectAnalysisError } from "../../domain/analysisErrorProjection.js"; +import { isExactJsonNumber } from "../../domain/exactJsonNumber.js"; import { safeParseJson } from "../../domain/safeJson.js"; import { PRODUCT_IDENTITY } from "../../identity.js"; import { @@ -155,7 +156,13 @@ const answerRpc = async (request: Request): Promise => { PAYLOAD_TOO_LARGE, `Request body must not exceed ${String(DEFAULT_MAX_REQUEST_BODY_SIZE)} bytes`, ); - const parsed = safeParseJson(read.text); + const parsed = safeParseJson(read.text, (key, value, context) => + key === "id" && + typeof value === "number" && + !isExactJsonNumber(value, context?.source) + ? Symbol("unrepresentable RPC id") + : value, + ); if (!parsed.ok) return jsonRpcErrorResponse(400, PARSE_ERROR, "parse error"); // A batch is answered message by message; notifications get no reply at all. if (Array.isArray(parsed.value)) { diff --git a/tests/fixtures/environmentServe.ts b/tests/fixtures/environmentServe.ts index 63044df56a..8d95b1ec24 100644 --- a/tests/fixtures/environmentServe.ts +++ b/tests/fixtures/environmentServe.ts @@ -68,3 +68,25 @@ export const writeEnvironmentServeFiles = async ( ]); return { manifestPath, capturePath }; }; + +/** A source whose only exchange carries the given body and content type. */ +export const environmentServeSourceWithBody = ( + bodyText: string | null, + contentType: string | null, +): EnvironmentSource => { + const source = environmentServeSource(); + const users = source.manifest.tools.find( + (tool) => tool.name === "get_users_by_id", + ); + if (users === undefined) throw new Error("Missing users fixture"); + const ordinal = users.record_ordinals[0] ?? 0; + const exchange = { + ordinal, + path: "/api/users/2", + query: null, + status: 200, + contentType, + bodyText, + }; + return { ...source, exchanges: new Map([[ordinal, exchange]]) }; +}; From fae1d2f5cae34ae14a4465f30e977762ea9cfcce Mon Sep 17 00:00:00 2001 From: gensokyo-dev Date: Mon, 19 Oct 2026 18:01:33 +0000 Subject: [PATCH 3/8] test(environment): keep replay test groups within lint bounds --- .../environment/environmentReplay.test.ts | 4 +- .../environment/serveEnvironment.test.ts | 40 ++++++++++--------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/application/environment/environmentReplay.test.ts b/src/application/environment/environmentReplay.test.ts index 96bba6b1d9..43b7851af0 100644 --- a/src/application/environment/environmentReplay.test.ts +++ b/src/application/environment/environmentReplay.test.ts @@ -81,7 +81,7 @@ const base64Source = (bytes: Buffer): EnvironmentSource => { }); }; -describe("replayEnvironmentTool selection", () => { +describe("replay argument mapping and query admission", () => { it("does not inspect a recorded query when no query argument can affect selection", () => { const users = tool("get_users_by_id"); const original = source.exchanges.get(users.record_ordinals[0]!); @@ -112,7 +112,9 @@ describe("replayEnvironmentTool selection", () => { selectReplayExchange(long, source.exchanges, { path_1: "3" }), ).toMatchObject({ exchange: { path: "/api/users/3" }, exact: true }); }); +}); +describe("replayEnvironmentTool selection", () => { it("replays the record whose path segments equal the supplied parameters as an exact match", () => { const result = replay("get_users_by_id", { id: "2" }); expect(result).toMatchObject({ diff --git a/src/server/environment/serveEnvironment.test.ts b/src/server/environment/serveEnvironment.test.ts index 2675762a4e..df6b3a4e4f 100644 --- a/src/server/environment/serveEnvironment.test.ts +++ b/src/server/environment/serveEnvironment.test.ts @@ -203,24 +203,6 @@ describe("environment routes", () => { body: JSON.stringify(body), }), ); - for (const id of [ - "9007199254740993", - "9.007199254740993e15", - "0.100000000000000005", - "-0", - ]) { - const refused = await rpc.handle( - new Request(`http://127.0.0.1${ENVIRONMENT_RPC_PATH}`, { - method: "POST", - body: `{"jsonrpc":"2.0","id":${id},"method":"data/reset"}`, - }), - ); - expect(refused.status).toBe(400); - await expect(refused.json()).resolves.toMatchObject({ - id: null, - error: { code: -32600 }, - }); - } // A notification gets no reply; a batch is answered per request, skipping notifications. expect((await post({ jsonrpc: "2.0", method: "data/reset" })).status).toBe( 202, @@ -276,6 +258,28 @@ describe("environment routes", () => { }); }); +it.each([ + "9007199254740993", + "9.007199254740993e15", + "0.100000000000000005", + "-0", +])( + "refuses a numeric RPC id outside the exact conversion policy: %s", + async (id) => { + const refused = await route("POST", ENVIRONMENT_RPC_PATH).handle( + new Request(`http://127.0.0.1${ENVIRONMENT_RPC_PATH}`, { + method: "POST", + body: `{"jsonrpc":"2.0","id":${id},"method":"data/reset"}`, + }), + ); + expect(refused.status).toBe(400); + await expect(refused.json()).resolves.toMatchObject({ + id: null, + error: { code: -32600 }, + }); + }, +); + describe("environment data plane validation", () => { it("refuses malformed JSON-RPC members and oversized batches as invalid requests", async () => { const rpc = route("POST", ENVIRONMENT_RPC_PATH); From 8c46c5b08b3801a48d376bb4a5e9093e53c555fa Mon Sep 17 00:00:00 2001 From: gensokyo-dev Date: Mon, 19 Oct 2026 18:04:37 +0000 Subject: [PATCH 4/8] docs(environment): clarify bounded argument and RPC identities --- docs/agent-environments.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/agent-environments.md b/docs/agent-environments.md index d2a04c7ccf..5b61a48b33 100644 --- a/docs/agent-environments.md +++ b/docs/agent-environments.md @@ -46,7 +46,7 @@ The MCP catalog and environment card use a 1 KiB UTF-8 budget for each captured display sample. Observation, body-key and cited-record lists show whole values with omission counts; long method, path and query-name displays show a marked prefix. Complete values remain in manifest Evidence. Argument -names longer than 64 characters use collision-safe path or query aliases; replay maps +names longer than 64 characters use collision-safe `path_` or `query_` aliases; replay maps those aliases to the complete captured names. ## What replay means and does not mean @@ -145,6 +145,10 @@ Three routes are served: | `GET /.well-known/agent-env.json` | The environment card | | `POST /agentenv` | JSON-RPC `-32601 method not found: ` for every method | +Numeric JSON-RPC IDs outside JavaScript's safe range or whose precision would +be lost are refused as invalid requests with a null ID. They are never echoed +as a different number. + The data plane at `/agentenv` is advertised by the card but carries no operations in this release; the error names the requested method so a caller can see that the method, not the transport, is unsupported. Its request body is @@ -221,6 +225,3 @@ only the cited records' request lines and response text in memory. No outbound network request is made, no process is launched and nothing is written to disk. A listener that fails after binding is logged and ends the command with a `resource_constraint` error and exit `1` instead of serving nothing. - -Numeric JSON-RPC IDs that cannot be represented and returned exactly are refused -as invalid requests with a null ID; they are never echoed as a different number. From ba49f8966678592464c3a568a386f55f166cfc10 Mon Sep 17 00:00:00 2001 From: gensokyo-dev Date: Mon, 19 Oct 2026 18:38:55 +0000 Subject: [PATCH 5/8] fix(environment): bound discovery and cancel listener startup --- docs/agent-environments.md | 14 +- .../environment-serve/spec.md | 16 ++ .../environment/environmentReplay.ts | 55 ++++-- .../environmentReplayCapacity.test.ts | 50 +++++ .../environment/environmentTools.test.ts | 18 +- .../environment/environmentTools.ts | 153 ++++++++++++---- src/cli/environmentCommands.ts | 8 +- src/domain/recordedTransportUrl.test.ts | 44 ++++- src/domain/recordedTransportUrl.ts | 75 +++++++- .../environment/environmentCapacity.test.ts | 70 +++++++ .../environment/serveEnvironment.test.ts | 18 +- src/server/environment/serveEnvironment.ts | 16 +- src/server/http/listenHttpServer.ts | 42 +++++ src/server/http/serveStreamableHttp.test.ts | 21 +++ src/server/http/serveStreamableHttp.ts | 32 ++-- .../boundary/cli/environmentServeCli.test.ts | 43 +++++ .../process/environmentServeShutdown.test.ts | 172 ++++++++++-------- 17 files changed, 694 insertions(+), 153 deletions(-) create mode 100644 src/application/environment/environmentReplayCapacity.test.ts create mode 100644 src/server/environment/environmentCapacity.test.ts create mode 100644 src/server/http/listenHttpServer.ts diff --git a/docs/agent-environments.md b/docs/agent-environments.md index 5b61a48b33..19f162194a 100644 --- a/docs/agent-environments.md +++ b/docs/agent-environments.md @@ -49,6 +49,15 @@ show a marked prefix. Complete values remain in manifest Evidence. Argument names longer than 64 characters use collision-safe `path_` or `query_` aliases; replay maps those aliases to the complete captured names. +Discovery is admitted as a complete catalog under a 1 MiB serialization +reservation before the listener binds. Reservation charges up to six bytes per +UTF-16 code unit for JSON escaping/UTF-8 expansion, plus schema overhead; +a catalog may therefore be refused even when its compact JSON is smaller. +This was calibrated with 1500 query fields and eight concurrent SDK discovery +requests (183,798-byte card, 507.27 MiB sampled process-family RSS). Over-capacity +catalogs return `resource_constraint`, retain complete input Evidence, and require +a smaller capture or manifest. Fields and tools are never silently omitted. + ## What replay means and does not mean Each MCP tool call selects one of the records its manifest tool cites and @@ -85,6 +94,8 @@ when both kinds of match are available, the record whose segment equals the value as written is chosen. `recorded_query` keeps the `?` of an explicit empty query, and a recorded path or query longer than 1 KiB is cut in the reply with `recorded_line_truncated: true`; selection always uses the full recorded line. +`content_type` also has a 1 KiB display bound with `content_type_truncated`; body +interpretation uses the full recorded media type. When no record matches, the tool still answers from the tool's first cited record and reports `exact_match: false`. `body` is parsed JSON when the recorded content type is JSON-like (`application/json`, `application/*+json`, @@ -133,7 +144,8 @@ listener serves one MCP client at a time with the same session, `Host` header and shutdown contract as [transports](mcp-contracts.md#transports) describes. It runs until SIGINT or SIGTERM, then closes the listener, prints one `{"status":"stopped", …}` document and exits `130` or `143`. -Signals during input loading cancel the readers and stop startup before binding; +Signals during input loading or DNS/bind cancel the owning reader/listener before +a listening announcement; that path returns a structured `cancelled` result instead of a stopped-listener document, with the same signal exit status. diff --git a/docs/specs/agent-environments/environment-serve/spec.md b/docs/specs/agent-environments/environment-serve/spec.md index d25cb2047c..b0f4ab8fb5 100644 --- a/docs/specs/agent-environments/environment-serve/spec.md +++ b/docs/specs/agent-environments/environment-serve/spec.md @@ -107,6 +107,22 @@ transitions. One MCP client is served at a time, as the shared listener defines. Non-loopback binds relax the `Host` check and are not exercised; a listener `error` after binding is wired but not provoked by a test. +## Resource and cancellation boundaries + +- Whole discovery admission reserves at most 1 MiB using six bytes per UTF-16 + unit plus schema overhead before retaining each schema property. Aggregate + tools, arguments and selected environment identity participate; oversized + catalogs fail with a non-retryable capacity error before bind. The calibration + used 1500 query fields with eight concurrent discovery requests; compact wire + size can be smaller than the conservative reservation. +- Query matching scans selected names one field at a time; path scoring scans + once and retains only supplied parameter coordinates. Native form-decoding + semantics, malformed escape handling and complete captured facts are preserved. +- Content-type display is bounded to 1 KiB and reports `content_type_truncated`; + complete media-type semantics remain available to body interpretation. +- Cancellation also owns DNS/listen startup; an observed abort closes the + listener before its URL is announced. + ## Deferred issue criteria Exporting a manifest and its evidence as an agent-environment bundle with a diff --git a/src/application/environment/environmentReplay.ts b/src/application/environment/environmentReplay.ts index fd415985ad..c3f709562c 100644 --- a/src/application/environment/environmentReplay.ts +++ b/src/application/environment/environmentReplay.ts @@ -4,6 +4,10 @@ import type { AnalysisError } from "../../domain/analysisErrorBase.js"; import { AnalysisInputError } from "../../domain/analysisErrorCore.js"; import { isExactJsonNumber } from "../../domain/exactJsonNumber.js"; import { jsonValueSchema, type JsonValue } from "../../domain/jsonValue.js"; +import { + iterateUrlQueryParameters, + recordedPathSegments, +} from "../../domain/recordedTransportUrl.js"; import { safeParseJson } from "../../domain/safeJson.js"; import { err, ok, type Result } from "../../domain/result.js"; import { environmentToolArguments } from "./environmentTools.js"; @@ -33,6 +37,7 @@ export interface ReplayResult { readonly recorded_query: string | null; /** Set when the recorded path or query was cut to REPLAY_METADATA_LIMIT_BYTES; the Evidence record keeps it whole. */ readonly recorded_line_truncated: boolean; + readonly content_type_truncated: boolean; readonly body_recorded: boolean; readonly body_encoding: "utf-8" | "base64"; readonly truncated: boolean; @@ -63,16 +68,24 @@ const scoreExchange = ( exchange: RecordedExchange, args: ReplayArguments, ): MatchScore => { - const segments = - tool.params.length === 0 ? [] : exchange.path.split("/").slice(1); - let pathProvided = 0; - let pathMatches = 0; - let verbatimMatches = 0; + const providedPaths: { index: number; value: string }[] = []; for (const param of tool.params) { const value = args[pathArguments.get(param.name) ?? param.name]; - if (typeof value !== "string") continue; - pathProvided += 1; - const segment = segments[param.index]; + if (typeof value === "string") + providedPaths.push({ index: param.index, value }); + } + const segments = + providedPaths.length === 0 + ? new Map() + : recordedPathSegments( + exchange.path, + providedPaths.map((param) => param.index), + ); + const pathProvided = providedPaths.length; + let pathMatches = 0; + let verbatimMatches = 0; + for (const { index, value } of providedPaths) { + const segment = segments.get(index); if (segment === value) verbatimMatches += 1; if ( segment !== undefined && @@ -80,16 +93,21 @@ const scoreExchange = ( ) pathMatches += 1; } - let query: URLSearchParams | undefined; - let queryProvided = 0; - let queryMatches = 0; + const supplied = new Map(); for (const [name, argument] of queryArguments) { const value = args[argument]; - if (typeof value !== "string") continue; - queryProvided += 1; - query ??= new URLSearchParams(exchange.query ?? ""); - if (query.getAll(name).includes(value)) queryMatches += 1; + if (typeof value === "string") supplied.set(name, value); } + const matched = new Set(); + if (supplied.size > 0) + for (const [name, value] of iterateUrlQueryParameters( + exchange.query ?? "", + )) { + if (supplied.get(name) === value) matched.add(name); + if (matched.size === supplied.size) break; + } + const queryProvided = supplied.size; + const queryMatches = matched.size; return { pathMatches, verbatimMatches, @@ -219,9 +237,13 @@ export const replayEnvironmentTool = ( exchange.query === null ? null : utf8Prefix(exchange.query, REPLAY_METADATA_LIMIT_BYTES); + const contentType = + exchange.contentType === null + ? null + : utf8Prefix(exchange.contentType, REPLAY_METADATA_LIMIT_BYTES); return ok({ status: exchange.status, - content_type: exchange.contentType, + content_type: contentType, body, replay: { record_ordinal: exchange.ordinal, @@ -232,6 +254,7 @@ export const replayEnvironmentTool = ( recorded_line_truncated: recordedPath.length < exchange.path.length || (recordedQuery?.length ?? 0) < (exchange.query?.length ?? 0), + content_type_truncated: contentType !== exchange.contentType, body_recorded: exchange.bodyText !== null, body_encoding: exchange.bodyEncoding ?? "utf-8", truncated, diff --git a/src/application/environment/environmentReplayCapacity.test.ts b/src/application/environment/environmentReplayCapacity.test.ts new file mode 100644 index 0000000000..d726e1a725 --- /dev/null +++ b/src/application/environment/environmentReplayCapacity.test.ts @@ -0,0 +1,50 @@ +import { expect, it } from "vitest"; +import { environmentServeSourceWithBody } from "../../../tests/fixtures/environmentServe.js"; +import { + replayEnvironmentTool, + selectReplayExchange, +} from "./environmentReplay.js"; + +it("matches a late repeated query and a distant path coordinate without full tuple/segment arrays", () => { + const source = environmentServeSourceWithBody("{}", "application/json"); + const original = source.manifest.tools.find( + (tool) => tool.name === "get_users_by_id", + )!; + const ordinal = original.record_ordinals[0]!; + const exchange = source.exchanges.get(ordinal)!; + const tool = { + ...original, + params: [{ ...original.params[0]!, index: 20000 }], + query_parameter_names: ["q"], + }; + const retained = { + ...exchange, + path: `/${"literal/".repeat(20000)}wanted`, + query: `?${"q=miss&".repeat(20000)}q=hit`, + }; + expect( + selectReplayExchange(tool, new Map([[ordinal, retained]]), { + id: "wanted", + q: "hit", + }), + ).toEqual({ exchange: retained, exact: true }); +}); + +it("bounds content-type display without changing body interpretation or retained source", () => { + const contentType = `application/json;${"x".repeat(100000)}`; + const source = environmentServeSourceWithBody('{"value":1}', contentType); + const tool = source.manifest.tools.find( + (tool) => tool.name === "get_users_by_id", + )!; + const result = replayEnvironmentTool(source, tool, { id: "2" }); + expect(result.ok).toBe(true); + if (!result.ok) throw result.error; + expect(result.value.body).toEqual({ value: 1 }); + expect( + Buffer.byteLength(result.value.content_type ?? ""), + ).toBeLessThanOrEqual(1024); + expect(result.value.replay.content_type_truncated).toBe(true); + expect(source.exchanges.get(tool.record_ordinals[0]!)?.contentType).toBe( + contentType, + ); +}); diff --git a/src/application/environment/environmentTools.test.ts b/src/application/environment/environmentTools.test.ts index 3e32bb1b46..5efc052337 100644 --- a/src/application/environment/environmentTools.test.ts +++ b/src/application/environment/environmentTools.test.ts @@ -5,12 +5,26 @@ import type { EnvironmentManifestTool } from "./environmentSource.js"; import { environmentCard, environmentToolArguments, - environmentToolDefinitions, - environmentToolInputSchema, + environmentToolDefinitions as prepareDefinitions, REPLAY_CATALOG_INSTRUCTIONS, } from "./environmentTools.js"; const source = environmentServeSource(); +const environmentToolDefinitions = ( + ...args: Parameters +) => { + const result = prepareDefinitions(...args); + if (!result.ok) throw result.error; + return result.value; +}; +const environmentToolInputSchema = (tool: EnvironmentManifestTool) => { + const [definition] = environmentToolDefinitions( + { ...source.manifest, tools: [tool] }, + source.evidenceId, + ); + if (definition === undefined) throw new Error("Missing tool definition"); + return definition.inputSchema; +}; const definitions = environmentToolDefinitions( source.manifest, source.evidenceId, diff --git a/src/application/environment/environmentTools.ts b/src/application/environment/environmentTools.ts index ce4ebaad4d..efbf083e0c 100644 --- a/src/application/environment/environmentTools.ts +++ b/src/application/environment/environmentTools.ts @@ -1,3 +1,6 @@ +import { AnalysisResourceConstraintError } from "../../domain/analysisErrorCore.js"; +import { err, ok, type Result } from "../../domain/result.js"; +import { ENVIRONMENT_SERVE_OPERATION } from "./environmentSource.js"; import type { EnvironmentManifest, EnvironmentManifestTool, @@ -148,49 +151,129 @@ export const environmentToolArguments = ( }; }; -/** Map one manifest tool to its advertised MCP input schema. */ -export const environmentToolInputSchema = ( +/** Lazily render properties so admission precedes retaining a whole schema. */ +function* environmentToolProperties( tool: EnvironmentManifestTool, -): EnvironmentToolInputSchema => { - // A Map, not an object literal, so names such as `constructor` are never mistaken for taken. - const properties = new Map(); - const args = environmentToolArguments(tool); + args: EnvironmentToolArguments, +): IterableIterator { for (const param of tool.params) - properties.set(args.path.get(param.name) ?? param.name, { - type: "string", - description: `Path segment ${String(param.index)} (${param.rule}); observed: ${describeList(param.observed_values, param.observed_values.length)}`, - }); + yield [ + args.path.get(param.name) ?? param.name, + { + type: "string", + description: `Path segment ${String(param.index)} (${param.rule}); observed: ${describeList(param.observed_values, param.observed_values.length)}`, + }, + true, + ]; for (const [name, argument] of args.query) - properties.set(argument, { - type: "string", - description: `Query parameter ${describeText(name, "query name")}; optional, matched against the recorded query on replay.`, - }); + yield [ + argument, + { + type: "string", + description: `Query parameter ${describeText(name, "query name")}; optional, matched against the recorded query on replay.`, + }, + false, + ]; if (args.body !== undefined) - properties.set(args.body, { - type: "object", - description: bodyDescription(tool), - }); - return { - type: "object", - properties: Object.fromEntries(properties), - required: tool.params.map( - (param) => args.path.get(param.name) ?? param.name, - ), - additionalProperties: false, - }; -}; + yield [ + args.body, + { type: "object", description: bodyDescription(tool) }, + false, + ]; +} -/** Map every manifest tool to an MCP tool definition in manifest order. */ +/** Serialization reservation calibrated with 1500 fields and eight concurrent SDK discovery requests. */ +export const ENVIRONMENT_CATALOG_LIMIT_BYTES = 1024 * 1024; +const EMPTY_PROPERTY_BYTES = + JSON.stringify({ type: "string", description: "" }).length + 4; +const EMPTY_TOOL_BYTES = JSON.stringify({ + name: "", + description: "", + inputSchema: { + type: "object", + properties: {}, + required: [], + additionalProperties: false, + }, +}).length; + +/** Admit complete discovery before binding; six bytes per UTF-16 unit covers JSON escaping and UTF-8 expansion. */ export const environmentToolDefinitions = ( manifest: EnvironmentManifest, evidenceId: string, -): readonly EnvironmentToolDefinition[] => - manifest.tools.map((tool) => ({ - name: tool.name, - description: `${REPLAY_CATALOG_INSTRUCTIONS} ${describeText(tool.method, "method")} ${describeText(tool.path_template, "path")}; replays retained records [${describeList(tool.record_ordinals, tool.record_ordinals.length)}]; evidence ${evidenceId}`, - inputSchema: environmentToolInputSchema(tool), - tool, - })); + name: string = manifest.name, +): Result< + readonly EnvironmentToolDefinition[], + AnalysisResourceConstraintError +> => { + let remaining = ENVIRONMENT_CATALOG_LIMIT_BYTES; + const reserve = (bytes: number): boolean => { + if (bytes > remaining) return false; + remaining -= bytes; + return true; + }; + const text = (value: string): boolean => reserve(2 + value.length * 6); + const refused = () => + err( + new AnalysisResourceConstraintError( + ENVIRONMENT_SERVE_OPERATION, + "capacity", + "Environment discovery exceeds its serialization reservation budget", + { + max_catalog_bytes: ENVIRONMENT_CATALOG_LIMIT_BYTES, + reserved_bytes: ENVIRONMENT_CATALOG_LIMIT_BYTES - remaining, + json_bytes_per_utf16_unit: 6, + }, + { + retryable: false, + remediationAction: + "Project a smaller capture or manifest before serving it; complete inputs remain in Evidence.", + }, + ), + ); + if (!text(name) || !reserve(JSON.stringify(environmentCard("", [])).length)) + return refused(); + const definitions: EnvironmentToolDefinition[] = []; + for (const tool of manifest.tools) { + if (!reserve(EMPTY_TOOL_BYTES) || !text(tool.name)) return refused(); + const count = + tool.params.length + + tool.query_parameter_names.length + + (WRITE_METHODS.has(tool.method) ? 1 : 0); + // A lower bound rejects enormous argument lists before constructing their alias map. + if (!reserve(count * EMPTY_PROPERTY_BYTES)) return refused(); + const description = `${REPLAY_CATALOG_INSTRUCTIONS} ${describeText(tool.method, "method")} ${describeText(tool.path_template, "path")}; replays retained records [${describeList(tool.record_ordinals, tool.record_ordinals.length)}]; evidence ${evidenceId}`; + if (!text(description)) return refused(); + const args = environmentToolArguments(tool); + const properties = new Map(); + const required: string[] = []; + for (const [argument, property, mandatory] of environmentToolProperties( + tool, + args, + )) { + if ( + !text(argument) || + !text(property.description) || + (mandatory && !text(argument)) + ) + return refused(); + properties.set(argument, property); + if (mandatory) required.push(argument); + } + definitions.push({ + name: tool.name, + description, + inputSchema: { + type: "object", + properties: Object.fromEntries(properties), + required, + additionalProperties: false, + }, + tool, + }); + } + return ok(definitions); +}; /** The environment card served at `/.well-known/agent-env.json`; key order is part of the contract. */ export interface EnvironmentCard { diff --git a/src/cli/environmentCommands.ts b/src/cli/environmentCommands.ts index e0a9819c22..b9f0427c37 100644 --- a/src/cli/environmentCommands.ts +++ b/src/cli/environmentCommands.ts @@ -116,10 +116,11 @@ const serveEnvironmentCommand = async ( const listenerFailed = new Promise((resolve) => { failListener = resolve; }); - let served: Awaited>; + let served: import("../server/environment/serveEnvironment.js").ServedEnvironment; try { - served = await serveEnvironment({ + const started = await serveEnvironment({ source: source.value, + signal, ...endpoint.value, ...(environmentName.value === undefined ? {} @@ -127,7 +128,10 @@ const serveEnvironmentCommand = async ( logger: listenerLogger(environment, logger), onerror: (error) => failListener(error), }); + if (!started.ok) return cliError(started.error); + served = started.value; } catch (cause: unknown) { + if (signal.aborted && cause === signal.reason) return cancelled(); if (!(cause instanceof StreamableHttpListenError)) throw cause; return cliError(transportError(endpoint.value, cause)); } diff --git a/src/domain/recordedTransportUrl.test.ts b/src/domain/recordedTransportUrl.test.ts index ddaacfce18..6a1070abbf 100644 --- a/src/domain/recordedTransportUrl.test.ts +++ b/src/domain/recordedTransportUrl.test.ts @@ -1,6 +1,11 @@ import { expect, it } from "vitest"; -import { parseRecordedTransportUrl } from "./recordedTransportUrl.js"; +import { + parseRecordedTransportUrl, + parseUrlQueryParameterNames, + iterateUrlQueryParameters, + recordedPathSegments, +} from "./recordedTransportUrl.js"; it("recognizes an empty userinfo marker at the authority start", () => { expect(parseRecordedTransportUrl("https://@host")).toMatchObject({ @@ -23,3 +28,40 @@ it.each([ ); expect(parseRecordedTransportUrl(url)).not.toHaveProperty("query"); }); + +it.each([ + "", + "?", + "?a=1&a=2&?a=3&&=x&x=1=2", + "??=x&%3F=y&+=space&%00=nul", + "?x=%FF&x=%E2%82&%ED%A0%80=%ZZ", + "?x=\ud800&y=\udfff", + "?🌟=😃&x=é", + "flag&flag&last=2", + "&=a&=&empty", + "?x=1+2&%2B=%26%3D", +])("streams form fields with native meanings: %s", (query) => { + const expected = new URLSearchParams(query); + expect([...iterateUrlQueryParameters(query)]).toEqual([...expected]); + expect([...parseUrlQueryParameterNames(query)]).toEqual([...expected.keys()]); + const selected = new Map([ + ["x", ""], + ["", ""], + ]); + expect([...iterateUrlQueryParameters(query, selected)]).toEqual( + [...expected].filter(([name]) => selected.has(name)), + ); +}); + +it("reads only requested path coordinates, retaining empty and raw segments", () => { + for (const path of ["", "/", "//a/", "/a/%2F/b", "relative/first/last"]) + for (const index of [-1, 0, 1, 2, 3, 8]) + expect(recordedPathSegments(path, [index]).get(index)).toBe( + path.split("/").slice(1)[index], + ); + expect( + recordedPathSegments(`/${"literal/".repeat(20000)}wanted`, [20000]).get( + 20000, + ), + ).toBe("wanted"); +}); diff --git a/src/domain/recordedTransportUrl.ts b/src/domain/recordedTransportUrl.ts index 8f5fa1e38f..3ea154d69e 100644 --- a/src/domain/recordedTransportUrl.ts +++ b/src/domain/recordedTransportUrl.ts @@ -90,7 +90,76 @@ export const parseRecordedTransportUrl = ( }; }; -/** WHATWG form-query name decoding; preserve order, repeats and empty names. */ -export const parseUrlQueryParameterNames = ( +/** WHATWG form decoding; native fallback preserves replacement behavior on malformed escapes/UTF-8. */ +const formComponent = (raw: string): string => { + if (!/[%+\uD800-\uDFFF]/u.test(raw)) return raw; + try { + return decodeURIComponent(raw.replaceAll("+", " ")).toWellFormed(); + } catch { + return new URLSearchParams(`value=${raw}`).get("value") ?? ""; + } +}; + +/** Separator coordinates shared by name projection and selected-value matching. */ +function* queryFields( query: string, -): IterableIterator => new URLSearchParams(query).keys(); +): IterableIterator { + let start = query.startsWith("?") ? 1 : 0; + let equals = query.indexOf("=", start); + while (start < query.length) { + const separator = query.indexOf("&", start); + const end = separator < 0 ? query.length : separator; + if (end > start) + yield [start, equals >= start && equals < end ? equals : end, end]; + start = end + 1; + if (equals >= 0 && equals < start) equals = query.indexOf("=", start); + } +} + +/** Scan one field at a time; selection avoids decoding unrelated values. */ +export function* iterateUrlQueryParameters( + query: string, + selected?: ReadonlyMap, +): IterableIterator<[string, string]> { + for (const [start, nameEnd, end] of queryFields(query)) { + const name = formComponent(query.slice(start, nameEnd)); + if (selected === undefined || selected.has(name)) + yield [ + name, + formComponent(nameEnd < end ? query.slice(nameEnd + 1, end) : ""), + ]; + } +} + +/** WHATWG form-query names, preserving order, repeats and empty names without decoding values. */ +export function* parseUrlQueryParameterNames( + query: string, +): IterableIterator { + for (const [start, nameEnd] of queryFields(query)) + yield formComponent(query.slice(start, nameEnd)); +} + +/** Scan a raw path once, retaining only caller-selected coordinates. */ +export const recordedPathSegments = ( + path: string, + indexes: Iterable, +): ReadonlyMap => { + const needed = new Set(); + for (const index of indexes) + if (Number.isSafeInteger(index) && index >= 0) needed.add(index); + const result = new Map(); + if (needed.size === 0) return result; + let start = path.indexOf("/"); + if (start < 0) return result; + start += 1; + let index = 0; + while (needed.size > 0) { + const end = path.indexOf("/", start); + if (needed.delete(index)) + result.set(index, path.slice(start, end < 0 ? path.length : end)); + if (end < 0) break; + start = end + 1; + index += 1; + } + return result; +}; diff --git a/src/server/environment/environmentCapacity.test.ts b/src/server/environment/environmentCapacity.test.ts new file mode 100644 index 0000000000..146e839b7b --- /dev/null +++ b/src/server/environment/environmentCapacity.test.ts @@ -0,0 +1,70 @@ +import { expect, it } from "vitest"; +import { environmentServeSource } from "../../../tests/fixtures/environmentServe.js"; +import { + environmentToolDefinitions, + ENVIRONMENT_CATALOG_LIMIT_BYTES, +} from "../../application/environment/environmentTools.js"; +import { serveEnvironment } from "./serveEnvironment.js"; + +it("refuses a complete oversized discovery before trying to bind", async () => { + const source = environmentServeSource(); + const tool = source.manifest.tools[0]!; + const manifest = { + ...source.manifest, + tools: [ + { + ...tool, + query_parameter_names: Array.from( + { length: 10000 }, + (_, index) => `q_${index}`, + ), + }, + ], + }; + const result = await serveEnvironment({ + source: { ...source, manifest }, + host: "invalid.rea.invalid", + port: 0, + }); + expect(result).toMatchObject({ + ok: false, + error: { + _tag: "AnalysisResourceConstraintError", + resource: "capacity", + retryable: false, + reportedLimits: { max_catalog_bytes: ENVIRONMENT_CATALOG_LIMIT_BYTES }, + }, + }); + expect(manifest.tools[0]?.query_parameter_names).toHaveLength(10000); +}); + +it("charges environment identity, aggregate tools and escaped text before producing a card", () => { + const source = environmentServeSource(); + const tools = Array.from({ length: 1000 }, (_, index) => ({ + ...source.manifest.tools[0]!, + name: `tool_${index}`, + params: [], + query_parameter_names: [], + })); + expect( + environmentToolDefinitions({ ...source.manifest, tools }, source.evidenceId) + .ok, + ).toBe(false); + expect( + environmentToolDefinitions( + source.manifest, + source.evidenceId, + "x".repeat(ENVIRONMENT_CATALOG_LIMIT_BYTES), + ).ok, + ).toBe(false); + const normal = environmentToolDefinitions(source.manifest, source.evidenceId); + expect(normal.ok).toBe(true); + if (!normal.ok) throw normal.error; + expect( + Buffer.byteLength( + JSON.stringify( + normal.value.map(({ tool: _, ...definition }) => definition), + ), + ), + ).toBeLessThan(ENVIRONMENT_CATALOG_LIMIT_BYTES); +}); diff --git a/src/server/environment/serveEnvironment.test.ts b/src/server/environment/serveEnvironment.test.ts index df6b3a4e4f..359b82e5e1 100644 --- a/src/server/environment/serveEnvironment.test.ts +++ b/src/server/environment/serveEnvironment.test.ts @@ -10,7 +10,7 @@ import { afterEach, describe, expect, it } from "vitest"; import { environmentServeSource } from "../../../tests/fixtures/environmentServe.js"; import { environmentCard, - environmentToolDefinitions, + environmentToolDefinitions as prepareDefinitions, REPLAY_CATALOG_INSTRUCTIONS, } from "../../application/environment/environmentTools.js"; import { TARGET_CONTENT_RESULT_META } from "../toolResultEncoding.js"; @@ -19,11 +19,25 @@ import { ENVIRONMENT_RPC_PATH, environmentRoutes, MAX_RPC_BATCH_MESSAGES, - serveEnvironment, + serveEnvironment as startEnvironment, type ServedEnvironment, } from "./serveEnvironment.js"; const source = environmentServeSource(); +const environmentToolDefinitions = ( + ...args: Parameters +) => { + const result = prepareDefinitions(...args); + if (!result.ok) throw result.error; + return result.value; +}; +const serveEnvironment = async ( + ...args: Parameters +) => { + const result = await startEnvironment(...args); + if (!result.ok) throw result.error; + return result.value; +}; const definitions = environmentToolDefinitions( source.manifest, source.evidenceId, diff --git a/src/server/environment/serveEnvironment.ts b/src/server/environment/serveEnvironment.ts index 70e8e997a6..d3e12f48bd 100644 --- a/src/server/environment/serveEnvironment.ts +++ b/src/server/environment/serveEnvironment.ts @@ -22,6 +22,8 @@ import { import { projectAnalysisError } from "../../domain/analysisErrorProjection.js"; import { isExactJsonNumber } from "../../domain/exactJsonNumber.js"; import { safeParseJson } from "../../domain/safeJson.js"; +import type { AnalysisResourceConstraintError } from "../../domain/analysisErrorCore.js"; +import { ok, type Result } from "../../domain/result.js"; import { PRODUCT_IDENTITY } from "../../identity.js"; import { jsonRpcErrorResponse, @@ -205,6 +207,7 @@ export interface ServeEnvironmentOptions { readonly source: EnvironmentSource; readonly host: string; readonly port: number; + readonly signal?: AbortSignal; /** Served name; defaults to the manifest's environment name. */ readonly environmentName?: string; readonly logger?: Logger; @@ -220,14 +223,21 @@ export interface ServedEnvironment extends StreamableHttpServer { /** Serve one manifest as an agent environment until closed. */ export const serveEnvironment = async ( options: ServeEnvironmentOptions, -): Promise => { +): Promise> => { const { source, host, port, logger } = options; const name = options.environmentName ?? source.manifest.name; - const tools = environmentToolDefinitions(source.manifest, source.evidenceId); + const prepared = environmentToolDefinitions( + source.manifest, + source.evidenceId, + name, + ); + if (!prepared.ok) return prepared; + const tools = prepared.value; const card = environmentCard(name, tools); const served = await serveStreamableHttp({ host, port, + ...(options.signal === undefined ? {} : { signal: options.signal }), createMcpServer: () => createEnvironmentMcpServer(name, source, tools), routes: environmentRoutes(card), ...(logger === undefined ? {} : { logger }), @@ -239,5 +249,5 @@ export const serveEnvironment = async ( options.onerror?.(error); }, }); - return { ...served, card }; + return ok({ ...served, card }); }; diff --git a/src/server/http/listenHttpServer.ts b/src/server/http/listenHttpServer.ts new file mode 100644 index 0000000000..18a5123d80 --- /dev/null +++ b/src/server/http/listenHttpServer.ts @@ -0,0 +1,42 @@ +import { once } from "node:events"; +import type { Server } from "node:http"; + +/** Cancel DNS/bind through Node's listener owner without coupling later shutdown to the startup signal. */ +export const listenHttpServer = async ( + server: Server, + host: string, + port: number, + signal?: AbortSignal, +): Promise => { + signal?.throwIfAborted(); + const startup = new AbortController(); + const cancel = () => startup.abort(signal?.reason); + signal?.addEventListener("abort", cancel, { once: true }); + try { + server.listen({ host, port, signal: startup.signal }); + await once(server, "listening", { signal: startup.signal }); + startup.signal.throwIfAborted(); + } catch (cause: unknown) { + if ( + startup.signal.aborted && + (cause === startup.signal.reason || + (cause instanceof Error && cause.name === "AbortError")) + ) { + await new Promise((resolve, reject) => { + server.close((error) => { + if ( + error === undefined || + ("code" in error && error.code === "ERR_SERVER_NOT_RUNNING") + ) + resolve(); + else reject(error); + }); + server.closeAllConnections(); + }); + throw startup.signal.reason; + } + throw cause; + } finally { + signal?.removeEventListener("abort", cancel); + } +}; diff --git a/src/server/http/serveStreamableHttp.test.ts b/src/server/http/serveStreamableHttp.test.ts index 9ec7af0ecd..b3234fa6dd 100644 --- a/src/server/http/serveStreamableHttp.test.ts +++ b/src/server/http/serveStreamableHttp.test.ts @@ -158,6 +158,27 @@ const initializeRequest = (): Request => }), }); +it("closes a bound listener without announcing it when startup aborts during factory creation", async () => { + const controller = new AbortController(); + const reason = new Error("startup cancelled"); + const lines: string[] = []; + const closed = vi.fn(); + await expect( + serveStreamableHttp({ + host: "127.0.0.1", + port: 0, + signal: controller.signal, + createMcpServer: () => { + controller.abort(reason); + return createEchoServer(closed); + }, + logger: pino({}, { write: (line) => lines.push(line) }), + }), + ).rejects.toBe(reason); + expect(closed).toHaveBeenCalledTimes(1); + expect(lines.join("")).not.toContain("MCP transport listening"); +}); + it("announces the bound URL and serves one session at a time on one live instance", async () => { const { served, entries, messages, factoryCalls, shutdownHooks } = await start(); diff --git a/src/server/http/serveStreamableHttp.ts b/src/server/http/serveStreamableHttp.ts index 978a2bd2ab..56c0fde8cc 100644 --- a/src/server/http/serveStreamableHttp.ts +++ b/src/server/http/serveStreamableHttp.ts @@ -11,7 +11,6 @@ import { type Server, } from "@modelcontextprotocol/server"; import { randomUUID } from "node:crypto"; -import { once } from "node:events"; import { createServer as createHttpServer, type IncomingMessage, @@ -25,6 +24,7 @@ import { UnsupportedRequestTargetError, writeWebResponse, } from "./nodeHttpBridge.js"; +import { listenHttpServer } from "./listenHttpServer.js"; import { StreamableHttpListenError } from "./streamableHttpListenError.js"; import { NodeHttpRequestOwner, @@ -47,6 +47,8 @@ export interface StreamableHttpRoute { export interface ServeStreamableHttpOptions { readonly host: string; readonly port: number; + /** Cancellation during listener startup; later shutdown remains owned by close(). */ + readonly signal?: AbortSignal; /** Called once; the instance serves every session in turn on a fresh transport. */ readonly createMcpServer: ( context: McpRequestContext, @@ -524,9 +526,13 @@ export const serveStreamableHttp = async ( () => requests.drain(), ]); try { - httpServer.listen(port, host); - await once(httpServer, "listening"); + await listenHttpServer(httpServer, host, port, options.signal); + options.signal?.throwIfAborted(); } catch (cause: unknown) { + if (options.signal?.aborted && cause === options.signal.reason) { + if (httpServer.listening) await prepareTransportShutdown(); + throw cause; + } throw new StreamableHttpListenError(cause); } const bound = httpServer.address(); @@ -568,6 +574,18 @@ export const serveStreamableHttp = async ( options.sessionDrainTimeoutMs, ); slot = live; + const prepareShutdown = stagedClose([ + prepareTransportShutdown, + () => live.close(), + ]); + const close = stagedClose([ + prepareShutdown, + ...(closeInstance ? [() => instance.close()] : []), + ]); + if (options.signal?.aborted) { + await close(); + throw options.signal.reason; + } if (allowedHosts === undefined) logger?.warn( { host }, @@ -577,14 +595,6 @@ export const serveStreamableHttp = async ( logger ?.child({}, { level: "info" }) .info({ transport: "streamable-http", url }, "MCP transport listening"); - const prepareShutdown = stagedClose([ - prepareTransportShutdown, - () => live.close(), - ]); - const close = stagedClose([ - prepareShutdown, - ...(closeInstance ? [() => instance.close()] : []), - ]); return { url, prepareShutdown, close }; }; diff --git a/tests/boundary/cli/environmentServeCli.test.ts b/tests/boundary/cli/environmentServeCli.test.ts index 073ba19f1c..49d471f042 100644 --- a/tests/boundary/cli/environmentServeCli.test.ts +++ b/tests/boundary/cli/environmentServeCli.test.ts @@ -3,9 +3,12 @@ import { join } from "node:path"; import { describe, expect } from "vitest"; +import { inspectAnalysisView } from "../../../src/application/analysisView/AnalysisViewService.js"; + import { analysisViewCaptureEvidence, analysisViewCaptureFixture, + environmentManifestCaptureFixture, } from "../../fixtures/analysisView.js"; import { writeEnvironmentServeFiles } from "../../fixtures/environmentServe.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; @@ -95,3 +98,43 @@ describe("rea environment-serve refusals", () => { }, ); }); + +cliTest( + "refuses a fully authenticated capture whose discovery exceeds capacity before listening", + async ({ cli }) => { + const records = environmentManifestCaptureFixture(); + const first = records.records[0]!; + first.reported.request.url = `https://example.test/api/users/2?${Array.from({ length: 10000 }, (_, index) => `q_${index}=x`).join("&")}`; + const capture = analysisViewCaptureEvidence(records); + const manifest = inspectAnalysisView({ + source: { kind: "inline", evidence: capture }, + view: { kind: "environment-manifest" }, + }); + if (!manifest.ok) throw manifest.error; + const root = await createTestTempDirectory( + "rea-environment-catalog-capacity-", + ); + const files = await writeEnvironmentServeFiles(root, { + capture, + manifest: manifest.value, + }); + const result = await cli.run({ + arguments: [ + "environment-serve", + files.manifestPath, + "--evidence", + files.capturePath, + "--port", + "0", + "--json", + ], + timeoutMs: 30000, + }); + expect(result.exitCode).toBe(1); + expect(result.json).toMatchObject({ + code: "resource_constraint", + details: { operation: "environment-serve", resource: "capacity" }, + }); + expect(result.stderr).not.toContain("MCP transport listening"); + }, +); diff --git a/tests/boundary/process/environmentServeShutdown.test.ts b/tests/boundary/process/environmentServeShutdown.test.ts index 357e5ef4c5..93a53461e3 100644 --- a/tests/boundary/process/environmentServeShutdown.test.ts +++ b/tests/boundary/process/environmentServeShutdown.test.ts @@ -57,99 +57,117 @@ describe("rea environment-serve shutdown", () => { describe.skipIf(process.platform === "win32")( "environment startup cancellation", () => { - processTest( - "cancels a paused Evidence read without announcing a listener", - async ({ processes }) => { - const root = await createTestTempDirectory( - "rea-environment-start-cancel-", - ); - const files = await writeEnvironmentServeFiles(root); - const preload = join(root, "pause-read.mjs"); - const marker = "REA_EVIDENCE_READ_STARTED"; - await writeFile( - preload, - ` + for (const phase of ["read", "bind"] as const) { + processTest( + `cancels paused startup at ${phase} without announcing a listener`, + async ({ processes }) => { + const root = await createTestTempDirectory( + "rea-environment-start-cancel-", + ); + const files = await writeEnvironmentServeFiles(root); + const preload = join(root, "pause-read.mjs"); + const marker = "REA_STARTUP_PAUSED"; + const pause = ` +const interrupted = new Promise(resolve => process.once("SIGTERM", resolve)); +const keepAlive = setInterval(() => undefined, 1000); +process.stderr.write(${JSON.stringify(marker + "\n")}); +try { await interrupted; } finally { clearInterval(keepAlive); } +`; + const source = + phase === "read" + ? ` import fs from "node:fs/promises"; -import { syncBuiltinESMExports } from "node:module"; const open = fs.open; let gated = false; fs.open = async (path, ...args) => { const handle = await open(path, ...args); if (path === ${JSON.stringify(files.manifestPath)}) { const read = handle.read.bind(handle); - handle.read = async (...readArgs) => { - if (!gated) { - gated = true; - const interrupted = new Promise(resolve => process.once("SIGTERM", resolve)); - const keepAlive = setInterval(() => undefined, 1000); - process.stderr.write(${JSON.stringify(marker + "\n")}); - try { await interrupted; } finally { clearInterval(keepAlive); } - } - return read(...readArgs); + handle.read = async (...args) => { + if (!gated) { gated = true; ${pause} } + return read(...args); }; } return handle; }; +` + : ` +import dns from "node:dns"; +const lookup = dns.lookup; +dns.lookup = (host, ...args) => { + if (host !== "localhost") return lookup(host, ...args); + const callback = args.at(-1); + void (async () => { ${pause} if (args[0]?.all) callback(null, [{address: "127.0.0.1", family: 4}]); else callback(null, "127.0.0.1", 4); })(); +}; +`; + await writeFile( + preload, + source + + ` +import { syncBuiltinESMExports } from "node:module"; syncBuiltinESMExports(); `, - ); - const child = processes.spawn( - process.execPath, - [ - "--import", - preload, - "scripts/rea.mjs", - "environment-serve", - files.manifestPath, - "--evidence", - files.capturePath, - "--port", - "0", - "--json", - ], - { - env: { - PATH: process.env.PATH ?? "", - NODE_OPTIONS: "--max-old-space-size=768", - REA_LOG_LEVEL: "info", - }, - }, - ); - const closed = once(child, "close"); - child.stdin.end(); - let stdout = ""; - let stderr = ""; - child.stdout.on("data", (chunk: Buffer) => { - stdout += chunk.toString("utf8"); - }); - await new Promise((resolve, reject) => { - const timeout = setTimeout( - () => reject(new Error(`No Evidence read marker: ${stderr}`)), - 30_000, ); - child.stderr.on("data", (chunk: Buffer) => { - stderr = (stderr + chunk.toString("utf8")).slice(-64 * 1024); - if (stderr.includes(marker)) { + const child = processes.spawn( + process.execPath, + [ + "--import", + preload, + "scripts/rea.mjs", + "environment-serve", + files.manifestPath, + "--evidence", + files.capturePath, + "--host", + phase === "bind" ? "localhost" : "127.0.0.1", + "--port", + "0", + "--json", + ], + { + env: { + PATH: process.env.PATH ?? "", + NODE_OPTIONS: "--max-old-space-size=768", + REA_LOG_LEVEL: "info", + }, + }, + ); + const closed = once(child, "close"); + child.stdin.end(); + let stdout = ""; + let stderr = ""; + child.stdout.on("data", (chunk: Buffer) => { + stdout += chunk.toString("utf8"); + }); + await new Promise((resolve, reject) => { + const timeout = setTimeout( + () => reject(new Error(`No Evidence read marker: ${stderr}`)), + 30_000, + ); + child.stderr.on("data", (chunk: Buffer) => { + stderr = (stderr + chunk.toString("utf8")).slice(-64 * 1024); + if (stderr.includes(marker)) { + clearTimeout(timeout); + resolve(); + } + }); + child.once("exit", () => { clearTimeout(timeout); - resolve(); - } + reject(new Error(`Exited before Evidence read: ${stderr}`)); + }); }); - child.once("exit", () => { - clearTimeout(timeout); - reject(new Error(`Exited before Evidence read: ${stderr}`)); + child.kill("SIGTERM"); + const [code, signal] = await closed; + expect(code).toBe(143); + expect(signal).toBeNull(); + expect(JSON.parse(stdout)).toMatchObject({ + code: "cancelled", + details: { operation: "environment-serve" }, }); - }); - child.kill("SIGTERM"); - const [code, signal] = await closed; - expect(code).toBe(143); - expect(signal).toBeNull(); - expect(JSON.parse(stdout)).toMatchObject({ - code: "cancelled", - details: { operation: "environment-serve" }, - }); - expect(stderr).not.toContain("MCP transport listening"); - }, - SHUTDOWN_TIMEOUT_MS, - ); + expect(stderr).not.toContain("MCP transport listening"); + }, + SHUTDOWN_TIMEOUT_MS, + ); + } }, ); From ac1e36bf60f1b97e9455a86bb21856768a15c071 Mon Sep 17 00:00:00 2001 From: gensokyo-dev Date: Mon, 19 Oct 2026 18:45:45 +0000 Subject: [PATCH 6/8] fix(environment): retain supported form decoding types --- src/domain/recordedTransportUrl.ts | 11 +++++++---- tests/boundary/cli/environmentServeCli.test.ts | 16 +++++++++++++++- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/domain/recordedTransportUrl.ts b/src/domain/recordedTransportUrl.ts index 3ea154d69e..06f68b4c7a 100644 --- a/src/domain/recordedTransportUrl.ts +++ b/src/domain/recordedTransportUrl.ts @@ -93,11 +93,14 @@ export const parseRecordedTransportUrl = ( /** WHATWG form decoding; native fallback preserves replacement behavior on malformed escapes/UTF-8. */ const formComponent = (raw: string): string => { if (!/[%+\uD800-\uDFFF]/u.test(raw)) return raw; - try { - return decodeURIComponent(raw.replaceAll("+", " ")).toWellFormed(); - } catch { - return new URLSearchParams(`value=${raw}`).get("value") ?? ""; + if (!/[\uD800-\uDFFF]/u.test(raw)) { + try { + return decodeURIComponent(raw.replaceAll("+", " ")); + } catch { + // Malformed escapes and UTF-8 use the native replacement semantics below. + } } + return new URLSearchParams(`value=${raw}`).get("value") ?? ""; }; /** Separator coordinates shared by name projection and selected-value matching. */ diff --git a/tests/boundary/cli/environmentServeCli.test.ts b/tests/boundary/cli/environmentServeCli.test.ts index 49d471f042..621bd428ef 100644 --- a/tests/boundary/cli/environmentServeCli.test.ts +++ b/tests/boundary/cli/environmentServeCli.test.ts @@ -104,7 +104,21 @@ cliTest( async ({ cli }) => { const records = environmentManifestCaptureFixture(); const first = records.records[0]!; - first.reported.request.url = `https://example.test/api/users/2?${Array.from({ length: 10000 }, (_, index) => `q_${index}=x`).join("&")}`; + const reported = first.reported; + if ( + typeof reported !== "object" || + reported === null || + Array.isArray(reported) + ) + throw new Error("Fixture has no reported request"); + const request = reported.request; + if ( + typeof request !== "object" || + request === null || + Array.isArray(request) + ) + throw new Error("Fixture has no request object"); + request.url = `https://example.test/api/users/2?${Array.from({ length: 10000 }, (_, index) => `q_${index}=x`).join("&")}`; const capture = analysisViewCaptureEvidence(records); const manifest = inspectAnalysisView({ source: { kind: "inline", evidence: capture }, From a215627ec773074716a75355637ebb98dca0fa37 Mon Sep 17 00:00:00 2001 From: gensokyo-dev Date: Mon, 19 Oct 2026 18:57:24 +0000 Subject: [PATCH 7/8] test(environment): derive capacity fixture from a retained API route --- .../boundary/cli/environmentServeCli.test.ts | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/tests/boundary/cli/environmentServeCli.test.ts b/tests/boundary/cli/environmentServeCli.test.ts index 621bd428ef..a05066475e 100644 --- a/tests/boundary/cli/environmentServeCli.test.ts +++ b/tests/boundary/cli/environmentServeCli.test.ts @@ -4,13 +4,17 @@ import { join } from "node:path"; import { describe, expect } from "vitest"; import { inspectAnalysisView } from "../../../src/application/analysisView/AnalysisViewService.js"; +import { loadEnvironmentSource } from "../../../src/application/environment/environmentSource.js"; import { analysisViewCaptureEvidence, analysisViewCaptureFixture, environmentManifestCaptureFixture, } from "../../fixtures/analysisView.js"; -import { writeEnvironmentServeFiles } from "../../fixtures/environmentServe.js"; +import { + environmentServeSource, + writeEnvironmentServeFiles, +} from "../../fixtures/environmentServe.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { cliTest } from "../../support/cli/cliFixture.js"; @@ -103,7 +107,11 @@ cliTest( "refuses a fully authenticated capture whose discovery exceeds capacity before listening", async ({ cli }) => { const records = environmentManifestCaptureFixture(); - const first = records.records[0]!; + const ordinal = environmentServeSource().manifest.tools.find( + (tool) => tool.name === "get_users_by_id", + )?.record_ordinals[0]; + const first = records.records.find((record) => record.ordinal === ordinal); + if (first === undefined) throw new Error("Fixture has no cited API record"); const reported = first.reported; if ( typeof reported !== "object" || @@ -118,13 +126,26 @@ cliTest( Array.isArray(request) ) throw new Error("Fixture has no request object"); - request.url = `https://example.test/api/users/2?${Array.from({ length: 10000 }, (_, index) => `q_${index}=x`).join("&")}`; + if (typeof request.url !== "string") throw new Error("Fixture has no URL"); + const url = new URL(request.url); + const query = new URLSearchParams(url.search); + for (let index = 0; index < 10000; index += 1) + query.append(`q_${index}`, "x"); + url.search = query.toString(); + request.url = url.href; const capture = analysisViewCaptureEvidence(records); const manifest = inspectAnalysisView({ source: { kind: "inline", evidence: capture }, view: { kind: "environment-manifest" }, }); if (!manifest.ok) throw manifest.error; + const source = loadEnvironmentSource(manifest.value, capture); + if (!source.ok) throw source.error; + expect( + source.value.manifest.tools.some( + (tool) => tool.query_parameter_names.length >= 10000, + ), + ).toBe(true); const root = await createTestTempDirectory( "rea-environment-catalog-capacity-", ); From 9f0116f7ee4ca095100b6d624038165fbcb26638 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Mon, 19 Oct 2026 12:44:10 -0700 Subject: [PATCH 8/8] fix(environment): reserve heap for every loading copy and refuse Windows device names Export's headroom check reserved one native parse per input, while the raw parse, its authenticated Evidence copy and the schema copy of its result are live together while the pair loads; it now reserves all three, so a file near the heap limit is refused as a resource constraint rather than exhausting the heap. A bundle name that Windows reserves as a device name (con, prn, aux, nul, com1-com9, lpt1-lpt9) is refused while planning, before any file is written, since the env folder could not be created there. Co-Authored-By: Claude Opus 5.5 --- docs/agent-environments.md | 3 +- docs/cli.md | 6 ++-- .../environment-export/spec.md | 4 ++- .../environment/environmentBundle.test.ts | 26 ++++++++++++++ .../environment/environmentBundle.ts | 19 +++++++++++ .../environment/environmentExport.test.ts | 34 +++++++++++++++++++ .../environment/environmentExport.ts | 8 +++-- 7 files changed, 94 insertions(+), 6 deletions(-) diff --git a/docs/agent-environments.md b/docs/agent-environments.md index 4c71331fb8..df63aefaca 100644 --- a/docs/agent-environments.md +++ b/docs/agent-environments.md @@ -248,7 +248,8 @@ grafana-example/ ``` The command reads each file once, after checking that reading and parsing the -whole file fits the process's live heap headroom (a file that does not is +whole file, and the two copies made while the pair is authenticated, fits the +process's live heap headroom (a file that does not is refused as a `resource_constraint` before it is read), then authenticates and cross-checks the pair as `environment-serve` does, so a manifest projected from another capture is refused with the same `invalid_request` before anything is written, and the diff --git a/docs/cli.md b/docs/cli.md index dd557703a6..efed8a8bea 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -383,7 +383,8 @@ readable by their owner only, since they hold captured content) with smoke task `tasks/smoke.json`. `--package-tarball` copies a packed `rea-agents` tarball into the env folder and installs it instead of the registry package, which is how the hosted lane proves a checkout rather than a release. `--name` -replaces the manifest's kebab-case name as the env folder and env id. The +replaces the manifest's kebab-case name as the env folder and env id; a device +name Windows reserves (`con`, `nul`, `com1` and the like) is refused. The command prints one JSON document listing the written files with their sizes and the pinned package source. See [agent environments](agent-environments.md#export-a-bundle) for the folder @@ -438,7 +439,8 @@ error and exit `1`, as does a listener that fails after binding. `rea environment-export` is one-shot: it prints the written files and exits `0`, or prints a structured `invalid_request` error and exits `1` for the same pair refusals, a non-empty or file-occupied directory, a missing package -tarball or a name that is not a kebab-case label, writing nothing in each case. +tarball or a name that is not a kebab-case label or is a Windows device name, +writing nothing in each case. A write that fails after the first file (a permission or disk-space failure on a later path) propagates as the host's failure and leaves the partial folder in place for inspection; remove it before retrying, since the next diff --git a/docs/specs/agent-environments/environment-export/spec.md b/docs/specs/agent-environments/environment-export/spec.md index 90a7feb5f7..f150f86112 100644 --- a/docs/specs/agent-environments/environment-export/spec.md +++ b/docs/specs/agent-environments/environment-export/spec.md @@ -33,7 +33,9 @@ previous increment. copies of the inputs (mode `0600`, so captured content stays readable by its owner only whatever the umask), `artifacts/check_/verify.py` and `tasks/smoke.json`, where `` defaults to the manifest's kebab-case name - and `--name` must be a kebab-case label of at most 64 characters. + and `--name` must be a kebab-case label of at most 64 characters that is not a + device name Windows reserves (`con`, `prn`, `aux`, `nul`, `com1`–`com9`, + `lpt1`–`lpt9`), since the env folder could not be created there. - The Dockerfile reads `FROM node:24.18.0-bookworm-slim`, sets `MCP_HOST=0.0.0.0` and `MCP_PORT=18765`, installs Debian's `python3` for the framework's readiness probe, installs diff --git a/src/application/environment/environmentBundle.test.ts b/src/application/environment/environmentBundle.test.ts index 69d85a1348..a2d18f75be 100644 --- a/src/application/environment/environmentBundle.test.ts +++ b/src/application/environment/environmentBundle.test.ts @@ -186,6 +186,32 @@ describe("planEnvironmentBundle", () => { }); }); +describe("planEnvironmentBundle portable names", () => { + it("refuses device names Windows reserves for files and folders, before planning any file", () => { + for (const name of ["con", "prn", "aux", "nul", "com1", "lpt9"]) { + const refused = planEnvironmentBundle(environmentServeSource(), { + name, + packageSource: registryPackageSource(), + }); + if (refused.ok) throw new Error(`Expected ${name} to be refused`); + expect(projectAnalysisError(refused.error)).toMatchObject({ + code: "invalid_request", + details: { + operation: "environment-export", + issues: [{ path: ["name"], reason: "invalid_value" }], + }, + }); + } + for (const name of ["console", "com10", "nul-check"]) + expect( + planEnvironmentBundle(environmentServeSource(), { + name, + packageSource: registryPackageSource(), + }).ok, + ).toBe(true); + }); +}); + describe("planEnvironmentBundle replay check", () => { it("keeps the first record's unknown status instead of borrowing another record's", () => { const source = environmentServeSource(); diff --git a/src/application/environment/environmentBundle.ts b/src/application/environment/environmentBundle.ts index 45cf72af95..589bbc37ae 100644 --- a/src/application/environment/environmentBundle.ts +++ b/src/application/environment/environmentBundle.ts @@ -22,6 +22,8 @@ export const BUNDLE_TARBALL_NAME = "rea-agents.tgz"; /** Folder and env id rule: a kebab-case label the agentenv framework accepts as a local id. */ const BUNDLE_NAME = /^[a-z0-9]+(?:-[a-z0-9]+)*$/u; const BUNDLE_NAME_MAX_LENGTH = 64; +/** Win32 reserves these device names for files and folders, so the env folder can never take one. */ +const WINDOWS_RESERVED_NAME = /^(?:con|prn|aux|nul|com[1-9]|lpt[1-9])$/u; /** Where the Dockerfile installs the CLI from. */ export type EnvironmentPackageSource = @@ -86,6 +88,22 @@ const nameError = (name: string): AnalysisInputError => ], ); +const reservedNameError = (name: string): AnalysisInputError => + new AnalysisInputError( + ENVIRONMENT_EXPORT_OPERATION, + { + userMessage: `The bundle name ${JSON.stringify(name)} is a device name Windows reserves, so it cannot name the env folder; pass --name with another label.`, + }, + [ + { + path: ["name"], + reason: "invalid_value", + message: + "con, prn, aux, nul, com1-com9 and lpt1-lpt9 are reserved file names on Windows.", + }, + ], + ); + const installLine = (source: EnvironmentPackageSource): string => source.kind === "registry" ? `RUN npm install -g --ignore-scripts --no-fund --no-audit ${source.specifier}` @@ -275,6 +293,7 @@ export const planEnvironmentBundle = ( const name = options.name ?? source.manifest.name; if (!BUNDLE_NAME.test(name) || name.length > BUNDLE_NAME_MAX_LENGTH) return err(nameError(name)); + if (WINDOWS_RESERVED_NAME.test(name)) return err(reservedNameError(name)); const toolCount = source.manifest.tools.length; const check = replayCheck(source); const envFolder = `envs/${name}`; diff --git a/src/application/environment/environmentExport.test.ts b/src/application/environment/environmentExport.test.ts index deba925f8f..ddebafc3e6 100644 --- a/src/application/environment/environmentExport.test.ts +++ b/src/application/environment/environmentExport.test.ts @@ -12,8 +12,10 @@ import { import { createTestTempDirectory } from "../../../tests/fixtures/temporaryDirectory.js"; import { projectAnalysisError } from "../../domain/analysisErrorProjection.js"; import { PACKAGE_METADATA } from "../../generatedPackageMetadata.js"; +import { nativeJsonParseRequiredBytes } from "../JsonInputFile.js"; import { exportEnvironmentBundle, + LIVE_PARSE_COPIES, readEnvironmentExportInputs, } from "./environmentExport.js"; @@ -270,3 +272,35 @@ describe("exportEnvironmentBundle file modes", () => { }, ); }); + +describe("readEnvironmentExportInputs heap admission", () => { + it("reserves headroom for every copy that is live while the pair loads, not only the native parse", async () => { + const root = await createTestTempDirectory( + "rea-environment-export-copies-", + ); + const files = await writeEnvironmentServeFiles( + root, + environmentServeFixture(), + ); + const { size } = await stat(files.manifestPath); + // Room for one native parse, but not for the authenticated and schema copies that follow it. + vi.mocked(v8.getHeapStatistics).mockReturnValueOnce({ + ...v8.getHeapStatistics(), + total_available_size: nativeJsonParseRequiredBytes(size) * 2, + }); + const refused = await readEnvironmentExportInputs( + files.manifestPath, + files.capturePath, + ); + expect(refused.ok).toBe(false); + if (refused.ok) return; + expect(refused.error).toMatchObject({ + _tag: "AnalysisResourceConstraintError", + reportedLimits: { + input_path: files.manifestPath, + required_heap_bytes: + nativeJsonParseRequiredBytes(size) * LIVE_PARSE_COPIES, + }, + }); + }); +}); diff --git a/src/application/environment/environmentExport.ts b/src/application/environment/environmentExport.ts index 8252de9f95..ccb389becd 100644 --- a/src/application/environment/environmentExport.ts +++ b/src/application/environment/environmentExport.ts @@ -177,13 +177,17 @@ const NOT_A_FILE_CODES = new Set([ "ENAMETOOLONG", ]); -/** Refuse, as a resource constraint, a file whose whole read and native parse would not fit live heap headroom. */ +/** The raw parse, its authenticated Evidence copy and the schema copy of its result are live together while the pair loads. */ +export const LIVE_PARSE_COPIES = 3; + +/** Refuse, as a resource constraint, a file whose whole read, native parse and loading copies would not fit live heap headroom. */ const headroomRefusal = ( path: string, inputBytes: number, ): AnalysisResourceConstraintError | undefined => { const heap = getHeapStatistics(); - const requiredBytes = nativeJsonParseRequiredBytes(inputBytes); + const requiredBytes = + nativeJsonParseRequiredBytes(inputBytes) * LIVE_PARSE_COPIES; return requiredBytes <= heap.total_available_size ? undefined : new AnalysisResourceConstraintError(