diff --git a/dotnet/src/Microsoft.Agents.Workflows/ExecutorIsh.cs b/dotnet/src/Microsoft.Agents.Workflows/ExecutorIsh.cs index c303286acb..dd026f541d 100644 --- a/dotnet/src/Microsoft.Agents.Workflows/ExecutorIsh.cs +++ b/dotnet/src/Microsoft.Agents.Workflows/ExecutorIsh.cs @@ -102,13 +102,22 @@ public sealed class ExecutorIsh : _ => throw new InvalidOperationException($"Unknown ExecutorIsh type: {this.ExecutorType}") }; + internal object? RawData => this.ExecutorType switch + { + Type.Unbound => this._idValue, + Type.Executor => this._executorValue, + Type.InputPort => this._inputPortValue, + Type.Agent => this._aiAgentValue, + _ => throw new InvalidOperationException($"Unknown ExecutorIsh type: {this.ExecutorType}") + }; + /// /// Gets the registration details for the current executor. /// /// The returned registration depends on the type of the executor. If the executor is unbound, an /// is thrown. For other executor types, the registration includes the /// appropriate ID, type, and provider based on the executor's configuration. - internal ExecutorRegistration Registration => new(this.Id, this.RuntimeType, this.ExecutorProvider); + internal ExecutorRegistration Registration => new(this.Id, this.RuntimeType, this.ExecutorProvider, this.RawData); private System.Type RuntimeType => this.ExecutorType switch { diff --git a/dotnet/src/Microsoft.Agents.Workflows/ExecutorRegistration.cs b/dotnet/src/Microsoft.Agents.Workflows/ExecutorRegistration.cs index d45a126472..bb1e060107 100644 --- a/dotnet/src/Microsoft.Agents.Workflows/ExecutorRegistration.cs +++ b/dotnet/src/Microsoft.Agents.Workflows/ExecutorRegistration.cs @@ -7,12 +7,14 @@ using ExecutorFactoryF = System.Func; namespace Microsoft.Agents.Workflows; -internal class ExecutorRegistration(string id, Type executorType, ExecutorFactoryF provider) +internal class ExecutorRegistration(string id, Type executorType, ExecutorFactoryF provider, object? rawData) { public string Id { get; } = Throw.IfNullOrEmpty(id); public Type ExecutorType { get; } = Throw.IfNull(executorType); public ExecutorFactoryF Provider { get; } = Throw.IfNull(provider); + internal object? RawExecutorishData { get; } = rawData; + public override string ToString() => $"{this.ExecutorType.Name}({this.Id})"; private Executor CheckId(Executor executor) diff --git a/dotnet/src/Microsoft.Agents.Workflows/WorkflowBuilder.cs b/dotnet/src/Microsoft.Agents.Workflows/WorkflowBuilder.cs index 4e61c0e3a8..1a37415d19 100644 --- a/dotnet/src/Microsoft.Agents.Workflows/WorkflowBuilder.cs +++ b/dotnet/src/Microsoft.Agents.Workflows/WorkflowBuilder.cs @@ -50,8 +50,32 @@ public class WorkflowBuilder } else if (!executorish.IsUnbound) { - // If we already have an executor with this ID, we need to update it (todo: should we throw on double binding?) - this._executors[executorish.Id] = executorish.Registration; + ExecutorRegistration incoming = executorish.Registration; + // If there is already a bound executor with this ID, we need to validate (to best efforts) + // that the two are matching (at least based on type) + if (this._executors.TryGetValue(executorish.Id, out ExecutorRegistration? existing)) + { + if (existing.ExecutorType != incoming.ExecutorType) + { + throw new InvalidOperationException( + $"Cannot bind executor with ID '{executorish.Id}' because an executor with the same ID but a different type ({existing.ExecutorType.Name} vs {incoming.ExecutorType.Name}) is already bound."); + } + + if (existing.RawExecutorishData != null && + !object.ReferenceEquals(existing.RawExecutorishData, incoming.RawExecutorishData)) + { + throw new InvalidOperationException( + $"Cannot bind executor with ID '{executorish.Id}' because an executor with the same ID but different instance is already bound."); + } + } + else + { + this._executors[executorish.Id] = executorish.Registration; + if (this._unboundExecutors.Contains(executorish.Id)) + { + this._unboundExecutors.Remove(executorish.Id); + } + } } if (executorish.ExecutorType == ExecutorIsh.Type.InputPort) diff --git a/dotnet/tests/Microsoft.Agents.Workflows.UnitTests/WorkflowBuilderSmokeTests.cs b/dotnet/tests/Microsoft.Agents.Workflows.UnitTests/WorkflowBuilderSmokeTests.cs new file mode 100644 index 0000000000..3bb33d4411 --- /dev/null +++ b/dotnet/tests/Microsoft.Agents.Workflows.UnitTests/WorkflowBuilderSmokeTests.cs @@ -0,0 +1,94 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using FluentAssertions; + +namespace Microsoft.Agents.Workflows.UnitTests; + +public partial class WorkflowBuilderSmokeTests +{ + private sealed class NoOpExecutor(string? id = null) : Executor(id) + { + protected override RouteBuilder ConfigureRoutes(RouteBuilder routeBuilder) + { + return routeBuilder.AddHandler( + (msg, ctx) => + { + return ctx.SendMessageAsync(msg); + }); + } + } + + private sealed class SomeOtherNoOpExecutor(string? id = null) : Executor(id) + { + protected override RouteBuilder ConfigureRoutes(RouteBuilder routeBuilder) + { + return routeBuilder.AddHandler( + (msg, ctx) => + { + return ctx.SendMessageAsync(msg); + }); + } + } + + [Fact] + public void Test_LateBinding_Executor() + { + Workflow workflow = new WorkflowBuilder("start") + .BindExecutor(new NoOpExecutor("start")) + .Build(); + + workflow.StartExecutorId.Should().Be("start"); + + workflow.Registrations.Should().HaveCount(1); + workflow.Registrations.Should().ContainKey("start"); + workflow.Registrations["start"].ExecutorType.Should().Be(typeof(NoOpExecutor)); + } + + [Fact] + public void Test_LateImplicitBinding_Executor() + { + NoOpExecutor start = new("start"); + Workflow workflow = new WorkflowBuilder("start") + .AddEdge(start, start) + .Build(); + + workflow.StartExecutorId.Should().Be("start"); + + workflow.Registrations.Should().HaveCount(1); + workflow.Registrations.Should().ContainKey("start"); + workflow.Registrations["start"].ExecutorType.Should().Be(typeof(NoOpExecutor)); + } + + [Fact] + public void Test_RebindToDifferent_Disallowed() + { + NoOpExecutor executor1 = new("start"); + SomeOtherNoOpExecutor executor2 = new("start"); + + Func act = () => + { + return new WorkflowBuilder("start") + .AddEdge(executor1, executor2) + .Build(); + }; + + act.Should().Throw(); + } + + [Fact] + public void Test_RebindToSameish_Allowed() + { + NoOpExecutor executor1 = new("start"); + + Workflow workflow = new WorkflowBuilder("start") + .AddEdge(executor1, executor1) + .Build(); + + workflow.StartExecutorId.Should().Be("start"); + + workflow.Registrations.Should().HaveCount(1); + workflow.Registrations.Should().ContainKey("start"); + workflow.Registrations["start"].ExecutorType.Should().Be(typeof(NoOpExecutor)); + } +}