Merge pull request #40494 from BerriAI/litellm_fix_mcp_dotted_properties_5777

fix(ui): preserve dotted MCP tool argument names

ui/litellm-dashboard/src/components/mcp_tools/MCPToolArgumentsForm.integration.test.tsx

@@ -1,5 +1,5 @@
 import React from "react";
-import { render, screen } from "@testing-library/react";
+import { fireEvent, render, screen } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 import { describe, it, expect } from "vitest";
 import MCPToolArgumentsForm, { MCPToolArgumentsFormRef } from "./MCPToolArgumentsForm";
@@ -26,6 +26,118 @@ const submitError = async (ref: React.RefObject<MCPToolArgumentsFormRef | null>)
 };
 
 describe("MCPToolArgumentsForm", () => {
+  it("keeps dotted arguments separate from a same-prefix object and converts their values", async () => {
+    const ref = renderForm({
+      type: "object",
+      properties: {
+        "filter.category": { type: "string" },
+        filter: { type: "object" },
+        "page.limit": { type: "integer" },
+        query: { type: "string" },
+      },
+      required: ["filter.category"],
+    });
+
+    fireEvent.change(screen.getByRole("textbox", { name: "filter.category *" }), {
+      target: { value: "invoices" },
+    });
+    fireEvent.change(screen.getByRole("textbox", { name: "filter" }), {
+      target: { value: '{"category":"receipts","metadata":{"region":"eu"}}' },
+    });
+    fireEvent.change(screen.getByRole("spinbutton", { name: "page.limit" }), { target: { value: "7" } });
+    fireEvent.change(screen.getByRole("textbox", { name: "query" }), { target: { value: "September" } });
+
+    const expected = {
+      "filter.category": "invoices",
+      filter: { category: "receipts", metadata: { region: "eu" } },
+      "page.limit": 7,
+      query: "September",
+    };
+    await expect(submit(ref)).resolves.toEqual(expected);
+  });
+
+  it("shows required validation on the literal dotted field and accepts a correction", async () => {
+    const ref = renderForm({
+      type: "object",
+      properties: { "filter.category": { type: "string" } },
+      required: ["filter.category"],
+    });
+
+    expect(await submitError(ref)).toEqual({
+      errorFields: [{ name: ["filter.category"], errors: ["Please enter filter.category"] }],
+    });
+    expect(await screen.findByText("Please enter filter.category")).toBeInTheDocument();
+    expect(screen.getByRole("textbox", { name: "filter.category *" })).toHaveAttribute("aria-invalid", "true");
+
+    fireEvent.change(screen.getByRole("textbox", { name: "filter.category *" }), {
+      target: { value: "invoices" },
+    });
+    await expect(submit(ref)).resolves.toEqual({ "filter.category": "invoices" });
+  });
+
+  it("validates JSON for dotted arguments inside params and preserves their literal names", async () => {
+    const ref = renderForm({
+      type: "object",
+      properties: {
+        params: {
+          type: "object",
+          properties: { "filter.options": { type: "object" } },
+          required: ["filter.options"],
+        },
+      },
+      required: [],
+    });
+    const field = screen.getByRole("textbox", { name: "filter.options *" });
+    fireEvent.change(field, { target: { value: "invalid" } });
+
+    expect(await submitError(ref)).toEqual({
+      errorFields: [{ name: ["filter.options"], errors: ["Invalid JSON"] }],
+    });
+    expect(await screen.findByText("Invalid JSON")).toBeInTheDocument();
+
+    fireEvent.change(field, { target: { value: '{"region":"eu"}' } });
+    await expect(submit(ref)).resolves.toEqual({ params: { "filter.options": { region: "eu" } } });
+  });
+
+  it("resets dotted defaults and positional values when the selected tool changes", async () => {
+    const ref = React.createRef<MCPToolArgumentsFormRef>();
+    const { rerender } = render(
+      <MCPToolArgumentsForm
+        ref={ref}
+        tool={toolWith({
+          type: "object",
+          properties: { "filter.category": { type: "string", default: "invoices" } },
+          required: [],
+        })}
+      />,
+    );
+    expect(screen.getByRole("textbox", { name: "filter.category" })).toHaveValue("invoices");
+    await expect(submit(ref)).resolves.toEqual({ "filter.category": "invoices" });
+    fireEvent.change(screen.getByRole("textbox", { name: "filter.category" }), {
+      target: { value: "edited" },
+    });
+    await expect(submit(ref)).resolves.toEqual({ "filter.category": "edited" });
+
+    rerender(
+      <MCPToolArgumentsForm
+        ref={ref}
+        tool={{
+          ...toolWith({
+            type: "object",
+            properties: {
+              query: { type: "string", default: "new tool" },
+              "filter.category": { type: "string", default: "receipts" },
+            },
+            required: [],
+          }),
+          name: "another_tool",
+        }}
+      />,
+    );
+    expect(screen.getByRole("textbox", { name: "filter.category" })).toHaveValue("receipts");
+    await expect(submit(ref)).resolves.toEqual({ query: "new tool", "filter.category": "receipts" });
+  });
+
   it("returns typed values for a string, integer, number and boolean field", async () => {
     const user = userEvent.setup();
     const ref = renderForm({

ui/litellm-dashboard/src/components/mcp_tools/MCPToolArgumentsForm.tsx

@@ -1,6 +1,6 @@
 import React, { forwardRef, useImperativeHandle, useMemo } from "react";
 import { CircleHelp } from "lucide-react";
-import { useForm, type Resolver } from "react-hook-form";
+import { useForm, type Resolver, type ResolverResult } from "react-hook-form";
 import { FieldGroup } from "@/components/ui/field";
 import { FormField } from "@/components/shared/form/FormField";
 import { Input } from "@/components/ui/input";
@@ -9,7 +9,10 @@ import { Textarea } from "@/components/ui/textarea";
 import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip";
 import { MCPTool, InputSchema, InputSchemaProperty } from "./types";
 
-type ToolFormValues = Record<string, unknown>;
+type ToolFormValues = { args: unknown[] };
+
+const argumentValues = (schema: InputSchema, values: ToolFormValues): Record<string, unknown> =>
+  Object.fromEntries(Object.keys(schema.properties ?? {}).map((key, index) => [key, values.args[index]]));
 
 const STRING_SCHEMA_MESSAGES: Readonly<Record<string, string>> = { input: "Please enter input for this tool" };
 
@@ -38,7 +41,7 @@ type FieldError = { type: string; message: string };
 const collectErrors = (
   actualSchema: InputSchema,
   requiredMessages: Readonly<Record<string, string>>,
-  values: ToolFormValues,
+  values: Record<string, unknown>,
 ): Record<string, FieldError> => {
   const entries = Object.entries(actualSchema.properties ?? {}).flatMap<[string, FieldError]>(([key, prop]) => {
     const value = values[key];
@@ -56,9 +59,19 @@ const collectErrors = (
 
 const buildResolver =
   (actualSchema: InputSchema, requiredMessages: Readonly<Record<string, string>> = {}): Resolver<ToolFormValues> =>
-  (values) => {
-    const errors = collectErrors(actualSchema, requiredMessages, values);
-    return Object.keys(errors).length > 0 ? { values: {}, errors } : { values, errors: {} };
+  (values): ResolverResult<ToolFormValues> => {
+    const errors = collectErrors(actualSchema, requiredMessages, argumentValues(actualSchema, values));
+    if (Object.keys(errors).length === 0) return { values, errors: {} };
+    return {
+      values: {},
+      errors: {
+        args: Object.fromEntries(
+          Object.keys(actualSchema.properties ?? {}).flatMap((key, index) =>
+            Object.hasOwn(errors, key) ? [[index, errors[key]]] : [],
+          ),
+        ),
+      },
+    };
   };
 
 const labelFor = (key: string, prop: InputSchemaProperty, required: boolean): React.ReactNode => (
@@ -238,10 +251,7 @@ const MCPToolArgumentsForm = forwardRef<MCPToolArgumentsFormRef, MCPToolArgument
     }, [schema]);
 
     const defaultValues = useMemo<ToolFormValues>(
-      () =>
-        Object.fromEntries(
-          Object.entries(actualSchema.properties ?? {}).map(([key, prop]) => [key, getInitialValueForField(prop)]),
-        ),
+      () => ({ args: Object.values(actualSchema.properties ?? {}).map(getInitialValueForField) }),
       [actualSchema],
     );
 
@@ -255,7 +265,7 @@ const MCPToolArgumentsForm = forwardRef<MCPToolArgumentsFormRef, MCPToolArgument
 
     useImperativeHandle(ref, () => ({
       getSubmitValues: async () => {
-        const values = form.getValues();
+        const values = argumentValues(actualSchema, form.getValues());
         const errors = collectErrors(actualSchema, requiredMessages, values);
         if (Object.keys(errors).length > 0) {
           await form.trigger();
@@ -286,14 +296,16 @@ const MCPToolArgumentsForm = forwardRef<MCPToolArgumentsFormRef, MCPToolArgument
           <FieldGroup>
             <FormField
               control={form.control}
-              name="input"
+              name="args.0"
               label={
                 <span>
                   Input <span className="text-destructive">*</span>
                 </span>
               }
             >
-              {(field) => <Input {...field} value={field.value as string} placeholder="Enter input for this tool" />}
+              {(field) => (
+                <Input {...field} value={(field.value as string) ?? ""} placeholder="Enter input for this tool" />
+              )}
             </FormField>
           </FieldGroup>
         </form>
@@ -318,13 +330,13 @@ const MCPToolArgumentsForm = forwardRef<MCPToolArgumentsFormRef, MCPToolArgument
           className={className}
         >
           <FieldGroup>
-            {Object.entries(actualSchema.properties).map(([key, prop]) => {
+            {Object.entries(actualSchema.properties).map(([key, prop], index) => {
               const required = actualSchema.required?.includes(key) ?? false;
               return (
                 <FormField
                   key={`${tool.name}-${key}`}
                   control={form.control}
-                  name={key}
+                  name={`args.${index}`}
                   label={labelFor(key, prop, required)}
                 >
                   {(field) => {
@@ -375,7 +387,7 @@ const MCPToolArgumentsForm = forwardRef<MCPToolArgumentsFormRef, MCPToolArgument
                           {...field}
                           type="number"
                           step={prop.type === "integer" ? 1 : undefined}
-                          value={field.value as number | string}
+                          value={(field.value as number | string) ?? ""}
                           placeholder={prop.description || `Enter ${key}`}
                         />
                       );
@@ -385,7 +397,7 @@ const MCPToolArgumentsForm = forwardRef<MCPToolArgumentsFormRef, MCPToolArgument
                         <Textarea
                           {...field}
                           rows={prop.type === "object" ? 4 : 3}
-                          value={field.value as string}
+                          value={(field.value as string) ?? ""}
                           spellCheck={false}
                           className="font-mono"
                           placeholder={
@@ -398,7 +410,7 @@ const MCPToolArgumentsForm = forwardRef<MCPToolArgumentsFormRef, MCPToolArgument
                     return (
                       <Input
                         {...field}
-                        value={field.value as string}
+                        value={(field.value as string) ?? ""}
                         placeholder={prop.description || `Enter ${key}`}
                       />
                     );