Skip to content

Conversation

@echapmanFromBunnings
Copy link

@echapmanFromBunnings echapmanFromBunnings commented Nov 17, 2025

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

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.
Copilot AI review requested due to automatic review settings November 17, 2025 14:09
@echapmanFromBunnings echapmanFromBunnings requested a review from a team as a code owner November 17, 2025 14:09
@github-actions github-actions bot added the area-ai Microsoft.Extensions.AI libraries label Nov 17, 2025
Copilot finished reviewing on behalf of echapmanFromBunnings November 17, 2025 14:12
Copy link
Contributor

Copilot AI left a 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 Headers property to HostedMcpServerTool for storing custom header key-value pairs
  • Updates OpenAIResponsesChatClient to 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_PropsDefault test is missing an assertion for the new Headers property. Add Assert.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.
@echapmanFromBunnings echapmanFromBunnings requested review from a team as code owners November 17, 2025 15:16
@echapmanFromBunnings
Copy link
Author

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 🙇

@stephentoub
Copy link
Member

cc: @jozkee

@@ -0,0 +1,145 @@
{
"Name": "Microsoft.Extensions.AI.Evaluation.NLP, Version=10.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35",
Copy link
Contributor

@shyamnamboodiripad shyamnamboodiripad Nov 17, 2025

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",
Copy link
Contributor

@shyamnamboodiripad shyamnamboodiripad Nov 17, 2025

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",
Copy link
Contributor

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);",
Copy link
Contributor

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.

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",
Copy link
Contributor

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",
Copy link
Contributor

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)",
Copy link
Contributor

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.
@echapmanFromBunnings
Copy link
Author

Bringing this back, the core issue is the below PR which has updated all of the assemblies.

#7047

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",

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-ai Microsoft.Extensions.AI libraries

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants