Skip to content

fix(ci): fork-sync 'Sync forks' step works under bash -e + existing-PR jq repair - #1721

Merged
POWERFULMOVES merged 1 commit into
mainfrom
fix/fork-sync-errexit
Jun 5, 2026
Merged

POWERFULMOVES merged 1 commit into
mainfrom
fix/fork-sync-errexit

Conversation

@POWERFULMOVES

@POWERFULMOVES POWERFULMOVES commented Jun 5, 2026

Copy link
Copy Markdown
Owner

Problem

The live fork-sync run (dry_run=false, #27033914982) failed on the first fork (SYNC PMOVES-ClawZ then exit 1) — no sync PRs created on any fork.

Two bugs, both only reachable when the sync step actually runs (dry-run exercises only the audit step, so #1718 didn't surface them):

  1. errexit incompatibility. GitHub runs run: steps under /usr/bin/bash -e. The loop is best-effort and relies on cmd > /dev/null 2>&1; if [ $? -ne 0 ] and || { ...; continue; } guards. Under errexit, a failing standalone gh api (e.g. branch-creation) aborts the whole step before its $? check — which is exactly where it died.
  2. Malformed jq in the existing-sync-PR check: [.[] | select(...) | .number is missing the closing ], so it errored silently and never detected existing sync PRs → duplicate-branch/PR risk on re-runs.

Fix

  1. set +e at the top of the Sync-forks loop (matches the script's own error-handling design).
  2. Close the jq array + | .[0] // empty.

Verification

  • YAML parses (job sync, 5 steps intact).
  • jq filter validated against a fixture (no syntax error; correct selection).
  • No logic change beyond enabling the existing guards to work.

Disposition

Unblocks the fork-sync remediation (CVE work). After merge, re-run dry_run=false. Note: heavily-drifted forks whose upstream merge conflicts will still be skipped (branch auto-deleted) — expected; those need local conflict resolution.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Improved internal fork synchronization workflow reliability and accuracy.

…isting-PR jq

Two bugs that only surface when dry_run=false (the sync step runs):

1) errexit: GitHub runs run: steps under 'bash -e'. The loop is best-effort
   and uses 'cmd > /dev/null 2>&1; if [ $? -ne 0 ]' and '|| { ...; continue; }'
   guards, which errexit defeats — a failing standalone gh api command aborts
   the whole step before its check. The live run died on the first fork
   (PMOVES-ClawZ) at branch creation. Fix: 'set +e' at the top of the loop.

2) malformed jq in the existing-sync-PR check ('[.[] | ... | .number' missing
   the closing ']') errored silently, so existing sync PRs were never detected
   -> duplicate-branch/PR risk on re-runs. Fix: close the array + '| .[0] // empty'.

Separate from #1718 (YAML block-scalar parse fix); that made workflow_dispatch/
dry-run work, this makes the actual sync run. YAML verified; jq filter validated.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Looking for one thing? Review this PR in Change Stack to search files, summaries, diffs, and code without losing your place.

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c6bf7001-da18-4deb-9c12-7bb62f642054

📥 Commits

Reviewing files that changed from the base of the PR and between 915d388 and ff49d51.

📒 Files selected for processing (1)
  • .github/workflows/fork-sync.yml

📝 Walkthrough

Walkthrough

This PR improves the fork sync GitHub Actions workflow by making it more resilient to iteration failures and fixing pull request detection logic. It disables bash error exit behavior in the fork-sync loop via set +e and corrects a jq query to properly identify existing sync pull requests.

Changes

Fork Sync Workflow Fixes

Layer / File(s) Summary
Loop resilience and PR detection fixes
.github/workflows/fork-sync.yml
Adds set +e with comments before the fork-sync loop to prevent step termination on individual iteration failures. Fixes the jq expression in the PR detection query to correctly filter for "sync/*" branches and extract the first matching PR number, enabling proper PR reuse when sync PRs already exist.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A workflow grows stronger with grace,
No longer does one failed loop spoil the race,
And when sync pulls are found with precision,
The jq query now makes the right decision,
Fork after fork, in harmonious sync! 🔄

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description provides clear problem statement, fixes, and verification, but lacks required sections: Testing with commands/output, Required Checks checklist, and proper template structure. Add Testing section with verification commands, complete Required Checks checklist (CHIT Contract Check, documentation updates), and follow the provided template structure.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes both main fixes (bash -e compatibility and jq repair) in the fork-sync workflow step.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/fork-sync-errexit

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@POWERFULMOVES
POWERFULMOVES merged commit 635cbd7 into main Jun 5, 2026
18 checks passed
@POWERFULMOVES
POWERFULMOVES deleted the fix/fork-sync-errexit branch June 5, 2026 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant