Skip to content

Add design for running CI in contributor forks (AIP-120) - #73378

Closed
potiuk wants to merge 6 commits into
apache:mainfrom
potiuk:fork-ci-for-external-contributors
Closed

potiuk wants to merge 6 commits into
apache:mainfrom
potiuk:fork-ci-for-external-contributors

Conversation

@potiuk

@potiuk potiuk commented Sep 19, 2026 •

Copy link
Copy Markdown
Member

Design document and ADR for the mechanics of AIP-120: Run CI for external contributions in contributor forks, discussed on the dev list.

The commits add the design (dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md) and ADR 0020. The implementation is in separate stacked PRs (see "Implementation" below), in this order:

  1. Add the fork CI gate to ci-amd.yml #74819: eligibility function, .github/fork-ci-gate.toml (ships with enabled = false) and the gate job in ci-amd.yml.
  2. Run fork pushes with pull-request selective checks #74820: fork-mode selective checks (changed files via the cross-repository compare API, no forced full matrix, public runners only).
  3. Add the fork CI reconciler and its scheduled monitor #74822: scheduled reconciler (breeze ci fork-ci-reconcile) and fork-ci-monitor.yml: labels, draft/undraft, state comment, a Fork CI / Tests (AMD) commit status on the PR head (red / yellow / green, linking to the run in the fork), close after 7 days without fork CI activity, close of gated PRs that tampered with the gate, and unknown failures plus attention items posted to internal-airflow-ci-cd.
  4. Add breeze ci audit for the fork CI setup #74823: breeze ci audit for the contributor's local pre-flight.
  5. Document fork CI for contributors and maintainers #74825: contribution and maintainer docs.

No pull_request_target is used; all pull request mutations come from the scheduled reconciler. Nothing changes behaviour until enabled = true is set in a follow-up PR after the AIP vote.

Gate tampering. pull_request runs use the workflow files of the PR's merge commit, so a gated PR can drop the gate job or add a workflow with a pull_request trigger and get project CI once. The gate job cannot prevent that; the reconciler detects it after the fact from what GitHub recorded, never from the PR tree: project CI ran for the head SHA and the PR changes files under .github/workflows/. Both signals are required so legitimate project CI runs (backlog reset, a removed use project ci) and legitimate workflow edits that keep the gate intact are never closed. Closing stops further pull_request runs, so one attempt buys one run; a maintainer who wants the workflow change reopens with use project ci. ADR 0020 records the trade-off.

Opt-in pilot. Following the dev list agreement to start with volunteers, .github/fork-ci-gate.toml has a [pilot] table of contributor logins and the instant each one joined. Pull requests those contributors open after joining are gated from their first run whether or not enabled is set, and go through the full flow (setup comment, draft, fork run, commit status, undraft, inactivity close, tampering check); everyone else keeps project CI. Contributors opt in or out with a pull request editing the table. Committers can still pilot single pull requests with the use fork ci label.

Implementation. The implementation is stacked from branches in apache/airflow: #74819 (gate) → #74820 (selective checks) → #74822 (reconciler) → #74823 (audit) → #74825 (docs). Review this design PR first. A security review led to the latest commit here: green fork runs are trusted only when the PR can't change its own CI, tampering is detected from recorded runs rather than the head commit, and the gate never fails open.


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

Generated-by: Claude Code (Fable 5.1) following the guidelines

🤖 Generated with Claude Code

AIP-120 moves CI for external contributions into contributor forks so
project runners serve committers, collaborators and exempted
organisations, and so maintainers only review pull requests that
already passed in the author's fork. The mechanics span the main test
workflow, a scheduled reconciler for gated pull requests, a commit
status that surfaces the fork CI state with a link to the fork run, a
local pre-flight command and the contribution docs, so the design is
recorded before implementation and the decisions that shape it are
captured as ADR 0018.

Generated-by: Claude Opus 5
The gate job in ci-amd.yml runs from the pull request's merge commit,
so a gated external pull request can drop the gate or add a workflow
with a pull_request trigger and get project CI anyway. The design
accepted that as a one-run cost but left it unbounded per pull
request, because every further push would get another run.

The reconciler now detects this after the fact from what GitHub
recorded rather than from the pull request tree: project CI ran for
the head SHA and the pull request changes files under
.github/workflows/. Both signals are required so that legitimate
project CI runs (backlog reset, a removed "use project ci") and
legitimate workflow edits that keep the gate intact are never closed.
Closing stops further pull_request runs, so one attempt buys one run,
and the closure is reported to Slack as an attention item rather than
as a reconciler failure. ADR 0018 records the trade-off, including the
external pull requests with wanted workflow changes that must keep
"use project ci".

Generated-by: Claude Opus 5
The dev list agreed to start with an opt-in pilot by a few volunteer
contributors rather than enabling fork CI for everyone at once. The
committer-only "use fork ci" label cannot serve that: it arrives after
the first pull_request run has already used project CI, and a
maintainer would have to label every pull request the volunteer opens.
A per-login join instant gates all their new pull requests from the
first run while leaving older ones on project CI, so those can never be
mistaken for gate tampering.

The reconciler also has to run while the gate is disabled, which the
earlier "exits early" wording contradicted for the label pilot as well.

Generated-by: Claude Opus 5
Enabling ci-amd.yml in a fork also enables its cron, which would run the
full canary matrix in every contributor fork twice a day. A join instant
or enabled_since earlier than the merge that sets it gates pull requests
that already ran project CI. Forks not named "airflow" would never show
a fork run, and a pull request reopened after an inactivity close would
be closed again at once. ADR 0018 was already taken.

Generated-by: Claude Opus 5
A fork run executes the workflows, breeze and CI scripts of the pushed commit, so a pull request changing them decides its own fork CI result. Gate tampering was only checked on the head commit, so pushing a tampered commit and then a clean one hid the bypass, and the check could be dodged by renaming a job to the gate's name or by truncated check-suite listings. The gate also fell back to project CI when its label read failed, which an exhausted token quota could trigger for every pull request.

Generated-by: Claude Opus 5
@potiuk
potiuk force-pushed the fork-ci-for-external-contributors branch from e9907e3 to 04bcbd5 Compare October 20, 2026 22:19
Bring the design doc and ADR in line with the five implementation pull requests after their review: head_is_upstream before the label and pilot rules, base_without_gate, the needs project ci label, the CI passed status for gated pull requests, the warning for a branch far behind its base, fork pushes reading labels and the merge base, and the audit and reconciler fixes. green_unverified is no longer posted to Slack, so the ADR's two expected Slack outcomes hold again.

Generated-by: Claude Opus 5
@potiuk
potiuk force-pushed the fork-ci-for-external-contributors branch from 04bcbd5 to f5f5d6a Compare October 20, 2026 22:52
@potiuk
potiuk requested a review from shahar1 October 24, 2026 12:25
@shahar1

shahar1 commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Closing as superseded: everything in this PR is already on main.

Rebasing this branch onto main makes all six commits empty, because main's text wins every conflict. There is nothing left to merge here.


Drafted-by: Claude Code (Opus 5.5); reviewed by @shahar1 before posting

@shahar1 shahar1 closed this Oct 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants