refactor(mcp): trim permission comments and tighten tool override wiring

Co-Authored-By: bot_apk <apk@cognition.ai>

litellm/proxy/_experimental/mcp_server/tool_permission_backfill.py

@@ -257,10 +257,6 @@ async def _convert_one_row(
         where={
             "object_permission_id": row.object_permission_id,
             "mcp_permission_version": {"in": [0, None]},
-            # prisma-client-py has no DbNull/JsonNull sentinel for `equals` on a
-            # Json? column, so a stored NULL field is left unguarded rather than
-            # filtered with a wrong null literal; the id and version still bound
-            # the CAS.
             **{field: {"equals": value} for field, value in stored_fields if value is not None},
         },
         data={

ui/litellm-dashboard/eslint-suppressions.json

@@ -2065,9 +2065,6 @@
   "src/components/templates/key_edit_view.tsx": {
     "local/filename-pascal-case": {
       "count": 1
-    },
-    "max-lines": {
-      "count": 1
     }
   },
   "src/components/templates/key_info_view.tsx": {

ui/litellm-dashboard/src/components/mcp_server_management/MCPToolPermissions.test.tsx

@@ -111,7 +111,6 @@ describe("MCPToolPermissions", () => {
       error: false,
     });
 
-    // A closed allowlist makes the server legacy-editable, so Select All writes the list.
     renderWithProviders(
       <MCPToolPermissions
         accessToken={mockAccessToken}

ui/litellm-dashboard/src/components/mcp_server_management/MCPToolPermissions.tsx

@@ -134,15 +134,10 @@ const MCPToolPermissions: React.FC<MCPToolPermissionsProps> = ({
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [servers, accessToken, toolsetsLoading]);
 
-  // Every allowlist write goes through here so an edit is authoritative for the SERVER, not for
-  // one of the equivalent keys that may name it.
   const writeAllowedTools = (entry: EffectiveMcpServer, allowed: string[]) => {
     onChange(applyToolPermissionWrite({ toolPermissions, entry, allowed }));
   };
 
-  // On a convention server there is no snapshot to edit: the checkbox flips this tool's entry in
-  // the server's overrides and nothing else. A delete tool can only sit in `allow`; a non-delete
-  // tool is carved out with `deny`. Anything not on the checkbox is left alone.
   const isDelete = (tool: MCPTool) => classifyToolOp(tool.name, tool.description || "") === "delete";
 
   const writeToolToggle = (entry: EffectiveMcpServer, tool: MCPTool, checked: boolean) => {
@@ -161,7 +156,6 @@ const MCPToolPermissions: React.FC<MCPToolPermissionsProps> = ({
     writeAllowedTools(entry, checked ? [...current, tool.name] : current.filter((name) => name !== tool.name));
   };
 
-  // Writes the override edits that make every editable displayed tool match `checked`.
   const writeConventionBulk = (entry: EffectiveMcpServer, tools: readonly MCPTool[], checked: boolean) => {
     onOverridesChange?.(
       applyToolOverrideWrites({

ui/litellm-dashboard/src/components/mcp_server_management/effectiveMcpServers.ts

@@ -19,8 +19,6 @@ export interface McpToolState {
   readonly locked: boolean;
 }
 
-// The OpenAPI schema types the stored entry's arrays as optional; the forms and write helpers
-// work with concrete arrays, so rows read from the API are defaulted here.
 export const normalizeMcpToolOverrides = (
   raw: Readonly<Record<string, { allow?: readonly string[]; deny?: readonly string[] }>> | null | undefined,
 ): Record<string, McpToolOverrideEntry> =>
@@ -54,9 +52,6 @@ export interface EffectiveMcpServer {
   // What this level actually allows on the server, which is what the backend enforces: the keyed
   // union widened by the toolset grant. `undefined` means nothing restricts the server from here.
   readonly allowedTools: readonly string[] | undefined;
-  // The merged mcp_tool_overrides entry for this server across every key naming it, or `undefined`
-  // when no key holds one. Overrides are per-tool allow/deny exceptions layered on the convention:
-  // on an unrestricted server a `deny` removes a non-delete tool and an `allow` re-arms a delete.
   readonly overrides: McpToolOverrideEntry | undefined;
   readonly source: McpGrantSource;
 }
@@ -108,9 +103,9 @@ export const mcpServersForIdentifier = (allServers: readonly MCPServer[], identi
 // Every key in the map that names this server, id first so an id key stays the one an edit keeps.
 // A key spelled like this server's name still belongs to another server when that string is that
 // server's id, so the catalog decides membership rather than a field-by-field comparison.
-export const mcpToolPermissionKeysFor = (
+export const mcpToolPermissionKeysFor = <T>(
   server: MCPServer,
-  toolPermissions: Readonly<Record<string, unknown>>,
+  toolPermissions: Readonly<Record<string, T>>,
   allServers: readonly MCPServer[],
 ): readonly string[] =>
   [server.server_id, server.server_name, server.alias].filter(
@@ -128,9 +123,9 @@ const mcpKeyNamesOneServerOnly = (allServers: readonly MCPServer[], key: string)
 // The key an edit writes: the first one that names this server and no other, falling back to the
 // server's own id. When the only entry is a key several servers share, that fallback creates an
 // id-keyed entry rather than rewriting the shared one, which would edit the other server too.
-export const mcpToolPermissionKeyFor = (
+export const mcpToolPermissionKeyFor = <T>(
   server: MCPServer,
-  toolPermissions: Readonly<Record<string, unknown>>,
+  toolPermissions: Readonly<Record<string, T>>,
   allServers: readonly MCPServer[],
 ): string =>
   mcpToolPermissionKeysFor(server, toolPermissions, allServers).find((key) =>
@@ -140,7 +135,7 @@ export const mcpToolPermissionKeyFor = (
 // The union the backend enforces across equivalent keys, first-seen order preserved.
 export const mcpAllowedToolsFor = (
   server: MCPServer,
-  toolPermissions: Readonly<Record<string, readonly string[]>>,
+  toolPermissions: Readonly<Record<string, readonly string[] | undefined>>,
   allServers: readonly MCPServer[],
 ): readonly string[] | undefined => {
   const keys = mcpToolPermissionKeysFor(server, toolPermissions, allServers);
@@ -148,8 +143,6 @@ export const mcpAllowedToolsFor = (
   return [...new Set(keys.flatMap((key) => toolPermissions[key] ?? []))];
 };
 
-// The override entry merged across every key naming this server, `undefined` when no key has one.
-// The backend unions allow/deny across equivalent keys the same way it unions allowlists.
 export const mcpToolOverridesFor = (
   server: MCPServer,
   toolOverrides: Readonly<Record<string, McpToolOverrideEntry>>,
@@ -272,10 +265,6 @@ export const resolveEffectiveMcpServers = ({
   );
 };
 
-// The checked/locked state a checkbox shows for one tool, mirroring the backend's
-// level_allowed_tools: a keyed allowlist stays a closed editable list, a toolset grant is
-// checked-but-locked, and an unrestricted (convention) server allows every non-delete tool unless
-// a stored deny says otherwise — an allow only re-arms a delete, and deny always beats allow.
 export const mcpToolState = (
   entry: EffectiveMcpServer,
   toolName: string,
@@ -295,14 +284,9 @@ export const mcpToolState = (
   return { checked: (isDeleteTool ? false : !denied) || (allowed && !denied), locked: false };
 };
 
-// Whether this server is edited through overrides (convention mode) rather than a closed
-// mcp_tool_permissions allowlist. The backend treats the same condition the same way.
 export const isConventionServer = (entry: EffectiveMcpServer): boolean =>
   entry.keyedTools === undefined && entry.toolsetTools === undefined;
 
-// Apply one checkbox change to the server's override entry: a delete tool can only be added to or
-// removed from `allow` (a deny would never let it back on), and a non-delete tool flips its `deny`
-// membership. Every other key in the map, and tools this edit did not touch, is preserved as-is.
 export const applyToolOverrideWrite = ({
   toolOverrides,
   permissionKey,
@@ -324,8 +308,6 @@ export const applyToolOverrideWrite = ({
   return { ...toolOverrides, [permissionKey]: written };
 };
 
-// Fold a list of {name, isDeleteTool} toggles over the same server entry, for Select All /
-// Deselect All applied to the displayed editable tools.
 export const applyToolOverrideWrites = ({
   toolOverrides,
   permissionKey,

ui/litellm-dashboard/src/components/templates/keyEditFormValues.ts

@@ -1,4 +1,5 @@
 import { z } from "zod/v4";
+import type { UseFormReturn } from "react-hook-form";
 
 import { KeyResponse } from "../key_team_helpers/key_list";
 import { extractLoggingSettings, formatMetadataForDisplay, stripTagsFromMetadata } from "../key_info_utils";
@@ -177,6 +178,14 @@ export interface MountedFieldGates {
   canViewPrompts: boolean;
 }
 
+export const mcpToolFieldProps = (form: Pick<UseFormReturn<KeyEditFormValues>, "watch" | "setValue">) => ({
+  toolPermissions: form.watch("mcp_tool_permissions") || {},
+  onChange: (permissions: Record<string, string[]>) => form.setValue("mcp_tool_permissions", permissions),
+  toolOverrides: form.watch("mcp_tool_overrides") || {},
+  onOverridesChange: (overrides: Record<string, { allow: string[]; deny: string[] }>) =>
+    form.setValue("mcp_tool_overrides", overrides),
+});
+
 export const toSubmittedValues = (
   values: KeyEditFormValues,
   { canViewPolicies, canViewPrompts }: MountedFieldGates,

ui/litellm-dashboard/src/components/templates/key_edit_view.tsx

@@ -43,6 +43,7 @@ import {
   KeyEditFormValues,
   keyEditFormSchema,
   McpServersAndGroups,
+  mcpToolFieldProps,
   toKeyEditFormValues,
   toSubmittedValues,
 } from "./keyEditFormValues";
@@ -149,7 +150,6 @@ export function KeyEditView({
   const mcpSelection = form.watch("mcp_servers_and_groups") as
     | { servers?: string[]; accessGroups?: string[]; toolsets?: string[] }
     | undefined;
-  const mcpToolPermissions = form.watch("mcp_tool_permissions");
 
   useEffect(() => {
     const fetchModels = async () => {
@@ -764,10 +764,7 @@ export function KeyEditView({
               selectedServers={mcpSelection?.servers || []}
               selectedAccessGroups={mcpSelection?.accessGroups || []}
               selectedToolsets={mcpSelection?.toolsets || []}
-              toolPermissions={(mcpToolPermissions as Record<string, string[]> | undefined) || {}}
-              onChange={(toolPerms) => form.setValue("mcp_tool_permissions", toolPerms)}
-              toolOverrides={form.watch("mcp_tool_overrides") || {}}
-              onOverridesChange={(overrides) => form.setValue("mcp_tool_overrides", overrides)}
+              {...mcpToolFieldProps(form)}
             />
           </div>
 

ui/litellm-dashboard/src/utils/mcpToolCrudClassification.ts

@@ -98,11 +98,7 @@ const tokenVariants = (token: string): string[] =>
     (variant) => variant.length > 0,
   );
 
-// Read is checked before delete/update/create so that tools like
-// `get_removed_entries` — where the primary verb is a read — are not silently
-// blocked by the delete-by-default policy for new servers. This mirrors
-// litellm/proxy/_experimental/mcp_server/tool_classification.py; the shared
-// fixture under tests/test_litellm pins parity between the two.
+// Matches litellm/proxy/_experimental/mcp_server/tool_classification.py, fixture-pinned.
 const classifyTokens = (tokens: string[]): CrudOp => {
   const variants = new Set(tokens.flatMap((token) => tokenVariants(token)));
   if ([...variants].some((variant) => READ_TOKENS.has(variant))) return "read";
@@ -112,8 +108,6 @@ const classifyTokens = (tokens: string[]): CrudOp => {
   return "unknown";
 };
 
-// The name alone decides first; a misleading description cannot reclassify a
-// tool whose name already carries a recognized verb.
 export function classifyToolOp(name: string, description = ""): CrudOp {
   const byName = classifyTokens(nameTokens(name));
   if (byName !== "unknown") return byName;