diff --git a/dotnet/src/Microsoft.Agents.AI.DurableTask/CHANGELOG.md b/dotnet/src/Microsoft.Agents.AI.DurableTask/CHANGELOG.md index 463b792..ecc8176 100644 --- a/dotnet/src/Microsoft.Agents.AI.DurableTask/CHANGELOG.md +++ b/dotnet/src/Microsoft.Agents.AI.DurableTask/CHANGELOG.md @@ -2,6 +2,7 @@ ## [Unreleased] +- Fixed `AddWorkflow` silently overwriting an existing workflow registered under the same name, which left the workflow and executor registries inconsistent. Registering a different workflow under a name that is already taken now throws, while re-registering the same workflow instance remains a no-op ([#66](https://github.com/microsoft/agent-framework-durable-extension/pull/66)) - Fixed a `JsonTypeInfo metadata ... was not provided` failure when persisting agent state for function calls or results that carry values the state serializer has no metadata for, such as the `AIContent` results returned by MCP tools ([#57](https://github.com/microsoft/agent-framework-durable-extension/pull/57)) - [BREAKING] Added `IWorkflowClient` overloads that start a registered workflow by name, and made workflow result deserialization case-insensitive so results can be read back when hosted in Azure Functions. External implementations of `IWorkflowClient` must implement the new members, and an untyped `null` first argument is now ambiguous between the `Workflow` and workflow-name overloads ([#48](https://github.com/microsoft/agent-framework-durable-extension/pull/48)) - [BREAKING] Removed the `AddAIAgents` and `AddWorkflows` bulk registration APIs and changed `AddWorkflow` to return `DurableWorkflowOptions` so multiple workflows can be registered fluently ([#39](https://github.com/microsoft/agent-framework-durable-extension/pull/39)) diff --git a/dotnet/src/Microsoft.Agents.AI.DurableTask/Workflows/DurableWorkflowOptions.cs b/dotnet/src/Microsoft.Agents.AI.DurableTask/Workflows/DurableWorkflowOptions.cs index 8222cc4..7d9b916 100644 --- a/dotnet/src/Microsoft.Agents.AI.DurableTask/Workflows/DurableWorkflowOptions.cs +++ b/dotnet/src/Microsoft.Agents.AI.DurableTask/Workflows/DurableWorkflowOptions.cs @@ -43,12 +43,21 @@ internal DurableWorkflowOptions(DurableOptions? parentOptions = null) /// The workflow instance to add. Cannot be null. /// The options instance, so that multiple calls can be chained. /// + /// /// When a workflow is added, all executors are registered in the executor registry. /// Any AI agent executors will also be automatically registered with the /// if available. + /// + /// + /// Workflow names must be unique because they identify the orchestration that runs the workflow. + /// Adding the same workflow instance more than once is a no-op, so a sub-workflow that is also + /// registered explicitly is not reported as a conflict when it is discovered during registration. + /// /// /// Thrown when is null. - /// Thrown when the workflow does not have a valid name. + /// + /// Thrown when the workflow does not have a valid name, or when a different workflow with the same name has already been registered. + /// public DurableWorkflowOptions AddWorkflow(Workflow workflow) { ArgumentNullException.ThrowIfNull(workflow); @@ -58,6 +67,17 @@ public DurableWorkflowOptions AddWorkflow(Workflow workflow) throw new ArgumentException("Workflow must have a valid Name property.", nameof(workflow)); } + if (this._workflows.TryGetValue(workflow.Name, out Workflow? existingWorkflow)) + { + if (!ReferenceEquals(existingWorkflow, workflow)) + { + throw new ArgumentException($"A workflow with name '{workflow.Name}' has already been registered.", nameof(workflow)); + } + + // The same instance was already registered, so its executors are registered too. + return this; + } + this._workflows[workflow.Name] = workflow; this.RegisterWorkflowExecutors(workflow); diff --git a/dotnet/tests/Microsoft.Agents.AI.DurableTask.UnitTests/Workflows/DurableWorkflowOptionsTests.cs b/dotnet/tests/Microsoft.Agents.AI.DurableTask.UnitTests/Workflows/DurableWorkflowOptionsTests.cs new file mode 100644 index 0000000..3a28eda --- /dev/null +++ b/dotnet/tests/Microsoft.Agents.AI.DurableTask.UnitTests/Workflows/DurableWorkflowOptionsTests.cs @@ -0,0 +1,121 @@ +// Copyright (c) Microsoft. All rights reserved. + +using Microsoft.Agents.AI.DurableTask.Workflows; +using Microsoft.Agents.AI.Workflows; + +namespace Microsoft.Agents.AI.DurableTask.UnitTests.Workflows; + +/// +/// Tests for workflow registration on . +/// +public sealed class DurableWorkflowOptionsTests +{ + [Fact] + public void AddWorkflow_ThrowsWhenDifferentWorkflowUsesRegisteredName() + { + // Arrange + DurableWorkflowOptions options = new DurableOptions().Workflows; + Workflow first = CreateWorkflow("OrderPipeline", "StepA"); + Workflow second = CreateWorkflow("OrderPipeline", "StepB"); + + options.AddWorkflow(first); + + // Act + ArgumentException ex = Assert.Throws(() => options.AddWorkflow(second)); + + // Assert + Assert.Contains("has already been registered", ex.Message, StringComparison.Ordinal); + Assert.Same(first, options.Workflows["OrderPipeline"]); + } + + [Fact] + public void AddWorkflow_ThrowsWhenNameDiffersOnlyByCase() + { + // Arrange - workflow names are compared case-insensitively because they map to orchestration names. + DurableWorkflowOptions options = new DurableOptions().Workflows; + Workflow first = CreateWorkflow("OrderPipeline", "StepA"); + options.AddWorkflow(first); + + // Act + Assert.Throws(() => options.AddWorkflow(CreateWorkflow("orderpipeline", "StepB"))); + + // Assert - the original registration is left untouched. + Assert.Same(first, Assert.Single(options.Workflows).Value); + } + + [Fact] + public void AddWorkflow_IsIdempotentForSameInstance() + { + // Arrange + DurableWorkflowOptions options = new DurableOptions().Workflows; + Workflow workflow = CreateWorkflow("OrderPipeline", "Step"); + + // Act + options.AddWorkflow(workflow); + options.AddWorkflow(workflow); + + // Assert + Assert.Single(options.Workflows); + Assert.Same(workflow, options.Workflows["OrderPipeline"]); + } + + [Fact] + public void AddWorkflow_AllowsSubWorkflowThatIsAlsoRegisteredExplicitly() + { + // Arrange - registering a parent and its sub-workflow explicitly means the recursive registration + // walk re-adds the same sub-workflow instance. + DurableWorkflowOptions options = new DurableOptions().Workflows; + Workflow subWorkflow = CreateWorkflow("SharedSub", "SubStep"); + Workflow parent = CreateParentWorkflow("Parent", "ParentStep", subWorkflow, "Sub"); + + // Act + options.AddWorkflow(subWorkflow); + options.AddWorkflow(parent); + AddSubWorkflows(options, parent); + AddSubWorkflows(options, parent); + + // Assert + Assert.Equal(2, options.Workflows.Count); + Assert.Same(subWorkflow, options.Workflows["SharedSub"]); + } + + [Fact] + public void AddWorkflow_ThrowsWhenWorkflowIsNullOrUnnamed() + { + DurableWorkflowOptions options = new DurableOptions().Workflows; + + Assert.Throws(() => options.AddWorkflow(null!)); + Assert.Throws(() => options.AddWorkflow( + new WorkflowBuilder(new FunctionExecutor("Step", (_, _, _) => default)).Build())); + } + + private static void AddSubWorkflows(DurableWorkflowOptions options, Workflow workflow) + { + foreach (SubworkflowBinding binding in workflow.ReflectExecutors() + .Select(e => e.Value) + .OfType()) + { + options.AddWorkflow(binding.WorkflowInstance); + } + } + + private static Workflow CreateWorkflow(string workflowName, string executorName) => + new WorkflowBuilder(new FunctionExecutor(executorName, (_, _, _) => default)) + .WithName(workflowName) + .Build(); + + private static Workflow CreateParentWorkflow( + string workflowName, + string executorName, + Workflow subWorkflow, + string subWorkflowExecutorName) + { + FunctionExecutor start = new(executorName, (_, _, _) => default); + ExecutorBinding subWorkflowExecutor = subWorkflow.BindAsExecutor(subWorkflowExecutorName); + + return new WorkflowBuilder(start) + .WithName(workflowName) + .AddEdge(start, subWorkflowExecutor) + .Build(); + } +}