Fix source SHA in commit diff links

src/Microsoft.DotNet.Darc/Darc/Operations/VirtualMonoRepo/CodeFlowOperation.cs

@@ -80,7 +80,7 @@ protected async Task FlowCodeLocallyAsync(
         Codeflow lastFlow;
         try
         {
-            lastFlow = await _codeFlower.GetLastFlowAsync(mapping, productRepo, currentFlow is Backflow);
+            (lastFlow, _, _) = await _codeFlower.GetLastFlowsAsync(mapping, productRepo, currentFlow is Backflow);
         }
         catch (InvalidSynchronizationException)
         {

src/Microsoft.DotNet.Darc/DarcLib/Models/VirtualMonoRepo/CodeFlowResult.cs

@@ -5,11 +5,12 @@
 using Microsoft.DotNet.DarcLib.Helpers;
 using Microsoft.DotNet.DarcLib.Models.Darc;
 
+#nullable enable
 namespace Microsoft.DotNet.DarcLib.Models.VirtualMonoRepo;
 
 public record CodeFlowResult(
     bool HadUpdates,
     IReadOnlyCollection<UnixPath> ConflictedFiles,
     NativePath RepoPath,
-    Codeflow PreviousFlow,
+    string? PreviouslyFlownSha,
     List<DependencyUpdate> DependencyUpdates);

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrBackflower.cs

@@ -110,7 +110,7 @@ public async Task<CodeFlowResult> FlowBackAsync(
             headBranch,
             cancellationToken);
 
-        Codeflow lastFlow = await GetLastFlowAsync(mapping, targetRepo, currentIsBackflow: true);
+        (Codeflow lastFlow, Backflow? lastBackFlow, _) = await GetLastFlowsAsync(mapping, targetRepo, currentIsBackflow: true);
 
         return await FlowBackAsync(
             mapping,
@@ -122,7 +122,10 @@ public async Task<CodeFlowResult> FlowBackAsync(
             headBranch,
             discardPatches,
             headBranchExisted,
-            cancellationToken);
+            cancellationToken) with
+        {
+            PreviouslyFlownSha = lastBackFlow?.SourceSha,
+        };
     }
 
     protected async Task<CodeFlowResult> FlowBackAsync(
@@ -168,7 +171,7 @@ protected async Task<CodeFlowResult> FlowBackAsync(
             hasChanges || mergeResult.DependencyUpdates.Count > 0,
             mergeResult.ConflictedFiles,
             targetRepo.Path,
-            lastFlow,
+            PreviouslyFlownSha: null,
             mergeResult.DependencyUpdates);
     }
 
@@ -445,7 +448,7 @@ private async Task RecreatePreviousFlowAndApplyBuild(
         // checkout the previous repo sha so we can get the last last flow
         await targetRepo.CheckoutAsync(previousRepoSha);
         await targetRepo.CreateBranchAsync(headBranch, overwriteExistingBranch: true);
-        var lastLastFlow = await GetLastFlowAsync(mapping, targetRepo, currentIsBackflow: true);
+        (Codeflow lastLastFlow, _, _) = await GetLastFlowsAsync(mapping, targetRepo, currentIsBackflow: true);
 
         Build previouslyAppliedVmrBuild;
         if (versionDetails.Source?.BarId != null)

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrCodeflower.cs

@@ -15,7 +15,7 @@ namespace Microsoft.DotNet.DarcLib.VirtualMonoRepo;
 
 public interface IVmrCodeFlower
 {
-    Task<Codeflow> GetLastFlowAsync(
+    Task<(Codeflow LastFlow, Backflow? LastBackFlow, ForwardFlow LastForwardFlow)> GetLastFlowsAsync(
         SourceMapping mapping,
         ILocalGitRepo repoClone,
         bool currentIsBackflow);
@@ -194,7 +194,10 @@ protected abstract Task<bool> OppositeDirectionFlowAsync(
     /// <summary>
     /// Checks the last flows between a repo and a VMR and returns the most recent one.
     /// </summary>
-    public async Task<Codeflow> GetLastFlowAsync(SourceMapping mapping, ILocalGitRepo repoClone, bool currentIsBackflow)
+    public async Task<(Codeflow LastFlow, Backflow? LastBackFlow, ForwardFlow LastForwardFlow)> GetLastFlowsAsync(
+        SourceMapping mapping,
+        ILocalGitRepo repoClone,
+        bool currentIsBackflow)
     {
         await _dependencyTracker.RefreshMetadata();
         _sourceManifest.Refresh(_vmrInfo.SourceManifestPath);
@@ -204,7 +207,7 @@ public async Task<Codeflow> GetLastFlowAsync(SourceMapping mapping, ILocalGitRep
 
         if (lastBackflow is null)
         {
-            return lastForwardFlow;
+            return (lastForwardFlow, lastBackflow, lastForwardFlow);
         }
 
         string backwardSha, forwardSha;
@@ -231,7 +234,12 @@ public async Task<Codeflow> GetLastFlowAsync(SourceMapping mapping, ILocalGitRep
         // If the SHA's are the same, it's a commit created by inflow which was then flown out
         if (forwardSha == backwardSha)
         {
-            return sourceRepo == repoClone ? lastForwardFlow : lastBackflow;
+            return
+            (
+                sourceRepo == repoClone ? lastForwardFlow : lastBackflow,
+                lastBackflow,
+                lastForwardFlow
+            );
         }
 
         // Let's determine the last flow by comparing source commit of last backflow with target commit of last forward flow
@@ -245,7 +253,12 @@ public async Task<Codeflow> GetLastFlowAsync(SourceMapping mapping, ILocalGitRep
             throw new InvalidSynchronizationException($"Failed to determine which commit of {sourceRepo} is older ({backwardSha}, {forwardSha})");
         }
 
-        return isBackwardOlder ? lastForwardFlow : lastBackflow;
+        return
+        (
+            isBackwardOlder ? lastForwardFlow : lastBackflow,
+            lastBackflow,
+            lastForwardFlow
+        );
     }
 
     /// <summary>

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs

@@ -116,7 +116,7 @@ public async Task<CodeFlowResult> FlowForwardAsync(
         await sourceRepo.FetchAllAsync([mapping.DefaultRemote, repoInfo.RemoteUri], cancellationToken);
         await sourceRepo.CheckoutAsync(build.Commit);
 
-        Codeflow lastFlow = await GetLastFlowAsync(mapping, sourceRepo, currentIsBackflow: false);
+        (Codeflow lastFlow, _, ForwardFlow lastForwardFlow) = await GetLastFlowsAsync(mapping, sourceRepo, currentIsBackflow: false);
         ForwardFlow currentFlow = new(lastFlow.TargetSha, build.Commit);
 
         bool hasChanges = await FlowCodeAsync(
@@ -150,9 +150,9 @@ public async Task<CodeFlowResult> FlowForwardAsync(
 
         return new CodeFlowResult(
             hasChanges,
-            conflictedFiles, 
+            conflictedFiles ?? [], 
             sourceRepo.Path,
-            lastFlow,
+            lastForwardFlow.SourceSha,
             DependencyUpdates: []);
     }
 
@@ -537,7 +537,7 @@ private async Task<bool> RecreatePreviousFlowAndApplyBuild(
     {
         _logger.LogInformation("Failed to create PR branch because of a conflict. Re-creating the previous flow..");
 
-        var lastLastFlow = await GetLastFlowAsync(mapping, sourceRepo, currentIsBackflow: true);
+        (Codeflow lastLastFlow, _, _) = await GetLastFlowsAsync(mapping, sourceRepo, currentIsBackflow: true);
 
         // Find the BarID of the last flown repo build
         RepositoryRecord previouslyAppliedRepositoryRecord = _sourceManifest.GetRepositoryRecord(mapping.Name);

src/ProductConstructionService/ProductConstructionService.DependencyFlow/PcsVmrBackFlower.cs

@@ -77,12 +77,12 @@ public async Task<CodeFlowResult> FlowBackAsync(
             targetBranch,
             cancellationToken);
 
-        Codeflow lastFlow = await GetLastFlowAsync(mapping, targetRepo, currentIsBackflow: true);
+        var lastFlows = await GetLastFlowsAsync(mapping, targetRepo, currentIsBackflow: true);
 
         var result = await FlowBackAsync(
             mapping,
             targetRepo,
-            lastFlow,
+            lastFlows.LastFlow,
             build,
             subscription.ExcludedAssets,
             subscription.TargetBranch,
@@ -91,12 +91,11 @@ public async Task<CodeFlowResult> FlowBackAsync(
             headBranchExisted,
             cancellationToken);
 
-        // TODO: https://github.com/dotnet/arcade-services/issues/4763
-        // We also return true when headBranchExisted so that we always flow even the <Source> tag change in an already existing PR
-        // This is a workaround and will be fixed later properly
         return result with
         {
-            HadUpdates = result.HadUpdates || headBranchExisted
+            // For already existing PRs, we want to always push the changes (even if only the <Source> tag changed)
+            HadUpdates = result.HadUpdates || headBranchExisted,
+            PreviouslyFlownSha = lastFlows.LastBackFlow?.SourceSha,
         };
     }
 

src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestBuilder.cs

@@ -57,7 +57,7 @@ string GenerateCodeFlowPRTitle(
     string GenerateCodeFlowPRDescription(
         SubscriptionUpdateWorkItem update,
         BuildDTO build,
-        string previousSourceCommit,
+        string? previousSourceCommit,
         List<DependencyUpdateSummary> dependencyUpdates,
         string? currentDescription,
         bool isForwardFlow);
@@ -220,7 +220,7 @@ public string GenerateCodeFlowPRTitle(
     public string GenerateCodeFlowPRDescription(
         SubscriptionUpdateWorkItem update,
         BuildDTO build,
-        string previousSourceCommit,
+        string? previousSourceCommit,
         List<DependencyUpdateSummary> dependencyUpdates,
         string? currentDescription,
         bool isForwardFlow)
@@ -239,7 +239,7 @@ public string GenerateCodeFlowPRDescription(
     private static string GenerateCodeFlowPRDescriptionInternal(
         SubscriptionUpdateWorkItem update,
         BuildDTO build,
-        string previousSourceCommit,
+        string? previousSourceCommit,
         List<DependencyUpdateSummary> dependencyUpdates,
         string? currentDescription,
         bool isForwardFlow)
@@ -278,7 +278,7 @@ This pull request brings the following source code changes
 
     private static string GenerateCodeFlowDescriptionForSubscription(
         Guid subscriptionId,
-        string previousSourceCommit,
+        string? previousSourceCommit,
         BuildDTO build,
         string repoUri,
         List<DependencyUpdateSummary> dependencyUpdates)
@@ -405,7 +405,7 @@ private static DependencyCategories CreateDependencyCategories(List<DependencyUp
         return null;
     }
 
-    private static string CreateSourceDiffLink(BuildDTO build, string previousSourceCommit)
+    private static string CreateSourceDiffLink(BuildDTO build, string? previousSourceCommit)
     {
         // previous source commit may be null in the case of the first code flow between a repo and the VMR ?
         if (string.IsNullOrEmpty(previousSourceCommit))

src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdater.cs

@@ -1008,21 +1008,21 @@ private async Task ProcessCodeFlowUpdateAsync(
 
         NativePath localRepoPath;
         CodeFlowResult codeFlowRes;
-        string previousSourceSha;
+        string? previousSourceSha;
 
         try
         {
             if (isForwardFlow)
             {
                 codeFlowRes = await _vmrForwardFlower.FlowForwardAsync(subscription, build, prHeadBranch, cancellationToken: default);
                 localRepoPath = _vmrInfo.VmrPath;
-                previousSourceSha = codeFlowRes.PreviousFlow.RepoSha;
+                previousSourceSha = codeFlowRes.PreviouslyFlownSha;
             }
             else
             {
                 codeFlowRes = await _vmrBackFlower.FlowBackAsync(subscription, build, prHeadBranch, cancellationToken: default);
                 localRepoPath = codeFlowRes.RepoPath;
-                previousSourceSha = codeFlowRes.PreviousFlow.VmrSha;
+                previousSourceSha = codeFlowRes.PreviouslyFlownSha;
             }
         }
         catch (ConflictInPrBranchException conflictException)
@@ -1100,7 +1100,7 @@ private async Task UpdateCodeFlowPullRequestAsync(
         SubscriptionUpdateWorkItem update,
         InProgressPullRequest pullRequest,
         PullRequest? prInfo,
-        string previousSourceSha,
+        string? previousSourceSha,
         SubscriptionDTO subscription,
         List<DependencyUpdate> newDependencyUpdates,
         bool isForwardFlow)
@@ -1167,7 +1167,7 @@ private async Task UpdateCodeFlowPullRequestAsync(
 
     private async Task<InProgressPullRequest> CreateCodeFlowPullRequestAsync(
         SubscriptionUpdateWorkItem update,
-        string previousSourceSha,
+        string? previousSourceSha,
         SubscriptionDTO subscription,
         string prBranch,
         List<DependencyUpdate> dependencyUpdates,

test/ProductConstructionService.DependencyFlow.Tests/PullRequestUpdaterTests.cs

@@ -53,7 +53,7 @@ protected override void RegisterServices(IServiceCollection services)
         services.AddSingleton(_forwardFlower.Object);
         services.AddSingleton(_gitClient.Object);
 
-        CodeFlowResult codeFlowRes = new CodeFlowResult(true, [], null, new Backflow("aaa1234", "bbb2345"), []);
+        CodeFlowResult codeFlowRes = new CodeFlowResult(true, [], new NativePath(VmrPath), "aaa1234", []);
         _forwardFlower.SetReturnsDefault(Task.FromResult(codeFlowRes));
         _backFlower.SetReturnsDefault(Task.FromResult(codeFlowRes));
         _gitClient.SetReturnsDefault(Task.CompletedTask);