fix: use PR-based sync instead of direct push to protected main - #44
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughImplements an idempotent branch-and-PR sync: uses GH_TOKEN (PARENT_REPO_PAT) to create or reuse branch Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as Action Runner
participant GHCLI as GitHub CLI / API
participant ParentRepo as Parent Repository
Runner->>GHCLI: authenticate with GH_TOKEN (PARENT_REPO_PAT)
Runner->>ParentRepo: check for branch sync/${REPO_NAME}/${SHORT_SHA}
alt branch exists
Runner->>ParentRepo: find existing PR for branch
alt PR exists
Runner->>GHCLI: enable auto-merge on PR
GHCLI-->>Runner: PR URL / status
else no PR
Runner->>ParentRepo: fetch & switch to branch (proceed to commit if needed)
end
else branch missing
Runner->>ParentRepo: create branch, commit cleaned message + Co‑authored-by, push
alt push race/failure
Runner->>ParentRepo: fetch & re-check branch/PR
end
end
Runner->>GHCLI: create PR (title/body) or find existing
Runner->>GHCLI: enable auto-merge (squash + delete branch)
GHCLI-->>Runner: return PR URL / status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR reworks the Key changes:
Issue: The Confidence Score: 2/5
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A([workflow_dispatch / repository_dispatch]) --> B[Extract payload<br/>REPO_NAME, SHORT_SHA, MESSAGE, TYPE, ACTOR]
B --> C{Branch<br/>sync/REPO/SHA<br/>exists remotely?}
C -- Yes --> D{Open PR exists<br/>for branch + base main?}
D -- Yes --> E[gh pr merge --auto --squash<br/>--delete-branch]
E --> F([exit 0])
D -- No --> G[git fetch + checkout<br/>existing branch]
G --> H[gh pr create<br/>--title --body --head --base main]
C -- No --> I[git checkout -b BRANCH<br/>git commit --allow-empty<br/>git push -u origin BRANCH]
I --> H
H --> J[gh pr merge PR_URL<br/>--auto --squash --delete-branch]
J --> K[echo Created PR: PR_URL]
K --> L([Log sync step])
Last reviewed commit: ca2bac0 |
| git checkout -b "$BRANCH" | ||
| git commit --allow-empty -m "$COMMIT_MSG" | ||
| git push -u origin "$BRANCH" |
There was a problem hiding this comment.
Branch push fails on workflow retry
If this workflow fails after the git push (e.g. during gh pr create) and is retried, the second run will fail at git push -u origin "$BRANCH" with "already exists" because the branch sync/${REPO_NAME}/${SHORT_SHA} was already created. Since the branch name is deterministic from REPO_NAME + SHORT_SHA, it never changes on retries.
A safe fix is to check whether the branch/PR already exists before pushing, or use --force on the push (though that's heavier). A lightweight approach:
# Check if branch already exists remotely
if git ls-remote --exit-code origin "refs/heads/$BRANCH" >/dev/null 2>&1; then
echo "Branch $BRANCH already exists, checking for open PR..."
PR_URL=$(gh pr list --head "$BRANCH" --base main --json url -q '.[0].url')
else
git checkout -b "$BRANCH"
git commit --allow-empty -m "$COMMIT_MSG"
git push -u origin "$BRANCH"
PR_URL=$(gh pr create \
--title "${TYPE}(${REPO_NAME}): ${CLEAN_MSG}" \
--body "Synced from \`${REPO_NAME}@${SHORT_SHA}\`
Co-authored-by: ${ACTOR} <${ACTOR}@users.noreply.github.com>" \
--head "$BRANCH" \
--base main)
fi
gh pr merge "$PR_URL" --auto --squash
There was a problem hiding this comment.
| # Create PR and enable auto-merge | ||
| PR_URL=$(gh pr create \ | ||
| --title "${TYPE}(${REPO_NAME}): ${CLEAN_MSG}" \ | ||
| --body "Synced from \`${REPO_NAME}@${SHORT_SHA}\`" \ | ||
| --head "$BRANCH" \ | ||
| --base main) | ||
|
|
||
| gh pr merge "$PR_URL" --auto --squash |
There was a problem hiding this comment.
Co-authored-by attribution lost in squash merge
When GitHub performs a squash merge, it generates the merge commit message from the PR title + PR body — not from the individual commit messages. The Co-authored-by trailer embedded in $COMMIT_MSG (on the empty commit) will be silently discarded in the squash commit.
To preserve attribution, include the trailer in the PR --body instead:
| # Create PR and enable auto-merge | |
| PR_URL=$(gh pr create \ | |
| --title "${TYPE}(${REPO_NAME}): ${CLEAN_MSG}" \ | |
| --body "Synced from \`${REPO_NAME}@${SHORT_SHA}\`" \ | |
| --head "$BRANCH" \ | |
| --base main) | |
| gh pr merge "$PR_URL" --auto --squash | |
| # Create PR and enable auto-merge | |
| PR_URL=$(gh pr create \ | |
| --title "${TYPE}(${REPO_NAME}): ${CLEAN_MSG}" \ | |
| --body "Synced from \`${REPO_NAME}@${SHORT_SHA}\` | |
| Co-authored-by: ${ACTOR} <${ACTOR}@users.noreply.github.com>" \ | |
| --head "$BRANCH" \ | |
| --base main) | |
| gh pr merge "$PR_URL" --auto --squash --delete-branch |
There was a problem hiding this comment.
Already fixed in 0bfb64a — Co-authored-by is now included in the PR body, which survives squash merge (GitHub copies PR body into squash commit message).
|
@greptile, please review. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/on-child-update.yml:
- Line 40: The workflow currently uses an org-level secret token reference
`secrets.PARENT_REPO_PAT` for checkout/gh auth which broadens blast radius;
replace this with a repository-scoped credential (preferably a GitHub App
installation token or a fine-grained PAT) that only has the minimal permissions
required (`contents:write` and `pull_requests:write`) and a rotation policy, and
update all occurrences of `token: ${{ secrets.PARENT_REPO_PAT }}` (including the
other instance flagged) to reference the new secret (e.g.,
`secrets.REPO_SCOPED_TOKEN`) and ensure the workflow steps that call
`actions/checkout` and `gh` use that limited-scope token.
- Around line 96-113: The current flow assumes branch and PR don't already exist
and will fail on retries; update the script to detect and reuse existing branch
and PR instead of always running git checkout -b and gh pr create. Specifically:
check whether BRANCH already exists locally or remotely and only run git
checkout -b / git push -u origin "$BRANCH" when it does not (otherwise checkout
the existing branch and update it), and when creating the PR replace the blind
PR_URL=$(gh pr create ...) step with an idempotent sequence that attempts to
create the PR and, on failure due to "already exists", queries for the existing
PR (e.g., via gh pr list/filter by --head "$BRANCH" --base main) and sets PR_URL
to that existing PR result so retries reuse the same PR. Ensure
commit/allow-empty behavior still runs only when needed and that error paths
return the existing branch/PR instead of failing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 933795c9-eca9-485a-b124-83de5082a01b
📒 Files selected for processing (1)
.github/workflows/on-child-update.yml
|
🤖 Agent starting ( |
|
🤖 Agent starting ( |
| # Check for an existing PR and just ensure auto-merge is enabled. | ||
| if git ls-remote --exit-code origin "refs/heads/$BRANCH" >/dev/null 2>&1; then | ||
| echo "Branch $BRANCH already exists — checking for existing PR" | ||
| EXISTING_PR=$(gh pr list --head "$BRANCH" --json url --jq '.[0].url // empty') |
There was a problem hiding this comment.
gh pr list missing --base main filter in idempotency check
Without --base main, this query returns any open PR from $BRANCH regardless of its target. If, for any reason, a PR was manually opened from this branch to a different base, the idempotency path would treat it as the sync PR, incorrectly enable auto-merge on it, and silently exit — leaving the actual sync PR never created.
Since gh pr create below always targets main, the idempotency lookup should match the same base:
| EXISTING_PR=$(gh pr list --head "$BRANCH" --json url --jq '.[0].url // empty') | |
| EXISTING_PR=$(gh pr list --head "$BRANCH" --base main --json url --jq '.[0].url // empty') |
There was a problem hiding this comment.
Fixed in ca2bac0 — added --base main to the gh pr list query so the idempotency check only matches PRs targeting main.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/on-child-update.yml (1)
40-40:⚠️ Potential issue | 🟠 MajorUse a repo-scoped credential instead of org-level PAT.
secrets.PARENT_REPO_PATis still used in both checkout andghauth, which keeps the secret blast radius broader than necessary.🔐 Suggested tightening
- name: Checkout uses: actions/checkout@v4 with: - token: ${{ secrets.PARENT_REPO_PAT }} + token: ${{ secrets.REPO_SCOPED_TOKEN }} @@ env: @@ - GH_TOKEN: ${{ secrets.PARENT_REPO_PAT }} + GH_TOKEN: ${{ secrets.REPO_SCOPED_TOKEN }}Also applies to: 89-89
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/on-child-update.yml at line 40, Replace the org-scoped secret referenced as secrets.PARENT_REPO_PAT with a repo-scoped secret and update all usages; create a repo-scoped secret (e.g., PARENT_REPO_TOKEN) in this repository and change the token: ${{ secrets.PARENT_REPO_PAT }} occurrences used by the checkout step and by the gh auth invocation to token: ${{ secrets.PARENT_REPO_TOKEN }} so both the checkout action and the gh authentication use the new repo-scoped credential instead of the org-level PAT.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/on-child-update.yml:
- Around line 109-111: The workflow currently masks failures from the gh pr
merge command by appending "|| true" to the gh pr merge "$EXISTING_PR" --auto
--squash --delete-branch invocation which causes subsequent exit 0 to always
succeed; remove the "|| true" (or otherwise capture and handle the exit status)
so that a failed gh pr merge causes the job to fail or to branch into explicit
error-handling logic instead of falling through to the unconditional exit 0;
update the block containing the gh pr merge "$EXISTING_PR" --auto --squash
--delete-branch call and the following exit 0 to propagate non-zero exit codes
or add explicit retry/error reporting logic.
---
Duplicate comments:
In @.github/workflows/on-child-update.yml:
- Line 40: Replace the org-scoped secret referenced as secrets.PARENT_REPO_PAT
with a repo-scoped secret and update all usages; create a repo-scoped secret
(e.g., PARENT_REPO_TOKEN) in this repository and change the token: ${{
secrets.PARENT_REPO_PAT }} occurrences used by the checkout step and by the gh
auth invocation to token: ${{ secrets.PARENT_REPO_TOKEN }} so both the checkout
action and the gh authentication use the new repo-scoped credential instead of
the org-level PAT.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1418a37c-ecaa-4091-84ef-1a2c4e743c35
📒 Files selected for processing (1)
.github/workflows/on-child-update.yml
|
🤖 Agent starting ( |
|
Pushed ca2bac0 addressing remaining review comments:
|
|
@greptile, @CodeRabbit. please review. |
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/on-child-update.yml:
- Around line 115-131: The workflow can race when two runs create the same
branch/PR; add a recovery path around the git push and gh pr create steps by
detecting failures and reusing any existing remote branch/PR: if git push -u
origin "$BRANCH" fails, run a recovery sequence that fetches origin, checks out
or resets to origin/"$BRANCH" (preserving the intended commit/empty commit
semantics) and sets the upstream instead of failing; similarly, if gh pr create
fails because a PR already exists, query GitHub for an existing PR for --head
"$BRANCH" (e.g., via gh pr list/gh pr view) and populate PR_URL/PR_BODY/PR_TITLE
from that existing PR instead of erroring. Ensure these recovery checks
reference the same BRANCH/PR_URL/PR_BODY/PR_TITLE variables so the script
becomes idempotent under concurrent runs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c33ef9c0-6136-47e6-b7bf-50741033fb7c
📒 Files selected for processing (1)
.github/workflows/on-child-update.yml
| PR_URL=$(gh pr create \ | ||
| --title "$PR_TITLE" \ | ||
| --body "$PR_BODY" \ | ||
| --head "$BRANCH" \ | ||
| --base main) |
There was a problem hiding this comment.
gh pr create unguarded against concurrent PR creation
There is a race condition between the idempotency check at line 106 and the gh pr create call here. If a concurrent workflow run creates the PR while this run is executing the code between the two calls, gh pr create will exit non-zero with a "PR already exists" error, causing the step to fail even though the desired state (a PR exists) has been achieved.
A safe fix is to use error suppression and fall back to looking up the existing PR:
| PR_URL=$(gh pr create \ | |
| --title "$PR_TITLE" \ | |
| --body "$PR_BODY" \ | |
| --head "$BRANCH" \ | |
| --base main) | |
| PR_URL=$(gh pr create \ | |
| --title "$PR_TITLE" \ | |
| --body "$PR_BODY" \ | |
| --head "$BRANCH" \ | |
| --base main) \ | |
| || PR_URL=$(gh pr list --head "$BRANCH" --base main --json url --jq '.[0].url') | |
| gh pr merge "$PR_URL" --auto --squash --delete-branch | |
| echo "Created PR: $PR_URL" |
This ensures the step succeeds whether the PR was created by this run or a concurrent one.
There was a problem hiding this comment.
Fixed in 47cace0 — gh pr create is now wrapped with failure recovery that queries for an existing PR on the same branch, making the entire workflow idempotent under concurrent execution.
|
🤖 Agent starting ( |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/on-child-update.yml (1)
102-130:⚠️ Potential issue | 🟠 MajorRace window still present in branch push and PR creation path.
Line 104 checks existence, but Line 122 and Line 126 still assume no concurrent run won the race in between. This can fail otherwise-valid retries with deterministic branch names.
Suggested race-safe idempotency patch
if git ls-remote --exit-code origin "refs/heads/$BRANCH" >/dev/null 2>&1; then echo "Branch $BRANCH already exists — checking for existing PR" EXISTING_PR=$(gh pr list --head "$BRANCH" --base main --json url --jq '.[0].url // empty') if [[ -n "$EXISTING_PR" ]]; then echo "PR already exists: $EXISTING_PR" gh pr merge "$EXISTING_PR" --auto --squash --delete-branch exit 0 fi # Branch exists but no PR — fetch it and create PR below git fetch origin "$BRANCH" git checkout "$BRANCH" else git checkout -b "$BRANCH" git commit --allow-empty -m "$PR_TITLE @@ Co-authored-by: ${ACTOR} <${ACTOR}@users.noreply.github.com>" - git push -u origin "$BRANCH" + if ! git push -u origin "$BRANCH"; then + echo "Branch push raced; reusing remote branch." + git fetch origin "$BRANCH" + git checkout -B "$BRANCH" "origin/$BRANCH" + fi fi # Create PR with Co-authored-by in body (survives squash merge) - PR_URL=$(gh pr create \ - --title "$PR_TITLE" \ - --body "$PR_BODY" \ - --head "$BRANCH" \ - --base main) + PR_URL="$(gh pr list --head "$BRANCH" --base main --state open --json url --jq '.[0].url // empty')" + if [[ -z "$PR_URL" ]]; then + if ! PR_URL=$(gh pr create \ + --title "$PR_TITLE" \ + --body "$PR_BODY" \ + --head "$BRANCH" \ + --base main); then + PR_URL="$(gh pr list --head "$BRANCH" --base main --state open --json url --jq '.[0].url // empty')" + [[ -n "$PR_URL" ]] || exit 1 + fi + fi#!/bin/bash set -euo pipefail FILE=".github/workflows/on-child-update.yml" echo "== Relevant section ==" sed -n '96,136p' "$FILE" echo echo "== Guard checks ==" rg -n 'git ls-remote|git push -u origin "\$BRANCH"|gh pr create|gh pr list --head "\$BRANCH" --base main' "$FILE" echo echo "Expected race-safe indicators:" echo "- guarded push fallback: if ! git push -u origin \"\$BRANCH\"; then ..." echo "- pre-create open PR lookup: --state open" echo "- post-create recovery lookup when create fails"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/on-child-update.yml around lines 102 - 130, The current flow has a race between checking branch existence and later git push/gh pr create calls; make the sequence robust by turning the optimistic branch push into a guarded push (attempt git push -u origin "$BRANCH" and if it fails because the remote branch already exists, fetch and checkout the remote branch instead), ensure pre-PR lookup uses gh pr list --head "$BRANCH" --base main --state open to only consider open PRs, and after gh pr create fails (or returns a non-zero) re-query gh pr list for an open PR and enable auto-merge/merge that PR (same behavior as the existing block that calls gh pr merge "$EXISTING_PR" --auto --squash --delete-branch); update logic around BRANCH, git push -u origin "$BRANCH", gh pr create, and gh pr list --head "$BRANCH" to implement these guarded/fallback checks so retrying workflows are idempotent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/on-child-update.yml:
- Around line 102-130: The current flow has a race between checking branch
existence and later git push/gh pr create calls; make the sequence robust by
turning the optimistic branch push into a guarded push (attempt git push -u
origin "$BRANCH" and if it fails because the remote branch already exists, fetch
and checkout the remote branch instead), ensure pre-PR lookup uses gh pr list
--head "$BRANCH" --base main --state open to only consider open PRs, and after
gh pr create fails (or returns a non-zero) re-query gh pr list for an open PR
and enable auto-merge/merge that PR (same behavior as the existing block that
calls gh pr merge "$EXISTING_PR" --auto --squash --delete-branch); update logic
around BRANCH, git push -u origin "$BRANCH", gh pr create, and gh pr list --head
"$BRANCH" to implement these guarded/fallback checks so retrying workflows are
idempotent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 088a9ca6-4aa6-4c83-88a6-a30ea25b7428
📒 Files selected for processing (1)
.github/workflows/on-child-update.yml
Branch protection on meta main requires PRs — no PAT can push directly. Changed the sync workflow to create a branch, open a PR, and enable auto-merge (squash) instead of pushing directly to main. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Retry safety: check if sync branch already exists before creating, reuse existing PR if found - Co-authored-by in PR body so it survives squash merge - --delete-branch on gh pr merge to prevent branch accumulation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…heck - Add --base main to gh pr list so idempotency check only matches PRs targeting main, not PRs from the same branch to other bases - Remove || true from gh pr merge so auto-merge failures are surfaced instead of silently swallowed Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add recovery paths for when two workflow runs execute simultaneously: - git push failure: fetch the remote branch and recover gracefully - gh pr create failure: look up the existing PR instead of failing - Extract shared helpers (find_existing_pr, enable_auto_merge) to reduce duplication across idempotency paths Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The 2>&1 on git push and gh pr create could mix stderr warnings into PR_URL, breaking downstream enable_auto_merge calls. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ca2bac0 to
13e0d56
Compare
Push: Race condition handling + self-review fixesNew commits
Comments addressed
|
Final SummaryAll CI checks green. All review comments addressed. Commits on this branch (5)
Review comments resolved
Self-review fixes
No issues flagged for human review |
Summary
mainrequires PRs — no PAT can bypass thison-child-update.ymlto create async/<repo>/<sha>branch, open a PR, and enable auto-merge (squash) instead of pushing directlyGH013: Repository rule violationsTest plan
workflow_dispatchwith test payload and verify it creates a branch + PR + auto-merges🤖 Generated with Claude Code
Summary by CodeRabbit