codeant-ai-for-open-source[bot] commented on code in PR #41698:
URL: https://github.com/apache/superset/pull/41698#discussion_r3555314684
##########
superset/mcp_service/middleware.py:
##########
@@ -1445,12 +1451,17 @@ def create_response_size_guard_middleware() ->
ResponseSizeGuardMiddleware | Non
logger.info("Response size guard is disabled")
return None
+ max_list_items = config.get("max_list_items", DEFAULT_MAX_LIST_ITEMS)
+ if max_list_items is None:
+ max_list_items = DEFAULT_MAX_LIST_ITEMS
+
middleware = ResponseSizeGuardMiddleware(
token_limit=int(config.get("token_limit", DEFAULT_TOKEN_LIMIT)),
warn_threshold_pct=int(
config.get("warn_threshold_pct", DEFAULT_WARN_THRESHOLD_PCT)
),
excluded_tools=config.get("excluded_tools"),
+ max_list_items=int(max_list_items),
Review Comment:
**Suggestion:** The new `max_list_items` config path is cast with `int()`
without handling bad user config values, so a non-numeric value (for example
from `superset_config.py`) will raise `ValueError`/`TypeError` and break
middleware initialization at runtime. Parse this value defensively and fall
back to `DEFAULT_MAX_LIST_ITEMS` (with a warning log) when conversion fails.
[type error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
❌ MCP HTTP server fails when max_list_items misconfigured.
❌ MCP stdio mode crashes during middleware initialization.
⚠️ Misconfigured deployments break instead of falling back defaults.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure the Flask app with an MCP response-size config where
`max_list_items` is
non-numeric, e.g. in `superset_config.py` set `MCP_RESPONSE_SIZE_CONFIG =
{"enabled":
True, "max_list_items": "many"}`; this dict is later read via
`flask_app.config.get("MCP_RESPONSE_SIZE_CONFIG", MCP_RESPONSE_SIZE_CONFIG)`
in
`create_response_size_guard_middleware()` at
`superset/mcp_service/middleware.py:44-49`
(verified in BulkRead output).
2. Start the MCP HTTP server using the default path in `run_server()` (or
equivalent) so
that `create_response_size_guard_middleware()` is invoked from
`superset/mcp_service/server.py:27-40`, where `middleware_list =
build_middleware_list()`
is followed by `size_guard_middleware =
create_response_size_guard_middleware()` and the
result is appended to `middleware_list` if truthy (confirmed via Read on
`server.py:27-40`).
3. During middleware factory execution in
`create_response_size_guard_middleware()` at
`superset/mcp_service/middleware.py:55-66`, the code assigns `max_list_items
=
config.get("max_list_items", DEFAULT_MAX_LIST_ITEMS)` (line ~1454), so
`max_list_items`
becomes the string `"many"`, then calls `max_list_items=int(max_list_items)`
in the
`ResponseSizeGuardMiddleware` constructor call (line ~1464), which raises a
`ValueError`
because the string is not a valid integer literal (no defensive conversion
is present in
this function).
4. Since the surrounding `try` block at
`superset/mcp_service/middleware.py:40-75` only
catches `(ImportError, AttributeError, KeyError)` and not `ValueError` or
`TypeError`, the
exception propagates out of `create_response_size_guard_middleware()`,
causing
`run_server()` in `server.py` and `_add_default_middlewares()` in
`superset/mcp_service/__main__.py:11-15` (which also calls
`create_response_size_guard_middleware()` without its own try/except) to
fail, and the MCP
service process aborts instead of starting with a safe default for
`max_list_items`.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6f904112a9404a9cb4c5d6d2f02bdc34&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=6f904112a9404a9cb4c5d6d2f02bdc34&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/middleware.py
**Line:** 1454:1464
**Comment:**
*Type Error: The new `max_list_items` config path is cast with `int()`
without handling bad user config values, so a non-numeric value (for example
from `superset_config.py`) will raise `ValueError`/`TypeError` and break
middleware initialization at runtime. Parse this value defensively and fall
back to `DEFAULT_MAX_LIST_ITEMS` (with a warning log) when conversion fails.
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%2F41698&comment_hash=e01ce7415b128ff979a5d6f21ce12bda5e6d30ae705c78ba6237dc15290661c4&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41698&comment_hash=e01ce7415b128ff979a5d6f21ce12bda5e6d30ae705c78ba6237dc15290661c4&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]
