codeant-ai-for-open-source[bot] commented on code in PR #42629:
URL: https://github.com/apache/superset/pull/42629#discussion_r3688743717
##########
superset/mcp_service/auth.py:
##########
@@ -1037,21 +1038,42 @@ def _get_app_context_manager() ->
AbstractContextManager[None]:
When no context exists at all, push a fresh app context from the
Flask singleton.
+ Pushing a new app context also tags the call with a per-call session
+ token (see session_scope.py) so each tool call gets its own SQLAlchemy
+ session — the app-context teardown would otherwise remove the single
+ greenlet-scoped session shared by all in-flight async calls.
+
This is the single source of truth for context selection — called
from both ``mcp_auth_hook`` (tool execution) and
``RBACToolVisibilityMiddleware`` (tools/list filtering).
"""
if has_request_context():
return nullcontext()
Review Comment:
**Suggestion:** Request-backed MCP calls bypass `_mcp_tool_call_context()`
and therefore never set `_mcp_session_token`. When concurrent async requests
execute on the same greenlet, their `db.session` lookups still use the shared
greenlet key, so one request's teardown can remove the session used by another
and detach its ORM objects. Apply the per-call session token to this path as
well, while preserving the externally supplied `g.user` context. [race
condition]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Concurrent request-backed MCP tools can lose ORM session state.
- ❌ Long-running tools may fail after another request completes.
- ⚠️ Affected deployments include streamable HTTP with external request
middleware.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=65724cc1c99b4f7bb3fb06ba4b03e79e&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=65724cc1c99b4f7bb3fb06ba4b03e79e&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/auth.py
**Line:** 1050:1051
**Comment:**
*Race Condition: Request-backed MCP calls bypass
`_mcp_tool_call_context()` and therefore never set `_mcp_session_token`. When
concurrent async requests execute on the same greenlet, their `db.session`
lookups still use the shared greenlet key, so one request's teardown can remove
the session used by another and detach its ORM objects. Apply the per-call
session token to this path as well, while preserving the externally supplied
`g.user` context.
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=1ce82beee786860aea56a723e078f075476fa6d7feac7b3cbaa3954b6eab94c4&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42629&comment_hash=1ce82beee786860aea56a723e078f075476fa6d7feac7b3cbaa3954b6eab94c4&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]
