mirror of
https://github.com/morluto/rea.git
synced 2026-11-03 13:26:10 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
1 parent
ad52275d22
commit
130bc4326a
8 files changed
+150
-16
No files matched your search
@@ -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:
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -61,6 +61,18 @@ const describeList = <Value>(
|
||||
: `${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,
|
||||
}));
|
||||
|
||||
@@ -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<NodeJS.ProcessEnv>,
|
||||
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<Error>((resolve) => {
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
Reference in new issue
Block a user