Skip to content

Auto-approve cheap CI runs of external contributors' PRs - #75455

Merged
potiuk merged 5 commits into
mainfrom
auto-approve-workflow-runs
Oct 25, 2026
Merged

Auto-approve cheap CI runs of external contributors' PRs#75455
potiuk merged 5 commits into
mainfrom
auto-approve-workflow-runs

Conversation

@potiuk

@potiuk potiuk commented Oct 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

GitHub holds the CI runs of fork PRs until a maintainer approves them. With self-hosted runners (AIP-118, #74889) that gate becomes part of the trust model, but approving every push by hand doesn't scale and makes contributors wait. This adds an Auto approve workflow runs workflow that approves the runs that are cheap and can't change how CI works, and leaves the rest to maintainers with a need workflow approval label.

PRs whose CI runs in the contributor's fork (AIP-120) are covered too. Their runs in apache/airflow only execute the fork CI gate, so they are approved whatever their size, as long as they leave CI alone.

related: #74889

What gets approved

Every 30 minutes, using only main's code (the PR's code is never checked out or run), breeze ci approve-workflow-runs approves the waiting runs of a commit started in the last 24 hours when:

  • the PR targets main
  • it changes nothing under .github/, dev/breeze/ or scripts/ci/
  • it changes no dependency manifest or lock file and adds no new provider
  • selective checks need neither the full test set, all versions, a canary build nor a dependency upgrade
  • the selected jobs are estimated at no more than 310 job-minutes on public runners (about a quarter of a full run)
  • it changes at most 100 lines of source code (tests, docs, translations and lock files are not counted)

A run selects its tests from the labels the PR had when it started, while the decision uses the current labels. So a run started before a label that changes which tests run was added or removed (full tests needed, all versions, default versions only, latest versions only, canary, upgrade to newer dependencies, area:e2e-tests, area:kubernetes-tests) waits for a maintainer; the next push is decided afresh.

The job summary lists each waiting PR with its decision, the expected job groups and the estimated job-minutes and wall-clock time. --report-only (or the report-only dispatch input) shows the decisions without approving or labelling.

PRs whose CI runs in the fork (AIP-120)

For a PR to main that the fork CI gate would leave to the contributor's fork (decided with fork_ci_gate.decide, the configuration of main and the same inputs the gate job uses), the rules above don't apply, because its Tests (AMD) run only executes the gate. Instead:

  • Its runs are approved when it changes none of the files the fork CI reconciler counts as defining CI: .github/, dev/breeze/, scripts/ci/, scripts/docker/, scripts/in_container/, Dockerfile*, .pre-commit-config.yaml. Size, estimated cost and dependency changes don't count. The other pull_request workflows (CodeQL for the changed languages, the newsfragment check, dependency review) run on GitHub-hosted runners.
  • If it changes such a file, its runs wait for a maintainer, it gets need workflow approval, and the workflow posts one comment ("Your CI change needs a maintainer") listing the changed CI files. Contributor-chosen file names are sanitised so they can't break out of the code spans.
  • A run started before use fork ci or use project ci was added or removed waits for a maintainer: the gate falls back to the labels of the run's event when it can't read the current ones.

Using the reconciler's own list keeps the two decisions consistent: a gated PR whose runs are approved automatically is also one whose green fork run is trusted, and one that changes CI is left waiting and later labelled needs project ci. The design is in the new "Approving gated runs" section of dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md.

CI-minutes estimate

ci_minutes_estimate.py turns selective-check outputs into job groups. The numbers are medians from 310 recent Tests (AMD) PR runs (224 of them with a partial provider set). Provider tests are weighted by the size of each provider's test suite. Jobs that need no CI image (Go, TypeScript and Java SDK, TypeScript SDK docs, Airflow CTL) are counted also when only basic checks run, provider tests only when unit tests run, and provider mypy as a group of its own. dev/calibrate_ci_minutes_estimates.py measures them again, counting only runs on public runners with the default versions, as the estimate models them; .github/AGENTS.md asks agents to refresh them after workflow changes. RunsOn estimates come later, once there is data.

Expected effect

Replaying the rules on the 349 external fork PRs from 2026-10-09..23: 147 (42%) would be approved automatically, about 10 a day. That's about 20% of their estimated job-minutes (27.7k of 141k). The rest wait for a maintainer:

Reason PRs
over 310 job-minutes 122
CI files 21
full tests 21
dependencies 16
over 100 source lines 16
not targeting main 6

This replay predates the fork CI rule and the estimate fixes. The fork CI gate is still off and the contributor pilot has no participants yet, so today only PRs labelled use fork ci take the fork CI path.

Verified

  • Approving a fork PR's action_required run with a workflow's GITHUB_TOKEN (actions: write) was tested in a scratch repository. It returned 201, the run started within seconds, and a downstream workflow_run workflow still triggered.
  • The dev/breeze test suite passes (1959 passed, 7 skipped), including the new tests in test_workflow_run_approval.py, test_ci_minutes_estimate.py, test_calibrate_ci_minutes_estimates.py and test_fork_ci_messages.py.
  • prek pre-commit hooks pass (the Docker-based lychee link check was not run), and so does mypy-dev.
  • breeze setup regenerate-command-images --check-only and breeze setup check-all-params-in-groups pass.

For it to cover returning contributors, the repository's fork-PR approval setting needs to become "Require approval for all external contributors" (ASF Infra). Today only first-time contributors wait for approval.

Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 5)

Generated-by: Claude Code (Opus 5) following the guidelines

🤖 Generated with Claude Code

@potiuk
potiuk requested a review from shahar1 October 23, 2026 11:00
shahar1
shahar1 approved these changes Oct 23, 2026
View reviewed changes

@shahar1 shahar1 left a comment

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.

The trust boundary holds up: the workflow runs only main's code, reads the PR as git objects, ties the decision to the head commit through the merge commit's second parent, and fails closed on every selective-checks error. Runner routing (the build-info job's runs-on, scripts/ci/install_breeze.sh, the uv version from the root uv.lock, and the routing logic in selective checks once AIP-118 lands) lives under the excluded prefixes or in dependency files, so an auto-approved fork run cannot move itself to self-hosted runners. The two findings below are small.

One design question, not a blocker: the 310 job-minute cap estimates what the selected jobs cost for an honest change. Test code is the contributor's, so each selected job can still run to its timeout-minutes, and every new push is approved again on the next tick with no per-PR or per-author limit. Is that acceptable on the shared public runners, or is a per-PR cap worth adding (for example at most N auto-approved commits per PR per 24 hours)? Relatedly, until ASF Infra switches the setting to "all external contributors", only first-time contributors are held, so merging this first means a new account's cheap PR runs with no human look. Is that the intended order?

The red Build documentation job is not caused by this PR: install_mermaid_cli.sh failed on 502 Bad Gateway from dhi.io while fetching libxcb-present0. A re-run should clear it. Locally, test_workflow_run_approval.py and test_ci_minutes_estimate.py pass at 3c3c661a (85 tests).

Smaller observations

  • See inline comments on dev/calibrate_ci_minutes_estimates.py:18, dev/breeze/src/airflow_breeze/utils/workflow_run_approval.py:471.

This review was drafted by an AI-assisted tool and
confirmed by an Apache Airflow maintainer. The maintainer
approving this PR has read the findings and signed off. If
something feels off, please reply on the PR and a maintainer
will follow up.

More on how Apache Airflow handles maintainer review:
Contributing guide.

Comment thread dev/calibrate_ci_minutes_estimates.py Resolved
Comment thread dev/breeze/src/airflow_breeze/utils/workflow_run_approval.py Resolved
@potiuk
potiuk force-pushed the auto-approve-workflow-runs branch from 9b60f9c to 2669637 Compare October 25, 2026 17:34
@potiuk
potiuk enabled auto-merge (squash) October 25, 2026 17:35
@potiuk
potiuk force-pushed the auto-approve-workflow-runs branch from 2669637 to 3952b5a Compare October 25, 2026 18:19
@github-actions

github-actions Bot commented Oct 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

⚠️ 12 job(s) of attempt 1 were stopped because AWS reclaimed their EC2 Spot instances. These are not test failures.

RunsOn re-runs them automatically when the run finishes; no action is needed.

potiuk and others added 5 commits October 25, 2026 19:41
@potiuk
Auto-approve cheap CI runs of external contributors' PRs
7dd7044 
GitHub holds the workflow runs of fork PRs until a maintainer approves
them. With self-hosted runners (AIP-118) the approval gate becomes part
of the trust model, so it should cover all external contributors, but
approving every push by hand does not scale and makes contributors wait.

Runs whose change cannot touch the CI definition and whose CI is cheap
are safe to start without a maintainer. Expensive or large changes, and
anything that changes how CI or runners are chosen, still get a human
look first, which also keeps the load on the shared public runners down.
The CI-minutes estimate lets maintainers see what a run will cost before
approving it.

Generated-by: Claude Opus 5
@shahar1 @potiuk
Add inline script metadata to the CI-minutes calibration script
2f4eb88 
The script uses only the standard library, so with a PEP 723 block it runs
with plain `uv run dev/calibrate_ci_minutes_estimates.py` instead of syncing
the whole Breeze environment. Document that invocation in the script and in
.github/AGENTS.md.
@shahar1 @potiuk
Keep one PR's API or git error from aborting the approval tick
14d55da 
Each PR is now processed in its own try block. A requests.RequestException
or subprocess.CalledProcessError for one PR no longer stops the remaining
PRs or skips the job summary. The failed PR is recorded with failed=True and
shown as "failed" in the summary, keeping its CI-time estimate when one was
computed and giving a short, single-line form of the error as the reason.
The workflow also tries to add the 'need workflow approval' label to it, so
it is not lost once its runs leave the query window. Nothing is approved on
failure.

approve_workflow_runs exits 1 after writing the summary when any PR failed,
so a persistent failure turns the scheduled run red instead of passing
silently.
@potiuk
Approve the runs of fork CI pull requests unless they change CI
190e2e9 
A pull request whose CI runs in the contributor's fork (AIP-120) still
gets pull_request runs in apache/airflow, and GitHub holds them until a
maintainer approves them. Those runs only execute the fork CI gate, so
judging them by the cost and size of project CI left cheap gate-only
runs waiting for a maintainer for no benefit, and a gated run that never
ran could not be cleaned up by the reconciler.

What still needs a maintainer is a change to the files that define CI,
since it could remove the gate or add a workflow that does run. Using
the reconciler's own list keeps the two decisions consistent, and the
author is told why their runs wait instead of only seeing a label.

Generated-by: Claude Opus 5
@potiuk
Keep CI-minutes estimates true to the runs they approve
b84ff18 
A run selects its tests from the labels the PR had when it started, so
approving it on the current labels could start a full-tests run whose
label was removed while it waited. Calibrating from self-hosted and
all-versions runs, and leaving out the jobs that need no CI image, made
the estimate describe runs other than those it is used to approve.

Generated-by: Claude Opus 5
@potiuk
potiuk force-pushed the auto-approve-workflow-runs branch from 3952b5a to b84ff18 Compare October 25, 2026 18:43
@potiuk
potiuk merged commit 2a3ee7f into main Oct 25, 2026
319 of 332 checks passed
@potiuk
potiuk deleted the auto-approve-workflow-runs branch October 25, 2026 20:14
@github-actions

github-actions Bot commented Oct 25, 2026

Copy link
Copy Markdown
Contributor

Backport failed to create: v3-4-test. View the failure log Run details

Note: As of Merging PRs targeted for Airflow 3.X
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.

In matter of doubt please ask in #release-management Slack channel.

Status Branch Result
❌ v3-4-test Commit Link

You can attempt to backport this manually by running:

cherry_picker 2a3ee7f v3-4-test

This should apply the commit to the v3-4-test branch and leave the commit in conflict state marking
the files that need manual conflict resolution.

After you have resolved the conflicts, you can continue the backport process by running:

cherry_picker --continue

If you don't have cherry-picker installed, see the installation guide.

This was referenced Oct 26, 2026
Open
Merged
vatsrahul1001 pushed a commit that referenced this pull request Oct 26, 2026
@potiuk @shahar1
[v3-4-test] Backport fork CI auto-approval (#75616, #75619, #75455, #…
1a578ef 
…75654, #75655) (#75680)

* Show each fork CI job's status on gated pull requests (#75616)

A gated pull request showed one aggregate fork CI status, so a maintainer
or contributor had to open the fork run to see which jobs were running,
which failed and which passed. Reporting every job as its own check, each
linking to the job in the fork, puts that on the pull request itself,
where project CI shows its jobs.

Generated-by: Claude Opus 5
(cherry picked from commit 2f7a7ea)

* Cancel the workflow runs of pull requests that have already merged (#75619)

Merging a pull request does not stop its pull_request runs, so they keep
using the project's GitHub-hosted minutes and self-hosted runners for a
result nobody reads once the change is on its target branch. Until now
maintainers cancelled such runs by hand, when they noticed them.

Generated-by: Claude Opus 5
(cherry picked from commit e5f2c87)

* Auto-approve cheap CI runs of external contributors' PRs (#75455)

* Auto-approve cheap CI runs of external contributors' PRs

GitHub holds the workflow runs of fork PRs until a maintainer approves
them. With self-hosted runners (AIP-118) the approval gate becomes part
of the trust model, so it should cover all external contributors, but
approving every push by hand does not scale and makes contributors wait.

Runs whose change cannot touch the CI definition and whose CI is cheap
are safe to start without a maintainer. Expensive or large changes, and
anything that changes how CI or runners are chosen, still get a human
look first, which also keeps the load on the shared public runners down.
The CI-minutes estimate lets maintainers see what a run will cost before
approving it.

Generated-by: Claude Opus 5

* Add inline script metadata to the CI-minutes calibration script

The script uses only the standard library, so with a PEP 723 block it runs
with plain `uv run dev/calibrate_ci_minutes_estimates.py` instead of syncing
the whole Breeze environment. Document that invocation in the script and in
.github/AGENTS.md.

* Keep one PR's API or git error from aborting the approval tick

Each PR is now processed in its own try block. A requests.RequestException
or subprocess.CalledProcessError for one PR no longer stops the remaining
PRs or skips the job summary. The failed PR is recorded with failed=True and
shown as "failed" in the summary, keeping its CI-time estimate when one was
computed and giving a short, single-line form of the error as the reason.
The workflow also tries to add the 'need workflow approval' label to it, so
it is not lost once its runs leave the query window. Nothing is approved on
failure.

approve_workflow_runs exits 1 after writing the summary when any PR failed,
so a persistent failure turns the scheduled run red instead of passing
silently.

* Approve the runs of fork CI pull requests unless they change CI

A pull request whose CI runs in the contributor's fork (AIP-120) still
gets pull_request runs in apache/airflow, and GitHub holds them until a
maintainer approves them. Those runs only execute the fork CI gate, so
judging them by the cost and size of project CI left cheap gate-only
runs waiting for a maintainer for no benefit, and a gated run that never
ran could not be cleaned up by the reconciler.

What still needs a maintainer is a change to the files that define CI,
since it could remove the gate or add a workflow that does run. Using
the reconciler's own list keeps the two decisions consistent, and the
author is told why their runs wait instead of only seeing a label.

Generated-by: Claude Opus 5

* Keep CI-minutes estimates true to the runs they approve

A run selects its tests from the labels the PR had when it started, so
approving it on the current labels could start a full-tests run whose
label was removed while it waited. Calibrating from self-hosted and
all-versions runs, and leaving out the jobs that need no CI image, made
the estimate describe runs other than those it is used to approve.

Generated-by: Claude Opus 5

---------

Co-authored-by: Shahar Epstein <60007259+shahar1@users.noreply.github.com>
(cherry picked from commit 2a3ee7f)

* Approve fork CI runs that started before "use fork ci" was added (#75654)

When a committer adds "use fork ci" right after a PR opens, the PR's first
runs were created without it, and the workflow-run approver left them for a
maintainer. It had to: the fork CI gate fell back to the labels of the run's
event when it could not read the current ones, and those labels could still
send the run to full project CI on code nobody had reviewed. Anyone able to
exhaust the shared token quota could force that fallback.

The gate now fails when it cannot read the labels, so a run only ever acts on
the PR's current labels and a re-run decides again. With that, runs started
before the label changed are as safe to approve as later ones.

Generated-by: Claude Opus 5
(cherry picked from commit 437daf6)

* Check for workflow runs to auto-approve every 15 minutes (#75655)

External contributors' runs wait for the next approver pass before they
start, and GitHub regularly drops scheduled runs under load, which turns the
30-minute wait into an hour or more. A pass takes about a minute, so running
it twice as often costs little and halves the wait.

Generated-by: Claude Opus 5
(cherry picked from commit 54f645d)

* Pin the real selective-checks auto-approve test to main

The test decides approval for a PR targeting main, but built SelectiveChecks
with the checkout's default branch. On a release branch that skips provider
tests, so the large provider change was estimated cheap and approved.

Generated-by: Claude Opus 5

---------

Co-authored-by: Shahar Epstein <60007259+shahar1@users.noreply.github.com>
@shahar1 shahar1 mentioned this pull request Oct 27, 2026
Merged
1 task done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shahar1 shahar1 shahar1 approved these changes
@ashb ashb Awaiting requested review from ashb ashb is a code owner
@gopidesupavan gopidesupavan Awaiting requested review from gopidesupavan gopidesupavan is a code owner
@amoghrajesh amoghrajesh Awaiting requested review from amoghrajesh amoghrajesh is a code owner
@jscheffl jscheffl Awaiting requested review from jscheffl jscheffl is a code owner
@bugraoz93 bugraoz93 Awaiting requested review from bugraoz93 bugraoz93 is a code owner
@jason810496 jason810496 Awaiting requested review from jason810496 jason810496 is a code owner
@jedcunningham jedcunningham Awaiting requested review from jedcunningham jedcunningham is a code owner
@ephraimbuddy ephraimbuddy Awaiting requested review from ephraimbuddy ephraimbuddy is a code owner
@choo121600 choo121600 Awaiting requested review from choo121600 choo121600 is a code owner
@vatsrahul1001 vatsrahul1001 Awaiting requested review from vatsrahul1001 vatsrahul1001 is a code owner

Assignees

No one assigned

Labels

area:dev-tools backport-to-v3-4-test Backport to v3-4-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@potiuk @shahar1