From 17a4c78dd3e787293539d619ed3c05cf28c9f251 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sat, 15 Aug 2026 16:59:37 -0400 Subject: [PATCH] fix: centralize the nullable-union collapse in core and apply it in the TUI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #2015, folded into #1928 rather than left as a follow-up. The TUI's `schemaToForm` has the same defect the web `SchemaForm` had: its local `JsonSchemaProperty` declares only `type?: string` — no `anyOf`, no array-valued `type` — so a `.nullish()` argument has no top-level `type` or `enum` for the dispatcher to read and falls to the `switch` default, losing its select. Milder than the web bug (a plain text field, not a corrupting JSON textarea), but the same class. Fixing it in a second place is the argument for not having the logic in a first place. So the collapse moves to `core/json/nullableUnion.ts` as `normalizeNullableUnion`, generic over a minimal `NullableUnionSchema` shape — the least a caller must expose to be *recognized* as a nullable union — so neither client has to adopt the other's schema type. Both form builders now share one copy and cannot drift on which schemas they render. - `core/json/nullableUnion.ts` — the shared collapse, plus tests at `clients/web/src/test/core/nullableUnion.test.ts` (the web suite is what gates `core/`). - `clients/web`: `schemaUtils.normalizeUnionType` is gone; `SchemaForm` and `resolveRefsInMessage` call the core function directly. - `clients/tui`: `schemaToForm` normalizes each property before dispatch, and `JsonSchemaProperty` grows the `anyOf` / array-`type` fields the encoding uses. Verified by driving the built TUI under a pty against the `nullable-fields-http.json` showcase server. Before, every argument of `record_shipment` rendered as a plain text field; after, `direction` is a select ("No value"), `quantity` an integer, `express` a boolean ("[Not set]"). Signed-off-by: cliffhall --- AGENTS.md | 10 +- README.md | 5 +- clients/tui/__tests__/schemaToForm.test.ts | 113 ++++++++ clients/tui/src/utils/schemaToForm.ts | 23 +- .../groups/SchemaForm/SchemaForm.tsx | 4 +- .../web/src/test/core/nullableUnion.test.ts | 250 ++++++++++++++++++ clients/web/src/utils/schemaUtils.test.ts | 221 ---------------- clients/web/src/utils/schemaUtils.ts | 84 +----- core/json/nullableUnion.ts | 120 +++++++++ 9 files changed, 519 insertions(+), 311 deletions(-) create mode 100644 clients/web/src/test/core/nullableUnion.test.ts create mode 100644 core/json/nullableUnion.ts diff --git a/AGENTS.md b/AGENTS.md index 0baea459..87d51230 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -55,7 +55,15 @@ v2/main/ │ │ # annotation scan/validation + mirrored-param │ │ # derivation, used by the Tools tab — #1632; │ │ # plus `Mcp-Param-*` header building for the -│ │ # wire, used by both `tools/call` paths — #1846) +│ │ # wire, used by both `tools/call` paths — #1846; +│ │ # nullableUnion.ts: collapses the two JSON Schema +│ │ # encodings of "T or null" (`anyOf` with a null +│ │ # branch, `type: [T,"null"]`) into a plain type, +│ │ # hoisting the surviving branch's own keywords. +│ │ # Shared by BOTH form builders — web SchemaForm +│ │ # and TUI schemaToForm — since each dispatches on +│ │ # a single `type` string and would otherwise miss +│ │ # a nullable field entirely — #1928/#2015) │ ├── logging/ # Silent pino logger singleton │ ├── mcp/ # InspectorClient runtime + state stores │ │ # (modernTaskSchemas.ts: SEP-2663 modern Tasks diff --git a/README.md b/README.md index 82164a98..467a4fc0 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,8 @@ inspector/ ├── core/ # Shared code consumed via the `@inspector/core` alias (no package.json) │ ├── auth/ # OAuth: providers, discovery, storage, mid-session recovery (browser/node/remote backends) │ ├── client/ # Install-level client config (`client.json`): browser-safe parse/validate + Node load/save, remote backend, secrets -│ ├── json/ # JSON + parameter/argument conversion utilities +│ ├── json/ # JSON + parameter/argument conversion utilities, and the nullable-union +│ │ # schema collapse shared by the web and TUI form builders │ ├── logging/ # Silent pino logger singleton │ ├── mcp/ # InspectorClient runtime, state stores, transports, config import │ ├── node/ # Node-only shared helpers: version reader, hostUrl (host normalize/canonicalize + all-interfaces/loopback detection) @@ -234,6 +235,8 @@ The duplicated copies are appended rather than placed beside their twin on purpo 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. +The **TUI** had the same gap and is worth checking against the same server (`--tui`, then test `record_shipment`): `direction` is a select, `quantity` an integer field, `express` a boolean. Both clients now share one collapse step — `normalizeNullableUnion` in [`core/json/nullableUnion.ts`](./core/json/nullableUnion.ts) — precisely so they cannot drift on which schemas they can render. + #### 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/tui/__tests__/schemaToForm.test.ts b/clients/tui/__tests__/schemaToForm.test.ts index 7efca937..93ca1ca9 100644 --- a/clients/tui/__tests__/schemaToForm.test.ts +++ b/clients/tui/__tests__/schemaToForm.test.ts @@ -213,4 +213,117 @@ describe("schemaToForm", () => { ); expect(form.sections[0]!.fields[0]).toMatchObject({ initialValue: "hi" }); }); + + // #2015 (the TUI twin of #1928): an argument declared with Zod's `.nullish()` + // has no top-level `type`/`enum` — they sit on the surviving `anyOf` branch — + // so before normalization every one of these degraded to a plain text field. + describe("nullable unions", () => { + it("renders a select for an anyOf string-enum|null argument", () => { + const form = schemaToForm( + { + properties: { + direction: { + anyOf: [ + { type: "string", enum: ["envio", "recebimento"] }, + { type: "null" }, + ], + }, + }, + }, + "nullableEnum", + ); + expect(form.sections[0]!.fields[0]).toMatchObject({ + name: "direction", + type: "select", + options: [ + { label: "envio", value: "envio" }, + { label: "recebimento", value: "recebimento" }, + ], + }); + }); + + it("maps the remaining nullable scalar branches to their typed fields", () => { + const form = schemaToForm( + { + properties: { + reference: { anyOf: [{ type: "string" }, { type: "null" }] }, + quantity: { + anyOf: [ + { type: "integer", minimum: 1, maximum: 9 }, + { type: "null" }, + ], + }, + ratio: { anyOf: [{ type: "number" }, { type: "null" }] }, + express: { anyOf: [{ type: "boolean" }, { type: "null" }] }, + }, + }, + "nullableScalars", + ); + const fields = form.sections[0]!.fields; + expect(fields.map((f) => ({ name: f.name, type: f.type }))).toEqual([ + { name: "reference", type: "string" }, + { name: "quantity", type: "integer" }, + { name: "ratio", type: "float" }, + { name: "express", type: "boolean" }, + ]); + // The branch's own constraints are hoisted along with its type. + expect(fields[1]).toMatchObject({ min: 1, max: 9 }); + }); + + it("renders a select for a nullable array-of-enum argument", () => { + const form = schemaToForm( + { + properties: { + tags: { + anyOf: [ + { type: "array", items: { enum: ["a", "b"] } }, + { type: "null" }, + ], + }, + }, + }, + "nullableArrayEnum", + ); + expect(form.sections[0]!.fields[0]).toMatchObject({ + type: "select", + options: [ + { label: "a", value: "a" }, + { label: "b", value: "b" }, + ], + }); + }); + + it("collapses the type: [T, null] encoding too", () => { + const form = schemaToForm( + { + properties: { + note: { type: ["string", "null"] }, + flag: { type: ["boolean", "null"] }, + }, + }, + "typeArrayNull", + ); + expect( + form.sections[0]!.fields.map((f) => ({ name: f.name, type: f.type })), + ).toEqual([ + { name: "note", type: "string" }, + { name: "flag", type: "boolean" }, + ]); + }); + + it("leaves a union of two real types as a plain string field", () => { + const form = schemaToForm( + { + properties: { + mixed: { anyOf: [{ type: "string" }, { type: "number" }] }, + }, + }, + "realUnion", + ); + expect(form.sections[0]!.fields[0]).toMatchObject({ + name: "mixed", + type: "string", + }); + }); + }); }); diff --git a/clients/tui/src/utils/schemaToForm.ts b/clients/tui/src/utils/schemaToForm.ts index bbb730f2..5201abbb 100644 --- a/clients/tui/src/utils/schemaToForm.ts +++ b/clients/tui/src/utils/schemaToForm.ts @@ -3,10 +3,16 @@ */ import type { FormStructure, FormSection, FormField } from "ink-form"; +import { normalizeNullableUnion } from "@inspector/core/json/nullableUnion.js"; /** Minimal JSON Schema property shape used when building tool parameter forms */ interface JsonSchemaProperty { - type?: string; + /** + * An array here is the `["string", "null"]` nullable encoding, which + * {@link normalizeNullableUnion} collapses to a single name before the + * dispatch below reads it. + */ + type?: string | string[]; title?: string; enum?: unknown[]; /** Non-standard legacy support: titles for enum values */ @@ -15,6 +21,8 @@ interface JsonSchemaProperty { minimum?: number; maximum?: number; default?: unknown; + /** Present on a nullable union; see {@link normalizeNullableUnion}. */ + anyOf?: readonly unknown[]; } /** @@ -68,9 +76,16 @@ export function schemaToForm( // `properties` values are `unknown` (the SDK schema admits anything), so // guard before treating a value as a schema object — a malformed server // schema with e.g. `properties: { foo: null }` must not throw on `.title`. - const property = ( - typeof prop === "object" && prop !== null ? prop : {} - ) as JsonSchemaProperty; + // Flatten a nullable union (`anyOf: [X, {type:"null"}]`, `type: [X,"null"]`) + // before dispatching. Every branch below reads a single `type` string, and + // for an enum the `enum` keyword sits on the union's surviving branch — so + // without this an argument declared with Zod's `.nullish()` loses its + // select and degrades to a plain text field (#2015, the TUI twin of #1928). + const property = normalizeNullableUnion( + (typeof prop === "object" && prop !== null + ? prop + : {}) as JsonSchemaProperty, + ); const baseField = { name: key, label: property.title || key, diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx index eadb0564..628f2ee3 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx @@ -12,7 +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"; +import { normalizeNullableUnion } from "@inspector/core/json/nullableUnion.js"; const FieldLabel = Text.withProps({ fw: 500, @@ -224,7 +224,7 @@ export function SchemaForm({ // 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 fieldSchema = normalizeNullableUnion(rawSchema); const isRequired = requiredFields.includes(fieldName); const label = fieldSchema.title ?? fieldName; const description = fieldSchema.description; diff --git a/clients/web/src/test/core/nullableUnion.test.ts b/clients/web/src/test/core/nullableUnion.test.ts new file mode 100644 index 00000000..f56abbdd --- /dev/null +++ b/clients/web/src/test/core/nullableUnion.test.ts @@ -0,0 +1,250 @@ +import { describe, it, expect } from "vitest"; +import { normalizeNullableUnion } from "@inspector/core/json/nullableUnion.js"; + +// The two JSON Schema encodings of "this type, or null". Both form builders — +// the web `SchemaForm` and the TUI's `schemaToForm` — dispatch on a single +// `type` string, so this collapse is what keeps a nullable field renderable +// (#1928 in the web client, #2015 in the TUI). +describe("normalizeNullableUnion", () => { + it("normalizes anyOf string|null", () => { + expect( + normalizeNullableUnion({ + anyOf: [{ type: "string" }, { type: "null" }], + }), + ).toEqual({ + type: "string", + anyOf: undefined, + nullable: true, + }); + }); + + it("normalizes anyOf boolean|null", () => { + expect( + normalizeNullableUnion({ + anyOf: [{ type: "boolean" }, { type: "null" }], + }), + ).toEqual({ + type: "boolean", + anyOf: undefined, + nullable: true, + }); + }); + + it("normalizes anyOf number|null", () => { + expect( + normalizeNullableUnion({ + anyOf: [{ type: "number" }, { type: "null" }], + }), + ).toEqual({ + type: "number", + anyOf: undefined, + nullable: true, + }); + }); + + it("normalizes anyOf integer|null", () => { + expect( + normalizeNullableUnion({ + anyOf: [{ type: "integer" }, { type: "null" }], + }), + ).toEqual({ + type: "integer", + anyOf: undefined, + nullable: true, + }); + }); + + it("normalizes anyOf array|null", () => { + expect( + normalizeNullableUnion({ + anyOf: [{ type: "array" }, { type: "null" }], + }), + ).toEqual({ + type: "array", + anyOf: undefined, + nullable: true, + }); + }); + + it("normalizes type array string|null", () => { + expect(normalizeNullableUnion({ type: ["string", "null"] })).toEqual({ + type: "string", + nullable: true, + }); + }); + + it("normalizes type array boolean|null", () => { + expect(normalizeNullableUnion({ type: ["boolean", "null"] })).toEqual({ + type: "boolean", + nullable: true, + }); + }); + + it("normalizes type array number|null", () => { + expect(normalizeNullableUnion({ type: ["number", "null"] })).toEqual({ + type: "number", + nullable: true, + }); + }); + + it("normalizes type array integer|null", () => { + expect(normalizeNullableUnion({ type: ["integer", "null"] })).toEqual({ + type: "integer", + nullable: true, + }); + }); + + it("returns schema unchanged when no union pattern matches", () => { + const schema = { type: "string" as const }; + expect(normalizeNullableUnion(schema)).toBe(schema); + }); + + it("ignores anyOf with more than two members", () => { + const schema = { + anyOf: [ + { type: "string" as const }, + { type: "null" as const }, + { type: "number" as const }, + ], + }; + expect(normalizeNullableUnion(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(normalizeNullableUnion(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(normalizeNullableUnion(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( + normalizeNullableUnion({ + 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( + normalizeNullableUnion({ + 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( + normalizeNullableUnion({ + 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( + normalizeNullableUnion({ + 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( + normalizeNullableUnion({ + anyOf: [{ type: "null" }, { type: "object" }], + }), + ).toEqual({ type: "object", anyOf: undefined, nullable: true }); + }); + + it("normalizes type array object|null", () => { + expect( + normalizeNullableUnion({ + type: ["object", "null"], + properties: { a: { type: "string" } }, + }), + ).toEqual({ + type: "object", + properties: { a: { type: "string" } }, + nullable: true, + }); + }); + + it("normalizes type array array|null", () => { + expect(normalizeNullableUnion({ 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( + normalizeNullableUnion({ ...schema, type: [...schema.type] }), + ).toEqual({ + type: ["string", "number"], + }); + }); + + // A server can put anything in `anyOf`; a non-object member must not be read + // as a branch (nor throw), it simply means no nullable union was recognized. + it("ignores an anyOf holding a non-object member", () => { + const schema = { anyOf: ["string", { type: "null" as const }] }; + expect(normalizeNullableUnion(schema)).toBe(schema); + }); + + it("ignores an anyOf holding an array member", () => { + const schema = { anyOf: [["string"], { type: "null" as const }] }; + expect(normalizeNullableUnion(schema)).toBe(schema); + }); + + it("ignores a type array whose non-null member has no widget", () => { + const schema = { type: ["null", "null"] }; + expect(normalizeNullableUnion(schema)).toBe(schema); + }); + + it("ignores a schema with no union keywords at all", () => { + const schema = { type: "object" as const, properties: {} }; + expect(normalizeNullableUnion(schema)).toBe(schema); + }); +}); diff --git a/clients/web/src/utils/schemaUtils.test.ts b/clients/web/src/utils/schemaUtils.test.ts index 59623fdb..7ff38c88 100644 --- a/clients/web/src/utils/schemaUtils.test.ts +++ b/clients/web/src/utils/schemaUtils.test.ts @@ -8,7 +8,6 @@ import { generateDefaultValue, isPropertyRequired, resolveRef, - normalizeUnionType, formatFieldLabel, resolveRefsInMessage, } from "./schemaUtils"; @@ -243,226 +242,6 @@ describe("resolveRef", () => { }); }); -describe("normalizeUnionType", () => { - it("normalizes anyOf string|null", () => { - expect( - normalizeUnionType({ - anyOf: [{ type: "string" }, { type: "null" }], - }), - ).toEqual({ - type: "string", - anyOf: undefined, - nullable: true, - }); - }); - - it("normalizes anyOf boolean|null", () => { - expect( - normalizeUnionType({ - anyOf: [{ type: "boolean" }, { type: "null" }], - }), - ).toEqual({ - type: "boolean", - anyOf: undefined, - nullable: true, - }); - }); - - it("normalizes anyOf number|null", () => { - expect( - normalizeUnionType({ - anyOf: [{ type: "number" }, { type: "null" }], - }), - ).toEqual({ - type: "number", - anyOf: undefined, - nullable: true, - }); - }); - - it("normalizes anyOf integer|null", () => { - expect( - normalizeUnionType({ - anyOf: [{ type: "integer" }, { type: "null" }], - }), - ).toEqual({ - type: "integer", - anyOf: undefined, - nullable: true, - }); - }); - - it("normalizes anyOf array|null", () => { - expect( - normalizeUnionType({ - anyOf: [{ type: "array" }, { type: "null" }], - }), - ).toEqual({ - type: "array", - anyOf: undefined, - nullable: true, - }); - }); - - it("normalizes type array string|null", () => { - expect(normalizeUnionType({ type: ["string", "null"] })).toEqual({ - type: "string", - nullable: true, - }); - }); - - it("normalizes type array boolean|null", () => { - expect(normalizeUnionType({ type: ["boolean", "null"] })).toEqual({ - type: "boolean", - nullable: true, - }); - }); - - it("normalizes type array number|null", () => { - expect(normalizeUnionType({ type: ["number", "null"] })).toEqual({ - type: "number", - nullable: true, - }); - }); - - it("normalizes type array integer|null", () => { - expect(normalizeUnionType({ type: ["integer", "null"] })).toEqual({ - type: "integer", - nullable: true, - }); - }); - - it("returns schema unchanged when no union pattern matches", () => { - const schema = { type: "string" as const }; - expect(normalizeUnionType(schema)).toBe(schema); - }); - - it("ignores anyOf with more than two members", () => { - const schema = { - anyOf: [ - { type: "string" as const }, - { type: "null" as const }, - { type: "number" as const }, - ], - }; - 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", () => { it("inserts spaces before capitals and capitalizes first letter", () => { expect(formatFieldLabel("firstName")).toBe("First Name"); diff --git a/clients/web/src/utils/schemaUtils.ts b/clients/web/src/utils/schemaUtils.ts index 2624fba1..b03582c0 100644 --- a/clients/web/src/utils/schemaUtils.ts +++ b/clients/web/src/utils/schemaUtils.ts @@ -3,6 +3,7 @@ import Ajv from "ajv"; import type { ValidateFunction } from "ajv"; import type { Tool, JSONRPCMessage } from "@modelcontextprotocol/client"; import { isJSONRPCRequest } from "@modelcontextprotocol/client"; +import { normalizeNullableUnion } from "@inspector/core/json/nullableUnion.js"; const ajv = new Ajv(); @@ -200,87 +201,6 @@ export function resolveRef( return schema; } -/** - * 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 when no nullable-union - * pattern matches - */ -export function normalizeUnionType( - schema: InspectorFormSchema, -): InspectorFormSchema { - 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 }; - } - } - } - - if ( - Array.isArray(schema.type) && - schema.type.length === 2 && - schema.type.includes("null") - ) { - const type = schema.type.find((member) => member !== "null"); - if (isRenderableType(type)) { - return { ...schema, type, nullable: true }; - } - } - - return schema; -} - /** * Formats a field key into a human-readable label * @param key The field key to format @@ -319,7 +239,7 @@ export function resolveRefsInMessage(message: JSONRPCMessage): JSONRPCMessage { Object.entries(requestedSchema.properties).map( ([key, propSchema]) => { const resolved = resolveRef(propSchema, requestedSchema); - const normalized = normalizeUnionType(resolved); + const normalized = normalizeNullableUnion(resolved); return [key, normalized]; }, ), diff --git a/core/json/nullableUnion.ts b/core/json/nullableUnion.ts new file mode 100644 index 00000000..4e81b791 --- /dev/null +++ b/core/json/nullableUnion.ts @@ -0,0 +1,120 @@ +/** + * Flattening of the two JSON Schema encodings of "this type, **or** null" into + * the plain type a form renderer can dispatch on. + * + * Shared because both form builders — the web client's `SchemaForm` and the + * TUI's `schemaToForm` — dispatch on a *single* `type` string, and so both miss + * a nullable field entirely without this step (#1928, #2015). Keeping one copy + * is what stops the two clients from disagreeing about which schemas they can + * render; the alternative was two implementations of the same subtle predicate. + */ + +/** + * JSON Schema type names a form renderer can build a widget for. + * + * `"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); +} + +/** + * The keywords {@link normalizeNullableUnion} reads. Both clients' own schema + * types are structurally assignable to this, so neither has to adopt the + * other's — the shape is deliberately the minimum needed to *recognize* a + * nullable union, not a full JSON Schema model. + */ +export interface NullableUnionSchema { + type?: string | string[]; + enum?: unknown[]; + anyOf?: readonly unknown[]; +} + +/** Narrow an `anyOf` member to a readable object, or `null` if it isn't one. */ +function toBranch(value: unknown): Record | null { + if (typeof value !== "object" || value === null || Array.isArray(value)) { + return null; + } + return value as Record; +} + +/** + * Collapse a nullable union — what Zod's `.nullish()` / `.nullable()` and + * FastMCP's optional arguments emit — into `{ type: , nullable: true }`. + * + * Two encodings mean the same thing and are both handled: + * + * - `anyOf: [, { type: "null" }]` — the branch's *own* keywords are + * hoisted onto the result, because that is where the detail a renderer needs + * lives. A nullable enum compiles to + * `anyOf: [{ type: "string", enum: [...] }, { type: "null" }]`, so hoisting + * `enum` is what makes it a dropdown rather than a raw-JSON fallback (#1928). + * The branch also wins over the wrapper on any shared key, matching v1.x. + * - `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 **by identity**, so a caller can use + * `===` to tell that nothing was recognized. + * + * @param schema The schema to normalize + * @returns A flattened copy, or `schema` itself when no nullable-union pattern + * matches + */ +export function normalizeNullableUnion( + schema: T, +): T { + if (schema.anyOf?.length === 2) { + const branches = schema.anyOf.map(toBranch); + const nullBranch = branches.find((entry) => entry?.type === "null"); + const branch = branches.find( + (entry) => entry !== null && entry.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)) { + // The result is `schema` with `type` narrowed to a value it already + // admitted, the branch's keywords merged in, and `nullable` added. TS + // cannot express "a spread of T and a subset of T is still a T", and a + // non-generic return would force every caller to cast instead. + return { + ...schema, + ...branch, + type, + anyOf: undefined, + nullable: true, + } as T; + } + } + } + + if ( + Array.isArray(schema.type) && + schema.type.length === 2 && + schema.type.includes("null") + ) { + const type = schema.type.find((member) => member !== "null"); + if (isRenderableType(type)) { + // Same reasoning as above. + return { ...schema, type, nullable: true } as T; + } + } + + return schema; +}