Repository navigation
.NET: fix: mcp workflow isolation - #9545
Vincent Biret (baywet) wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Cache eviction can terminate unfinished workflows’ MCP sessions, breaking continuity and disrupting pending callers.
1 open issue
What changed in this PR
Implements the .NET portion of #8662, isolating declarative MCP sessions by workflow while preserving compatibility with existing handlers.
Changes:
- Propagates workflow identity through execution contexts and subworkflows.
- Adds workflow-scoped MCP caching, capacity limits, and coordinated cleanup.
- Adds isolation, continuation, cancellation, and lifetime tests.
| File | Description |
|---|---|
| dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/RepresentationTests.cs | Tests subworkflow identity and checkpoint compatibility. |
| dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/PowerFx/WorkflowFormulaStateTests.cs | Tests persisted fallback identity. |
| dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/ObjectModel/InvokeMcpToolExecutorTest.cs | Tests scoped invocation and approval handling. |
| dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/Kit/RootExecutorTests.cs | Updates context setup. |
| dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/Kit/IWorkflowContextExtensionsTests.cs | Updates context creation helpers. |
| dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs | Tests workflow-aware cache keys. |
| dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs | Tests cache capacity, concurrency, and cleanup. |
| dotnet/src/Microsoft.Agents.AI.Workflows/SubworkflowBinding.cs | Derives hierarchical session identifiers. |
| dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/WorkflowHostExecutor.cs | Propagates child workflow identity. |
| dotnet/src/Microsoft.Agents.AI.Workflows/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt | Records session-context API. |
| dotnet/src/Microsoft.Agents.AI.Workflows/PublicAPI/net9.0/PublicAPI.Unshipped.txt | Records session-context API. |
| dotnet/src/Microsoft.Agents.AI.Workflows/PublicAPI/net8.0/PublicAPI.Unshipped.txt | Records session-context API. |
| dotnet/src/Microsoft.Agents.AI.Workflows/PublicAPI/net472/PublicAPI.Unshipped.txt | Records session-context API. |
| dotnet/src/Microsoft.Agents.AI.Workflows/PublicAPI/net10.0/PublicAPI.Unshipped.txt | Records session-context API. |
| dotnet/src/Microsoft.Agents.AI.Workflows/IWorkflowContext.cs | Defines optional session identity contract. |
| dotnet/src/Microsoft.Agents.AI.Workflows/InProc/InProcessRunnerContext.cs | Exposes identity through bound contexts. |
| dotnet/src/Microsoft.Agents.AI.Workflows/InProc/InProcessRunner.cs | Separates workflow and checkpoint identities. |
| dotnet/src/Microsoft.Agents.AI.Workflows/GroupChatManager.cs | Preserves identity through context wrappers. |
| dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt | Records scoped handler contract. |
| dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/PublicAPI/net9.0/PublicAPI.Unshipped.txt | Records scoped handler contract. |
| dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/PublicAPI/net8.0/PublicAPI.Unshipped.txt | Records scoped handler contract. |
| dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/PublicAPI/net472/PublicAPI.Unshipped.txt | Records scoped handler contract. |
| dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/PublicAPI/net10.0/PublicAPI.Unshipped.txt | Records scoped handler contract. |
| dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/ObjectModel/InvokeMcpToolExecutor.cs | Dispatches calls with workflow scope. |
| dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Kit/RootExecutor.cs | Creates session-aware contexts. |
| dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Kit/ActionExecutor.cs | Creates session-aware contexts. |
| dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Interpreter/DelegateActionExecutor.cs | Creates session-aware contexts. |
| dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Interpreter/DeclarativeWorkflowExecutor.cs | Creates session-aware contexts. |
| dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Interpreter/DeclarativeWorkflowContext.cs | Resolves and persists workflow identity. |
| dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Interpreter/DeclarativeActionExecutor.cs | Creates session-aware contexts. |
| dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/IMcpToolHandler.cs | Adds compatible scoped handler interface. |
| dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt | Records scoped invocation API. |
| dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt | Records scoped invocation API. |
| dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt | Records scoped invocation API. |
| dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt | Records scoped invocation API. |
| dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt | Records scoped invocation API. |
| dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs | Implements scoped caching and resource lifetimes. |
Review effort: Balanced
Code coverage overviewLanguages: C# C# / code-coverage/dotnet
View coverage of the 10 most impacted files.
ℹ️ Calculations represent line coverage. Results may change as calculations are updated. Updated |
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (1 commit(s)): 96cbf3c5c41f
Model: gpt-5.6-sol
Overview
The PR propagates a framework-owned workflow session identifier into MCP caching and adds substantial concurrency, cancellation, disposal, provider-isolation, and credential-origin guardrails. The new bounded cache coalesces same-key creation and carefully drains active and retired clients. Residual risks remain for legacy workflow contexts that initialize identity through queued state, for stateful sessions evicted while their workflows are still live, and for unrelated tenants sharing the fixed active-session limit.
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, dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Interpreter/DeclarativeWorkflowContext.cs
Extract workflow-scoped caching, connection creation, provider lifetime coordination, and resource cleanup from DefaultMcpToolHandler without changing session behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 94e932cb-8d65-4169-8c8b-3b5ceff9d09d
Preserve both workflow session and checkpoint callback API entries. Adapt the merged checkpoint test to the session-aware context factory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 94e932cb-8d65-4169-8c8b-3b5ceff9d09d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 94e932cb-8d65-4169-8c8b-3b5ceff9d09d
|
Added proposed ADR 0047 in 2c89515. It documents workflow-owned MCP sessions, factory-managed HTTP handler reuse, caller-owned HTTP clients, active-call draining, approval/checkpoint behavior, examples, alternatives, trade-offs, and potential compatibility and user-experience changes. The proposal is not implemented; this commit adds only the ADR. westey (@westey-m): review requested as a listed decider alongside Vincent Biret (@baywet). Ryan Hurey (@rhurey): informing you for architecture/EM review of the proposed direction. |

dotnet part of the #8662 PR