codeant-ai-for-open-source[bot] commented on code in PR #40906:
URL: https://github.com/apache/superset/pull/40906#discussion_r3404187097
##########
superset/mcp_service/server.py:
##########
@@ -509,6 +509,27 @@ def _fix_call_tool_arguments(tool: Any) -> Any:
return tool
+def _fix_search_tool_query(tool: Any) -> Any:
+ """Fix anyOf schema in search_tools ``query`` for MCP bridge compatibility.
+
+ The optional ``query: str | None`` parameter emits an ``anyOf`` JSON
+ Schema with no top-level ``type``. Some MCP bridges (mcp-remote,
+ Claude Desktop) don't handle ``anyOf`` and strip it, leaving the field
+ typeless โ the same failure mode ``_fix_call_tool_arguments`` guards
+ against. Replaces the ``anyOf`` with a flat ``type: string``.
+
+ Only the advertised schema changes; FastMCP validates calls against
+ the function signature, so omitting ``query`` remains valid.
+ """
+ if "query" in (props := (tool.parameters or {}).get("properties", {})):
+ props["query"] = {
+ "default": None,
+ "description": "Natural language query. Omit to list all available
tools.",
+ "type": "string",
+ }
Review Comment:
**Suggestion:** The advertised schema declares `query` as `"type": "string"`
but also sets `"default": None`, which is a type-contract mismatch. Strict
schema consumers/bridges can reject or mishandle this field, reintroducing
discovery failures; keep the schema/default type-consistent (or represent
nullability explicitly in a bridge-compatible way). [api mismatch]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ Tool schema advertises inconsistent type and default for query.
- โ ๏ธ Strict MCP bridges may reject or mangle search_tools schema.
- โ ๏ธ Could reintroduce intermittent tool discovery or invocation failures.
```
</details>
<details>
<summary><b>Steps of Reproduction โ
</b></summary>
```mdx
1. With the MCP tool search transform enabled,
`_apply_tool_search_transform` in
`superset/mcp_service/server.py:564-191` calls `_create_search_transform` at
`server.py:179-183`, which will build the synthetic `search_tools` tool via
`_make_optional_query_search_tool` at `server.py:653-228`.
2. `_make_optional_query_search_tool` registers the `search_tools` coroutine
as a tool
using `Tool.from_function` at `superset/mcp_service/server.py:675`, then
immediately calls
`_fix_search_tool_query(tool)` at `server.py:676` to patch the JSON Schema
for the `query`
parameter.
3. `_fix_search_tool_query` is defined at
`superset/mcp_service/server.py:512-81` and,
when it sees a `"query"` property, overwrites it at lines `525-529` with
`{"default":
None, "description": "...", "type": "string"}`, meaning the schema
advertises `"type":
"string"` while also declaring a `null` default value.
4. When FastMCP exposes this tool schema to MCP bridges (e.g., via the
search result
serializer configured in `_apply_tool_search_transform` at
`server.py:133-140`), any
strict schema consumer that validates or applies defaults may treat the
`"default": null`
for a `"type": "string"` field as a type violation, potentially rejecting or
lossy-normalizing the `query` parameter and reintroducing tool-discovery or
invocation
issues similar to the earlier `anyOf`-stripping problem.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9a2e364e5acb4f2382b82bd5606ce0bd&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=9a2e364e5acb4f2382b82bd5606ce0bd&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/server.py
**Line:** 525:529
**Comment:**
*Api Mismatch: The advertised schema declares `query` as `"type":
"string"` but also sets `"default": None`, which is a type-contract mismatch.
Strict schema consumers/bridges can reject or mishandle this field,
reintroducing discovery failures; keep the schema/default type-consistent (or
represent nullability explicitly in a bridge-compatible way).
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40906&comment_hash=4424a1c813428ef2da1c819e4c66c3435fb49c2f75862767a056bdc9b435212b&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40906&comment_hash=4424a1c813428ef2da1c819e4c66c3435fb49c2f75862767a056bdc9b435212b&reaction=dislike'>๐</a>
##########
superset/mcp_service/server.py:
##########
@@ -619,14 +640,40 @@ async def call_tool(
)
-def _create_search_transform(
+def _create_search_transform( # noqa: C901
*,
strategy: str,
kwargs: dict[str, Any],
make_normalizing_call_tool: Callable[[Any], Any],
) -> Any:
"""Create the configured search transform with tool-permission
filtering."""
from fastmcp.server.context import Context
+ from fastmcp.tools.base import Tool
Review Comment:
**Suggestion:** The search tool factory imports `Tool` from a different
module than the rest of this file (`fastmcp.tools.tool`), then calls
`Tool.from_function`. If `fastmcp.tools.base.Tool` is the base/abstract type
(as its module name suggests), this can fail at runtime when building the
transform (missing `from_function` or wrong tool type). Use the same concrete
`Tool` class/module already used by the call-tool path to avoid runtime API
mismatches. [api mismatch]
<details>
<summary><b>Severity Level:</b> Critical ๐จ</summary>
```mdx
- โ MCP tool search transform fails when constructing search tool.
- โ ๏ธ LLM clients cannot use search_tools for discovery.
- โ ๏ธ MCP_TOOL_SEARCH_CONFIG.enabled feature flag becomes unusable.
```
</details>
<details>
<summary><b>Steps of Reproduction โ
</b></summary>
```mdx
1. Enable the MCP tool search transform by setting
`MCP_TOOL_SEARCH_CONFIG["enabled"] =
True`, so that `run_server()` in `superset/mcp_service/server.py:827-878`
calls
`_apply_tool_search_transform(mcp_instance, tool_search_config)` at
`server.py:867`.
2. During `_apply_tool_search_transform` (defined at
`superset/mcp_service/server.py:564`), the function
`_create_search_transform` is invoked
at `server.py:179-183` to construct the BM25/regex search transform.
3. Inside `_create_search_transform` (defined at
`superset/mcp_service/server.py:643`),
`_make_optional_query_search_tool` builds the `search_tools` synthetic tool
and calls
`Tool.from_function(fn=search_tools, name=transform._search_tool_name)` at
`server.py:675`, using `Tool` imported from `fastmcp.tools.base` at
`server.py:651`.
4. Elsewhere in the same codebase, all usages of `Tool.from_function` import
the concrete
Tool type from `fastmcp.tools.tool` or `fastmcp.tools` (e.g.,
`superset/mcp_service/server.py:580` and
`superset/core/mcp/core_mcp_injection.py:133-135`), indicating that
`fastmcp.tools.base.Tool` is not the intended factory; if that base class
does not
implement `from_function`, `_create_search_transform` will raise
`AttributeError: type
object 'Tool' has no attribute 'from_function'`, causing MCP startup to fail
whenever the
tool search transform is enabled.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=782200b65e4248b995c53d547092aeed&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=782200b65e4248b995c53d547092aeed&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/server.py
**Line:** 651:651
**Comment:**
*Api Mismatch: The search tool factory imports `Tool` from a different
module than the rest of this file (`fastmcp.tools.tool`), then calls
`Tool.from_function`. If `fastmcp.tools.base.Tool` is the base/abstract type
(as its module name suggests), this can fail at runtime when building the
transform (missing `from_function` or wrong tool type). Use the same concrete
`Tool` class/module already used by the call-tool path to avoid runtime API
mismatches.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40906&comment_hash=853b670e38f8e587f85150603e382ff634ac0b7b60c6566e822500b574dfafd3&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40906&comment_hash=853b670e38f8e587f85150603e382ff634ac0b7b60c6566e822500b574dfafd3&reaction=dislike'>๐</a>
##########
superset/mcp_service/server.py:
##########
@@ -619,14 +640,40 @@ async def call_tool(
)
-def _create_search_transform(
+def _create_search_transform( # noqa: C901
*,
strategy: str,
kwargs: dict[str, Any],
make_normalizing_call_tool: Callable[[Any], Any],
) -> Any:
"""Create the configured search transform with tool-permission
filtering."""
from fastmcp.server.context import Context
+ from fastmcp.tools.base import Tool
+
+ def _make_optional_query_search_tool(transform: Any) -> Any:
+ """Create search tool with optional query โ returns all tools when
omitted."""
+
+ async def search_tools(
+ query: Annotated[
+ str | None,
+ "Natural language query. Omit to list all available tools.",
+ ] = None,
+ ctx: Context = None,
+ ) -> str | list[dict[str, Any]]:
+ """Search for tools using natural language.
+
+ Returns matching tool definitions ranked by relevance.
+ If no query is provided, returns all available tools.
+ """
+ hidden = await transform._get_visible_tools(ctx)
+ if not query:
+ results = hidden
+ else:
+ results = await transform._search(hidden, query)
Review Comment:
**Suggestion:** This condition treats an empty string the same as an omitted
query, so `search_tools({"query": ""})` returns the full visible catalog
instead of running the search path. That silently changes behavior for
provided-but-empty input and can unexpectedly return a very large result set;
check specifically for `None` to represent "omitted query." [incorrect
condition logic]
<details>
<summary><b>Severity Level:</b> Major โ ๏ธ</summary>
```mdx
- โ ๏ธ Empty-string queries return full catalog, not filtered results.
- โ ๏ธ Tool search responses may be much larger than expected.
- โ ๏ธ Behavior contradicts documented contract for omitted queries.
```
</details>
<details>
<summary><b>Steps of Reproduction โ
</b></summary>
```mdx
1. Enable the tool search transform so that `run_server()` in
`superset/mcp_service/server.py:827-878` calls
`_apply_tool_search_transform(mcp_instance,
tool_search_config)` at `server.py:867`, which in turn calls
`_create_search_transform` at
`server.py:179-183`.
2. `_create_search_transform` defines `_make_optional_query_search_tool` at
`superset/mcp_service/server.py:653-228`, which in turn defines the
`search_tools`
function at `server.py:656-223` and registers it as a synthetic tool via
`Tool.from_function` at `server.py:675`.
3. An MCP client (e.g. an LLM bridge) invokes the `search_tools` tool with
an explicitly
empty query string, sending arguments equivalent to `{"query": ""}`; FastMCP
routes this
call to the `search_tools` coroutine defined at `server.py:656-223`.
4. Inside `search_tools`, `query` is the empty string, so the condition `if
not query:` at
`superset/mcp_service/server.py:669` evaluates to True, setting `results =
hidden` (all
visible tools) instead of calling `transform._search(hidden, query)` at
`server.py:672`,
even though the docstring at `server.py:214-218` says "If no query is
provided, returns
all available tools," treating omitted `query` (None) differently from an
explicitly
supplied empty string; this silently changes behavior and can return the
full catalog for
provided-but-empty input.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6c6db668365d46069bf0b7a31f52503b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=6c6db668365d46069bf0b7a31f52503b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/server.py
**Line:** 669:672
**Comment:**
*Incorrect Condition Logic: This condition treats an empty string the
same as an omitted query, so `search_tools({"query": ""})` returns the full
visible catalog instead of running the search path. That silently changes
behavior for provided-but-empty input and can unexpectedly return a very large
result set; check specifically for `None` to represent "omitted query."
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40906&comment_hash=7306508cc7f316ae7953a40eeb1322c3fa95e8ca0e8411ebe47e58cb53a3085f&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40906&comment_hash=7306508cc7f316ae7953a40eeb1322c3fa95e8ca0e8411ebe47e58cb53a3085f&reaction=dislike'>๐</a>
--
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]
