.NET: [BREAKING] Scope provider-backed MCP sessions per invocation - #8425
Jose Alvarez (jpalvarezl) merged 3 commits into
Conversation
Create and clean up separate MCP sessions for custom client-provider calls in .NET and Python. Preserve default session caching and caller-owned HTTP clients, and add lifecycle coverage and compatibility guidance. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
Cross-runtime ownership, cancellation, and concurrent-shutdown behavior remains lifecycle-sensitive despite broad regression coverage.
Pull request overview
Scopes provider-backed MCP sessions per invocation across Python and .NET while preserving no-provider caching.
Changes:
- Adds per-invocation session creation and cleanup for custom providers.
- Coordinates cancellation and shutdown with active invocations.
- Documents and tests ownership and lifecycle behavior.
File summaries
| File | Description |
|---|---|
python/packages/declarative/tests/test_default_mcp_tool_handler.py |
Expands lifecycle and concurrency coverage. |
python/packages/declarative/README.md |
Documents Python session lifetimes. |
python/packages/declarative/AGENTS.md |
Records package lifecycle guidance. |
python/packages/declarative/agent_framework_declarative/_workflows/_mcp_handler.py |
Implements invocation-scoped provider sessions. |
dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs |
Adds .NET lifecycle regression tests. |
dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md |
Documents .NET session lifetimes. |
dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs |
Implements provider session ownership and shutdown tracking. |
Review details
- Files reviewed: 7/7 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.
Assert cancellation and resource cleanup without depending on a cancellation message that older asyncio task implementations may consume. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (1 commit(s)): a8f0aca843f9
Model: gpt-5.6-sol-fast
Overview
The change consistently isolates provider-backed MCP sessions per invocation while preserving no-provider caching, caller-owned HTTP clients, and cleanup across ordinary failure and cancellation paths. The new tests provide strong coverage for concurrent calls, shutdown draining, and transport ownership. Two reentrant shutdown paths can nevertheless deadlock when a provider disposes its own handler, and the .NET disposal path can replace a completed tool result with a cleanup exception.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
3 verified findings remained after source verification (3 medium) across 2 files. Details are attached to the affected lines below.
Affected areas: dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs, python/packages/declarative/agent_framework_declarative/_workflows/_mcp_handler.py
Reject self-draining shutdown across inherited invocation contexts. Preserve invocation outcomes when session or transport cleanup fails, report non-cancellation cleanup failures separately, and add lifecycle regression coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
MAF Automated Review — Iteration 2
Result: No findings
Scope: 2 net-new commit(s): 43694526cf15, 24b1029b1adc
Model: gpt-5.6-sol-fast
Overview
This incremental update adds execution-context ancestry guards in .NET and Python so provider callbacks, nested invocations, and inherited child tasks cannot deadlock by shutting down their own handler. It also preserves .NET invocation outcomes across non-cancellation cleanup failures, with focused tests covering provider, connect, call, list, close, nested, cancellation, and external-shutdown paths. The remaining candidates were either already covered by prior feedback, pre-existing relative to this review range, or unsupported as a concrete shared-boundary failure.
Reviewed the supplied incremental change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.
Eduard van Valkenburg (eavanvalkenburg)
left a comment
There was a problem hiding this comment.
approved for python side
Motivation & Context
Make MCP session ownership explicit for declarative workflow handlers that use custom HTTP client providers. Provider-selected authentication is not represented in the existing session cache key, so that key alone cannot establish whether a previous session is appropriate for each invocation.
Description & Review Guide
tools/listand when the provider returns the same HTTP client ornull/None. Sessions and owned fallback transports are cleaned up after use, active provider-backed invocations are tracked during shutdown, and caller-owned HTTP clients remain open. Existing no-provider session caching is preserved. Documentation and in-memory functional regression tests cover the ownership and lifecycle contract.Related Issue
N/A
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.