.NET: Fix: OutputConverter.FunctionResultContent creates OutputItemFunction…#6247
.NET: Fix: OutputConverter.FunctionResultContent creates OutputItemFunction…#6247enhendrickson wants to merge 3 commits into
Conversation
…ToolCallOutput with null id - Root cause: 2-arg constructor leaves Id property null (get-only) - Solution: Use ResponseEventStream.OutputItemFunctionCallOutput convenience method - Impact: Prevents HTTP 500 on function_call_output persistence - Ref: issue microsoft/agent-framework (filed separately)
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Refactors function tool call output emission to use a new helper method OutputItemFunctionCallOutput on the stream, replacing inline construction and emission of added/done events.
Changes:
- Replaces manual
OutputItemFunctionToolCallOutputconstruction andAddOutputItem/EmitAdded/EmitDonecalls with a single helper method that yields the events.
moonbox3
left a comment
There was a problem hiding this comment.
Is there a unit test we should be adding/updating related to this change?
…have non-null Id Adds a unit test that verifies the OutputItemFunctionToolCallOutput emitted by the FunctionResultContent case has a non-null, non-empty Id with the correct 'fco_' prefix. Regression guard for issue microsoft#6245: the old code used the 2-arg constructor (OutputItemFunctionToolCallOutput(callId, output)), which leaves the Id property null (get-only). A null id causes the Responses API storage layer to reject the function_call_output with HTTP 500. The fix uses the convenience method ResponseEventStream.OutputItemFunctionCallOutput(), which calls IdGenerator.NewFunctionCallOutputItemId() internally to produce a proper id.
|
@microsoft-github-policy-service agree company="Provisions Group" |
good question, I just amended/pushed the one that was there @moonbox3 |
Resolves merge conflicts by accepting origin/main versions of: - OutputConverter.cs: keeps the explanatory documentation added upstream - OutputConverterTests.cs: uses the more comprehensive K-06e regression test from main
|
I also accepted someone else's changes that came in via MC - I'm guessing it was an agent coded solution to my bug, but either way, it was G2G... this PR is now ready to merge with both the fixed bug and updated test |
Summary
When a Foundry-hosted agent invokes a tool, the
OutputConverter.FunctionResultContentcase createsOutputItemFunctionToolCallOutputitems with a nullIdfield. This causes the Responses API storage layer to reject the persistence request with HTTP 500, followed by a 409 on retry (confirming partial resource creation). WhileAzure.AI.AgentServer.Responses v1.0.0-beta.5gracefully catches this and emits aresponse.failedevent instead of breaking the SSE stream, the root cause remains unfixed.Root Cause
In OutputConverter.cs (~line 450-460), the
FunctionResultContentcase:Problem: The 2-arg constructor
OutputItemFunctionToolCallOutput(callId, output)doesn't set theIdproperty (it's get-only). When the item is serialized for persistence (usingModelReaderWriterOptions.Json), theidfield is written asnull, which violates the Responses API schema and causes HTTP 500.Why it happens: The 2-arg public constructor is for deserialization. The internal 9-arg constructor (used by the SDK's convenience method) has the proper id setup:
Reproduction
AddFoundryToolboxes()orAsOpenAIResponseTool()/storage/responsesreturns HTTP 500response.failedevent (beta.5)Smoke test evidence (from this session):
Expected Behavior
Tool invocations should persist cleanly and the agent should return results to the client without storage errors.
Current Workaround (beta.5)
Upgrade to
Azure.AI.AgentServer.Responses v1.0.0-beta.5, which containsResponseOrchestrator.CreateStreamingAsyncexception handling that catches persistence failures and emitsresponse.failedwith code "storage_error". The SSE stream completes gracefully instead of terminating abruptly.Client-side mitigation: listen for
response.failedevents withcode="storage_error"and display a user-friendly message.Suggested Fix
Replace the manual 2-arg constructor usage with the SDK's convenience method, which handles id generation correctly:
The convenience method uses
IdGenerator.NewFunctionCallOutputItemId()internally, which generates proper "fco_{partitionKey}{entropy}" formatted ids.Code Location
dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/OutputConverter.csOutputConverterFunctionResultContentcase)Environment
/responsesprotocol (streaming SSE)Related
externalLoop+ApprovalRequiredAIFunctionleaves danglingfunction_callin Azure Conversations API, breaks resume #5662 via deferred emissionagent-framework-ag-ui(Foundry project endpoint) #5941I have submitted a ticket for this: #6245