diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/ToolTestPanel.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/ToolTestPanel.test.tsx index 26e66c8338c..f1414a4b104 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/ToolTestPanel.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/ToolTestPanel.test.tsx @@ -158,6 +158,78 @@ describe("ToolTestPanel defaults", () => { expect(callButton.closest("form")).not.toBeNull(); expect(callButton).toHaveAttribute("type", "button"); }); + + it("renders inputs for nullable (type-array) schema properties instead of leaving them blank", () => { + // A real Azure DevOps MCP tool schema (testplan_test_suite_write) declares its optional + // fields exactly this way. + const schema: InputSchema = { + type: "object", + properties: { + name: { type: ["string", "null"], description: "Optional name" }, + parentSuiteId: { type: ["integer", "null"], description: "Optional parent suite id" }, + active: { type: ["boolean", "null"], description: "Optional flag" }, + }, + }; + + renderPanel(schema); + + expect(screen.getByLabelText("name")).toHaveValue(""); + expect(screen.getByLabelText("parentSuiteId")).toHaveValue(null); + expect(screen.getByLabelText("active")).toBeInTheDocument(); + }); + + it("omits an untouched nullable numeric field from the submitted payload instead of sending a synthetic 0", async () => { + const schema: InputSchema = { + type: "object", + properties: { + parentSuiteId: { type: ["integer", "null"], description: "Optional parent suite id" }, + }, + }; + const onSubmit = vi.fn(); + + render( + , + ); + + await userEvent.click(screen.getByRole("button", { name: "Call Tool" })); + + expect(onSubmit).toHaveBeenCalledWith({}); + }); + + it("coerces a nullable integer field to a number on submit, not a string", async () => { + const schema: InputSchema = { + type: "object", + properties: { + parentSuiteId: { type: ["integer", "null"], description: "Optional parent suite id" }, + }, + }; + const onSubmit = vi.fn(); + + render( + , + ); + + const input = screen.getByLabelText("parentSuiteId"); + await userEvent.clear(input); + await userEvent.type(input, "42"); + await userEvent.click(screen.getByRole("button", { name: "Call Tool" })); + + expect(onSubmit).toHaveBeenCalledWith(expect.objectContaining({ parentSuiteId: 42 })); + }); }); describe("ToolTestPanel argument payload", () => { diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/toolCallArguments.ts b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/toolCallArguments.ts index bdf0a52d02d..4e2833b7103 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/toolCallArguments.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/toolCallArguments.ts @@ -16,6 +16,12 @@ const isPlainObject = (value: unknown): value is Record => typeof value === "object" && value !== null && !Array.isArray(value); export const resolveSchemaProperty = (prop: InputSchemaProperty): InputSchemaProperty => { + // JSON Schema represents a nullable/optional field as a type array (e.g. ["integer", "null"]) - + // the shape Pydantic's model_json_schema() emits for Optional[int]. Collapse to the single + // non-null type before any caller branches on it. + if (Array.isArray(prop.type)) { + return { ...prop, type: prop.type.find((t) => t !== "null") ?? prop.type[0] }; + } if (prop.type !== undefined) return prop; const members = (prop.anyOf ?? prop.oneOf ?? []).filter((member) => member.type !== "null"); if (members.length !== 1 || members[0].type === undefined) return prop; @@ -185,12 +191,17 @@ function buildDefaultValue(declared: InputSchemaProperty | undefined, overrideDe if (prop.type === "array") return buildArrayDefault(prop, effectiveDefault); if (effectiveDefault !== undefined) return effectiveDefault; + // A nullable numeric/boolean field (an array "type" on the ORIGINAL declaration) with no + // explicit default starts empty rather than a synthetic 0/false: unlike a string's natural "" + // default, that value looks user-provided, passes the non-empty submission filter in + // buildToolCallArguments, and gets sent to the tool even when the field was never touched. + const isNullable = Array.isArray(declared.type); switch (prop.type) { case "integer": case "number": - return 0; + return isNullable ? undefined : 0; case "boolean": - return false; + return isNullable ? undefined : false; default: return ""; } @@ -199,7 +210,11 @@ function buildDefaultValue(declared: InputSchemaProperty | undefined, overrideDe export const initialArgumentValues = (fields: readonly ToolArgumentField[]): unknown[] => fields.map(({ prop }) => { const resolved = resolveSchemaProperty(prop); - const defaultValue = buildDefaultValue(resolved); + // Pass the RAW prop, not `resolved` - buildDefaultValue resolves it again internally, and + // needs the original `type` (possibly still an array) to know the field is nullable at all. + // Passing the pre-resolved (already-scalar) type here would make Array.isArray(declared.type) + // always false, silently defeating the nullable-omits-default behavior above. + const defaultValue = buildDefaultValue(prop); if (isJsonField(resolved)) { return isBlank(defaultValue) ? "" : JSON.stringify(defaultValue, null, 2); } diff --git a/ui/litellm-dashboard/src/components/mcp_tools/MCPToolArgumentsForm.test.tsx b/ui/litellm-dashboard/src/components/mcp_tools/MCPToolArgumentsForm.test.tsx index d6e614da5eb..b5972c1fb12 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/MCPToolArgumentsForm.test.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/MCPToolArgumentsForm.test.tsx @@ -178,3 +178,51 @@ describe("MCPToolArgumentsForm", () => { await expect(submit(ref)).resolves.toEqual({}); }); }); + +describe("MCPToolArgumentsForm nullable schema types", () => { + it("renders a number widget for nullable (type-array) schema properties instead of a plain text fallback", () => { + // Unlike ToolTestPanel, this component has a final "else" branch that falls back to a plain + // text Input, so the pre-fix bug here was a wrong widget (text instead of number), not a + // missing one; the field must also start empty, not a synthetic 0. + renderForm({ + type: "object", + properties: { + parentSuiteId: { type: ["integer", "null"], description: "Optional parent suite id" }, + }, + required: [], + }); + + // Native type="number" input: jest-dom reports an empty one as null, not "". + const input = screen.getByLabelText("parentSuiteId"); + expect(input).toHaveValue(null); + }); + + it("omits an untouched nullable numeric field from getSubmitValues instead of sending a synthetic 0", async () => { + const ref = renderForm({ + type: "object", + properties: { + parentSuiteId: { type: ["integer", "null"], description: "Optional parent suite id" }, + }, + required: [], + }); + + await expect(submit(ref)).resolves.toEqual({}); + }); + + it("coerces a nullable integer field to a number in getSubmitValues, not a string", async () => { + const user = userEvent.setup(); + const ref = renderForm({ + type: "object", + properties: { + parentSuiteId: { type: ["integer", "null"], description: "Optional parent suite id" }, + }, + required: [], + }); + + const input = screen.getByLabelText("parentSuiteId"); + await user.clear(input); + await user.type(input, "42"); + + await expect(submit(ref)).resolves.toEqual(expect.objectContaining({ parentSuiteId: 42 })); + }); +}); diff --git a/ui/litellm-dashboard/src/components/mcp_tools/MCPToolArgumentsForm.tsx b/ui/litellm-dashboard/src/components/mcp_tools/MCPToolArgumentsForm.tsx index ca8c5697e6e..48fbcca0b88 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/MCPToolArgumentsForm.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/MCPToolArgumentsForm.tsx @@ -8,6 +8,7 @@ import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@ import { Textarea } from "@/components/ui/textarea"; import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip"; import { MCPTool, InputSchema, InputSchemaProperty } from "./types"; +import { resolveSchemaType, getInitialValueForField } from "./mcpToolSchemaDefaults"; type ToolFormValues = Record; @@ -20,14 +21,14 @@ const BOOLEAN_ITEMS = [ const isBlank = (value: unknown): boolean => value === undefined || value === null || value === ""; -const jsonErrorFor = (prop: InputSchemaProperty, value: unknown): string | null => { +const jsonErrorFor = (effectiveType: string | undefined, value: unknown): string | null => { try { const parsed = typeof value === "string" ? JSON.parse(value) : value; const isValidObject = - prop.type === "object" && parsed !== null && typeof parsed === "object" && !Array.isArray(parsed); - const isValidArray = prop.type === "array" && Array.isArray(parsed); + effectiveType === "object" && parsed !== null && typeof parsed === "object" && !Array.isArray(parsed); + const isValidArray = effectiveType === "array" && Array.isArray(parsed); if (isValidObject || isValidArray) return null; - return prop.type === "object" ? "Please enter a JSON object" : "Please enter a JSON array"; + return effectiveType === "object" ? "Please enter a JSON object" : "Please enter a JSON array"; } catch { return "Invalid JSON"; } @@ -46,9 +47,10 @@ const collectErrors = ( if (actualSchema.required?.includes(key) && blank) { return [[key, { type: "required", message: requiredMessages[key] ?? `Please enter ${key}` }]]; } - if (prop.type !== "object" && prop.type !== "array") return []; + const effectiveType = resolveSchemaType(prop.type); + if (effectiveType !== "object" && effectiveType !== "array") return []; if (blank) return []; - const message = jsonErrorFor(prop, value); + const message = jsonErrorFor(effectiveType, value); return message === null ? [] : [[key, { type: "validate", message }]]; }); return Object.fromEntries(entries); @@ -73,74 +75,6 @@ const labelFor = (key: string, prop: InputSchemaProperty, required: boolean): Re ); -const isPlainObject = (value: unknown): value is Record => - typeof value === "object" && value !== null && !Array.isArray(value); - -function buildArrayItems(items?: InputSchemaProperty | InputSchemaProperty[]): any[] { - if (!items) return []; - if (Array.isArray(items)) { - return items.map((item) => buildDefaultValue(item)).filter((value) => value !== undefined); - } - const itemDefault = buildDefaultValue(items); - return itemDefault !== undefined ? [itemDefault] : []; -} - -function buildDefaultValue(prop?: InputSchemaProperty, overrideDefault?: any): any { - if (!prop) return undefined; - const effectiveDefault = overrideDefault !== undefined ? overrideDefault : prop.default; - - if (prop.type === "object") { - const base = isPlainObject(effectiveDefault) ? { ...effectiveDefault } : {}; - if (prop.properties) { - Object.entries(prop.properties).forEach(([childKey, childProp]) => { - base[childKey] = buildDefaultValue(childProp, base[childKey]); - }); - } - return base; - } - - if (prop.type === "array") { - if (Array.isArray(effectiveDefault)) { - const itemSchema = prop.items; - if (!itemSchema) return effectiveDefault; - if (effectiveDefault.length === 0) { - const sample = buildArrayItems(itemSchema); - return sample.length ? sample : effectiveDefault; - } - if (Array.isArray(itemSchema)) { - return effectiveDefault.map((value, index) => { - const schema = itemSchema[index] ?? itemSchema[itemSchema.length - 1]; - return buildDefaultValue(schema, value); - }); - } - return effectiveDefault.map((value) => buildDefaultValue(itemSchema, value)); - } - if (effectiveDefault !== undefined) return effectiveDefault; - return buildArrayItems(prop.items); - } - - if (effectiveDefault !== undefined) return effectiveDefault; - switch (prop.type) { - case "integer": - case "number": - return 0; - case "boolean": - return false; - case "string": - default: - return ""; - } -} - -const getInitialValueForField = (prop: InputSchemaProperty): any => { - const defaultValue = buildDefaultValue(prop); - if (prop.type === "object" || prop.type === "array") { - const fallback = prop.type === "array" ? [] : {}; - return JSON.stringify(defaultValue ?? fallback, null, 2); - } - return defaultValue; -}; - function convertFormValues( values: Record, actualSchema: InputSchema, @@ -152,7 +86,8 @@ function convertFormValues( Object.entries(values).forEach(([key, value]) => { const prop = schemaToUse.properties?.[key]; if (prop && value !== null && value !== undefined && value !== "") { - switch (prop.type) { + const effectiveType = resolveSchemaType(prop.type); + switch (effectiveType) { case "boolean": convertedValues[key] = value === "true" || value === true; break; @@ -161,7 +96,7 @@ function convertFormValues( const numericValue = Number(value); convertedValues[key] = Number.isNaN(numericValue) ? value - : prop.type === "integer" + : effectiveType === "integer" ? Math.trunc(numericValue) : numericValue; break; @@ -171,9 +106,9 @@ function convertFormValues( try { const parsed = typeof value === "string" ? JSON.parse(value) : value; const isValidObject = - prop.type === "object" && parsed !== null && typeof parsed === "object" && !Array.isArray(parsed); - const isValidArray = prop.type === "array" && Array.isArray(parsed); - if ((prop.type === "object" && isValidObject) || (prop.type === "array" && isValidArray)) { + effectiveType === "object" && parsed !== null && typeof parsed === "object" && !Array.isArray(parsed); + const isValidArray = effectiveType === "array" && Array.isArray(parsed); + if ((effectiveType === "object" && isValidObject) || (effectiveType === "array" && isValidArray)) { convertedValues[key] = parsed; } else { convertedValues[key] = value; @@ -194,7 +129,8 @@ function convertFormValues( } }); - const isNestedParams = schema.properties?.params?.type === "object" && schema.properties.params.properties; + const isNestedParams = + resolveSchemaType(schema.properties?.params?.type) === "object" && schema.properties?.params?.properties; return isNestedParams ? { params: convertedValues } : convertedValues; } @@ -227,7 +163,7 @@ const MCPToolArgumentsForm = forwardRef { - if (schema.properties?.params?.type === "object" && schema.properties.params.properties) { + if (resolveSchemaType(schema.properties?.params?.type) === "object" && schema.properties?.params?.properties) { return { type: "object", properties: schema.properties.params.properties, @@ -320,6 +256,7 @@ const MCPToolArgumentsForm = forwardRef {Object.entries(actualSchema.properties).map(([key, prop]) => { const required = actualSchema.required?.includes(key) ?? false; + const effectiveType = resolveSchemaType(prop.type); return ( {(field) => { - if (prop.type === "string" && prop.enum) { + if (effectiveType === "string" && prop.enum) { return ( ); } - if (prop.type === "number" || prop.type === "integer") { + if (effectiveType === "number" || effectiveType === "integer") { return ( ); } - if (prop.type === "object" || prop.type === "array") { + if (effectiveType === "object" || effectiveType === "array") { return (