Add A2b: HandoffWorkflowBuilder variant of A2 — completes cleanly

Agent-Logs-Url: https://github.com/microsoft/agent-framework/sessions/e7100d8a-ca6d-48e6-8210-78ba33dbd53c

Co-authored-by: lokitoth <6936551+lokitoth@users.noreply.github.com>

docs/working/issue-5350-root-cause-validation-plan.md

@@ -84,6 +84,7 @@ not reproduce in any of them; A2 did however uncover a *separate, unrelated* bug
 |------|--------------------------------------------------------------------------------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------------------------------------------------------------------------|
 | A1   | ~~Use `ChatClientAgent` + `ApprovalRequiredAIFunction` bound into a `WorkflowBuilder`~~ — **covered by test #7 in this PR; passes.**                  | Was the largest gap to the OP's repro. Closed.                                                                                                                                                                       | OP hypothesis disproved — `TARC.ToolCall is FunctionCallContent` post-resume, and the tool is actually invoked exactly once when the approval response is sent. |
 | A2   | Multi-agent variant: same as #7 but with the agent inside a `GroupChatBuilder` (the OP's actual orchestration) with `RoundRobinGroupChatManager`.     | If group chat re-encodes TARC as part of `ChatMessage.Contents` (`AIContent` polymorphism is two-deep through `ToolApprovalRequestContent`), one branch may resolve and the other not.                              | OP hypothesis disproved — `TARC.ToolCall is FunctionCallContent` post-resume. **But:** sending the approval response surfaces a *different* real bug — `FunctionInvokingChatClient.ExtractAndRemoveApprovalRequestsAndResponses` throws `ArgumentException: An item with the same key has already been added. Key: ficc_call-1`. Pinned by test as documented misbehavior. |
+| A2b  | Same as A2 but using `HandoffWorkflowBuilder` (initial agent has the approval tool, a no-op peer agent makes the handoff graph valid; the mock chat client never emits a `handoff_to_*` call). | If the duplicate-key bug from A2 is broader than `RoundRobinGroupChatManager` and lives in the shared `AIAgentHostExecutor` / `ChatProtocolExecutor` path, the handoff workflow should hit it too. | OP hypothesis disproved — `TARC.ToolCall is FunctionCallContent` post-resume **and** the workflow completes cleanly: tool invoked exactly once, zero errors, zero executor failures. The duplicate-key bug from A2 does **not** occur on the handoff path, narrowing it to the group-chat-specific orchestration. |
 | A3   | Same as #7 but with the checkpoint `JsonElement` round-tripped through `string` + `JsonDocument.Parse` between commit and retrieve (`StringRoundTripJsonStore`), emulating the SQL `nvarchar` hop in the OP's Dapper store.                                                                                | The OP uses Dapper + SQL Server. If the column / driver round-trip preserves ordering, this should be identity-preserving — but if it reorders metadata properties, the `$type` discriminator can be moved out of first position, which then requires `AllowOutOfOrderMetadataProperties = true`. | OP hypothesis disproved — byte-preserving string round-trip is identity-preserving for the relevant payload; `TARC.ToolCall is FunctionCallContent` post-resume; tool invoked exactly once. The OP's storage layer would have to *perturb* the JSON (e.g. reorder metadata) for this to reproduce. |
 | A4   | Same as #7 but with non-default `JsonSerializerOptions` (`JsonSerializerDefaults.Web`, no AIJsonUtilities resolver) passed as `customOptions` to `CheckpointManager.CreateJson`.                                                                                                                          | `JsonMarshaller.LookupTypeInfo` only goes to the external options when the internal chain doesn't know about the type. For most cases this won't trigger, but it's worth confirming that supplying a custom `JsonSerializerOptions` does not silently displace the internal chain.                            | OP hypothesis disproved — custom external options that DO NOT know about `AIContent` types are correctly ignored for known types; the internal `WorkflowsJsonUtilities.DefaultOptions` chain wins. `TARC.ToolCall is FunctionCallContent` post-resume; tool invoked exactly once. |
 

dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/ToolApprovalRequestCheckpointReproTests.cs

@@ -539,6 +539,51 @@ public class ToolApprovalRequestCheckpointReproTests
             });
     }
 
+    /// <summary>
+    /// Track A2b — same as A2, but using a <c>HandoffWorkflowBuilder</c> instead of a group
+    /// chat. The initial agent is the same approval-tool-equipped <see cref="ChatClientAgent"/>
+    /// as test #7; a second dummy agent is registered solely so the handoff graph has a valid
+    /// peer (the mock chat client never emits a <c>handoff_to_*</c> call, so the workflow stays
+    /// on the initial agent). This isolates whether anything in the handoff-specific
+    /// orchestration (handoff tool injection, <c>HandoffMessagesFilter</c>, the
+    /// <c>HandoffStartExecutor</c>/<c>HandoffEndExecutor</c> wrap-up) perturbs the
+    /// <see cref="ToolApprovalRequestContent"/> payload across a JSON checkpoint resume.
+    ///
+    /// <para><b>Finding:</b> the handoff path is fully clean. The <see cref="FunctionCallContent"/>
+    /// type is preserved after resume <i>and</i> approving the resumed request completes the
+    /// workflow without errors and invokes the wrapped <see cref="AIFunction"/> exactly once.
+    /// This is in contrast to A2 (group chat), which preserves the type but crashes on
+    /// approval with a duplicate-key <see cref="ArgumentException"/> in FICC. The OP's
+    /// hypothesis (<c>TARC.ToolCall is not FunctionCallContent</c>) still does not reproduce.
+    /// </para>
+    /// </summary>
+    [Fact]
+    public async Task Repro_5350_A2b_HandoffWorkflowBuilder_WithApprovalRequiredTool_JsonCheckpointResume_PreservesFunctionCallContentAndInvokesToolAsync()
+    {
+        ReproHarness harness = new();
+
+        // Second agent is a no-op peer required to give the handoff graph a valid target.
+        // The mock chat client only ever emits a FunctionCallContent for GetWeather, so the
+        // initial agent never actually hands off; the second agent is never invoked.
+        MockChatClient peerChatClient = new((messages, options) =>
+            new ChatResponse(new ChatMessage(ChatRole.Assistant, "(unused peer)")));
+        ChatClientAgent peerAgent = new(
+            peerChatClient,
+            instructions: "Unused peer agent.",
+            name: "PeerAgent");
+
+        Workflow workflow = AgentWorkflowBuilder
+            .CreateHandoffBuilderWith(harness.Agent)
+            .WithHandoff(harness.Agent, peerAgent)
+            .Build();
+
+        await RunReproAsync(
+            workflow,
+            harness,
+            CheckpointManager.CreateJson(new InMemoryJsonStore()),
+            scenarioName: "A2b (handoff)");
+    }
+
     /// <summary>
     /// Track A3 — same as the maximal repro (test #7) but the checkpoint <see cref="JsonElement"/>
     /// is round-tripped through <see cref="JsonElement.GetRawText"/> + <see cref="JsonDocument.Parse(string,JsonDocumentOptions)"/>