Add support for custom headers in HostedMcpServerTool #7053
Conversation
Introduces a Headers property to HostedMcpServerTool for specifying additional request headers. Updates OpenAIResponsesChatClient to pass these headers when creating MCP tools and adds unit tests to verify header roundtripping.
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for custom HTTP headers in HostedMcpServerTool, enabling users to specify additional request headers when communicating with remote MCP servers.
- Introduces a
Headersproperty of typeIDictionary<string, string>?to theHostedMcpServerToolclass - Updates
OpenAIResponsesChatClientto pass these headers to both variants ofResponseTool.CreateMcpTool() - Adds roundtrip tests to verify the Headers property can be set, retrieved, and cleared
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Libraries/Microsoft.Extensions.AI.Abstractions/Tools/HostedMcpServerTool.cs | Adds the nullable Headers property with XML documentation |
| src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIResponsesChatClient.cs | Passes mcpTool.Headers parameter to both CreateMcpTool overloads (URI-based and connector ID-based) |
| test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Tools/HostedMcpServerToolTests.cs | Adds assertions to verify Headers can be set to a dictionary, retrieved, and reset to null |
test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Tools/HostedMcpServerToolTests.cs
Show resolved
Hide resolved
|
@stephentoub any chance I can get an assignee on this one to help with getting it through? This is #7049 with the reverts. |
|
Thanks for your PR @echapmanFromBunnings. We originally pursued this API shape because OpenAI supported headers in the request, while Anthropic only supported passing an auth token. I initially took the broader approach and used a dictionary in Later, OpenAI added support for passing the token in the same way Anthropic does, which suggested to me that there might not be many use cases for headers beyond auth and that we don't need a headers dictionary in the class. For the sake of doing an informed decision, could you share more about what kinds of headers you need to pass to your MCP server? If we bring the headers dictionary back, we need to reconsider if we want to keep |
|
Hi @jozkee Since the MCP server essentially operates as an HTTP client, couldn’t we simply allow these headers to leverage the underlying protocol? In this context, headers are not being used for authentication, so encountering a header should not imply it is solely for that purpose. These are two distinct concerns and should be treated as such. |
src/Libraries/Microsoft.Extensions.AI.Abstractions/Tools/HostedMcpServerTool.cs
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIResponsesChatClient.cs
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIResponsesChatClient.cs
Outdated
Show resolved
Hide resolved
|
CI errors are about CompatibilitySuppression.xml since we are bringing the property back. I suspect we need to remove them from: cc @joperezr |
Deleted baseline suppressions for the Headers property and its setter in HostedMcpServerTool from CompatibilitySuppressions.xml.
Expanded XML documentation for the Headers property in HostedMcpServerTool to clarify usage and behavior. Minor formatting adjustments in OpenAIResponsesChatClient for MCP tool creation calls.
|
Addressed all PR comments, and added extensive documents on that headers prop @jozkee |
Deleted baseline suppressions for get_Headers and set_Headers members of HostedMcpServerTool across net8.0, net9.0, and netstandard2.0. This likely reflects removal or resolution of related compatibility issues.
…manFromBunnings/extensions into AddHeaderToMcpToolCreation
src/Libraries/Microsoft.Extensions.AI.Abstractions/Tools/HostedMcpServerTool.cs
Show resolved
Hide resolved
| /// <para> | ||
| /// The underlying provider may respond with an error if <see cref="AuthorizationToken"/> is set and an Authorization header is included. | ||
| /// </para> |
There was a problem hiding this comment.
If a provider would have problems with that, wouldn't it be better for it to just not send the auth header? Basically say that if AuthorizationToken is set, any Authorization header in Headers may be ignored, or alternatively say that AuthorizationToken may be ignored if Headers contains an Authorization header?
There was a problem hiding this comment.
That said, what's the benefit of having both? If we only had Headers, is that not sufficient? The token can be extracted from an auth header.
It feels awkward having both, like there are two possible sources of truth. (At the same time I understand that for the common case, having the simple AuthorizationToken property is simpler.)
There was a problem hiding this comment.
If we only had Headers, is that not sufficient?
It is sufficient but it still creates friction with Claude and OpenAI Connectors that only support Authorization Tokens. Converting the Authorization HTTP Header implies trimming the "Bearer " prefix.
If a provider would have problems with that, wouldn't it be better for it to just not send the auth header? Basically say that if AuthorizationToken is set, any Authorization header in Headers may be ignored, or alternatively say that AuthorizationToken may be ignored if Headers contains an Authorization header?
That would mean us (MEAI) deciding for users and providers in this behavior. My thinking was that is better to simply rely what was given to us to the provider. If we are going to decide how to handle this in MEAI.OpenAI I would rather choose throwing to signal there's a bug, which aligns with OpenAI's current behavior.
There was a problem hiding this comment.
it still creates friction with Claude and OpenAI Connectors that only support Authorization Tokens. Converting the Authorization HTTP Header implies trimming the "Bearer " prefix.
Having both properties introduces that friction. Implementations will need to account for both existing and the authorization token being in either the AuthorizationToken property or the Headers. A developer could put it in either and the implementation should still work. If a particular provider's API only supports one or the other, it'll need to extract the relevant data. Forcing a developer to know the details of the provider breaks the abstraction.
That would mean us (MEAI) deciding for users and providers in this behavior.
We have to provide a consistent abstraction. A developer using these APIs should not need to know the details of the exact provider being used. That's the point of an abstraction.
There was a problem hiding this comment.
Not all authorisations are bearer tokens auth schemes also, so it's not just a trim and push into the token property solution.
What's the thoughts here? The documentation is solving this really (so agents should be satisfied) and this is really just protecting a user from doing something incorrect. Do I go further and provide valid authorisation header setter validation also? Or block authorisation from being set as a header at all?
There was a problem hiding this comment.
We could add the property representing the authorization header as bearer scheme, it would be another path to interact with the dictionary, but we could make that clear in the docs.
public string? BearerToken
{
get => Headers?["Authorization"] is string token && token.StartsWith("Bearer ", StringComparison.OrdinalIgnoreCase) ? token : null;
set
{
var headers = (Headers ??= new Dictionary<string, string>());
headers["Authorization"] = $"Bearer {value}"
}
}Or we could add a method GetAuthorizationToken(AuthScheme schene) in case bearer gets relegated by another scheme.
I think we could add this later, hence the "must".
There was a problem hiding this comment.
Except that if we want to have such a property, we could just keep the public API that's there and avoid the breaking change / churn.
Let's design the final end-to-end that we really want here, factoring in everything we know about all implementers and what consumers are going go to want to do. We're at this point with these APIs where we need to stop churning them, so we want to get this next round right.
There was a problem hiding this comment.
This would keep the headers as the source of truth while making it easier to implementors to map it to providers.
There was a problem hiding this comment.
There's a problem with this kind of set up, though... it creates an ordering issue between properties, e.g.
new HostedMcpServerTool(...)
{
AuthorizationToken = ...,
Headers = ...,
}has very different behavior from:
new HostedMcpServerTool(...)
{
Headers = ...,
AuthorizationToken = ...,
}There are various ways to address that, such as by making at least one of them non-settable, e.g. if the Headers property returns a non-null mutable collection and only has a getter. That of course can slightly complicate other use cases, though, so the tradeoffs need to be evaluated.
Otherwise, having AuthorizationToken as an accelerator into Headers makes a lot of sense. We still need to evaluate what each implementer would do with the data, though.
There was a problem hiding this comment.
There's a problem with this kind of set up, though... it creates an ordering issue between properties ... if the Headers property returns a non-null mutable collection and only has a getter. That of course can slightly complicate other use cases, though, so the tradeoffs need to be evaluated.
Great catch and great suggestion. Headers as get-only allows this to work and provide better usability; the token as get-only will just force folks to walk the long path.
As discussed offline, this can encumber cases that want to [re]use an existing dictionary as the Headers, forcing them to manually iterate to copy the elements. That seems to me like an acceptable trade-off.
We still need to evaluate what each implementer would do with the data, though
If headers are supported, implementors should always send the headers, if not (Claude and OpenAI with Connector), send the token and ignore all other headers.
src/Libraries/Microsoft.Extensions.AI.Abstractions/Tools/HostedMcpServerTool.cs
Outdated
Show resolved
Hide resolved
| mcpTool.AuthorizationToken, | ||
| mcpTool.ServerDescription); | ||
| mcpTool.ServerDescription, | ||
| mcpTool.Headers); |
There was a problem hiding this comment.
We should be validating these with unit tests, ala
.There was a problem hiding this comment.
Will do, but will do once I know which direction we want to take the model.
Updated the XML documentation for the HTTP headers property to clarify that these headers are included when calling the remote MCP server.
…manFromBunnings/extensions into AddHeaderToMcpToolCreation
jozkee
added
the
* NO MERGE *
4 participants
Introduces a Headers property to HostedMcpServerTool for specifying additional request headers. Updates OpenAIResponsesChatClient to pass these headers when creating MCP tools and adds unit tests to verify header roundtripping.
Microsoft Reviewers: Open in CodeFlow