From bc8520df0486fe05abe80a8e7f54eb096e3fea94 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Mon, 26 Oct 2026 12:18:38 -0400 Subject: [PATCH 01/13] docs: revise workflow MCP connection lifetime ADR Separate unattended workflow state continuity from MCP client security isolation and document protocol evolution. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 27dcc08f-0b3a-40a6-a3ad-2c45eafce159 --- ...0047-dotnet-workflow-owned-mcp-sessions.md | 396 ++++++++++++++++++ 1 file changed, 396 insertions(+) create mode 100644 docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md diff --git a/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md b/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md new file mode 100644 index 00000000000..026b373a26a --- /dev/null +++ b/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md @@ -0,0 +1,396 @@ +--- +status: proposed +contact: baywet +date: 2026-10-26 +deciders: [baywet, westey-m] +informed: [rhurey] +--- + +# .NET workflow MCP isolation without workflow-lifetime connection ownership + +## Context and Problem Statement + +This ADR was recovered from [#9545](https://github.com/microsoft/agent-framework/pull/9545). +Its original proposal tied initialized MCP clients to logical workflow sessions, +including approval waits and later resumes. This revision separates the need for +client isolation from the assumption that a workflow must retain a connection to +preserve protocol state. + +The implementation proposed in #9545 combines several resource-management +policies: workflow-specific MCP session lookup, bounded least-recently-used +eviction, active-call tracking, concurrent initialization, shutdown draining, and +reference-counted sharing of handler-owned `HttpClient` instances. Extracting these +into separate classes improves code organization but does not establish that +workflow-lifetime connection retention is necessary. + +### Workflow tool calls are normally unattended + +Declarative workflows use MCP primarily to discover and invoke tools. They +typically run without a user available to answer interactive questions. MCP +elicitation therefore is not a reason to preserve a connection for these +workflows. A workflow approval is a workflow-runtime interaction, not an MCP +`elicitation/create` exchange. Waiting for approval before invoking a tool does +not require keeping an MCP operation or connection open. + +Resource subscriptions and background catalog-change subscriptions are likewise +outside this tool-invocation use case. The workflow does not need to maintain a +live resource view while paused. If future functionality introduces interactive +MCP exchanges or subscriptions, it must define their explicit lifetimes rather +than assuming every workflow already owns a permanent connection. + +Durable workflow state belongs in workflow state and checkpoints. Server-side +application state needed across calls should have an explicit handle and +authorization contract; it must not depend implicitly on retaining one live +`McpClient` object in the workflow host. + +### HTTP connections, protocol sessions, and clients are different boundaries + +| Concern | Lifetime and isolation | +|---|---| +| Physical HTTP connection | A transport resource; pooling and reconnection do not by themselves define MCP session identity | +| MCP protocol session in initialization-based revisions | A logical interaction identified by protocol state and, when supplied, `MCP-Session-Id`; it can span HTTP connections | +| MCP client and transport objects | SDK resources that may contain mutable protocol, authentication, or callback state; their safe reuse needs a separate isolation contract | +| Active tool invocation | Must retain the resources needed to complete the entire operation, not merely until headers or an intermediate result arrive | +| Workflow execution | Owns durable execution state, approvals, and checkpoint/resume behavior; does not inherently require a live MCP connection between tool calls | + +Under the `2025-11-25` Streamable HTTP specification, closing an HTTP connection +does not necessarily end the logical MCP session. Session IDs can be carried on +subsequent HTTP requests, and optional SSE replay can recover a disconnected +stream. Conversely, creating a fresh initialized client is not guaranteed to +reattach to an existing session. HTTP handler pooling alone provides neither +protocol session continuity nor durable application state. + +### Protocol evolution removes most cross-call state requirements + +The following reflects the `2026-07-28` specification text, not a claim that the +repository's current SDK or every deployed server already implements it. +Compatibility with initialization-based protocol revisions remains necessary. + +| Feature | Protocol direction | Relevance to unattended workflow tool calls | +|---|---|---| +| Initialization and protocol sessions | Remove `initialize` / `notifications/initialized` and `MCP-Session-Id`; carry version and client capabilities per request, with `server/discover` for discovery | No protocol-session retention requirement in the newer revision | +| Roots | Deprecated; prefer explicit tool parameters, resource URIs, or server configuration | Not a reason to retain workflow-lifetime client state | +| Sampling | Deprecated; prefer direct LLM-provider integration; remaining support uses multi round-trip requests (MRTR) | Not a baseline requirement of the workflow MCP tool handler | +| Logging | Deprecated; prefer stderr for stdio or OpenTelemetry; remove `logging/setLevel` in favor of per-request metadata for remaining protocol logging | No persistent per-session log-level setting to preserve in the newer revision | +| Elicitation | Retained through MRTR: return input requirements, then retry with answers and opaque `requestState`; remove URL-elicitation completion notifications | Interactive elicitation is outside the normal unattended workflow use case | +| Resource and catalog subscriptions | Replace `resources/subscribe`, `resources/unsubscribe`, and the HTTP GET stream with `subscriptions/listen` | Outside this use case; a future subscription needs its own explicit stream lifetime | +| In-flight calls, progress, and cancellation | Remain request-scoped; closing the SSE response stream cancels the request, and `Last-Event-ID` replay is removed | Protect the active invocation; a new connection cannot transparently resume the old stream | +| Experimental tasks | Move out of core into an official extension with polling and explicit input updates | A future integration should persist authorized task handles, not rely on a workflow keeping a connection open | + +Deprecated features remain available during their deprecation window; they are +not already removed. MRTR itself does not require connection continuity between +round trips: the client resubmits the original request with input responses and +any opaque server state. + +For older protocol revisions, subscriptions, pending server-to-client requests, +and session configuration can require retained state. They must not be silently +broken if an application explicitly depends on them. However, their existence +does not justify imposing their retention model on unattended workflow tool +calls that do not use them. + +The remaining baseline continuity requirement is the **active invocation**, not +the entire workflow execution. A completed call followed by an approval wait is +different from a tool call that is still awaiting its result. + +This proposal is **documentation-only**, does not change the current runtime or +SDK behavior, and does not change the Python implementation. + +## Decision Drivers + +- Avoid coupling workflow lifetime to protocol state unused by unattended tools. +- Preserve isolation between callers, workflows, and authentication identities. +- Protect active operations during cancellation and disposal. +- Keep durable workflow state independent of live network objects. +- Respect ownership and configuration of user-supplied HTTP clients. +- Delegate reusable HTTP handler management to standard .NET infrastructure. +- Support console applications, worker services, and web applications without + requiring ASP.NET or a DI container per invocation. +- Preserve existing transport protections and authentication-provider semantics. +- Make compatibility, resource limits, and any stateful opt-in behavior explicit. + +## Considered Options + +1. Retain the handler-wide bounded MCP cache and reference-counted HTTP sharing. +2. Own MCP connections for the entire logical workflow session, with optional + factory-managed HTTP handler reuse, as originally proposed in this ADR. +3. Decouple connection lifetime from workflow state management, with optional + factory-managed HTTP handler reuse and a separately defined client-isolation + policy; invocation-scoped clients are one possible implementation. +4. Share one application-wide initialized MCP client or `HttpClient`. + +## Decision Outcome + +Proposed chosen option: **decouple connection lifetime from workflow state +management, with optional factory-managed HTTP handler reuse and explicit client +security boundaries**. + +Do not introduce workflow-runtime resource ownership hooks solely to preserve MCP +session state between completed tool calls. A workflow pause, approval wait, or +checkpoint does not by itself require keeping an MCP client alive. Reuse of a +compatible HTTP handler and connection pool is independent of MCP client reuse. + +This revises the original workflow-lifetime ownership proposal. It does not +establish that arbitrary MCP clients are interchangeable or safe to share across +workflows. Whether clients should be invocation-scoped or retained within an +explicit security boundary remains open. Exact API shape, migration of currently +cached connections, and any optional stateful integration also remain subject to +design review. + +### Security isolation remains a separate requirement + +Not needing session continuity is not permission to share an initialized +`McpClient` across unrelated executions. A client, transport, authentication +provider, or handler pipeline can contain credentials, headers, pending requests, +callbacks, or server-side state from an older protocol revision. + +Invocation-scoped clients are one way to limit retention of such state. Any +client reuse must establish an explicit trusted isolation boundary: + +- Different principals, tenants, credentials, endpoints, or transport policies + must not select an incompatible existing client. +- A workflow identifier alone is not authority to access another execution's + client or resources. +- Changed credentials or configuration must not silently select stale client + state. Token refresh and identity changes need distinct, documented behavior. +- Shared pooled handlers must not retain per-workflow credentials, mutable + default headers, cookies, or origin-pinning state. +- Existing distinctions such as connection name, label, and explicit headers + must not be collapsed without a compatibility decision. + +A security requirement may justify a workflow-scoped client boundary for a +particular integration. That is a reason to isolate a client, not proof that its +connection must remain open throughout an approval wait. The concrete client +isolation and transport policy must be reviewed before implementation. + +### Invocation lifecycle and workflow resume + +An invocation must hold its MCP client and transport until the entire operation +has completed or unwound. Initialization, protocol round trips, result +consumption, and cleanup are part of that protected lifetime. Do not dispose +resources immediately after starting an asynchronous call or receiving an +intermediate result. + +Cancellation is a request, not evidence that work has stopped. Handler shutdown +must reject new invocations and coordinate completion of admitted operations +before disposing resources they use. Initialization failure, cleanup failure, +and reentrant disposal must have explicit, observable handling rather than +leaking resources or deadlocking. + +An approval continuation starts the approved invocation with the current +authorized connection configuration. It does not require retaining an idle MCP +client from before the approval. Restoring a checkpoint likewise creates the +resources required for subsequent calls; checkpoints do not serialize clients, +transports, HTTP objects, or promises of protocol-session reattachment. + +Do not automatically retry a tool invocation merely because the connection was +lost. The server may already have performed side effects even if the result was +not received. Newer protocol stream cancellation is not an application rollback. +Retry and idempotency behavior must follow the tool's explicit contract. + +Support for durable remote tasks would require persisting authorized task +handles and defining polling, expiry, input delivery, and cancellation behavior. +It is not supplied by retaining a workflow-lifetime connection. + +### HTTP ownership and factory integration + +| Source of HTTP client | Use and disposal contract | +|---|---| +| User-supplied client | Borrow as supplied; do not mutate shared configuration or dispose it | +| Application-provided factory | Obtain an owned wrapper for the chosen client scope; dispose it after MCP cleanup; let the factory manage pooled handlers | +| No supplied client or factory | Preserve a non-DI path that creates and owns a client with documented transport defaults | + +Owned wrappers must be disposed exactly once, including when initialization +fails. MCP and transport cleanup must release owned HTTP resources even if +earlier cleanup fails. Borrowed clients and application factories must not be +disposed by the MCP integration. + +`IHttpClientFactory` belongs to `Microsoft.Extensions.Http` and does not require +ASP.NET. The application may use the Generic Host or a `ServiceCollection`. +Do not create a service provider or factory per invocation. Use a bounded set of +stable factory configuration names, not names derived from workflow IDs, +credentials, or arbitrary server URLs. + +Factory reuse is not exclusive checkout/return: multiple wrappers may use one +underlying handler concurrently. Distinct proxies, client certificates, or other +transport policies can require separate configurations. There is no additional +handler-owned HTTP-client dictionary or reference-counted lease layer solely to +provide pooling. + +### Transport policy + +Internally created clients retain explicit defaults: cookies disabled, automatic +redirects disabled, certificate revocation checking, Streamable HTTP, and +origin-bound credential handling. Factory defaults alone are not a substitute. + +The current `OriginPinningHandler` stores one origin. It cannot be placed in a +single pooled pipeline for unrelated origins. A connection-specific outer +policy handler over a factory-managed pipeline, following the documented +`IHttpMessageHandlerFactory` pattern, or an equivalently validated request-specific +policy must preserve this protection. Origin policy must run after credential +injection in outbound processing. Caller-supplied clients remain under the +caller's transport-policy contract; do not replace their pipelines silently. + +Wrapper disposal must not disrupt other invocations sharing a factory-managed +handler. Conversely, factory handler expiry must not terminate an active call. +Any supported long-lived integration must deliberately configure connection +refresh; pooled connection lifetime does not transfer protocol-session state. + +The package supports modern .NET, .NET Standard 2.0, and .NET Framework 4.7.2. +Compatible factory-package versions, handler APIs, and DNS-refresh behavior must +be established for every supported target; this proposal does not drop targets. + +### Compatibility and explicitly stateful integrations + +The current `httpClientProvider` callback runs for every invocation and creates +a fresh MCP session for every call, including discovery, even when it returns the +same HTTP client or `null`. Preserve that contract. The callback may select +different credentials on successive invocations and must not silently become a +once-per-workflow factory. + +The non-provider path currently caches initialized clients by server URL, label, +connection name, and explicit headers. Moving that path to invocation-scoped +clients would be a behavioral change for servers relying on implicit session +state, and would add initialization cost for older protocol revisions. It needs +explicit migration documentation, not a claim that all deployed MCP servers are +stateless. + +Applications that genuinely require cross-call MCP session continuity need an +explicitly scoped integration, such as a custom `IMcpToolHandler`, with +documented ownership, authentication, capacity, and release contracts. It must +not be the default requirement for all workflows. Such an integration must not +evict live state silently or promise checkpoint reattachment unsupported by its +SDK and server. + +Keep public API changes additive where possible; do not introduce mandatory +members on existing custom handlers or ambiguous constructor overloads. Released +API changes require the repository's API-baseline and compatibility process. +Intentional breaking changes require the appropriate designation and release +notes when implementation is proposed. + +### Consequences + +- Good, because unattended workflow state and resume behavior do not depend on + retaining idle MCP connections. +- Good, because invocation-scoped clients can avoid implicit cross-call + client-state reuse while compatible HTTP infrastructure can still be pooled. +- Good, because workflow state continuity alone does not require a + workflow-session connection registry, idle-session LRU policy, or HTTP + reference-counting layer. +- Neutral, because active-invocation tracking, safe shutdown, and explicit + authentication boundaries remain necessary. +- Bad, because invocation-scoped clients require initialization per invocation + with older protocol revisions. +- Bad, because applications relying on the existing cached session behavior + need a migration path or an explicit stateful integration. +- Bad, because factory integration still requires careful transport-policy and + cross-target configuration. + +## Validation + +Before implementation is accepted, add focused regression coverage for: + +- Sequential invocations, concurrent workflows, and sibling executions do not + inherit one another's MCP client state or credentials. +- Approval waits and checkpoint resumes require no retained live connection; + the resumed invocation uses the correct authorized configuration. +- Admission, initialization, cancellation, and shutdown do not cause early + disposal, duplicate cleanup, leaked resources, or deadlock. +- A lost result does not silently trigger a duplicate side-effecting tool call. +- Owned wrappers are disposed once; borrowed clients and application factories + are not disposed; disposal does not disrupt another invocation's pooled HTTP + resources. +- Shared handlers contain no per-caller mutable authentication or origin state; + origin, redirect, cookie, and credential protections remain intact. +- Existing provider callbacks retain their per-invocation behavior, including + callbacks returning the same client or `null`. +- Compatibility with the currently supported SDK and protocol revisions is + preserved; tests do not assume the newer protocol is already deployed. +- Any explicit stateful integration follows its documented isolation and release + contract without imposing connection retention on the default path. + +Validate HTTP configuration and public API compatibility on every supported +target. Compare initialization cost, resource retention, shutdown behavior, and +handler/connection reuse before claiming a performance improvement. + +## Pros and Cons of the Options + +### 1. Handler-wide bounded cache and reference-counted HTTP sharing + +- Good, because it limits retained entries and reduces repeated initialization. +- Neutral, because active-operation coordination is still needed. +- Bad, because shared cache identity and eviction become correctness and + isolation concerns unrelated to the default unattended tool-call use case. + +### 2. Workflow-lifetime MCP connections with optional HTTP factory reuse + +- Good, because an explicitly stateful integration can retain its protocol state + and have a deterministic release boundary. +- Neutral, because HTTP factory reuse remains independent of MCP ownership. +- Bad, because it introduces runtime ownership across waits, run handles, + workflow-backed agents, and subworkflows without a default tool-call need. +- Bad, because idle retention consumes resources and does not provide + cross-process checkpoint continuity or justify safe client sharing. + +### 3. Independent client-isolation policy with optional HTTP factory reuse + +- Good, because resource retention can follow actual security and operation + requirements rather than the workflow's potentially long unattended lifetime. +- Good, because compatible pooled HTTP infrastructure is reusable without + retaining MCP session state or sharing initialized clients. +- Neutral, because invocation-scoped clients and explicit stateful integrations + require different ownership and compatibility contracts. +- Bad, because choosing invocation-scoped clients adds setup per call on + initialization-based servers and requires a documented cache migration. + +### 4. One application-wide initialized MCP client or HttpClient + +- Good, because compatible HTTP requests can share transport infrastructure. +- Neutral, because a shared HTTP wrapper does not imply shared MCP state. +- Bad, because one initialized MCP client can mix protocol and caller state; + one mutable HTTP pipeline also cannot safely represent every authentication, + origin, proxy, or client-certificate policy. + +## More Information + +### Protocol references + +- [2025-11-25 Streamable HTTP](https://modelcontextprotocol.io/specification/2025-11-25/basic/transports): + logical sessions, optional SSE replay, and disconnect versus cancellation. +- [2026-07-28 changelog](https://modelcontextprotocol.io/specification/2026-07-28/changelog): + stateless request metadata, session removal, MRTR, and the tasks extension. +- [Deprecated features](https://modelcontextprotocol.io/specification/2026-07-28/deprecated): + roots, sampling, and logging migration paths and deprecation windows. +- [Multi round-trip requests](https://modelcontextprotocol.io/specification/2026-07-28/basic/patterns/mrtr): + client-carried input responses and opaque request state across independent requests. +- [Subscriptions](https://modelcontextprotocol.io/specification/2026-07-28/basic/patterns/subscriptions): + explicit subscription stream lifetimes. +- [2026-07-28 Streamable HTTP](https://modelcontextprotocol.io/specification/2026-07-28/basic/transports/streamable-http): + request-scoped streams, cancellation on disconnect, and no SSE replay. + +### HTTP infrastructure references + +- [IHttpClientFactory with .NET](https://learn.microsoft.com/en-us/dotnet/core/extensions/httpclient-factory): + registration through `AddHttpClient`, including non-web usage. +- [HTTP client lifetime management](https://learn.microsoft.com/en-us/dotnet/core/extensions/httpclient-factory#httpclient-lifetime-management): + wrappers, pooled handlers, and disposal. +- [Using the factory with SocketsHttpHandler](https://learn.microsoft.com/en-us/dotnet/core/extensions/httpclient-factory#using-ihttpclientfactory-together-with-socketshttphandler): + connection refresh for explicitly long-lived integrations. +- [Message handler scopes](https://learn.microsoft.com/en-us/dotnet/core/extensions/httpclient-factory#message-handler-scopes-in-ihttpclientfactory): + handler scopes and connection-specific outer policy handlers. +- [Named clients](https://learn.microsoft.com/en-us/dotnet/core/extensions/httpclient-factory#named-clients): + avoid unbounded configuration names. + +### Implementation scope and remaining design work + +This proposal targets the .NET declarative MCP integration. It does not implement +the new protocol, add interactive elicitation or subscriptions, redesign Python, +change result conversion, or introduce durable MCP reconnection. + +Before implementation, deciders must approve the concrete client security +boundary and retention policy, legacy-cache migration, factory API and +transport configuration, and any explicitly stateful escape hatch. Preserve +existing guards until their replacement lifecycle is implemented and validated. + +Confirm EM/architect involvement through the listed review participants before +acceptance. Request `westey-m`'s review and inform `rhurey`; PR approval is the +decision record, not an assertion made by this document. From 964ac704e3f296a4537b4e2a2f26ac5e6b3c3d9c Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Mon, 26 Oct 2026 12:44:05 -0400 Subject: [PATCH 02/13] fix(mcp): scope workflow clients to individual invocations Give each invocation its own MCP client and transport while reusing the handler-owned HTTP pipeline. Drain active invocations before shutdown and document the changed cross-call session lifetime. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 27dcc08f-0b3a-40a6-a3ad-2c45eafce159 --- ...0047-dotnet-workflow-owned-mcp-sessions.md | 52 +-- .../DefaultMcpToolHandler.cs | 281 +++++----------- .../README.md | 33 +- .../DefaultMcpToolHandlerLifetimeTests.cs | 301 +++++++++++++----- .../DefaultMcpToolHandlerTests.cs | 193 +---------- .../references/declarative.md | 5 + 6 files changed, 365 insertions(+), 500 deletions(-) diff --git a/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md b/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md index 026b373a26a..df23dac951d 100644 --- a/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md +++ b/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md @@ -92,8 +92,9 @@ The remaining baseline continuity requirement is the **active invocation**, not the entire workflow execution. A completed call followed by an approval wait is different from a tool call that is still awaiting its result. -This proposal is **documentation-only**, does not change the current runtime or -SDK behavior, and does not change the Python implementation. +The accompanying implementation changes the .NET default tool handler's client +lifetime, not the supported MCP protocol or the workflow runtime's ownership +model. It does not change the Python implementation. ## Decision Drivers @@ -113,16 +114,14 @@ SDK behavior, and does not change the Python implementation. 1. Retain the handler-wide bounded MCP cache and reference-counted HTTP sharing. 2. Own MCP connections for the entire logical workflow session, with optional factory-managed HTTP handler reuse, as originally proposed in this ADR. -3. Decouple connection lifetime from workflow state management, with optional - factory-managed HTTP handler reuse and a separately defined client-isolation - policy; invocation-scoped clients are one possible implementation. +3. Use invocation-scoped MCP clients, with reusable HTTP handlers and a + separately defined client-isolation policy. 4. Share one application-wide initialized MCP client or `HttpClient`. ## Decision Outcome -Proposed chosen option: **decouple connection lifetime from workflow state -management, with optional factory-managed HTTP handler reuse and explicit client -security boundaries**. +Proposed chosen option: **invocation-scoped MCP clients with reusable HTTP +handlers**, decoupling connection lifetime from workflow state management. Do not introduce workflow-runtime resource ownership hooks solely to preserve MCP session state between completed tool calls. A workflow pause, approval wait, or @@ -131,10 +130,10 @@ compatible HTTP handler and connection pool is independent of MCP client reuse. This revises the original workflow-lifetime ownership proposal. It does not establish that arbitrary MCP clients are interchangeable or safe to share across -workflows. Whether clients should be invocation-scoped or retained within an -explicit security boundary remains open. Exact API shape, migration of currently -cached connections, and any optional stateful integration also remain subject to -design review. +workflows. The default handler uses invocation-scoped clients; applications +requiring cross-call session continuity supply an explicitly scoped integration. +The existing public invocation and provider contracts remain unchanged. Review +of the changed no-provider lifetime and its migration guidance remains required. ### Security isolation remains a separate requirement @@ -247,12 +246,11 @@ same HTTP client or `null`. Preserve that contract. The callback may select different credentials on successive invocations and must not silently become a once-per-workflow factory. -The non-provider path currently caches initialized clients by server URL, label, +The previous non-provider path cached initialized clients by server URL, label, connection name, and explicit headers. Moving that path to invocation-scoped -clients would be a behavioral change for servers relying on implicit session -state, and would add initialization cost for older protocol revisions. It needs -explicit migration documentation, not a claim that all deployed MCP servers are -stateless. +clients is a behavioral change for servers relying on implicit session state, +and adds initialization cost for older protocol revisions. It needs explicit +migration documentation, not a claim that all deployed MCP servers are stateless. Applications that genuinely require cross-call MCP session continuity need an explicitly scoped integration, such as a custom `IMcpToolHandler`, with @@ -331,7 +329,7 @@ handler/connection reuse before claiming a performance improvement. - Bad, because idle retention consumes resources and does not provide cross-process checkpoint continuity or justify safe client sharing. -### 3. Independent client-isolation policy with optional HTTP factory reuse +### 3. Invocation-scoped clients with reusable HTTP handlers - Good, because resource retention can follow actual security and operation requirements rather than the workflow's potentially long unattended lifetime. @@ -386,10 +384,20 @@ This proposal targets the .NET declarative MCP integration. It does not implemen the new protocol, add interactive elicitation or subscriptions, redesign Python, change result conversion, or introduce durable MCP reconnection. -Before implementation, deciders must approve the concrete client security -boundary and retention policy, legacy-cache migration, factory API and -transport configuration, and any explicitly stateful escape hatch. Preserve -existing guards until their replacement lifecycle is implemented and validated. +The implementation keeps one lazily created, handler-owned `HttpMessageInvoker` +for the default HTTP pipeline. Each invocation owns a separate MCP client, +transport, HTTP wrapper, and origin-policy handler. Its forwarding handler borrows +the shared invoker rather than disposing it. Handler shutdown drains admitted +invocations before disposing the shared HTTP pipeline. No workflow-runtime +ownership hooks, connection registry, HTTP-client cache, or lease-counting pool +are introduced. + +This does not add an `IHttpClientFactory` API or dependency. Factory integration +above remains guidance for a future additive API. The existing provider callback +continues to borrow application-supplied clients under its existing contract. + +Deciders must review the invocation boundary, legacy-cache migration, transport +configuration, and explicitly stateful escape hatch before accepting the ADR. Confirm EM/architect involvement through the listed review participants before acceptance. Request `westey-m`'s review and inform `rhurey`; PR approval is the diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs index 9bd67e55eb4..0e566dec06f 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs @@ -6,7 +6,6 @@ using System.IO; using System.Linq; using System.Net.Http; -using System.Security.Cryptography; using System.Text; using System.Text.Json; using System.Threading; @@ -26,10 +25,9 @@ namespace Microsoft.Agents.AI.Workflows.Declarative.Mcp; /// This provider supports per-server authentication via the httpClientProvider callback. /// The callback allows different MCP servers to use different authentication configurations by returning /// a pre-configured for each server. -/// Provider-backed invocations create and dispose a separate MCP session for every call, including -/// tools/list, because provider authentication is not represented in the session cache key. -/// Without a provider, sessions are cached by server URL, label, connection name, and explicit headers. -/// Non-cancellation cleanup failures are reported through warnings without replacing +/// Every invocation creates and disposes a separate MCP client, including tools/list. +/// Handler-created HTTP clients reuse an underlying HTTP handler without sharing MCP session state. +/// Non-cancellation session and transport cleanup failures are reported through warnings without replacing /// the invocation result or error. /// public sealed class DefaultMcpToolHandler : IMcpToolHandler, IAsyncDisposable @@ -45,13 +43,11 @@ public sealed class DefaultMcpToolHandler : IMcpToolHandler, IAsyncDisposable private static readonly JsonWriterOptions s_toolListJsonWriterOptions = new() { Indented = true }; private readonly Func>? _httpClientProvider; - private readonly Func _httpMessageHandlerFactory; - private readonly Dictionary<(string Url, string Label, string Connection, string HeadersHash), ClientConnection> _clients = []; - private readonly Dictionary _ownedHttpClients = []; - private readonly SemaphoreSlim _clientLock = new(1, 1); - private readonly AsyncLocal _providerInvocationContext = new(); - private TaskCompletionSource? _providerInvocationsDrained; - private int _activeProviderInvocations; + private readonly Lazy _httpMessageInvoker; + private readonly object _invocationLock = new(); + private readonly AsyncLocal _currentInvocation = new(); + private TaskCompletionSource? _invocationsDrained; + private int _activeInvocations; private bool _disposing; /// @@ -63,9 +59,9 @@ public sealed class DefaultMcpToolHandler : IMcpToolHandler, IAsyncDisposable /// configured with any required authentication. Return to use a default HttpClient with no auth. /// /// The callback is invoked for each tool invocation. MCP sessions are not cached when a callback is - /// configured, even if it returns the same client or . This adds session setup - /// per call and does not preserve server-side session state between calls. Supplied HTTP clients - /// remain caller-owned. Applications requiring session continuity should provide an + /// configured, even if it returns the same client or . MCP clients are also + /// invocation-scoped without a callback; server-side session state is not preserved between calls. + /// Supplied HTTP clients remain caller-owned. Applications requiring session continuity should provide an /// with an explicitly scoped authentication and session lifetime. /// /// @@ -86,7 +82,8 @@ internal DefaultMcpToolHandler( Func httpMessageHandlerFactory) { this._httpClientProvider = httpClientProvider; - this._httpMessageHandlerFactory = Throw.IfNull(httpMessageHandlerFactory); + Throw.IfNull(httpMessageHandlerFactory); + this._httpMessageInvoker = new(() => new HttpMessageInvoker(httpMessageHandlerFactory(), disposeHandler: true)); } /// @@ -119,55 +116,14 @@ public async Task InvokeToolAsync( } } - if (this._httpClientProvider is not null) + cancellationToken.ThrowIfCancellationRequested(); + using InvocationScope invocation = this.EnterInvocation(); + ClientConnection connection = await this.CreateClientAsync( + serverUrl.Trim(), serverLabel, headers, cancellationToken).ConfigureAwait(false); + await using (connection.ConfigureAwait(false)) { - TaskCompletionSource invocationCompleted = new(TaskCreationOptions.RunContinuationsAsynchronously); - await this._clientLock.WaitAsync(cancellationToken).ConfigureAwait(false); - try - { - this.ThrowIfDisposing(); - if (this._activeProviderInvocations++ == 0) - { - this._providerInvocationsDrained = new(TaskCreationOptions.RunContinuationsAsynchronously); - } - } - finally - { - this._clientLock.Release(); - } - - ProviderInvocationContext? previousInvocation = this._providerInvocationContext.Value; - this._providerInvocationContext.Value = new(invocationCompleted.Task, previousInvocation); - try - { - ClientConnection invocationClient = await this.CreateClientAsync( - serverUrl.Trim(), serverLabel, headers, httpClientCacheKey: null, cancellationToken).ConfigureAwait(false); - await using (invocationClient.ConfigureAwait(false)) - { - return await InvokeClientAsync(invocationClient.Client, toolName, arguments, cancellationToken).ConfigureAwait(false); - } - } - finally - { - await this._clientLock.WaitAsync(CancellationToken.None).ConfigureAwait(false); - try - { - if (--this._activeProviderInvocations == 0) - { - this._providerInvocationsDrained!.SetResult(true); - } - } - finally - { - invocationCompleted.SetResult(true); - this._providerInvocationContext.Value = previousInvocation; - this._clientLock.Release(); - } - } + return await InvokeClientAsync(connection.Client, toolName, arguments, cancellationToken).ConfigureAwait(false); } - - McpClient client = await this.GetOrCreateClientAsync(serverUrl, serverLabel, headers, connectionName, cancellationToken).ConfigureAwait(false); - return await InvokeClientAsync(client, toolName, arguments, cancellationToken).ConfigureAwait(false); } private static async Task InvokeClientAsync( @@ -219,61 +175,36 @@ internal static McpServerToolResultContent CreateListToolsResultContent(IEnumera /// /// - /// Disposal is requested from an active provider-backed invocation, including a child execution context. + /// Disposal is requested from an active MCP invocation, including a child execution context. /// Dispose the handler from its owning scope instead. /// public async ValueTask DisposeAsync() { - for (ProviderInvocationContext? context = this._providerInvocationContext.Value; context is not null; context = context.Parent) + for (InvocationScope? context = this._currentInvocation.Value; context is not null; context = context.Parent) { if (!context.Completion.IsCompleted) { - throw new InvalidOperationException("Cannot dispose the MCP handler from an active provider-backed invocation."); + throw new InvalidOperationException("Cannot dispose the MCP handler from an active MCP invocation."); } } - Task? providerInvocations; - await this._clientLock.WaitAsync().ConfigureAwait(false); - try + Task? invocations; + lock (this._invocationLock) { this.ThrowIfDisposing(); this._disposing = true; - providerInvocations = this._providerInvocationsDrained?.Task; - } - finally - { - this._clientLock.Release(); + invocations = this._invocationsDrained?.Task; } - if (providerInvocations is not null) + if (invocations is not null) { - await providerInvocations.ConfigureAwait(false); + await invocations.ConfigureAwait(false); } - await this._clientLock.WaitAsync().ConfigureAwait(false); - try - { - foreach (ClientConnection client in this._clients.Values) - { - await client.DisposeAsync().ConfigureAwait(false); - } - - this._clients.Clear(); - - // Dispose only HttpClients that the handler created (not user-provided ones) - foreach (HttpClient httpClient in this._ownedHttpClients.Values) - { - httpClient.Dispose(); - } - - this._ownedHttpClients.Clear(); - } - finally + if (this._httpMessageInvoker.IsValueCreated) { - this._clientLock.Release(); + this._httpMessageInvoker.Value.Dispose(); } - - this._clientLock.Dispose(); } [System.Diagnostics.CodeAnalysis.SuppressMessage("Maintainability", "CA1513:Use ObjectDisposedException throw helper", @@ -286,66 +217,37 @@ private void ThrowIfDisposing() } } - private async Task GetOrCreateClientAsync( - string serverUrl, - string? serverLabel, - IDictionary? headers, - string? connectionName, - CancellationToken cancellationToken) + private InvocationScope EnterInvocation() { - string trimmedUrl = serverUrl.Trim(); - var clientCacheKey = BuildCacheKey(trimmedUrl, serverLabel, connectionName, headers); - - await this._clientLock.WaitAsync(cancellationToken).ConfigureAwait(false); - try + lock (this._invocationLock) { this.ThrowIfDisposing(); - if (this._clients.TryGetValue(clientCacheKey, out ClientConnection? existingClient)) + if (this._activeInvocations++ == 0) { - return existingClient.Client; + this._invocationsDrained = new(TaskCreationOptions.RunContinuationsAsynchronously); } - - ClientConnection newClient = await this.CreateClientAsync(trimmedUrl, serverLabel, headers, trimmedUrl, cancellationToken).ConfigureAwait(false); - this._clients[clientCacheKey] = newClient; - return newClient.Client; - } - finally - { - this._clientLock.Release(); } - } - /// - /// Builds the per-client cache key as a 4-tuple of - /// (trimmed serverUrl, serverLabel, connectionName, headers hash). All four components - /// participate so that callers using different labels/connections/headers receive - /// distinct instances even when targeting the same URL. - /// - internal static (string Url, string Label, string Connection, string HeadersHash) BuildCacheKey( - string trimmedUrl, - string? serverLabel, - string? connectionName, - IDictionary? headers) => - (trimmedUrl, serverLabel ?? string.Empty, connectionName ?? string.Empty, ComputeHeadersHash(headers)); + InvocationScope invocation = new(this, this._currentInvocation.Value); + this._currentInvocation.Value = invocation; + return invocation; + } private async Task CreateClientAsync( string serverUrl, string? serverLabel, IDictionary? headers, - string? httpClientCacheKey, CancellationToken cancellationToken) { - // Only the no-provider path shares handler-owned HTTP clients. + Uri endpoint = new(serverUrl, UriKind.Absolute); HttpClient? httpClient = null; - bool ownsHttpClient = false; - if (this._httpClientProvider is not null) { httpClient = await this._httpClientProvider(serverUrl, cancellationToken).ConfigureAwait(false); } - if (httpClient is null && - (httpClientCacheKey is null || !this._ownedHttpClients.TryGetValue(httpClientCacheKey, out httpClient))) + bool ownsHttpClient = httpClient is null; + if (httpClient is null) { // Pin credential headers to the configured server origin as defense-in-depth. Forcing // StreamableHttp (below) already removes the primary vector (a server-advertised cross-origin @@ -359,21 +261,16 @@ private async Task CreateClientAsync( // SDK's built-in OAuth flow. If OAuth support is ever added here, requests to the authorization // server (a different origin than the MCP resource server) legitimately carry credentials, so // those auth-server origins would need to be allow-listed to avoid breaking the token exchange. - OriginPinningHandler pinningHandler = new(new Uri(serverUrl)) { InnerHandler = this._httpMessageHandlerFactory() }; - httpClient = new HttpClient(pinningHandler); - if (httpClientCacheKey is null) - { - ownsHttpClient = true; - } - else + OriginPinningHandler pinningHandler = new(endpoint) { - this._ownedHttpClients[httpClientCacheKey] = httpClient; - } + InnerHandler = new SharedHttpMessageHandler(this._httpMessageInvoker.Value) + }; + httpClient = new HttpClient(pinningHandler); } HttpClientTransportOptions transportOptions = new() { - Endpoint = new Uri(serverUrl), + Endpoint = endpoint, Name = serverLabel ?? "McpClient", AdditionalHeaders = headers, // Use Streamable HTTP rather than AutoDetect so the client does not negotiate down to the @@ -383,16 +280,24 @@ private async Task CreateClientAsync( TransportMode = HttpTransportMode.StreamableHttp }; - HttpClientTransport transport = new(transportOptions, httpClient, ownsHttpClient: ownsHttpClient); - + HttpClientTransport? transport = null; try { + transport = new(transportOptions, httpClient, ownsHttpClient: ownsHttpClient); McpClient client = await McpClient.CreateAsync(transport, cancellationToken: cancellationToken).ConfigureAwait(false); return new ClientConnection(client, transport); } catch { - await DisposeResourceAsync(transport, "transport").ConfigureAwait(false); + if (transport is not null) + { + await DisposeResourceAsync(transport, "transport").ConfigureAwait(false); + } + else if (ownsHttpClient) + { + httpClient?.Dispose(); + } + throw; } } @@ -406,11 +311,34 @@ private static HttpMessageHandler CreateHttpMessageHandler() => CheckCertificateRevocationList = true }; - private sealed class ProviderInvocationContext(Task completion, ProviderInvocationContext? parent) + private sealed class InvocationScope(DefaultMcpToolHandler owner, InvocationScope? parent) : IDisposable { - public Task Completion { get; } = completion; + private readonly TaskCompletionSource _completion = new(TaskCreationOptions.RunContinuationsAsynchronously); + + public Task Completion => this._completion.Task; + + public InvocationScope? Parent { get; } = parent; + + public void Dispose() + { + lock (owner._invocationLock) + { + this._completion.SetResult(true); + if (--owner._activeInvocations == 0) + { + owner._invocationsDrained!.SetResult(true); + } + } + + owner._currentInvocation.Value = this.Parent; + } + } - public ProviderInvocationContext? Parent { get; } = parent; + // Each invocation disposes its own pipeline without disposing the handler-owned connection pool. + private sealed class SharedHttpMessageHandler(HttpMessageInvoker invoker) : HttpMessageHandler + { + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) => + invoker.SendAsync(request, cancellationToken); } internal sealed class ClientConnection(McpClient client, IAsyncDisposable transport) : IAsyncDisposable @@ -444,51 +372,6 @@ private static async ValueTask DisposeResourceAsync(T resource, string resour } } - /// - /// Computes a deterministic, order-independent hash of the header set. - /// Header names are lower-cased for case-insensitive matching (RFC 7230 §3.2). - /// Header values remain case-sensitive (RFC 7235 — credentials are case-sensitive). - /// -#pragma warning disable CA1308 // RFC 7230 §3.2 requires lower-cased header names for case-insensitive comparison; CA1308's uppercase preference does not apply here - internal static string ComputeHeadersHash(IDictionary? headers) - { - if (headers is null || headers.Count == 0) - { - return string.Empty; - } - - // Sort by lower-cased key for deterministic ordering, preserving value case. - SortedDictionary sorted = new(StringComparer.Ordinal); - foreach (KeyValuePair header in headers) - { - sorted[header.Key.ToLowerInvariant()] = header.Value; - } - - StringBuilder payload = new(); - foreach (KeyValuePair kvp in sorted) - { - payload.Append(kvp.Key).Append(':').Append(kvp.Value).Append('\n'); - } - - byte[] inputBytes = Encoding.UTF8.GetBytes(payload.ToString()); -#if NET5_0_OR_GREATER - byte[] hashBytes = SHA256.HashData(inputBytes); -#else - using SHA256 sha256 = SHA256.Create(); - byte[] hashBytes = sha256.ComputeHash(inputBytes); -#endif - - // Convert to hex string (compatible with net472/netstandard2.0) - StringBuilder hex = new(hashBytes.Length * 2); - foreach (byte b in hashBytes) - { - hex.Append(b.ToString("X2", System.Globalization.CultureInfo.InvariantCulture)); - } - - return hex.ToString(); - } -#pragma warning restore CA1308 - private static void ThrowIfListToolsArgumentsSpecified(IDictionary? arguments) { if (arguments is { Count: > 0 }) diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md index 7cfd8de1341..36e0b5ed40a 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md @@ -58,16 +58,16 @@ resumes the registered invocation only after a matching approval response. ### MCP session lifetime -`DefaultMcpToolHandler` caches MCP sessions by server URL, label, connection name, -and explicit request headers when no custom HTTP client provider is configured. -Dispose the handler when its owning scope ends. - -When `httpClientProvider` is configured, each invocation (including `tools/list`) -calls the provider, opens a separate MCP session, and disposes that session before -completing. This also applies when the provider returns the same `HttpClient` or -returns `null` for the default transport. Caller-supplied HTTP clients remain -caller-owned; handler-created fallback clients are disposed with the invocation. -Handler disposal waits for active provider-backed invocations to finish cleanup. +`DefaultMcpToolHandler` creates a separate MCP client for every invocation, +including `tools/list`, and disposes it before completing. This applies with or +without `httpClientProvider`, even when the provider returns the same +`HttpClient` or `null`. The provider is still called for every invocation. + +Each handler-created HTTP client is disposed with its invocation. These clients +reuse a handler-owned HTTP connection pool, while their request headers and +endpoint-specific policy remain separate. Caller-supplied HTTP clients remain +caller-owned. Dispose the handler when its owning scope ends; disposal waits for +all active invocations to finish cleanup before releasing its HTTP handler. Disposal from within an active invocation (including a provider callback or its child tasks) throws `InvalidOperationException` without starting shutdown, rather than waiting on itself. Dispose the handler from its owning scope. @@ -75,11 +75,14 @@ Non-cancellation session and transport cleanup failures are reported through `System.Diagnostics.Trace` warnings and do not replace a tool result or its original error. -Provider-backed calls therefore incur session setup per invocation and do not -preserve server-side session state between calls. Applications requiring session -continuity should implement `IMcpToolHandler` with an explicit authentication -ownership and session lifetime contract, rather than sharing sessions based only -on the server address. +This changes the previous no-provider behavior, which retained initialized MCP +clients across calls. Calls now incur any protocol session setup per invocation +and do not preserve implicit server-side session state between calls, including +across approval waits or checkpoint resumes. Keep durable workflow state in +workflow variables/checkpoints and pass explicit remote state handles as tool +arguments where supported. Applications requiring session continuity should +implement `IMcpToolHandler` with an explicit authentication ownership and session +lifetime contract. HTTP connection pooling does not provide MCP session continuity. ### ⚙️ Foundry Actions diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs index 3a481fcd0e6..77c1b02e4e8 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs @@ -24,6 +24,28 @@ namespace Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests; Justification = "Provider callbacks return completed tasks; each invocation and handler disposal is awaited before disposing caller-owned clients.")] public sealed class DefaultMcpToolHandlerLifetimeTests { + [Fact] + public async Task NoProvider_SequentialInvocations_UseSeparateSessionsAsync() + { + // Arrange + ProtocolStub stub = new(); + await using DefaultMcpToolHandler handler = new(null, stub.CreateMessageHandler); + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + + // Act + McpServerToolResultContent first = await InvokeAsync(handler, "session", timeout.Token); + McpServerToolResultContent second = await InvokeAsync(handler, "session", timeout.Token); + + // Assert + Assert.NotNull(first.Outputs); + Assert.NotNull(second.Outputs); + Assert.NotEqual( + Assert.IsType(Assert.Single(first.Outputs)).Text, + Assert.IsType(Assert.Single(second.Outputs)).Text); + Assert.Equal(2, stub.Initializations); + Assert.Equal(2, stub.Terminations); + } + [Theory] [InlineData("ping")] [InlineData(DefaultMcpToolHandler.ListToolsToolName)] @@ -54,12 +76,12 @@ public async Task Provider_SequentialInvocations_CreateAndDisposeSeparateSession } [Fact] - public async Task ProviderReturningNull_DisposesFallbackTransportPerInvocationAsync() + public async Task ProviderReturningNull_ReusesHttpHandlerButNotSessionsAsync() { // Arrange ProtocolStub stub = new(); int providerCalls = 0; - await using DefaultMcpToolHandler handler = new((_, _) => + DefaultMcpToolHandler handler = new((_, _) => { providerCalls++; return Task.FromResult(null); @@ -74,15 +96,14 @@ public async Task ProviderReturningNull_DisposesFallbackTransportPerInvocationAs Assert.Equal(2, providerCalls); Assert.Equal(2, stub.Initializations); Assert.Equal(2, stub.Terminations); - Assert.Equal(2, stub.Handlers.Count); - foreach (Mock transport in stub.Handlers) - { - transport.Protected().Verify("Dispose", Times.AtLeastOnce(), ItExpr.Is(disposing => disposing)); - } + Mock transport = Assert.Single(stub.Handlers); + transport.Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); + await handler.DisposeAsync(); + transport.Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); } [Fact] - public async Task NoProvider_ReusesSessionUntilHandlerDisposalAsync() + public async Task NoProvider_DiscoveryAndToolCalls_ReuseHttpHandlerButNotSessionsAsync() { // Arrange ProtocolStub stub = new(); @@ -94,18 +115,18 @@ public async Task NoProvider_ReusesSessionUntilHandlerDisposalAsync() await InvokeAsync(handler, DefaultMcpToolHandler.ListToolsToolName, timeout.Token); // Assert - Assert.Equal(1, stub.Initializations); - Assert.Equal(0, stub.Terminations); + Assert.Equal(2, stub.Initializations); + Assert.Equal(2, stub.Terminations); Mock transport = Assert.Single(stub.Handlers); transport.Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); await handler.DisposeAsync(); - Assert.Equal(1, stub.Terminations); - transport.Protected().Verify("Dispose", Times.AtLeastOnce(), ItExpr.Is(disposing => disposing)); + Assert.Equal(2, stub.Terminations); + transport.Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); } [Fact] - public async Task NoProvider_DifferentConnectionNames_UseSeparateCachedSessionsAsync() + public async Task NoProvider_DifferentConnectionNames_DoNotRetainSessionsAsync() { // Arrange ProtocolStub stub = new(); @@ -120,11 +141,130 @@ await handler.InvokeToolAsync( } // Assert - Assert.Equal(2, stub.Initializations); + Assert.Equal(3, stub.Initializations); Assert.Single(stub.Handlers); - Assert.Equal(0, stub.Terminations); + Assert.Equal(3, stub.Terminations); await handler.DisposeAsync(); + Assert.Equal(3, stub.Terminations); + } + + [Fact] + public async Task NoProvider_DifferentOrigins_KeepRequestConfigurationAsync() + { + // Arrange + ProtocolStub stub = new(); + await using DefaultMcpToolHandler handler = new(null, stub.CreateMessageHandler); + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + + // Act + foreach (string name in new[] { "first", "second" }) + { + Uri endpoint = new($"https://{name}.example/mcp"); + string headerValue = $"Bearer {name}"; + stub.OnOperation = request => + { + Assert.Equal(endpoint, request.RequestUri); + Assert.Equal(headerValue, Assert.Single(request.Headers.GetValues("Authorization"))); + }; + await handler.InvokeToolAsync( + endpoint.AbsoluteUri, name, "ping", null, + new Dictionary { ["Authorization"] = headerValue }, name, timeout.Token); + } + + // Assert + Assert.Equal(2, stub.Initializations); Assert.Equal(2, stub.Terminations); + Assert.Single(stub.Handlers); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task NoProvider_FailedInvocation_DoesNotPreventNextInvocationAsync(bool cancel) + { + // Arrange + ProtocolStub stub = new() { FailOperation = !cancel }; + await using DefaultMcpToolHandler handler = new(null, stub.CreateMessageHandler); + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + using CancellationTokenSource cancellation = CancellationTokenSource.CreateLinkedTokenSource(timeout.Token); + using SemaphoreSlim started = new(0); + if (cancel) + { + stub.BeforeOperationAsync = async token => + { + started.Release(); + await Task.Delay(Timeout.Infinite, token); + }; + } + + // Act + Task invocation = InvokeAsync(handler, "ping", cancellation.Token); + if (cancel) + { + await started.WaitAsync(timeout.Token); + cancellation.Cancel(); + await Assert.ThrowsAnyAsync(() => invocation); + } + else + { + await Assert.ThrowsAsync(() => invocation); + } + + stub.BeforeOperationAsync = null; + stub.FailOperation = false; + McpServerToolResultContent result = await InvokeAsync(handler, "ping", timeout.Token); + + // Assert + Assert.NotNull(result.Outputs); + Assert.Equal("ok", Assert.IsType(Assert.Single(result.Outputs)).Text); + Assert.Equal(2, stub.Initializations); + Assert.Equal(2, stub.Terminations); + Assert.Single(stub.Handlers); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task DisposeAsync_WaitsForDefaultInvocationCleanupAsync(bool pauseInitialization) + { + // Arrange + ProtocolStub stub = new(); + using SemaphoreSlim started = new(0); + using SemaphoreSlim finish = new(0); + async Task PauseAsync(CancellationToken token) + { + started.Release(); + await finish.WaitAsync(token); + } + + if (pauseInitialization) + { + stub.BeforeInitializationAsync = PauseAsync; + } + else + { + stub.BeforeOperationAsync = PauseAsync; + } + + DefaultMcpToolHandler handler = new(null, stub.CreateMessageHandler); + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + + // Act + Task invocation = InvokeAsync(handler, "ping", timeout.Token); + await started.WaitAsync(timeout.Token); + Task disposal = handler.DisposeAsync().AsTask(); + Assert.False(disposal.IsCompleted); + await Assert.ThrowsAsync(() => InvokeAsync(handler, "ping", timeout.Token)); + Mock transport = Assert.Single(stub.Handlers); + transport.Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); + finish.Release(); + await invocation; + await disposal; + + // Assert + Assert.Equal(1, stub.Initializations); + Assert.Equal(1, stub.Terminations); + transport.Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); } [Fact] @@ -150,12 +290,14 @@ public async Task Provider_Failure_DoesNotPreventHandlerDisposalAsync() Assert.Equal(1, providerCalls); } - [Fact] - public async Task Provider_ConcurrentInvocations_DoNotCoalesceOrSerializeSessionsAsync() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ConcurrentInvocations_DoNotCoalesceOrSerializeSessionsAsync(bool useProvider) { // Arrange ProtocolStub stub = new(); - using HttpClient client = new(stub.CreateMessageHandler()); + using HttpClient? client = useProvider ? new(stub.CreateMessageHandler()) : null; using SemaphoreSlim started = new(0); using SemaphoreSlim finish = new(0); stub.BeforeOperationAsync = async token => @@ -164,25 +306,34 @@ public async Task Provider_ConcurrentInvocations_DoNotCoalesceOrSerializeSession await finish.WaitAsync(token); }; int providerCalls = 0; - await using DefaultMcpToolHandler handler = new((_, _) => + await using DefaultMcpToolHandler handler = new(useProvider ? (_, _) => { Interlocked.Increment(ref providerCalls); - return Task.FromResult(client); - }); + return Task.FromResult(client); + } + : null, stub.CreateMessageHandler); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); // Act - Task first = InvokeAsync(handler, "ping", timeout.Token); - Task second = InvokeAsync(handler, "ping", timeout.Token); + Task first = InvokeAsync(handler, "session", timeout.Token); + Task second = InvokeAsync(handler, "session", timeout.Token); await started.WaitAsync(timeout.Token); await started.WaitAsync(timeout.Token); finish.Release(2); - await Task.WhenAll(first, second); + McpServerToolResultContent[] results = await Task.WhenAll(first, second); // Assert - Assert.Equal(2, providerCalls); + Assert.Equal(useProvider ? 2 : 0, providerCalls); Assert.Equal(2, stub.Initializations); Assert.Equal(2, stub.Terminations); + IList? firstOutputs = results[0].Outputs; + IList? secondOutputs = results[1].Outputs; + Assert.NotNull(firstOutputs); + Assert.NotNull(secondOutputs); + Assert.NotEqual( + Assert.IsType(Assert.Single(firstOutputs)).Text, + Assert.IsType(Assert.Single(secondOutputs)).Text); + Assert.Single(stub.Handlers); } [Fact] @@ -204,15 +355,16 @@ public async Task Provider_OperationFailure_DisposesSessionAndPreservesCallerCli } [Theory] - [InlineData(false)] - [InlineData(true)] - public async Task Provider_InitializationFailure_RespectsTransportOwnershipAsync(bool provideClient) + [InlineData(false, false)] + [InlineData(true, false)] + [InlineData(true, true)] + public async Task InitializationFailure_RespectsTransportOwnershipAsync(bool useProvider, bool provideClient) { // Arrange ProtocolStub stub = new() { FailInitialization = true }; using HttpClient? client = provideClient ? new(stub.CreateMessageHandler()) : null; - await using DefaultMcpToolHandler handler = new( - (_, _) => Task.FromResult(client), stub.CreateMessageHandler); + DefaultMcpToolHandler handler = new( + useProvider ? (_, _) => Task.FromResult(client) : null, stub.CreateMessageHandler); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); // Act @@ -220,7 +372,9 @@ public async Task Provider_InitializationFailure_RespectsTransportOwnershipAsync // Assert Mock transport = Assert.Single(stub.Handlers); - transport.Protected().Verify("Dispose", provideClient ? Times.Never() : Times.AtLeastOnce(), ItExpr.Is(disposing => disposing)); + transport.Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); + await handler.DisposeAsync(); + transport.Protected().Verify("Dispose", provideClient ? Times.Never() : Times.Once(), ItExpr.Is(disposing => disposing)); } [Fact] @@ -334,7 +488,7 @@ public async Task Provider_ReentrantDisposal_ThrowsWithoutStartingShutdownAsync( async Task DisposeAsync() => await handler!.DisposeAsync(); InvalidOperationException error = await Assert.ThrowsAsync( () => fromChildTask ? Task.Run(DisposeAsync) : DisposeAsync()); - Assert.Contains("active provider-backed invocation", error.Message); + Assert.Contains("active MCP invocation", error.Message); rejectedDisposals++; return client; }); @@ -413,39 +567,33 @@ public async Task Provider_NestedInvocation_RestoresOuterDisposalGuardAsync() [Theory] [InlineData(false)] [InlineData(true)] - public async Task Provider_TransportCleanupFailure_PreservesInvocationOutcomeAsync(bool failOperation) + public async Task DisposeAsync_HttpHandlerCleanupFailure_DoesNotReplaceInvocationOutcomeAsync(bool failOperation) { // Arrange ProtocolStub stub = new() { FailOperation = failOperation, FailTransportDisposal = true }; - await using DefaultMcpToolHandler handler = new( - (_, _) => Task.FromResult(null), stub.CreateMessageHandler); + DefaultMcpToolHandler handler = new(null, stub.CreateMessageHandler); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); - using CleanupTraceListener listener = new(); - Trace.Listeners.Add(listener); - try - { - // Act - if (failOperation) - { - await Assert.ThrowsAsync(() => InvokeAsync(handler, "ping", timeout.Token)); - } - else - { - McpServerToolResultContent result = await InvokeAsync(handler, "ping", timeout.Token); - Assert.Equal("ok", Assert.IsType(Assert.Single(result.Outputs!)).Text); - } - // Assert - Assert.Equal(1, stub.Terminations); - Assert.Contains("Failed to dispose MCP transport", listener.Output); - Assert.Contains("transport cleanup failed", listener.Output); - Assert.Single(stub.Handlers).Protected().Verify( - "Dispose", Times.AtLeastOnce(), ItExpr.Is(disposing => disposing)); + // Act + if (failOperation) + { + await Assert.ThrowsAsync(() => InvokeAsync(handler, "ping", timeout.Token)); } - finally + else { - Trace.Listeners.Remove(listener); + McpServerToolResultContent result = await InvokeAsync(handler, "ping", timeout.Token); + Assert.NotNull(result.Outputs); + Assert.Equal("ok", Assert.IsType(Assert.Single(result.Outputs)).Text); } + + InvalidOperationException exception = await Assert.ThrowsAsync( + () => handler.DisposeAsync().AsTask()); + + // Assert + Assert.Equal(1, stub.Terminations); + Assert.Equal("transport cleanup failed", exception.Message); + Assert.Single(stub.Handlers).Protected().Verify( + "Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); } [Fact] @@ -489,29 +637,22 @@ public async Task Provider_CompletedNestedContext_StillGuardsActiveOuterInvocati } [Fact] - public async Task Provider_InitializationAndCleanupFailure_PreservesInitializationErrorAsync() + public async Task InitializationFailure_ReportsHttpHandlerCleanupFailureAtShutdownAsync() { // Arrange ProtocolStub stub = new() { FailInitialization = true, FailTransportDisposal = true }; - await using DefaultMcpToolHandler handler = new( - (_, _) => Task.FromResult(null), stub.CreateMessageHandler); + DefaultMcpToolHandler handler = new(null, stub.CreateMessageHandler); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); - using CleanupTraceListener listener = new(); - Trace.Listeners.Add(listener); - try - { - // Act - await Assert.ThrowsAsync(() => InvokeAsync(handler, "ping", timeout.Token)); - // Assert - Assert.Contains("Failed to dispose MCP transport", listener.Output); - Assert.Single(stub.Handlers).Protected().Verify( - "Dispose", Times.AtLeastOnce(), ItExpr.Is(disposing => disposing)); - } - finally - { - Trace.Listeners.Remove(listener); - } + // Act + await Assert.ThrowsAsync(() => InvokeAsync(handler, "ping", timeout.Token)); + InvalidOperationException exception = await Assert.ThrowsAsync( + () => handler.DisposeAsync().AsTask()); + + // Assert + Assert.Equal("transport cleanup failed", exception.Message); + Assert.Single(stub.Handlers).Protected().Verify( + "Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); } [Theory] @@ -568,7 +709,9 @@ private sealed class ProtocolStub public int Initializations => this._initializations; public int Terminations => this._terminations; public List> Handlers { get; } = []; + public Func? BeforeInitializationAsync { get; set; } public Func? BeforeOperationAsync { get; set; } + public Action? OnOperation { get; set; } public bool FailInitialization { get; set; } public bool FailOperation { get; set; } public bool FailTransportDisposal { get; set; } @@ -629,6 +772,11 @@ private async Task SendAsync(HttpRequestMessage request, Ca string? sessionId = null; if (method == "initialize") { + if (this.BeforeInitializationAsync is not null) + { + await this.BeforeInitializationAsync(cancellationToken); + } + if (this.FailInitialization) { return EmptyResponse(HttpStatusCode.BadRequest, request); @@ -640,6 +788,7 @@ private async Task SendAsync(HttpRequestMessage request, Ca } else { + this.OnOperation?.Invoke(request); if (this.BeforeOperationAsync is not null) { await this.BeforeOperationAsync(cancellationToken); @@ -653,6 +802,8 @@ private async Task SendAsync(HttpRequestMessage request, Ca result = method switch { "tools/list" => """{"tools":[]}""", + "tools/call" when message.GetProperty("params").GetProperty("name").GetString() == "session" => + $$$"""{"content":[{"type":"text","text":"{{{Assert.Single(request.Headers.GetValues("Mcp-Session-Id"))}}}"}]}""", "tools/call" => """{"content":[{"type":"text","text":"ok"}]}""", _ => throw new InvalidOperationException($"Unexpected MCP method: {method}") }; diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs index aa1fb0440aa..b610bc717e5 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs @@ -82,7 +82,7 @@ public async Task DisposeAsync_WhenCalledMultipleTimes_ShouldHandleGracefullyAsy await handler.DisposeAsync(); async Task actAsync() => await handler.DisposeAsync(); - // Assert - Second dispose should throw ObjectDisposedException from the semaphore + // Assert await Assert.ThrowsAsync(actAsync); } @@ -227,7 +227,7 @@ await handler.InvokeToolAsync( #endregion - #region Caching Tests + #region Invocation Tests [Fact] public async Task InvokeToolAsync_SameServerUrl_ShouldCallProviderOncePerAttemptWhenConnectionFailsAsync() @@ -247,8 +247,6 @@ public async Task InvokeToolAsync_SameServerUrl_ShouldCallProviderOncePerAttempt try { // Act - Call twice with the same server URL - // Since there's no real server, the McpClient.CreateAsync will fail, - // so the client won't be cached and the provider will be called each time for (int i = 0; i < 2; i++) { try @@ -267,7 +265,7 @@ await handler.InvokeToolAsync( } } - // Assert - Provider is called each time because McpClient creation fails before caching + // Assert Assert.Equal(2, providerCallCount); } finally @@ -362,7 +360,7 @@ await handler.InvokeToolAsync( } } - // Assert - Different headers should create different cache keys + // Assert Assert.Equal(2, providerCallCount); } finally @@ -373,189 +371,6 @@ await handler.InvokeToolAsync( #endregion - #region ComputeHeadersHash Tests - - [Fact] - public void ComputeHeadersHash_WithNullHeaders_ReturnsEmptyString() - { - // Act - string result = DefaultMcpToolHandler.ComputeHeadersHash(null); - - // Assert - Assert.Equal(string.Empty, result); - } - - [Fact] - public void ComputeHeadersHash_WithEmptyHeaders_ReturnsEmptyString() - { - // Act - string result = DefaultMcpToolHandler.ComputeHeadersHash(new Dictionary()); - - // Assert - Assert.Equal(string.Empty, result); - } - - [Fact] - public void ComputeHeadersHash_SameHeadersDifferentOrder_ReturnsSameHash() - { - // Arrange - Dictionary headers1 = new() - { - ["Authorization"] = "Bearer token123", - ["X-Custom"] = "value1" - }; - Dictionary headers2 = new() - { - ["X-Custom"] = "value1", - ["Authorization"] = "Bearer token123" - }; - - // Act - string hash1 = DefaultMcpToolHandler.ComputeHeadersHash(headers1); - string hash2 = DefaultMcpToolHandler.ComputeHeadersHash(headers2); - - // Assert - Assert.Equal(hash2, hash1); - } - - [Fact] - public void ComputeHeadersHash_SameKeysDifferentCaseKeys_ReturnsSameHash() - { - // Arrange — RFC 7230: header names are case-insensitive - Dictionary headers1 = new() { ["Authorization"] = "Bearer token" }; - Dictionary headers2 = new() { ["authorization"] = "Bearer token" }; - - // Act - string hash1 = DefaultMcpToolHandler.ComputeHeadersHash(headers1); - string hash2 = DefaultMcpToolHandler.ComputeHeadersHash(headers2); - - // Assert - Assert.Equal(hash2, hash1); - } - - [Fact] - public void ComputeHeadersHash_SameKeysDifferentCaseValues_ReturnsDifferentHash() - { - // Arrange — RFC 7235: credentials are case-sensitive - Dictionary headers1 = new() { ["Authorization"] = "Bearer ABC" }; - Dictionary headers2 = new() { ["Authorization"] = "Bearer abc" }; - - // Act - string hash1 = DefaultMcpToolHandler.ComputeHeadersHash(headers1); - string hash2 = DefaultMcpToolHandler.ComputeHeadersHash(headers2); - - // Assert - Assert.NotEqual(hash2, hash1); - } - - [Fact] - public void ComputeHeadersHash_DifferentHeaders_ReturnsDifferentHash() - { - // Arrange - Dictionary headers1 = new() { ["Authorization"] = "Bearer token1" }; - Dictionary headers2 = new() { ["Authorization"] = "Bearer token2" }; - - // Act - string hash1 = DefaultMcpToolHandler.ComputeHeadersHash(headers1); - string hash2 = DefaultMcpToolHandler.ComputeHeadersHash(headers2); - - // Assert - Assert.NotEqual(hash2, hash1); - } - - #endregion - - #region Cache Key Discrimination Tests - - // These tests exercise BuildCacheKey directly because the integration path - // (InvokeToolAsync against a fake server) doesn't surface cache-hit behavior - // without standing up a real MCP server — McpClient.CreateAsync fails before - // _clients[key] = newClient runs, so nothing ever gets cached. - // Tuple equality on the returned 4-tuple verifies that the dimensions - // collectively discriminate cache entries. - - [Fact] - public void BuildCacheKey_SameInputs_ReturnsEqualKeys() - { - // Arrange - Dictionary headers = new() { ["Authorization"] = "Bearer token" }; - - // Act - var key1 = DefaultMcpToolHandler.BuildCacheKey("http://localhost/mcp", "label", "conn", headers); - var key2 = DefaultMcpToolHandler.BuildCacheKey("http://localhost/mcp", "label", "conn", headers); - - // Assert - Assert.Equal(key2, key1); - } - - [Fact] - public void BuildCacheKey_DifferentConnectionName_ReturnsDifferentKeys() - { - // Act - var key1 = DefaultMcpToolHandler.BuildCacheKey("http://localhost/mcp", "label", "connection-a", null); - var key2 = DefaultMcpToolHandler.BuildCacheKey("http://localhost/mcp", "label", "connection-b", null); - - // Assert - Assert.NotEqual(key2, key1); - Assert.Equal("connection-a", key1.Connection); - Assert.Equal("connection-b", key2.Connection); - } - - [Fact] - public void BuildCacheKey_DifferentServerLabel_ReturnsDifferentKeys() - { - // Act - var key1 = DefaultMcpToolHandler.BuildCacheKey("http://localhost/mcp", "label-a", null, null); - var key2 = DefaultMcpToolHandler.BuildCacheKey("http://localhost/mcp", "label-b", null, null); - - // Assert - Assert.NotEqual(key2, key1); - Assert.Equal("label-a", key1.Label); - Assert.Equal("label-b", key2.Label); - } - - [Fact] - public void BuildCacheKey_CaseSensitiveUrlPath_ReturnsDifferentKeys() - { - // Arrange — RFC 3986: URL path is case-sensitive - // Act - var key1 = DefaultMcpToolHandler.BuildCacheKey("http://localhost/Tools", null, null, null); - var key2 = DefaultMcpToolHandler.BuildCacheKey("http://localhost/tools", null, null, null); - - // Assert - Assert.NotEqual(key2, key1); - } - - [Fact] - public void BuildCacheKey_HeaderValuesCaseSensitive_ReturnsDifferentKeys() - { - // Arrange — RFC 7235: credentials are case-sensitive - Dictionary headers1 = new() { ["Authorization"] = "Bearer ABC" }; - Dictionary headers2 = new() { ["Authorization"] = "Bearer abc" }; - - // Act - var key1 = DefaultMcpToolHandler.BuildCacheKey("http://localhost/mcp", null, null, headers1); - var key2 = DefaultMcpToolHandler.BuildCacheKey("http://localhost/mcp", null, null, headers2); - - // Assert — header value case must propagate into the cache key - Assert.NotEqual(key2, key1); - Assert.NotEqual(key2.HeadersHash, key1.HeadersHash); - } - - [Fact] - public void BuildCacheKey_NullLabelAndConnection_NormalizesToEmptyString() - { - // Act - var key = DefaultMcpToolHandler.BuildCacheKey("http://localhost/mcp", null, null, null); - - // Assert — verifies null-safety contract callers rely on - Assert.Equal(string.Empty, key.Label); - Assert.Equal(string.Empty, key.Connection); - Assert.Equal(string.Empty, key.HeadersHash); - } - - #endregion - #region Reserved Tools/List Tests [Fact] diff --git a/skills/agent-framework-builder-dotnet/references/declarative.md b/skills/agent-framework-builder-dotnet/references/declarative.md index 636a61b284c..2d29b5ff6a1 100644 --- a/skills/agent-framework-builder-dotnet/references/declarative.md +++ b/skills/agent-framework-builder-dotnet/references/declarative.md @@ -22,6 +22,11 @@ commitments. - Supply the chat clients, project clients, connections, tools, and external handlers the definition references. - Provide handlers for HTTP, MCP, or Foundry actions when a workflow uses them. +- `DefaultMcpToolHandler` creates and disposes an MCP client per invocation, including discovery; it does not retain + implicit server-side session state across calls, approval waits, or checkpoint resumes. Use explicit state handles + or a custom handler with a documented session-lifetime contract when continuity is required. +- Dispose the default MCP handler when its host scope ends; its HTTP handler is reusable across invocations, but + supplied `HttpClient` instances remain caller-owned. - Keep configuration exposure restricted to intentional values. - Treat definitions as executable configuration and load them only from trusted sources. - Allowlist external destinations and register only resources the definition may invoke. From 9cb20a4e36b06ba0181193beddfa202c9c9b0a37 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Mon, 26 Oct 2026 13:52:15 -0400 Subject: [PATCH 03/13] fix(mcp): use factory-managed HTTP clients Create invocation-owned named factory clients, retain dedicated non-DI fallback and borrowed callback clients, and document host-owned admission and deadlines. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 27dcc08f-0b3a-40a6-a3ad-2c45eafce159 --- ...0047-dotnet-workflow-owned-mcp-sessions.md | 57 ++-- dotnet/Directory.Packages.props | 1 + .../DefaultMcpToolHandler.cs | 65 +++-- ...Agents.AI.Workflows.Declarative.Mcp.csproj | 1 + .../PublicAPI/net10.0/PublicAPI.Unshipped.txt | 1 + .../PublicAPI/net472/PublicAPI.Unshipped.txt | 1 + .../PublicAPI/net8.0/PublicAPI.Unshipped.txt | 1 + .../PublicAPI/net9.0/PublicAPI.Unshipped.txt | 1 + .../netstandard2.0/PublicAPI.Unshipped.txt | 1 + .../README.md | 42 ++- .../DefaultMcpToolHandlerLifetimeTests.cs | 270 ++++++++++++++---- .../DefaultMcpToolHandlerTests.cs | 2 +- .../references/declarative.md | 11 +- 13 files changed, 359 insertions(+), 95 deletions(-) diff --git a/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md b/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md index df23dac951d..70a4aa2aa4f 100644 --- a/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md +++ b/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md @@ -181,6 +181,13 @@ client from before the approval. Restoring a checkpoint likewise creates the resources required for subsequent calls; checkpoints do not serialize clients, transports, HTTP objects, or promises of protocol-session reattachment. +The host owns admission limits, invocation deadlines, and shutdown cancellation. +It should bound submitted work, pass cancellation covering the entire invocation, +and cancel and await calls before disposing the handler. Handler disposal stops +admitting new calls and drains admitted invocations without cancelling them. It +does not impose a new global concurrency limit or a fixed shutdown deadline that +could interrupt supported long-running tools. + Do not automatically retry a tool invocation merely because the connection was lost. The server may already have performed side effects even if the result was not received. Newer protocol stream cancellation is not an application rollback. @@ -205,6 +212,10 @@ disposed by the MCP integration. `IHttpClientFactory` belongs to `Microsoft.Extensions.Http` and does not require ASP.NET. The application may use the Generic Host or a `ServiceCollection`. +The additive `DefaultMcpToolHandler(IHttpClientFactory, string)` constructor uses +the application's stable named-client configuration on every invocation. The +required name parameter avoids ambiguity with the existing single-argument +callback constructor, including `new DefaultMcpToolHandler(null)`. Do not create a service provider or factory per invocation. Use a bounded set of stable factory configuration names, not names derived from workflow IDs, credentials, or arbitrary server URLs. @@ -217,17 +228,22 @@ provide pooling. ### Transport policy -Internally created clients retain explicit defaults: cookies disabled, automatic +Non-DI fallback clients retain explicit defaults: cookies disabled, automatic redirects disabled, certificate revocation checking, Streamable HTTP, and origin-bound credential handling. Factory defaults alone are not a substitute. -The current `OriginPinningHandler` stores one origin. It cannot be placed in a -single pooled pipeline for unrelated origins. A connection-specific outer -policy handler over a factory-managed pipeline, following the documented -`IHttpMessageHandlerFactory` pattern, or an equivalently validated request-specific -policy must preserve this protection. Origin policy must run after credential -injection in outbound processing. Caller-supplied clients remain under the -caller's transport-policy contract; do not replace their pipelines silently. +Application factory clients enter under the same transport-policy contract as +callback-supplied clients; the integration does not silently replace their +pipelines. Configure their primary handlers with cookies and automatic redirects +disabled and certificate revocation checking enabled. Any credential-injecting +handler must restrict credentials to the configured server origin. Streamable +HTTP remains enforced regardless of the HTTP client's source. + +The fallback's `OriginPinningHandler` stores one origin and is owned by one +invocation. Do not move this mutable policy into one pooled pipeline for +unrelated origins. Factory handlers must instead use an appropriately bounded, +stable configuration or validated request-specific policy. Origin checks must +account for every credential-injection step in outbound processing. Wrapper disposal must not disrupt other invocations sharing a factory-managed handler. Conversely, factory handler expiry must not terminate an active call. @@ -378,23 +394,30 @@ handler/connection reuse before claiming a performance improvement. - [Named clients](https://learn.microsoft.com/en-us/dotnet/core/extensions/httpclient-factory#named-clients): avoid unbounded configuration names. -### Implementation scope and remaining design work +### Implementation scope This proposal targets the .NET declarative MCP integration. It does not implement the new protocol, add interactive elicitation or subscriptions, redesign Python, change result conversion, or introduce durable MCP reconnection. -The implementation keeps one lazily created, handler-owned `HttpMessageInvoker` -for the default HTTP pipeline. Each invocation owns a separate MCP client, -transport, HTTP wrapper, and origin-policy handler. Its forwarding handler borrows -the shared invoker rather than disposing it. Handler shutdown drains admitted -invocations before disposing the shared HTTP pipeline. No workflow-runtime +The implementation uses an application-provided `IHttpClientFactory` for +reusable HTTP infrastructure. Each invocation requests a fresh named HTTP +wrapper, owns its separate MCP client and transport, and disposes the wrapper +after MCP cleanup. The factory owns handler pooling and rotation; the MCP handler +does not dispose the application factory or its shared handlers. + +The non-DI path creates and owns a dedicated HTTP handler and origin-policy +handler for each invocation. The existing provider callback continues to borrow +application-supplied clients under its existing contract; returning `null` selects +the dedicated fallback. Handler shutdown drains admitted invocations. No +handler-owned shared invoker, internal service provider, workflow-runtime ownership hooks, connection registry, HTTP-client cache, or lease-counting pool are introduced. -This does not add an `IHttpClientFactory` API or dependency. Factory integration -above remains guidance for a future additive API. The existing provider callback -continues to borrow application-supplied clients under its existing contract. +The package references `Microsoft.Extensions.Http` for all supported target +frameworks. The additive constructor is recorded in each target's public API +baseline. Factory integration is the reusable-transport path, not deferred work +or a bespoke connection-rotation implementation. Deciders must review the invocation boundary, legacy-cache migration, transport configuration, and explicitly stateful escape hatch before accepting the ADR. diff --git a/dotnet/Directory.Packages.props b/dotnet/Directory.Packages.props index a87bf69d948..6574c1bbe73 100644 --- a/dotnet/Directory.Packages.props +++ b/dotnet/Directory.Packages.props @@ -101,6 +101,7 @@ + diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs index 0e566dec06f..cdee4e10b23 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs @@ -26,7 +26,8 @@ namespace Microsoft.Agents.AI.Workflows.Declarative.Mcp; /// The callback allows different MCP servers to use different authentication configurations by returning /// a pre-configured for each server. /// Every invocation creates and disposes a separate MCP client, including tools/list. -/// Handler-created HTTP clients reuse an underlying HTTP handler without sharing MCP session state. +/// Factory-created HTTP clients reuse factory-managed handlers without sharing MCP session state. +/// Without a factory or supplied client, each invocation owns a dedicated HTTP handler. /// Non-cancellation session and transport cleanup failures are reported through warnings without replacing /// the invocation result or error. /// @@ -43,7 +44,9 @@ public sealed class DefaultMcpToolHandler : IMcpToolHandler, IAsyncDisposable private static readonly JsonWriterOptions s_toolListJsonWriterOptions = new() { Indented = true }; private readonly Func>? _httpClientProvider; - private readonly Lazy _httpMessageInvoker; + private readonly Func _httpMessageHandlerFactory; + private readonly IHttpClientFactory? _httpClientFactory; + private readonly string _httpClientName = string.Empty; private readonly object _invocationLock = new(); private readonly AsyncLocal _currentInvocation = new(); private TaskCompletionSource? _invocationsDrained; @@ -72,18 +75,44 @@ public sealed class DefaultMcpToolHandler : IMcpToolHandler, IAsyncDisposable /// a different origin cannot capture the Authorization token. /// /// + [System.Diagnostics.CodeAnalysis.SuppressMessage("ApiDesign", "RS0027:Public API with optional parameter(s) should have the most parameters amongst its public overloads", + Justification = "Preserve the released callback constructor. The factory overload requires two arguments and cannot make existing calls ambiguous.")] public DefaultMcpToolHandler(Func>? httpClientProvider = null) : this(httpClientProvider, CreateHttpMessageHandler) { } + /// + /// Initializes a new instance of the class using an application-provided HTTP client factory. + /// + /// + /// The factory that creates a separate for every invocation. + /// Factory-created clients are disposed after MCP cleanup; the factory and its pooled handlers remain application-owned. + /// + /// + /// The stable named-client configuration to use, or an empty string for the default configuration. + /// Do not derive names from workflow identifiers, credentials, or arbitrary server URLs. + /// + /// + /// As with callback-supplied clients, the application is responsible for the HTTP pipeline's transport policy. + /// Disable cookies and automatic redirects, enable certificate revocation checking, and restrict credential injection + /// to the configured server origin. Pooled handlers must not retain per-invocation credentials or mutable origin state. + /// Configure a finite handler lifetime so new invocations can pick up endpoint changes without disrupting active calls. + /// + /// A required argument is . + public DefaultMcpToolHandler(IHttpClientFactory httpClientFactory, string httpClientName) + : this(null, CreateHttpMessageHandler) + { + this._httpClientFactory = Throw.IfNull(httpClientFactory); + this._httpClientName = Throw.IfNull(httpClientName); + } + internal DefaultMcpToolHandler( Func>? httpClientProvider, Func httpMessageHandlerFactory) { this._httpClientProvider = httpClientProvider; - Throw.IfNull(httpMessageHandlerFactory); - this._httpMessageInvoker = new(() => new HttpMessageInvoker(httpMessageHandlerFactory(), disposeHandler: true)); + this._httpMessageHandlerFactory = Throw.IfNull(httpMessageHandlerFactory); } /// @@ -178,6 +207,10 @@ internal static McpServerToolResultContent CreateListToolsResultContent(IEnumera /// Disposal is requested from an active MCP invocation, including a child execution context. /// Dispose the handler from its owning scope instead. /// + /// + /// Stops admitting new invocations and waits for admitted invocations to finish cleanup without cancelling them. + /// The host owns concurrency limits and invocation deadlines and should cancel and await its calls before disposal. + /// public async ValueTask DisposeAsync() { for (InvocationScope? context = this._currentInvocation.Value; context is not null; context = context.Parent) @@ -200,11 +233,6 @@ public async ValueTask DisposeAsync() { await invocations.ConfigureAwait(false); } - - if (this._httpMessageInvoker.IsValueCreated) - { - this._httpMessageInvoker.Value.Dispose(); - } } [System.Diagnostics.CodeAnalysis.SuppressMessage("Maintainability", "CA1513:Use ObjectDisposedException throw helper", @@ -247,7 +275,11 @@ private async Task CreateClientAsync( } bool ownsHttpClient = httpClient is null; - if (httpClient is null) + if (httpClient is null && this._httpClientFactory is not null) + { + httpClient = this._httpClientFactory.CreateClient(this._httpClientName); + } + else if (httpClient is null) { // Pin credential headers to the configured server origin as defense-in-depth. Forcing // StreamableHttp (below) already removes the primary vector (a server-advertised cross-origin @@ -263,7 +295,7 @@ private async Task CreateClientAsync( // those auth-server origins would need to be allow-listed to avoid breaking the token exchange. OriginPinningHandler pinningHandler = new(endpoint) { - InnerHandler = new SharedHttpMessageHandler(this._httpMessageInvoker.Value) + InnerHandler = this._httpMessageHandlerFactory() }; httpClient = new HttpClient(pinningHandler); } @@ -295,7 +327,7 @@ private async Task CreateClientAsync( } else if (ownsHttpClient) { - httpClient?.Dispose(); + httpClient.Dispose(); } throw; @@ -305,7 +337,7 @@ private async Task CreateClientAsync( private static HttpMessageHandler CreateHttpMessageHandler() => new HttpClientHandler { - // Keep cookies out of pooled transport state and prevent credential-bearing redirects. + // Keep cookies out of transport state and prevent credential-bearing redirects. UseCookies = false, AllowAutoRedirect = false, CheckCertificateRevocationList = true @@ -334,13 +366,6 @@ public void Dispose() } } - // Each invocation disposes its own pipeline without disposing the handler-owned connection pool. - private sealed class SharedHttpMessageHandler(HttpMessageInvoker invoker) : HttpMessageHandler - { - protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) => - invoker.SendAsync(request, cancellationToken); - } - internal sealed class ClientConnection(McpClient client, IAsyncDisposable transport) : IAsyncDisposable { public McpClient Client { get; } = client; diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/Microsoft.Agents.AI.Workflows.Declarative.Mcp.csproj b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/Microsoft.Agents.AI.Workflows.Declarative.Mcp.csproj index 0bb04cc608e..9b33f7001cd 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/Microsoft.Agents.AI.Workflows.Declarative.Mcp.csproj +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/Microsoft.Agents.AI.Workflows.Declarative.Mcp.csproj @@ -29,6 +29,7 @@ + diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt index ab058de62d4..8d635ccfca8 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt @@ -1 +1,2 @@ #nullable enable +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt index ab058de62d4..8d635ccfca8 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt @@ -1 +1,2 @@ #nullable enable +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt index ab058de62d4..8d635ccfca8 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt @@ -1 +1,2 @@ #nullable enable +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt index ab058de62d4..8d635ccfca8 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt @@ -1 +1,2 @@ #nullable enable +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt index ab058de62d4..8d635ccfca8 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt @@ -1 +1,2 @@ #nullable enable +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md index 36e0b5ed40a..89490e6af21 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md @@ -64,10 +64,44 @@ without `httpClientProvider`, even when the provider returns the same `HttpClient` or `null`. The provider is still called for every invocation. Each handler-created HTTP client is disposed with its invocation. These clients -reuse a handler-owned HTTP connection pool, while their request headers and -endpoint-specific policy remain separate. Caller-supplied HTTP clients remain -caller-owned. Dispose the handler when its owning scope ends; disposal waits for -all active invocations to finish cleanup before releasing its HTTP handler. +can reuse factory-managed HTTP handlers through the +`DefaultMcpToolHandler(IHttpClientFactory, string)` constructor. The second +argument is a stable named-client configuration, or `string.Empty` for the +default configuration. Factory-created wrappers are owned by the invocation; +the factory and its pooled handlers remain application-owned. Without a factory, +the fallback creates a dedicated HTTP handler per invocation. Clients supplied +through the existing callback remain caller-owned. + +Configure the factory once in the application, not once per invocation. For +example, with `Microsoft.Extensions.DependencyInjection`: + +```csharp +services.AddHttpClient("workflow-mcp") + .ConfigurePrimaryHttpMessageHandler(() => new HttpClientHandler + { + UseCookies = false, + AllowAutoRedirect = false, + CheckCertificateRevocationList = true + }) + .SetHandlerLifetime(TimeSpan.FromMinutes(2)); + +services.AddSingleton(serviceProvider => new DefaultMcpToolHandler( + serviceProvider.GetRequiredService(), "workflow-mcp")); +``` + +As with callback-supplied clients, factory clients use the application's +transport-policy configuration. Restrict credential-injecting handlers to the +configured server origin. Do not store per-workflow credentials, cookies, or +mutable origin policy in pooled handlers. Use a bounded set of configuration +names, not workflow IDs, credentials, or arbitrary URLs. A finite factory handler +lifetime lets subsequent invocations refresh connections without interrupting +active calls. Streamable HTTP remains enforced for every client source. + +Dispose the handler when its owning scope ends; disposal rejects new calls and +waits for all active invocations to finish cleanup without cancelling them. +The workflow host owns admission limits, invocation deadlines, and shutdown +cancellation. Bound concurrent work and pass a cancellation token covering the +whole invocation; cancel and await those calls before disposing the handler. Disposal from within an active invocation (including a provider callback or its child tasks) throws `InvalidOperationException` without starting shutdown, rather than waiting on itself. Dispose the handler from its owning scope. diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs index 77c1b02e4e8..265a05fafb1 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs @@ -11,6 +11,7 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.AI; +using Microsoft.Extensions.DependencyInjection; using ModelContextProtocol.Client; using Moq; using Moq.Protected; @@ -76,7 +77,7 @@ public async Task Provider_SequentialInvocations_CreateAndDisposeSeparateSession } [Fact] - public async Task ProviderReturningNull_ReusesHttpHandlerButNotSessionsAsync() + public async Task ProviderReturningNull_DisposesSeparateHttpClientsAndSessionsAsync() { // Arrange ProtocolStub stub = new(); @@ -96,14 +97,17 @@ public async Task ProviderReturningNull_ReusesHttpHandlerButNotSessionsAsync() Assert.Equal(2, providerCalls); Assert.Equal(2, stub.Initializations); Assert.Equal(2, stub.Terminations); - Mock transport = Assert.Single(stub.Handlers); - transport.Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); + Assert.Equal(2, stub.Handlers.Count); + foreach (Mock transport in stub.Handlers) + { + transport.Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); + } + await handler.DisposeAsync(); - transport.Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); } [Fact] - public async Task NoProvider_DiscoveryAndToolCalls_ReuseHttpHandlerButNotSessionsAsync() + public async Task NoProvider_DiscoveryAndToolCalls_DisposeSeparateHttpClientsAndSessionsAsync() { // Arrange ProtocolStub stub = new(); @@ -117,12 +121,14 @@ public async Task NoProvider_DiscoveryAndToolCalls_ReuseHttpHandlerButNotSession // Assert Assert.Equal(2, stub.Initializations); Assert.Equal(2, stub.Terminations); - Mock transport = Assert.Single(stub.Handlers); - transport.Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); + Assert.Equal(2, stub.Handlers.Count); + foreach (Mock transport in stub.Handlers) + { + transport.Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); + } await handler.DisposeAsync(); Assert.Equal(2, stub.Terminations); - transport.Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); } [Fact] @@ -142,7 +148,7 @@ await handler.InvokeToolAsync( // Assert Assert.Equal(3, stub.Initializations); - Assert.Single(stub.Handlers); + Assert.Equal(3, stub.Handlers.Count); Assert.Equal(3, stub.Terminations); await handler.DisposeAsync(); Assert.Equal(3, stub.Terminations); @@ -174,7 +180,7 @@ await handler.InvokeToolAsync( // Assert Assert.Equal(2, stub.Initializations); Assert.Equal(2, stub.Terminations); - Assert.Single(stub.Handlers); + Assert.Equal(2, stub.Handlers.Count); } [Theory] @@ -219,13 +225,140 @@ public async Task NoProvider_FailedInvocation_DoesNotPreventNextInvocationAsync( Assert.Equal("ok", Assert.IsType(Assert.Single(result.Outputs)).Text); Assert.Equal(2, stub.Initializations); Assert.Equal(2, stub.Terminations); - Assert.Single(stub.Handlers); + Assert.Equal(2, stub.Handlers.Count); + foreach (Mock transport in stub.Handlers) + { + transport.Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); + } } [Theory] - [InlineData(false)] - [InlineData(true)] - public async Task DisposeAsync_WaitsForDefaultInvocationCleanupAsync(bool pauseInitialization) + [InlineData("session")] + [InlineData(DefaultMcpToolHandler.ListToolsToolName)] + public async Task Factory_SequentialInvocations_OwnClientsButReusePooledHandlersAsync(string toolName) + { + // Arrange + const string ClientName = "workflow-mcp"; + ProtocolStub stub = new() + { + OnOperation = request => Assert.Equal("configured", Assert.Single(request.Headers.GetValues("X-Client-Configuration"))) + }; + ServiceCollection services = new(); + services.AddHttpClient(ClientName, client => client.DefaultRequestHeaders.Add("X-Client-Configuration", "configured")) + .ConfigurePrimaryHttpMessageHandler(stub.CreateMessageHandler); + using ServiceProvider serviceProvider = services.BuildServiceProvider(); + IHttpClientFactory applicationFactory = serviceProvider.GetRequiredService(); + List clients = []; + Mock factory = new(); + Mock factoryOwner = factory.As(); + factory.Setup(f => f.CreateClient(ClientName)).Returns(() => + { + HttpClient client = applicationFactory.CreateClient(ClientName); + clients.Add(client); + return client; + }); + DefaultMcpToolHandler handler = new(factory.Object, ClientName); + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + + // Act + McpServerToolResultContent first = await InvokeAsync(handler, toolName, timeout.Token); + McpServerToolResultContent second = await InvokeAsync(handler, toolName, timeout.Token); + await handler.DisposeAsync(); + + // Assert + Assert.Equal(2, clients.Count); + Assert.NotSame(clients[0], clients[1]); + if (toolName == "session") + { + Assert.NotNull(first.Outputs); + Assert.NotNull(second.Outputs); + Assert.NotEqual( + Assert.IsType(Assert.Single(first.Outputs)).Text, + Assert.IsType(Assert.Single(second.Outputs)).Text); + } + + foreach (HttpClient client in clients) + { + await Assert.ThrowsAsync(() => client.GetAsync(new Uri("https://mcp.example/api"), timeout.Token)); + } + + Assert.Equal(2, stub.Initializations); + Assert.Equal(2, stub.Terminations); + factory.Verify(f => f.CreateClient(ClientName), Times.Exactly(2)); + factoryOwner.Verify(f => f.Dispose(), Times.Never); + Assert.Single(stub.Handlers).Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); + using HttpClient subsequentClient = applicationFactory.CreateClient(ClientName); + using HttpResponseMessage response = await subsequentClient.GetAsync(new Uri("https://mcp.example/api"), timeout.Token); + Assert.Equal(HttpStatusCode.MethodNotAllowed, response.StatusCode); + } + + [Theory] + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(false, false)] + public async Task Factory_InvocationFailure_DisposesClientAndAllowsNextInvocationAsync(bool failInitialization, bool failOperation) + { + // Arrange + ProtocolStub stub = new() { FailInitialization = failInitialization, FailOperation = failOperation }; + using HttpMessageInvoker poolOwner = new(stub.CreateMessageHandler()); + HttpMessageHandler pooledHandler = Assert.Single(stub.Handlers).Object; + List clients = []; + Mock factory = new(); + factory.Setup(f => f.CreateClient("workflow-mcp")).Returns(() => + { + HttpClient client = new(pooledHandler, disposeHandler: false); + clients.Add(client); + return client; + }); + using SemaphoreSlim started = new(0); + bool cancel = !failInitialization && !failOperation; + if (cancel) + { + stub.BeforeOperationAsync = async token => + { + started.Release(); + await Task.Delay(Timeout.Infinite, token); + }; + } + + await using DefaultMcpToolHandler handler = new(factory.Object, "workflow-mcp"); + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + using CancellationTokenSource cancellation = CancellationTokenSource.CreateLinkedTokenSource(timeout.Token); + + // Act + Task invocation = InvokeAsync(handler, "ping", cancellation.Token); + if (cancel) + { + await started.WaitAsync(timeout.Token); + cancellation.Cancel(); + await Assert.ThrowsAnyAsync(() => invocation); + } + else + { + await Assert.ThrowsAsync(() => invocation); + } + + HttpClient firstClient = Assert.Single(clients); + await Assert.ThrowsAsync(() => firstClient.GetAsync(new Uri("https://mcp.example/api"), timeout.Token)); + stub.BeforeOperationAsync = null; + stub.FailInitialization = false; + stub.FailOperation = false; + McpServerToolResultContent result = await InvokeAsync(handler, "ping", timeout.Token); + + // Assert + Assert.NotNull(result.Outputs); + Assert.Equal("ok", Assert.IsType(Assert.Single(result.Outputs)).Text); + Assert.Equal(2, clients.Count); + await Assert.ThrowsAsync(() => clients[1].GetAsync(new Uri("https://mcp.example/api"), timeout.Token)); + Assert.Single(stub.Handlers).Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); + } + + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public async Task DisposeAsync_WaitsForOwnedInvocationCleanupAsync(bool useFactory, bool pauseInitialization) { // Arrange ProtocolStub stub = new(); @@ -246,7 +379,12 @@ async Task PauseAsync(CancellationToken token) stub.BeforeOperationAsync = PauseAsync; } - DefaultMcpToolHandler handler = new(null, stub.CreateMessageHandler); + Mock factory = new(); + HttpClient? factoryClient = null; + factory.Setup(f => f.CreateClient("workflow-mcp")).Returns(() => factoryClient = new(stub.CreateMessageHandler())); + DefaultMcpToolHandler handler = useFactory + ? new(factory.Object, "workflow-mcp") + : new(null, stub.CreateMessageHandler); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); // Act @@ -265,6 +403,11 @@ async Task PauseAsync(CancellationToken token) Assert.Equal(1, stub.Initializations); Assert.Equal(1, stub.Terminations); transport.Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); + if (useFactory) + { + Assert.NotNull(factoryClient); + await Assert.ThrowsAsync(() => factoryClient.GetAsync(new Uri("https://mcp.example/api"), timeout.Token)); + } } [Fact] @@ -291,13 +434,18 @@ public async Task Provider_Failure_DoesNotPreventHandlerDisposalAsync() } [Theory] - [InlineData(false)] - [InlineData(true)] - public async Task ConcurrentInvocations_DoNotCoalesceOrSerializeSessionsAsync(bool useProvider) + [InlineData(false, false)] + [InlineData(true, false)] + [InlineData(false, true)] + public async Task ConcurrentInvocations_DoNotCoalesceOrSerializeSessionsAsync(bool useProvider, bool useFactory) { // Arrange ProtocolStub stub = new(); using HttpClient? client = useProvider ? new(stub.CreateMessageHandler()) : null; + using ServiceProvider? serviceProvider = useFactory + ? new ServiceCollection().AddHttpClient("workflow-mcp") + .ConfigurePrimaryHttpMessageHandler(stub.CreateMessageHandler).Services.BuildServiceProvider() + : null; using SemaphoreSlim started = new(0); using SemaphoreSlim finish = new(0); stub.BeforeOperationAsync = async token => @@ -306,11 +454,13 @@ public async Task ConcurrentInvocations_DoNotCoalesceOrSerializeSessionsAsync(bo await finish.WaitAsync(token); }; int providerCalls = 0; - await using DefaultMcpToolHandler handler = new(useProvider ? (_, _) => - { - Interlocked.Increment(ref providerCalls); - return Task.FromResult(client); - } + await using DefaultMcpToolHandler handler = serviceProvider is not null + ? new(serviceProvider.GetRequiredService(), "workflow-mcp") + : new(useProvider ? (_, _) => + { + Interlocked.Increment(ref providerCalls); + return Task.FromResult(client); + } : null, stub.CreateMessageHandler); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); @@ -333,7 +483,7 @@ public async Task ConcurrentInvocations_DoNotCoalesceOrSerializeSessionsAsync(bo Assert.NotEqual( Assert.IsType(Assert.Single(firstOutputs)).Text, Assert.IsType(Assert.Single(secondOutputs)).Text); - Assert.Single(stub.Handlers); + Assert.Equal(useProvider || useFactory ? 1 : 2, stub.Handlers.Count); } [Fact] @@ -372,7 +522,7 @@ public async Task InitializationFailure_RespectsTransportOwnershipAsync(bool use // Assert Mock transport = Assert.Single(stub.Handlers); - transport.Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); + transport.Protected().Verify("Dispose", provideClient ? Times.Never() : Times.Once(), ItExpr.Is(disposing => disposing)); await handler.DisposeAsync(); transport.Protected().Verify("Dispose", provideClient ? Times.Never() : Times.Once(), ItExpr.Is(disposing => disposing)); } @@ -567,33 +717,42 @@ public async Task Provider_NestedInvocation_RestoresOuterDisposalGuardAsync() [Theory] [InlineData(false)] [InlineData(true)] - public async Task DisposeAsync_HttpHandlerCleanupFailure_DoesNotReplaceInvocationOutcomeAsync(bool failOperation) + public async Task Invocation_HttpHandlerCleanupFailure_DoesNotReplaceOutcomeAsync(bool failOperation) { // Arrange ProtocolStub stub = new() { FailOperation = failOperation, FailTransportDisposal = true }; DefaultMcpToolHandler handler = new(null, stub.CreateMessageHandler); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); - // Act - if (failOperation) + using CleanupTraceListener listener = new(); + Trace.Listeners.Add(listener); + try { - await Assert.ThrowsAsync(() => InvokeAsync(handler, "ping", timeout.Token)); + // Act + if (failOperation) + { + await Assert.ThrowsAsync(() => InvokeAsync(handler, "ping", timeout.Token)); + } + else + { + McpServerToolResultContent result = await InvokeAsync(handler, "ping", timeout.Token); + Assert.NotNull(result.Outputs); + Assert.Equal("ok", Assert.IsType(Assert.Single(result.Outputs)).Text); + } + + await handler.DisposeAsync(); + + // Assert + Assert.Equal(1, stub.Terminations); + Assert.Contains("Failed to dispose MCP ", listener.Output); + Assert.Contains("transport cleanup failed", listener.Output); + Assert.Single(stub.Handlers).Protected().Verify( + "Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); } - else + finally { - McpServerToolResultContent result = await InvokeAsync(handler, "ping", timeout.Token); - Assert.NotNull(result.Outputs); - Assert.Equal("ok", Assert.IsType(Assert.Single(result.Outputs)).Text); + Trace.Listeners.Remove(listener); } - - InvalidOperationException exception = await Assert.ThrowsAsync( - () => handler.DisposeAsync().AsTask()); - - // Assert - Assert.Equal(1, stub.Terminations); - Assert.Equal("transport cleanup failed", exception.Message); - Assert.Single(stub.Handlers).Protected().Verify( - "Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); } [Fact] @@ -637,22 +796,31 @@ public async Task Provider_CompletedNestedContext_StillGuardsActiveOuterInvocati } [Fact] - public async Task InitializationFailure_ReportsHttpHandlerCleanupFailureAtShutdownAsync() + public async Task InitializationFailure_ReportsHttpHandlerCleanupFailureAsync() { // Arrange ProtocolStub stub = new() { FailInitialization = true, FailTransportDisposal = true }; DefaultMcpToolHandler handler = new(null, stub.CreateMessageHandler); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); - // Act - await Assert.ThrowsAsync(() => InvokeAsync(handler, "ping", timeout.Token)); - InvalidOperationException exception = await Assert.ThrowsAsync( - () => handler.DisposeAsync().AsTask()); + using CleanupTraceListener listener = new(); + Trace.Listeners.Add(listener); + try + { + // Act + await Assert.ThrowsAsync(() => InvokeAsync(handler, "ping", timeout.Token)); + await handler.DisposeAsync(); - // Assert - Assert.Equal("transport cleanup failed", exception.Message); - Assert.Single(stub.Handlers).Protected().Verify( - "Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); + // Assert + Assert.Contains("Failed to dispose MCP transport", listener.Output); + Assert.Contains("transport cleanup failed", listener.Output); + Assert.Single(stub.Handlers).Protected().Verify( + "Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); + } + finally + { + Trace.Listeners.Remove(listener); + } } [Theory] diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs index b610bc717e5..68022e544ba 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs @@ -34,7 +34,7 @@ public async Task Constructor_WithNoParameters_ShouldCreateInstanceAsync() public async Task Constructor_WithNullHttpClientProvider_ShouldCreateInstanceAsync() { // Act - DefaultMcpToolHandler handler = new(httpClientProvider: null); + DefaultMcpToolHandler handler = new(null); // Assert Assert.NotNull(handler); diff --git a/skills/agent-framework-builder-dotnet/references/declarative.md b/skills/agent-framework-builder-dotnet/references/declarative.md index 2d29b5ff6a1..d83ed72d1e4 100644 --- a/skills/agent-framework-builder-dotnet/references/declarative.md +++ b/skills/agent-framework-builder-dotnet/references/declarative.md @@ -25,8 +25,15 @@ commitments. - `DefaultMcpToolHandler` creates and disposes an MCP client per invocation, including discovery; it does not retain implicit server-side session state across calls, approval waits, or checkpoint resumes. Use explicit state handles or a custom handler with a documented session-lifetime contract when continuity is required. -- Dispose the default MCP handler when its host scope ends; its HTTP handler is reusable across invocations, but - supplied `HttpClient` instances remain caller-owned. +- Prefer `DefaultMcpToolHandler(IHttpClientFactory, string)` with an application-provided factory and stable named + configuration for HTTP handler reuse and rotation. Factory-created clients are disposed per invocation; the + factory remains application-owned. The non-DI fallback owns a dedicated HTTP handler per invocation, while + callback-supplied `HttpClient` instances remain caller-owned. +- Configure factory clients with cookies and redirects disabled, certificate revocation checking enabled, and + origin-bound credential injection. Do not retain per-workflow credentials or mutable origin state in pooled + handlers, and do not derive client names from workflow IDs, credentials, or arbitrary URLs. +- Bound concurrent MCP work and supply invocation deadlines/cancellation from the host. Handler disposal stops + admission and drains existing calls without cancelling them; cancel and await calls before disposing the handler. - Keep configuration exposure restricted to intentional values. - Treat definitions as executable configuration and load them only from trusted sources. - Allowlist external destinations and register only resources the definition may invoke. From bfeec8fe1be80fa4a5d8115ccc45e86542a4c98d Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Mon, 26 Oct 2026 14:11:20 -0400 Subject: [PATCH 04/13] fix(mcp): validate transport options before client acquisition Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 27dcc08f-0b3a-40a6-a3ad-2c45eafce159 --- .../DefaultMcpToolHandler.cs | 24 ++++++------ .../DefaultMcpToolHandlerLifetimeTests.cs | 39 +++++++++++++++++++ 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs index cdee4e10b23..af3d5122635 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs @@ -268,6 +268,18 @@ private async Task CreateClientAsync( CancellationToken cancellationToken) { Uri endpoint = new(serverUrl, UriKind.Absolute); + HttpClientTransportOptions transportOptions = new() + { + Endpoint = endpoint, + Name = serverLabel ?? "McpClient", + AdditionalHeaders = headers, + // Use Streamable HTTP rather than AutoDetect so the client does not negotiate down to the + // legacy HTTP+SSE transport, which trusts a server-advertised message endpoint. That + // server-controlled endpoint is the primary vector for redirecting the Authorization token + // to a different origin; Streamable HTTP keeps every request on the configured origin. + TransportMode = HttpTransportMode.StreamableHttp + }; + HttpClient? httpClient = null; if (this._httpClientProvider is not null) { @@ -300,18 +312,6 @@ private async Task CreateClientAsync( httpClient = new HttpClient(pinningHandler); } - HttpClientTransportOptions transportOptions = new() - { - Endpoint = endpoint, - Name = serverLabel ?? "McpClient", - AdditionalHeaders = headers, - // Use Streamable HTTP rather than AutoDetect so the client does not negotiate down to the - // legacy HTTP+SSE transport, which trusts a server-advertised message endpoint. That - // server-controlled endpoint is the primary vector for redirecting the Authorization token - // to a different origin; Streamable HTTP keeps every request on the configured origin. - TransportMode = HttpTransportMode.StreamableHttp - }; - HttpClientTransport? transport = null; try { diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs index 265a05fafb1..00b706e28be 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs @@ -47,6 +47,45 @@ public async Task NoProvider_SequentialInvocations_UseSeparateSessionsAsync() Assert.Equal(2, stub.Terminations); } + [Theory] + [InlineData("ftp://mcp.example/api", false, false)] + [InlineData("ftp://mcp.example/api", true, false)] + [InlineData("ftp://mcp.example/api", false, true)] + [InlineData("ws://mcp.example/api", false, false)] + [InlineData("ws://mcp.example/api", true, false)] + [InlineData("ws://mcp.example/api", false, true)] + public async Task UnsupportedScheme_DoesNotAcquireHttpClientAsync(string serverUrl, bool useProvider, bool useFactory) + { + // Arrange + using HttpClient client = new(); + using HttpMessageHandler messageHandler = Mock.Of(); + bool clientAcquired = false; + HttpClient AcquireClient() + { + clientAcquired = true; + return client; + } + + HttpMessageHandler CreateMessageHandler() + { + clientAcquired = true; + return messageHandler; + } + + Mock factory = new(); + factory.Setup(f => f.CreateClient("workflow-mcp")).Returns(AcquireClient); + await using DefaultMcpToolHandler handler = useFactory + ? new(factory.Object, "workflow-mcp") + : new(useProvider ? (_, _) => Task.FromResult(AcquireClient()) : null, CreateMessageHandler); + + // Act + await Assert.ThrowsAsync(() => handler.InvokeToolAsync( + serverUrl, null, "ping", null, null, null)); + + // Assert + Assert.False(clientAcquired); + } + [Theory] [InlineData("ping")] [InlineData(DefaultMcpToolHandler.ListToolsToolName)] From 51d7639cc7ba3940aea84f80c3f3114c08b81602 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Mon, 26 Oct 2026 15:19:24 -0400 Subject: [PATCH 05/13] fix(samples): use factory-managed MCP HTTP clients Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 27dcc08f-0b3a-40a6-a3ad-2c45eafce159 --- .../InvokeFoundryToolboxMcp/Program.cs | 82 +++++++------- .../Declarative/InvokeMcpTool/Program.cs | 101 ++++++++---------- 2 files changed, 83 insertions(+), 100 deletions(-) diff --git a/dotnet/samples/03-workflows/Declarative/InvokeFoundryToolboxMcp/Program.cs b/dotnet/samples/03-workflows/Declarative/InvokeFoundryToolboxMcp/Program.cs index 22a7a48deb2..73123815a5b 100644 --- a/dotnet/samples/03-workflows/Declarative/InvokeFoundryToolboxMcp/Program.cs +++ b/dotnet/samples/03-workflows/Declarative/InvokeFoundryToolboxMcp/Program.cs @@ -5,7 +5,6 @@ // through the reserved tools/list operation, then calls microsoft_docs_search from the workflow. using System.ClientModel; -using System.Collections.Concurrent; using System.Net.Http.Headers; using Azure.AI.Projects; using Azure.AI.Projects.Agents; @@ -13,6 +12,7 @@ using Azure.Identity; using Microsoft.Agents.AI.Workflows.Declarative.Mcp; using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; using OpenAI.Responses; using Shared.Foundry; using Shared.Workflows; @@ -30,6 +30,7 @@ namespace Demo.Workflows.Declarative.InvokeFoundryToolboxMcp; /// internal sealed class Program { + private const string McpHttpClientName = "foundry-toolbox-mcp"; private const string ToolboxNameSetting = "FOUNDRY_TOOLBOX_NAME"; private const string ToolboxApiVersionSetting = "FOUNDRY_AGENT_TOOLSET_API_VERSION"; private const string ToolboxMcpServerUrlSetting = "FOUNDRY_TOOLBOX_MCP_SERVER_URL"; @@ -73,51 +74,32 @@ public static async Task Main(string[] args) // Get input from command line or console string workflowInput = Application.GetInput(args); - // Create the MCP tool handler for invoking the Foundry toolbox MCP proxy. - ConcurrentBag createdHttpClients = []; - DefaultMcpToolHandler mcpToolHandler = new( - httpClientProvider: async (serverUrl, _) => + // Reuse HTTP handlers while each MCP invocation owns its client. + ServiceCollection services = new(); + services.AddHttpClient(McpHttpClientName) + .ConfigurePrimaryHttpMessageHandler(() => new HttpClientHandler { - await Task.CompletedTask.ConfigureAwait(false); - - if (!string.Equals(serverUrl, toolboxMcpServerUrl, StringComparison.OrdinalIgnoreCase)) - { - return null; - } - - FoundryToolboxBearerTokenHandler handler = new(credential) - { - InnerHandler = new HttpClientHandler() - }; - HttpClient httpClient = new(handler); - createdHttpClients.Add(httpClient); - return httpClient; - }); - - try - { - // Create the workflow factory with MCP tool provider - WorkflowFactory workflowFactory = new("InvokeFoundryToolboxMcp.yaml", foundryEndpoint) - { - Configuration = workflowConfiguration, - AllowedEnvironmentVariables = [ToolboxMcpServerUrlSetting, DocsServerLabelSetting, WebSearchToolNameSetting], - McpToolHandler = mcpToolHandler - }; - - // Execute the workflow - WorkflowRunner runner = new() { UseJsonCheckpoints = true }; - await runner.ExecuteAsync(workflowFactory.CreateWorkflow, workflowInput); - } - finally + UseCookies = false, + AllowAutoRedirect = false, + CheckCertificateRevocationList = true + }) + .AddHttpMessageHandler(() => new FoundryToolboxBearerTokenHandler(credential, new Uri(toolboxMcpServerUrl))) + .SetHandlerLifetime(TimeSpan.FromMinutes(2)); + await using ServiceProvider serviceProvider = services.BuildServiceProvider(); + await using DefaultMcpToolHandler mcpToolHandler = new( + serviceProvider.GetRequiredService(), McpHttpClientName); + + // Create the workflow factory with MCP tool provider + WorkflowFactory workflowFactory = new("InvokeFoundryToolboxMcp.yaml", foundryEndpoint) { - // Clean up connections and dispose created HttpClients - await mcpToolHandler.DisposeAsync(); + Configuration = workflowConfiguration, + AllowedEnvironmentVariables = [ToolboxMcpServerUrlSetting, DocsServerLabelSetting, WebSearchToolNameSetting], + McpToolHandler = mcpToolHandler + }; - foreach (HttpClient httpClient in createdHttpClients) - { - httpClient.Dispose(); - } - } + // Execute the workflow + WorkflowRunner runner = new() { UseJsonCheckpoints = true }; + await runner.ExecuteAsync(workflowFactory.CreateWorkflow, workflowInput); } private static async Task CreateAgentAsync(Uri foundryEndpoint, IConfiguration configuration, TokenCredential credential) @@ -180,17 +162,25 @@ private static async Task CreateSampleToolboxAsync(string name, string s private static string BuildToolboxMcpServerUrl(string toolboxEndpoint, string toolboxName, string apiVersion) => $"{toolboxEndpoint.TrimEnd('/')}/{toolboxName}/mcp?api-version={Uri.EscapeDataString(apiVersion)}"; - private sealed class FoundryToolboxBearerTokenHandler(TokenCredential credential) : DelegatingHandler + private sealed class FoundryToolboxBearerTokenHandler(TokenCredential credential, Uri serverEndpoint) : DelegatingHandler { private static readonly TokenRequestContext s_tokenContext = new(["https://ai.azure.com/.default"]); + private readonly string _serverOrigin = serverEndpoint.GetLeftPart(UriPartial.Authority); + protected override async Task SendAsync( HttpRequestMessage request, CancellationToken cancellationToken) { - AccessToken token = await credential.GetTokenAsync(s_tokenContext, cancellationToken); - request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token.Token); + Uri? requestUri = request.RequestUri; + if (requestUri is not null + && string.Equals(requestUri.Scheme, Uri.UriSchemeHttps, StringComparison.OrdinalIgnoreCase) + && string.Equals(requestUri.GetLeftPart(UriPartial.Authority), this._serverOrigin, StringComparison.OrdinalIgnoreCase)) + { + AccessToken token = await credential.GetTokenAsync(s_tokenContext, cancellationToken); + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token.Token); + } return await base.SendAsync(request, cancellationToken); } diff --git a/dotnet/samples/03-workflows/Declarative/InvokeMcpTool/Program.cs b/dotnet/samples/03-workflows/Declarative/InvokeMcpTool/Program.cs index 24d5a6e2671..3a7c022acbb 100644 --- a/dotnet/samples/03-workflows/Declarative/InvokeMcpTool/Program.cs +++ b/dotnet/samples/03-workflows/Declarative/InvokeMcpTool/Program.cs @@ -4,12 +4,14 @@ // server tools directly from a declarative workflow. MCP servers expose tools that can be // invoked to perform specific tasks, like searching documentation or executing operations. +using System.Net.Http.Headers; using Azure.AI.Projects; using Azure.AI.Projects.Agents; using Azure.Core; using Azure.Identity; using Microsoft.Agents.AI.Workflows.Declarative.Mcp; using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; using Shared.Foundry; using Shared.Workflows; @@ -41,6 +43,8 @@ namespace Demo.Workflows.Declarative.InvokeMcpTool; /// internal sealed class Program { + private const string McpHttpClientName = "workflow-mcp"; + public static async Task Main(string[] args) { // Initialize configuration @@ -53,65 +57,33 @@ public static async Task Main(string[] args) // Get input from command line or console string workflowInput = Application.GetInput(args); - // Create the MCP tool handler for invoking MCP server tools. - // The HttpClient callback allows configuring authentication per MCP server. - // Different MCP servers may require different authentication configurations. - // For Production scenarios, consider implementing a more robust HttpClient management strategy to reuse HttpClient instances and manage their lifetimes appropriately. - List createdHttpClients = []; // WARNING: DefaultAzureCredential is convenient for development but requires careful consideration in production. DefaultAzureCredential credential = new(); - DefaultMcpToolHandler mcpToolHandler = new( - httpClientProvider: async (serverUrl, cancellationToken) => - { - if (serverUrl.StartsWith("https://mcp.ai.azure.com", StringComparison.OrdinalIgnoreCase)) - { - // Acquire token for the Azure MCP server - AccessToken token = await credential.GetTokenAsync( - new TokenRequestContext(["https://mcp.ai.azure.com/.default"]), - cancellationToken); - - // Create HttpClient with Authorization header - HttpClient httpClient = new(); - httpClient.DefaultRequestHeaders.Authorization = - new System.Net.Http.Headers.AuthenticationHeaderValue("Bearer", token.Token); - createdHttpClients.Add(httpClient); - return httpClient; - } - - if (serverUrl.StartsWith("https://learn.microsoft.com", StringComparison.OrdinalIgnoreCase)) - { - // Microsoft Learn MCP server does not require authentication - HttpClient httpClient = new(); - createdHttpClients.Add(httpClient); - return httpClient; - } - - // Return null for unknown servers to use the default HttpClient without auth. - return null; - }); - - try - { - // Create the workflow factory with MCP tool provider - WorkflowFactory workflowFactory = new("InvokeMcpTool.yaml", foundryEndpoint) - { - McpToolHandler = mcpToolHandler - }; - // Execute the workflow - WorkflowRunner runner = new() { UseJsonCheckpoints = true }; - await runner.ExecuteAsync(workflowFactory.CreateWorkflow, workflowInput); - } - finally + // Reuse HTTP handlers while each MCP invocation owns its client. + ServiceCollection services = new(); + services.AddHttpClient(McpHttpClientName) + .ConfigurePrimaryHttpMessageHandler(() => new HttpClientHandler + { + UseCookies = false, + AllowAutoRedirect = false, + CheckCertificateRevocationList = true + }) + .AddHttpMessageHandler(() => new AzureMcpBearerTokenHandler(credential)) + .SetHandlerLifetime(TimeSpan.FromMinutes(2)); + await using ServiceProvider serviceProvider = services.BuildServiceProvider(); + await using DefaultMcpToolHandler mcpToolHandler = new( + serviceProvider.GetRequiredService(), McpHttpClientName); + + // Create the workflow factory with MCP tool provider + WorkflowFactory workflowFactory = new("InvokeMcpTool.yaml", foundryEndpoint) { - // Clean up connections and dispose created HttpClients - await mcpToolHandler.DisposeAsync(); + McpToolHandler = mcpToolHandler + }; - foreach (HttpClient httpClient in createdHttpClients) - { - httpClient.Dispose(); - } - } + // Execute the workflow + WorkflowRunner runner = new() { UseJsonCheckpoints = true }; + await runner.ExecuteAsync(workflowFactory.CreateWorkflow, workflowInput); } private static async Task CreateAgentAsync(Uri foundryEndpoint, IConfiguration configuration) @@ -138,4 +110,25 @@ Be concise and helpful in your responses. """ }; } + + private sealed class AzureMcpBearerTokenHandler(TokenCredential credential) : DelegatingHandler + { + private static readonly TokenRequestContext s_tokenContext = + new(["https://mcp.ai.azure.com/.default"]); + + protected override async Task SendAsync( + HttpRequestMessage request, + CancellationToken cancellationToken) + { + Uri? requestUri = request.RequestUri; + if (requestUri is not null + && string.Equals(requestUri.GetLeftPart(UriPartial.Authority), "https://mcp.ai.azure.com", StringComparison.OrdinalIgnoreCase)) + { + AccessToken token = await credential.GetTokenAsync(s_tokenContext, cancellationToken); + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token.Token); + } + + return await base.SendAsync(request, cancellationToken); + } + } } From 7cfbadaa339ea543064f771b0f5f4c4124ccb96c Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 27 Oct 2026 10:20:41 -0400 Subject: [PATCH 06/13] refactor: make declarative MCP handler stateless Remove callback-based construction and handler-level shutdown coordination. Keep resource ownership within each invocation and update callers, API baselines, tests, and migration guidance. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 27dcc08f-0b3a-40a6-a3ad-2c45eafce159 --- ...0047-dotnet-workflow-owned-mcp-sessions.md | 84 +-- .../InvokeFoundryToolboxMcp/Program.cs | 2 +- .../Declarative/InvokeMcpTool/Program.cs | 2 +- .../DefaultMcpToolHandler.cs | 161 +---- .../PublicAPI/net10.0/PublicAPI.Unshipped.txt | 3 + .../PublicAPI/net472/PublicAPI.Unshipped.txt | 3 + .../PublicAPI/net8.0/PublicAPI.Unshipped.txt | 3 + .../PublicAPI/net9.0/PublicAPI.Unshipped.txt | 3 + .../netstandard2.0/PublicAPI.Unshipped.txt | 3 + .../README.md | 34 +- .../InvokeToolWorkflowTest.cs | 3 - .../DefaultMcpToolHandlerLifetimeTests.cs | 605 +++--------------- .../DefaultMcpToolHandlerTests.cs | 380 +---------- .../references/declarative.md | 8 +- 14 files changed, 220 insertions(+), 1074 deletions(-) diff --git a/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md b/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md index 70a4aa2aa4f..a1d26bbf6e7 100644 --- a/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md +++ b/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md @@ -102,11 +102,11 @@ model. It does not change the Python implementation. - Preserve isolation between callers, workflows, and authentication identities. - Protect active operations during cancellation and disposal. - Keep durable workflow state independent of live network objects. -- Respect ownership and configuration of user-supplied HTTP clients. +- Respect ownership and configuration of application-provided HTTP infrastructure. - Delegate reusable HTTP handler management to standard .NET infrastructure. - Support console applications, worker services, and web applications without requiring ASP.NET or a DI container per invocation. -- Preserve existing transport protections and authentication-provider semantics. +- Preserve transport protections with explicit factory-based authentication. - Make compatibility, resource limits, and any stateful opt-in behavior explicit. ## Considered Options @@ -132,8 +132,9 @@ This revises the original workflow-lifetime ownership proposal. It does not establish that arbitrary MCP clients are interchangeable or safe to share across workflows. The default handler uses invocation-scoped clients; applications requiring cross-call session continuity supply an explicitly scoped integration. -The existing public invocation and provider contracts remain unchanged. Review -of the changed no-provider lifetime and its migration guidance remains required. +The public invocation contract remains unchanged. The callback constructor and +handler-level disposal contract are removed in favor of a stateless handler. +Review of these breaking changes and their migration guidance remains required. ### Security isolation remains a separate requirement @@ -169,11 +170,11 @@ consumption, and cleanup are part of that protected lifetime. Do not dispose resources immediately after starting an asynchronous call or receiving an intermediate result. -Cancellation is a request, not evidence that work has stopped. Handler shutdown -must reject new invocations and coordinate completion of admitted operations -before disposing resources they use. Initialization failure, cleanup failure, -and reentrant disposal must have explicit, observable handling rather than -leaking resources or deadlocking. +Cancellation is a request, not evidence that work has stopped. Each invocation +awaits its own cleanup before completing. Initialization and cleanup failures +must have explicit, observable handling rather than leaking resources. +The handler owns no shared disposable resources and has no shutdown operation, +active-call registry, synchronization lock, or reentrant-disposal guard. An approval continuation starts the approved invocation with the current authorized connection configuration. It does not require retaining an idle MCP @@ -183,10 +184,10 @@ transports, HTTP objects, or promises of protocol-session reattachment. The host owns admission limits, invocation deadlines, and shutdown cancellation. It should bound submitted work, pass cancellation covering the entire invocation, -and cancel and await calls before disposing the handler. Handler disposal stops -admitting new calls and drains admitted invocations without cancelling them. It -does not impose a new global concurrency limit or a fixed shutdown deadline that -could interrupt supported long-running tools. +and cancel and await calls before disposing the application's HTTP factory +infrastructure or service provider. The handler does not impose a new global +concurrency limit or a fixed shutdown deadline that could interrupt supported +long-running tools. Do not automatically retry a tool invocation merely because the connection was lost. The server may already have performed side effects even if the result was @@ -201,21 +202,21 @@ It is not supplied by retaining a workflow-lifetime connection. | Source of HTTP client | Use and disposal contract | |---|---| -| User-supplied client | Borrow as supplied; do not mutate shared configuration or dispose it | | Application-provided factory | Obtain an owned wrapper for the chosen client scope; dispose it after MCP cleanup; let the factory manage pooled handlers | -| No supplied client or factory | Preserve a non-DI path that creates and owns a client with documented transport defaults | +| No factory | Preserve a parameterless non-DI path that creates and owns a client with documented transport defaults | Owned wrappers must be disposed exactly once, including when initialization fails. MCP and transport cleanup must release owned HTTP resources even if -earlier cleanup fails. Borrowed clients and application factories must not be -disposed by the MCP integration. +earlier cleanup fails. Application factories must not be disposed by the MCP +integration. A factory must return a fresh client wrapper, not a borrowed shared +client. `IHttpClientFactory` belongs to `Microsoft.Extensions.Http` and does not require ASP.NET. The application may use the Generic Host or a `ServiceCollection`. -The additive `DefaultMcpToolHandler(IHttpClientFactory, string)` constructor uses +The `DefaultMcpToolHandler(IHttpClientFactory, string)` constructor uses the application's stable named-client configuration on every invocation. The -required name parameter avoids ambiguity with the existing single-argument -callback constructor, including `new DefaultMcpToolHandler(null)`. +name parameter selects the named configuration, with an empty string selecting +the default configuration. Do not create a service provider or factory per invocation. Use a bounded set of stable factory configuration names, not names derived from workflow IDs, credentials, or arbitrary server URLs. @@ -232,8 +233,7 @@ Non-DI fallback clients retain explicit defaults: cookies disabled, automatic redirects disabled, certificate revocation checking, Streamable HTTP, and origin-bound credential handling. Factory defaults alone are not a substitute. -Application factory clients enter under the same transport-policy contract as -callback-supplied clients; the integration does not silently replace their +The integration does not silently replace application factory clients' pipelines. Configure their primary handlers with cookies and automatic redirects disabled and certificate revocation checking enabled. Any credential-injecting handler must restrict credentials to the configured server origin. Streamable @@ -256,11 +256,15 @@ be established for every supported target; this proposal does not drop targets. ### Compatibility and explicitly stateful integrations -The current `httpClientProvider` callback runs for every invocation and creates -a fresh MCP session for every call, including discovery, even when it returns the -same HTTP client or `null`. Preserve that contract. The callback may select -different credentials on successive invocations and must not silently become a -once-per-workflow factory. +Remove the `httpClientProvider` callback constructor and the internal +`httpMessageHandlerFactory` callback seam. Applications migrate authentication +to named `IHttpClientFactory` configurations and origin-bound delegating handlers. +Credentials that change per invocation must be resolved per request, not retained +in pooled handlers. Tests use the same factory interface as applications. + +Remove `IAsyncDisposable` and `DisposeAsync` from `DefaultMcpToolHandler`. +Applications remove handler disposal and instead cancel and await their active +calls before disposing application-owned HTTP infrastructure. The previous non-provider path cached initialized clients by server URL, label, connection name, and explicit headers. Moving that path to invocation-scoped @@ -290,7 +294,9 @@ notes when implementation is proposed. - Good, because workflow state continuity alone does not require a workflow-session connection registry, idle-session LRU policy, or HTTP reference-counting layer. -- Neutral, because active-invocation tracking, safe shutdown, and explicit +- Good, because invocation-local resource ownership removes handler-level + admission/drain synchronization and reentrant-disposal bookkeeping. +- Neutral, because safe shutdown remains a host responsibility and explicit authentication boundaries remain necessary. - Bad, because invocation-scoped clients require initialization per invocation with older protocol revisions. @@ -307,16 +313,15 @@ Before implementation is accepted, add focused regression coverage for: inherit one another's MCP client state or credentials. - Approval waits and checkpoint resumes require no retained live connection; the resumed invocation uses the correct authorized configuration. -- Admission, initialization, cancellation, and shutdown do not cause early +- Initialization, cancellation, and host shutdown do not cause early disposal, duplicate cleanup, leaked resources, or deadlock. - A lost result does not silently trigger a duplicate side-effecting tool call. -- Owned wrappers are disposed once; borrowed clients and application factories - are not disposed; disposal does not disrupt another invocation's pooled HTTP - resources. +- Owned wrappers are disposed once; application factories are not disposed; + cleanup does not disrupt another invocation's pooled HTTP resources. - Shared handlers contain no per-caller mutable authentication or origin state; origin, redirect, cookie, and credential protections remain intact. -- Existing provider callbacks retain their per-invocation behavior, including - callbacks returning the same client or `null`. +- Factory-created wrappers are acquired per invocation and the configured named + pipeline is preserved. - Compatibility with the currently supported SDK and protocol revisions is preserved; tests do not assume the newer protocol is already deployed. - Any explicit stateful integration follows its documented isolation and release @@ -407,16 +412,15 @@ after MCP cleanup. The factory owns handler pooling and rotation; the MCP handle does not dispose the application factory or its shared handlers. The non-DI path creates and owns a dedicated HTTP handler and origin-policy -handler for each invocation. The existing provider callback continues to borrow -application-supplied clients under its existing contract; returning `null` selects -the dedicated fallback. Handler shutdown drains admitted invocations. No +handler for each invocation. Callback construction and handler-level disposal +are removed. The host awaits its own invocations during shutdown. No handler-owned shared invoker, internal service provider, workflow-runtime -ownership hooks, connection registry, HTTP-client cache, or lease-counting pool +ownership hooks, invocation registry, synchronization lock, HTTP-client cache, or lease-counting pool are introduced. The package references `Microsoft.Extensions.Http` for all supported target -frameworks. The additive constructor is recorded in each target's public API -baseline. Factory integration is the reusable-transport path, not deferred work +frameworks. New constructors and removed APIs are recorded in each target's public +API baseline. Factory integration is the reusable-transport path, not deferred work or a bespoke connection-rotation implementation. Deciders must review the invocation boundary, legacy-cache migration, transport diff --git a/dotnet/samples/03-workflows/Declarative/InvokeFoundryToolboxMcp/Program.cs b/dotnet/samples/03-workflows/Declarative/InvokeFoundryToolboxMcp/Program.cs index 73123815a5b..79c6ddbbefd 100644 --- a/dotnet/samples/03-workflows/Declarative/InvokeFoundryToolboxMcp/Program.cs +++ b/dotnet/samples/03-workflows/Declarative/InvokeFoundryToolboxMcp/Program.cs @@ -86,7 +86,7 @@ public static async Task Main(string[] args) .AddHttpMessageHandler(() => new FoundryToolboxBearerTokenHandler(credential, new Uri(toolboxMcpServerUrl))) .SetHandlerLifetime(TimeSpan.FromMinutes(2)); await using ServiceProvider serviceProvider = services.BuildServiceProvider(); - await using DefaultMcpToolHandler mcpToolHandler = new( + DefaultMcpToolHandler mcpToolHandler = new( serviceProvider.GetRequiredService(), McpHttpClientName); // Create the workflow factory with MCP tool provider diff --git a/dotnet/samples/03-workflows/Declarative/InvokeMcpTool/Program.cs b/dotnet/samples/03-workflows/Declarative/InvokeMcpTool/Program.cs index 3a7c022acbb..4c2e3752ec8 100644 --- a/dotnet/samples/03-workflows/Declarative/InvokeMcpTool/Program.cs +++ b/dotnet/samples/03-workflows/Declarative/InvokeMcpTool/Program.cs @@ -72,7 +72,7 @@ public static async Task Main(string[] args) .AddHttpMessageHandler(() => new AzureMcpBearerTokenHandler(credential)) .SetHandlerLifetime(TimeSpan.FromMinutes(2)); await using ServiceProvider serviceProvider = services.BuildServiceProvider(); - await using DefaultMcpToolHandler mcpToolHandler = new( + DefaultMcpToolHandler mcpToolHandler = new( serviceProvider.GetRequiredService(), McpHttpClientName); // Create the workflow factory with MCP tool provider diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs index af3d5122635..6fcf457f315 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs @@ -22,16 +22,15 @@ namespace Microsoft.Agents.AI.Workflows.Declarative.Mcp; /// Default implementation of using the MCP C# SDK. /// /// -/// This provider supports per-server authentication via the httpClientProvider callback. -/// The callback allows different MCP servers to use different authentication configurations by returning -/// a pre-configured for each server. /// Every invocation creates and disposes a separate MCP client, including tools/list. /// Factory-created HTTP clients reuse factory-managed handlers without sharing MCP session state. -/// Without a factory or supplied client, each invocation owns a dedicated HTTP handler. +/// Without a factory, each invocation owns a dedicated HTTP handler. +/// The handler owns no shared disposable resources. The host must await its invocations before disposing +/// the application-owned factory infrastructure. /// Non-cancellation session and transport cleanup failures are reported through warnings without replacing /// the invocation result or error. /// -public sealed class DefaultMcpToolHandler : IMcpToolHandler, IAsyncDisposable +public sealed class DefaultMcpToolHandler : IMcpToolHandler { private const string FilenameAdditionalPropertyName = "filename"; @@ -43,42 +42,18 @@ public sealed class DefaultMcpToolHandler : IMcpToolHandler, IAsyncDisposable private static readonly JsonWriterOptions s_toolListJsonWriterOptions = new() { Indented = true }; - private readonly Func>? _httpClientProvider; - private readonly Func _httpMessageHandlerFactory; private readonly IHttpClientFactory? _httpClientFactory; private readonly string _httpClientName = string.Empty; - private readonly object _invocationLock = new(); - private readonly AsyncLocal _currentInvocation = new(); - private TaskCompletionSource? _invocationsDrained; - private int _activeInvocations; - private bool _disposing; /// /// Initializes a new instance of the class. /// - /// - /// An optional callback that provides an for each MCP server. - /// The callback receives (serverUrl, cancellationToken) and should return an HttpClient - /// configured with any required authentication. Return to use a default HttpClient with no auth. - /// - /// The callback is invoked for each tool invocation. MCP sessions are not cached when a callback is - /// configured, even if it returns the same client or . MCP clients are also - /// invocation-scoped without a callback; server-side session state is not preserved between calls. - /// Supplied HTTP clients remain caller-owned. Applications requiring session continuity should provide an - /// with an explicitly scoped authentication and session lifetime. - /// - /// - /// Security: HttpClients created by this handler pin credential headers to the configured server - /// origin and disable auto-redirect. When you supply your own , you are - /// responsible for equivalent protection — attach credentials only for the configured server origin - /// (for example via a ) so a server-advertised endpoint or redirect on - /// a different origin cannot capture the Authorization token. - /// - /// - [System.Diagnostics.CodeAnalysis.SuppressMessage("ApiDesign", "RS0027:Public API with optional parameter(s) should have the most parameters amongst its public overloads", - Justification = "Preserve the released callback constructor. The factory overload requires two arguments and cannot make existing calls ambiguous.")] - public DefaultMcpToolHandler(Func>? httpClientProvider = null) - : this(httpClientProvider, CreateHttpMessageHandler) + /// + /// Each invocation owns its HTTP client and handler. Cookies and automatic redirects are disabled, + /// certificate revocation checking is enabled, and credential headers are pinned to the configured server origin. + /// Use the factory constructor for application-managed HTTP handler pooling and authentication. + /// + public DefaultMcpToolHandler() { } @@ -94,27 +69,18 @@ public DefaultMcpToolHandler(Func>? /// Do not derive names from workflow identifiers, credentials, or arbitrary server URLs. /// /// - /// As with callback-supplied clients, the application is responsible for the HTTP pipeline's transport policy. + /// The application is responsible for the HTTP pipeline's transport policy. /// Disable cookies and automatic redirects, enable certificate revocation checking, and restrict credential injection /// to the configured server origin. Pooled handlers must not retain per-invocation credentials or mutable origin state. /// Configure a finite handler lifetime so new invocations can pick up endpoint changes without disrupting active calls. /// /// A required argument is . public DefaultMcpToolHandler(IHttpClientFactory httpClientFactory, string httpClientName) - : this(null, CreateHttpMessageHandler) { this._httpClientFactory = Throw.IfNull(httpClientFactory); this._httpClientName = Throw.IfNull(httpClientName); } - internal DefaultMcpToolHandler( - Func>? httpClientProvider, - Func httpMessageHandlerFactory) - { - this._httpClientProvider = httpClientProvider; - this._httpMessageHandlerFactory = Throw.IfNull(httpMessageHandlerFactory); - } - /// public async Task InvokeToolAsync( string serverUrl, @@ -146,7 +112,6 @@ public async Task InvokeToolAsync( } cancellationToken.ThrowIfCancellationRequested(); - using InvocationScope invocation = this.EnterInvocation(); ClientConnection connection = await this.CreateClientAsync( serverUrl.Trim(), serverLabel, headers, cancellationToken).ConfigureAwait(false); await using (connection.ConfigureAwait(false)) @@ -202,65 +167,6 @@ internal static McpServerToolResultContent CreateListToolsResultContent(IEnumera return resultContent; } - /// - /// - /// Disposal is requested from an active MCP invocation, including a child execution context. - /// Dispose the handler from its owning scope instead. - /// - /// - /// Stops admitting new invocations and waits for admitted invocations to finish cleanup without cancelling them. - /// The host owns concurrency limits and invocation deadlines and should cancel and await its calls before disposal. - /// - public async ValueTask DisposeAsync() - { - for (InvocationScope? context = this._currentInvocation.Value; context is not null; context = context.Parent) - { - if (!context.Completion.IsCompleted) - { - throw new InvalidOperationException("Cannot dispose the MCP handler from an active MCP invocation."); - } - } - - Task? invocations; - lock (this._invocationLock) - { - this.ThrowIfDisposing(); - this._disposing = true; - invocations = this._invocationsDrained?.Task; - } - - if (invocations is not null) - { - await invocations.ConfigureAwait(false); - } - } - - [System.Diagnostics.CodeAnalysis.SuppressMessage("Maintainability", "CA1513:Use ObjectDisposedException throw helper", - Justification = "The helper is not available on .NET Framework or .NET Standard 2.0.")] - private void ThrowIfDisposing() - { - if (this._disposing) - { - throw new ObjectDisposedException(nameof(DefaultMcpToolHandler)); - } - } - - private InvocationScope EnterInvocation() - { - lock (this._invocationLock) - { - this.ThrowIfDisposing(); - if (this._activeInvocations++ == 0) - { - this._invocationsDrained = new(TaskCreationOptions.RunContinuationsAsynchronously); - } - } - - InvocationScope invocation = new(this, this._currentInvocation.Value); - this._currentInvocation.Value = invocation; - return invocation; - } - private async Task CreateClientAsync( string serverUrl, string? serverLabel, @@ -280,18 +186,12 @@ private async Task CreateClientAsync( TransportMode = HttpTransportMode.StreamableHttp }; - HttpClient? httpClient = null; - if (this._httpClientProvider is not null) - { - httpClient = await this._httpClientProvider(serverUrl, cancellationToken).ConfigureAwait(false); - } - - bool ownsHttpClient = httpClient is null; - if (httpClient is null && this._httpClientFactory is not null) + HttpClient httpClient; + if (this._httpClientFactory is not null) { httpClient = this._httpClientFactory.CreateClient(this._httpClientName); } - else if (httpClient is null) + else { // Pin credential headers to the configured server origin as defense-in-depth. Forcing // StreamableHttp (below) already removes the primary vector (a server-advertised cross-origin @@ -307,7 +207,7 @@ private async Task CreateClientAsync( // those auth-server origins would need to be allow-listed to avoid breaking the token exchange. OriginPinningHandler pinningHandler = new(endpoint) { - InnerHandler = this._httpMessageHandlerFactory() + InnerHandler = CreateHttpMessageHandler() }; httpClient = new HttpClient(pinningHandler); } @@ -315,7 +215,7 @@ private async Task CreateClientAsync( HttpClientTransport? transport = null; try { - transport = new(transportOptions, httpClient, ownsHttpClient: ownsHttpClient); + transport = new(transportOptions, httpClient, ownsHttpClient: true); McpClient client = await McpClient.CreateAsync(transport, cancellationToken: cancellationToken).ConfigureAwait(false); return new ClientConnection(client, transport); } @@ -325,7 +225,7 @@ private async Task CreateClientAsync( { await DisposeResourceAsync(transport, "transport").ConfigureAwait(false); } - else if (ownsHttpClient) + else { httpClient.Dispose(); } @@ -334,8 +234,8 @@ private async Task CreateClientAsync( } } - private static HttpMessageHandler CreateHttpMessageHandler() => - new HttpClientHandler + private static HttpClientHandler CreateHttpMessageHandler() => + new() { // Keep cookies out of transport state and prevent credential-bearing redirects. UseCookies = false, @@ -343,29 +243,6 @@ private static HttpMessageHandler CreateHttpMessageHandler() => CheckCertificateRevocationList = true }; - private sealed class InvocationScope(DefaultMcpToolHandler owner, InvocationScope? parent) : IDisposable - { - private readonly TaskCompletionSource _completion = new(TaskCreationOptions.RunContinuationsAsynchronously); - - public Task Completion => this._completion.Task; - - public InvocationScope? Parent { get; } = parent; - - public void Dispose() - { - lock (owner._invocationLock) - { - this._completion.SetResult(true); - if (--owner._activeInvocations == 0) - { - owner._invocationsDrained!.SetResult(true); - } - } - - owner._currentInvocation.Value = this.Parent; - } - } - internal sealed class ClientConnection(McpClient client, IAsyncDisposable transport) : IAsyncDisposable { public McpClient Client { get; } = client; diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt index 8d635ccfca8..ef812db0a70 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt @@ -1,2 +1,5 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler() -> void +*REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void +*REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DisposeAsync() -> System.Threading.Tasks.ValueTask diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt index 8d635ccfca8..ef812db0a70 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt @@ -1,2 +1,5 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler() -> void +*REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void +*REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DisposeAsync() -> System.Threading.Tasks.ValueTask diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt index 8d635ccfca8..ef812db0a70 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt @@ -1,2 +1,5 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler() -> void +*REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void +*REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DisposeAsync() -> System.Threading.Tasks.ValueTask diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt index 8d635ccfca8..ef812db0a70 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt @@ -1,2 +1,5 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler() -> void +*REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void +*REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DisposeAsync() -> System.Threading.Tasks.ValueTask diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt index 8d635ccfca8..ef812db0a70 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt @@ -1,2 +1,5 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler() -> void +*REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void +*REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DisposeAsync() -> System.Threading.Tasks.ValueTask diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md index 89490e6af21..61a407c1df1 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md @@ -59,9 +59,8 @@ resumes the registered invocation only after a matching approval response. ### MCP session lifetime `DefaultMcpToolHandler` creates a separate MCP client for every invocation, -including `tools/list`, and disposes it before completing. This applies with or -without `httpClientProvider`, even when the provider returns the same -`HttpClient` or `null`. The provider is still called for every invocation. +including `tools/list`, and disposes it before completing. The handler retains +no per-invocation state or shared disposable resources. Each handler-created HTTP client is disposed with its invocation. These clients can reuse factory-managed HTTP handlers through the @@ -69,8 +68,7 @@ can reuse factory-managed HTTP handlers through the argument is a stable named-client configuration, or `string.Empty` for the default configuration. Factory-created wrappers are owned by the invocation; the factory and its pooled handlers remain application-owned. Without a factory, -the fallback creates a dedicated HTTP handler per invocation. Clients supplied -through the existing callback remain caller-owned. +the parameterless constructor creates a dedicated HTTP handler per invocation. Configure the factory once in the application, not once per invocation. For example, with `Microsoft.Extensions.DependencyInjection`: @@ -89,27 +87,29 @@ services.AddSingleton(serviceProvider => new DefaultMcpToolHand serviceProvider.GetRequiredService(), "workflow-mcp")); ``` -As with callback-supplied clients, factory clients use the application's -transport-policy configuration. Restrict credential-injecting handlers to the -configured server origin. Do not store per-workflow credentials, cookies, or +Factory clients use the application's transport-policy configuration. +Restrict credential-injecting handlers to the configured server origin. +Do not store per-workflow credentials, cookies, or mutable origin policy in pooled handlers. Use a bounded set of configuration names, not workflow IDs, credentials, or arbitrary URLs. A finite factory handler lifetime lets subsequent invocations refresh connections without interrupting active calls. Streamable HTTP remains enforced for every client source. -Dispose the handler when its owning scope ends; disposal rejects new calls and -waits for all active invocations to finish cleanup without cancelling them. -The workflow host owns admission limits, invocation deadlines, and shutdown -cancellation. Bound concurrent work and pass a cancellation token covering the -whole invocation; cancel and await those calls before disposing the handler. -Disposal from within an active invocation (including a provider callback or its -child tasks) throws `InvalidOperationException` without starting shutdown, rather -than waiting on itself. Dispose the handler from its owning scope. +The handler is not disposable. The workflow host owns admission limits, +invocation deadlines, and shutdown cancellation. Bound concurrent work and pass +a cancellation token covering the whole invocation; cancel and await those calls +before disposing the application's HTTP factory infrastructure or service provider. Non-cancellation session and transport cleanup failures are reported through `System.Diagnostics.Trace` warnings and do not replace a tool result or its original error. -This changes the previous no-provider behavior, which retained initialized MCP +The callback-based constructor and handler-level `DisposeAsync`/`IAsyncDisposable` +contract have been removed. Migrate callback authentication to named factory +clients and origin-bound delegating handlers; every factory call must return a +fresh HTTP client wrapper owned by the invocation, not a borrowed shared client. +Remove handler `await using`/disposal calls and await active invocations in the host. + +This also changes the previous default behavior, which retained initialized MCP clients across calls. Calls now incur any protocol session setup per invocation and do not preserve implicit server-side session state between calls, including across approval waits or checkpoint resumes. Keep durable workflow state in diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.IntegrationTests/InvokeToolWorkflowTest.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.IntegrationTests/InvokeToolWorkflowTest.cs index 01db8d7d693..bcb4be325d6 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.IntegrationTests/InvokeToolWorkflowTest.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.IntegrationTests/InvokeToolWorkflowTest.cs @@ -229,9 +229,6 @@ private async Task RunInvokeMcpToolTestAsync( { AssertResultContains(workflowEvents, expectedResultContains); } - - // Cleanup - await mcpToolProvider.DisposeAsync().ConfigureAwait(false); } /// diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs index 00b706e28be..610959f8ba7 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs @@ -21,16 +21,14 @@ namespace Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests; /// /// Functional session-lifetime tests using an in-memory HTTP transport and inert tool results. /// -[System.Diagnostics.CodeAnalysis.SuppressMessage("Reliability", "CA2025:Do not pass disposable objects into unawaited tasks", - Justification = "Provider callbacks return completed tasks; each invocation and handler disposal is awaited before disposing caller-owned clients.")] public sealed class DefaultMcpToolHandlerLifetimeTests { [Fact] - public async Task NoProvider_SequentialInvocations_UseSeparateSessionsAsync() + public async Task SequentialInvocations_UseSeparateSessionsAsync() { // Arrange ProtocolStub stub = new(); - await using DefaultMcpToolHandler handler = new(null, stub.CreateMessageHandler); + DefaultMcpToolHandler handler = CreateHandler(stub); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); // Act @@ -48,109 +46,30 @@ public async Task NoProvider_SequentialInvocations_UseSeparateSessionsAsync() } [Theory] - [InlineData("ftp://mcp.example/api", false, false)] - [InlineData("ftp://mcp.example/api", true, false)] - [InlineData("ftp://mcp.example/api", false, true)] - [InlineData("ws://mcp.example/api", false, false)] - [InlineData("ws://mcp.example/api", true, false)] - [InlineData("ws://mcp.example/api", false, true)] - public async Task UnsupportedScheme_DoesNotAcquireHttpClientAsync(string serverUrl, bool useProvider, bool useFactory) + [InlineData("ftp://mcp.example/api", false)] + [InlineData("ftp://mcp.example/api", true)] + [InlineData("ws://mcp.example/api", false)] + [InlineData("ws://mcp.example/api", true)] + public async Task UnsupportedScheme_DoesNotAcquireHttpClientAsync(string serverUrl, bool useFactory) { // Arrange - using HttpClient client = new(); - using HttpMessageHandler messageHandler = Mock.Of(); - bool clientAcquired = false; - HttpClient AcquireClient() - { - clientAcquired = true; - return client; - } - - HttpMessageHandler CreateMessageHandler() - { - clientAcquired = true; - return messageHandler; - } - - Mock factory = new(); - factory.Setup(f => f.CreateClient("workflow-mcp")).Returns(AcquireClient); - await using DefaultMcpToolHandler handler = useFactory - ? new(factory.Object, "workflow-mcp") - : new(useProvider ? (_, _) => Task.FromResult(AcquireClient()) : null, CreateMessageHandler); + Mock factory = new(MockBehavior.Strict); + DefaultMcpToolHandler handler = useFactory ? new(factory.Object, "workflow-mcp") : new(); // Act await Assert.ThrowsAsync(() => handler.InvokeToolAsync( serverUrl, null, "ping", null, null, null)); // Assert - Assert.False(clientAcquired); - } - - [Theory] - [InlineData("ping")] - [InlineData(DefaultMcpToolHandler.ListToolsToolName)] - public async Task Provider_SequentialInvocations_CreateAndDisposeSeparateSessionsAsync(string toolName) - { - // Arrange - ProtocolStub stub = new(); - using HttpClient client = new(stub.CreateMessageHandler()); - int providerCalls = 0; - DefaultMcpToolHandler handler = new((_, _) => - { - providerCalls++; - return Task.FromResult(client); - }); - using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); - - // Act - await InvokeAsync(handler, toolName, timeout.Token); - Assert.Equal(1, stub.Terminations); - await InvokeAsync(handler, toolName, timeout.Token); - await handler.DisposeAsync(); - - // Assert - Assert.Equal(2, providerCalls); - Assert.Equal(2, stub.Initializations); - Assert.Equal(2, stub.Terminations); - stub.Handlers[0].Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); + factory.Verify(f => f.CreateClient(It.IsAny()), Times.Never); } [Fact] - public async Task ProviderReturningNull_DisposesSeparateHttpClientsAndSessionsAsync() + public async Task DiscoveryAndToolCalls_DisposeSeparateHttpClientsAndSessionsAsync() { // Arrange ProtocolStub stub = new(); - int providerCalls = 0; - DefaultMcpToolHandler handler = new((_, _) => - { - providerCalls++; - return Task.FromResult(null); - }, stub.CreateMessageHandler); - using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); - - // Act - await InvokeAsync(handler, "ping", timeout.Token); - await InvokeAsync(handler, "ping", timeout.Token); - - // Assert - Assert.Equal(2, providerCalls); - Assert.Equal(2, stub.Initializations); - Assert.Equal(2, stub.Terminations); - Assert.Equal(2, stub.Handlers.Count); - foreach (Mock transport in stub.Handlers) - { - transport.Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); - } - - await handler.DisposeAsync(); - } - - [Fact] - public async Task NoProvider_DiscoveryAndToolCalls_DisposeSeparateHttpClientsAndSessionsAsync() - { - // Arrange - ProtocolStub stub = new(); - DefaultMcpToolHandler handler = new(null, stub.CreateMessageHandler); + DefaultMcpToolHandler handler = CreateHandler(stub); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); // Act @@ -165,17 +84,14 @@ public async Task NoProvider_DiscoveryAndToolCalls_DisposeSeparateHttpClientsAnd { transport.Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); } - - await handler.DisposeAsync(); - Assert.Equal(2, stub.Terminations); } [Fact] - public async Task NoProvider_DifferentConnectionNames_DoNotRetainSessionsAsync() + public async Task DifferentConnectionNames_DoNotRetainSessionsAsync() { // Arrange ProtocolStub stub = new(); - DefaultMcpToolHandler handler = new(null, stub.CreateMessageHandler); + DefaultMcpToolHandler handler = CreateHandler(stub); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); // Act @@ -189,16 +105,14 @@ await handler.InvokeToolAsync( Assert.Equal(3, stub.Initializations); Assert.Equal(3, stub.Handlers.Count); Assert.Equal(3, stub.Terminations); - await handler.DisposeAsync(); - Assert.Equal(3, stub.Terminations); } [Fact] - public async Task NoProvider_DifferentOrigins_KeepRequestConfigurationAsync() + public async Task DifferentOrigins_KeepRequestConfigurationAsync() { // Arrange ProtocolStub stub = new(); - await using DefaultMcpToolHandler handler = new(null, stub.CreateMessageHandler); + DefaultMcpToolHandler handler = CreateHandler(stub); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); // Act @@ -222,55 +136,6 @@ await handler.InvokeToolAsync( Assert.Equal(2, stub.Handlers.Count); } - [Theory] - [InlineData(false)] - [InlineData(true)] - public async Task NoProvider_FailedInvocation_DoesNotPreventNextInvocationAsync(bool cancel) - { - // Arrange - ProtocolStub stub = new() { FailOperation = !cancel }; - await using DefaultMcpToolHandler handler = new(null, stub.CreateMessageHandler); - using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); - using CancellationTokenSource cancellation = CancellationTokenSource.CreateLinkedTokenSource(timeout.Token); - using SemaphoreSlim started = new(0); - if (cancel) - { - stub.BeforeOperationAsync = async token => - { - started.Release(); - await Task.Delay(Timeout.Infinite, token); - }; - } - - // Act - Task invocation = InvokeAsync(handler, "ping", cancellation.Token); - if (cancel) - { - await started.WaitAsync(timeout.Token); - cancellation.Cancel(); - await Assert.ThrowsAnyAsync(() => invocation); - } - else - { - await Assert.ThrowsAsync(() => invocation); - } - - stub.BeforeOperationAsync = null; - stub.FailOperation = false; - McpServerToolResultContent result = await InvokeAsync(handler, "ping", timeout.Token); - - // Assert - Assert.NotNull(result.Outputs); - Assert.Equal("ok", Assert.IsType(Assert.Single(result.Outputs)).Text); - Assert.Equal(2, stub.Initializations); - Assert.Equal(2, stub.Terminations); - Assert.Equal(2, stub.Handlers.Count); - foreach (Mock transport in stub.Handlers) - { - transport.Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); - } - } - [Theory] [InlineData("session")] [InlineData(DefaultMcpToolHandler.ListToolsToolName)] @@ -302,7 +167,6 @@ public async Task Factory_SequentialInvocations_OwnClientsButReusePooledHandlers // Act McpServerToolResultContent first = await InvokeAsync(handler, toolName, timeout.Token); McpServerToolResultContent second = await InvokeAsync(handler, toolName, timeout.Token); - await handler.DisposeAsync(); // Assert Assert.Equal(2, clients.Count); @@ -335,17 +199,15 @@ public async Task Factory_SequentialInvocations_OwnClientsButReusePooledHandlers [InlineData(true, false)] [InlineData(false, true)] [InlineData(false, false)] - public async Task Factory_InvocationFailure_DisposesClientAndAllowsNextInvocationAsync(bool failInitialization, bool failOperation) + public async Task InvocationFailure_DisposesClientAndAllowsNextInvocationAsync(bool failInitialization, bool failOperation) { // Arrange ProtocolStub stub = new() { FailInitialization = failInitialization, FailOperation = failOperation }; - using HttpMessageInvoker poolOwner = new(stub.CreateMessageHandler()); - HttpMessageHandler pooledHandler = Assert.Single(stub.Handlers).Object; List clients = []; Mock factory = new(); factory.Setup(f => f.CreateClient("workflow-mcp")).Returns(() => { - HttpClient client = new(pooledHandler, disposeHandler: false); + HttpClient client = new(stub.CreateMessageHandler()); clients.Add(client); return client; }); @@ -360,7 +222,7 @@ public async Task Factory_InvocationFailure_DisposesClientAndAllowsNextInvocatio }; } - await using DefaultMcpToolHandler handler = new(factory.Object, "workflow-mcp"); + DefaultMcpToolHandler handler = new(factory.Object, "workflow-mcp"); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); using CancellationTokenSource cancellation = CancellationTokenSource.CreateLinkedTokenSource(timeout.Token); @@ -379,6 +241,7 @@ public async Task Factory_InvocationFailure_DisposesClientAndAllowsNextInvocatio HttpClient firstClient = Assert.Single(clients); await Assert.ThrowsAsync(() => firstClient.GetAsync(new Uri("https://mcp.example/api"), timeout.Token)); + Assert.Single(stub.Handlers).Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); stub.BeforeOperationAsync = null; stub.FailInitialization = false; stub.FailOperation = false; @@ -388,100 +251,45 @@ public async Task Factory_InvocationFailure_DisposesClientAndAllowsNextInvocatio Assert.NotNull(result.Outputs); Assert.Equal("ok", Assert.IsType(Assert.Single(result.Outputs)).Text); Assert.Equal(2, clients.Count); + Assert.Equal(failInitialization ? 1 : 2, stub.Initializations); + Assert.Equal(failInitialization ? 1 : 2, stub.Terminations); await Assert.ThrowsAsync(() => clients[1].GetAsync(new Uri("https://mcp.example/api"), timeout.Token)); - Assert.Single(stub.Handlers).Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); + stub.Handlers[1].Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); } - [Theory] - [InlineData(false, false)] - [InlineData(false, true)] - [InlineData(true, false)] - [InlineData(true, true)] - public async Task DisposeAsync_WaitsForOwnedInvocationCleanupAsync(bool useFactory, bool pauseInitialization) + [Fact] + public async Task FactoryFailure_DoesNotPreventNextInvocationAsync() { // Arrange ProtocolStub stub = new(); - using SemaphoreSlim started = new(0); - using SemaphoreSlim finish = new(0); - async Task PauseAsync(CancellationToken token) - { - started.Release(); - await finish.WaitAsync(token); - } - - if (pauseInitialization) - { - stub.BeforeInitializationAsync = PauseAsync; - } - else - { - stub.BeforeOperationAsync = PauseAsync; - } - Mock factory = new(); - HttpClient? factoryClient = null; - factory.Setup(f => f.CreateClient("workflow-mcp")).Returns(() => factoryClient = new(stub.CreateMessageHandler())); - DefaultMcpToolHandler handler = useFactory - ? new(factory.Object, "workflow-mcp") - : new(null, stub.CreateMessageHandler); + factory.SetupSequence(f => f.CreateClient("workflow-mcp")) + .Throws(new InvalidOperationException("Factory unavailable.")) + .Returns(() => new HttpClient(stub.CreateMessageHandler())); + DefaultMcpToolHandler handler = new(factory.Object, "workflow-mcp"); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); // Act - Task invocation = InvokeAsync(handler, "ping", timeout.Token); - await started.WaitAsync(timeout.Token); - Task disposal = handler.DisposeAsync().AsTask(); - Assert.False(disposal.IsCompleted); - await Assert.ThrowsAsync(() => InvokeAsync(handler, "ping", timeout.Token)); - Mock transport = Assert.Single(stub.Handlers); - transport.Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); - finish.Release(); - await invocation; - await disposal; + InvalidOperationException error = await Assert.ThrowsAsync( + () => InvokeAsync(handler, "ping", timeout.Token)); + McpServerToolResultContent result = await InvokeAsync(handler, "ping", timeout.Token); // Assert + Assert.Equal("Factory unavailable.", error.Message); + Assert.NotNull(result.Outputs); + Assert.Equal("ok", Assert.IsType(Assert.Single(result.Outputs)).Text); Assert.Equal(1, stub.Initializations); Assert.Equal(1, stub.Terminations); - transport.Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); - if (useFactory) - { - Assert.NotNull(factoryClient); - await Assert.ThrowsAsync(() => factoryClient.GetAsync(new Uri("https://mcp.example/api"), timeout.Token)); - } - } - - [Fact] - public async Task Provider_Failure_DoesNotPreventHandlerDisposalAsync() - { - // Arrange - int providerCalls = 0; - DefaultMcpToolHandler handler = new((_, _) => - { - providerCalls++; - throw new InvalidOperationException("Provider unavailable."); - }); - using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); - - // Act - InvalidOperationException exception = await Assert.ThrowsAsync( - () => InvokeAsync(handler, "ping", timeout.Token)); - await handler.DisposeAsync(); - await Assert.ThrowsAsync(() => InvokeAsync(handler, "ping", timeout.Token)); - - // Assert - Assert.Equal("Provider unavailable.", exception.Message); - Assert.Equal(1, providerCalls); } [Theory] - [InlineData(false, false)] - [InlineData(true, false)] - [InlineData(false, true)] - public async Task ConcurrentInvocations_DoNotCoalesceOrSerializeSessionsAsync(bool useProvider, bool useFactory) + [InlineData(false)] + [InlineData(true)] + public async Task ConcurrentInvocations_DoNotCoalesceOrSerializeSessionsAsync(bool reuseHandlers) { // Arrange ProtocolStub stub = new(); - using HttpClient? client = useProvider ? new(stub.CreateMessageHandler()) : null; - using ServiceProvider? serviceProvider = useFactory + using ServiceProvider? serviceProvider = reuseHandlers ? new ServiceCollection().AddHttpClient("workflow-mcp") .ConfigurePrimaryHttpMessageHandler(stub.CreateMessageHandler).Services.BuildServiceProvider() : null; @@ -492,15 +300,9 @@ public async Task ConcurrentInvocations_DoNotCoalesceOrSerializeSessionsAsync(bo started.Release(); await finish.WaitAsync(token); }; - int providerCalls = 0; - await using DefaultMcpToolHandler handler = serviceProvider is not null + DefaultMcpToolHandler handler = serviceProvider is not null ? new(serviceProvider.GetRequiredService(), "workflow-mcp") - : new(useProvider ? (_, _) => - { - Interlocked.Increment(ref providerCalls); - return Task.FromResult(client); - } - : null, stub.CreateMessageHandler); + : CreateHandler(stub); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); // Act @@ -508,11 +310,20 @@ public async Task ConcurrentInvocations_DoNotCoalesceOrSerializeSessionsAsync(bo Task second = InvokeAsync(handler, "session", timeout.Token); await started.WaitAsync(timeout.Token); await started.WaitAsync(timeout.Token); - finish.Release(2); + finish.Release(); + Task completed = await Task.WhenAny(first, second); + await completed; + Assert.False((completed == first ? second : first).IsCompleted); + Assert.Equal(1, stub.Terminations); + if (reuseHandlers) + { + Assert.Single(stub.Handlers).Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); + } + + finish.Release(); McpServerToolResultContent[] results = await Task.WhenAll(first, second); // Assert - Assert.Equal(useProvider ? 2 : 0, providerCalls); Assert.Equal(2, stub.Initializations); Assert.Equal(2, stub.Terminations); IList? firstOutputs = results[0].Outputs; @@ -522,84 +333,16 @@ public async Task ConcurrentInvocations_DoNotCoalesceOrSerializeSessionsAsync(bo Assert.NotEqual( Assert.IsType(Assert.Single(firstOutputs)).Text, Assert.IsType(Assert.Single(secondOutputs)).Text); - Assert.Equal(useProvider || useFactory ? 1 : 2, stub.Handlers.Count); - } - - [Fact] - public async Task Provider_OperationFailure_DisposesSessionAndPreservesCallerClientAsync() - { - // Arrange - ProtocolStub stub = new() { FailOperation = true }; - using HttpClient client = new(stub.CreateMessageHandler()); - await using DefaultMcpToolHandler handler = new((_, _) => Task.FromResult(client)); - using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); - - // Act - await Assert.ThrowsAsync(() => InvokeAsync(handler, "ping", timeout.Token)); - - // Assert - Assert.Equal(1, stub.Initializations); - Assert.Equal(1, stub.Terminations); - stub.Handlers[0].Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); - } - - [Theory] - [InlineData(false, false)] - [InlineData(true, false)] - [InlineData(true, true)] - public async Task InitializationFailure_RespectsTransportOwnershipAsync(bool useProvider, bool provideClient) - { - // Arrange - ProtocolStub stub = new() { FailInitialization = true }; - using HttpClient? client = provideClient ? new(stub.CreateMessageHandler()) : null; - DefaultMcpToolHandler handler = new( - useProvider ? (_, _) => Task.FromResult(client) : null, stub.CreateMessageHandler); - using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); - - // Act - await Assert.ThrowsAsync(() => InvokeAsync(handler, "ping", timeout.Token)); - - // Assert - Mock transport = Assert.Single(stub.Handlers); - transport.Protected().Verify("Dispose", provideClient ? Times.Never() : Times.Once(), ItExpr.Is(disposing => disposing)); - await handler.DisposeAsync(); - transport.Protected().Verify("Dispose", provideClient ? Times.Never() : Times.Once(), ItExpr.Is(disposing => disposing)); - } - - [Fact] - public async Task Provider_Cancellation_DisposesSessionAsync() - { - // Arrange - ProtocolStub stub = new(); - using HttpClient client = new(stub.CreateMessageHandler()); - using SemaphoreSlim started = new(0); - stub.BeforeOperationAsync = async token => - { - started.Release(); - await Task.Delay(Timeout.Infinite, token); - }; - await using DefaultMcpToolHandler handler = new((_, _) => Task.FromResult(client)); - using CancellationTokenSource cancellation = new(TimeSpan.FromSeconds(10)); - - // Act - Task invocation = InvokeAsync(handler, "ping", cancellation.Token); - await started.WaitAsync(cancellation.Token); - cancellation.Cancel(); - await Assert.ThrowsAnyAsync(() => invocation); - - // Assert - Assert.Equal(1, stub.Terminations); - stub.Handlers[0].Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); + Assert.Equal(reuseHandlers ? 1 : 2, stub.Handlers.Count); } [Theory] [InlineData(false)] [InlineData(true)] - public async Task DisposeAsync_WaitsForProviderInvocationCleanupAsync(bool pauseProvider) + public async Task ActiveInvocation_RetainsResourcesUntilCompletionAsync(bool pauseInitialization) { // Arrange ProtocolStub stub = new(); - using HttpClient client = new(stub.CreateMessageHandler()); using SemaphoreSlim started = new(0); using SemaphoreSlim finish = new(0); async Task PauseAsync(CancellationToken token) @@ -608,167 +351,53 @@ async Task PauseAsync(CancellationToken token) await finish.WaitAsync(token); } - if (!pauseProvider) + if (pauseInitialization) { - stub.BeforeOperationAsync = PauseAsync; + stub.BeforeInitializationAsync = PauseAsync; } - - int providerCalls = 0; - DefaultMcpToolHandler handler = new(async (_, token) => + else { - providerCalls++; - if (pauseProvider) - { - await PauseAsync(token); - } + stub.BeforeOperationAsync = PauseAsync; + } - return client; - }); + DefaultMcpToolHandler handler = CreateHandler(stub); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); // Act Task invocation = InvokeAsync(handler, "ping", timeout.Token); await started.WaitAsync(timeout.Token); - Task disposal = handler.DisposeAsync().AsTask(); - Assert.False(disposal.IsCompleted); - await Assert.ThrowsAsync(() => InvokeAsync(handler, "ping", timeout.Token)); + Mock transport = Assert.Single(stub.Handlers); + transport.Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); finish.Release(); await invocation; - await disposal; // Assert - Assert.Equal(1, providerCalls); + Assert.Equal(1, stub.Initializations); Assert.Equal(1, stub.Terminations); - stub.Handlers[0].Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); - } - - [Fact] - public async Task Provider_InvalidDiscoveryArguments_DoesNotCallProviderAsync() - { - // Arrange - int providerCalls = 0; - await using DefaultMcpToolHandler handler = new((_, _) => - { - providerCalls++; - return Task.FromResult(null); - }); - - // Act - await Assert.ThrowsAsync(() => handler.InvokeToolAsync( - "https://mcp.example/api", null, DefaultMcpToolHandler.ListToolsToolName, - new Dictionary { ["unused"] = true }, null, null)); - - // Assert - Assert.Equal(0, providerCalls); + transport.Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); } [Theory] - [InlineData(false)] - [InlineData(true)] - public async Task Provider_ReentrantDisposal_ThrowsWithoutStartingShutdownAsync(bool fromChildTask) - { - // Arrange - ProtocolStub stub = new(); - using HttpClient client = new(stub.CreateMessageHandler()); - DefaultMcpToolHandler? handler = null; - int rejectedDisposals = 0; - handler = new(async (_, _) => - { - async Task DisposeAsync() => await handler!.DisposeAsync(); - InvalidOperationException error = await Assert.ThrowsAsync( - () => fromChildTask ? Task.Run(DisposeAsync) : DisposeAsync()); - Assert.Contains("active MCP invocation", error.Message); - rejectedDisposals++; - return client; - }); - using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); - - // Act - await InvokeAsync(handler, "ping", timeout.Token); - await InvokeAsync(handler, "ping", timeout.Token); - await handler.DisposeAsync(); - - // Assert - Assert.Equal(2, rejectedDisposals); - Assert.Equal(2, stub.Terminations); - } - - [Fact] - public async Task Provider_CompletedInheritedContext_CanDisposeHandlerAsync() - { - // Arrange - ProtocolStub stub = new(); - using HttpClient client = new(stub.CreateMessageHandler()); - using SemaphoreSlim finished = new(0); - using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); - Task? inheritedDisposal = null; - DefaultMcpToolHandler? handler = null; - handler = new((_, token) => - { - inheritedDisposal = Task.Run(async () => - { - await finished.WaitAsync(token); - await handler!.DisposeAsync(); - }, token); - return Task.FromResult(client); - }); - - // Act - await InvokeAsync(handler, "ping", timeout.Token); - finished.Release(); - await Assert.IsAssignableFrom(inheritedDisposal); - - // Assert - Assert.Equal(1, stub.Terminations); - await Assert.ThrowsAsync(() => InvokeAsync(handler, "ping", timeout.Token)); - } - - [Fact] - public async Task Provider_NestedInvocation_RestoresOuterDisposalGuardAsync() + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(false, false)] + public async Task CleanupFailure_DoesNotReplaceOutcomeAsync(bool failInitialization, bool failOperation) { // Arrange - ProtocolStub stub = new(); - using HttpClient client = new(stub.CreateMessageHandler()); - bool nested = false; - DefaultMcpToolHandler? handler = null; - handler = new(async (_, token) => + ProtocolStub stub = new() { - if (!nested) - { - nested = true; - await InvokeAsync(handler!, "ping", token); - await Assert.ThrowsAsync(() => handler!.DisposeAsync().AsTask()); - } - - return client; - }); - using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); - - // Act - await InvokeAsync(handler, "ping", timeout.Token); - await handler.DisposeAsync(); - - // Assert - Assert.True(nested); - Assert.Equal(2, stub.Terminations); - } - - [Theory] - [InlineData(false)] - [InlineData(true)] - public async Task Invocation_HttpHandlerCleanupFailure_DoesNotReplaceOutcomeAsync(bool failOperation) - { - // Arrange - ProtocolStub stub = new() { FailOperation = failOperation, FailTransportDisposal = true }; - DefaultMcpToolHandler handler = new(null, stub.CreateMessageHandler); + FailInitialization = failInitialization, + FailOperation = failOperation, + FailTransportDisposal = true + }; + DefaultMcpToolHandler handler = CreateHandler(stub); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); - using CleanupTraceListener listener = new(); Trace.Listeners.Add(listener); try { // Act - if (failOperation) + if (failInitialization || failOperation) { await Assert.ThrowsAsync(() => InvokeAsync(handler, "ping", timeout.Token)); } @@ -779,10 +408,8 @@ public async Task Invocation_HttpHandlerCleanupFailure_DoesNotReplaceOutcomeAsyn Assert.Equal("ok", Assert.IsType(Assert.Single(result.Outputs)).Text); } - await handler.DisposeAsync(); - // Assert - Assert.Equal(1, stub.Terminations); + Assert.Equal(failInitialization ? 0 : 1, stub.Terminations); Assert.Contains("Failed to dispose MCP ", listener.Output); Assert.Contains("transport cleanup failed", listener.Output); Assert.Single(stub.Handlers).Protected().Verify( @@ -794,74 +421,6 @@ public async Task Invocation_HttpHandlerCleanupFailure_DoesNotReplaceOutcomeAsyn } } - [Fact] - public async Task Provider_CompletedNestedContext_StillGuardsActiveOuterInvocationAsync() - { - // Arrange - ProtocolStub stub = new(); - using HttpClient client = new(stub.CreateMessageHandler()); - using SemaphoreSlim nestedFinished = new(0); - Task? inheritedDisposal = null; - int providerCalls = 0; - DefaultMcpToolHandler? handler = null; - handler = new(async (_, token) => - { - if (++providerCalls == 1) - { - await InvokeAsync(handler!, "ping", token); - nestedFinished.Release(); - await Assert.IsAssignableFrom(inheritedDisposal); - } - else - { - inheritedDisposal = Task.Run(async () => - { - await nestedFinished.WaitAsync(token); - await Assert.ThrowsAsync(() => handler!.DisposeAsync().AsTask()); - }, token); - } - - return client; - }); - using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); - - // Act - await InvokeAsync(handler, "ping", timeout.Token); - await handler.DisposeAsync(); - - // Assert - Assert.Equal(2, providerCalls); - Assert.Equal(2, stub.Terminations); - } - - [Fact] - public async Task InitializationFailure_ReportsHttpHandlerCleanupFailureAsync() - { - // Arrange - ProtocolStub stub = new() { FailInitialization = true, FailTransportDisposal = true }; - DefaultMcpToolHandler handler = new(null, stub.CreateMessageHandler); - using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); - - using CleanupTraceListener listener = new(); - Trace.Listeners.Add(listener); - try - { - // Act - await Assert.ThrowsAsync(() => InvokeAsync(handler, "ping", timeout.Token)); - await handler.DisposeAsync(); - - // Assert - Assert.Contains("Failed to dispose MCP transport", listener.Output); - Assert.Contains("transport cleanup failed", listener.Output); - Assert.Single(stub.Handlers).Protected().Verify( - "Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); - } - finally - { - Trace.Listeners.Remove(listener); - } - } - [Theory] [InlineData(false)] [InlineData(true)] @@ -904,6 +463,13 @@ public async Task ClientConnection_SessionCleanupFailure_StillCleansUpTransportA } } + private static DefaultMcpToolHandler CreateHandler(ProtocolStub stub) + { + Mock factory = new(); + factory.Setup(f => f.CreateClient("workflow-mcp")).Returns(() => new HttpClient(stub.CreateMessageHandler())); + return new(factory.Object, "workflow-mcp"); + } + private static Task InvokeAsync( DefaultMcpToolHandler handler, string toolName, CancellationToken cancellationToken) => handler.InvokeToolAsync("https://mcp.example/api", null, toolName, null, null, null, cancellationToken); @@ -952,10 +518,11 @@ private async Task SendAsync(HttpRequestMessage request, Ca return EmptyResponse(HttpStatusCode.OK, request); } + Assert.NotNull(request.Content); #if NET - using JsonDocument document = JsonDocument.Parse(await request.Content!.ReadAsStringAsync(cancellationToken)); + using JsonDocument document = JsonDocument.Parse(await request.Content.ReadAsStringAsync(cancellationToken)); #else - using JsonDocument document = JsonDocument.Parse(await request.Content!.ReadAsStringAsync()); + using JsonDocument document = JsonDocument.Parse(await request.Content.ReadAsStringAsync()); #endif JsonElement message = document.RootElement; if (!message.TryGetProperty("id", out JsonElement id)) diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs index 68022e544ba..c28cb1e7b05 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs @@ -9,6 +9,7 @@ using System.Threading.Tasks; using Microsoft.Extensions.AI; using ModelContextProtocol.Protocol; +using Moq; namespace Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests; @@ -17,79 +18,29 @@ namespace Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests; /// public sealed class DefaultMcpToolHandlerTests { - #region Constructor Tests - [Fact] - public async Task Constructor_WithNoParameters_ShouldCreateInstanceAsync() + public void Constructor_WithNullHttpClientFactory_ShouldThrow() { // Act - DefaultMcpToolHandler handler = new(); + ArgumentNullException exception = Assert.Throws(() => new DefaultMcpToolHandler(null!, "workflow-mcp")); // Assert - Assert.NotNull(handler); - await handler.DisposeAsync(); + Assert.Equal("httpClientFactory", exception.ParamName); } [Fact] - public async Task Constructor_WithNullHttpClientProvider_ShouldCreateInstanceAsync() - { - // Act - DefaultMcpToolHandler handler = new(null); - - // Assert - Assert.NotNull(handler); - await handler.DisposeAsync(); - } - - [Fact] - public async Task Constructor_WithHttpClientProvider_ShouldCreateInstanceAsync() + public void Constructor_WithNullHttpClientName_ShouldThrow() { // Arrange - static Task ProviderAsync(string url, CancellationToken ct) => Task.FromResult(new HttpClient()); + Mock factory = new(); // Act - DefaultMcpToolHandler handler = new(httpClientProvider: ProviderAsync); + ArgumentNullException exception = Assert.Throws(() => new DefaultMcpToolHandler(factory.Object, null!)); // Assert - Assert.NotNull(handler); - await handler.DisposeAsync(); + Assert.Equal("httpClientName", exception.ParamName); } - #endregion - - #region DisposeAsync Tests - - [Fact] - public async Task DisposeAsync_WhenCalled_ShouldCompleteWithoutErrorAsync() - { - // Arrange - DefaultMcpToolHandler handler = new(); - - // Act - async Task actAsync() => await handler.DisposeAsync(); - - // Assert - Assert.Null(await Record.ExceptionAsync(actAsync)); - } - - [Fact] - public async Task DisposeAsync_WhenCalledMultipleTimes_ShouldHandleGracefullyAsync() - { - // Arrange - DefaultMcpToolHandler handler = new(); - - // Act - await handler.DisposeAsync(); - async Task actAsync() => await handler.DisposeAsync(); - - // Assert - await Assert.ThrowsAsync(actAsync); - } - - #endregion - - #region HttpClientProvider Tests - [Theory] [InlineData("\r", false, false)] [InlineData("\n", false, false)] @@ -106,271 +57,42 @@ public async Task DisposeAsync_WhenCalledMultipleTimes_ShouldHandleGracefullyAsy public async Task InvokeToolAsync_WithInvalidHeader_ShouldRejectBeforeClientCreationAsync( string invalidCharacter, bool invalidName, - bool useProvider) + bool useFactory) { // Arrange - bool clientCreationAttempted = false; - Task ProviderAsync(string url, CancellationToken ct) - { - clientCreationAttempted = true; - throw new InvalidOperationException("Client creation should not be reached."); - } - - HttpMessageHandler CreateHandler() - { - clientCreationAttempted = true; - throw new InvalidOperationException("Client creation should not be reached."); - } - - await using DefaultMcpToolHandler handler = new(useProvider ? ProviderAsync : null, CreateHandler); + Mock factory = new(MockBehavior.Strict); + DefaultMcpToolHandler handler = useFactory ? new(factory.Object, "workflow-mcp") : new(); Dictionary headers = new() { [invalidName ? $"X-Test{invalidCharacter}" : "X-Test"] = invalidName ? "value" : $"value{invalidCharacter}" }; // Act - async Task actAsync() => await handler.InvokeToolAsync( - serverUrl: "https://example.com/mcp", - serverLabel: null, - toolName: "testTool", - arguments: null, - headers: headers, - connectionName: null); + ArgumentException exception = await Assert.ThrowsAsync(() => handler.InvokeToolAsync( + "https://example.com/mcp", null, "ping", null, headers, null)); // Assert - ArgumentException exception = await Assert.ThrowsAsync(actAsync); Assert.Equal("headers", exception.ParamName); - Assert.False(clientCreationAttempted); - } - - [Fact] - public async Task InvokeToolAsync_WithHttpClientProvider_ShouldCallProviderAsync() - { - // Arrange - bool providerCalled = false; - string? capturedServerUrl = null; - - Task ProviderAsync(string url, CancellationToken ct) - { - providerCalled = true; - capturedServerUrl = url; - return Task.FromResult(null); - } - - DefaultMcpToolHandler handler = new(httpClientProvider: ProviderAsync); - - // Act & Assert - The call will fail because there's no real MCP server, but the provider should be called - try - { - await handler.InvokeToolAsync( - serverUrl: "http://localhost:12345/mcp", - serverLabel: "test", - toolName: "testTool", - arguments: null, - headers: null, - connectionName: null); - } - catch - { - // Expected to fail - no real server - } - finally - { - await handler.DisposeAsync(); - } - - // Assert - Assert.True(providerCalled); - Assert.Equal("http://localhost:12345/mcp", capturedServerUrl); + factory.Verify(f => f.CreateClient(It.IsAny()), Times.Never); } [Fact] - public async Task InvokeToolAsync_WithHttpClientProviderReturningClient_ShouldUseProvidedClientAsync() + public async Task InvokeToolAsync_WithCancelledToken_ShouldRejectBeforeClientCreationAsync() { // Arrange - bool providerCalled = false; - HttpClient? providedClient = null; - - Task ProviderAsync(string url, CancellationToken ct) - { - providerCalled = true; - providedClient = new HttpClient(); - return Task.FromResult(providedClient); - } - - DefaultMcpToolHandler handler = new(httpClientProvider: ProviderAsync); + Mock factory = new(MockBehavior.Strict); + DefaultMcpToolHandler handler = new(factory.Object, "workflow-mcp"); + using CancellationTokenSource cancellation = new(); + cancellation.Cancel(); - // Act & Assert - The call will fail because there's no real MCP server, but the provider should be called - try - { - await handler.InvokeToolAsync( - serverUrl: "http://localhost:12345/mcp", - serverLabel: "test", - toolName: "testTool", - arguments: null, - headers: null, - connectionName: null); - } - catch - { - // Expected to fail - no real server - } - finally - { - await handler.DisposeAsync(); - providedClient?.Dispose(); - } + // Act + await Assert.ThrowsAnyAsync(() => handler.InvokeToolAsync( + "https://example.com/mcp", null, "ping", null, null, null, cancellation.Token)); // Assert - Assert.True(providerCalled); - } - - #endregion - - #region Invocation Tests - - [Fact] - public async Task InvokeToolAsync_SameServerUrl_ShouldCallProviderOncePerAttemptWhenConnectionFailsAsync() - { - // Arrange - int providerCallCount = 0; - - Task ProviderAsync(string url, CancellationToken ct) - { - providerCallCount++; - return Task.FromResult(null); - } - - DefaultMcpToolHandler handler = new(httpClientProvider: ProviderAsync); - const string ServerUrl = "http://localhost:12345/mcp"; - - try - { - // Act - Call twice with the same server URL - for (int i = 0; i < 2; i++) - { - try - { - await handler.InvokeToolAsync( - serverUrl: ServerUrl, - serverLabel: "test", - toolName: "testTool", - arguments: null, - headers: null, - connectionName: null); - } - catch - { - // Expected to fail - no real server - } - } - - // Assert - Assert.Equal(2, providerCallCount); - } - finally - { - await handler.DisposeAsync(); - } + factory.Verify(f => f.CreateClient(It.IsAny()), Times.Never); } - [Fact] - public async Task InvokeToolAsync_DifferentServerUrls_ShouldCreateSeparateClientsAsync() - { - // Arrange - int providerCallCount = 0; - - Task ProviderAsync(string url, CancellationToken ct) - { - providerCallCount++; - return Task.FromResult(null); - } - - DefaultMcpToolHandler handler = new(httpClientProvider: ProviderAsync); - - try - { - // Act - Call with different server URLs - foreach (string serverUrl in new[] { "http://localhost:12345/mcp1", "http://localhost:12345/mcp2" }) - { - try - { - await handler.InvokeToolAsync( - serverUrl: serverUrl, - serverLabel: "test", - toolName: "testTool", - arguments: null, - headers: null, - connectionName: null); - } - catch - { - // Expected to fail - no real server - } - } - - // Assert - Provider should be called once per unique server URL - Assert.Equal(2, providerCallCount); - } - finally - { - await handler.DisposeAsync(); - } - } - - [Fact] - public async Task InvokeToolAsync_SameUrlDifferentHeaders_ShouldCreateSeparateClientsAsync() - { - // Arrange - int providerCallCount = 0; - - Task ProviderAsync(string url, CancellationToken ct) - { - providerCallCount++; - return Task.FromResult(null); - } - - DefaultMcpToolHandler handler = new(httpClientProvider: ProviderAsync); - const string ServerUrl = "http://localhost:12345/mcp"; - - try - { - // Act - Call with same URL but different headers - Dictionary[] headerSets = - [ - new() { ["Authorization"] = "Bearer token1" }, - new() { ["Authorization"] = "Bearer token2" } - ]; - - foreach (Dictionary headers in headerSets) - { - try - { - await handler.InvokeToolAsync( - serverUrl: ServerUrl, - serverLabel: "test", - toolName: "testTool", - arguments: null, - headers: headers, - connectionName: null); - } - catch - { - // Expected to fail - no real server - } - } - - // Assert - Assert.Equal(2, providerCallCount); - } - finally - { - await handler.DisposeAsync(); - } - } - - #endregion - #region Reserved Tools/List Tests [Fact] @@ -397,27 +119,17 @@ public void IsListToolsToolName_WithRegularToolName_ShouldReturnFalse() public async Task InvokeToolAsync_WithListToolsArguments_ShouldThrowArgumentExceptionAsync() { // Arrange - DefaultMcpToolHandler handler = new(); + Mock factory = new(MockBehavior.Strict); + DefaultMcpToolHandler handler = new(factory.Object, "workflow-mcp"); - try - { - // Act - async Task actAsync() => await handler.InvokeToolAsync( - serverUrl: "http://localhost:12345/mcp", - serverLabel: "test", - toolName: DefaultMcpToolHandler.ListToolsToolName, - arguments: new Dictionary { ["ignored"] = true }, - headers: null, - connectionName: null); - - // Assert - ArgumentException exception = await Assert.ThrowsAsync(actAsync); - Assert.Contains("does not accept tool arguments", exception.Message); - } - finally - { - await handler.DisposeAsync(); - } + // Act + ArgumentException exception = await Assert.ThrowsAsync(() => handler.InvokeToolAsync( + "https://example.com/mcp", "test", DefaultMcpToolHandler.ListToolsToolName, + new Dictionary { ["ignored"] = true }, null, null)); + + // Assert + Assert.Contains("does not accept tool arguments", exception.Message); + factory.Verify(f => f.CreateClient(It.IsAny()), Times.Never); } [Fact] @@ -457,32 +169,6 @@ public async Task CreateListToolsResultContent_WithTools_ShouldSerializeToolMeta #endregion - #region Interface Implementation Tests - - [Fact] - public async Task DefaultMcpToolHandler_ShouldImplementIMcpToolHandlerAsync() - { - // Arrange & Act - DefaultMcpToolHandler handler = new(); - - // Assert - Assert.IsAssignableFrom(handler); - await handler.DisposeAsync(); - } - - [Fact] - public async Task DefaultMcpToolHandler_ShouldImplementIAsyncDisposableAsync() - { - // Arrange & Act - DefaultMcpToolHandler handler = new(); - - // Assert - Assert.IsAssignableFrom(handler); - await handler.DisposeAsync(); - } - - #endregion - #region ConvertContentBlock Tests [Fact] diff --git a/skills/agent-framework-builder-dotnet/references/declarative.md b/skills/agent-framework-builder-dotnet/references/declarative.md index 6653becf47b..2a34b24c496 100644 --- a/skills/agent-framework-builder-dotnet/references/declarative.md +++ b/skills/agent-framework-builder-dotnet/references/declarative.md @@ -27,13 +27,13 @@ commitments. or a custom handler with a documented session-lifetime contract when continuity is required. - Prefer `DefaultMcpToolHandler(IHttpClientFactory, string)` with an application-provided factory and stable named configuration for HTTP handler reuse and rotation. Factory-created clients are disposed per invocation; the - factory remains application-owned. The non-DI fallback owns a dedicated HTTP handler per invocation, while - callback-supplied `HttpClient` instances remain caller-owned. + factory remains application-owned. The parameterless non-DI fallback owns a dedicated HTTP handler per invocation. + Callback-based constructors and handler-level disposal are no longer supported. - Configure factory clients with cookies and redirects disabled, certificate revocation checking enabled, and origin-bound credential injection. Do not retain per-workflow credentials or mutable origin state in pooled handlers, and do not derive client names from workflow IDs, credentials, or arbitrary URLs. -- Bound concurrent MCP work and supply invocation deadlines/cancellation from the host. Handler disposal stops - admission and drains existing calls without cancelling them; cancel and await calls before disposing the handler. +- Bound concurrent MCP work and supply invocation deadlines/cancellation from the host. The handler owns no shared + disposable resources; cancel and await calls before disposing the application's HTTP factory infrastructure. - When using `DefaultHttpRequestHandler`, its client provider is also called for redirect destinations. Return `null` to continue with the internal client or throw to reject a destination; returning a client on a redirect is rejected. - Keep configuration exposure restricted to intentional values. From fc0979ed13056ea7284cf2ae7a8ca2c672029982 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 27 Oct 2026 11:05:21 -0400 Subject: [PATCH 07/13] perf: disable standalone MCP streams for workflow tool calls Keep request-scoped SSE responses and invocation-owned session cleanup while avoiding unsolicited GET streams. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 27dcc08f-0b3a-40a6-a3ad-2c45eafce159 --- ...0047-dotnet-workflow-owned-mcp-sessions.md | 6 ++++ .../DefaultMcpToolHandler.cs | 4 ++- .../README.md | 4 +++ .../DefaultMcpToolHandlerLifetimeTests.cs | 31 +++++++++++++++++-- .../references/declarative.md | 2 ++ 5 files changed, 44 insertions(+), 3 deletions(-) diff --git a/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md b/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md index a1d26bbf6e7..8491396c28d 100644 --- a/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md +++ b/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md @@ -233,6 +233,12 @@ Non-DI fallback clients retain explicit defaults: cookies disabled, automatic redirects disabled, certificate revocation checking, Streamable HTTP, and origin-bound credential handling. Factory defaults alone are not a substitute. +Disable the SDK's standalone GET/SSE stream for unsolicited server messages. +Unattended tool invocations do not require a background subscription channel. +This avoids opening and closing that channel for each legacy initialization; +request-scoped SSE responses to tool calls remain supported. This policy applies +to both factory-backed and non-DI calls and does not change session ownership. + The integration does not silently replace application factory clients' pipelines. Configure their primary handlers with cookies and automatic redirects disabled and certificate revocation checking enabled. Any credential-injecting diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs index 6fcf457f315..a0982758e77 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs @@ -25,6 +25,7 @@ namespace Microsoft.Agents.AI.Workflows.Declarative.Mcp; /// Every invocation creates and disposes a separate MCP client, including tools/list. /// Factory-created HTTP clients reuse factory-managed handlers without sharing MCP session state. /// Without a factory, each invocation owns a dedicated HTTP handler. +/// Standalone GET streams for unsolicited server messages are disabled; request-scoped SSE responses remain supported. /// The handler owns no shared disposable resources. The host must await its invocations before disposing /// the application-owned factory infrastructure. /// Non-cancellation session and transport cleanup failures are reported through warnings without replacing @@ -183,7 +184,8 @@ private async Task CreateClientAsync( // legacy HTTP+SSE transport, which trusts a server-advertised message endpoint. That // server-controlled endpoint is the primary vector for redirecting the Authorization token // to a different origin; Streamable HTTP keeps every request on the configured origin. - TransportMode = HttpTransportMode.StreamableHttp + TransportMode = HttpTransportMode.StreamableHttp, + EnableStandaloneGetStream = false }; HttpClient httpClient; diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md index 61a407c1df1..fe8dee3a50b 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md @@ -62,6 +62,10 @@ resumes the registered invocation only after a matching approval response. including `tools/list`, and disposes it before completing. The handler retains no per-invocation state or shared disposable resources. +Standalone GET/SSE streams for unsolicited server messages are disabled because +this handler performs request-scoped tool operations, not background subscriptions. +SSE responses to individual tool requests remain supported. + Each handler-created HTTP client is disposed with its invocation. These clients can reuse factory-managed HTTP handlers through the `DefaultMcpToolHandler(IHttpClientFactory, string)` constructor. The second diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs index 610959f8ba7..f29ae6e481c 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs @@ -23,6 +23,27 @@ namespace Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests; /// public sealed class DefaultMcpToolHandlerLifetimeTests { + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task RequestOnlyInvocation_DoesNotOpenStandaloneStreamAsync(bool useEventStream) + { + // Arrange + ProtocolStub stub = new() { UseEventStreamResult = useEventStream }; + DefaultMcpToolHandler handler = CreateHandler(stub); + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + + // Act + McpServerToolResultContent result = await InvokeAsync(handler, "ping", timeout.Token); + + // Assert + Assert.NotNull(result.Outputs); + Assert.Equal("ok", Assert.IsType(Assert.Single(result.Outputs)).Text); + Assert.Equal(0, stub.StandaloneGetRequests); + Assert.Equal(1, stub.Initializations); + Assert.Equal(1, stub.Terminations); + } + [Fact] public async Task SequentialInvocations_UseSeparateSessionsAsync() { @@ -478,9 +499,11 @@ private sealed class ProtocolStub { private int _initializations; private int _terminations; + private int _standaloneGetRequests; public int Initializations => this._initializations; public int Terminations => this._terminations; + public int StandaloneGetRequests => this._standaloneGetRequests; public List> Handlers { get; } = []; public Func? BeforeInitializationAsync { get; set; } public Func? BeforeOperationAsync { get; set; } @@ -488,6 +511,7 @@ private sealed class ProtocolStub public bool FailInitialization { get; set; } public bool FailOperation { get; set; } public bool FailTransportDisposal { get; set; } + public bool UseEventStreamResult { get; set; } public HttpMessageHandler CreateMessageHandler() { @@ -509,6 +533,7 @@ private async Task SendAsync(HttpRequestMessage request, Ca { if (request.Method == HttpMethod.Get) { + Interlocked.Increment(ref this._standaloneGetRequests); return EmptyResponse(HttpStatusCode.MethodNotAllowed, request); } @@ -583,12 +608,14 @@ private async Task SendAsync(HttpRequestMessage request, Ca }; } + string responseBody = $$$"""{"jsonrpc":"2.0","id":{{{id.GetRawText()}}},"result":{{{result}}}}"""; + bool useEventStream = this.UseEventStreamResult && string.Equals(method, "tools/call", StringComparison.Ordinal); HttpResponseMessage response = new(HttpStatusCode.OK) { RequestMessage = request, Content = new StringContent( - $$$"""{"jsonrpc":"2.0","id":{{{id.GetRawText()}}},"result":{{{result}}}}""", - Encoding.UTF8, "application/json") + useEventStream ? $"event: message\ndata: {responseBody}\n\n" : responseBody, + Encoding.UTF8, useEventStream ? "text/event-stream" : "application/json") }; if (sessionId is not null) { diff --git a/skills/agent-framework-builder-dotnet/references/declarative.md b/skills/agent-framework-builder-dotnet/references/declarative.md index 2a34b24c496..1197f14bd85 100644 --- a/skills/agent-framework-builder-dotnet/references/declarative.md +++ b/skills/agent-framework-builder-dotnet/references/declarative.md @@ -25,6 +25,8 @@ commitments. - `DefaultMcpToolHandler` creates and disposes an MCP client per invocation, including discovery; it does not retain implicit server-side session state across calls, approval waits, or checkpoint resumes. Use explicit state handles or a custom handler with a documented session-lifetime contract when continuity is required. +- Standalone MCP GET/SSE streams for unsolicited messages are disabled; request-scoped SSE tool responses remain + supported. Use an explicitly scoped integration for background subscriptions. - Prefer `DefaultMcpToolHandler(IHttpClientFactory, string)` with an application-provided factory and stable named configuration for HTTP handler reuse and rotation. Factory-created clients are disposed per invocation; the factory remains application-owned. The parameterless non-DI fallback owns a dedicated HTTP handler per invocation. From b63ace963a27cb8c3527b7f9c6097769ac9bd7ef Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 27 Oct 2026 11:30:48 -0400 Subject: [PATCH 08/13] feat: allow explicit protocol selection for workflow MCP calls Preserve automatic negotiation by default and pass fresh SDK options per invocation. Cover legacy probe avoidance, strict version failures, and resource cleanup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 27dcc08f-0b3a-40a6-a3ad-2c45eafce159 --- ...0047-dotnet-workflow-owned-mcp-sessions.md | 15 +++ .../DefaultMcpToolHandler.cs | 19 +++- .../PublicAPI/net10.0/PublicAPI.Unshipped.txt | 2 + .../PublicAPI/net472/PublicAPI.Unshipped.txt | 2 + .../PublicAPI/net8.0/PublicAPI.Unshipped.txt | 2 + .../PublicAPI/net9.0/PublicAPI.Unshipped.txt | 2 + .../netstandard2.0/PublicAPI.Unshipped.txt | 2 + .../README.md | 19 ++++ .../DefaultMcpToolHandlerLifetimeTests.cs | 99 +++++++++++++++++-- .../references/declarative.md | 4 + 10 files changed, 159 insertions(+), 7 deletions(-) diff --git a/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md b/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md index 8491396c28d..6aaf1e82f1f 100644 --- a/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md +++ b/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md @@ -260,6 +260,21 @@ The package supports modern .NET, .NET Standard 2.0, and .NET Framework 4.7.2. Compatible factory-package versions, handler APIs, and DNS-refresh behavior must be established for every supported target; this proposal does not drop targets. +### Protocol setup + +Expose an init-only optional `DefaultMcpToolHandler.ProtocolVersion` property +and pass it through a fresh SDK `McpClientOptions` for every invocation. +Leave it unset by default so the SDK retains automatic discovery and legacy +fallback. Do not select an older version globally to optimize one server. + +An application that knows a server requires an initialize-handshake revision +can explicitly select that version to avoid the initial discovery probe and its +possible timeout. The server must negotiate that exact legacy version. This +does not skip initialization, share a session, or eliminate session termination. +Explicit newer revisions still perform discovery and reject legacy-only servers +rather than silently downgrading. Incompatible selections surface SDK errors. +Use separately configured handlers where server protocol policies differ. + ### Compatibility and explicitly stateful integrations Remove the `httpClientProvider` callback constructor and the internal diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs index a0982758e77..61611c1e311 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs @@ -46,6 +46,20 @@ public sealed class DefaultMcpToolHandler : IMcpToolHandler private readonly IHttpClientFactory? _httpClientFactory; private readonly string _httpClientName = string.Empty; + /// + /// Gets or initializes the protocol version requested for every invocation. + /// + /// + /// The protocol version to pass to the MCP SDK, or to use its automatic negotiation. + /// The default is . + /// + /// + /// Selecting a known legacy version skips the discovery probe but still initializes a separate session per invocation. + /// Explicit selection restricts negotiation and fails if the server cannot satisfy the SDK's version requirements. + /// Use separately configured handlers for servers requiring different protocol policies. + /// + public string? ProtocolVersion { get; init; } + /// /// Initializes a new instance of the class. /// @@ -218,7 +232,10 @@ private async Task CreateClientAsync( try { transport = new(transportOptions, httpClient, ownsHttpClient: true); - McpClient client = await McpClient.CreateAsync(transport, cancellationToken: cancellationToken).ConfigureAwait(false); + McpClient client = await McpClient.CreateAsync( + transport, + new McpClientOptions { ProtocolVersion = this.ProtocolVersion }, + cancellationToken: cancellationToken).ConfigureAwait(false); return new ClientConnection(client, transport); } catch diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt index ef812db0a70..7b2d44f24b1 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt @@ -1,5 +1,7 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler() -> void +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DisposeAsync() -> System.Threading.Tasks.ValueTask diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt index ef812db0a70..7b2d44f24b1 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt @@ -1,5 +1,7 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler() -> void +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DisposeAsync() -> System.Threading.Tasks.ValueTask diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt index ef812db0a70..7b2d44f24b1 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt @@ -1,5 +1,7 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler() -> void +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DisposeAsync() -> System.Threading.Tasks.ValueTask diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt index ef812db0a70..7b2d44f24b1 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt @@ -1,5 +1,7 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler() -> void +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DisposeAsync() -> System.Threading.Tasks.ValueTask diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt index ef812db0a70..7b2d44f24b1 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt @@ -1,5 +1,7 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler() -> void +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? +Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DisposeAsync() -> System.Threading.Tasks.ValueTask diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md index fe8dee3a50b..47f4dd42023 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md @@ -99,6 +99,25 @@ names, not workflow IDs, credentials, or arbitrary URLs. A finite factory handle lifetime lets subsequent invocations refresh connections without interrupting active calls. Streamable HTTP remains enforced for every client source. +`ProtocolVersion` defaults to `null`, preserving the MCP SDK's automatic protocol +discovery and legacy fallback. If the server's supported protocol is known, set +the init-only property on either constructor: + +```csharp +DefaultMcpToolHandler handler = new(httpClientFactory, "workflow-mcp") +{ + ProtocolVersion = "2025-11-25" +}; +``` + +An explicit legacy version skips the discovery probe, including its possible +timeout, but still initializes and disposes a separate session for every call. +The server must negotiate that exact legacy version. Explicit newer revisions +still perform discovery and do not downgrade to a legacy-only server. +Incompatible selections fail rather than silently reverting to automatic +negotiation. Use separately configured handlers for differing server protocol +policies; do not pin a version without knowing the server supports it. + The handler is not disposable. The workflow host owns admission limits, invocation deadlines, and shutdown cancellation. Bound concurrent work and pass a cancellation token covering the whole invocation; cancel and await those calls diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs index f29ae6e481c..7942255f45d 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs @@ -12,6 +12,7 @@ using System.Threading.Tasks; using Microsoft.Extensions.AI; using Microsoft.Extensions.DependencyInjection; +using ModelContextProtocol; using ModelContextProtocol.Client; using Moq; using Moq.Protected; @@ -23,6 +24,83 @@ namespace Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests; /// public sealed class DefaultMcpToolHandlerLifetimeTests { + [Theory] + [InlineData(null, 2)] + [InlineData("2025-11-25", 0)] + public async Task ProtocolSelection_PreservesPerInvocationInitializationAsync(string? protocolVersion, int expectedDiscoveries) + { + // Arrange + ProtocolStub stub = new(); + DefaultMcpToolHandler handler = CreateHandler(stub, protocolVersion); + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + + // Act + McpServerToolResultContent first = await InvokeAsync(handler, "session", timeout.Token); + McpServerToolResultContent second = await InvokeAsync(handler, "session", timeout.Token); + + // Assert + Assert.NotNull(first.Outputs); + Assert.NotNull(second.Outputs); + Assert.NotEqual( + Assert.IsType(Assert.Single(first.Outputs)).Text, + Assert.IsType(Assert.Single(second.Outputs)).Text); + Assert.Equal(expectedDiscoveries, stub.DiscoverRequests); + Assert.Equal(2, stub.Initializations); + Assert.Equal(2, stub.Terminations); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task ExplicitNewProtocol_DiscoversWithoutLegacyFallbackAsync(bool supportsDiscovery) + { + // Arrange + ProtocolStub stub = new() { SupportsDiscovery = supportsDiscovery }; + DefaultMcpToolHandler handler = CreateHandler(stub, "2026-07-28"); + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + + // Act + if (supportsDiscovery) + { + McpServerToolResultContent result = await InvokeAsync(handler, "ping", timeout.Token); + Assert.NotNull(result.Outputs); + Assert.Equal("ok", Assert.IsType(Assert.Single(result.Outputs)).Text); + } + else + { + await Assert.ThrowsAsync(() => InvokeAsync(handler, "ping", timeout.Token)); + } + + // Assert + Assert.Equal(1, stub.DiscoverRequests); + Assert.Equal(0, stub.Initializations); + Assert.Equal(0, stub.Terminations); + Assert.Single(stub.Handlers).Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); + } + + [Fact] + public async Task ExplicitLegacyProtocol_RejectsVersionMismatchAndDisposesResourcesAsync() + { + // Arrange + ProtocolStub stub = new() + { + InitializationProtocolVersion = "2025-06-18", + OnOperation = _ => Assert.Fail("A tool must not run after a protocol version mismatch.") + }; + DefaultMcpToolHandler handler = CreateHandler(stub, "2025-11-25"); + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + + // Act + McpException exception = await Assert.ThrowsAsync(() => InvokeAsync(handler, "ping", timeout.Token)); + + // Assert + Assert.Contains("protocol version mismatch", exception.Message); + Assert.Equal(0, stub.DiscoverRequests); + Assert.Equal(1, stub.Initializations); + Assert.Equal(1, stub.Terminations); + Assert.Single(stub.Handlers).Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); + } + [Theory] [InlineData(false)] [InlineData(true)] @@ -484,11 +562,11 @@ public async Task ClientConnection_SessionCleanupFailure_StillCleansUpTransportA } } - private static DefaultMcpToolHandler CreateHandler(ProtocolStub stub) + private static DefaultMcpToolHandler CreateHandler(ProtocolStub stub, string? protocolVersion = null) { Mock factory = new(); factory.Setup(f => f.CreateClient("workflow-mcp")).Returns(() => new HttpClient(stub.CreateMessageHandler())); - return new(factory.Object, "workflow-mcp"); + return new(factory.Object, "workflow-mcp") { ProtocolVersion = protocolVersion }; } private static Task InvokeAsync( @@ -500,10 +578,12 @@ private sealed class ProtocolStub private int _initializations; private int _terminations; private int _standaloneGetRequests; + private int _discoverRequests; public int Initializations => this._initializations; public int Terminations => this._terminations; public int StandaloneGetRequests => this._standaloneGetRequests; + public int DiscoverRequests => this._discoverRequests; public List> Handlers { get; } = []; public Func? BeforeInitializationAsync { get; set; } public Func? BeforeOperationAsync { get; set; } @@ -512,6 +592,8 @@ private sealed class ProtocolStub public bool FailOperation { get; set; } public bool FailTransportDisposal { get; set; } public bool UseEventStreamResult { get; set; } + public bool SupportsDiscovery { get; set; } + public string? InitializationProtocolVersion { get; set; } public HttpMessageHandler CreateMessageHandler() { @@ -558,12 +640,15 @@ private async Task SendAsync(HttpRequestMessage request, Ca string? method = message.GetProperty("method").GetString(); if (method == "server/discover") { + Interlocked.Increment(ref this._discoverRequests); + const string DiscoverResult = """{"supportedVersions":["2026-07-28"],"capabilities":{"tools":{}}}"""; + string discoverResponse = this.SupportsDiscovery + ? $$$"""{"jsonrpc":"2.0","id":{{{id.GetRawText()}}},"result":{{{DiscoverResult}}}}""" + : $$$"""{"jsonrpc":"2.0","id":{{{id.GetRawText()}}},"error":{"code":-32601,"message":"Method not found"}}"""; return new(HttpStatusCode.OK) { RequestMessage = request, - Content = new StringContent( - $$$"""{"jsonrpc":"2.0","id":{{{id.GetRawText()}}},"error":{"code":-32601,"message":"Method not found"}}""", - Encoding.UTF8, "application/json") + Content = new StringContent(discoverResponse, Encoding.UTF8, "application/json") }; } @@ -582,7 +667,9 @@ private async Task SendAsync(HttpRequestMessage request, Ca } sessionId = Interlocked.Increment(ref this._initializations).ToString(CultureInfo.InvariantCulture); - string protocolVersion = message.GetProperty("params").GetProperty("protocolVersion").GetRawText(); + string protocolVersion = this.InitializationProtocolVersion is null + ? message.GetProperty("params").GetProperty("protocolVersion").GetRawText() + : JsonSerializer.Serialize(this.InitializationProtocolVersion); result = $$$"""{"protocolVersion":{{{protocolVersion}}},"capabilities":{"tools":{}},"serverInfo":{"name":"stub","version":"1.0"}}"""; } else diff --git a/skills/agent-framework-builder-dotnet/references/declarative.md b/skills/agent-framework-builder-dotnet/references/declarative.md index 1197f14bd85..cc3b1280ff6 100644 --- a/skills/agent-framework-builder-dotnet/references/declarative.md +++ b/skills/agent-framework-builder-dotnet/references/declarative.md @@ -31,6 +31,10 @@ commitments. configuration for HTTP handler reuse and rotation. Factory-created clients are disposed per invocation; the factory remains application-owned. The parameterless non-DI fallback owns a dedicated HTTP handler per invocation. Callback-based constructors and handler-level disposal are no longer supported. +- Leave `DefaultMcpToolHandler.ProtocolVersion` unset for SDK automatic negotiation. When a server's protocol is + known, an explicit legacy version skips the discovery probe but still creates a fresh session per invocation. + Selection restricts negotiation and incompatible versions fail. Configure separate handlers for differing server + policies; explicit newer revisions still perform discovery and do not downgrade to legacy-only servers. - Configure factory clients with cookies and redirects disabled, certificate revocation checking enabled, and origin-bound credential injection. Do not retain per-workflow credentials or mutable origin state in pooled handlers, and do not derive client names from workflow IDs, credentials, or arbitrary URLs. From 2420a90b7ad9ce87f66e66ccebf59e07cd1b2be0 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 27 Oct 2026 11:34:17 -0400 Subject: [PATCH 09/13] docs: consider isolated MCP discovery metadata optimization Record scope, freshness, and supported-SDK requirements without implementing a cache or reusing clients and sessions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 27dcc08f-0b3a-40a6-a3ad-2c45eafce159 --- ...0047-dotnet-workflow-owned-mcp-sessions.md | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md b/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md index 6aaf1e82f1f..5827bbf0f89 100644 --- a/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md +++ b/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md @@ -275,6 +275,27 @@ Explicit newer revisions still perform discovery and reject legacy-only servers rather than silently downgrading. Incompatible selections surface SDK errors. Use separately configured handlers where server protocol policies differ. +### Future discovery-metadata optimization (not implemented) + +If measurements justify further setup optimization, consider a scoped metadata +profile or proxy containing detached protocol versions, capabilities, server +identity, and instructions. Its identity must include the complete endpoint, +configuration, authorization scope, and client capabilities; an origin alone is +not sufficient. Define bounded capacity, expiry, invalidation, and immutable or +defensively copied values. Server metadata must not become an authorization +decision or permit stale capabilities to override current policy. + +For discovery-based revisions, the SDK currently discovers on each fresh +`CreateAsync`. An `IMcpToolHandler` decorator alone cannot bypass that step. +Any metadata bootstrap would require supported SDK semantics beneath that +boundary; do not fake initialization or misuse session-resumption APIs. +Legacy sessions must still perform their real initialization handshake. + +This is a future consideration, not an implemented cache or a plan to reuse MCP +clients. Never retain clients, transports, session IDs, invocation credentials, +or mutable protocol machinery in such a profile. Every invocation must continue +to own and dispose its client and transport independently. + ### Compatibility and explicitly stateful integrations Remove the `httpClientProvider` callback constructor and the internal From 97c658aa53c6b84d56d840408974dd0da8388b31 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 27 Oct 2026 11:35:17 -0400 Subject: [PATCH 10/13] docs: consider an isolated MCP inventory proxy Describe optional tools/list metadata caching with bounded identity-scoped entries, fresh results, and no client or session reuse. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 27dcc08f-0b3a-40a6-a3ad-2c45eafce159 --- ...0047-dotnet-workflow-owned-mcp-sessions.md | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md b/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md index 5827bbf0f89..4720bc78e59 100644 --- a/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md +++ b/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md @@ -296,6 +296,27 @@ clients. Never retain clients, transports, session IDs, invocation credentials, or mutable protocol machinery in such a profile. Every invocation must continue to own and dispose its client and transport independently. +### Future inventory proxy (not implemented) + +For inventory-heavy workflows, consider an optional `IMcpToolHandler` decorator +that caches explicit `tools/list` requests while forwarding ordinary tool calls +unchanged. This would be new inventory caching, not preservation of the old +initialized-client cache. Named tool calls do not automatically list tools, so +this would not remove protocol setup from forwarded tool calls. + +Cache only detached, immutable or defensively copied tool metadata, never +`McpClientTool` objects bound to a client, clients, transports, session IDs, or +credentials. Scope entries by the complete endpoint, configuration, +authorization scope, and relevant client capabilities, not merely origin. +Preserve input validation and authorization on every access. Define bounded +capacity, expiry, and invalidation without depending on unsolicited +notifications from a session that is no longer retained. + +Each cache hit must produce fresh result content and correlation identity, +without exposing mutable cached objects to callers. Do not cache failures, +cancellations, or tool-call results. This proxy is a possible future improvement +only; no inventory cache or MCP client reuse is introduced by this change. + ### Compatibility and explicitly stateful integrations Remove the `httpClientProvider` callback constructor and the internal From 0e0d1bf3d7bd67e933f081a798783955b6887360 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 27 Oct 2026 12:12:26 -0400 Subject: [PATCH 11/13] fix: acknowledge intentional workflow MCP API removals Add target-scoped package compatibility suppressions for the removed callback constructor, DisposeAsync method, and IAsyncDisposable contract. Retain all other package validation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 27dcc08f-0b3a-40a6-a3ad-2c45eafce159 --- .../CompatibilitySuppressions.xml | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/CompatibilitySuppressions.xml diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/CompatibilitySuppressions.xml b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/CompatibilitySuppressions.xml new file mode 100644 index 00000000000..f233d3dd3a6 --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/CompatibilitySuppressions.xml @@ -0,0 +1,109 @@ + + + + + CP0002 + M:Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.#ctor(System.Func{System.String,System.Threading.CancellationToken,System.Threading.Tasks.Task{System.Net.Http.HttpClient}}) + lib/net10.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + lib/net10.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + true + + + CP0002 + M:Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DisposeAsync + lib/net10.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + lib/net10.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + true + + + CP0002 + M:Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.#ctor(System.Func{System.String,System.Threading.CancellationToken,System.Threading.Tasks.Task{System.Net.Http.HttpClient}}) + lib/net472/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + lib/net472/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + true + + + CP0002 + M:Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DisposeAsync + lib/net472/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + lib/net472/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + true + + + CP0002 + M:Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.#ctor(System.Func{System.String,System.Threading.CancellationToken,System.Threading.Tasks.Task{System.Net.Http.HttpClient}}) + lib/net8.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + lib/net8.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + true + + + CP0002 + M:Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DisposeAsync + lib/net8.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + lib/net8.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + true + + + CP0002 + M:Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.#ctor(System.Func{System.String,System.Threading.CancellationToken,System.Threading.Tasks.Task{System.Net.Http.HttpClient}}) + lib/net9.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + lib/net9.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + true + + + CP0002 + M:Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DisposeAsync + lib/net9.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + lib/net9.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + true + + + CP0002 + M:Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.#ctor(System.Func{System.String,System.Threading.CancellationToken,System.Threading.Tasks.Task{System.Net.Http.HttpClient}}) + lib/netstandard2.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + lib/netstandard2.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + true + + + CP0002 + M:Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DisposeAsync + lib/netstandard2.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + lib/netstandard2.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + true + + + CP0008 + T:Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler + lib/net10.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + lib/net10.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + true + + + CP0008 + T:Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler + lib/net472/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + lib/net472/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + true + + + CP0008 + T:Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler + lib/net8.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + lib/net8.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + true + + + CP0008 + T:Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler + lib/net9.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + lib/net9.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + true + + + CP0008 + T:Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler + lib/netstandard2.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + lib/netstandard2.0/Microsoft.Agents.AI.Workflows.Declarative.Mcp.dll + true + + \ No newline at end of file From b9856e4797e9f23043ea4d1e73bdde2776ce1e6e Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Wed, 28 Oct 2026 14:09:25 -0400 Subject: [PATCH 12/13] fix: cover parameterless workflow MCP session lifetime Exercise the real fallback HTTP pipeline against a bounded loopback server and verify separate sessions terminate before each invocation returns. Reuse the existing protocol stub for automatic and explicit legacy negotiation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 27dcc08f-0b3a-40a6-a3ad-2c45eafce159 --- .../DefaultMcpToolHandlerLifetimeTests.cs | 200 +++++++++++++++++- 1 file changed, 198 insertions(+), 2 deletions(-) diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs index 7942255f45d..d8951996168 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs @@ -4,8 +4,10 @@ using System.Collections.Generic; using System.Diagnostics; using System.Globalization; +using System.IO; using System.Net; using System.Net.Http; +using System.Net.Sockets; using System.Text; using System.Text.Json; using System.Threading; @@ -20,10 +22,45 @@ namespace Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests; /// -/// Functional session-lifetime tests using an in-memory HTTP transport and inert tool results. +/// Functional session-lifetime tests using in-memory and loopback HTTP transports and inert tool results. /// public sealed class DefaultMcpToolHandlerLifetimeTests { + [Theory] + [InlineData(null)] + [InlineData("2025-11-25")] + public async Task ParameterlessHandler_SequentialInvocations_DisposeSeparateSessionsBeforeReturningAsync(string? protocolVersion) + { + // Arrange + ProtocolStub stub = new(); + await using LoopbackMcpServer server = new(stub); + DefaultMcpToolHandler handler = new() { ProtocolVersion = protocolVersion }; + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(30)); + + // Act + McpServerToolResultContent first = await handler.InvokeToolAsync( + server.Endpoint, null, "session", null, null, null, timeout.Token); + + // Assert + Assert.NotNull(first.Outputs); + string firstSession = Assert.IsType(Assert.Single(first.Outputs)).Text; + Assert.Equal(1, stub.Initializations); + Assert.Equal(1, stub.Terminations); + Assert.Equal(firstSession, Assert.Single(server.TerminatedSessions)); + + // Act + McpServerToolResultContent second = await handler.InvokeToolAsync( + server.Endpoint, null, "session", null, null, null, timeout.Token); + + // Assert + Assert.NotNull(second.Outputs); + string secondSession = Assert.IsType(Assert.Single(second.Outputs)).Text; + Assert.NotEqual(firstSession, secondSession); + Assert.Equal(2, stub.Initializations); + Assert.Equal(2, stub.Terminations); + Assert.Equal(new[] { firstSession, secondSession }, server.TerminatedSessions); + } + [Theory] [InlineData(null, 2)] [InlineData("2025-11-25", 0)] @@ -611,7 +648,7 @@ public HttpMessageHandler CreateMessageHandler() return handler.Object; } - private async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + public async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { if (request.Method == HttpMethod.Get) { @@ -720,6 +757,165 @@ private static HttpResponseMessage EmptyResponse(HttpStatusCode statusCode, Http }; } + private sealed class LoopbackMcpServer : IAsyncDisposable + { + private readonly TcpListener _listener = new(IPAddress.Loopback, 0); + private readonly CancellationTokenSource _shutdown = new(TimeSpan.FromSeconds(30)); + private readonly Task _requestsTask; + + public LoopbackMcpServer(ProtocolStub stub) + { + this._listener.Start(); + this.Endpoint = $"http://127.0.0.1:{((IPEndPoint)this._listener.LocalEndpoint).Port}/mcp"; + this._requestsTask = this.ProcessRequestsAsync(stub); + } + + public string Endpoint { get; } + + public List TerminatedSessions { get; } = []; + + public async ValueTask DisposeAsync() + { + this._shutdown.Cancel(); + this._listener.Stop(); + try + { + await this._requestsTask; + } + finally + { +#if NET + this._listener.Dispose(); +#endif + this._shutdown.Dispose(); + } + } + + private async Task ProcessRequestsAsync(ProtocolStub stub) + { + CancellationToken cancellationToken = this._shutdown.Token; + using CancellationTokenRegistration stopListener = cancellationToken.Register(this._listener.Stop); + try + { + while (true) + { + using TcpClient client = await this._listener.AcceptTcpClientAsync(); + using CancellationTokenRegistration closeClient = cancellationToken.Register(client.Close); + using NetworkStream stream = client.GetStream(); + string requestLine = await ReadLineAsync(stream, cancellationToken); + using HttpRequestMessage request = new(new HttpMethod(requestLine.Split(' ')[0]), this.Endpoint); + Dictionary headers = new(StringComparer.OrdinalIgnoreCase); + string header; + while ((header = await ReadLineAsync(stream, cancellationToken)).Length > 0) + { + int separator = header.IndexOf(':'); + headers.Add(header.Substring(0, separator), header.Substring(separator + 1).Trim()); + } + + using MemoryStream body = new(); + if (headers.TryGetValue("Transfer-Encoding", out string? transferEncoding) && + string.Equals(transferEncoding, "chunked", StringComparison.OrdinalIgnoreCase)) + { + int size; + while ((size = int.Parse(await ReadLineAsync(stream, cancellationToken), NumberStyles.HexNumber, CultureInfo.InvariantCulture)) > 0) + { + byte[] chunk = await ReadBytesAsync(stream, size, cancellationToken); + body.Write(chunk, 0, chunk.Length); + Assert.Empty(await ReadLineAsync(stream, cancellationToken)); + } + + Assert.Empty(await ReadLineAsync(stream, cancellationToken)); + } + else if (headers.TryGetValue("Content-Length", out string? length)) + { + byte[] bytes = await ReadBytesAsync(stream, int.Parse(length, CultureInfo.InvariantCulture), cancellationToken); + body.Write(bytes, 0, bytes.Length); + } + + request.Content = new ByteArrayContent(body.ToArray()); + foreach (KeyValuePair pair in headers) + { + if (!request.Headers.TryAddWithoutValidation(pair.Key, pair.Value)) + { + request.Content.Headers.TryAddWithoutValidation(pair.Key, pair.Value); + } + } + + if (request.Method == HttpMethod.Delete) + { + this.TerminatedSessions.Add(Assert.Single(request.Headers.GetValues("Mcp-Session-Id"))); + } + + using HttpResponseMessage response = await stub.SendAsync(request, cancellationToken); + byte[] content = await response.Content.ReadAsByteArrayAsync(); + StringBuilder responseHeaders = new($"HTTP/1.1 {(int)response.StatusCode} {response.ReasonPhrase}\r\n"); + foreach (KeyValuePair> pair in response.Headers) + { + responseHeaders.Append($"{pair.Key}: {string.Join(", ", pair.Value)}\r\n"); + } + + responseHeaders.Append($"Content-Type: {response.Content.Headers.ContentType}\r\nContent-Length: {content.Length}\r\nConnection: close\r\n\r\n"); + byte[] headerBytes = Encoding.ASCII.GetBytes(responseHeaders.ToString()); +#if NET + await stream.WriteAsync(headerBytes, cancellationToken); + await stream.WriteAsync(content, cancellationToken); +#else + await stream.WriteAsync(headerBytes, 0, headerBytes.Length, cancellationToken); + await stream.WriteAsync(content, 0, content.Length, cancellationToken); +#endif + } + } + catch (Exception exception) when (cancellationToken.IsCancellationRequested && + exception is SocketException or ObjectDisposedException or IOException or OperationCanceledException) + { + Trace.WriteLine($"Loopback MCP server stopped: {exception.Message}"); + } + } + + private static async Task ReadLineAsync(NetworkStream stream, CancellationToken cancellationToken) + { + StringBuilder line = new(); + byte[] buffer = new byte[1]; +#if NET + while (await stream.ReadAsync(buffer, cancellationToken) > 0) +#else + while (await stream.ReadAsync(buffer, 0, 1, cancellationToken) > 0) +#endif + { + if (buffer[0] == '\n') + { + return line.ToString().TrimEnd('\r'); + } + + line.Append((char)buffer[0]); + } + + throw new EndOfStreamException("The HTTP request ended before its headers were complete."); + } + + private static async Task ReadBytesAsync(NetworkStream stream, int count, CancellationToken cancellationToken) + { + byte[] buffer = new byte[count]; + int offset = 0; + while (offset < count) + { +#if NET + int read = await stream.ReadAsync(buffer.AsMemory(offset, count - offset), cancellationToken); +#else + int read = await stream.ReadAsync(buffer, offset, count - offset, cancellationToken); +#endif + if (read == 0) + { + throw new EndOfStreamException("The HTTP request ended before its body was complete."); + } + + offset += read; + } + + return buffer; + } + } + private sealed class CleanupTraceListener : TraceListener { private readonly StringBuilder _output = new(); From 0710626414250a1dbf3daddeb8d1771984b13649 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Wed, 28 Oct 2026 14:58:43 -0400 Subject: [PATCH 13/13] fix(dotnet): require application-owned HTTP factory for workflow MCP Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 27dcc08f-0b3a-40a6-a3ad-2c45eafce159 --- ...0047-dotnet-workflow-owned-mcp-sessions.md | 47 ++++++++-------- .../DefaultMcpToolHandler.cs | 53 ++----------------- ...Agents.AI.Workflows.Declarative.Mcp.csproj | 1 - .../PublicAPI/net10.0/PublicAPI.Unshipped.txt | 1 - .../PublicAPI/net472/PublicAPI.Unshipped.txt | 1 - .../PublicAPI/net8.0/PublicAPI.Unshipped.txt | 1 - .../PublicAPI/net9.0/PublicAPI.Unshipped.txt | 1 - .../netstandard2.0/PublicAPI.Unshipped.txt | 1 - .../README.md | 20 +++---- .../Shared/OriginPinningHandlerTests.cs | 2 +- .../InvokeToolWorkflowTest.cs | 12 ++++- .../DefaultMcpToolHandlerLifetimeTests.cs | 32 ++++++++--- .../DefaultMcpToolHandlerTests.cs | 23 +++----- .../references/declarative.md | 6 +-- 14 files changed, 86 insertions(+), 115 deletions(-) rename dotnet/tests/{Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests => Microsoft.Agents.AI.Foundry.Hosting.UnitTests}/Shared/OriginPinningHandlerTests.cs (99%) diff --git a/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md b/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md index 4720bc78e59..69a4902fa6f 100644 --- a/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md +++ b/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md @@ -132,8 +132,9 @@ This revises the original workflow-lifetime ownership proposal. It does not establish that arbitrary MCP clients are interchangeable or safe to share across workflows. The default handler uses invocation-scoped clients; applications requiring cross-call session continuity supply an explicitly scoped integration. -The public invocation contract remains unchanged. The callback constructor and -handler-level disposal contract are removed in favor of a stateless handler. +The public invocation contract remains unchanged. Parameterless construction, +the callback constructor, and handler-level disposal are removed in favor of a +stateless handler requiring an application-owned HTTP client factory. Review of these breaking changes and their migration guidance remains required. ### Security isolation remains a separate requirement @@ -200,10 +201,12 @@ It is not supplied by retaining a workflow-lifetime connection. ### HTTP ownership and factory integration -| Source of HTTP client | Use and disposal contract | -|---|---| -| Application-provided factory | Obtain an owned wrapper for the chosen client scope; dispose it after MCP cleanup; let the factory manage pooled handlers | -| No factory | Preserve a parameterless non-DI path that creates and owns a client with documented transport defaults | +Require an application-provided `IHttpClientFactory`. Obtain an owned wrapper +for every invocation, dispose it after MCP cleanup, and let the factory manage +pooled handlers. Do not retain a parameterless fallback that creates a dedicated +HTTP handler per invocation: repeated connection teardown can exhaust ephemeral +ports even when every wrapper is correctly disposed. Applications configure +handler reuse and rotation once, including in non-web hosts. Owned wrappers must be disposed exactly once, including when initialization fails. MCP and transport cleanup must release owned HTTP resources even if @@ -229,26 +232,25 @@ provide pooling. ### Transport policy -Non-DI fallback clients retain explicit defaults: cookies disabled, automatic -redirects disabled, certificate revocation checking, Streamable HTTP, and -origin-bound credential handling. Factory defaults alone are not a substitute. +Applications configure cookies disabled, automatic redirects disabled, +certificate revocation checking, and origin-bound credential handling. +Unconfigured factory defaults alone are not a substitute for transport policy. Disable the SDK's standalone GET/SSE stream for unsolicited server messages. Unattended tool invocations do not require a background subscription channel. This avoids opening and closing that channel for each legacy initialization; -request-scoped SSE responses to tool calls remain supported. This policy applies -to both factory-backed and non-DI calls and does not change session ownership. +request-scoped SSE responses to tool calls remain supported. This policy does +not change session ownership. The integration does not silently replace application factory clients' pipelines. Configure their primary handlers with cookies and automatic redirects disabled and certificate revocation checking enabled. Any credential-injecting handler must restrict credentials to the configured server origin. Streamable -HTTP remains enforced regardless of the HTTP client's source. +HTTP remains enforced. -The fallback's `OriginPinningHandler` stores one origin and is owned by one -invocation. Do not move this mutable policy into one pooled pipeline for -unrelated origins. Factory handlers must instead use an appropriately bounded, -stable configuration or validated request-specific policy. Origin checks must +Do not store per-invocation origin-pinning state in a pooled pipeline for +unrelated origins. Factory handlers must use an appropriately bounded, stable +configuration or validated request-specific policy. Origin checks must account for every credential-injection step in outbound processing. Wrapper disposal must not disrupt other invocations sharing a factory-managed @@ -319,9 +321,10 @@ only; no inventory cache or MCP client reuse is introduced by this change. ### Compatibility and explicitly stateful integrations -Remove the `httpClientProvider` callback constructor and the internal -`httpMessageHandlerFactory` callback seam. Applications migrate authentication -to named `IHttpClientFactory` configurations and origin-bound delegating handlers. +Remove parameterless construction, the `httpClientProvider` callback constructor, +and the internal `httpMessageHandlerFactory` callback seam. Applications migrate +default construction and authentication to named `IHttpClientFactory` +configurations and origin-bound delegating handlers. Credentials that change per invocation must be resolved per request, not retained in pooled handlers. Tests use the same factory interface as applications. @@ -474,9 +477,9 @@ wrapper, owns its separate MCP client and transport, and disposes the wrapper after MCP cleanup. The factory owns handler pooling and rotation; the MCP handler does not dispose the application factory or its shared handlers. -The non-DI path creates and owns a dedicated HTTP handler and origin-policy -handler for each invocation. Callback construction and handler-level disposal -are removed. The host awaits its own invocations during shutdown. No +There is no internally created HTTP client or handler pipeline. Parameterless +and callback construction and handler-level disposal are removed. The host +provides a factory and awaits its own invocations during shutdown. No handler-owned shared invoker, internal service provider, workflow-runtime ownership hooks, invocation registry, synchronization lock, HTTP-client cache, or lease-counting pool are introduced. diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs index 61611c1e311..5ba5bf31052 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs @@ -24,7 +24,6 @@ namespace Microsoft.Agents.AI.Workflows.Declarative.Mcp; /// /// Every invocation creates and disposes a separate MCP client, including tools/list. /// Factory-created HTTP clients reuse factory-managed handlers without sharing MCP session state. -/// Without a factory, each invocation owns a dedicated HTTP handler. /// Standalone GET streams for unsolicited server messages are disabled; request-scoped SSE responses remain supported. /// The handler owns no shared disposable resources. The host must await its invocations before disposing /// the application-owned factory infrastructure. @@ -43,8 +42,8 @@ public sealed class DefaultMcpToolHandler : IMcpToolHandler private static readonly JsonWriterOptions s_toolListJsonWriterOptions = new() { Indented = true }; - private readonly IHttpClientFactory? _httpClientFactory; - private readonly string _httpClientName = string.Empty; + private readonly IHttpClientFactory _httpClientFactory; + private readonly string _httpClientName; /// /// Gets or initializes the protocol version requested for every invocation. @@ -60,18 +59,6 @@ public sealed class DefaultMcpToolHandler : IMcpToolHandler /// public string? ProtocolVersion { get; init; } - /// - /// Initializes a new instance of the class. - /// - /// - /// Each invocation owns its HTTP client and handler. Cookies and automatic redirects are disabled, - /// certificate revocation checking is enabled, and credential headers are pinned to the configured server origin. - /// Use the factory constructor for application-managed HTTP handler pooling and authentication. - /// - public DefaultMcpToolHandler() - { - } - /// /// Initializes a new instance of the class using an application-provided HTTP client factory. /// @@ -202,32 +189,7 @@ private async Task CreateClientAsync( EnableStandaloneGetStream = false }; - HttpClient httpClient; - if (this._httpClientFactory is not null) - { - httpClient = this._httpClientFactory.CreateClient(this._httpClientName); - } - else - { - // Pin credential headers to the configured server origin as defense-in-depth. Forcing - // StreamableHttp (below) already removes the primary vector (a server-advertised cross-origin - // SSE message endpoint), and AllowAutoRedirect=false blocks auto-redirects. This handler is the - // backstop: it guarantees the Authorization token and other credentials never leave the pinned - // origin even if a future change re-enables AutoDetect or redirects, or the SDK constructs a - // request to a new URI (AdditionalHeaders are re-stamped by the transport, so HttpClient's own - // redirect header-stripping does not cover them). - // Caveat for future maintainers: the handler strips credentials on every cross-origin request. - // That is safe today because this handler carries auth via static request headers, not the - // SDK's built-in OAuth flow. If OAuth support is ever added here, requests to the authorization - // server (a different origin than the MCP resource server) legitimately carry credentials, so - // those auth-server origins would need to be allow-listed to avoid breaking the token exchange. - OriginPinningHandler pinningHandler = new(endpoint) - { - InnerHandler = CreateHttpMessageHandler() - }; - httpClient = new HttpClient(pinningHandler); - } - + HttpClient httpClient = this._httpClientFactory.CreateClient(this._httpClientName); HttpClientTransport? transport = null; try { @@ -253,15 +215,6 @@ private async Task CreateClientAsync( } } - private static HttpClientHandler CreateHttpMessageHandler() => - new() - { - // Keep cookies out of transport state and prevent credential-bearing redirects. - UseCookies = false, - AllowAutoRedirect = false, - CheckCertificateRevocationList = true - }; - internal sealed class ClientConnection(McpClient client, IAsyncDisposable transport) : IAsyncDisposable { public McpClient Client { get; } = client; diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/Microsoft.Agents.AI.Workflows.Declarative.Mcp.csproj b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/Microsoft.Agents.AI.Workflows.Declarative.Mcp.csproj index 9b33f7001cd..e4728906bf2 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/Microsoft.Agents.AI.Workflows.Declarative.Mcp.csproj +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/Microsoft.Agents.AI.Workflows.Declarative.Mcp.csproj @@ -8,7 +8,6 @@ true true - true true true diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt index 7b2d44f24b1..6ab40b3b9f5 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net10.0/PublicAPI.Unshipped.txt @@ -1,6 +1,5 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void -Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler() -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt index 7b2d44f24b1..6ab40b3b9f5 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net472/PublicAPI.Unshipped.txt @@ -1,6 +1,5 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void -Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler() -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt index 7b2d44f24b1..6ab40b3b9f5 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net8.0/PublicAPI.Unshipped.txt @@ -1,6 +1,5 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void -Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler() -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt index 7b2d44f24b1..6ab40b3b9f5 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/net9.0/PublicAPI.Unshipped.txt @@ -1,6 +1,5 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void -Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler() -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt index 7b2d44f24b1..6ab40b3b9f5 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt @@ -1,6 +1,5 @@ #nullable enable Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Net.Http.IHttpClientFactory! httpClientFactory, string! httpClientName) -> void -Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler() -> void Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.get -> string? Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.ProtocolVersion.init -> void *REMOVED*Microsoft.Agents.AI.Workflows.Declarative.Mcp.DefaultMcpToolHandler.DefaultMcpToolHandler(System.Func!>? httpClientProvider = null) -> void diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md index 47f4dd42023..88b19029341 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md @@ -66,13 +66,14 @@ Standalone GET/SSE streams for unsolicited server messages are disabled because this handler performs request-scoped tool operations, not background subscriptions. SSE responses to individual tool requests remain supported. -Each handler-created HTTP client is disposed with its invocation. These clients -can reuse factory-managed HTTP handlers through the -`DefaultMcpToolHandler(IHttpClientFactory, string)` constructor. The second +Each factory-created HTTP client is disposed with its invocation. Construct the +handler with an application-owned `IHttpClientFactory` through +`DefaultMcpToolHandler(IHttpClientFactory, string)`. The second argument is a stable named-client configuration, or `string.Empty` for the default configuration. Factory-created wrappers are owned by the invocation; -the factory and its pooled handlers remain application-owned. Without a factory, -the parameterless constructor creates a dedicated HTTP handler per invocation. +the factory and its pooled handlers remain application-owned. There is no +parameterless constructor or internally created HTTP pipeline. Configure handler +reuse in the factory to avoid connection churn and port exhaustion. Configure the factory once in the application, not once per invocation. For example, with `Microsoft.Extensions.DependencyInjection`: @@ -97,11 +98,11 @@ Do not store per-workflow credentials, cookies, or mutable origin policy in pooled handlers. Use a bounded set of configuration names, not workflow IDs, credentials, or arbitrary URLs. A finite factory handler lifetime lets subsequent invocations refresh connections without interrupting -active calls. Streamable HTTP remains enforced for every client source. +active calls. Streamable HTTP remains enforced. `ProtocolVersion` defaults to `null`, preserving the MCP SDK's automatic protocol discovery and legacy fallback. If the server's supported protocol is known, set -the init-only property on either constructor: +the init-only property when constructing the handler: ```csharp DefaultMcpToolHandler handler = new(httpClientFactory, "workflow-mcp") @@ -126,8 +127,9 @@ Non-cancellation session and transport cleanup failures are reported through `System.Diagnostics.Trace` warnings and do not replace a tool result or its original error. -The callback-based constructor and handler-level `DisposeAsync`/`IAsyncDisposable` -contract have been removed. Migrate callback authentication to named factory +Parameterless construction, the callback-based constructor, and handler-level +`DisposeAsync`/`IAsyncDisposable` are not supported. Migrate default construction +and callback authentication to application-owned named factory clients and origin-bound delegating handlers; every factory call must return a fresh HTTP client wrapper owned by the invocation, not a borrowed shared client. Remove handler `await using`/disposal calls and await active invocations in the host. diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/Shared/OriginPinningHandlerTests.cs b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/Shared/OriginPinningHandlerTests.cs similarity index 99% rename from dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/Shared/OriginPinningHandlerTests.cs rename to dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/Shared/OriginPinningHandlerTests.cs index a060ec5a0b1..eb79f2d516b 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/Shared/OriginPinningHandlerTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/Shared/OriginPinningHandlerTests.cs @@ -8,7 +8,7 @@ using System.Threading; using System.Threading.Tasks; -namespace Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests; +namespace Microsoft.Agents.AI.Foundry.Hosting.UnitTests; /// /// Unit tests for the shared . diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.IntegrationTests/InvokeToolWorkflowTest.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.IntegrationTests/InvokeToolWorkflowTest.cs index bcb4be325d6..0a9a69f1299 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.IntegrationTests/InvokeToolWorkflowTest.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.IntegrationTests/InvokeToolWorkflowTest.cs @@ -16,6 +16,7 @@ using Microsoft.Agents.AI.Workflows.Declarative.Kit; using Microsoft.Agents.AI.Workflows.Declarative.Mcp; using Microsoft.Extensions.AI; +using Microsoft.Extensions.DependencyInjection; using Shared.IntegrationTests; namespace Microsoft.Agents.AI.Workflows.Declarative.IntegrationTests; @@ -187,7 +188,16 @@ private async Task RunInvokeMcpToolTestAsync( { // Arrange string workflowPath = GetWorkflowPath(workflowFileName); - DefaultMcpToolHandler mcpToolProvider = new(); + ServiceCollection services = new(); + services.AddHttpClient("workflow-mcp") + .ConfigurePrimaryHttpMessageHandler(() => new HttpClientHandler + { + UseCookies = false, + AllowAutoRedirect = false, + CheckCertificateRevocationList = true + }); + using ServiceProvider serviceProvider = services.BuildServiceProvider(); + DefaultMcpToolHandler mcpToolProvider = new(serviceProvider.GetRequiredService(), "workflow-mcp"); DeclarativeWorkflowOptions workflowOptions = await this.CreateOptionsAsync( externalConversation: false, mcpToolProvider: mcpToolProvider); diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs index d8951996168..1645d6a0d29 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerLifetimeTests.cs @@ -29,12 +29,29 @@ public sealed class DefaultMcpToolHandlerLifetimeTests [Theory] [InlineData(null)] [InlineData("2025-11-25")] - public async Task ParameterlessHandler_SequentialInvocations_DisposeSeparateSessionsBeforeReturningAsync(string? protocolVersion) + public async Task Factory_LoopbackInvocations_ReuseHandlerAndDisposeSeparateSessionsBeforeReturningAsync(string? protocolVersion) { // Arrange ProtocolStub stub = new(); await using LoopbackMcpServer server = new(stub); - DefaultMcpToolHandler handler = new() { ProtocolVersion = protocolVersion }; + int primaryHandlersCreated = 0; + ServiceCollection services = new(); + services.AddHttpClient("workflow-mcp") + .ConfigurePrimaryHttpMessageHandler(() => + { + Interlocked.Increment(ref primaryHandlersCreated); + return new HttpClientHandler + { + UseCookies = false, + AllowAutoRedirect = false, + CheckCertificateRevocationList = true + }; + }); + using ServiceProvider serviceProvider = services.BuildServiceProvider(); + DefaultMcpToolHandler handler = new(serviceProvider.GetRequiredService(), "workflow-mcp") + { + ProtocolVersion = protocolVersion + }; using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(30)); // Act @@ -59,6 +76,7 @@ public async Task ParameterlessHandler_SequentialInvocations_DisposeSeparateSess Assert.Equal(2, stub.Initializations); Assert.Equal(2, stub.Terminations); Assert.Equal(new[] { firstSession, secondSession }, server.TerminatedSessions); + Assert.Equal(1, primaryHandlersCreated); } [Theory] @@ -182,15 +200,13 @@ public async Task SequentialInvocations_UseSeparateSessionsAsync() } [Theory] - [InlineData("ftp://mcp.example/api", false)] - [InlineData("ftp://mcp.example/api", true)] - [InlineData("ws://mcp.example/api", false)] - [InlineData("ws://mcp.example/api", true)] - public async Task UnsupportedScheme_DoesNotAcquireHttpClientAsync(string serverUrl, bool useFactory) + [InlineData("ftp://mcp.example/api")] + [InlineData("ws://mcp.example/api")] + public async Task UnsupportedScheme_DoesNotAcquireHttpClientAsync(string serverUrl) { // Arrange Mock factory = new(MockBehavior.Strict); - DefaultMcpToolHandler handler = useFactory ? new(factory.Object, "workflow-mcp") : new(); + DefaultMcpToolHandler handler = new(factory.Object, "workflow-mcp"); // Act await Assert.ThrowsAsync(() => handler.InvokeToolAsync( diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs index c28cb1e7b05..13f0c65fffa 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs @@ -42,26 +42,19 @@ public void Constructor_WithNullHttpClientName_ShouldThrow() } [Theory] - [InlineData("\r", false, false)] - [InlineData("\n", false, false)] - [InlineData("\0", false, false)] - [InlineData("\r", true, false)] - [InlineData("\n", true, false)] - [InlineData("\0", true, false)] - [InlineData("\r", false, true)] - [InlineData("\n", false, true)] - [InlineData("\0", false, true)] - [InlineData("\r", true, true)] - [InlineData("\n", true, true)] - [InlineData("\0", true, true)] + [InlineData("\r", false)] + [InlineData("\n", false)] + [InlineData("\0", false)] + [InlineData("\r", true)] + [InlineData("\n", true)] + [InlineData("\0", true)] public async Task InvokeToolAsync_WithInvalidHeader_ShouldRejectBeforeClientCreationAsync( string invalidCharacter, - bool invalidName, - bool useFactory) + bool invalidName) { // Arrange Mock factory = new(MockBehavior.Strict); - DefaultMcpToolHandler handler = useFactory ? new(factory.Object, "workflow-mcp") : new(); + DefaultMcpToolHandler handler = new(factory.Object, "workflow-mcp"); Dictionary headers = new() { [invalidName ? $"X-Test{invalidCharacter}" : "X-Test"] = invalidName ? "value" : $"value{invalidCharacter}" diff --git a/skills/agent-framework-builder-dotnet/references/declarative.md b/skills/agent-framework-builder-dotnet/references/declarative.md index cc3b1280ff6..651801b6531 100644 --- a/skills/agent-framework-builder-dotnet/references/declarative.md +++ b/skills/agent-framework-builder-dotnet/references/declarative.md @@ -27,10 +27,10 @@ commitments. or a custom handler with a documented session-lifetime contract when continuity is required. - Standalone MCP GET/SSE streams for unsolicited messages are disabled; request-scoped SSE tool responses remain supported. Use an explicitly scoped integration for background subscriptions. -- Prefer `DefaultMcpToolHandler(IHttpClientFactory, string)` with an application-provided factory and stable named +- Construct `DefaultMcpToolHandler(IHttpClientFactory, string)` with an application-provided factory and stable named configuration for HTTP handler reuse and rotation. Factory-created clients are disposed per invocation; the - factory remains application-owned. The parameterless non-DI fallback owns a dedicated HTTP handler per invocation. - Callback-based constructors and handler-level disposal are no longer supported. + factory remains application-owned. Configure the factory once with pooled handlers to avoid connection churn and + port exhaustion. Parameterless construction, callback-based constructors, and handler-level disposal are unsupported. - Leave `DefaultMcpToolHandler.ProtocolVersion` unset for SDK automatic negotiation. When a server's protocol is known, an explicit legacy version skips the discovery probe but still creates a fresh session per invocation. Selection restricts negotiation and incompatible versions fail. Configure separate handlers for differing server