[Rollout] Production Rollout 2025-10-13 (#5367)

Author: premun

docs/Darc.md

@@ -2745,7 +2745,10 @@ Local dependencies updated from channel '.NET 5 Dev'.
 Update an existing subscription. Opens an editor so that some properties of a
 subscription may be altered. Because of the way that Maestro++ tracks pull
 requests, the *target* parameters of a subscription (target repository and
-target branch) may not be edited.
+target branch) may not be edited. Additionally, the subscription ID and Source
+Enabled flag cannot be modified. If any of these immutable fields are changed,
+the update will fail with an error listing all the fields that cannot be
+modified.
 
 **Sample**:
 ```

src/Microsoft.DotNet.Darc/Darc/Operations/UpdateSubscriptionOperation.cs

@@ -251,6 +251,34 @@ public override async Task<int> ExecuteAsync()
             sourceDirectory = updateSubscriptionPopUp.SourceDirectory;
             targetDirectory = updateSubscriptionPopUp.TargetDirectory;
             excludedAssets = [..updateSubscriptionPopUp.ExcludedAssets];
+
+            // Validate that immutable fields have not been changed
+            var immutableFieldErrors = new List<string>();
+
+            if (updateSubscriptionPopUp.TargetRepository != subscription.TargetRepository)
+            {
+                immutableFieldErrors.Add($"Target Repository URL (cannot be changed from '{subscription.TargetRepository}')");
+            }
+
+            if (updateSubscriptionPopUp.TargetBranch != subscription.TargetBranch)
+            {
+                immutableFieldErrors.Add($"Target Branch (cannot be changed from '{subscription.TargetBranch}')");
+            }
+
+            if (updateSubscriptionPopUp.SourceEnabled != subscription.SourceEnabled)
+            {
+                immutableFieldErrors.Add($"Source Enabled (cannot be changed from '{subscription.SourceEnabled}')");
+            }
+
+            if (immutableFieldErrors.Any())
+            {
+                _logger.LogError("The following immutable fields cannot be modified:");
+                foreach (var error in immutableFieldErrors)
+                {
+                    _logger.LogError($"  - {error}");
+                }
+                return Constants.ErrorCode;
+            }
         }
 
 
@@ -357,6 +385,7 @@ private bool UpdatingViaCommandLine()
            || _options.SourceEnabled != null
            || _options.SourceDirectory != null
            || _options.ExcludedAssets != null
+           || _options.TargetDirectory != null
            || UpdatingMergePoliciesViaCommandLine();
 
     private bool UpdatingMergePoliciesViaCommandLine()

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

@@ -4,9 +4,12 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Text;
+using System.Text.RegularExpressions;
 using System.Threading;
 using System.Threading.Tasks;
 using LibGit2Sharp;
+using Maestro.Common;
 using Microsoft.DotNet.DarcLib.Helpers;
 using Microsoft.DotNet.DarcLib.Models.VirtualMonoRepo;
 using Microsoft.DotNet.ProductConstructionService.Client.Models;
@@ -59,8 +62,17 @@ public class VmrForwardFlower : VmrCodeFlower, IVmrForwardFlower
     private readonly IForwardFlowConflictResolver _conflictResolver;
     private readonly IWorkBranchFactory _workBranchFactory;
     private readonly IProcessManager _processManager;
+    private readonly ICommentCollector _commentCollector;
     private readonly ILogger<VmrCodeFlower> _logger;
 
+    // Regex to extract PR number from a github merge commit message
+    // e.g.: "Update dependencies from source-repo (#12345)" extracts "12345"
+    private readonly static Regex GitHubPullRequestNumberExtractionRegex = new Regex(".+\\(#(\\d+)\\)$");
+
+    // Regex to extract PR number from an AzDO merge commit message
+    // e.g.: "Merged PR 12345: Update dependencies from source-repo" extracts "12345"
+    private readonly static Regex AzDoPullRequestNumberExtractionRegex = new Regex("^Merged PR (\\d+):");
+
     public VmrForwardFlower(
             IVmrInfo vmrInfo,
             ISourceManifest sourceManifest,
@@ -91,6 +103,7 @@ public VmrForwardFlower(
         _conflictResolver = conflictResolver;
         _workBranchFactory = workBranchFactory;
         _processManager = processManager;
+        _commentCollector = commentCollector;
         _logger = logger;
     }
 
@@ -151,6 +164,11 @@ public async Task<CodeFlowResult> FlowForwardAsync(
             hasChanges &= await _codeflowChangeAnalyzer.ForwardFlowHasMeaningfulChangesAsync(mapping.Name, headBranch, targetBranch);
         }
 
+        if (hasChanges)
+        {
+            await CommentIncludedPRs(sourceRepo, lastFlows.LastForwardFlow.RepoSha, build.Commit, mapping.DefaultRemote, cancellationToken);
+        }
+
         return new CodeFlowResult(
             hasChanges,
             conflictedFiles ?? [],
@@ -366,6 +384,44 @@ .. DependencyFileManager.CodeflowDependencyFiles.Select(VmrPatchHandler.GetExclu
         return hadChanges;
     }
 
+    private async Task CommentIncludedPRs(
+        ILocalGitRepo sourceRepo,
+        string lastCommit,
+        string currentCommit,
+        string repoUri,
+        CancellationToken cancellationToken)
+    {
+        var gitRepoType = GitRepoUrlUtils.ParseTypeFromUri(repoUri);
+        // codeflow tests set the defaultRemote to a local path, we have to skip those
+        if (gitRepoType == GitRepoType.Local)
+        {
+            return;
+        }
+
+        var result = await sourceRepo.ExecuteGitCommand(["log", "--pretty=%s", $"{lastCommit}..{currentCommit}"], cancellationToken);
+        result.ThrowIfFailed($"Failed to get the list of commits between {lastCommit} and {currentCommit} in {sourceRepo.Path}");
+
+        (Regex regex, string prLinkFormat) = gitRepoType switch
+        {
+            GitRepoType.GitHub => (GitHubPullRequestNumberExtractionRegex, $"- {repoUri}/pull/{{0}}"),
+            GitRepoType.AzureDevOps => (AzDoPullRequestNumberExtractionRegex, $"- {repoUri}/pullrequest/{{0}}"),
+            _ => throw new NotSupportedException($"Repository type for URI '{repoUri}' is not supported for PR link extraction.")
+        };
+        var commitMessages = result.GetOutputLines();
+        StringBuilder str = new("PRs from original repository included in this codeflow update:");
+        foreach (var message in commitMessages)
+        {
+            var match = regex.Match(message);
+            if (match.Success && match.Groups.Count > 1)
+            {
+                str.AppendLine();
+                str.AppendFormat(prLinkFormat, match.Groups[1].Value);
+            }
+        }
+
+        _commentCollector.AddComment(str.ToString(), CommentType.Information);
+    }
+
     protected override async Task<Codeflow?> DetectCrossingFlow(
         Codeflow lastFlow,
         Backflow? lastBackFlow,

test/Microsoft.DotNet.DarcLib.Codeflow.Tests/ForwardFlowTests.cs

@@ -248,4 +248,3 @@ public async Task NonLinearForwardflowIsDetectedTest()
         await act.Should().ThrowAsync<NonLinearCodeflowException>();
     }
 }
-

test/ProductConstructionService.ScenarioTests/CodeFlowScenarioTestBase.cs

@@ -14,7 +14,7 @@ namespace ProductConstructionService.ScenarioTests;
 
 internal class CodeFlowScenarioTestBase : ScenarioTestBase
 {
-    protected async Task CheckForwardFlowGitHubPullRequest(
+    protected async Task<PullRequest> CheckForwardFlowGitHubPullRequest(
         (string Repo, string Commit)[] repoUpdates,
         string targetRepoName,
         string targetBranch,
@@ -62,6 +62,8 @@ protected async Task CheckForwardFlowGitHubPullRequest(
                 manifestRecord.CommitSha.Should().Be(update.Commit);
             }
         }
+
+        return pullRequest;
     }
 
     protected async Task CheckBackwardFlowGitHubPullRequest(
@@ -222,11 +224,10 @@ public static async Task CheckIfPullRequestCommentExists(
         string[] stringsExpectedInComment)
     {
         IReadOnlyList<IssueComment> comments = await GitHubApi.Issue.Comment.GetAllForIssue(TestParameters.GitHubTestOrg, targetRepo, pullRequest.Number);
-        var conflictComment = comments.First(comment => comment.Body.Contains("conflict"));
 
         foreach (var s in stringsExpectedInComment)
         {
-            conflictComment.Body.Should().Contain(s);
+            comments.Any(c => c.Body.Contains(s)).Should().BeTrue();
         }
     }
 }

test/ProductConstructionService.ScenarioTests/ScenarioTests/ScenarioTests_CodeFlow.cs

@@ -42,6 +42,7 @@ public async Task Vmr_ForwardFlowTest()
         var branchName = GetTestBranchName();
         var productRepo = GetGitHubRepoUrl(TestRepository.TestRepo1Name);
         var targetBranchName = GetTestBranchName();
+        var testPrNumber = 23;
 
         await using AsyncDisposableValue<string> testChannel = await CreateTestChannelAsync(channelName);
 
@@ -69,7 +70,7 @@ await CreateTargetBranchAndExecuteTest(targetBranchName, vmrDirectory.Directory,
                     await File.WriteAllTextAsync(newFilePath, TestFilesContent[TestFile1Name]);
 
                     await GitAddAllAsync();
-                    await GitCommitAsync("Add new file");
+                    await GitCommitAsync($"Change that would have been made in a pr (#{testPrNumber})");
 
                     // Push it to github
                     await using (await PushGitBranchAsync("origin", branchName))
@@ -92,12 +93,17 @@ await CreateTargetBranchAndExecuteTest(targetBranchName, vmrDirectory.Directory,
                         await TriggerSubscriptionAsync(subscriptionId.Value);
 
                         TestContext.WriteLine("Verifying subscription PR");
-                        await CheckForwardFlowGitHubPullRequest(
+                        var pr = await CheckForwardFlowGitHubPullRequest(
                             [(TestRepository.TestRepo1Name, repoSha)],
                             TestRepository.VmrTestRepoName,
                             targetBranchName,
                             [$"src/{TestRepository.TestRepo1Name}/{TestFile1Name}"],
                             TestFilePatches);
+
+                        await CheckIfPullRequestCommentExists(
+                            TestRepository.VmrTestRepoName,
+                            pr,
+                            [$"https://github.com/maestro-auth-test/maestro-test1/pull/{testPrNumber}"]);
                     }
                 }
             }

tools/ProductConstructionService.ReproTool/Operations/FlowCommitOperation.cs

@@ -57,34 +57,16 @@ internal override async Task RunAsync()
         Channel? channel = channels.FirstOrDefault(c => c.Name == _options.Channel)
             ?? await _localPcsApi.Channels.CreateChannelAsync("test", _options.Channel);
 
-        var (repoName, owner) = GitRepoUrlUtils.GetRepoNameAndOwner(_options.SourceRepository);
-        var (sourceRepoName, sourceOwner) = (repoName, owner);
+        var (sourceRepo, sourceOwner) = GitRepoUrlUtils.GetRepoNameAndOwner(_options.SourceRepository);
+        var (targetRepo, targetOwner) = GitRepoUrlUtils.GetRepoNameAndOwner(_options.TargetRepository);
 
-        bool? isBackflow = null;
-        try
-        {
-            (repoName, owner) = GitRepoUrlUtils.GetRepoNameAndOwner(_options.TargetRepository);
-            await _ghClient.Repository.Content.GetAllContents(owner, repoName, SourceMappingsPath);
-            isBackflow = false;
-        }
-        catch { }
-
-        if (!isBackflow.HasValue)
-        {
-            try
-            {
-                (repoName, owner) = GitRepoUrlUtils.GetRepoNameAndOwner(_options.SourceRepository);
-                await _ghClient.Repository.Content.GetAllContents(owner, repoName, SourceMappingsPath);
-                isBackflow = true;
-            }
-            catch { }
-        }
+        bool isForwardFlow = await IsForwardFlow(sourceOwner, sourceRepo, targetOwner, targetRepo);
 
         var subscriptions = await _localPcsApi.Subscriptions.ListSubscriptionsAsync(
             channelId: channel.Id,
             sourceRepository: _options.SourceRepository,
             targetRepository: _options.TargetRepository,
-            sourceEnabled: isBackflow.HasValue);
+            sourceEnabled: true);
 
         Subscription subscription = subscriptions.FirstOrDefault(s => s.TargetBranch == _options.TargetBranch)
             ?? await _localPcsApi.Subscriptions.CreateAsync(
@@ -99,17 +81,33 @@ internal override async Task RunAsync()
                     },
                     null)
                 {
-                    SourceEnabled = isBackflow.HasValue,
-                    SourceDirectory = isBackflow == true ? repoName : null,
-                    TargetDirectory = isBackflow == false ? sourceRepoName : null,
+                    SourceEnabled = true,
+                    SourceDirectory = isForwardFlow ? null : targetRepo,
+                    TargetDirectory = isForwardFlow ? sourceRepo : null,
                 });
 
-        var commit = (await _ghClient.Repository.Branch.Get(sourceOwner, sourceRepoName, _options.SourceBranch)).Commit;
+        string sourceCommit;
+
+        if (string.IsNullOrEmpty(_options.SourceCommit) && string.IsNullOrEmpty(_options.SourceBranch))
+        {
+            throw new ArgumentException("Please provide a source-branch or source-commit value.");
+        }
+
+        if (string.IsNullOrEmpty(_options.SourceCommit))
+        {
+            sourceCommit = (await _ghClient.Repository.Branch.Get(sourceOwner, sourceRepo, _options.SourceBranch))
+                .Commit.Sha;
+        }
+        else
+        {
+            sourceCommit = _options.SourceCommit;
+        }
 
         _logger.LogInformation("Creating build for {repo}@{branch} (commit {commit})",
             _options.SourceRepository,
             _options.SourceBranch,
-            Microsoft.DotNet.DarcLib.Commit.GetShortSha(commit.Sha));
+            Microsoft.DotNet.DarcLib.Commit.GetShortSha(sourceCommit));
+
         List<AssetData> assets;
         if (_options.RealBuildId > 0)
         {
@@ -126,13 +124,16 @@ internal override async Task RunAsync()
             ];
         }
 
+        _logger.LogInformation("Source commit is {}", sourceCommit);
+        _logger.LogInformation("Subscription is forward-flow: {}", isForwardFlow);
+
         var build = await _localPcsApi.Builds.CreateAsync(new BuildData(
-                commit.Sha,
+                sourceCommit,
                 "dnceng",
                 "internal",
                 $"{DateTime.UtcNow:yyyyMMdd}.{new Random().Next(1, 75)}",
-                $"https://dev.azure.com/dnceng/internal/_git/{owner}-{repoName}",
-                _options.SourceBranch,
+                $"https://dev.azure.com/dnceng/internal/_git/{sourceOwner}-{sourceRepo}",
+                _options.SourceBranch ?? sourceCommit,
                 released: false,
                 stable: false)
             {
@@ -149,4 +150,40 @@ internal override async Task RunAsync()
 
         _logger.LogInformation("Subscription triggered. Wait for a PR in {url}", $"{_options.TargetRepository}/pulls");
     }
+
+    private async Task<bool> IsVmr(string repoName, string repoOwner)
+    {
+        try
+        {
+            await _ghClient.Repository.Content.GetAllContents(repoOwner, repoName, SourceManifestPath);
+            return true;
+        }
+        catch (Octokit.ApiException e)
+        {
+            if (e.StatusCode == System.Net.HttpStatusCode.NotFound)
+            {
+                return false;
+            }
+            throw;
+        }
+    }
+
+    private async Task<bool> IsForwardFlow(
+        string sourceOwner,
+        string sourceRepo,
+        string targetOwner,
+        string targetRepo)
+    {
+        if (await IsVmr(targetRepo, targetOwner))
+        {
+            return true;
+        }
+
+        if (await IsVmr(sourceRepo, sourceOwner))
+        {
+            return false;
+        }
+
+        throw new InvalidOperationException("Neither the source nor the target repository appears to be a VMR.");
+    }
 }

tools/ProductConstructionService.ReproTool/Options/FlowCommitOptions.cs

@@ -16,8 +16,11 @@ internal class FlowCommitOptions : Options
     [Option("source-repo", HelpText = "Repo (full URI) to flow the commit from (must be under maestro-auth-test/)", Required = true)]
     public required string SourceRepository { get; init; }
 
-    [Option("source-branch", HelpText = "Branch whose commit will be flown", Required = true)]
-    public required string SourceBranch { get; init; }
+    [Option("source-branch", HelpText = "Either --source-branch or --source-commit is required. Branch whose commit will be flown. Ignored if source-commit is provided.", Required = false)]
+    public string? SourceBranch { get; init; }
+
+    [Option("source-commit", HelpText = "Either --source-branch or --source-commit is required. Source commit that will be flown.", Required = false)]
+    public string? SourceCommit { get; init; }
 
     [Option("target-repo", HelpText = "Repo (full URI) to flow the commit to (must be under maestro-auth-test/)", Required = true)]
     public required string TargetRepository { get; init; }
@@ -28,7 +31,7 @@ internal class FlowCommitOptions : Options
     [Option("packages", HelpText = "Name(s) of package(s) to include in the flown build", Required = false)]
     public IEnumerable<string> Packages { get; init; } = [];
 
-    [Option("realBuildId", HelpText = "Build to take assets from. Shouldn't be combined with packages", Required = false)]
+    [Option("assets-from-build", HelpText = "A real build id from which to take assets. Shouldn't be provided if using the --packages option", Required = false)]
     public int RealBuildId { get; init; }
 
     internal override Operation GetOperation(IServiceProvider sp)