Merge pull request #2276 from modelcontextprotocol/v2/fix/2224-union-required-undeclared

fix: decline a union branch requiring a name it never declares
This commit is contained in:
Cliff Hall 2026-09-06 20:00:53 -04:00 • committed by GitHub
commit eab42d393d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 330 additions and 6 deletions

View file

@ -656,6 +656,58 @@ describe("schemaToForm", () => {
expect(names).toContain("__b_0____b0__x");
});
// #2224: `required` lists names, not declarations, so a member may require
// one it never declares. `buildFields` enumerates `properties` alone, so no
// control was rendered for it while `missingRequiredFields` reported it
// missing at every submit — a section the user could never complete.
it("declines a member requiring a name it never declares", () => {
const schema = {
type: "object",
anyOf: [
{
type: "object",
properties: { kind: { type: "string", const: "a" } },
required: ["kind", "payload"],
},
{
type: "object",
properties: { kind: { type: "string", const: "b" } },
required: ["kind"],
},
],
};
const form = schemaToForm(schema, "undeclared_required");
// No variant select and no per-branch section: the union is declined, so
// the root's own (here empty) properties are what render.
expect(form.sections).toEqual([{ title: "Parameters", fields: [] }]);
// And nothing is reported missing, so the call is no longer blocked on a
// field that has nowhere to be typed.
expect(
missingRequiredFields(schema, decodeFormValues(schema, {})),
).toEqual([]);
});
it("keeps offering a member requiring a name the root declares", () => {
// The regression guard for the check above: the merge is what is judged,
// and `anyOf: [{ required: ["email"] }, …]` is an ordinary union.
const form = schemaToForm(
{
type: "object",
properties: {
email: { type: "string" },
phone: { type: "string" },
},
anyOf: [
{ type: "object", required: ["email"] },
{ type: "object", required: ["phone"] },
],
},
"inherited_required",
);
expect(form.sections).toHaveLength(3);
expect(form.sections[0]!.fields[0]).toMatchObject({ name: "__variant" });
});
describe("decodeFormValues", () => {
it("submits the chosen branch's fields under their real names", () => {
expect(

View file

@ -2284,6 +2284,86 @@ describe("SchemaForm raw JSON (#2151)", () => {
await enableRawJson(user);
expect(screen.getAllByLabelText("Edit as JSON")).toHaveLength(1);
});
// #2224: a root union whose alternatives are all declined resolves to a base
// with nothing on it, so the form renders no picker and no fields. The switch
// is the only way to make the call — open it rather than leaving the user in
// front of a blank form to work that out.
describe("fallback for an unrenderable root union (#2224)", () => {
// Every member requires a name it never declares, so none is offerable.
const deadEnd: InspectorFormSchema = {
type: "object",
anyOf: [
{
type: "object",
properties: { kind: { type: "string", const: "a" } },
required: ["kind", "payload"],
},
{
type: "object",
properties: { kind: { type: "string", const: "b" } },
required: ["kind", "payload"],
},
],
};
function switchElement(): HTMLInputElement {
return screen.getByLabelText("Edit as JSON") as HTMLInputElement;
}
it("opens the editor when the form would otherwise be empty", () => {
renderWithMantine(<RawHarness schema={deadEnd} />);
expect(switchElement().checked).toBe(true);
expect(getAceTextByLabel(/Arguments JSON/)).toBe("{}");
});
it("leaves the editor closed for a tool that takes no arguments", () => {
// Nothing to render here either, but nothing is missing: seeding a JSON
// editor for `{}` would be noise on every no-argument tool.
renderWithMantine(
<RawHarness schema={{ type: "object", properties: {} }} />,
);
expect(switchElement().checked).toBe(false);
});
it("still lets the user switch back to the fields", async () => {
const user = userEvent.setup();
renderWithMantine(<RawHarness schema={deadEnd} />);
await user.click(switchElement());
expect(switchElement().checked).toBe(false);
});
it("opens the editor for a schema that arrives after the first render", () => {
// The form is reused across tools rather than remounted, so a `useState`
// initializer alone would only ever see the tool it mounted on.
const { rerender } = renderWithMantine(
<SchemaForm schema={schema} values={{}} onChange={vi.fn()} />,
);
expect(switchElement().checked).toBe(false);
rerender(<SchemaForm schema={deadEnd} values={{}} onChange={vi.fn()} />);
expect(switchElement().checked).toBe(true);
});
it("does not close an editor the user opened when the schema changes", async () => {
// One-way: the fallback opens the editor, and nothing closes it but the
// user.
const user = userEvent.setup();
const { rerender } = renderWithMantine(
<SchemaForm schema={schema} values={{}} onChange={vi.fn()} />,
);
await enableRawJson(user);
rerender(
<SchemaForm
schema={{ type: "object", properties: {} }}
values={{}}
onChange={vi.fn()}
/>,
);
expect(switchElement().checked).toBe(true);
});
});
});
// A plain `<input>` swallows Enter, so a string argument could not be given a

View file

@ -931,7 +931,26 @@ export function SchemaForm({
// rather than reported under a reserved name: field names come straight out
// of a server's schema, so any sentinel this form invented could collide with
// a real argument and clear a block the user cannot see.
const [rawJsonMode, setRawJsonMode] = useState(false);
//
// A root union whose alternatives are all declined resolves to a base with
// nothing on it, so the form renders neither a picker nor a field — the user
// is left staring at a switch they have to know to reach for in order to make
// the call at all (#2224). Open the editor for them in that one case: the
// arguments are still expressible, just not as fields. Seeded rather than
// forced, so the switch keeps working; re-seeded during render (never in an
// effect) because this form is reused across tools rather than remounted, and
// a `useState` initializer would only ever see the first one.
const rawJsonFallback =
allowRawJson &&
branches.length === 0 &&
(schema.oneOf !== undefined || schema.anyOf !== undefined) &&
Object.keys(properties).length === 0;
const [rawJsonMode, setRawJsonMode] = useState(rawJsonFallback);
useValueChange(rawJsonFallback, (fallback) => {
// One-way: a schema that stops needing the fallback does not close an
// editor the user may have opened on purpose.
if (fallback) setRawJsonMode(true);
});
const [rawJsonInvalid, setRawJsonInvalid] = useState(false);
// Stable so `RawArgumentsField`'s reporting effect subscribes once rather

View file

@ -809,6 +809,84 @@ describe("resolveRootUnion", () => {
expect(branches).toHaveLength(2);
});
it("declines a member requiring a name nothing declares", () => {
// `required` is a list of names, not of declarations. Both form builders
// enumerate `properties` alone, so `payload` gets no control while the
// submit gate reports it missing forever — the picker offers an option
// that can never be completed (#2224).
const { branches } = resolveRootUnion({
type: "object",
anyOf: [
{
type: "object",
properties: { kind: { const: "a" } },
required: ["kind", "payload"],
},
SMS,
],
});
expect(branches).toEqual([]);
});
it("declines a member requiring a name only ANOTHER member declares", () => {
// Branches are alternatives, not a conjunction: a name the sibling
// declares is not one this branch can render, so the dead end is the
// same one.
const { branches } = resolveRootUnion({
type: "object",
anyOf: [
{
type: "object",
properties: { kind: { const: "a" } },
required: ["kind", "phone"],
},
SMS,
],
});
expect(branches).toEqual([]);
});
it("offers a member requiring a name it inherits from the root", () => {
// The merge is what is judged, so a member requiring a name the ROOT
// declares stays offerable — the `anyOf: [{ required: ["email"] }, …]`
// shape above must not regress.
const { branches } = resolveRootUnion({
type: "object",
properties: { token: { type: "string" } },
anyOf: [
{
type: "object",
properties: { a: { type: "string" } },
required: ["token"],
},
{
type: "object",
properties: { b: { type: "string" } },
required: ["token"],
},
],
});
expect(branches).toHaveLength(2);
});
it("does not read an inherited name as a declared property", () => {
// An argument legally named `constructor` resolves through the prototype
// on a plain object, so a membership test that is not `hasOwn` would call
// this branch offerable and render nothing for it.
const { branches } = resolveRootUnion({
type: "object",
anyOf: [
{
type: "object",
properties: { kind: { const: "a" } },
required: ["constructor"],
},
SMS,
],
});
expect(branches).toEqual([]);
});
it("declines a member carrying a `false` property schema", () => {
// `false` admits no value at all, so the field can never be filled — and
// a required one makes the branch unsatisfiable.
@ -919,6 +997,24 @@ describe("resolveRootUnion", () => {
).toBe(true);
});
it("still counts a union whose every branch is now declined", () => {
// The #2224 check makes such a union render nothing, which must not be
// read as "this tool takes no arguments" — an App tool carrying one would
// then be auto-invoked with `{}` rather than asked about.
expect(
declaresAnyFields({
type: "object",
oneOf: [
{
type: "object",
properties: { kind: { const: "a" } },
required: ["kind", "payload"],
},
],
}),
).toBe(true);
});
it("counts a required name a schema never declares", () => {
// Legal, and the tool plainly takes an argument — an App tool shaped this
// way must ask rather than being auto-invoked with `{}`.

View file

@ -162,7 +162,7 @@ function propertiesOf(schema: RootUnionSchema): Record<string, unknown> {
/**
* Whether a branch is one a form can offer as an alternative.
*
* Three ways it is not, all of which would put an option in the picker that
* Four ways it is not, all of which would put an option in the picker that
* cannot be filled in:
*
* - **It carries no fields.** A `{ type: "null" }` member — the nullable
@ -174,6 +174,12 @@ function propertiesOf(schema: RootUnionSchema): Record<string, unknown> {
* - **Its `type` rules objects out.** Tool arguments are a JSON object, so a
* `{ type: "string", properties: {…} }` member can never match — rendering it
* as a fillable form would offer a call that cannot be valid.
* - **It requires a name it never declares.** `required` is a list of names,
* not of declarations, so `{ properties: { kind: … }, required: ["kind",
* "payload"] }` is legal and says nothing about what `payload` accepts. Both
* form builders enumerate `properties` alone, so no control is rendered for
* it and the submit-time check reports it missing *permanently* — the same
* dead end a `false`-schema required field produces (#2224).
*/
function isOfferable(
branch: RootUnionSchema,
@ -191,7 +197,19 @@ function isOfferable(
// a perfectly ordinary way to say "one of these two" — and judging it on its
// own properties would decline it, leaving the gate checking the base alone
// and accepting `{}`, which the schema rejects.
const properties = Object.values(propertiesOf(merged));
const mergedProperties = propertiesOf(merged);
// Judged on the MERGE for the same reason the fields are: a member may
// require a name the root declares, which is exactly the
// `anyOf: [{ required: ["email"] }, …]` shape above. `hasOwn` rather than
// `in`, since an argument legally named `constructor` or `toString` would
// otherwise resolve to the inherited one and read as declared.
if (
requiredOf(merged).some((name) => !Object.hasOwn(mergedProperties, name))
) {
return false;
}
const properties = Object.values(mergedProperties);
return (
properties.length > 0 &&
// Every value has to be something a renderer can read AND something a

View file

@ -45,7 +45,7 @@ as a missing capability rather than an error.
| `structured-output-http.json` **(legacy era)** | Tools tab: a result's `structuredContent` section | [#1908](https://github.com/modelcontextprotocol/inspector/issues/1908) |
| `duplicate-tool-names-http.json` **(legacy era)** | A `tools/list` that repeats a tool name | [#1957](https://github.com/modelcontextprotocol/inspector/issues/1957) |
| `nullable-fields-http.json` **(legacy era)** | Tools tab: nullable (`anyOf` + `null`) arguments | [#1928](https://github.com/modelcontextprotocol/inspector/issues/1928) |
| `root-union-schemas-http.json` **(legacy era)** | Tool schemas whose arguments are a root `anyOf` / `oneOf` | [#2123](https://github.com/modelcontextprotocol/inspector/issues/2123) |
| `root-union-schemas-http.json` **(legacy era)** | Tool schemas whose arguments are a root `anyOf` / `oneOf`, including one no branch of which can be offered | [#2123](https://github.com/modelcontextprotocol/inspector/issues/2123), [#2224](https://github.com/modelcontextprotocol/inspector/issues/2224) |
| `unportable-schemas-http.json` **(legacy era)** | Tool schemas a real client rejects, flagged in all three clients | [#1005](https://github.com/modelcontextprotocol/inspector/issues/1005) |
| `rfc6570-templates-http.json` **(legacy era)** | Resources tab: RFC 6570 resource-template expansion | [#1919](https://github.com/modelcontextprotocol/inspector/issues/1919) |
| `advertised-extensions-http.json` **(legacy era)** | Tool registration gated on advertised extensions | [#1739](https://github.com/modelcontextprotocol/inspector/issues/1739) |
@ -280,7 +280,7 @@ The **TUI** had the same gap and is worth checking against the same server (`--t
## Root-level unions
`root-union-schemas-http.json` serves two tools whose arguments are declared as a **composition at the root** of `inputSchema` rather than as a flat `properties` map — `echo` with an `anyOf` beside its own `message` property, and `get_weather` with an OpenAPI-style `discriminator` over a `oneOf`. Plain streamable-HTTP — connect with the **default (legacy)** protocol era.
`root-union-schemas-http.json` serves three tools whose arguments are declared as a **composition at the root** of `inputSchema` rather than as a flat `properties` map — `echo` with an `anyOf` beside its own `message` property, `get_weather` with an OpenAPI-style `discriminator` over a `oneOf`, and `record_shipment_by` with a `oneOf` neither of whose branches can be offered. Plain streamable-HTTP — connect with the **default (legacy)** protocol era.
The 2026-07-28 revision makes this shape explicitly legal: `type: "object"` is required at the root, and beyond that "any JSON Schema 2020-12 keyword may appear alongside `type`, including composition keywords (`oneOf`, `anyOf`, `allOf`, `not`)".
@ -301,6 +301,7 @@ All three read one helper, [`core/json/rootUnion.ts`](../core/json/rootUnion.ts)
What it declines to flatten is as deliberate as what it flattens, and every case falls back to whatever the schema's own `properties` describe rather than claiming something untrue:
- **A union whose members are not all field-carrying object schemas** — including one whose member `type` rules objects out, since tool arguments are a JSON object and such a member can never match. A picker whose options render nothing is no better than no picker.
- **A branch requiring a name nothing declares.** `required` lists names, not declarations, so `{ "properties": { "by": … }, "required": ["by", "address"] }` is legal and says nothing about what `address` accepts. Every form builder enumerates `properties` alone, so no control is rendered for it while the submit-time check reports it missing *permanently* — an option the picker offers and the user can never complete ([#2224](https://github.com/modelcontextprotocol/inspector/issues/2224)). The name only has to be declared *somewhere the merge reaches*: a branch requiring one the **root** declares — `anyOf: [{ "required": ["email"] }, …]`, an ordinary way to say "one of these two" — is offered as before.
- **A branch that restates a constraint the root already states.** The two are conjunctive, so root `minimum: 10` under branch `minimum: 0` is still 10, disjoint `enum`s leave nothing satisfiable, and `type: "string"` under `type: "number"` describes a value that cannot exist — rendering either side would accept what the schema rejects. A property both declare *compatibly* is merged rather than replaced, so a root's `minimum` survives a branch's `maximum`, and a disagreement about `title`/`description` is not a conflict at all.
- **A composition member stating anything the merge cannot apply.** Only `type`, `properties` and `required` are folded in, so a member carrying a nested `allOf`/`anyOf`, a `not`, an `additionalProperties`, or a `$ref` would have that constraint erased along with the keyword — turning an unsatisfiable schema (`allOf: [false, …]` admits nothing) into a fillable form. `allOf` members are checked against the accumulated merge rather than the root alone, so two of them contradicting each other is caught even when neither contradicts the root.
- **A `oneOf` whose alternatives are not mutually exclusive.** `oneOf` demands that *exactly one* alternative match, which flattening cannot preserve — the branches are offered as if any would do. It is only safe with a discriminator: a property every branch pins to a `const` of its own **and requires**, since an optional one leaves `{}` matching every branch. An undiscriminated `oneOf` is declined; `anyOf` makes no such claim and is offered either way.
@ -310,6 +311,8 @@ What it declines to flatten is as deliberate as what it flattens, and every case
Declining changes what *renders*, never whether the tool is treated as taking arguments: a declined union still has fields, so an App tool carrying one still asks for them rather than auto-invoking with `{}`.
`record_shipment_by` is the case where declining leaves nothing on screen: its fields live entirely on branches that are all declined, so the web form has neither a picker nor a field to show. It opens the **Edit as JSON** editor instead of rendering a blank form the user would have to work out for themselves ([#2224](https://github.com/modelcontextprotocol/inspector/issues/2224)) — the arguments are still expressible, just not as fields. The switch is only *seeded*, so turning it back off works normally, and a tool that genuinely takes no arguments is left alone. The TUI has no such editor, so there the tool renders an empty Parameters section — no longer a form with a branch section that could never be submitted.
## Unportable tool schemas
`unportable-schemas-http.json` serves four tools, three of whose advertised

View file

@ -3,7 +3,11 @@
"name": "root-union-schemas-showcase",
"version": "1.0.0"
},
"tools": [{ "preset": "echo" }, { "preset": "get_weather" }],
"tools": [
{ "preset": "echo" },
{ "preset": "get_weather" },
{ "preset": "record_shipment_by" }
],
"rawToolSchemas": {
"echo": {
"inputSchema": {
@ -32,6 +36,26 @@
]
}
},
"record_shipment_by": {
"inputSchema": {
"type": "object",
"discriminator": { "propertyName": "by" },
"oneOf": [
{
"type": "object",
"title": "By address",
"properties": { "by": { "type": "string", "const": "address" } },
"required": ["by", "address"]
},
{
"type": "object",
"title": "By tracking number",
"properties": { "by": { "type": "string", "const": "tracking" } },
"required": ["by", "tracking"]
}
]
}
},
"get_weather": {
"inputSchema": {
"type": "object",

View file

@ -16,6 +16,7 @@ import {
createGetEnvTool,
createAddTool,
createGetSumTool,
createDeadEndUnionTool,
createGetWeatherTool,
createInvalidHeaderTool,
createSpecErrorTriggerTool,
@ -111,6 +112,8 @@ function resolveToolPreset(
return createGetSumTool();
case "get_weather":
return createGetWeatherTool();
case "record_shipment_by":
return createDeadEndUnionTool();
case "invalid_header_tool":
return createInvalidHeaderTool();
case "trigger_header_mismatch":

View file

@ -272,6 +272,35 @@ export function createGetWeatherTool(): ToolDefinition {
};
}
/**
* Create a tool whose advertised arguments are a root `oneOf` **no branch of
* which can be offered**, for #2224. Each branch requires a name it never
* declares, so a form builder that enumerates `properties` alone renders no
* control for it while the submit gate reports it missing forever.
*
* The Zod `inputSchema` here is a placeholder: the shape that matters is the
* raw JSON Schema `root-union-schemas-http.json` substitutes through
* `rawToolSchemas`, since Zod cannot emit a root composition. The handler is
* what a real server would do with the arguments — echo them back, so what the
* form actually sent is visible in the result.
*/
export function createDeadEndUnionTool(): ToolDefinition {
return {
name: "record_shipment_by",
description:
"Record a shipment, by address or by tracking number. Its advertised schema is a root oneOf whose branches each require a name they never declare, so no branch is renderable.",
inputSchema: {
by: z
.string()
.optional()
.describe("Which alternative the call is making"),
},
handler: async (params: Record<string, unknown>) => {
return toToolResult(`Recorded shipment: ${JSON.stringify(params)}`);
},
};
}
/**
* Create a tool whose SEP-2243 `x-mcp-header` annotation is INVALID: the header
* name `"Bad Header"` contains a space, so it is not a valid RFC 9110 token.