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


##########
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:
   Fixed in 5aa02f9a77. The registration test now records every tool whose 
`openWorldHint` is not exactly `False` and reports the offending tool names. 
The registration suite passes (16 tests), and pre-commit passes.



##########
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

Review Comment:
   Fixed in 5aa02f9a77. The test now fails if any registered tool has 
`openWorldHint` set to anything other than `False`, while retaining the 
completeness check for all required annotations.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to