From c180849210a8da007860951e0f995ef701331a55 Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Tue, 18 Aug 2026 14:41:53 -0700 Subject: [PATCH] refactor(ui): migrate the MCP per-user env vars, toolset and tool arguments forms to react-hook-form and shadcn (#37349) * refactor(ui): migrate the MCP per-user env vars modal to react-hook-form and shadcn Moves UserEnvVarsModal off the antd Form store onto react-hook-form with a zod schema built from the server's declared per-user variables, and swaps antd Input.Password for the shared PasswordInput. The submit payload is unchanged: every declared variable is still sent as a key, trimmed, with an untouched field sending an empty string. antd reset the store from the modal's afterOpenChange; the migrated form reproduces that by remounting on the same callback, so reopening still starts blank. Adds UserEnvVarsModal.test.tsx, which was written against the antd original and proven green before any production change, then re-run unedited against the migration. Two further cases cover the reveal toggle, which antd provided through visibilityToggle. * refactor(ui): migrate the MCP toolset create and edit form to react-hook-form and shadcn Moves the toolset name and description fields off the antd Form store onto react-hook-form with a zod schema, and takes the surrounding panel onto semantic colour tokens so the tab renders in dark mode. The purple selected tool styling keeps its hue and gains dark variants rather than flattening to neutral. Payload is unchanged: create still sends toolset_name, description and tools, an untouched description is still the empty string rather than undefined, and the tool selection is still held outside the form. The antd form carried no onFinish and its buttons sit outside the form element, so the migrated form keeps submit on the footer button and neutralises its own submit rather than introducing Enter to save. Adds MCPToolsetsTab.test.tsx, proven green against the antd original before any production change and re-run unedited afterwards. * refactor(ui): migrate the MCP tool arguments form to react-hook-form and shadcn Moves the schema-driven tool argument form off the antd Form store onto react-hook-form. Validation moves to an explicit resolver that reproduces antd's rules field by field, including the per-field required message and the JSON object and array messages, and the same resolver is reused by getSubmitValues so the imperative path and the rendered errors cannot disagree. getSubmitValues still rejects with a plain object carrying errorFields rather than an Error. ChatUI branches on `err instanceof Error` to choose its toast, so rejecting with an Error would have silently changed the message the user sees. That is pinned by a test proven green against the antd original with a Form.Item liveness gate, and proven red when the rejection is switched to an Error. Enum and boolean fields keep the antd Select, whose allowClear has no shadcn equivalent; dropping it would remove the only way to unset an optional enum. Everything else moves to the shadcn Input and Textarea and onto semantic colour tokens. Adds MCPToolArgumentsForm.test.tsx covering the string, integer, number, boolean, object, array, nested-params and string-schema paths, written against the antd original and re-run unedited afterwards. * refactor(ui): drop the decorative antd Form.Item from the MCP connect guide The connect guide rendered a single antd Form.Item with no field name and no Form ancestor, so it registered nothing and carried no payload; it was only supplying bottom margin. It becomes a div with the same margin class, which removes the file's last antd Form dependency. Also takes the guide onto semantic colour tokens so it renders in dark mode. The blue and green callouts keep their hue and gain dark variants rather than flattening to neutral, since the colour carries meaning there. * chore(ui): ratchet the MCP tool arguments form lint suppressions The react-hook-form migration removed four of the five nested ternaries in MCPToolArgumentsForm, so lower the grandfathered count to match and hoist the one inline object literal the budget rule flags. * test(ui): classify the MCP modal batteries as integration tests Both render a real component tree down to the form controls and stub only the network boundary, which is the repo's definition of an integration test rather than a unit test. The tool arguments battery renders a single module in milliseconds, so it stays unsuffixed. --- ui/litellm-dashboard/eslint-suppressions.json | 2 +- .../MCPToolsetsTab.integration.test.tsx | 195 ++++++++++ .../_components/MCPToolsetsTab.tsx | 141 ++++---- .../UserEnvVarsModal.integration.test.tsx | 211 +++++++++++ .../_components/UserEnvVarsModal.tsx | 116 ++++-- .../mcp-servers/_components/mcp_connect.tsx | 80 ++--- .../mcp_tools/MCPToolArgumentsForm.test.tsx | 180 ++++++++++ .../mcp_tools/MCPToolArgumentsForm.tsx | 340 +++++++++++------- 8 files changed, 997 insertions(+), 268 deletions(-) create mode 100644 ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPToolsetsTab.integration.test.tsx create mode 100644 ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/UserEnvVarsModal.integration.test.tsx create mode 100644 ui/litellm-dashboard/src/components/mcp_tools/MCPToolArgumentsForm.test.tsx diff --git a/ui/litellm-dashboard/eslint-suppressions.json b/ui/litellm-dashboard/eslint-suppressions.json index 6fc646e0069..e11ff09d41b 100644 --- a/ui/litellm-dashboard/eslint-suppressions.json +++ b/ui/litellm-dashboard/eslint-suppressions.json @@ -2354,7 +2354,7 @@ }, "src/components/mcp_tools/MCPToolArgumentsForm.tsx": { "no-nested-ternary": { - "count": 5 + "count": 1 }, "no-restricted-imports": { "count": 1 diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPToolsetsTab.integration.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPToolsetsTab.integration.test.tsx new file mode 100644 index 00000000000..2b192f1777d --- /dev/null +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPToolsetsTab.integration.test.tsx @@ -0,0 +1,195 @@ +import React from "react"; +import { render, screen, waitFor, within } from "@testing-library/react"; +import userEvent, { PointerEventsCheckLevel } from "@testing-library/user-event"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { MCPToolsetsTab } from "./MCPToolsetsTab"; +import * as networking from "@/components/networking"; +import { useMCPToolsets } from "@/app/(dashboard)/hooks/mcpServers/useMCPToolsets"; +import { useMCPServers } from "@/app/(dashboard)/hooks/mcpServers/useMCPServers"; +import { MCPToolset } from "@/components/mcp_tools/types"; + +vi.mock("@/components/networking", () => ({ + createMCPToolset: vi.fn(), + updateMCPToolset: vi.fn(), + deleteMCPToolset: vi.fn(), + listMCPTools: vi.fn(), + getProxyBaseUrl: vi.fn().mockReturnValue("http://localhost:4000"), +})); + +vi.mock("@/app/(dashboard)/hooks/mcpServers/useMCPToolsets", () => ({ useMCPToolsets: vi.fn() })); +vi.mock("@/app/(dashboard)/hooks/mcpServers/useMCPServers", () => ({ useMCPServers: vi.fn() })); + +const setup = () => userEvent.setup({ pointerEventsCheck: PointerEventsCheckLevel.Never }); + +const renderTab = (toolsets: MCPToolset[] = []) => { + vi.mocked(useMCPToolsets).mockReturnValue({ + data: toolsets, + isLoading: false, + } as unknown as ReturnType); + vi.mocked(useMCPServers).mockReturnValue({ data: [] } as unknown as ReturnType); + render( + + + , + ); +}; + +const dialogWithButton = async (name: string) => { + const button = await screen.findByRole("button", { name }); + const dialog = button.closest('[role="dialog"]'); + if (dialog === null) { + throw new Error(`no dialog contains a "${name}" button`); + } + return within(dialog as HTMLElement); +}; + +const openEditFor = async (user: ReturnType) => { + await user.click(await screen.findByRole("button", { name: "Open toolset actions" })); + await user.click(await screen.findByRole("menuitem", { name: "Edit" })); + return dialogWithButton("Save Changes"); +}; + +const openCreate = async (user: ReturnType) => { + await user.click(screen.getByRole("button", { name: /new toolset/i })); + return dialogWithButton("Create Toolset"); +}; + +describe("MCPToolsetsTab create/edit toolset form", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("creates a toolset with the typed name, description and no tools", async () => { + const user = setup(); + vi.mocked(networking.createMCPToolset).mockResolvedValue( + {} as Awaited>, + ); + renderTab(); + + const dialog = await openCreate(user); + await user.type(dialog.getByPlaceholderText("e.g. github-linear-tools"), "github-linear-tools"); + await user.type(dialog.getByPlaceholderText("Optional description"), "tools for triage"); + await user.click(dialog.getByRole("button", { name: "Create Toolset" })); + + await waitFor(() => { + expect(networking.createMCPToolset).toHaveBeenCalledWith("sk-test", { + toolset_name: "github-linear-tools", + description: "tools for triage", + tools: [], + }); + }); + expect(networking.createMCPToolset).toHaveBeenCalledTimes(1); + }); + + it("sends an empty string when the description is left untouched", async () => { + const user = setup(); + vi.mocked(networking.createMCPToolset).mockResolvedValue( + {} as Awaited>, + ); + renderTab(); + + const dialog = await openCreate(user); + await user.type(dialog.getByPlaceholderText("e.g. github-linear-tools"), "solo"); + await user.click(dialog.getByRole("button", { name: "Create Toolset" })); + + await waitFor(() => { + expect(networking.createMCPToolset).toHaveBeenCalledWith("sk-test", { + toolset_name: "solo", + description: "", + tools: [], + }); + }); + }); + + it("blocks the submit and shows the required message when the name is empty", async () => { + const user = setup(); + renderTab(); + + const dialog = await openCreate(user); + await user.click(dialog.getByRole("button", { name: "Create Toolset" })); + + expect(await dialog.findByText("Please enter a toolset name")).toBeInTheDocument(); + expect(networking.createMCPToolset).not.toHaveBeenCalled(); + }); + + it("does not treat a whitespace-only description as absent", async () => { + const user = setup(); + vi.mocked(networking.createMCPToolset).mockResolvedValue( + {} as Awaited>, + ); + renderTab(); + + const dialog = await openCreate(user); + await user.type(dialog.getByPlaceholderText("e.g. github-linear-tools"), "spaced"); + await user.type(dialog.getByPlaceholderText("Optional description"), " "); + await user.click(dialog.getByRole("button", { name: "Create Toolset" })); + + await waitFor(() => { + expect(networking.createMCPToolset).toHaveBeenCalledWith("sk-test", { + toolset_name: "spaced", + description: " ", + tools: [], + }); + }); + }); + + it("seeds the edit form from the toolset and updates it by id", async () => { + const user = setup(); + vi.mocked(networking.updateMCPToolset).mockResolvedValue( + {} as Awaited>, + ); + const toolset = { + toolset_id: "ts-1", + toolset_name: "existing", + description: "old description", + tools: [{ server_id: "srv-1", tool_name: "search" }], + } as MCPToolset; + renderTab([toolset]); + + const dialog = await openEditFor(user); + const name = await dialog.findByDisplayValue("existing"); + expect(name).toBe(dialog.getByPlaceholderText("e.g. github-linear-tools")); + expect(dialog.getByText("Toolset Name")).toBeInTheDocument(); + expect(dialog.getByPlaceholderText("Optional description")).toHaveValue("old description"); + + await user.clear(name); + await user.type(name, "renamed"); + await user.click(dialog.getByRole("button", { name: "Save Changes" })); + + const expectedUpdate = { + toolset_id: "ts-1", + toolset_name: "renamed", + description: "old description", + tools: [{ server_id: "srv-1", tool_name: "search" }], + }; + await waitFor(() => { + expect(networking.updateMCPToolset).toHaveBeenCalledWith("sk-test", expectedUpdate); + }); + }); + + it("seeds an absent description as an empty string rather than failing", async () => { + const user = setup(); + vi.mocked(networking.updateMCPToolset).mockResolvedValue( + {} as Awaited>, + ); + const toolset = { + toolset_id: "ts-2", + toolset_name: "no-desc", + description: null, + tools: [], + } as unknown as MCPToolset; + renderTab([toolset]); + + const dialog = await openEditFor(user); + await dialog.findByDisplayValue("no-desc"); + expect(dialog.getByPlaceholderText("Optional description")).toHaveValue(""); + + await user.click(dialog.getByRole("button", { name: "Save Changes" })); + + const expectedUpdate = { toolset_id: "ts-2", toolset_name: "no-desc", description: "", tools: [] }; + await waitFor(() => { + expect(networking.updateMCPToolset).toHaveBeenCalledWith("sk-test", expectedUpdate); + }); + }); +}); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPToolsetsTab.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPToolsetsTab.tsx index a7a56fe040f..8d0901d7176 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPToolsetsTab.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPToolsetsTab.tsx @@ -1,7 +1,6 @@ import React, { useState, useCallback } from "react"; -import { Button } from "@/components/ui/button"; -import { UiLoadingSpinner } from "@/components/ui/ui-loading-spinner"; -import { Modal, Form, Input, message, Spin } from "antd"; +import { Modal, Input, message, Spin } from "antd"; +import { z } from "zod/v4"; import { SortingState } from "@tanstack/react-table"; import { Inbox, Plus } from "lucide-react"; import { useMCPToolsets } from "@/app/(dashboard)/hooks/mcpServers/useMCPToolsets"; @@ -16,6 +15,12 @@ import { getProxyBaseUrl, } from "@/components/networking"; import { MCPToolset, MCPToolsetTool } from "@/components/mcp_tools/types"; +import { FieldGroup } from "@/components/shared/form/field"; +import { FormField } from "@/components/shared/form/FormField"; +import { Button } from "@/components/ui/button"; +import { Input as ShadcnInput } from "@/components/ui/input"; +import { UiLoadingSpinner } from "@/components/ui/ui-loading-spinner"; +import { useZodForm } from "@/lib/forms/useZodForm"; import { displayToolName, getMCPToolsetTableColumns } from "./MCPToolsetTableColumns"; interface MCPToolsetsTabProps { @@ -23,10 +28,12 @@ interface MCPToolsetsTabProps { userRole: string | null; } -interface ToolsetFormValues { - toolset_name: string; - description?: string; -} +const toolsetSchema = z.object({ + toolset_name: z.string().min(1, "Please enter a toolset name"), + description: z.string(), +}); + +type ToolsetFormValues = z.infer; interface MCPToolListProps { serverId: string; @@ -68,20 +75,22 @@ function MCPToolList({ serverId, serverName, accessToken, selectedTools, onToggl }; return ( -
+
{expanded && (
@@ -90,7 +99,7 @@ function MCPToolList({ serverId, serverName, accessToken, selectedTools, onToggl
) : tools.length === 0 ? ( -

No tools found for this server.

+

No tools found for this server.

) : (
{tools.map((tool) => { @@ -102,21 +111,27 @@ function MCPToolList({ serverId, serverName, accessToken, selectedTools, onToggl onClick={() => onToggle({ server_id: serverId, tool_name: tool.name })} className={`flex items-start justify-between px-3 py-2 rounded-lg text-left transition-colors ${ selected - ? "bg-purple-50 border border-purple-300" - : "bg-white border border-gray-100 hover:bg-gray-50" + ? "bg-purple-50 border border-purple-300 dark:bg-purple-950 dark:border-purple-700" + : "bg-card border border-border hover:bg-muted" }`} >

{tool.name}

{tool.description && ( -

{tool.description}

+

+ {tool.description} +

)}
- {selected && ✓} + {selected && ( + + ✓ + + )} ); })} @@ -137,7 +152,12 @@ interface CreateToolsetModalProps { } function CreateToolsetModal({ open, onClose, onSave, accessToken, initialToolset }: CreateToolsetModalProps) { - const [form] = Form.useForm(); + const form = useZodForm(toolsetSchema, { + defaultValues: { + toolset_name: initialToolset?.toolset_name || "", + description: initialToolset?.description || "", + }, + }); const [selectedTools, setSelectedTools] = useState(initialToolset?.tools || []); const [saving, setSaving] = useState(false); const [serverSearch, setServerSearch] = useState(""); @@ -149,14 +169,14 @@ function CreateToolsetModal({ open, onClose, onSave, accessToken, initialToolset React.useEffect(() => { if (open) { - form.setFieldsValue({ + form.reset({ toolset_name: initialToolset?.toolset_name || "", description: initialToolset?.description || "", }); setSelectedTools(initialToolset?.tools || []); setServerSearch(""); } - }, [open, initialToolset]); + }, [open, initialToolset, form]); const handleToggleTool = (tool: MCPToolsetTool) => { setSelectedTools((prev) => { @@ -167,8 +187,7 @@ function CreateToolsetModal({ open, onClose, onSave, accessToken, initialToolset }); }; - const handleSubmit = async () => { - const values = await form.validateFields(); + const handleSubmit = async (values: ToolsetFormValues) => { setSaving(true); try { await onSave(values.toolset_name, values.description, selectedTools); @@ -192,27 +211,22 @@ function CreateToolsetModal({ open, onClose, onSave, accessToken, initialToolset footer={null} forceRender > -
-
- - - - - - -
-
+
event.preventDefault()} className="mt-2"> + + + {(field) => } + + + {(field) => } + + +
{/* Left panel: Available Tools */}
-

Available Tools

+

Available Tools

{filteredServers.length === 0 ? ( -

+

{mcpServers.length === 0 ? "No MCP servers configured" : "No servers match your search"}

) : ( @@ -242,31 +256,36 @@ function CreateToolsetModal({ open, onClose, onSave, accessToken, initialToolset
{/* Divider */} -
+
{/* Right panel: Your Toolset */}
-

- Your Toolset ({selectedTools.length} tools) +

+ Your Toolset{" "} + ({selectedTools.length} tools)

{selectedTools.length === 0 ? ( -

No tools added yet

+

No tools added yet

) : ( selectedTools.map((tool, idx) => ( )) )} @@ -274,11 +293,11 @@ function CreateToolsetModal({ open, onClose, onSave, accessToken, initialToolset
-
- - @@ -325,22 +344,22 @@ function ToolsetUsageGuide() { }; return ( -
-

How toolsets work

-

+

+

How toolsets work

+

Create a toolset, assign it to a key via{" "} - API Keys → Edit Key → MCP Servers, then point your MCP client - at the toolset URL. The client only sees the tools you picked. + API Keys → Edit Key → MCP Servers, then point your MCP + client at the toolset URL. The client only sees the tools you picked.

-
Claude Code / Cursor config
+
Claude Code / Cursor config
- 
+        
           {snippet}
         

         
@@ -407,8 +426,8 @@ export function MCPToolsetsTab({ accessToken, userRole }: MCPToolsetsTabProps) {
     

       

         

-          
MCP Toolsets

-          

+          
MCP Toolsets

+          

             Curated collections of tools from one or more MCP servers. Assign toolsets to keys and teams via the MCP
             permissions dropdown.
           

diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/UserEnvVarsModal.integration.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/UserEnvVarsModal.integration.test.tsx
new file mode 100644
index 00000000000..9c91a25cf37
--- /dev/null
+++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/UserEnvVarsModal.integration.test.tsx
@@ -0,0 +1,211 @@
+import React from "react";
+import { render, screen, waitFor } from "@testing-library/react";
+import userEvent, { PointerEventsCheckLevel } from "@testing-library/user-event";
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
+import UserEnvVarsModal from "./UserEnvVarsModal";
+import * as networking from "@/components/networking";
+import { MCPServer, MCPUserEnvVarsStatus } from "@/components/mcp_tools/types";
+
+vi.mock("@/components/networking", () => ({
+  getMCPUserEnvVars: vi.fn(),
+  storeMCPUserEnvVars: vi.fn(),
+}));
+
+const createQueryClient = () => new QueryClient({ defaultOptions: { queries: { retry: false, gcTime: 0 } } });
+
+const setup = () => userEvent.setup({ pointerEventsCheck: PointerEventsCheckLevel.Never });
+
+const server = { server_id: "srv-1", server_name: "Payments", alias: "payments" } as MCPServer;
+
+const statusWith = (required: MCPUserEnvVarsStatus["required"]): MCPUserEnvVarsStatus =>
+  ({ required }) as MCPUserEnvVarsStatus;
+
+const renderModal = (status: MCPUserEnvVarsStatus, onSaved = vi.fn(), onClose = vi.fn()) => {
+  vi.mocked(networking.getMCPUserEnvVars).mockResolvedValue(status);
+  render(
+    
+      
+    ,
+  );
+  return { onSaved, onClose };
+};
+
+const save = (user: ReturnType) => user.click(screen.getByRole("button", { name: "Save Credentials" }));
+
+const fieldAfterOpen = async (label: RegExp): Promise => {
+  const initial = await screen.findByLabelText(label);
+  await waitFor(() => {
+    expect(initial).not.toBeInTheDocument();
+  });
+  return screen.getByLabelText(label);
+};
+
+describe("UserEnvVarsModal", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it("submits every declared field, trimmed, keyed by env var name", async () => {
+    const user = setup();
+    vi.mocked(networking.storeMCPUserEnvVars).mockResolvedValue(statusWith([]));
+    renderModal(
+      statusWith([
+        { name: "API_KEY", description: "Your API key", is_set: false },
+        { name: "REGION", description: null, is_set: false },
+      ]),
+    );
+
+    await user.type(await fieldAfterOpen(/^API_KEY/), "  secret-value  ");
+    await user.type(screen.getByLabelText(/^REGION/), "us-east-1");
+    await save(user);
+
+    await waitFor(() => {
+      expect(networking.storeMCPUserEnvVars).toHaveBeenCalledWith("sk-test", "srv-1", {
+        API_KEY: "secret-value",
+        REGION: "us-east-1",
+      });
+    });
+    expect(networking.storeMCPUserEnvVars).toHaveBeenCalledTimes(1);
+  });
+
+  it("sends an empty string for an already-set field left blank", async () => {
+    const user = setup();
+    vi.mocked(networking.storeMCPUserEnvVars).mockResolvedValue(statusWith([]));
+    renderModal(
+      statusWith([
+        { name: "API_KEY", description: null, is_set: true },
+        { name: "REGION", description: null, is_set: true },
+      ]),
+    );
+
+    await user.type(await fieldAfterOpen(/^REGION/), "eu-west-2");
+    await save(user);
+
+    await waitFor(() => {
+      expect(networking.storeMCPUserEnvVars).toHaveBeenCalledWith("sk-test", "srv-1", {
+        API_KEY: "",
+        REGION: "eu-west-2",
+      });
+    });
+  });
+
+  it("blocks the submit and shows the required message when an unset field is empty", async () => {
+    const user = setup();
+    renderModal(
+      statusWith([
+        { name: "API_KEY", description: null, is_set: false },
+        { name: "REGION", description: null, is_set: true },
+      ]),
+    );
+
+    await fieldAfterOpen(/^API_KEY/);
+    await save(user);
+
+    expect(await screen.findByText("API_KEY is required")).toBeInTheDocument();
+    expect(networking.storeMCPUserEnvVars).not.toHaveBeenCalled();
+  });
+
+  it("does not require an already-set field", async () => {
+    const user = setup();
+    vi.mocked(networking.storeMCPUserEnvVars).mockResolvedValue(statusWith([]));
+    renderModal(statusWith([{ name: "API_KEY", description: null, is_set: true }]));
+
+    await fieldAfterOpen(/^API_KEY/);
+    await save(user);
+
+    await waitFor(() => {
+      expect(networking.storeMCPUserEnvVars).toHaveBeenCalledWith("sk-test", "srv-1", { API_KEY: "" });
+    });
+  });
+
+  it("renders the admin description as the placeholder for an unset field", async () => {
+    renderModal(statusWith([{ name: "API_KEY", description: "Grab it from the console", is_set: false }]));
+
+    expect(await screen.findByPlaceholderText("Grab it from the console")).toBeInTheDocument();
+  });
+
+  it("renders the overwrite placeholder and a Set marker for an already-set field", async () => {
+    renderModal(statusWith([{ name: "API_KEY", description: "Grab it from the console", is_set: true }]));
+
+    expect(await screen.findByPlaceholderText("Enter a new value to overwrite")).toBeInTheDocument();
+    expect(screen.getByText("Set")).toBeInTheDocument();
+  });
+
+  it("renders the admin description as always-visible help text", async () => {
+    renderModal(statusWith([{ name: "API_KEY", description: "Grab it from the console", is_set: true }]));
+
+    await fieldAfterOpen(/^API_KEY/);
+    expect(screen.getByText("Grab it from the console")).toBeVisible();
+  });
+
+  it("masks the entered value", async () => {
+    const user = setup();
+    renderModal(statusWith([{ name: "API_KEY", description: null, is_set: false }]));
+
+    const input = await fieldAfterOpen(/^API_KEY/);
+    expect(input).toHaveAttribute("type", "password");
+    await user.type(input, "hunter2");
+    expect(screen.getByLabelText(/^API_KEY/)).toHaveAttribute("type", "password");
+  });
+
+  it("reports the empty state instead of a form when nothing is required", async () => {
+    renderModal(statusWith([]));
+
+    expect(await screen.findByText("No per-user fields configured for this server.")).toBeInTheDocument();
+    expect(screen.queryByRole("button", { name: "Save Credentials" })).not.toBeInTheDocument();
+  });
+
+  it("closes and reports the saved status on success", async () => {
+    const user = setup();
+    const saved = statusWith([{ name: "API_KEY", description: null, is_set: true }]);
+    vi.mocked(networking.storeMCPUserEnvVars).mockResolvedValue(saved);
+    const { onSaved, onClose } = renderModal(statusWith([{ name: "API_KEY", description: null, is_set: false }]));
+
+    await user.type(await fieldAfterOpen(/^API_KEY/), "abc");
+    await save(user);
+
+    await waitFor(() => {
+      expect(onSaved).toHaveBeenCalledWith(saved);
+    });
+    expect(onClose).toHaveBeenCalled();
+  });
+
+  it("reveals and re-masks the value through the visibility toggle", async () => {
+    const user = setup();
+    renderModal(statusWith([{ name: "API_KEY", description: null, is_set: false }]));
+
+    await user.type(await fieldAfterOpen(/^API_KEY/), "hunter2");
+    await user.click(screen.getByRole("button", { name: "Show password" }));
+    expect(screen.getByLabelText(/^API_KEY/)).toHaveAttribute("type", "text");
+    expect(screen.getByLabelText(/^API_KEY/)).toHaveValue("hunter2");
+
+    await user.click(screen.getByRole("button", { name: "Hide password" }));
+    expect(screen.getByLabelText(/^API_KEY/)).toHaveAttribute("type", "password");
+  });
+
+  it("does not submit when the visibility toggle is clicked", async () => {
+    const user = setup();
+    renderModal(statusWith([{ name: "API_KEY", description: null, is_set: true }]));
+
+    await fieldAfterOpen(/^API_KEY/);
+    await user.click(screen.getByRole("button", { name: "Show password" }));
+
+    expect(networking.storeMCPUserEnvVars).not.toHaveBeenCalled();
+  });
+
+  it("surfaces a save failure without closing", async () => {
+    const user = setup();
+    vi.mocked(networking.storeMCPUserEnvVars).mockRejectedValue(new Error("boom"));
+    const { onSaved, onClose } = renderModal(statusWith([{ name: "API_KEY", description: null, is_set: false }]));
+
+    await user.type(await fieldAfterOpen(/^API_KEY/), "abc");
+    await save(user);
+
+    await waitFor(() => {
+      expect(networking.storeMCPUserEnvVars).toHaveBeenCalledTimes(1);
+    });
+    expect(onSaved).not.toHaveBeenCalled();
+    expect(onClose).not.toHaveBeenCalled();
+  });
+});
diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/UserEnvVarsModal.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/UserEnvVarsModal.tsx
index e92567c3974..df97f05c6fe 100644
--- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/UserEnvVarsModal.tsx
+++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/UserEnvVarsModal.tsx
@@ -1,9 +1,16 @@
 import React from "react";
-import { Modal, Form, Input, Button, Alert, Spin, Tag, Typography } from "antd";
+import { Modal, Alert, Spin, Tag, Typography } from "antd";
 import { useMutation, useQuery } from "@tanstack/react-query";
-import { MCPServer, MCPUserEnvVarsStatus } from "@/components/mcp_tools/types";
+import { z } from "zod/v4";
+import { MCPServer, MCPUserEnvVarsStatus, MCPUserEnvVarSpec } from "@/components/mcp_tools/types";
 import { getMCPUserEnvVars, storeMCPUserEnvVars } from "@/components/networking";
 import { toast } from "@/lib/toast";
+import { FieldGroup } from "@/components/shared/form/field";
+import { FormField } from "@/components/shared/form/FormField";
+import { PasswordInput } from "@/components/shared/PasswordInput";
+import { Button } from "@/components/ui/button";
+import { UiLoadingSpinner } from "@/components/ui/ui-loading-spinner";
+import { useZodForm } from "@/lib/forms/useZodForm";
 
 const { Text, Title } = Typography;
 
@@ -15,6 +22,67 @@ interface UserEnvVarsModalProps {
   onSaved?: (status: MCPUserEnvVarsStatus) => void;
 }
 
+interface UserEnvVarsFormProps {
+  required: readonly MCPUserEnvVarSpec[];
+  isSaving: boolean;
+  onCancel: () => void;
+  onSubmit: (values: Record) => void;
+}
+
+const buildSchema = (required: readonly MCPUserEnvVarSpec[]) =>
+  z.object(
+    Object.fromEntries(
+      required.map((spec) => [spec.name, spec.is_set ? z.string() : z.string().min(1, `${spec.name} is required`)]),
+    ),
+  );
+
+const emptyValues = (required: readonly MCPUserEnvVarSpec[]): Record =>
+  Object.fromEntries(required.map((spec) => [spec.name, ""]));
+
+const UserEnvVarsForm: React.FC = ({ required, isSaving, onCancel, onSubmit }) => {
+  const form = useZodForm(buildSchema(required), { defaultValues: emptyValues(required) });
+
+  return (
+    

+      
+        {required.map((spec) => (
+          
+                {spec.name}
+                {spec.is_set && Set}
+              
+            }
+          >
+            {(field) => (
+              
+            )}
+          
+        ))}
+      
+      

+        
+        
+      

+    

+  );
+};
+
 /**
  * User-facing modal for filling in per-user MCP environment variables.
  *
@@ -23,7 +91,7 @@ interface UserEnvVarsModalProps {
  * description as the placeholder.
  */
 const UserEnvVarsModal: React.FC = ({ server, open, accessToken, onClose, onSaved }) => {
-  const [form] = Form.useForm();
+  const [formGeneration, setFormGeneration] = React.useState(0);
 
   const {
     data: status,
@@ -68,7 +136,7 @@ const UserEnvVarsModal: React.FC = ({ server, open, acces
       width={520}
       destroyOnHidden
       afterOpenChange={(opened) => {
-        if (opened) form.resetFields();
+        if (opened) setFormGeneration((generation) => generation + 1);
       }}
       title={
         

@@ -95,42 +163,18 @@ const UserEnvVarsModal: React.FC = ({ server, open, acces
           
         ) : (
           <>
-            
+            
               These values are private to you. Your admin configured this MCP server to require these per-user
               credentials. Saved values are never shown back; leave an already-set field blank to keep it, or enter a
               value to set or change it.
             
-            

-              {required.map((spec) => (
-                
-                      {spec.name}
-                      {spec.is_set && Set}
-                    
-                  }
-                  extra={spec.description || undefined}
-                  rules={spec.is_set ? undefined : [{ required: true, message: `${spec.name} is required` }]}
-                >
-                  
-                
-              ))}
-              

-                
-                
-              

-            

+            
           
         )}
       

diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_connect.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_connect.tsx
index e32b968c1d3..066b26a9832 100644
--- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_connect.tsx
+++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_connect.tsx
@@ -1,7 +1,7 @@
 /* eslint-disable react/no-unescaped-entities */
 
 import React, { useState } from "react";
-import { Card, Typography, Space, Alert, Button, Switch, Form } from "antd";
+import { Card, Typography, Space, Alert, Button, Switch } from "antd";
 import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs";
 import { CopyIcon, Code, Terminal, Globe, CheckIcon, ExternalLinkIcon, KeyIcon, ServerIcon, Zap } from "lucide-react";
 import { getProxyBaseUrl } from "@/components/networking";
@@ -49,18 +49,18 @@ const FeatureCard: React.FC = ({
   };
 
   return (
-    
+    
       

-        {icon}
+        {icon}
         

           
             {title}
           
-          {description}
+          {description}
         

       

       {serverName && (title === "Implementation Example" || title === "Configuration") && (
-        
+        

           

             
             
@@ -81,14 +81,14 @@ const FeatureCard: React.FC = ({
                   

                     Option 2: Get a group of MCPs: "dev-group"
                   

-                  

+                  

                     You can also mix both: "Server1,dev-group"
                   

                 

               }
             />
           )}
-        
+        

       )}
       {React.Children.map(children, (child) => {
         if (
@@ -137,13 +137,13 @@ const MCPConnect: React.FC = ({ currentServerAccessGroups = []
     

       {title && (
         

-          
-          
+          
+          
             {title}
           
         

       )}
-      
+      
         

   );
@@ -167,12 +167,12 @@ const MCPConnect: React.FC = ({ currentServerAccessGroups = []
   }> = ({ step, title, children }) => (
     

       

-        

+        

           {step}
         

       

       

-        
+        
           {title}
         
         {children}
@@ -254,21 +254,21 @@ const MCPConnect: React.FC = ({ currentServerAccessGroups = []
 
   const OpenAITab = () => (
     
-      

+      

         

-          
-          
+          <Code className="text-blue-600 dark:text-blue-400" size={24} />
+          <Title level={4} className="mb-0 text-blue-900 dark:text-blue-100">
             OpenAI Responses API Integration
           
         

-        
+        
           Connect OpenAI Responses API to your LiteLLM MCP server for seamless tool integration
         
       

 
       
         }
+          icon={}
           title="API Key Setup"
           description="Configure your OpenAI API key for authentication"
         >
@@ -281,7 +281,7 @@ const MCPConnect: React.FC = ({ currentServerAccessGroups = []
                   href="https://platform.openai.com/api-keys"
                   target="_blank"
                   rel="noopener noreferrer"
-                  className="text-blue-600 hover:text-blue-700 inline-flex items-center gap-1"
+                  className="text-blue-600 hover:text-blue-700 inline-flex items-center gap-1 dark:text-blue-400 dark:hover:text-blue-300"
                 >
                   OpenAI platform 
                 
@@ -292,7 +292,7 @@ const MCPConnect: React.FC = ({ currentServerAccessGroups = []
         
 
         }
+          icon={}
           title="MCP Server Information"
           description="Connection details for your LiteLLM MCP server"
         >
@@ -300,7 +300,7 @@ const MCPConnect: React.FC = ({ currentServerAccessGroups = []
         
 
         }
+          icon={}
           title="Implementation Example"
           description="Complete cURL example for using the Responses API"
           serverName="Zapier Gmail"
@@ -350,27 +350,27 @@ const MCPConnect: React.FC = ({ currentServerAccessGroups = []
         
       

 
-      
-        
+      <Card className="border border-border">
+        <Title level={5} className="mb-4 text-foreground">
           Setup Instructions
         
         
           
-            
-              Use the keyboard shortcut ⇧+⌘+J (Mac) or{" "}
-              Ctrl+Shift+J (Windows/Linux)
+            
+              Use the keyboard shortcut ⇧+⌘+J (Mac) or{" "}
+              Ctrl+Shift+J (Windows/Linux)
             
           
 
           
-            Go to the "MCP Tools" tab and click "New MCP Server"
+            Go to the "MCP Tools" tab and click "New MCP Server"
           
 
           
-            
+            
               Copy the JSON configuration below and paste it into Cursor, then save with{" "}
-              Cmd+S or{" "}
-              Ctrl+S
+              Cmd+S or{" "}
+              Ctrl+S
             
             }
@@ -405,18 +405,18 @@ const MCPConnect: React.FC = ({ currentServerAccessGroups = []
     
       

         

-          
-          
+          <Globe className="text-green-600 dark:text-green-400" size={24} />
+          <Title level={4} className="mb-0 text-green-900 dark:text-green-100">
             Streamable HTTP Transport
           
         

-        
+        
           Connect to LiteLLM MCP using HTTP transport. Compatible with any MCP client that supports HTTP streaming.
         
       

 
       }
+        icon={}
         title="Universal MCP Connection"
         description="Use this URL with any MCP client that supports HTTP transport"
       >
@@ -442,7 +442,7 @@ const MCPConnect: React.FC = ({ currentServerAccessGroups = []