.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? |
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.