bito-code-review[bot] commented on code in PR #36078:
URL: https://github.com/apache/superset/pull/36078#discussion_r2516509271
##########
superset/mcp_service/server.py:
##########
@@ -82,9 +82,20 @@ def run_server(
factory_config = get_mcp_factory_config()
mcp_instance = create_mcp_app(**factory_config)
else:
- # Use default initialization
+ # Use default initialization with response caching middleware
logging.info("Creating MCP app with default configuration...")
- mcp_instance = init_fastmcp_server()
+
+ # Import here to avoid circular imports
+ from superset.mcp_service.mcp_config import
create_response_caching_middleware
+
+ # Create middleware list - response caching is enabled by default
+ middleware_list = []
+ caching_middleware = create_response_caching_middleware()
+ if caching_middleware:
+ middleware_list.append(caching_middleware)
+
+ # Create MCP instance with middleware
+ mcp_instance = create_mcp_app(middleware=middleware_list or None)
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Broken MCP tool registration on new instance</b></div>
<div id="fix">
The change from `init_fastmcp_server()` to
`create_mcp_app(middleware=middleware_list or None)` creates a new FastMCP
instance that does not include the registered tools, since tools are registered
on the global `mcp` instance in `app.py`. This breaks the MCP server's core
functionality by making all tools unavailable. To fix this, use the global
`mcp` instance and add the middleware to it using `add_middleware()`. The
`run_server` function in `server.py` calls this initialization, and downstream
the `mcp_instance.run()` method expects a properly configured FastMCP instance
with tools.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
-from superset.mcp_service.app import create_mcp_app
+from superset.mcp_service.app import create_mcp_app, mcp
@@ -98,1 +98,4 @@
- mcp_instance = create_mcp_app(middleware=middleware_list or None)
+ mcp_instance = mcp
+ for mw in middleware_list:
+ mcp.add_middleware(mw)
```
</div>
</details>
</div>
<details>
<summary><b>Citations</b></summary>
<ul>
<li>
Rule Violated: <a
href="https://github.com/apache/superset/blob/d7fcffc/superset/mcp_service/CLAUDE.md#L72";>CLAUDE.md:72</a>
</li>
</ul>
</details>
<small><i>Code Review Run <a
href=https://github.com/apache/superset/pull/36078#issuecomment-3519615088>#0b4305</a></i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
