[campoy]

What was changed

temporalio-strands-agents now runs its MCP activities through temporalio-mcp, as
temporalio-openai-agents does, and works with the MCP Python SDK 2. 0.1.0 capped mcp<2
because its own activities read mcp 1 field names (nextCursor) and failed with mcp 2.

• StrandsPlugin(mcp_clients=...) takes mcp.Client factories instead of Strands MCPClient
  factories. It builds a temporalio.mcp.MCPPlugin and registers its activities and
  run_context, so users still add one plugin. Factories are called with no arguments, and one that
  does not return an mcp.Client fails each activity with a retryable error that says so.
  mcp_connection_idle_timeout defaults to five minutes and passes through as in MCPPlugin and
  OpenAIAgentsPlugin: None keeps connections until the worker shuts down, zero closes them when
  idle.
• TemporalMCPClient wraps temporalio.mcp.TemporalMCPClient and keeps its activity options. It
  now takes prefix and tool_filters, which used to come from the worker-side MCPClient, and
  applies them as Strands' MCPClient does: callables, compiled patterns and names, ignoring other
  entries.
• TemporalMCPTool subclasses Strands' MCPAgentTool, with the durable client as mcp_client,
  so its name, spec and callable filters work as in Strands. It turns the CallToolResult into
  Strands' MCPToolResult in the workflow, as Strands' MCPClient does: text, images, embedded
  resources, structured content, metadata and isError; an image it cannot map returns
  Tool execution failed: ..., and a URL elicitation error carries its elicitations. A tool input
  that is not a JSON object fails the call before the activity runs.
• The plugin's own connection pool and its tests (test_mcp_connection_eviction.py) are gone;
  temporalio-mcp has its own. So are encode_tool_result/decode_tool_result, which only the
  old call-tool activity used.
• pyproject.toml: the base mcp range becomes >=1.23,<3 (what strands-agents requires),
  since the package imports mcp.types. A new mcp extra adds mcp>=2, mcp-types and
  temporalio-mcp>=0.2.0,<0.3, as in openai_agents. The dev group has the same three, and
  temporalio-mcp is exempt from the cooldown. The lock moves mcp to 2.1.1 (strands-agents
  caps it below 2.2) and adds temporalio-mcp 0.2.0.
• Tests: the test MCP servers use mcp 2's MCPServer, and the MCP tests use mcp.Client factories
  and the new activity names. test_mcp uses a factory with a captured default, and
  test_mcp_reuses_connection checks that worker shutdown closes the cached connection. The prefix
  tests target the workflow-side prefix and filters. New cases cover images that don't map, URL
  elicitation errors, non-object input, filter entries Strands ignores, and a factory that returns
  Strands' MCPClient. The idle-timeout test
  uses a zero timeout and counts connections, since the old pool it polled is gone. The marker test
  checks only the metadata keys the tool set, because an mcp 2 server adds its own serverInfo
  entry.
• README: the MCP section covers the extra, mcp.Client factories, prefix and tool_filters,
  multi-round calls, errors, timeouts and retries, what goes into history, connection reuse, the
  activity names shared with MCPPlugin, and the upgrade.

This replaces the _field change this PR started with.

Breaking change:

• MCP needs the mcp extra and the MCP Python SDK 2. MCP tools no longer work with mcp 1.x.
• mcp_clients factories return mcp.Client. Set prefix and tool_filters on
  TemporalMCPClient, and connection options such as elicitation_callback or
  read_timeout_seconds on mcp.Client. Strands MCPClient options with no mcp.Client
  counterpart, such as startup_timeout, no longer apply.
• A callable in tool_filters now runs in the workflow, so it must be deterministic.
• Activity names change from {server}-list-tools and {server}-call-tool to
  temporalio.mcp.{server}.list-tools and temporalio.mcp.{server}.call-tool, and the plugin also
  registers the other temporalio-mcp operations. Registering the same server name through
  MCPPlugin or OpenAIAgentsPlugin on the same worker fails.
• A lost MCP connection now fails the activity attempt, which is retried under the activity's retry
  policy. Before, the model got Tool execution failed at once. JSON-RPC errors still reach the
  model as a failed tool result when a retry cannot fix them, such as an unknown tool; internal errors
  and timeouts are retried like a lost connection. With no timeout set, the activities get temporalio-mcp's
  one-minute start_to_close_timeout, where before they failed to schedule.
• Only servers that negotiate the modern MCP protocol share a connection. A server on an older
  protocol, such as one built on mcp 1.x, gets a new connection for every activity; before, one
  session per server was reused.
• mcp_connection_idle_timeout=None keeps connections until the worker shuts down; before, it meant
  five minutes.
• The listing is filtered in the workflow, so each listing records the server's full tool list in
  history.

The title starts with Breaking: because release notes come from commit subjects.

There is no version gate. A run that used MCP tools before this version schedules
temporalio.mcp.{server}.list-tools on replay where its History has {server}-list-tools, and
fails with [TMPRL1100]. Open runs fail their Workflow Tasks; finished runs fail when they are
replayed. Runs without MCP tools are not affected.

Upgrade: let runs that use MCP tools finish on the earlier version before you upgrade, or keep
them there with Worker Versioning. The same applies to rolling back.

Why?

See #189. The review asked for temporalio-mcp instead of field-name handling, and for one plugin
for users, like the OpenAI Agents plugin.

Checklist

1. Closes [Bug] strands_agents: MCP tool listing fails with mcp 2, and 0.1.0 caps mcp<2 instead of supporting it #189, closes [Bug] strands_agents: an MCP tool call that raises returns a tool error at once, so the call Activity's retry policy never applies #375

2. How was this tested:

   • On main with mcp 2.1.1, the 4 Workflow tests in tests/test_mcp.py time out, and [Bug] strands_agents: MCP tool listing fails with mcp 2, and 0.1.0 caps mcp<2 instead of supporting it #189's
     one-file check fails the listing with 'ListToolsResult' object has no attribute 'nextCursor'. With the change, the full suite passes with the lock (mcp 2.1.1,
     strands-agents 1.56.0): 170 tests.
   • The new test cases fail on the previous head (719695a): the captured-default factory times out,
     and the image, elicitation, non-object input and filter cases fail their assertions. The
     shutdown check times out when run_context is not passed.
   • A multi-round tool, which returns InputRequiredResult once and then finished, run through a
     Worker returns finished from one temporalio.mcp.{server}.call-tool Activity. Before, it
     failed with Handler returned an invalid result.
   • Replay check, with a control that adds an Activity and fails with [TMPRL1100] on both
     versions: each version replays its own MCP and non-MCP histories. Non-MCP histories replay on
     the other version too, and open non-MCP runs finish across the upgrade and the rollback. MCP
     histories and open MCP runs fail across versions with [TMPRL1100] at list-tools, as listed
     above. The current head replays the histories recorded before the second review round.
   • In Docker with uv 0.12.24 (inside the plugin's required-version): uv lock --check,
     make sync, make lint (ruff, ruff format, pyright, mypy, basedpyright, pydocstyle),
     make test (170 tests) and the repository conventions check pass. CI covers the other
     platforms.

3. Any docs updates needed?
   The plugin README's MCP section.

Not covered:

• Only stdio ran. Streamable HTTP and mcp 2.0.x were not run.
• temporalio 1.34.x still bundles temporalio.contrib.strands with no mcp range, and it fails
  with mcp 2. No backport here.

[brianstrauch]

We want to use temporalio-mcp here for clean MCP v2 support

[campoy]

Makes sense, I'll move it onto temporalio-mcp. Plan: users register MCPPlugin directly, StrandsPlugin drops mcp_clients, and prefix/tool_filters move to the workflow-side TemporalMCPClient. This also gets us multi-round tool calls. Activity names change, so I'll mark it Breaking.

Or would you rather keep mcp_clients= on StrandsPlugin, like OpenAI's mcp_servers=?

[brianstrauch]

Makes sense, I'll move it onto temporalio-mcp. Plan: users register MCPPlugin directly, StrandsPlugin drops mcp_clients, and prefix/tool_filters move to the workflow-side TemporalMCPClient. This also gets us multi-round tool calls. Activity names change, so I'll mark it Breaking.

Or would you rather keep mcp_clients= on StrandsPlugin, like OpenAI's mcp_servers=?

I'd rather match the OpenAI Agents SDK plugin so users only need to install one plugin.

[campoy]

Done, PTAL @brianstrauch

[brianstrauch]

Found one factory invocation regression, detailed inline. All 125 existing tests and make lint passed locally on Python 3.14.4; the captured-default factory case was reproduced separately.

[brianstrauch]

[P2] Preserve zero-argument factory invocation

Passing the factory directly to _mcp_client_backend_factory changes how valid Callable[[], Client] factories are invoked: its _FactoryInvoker treats any positional parameter, including one with a default, as a factory_argument parameter and supplies None. For example, lambda params=params: Client(stdio_client(params)), a common way to capture each server's configuration in a loop, now receives params=None. I reproduced AttributeError: 'NoneType' object has no attribute 'command' when entering the resulting backend, so MCP activities fail and retry instead of connecting.

Please wrap these factories so they are invoked without arguments, and add a regression test for a factory with a captured default parameter.

[campoy]

Fixed in 002b63a. StrandsPlugin now wraps each factory in a function with no parameters before handing it to MCPPlugin, so lambda params=params: Client(...) is called as factory(). test_mcp now uses a factory with a captured default; on the previous head it times out because the activity keeps failing.

[DABH]

Review of the move onto temporalio-mcp. 21 deduplicated candidates were each verified by an independent pass; 18 confirmed, 3 refuted (the strands-agents>=1.51.0 floor against the mcp>=2 extra resolves cleanly under --resolution lowest-direct; the idle-timeout test gap is covered by python/mcp's own timer tests; a RuntimeError from workflow code on a base install is the standard SDK task-failure path). make lint and make test (125 tests) pass on the head; the conventions check and uv lock --check pass; base-install smoke imports were verified with and without mcp 2. Dev-server reproductions are noted where they were run.

Severity: P1 = wrong result or stuck run in realistic use; P2 = correctness or robustness in an edge case; P3 = docs, tests, cleanup.

The recurring theme is that work which used to happen worker-side (Strands' content mapping, tool filtering, error formatting, connection pooling) now happens in workflow code or in a shared pool with different semantics, and the Strands README still describes the old behavior: unfiltered listings and discarded content are recorded in history, servers on the mcp 1.x protocol get no connection reuse, call-tool failures retry without bound under the default policy, and a factory ported with prefix= hangs the first turn.

Cut for length: the stale 'both packages' note in python/mcp/_backend.py:32, the negative-idle-timeout ValueError naming MCPPlugin's keyword, the duplicated import-guard message, duplicated test factories, README prose duplicating docstrings, and the dev group restating the extra.

[DABH]

P1: unsupported images are silently dropped and the result still says success. _image returns nothing for an ImageContent whose MIME type is outside MIME_TO_FORMAT (image/svg+xml) or whose base64 does not decode, and _tool_result then reports status='success'/isError=False with empty content (verified: {'status': 'success', 'content': [], 'isError': False}); the Bedrock formatter rewrites that to [{'text': ''}], so the model believes the tool succeeded with no output. Strands' map_mcp_content_to_tool_result_content indexes MIME_TO_FORMAT[...] and b64decode unguarded and call_tool_async converts the exception to {'status': 'error', 'content': [{'text': "Tool execution failed: 'image/svg+xml'"}]}, which the pre-PR activity delegated to. The docstring 'drop the ones Strands' MCPClient drops' is wrong for ImageContent (Strands only drops embedded-resource blobs). No test covers either case. Also, _image decodes before checking the MIME type; check first.

[campoy]

Fixed in dd5d1e5. Images now map as Strands' MCPClient does: an unsupported type or data that doesn't decode makes the call return Tool execution failed: ... (for example Tool execution failed: 'image/svg+xml'), not an empty success. The MIME type is looked up before decoding. The docstring now says it drops only what Strands drops. New cases in test_mcp_image.py cover the SVG and bad base64 cases.

[DABH]

P1: filtering and projection moved from the activity into the workflow, so history now carries everything the server returns. With the default cache_tools=False, every turn records the server's full unfiltered ListToolsResult: measured 134,582 B per turn for a 90-tool server with tool_filters={'allowed': ['search_issues']}, versus 1,424 B for the old filtered projection (~95x), about 4 MB over 30 turns for one surviving tool. Call-tool results carrying content the workflow then discards (AudioContent, ResourceLink, unsupported-MIME images, non-text blobs) are stored verbatim, and when that content exceeds the payload limit the activity cannot complete at all: a tool returning a text block plus ~2.1 MB of base64 audio logs [TMPRL1103] Attempted to upload payloads with size that exceeded the error limit on every attempt (dev-server reproduction), and the model never sees the text block, where main's activity returned the already-mapped Strands result and completed. The README presents cache_tools=True only as 'one fewer activity per turn' and says nothing about history size or recorded-but-dropped content. Either project worker-side again (a thin activity wrapper around temporalio-mcp's call), or document the cost and recommend cache_tools=True plus filters.

[campoy]

Documented, not moved back to the worker. Filtering in the workflow is how the OpenAI Agents plugin uses temporalio-mcp too, and this PR moved onto its activities on purpose. The README (d4d1111) and the TemporalMCPClient docstring (dd5d1e5) now say that each listing records the full tool list before tool_filters apply, and recommend cache_tools=True for servers with many tools. They also say that call-tool results are recorded as returned, including content the agent doesn't get, and that a result over the payload limit fails the activity.

[DABH]

P1: connection reuse is gone for every server on the mcp 1.x protocol, and the idle timeout is a no-op for them. temporalio-mcp's _MCPClientBackend.cacheable is True only for protocol 2026-07-28; a server built on the mcp 1.x SDK rejects server/discover, the client falls back to initialize at 2025-11-25, and the pool gives it a fresh client, stdio subprocess spawn and handshake per list-tools and per call-tool. Reproduced with main's FastMCP echo server under mcp 1.29.0 against this venv's mcp 2.1.1 client: factory_calls 1/2/3 across three pool operations at ~240-270 ms each, while the PR's mcp-2 echo server reuses one connection (second op 1 ms). With cache_tools=False a Strands agent pays a subprocess spawn for list-tools plus one per tool call on every turn (HTTP transports re-initialize per operation). The deleted Strands pool cached one ClientSession per server regardless of protocol version; the README (line 652) and this docstring still promise a shared, idle-evicted connection and the PR body says the option 'keeps its meaning'; test_mcp_reuses_connection only exercises a modern server so CI cannot see it. python/mcp's README documents the exclusion; the Strands docs do not and recommend nothing for legacy servers.

[campoy]

Documented, matching the OpenAI Agents plugin, which has the same temporalio-mcp limit. The StrandsPlugin docstring (002b63a) and the README (d4d1111) now say that only servers on the modern protocol share a connection, and that an older-protocol server, such as one built on mcp 1.x, gets a new connection per activity (a new process for stdio). I also took "keeps its meaning" out of the PR description.

[DABH]

P1: the migration note omits the prefix/tool_filters move, and a factory ported as-is hangs the first turn. The old README told users to put prefix=/tool_filters= on the worker-side factory; they now belong on the workflow-side TemporalMCPClient. A worker upgraded from 0.1.0 keeping "gh": lambda: Client(stdio_client(...), prefix="gh") constructs the plugin and Worker cleanly (the factory signature has no parameters, so nothing inspects it); the first BeforeModelCallEvent schedules temporalio.mcp.gh.list-tools, the pool calls the factory, TypeError: Client.__init__() got an unexpected keyword argument 'prefix' is raised (verified), _activities._run maps only MCPError/ValidationError, so the SDK records a retryable ApplicationError(type='TypeError') and, with the README's start_to_close_timeout only and the default unlimited retry policy, retries forever; failed records are never cached, so every attempt re-invokes the factory. Conversely, dropping the kwarg without adding it to the handle silently merges same-named tools from different servers via ToolRegistry.replace. The PR body lists the move; this note, which users read, mentions only the activity-name change.

[campoy]

Fixed in d4d1111. The upgrade note now lists the steps: return mcp.Client, and move prefix and tool_filters to TemporalMCPClient. It says a factory still passing prefix= fails every activity with a retried TypeError, and that without a prefix same-named tools replace each other. It also notes that callable filters now run in the workflow, that Strands-only options such as startup_timeout no longer apply, and that runs must finish before the upgrade.

[DABH]

P2: URL-elicitation-required errors lose their payload. On servers negotiating protocol 2025-11-25, -32042 still travels as a JSON-RPC error; mcp 2.1.1's Client has no special handling and raises MCPError(code=-32042, data={'elicitations': [...]}), which temporalio-mcp maps to a non-retryable ApplicationError('URL elicitation required', -32042, {...}, type='MCPProtocolError') with the URLs in details[1]. _activity_error_event builds the tool result from ApplicationError.message only, so the agent receives {'status': 'error', 'content': [{'text': 'URL elicitation required'}]} with no URL and cannot ask the user to authorize (verified), whereas Strands' MCPClient._handle_tool_execution_error and the pre-PR activity formatted the URLs into the text. In the except ActivityError branch, read the cause's details when type == 'MCPProtocolError' and details[0] == URL_ELICITATION_REQUIRED, and format details[1] as Strands does.

[campoy]

Fixed in dd5d1e5. When the call-tool failure is an MCPProtocolError with code -32042, the tool result now carries the elicitations in Strands' format: MCP Elicitation required: [URL elicitation required] with data [...]. Other errors keep the message-only result. A new case in test_tool_errors.py covers it.

[DABH]

P3: the activity namespace now collides with MCPPlugin and OpenAIAgentsPlugin. Registering the same server name through StrandsPlugin and either of those on one worker fails at construction: Worker(plugins=[StrandsPlugin(mcp_clients={'github': f}), MCPPlugin({'github': f})]) raises ValueError('More than one activity named temporalio.mcp.github.list-tools') (live run). Startup-time and clearly worded, but the OpenAI README documents this collision for its plugin while the Strands README documents neither it nor its flip side: StrandsPlugin's registration alone already serves temporalio.mcp.TemporalMCPClient for prompts and resources, so a user following temporalio-mcp's README to add MCPPlugin for those hits the error with no guidance.

[campoy]

Documented in 002b63a and d4d1111. The StrandsPlugin docstring and the README say that these are MCPPlugin's activities, so the same server name must not be registered through MCPPlugin or OpenAIAgentsPlugin on the same worker. The README also says temporalio.mcp.TemporalMCPClient can use the servers registered here, for prompts and resources for example.

[DABH]

P3 (design): the worker side is wired through underscore-private temporalio-mcp internals when the public MCPPlugin wraps exactly those two calls. _activities._MCPActivities and _client._mcp_client_backend_factory can change at any time inside the >=0.2.0,<0.3 pin (cooldown-exempt via exclude-newer-package), guarded only by 'coordinated release' docstrings, and python/mcp/_backend.py:32 still says 'across both packages'. Verified: MCPPlugin({...}).activities yields the identical seven activities and .run_context is the same bound method; a throwaway Worker(plugins=[StrandsPlugin(models=...), MCPPlugin({'echo': ...})]) using Strands' TemporalMCPClient passed with the same history, one reused connection, and a clean Replayer with both plugins. Unlike openai_agents, Strands injects no custom backend adapter, so the private surface buys nothing. Build an MCPPlugin inside StrandsPlugin and extend its activities, run_context and workflow_runner (which already adds the mcp/mcp_types passthrough duplicated at line 153), keeping the lazy-import guard and the idle-timeout mapping; or document the two-plugin composition. MCPPlugin is itself marked experimental, so this is public-experimental versus private rather than stable versus private.

[campoy]

Done in 002b63a. StrandsPlugin builds a temporalio.mcp.MCPPlugin and registers its activities and run_context, so users still add one plugin and nothing private is imported. The two python/mcp docstring edits are reverted. I kept Strands' own workflow runner, which already passes mcp and mcp_types through along with strands.

[DABH]

P3 (tests): the only Strands-level coverage of the MCP run_context wiring is deleted without replacement. The post-shutdown assert "echo_cached" not in _CONNECTIONS here and test_mcp_connection_eviction.py's shutdown test are gone, and factory_calls == 1 cannot distinguish 'reused' from 'never closed'. Mutation experiment: changing _plugin.py:139 to run_context=None leaves all four MCP test files (30 tests) and make test (125) green, while a script reproducing test_mcp_reuses_connection shows the echo server child still alive 5 s after the Worker exited (pristine branch: gone within 2 s). In production every worker shutdown cycle would leak one live MCP server subprocess or HTTP session per configured server. A test factory returning a Client subclass that records __aexit__ (or observing the child process) asserted after the async with Worker(...) block covers it.

[campoy]

Added in 002b63a. test_mcp_reuses_connection's factory now returns a Client subclass that records __aexit__, and the test checks that worker shutdown closes the cached connection. Leaving async with Worker doesn't wait for the plugin's run_context cleanup, so the test waits up to 10 s for the close. With run_context=None it times out.

[DABH]

P3: TemporalMCPTool hand-copies MCPAgentTool. Lines 36-75 are line for line mcp_agent_tool.py:34-99 minus the _compat helpers (__init__, tool_name, tool_spec, the {prefix}_{name} rule); MCPAgentTool only touches mcp_client inside stream, the one method this class overrides anyway. Verified in the venv: class TemporalMCPTool(MCPAgentTool) with super().__init__(tool, cast(Any, client), name_override=...), a tool_type override and the existing stream body passes every test_mcp_prefix.py assertion with byte-identical tool_spec/tool_name across the five cases, makes isinstance(..., MCPAgentTool) True and mcp_client resolve to the durable client (which fixes the ported-filter case at line 155). The copy drifts from upstream's evolving spec derivation (its own SEP-1984/SEP-1913 comment) under the floating strands-agents bound, with no parity test to catch it; on main subclassing was impossible (no mcp.types.Tool in the workflow), and this PR's rewrite with a real Tool is what makes it feasible.

[campoy]

Done in dd5d1e5. TemporalMCPTool subclasses MCPAgentTool, passing the durable client as mcp_client and name_override for the prefix. It keeps only tool_type (temporal_mcp, as before) and stream. The name and spec now come from Strands, and the existing test_mcp_prefix.py assertions pass unchanged.

[DABH]

P3 (cleanup): the function-local from mcp.types import (...) (run per content block) and the TYPE_CHECKING-only CallToolResult/Tool imports buy no optionality: import temporalio.strands_agents already puts mcp and mcp.types in sys.modules through strands.tools.mcp, strands-agents hard-requires mcp, and all seven names exist on mcp 1.x (verified on 1.29.0). A reader infers an optional dependency that does not exist, typing.get_type_hints raises NameError on StrandsPlugin.__init__, TemporalMCPTool.__init__ and _tool_result for doc/IDE tooling, and the base manifest no longer declares a dependency the import graph pulls in unconditionally (AGENTS.md: 'Declare exactly what the package imports at module level'; openai_agents keeps a base mcp pin with the extra narrowing it). Move the mcp.types names to module level next to the MIME_TO_FORMAT import, keep Client/DurableMCPClient under TYPE_CHECKING (those guards are load-bearing for a base install), and consider restoring a base mcp>=1.23,<3 pin. Related dead state: the inner DurableMCPClient(cache_tools_list=cache_tools) cache at _temporal_mcp_client.py:98 has no caller outside _refresh, which sits behind the outer _cache_tools/_fetched guard, and _server duplicates self._client.name.

[campoy]

Done in dd5d1e5. The mcp.types names are now imported at module level, and Client/DurableMCPClient stay under TYPE_CHECKING. mcp>=1.23,<3 is back in the base dependencies, the floor strands-agents requires, and the mcp extra still needs mcp 2. The durable client no longer caches the listing itself. I kept _server: it backs the public server property, and the open #184 also reads it, so dropping it now would only add a merge conflict.

[campoy]

This round is in d4d1111, on top of a merge of main, and each thread has a reply. On the review summary's cut-for-length items:

• The "both packages" note in python/mcp/_backend.py is accurate again: Strands now uses the public MCPPlugin, and this PR no longer touches python/mcp.
• The rest are left as they are. The negative-timeout ValueError comes from MCPPlugin, and the dev group restating the extra matches openai_agents. The duplicated guard message, test factories and README prose are small.

[DABH]

Second round, at head d4d1111 (002b63a, dd5d1e5 and d4d1111 on top of a merge of main). In a worktree at the head: make sync, make lint (ruff, ruff format, pyright, mypy, basedpyright, pydocstyle) clean, make test 169 passed; CI is green. Findings were verified with a temporary test file run against the dev server (removed afterwards): a bounded retry policy with a server that is down, a factory left returning Strands' MCPClient, and the mutation plugin.run_context = None. I also compared the new workflow-side mapping with Strands 1.56's MCPClient source, and checked the base-install floor: mcp 1.23.0 defines every mcp.types name now imported at module level, strands-agents 1.51 exports MIME_TO_FORMAT and ToolFilters, and the mcp extra forces strands-agents 1.55 or later (1.51 to 1.54 cap mcp<2), whose MCPAgentTool.tool_spec reads mcp 2 tools through _compat. Refuted and dropped: a parity gap in content mapping (text, image, embedded-resource and the KeyError text all match map_mcp_content_to_tool_result_content/_handle_tool_execution_error, including the snake_case elicitation_id that Strands also emits); lost trace-context propagation into _meta (main's activity called session.call_tool directly, so nothing changes); and Strands special-casing MCPAgentTool instances outside strands.tools.mcp (it does not). Not verified here, as the PR body says: streamable HTTP and mcp 2.0.x.

Severity: P1 = wrong result or stuck run in realistic use; P2 = correctness or robustness in an edge case; P3 = docs, tests, cleanup.

Previous rounds. Addressed: an image that cannot be mapped now returns Tool execution failed: 'image/svg+xml' (the MIME type is checked before decoding, and the case is tested); the upgrade note lists the factory, prefix/tool_filters, callable-filter and option changes; the URL-elicitation result carries the elicitations in Strands' format; unbounded retries are documented with a bounded example (but see the P2 below); non-object input is rejected before scheduling; callable filters get an MCPAgentTool and entries are matched in Strands' order, with other entry types ignored; None idle timeout passes through; the activity-name collision is documented; the worker side uses the public MCPPlugin; the run_context wiring is now tested (with plugin.run_context = None the connection is still open after the test's 10 s wait, so the new assertion catches the mutation); TemporalMCPTool subclasses MCPAgentTool; the mcp.types imports are at module level, the base mcp>=1.23,<3 pin is back, and the durable client's inner cache is off. Brian's captured-default factory is fixed by _without_argument and covered by test_mcp. Declines I accept as recorded: full listings and as-returned call-tool results in History, no connection reuse for older-protocol servers, and the one-minute default timeout (all three now documented, and the same as the OpenAI Agents plugin on temporalio-mcp); no __init__ validation of filters; keeping _server; and the cut-for-length items. Nothing from earlier rounds is still open.

Where this round's items cluster: the new README text on failures describes call-tool and leaves out list-tools and the retryable protocol codes, and one upgrade mistake (a factory left unchanged) still retries without a useful message. Approving because no P1 or blocking item remains and CI is green; the three items below are non-blocking.

Related: #184 changes the same files. It refuses duplicate MCP tool names, which would make the upgrade note's 'same-named tools from different servers replace each other' stale. Its legacy-history replay fixtures also record the {server}-list-tools names this PR renames, so whichever PR lands second needs to reconcile both.

[DABH]

P2: the bounded-retry example does not hand the agent a failed tool result when the server is down at the start of a turn; it fails the workflow. The activity options apply to every MCP activity (_temporal_mcp_client.py:51), so retry_policy=RetryPolicy(maximum_attempts=3) also bounds list-tools. With this example's default cache_tools=False, that runs before every model call. Once list-tools uses up its attempts, _refresh raises ActivityError out of TemporalAgent's BeforeModelCallEvent hook (_temporal_agent.py:133-137) and through invoke_async, and the run ends as Failed. I reproduced this on the dev server with this example's settings and a factory raising ConnectionError('server down'): WorkflowFailureError caused by ActivityError / ApplicationError('ConnectionError: server down'), and the agent never got a tool result. With the same settings, a failed call-tool does become a failed tool result, as written. Main also let a failed list-tools escape, so the behaviour is not new; the promise in the README is. Suggested: say that bounding retries returns a failed tool result for tool calls, while a failed listing raises ActivityError from invoke_async. The workflow can catch it, or use cache_tools=True so that only the first turn lists. If you would rather the turn continue, keep the previous listing when a re-list fails.

[campoy]

Fixed in 7128458, in the README. The example now says only "bound the retries", and the paragraph after it separates the cases. A call-tool activity that runs out of attempts returns a failed tool result. A list-tools activity that runs out does not reach the agent, and the outcome depends on the Strands event-loop cycle. In an invocation's first cycle, as in your reproduction, the ActivityError comes out of invoke_async() and fails the workflow unless the workflow catches it. In a later cycle (after a tool call, or Strands' structured-output retry), Strands wraps it in EventLoopException: the workflow task fails on every retry and the run stays open. Our analysis ran those cases through a Worker. The second is a separate, existing problem (an Activity failure after a tool call leaves the run open), not something this PR changes; the refresh hook is the same as on main. I left the hook as it is rather than keeping the previous listing.

[DABH]

P3: 'An MCP protocol error fails the call-tool activity, which the agent also receives as a failed tool result' is only true for the non-retryable codes. temporalio-mcp marks an MCPError non-retryable only for the parse, invalid-request, method-not-found, invalid-params, header, capability, protocol-version and URL-elicitation codes (_activities.py:48-57, applied at line 109). INTERNAL_ERROR (-32603), REQUEST_TIMEOUT, CONNECTION_CLOSED and server-defined codes stay retryable. Under the default policy, a server that answers a call with -32603 is retried without limit and the agent never sees the error, the same as with a lost connection. The PR body's 'JSON-RPC errors still reach the model as a failed tool result' has the same gap. Suggested: 'Errors a retry cannot fix, such as an unknown tool or invalid arguments, reach the agent at once as a failed tool result; internal server errors, timeouts and lost connections fail the attempt and are retried under retry_policy.'

[campoy]

Fixed in 7128458 with your wording. Errors a retry can't fix, such as an unknown tool or invalid arguments, reach the agent at once. Internal server errors, timeouts and lost connections are retried under retry_policy. I corrected the PR description's sentence too.

[DABH]

P3 (hardening): a factory left returning Strands' MCPClient, probably the most common unported upgrade, retries forever with an AttributeError, and the upgrade note covers only the prefix= case. I reproduced this on the dev server with mcp_clients={'old': lambda: strands.tools.mcp.MCPClient(lambda: stdio_client(params))}. Every list-tools attempt fails with ApplicationError("AttributeError: 'MCPClient' object has no attribute '__aenter__'", type='AttributeError'), which is retryable, so under the default policy the first turn waits forever. A type checker flags the factory; untyped worker code gets no warning. create() sees every factory result. It could check isinstance(client, mcp.Client) and raise a non-retryable ApplicationError that names the upgrade step. mcp can be imported there because mcp_clients already requires the extra, and the pool re-raises the factory's exception unchanged, so the activity fails once with a clear message. Or add this case to the upgrade note next to the TypeError one.

[campoy]

Added in 652b620. The wrapper, now _client_factory, checks that the factory returned an mcp.Client. If not, it raises an ApplicationError that names the server, the returned type and the upgrade notes, for example MCP server factory 'old' returned strands.tools.mcp.mcp_client.MCPClient, not mcp.Client; .... I kept it retryable, so a run waits for the worker to be fixed instead of failing. A new test in test_plugin.py covers it, and the upgrade note now mentions the case (7128458). MCPPlugin and the OpenAI Agents plugin have the same gap, so I filed #320 for temporalio-mcp to check the same way.

[campoy]

On the review summary's note about #184: agreed. Whichever of #184 and #190 lands second will update the upgrade note's same-name line and re-record #184's legacy replay fixtures with the new activity names.

[DABH]

Third round, at head d7ccd75: 652b620 and 7128458 on top of d4d1111, plus two merges of main (7a139a1 resolved conflicts in _plugin.py and tests/test_plugin.py against #192 and #155; d7ccd75 merged cleanly). In a worktree at the head: make sync, make lint (ruff, ruff format, pyright, mypy, basedpyright, pydocstyle) clean, make test 183 passed, and the conventions check and uv lock --check pass; CI is green. Each finding below was reproduced on the dev server or traced to file:line, and the test gaps by mutation. I also ran the suite on the nightly lane's versions (strands-agents 1.61.1, mcp 2.1.1, temporalio 1.35.0), on the newest releases (strands-agents 1.63.0, mcp 2.3.0) and on the lowest set the mcp extra resolves (strands-agents 1.55.0, mcp 2.0.0): 183 passed each time, so the body's 'mcp 2.0.x not run' is now covered for stdio. make build, check_wheel.py and the smoke script pass, and the wheel installed without the extra on mcp 1.23.3 and on 2.3.0 imports cleanly, with StrandsPlugin(mcp_clients=...) and TemporalMCPClient(...) raising the documented error. Replay: a main history with MCP tools fails on this head with [TMPRL1100] Activity type of scheduled event 'echo-list-tools' does not match activity type of activity command 'temporalio.mcp.echo.list-tools' (with cache_tools on and off); an open MCP run picked up by a head worker fails its Workflow Task and stays open without scheduling anything; non-MCP histories replay in both directions and an open non-MCP run finishes across the upgrade. That is the break the PR body and README disclose. Refuted and dropped: drift in MCPAgentTool, MIME_TO_FORMAT, result mapping or filter matching between Strands 1.56 and 1.63 (there is none); the 7a139a1 resolution losing #192's guard or #155's converter wiring (both intact, and main's tests are unchanged); MCP errors under DefaultFailureConverterWithEncodedAttributes with a codec (URL elicitation, -32602 and -32603 reach the model with the right text, and replay is clean); timing flakiness in the zero-idle-timeout count and the 10 s close wait (both deterministic, and five repeated runs were green). Cross-language: TypeScript Strands still runs its own {server}-listTools/{server}-callTool activities and has no temporalio-mcp counterpart, so no shared contract changes. Not verified here, as the PR body says: streamable HTTP.

Severity: P1 = wrong result or stuck run in realistic use; P2 = correctness or robustness in an edge case; P3 = docs, tests, cleanup.

Previous rounds. Every thread has a reply, and each fix holds at this head. Brian's captured-default factory is fixed: with _client_factory bypassed, every list-tools attempt fails with AttributeError: 'NoneType' object has no attribute 'command' and test_mcp times out, and at the head it passes. His request to build on temporalio-mcp is met through the public MCPPlugin, with nothing private imported. From my last round: the README now separates a call-tool that runs out of attempts (a failed tool result) from a list-tools that does. I confirmed each case on the dev server: the first cycle raises ActivityError out of invoke_async, a later cycle gives EventLoopException and a Workflow Task failure loop, and the structured-output re-ask, an AfterModelCallEvent retry and cache_tools=True behave as written. One wording gap remains, below. The error paragraph uses the suggested wording; the first P2 below shows a class of errors it still misses. A factory left returning Strands' MCPClient now fails list-tools with MCP server factory 'old' returned strands.tools.mcp.mcp_client.MCPClient, not mcp.Client; ... (dev server: two attempts, factory called twice, then the run failed with that message under maximum_attempts=2). Keeping that error retryable is a reasonable call, and #320 tracks the same check in temporalio-mcp. Nothing from earlier rounds is still open.

Where the problems cluster: the worker now hands MCP work to temporalio-mcp, and the README states its contracts more narrowly than they are. The MCP client's own validation errors are retried without limit. Factories are called once per connection, so a shared Client breaks after its first close. A factory_argument cannot work with these servers. The rest is upgrade text and a test of the real filter path. Approving: no P1 or blocking item remains, every earlier blocking item (Brian's and mine) is fixed at this head, and CI is green. All items below are non-blocking; the first P2's real fix belongs in temporalio-mcp.

Outside the diff

• P3, docs follow-up: the published Strands page (docs/develop/python/integrations/strands-agents.mdx in temporalio/documentation) renders the python-strands-mcp-worker snippet from temporalio/samples-python's strands_plugin/mcp/run_worker.py. That snippet builds strands.tools.mcp.mcp_client.MCPClient(lambda: stdio_client(...)) and installs no [mcp] extra, so after this release it fails with the new factory error. The PR body's 'Any docs updates needed? The plugin README's MCP section.' should list both as follow-ups to land with the release.
• P3, cross-file: python/openai_agents/README.md:483-486 says OpenAIAgentsPlugin and MCPPlugin 'both' register temporalio.mcp.x.*. StrandsPlugin is now a third, and that note could name it.
• Pre-existing, not from this PR: cancelling a workflow during an MCP call-tool leaves the run open indefinitely, on main and on this head. On the dev server the run was still RUNNING 50 s after the cancel (20 s after the activity finished), with no pending activity or Workflow Task. stream turns the cancelled ActivityError into a tool result, as main's did, and the run never finishes. activity_as_tool has the same except ActivityError. It is worth its own issue.

[DABH]

P2 (non-blocking): a result that does not match the tool's declared output schema is retried without limit, although this paragraph says errors a retry cannot fix reach the agent at once. mcp 2's client checks every non-error CallToolResult against the tool's outputSchema and raises RuntimeError (mcp/client/session.py:1145 and :1155). temporalio-mcp's _run maps only MCPError and pydantic ValidationError (temporalio/mcp/_activities.py:103-117), so the activity fails with a retryable RuntimeError. Dev server, with a stdio server whose tool declares an outputSchema and answers with text only: at this head with RetryPolicy(maximum_attempts=3), call-tool failed three times with RuntimeError: Tool structured has an output schema but did not return structured content (non_retryable=False; each failure retires the connection, so every retry starts a new server process) before the agent got a result. Under the default policy the turn waits until the server's behaviour changes. On main the same server gave the agent Tool execution failed: Tool structured has an output schema but did not return structured content at once, with no failed activity, because main's activity returned any exception as a failed tool result (_temporal_mcp_client.py:386-390 at 88745dc), as Strands' MCPClient does. InputRequiredRoundsExceededError is a RuntimeError too (mcp/client/_input_required.py:40). Fix: in temporalio-mcp, raise these client-side rejections as a non-retryable MCPProtocolError (it would sit well next to #320). Here, say in this paragraph that a result the MCP client rejects, such as one that does not match the tool's output schema, is retried like a lost connection, and add it to the PR body's breaking changes.

[DABH]

P2 (non-blocking): a factory that returns one shared mcp.Client works only until its first connection closes; every later MCP activity then fails with Client is already entered; cannot reenter and is retried without limit, and 'mirroring the models= pattern' invites that, since model factories are called once and cached (line 84). temporalio-mcp calls the factory for every new connection: after each idle eviction (five minutes by default), after a failure retires a connection, and for every activity against a server on the older protocol (temporalio/mcp/_pool.py:129-137 and :206-222). An mcp 2.1.1 Client cannot be entered twice (mcp/client/client.py:454-456). Dev server with mcp_clients={'shared': lambda: shared_client} and mcp_connection_idle_timeout=timedelta(0), standing in for the eviction: list-tools succeeded, then both call-tool attempts failed with that RuntimeError (three factory calls). With the default timeout a quick test passes, and the first activity after an idle eviction (five minutes) fails. On main this shape kept working, because the pool called only the Strands client's _transport_callable() for each connection. Fix: replace the phrase with something like 'The worker calls a factory each time it opens a connection, so return a new mcp.Client and transport from every call, and do not cache it', and say the same in the StrandsPlugin docstring (_plugin.py:52).

[DABH]

P3 (non-blocking): 'That covers the first model call of every invocation' is wrong for an invocation that resumes a tool interrupt, so a workflow that catches ActivityError around the resume never sees the failure and the run stays open. On resume, Strands' first cycle replays the interrupted tool call without calling the model (strands/event_loop/event_loop.py:289-296; the same branch runs when the latest message already holds a toolUse). The first model call, and its list-tools, therefore run in the next cycle, where the outer cycle wraps the error in EventLoopException (:404-410). Dev server: a resume wrapped in try/except ActivityError ran call-tool, then list-tools failed, then the Workflow Task failed with EventLoopException. The except never ran, and the run stayed open with the task retrying. Interrupts are a documented feature of this plugin (line 470). Suggested: 'In the first cycle of an invoke_async() that starts with a model call, ...', plus 'An invocation that resumes a tool interrupt runs the approved tools first, so its first model call is in a later cycle.'

[DABH]

P3 (non-blocking): this invites temporalio.mcp.TemporalMCPClient use but does not say that a factory_argument cannot work with these servers, and following the resulting error's advice breaks the server for Strands too. The plugin wraps each factory in a function with no parameters (_plugin.py:156-175). temporalio.mcp.TemporalMCPClient('x', factory_argument='acme') therefore fails non-retryably with 'declares no parameters ... give the factory a single positional parameter to accept one' (temporalio/mcp/_backend.py:135-141). Doing that (lambda tenant: Client(...)) then fails every activity for the server, Strands' own list-tools and call-tool included, with a retryable TypeError: <lambda>() missing 1 required positional argument. The wrapper also skips MCPPlugin's construction-time signature check, which exists so that a mismatch does not become an activity failure that retries forever (_backend.py:81-89). Fix: add 'as long as it passes no factory_argument' here. Optionally, call inspect.signature(factory).bind() in _client_factory so that a factory that needs arguments fails when the plugin is built.

[DABH]

P3 (non-blocking): porting an old factory by changing only the import passes the new check, then retries forever with a TypeError that names neither the server nor the upgrade. Main's README showed lambda: MCPClient(lambda: stdio_client(...)). With from mcp import Client as MCPClient, the alias this PR's tests use, that becomes Client(lambda: stdio_client(...)). mcp.Client accepts the callable when it is built and fails when it is entered, so every activity fails with a retryable TypeError ('... object does not support the asynchronous context manager protocol'), reproduced on the dev server. Fix: add to this bullet 'pass the transport itself, a StdioServerParameters or a URL, not a function that returns the transport as Strands' MCPClient took'. Optionally, have _client_factory reject callable(client.server) and not hasattr(client.server, '__aenter__') with a message like the one for Strands' MCPClient.

[DABH]

P3 (non-blocking): the upgrade list, the only migration text users get since release notes carry just the commit subject, leaves out behaviour changes the PR body lists as breaking, and this bullet's example did not apply before either. Missing: (a) call-tool failures a retry can fix (lost connections, internal server errors, timeouts) used to give the model Tool execution failed at once; they now retry under retry_policy, without limit by default, so with the basic example above (only start_to_close_timeout) a turn now waits while the server is down; (b) mcp_connection_idle_timeout=None meant five minutes and now means connections are never evicted; (c) a server on the older protocol now gets a new connection, a new process for stdio, for every activity. Also, main never applied startup_timeout: the old pool used only _transport_callable, _elicitation_callback, prefix, tool_filters and the two _handle_* methods. It did honour a subclass override of MCPClient.map_mcp_content_to_tool_result_content, which Strands documents as an extension point, through client._handle_tool_result (main's _temporal_mcp_client.py:383-385, Strands' mcp_client.py:1435); the workflow-side mapping has no such hook. Suggested: one bullet covering (a) to (c), and for this one: 'Strands MCPClient options and subclass overrides, such as map_mcp_content_to_tool_result_content, do not apply; tool results are mapped as Strands' default MCPClient maps them.'

[DABH]

P3 (non-blocking, tests): tool_filters are tested only through the private _included, so if _refresh stopped applying them (_temporal_mcp_client.py:138) every MCP test would stay green while every listed tool, the rejected ones included, reached the model. I made that mutation (self._tools = list(tools)), and the six MCP test files plus test_installed_matches_source pass (51 tests). The full suite also stays green when isError is mapped as success, when mcp_connection_idle_timeout=None is remapped to five minutes (the round-one bug), and when StrandsPlugin stops using _client_factory (the unit test calls _client_factory directly). Suggested tests: _refresh with a stub durable client whose list_tools returns echo and delete_repo, asserting that prefix='gh', tool_filters={'rejected': ['delete_repo']} gives ['gh_echo'] from load_tools(); an _ImageServer-style stub returning CallToolResult(is_error=True, ...); a parametrized test that records connection_idle_timeout through an MCPPlugin subclass monkeypatched into temporalio.mcp; and list-tools taken from plugin.activities and run with ActivityEnvironment against an old Strands factory.