richardfogaca commented on PR #39835: URL: https://github.com/apache/superset/pull/39835#issuecomment-4452649127
I like the direction of `MCP_DISABLED_TOOLS` as an operator escape hatch, especially for deployments that want to replace built-in write/object-creation tools with stricter extension tools. One thing I’d like to see before merge: can we make the default instructions fully capability-aware, not just filter the `- tool_name:` bullets? For example, if `execute_sql` is disabled, several SQL workflow examples/rules still mention it; if `get_instance_info` is disabled, the startup guidance still tells clients to call it. That could leave clients with instructions for tools they cannot call. Also, would it be worth using the public server visibility API if available in our pinned dependency version, e.g. disabling `tool:<name>` via the server visibility layer, instead of removing from `_local_provider` directly? That seems closer to the intent: hide from listing and prevent calls without mutating provider internals. Overall I’m +1 on the feature direction; I think the main thing is avoiding instruction/tool-registry drift. -- 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]
