feat(mcp): add list and get tools for action log and tasks - #40304
Closed
aminghadersohi wants to merge 6 commits into
Closed
feat(mcp): add list and get tools for action log and tasks#40304aminghadersohi wants to merge 6 commits into
aminghadersohi wants to merge 6 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #40304 +/- ##
==========================================
- Coverage 64.14% 63.83% -0.32%
==========================================
Files 2592 2600 +8
Lines 138846 140236 +1390
Branches 32201 32497 +296
==========================================
+ Hits 89069 89516 +447
- Misses 48245 49155 +910
- Partials 1532 1565 +33
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
aminghadersohi
force-pushed
the
mcp-action-tasks-99978
branch
3 times, most recently
from
May 21, 2026 18:04
5dbc511 to
66a4ef4
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The injected 7-day default filter used a datetime object as the value, but ActionLogFilter.value only allows str|int|float|bool|list. Pydantic rejects the datetime when building the filters_applied list in ActionLogList, causing a ValidationError on every call that triggered the default filter. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add model_serializer to ActionLogInfo and TaskInfo that drops non-requested fields from output when select_columns context is set, matching the DatabaseInfo pattern - Switch list_action_logs and list_tasks to return model_dump with serialization context so only requested columns appear in responses - Add search field + search-XOR-filters validator to ListActionLogsRequest and ListTasksRequest - Pass search=request.search through to ModelListCore.run_tool() Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rage - Add task_key and task_name fields to TaskInfo schema and ALL_TASK_COLUMNS; these are real Task model columns present in the REST API search_columns - Expand search_columns in list_tasks to include task_key and task_name - Strengthen test_list_action_logs_default_7day_filter_applied to also assert the injected filter appears in filters_applied with an ISO string value Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Pydantic v2 rejects a ColumnOperator instance when validating list[ActionLogFilter] — it requires an exact instance or dict, not a parent-class instance. The injected 7-day default dttm filter was created as a plain ColumnOperator, causing every test_list_action_logs_* call to fail with '1 validation error for ActionLogList'. Fix: construct the default filter as ActionLogFilter (which is a subclass of ColumnOperator), so it passes pydantic validation for ActionLogList.filters_applied: list[ActionLogFilter] and is still accepted everywhere ColumnOperator is expected.
aminghadersohi
force-pushed
the
mcp-action-tasks-99978
branch
from
May 21, 2026 19:24
66a4ef4 to
0f9b0ee
Compare
aminghadersohi
marked this pull request as ready for review
May 22, 2026 03:02
Contributor
Author
|
Closing in favor of ##40344 |
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review Agent Run #8033ab
Actionable Suggestions - 2
-
superset/mcp_service/action_log/schemas.py - 1
- Missing error sanitization validator · Line 201-215
-
superset/mcp_service/task/tool/get_task_info.py - 1
- Broad exception catch · Line 99-99
Additional Suggestions - 3
-
superset/mcp_service/action_log/schemas.py - 1
-
Inconsistent timezone in error create · Line 207-215ActionLogError.create() uses timezone-aware datetime.now(timezone.utc) while DashboardError.create() uses naive datetime.now(). For consistency with other error schemas, use naive datetime.
Code suggestion
--- superset/mcp_service/action_log/schemas.py (lines 207-215) --- 207: @classmethod 208: def create(cls, error: str, error_type: str) -> "ActionLogError": 209: from datetime import timezone 210: 211: return cls( 212: error=error, 213: error_type=error_type, 214: - timestamp=datetime.now(timezone.utc), 215: + timestamp=datetime.now(), 216: )
-
-
superset/mcp_service/action_log/tool/list_action_logs.py - 1
-
Unused function parameter · Line 102-102The `_serialize` function at line 102 accepts a `cols` parameter that is never used in the function body. Either remove it or prefix with underscore to indicate intentional unused parameter.
Code suggestion
--- superset/mcp_service/action_log/tool/list_action_logs.py +++ superset/mcp_service/action_log/tool/list_action_logs.py @@ -99,7 +99,7 @@ async def list_action_logs( filters = [default_filter] + filters await ctx.debug("Applied default 7-day dttm filter: cutoff=%s" % (cutoff,)) - def _serialize(obj: object, cols: list[str] | None) -> ActionLogInfo | None: + def _serialize(obj: object, _cols: list[str] | None) -> ActionLogInfo | None: return serialize_action_log_object(obj)
-
-
superset/mcp_service/task/tool/list_tasks.py - 1
-
Unused parameter in callback · Line 87-87The `_serialize` function accepts a `cols` parameter but never uses it. The field filtering is handled by `TaskInfo._filter_fields_by_context` (model_serializer), so the `cols` parameter is unused here. Consider removing it for clarity, or add a comment like list_charts.py line 167 does: 'field filtering handled by model_serializer'.
-
Filtered by Review Rules
Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
-
superset/mcp_service/task/schemas.py - 1
- Missing return type hint · Line 226-226
-
superset/mcp_service/action_log/schemas.py - 1
- Wrong Literal type for order_column · Line 158-161
Review Details
-
Files reviewed - 17 · Commit Range:
9a6c927..0f9b0ee- superset/mcp_service/action_log/__init__.py
- superset/mcp_service/action_log/schemas.py
- superset/mcp_service/action_log/tool/__init__.py
- superset/mcp_service/action_log/tool/get_action_log_info.py
- superset/mcp_service/action_log/tool/list_action_logs.py
- superset/mcp_service/app.py
- superset/mcp_service/task/__init__.py
- superset/mcp_service/task/schemas.py
- superset/mcp_service/task/tool/__init__.py
- superset/mcp_service/task/tool/get_task_info.py
- superset/mcp_service/task/tool/list_tasks.py
- tests/unit_tests/mcp_service/action_log/__init__.py
- tests/unit_tests/mcp_service/action_log/tool/__init__.py
- tests/unit_tests/mcp_service/action_log/tool/test_action_log_tools.py
- tests/unit_tests/mcp_service/task/__init__.py
- tests/unit_tests/mcp_service/task/tool/__init__.py
- tests/unit_tests/mcp_service/task/tool/test_task_tools.py
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
Comment on lines
+201
to
+215
| class ActionLogError(BaseModel): | ||
| error: str = Field(..., description="Error message") | ||
| error_type: str = Field(..., description="Error type") | ||
| timestamp: str | datetime | None = Field(None, description="Error timestamp") | ||
| model_config = ConfigDict(ser_json_timedelta="iso8601") | ||
|
|
||
| @classmethod | ||
| def create(cls, error: str, error_type: str) -> "ActionLogError": | ||
| from datetime import timezone | ||
|
|
||
| return cls( | ||
| error=error, | ||
| error_type=error_type, | ||
| timestamp=datetime.now(timezone.utc), | ||
| ) |
Contributor
There was a problem hiding this comment.
Missing error sanitization validator
ActionLogError is missing the sanitize_for_llm_context validator present in all other error schemas. Error messages could contain unsanitized content when exposed to LLM context. Add the validator decorator to match DashboardError (line 123-127), ChartError (line 188-191), and DatasetError (line 301-304).
Code suggestion
Check the AI-generated fix before applying
--- superset/mcp_service/action_log/schemas.py (lines 201-215) ---
201: class ActionLogError(BaseModel):
202: error: str = Field(..., description="Error message")
203: error_type: str = Field(..., description="Error type")
204: timestamp: str | datetime | None = Field(None, description="Error timestamp")
205: model_config = ConfigDict(ser_json_timedelta="iso8601")
206:
207: + @field_validator("error")
208: + @classmethod
209: + def sanitize_error_for_llm_context(cls, value: str) -> str:
210: + """Wrap error text before it is exposed to LLM context."""
211: + from superset.mcp_service.utils import sanitize_for_llm_context
212: +
213: + return sanitize_for_llm_context(value, field_path=("error",))
214: +
215: @classmethod
216: def create(cls, error: str, error_type: str) -> "ActionLogError":
Code Review Run #8033ab
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
|
|
||
| return result | ||
|
|
||
| except Exception as e: |
Contributor
There was a problem hiding this comment.
Broad exception catch
Broad except Exception at line 99 masks unexpected errors that may indicate real bugs. Consider catching specific exceptions (e.g., DAOFindFailedError, SQLAlchemyError) or re-raising unexpected ones to avoid silent failure on unexpected error types.
Code Review Run #8033ab
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SUMMARY
Adds 4 new MCP tools across two new domains as part of story #99978:
Action Log domain (
superset/mcp_service/action_log/):list_action_logs: Lists audit log entries with filtering, pagination, and a default 7-day dttm window to prevent oversized result sets on large instances. Admin-only.get_action_log_info: Retrieves a single log entry by integer ID. Admin-only.Task domain (
superset/mcp_service/task/):list_tasks: Lists async tasks. Non-admin users are automatically scoped to tasks they are subscribed to viaTaskDAO.base_filter = TaskFilter. Admins see all tasks.get_task_info: Retrieves a single task by integer ID or UUID string.Both domains follow the established
ModelListCore/ModelGetInfoCorepatterns from the database domain.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — backend-only MCP tools.
TESTING INSTRUCTIONS
Run the new unit tests:
Key behaviors verified by tests:
list_action_logsinjects adttm >= now - 7 daysfilter when nodttmfilter is providedlist_action_logsskips the default filter when the caller provides adttmfilterlist_action_logsdefaults toorder_column=dttm, order_direction=desclist_tasksdelegates toTaskDAO.list()soTaskFilterscoping applies automaticallyget_task_inforesolves by both integer ID and UUID stringerror_type: not_foundresponsesADDITIONAL INFORMATION