From 854d5e3bf3f02e07375d2323c5efeeb61af39c63 Mon Sep 17 00:00:00 2001
From: Thomas Boop <52323235+thboop@users.noreply.github.com>
Date: Wed, 27 Oct 2021 09:31:58 -0400
Subject: [PATCH] Fix an issue where nested local composite actions did not
 correctly register post steps (#1433)

* Always register post steps for local actions

* Register post steps along with their conditions

* remove debug code

Co-authored-by: Ferenc Hammerl <fhammerl@github.com>
---
 src/Runner.Worker/ActionManager.cs            | 13 +++++++++
 src/Runner.Worker/ExecutionContext.cs         | 29 ++++++++++++-------
 .../Handlers/CompositeActionHandler.cs        |  3 +-
 3 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/src/Runner.Worker/ActionManager.cs b/src/Runner.Worker/ActionManager.cs
index 34da0a65b..65b47d3f1 100644
--- a/src/Runner.Worker/ActionManager.cs
+++ b/src/Runner.Worker/ActionManager.cs
@@ -267,6 +267,19 @@ namespace GitHub.Runner.Worker
                             _cachedEmbeddedPostSteps[parentStepId].Push(clonedAction);
                         }
                     }
+                    else if (depth > 0)
+                    {
+                        // if we're in a composite action and haven't loaded the local action yet
+                        // we assume it has a post step
+                        if (!_cachedEmbeddedPostSteps.ContainsKey(parentStepId))
+                        {
+                            // If we haven't done so already, add the parent to the post steps
+                            _cachedEmbeddedPostSteps[parentStepId] = new Stack<Pipelines.ActionStep>();
+                        }
+                        // Clone action so we can modify the condition without affecting the original
+                        var clonedAction = action.Clone() as Pipelines.ActionStep;
+                        _cachedEmbeddedPostSteps[parentStepId].Push(clonedAction);                        
+                    }
                 }
             }
 
diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs
index 274bef361..d7bf42249 100644
--- a/src/Runner.Worker/ExecutionContext.cs
+++ b/src/Runner.Worker/ExecutionContext.cs
@@ -62,7 +62,7 @@ namespace GitHub.Runner.Worker
 
         // Only job level ExecutionContext has PostJobSteps
         Stack<IStep> PostJobSteps { get; }
-        HashSet<Guid> EmbeddedStepsWithPostRegistered{ get; }
+        Dictionary<Guid, string> EmbeddedStepsWithPostRegistered { get; }
 
         // Keep track of embedded steps states
         Dictionary<Guid, Dictionary<string, string>> EmbeddedIntraActionState { get; }
@@ -168,7 +168,7 @@ namespace GitHub.Runner.Worker
         public HashSet<Guid> StepsWithPostRegistered { get; private set; }
 
         // Only job level ExecutionContext has EmbeddedStepsWithPostRegistered
-        public HashSet<Guid> EmbeddedStepsWithPostRegistered { get; private set; }
+        public Dictionary<Guid, string> EmbeddedStepsWithPostRegistered { get; private set; }
 
         public Dictionary<Guid, Dictionary<string, string>> EmbeddedIntraActionState { get; private set; }
 
@@ -265,12 +265,19 @@ namespace GitHub.Runner.Worker
             string siblingScopeName = null;
             if (this.IsEmbedded)
             {
-                if (step is IActionRunner actionRunner && !Root.EmbeddedStepsWithPostRegistered.Add(actionRunner.Action.Id))
+                if (step is IActionRunner actionRunner)
                 {
-                    Trace.Info($"'post' of '{actionRunner.DisplayName}' already push to child post step stack.");
+                    if (Root.EmbeddedStepsWithPostRegistered.ContainsKey(actionRunner.Action.Id))
+                    {
+                        Trace.Info($"'post' of '{actionRunner.DisplayName}' already push to child post step stack.");
+                    }
+                    else 
+                    {
+                        Root.EmbeddedStepsWithPostRegistered[actionRunner.Action.Id] = actionRunner.Condition;    
+                    }
+                    return;
                 }
-                return;
-            } 
+            }
             else if (step is IActionRunner actionRunner && !Root.StepsWithPostRegistered.Add(actionRunner.Action.Id))
             {
                 Trace.Info($"'post' of '{actionRunner.DisplayName}' already push to post step stack.");
@@ -543,8 +550,8 @@ namespace GitHub.Runner.Worker
                 }
 
                 _record.WarningCount++;
-            } 
-            else if (issue.Type == IssueType.Notice) 
+            }
+            else if (issue.Type == IssueType.Notice)
             {
 
                 // tracking line number for each issue in log file
@@ -716,10 +723,10 @@ namespace GitHub.Runner.Worker
             StepsWithPostRegistered = new HashSet<Guid>();
 
             // EmbeddedStepsWithPostRegistered for job ExecutionContext
-            EmbeddedStepsWithPostRegistered = new HashSet<Guid>();
+            EmbeddedStepsWithPostRegistered = new Dictionary<Guid, string>();
 
             // EmbeddedIntraActionState for job ExecutionContext
-            EmbeddedIntraActionState = new Dictionary<Guid, Dictionary<string,string>>();
+            EmbeddedIntraActionState = new Dictionary<Guid, Dictionary<string, string>>();
 
             // Job timeline record.
             InitializeTimelineRecord(
@@ -970,7 +977,7 @@ namespace GitHub.Runner.Worker
         // Do not add a format string overload. See comment on ExecutionContext.Write().
         public static void InfrastructureError(this IExecutionContext context, string message)
         {
-            context.AddIssue(new Issue() { Type = IssueType.Error, Message = message, IsInfrastructureIssue = true});
+            context.AddIssue(new Issue() { Type = IssueType.Error, Message = message, IsInfrastructureIssue = true });
         }
 
         // Do not add a format string overload. See comment on ExecutionContext.Write().
diff --git a/src/Runner.Worker/Handlers/CompositeActionHandler.cs b/src/Runner.Worker/Handlers/CompositeActionHandler.cs
index fa95180cd..4e8284e44 100644
--- a/src/Runner.Worker/Handlers/CompositeActionHandler.cs
+++ b/src/Runner.Worker/Handlers/CompositeActionHandler.cs
@@ -50,8 +50,9 @@ namespace GitHub.Runner.Worker.Handlers
                 // Only register post steps for steps that actually ran
                 foreach (var step in Data.PostSteps.ToList())
                 {
-                    if (ExecutionContext.Root.EmbeddedStepsWithPostRegistered.Contains(step.Id))
+                    if (ExecutionContext.Root.EmbeddedStepsWithPostRegistered.ContainsKey(step.Id))
                     {
+                        step.Condition = ExecutionContext.Root.EmbeddedStepsWithPostRegistered[step.Id];
                         steps.Add(step);
                     }
                     else