You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/08/09 23:35:15 UTC
[PR] docs(mcp): add update_dashboard to write-tools enumeration [superset]
rusackas opened a new pull request, #42943:
URL: https://github.com/apache/superset/pull/42943
Follow-up to #40399.
### SUMMARY
`update_dashboard` (added in #40399) is already listed in the Dashboard Management bullet list in `get_default_instructions` and correctly gates on write access, but the separate "Permission Awareness" paragraph that explicitly enumerates write tools (`generate_chart, generate_dashboard, update_chart, ...`) still didn't mention it. Agents that key off that paragraph specifically to figure out which tools require write access wouldn't know `update_dashboard` needs it.
This just adds `update_dashboard` to that enumeration in `superset/mcp_service/app.py`, right after `update_chart`.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A, docstring-only change.
### TESTING INSTRUCTIONS
No behavior change. Confirmed pre-commit passes on the changed file:
```
pre-commit run --files superset/mcp_service/app.py
```
### 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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): add update_dashboard to write-tools enumeration [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42943:
URL: https://github.com/apache/superset/pull/42943#issuecomment-5234444246
<!-- Bito Reply -->
The flagged issue is correct. The `generate_chart` tool is currently listed under write-permission tools in the documentation, even though it supports a non-persisting preview mode that only requires read access.
To resolve this, you can update the documentation to distinguish between preview and save operations, or adjust the tool's authorization logic. A concise fix is to update the guidance text to clarify that `generate_chart` requires write access only when `save_chart=True` is used.
I have checked the PR comments, and there are no other comments to address. Would you like me to implement the documentation update for you?
**superset/mcp_service/app.py**
```
- Write tools (generate_chart, generate_dashboard, update_chart, update_dashboard,
+ Write tools (generate_dashboard, update_chart, update_dashboard,
+ duplicate_dashboard, create_dataset, create_virtual_dataset, update_dataset_metric,
+ save_sql_query, add_chart_to_existing_dashboard, manage_native_filters,
+ remove_chart_from_dashboard, update_chart_preview, manage_dashboard_owners,
+ manage_dashboard_roles, manage_dashboard_certification) require write
+ permissions. generate_chart requires write access only when save_chart=True.
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): add update_dashboard to write-tools enumeration [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe merged PR #42943:
URL: https://github.com/apache/superset/pull/42943
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): add update_dashboard to write-tools enumeration [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #42943:
URL: https://github.com/apache/superset/pull/42943#discussion_r3745823006
##########
superset/mcp_service/app.py:
##########
@@ -493,11 +493,11 @@ def get_default_instructions(
{_feature_availability}Permission Awareness:
{_instance_info_role_bullet}- ALWAYS check the user's roles BEFORE suggesting write operations (creating datasets,
charts, or dashboards). SQL execution is a separate permission — see execute_sql below.
-- Write tools (generate_chart, generate_dashboard, update_chart, duplicate_dashboard,
- create_dataset, create_virtual_dataset, update_dataset_metric, save_sql_query,
- add_chart_to_existing_dashboard, manage_native_filters, remove_chart_from_dashboard,
- update_chart_preview, manage_dashboard_owners, manage_dashboard_roles,
- manage_dashboard_certification) require write
+- Write tools (generate_chart, generate_dashboard, update_chart, update_dashboard,
Review Comment:
generate_chart is tagged \`["mutate"]\` in its \`@tool\` decorator with no explicit \`method_permission_name\`, and per \`core_mcp_injection.py\` that defaults to write, not read. So the doc here is accurate: write access is required regardless of save_chart, previews included. Not going to change this.
##########
superset/mcp_service/app.py:
##########
@@ -493,11 +493,11 @@ def get_default_instructions(
{_feature_availability}Permission Awareness:
{_instance_info_role_bullet}- ALWAYS check the user's roles BEFORE suggesting write operations (creating datasets,
charts, or dashboards). SQL execution is a separate permission — see execute_sql below.
-- Write tools (generate_chart, generate_dashboard, update_chart, duplicate_dashboard,
- create_dataset, create_virtual_dataset, update_dataset_metric, save_sql_query,
- add_chart_to_existing_dashboard, manage_native_filters, remove_chart_from_dashboard,
- update_chart_preview, manage_dashboard_owners, manage_dashboard_roles,
- manage_dashboard_certification) require write
+- Write tools (generate_chart, generate_dashboard, update_chart, update_dashboard,
Review Comment:
generate_chart is tagged `["mutate"]` in its `@tool` decorator with no explicit `method_permission_name`, and per `core_mcp_injection.py` that defaults to write, not read. So the doc here is accurate: write access is required regardless of save_chart, previews included. Not going to change this.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): add update_dashboard to write-tools enumeration [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42943:
URL: https://github.com/apache/superset/pull/42943#issuecomment-5234427155
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): add update_dashboard to write-tools enumeration [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #42943:
URL: https://github.com/apache/superset/pull/42943#discussion_r3745684825
##########
superset/mcp_service/app.py:
##########
@@ -493,11 +493,11 @@ def get_default_instructions(
{_feature_availability}Permission Awareness:
{_instance_info_role_bullet}- ALWAYS check the user's roles BEFORE suggesting write operations (creating datasets,
charts, or dashboards). SQL execution is a separate permission — see execute_sql below.
-- Write tools (generate_chart, generate_dashboard, update_chart, duplicate_dashboard,
- create_dataset, create_virtual_dataset, update_dataset_metric, save_sql_query,
- add_chart_to_existing_dashboard, manage_native_filters, remove_chart_from_dashboard,
- update_chart_preview, manage_dashboard_owners, manage_dashboard_roles,
- manage_dashboard_certification) require write
+- Write tools (generate_chart, generate_dashboard, update_chart, update_dashboard,
Review Comment:
**Suggestion:** The permission guidance incorrectly classifies `generate_chart` as requiring write access. Its decorator uses the default read permission, and the documented default operation is a non-persisting preview, so users with chart read access but without chart write access can validly use it. This guidance may cause agents to suppress valid preview requests; either separate preview usage from `save_chart=True` or align the tool's actual authorization with this enumeration. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Read-only users lose valid chart preview workflows.
- ⚠️ MCP agents may incorrectly suppress `generate_chart`.
- ⚠️ Permission guidance conflicts with actual RBAC behavior.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f02f5190a60b4f2c9f1abb28c7ab9fac&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f02f5190a60b4f2c9f1abb28c7ab9fac&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/app.py
**Line:** 496:496
**Comment:**
*Api Mismatch: The permission guidance incorrectly classifies `generate_chart` as requiring write access. Its decorator uses the default read permission, and the documented default operation is a non-persisting preview, so users with chart read access but without chart write access can validly use it. This guidance may cause agents to suppress valid preview requests; either separate preview usage from `save_chart=True` or align the tool's actual authorization with this enumeration.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42943&comment_hash=10f2ff329bf63700066dff8945c8ee719b359bb39626db6b0b8c80bd1d7c6e1c&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42943&comment_hash=10f2ff329bf63700066dff8945c8ee719b359bb39626db6b0b8c80bd1d7c6e1c&reaction=dislike'>👎</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] docs(mcp): add update_dashboard to write-tools enumeration [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #42943:
URL: https://github.com/apache/superset/pull/42943#issuecomment-5234445196
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/42943?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 57.13%. Comparing base ([`3b164e4`](https://app.codecov.io/gh/apache/superset/commit/3b164e4270860ac07223d4df1a60ca7b56312362?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`aec79c6`](https://app.codecov.io/gh/apache/superset/commit/aec79c686a7741c0c650bc026b840a936ff1e273?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
> :exclamation: There is a different number of reports uploaded between BASE (3b164e4) and HEAD (aec79c6). Click for more details.
>
> <details><summary>HEAD has 131 uploads less than BASE</summary>
>
>| Flag | BASE (3b164e4) | HEAD (aec79c6) |
>|------|------|------|
>|hive|17|1|
>|python|101|2|
>|presto|17|1|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #42943 +/- ##
==========================================
- Coverage 66.41% 57.13% -9.29%
==========================================
Files 2858 2858
Lines 161446 161446
Branches 37190 37190
==========================================
- Hits 107222 92235 -14987
- Misses 52187 68355 +16168
+ Partials 2037 856 -1181
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/42943/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/42943/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `38.21% <ø> (ø)` | |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/42943/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/42943/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/42943/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.17% <ø> (ø)` | |
| [python](https://app.codecov.io/gh/apache/superset/pull/42943/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.23% <ø> (-18.99%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/42943/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/42943/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/42943?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org