Add OpenAI Agents MCP v2 support - #6
Conversation
Migrated-From: temporalio/sdk-python@a43349a
A failed operation evicted the pooled connection by closing it outright, which unwound the owning task's `async with backend` while concurrent operations were still using the same connection. Since `CancelledError` is a `BaseException`, activity cancellation and worker shutdown hit this path too, so cancelling one MCP activity aborted every other in-flight activity sharing the connection. The failing operation also never released its slot. Failures now retire the record: it is dropped from the cache so no later operation reuses it, and the last operation to release it closes it. Also drop a dead uniqueness check in `_MCPActivities` (`set()` over a dict compares key counts, so it never fired) and correct the deprecation directives to 1.32, the unreleased version. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Migrated-From: temporalio/sdk-python@4fb9cc8
`temporalio.contrib.mcp` is an implementation detail shared by the contrib integrations, not a public API, but it was advertised as one: the CHANGELOG called it stable, `workflow.py`/`MCPClient` were the only non-underscore names in an otherwise private package, and pydoctor rendered the package into the published docs. Rename to `_workflow._MCPClient`, mark the package PRIVATE for pydoctor, and describe the release in terms of the OpenAI Agents surface users actually call. `meta` was threaded through all seven operations, but only `call_tool` has a caller that supplies it (the OpenAI Agents base `MCPServer` resolves it per tool call). The other six accepted it and dropped it on the floor, so a caller passing metadata would silently get none. Drop the parameter there; `_CallToolRequest` now owns the field. Replace the `-k 'legacy_mcp_apis_are_deprecated or mcp_server'` CI selection with `@pytest.mark.mcp_v1`. Substring matching silently under-selects on a rename while still passing, and `mcp_server` also matches `mcp_servers` and any future `test_hosted_mcp_server_*`. The marker selects the same 13 tests today and stays correct as tests are renamed or added. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Migrated-From: temporalio/sdk-python@8b74c9e
An idle-eviction task armed before an operation acquired a pooled record could close the connection while that operation was still in flight: once a peer failure unmapped the record, `_unmap` skipped the idle check and reported the record closable. Apply the idle check regardless of whether the record is still mapped. Restore the legacy `inspect.signature` handling for MCP server factories, now shared by both backends. A factory declaring a positional parameter receives the `factory_argument` (`None` when a workflow supplied none), and a parameterless factory is called bare. Signatures that could satisfy neither form raise when the plugin is built, and passing a `factory_argument` to a parameterless factory raises a non-retryable error instead of a bare `TypeError` that retries forever. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Migrated-From: temporalio/sdk-python@0168e19
Migrated-From: temporalio/sdk-python@51d4aff
Fix the proto Docker build, which pinned googleapis-common-protos into the dev group while the conflicting requirement now lives in dev-common, leaving the following uv sync unsatisfiable. Bound the MCP pool close during worker shutdown. The run context swallows the shutdown cancellation, so an MCP server that never finishes closing its transport would hang the worker with no cancellation left to break out with. Stop treating activity cancellation as a transport failure in the connection pool. The MCP client cancels the in-flight request on the wire, so the shared connection stays healthy and should not be retired out from under every other workflow using it. Reject a callable tool_filter on a worker-side MCPServer. It needs the run context and agent, which exist only in the workflow, so the Agents SDK raises for it and the list-tools Activity would retry forever. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Migrated-From: temporalio/sdk-python@7100059
…cb4e8e (history preserved; git-filter-repo 2.47.0)
brianstrauch
added
the
history-import
brianstrauch
removed
the
history-import
…andbox, use the public MCP client import exclude-newer = "2 weeks" limits candidates to distributions uploaded before the cutoff, so a freshly published temporalio-mcp would have no eligible release for two weeks and uv lock would keep failing after the release; exempt it like temporalio. MCPPlugin passes mcp_types through the sandbox and the adapter imports it directly, so OpenAIAgentsPlugin does the same. TemporalMCPClient is a public export of temporalio.contrib.mcp; import it from there instead of the private module.
…mported-files rule The adapter was developed in sdk-python PR #1793, closed unmerged on 2026-09-03, and migrated here with Migrated-From trailers. Its commits touch ten imported files and can never arrive from sdk-python main, so AGENTS.md, CONTRIBUTING.md, scripts/migrate/README.md and IMPORTS.md now say so, and future re-syncs merge on top of them. The cooldown docs cover the second exclude-newer-package exemption.
DABH
added
the
history-import
DABH
left a comment
DABH
left a comment
There was a problem hiding this comment.
Reviewed at beed6dd (Brian's ae8d088 plus two adaptation commits I pushed today; details below). Overall the adapter reads well: the legacy v1 surface and activity names are preserved (verified symbol by symbol against main), the re-sync from the closed sdk-python#1793 did not regress anything on main (_invoke_model_activity.py and the retry-after tests are identical), and the five new tests run offline. Because the lock could never resolve, no lint, type-check or test has run in CI on this branch yet; the first uv lock after the temporalio-mcp release will be the first signal.
What I pushed (shape-independent, per the plan David approved):
- 88367ad:
exclude-newer-package = { temporalio = false, temporalio-mcp = false }(uv limits candidates to distributions uploaded before the two-week cutoff, so a freshly publishedtemporalio-mcpwould otherwise have zero eligible releases for 14 days anduv lockwould keep failing after the release);mcp_typesadded to the sandbox passthrough for parity withMCPPlugin;TemporalMCPClientimported from the publictemporalio.contrib.mcp. - beed6dd: AGENTS.md / CONTRIBUTING.md / migrate README / IMPORTS.md record this adapter as the one exception to "never edit imported files" (ten imported files, closed upstream PR, non-descendant
SRC_REF), and the cooldown docs mention the second exemption. Labelhistory-importadded.
One decision for you (D1), left untouched on purpose: temporalio-mcp>=0.1.0,<0.2 and mcp-types>=2,<3 are hard dependencies, which forces mcp>=2; but mcp>=1.24,<3, the ImportError fallbacks, the README's "can still run with MCP Python SDK v1", the mcp_v1 marker (no lane runs it here) and test_mcp_servers_requires_v2's < 2 branch all assume MCP v1 stays installable. As written, nobody pinned to mcp 1.x (anything next to google-adk, which pins mcp<2) can install the package. Either (a) keep the hard dependency and make the text coherent (mcp>=2,<3, reword the v1 claims, drop the dead branch and marker), or (b) make temporalio-mcp an optional [mcp] extra, which matches the lazy imports and AGENTS.md's "lazy imports go in an extra" rule and keeps mcp 1.x users installable during the deprecation period. I lean (b) but it is your call; inline comments mark the exact sites.
Sequencing: #5 merges with a merge commit (Thursday), then python/mcp/v0.1.0 (held until the repo is public), then uv lock here, make sync && make lint && make test && make build, merge main (expect a one-line conflict with the litellm fix next to openai-agents[litellm]), mark ready. The 1.0.0rc1 dry run consumes that version, so this PR's own pre-release will need a 1.0.0rc2 release PR.
Smaller items inline: deprecation version text, the shared activity namespace, the ResponseBuilders.tool_call default change, private cross-package imports, and adapter test coverage.
brianstrauch
removed
the
history-import
…ents-mcp-v2 # Conflicts: # CONTRIBUTING.md # python/mcp/README.md # python/mcp/plugin.toml # python/mcp/src/temporalio/contrib/mcp/_activities.py # python/mcp/src/temporalio/contrib/mcp/_backend.py # python/mcp/src/temporalio/contrib/mcp/_client.py # python/mcp/src/temporalio/contrib/mcp/_pool.py # python/mcp/src/temporalio/contrib/mcp/_workflow.py # python/mcp/tests/test_activities.py # python/mcp/tests/test_pool.py # python/mcp/tests/test_workflow.py # python/mcp/uv.lock # python/openai_agents/pyproject.toml
brianstrauch
added
history-import
…ents-mcp-v2 # Conflicts: # python/openai_agents/pyproject.toml # scripts/ci/check_conventions.py
There was a problem hiding this comment.
🟡 Changes recommended
Pagination fails when a later valid response omits its TTL hint.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds durable MCP Python SDK v2 support to the OpenAI Agents integration while preserving legacy MCP APIs and workflow-history compatibility.
Changes:
- Adds worker-side MCP factories, workflow proxies, connection reuse, and protocol handling.
- Updates dependencies, documentation, and MCP-related tests.
- Clarifies history-import policy and removes the commit-count heuristic.
File summaries
| File | Description |
|---|---|
AGENTS.md |
Documents migration and dependency rules. |
CONTRIBUTING.md |
Clarifies history-import eligibility. |
.github/workflows/ci.yml |
Removes commit-count metadata. |
scripts/ci/check_conventions.py |
Removes commit-count enforcement. |
scripts/tests/test_check_conventions.py |
Updates convention tests. |
scripts/migrate/README.md |
Documents default-branch import requirements. |
python/README.md |
Documents first-party dependency exemptions. |
python/openai_agents/plugin.toml |
Records the local adapter divergence. |
python/openai_agents/pyproject.toml |
Adds MCP v2 dependencies and extra. |
python/openai_agents/uv.lock |
Locks updated MCP and Agents packages. |
python/openai_agents/README.md |
Documents MCP v2 configuration and lifecycle. |
python/openai_agents/src/temporalio/contrib/openai_agents/_mcp.py |
Deprecates legacy MCP providers. |
python/openai_agents/src/temporalio/contrib/openai_agents/_mcp_backend.py |
Adds the worker-side MCP adapter. |
python/openai_agents/src/temporalio/contrib/openai_agents/_openai_runner.py |
Accepts the durable MCP v2 proxy. |
python/openai_agents/src/temporalio/contrib/openai_agents/_temporal_mcp_server.py |
Adds the workflow-side MCP server proxy. |
python/openai_agents/src/temporalio/contrib/openai_agents/_temporal_openai_agents.py |
Registers and manages MCP v2 activities. |
python/openai_agents/src/temporalio/contrib/openai_agents/testing.py |
Extends testing helpers for MCP v2. |
python/openai_agents/src/temporalio/contrib/openai_agents/workflow.py |
Exposes temporal_mcp_server(). |
python/openai_agents/tests/contrib/openai_agents/test_mcp_v2.py |
Adds MCP v2 integration coverage. |
python/openai_agents/tests/contrib/openai_agents/test_openai.py |
Updates IDs and deprecation coverage. |
python/openai_agents/tests/contrib/openai_agents/test_openai_sandbox.py |
Uses unique tool-call IDs. |
Review details
- Files reviewed: 20/21 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
DABH
left a comment
DABH
left a comment
There was a problem hiding this comment.
Did a full review pass on head c69c44e, including running the whole local toolchain (make sync/lint/test — 227 passed 2 skipped, matching the PR body — plus the scripts suite, check_conventions, and wheel checks, all green) and verifying the code against the installed locked dependency surfaces (agents 0.20 signatures, mcp-types field types, temporalio-mcp private API contract). All nine earlier review threads' fixes are verifiably present on this head, and Copilot's ttlMs concern is refuted with a regression test (details in that thread) — those can all be resolved.
One thing I'd fix pre-merge, because re-sync correctness depends on it:
- The divergence policy is now self-contradictory.
scripts/migrate/README.mdsays the MCP v2 adapter files to preserve are "listed in AGENTS.md, 'Transition rules'" — but that section lists no files, and still says adaptation-only changes "are the only local commits" / "never cherry-pick or patch imported files", both of which this PR (correctly) violates for seven imported files plus two new ones. The next upstream re-sync has no authoritative list of what to preserve. Suggest enumerating the diverged/new files in the Transition rules section and carving the documented-divergence exception into those two bullets.
Smaller items, fine as follow-ups:
_TemporalMCPServer.cached_toolsreturns the client cache unfiltered whilelist_toolsapplies the static tool filter. No agents-0.20 code path reads the property, so it's latent — but worth filtering or documenting.- The repeated-cursor detection string-matches agents' error text (verified to match both 0.20 wordings, and the pin is <0.21, so safe today) — but the test fabricates the message itself, so an upstream rewording would silently degrade the error to retryable without failing any test. Worth a line in the version-bump checklist or one test asserting through the real agents error path.
get_promptargument stringification (str(True)→ "True", dicts → repr) diverges from native agents passthrough — forced by thedict[str, str]client signature; a docstring note would do.- A dynamic
tool_filtercallable runs workflow-side and must be deterministic; neither README nor docstring says so — one sentence in thetemporal_mcp_server()docstring would cover it. - The "legacy v1 APIs still work without the extra" claim has no CI lane exercising an mcp 1.x environment, so a v1 regression would ship blind.
None of these block my overall take: the adapter matches the dependency surfaces exactly, workflow-side code is replay-safe (no nondeterminism, sandbox passthrough correct, legacy activity names byte-identical to main), and the legacy/v2 activity namespaces can't collide.
|
Addressed the four non-CI follow-ups in ac5459b:
The “Divergence policy is now self-contradictory” concern is resolved by #23. That cutover PR removes |
Summary
temporalio-openai-agentsintegrationMCPServerfactories on the worker and expose them throughtemporal_mcp_server()in workflow codehistory-importfor commits reachable from an upstream repository's default branch and remove the commit-count heuristicThe adapter is based on work proposed in temporalio/sdk-python#1793, but that PR was never merged into sdk-python's default branch. Per this repository's policy, the adapter is ordinary local work, not a history import; it does not receive a
history-importlabel or anIMPORTS.mdrow.Dependency
Depends on #5. The branch now resolves the published
temporalio-mcp==0.1.0package from PyPI and records it inpython/openai_agents/uv.lock; it does not use a local path or workspace source.Testing
Validated against published
temporalio-mcp==0.1.0:Merge
This is an ordinary local-work PR, not a history import. Squash merge it per
CONTRIBUTING.md.