codeant-ai-for-open-source[bot] commented on code in PR #40957:
URL: https://github.com/apache/superset/pull/40957#discussion_r3493379042
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -871,6 +902,28 @@ def sanitize_dashboard_title(cls, v: str | None) -> str |
None:
v, "Dashboard title", max_length=500, allow_empty=True
)
+ @field_validator("slug")
+ @classmethod
+ def normalize_slug(cls, v: str | None) -> str | None:
+ """Normalize the slug to match the REST DashboardPutSchema contract.
+
+ Mirrors ``BaseDashboardSchema.post_load``: strip, replace spaces with
+ hyphens, and drop characters outside ``[\\w-]`` so the tool cannot
+ persist slugs the REST update path would have cleaned.
+
+ Raises ``ValueError`` when a non-empty input normalizes to an empty
+ string (e.g. ``"!!!"``) to prevent silently clearing the slug.
+ """
+ if not v:
+ return v
+ normalized = re.sub(r"[^\w\-]+", "", v.strip().replace(" ", "-"))
+ if not normalized:
+ raise ValueError(
+ "slug contains only characters that are removed during "
+ "normalization; use letters, digits, underscores, or hyphens"
+ )
Review Comment:
**Suggestion:** This validator raises for any non-empty input that
normalizes to empty, which makes whitespace-only slugs fail instead of being
normalized the way the REST schema does. That creates a contract mismatch
between MCP and REST for equivalent slug inputs; align behavior so slug
normalization semantics are consistent across both update paths. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ MCP update_dashboard rejects slugs accepted by REST update.
- ⚠️ Inconsistent slug clearing semantics between MCP and REST.
- ⚠️ LLM clients may see unexpected validation failures.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Call the MCP `update_dashboard` tool defined in
`superset/mcp_service/dashboard/tool/update_dashboard.py:190-221` from an
MCP client,
providing a valid `identifier` and setting `slug` in the `request` payload
to a
whitespace-only string (for example `" "`), while leaving other fields valid.
2. FastMCP constructs an `UpdateDashboardRequest` instance (schema in
`superset/mcp_service/dashboard/schemas.py:3-24`), which triggers the
`@field_validator("slug")` method `normalize_slug` at
`superset/mcp_service/dashboard/schemas.py:148-166` during Pydantic
validation.
3. Inside `normalize_slug`, for input `" "`, `v.strip().replace(" ", "-")`
becomes `""`,
and `re.sub(r"[^\w\-]+", "", ...)` leaves it as `""`; this hits the `if not
normalized:`
branch at line 920 and raises `ValueError` via the block at lines 920-924,
causing the MCP
request to fail validation and preventing the update.
4. Send the same slug value (`" "`) to the REST dashboard update endpoint,
which uses
`DashboardPutSchema` and `BaseDashboardSchema.post_load` in
`superset/dashboards/schemas.py:395-403` and `469-548`; there, `post_load`
strips and
normalizes `slug` without raising when it becomes empty, and with `slug`
validated as
`Length(0, 255)` the REST path accepts the payload and effectively clears
the slug. This
demonstrates a concrete contract mismatch between MCP and REST for
equivalent slug inputs.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=65fd30d02761408191f5e738cdac7594&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=65fd30d02761408191f5e738cdac7594&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/dashboard/schemas.py
**Line:** 920:924
**Comment:**
*Api Mismatch: This validator raises for any non-empty input that
normalizes to empty, which makes whitespace-only slugs fail instead of being
normalized the way the REST schema does. That creates a contract mismatch
between MCP and REST for equivalent slug inputs; align behavior so slug
normalization semantics are consistent across both update paths.
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%2F40957&comment_hash=517437bc2e76d315a28bc135c414a9926d7c0fdc8091ec1da4ceed61e5bb1100&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40957&comment_hash=517437bc2e76d315a28bc135c414a9926d7c0fdc8091ec1da4ceed61e5bb1100&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]
