aminghadersohi opened a new pull request, #45081:
URL: https://github.com/apache/superset/pull/45081
### SUMMARY
The MCP service serves one set of registered tools in two discovery shapes,
both built from `build_middleware_list()`:
- **compatibility (default):** `MCP_TOOL_SEARCH_CONFIG["enabled"] = True`.
`tools/list` returns the pinned tools plus `search_tools`/`call_tool`.
- **native:** `enabled = False`. `tools/list` returns every permitted tool
under its real name.
Neither shape had a contract test for the native surface or for parity
between the two. This PR adds those tests, records a measured inventory of the
native catalog, and fixes one parity bug they found. No default changes.
**Fix: `call_tool` lost `isError`.** `LoggingMiddleware` rebuilds the
`ToolResult` to attach `mcp_call_id`, and the rebuild dropped `is_error`. A
direct call raises at that layer, so the flag survived. A call forwarded by the
`call_tool` proxy is different: its inner middleware chain has already turned
the exception into an error result. The outer rebuild then turned that into a
success, so a permission denial or validation error sent through `call_tool`
reached the client with `isError: false`. It was also logged as successful. The
rebuild now keeps `is_error`, and a returned error result is logged as a
failure.
**Native inventory (`native_tool_inventory.json`):** for each registered
tool it records the annotations, description length, input/output schema size,
and the full `tools/list` entry size with structured output disabled and
enabled, plus catalog totals. These are measured from the canonical definitions
over the MCP protocol, not maintained by hand. Sizes may shrink; growth past 2%
or 200 bytes fails until the report is regenerated with
`SUPERSET_MCP_UPDATE_TOOL_INVENTORY=1`. Measured on this branch: 79 tools. The
full native listing is 218,156 bytes text-only and 524,788 bytes structured.
The largest entry is `manage_native_filters` at 10,916 bytes text-only and
`generate_chart` at 28,070 bytes structured. The longest description is
`generate_chart` at 6,688 characters.
**Contract tests (`test_native_tool_surface.py`), all checked on the native
path, a direct named call in compatibility mode, and the `call_tool` proxy:**
- Native `tools/list` lists every registered tool exactly once, as one
deterministic page with no cursor and no synthetic tools.
- Listed descriptions, annotations, and input schemas equal the canonical
definitions. Schemas are dereferenced by FastMCP's default
`dereference_schemas`.
- `outputSchema` is present for every tool only when
`MCP_STRUCTURED_OUTPUT_ENABLED = True`.
- Every advertised schema is valid Draft 2020-12 with no dangling `$ref`.
- No single entry exceeds a 100 KB list page.
- Every registered tool dispatches under its real name. Each call ends at
schema validation or the authorization gate, with identical error results on
all three paths. Tools in `ALLOWED_UNPROTECTED` run for every caller, as
designed.
- A successful call returns identical content and `structuredContent` on all
three paths under both structured-output settings, and the structured result
validates against the advertised schema.
- Validation errors keep identical details through the proxy.
- RBAC, token scopes, and the restricted-principal policy give the same
visible set (native `tools/list` vs `search_tools`) and the same denial. A
hidden tool named directly or through `call_tool` is rejected.
- Revoking a permission hides and blocks the tool on the next request on
every path.
- A tool removed from the registry, for example by `MCP_DISABLED_TOOLS`, is
unreachable and absent from search.
**Docs:** `mcp-server.mdx` now describes both discovery modes and
authorization in each. It adds migration steps and mixed-version and multi-pod
rollout notes, and states that compatibility mode remains supported and any
default change will be listed in `UPDATING.md`. It also removes a stale
reference to middleware classes that do not exist; the service applies no rate
limiting of its own.
Known differences, kept unchanged and documented:
- With no authentication source configured (development only), native
`tools/list` fails open while `search_tools` shows only permission-free tools.
Protected calls are rejected either way.
- Read-only tools leave `idempotentHint` unset.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable; MCP protocol behavior and tests only.
### TESTING INSTRUCTIONS
```bash
PYTHONPATH="$PWD/superset-core/src" pytest -q \
tests/unit_tests/mcp_service/test_native_tool_surface.py \
tests/unit_tests/mcp_service/test_middleware_logging.py
```
Without the `LoggingMiddleware` change, 7 of the 17 surface tests fail
because proxied errors arrive with `isError: false`.
### ADDITIONAL INFORMATION
- [ ] 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]
