From 84aec8a9816511db902b58fed44140d59a4df781 Mon Sep 17 00:00:00 2001 From: Sorah Fukumori Date: Mon, 2 Mar 2026 06:32:12 +0900 Subject: [PATCH 1/4] GenerateSponsorsYamlFileJob: stable branch, reuse PR Previously each edit created a new branch (sponsor-app/) and PR, leaving stale PRs open. Now uses a stable branch per conference (sponsor-app/) and reuses the existing open PR. - Staleness check: skip push if branch already has newer editing history data - Optimistic concurrency: retry on blob_sha conflict when our data is fresher, bail otherwise - PR reuse: find existing open PR on the branch and update its title, with race-condition handling on create Co-Authored-By: Claude Opus 4.6 --- app/jobs/generate_sponsors_yaml_file_job.rb | 77 ++++++++++++++++----- 1 file changed, 61 insertions(+), 16 deletions(-) diff --git a/app/jobs/generate_sponsors_yaml_file_job.rb b/app/jobs/generate_sponsors_yaml_file_job.rb index 136f67c9..2eddd425 100644 --- a/app/jobs/generate_sponsors_yaml_file_job.rb +++ b/app/jobs/generate_sponsors_yaml_file_job.rb @@ -126,13 +126,23 @@ def json_data @json_data = data ? "#{data.to_json}\n" : nil end + MAX_PUSH_RETRIES = 3 + def push_to_github return unless repo return if yaml_data.nil? # to generate push_id = @last_id || "event-#{@last_event_editing_history_id}" - @branch_name = "sponsor-app/#{push_id}" + @branch_name = "sponsor-app/#{@conference.slug}" @filepath = repo.path + pr_title = "Update sponsors.yml for #{@conference.slug} (#{push_id})" + + # Check if existing branch already has newer data + if branch_has_newer_data? + Rails.logger.info "GenerateSponsorsYamlFileJob: branch has newer data, skipping" + return + end + # Delete and recreate branch from base HEAD begin octokit.delete_branch(repo.name, @branch_name) rescue Octokit::UnprocessableEntity @@ -141,27 +151,35 @@ def push_to_github head = octokit.branch(repo.name, base_branch) octokit.create_ref(repo.name, "refs/heads/#{@branch_name}", head[:commit][:sha]) + # Get blob_sha from base branch (acts as optimistic lock) begin blob_sha = octokit.contents(repo.name, path: @filepath)[:sha] rescue Octokit::NotFound blob_sha = nil end - octokit.update_contents( - repo.name, - @filepath, - "Update sponsors.yml for #{@conference.slug} (#{push_id})", - blob_sha, - yaml_data, - branch: @branch_name, - ) - octokit.create_pull_request( - repo.name, - base_branch, - @branch_name, - "Update sponsors.yml for #{@conference.slug} (#{push_id})", - nil, - ) + push_retries = 0 + begin + octokit.update_contents(repo.name, @filepath, pr_title, blob_sha, yaml_data, branch: @branch_name) + rescue Octokit::Conflict, Octokit::UnprocessableEntity => e + # Conflict: another job committed first. Check if our data is newer. + if push_retries < MAX_PUSH_RETRIES && !branch_has_newer_data? + push_retries += 1 + # Re-read blob_sha from the branch (now updated by the other job) + begin + blob_sha = octokit.contents(repo.name, path: @filepath, ref: @branch_name)[:sha] + rescue Octokit::NotFound + blob_sha = nil + end + Rails.logger.info "GenerateSponsorsYamlFileJob: commit conflict, retrying (#{push_retries}/#{MAX_PUSH_RETRIES})" + retry + else + Rails.logger.info "GenerateSponsorsYamlFileJob: commit conflict and branch has newer data (or max retries), skipping" + return + end + end + + create_or_update_pull_request(pr_title) end # For debugging @@ -171,6 +189,33 @@ def self.get_octokit(repo) private + def branch_has_newer_data? + existing = octokit.contents(repo.name, path: @filepath, ref: @branch_name) + existing_content = Base64.decode64(existing[:content]) + if existing_content =~ /last_editing_history: (\d+)/ + return @last_id && $1.to_i > @last_id + end + false + rescue Octokit::NotFound + false # Branch or file doesn't exist + end + + def create_or_update_pull_request(pr_title) + owner = repo.name.split('/')[0] + existing_prs = octokit.pull_requests(repo.name, state: 'open', head: "#{owner}:#{@branch_name}") + if existing_prs.any? + octokit.update_pull_request(repo.name, existing_prs[0][:number], title: pr_title) + else + begin + octokit.create_pull_request(repo.name, base_branch, @branch_name, pr_title, nil) + rescue Octokit::UnprocessableEntity + # Concurrent job already created PR + existing_prs = octokit.pull_requests(repo.name, state: 'open', head: "#{owner}:#{@branch_name}") + octokit.update_pull_request(repo.name, existing_prs[0][:number], title: pr_title) if existing_prs.any? + end + end + end + def github_installation @github_installation ||= GithubInstallation.new(repo.name, branch: repo.branch) end From e6151659ad5d53365542cbf52697ce489176f8b7 Mon Sep 17 00:00:00 2001 From: Sorah Fukumori Date: Mon, 2 Mar 2026 06:37:26 +0900 Subject: [PATCH 2/4] extract GitHubPusher from GenerateSponsorsYamlFileJob Separate data generation from GitHub interaction by moving push_to_github internals (branch reset, commit with retry, staleness check, PR upsert) into a nested GitHubPusher PORO. The pusher takes explicit constructor args instead of relying on job instance variables. Co-Authored-By: Claude Opus 4.6 --- app/jobs/generate_sponsors_yaml_file_job.rb | 170 +++++++++++--------- 1 file changed, 96 insertions(+), 74 deletions(-) diff --git a/app/jobs/generate_sponsors_yaml_file_job.rb b/app/jobs/generate_sponsors_yaml_file_job.rb index 2eddd425..786dc643 100644 --- a/app/jobs/generate_sponsors_yaml_file_job.rb +++ b/app/jobs/generate_sponsors_yaml_file_job.rb @@ -1,6 +1,4 @@ class GenerateSponsorsYamlFileJob < ApplicationJob - delegate :octokit, :base_branch, to: :github_installation - def perform(conference, push: true) @conference = conference @@ -126,97 +124,121 @@ def json_data @json_data = data ? "#{data.to_json}\n" : nil end - MAX_PUSH_RETRIES = 3 - def push_to_github return unless repo - return if yaml_data.nil? # to generate + return if yaml_data.nil? + push_id = @last_id || "event-#{@last_event_editing_history_id}" - @branch_name = "sponsor-app/#{@conference.slug}" - @filepath = repo.path - pr_title = "Update sponsors.yml for #{@conference.slug} (#{push_id})" - - # Check if existing branch already has newer data - if branch_has_newer_data? - Rails.logger.info "GenerateSponsorsYamlFileJob: branch has newer data, skipping" - return - end + GitHubPusher.new( + conference: @conference, + filepath: repo.path, + content: yaml_data, + last_editing_history_id: @last_id, + push_id:, + ).push + end - # Delete and recreate branch from base HEAD - begin - octokit.delete_branch(repo.name, @branch_name) - rescue Octokit::UnprocessableEntity - end + # For debugging + def self.get_octokit(repo) + GithubInstallation.new(repo).octokit + end - head = octokit.branch(repo.name, base_branch) - octokit.create_ref(repo.name, "refs/heads/#{@branch_name}", head[:commit][:sha]) + class GitHubPusher + MAX_RETRIES = 3 - # Get blob_sha from base branch (acts as optimistic lock) - begin - blob_sha = octokit.contents(repo.name, path: @filepath)[:sha] - rescue Octokit::NotFound - blob_sha = nil + delegate :octokit, :base_branch, to: :github_installation + + def initialize(conference:, filepath:, content:, last_editing_history_id:, push_id:) + @conference = conference + @repo = conference.github_repo + @filepath = filepath + @content = content + @last_id = last_editing_history_id + @branch_name = "sponsor-app/#{conference.slug}" + @pr_title = "Update sponsors.yml for #{conference.slug} (#{push_id})" end - push_retries = 0 - begin - octokit.update_contents(repo.name, @filepath, pr_title, blob_sha, yaml_data, branch: @branch_name) - rescue Octokit::Conflict, Octokit::UnprocessableEntity => e - # Conflict: another job committed first. Check if our data is newer. - if push_retries < MAX_PUSH_RETRIES && !branch_has_newer_data? - push_retries += 1 - # Re-read blob_sha from the branch (now updated by the other job) - begin - blob_sha = octokit.contents(repo.name, path: @filepath, ref: @branch_name)[:sha] - rescue Octokit::NotFound - blob_sha = nil - end - Rails.logger.info "GenerateSponsorsYamlFileJob: commit conflict, retrying (#{push_retries}/#{MAX_PUSH_RETRIES})" - retry - else - Rails.logger.info "GenerateSponsorsYamlFileJob: commit conflict and branch has newer data (or max retries), skipping" + def push + return unless @repo + + if branch_has_newer_data? + Rails.logger.info "GenerateSponsorsYamlFileJob: branch has newer data, skipping" return end - end - create_or_update_pull_request(pr_title) - end + reset_branch + commit_content + create_or_update_pull_request + end - # For debugging - def self.get_octokit(repo) - GithubInstallation.new(repo).octokit - end + private - private + def reset_branch + begin + octokit.delete_branch(@repo.name, @branch_name) + rescue Octokit::UnprocessableEntity + end - def branch_has_newer_data? - existing = octokit.contents(repo.name, path: @filepath, ref: @branch_name) - existing_content = Base64.decode64(existing[:content]) - if existing_content =~ /last_editing_history: (\d+)/ - return @last_id && $1.to_i > @last_id + head = octokit.branch(@repo.name, base_branch) + octokit.create_ref(@repo.name, "refs/heads/#{@branch_name}", head[:commit][:sha]) end - false - rescue Octokit::NotFound - false # Branch or file doesn't exist - end - def create_or_update_pull_request(pr_title) - owner = repo.name.split('/')[0] - existing_prs = octokit.pull_requests(repo.name, state: 'open', head: "#{owner}:#{@branch_name}") - if existing_prs.any? - octokit.update_pull_request(repo.name, existing_prs[0][:number], title: pr_title) - else + def commit_content begin - octokit.create_pull_request(repo.name, base_branch, @branch_name, pr_title, nil) - rescue Octokit::UnprocessableEntity - # Concurrent job already created PR - existing_prs = octokit.pull_requests(repo.name, state: 'open', head: "#{owner}:#{@branch_name}") - octokit.update_pull_request(repo.name, existing_prs[0][:number], title: pr_title) if existing_prs.any? + blob_sha = octokit.contents(@repo.name, path: @filepath)[:sha] + rescue Octokit::NotFound + blob_sha = nil + end + + retries = 0 + begin + octokit.update_contents(@repo.name, @filepath, @pr_title, blob_sha, @content, branch: @branch_name) + rescue Octokit::Conflict, Octokit::UnprocessableEntity => e + if retries < MAX_RETRIES && !branch_has_newer_data? + retries += 1 + begin + blob_sha = octokit.contents(@repo.name, path: @filepath, ref: @branch_name)[:sha] + rescue Octokit::NotFound + blob_sha = nil + end + Rails.logger.info "GenerateSponsorsYamlFileJob: commit conflict, retrying (#{retries}/#{MAX_RETRIES})" + retry + else + Rails.logger.info "GenerateSponsorsYamlFileJob: commit conflict and branch has newer data (or max retries), skipping" + return + end end end - end - def github_installation - @github_installation ||= GithubInstallation.new(repo.name, branch: repo.branch) + def branch_has_newer_data? + existing = octokit.contents(@repo.name, path: @filepath, ref: @branch_name) + existing_content = Base64.decode64(existing[:content]) + if existing_content =~ /last_editing_history: (\d+)/ + return @last_id && $1.to_i > @last_id + end + false + rescue Octokit::NotFound + false # Branch or file doesn't exist + end + + def create_or_update_pull_request + owner = @repo.name.split('/')[0] + existing_prs = octokit.pull_requests(@repo.name, state: 'open', head: "#{owner}:#{@branch_name}") + if existing_prs.any? + octokit.update_pull_request(@repo.name, existing_prs[0][:number], title: @pr_title) + else + begin + octokit.create_pull_request(@repo.name, base_branch, @branch_name, @pr_title, nil) + rescue Octokit::UnprocessableEntity + # Concurrent job already created PR + existing_prs = octokit.pull_requests(@repo.name, state: 'open', head: "#{owner}:#{@branch_name}") + octokit.update_pull_request(@repo.name, existing_prs[0][:number], title: @pr_title) if existing_prs.any? + end + end + end + + def github_installation + @github_installation ||= GithubInstallation.new(@repo.name, branch: @repo.branch) + end end end From 4057c11678ff1779dde58c9404022db3f7677352 Mon Sep 17 00:00:00 2001 From: Sorah Fukumori Date: Mon, 2 Mar 2026 15:17:04 +0900 Subject: [PATCH 3/4] GitHubPusher: compare both history IDs for staleness The old check only compared last_editing_history, so event-only changes could overwrite newer branch data (or skip when they shouldn't). Always emit both IDs in the YAML comment (defaulting to 0) and compare both with domination logic: branch is newer only when both IDs are >= ours and at least one is strictly >. Co-Authored-By: Claude Opus 4.6 --- app/jobs/generate_sponsors_yaml_file_job.rb | 24 +++++++++++++-------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/app/jobs/generate_sponsors_yaml_file_job.rb b/app/jobs/generate_sponsors_yaml_file_job.rb index 786dc643..5766f36f 100644 --- a/app/jobs/generate_sponsors_yaml_file_job.rb +++ b/app/jobs/generate_sponsors_yaml_file_job.rb @@ -107,9 +107,10 @@ def yaml_data end combined_data = data ? data.merge("_events" => events) : { "_events" => events } - comment_parts = [] - comment_parts << "last_editing_history: #{@last_id}" if @last_id - comment_parts << "last_event_editing_history: #{@last_event_editing_history_id}" if @last_event_editing_history_id + comment_parts = [ + "last_editing_history: #{@last_id || 0}", + "last_event_editing_history: #{@last_event_editing_history_id || 0}", + ] @yaml_data = [ "# #{comment_parts.join(', ')}", combined_data.to_yaml, @@ -134,6 +135,7 @@ def push_to_github filepath: repo.path, content: yaml_data, last_editing_history_id: @last_id, + last_event_editing_history_id: @last_event_editing_history_id, push_id:, ).push end @@ -148,12 +150,13 @@ class GitHubPusher delegate :octokit, :base_branch, to: :github_installation - def initialize(conference:, filepath:, content:, last_editing_history_id:, push_id:) + def initialize(conference:, filepath:, content:, last_editing_history_id:, last_event_editing_history_id:, push_id:) @conference = conference @repo = conference.github_repo @filepath = filepath @content = content - @last_id = last_editing_history_id + @last_id = last_editing_history_id || 0 + @last_event_id = last_event_editing_history_id || 0 @branch_name = "sponsor-app/#{conference.slug}" @pr_title = "Update sponsors.yml for #{conference.slug} (#{push_id})" end @@ -213,10 +216,13 @@ def commit_content def branch_has_newer_data? existing = octokit.contents(@repo.name, path: @filepath, ref: @branch_name) existing_content = Base64.decode64(existing[:content]) - if existing_content =~ /last_editing_history: (\d+)/ - return @last_id && $1.to_i > @last_id - end - false + + branch_last_id = existing_content =~ /\blast_editing_history: (\d+)/ ? $1.to_i : 0 + branch_last_event_id = existing_content =~ /\blast_event_editing_history: (\d+)/ ? $1.to_i : 0 + + # Branch is newer when both IDs are >= ours and at least one is strictly greater + branch_last_id >= @last_id && branch_last_event_id >= @last_event_id && + (branch_last_id > @last_id || branch_last_event_id > @last_event_id) rescue Octokit::NotFound false # Branch or file doesn't exist end From 4029ec0f3e3f0ffc2beef9fadcd00e207d5bf027 Mon Sep 17 00:00:00 2001 From: Sorah Fukumori Date: Mon, 2 Mar 2026 15:18:42 +0900 Subject: [PATCH 4/4] GitHubPusher: read initial blob_sha from base_branch Without ref:, the API defaults to the repo's default branch, which differs from base_branch when the conference specifies a custom branch. This caused a guaranteed conflict on the first update_contents attempt, wasting a retry. Co-Authored-By: Claude Opus 4.6 --- app/jobs/generate_sponsors_yaml_file_job.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/jobs/generate_sponsors_yaml_file_job.rb b/app/jobs/generate_sponsors_yaml_file_job.rb index 5766f36f..fcb835a3 100644 --- a/app/jobs/generate_sponsors_yaml_file_job.rb +++ b/app/jobs/generate_sponsors_yaml_file_job.rb @@ -188,7 +188,7 @@ def reset_branch def commit_content begin - blob_sha = octokit.contents(@repo.name, path: @filepath)[:sha] + blob_sha = octokit.contents(@repo.name, path: @filepath, ref: base_branch)[:sha] rescue Octokit::NotFound blob_sha = nil end