2 Commits

• .NET: Issue 5662 (#5668)

  * 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_<callId>) and Name hardcoded to 'mcp_approval'. This (a) broke Azure Conversations pairing because the persisted function_call had CallId <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<FunctionCallContent,...> 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 <bentho@microsoft.com>
  Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

  Ben Thomas · 2026-05-07 00:30:41 +00:00
  cdd80c61ac
• .NET: Add dedicated Foundry.Hosting UnitTest project (#5592)

  * Foundry.Hosting.UnitTests: extract project from Foundry.UnitTests
  
  Move all Hosting/* tests, three toolbox TestData JSONs, and the FakeAuthenticationTokenProvider/HttpHandlerAssert/TestDataUtil helpers (trimmed to toolbox getters) into a new Microsoft.Agents.AI.Foundry.Hosting.UnitTests project. Add it to the slnx and grant the new assembly InternalsVisibleTo from Microsoft.Agents.AI.Foundry and Microsoft.Agents.AI.Foundry.Hosting.
  
  * Foundry.Hosting.UnitTests: align namespaces to assembly name
  
  Rename namespaces from Microsoft.Agents.AI.Foundry.UnitTests(.Hosting) to Microsoft.Agents.AI.Foundry.Hosting.UnitTests across all moved tests, the duplicated helpers, and the trimmed TestDataUtil. Also fixes the prior namespace inconsistency in FoundryToolboxTests.
  
  * Foundry.Hosting.UnitTests: split WorkflowIntegrationTests by SUT
  
  Replace the WorkflowIntegrationTests file (an IT-named file inside a UT project) with two SUT-focused files plus a shared test-doubles file:
  
  - AgentFrameworkResponseHandlerWorkflowTests.cs - the 5 handler-driven tests that exercise AgentFrameworkResponseHandler with a real workflow agent.
  - OutputConverterWorkflowTests.cs - the 5 OutputConverter tests driven by hand-crafted update sequences mirroring real workflow patterns.
  - WorkflowTestAgents.cs - StreamingTextAgent and ThrowingStreamingAgent extracted as internal types used by both files.
  
  * Foundry.UnitTests: trim Hosting-related conditionals and dead testdata
  
  Now that Hosting tests live in their own project:
  - drop the Compile Remove guard for the Hosting subfolder,
  - drop the .NETCoreApp-only PackageReferences (Azure.AI.AgentServer.Responses, Microsoft.AspNetCore.TestHost, OpenTelemetry, OpenTelemetry.Exporter.InMemory),
  - drop the conditional ProjectReference to Microsoft.Agents.AI.Foundry.Hosting,
  - delete the three Toolbox JSON files and the matching Toolbox getters in TestDataUtil.
  
  * Foundry.Hosting.UnitTests: drop redundant 'using Microsoft.Agents.AI.Foundry.Hosting'
  
  The new project namespace is Microsoft.Agents.AI.Foundry.Hosting.UnitTests, which already brings the parent Microsoft.Agents.AI.Foundry.Hosting namespace into scope. The explicit using statement is therefore redundant (IDE0005). Caught by 'dotnet format --verify-no-changes' running on Linux against the .NET 10 SDK.
  
  * Foundry.Hosting: drop InternalsVisibleTo to Foundry.UnitTests
  
  The non-hosting Foundry.UnitTests project no longer holds any Hosting tests after the split, so it doesn't need access to internal types in Microsoft.Agents.AI.Foundry.Hosting. Only Microsoft.Agents.AI.Foundry.Hosting.UnitTests needs it.
  
  * Foundry.Hosting: rename DelegatingResponsesClient to UserAgentResponsesClient
  
  Address westey-m's review feedback on PR #5453: `Delegating*` is conventionally reserved for inheritable base classes (mirroring `DelegatingHandler`) where consumers override one or two members. This polyfill is sealed and only injects the User-Agent supplement, so the new name reflects its actual purpose.
  
  Renamed via `git mv` to preserve history:
  * `src/Microsoft.Agents.AI.Foundry.Hosting/DelegatingResponsesClient.cs` to `UserAgentResponsesClient.cs`
  * `tests/Microsoft.Agents.AI.Foundry.Hosting.UnitTests/DelegatingResponsesClientTests.cs` to `UserAgentResponsesClientTests.cs`
  
  Class, constructor, and all references updated across:
  * `src/.../UserAgentResponsesClient.cs` (class + constructor + internal log message)
  * `src/.../ServiceCollectionExtensions.cs` (cref + type check + instantiation)
  * `src/.../HostedAgentUserAgentPolicy.cs` (cref)
  * `tests/Foundry.UnitTests/RequestOptionsExtensionsTests.cs` (comment)
  * `tests/Foundry.Hosting.UnitTests/UserAgentResponsesClientTests.cs` (class + cref + instantiations)

  Roger Barreto · 2026-04-30 21:09:25 +00:00
  fb97e93a01