You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@superset.apache.org by "aminghadersohi (via GitHub)" <gi...@apache.org> on 2026/08/25 23:07:32 UTC

[PR] fix(mcp): add open-world tool annotations [superset]

aminghadersohi opened a new pull request, #43529:
URL: https://github.com/apache/superset/pull/43529

### SUMMARY

The MCP `ToolAnnotations` model includes `openWorldHint` so clients can distinguish tools that stay within a deployment boundary from tools that may interact with external entities. On current `master`, Superset registers 70 MCP tools; all 70 declared `title`, `readOnlyHint`, and `destructiveHint`, but none declared `openWorldHint`.

This change adds an explicit `openWorldHint=False` to every registered tool and adds a registration test that requires every advertised tool to provide `title`, `readOnlyHint`, `destructiveHint`, and `openWorldHint`. The MCP contributor guide is updated so new tools follow the same convention. The broad file count is unavoidable because each tool owns its `ToolAnnotations` declaration and the classification must be explicit at that declaration.

#### Open-world classification

No tool is marked `openWorldHint=True`:

- Metadata read/write tools operate on Superset's metadata database.
- Query tools operate on databases and semantic views configured as Superset datasources, which are part of the deployment's data boundary rather than an open-ended external entity set.
- `generate_explore_link` and `open_sql_lab_with_context` construct Superset-local URLs without fetching them.
- Report tools inspect report configuration but do not send reports or notifications.
- `generate_bug_report` formats local diagnostic data for the caller to copy; it does not submit the report.

Superset currently exposes no MCP tool that fetches an arbitrary external URL, sends mail or alerts, or calls a third-party service.

#### Output schemas

`outputSchema` is intentionally out of scope for this focused annotation change. FastMCP already derives internal output schemas for all 70 registered functions from their return annotations. Superset's `StructuredContentStripperMiddleware` deliberately removes those schemas together with structured content because some MCP client transports cannot encode or validate the structured responses correctly. Advertising output schemas therefore requires a separate compatibility change that pairs schemas with structured results and transport coverage; it is not a schema-derivation task.

#### Blast radius and risk

This only changes MCP `tools/list` metadata and contributor guidance. Tool inputs, outputs, permissions, and execution paths are unchanged. The main risk is an incorrect behavioral classification; each current tool was reviewed against the MCP open-world definition, and the registration test prevents future tools from silently omitting the field. Rollback is a revert of this commit.

### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Not applicable; there is no UI change.

### TESTING INSTRUCTIONS

- `uvx pre-commit run --files $(git diff --name-only origin/master...HEAD)`
- Includes mypy, Ruff formatting/linting, and pylint.
- Full MCP unit suite: `PYTHONPATH="$PWD/superset-core/src" pytest -q tests/unit_tests/mcp_service`
- `3562 passed`
- Registration inventory: 70 tools registered; no missing required annotations; no tools with `openWorldHint=True`.

### EVAL EVIDENCE

Not applicable. This changes deterministic tool-discovery metadata only; tool selection descriptions, arguments, results, and behavior are unchanged.

### COST AND LATENCY DELTA

No tool-call runtime, model, prompt, or parameter changes. `tools/list` adds one boolean annotation per tool.

### PROMPT / NON-DETERMINISM

No prompt, model-routing, or non-deterministic behavior changes.

### ADDITIONAL INFORMATION

- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [x] Introduces new feature or API
- [ ] Removes existing feature or API

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(mcp): add open-world tool annotations [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.

rusackas commented on code in PR #43529:
URL: https://github.com/apache/superset/pull/43529#discussion_r3865110516


##########
tests/unit_tests/mcp_service/test_mcp_tool_registration.py:
##########
@@ -57,6 +57,31 @@ 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."""
+    required_fields = (
+        "title",
+        "readOnlyHint",
+        "destructiveHint",
+        "openWorldHint",
+    )
+    missing_by_tool = {}
+
+    for registered_tool in _run(mcp.list_tools()):
+        annotations = registered_tool.annotations
+        missing = [
+            field
+            for field in required_fields
+            if annotations is None or getattr(annotations, field, None) is None
+        ]
+        if missing:
+            missing_by_tool[registered_tool.name] = missing
+
+    assert not missing_by_tool, (
+        f"Registered MCP tools have incomplete annotations: {missing_by_tool}"
+    )

Review Comment:
   ```suggestion
       missing_by_tool = {}
       open_world_tools = []
   
       for registered_tool in _run(mcp.list_tools()):
           annotations = registered_tool.annotations
           missing = [
               field
               for field in required_fields
               if annotations is None or getattr(annotations, field, None) is None
           ]
           if missing:
               missing_by_tool[registered_tool.name] = missing
           elif annotations.openWorldHint is not False:
               open_world_tools.append(registered_tool.name)
   
       assert not missing_by_tool, (
           f"Registered MCP tools have incomplete annotations: {missing_by_tool}"
       )
       assert not open_world_tools, (
           f"Tools must be explicitly closed-world (openWorldHint=False): {open_world_tools}"
       )
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(mcp): add open-world tool annotations [superset]

Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.

netlify[bot] commented on PR #43529:
URL: https://github.com/apache/superset/pull/43529#issuecomment-5418143005

   ### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
   
   
   |  Name | Link |
   |:-:|------------------------|
   |<span aria-hidden="true">🔨</span> Latest commit | 09741f75457dc33855f4c6ea0debe7dd3a2831a9 |
   |<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a8e2033768b9d00084df036 |
   |<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-43529--superset-docs-preview.netlify.app](https://deploy-preview-43529--superset-docs-preview.netlify.app) |
   |<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br />![QR Code](https://app.netlify.com/qr-code/eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1cmwiOiJodHRwczovL2RlcGxveS1wcmV2aWV3LTQzNTI5LS1zdXBlcnNldC1kb2NzLXByZXZpZXcubmV0bGlmeS5hcHAifQ.FnnS1LL7pb3Rm4t6DP1B5qhxJpdghjFhQ7EUXA6DF-k)<br /><br />_Use your smartphone camera to open QR code link._</details> |
   |<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DctRCsMgDADQu-S7sjCHdbvBjpE2bgo1CVYoZfTu8-_9vB9Y02odXgATLI1kzcNUi3wzcWq75nKrqzm1JO7QtrEjEe3Ui8o-Eifb9HzzaIFiuqP3c4jLkxExPviDPsD1Bw) |
   ---
   <!-- [superset-docs-preview Preview](https://deploy-preview-43529--superset-docs-preview.netlify.app) -->
   _To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org