Repository navigation
fix(mcp): gate update_cloud_connection_unsafe behind insiders mode - #1418
Aaron ("AJ") Steers (aaronsteers) wants to merge 1 commit into
Conversation
Co-Authored-By: AJ Steers <[email protected]>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksTesting This PyAirbyte VersionYou can test this version of PyAirbyte using the following: # Run PyAirbyte CLI from this branch:
uvx --from 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1792616792-unsafe-tool-insiders' pyairbyte --help
# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1792616792-unsafe-tool-insiders'PR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
📚 Show Repo GuidanceHelpful ResourcesCommunity SupportQuestions? Join the #pyairbyte channel in our Slack workspace. |
Code coverage overviewLanguages: Python Python / code-coverage/pytest-no-creds
View coverage of the 10 most impacted files.
Python / code-coverage/pytest
View coverage of the 10 most impacted files.
Python / code-coverage/pytest-unit
Line coverage data for the devin/1792179543-rc-... branch is not yet available. View coverage of the 10 most covered files.
ℹ️ Calculations represent line coverage. Results may change as calculations are updated. Updated |
There was a problem hiding this comment.
🟢 Approval recommended
The focused gating and guidance changes have targeted regression coverage and no identified blocking defects.
No open issues 🎉
What changed in this PR
Restricts the unsafe Cloud connection update tool introduced in #1397 to insiders mode and adjusts guidance to respect the relevant visibility policies.
Changes:
- Marks
update_cloud_connection_unsafeas insiders-only. - Checks insiders, destructive-tool, pipeline-change, and read-only policies before recommending the unsafe tool; otherwise directs callers to the Airbyte UI.
- Adds regression coverage for tool registration, visibility, and error guidance.
| File | Description |
|---|---|
tests/unit_tests/test_mcp_tool_utils.py |
Adds the unsafe tool to expected insiders-only registrations. |
tests/unit_tests/test_mcp_tool_filters.py |
Tests the combined visibility-policy helper. |
tests/unit_tests/test_mcp_connection_schedules.py |
Verifies policy-aware guidance and the UI fallback. |
tests/unit_tests/test_mcp_cloud.py |
Tests advertised tool visibility with insiders mode on and off. |
airbyte/mcp/cloud.py |
Gates the unsafe tool and updates guidance and parameter documentation. |
airbyte/mcp/_tool_utils.py |
Allows insiders-policy checks to accept a server or request context. |
Review effort: Balanced
Overview
👉 TL;DR:
update_cloud_connection_unsafeis now insiders-only, and nothing sends callers to it unless it's actually advertised to them.Stacked on #1397 (targets its branch) to address this review comment.
Changes
update_cloud_connection_unsafe:@mcp_tool(destructive=True, insiders_only=True)._is_unsafe_connection_update_tool_enabled(ctx)inmcp/cloud.py. It mirrors the filters that can hide the tool:update_cloud_connection_unsafe" only when the helper returnsTrue. Otherwise it says to make the change in the Airbyte UI and gives the connection URL. Before this, it only checkeddestructive_tools_allowed.override_stream_configsdescription onupdate_cloud_connectionno longer names the unsafe tool. Schema text is built at registration and can't vary per request. It now says: "Changes that would invalidate a stream's saved state are rejected, and the error says how to proceed."_insiders_modeacceptsFastMCP | Context.Review Spotlight
_is_unsafe_connection_update_tool_enabled: it duplicates the filter chain, so it must be updated when those filters change.Risks
update_cloud_connection_unsafe. In that case the guidance still suggests the tool.Test plan
tests/unit_tests/test_mcp_tool_filters.py::test_unsafe_connection_update_tool_enabled: insiders unset, on, and on with destructive tools off, pipeline changes off, or read-only on.tests/unit_tests/test_mcp_cloud.py::test_unsafe_connection_update_visibility:server.app.list_tools()hides the unsafe tool with insiders off and shows it with insiders on.tests/unit_tests/test_mcp_connection_schedules.py::test_unsafe_change_guidance_uses_unsafe_tool_enabledLink to Devin session: https://app.devin.ai/sessions/4d1ece6885c44c52a07034e025db11e6
Open in Devin Desktop: https://app.devin.ai/desktop/session/4d1ece6885c44c52a07034e025db11e6?variant=devin
Requested by: Aaron ("AJ") Steers (@aaronsteers)