-
Notifications
You must be signed in to change notification settings - Fork 69
feat(mcp): Add mcp_smoke_tests_prompt for validating all tools and resources #859
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…sources - Add new smoke_tests.py module with mcp_smoke_tests_prompt function - Prompt provides comprehensive testing instructions for all MCP tools and resources - Supports read_only parameter (default: True) to filter tools by read-only status - Register smoke test prompt in MCP server initialization - Groups tools by domain and generates detailed testing instructions Co-Authored-By: AJ Steers <aj@airbyte.io>
Original prompt from AJ Steers |
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. Testing 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/1762905862-add-mcp-smoke-test-prompt' pyairbyte --help
# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1762905862-add-mcp-smoke-test-prompt'Helpful ResourcesPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
Community SupportQuestions? Join the #pyairbyte channel in our Slack workspace. |
📝 WalkthroughWalkthroughThe PR adds smoke test functionality to the MCP server by introducing a new module that registers a prompt for testing MCP tools. The prompt retrieves registered tools, filters them by read-only status based on annotations, groups them by domain, and generates structured testing instructions. Changes
Sequence DiagramsequenceDiagram
participant ServerInit as Server Initialization
participant FastMCP
participant SmokeTestModule as smoke_tests.py
participant ToolRegistry as Tool Registry
ServerInit->>SmokeTestModule: register_smoke_test_prompt(app)
SmokeTestModule->>FastMCP: register prompt "mcp_smoke_tests_prompt"
Note over FastMCP: Prompt registered
FastMCP->>SmokeTestModule: invoke mcp_smoke_tests_prompt(read_only)
SmokeTestModule->>ToolRegistry: get_registered_tools()
ToolRegistry-->>SmokeTestModule: list of tools with annotations
SmokeTestModule->>SmokeTestModule: filter by readOnlyHint if read_only=True
SmokeTestModule->>SmokeTestModule: group tools by domain
SmokeTestModule->>SmokeTestModule: build markdown prompt
SmokeTestModule-->>FastMCP: return user prompt with testing instructions
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
airbyte/mcp/smoke_tests.py (1)
117-122: Consider simplifying the description string formatting, wdyt?The description on lines 119-120 splits the string oddly:
"Run smoke tests on all MCP tools and resources to confirm " "they are working properly"While valid Python (adjacent strings concatenate), you could simplify it to:
"Run smoke tests on all MCP tools and resources to confirm they are working properly"or use parentheses if line length is a concern:
( "Run smoke tests on all MCP tools and resources to confirm " "they are working properly" )Just a minor style suggestion!
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
airbyte/mcp/server.py(2 hunks)airbyte/mcp/smoke_tests.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
airbyte/mcp/smoke_tests.py (1)
airbyte/mcp/_tool_utils.py (1)
get_registered_tools(85-99)
airbyte/mcp/server.py (1)
airbyte/mcp/smoke_tests.py (1)
register_smoke_test_prompt(111-122)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Pytest (All, Python 3.10, Windows)
- GitHub Check: Pytest (All, Python 3.11, Windows)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (No Creds)
- GitHub Check: Pytest (Fast)
🔇 Additional comments (4)
airbyte/mcp/smoke_tests.py (3)
78-82: Generic resource testing instructions noted.The resource testing section provides generic guidance without discovering actual resources. Based on the PR objectives, this is expected since PyAirbyte MCP doesn't have a resource registry. Just flagging this for visibility in case future enhancements add resource discovery capabilities.
12-108: Nice work on the prompt structure!The function cleanly generates comprehensive testing instructions with good organization. The domain-based grouping and clear sections make the output very readable. The approach of generating instructions rather than running automated tests aligns well with the PR objectives.
38-41: I'll search for actual tool definitions to verify howreadOnlyHintis being used:Filtering logic is correct per MCP specification; no changes needed.
The MCP specification defines
readOnlyHintas an optional boolean annotation that indicates if a tool does not modify its environment, with a default offalsewhen unspecified. The MCP spec specifies thatreadOnlyHintdefaults to false, which is exactly what the filtering logic implements viaann.get("readOnlyHint", False)on line 39.This design is intentional and correct: tools must explicitly declare
readOnlyHint=Trueto be included in read-only test suites. This follows the principle that tools must opt-in to such categorization rather than being implicitly assumed to be safe. MCP guidance emphasizes being accurate: "Ensure that the annotations truthfully reflect the tool's behavior. Misleading annotations can lead to poor user experiences or safety issues."The filtering logic aligns with standard MCP tool annotation practices and is not a bug.
airbyte/mcp/server.py (1)
14-14: Clean integration! LGTM.The import and registration follow the established pattern for other MCP tools. The placement is logical and consistent with the existing code structure.
Also applies to: 26-26
PyTest Results (Full)381 tests ±0 364 ✅ - 1 28m 17s ⏱️ + 1m 51s For more details on these failures, see this check. Results for commit 492cf31. ± Comparison against base commit 813d0d0. |
CI Failure AnalysisThe single failing test Failure Details: This is a 500 error from the Airbyte Cloud API indicating a missing secret reference in the CI workspace's destination configuration. The failure is environmental and not caused by the code changes in this PR, which only:
Test Results:
Request: |
feat(mcp): Add mcp_smoke_tests_prompt for validating all tools and resources
Summary
Adds a new MCP prompt called
mcp_smoke_tests_promptthat provides comprehensive testing instructions for validating all MCP tools and resources in the PyAirbyte MCP server. The prompt:get_registered_tools()functionread_onlyparameter (default:True) to filter tools by their read-only statusImplementation Details:
airbyte/mcp/smoke_tests.pycontaining the prompt function and registration logicairbyte/mcp/server.pyto import and register the smoke test promptReview & Testing Checklist for Human
Confirm this is what you wanted: This PR implements a prompt that provides testing instructions, not an automated test runner. When invoked, it generates a markdown document with step-by-step guidance for manually testing tools. If you wanted an automated test function that actually calls all tools and reports results, please clarify and I can revise the implementation.
Test the prompt end-to-end: Invoke the
mcp_smoke_tests_promptthrough an MCP client (e.g., Claude Desktop) with bothread_only=Trueandread_only=Falseto verify it generates useful testing instructionsVerify read-only filtering: Check that the
readOnlyHintannotation filtering correctly identifies all read-only tools. Some tools might be implicitly read-only but not marked as such in their annotations.Resource testing coverage: Note that the prompt provides generic resource testing instructions but doesn't enumerate specific resources (PyAirbyte MCP doesn't appear to have a resource registry like Connector Builder does). Verify this is acceptable or if resource enumeration should be added.
Notes
Requested by: AJ Steers (aj@airbyte.io / @aaronsteers)
Devin Session: https://app.devin.ai/sessions/1145d85639d849b396b2021ba80433b8
Summary by CodeRabbit