Add support for Connector ID and other follow ups #6881

jozkee · 2025-10-02T15:31:12Z

Contributes to #6492.

Add support for connector ID.
Add AuthorizationToken to HostedMcpServerTool since its now promoted in both OpenAI and Anthropic.
Relax McpServerToolCallContent ToolName and ServerName
Messages with user role can also create approval responses.

stephentoub · 2025-10-02T15:40:51Z

src/Libraries/Microsoft.Extensions.AI.Abstractions/Tools/HostedMcpServerTool.cs

+    {
+        ServerName = Throw.IfNullOrWhitespace(serverName);
+        ConnectorID = Throw.IfNullOrWhitespace(connectorID);
+        Url = url; // needed for disambiguation.


Connector ID seems very specific to OpenAI?

With their connectors, do you specify both a name and a URL? I thought they'd be mutually exclusive, in which case I was expecting we'd just have a single "location" (pick a better word) that's either a url or an identifier. That also means it doesn't make it specific to OpenAI.

I thought they'd be mutually exclusive,

They are mutually exclusive. This argument is just here for disambiguation because there's already a .ctor(string, string).

we'd just have a single "location" (pick a better word) that's either a url or an identifier

Do you mean having a type like struct ServerLocation with mutually exclusive properties Url and ConnectorID(for the lack of a better name)? If so, that's sounds fine too, just wanted to make sure that we don't want to follow the simple route of having a factory method with a string connectorId

I meant just having it be a string, and having the implementation decide whether it's a name or a url, e.g. does it start with a schema. Would that work? Is it problematic?

They are mutually exclusive. This argument is just here for disambiguation because there's already a .ctor(string, string).

We should not do that. Having a constructor that accepts both makes it look like both can be provided and utilized.

I meant just having it be a string, and having the implementation decide whether it's a name or a url

Ah so you mean updating the existing .ctor(string, string url) and rename the param to urlOrConnectorId and then have something like

if (Uri.TryCreate(urlOrConnectorId, UriKind.Absolute, out url)) Url = url else ConnectorID = urlOrConnectorId

My only concern with that is that we will be overloading the argument.

We should not do that. Having a constructor that accepts both makes it look like both can be provided and utilized.

I thought it was fine since it was private, but OK, will take note.

Ah so you mean updating the existing .ctor(string, string url) and rename the param

Yes, with some name that conveys "this is the thing to connect to, which could be a url or could be a descriptor that the backend service understands"

I thought it was fine since it was private

Ah, I didn't see that it was private.

Do other services that support remote MCP, like Anthropic, have such a concept yet?

like Anthropic, have such a concept yet?

No, I know anthropic doesn't from looking at their docs. I'm not aware of other providers that support remote MCP.

src/Libraries/Microsoft.Extensions.AI.Abstractions/Tools/HostedMcpServerTool.cs

* Add AuthorizationToken property since it is now promoted in both OpenAI and Anthropic * Relax McpServerToolCallContent ToolName and ServerName

…ector_id

Copilot

Pull Request Overview

This PR adds support for connector ID in MCP (Model Context Protocol) tools and implements several related improvements. The changes enable the use of connector identifiers instead of just URLs for MCP servers, add OAuth authorization support, and relax validation requirements for tool properties.

Add connector ID support to HostedMcpServerTool allowing string addresses that can be either URLs or connector IDs
Add AuthorizationToken property to HostedMcpServerTool for OAuth authentication with MCP servers
Relax validation in McpServerToolCallContent by making ToolName and ServerName optional properties
Enable user role messages to create approval responses for MCP tool calls

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
`src/Libraries/Microsoft.Extensions.AI.Abstractions/Tools/HostedMcpServerTool.cs`	Updated constructor to accept string addresses, added AuthorizationToken property, simplified API
`src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/McpServerToolCallContent.cs`	Made ToolName and ServerName optional settable properties, simplified constructor validation
`src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIResponsesChatClient.cs`	Updated to handle connector IDs vs URLs, support authorization tokens, and allow user role approval responses
`test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Tools/HostedMcpServerToolTests.cs`	Updated tests to reflect new constructor signature and added AuthorizationToken property tests
`test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Contents/McpServerToolCallContentTests.cs`	Updated tests for simplified constructor and optional properties
`test/Libraries/Microsoft.Extensions.AI.OpenAI.Tests/OpenAIResponseClientTests.cs`	Added comprehensive test for MCP tool approval workflow with both user and tool roles
`test/Libraries/Microsoft.Extensions.AI.OpenAI.Tests/OpenAIResponseClientIntegrationTests.cs`	Added integration test for connector-based MCP servers with authorization

stephentoub · 2025-10-09T20:48:31Z

src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/McpServerToolCallContent.cs

+    /// Gets or sets the name of the tool called.
    /// </summary>
-    public string ToolName { get; }
+    public string? ToolName { get; set; }


I'm still not convinced this makes sense. A call without any information about what's being called is useless. ToolName is what's being called. In contrast, ServerName is valuable as a disambiguator, but in general it's just informative.

If you really believe it's all or nothing, that both ToolName and ServerName need to be required or neither need to be required, then we should just leave it as is rather than have neither be required.

stephentoub · 2025-10-09T20:49:46Z

src/Libraries/Microsoft.Extensions.AI.Abstractions/Tools/HostedMcpServerTool.cs

-    /// <exception cref="ArgumentNullException"><paramref name="serverName"/> or <paramref name="url"/> is <see langword="null"/>.</exception>
-    /// <exception cref="ArgumentException"><paramref name="serverName"/> is empty or composed entirely of whitespace.</exception>
-    public HostedMcpServerTool(string serverName, Uri url)
+    /// <param name="serverAddress">The address of the remote MCP server.</param>


Suggested change

/// <param name="serverAddress">The address of the remote MCP server.</param>

/// <param name="serverAddress">The address of the remote MCP server. This may be a URL, or in the case of a service providing built-in MCP servers with known names, it can be such a name.</param>

stephentoub · 2025-10-09T20:50:11Z

src/Libraries/Microsoft.Extensions.AI.Abstractions/Tools/HostedMcpServerTool.cs

+    /// Gets or sets the OAuth authorization token that the AI service should use when calling the remote MCP server.
    /// </summary>
-    public Uri Url { get; }
+    public string? AuthorizationToken { get; set; }


What is the relationship between this and Headers?

I expect that this should serve the same purpose as Headers.Add("Authorization", "<token>"). However, I tested with the connector and endpoint returns Cannot specify 'headers' parameter with 'connector_id'.. A bit unfortunate that OpenAI's ResponseTool.CreateMcpTool accepts them.

We need to choose the right shared representation, and then each implementation maps that apppropriately. I don't understand the argument for having both, if auth is representable as a header. You couldn't extract the auth token from the headers collection of just that collection existed, removing it from the collection passed to the underlying impl?

Is the idea that you always use this property for an auth token and the headers collection for everything else?

Is the idea that you always use this property for an auth token and the headers collection for everything else?

Yes. OpenAI started with only Headers and now it promoted the auth header to be its own field in the payload.

I think 99% use cases will be for the authorization header. Having only Headers is more future proof but is less straightforward. Anthropic also only accepts the auth_token. It sucks to not have a crystal ball.

We can switch to only have AuthorizationToken. If someone wants to pass headers in OpenAI they can pass the raw tool. And if Headers becomes a thing in the future because more providers start adding them, we can bring it back. Wdyt?

We can switch to only have AuthorizationToken. [...] And if Headers becomes a thing in the future because more providers start adding them, we can bring it back. Wdyt?

Do we have any examples where headers are required? @halter73, any opinions?

Otherwise, sounds good.

stephentoub · 2025-10-09T21:23:28Z

src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIResponsesChatClient.cs

+                    {
+                        yield return ResponseItem.CreateMcpApprovalResponseItem(mcpApprovalResponseContent.Id, mcpApprovalResponseContent.Approved);
+                    }
+                }


Were MCP approvals just broken before?

Oh, I see, this is being done to accomodate the approval being in a user message in addition to being in a tools message, where it's currently handled?

For ChatRole.User, yes. This change is in response to your point in #6779.

The OpenAI Responses implementation currently looks for the tool approval as part of a Tool message. Is that right? Should it be User instead? Should it be any kind of message?

After looking into what kind of messages the other roles process, I think it doesn't make sense to handle mcp approvals for system/developer/assistant. However, worth mentioning that I think Function Approvals are handled for any role.

to accomodate the approval being in a user message in addition to being in a tools message, where it's currently handled?

Yes.

However, worth mentioning that I think Function Approvals are handled for any role.

Where? I see handling for it under case ChatRole.Tool.

In FunctionInvokingChatClient, I don't think there's anything preventing you from passing the FunctionApprovalResponseContent in a message with any role, not just user/tool.

Ah, I see. @westey-m, is that desirable?

I typically prefer to be stricter, since it helps to catch bugs. E.g. passing FARC in anything but a user message sounds like a bug to me. It is easy to relax the restriction later if we find a real scenario for it.
So I think tightening this in FICC as well makes sense.

stephentoub · 2025-10-09T21:26:47Z

src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIResponsesChatClient.cs

            {
                yield return ResponseItem.CreateUserMessageItem(ToResponseContentParts(input.Contents));
+
+                foreach (AIContent item in input.Contents)


If the user message exists just to carry MCP approval, won't the above end up producing a separate empty user message item (with a dummy content part)?

Seems like it'd be better to restructure this, where this becomes:
yield return CreateUserResponseItems()
(or just do it inline as it's done for other kinds), iterating over the contents once rather than twice, and yielding the appropriate response items.

Yes, it does create that dummy message you describe, I left it like that because that's the case for any other kind of content that doesn't map to a ResponseContentPart but it does seem like something that should be fixed. Will do.

that's the case for any other kind of content that doesn't map to a ResponseContentPart

Not exactly. Someone would need to create an empty ChatMessage to hit that. With this, they've created a ChatMessage that contains just the McpApproval, which would be a very common thing to do (just as it is for creating tool messages just with function approval), yet we're then treating it as an empty message.

Add support for Connector ID

19077d8

github-actions bot added the area-ai Microsoft.Extensions.AI libraries label Oct 2, 2025

dotnet-policy-service bot assigned jozkee Oct 2, 2025

stephentoub reviewed Oct 2, 2025

View reviewed changes

jozkee commented Oct 8, 2025

View reviewed changes

src/Libraries/Microsoft.Extensions.AI.Abstractions/Tools/HostedMcpServerTool.cs Outdated Show resolved Hide resolved

jozkee added 3 commits October 8, 2025 11:50

* Convert Url to string ServerAddress capable of carrying connector ids

a9d0a70

* Add AuthorizationToken property since it is now promoted in both OpenAI and Anthropic * Relax McpServerToolCallContent ToolName and ServerName

Create MCP approval responses also with user chat role

4a0394d

Merge branch 'main' of https://github.com/dotnet/extensions into conn…

20e0592

…ector_id

jozkee changed the title ~~Add support for Connector ID~~ Add support for Connector ID and other follow ups Oct 8, 2025

jozkee marked this pull request as ready for review October 8, 2025 23:20

jozkee requested a review from a team as a code owner October 8, 2025 23:20

Copilot AI review requested due to automatic review settings October 8, 2025 23:20

Copilot AI reviewed Oct 8, 2025

View reviewed changes

stephentoub reviewed Oct 9, 2025

View reviewed changes

	/// <param name="serverAddress">The address of the remote MCP server.</param>
	/// <param name="serverAddress">The address of the remote MCP server. This may be a URL, or in the case of a service providing built-in MCP servers with known names, it can be such a name.</param>

Add support for Connector ID and other follow ups #6881

Are you sure you want to change the base?

Add support for Connector ID and other follow ups #6881

Uh oh!

Conversation

jozkee commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

stephentoub Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stephentoub Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jozkee commented Oct 2, 2025 •

edited

Loading

stephentoub Oct 9, 2025 •

edited

Loading

stephentoub Oct 9, 2025 •

edited

Loading