From d128237640253d20c2ebb4e37db953d269fa48ac Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Fri, 30 Oct 2026 10:18:44 -0600 Subject: [PATCH 1/2] fix(mcp): propagate semantic metric discovery refusals --- docs/admin_docs/configuration/mcp-server.mdx | 7 +++ .../semantic_layer/tool/list_metrics.py | 3 + .../semantic_layer/tool/test_list_metrics.py | 48 ++++++++++++++++ .../semantic_layers/query_errors_test.py | 57 +++++++++++++++++++ 4 files changed, 115 insertions(+) diff --git a/docs/admin_docs/configuration/mcp-server.mdx b/docs/admin_docs/configuration/mcp-server.mdx index 9c827bcacbdc..0a4cc9a01faf 100644 --- a/docs/admin_docs/configuration/mcp-server.mdx +++ b/docs/admin_docs/configuration/mcp-server.mdx @@ -173,6 +173,13 @@ Chart configuration examples use plain number formats. Currency formatting shoul come from user instructions or verified source metadata, not a field name such as sales, revenue or CPC. +## Metric discovery refusals + +`list_metrics` returns an `AccessDenied` error when a semantic view refuses +metric discovery, including compatible-dimension discovery. It does not report +an empty or partial successful catalog for that refusal. Unexpected failures +in individual providers retain the existing warning and partial-result behavior. + ## Bounding dashboard dataset metadata `get_dashboard_datasets` lists a dashboard's datasets and semantic views with diff --git a/superset/mcp_service/semantic_layer/tool/list_metrics.py b/superset/mcp_service/semantic_layer/tool/list_metrics.py index 8844682b5ee2..9438a35ab8f6 100644 --- a/superset/mcp_service/semantic_layer/tool/list_metrics.py +++ b/superset/mcp_service/semantic_layer/tool/list_metrics.py @@ -229,6 +229,9 @@ async def _collect_external_metrics( compatible_dimensions=compat_dims, ) ) + except SupersetSecurityException: + # Refusals must reach the tool boundary, never a partial catalog. + raise except AmbiguousDimensionError as exc: await ctx.warning( "Could not load metrics for view id=%s: %s" % (view.id, str(exc)) diff --git a/tests/unit_tests/mcp_service/semantic_layer/tool/test_list_metrics.py b/tests/unit_tests/mcp_service/semantic_layer/tool/test_list_metrics.py index 9014ba454a23..4a7a77f589cf 100644 --- a/tests/unit_tests/mcp_service/semantic_layer/tool/test_list_metrics.py +++ b/tests/unit_tests/mcp_service/semantic_layer/tool/test_list_metrics.py @@ -875,3 +875,51 @@ async def test_list_metrics_partial_failure_keeps_only_safe_warning( ) assert "provider-private-detail" not in str(result) assert f"{error_class.__name__}: provider-private-detail" in caplog.text + + +@pytest.mark.usefixtures("semantic_layers_enabled") +@pytest.mark.asyncio +@pytest.mark.parametrize("scoped", [False, True]) +@pytest.mark.parametrize("failure_site", ["metrics", "columns", "compatibility"]) +async def test_list_metrics_refusal_over_http( + mcp_server: FastMCP, scoped: bool, failure_site: str +) -> None: + """Discovery refusals must not become empty or partial successful catalogs.""" + from tests.unit_tests.mcp_service.test_mcp_e2e_smoke import _asgi_client + + message: str = "This data source can't be queried in the current context." + refusal: SupersetSecurityException = SupersetSecurityException( + SupersetError( + message=message, + error_type=SupersetErrorType.SEMANTIC_EXECUTION_REFUSED_ERROR, + level=ErrorLevel.ERROR, + extra={"reason": "method_not_supported"}, + ) + ) + view: MagicMock = _make_view(5) + if failure_site == "compatibility": + view.get_compatible_dimensions.side_effect = refusal + else: + setattr(type(view), failure_site, PropertyMock(side_effect=refusal)) + request: dict[str, object] = {"include_compatible_dimensions": True, "page_size": 4} + if scoped: + request["view_id"] = 5 + daos: tuple[MagicMock, MagicMock, MagicMock] + client: Client + with _patched_dataset_search([_make_dataset()]) as daos: + daos[1].find_accessible.return_value = [_make_view(6), view] + daos[1].find_by_id.return_value = view + async with _asgi_client(mcp_server) as client: + result: CallToolResult = await client.call_tool( + "list_metrics", {"request": request} + ) + data: dict[str, Any] = json.loads(result.content[0].text) + assert data["success"] is False + assert data["error_type"] == "AccessDenied" + assert data["error"] == message + assert "metrics" not in data + assert "total_count" not in data + if scoped: + view.raise_for_access.assert_called_once() + else: + view.raise_for_access.assert_not_called() diff --git a/tests/unit_tests/semantic_layers/query_errors_test.py b/tests/unit_tests/semantic_layers/query_errors_test.py index 079de7f0e0e3..ffc776de7abc 100644 --- a/tests/unit_tests/semantic_layers/query_errors_test.py +++ b/tests/unit_tests/semantic_layers/query_errors_test.py @@ -1297,3 +1297,60 @@ def test_async_provider_error_matches_sync_chart_error( assert "private" not in str(captured.value.to_dict()) task_context.update_task.assert_not_called() date_view.get_table.assert_called_once() + + +@pytest.mark.parametrize("force", [False, True]) +def test_async_refusal_over_http( + client: FlaskClient, + full_api_access: None, + date_view: MagicMock, + mocker: MockerFixture, + force: bool, +) -> None: + """Async chart requests preserve refusals instead of returning a job or 500.""" + from superset.errors import ErrorLevel, SupersetError, SupersetErrorType + from superset.exceptions import SupersetSecurityException + + message: str = "This data source can't be queried in the current context." + refusal: SupersetSecurityException = SupersetSecurityException( + SupersetError( + message=message, + error_type=SupersetErrorType.SEMANTIC_EXECUTION_REFUSED_ERROR, + level=ErrorLevel.ERROR, + extra={"reason": "method_not_supported"}, + ) + ) + # Exercise both async paths: cache probe and fresh task submission. + mocker.patch( + "superset.charts.data.api.ChartDataRestApi._should_run_async", return_value=True + ) + execute: MagicMock = mocker.patch( + "superset.charts.data.api.ChartDataCommand.execute", side_effect=refusal + ) + submit: MagicMock = mocker.patch( + "superset.charts.data.api.submit_chart_data_query_tasks", side_effect=refusal + ) + response: TestResponse = client.post( + "/api/v1/chart/data", + json={ + "datasource": {"id": 7, "type": "semantic_view"}, + "queries": [{"columns": ["ORDER_DATE"], "metrics": ["ORDER_COUNT"]}], + "result_format": "json", + "result_type": "full", + "async_mode": True, + "force": force, + }, + ) + assert response.status_code == 403 + assert ( + response.json["errors"][0]["error_type"] == "SEMANTIC_EXECUTION_REFUSED_ERROR" + ) + assert response.json["errors"][0]["message"] == message + assert "job_id" not in response.json + if force: + execute.assert_not_called() + submit.assert_called_once() + else: + execute.assert_called_once_with(force_cached=True) + submit.assert_not_called() + date_view.get_table.assert_not_called() From aba100c25d8f31c11c14f55bd9212b4f293e1a32 Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Fri, 30 Oct 2026 13:39:55 -0600 Subject: [PATCH 2/2] docs(mcp): explain unscoped metric refusal impact --- docs/admin_docs/configuration/mcp-server.mdx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/admin_docs/configuration/mcp-server.mdx b/docs/admin_docs/configuration/mcp-server.mdx index 0a4cc9a01faf..86abea1f2fbb 100644 --- a/docs/admin_docs/configuration/mcp-server.mdx +++ b/docs/admin_docs/configuration/mcp-server.mdx @@ -180,6 +180,8 @@ metric discovery, including compatible-dimension discovery. It does not report an empty or partial successful catalog for that refusal. Unexpected failures in individual providers retain the existing warning and partial-result behavior. +If any visible view refuses discovery, an unscoped `list_metrics` call fails as a whole, including built-in dataset metrics; use `dataset_id` or `view_id` to scope the call to a source that does not refuse. + ## Bounding dashboard dataset metadata `get_dashboard_datasets` lists a dashboard's datasets and semantic views with