This is an automated email from the ASF dual-hosted git repository. potiuk pushed a commit to branch fork-ci-no-draft-while-running in repository https://gitbox.apache.org/repos/asf/airflow.git
commit 608ca6a38bb434bbaf9f4a99cf6e173538c3d699 Author: Jarek Potiuk <[email protected]> AuthorDate: Sat Oct 31 17:37:52 2026 +0100 Do not draft fork CI pull requests while their fork run is in progress The fork CI monitor drafted a pull request for every state but green, including a run that had only just started. Rebasing an approved pull request therefore drafted it within minutes, before its CI had a chance to pass, which read as a failure to reviewers and authors alike. A running run asks nothing of the author; only a failed run, or a fork that needs setup, does. Generated-by: Claude Opus 5 --- .../doc/ci/08_fork_ci_for_external_contributors.md | 9 +++++---- .../src/airflow_breeze/utils/fork_ci_reconcile.py | 4 +++- dev/breeze/tests/test_fork_ci_reconcile.py | 18 +++++++++++++++--- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md b/dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md index 122081de163..51b595aa268 100644 --- a/dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md +++ b/dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md @@ -521,7 +521,7 @@ pull requests without a fork run for their head commit, not by the total. |---|---|---|---|---| | **setup_required**: workflow state is `disabled_fork`, `disabled_manually` or `disabled_inactivity`, or the fork has no `ci-amd.yml` | draft | remove | `error` (red), link to the fork's Actions page for `ci-amd.yml` | create or update: how to enable, with the exact `gh repo sync <fork> --branch main` and `gh workflow enable ci-amd.yml -R <fork>` lines, and `breeze ci audit`; on transition from any other state but `closed` (the workflow stopped running after it worked): also post a **ne [...] | **awaiting_push**: enabled, no run for the head SHA | draft | remove | `pending` (yellow), link to the fork's Actions page | on transition from setup_required: post a **new** comment asking to push again (an edit would not notify) | -| **running**: run in progress | draft | remove | `pending` (yellow), link to the run | update state and run link | +| **running**: run in progress | unchanged (a rebase of an approved pull request does not draft it; a failed run does) | remove | `pending` (yellow), link to the run | update state and run link | | **failed**: conclusion is failure, cancelled or timed out | draft | remove | `failure` (red), link to the run | update with the run link | | **green**: conclusion success and the pull request does not change CI-defining files | undraft\* | add | `success` (green), link to the run | update: ready for review, run link | | **green_unverified**: conclusion success, but the pull request changes CI-defining files (see "Trusting a green fork run") | draft | remove | `pending` (yellow), link to the run | update: a maintainer must run project CI; labelled `needs project ci` | @@ -541,9 +541,10 @@ Rules that sit under the table: loses the review label and is closed on the same run. - **A new push resets the row.** The head SHA is recorded in the state comment. A push to a - green pull request is observed as `running` or `awaiting_push` on the next run, which drafts - it again and removes the review label, so maintainers never see an unverified push in the - queue. + green pull request is observed as `running` or `awaiting_push` on the next run, which removes + the review label, so maintainers never see an unverified push in the queue. `awaiting_push` + also drafts it; `running` leaves the draft state alone, so rebasing an approved pull request + drafts it only if the new run fails. - **Author drafts are respected.** "draft" in the table means the reconciler converts to draft if the pull request is not already one. On `green` it undrafts only when GitHub's timeline shows the last convert-to-draft actor was `github-actions[bot]`. A pull request the author diff --git a/dev/breeze/src/airflow_breeze/utils/fork_ci_reconcile.py b/dev/breeze/src/airflow_breeze/utils/fork_ci_reconcile.py index c5470bf1b22..686ccc44136 100644 --- a/dev/breeze/src/airflow_breeze/utils/fork_ci_reconcile.py +++ b/dev/breeze/src/airflow_breeze/utils/fork_ci_reconcile.py @@ -737,7 +737,9 @@ def classify( if ready and LABEL_READY_FOR_REVIEW not in labels: plan.add_labels.append(LABEL_READY_FOR_REVIEW) else: - if not pr.is_draft: + # A run in progress needs nothing from the author, and drafting on it would draft every + # approved pull request the moment it is rebased; it is drafted only if the run then fails. + if not pr.is_draft and state != ForkState.RUNNING: plan.draft = True if LABEL_READY_FOR_REVIEW in labels: plan.remove_labels.append(LABEL_READY_FOR_REVIEW) diff --git a/dev/breeze/tests/test_fork_ci_reconcile.py b/dev/breeze/tests/test_fork_ci_reconcile.py index d0cf3feb67d..eeaf790570a 100644 --- a/dev/breeze/tests/test_fork_ci_reconcile.py +++ b/dev/breeze/tests/test_fork_ci_reconcile.py @@ -225,12 +225,24 @@ def test_setup_required_comment_gives_the_command(): assert "gh workflow enable ci-amd.yml -R someone/airflow" in plan.marker_comment -def test_non_green_drafts_and_removes_review_label(): [email protected]( + ("fork", "draft"), + [ + pytest.param(ForkObservation(suite_status="IN_PROGRESS", run_url=RUN_URL), None, id="running"), + pytest.param( + ForkObservation(suite_status="COMPLETED", suite_conclusion="FAILURE", run_url=RUN_URL), + True, + id="failed", + ), + pytest.param(ForkObservation(workflow_state="disabled_fork"), True, id="setup-required"), + ], +) +def test_non_green_removes_review_label_and_drafts_only_when_the_author_must_act(fork, draft): pr = _pr(is_draft=False, labels=("fork ci required", "ready for maintainer review")) - plan, _, _ = _classify(pr, ForkObservation(suite_status="IN_PROGRESS", run_url=RUN_URL)) + plan, _, _ = _classify(pr, fork) - assert plan.draft is True + assert plan.draft is draft assert plan.remove_labels == ["ready for maintainer review"]
