fix(mcp): address PR review comments

- Use global mcp instance with add_middleware() instead of creating new instance (fixes tool registration)
- Add error handling for middleware creation with logging.exception
- Use Python 3.10+ lowercase dict type hints instead of Dict
- Fix multi-line docstring format with summary on first line
- Add missing trailing commas for better code style
- Improve middleware creation efficiency with try-except blocks

Author: aminghadersohi

superset/mcp_service/mcp_config.py

@@ -18,7 +18,7 @@
 
 import logging
 import secrets
-from typing import Any, Dict, Optional
+from typing import Any
 
 from flask import Flask
 
@@ -34,7 +34,7 @@
 WEBDRIVER_BASEURL_USER_FRIENDLY = WEBDRIVER_BASEURL
 
 # Feature flags for MCP
-MCP_FEATURE_FLAGS: Dict[str, Any] = {
+MCP_FEATURE_FLAGS: dict[str, Any] = {
     "MCP_SERVICE": True,
 }
 
@@ -106,7 +106,7 @@
 }
 
 
-def create_default_mcp_auth_factory(app: Flask) -> Optional[Any]:
+def create_default_mcp_auth_factory(app: Flask) -> Any | None:
     """Default MCP auth factory using app.config values."""
     if not app.config.get("MCP_AUTH_ENABLED", False):
         return None
@@ -154,7 +154,7 @@ def create_default_mcp_auth_factory(app: Flask) -> Optional[Any]:
         return None
 
 
-def default_user_resolver(app: Any, access_token: Any) -> Optional[str]:
+def default_user_resolver(app: Any, access_token: Any) -> str | None:
     """Extract username from JWT token claims."""
     logger.info(
         "Resolving user from token: type=%s, token=%s",
@@ -179,7 +179,7 @@ def generate_secret_key() -> str:
     return secrets.token_urlsafe(42)
 
 
-def get_mcp_config(app_config: Dict[str, Any] | None = None) -> Dict[str, Any]:
+def get_mcp_config(app_config: dict[str, Any] | None = None) -> dict[str, Any]:
     """
     Get complete MCP configuration dictionary.
 
@@ -207,8 +207,8 @@ def get_mcp_config(app_config: Dict[str, Any] | None = None) -> Dict[str, Any]:
 
 
 def get_mcp_config_with_overrides(
-    app_config: Dict[str, Any] | None = None,
-) -> Dict[str, Any]:
+    app_config: dict[str, Any] | None = None,
+) -> dict[str, Any]:
     """
     Alternative approach: Allow any app_config keys, not just predefined ones.
 
@@ -223,16 +223,16 @@ def get_mcp_config_with_overrides(
 
 
 def create_response_caching_middleware(
-    app_config: Dict[str, Any] | None = None,
-) -> Optional[Any]:
-    """
-    Create ResponseCachingMiddleware with configuration from app.config.
+    app_config: dict[str, Any] | None = None,
+) -> Any | None:
+    """Create ResponseCachingMiddleware with configuration from app.config.
 
     Args:
         app_config: Optional Flask app configuration dict
 
     Returns:
         ResponseCachingMiddleware instance or None if caching is disabled
+
     """
     app_config = app_config or {}
 
@@ -269,7 +269,7 @@ def create_response_caching_middleware(
         }
 
         # Tool caching with include/exclude lists
-        call_tool_settings: Dict[str, Any] = {
+        call_tool_settings: dict[str, Any] = {
             "ttl": cache_config.get("call_tool_ttl", 3600),
             "enabled": True,
         }
@@ -298,7 +298,8 @@ def create_response_caching_middleware(
 
         if cache_config.get("excluded_tools"):
             logger.info(
-                "Excluded tools from caching: %s", cache_config["excluded_tools"]
+                "Excluded tools from caching: %s",
+                cache_config["excluded_tools"],
             )
 
         return middleware
@@ -309,11 +310,11 @@ def create_response_caching_middleware(
         )
         return None
     except Exception as e:
-        logger.error("Failed to create response caching middleware: %s", e)
+        logger.exception("Failed to create response caching middleware: %s", e)
         return None
 
 
-def get_mcp_factory_config() -> Dict[str, Any]:
+def get_mcp_factory_config() -> dict[str, Any]:
     """
     Get FastMCP factory configuration.
 

superset/mcp_service/server.py

@@ -82,20 +82,24 @@ def run_server(
         factory_config = get_mcp_factory_config()
         mcp_instance = create_mcp_app(**factory_config)
     else:
-        # Use default initialization with response caching middleware
-        logging.info("Creating MCP app with default configuration...")
+        # Use default global mcp instance with middleware
+        # This ensures all registered tools are available
+        logging.info("Using global MCP instance with middleware...")
 
-        # Import here to avoid circular imports
+        # Import global mcp instance and middleware config
+        from superset.mcp_service.app import mcp
         from superset.mcp_service.mcp_config import create_response_caching_middleware
 
-        # Create middleware list - response caching is enabled by default
-        middleware_list = []
-        caching_middleware = create_response_caching_middleware()
-        if caching_middleware:
-            middleware_list.append(caching_middleware)
+        # Add response caching middleware if available
+        try:
+            caching_middleware = create_response_caching_middleware()
+            if caching_middleware is not None:
+                mcp.add_middleware(caching_middleware)
+                logging.info("Response caching middleware added successfully")
+        except Exception as e:
+            logging.warning("Failed to add response caching middleware: %s", e)
 
-        # Create MCP instance with middleware
-        mcp_instance = create_mcp_app(middleware=middleware_list or None)
+        mcp_instance = mcp
 
     env_key = f"FASTMCP_RUNNING_{port}"
     if not os.environ.get(env_key):