Merge branch 'main' into copilot/fix-flaky-workflow-test

dotnet/src/Microsoft.Agents.AI.Workflows/SwitchBuilder.cs

@@ -36,9 +36,13 @@ public sealed class SwitchBuilder
         Throw.IfNull(executors);
 
         HashSet<int> indicies = [];
+        int executorIndex = 0;
 
         foreach (ExecutorBinding executor in executors)
         {
+            // Explicit name: null element inside the collection argument.
+            Throw.IfNull(executor, $"{nameof(executors)}[{executorIndex++}]");
+
             if (!this._executorIndicies.TryGetValue(executor.Id, out int index))
             {
                 index = this._executors.Count;
@@ -64,8 +68,13 @@ public sealed class SwitchBuilder
     {
         Throw.IfNull(executors);
 
+        int executorIndex = 0;
+
         foreach (ExecutorBinding executor in executors)
         {
+            // Explicit name: null element inside the collection argument.
+            Throw.IfNull(executor, $"{nameof(executors)}[{executorIndex++}]");
+
             if (!this._executorIndicies.TryGetValue(executor.Id, out int index))
             {
                 index = this._executors.Count;

dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowBuilderExtensions.cs

@@ -25,7 +25,11 @@ public static class WorkflowBuilderExtensions
     /// <param name="target">The target executor to which messages will be forwarded.</param>
     /// <returns>The updated <see cref="WorkflowBuilder"/> instance.</returns>
     public static WorkflowBuilder ForwardMessage<TMessage>(this WorkflowBuilder builder, ExecutorBinding source, ExecutorBinding target)
-        => builder.ForwardMessage<TMessage>(source, [target], condition: null);
+    {
+        Throw.IfNull(target, nameof(target));
+
+        return builder.ForwardMessage<TMessage>(source, [target], condition: null);
+    }
 
     /// <summary>
     /// Adds edges to the workflow that forward messages of the specified type from the source executor to
@@ -52,6 +56,8 @@ public static class WorkflowBuilderExtensions
     /// <returns>The updated <see cref="WorkflowBuilder"/> instance.</returns>
     public static WorkflowBuilder ForwardMessage<TMessage>(this WorkflowBuilder builder, ExecutorBinding source, IEnumerable<ExecutorBinding> targets, Func<TMessage, bool>? condition = null)
     {
+        Throw.IfNull(builder);
+        Throw.IfNull(source);
         Throw.IfNull(targets);
 
         Func<object?, bool> predicate = WorkflowBuilder.CreateConditionFunc<TMessage>(IsAllowedTypeAndMatchingCondition)!;
@@ -62,14 +68,16 @@ public static class WorkflowBuilderExtensions
         if (targets is ICollection<ExecutorBinding> { Count: 1 })
 #endif
         {
-            return builder.AddEdge(source, targets.First(), predicate);
+            return builder.AddEdge(source, Throw.IfNull(targets.First(), nameof(targets)), predicate);
         }
 
-        return builder.AddSwitch(source, (switch_) => switch_.AddCase(predicate, targets));
+        return builder.AddSwitch(source, (switch_) => switch_.AddCase(predicate, targets.Select(ValidateTarget)));
 
         // The reason we can check for "not null" here is that CreateConditionFunc<T> will do the correct unwrapping
         // logic for PortableValues.
         bool IsAllowedTypeAndMatchingCondition(TMessage? message) => message != null && (condition == null || condition(message));
+
+        ExecutorBinding ValidateTarget(ExecutorBinding target) => Throw.IfNull(target, nameof(targets));
     }
 
     /// <summary>
@@ -81,7 +89,11 @@ public static class WorkflowBuilderExtensions
     /// <param name="target">The target executor to which messages, except those of type <typeparamref name="TMessage"/>, will be forwarded.</param>
     /// <returns>The updated <see cref="WorkflowBuilder"/> instance with the added edges.</returns>
     public static WorkflowBuilder ForwardExcept<TMessage>(this WorkflowBuilder builder, ExecutorBinding source, ExecutorBinding target)
-        => builder.ForwardExcept<TMessage>(source, [target]);
+    {
+        Throw.IfNull(target, nameof(target));
+
+        return builder.ForwardExcept<TMessage>(source, [target]);
+    }
 
     /// <summary>
     /// Adds edges from the specified source to the provided executors, excluding messages of a specified type.
@@ -93,6 +105,8 @@ public static class WorkflowBuilderExtensions
     /// <returns>The updated <see cref="WorkflowBuilder"/> instance with the added edges.</returns>
     public static WorkflowBuilder ForwardExcept<TMessage>(this WorkflowBuilder builder, ExecutorBinding source, IEnumerable<ExecutorBinding> targets)
     {
+        Throw.IfNull(builder);
+        Throw.IfNull(source);
         Throw.IfNull(targets);
 
         Func<object?, bool> predicate = WorkflowBuilder.CreateConditionFunc<TMessage>((Func<object?, bool>)IsAllowedType)!;
@@ -103,14 +117,16 @@ public static class WorkflowBuilderExtensions
         if (targets is ICollection<ExecutorBinding> { Count: 1 })
 #endif
         {
-            return builder.AddEdge(source, targets.First(), predicate);
+            return builder.AddEdge(source, Throw.IfNull(targets.First(), nameof(targets)), predicate);
         }
 
-        return builder.AddSwitch(source, (switch_) => switch_.AddCase(predicate, targets));
+        return builder.AddSwitch(source, (switch_) => switch_.AddCase(predicate, targets.Select(ValidateTarget)));
 
         // The reason we can check for "null" here is that CreateConditionFunc<T> will do the correct unwrapping
         // logic for PortableValues.
         static bool IsAllowedType(object? message) => message is null;
+
+        ExecutorBinding ValidateTarget(ExecutorBinding target) => Throw.IfNull(target, nameof(targets));
     }
 
     /// <summary>
@@ -129,6 +145,7 @@ public static class WorkflowBuilderExtensions
     {
         Throw.IfNull(builder);
         Throw.IfNull(source);
+        Throw.IfNull(executors);
 
         HashSet<string> seenExecutors = [source.Id];
 

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

@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft. All rights reserved.
 
 using System;
+using System.Collections.Generic;
 using FluentAssertions;
 
 namespace Microsoft.Agents.AI.Workflows.UnitTests;
@@ -157,4 +158,301 @@ public partial class WorkflowBuilderSmokeTests
         workflow3.Name.Should().Be("Named Only");
         workflow3.Description.Should().BeNull();
     }
+
+    [Fact]
+    public void ForwardMessage_WithSingleTarget_CreatesDirectEdge()
+    {
+        // Arrange
+        NoOpExecutor source = new("start");
+        NoOpExecutor target = new("target");
+
+        // Act
+        Workflow workflow = new WorkflowBuilder(source.Id)
+            .ForwardMessage<string>(source, target)
+            .Build();
+
+        // Assert
+        Edge edge = GetSingleEdge(workflow, source.Id);
+        edge.Kind.Should().Be(EdgeKind.Direct);
+        edge.DirectEdgeData.Should().NotBeNull();
+        edge.DirectEdgeData!.SourceId.Should().Be(source.Id);
+        edge.DirectEdgeData!.SinkId.Should().Be(target.Id);
+        edge.DirectEdgeData.Condition.Should().NotBeNull();
+        edge.DirectEdgeData.Condition!("message").Should().BeTrue();
+        edge.DirectEdgeData.Condition!(42).Should().BeFalse();
+        edge.DirectEdgeData.Condition!(null).Should().BeFalse();
+    }
+
+    [Fact]
+    public void ForwardMessage_WithMultipleTargets_CreatesFanOutEdge()
+    {
+        // Arrange
+        NoOpExecutor source = new("start");
+        NoOpExecutor target1 = new("target1");
+        NoOpExecutor target2 = new("target2");
+
+        // Act
+        Workflow workflow = new WorkflowBuilder(source.Id)
+            .ForwardMessage<string>(source, [target1, target2], message => message == "match")
+            .Build();
+
+        // Assert
+        Edge edge = GetSingleEdge(workflow, source.Id);
+        edge.Kind.Should().Be(EdgeKind.FanOut);
+        edge.FanOutEdgeData.Should().NotBeNull();
+        edge.FanOutEdgeData!.SourceId.Should().Be(source.Id);
+        edge.FanOutEdgeData!.SinkIds.Should().Equal([target1.Id, target2.Id]);
+        edge.FanOutEdgeData.EdgeAssigner.Should().NotBeNull();
+        edge.FanOutEdgeData.EdgeAssigner!("match", 2).Should().Equal([0, 1]);
+        edge.FanOutEdgeData.EdgeAssigner!("other", 2).Should().BeEmpty();
+        edge.FanOutEdgeData.EdgeAssigner!(42, 2).Should().BeEmpty();
+    }
+
+    [Fact]
+    public void ForwardExcept_WithSingleTarget_CreatesDirectEdge()
+    {
+        // Arrange
+        NoOpExecutor source = new("start");
+        NoOpExecutor target = new("target");
+
+        // Act
+        Workflow workflow = new WorkflowBuilder(source.Id)
+            .ForwardExcept<string>(source, target)
+            .Build();
+
+        // Assert
+        Edge edge = GetSingleEdge(workflow, source.Id);
+        edge.Kind.Should().Be(EdgeKind.Direct);
+        edge.DirectEdgeData.Should().NotBeNull();
+        edge.DirectEdgeData!.SourceId.Should().Be(source.Id);
+        edge.DirectEdgeData!.SinkId.Should().Be(target.Id);
+        edge.DirectEdgeData.Condition.Should().NotBeNull();
+        edge.DirectEdgeData.Condition!("message").Should().BeFalse();
+        edge.DirectEdgeData.Condition!(42).Should().BeTrue();
+        edge.DirectEdgeData.Condition!(null).Should().BeTrue();
+    }
+
+    [Fact]
+    public void ForwardExcept_WithMultipleTargets_CreatesFanOutEdge()
+    {
+        // Arrange
+        NoOpExecutor source = new("start");
+        NoOpExecutor target1 = new("target1");
+        NoOpExecutor target2 = new("target2");
+
+        // Act
+        Workflow workflow = new WorkflowBuilder(source.Id)
+            .ForwardExcept<string>(source, [target1, target2])
+            .Build();
+
+        // Assert
+        Edge edge = GetSingleEdge(workflow, source.Id);
+        edge.Kind.Should().Be(EdgeKind.FanOut);
+        edge.FanOutEdgeData.Should().NotBeNull();
+        edge.FanOutEdgeData!.SourceId.Should().Be(source.Id);
+        edge.FanOutEdgeData!.SinkIds.Should().Equal([target1.Id, target2.Id]);
+        edge.FanOutEdgeData.EdgeAssigner.Should().NotBeNull();
+        edge.FanOutEdgeData.EdgeAssigner!(42, 2).Should().Equal([0, 1]);
+        edge.FanOutEdgeData.EdgeAssigner!("message", 2).Should().BeEmpty();
+    }
+
+    [Fact]
+    public void AddChain_CreatesSequentialDirectEdges()
+    {
+        // Arrange
+        NoOpExecutor source = new("start");
+        NoOpExecutor middle = new("middle");
+        NoOpExecutor end = new("end");
+
+        // Act
+        Workflow workflow = new WorkflowBuilder(source.Id)
+            .AddChain(source, [middle, end])
+            .Build();
+
+        // Assert
+        Edge firstEdge = GetSingleEdge(workflow, source.Id);
+        firstEdge.Kind.Should().Be(EdgeKind.Direct);
+        firstEdge.DirectEdgeData!.SourceId.Should().Be(source.Id);
+        firstEdge.DirectEdgeData.SinkId.Should().Be(middle.Id);
+
+        Edge secondEdge = GetSingleEdge(workflow, middle.Id);
+        secondEdge.Kind.Should().Be(EdgeKind.Direct);
+        secondEdge.DirectEdgeData!.SourceId.Should().Be(middle.Id);
+        secondEdge.DirectEdgeData.SinkId.Should().Be(end.Id);
+    }
+
+    [Fact]
+    public void AddChain_WhenExecutorRepeats_Throws()
+    {
+        // Arrange
+        NoOpExecutor source = new("start");
+        NoOpExecutor middle = new("middle");
+
+        // Act
+        Action act = () => new WorkflowBuilder(source.Id)
+            .AddChain(source, [middle, source]);
+
+        // Assert
+        act.Should().Throw<ArgumentException>()
+            .WithParameterName("executors");
+    }
+
+    [Fact]
+    public void AddExternalCall_CreatesRequestPortAndRoundTripEdges()
+    {
+        // Arrange
+        const string PortId = "port1";
+        NoOpExecutor source = new("start");
+
+        // Act
+        Workflow workflow = new WorkflowBuilder(source.Id)
+            .AddExternalCall<string, int>(source, PortId)
+            .Build();
+
+        // Assert
+        workflow.Ports.Should().ContainKey(PortId);
+        workflow.Ports[PortId].Request.Should().Be(typeof(string));
+        workflow.Ports[PortId].Response.Should().Be(typeof(int));
+        workflow.ExecutorBindings.Should().ContainKey(PortId);
+
+        Edge requestEdge = GetSingleEdge(workflow, source.Id);
+        requestEdge.Kind.Should().Be(EdgeKind.Direct);
+        requestEdge.DirectEdgeData!.SourceId.Should().Be(source.Id);
+        requestEdge.DirectEdgeData.SinkId.Should().Be(PortId);
+
+        Edge responseEdge = GetSingleEdge(workflow, PortId);
+        responseEdge.Kind.Should().Be(EdgeKind.Direct);
+        responseEdge.DirectEdgeData!.SourceId.Should().Be(PortId);
+        responseEdge.DirectEdgeData.SinkId.Should().Be(source.Id);
+    }
+
+    [Fact]
+    public void AddSwitch_CreatesFanOutEdgeWithCasesAndDefault()
+    {
+        // Arrange
+        NoOpExecutor source = new("start");
+        NoOpExecutor stringTarget = new("string-target");
+        NoOpExecutor intTarget = new("int-target");
+        NoOpExecutor defaultTarget = new("default-target");
+
+        // Act
+        Workflow workflow = new WorkflowBuilder(source.Id)
+            .AddSwitch(source, switchBuilder => switchBuilder
+                .AddCase<string>(message => message == "match", [stringTarget])
+                .AddCase<int>(message => message > 0, [intTarget])
+                .WithDefault([defaultTarget]))
+            .Build();
+
+        // Assert
+        Edge edge = GetSingleEdge(workflow, source.Id);
+        edge.Kind.Should().Be(EdgeKind.FanOut);
+        edge.FanOutEdgeData.Should().NotBeNull();
+        edge.FanOutEdgeData!.SourceId.Should().Be(source.Id);
+        edge.FanOutEdgeData!.SinkIds.Should().Equal([stringTarget.Id, intTarget.Id, defaultTarget.Id]);
+        edge.FanOutEdgeData.EdgeAssigner.Should().NotBeNull();
+        edge.FanOutEdgeData.EdgeAssigner!("match", 3).Should().Equal([0]);
+        edge.FanOutEdgeData.EdgeAssigner!(2, 3).Should().Equal([1]);
+        edge.FanOutEdgeData.EdgeAssigner!("other", 3).Should().Equal([2]);
+    }
+
+    [Fact]
+    public void ForwardMessage_InvalidArguments_Throw()
+    {
+        // Arrange
+        WorkflowBuilder builder = new("start");
+        NoOpExecutor source = new("start");
+        NoOpExecutor target = new("target");
+
+        // Act/Assert
+        Assert.Throws<ArgumentNullException>(() => ((WorkflowBuilder)null!).ForwardMessage<string>(source, target));
+        Assert.Throws<ArgumentNullException>("source", () => builder.ForwardMessage<string>(null!, target));
+        Assert.Throws<ArgumentNullException>("target", () => builder.ForwardMessage<string>(source, (ExecutorBinding)null!));
+        Assert.Throws<ArgumentNullException>("targets", () => builder.ForwardMessage<string>(source, (IEnumerable<ExecutorBinding>)null!));
+        Assert.Throws<ArgumentNullException>("targets", () => builder.ForwardMessage<string>(source, [target, null!]));
+        Assert.Throws<ArgumentException>("targets", () => builder.ForwardMessage<string>(source, []));
+    }
+
+    [Fact]
+    public void ForwardExcept_InvalidArguments_Throw()
+    {
+        // Arrange
+        WorkflowBuilder builder = new("start");
+        NoOpExecutor source = new("start");
+        NoOpExecutor target = new("target");
+
+        // Act/Assert
+        Assert.Throws<ArgumentNullException>(() => ((WorkflowBuilder)null!).ForwardExcept<string>(source, target));
+        Assert.Throws<ArgumentNullException>("source", () => builder.ForwardExcept<string>(null!, target));
+        Assert.Throws<ArgumentNullException>("target", () => builder.ForwardExcept<string>(source, (ExecutorBinding)null!));
+        Assert.Throws<ArgumentNullException>("targets", () => builder.ForwardExcept<string>(source, (IEnumerable<ExecutorBinding>)null!));
+        Assert.Throws<ArgumentNullException>("targets", () => builder.ForwardExcept<string>(source, [target, null!]));
+        Assert.Throws<ArgumentException>("targets", () => builder.ForwardExcept<string>(source, []));
+    }
+
+    [Fact]
+    public void AddChain_InvalidArguments_Throw()
+    {
+        // Arrange
+        WorkflowBuilder builder = new("start");
+        NoOpExecutor source = new("start");
+        NoOpExecutor target = new("target");
+        NoOpExecutor otherTarget = new("other-target");
+
+        // Act/Assert
+        Assert.Throws<ArgumentNullException>(() => ((WorkflowBuilder)null!).AddChain(source, [target]));
+        Assert.Throws<ArgumentNullException>("source", () => builder.AddChain(null!, [target]));
+        Assert.Throws<ArgumentNullException>("executors", () => builder.AddChain(source, null!));
+        Assert.Throws<ArgumentNullException>("executors", () => builder.AddChain(source, [target, null!]));
+        Assert.Throws<ArgumentException>("executors", () => builder.AddChain(source, [target, source]));
+        Assert.Throws<ArgumentException>("executors", () => builder.AddChain(source, [target, otherTarget, target]));
+    }
+
+    [Fact]
+    public void AddExternalCall_InvalidArguments_Throw()
+    {
+        // Arrange
+        WorkflowBuilder builder = new("start");
+        NoOpExecutor source = new("start");
+
+        // Act/Assert
+        Assert.Throws<ArgumentNullException>(() => ((WorkflowBuilder)null!).AddExternalCall<string, int>(source, "port"));
+        Assert.Throws<ArgumentNullException>("source", () => builder.AddExternalCall<string, int>(null!, "port"));
+        Assert.Throws<ArgumentNullException>("portId", () => builder.AddExternalCall<string, int>(source, null!));
+    }
+
+    [Fact]
+    public void AddSwitch_InvalidArguments_Throw()
+    {
+        // Arrange
+        WorkflowBuilder builder = new("start");
+        NoOpExecutor source = new("start");
+
+        // Act/Assert
+        Assert.Throws<ArgumentNullException>(() => ((WorkflowBuilder)null!).AddSwitch(source, _ => { }));
+        Assert.Throws<ArgumentNullException>("source", () => builder.AddSwitch(null!, _ => { }));
+        Assert.Throws<ArgumentNullException>("configureSwitch", () => builder.AddSwitch(source, null!));
+        Assert.Throws<ArgumentException>("targets", () => builder.AddSwitch(source, _ => { }));
+        Assert.Throws<ArgumentException>("targets", () => builder.AddSwitch(source, switchBuilder => switchBuilder.AddCase<string>(_ => true, [])));
+    }
+
+    [Fact]
+    public void SwitchBuilder_InvalidArguments_Throw()
+    {
+        // Arrange
+        SwitchBuilder switchBuilder = new();
+        NoOpExecutor target = new("target");
+
+        // Act/Assert
+        Assert.Throws<ArgumentNullException>("predicate", () => switchBuilder.AddCase<string>(null!, [target]));
+        Assert.Throws<ArgumentNullException>("executors", () => switchBuilder.AddCase<string>(_ => true, null!));
+        Assert.Throws<ArgumentNullException>("executors[1]", () => switchBuilder.AddCase<string>(_ => true, [target, null!]));
+        Assert.Throws<ArgumentNullException>("executors", () => switchBuilder.WithDefault(null!));
+        Assert.Throws<ArgumentNullException>("executors[1]", () => switchBuilder.WithDefault([target, null!]));
+    }
+
+    /// <summary>
+    /// Gets the only edge emitted by the specified workflow source.
+    /// </summary>
+    private static Edge GetSingleEdge(Workflow workflow, string sourceId)
+        => workflow.Edges[sourceId].Should().ContainSingle().Subject;
 }