codeant-ai-for-open-source[bot] commented on code in PR #42629:
URL: https://github.com/apache/superset/pull/42629#discussion_r3693113499
##########
superset/mcp_service/server.py:
##########
@@ -965,6 +965,16 @@ def run_server(
logging.info("Creating MCP app from factory configuration...")
factory_config = get_mcp_factory_config()
mcp_instance = create_mcp_app(**factory_config)
+ # The factory path bypasses init_fastmcp_server(), so install the
+ # per-tool-call session scoping here as well; without it concurrent
+ # tool calls share the greenlet-scoped db.session.
+ # Lazy import mirrors init_fastmcp_server() to avoid the circular
+ # import through superset.extensions during startup.
+ from superset.mcp_service.session_scope import ( # noqa: PLC0415
+ install_mcp_session_scoping,
+ )
+
+ install_mcp_session_scoping()
Review Comment:
**Suggestion:** Session scoping is installed only in `run_server()` after
the factory returns. Callers that use the public `create_mcp_app()` factory
directly receive a FastMCP instance whose protected tools still execute with
the original greenlet-scoped `db.session`, so concurrent calls through that
supported factory path retain the DetachedInstanceError/session-sharing
failure. Install the MCP session scope as part of factory initialization or
otherwise guarantee it before the returned instance can serve requests. [api
mismatch]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Direct factory deployments lack concurrent session isolation.
- ❌ Concurrent tool calls can produce detached ORM instances.
- ⚠️ Public factory behavior differs from `run_server()`.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ff0c9c01966341798f01b8590f462118&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=ff0c9c01966341798f01b8590f462118&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:** 967:977
**Comment:**
*Api Mismatch: Session scoping is installed only in `run_server()`
after the factory returns. Callers that use the public `create_mcp_app()`
factory directly receive a FastMCP instance whose protected tools still execute
with the original greenlet-scoped `db.session`, so concurrent calls through
that supported factory path retain the DetachedInstanceError/session-sharing
failure. Install the MCP session scope as part of factory initialization or
otherwise guarantee it before the returned instance can serve requests.
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%2F42629&comment_hash=ee6867e840f36a6f529ab5c84837c70bc20fd028e0e060740b35f63b7075dfa8&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42629&comment_hash=ee6867e840f36a6f529ab5c84837c70bc20fd028e0e060740b35f63b7075dfa8&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]
