diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Interpreter/WorkflowActionVisitor.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Interpreter/WorkflowActionVisitor.cs index 1cd1b2bc941..a14e6004754 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Interpreter/WorkflowActionVisitor.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Interpreter/WorkflowActionVisitor.cs @@ -171,8 +171,8 @@ protected override void Visit(GotoAction item) // Represent action with default executor DefaultActionExecutor action = new(item, this._workflowState); this.ContinueWith(action); - // Transition to target action - this._workflowModel.AddLink(action.Id, item.ActionId.Value); + // Transition to target action, injecting completion events for non-discrete parents + this.InjectParentCompletionEvents(action.Id, item.ActionId.Value); // Define a clean-start to ensure "goto" is not a source for any edge this.RestartAfter(action.Id, action.ParentId); } @@ -219,8 +219,9 @@ protected override void Visit(BreakLoop item) // Represent action with default executor DefaultActionExecutor action = new(item, this._workflowState); this.ContinueWith(action); - // Transition to post action - this._workflowModel.AddLink(action.Id, Steps.Post(loopAction.Id)); + // Transition to post action, injecting completion events for non-discrete parents + // Stop before the Foreach's own Post step since that handles Foreach completion + this.InjectParentCompletionEvents(action.Id, Steps.Post(loopAction.Id), stopBeforeParentId: loopAction.Id); // Define a clean-start to ensure "break" is not a source for any edge this.RestartAfter(action.Id, action.ParentId); } @@ -238,8 +239,9 @@ protected override void Visit(ContinueLoop item) // Represent action with default executor DefaultActionExecutor action = new(item, this._workflowState); this.ContinueWith(action); - // Transition to select the next item - this._workflowModel.AddLink(action.Id, ForeachExecutor.Steps.Next(loopAction.Id)); + // Transition to select the next item, injecting completion events for non-discrete parents + // Stop before the Foreach's own Next step since that handles Foreach iteration + this.InjectParentCompletionEvents(action.Id, ForeachExecutor.Steps.Next(loopAction.Id), stopBeforeParentId: loopAction.Id); // Define a clean-start to ensure "continue" is not a source for any edge this.RestartAfter(action.Id, action.ParentId); } @@ -302,8 +304,28 @@ protected override void Visit(EndDialog item) // Represent action with default executor DefaultActionExecutor action = new(item, this._workflowState); this.ContinueWith(action); + + var nonDiscreteAncestors = this._workflowModel + .LocateNonDiscreteAncestors(action.Id) + .ToList(); + + string currentId = action.Id; + foreach (var ancestor in nonDiscreteAncestors) + { + string completionId = $"{action.Id}_Complete_{ancestor.Id}"; + var completionStep = new DelegateActionExecutor( + completionId, + this._workflowState, + async (context, _, ct) => await context.RaiseCompletionEventAsync(ancestor.Model, ct).ConfigureAwait(false), + emitResult: false); + + this._workflowModel.AddNode(completionStep, ancestor.ParentId); + this._workflowModel.AddLink(currentId, completionId); + currentId = completionId; + } + // Define a clean-start to ensure "end" is not a source for any edge - this.RestartAfter(item.Id.Value, action.ParentId); + this.RestartAfter(currentId, action.ParentId); } protected override void Visit(EndConversation item) @@ -313,8 +335,28 @@ protected override void Visit(EndConversation item) // Represent action with default executor DefaultActionExecutor action = new(item, this._workflowState); this.ContinueWith(action); + + var nonDiscreteAncestors = this._workflowModel + .LocateNonDiscreteAncestors(action.Id) + .ToList(); + + string currentId = action.Id; + foreach (var ancestor in nonDiscreteAncestors) + { + string completionId = $"{action.Id}_Complete_{ancestor.Id}"; + var completionStep = new DelegateActionExecutor( + completionId, + this._workflowState, + async (context, _, ct) => await context.RaiseCompletionEventAsync(ancestor.Model, ct).ConfigureAwait(false), + emitResult: false); + + this._workflowModel.AddNode(completionStep, ancestor.ParentId); + this._workflowModel.AddLink(currentId, completionId); + currentId = completionId; + } + // Define a clean-start to ensure "end" is not a source for any edge - this.RestartAfter(action.Id, action.ParentId); + this.RestartAfter(currentId, action.ParentId); } protected override void Visit(CancelAllDialogs item) @@ -324,8 +366,28 @@ protected override void Visit(CancelAllDialogs item) // Represent action with default executor DefaultActionExecutor action = new(item, this._workflowState); this.ContinueWith(action); + + var nonDiscreteAncestors = this._workflowModel + .LocateNonDiscreteAncestors(action.Id) + .ToList(); + + string currentId = action.Id; + foreach (var ancestor in nonDiscreteAncestors) + { + string completionId = $"{action.Id}_Complete_{ancestor.Id}"; + var completionStep = new DelegateActionExecutor( + completionId, + this._workflowState, + async (context, _, ct) => await context.RaiseCompletionEventAsync(ancestor.Model, ct).ConfigureAwait(false), + emitResult: false); + + this._workflowModel.AddNode(completionStep, ancestor.ParentId); + this._workflowModel.AddLink(currentId, completionId); + currentId = completionId; + } + // Define a clean-start to ensure "end" is not a source for any edge - this.RestartAfter(item.Id.Value, action.ParentId); + this.RestartAfter(currentId, action.ParentId); } protected override void Visit(CancelDialog item) @@ -335,8 +397,27 @@ protected override void Visit(CancelDialog item) // Represent action with default executor DefaultActionExecutor action = new(item, this._workflowState); this.ContinueWith(action); - // Define a clean-start to ensure "end" is not a source for any edge - this.RestartAfter(action.Id, action.ParentId); + + var nonDiscreteAncestors = this._workflowModel + .LocateNonDiscreteAncestors(action.Id) + .ToList(); + + string currentId = action.Id; + foreach (var ancestor in nonDiscreteAncestors) + { + string completionId = $"{action.Id}_Complete_{ancestor.Id}"; + var completionStep = new DelegateActionExecutor( + completionId, + this._workflowState, + async (context, _, ct) => await context.RaiseCompletionEventAsync(ancestor.Model, ct).ConfigureAwait(false), + emitResult: false); + + this._workflowModel.AddNode(completionStep, ancestor.ParentId); + this._workflowModel.AddLink(currentId, completionId); + currentId = completionId; + } + + this.RestartAfter(currentId, action.ParentId); } protected override void Visit(CreateConversation item) @@ -541,6 +622,49 @@ protected override void Visit(HttpRequestAction item) this.ContinueWith(new HttpRequestExecutor(item, this._workflowOptions.HttpRequestHandler, this._workflowOptions.AgentProvider, this._workflowState)); } + /// + /// Injects completion event steps for all non-discrete parent actions when + /// a terminal action (goto, end, break, etc.) bypasses the normal Post step flow. + /// This ensures DeclarativeActionCompletedEvent is raised for every action. + /// + private void InjectParentCompletionEvents(string terminalActionId, string targetId, string? stopBeforeParentId = null) + { + var nonDiscreteAncestors = this._workflowModel + .LocateNonDiscreteAncestors(terminalActionId) + .ToList(); + + if (nonDiscreteAncestors.Count == 0) + { + this._workflowModel.AddLink(terminalActionId, targetId); + return; + } + + string currentId = terminalActionId; + + foreach (var ancestor in nonDiscreteAncestors) + { + if (stopBeforeParentId is not null && + string.Equals(ancestor.Id, stopBeforeParentId, StringComparison.Ordinal)) + { + break; + } + + string completionId = $"{terminalActionId}_Complete_{ancestor.Id}"; + var completionStep = new DelegateActionExecutor( + completionId, + this._workflowState, + async (context, _, ct) => await context.RaiseCompletionEventAsync(ancestor.Model, ct).ConfigureAwait(false), + emitResult: false); + + this._workflowModel.AddNode(completionStep, ancestor.ParentId); + this._workflowModel.AddLink(currentId, completionId); + + currentId = completionId; + } + + this._workflowModel.AddLink(currentId, targetId); + } + #region Not supported protected override void Visit(AnswerQuestionWithAI item) => this.NotSupported(item); diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Interpreter/WorkflowModel.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Interpreter/WorkflowModel.cs index 1837c3b3340..80a4d2cbf1f 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Interpreter/WorkflowModel.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Interpreter/WorkflowModel.cs @@ -140,6 +140,57 @@ private ModelNode DefineNode(IModeledAction action, ModelNode? parentNode = null return null; } + /// + /// Finds all non-discrete parent actions that need completion events raised. + /// These are actions that rely on their Post step to raise completion events + /// (e.g., ConditionGroupExecutor, ForeachExecutor). + /// + public IEnumerable LocateNonDiscreteAncestors(string? itemId) where TAction : class, IModeledAction + { + if (string.IsNullOrEmpty(itemId)) + { + yield break; + } + + // Get the starting node and move to its parent first (don't include the starting node) + if (!this.Nodes.TryGetValue(itemId, out ModelNode? startNode)) + { + yield break; + } + + string? currentId = startNode.Parent?.Action.Id; + + while (currentId is not null) + { + if (!this.Nodes.TryGetValue(currentId, out ModelNode? itemNode)) + { + yield break; + } + + // Check if this is a non-discrete action type that needs completion events + // (actions that rely on Post step for completion, not HandleAsync finally block) + if (itemNode.Action is TAction nonDiscreteAction) + { + Type actionType = nonDiscreteAction.GetType(); + Type? baseType = actionType.BaseType; + + // ConditionGroupExecutor and ForeachExecutor are non-discrete + // They have IsDiscreteAction = false and raise completion in their Post step + bool isNonDiscrete = actionType.Name.Contains("ConditionGroupExecutor") || + actionType.Name.Contains("ForeachExecutor") || + (baseType is not null && (baseType.Name.Contains("ConditionGroupExecutor") || + baseType.Name.Contains("ForeachExecutor"))); + + if (isNonDiscrete) + { + yield return nonDiscreteAction; + } + } + + currentId = itemNode.Parent?.Action.Id; + } + } + private sealed class ModelNode(IModeledAction action, ModelNode? parent = null, Action? completionHandler = null) { public IModeledAction Action => action; diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/DeclarativeWorkflowTest.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/DeclarativeWorkflowTest.cs index d3e828fef65..c537448e7b4 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/DeclarativeWorkflowTest.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/DeclarativeWorkflowTest.cs @@ -91,6 +91,25 @@ public async Task GotoActionAsync() this.AssertNotExecuted("sendActivity_3"); } + /// + /// Regression test for issue #3743: DeclarativeActionCompletedEvent must be raised + /// for parent actions when GotoAction bypasses the default terminal edge. + /// + [Fact] + public async Task GotoInsideCondition_RaisesParentCompletionEventAsync() + { + await this.RunWorkflowAsync("GotoInsideCondition.yaml", 1); + + this.AssertExecuted("goto_end"); + + Assert.Contains( + this.WorkflowEvents.OfType(), + e => e.ActionId == "condition_test"); + + this.AssertNotExecuted("send_activity_normal"); + this.AssertNotExecuted("send_activity_after"); + } + [Theory] [InlineData(12)] [InlineData(37)] @@ -180,7 +199,7 @@ public async Task ConditionActionWithFallThroughAsync(int input, int expectedAct [InlineData("ClearAllVariables.yaml", 1, "clear_all")] [InlineData("ResetVariable.yaml", 2, "clear_var")] [InlineData("MixedScopes.yaml", 2, "activity_input")] - [InlineData("CaseInsensitive.yaml", 6, "end_when_match")] + [InlineData("CaseInsensitive.yaml", 7, "end_when_match")] [InlineData("HttpRequest.yaml", 1, "http_request")] public async Task ExecuteActionAsync(string workflowFile, int expectedCount, string expectedId) { diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/Workflows/GotoInsideCondition.yaml b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/Workflows/GotoInsideCondition.yaml new file mode 100644 index 00000000000..0b8ae004d00 --- /dev/null +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/Workflows/GotoInsideCondition.yaml @@ -0,0 +1,35 @@ +kind: Workflow +trigger: + + kind: OnConversationStart + id: my_workflow + actions: + + - kind: SetVariable + id: set_input + variable: Local.TestValue + value: "=Value(System.LastMessageText)" + + - kind: ConditionGroup + id: condition_test + conditions: + - id: condition_goto_branch + condition: "=Local.TestValue = 1" + actions: + - kind: GotoAction + id: goto_end + actionId: end_all + + - id: condition_other_branch + condition: "=Local.TestValue <> 1" + actions: + - kind: SendActivity + id: send_activity_normal + activity: NORMAL PATH + + - kind: SendActivity + id: send_activity_after + activity: SHOULD NOT SEE THIS + + - kind: EndConversation + id: end_all