fix: render tool schemas with root-level anyOf/oneOf composition
A tool's `inputSchema` may carry composition keywords at its root — the 2026-07-28 revision makes this explicit — but both form builders enumerate the root's `properties` and nothing else. A schema keeping its fields on `allOf`/`oneOf`/`anyOf` branches therefore rendered no controls at all: no branch picker, no fields, not even the raw-JSON fallback a union-typed *property* falls back to, so the tool could only be called with empty arguments. Adds `core/json/rootUnion.ts`, one flattening shared by all three clients so they cannot drift on which schemas they can render: - `allOf` is merged unconditionally (conjunctive — there is no choice to present), a root `oneOf`/`anyOf` is returned as branches. - Web `SchemaForm` renders a Variant picker above the fields and prunes the outgoing branch's values on a switch, since they are no longer visible and would describe a shape the call is not making. - TUI `schemaToForm` gives each branch its own section with optional fields — ink-form is static, and only one alternative applies to a call. - `convertToolParameters` looks through the branches, so a CLI `--tool-arg` is typed by the schema that declares it rather than sent as a string. Required-field gating asks whether *any* branch is satisfied, which is selection-independent and never blocks arguments the schema accepts. A `const` is seeded like a `default` and rendered read-only: it admits exactly one value, so anything else the user could type produces a rejected call. Adds a `root-union-schemas-http.json` showcase server. Closes #2123 Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall
parent
ca2846de28
commit
5933162dd0
9 changed files with 1259 additions and 29 deletions
|
|
@ -180,3 +180,41 @@ export const Disabled: Story = {
|
|||
disabled: true,
|
||||
},
|
||||
};
|
||||
|
||||
/**
|
||||
* Arguments declared as a composition at the root of the schema (#2123): a
|
||||
* picker chooses the alternative, and its fields render beneath. Before this,
|
||||
* such a schema produced a form with no controls at all.
|
||||
*/
|
||||
export const RootUnion: Story = {
|
||||
args: {
|
||||
schema: {
|
||||
type: "object",
|
||||
properties: {
|
||||
message: { type: "string", title: "Message" },
|
||||
},
|
||||
required: ["message"],
|
||||
anyOf: [
|
||||
{
|
||||
type: "object",
|
||||
title: "By email",
|
||||
properties: {
|
||||
kind: { type: "string", const: "email" },
|
||||
address: { type: "string", title: "Address" },
|
||||
},
|
||||
required: ["kind", "address"],
|
||||
},
|
||||
{
|
||||
type: "object",
|
||||
title: "By SMS",
|
||||
properties: {
|
||||
kind: { type: "string", const: "sms" },
|
||||
phone: { type: "string", title: "Phone" },
|
||||
},
|
||||
required: ["kind", "phone"],
|
||||
},
|
||||
],
|
||||
},
|
||||
values: { kind: "email" },
|
||||
},
|
||||
};
|
||||
|
|
|
|||
|
|
@ -1969,4 +1969,166 @@ describe("SchemaForm multiline strings (#2042)", () => {
|
|||
);
|
||||
expect(screen.getByRole("button", { name: "Enlarge Note" })).toBeDisabled();
|
||||
});
|
||||
describe("a root-level union (#2123)", () => {
|
||||
const UNION_SCHEMA: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: { note: { type: "string", title: "Note" } },
|
||||
anyOf: [
|
||||
{
|
||||
type: "object",
|
||||
properties: {
|
||||
kind: { type: "string", const: "email" },
|
||||
address: { type: "string", title: "Address" },
|
||||
},
|
||||
required: ["kind", "address"],
|
||||
},
|
||||
{
|
||||
type: "object",
|
||||
properties: {
|
||||
kind: { type: "string", const: "sms" },
|
||||
phone: { type: "string", title: "Phone" },
|
||||
},
|
||||
required: ["kind", "phone"],
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
it("renders the first branch's fields with a picker, not an empty form", () => {
|
||||
renderWithMantine(
|
||||
<SchemaForm schema={UNION_SCHEMA} values={{}} onChange={vi.fn()} />,
|
||||
);
|
||||
expect(screen.getByRole("textbox", { name: /Note/ })).toBeTruthy();
|
||||
expect(screen.getByRole("textbox", { name: /Address/ })).toBeTruthy();
|
||||
expect(screen.queryByRole("textbox", { name: /Phone/ })).toBeNull();
|
||||
expect(
|
||||
(screen.getByRole("textbox", { name: /Variant/ }) as HTMLInputElement)
|
||||
.value,
|
||||
).toBe("email");
|
||||
});
|
||||
|
||||
it("switches to the chosen branch's fields", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithMantine(
|
||||
<SchemaForm schema={UNION_SCHEMA} values={{}} onChange={vi.fn()} />,
|
||||
);
|
||||
await user.click(screen.getByRole("textbox", { name: /Variant/ }));
|
||||
await user.click(screen.getByRole("option", { name: "sms" }));
|
||||
expect(screen.getByRole("textbox", { name: /Phone/ })).toBeTruthy();
|
||||
expect(screen.queryByRole("textbox", { name: /Address/ })).toBeNull();
|
||||
});
|
||||
|
||||
it("drops the outgoing branch's values and seeds the incoming branch's const", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
renderWithMantine(
|
||||
<SchemaForm
|
||||
schema={UNION_SCHEMA}
|
||||
values={{ note: "hi", kind: "email", address: "a@b.c" }}
|
||||
onChange={onChange}
|
||||
/>,
|
||||
);
|
||||
await user.click(screen.getByRole("textbox", { name: /Variant/ }));
|
||||
await user.click(screen.getByRole("option", { name: "sms" }));
|
||||
// `address` belongs to a shape this call is no longer making, so it must
|
||||
// not ride along invisibly into the submitted arguments.
|
||||
expect(onChange).toHaveBeenCalledWith({ note: "hi", kind: "sms" });
|
||||
});
|
||||
|
||||
it("renders a const-pinned field read-only", () => {
|
||||
renderWithMantine(
|
||||
<SchemaForm
|
||||
schema={UNION_SCHEMA}
|
||||
values={{ kind: "email" }}
|
||||
onChange={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
const kind = screen.getByRole("textbox", {
|
||||
name: /kind/,
|
||||
}) as HTMLInputElement;
|
||||
expect(kind.readOnly).toBe(true);
|
||||
expect(kind.value).toBe("email");
|
||||
});
|
||||
|
||||
it("renders no picker for a single-branch union but still shows its fields", () => {
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
anyOf: [
|
||||
{
|
||||
type: "object",
|
||||
properties: { only: { type: "string", title: "Only" } },
|
||||
},
|
||||
],
|
||||
};
|
||||
renderWithMantine(
|
||||
<SchemaForm schema={schema} values={{}} onChange={vi.fn()} />,
|
||||
);
|
||||
expect(screen.getByRole("textbox", { name: /Only/ })).toBeTruthy();
|
||||
expect(screen.queryByRole("textbox", { name: /Variant/ })).toBeNull();
|
||||
});
|
||||
|
||||
it("renders root allOf fields", () => {
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
allOf: [
|
||||
{
|
||||
type: "object",
|
||||
properties: { merged: { type: "string", title: "Merged" } },
|
||||
},
|
||||
],
|
||||
};
|
||||
renderWithMantine(
|
||||
<SchemaForm schema={schema} values={{}} onChange={vi.fn()} />,
|
||||
);
|
||||
expect(screen.getByRole("textbox", { name: /Merged/ })).toBeTruthy();
|
||||
});
|
||||
|
||||
it("returns to the first branch when resetKey says the form moved on", async () => {
|
||||
const user = userEvent.setup();
|
||||
const { rerender } = renderWithMantine(
|
||||
<SchemaForm
|
||||
schema={UNION_SCHEMA}
|
||||
values={{}}
|
||||
onChange={vi.fn()}
|
||||
resetKey="tool-a"
|
||||
/>,
|
||||
);
|
||||
await user.click(screen.getByRole("textbox", { name: /Variant/ }));
|
||||
await user.click(screen.getByRole("option", { name: "sms" }));
|
||||
expect(screen.getByRole("textbox", { name: /Phone/ })).toBeTruthy();
|
||||
|
||||
rerender(
|
||||
<SchemaForm
|
||||
schema={UNION_SCHEMA}
|
||||
values={{}}
|
||||
onChange={vi.fn()}
|
||||
resetKey="tool-b"
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByRole("textbox", { name: /Address/ })).toBeTruthy();
|
||||
expect(screen.queryByRole("textbox", { name: /Phone/ })).toBeNull();
|
||||
});
|
||||
|
||||
it("clamps a selection the next schema's shorter union cannot hold", async () => {
|
||||
const user = userEvent.setup();
|
||||
const { rerender } = renderWithMantine(
|
||||
<SchemaForm schema={UNION_SCHEMA} values={{}} onChange={vi.fn()} />,
|
||||
);
|
||||
await user.click(screen.getByRole("textbox", { name: /Variant/ }));
|
||||
await user.click(screen.getByRole("option", { name: "sms" }));
|
||||
|
||||
// No `resetKey`, so nothing resets the selection: index 1 must be clamped
|
||||
// rather than reaching past the end of a one-branch union.
|
||||
const shorter: InspectorFormSchema = {
|
||||
type: "object",
|
||||
anyOf: [
|
||||
{
|
||||
type: "object",
|
||||
properties: { solo: { type: "string", title: "Solo" } },
|
||||
},
|
||||
],
|
||||
};
|
||||
rerender(<SchemaForm schema={shorter} values={{}} onChange={vi.fn()} />);
|
||||
expect(screen.getByRole("textbox", { name: /Solo/ })).toBeTruthy();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -31,6 +31,8 @@ import {
|
|||
isStringEnum,
|
||||
normalizeNullableUnion,
|
||||
} from "@inspector/core/json/nullableUnion.js";
|
||||
import { resolveRootUnion } from "@inspector/core/json/rootUnion.js";
|
||||
import { collectSchemaDefaults } from "../../../utils/jsonUtils";
|
||||
|
||||
const FieldLabel = Text.withProps({
|
||||
fw: 500,
|
||||
|
|
@ -45,6 +47,14 @@ const FieldDescription = Text.withProps({
|
|||
// Indented column for a nested object's sub-fields.
|
||||
const IndentedStack = Stack.withProps({ gap: "sm", pl: "md" });
|
||||
|
||||
// The picker for a root `oneOf`/`anyOf` (#2123). Not clearable: one branch is
|
||||
// always in effect, so "no branch" is not a state the arguments can be in.
|
||||
const BranchSelect = Select.withProps({
|
||||
label: "Variant",
|
||||
description: "This tool accepts one of several argument shapes.",
|
||||
allowDeselect: false,
|
||||
});
|
||||
|
||||
const SchemaJsonInput = JsonInput.withProps({
|
||||
formatOnBlur: true,
|
||||
autosize: true,
|
||||
|
|
@ -569,8 +579,27 @@ export function SchemaForm({
|
|||
resetKey,
|
||||
onValidityChange,
|
||||
}: SchemaFormProps) {
|
||||
const properties = schema.properties ?? {};
|
||||
const requiredFields = schema.required ?? [];
|
||||
// Composition at the root of the schema, flattened before anything is
|
||||
// rendered (#2123). `allOf` is folded into `base`; a top-level `oneOf`/`anyOf`
|
||||
// becomes the branches a picker chooses between. Both are empty for the
|
||||
// ordinary object schema, where `base` is the schema itself.
|
||||
const { base, branches } = resolveRootUnion(schema);
|
||||
|
||||
// Which alternative the form is currently showing. Held here because it is a
|
||||
// property of this rendering, not of the arguments: `values` carries what the
|
||||
// user typed, and nothing in it names a branch.
|
||||
const [branchIndex, setBranchIndex] = useState(0);
|
||||
// A form reused for another entity can be handed a shorter union, so the
|
||||
// index is clamped rather than trusted — `resetKey` resets it below, but a
|
||||
// caller that omits it (the elicitation panels mount fresh) supplies none.
|
||||
const activeBranch =
|
||||
branches.length > 0
|
||||
? (branches[Math.min(branchIndex, branches.length - 1)] ?? null)
|
||||
: null;
|
||||
const effectiveSchema = activeBranch?.schema ?? base;
|
||||
|
||||
const properties = effectiveSchema.properties ?? {};
|
||||
const requiredFields = effectiveSchema.required ?? [];
|
||||
|
||||
// The names of fields currently holding unsendable text. Held here rather
|
||||
// than in each field because only the form sees them all, and only the form
|
||||
|
|
@ -590,7 +619,12 @@ export function SchemaForm({
|
|||
// *name*, so it must not carry across to another entity's same-named field —
|
||||
// the same reasoning `resetKey` documents for the number field's draft. Reset
|
||||
// during render rather than in an effect so no frame paints the wrong shape.
|
||||
useValueChange(resetKey, () => setEnlargedFields(new Set()));
|
||||
useValueChange(resetKey, () => {
|
||||
setEnlargedFields(new Set());
|
||||
// Which branch is selected belongs to the entity it was chosen for, for the
|
||||
// same reason enlargement does.
|
||||
setBranchIndex(0);
|
||||
});
|
||||
|
||||
// Stable so a field's reporting effect subscribes once, not per render. The
|
||||
// updater returns the previous set unchanged when nothing moved, which is
|
||||
|
|
@ -637,6 +671,35 @@ export function SchemaForm({
|
|||
onChange({ ...values, [fieldName]: fieldValue });
|
||||
}
|
||||
|
||||
/**
|
||||
* Switch branches, and move `values` with the form.
|
||||
*
|
||||
* The fields the outgoing branch owned are dropped rather than left behind:
|
||||
* they are no longer rendered, so the user cannot see or clear them, and
|
||||
* submitting them would send the server arguments belonging to a shape the
|
||||
* call is not making. Whatever the base contributes is kept — it applies to
|
||||
* every branch — and the incoming branch's defaults (a discriminator `const`
|
||||
* among them) are seeded the way the initial ones were.
|
||||
*/
|
||||
function handleBranchChange(nextIndex: number) {
|
||||
const nextBranch = branches[nextIndex];
|
||||
/* v8 ignore next -- the Select's options are built from `branches` */
|
||||
if (!nextBranch) return;
|
||||
setBranchIndex(nextIndex);
|
||||
const nextProperties = nextBranch.schema.properties ?? {};
|
||||
const carried: Record<string, unknown> = {};
|
||||
for (const [name, fieldSchema] of Object.entries(nextProperties)) {
|
||||
// A field the incoming branch pins to a `const` is not carried: the two
|
||||
// branches of a discriminated union share the discriminator's *name* and
|
||||
// disagree about its value, so keeping what the outgoing branch put
|
||||
// there would leave the arguments claiming the shape they no longer have.
|
||||
if (values[name] !== undefined && fieldSchema.const === undefined) {
|
||||
carried[name] = values[name];
|
||||
}
|
||||
}
|
||||
onChange({ ...collectSchemaDefaults(nextBranch.schema), ...carried });
|
||||
}
|
||||
|
||||
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
|
||||
|
|
@ -687,6 +750,26 @@ export function SchemaForm({
|
|||
);
|
||||
}
|
||||
|
||||
// A string pinned to a single value. Rendered read-only rather than as an
|
||||
// editable box: `const` admits exactly one value, so anything the user
|
||||
// could type into it produces a call the schema rejects. This is what a
|
||||
// discriminated union's `kind`/`by` field is (#2123), and the picker has
|
||||
// already set it — but the rule is the keyword's, not the union's, so it
|
||||
// holds for a lone `const` property too.
|
||||
if (typeof fieldSchema.const === "string") {
|
||||
return (
|
||||
<TextInput
|
||||
key={fieldName}
|
||||
label={label}
|
||||
description={description}
|
||||
withAsterisk={isRequired}
|
||||
readOnly
|
||||
disabled={disabled}
|
||||
value={fieldSchema.const}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
// plain string
|
||||
if (fieldSchema.type === "string") {
|
||||
const clearButton = rawValue ? (
|
||||
|
|
@ -878,6 +961,20 @@ export function SchemaForm({
|
|||
|
||||
return (
|
||||
<Stack gap="sm">
|
||||
{activeBranch && branches.length > 1 && (
|
||||
<BranchSelect
|
||||
data={branches.map((branch, index) => ({
|
||||
value: String(index),
|
||||
label: branch.label,
|
||||
}))}
|
||||
value={String(branches.indexOf(activeBranch))}
|
||||
disabled={disabled}
|
||||
onChange={(value) =>
|
||||
/* v8 ignore next -- Select only ever reports one of its own options */
|
||||
value === null ? undefined : handleBranchChange(Number(value))
|
||||
}
|
||||
/>
|
||||
)}
|
||||
{Object.entries(properties).map(([fieldName, fieldSchema]) =>
|
||||
renderField(fieldName, fieldSchema),
|
||||
)}
|
||||
|
|
|
|||
|
|
@ -75,6 +75,50 @@ describe("JSON Utils", () => {
|
|||
},
|
||||
};
|
||||
|
||||
it("coerces a value whose schema lives on a root union branch (#2123)", () => {
|
||||
const unionTool: Tool = {
|
||||
name: "union-tool",
|
||||
inputSchema: {
|
||||
type: "object",
|
||||
properties: { note: { type: "string" } },
|
||||
anyOf: [
|
||||
{
|
||||
type: "object",
|
||||
properties: { count: { type: "number" } },
|
||||
},
|
||||
{
|
||||
type: "object",
|
||||
properties: { enabled: { type: "boolean" } },
|
||||
},
|
||||
],
|
||||
},
|
||||
};
|
||||
// Reading the root's `properties` alone finds no schema for either, so
|
||||
// both would have been sent as the strings the user typed.
|
||||
expect(
|
||||
convertToolParameters(unionTool, {
|
||||
note: "hi",
|
||||
count: "42",
|
||||
enabled: "true",
|
||||
}),
|
||||
).toEqual({ note: "hi", count: 42, enabled: true });
|
||||
});
|
||||
|
||||
it("coerces a value whose schema lives on a root allOf branch (#2123)", () => {
|
||||
const allOfTool: Tool = {
|
||||
name: "allof-tool",
|
||||
inputSchema: {
|
||||
type: "object",
|
||||
allOf: [
|
||||
{ type: "object", properties: { count: { type: "number" } } },
|
||||
],
|
||||
},
|
||||
};
|
||||
expect(convertToolParameters(allOfTool, { count: "7" })).toEqual({
|
||||
count: 7,
|
||||
});
|
||||
});
|
||||
|
||||
it("should convert string parameters", () => {
|
||||
const result = convertToolParameters(tool, {
|
||||
message: "hello",
|
||||
|
|
|
|||
235
clients/web/src/test/core/rootUnion.test.ts
Normal file
235
clients/web/src/test/core/rootUnion.test.ts
Normal file
|
|
@ -0,0 +1,235 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import { resolveRootUnion } from "@inspector/core/json/rootUnion.js";
|
||||
|
||||
const EMAIL = {
|
||||
type: "object",
|
||||
properties: {
|
||||
kind: { type: "string", const: "email" },
|
||||
address: { type: "string" },
|
||||
},
|
||||
required: ["kind", "address"],
|
||||
};
|
||||
|
||||
const SMS = {
|
||||
type: "object",
|
||||
properties: {
|
||||
kind: { type: "string", const: "sms" },
|
||||
phone: { type: "string" },
|
||||
},
|
||||
required: ["kind", "phone"],
|
||||
};
|
||||
|
||||
describe("resolveRootUnion", () => {
|
||||
it("leaves an ordinary object schema alone", () => {
|
||||
const schema = {
|
||||
type: "object" as const,
|
||||
properties: { message: { type: "string" as const } },
|
||||
required: ["message"],
|
||||
};
|
||||
const { base, branches } = resolveRootUnion(schema);
|
||||
expect(branches).toEqual([]);
|
||||
expect(base.properties).toEqual(schema.properties);
|
||||
expect(base.required).toEqual(["message"]);
|
||||
});
|
||||
|
||||
it("returns a branch per anyOf member, merged with the root", () => {
|
||||
const { base, branches } = resolveRootUnion({
|
||||
type: "object",
|
||||
properties: { note: { type: "string" } },
|
||||
required: ["note"],
|
||||
anyOf: [EMAIL, SMS],
|
||||
});
|
||||
|
||||
// The root's own fields render for every branch, so they stay on the base
|
||||
// and are merged into each branch rather than belonging to one.
|
||||
expect(Object.keys(base.properties ?? {})).toEqual(["note"]);
|
||||
expect(branches).toHaveLength(2);
|
||||
expect(Object.keys(branches[0].schema.properties ?? {})).toEqual([
|
||||
"note",
|
||||
"kind",
|
||||
"address",
|
||||
]);
|
||||
expect(branches[0].schema.required).toEqual(["note", "kind", "address"]);
|
||||
expect(branches[0].ownFields).toEqual(["kind", "address"]);
|
||||
expect(branches[1].ownFields).toEqual(["kind", "phone"]);
|
||||
});
|
||||
|
||||
it("strips the composition keywords it has absorbed", () => {
|
||||
const { base, branches } = resolveRootUnion({
|
||||
type: "object",
|
||||
anyOf: [EMAIL, SMS],
|
||||
});
|
||||
expect(base.anyOf).toBeUndefined();
|
||||
expect(branches[0].schema.anyOf).toBeUndefined();
|
||||
});
|
||||
|
||||
it("prefers oneOf when a schema carries both", () => {
|
||||
const { branches } = resolveRootUnion({
|
||||
type: "object",
|
||||
oneOf: [EMAIL],
|
||||
anyOf: [EMAIL, SMS],
|
||||
});
|
||||
expect(branches).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("merges allOf branches unconditionally", () => {
|
||||
const { base, branches } = resolveRootUnion({
|
||||
type: "object",
|
||||
properties: { a: { type: "string" } },
|
||||
required: ["a"],
|
||||
allOf: [
|
||||
{
|
||||
type: "object",
|
||||
properties: { b: { type: "number" } },
|
||||
required: ["b"],
|
||||
},
|
||||
],
|
||||
});
|
||||
expect(branches).toEqual([]);
|
||||
expect(Object.keys(base.properties ?? {})).toEqual(["a", "b"]);
|
||||
expect(base.required).toEqual(["a", "b"]);
|
||||
expect(base.allOf).toBeUndefined();
|
||||
});
|
||||
|
||||
it("merges allOf into every union branch", () => {
|
||||
const { branches } = resolveRootUnion({
|
||||
type: "object",
|
||||
allOf: [{ type: "object", properties: { shared: { type: "string" } } }],
|
||||
oneOf: [EMAIL, SMS],
|
||||
});
|
||||
expect(Object.keys(branches[0].schema.properties ?? {})).toContain(
|
||||
"shared",
|
||||
);
|
||||
});
|
||||
|
||||
it("lets a branch's declaration win a name collision with the root", () => {
|
||||
const { branches } = resolveRootUnion({
|
||||
type: "object",
|
||||
properties: { id: { type: "string" } },
|
||||
required: ["id"],
|
||||
anyOf: [{ type: "object", properties: { id: { type: "number" } } }, SMS],
|
||||
});
|
||||
expect(branches[0].schema.properties?.id).toEqual({ type: "number" });
|
||||
// `required` unions rather than duplicating.
|
||||
expect(branches[0].schema.required).toEqual(["id"]);
|
||||
});
|
||||
|
||||
describe("branch labels", () => {
|
||||
it("uses the branch's own title first", () => {
|
||||
const { branches } = resolveRootUnion({
|
||||
type: "object",
|
||||
anyOf: [{ ...EMAIL, title: "By email" }, SMS],
|
||||
});
|
||||
expect(branches[0].label).toBe("By email");
|
||||
});
|
||||
|
||||
it("uses the discriminator property's const when one is named", () => {
|
||||
const { branches } = resolveRootUnion({
|
||||
type: "object",
|
||||
discriminator: { propertyName: "kind" },
|
||||
oneOf: [EMAIL, SMS],
|
||||
});
|
||||
expect(branches.map((branch) => branch.label)).toEqual(["email", "sms"]);
|
||||
});
|
||||
|
||||
it("uses a lone constant-valued property when there is no discriminator", () => {
|
||||
const { branches } = resolveRootUnion({
|
||||
type: "object",
|
||||
anyOf: [EMAIL, SMS],
|
||||
});
|
||||
expect(branches.map((branch) => branch.label)).toEqual(["email", "sms"]);
|
||||
});
|
||||
|
||||
it("falls back to a position when a branch has several constants", () => {
|
||||
const twoConstants = {
|
||||
type: "object",
|
||||
properties: {
|
||||
kind: { const: "a" },
|
||||
other: { const: "b" },
|
||||
},
|
||||
};
|
||||
const { branches } = resolveRootUnion({
|
||||
type: "object",
|
||||
anyOf: [twoConstants, SMS],
|
||||
});
|
||||
expect(branches[0].label).toBe("Option 1");
|
||||
});
|
||||
|
||||
it("falls back to a position when a branch names no constant", () => {
|
||||
const { branches } = resolveRootUnion({
|
||||
type: "object",
|
||||
anyOf: [
|
||||
{ type: "object", properties: { a: { type: "string" } } },
|
||||
{ type: "object", properties: { b: { type: "string" } } },
|
||||
],
|
||||
});
|
||||
expect(branches.map((branch) => branch.label)).toEqual([
|
||||
"Option 1",
|
||||
"Option 2",
|
||||
]);
|
||||
});
|
||||
|
||||
it("ignores a discriminator naming a property without a usable const", () => {
|
||||
const { branches } = resolveRootUnion({
|
||||
type: "object",
|
||||
discriminator: { propertyName: "missing" },
|
||||
anyOf: [EMAIL, SMS],
|
||||
});
|
||||
expect(branches[0].label).toBe("email");
|
||||
});
|
||||
|
||||
it("labels a numeric const by its value", () => {
|
||||
const { branches } = resolveRootUnion({
|
||||
type: "object",
|
||||
anyOf: [
|
||||
{ type: "object", properties: { v: { const: 1 } } },
|
||||
{ type: "object", properties: { v: { const: 2 } } },
|
||||
],
|
||||
});
|
||||
expect(branches.map((branch) => branch.label)).toEqual(["1", "2"]);
|
||||
});
|
||||
|
||||
it("ignores a blank title", () => {
|
||||
const { branches } = resolveRootUnion({
|
||||
type: "object",
|
||||
anyOf: [{ ...EMAIL, title: " " }, SMS],
|
||||
});
|
||||
expect(branches[0].label).toBe("email");
|
||||
});
|
||||
});
|
||||
|
||||
describe("unions it declines to offer", () => {
|
||||
// Each of these would produce a picker with an option that renders
|
||||
// nothing, which is the failure this module exists to prevent.
|
||||
it("declines a union with a member that is not an object", () => {
|
||||
const { branches } = resolveRootUnion({
|
||||
type: "object",
|
||||
anyOf: [EMAIL, "nope" as unknown],
|
||||
});
|
||||
expect(branches).toEqual([]);
|
||||
});
|
||||
|
||||
it("declines a union with a fieldless member", () => {
|
||||
const { branches } = resolveRootUnion({
|
||||
type: "object",
|
||||
anyOf: [EMAIL, { type: "null" }],
|
||||
});
|
||||
expect(branches).toEqual([]);
|
||||
});
|
||||
|
||||
it("declines an empty union", () => {
|
||||
expect(resolveRootUnion({ type: "object", anyOf: [] }).branches).toEqual(
|
||||
[],
|
||||
);
|
||||
});
|
||||
|
||||
it("ignores an allOf member that is not an object", () => {
|
||||
const { base } = resolveRootUnion({
|
||||
type: "object",
|
||||
properties: { a: { type: "string" } },
|
||||
allOf: [null as unknown, 3 as unknown],
|
||||
});
|
||||
expect(Object.keys(base.properties ?? {})).toEqual(["a"]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -340,3 +340,96 @@ describe("hasMissingRequiredFields", () => {
|
|||
expect(hasMissingRequiredFields(undescribed, { ghost: null })).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("root composition (#2123)", () => {
|
||||
const UNION: InspectorFormSchema = {
|
||||
type: "object",
|
||||
properties: { note: { type: "string" } },
|
||||
anyOf: [
|
||||
{
|
||||
type: "object",
|
||||
properties: {
|
||||
kind: { type: "string", const: "email" },
|
||||
address: { type: "string" },
|
||||
},
|
||||
required: ["kind", "address"],
|
||||
},
|
||||
{
|
||||
type: "object",
|
||||
properties: {
|
||||
kind: { type: "string", const: "sms" },
|
||||
phone: { type: "string" },
|
||||
},
|
||||
required: ["kind", "phone"],
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
it("seeds the first branch's defaults, including its const", () => {
|
||||
expect(collectSchemaDefaults(UNION)).toEqual({ kind: "email" });
|
||||
});
|
||||
|
||||
it("does not seed fields of branches the form is not showing", () => {
|
||||
expect(collectSchemaDefaults(UNION)).not.toHaveProperty("phone");
|
||||
});
|
||||
|
||||
it("seeds a const property on an ordinary schema too", () => {
|
||||
expect(
|
||||
collectSchemaDefaults({
|
||||
type: "object",
|
||||
properties: { version: { type: "string", const: "1" } },
|
||||
}),
|
||||
).toEqual({ version: "1" });
|
||||
});
|
||||
|
||||
it("prefers an explicit default over a const", () => {
|
||||
expect(
|
||||
collectSchemaDefaults({
|
||||
type: "object",
|
||||
properties: {
|
||||
v: { type: "string", const: "a", default: "b" },
|
||||
},
|
||||
}),
|
||||
).toEqual({ v: "b" });
|
||||
});
|
||||
|
||||
it("collects defaults from a root allOf", () => {
|
||||
expect(
|
||||
collectSchemaDefaults({
|
||||
type: "object",
|
||||
allOf: [
|
||||
{
|
||||
type: "object",
|
||||
properties: { merged: { type: "string", default: "m" } },
|
||||
},
|
||||
],
|
||||
}),
|
||||
).toEqual({ merged: "m" });
|
||||
});
|
||||
|
||||
it("blocks submission while no branch is satisfied", () => {
|
||||
expect(hasMissingRequiredFields(UNION, {})).toBe(true);
|
||||
expect(hasMissingRequiredFields(UNION, { kind: "email" })).toBe(true);
|
||||
});
|
||||
|
||||
it("allows submission once one branch is satisfied", () => {
|
||||
expect(hasMissingRequiredFields(UNION, { kind: "sms", phone: "555" })).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
it("gates on required fields declared only in a root allOf", () => {
|
||||
const schema: InspectorFormSchema = {
|
||||
type: "object",
|
||||
allOf: [
|
||||
{
|
||||
type: "object",
|
||||
properties: { a: { type: "string" } },
|
||||
required: ["a"],
|
||||
},
|
||||
],
|
||||
};
|
||||
expect(hasMissingRequiredFields(schema, {})).toBe(true);
|
||||
expect(hasMissingRequiredFields(schema, { a: "x" })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import {
|
|||
admitsNull,
|
||||
normalizeNullableUnion,
|
||||
} from "@inspector/core/json/nullableUnion.js";
|
||||
import { resolveRootUnion } from "@inspector/core/json/rootUnion.js";
|
||||
|
||||
export type JsonValue =
|
||||
| string
|
||||
|
|
@ -58,6 +59,11 @@ export type InspectorFormSchema = {
|
|||
const?: JsonValue;
|
||||
oneOf?: (InspectorFormSchema | JsonSchemaConst)[];
|
||||
anyOf?: (InspectorFormSchema | JsonSchemaConst)[];
|
||||
// Root composition the form flattens before rendering (#2123). `allOf` is
|
||||
// merged into the schema; `oneOf`/`anyOf` at the root become the branches the
|
||||
// Variant picker chooses between, labelled via `discriminator` when present.
|
||||
allOf?: InspectorFormSchema[];
|
||||
discriminator?: { propertyName?: string };
|
||||
$ref?: string;
|
||||
};
|
||||
|
||||
|
|
@ -119,7 +125,13 @@ export function getDataType(value: JsonValue): DataType {
|
|||
export function collectSchemaDefaults(
|
||||
schema: InspectorFormSchema,
|
||||
): Record<string, unknown> {
|
||||
const properties = schema.properties ?? {};
|
||||
// Seed from the shape the form actually renders: root `allOf` merged in, and
|
||||
// for a root union the branch the picker starts on (#2123). Seeding every
|
||||
// branch would put fields of shapes the call is not making into the
|
||||
// arguments; seeding none would leave the branch's defaults — its
|
||||
// discriminator `const` among them — displayed but never submitted.
|
||||
const { base, branches } = resolveRootUnion(schema);
|
||||
const properties = (branches[0]?.schema ?? base).properties ?? {};
|
||||
const result: Record<string, unknown> = {};
|
||||
for (const [fieldName, rawSchema] of Object.entries(properties)) {
|
||||
// Collapse a nullable union first, for the same reason `SchemaForm` does:
|
||||
|
|
@ -129,6 +141,11 @@ export function collectSchemaDefaults(
|
|||
const fieldSchema = normalizeNullableUnion(rawSchema);
|
||||
if (fieldSchema.default !== undefined) {
|
||||
result[fieldName] = fieldSchema.default;
|
||||
} else if (fieldSchema.const !== undefined) {
|
||||
// `const` is a one-value enumeration, so the only submittable value is
|
||||
// already known — seeding it spares the user hand-typing a discriminator
|
||||
// the schema has fixed (#2123), and matches what a `default` would do.
|
||||
result[fieldName] = fieldSchema.const;
|
||||
} else if (fieldSchema.type === "object" && fieldSchema.properties) {
|
||||
const nested = collectSchemaDefaults(fieldSchema);
|
||||
if (Object.keys(nested).length > 0) {
|
||||
|
|
@ -160,6 +177,25 @@ export function collectSchemaDefaults(
|
|||
export function hasMissingRequiredFields(
|
||||
schema: InspectorFormSchema,
|
||||
values: Record<string, unknown>,
|
||||
): boolean {
|
||||
// Root composition first, so a schema keeping its `required` on branches is
|
||||
// gated at all (#2123). For a union the answer is selection-independent by
|
||||
// construction: valid arguments must satisfy *some* branch, so the submit is
|
||||
// blocked only when **every** branch is still missing something. That is
|
||||
// deliberately weaker than gating on the branch the picker is showing — this
|
||||
// function is handed `values`, never the selection — but it is sound in the
|
||||
// direction that matters: it never blocks arguments the schema accepts.
|
||||
const { base, branches } = resolveRootUnion(schema);
|
||||
if (branches.length > 0) {
|
||||
return branches.every((branch) => hasMissingIn(branch.schema, values));
|
||||
}
|
||||
return hasMissingIn(base, values);
|
||||
}
|
||||
|
||||
/** {@link hasMissingRequiredFields} against one already-flattened schema. */
|
||||
function hasMissingIn(
|
||||
schema: InspectorFormSchema,
|
||||
values: Record<string, unknown>,
|
||||
): boolean {
|
||||
const required = schema.required ?? [];
|
||||
const properties = schema.properties ?? {};
|
||||
|
|
|
|||
|
|
@ -86,3 +86,39 @@ describe("toolRowKey / findToolByRowKey", () => {
|
|||
expect(findToolByRowKey(tools, "1:get_weather")).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("hasInputFields with root composition (#2123)", () => {
|
||||
const tool = (inputSchema: Tool["inputSchema"]): Tool => ({
|
||||
name: "t",
|
||||
inputSchema,
|
||||
});
|
||||
|
||||
it("sees fields declared on a root union branch", () => {
|
||||
expect(
|
||||
hasInputFields(
|
||||
tool({
|
||||
type: "object",
|
||||
anyOf: [
|
||||
{ type: "object", properties: { a: { type: "string" } } },
|
||||
{ type: "object", properties: { b: { type: "string" } } },
|
||||
],
|
||||
}),
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("sees fields declared on a root allOf", () => {
|
||||
expect(
|
||||
hasInputFields(
|
||||
tool({
|
||||
type: "object",
|
||||
allOf: [{ type: "object", properties: { a: { type: "string" } } }],
|
||||
}),
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("still reports no fields for a bare object schema", () => {
|
||||
expect(hasInputFields(tool({ type: "object" }))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import type { Tool } from "@modelcontextprotocol/client";
|
||||
import { resolveRootUnion } from "@inspector/core/json/rootUnion.js";
|
||||
|
||||
/**
|
||||
* Returns the display label for an MCP entity that follows the BaseMetadata
|
||||
|
|
@ -14,10 +15,21 @@ export function resolveDisplayLabel(name: string, title?: string): string {
|
|||
* True when the tool's input schema declares at least one property — used by
|
||||
* App-flow callers to decide whether to render a form or auto-launch. Kept in
|
||||
* one place so the definition of "has fields" stays consistent if it ever
|
||||
* grows to consider `additionalProperties`, `anyOf`, etc.
|
||||
* grows to consider `additionalProperties` etc.
|
||||
*
|
||||
* Root composition is resolved first, since a schema declaring its fields on a
|
||||
* root `allOf`/`oneOf`/`anyOf` has none of its own (#2123) — an App tool with
|
||||
* such a schema would otherwise launch with empty arguments rather than asking
|
||||
* for them.
|
||||
*/
|
||||
export function hasInputFields(tool: Tool): boolean {
|
||||
return Object.keys(tool.inputSchema.properties ?? {}).length > 0;
|
||||
const { base, branches } = resolveRootUnion(tool.inputSchema);
|
||||
return (
|
||||
Object.keys(base.properties ?? {}).length > 0 ||
|
||||
branches.some(
|
||||
(branch) => Object.keys(branch.schema.properties ?? {}).length > 0,
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
Loading…
Reference in a new issue