From 130bc4326a51e7eeb341b1ab211cce3ef25b4a1d Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Mon, 19 Oct 2026 10:06:59 -0700 Subject: [PATCH] fix(environment): keep exact JSON numbers, stop startup on shutdown, bound described paths Replay parses a JSON body only when every integer survives exactly; a body with an integer outside JavaScript's safe range, such as an id of 9007199254740993, is returned as its recorded text instead of with other digits. A SIGINT or SIGTERM that arrives while environment-serve reads and authenticates its files now ends startup with a cancelled result before the listener binds, rather than binding and stopping at once. A tool description cuts a captured path longer than 1 KiB at a character boundary and points to the manifest Evidence, matching the list sample budget; the manifest keeps the full path. Co-Authored-By: Claude Opus 5.5 --- docs/agent-environments.md | 12 ++++-- .../environment-serve/spec.md | 10 +++-- .../environment/environmentReplay.test.ts | 21 ++++++++++ .../environment/environmentReplay.ts | 34 +++++++++++++--- .../environment/environmentTools.test.ts | 17 ++++++++ .../environment/environmentTools.ts | 14 ++++++- src/cli/environmentCommands.ts | 18 ++++++++- .../cli/environmentServeCancellation.test.ts | 40 +++++++++++++++++++ 8 files changed, 150 insertions(+), 16 deletions(-) create mode 100644 tests/boundary/cli/environmentServeCancellation.test.ts diff --git a/docs/agent-environments.md b/docs/agent-environments.md index 0c7c83197a..2f149ef6db 100644 --- a/docs/agent-environments.md +++ b/docs/agent-environments.md @@ -45,7 +45,8 @@ 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. +a captured path longer than 1 KiB is cut the same way in its tool description. +This only limits descriptive context, not arguments, record selection or replay. ## 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 without changing a number; +otherwise it is the recorded text, so a body with an integer outside +JavaScript's safe range (such as an id of `9007199254740993`) keeps its digits. 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 @@ -125,7 +127,9 @@ manifest's `name` is used, and a set but blank value is refused as 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`. +`{"status":"stopped", …}` document and exits `130` or `143`. A signal that +arrives while the files load ends startup with a `cancelled` result, and the same +exit status, before the listener binds. 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..bd210e8ec6 100644 --- a/docs/specs/agent-environments/environment-serve/spec.md +++ b/docs/specs/agent-environments/environment-serve/spec.md @@ -38,14 +38,15 @@ later increment. standalone card readers receive the trust notice before MCP initialization. - 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. + manifest-Evidence notice, and cut a captured path longer than 1 KiB in its + tool description. Keep complete arrays and replay semantics unchanged. - 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, - else text; bodies over 256 KiB cut at a UTF-8 boundary with + `content_type`, `body` (parsed JSON for JSON-like content types that parse + without changing a number, 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 when the capture kept no response body, so a `null` body is not mistaken for @@ -67,7 +68,8 @@ later increment. then `127.0.0.1` and port `0`), announce the bound URL as one pino JSON line on stderr, serve through the shared streamable HTTP listener, and run until SIGINT or SIGTERM only, then close, print one `stopped` document and exit - `130` or `143` through the shared command cancellation handler; a listener + `130` or `143` through the shared command cancellation handler; a signal + during loading returns `cancelled` before binding; a listener failure after binding is logged and ends the command with a `resource_constraint` error and exit `1`. - Register the flat `environment-serve` command in the CLI inventory with diff --git a/src/application/environment/environmentReplay.test.ts b/src/application/environment/environmentReplay.test.ts index 72720f072a..22a84f11a7 100644 --- a/src/application/environment/environmentReplay.test.ts +++ b/src/application/environment/environmentReplay.test.ts @@ -278,6 +278,27 @@ 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 26c99431cd..f5a515452a 100644 --- a/src/application/environment/environmentReplay.ts +++ b/src/application/environment/environmentReplay.ts @@ -4,7 +4,6 @@ import type { AnalysisError } from "../../domain/analysisErrorBase.js"; import { AnalysisInputError } from "../../domain/analysisErrorCore.js"; import { jsonValueSchema, type JsonValue } from "../../domain/jsonValue.js"; import { err, ok, type Result } from "../../domain/result.js"; -import { safeParseJson } from "../../domain/safeJson.js"; import { environmentToolArguments } from "./environmentTools.js"; import { REPLAY_BODY_LIMIT_BYTES, @@ -160,15 +159,40 @@ const replayBody = ( exchange.contentType !== null && isJsonLike(mediaTypeEssence(exchange.contentType)) ) { - const parsed = safeParseJson(text); - const value = parsed.ok - ? jsonValueSchema.safeParse(parsed.value) - : undefined; + const parsed = parseExactJson(text); + const value = + parsed === undefined ? undefined : jsonValueSchema.safeParse(parsed); if (value?.success === true) return { body: value.data, truncated: false }; } return { body: text, truncated: false }; }; +/** Parsed JSON only when every integer survives exactly; one outside the safe range keeps the recorded text. */ +const parseExactJson = (text: string): unknown => { + let lossy = false; + try { + const value: unknown = JSON.parse( + text, + (_key, item: unknown, context?: { readonly source?: string }) => { + const source = context?.source; + if ( + typeof item === "number" && + Number.isInteger(item) && + !Number.isSafeInteger(item) && + source !== undefined && + /^-?\d+$/u.test(source) && + BigInt(source) !== BigInt(item) + ) + lossy = true; + return item; + }, + ); + return lossy ? undefined : value; + } catch { + return undefined; + } +}; + /** Replay one tool call from the retained record that best matches its arguments. */ export const replayEnvironmentTool = ( source: EnvironmentSource, diff --git a/src/application/environment/environmentTools.test.ts b/src/application/environment/environmentTools.test.ts index 1ef60f35f7..284c253b0d 100644 --- a/src/application/environment/environmentTools.test.ts +++ b/src/application/environment/environmentTools.test.ts @@ -131,6 +131,23 @@ describe("environment catalog description budgets", () => { ); }); + it("cuts a long captured path in the tool description and keeps it whole in the manifest", () => { + const users = tool("get_users_by_id"); + const long = { + ...users, + path_template: `/api/${"segment/".repeat(200_000)}{id}`, + }; + const [definition] = environmentToolDefinitions( + { ...source.manifest, tools: [long] }, + source.evidenceId, + ); + expect(Buffer.byteLength(definition?.description ?? "")).toBeLessThan(2048); + expect(definition?.description).toContain( + "… (full path in manifest Evidence)", + ); + expect(definition?.tool.path_template).toBe(long.path_template); + }); + it("reports an omitted oversized value instead of publishing an unbounded or partial observation", () => { const users = tool("get_users_by_id"); const schema = environmentToolInputSchema({ diff --git a/src/application/environment/environmentTools.ts b/src/application/environment/environmentTools.ts index 58cc6e6981..d0ef287d76 100644 --- a/src/application/environment/environmentTools.ts +++ b/src/application/environment/environmentTools.ts @@ -61,6 +61,18 @@ const describeList = ( : `${listed}${sample.length === 0 ? "" : ", "}… (${String(sample.length)}/${String(values.length)} shown; full list in manifest Evidence)`; }; +/** The first DESCRIPTION_LIST_SAMPLE_BYTES of a captured path, cut on a character boundary, with a pointer to the rest. */ +const describePath = (path: string): string => { + // encodeInto stops before the first character that does not fit, so `read` ends on a boundary. + const { read } = new TextEncoder().encodeInto( + path, + new Uint8Array(DESCRIPTION_LIST_SAMPLE_BYTES), + ); + return read === path.length + ? path + : `${path.slice(0, read)}… (full path in manifest Evidence)`; +}; + const bodyDescription = (tool: EnvironmentManifestTool): string => tool.body_shape === null ? "Request body; the capture retained no JSON body shape for this tool." @@ -145,7 +157,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} ${tool.method} ${describePath(tool.path_template)}; 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..934d720fe2 100644 --- a/src/cli/environmentCommands.ts +++ b/src/cli/environmentCommands.ts @@ -14,7 +14,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"; @@ -71,8 +74,16 @@ interface ServeArguments { readonly port?: number | undefined; } +/** A shutdown requested while the files load ends startup before the listener binds. */ +const startupCancelled = (signal: AbortSignal): JsonValue => + cliError( + new AnalysisCancelledError(ENVIRONMENT_SERVE_OPERATION, { + cause: signal.reason, + }), + ); + /** Load both Evidence files, bind the listener and run until a signal ends it. */ -const serveEnvironmentCommand = async ( +export const serveEnvironmentCommand = async ( input: ServeArguments, environment: Readonly, logger: Logger, @@ -90,11 +101,13 @@ const serveEnvironmentCommand = async ( ENVIRONMENT_SERVE_OPERATION, ); if (!manifest.ok) return manifest.error; + if (signal.aborted) return startupCancelled(signal); const evidence = await parseCliJsonInput( input.evidence, ENVIRONMENT_SERVE_OPERATION, ); if (!evidence.ok) return evidence.error; + if (signal.aborted) return startupCancelled(signal); const source = loadEnvironmentSource(manifest.value, evidence.value); if (!source.ok) return cliError(source.error); // Loaded here so registering the command catalog never loads the HTTP listener. @@ -102,6 +115,7 @@ const serveEnvironmentCommand = async ( await import("../server/environment/serveEnvironment.js"); const { StreamableHttpListenError } = await import("../server/http/streamableHttpListenError.js"); + if (signal.aborted) return startupCancelled(signal); // 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/tests/boundary/cli/environmentServeCancellation.test.ts b/tests/boundary/cli/environmentServeCancellation.test.ts new file mode 100644 index 0000000000..b667064556 --- /dev/null +++ b/tests/boundary/cli/environmentServeCancellation.test.ts @@ -0,0 +1,40 @@ +import { EventEmitter } from "node:events"; + +import pino from "pino"; +import { expect, it } from "vitest"; + +import { withCommandCancellation } from "../../../src/cli/commandCancellation.js"; +import { serveEnvironmentCommand } from "../../../src/cli/environmentCommands.js"; +import { writeEnvironmentServeFiles } from "../../fixtures/environmentServe.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; + +it("ends environment-serve startup as cancelled, before binding, when a shutdown signal arrives while loading", async () => { + const root = await createTestTempDirectory("rea-environment-serve-cancel-"); + const files = await writeEnvironmentServeFiles(root); + const events = new EventEmitter(); + const host: { + on: EventEmitter["on"]; + off: EventEmitter["off"]; + exitCode?: number | string | null | undefined; + } = { on: events.on.bind(events), off: events.off.bind(events) }; + const command = withCommandCancellation( + (signal) => + serveEnvironmentCommand( + { + manifest: files.manifestPath, + evidence: files.capturePath, + host: "127.0.0.1", + port: 0, + }, + {}, + pino({ level: "silent" }), + signal, + ), + host, + ); + events.emit("SIGTERM"); + const result = await command; + expect(result).toMatchObject({ code: "cancelled" }); + expect(result).not.toHaveProperty("status", "stopped"); + expect(host.exitCode).toBe(143); +});