style: apply auto-walrus to _merge_json_metadata

Collapses the two-line `parsed = ...; if isinstance(parsed, dict):` into
a single walrus expression to satisfy the auto-walrus pre-commit hook.

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
 
@@ -796,6 +797,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=(
@@ -858,6 +889,20 @@ 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.
+        """
+        if not v:
+            return v
+        v = v.strip().replace(" ", "-")
+        return re.sub(r"[^\w\-]+", "", v)
+
 
 class UpdateDashboardResponse(BaseModel):
     """Response schema for ``update_dashboard``.

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 := json.loads(dashboard.json_metadata), dict):
-            if isinstance(parsed, 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 = (
+    "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 = {
+        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
+    in place. ``json_metadata_overrides`` (plus the typed metadata fields) is
-    existing ``json_metadata``; an empty string in ``slug`` or ``css``
+    merged shallowly with the existing ``json_metadata``; an empty string in
-    clears the underlying value.
+    ``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,84 @@ 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:
+    metadata_overrides = _collect_metadata_overrides(request)
-        dashboard.json_metadata = _merge_json_metadata(
+    if metadata_overrides:
-            dashboard, request.json_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")
+
+    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 +273,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
+    Companion to ``generate_dashboard`` for incremental edits. An LLM can:
-    the same layout/theme/CSS fields that ``generate_dashboard`` does, so
-    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": {
+            "tags": [3, 7],
-                "label_colors": {"Electronics": "#4C78A8"},
+            "refresh_frequency": 300,
-                "cross_filters_enabled": False,
-            },
             "css": ".header-controls {display: none;}",
         })
     """
@@ -212,6 +308,10 @@ def update_dashboard(
     if auth_error is not None:
         return auth_error
 
+    validation_error = _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)
 

tests/unit_tests/mcp_service/dashboard/tool/test_update_dashboard.py

@@ -309,3 +309,152 @@ 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_get, mock_validate_tags, mock_update_tags, mcp_server
+    ) -> 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_get, mock_validate_tags, mock_update_tags, mcp_server
+    ) -> 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_get, mcp_server
+    ) -> 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_get, mcp_server
+    ) -> 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_get, mock_validate_css, mcp_server
+    ) -> 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 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