Skip to content

GitHubPusher: add commits instead of resetting branch #133

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.

Sign up for GitHub

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

Merged
sorah merged 1 commit into from
Mar 2, 2026
Merged

GitHubPusher: add commits instead of resetting branch #133

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 7 additions & 10 deletions app/jobs/generate_sponsors_yaml_file_job.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,26 +169,23 @@ def push
return
end

reset_branch
ensure_branch
commit_content
create_or_update_pull_request
end

private

def reset_branch
begin
octokit.delete_branch(@repo.name, @branch_name)
rescue Octokit::UnprocessableEntity
end

head = octokit.branch(@repo.name, base_branch)
octokit.create_ref(@repo.name, "refs/heads/#{@branch_name}", head[:commit][:sha])
def ensure_branch
octokit.branch(@repo.name, @branch_name)
rescue Octokit::NotFound
head_sha = octokit.branch(@repo.name, base_branch)[:commit][:sha]
octokit.create_ref(@repo.name, "refs/heads/#{@branch_name}", head_sha)
Comment on lines +179 to +183
Copy link
Copy Markdown

@sentry sentry Bot Mar 2, 2026

Choose a reason for hiding this comment

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

Bug: The ensure_branch method has a race condition where concurrent jobs can both attempt to create the same branch, causing the second job to crash with an unhandled exception.
Severity: HIGH

Suggested Fix

Modify the ensure_branch method to rescue the Octokit::UnprocessableEntity exception. If this error occurs, it means another job has already created the branch, which is the desired state, so the exception can be safely ignored.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: app/jobs/generate_sponsors_yaml_file_job.rb#L179-L183

Potential issue: The `ensure_branch` method is not atomic. When multiple
`GenerateSponsorsYamlFileJob` instances run concurrently for the same conference, they
can both check if a branch exists and find it missing. The first job will create the
branch successfully. When the second job attempts to create the same branch using
`octokit.create_ref`, the GitHub API will raise an `Octokit::UnprocessableEntity`
exception. Because this exception is not handled within the `ensure_branch` method, the
second job will crash, leading to a failed background job.

Did we get this right? 👍 / 👎 to inform future reviews.

end

def commit_content
begin
blob_sha = octokit.contents(@repo.name, path: @filepath, ref: base_branch)[:sha]
blob_sha = octokit.contents(@repo.name, path: @filepath, ref: @branch_name)[:sha]
rescue Octokit::NotFound
blob_sha = nil
end
Expand Down