fix(visibility): prevent auto-refresh from overriding tool/prompt/resource visibility#3574
Conversation
Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
…coded 'public' _create_db_tool() hardcoded visibility="public" for all newly discovered tools, while resources and prompts correctly used gateway.visibility. This caused new tools on team-scoped or private gateways to be created with public visibility, breaking team isolation. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
…ed_via check The previous check `should_update_visibility = created_via == "update"` was too broad: any gateway edit (e.g. description change) would trigger visibility propagation to all linked tools/resources/prompts, overwriting per-item customizations. Replace with an explicit `update_visibility: bool` parameter on the three sync helpers, set to True only when the user actually changed the gateway's visibility in the request (`gateway_update.visibility is not None`). This aligns the helper-level gate with the direct propagation logic at lines 1902-1912 which already checks the same condition. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
b59cf9a to
20dee56
Compare
Review & ChangesRebased onto main (clean, no conflicts) and reviewed the implementation. The core fix — preserving per-item visibility during auto-refresh — is correct and well-targeted. I've added three commits on top: 1. Formatting cleanup (
|
crivetimihai
left a comment
There was a problem hiding this comment.
Approved. The core fix is correct and I've added three commits addressing a pre-existing bug (_create_db_tool hardcoding visibility), a correctness issue (unrelated gateway edits overwriting per-item visibility), and formatting. All verified against live deployment and 4958 unit tests pass.
crivetimihai
left a comment
There was a problem hiding this comment.
Approved. The core fix is correct and I've added three commits addressing a pre-existing bug (_create_db_tool hardcoding visibility), a correctness issue (unrelated gateway edits overwriting per-item visibility), and formatting. All verified against live deployment and 4958 unit tests pass.
…ource visibility (#3574) * restrict visibility change with auto refresh Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com> * linting Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com> * fix: formatting cleanup in test_gateway_service_extended.py Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(visibility): new tools inherit gateway visibility instead of hardcoded 'public' _create_db_tool() hardcoded visibility="public" for all newly discovered tools, while resources and prompts correctly used gateway.visibility. This caused new tools on team-scoped or private gateways to be created with public visibility, breaking team isolation. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(visibility): use explicit update_visibility flag instead of created_via check The previous check `should_update_visibility = created_via == "update"` was too broad: any gateway edit (e.g. description change) would trigger visibility propagation to all linked tools/resources/prompts, overwriting per-item customizations. Replace with an explicit `update_visibility: bool` parameter on the three sync helpers, set to True only when the user actually changed the gateway's visibility in the request (`gateway_update.visibility is not None`). This aligns the helper-level gate with the direct propagation logic at lines 1902-1912 which already checks the same condition. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
…ource visibility (#3574) * restrict visibility change with auto refresh Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com> * linting Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com> * fix: formatting cleanup in test_gateway_service_extended.py Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(visibility): new tools inherit gateway visibility instead of hardcoded 'public' _create_db_tool() hardcoded visibility="public" for all newly discovered tools, while resources and prompts correctly used gateway.visibility. This caused new tools on team-scoped or private gateways to be created with public visibility, breaking team isolation. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(visibility): use explicit update_visibility flag instead of created_via check The previous check `should_update_visibility = created_via == "update"` was too broad: any gateway edit (e.g. description change) would trigger visibility propagation to all linked tools/resources/prompts, overwriting per-item customizations. Replace with an explicit `update_visibility: bool` parameter on the three sync helpers, set to True only when the user actually changed the gateway's visibility in the request (`gateway_update.visibility is not None`). This aligns the helper-level gate with the direct propagation logic at lines 1902-1912 which already checks the same condition. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com> Signed-off-by: Yosief Eyob <yosiefogbazion@gmail.com>
🐛 Bug-fix PR
Closes #3530
📌 Summary
Fixed a bug where the
AUTO_REFRESH_SERVERS=trueconfiguration overrides the granular visibility settings of individual tools, resources, and prompts. Previously, during a server auto-refresh, any explicitly configured "Private" or "Team" items would automatically be changed to match the parent Gateway's visibility (e.g., "Public"). This fix ensures that background sync operations preserve existing visibility configurations while still allowing manual UI/API updates to propagate visibility changes correctly.🔁 Reproduction Steps
AUTO_REFRESH_SERVERS=trueor a health check).🐞 Root Cause
The
_update_or_create_tools,_update_or_create_resources, and_update_or_create_promptshelper methods inmcpgateway/services/gateway_service.pycontained logic that unconditionally appliedexisting_item.visibility = gateway.visibilityduring the synchronization process. The methods did not differentiate between an automatic background refresh and a manual update triggered by a user.💡 Fix Description
The
_update_or_create_tools,_update_or_create_resources, and_update_or_create_promptshelper methods inmcpgateway/services/gateway_service.pycontained logic that unconditionally appliedexisting_item.visibility = gateway.visibilityduring the synchronization process. The methods did not differentiate between an automatic background refresh and a manual update triggered by a user.💡 Fix Description
Updated the synchronization logic across tools, resources, and prompts to conditionally evaluate visibility changes based on the operation's context.
should_update_visibility = created_via == "update"condition.visibilityattribute is now explicitly excluded from thefields_to_updateconditions and field assignment blocks unless the sync is triggered by a manual update ("update").test_update_visibility_logicand updated removal scenarios) intest_gateway_service_extended.pyto assert that visibility correctly inherits during manual updates but is preserved duringauto_refresh,health_check, andrediscoverycontexts.🧪 Verification
make lintmake testmake coverage📐 MCP Compliance (if relevant)
✅ Checklist
make black isort pre-commit)