Add ToolApprovalRequestContent.RequiresConfirmation#7549
Open
javiercn wants to merge 10 commits into
Open
Conversation
Collaborator
🎉 Good job! The coverage increased 🎉
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1456605&view=codecoverage-tab |
jozkee
reviewed
Jun 10, 2026
javiercn
added a commit
that referenced
this pull request
Jun 10, 2026
Renames the property added in #7549 to use Abstractions-neutral vocabulary (per @jozkee feedback on #7550) and flips the polarity so the default (true) is correct for any producer that constructs a ToolApprovalRequestContent because the underlying tool genuinely requires approval. FICC now explicitly sets RequiresConfirmation = false for requests it wraps as a no-op because a peer in the same response requires approval. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
javiercn
added a commit
that referenced
this pull request
Jun 10, 2026
1. Drop [Experimental(MEAI001)] from RequiresConfirmation; the rest of the approval surface is no longer experimental. 2. Streaming TryReplaceFunctionCallsWithApprovalRequests now takes the pre-computed approvalRequiredFunctions array instead of the raw tool lists, avoiding a second FindTool + GetService scan per FCC. Non-streaming path is unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
🎉 Good job! The coverage increased 🎉
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1457358&view=codecoverage-tab |
javiercn
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the AI abstractions approval envelope (ToolApprovalRequestContent) with a RequiresConfirmation boolean so downstream consumers can distinguish between approvals that are truly required by the underlying tool vs. “collateral” approval requests introduced by FunctionInvokingChatClient to keep mixed batches coherent.
Changes:
- Add
ToolApprovalRequestContent.RequiresConfirmation(defaulttrue) and include it in JSON round-tripping. - Populate
RequiresConfirmationinFunctionInvokingChatClientfor both non-streaming and streaming wrapping paths. - Update test helpers and add/adjust tests to assert the new per-call flag behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/ToolApprovalRequestContent.cs |
Adds the new RequiresConfirmation property and documentation. |
src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs |
Sets RequiresConfirmation when wrapping FCCs into approval requests (streaming + non-streaming). |
src/Libraries/Microsoft.Extensions.AI.Abstractions/Microsoft.Extensions.AI.Abstractions.json |
Updates API baseline to include the new property. |
test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/Contents/ToolApprovalRequestContentTests.cs |
Adds unit tests for default value and JSON round-trip. |
test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/AssertExtensions.cs |
Extends message equality assertions to include RequiresConfirmation. |
test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientApprovalsTests.cs |
Updates/extends FICC approval tests to validate per-call RequiresConfirmation. |
Collaborator
🎉 Good job! The coverage increased 🎉
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1457523&view=codecoverage-tab |
Collaborator
🎉 Good job! The coverage increased 🎉
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1457721&view=codecoverage-tab |
Lets consumers distinguish approval requests raised because the underlying function is an ApprovalRequiredAIFunction (IsInvokerRequested = false, the default) from requests that FunctionInvokingChatClient added as collateral because a peer call in the same batch required approval (IsInvokerRequested = true). Both FICC wrap sites set the flag using the existing FindTool + GetService<ApprovalRequiredAIFunction>() pattern so additional DelegatingAIFunction wrappers still classify correctly. The streaming helper now takes the tool lists symmetrically with the non-streaming path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
STJ source generator emits read/write code for ToolApprovalRequestContent.IsInvokerRequested, which is [Experimental(MEAI001)]. The generated code can't carry pragma suppressions, so suppress at the project level. Matches the existing pattern used by Microsoft.Extensions.AI.Abstractions and Microsoft.Extensions.AI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Renames the property added in #7549 to use Abstractions-neutral vocabulary (per @jozkee feedback on #7550) and flips the polarity so the default (true) is correct for any producer that constructs a ToolApprovalRequestContent because the underlying tool genuinely requires approval. FICC now explicitly sets RequiresConfirmation = false for requests it wraps as a no-op because a peer in the same response requires approval. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1. Drop [Experimental(MEAI001)] from RequiresConfirmation; the rest of the approval surface is no longer experimental. 2. Streaming TryReplaceFunctionCallsWithApprovalRequests now takes the pre-computed approvalRequiredFunctions array instead of the raw tool lists, avoiding a second FindTool + GetService scan per FCC. Non-streaming path is unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Now that RequiresConfirmation no longer carries [Experimental(MEAI001)], the STJ source-generated JsonContext for ToolApprovalRequestContent does not touch any experimental member, so the suppression added in 736f112 is no longer needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Frames the property in consumer terms and removes invoker/wrap-site detail that does not belong in the Abstractions surface. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Non-streaming wrap site now takes the precomputed approvalRequiredFunctions array and uses a name-set check, matching what the streaming wrap site does. Removes the small streaming/non-streaming divergence in RequiresConfirmation classification introduced earlier in this PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Spell out that a ToolApprovalResponseContent is still required even when RequiresConfirmation is false, so the underlying tool call can be invoked. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
RequiresConfirmation is again [Experimental(MEAI001)] after 566be62, so the STJ source-generated JsonContext for ToolApprovalRequestContent reaches an experimental member and the Reporting projects fail to build at Release without the suppression. Restores the same NoWarn that was in 736f112. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
javiercn
force-pushed
the
javiercn/invokerrequested
branch
from
bfb8335 to
2f56556
Compare
Collaborator
🎉 Good job! The coverage increased 🎉
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1458062&view=codecoverage-tab |
shyamnamboodiripad
approved these changes
Jun 10, 2026
Member
Author
|
@jozkee It's been a while since I have done within extensions. What's the process. Do we merge now? Or do we want to go through API Review first or discuss it any further? Is there anyone else that needs to have a look? |
peterwald
approved these changes
Jun 10, 2026
Member
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements #7550.
Adds
ToolApprovalRequestContent.RequiresConfirmationso consumers can distinguish approval requests where the underlying function genuinely needs approval (RequiresConfirmation = true, the default) from requests thatFunctionInvokingChatClientadded as a no-op because a peer call in the same response requires approval (RequiresConfirmation = false).See #7550 for the full proposal (summary, motivation, goals, non-goals, detailed design). The name is still being iterated on in #7550; the property name in this PR may change to the agreed-upon name before merge.
Changes
Microsoft.Extensions.AI.Abstractions— newbool ToolApprovalRequestContent.RequiresConfirmation { get; set; } = true;, serialized likeFunctionCallContent.InformationalOnly. Defaulttruemeans any non-FICC producer that constructs aToolApprovalRequestContentbecause the underlying tool genuinely requires approval is correct without code changes.FunctionInvokingChatClient— both wrap sites now set the flag per call.ReplaceFunctionCallsWithApprovalRequestsusesFindTool+GetService<ApprovalRequiredAIFunction>()per FCC; the lookup is captured in the existing index list so the wrap site setsRequiresConfirmationwithout a second pass.TryReplaceFunctionCallsWithApprovalRequestsnow takes the pre-computedapprovalRequiredFunctionsarray (already computed at the caller for batch buffering) and matches by name, avoiding a secondFindTool+GetServicescan per FCC.Microsoft.Extensions.AI.Abstractions.jsonupdated.Tests
AssertExtensions.EqualMessageListsand the test-localCloneFccpropagate and assert the new flag.DelegatingAIFunction; (b) FCC whose name doesn't match any tool when a peer requires approval.ToolApprovalRequestContent: default value, JSON round-trip.40 approval tests, 161 FICC tests, and 21
ToolApprovalRequestContentunit tests pass on net9.0.