fix(sync): wait for GitHub to index branch before creating PR#372
fix(sync): wait for GitHub to index branch before creating PR#372grokspawn wants to merge 1 commit into
Conversation
gh pr create fails with "Head sha can't be blank / No commits between main and upstream-sync" when called immediately after git push -f, because GitHub's refs API hasn't yet propagated the newly-pushed SHA. Poll the git/refs API until the indexed SHA matches the pushed SHA before attempting PR creation, eliminating the race condition.
📝 WalkthroughWalkthroughThe sync-upstream-pr Makefile target now records the pushed branch's SHA, force-pushes it, and polls the GitHub ref endpoint via ChangesSync-Upstream Push Indexing
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build/openshift/sync-upstream.mk`:
- Around line 57-66: The polling loop in sync-upstream.mk can finish all 12
attempts without ever confirming the branch index, but it currently falls
through silently. Update the loop around the gh api check for SYNC_BRANCH_NAME
so that after the for-loop completes without a match it emits a clear failure
message and exits non-zero instead of proceeding to PR creation; keep the
success path unchanged when INDEXED matches PUSHED_SHA.
- Line 54: The force-push step in the sync recipe does not stop the flow when it
fails, so the later polling and PR-creation logic can run against a stale ref.
Update the push step in the sync target to check the result of git push -f using
the existing shell recipe flow, and ensure the recipe exits immediately on
failure before reaching the wait/PR creation steps; use the sync branch and
origin remote variables already present in the target to keep the fix localized.
- Around line 58-59: The ref lookup in the sync polling logic is using the
plural GitHub endpoint path, which prevents INDEXED from ever matching the
pushed SHA. Update the gh api call in the sync-upstream.mk polling block to use
the singular git/ref/heads/$(SYNC_BRANCH_NAME) endpoint, keeping the rest of the
INDEXED assignment and timeout loop logic unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ee015d59-d0f2-43af-9e6f-0554a3561876
📒 Files selected for processing (1)
build/openshift/sync-upstream.mk
| @echo "📤 Pushing sync branch..." | ||
| @git push -f $(ORIGIN_REMOTE) $(SYNC_BRANCH_NAME) | ||
| @PUSHED_SHA=$$(git rev-parse $(SYNC_BRANCH_NAME)); \ | ||
| git push -f $(ORIGIN_REMOTE) $(SYNC_BRANCH_NAME); \ |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
git push -f failure is not checked.
The recipe continues (;) into the polling/PR-creation steps even if the force-push fails, wasting the 60s wait and potentially operating against a stale remote ref.
🔧 Proposed fix
- git push -f $(ORIGIN_REMOTE) $(SYNC_BRANCH_NAME); \
+ git push -f $(ORIGIN_REMOTE) $(SYNC_BRANCH_NAME) || { echo "❌ Push failed"; exit 1; }; \📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| git push -f $(ORIGIN_REMOTE) $(SYNC_BRANCH_NAME); \ | |
| git push -f $(ORIGIN_REMOTE) $(SYNC_BRANCH_NAME) || { echo "❌ Push failed"; exit 1; }; \ |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build/openshift/sync-upstream.mk` at line 54, The force-push step in the sync
recipe does not stop the flow when it fails, so the later polling and
PR-creation logic can run against a stale ref. Update the push step in the sync
target to check the result of git push -f using the existing shell recipe flow,
and ensure the recipe exits immediately on failure before reaching the wait/PR
creation steps; use the sync branch and origin remote variables already present
in the target to keep the fix localized.
| for i in $$(seq 1 12); do \ | ||
| INDEXED=$$(gh api "repos/$$OWNER_REPO/git/refs/heads/$(SYNC_BRANCH_NAME)" \ | ||
| --jq '.object.sha' 2>/dev/null || echo ""); \ | ||
| if [ "$$INDEXED" = "$$PUSHED_SHA" ]; then \ | ||
| echo "✅ Branch indexed."; \ | ||
| break; \ | ||
| fi; \ | ||
| echo " (attempt $$i/12: waiting for index, retrying in 5s...)"; \ | ||
| sleep 5; \ | ||
| done |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
No failure signal when polling times out.
If the loop exhausts all 12 attempts without a match, execution silently falls through to PR creation with no warning or non-zero exit — reintroducing exactly the race condition this change is meant to prevent, just without any diagnostic trail.
🔧 Proposed fix
for i in $$(seq 1 12); do \
- INDEXED=$$(gh api "repos/$$OWNER_REPO/git/refs/heads/$(SYNC_BRANCH_NAME)" \
+ INDEXED=$$(gh api "repos/$$OWNER_REPO/git/ref/heads/$(SYNC_BRANCH_NAME)" \
--jq '.object.sha' 2>/dev/null || echo ""); \
if [ "$$INDEXED" = "$$PUSHED_SHA" ]; then \
echo "✅ Branch indexed."; \
break; \
fi; \
echo " (attempt $$i/12: waiting for index, retrying in 5s...)"; \
sleep 5; \
- done
+ done; \
+ if [ "$$INDEXED" != "$$PUSHED_SHA" ]; then \
+ echo "⚠️ Warning: GitHub did not report the indexed SHA after 60s; proceeding anyway."; \
+ fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build/openshift/sync-upstream.mk` around lines 57 - 66, The polling loop in
sync-upstream.mk can finish all 12 attempts without ever confirming the branch
index, but it currently falls through silently. Update the loop around the gh
api check for SYNC_BRANCH_NAME so that after the for-loop completes without a
match it emits a clear failure message and exits non-zero instead of proceeding
to PR creation; keep the success path unchanged when INDEXED matches PUSHED_SHA.
| INDEXED=$$(gh api "repos/$$OWNER_REPO/git/refs/heads/$(SYNC_BRANCH_NAME)" \ | ||
| --jq '.object.sha' 2>/dev/null || echo ""); \ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target section with line numbers
sed -n '45,75p' build/openshift/sync-upstream.mk | cat -n
# Find nearby uses of the same GitHub API pattern
rg -n 'git/refs?/heads|INDEXED=|PUSHED_SHA|SYNC_BRANCH_NAME' build/openshift/sync-upstream.mk
# Look for any explicit failure handling after the polling loop
sed -n '75,110p' build/openshift/sync-upstream.mk | cat -nRepository: openshift/openshift-mcp-server
Length of output: 3355
🌐 Web query:
GitHub REST API git reference endpoint /repos/{owner}/{repo}/git/ref/{ref} matching refs /git/matching-refs docs
💡 Result:
The GitHub REST API provides two distinct, well-defined endpoints for interacting with Git references, designed to handle specific retrieval needs [1][2][3]. 1. Get a single reference: GET /repos/{owner}/{repo}/git/ref/{ref} This endpoint is used to retrieve a single, specific Git reference from your Git database [2]. The {ref} parameter must be the full path of the reference (e.g., heads/branch-name or tags/tag-name) [2]. If the requested reference does not exist, the API returns a 404 Not Found error [2]. 2. List matching references: GET /repos/{owner}/{repo}/git/matching-refs/{ref} This endpoint returns an array of references that match the supplied {ref} prefix [3][4]. Unlike the "Get a reference" endpoint, this is a wildcard-style search [3]. For example, if you provide heads/feature as the {ref}, it will return all branches starting with that prefix (e.g., heads/feature-a and heads/feature-b) [3][4]. If you call this endpoint without providing a {ref}, it returns all references in the repository [3]. If no references match your request, it returns an empty array rather than an error [3][5]. In summary, use "Get a reference" when you need to retrieve exactly one specific ref and want to handle cases where it might be missing [2][5]. Use "List matching references" when you need to retrieve a group of references based on a prefix or want to list all available references [3][5]. These endpoints effectively replaced older, less consistent reference-handling patterns in the GitHub API [5].
Citations:
- 1: https://docs.github.com/rest/git/refs
- 2: https://docs.github.com/enterprise-server%403.18/rest/git/refs
- 3: https://docs.github.com/en/enterprise-server@3.20/rest/git/refs?apiVersion=2022-11-28
- 4: https://docs.github.com/en/enterprise-server@3.5/rest/git/refs
- 5: Support new Git reference get/list endpoints google/go-github#1512
Use the singular GitHub ref endpoint here. gh api should call repos/$$OWNER_REPO/git/ref/heads/$(SYNC_BRANCH_NAME); git/refs/heads/... is the wrong path, so this poll never matches the pushed SHA and just waits out the full timeout.
🔧 Proposed fix
- INDEXED=$$(gh api "repos/$$OWNER_REPO/git/refs/heads/$(SYNC_BRANCH_NAME)" \
+ INDEXED=$$(gh api "repos/$$OWNER_REPO/git/ref/heads/$(SYNC_BRANCH_NAME)" \
--jq '.object.sha' 2>/dev/null || echo ""); \📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| INDEXED=$$(gh api "repos/$$OWNER_REPO/git/refs/heads/$(SYNC_BRANCH_NAME)" \ | |
| --jq '.object.sha' 2>/dev/null || echo ""); \ | |
| INDEXED=$$(gh api "repos/$$OWNER_REPO/git/ref/heads/$(SYNC_BRANCH_NAME)" \ | |
| --jq '.object.sha' 2>/dev/null || echo ""); \ |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build/openshift/sync-upstream.mk` around lines 58 - 59, The ref lookup in the
sync polling logic is using the plural GitHub endpoint path, which prevents
INDEXED from ever matching the pushed SHA. Update the gh api call in the
sync-upstream.mk polling block to use the singular
git/ref/heads/$(SYNC_BRANCH_NAME) endpoint, keeping the rest of the INDEXED
assignment and timeout loop logic unchanged.
|
@grokspawn: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Cali0707, grokspawn The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
/override "Red Hat Konflux / openshift-mcp-server-main-on-pull-request" |
|
@Cali0707: Overrode contexts on behalf of Cali0707: Red Hat Konflux / openshift-mcp-server-main-on-pull-request DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
2 participants
gh pr create fails with "Head sha can't be blank / No commits between main and upstream-sync" when called immediately after git push -f, because GitHub's refs API hasn't yet propagated the newly-pushed SHA.
Poll the git/refs API until the indexed SHA matches the pushed SHA before attempting PR creation, eliminating the race condition.
Summary by CodeRabbit