From 409c5f795fd3bcfa98591a64c86c909e62f89f08 Mon Sep 17 00:00:00 2001
From: Ishaan Jaffer <ishaanjaffer0324@gmail.com>
Date: Wed, 11 Mar 2026 18:39:51 -0700
Subject: [PATCH] fix(mcp-crud-ui): address greptile 3/5 review

- remove non-functional XIcon remove-server button (no onRemoveServer prop wired)
- fix stale closure in MCPAppsPanel auto-enable effect: use serversRef/selectedServersRef
- remove utility re-export from McpCrudPermissionPanel (classifyToolOp, groupToolsByCrud)
- remove redundant selectedTools.length === 0 guard (always true when !toolPermissions[id])
---
 .../src/components/chat/MCPAppsPanel.tsx      | 20 ++++++++++++++-----
 .../MCPToolPermissions.tsx                    | 12 +----------
 .../mcp_tools/McpCrudPermissionPanel.tsx      |  2 --
 3 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/ui/litellm-dashboard/src/components/chat/MCPAppsPanel.tsx b/ui/litellm-dashboard/src/components/chat/MCPAppsPanel.tsx
index eaae0212a9e..2afe032e0db 100644
--- a/ui/litellm-dashboard/src/components/chat/MCPAppsPanel.tsx
+++ b/ui/litellm-dashboard/src/components/chat/MCPAppsPanel.tsx
@@ -1,6 +1,6 @@
 "use client";
 
-import React, { useCallback, useEffect, useState } from "react";
+import React, { useCallback, useEffect, useRef, useState } from "react";
 import { Spin, Input, Button, Skeleton } from "antd";
 import { SearchOutlined, ArrowLeftOutlined, RightOutlined, ToolOutlined, CheckCircleOutlined } from "@ant-design/icons";
 import { deleteMCPOAuthUserCredential, fetchMCPServers, getMCPOAuthUserCredentialStatus, listMCPTools } from "../networking";
@@ -99,6 +99,14 @@ const MCPAppsPanel: React.FC<Props> = ({ accessToken, selectedServers, onChange
   // OAuth2 connect state — tracks which server_ids have a stored user credential
   const [oauthConnected, setOauthConnected] = useState<Set<string>>(new Set());
 
+  // Refs keep the latest values for the auto-enable effect so it always reads
+  // the current servers/selectedServers without needing them as dependencies
+  // (which would cause the effect to fire on every render).
+  const serversRef = useRef<MCPServer[]>([]);
+  useEffect(() => { serversRef.current = servers; }, [servers]);
+  const selectedServersRef = useRef<string[]>(selectedServers);
+  useEffect(() => { selectedServersRef.current = selectedServers; }, [selectedServers]);
+
   const nameOf = (s: MCPServer) => s.server_name ?? s.alias ?? s.server_id;
 
   useEffect(() => {
@@ -157,15 +165,17 @@ const MCPAppsPanel: React.FC<Props> = ({ accessToken, selectedServers, onChange
 
   // Auto-enable oauth2 servers for the current chat session when a valid
   // credential is detected (either on mount or after a fresh OAuth sign-in).
+  // Uses refs for servers/selectedServers to avoid stale closures without
+  // adding them as deps (which would cause the effect to re-fire on every render).
   useEffect(() => {
     if (oauthConnected.size === 0) return;
-    const namesToAdd = servers
-      .filter((s) => oauthConnected.has(s.server_id) && !selectedServers.includes(nameOf(s)))
+    const namesToAdd = serversRef.current
+      .filter((s) => oauthConnected.has(s.server_id) && !selectedServersRef.current.includes(nameOf(s)))
       .map(nameOf);
     if (namesToAdd.length > 0) {
-      onChange([...selectedServers, ...namesToAdd]);
+      onChange([...selectedServersRef.current, ...namesToAdd]);
     }
-  }, [oauthConnected]); // eslint-disable-line react-hooks/exhaustive-deps
+  }, [oauthConnected, onChange]);
 
   const handleToggle = async (serverName: string, checked: boolean, serverId?: string) => {
     if (!checked) {
diff --git a/ui/litellm-dashboard/src/components/mcp_server_management/MCPToolPermissions.tsx b/ui/litellm-dashboard/src/components/mcp_server_management/MCPToolPermissions.tsx
index 582db111bf1..df3e5958c14 100644
--- a/ui/litellm-dashboard/src/components/mcp_server_management/MCPToolPermissions.tsx
+++ b/ui/litellm-dashboard/src/components/mcp_server_management/MCPToolPermissions.tsx
@@ -3,7 +3,6 @@ import { listMCPTools } from "../networking";
 import { MCPTool, MCPServer } from "../mcp_tools/types";
 import { Text } from "@tremor/react";
 import { Spin, Radio } from "antd";
-import { XIcon } from "lucide-react";
 import { useMCPServers } from "../../app/(dashboard)/hooks/mcpServers/useMCPServers";
 import McpCrudPermissionPanel from "../mcp_tools/McpCrudPermissionPanel";
 import { classifyToolOp } from "../../utils/mcpToolCrudClassification";
@@ -157,15 +156,6 @@ const MCPToolPermissions: React.FC<MCPToolPermissionsProps> = ({
                     </button>
                   </>
                 )}
-                <button
-                  type="button"
-                  className="text-gray-400 hover:text-gray-600"
-                  onClick={() => {
-                    // Handle remove server if needed
-                  }}
-                >
-                  <XIcon className="w-4 h-4" />
-                </button>
               </div>
             </div>
 
@@ -191,7 +181,7 @@ const MCPToolPermissions: React.FC<MCPToolPermissionsProps> = ({
               {!isLoading && !error && tools.length > 0 && viewMode === "crud" && (
                 <McpCrudPermissionPanel
                   tools={tools}
-                  value={selectedTools.length === 0 && !toolPermissions[server.server_id] ? undefined : selectedTools}
+                  value={!toolPermissions[server.server_id] ? undefined : selectedTools}
                   onChange={(allowed) => handleCrudPanelChange(server.server_id, allowed)}
                   readOnly={disabled}
                 />
diff --git a/ui/litellm-dashboard/src/components/mcp_tools/McpCrudPermissionPanel.tsx b/ui/litellm-dashboard/src/components/mcp_tools/McpCrudPermissionPanel.tsx
index 1f1f3a499e1..bfda8277203 100644
--- a/ui/litellm-dashboard/src/components/mcp_tools/McpCrudPermissionPanel.tsx
+++ b/ui/litellm-dashboard/src/components/mcp_tools/McpCrudPermissionPanel.tsx
@@ -244,5 +244,3 @@ const McpCrudPermissionPanel: React.FC<McpCrudPermissionPanelProps> = ({
 };
 
 export default McpCrudPermissionPanel;
-
-export { classifyToolOp, groupToolsByCrud };