Add native MCP integration - #5
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Shared-pool shutdown can disrupt concurrent workers, and transport cleanup is not always bounded.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds the standalone temporalio-mcp plugin implementing native MCP v2 operations through durable Temporal Activities.
Changes:
- Adds workflow APIs, worker registration, connection pooling, and transport support.
- Adds packaging, provenance checks, documentation, and comprehensive tests.
- Extends migration tooling for importing MCP history from an unmerged PR.
File summaries
| File | Description |
|---|---|
README.md |
Lists and documents the MCP plugin. |
python/_template/tests/conftest.py |
Adjusts optional Agents SDK import typing. |
python/_template/tests/helpers/__init__.py |
Adds TCP port helper. |
python/mcp/.sync-identical |
Declares template-synchronized files. |
python/mcp/LICENSE |
Packages the repository license. |
python/mcp/Makefile |
Uses shared Python targets. |
python/mcp/README.md |
Documents installation and usage. |
python/mcp/plugin.toml |
Defines plugin and release metadata. |
python/mcp/pyproject.toml |
Configures packaging and dependencies. |
python/mcp/src/temporalio/contrib/mcp/__init__.py |
Exports the public API lazily. |
python/mcp/src/temporalio/contrib/mcp/_activities.py |
Implements MCP Activities and cleanup. |
python/mcp/src/temporalio/contrib/mcp/_activity.py |
Defines Activity request models and names. |
python/mcp/src/temporalio/contrib/mcp/_backend.py |
Defines backend and factory abstractions. |
python/mcp/src/temporalio/contrib/mcp/_client.py |
Adapts MCP v2 clients. |
python/mcp/src/temporalio/contrib/mcp/_plugin.py |
Registers the worker plugin. |
python/mcp/src/temporalio/contrib/mcp/_pool.py |
Pools reusable MCP connections. |
python/mcp/src/temporalio/contrib/mcp/_workflow.py |
Implements the workflow-side client. |
python/mcp/src/temporalio/contrib/mcp/py.typed |
Marks the package as typed. |
python/mcp/tests/__init__.py |
Pins the Temporal test server. |
python/mcp/tests/conftest.py |
Configures offline and provenance testing. |
python/mcp/tests/contrib/mcp/echo_mcp_server.py |
Provides a stdio test server. |
python/mcp/tests/contrib/mcp/test_activities.py |
Tests Activity operations and pagination. |
python/mcp/tests/contrib/mcp/test_backend.py |
Tests factory invocation behavior. |
python/mcp/tests/contrib/mcp/test_pool.py |
Tests pooling and cleanup races. |
python/mcp/tests/contrib/mcp/test_workflow.py |
Tests workflows and transports. |
python/mcp/tests/helpers/__init__.py |
Provides general test helpers. |
python/mcp/tests/helpers/nexus.py |
Provides the required Nexus helper. |
python/mcp/tests/helpers/plugin_meta.py |
Reads plugin metadata for tests. |
python/mcp/tests/helpers/provenance.py |
Validates installed-package provenance. |
python/mcp/tests/test_installed_matches_source.py |
Compares installed and source files. |
python/mcp/uv.lock |
Locks MCP plugin dependencies. |
python/openai_agents/tests/conftest.py |
Aligns optional import typing. |
python/openai_agents/tests/helpers/__init__.py |
Synchronizes the TCP port helper. |
scripts/migrate/IMPORTS.md |
Records MCP import provenance. |
scripts/migrate/README.md |
Documents MCP import verification. |
scripts/migrate/extract-sdk-python.sh |
Supports MCP and pull-request refs. |
Review details
- Files reviewed: 34/36 changed files
- Comments generated: 4
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
88fc43f to
6638d71
Compare
6638d71 to
9b3c2c3
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Connection-pool shutdown can race with plugin reuse, and malformed pagination can retry indefinitely.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 28/30 changed files
- Comments generated: 3
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
A failure race can reuse a retired connection, and the plugin omits required offline test isolation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
python/mcp/src/temporalio/contrib/mcp/_pool.py:152
- A failed operation marks the record retired at line 187 before
_unmap()acquires this lock. Another acquisition can enter here during that window, see the still-mapped record, increment it, and receive the transport that was just retired. Treat a retired record as absent while holding the key lock (or make retirement and unmapping atomic) so requests started after a failure create a replacement.
record = self._records.get(key)
- Files reviewed: 27/29 changed files
- Comments generated: 1
- Review effort level: Balanced
DABH
left a comment
There was a problem hiding this comment.
Reviewed at f768206. Overall this looks good to me, and I'd be happy to see it merge after a couple of small fixes.
What I checked: ran the suite locally at head (30 passed offline, no npx, no provider key); call_tool(meta=...) round-trips through the default data converter because RequestParamsMeta is a dict subclass; mcp-types is the official MCP wire-types distribution, and the direct dependency is right because _client.py imports mcp_types.version; with sdk-python#1793 closed this repo is the only home for the code, so no history-import handling applies; 0.1.0 plus allow-final = true matches the version policy for a new experimental coordinate with no SDK overlap.
Inline: two should-fix items (sandbox passthrough of mcp_types, undeclared test dependencies), one worth a decision (protocol errors retry forever under the default activity config), and a few nits that can ride along here or land with #7.
After merge: an admin needs to register pending trusted publishers for temporalio-mcp on test.pypi.org and pypi.org (workflow release-python.yml, environments testpypi and pypi) before pushing the python/mcp/v0.1.0 tag, which is what unblocks #6.
# Conflicts: # AGENTS.md
The Activity layer converted MCPError to ApplicationError outside the pool's context manager, so the pool saw every JSON-RPC error response as a connection failure and retired the shared record: an unknown tool or resource respawned the stdio server or reconnected the HTTP client for every workflow using it. An error response proves the transport works. Only CONNECTION_CLOSED retires the connection, plus REQUEST_TIMEOUT as a conservative recovery from a stuck peer; any other exception still retires it.
…rs without retry mcp 2.x defaults list_prompts, list_resources, list_resource_templates and read_resource to cache_mode="use" with caching on, so a server sending ttlMs hints could answer a second Activity from a worker-local cache; only list_tools was bypassed. Workflow history is the durable cache, so every cacheable verb now bypasses the client cache. HEADER_MISMATCH, MISSING_REQUIRED_CLIENT_CAPABILITY, UNSUPPORTED_PROTOCOL_VERSION and URL_ELICITATION_REQUIRED cannot be fixed by retrying the same request and join the non-retryable set. The README gains an errors-and-retries section (MCPProtocolError, details=[code], when to set retry_policy) and says the tool cache is per client instance. Docstrings note that temporalio-openai-agents builds on _MCPBackend, _FactoryInvoker and _MCPActivities.
activity_config replaced the default instead of extending it, so activity_config={"retry_policy": ...} scheduled Activities with no timeout at all and every workflow task failed with "Activity must have start_to_close_timeout or schedule_to_close_timeout". The one-minute start-to-close default is now added only when the config sets neither timeout.
The workflow test imports mcp_types outside imports_passed_through() so the isinstance assertion exercises MCPPlugin's sandbox passthrough rather than the test module's own. The unused _MCPClient alias is removed; temporalio-openai-agents imports TemporalMCPClient.
…ap warning, re-lock to mcp 2.1.1 temporalio.contrib.mcp is fully lazy, so smoke-importing only the root API never loaded _plugin.py or _workflow.py in the wheel checks. The root README's temporalio<=1.32 file-overlap warning applies to temporalio-openai-agents only; temporalio-mcp shares no files with the SDK and releases to PyPI. AGENTS.md keeps the Node note because one upstream openai_agents test still spawns npx. The lock moves mcp and mcp-types to 2.1.1, the newest release inside the cooldown and the version the openai_agents plugin will lock.
DABH
left a comment
There was a problem hiding this comment.
Approving at 6ab4cd3. Everything from my earlier review is addressed in d68ec60, and while finishing the PR on your behalf I pushed four small commits on top; please glance at them before merging:
- 26ac8ea Keep pooled MCP connections open on JSON-RPC error responses. The pool retired the shared record on any exception, and the
MCPErrorconversion happened outside its context manager, so a tool or resource typo respawned the stdio server / reconnected the HTTP client for every workflow sharing it. OnlyCONNECTION_CLOSED(and, conservatively,REQUEST_TIMEOUT) retire the connection now; tests cover both paths. - 53c87e4 Bypass the MCP client response cache and fail permanent protocol errors without retry. mcp 2.x defaults the other four cacheable verbs to
cache_mode="use", so onlylist_toolswas bypassed; every verb now hits the server (history is the cache).HEADER_MISMATCH,MISSING_REQUIRED_CLIENT_CAPABILITY,UNSUPPORTED_PROTOCOL_VERSION,URL_ELICITATION_REQUIREDjoin the non-retryable set. README gets an "Errors and retries" section and says the tool cache is per client instance; docstrings note the internalstemporalio-openai-agentsbuilds on. - 8f67ee0 Merge the default MCP Activity timeout into a caller's
activity_config.activity_config={"retry_policy": ...}previously dropped the default timeout and every workflow task failed with "Activity must have start_to_close_timeout or schedule_to_close_timeout". The default is added only when neither timeout is set. Themcp_typespassthrough assertion now imports outsideimports_passed_through()so it actually exercises the plugin; the unused_MCPClientalias is gone (#6 importsTemporalMCPClient). - 6ab4cd3 Smoke imports, README scope, lock.
[smoke] importsnow names_plugin/_workflow(the root package is lazy, so the wheel smoke never imported them); the root README's SDK-overlap warning is scoped totemporalio-openai-agents; the npx note stays until #8 lands; lock moved to mcp/mcp-types 2.1.1 (newest inside the cooldown, and what #6 will lock).
Verified locally: lint clean (pyright, basedpyright, mypy, ruff, pydocstyle); 37 tests pass at the lock and at the genuine floors (mcp 2.0.0, pydantic 2.12.0, uvicorn 0.31.1); make build, check_wheel.py, isolated smoke.py, conventions and tooling tests pass. CI is green on all cells.
Please merge with Create a merge commit (#6 stacks on these commits; a squash would make its later merge of main conflict on every python/mcp file). python/mcp/v0.1.0 waits until the repo is public, since every URL in the PyPI metadata 404s for the public today; environments testpypi/pypi and the release-tag ruleset are already configured. Follow-ups are in #12 and the #6 review.
Summary
temporalio-mcppackage for native Temporal workflowsMCPPluginandTemporalMCPClientfor calling MCP Python SDK v2 from native Temporal workflow codemcp.ClientfactoriesThe OpenAI Agents adapter is split into dependent follow-up #6 because each plugin must resolve from published registry coordinates in isolated CI. That PR targets
temporalio-mcp>=0.1.0,<0.2and will be unlocked after this package is released.Issue coverage
This implements the native MCP scope of temporalio/sdk-python#1056:
Each MCP request is durably represented by an Activity. Transport connections remain worker-local optimizations rather than durable sessions; process restarts reconnect while completed results replay from workflow history.
Closes temporalio/sdk-python#1056
Testing
cd python/mcp && make sync && make lint && make test && make build(37 passed; lint clean with pyright, basedpyright, mypy, ruff, pydocstyle)uv lock --upgrade --resolution lowest-direct: mcp 2.0.0, pydantic 2.12.0, uvicorn 0.31.1): 37 passed, lint cleanuv run --project scripts --locked pytest scripts/tests -q(67 passed)uv run --project scripts --locked python scripts/ci/check_conventions.pyuv run --project scripts --locked python scripts/ci/check_wheel.py --plugin-dir python/mcp --dist python/mcp/distscripts/ci/smoke.pyMerge
Merge with Create a merge commit: #6 stacks on these exact commits, and a squash would turn its later merge of
maininto add/add conflicts on everypython/mcpfile.