-
Notifications
You must be signed in to change notification settings - Fork 844
Add support for custom headers in HostedMcpServerTool #7049
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add support for custom headers in HostedMcpServerTool #7049
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for custom HTTP headers in MCP server tool configurations. The Headers property allows specifying additional request headers when communicating with remote MCP servers.
Key changes:
- Adds a
Headersproperty toHostedMcpServerToolfor storing custom header key-value pairs - Updates
OpenAIResponsesChatClientto pass headers when creating MCP tools via both URI-based and connector-based paths - Adds unit tests to verify headers can be set, retrieved, and cleared
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/Libraries/Microsoft.Extensions.AI.Abstractions/Tools/HostedMcpServerTool.cs |
Adds the Headers property with XML documentation |
src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIResponsesChatClient.cs |
Passes mcpTool.Headers to both ResponseTool.CreateMcpTool overloads |
test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Tools/HostedMcpServerToolTests.cs |
Adds test assertions for roundtripping the Headers property |
Comments suppressed due to low confidence (1)
test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Tools/HostedMcpServerToolTests.cs:26
- The
Constructor_PropsDefaulttest is missing an assertion for the newHeadersproperty. AddAssert.Null(tool.Headers);after line 26 to ensure the default state of the Headers property is properly tested, consistent with the pattern used for other nullable properties.
Assert.Empty(tool.Description);
Assert.Null(tool.AuthorizationToken);
Assert.Null(tool.ServerDescription);
Assert.Null(tool.AllowedTools);
Assert.Null(tool.ApprovalMode);
Updated MakeApiBaselines.ps1 and all API baseline JSONs to target .NET 10.0.0.0. Added new API baseline files for Microsoft.Extensions.AI.AzureAIInference, Evaluation.NLP, Evaluation.Safety, AmbientMetadata.Build, DataIngestion, DataIngestion.Abstractions, DataIngestion.MarkItDown, and DataIngestion.Markdig. The Microsoft.Extensions.AI.Abstractions API baseline includes significant additions and changes for experimental AI features, image generation, tool approval, and continuation tokens.
|
I've added the baselines, but it looks like its not been done since the move to 10, nor does it look like a minor PR change now... Sorry guys, lmk if this needs reverting 🙇 |
|
cc: @jozkee |
| @@ -0,0 +1,145 @@ | |||
| { | |||
| "Name": "Microsoft.Extensions.AI.Evaluation.NLP, Version=10.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35", | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The NLP package is still in preview and should not have the API json turned on. Could you please revert this?
| // After generating the baseline, manually edit this file to remove primary constructor portion | ||
| // This is needed until ICSharpCode.Decompiler adds support for primary constructors | ||
| // See: https://github.com/icsharpcode/ILSpy/issues/829 | ||
| "Type": "sealed class Microsoft.Extensions.AI.Evaluation.Quality.CompletenessEvaluatorContext : Microsoft.Extensions.AI.Evaluation.EvaluationContext", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove the primary constructor syntax per instructions in the comment. Please also preserve the comments as they can be helpful for future updates. Thanks!
| }, | ||
| { | ||
| "Type": "sealed class Microsoft.Extensions.AI.Evaluation.Quality.IntentResolutionEvaluator : Microsoft.Extensions.AI.Evaluation.IEvaluator", | ||
| "Stage": "Experimental", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 3 experimental evaluators (IntentResolution, TaskAdherence, ToolCallAccuracy) were not added to this file because they are still considered in preview.. Can we please revert the corresponding changes?
| }, | ||
| { | ||
| "Member": "Microsoft.Extensions.AI.Evaluation.Quality.RetrievalEvaluatorContext.RetrievalEvaluatorContext(params string[] retrievedContextChunks);", | ||
| "Member": "Microsoft.Extensions.AI.Evaluation.Quality.RetrievalEvaluatorContext.RetrievalEvaluatorContext(System.Collections.Generic.IEnumerable<string> retrievedContextChunks);", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a bug in the tool and should probably be switched back to the params syntax.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its there, that is member, not the type. There is a params method, and an enumerable method in that array.
| // This is needed until ICSharpCode.Decompiler adds support for primary constructors | ||
| // See: https://github.com/icsharpcode/ILSpy/issues/829 | ||
| "Type": "sealed class Microsoft.Extensions.AI.Evaluation.Reporting.Storage.AzureStorageResultStore : Microsoft.Extensions.AI.Evaluation.Reporting.IEvaluationResultStore", | ||
| "Type": "sealed class Microsoft.Extensions.AI.Evaluation.Reporting.Storage.AzureStorageResultStore(Azure.Storage.Files.DataLake.DataLakeDirectoryClient client) : Microsoft.Extensions.AI.Evaluation.Reporting.IEvaluationResultStore", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned in a comment for a different file, please remove the primary constructor syntax (and preserve the comments). Thanks!
| @@ -0,0 +1,329 @@ | |||
| { | |||
| "Name": "Microsoft.Extensions.AI.Evaluation.Safety, Version=10.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35", | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Safety package is still in preview and should not have the API json turned on. Could you please revert this file?
| // This is needed until ICSharpCode.Decompiler adds support for primary constructors | ||
| // See: https://github.com/icsharpcode/ILSpy/issues/829 | ||
| "Type": "sealed class Microsoft.Extensions.AI.Evaluation.ChatConfiguration", | ||
| "Type": "sealed class Microsoft.Extensions.AI.Evaluation.ChatConfiguration(Microsoft.Extensions.AI.IChatClient chatClient)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above regarding primary constructor syntax and preserving the comments around how to fix.
Deleted Microsoft.Extensions.AI.Evaluation.NLP.json baseline. Updated .json API files for Quality, Reporting.Azure, and Evaluation libraries to remove primary constructor portions and add manual edit comments, addressing ICSharpCode.Decompiler limitations (see ILSpy issue dotnet#829).
Deleted Microsoft.Extensions.AI.Evaluation.Safety.json.
Deleted IntentResolutionEvaluator, TaskAdherenceEvaluator, ToolCallAccuracyEvaluator and their context classes from Microsoft.Extensions.AI.Evaluation.Quality.json. This streamlines the API surface by removing experimental features.
Changed the 'Stage' of GetCacheKey methods in CachingChatClient, CachingEmbeddingGenerator, DistributedCachingChatClient, and DistributedCachingEmbeddingGenerator from 'Stable' to 'Experimental' to reflect their current API status.
|
Bringing this back, the core issue is the below PR which has updated all of the assemblies. |
Added the [Experimental("MEAI001")] attribute to GetCacheKey methods in caching and distributed caching classes for chat and embedding clients. This clarifies that cache key generation is not guaranteed to be stable across releases.
The FakeLogger class no longer implements the IBufferedLogger interface. This change updates the type definition in the JSON metadata to reflect the current class inheritance.
| }, | ||
| { | ||
| "Type": "class Microsoft.Extensions.Logging.Testing.FakeLogger : Microsoft.Extensions.Logging.ILogger, Microsoft.Extensions.Logging.Abstractions.IBufferedLogger", | ||
| "Type": "class Microsoft.Extensions.Logging.Testing.FakeLogger : Microsoft.Extensions.Logging.ILogger", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need someone to review this in particular. It seems that the baseline has this in, but we're targeting assemblies that do not have access to this interface?
Changed the 'Stage' of the Snapshot property in the JSON metadata from 'Experimental' to 'Obsolete' and removed the Experimental attribute from the Snapshot property in ResourceUtilization.cs. This reflects the deprecation of the Snapshot property and the obseletion of the class.
The [Experimental] attribute was removed from ISnapshotProvider and Snapshot classes, leaving only the [Obsolete] attribute. This clarifies the API status and reduces attribute clutter.
Changed the stage of ISnapshotProvider and Snapshot types and their members from 'Experimental' to 'Obsolete' in the resource monitoring JSON manifest. Also removed unused usings from ISnapshotProvider.cs.
Added baseline suppressions for CP0002 diagnostics on new .NET 10.0 API members in Microsoft.Extensions.AI.Abstractions, including properties and methods for ChatOptions, ChatResponse, ChatResponseUpdate, and HostedMcpServerTool.
Added 'using System' and 'using Microsoft.Shared.DiagnosticIds' to ISnapshotProvider.cs to ensure required namespaces are available.
…nnings/extensions into headersOnMcpTools
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