Merge pull request #41074 from BerriAI/litellm_mcp_server_list_stable_order

fix(mcp): keep server lists stable across refreshes

litellm/proxy/management_endpoints/mcp_management_endpoints.py

@@ -1078,6 +1078,9 @@ if MCP_AVAILABLE:
         return {"servers": registry_servers}
 
     ## FastAPI Routes
+    def _mcp_server_display_order(server: LiteLLM_MCPServerTable) -> tuple[str, str]:
+        return ((server.server_name or server.alias or server.server_id).lower(), server.server_id)
+
     def _get_user_mcp_management_mode() -> UserMCPManagementMode:
         from litellm.proxy.proxy_server import (
             general_settings as proxy_general_settings,
@@ -1228,10 +1231,12 @@ if MCP_AVAILABLE:
                         detail="You do not have permission to view MCP servers for this team.",
                     )
 
-            redacted_mcp_servers = await _get_team_scoped_mcp_server_list(sanitized_team_id)
+            redacted_mcp_servers = sorted(
+                await _get_team_scoped_mcp_server_list(sanitized_team_id), key=_mcp_server_display_order
+            )
         else:
             servers: Final = await _resolve_accessible_mcp_servers(user_api_key_dict)
-            redacted_mcp_servers = _redact_mcp_credentials_list(servers)
+            redacted_mcp_servers = sorted(_redact_mcp_credentials_list(servers), key=_mcp_server_display_order)
 
         if connected_app_view is True and is_ui_session_credential(user_api_key_dict):
             reachable_ids: Final = await _connected_app_reachable_server_ids(user_api_key_dict)

tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py

@@ -6,7 +6,7 @@ import logging
 from contextlib import ExitStack
 from datetime import datetime, timedelta
 from types import SimpleNamespace
-from typing import List, Optional, cast
+from typing import Final, List, Optional, cast
 from unittest.mock import AsyncMock, MagicMock, patch
 
 import pytest
@@ -1614,6 +1614,60 @@ class TestTeamScopedMCPServerAccess:
             result = await fetch_all_mcp_servers(user_api_key_dict=mock_user_auth, team_id="any-team-id")
             assert len(result) == 1
 
+
+class TestFetchAllMCPServersOrdering:
+    def test_display_order_is_case_insensitive_name_then_id(self) -> None:
+        servers: Final = (
+            LiteLLM_MCPServerTable(server_id="s-2", server_name="GitHub", alias="aaa", transport=MCPTransport.http),
+            LiteLLM_MCPServerTable(server_id="s-1", alias="github", transport=MCPTransport.http),
+            LiteLLM_MCPServerTable(server_id="s-0", server_name="Slack", alias="zzz", transport=MCPTransport.http),
+            LiteLLM_MCPServerTable(server_id="confluence", server_name="", alias="", transport=MCPTransport.http),
+        )
+
+        ordered: Final = sorted(servers, key=mgmt_endpoints._mcp_server_display_order)
+        assert [s.server_id for s in ordered] == ["confluence", "s-1", "s-2", "s-0"]
+
+    @pytest.mark.parametrize("team_id", [None, "team-1"])
+    @pytest.mark.parametrize("reverse", [False, True])
+    @pytest.mark.asyncio
+    async def test_list_is_sorted_by_display_name_regardless_of_resolution_order(
+        self, team_id: str | None, reverse: bool
+    ) -> None:
+        mock_user_auth: Final = generate_mock_user_api_key_auth(
+            user_role=LitellmUserRoles.PROXY_ADMIN,
+            user_id="admin_user",
+        )
+        servers: Final = (
+            generate_mock_mcp_server_db_record(server_id="s-zeta", alias="zeta"),
+            generate_mock_mcp_server_db_record(server_id="s-alpha", alias="Alpha"),
+            generate_mock_mcp_server_db_record(server_id="s-mid", alias="mid"),
+        )
+        resolved: Final = list(reversed(servers) if reverse else servers)
+        mock_manager: Final = MagicMock()
+        mock_manager.get_all_allowed_mcp_servers = AsyncMock(return_value=resolved)
+        with (
+            patch(  # test-quality-ok: the route reads a module-global manager with no injection seam
+                "litellm.proxy.management_endpoints.mcp_management_endpoints.global_mcp_server_manager",
+                mock_manager,
+            ),
+            patch(  # test-quality-ok: admin view is derived from module-global proxy settings
+                "litellm.proxy.management_endpoints.mcp_management_endpoints._user_has_admin_view",
+                return_value=True,
+            ),
+            patch(  # test-quality-ok: auth contexts need a live prisma client
+                "litellm.proxy.management_endpoints.mcp_management_endpoints.build_effective_auth_contexts",
+                AsyncMock(return_value=[mock_user_auth]),
+            ),
+            patch(  # test-quality-ok: isolate the route's ordering from team database resolution
+                "litellm.proxy.management_endpoints.mcp_management_endpoints._get_team_scoped_mcp_server_list",
+                AsyncMock(return_value=resolved),
+            ),
+        ):
+            result: Final = await mgmt_endpoints.fetch_all_mcp_servers(
+                user_api_key_dict=mock_user_auth, team_id=team_id
+            )
+            assert [s.server_id for s in result] == ["s-alpha", "s-mid", "s-zeta"]
+
     @pytest.mark.asyncio
     async def test_restricted_virtual_key_cannot_use_team_id_filter(self):
         """Restricted virtual keys must not bypass access limits via team_id."""

ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_servers.test.tsx

@@ -3,7 +3,8 @@ import { render, waitFor, screen, act, within } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 import { describe, it, expect, vi, beforeEach } from "vitest";
 import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
-import MCPServers from "./mcp_servers";
+import MCPServers, { compareServers, type SortKey } from "./mcp_servers";
+import type { MCPServer } from "@/components/mcp_tools/types";
 import * as networking from "@/components/networking";
 
 // Mock the networking module
@@ -31,6 +32,98 @@ const createQueryClient = () =>
     },
   });
 
+describe("compareServers", () => {
+  const server = (server_id: string, name: string, created_at = ""): MCPServer => ({
+    server_id,
+    server_name: name,
+    created_at,
+    updated_at: created_at,
+    created_by: "user",
+    updated_by: "user",
+  });
+
+  const shuffled = [server("c", "github"), server("a", "slack"), server("b", "Jira")];
+
+  it("orders servers without timestamps by name so config.yaml servers render in a stable order", () => {
+    const byCreated = [...shuffled].sort((a, b) => compareServers(a, b, "created_desc")).map((s) => s.server_id);
+    const byUpdated = [...shuffled].sort((a, b) => compareServers(a, b, "updated_desc")).map((s) => s.server_id);
+    const byHealth = [...shuffled].sort((a, b) => compareServers(a, b, "health")).map((s) => s.server_id);
+
+    expect(byCreated).toEqual(["c", "b", "a"]);
+    expect(byUpdated).toEqual(["c", "b", "a"]);
+    expect(byHealth).toEqual(["c", "b", "a"]);
+  });
+
+  it("keeps newest-first when timestamps differ", () => {
+    const newest = server("new", "zzz", "2026-02-01T00:00:00Z");
+    const oldest = server("old", "aaa", "2026-01-01T00:00:00Z");
+    expect([oldest, newest].sort((a, b) => compareServers(a, b, "created_desc")).map((s) => s.server_id)).toEqual([
+      "new",
+      "old",
+    ]);
+  });
+
+  it.each<SortKey>(["created_desc", "updated_desc", "name_asc", "health"])(
+    "breaks equal timestamps and names by ID for %s regardless of input order",
+    (sort) => {
+      const servers = [
+        server("b", "GitHub", "2026-01-01T00:00:00Z"),
+        server("c", "Slack", "2026-01-01T00:00:00Z"),
+        server("a", "github", "2026-01-01T00:00:00Z"),
+      ];
+      for (const input of [servers, [...servers].reverse()]) {
+        expect([...input].sort((a, b) => compareServers(a, b, sort)).map((s) => s.server_id)).toEqual(["a", "b", "c"]);
+      }
+    },
+  );
+
+  it("uses the display name before alias, then falls back to alias and ID", () => {
+    const servers: MCPServer[] = [
+      { ...server("s-slack", "Slack"), alias: "aaa" },
+      { ...server("s-github", ""), server_name: null, alias: "GitHub" },
+      { ...server("confluence", ""), alias: "" },
+    ];
+    for (const input of [servers, [...servers].reverse()]) {
+      expect([...input].sort((a, b) => compareServers(a, b, "name_asc")).map((s) => s.server_id)).toEqual([
+        "confluence",
+        "s-github",
+        "s-slack",
+      ]);
+    }
+  });
+
+  it.each<SortKey>(["created_desc", "updated_desc", "health"])(
+    "keeps timestamped servers before missing timestamps for %s",
+    (sort) => {
+      const servers = [
+        server("config", "aaa"),
+        server("older", "bbb", "2026-01-01T00:00:00Z"),
+        server("newer", "zzz", "2026-02-01T00:00:00Z"),
+      ];
+      for (const input of [servers, [...servers].reverse()]) {
+        expect([...input].sort((a, b) => compareServers(a, b, sort)).map((s) => s.server_id)).toEqual([
+          "newer",
+          "older",
+          "config",
+        ]);
+      }
+    },
+  );
+
+  it("sorts health before recency and display name", () => {
+    const servers: MCPServer[] = [
+      { ...server("healthy", "aaa", "2026-03-01T00:00:00Z"), status: "healthy" },
+      { ...server("unknown", "bbb", "2026-02-01T00:00:00Z"), status: "unknown" },
+      { ...server("unhealthy", "zzz", "2026-01-01T00:00:00Z"), status: "unhealthy" },
+    ];
+    expect(servers.sort((a, b) => compareServers(a, b, "health")).map((s) => s.server_id)).toEqual([
+      "unhealthy",
+      "unknown",
+      "healthy",
+    ]);
+  });
+});
+
 describe("MCPServers", () => {
   const defaultProps = {
     accessToken: "123",
@@ -74,6 +167,134 @@ 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 = {
+      created_at: "",
+      updated_at: "",
+      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" }));
+      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" }));
+    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 = {
+      created_at: "",
+      updated_at: "",
+      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 () => {
@@ -316,9 +537,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);
 

ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_servers.tsx

@@ -49,7 +49,7 @@ import { cn } from "@/lib/cva.config";
 import UserEnvVarsModal from "./UserEnvVarsModal";
 import { listMCPUserEnvVarStatus } from "@/components/networking";
 
-type SortKey = "created_desc" | "updated_desc" | "name_asc" | "health";
+export type SortKey = "created_desc" | "updated_desc" | "name_asc" | "health";
 
 const SORT_OPTIONS: { value: SortKey; label: string }[] = [
   { value: "created_desc", label: "Recently created" },
@@ -64,32 +64,33 @@ const HEALTH_RANK: Record<string, number> = {
   healthy: 2,
 };
 
-const compareServers = (a: MCPServer, b: MCPServer, sort: SortKey): number => {
+const compareByName = (a: MCPServer, b: MCPServer): number => {
+  const nameA = (a.server_name || a.alias || a.server_id).toLowerCase();
+  const nameB = (b.server_name || b.alias || b.server_id).toLowerCase();
+  return nameA.localeCompare(nameB) || a.server_id.localeCompare(b.server_id);
+};
+
+const compareByTimestampDesc = (a: string | null | undefined, b: string | null | undefined): number => {
+  const ta = a ? new Date(a).getTime() : 0;
+  const tb = b ? new Date(b).getTime() : 0;
+  return tb - ta;
+};
+
+export const compareServers = (a: MCPServer, b: MCPServer, sort: SortKey): number => {
   switch (sort) {
-    case "name_asc": {
-      const nameA = (a.server_name || a.alias || a.server_id).toLowerCase();
-      const nameB = (b.server_name || b.alias || b.server_id).toLowerCase();
-      return nameA.localeCompare(nameB);
-    }
-    case "updated_desc": {
-      const ta = a.updated_at ? new Date(a.updated_at).getTime() : 0;
-      const tb = b.updated_at ? new Date(b.updated_at).getTime() : 0;
-      return tb - ta;
-    }
+    case "name_asc":
+      return compareByName(a, b);
+    case "updated_desc":
+      return compareByTimestampDesc(a.updated_at, b.updated_at) || compareByName(a, b);
     case "health": {
       const ra = HEALTH_RANK[a.status ?? "unknown"] ?? 1;
       const rb = HEALTH_RANK[b.status ?? "unknown"] ?? 1;
       if (ra !== rb) return ra - rb;
-      const ta = a.created_at ? new Date(a.created_at).getTime() : 0;
-      const tb = b.created_at ? new Date(b.created_at).getTime() : 0;
-      return tb - ta;
+      return compareByTimestampDesc(a.created_at, b.created_at) || compareByName(a, b);
     }
     case "created_desc":
-    default: {
-      const ta = a.created_at ? new Date(a.created_at).getTime() : 0;
-      const tb = b.created_at ? new Date(b.created_at).getTime() : 0;
-      return tb - ta;
-    }
+    default:
+      return compareByTimestampDesc(a.created_at, b.created_at) || compareByName(a, b);
   }
 };
 
@@ -112,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();
 
@@ -298,16 +355,12 @@ 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,
+          ),
         );
       }
-      const sorted = [...filtered].sort((a, b) => {
-        if (!a.created_at && !b.created_at) return 0;
-        if (!a.created_at) return 1;
-        if (!b.created_at) return -1;
-        return new Date(b.created_at).getTime() - new Date(a.created_at).getTime();
-      });
-      setFilteredServers(sorted);
+      setFilteredServers(filtered);
     },
     [serversWithHealth],
   );
@@ -338,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));
@@ -381,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]);
@@ -425,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}
@@ -492,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
@@ -501,8 +520,7 @@ const MCPServers: React.FC<MCPServerProps> = ({ accessToken, userRole, userID, i
                   + Add New MCP Server
                 </Button>
               </>
-            )}
-            {!isAdminRole(userRole) && (
+            ) : (
               <Button
                 className="shrink-0"
                 onClick={() => {
@@ -549,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">
@@ -601,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}>
@@ -641,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>
@@ -742,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">