diff --git a/dotnet/src/Microsoft.Agents.AI/ChatClient/ApprovalResponseBindingChatClient.cs b/dotnet/src/Microsoft.Agents.AI/ChatClient/ApprovalResponseBindingChatClient.cs index de56a82626..acf09b0b1f 100644 --- a/dotnet/src/Microsoft.Agents.AI/ChatClient/ApprovalResponseBindingChatClient.cs +++ b/dotnet/src/Microsoft.Agents.AI/ChatClient/ApprovalResponseBindingChatClient.cs @@ -155,11 +155,14 @@ private bool TryGetSession([NotNullWhen(true)] out AgentSession? session) } /// - /// Rewrites the inbound messages so that each is bound to a known - /// , with its tool call rebound to the request's call when it differs. - /// A response with no known request is removed so a forged approval cannot drive execution. Approval requests - /// are left untouched: a request present in the message history is itself the pairing authority. + /// Validates inbound against known requests and rebinds matched + /// responses to the model-originated tool call. Responses with no known request are dropped. /// + /// + /// Pending session entries are consumed only when a matching approval response is bound in this turn. + /// An unrelated user message must not clear pending approvals (#7872); otherwise a restored session can + /// lose binding authority while a dangling remains in history. + /// private IEnumerable ValidateInboundApprovalResponses(IEnumerable messages, AgentSession session) { var messageList = messages as IList ?? new List(messages); @@ -170,12 +173,11 @@ private IEnumerable ValidateInboundApprovalResponses(IEnumerable 0) + var pendingFromBag = LoadPendingApprovalRequests(session); + var knownRequests = new Dictionary(pendingFromBag.Count, StringComparer.Ordinal); + foreach (var request in pendingFromBag) { - session.StateBag.TryRemoveValue(StateBagKey); + knownRequests[request.RequestId] = request; } bool hasResponse = false; @@ -196,6 +198,7 @@ private IEnumerable ValidateInboundApprovalResponses(IEnumerable ValidateInboundApprovalResponses(IEnumerable(pendingFromBag.Count); + foreach (var request in pendingFromBag) + { + if (knownRequests.ContainsKey(request.RequestId)) + { + remainingPending.Add(request); + } + } + + SavePendingApprovalRequests(remainingPending, session); + return result ?? messageList; } @@ -376,18 +392,6 @@ private static bool ArgumentsEquivalent(IDictionary? responseAr return true; } - private static Dictionary LoadPendingApprovalRequestLookup(AgentSession session) - { - var pendingRequests = LoadPendingApprovalRequests(session); - var byRequestId = new Dictionary(pendingRequests.Count, StringComparer.Ordinal); - foreach (var request in pendingRequests) - { - byRequestId[request.RequestId] = request; - } - - return byRequestId; - } - /// /// Records model-originated items found in the response messages into /// the session so they can be matched against the caller's approval responses on the next request. @@ -445,10 +449,10 @@ private void MergePendingApprovalRequests(List emitt } /// - /// Creates a snapshot of an approval request so a later mutation of the caller-visible instance - /// (for example changing the tool call arguments) cannot alter the recorded request used for binding. + /// Creates a deep snapshot of an approval request for durable storage or public enumeration so callers + /// cannot mutate the model-originated binding authority through shared . /// - private static ToolApprovalRequestContent SnapshotRequest(ToolApprovalRequestContent request) + internal static ToolApprovalRequestContent SnapshotRequest(ToolApprovalRequestContent request) { if (request.ToolCall is FunctionCallContent functionCall) { diff --git a/dotnet/src/Microsoft.Agents.AI/ChatClient/ToolApprovalAgentSessionExtensions.cs b/dotnet/src/Microsoft.Agents.AI/ChatClient/ToolApprovalAgentSessionExtensions.cs new file mode 100644 index 0000000000..57063e88d8 --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI/ChatClient/ToolApprovalAgentSessionExtensions.cs @@ -0,0 +1,124 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using Microsoft.Extensions.AI; +using Microsoft.Shared.Diagnostics; + +namespace Microsoft.Agents.AI; + +/// +/// Provides extension methods for reading and closing human-in-the-loop tool approval state on an +/// . +/// +/// +/// +/// When a run surfaces and is cancelled, refreshed, or otherwise +/// interrupted before the host submits a matching , the framework +/// retains those requests in session state. Durable hosts can enumerate them after restore, re-surface UI, +/// or drain them as explicit rejections before accepting a new normal user turn (#7862, #7872). +/// +/// +/// Rejection helpers return response content the host should send on the next agent run so +/// can emit terminal and close +/// the tool lifecycle. Pending bag entries stay until those responses are validated and consumed on that run; +/// clearing the bag first would drop the only binding authority on restore paths where message history has a +/// dangling without a matching approval request. +/// +/// +public static class ToolApprovalAgentSessionExtensions +{ + /// + /// Attempts to retrieve the tool approval requests that the framework has surfaced for the specified session + /// and that have not yet been answered with a matching . + /// + /// The agent session to read pending approval requests from. + /// + /// When this method returns, contains deep snapshots of the pending approval requests if any were found; + /// otherwise, . + /// + /// + /// if at least one pending approval request was found; otherwise, + /// . + /// + public static bool TryGetPendingToolApprovalRequests( + this AgentSession session, + [NotNullWhen(true)] out IReadOnlyList? requests) + { + _ = Throw.IfNull(session); + + if (session.StateBag.TryGetValue>( + ApprovalResponseBindingChatClient.StateBagKey, + out var pending, + AgentJsonUtilities.DefaultOptions) + && pending is { Count: > 0 }) + { + // Deep-snapshot each request so hosts cannot mutate FunctionCallContent.Arguments on the + // model-originated binding authority retained in the session bag. + var snapshots = new List(pending.Count); + foreach (var request in pending) + { + snapshots.Add(ApprovalResponseBindingChatClient.SnapshotRequest(request)); + } + + requests = snapshots; + return true; + } + + requests = null; + return false; + } + + /// + /// Creates rejection responses for every pending tool approval request on the session. + /// + /// The agent session whose pending approvals should be closed. + /// + /// An optional reason recorded on each rejection (for example cancellation or host-initiated drain). + /// + /// + /// The rejection responses to send on the next agent run so the function loop can emit terminal + /// . Empty when nothing was pending. Pending bag entries remain until + /// those responses are bound and consumed on that run. + /// + public static IReadOnlyList CreatePendingApprovalRejections( + this AgentSession session, + string? reason = null) + { + _ = Throw.IfNull(session); + + if (!session.TryGetPendingToolApprovalRequests(out var pending)) + { + return []; + } + + var responses = new List(pending.Count); + foreach (var request in pending) + { + responses.Add(request.CreateResponse(approved: false, reason)); + } + + // Do not clear the bag here. On restore, history may only contain a dangling FunctionCallContent; + // ValidateInboundApprovalResponses needs these entries to honor the returned rejections. + return responses; + } + + /// + /// Removes all pending tool approval requests from the session without creating rejection responses. + /// + /// The agent session to clear. + /// + /// if pending approval state was present and removed; otherwise, + /// . + /// + /// + /// Prefer when the host can still run the agent, so the + /// function loop can close each tool call with a terminal result. Use this only when discarding binding + /// authority intentionally without producing responses. + /// + public static bool ClearPendingToolApprovalRequests(this AgentSession session) + { + _ = Throw.IfNull(session); + return session.StateBag.TryRemoveValue(ApprovalResponseBindingChatClient.StateBagKey); + } +} diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ApprovalResponseBindingChatClientTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ApprovalResponseBindingChatClientTests.cs index e89e2eae97..0e8f7ab1e5 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ApprovalResponseBindingChatClientTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ApprovalResponseBindingChatClientTests.cs @@ -270,6 +270,150 @@ public async Task GetResponseAsync_NoSession_PassesThroughUnvalidatedAsync() Assert.Contains(capture.Messages!.SelectMany(m => m.Contents), c => c is ToolApprovalResponseContent); } + [Fact] + public async Task GetResponseAsync_UnrelatedUserMessage_DoesNotConsumePendingEntryAsync() + { + // Arrange — an approval was surfaced, then the host sends a normal user message with the same session. + var session = new ChatClientAgentSession(); + await RecordRequestAsync(session, new ToolApprovalRequestContent(RequestId, new FunctionCallContent("call1", "toolA"))); + Assert.True(session.TryGetPendingToolApprovalRequests(out _)); + + var capture = new Capture(); + var decorator = new ApprovalResponseBindingChatClient(CreateCapturingChatClient(capture)); + + // Act + await RunAsync(decorator, session, [new ChatMessage(ChatRole.User, "unrelated follow-up")]); + + // Assert — pending approval authority must survive (#7872); only matching responses consume it. + Assert.True(session.TryGetPendingToolApprovalRequests(out var pending)); + Assert.Equal(RequestId, Assert.Single(pending!).RequestId); + Assert.DoesNotContain(capture.Messages!.SelectMany(m => m.Contents), c => c is ToolApprovalResponseContent); + } + + [Fact] + public async Task TryGetPendingToolApprovalRequests_SurvivesSessionStateRoundTripAsync() + { + // Arrange — a run stops on an approval request, then the session is persisted and reloaded. + var session = new ChatClientAgentSession(); + var call = new FunctionCallContent("call1", "get_weather", new Dictionary { ["location"] = "Beijing" }); + await RecordRequestAsync(session, new ToolApprovalRequestContent(RequestId, call)); + + var restored = new ChatClientAgentSession( + stateBag: AgentSessionStateBag.Deserialize(session.StateBag.Serialize())); + + // Act + var found = restored.TryGetPendingToolApprovalRequests(out var pending); + + // Assert — the host can discover the pending approval without reading private state bag keys. + Assert.True(found); + var request = Assert.Single(pending!); + Assert.Equal(RequestId, request.RequestId); + var pendingCall = Assert.IsType(request.ToolCall); + Assert.Equal("get_weather", pendingCall.Name); + Assert.Equal("call1", pendingCall.CallId); + } + + [Fact] + public async Task TryGetPendingToolApprovalRequests_AfterResponseIsConsumed_ReturnsFalseAsync() + { + // Arrange — record a request, then answer it. + var session = new ChatClientAgentSession(); + await RecordRequestAsync(session, new ToolApprovalRequestContent(RequestId, new FunctionCallContent("call1", "toolA"))); + Assert.True(session.TryGetPendingToolApprovalRequests(out _)); + + var decorator = new ApprovalResponseBindingChatClient(CreateCapturingChatClient(new Capture())); + var approval = new ToolApprovalResponseContent(RequestId, approved: true, new FunctionCallContent("call1", "toolA")); + + // Act + await RunAsync(decorator, session, [new ChatMessage(ChatRole.User, [approval])]); + + // Assert — the answered request is no longer pending. + Assert.False(session.TryGetPendingToolApprovalRequests(out var pending)); + Assert.Null(pending); + } + + [Fact] + public void TryGetPendingToolApprovalRequests_NoApprovalState_ReturnsFalse() + { + Assert.False(new ChatClientAgentSession().TryGetPendingToolApprovalRequests(out var pending)); + Assert.Null(pending); + } + + [Fact] + public async Task TryGetPendingToolApprovalRequests_ReturnsDeepSnapshot_HostMutationDoesNotAffectBagAsync() + { + // Arrange + var session = new ChatClientAgentSession(); + await RecordRequestAsync( + session, + new ToolApprovalRequestContent( + RequestId, + new FunctionCallContent("call1", "toolA", new Dictionary { ["amount"] = 1 }))); + + Assert.True(session.TryGetPendingToolApprovalRequests(out var pending)); + var enumeratedCall = Assert.IsType(Assert.Single(pending!).ToolCall); + + // Act — mutate the publicly enumerated instance. + enumeratedCall.Arguments!["amount"] = 9999999; + + // Assert — bag snapshot used for binding is unchanged. + var capture = new Capture(); + var decorator = new ApprovalResponseBindingChatClient(CreateCapturingChatClient(capture)); + var response = new ToolApprovalResponseContent( + RequestId, + approved: true, + new FunctionCallContent("call1", "toolA", new Dictionary { ["amount"] = 9999999 })); + await RunAsync(decorator, session, [new ChatMessage(ChatRole.User, [response])]); + + var forwarded = capture.Messages!.SelectMany(m => m.Contents).OfType().Single(); + Assert.Equal(1, Assert.IsType(forwarded.ToolCall).Arguments!["amount"]); + } + + [Fact] + public async Task CreatePendingApprovalRejections_KeepsPendingUntilResponsesAreConsumedAsync() + { + // Arrange — restore-style session: pending in the bag, no approval request in inbound history. + var session = new ChatClientAgentSession(); + await RecordRequestAsync( + session, + new ToolApprovalRequestContent(RequestId, new FunctionCallContent("call1", "toolA"))); + + // Act — host drains by creating rejections (bag must remain for binding on the next run). + var responses = session.CreatePendingApprovalRejections(reason: "host drain"); + + // Assert — rejections are ready, but binding authority is still present. + var rejection = Assert.Single(responses); + Assert.Equal(RequestId, rejection.RequestId); + Assert.False(rejection.Approved); + Assert.Equal("host drain", rejection.Reason); + Assert.True(session.TryGetPendingToolApprovalRequests(out _)); + + // Submitting the rejections consumes the bag so FICC can emit terminal results (#7872). + var capture = new Capture(); + var decorator = new ApprovalResponseBindingChatClient(CreateCapturingChatClient(capture)); + await RunAsync(decorator, session, [new ChatMessage(ChatRole.User, [.. responses])]); + + Assert.Contains(capture.Messages!.SelectMany(m => m.Contents), c => c is ToolApprovalResponseContent { Approved: false }); + Assert.False(session.TryGetPendingToolApprovalRequests(out _)); + } + + [Fact] + public void ClearPendingToolApprovalRequests_RemovesBagEntry() + { + var session = new ChatClientAgentSession(); + session.StateBag.SetValue( + ApprovalResponseBindingChatClient.StateBagKey, + new List + { + new(RequestId, new FunctionCallContent("call1", "toolA")), + }, + AgentJsonUtilities.DefaultOptions); + + Assert.True(session.ClearPendingToolApprovalRequests()); + Assert.False(session.TryGetPendingToolApprovalRequests(out _)); + Assert.False(session.ClearPendingToolApprovalRequests()); + } + private static async Task RecordRequestAsync(ChatClientAgentSession session, ToolApprovalRequestContent request) { var inner = CreateMockChatClient((_, _, _) =>