Repository navigation
Python: Clarify MCP client ownership and add workflow sample - #9532
Jose Alvarez (jpalvarezl) wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The sample leaves wrapper-owned background tasks unclosed, and URL-less wrappers report unverified HTTP transport metadata.
2 open issues
What changed in this PR
Clarifies Python MCP client ownership and demonstrates keeping a caller-owned Client alive throughout a sequential workflow.
Changes:
- Validates constructor ownership combinations while preserving deprecated session compatibility.
- Adds ownership and telemetry tests.
- Documents lifecycle boundaries and adds a researcher-to-writer workflow sample.
| File | Description |
|---|---|
| python/samples/03-workflows/README.md | Links the lifecycle sample. |
| python/samples/03-workflows/agents/mcp_client_lifecycle.py | Demonstrates caller-owned Client lifetime across a workflow. |
| python/packages/core/tests/core/test_mcp.py | Tests ownership validation and session compatibility. |
| python/packages/core/tests/core/test_mcp_observability.py | Adds caller-owned connection telemetry tests. |
| python/packages/core/AGENTS.md | Documents constructor ownership rules. |
| python/packages/core/agent_framework/_mcp.py | Validates ownership options and adjusts transport telemetry. |
| docs/decisions/0045-python-mcp-v2-client-lifecycle.md | Clarifies ownership boundaries and records ADR acceptance. |
Review effort: Balanced
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (3 commit(s)): 86584815684a, 284a065329a8, b322d752306b
Model: gpt-5.6-sol
Overview
The PR makes MCP connection ownership explicit with fail-closed constructor validation, preserves deprecated session paths, and adds focused ownership and observability coverage. The supplied-client lifecycle guards are strong, but the new mixed-ownership rejection currently breaks an existing core test, and the workflow sample does not close its lazily opened MCP wrapper before shutting down the caller-owned client.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
2 verified findings remained after source verification (2 medium) across 2 files. Details are attached to the affected lines below.
Affected areas: python/packages/core/agent_framework/_mcp.py, python/samples/03-workflows/agents/mcp_client_lifecycle.py

Motivation & Context
Agents used inside workflows need a clear lifecycle boundary when their MCP tools reuse a caller-created high-level
mcp.Client. Previously, the HTTP and stdio wrappers still required framework-owned transport parameters and could silently ignore wrapper configuration when a Client was supplied. This change makes construction ownership explicit while preserving the released deprecatedClientSessioncompatibility paths.It also adds a sequential researcher-to-writer workflow sample that keeps a caller-entered Microsoft Learn MCP Client alive around the complete workflow run.
Description & Review Guide
mcp_clientconstruction forMCPStreamableHTTPToolandMCPStdioTool; reject mixed ownership and ignored transport/process options; preserve deprecated session compatibility; make transport telemetry reflect only known wrapper-owned configuration; add targeted ownership, compatibility, invariant, and observability tests; update ADR/contributor guidance; and add the workflow lifecycle sample.mcp_clientcombinations were not formally released, so this is not a breaking change; released deprecated session paths remain supported. Removal of legacy constructor/session compatibility is tracked separately in Python: [BREAKING] Remove deprecated MCP compatibility surfaces in vNext #9452. Targeted MCP tests and core syntax/type checks pass.Related Issue
Fixes #9369
AI Assistance
AI assistance details: Constructor ownership analysis, targeted tests, documentation and sample refinement, validation, and this PR description.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.