aminghadersohi opened a new pull request, #44591:
URL: https://github.com/apache/superset/pull/44591
### SUMMARY
Return actionable, input-free MCP argument-validation diagnostics through
both the global error handler and the compatibility fallback.
The fallback used `_sanitize_error_for_logging`, whose validation branch
replaces all detail with `Request validation failed`
(`superset/mcp_service/middleware.py:193`). The global handler instead rendered
FastMCP's validation exception verbatim, including Pydantic input values.
FastMCP 3.4.7 retains the structured Pydantic error as its direct cause, so no
dependency change is needed.
A shared formatter uses declared schema field names and server-owned
reasons, masks dynamic keys/unknown fields, omits input/message/context/URL
details, and bounds the number and depth of diagnostics. Unstructured
validation exceptions fail closed. Authorization and response-size guards are
unchanged.
The base already sets `ToolResult.is_error=True` for caught exceptions and
preserves it during structured-output stripping. This PR adds wire-level
regression coverage rather than claiming to introduce that existing fix.
Explicit `column_not_found` results retain their content, metadata, and error
flag, with structured output following the existing compatibility setting.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable; protocol-only change.
- Before, compatibility fallback: `Error: Request validation failed`.
- After: `Error: Validation error in list_datasets: request.page_size:
Expected an integer` with `isError: true`.
- Missing wrappers report `request: Field required`, without returning the
supplied arguments.
### TESTING INSTRUCTIONS
Uses real in-memory FastMCP client/server serialization with the production
`ListDatasetsRequest` schema; no external service is contacted.
```bash
pytest -q tests/unit_tests/mcp_service/test_validation_contract.py \
tests/unit_tests/mcp_service/test_error_classification.py \
tests/unit_tests/mcp_service/test_middleware.py
pre-commit run
```
- Red before: restored the original middleware and ran the new contract
suite: **14 failed, 4 passed**. Both wrong-type and missing-wrapper cases
failed in both middleware configurations and both structured-output modes.
Error flags already passed; missing detail or input disclosure caused the
failures. The four domain-error preservation cases passed unchanged.
- Green after: **203 passed**, including all 18 new contract cases. Covers
validation locations/reasons, error flags, secret-bearing validators/dictionary
keys, bounded diagnostics, fail-closed fallback, and unchanged structured
errors.
- Pre-commit passed, including Ruff, MyPy, and Pylint.
- Tested with FastMCP 3.4.7 and MCP 1.29.1.
### ADDITIONAL INFORMATION
Live-deployment verification is deferred to a separate acceptance phase. No
production or staging endpoint was called. No translatable strings, UI changes,
or migrations.
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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]
