Skip to content

Drop a comment with files that fail the approval check - #6604

Merged
dkurepa merged 3 commits into
dotnet:mainfrom
dkurepa:dkurepa/CommenFilesThatCauseCheckFail
Aug 18, 2026
Merged

Drop a comment with files that fail the approval check#6604
dkurepa merged 3 commits into
dotnet:mainfrom
dkurepa:dkurepa/CommenFilesThatCauseCheckFail

Conversation

@dkurepa

@dkurepa dkurepa commented Aug 17, 2026

Copy link
Copy Markdown
Member

Copilot AI lite review requested due to automatic review settings August 17, 2026 12:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the forward-flow “source diff” verification pipeline so it can report which files are mismatched (instead of only pass/fail), and uses that to post an informational PR comment when auto-approval is withheld due to unexpected diffs.

Changes:

  • Change ICodeflowSourceDiffVerifier.ForwardFlowMatchesSourceDiffAsync to return a sorted list of mismatched mapping-relative paths.
  • Update codeflow tests and PCS dependency-flow tests to assert on mismatched paths (empty list vs specific filenames).
  • In CodeFlowPullRequestUpdater, add an informational PR comment listing mismatched files when the approval check fails.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tools/ProductConstructionService.ReproTool/Operations/Operation.cs Changes ReproTool’s created subscriptions to default autoApprove on.
test/ProductConstructionService/ProductConstructionService.DependencyFlow.Tests/PullRequestUpdaterTests.cs Updates mock verifier return type from bool to mismatched-file list.
test/Darc/Microsoft.DotNet.DarcLib.Codeflow.Tests/ForwardFlowTests.cs Updates assertions to expect “no mismatches” via empty list.
test/Darc/Microsoft.DotNet.DarcLib.Codeflow.Tests/CodeFlowUpdatingPRsTests.cs Updates assertions to expect mismatches to include specific files.
test/Darc/Microsoft.DotNet.DarcLib.Codeflow.Tests/CodeFlowTestsBase.cs Updates helper contract + docs for mismatched-file list return type.
test/Darc/Microsoft.DotNet.DarcLib.Codeflow.Tests/CodeflowSourceDiffVerifierTests.cs Updates unit tests to assert mismatched paths instead of true/false.
src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdaters/CodeFlowPullRequestUpdater.cs Posts a PR comment listing mismatched files when auto-approval is blocked. (Also currently has a commented-out approval gate.)
src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/CodeflowSourceDiffVerifier.cs Implements mismatch collection and returns sorted mismatched path list.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +750 to +757
//if (!await remote.IsLastPushApprovalRequiredAsync(subscription.TargetRepository, prInfo.BaseBranch))
//{
// _logger.LogInformation(
// "Skipping codeflow approval check for PR {prUrl} because branch {branch} does not require approval of the most recent push",
// pr.Url,
// prInfo.BaseBranch);
// return;
//}
Comment on lines +816 to +828
_commentCollector.AddComment(
$"""
This pull request was not automatically approved because the following files did not match the source diff:

{mismatchedFileList}

Please manually review these files to ensure their changes are expected before merging this pull request.
""",
CommentType.Information);
await _pullRequestCommenter.PostCollectedCommentsAsync(
pr.Url,
subscription.TargetRepository,
[]);
Comment on lines 94 to 105
ClientSubscriptionYaml subscription = new(
channel: channelName,
sourceRepository: sourceRepository,
targetRepository: targetRepository,
targetBranch: targetBranch,
sourceEnabled: sourceEnabled,
batchable: false,
enabled: true,
id: Guid.NewGuid(),
updateFrequency: ClientUpdateFrequency.None,
autoApprove: false)
autoApprove: true)
{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dkurepa maybe we don't want this one? Was it part of testing?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd say there's no harm in having it there, and if we keep making changes to auto approval we'll have to keep setting it to true anyway. Might as well just make the change since it's just the repro tool

@dkurepa
dkurepa enabled auto-merge (squash) August 18, 2026 13:20
@dkurepa
dkurepa merged commit bf3fd7b into dotnet:main Aug 18, 2026
8 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants