fix(mcp): await ctx.info calls in update_dashboard tool#41920
Conversation
Context.info is async; calling it without await inside the sync update_dashboard tool created unawaited coroutines, silently dropping the log lines. Make the tool async and await both calls, matching the convention used by every other MCP tool.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #41920 +/- ##
==========================================
- Coverage 64.80% 64.79% -0.02%
==========================================
Files 2740 2739 -1
Lines 153135 152940 -195
Branches 35129 35044 -85
==========================================
- Hits 99246 99101 -145
+ Misses 51991 51938 -53
- Partials 1898 1901 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes missing MCP client log output from the update_dashboard tool by converting it to an async tool function and correctly awaiting fastmcp.Context.info(...) calls, preventing unawaited coroutine drops.
Changes:
- Convert
update_dashboardMCP tool toasync defandawaitbothctx.info(...)calls so logs are emitted to the MCP client. - Add a unit test asserting that
Context.infois actually awaited during anupdate_dashboardcall (regression coverage for the unawaited-coroutine bug).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
superset/mcp_service/dashboard/tool/update_dashboard.py |
Makes the MCP tool async and awaits ctx.info so log lines are not silently dropped. |
tests/unit_tests/mcp_service/dashboard/tool/test_update_dashboard.py |
Adds a regression test to ensure ctx.info is awaited when calling the MCP tool. |
|
The current test implementation is indeed brittle because it asserts an exact count of awaits ( Here is a suggested update for the test logic: tests/unit_tests/mcp_service/dashboard/tool/test_update_dashboard.py |
Code Review Agent Run #d179a5Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
Code Review Agent Run #119f91Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
SUMMARY
update_dashboardin the MCP service calledctx.info(...)withoutawaitinside a sync tool function.fastmcp.Context.infois an async method, so these calls created unawaited coroutines and the log lines were silently dropped instead of being emitted.Made the tool async and awaited both
ctx.infocalls, matching the pattern used by every other MCP tool in the service.A repo-wide grep for the same pattern (sync
deftool functions callingctx.info/debug/warning/errorwithoutawait) turned up no other occurrences.BEFORE/AFTER
Before: the two
ctx.info(...)calls inupdate_dashboardcreated coroutines that were never awaited or scheduled, so nothing was logged to the MCP client.After: both calls are awaited and the log lines are emitted as expected.
TESTING INSTRUCTIONS
pytest tests/unit_tests/mcp_service/dashboard/tool/test_update_dashboard.pytest_ctx_info_is_awaited, which patchesfastmcp.Context.infowith anAsyncMockand asserts it is awaited exactly twice with the expected messages — this test fails without the fix.ADDITIONAL INFORMATION