Allow some PR checks to be skipped

src/Maestro/Maestro.MergePolicies/AllChecksSuccessfulMergePolicy.cs

@@ -56,15 +56,15 @@ int ListChecksCount(CheckState state)
             {
                 listChecks.AppendLine($"[{status.Name}]({status.Url})");
             }
-            return Fail($"{ListChecksCount(CheckState.Error)} unsuccessful check(s)", listChecks.ToString());
+            return FailTransiently($"{ListChecksCount(CheckState.Error)} unsuccessful check(s)", listChecks.ToString());
         }
 
         if (statuses.Contains(CheckState.Pending))
         {
             return Pending($"{ListChecksCount(CheckState.Pending)} pending check(s)");
         }
 
-        return Succeed($"{ListChecksCount(CheckState.Success)} successful check(s)");
+        return SucceedTransiently($"{ListChecksCount(CheckState.Success)} successful check(s)");
     }
 }
 

src/Maestro/Maestro.MergePolicies/BackFlowMergePolicy.cs

@@ -25,7 +25,7 @@ public override async Task<MergePolicyEvaluationResult> EvaluateAsync(PullReques
         }
         catch (System.Xml.XmlException e)
         {
-            return Fail($"Error reading file `{VersionFiles.VersionDetailsXml}`",
+            return FailDecisively($"Error reading file `{VersionFiles.VersionDetailsXml}`",
                 $"""
                 ### Error: failed to parse file `{VersionFiles.VersionDetailsXml}`.
                 The file `{VersionFiles.VersionDetailsXml}` is corrupted or improperly structured.
@@ -37,7 +37,7 @@ The file `{VersionFiles.VersionDetailsXml}` is corrupted or improperly structure
         {
             // Here also, DarcException is an xml parsing exception... that's how the version details parser throws it
             // messasges from DarcException types should be safe to expose to the client
-            return Fail($"Failed to parse file `{VersionFiles.VersionDetailsXml}`",
+            return FailDecisively($"Failed to parse file `{VersionFiles.VersionDetailsXml}`",
                 $"""
                 ### Error: failed to parse file `{VersionFiles.VersionDetailsXml}`.
                 There was some unexpected or missing information in the file.
@@ -47,7 +47,7 @@ There was some unexpected or missing information in the file.
         }
         catch (Exception)
         {
-            return Fail(
+            return FailTransiently(
                 $"Failed to retrieve file `{VersionFiles.VersionDetailsXml}`",
                 $"""
                 ### Error: unexpected server error.
@@ -66,10 +66,10 @@ This could be due to a temporary exception and may be resolved automatically wit
                 configurationErrorsHeader,
                 string.Join(Environment.NewLine, configurationErrors),
                 SeekHelpMsg);
-            return Fail($"Missing or mismatched values found in `{VersionFiles.VersionDetailsXml}`", failureMessage);
+            return FailDecisively($"Missing or mismatched values found in `{VersionFiles.VersionDetailsXml}`", failureMessage);
         }
 
-        return Succeed($"Backflow checks succeeded.");
+        return SucceedDecisively($"Backflow checks succeeded.");
     }
 
     private static List<string> CalculateConfigurationErrors(

src/Maestro/Maestro.MergePolicies/DontAutomergeDowngradesMergePolicy.cs

@@ -29,18 +29,18 @@ public override Task<MergePolicyEvaluationResult> EvaluateAsync(PullRequestUpdat
  you can ignore the check in this case.
  If you think this PR should merge but lack permission to override this check, consider finding an admin or recreating the pull request manually.
  If you feel you are seeing this message in error, please contact the dnceng team.";
-                return Task.FromResult(Fail(errorMessage));
+                return Task.FromResult(FailDecisively(errorMessage));
             }
             else
             {
                 return Task.FromResult(
-                    Succeed("No version downgrade detected and all specified versions semantically valid."));
+                    SucceedDecisively("No version downgrade detected and all specified versions semantically valid."));
             }
         }
         catch (Exception e)
         {
             return Task.FromResult(
-                Fail($"Failed to check version downgrades. Aborting auto-merge. {e.Message}"));
+                FailTransiently($"Failed to check version downgrades. Aborting auto-merge. {e.Message}"));
         }
     }
 

src/Maestro/Maestro.MergePolicies/ForwardFlowMergePolicy.cs

@@ -22,7 +22,7 @@ public override async Task<MergePolicyEvaluationResult> EvaluateAsync(PullReques
         }
         catch (Exception)
         {
-            return Fail(
+            return FailTransiently(
                 "Error while retrieving source manifest",
                 $"An issue occurred while retrieving the source manifest. This could be due to a misconfiguration of the `{VmrInfo.DefaultRelativeSourceManifestPath}` file, or because of a server error."
                 + SeekHelpMsg);
@@ -33,7 +33,7 @@ public override async Task<MergePolicyEvaluationResult> EvaluateAsync(PullReques
         if (!TryCreateBarIdDictionaryFromSourceManifest(sourceManifest, out repoNamesToBarIds) ||
             !TryCreateCommitShaDictionaryFromSourceManifest(sourceManifest, out repoNamesToCommitSha))
         {
-            return Fail(
+            return FailDecisively(
                 "The source manifest file is malformed",
                 $"Duplicate repository URIs were found in {VmrInfo.DefaultRelativeSourceManifestPath}." + SeekHelpMsg);
         }
@@ -46,10 +46,10 @@ public override async Task<MergePolicyEvaluationResult> EvaluateAsync(PullReques
                 configurationErrorsHeader,
                 string.Join(Environment.NewLine, configurationErrors),
                 SeekHelpMsg);
-            return Fail($"Missing or mismatched values found in {VmrInfo.DefaultRelativeSourceManifestPath}", failureMessage);
+            return FailDecisively($"Missing or mismatched values found in {VmrInfo.DefaultRelativeSourceManifestPath}", failureMessage);
         }
 
-        return Succeed($"Forward-flow checks succeeded.");
+        return SucceedDecisively($"Forward-flow checks succeeded.");
     }
 
     private static List<string> CalculateConfigurationErrors(

src/Maestro/Maestro.MergePolicies/MergePolicy.cs

@@ -50,13 +50,18 @@ public string Name
 
     public abstract Task<MergePolicyEvaluationResult> EvaluateAsync(PullRequestUpdateSummary pr, IRemote darc);
 
-    public MergePolicyEvaluationResult Pending(string title) => new(MergePolicyEvaluationStatus.Pending, title, string.Empty, this);
+    public MergePolicyEvaluationResult Pending(string title) => new(MergePolicyEvaluationStatus.Pending, title, string.Empty, this.Name, this.DisplayName);
 
-    public MergePolicyEvaluationResult Succeed(string title) => new(MergePolicyEvaluationStatus.Success, title, string.Empty, this);
+    public MergePolicyEvaluationResult SucceedDecisively(string title) => new(MergePolicyEvaluationStatus.DecisiveSuccess, title, string.Empty, this.Name, this.DisplayName);
+    public MergePolicyEvaluationResult SucceedTransiently(string title) => new(MergePolicyEvaluationStatus.TransientSuccess, title, string.Empty, this.Name, this.DisplayName);
 
-    public MergePolicyEvaluationResult Fail(string title) => new(MergePolicyEvaluationStatus.Failure, title, string.Empty, this);
+    public MergePolicyEvaluationResult FailDecisively(string title) => new(MergePolicyEvaluationStatus.DecisiveFailure, title, string.Empty, this.Name, this.DisplayName);
 
-    public MergePolicyEvaluationResult Fail(string title, string message) => new(MergePolicyEvaluationStatus.Failure, title, message, this);
+    public MergePolicyEvaluationResult FailTransiently(string title) => new(MergePolicyEvaluationStatus.TransientFailure, title, string.Empty, this.Name, this.DisplayName);
+
+    public MergePolicyEvaluationResult FailDecisively(string title, string message) => new(MergePolicyEvaluationStatus.DecisiveFailure, title, message, this.Name, this.DisplayName);
+
+    public MergePolicyEvaluationResult FailTransiently(string title, string message) => new(MergePolicyEvaluationStatus.TransientFailure, title, message, this.Name, this.DisplayName);
 }
 
 public interface IMergePolicy : IMergePolicyInfo

src/Maestro/Maestro.MergePolicies/NoRequestedChangesMergePolicy.cs

@@ -20,11 +20,11 @@ public override async Task<MergePolicyEvaluationResult> EvaluateAsync(PullReques
 
         if (reviews.Any(r => r.Status == ReviewState.ChangesRequested || r.Status == ReviewState.Rejected))
         {
-            return Fail("There are reviews that have requested changes.");
+            return FailTransiently("There are reviews that have requested changes.");
         }
         else
         {
-            return Succeed("No reviews have requested changes.");
+            return SucceedTransiently("No reviews have requested changes.");
         }
     }
 }

src/Maestro/Maestro.MergePolicies/ValidateCoherencyMergePolicy.cs

@@ -17,7 +17,7 @@ public class ValidateCoherencyMergePolicy : MergePolicy
     public override Task<MergePolicyEvaluationResult> EvaluateAsync(PullRequestUpdateSummary pr, IRemote darc)
     {
         if (pr.CoherencyCheckSuccessful.GetValueOrDefault(true))
-            return Task.FromResult(Succeed("Coherency check successful."));
+            return Task.FromResult(SucceedDecisively("Coherency check successful."));
 
         var description = new StringBuilder("Coherency update failed for the following dependencies:");
         foreach (CoherencyErrorDetails error in pr.CoherencyErrors ?? [])
@@ -37,7 +37,7 @@ public override Task<MergePolicyEvaluationResult> EvaluateAsync(PullRequestUpdat
             .Append("\n\nThe documentation can be found at this location: ")
             .Append("https://github.com/dotnet/arcade/blob/main/Documentation/Darc.md#coherent-parent-dependencies");
 
-        return Task.FromResult(Fail("Coherency check failed.", description.ToString()));
+        return Task.FromResult(FailDecisively("Coherency check failed.", description.ToString()));
     }
 }
 

src/Maestro/Maestro.MergePolicyEvaluation/MergePolicyEvaluationResult.cs

@@ -5,29 +5,20 @@ namespace Maestro.MergePolicyEvaluation;
 
 public class MergePolicyEvaluationResult
 {
-    public MergePolicyEvaluationResult(MergePolicyEvaluationStatus status, string title, string message, IMergePolicyInfo mergePolicy)
+    public MergePolicyEvaluationResult(MergePolicyEvaluationStatus status, string title, string message, string mergePolicyName, string mergePolicyDisplayName)
     {
-        if (mergePolicy == null)
-        {
-            throw new ArgumentNullException(nameof(mergePolicy));
-        }
-        if (mergePolicy.Name == null)
-        {
-            throw new ArgumentNullException($"{nameof(mergePolicy)}.{nameof(mergePolicy.Name)}");
-        }
-        if (mergePolicy.DisplayName == null)
-        {
-            throw new ArgumentNullException($"{nameof(mergePolicy)}.{nameof(mergePolicy.DisplayName)}");
-        }
-
+        ArgumentNullException.ThrowIfNull(mergePolicyName);
+        ArgumentNullException.ThrowIfNull(mergePolicyDisplayName);
         Status = status;
         Title = title;
         Message = message;
-        MergePolicyInfo = mergePolicy;
+        MergePolicyName = mergePolicyName;
+        MergePolicyDisplayName = mergePolicyDisplayName;
     }
 
     public MergePolicyEvaluationStatus Status { get; }
     public string Title { get; }
     public string Message { get; }
-    public IMergePolicyInfo MergePolicyInfo { get; }
+    public string MergePolicyName { get; }
+    public string MergePolicyDisplayName { get; }
 }

src/Maestro/Maestro.MergePolicyEvaluation/MergePolicyEvaluationResults.cs

@@ -1,22 +1,27 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.Collections.Immutable;
-
 namespace Maestro.MergePolicyEvaluation;
 
 public class MergePolicyEvaluationResults
 {
-    public MergePolicyEvaluationResults(IEnumerable<MergePolicyEvaluationResult> results)
+
+    public MergePolicyEvaluationResults(string id, IReadOnlyCollection<MergePolicyEvaluationResult> results, string targetCommitSha)
     {
-        Results = results.ToImmutableList();
+        Id = id;
+        Results = results;
+        TargetCommitSha = targetCommitSha;
     }
 
-    public IImmutableList<MergePolicyEvaluationResult> Results { get; }
+    public string Id { get; set; }
+
+    public IReadOnlyCollection<MergePolicyEvaluationResult> Results { get; set; }
+
+    public string TargetCommitSha { get; set; }
 
-    public bool Succeeded => Results.Count > 0 && Results.All(r => r.Status == MergePolicyEvaluationStatus.Success);
+    public bool Succeeded => Results.Any() && Results.All(r => r.Status is MergePolicyEvaluationStatus.DecisiveSuccess or MergePolicyEvaluationStatus.TransientSuccess);
 
     public bool Pending => Results.Any(r => r.Status == MergePolicyEvaluationStatus.Pending);
 
-    public bool Failed => Results.Any(r => r.Status == MergePolicyEvaluationStatus.Failure);
+    public bool Failed => Results.Any(r => r.Status is MergePolicyEvaluationStatus.DecisiveFailure or MergePolicyEvaluationStatus.TransientFailure);
 }

src/Maestro/Maestro.MergePolicyEvaluation/MergePolicyEvaluationStatus.cs

@@ -6,6 +6,8 @@ namespace Maestro.MergePolicyEvaluation;
 public enum MergePolicyEvaluationStatus
 {
     Pending = 0,
-    Success,
-    Failure,
+    DecisiveSuccess, // success that doesn't need to be evaluated again if the commit didn't change
+    TransientSuccess, // success that needs to be re-evaluated even when on the same commit
+    DecisiveFailure, // failure that cannot be resolved by simply re-evaluating without adding new commits
+    TransientFailure, // failure that may be resolved by simply re-evaluating
 }

src/Microsoft.DotNet.Darc/DarcLib/AzureDevOpsClient.cs

@@ -341,6 +341,7 @@ public async Task<PullRequest> GetPullRequestAsync(string pullRequestUrl)
                 _ => PrStatus.None,
             },
             UpdatedAt = DateTimeOffset.UtcNow,
+            TargetBranchCommitSha = pr.LastMergeTargetCommit.CommitId,
         };
     }
 
@@ -522,14 +523,14 @@ private async Task CreateOrUpdatePullRequestCommentAsync(string pullRequestUrl,
         await client.CreateThreadAsync(newCommentThread, repoName, id);
     }
 
-    public async Task CreateOrUpdatePullRequestMergeStatusInfoAsync(string pullRequestUrl, IReadOnlyList<MergePolicyEvaluationResult> evaluations)
+    public async Task CreateOrUpdatePullRequestMergeStatusInfoAsync(string pullRequestUrl, IReadOnlyCollection<MergePolicyEvaluationResult> evaluations)
     {
         await CreateOrUpdatePullRequestCommentAsync(pullRequestUrl,
             $"""
             ## Auto-Merge Status
 
             This pull request has not been merged because Maestro++ is waiting on the following merge policies.
-            {string.Join(Environment.NewLine, evaluations.OrderBy(r => r.MergePolicyInfo.Name).Select(DisplayPolicy))}
+            {string.Join(Environment.NewLine, evaluations.OrderBy(r => r.MergePolicyName).Select(DisplayPolicy))}
             """);
     }
 
@@ -540,13 +541,13 @@ private string DisplayPolicy(MergePolicyEvaluationResult result)
             return $"- ❓ **{result.Title}** - {result.Message}";
         }
 
-        if (result.Status == MergePolicyEvaluationStatus.Success)
+        if (result.Status == MergePolicyEvaluationStatus.DecisiveSuccess)
         {
-            return $"- ✔️ **{result.MergePolicyInfo.DisplayName}** Succeeded"
+            return $"- ✔️ **{result.MergePolicyDisplayName}** Succeeded"
                 + (result.Title == null ? string.Empty: $" - {result.Title}");
         }
 
-        return $"- ❌ **{result.MergePolicyInfo.DisplayName}** {result.Title} - {result.Message}";
+        return $"- ❌ **{result.MergePolicyDisplayName}** {result.Title} - {result.Message}";
     }
 
     /// <summary>

src/Microsoft.DotNet.Darc/DarcLib/GitHubClient.cs

@@ -321,6 +321,7 @@ public async Task<PullRequest> GetPullRequestAsync(string pullRequestUrl)
             HeadBranch = pr.Head.Ref,
             Status = status,
             UpdatedAt = pr.UpdatedAt,
+            TargetBranchCommitSha = pr.Head.Sha,
         };
     }
 
@@ -462,10 +463,10 @@ public async Task CreateOrUpdatePullRequestCommentAsync(string pullRequestUrl, s
     /// <param name="sha">Sha of the latest commit in the PR</param>
     private static string CheckRunId(MergePolicyEvaluationResult result, string sha)
     {
-        return $"{MergePolicyConstants.MaestroMergePolicyCheckRunPrefix}{result.MergePolicyInfo.Name}-{sha}";
+        return $"{MergePolicyConstants.MaestroMergePolicyCheckRunPrefix}{result.MergePolicyName}-{sha}";
     }
 
-    public async Task CreateOrUpdatePullRequestMergeStatusInfoAsync(string pullRequestUrl, IReadOnlyList<MergePolicyEvaluationResult> evaluations)
+    public async Task CreateOrUpdatePullRequestMergeStatusInfoAsync(string pullRequestUrl, IReadOnlyCollection<MergePolicyEvaluationResult> evaluations)
     {
         (string owner, string repo, int id) = ParsePullRequestUri(pullRequestUrl);
         var client = GetClient(owner, repo);
@@ -507,7 +508,7 @@ public async Task CreateOrUpdatePullRequestMergeStatusInfoAsync(string pullReque
     /// <returns>The new check run</returns>
     private static NewCheckRun CheckRunForAdd(MergePolicyEvaluationResult result, string sha)
     {
-        var newCheckRun = new NewCheckRun($"{MergePolicyConstants.MaestroMergePolicyDisplayName} - {result.MergePolicyInfo.DisplayName}", sha)
+        var newCheckRun = new NewCheckRun($"{MergePolicyConstants.MaestroMergePolicyDisplayName} - {result.MergePolicyDisplayName}", sha)
         {
             ExternalId = CheckRunId(result, sha)
         };
@@ -559,7 +560,7 @@ private static void UpdateCheckRun(NewCheckRun newCheckRun, MergePolicyEvaluatio
         {
             newCheckRun.Status = CheckStatus.InProgress;
         }
-        else if (result.Status == MergePolicyEvaluationStatus.Success)
+        else if (result.Status == MergePolicyEvaluationStatus.DecisiveSuccess)
         {
             newCheckRun.Conclusion = "success";
             newCheckRun.CompletedAt = DateTime.Now;
@@ -591,7 +592,7 @@ private static void UpdateCheckRun(CheckRunUpdate newUpdateCheckRun, MergePolicy
         {
             newUpdateCheckRun.Status = CheckStatus.InProgress;
         }
-        else if (result.Status == MergePolicyEvaluationStatus.Success)
+        else if (result.Status == MergePolicyEvaluationStatus.DecisiveSuccess)
         {
             newUpdateCheckRun.Conclusion = "success";
             newUpdateCheckRun.CompletedAt = DateTime.Now;

src/Microsoft.DotNet.Darc/DarcLib/IRemote.cs

@@ -28,7 +28,7 @@ public interface IRemote
     /// </summary>
     /// <param name="pullRequestUrl">Url of pull request.</param>
     /// <param name="evaluations">List of merge policies.</param>
-    Task CreateOrUpdatePullRequestMergeStatusInfoAsync(string pullRequestUrl, IReadOnlyList<MergePolicyEvaluationResult> evaluations);
+    Task CreateOrUpdatePullRequestMergeStatusInfoAsync(string pullRequestUrl, IReadOnlyCollection<MergePolicyEvaluationResult> evaluations);
 
     /// <summary>
     ///     Get the checks that are being run on a pull request.

src/Microsoft.DotNet.Darc/DarcLib/IRemoteGitRepo.cs

@@ -103,7 +103,7 @@ Task<IEnumerable<int>> SearchPullRequestsAsync(
     /// </summary>
     /// <param name="pullRequestUrl">Url of pull request</param>
     /// <param name="evaluations">List of merge policies</param>
-    Task CreateOrUpdatePullRequestMergeStatusInfoAsync(string pullRequestUrl, IReadOnlyList<MergePolicyEvaluationResult> evaluations);
+    Task CreateOrUpdatePullRequestMergeStatusInfoAsync(string pullRequestUrl, IReadOnlyCollection<MergePolicyEvaluationResult> evaluations);
 
     /// <summary>
     ///     Get the latest commit in a repo on the specific branch 
@@ -180,4 +180,5 @@ public class PullRequest
     public string HeadBranch { get; set; }
     public PrStatus Status { get; set; }
     public DateTimeOffset UpdatedAt { get; set; }
+    public string TargetBranchCommitSha { get; set; }
 }

src/Microsoft.DotNet.Darc/DarcLib/Remote.cs

@@ -88,7 +88,7 @@ public async Task<IEnumerable<Review>> GetPullRequestReviewsAsync(string pullReq
         return await _remoteGitClient.GetLatestPullRequestReviewsAsync(pullRequestUrl);
     }
 
-    public Task CreateOrUpdatePullRequestMergeStatusInfoAsync(string pullRequestUrl, IReadOnlyList<MergePolicyEvaluationResult> evaluations)
+    public Task CreateOrUpdatePullRequestMergeStatusInfoAsync(string pullRequestUrl, IReadOnlyCollection<MergePolicyEvaluationResult> evaluations)
     {
         CheckForValidGitClient();
         return _remoteGitClient.CreateOrUpdatePullRequestMergeStatusInfoAsync(pullRequestUrl, evaluations);