You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2026/09/16 09:44:04 UTC
(superset) branch master updated: fix(mcp): advertise typed tool outputs (#43828)
This is an automated email from the ASF dual-hosted git repository.
aminghadersohi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 0f021023f3d fix(mcp): advertise typed tool outputs (#43828)
0f021023f3d is described below
commit 0f021023f3d96005b27feeedde6e49fda6f95449
Author: Amin Ghadersohi <am...@gmail.com>
AuthorDate: Wed Sep 16 19:43:55 2026 +1000
fix(mcp): advertise typed tool outputs (#43828)
---
UPDATING.md | 20 ++
docs/admin_docs/configuration/mcp-server.mdx | 26 ++-
superset/core/mcp/core_mcp_injection.py | 106 ++++++++-
superset/mcp_service/CLAUDE.md | 23 +-
superset/mcp_service/PRODUCTION.md | 2 +-
superset/mcp_service/__main__.py | 21 +-
superset/mcp_service/chart/schemas.py | 23 +-
superset/mcp_service/chart/tool/get_chart_info.py | 22 +-
.../chart/tool/get_chart_type_schema.py | 18 +-
.../mcp_service/chart/tool/update_chart_preview.py | 9 +-
.../dashboard/tool/get_dashboard_info.py | 13 +-
.../mcp_service/dataset/tool/get_dataset_info.py | 21 +-
superset/mcp_service/mcp_config.py | 10 +-
superset/mcp_service/middleware.py | 71 ++++--
superset/mcp_service/server.py | 42 +++-
.../mcp_service/chart/tool/test_get_chart_info.py | 29 ++-
.../chart/tool/test_update_chart_preview.py | 48 +++-
.../dashboard/tool/test_dashboard_tools.py | 206 ++++++++++-------
.../dashboard/tool/test_update_dashboard.py | 8 +-
.../database/tool/test_database_tools.py | 2 +-
.../mcp_service/dataset/tool/test_dataset_tools.py | 26 ++-
.../mcp_service/query/tool/test_query_tools.py | 2 +-
.../saved_query/tool/test_saved_query_tools.py | 2 +-
.../unit_tests/mcp_service/test_main_entrypoint.py | 7 +-
tests/unit_tests/mcp_service/test_mcp_config.py | 16 ++
tests/unit_tests/mcp_service/test_mcp_e2e_smoke.py | 123 ++++++++---
tests/unit_tests/mcp_service/test_mcp_server.py | 106 +++++++--
.../mcp_service/test_mcp_tool_registration.py | 243 ++++++++++++++++++++-
tests/unit_tests/mcp_service/test_middleware.py | 122 +++++++++--
.../mcp_service/test_middleware_logging.py | 16 +-
30 files changed, 1128 insertions(+), 255 deletions(-)
diff --git a/UPDATING.md b/UPDATING.md
index 7229812187d..165b82873cc 100644
--- a/UPDATING.md
+++ b/UPDATING.md
@@ -126,6 +126,26 @@ tags are included in asset export and import.
Set `FEATURE_FLAGS = {"TAGGING_SYSTEM": False}` to restore the previous
behavior. Existing tag rows are left untouched.
+### MCP structured tool outputs are opt-in
+
+Native MCP tools define concrete output schemas, but Superset preserves the
+text-only wire contract by default for compatibility with clients and transport
+bridges that cannot encode structured results. Set the following only after
+validating every MCP client and bridge used by the deployment:
+
+```python
+MCP_STRUCTURED_OUTPUT_ENABLED = True
+```
+
+When enabled, tool discovery includes `outputSchema` and successful tool calls
+include matching `structuredContent` alongside the existing text representation.
+When disabled, the outer compatibility middleware removes both fields as a pair;
+server-side output validation still applies to native tools.
+
+`StructuredContentStripperMiddleware` is deprecated for custom startup paths but
+retains its original stripping behavior. Replace it with
+`ToolResultCompatibilityMiddleware(structured_output_enabled=False)`.
+
### Version-history and activity endpoints are edit-gated
Version-history and activity endpoints (`GET /api/v1/{chart,dashboard,dataset}/<uuid>/versions/…` and `…/activity/`) are now edit-gated: they require object-level editorship (owner/editor/admin) of the entity, matching the UI's edit-gated Version history menu and the restore endpoint's gate. Read-only users who could previously retrieve the full change log (author identities, field-level before/after diffs) via the API now receive 403. Embedded guest-token principals are always refused o [...]
diff --git a/docs/admin_docs/configuration/mcp-server.mdx b/docs/admin_docs/configuration/mcp-server.mdx
index cca761e5d49..d343484e3db 100644
--- a/docs/admin_docs/configuration/mcp-server.mdx
+++ b/docs/admin_docs/configuration/mcp-server.mdx
@@ -561,6 +561,7 @@ All MCP settings go in `superset_config.py`. Defaults are defined in `superset/m
| `MCP_STATELESS_HTTP` | `True` | Streamable-HTTP session mode. `True` gives each request a fresh, ephemeral transport, torn down as soon as that request completes; a client disconnecting mid-tool-call can crash not just its own session but other concurrent sessions on the same worker. `False` keeps the transport alive for the session's lifetime, avoiding that crash, but requires session-affinity routing on `Mcp-Session-Id` for multi-pod deployments (see [Multi-Pod (Kubernetes)](# [...]
| `MCP_RBAC_ENABLED` | `True` | Enforce Superset's role-based access control on MCP tool calls. When `True`, each tool checks that the authenticated user has the required FAB permission before executing. Disable only for testing or trusted-network deployments. |
| `MCP_DISABLED_TOOLS` | `set()` | Set of tool names to remove from the MCP server at startup. Disabled tools are never advertised to AI clients during tool discovery. Useful when a custom extension tool should replace a built-in Superset tool. See [Disabling built-in tools](#disabling-built-in-tools). |
+| `MCP_STRUCTURED_OUTPUT_ENABLED` | `False` | Advertise native tool `outputSchema` metadata and return matching `structuredContent`. The text-only default supports clients and transport bridges that cannot encode structured results. Enable only after validating every MCP client and bridge used by the deployment. |
| `MCP_DISABLED_CHART_PLUGINS` | `frozenset()` | Set of chart type plugin names (e.g. `"handlebars"`) to hide from `generate_chart`. Does not affect `get_chart_type_schema`. See [Disabling chart type plugins](#disabling-chart-type-plugins). |
| `MCP_CHART_PLUGIN_ENABLED_FUNC` | `None` | Callable `(chart_type: str) -> bool` evaluated per registry lookup for dynamic enable/disable decisions. Takes precedence over `MCP_DISABLED_CHART_PLUGINS` when set. See [Disabling chart type plugins](#disabling-chart-type-plugins). |
@@ -769,18 +770,29 @@ This makes MCP activity fully auditable alongside regular Superset activity. The
Every MCP request passes through a middleware stack before reaching the tool function. The default stack (assembled in `build_middleware_list()` in `server.py`) is:
-| Middleware | Purpose | Default |
-| ------------------------------------- | -------------------------------------------------------------------------------------- | ----------------------------------------------------- |
-| `StructuredContentStripperMiddleware` | Strips `structuredContent` from responses for Claude.ai bridge compatibility | Enabled |
-| `LoggingMiddleware` | Logs each tool call with user, parameters, and duration | Enabled |
-| `GlobalErrorHandlerMiddleware` | Catches unhandled exceptions and sanitizes sensitive data before it reaches the client | Enabled |
-| `ResponseSizeGuardMiddleware` | Estimates token count, warns at 80% of limit, blocks at limit | Enabled (configurable via `MCP_RESPONSE_SIZE_CONFIG`) |
-| `ResponseCachingMiddleware` | Caches read-heavy tool responses (in-memory or Redis) | Disabled (enable via `MCP_CACHE_CONFIG`) |
+| Middleware | Purpose | Default |
+| ----------------------------------- | -------------------------------------------------------------------------------------- | ----------------------------------------------------- |
+| `ToolResultCompatibilityMiddleware` | Gates structured results and sanitizes errors that escape the inner error handler | Enabled |
+| `LoggingMiddleware` | Logs each tool call with user, parameters, and duration | Enabled |
+| `GlobalErrorHandlerMiddleware` | Catches unhandled exceptions and sanitizes sensitive data before it reaches the client | Enabled |
+| `ResponseSizeGuardMiddleware` | Estimates token count, warns at 80% of limit, blocks at limit | Enabled (configurable via `MCP_RESPONSE_SIZE_CONFIG`) |
+| `ResponseCachingMiddleware` | Caches read-heavy tool responses (in-memory or Redis) | Disabled (enable via `MCP_CACHE_CONFIG`) |
Additional middleware classes (`RateLimitMiddleware`, `FieldPermissionsMiddleware`, `PrivateToolMiddleware`) are implemented in `superset/mcp_service/middleware.py` but are not added to the default pipeline. They are available for operators who want to layer them in via a custom startup path.
+`StructuredContentStripperMiddleware` is a deprecated compatibility class for
+custom startup paths that retains its original stripping behavior. Replace it with
+`ToolResultCompatibilityMiddleware(structured_output_enabled=False)`.
+
### Tool Result Value Contract
+When `MCP_STRUCTURED_OUTPUT_ENABLED=True`, native tools advertise a concrete
+`outputSchema` during tool discovery and return matching `structuredContent`
+alongside their text representation. Clients can use the schema to understand
+result fields and validate structured tool-call results. The default is `False`:
+the compatibility middleware removes both fields as a pair and preserves the
+text representation for clients and bridges that do not support structured output.
+
Structured tool results preserve Superset domain values exactly. In particular,
string fields are not wrapped in trust delimiters, and text that resembles a
delimiter is returned as literal application data. This lets clients safely use a
diff --git a/superset/core/mcp/core_mcp_injection.py b/superset/core/mcp/core_mcp_injection.py
index 9c5cdb60d7a..d70fdec7f6e 100644
--- a/superset/core/mcp/core_mcp_injection.py
+++ b/superset/core/mcp/core_mcp_injection.py
@@ -22,8 +22,11 @@ This module provides the concrete implementation of MCP abstractions
that replaces the abstract functions in superset-core during initialization.
"""
+import inspect
import logging
-from typing import Any, Callable, Optional, TypeVar
+from typing import Any, Callable, get_type_hints, Optional, TypeVar
+
+from pydantic import TypeAdapter
try:
from mcp.types import ToolAnnotations
@@ -38,6 +41,85 @@ F = TypeVar("F", bound=Callable[..., Any])
logger = logging.getLogger(__name__)
+def _strip_schema_titles(value: Any) -> Any:
+ """Remove generated JSON Schema titles while preserving useful metadata."""
+ if isinstance(value, list):
+ return [_strip_schema_titles(item) for item in value]
+ if not isinstance(value, dict):
+ return value
+ return {
+ key: _strip_schema_titles(item)
+ for key, item in value.items()
+ if not (key == "title" and isinstance(item, str))
+ }
+
+
+def _is_object_schema(schema: dict[str, Any], root_schema: dict[str, Any]) -> bool:
+ """Return whether a schema, including a local reference, is an object."""
+ if schema.get("type") == "object" or "properties" in schema:
+ return True
+
+ reference = schema.get("$ref")
+ if not isinstance(reference, str) or not reference.startswith("#/$defs/"):
+ return False
+ definition = root_schema.get("$defs", {}).get(reference.removeprefix("#/$defs/"))
+ return isinstance(definition, dict) and (
+ definition.get("type") == "object" or "properties" in definition
+ )
+
+
+def _native_tool_output_schema(func: Callable[..., Any]) -> dict[str, Any] | None:
+ """Build an output schema from the callable's validation shape.
+
+ FastMCP normally derives schemas in Pydantic's serialization mode. A model
+ serializer annotated as returning ``dict[str, Any]`` then collapses an
+ otherwise typed model to an unconstrained object. Superset uses such
+ serializers to omit unrequested columns and normalize aliases, but the
+ model's validation schema remains an accurate superset of every field the
+ serializer can emit.
+
+ MCP requires an object at the output-schema root, so non-object return
+ types are wrapped the same way FastMCP wraps them at execution time.
+ """
+ try:
+ return_type = get_type_hints(func, include_extras=True).get("return")
+ if return_type in (None, Any, inspect.Signature.empty):
+ return None
+
+ schema = TypeAdapter(return_type).json_schema(mode="validation")
+ except Exception: # noqa: BLE001
+ logger.debug("Could not derive MCP output schema", exc_info=True)
+ return None
+
+ # A union of object response models is still an object on the wire. Marking
+ # that fact at the root satisfies MCP's object-schema requirement without
+ # introducing an artificial {"result": ...} envelope around every
+ # success/error union.
+ alternatives = schema.get("anyOf")
+ if (
+ isinstance(alternatives, list)
+ and alternatives
+ and all(
+ isinstance(alternative, dict) and _is_object_schema(alternative, schema)
+ for alternative in alternatives
+ )
+ ):
+ schema["type"] = "object"
+
+ if not _is_object_schema(schema, schema):
+ definitions = schema.pop("$defs", None)
+ schema = {
+ "properties": {"result": schema},
+ "required": ["result"],
+ "type": "object",
+ "x-fastmcp-wrap-result": True,
+ }
+ if definitions:
+ schema["$defs"] = definitions
+
+ return _strip_schema_titles(schema)
+
+
def _get_prefixed_id_with_context(base_id: str) -> tuple[str, str]:
"""
Get ID with extension prefixing based on ambient context.
@@ -134,13 +216,21 @@ def create_tool_decorator(
from fastmcp.tools import Tool
- tool = Tool.from_function(
- wrapped_func,
- name=tool_name,
- description=tool_description,
- tags=tool_tags,
- annotations=annotations,
- )
+ tool_options: dict[str, Any] = {
+ "name": tool_name,
+ "description": tool_description,
+ "tags": tool_tags,
+ "annotations": annotations,
+ }
+ # Extension tools keep FastMCP's schema behavior because their
+ # serializers are extension-owned and cannot be assumed to follow
+ # Superset's validation-schema-as-output-superset convention.
+ if context_type == "host" and (
+ output_schema := _native_tool_output_schema(wrapped_func)
+ ):
+ tool_options["output_schema"] = output_schema
+
+ tool = Tool.from_function(wrapped_func, **tool_options)
mcp.add_tool(tool)
protected_status = "protected" if protect else "public"
diff --git a/superset/mcp_service/CLAUDE.md b/superset/mcp_service/CLAUDE.md
index 5e092ab1356..42782e57ad1 100644
--- a/superset/mcp_service/CLAUDE.md
+++ b/superset/mcp_service/CLAUDE.md
@@ -445,7 +445,7 @@ The MCP service uses FastMCP middleware (registered in `server.py`):
- **`LoggingMiddleware`**: Logs tool calls with duration, entity IDs, sanitizes sensitive data
- **`GlobalErrorHandlerMiddleware`**: Catches unhandled exceptions, converts to ToolError
-- **`StructuredContentStripperMiddleware`**: Strips structuredContent from responses (Claude.ai compatibility)
+- **`ToolResultCompatibilityMiddleware`**: Gates structured results using `MCP_STRUCTURED_OUTPUT_ENABLED` and sanitizes last-resort errors
- **`ResponseSizeGuardMiddleware`**: Prevents oversized responses from crashing clients
- **`ResponseCachingMiddleware`**: Optional response caching (in-memory by default, Redis when store enabled)
@@ -581,27 +581,35 @@ async def test_my_tool_success(mcp_server):
`openWorldHint`. Mutating tools (`readOnlyHint=False`) must also include
`idempotentHint`.
-### 5. Using `Optional` Instead of Union Syntax
+### 5. Unconstrained Output Schema
+**Problem**: Returning `dict[str, Any]` advertises an object with no useful
+fields to MCP clients.
+**Solution**: Give every tool a concrete Pydantic model or `TypedDict` return
+annotation that describes both success and error fields. Superset derives
+`outputSchema` from that annotation and advertises it when
+`MCP_STRUCTURED_OUTPUT_ENABLED=True`.
+
+### 6. Using `Optional` Instead of Union Syntax
**Problem**: Old-style `Optional[T]` is not Python 3.10+ style.
**Solution**: Use `T | None` and `list[str]` instead of `Optional[T]` and `List[str]`.
-### 6. Direct Database Queries
+### 7. Direct Database Queries
**Problem**: Bypasses Superset's security and caching layers.
**Solution**: Use DAO classes (ChartDAO, DashboardDAO, DatasetDAO, DatabaseDAO).
-### 7. Not Using Core Classes
+### 8. Not Using Core Classes
**Problem**: Duplicating list/get_info logic across tools.
**Solution**: Use `ModelListCore`, `ModelGetInfoCore`, `ModelGetSchemaCore`.
-### 8. Missing Apache License Headers
+### 9. Missing Apache License Headers
**Problem**: CI fails on license check.
**Solution**: Add ASF license header to all new `.py` files (see template at top of this doc).
-### 9. Circular Imports
+### 10. Circular Imports
**Problem**: Importing from `app.py` in tool files causes circular dependencies.
**Solution**: Use `from superset_core.mcp.decorators import tool` for tools/prompts. Only import `from superset.mcp_service.app import mcp` in resource files.
-### 10. Missing event_logger Instrumentation
+### 11. Missing event_logger Instrumentation
**Problem**: Tool operations are invisible to observability.
**Solution**: Wrap key operations with `event_logger.log_context(action="mcp.tool_name.step")`.
@@ -612,6 +620,7 @@ async def test_my_tool_success(mcp_server):
- [ ] Used `@tool(tags=[...], class_permission_name="...", annotations=ToolAnnotations(...))` decorator
- [ ] Import: `from superset_core.mcp.decorators import tool, ToolAnnotations`
- [ ] Created Pydantic request/response schemas in `{module}/schemas.py`
+- [ ] Used a concrete response model or `TypedDict` return annotation
- [ ] Used DAO classes instead of direct queries
- [ ] Added `event_logger.log_context()` instrumentation
- [ ] Used `await ctx.info/error/debug()` for context logging
diff --git a/superset/mcp_service/PRODUCTION.md b/superset/mcp_service/PRODUCTION.md
index 3bfffc6d9f7..2ba242aa9a7 100644
--- a/superset/mcp_service/PRODUCTION.md
+++ b/superset/mcp_service/PRODUCTION.md
@@ -274,7 +274,7 @@ MCP_ERROR_HOOK = _mcp_error_hook
`MCP_ERROR_HOOK` is invoked from `GlobalErrorHandlerMiddleware` (the primary
capture point, for every system-class error) and from
-`StructuredContentStripperMiddleware`'s last-resort exception handler (for
+`ToolResultCompatibilityMiddleware`'s last-resort exception handler (for
errors that slip past the primary handler entirely). Hook failures are
caught and logged; they never affect the MCP response.
diff --git a/superset/mcp_service/__main__.py b/superset/mcp_service/__main__.py
index cb33e3da1aa..b67ed5f81fe 100644
--- a/superset/mcp_service/__main__.py
+++ b/superset/mcp_service/__main__.py
@@ -59,11 +59,14 @@ if os.environ.get("FASTMCP_TRANSPORT", "stdio") == "stdio":
from superset.mcp_service.app import init_fastmcp_server, mcp
from superset.mcp_service.caching import create_response_caching_middleware
+from superset.mcp_service.mcp_config import MCP_STRUCTURED_OUTPUT_ENABLED
from superset.mcp_service.middleware import create_response_size_guard_middleware
from superset.mcp_service.server import build_middleware_list
-def _add_default_middlewares() -> None:
+def _add_default_middlewares(
+ *, structured_output_enabled: bool = MCP_STRUCTURED_OUTPUT_ENABLED
+) -> None:
"""Add the standard middleware stack to the MCP instance.
Delegates to ``server.build_middleware_list()`` for the core stack so
@@ -76,7 +79,9 @@ def _add_default_middlewares() -> None:
``build_middleware_list()`` already returns middlewares in the correct
outermost-first order.
"""
- for middleware in build_middleware_list():
+ for middleware in build_middleware_list(
+ structured_output_enabled=structured_output_enabled
+ ):
mcp.add_middleware(middleware)
# Response size guard is innermost (added last), then response caching.
@@ -143,7 +148,11 @@ def main() -> None:
# Initialize the FastMCP server
# Disable auth config for stdio mode to avoid Flask app output
init_fastmcp_server()
- _add_default_middlewares()
+ _add_default_middlewares(
+ structured_output_enabled=flask_app.config.get(
+ "MCP_STRUCTURED_OUTPUT_ENABLED", MCP_STRUCTURED_OUTPUT_ENABLED
+ )
+ )
# Log captured output to stderr for debugging (optional)
captured = captured_output.getvalue()
@@ -176,7 +185,11 @@ def main() -> None:
flask_app = get_flask_app()
auth_provider = _create_auth_provider(flask_app)
init_fastmcp_server(auth=auth_provider)
- _add_default_middlewares()
+ _add_default_middlewares(
+ structured_output_enabled=flask_app.config.get(
+ "MCP_STRUCTURED_OUTPUT_ENABLED", MCP_STRUCTURED_OUTPUT_ENABLED
+ )
+ )
# Run with specified transport
if transport == "streamable-http":
diff --git a/superset/mcp_service/chart/schemas.py b/superset/mcp_service/chart/schemas.py
index 7561689c5d2..b1e50cdbb1b 100644
--- a/superset/mcp_service/chart/schemas.py
+++ b/superset/mcp_service/chart/schemas.py
@@ -40,7 +40,7 @@ from pydantic import (
StrictBool,
ValidationError,
)
-from typing_extensions import Self
+from typing_extensions import Self, TypedDict
from superset.constants import NO_TIME_RANGE, TimeGrain
from superset.daos.base import ColumnOperator, ColumnOperatorEnum
@@ -3007,6 +3007,27 @@ class UpdateChartPreviewRequest(ChartRequestNormalizerMixin, FormDataCacheContro
)
+class UpdateChartPreviewResponse(TypedDict, total=False):
+ """Output fields returned while iterating on a cached chart preview."""
+
+ chart: dict[str, Any] | None
+ previews: dict[str, Any]
+ capabilities: dict[str, Any] | None
+ semantics: dict[str, Any] | None
+ explore_url: str
+ form_data_key: str
+ form_data: dict[str, Any]
+ previous_form_data_key: str | None
+ warnings: list[str]
+ api_endpoints: dict[str, str]
+ performance: dict[str, Any] | None
+ accessibility: dict[str, Any] | None
+ success: bool
+ error: str | dict[str, Any] | None
+ schema_version: str
+ api_version: str
+
+
class GetChartDataRequest(QueryCacheControl):
"""Request for chart data with cache control.
diff --git a/superset/mcp_service/chart/tool/get_chart_info.py b/superset/mcp_service/chart/tool/get_chart_info.py
index dfe801af73e..2e905a6395b 100644
--- a/superset/mcp_service/chart/tool/get_chart_info.py
+++ b/superset/mcp_service/chart/tool/get_chart_info.py
@@ -20,7 +20,7 @@ MCP tool: get_chart_info
"""
import logging
-from typing import Any
+from typing import Any, cast
from fastmcp import Context
from sqlalchemy.orm import subqueryload
@@ -233,7 +233,7 @@ def _apply_unsaved_state_override(result: ChartInfo, form_data_key: str) -> None
)
async def get_chart_info( # noqa: C901
request: GetChartInfoRequest, ctx: Context
-) -> dict[str, Any] | ChartError:
+) -> ChartInfo | ChartError:
"""Get chart metadata by ID or UUID.
IMPORTANT FOR LLM CLIENTS:
@@ -303,9 +303,12 @@ async def get_chart_info( # noqa: C901
result = redact_chart_data_model_fields(result)
if request.extra_form_data:
_attach_active_filters(result, request.extra_form_data)
- return result.model_dump(
- mode="json",
- context={"select_columns": request.select_columns},
+ return cast(
+ ChartInfo,
+ result.model_dump(
+ mode="json",
+ context={"select_columns": request.select_columns},
+ ),
)
# At this point identifier must be set (validator ensures at least one
@@ -366,9 +369,12 @@ async def get_chart_info( # noqa: C901
if request.extra_form_data:
_attach_active_filters(result, request.extra_form_data)
- return result.model_dump(
- mode="json",
- context={"select_columns": request.select_columns},
+ return cast(
+ ChartInfo,
+ result.model_dump(
+ mode="json",
+ context={"select_columns": request.select_columns},
+ ),
)
else:
await ctx.warning("Chart retrieval failed: error=%s" % (str(result),))
diff --git a/superset/mcp_service/chart/tool/get_chart_type_schema.py b/superset/mcp_service/chart/tool/get_chart_type_schema.py
index 18897163780..19f40392545 100644
--- a/superset/mcp_service/chart/tool/get_chart_type_schema.py
+++ b/superset/mcp_service/chart/tool/get_chart_type_schema.py
@@ -26,6 +26,7 @@ from typing import Any, Dict
from pydantic import TypeAdapter
from superset_core.mcp.decorators import tool, ToolAnnotations
+from typing_extensions import TypedDict
from superset.extensions import event_logger
from superset.mcp_service.chart.schemas import (
@@ -46,6 +47,17 @@ from superset.mcp_service.chart.schemas import (
logger = logging.getLogger(__name__)
+
+class ChartTypeSchemaResponse(TypedDict, total=False):
+ """Output fields returned by ``get_chart_type_schema``."""
+
+ chart_type: str
+ schema: dict[str, Any]
+ examples: list[dict[str, Any]]
+ error: dict[str, Any]
+ valid_chart_types: list[str]
+
+
# Module-level TypeAdapters — one per chart type, compiled once.
_CHART_TYPE_ADAPTERS: Dict[str, TypeAdapter[Any]] = {
"xy": TypeAdapter(XYChartConfig),
@@ -227,7 +239,7 @@ _CHART_EXAMPLES: Dict[str, list[Dict[str, Any]]] = {
def _get_chart_type_schema_impl(
chart_type: str,
include_examples: bool = True,
-) -> Dict[str, Any]:
+) -> ChartTypeSchemaResponse:
"""Pure logic for chart type schema lookup — no auth, no decorators."""
from superset.mcp_service.chart.registry import get_registry
@@ -279,7 +291,7 @@ def _get_chart_type_schema_impl(
}
schema = adapter.json_schema()
- result: Dict[str, Any] = {
+ result: ChartTypeSchemaResponse = {
"chart_type": chart_type,
"schema": schema,
}
@@ -303,7 +315,7 @@ def _get_chart_type_schema_impl(
def get_chart_type_schema(
chart_type: str,
include_examples: bool = True,
-) -> Dict[str, Any]:
+) -> ChartTypeSchemaResponse:
"""Get the full JSON Schema and examples for a specific chart type.
Use this tool to discover the exact fields, types, and constraints
diff --git a/superset/mcp_service/chart/tool/update_chart_preview.py b/superset/mcp_service/chart/tool/update_chart_preview.py
index ead15007f13..1c52d1fdb9f 100644
--- a/superset/mcp_service/chart/tool/update_chart_preview.py
+++ b/superset/mcp_service/chart/tool/update_chart_preview.py
@@ -52,6 +52,7 @@ from superset.mcp_service.chart.schemas import (
ChartError,
PerformanceMetadata,
UpdateChartPreviewRequest,
+ UpdateChartPreviewResponse,
)
from superset.mcp_service.utils.oauth2_utils import (
build_oauth2_redirect_message,
@@ -119,7 +120,7 @@ def _get_previous_form_data(form_data_key: str) -> dict[str, Any] | None:
)
def update_chart_preview( # noqa: C901
request: UpdateChartPreviewRequest, ctx: Context
-) -> Dict[str, Any]:
+) -> UpdateChartPreviewResponse:
"""Update cached chart preview without saving.
IMPORTANT:
@@ -356,7 +357,7 @@ def update_chart_preview( # noqa: C901
logger.warning("Preview generation failed: %s", e)
# Return enhanced data
- result = {
+ result: UpdateChartPreviewResponse = {
"chart": {
"id": None,
"slice_name": chart_name,
@@ -394,6 +395,8 @@ def update_chart_preview( # noqa: C901
"chart": None,
"error": build_oauth2_redirect_message(ex),
"success": False,
+ "schema_version": "2.0",
+ "api_version": "v1",
}
except OAuth2Error:
logger.warning(
@@ -403,6 +406,8 @@ def update_chart_preview( # noqa: C901
"chart": None,
"error": OAUTH2_CONFIG_ERROR_MESSAGE,
"success": False,
+ "schema_version": "2.0",
+ "api_version": "v1",
}
except (
SupersetException,
diff --git a/superset/mcp_service/dashboard/tool/get_dashboard_info.py b/superset/mcp_service/dashboard/tool/get_dashboard_info.py
index 3070c9acda8..fbda5943d32 100644
--- a/superset/mcp_service/dashboard/tool/get_dashboard_info.py
+++ b/superset/mcp_service/dashboard/tool/get_dashboard_info.py
@@ -24,7 +24,7 @@ about a specific dashboard.
import logging
from datetime import datetime, timezone
-from typing import Any
+from typing import cast
from fastmcp import Context
from sqlalchemy.orm import subqueryload
@@ -105,7 +105,7 @@ def _lookup_dashboard(
)
async def get_dashboard_info(
request: GetDashboardInfoRequest, ctx: Context
-) -> dict[str, Any] | DashboardError:
+) -> DashboardInfo | DashboardError:
"""
Get dashboard metadata by ID, UUID, slug, or dashboard permalink.
@@ -249,9 +249,12 @@ async def get_dashboard_info(
):
effective_select_columns.append("filter_state")
- return result.model_dump(
- mode="json",
- context={"select_columns": effective_select_columns},
+ return cast(
+ DashboardInfo,
+ result.model_dump(
+ mode="json",
+ context={"select_columns": effective_select_columns},
+ ),
)
else:
await ctx.warning(
diff --git a/superset/mcp_service/dataset/tool/get_dataset_info.py b/superset/mcp_service/dataset/tool/get_dataset_info.py
index d9ee6ba4814..44ce55f5e84 100644
--- a/superset/mcp_service/dataset/tool/get_dataset_info.py
+++ b/superset/mcp_service/dataset/tool/get_dataset_info.py
@@ -24,7 +24,7 @@ about a specific dataset.
import logging
from datetime import datetime, timezone
-from typing import Any
+from typing import cast
from fastmcp import Context
from sqlalchemy.orm import joinedload, subqueryload
@@ -60,7 +60,7 @@ logger = logging.getLogger(__name__)
@requires_data_model_metadata_access
async def get_dataset_info(
request: GetDatasetInfoRequest, ctx: Context
-) -> dict[str, Any] | DatasetError:
+) -> DatasetInfo | DatasetError:
"""Get dataset metadata by ID or UUID.
Returns columns, metrics, and schema details.
@@ -168,13 +168,16 @@ async def get_dataset_info(
% (request.select_columns, request.column_fields)
)
with event_logger.log_context(action="mcp.get_dataset_info.serialization"):
- return result.model_dump(
- mode="json",
- by_alias=True,
- context={
- "select_columns": request.select_columns,
- "column_fields": request.column_fields,
- },
+ return cast(
+ DatasetInfo,
+ result.model_dump(
+ mode="json",
+ by_alias=True,
+ context={
+ "select_columns": request.select_columns,
+ "column_fields": request.column_fields,
+ },
+ ),
)
else:
await ctx.warning(
diff --git a/superset/mcp_service/mcp_config.py b/superset/mcp_service/mcp_config.py
index 124d87acc96..c399190c03a 100644
--- a/superset/mcp_service/mcp_config.py
+++ b/superset/mcp_service/mcp_config.py
@@ -101,6 +101,13 @@ MCP_RBAC_ENABLED = True
# MCP_DISABLED_TOOLS = {"extensions.myorg.myext.some_tool"}
MCP_DISABLED_TOOLS: set[str] = set()
+# Structured MCP tool output is opt-in because some clients and transport
+# bridges cannot handle outputSchema and structuredContent consistently. False
+# preserves the legacy text-only wire contract by removing both fields at the
+# outer compatibility middleware. Enable only after validating every MCP client
+# and bridge used by the deployment.
+MCP_STRUCTURED_OUTPUT_ENABLED = False
+
# Pluggable error-capture hook, invoked for system-class MCP tool errors
# (unexpected exceptions — database down, bugs — not user errors like bad
# params or permission denials). Lets operators forward failures to an
@@ -114,7 +121,7 @@ MCP_DISABLED_TOOLS: set[str] = set()
# "user_id", "error_type", "sanitized_message", and "duration_ms" — but
# values may be unavailable depending on the capture path: "user_id" and
# "duration_ms" are None on the last-resort path
-# (StructuredContentStripperMiddleware), "mcp_call_id" is None outside a
+# (ToolResultCompatibilityMiddleware), "mcp_call_id" is None outside a
# tool call, and "tool_name" falls back to "unknown" for non-tool
# messages. Only "sanitized_message" is scrubbed — the ``error`` argument
# is the RAW exception and may contain sensitive data (connection
@@ -887,6 +894,7 @@ def get_mcp_config(app_config: dict[str, Any] | None = None) -> dict[str, Any]:
"MCP_STATELESS_HTTP": MCP_STATELESS_HTTP,
"MCP_RBAC_ENABLED": MCP_RBAC_ENABLED,
"MCP_DISABLED_TOOLS": set(MCP_DISABLED_TOOLS),
+ "MCP_STRUCTURED_OUTPUT_ENABLED": MCP_STRUCTURED_OUTPUT_ENABLED,
"MCP_DISABLED_CHART_PLUGINS": MCP_DISABLED_CHART_PLUGINS,
"MCP_CHART_PLUGIN_ENABLED_FUNC": MCP_CHART_PLUGIN_ENABLED_FUNC,
"MCP_EMBEDDED_GUEST_AUTH_ENABLED": MCP_EMBEDDED_GUEST_AUTH_ENABLED,
diff --git a/superset/mcp_service/middleware.py b/superset/mcp_service/middleware.py
index 1a2bfe186d5..439de294b43 100644
--- a/superset/mcp_service/middleware.py
+++ b/superset/mcp_service/middleware.py
@@ -19,6 +19,7 @@ import logging
import re
import secrets
import time
+import warnings
from contextvars import ContextVar
from typing import Any, Awaitable, Callable, Sequence
@@ -737,25 +738,25 @@ class LoggingMiddleware(Middleware):
)
-class StructuredContentStripperMiddleware(Middleware):
- """Strip ``outputSchema`` and ``structured_content`` to prevent encoding errors.
+class ToolResultCompatibilityMiddleware(Middleware):
+ """Gate structured results while providing a last-resort error boundary.
FastMCP 3.x auto-generates ``outputSchema`` in tool definitions
(``tools/list``) and ``structuredContent`` in tool call responses
(``tools/call``) when the tool has a typed return annotation.
- Some MCP client transports (e.g. Claude.ai's MCP bridge) cannot handle
- ``structuredContent`` dicts, causing ``TypeError: encoding without a
- string argument``. Additionally, if ``outputSchema`` is advertised but
- ``structuredContent`` is stripped from the response, clients may raise
- ``Output validation error: outputSchema defined but no structured output
- returned``.
-
- This middleware handles both sides:
- - ``on_list_tools``: removes ``output_schema`` from every tool definition
- - ``on_call_tool``: removes ``structured_content`` from every tool result
+ Structured output is part of the MCP contract, but some transport bridges
+ cannot encode it. When ``structured_output_enabled`` is false, this
+ middleware preserves the legacy text-only contract by stripping both sides
+ of that contract: ``outputSchema`` from discovery and ``structuredContent``
+ from successful calls. It always converts exceptions that escape the inner
+ error handler into a sanitized text result and returns an empty tool list if
+ discovery itself fails.
"""
+ def __init__(self, *, structured_output_enabled: bool = False) -> None:
+ self.structured_output_enabled = structured_output_enabled
+
async def on_list_tools(
self,
context: MiddlewareContext[mt.ListToolsRequest],
@@ -769,18 +770,20 @@ class StructuredContentStripperMiddleware(Middleware):
# list, not an error object — causing "encoding without a string argument".
# Return an empty list; GlobalErrorHandlerMiddleware already logged it.
return []
+ if self.structured_output_enabled:
+ return tools
return [
- t.model_copy(update={"output_schema": None})
- if t.output_schema is not None
- else t
- for t in tools
+ tool.model_copy(update={"output_schema": None})
+ if tool.output_schema is not None
+ else tool
+ for tool in tools
]
async def on_call_tool(
self,
context: MiddlewareContext[mt.CallToolRequestParams],
- call_next: Callable[[MiddlewareContext], Awaitable[ToolResult]],
- ) -> ToolResult:
+ call_next: Callable[[MiddlewareContext], Awaitable[Any]],
+ ) -> Any:
try:
result = await call_next(context)
except Exception as e:
@@ -832,18 +835,42 @@ class StructuredContentStripperMiddleware(Middleware):
# CallToolResult(isError=True) (see ToolResult.to_mcp_result);
# what keeps it encodable is that structured_content stays None
# and only the boolean flips false->true, not the structured
- # payload implicated in the bridge failure above. That leg is
- # unverified against the live Claude.ai bridge.
+ # payload implicated in transport-level encoding failures.
return ToolResult(
content=[mt.TextContent(type="text", text=error_text)],
meta={"mcp_call_id": mcp_call_id} if mcp_call_id else None,
is_error=True,
)
- if isinstance(result, ToolResult) and result.structured_content is not None:
- result = ToolResult(content=result.content, meta=result.meta)
+ if (
+ not self.structured_output_enabled
+ and isinstance(result, ToolResult)
+ and result.structured_content is not None
+ ):
+ return ToolResult(
+ content=result.content,
+ # A non-null meta value makes ToolResult.to_mcp_result() retain
+ # the CallToolResult envelope. Without it, FastMCP returns a bare
+ # content list and the MCP SDK rejects the missing structured
+ # content against the live tool's outputSchema.
+ meta=result.meta or {},
+ is_error=result.is_error,
+ )
return result
+class StructuredContentStripperMiddleware(ToolResultCompatibilityMiddleware):
+ """Deprecated compatibility middleware that retains its stripping behavior."""
+
+ def __init__(self) -> None:
+ warnings.warn(
+ "StructuredContentStripperMiddleware is deprecated; use "
+ "ToolResultCompatibilityMiddleware(structured_output_enabled=False)",
+ DeprecationWarning,
+ stacklevel=2,
+ )
+ super().__init__(structured_output_enabled=False)
+
+
class RBACToolVisibilityMiddleware(Middleware):
"""Filter tools/list response based on current user's RBAC permissions.
diff --git a/superset/mcp_service/server.py b/superset/mcp_service/server.py
index feef18a71d8..15afa7397a2 100644
--- a/superset/mcp_service/server.py
+++ b/superset/mcp_service/server.py
@@ -38,6 +38,7 @@ from superset.mcp_service.mcp_config import (
get_mcp_factory_config,
MCP_STATELESS_HTTP,
MCP_STORE_CONFIG,
+ MCP_STRUCTURED_OUTPUT_ENABLED,
MCP_TOOL_SEARCH_CONFIG,
)
from superset.mcp_service.middleware import (
@@ -45,7 +46,7 @@ from superset.mcp_service.middleware import (
GlobalErrorHandlerMiddleware,
LoggingMiddleware,
RBACToolVisibilityMiddleware,
- StructuredContentStripperMiddleware,
+ ToolResultCompatibilityMiddleware,
)
from superset.mcp_service.storage import _create_redis_store
from superset.utils import json
@@ -887,28 +888,39 @@ def _create_auth_provider(flask_app: Any) -> Any | None:
return auth_provider
-def build_middleware_list() -> list[Middleware]:
+def build_middleware_list(
+ *, structured_output_enabled: bool = MCP_STRUCTURED_OUTPUT_ENABLED
+) -> list[Middleware]:
"""Build the core MCP middleware list in the correct order.
FastMCP wraps handlers so that the FIRST-added middleware is
outermost. Order here is outermost → innermost:
- 1. StructuredContentStripper — safety net, converts exceptions
- to safe ToolResult text for transports that can't encode errors
+ 1. ToolResultCompatibility — applies the structured-output compatibility
+ setting and converts exceptions to safe ToolResult text
2. RBACToolVisibilityMiddleware — filters tools/list by RBAC;
- positioned inside the Stripper so it sees full tool objects
- (with outputSchema) before stripping occurs
+ positioned inside the compatibility boundary so it sees full tool objects
+ (with outputSchema) before results are returned
3. LoggingMiddleware — logs tool calls with success/failure status
4. GlobalErrorHandler — catches tool exceptions, raises ToolError
"""
return [
- StructuredContentStripperMiddleware(),
+ ToolResultCompatibilityMiddleware(
+ structured_output_enabled=structured_output_enabled
+ ),
RBACToolVisibilityMiddleware(),
LoggingMiddleware(),
GlobalErrorHandlerMiddleware(),
]
+def _structured_output_enabled(flask_app: Any) -> bool:
+ """Resolve the structured-output setting for server-managed startup paths."""
+ return flask_app.config.get(
+ "MCP_STRUCTURED_OUTPUT_ENABLED", MCP_STRUCTURED_OUTPUT_ENABLED
+ )
+
+
def _build_starlette_middleware(
flask_app: Any | None = None, auth_provider: Any | None = None
) -> list[Any]:
@@ -1007,6 +1019,18 @@ def run_server(
# Use factory configuration for customization
logging.info("Creating MCP app from factory configuration...")
factory_config = get_mcp_factory_config()
+ from superset.mcp_service.flask_singleton import ( # noqa: PLC0415
+ get_flask_app,
+ )
+
+ factory_flask_app = get_flask_app()
+ factory_middleware = factory_config.get("middleware") or ()
+ factory_config["middleware"] = [
+ *build_middleware_list(
+ structured_output_enabled=_structured_output_enabled(factory_flask_app)
+ ),
+ *factory_middleware,
+ ]
mcp_instance = create_mcp_app(**factory_config)
# The factory path bypasses init_fastmcp_server(), so install the
# per-tool-call session scoping here as well; without it concurrent
@@ -1035,7 +1059,9 @@ def run_server(
flask_app = get_flask_app()
auth_provider = _create_auth_provider(flask_app)
- middleware_list = build_middleware_list()
+ middleware_list = build_middleware_list(
+ structured_output_enabled=_structured_output_enabled(flask_app)
+ )
# Add optional middleware (innermost, closest to tool)
size_guard_middleware = create_response_size_guard_middleware()
diff --git a/tests/unit_tests/mcp_service/chart/tool/test_get_chart_info.py b/tests/unit_tests/mcp_service/chart/tool/test_get_chart_info.py
index c9dc3764b02..0213da3a7c4 100644
--- a/tests/unit_tests/mcp_service/chart/tool/test_get_chart_info.py
+++ b/tests/unit_tests/mcp_service/chart/tool/test_get_chart_info.py
@@ -27,6 +27,7 @@ from unittest.mock import AsyncMock, MagicMock, Mock, patch
import pytest
from fastmcp import Client
+from jsonschema import validate
from superset.commands.dashboard.exceptions import DashboardNotFoundError
from superset.mcp_service.app import mcp
@@ -667,9 +668,13 @@ class TestGetChartInfoPrivacy:
assert "form_data" not in result
assert "datasource_name" not in result
+ tool = await mcp_server.get_tool("get_chart_info")
+ assert tool.output_schema is not None
+ validate(instance=response.structured_content, schema=tool.output_schema)
+
@pytest.mark.asyncio
- async def test_unsaved_chart_error_returned_unchanged(self) -> None:
- """ChartError results should not be serialized as success dictionaries."""
+ async def test_unsaved_chart_error_matches_output_schema(self, mcp_server) -> None:
+ """The protocol error payload conforms to the advertised union branch."""
error = ChartError(error="Missing cached chart data", error_type="NotFound")
with (
@@ -689,13 +694,23 @@ class TestGetChartInfoPrivacy:
"_build_unsaved_chart_info",
return_value=error,
),
+ patch("superset.mcp_service.auth.check_tool_permission", return_value=True),
):
- result = await get_chart_info_module.get_chart_info(
- request=GetChartInfoRequest(form_data_key="missing-key"),
- ctx=SimpleNamespace(info=AsyncMock()),
- )
+ async with Client(mcp_server) as client:
+ response = await client.call_tool(
+ "get_chart_info",
+ {
+ "request": GetChartInfoRequest(
+ form_data_key="missing-key"
+ ).model_dump()
+ },
+ )
- assert result is error
+ assert response.structured_content["error"] == "Missing cached chart data"
+ assert response.structured_content["error_type"] == "NotFound"
+ tool = await mcp_server.get_tool("get_chart_info")
+ assert tool.output_schema is not None
+ validate(instance=response.structured_content, schema=tool.output_schema)
@pytest.mark.asyncio
async def test_unsaved_chart_surfaces_active_filters(self) -> None:
diff --git a/tests/unit_tests/mcp_service/chart/tool/test_update_chart_preview.py b/tests/unit_tests/mcp_service/chart/tool/test_update_chart_preview.py
index e7306bb993a..3f73d64ba3c 100644
--- a/tests/unit_tests/mcp_service/chart/tool/test_update_chart_preview.py
+++ b/tests/unit_tests/mcp_service/chart/tool/test_update_chart_preview.py
@@ -185,6 +185,42 @@ def test_cached_gauge_update_preserves_controls_and_compiles(
class TestUpdateChartPreview:
"""Tests for update_chart_preview MCP tool."""
+ @pytest.mark.parametrize("redirect", [False, True])
+ def test_oauth_errors_include_response_versions(
+ self, redirect: bool, mock_auth: Mock
+ ) -> None:
+ """OAuth error branches retain the common response contract."""
+ from superset.exceptions import OAuth2Error, OAuth2RedirectError
+
+ error = (
+ OAuth2RedirectError(
+ "https://example.com/oauth",
+ "tab-1",
+ "https://example.com/redirect",
+ )
+ if redirect
+ else OAuth2Error("token refresh failed")
+ )
+ request = UpdateChartPreviewRequest(
+ dataset_id=1,
+ config=TableChartConfig(
+ chart_type="table",
+ columns=[ColumnRef(name="country")],
+ ),
+ )
+
+ with patch.object(
+ update_chart_preview_module, "_find_dataset", side_effect=error
+ ):
+ result = update_chart_preview_module.update_chart_preview(
+ request=request,
+ ctx=Mock(),
+ )
+
+ assert result["success"] is False
+ assert result["schema_version"] == "2.0"
+ assert result["api_version"] == "v1"
+
@pytest.mark.asyncio
async def test_update_chart_preview_request_structure(self):
"""Test that chart preview update request structures are properly formed."""
@@ -1362,9 +1398,9 @@ class TestUpdateChartPreviewValidation:
"update_chart_preview", {"request": request.model_dump()}
)
- assert result.data["success"] is False
- assert result.data["chart"] is None
- error = result.data["error"]
+ assert result.structured_content["success"] is False
+ assert result.structured_content["chart"] is None
+ error = result.structured_content["error"]
assert isinstance(error, dict)
assert error["error_code"] == "CHART_VALIDATION_FAILED"
assert "sum_boys" in error["suggestions"]
@@ -1402,9 +1438,9 @@ class TestUpdateChartPreviewValidation:
"update_chart_preview", {"request": request.model_dump()}
)
- assert result.data["success"] is False
- assert result.data["chart"] is None
- error = result.data["error"]
+ assert result.structured_content["success"] is False
+ assert result.structured_content["chart"] is None
+ error = result.structured_content["error"]
assert isinstance(error, dict)
assert error["error_type"] == "DatasetNotAccessible"
mock_create_form_data.assert_not_called()
diff --git a/tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_tools.py b/tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_tools.py
index 2370deac288..3199711140b 100644
--- a/tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_tools.py
+++ b/tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_tools.py
@@ -26,6 +26,7 @@ from unittest.mock import Mock, patch
import pytest
from fastmcp import Client
from fastmcp.exceptions import ToolError
+from jsonschema import validate
from superset.mcp_service.app import mcp
from superset.mcp_service.dashboard.schemas import (
@@ -385,17 +386,21 @@ async def test_get_dashboard_info_success(
result = await client.call_tool(
"get_dashboard_info", {"request": {"identifier": 1}}
)
- assert result.data["dashboard_title"] == _wrapped("Test Dashboard")
- assert result.data["description"] == _wrapped("Test description")
- assert result.data["certification_details"] == _wrapped(
+ assert result.structured_content["dashboard_title"] == _wrapped(
+ "Test Dashboard"
+ )
+ assert result.structured_content["description"] == _wrapped("Test description")
+ assert result.structured_content["certification_details"] == _wrapped(
"Certified by data team"
)
- assert result.data["slug"] == "test-dashboard"
- assert result.data["url"].endswith("/dashboard/1")
- assert result.data["uuid"] == "dashboard-uuid-1"
- assert result.data["native_filters"][0]["id"] == "native-filter-1"
- assert result.data["native_filters"][0]["name"] == _wrapped("Region Filter")
- assert result.data["native_filters"][0]["targets"] == [
+ assert result.structured_content["slug"] == "test-dashboard"
+ assert result.structured_content["url"].endswith("/dashboard/1")
+ assert result.structured_content["uuid"] == "dashboard-uuid-1"
+ assert result.structured_content["native_filters"][0]["id"] == "native-filter-1"
+ assert result.structured_content["native_filters"][0]["name"] == _wrapped(
+ "Region Filter"
+ )
+ assert result.structured_content["native_filters"][0]["targets"] == [
{"column": {"name": _wrapped("region")}, "datasetId": 12}
]
@@ -482,22 +487,28 @@ async def test_get_dashboard_info_permalink_does_not_double_sanitize(
{"request": {"identifier": 1, "permalink_key": "permalink-1"}},
)
- assert result.data["dashboard_title"] == _wrapped("Test Dashboard")
- assert result.data["description"] == _wrapped("Test description")
- assert result.data["certification_details"] == _wrapped("Certified by data team")
- assert result.data["native_filters"][0]["name"] == _wrapped("Region Filter")
- assert result.data["permalink_key"] == "permalink-1"
- assert result.data["is_permalink_state"] is True
- assert result.data["filter_state"]["dataMask"]["native-filter-1"]["filterState"][
- "label"
- ] == _wrapped("EMEA")
- assert result.data["filter_state"]["dataMask"]["native-filter-1"]["filterState"][
- "url"
- ] == _wrapped("https://example.com/filter-value")
- assert result.data["filter_state"]["dataMask"]["native-filter-1"]["extraFormData"][
- "filters"
- ][0]["val"][0] == _wrapped("EMEA")
- assert result.data["filter_state"]["activeTabs"][0] == _wrapped("TAB-1")
+ assert result.structured_content["dashboard_title"] == _wrapped("Test Dashboard")
+ assert result.structured_content["description"] == _wrapped("Test description")
+ assert result.structured_content["certification_details"] == _wrapped(
+ "Certified by data team"
+ )
+ assert result.structured_content["native_filters"][0]["name"] == _wrapped(
+ "Region Filter"
+ )
+ assert result.structured_content["permalink_key"] == "permalink-1"
+ assert result.structured_content["is_permalink_state"] is True
+ assert result.structured_content["filter_state"]["dataMask"]["native-filter-1"][
+ "filterState"
+ ]["label"] == _wrapped("EMEA")
+ assert result.structured_content["filter_state"]["dataMask"]["native-filter-1"][
+ "filterState"
+ ]["url"] == _wrapped("https://example.com/filter-value")
+ assert result.structured_content["filter_state"]["dataMask"]["native-filter-1"][
+ "extraFormData"
+ ]["filters"][0]["val"][0] == _wrapped("EMEA")
+ assert result.structured_content["filter_state"]["activeTabs"][0] == _wrapped(
+ "TAB-1"
+ )
@patch("superset.daos.dashboard.DashboardDAO.find_by_id")
@@ -567,9 +578,9 @@ async def test_get_dashboard_info_permalink_key_includes_filter_state(
{"request": {"identifier": 42, "permalink_key": "some-key"}},
)
- assert "filter_state" in result.data
- assert result.data["is_permalink_state"] is True
- assert result.data["permalink_key"] == "some-key"
+ assert "filter_state" in result.structured_content
+ assert result.structured_content["is_permalink_state"] is True
+ assert result.structured_content["permalink_key"] == "some-key"
@patch("superset.mcp_service.mcp_core.ModelGetInfoCore.run_tool")
@@ -589,9 +600,11 @@ async def test_get_dashboard_info_resolves_permalink_without_identifier(
"get_dashboard_info", {"request": {"permalink_key": "shared-key"}}
)
- assert result.data["id"] == 42
- assert result.data["permalink_key"] == "shared-key"
- assert result.data["filter_state"]["activeTabs"] == [_wrapped("TAB-A")]
+ assert result.structured_content["id"] == 42
+ assert result.structured_content["permalink_key"] == "shared-key"
+ assert result.structured_content["filter_state"]["activeTabs"] == [
+ _wrapped("TAB-A")
+ ]
mock_run_tool.assert_called_once_with("42")
@@ -608,8 +621,8 @@ async def test_get_dashboard_info_invalid_permalink_is_actionable(
"get_dashboard_info", {"request": {"permalink_key": "expired-key"}}
)
- assert result.data["error_type"] == "permalink_not_found"
- assert "fresh shared dashboard link" in result.data["error"]
+ assert result.structured_content["error_type"] == "permalink_not_found"
+ assert "fresh shared dashboard link" in result.structured_content["error"]
@patch("superset.mcp_service.mcp_core.ModelGetInfoCore.run_tool")
@@ -632,9 +645,9 @@ async def test_get_dashboard_info_identifier_takes_precedence_over_permalink(
{"request": {"identifier": 10, "permalink_key": "dashboard-20-key"}},
)
- assert result.data["id"] == 10
- assert result.data["is_permalink_state"] is False
- assert "filter_state" not in result.data
+ assert result.structured_content["id"] == 10
+ assert result.structured_content["is_permalink_state"] is False
+ assert "filter_state" not in result.structured_content
mock_run_tool.assert_called_once_with(10)
@@ -662,10 +675,12 @@ async def test_get_dashboard_info_permalink_with_uuid_dashboard_id(
{"request": {"identifier": 42, "permalink_key": "uuid-key"}},
)
- assert result.data["id"] == 42
- assert result.data["is_permalink_state"] is True
- assert result.data["permalink_key"] == "uuid-key"
- assert result.data["filter_state"]["activeTabs"] == [_wrapped("TAB-A")]
+ assert result.structured_content["id"] == 42
+ assert result.structured_content["is_permalink_state"] is True
+ assert result.structured_content["permalink_key"] == "uuid-key"
+ assert result.structured_content["filter_state"]["activeTabs"] == [
+ _wrapped("TAB-A")
+ ]
@patch("superset.mcp_service.mcp_core.ModelGetInfoCore.run_tool")
@@ -689,8 +704,10 @@ async def test_get_dashboard_info_permalink_with_slug_dashboard_id(
{"request": {"identifier": 42, "permalink_key": "slug-key"}},
)
- assert result.data["is_permalink_state"] is True
- assert result.data["filter_state"]["activeTabs"] == [_wrapped("TAB-A")]
+ assert result.structured_content["is_permalink_state"] is True
+ assert result.structured_content["filter_state"]["activeTabs"] == [
+ _wrapped("TAB-A")
+ ]
@patch("superset.mcp_service.mcp_core.ModelGetInfoCore.run_tool")
@@ -714,9 +731,9 @@ async def test_get_dashboard_info_unknown_slug_keeps_not_found_error(
"get_dashboard_info", {"request": {"identifier": "sales-dashbord"}}
)
- assert result.data["error_type"] == "not_found"
- assert "sales-dashbord" in result.data["error"]
- assert "fresh shared dashboard link" not in result.data["error"]
+ assert result.structured_content["error_type"] == "not_found"
+ assert "sales-dashbord" in result.structured_content["error"]
+ assert "fresh shared dashboard link" not in result.structured_content["error"]
@patch("superset.daos.dashboard.DashboardDAO.find_by_id")
@@ -727,7 +744,11 @@ async def test_get_dashboard_info_not_found(mock_info, mcp_server):
result = await client.call_tool(
"get_dashboard_info", {"request": {"identifier": 999}}
)
- assert result.data["error_type"] == "not_found"
+ assert result.structured_content["error_type"] == "not_found"
+
+ tool = await mcp_server.get_tool("get_dashboard_info")
+ assert tool.output_schema is not None
+ validate(instance=result.structured_content, schema=tool.output_schema)
@patch("superset.daos.dashboard.DashboardDAO.find_by_id")
@@ -738,7 +759,7 @@ async def test_get_dashboard_info_access_denied(mock_info, mcp_server):
result = await client.call_tool(
"get_dashboard_info", {"request": {"identifier": 1}}
)
- assert result.data["error_type"] == "not_found"
+ assert result.structured_content["error_type"] == "not_found"
@patch("superset.daos.dashboard.DashboardDAO.find_by_id")
@@ -799,13 +820,15 @@ async def test_get_dashboard_info_does_not_expose_access_list(mock_info, mcp_ser
"get_dashboard_info", {"request": {"identifier": 1}}
)
- assert result.data["dashboard_title"] == _wrapped("Customer Success Home Dashboard")
- assert "created_by" not in result.data
- assert "changed_by" not in result.data
- assert "editors" not in result.data
- assert "created_by" not in result.data["charts"][0]
- assert "changed_by" not in result.data["charts"][0]
- assert "editors" not in result.data["charts"][0]
+ assert result.structured_content["dashboard_title"] == _wrapped(
+ "Customer Success Home Dashboard"
+ )
+ assert "created_by" not in result.structured_content
+ assert "changed_by" not in result.structured_content
+ assert "editors" not in result.structured_content
+ assert "created_by" not in result.structured_content["charts"][0]
+ assert "changed_by" not in result.structured_content["charts"][0]
+ assert "editors" not in result.structured_content["charts"][0]
@patch("superset.daos.dashboard.DashboardDAO.find_by_id")
@@ -875,12 +898,18 @@ async def test_get_dashboard_info_restricted_user_redacts_data_model_metadata(
{"request": {"identifier": 1}},
)
- assert result.data["dashboard_title"] == _wrapped("Sales Dashboard")
- assert result.data["charts"][0]["slice_name"] == _wrapped("Revenue by Deal Size")
- assert result.data["charts"][0]["viz_type"] == "echarts_timeseries_bar"
- assert result.data["charts"][0]["datasource_name"] is None
- assert result.data["native_filters"][0]["name"] == _wrapped("Product Line")
- assert result.data["native_filters"][0]["targets"] == []
+ assert result.structured_content["dashboard_title"] == _wrapped("Sales Dashboard")
+ assert result.structured_content["charts"][0]["slice_name"] == _wrapped(
+ "Revenue by Deal Size"
+ )
+ assert (
+ result.structured_content["charts"][0]["viz_type"] == "echarts_timeseries_bar"
+ )
+ assert result.structured_content["charts"][0]["datasource_name"] is None
+ assert result.structured_content["native_filters"][0]["name"] == _wrapped(
+ "Product Line"
+ )
+ assert result.structured_content["native_filters"][0]["targets"] == []
@patch("superset.daos.dashboard.DashboardDAO.find_by_id")
@@ -969,9 +998,11 @@ async def test_get_dashboard_info_restricted_user_redacts_permalink_filter_state
{"request": {"identifier": 1, "permalink_key": "abc123"}},
)
- assert result.data["permalink_key"] == "abc123"
- assert result.data["is_permalink_state"] is True
- assert result.data["filter_state"] == {"activeTabs": [_wrapped("TAB-products")]}
+ assert result.structured_content["permalink_key"] == "abc123"
+ assert result.structured_content["is_permalink_state"] is True
+ assert result.structured_content["filter_state"] == {
+ "activeTabs": [_wrapped("TAB-products")]
+ }
@patch("superset.daos.dashboard.DashboardDAO.list")
@@ -1074,8 +1105,13 @@ async def test_get_dashboard_info_includes_embedded_uuid(mock_find_object, mcp_s
result = await client.call_tool(
"get_dashboard_info", {"request": {"identifier": 1}}
)
- assert result.data["uuid"] == "94b826a5-dbd5-473d-ab58-1af676ee07e4"
- assert result.data["embedded_uuid"] == "37c56048-d3f1-452d-b3ae-0879802dcb1f"
+ assert (
+ result.structured_content["uuid"] == "94b826a5-dbd5-473d-ab58-1af676ee07e4"
+ )
+ assert (
+ result.structured_content["embedded_uuid"]
+ == "37c56048-d3f1-452d-b3ae-0879802dcb1f"
+ )
@patch("superset.mcp_service.mcp_core.ModelGetInfoCore._find_object")
@@ -1114,7 +1150,7 @@ async def test_get_dashboard_info_embedded_uuid_none_when_not_embedded(
result = await client.call_tool(
"get_dashboard_info", {"request": {"identifier": 2}}
)
- assert result.data.get("embedded_uuid") is None
+ assert result.structured_content.get("embedded_uuid") is None
# TODO (Phase 3+): Add tests for get_dashboard_available_filters tool
@@ -1157,7 +1193,9 @@ async def test_get_dashboard_info_by_uuid(mock_find_object, mcp_server):
result = await client.call_tool(
"get_dashboard_info", {"request": {"identifier": uuid_str}}
)
- assert result.data["dashboard_title"] == _wrapped("Test Dashboard UUID")
+ assert result.structured_content["dashboard_title"] == _wrapped(
+ "Test Dashboard UUID"
+ )
@patch("superset.mcp_service.mcp_core.ModelGetInfoCore._find_object")
@@ -1196,7 +1234,9 @@ async def test_get_dashboard_info_by_slug(mock_find_object, mcp_server):
result = await client.call_tool(
"get_dashboard_info", {"request": {"identifier": "test-dashboard-slug"}}
)
- assert result.data["dashboard_title"] == _wrapped("Test Dashboard Slug")
+ assert result.structured_content["dashboard_title"] == _wrapped(
+ "Test Dashboard Slug"
+ )
@patch("superset.daos.dashboard.DashboardDAO.list")
@@ -1634,9 +1674,12 @@ async def test_get_dashboard_info_direct_filter_state(mock_info, mcp_server):
"get_dashboard_info",
{"request": {"identifier": 1, "filter_state": filter_state}},
)
- assert result.data["is_permalink_state"] is False
- assert result.data["permalink_key"] is None
- assert result.data["filter_state"]["applied_filters"][0]["col"] == "gender"
+ assert result.structured_content["is_permalink_state"] is False
+ assert result.structured_content["permalink_key"] is None
+ assert (
+ result.structured_content["filter_state"]["applied_filters"][0]["col"]
+ == "gender"
+ )
@patch("superset.daos.dashboard.DashboardDAO.find_by_id")
@@ -1666,9 +1709,9 @@ async def test_direct_filter_state_redacts_data_model_metadata(mock_info, mcp_se
"get_dashboard_info",
{"request": {"identifier": 1, "filter_state": filter_state}},
)
- assert "dataMask" not in result.data["filter_state"]
- assert "chartStates" not in result.data["filter_state"]
- assert "applied_filters" in result.data["filter_state"]
+ assert "dataMask" not in result.structured_content["filter_state"]
+ assert "chartStates" not in result.structured_content["filter_state"]
+ assert "applied_filters" in result.structured_content["filter_state"]
@patch("superset.mcp_service.dashboard.permalink.get_dashboard_permalink")
@@ -1706,9 +1749,9 @@ async def test_get_dashboard_info_permalink_wins_over_filter_state(
}
},
)
- assert result.data["permalink_key"] == "permalink-1"
- assert "dataMask" in result.data["filter_state"]
- assert "applied_filters" not in result.data["filter_state"]
+ assert result.structured_content["permalink_key"] == "permalink-1"
+ assert "dataMask" in result.structured_content["filter_state"]
+ assert "applied_filters" not in result.structured_content["filter_state"]
@patch("superset.daos.dashboard.DashboardDAO.find_by_id")
@@ -1725,8 +1768,8 @@ async def test_direct_empty_filter_state_is_honored(mock_info, mcp_server):
"get_dashboard_info",
{"request": {"identifier": 1, "filter_state": {}}},
)
- assert result.data["is_permalink_state"] is False
- assert result.data["filter_state"] == {}
+ assert result.structured_content["is_permalink_state"] is False
+ assert result.structured_content["filter_state"] == {}
@patch("superset.daos.dashboard.DashboardDAO.find_by_id")
@@ -1751,4 +1794,7 @@ async def test_explicit_default_columns_excludes_filter_state(mock_info, mcp_ser
}
},
)
- assert "filter_state" not in result.data
+ assert "filter_state" not in result.structured_content
+ tool = await mcp_server.get_tool("get_dashboard_info")
+ assert tool.output_schema is not None
+ validate(instance=result.structured_content, schema=tool.output_schema)
diff --git a/tests/unit_tests/mcp_service/dashboard/tool/test_update_dashboard.py b/tests/unit_tests/mcp_service/dashboard/tool/test_update_dashboard.py
index 1b217191851..d2fd2a5ce15 100644
--- a/tests/unit_tests/mcp_service/dashboard/tool/test_update_dashboard.py
+++ b/tests/unit_tests/mcp_service/dashboard/tool/test_update_dashboard.py
@@ -146,8 +146,8 @@ class TestUpdateDashboard:
assert merged["cross_filters_enabled"] is False
assert mock_session.commit.call_count >= 1
# changed_fields enumerates what actually changed.
- # StructuredContentStripperMiddleware strips structured_content;
- # the JSON-encoded response lives in content[0].text.
+ # The JSON-encoded response is always available in content[0].text,
+ # including when compatibility mode omits structured_content.
payload = json.loads(result.content[0].text)
changed = set(payload.get("changed_fields") or [])
assert {"position_json", "json_metadata", "css"} <= changed
@@ -289,8 +289,8 @@ class TestUpdateDashboard:
# Nothing modified — dashboard fields unchanged, warning emitted
assert dash.css == original_css
assert dash.dashboard_title == original_title
- # StructuredContentStripperMiddleware strips structured_content;
- # the JSON-encoded response lives in content[0].text.
+ # The JSON-encoded response is always available in content[0].text,
+ # including when compatibility mode omits structured_content.
payload = json.loads(result.content[0].text)
warnings = payload.get("warnings") or []
assert any("No fields provided" in w for w in warnings)
diff --git a/tests/unit_tests/mcp_service/database/tool/test_database_tools.py b/tests/unit_tests/mcp_service/database/tool/test_database_tools.py
index 01b0661bb2b..26910ea98cc 100644
--- a/tests/unit_tests/mcp_service/database/tool/test_database_tools.py
+++ b/tests/unit_tests/mcp_service/database/tool/test_database_tools.py
@@ -337,7 +337,7 @@ async def test_get_database_info_not_found(mock_find, mcp_server):
result = await client.call_tool(
"get_database_info", {"request": {"identifier": 999}}
)
- assert result.data["error_type"] == "not_found"
+ assert result.structured_content["error_type"] == "not_found"
@patch("superset.daos.database.DatabaseDAO.list")
diff --git a/tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py b/tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py
index 4a70319d95e..001035ab390 100644
--- a/tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py
+++ b/tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py
@@ -25,6 +25,7 @@ import fastmcp
import pytest
from fastmcp import Client
from fastmcp.exceptions import ToolError
+from jsonschema import validate
from superset.mcp_service.app import mcp
from superset.mcp_service.dataset.schemas import (
@@ -209,7 +210,7 @@ async def test_list_datasets_basic(mock_list, mcp_server):
Note: Dataset tests use json.loads(result.content[0].text) pattern
for response parsing, which differs from dashboard/chart tests that
- use result.data directly. This is intentional based on how the
+ use result.structured_content directly. This is intentional based on how the
dataset tool responses are structured.
"""
dataset = MagicMock()
@@ -1101,7 +1102,7 @@ async def test_get_dataset_info_not_found(mock_info, mcp_server):
result = await client.call_tool(
"get_dataset_info", {"request": {"identifier": 999}}
)
- assert result.data["error_type"] == "not_found"
+ assert result.structured_content["error_type"] == "not_found"
# TODO (Phase 3+): Add tests for get_dataset_available_filters tool
@@ -1233,12 +1234,25 @@ async def test_get_dataset_info_includes_columns_and_metrics(mock_info, mcp_serv
mock_info.return_value = dataset
async with Client(mcp_server) as client:
result = await client.call_tool(
- "get_dataset_info", {"request": {"identifier": 10}}
+ "get_dataset_info",
+ {
+ "request": {
+ "identifier": 10,
+ "select_columns": [
+ "id",
+ "table_name",
+ "schema",
+ "columns",
+ "metrics",
+ ],
+ }
+ },
)
assert result.content is not None
data = json.loads(result.content[0].text)
assert data["table_name"] == "Dataset With Columns"
- assert data["database_name"] == "examples"
+ assert data["schema"] == "main"
+ assert "database_name" not in data
# Check that columns and metrics are included
assert len(data["columns"]) == 2
assert len(data["metrics"]) == 2
@@ -1247,6 +1261,10 @@ async def test_get_dataset_info_includes_columns_and_metrics(mock_info, mcp_serv
assert data["metrics"][0]["metric_name"] == "sum_sales"
assert data["metrics"][1]["metric_name"] == "count_orders"
+ tool = await mcp_server.get_tool("get_dataset_info")
+ assert tool.output_schema is not None
+ validate(instance=result.structured_content, schema=tool.output_schema)
+
@patch("superset.daos.dataset.DatasetDAO.list")
@pytest.mark.asyncio
diff --git a/tests/unit_tests/mcp_service/query/tool/test_query_tools.py b/tests/unit_tests/mcp_service/query/tool/test_query_tools.py
index 3bf370a2627..3ba72e03655 100644
--- a/tests/unit_tests/mcp_service/query/tool/test_query_tools.py
+++ b/tests/unit_tests/mcp_service/query/tool/test_query_tools.py
@@ -229,7 +229,7 @@ async def test_get_query_info_not_found(mock_find, mcp_server):
result = await client.call_tool(
"get_query_info", {"request": {"identifier": 999}}
)
- assert result.data["error_type"] == "not_found"
+ assert result.structured_content["error_type"] == "not_found"
@patch("superset.daos.query.QueryDAO.list")
diff --git a/tests/unit_tests/mcp_service/saved_query/tool/test_saved_query_tools.py b/tests/unit_tests/mcp_service/saved_query/tool/test_saved_query_tools.py
index b869f70bc7b..3930b654597 100644
--- a/tests/unit_tests/mcp_service/saved_query/tool/test_saved_query_tools.py
+++ b/tests/unit_tests/mcp_service/saved_query/tool/test_saved_query_tools.py
@@ -232,7 +232,7 @@ async def test_get_saved_query_info_not_found(mock_find, mcp_server):
result = await client.call_tool(
"get_saved_query_info", {"request": {"identifier": 999}}
)
- assert result.data["error_type"] == "not_found"
+ assert result.structured_content["error_type"] == "not_found"
@patch("superset.daos.query.SavedQueryDAO.list")
diff --git a/tests/unit_tests/mcp_service/test_main_entrypoint.py b/tests/unit_tests/mcp_service/test_main_entrypoint.py
index 9a57ad3e61f..9b51a18607b 100644
--- a/tests/unit_tests/mcp_service/test_main_entrypoint.py
+++ b/tests/unit_tests/mcp_service/test_main_entrypoint.py
@@ -107,7 +107,9 @@ def test_add_default_middlewares_installs_response_caching() -> None:
caching_middleware = object()
with (
- patch.object(main_module, "build_middleware_list", return_value=[]),
+ patch.object(
+ main_module, "build_middleware_list", return_value=[]
+ ) as mock_build_middleware_list,
patch.object(
main_module, "create_response_size_guard_middleware", return_value=None
),
@@ -118,6 +120,7 @@ def test_add_default_middlewares_installs_response_caching() -> None:
),
patch.object(main_module.mcp, "add_middleware") as mock_add_middleware,
):
- main_module._add_default_middlewares()
+ main_module._add_default_middlewares(structured_output_enabled=True)
+ mock_build_middleware_list.assert_called_once_with(structured_output_enabled=True)
mock_add_middleware.assert_called_once_with(caching_middleware)
diff --git a/tests/unit_tests/mcp_service/test_mcp_config.py b/tests/unit_tests/mcp_service/test_mcp_config.py
index cbc4a68a2f9..29cb8f0242d 100644
--- a/tests/unit_tests/mcp_service/test_mcp_config.py
+++ b/tests/unit_tests/mcp_service/test_mcp_config.py
@@ -229,6 +229,22 @@ def test_get_mcp_config_respects_app_config_override() -> None:
assert config["MCP_DISABLED_TOOLS"] == custom
+def test_get_mcp_config_structured_output_is_opt_in() -> None:
+ """Text-only results remain the default for bridge compatibility."""
+ from superset.mcp_service.mcp_config import get_mcp_config
+
+ config = get_mcp_config()
+ assert config["MCP_STRUCTURED_OUTPUT_ENABLED"] is False
+
+
+def test_get_mcp_config_respects_structured_output_override() -> None:
+ """Operators can enable outputSchema and structuredContent together."""
+ from superset.mcp_service.mcp_config import get_mcp_config
+
+ config = get_mcp_config({"MCP_STRUCTURED_OUTPUT_ENABLED": True})
+ assert config["MCP_STRUCTURED_OUTPUT_ENABLED"] is True
+
+
def test_get_mcp_config_includes_mcp_stateless_http_key() -> None:
"""get_mcp_config must include MCP_STATELESS_HTTP in its defaults dict, like
MCP_DEBUG and MCP_RBAC_ENABLED, so an operator override in superset_config.py
diff --git a/tests/unit_tests/mcp_service/test_mcp_e2e_smoke.py b/tests/unit_tests/mcp_service/test_mcp_e2e_smoke.py
index 36b466f8e4d..b30ba085940 100644
--- a/tests/unit_tests/mcp_service/test_mcp_e2e_smoke.py
+++ b/tests/unit_tests/mcp_service/test_mcp_e2e_smoke.py
@@ -23,7 +23,7 @@ never serializes a JSON-RPC message over HTTP and never runs through the
Starlette ASGI app that ``superset.mcp_service.server.run_server()`` builds
via ``mcp_instance.http_app(...)`` -- so the FastMCP-level middleware stack
(``LoggingMiddleware``, ``GlobalErrorHandlerMiddleware``,
-``StructuredContentStripperMiddleware``, etc., see
+``ToolResultCompatibilityMiddleware``, etc., see
``build_middleware_list()`` in ``server.py``) is never actually exercised in
CI.
@@ -58,6 +58,7 @@ restores that list so this file cannot leak middleware into other tests.
"""
import contextlib
+import importlib
from collections.abc import AsyncIterator, Iterator, Mapping
from typing import Any
from unittest.mock import Mock, patch
@@ -65,11 +66,12 @@ from unittest.mock import Mock, patch
import anyio
import httpx
import pytest
-from fastmcp import Client
+from fastmcp import Client, FastMCP
from fastmcp.client.transports import StreamableHttpTransport
from starlette.applications import Starlette
from superset.mcp_service.app import mcp
+from superset.mcp_service.middleware import ToolResultCompatibilityMiddleware
from superset.mcp_service.server import build_middleware_list
from superset.utils import json
@@ -141,8 +143,31 @@ async def _run_asgi_lifespan(app: Starlette) -> AsyncIterator[None]:
@contextlib.asynccontextmanager
-async def _real_asgi_client() -> AsyncIterator[Client]:
- """A FastMCP ``Client`` wired to the real ASGI app over real HTTP semantics.
+async def _asgi_client(mcp_instance: FastMCP) -> AsyncIterator[Client]:
+ """Connect a FastMCP client through its real streamable-HTTP ASGI app."""
+ asgi_app = mcp_instance.http_app(transport="streamable-http", stateless_http=False)
+
+ def httpx_client_factory(**kwargs: Any) -> httpx.AsyncClient:
+ return httpx.AsyncClient(
+ transport=httpx.ASGITransport(app=asgi_app),
+ base_url="http://testserver",
+ **kwargs,
+ )
+
+ transport = StreamableHttpTransport(
+ "http://testserver/mcp", httpx_client_factory=httpx_client_factory
+ )
+
+ async with _run_asgi_lifespan(asgi_app):
+ async with Client(transport) as client:
+ yield client
+
+
+@contextlib.asynccontextmanager
+async def _real_asgi_client(
+ *, structured_output_enabled: bool = True
+) -> AsyncIterator[Client]:
+ """Wire the shared Superset MCP instance through real HTTP semantics.
Builds the app the way ``run_server()`` does for the multi-pod/http_app
path (``server.py:938``): FastMCP-level middleware from
@@ -167,26 +192,14 @@ async def _real_asgi_client() -> AsyncIterator[Client]:
task sidesteps that entirely.
"""
original_middleware = list(mcp.middleware)
- for middleware in build_middleware_list():
+ for middleware in build_middleware_list(
+ structured_output_enabled=structured_output_enabled
+ ):
mcp.add_middleware(middleware)
try:
- asgi_app = mcp.http_app(transport="streamable-http", stateless_http=False)
-
- def httpx_client_factory(**kwargs: Any) -> httpx.AsyncClient:
- return httpx.AsyncClient(
- transport=httpx.ASGITransport(app=asgi_app),
- base_url="http://testserver",
- **kwargs,
- )
-
- transport = StreamableHttpTransport(
- "http://testserver/mcp", httpx_client_factory=httpx_client_factory
- )
-
- async with _run_asgi_lifespan(asgi_app):
- async with Client(transport) as client:
- yield client
+ async with _asgi_client(mcp) as client:
+ yield client
finally:
# Restore the shared FastMCP singleton's middleware list in place
# (not by reassignment) so this file cannot leak state into other
@@ -201,7 +214,7 @@ async def test_tools_list_over_real_asgi_transport() -> None:
This alone proves the full stack boots: the Starlette app built by
``http_app()``, the FastMCP-level middleware chain (Logging,
- GlobalErrorHandler, StructuredContentStripper, RBAC visibility), the
+ GlobalErrorHandler, ToolResultCompatibility, RBAC visibility), the
streamable-HTTP session manager, and real JSON-RPC (de)serialization --
none of which the in-process ``Client(mcp)`` tests elsewhere in this
package exercise.
@@ -224,6 +237,7 @@ async def test_tools_list_over_real_asgi_transport() -> None:
and tool.annotations.readOnlyHint is False
and tool.annotations.idempotentHint is not False
}
+ assert not {tool.name for tool in tools if tool.outputSchema is None}
@pytest.mark.asyncio
@@ -238,12 +252,71 @@ async def test_tools_call_health_check_over_real_asgi_transport() -> None:
request arrived (in-process client vs. real HTTP transport), so the same
mock works unmodified here.
"""
- async with _real_asgi_client() as client:
- result = await client.call_tool("health_check", {})
+ health_module = importlib.import_module(
+ "superset.mcp_service.system.tool.health_check"
+ )
+ with patch.object(
+ health_module,
+ "get_version_metadata",
+ return_value={"version_string": "test-version"},
+ ):
+ async with _real_asgi_client() as client:
+ result = await client.call_tool("health_check", {})
data = json.loads(result.content[0].text)
assert data["status"] == "healthy"
assert data["service"] == "Superset MCP Service"
+ assert result.structured_content == data
+
+
+@pytest.mark.asyncio
+async def test_structured_output_can_be_disabled_over_real_asgi_transport() -> None:
+ """Compatibility mode removes both halves of the structured contract."""
+ health_module = importlib.import_module(
+ "superset.mcp_service.system.tool.health_check"
+ )
+ with patch.object(
+ health_module,
+ "get_version_metadata",
+ return_value={"version_string": "test-version"},
+ ):
+ async with _real_asgi_client(structured_output_enabled=False) as client:
+ tools = await client.list_tools()
+ result = await client.call_tool("health_check", {})
+
+ assert all(tool.outputSchema is None for tool in tools)
+ assert result.structured_content is None
+ assert json.loads(result.content[0].text)["status"] == "healthy"
+
+
+@pytest.mark.asyncio
+async def test_compatibility_middleware_is_self_contained_over_real_asgi() -> None:
+ """The compatibility boundary alone avoids output validation failures."""
+ factory_mcp = FastMCP(
+ "factory-test",
+ middleware=[ToolResultCompatibilityMiddleware()],
+ )
+ health_tool = await mcp.get_tool("health_check")
+ assert health_tool is not None
+ assert health_tool.output_schema is not None
+ factory_mcp.add_tool(health_tool)
+
+ health_module = importlib.import_module(
+ "superset.mcp_service.system.tool.health_check"
+ )
+ with patch.object(
+ health_module,
+ "get_version_metadata",
+ return_value={"version_string": "test-version"},
+ ):
+ async with _asgi_client(factory_mcp) as client:
+ result = await client.call_tool("health_check", {}, raise_on_error=False)
+ tools = await client.list_tools()
+
+ assert result.is_error is False
+ assert result.structured_content is None
+ assert json.loads(result.content[0].text)["status"] == "healthy"
+ assert tools[0].outputSchema is None
@pytest.mark.asyncio
@@ -252,7 +325,7 @@ async def test_tools_call_failure_sets_is_error_over_real_asgi_transport() -> No
An unknown tool name raises inside the middleware chain, so this exercises
the real path a permission denial takes: the exception propagates to the
- outermost ``StructuredContentStripperMiddleware`` catch-all, whose result
+ outermost ``ToolResultCompatibilityMiddleware`` catch-all, whose result
then crosses the real JSON-RPC wire. Isolated middleware tests with a mocked
``call_next`` cannot prove this -- inner middlewares rebuild the result and
can drop the flag -- which is why this runs over the full chain.
diff --git a/tests/unit_tests/mcp_service/test_mcp_server.py b/tests/unit_tests/mcp_service/test_mcp_server.py
index c899c0216d2..aab6b20594c 100644
--- a/tests/unit_tests/mcp_service/test_mcp_server.py
+++ b/tests/unit_tests/mcp_service/test_mcp_server.py
@@ -435,14 +435,13 @@ def test_create_auth_provider_fails_closed_on_custom_factory_error() -> None:
@contextlib.contextmanager
def _run_server_dependencies(
flask_config: dict[str, Any],
-) -> Iterator[MagicMock]:
- """Patch every ``run_server()`` collaborator except stateless_http resolution.
-
- Returns the ``mcp_instance`` mock so callers can assert on the kwargs its
- ``run()`` was called with -- everything else (auth, middleware, event
- store, health endpoint) is stubbed out since this is only exercising the
- ``flask_app.config.get("MCP_STATELESS_HTTP", ...)`` wiring, not those
- other startup steps.
+) -> Iterator[tuple[MagicMock, MagicMock]]:
+ """Patch ``run_server()`` collaborators while retaining config resolution.
+
+ Returns the MCP instance and middleware-builder mocks so callers can assert
+ that resolved Flask settings reach their runtime consumers. Auth, event
+ storage, and health collaborators are stubbed because these tests exercise
+ startup configuration wiring only.
"""
from superset.mcp_service import server
@@ -458,7 +457,9 @@ def _run_server_dependencies(
return_value=flask_app,
),
patch.object(server, "_create_auth_provider", return_value=None),
- patch.object(server, "build_middleware_list", return_value=[]),
+ patch.object(
+ server, "build_middleware_list", return_value=[]
+ ) as mock_build_middleware_list,
patch.object(
server, "create_response_size_guard_middleware", return_value=None
),
@@ -471,7 +472,7 @@ def _run_server_dependencies(
patch.object(server, "create_event_store", return_value=None),
patch.object(server, "_build_starlette_middleware", return_value=[]),
):
- yield mcp_instance
+ yield mcp_instance, mock_build_middleware_list
def test_run_server_defaults_stateless_http_to_true_when_unset() -> None:
@@ -490,7 +491,10 @@ def test_run_server_defaults_stateless_http_to_true_when_unset() -> None:
port = 59901
os.environ.pop(f"FASTMCP_RUNNING_{port}", None)
try:
- with _run_server_dependencies(flask_config={}) as mcp_instance:
+ with _run_server_dependencies(flask_config={}) as (
+ mcp_instance,
+ _mock_build_middleware_list,
+ ):
run_server(host="127.0.0.1", port=port)
mcp_instance.run.assert_called_once()
@@ -508,12 +512,86 @@ def test_run_server_respects_mcp_stateless_http_false_override() -> None:
port = 59902
os.environ.pop(f"FASTMCP_RUNNING_{port}", None)
try:
- with _run_server_dependencies(
- flask_config={"MCP_STATELESS_HTTP": False}
- ) as mcp_instance:
+ with _run_server_dependencies(flask_config={"MCP_STATELESS_HTTP": False}) as (
+ mcp_instance,
+ _mock_build_middleware_list,
+ ):
run_server(host="127.0.0.1", port=port)
mcp_instance.run.assert_called_once()
assert mcp_instance.run.call_args.kwargs["stateless_http"] is False
finally:
os.environ.pop(f"FASTMCP_RUNNING_{port}", None)
+
+
+def test_run_server_respects_structured_output_override() -> None:
+ """The Flask setting reaches the outer compatibility middleware."""
+ from superset.mcp_service.server import run_server
+
+ port = 59903
+ os.environ.pop(f"FASTMCP_RUNNING_{port}", None)
+ try:
+ with _run_server_dependencies(
+ flask_config={"MCP_STRUCTURED_OUTPUT_ENABLED": True}
+ ) as (_mcp_instance, mock_build_middleware_list):
+ run_server(host="127.0.0.1", port=port)
+
+ mock_build_middleware_list.assert_called_once_with(
+ structured_output_enabled=True
+ )
+ finally:
+ os.environ.pop(f"FASTMCP_RUNNING_{port}", None)
+
+
+@pytest.mark.parametrize("enabled", [False, True])
+def test_factory_server_respects_structured_output_setting(enabled: bool) -> None:
+ """Factory servers prepend the configured core stack to custom middleware."""
+ from superset.mcp_service.server import run_server
+
+ port = 59904
+ os.environ.pop(f"FASTMCP_RUNNING_{port}", None)
+ flask_app = MagicMock()
+ flask_app.config = {"MCP_STRUCTURED_OUTPUT_ENABLED": enabled}
+ mcp_instance = MagicMock()
+ core_middleware = [MagicMock(), MagicMock()]
+ custom_middleware = MagicMock()
+ factory_config = {"auth": None, "middleware": [custom_middleware]}
+
+ try:
+ with (
+ patch("superset.mcp_service.server.configure_logging"),
+ patch("superset.mcp_service.server._suppress_third_party_warnings"),
+ patch(
+ "superset.mcp_service.server.get_mcp_factory_config",
+ return_value=factory_config,
+ ),
+ patch(
+ "superset.mcp_service.flask_singleton.get_flask_app",
+ return_value=flask_app,
+ ),
+ patch(
+ "superset.mcp_service.server.create_mcp_app",
+ return_value=mcp_instance,
+ ) as mock_create_mcp_app,
+ patch(
+ "superset.mcp_service.server.build_middleware_list",
+ return_value=core_middleware,
+ ) as mock_build_middleware_list,
+ patch("superset.mcp_service.session_scope.install_mcp_session_scoping"),
+ patch("superset.mcp_service.server._apply_tool_search_transform"),
+ patch("superset.mcp_service.server._register_health_endpoint"),
+ patch("superset.mcp_service.server.create_event_store", return_value=None),
+ patch(
+ "superset.mcp_service.server._build_starlette_middleware",
+ return_value=[],
+ ),
+ ):
+ run_server(host="127.0.0.1", port=port, use_factory_config=True)
+
+ mock_build_middleware_list.assert_called_once_with(
+ structured_output_enabled=enabled
+ )
+ middleware = mock_create_mcp_app.call_args.kwargs["middleware"]
+ assert middleware == [*core_middleware, custom_middleware]
+ finally:
+ os.environ.pop(f"FASTMCP_RUNNING_{port}", None)
diff --git a/tests/unit_tests/mcp_service/test_mcp_tool_registration.py b/tests/unit_tests/mcp_service/test_mcp_tool_registration.py
index 46db53a8e13..4de4ad48d33 100644
--- a/tests/unit_tests/mcp_service/test_mcp_tool_registration.py
+++ b/tests/unit_tests/mcp_service/test_mcp_tool_registration.py
@@ -19,6 +19,7 @@
import asyncio
import logging
+from typing import Any
from unittest.mock import MagicMock, patch
import pytest
@@ -29,6 +30,192 @@ from superset.mcp_service.app import get_default_instructions, init_fastmcp_serv
_FFM_PATH = "superset.extensions.feature_flag_manager"
+# One response-specific field per native tool. Keeping the complete inventory
+# here makes every new tool go through the metadata review below, while the
+# field assertion rejects missing or fully unconstrained output schemas.
+EXPECTED_TOOL_OUTPUT_FIELDS = {
+ "add_chart_to_existing_dashboard": "dashboard_url",
+ "create_dataset": "table_name",
+ "create_theme": "theme_name",
+ "create_virtual_dataset": "dataset_name",
+ "delete_chart": "deleted_id",
+ "delete_dashboard": "deleted_id",
+ "duplicate_dashboard": "duplicated_slices",
+ "execute_sql": "statements",
+ "find_users": "users",
+ "generate_bug_report": "report",
+ "generate_chart": "chart",
+ "generate_dashboard": "dashboard",
+ "generate_explore_link": "form_data_key",
+ "get_annotation_layer_info": "descr",
+ "get_chart_data": "query_results",
+ "get_chart_info": "slice_name",
+ "get_chart_preview": "explore_url",
+ "get_chart_sql": "sql",
+ "get_chart_type_schema": "chart_type",
+ "get_compatible_dimensions": "compatible_dimensions",
+ "get_compatible_metrics": "compatible_metrics",
+ "get_dashboard_data": "charts_returned",
+ "get_dashboard_datasets": "datasets",
+ "get_dashboard_info": "dashboard_title",
+ "get_dashboard_layout": "tabs",
+ "get_database_info": "database_name",
+ "get_dataset_info": "table_name",
+ "get_instance_info": "instance_summary",
+ "get_layer_annotation_info": "short_descr",
+ "get_query_info": "executed_sql",
+ "get_report_info": "crontab",
+ "get_rls_filter_info": "clause",
+ "get_role_info": "permissions",
+ "get_saved_query_info": "label",
+ "get_schema": "schema_info",
+ "get_table": "source",
+ "get_tag_info": "name",
+ "get_task_info": "task_type",
+ "get_theme_info": "theme_name",
+ "get_user_info": "username",
+ "health_check": "status",
+ "list_annotation_layers": "annotation_layers",
+ "list_charts": "charts",
+ "list_dashboards": "dashboards",
+ "list_databases": "databases",
+ "list_datasets": "datasets",
+ "list_layer_annotations": "annotations",
+ "list_metrics": "metrics",
+ "list_queries": "queries",
+ "list_reports": "reports",
+ "list_rls_filters": "rls_filters",
+ "list_roles": "roles",
+ "list_saved_queries": "saved_queries",
+ "list_tags": "tags",
+ "list_tasks": "tasks",
+ "list_themes": "themes",
+ "list_users": "users",
+ "manage_dashboard_certification": "changed_fields",
+ "manage_dashboard_owners": "owners",
+ "manage_dashboard_roles": "roles",
+ "manage_native_filters": "filters",
+ "open_sql_lab_with_context": "title",
+ "query_dataset": "applied_filters",
+ "remove_chart_from_dashboard": "removed_layout_keys",
+ "restore_chart": "restored_id",
+ "restore_dashboard": "restored_id",
+ "save_sql_query": "label",
+ "update_chart": "chart",
+ "update_chart_preview": "previous_form_data_key",
+ "update_dashboard": "changed_fields",
+ "update_dataset_metric": "updated_properties",
+}
+
+# Representative full top-level contracts guard against partial schema drift,
+# including fields whose names overlap JSON Schema annotations.
+EXPECTED_TOP_LEVEL_OUTPUT_FIELDS = {
+ "get_chart_info": {"id", "slice_name", "viz_type", "form_data"},
+ "get_dashboard_info": {"dashboard_title", "charts", "filter_state"},
+ "get_dataset_info": {"table_name", "schema", "columns", "metrics"},
+ "open_sql_lab_with_context": {"url", "title", "database_id"},
+ "update_chart_preview": {"chart", "error", "form_data", "success"},
+}
+
+MUTATING_TOOLS = {
+ "add_chart_to_existing_dashboard",
+ "create_dataset",
+ "create_theme",
+ "create_virtual_dataset",
+ "delete_chart",
+ "delete_dashboard",
+ "duplicate_dashboard",
+ "execute_sql",
+ "generate_chart",
+ "generate_dashboard",
+ "generate_explore_link",
+ "manage_dashboard_certification",
+ "manage_dashboard_owners",
+ "manage_dashboard_roles",
+ "manage_native_filters",
+ "remove_chart_from_dashboard",
+ "restore_chart",
+ "restore_dashboard",
+ "save_sql_query",
+ "update_chart",
+ "update_chart_preview",
+ "update_dashboard",
+ "update_dataset_metric",
+}
+
+DESTRUCTIVE_TOOLS = {
+ "delete_chart",
+ "delete_dashboard",
+ "execute_sql",
+ "manage_dashboard_owners",
+ "manage_dashboard_roles",
+ "manage_native_filters",
+ "remove_chart_from_dashboard",
+ "update_chart",
+ "update_dashboard",
+ "update_dataset_metric",
+}
+
+
+def _schema_property_names(value: Any) -> set[str]:
+ """Collect declared property names from a nested JSON Schema."""
+ if isinstance(value, list):
+ names: set[str] = set()
+ for item in value:
+ names.update(_schema_property_names(item))
+ return names
+ if not isinstance(value, dict):
+ return set()
+
+ names = set(value.get("properties", {}))
+ for item in value.values():
+ names.update(_schema_property_names(item))
+ return names
+
+
+def _top_level_schema_property_names(
+ schema: dict[str, Any], root_schema: dict[str, Any] | None = None
+) -> set[str]:
+ """Collect properties at the root or its composed response variants."""
+ root_schema = root_schema or schema
+ if reference := schema.get("$ref"):
+ if isinstance(reference, str) and reference.startswith("#/$defs/"):
+ definition = root_schema.get("$defs", {}).get(
+ reference.removeprefix("#/$defs/"), {}
+ )
+ if isinstance(definition, dict):
+ return _top_level_schema_property_names(definition, root_schema)
+
+ names = set(schema.get("properties", {}))
+ for keyword in ("anyOf", "oneOf", "allOf"):
+ alternatives = schema.get(keyword, [])
+ if isinstance(alternatives, list):
+ for alternative in alternatives:
+ if isinstance(alternative, dict):
+ names.update(
+ _top_level_schema_property_names(alternative, root_schema)
+ )
+ return names
+
+
+def test_schema_title_annotations_do_not_remove_title_fields() -> None:
+ """Generated titles are noise, but a response field named title is data."""
+ from superset.core.mcp.core_mcp_injection import _strip_schema_titles
+
+ schema = {
+ "title": "Response",
+ "type": "object",
+ "properties": {
+ "title": {"title": "Title", "type": "string"},
+ },
+ }
+
+ assert _strip_schema_titles(schema) == {
+ "type": "object",
+ "properties": {"title": {"type": "string"}},
+ }
+
+
@pytest.fixture(autouse=True)
def gtf_ffm():
"""Default for this module: GLOBAL_TASK_FRAMEWORK is enabled.
@@ -57,8 +244,8 @@ def test_mcp_app_imports_successfully():
assert "list_charts" in tool_names
-def test_all_registered_tools_have_complete_annotations():
- """Every advertised tool declares all MCP safety annotations."""
+def test_all_registered_tools_have_complete_metadata(): # noqa: C901
+ """Every native tool declares reviewed safety hints and useful output."""
required_fields = (
"title",
"readOnlyHint",
@@ -66,10 +253,17 @@ def test_all_registered_tools_have_complete_annotations():
"openWorldHint",
)
missing_by_tool = {}
+ missing_output_fields = {}
+ missing_top_level_output_fields = {}
open_world_tools = []
idempotent_mutating_tools = []
+ incorrect_read_only_tools = []
+ incorrect_destructive_tools = []
+
+ registered_tools = _run(mcp.list_tools())
+ assert {tool.name for tool in registered_tools} == set(EXPECTED_TOOL_OUTPUT_FIELDS)
- for registered_tool in _run(mcp.list_tools()):
+ for registered_tool in registered_tools:
annotations = registered_tool.annotations
missing = [
field
@@ -81,11 +275,38 @@ def test_all_registered_tools_have_complete_annotations():
missing.append("idempotentHint")
elif annotations.idempotentHint is not False:
idempotent_mutating_tools.append(registered_tool.name)
+ if annotations is not None:
+ if annotations.readOnlyHint is not (
+ registered_tool.name not in MUTATING_TOOLS
+ ):
+ incorrect_read_only_tools.append(registered_tool.name)
+ if annotations.destructiveHint is not (
+ registered_tool.name in DESTRUCTIVE_TOOLS
+ ):
+ incorrect_destructive_tools.append(registered_tool.name)
if missing:
missing_by_tool[registered_tool.name] = missing
elif annotations.openWorldHint is not False:
open_world_tools.append(registered_tool.name)
+ output_schema = registered_tool.output_schema
+ expected_field = EXPECTED_TOOL_OUTPUT_FIELDS[registered_tool.name]
+ if output_schema is None or expected_field not in _schema_property_names(
+ output_schema
+ ):
+ missing_output_fields[registered_tool.name] = expected_field
+ if registered_tool.name in EXPECTED_TOP_LEVEL_OUTPUT_FIELDS:
+ actual_top_level_fields = _top_level_schema_property_names(
+ output_schema or {}
+ )
+ expected_top_level_fields = EXPECTED_TOP_LEVEL_OUTPUT_FIELDS[
+ registered_tool.name
+ ]
+ if missing_fields := expected_top_level_fields - actual_top_level_fields:
+ missing_top_level_output_fields[registered_tool.name] = sorted(
+ missing_fields
+ )
+
assert not missing_by_tool, (
f"Registered MCP tools have incomplete annotations: {missing_by_tool}"
)
@@ -97,6 +318,22 @@ def test_all_registered_tools_have_complete_annotations():
"Mutating tools must not claim idempotency (idempotentHint=False): "
f"{idempotent_mutating_tools}"
)
+ assert not incorrect_read_only_tools, (
+ f"Tools have incorrect readOnlyHint classifications: "
+ f"{incorrect_read_only_tools}"
+ )
+ assert not incorrect_destructive_tools, (
+ f"Tools have incorrect destructiveHint classifications: "
+ f"{incorrect_destructive_tools}"
+ )
+ assert not missing_output_fields, (
+ "Registered MCP tools have missing or unconstrained output schemas: "
+ f"{missing_output_fields}"
+ )
+ assert not missing_top_level_output_fields, (
+ "Representative MCP output schemas lost top-level fields: "
+ f"{missing_top_level_output_fields}"
+ )
def test_mcp_prompts_registered():
diff --git a/tests/unit_tests/mcp_service/test_middleware.py b/tests/unit_tests/mcp_service/test_middleware.py
index bc0a8a781ec..2921c5687f3 100644
--- a/tests/unit_tests/mcp_service/test_middleware.py
+++ b/tests/unit_tests/mcp_service/test_middleware.py
@@ -45,6 +45,7 @@ from superset.mcp_service.middleware import (
RBACToolVisibilityMiddleware,
ResponseSizeGuardMiddleware,
StructuredContentStripperMiddleware,
+ ToolResultCompatibilityMiddleware,
)
from superset.mcp_service.utils.token_utils import estimate_token_count
from superset.utils import json as utils_json
@@ -2072,16 +2073,16 @@ class TestGlobalErrorHandlerErrorIdUsesCallId:
_mcp_call_id_var.reset(token)
-class TestStructuredContentStripperErrorHook:
+class TestToolResultCompatibilityErrorHook:
"""Test the last-resort MCP_ERROR_HOOK capture point in
- StructuredContentStripperMiddleware.on_call_tool's except block."""
+ ToolResultCompatibilityMiddleware.on_call_tool's except block."""
@pytest.mark.asyncio
async def test_invokes_hook_for_exception_bypassing_error_handler(self) -> None:
"""A non-ToolError exception reaching this final catch means it
slipped past GlobalErrorHandlerMiddleware entirely — invoke the
hook here as the true last-resort capture point."""
- middleware = StructuredContentStripperMiddleware()
+ middleware = ToolResultCompatibilityMiddleware()
context = MagicMock()
context.message.name = "list_charts"
call_next = AsyncMock(side_effect=RuntimeError("boom"))
@@ -2119,7 +2120,7 @@ class TestStructuredContentStripperErrorHook:
"""ToolError has already been classified and hooked by
GlobalErrorHandlerMiddleware — avoid double-reporting the same
failure to the error tracker."""
- middleware = StructuredContentStripperMiddleware()
+ middleware = ToolResultCompatibilityMiddleware()
context = MagicMock()
context.message.name = "list_charts"
call_next = AsyncMock(side_effect=ToolError("already handled"))
@@ -2147,7 +2148,7 @@ class TestStructuredContentStripperErrorHook:
def __str__(self) -> str:
raise RuntimeError("hostile __str__")
- middleware = StructuredContentStripperMiddleware()
+ middleware = ToolResultCompatibilityMiddleware()
context = MagicMock()
context.message.name = "list_charts"
call_next = AsyncMock(side_effect=HostileStrError())
@@ -2167,7 +2168,7 @@ class TestStructuredContentStripperErrorHook:
"""An exception bypassing GlobalErrorHandlerMiddleware must not
leak raw internals to the client — the last-resort response text
goes through the same sanitizer as every other error path."""
- middleware = StructuredContentStripperMiddleware()
+ middleware = ToolResultCompatibilityMiddleware()
context = MagicMock()
context.message.name = "execute_sql"
# Connection string with embedded credentials — must be redacted.
@@ -2192,8 +2193,8 @@ class TestStructuredContentStripperErrorHook:
assert "[REDACTED]" in text
-class TestStructuredContentStripperIsErrorFlag:
- """Failures caught by StructuredContentStripperMiddleware must still be
+class TestToolResultCompatibilityIsErrorFlag:
+ """Failures caught by ToolResultCompatibilityMiddleware must still be
reported as errors on the wire — a client that only inspects isError
would otherwise read a denial or a crash as a successful call."""
@@ -2201,7 +2202,7 @@ class TestStructuredContentStripperIsErrorFlag:
async def test_tool_error_is_flagged_as_error(self) -> None:
"""A permission denial surfaces as ToolError; it must not come back
looking like a successful tool call."""
- middleware = StructuredContentStripperMiddleware()
+ middleware = ToolResultCompatibilityMiddleware()
context = MagicMock()
context.message.name = "save_sql_query"
call_next = AsyncMock(
@@ -2223,7 +2224,7 @@ class TestStructuredContentStripperIsErrorFlag:
async def test_unexpected_exception_is_flagged_as_error(self) -> None:
"""The same holds for exceptions that bypass
GlobalErrorHandlerMiddleware and reach the last-resort catch."""
- middleware = StructuredContentStripperMiddleware()
+ middleware = ToolResultCompatibilityMiddleware()
context = MagicMock()
context.message.name = "list_charts"
call_next = AsyncMock(side_effect=RuntimeError("boom"))
@@ -2240,18 +2241,113 @@ class TestStructuredContentStripperIsErrorFlag:
@pytest.mark.asyncio
async def test_successful_result_is_not_flagged(self) -> None:
- """The success path must stay untouched."""
+ """The success path, including structured output, stays untouched."""
from fastmcp.tools.tool import ToolResult
from mcp.types import TextContent
- middleware = StructuredContentStripperMiddleware()
+ middleware = ToolResultCompatibilityMiddleware(structured_output_enabled=True)
context = MagicMock()
context.message.name = "list_charts"
call_next = AsyncMock(
- return_value=ToolResult(content=[TextContent(type="text", text="ok")])
+ return_value=ToolResult(
+ content=[TextContent(type="text", text="ok")],
+ structured_content={"status": "ok"},
+ )
)
result = await middleware.on_call_tool(context, call_next)
assert result.is_error is False
assert result.content[0].text == "ok"
+ assert result.structured_content == {"status": "ok"}
+
+ @pytest.mark.asyncio
+ async def test_default_strips_structured_content(self) -> None:
+ """The default preserves the legacy text-only bridge contract."""
+ from fastmcp.tools.tool import ToolResult
+ from mcp.types import TextContent
+
+ middleware = ToolResultCompatibilityMiddleware()
+ context = MagicMock()
+ context.message.name = "list_charts"
+ call_next = AsyncMock(
+ return_value=ToolResult(
+ content=[TextContent(type="text", text='{"status":"ok"}')],
+ structured_content={"status": "ok"},
+ )
+ )
+
+ result = await middleware.on_call_tool(context, call_next)
+
+ assert result.content[0].text == '{"status":"ok"}'
+ assert result.structured_content is None
+ assert result.meta == {}
+
+ @pytest.mark.asyncio
+ async def test_task_protocol_result_passes_through_unchanged(self) -> None:
+ """Compatibility mode must not treat task results as tool results."""
+ from datetime import datetime, timezone
+
+ from mcp.types import CreateTaskResult, Task
+
+ now = datetime.now(timezone.utc)
+ task_result = CreateTaskResult(
+ task=Task(
+ taskId="task-1",
+ status="working",
+ createdAt=now,
+ lastUpdatedAt=now,
+ ttl=None,
+ )
+ )
+
+ result = await ToolResultCompatibilityMiddleware().on_call_tool(
+ MagicMock(), AsyncMock(return_value=task_result)
+ )
+
+ assert result is task_result
+
+ @pytest.mark.asyncio
+ @pytest.mark.parametrize("enabled", [False, True])
+ async def test_output_schema_follows_compatibility_setting(
+ self, enabled: bool
+ ) -> None:
+ """Discovery and call results use the same structured-output setting."""
+ from fastmcp.tools import Tool
+
+ def sample_tool() -> dict[str, str]:
+ """Return a structured test result."""
+ return {"status": "ok"}
+
+ tool = Tool.from_function(sample_tool)
+ assert tool.output_schema is not None
+ middleware = ToolResultCompatibilityMiddleware(
+ structured_output_enabled=enabled
+ )
+
+ result = await middleware.on_list_tools(
+ MagicMock(), AsyncMock(return_value=[tool])
+ )
+
+ assert (result[0].output_schema is not None) is enabled
+
+ @pytest.mark.asyncio
+ async def test_deprecated_stripper_warns_and_still_strips(self) -> None:
+ """The deprecated import name must not silently reverse behavior."""
+ from fastmcp.tools.tool import ToolResult
+ from mcp.types import TextContent
+
+ with pytest.warns(DeprecationWarning, match="is deprecated"):
+ middleware = StructuredContentStripperMiddleware()
+
+ result = await middleware.on_call_tool(
+ MagicMock(),
+ AsyncMock(
+ return_value=ToolResult(
+ content=[TextContent(type="text", text="ok")],
+ structured_content={"status": "ok"},
+ )
+ ),
+ )
+
+ assert result.structured_content is None
diff --git a/tests/unit_tests/mcp_service/test_middleware_logging.py b/tests/unit_tests/mcp_service/test_middleware_logging.py
index 5b8af9a481f..b77993d7ca3 100644
--- a/tests/unit_tests/mcp_service/test_middleware_logging.py
+++ b/tests/unit_tests/mcp_service/test_middleware_logging.py
@@ -121,7 +121,7 @@ class TestLoggingMiddlewareOnCallTool:
This simulates the real middleware chain: GlobalErrorHandler catches
tool exceptions and re-raises them as ToolError. Since LoggingMiddleware
- sits between GlobalErrorHandler and StructuredContentStripper, it
+ sits between GlobalErrorHandler and ToolResultCompatibility, it
catches the ToolError directly.
"""
middleware = LoggingMiddleware()
@@ -750,7 +750,7 @@ class TestExtractOutputIds:
class TestMiddlewareChainOrder:
"""Test that the middleware order from server.py logs failures correctly.
- If the order is wrong (StructuredContentStripper innermost),
+ If the order is wrong (ToolResultCompatibility innermost),
it swallows exceptions before LoggingMiddleware can see them,
causing success=True for failures.
"""
@@ -778,7 +778,7 @@ class TestMiddlewareChainOrder:
ctx = _make_context(name="get_chart_info")
result = await chain(ctx)
- # StructuredContentStripper (outermost) must catch the re-raised
+ # ToolResultCompatibility (outermost) must catch the re-raised
# exception and convert it to a safe ToolResult with "Error:" text.
# If it's not outermost, the exception would leak to the MCP SDK.
assert isinstance(result, ToolResult)
@@ -786,7 +786,7 @@ class TestMiddlewareChainOrder:
# LoggingMiddleware must log
# success=False. If the middleware order is wrong
- # (StructuredContentStripper innermost), this would be
+ # (ToolResultCompatibility innermost), this would be
# success=True because the exception gets swallowed
# before LoggingMiddleware sees it.
log_calls = [
@@ -796,9 +796,9 @@ class TestMiddlewareChainOrder:
]
assert len(log_calls) == 1
assert log_calls[0][1]["curated_payload"]["success"] is False, (
- "Middleware order is wrong: StructuredContentStripper is "
+ "Middleware order is wrong: ToolResultCompatibility is "
"swallowing exceptions before LoggingMiddleware can detect "
- "them. Ensure StructuredContentStripper is outermost "
+ "them. Ensure ToolResultCompatibility is outermost "
"(first added) in build_middleware_list()."
)
@@ -809,7 +809,7 @@ class TestMiddlewareChainOrder:
self, mock_get_user_id, mock_event_logger
) -> None:
"""When a tool raises, the error ToolResult from
- StructuredContentStripper still carries mcp_call_id in meta."""
+ ToolResultCompatibility still carries mcp_call_id in meta."""
from superset.mcp_service.server import build_middleware_list
middleware_list = build_middleware_list()
@@ -836,7 +836,7 @@ class TestMiddlewareChainOrder:
ToolError raised by GlobalErrorHandlerMiddleware cannot be encoded
by the MCP SDK in a tools/list response, producing "encoding without
- a string argument". StructuredContentStripperMiddleware.on_list_tools
+ a string argument". ToolResultCompatibilityMiddleware.on_list_tools
must catch it and return an empty list.
"""
from superset.mcp_service.server import build_middleware_list