Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1349,7 +1349,7 @@ FunctionResultContent CreateFunctionResultContent(FunctionInvocationResult resul
private (List<ApprovalResultWithRequestMessage>? approvals, List<ApprovalResultWithRequestMessage>? rejections) ExtractAndRemoveApprovalRequestsAndResponses(
List<ChatMessage> messages)
{
Dictionary<string, ChatMessage>? allApprovalRequestsMessages = null;
Dictionary<string, (ChatMessage Message, ToolApprovalRequestContent RequestContent)>? allApprovalRequestsMessages = null;
List<ToolApprovalResponseContent>? allApprovalResponses = null;
HashSet<string>? approvalRequestCallIds = null;
HashSet<string>? functionResultCallIds = null;
Expand All @@ -1376,7 +1376,7 @@ FunctionResultContent CreateFunctionResultContent(FunctionInvocationResult resul
case ToolApprovalRequestContent tarc when tarc.ToolCall is FunctionCallContent { InformationalOnly: false }:
// Validation: Capture each call id for each approval request to ensure later we have a matching response.
_ = (approvalRequestCallIds ??= []).Add(tarc.ToolCall.CallId);
(allApprovalRequestsMessages ??= []).Add(tarc.RequestId, message);
(allApprovalRequestsMessages ??= []).Add(tarc.RequestId, (message, tarc));
break;

case ToolApprovalResponseContent tarc when tarc.ToolCall is FunctionCallContent { InformationalOnly: false }:
Expand All @@ -1385,6 +1385,12 @@ FunctionResultContent CreateFunctionResultContent(FunctionInvocationResult resul
(allApprovalResponses ??= []).Add(tarc);
break;

case ToolApprovalResponseContent tarc when tarc.ToolCall is FunctionCallContent { InformationalOnly: true }:
// Remove from validation set to handle sessions serialized before the fix
// for https://github.com/dotnet/extensions/pull/7468.
_ = approvalRequestCallIds?.Remove(tarc.ToolCall.CallId);
goto default;

case FunctionResultContent frc:
// Maintain a list of function calls that have already been invoked to avoid invoking them twice.
_ = (functionResultCallIds ??= []).Add(frc.CallId);
Expand Down Expand Up @@ -1451,9 +1457,14 @@ FunctionResultContent CreateFunctionResultContent(FunctionInvocationResult resul
ref List<ApprovalResultWithRequestMessage>? targetList = ref approvalResponse.Approved ? ref approvedFunctionCalls : ref rejectedFunctionCalls;

ChatMessage? requestMessage = null;
_ = allApprovalRequestsMessages?.TryGetValue(approvalResponse.RequestId, out requestMessage);
ToolApprovalRequestContent? requestContent = null;
if (allApprovalRequestsMessages?.TryGetValue(approvalResponse.RequestId, out var requestInfo) is true)
{
requestMessage = requestInfo.Message;
requestContent = requestInfo.RequestContent;
}

(targetList ??= []).Add(new() { Response = approvalResponse, RequestMessage = requestMessage });
(targetList ??= []).Add(new() { Response = approvalResponse, Request = requestContent, RequestMessage = requestMessage });
}
}

Expand All @@ -1469,17 +1480,21 @@ FunctionResultContent CreateFunctionResultContent(FunctionInvocationResult resul
rejections is { Count: > 0 } ?
rejections.ConvertAll(m =>
{
LogFunctionRejected(m.FunctionCallContent.Name, m.Response.Reason);
LogFunctionRejected(m.ResponseFunctionCallContent.Name, m.Response.Reason);

string result = "Tool call invocation rejected.";
if (!string.IsNullOrWhiteSpace(m.Response.Reason))
{
result = $"{result} {m.Response.Reason}";
}

// Mark the function call as purely informational since we're handling it (by rejecting it)
m.FunctionCallContent.InformationalOnly = true;
return (AIContent)new FunctionResultContent(m.FunctionCallContent.CallId, result);
// Mark the function call as purely informational since we're handling it (by rejecting it).
// We mark both the response and request FunctionCallContent to ensure consistency
// across serialization boundaries where they may be separate object instances.
m.ResponseFunctionCallContent.InformationalOnly = true;
_ = m.RequestFunctionCallContent?.InformationalOnly = true;
Comment thread
jozkee marked this conversation as resolved.

return (AIContent)new FunctionResultContent(m.ResponseFunctionCallContent.CallId, result);
}) :
null;

Expand Down Expand Up @@ -1708,6 +1723,13 @@ private IList<ChatMessage> ReplaceFunctionCallsWithApprovalRequests(
originalMessages, options, notInvokedApprovals.Select(x => x.Response.ToolCall).OfType<FunctionCallContent>().ToList(), 0, consecutiveErrorCount, isStreaming, cancellationToken);
consecutiveErrorCount = modeAndMessages.NewConsecutiveErrorCount;

// Also mark the request's FCC as InformationalOnly to ensure consistency
// across serialization boundaries where they may be separate object instances.
foreach (var approval in notInvokedApprovals)
{
_ = approval.RequestFunctionCallContent?.InformationalOnly = true;
Comment thread
jozkee marked this conversation as resolved.
}

return (modeAndMessages.MessagesAdded, modeAndMessages.ShouldTerminate, consecutiveErrorCount);
}

Expand Down Expand Up @@ -1788,7 +1810,9 @@ public enum FunctionInvocationStatus
private readonly struct ApprovalResultWithRequestMessage
{
public ToolApprovalResponseContent Response { get; init; }
public ToolApprovalRequestContent? Request { get; init; }
public ChatMessage? RequestMessage { get; init; }
public FunctionCallContent FunctionCallContent => (FunctionCallContent)Response.ToolCall;
public FunctionCallContent ResponseFunctionCallContent => (FunctionCallContent)Response.ToolCall;
public FunctionCallContent? RequestFunctionCallContent => Request?.ToolCall as FunctionCallContent;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,148 @@ public async Task AlreadyExecutedApprovalsAreIgnoredAsync()
await InvokeAndAssertStreamingAsync(options, input, downstreamClientOutput, output, expectedDownstreamClientInput);
}

/// <summary>
/// After serialization/deserialization, the TARC and TAResp may contain separate FCC object instances
/// for the same call. When a rejection is processed, GenerateRejectedFunctionResults must set
/// InformationalOnly=true on BOTH the TAResp's FCC and the TARC's FCC to ensure consistency
/// across serialization boundaries. This test verifies that both FCC instances are correctly
/// marked after rejection processing.
/// </summary>
[Theory]
[InlineData(false)]
[InlineData(true)]
public async Task RejectionSetsInformationalOnlyOnBothRequestAndResponseFccInstancesAsync(bool streaming)
{
// Create two separate FCC objects for the same call — simulating deserialization
// where TARC and TAResp hold different FCC instances with the same CallId.
var requestFcc = new FunctionCallContent("callId1", "Func1");
var responseFcc = new FunctionCallContent("callId1", "Func1");

Assert.False(requestFcc.InformationalOnly);
Assert.False(responseFcc.InformationalOnly);

var options = new ChatOptions
{
Tools =
[
new ApprovalRequiredAIFunction(AIFunctionFactory.Create(() => "Result 1", "Func1")),
]
};

List<ChatMessage> input =
[
new ChatMessage(ChatRole.User, "hello"),
new ChatMessage(ChatRole.Assistant,
[
new ToolApprovalRequestContent("ficc_callId1", requestFcc),
]) { MessageId = "resp1" },
new ChatMessage(ChatRole.User,
[
new ToolApprovalResponseContent("ficc_callId1", false, responseFcc),
]),
];

using var innerClient = new TestChatClient
{
GetResponseAsyncCallback = (contents, actualOptions, actualCancellationToken) =>
Task.FromResult(new ChatResponse([new ChatMessage(ChatRole.Assistant, "world")])),
GetStreamingResponseAsyncCallback = (contents, actualOptions, actualCancellationToken) =>
YieldAsync(new ChatResponse([new ChatMessage(ChatRole.Assistant, "world")]).ToChatResponseUpdates()),
};

IChatClient service = innerClient.AsBuilder()
.Use(s => new FunctionInvokingChatClient(s))
.Build();

if (streaming)
{
await service.GetStreamingResponseAsync(input, options).ToChatResponseAsync();
}
else
{
await service.GetResponseAsync(input, options);
}

// The fix ensures both FCC instances are marked InformationalOnly=true,
// even when they are separate objects (as happens after serialization).
Assert.True(requestFcc.InformationalOnly);
Assert.True(responseFcc.InformationalOnly);
}

/// <summary>
/// After serialization/deserialization, the TARC and TAResp may contain separate FCC object instances
/// for the same call. When a rejection is processed, GenerateRejectedFunctionResults must set
/// InformationalOnly=true on BOTH the TAResp's FCC and the TARC's FCC to ensure consistency
/// across serialization boundaries. Previously, this was not always happening, so adding
/// a test to ensure that this case does not throw.
/// See https://github.com/dotnet/extensions/pull/7468.
/// </summary>
[Theory]
[InlineData(false)]
[InlineData(true)]
public async Task MixedInformationalOnlyDoesNotThrowAsync(bool streaming)
{
// Create two separate FCC objects for the same call — simulating deserialization
// where TARC and TAResp hold different FCC instances with the same CallId.
var request1Fcc = new FunctionCallContent("callId1", "Func1");
var response1Fcc = new FunctionCallContent("callId1", "Func1") { InformationalOnly = true };

var request2Fcc = new FunctionCallContent("callId2", "Func1");

Assert.False(request1Fcc.InformationalOnly);
Assert.True(response1Fcc.InformationalOnly);

var options = new ChatOptions
{
Tools =
[
new ApprovalRequiredAIFunction(AIFunctionFactory.Create(() => "Result 1", "Func1")),
]
};

List<ChatMessage> input =
[
new ChatMessage(ChatRole.User, "hello"),
new ChatMessage(ChatRole.Assistant,
[
new ToolApprovalRequestContent("ficc_callId1", request1Fcc),
]) { MessageId = "resp1" },
new ChatMessage(ChatRole.User,
[
new ToolApprovalResponseContent("ficc_callId1", false, response1Fcc),
]),
new ChatMessage(ChatRole.Assistant,
[
new ToolApprovalRequestContent("ficc_callId2", request2Fcc),
]) { MessageId = "resp2" },
new ChatMessage(ChatRole.User,
[
new ToolApprovalResponseContent("ficc_callId2", false, request2Fcc),
]),
];

using var innerClient = new TestChatClient
{
GetResponseAsyncCallback = (contents, actualOptions, actualCancellationToken) =>
Task.FromResult(new ChatResponse([new ChatMessage(ChatRole.Assistant, "world")])),
GetStreamingResponseAsyncCallback = (contents, actualOptions, actualCancellationToken) =>
YieldAsync(new ChatResponse([new ChatMessage(ChatRole.Assistant, "world")]).ToChatResponseUpdates()),
};

IChatClient service = innerClient.AsBuilder()
.Use(s => new FunctionInvokingChatClient(s))
.Build();

if (streaming)
{
await service.GetStreamingResponseAsync(input, options).ToChatResponseAsync();
}
else
{
await service.GetResponseAsync(input, options);
}
}

/// <summary>
/// This verifies the following scenario:
/// 1. We are streaming (also including non-streaming in the test for completeness).
Expand Down
Loading