.NET: Issue 5662

[alliscode]

This pull request makes significant improvements to how tool approval requests and responses are handled, ensuring that function call details are preserved and correctly reconstructed across HTTP turns. It also updates the logic to suppress internal function call events from being emitted on the wire, preventing orphaned function calls and related errors. Comprehensive tests are added and updated to validate these behaviors.

Closes #5662

[github-actions]

Automated Code Review

Reviewers: 4 | Confidence: 88%

✓ Correctness

The PR replaces a broken placeholder FunctionCallContent in MCP approval responses with a faithfully reconstructed one (preserving CallId, Name, Arguments from the original request), and suppresses FunctionCallContent at the wire to prevent orphan function_call items. The logic is internally consistent: Record stores the details, ResolveEntry retrieves them, and the reconstructed FCC matches what the backend expects. Tests are updated to reflect the new behavior. No correctness issues found.

✓ Security Reliability

The changes replace a fallback-to-wire-id pattern with a fail-fast approach when no approval mapping exists, and suppress FunctionCallContent from wire output to prevent orphan function_call items. The code is well-structured from a security and reliability perspective: the state bag is server-side (not client-controllable), SHA-256 prevents wire-id collision attacks, deserialization failures are defensively handled, and the exception message with the wire id poses no injection risk in .NET's exception handling model. No security or reliability issues identified.

✓ Test Coverage

The PR updates tests comprehensively for both the FunctionCallContent wire suppression and the richer ToolApprovalIdMap round-trip. However, there is a clearly duplicated test (same scenario tested twice with trivially different wire ids), and edge cases for null arguments in the approval entry and the defensive JsonException path in TryLoadMap are not exercised.

✗ Design Approach

I found two design-level problems. First, the new approval map persists a custom Dictionary<string, ApprovalEntry> directly into AgentSessionStateBag, but that bag serializes values using the framework's default JSON metadata, which only source-generates a fixed set of known exchange types; in AOT/no-reflection configurations this makes any session containing an approval request unserializable. Second, the new hard failure on missing approval mappings turns pre-existing or state-lost approval requests into top-level request failures, even though the new TryLoadMap path explicitly expects older-format state and drops it.

---

Automated review by alliscode's agents

[Copilot]

Pull request overview

This PR updates the Foundry Hosting wire converters to (1) suppress internal FunctionCallContent events from being emitted to the Responses API stream and (2) persist enough tool-call details during mcp_approval_request emission to reconstruct a lossless ToolApprovalResponseContent when the corresponding mcp_approval_response arrives on a later HTTP turn (issue #5662).

Changes:

• Suppress FunctionCallContent (and related function-call wire events) in OutputConverter to prevent orphaned function_call items and subsequent resume failures.
• Persist a full approval mapping entry (AF request id + original function call details) in session state and reconstruct the original FunctionCallContent on inbound approval responses.
• Update/add unit tests to validate suppression behavior and lossless reconstruction, and to enforce fail-fast behavior when approval mappings are missing.

Show a summary per file

File	Description
dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/OutputConverter.cs	Stops emitting function-call wire items and records full approval mappings when emitting mcp_approval_request.
dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/InputConverter.cs	Reconstructs ToolApprovalResponseContent using stored mapping entries and throws when the mapping is missing.
dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/ToolApprovalIdMap.cs	Changes stored state from wire-id→string to wire-id→approval entry (includes call id/name/args) and adds entry resolution helpers.
dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OutputConverterTests.cs	Updates expectations to ensure function-call items are suppressed at the wire and adjusts output-item counts accordingly.
dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/OutputConverterWorkflowTests.cs	Updates workflow test expectations to account for suppressed function-call wire items.
dotnet/tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/InputConverterTests.cs	Adds/updates tests for lossless tool-call reconstruction and missing-mapping failure behavior.

Copilot's findings

• Files reviewed: 6/6 changed files
• Comments generated: 1