-
Notifications
You must be signed in to change notification settings - Fork 849
Replace Mcp approvals with Function approvals #7104
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?
Conversation
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 refactors the approval mechanism for tool calls by replacing MCP-specific approval types (McpServerToolApprovalRequestContent and McpServerToolApprovalResponseContent) with generalized FunctionApprovalRequestContent and FunctionApprovalResponseContent types. This allows the function approval system to handle both regular function calls and MCP server tool calls uniformly.
Key Changes:
- Deleted MCP-specific approval content types and consolidated them into the existing Function approval types
- Changed
FunctionApprovalRequestContentandFunctionApprovalResponseContentto acceptAIContentinstead ofFunctionCallContent, enabling support for multiple call types (e.g.,FunctionCallContent,McpServerToolCallContent) - Updated property names from
FunctionCalltoCallContentto reflect the more general nature - Added pattern matching logic in
FunctionInvokingChatClientto handle mixed approval scenarios where onlyFunctionCallContentapprovals are processed - Updated OpenAI response client to map MCP approval items to the generalized approval types
- Added comprehensive tests for mixed approval scenarios
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/McpServerToolApprovalRequestContent.cs |
Deleted - functionality replaced by generalized FunctionApprovalRequestContent |
src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/McpServerToolApprovalResponseContent.cs |
Deleted - functionality replaced by generalized FunctionApprovalResponseContent |
src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/FunctionApprovalRequestContent.cs |
Changed to accept AIContent instead of FunctionCallContent; renamed property FunctionCall → CallContent |
src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/FunctionApprovalResponseContent.cs |
Changed to accept AIContent instead of FunctionCallContent; renamed property FunctionCall → CallContent |
src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/UserInputRequestContent.cs |
Removed McpServerToolApprovalRequestContent from JSON polymorphic type hierarchy |
src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/UserInputResponseContent.cs |
Removed McpServerToolApprovalResponseContent from JSON polymorphic type hierarchy |
src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/AIContent.cs |
Removed commented-out type discriminators for deleted MCP approval types |
src/Libraries/Microsoft.Extensions.AI.Abstractions/Utilities/AIJsonUtilities.Defaults.cs |
Removed JSON serialization registration for deleted MCP approval types |
src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs |
Added pattern matching to process only FunctionCallContent from approval types; updated struct to be readonly with init properties; fixed spelling error |
src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIResponsesChatClient.cs |
Updated to map MCP approval items to generalized FunctionApprovalRequestContent and FunctionApprovalResponseContent with McpServerToolCallContent |
test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientApprovalsTests.cs |
Changed namespace; updated assertions to use CallContent property; added comprehensive tests for mixed approval scenarios |
test/Libraries/Microsoft.Extensions.AI.OpenAI.Tests/OpenAIResponseClientTests.cs |
Updated test assertions to use generalized approval types |
test/Libraries/Microsoft.Extensions.AI.OpenAI.Tests/OpenAIResponseClientIntegrationTests.cs |
Updated test assertions and approval response creation logic; reordered test execution calls |
test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Contents/FunctionApprovalRequestContentTests.cs |
Updated tests to use CallContent property and added type assertions for deserialized content |
test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Contents/FunctionApprovalResponseContentTests.cs |
Updated tests to use CallContent property and added type assertions for deserialized content |
test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Contents/UserInputRequestContentTests.cs |
Commented out MCP-specific test case; updated deserialization type |
test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Contents/UserInputResponseContentTests.cs |
Commented out MCP-specific test case with note about experimental status |
test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Contents/AIContentTests.cs |
Updated test to use generalized approval types with MCP content |
Strawman proposal for replacing
McpServerToolApprovalRequestContentandMcpServerToolApprovalResponseContentwith Function approval types.#6492 (comment)
cc @westey-m
Microsoft Reviewers: Open in CodeFlow