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..4720bc78e59 --- /dev/null +++ b/docs/decisions/0047-dotnet-workflow-owned-mcp-sessions.md @@ -0,0 +1,494 @@ +--- +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. + +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 + +- 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 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 transport protections with explicit factory-based authentication. +- 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. 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: **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 +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. 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. +Review of these breaking changes and their migration guidance remains required. + +### 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. 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 +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 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 +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 | +|---|---| +| 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 | + +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. 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 `DefaultMcpToolHandler(IHttpClientFactory, string)` constructor uses +the application's stable named-client configuration on every invocation. The +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. + +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 + +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 +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. +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. + +### 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. + +### 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. + +### 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 +`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 +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 +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. +- 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. +- 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. +- 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; 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. +- 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 + 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. 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. +- 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 + +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 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. 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, invocation registry, synchronization lock, HTTP-client cache, or lease-counting pool +are introduced. + +The package references `Microsoft.Extensions.Http` for all supported target +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 +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 +decision record, not an assertion made by this document. 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/samples/03-workflows/Declarative/InvokeFoundryToolboxMcp/Program.cs b/dotnet/samples/03-workflows/Declarative/InvokeFoundryToolboxMcp/Program.cs index 22a7a48deb2..79c6ddbbefd 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; - }); + UseCookies = false, + AllowAutoRedirect = false, + CheckCertificateRevocationList = true + }) + .AddHttpMessageHandler(() => new FoundryToolboxBearerTokenHandler(credential, new Uri(toolboxMcpServerUrl))) + .SetHandlerLifetime(TimeSpan.FromMinutes(2)); + await using ServiceProvider serviceProvider = services.BuildServiceProvider(); + DefaultMcpToolHandler mcpToolHandler = new( + serviceProvider.GetRequiredService(), McpHttpClientName); - 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 + // 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..4c2e3752ec8 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) + + // Reuse HTTP handlers while each MCP invocation owns its client. + ServiceCollection services = new(); + services.AddHttpClient(McpHttpClientName) + .ConfigurePrimaryHttpMessageHandler(() => new HttpClientHandler { - McpToolHandler = mcpToolHandler - }; + UseCookies = false, + AllowAutoRedirect = false, + CheckCertificateRevocationList = true + }) + .AddHttpMessageHandler(() => new AzureMcpBearerTokenHandler(credential)) + .SetHandlerLifetime(TimeSpan.FromMinutes(2)); + await using ServiceProvider serviceProvider = services.BuildServiceProvider(); + DefaultMcpToolHandler mcpToolHandler = new( + serviceProvider.GetRequiredService(), McpHttpClientName); - // Execute the workflow - WorkflowRunner runner = new() { UseJsonCheckpoints = true }; - await runner.ExecuteAsync(workflowFactory.CreateWorkflow, workflowInput); - } - finally + // 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); + } + } } 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 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..61611c1e311 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; @@ -23,16 +22,16 @@ 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. -/// 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. +/// 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 /// the invocation result or error. /// -public sealed class DefaultMcpToolHandler : IMcpToolHandler, IAsyncDisposable +public sealed class DefaultMcpToolHandler : IMcpToolHandler { private const string FilenameAdditionalPropertyName = "filename"; @@ -44,49 +43,57 @@ 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 bool _disposing; + 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. /// - /// - /// 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 . 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 - /// 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. - /// - /// - 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() { } - internal DefaultMcpToolHandler( - Func>? httpClientProvider, - Func httpMessageHandlerFactory) + /// + /// 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. + /// + /// + /// 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._httpClientProvider = httpClientProvider; - this._httpMessageHandlerFactory = Throw.IfNull(httpMessageHandlerFactory); + this._httpClientFactory = Throw.IfNull(httpClientFactory); + this._httpClientName = Throw.IfNull(httpClientName); } /// @@ -119,55 +126,13 @@ public async Task InvokeToolAsync( } } - if (this._httpClientProvider is not null) + cancellationToken.ThrowIfCancellationRequested(); + 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( @@ -217,135 +182,32 @@ internal static McpServerToolResultContent CreateListToolsResultContent(IEnumera return resultContent; } - /// - /// - /// Disposal is requested from an active provider-backed 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) - { - if (!context.Completion.IsCompleted) - { - throw new InvalidOperationException("Cannot dispose the MCP handler from an active provider-backed invocation."); - } - } - - Task? providerInvocations; - await this._clientLock.WaitAsync().ConfigureAwait(false); - try - { - this.ThrowIfDisposing(); - this._disposing = true; - providerInvocations = this._providerInvocationsDrained?.Task; - } - finally - { - this._clientLock.Release(); - } - - if (providerInvocations is not null) - { - await providerInvocations.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 - { - this._clientLock.Release(); - } - - this._clientLock.Dispose(); - } - - [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 async Task GetOrCreateClientAsync( - string serverUrl, - string? serverLabel, - IDictionary? headers, - string? connectionName, - CancellationToken cancellationToken) - { - string trimmedUrl = serverUrl.Trim(); - var clientCacheKey = BuildCacheKey(trimmedUrl, serverLabel, connectionName, headers); - - await this._clientLock.WaitAsync(cancellationToken).ConfigureAwait(false); - try - { - this.ThrowIfDisposing(); - if (this._clients.TryGetValue(clientCacheKey, out ClientConnection? existingClient)) - { - return existingClient.Client; - } - - 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)); - private async Task CreateClientAsync( string serverUrl, string? serverLabel, IDictionary? headers, - string? httpClientCacheKey, CancellationToken cancellationToken) { - // Only the no-provider path shares handler-owned HTTP clients. - HttpClient? httpClient = null; - bool ownsHttpClient = false; + 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, + EnableStandaloneGetStream = false + }; - if (this._httpClientProvider is not null) + HttpClient httpClient; + if (this._httpClientFactory is not null) { - httpClient = await this._httpClientProvider(serverUrl, cancellationToken).ConfigureAwait(false); + httpClient = this._httpClientFactory.CreateClient(this._httpClientName); } - - if (httpClient is null && - (httpClientCacheKey is null || !this._ownedHttpClients.TryGetValue(httpClientCacheKey, out httpClient))) + 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 @@ -359,60 +221,47 @@ 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) + OriginPinningHandler pinningHandler = new(endpoint) { - ownsHttpClient = true; - } - else - { - this._ownedHttpClients[httpClientCacheKey] = httpClient; - } + InnerHandler = CreateHttpMessageHandler() + }; + httpClient = new HttpClient(pinningHandler); } - HttpClientTransportOptions transportOptions = new() - { - Endpoint = new Uri(serverUrl), - 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 = new(transportOptions, httpClient, ownsHttpClient: ownsHttpClient); - + HttpClientTransport? transport = null; try { - McpClient client = await McpClient.CreateAsync(transport, cancellationToken: cancellationToken).ConfigureAwait(false); + transport = new(transportOptions, httpClient, ownsHttpClient: true); + McpClient client = await McpClient.CreateAsync( + transport, + new McpClientOptions { ProtocolVersion = this.ProtocolVersion }, + 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 + { + httpClient.Dispose(); + } + throw; } } - private static HttpMessageHandler CreateHttpMessageHandler() => - new HttpClientHandler + private static HttpClientHandler CreateHttpMessageHandler() => + new() { - // 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 }; - private sealed class ProviderInvocationContext(Task completion, ProviderInvocationContext? parent) - { - public Task Completion { get; } = completion; - - public ProviderInvocationContext? Parent { get; } = parent; - } - internal sealed class ClientConnection(McpClient client, IAsyncDisposable transport) : IAsyncDisposable { public McpClient Client { get; } = client; @@ -444,51 +293,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.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..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 +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 ab058de62d4..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 +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 ab058de62d4..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 +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 ab058de62d4..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 +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 ab058de62d4..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 +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 7cfd8de1341..47f4dd42023 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/README.md @@ -58,28 +58,88 @@ 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. -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. +`DefaultMcpToolHandler` creates a separate MCP client for every invocation, +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 +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. + +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")); +``` + +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. + +`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 +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. -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. +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 +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.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 3a481fcd0e6..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 @@ -11,6 +11,8 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.AI; +using Microsoft.Extensions.DependencyInjection; +using ModelContextProtocol; using ModelContextProtocol.Client; using Moq; using Moq.Protected; @@ -20,493 +22,497 @@ 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 { [Theory] - [InlineData("ping")] - [InlineData(DefaultMcpToolHandler.ListToolsToolName)] - public async Task Provider_SequentialInvocations_CreateAndDisposeSeparateSessionsAsync(string toolName) + [InlineData(null, 2)] + [InlineData("2025-11-25", 0)] + public async Task ProtocolSelection_PreservesPerInvocationInitializationAsync(string? protocolVersion, int expectedDiscoveries) { // Arrange ProtocolStub stub = new(); - using HttpClient client = new(stub.CreateMessageHandler()); - int providerCalls = 0; - DefaultMcpToolHandler handler = new((_, _) => - { - providerCalls++; - return Task.FromResult(client); - }); + DefaultMcpToolHandler handler = CreateHandler(stub, protocolVersion); 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(); + McpServerToolResultContent first = await InvokeAsync(handler, "session", timeout.Token); + McpServerToolResultContent second = await InvokeAsync(handler, "session", timeout.Token); // Assert - Assert.Equal(2, providerCalls); + 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); - stub.Handlers[0].Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); } - [Fact] - public async Task ProviderReturningNull_DisposesFallbackTransportPerInvocationAsync() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task ExplicitNewProtocol_DiscoversWithoutLegacyFallbackAsync(bool supportsDiscovery) { // Arrange - ProtocolStub stub = new(); - int providerCalls = 0; - await using DefaultMcpToolHandler handler = new((_, _) => - { - providerCalls++; - return Task.FromResult(null); - }, stub.CreateMessageHandler); + ProtocolStub stub = new() { SupportsDiscovery = supportsDiscovery }; + DefaultMcpToolHandler handler = CreateHandler(stub, "2026-07-28"); 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) + 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 { - transport.Protected().Verify("Dispose", Times.AtLeastOnce(), ItExpr.Is(disposing => disposing)); + 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 NoProvider_ReusesSessionUntilHandlerDisposalAsync() + public async Task ExplicitLegacyProtocol_RejectsVersionMismatchAndDisposesResourcesAsync() { // Arrange - ProtocolStub stub = new(); - DefaultMcpToolHandler handler = new(null, stub.CreateMessageHandler); + 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 - await InvokeAsync(handler, "ping", timeout.Token); - await InvokeAsync(handler, DefaultMcpToolHandler.ListToolsToolName, timeout.Token); + 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(0, 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.Single(stub.Handlers).Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); } - [Fact] - public async Task NoProvider_DifferentConnectionNames_UseSeparateCachedSessionsAsync() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task RequestOnlyInvocation_DoesNotOpenStandaloneStreamAsync(bool useEventStream) { // Arrange - ProtocolStub stub = new(); - DefaultMcpToolHandler handler = new(null, stub.CreateMessageHandler); + ProtocolStub stub = new() { UseEventStreamResult = useEventStream }; + DefaultMcpToolHandler handler = CreateHandler(stub); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); // Act - foreach (string connectionName in new[] { "first", "second", "first" }) - { - await handler.InvokeToolAsync( - "https://mcp.example/api", null, "ping", null, null, connectionName, timeout.Token); - } + McpServerToolResultContent result = await InvokeAsync(handler, "ping", timeout.Token); // Assert - Assert.Equal(2, stub.Initializations); - Assert.Single(stub.Handlers); - Assert.Equal(0, stub.Terminations); - await handler.DisposeAsync(); - Assert.Equal(2, stub.Terminations); + 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 Provider_Failure_DoesNotPreventHandlerDisposalAsync() + public async Task SequentialInvocations_UseSeparateSessionsAsync() { // Arrange - int providerCalls = 0; - DefaultMcpToolHandler handler = new((_, _) => - { - providerCalls++; - throw new InvalidOperationException("Provider unavailable."); - }); + ProtocolStub stub = new(); + DefaultMcpToolHandler handler = CreateHandler(stub); 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)); + McpServerToolResultContent first = await InvokeAsync(handler, "session", timeout.Token); + McpServerToolResultContent second = await InvokeAsync(handler, "session", timeout.Token); // Assert - Assert.Equal("Provider unavailable.", exception.Message); - Assert.Equal(1, providerCalls); + 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); } - [Fact] - public async Task Provider_ConcurrentInvocations_DoNotCoalesceOrSerializeSessionsAsync() + [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) { // Arrange - ProtocolStub stub = new(); - using HttpClient client = new(stub.CreateMessageHandler()); - using SemaphoreSlim started = new(0); - using SemaphoreSlim finish = new(0); - stub.BeforeOperationAsync = async token => - { - started.Release(); - await finish.WaitAsync(token); - }; - int providerCalls = 0; - await using DefaultMcpToolHandler handler = new((_, _) => - { - Interlocked.Increment(ref providerCalls); - return Task.FromResult(client); - }); - using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + Mock factory = new(MockBehavior.Strict); + DefaultMcpToolHandler handler = useFactory ? new(factory.Object, "workflow-mcp") : new(); // Act - Task first = InvokeAsync(handler, "ping", timeout.Token); - Task second = InvokeAsync(handler, "ping", timeout.Token); - await started.WaitAsync(timeout.Token); - await started.WaitAsync(timeout.Token); - finish.Release(2); - await Task.WhenAll(first, second); + await Assert.ThrowsAsync(() => handler.InvokeToolAsync( + serverUrl, null, "ping", null, null, null)); // Assert - Assert.Equal(2, providerCalls); - Assert.Equal(2, stub.Initializations); - Assert.Equal(2, stub.Terminations); + factory.Verify(f => f.CreateClient(It.IsAny()), Times.Never); } [Fact] - public async Task Provider_OperationFailure_DisposesSessionAndPreservesCallerClientAsync() + public async Task DiscoveryAndToolCalls_DisposeSeparateHttpClientsAndSessionsAsync() { // Arrange - ProtocolStub stub = new() { FailOperation = true }; - using HttpClient client = new(stub.CreateMessageHandler()); - await using DefaultMcpToolHandler handler = new((_, _) => Task.FromResult(client)); + ProtocolStub stub = new(); + DefaultMcpToolHandler handler = CreateHandler(stub); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); // Act - await Assert.ThrowsAsync(() => InvokeAsync(handler, "ping", timeout.Token)); + await InvokeAsync(handler, "ping", timeout.Token); + await InvokeAsync(handler, DefaultMcpToolHandler.ListToolsToolName, 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)); + 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(false)] - [InlineData(true)] - public async Task Provider_InitializationFailure_RespectsTransportOwnershipAsync(bool provideClient) + [Fact] + public async Task DifferentConnectionNames_DoNotRetainSessionsAsync() { // Arrange - ProtocolStub stub = new() { FailInitialization = true }; - using HttpClient? client = provideClient ? new(stub.CreateMessageHandler()) : null; - await using DefaultMcpToolHandler handler = new( - (_, _) => Task.FromResult(client), stub.CreateMessageHandler); + ProtocolStub stub = new(); + DefaultMcpToolHandler handler = CreateHandler(stub); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); // Act - await Assert.ThrowsAsync(() => InvokeAsync(handler, "ping", timeout.Token)); + foreach (string connectionName in new[] { "first", "second", "first" }) + { + await handler.InvokeToolAsync( + "https://mcp.example/api", null, "ping", null, null, connectionName, timeout.Token); + } // Assert - Mock transport = Assert.Single(stub.Handlers); - transport.Protected().Verify("Dispose", provideClient ? Times.Never() : Times.AtLeastOnce(), ItExpr.Is(disposing => disposing)); + Assert.Equal(3, stub.Initializations); + Assert.Equal(3, stub.Handlers.Count); + Assert.Equal(3, stub.Terminations); } [Fact] - public async Task Provider_Cancellation_DisposesSessionAsync() + public async Task DifferentOrigins_KeepRequestConfigurationAsync() { // 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)); + DefaultMcpToolHandler handler = CreateHandler(stub); + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); // Act - Task invocation = InvokeAsync(handler, "ping", cancellation.Token); - await started.WaitAsync(cancellation.Token); - cancellation.Cancel(); - await Assert.ThrowsAnyAsync(() => invocation); + 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(1, stub.Terminations); - stub.Handlers[0].Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); + Assert.Equal(2, stub.Initializations); + Assert.Equal(2, stub.Terminations); + Assert.Equal(2, stub.Handlers.Count); } [Theory] - [InlineData(false)] - [InlineData(true)] - public async Task DisposeAsync_WaitsForProviderInvocationCleanupAsync(bool pauseProvider) + [InlineData("session")] + [InlineData(DefaultMcpToolHandler.ListToolsToolName)] + public async Task Factory_SequentialInvocations_OwnClientsButReusePooledHandlersAsync(string toolName) { // 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) + const string ClientName = "workflow-mcp"; + ProtocolStub stub = new() { - started.Release(); - await finish.WaitAsync(token); - } - - if (!pauseProvider) - { - stub.BeforeOperationAsync = PauseAsync; - } - - int providerCalls = 0; - DefaultMcpToolHandler handler = new(async (_, token) => + 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(() => { - providerCalls++; - if (pauseProvider) - { - await PauseAsync(token); - } - + HttpClient client = applicationFactory.CreateClient(ClientName); + clients.Add(client); return client; }); + DefaultMcpToolHandler handler = new(factory.Object, ClientName); 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)); - finish.Release(); - await invocation; - await disposal; + McpServerToolResultContent first = await InvokeAsync(handler, toolName, timeout.Token); + McpServerToolResultContent second = await InvokeAsync(handler, toolName, timeout.Token); // Assert - Assert.Equal(1, providerCalls); - 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((_, _) => + Assert.Equal(2, clients.Count); + Assert.NotSame(clients[0], clients[1]); + if (toolName == "session") { - providerCalls++; - return Task.FromResult(null); - }); + Assert.NotNull(first.Outputs); + Assert.NotNull(second.Outputs); + Assert.NotEqual( + Assert.IsType(Assert.Single(first.Outputs)).Text, + Assert.IsType(Assert.Single(second.Outputs)).Text); + } - // Act - await Assert.ThrowsAsync(() => handler.InvokeToolAsync( - "https://mcp.example/api", null, DefaultMcpToolHandler.ListToolsToolName, - new Dictionary { ["unused"] = true }, null, null)); + foreach (HttpClient client in clients) + { + await Assert.ThrowsAsync(() => client.GetAsync(new Uri("https://mcp.example/api"), timeout.Token)); + } - // Assert - Assert.Equal(0, providerCalls); + 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(false)] - [InlineData(true)] - public async Task Provider_ReentrantDisposal_ThrowsWithoutStartingShutdownAsync(bool fromChildTask) + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(false, false)] + public async Task InvocationFailure_DisposesClientAndAllowsNextInvocationAsync(bool failInitialization, bool failOperation) { // Arrange - ProtocolStub stub = new(); - using HttpClient client = new(stub.CreateMessageHandler()); - DefaultMcpToolHandler? handler = null; - int rejectedDisposals = 0; - handler = new(async (_, _) => + ProtocolStub stub = new() { FailInitialization = failInitialization, FailOperation = failOperation }; + List clients = []; + Mock factory = new(); + factory.Setup(f => f.CreateClient("workflow-mcp")).Returns(() => { - async Task DisposeAsync() => await handler!.DisposeAsync(); - InvalidOperationException error = await Assert.ThrowsAsync( - () => fromChildTask ? Task.Run(DisposeAsync) : DisposeAsync()); - Assert.Contains("active provider-backed invocation", error.Message); - rejectedDisposals++; + HttpClient client = new(stub.CreateMessageHandler()); + 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); + }; + } + + DefaultMcpToolHandler handler = new(factory.Object, "workflow-mcp"); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + using CancellationTokenSource cancellation = CancellationTokenSource.CreateLinkedTokenSource(timeout.Token); // Act - await InvokeAsync(handler, "ping", timeout.Token); - await InvokeAsync(handler, "ping", timeout.Token); - await handler.DisposeAsync(); + 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)); + Assert.Single(stub.Handlers).Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); + stub.BeforeOperationAsync = null; + stub.FailInitialization = false; + stub.FailOperation = false; + McpServerToolResultContent result = await InvokeAsync(handler, "ping", timeout.Token); // Assert - Assert.Equal(2, rejectedDisposals); - Assert.Equal(2, stub.Terminations); + 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)); + stub.Handlers[1].Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); } [Fact] - public async Task Provider_CompletedInheritedContext_CanDisposeHandlerAsync() + public async Task FactoryFailure_DoesNotPreventNextInvocationAsync() { // Arrange ProtocolStub stub = new(); - using HttpClient client = new(stub.CreateMessageHandler()); - using SemaphoreSlim finished = new(0); + Mock factory = new(); + 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)); - 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); + 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); - await Assert.ThrowsAsync(() => InvokeAsync(handler, "ping", timeout.Token)); } - [Fact] - public async Task Provider_NestedInvocation_RestoresOuterDisposalGuardAsync() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ConcurrentInvocations_DoNotCoalesceOrSerializeSessionsAsync(bool reuseHandlers) { // Arrange ProtocolStub stub = new(); - using HttpClient client = new(stub.CreateMessageHandler()); - bool nested = false; - DefaultMcpToolHandler? handler = null; - handler = new(async (_, token) => + using ServiceProvider? serviceProvider = reuseHandlers + ? 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 => { - if (!nested) - { - nested = true; - await InvokeAsync(handler!, "ping", token); - await Assert.ThrowsAsync(() => handler!.DisposeAsync().AsTask()); - } - - return client; - }); + started.Release(); + await finish.WaitAsync(token); + }; + DefaultMcpToolHandler handler = serviceProvider is not null + ? new(serviceProvider.GetRequiredService(), "workflow-mcp") + : CreateHandler(stub); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); // Act - await InvokeAsync(handler, "ping", timeout.Token); - await handler.DisposeAsync(); + 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(); + 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.True(nested); + 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.Equal(reuseHandlers ? 1 : 2, stub.Handlers.Count); } [Theory] [InlineData(false)] [InlineData(true)] - public async Task Provider_TransportCleanupFailure_PreservesInvocationOutcomeAsync(bool failOperation) + public async Task ActiveInvocation_RetainsResourcesUntilCompletionAsync(bool pauseInitialization) { // Arrange - ProtocolStub stub = new() { FailOperation = failOperation, FailTransportDisposal = true }; - await using DefaultMcpToolHandler handler = new( - (_, _) => Task.FromResult(null), stub.CreateMessageHandler); - using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); - using CleanupTraceListener listener = new(); - Trace.Listeners.Add(listener); - try + ProtocolStub stub = new(); + using SemaphoreSlim started = new(0); + using SemaphoreSlim finish = new(0); + async Task PauseAsync(CancellationToken token) { - // 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)); + started.Release(); + await finish.WaitAsync(token); } - finally + + if (pauseInitialization) { - Trace.Listeners.Remove(listener); + stub.BeforeInitializationAsync = PauseAsync; } - } - - [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) => + else { - 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); - } + stub.BeforeOperationAsync = PauseAsync; + } - return client; - }); + DefaultMcpToolHandler handler = CreateHandler(stub); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); // Act - await InvokeAsync(handler, "ping", timeout.Token); - await handler.DisposeAsync(); + Task invocation = InvokeAsync(handler, "ping", timeout.Token); + await started.WaitAsync(timeout.Token); + Mock transport = Assert.Single(stub.Handlers); + transport.Protected().Verify("Dispose", Times.Never(), ItExpr.Is(disposing => disposing)); + finish.Release(); + await invocation; // Assert - Assert.Equal(2, providerCalls); - Assert.Equal(2, stub.Terminations); + Assert.Equal(1, stub.Initializations); + Assert.Equal(1, stub.Terminations); + transport.Protected().Verify("Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); } - [Fact] - public async Task Provider_InitializationAndCleanupFailure_PreservesInitializationErrorAsync() + [Theory] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(false, false)] + public async Task CleanupFailure_DoesNotReplaceOutcomeAsync(bool failInitialization, bool failOperation) { // Arrange - ProtocolStub stub = new() { FailInitialization = true, FailTransportDisposal = true }; - await using DefaultMcpToolHandler handler = new( - (_, _) => Task.FromResult(null), stub.CreateMessageHandler); + ProtocolStub stub = new() + { + 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 - await Assert.ThrowsAsync(() => InvokeAsync(handler, "ping", timeout.Token)); + if (failInitialization || 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); + } // Assert - Assert.Contains("Failed to dispose MCP transport", listener.Output); + 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( - "Dispose", Times.AtLeastOnce(), ItExpr.Is(disposing => disposing)); + "Dispose", Times.Once(), ItExpr.Is(disposing => disposing)); } finally { @@ -556,6 +562,13 @@ public async Task ClientConnection_SessionCleanupFailure_StillCleansUpTransportA } } + 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") { ProtocolVersion = protocolVersion }; + } + private static Task InvokeAsync( DefaultMcpToolHandler handler, string toolName, CancellationToken cancellationToken) => handler.InvokeToolAsync("https://mcp.example/api", null, toolName, null, null, null, cancellationToken); @@ -564,14 +577,23 @@ 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; } + public Action? OnOperation { get; set; } public bool FailInitialization { get; set; } 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() { @@ -593,6 +615,7 @@ private async Task SendAsync(HttpRequestMessage request, Ca { if (request.Method == HttpMethod.Get) { + Interlocked.Increment(ref this._standaloneGetRequests); return EmptyResponse(HttpStatusCode.MethodNotAllowed, request); } @@ -602,10 +625,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)) @@ -616,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") }; } @@ -629,17 +656,25 @@ 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); } 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 { + this.OnOperation?.Invoke(request); if (this.BeforeOperationAsync is not null) { await this.BeforeOperationAsync(cancellationToken); @@ -653,17 +688,21 @@ 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}") }; } + 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/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..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() - { - // Act - DefaultMcpToolHandler handler = new(); - - // Assert - Assert.NotNull(handler); - await handler.DisposeAsync(); - } - [Fact] - public async Task Constructor_WithNullHttpClientProvider_ShouldCreateInstanceAsync() + public void Constructor_WithNullHttpClientFactory_ShouldThrow() { // Act - DefaultMcpToolHandler handler = new(httpClientProvider: null); + 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_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 - Second dispose should throw ObjectDisposedException from the semaphore - await Assert.ThrowsAsync(actAsync); - } - - #endregion - - #region HttpClientProvider Tests - [Theory] [InlineData("\r", false, false)] [InlineData("\n", false, false)] @@ -106,456 +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); + factory.Verify(f => f.CreateClient(It.IsAny()), Times.Never); } [Fact] - public async Task InvokeToolAsync_WithHttpClientProvider_ShouldCallProviderAsync() + public async Task InvokeToolAsync_WithCancelledToken_ShouldRejectBeforeClientCreationAsync() { // Arrange - bool providerCalled = false; - string? capturedServerUrl = null; - - Task ProviderAsync(string url, CancellationToken ct) - { - providerCalled = true; - capturedServerUrl = url; - return Task.FromResult(null); - } + Mock factory = new(MockBehavior.Strict); + DefaultMcpToolHandler handler = new(factory.Object, "workflow-mcp"); + using CancellationTokenSource cancellation = new(); + cancellation.Cancel(); - 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); - } - - [Fact] - public async Task InvokeToolAsync_WithHttpClientProviderReturningClient_ShouldUseProvidedClientAsync() - { - // 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); - - // 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(); - } - - // Assert - Assert.True(providerCalled); - } - - #endregion - - #region Caching 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 - // 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 - { - await handler.InvokeToolAsync( - serverUrl: ServerUrl, - serverLabel: "test", - toolName: "testTool", - arguments: null, - headers: null, - connectionName: null); - } - catch - { - // Expected to fail - no real server - } - } - - // Assert - Provider is called each time because McpClient creation fails before caching - Assert.Equal(2, providerCallCount); - } - finally - { - await handler.DisposeAsync(); - } - } - - [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 - Different headers should create different cache keys - Assert.Equal(2, providerCallCount); - } - finally - { - await handler.DisposeAsync(); - } - } - - #endregion - - #region ComputeHeadersHash Tests - - [Fact] - public void ComputeHeadersHash_WithNullHeaders_ReturnsEmptyString() - { // Act - string result = DefaultMcpToolHandler.ComputeHeadersHash(null); + await Assert.ThrowsAnyAsync(() => handler.InvokeToolAsync( + "https://example.com/mcp", null, "ping", null, null, null, cancellation.Token)); // Assert - Assert.Equal(string.Empty, result); + factory.Verify(f => f.CreateClient(It.IsAny()), Times.Never); } - [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] @@ -582,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] @@ -642,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 b5bddca2c74..cc3b1280ff6 100644 --- a/skills/agent-framework-builder-dotnet/references/declarative.md +++ b/skills/agent-framework-builder-dotnet/references/declarative.md @@ -22,6 +22,24 @@ 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. +- 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. + 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. +- 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.