Merge branch 'v2/main' into v2/fix/tool-row-keys
Both sides added a showcase config row and section after the pagination entry; kept both, structured output first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
This commit is contained in:
cliffhall
commit
0f44288c26
13 changed files with 1161 additions and 35 deletions
|
|
@ -172,7 +172,7 @@ The guard blocks only the **wildcard** all-interfaces addresses. Binding a **spe
|
|||
|
||||
- **Bind a specific address.** `HOST=192.168.1.50` (a LAN IP) or a public IP works directly: the default origin allow-list follows the bind host, so `allowedOrigins` becomes `http://<that-host>:PORT` and a browser hitting that address is accepted with no extra config. (The host is canonicalized the way a browser is — so `HOST=127.1` advertises `http://127.0.0.1:PORT`, an IPv6 bind host is bracketed as `http://[2001:db8::1]:PORT`, and the port is dropped when it's the http default `:80` — matching what the browser sends.)
|
||||
- **Behind TLS or a reverse proxy**, the browser's `Origin` becomes the public origin (e.g. `https://inspector.example.com`, often without a port), which won't match the auto-derived `http://<bind-host>:PORT`. Set `ALLOWED_ORIGINS` to the real public origin(s): `ALLOWED_ORIGINS=https://inspector.example.com`.
|
||||
- **Using the `0.0.0.0` wildcard** (opt-in via `DANGEROUSLY_BIND_ALL_INTERFACES=true`, as the Docker image does): a wildcard bind also serves loopback, so the default allow-list is the loopback trio plus the canonical wildcard origins (`http://0.0.0.0:PORT`, `http://[::]:PORT`), and **local access works out of the box** — `docker run -p 6274:6274` browsed at `http://localhost:6274` connects with no extra config. Reaching it at a **non-loopback** address (a LAN IP, a public hostname) still needs `ALLOWED_ORIGINS` — but since that **replaces** the default, keep the loopback forms in the list if you also browse locally: `ALLOWED_ORIGINS=http://localhost:PORT,http://127.0.0.1:PORT,http://192.168.1.50:PORT,https://inspector.example.com`.
|
||||
- **Using the `0.0.0.0` wildcard** (opt-in via `DANGEROUSLY_BIND_ALL_INTERFACES=true`, as the Docker image does): a wildcard bind also serves loopback, so the default allow-list is the loopback trio plus the canonical wildcard origins (`http://0.0.0.0:PORT`, `http://[::]:PORT`), and **local access works out of the box** — `docker run -p 127.0.0.1:6274:6274` browsed at `http://localhost:6274` connects with no extra config. Reaching it at a **non-loopback** address (a LAN IP, a public hostname) still needs `ALLOWED_ORIGINS` — but since that **replaces** the default, keep the loopback forms in the list if you also browse locally: `ALLOWED_ORIGINS=http://localhost:PORT,http://127.0.0.1:PORT,http://192.168.1.50:PORT,https://inspector.example.com`.
|
||||
|
||||
The bind-host guard and the `ALLOWED_ORIGINS` allow-list apply to both the prod server and `--dev`. Note that in **`--dev`** the Vite dev server _additionally_ enforces its own `server.allowedHosts` Host-header check, whose default accepts loopback and IP-literal hosts. The host you **bind** is auto-allowed (Vite adds the resolved `server.host` — which this config sets from `HOST` — to the allow-list), so `HOST=<hostname>` works out of the box under `--dev` too. What needs an explicit `server.allowedHosts` entry is reaching the dev server at a **different** name than the one bound — e.g. a wildcard bind reached by hostname, or a reverse-proxy domain. For those, prefer the prod server (`mcp-inspector --web`) or add the host to `server.allowedHosts`.
|
||||
|
||||
|
|
|
|||
|
|
@ -149,4 +149,28 @@ describe("AppDetailPanel", () => {
|
|||
await user.click(screen.getByRole("button", { name: /open app/i }));
|
||||
expect(onOpenApp).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("drops a previous app's in-progress number text when the app changes", async () => {
|
||||
const user = userEvent.setup();
|
||||
// AppsScreen swaps selectedAppName + formValues in place rather than
|
||||
// remounting this panel, so two apps exposing a same-named number field
|
||||
// share the field component. Both values here are undefined, which the
|
||||
// draft/value re-sync cannot distinguish — only the resetKey identity can.
|
||||
const numberFieldTool = (name: string): Tool => ({
|
||||
name,
|
||||
title: name,
|
||||
inputSchema: {
|
||||
type: "object",
|
||||
properties: { scale: { type: "number", title: "Scale" } },
|
||||
},
|
||||
});
|
||||
const { rerender } = renderWithMantine(
|
||||
<AppDetailPanel {...baseProps} tool={numberFieldTool("app_a")} />,
|
||||
);
|
||||
const input = () => screen.getByLabelText(/Scale/) as HTMLInputElement;
|
||||
await user.type(input(), "-");
|
||||
expect(input().value).toBe("-");
|
||||
rerender(<AppDetailPanel {...baseProps} tool={numberFieldTool("app_b")} />);
|
||||
expect(input().value).toBe("");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -76,6 +76,12 @@ export function AppDetailPanel({
|
|||
values={formValues}
|
||||
onChange={onFormChange}
|
||||
disabled={isOpening}
|
||||
// Like ToolDetailPanel, this panel is reused across app selections
|
||||
// rather than remounted (AppsScreen.handleSelect swaps
|
||||
// selectedAppName + formValues in place), so the form needs the app
|
||||
// tool's name to drop another app's in-progress field text. See
|
||||
// SchemaFormProps.resetKey.
|
||||
resetKey={tool.name}
|
||||
/>
|
||||
|
||||
<OpenAppButton
|
||||
|
|
|
|||
|
|
@ -1,7 +1,12 @@
|
|||
import { useState } from "react";
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import type { InspectorFormSchema } from "../../../utils/jsonUtils";
|
||||
import { renderWithMantine, screen } from "../../../test/renderWithMantine";
|
||||
import {
|
||||
fireEvent,
|
||||
renderWithMantine,
|
||||
screen,
|
||||
} from "../../../test/renderWithMantine";
|
||||
import { SchemaForm } from "./SchemaForm";
|
||||
|
||||
describe("SchemaForm", () => {
|
||||
|
|
@ -228,6 +233,238 @@ describe("SchemaForm", () => {
|
|||
expect(lastCall.count).toBeUndefined();
|
||||
});
|
||||
|
||||
describe("number fields (#1888)", () => {
|
||||
// The real callers keep the form values in state and feed them back in, so
|
||||
// the bug only reproduces against a genuinely controlled SchemaForm: an
|
||||
// uncontrolled render never rewrites the box and would pass either way.
|
||||
function ControlledSchemaForm({
|
||||
schema,
|
||||
initialValues = {},
|
||||
onChange,
|
||||
}: {
|
||||
schema: InspectorFormSchema;
|
||||
initialValues?: Record<string, unknown>;
|
||||
onChange: (values: Record<string, unknown>) => void;
|
||||
}) {
|
||||
const [values, setValues] =
|
||||
useState<Record<string, unknown>>(initialValues);
|
||||
return (
|
||||
<SchemaForm
|
||||
schema={schema}
|
||||
values={values}
|
||||
onChange={(next) => {
|
||||
setValues(next);
|
||||
onChange(next);
|
||||
}}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
const numberSchema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
divisor: { type: "number", title: "Divisor" },
|
||||
},
|
||||
};
|
||||
|
||||
it("lets a decimal be typed all the way through", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
renderWithMantine(
|
||||
<ControlledSchemaForm schema={numberSchema} onChange={onChange} />,
|
||||
);
|
||||
const input = screen.getByLabelText(/Divisor/) as HTMLInputElement;
|
||||
await user.type(input, "1.5");
|
||||
expect(input.value).toBe("1.5");
|
||||
expect(onChange).toHaveBeenLastCalledWith({ divisor: 1.5 });
|
||||
});
|
||||
|
||||
it("keeps the trailing decimal point visible mid-entry", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
renderWithMantine(
|
||||
<ControlledSchemaForm schema={numberSchema} onChange={onChange} />,
|
||||
);
|
||||
const input = screen.getByLabelText(/Divisor/) as HTMLInputElement;
|
||||
await user.type(input, "1.");
|
||||
// The point survives on screen even though "1." parses to plain 1.
|
||||
expect(input.value).toBe("1.");
|
||||
expect(onChange).toHaveBeenLastCalledWith({ divisor: 1 });
|
||||
});
|
||||
|
||||
it("keeps a trailing zero after the decimal point", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
renderWithMantine(
|
||||
<ControlledSchemaForm schema={numberSchema} onChange={onChange} />,
|
||||
);
|
||||
const input = screen.getByLabelText(/Divisor/) as HTMLInputElement;
|
||||
await user.type(input, "1.50");
|
||||
expect(input.value).toBe("1.50");
|
||||
expect(onChange).toHaveBeenLastCalledWith({ divisor: 1.5 });
|
||||
});
|
||||
|
||||
it("reports a lone minus sign as no value while leaving it typed", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
renderWithMantine(
|
||||
<ControlledSchemaForm schema={numberSchema} onChange={onChange} />,
|
||||
);
|
||||
const input = screen.getByLabelText(/Divisor/) as HTMLInputElement;
|
||||
await user.type(input, "-");
|
||||
expect(input.value).toBe("-");
|
||||
expect(onChange).toHaveBeenLastCalledWith({ divisor: undefined });
|
||||
await user.type(input, "2.5");
|
||||
expect(onChange).toHaveBeenLastCalledWith({ divisor: -2.5 });
|
||||
});
|
||||
|
||||
it("rejects a decimal point in an integer field", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
count: { type: "integer", title: "Count" },
|
||||
},
|
||||
};
|
||||
renderWithMantine(
|
||||
<ControlledSchemaForm schema={schema} onChange={onChange} />,
|
||||
);
|
||||
const input = screen.getByLabelText(/Count/) as HTMLInputElement;
|
||||
await user.type(input, "1.5");
|
||||
expect(input.value).toBe("15");
|
||||
expect(onChange).toHaveBeenLastCalledWith({ count: 15 });
|
||||
});
|
||||
|
||||
it("reports no value for a magnitude JS cannot hold exactly", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
renderWithMantine(
|
||||
<ControlledSchemaForm schema={numberSchema} onChange={onChange} />,
|
||||
);
|
||||
const input = screen.getByLabelText(/Divisor/) as HTMLInputElement;
|
||||
// Past Number.MAX_SAFE_INTEGER, Mantine stops emitting a number and hands
|
||||
// back the raw string to avoid destroying precision. Number() would round
|
||||
// this to ...904, so parsing it would send a value the user never typed.
|
||||
await user.type(input, "90071992547409910");
|
||||
expect(input.value).toBe("90071992547409910");
|
||||
expect(onChange).toHaveBeenLastCalledWith({ divisor: undefined });
|
||||
});
|
||||
|
||||
it("still parses a long decimal, which stays exactly representable", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
renderWithMantine(
|
||||
<ControlledSchemaForm schema={numberSchema} onChange={onChange} />,
|
||||
);
|
||||
const input = screen.getByLabelText(/Divisor/) as HTMLInputElement;
|
||||
// Guards the safe-integer check against over-rejecting: the fractional
|
||||
// digits are not what overflows, so this must not be dropped.
|
||||
await user.type(input, "3.14159265358979");
|
||||
expect(onChange).toHaveBeenLastCalledWith({ divisor: 3.14159265358979 });
|
||||
});
|
||||
|
||||
it("drops in-progress text when resetKey moves the form to another entity", async () => {
|
||||
const user = userEvent.setup();
|
||||
// Both "tools" expose the same-named number field with no default, so the
|
||||
// value is `undefined` before and after the switch. The value comparison
|
||||
// sees no divergence, and only resetKey can tell the field to start over.
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: { divisor: { type: "number", title: "Divisor" } },
|
||||
};
|
||||
function Harness() {
|
||||
const [tool, setTool] = useState("tool-a");
|
||||
const [values, setValues] = useState<Record<string, unknown>>({});
|
||||
return (
|
||||
<>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => {
|
||||
setTool("tool-b");
|
||||
// What ToolsScreen does on select: replace the form values.
|
||||
setValues({});
|
||||
}}
|
||||
>
|
||||
Switch tool
|
||||
</button>
|
||||
<SchemaForm
|
||||
schema={schema}
|
||||
values={values}
|
||||
onChange={setValues}
|
||||
resetKey={tool}
|
||||
/>
|
||||
</>
|
||||
);
|
||||
}
|
||||
renderWithMantine(<Harness />);
|
||||
const input = () => screen.getByLabelText(/Divisor/) as HTMLInputElement;
|
||||
await user.type(input(), "-");
|
||||
expect(input().value).toBe("-");
|
||||
// fireEvent, not user.click: a real click also blurs the input, and
|
||||
// Mantine sanitizes an incomplete value on blur — which would mask
|
||||
// whether the switch itself cleared the draft. This drives the state
|
||||
// change without the blur, isolating the reset to resetKey.
|
||||
fireEvent.click(screen.getByRole("button", { name: "Switch tool" }));
|
||||
expect(input().value).toBe("");
|
||||
});
|
||||
|
||||
it("keeps in-progress text across re-renders of the same entity", async () => {
|
||||
const user = userEvent.setup();
|
||||
// The counterpart to the test above: a stable resetKey must NOT remount
|
||||
// the field, or every keystroke would wipe the draft and reinstate #1888.
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: { divisor: { type: "number", title: "Divisor" } },
|
||||
};
|
||||
function Harness() {
|
||||
const [values, setValues] = useState<Record<string, unknown>>({});
|
||||
return (
|
||||
<SchemaForm
|
||||
schema={schema}
|
||||
values={values}
|
||||
onChange={setValues}
|
||||
resetKey="tool-a"
|
||||
/>
|
||||
);
|
||||
}
|
||||
renderWithMantine(<Harness />);
|
||||
const input = screen.getByLabelText(/Divisor/) as HTMLInputElement;
|
||||
await user.type(input, "1.5");
|
||||
expect(input.value).toBe("1.5");
|
||||
});
|
||||
|
||||
it("re-syncs the displayed text when the value changes externally", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
function Harness() {
|
||||
const [values, setValues] = useState<Record<string, unknown>>({
|
||||
divisor: 1.5,
|
||||
});
|
||||
return (
|
||||
<>
|
||||
<button type="button" onClick={() => setValues({ divisor: 42 })}>
|
||||
Load example
|
||||
</button>
|
||||
<SchemaForm
|
||||
schema={numberSchema}
|
||||
values={values}
|
||||
onChange={(next) => {
|
||||
setValues(next);
|
||||
onChange(next);
|
||||
}}
|
||||
/>
|
||||
</>
|
||||
);
|
||||
}
|
||||
renderWithMantine(<Harness />);
|
||||
const input = screen.getByLabelText(/Divisor/) as HTMLInputElement;
|
||||
expect(input.value).toBe("1.5");
|
||||
await user.click(screen.getByRole("button", { name: "Load example" }));
|
||||
expect(input.value).toBe("42");
|
||||
});
|
||||
});
|
||||
|
||||
it("falls back to empty/const labels for oneOf items missing const and title", () => {
|
||||
const onChange = vi.fn();
|
||||
const schema: InspectorFormSchema = {
|
||||
|
|
|
|||
|
|
@ -8,7 +8,9 @@ import {
|
|||
Text,
|
||||
TextInput,
|
||||
} from "@mantine/core";
|
||||
import { useState } from "react";
|
||||
import { ClearButton } from "../../elements/ClearButton/ClearButton";
|
||||
import { useValueChange } from "../../../hooks/useValueChange";
|
||||
import type { InspectorFormSchema } from "../../../utils/jsonUtils";
|
||||
|
||||
const FieldLabel = Text.withProps({
|
||||
|
|
@ -49,11 +51,129 @@ function toEnumData(
|
|||
return values;
|
||||
}
|
||||
|
||||
/**
|
||||
* Interpret whatever Mantine's `NumberInput` reported as the JSON value for the
|
||||
* field. Anything that is not a finite number becomes `undefined`, which is how
|
||||
* an absent optional argument is represented everywhere else in this form.
|
||||
*
|
||||
* `NumberInput` emits a `number` only when the text both parses *and* is exactly
|
||||
* representable; otherwise it hands back the **raw string** (see its
|
||||
* `isValidNumber` guard). Two quite different situations produce a string, and
|
||||
* they are treated differently here:
|
||||
*
|
||||
* 1. **Mid-entry text** — `""` when cleared, plus `"1."`, `"1.50"`, and a lone
|
||||
* `"-"`. These are parsed: `"1."` really does mean `1`. (Note that an
|
||||
* exponent is *not* in this set — `NumberInput` masks input through
|
||||
* `NumericFormat`, which rejects `e` outright, so `"1e"` can never be typed.)
|
||||
* 2. **Values JS cannot hold exactly** — anything at or beyond
|
||||
* `Number.MAX_SAFE_INTEGER`. `Number("90071992547409910")` silently yields
|
||||
* `90071992547409904`, so parsing here would send the server a number the
|
||||
* user never entered. An inspector must not misreport what it transmits, so
|
||||
* these report no value instead — which is also what this field did with such
|
||||
* input before #1888, making it no regression. Preserving them properly needs
|
||||
* an exact-serialization path down the whole `tools/call` chain, which is a
|
||||
* separate concern from being able to type a decimal.
|
||||
*/
|
||||
function toNumericValue(raw: string | number): number | undefined {
|
||||
if (typeof raw === "number") {
|
||||
return Number.isFinite(raw) ? raw : undefined;
|
||||
}
|
||||
if (raw.trim() === "") {
|
||||
return undefined;
|
||||
}
|
||||
const parsed = Number(raw);
|
||||
if (!Number.isFinite(parsed)) {
|
||||
return undefined;
|
||||
}
|
||||
// Case 2 above. The integer part is what overflows exact representation; the
|
||||
// fractional digits are bounded by the same guard and stay lossless.
|
||||
return Number.isSafeInteger(Math.trunc(parsed)) ? parsed : undefined;
|
||||
}
|
||||
|
||||
interface SchemaNumberInputProps {
|
||||
label: string;
|
||||
description?: string;
|
||||
withAsterisk: boolean;
|
||||
disabled: boolean;
|
||||
value: number | undefined;
|
||||
min?: number;
|
||||
max?: number;
|
||||
allowDecimal: boolean;
|
||||
onChange: (value: number | undefined) => void;
|
||||
}
|
||||
|
||||
/**
|
||||
* A `NumberInput` that keeps the text the user is typing, not just the number it
|
||||
* currently parses to.
|
||||
*
|
||||
* Driving `NumberInput` directly off the parent's numeric value makes a decimal
|
||||
* impossible to enter (#1888): typing `.` after `1` produces the unparseable
|
||||
* string `"1."`, the numeric value stays `1`, and the controlled `value` prop
|
||||
* immediately rewrites the box back to `"1"` — so the `.` vanishes and `1.5` can
|
||||
* never be reached. Trailing zeros (`"1.50"`) and a lone leading `"-"` fail the
|
||||
* same way.
|
||||
*
|
||||
* So the raw text is held here as the source of truth for what is *displayed*,
|
||||
* while the parent still only ever sees a `number | undefined`. The two are
|
||||
* re-synced only when the parent's value genuinely diverges from what the draft
|
||||
* parses to, which leaves an external reset (a cleared form, a loaded example)
|
||||
* working while an in-progress `"1."` — whose parse is `1`, matching the value we
|
||||
* just emitted — is left alone.
|
||||
*
|
||||
* That value comparison cannot see a reset to an *equal* value, so the caller is
|
||||
* additionally expected to vary this component's React key via `SchemaForm`'s
|
||||
* `resetKey` when it switches which entity the form edits. See the note on that
|
||||
* prop for the case it covers.
|
||||
*/
|
||||
function SchemaNumberInput({
|
||||
value,
|
||||
onChange,
|
||||
...inputProps
|
||||
}: SchemaNumberInputProps) {
|
||||
const [draft, setDraft] = useState<string | number>(value ?? "");
|
||||
|
||||
useValueChange(value, (next) => {
|
||||
if (!Object.is(toNumericValue(draft), next)) {
|
||||
setDraft(next ?? "");
|
||||
}
|
||||
});
|
||||
|
||||
return (
|
||||
<NumberInput
|
||||
{...inputProps}
|
||||
value={draft}
|
||||
onChange={(next) => {
|
||||
setDraft(next);
|
||||
onChange(toNumericValue(next));
|
||||
}}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
export interface SchemaFormProps {
|
||||
schema: InspectorFormSchema;
|
||||
values: Record<string, unknown>;
|
||||
onChange: (values: Record<string, unknown>) => void;
|
||||
disabled?: boolean;
|
||||
/**
|
||||
* Stable identity of whatever this form is editing — a tool name, a request
|
||||
* id. Pass it whenever the same mounted form is reused for a *different*
|
||||
* entity, which is the case for the Tools tab: `ToolDetailPanel` is not keyed
|
||||
* by tool, so selecting another tool re-renders the same field components.
|
||||
*
|
||||
* It exists because the number field's draft/value re-sync compares parsed
|
||||
* numbers, and so cannot detect a reset to an equal value. Type `-` (draft
|
||||
* `"-"`, value `undefined`), then switch to a tool with a same-named number
|
||||
* field and no default: the value is `undefined` on both sides, no divergence
|
||||
* is seen, and the stale `-` would otherwise be left in the box for the new
|
||||
* tool to continue from. Varying `resetKey` remounts the field instead, so no
|
||||
* in-progress text can outlive the entity it was typed into.
|
||||
*
|
||||
* Omit it when the form is mounted fresh per entity (the elicitation panels),
|
||||
* where unmounting already discards the draft. The schema object itself is no
|
||||
* substitute — callers rebuild it every render, so its identity is unstable.
|
||||
*/
|
||||
resetKey?: string;
|
||||
}
|
||||
|
||||
function getDefaultValue(fieldSchema: InspectorFormSchema): unknown {
|
||||
|
|
@ -78,6 +198,7 @@ export function SchemaForm({
|
|||
values,
|
||||
onChange,
|
||||
disabled = false,
|
||||
resetKey,
|
||||
}: SchemaFormProps) {
|
||||
const properties = schema.properties ?? {};
|
||||
const requiredFields = schema.required ?? [];
|
||||
|
|
@ -156,19 +277,21 @@ export function SchemaForm({
|
|||
// number or integer
|
||||
if (fieldSchema.type === "number" || fieldSchema.type === "integer") {
|
||||
return (
|
||||
<NumberInput
|
||||
key={fieldName}
|
||||
<SchemaNumberInput
|
||||
// The only field holding local state, so the only one that has to be
|
||||
// remounted when `resetKey` says the form moved to another entity.
|
||||
key={resetKey === undefined ? fieldName : `${resetKey}:${fieldName}`}
|
||||
label={label}
|
||||
description={description}
|
||||
withAsterisk={isRequired}
|
||||
disabled={disabled}
|
||||
value={(rawValue as number) ?? ""}
|
||||
value={rawValue as number | undefined}
|
||||
min={fieldSchema.minimum}
|
||||
max={fieldSchema.maximum}
|
||||
onChange={(val) => {
|
||||
const numericValue = typeof val === "string" ? undefined : val;
|
||||
handleFieldChange(fieldName, numericValue);
|
||||
}}
|
||||
// An `integer` field rejects the decimal point outright rather than
|
||||
// accepting a value the schema forbids.
|
||||
allowDecimal={fieldSchema.type === "number"}
|
||||
onChange={(val) => handleFieldChange(fieldName, val)}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
|
@ -243,6 +366,8 @@ export function SchemaForm({
|
|||
handleFieldChange(fieldName, nestedValues)
|
||||
}
|
||||
disabled={disabled}
|
||||
// Sub-fields belong to the same entity, so they reset with it.
|
||||
resetKey={resetKey}
|
||||
/>
|
||||
</IndentedStack>
|
||||
</Stack>
|
||||
|
|
|
|||
|
|
@ -0,0 +1,109 @@
|
|||
import type { Meta, StoryObj } from "@storybook/react-vite";
|
||||
import { expect, userEvent, waitFor, within } from "storybook/test";
|
||||
import { StructuredOutputPanel } from "./StructuredOutputPanel";
|
||||
|
||||
const nested = {
|
||||
items: [
|
||||
{ id: 1, name: "Item A", tags: ["foo", "bar"] },
|
||||
{ id: 2, name: "Item B", tags: ["baz"] },
|
||||
],
|
||||
total: 2,
|
||||
};
|
||||
|
||||
const flat = {
|
||||
temperature: 25,
|
||||
unit: "C",
|
||||
city: "San Francisco",
|
||||
};
|
||||
|
||||
// Long enough that the payload exceeds the section's max height and scrolls
|
||||
// within it rather than pushing the rest of the result panel down.
|
||||
const large = {
|
||||
rows: Array.from({ length: 60 }, (_, index) => ({
|
||||
id: index + 1,
|
||||
label: `Row ${index + 1}`,
|
||||
value: index * 7,
|
||||
})),
|
||||
total: 60,
|
||||
};
|
||||
|
||||
const meta: Meta<typeof StructuredOutputPanel> = {
|
||||
title: "Groups/StructuredOutputPanel",
|
||||
component: StructuredOutputPanel,
|
||||
};
|
||||
|
||||
export default meta;
|
||||
type Story = StoryObj<typeof StructuredOutputPanel>;
|
||||
|
||||
export const Nested: Story = {
|
||||
args: {
|
||||
structuredContent: nested,
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
expect(
|
||||
canvas.getByRole("heading", { name: "Structured Output" }),
|
||||
).toBeInTheDocument();
|
||||
// The section starts expanded, so the payload is visible without a click.
|
||||
// Asserted on the container's text: once the Prism grammar loads, the JSON
|
||||
// is split across per-token elements, so no single node holds the value.
|
||||
await waitFor(() =>
|
||||
expect(canvasElement.textContent).toContain('"Item A"'),
|
||||
);
|
||||
},
|
||||
};
|
||||
|
||||
export const Flat: Story = {
|
||||
args: {
|
||||
structuredContent: flat,
|
||||
},
|
||||
};
|
||||
|
||||
// A payload taller than the section's cap scrolls inside it rather than
|
||||
// pushing the rest of the result panel down. Asserted on the real geometry,
|
||||
// so dropping `mah` (or moving it to the wrong element) fails here.
|
||||
export const Large: Story = {
|
||||
args: {
|
||||
structuredContent: large,
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
await waitFor(() =>
|
||||
expect(canvasElement.textContent).toContain('"Row 60"'),
|
||||
);
|
||||
const viewport = canvasElement.querySelector(
|
||||
".mantine-ScrollArea-viewport",
|
||||
);
|
||||
if (!(viewport instanceof HTMLElement)) {
|
||||
throw new Error("scroll viewport not found");
|
||||
}
|
||||
// Capped: the visible height stays at the section's `mah`, well under the
|
||||
// payload's natural height…
|
||||
expect(viewport.clientHeight).toBeLessThanOrEqual(400);
|
||||
// …and the overflow is reachable by scrolling rather than clipped away.
|
||||
expect(viewport.scrollHeight).toBeGreaterThan(viewport.clientHeight);
|
||||
},
|
||||
};
|
||||
|
||||
// Collapsing hides the payload; expanding brings it back.
|
||||
export const Collapsed: Story = {
|
||||
args: {
|
||||
structuredContent: nested,
|
||||
defaultExpanded: false,
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
const toggle = canvas.getByRole("button", {
|
||||
name: "Expand structured output",
|
||||
});
|
||||
expect(toggle).toHaveAttribute("aria-expanded", "false");
|
||||
await userEvent.click(toggle);
|
||||
await waitFor(() =>
|
||||
expect(
|
||||
canvas.getByRole("button", { name: "Collapse structured output" }),
|
||||
).toHaveAttribute("aria-expanded", "true"),
|
||||
);
|
||||
await waitFor(() =>
|
||||
expect(canvasElement.textContent).toContain('"Item A"'),
|
||||
);
|
||||
},
|
||||
};
|
||||
|
|
@ -0,0 +1,87 @@
|
|||
import { describe, it, expect, vi } from "vitest";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { renderWithMantine, screen } from "../../../test/renderWithMantine";
|
||||
import { StructuredOutputPanel } from "./StructuredOutputPanel";
|
||||
|
||||
// The real CodeHighlight dynamic-imports the Prism runtime and each grammar
|
||||
// (its own behavior is covered in CodeHighlight.test.tsx). Stub it so these
|
||||
// tests assert on the JSON handed to it, synchronously.
|
||||
vi.mock("../../elements/CodeHighlight/CodeHighlight", () => ({
|
||||
CodeHighlight: ({ language, code }: { language: string; code: string }) => (
|
||||
<pre data-testid="code-highlight" data-language={language}>
|
||||
{code}
|
||||
</pre>
|
||||
),
|
||||
}));
|
||||
|
||||
const structuredContent = {
|
||||
items: [
|
||||
{ id: 1, name: "Item A", tags: ["foo", "bar"] },
|
||||
{ id: 2, name: "Item B", tags: ["baz"] },
|
||||
],
|
||||
total: 2,
|
||||
};
|
||||
|
||||
describe("StructuredOutputPanel", () => {
|
||||
it("renders a labeled section with the payload as pretty-printed JSON", () => {
|
||||
renderWithMantine(
|
||||
<StructuredOutputPanel structuredContent={structuredContent} />,
|
||||
);
|
||||
expect(
|
||||
screen.getByRole("heading", { name: "Structured Output" }),
|
||||
).toBeInTheDocument();
|
||||
const code = screen.getByTestId("code-highlight");
|
||||
expect(code).toHaveAttribute("data-language", "json");
|
||||
expect(code).toHaveTextContent(/"total": 2/);
|
||||
// Nested values are inspectable field by field, not summarized away.
|
||||
expect(code).toHaveTextContent(/"name": "Item A"/);
|
||||
expect(code).toHaveTextContent(/"tags"/);
|
||||
});
|
||||
|
||||
it("starts expanded by default", () => {
|
||||
renderWithMantine(
|
||||
<StructuredOutputPanel structuredContent={structuredContent} />,
|
||||
);
|
||||
expect(
|
||||
screen.getByRole("button", { name: "Collapse structured output" }),
|
||||
).toHaveAttribute("aria-expanded", "true");
|
||||
});
|
||||
|
||||
it("starts collapsed when defaultExpanded is false", () => {
|
||||
renderWithMantine(
|
||||
<StructuredOutputPanel
|
||||
structuredContent={structuredContent}
|
||||
defaultExpanded={false}
|
||||
/>,
|
||||
);
|
||||
expect(
|
||||
screen.getByRole("button", { name: "Expand structured output" }),
|
||||
).toHaveAttribute("aria-expanded", "false");
|
||||
});
|
||||
|
||||
it("toggles between expanded and collapsed", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithMantine(
|
||||
<StructuredOutputPanel structuredContent={structuredContent} />,
|
||||
);
|
||||
await user.click(
|
||||
screen.getByRole("button", { name: "Collapse structured output" }),
|
||||
);
|
||||
const toggle = screen.getByRole("button", {
|
||||
name: "Expand structured output",
|
||||
});
|
||||
expect(toggle).toHaveAttribute("aria-expanded", "false");
|
||||
await user.click(toggle);
|
||||
expect(
|
||||
screen.getByRole("button", { name: "Collapse structured output" }),
|
||||
).toHaveAttribute("aria-expanded", "true");
|
||||
});
|
||||
|
||||
it("renders an empty structured payload rather than nothing", () => {
|
||||
renderWithMantine(<StructuredOutputPanel structuredContent={{}} />);
|
||||
expect(
|
||||
screen.getByRole("heading", { name: "Structured Output" }),
|
||||
).toBeInTheDocument();
|
||||
expect(screen.getByTestId("code-highlight")).toHaveTextContent("{}");
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,102 @@
|
|||
import { useState } from "react";
|
||||
import {
|
||||
Collapse,
|
||||
Group,
|
||||
Paper,
|
||||
ScrollArea,
|
||||
Stack,
|
||||
Title,
|
||||
} from "@mantine/core";
|
||||
import type { CallToolResult } from "@modelcontextprotocol/client";
|
||||
import { ContentViewer } from "../../elements/ContentViewer/ContentViewer";
|
||||
import { ExpandToggle } from "../../elements/ExpandToggle/ExpandToggle";
|
||||
|
||||
export interface StructuredOutputPanelProps {
|
||||
/** The result's `structuredContent` — the tool's schema-validated payload. */
|
||||
structuredContent: NonNullable<CallToolResult["structuredContent"]>;
|
||||
/** Whether the section starts expanded. Defaults to `true`. */
|
||||
defaultExpanded?: boolean;
|
||||
}
|
||||
|
||||
// Bordered box matching the "Resource Links" group in the result panel, so the
|
||||
// two supplementary sections of a tool result read as siblings.
|
||||
const StructuredBox = Paper.withProps({
|
||||
withBorder: true,
|
||||
radius: "md",
|
||||
p: "md",
|
||||
variant: "panel",
|
||||
});
|
||||
|
||||
const StructuredInner = Stack.withProps({
|
||||
gap: "sm",
|
||||
});
|
||||
|
||||
const HeaderRow = Group.withProps({
|
||||
justify: "space-between",
|
||||
wrap: "nowrap",
|
||||
});
|
||||
|
||||
// h4 (size h5) for the same reason as the "Resource Links" heading: the panel's
|
||||
// "Results" title is h3, so a sub-box heading is h4 and the heading order never
|
||||
// skips a level (axe `heading-order`).
|
||||
const StructuredHeader = Title.withProps({
|
||||
order: 4,
|
||||
size: "h5",
|
||||
});
|
||||
|
||||
// Caps the payload so a large structured result scrolls within the box instead
|
||||
// of pushing the content blocks out of view. `Autosize` sizes to the content up
|
||||
// to `mah`, so a small object still takes only what it needs.
|
||||
const StructuredScroll = ScrollArea.Autosize.withProps({
|
||||
mah: 400,
|
||||
type: "auto",
|
||||
scrollbars: "y",
|
||||
offsetScrollbars: true,
|
||||
});
|
||||
|
||||
/**
|
||||
* Collapsible "Structured Output" section for a tool result's
|
||||
* `structuredContent` (#1908). A tool declaring an `outputSchema` returns its
|
||||
* real payload here — the `content[]` blocks usually only summarize it — so v1
|
||||
* rendered it as its own inspectable JSON section. Without this, the payload is
|
||||
* dropped from the Tools screen entirely, with no hint it was ever returned.
|
||||
*
|
||||
* The JSON is pretty-printed and syntax-highlighted through {@link ContentViewer}
|
||||
* (an `application/json` text block), so it is copyable and scannable field by
|
||||
* field.
|
||||
*/
|
||||
export function StructuredOutputPanel({
|
||||
structuredContent,
|
||||
defaultExpanded = true,
|
||||
}: StructuredOutputPanelProps) {
|
||||
const [expanded, setExpanded] = useState(defaultExpanded);
|
||||
|
||||
return (
|
||||
<StructuredBox>
|
||||
<StructuredInner>
|
||||
<HeaderRow>
|
||||
<StructuredHeader>Structured Output</StructuredHeader>
|
||||
<ExpandToggle
|
||||
expanded={expanded}
|
||||
onToggle={() => setExpanded((value) => !value)}
|
||||
ariaLabel={`${expanded ? "Collapse" : "Expand"} structured output`}
|
||||
/>
|
||||
</HeaderRow>
|
||||
{/* Content stays mounted across a collapse (Mantine `Collapse`), so the
|
||||
highlighted JSON isn't re-rendered from scratch on every toggle. */}
|
||||
<Collapse in={expanded}>
|
||||
<StructuredScroll>
|
||||
<ContentViewer
|
||||
block={{
|
||||
type: "text",
|
||||
text: JSON.stringify(structuredContent, null, 2),
|
||||
}}
|
||||
mimeType="application/json"
|
||||
copyable
|
||||
/>
|
||||
</StructuredScroll>
|
||||
</Collapse>
|
||||
</StructuredInner>
|
||||
</StructuredBox>
|
||||
);
|
||||
}
|
||||
|
|
@ -317,6 +317,10 @@ export function ToolDetailPanel({
|
|||
values={formValues}
|
||||
onChange={onFormChange}
|
||||
disabled={isExecuting}
|
||||
// This panel is reused across tool selections rather than remounted,
|
||||
// so the form needs the tool name to drop another tool's
|
||||
// in-progress field text. See SchemaFormProps.resetKey.
|
||||
resetKey={name}
|
||||
/>
|
||||
|
||||
{progress && <ProgressDisplay params={progress} />}
|
||||
|
|
|
|||
|
|
@ -173,6 +173,24 @@ export const ResourceLinksWithLongText: Story = {
|
|||
decorators: fillHeightDecorators,
|
||||
};
|
||||
|
||||
// A tool declaring an `outputSchema` returns its real payload in
|
||||
// `structuredContent`, which the text block only summarizes (#1908). It gets
|
||||
// its own collapsible "Structured Output" section below the content blocks.
|
||||
export const StructuredOutput: Story = {
|
||||
args: {
|
||||
result: {
|
||||
content: [{ type: "text", text: "Found 2 items." }],
|
||||
structuredContent: {
|
||||
items: [
|
||||
{ id: 1, name: "Item A", tags: ["foo", "bar"] },
|
||||
{ id: 2, name: "Item B", tags: ["baz"] },
|
||||
],
|
||||
total: 2,
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
export const ErrorResult: Story = {
|
||||
args: {
|
||||
result: {
|
||||
|
|
|
|||
|
|
@ -109,6 +109,78 @@ describe("ToolResultPanel", () => {
|
|||
expect(screen.getByText("divider")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
describe("structuredContent (#1908)", () => {
|
||||
const structured = { items: [{ id: 1, name: "Item A" }], total: 1 };
|
||||
|
||||
it("renders a Structured Output section alongside the content blocks", () => {
|
||||
renderWithMantine(
|
||||
<ToolResultPanel
|
||||
result={{ ...okResult, structuredContent: structured }}
|
||||
onClear={() => {}}
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByText("ok")).toBeInTheDocument();
|
||||
expect(
|
||||
screen.getByRole("heading", { name: "Structured Output" }),
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("omits the section when the result has no structuredContent", () => {
|
||||
renderWithMantine(
|
||||
<ToolResultPanel result={okResult} onClear={() => {}} />,
|
||||
);
|
||||
expect(
|
||||
screen.queryByRole("heading", { name: "Structured Output" }),
|
||||
).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("renders the section instead of the empty state when content is empty", () => {
|
||||
renderWithMantine(
|
||||
<ToolResultPanel
|
||||
result={{ content: [], structuredContent: structured }}
|
||||
onClear={() => {}}
|
||||
/>,
|
||||
);
|
||||
expect(screen.queryByText("No results yet")).not.toBeInTheDocument();
|
||||
expect(
|
||||
screen.getByRole("heading", { name: "Structured Output" }),
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("renders the section below the alert on an error result", () => {
|
||||
renderWithMantine(
|
||||
<ToolResultPanel
|
||||
result={{ ...errorResult, structuredContent: structured }}
|
||||
onClear={() => {}}
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByText("Tool Error")).toBeInTheDocument();
|
||||
expect(
|
||||
screen.getByRole("heading", { name: "Structured Output" }),
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("renders the section alongside a Resource Links box", () => {
|
||||
renderWithMantine(
|
||||
<ToolResultPanel
|
||||
result={{
|
||||
content: [
|
||||
{ type: "resource_link", uri: "demo://r/1", name: "One" },
|
||||
],
|
||||
structuredContent: structured,
|
||||
}}
|
||||
onClear={() => {}}
|
||||
/>,
|
||||
);
|
||||
expect(
|
||||
screen.getByRole("heading", { name: "Resource Links" }),
|
||||
).toBeInTheDocument();
|
||||
expect(
|
||||
screen.getByRole("heading", { name: "Structured Output" }),
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
it("invokes onClear when the close button is clicked", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onClear = vi.fn();
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ import type {
|
|||
} from "@modelcontextprotocol/client";
|
||||
import { ContentViewer } from "../../elements/ContentViewer/ContentViewer";
|
||||
import { ResourceLink } from "../ResourceLink/ResourceLink";
|
||||
import { StructuredOutputPanel } from "../StructuredOutputPanel/StructuredOutputPanel";
|
||||
import { resultHasResourceLinks } from "./toolResultUtils";
|
||||
|
||||
export interface ToolResultPanelProps {
|
||||
|
|
@ -217,6 +218,16 @@ export function ToolResultPanel({
|
|||
// available height (and scrolls inside). Plain text/image results keep the
|
||||
// scroll-within-card body so a short result doesn't reserve empty height.
|
||||
const hasLinks = resultHasResourceLinks(result);
|
||||
// A tool with an `outputSchema` returns its real payload in
|
||||
// `structuredContent`, which the `content[]` blocks typically only summarize
|
||||
// (#1908). Render it as its own section — including alongside an error or an
|
||||
// empty `content` array, so it is never silently dropped.
|
||||
const structuredNode = result.structuredContent ? (
|
||||
<StructuredOutputPanel
|
||||
key="structured-output"
|
||||
structuredContent={result.structuredContent}
|
||||
/>
|
||||
) : null;
|
||||
|
||||
const segmentNodes = segments.map((segment) => {
|
||||
if (segment.kind === "links") {
|
||||
|
|
@ -254,22 +265,31 @@ export function ToolResultPanel({
|
|||
</HeaderRow>
|
||||
{result.isError ? (
|
||||
<ResultScroll>
|
||||
<ErrorAlert>
|
||||
{result.content
|
||||
.filter((b) => b.type === "text")
|
||||
.map((b) => b.text)
|
||||
.join("\n")}
|
||||
</ErrorAlert>
|
||||
<ResultStack>
|
||||
<ErrorAlert>
|
||||
{result.content
|
||||
.filter((b) => b.type === "text")
|
||||
.map((b) => b.text)
|
||||
.join("\n")}
|
||||
</ErrorAlert>
|
||||
{structuredNode}
|
||||
</ResultStack>
|
||||
</ResultScroll>
|
||||
) : result.content.length === 0 ? (
|
||||
) : result.content.length === 0 && !structuredNode ? (
|
||||
<ResultScroll>
|
||||
<Text c="dimmed">No results yet</Text>
|
||||
</ResultScroll>
|
||||
) : hasLinks ? (
|
||||
<FillStack>{segmentNodes}</FillStack>
|
||||
<FillStack>
|
||||
{segmentNodes}
|
||||
{structuredNode}
|
||||
</FillStack>
|
||||
) : (
|
||||
<ResultScroll>
|
||||
<ResultStack>{segmentNodes}</ResultStack>
|
||||
<ResultStack>
|
||||
{segmentNodes}
|
||||
{structuredNode}
|
||||
</ResultStack>
|
||||
</ResultScroll>
|
||||
)}
|
||||
</PanelStack>
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
|
||||
import { resolve } from "node:path";
|
||||
import * as z from "zod/v4";
|
||||
import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js";
|
||||
import {
|
||||
|
|
@ -45,6 +46,8 @@ import {
|
|||
createAddResourceTool,
|
||||
createAddToolTool,
|
||||
createAddPromptTool,
|
||||
loadConfig,
|
||||
resolveConfig,
|
||||
} from "@modelcontextprotocol/inspector-test-server";
|
||||
import type {
|
||||
MessageEntry,
|
||||
|
|
@ -1214,6 +1217,52 @@ describe("InspectorClient", () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe("Structured output showcase config (#1908)", () => {
|
||||
// Drives the shipped `structured-output-http.json` end to end — the file
|
||||
// itself, the `list_items` preset registration, and the fixture's nested
|
||||
// payload. A typo in any of the three fails here rather than leaving the
|
||||
// documented showcase quietly broken.
|
||||
const showcaseConfigPath = resolve(
|
||||
import.meta.dirname,
|
||||
"../../../../../../test-servers/configs/structured-output-http.json",
|
||||
);
|
||||
|
||||
it("serves list_items with a summary block and a nested structuredContent", async () => {
|
||||
server = createTestServerHttp(
|
||||
resolveConfig(loadConfig(showcaseConfigPath)),
|
||||
);
|
||||
await server.start();
|
||||
|
||||
client = new InspectorClient(
|
||||
{ type: "streamable-http", url: server.url },
|
||||
{ environment: { transport: createTransportNode } },
|
||||
);
|
||||
await client.connect();
|
||||
|
||||
const tool = await getTool(client, "list_items");
|
||||
expect(tool.outputSchema).toBeDefined();
|
||||
|
||||
const result = await client.callTool(tool, {});
|
||||
expect(result.success).toBe(true);
|
||||
|
||||
// The text block only summarizes — the payload is the structured half.
|
||||
const content = result.result!.content as Array<{
|
||||
type: string;
|
||||
text?: string;
|
||||
}>;
|
||||
expect(content[0].type).toBe("text");
|
||||
expect(content[0].text).toBe("Found 2 items.");
|
||||
|
||||
expect(result.result!.structuredContent).toEqual({
|
||||
items: [
|
||||
{ id: 1, name: "Item A", tags: ["foo", "bar"] },
|
||||
{ id: 2, name: "Item B", tags: ["baz"] },
|
||||
],
|
||||
total: 2,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("Default metadata (server-wide _meta)", () => {
|
||||
function metaOf(req: { message: unknown }): Record<string, unknown> {
|
||||
const params = (req.message as { params?: { _meta?: unknown } }).params;
|
||||
|
|
|
|||
Loading…
Reference in a new issue