diff --git a/README.md b/README.md index a15210d1..82164a98 100644 --- a/README.md +++ b/README.md @@ -145,6 +145,7 @@ Each config below is a ready-made server for exercising one feature by hand. Loa | `pagination-http.json` | Page-by-page list fetching | [#1721](https://github.com/modelcontextprotocol/inspector/issues/1721) | | `structured-output-http.json` | Tools tab: a result's `structuredContent` section | [#1908](https://github.com/modelcontextprotocol/inspector/issues/1908) | | `duplicate-tool-names-http.json` | A `tools/list` that repeats a tool name | [#1957](https://github.com/modelcontextprotocol/inspector/issues/1957) | +| `nullable-fields-http.json` | Tools tab: nullable (`anyOf` + `null`) arguments | [#1928](https://github.com/modelcontextprotocol/inspector/issues/1928) | | `advertised-extensions-http.json` | Tool registration gated on advertised extensions | [#1739](https://github.com/modelcontextprotocol/inspector/issues/1739) | | `logging-{legacy,modern}-http.json` | Logging, both eras | [#1629](https://github.com/modelcontextprotocol/inspector/issues/1629) | | `subscriptions-{legacy,modern}-http.json` | Resource subscriptions, both eras | [#1630](https://github.com/modelcontextprotocol/inspector/issues/1630) | @@ -227,6 +228,12 @@ Connect (default legacy era), open the Tools tab, and type `get` into **Search t The duplicated copies are appended rather than placed beside their twin on purpose. React matches a leading run of same-key children first, so a head-adjacent duplicate happens to line up and the defect hides; separating the pair is what makes it observable — and it is also the realistic shape, two tool sources concatenated. +#### Nullable arguments + +`nullable-fields-http.json` serves `record_shipment`, whose four arguments are each declared with Zod's `.nullish()` — "optional **and** explicitly nullable". That compiles to `anyOf: [, { "type": "null" }]`, so the real type (and, for the enum, its `enum` list) sits on a branch rather than at the top level. `get_temp` sits alongside it with a plain, non-nullable `units` enum for comparison. Plain streamable-HTTP — connect with the **default (legacy)** protocol era. + +Open the Tools tab and select `record_shipment`: `direction` must render as a **Select** (`envio` / `recebimento`) with a clear button that sets it back to `null`, `reference` as a text input, `quantity` as a number input, and `express` as a checkbox. On the broken build every one of them fell through to the raw-JSON textarea, which re-escaped its own contents on each keystroke until the value was unusable ([#1928](https://github.com/modelcontextprotocol/inspector/issues/1928)). The tool echoes the arguments it received, so the result panel shows exactly what was sent. + #### Advertised extensions `advertised-extensions-http.json` serves `echo` (always) and a `get_weather` tool **gated on the `io.modelcontextprotocol/tasks` extension** (`extensionGatedTools`): the tool is registered but starts disabled, and the server enables it on `notifications/initialized` only when the client declared that extension in its `capabilities.extensions`. diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx index 5b9f064a..3ff41b50 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx @@ -811,3 +811,179 @@ describe("SchemaForm", () => { expect(container.firstChild).not.toBeNull(); }); }); + +// #1928: "optional AND explicitly nullable" (Zod's `.nullish()`) compiles to a +// nullable union rather than a plain type. Before the normalization step these +// matched no branch and fell through to the raw-JSON fallback, where every +// keystroke re-escaped the value. +describe("SchemaForm nullable unions", () => { + it("renders a Select for an anyOf string-enum|null field", async () => { + const user = userEvent.setup(); + const onChange = vi.fn(); + const schema: InspectorFormSchema = { + type: "object", + properties: { + type: { + title: "Type", + anyOf: [ + { type: "string", enum: ["envio", "recebimento"] }, + { type: "null" }, + ], + }, + }, + }; + renderWithMantine( + , + ); + await user.click(screen.getByRole("textbox", { name: "Type" })); + await user.click( + await screen.findByRole("option", { name: "envio", hidden: true }), + ); + expect(onChange).toHaveBeenCalledWith({ type: "envio" }); + }); + + it("clears a nullable enum back to null", async () => { + const user = userEvent.setup(); + const onChange = vi.fn(); + const schema: InspectorFormSchema = { + type: "object", + properties: { + type: { + title: "Type", + anyOf: [{ type: "string", enum: ["envio"] }, { type: "null" }], + }, + }, + }; + renderWithMantine( + , + ); + // Mantine marks its combobox clear button `aria-hidden` (it is mouse-only, + // `tabIndex={-1}`), so it is only reachable with `hidden: true`. + await user.click(screen.getByRole("button", { hidden: true })); + expect(onChange).toHaveBeenCalledWith({ type: null }); + }); + + it("offers no clear affordance on a non-nullable enum", () => { + const schema: InspectorFormSchema = { + type: "object", + properties: { + format: { type: "string", title: "Format", enum: ["json"] }, + }, + }; + renderWithMantine( + , + ); + expect( + screen.queryByRole("button", { hidden: true }), + ).not.toBeInTheDocument(); + }); + + it("renders a TextInput for a type: [string, null] field", async () => { + const user = userEvent.setup(); + const onChange = vi.fn(); + const schema: InspectorFormSchema = { + type: "object", + properties: { + note: { type: ["string", "null"], title: "Note" }, + }, + }; + renderWithMantine( + , + ); + await user.type(screen.getByLabelText(/Note/), "a"); + expect(onChange).toHaveBeenCalledWith({ note: "a" }); + }); + + it("renders a checkbox for an anyOf boolean|null field", async () => { + const user = userEvent.setup(); + const onChange = vi.fn(); + const schema: InspectorFormSchema = { + type: "object", + properties: { + enabled: { + title: "Enabled", + anyOf: [{ type: "boolean" }, { type: "null" }], + }, + }, + }; + renderWithMantine( + , + ); + await user.click(screen.getByLabelText("Enabled")); + expect(onChange).toHaveBeenCalledWith({ enabled: true }); + }); + + it("renders a MultiSelect for an anyOf array-of-enum|null field", async () => { + const user = userEvent.setup(); + const onChange = vi.fn(); + const schema: InspectorFormSchema = { + type: "object", + properties: { + tags: { + title: "Tags", + anyOf: [ + { type: "array", items: { type: "string", enum: ["a", "b"] } }, + { type: "null" }, + ], + }, + }, + }; + renderWithMantine( + , + ); + await user.click(screen.getByRole("textbox", { name: "Tags" })); + await user.click( + await screen.findByRole("option", { name: "a", hidden: true }), + ); + expect(onChange).toHaveBeenCalledWith({ tags: ["a"] }); + }); + + it("renders nested fields for an anyOf object|null field", async () => { + const user = userEvent.setup(); + const onChange = vi.fn(); + const schema: InspectorFormSchema = { + type: "object", + properties: { + profile: { + title: "Profile", + anyOf: [ + { + type: "object", + properties: { nick: { type: "string", title: "Nick" } }, + }, + { type: "null" }, + ], + }, + }, + }; + renderWithMantine( + , + ); + await user.type(screen.getByLabelText(/Nick/), "z"); + expect(onChange).toHaveBeenCalledWith({ profile: { nick: "z" } }); + }); + + it("still falls back to the JSON input for a union of two real types", () => { + const schema: InspectorFormSchema = { + type: "object", + properties: { + mixed: { + title: "Mixed", + anyOf: [{ type: "string" }, { type: "number" }], + }, + }, + }; + renderWithMantine( + , + ); + expect(screen.getByLabelText(/Mixed/).tagName).toBe("TEXTAREA"); + }); +}); diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx index 232b5327..eadb0564 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx @@ -12,6 +12,7 @@ import { useState } from "react"; import { ClearButton } from "../../elements/ClearButton/ClearButton"; import { useValueChange } from "../../../hooks/useValueChange"; import type { InspectorFormSchema } from "../../../utils/jsonUtils"; +import { normalizeUnionType } from "../../../utils/schemaUtils"; const FieldLabel = Text.withProps({ fw: 500, @@ -51,6 +52,16 @@ function toEnumData( return values; } +/** + * Whether an enum `Select` should offer a clear affordance. Only a schema that + * admits `null` does: clearing emits `null`, and an enum without a null branch + * would reject it. Without this a nullable enum is a one-way door — once a + * value is picked there is no way back to "no answer". + */ +function isClearable(fieldSchema: InspectorFormSchema): boolean { + return fieldSchema.nullable === true; +} + /** * 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 @@ -207,7 +218,13 @@ export function SchemaForm({ onChange({ ...values, [fieldName]: fieldValue }); } - function renderField(fieldName: string, fieldSchema: InspectorFormSchema) { + function renderField(fieldName: string, rawSchema: InspectorFormSchema) { + // Flatten a nullable union (`anyOf: [X, {type:"null"}]`, `type: [X,"null"]`) + // before dispatching. Every branch below tests a single `type` string, so + // without this an "optional and explicitly nullable" field — what Zod's + // `.nullish()` emits — matches nothing and falls through to the raw-JSON + // fallback, which is unusable for a value the user has to type (#1928). + const fieldSchema = normalizeUnionType(rawSchema); const isRequired = requiredFields.includes(fieldName); const label = fieldSchema.title ?? fieldName; const description = fieldSchema.description; @@ -223,6 +240,7 @@ export function SchemaForm({ withAsterisk={isRequired} disabled={disabled} data={toEnumData(fieldSchema.enum, fieldSchema.enumNames)} + clearable={isClearable(fieldSchema)} value={(rawValue as string) ?? null} onChange={(val) => handleFieldChange(fieldName, val)} /> @@ -243,6 +261,7 @@ export function SchemaForm({ withAsterisk={isRequired} disabled={disabled} data={data} + clearable={isClearable(fieldSchema)} value={(rawValue as string) ?? null} onChange={(val) => handleFieldChange(fieldName, val)} /> diff --git a/clients/web/src/utils/schemaUtils.test.ts b/clients/web/src/utils/schemaUtils.test.ts index 2689f317..59623fdb 100644 --- a/clients/web/src/utils/schemaUtils.test.ts +++ b/clients/web/src/utils/schemaUtils.test.ts @@ -347,6 +347,120 @@ describe("normalizeUnionType", () => { }; expect(normalizeUnionType(schema)).toBe(schema); }); + + it("ignores a two-member anyOf with no null branch", () => { + const schema = { + anyOf: [{ type: "string" as const }, { type: "number" as const }], + }; + expect(normalizeUnionType(schema)).toBe(schema); + }); + + it("ignores an anyOf whose non-null branch has no renderable type (#1928)", () => { + const schema = { + anyOf: [{ const: "only" }, { type: "null" as const }], + }; + expect(normalizeUnionType(schema)).toBe(schema); + }); + + // #1928: the enum lives on the surviving branch, and hoisting it is what makes + // the field render as a Select instead of the raw-JSON fallback. + it("hoists enum out of an anyOf string-enum|null branch", () => { + expect( + normalizeUnionType({ + description: "Direction", + anyOf: [ + { type: "string", enum: ["envio", "recebimento"] }, + { type: "null" }, + ], + }), + ).toEqual({ + type: "string", + description: "Direction", + enum: ["envio", "recebimento"], + anyOf: undefined, + nullable: true, + }); + }); + + it("infers string for a typeless enum branch", () => { + expect( + normalizeUnionType({ + anyOf: [{ enum: ["a", "b"] }, { type: "null" }], + }), + ).toEqual({ + type: "string", + enum: ["a", "b"], + anyOf: undefined, + nullable: true, + }); + }); + + it("hoists items out of an anyOf array|null branch", () => { + expect( + normalizeUnionType({ + anyOf: [ + { type: "array", items: { type: "string", enum: ["a", "b"] } }, + { type: "null" }, + ], + }), + ).toEqual({ + type: "array", + items: { type: "string", enum: ["a", "b"] }, + anyOf: undefined, + nullable: true, + }); + }); + + it("hoists properties out of an anyOf object|null branch", () => { + expect( + normalizeUnionType({ + anyOf: [ + { type: "object", properties: { a: { type: "string" } } }, + { type: "null" }, + ], + }), + ).toEqual({ + type: "object", + properties: { a: { type: "string" } }, + anyOf: undefined, + nullable: true, + }); + }); + + it("normalizes anyOf object|null regardless of branch order", () => { + expect( + normalizeUnionType({ + anyOf: [{ type: "null" }, { type: "object" }], + }), + ).toEqual({ type: "object", anyOf: undefined, nullable: true }); + }); + + it("normalizes type array object|null", () => { + expect( + normalizeUnionType({ + type: ["object", "null"], + properties: { a: { type: "string" } }, + }), + ).toEqual({ + type: "object", + properties: { a: { type: "string" } }, + nullable: true, + }); + }); + + it("normalizes type array array|null", () => { + expect(normalizeUnionType({ type: ["array", "null"] })).toEqual({ + type: "array", + nullable: true, + }); + }); + + it("keeps a type array of two non-null members unchanged", () => { + const schema = { type: ["string", "number"] as const }; + expect(normalizeUnionType({ ...schema, type: [...schema.type] })).toEqual({ + type: ["string", "number"], + }); + }); }); describe("formatFieldLabel", () => { diff --git a/clients/web/src/utils/schemaUtils.ts b/clients/web/src/utils/schemaUtils.ts index 4e578a1c..2624fba1 100644 --- a/clients/web/src/utils/schemaUtils.ts +++ b/clients/web/src/utils/schemaUtils.ts @@ -201,101 +201,81 @@ export function resolveRef( } /** - * Normalizes union types (like string|null from FastMCP) to simple types for form rendering + * The `type` values {@link SchemaForm} dispatches on. `"null"` is deliberately + * absent: it is the branch a nullable union is being flattened *away* from, and + * there is no widget for a field whose only permitted value is `null`. + */ +const RENDERABLE_TYPES = [ + "string", + "number", + "integer", + "boolean", + "array", + "object", +] as const; + +type RenderableType = (typeof RENDERABLE_TYPES)[number]; + +function isRenderableType(type: unknown): type is RenderableType { + return RENDERABLE_TYPES.includes(type as RenderableType); +} + +/** + * Normalizes a nullable union (`string|null` from FastMCP, or anything Zod's + * `.nullish()` / `.nullable()` compiles to) into the plain type the form + * renderer dispatches on. + * + * Two encodings mean the same thing and are both flattened to + * `{ type: , nullable: true }`: + * + * - `anyOf: [, { type: "null" }]` — the branch's *own* keywords are + * hoisted onto the result, because that is where the detail the renderer + * needs lives. A nullable enum compiles to + * `anyOf: [{ type: "string", enum: [...] }, { type: "null" }]`, so hoisting + * `enum` is what makes it a `Select` instead of falling through to the raw + * JSON fallback (#1928). The branch also wins over the wrapper on any shared + * key, matching v1.x's behavior. + * - `type: [, "null"]` — the keywords already sit at the top level, so only + * `type` collapses. + * + * Anything else — a union of two real types, a three-member `anyOf`, a branch + * whose type has no widget — is returned untouched and renders through the + * JSON fallback, which is the honest representation of a shape the form cannot + * model. + * * @param schema The JSON schema to normalize - * @returns A normalized schema or the original schema + * @returns A normalized schema, or the original schema when no nullable-union + * pattern matches */ export function normalizeUnionType( schema: InspectorFormSchema, ): InspectorFormSchema { - // Handle anyOf with exactly string and null (FastMCP pattern) - if ( - schema.anyOf && - schema.anyOf.length === 2 && - schema.anyOf.some((t) => (t as InspectorFormSchema).type === "string") && - schema.anyOf.some((t) => (t as InspectorFormSchema).type === "null") - ) { - return { ...schema, type: "string", anyOf: undefined, nullable: true }; + if (schema.anyOf && schema.anyOf.length === 2) { + // `JsonSchemaConst` is structurally a subset of `InspectorFormSchema` (it + // just carries no `type`), so reading both as the latter is safe. + const branches = schema.anyOf as InspectorFormSchema[]; + const nullBranch = branches.find((branch) => branch?.type === "null"); + const branch = branches.find((candidate) => candidate?.type !== "null"); + + if (nullBranch && branch) { + // A bare `{ enum: [...] }` branch carries no `type`; JSON Schema allows + // that, and every value such a schema admits here is a string. + const type = branch.type ?? (branch.enum ? "string" : undefined); + if (isRenderableType(type)) { + return { ...schema, ...branch, type, anyOf: undefined, nullable: true }; + } + } } - // Handle anyOf with exactly boolean and null (FastMCP pattern) - if ( - schema.anyOf && - schema.anyOf.length === 2 && - schema.anyOf.some((t) => (t as InspectorFormSchema).type === "boolean") && - schema.anyOf.some((t) => (t as InspectorFormSchema).type === "null") - ) { - return { ...schema, type: "boolean", anyOf: undefined, nullable: true }; - } - - // Handle anyOf with exactly number and null (FastMCP pattern) - if ( - schema.anyOf && - schema.anyOf.length === 2 && - schema.anyOf.some((t) => (t as InspectorFormSchema).type === "number") && - schema.anyOf.some((t) => (t as InspectorFormSchema).type === "null") - ) { - return { ...schema, type: "number", anyOf: undefined, nullable: true }; - } - - // Handle anyOf with exactly integer and null (FastMCP pattern) - if ( - schema.anyOf && - schema.anyOf.length === 2 && - schema.anyOf.some((t) => (t as InspectorFormSchema).type === "integer") && - schema.anyOf.some((t) => (t as InspectorFormSchema).type === "null") - ) { - return { ...schema, type: "integer", anyOf: undefined, nullable: true }; - } - - // Handle anyOf with exactly array and null (FastMCP pattern) - if ( - schema.anyOf && - schema.anyOf.length === 2 && - schema.anyOf.some((t) => (t as InspectorFormSchema).type === "array") && - schema.anyOf.some((t) => (t as InspectorFormSchema).type === "null") - ) { - return { ...schema, type: "array", anyOf: undefined, nullable: true }; - } - - // Handle array type with exactly string and null if ( Array.isArray(schema.type) && schema.type.length === 2 && - schema.type.includes("string") && schema.type.includes("null") ) { - return { ...schema, type: "string", nullable: true }; - } - - // Handle array type with exactly boolean and null - if ( - Array.isArray(schema.type) && - schema.type.length === 2 && - schema.type.includes("boolean") && - schema.type.includes("null") - ) { - return { ...schema, type: "boolean", nullable: true }; - } - - // Handle array type with exactly number and null - if ( - Array.isArray(schema.type) && - schema.type.length === 2 && - schema.type.includes("number") && - schema.type.includes("null") - ) { - return { ...schema, type: "number", nullable: true }; - } - - // Handle array type with exactly integer and null - if ( - Array.isArray(schema.type) && - schema.type.length === 2 && - schema.type.includes("integer") && - schema.type.includes("null") - ) { - return { ...schema, type: "integer", nullable: true }; + const type = schema.type.find((member) => member !== "null"); + if (isRenderableType(type)) { + return { ...schema, type, nullable: true }; + } } return schema; diff --git a/test-servers/configs/nullable-fields-http.json b/test-servers/configs/nullable-fields-http.json new file mode 100644 index 00000000..91c4b647 --- /dev/null +++ b/test-servers/configs/nullable-fields-http.json @@ -0,0 +1,11 @@ +{ + "serverInfo": { + "name": "nullable-fields-showcase", + "version": "1.0.0" + }, + "tools": [{ "preset": "record_shipment" }, { "preset": "get_temp" }], + "transport": { + "type": "streamable-http", + "port": 6602 + } +} diff --git a/test-servers/src/preset-registry.ts b/test-servers/src/preset-registry.ts index 806769b3..d78998ec 100644 --- a/test-servers/src/preset-registry.ts +++ b/test-servers/src/preset-registry.ts @@ -12,6 +12,7 @@ import type { } from "./composable-test-server.js"; import { createEchoTool, + createNullableFieldsTool, createGetEnvTool, createAddTool, createGetSumTool, @@ -91,6 +92,9 @@ function resolveToolPreset( switch (name) { case "echo": return createEchoTool(); + case "record_shipment": + case "nullable_fields": + return createNullableFieldsTool(); case "get-env": case "get_env": return createGetEnvTool(); diff --git a/test-servers/src/test-server-fixtures.ts b/test-servers/src/test-server-fixtures.ts index 5a05b07b..070b5750 100644 --- a/test-servers/src/test-server-fixtures.ts +++ b/test-servers/src/test-server-fixtures.ts @@ -162,6 +162,39 @@ export function createEchoTool(): ToolDefinition { }; } +/** + * Create a "record_shipment" tool whose arguments are all **nullable** — the + * shape Zod's `.nullish()` / `.nullable()` compiles to, where the real type + * sits inside `anyOf: [, { type: "null" }]` rather than at the top + * level. + * + * `direction` is the case from #1928: a nullable *enum*, whose `enum` keyword + * lives on the surviving branch. Before the fix nothing in the tool-call form + * matched such a field, so it fell through to the raw-JSON textarea and + * re-escaped its own value on every keystroke. The other three cover the + * remaining scalar branches so a regression in one is visible next to a + * working sibling. + */ +export function createNullableFieldsTool(): ToolDefinition { + return { + name: "record_shipment", + description: + "Record a shipment. Every argument is optional AND explicitly nullable (Zod .nullish()), so each compiles to an anyOf union with a null branch.", + inputSchema: { + direction: z + .enum(["envio", "recebimento"]) + .nullish() + .describe("Direction of the shipment (nullable enum — #1928)"), + reference: z.string().nullish().describe("Free-text reference"), + quantity: z.number().int().nullish().describe("Number of packages"), + express: z.boolean().nullish().describe("Ship express"), + }, + handler: async (params: Record) => { + return toToolResult(JSON.stringify(params, null, 2)); + }, + }; +} + /** * Create a "get-env" tool matching @modelcontextprotocol/server-everything. * Returns the server process environment as pretty-printed JSON text.