build(workflows): retry publish_coverage_pr.yml push on concurrent PR updates - #13698
Draft
Planeshifter wants to merge 1 commit into
Draft
build(workflows): retry publish_coverage_pr.yml push on concurrent PR updates#13698Planeshifter wants to merge 1 commit into
publish_coverage_pr.yml push on concurrent PR updates#13698Planeshifter wants to merge 1 commit into
Conversation
…PR updates The job `Publish PR coverage results` in workflow `publish_coverage_pr` failed 4 times on develop within a 45-minute window today, all for the same PR (#12370), with: ! [rejected] pr-12370 -> pr-12370 (fetch first) Root cause: multiple `publish_coverage_pr` runs for the same PR (e.g., successive pushes to the PR each trigger their own coverage run) race to push a commit to the same `pr-<number>` branch of the `www-test-code-coverage` repo. The step did a single fetch, checkout, copy, commit, and push with no retry, so whenever two runs for the same PR land close together, the second push is a plain non-fast- forward rejection and the job fails outright every time. This commit merges the checkout, copy-artifacts, and commit-and-push steps into one step wrapped in a retry loop (up to 5 attempts): on a rejected push, it re-fetches the branch's current tip and redoes the checkout, wipe, and artifact copy against it before retrying, rather than rebasing or merging a stale commit onto it (which risks content conflicts, since the copied coverage report files are wholesale replaced on each run). Ref: https://github.com/stdlib-js/stdlib/actions/runs/30238689016
Contributor
|
Hello! 👋 We've noticed that you've been opening a number of PRs addressing good first issues. Thank you for your interest and enthusiasm! Now that you've made a few contributions, we suggest no longer working on good first issues. Instead, we encourage you to prioritize cleaning up any PRs which have yet to be merged and then proceed to work on more involved tasks. Not only does this ensure that other new contributors can work on things and get ramped up on all things stdlib, it also ensures that you can spend your time on more challenging problems. 🚀 For ideas for future PRs, feel free to search the codebase for TODOs and FIXMEs and be sure to check out other open issues on the issue tracker. Cheers! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
None.
Description
This pull request:
publish_coverage_prworkflow's "Commit and push changes" step, where tworun_tests_coverage_prcompletions for the same PR trigger overlapping coverage-publish runs, and the second run's push is rejected because its local commit is based on a stale fetch ofstdlib-js/www-test-code-coverage.git checkout -B(creating it fresh if no upstream branch exists yet), wipes all tracked directories except.github/.git, copies artifacts fresh, appends acoverage.ndjsonrow with a per-attempt timestamp, commits, and pushes. On rejection, re-fetches and redoes the full cycle against the new tip, rather than rebasing or merging the stale commit.Related Issues
None.
Questions
No.
Other
Observed failure: 4 runs failed on 2026-07-27, all for PR #12370, within a 45-minute window, all at "Commit and push changes" with
! [rejected] pr-12370 -> pr-12370 (fetch first). Example: https://github.com/stdlib-js/stdlib/actions/runs/30238689016 (also 30238765390, 30238891680, 30240767267).This is deterministic under concurrency, not flaky. It recurs on any PR that receives rapid successive pushes, since each push triggers its own
run_tests_coverage_pr→publish_coverage_prrun.Rebase/merge onto the new tip was considered and rejected: the copied coverage HTML files are wholesale-replaced each run, not line-diffable, so a rebase would very likely hit real conflicts. Redoing the checkout/copy fresh against the new tip avoids that.
Validation: YAML parsed with PyYAML; shell checked with
bash -n; local simulation using a bare repo standing in forwww-test-code-coveragewith two independent checkouts based on the same prior commit — one completes a full publish, the second is left on its now-stale base and reproduces the exact! [rejected] ... fetch firsterror, then the retry logic re-fetches, redoes checkout/copy/commit, and re-pushes cleanly.Three review passes were run against the diff. Reviewers A and B both flagged that the retry loop's no-upstream-branch branch used
git checkout -b, which fails with "branch already exists" on any iteration after the first, since iteration 1 already created the local branch. Fixed by usinggit checkout -Bin both branches of the fetch/checkout logic, which is idempotent across retries; verified with a standalone repro. Reviewer C approved with nitpicks, both addressed: the step rename above, and moving the per-attempt timestamp computation inside the retry loop so each attempt'scoverage.ndjsonrow reflects its own attempt time rather than a single time computed outside the loop.Not implemented, flagged for follow-up: a
concurrency:group keyed on PR number would serialize these runs and avoid the race entirely, as an alternative/complementary fix; kept out of this PR to keep the diff scoped to the observed failure. Also note that under the new design, if two runs for the same PR both eventually push, the published report reflects whichever run's retry loop finishes last chronologically, not necessarily the PR's most recent commit — this last-write-wins property already existed before this fix and is now just reachable by both racers instead of only the first.Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
This PR was written primarily by Claude Code, based on failed run logs for PR #12370. The diff was validated with YAML/shell syntax checks and a local git simulation reproducing the race, and reviewed through three independent automated passes; one real bug found during review (
git checkout -bvs-B) was fixed and reverified before submission.@stdlib-js/reviewers
Generated by Claude Code