Conversation
moonbox3
added
documentation
There was a problem hiding this comment.
Pull request overview
Adds durable-workflow runner support for AddSwitch / selector-based fan-out routing by propagating FanOutEdgeData.EdgeAssigner through durable graph analysis and routing, plus tests and a new console sample demonstrating multi-way routing.
Changes:
- Capture fan-out selector metadata (
EdgeAssigner+ orderedSinkIds) during workflow analysis and expose it viaWorkflowGraphInfo. - Build a selector-aware durable fan-out router that evaluates the assigner and routes only to selected targets.
- Add unit + integration coverage and a new
09_SwitchRoutingdurable console sample.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| dotnet/src/Microsoft.Agents.AI.DurableTask/Workflows/WorkflowGraphInfo.cs | Adds FanOutRoutings to carry selector routing metadata into the durable runtime. |
| dotnet/src/Microsoft.Agents.AI.DurableTask/Workflows/WorkflowAnalyzer.cs | Captures FanOutEdgeData.EdgeAssigner and sink ordering into FanOutRoutings. |
| dotnet/src/Microsoft.Agents.AI.DurableTask/Workflows/EdgeRouters/DurableEdgeMap.cs | Uses captured routing to construct a selector-aware durable fan-out router. |
| dotnet/src/Microsoft.Agents.AI.DurableTask/Workflows/EdgeRouters/DurableFanOutEdgeRouter.cs | Implements selector evaluation and selective routing for fan-out edges. |
| dotnet/src/Microsoft.Agents.AI.DurableTask/Workflows/EdgeRouters/DurableDirectEdgeRouter.cs | Deduplicates message deserialization by calling DurableSerialization.DeserializeMessage. |
| dotnet/src/Microsoft.Agents.AI.DurableTask/Workflows/DurableSerialization.cs | Introduces shared message deserialization helper with trimming/AOT suppressions. |
| dotnet/src/Microsoft.Agents.AI.DurableTask/Logs.cs | Adds log events for selector matches and selector evaluation failures. |
| dotnet/tests/Microsoft.Agents.AI.DurableTask.UnitTests/Workflows/DurableEdgeMapSwitchTests.cs | New unit tests validating switch/default behavior and non-selector fan-out behavior. |
| dotnet/tests/Microsoft.Agents.AI.DurableTask.IntegrationTests/WorkflowConsoleAppSamplesValidation.cs | Adds end-to-end console sample validation for switch routing. |
| dotnet/samples/04-hosting/DurableWorkflows/ConsoleApps/09_SwitchRouting/README.md | Documents the new switch-routing sample and contrasts it with conditional edges. |
| dotnet/samples/04-hosting/DurableWorkflows/ConsoleApps/09_SwitchRouting/Program.cs | Console app wiring a durable workflow using AddSwitch. |
| dotnet/samples/04-hosting/DurableWorkflows/ConsoleApps/09_SwitchRouting/Executors.cs | Executors used by the sample (parse expense, route to approval path). |
| dotnet/samples/04-hosting/DurableWorkflows/ConsoleApps/09_SwitchRouting/09_SwitchRouting.csproj | New sample project definition and references. |
| dotnet/agent-framework-dotnet.slnx | Adds the new sample project to the solution. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated Code Review
Reviewers: 5 | Confidence: 87%
✓ Correctness
The PR correctly implements AddSwitch support in the durable workflow runner. The WorkflowAnalyzer now captures EdgeAssigner and SinkIds from FanOutEdgeData, DurableEdgeMap builds a selector-aware DurableFanOutEdgeRouter when an assigner is present, and the router faithfully mirrors the in-process FanOutEdgeRunner's evaluation logic (calling the assigner with the deserialized message and target count, then routing to selected indices). The deserialization helper is cleanly deduplicated, error handling is appropriate for a durable context, and bounds checking is defensively added. Unit and integration tests validate all routing branches.
✓ Security Reliability
This PR adds AddSwitch support to the durable workflow runner with well-structured routing logic. The code follows established patterns (catch-and-log-and-drop for routing failures, consistent with DurableDirectEdgeRouter), includes proper bounds checking on assigner-returned indices, and uses safe System.Text.Json deserialization with types known at startup. No security vulnerabilities, resource leaks, or unsafe deserialization patterns were found.
✓ Test Coverage
The PR adds good unit tests covering the core switch routing scenarios (matching case, default fallback, plain fan-out) plus an integration test that exercises all three branches end-to-end. The tests use meaningful assertions that verify specific routing outcomes rather than just 'no exception'. However, the error-handling path in DurableFanOutEdgeRouter (where the assigner throws an exception and the message is silently dropped) has no test coverage, which is notable given it represents a message-loss scenario.
✓ Failure Modes
The PR correctly implements switch routing support for the durable workflow runner, faithfully mirroring the in-process FanOutEdgeRunner's target selection logic. The code is well-structured with proper deduplication of deserialization logic. One operational concern: when the edge assigner throws (e.g., due to deserialization failure or a user predicate exception), the exception is caught and swallowed with only a Warning log, causing the orchestration to hang indefinitely. The in-process runtime re-throws in this scenario, giving the caller a clear failure signal. Since this is the sole routing path for the source executor, there is no fallback.
✓ Design Approach
I found one blocking design regression. The durable routing change models selector-based fan-out at the source-executor level instead of the edge level, which causes durable workflows to silently drop other outgoing edges from the same source (for example, a switch plus an additional direct edge, or multiple selector fan-outs).
Suggestions
- Consider whether the catch block in DurableFanOutEdgeRouter.RouteMessage (line 101-104) should re-throw (or mark the orchestration as failed) rather than swallowing the exception, since unlike DurableDirectEdgeRouter where other edges can still fire, this is the only routing path and swallowing causes a silent workflow hang. The in-process FanOutEdgeRunner re-throws in this scenario (FanOutEdgeRunner.cs:58-61).
Automated review by kshyju's agents
Add the UTF-8 BOM required by dotnet/.editorconfig (charset = utf-8-bom) to Executors.cs and Program.cs, which was lost during EOL normalization and caused the dotnet format CHARSET check to fail. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add the UTF-8 BOM required by dotnet/.editorconfig (charset = utf-8-bom) to the new test file, which caused the dotnet format CHARSET check to fail. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Automated Code Review
Reviewers: 5 | Confidence: 90%
✓ Correctness
The PR correctly implements AddSwitch support in the durable workflow runner. The routing logic in DurableEdgeMap properly handles multiple switches from the same source, sibling direct edges, and shared targets through a well-designed selector-sink counting mechanism. DurableFanOutEdgeRouter correctly evaluates the assigner with per-index bounds checking, mirroring the in-process FanOutEdgeRunner's behavior. The DurableSerialization.DeserializeMessage refactoring is a clean deduplication. All previously-resolved review comments have been addressed. No correctness bugs found.
✓ Security Reliability
This PR is well-structured from a security and reliability perspective. Exception handling around deserialization and selector evaluation is appropriate - failures are caught, logged, and don't crash the orchestration. Per-index range checking ensures a single bad index doesn't drop valid deliveries. The
selectorSinkCountslogic correctly handles sibling edges coexisting with selector targets. No injection risks, resource leaks, or unguarded failure modes were identified.
✓ Test Coverage
The test coverage for the new AddSwitch/fan-out selector feature is strong overall, covering the core routing logic, edge cases like duplicate/out-of-range indices, sibling edges, and multiple switches. However, there are two notable gaps: (1) no test verifies the behavior when the edge assigner throws an exception (the catch block in DurableFanOutEdgeRouter lines 96-100), and (2) no test exercises a conditional sibling edge alongside a switch (the EdgeConditions lookup at DurableEdgeMap line 143). Both are new code paths that could silently fail without test coverage.
✓ Failure Modes
The PR introduces switch/selector support for durable fan-out routing. The implementation follows established patterns (log-and-drop on evaluation failure) consistent with the existing DurableDirectEdgeRouter. The selector evaluation, per-index range checking, sibling-edge wiring, and multiple-switch support are well-structured and thoroughly tested. No new silent failure modes beyond the pre-existing design pattern are introduced.
✗ Design Approach
The routing changes mostly line up with the PR goal, but there is one design-level correctness gap: once this PR allows the same target to be reached twice from the same source (for example, a selected switch case plus a sibling direct edge), the durable runner still interprets that as fan-in and aggregates those two deliveries into a single executor invocation. That does not match the in-process contract, where aggregation is reserved for explicit fan-in edges.
Flagged Issues
- When a selector switch case and a sibling direct edge both route to the same target, WorkflowAnalyzer.AddSuccessorsFromEdge records duplicate predecessor entries (WorkflowAnalyzer.cs:144-152), causing IsFanInExecutor to return true (DurableEdgeMap.cs:235-237) and CollectExecutorInputs to aggregate the two messages into one array (DurableWorkflowRunner.cs:319-323). In-process execution only aggregates behind explicit FanInEdgeData (Execution/EdgeMap.cs:40-44; WorkflowBuilder.cs:503-505), so this changes execution semantics—the target runs once with aggregated input instead of twice independently.
Suggestions
- Add a unit test verifying that when the edge assigner throws an exception, no message is delivered to any target and no exception escapes (exercising the catch block at DurableFanOutEdgeRouter.cs:96-100). A test like
RouteMessage_SelectorThrows_DoesNotDeliverOrThrowwith an assigner(_, _) => throw new InvalidOperationException("boom")would cover this.
Automated review by kshyju's agents
|
Flagged issue When a selector switch case and a sibling direct edge both route to the same target, WorkflowAnalyzer.AddSuccessorsFromEdge records duplicate predecessor entries (WorkflowAnalyzer.cs:144-152), causing IsFanInExecutor to return true (DurableEdgeMap.cs:235-237) and CollectExecutorInputs to aggregate the two messages into one array (DurableWorkflowRunner.cs:319-323). In-process execution only aggregates behind explicit FanInEdgeData (Execution/EdgeMap.cs:40-44; WorkflowBuilder.cs:503-505), so this changes execution semantics—the target runs once with aggregated input instead of twice independently. Source: automated DevFlow PR review |
A single source can reach the same target through more than one edge (for example a switch case plus a sibling direct edge to the same executor). Counting those repeated deliveries as multiple predecessors made the target look like a fan-in point, so the durable runner aggregated the deliveries into one invocation instead of running the target once per delivery as in-process does. Count distinct predecessor sources so only genuine fan-in (multiple distinct sources) is aggregated. Adds regression tests for the selector-throws path, a conditional sibling edge alongside a switch, and fan-in detection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Thanks for the thorough review. Addressed in efe227c: Design Approach (fan-in mis-aggregation): Confirmed and fixed. Test coverage: Added the suggested Full unit suite green (164/164). |
Motivation & Context
This PR adds support for
AddSwitch(and target-selecting fan-out edges) in the durable workflow runner. A switch lets a workflow route a message to the first matching case among several branches, or to a default branch when none match, which is a natural fit for multi-way routing such as approval flows.AddSwitchis closely related toAddEdge(..., condition:), which the durable runner already supports. The difference is in how each models branching:AddEdgewith a condition attaches an independent boolean to each edge. Every edge whose condition is true is traversed, so multiple branches can run.AddSwitchmodels mutually exclusive, first-match-wins routing with a single default, so exactly one branch runs. Under the hood it compiles to a single fan-out edge whoseEdgeAssignerselects the matching case's target(s).The durable graph analyzer already read the per-edge condition on
DirectEdgeDatabut did not carry theEdgeAssignerfromFanOutEdgeData, so a switch lost its case selection and forwarded the message to every target instead of the selected one. This change threads the assigner through the durable routing layer so case selection is honored, matching the in-process runtime's behavior.Description & Review Guide
What are the major changes?
WorkflowAnalyzernow captures theEdgeAssignerandSinkIdsfromFanOutEdgeDatainto a newWorkflowGraphInfo.FanOutRoutingsmap.DurableEdgeMapbuilds a selector-awareDurableFanOutEdgeRouterwhen an assigner is present, wrapping one direct router per ordered sink.DurableFanOutEdgeRouterevaluates the assigner against the deserialized message and routes to only the selected targets, mirroring thein-process
FanOutEdgeRunner. With no assigner it keeps the existing forward-to-all behavior.DurableSerialization.DeserializeMessage.09_SwitchRoutingconsole sample (expense-approval routing) plus its samples-validation integration test.What is the impact of these changes?
What do you want reviewers to focus on?
DurableFanOutEdgeRouter.RouteMessageand the assigner/sink ordering inDurableEdgeMap, i.e. that the durable path faithfully mirrors the in-processFanOutEdgeRunner.Related Issue
Fixes #6722
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.