Skip to content

Python: Clarify MCP client ownership and add workflow sample - #9532

Open
Jose Alvarez (jpalvarezl) wants to merge 5 commits into
mainfrom
jpavlvarezl/feature/workflow_mcp_sample
Open

Jose Alvarez (jpalvarezl) wants to merge 5 commits into
mainfrom
jpavlvarezl/feature/workflow_mcp_sample

Conversation

@jpalvarezl

Copy link
Copy Markdown
Member

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 deprecated ClientSession compatibility 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

  • What are the major changes? Stabilize caller-owned mcp_client construction for MCPStreamableHTTPTool and MCPStdioTool; 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.
  • What is the impact of these changes? Callers can pass an already-entered high-level Client without placeholder URL or command values, and invalid combinations fail immediately with actionable errors. The stabilized mcp_client combinations 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.
  • What do you want reviewers to focus on? Please verify the constructor ownership boundaries, preservation of deprecated session behavior, and that the sample scopes the caller-owned Client around the complete workflow execution.

Related Issue

Fixes #9369

AI Assistance

  • No material AI assistance was used.
  • This is an AI-assisted contribution. I reviewed, understood, and verified all submitted content and accept responsibility for it.

AI assistance details: Constructor ownership analysis, targeted tests, documentation and sample refinement, validation, and this PR description.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR links to an agreed issue with no competing open PR, or the Related Issue section documents a trivial-change or repository-automation exception.
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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

Comment thread python/packages/core/agent_framework/_mcp.py
Comment thread python/samples/03-workflows/agents/mcp_client_lifecycle.py Outdated

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Comment thread python/packages/core/agent_framework/_mcp.py
Comment thread python/samples/03-workflows/agents/mcp_client_lifecycle.py Outdated
@github-code-quality

github-code-quality Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Code coverage overview

Languages: Python

Python / code-coverage/python

main cca5b3d jpavlvarezl/feature/... 36812c6 +/-
92% 92% 0%

ℹ️ Calculations represent line coverage. Results may change as calculations are updated.


Updated October 19, 2026 13:02 UTC

This branch was successfully deployed

1 active deployment
github-app-auth — 36812c6d Deployed Oct 19, 2026 by jpalvarezl via add_label #25735
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Feature]: Python: Define lifecycle ownership for agents with MCP tools inside workflows and orchestrations

2 participants