Skip to content

build(workflows): retry publish_coverage_pr.yml push on concurrent PR updates - #13698

Draft
Planeshifter wants to merge 1 commit into
developfrom
claude/elegant-feynman-eaz87u
Draft

build(workflows): retry publish_coverage_pr.yml push on concurrent PR updates#13698
Planeshifter wants to merge 1 commit into
developfrom
claude/elegant-feynman-eaz87u

Conversation

@Planeshifter

Copy link
Copy Markdown
Member

None.

Description

What is the purpose of this pull request?

This pull request:

  • fixes a race condition in the publish_coverage_pr workflow's "Commit and push changes" step, where two run_tests_coverage_pr completions 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 of stdlib-js/www-test-code-coverage.
  • merges the "Checkout coverage repository branch", "Copy artifacts to the repository", and "Commit and push changes" steps into a single step, renamed "Checkout, copy artifacts, and push coverage report", wrapped in a retry loop (up to 5 attempts, with 1-5s jitter between attempts).
  • on each attempt: fetches the branch, checks out via git checkout -B (creating it fresh if no upstream branch exists yet), wipes all tracked directories except .github/.git, copies artifacts fresh, appends a coverage.ndjson row 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

Does this pull request have any related issues?

None.

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

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_prpublish_coverage_pr run.

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 for www-test-code-coverage with 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 first error, 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 using git checkout -B in 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's coverage.ndjson row 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

Please ensure the following tasks are completed before submitting this pull request.

AI Assistance

When authoring the changes proposed in this PR, did you use any kind of AI assistance?

  • Yes
  • No

If you answered "yes" above, how did you use AI assistance?

  • Code generation (e.g., when writing an implementation or fixing a bug)
  • Test/benchmark generation
  • Documentation (including examples)
  • Research and understanding

Disclosure

If you answered "yes" to using AI assistance, please provide a short disclosure indicating how you used AI assistance. This helps reviewers determine how much scrutiny to apply when reviewing your contribution. Example disclosures: "This PR was written primarily by Claude Code." or "I consulted ChatGPT to understand the codebase, but the proposed changes were fully authored manually by myself.".

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 -b vs -B) was fixed and reverified before submission.


@stdlib-js/reviewers


Generated by Claude Code

…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
@stdlib-bot stdlib-bot added the Good First PR A pull request resolving a Good First Issue. label Jul 27, 2026
@stdlib-bot

Copy link
Copy Markdown
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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Good First PR A pull request resolving a Good First Issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants