Make sure comment about conflicts is sent always

[premun]

Resolves #4854

[Copilot]

Pull Request Overview

This PR ensures that a comment about conflicts is always sent by updating conflict exception handling throughout the codebase. Key changes include updating exception constructors to accept error strings from StandardError, refactoring property names from FilesInConflict to ConflictedFiles, and removing conditional checks that prevented notification when no version files conflicted.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
test/ProductConstructionService.DependencyFlow.Tests/PullRequestUpdaterTests.cs	Updated test to reflect changes in exception constructor parameters.
src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestConflictNotifier.cs	Adjusted property name usage and removed early exits to always send conflict notifications.
src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrForwardFlower.cs	Updated exception throwing to use StandardError for conflict details.
src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrBackflower.cs	Made similar adjustments for conflict exception handling in backflow scenarios.
src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/Exceptions.cs	Refactored ConflictInPrBranchException with updated parsing logic and new collection expressions.
src/Microsoft.DotNet.Darc/DarcLib/Helpers/ProcessExecutionResult.cs	Replaced generic Exception with a custom ProcessFailedException for improved error reporting.

Comments suppressed due to low confidence (8)

test/ProductConstructionService.DependencyFlow.Tests/PullRequestUpdaterTests.cs:506

• Ensure the provided error message pattern in the new constructor accurately captures all expected conflict messages, as this replaces the previous gitMergeResult parameter.

.Throws(() => new ConflictInPrBranchException(

src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestConflictNotifier.cs:66

• Update the loop to use the new property 'ConflictedFiles' and verify that all references to conflicting files are consistent with this change.

foreach (var file in conflictException.ConflictedFiles)

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

• Using 'e.Result.StandardError' aligns with the updated exception constructor; please confirm that 'StandardError' reliably contains the complete error details needed for conflict parsing.

throw new ConflictInPrBranchException(e.Result.StandardError, targetBranch, mapping.Name, isForwardFlow: true);

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

• Confirm that passing 'e.Result.StandardError' for backflow scenarios provides sufficient error details for accurate conflict reporting.

throw new ConflictInPrBranchException(e.Result.StandardError, targetBranch, mapping.Name, isForwardFlow: false);

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

• Ensure that catching ProcessFailedException and rethrowing as ConflictInPrBranchException preserves all necessary error details for troubleshooting merge conflicts.

throw new ConflictInPrBranchException(e.ExecutionResult.StandardError, targetBranch, mapping.Name, isForwardFlow: false);

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/Exceptions.cs:88

• Ensure all file paths processed here have at least three segments to avoid potential index out-of-range errors during the split operation.

return [..filesInConflict.Select(file => file.Split('/', 3)[2]).Distinct()];

src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/Exceptions.cs:93

• Review the file path normalization logic to ensure it fully meets expected formats for all backflow conflict cases.

return [..filesInConflict.Distinct().Select(file => $"src/{mappingName}/{file}")];

src/Microsoft.DotNet.Darc/DarcLib/Helpers/ProcessExecutionResult.cs:21

• The change to a custom ProcessFailedException improves error reporting; ensure that all consumers of ProcessExecutionResult are updated to handle this new exception type appropriately.

throw new ProcessFailedException(this, failureMessage);

[adamzip]

Is there no better way to find that we are in this situation than matching hardcoded strings?
Would it be possible to use LibGit2Sharp to recognize that a merge conflict occurred while running git merge?

[premun]

LibGit2Sharp would just fail and not tell us any details. It only returns a MergeResult.Conflicts. The merge can fail for more reasons and since there's no special exit code from git, we do have to parse the outputs unfortunately.