Skip to content

ci: migrate pull_request_target workflows to workflow_run - #26683

Draft
comphead wants to merge 1 commit into
apache:mainfrom
comphead:ci-pr-target-workflow-run
Draft

comphead wants to merge 1 commit into
apache:mainfrom
comphead:ci-pr-target-workflow-run

Conversation

@comphead

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

Three workflows use pull_request_target: labeler.yml, fork_ci_status.yml and release_version_labeler.yaml. ASF infra disables the trigger from November 2026. GitHub also adds a default policy that blocks it on public repositories, enforced from 2026-12-02. Without this change, PRs stop getting labels, the Rust (fork) check and the release version label.

What changes are included in this PR?

  • Add pull_request_signal.yml, an unprivileged pull_request workflow with permissions: {} and one echo step.
  • labeler.yml and fork_ci_status.yml run on workflow_run of that workflow with types: [requested]. GitHub runs them from the default branch, so they never run PR code. requested is sent at once, even for a first-time contributor whose run waits for approval.
  • workflow_run.pull_requests is empty for fork PRs. Both workflows find the PR from head_repository, head_branch and head_sha with pulls.list, and skip a commit that is no longer the PR head. labeler.yml passes the number to actions/labeler as pr-number.
  • Concurrency groups use the workflow_run head repository and branch, since github.head_ref and the PR number are not set.
  • release_version_labeler.yaml runs on push to main and labels the merged PR found with listPullRequestsAssociatedWithCommit.
  • test_fork_ci_status.cjs now drives the notification with a workflow_run payload and covers the stale commit case.

What is the testing strategy for this PR?

  • node --test ci/scripts/test_fork_ci_status.cjs passes (25 tests).
  • actionlint reports nothing on the four workflows.
  • The workflow_run flow only takes effect after the merge to main, because GitHub runs these workflows from the default branch. It is not tested end to end. Things to check after the merge: that actions/labeler reads labeler-config.yml from the default branch, that the push-time PR lookup finds merge queue commits without a race on merged_at, and that a PR from a fork gets labels and the Rust (fork) check.

Are there any user-facing changes?

No. A PR with merge conflicts does not start pull_request workflows, so it is labeled and gets the Rust (fork) check once the conflicts are resolved. The edited label trigger was dropped, since the old workflow skipped that action already.

pull_request_target is disabled by ASF infra policy and GitHub is adding a
default policy that blocks it. Add pull_request_signal.yml and trigger
labeler.yml and fork_ci_status.yml from its workflow_run. Label merged pull
requests from the push to main in release_version_labeler.yaml.
@github-actions github-actions Bot added the development-process Related to development process of DataFusion label Oct 23, 2026
@alamb

alamb commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Thank you for working on this

@comphead

comphead commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor Author

Thank you for working on this

All the credits to Apache Spark community, lets see if it works for us

@potiuk

potiuk commented Oct 23, 2026

Copy link
Copy Markdown
Member

Nice! Good job :) . World is better with every single pull_request_target removed!

@potiuk

potiuk commented Oct 23, 2026

Copy link
Copy Markdown
Member

Glad my examples were helpful and useful to replicate :D

state: 'open',
head: `${run.head_repository.owner.login}:${run.head_branch}`,
});
const pr = data.find((pr) => pr.head.sha === run.head_sha);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A head branch and SHA do not uniquely identify a PR: the same branch can target main and a release branch. pulls.list defaults to newest first, so find() selects the newer PR for every signal and the older PR never receives labels.
The equivalent lookup in fork_ci_status.yml should filter to main before selecting, otherwise it can select the release PR and skip the main notification.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, confirmed. A fork can have open PRs to main and to a release branch from the same branch, and both match head + head_sha. pulls.list returns newest first, so find() always picks the newer PR.

There is one more piece: both PRs share the new concurrency group (head repo + branch) with cancel-in-progress: true, so the two signal runs from one push cancel each other. Fixing find() alone is not enough. The run that survives has to handle every matching PR.

Suggested changes:

labeler.yml: label every matching PR. actions/labeler v7 accepts several numbers in pr-number, one per line:

-            const pr = data.find((pr) => pr.head.sha === run.head_sha);
-            if (pr) core.setOutput('number', String(pr.number));
-            else core.info(`No open pull request for ${run.head_sha}`);
+            const numbers = data
+              .filter((pr) => pr.head.sha === run.head_sha)
+              .map((pr) => String(pr.number));
+            if (numbers.length) core.setOutput('numbers', numbers.join('\n'));
+            else core.info(`No open pull request for ${run.head_sha}`);
       - name: Assign GitHub labels
-        if: steps.pr.outputs.number != ''
+        if: steps.pr.outputs.numbers != ''
         uses: actions/labeler@bf12e9b00b37c5c0ca2b87b79b2daf7891dbda13  # v7.0.0
         with:
           repo-token: ${{ secrets.GITHUB_TOKEN }}
-          pr-number: ${{ steps.pr.outputs.number }}
+          pr-number: ${{ steps.pr.outputs.numbers }}

fork_ci_status.yml: only PRs to main get the mirrored check, so filter on the base in the lookup:

               const { data } = await github.rest.pulls.list({
                 ...upstream,
                 state: 'open',
                 head: `${run.head_repository.owner.login}:${run.head_branch}`,
+                base: 'main',
               });

With that, whichever signal run survives the cancellation finds the main PR.

test_fork_ci_status.cjs: add a case with two open PRs from the same head branch, one to main and one to a release branch, where the release PR is newer. The Rust (fork) check should still be created for the main PR.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to draft PR to your PR if you prefer.

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

Labels

development-process Related to development process of DataFusion

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants