-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Renovate deleted a PR branch that contained additional non-bot commits #2391
Comments
Looks like it was a "race condition":
Renovate started running right after the PR closure as it triggers a webhook. It sees that the branch exists but the PR is closed. Normally people want that branch deleted, so that's what happened. However in the meantime (after Renovate retrieved the PR list, before it took action) the PR was reopened so then by deleting the branch, it also re-closed the PR. It doesn't look like the extra commit or rebasing had anything to do with it, so the shorter reproduction steps would be:
I'll need to think how to best avoid this type of race condition. We don't want to be re-fetching every single branch/PR/issue at all times just in case one has changed since we started, as it would greatly increase the API use and potentially exhaust it. |
Thank you for looking into this. The reason I thought the human commits would have made a difference - is that normally they result in the "this PR has been modified, Renovate won't update it any more" message. However even before the PR was closed, we hadn't received such a message. In addition, perhaps Renovate should be checking when deleting branches, and only doing so if they didn't contain extra commits? (These extra commits should presumably still show up in the cached PR list, so not need additional API requests, yeah?) |
@edmorley actually the edited notification message was there from 9 days ago: And I think in a case like this, people wouldn't want the branch deleted regardless of number of commits. i.e. if you close and then reopen a PR by accident, you don't want it then deleted by Renovate. So maybe the requirement is checking one more time (with a cache-busted request) that the PR is still closed before deleting the branch. |
Ah sorry missed that bot message! Yeah agree perhaps a cache-busted request is the only way. Though hopefully the "PR closed without merge" case is infrequent so not an issue with API request credits. |
🎉 This issue has been resolved in version 13.46.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What Renovate type are you using?
Renovate GitHub App
Describe the bug
Renovate repeatedly deletes a renovate branch that was manually updated by a human.
See:
neutrinojs/neutrino#1002
To Reproduce
Steps to reproduce the behavior:
master
Expected behavior
Renovate doesn't delete the branch / modify the contents, since a human had added a second commit.
The text was updated successfully, but these errors were encountered: