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

16 KiB
Raw Permalink Blame History

Issue #5350 — Root-cause validation plan

Issue: .NET: [Bug]: Checkpoint round-trip loses ToolApprovalRequestContent.ToolCall concrete type (FunctionCallContent → base ToolCallContent), breaking FICC approval resume

Status of repro: A focused repro test class dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/ToolApprovalRequestCheckpointReproTests.cs was added covering seven progressively more end-to-end variants of the path the issue describes — including a maximal end-to-end test that uses a real ChatClientAgent driven by FunctionInvokingChatClient with an ApprovalRequiredAIFunction. All seven tests pass on main, consistently across 5 back-to-back runs, i.e. the bug as described does not reproduce at any of the layers exercised here:

# Test What it exercises Result
1 Repro_5350_ToolApprovalRequestContent_DirectJsonMarshallerRoundtrip_PreservesFunctionCallContent JsonMarshaller.Marshal/Marshal<T> on ToolApprovalRequestContent directly (same options chain as CheckpointManager.CreateJson) Pass
2 Repro_5350_ToolApprovalRequestContent_WrappedInPortableValue_PreservesFunctionCallContent Same, wrapped in a PortableValue (as PortableMessageEnvelope would store it) Pass
3 Repro_5350_ToolApprovalRequestContent_AsExternalRequestData_PreservesFunctionCallContent Same, as the Data payload of an ExternalRequest Pass
4 Repro_5350_DirectJsonMarshallerRoundtrip_IsDeterministic 25× repetition of #1 to rule out flakiness / JIT order Pass
5 Repro_5350_CaptureWireFormat_ForInspection Captures the on-the-wire shape so we can compare against the OP's SQL row Pass
6 Repro_5350_EndToEnd_JsonCheckpointResume_PreservesFunctionCallContentAsync Full CheckpointManager.CreateJson(InMemoryJsonStore) → RunStreamingAsync → SuperStep checkpoint → ResumeStreamingAsync cycle with a RequestPort<TARC, TARR> Pass
7 Repro_5350_EndToEnd_ChatClientAgent_WithApprovalRequiredTool_JsonCheckpointResume_PreservesFunctionCallContentAndInvokesToolAsync Maximal: ChatClientAgent over a MockChatClient with an ApprovalRequiredAIFunction, single-agent WorkflowBuilder (no orchestration), CheckpointManager.CreateJson(InMemoryJsonStore). Asserts both that the resumed RequestInfoEvent.Request still carries a FunctionCallContent AND that approving the request actually invokes the underlying AIFunction and lets the workflow continue. Pass

This is consistent with @lokitoth's second comment:

Looking at the MEAI types, it does have [JsonDerivedType(typeof(FunctionCallContent), "functionCall")] set on it, and has had it that way for some time, so it is unlikely to be the root cause. […] JsonMarshaller […] takes an optional JsonSerializationOptions provided by the user […] but this is used only if the internal one (via WorkflowsJsonUtilities, which chains to AgentAbstractionsJsonUtilities, which then chains to the AIJsonUtilities class) [is missing the type].

Inspection of the actual wire format produced by JsonMarshaller for a ToolApprovalRequestContent containing a FunctionCallContent confirms the discriminator is emitted:

// TARC at top level
{
  "toolCall": {
    "$type": "functionCall",      // ← polymorphism discriminator present
    "name": "DoTheThing",
    "arguments": { "x": 42 },
    "informationalOnly": false,
    "callId": "call-1"
  },
  "requestId": "req-1"
}

// TARC inside a PortableValue (= shape stored by PortableMessageEnvelope)
{
  "typeId": {
    "assemblyName": "Microsoft.Extensions.AI.Abstractions, Version=10.5.0.0, …",
    "typeName": "Microsoft.Extensions.AI.ToolApprovalRequestContent"
  },
  "value": {
    "toolCall": {
      "$type": "functionCall",
      
    },
    "requestId": "req-1"
  }
}

So the OP's stated root-cause hypothesis — "AIContent/ToolCallContent/ FunctionCallContent are missing [JsonPolymorphic]/[JsonDerivedType], or CheckpointManager.CreateJson does not pull from AIJsonUtilities.DefaultOptions" — is not what is producing the failure described in the issue. The annotations exist, the resolver chain wires them through, and the discriminator does survive both the direct JsonMarshaller round-trip and a real Run → checkpoint → Resume cycle for a RequestPort whose request type is ToolApprovalRequestContent.

The remaining sections lay out, in priority order, the work needed to identify the actual root cause. The plan deliberately keeps the failing repro from above as the baseline ("this is what works") and walks outward from it toward the OP's reported scenario, varying one dimension at a time.

Plan

Track A — Reproduce in a configuration closer to the OP's pattern "B"

The OP's repro path differs from the new tests in three concrete ways. Tests #7 (maximal repro) and #8#10 (A2 / A3 / A4 below) close all four gaps. The OP's specific hypothesis (TARC.ToolCall is not FunctionCallContent after resume) does not reproduce in any of them; A2 did however uncover a separate, unrelated bug.

Step Variable that changes vs. the passing tests in this repo Why it matters Outcome
A1 Use ChatClientAgent + ApprovalRequiredAIFunction bound into a WorkflowBuildercovered 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.

Track B — Validate the wire format the OP actually persists

Once Track A has reproduced (or has clearly failed to reproduce) the symptom, ask the OP for one of the following, in order of preference:

  1. The raw JsonElement.GetRawText() they pass to their SQL store on commit, and the raw string they read back on RetrieveCheckpointAsync — for the exact checkpoint that contains the failing ToolApprovalRequestContent. This tells us in a single round-trip whether:
    • the $type discriminator is present on commit (rules out write-side bug),
    • the $type discriminator survives the SQL round-trip (rules out store bug),
    • the discriminator is in metadata-first position (rules out the AllowOutOfOrderMetadataProperties story).
  2. A repro PR/gist that runs against CheckpointManager.CreateJson(...) with an in-memory JsonCheckpointStore shim that mirrors how their SQL subclass passes bytes through. The OP already offered this in the issue body ("I can produce a minimal standalone repro against a fake IChatClient if useful — let me know.") — taking them up on it short-circuits a lot of guessing.

Track C — Defense-in-depth fixes worth landing regardless of root cause

These are addressed in the OP's "Asks" section (asks 24) and are useful documentation/ergonomics improvements even if the root cause turns out to be in the OP's store implementation:

  1. Doc-only: in the XML docs for CheckpointManager.CreateJson and on the JsonCheckpointStore base class, explicitly state that the internal options chain through WorkflowsJsonUtilities → AgentAbstractionsJsonUtilities → AIJsonUtilities, and that user-supplied customOptions are consulted only when the internal chain has no TypeInfo for a requested type.
  2. Doc-only: document the contract that any user JsonCheckpointStore subclass MUST preserve the exact byte sequence of the JsonElement it is given. If the underlying store reorders JSON metadata, the consumer must opt in by setting AllowOutOfOrderMetadataProperties = true on a JsonSerializerOptions passed as customOptions to CheckpointManager.CreateJson(...).
  3. Optional, only if Track A reproduces: add a regression test mirroring the failing path under dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests so the fix is locked in.

Track D — Reject the OP's hypothesis (if not already done)

The combination of:

  • the polymorphism annotations existing in Microsoft.Extensions.AI for some time (per @lokitoth),
  • the resolver chain in WorkflowsJsonUtilities.CreateDefaultOptions() already putting AgentAbstractionsJsonUtilities.DefaultOptions.TypeInfoResolver first (which itself puts AIJsonUtilities.DefaultOptions.TypeInfoResolver first),
  • the wire format captured above showing "$type": "functionCall" is present,
  • the full Run → checkpoint → Resume test in this PR passing for a plain RequestPort<TARC, ToolApprovalResponseContent> workflow, and
  • the maximal ChatClientAgent + FunctionInvokingChatClient + ApprovalRequiredAIFunction test in this PR also passing — including the assertion that the wrapped AIFunction is actually invoked exactly once after approval and that the workflow then receives the resulting FunctionResultContent and produces a final assistant message,

is sufficient to disprove the OP's stated hypothesis. Once Track A or Track B identifies the actual cause, the issue should be updated with a brief explanation of why the original guess was incorrect, so future readers don't re-tread it.

Pointers for the next investigator

  • Repro tests: dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/ToolApprovalRequestCheckpointReproTests.cs
  • Marshaller under test: dotnet/src/Microsoft.Agents.AI.Workflows/Checkpointing/JsonMarshaller.cs
  • Where CheckpointManager.CreateJson enters that path: dotnet/src/Microsoft.Agents.AI.Workflows/CheckpointManager.cs
  • Options chain (TARC ⇒ resolver): dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowsJsonUtilities.csdotnet/src/Microsoft.Agents.AI.Abstractions/AgentAbstractionsJsonUtilities.csMicrosoft.Extensions.AI.AIJsonUtilities.DefaultOptions
  • PortableValue-side serialization (this is where value.Value.GetType() is used on write, which is the most plausible internal place a polymorphism bug could hide, but the wire capture above shows it is not the cause in the TARC-as-RequestPort scenario): dotnet/src/Microsoft.Agents.AI.Workflows/Checkpointing/PortableValueConverter.cs
  • Likely next code to inspect for Track A1/A2 (agent-host serialization path): dotnet/src/Microsoft.Agents.AI.Workflows/AIAgentHostExecutor*.cs and whatever turns the FICC-generated TARC into something AIAgentHostExecutor stores in its state bag.