-
Notifications
You must be signed in to change notification settings - Fork 67
chore(mcp): add mcp-tool-test
Poe task
#789
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
chore(mcp): add mcp-tool-test
Poe task
#789
Conversation
- Add mcp-tool-test Poe task for testing MCP tools with JSON arguments - Update airbyte.mcp module documentation to reference poe mcp-tool-test method - Remove all mcp-cli references from documentation since it's not publicly available - Update bin/test_mcp_tool.py with PyAirbyte-specific usage examples - Move usage examples to docstring and reference __doc__ for help text - Document other MCP-related Poe tasks (mcp-serve-local, mcp-serve-http, mcp-serve-sse, mcp-inspect) Co-Authored-By: AJ Steers <[email protected]>
Co-Authored-By: AJ Steers <[email protected]>
- Fix line length issues in documentation examples - Add copyright notice and shebang to test script - Fix import ordering and typing issues - Make test script executable 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:
|
👋 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/1758051186-add-mcp-tool-test-poe-task' pyairbyte --help
# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1758051186-add-mcp-tool-test-poe-task' Helpful ResourcesPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
Community SupportQuestions? Join the #pyairbyte channel in our Slack workspace. |
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.
Pull Request Overview
This PR adds built-in Poe tasks for MCP testing and removes dependencies on the external mcp-cli
tool to improve developer experience.
- Adds 5 new Poe tasks for serving, inspecting, and testing MCP tools
- Creates a standalone test script for MCP tools with JSON arguments
- Updates documentation to use Poe-based workflows instead of external CLI tools
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
pyproject.toml | Adds 5 new MCP-related Poe tasks for server operations and testing |
airbyte/mcp/init.py | Replaces mcp-cli documentation with Poe task examples and simplified testing workflow |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
📝 WalkthroughWalkthroughReplaces MCP CLI-based testing guidance with Poe task-based workflows, adds a new CLI script to invoke MCP tools via FastMCP, and introduces Poe tasks in pyproject.toml to serve, inspect, and test the MCP server and tools. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant Poe as Poe Tasks
participant CLI as bin/test_mcp_tool.py
participant FMC as FastMCP Client
participant App as Airbyte MCP Server (app)
participant Tool as MCP Tool
U->>Poe: poe mcp-tool-test <tool> '<json_args>'
Poe->>CLI: Launch script with args
CLI->>FMC: Open async client to app
FMC->>App: Connect and call tool(<json_args>)
App->>Tool: Execute with provided args
Tool-->>App: Result
App-->>FMC: Tool response
FMC-->>CLI: Response object
CLI-->>U: Print result (text or str)
rect rgba(230, 246, 255, 0.6)
note over Poe,CLI: New testing flow via Poe + FastMCP
end
sequenceDiagram
autonumber
participant U as User
participant Poe as Poe Tasks
participant App as Airbyte MCP Server (app)
U->>Poe: poe mcp-serve-local | http | sse
Poe->>App: app.run(transport=stdio/http/sse, host, port)
App-->>U: Server ready
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Would you like to add brief examples in the docs showing typical Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. 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: 1
🧹 Nitpick comments (8)
airbyte/mcp/__init__.py (3)
60-75
: Document required env vars for cloud ops?Since these commands rely on inherited env, could we add a short “Environment” note listing the expected variables for cloud calls (e.g., API base URL, auth token, workspace id if needed) plus a reminder to
poetry install --all-extras
before running, wdyt?
79-84
: Make server host/port configurable in the examplesWould you add a note that
poe mcp-serve-http
/mcp-serve-sse
use 127.0.0.1:8000 by default and how to override (env vars or flags) if a port is busy, wdyt?
79-84
: Reference the newpoe mcp-inspect
earlierSince inspection helps discover tool names/schemas, should we cross-link
poe mcp-inspect
right before themcp-tool-test
examples to guide first-time users, wdyt?bin/test_mcp_tool.py (4)
34-38
: Add a timeout to avoid hanging callsIf a tool misbehaves, the CLI can hang indefinitely. Shall we wrap the await in
asyncio.wait_for
with a sensible default (e.g., 120s) and make it configurable, wdyt?Apply this diff:
-async def call_mcp_tool(tool_name: str, args: dict[str, Any]) -> object: - """Call an MCP tool using the FastMCP client.""" - async with Client(app) as client: - return await client.call_tool(tool_name, args) +async def call_mcp_tool(tool_name: str, args: dict[str, Any], timeout: float | None = 120.0) -> object: + """Call an MCP tool using the FastMCP client.""" + async with Client(app) as client: + coro = client.call_tool(tool_name, args) + return await (asyncio.wait_for(coro, timeout) if timeout else coro)
40-54
: Improve CLI UX: help flag, file/stdin JSON, and object-type validationWould you consider: (1) supporting
-h/--help
, (2) allowing JSON via@file.json
or stdin-
, and (3) ensuring args are a JSON object (mapping), wdyt?Apply this diff:
def main() -> None: """Main entry point for the MCP tool tester.""" - if len(sys.argv) < MIN_ARGS: + if any(a in ("-h", "--help", "help") for a in sys.argv[1:]): + print(__doc__) + sys.exit(0) + if len(sys.argv) < MIN_ARGS: print(__doc__, file=sys.stderr) sys.exit(1) tool_name = sys.argv[1] json_args = sys.argv[2] try: - args: dict[str, Any] = json.loads(json_args) + if json_args == "-": + raw = sys.stdin.read() + args = json.loads(raw) + elif json_args.startswith("@"): + with open(json_args[1:], "r", encoding="utf-8") as f: + args = json.load(f) + else: + args = json.loads(json_args) + if not isinstance(args, dict): + raise TypeError("Tool arguments must be a JSON object (e.g., {}).") except json.JSONDecodeError as e: print(f"Error parsing JSON arguments: {e}", file=sys.stderr) sys.exit(1) + except TypeError as e: + print(str(e), file=sys.stderr) + sys.exit(1)
58-62
: Prefer structured output for JSON-like resultsTo improve ergonomics, shall we pretty-print dict/list results as JSON and fall back gracefully otherwise, wdyt?
Apply this diff:
- if hasattr(result, "text"): - print(result.text) - else: - print(str(result)) + if hasattr(result, "text"): + print(result.text) + elif isinstance(result, (dict, list)): + print(json.dumps(result, indent=2, ensure_ascii=False)) + else: + print(str(result))
3-18
: Docstring: mention stdin/@file JSON and--help
If we add those UX tweaks, would you extend the usage examples to show
'-'
and@args.json
, plus--help
, wdyt?Example:
- poe mcp-tool-test <tool_name> '<json_args>' + poe mcp-tool-test <tool_name> '<json_args|@file.json|->' + + # read JSON from file + poe mcp-tool-test validate_config @examples/pokeapi_config.json + # read JSON from stdin + cat examples/pokeapi_config.json | poe mcp-tool-test validate_config - + # help + poe mcp-tool-test --helppyproject.toml (1)
202-208
: Tasks are great; consider hardening cross‑platform invocation and configurability
- For the
python -c
tasks, quoting can get brittle across shells/OS. Would you consider routing through a tiny runner (e.g.,airbyte.mcp.server_cli:main
) so tasks becomepoetry run airbyte-mcp --transport http --host 127.0.0.1 --port 8000
if supported, wdyt?- Do we want to allow overriding host/port via env (e.g.,
MCP_HOST
,MCP_PORT
) so users can avoid port conflicts without editing the file, wdyt?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
airbyte/mcp/__init__.py
(1 hunks)bin/test_mcp_tool.py
(1 hunks)pyproject.toml
(1 hunks)
⏰ 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.10, Ubuntu)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (No Creds)
- GitHub Check: Pytest (Fast)
🔇 Additional comments (3)
airbyte/mcp/__init__.py (1)
60-75
: Docs shift to Poe tasks reads clearly — nice!The examples are straightforward and replace the prior mcp-cli guidance cleanly.
bin/test_mcp_tool.py (1)
26-37
: Confirm FastMCP client usage against current API
Client(app)
andcall_tool(name, args)
look right, but FastMCP has had minor API shifts. Can you double-check againstfastmcp>=2.11.3
that the return shape for tool calls aligns with yourresult.text
assumption (and adjust printing accordingly if it’s.content
), wdyt?pyproject.toml (1)
207-207
: Confirm Poe arg pass‑through formcp-tool-test
Sandbox lacks
poe
(command not found); bin/test_mcp_tool.py reads sys.argv[1] and sys.argv[2] (lines 42,46–47). Can you run locally and confirm: 1)poe --version
; 2)poe mcp-tool-test list_connectors '{}'
after adding a temporaryprint(sys.argv)
to bin/test_mcp_tool.py and paste the printed argv so we can verify argv[1:] == ['list_connectors','{}'] — wdyt?
chore(mcp): add
mcp-tool-test
Poe taskSummary
This PR adds a new
mcp-tool-test
Poe task and updates MCP testing documentation to use built-in Poe tasks instead of the externalmcp-cli
tool (which is not publicly available).Key changes:
pyproject.toml
:mcp-serve-local
,mcp-serve-http
,mcp-serve-sse
,mcp-inspect
, andmcp-tool-test
bin/test_mcp_tool.py
- executable script for testing MCP tools with JSON arguments, featuring PyAirbyte-specific examplesairbyte/mcp/__init__.py
to completely removemcp-cli
references and replace with Poe task examplesThe new
poe mcp-tool-test
command allows developers to easily test both connector operations (list_connectors
,get_config_spec
, etc.) and cloud operations (check_airbyte_cloud_workspace
,list_deployed_cloud_connections
) without requiring external tooling.Review & Testing Checklist for Human
This is a medium-risk change affecting developer tooling and documentation:
poe mcp-tool-test check_airbyte_cloud_workspace '{}'
to verify cloud operations work.envrc
credentials are automatically picked up by the poe tasks (no manual export needed)poe mcp-tool-test list_connectors '{}'
to verify connector functionality workspoe mcp-serve-local
,poe mcp-inspect
to ensure they work as documentedNotes
poe check
successful)mcp-cli
tool per maintainer requestRequested by: @aaronsteers
Devin session: https://app.devin.ai/sessions/6b6e50fa185a426e966816a8f6cf46ec
Summary by CodeRabbit
New Features
Documentation
Important
Auto-merge enabled.
This PR is set to merge automatically when all requirements are met.