.NET: keep the first request seen for a repeated request ID - #7947
Conversation
AIAgentUnservicedRequestsCollector.ProcessAIContents threw when a CallId or RequestId was already recorded. It runs once per streamed AgentResponseUpdate, so an agent that re-emits one call across updates never reached its request: the throw escaped InvokeAgentAsync and ended the run in ExecutorFailedEvent and WorkflowErrorEvent. AIAgentHostExecutor and HandoffAgentExecutor both build this collector, so both were affected. A repeated ID inside one run is one pending request. The collector now keeps the first content seen, matching AIContentExternalHandler, which treats a repeat as an idempotent re-emission, and ApprovalResponseBindingChatClient, which binds a response against the first request recorded for the ID. A later content that is not the same request cannot be serviced alongside the recorded one, so SubmitAsync reports those IDs in a WorkflowWarningEvent instead of dropping them silently. HandoffAgentExecutor collected handoff candidates on every emission, so one re-emitted handoff call read as two competing handoffs. It now skips a candidate whose CallId is already collected.
There was a problem hiding this comment.
Pull request overview
Coalesces repeated request IDs in .NET agent streams while preserving the first request and warning on conflicting requests.
Changes:
- Implements first-wins request collection with collision warnings.
- Deduplicates repeated streamed handoff calls.
- Adds regression coverage for function, approval, and handoff requests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
AIAgentUnservicedRequestsCollector.cs |
Coalesces repeated IDs and reports conflicts. |
HandoffAgentExecutor.cs |
Deduplicates streamed handoff requests. |
AIAgentHostExecutorTests.cs |
Tests warnings and first-wins behavior. |
HandoffAgentExecutorTests.cs |
Tests repeated handoff handling. |
WorkflowHostSmokeTests.cs |
Tests end-to-end repeated request handling. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
The handoff candidate filter coalesced on call ID alone, so two different handoff targets sharing one call ID collapsed into the first and the duplicate handoff warning no longer fired for them. It now compares the target name alongside the ID, which is the same rule the collector applies to a repeated call, so only a genuine re-emission is coalesced.
…eated-request-ids
microsoft#7938 removed FluentAssertions from the tree over licensing and rewrote the other tests in this project, so the assertions added here no longer compiled once main was merged in. The merge was clean because neither side touched the same lines: main deleted the using and the package reference, this branch only added Should() calls under them. The because strings become comments, which is what the rest of the project does now. Also record why handoff target selection stays last-wins while the coalescing beside it is first-wins. The two answer different questions, and with the re-emission filter in front of it the selection only ever sees genuinely competing targets.
|
Pushed The tests no longer compiled. #7938 removed FluentAssertions from the tree over licensing and rewrote the other tests in this project. The merge was clean because neither side touched the same lines: main deleted the Also recorded why handoff target selection stays last-wins while the coalescing beside it is first-wins, since the difference is otherwise the kind of thing that reads as an oversight. They answer different questions: coalescing asks whether this is the same request seen twice, where the first copy is real and the rest are re-emissions; the handoff selection asks which of several genuinely different targets the agent meant, which is real ambiguity in the output. With the Two things I would rather raise than have you find: The Scoping against |
Motivation & Context
A repeated
CallIdorRequestIdin an agent's stream took down the whole workflow run.AIAgentUnservicedRequestsCollector.ProcessAIContentsthrewInvalidOperationExceptionwhen an ID was already in its dictionary. It is called once per streamedAgentResponseUpdate, so an agent that re-emits the same call across updates never reached its request: the throw escapedInvokeAgentAsyncand the run ended inExecutorFailedEventplusWorkflowErrorEvent. BothAIAgentHostExecutorandHandoffAgentExecutorbuild this collector, and it is whatworkflow.AsAIAgent()runs on.Repeating an ID inside one run identifies one pending request, not two. Two layers already say so and neither fails:
AIContentExternalHandler.ProcessRequestContentAsynctreats a repeat as an idempotent re-emission, andApprovalResponseBindingChatClientkeeps the first request recorded for an ID and binds the response against a snapshot of it.Description & Review Guide
ProcessAIContentskeeps the first content seen for a request ID instead of throwing, so all three layers now agree on one rule and the response a caller approves is bound to the request they were shown.A later content under that ID that is not the same request — a different
ToolCall.CallIdunder one approval ID, or a different functionNameunder one call ID — is still worth reporting, since only one of them can be serviced.SubmitAsyncemits a singleWorkflowWarningEventnaming those IDs rather than dropping them silently. A plain re-emission reports nothing.HandoffAgentExecutor.CollectHandoffRequestsFilterhad the same defect in its own collection: it appended on every emission, so one re-emitted handoff call producedDuplicate handoff requests in single turn ([handoff_to_x, handoff_to_x]). It now skips a candidate whoseCallIdis already collected, so that warning fires only for genuinely different handoff targets.A run survives a repeated request ID and raises the request once. No public API changed:
AIAgentUnservicedRequestsCollectoris internal and no signature moved.Two things worth deciding rather than skimming. First, a genuinely distinct request colliding on an outstanding ID goes from fatal to dropped-with-a-warning; the reachable shape of that is an empty-string
CallIdfallback such asGitHubCopilotAgent.cs'stoolStart.Data?.ToolCallId ?? string.Empty, where two tool starts key on"". Second,Magentic/StreamingToolCallResultPairMatcherstill throws on a repeatedCallIdin a stream, soSpecialized/holds two answers to the same question until someone reconciles them; that path is separate and untouched here.Whether first-wins is the rule you want, and whether the warning belongs in
SubmitAsyncor should stay silent.Related Issue
Fixes #7946
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.