You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/06/30 17:36:38 UTC
(superset) branch master updated: feat(mcp): add tags + typed metadata fields to update_dashboard (#40957)
This is an automated email from the ASF dual-hosted git repository.
rusackas 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 c60d8bb656f feat(mcp): add tags + typed metadata fields to update_dashboard (#40957)
c60d8bb656f is described below
commit c60d8bb656f988927f177a57d5e1e470de46ec99
Author: Amin Ghadersohi <am...@gmail.com>
AuthorDate: Tue Jun 30 13:36:31 2026 -0400
feat(mcp): add tags + typed metadata fields to update_dashboard (#40957)
---
superset/mcp_service/dashboard/schemas.py | 57 ++++++
.../mcp_service/dashboard/tool/update_dashboard.py | 148 ++++++++++++---
.../dashboard/tool/test_update_dashboard.py | 198 +++++++++++++++++++--
3 files changed, 373 insertions(+), 30 deletions(-)
diff --git a/superset/mcp_service/dashboard/schemas.py b/superset/mcp_service/dashboard/schemas.py
index f550e86a55f..8a1fe83b3a5 100644
--- a/superset/mcp_service/dashboard/schemas.py
+++ b/superset/mcp_service/dashboard/schemas.py
@@ -66,6 +66,7 @@ Example usage:
from __future__ import annotations
import logging
+import re
from datetime import datetime
from typing import Annotated, Any, cast, Dict, List, Literal, TYPE_CHECKING
@@ -809,6 +810,36 @@ class UpdateDashboardRequest(BaseModel):
"Optional new dashboard CSS. Pass empty string to clear existing CSS."
),
)
+ tags: List[int] | None = Field(
+ None,
+ description=(
+ "Optional FULL-REPLACEMENT list of tag IDs to associate with the "
+ "dashboard. Discover IDs with ``list_tags``. An empty list clears "
+ "all custom tags. Omit (None) to leave tags unchanged."
+ ),
+ )
+ cross_filters_enabled: bool | None = Field(
+ None,
+ description=(
+ "Optional toggle for dashboard-wide cross filtering. Typed "
+ "convenience for the ``cross_filters_enabled`` json_metadata key."
+ ),
+ )
+ refresh_frequency: int | None = Field(
+ None,
+ ge=0,
+ description=(
+ "Optional auto-refresh interval in seconds (0 = off). Typed "
+ "convenience for the ``refresh_frequency`` json_metadata key."
+ ),
+ )
+ filter_bar_orientation: Literal["VERTICAL", "HORIZONTAL"] | None = Field(
+ None,
+ description=(
+ "Optional native filter bar orientation. Typed convenience for "
+ "the ``filter_bar_orientation`` json_metadata key."
+ ),
+ )
sanitization_warnings: List[str] = Field(
default_factory=list,
description=(
@@ -871,6 +902,32 @@ class UpdateDashboardRequest(BaseModel):
v, "Dashboard title", max_length=500, allow_empty=True
)
+ @field_validator("slug")
+ @classmethod
+ def normalize_slug(cls, v: str | None) -> str | None:
+ """Normalize the slug to match the REST DashboardPutSchema contract.
+
+ Mirrors ``BaseDashboardSchema.post_load``: strip, replace spaces with
+ hyphens, and drop characters outside ``[\\w-]`` so the tool cannot
+ persist slugs the REST update path would have cleaned.
+
+ Whitespace-only inputs normalize to ``""`` (clears the slug), matching
+ REST schema behavior. Raises ``ValueError`` when a non-whitespace input
+ normalizes to empty (e.g. ``"!!!"``), preventing accidental slug clearing.
+ """
+ if not v:
+ return v
+ stripped = v.strip()
+ if not stripped:
+ return "" # whitespace-only → same as empty string (clears slug)
+ normalized = re.sub(r"[^\w\-]+", "", stripped.replace(" ", "-"))
+ if not normalized:
+ raise ValueError(
+ "slug contains only characters that are removed during "
+ "normalization; use letters, digits, underscores, or hyphens"
+ )
+ return normalized
+
class UpdateDashboardResponse(BaseModel):
"""Response schema for ``update_dashboard``.
diff --git a/superset/mcp_service/dashboard/tool/update_dashboard.py b/superset/mcp_service/dashboard/tool/update_dashboard.py
index ebe7952a4a7..fa0782de45d 100644
--- a/superset/mcp_service/dashboard/tool/update_dashboard.py
+++ b/superset/mcp_service/dashboard/tool/update_dashboard.py
@@ -113,8 +113,7 @@ def _merge_json_metadata(dashboard: Any, overrides: dict[str, Any]) -> str:
existing: dict[str, Any] = {}
if dashboard.json_metadata:
try:
- parsed = json.loads(dashboard.json_metadata)
- if isinstance(parsed, dict):
+ if isinstance(parsed := json.loads(dashboard.json_metadata), dict):
existing = parsed
except (ValueError, TypeError):
pass
@@ -122,13 +121,50 @@ def _merge_json_metadata(dashboard: Any, overrides: dict[str, Any]) -> str:
return json.dumps(existing)
+# Typed json_metadata convenience fields. Each maps 1:1 to a json_metadata
+# key but is exposed as a validated field so an LLM does not have to hand-build
+# the raw ``json_metadata_overrides`` dict for common toggles.
+_TYPED_METADATA_FIELDS: tuple[str, ...] = (
+ "cross_filters_enabled",
+ "refresh_frequency",
+ "filter_bar_orientation",
+)
+
+
+def _collect_metadata_overrides(request: UpdateDashboardRequest) -> dict[str, Any]:
+ """Combine the generic ``json_metadata_overrides`` with the typed fields.
+
+ A key set via both a typed field and the generic dict is ambiguous, so a
+ collision raises ``ValueError``. Otherwise the typed fields are layered on
+ top of the generic overrides. The generic dict stays as an escape hatch for
+ keys without a typed field.
+ """
+ overrides: dict[str, Any] = dict(request.json_metadata_overrides or {})
+ typed: dict[str, Any] = {
+ field: value
+ for field in _TYPED_METADATA_FIELDS
+ if (value := getattr(request, field)) is not None
+ }
+ if clashes := sorted(set(overrides) & set(typed)):
+ raise ValueError(
+ "Conflicting metadata for "
+ + ", ".join(clashes)
+ + ": set via both a typed field and json_metadata_overrides. "
+ + "Pass each key only once."
+ )
+ overrides.update(typed)
+ return overrides
+
+
def _apply_field_updates(dashboard: Any, request: UpdateDashboardRequest) -> list[str]:
"""Apply each explicitly-passed field to the dashboard.
Returns the names of fields actually changed. Mutates ``dashboard``
- in place. ``json_metadata_overrides`` is merged shallowly with the
- existing ``json_metadata``; an empty string in ``slug`` or ``css``
- clears the underlying value.
+ in place. ``json_metadata_overrides`` (plus the typed metadata fields) is
+ merged shallowly with the existing ``json_metadata``; an empty string in
+ ``slug`` or ``css`` clears the underlying value; ``tags`` fully replaces the
+ dashboard's custom tags. Inputs are assumed pre-validated by
+ ``_validate_update_request``.
"""
changed: list[str] = []
@@ -152,25 +188,90 @@ def _apply_field_updates(dashboard: Any, request: UpdateDashboardRequest) -> lis
dashboard.position_json = json.dumps(request.position_json)
changed.append("position_json")
- if request.json_metadata_overrides is not None:
- dashboard.json_metadata = _merge_json_metadata(
- dashboard, request.json_metadata_overrides
- )
+ metadata_overrides: dict[str, Any] = _collect_metadata_overrides(request)
+ if metadata_overrides:
+ dashboard.json_metadata = _merge_json_metadata(dashboard, metadata_overrides)
changed.append("json_metadata")
if request.css is not None:
dashboard.css = request.css or None
changed.append("css")
+ if request.tags is not None:
+ # Reuse the same helper the REST UpdateDashboardCommand uses so tag
+ # association semantics (custom-tag full replacement) stay identical.
+ from superset.commands.utils import update_tags
+ from superset.tags.models import ObjectType
+
+ update_tags(ObjectType.dashboard, dashboard.id, dashboard.tags, request.tags)
+ changed.append("tags")
+
return changed
+def _validate_update_request(
+ dashboard: Any, request: UpdateDashboardRequest
+) -> DashboardError | None:
+ """Pre-flight validation mirroring the REST update path.
+
+ Runs before any mutation so the tool rejects the same payloads the REST
+ ``DashboardPutSchema`` / ``UpdateDashboardCommand`` would — invalid CSS,
+ conflicting metadata keys, and unauthorized or unknown tag IDs — returning a
+ structured error instead of failing deep inside the commit.
+ """
+ from marshmallow import ValidationError as MarshmallowValidationError
+
+ from superset.commands.exceptions import (
+ TagForbiddenError,
+ TagNotFoundValidationError,
+ )
+ from superset.commands.utils import validate_tags
+ from superset.dashboards.schemas import validate_css
+ from superset.tags.models import ObjectType
+
+ # Empty string clears CSS (no validation needed); only validate real content.
+ if request.css:
+ try:
+ validate_css(request.css)
+ except MarshmallowValidationError as ex:
+ detail = (
+ "; ".join(str(m) for m in ex.messages)
+ if isinstance(ex.messages, list)
+ else str(ex.messages)
+ )
+ return DashboardError(
+ error=f"Dashboard CSS is invalid: {detail}",
+ error_type="InvalidCSS",
+ )
+
+ try:
+ _collect_metadata_overrides(request)
+ except ValueError as ex:
+ return DashboardError(error=str(ex), error_type="InvalidRequest")
+
+ if request.tags is not None:
+ try:
+ validate_tags(ObjectType.dashboard, dashboard.tags, request.tags)
+ except TagForbiddenError as ex:
+ return DashboardError(error=str(ex), error_type="TagForbidden")
+ except TagNotFoundValidationError as ex:
+ return DashboardError(error=str(ex), error_type="TagNotFound")
+ except SQLAlchemyError:
+ logger.warning("Database error during tag validation", exc_info=True)
+ return DashboardError(
+ error="Failed to validate tags due to a database error.",
+ error_type="DatabaseError",
+ )
+
+ return None
+
+
@tool(
tags=["mutate"],
class_permission_name="Dashboard",
method_permission_name="write",
annotations=ToolAnnotations(
- title="Update dashboard layout/theme/CSS",
+ title="Update dashboard layout/theme/CSS/metadata",
readOnlyHint=False,
destructiveHint=False,
),
@@ -178,31 +279,32 @@ def _apply_field_updates(dashboard: Any, request: UpdateDashboardRequest) -> lis
def update_dashboard(
request: UpdateDashboardRequest, ctx: Context
) -> UpdateDashboardResponse | DashboardError:
- """Patch an existing dashboard's layout, theme, or styling.
+ """Patch an existing dashboard's layout, theme, styling, or metadata.
- Companion to ``generate_dashboard`` for incremental edits. Accepts
- the same layout/theme/CSS fields that ``generate_dashboard`` does, so
- an LLM can:
+ Companion to ``generate_dashboard`` for incremental edits. An LLM can:
- Set or replace ``position_json`` after auto-generation
- Apply brand ``label_colors`` and ``color_scheme`` via
``json_metadata_overrides``
- - Toggle ``cross_filters_enabled`` via ``json_metadata_overrides``
- Inject ``css`` to hide chrome on print-ready dashboards
- Update ``dashboard_title``, ``description``, ``slug``, ``published``
+ - Replace the dashboard's ``tags`` (FULL list of IDs; find them with
+ ``list_tags``)
+ - Toggle ``cross_filters_enabled``, ``refresh_frequency``, or
+ ``filter_bar_orientation`` via typed fields (no need to hand-build
+ ``json_metadata_overrides``)
Only the fields explicitly passed are applied; other fields are left
unchanged. ``json_metadata_overrides`` is merged shallowly with the
- existing json_metadata — pass only the keys you want to change.
+ existing json_metadata — pass only the keys you want to change. A key may
+ not be set via both a typed field and ``json_metadata_overrides``.
Example::
update_dashboard(request={
"identifier": 42,
- "json_metadata_overrides": {
- "label_colors": {"Electronics": "#4C78A8"},
- "cross_filters_enabled": False,
- },
+ "tags": [3, 7],
+ "refresh_frequency": 300,
"css": ".header-controls {display: none;}",
})
"""
@@ -212,6 +314,12 @@ def update_dashboard(
if auth_error is not None:
return auth_error
+ validation_error: DashboardError | None = _validate_update_request(
+ dashboard, request
+ )
+ if validation_error is not None:
+ return validation_error
+
changed_fields: list[str] = []
warnings: list[str] = list(request.sanitization_warnings)
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 3698a808ab0..065bc7da197 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
@@ -28,7 +28,7 @@ from superset.utils import json
@pytest.fixture
-def mcp_server():
+def mcp_server() -> object:
return mcp
@@ -52,7 +52,7 @@ def _mock_dashboard(
css: str | None = None,
json_metadata: str | None = None,
position_json: str | None = None,
-):
+) -> Mock:
"""Build a Mock with EVERY field the DashboardInfo serializer touches
explicitly set. Without this, Mock returns auto-Mock objects for
unset attributes, which Pydantic rejects as wrong-type."""
@@ -92,7 +92,7 @@ class TestUpdateDashboard:
@patch("superset.extensions.db.session")
@pytest.mark.asyncio
async def test_update_layout_theme_and_css(
- self, mock_session, mock_get, mcp_server
+ self, mock_session: Mock, mock_get: Mock, mcp_server: object
) -> None:
dash = _mock_dashboard(
id=42,
@@ -141,7 +141,7 @@ class TestUpdateDashboard:
@patch("superset.extensions.db.session")
@pytest.mark.asyncio
async def test_update_with_no_fields_is_noop(
- self, mock_session, mock_get, mcp_server
+ self, mock_session: Mock, mock_get: Mock, mcp_server: object
) -> None:
dash = _mock_dashboard(id=42)
original_css = dash.css
@@ -165,7 +165,7 @@ class TestUpdateDashboard:
@patch("superset.daos.dashboard.DashboardDAO.get_by_id_or_slug")
@pytest.mark.asyncio
async def test_update_missing_dashboard_returns_error(
- self, mock_get, mcp_server
+ self, mock_get: Mock, mcp_server: object
) -> None:
# get_by_id_or_slug raises when not found; the tool catches and
# returns a structured DashboardError
@@ -187,7 +187,7 @@ class TestUpdateDashboard:
@patch("superset.extensions.db.session")
@pytest.mark.asyncio
async def test_update_title_and_slug_and_published(
- self, mock_session, mock_get, mcp_server
+ self, mock_session: Mock, mock_get: Mock, mcp_server: object
) -> None:
dash = _mock_dashboard(id=42, published=False)
mock_get.return_value = dash
@@ -212,7 +212,9 @@ class TestUpdateDashboard:
@patch("superset.daos.dashboard.DashboardDAO.get_by_id_or_slug")
@patch("superset.extensions.db.session")
@pytest.mark.asyncio
- async def test_update_description(self, mock_session, mock_get, mcp_server) -> None:
+ async def test_update_description(
+ self, mock_session: Mock, mock_get: Mock, mcp_server: object
+ ) -> None:
"""A description-only update writes ``description`` and reports
it in ``changed_fields`` without touching other fields."""
dash = _mock_dashboard(id=42)
@@ -242,7 +244,7 @@ class TestUpdateDashboard:
@patch("superset.extensions.db.session")
@pytest.mark.asyncio
async def test_empty_slug_clears_slug(
- self, mock_session, mock_get, mcp_server
+ self, mock_session: Mock, mock_get: Mock, mcp_server: object
) -> None:
"""An explicit empty string clears the slug."""
dash = _mock_dashboard(id=42, slug="had-a-slug")
@@ -258,7 +260,9 @@ class TestUpdateDashboard:
@patch("superset.daos.dashboard.DashboardDAO.get_by_id_or_slug")
@pytest.mark.asyncio
- async def test_non_owner_gets_permission_denied(self, mock_get, mcp_server) -> None:
+ async def test_non_owner_gets_permission_denied(
+ self, mock_get: Mock, mcp_server: object
+ ) -> None:
"""A user without ownership on the dashboard receives a
permission_denied response — the class-level Dashboard.write
permission is not enough on its own.
@@ -293,7 +297,7 @@ class TestUpdateDashboard:
assert dash.dashboard_title == "Test Dashboard"
@pytest.mark.asyncio
- async def test_xss_only_title_is_rejected(self, mcp_server) -> None:
+ async def test_xss_only_title_is_rejected(self, mcp_server: object) -> None:
"""A dashboard_title that sanitizes to an empty string raises at
the Pydantic layer — same guard as ``generate_dashboard``. The
update path must not be a backdoor for XSS payloads."""
@@ -310,3 +314,177 @@ class TestUpdateDashboard:
}
},
)
+
+ @patch("superset.commands.utils.update_tags")
+ @patch("superset.commands.utils.validate_tags")
+ @patch("superset.daos.dashboard.DashboardDAO.get_by_id_or_slug")
+ @patch("superset.extensions.db.session")
+ @pytest.mark.asyncio
+ async def test_update_tags_replaces(
+ self,
+ mock_session: Mock,
+ mock_get: Mock,
+ mock_validate_tags: Mock,
+ mock_update_tags: Mock,
+ mcp_server: object,
+ ) -> None:
+ """``tags`` routes through the same validate/update helpers the REST
+ UpdateDashboardCommand uses, and reports ``tags`` as changed."""
+ from superset.tags.models import ObjectType
+
+ dash = _mock_dashboard(id=42)
+ mock_get.return_value = dash
+
+ async with Client(mcp_server) as client:
+ result = await client.call_tool(
+ "update_dashboard",
+ {"request": {"identifier": 42, "tags": [3, 7]}},
+ )
+
+ mock_validate_tags.assert_called_once()
+ mock_update_tags.assert_called_once()
+ update_args = mock_update_tags.call_args.args
+ assert update_args[0] == ObjectType.dashboard
+ assert update_args[1] == 42
+ assert update_args[3] == [3, 7]
+ payload = json.loads(result.content[0].text)
+ assert "tags" in payload.get("changed_fields", [])
+
+ @patch("superset.commands.utils.update_tags")
+ @patch("superset.commands.utils.validate_tags")
+ @patch("superset.daos.dashboard.DashboardDAO.get_by_id_or_slug")
+ @patch("superset.extensions.db.session")
+ @pytest.mark.asyncio
+ async def test_update_tags_empty_list_clears(
+ self,
+ mock_session: Mock,
+ mock_get: Mock,
+ mock_validate_tags: Mock,
+ mock_update_tags: Mock,
+ mcp_server: object,
+ ) -> None:
+ """An empty ``tags`` list is a full replacement that clears all
+ custom tags — it must still reach ``update_tags`` (not be treated
+ as 'unchanged')."""
+ dash = _mock_dashboard(id=42)
+ mock_get.return_value = dash
+
+ async with Client(mcp_server) as client:
+ await client.call_tool(
+ "update_dashboard",
+ {"request": {"identifier": 42, "tags": []}},
+ )
+
+ mock_update_tags.assert_called_once()
+ assert mock_update_tags.call_args.args[3] == []
+
+ @patch("superset.daos.dashboard.DashboardDAO.get_by_id_or_slug")
+ @patch("superset.extensions.db.session")
+ @pytest.mark.asyncio
+ async def test_typed_metadata_toggles_fold_into_json_metadata(
+ self, mock_session: Mock, mock_get: Mock, mcp_server: object
+ ) -> None:
+ """Typed convenience fields are merged into json_metadata without
+ clobbering unrelated keys."""
+ dash = _mock_dashboard(
+ id=42, json_metadata=json.dumps({"label_colors": {"A": "#111"}})
+ )
+ mock_get.return_value = dash
+
+ async with Client(mcp_server) as client:
+ result = await client.call_tool(
+ "update_dashboard",
+ {
+ "request": {
+ "identifier": 42,
+ "cross_filters_enabled": False,
+ "refresh_frequency": 300,
+ "filter_bar_orientation": "HORIZONTAL",
+ }
+ },
+ )
+
+ merged = json.loads(dash.json_metadata)
+ assert merged["cross_filters_enabled"] is False
+ assert merged["refresh_frequency"] == 300
+ assert merged["filter_bar_orientation"] == "HORIZONTAL"
+ assert merged["label_colors"] == {"A": "#111"} # preserved
+ payload = json.loads(result.content[0].text)
+ assert "json_metadata" in payload.get("changed_fields", [])
+
+ @patch("superset.daos.dashboard.DashboardDAO.get_by_id_or_slug")
+ @patch("superset.extensions.db.session")
+ @pytest.mark.asyncio
+ async def test_typed_metadata_conflict_is_rejected(
+ self, mock_session: Mock, mock_get: Mock, mcp_server: object
+ ) -> None:
+ """Setting the same key via a typed field AND json_metadata_overrides
+ is ambiguous and rejected before any write."""
+ dash = _mock_dashboard(id=42)
+ mock_get.return_value = dash
+
+ async with Client(mcp_server) as client:
+ result = await client.call_tool(
+ "update_dashboard",
+ {
+ "request": {
+ "identifier": 42,
+ "cross_filters_enabled": False,
+ "json_metadata_overrides": {"cross_filters_enabled": True},
+ }
+ },
+ )
+
+ payload = json.loads(result.content[0].text)
+ assert "cross_filters_enabled" in (payload.get("error") or "")
+ mock_session.commit.assert_not_called()
+
+ @patch("superset.dashboards.schemas.validate_css")
+ @patch("superset.daos.dashboard.DashboardDAO.get_by_id_or_slug")
+ @patch("superset.extensions.db.session")
+ @pytest.mark.asyncio
+ async def test_invalid_css_is_rejected(
+ self,
+ mock_session: Mock,
+ mock_get: Mock,
+ mock_validate_css: Mock,
+ mcp_server: object,
+ ) -> None:
+ """CSS is run through the same ``validate_css`` the REST schema uses;
+ a rejection short-circuits before any write."""
+ from marshmallow import ValidationError
+
+ dash = _mock_dashboard(id=42)
+ mock_get.return_value = dash
+ mock_validate_css.side_effect = ValidationError("CSS is invalid")
+
+ async with Client(mcp_server) as client:
+ result = await client.call_tool(
+ "update_dashboard",
+ {"request": {"identifier": 42, "css": "@import url(evil);"}},
+ )
+
+ payload = json.loads(result.content[0].text)
+ assert "css is invalid" in (payload.get("error") or "").lower()
+ mock_session.commit.assert_not_called()
+
+ def test_request_slug_is_normalized(self) -> None:
+ """Slug is cleaned to match the REST DashboardPutSchema contract."""
+ from pydantic import ValidationError as PydanticValidationError
+
+ from superset.mcp_service.dashboard.schemas import UpdateDashboardRequest
+
+ assert (
+ UpdateDashboardRequest(identifier=1, slug=" My Slug!? ").slug == "My-Slug"
+ )
+ assert UpdateDashboardRequest(identifier=1, slug="").slug == ""
+ assert UpdateDashboardRequest(identifier=1).slug is None
+ # Whitespace-only normalizes to empty string (clears slug), matching REST.
+ assert UpdateDashboardRequest(identifier=1, slug=" ").slug == ""
+ # A slug containing only non-word characters is rejected (can't be
+ # silently cleared when the intent is to set a slug).
+ with pytest.raises(
+ PydanticValidationError,
+ match="characters that are removed during normalization",
+ ):
+ UpdateDashboardRequest(identifier=1, slug="!!!")