Repository navigation
Python: Allow concurrent MCP calls with matching headers - #10142
Jose Alvarez (jpalvarezl) wants to merge 5 commits into
Conversation
GitHub Actions
Active
GitHub Actions
Active
agent-framework-automation
Bot
added
documentation
Code coverage overviewLanguages: Python Python / code-coverage/python
View coverage of the 3 most impacted files.
ℹ️ Calculations represent line coverage. Results may change as calculations are updated. Updated |
There was a problem hiding this comment.
🟡 Changes recommended
Critical cancellation and recovery races can leave active counts stuck, admit calls with stale metadata, reconnect after close, and deadlock concurrent connection attempts.
4 open issues
What changed in this PR
This PR enables concurrent Python MCP operations with matching effective HTTP headers while retaining session isolation when headers differ.
Changes:
- Replace whole-call locking with shared-header coordination for tools, generated prompts, and Foundry Toolbox resource reads.
- Coordinate connection recovery, shutdown handling, and timeout budgets.
- Add concurrency and lifecycle regression tests and update guidance.
| File | Description |
|---|---|
| skills/agent-framework-builder-python/references/integrations.md | Document matching-header concurrency and timeout behavior. |
| python/packages/foundry_hosting/tests/test_toolbox.py | Test resource reads during identity reconnection. |
| python/packages/foundry_hosting/agent_framework_foundry_hosting/_toolbox.py | Use shared-header coordination for resource reads. |
| python/packages/core/tests/core/test_mcp.py | Adapt recovery tests to transport-only replacement. |
| python/packages/core/tests/core/test_mcp_observability.py | Update recovery mocking in logging coverage. |
| python/packages/core/tests/core/test_mcp_http_auth.py | Test concurrency, recovery, cancellation, shutdown, and timeouts. |
| python/packages/core/README.md | Explain HTTP concurrency and timeout semantics. |
| python/packages/core/AGENTS.md | Update header-scoping and recovery guidance. |
| python/packages/core/agent_framework/_mcp.py | Implement shared-header admission and coordinated connection recovery. |
Review effort: Balanced
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (1 commit(s)): 9d4978090fcc
Model: gpt-5.6-sol
Overview
The PR replaces whole-call header locking with condition-based admission and an active-operation count, allowing calls with the same effective headers to overlap while draining before an identity switch. Owner-scoped header injection, generation checks around recovery, finally-based release, and extensive cancellation/recovery tests provide strong isolation and lifecycle guardrails. However, the admission predicate has no fairness mechanism, so sustained traffic for the current identity can indefinitely starve a queued caller with different headers.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
1 verified finding remained after source verification (1 medium) across 1 file. Details are attached to the affected lines below.
Affected areas: python/packages/core/agent_framework/_mcp.py
GitHub Actions
Active
GitHub Actions
Active
GitHub Actions
Active
…sue-investigation # Conflicts: # python/packages/foundry_hosting/tests/test_toolbox.py
GitHub Actions
Active
GitHub Actions
Active
GitHub Actions
Active
GitHub Actions
Active
Eduard van Valkenburg (eavanvalkenburg)
left a comment
Eduard van Valkenburg (eavanvalkenburg)
left a comment
There was a problem hiding this comment.
Reviewed the latest head, 8dbf27f0b424a85696ae69e4a5606f63420e8c60.
The earlier direct-prompt identity, stale retry arguments/metadata, resource URI conversion, and cancelled Toolbox-close issues are fixed in the offline checks. Same-identity calls overlap. I also ran the selected MCP, HTTP auth, observability, skills, and Toolbox unit suites against an archived copy of this head: 1,474 passed, 2 deselected.
One admission-fairness issue still reproduces; the inline comment gives the scenario.
| while True: | ||
| await self._wait_for_lifecycle() | ||
| async with self._call_headers_lock: | ||
| if self._active_call_headers is None or identity == _mcp_header_identity( |
There was a problem hiding this comment.
[P2] Prevent newer matching-identity calls from starving a waiting identity change
This still reproduces on 8dbf27f0, despite the earlier fairness thread being resolved. Once B is waiting for A's leases to drain, this predicate keeps admitting unrelated new A calls. The idle event is therefore never reached under sustained A traffic, so B repeatedly hits its admission timeout, or can wait indefinitely with request_timeout=None.
The offline repro starts one A call, queues B, then admits three successive newer A calls. Each preceding A call is released and completes. The original call and the first two newer calls have all finished, but the third newer A call alone keeps B blocked until its admission deadline. This is not just B waiting for the original batch.
Please account for a queued identity transition when admitting new unrelated calls, while retaining the same-identity nested callbacks needed by an active parent. A regression should verify that later unrelated A calls cannot continually overtake B after the original A calls finish, and that a required nested A callback still completes.
There was a problem hiding this comment.
MAF Automated Review — Iteration 2
Result: No findings
Scope: 19 net-new commit(s): df7d02bdc99c, e9292752c1ea, 18cd5903b902, 2fa7b9ab21be, 0c479365fca9, 83c719065840, 4ffc820cd62d, 082217287cb4, 6e770b95c033, d0e191cf4d3d, 44f5495c0851, 9db7efb0841b, 80d0cbf5bc30, dee716e6150c, 28163f5e23d2, c04fecdfdc2f, d2017c1c5d0e, 1d959983c7b5, 8dbf27f0b424
Model: gpt-5.6-sol
Overview
The change moves managed MCP connection setup, discovery, recovery, and teardown onto a lifecycle owner while allowing operations with the same effective header identity to overlap. Synchronous lease release, owner-tagged header context, origin checks, generation checks, bounded admission/recovery waits, and focused concurrency/cancellation tests provide strong isolation and cleanup guardrails. No new Critical, High, or Medium defect remained after reconciliation against the implementation, tests, and existing feedback.
Reviewed the supplied incremental change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.
Motivation & Context
Concurrent calls through one
FoundryToolboxor anMCPStreamableHTTPToolwith aheader_providercurrently serialize across the complete RPC, even when theireffective headers are identical. This prevents an independent batch scheduled
with
asyncio.gather()from executing concurrently.Allow calls sharing the same effective header identity to overlap while retaining
the existing isolation boundary when headers differ. This is a draft for
maintainer review of the concurrency and lifecycle changes.
Description & Review Guide
same-identity leases. A short admission lock and idle event let matching-header
calls overlap; different identities wait outside the lock. Lease release is
synchronous, including during cancellation. The lifecycle owner handles complete
connection setup, discovery, recovery, and cleanup. Calls only borrow ready
connections; retries reprepare arguments and metadata from the refreshed catalog.
Direct prompts resolve ambient headers, generated prompts retain runtime headers,
and Toolbox HTTP cleanup runs on the owner after MCP teardown.
concurrently without changing public signatures, dependencies, header
precedence, or caller-owned connection ownership.
request_timeoutalso boundsadmission/reconciliation and recovery as separate budgets;
Nonedisablesthose budgets. A waiting caller's timeout does not cancel another caller's
active operation. Managed discovery remains owner-serialized and restarts a
complete snapshot after connection replacement. Externally supplied MCP Clients
and ClientSessions retain caller ownership and do not gain these synchronization
guarantees. Explicit connection and discovery after a completed close remain
supported. Close/reset retain teardown semantics; this is not a new graceful
drain or arbitrary nested cross-identity-call feature.
different-identity draining, nested same-header calls, cancellation cleanup,
late failures from replaced connections, shutdown during recovery discovery,
and admission during abandoned setup. Also check direct/detached prompt identity,
separation of prompt arguments from authentication, and cancelling a Toolbox
close caller while the owner still needs HTTP for termination. The implementation
deliberately avoids a separate HTTP exclusive-barrier/ancestry stack.
Local validation:
uv run --directory python\packages\core --frozen --group dev --group test --inexact pytest --no-cov -q -o addopts= --disable-warnings --tb=short tests ..\foundry_hosting\tests\test_toolbox.py -m "not integration":passed, 9,450 passed, 44 skipped, 2 deselected, 2 xfailed. Existing deprecation
and telemetry warnings remain; the runner exited successfully.
passed.
Live Toolbox deployment validation has not been run.
The full local run above predates the latest main merge. Its manual conflict
resolution only combined test imports; the full suite was not rerun for that merge.
Related Issue
No linked GitHub issue. Maintainer-directed draft for discussing the implementation;
this change is not presented as a trivial-change exception.
Alternative implementation: #10147.
Both address the same batch bottleneck; this draft explores a lifecycle-owner
readiness boundary rather than per-network-attempt exclusive HTTP transitions.
AI Assistance
AI assistance details: Copilot assisted with implementation, regression tests,
documentation, independent code/design review, and this 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.