-
Notifications
You must be signed in to change notification settings - Fork 2
feat(mcp): Add mcp_smoke_tests_prompt for validating all tools and resources #185
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, prompts, and resources - Supports read_only parameter (default: True) to filter tools by read-only status - Uses @mcp_prompt decorator for automatic registration - Groups tools by domain and generates detailed testing instructions - Includes resource and prompt testing in addition to tool testing Co-Authored-By: AJ Steers <[email protected]>
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:
|
|
Warning Rate limit exceeded@devin-ai-integration[bot] has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 11 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR introduces a new MCP smoke test prompt module that generates comprehensive testing documentation. The module provides a function to dynamically build smoke test instructions by aggregating registered tools, resources, and prompts—filtered by tool access level when requested—and organizing them by domain. An import is added to the server module. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
✨ 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 |
👋 Welcome to the Airbyte Connector Builder MCP!Thank you for your contribution! Here are some helpful tips and reminders for your convenience. Testing This Branch via MCPTo test the changes in this specific branch with an MCP client like Claude Desktop, use the following configuration: {
"mcpServers": {
"connector-builder-mcp-dev": {
"command": "uvx",
"args": ["--from", "git+https://github.com/airbytehq/connector-builder-mcp.git@devin/1762905945-add-mcp-smoke-test-prompt", "connector-builder-mcp"]
}
}
}Testing This Branch via CLIYou can test this version of the MCP Server using the following CLI snippet: # Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/connector-builder-mcp.git@devin/1762905945-add-mcp-smoke-test-prompt#egg=airbyte-connector-builder-mcp' --helpPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
AI Builder EvaluationsAI builder evaluations run automatically under the following conditions:
A set of standardized evaluations also run on a schedule (Mon/Wed/Fri at midnight UTC) and can be manually triggered via workflow dispatch. Helpful ResourcesIf you have any questions, feel free to ask in the PR comments or join our Slack community. |
| from connector_builder_mcp.mcp.prompts import register_mcp_prompts | ||
| from connector_builder_mcp.mcp.secrets_config import register_secrets_tools | ||
| from connector_builder_mcp.mcp.server_info import register_server_info_resources | ||
| from connector_builder_mcp.mcp.smoke_tests import mcp_smoke_tests_prompt # noqa: F401 |
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: 1
🧹 Nitpick comments (1)
connector_builder_mcp/mcp/smoke_tests.py (1)
60-61: The prompt includes itself in the listing (expected behavior).Since the
@mcp_promptdecorator registers this function at import time,_REGISTERED_PROMPTSwill includemcp_smoke_tests_promptitself when this code executes. This means the generated smoke test instructions will include testing the smoke test prompt, creating a minor circular reference. This is likely acceptable as it provides complete coverage, but be aware of this self-referential behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
connector_builder_mcp/mcp/smoke_tests.py(1 hunks)connector_builder_mcp/server.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-11T02:19:05.231Z
Learnt from: aaronsteers
Repo: airbytehq/connector-builder-mcp PR: 136
File: connector_builder_agents/src/evals/evaluators.py:233-245
Timestamp: 2025-10-11T02:19:05.231Z
Learning: In the connector-builder-mcp repository Python files, all imports must be placed at the module level (top of the file), never inside functions or methods. This is a recurring issue that should be flagged during code reviews.
Applied to files:
connector_builder_mcp/server.py
📚 Learning: 2025-11-08T06:53:26.977Z
Learnt from: aaronsteers
Repo: airbytehq/connector-builder-mcp PR: 176
File: connector_builder_mcp/_manifest_history_utils.py:38-49
Timestamp: 2025-11-08T06:53:26.977Z
Learning: In the connector-builder-mcp repository, all imports must be at the module level (top of the file). Inline imports inside functions or methods are not allowed unless explicit permission is granted. This rule must never be violated.
Applied to files:
connector_builder_mcp/server.py
🧬 Code graph analysis (2)
connector_builder_mcp/mcp/smoke_tests.py (1)
connector_builder_mcp/mcp/_mcp_utils.py (2)
ToolDomain(49-80)mcp_prompt(152-182)
connector_builder_mcp/server.py (1)
connector_builder_mcp/mcp/smoke_tests.py (1)
mcp_smoke_tests_prompt(23-137)
⏰ 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). (3)
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
🔇 Additional comments (4)
connector_builder_mcp/mcp/smoke_tests.py (3)
23-137: Well-structured smoke test prompt generation.The function correctly:
- Filters tools based on the
read_onlyparameter- Groups tools, resources, and prompts by domain for organized output
- Generates comprehensive, actionable testing instructions
- Returns properly formatted prompt content
The string-building approach using list concatenation and join is efficient and readable.
9-15: No action required—private registry access is acceptable for internal package use.Direct access to
_REGISTERED_TOOLS,_REGISTERED_RESOURCES, and_REGISTERED_PROMPTSis appropriate here. These are internal implementation details of themcppackage with no public accessors defined elsewhere in the codebase. The access is limited to internal package files (_mcp_utils.pyandsmoke_tests.py) and follows the established deferred registration pattern. Adding public wrapper functions would only introduce unnecessary boilerplate without improving encapsulation in this context.
37-42: The original review comment is incorrect—tools are properly annotated withreadOnlyHint.The filtering logic is sound and working as intended. Tools are explicitly decorated with
read_only=True(e.g., manifest_checks.py, checklist.py, guidance.py), which correctly maps to thereadOnlyHintannotation key through themcp_tooldecorator. Whenread_only=True, the filter returns tools marked as read-only; whenread_only=False, it includes all tools. The annotation convention is established and followed.Likely an incorrect or invalid review comment.
connector_builder_mcp/server.py (1)
24-24: Side-effect import pattern is correctly implemented.The
@mcp_promptdecorator registers the smoke test prompt in_REGISTERED_PROMPTSat module load time, andregister_mcp_prompts()later consumes the registry. Thenoqa: F401suppression is justified because the import triggers necessary decorator registration, not a true unused import. Verification confirms the flow works correctly.
| def register_smoke_test_prompt(app: FastMCP) -> None: | ||
| """Register the smoke test prompt with the FastMCP app.""" | ||
| pass |
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.
🛠️ Refactor suggestion | 🟠 Major
Remove unused registration function.
The register_smoke_test_prompt() function is never called and serves no purpose since registration happens via the @mcp_prompt decorator. This is dead code that should be removed to avoid confusion.
Apply this diff to remove the unused function:
-def register_smoke_test_prompt(app: FastMCP) -> None:
- """Register the smoke test prompt with the FastMCP app."""
- pass📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def register_smoke_test_prompt(app: FastMCP) -> None: | |
| """Register the smoke test prompt with the FastMCP app.""" | |
| pass |
🤖 Prompt for AI Agents
In connector_builder_mcp/mcp/smoke_tests.py around lines 140 to 142, remove the
unused dead function register_smoke_test_prompt(app: FastMCP) which is never
called because registration is handled by the @mcp_prompt decorator; delete the
entire function definition and its docstring so only the decorator-based
registration remains, and run tests/lint to confirm no references remain.
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.
Code rabbit, that's not exactly correct but thanks for flagging.
Devin,
this function should call the register_prompts helper with the applicable domain as input.
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for register_prompts function definition
echo "=== Searching for register_prompts function ==="
rg -n "def register_prompts" -A 5
echo -e "\n=== Searching for register_prompts usage ==="
rg -n "register_prompts\(" -B 2 -A 2
echo -e "\n=== Searching for similar register_ functions in mcp directory ==="
fd -e py . connector_builder_mcp/mcp/ --exec rg -n "def register_" {} \;
echo -e "\n=== Checking _mcp_utils module for helper functions ==="
fd -e py . connector_builder_mcp/mcp/ -x basename {} | grep -E "(_mcp|utils)" | head -5
rg -n "def register" connector_builder_mcp/mcp/_mcp_utils.py -A 3
Length of output: 1933
🏁 Script executed:
#!/bin/bash
# Check the implementation of register_mcp_prompts
echo "=== register_mcp_prompts implementation ==="
rg -n "def register_mcp_prompts" connector_builder_mcp/mcp/_mcp_utils.py -A 20
echo -e "\n=== Check how other domains use these registration functions ==="
rg -n "register_mcp_prompts\(" -B 1 -A 1
rg -n "register_mcp_tools\(" -B 1 -A 1 | head -20
Length of output: 2567
@aaronsteers, thank you for the clarification! You're absolutely right. The function should call the register_mcp_prompts helper. Here's the corrected implementation:
Step 1: Add register_mcp_prompts to the imports
from connector_builder_mcp.mcp._mcp_utils import (
_REGISTERED_PROMPTS,
_REGISTERED_RESOURCES,
_REGISTERED_TOOLS,
ToolDomain,
mcp_prompt,
register_mcp_prompts, # Add this import
)Step 2: Implement the function body
def register_smoke_test_prompt(app: FastMCP) -> None:
"""Register the smoke test prompt with the FastMCP app."""
register_mcp_prompts(app, domain=ToolDomain.PROMPTS)This follows the same pattern used in prompts.py and other domain registration functions throughout the codebase.
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.
Thanks for the feedback! I've updated the register_smoke_test_prompt() function to call register_mcp_prompts(app, domain=ToolDomain.PROMPTS), following the same pattern used in connector_builder_mcp/mcp/prompts.py.
Implementation details
The function now delegates to the existing register_mcp_prompts helper from _mcp_utils.py, which filters and registers all prompts in the PROMPTS domain. Since the smoke test prompt is already decorated with @mcp_prompt(domain=ToolDomain.PROMPTS), it will be included when this helper is called.
The current server.py already calls register_mcp_prompts(app, domain=ToolDomain.PROMPTS) on line 53, which registers all prompts including the smoke test. This function provides an explicit registration path that follows the established pattern in the codebase.
Pushing the update now.
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.
Skipped: comment is from another GitHub bot.
…rompts helper - Add register_mcp_prompts import from _mcp_utils - Update register_smoke_test_prompt() to call register_mcp_prompts(app, domain=ToolDomain.PROMPTS) - Follows the same pattern as register_prompts() in prompts.py Addresses feedback from @aaronsteers Co-Authored-By: AJ Steers <[email protected]>
feat(mcp): Add mcp_smoke_tests_prompt for validating all tools and resources
Summary
Adds a new MCP prompt called
mcp_smoke_tests_promptto the Connector Builder MCP server that provides comprehensive testing instructions for validating all registered MCP tools, prompts, and resources. This prompt was requested by AJ Steers to enable systematic smoke testing of the MCP server's functionality.Key changes:
connector_builder_mcp/mcp/smoke_tests.pymodule with the smoke test prompt implementation@mcp_promptdecorator for automatic registration in the PROMPTS domainread_onlyparameter (default:True) to filter tools by read-only statusserver.pyto trigger decorator registrationReview & Testing Checklist for Human
This is a YELLOW risk PR - the implementation is straightforward but has some areas that need verification:
read_only=Trueandread_only=Falseto verify the tool filtering works correctly. Check that the annotation keyreadOnlyHintmatches what's actually used in the tool decorators._REGISTERED_TOOLS,_REGISTERED_RESOURCES, and_REGISTERED_PROMPTS(private variables). Consider if this is the intended API or if there should be a public accessor function.Test Plan
mcp_smoke_tests_promptappearsread_only=Falseand verify it includes destructive tools in the outputNotes
register_smoke_test_prompt()function exists but is empty (justpass) because registration happens automatically via the@mcp_promptdecorator. This function could be removed or documented better.server.pyuses# noqa: F401to suppress unused import warnings since the import is only needed for decorator side effects.Requested by: AJ Steers ([email protected]) via GitHub (@aaronsteers)
Devin session: https://app.devin.ai/sessions/1145d85639d849b396b2021ba80433b8
Summary by CodeRabbit
Release Notes