Skip to content

Python: preserve MCP request ownership on low-level sends - #8246

Merged
Eduard van Valkenburg (eavanvalkenburg) merged 1 commit into
microsoft:mainfrom
eavanvalkenburg:cuddly-garbanzo
Sep 10, 2026
Merged

Python: preserve MCP request ownership on low-level sends#8246
Eduard van Valkenburg (eavanvalkenburg) merged 1 commit into
microsoft:mainfrom
eavanvalkenburg:cuddly-garbanzo

Conversation

@eavanvalkenburg

Copy link
Copy Markdown
Member

Motivation & Context

Supported MCP dependencies can issue streamable HTTP requests through AsyncClient.send(). That path bypassed the transport wrapper's request ownership marker, so header_provider hooks ignored those requests.

Description & Review Guide

  • What are the major changes? Add a focused send() wrapper that applies the same private request marker as the existing stream and delete paths, plus a regression test for that invariant.
  • What is the impact of these changes? Request-scoped headers continue to work on the low-level send path. The rebased focused suite passes all 75 cases, together with core syntax, source typing, and test typing checks.
  • What do you want reviewers to focus on? Confirm that marking the request immediately before delegation preserves the existing client ownership and lifecycle behavior.

Related Issue

Related to #7841.

Follow-up to #8225, which supplies run kwargs during connection setup; this change preserves request ownership when the MCP dependency sends through AsyncClient.send().

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 is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • 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.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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.

🟢 Approval recommended

The focused implementation consistently extends existing request-scoping behavior and includes appropriate regression coverage.

Pull request overview

Ensures MCP low-level HTTP sends retain request ownership so scoped header hooks apply correctly.

Changes:

  • Wraps AsyncClient.send() to attach the MCP ownership marker.
  • Adds regression coverage for low-level sends.
File summaries
File Description
python/packages/core/agent_framework/_mcp.py Tags requests before delegating low-level sends.
python/packages/core/tests/core/test_mcp.py Verifies ownership tagging through send().
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

@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: No findings
Scope: full PR (1 commit(s)): 0ad4dfe38bbd
Model: gpt-5.6-sol-fast

Overview

The PR extends the private MCP HTTP client wrapper so prebuilt requests sent through AsyncClient.send() receive the same per-tool ownership marker as stream and delete requests. Identity checks, origin checks, redirect cleanup, lifecycle cleanup, and the focused regression test constrain the change without altering the public API or client ownership model. No publishable Critical, High, or Medium defect was established.

Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.

Merged via the queue into microsoft:main with commit 501bd52 Sep 10, 2026
46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants