From 64078689d303fdd85d2c9c25ca496cb7bebd834d Mon Sep 17 00:00:00 2001
From: Joshua Valluru <326636767+joshua-berri@users.noreply.github.com>
Date: Mon, 21 Sep 2026 14:18:01 -0700
Subject: [PATCH] refactor(mcp): clear server list lint warnings

---
 .../_components/mcp_servers.test.tsx          | 128 +++++++++++-
 .../mcp-servers/_components/mcp_servers.tsx   | 195 ++++++++++--------
 2 files changed, 232 insertions(+), 91 deletions(-)

diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_servers.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_servers.test.tsx
index 61880363234..7198cb3cce9 100644
--- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_servers.test.tsx
+++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_servers.test.tsx
@@ -167,6 +167,130 @@ describe("MCPServers", () => {
     const myConnections = await screen.findByRole("link", { name: "My Connections" });
     expect(myConnections).toBeVisible();
     expect(myConnections).toHaveAttribute("href", "/ui/connect");
+    for (const name of ["Semantic Filter", "Tool Search", "Network Settings", "Submitted MCPs"]) {
+      const tab = screen.queryByRole("tab", { name });
+      if (userRole === "Admin") {
+        expect(tab).toBeVisible();
+      } else {
+        expect(tab).not.toBeInTheDocument();
+      }
+    }
+    expect(
+      screen.getByRole("button", {
+        name: userRole === "Admin" ? "+ Add New MCP Server" : "+ Submit MCP Server",
+      }),
+    ).toBeVisible();
+  });
+
+  it.each(["cancel", "success", "failure", "unnamed"])("preserves delete confirmation on %s", async (outcome) => {
+    const server: MCPServer = {
+      server_id: "delete-server",
+      server_name: outcome === "unnamed" ? null : "Delete fixture",
+      alias: "delete-alias",
+      url: outcome === "unnamed" ? null : "https://example.com/mcp",
+      created_by: "user",
+      updated_by: "user",
+    };
+    vi.mocked(networking.fetchMCPServers).mockResolvedValue([server]);
+    vi.mocked(networking.fetchMCPServerHealth).mockResolvedValue([]);
+    let finishDelete: () => void = () => {};
+    vi.mocked(networking.deleteMCPServer).mockImplementation(
+      () =>
+        new Promise((resolve, reject) => {
+          finishDelete = () => (outcome === "failure" ? reject(new Error("Delete failed")) : resolve(undefined));
+        }),
+    );
+    render(
+      <QueryClientProvider client={createQueryClient()}>
+        <MCPServers {...defaultProps} />
+      </QueryClientProvider>,
+    );
+    await userEvent.click(await screen.findByRole("button", { name: "Server actions" }));
+    await userEvent.click(await screen.findByRole("menuitem", { name: "Delete" }));
+    const dialog = await screen.findByRole("alertdialog", { name: "Delete MCP Server?" });
+    expect(within(dialog).getByText("delete-server")).toBeVisible();
+    if (outcome === "unnamed") {
+      expect(within(dialog).queryByText("Name")).not.toBeInTheDocument();
+      expect(within(dialog).queryByText("URL")).not.toBeInTheDocument();
+    } else {
+      expect(within(dialog).getByText("Delete fixture")).toBeVisible();
+      expect(within(dialog).getByText("https://example.com/mcp")).toBeVisible();
+    }
+    if (outcome === "cancel") {
+      await userEvent.click(within(dialog).getByRole("button", { name: "Cancel" }));
+      expect(networking.deleteMCPServer).not.toHaveBeenCalled();
+    } else {
+      await userEvent.click(within(dialog).getByRole("button", { name: "Delete", exact: true }));
+      expect(within(dialog).getByRole("button", { name: "Deleting..." })).toBeDisabled();
+      expect(within(dialog).getByRole("button", { name: "Cancel" })).toBeDisabled();
+      expect(networking.deleteMCPServer).toHaveBeenCalledWith("123", "delete-server");
+      await act(async () => finishDelete());
+    }
+    await waitFor(() => expect(screen.queryByRole("alertdialog")).not.toBeInTheDocument());
+  });
+
+  it("filters servers by access group", async () => {
+    const server = { created_by: "user", updated_by: "user" };
+    vi.mocked(networking.fetchMCPServers).mockResolvedValue([
+      {
+        ...server,
+        server_id: "string-group",
+        server_name: "String group",
+        alias: "string-alias",
+        mcp_access_groups: ["shared"],
+      },
+      {
+        ...server,
+        server_id: "legacy-group",
+        server_name: "Legacy group",
+        alias: "legacy-alias",
+        mcp_access_groups: ["shared"],
+      },
+      {
+        ...server,
+        server_id: "other-group",
+        server_name: "Other group",
+        alias: "other-alias",
+        mcp_access_groups: ["different"],
+      },
+    ]);
+    vi.mocked(networking.fetchMCPServerHealth).mockResolvedValue([]);
+    render(
+      <QueryClientProvider client={createQueryClient()}>
+        <MCPServers {...defaultProps} />
+      </QueryClientProvider>,
+    );
+    await screen.findByText("String group");
+    await userEvent.click(screen.getByRole("combobox", { name: "Access Group" }));
+    await userEvent.click(await screen.findByRole("option", { name: "shared", exact: true }));
+    expect(screen.getByText("String group")).toBeVisible();
+    expect(screen.getByText("Legacy group")).toBeVisible();
+    expect(screen.queryByText("Other group")).not.toBeInTheDocument();
+  });
+
+  it.each(["server_name", "alias", "url", "server_id"] as const)("searches by %s case-insensitively", async (field) => {
+    const server: MCPServer = {
+      server_id: "search-server",
+      server_name: "Search fixture",
+      created_by: "user",
+      updated_by: "user",
+      [field]: "Needle",
+    };
+    vi.mocked(networking.fetchMCPServers).mockResolvedValue([server]);
+    vi.mocked(networking.fetchMCPServerHealth).mockResolvedValue([]);
+    render(
+      <QueryClientProvider client={createQueryClient()}>
+        <MCPServers {...defaultProps} />
+      </QueryClientProvider>,
+    );
+    await screen.findByTestId("mcp-servers-grid");
+    const search = screen.getByPlaceholderText("Search by name, alias, URL, or ID");
+    await userEvent.type(search, "  NEEDLE  ");
+    expect(screen.getByTestId("mcp-servers-grid")).toBeVisible();
+    await userEvent.clear(search);
+    await userEvent.type(search, "no-match");
+    expect(screen.queryByTestId("mcp-servers-grid")).not.toBeInTheDocument();
+    expect(screen.getByText("No servers match the current filters or search.")).toBeVisible();
   });
 
   it("should render mocked MCP servers data in the table", async () => {
@@ -409,9 +533,7 @@ describe("MCPServers", () => {
     expect(screen.getByText("Team B Server")).toBeInTheDocument();
     expect(screen.getByText("Team A Server 2")).toBeInTheDocument();
 
-    // Find the team select by its "Team" label, then the combobox it labels
-    const teamLabel = screen.getByText("Team");
-    const teamSelect = within(teamLabel.parentElement!).getByRole("combobox");
+    const teamSelect = screen.getByRole("combobox", { name: "Team" });
 
     await userEvent.click(teamSelect);
 
diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_servers.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_servers.tsx
index 4a5ab379969..818b5150650 100644
--- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_servers.tsx
+++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_servers.tsx
@@ -113,6 +113,62 @@ const readToolsOAuthServerId = (): string | null => {
   }
 };
 
+function DeleteServerDialog({
+  open,
+  onOpenChange,
+  server,
+  isDeleting,
+  onConfirm,
+}: {
+  open: boolean;
+  onOpenChange: (open: boolean) => void;
+  server: MCPServer | undefined;
+  isDeleting: boolean;
+  onConfirm: () => Promise<void>;
+}) {
+  return (
+    <AlertDialog open={open} onOpenChange={onOpenChange}>
+      <AlertDialogContent>
+        <AlertDialogHeader>
+          <AlertDialogTitle>Delete MCP Server?</AlertDialogTitle>
+        </AlertDialogHeader>
+        <div className="space-y-4">
+          <p className="text-sm text-muted-foreground">
+            This action is permanent and cannot be undone. All associated configurations will be removed.
+          </p>
+
+          {server && (
+            <dl className="mt-3 space-y-1 rounded-lg border border-border bg-muted p-4">
+              {server.server_name && (
+                <div className="flex gap-2">
+                  <dt className="text-sm text-muted-foreground">Name</dt>
+                  <dd className="text-sm font-semibold">{server.server_name}</dd>
+                </div>
+              )}
+              <div className="flex gap-2">
+                <dt className="text-sm text-muted-foreground">ID</dt>
+                <dd className="font-mono text-xs">{server.server_id}</dd>
+              </div>
+              {server.url && (
+                <div className="flex gap-2">
+                  <dt className="text-sm text-muted-foreground">URL</dt>
+                  <dd className="font-mono text-xs break-all">{server.url}</dd>
+                </div>
+              )}
+            </dl>
+          )}
+        </div>
+        <AlertDialogFooter>
+          <AlertDialogCancel disabled={isDeleting}>Cancel</AlertDialogCancel>
+          <Button variant="destructive" disabled={isDeleting} onClick={onConfirm}>
+            {isDeleting ? "Deleting..." : "Delete"}
+          </Button>
+        </AlertDialogFooter>
+      </AlertDialogContent>
+    </AlertDialog>
+  );
+}
+
 const MCPServers: React.FC<MCPServerProps> = ({ accessToken, userRole, userID, isViewOnly = false }) => {
   const { data: mcpServers, isLoading: isLoadingServers, refetch } = useMCPServers();
 
@@ -299,7 +355,9 @@ const MCPServers: React.FC<MCPServerProps> = ({ accessToken, userRole, userID, i
       }
       if (group !== "all") {
         filtered = filtered.filter((server) =>
-          server.mcp_access_groups?.some((g: any) => (typeof g === "string" ? g === group : g && g.name === group)),
+          server.mcp_access_groups?.some((g: string | { name?: string } | null) =>
+            typeof g === "string" ? g === group : g?.name === group,
+          ),
         );
       }
       setFilteredServers(filtered);
@@ -333,7 +391,7 @@ const MCPServers: React.FC<MCPServerProps> = ({ accessToken, userRole, userID, i
           const alias = (s.alias || "").toLowerCase();
           const url = (s.url || "").toLowerCase();
           const id = s.server_id.toLowerCase();
-          return name.includes(q) || alias.includes(q) || url.includes(q) || id.includes(q);
+          return [name, alias, url, id].some((value) => value.includes(q));
         })
       : filteredServers;
     return [...matches].sort((a, b) => compareServers(a, b, sortKey));
@@ -376,9 +434,7 @@ const MCPServers: React.FC<MCPServerProps> = ({ accessToken, userRole, userID, i
   };
 
   // Find the server to delete from the servers list
-  const serverToDelete = serverIdToDelete
-    ? (mcpServers || []).find((server) => server.server_id === serverIdToDelete)
-    : null;
+  const serverToDelete = mcpServers?.find((server) => server.server_id === serverIdToDelete);
 
   const handleCreateSuccess = (newMcpServer: MCPServer) => {
     setFilteredServers((prev) => [...prev, newMcpServer]);
@@ -420,45 +476,13 @@ const MCPServers: React.FC<MCPServerProps> = ({ accessToken, userRole, userID, i
   return (
     <TooltipProvider>
       <div className="h-full w-full p-6">
-        <AlertDialog open={isDeleteModalOpen} onOpenChange={(open) => !open && cancelDelete()}>
-          <AlertDialogContent>
-            <AlertDialogHeader>
-              <AlertDialogTitle>Delete MCP Server?</AlertDialogTitle>
-            </AlertDialogHeader>
-            <div className="space-y-4">
-              <p className="text-sm text-muted-foreground">
-                This action is permanent and cannot be undone. All associated configurations will be removed.
-              </p>
-
-              {serverToDelete && (
-                <dl className="mt-3 space-y-1 rounded-lg border border-border bg-muted p-4">
-                  {serverToDelete.server_name && (
-                    <div className="flex gap-2">
-                      <dt className="text-sm text-muted-foreground">Name</dt>
-                      <dd className="text-sm font-semibold">{serverToDelete.server_name}</dd>
-                    </div>
-                  )}
-                  <div className="flex gap-2">
-                    <dt className="text-sm text-muted-foreground">ID</dt>
-                    <dd className="font-mono text-xs">{serverToDelete.server_id}</dd>
-                  </div>
-                  {serverToDelete.url && (
-                    <div className="flex gap-2">
-                      <dt className="text-sm text-muted-foreground">URL</dt>
-                      <dd className="font-mono text-xs break-all">{serverToDelete.url}</dd>
-                    </div>
-                  )}
-                </dl>
-              )}
-            </div>
-            <AlertDialogFooter>
-              <AlertDialogCancel disabled={isDeletingServer}>Cancel</AlertDialogCancel>
-              <Button variant="destructive" disabled={isDeletingServer} onClick={confirmDelete}>
-                {isDeletingServer ? "Deleting..." : "Delete"}
-              </Button>
-            </AlertDialogFooter>
-          </AlertDialogContent>
-        </AlertDialog>
+        <DeleteServerDialog
+          open={isDeleteModalOpen}
+          onOpenChange={(open) => !open && cancelDelete()}
+          server={serverToDelete}
+          isDeleting={isDeletingServer}
+          onConfirm={confirmDelete}
+        />
         <CreateMCPServer
           userRole={userRole}
           userID={userID}
@@ -487,7 +511,7 @@ const MCPServers: React.FC<MCPServerProps> = ({ accessToken, userRole, userID, i
               <Plug />
               My Connections
             </Link>
-            {isAdminRole(userRole) && (
+            {isAdminRole(userRole) ? (
               <>
                 <Button className="shrink-0" variant="secondary" onClick={() => setImportVisible(true)}>
                   Import from JSON
@@ -496,8 +520,7 @@ const MCPServers: React.FC<MCPServerProps> = ({ accessToken, userRole, userID, i
                   + Add New MCP Server
                 </Button>
               </>
-            )}
-            {!isAdminRole(userRole) && (
+            ) : (
               <Button
                 className="shrink-0"
                 onClick={() => {
@@ -544,24 +567,23 @@ const MCPServers: React.FC<MCPServerProps> = ({ accessToken, userRole, userID, i
               Connect
             </TabsTrigger>
             {isAdminRole(userRole) && (
-              <TabsTrigger value="semantic-filter" className="flex-none rounded-none px-4 py-2">
-                Semantic Filter
-              </TabsTrigger>
-            )}
-            {isAdminRole(userRole) && (
-              <TabsTrigger value="tool-search" className="flex-none rounded-none px-4 py-2">
-                Tool Search
-              </TabsTrigger>
-            )}
-            {isAdminRole(userRole) && (
-              <TabsTrigger value="network-settings" className="flex-none rounded-none px-4 py-2">
-                Network Settings
-              </TabsTrigger>
-            )}
-            {isAdminRole(userRole) && (
-              <TabsTrigger value="submitted" className="flex-none rounded-none px-4 py-2">
-                Submitted MCPs
-              </TabsTrigger>
+              <>
+                <TabsTrigger value="semantic-filter" className="flex-none rounded-none px-4 py-2">
+                  Semantic Filter
+                </TabsTrigger>
+
+                <TabsTrigger value="tool-search" className="flex-none rounded-none px-4 py-2">
+                  Tool Search
+                </TabsTrigger>
+
+                <TabsTrigger value="network-settings" className="flex-none rounded-none px-4 py-2">
+                  Network Settings
+                </TabsTrigger>
+
+                <TabsTrigger value="submitted" className="flex-none rounded-none px-4 py-2">
+                  Submitted MCPs
+                </TabsTrigger>
+              </>
             )}
             {isProxyAdminTierRole(userRole) && (
               <TabsTrigger value="connections" className="flex-none rounded-none px-4 py-2">
@@ -596,13 +618,11 @@ const MCPServers: React.FC<MCPServerProps> = ({ accessToken, userRole, userID, i
                           value={selectedTeam}
                           onValueChange={(v: string | null) => handleTeamChange(v ?? "all")}
                         >
-                          <SelectTrigger className="w-55">
+                          <SelectTrigger className="w-55" aria-label="Team">
                             <SelectValue />
                           </SelectTrigger>
                           <SelectContent>
-                            <SelectItem value="all">
-                              {isInternalUser ? "All Available Servers" : "All Servers"}
-                            </SelectItem>
+                            <SelectItem value="all">{teamSelectItems.all}</SelectItem>
                             <SelectItem value="personal">Personal</SelectItem>
                             {uniqueTeams.map((team) => (
                               <SelectItem key={team.team_id} value={team.team_id}>
@@ -636,7 +656,7 @@ const MCPServers: React.FC<MCPServerProps> = ({ accessToken, userRole, userID, i
                           value={selectedMcpAccessGroup}
                           onValueChange={(v: string | null) => handleMcpAccessGroupChange(v ?? "all")}
                         >
-                          <SelectTrigger className="w-55">
+                          <SelectTrigger className="w-55" aria-label="Access Group">
                             <SelectValue />
                           </SelectTrigger>
                           <SelectContent>
@@ -737,24 +757,23 @@ const MCPServers: React.FC<MCPServerProps> = ({ accessToken, userRole, userID, i
             <MCPConnect />
           </TabsContent>
           {isAdminRole(userRole) && (
-            <TabsContent value="semantic-filter" keepMounted>
-              <MCPSemanticFilterSettings accessToken={accessToken} />
-            </TabsContent>
-          )}
-          {isAdminRole(userRole) && (
-            <TabsContent value="tool-search" keepMounted>
-              <MCPToolSearchSettings accessToken={accessToken} />
-            </TabsContent>
-          )}
-          {isAdminRole(userRole) && (
-            <TabsContent value="network-settings" keepMounted>
-              <MCPNetworkSettings accessToken={accessToken} />
-            </TabsContent>
-          )}
-          {isAdminRole(userRole) && (
-            <TabsContent value="submitted" keepMounted>
-              <MCPSubmissionsTab accessToken={accessToken} />
-            </TabsContent>
+            <>
+              <TabsContent value="semantic-filter" keepMounted>
+                <MCPSemanticFilterSettings accessToken={accessToken} />
+              </TabsContent>
+
+              <TabsContent value="tool-search" keepMounted>
+                <MCPToolSearchSettings accessToken={accessToken} />
+              </TabsContent>
+
+              <TabsContent value="network-settings" keepMounted>
+                <MCPNetworkSettings accessToken={accessToken} />
+              </TabsContent>
+
+              <TabsContent value="submitted" keepMounted>
+                <MCPSubmissionsTab accessToken={accessToken} />
+              </TabsContent>
+            </>
           )}
           {isProxyAdminTierRole(userRole) && (
             <TabsContent value="connections">