aminghadersohi commented on PR #41925: URL: https://github.com/apache/superset/pull/41925#issuecomment-4960306810
@richardfogaca Good catch, and confirmed — I independently verified the same thing while testing this locally: `create_tool_decorator` in `core_mcp_injection.py` builds `wrapped_func = mcp_auth_hook(func)` and registers `Tool.fn = wrapped_func`, then returns that same object to the module scope. Since nothing downstream copies it, whichever decorator runs last mutates the identical object FastMCP already registered — so the marker lands on `tool.fn` regardless of decorator order in this FastMCP version. I reproduced this directly (temporarily reverting the order and re-running the tests) and confirmed the original two tests pass either way. Pushed 50d7e6e adding `test_query_dataset_decorator_order_matches_convention`, which inspects the AST of the decorator list directly rather than relying on the runtime marker. Verified it fails on the pre-fix ordering and passes after. Also reworded the two existing tests' docstrings so they no longer claim to be order-sensitive — they now document the desired end state (tool correctly hidden from restricted users), while the AST test is the one that actually catches a regression. -- 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]
