Skip to content

fix(mcp): keep the MCP connection on its own task (0.16.7.post2, for release/s201) - #1095

Merged
PopescuTudor merged 1 commit into
release/uipath-langchain-0.16.7from
fix/mcp-client-owns-session-task-0.16.7
Sep 16, 2026
Merged

PopescuTudor merged 1 commit into
release/uipath-langchain-0.16.7from
fix/mcp-client-owns-session-task-0.16.7

Conversation

@PopescuTudor

@PopescuTudor PopescuTudor commented Sep 16, 2026 •

Copy link
Copy Markdown
Collaborator

Same fix as #1093, on exactly what uipath-agents release/s201 (ring 1) ships today: uipath-langchain 0.16.7.post1. Bumps to 0.16.7.post2 so CD publishes on merge — the same shape as 0.16.7.post1 (#1065), a code hotfix published from this branch. (PRODEV-1583)

Why a second artifact: #1093 lands as 0.16.20 on release/uipath-langchain-0.16.16, which since #1092 requires deepagents>=0.7.11. release/s202 can take that — #744 moved it to deepagents 0.7.x / 0.16.19 today — but release/s201 still pins deepagents==0.5.9 and its code imports BackendFactory, gone in 0.7.x, so 0.16.19+ is not resolvable there. mcp_client.py is byte-identical between 0.16.7 and 0.16.18, so the fix applies as-is; the deepagents constraint here stays >=0.5.9,<0.6.0.

Agents with 2+ MCP sessions open finish the run correctly and then fail at teardown with ERROR_CancelledError, so the job shows up as Faulted. The HTTP client, transport and ClientSession sit in one AsyncExitStack; its anyio cancel scopes must be exited by the task that entered them, LIFO. Callers can't guarantee either: langgraph's ToolNode opens the connection inside a tool task while dispose() runs on the teardown task, and an agent with several servers disposes them oldest-first. The scope error was caught under except Exception and logged at DEBUG, and the half-closed scope resurfaced later as a CancelledError.

A dedicated task now opens the whole stack and holds it until dispose() signals it to close, or cancels it if the handshake hasn't finished. The task's finally also clears _client_initialized, so a connection that dies on its own is rebuilt by the next call instead of poisoning the client, and _ensure_session raises rather than handing out a missing session. Setup errors still surface as themselves. Recovery is unchanged (re-runs the handshake on the same session). Includes the #1093 review fixes.

Tests, against a real FastMCP 1.x server that records what reached it, all asserting nothing was swallowed at DEBUG:

  • a session opened on a tool task disposes from the teardown task, and the session DELETE reaches the server
  • three clients dispose cleanly in creation order — fails on this branch as-is with the production CancelledError
  • a caller cancelled mid-handshake unwinds in well under a second
  • a connection that dies on its own is rebuilt on the next call

tests/agent/tools/test_mcp green (76); ruff and mypy clean; agent/tools/mcp/claude.md updated.

Verified on the s201 stack itself — uipath-agents release/s201 (factory as-is, creation-order dispose) with this build installed with its dependencies: deepagents 0.5.9 and uipath-langchain-client 1.17.3 unchanged; 3 Dynamic servers with one tool call and 2 Cached servers with both/one called from tool tasks all clean, 0 swallowed. uv lock --dry-run on release/s201 with uipath-langchain[bedrock,vertex]==0.16.7.post2 changes exactly one package.

🤖 Generated with Claude Code

Backport of #1087 onto the mcp 1.26 client. The HTTP client, transport and
ClientSession live in one AsyncExitStack whose anyio cancel scopes have to
be exited by the task that entered them, LIFO. Callers can't do that:
langgraph opens the connection inside a tool task while dispose() runs on
the teardown task, and an agent with several servers closes them
oldest-first. Result was "Attempted to exit a cancel scope that isn't the
current task's current cancel scope", swallowed on the way out and coming
back as a CancelledError that failed a job which had already produced the
right answer.

Now a dedicated task opens the whole stack and holds it until dispose()
signals it to close (or cancels it mid-handshake), so enter and exit happen
on the same task and order stops mattering. Recovery still re-runs the
handshake on the same session.

Bumps to 0.16.7.post2 (hotfix on exactly what release/s201 ships; nothing else moves).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 16, 2026 16:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@PopescuTudor PopescuTudor self-assigned this Sep 16, 2026
@sonarqubecloud

Copy link
Copy Markdown

@alessandromuresan alessandromuresan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked this as a port rather than re-reviewing the design — I reviewed #1093 and the findings there are all in this tree. No changes requested.

The three source files are byte-identical to what merged in #1093, which as of 16:52 is what release/uipath-langchain-0.16.16 carries:

file vs #1093 head 46094a5 vs release/uipath-langchain-0.16.16
mcp_client.py identical identical
claude.md identical identical
test_mcp_client_task_ownership.py identical identical

So the review fixes from #1093 are all present and didn't need re-deriving: _client_initialized = False in the connection task's finally, _ensure_session raising instead of handing out a missing session, _session = None in _close_connection before the if task is None return, close_requested passed in rather than asserted off self, the try/finally in dispose(), the four claude.md lifecycle spots, and the fourth test (test_a_connection_that_dies_on_its_own_is_rebuilt_on_the_next_call) plus the ASGI recorder behind the DELETE assertion.

The "applies as-is" claim holds — the relevant base files are byte-identical between the two release branches, not just mcp_client.py:

  • src/uipath_langchain/agent/tools/mcp/mcp_client.py — identical on release/uipath-langchain-0.16.7 and release/uipath-langchain-0.16.16
  • src/uipath_langchain/agent/tools/mcp/streamable_http.py — identical. This one matters beyond diff arithmetic: the poison-the-client path that #1093's finally fix addresses comes from the transport being an @asynccontextmanager yielding out of async with anyio.create_task_group() with tg.start_soon(...) children (streamable_http.py:723-748). Same code here, so the fix is load-bearing for the same reason and the new regression test covers the same scenario.
  • src/uipath_langchain/agent/tools/mcp/mcp_tool.py — identical, so nothing else in the package touches the changed internals.
  • tests/agent/tools/test_mcp/test_mcp_client.py:524-525 has the same _session is None / _stack is None assertions, so retaining the self._stack assignment keeps the purpose it was kept for in #1093.

claude.md is the only one whose bases differ, and only because #1093 already landed on the newer branch — 0.16.7's copy is the pre-fix text, and this PR brings it to the same post-fix content.

Packaging, which is the only real delta from #1093:

  • 0.16.7.post1 → 0.16.7.post2; deepagents correctly left at >=0.5.9, <0.6.0 and mcp==1.26.0 untouched, which is the whole point of the second artifact.
  • Publish-on-merge works: cd.yml on this branch triggers on push to release/** filtered to paths: [pyproject.toml], and this PR touches pyproject.toml. #1065 is the precedent for a .post release off this same branch.
  • The uv.lock [options] exclude-newer rewrite to the 0001-01-01T00:00:00Z sentinel looked like stray churn from a newer uv, so I checked before flagging it: main and release/uipath-langchain-0.16.16 already carry that sentinel and 0.16.7 was the stale one, exclude-newer-span = "P2D" is present and unchanged, and uv's own inline comment says it has no effect. Convergence, not drift — no action.

CI is green on all 13 checks including the full 3.11/3.12/3.13 × ubuntu/windows test matrix. One incidental note that is inert here but worth knowing if this fixture gets reused: the test's import uvicorn is satisfied transitively through mcp (uv.lock:2361, under mcp==1.26.0) rather than by a declared dev dependency. On this branch mcp is pinned exactly, so it can't move; on main it's a >= range and a future mcp that drops uvicorn would break the import rather than skip the test.

Leaving this as a comment, not an approval — the branch shows REVIEW_REQUIRED and I'd rather the gate on a ring-1 artifact stay with you.

@PopescuTudor
PopescuTudor merged commit 5ba45fe into release/uipath-langchain-0.16.7 Sep 16, 2026
17 checks passed
@PopescuTudor
PopescuTudor deleted the fix/mcp-client-owns-session-task-0.16.7 branch September 16, 2026 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants