feat: allow complex JSON values in request _meta
Closes #1910 `_meta` was typed `Record<string, string>` end to end in the Inspector — on disk as a `{ key, value }[]` pair array, in the Server Settings form as key/value rows, and in every request verb's per-call metadata parameter. Nothing in the MCP spec restricts a `_meta` value to a string, and the SDK models the field as a `z.looseObject`, so the restriction was ours alone. Widen the whole path to `RequestMetadata` (`Record<string, JsonValue>`): - Per-server default metadata is a JSON **object** on disk and in memory. The pre-#1910 pair array is still read (`normalizeStoredMetadata`) so an existing `mcp.json` keeps working; it is never written back, so a save migrates the entry. - The Server Settings → Request Metadata section is a JSON editor (`JsonObjectInput`) instead of key/value rows — rows cannot express an object value. It keeps the typed text as the display source and emits only parsed JSON, the same draft split `SchemaJsonField` uses, so the box stays typeable; invalid text is flagged inline and not applied. - The CLI stops flattening `--metadata` / `--tool-metadata` through `metaValueToString`. Those values were already JSON-parsed, so `trace={"id":"abc"}` used to reach the wire as the *string* `{"id":"abc"}` and now reaches it as an object. - `servers/show` redaction replaces a sensitive key's whole value rather than a string, so a structured value cannot leak through its members. - `progressTokenOf` narrows the reserved `progressToken` key back to `string | number` before it reaches the SDK; any other JSON still travels inside `_meta` but wires up no progress callback. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall
parent
6829b45f60
commit
b33532c024
61 changed files with 1004 additions and 495 deletions
|
|
@ -110,15 +110,15 @@ Options that specify the MCP server (catalog/config file, ad-hoc command/URL, en
|
|||
| `--prompt-name <name>` | Prompt name (for `prompts/get`). |
|
||||
| `--prompt-args <key=value>` | Prompt arguments; repeat for multiple. |
|
||||
| `--log-level <level>` | Logging level for `logging/setLevel` (e.g. `debug`, `info`). |
|
||||
| `--metadata <key=value>` | General metadata (key=value); applied to all methods. |
|
||||
| `--tool-metadata <key=value>` | Tool-specific metadata for `tools/call`. |
|
||||
| `--metadata <key=value>` | General `_meta` entries (key=value); applied to all methods. The value is JSON-parsed when it parses, so `trace={"id":"abc"}` sends a real object and `n=3` a number; anything that is not valid JSON is sent as the literal string. |
|
||||
| `--tool-metadata <key=value>` | Tool-specific `_meta` entries for `tools/call`. Same JSON-parsed value handling as `--metadata`. |
|
||||
| `--connect-timeout <ms>` | Connection timeout in ms. Defaults to `15000` for ad-hoc `--server-url`/target runs (so a black-holed host fails fast) and to the file-level timeout for `--catalog`/`--config` runs. `0` disables the timeout. |
|
||||
| `--app-info` | Probe a tool's MCP App UI metadata without invoking it. With `--method tools/call --tool-name <name>`: prints one JSON line (`hasApp`, `resourceUri`, `csp`, `permissions`, `domain`, …) and exits `0` if the tool has an app or `2` (`no_app`) if not. With `--method tools/list`: emits NDJSON — one app-info line per tool over a single connection. |
|
||||
| `--format <text\|json>` | Output format. `text` (default) pretty-prints the result. `json` emits a single JSON object on stdout (`{ "result": … }`, plus `{ "appInfo": … }` as a sibling key for App tools) with no banners, so the whole output pipes cleanly into `jq`. |
|
||||
| `--relogin` | Delete stored OAuth for this server URL from the shared store before connect; interactive login still only runs if the server requires auth. Requires an HTTP/SSE URL (rejected for stdio). Conflicts with `--stored-auth-only` / `--use-stored-auth` / `--wait-for-auth` / catalog short-circuits. |
|
||||
| `--stored-auth-only` | **CI / non-interactive safe:** never start interactive OAuth / step-up (and never auto-open a browser); use the shared store if present, otherwise fail immediately with `auth_required`. Prefer this over a bare pipe/CI run that would otherwise attempt interactive login. |
|
||||
|
||||
`servers/show` redacts secret-bearing fields (`env` values, sensitive headers / `settings.metadata` keys, `requestInit` / `eventSourceInit` headers, `oauthClientSecret`). It does **not** scrub credentials embedded in a server `url` (userinfo or query tokens) or in stdio `args` — treat `detail` / raw URL fields as potentially sensitive before pasting into issues.
|
||||
`servers/show` redacts secret-bearing fields (`env` values, sensitive headers, sensitive `settings.metadata` keys whose whole value is replaced whether or not it is structured, `requestInit` / `eventSourceInit` headers, `oauthClientSecret`). It does **not** scrub credentials embedded in a server `url` (userinfo or query tokens) or in stdio `args` — treat `detail` / raw URL fields as potentially sensitive before pasting into issues.
|
||||
|
||||
#### App probing (`--app-info`) and machine-readable output (`--format json`)
|
||||
|
||||
|
|
|
|||
|
|
@ -63,7 +63,7 @@ export function makeFakeServerSettings(
|
|||
): InspectorServerSettings {
|
||||
return {
|
||||
headers: [],
|
||||
metadata: [],
|
||||
metadata: {},
|
||||
env: [],
|
||||
connectionTimeout: 0,
|
||||
requestTimeout: 0,
|
||||
|
|
|
|||
|
|
@ -373,16 +373,17 @@ describe("Metadata Tests", () => {
|
|||
|
||||
expectCliSuccess(result);
|
||||
|
||||
// Validate metadata values are sent as strings
|
||||
// A JSON-parseable value goes out as that JSON type, not as its
|
||||
// string spelling (#1910) — `_meta` takes any JSON.
|
||||
const recordedRequests = server.getRecordedRequests();
|
||||
const toolsListRequest = recordedRequests.find(
|
||||
(r) => r.method === "tools/list",
|
||||
);
|
||||
expect(toolsListRequest).toBeDefined();
|
||||
expect(toolsListRequest?.metadata).toEqual({
|
||||
integer_value: "42",
|
||||
decimal_value: "3.14159",
|
||||
negative_value: "-10",
|
||||
integer_value: 42,
|
||||
decimal_value: 3.14159,
|
||||
negative_value: -10,
|
||||
});
|
||||
} finally {
|
||||
await server.stop();
|
||||
|
|
@ -428,7 +429,7 @@ describe("Metadata Tests", () => {
|
|||
}
|
||||
});
|
||||
|
||||
it("JSON.stringifies object/array metadata (not String → [object Object])", async () => {
|
||||
it("sends object/array/boolean metadata as real JSON, not stringified (#1910)", async () => {
|
||||
const server = createTestServerHttp({
|
||||
serverInfo: createTestServerInfo(),
|
||||
tools: [createEchoTool()],
|
||||
|
|
@ -456,9 +457,9 @@ describe("Metadata Tests", () => {
|
|||
(r) => r.method === "tools/list",
|
||||
);
|
||||
expect(toolsListRequest?.metadata).toEqual({
|
||||
nested: '{"key":"value"}',
|
||||
list: "[1,2,3]",
|
||||
flag: "true",
|
||||
nested: { key: "value" },
|
||||
list: [1, 2, 3],
|
||||
flag: true,
|
||||
});
|
||||
} finally {
|
||||
await server.stop();
|
||||
|
|
@ -819,7 +820,7 @@ describe("Metadata Tests", () => {
|
|||
);
|
||||
expect(toolsListRequest).toBeDefined();
|
||||
expect(toolsListRequest?.metadata).toEqual({
|
||||
integration_test: "true",
|
||||
integration_test: true,
|
||||
test_phase: "all_methods",
|
||||
});
|
||||
} finally {
|
||||
|
|
@ -866,8 +867,8 @@ describe("Metadata Tests", () => {
|
|||
);
|
||||
expect(toolCallRequest).toBeDefined();
|
||||
expect(toolCallRequest?.metadata).toEqual({
|
||||
session_id: "12345",
|
||||
user_id: "67890",
|
||||
session_id: 12345,
|
||||
user_id: 67890,
|
||||
timestamp: "2024-01-01T00:00:00Z",
|
||||
request_id: "req-abc-123",
|
||||
tool_session: "session-xyz-789",
|
||||
|
|
@ -919,8 +920,8 @@ describe("Metadata Tests", () => {
|
|||
expect(toolCallRequest).toBeDefined();
|
||||
expect(toolCallRequest?.metadata).toEqual({
|
||||
valid_key: "valid_value",
|
||||
numeric_key: "123",
|
||||
boolean_key: "true",
|
||||
numeric_key: 123,
|
||||
boolean_key: true,
|
||||
json_key: '\'{"test":"value"}\'', // Single quotes are preserved
|
||||
special_key: "!@#$%^&*()",
|
||||
unicode_key: "🚀🎉✨",
|
||||
|
|
|
|||
|
|
@ -1,7 +1,6 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import {
|
||||
isOneShotMethod,
|
||||
metaValueToString,
|
||||
ONE_SHOT_METHODS,
|
||||
SESSION_RPC_METHODS,
|
||||
} from "../src/handlers/method-types.js";
|
||||
|
|
@ -26,11 +25,3 @@ describe("ONE_SHOT_METHODS", () => {
|
|||
expect(ONE_SHOT_METHODS).not.toContain("logging/tail");
|
||||
});
|
||||
});
|
||||
|
||||
describe("metaValueToString", () => {
|
||||
it("passes strings through and JSON-encodes structured values", () => {
|
||||
expect(metaValueToString("plain")).toBe("plain");
|
||||
expect(metaValueToString({ a: 1 })).toBe('{"a":1}');
|
||||
expect(metaValueToString([1, 2])).toBe("[1,2]");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@ import {
|
|||
describe("withConnectTimeout", () => {
|
||||
const baseSettings: InspectorServerSettings = {
|
||||
headers: [{ key: "X-Test", value: "1" }],
|
||||
metadata: [],
|
||||
metadata: {},
|
||||
env: [],
|
||||
connectionTimeout: 42,
|
||||
requestTimeout: 0,
|
||||
|
|
|
|||
|
|
@ -219,10 +219,14 @@ describe("showServerEntry / servers/show", () => {
|
|||
{ key: "Authorization", value: "Bearer x" },
|
||||
{ key: "X-Custom", value: "ok" },
|
||||
],
|
||||
metadata: [
|
||||
{ key: "Authorization", value: "Bearer meta" },
|
||||
{ key: "X-Custom", value: "ok" },
|
||||
],
|
||||
metadata: {
|
||||
Authorization: "Bearer meta",
|
||||
"X-Custom": "ok",
|
||||
// A structured value under a sensitive key: the redaction must replace
|
||||
// the whole value, not walk into it (#1910).
|
||||
"X-Api-Key": { primary: "sk-live-1", fallback: "sk-live-2" },
|
||||
nested: { keep: true },
|
||||
},
|
||||
env: [
|
||||
{ key: "TOKEN", value: "secret" },
|
||||
{ key: "", value: "still-secret" },
|
||||
|
|
@ -242,10 +246,13 @@ describe("showServerEntry / servers/show", () => {
|
|||
{ key: "Authorization", value: "[redacted]" },
|
||||
{ key: "X-Custom", value: "ok" },
|
||||
]);
|
||||
expect(sanitized.metadata).toEqual([
|
||||
{ key: "Authorization", value: "[redacted]" },
|
||||
{ key: "X-Custom", value: "ok" },
|
||||
]);
|
||||
expect(sanitized.metadata).toEqual({
|
||||
Authorization: "[redacted]",
|
||||
"X-Custom": "ok",
|
||||
"X-Api-Key": "[redacted]",
|
||||
// A structured value under a non-sensitive key survives intact.
|
||||
nested: { keep: true },
|
||||
});
|
||||
expect(sanitized.env).toEqual([
|
||||
{ key: "TOKEN", value: "[redacted]" },
|
||||
{ key: "", value: "[redacted]" },
|
||||
|
|
|
|||
|
|
@ -33,7 +33,6 @@ import { consumeMethodOutcome } from "./handlers/consume-outcome.js";
|
|||
import { runMethod } from "./handlers/run-method.js";
|
||||
import {
|
||||
isOneShotMethod,
|
||||
metaValueToString,
|
||||
ONE_SHOT_METHODS,
|
||||
type MethodArgs,
|
||||
} from "./handlers/method-types.js";
|
||||
|
|
@ -999,22 +998,12 @@ async function parseArgs(argv?: string[]): Promise<ParseResult> {
|
|||
promptName: options.promptName,
|
||||
promptArgs: options.promptArgs,
|
||||
logLevel: options.logLevel,
|
||||
metadata: options.metadata
|
||||
? Object.fromEntries(
|
||||
Object.entries(options.metadata).map(([key, value]) => [
|
||||
key,
|
||||
metaValueToString(value),
|
||||
]),
|
||||
)
|
||||
: undefined,
|
||||
toolMeta: options.toolMetadata
|
||||
? Object.fromEntries(
|
||||
Object.entries(options.toolMetadata).map(([key, value]) => [
|
||||
key,
|
||||
metaValueToString(value),
|
||||
]),
|
||||
)
|
||||
: undefined,
|
||||
// `--metadata`/`--tool-metadata` values are parsed as JSON, and `_meta`
|
||||
// takes any JSON — so they go through unflattened (#1910). They used to be
|
||||
// squeezed through `metaValueToString`, which sent `{"a":1}` as the
|
||||
// *string* `'{"a":1}'`.
|
||||
metadata: options.metadata,
|
||||
toolMeta: options.toolMetadata,
|
||||
appInfo: options.appInfo === true,
|
||||
format: options.format,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import { InspectorClient } from "@inspector/core/mcp/index.js";
|
|||
import { extractAppInfo } from "@inspector/core/mcp/apps.js";
|
||||
import type { AppInfo } from "@inspector/core/mcp/apps.js";
|
||||
import type { CliAppInfo } from "./method-types.js";
|
||||
import type { RequestMetadata } from "@inspector/core/mcp/types.js";
|
||||
|
||||
/**
|
||||
* Build the CLI's app-info for a tool. Never throws — failures fold into
|
||||
|
|
@ -10,7 +11,7 @@ import type { CliAppInfo } from "./method-types.js";
|
|||
export async function collectAppInfo(
|
||||
client: Pick<InspectorClient, "readResource">,
|
||||
tool: Parameters<typeof extractAppInfo>[0],
|
||||
metadata: Record<string, string> | undefined,
|
||||
metadata: RequestMetadata | undefined,
|
||||
): Promise<CliAppInfo> {
|
||||
let base: AppInfo;
|
||||
try {
|
||||
|
|
|
|||
|
|
@ -22,7 +22,7 @@ export function withConnectTimeout(
|
|||
if (settings) return { ...settings, connectionTimeout };
|
||||
return {
|
||||
headers: [],
|
||||
metadata: [],
|
||||
metadata: {},
|
||||
env: [],
|
||||
connectionTimeout,
|
||||
requestTimeout: 0,
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import type { JsonValue } from "@inspector/core/mcp/index.js";
|
||||
import type { RequestMetadata } from "@inspector/core/mcp/types.js";
|
||||
import type { AppInfo } from "@inspector/core/mcp/apps.js";
|
||||
import type { LoggingLevel } from "@modelcontextprotocol/client";
|
||||
import type { OutputFormat } from "./format-output.js";
|
||||
|
|
@ -20,8 +21,8 @@ export type MethodArgs = {
|
|||
logLevel?: LoggingLevel;
|
||||
toolName?: string;
|
||||
toolArg?: Record<string, JsonValue>;
|
||||
toolMeta?: Record<string, string>;
|
||||
metadata?: Record<string, string>;
|
||||
toolMeta?: RequestMetadata;
|
||||
metadata?: RequestMetadata;
|
||||
appInfo?: boolean;
|
||||
format?: OutputFormat;
|
||||
/** Task id for tasks/get, tasks/cancel, tasks/result. */
|
||||
|
|
@ -110,13 +111,3 @@ export type OneShotMethod = (typeof ONE_SHOT_METHODS)[number];
|
|||
export function isOneShotMethod(method: string): method is OneShotMethod {
|
||||
return (ONE_SHOT_METHODS as readonly string[]).includes(method);
|
||||
}
|
||||
|
||||
/**
|
||||
* Encode a metadata / tool-metadata value for MethodArgs (string map).
|
||||
* Strings pass through; objects/arrays use JSON.stringify so structure is kept
|
||||
* (avoids String(obj) → "[object Object]").
|
||||
*/
|
||||
export function metaValueToString(value: JsonValue): string {
|
||||
if (typeof value === "string") return value;
|
||||
return JSON.stringify(value);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -157,10 +157,15 @@ export function sanitizeServerSettings(
|
|||
key: h.key,
|
||||
value: isSensitiveHeader(h.key) ? REDACTED : h.value,
|
||||
})),
|
||||
metadata: (settings.metadata ?? []).map((m) => ({
|
||||
key: m.key,
|
||||
value: isSensitiveHeader(m.key) ? REDACTED : m.value,
|
||||
})),
|
||||
// Metadata is a JSON object whose values may be any JSON (#1910), so the
|
||||
// redaction replaces the whole value rather than a string — a sensitive
|
||||
// key holding an object must not leak through its members.
|
||||
metadata: Object.fromEntries(
|
||||
Object.entries(settings.metadata ?? {}).map(([key, value]) => [
|
||||
key,
|
||||
isSensitiveHeader(key) ? REDACTED : value,
|
||||
]),
|
||||
),
|
||||
env: (settings.env ?? []).map((e) => ({
|
||||
key: e.key,
|
||||
value: REDACTED,
|
||||
|
|
|
|||
|
|
@ -340,7 +340,7 @@ function oneEmaHttp(): Record<string, TuiServer> {
|
|||
config: { type: "streamable-http", url: "http://localhost:8080/mcp" },
|
||||
settings: {
|
||||
requestTimeout: 0,
|
||||
metadata: [],
|
||||
metadata: {},
|
||||
headers: [],
|
||||
env: [],
|
||||
roots: [],
|
||||
|
|
|
|||
|
|
@ -19,7 +19,7 @@ export function makeFakeServerSettings(
|
|||
): InspectorServerSettings {
|
||||
return {
|
||||
headers: [],
|
||||
metadata: [],
|
||||
metadata: {},
|
||||
env: [],
|
||||
connectionTimeout: 0,
|
||||
requestTimeout: 0,
|
||||
|
|
|
|||
|
|
@ -291,13 +291,9 @@ function App({
|
|||
transport: createTransportNode,
|
||||
logger: getTuiLogger(),
|
||||
};
|
||||
const defaultMetadata = savedSettings?.metadata
|
||||
? Object.fromEntries(
|
||||
savedSettings.metadata
|
||||
.filter((m) => m.key.trim() !== "")
|
||||
.map((m) => [m.key, m.value]),
|
||||
)
|
||||
: undefined;
|
||||
// Per-server default `_meta` is already a JSON object (#1910) — no
|
||||
// pair-array flattening left to do; `{}` means "no defaults".
|
||||
const defaultMetadata = savedSettings?.metadata;
|
||||
const clientAuthOptions = buildRunnerClientAuthOptions(
|
||||
clientConfig,
|
||||
savedSettings,
|
||||
|
|
|
|||
|
|
@ -1635,7 +1635,7 @@ const settingsWithRoots = (
|
|||
): InspectorServerSettings => ({
|
||||
headers: [],
|
||||
env: [],
|
||||
metadata: [],
|
||||
metadata: {},
|
||||
connectionTimeout: 0,
|
||||
requestTimeout: 0,
|
||||
taskTtl: 60000,
|
||||
|
|
|
|||
|
|
@ -384,7 +384,7 @@ async function replayProtocolRequest(
|
|||
const EMPTY_SETTINGS: InspectorServerSettings = {
|
||||
headers: [],
|
||||
env: [],
|
||||
metadata: [],
|
||||
metadata: {},
|
||||
connectionTimeout: 0,
|
||||
requestTimeout: 0,
|
||||
taskTtl: DEFAULT_TASK_TTL_MS,
|
||||
|
|
@ -2466,13 +2466,9 @@ function App() {
|
|||
const activeCimdUrl = getActiveCimdClientMetadataUrl(clientConfig);
|
||||
// Flatten the persisted settings into the InspectorClient options shape.
|
||||
// Empty / zero values stay unset so the SDK defaults apply.
|
||||
const defaultMetadata = savedSettings?.metadata
|
||||
? Object.fromEntries(
|
||||
savedSettings.metadata
|
||||
.filter((m) => m.key.trim() !== "")
|
||||
.map((m) => [m.key, m.value]),
|
||||
)
|
||||
: undefined;
|
||||
// Per-server default `_meta` is already a JSON object (#1910) — no
|
||||
// pair-array flattening left to do; `{}` means "no defaults".
|
||||
const defaultMetadata = savedSettings?.metadata;
|
||||
const serverAuthorizationParams = savedSettings
|
||||
? oauthAuthorizationParamsFromSettings(savedSettings)
|
||||
: undefined;
|
||||
|
|
|
|||
|
|
@ -0,0 +1,92 @@
|
|||
import { useState } from "react";
|
||||
import type { Meta, StoryObj } from "@storybook/react-vite";
|
||||
import { expect, userEvent, within } from "storybook/test";
|
||||
import type { JsonObject } from "@inspector/core/json/jsonUtils.js";
|
||||
import { JsonObjectInput } from "./JsonObjectInput";
|
||||
|
||||
/**
|
||||
* The component is controlled, so every story drives it through a small
|
||||
* stateful wrapper — otherwise the box would not accept a keystroke and the
|
||||
* play functions could not exercise the draft behavior at all.
|
||||
*/
|
||||
function ControlledJsonObjectInput({ initial }: { initial: JsonObject }) {
|
||||
const [value, setValue] = useState<JsonObject>(initial);
|
||||
return (
|
||||
<JsonObjectInput
|
||||
label="Request metadata"
|
||||
aria-label="Request metadata JSON"
|
||||
value={value}
|
||||
onChange={setValue}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
const meta: Meta<typeof ControlledJsonObjectInput> = {
|
||||
title: "Elements/JsonObjectInput",
|
||||
component: ControlledJsonObjectInput,
|
||||
};
|
||||
|
||||
export default meta;
|
||||
type Story = StoryObj<typeof ControlledJsonObjectInput>;
|
||||
|
||||
/** A structured payload renders as formatted JSON, nesting intact. */
|
||||
export const Populated: Story = {
|
||||
args: {
|
||||
initial: {
|
||||
tenant: "acme",
|
||||
trace: { id: "abc123", sampled: true },
|
||||
features: ["apps", "tasks"],
|
||||
},
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
const box = await canvas.findByLabelText("Request metadata JSON");
|
||||
await expect(JSON.parse((box as HTMLTextAreaElement).value)).toEqual({
|
||||
tenant: "acme",
|
||||
trace: { id: "abc123", sampled: true },
|
||||
features: ["apps", "tasks"],
|
||||
});
|
||||
},
|
||||
};
|
||||
|
||||
/** An empty payload opens as `{}` and carries no error. */
|
||||
export const Empty: Story = {
|
||||
args: { initial: {} },
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
const box = await canvas.findByLabelText("Request metadata JSON");
|
||||
await expect(box).toHaveValue("{}");
|
||||
await expect(canvas.queryByText(/Not valid JSON/)).toBeNull();
|
||||
},
|
||||
};
|
||||
|
||||
/**
|
||||
* Half-typed text is displayed as typed and flagged, rather than being
|
||||
* re-escaped back into the box each keystroke (the #1928 failure this draft
|
||||
* split exists to avoid).
|
||||
*/
|
||||
export const InvalidDraft: Story = {
|
||||
args: { initial: {} },
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
const box = await canvas.findByLabelText("Request metadata JSON");
|
||||
await userEvent.clear(box);
|
||||
await userEvent.type(box, '{{"a":');
|
||||
await expect(box).toHaveValue('{"a":');
|
||||
await expect(await canvas.findByText(/Not valid JSON/)).toBeVisible();
|
||||
},
|
||||
};
|
||||
|
||||
/** Valid JSON that is not an object is rejected with its own message. */
|
||||
export const NotAnObject: Story = {
|
||||
args: { initial: {} },
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
const box = await canvas.findByLabelText("Request metadata JSON");
|
||||
await userEvent.clear(box);
|
||||
await userEvent.type(box, "[[1,2]");
|
||||
await expect(
|
||||
await canvas.findByText(/Must be a JSON object/),
|
||||
).toBeVisible();
|
||||
},
|
||||
};
|
||||
|
|
@ -0,0 +1,172 @@
|
|||
import { useState } from "react";
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import { screen } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import type { JsonObject } from "@inspector/core/json/jsonUtils.js";
|
||||
import { renderWithMantine } from "../../../test/renderWithMantine";
|
||||
import { JsonObjectInput, parseObjectDraft } from "./JsonObjectInput";
|
||||
|
||||
const LABEL = "Payload JSON";
|
||||
|
||||
/** Controlled harness: the component never owns the value, only the draft. */
|
||||
function Harness({
|
||||
initial,
|
||||
onChange,
|
||||
}: {
|
||||
initial: JsonObject;
|
||||
onChange?: (v: JsonObject) => void;
|
||||
}) {
|
||||
const [value, setValue] = useState<JsonObject>(initial);
|
||||
return (
|
||||
<>
|
||||
<JsonObjectInput
|
||||
aria-label={LABEL}
|
||||
value={value}
|
||||
onChange={(next) => {
|
||||
setValue(next);
|
||||
onChange?.(next);
|
||||
}}
|
||||
/>
|
||||
<output data-testid="emitted">{JSON.stringify(value)}</output>
|
||||
</>
|
||||
);
|
||||
}
|
||||
|
||||
function box() {
|
||||
return screen.getByLabelText(LABEL) as HTMLTextAreaElement;
|
||||
}
|
||||
|
||||
describe("parseObjectDraft", () => {
|
||||
it("reads empty text as the empty object rather than an error", () => {
|
||||
expect(parseObjectDraft("")).toEqual({ ok: true, value: {} });
|
||||
expect(parseObjectDraft(" \n ")).toEqual({ ok: true, value: {} });
|
||||
});
|
||||
|
||||
it("accepts a JSON object with values of any JSON type", () => {
|
||||
const result = parseObjectDraft(
|
||||
'{"s":"a","n":1,"b":true,"z":null,"arr":[1,{"k":2}],"o":{"deep":{"er":1}}}',
|
||||
);
|
||||
expect(result).toEqual({
|
||||
ok: true,
|
||||
value: {
|
||||
s: "a",
|
||||
n: 1,
|
||||
b: true,
|
||||
z: null,
|
||||
arr: [1, { k: 2 }],
|
||||
o: { deep: { er: 1 } },
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects text that is not JSON", () => {
|
||||
expect(parseObjectDraft('{"a":')).toEqual({
|
||||
ok: false,
|
||||
error: "Not valid JSON — changes are not applied",
|
||||
});
|
||||
});
|
||||
|
||||
it.each([
|
||||
["an array", "[1,2]"],
|
||||
["a string", '"hello"'],
|
||||
["a number", "42"],
|
||||
["null", "null"],
|
||||
])("rejects valid JSON that is not an object: %s", (_label, text) => {
|
||||
const result = parseObjectDraft(text);
|
||||
expect(result.ok).toBe(false);
|
||||
expect(result.ok === false && result.error).toMatch(
|
||||
/Must be a JSON object/,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("JsonObjectInput", () => {
|
||||
it("opens with the value serialized two-space, nesting intact", () => {
|
||||
renderWithMantine(<Harness initial={{ a: { b: [1, 2] } }} />);
|
||||
expect(box().value).toBe(
|
||||
'{\n "a": {\n "b": [\n 1,\n 2\n ]\n }\n}',
|
||||
);
|
||||
});
|
||||
|
||||
it("emits the parsed object — a structured value stays structured", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
renderWithMantine(<Harness initial={{}} onChange={onChange} />);
|
||||
await user.clear(box());
|
||||
await user.type(box(), '{{"trace":{{"id":"x"},"n":3}');
|
||||
expect(onChange).toHaveBeenLastCalledWith({ trace: { id: "x" }, n: 3 });
|
||||
});
|
||||
|
||||
it("shows the text as typed while it is invalid and does not emit", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
renderWithMantine(<Harness initial={{ keep: "me" }} onChange={onChange} />);
|
||||
// Append a stray character rather than clearing: clearing is itself a
|
||||
// valid edit (it means `{}`) and would emit, which is not what is under
|
||||
// test here.
|
||||
await user.click(box());
|
||||
await user.keyboard("{End}x");
|
||||
// Displayed verbatim — not re-escaped back through JSON.stringify.
|
||||
expect(box().value.endsWith("}x")).toBe(true);
|
||||
expect(
|
||||
screen.getByText("Not valid JSON — changes are not applied"),
|
||||
).toBeInTheDocument();
|
||||
// Nothing emitted, so the caller still holds the last valid object.
|
||||
expect(onChange).not.toHaveBeenCalled();
|
||||
expect(screen.getByTestId("emitted").textContent).toBe('{"keep":"me"}');
|
||||
});
|
||||
|
||||
it("flags valid JSON that is not an object without emitting", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
renderWithMantine(<Harness initial={{ keep: "me" }} onChange={onChange} />);
|
||||
await user.clear(box());
|
||||
// The clear is a legitimate edit to `{}`; only what follows is under test.
|
||||
expect(onChange).toHaveBeenLastCalledWith({});
|
||||
onChange.mockClear();
|
||||
await user.type(box(), "[[1]");
|
||||
expect(screen.getByText(/Must be a JSON object/)).toBeInTheDocument();
|
||||
expect(onChange).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("treats a cleared box as the empty object", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
renderWithMantine(<Harness initial={{ a: 1 }} onChange={onChange} />);
|
||||
await user.clear(box());
|
||||
expect(onChange).toHaveBeenLastCalledWith({});
|
||||
expect(screen.queryByText(/Not valid JSON/)).toBeNull();
|
||||
});
|
||||
|
||||
it("re-syncs when the parent's value diverges from the draft", async () => {
|
||||
// An external reset (switching servers, a reloaded catalog) must reach the
|
||||
// box; an in-progress edit must not be overwritten by the value it emitted.
|
||||
function Externally() {
|
||||
const [value, setValue] = useState<JsonObject>({ a: 1 });
|
||||
return (
|
||||
<>
|
||||
<JsonObjectInput
|
||||
aria-label={LABEL}
|
||||
value={value}
|
||||
onChange={setValue}
|
||||
/>
|
||||
<button onClick={() => setValue({ b: 2 })}>reset</button>
|
||||
</>
|
||||
);
|
||||
}
|
||||
const user = userEvent.setup();
|
||||
renderWithMantine(<Externally />);
|
||||
await user.click(screen.getByRole("button", { name: "reset" }));
|
||||
expect(JSON.parse(box().value)).toEqual({ b: 2 });
|
||||
});
|
||||
|
||||
it("leaves an in-progress edit alone when the parent echoes it back", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithMantine(<Harness initial={{}} />);
|
||||
await user.clear(box());
|
||||
// Typed without the closing brace auto-pair: the text is what the user has
|
||||
// so far, and the echo of the parsed value must not reformat it mid-edit.
|
||||
await user.type(box(), '{{"a":1}');
|
||||
expect(box().value).toBe('{"a":1}');
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,113 @@
|
|||
import { useState } from "react";
|
||||
import { JsonInput } from "@mantine/core";
|
||||
import type { JsonObject } from "@inspector/core/json/jsonUtils.js";
|
||||
import { useValueChange } from "../../../hooks/useValueChange";
|
||||
|
||||
const ObjectJsonInput = JsonInput.withProps({
|
||||
formatOnBlur: true,
|
||||
autosize: true,
|
||||
minRows: 4,
|
||||
spellCheck: false,
|
||||
});
|
||||
|
||||
/** Two-space canonical form — the same shape the catalog file is written in. */
|
||||
function serialize(value: JsonObject): string {
|
||||
return JSON.stringify(value, null, 2);
|
||||
}
|
||||
|
||||
/** A plain JSON object, i.e. not an array and not a scalar. */
|
||||
function isJsonObject(value: unknown): value is JsonObject {
|
||||
return value !== null && typeof value === "object" && !Array.isArray(value);
|
||||
}
|
||||
|
||||
/**
|
||||
* What a draft's text currently means: the object to emit, or why it can't be.
|
||||
*
|
||||
* Empty text is the object `{}` rather than an error — clearing the box is how
|
||||
* "send no metadata" is spelled, and an empty editor showing a red error would
|
||||
* read as broken.
|
||||
*/
|
||||
type DraftState =
|
||||
| { ok: true; value: JsonObject }
|
||||
| { ok: false; error: string };
|
||||
|
||||
export function parseObjectDraft(text: string): DraftState {
|
||||
if (text.trim() === "") return { ok: true, value: {} };
|
||||
let parsed: unknown;
|
||||
try {
|
||||
parsed = JSON.parse(text);
|
||||
} catch {
|
||||
return { ok: false, error: "Not valid JSON — changes are not applied" };
|
||||
}
|
||||
if (!isJsonObject(parsed)) {
|
||||
return {
|
||||
ok: false,
|
||||
error: "Must be a JSON object (`{ … }`) — changes are not applied",
|
||||
};
|
||||
}
|
||||
return { ok: true, value: parsed };
|
||||
}
|
||||
|
||||
export interface JsonObjectInputProps {
|
||||
value: JsonObject;
|
||||
onChange: (value: JsonObject) => void;
|
||||
label?: string;
|
||||
description?: string;
|
||||
placeholder?: string;
|
||||
"aria-label"?: string;
|
||||
"data-testid"?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* A JSON-object editor: the user types JSON, the parent receives the parsed
|
||||
* object.
|
||||
*
|
||||
* This is the editor for payloads whose *values* may be any JSON, which is why
|
||||
* it exists beside the key/value row editors used for headers and env. `_meta`
|
||||
* is the case that forced it (#1910) — nothing in the MCP spec restricts a
|
||||
* `_meta` value to a string, so a `{ key, value }` row editor cannot express
|
||||
* what the protocol allows.
|
||||
*
|
||||
* The text the user is typing is the source of truth for what is *displayed*;
|
||||
* the parent only ever sees a parsed object. That split is what makes the box
|
||||
* typeable: driving it off the parent's value and stringifying on every render
|
||||
* re-escapes half-typed text once per keystroke, which is the #1928 symptom.
|
||||
* `SchemaJsonField` in `SchemaForm.tsx` carries the same split for the same
|
||||
* reason — see the long comment there.
|
||||
*
|
||||
* While the text is invalid the parent is **not** told, so the last valid
|
||||
* object stands. The alternative — emitting `{}` — would silently discard a
|
||||
* user's configured metadata the moment they typed a stray character, and this
|
||||
* form has no Save button to gate instead: `onChange` writes straight through.
|
||||
* The state is not silent, though; the field says so on itself via `error`.
|
||||
*/
|
||||
export function JsonObjectInput({
|
||||
value,
|
||||
onChange,
|
||||
...inputProps
|
||||
}: JsonObjectInputProps) {
|
||||
const [draft, setDraft] = useState(() => serialize(value));
|
||||
|
||||
// Re-sync only when the parent's value genuinely diverges from what the draft
|
||||
// parses to, so an external reset (switching servers, a reloaded catalog)
|
||||
// reaches the box while an in-progress edit is left alone. Compared through
|
||||
// canonical JSON because the parent hands back a fresh object each time.
|
||||
const parsed = parseObjectDraft(draft);
|
||||
const draftJson = parsed.ok ? serialize(parsed.value) : null;
|
||||
useValueChange(serialize(value), (next) => {
|
||||
if (next !== draftJson) setDraft(next);
|
||||
});
|
||||
|
||||
return (
|
||||
<ObjectJsonInput
|
||||
{...inputProps}
|
||||
value={draft}
|
||||
error={parsed.ok ? undefined : parsed.error}
|
||||
onChange={(text) => {
|
||||
setDraft(text);
|
||||
const result = parseObjectDraft(text);
|
||||
if (result.ok) onChange(result.value);
|
||||
}}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
|
@ -10,7 +10,7 @@ import {
|
|||
const defaultSettings: InspectorServerSettings = {
|
||||
headers: [],
|
||||
env: [],
|
||||
metadata: [],
|
||||
metadata: {},
|
||||
connectionTimeout: 30000,
|
||||
requestTimeout: 60000,
|
||||
taskTtl: 60000,
|
||||
|
|
@ -57,34 +57,9 @@ function InteractiveRender(args: ServerSettingsFormProps) {
|
|||
},
|
||||
});
|
||||
}}
|
||||
onAddMetadata={() => {
|
||||
args.onAddMetadata();
|
||||
updateArgs({
|
||||
settings: {
|
||||
...args.settings,
|
||||
metadata: [...args.settings.metadata, { key: "", value: "" }],
|
||||
},
|
||||
});
|
||||
}}
|
||||
onRemoveMetadata={(index) => {
|
||||
args.onRemoveMetadata(index);
|
||||
updateArgs({
|
||||
settings: {
|
||||
...args.settings,
|
||||
metadata: args.settings.metadata.filter((_, i) => i !== index),
|
||||
},
|
||||
});
|
||||
}}
|
||||
onMetadataChange={(index, key, value) => {
|
||||
args.onMetadataChange(index, key, value);
|
||||
updateArgs({
|
||||
settings: {
|
||||
...args.settings,
|
||||
metadata: args.settings.metadata.map((m, i) =>
|
||||
i === index ? { key, value } : m,
|
||||
),
|
||||
},
|
||||
});
|
||||
onMetadataChange={(metadata) => {
|
||||
args.onMetadataChange(metadata);
|
||||
updateArgs({ settings: { ...args.settings, metadata } });
|
||||
}}
|
||||
onAddEnv={() => {
|
||||
args.onAddEnv();
|
||||
|
|
@ -197,8 +172,6 @@ const meta: Meta<typeof ServerSettingsForm> = {
|
|||
onRemoveEnv: fn(),
|
||||
onEnvChange: fn(),
|
||||
onCwdChange: fn(),
|
||||
onAddMetadata: fn(),
|
||||
onRemoveMetadata: fn(),
|
||||
onMetadataChange: fn(),
|
||||
onTimeoutChange: fn(),
|
||||
onAutoRefreshChange: fn(),
|
||||
|
|
@ -278,10 +251,13 @@ export const AllConfigured: Story = {
|
|||
{ key: "X-Request-Id", value: "req-456" },
|
||||
],
|
||||
env: [],
|
||||
metadata: [
|
||||
{ key: "userId", value: "user-789" },
|
||||
{ key: "sessionId", value: "session-012" },
|
||||
],
|
||||
metadata: {
|
||||
userId: "user-789",
|
||||
sessionId: "session-012",
|
||||
// A structured value — the case a `{ key, value }` row editor could
|
||||
// not express (#1910).
|
||||
client: { name: "inspector", features: ["apps", "tasks"], retries: 3 },
|
||||
},
|
||||
connectionTimeout: 15000,
|
||||
requestTimeout: 45000,
|
||||
taskTtl: 45000,
|
||||
|
|
|
|||
Some files were not shown because too many files have changed in this diff Show more
Loading…
Reference in a new issue