From cdd80c61ac9cb698fb479d924a0b1d474a75392a Mon Sep 17 00:00:00 2001 From: Ben Thomas Date: Wed, 6 May 2026 17:30:41 -0700 Subject: [PATCH] .NET: Issue 5662 (#5668) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix dangling function_call on approval response in Foundry hosting (#5662) Make the wire<->AF approval translation in Microsoft.Agents.AI.Foundry.Hosting lossless so the resume turn pairs function_call/function_call_output correctly. Root cause: InputConverter.ConvertMcpApprovalResponse rebuilt FunctionCallContent with CallId set to the FICC-composed AF request id (ficc_) and Name hardcoded to 'mcp_approval'. This (a) broke Azure Conversations pairing because the persisted function_call had CallId without prefix, and (b) made FICC unable to invoke the original tool by name on resume. Fix: ToolApprovalIdMap now records the original FunctionCallContent (CallId, Name, Arguments) keyed by wire id at outbound time. InputConverter reconstructs the original FCC on inbound, falling back to the legacy placeholder when no mapping exists. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Suppress orphan function_call items at the wire (#5662) Foundry-Hosting's OutputConverter was emitting FunctionCallContent as wire `function_call` items while dropping the paired FunctionResultContent. The result: every auto-invoked tool call left an orphan `function_call` in the response store. The next turn (chained via previous_response_id or via a workflow that yields after one turn under externalLoop) reloaded that history and submitted it to Azure Conversations, which rejected it with HTTP 400 `No tool output found for function call ...`. Function call/result pairs are entirely internal to the agent's tool-calling loop and have no place on the wire. Approval-required calls already surface separately via ToolApprovalRequestContent → mcp_approval_request, so dropping FCC is safe. FCC's message-close behavior is preserved so pre-tool text doesn't accidentally concatenate with post-tool text under the same MessageId. Existing OutputConverter tests asserting FCC wire emission are updated to assert suppression. Verified end-to-end against the declarative-workflow-menu external_loop bench: three-turn previous_response_id chain (menu → carbonara price → EXIT) now completes, where it previously failed at turn 2 with HTTP 400. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fail fast when no approval mapping is recorded (#5662) The previous best-effort placeholder fallback in InputConverter.ConvertMcpApprovalResponse couldn't actually round-trip — it just delayed and obscured the failure as an HTTP 400 deep inside the agent loop. Throw InvalidOperationException with the wire id and a clear cause hint instead so the failure is local and actionable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Trim narrative comments and exception message (#5662) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Defer FunctionCallContent emission until matched FunctionResultContent (#5662) Replace blanket FCC suppression with deferred emission. FunctionCallContent is buffered (name + serialized arguments) keyed by CallId; the function_call and function_call_output wire items are only flushed once the matching FunctionResultContent arrives. - Auto-invoked FCC/FRC pairs surface as paired wire items so Azure's stored conversation has matched call+output and previous_response_id resume works (closes the orphan-function_call symptom from #5662). - Orphan FCCs (e.g. workflow paused at a checkpoint mid-tool-loop) are dropped so they never poison the response store. - Approval flows are unchanged: TARC still emits mcp_approval_request and the post-approval FRC has no buffered FCC to pair with so it is dropped; the approval round-trip handles its own pairing via mcp_approval_*. - Leaves the door open for future client-side function calling: that pattern would surface an FCC without an FRC, would need to opt out of buffering, but the wire shape is already correct. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Emit FunctionCallContent and FunctionResultContent directly (option B) Replace the deferred-emission/buffer-and-drop strategy with direct emission of both function_call and function_call_output wire items. Rationale: a lone FunctionCallContent in OutputConverter's input can mean two semantically different things, and only the caller knows which: - Auto-invoke (FICC response surface): always paired with a matching FRC; both halves should appear on the wire as historical record. - HITL / port-pause request (typed RequestPort or workflow synthesizing a request): a lone FCC IS the wire signal that the caller must resume by supplying a function_call_output. Buffering+dropping orphans silently swallows the second case. Emitting both directly is the only correct shape for OpenAI Responses semantics. The InputConverter already accepts function_call_output and mcp_approval_response on resume, so the round-trip works for both kinds. The approval-flow round-trip fixes (ToolApprovalIdMap rich ApprovalEntry, fail-fast on missing mapping in ConvertMcpApprovalResponse) remain intact. Tests: updated 7 OutputConverter tests + 1 OutputConverterWorkflow test that asserted the old buffer/drop semantics; all 227 tests pass. Refs #5662 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR #5668 review feedback on TryLoadMap Stop swallowing JsonException in ToolApprovalIdMap.TryLoadMap. The catch block recovered to an empty map and a stale comment claimed the caller would gracefully degrade via a 'wire-id fallback path' — but that path no longer exists: InputConverter.ConvertMcpApprovalResponse fails fast when no entry is found. Letting the JsonException propagate produces an error message that points at the actual cause (a state-bag format incompatibility), instead of converting it into a confusing 'no approval mapping recorded' InvalidOperationException one stack frame later. Refs #5662, PR #5668 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR #5668 review feedback round 2 - OutputConverter FRC: emit string results as raw text (no JSON-quoting), matching the wire contract for function_call_output.output. - OutputConverter FCC: validate non-empty CallId before closing the in-flight text message, so a skipped FCC no longer breaks output-item boundaries. - ToolApprovalIdMap.Record: take pre-serialized arguments JSON (string) and primitive callId/name. Drops [RequiresUnreferencedCode]/[RequiresDynamicCode] so trim/AOT warnings stop propagating to call sites. - ToolApprovalIdMap.Record: no-op when callId or name is empty. - Tests: dedup duplicate ConvertItemsToMessages_McpApprovalResponse no-mapping test; add coverage for empty-CallId boundary, raw-string FRC payload, and Record empty-key no-op. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: alliscode Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../InputConverter.cs | 24 ++-- .../OutputConverter.cs | 68 +++++++--- .../ToolApprovalIdMap.cs | 92 ++++++++++++-- .../InputConverterTests.cs | 71 +++++++++-- .../OutputConverterTests.cs | 117 +++++++++++++++--- .../OutputConverterWorkflowTests.cs | 4 +- 6 files changed, 314 insertions(+), 62 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/InputConverter.cs b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/InputConverter.cs index ed8c8823b6..7d501f588a 100644 --- a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/InputConverter.cs +++ b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/InputConverter.cs @@ -233,18 +233,28 @@ internal static class InputConverter /// /// Converts an inbound mcp_approval_response wire item to a - /// . Looks up the original AF request id - /// via ; falls back to the wire id when the mapping - /// is unavailable. Carries a placeholder because - /// the original tool-call details are not echoed by clients in the response item. + /// . Looks up the original + /// via so the + /// reconstructed response carries the original tool name, call id, and arguments. /// + /// + /// Thrown when no mapping is recorded for . + /// Without the mapping the original call cannot be reconstructed, so we fail the request. + /// private static ChatMessage ConvertMcpApprovalResponse(string approvalRequestId, bool approve, AgentSessionStateBag? stateBag) { - var afRequestId = ToolApprovalIdMap.Resolve(stateBag, approvalRequestId); - var placeholderFunctionCall = new FunctionCallContent(afRequestId, "mcp_approval"); + var entry = ToolApprovalIdMap.ResolveEntry(stateBag, approvalRequestId) + ?? throw new InvalidOperationException( + $"No approval mapping recorded for wire id '{approvalRequestId}'."); + + var functionCall = new FunctionCallContent( + entry.CallId, + entry.Name, + ParseFunctionArgumentsObject(entry.Arguments)); + return new ChatMessage( ChatRole.User, - [new ToolApprovalResponseContent(afRequestId, approve, placeholderFunctionCall)]); + [new ToolApprovalResponseContent(entry.AfRequestId, approve, functionCall)]); } [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "Deserializing tool-call arguments from SDK input.")] diff --git a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/OutputConverter.cs b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/OutputConverter.cs index 97d01f1afb..5d2524fda3 100644 --- a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/OutputConverter.cs +++ b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/OutputConverter.cs @@ -118,8 +118,13 @@ internal static class OutputConverter break; } - case FunctionCallContent funcCall: + case FunctionCallContent functionCall: { + if (functionCall.CallId is not { Length: > 0 }) + { + break; + } + foreach (var evt in CloseCurrentMessage(currentMessageBuilder, currentTextBuilder, accumulatedText)) { yield return evt; @@ -130,17 +135,15 @@ internal static class OutputConverter accumulatedText = null; previousMessageId = null; - var callId = funcCall.CallId ?? Guid.NewGuid().ToString("N"); - var funcBuilder = stream.AddOutputItemFunctionCall(funcCall.Name, callId); - yield return funcBuilder.EmitAdded(); - - var arguments = funcCall.Arguments is not null - ? JsonSerializer.Serialize(funcCall.Arguments) + var arguments = functionCall.Arguments is not null + ? JsonSerializer.Serialize(functionCall.Arguments) : "{}"; - yield return funcBuilder.EmitArgumentsDelta(arguments); - yield return funcBuilder.EmitArgumentsDone(arguments); - yield return funcBuilder.EmitDone(); + var fcBuilder = stream.AddOutputItemFunctionCall(functionCall.Name, functionCall.CallId); + yield return fcBuilder.EmitAdded(); + yield return fcBuilder.EmitArgumentsDelta(arguments); + yield return fcBuilder.EmitArgumentsDone(arguments); + yield return fcBuilder.EmitDone(); break; } @@ -191,12 +194,19 @@ internal static class OutputConverter // wireId↔afRequestId mapping in the session state bag for later lookup // when the matching `mcp_approval_response` arrives on a subsequent turn. var wireId = ToolApprovalIdMap.ComputeWireId(approvalRequest.RequestId); - ToolApprovalIdMap.Record(stateBag, wireId, approvalRequest.RequestId); var approvalArguments = approvalFunctionCall.Arguments is not null ? JsonSerializer.Serialize(approvalFunctionCall.Arguments) : "{}"; + ToolApprovalIdMap.Record( + stateBag, + wireId, + approvalRequest.RequestId, + approvalFunctionCall.CallId, + approvalFunctionCall.Name, + approvalArguments); + var approvalItem = new OutputItemMcpApprovalRequest( wireId, "agent_framework", @@ -252,10 +262,40 @@ internal static class OutputConverter // These would need to be serialized as base64 or URL references. break; - case FunctionResultContent: - // Function results are internal to the agent's tool-calling loop - // and are not emitted as output items in the response stream. + case FunctionResultContent functionResult: + { + if (functionResult.CallId is not { Length: > 0 }) + { + break; + } + + foreach (var evt in CloseCurrentMessage(currentMessageBuilder, currentTextBuilder, accumulatedText)) + { + yield return evt; + } + + currentTextBuilder = null; + currentMessageBuilder = null; + accumulatedText = null; + previousMessageId = null; + + var outputText = functionResult.Result switch + { + null => string.Empty, + string s => s, + _ => JsonSerializer.Serialize(functionResult.Result), + }; + + var itemId = GenerateItemId("fc"); + var outputItem = new OutputItemFunctionToolCallOutput( + functionResult.CallId, + BinaryData.FromString(outputText)); + + var outputBuilder = stream.AddOutputItem(itemId); + yield return outputBuilder.EmitAdded(outputItem); + yield return outputBuilder.EmitDone(outputItem); break; + } default: break; diff --git a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ToolApprovalIdMap.cs b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ToolApprovalIdMap.cs index 64f6791455..a658155cf4 100644 --- a/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ToolApprovalIdMap.cs +++ b/dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ToolApprovalIdMap.cs @@ -4,23 +4,41 @@ using System; using System.Collections.Generic; using System.Security.Cryptography; using System.Text; +using Microsoft.Extensions.AI; namespace Microsoft.Agents.AI.Foundry.Hosting; /// /// Helper for translating between agent-framework tool-approval request ids and the /// strict-format wire ids required by the Responses Server SDK mcp_approval_request -/// item type. The mapping is persisted in so an -/// approval request emitted on one HTTP turn can be matched to the response posted -/// back on the next turn. +/// item type, and for preserving the original across +/// the request/response round trip. The mapping is persisted in +/// . /// internal static class ToolApprovalIdMap { /// - /// State-bag key used to store the wire-id ↔ AF-request-id mapping. + /// State-bag key used to store the wire-id ↔ approval-entry mapping. /// public const string StateBagKey = "Microsoft.Agents.AI.Foundry.Hosting.ToolApprovalIdMap"; + /// + /// Captures the data needed to reconstruct the original + /// on the inbound (response) side. + /// + /// + /// FICC composes RequestId as "ficc_{CallId}"; CallId is stored + /// independently so the reconstructed function-call id matches the one the model + /// emitted and the backend Conversations API persisted. + /// + internal sealed class ApprovalEntry + { + public string AfRequestId { get; set; } = string.Empty; + public string CallId { get; set; } = string.Empty; + public string Name { get; set; } = string.Empty; + public string? Arguments { get; set; } + } + /// /// SDK item-id format constraints: {prefix}_{50_or_48_chars}. We use the /// canonical mcpr_ prefix and a SHA-256 truncated to 50 hex chars (25 bytes) @@ -41,33 +59,81 @@ internal static class ToolApprovalIdMap } /// - /// Records the wire-id → AF-request-id mapping in the supplied state bag. + /// Records the wire-id → approval-entry mapping in the supplied state bag. + /// Arguments are passed as already-serialized JSON to keep this method + /// trim/AOT-friendly (no polymorphic object serialization here). + /// No-op when or is empty — + /// without those fields the entry cannot be used to faithfully reconstruct + /// the original on the inbound side. /// - public static void Record(AgentSessionStateBag? stateBag, string wireId, string afRequestId) + public static void Record(AgentSessionStateBag? stateBag, string wireId, string afRequestId, string? callId, string? name, string? argumentsJson) { if (stateBag is null) { return; } - var map = stateBag.GetValue>(StateBagKey) - ?? new Dictionary(StringComparer.Ordinal); - map[wireId] = afRequestId; + if (string.IsNullOrEmpty(callId) || string.IsNullOrEmpty(name)) + { + return; + } + + var map = LoadMap(stateBag); + map[wireId] = new ApprovalEntry + { + AfRequestId = afRequestId, + CallId = callId!, + Name = name!, + Arguments = argumentsJson, + }; stateBag.SetValue(StateBagKey, map); } /// /// Looks up the AF request id for a given wire id. Returns the wire id verbatim - /// when no mapping is present (best-effort fallback that keeps converters total). + /// when no mapping is present. /// public static string Resolve(AgentSessionStateBag? stateBag, string wireId) { - if (stateBag?.GetValue>(StateBagKey) is { } map - && map.TryGetValue(wireId, out var afRequestId)) + if (TryLoadMap(stateBag, out var map) + && map.TryGetValue(wireId, out var entry)) { - return afRequestId; + return entry.AfRequestId; } return wireId; } + + /// + /// Looks up the full approval entry for a given wire id, or + /// when no mapping is present. + /// + public static ApprovalEntry? ResolveEntry(AgentSessionStateBag? stateBag, string wireId) + { + if (TryLoadMap(stateBag, out var map) + && map.TryGetValue(wireId, out var entry)) + { + return entry; + } + + return null; + } + + private static Dictionary LoadMap(AgentSessionStateBag stateBag) + => TryLoadMap(stateBag, out var map) ? map : new Dictionary(StringComparer.Ordinal); + + private static bool TryLoadMap(AgentSessionStateBag? stateBag, out Dictionary map) + { + if (stateBag is null) + { + map = null!; + return false; + } + + // Don't swallow JsonException: ConvertMcpApprovalResponse fails fast on a missing entry, + // so an empty map here would just turn a clear deserialization error into a confusing one. + map = stateBag.GetValue>(StateBagKey) + ?? new Dictionary(StringComparer.Ordinal); + return true; + } } diff --git a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/InputConverterTests.cs b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/InputConverterTests.cs index d45ba3af6d..fcf6001bfc 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/InputConverterTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/InputConverterTests.cs @@ -780,25 +780,33 @@ public class InputConverterTests } [Fact] - public void ConvertItemsToMessages_McpApprovalResponse_ProducesToolApprovalResponse_FallsBackToWireIdWhenNoMapping() + public void ConvertItemsToMessages_McpApprovalResponse_ThrowsWhenNoMapping() { + // Without a recorded ApprovalEntry the converter cannot reconstruct the original + // function call faithfully — any placeholder it produced would still fail downstream + // (FICC has no tool to invoke; Azure's stored function_call can't pair with the + // synthetic id). Fail fast with a clear error instead of continuing into a confusing + // HTTP 400 deep inside the agent loop. var wireId = "mcpr_" + new string('a', 50); var item = new MCPApprovalResponse(approvalRequestId: wireId, approve: true); - var messages = InputConverter.ConvertItemsToMessages([item]); - - var content = Assert.IsType(Assert.Single(messages[0].Contents)); - Assert.Equal(wireId, content.RequestId); - Assert.True(content.Approved); + var ex = Assert.Throws(() => InputConverter.ConvertItemsToMessages([item])); + Assert.Contains(wireId, ex.Message); } [Fact] public void ConvertItemsToMessages_McpApprovalResponse_ResolvesAfRequestIdFromStateBag() { - const string AfRequestId = "af_request_xyz"; + const string AfRequestId = "ficc_call_xyz"; var wireId = ToolApprovalIdMap.ComputeWireId(AfRequestId); var stateBag = new AgentSessionStateBag(); - ToolApprovalIdMap.Record(stateBag, wireId, AfRequestId); + ToolApprovalIdMap.Record( + stateBag, + wireId, + AfRequestId, + "call_xyz", + "issue_refund", + "{\"order_id\":123}"); var item = new MCPApprovalResponse(approvalRequestId: wireId, approve: false); @@ -807,6 +815,17 @@ public class InputConverterTests var content = Assert.IsType(Assert.Single(messages[0].Contents)); Assert.Equal(AfRequestId, content.RequestId); Assert.False(content.Approved); + + // Verify the original FunctionCallContent is reconstructed losslessly: + // - CallId matches the model-issued id (without FICC's "ficc_" prefix), so the + // resulting function_call_output pairs with Azure's stored function_call. + // - Name matches the original tool, so FICC can invoke the right function on resume. + // - Arguments are preserved. + var fcc = Assert.IsType(content.ToolCall); + Assert.Equal("call_xyz", fcc.CallId); + Assert.Equal("issue_refund", fcc.Name); + Assert.NotNull(fcc.Arguments); + Assert.Equal(123, ((System.Text.Json.JsonElement)fcc.Arguments!["order_id"]!).GetInt32()); } [Fact] @@ -828,10 +847,16 @@ public class InputConverterTests [Fact] public void ConvertOutputItemsToMessages_McpApprovalResponse_ProducesToolApprovalResponse() { - const string AfRequestId = "af_request_history"; + const string AfRequestId = "ficc_call_history"; var wireId = ToolApprovalIdMap.ComputeWireId(AfRequestId); var stateBag = new AgentSessionStateBag(); - ToolApprovalIdMap.Record(stateBag, wireId, AfRequestId); + ToolApprovalIdMap.Record( + stateBag, + wireId, + AfRequestId, + "call_history", + "delete_file", + "{\"path\":\"/tmp/x\"}"); var item = new OutputItemMcpApprovalResponseResource( id: "ar_history_id", @@ -843,6 +868,10 @@ public class InputConverterTests var content = Assert.IsType(Assert.Single(messages[0].Contents)); Assert.Equal(AfRequestId, content.RequestId); Assert.True(content.Approved); + + var fcc = Assert.IsType(content.ToolCall); + Assert.Equal("call_history", fcc.CallId); + Assert.Equal("delete_file", fcc.Name); } [Fact] @@ -862,6 +891,28 @@ public class InputConverterTests Assert.Equal("not valid json", fc.Arguments!["_raw"]?.ToString()); } + [Fact] + public void ToolApprovalIdMap_Record_EmptyCallId_IsNoOp() + { + var stateBag = new AgentSessionStateBag(); + var wireId = "mcpr_" + new string('d', 50); + + ToolApprovalIdMap.Record(stateBag, wireId, "ficc_x", callId: string.Empty, name: "tool", argumentsJson: "{}"); + + Assert.Null(ToolApprovalIdMap.ResolveEntry(stateBag, wireId)); + } + + [Fact] + public void ToolApprovalIdMap_Record_EmptyName_IsNoOp() + { + var stateBag = new AgentSessionStateBag(); + var wireId = "mcpr_" + new string('e', 50); + + ToolApprovalIdMap.Record(stateBag, wireId, "ficc_x", callId: "call_xyz", name: string.Empty, argumentsJson: "{}"); + + Assert.Null(ToolApprovalIdMap.ResolveEntry(stateBag, wireId)); + } + // ── input_file data-URI decoding (TryDecodeTextDataUri) ── [Fact] diff --git a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OutputConverterTests.cs b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OutputConverterTests.cs index e66d6dbb4c..883da91171 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OutputConverterTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OutputConverterTests.cs @@ -84,7 +84,7 @@ public class OutputConverterTests } [Fact] - public async Task ConvertUpdatesToEventsAsync_FunctionCall_EmitsFunctionCallEventsAsync() + public async Task ConvertUpdatesToEventsAsync_FunctionCallWithoutResult_EmitsFunctionCallWireItemAsync() { var (stream, _) = CreateTestStream(); var update = new AgentResponseUpdate @@ -99,10 +99,12 @@ public class OutputConverterTests events.Add(evt); } - // Should have: FuncAdded, ArgsDelta, ArgsDone, FuncDone, Completed - Assert.IsType(events[0]); + // A lone FunctionCallContent (no paired FunctionResultContent) is the + // OpenAI Responses encoding of a HITL request: the caller is expected to + // resume with a function_call_output for this call_id. + Assert.Single(events.OfType()); + Assert.Single(events.OfType()); Assert.IsType(events[^1]); - Assert.True(events.Count >= 4, $"Expected at least 4 events for function call, got {events.Count}"); } [Fact] @@ -302,6 +304,8 @@ public class OutputConverterTests events.Add(evt); } + // FCC closes any in-flight assistant message, then emits its own function_call + // wire item. Result: 2 output items (text message + function_call). Assert.Equal(2, events.OfType().Count()); Assert.Equal(2, events.OfType().Count()); Assert.IsType(events[^1]); @@ -328,7 +332,7 @@ public class OutputConverterTests // G-04 [Fact] - public async Task ConvertUpdatesToEventsAsync_FunctionCallWithEmptyCallId_GeneratesCallIdAsync() + public async Task ConvertUpdatesToEventsAsync_FunctionCallWithEmptyCallId_DoesNotEmitWireItemAsync() { var (stream, _) = CreateTestStream(); var update = new AgentResponseUpdate @@ -342,12 +346,14 @@ public class OutputConverterTests events.Add(evt); } - Assert.Contains(events, e => e is ResponseOutputItemAddedEvent); + // Empty CallId is invalid for the wire format; emission is skipped. + Assert.DoesNotContain(events, e => e is ResponseOutputItemAddedEvent); + Assert.IsType(events[^1]); } // G-05 [Fact] - public async Task ConvertUpdatesToEventsAsync_MultipleFunctionCalls_EmitsSeparateBuildersAsync() + public async Task ConvertUpdatesToEventsAsync_MultipleFunctionCallsWithoutResults_EachEmitsWireItemAsync() { var (stream, _) = CreateTestStream(); var updates = new[] @@ -362,7 +368,10 @@ public class OutputConverterTests events.Add(evt); } + // Each lone FCC surfaces as its own function_call wire item (HITL request shape). Assert.Equal(2, events.OfType().Count()); + Assert.Equal(2, events.OfType().Count()); + Assert.IsType(events[^1]); } // H-02 @@ -537,7 +546,7 @@ public class OutputConverterTests // K-03 [Fact] - public async Task ConvertUpdatesToEventsAsync_FunctionResultContent_IsSkippedWithNoEventsAsync() + public async Task ConvertUpdatesToEventsAsync_FunctionResultWithoutMatchingCall_EmitsFunctionCallOutputAsync() { var (stream, _) = CreateTestStream(); var update = new AgentResponseUpdate { Contents = [new FunctionResultContent("call_1", "result data")] }; @@ -548,8 +557,82 @@ public class OutputConverterTests events.Add(evt); } - Assert.Single(events); - Assert.IsType(events[0]); + // A FunctionResultContent always emits a function_call_output wire item; pairing + // with a function_call (if any) is established by call_id at the wire layer. + Assert.Single(events.OfType()); + Assert.Single(events.OfType()); + Assert.IsType(events[^1]); + } + + // K-04 + [Fact] + public async Task ConvertUpdatesToEventsAsync_FunctionCallThenResult_EmitsPairedItemsAsync() + { + var (stream, _) = CreateTestStream(); + var updates = new[] + { + new AgentResponseUpdate { Contents = [new FunctionCallContent("call_1", "search", new Dictionary { ["q"] = "weather" })] }, + new AgentResponseUpdate { Contents = [new FunctionResultContent("call_1", "sunny")] }, + }; + + var events = new List(); + await foreach (var evt in OutputConverter.ConvertUpdatesToEventsAsync(ToAsync(updates), stream)) + { + events.Add(evt); + } + + // Issue #5662: function_call and function_call_output must both surface as + // wire items so Azure's stored conversation has a paired call+output and + // resume via previous_response_id works. + Assert.Equal(2, events.OfType().Count()); + Assert.Equal(2, events.OfType().Count()); + Assert.Single(events.OfType()); + Assert.IsType(events[^1]); + } + + // K-05: An FCC with an empty CallId is dropped without disturbing in-flight text. + [Fact] + public async Task ConvertUpdatesToEventsAsync_FunctionCallEmptyCallIdMidText_PreservesTextBoundaryAsync() + { + var (stream, _) = CreateTestStream(); + var updates = new[] + { + new AgentResponseUpdate { MessageId = "msg_1", Contents = [new MeaiTextContent("Hello, ")] }, + new AgentResponseUpdate { Contents = [new FunctionCallContent(string.Empty, "skipped", new Dictionary())] }, + new AgentResponseUpdate { MessageId = "msg_1", Contents = [new MeaiTextContent("world!")] }, + }; + + var events = new List(); + await foreach (var evt in OutputConverter.ConvertUpdatesToEventsAsync(ToAsync(updates), stream)) + { + events.Add(evt); + } + + // The FCC is skipped (no CallId), and because we now validate CallId before + // closing the in-flight assistant message, both text deltas land in the same + // output item — only one message-added event is emitted. + Assert.Single(events.OfType()); + Assert.Equal(2, events.OfType().Count()); + Assert.IsType(events[^1]); + } + + // K-06: FRC string results are emitted as raw text on the wire (not JSON-quoted). + [Fact] + public async Task ConvertUpdatesToEventsAsync_FunctionResultStringPayload_EmittedAsRawTextAsync() + { + var (stream, _) = CreateTestStream(); + var update = new AgentResponseUpdate { Contents = [new FunctionResultContent("call_1", "sunny")] }; + + var events = new List(); + await foreach (var evt in OutputConverter.ConvertUpdatesToEventsAsync(ToAsync(new[] { update }), stream)) + { + events.Add(evt); + } + + var added = Assert.Single(events.OfType()); + var output = Assert.IsType(added.Item); + // String FRC payloads must not be double-encoded — `sunny`, not `"sunny"`. + Assert.Equal("sunny", output.Output.ToString()); } // L-01 @@ -666,6 +749,7 @@ public class OutputConverterTests events.Add(evt); } + // text(msg_1) → function_call(call_1) → text(msg_2): three output items. Assert.Equal(3, events.OfType().Count()); } @@ -729,6 +813,7 @@ public class OutputConverterTests events.Add(evt); } + // Three output items: function_call(call_1), text(msg_1), function_call(call_2). Assert.Equal(3, events.OfType().Count()); } @@ -821,9 +906,10 @@ public class OutputConverterTests events.Add(evt); } - // Should have: 4 workflow actions + 1 function call + 1 text message = 6 output items + // Workflow actions: 4. Lone FCC: 1 (function_call wire item). + // Text message: 1. Total output items: 6. Assert.Equal(6, events.OfType().Count()); - Assert.Contains(events, e => e is ResponseFunctionCallArgumentsDoneEvent); + Assert.Single(events.OfType()); Assert.Contains(events, e => e is ResponseTextDeltaEvent); Assert.IsType(events[^1]); } @@ -960,11 +1046,10 @@ public class OutputConverterTests events.Add(evt); } - // Workflow actions: invoked triage, completed triage, invoked expert, completed expert = 4 - // Content items: 1 function call, 1 text message = 2 - // Total output items: 6 + // Workflow actions: 4. Lone FCC: 1 (function_call wire item). + // Text message: 1. Total output items: 6. Assert.Equal(6, events.OfType().Count()); - Assert.Contains(events, e => e is ResponseFunctionCallArgumentsDoneEvent); + Assert.Single(events.OfType()); // Two text deltas for the two streaming chunks Assert.Equal(2, events.OfType().Count()); Assert.IsType(events[^1]); diff --git a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OutputConverterWorkflowTests.cs b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OutputConverterWorkflowTests.cs index f991744292..5cd73404f8 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OutputConverterWorkflowTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OutputConverterWorkflowTests.cs @@ -185,10 +185,10 @@ public class OutputConverterWorkflowTests } // Workflow actions: 4 (2 invoked + 2 completed) - // Content: 1 reasoning + 1 function call + 1 text message = 3 + // Content: 1 reasoning + 1 function_call (lone FCC = HITL request) + 1 text = 3 // Total: 7 output items Assert.Equal(7, events.OfType().Count()); - Assert.Contains(events, e => e is ResponseFunctionCallArgumentsDoneEvent); + Assert.Single(events.OfType()); Assert.Equal(2, events.OfType().Count()); Assert.IsType(events[^1]); }