fix(automerge): validate head commit before merge - #39235
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0612cb687
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
If it's difficult to write a correct test, then don't write the test, since the fix is simple |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7050e6f1bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Fixes #39214.
Auto-merge already validates that the queued head commit still matches the pull request head before checking mergeability, but the expected head commit was not passed to the final merge operation.
Pass
expectedHeadCommitIDtopull_service.Mergeso the head commit is validated again when the merge is actually performed.The "automerge" related tests are already very messy, and it's very difficult to test the "validation" behaviors without data-race. Since the fix is simple, this PR doesn't introduce new tests.