fix(mcp): keep a null tools list a no-op on toolset update

Treating a null tools list as a clear meant an existing client that sends
tools=null during a partial update, meaning "leave the selection alone",
silently lost every tool the toolset grants. That is a permission surface,
so the quiet version of it is the worst version.

A toolset always has a tool list, the same way it always has a name, so a
null on either is now a no-op. Emptying the selection is an explicit [],
which cannot be confused with a field the caller left out, and which is
what the dashboard already sends.

litellm/proxy/_experimental/mcp_server/toolset_db.py

@@ -132,13 +132,17 @@ async def update_mcp_toolset(
     data: UpdateMCPToolsetRequest,
     touched_by: str,
 ) -> MCPToolset | None:
-    """A partial update: absent keeps, null clears, except that a toolset always has a
-    name, so a null ``toolset_name`` is ignored."""
+    """A partial update: absent keeps, null clears. A toolset always has a name and a
+    tool list, so a null ``toolset_name`` or ``tools`` is a no-op rather than a clear;
+    emptying the tool selection is an explicit ``[]``, which cannot be mistaken for a
+    caller that left the field out."""
     data_dict: Final = data.model_dump(exclude_unset=True, exclude={"toolset_id"})
-    if "tools" in data_dict:
-        data_dict["tools"] = json.dumps(data_dict["tools"] or ())
     if data_dict.get("toolset_name", "") is None:
         _ = data_dict.pop("toolset_name")
+    if data_dict.get("tools", "") is None:
+        _ = data_dict.pop("tools")
+    if "tools" in data_dict:
+        data_dict["tools"] = json.dumps(data_dict["tools"])
     data_dict["updated_by"] = touched_by
     try:
         row: Final = await _toolset_table(prisma_client).update(

litellm/proxy/management_endpoints/mcp_management_endpoints.py

@@ -3100,7 +3100,8 @@ if MCP_AVAILABLE:
         user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth),
         litellm_changed_by: str | None = Header(None),
     ):
-        """Partial update: a field left out of the payload keeps its stored value, and a field sent as null is cleared."""
+        """Partial update: a field left out keeps its stored value, and a field sent as null is cleared, except
+        ``toolset_name`` and ``tools``, which a toolset always has; empty the tool selection with an explicit []."""
         prisma_client: Final = get_prisma_client_or_throw("Database not connected. Connect a database to your proxy")
         if LitellmUserRoles.PROXY_ADMIN != user_api_key_dict.user_role:
             raise HTTPException(

tests/e2e/proxy_client.py

@@ -540,10 +540,14 @@ class ProxyClient:
 
     def replicas_for(self, path: str) -> Mapping[str, Transport]:
         """The replicas that serve `path`: every data-plane replica for an LLM route,
-        and for a management route the control-plane replicas, which in a split
-        deployment is the one service that serves it (the data-plane replicas trim
-        management routes) and in a monolith is every replica. Never empty: a
-        read-back against no replica would assert nothing and pass."""
+        and for a management route the control-plane replicas, since the data-plane
+        replicas trim management routes and answer them 404. A monolith serves both
+        from every replica, so a management read-back polls all of them; a split
+        deployment exposes one control-plane address (there is one backend process
+        behind it on the stack these suites run against), so it polls that. A
+        control plane fronting several backends would need its own replica list to
+        prove each one converged, the way PROXY_REPLICA_URLS does for the gateways.
+        Never empty: a read-back against no replica would assert nothing and pass."""
         replicas: Final = self.control_replicas if is_control_plane_path(path) else self.replicas
         assert replicas, f"no replica is configured to serve {path}, so a read-back there would prove nothing"
         return replicas

tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_partial_update.py

@@ -893,10 +893,18 @@ async def test_toolset_partial_update_omits_the_fields_the_caller_left_out():
 
 
 @pytest.mark.asyncio
-async def test_toolset_partial_update_writes_null_tools_as_an_empty_list():
-    """Prisma ``tools`` is a required Json column defaulting to [], so a null clears
-    the selection to none rather than writing SQL null."""
-    assert await _run_toolset_update({"toolset_id": "ts-1", "tools": None}) == {"tools": "[]"}
+async def test_toolset_partial_update_ignores_null_tools_rather_than_revoking_them():
+    """A client that sends tools=null means "leave the selection alone", so the grants
+    survive. Clearing them is an explicit [], which cannot be confused with an omitted
+    field; treating null as a clear would silently revoke every tool the toolset grants."""
+    assert await _run_toolset_update({"toolset_id": "ts-1", "tools": None, "description": "kept"}) == {
+        "description": "kept"
+    }
+
+
+@pytest.mark.asyncio
+async def test_toolset_partial_update_empties_the_selection_on_an_explicit_empty_list():
+    assert await _run_toolset_update({"toolset_id": "ts-1", "tools": []}) == {"tools": "[]"}
 
 
 @pytest.mark.asyncio