Skip to content

Wait for a maintainer when a fork PR changes any CI-defining file - #75718

Merged
potiuk merged 1 commit into
apache:mainfrom
shahar1:auto-approve-ci-definition-files
Oct 27, 2026
Merged

potiuk merged 1 commit into
apache:mainfrom
shahar1:auto-approve-ci-definition-files

Conversation

@shahar1

@shahar1 shahar1 commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

As-titled, CI-defining files should be gated.

AI Summary

The workflow run auto-approval added in #75455 has two rules. A PR whose CI runs in the contributor's fork is held for a maintainer when it changes a file the fork CI reconciler counts as CI-defining. A PR whose CI runs in the project was checked against a shorter list: .github/, dev/breeze/ and scripts/ci/.

Running the decision with real selective checks on main (54f645d) showed the effect:

  • A fork PR that changed only a .pre-commit-config.yaml had its runs approved without a maintainer's look. This holds for the root file (estimated 274 job-minutes) and for nested ones such as airflow-core/.pre-commit-config.yaml (6 job-minutes). These files decide which static checks CI runs.
  • Changes to Dockerfile*, scripts/docker/ and scripts/in_container/ were refused, but only because selective checks currently run the full set of tests for them. Narrowing that selective-checks rule would have let them through.

This PR makes the project rule use the existing find_fork_ci_definition_files. Both rules now read the reconciler's list, so a path added there applies to both. The docs that list the paths are updated.

Not covered here, and the same for both rules: other files that CI runs, such as scripts/tools/, devel-common/ or nested Dockerfiles like airflow-e2e-tests/docker/*. Widening CI_DEFINING_PREFIXES in fork_ci_reconcile.py would cover both rules.

Checks run locally:

  • New parametrized cases in test_decide_approval for .pre-commit-config.yaml, airflow-core/.pre-commit-config.yaml, Dockerfile.ci, scripts/docker/entrypoint_ci.sh and scripts/in_container/run_ci_tests.sh. All five fail without the change.
  • test_workflow_run_approval.py and test_fork_ci_reconcile.py: 321 passed.
  • prek pre-commit and manual stages passed. Skipped: lychee-docker, because Docker was unavailable in the session.
  • breeze verify commands (full dev/breeze suite and its integration tests): 23 failures in test_packages.py, test_selective_checks.py, test_shim_version_check.py, test_workflow_commands.py and the reproducible-build integration test. The 22 non-integration ones fail the same way with this change reverted. test_publish_docs_to_s3.py could not be collected locally (needs botocore[crt]).

related: #75455


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

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

The project-CI rule of the workflow run auto-approval checked a shorter
list of CI-defining paths than the fork CI reconciler. A PR changing a
.pre-commit-config.yaml, which decides the static checks CI runs, had its
runs approved without a maintainer's look. Changes to Dockerfile*,
scripts/docker/ and scripts/in_container/ were held back only because
selective checks currently run the full set of tests for them. Both
rules now read the reconciler's list, so a path added there applies to
both.
@potiuk
potiuk merged commit c79293e into apache:main Oct 27, 2026
148 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

Backport successfully created: v3-4-test

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 PR Link

github-actions Bot pushed a commit to aws-mwaa/upstream-to-airflow that referenced this pull request Oct 27, 2026
…ing file (apache#75718)

The project-CI rule of the workflow run auto-approval checked a shorter
list of CI-defining paths than the fork CI reconciler. A PR changing a
.pre-commit-config.yaml, which decides the static checks CI runs, had its
runs approved without a maintainer's look. Changes to Dockerfile*,
scripts/docker/ and scripts/in_container/ were held back only because
selective checks currently run the full set of tests for them. Both
rules now read the reconciler's list, so a path added there applies to
both.
(cherry picked from commit c79293e)

Co-authored-by: Shahar Epstein <60007259+shahar1@users.noreply.github.com>
vatsrahul1001 pushed a commit that referenced this pull request Oct 27, 2026
…ing file (#75718)

The project-CI rule of the workflow run auto-approval checked a shorter
list of CI-defining paths than the fork CI reconciler. A PR changing a
.pre-commit-config.yaml, which decides the static checks CI runs, had its
runs approved without a maintainer's look. Changes to Dockerfile*,
scripts/docker/ and scripts/in_container/ were held back only because
selective checks currently run the full set of tests for them. Both
rules now read the reconciler's list, so a path added there applies to
both.
(cherry picked from commit c79293e)

Co-authored-by: Shahar Epstein <60007259+shahar1@users.noreply.github.com>
vatsrahul1001 pushed a commit that referenced this pull request Oct 27, 2026
…ing file (#75718) (#75733)

The project-CI rule of the workflow run auto-approval checked a shorter
list of CI-defining paths than the fork CI reconciler. A PR changing a
.pre-commit-config.yaml, which decides the static checks CI runs, had its
runs approved without a maintainer's look. Changes to Dockerfile*,
scripts/docker/ and scripts/in_container/ were held back only because
selective checks currently run the full set of tests for them. Both
rules now read the reconciler's list, so a path added there applies to
both.
(cherry picked from commit c79293e)

Co-authored-by: Shahar Epstein <60007259+shahar1@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants