You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ju...@apache.org on 2026/08/03 22:29:44 UTC
(superset) 02/02: fix(mcp): missing params in MCP tool call logs (#42660)
This is an automated email from the ASF dual-hosted git repository.
justinpark pushed a commit to branch 6.2
in repository https://gitbox.apache.org/repos/asf/superset.git
commit d4c7513898d47dc5963668bcfa18497e754c057d
Author: JUST.in DO IT <ju...@airbnb.com>
AuthorDate: Mon Aug 3 08:38:22 2026 -0700
fix(mcp): missing params in MCP tool call logs (#42660)
(cherry picked from commit ac97484b2a7b22531c2494242c1e42f9969ffafc)
---
superset/mcp_service/middleware.py | 4 +--
tests/unit_tests/mcp_service/test_middleware.py | 26 +++++++++----------
.../mcp_service/test_middleware_logging.py | 29 +++++++++++++++++++++-
3 files changed, 43 insertions(+), 16 deletions(-)
diff --git a/superset/mcp_service/middleware.py b/superset/mcp_service/middleware.py
index 2b29a89e474..105b0afff4e 100644
--- a/superset/mcp_service/middleware.py
+++ b/superset/mcp_service/middleware.py
@@ -153,7 +153,7 @@ class LoggingMiddleware(Middleware):
dashboard_id = None
slice_id = None
dataset_id = None
- params = getattr(context.message, "params", {}) or {}
+ params = getattr(context.message, "arguments", {}) or {}
if hasattr(context, "metadata") and context.metadata:
agent_id = context.metadata.get("agent_id")
if not agent_id and hasattr(context, "session") and context.session:
@@ -1204,7 +1204,7 @@ class ResponseSizeGuardMiddleware(Middleware):
# Block if over limit
if estimated_tokens > self.token_limit:
- params = getattr(context.message, "params", {}) or {}
+ params = getattr(context.message, "arguments", {}) or {}
# For info tools, try dynamic truncation before blocking
from superset.mcp_service.utils.token_utils import INFO_TOOLS
diff --git a/tests/unit_tests/mcp_service/test_middleware.py b/tests/unit_tests/mcp_service/test_middleware.py
index bcc164e048f..7311aaa3a94 100644
--- a/tests/unit_tests/mcp_service/test_middleware.py
+++ b/tests/unit_tests/mcp_service/test_middleware.py
@@ -68,7 +68,7 @@ class TestResponseSizeGuardMiddleware:
# Create mock context
context = MagicMock()
context.message.name = "list_charts"
- context.message.params = {}
+ context.message.arguments = {}
# Create mock call_next that returns small response
small_response = {"charts": [{"id": 1, "name": "test"}]}
@@ -91,7 +91,7 @@ class TestResponseSizeGuardMiddleware:
# Create mock context
context = MagicMock()
context.message.name = "list_charts"
- context.message.params = {"page_size": 100}
+ context.message.arguments = {"page_size": 100}
# Create large response
large_response = {
@@ -121,7 +121,7 @@ class TestResponseSizeGuardMiddleware:
# Create mock context for excluded tool
context = MagicMock()
context.message.name = "health_check"
- context.message.params = {}
+ context.message.arguments = {}
# Create response that would exceed limit
large_response = {"data": "x" * 10000}
@@ -140,7 +140,7 @@ class TestResponseSizeGuardMiddleware:
context = MagicMock()
context.message.name = "list_charts"
- context.message.params = {}
+ context.message.arguments = {}
# Response at ~85% of limit (should trigger warning but not block)
response = {"data": "x" * 2900} # ~828 tokens at 3.5 chars/token
@@ -165,7 +165,7 @@ class TestResponseSizeGuardMiddleware:
context = MagicMock()
context.message.name = "list_charts"
- context.message.params = {"page_size": 100}
+ context.message.arguments = {"page_size": 100}
large_response = {"charts": [{"id": i} for i in range(1000)]}
call_next = AsyncMock(return_value=large_response)
@@ -190,7 +190,7 @@ class TestResponseSizeGuardMiddleware:
context = MagicMock()
context.message.name = "list_charts"
- context.message.params = {}
+ context.message.arguments = {}
large_response = {"data": "x" * 10000}
call_next = AsyncMock(return_value=large_response)
@@ -214,7 +214,7 @@ class TestResponseSizeGuardMiddleware:
context = MagicMock()
context.message.name = "get_dataset_info"
- context.message.params = {}
+ context.message.arguments = {}
# Large info tool response with a big description
large_response = {
@@ -243,7 +243,7 @@ class TestResponseSizeGuardMiddleware:
context = MagicMock()
context.message.name = "get_chart_info"
- context.message.params = {}
+ context.message.arguments = {}
large_response = {
"id": 1,
@@ -269,7 +269,7 @@ class TestResponseSizeGuardMiddleware:
context = MagicMock()
context.message.name = "list_charts" # Not an info tool
- context.message.params = {}
+ context.message.arguments = {}
large_response = {"data": "x" * 10000}
call_next = AsyncMock(return_value=large_response)
@@ -288,7 +288,7 @@ class TestResponseSizeGuardMiddleware:
context = MagicMock()
context.message.name = "get_dashboard_info"
- context.message.params = {}
+ context.message.arguments = {}
large_response = {
"id": 1,
@@ -394,7 +394,7 @@ class TestMiddlewareIntegration:
context = MagicMock()
context.message.name = "get_chart_info"
- context.message.params = {}
+ context.message.arguments = {}
response = ChartInfo(id=1, name="Test Chart")
call_next = AsyncMock(return_value=response)
@@ -414,7 +414,7 @@ class TestMiddlewareIntegration:
context = MagicMock()
context.message.name = "list_charts"
- context.message.params = {}
+ context.message.arguments = {}
response = [{"id": 1}, {"id": 2}, {"id": 3}]
call_next = AsyncMock(return_value=response)
@@ -434,7 +434,7 @@ class TestMiddlewareIntegration:
context = MagicMock()
context.message.name = "health_check"
- context.message.params = {}
+ context.message.arguments = {}
response = "OK"
call_next = AsyncMock(return_value=response)
diff --git a/tests/unit_tests/mcp_service/test_middleware_logging.py b/tests/unit_tests/mcp_service/test_middleware_logging.py
index fef1b3e816d..1665bffaf52 100644
--- a/tests/unit_tests/mcp_service/test_middleware_logging.py
+++ b/tests/unit_tests/mcp_service/test_middleware_logging.py
@@ -45,7 +45,7 @@ def _make_context(
ctx.method = method
message = MagicMock()
message.name = name
- message.params = params or {}
+ message.arguments = params or {}
ctx.message = message
if metadata is not None:
ctx.metadata = metadata
@@ -332,6 +332,33 @@ class TestExtractContextInfo:
assert slice_id == 66
+ @patch("superset.mcp_service.middleware.get_user_id", return_value=1)
+ def test_extract_reads_arguments_on_real_call_tool_request_params(
+ self, mock_get_user_id
+ ) -> None:
+ """Regression test: the real MCP ``CallToolRequestParams`` object
+ exposes tool arguments as ``.arguments``, not ``.params`` -- a
+ ``MagicMock``-based context would auto-vivify a ``.params``
+ attribute and hide a mismatch. Using the real SDK type here
+ ensures params/dashboard_id/etc. are actually populated instead
+ of silently logging as empty."""
+ middleware = LoggingMiddleware()
+ message = mt.CallToolRequestParams(
+ name="get_dashboard_info",
+ arguments={"dashboard_id": 7},
+ )
+ ctx = MagicMock()
+ ctx.message = message
+ ctx.metadata = None
+ ctx.session = None
+
+ agent_id, user_id, dashboard_id, slice_id, dataset_id, params = (
+ middleware._extract_context_info(ctx)
+ )
+
+ assert params == {"dashboard_id": 7}
+ assert dashboard_id == 7
+
class TestIsErrorResponse:
"""Tests for LoggingMiddleware._is_error_response()."""