.NET: Add pending tool approval closure semantics (#7872) - #7909
.NET: Add pending tool approval closure semantics (#7872)#7909Shivani . (Shivani767) wants to merge 2 commits into
Conversation
Keep pending approval bag entries across unrelated user turns, and expose enumerate/reject/clear helpers so hosts can drain restored sessions before the next normal message (microsoft#7872).
There was a problem hiding this comment.
Pull request overview
Adds durable pending tool-approval discovery and explicit closure APIs while preserving pending approvals across unrelated turns.
Changes:
- Preserves pending approvals until matching responses arrive.
- Adds APIs to enumerate, reject, or clear pending approvals.
- Adds lifecycle and serialization tests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
ApprovalResponseBindingChatClient.cs |
Retains and selectively consumes pending approvals. |
ToolApprovalAgentSessionExtensions.cs |
Adds public pending-approval session helpers. |
ApprovalResponseBindingChatClientTests.cs |
Tests preservation, restoration, rejection, and clearing. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Return cloned FunctionCallContent arguments from TryGet, and leave pending entries in place until CreatePendingApprovalRejections responses are bound so restore-path rejections are not dropped as unbound.
|
Thanks for the update. I want to double-check one remaining case from #7872. This PR appears to address pending approvals, but does it also handle ordinary non-approval tool calls that become My concern is that a cancelled run can still leave unresolved FunctionCallContent for normal tools, and the next turn Is that scenario expected to be fixed in this PR, or should #7872 remain open / be tracked by a follow-up issue? |
|
1448376744 (@soul-soft) Thanks for raising this. You’re right to distinguish the two cases. My understanding of this PR is that it specifically addresses the lifecycle of pending tool approvals—preserving the approval binding across turns and providing explicit APIs to reject or clear those pending approvals. The case where a normal (non-approval) tool call becomes dangling because I’ll take a closer look at the cancellation path to confirm whether there is any existing cleanup that covers this case. If it isn’t covered, I agree that #7872 should either remain open or have a follow-up issue tracking the non-approval dangling |
|
Thanks for the quick clarification. That answers my concern. I agree that the closure behavior for cancelled normal tool calls should be handled separately from the pending approval lifecycle covered by this PR. For the non-approval cancellation case, my current thinking is that the more natural place to handle the cancellation event is at the So I agree with keeping this PR focused on pending approvals, and tracking the non-approval dangling |
Motivation & Context
When a run is cancelled, refreshed, or otherwise interrupted after the model emits a tool call (or after
ToolApprovalRequestContentis surfaced), the session can retain pending approval state with no public, reliable way for the host to enumerate or close it. Separately,ApprovalResponseBindingChatClientcleared the pending bag on any subsequent turn—even an unrelated user message with no approval response—so a restored session could lose binding authority while a danglingFunctionCallContentremained in history (#7862 / #7872).This PR addresses the triage-sized slice of #7872: stop the silent consume bug and give hosts a public drain/closure path. Moving the FICC loop out of
Microsoft.Extensions.AIand adding a configurable auto-drain policy are left for follow-up.Assigned on the issue to Roger Barreto (@rogerbarreto) — happy to coordinate or close if a maintainer PR supersedes this.
Description & Review Guide
What are the major changes?
ValidateInboundApprovalResponsesonly consumes bag entries when a matchingToolApprovalResponseContentis bound in the current turn; an ordinary user follow-up leaves pending state intact.ToolApprovalAgentSessionExtensions:TryGetPendingToolApprovalRequests— enumerate pending approvals (including after session restore).CreatePendingApprovalRejections— drain as explicit rejections the host can send on the next run so FICC can emit terminalFunctionResultContent.ClearPendingToolApprovalRequests— drop bag entries without producing responses (documented as the weaker option).What is the impact of these changes?
FunctionCallContentclosure in FICC.What do you want reviewers to focus on?
TryGetPendingToolApprovalRequestsonly) — this PR intentionally includes that read API plus drain helpers and the silent-consume fix.Related Issue
Fixes #7872
Related: #7862, #7867
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.