Track commits made by the service - #6498
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens forward-flow auto-approval in Product Construction Service (PCS) by tracking the exact commits the service generated for a codeflow PR and refusing to auto-approve if the PR contains any additional commits. It also pins the approval review to the PR head SHA that was validated, preventing “approve after push” races.
Changes:
- Add a scoped
IServiceCommitTrackerand trackingILocalGitClient/ILocalLibGit2Clientwrappers to record commits created during a work item, and persist reachable service-generated SHAs onInProgressPullRequest. - Update codeflow approval to (1) refuse PRs containing commits not in
ServiceGeneratedCommitsand (2) approve via GitHub review pinned to the checked head SHA. - Add/update unit/scenario tests covering commit tracking and the new approval gate.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/ProductConstructionService/ProductConstructionService.ScenarioTests/ScenarioTests/ScenarioTests_CodeFlow.cs | Re-enables waiting for PR approval in codeflow scenario coverage. |
| test/ProductConstructionService/ProductConstructionService.DependencyFlow.Tests/ServiceCommitTrackerTests.cs | Adds tests for commit tracking + tracking git client wrappers. |
| test/ProductConstructionService/ProductConstructionService.DependencyFlow.Tests/PullRequestUpdaterTests.cs | Extends test scaffolding to include tracked commit SHAs and updated approval API. |
| test/ProductConstructionService/ProductConstructionService.DependencyFlow.Tests/CodeflowApprovalCheckTests.cs | Enables and expands approval-check tests to cover non-service commit rejection and SHA-pinned approvals. |
| src/ProductConstructionService/ProductConstructionService.DependencyFlow/ServiceCommitTracker.cs | Introduces commit tracker and tracking git client implementations. |
| src/ProductConstructionService/ProductConstructionService.DependencyFlow/PullRequestUpdaters/CodeFlowPullRequestUpdater.cs | Persists reachable service commits into PR state and enforces “no extra commits” before diff verification/approval. |
| src/ProductConstructionService/ProductConstructionService.DependencyFlow/Model/InProgressPullRequest.cs | Adds ServiceGeneratedCommits to persisted PR state. |
| src/ProductConstructionService/ProductConstructionService.DependencyFlow/DependencyFlowConfiguration.cs | Registers commit tracker and tracking git clients in DI. |
| src/Microsoft.DotNet.Darc/DarcLib/GitHubPullRequestApprover.cs | Updates approval API to include an explicit CommitId (approved head SHA) when creating the review. |
Comment on lines
+62
to
+68
| foreach (string commit in candidates) | ||
| { | ||
| if (await gitClient.IsAncestorCommit(repositoryPath, commit, branch)) | ||
| { | ||
| reachableCommits.Add(commit); | ||
| } | ||
| } |
|
|
||
| // Assert | ||
| _tracker.Verify( | ||
| t => t.TrackCommit(It.Is<NativePath>(p => p == RepoPath), createdCommit), |
|
|
||
| // Assert | ||
| _tracker.Verify( | ||
| t => t.ReplaceCommit(It.Is<NativePath>(p => p == RepoPath), originalCommit, amendedCommit), |
oleksandr-didyk
requested changes
Jul 20, 2026
oleksandr-didyk
left a comment
oleksandr-didyk
left a comment
Contributor
There was a problem hiding this comment.
Few wording nitpicks and a minor comment for simplifying a call, other than that LGTM 🙇
| Url = pr.Url, | ||
| HeadBranch = prBranch, | ||
| HeadBranchSha = pr.HeadBranchSha, | ||
| ServiceGeneratedCommits = await _serviceCommitTracker.GetReachableCommitsAsync( |
Contributor
There was a problem hiding this comment.
We explicitly provide an empty previouslyTrackedCommits. That would just lead to this call returning an empty list. Why not just set ServiceGeneratedCommits to []?
Member
Author
There was a problem hiding this comment.
we provide an empty previously tracked commits because the PR is new. so in this case, we'll get only PRs that are in the PR.
In the case of an update, we pass the old service generated commits and concat it with the commits made in this PR
Co-authored-by: Oleksandr Didyk <106967057+oleksandr-didyk@users.noreply.github.com>
Co-authored-by: Oleksandr Didyk <106967057+oleksandr-didyk@users.noreply.github.com>
Co-authored-by: Oleksandr Didyk <106967057+oleksandr-didyk@users.noreply.github.com>
Co-authored-by: Oleksandr Didyk <106967057+oleksandr-didyk@users.noreply.github.com>
premun
approved these changes
Jul 21, 2026
oleksandr-didyk
approved these changes
Jul 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#6499