fix(strands): don't evict an MCP connection with a call in flight - #2259
Open
brianstrauch wants to merge 2 commits into
Open
fix(strands): don't evict an MCP connection with a call in flight#2259brianstrauch wants to merge 2 commits into
brianstrauch wants to merge 2 commits into
Conversation
The worker-process MCP connection cache armed its idle timer in `getConnection`, i.e. at the *start* of each call, and nothing extended it while the call ran. A `callTool` or `listTools` activity that outlived `mcpConnectionIdleTimeout` (5 minutes by default) had its transport disconnected underneath it, failing the call; since each retry was cut off the same way, an MCP tool slower than the idle window could never succeed. Track in-flight uses per connection, as the Python SDK already does: acquiring clears the pending idle timer, and releasing re-arms it only once no calls remain and the record is still the cached one, so the window measures genuine idleness. Eviction now also removes the entry from the cache before awaiting `disconnect()`, so a call arriving during teardown opens a fresh connection instead of joining the dying one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was wrong
The worker-process MCP connection cache in
contrib/strands/src/temporal-mcp-client.tsarmed its idle timer insidegetConnection— that is, at the start of each call — and nothing extended it while the call ran. So the window measured time since the last call started, not since the last call finished.A
callToolorlistToolsactivity that outlivedmcpConnectionIdleTimeout(5 minutes by default, against a 10-minute defaultstartToCloseTimeout) hadclient.disconnect()run underneath it, rejecting the in-flight request. Each retry was cut off the same way, so an MCP tool slower than the idle window could never succeed. A concurrent call holding the same client also continued against a closed transport.The fix
Track in-flight uses per connection, matching what the Python SDK already does in
temporalio/contrib/strands/_temporal_mcp_client.py:acquireConnection()incrementsinflightand clears any pending idle timer.releaseConnection(), called from afinallyin both activities, re-arms the timer only onceinflightis 0 and the record is still the cached one — an already-evicted record must not schedule a timer that would later kill its healthy replacement._evictConnection()removes the entry from the cache before awaitingdisconnect(), so a call arriving during teardown opens a fresh connection instead of joining the dying one.The two parallel maps (
CONNECTIONS+IDLE_TIMERS) collapse into oneConnectionRecord { client, inflight, idleTimer }.Test plan
New test
mcpConnectionIdleTimeout does not evict a connection with a call in flight: a 500ms tool call against a 50ms idle window, asserting nodisconnect()lands while the call is running. It fails onmain("idle timer disconnected a connection with a call in flight") and passes with the fix.Full strands suite passes (16/16), including the existing
mcpConnectionIdleTimeout evicts the cached connection while the worker runstest — idle eviction still happens between activities, just not during one.🤖 Generated with Claude Code