ci: migrate pull_request_target workflows to workflow_run - #26683

On push, the script looks up PRs only for context.sha, so every other PR merged by the same push never gets the release version label.

Fixed in 4c3c2e9. The script now looks up every commit in context.payload.commits (or context.sha when the payload has none) and labels each merged PR once. In the last 40 merges every merge commit was the head of its own push, so this is robustness and not a current gap. test_release_version_labeler.cjs covers a push with two commits that share a PR.

The script now throws when a push to main has no merged PR, which turns ordinary direct pushes into failed workflow runs on main.

Agreed. After the three attempts it now logs No merged pull request found for <sha> and the run succeeds. All of the last 100 commits on main had a merged PR, but a failed run on main is too noisy for a cosmetic label. A test covers it.

The signal workflow runs from the PR's own version of the file, so a PR that renames, edits or deletes it turns off labeling and the Rust (fork) check for itself without any error.

True, a PR runs its own copy of the file. It can only skip the labels and the immediate Rust (fork) check for itself, and it gains nothing else. The signal run has permissions: {}, and the privileged workflows only read the head repository, branch and SHA of the run, which the PR author controls anyway. The scheduled update in fork_ci_status.yml still creates a missing Rust (fork) check, and the merge queue stays the gate. I documented this in the header of the signal workflow in 4c3c2e9.

The script dereferences run.head_repository.owner.login without a null check; head_repository is null when the fork was deleted or is unavailable.

Fixed in 4c3c2e9. The shared lookup returns no pull requests when head_repository is null, so neither workflow dereferences it. Tests cover the lookup and the fork_ci_status notification.

The PR lookup by head branch plus SHA is written twice: inline in labeler.yml and in pullRequestOf in fork_ci_status.yml. Only the second copy has tests.

Fixed in 4c3c2e9. Both workflows now call ci/scripts/pull_requests_of_run.js, after a sparse checkout of ci/scripts from the default branch (never the pull request). The tests in test_fork_ci_status.cjs cover it directly, including the no-base case that the labeler uses.

The design relies on GitHub sending workflow_run: requested for a run that is waiting for first-time-contributor approval. That is not documented; GitHub may hold the event until a maintainer approves the run.

You are right that it is not documented. It is observed behavior. In apache/spark-kubernetes-operator two signal runs from an outside fork stayed completed/action_required (never approved), and each still started its workflow_run consumer 2 to 3 seconds later. I noted this in the header of the signal workflow in 4c3c2e9. If GitHub changes it, the scheduled update in fork_ci_status.yml still creates a missing Rust (fork) check, but labeling would need a follow-up.

The concurrency group still uses github.head_ref || github.sha. With a push trigger head_ref is always empty and every SHA is unique, so the group and cancel-in-progress no longer do anything.

Fixed in 4c3c2e9. I removed the concurrency block. Each push needs its own run, so there is nothing to cancel or serialize.

The rewritten header comment is not re-wrapped; the line "requests plus a rotating slice ... The merge" runs far past the wrap width of the lines around it.

Fixed in 4c3c2e9. Re-wrapped, and I dropped the parenthetical that the header of the signal workflow already explains.

A misplaced closing brace leaves five new tests, plus the findPullRequests/lookup helpers, inside the body of the test "notification skips a commit that is no longer a pull request head"; that test's own assertion ends up at line 266, after them.

Failure scenario: The inner test() calls (deleted fork, three lookup tests) become unawaited subtests of the stale-head test instead of top-level tests. If the parent finishes or fails first, node:test cancels them or reports them as failures under the wrong name. Whether they run at all depends on the Node version's subtest handling. The stale-head check itself only runs after these extra subtests have been registered.

Good catch. The }); I matched was the end of the execute(...) call, so the new tests ran as subtests of the stale-head test. Fixed in f64546b. They are top-level tests now (the runner output has no indented test lines), and the stale-head test asserts on its own again.

The new push: main trigger has no if: github.repository == 'apache/datafusion' guard, so it now runs in every fork that has Actions enabled. Forks were affected before only when PRs were closed inside the fork itself.

Failure scenario: The contributor guide asks fork owners to enable workflows. A contributor who syncs their fork's main then triggers this job on each sync. It creates a v57.0.0 label in the fork, waits through the retries, and calls addLabels on the fork for any PR numbers the lookup returns. That spends the fork's minutes and can label the wrong fork issue or fail with a 404. merge_queue_cleanup.yml and draft_inactive_prs.yml already use this guard.

Fixed in f64546b. The job now has if: github.repository == 'apache/datafusion', like merge_queue_cleanup.yml and draft_inactive_prs.yml.

The signal uses pull_request, which GitHub does not run while a PR has merge conflicts (there is no merge ref). The old pull_request_target triggers ran either way.

Failure scenario: A contributor pushes to a PR that conflicts with main. No signal run starts, so labeler.yml never re-syncs labels for the new files and fork_ci_status.yml never creates the immediate Rust (fork) check. Labels stay stale until the conflict is fixed and another push lands, and the header comment does not mention this gap.

Right, this is a known gap. I noted it in the header of the signal workflow in f64546b. A PR with conflicts gets its labels after the push that resolves them. The scheduled update in fork_ci_status.yml creates the Rust (fork) check regardless, because it selects PRs by their head and base and not by mergeability. The Rust workflow has the same limit, since it also runs on pull_request.

When workflow_run.head_repository is null (deleted fork), the concurrency expression falls back to the update group used by the scheduled sweep, while cancel-in-progress is still true because the event is workflow_run.

A signal run, or a rerun, from a PR whose fork was deleted triggers this workflow. It joins group Fork CI status-update and cancels the scheduled or manually dispatched sweep that is running, even though its own notify() finds nothing to do.

Confirmed and fixed in f64546b. The per-branch group is now chosen by github.event_name == 'workflow_run', so a signal run from a deleted fork gets its own group and no longer cancels the sweep. Two tests cover the groups, and the one for the deleted fork fails with the old expression.

Each commit in the push is looked up one after another, and every commit without a merged PR costs two 5-second waits.
A direct push to main with N commits that have no PR blocks the job for about 10s × N (20 commits is about 200s of runner time) before it gives up. Retrying only the head commit, or looking up commits in parallel, would avoid most of that.

Fixed in f64546b. The commits are looked up in parallel with Promise.all, each with its own retries, so a push whose commits have no PR waits about 10 seconds in total and not 10 seconds per commit. A test checks that the lookups overlap and that only the lagging commit is retried.

The "No merged pull request found" message prints only context.sha, although the script searched every commit in payload.commits.

A multi-commit push where none of the commits maps to a merged PR logs only the head SHA, so whoever debugs a missing label can't tell which commits were checked.

Fixed in f64546b. The message lists every commit that was searched. The old test had locked in the head SHA, so it now checks a push with two commits.

All commits of the push are looked up at the same time with no limit, and one failed lookup makes Promise.all reject, so no pull request in the push gets the label.

A push to main can hold many commits, for example a branch merged with a merge commit or a large merge queue group. Each commit starts its own listPullRequestsAssociatedWithCommit call at once. A burst of up to 2048 calls can hit GitHub's secondary rate limit (about 100 requests at the same time). Then one call fails with 403/429, or one call fails with a normal 5xx. Promise.all rejects, the job fails before addLabels runs, and no pull request in that push gets the label. The old pull_request_target flow labeled each pull request in its own run, so one error never cost other pull requests their label. Fix: look the commits up in small batches, and catch the error for each commit (for example with Promise.allSettled) so that the lookups that worked still lead to labels.

Sound, and fixed in aa555be. The commits are looked up ten at a time with Promise.allSettled, so a failed lookup no longer drops the labels of the other pull requests. The script labels what it found and then rethrows the first error, so the run is red and a rerun can finish the job. A failed addLabels is isolated the same way. The docs allow up to 2048 commits in the push payload, so the bound is not only theoretical, although each of the last 40 merges to main was a single commit. Tests: 25 commits never have more than 10 lookups in flight, and a failed lookup or a failed label still labels the others.

The concurrency group is the head repository and branch with cancel-in-progress, but the lookup only matches the run's own head SHA. If an older run is queued after a newer one, it cancels the newer run and then labels nothing.

Failure scenario: A contributor pushes twice quickly to a fork branch. If the workflow_run requested event for push 1 is queued after the event for push 2, the push 1 run cancels the push 2 run. pull_requests_of_run.js then drops the pull request, because pull.head.sha no longer equals run.head_sha. Neither run labels the new head, and labels stay stale until the next push. fork_ci_status.yml has the same group shape, but its scheduled sweep fixes the check later. The labeler has no such recovery. Fix: let the run that lost the race use the current head (drop the SHA filter for the labeler), or put the SHA in the group and turn off cancel-in-progress.

Sound, and fixed in aa555be with your first suggestion. pull_requests_of_run.js no longer compares head.sha with the run's commit, so the run that survives a cancellation labels the current head. That is safe because actions/labeler computes labels from the files of the PR as it is now, so a delayed run for an older commit gives the same result as the newest one. I kept the group and cancel-in-progress as they are, since putting the SHA in the group would only multiply runs. A test covers a PR whose head moved after the run started.

The job runs a runner, a checkout and a pulls.list call for every signal run, even when the payload already shows that the head repository is not a fork or was deleted.

Failure scenario: Every push to a pull request from an apache/datafusion branch starts the signal. The bridge job then gets a runner, does the sparse checkout and calls the API, and only after that notify() says 'Not a fork pull request to main'. The same happens for a deleted fork (head_repository is null). Fix: add github.event.workflow_run.head_repository.fork == true to the job-level if for the workflow_run event. That skips these runs before any runner starts, and the release-branch check stays in notify().

Fixed in aa555be. For workflow_run events the job if now also requires github.event.workflow_run.head_repository.fork == true. Branches of this repository (Dependabot among them) and deleted forks no longer start a runner, and notify() still checks the base. Tests evaluate the expression for a fork, a branch of this repository, a deleted fork, another triggering event and the scheduled and manual runs.

Prevent stale signals from cancelling the current label update. For rapid successive pushes, GitHub does not guarantee execution order within a concurrency group. If the older signal's consumer arrives last, labeler.yml cancels the newer consumer because both share the branch-scoped group. This filter then discards the older SHA, so neither run updates the labels, and there is no scheduled label reconciliation. Reconcile the current PR head or ensure obsolete signals cannot cancel its update.

Same fix as in the labeler thread. The lookup now returns the open pull requests of the branch as they are, so whichever signal run survives labels the current head. fork_ci_status.yml still compares the commit in notify(). A late run that created the check for the current head could add a second Rust (fork) check to a commit that already has one, and its scheduled sweep covers the cancelled case.

Very unlikely but for a push containing more than 100 commits, this launches enough simultaneous requests to exceed GitHub's secondary limit of 100 concurrent API requests. A rejected lookup rejects the entire Promise.all before any labels are applied, including for successfully resolved PRs. Use bounded parallelism rather than launching one request per commit; a 101-commit stub reproduces 101 concurrent requests and zero labels after a rate-limit rejection.

Covered by the same change in aa555be. Lookups run ten at a time, and a rejected lookup no longer drops the labels of the others. A 25-commit stub never has more than 10 requests in flight.

When a second pull request from the same fork branch targets a release branch, its signal run creates a second Rust (fork) check on the pull request to main

Confirmed. The old pull_request_target run of the release-branch PR returned early, but the new lookup goes through the branch, so it finds the main PR at the same commit and creates a second check. Fixed in 6ce320a: notify() first lists the Rust (fork) checks of the commit, and when one exists it updates it through the existing syncCheck() instead of creating another. That also covers a reopened PR, which got a second check in the old flow too. A test covers a release-branch PR next to the main PR. It fails both without the lookup and when the existing check is skipped instead of updated.

The signal uses pull_request, and GitHub skips that event when the head commit message contains [skip ci] (or [ci skip], [no ci] and similar). The old pull_request_target triggers ran anyway, and the header comment does not list this gap.**

Confirmed. GitHub documents that the markers skip pull_request workflows when they are in the head commit, and that they do not stop pull_request_target. I listed it next to merge conflicts in the header in 6ce320a. The fork's own rust.yml push run is skipped by the same marker, so the scheduled update creates a Rust (fork) check that reports no fork run for the commit, which matches what happened. The labels follow with the next push that has no marker. I also noted that a re-run of the signal does not send requested, which GitHub documents, and how to trigger it again.

Only the first error is thrown. The other lookup and label errors are counted but their messages are never logged.

Fixed in 6ce320a. Each failed lookup is logged with its commit, and each failed label with its pull request, when it happens. A summary line follows, and the first error is still thrown so the run is red. A test checks the log lines for a run with one failed lookup and one failed label.

Recover release labels for merges that skip push workflows. If a squash-merge commit contains [skip ci], [ci skip], or another supported skip marker, GitHub suppresses this push workflow. Unlike the previous pull_request_target: closed trigger, the merged PR then never receives its release-version label. Subsequent pushes only inspect their own commits, so they cannot recover the missing label. Add a reconciliation path that does not depend on the merge's push workflow running.

Sound, and it applies more widely than it looks. The merge queue squashes with the PR body as the commit message, so a skip marker anywhere in a PR description reaches main. None of the last 3000 commits on main has one, but this PR's own description had two until today, which I removed. Fixed in c24e43a: each push run now labels every PR merged to main in the last 7 days that has no release label, with the version that release_version_labeler.js names at the PR's merge commit. So a later push labels a merge whose own push skipped the workflow, without depending on that push. This replaces the lookup of the pushed commits. A test checks that dev/update_datafusion_versions.py still rewrites only the version constant, since the script now also reads that constant back at older commits.

notify() checks for an existing Rust (fork) check before createCheck() starts its wait for the fork run, which can last up to 60 s (12 attempts x 5 s). A check created during that wait is not seen, so the new dedup fails exactly when it is needed.

Sound. The overlap that matters is with the scheduled update, which has its own concurrency group. Two signals for the same branch share one, so only the newer runs. Fixed in cb97e0b: createCheck() looks for the commit's check only after the wait, and updates it if one exists. The scheduled update's recovery path calls createCheck() too, so it is covered from that side as well. A test creates the check while the run looks for the fork run, and it fails with the earlier order.

The lookup gives up about 10 s after the push (3 attempts, 5 s apart) and then only logs, so the run stays green. The old pull_request_target: closed trigger fired only after the PR was marked merged and always carried the PR number.

Fixed by the same change in c24e43a. The run no longer looks up the pushed commits, so there is nothing to wait for. It lists the recently merged PRs, and for the last 40 merges merged_at was always set before the push run was created. A PR that a run still misses, or a run that fails, is labeled by the next push.