This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 2b59699b0fe Rework fork CI draft and label rules into one state table
(#76681)
2b59699b0fe is described below
commit 2b59699b0fe9683c418806a334b47077513bde5a
Author: Andrew Chang <[email protected]>
AuthorDate: Tue Nov 10 20:32:23 2026 +0800
Rework fork CI draft and label rules into one state table (#76681)
* Rework fork CI draft and label rules into one state table
Each branch of the reconciler set the draft state and the review labels
on its own, and the branches disagreed with each other and with the
state comment. A green pull request opened as a draft was told it was
ready for maintainer review while it had no review label and stayed a
draft. A review label added before the author drafted a pull request
stayed on the draft. Closing for the gate asked for a draft change that
GitHub always refuses on a closed pull request, which reported a
failure to Slack on every such closure.
One function now maps each state, and whether the pull request is the
author's own draft, to its draft state and labels, so each state shows
the same everywhere and the table can be read in one place.
The AIP-120 vote thread also asked for two rule changes, which are now
rows of that table and one setting: a failed fork run no longer drafts
the pull request, because a flaky test after a rebase drafted approved
pull requests and cancelled their auto-merge, and the inactivity close
waits 30 days instead of 7, for contributors who work in spare moments.
Generated-by: Claude Code (Opus 5.5)
* Keep the fork CI draft comment to what the pilot showed
Generated-by: Claude Code (Opus 5.5)
---
.github/fork-ci-gate.toml | 2 +-
contributing-docs/18_contribution_workflow.rst | 5 +-
contributing-docs/25_maintainer_pr_triage.md | 14 +-
.../doc/ci/08_fork_ci_for_external_contributors.md | 117 +++++++++++++----
.../src/airflow_breeze/utils/fork_ci_gate.py | 2 +-
.../src/airflow_breeze/utils/fork_ci_messages.py | 28 +++-
.../src/airflow_breeze/utils/fork_ci_reconcile.py | 110 ++++++++++------
dev/breeze/tests/test_fork_ci_gate.py | 2 +-
dev/breeze/tests/test_fork_ci_messages.py | 17 +++
dev/breeze/tests/test_fork_ci_reconcile.py | 143 +++++++++++++++++++--
10 files changed, 346 insertions(+), 94 deletions(-)
diff --git a/.github/fork-ci-gate.toml b/.github/fork-ci-gate.toml
index 6e3d5f5ec9e..0892c9b241c 100644
--- a/.github/fork-ci-gate.toml
+++ b/.github/fork-ci-gate.toml
@@ -32,7 +32,7 @@ enabled = false
enabled_since = 2026-01-01T00:00:00Z
# Days without any fork CI activity after which a gated pull request is closed.
-close_after_days = 7
+close_after_days = 30
# Report each job of a gated pull request's fork run as its own commit status.
Set to false when
# the fork CI monitor exceeds GitHub's rate limits (see dev/AGENTS.md); the
`Fork CI / Tests (AMD)`
diff --git a/contributing-docs/18_contribution_workflow.rst
b/contributing-docs/18_contribution_workflow.rst
index ab1960505a3..663808a4ed3 100644
--- a/contributing-docs/18_contribution_workflow.rst
+++ b/contributing-docs/18_contribution_workflow.rst
@@ -178,12 +178,13 @@ Set up your checkout and fork once:
breeze ci audit
When fork CI applies to your pull request, it is labelled ``fork ci required``
and kept in draft
-until the ``Tests (AMD)`` run in your fork for its latest commit is green. A
+until the ``Tests (AMD)`` workflow runs in your fork for its latest commit. A
failed run does not
+draft it. A
``Fork CI / Tests (AMD)`` status on the pull request links to that run, and
one comment on the pull
request explains the current state. Once the run is green, a pull request the
monitor drafted is
marked ready for review and labelled ``ready for maintainer review``; a pull
request you drafted
yourself stays a draft, and gets the label, once you mark it ready. A pull
request whose fork CI
-does not run for 7 days is closed with an explanation; you can reopen it after
setting up fork CI. If your pull request changes
+does not run for 30 days is closed with an explanation; you can reopen it
after setting up fork CI. If your pull request changes
files that decide which CI jobs run and where, such as ``.github/``, the
breeze sources or
``scripts/ci/prek/``, a green fork run is not enough: a maintainer reviews the
change and runs project CI for it.
diff --git a/contributing-docs/25_maintainer_pr_triage.md
b/contributing-docs/25_maintainer_pr_triage.md
index 333faefdd1f..17ec8edcb81 100644
--- a/contributing-docs/25_maintainer_pr_triage.md
+++ b/contributing-docs/25_maintainer_pr_triage.md
@@ -253,16 +253,18 @@ Pull requests labelled `fork ci required` run their CI in
the contributor's fork
fork CI reconciler (`breeze ci fork-ci-reconcile`, run by
`fork-ci-monitor.yml`) owns five things
for them, and the triage skill must treat those pull requests as out of scope
for each:
-- **Draft state.** The reconciler keeps the pull request in draft until the
fork run for its
- latest commit is green, and undrafts only what it drafted.
+- **Draft state.** The reconciler keeps the pull request in draft until the
fork CI runs for its
+ latest commit, undrafts it only when that run is green, and undrafts only
what it drafted. A
+ failed run does not draft it.
- **`ready for maintainer review`.** The reconciler adds it when the fork run
is green and the
pull request is not a draft the author made (it adds it once the author
marks it ready), and
- removes it when a new push starts a new run.
+ removes it when a new push starts a new run or the author converts the pull
request to draft.
- **`needs project ci`.** The reconciler adds it when the fork run is green
but the pull request
changes CI-defining files, and removes it once the state is anything else.
These pull requests
are not drafted and get no `ready for maintainer review` label. Their `Fork
CI / Tests (AMD)`
status stays pending without `CI passed`, and they keep `needs project ci`
until a maintainer
- reviews the CI change and adds `use project ci`.
+ reviews the CI change and adds `use project ci`. A draft the author made
gets neither label
+ until the author marks it ready for review.
- **Closing for changing the gate.** The reconciler closes a pull request that
changes the fork
CI gate (its code, configuration other than the author's own `[pilot]`
entry, the `gate` job
of `ci-amd.yml` or the monitor), and reports it on Slack. Add `use project
ci` to accept such
@@ -280,8 +282,8 @@ reopen the pull request (`gh pr close <n> -R apache/airflow
&& gh pr reopen <n>
or ask the author to push. Pull requests that change CI-defining files are not
drafted
and get no `ready for maintainer review` label even when the fork run is
green. Their
`Fork CI / Tests (AMD)` status stays pending without `CI passed`, and they are
labelled
-`needs project ci` until a maintainer reviews the CI change and adds `use
project ci`. For
-every other pull request, when the fork run is green and trusted, the
reconciler also posts the
+`needs project ci` until a maintainer reviews the CI change and adds `use
project ci`, unless
+the author keeps the pull request a draft. For every other pull request, when
the fork run is green and trusted, the reconciler also posts the
`CI passed` status the merge ruleset requires, so it can be merged or
auto-merged without project
CI. See
[Fork CI for external
contributors](../dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md).
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 e60311dfa06..1e73184cf61 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
@@ -93,7 +93,7 @@ Non-goals:
│ GraphQL: checkSuites │ ├─ per labelled PR: fork
state -> │
│ REST: workflow state │ │ draft/undraft,
labels, comment, │
└───────────────────────│ │ commit status with
run link, │
- │ │ close after 7 days or
when the │
+ │ │ close after 30 days
or when the │
│ │ PR tampered with the
gate │
│ └─ unknown failures ->
Slack │
└───────────────────────────────────────────┘
@@ -126,7 +126,7 @@ enabled = false
enabled_since = 2026-01-01T00:00:00Z
# Days without any fork CI activity after which a gated pull request is closed.
-close_after_days = 7
+close_after_days = 30
# Organisations exempted under AIP-120 §2.2. Keys are display names used in
comments,
# values are GitHub logins. Changes go through a reviewed pull request.
@@ -408,7 +408,8 @@ run's event, and fails when it cannot read them.
Both rules use the same list as "Trusting a green fork run", so a gated pull
request whose runs are
approved automatically is also one whose green fork run is trusted, and one
that changes CI is both
-left waiting for approval and labelled `needs project ci` once its fork run is
green. The gate
+left waiting for approval and labelled `needs project ci` once its fork run is
green, unless its
+author keeps it a draft. The gate
tampering check still applies to every run a maintainer approves by hand.
The approval reads only API metadata and the git objects of the merge commit;
it never checks out or
@@ -536,34 +537,98 @@ pull requests without a fork run for their head commit,
not by the total.
### State machine
-| Observed fork state | Draft | `ready for maintainer review` | Commit status
| Comment |
+```mermaid
+%%{init: {'theme': 'neutral'}}%%
+stateDiagram-v2
+ [*] --> gated: discovery adds fork ci required
+ state gated {
+ [*] --> setup_required: workflow not enabled in the fork
+ [*] --> awaiting_push: workflow enabled, no run for the head commit
+ [*] --> running: fork run in progress
+ setup_required --> awaiting_push: author enables the workflow
+ awaiting_push --> running: author pushes
+ running --> failed: run fails, is cancelled or times out
+ running --> green: run passes, no CI-defining change
+ running --> green_unverified: run passes, CI-defining change
+ failed --> running: author pushes
+ green --> running: author pushes
+ green_unverified --> running: author pushes
+ running --> setup_required: workflow disabled
+ awaiting_push --> setup_required: workflow disabled
+ state green {
+ [*] --> green_ready: not a draft, or the bot's draft
+ [*] --> green_author_draft: the author's draft
+ green_ready --> green_author_draft: author converts to draft
+ green_author_draft --> green_ready: author marks it ready
+ }
+ state green_unverified {
+ [*] --> unverified_ready: not a draft, or the bot's draft
+ [*] --> unverified_author_draft: the author's draft
+ unverified_ready --> unverified_author_draft: author converts to
draft
+ unverified_author_draft --> unverified_ready: author marks it ready
+ }
+ }
+ gated --> closed: setup_required or awaiting_push for close_after_days (30
days)
+ gated --> released: maintainer adds use project ci
+ gated --> ungated: base branch without fork CI, e.g. v3-3-test
+ gated --> gate_modified: pull request changes the gate
+ gated --> gate_bypassed: project CI ran without use project ci (gate
tampering)
+ closed --> gated: reopened
+```
+
+The draft state and the two review labels depend only on the state and on
whether the pull
+request is the author's draft: a draft whose last convert-to-draft event was
not by
+`github-actions[bot]`, or a pull request opened as a draft.
`compute_projection` in
+`fork_ci_reconcile.py` holds this table, and `test_projection_table` checks
it. The table follows
+one rule: a draft waits for the author, `ready for maintainer review` waits
for a maintainer
+review, and `needs project ci` waits for a maintainer to review the CI change
and add
+`use project ci`.
+
+| State | Next | Draft | `ready for maintainer review` | `needs project ci` |
|---|---|---|---|---|
-| **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 | 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") | undraft\* | remove |
`pending` (yellow), link to the run | update: a maintainer must run project CI;
labelled `needs project ci` |
-| setup_required or awaiting_push for `close_after_days` | unchanged |
unchanged | unchanged | close with a new comment; reopening restarts the clock |
-| **ungated**: the base branch has no gate, for example after retargeting to
an older release branch | undraft\* | unchanged | `success` (green), link to
the pull request's checks | update: project CI is in use; `fork ci required` is
removed |
-| **gate_modified**: the pull request changes the fork CI gate (see "Gate
protection") | draft | remove | `error` (red), link to the pull request's
changed files | post a **new** comment explaining why, then close; checked
after gate tampering, before the fork state |
-| **gate_bypassed**: a recorded project CI run of the pull request's branch
bypassed the gate (see "Gate tampering") | draft | remove | `error` (red), link
to the project CI run that should not have happened | post a **new** comment
explaining why, then close; takes precedence over every row above |
-
-\* only if the last convert-to-draft event on the pull request was by
`github-actions[bot]`;
-a pull request the author drafted stays a draft.
+| setup_required | Author enables the workflow | draft | remove | remove |
+| awaiting_push | Author pushes | draft | remove | remove |
+| failed | Author fixes and pushes | unchanged | remove | remove |
+| running | Nobody, fork CI runs | unchanged | remove | remove |
+| green | Maintainer reviews | undraft | add | remove |
+| green, author's draft | Author marks it ready | unchanged | remove | remove |
+| green_unverified | Maintainer reviews the CI change and adds `use project
ci` | undraft | remove | add |
+| green_unverified, author's draft | Author marks it ready | unchanged |
remove | remove |
+| released | Usual review with project CI | undraft, except the author's draft
| unchanged | remove |
+| ungated | Usual review with project CI | undraft, except the author's draft
| unchanged | remove |
+| closed | Author sets up fork CI and reopens | unchanged | remove | remove |
+| gate_modified | Maintainer reopens and adds `use project ci` | unchanged |
remove | remove |
+| gate_bypassed | Maintainer reopens and adds `use project ci` | unchanged |
remove | remove |
+
+The reconciler never undrafts the author's draft. It does not draft a closed
pull request either,
+since the draft-state job runs after the close and GitHub refuses to draft a
closed pull request.
+
+| Observed fork state | Commit status | Comment |
+|---|---|---|
+| **setup_required**: workflow state is `disabled_fork`, `disabled_manually`
or `disabled_inactivity`, or the fork has no `ci-amd.yml` | `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 **new** comment sayin [...]
+| **awaiting_push**: enabled, no run for the head SHA | `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 | `pending` (yellow), link to the run | update
state and run link |
+| **failed**: conclusion is failure, cancelled or timed out | `failure` (red),
link to the run | update with the run link |
+| **green**: conclusion success and the pull request does not change
CI-defining files | `success` (green), link to the run | update: ready for
review, run link; on the author's draft: mark it ready for review when it is |
+| **green_unverified**: conclusion success, but the pull request changes
CI-defining files (see "Trusting a green fork run") | `pending` (yellow), link
to the run | update: a maintainer must run project CI; on the author's draft:
mark it ready for review, then a maintainer runs project CI |
+| **closed**: setup_required or awaiting_push for `close_after_days` |
unchanged | close with a new comment; reopening restarts the clock |
+| **released**: the pull request carries `use project ci` | `success` (green),
link to the pull request's checks | update: project CI is in use; `fork ci
required` is removed |
+| **ungated**: the base branch has no gate, for example after retargeting to
an older release branch | `success` (green), link to the pull request's checks
| update: project CI is in use; `fork ci required` is removed |
+| **gate_modified**: the pull request changes the fork CI gate (see "Gate
protection") | `error` (red), link to the pull request's changed files | post a
**new** comment explaining why, then close; checked after gate tampering,
before the fork state |
+| **gate_bypassed**: a recorded project CI run of the pull request's branch
bypassed the gate (see "Gate tampering") | `error` (red), link to the project
CI run that should not have happened | post a **new** comment explaining why,
then close; takes precedence over every row above |
Rules that sit under the table:
- **Gate tampering wins.** The `gate_bypassed` check is evaluated first for
every pull
request whose eligibility reason is `external` or `pilot`; when it holds,
the fork state is not
- consulted at all. A pull request that was already `green` and undrafted is
drafted again,
- loses the review label and is closed on the same run.
+ consulted at all. A pull request that was already `green` 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 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.
+ also drafts it. `running` and `failed` leave the draft state alone, so a
rebase of an approved
+ pull request that hits a flaky test does not draft it and cancel its
auto-merge.
- **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
@@ -611,8 +676,8 @@ decided or recorded for the old commit, and the next run
decides for the new one
seen, the state is `green_unverified` instead of `green`. The pull request is
not drafted (a
draft the reconciler made earlier is undone, as for `green`), but it gets no
review label, the
status is pending, and it is labelled `needs project ci` so maintainers can
list these pull
-requests, review the change and add `use project ci`. Without `CI passed` it
cannot be merged
-until then. It is not posted to Slack:
+requests, review the change and add `use project ci`. A draft the author made
gets no label
+until the author marks it ready for review. Without `CI passed` it cannot be
merged until then. It is not posted to Slack:
the label already lists these pull requests, and a post for every new green
commit would bury
the CI breakages that channel is for.
The reconciler removes `needs project ci` as soon as the state is anything
else. A renamed file
@@ -714,8 +779,8 @@ contributor losing their collaborator association) with
full runs in the last si
(`TAMPER_WINDOW`) is closed. Signal 1 exists so that an external pull request
that edits
`ci-amd.yml` legitimately, and leaves the gate intact, is not closed for
touching the file.
-When both hold the reconciler records `gate_bypassed` in the state comment
marker, drafts the
-pull request, removes `ready for maintainer review`, posts an `error` status
whose
+When both hold the reconciler records `gate_bypassed` in the state comment
marker, removes
+`ready for maintainer review`, posts an `error` status whose
`target_url` is the project CI run that should not have happened, posts a new
comment and
closes. The comment says which run was observed, that changes to
`.github/workflows/` from a
gated pull request cannot be verified in `apache/airflow` without a
maintainer, and that a
@@ -770,7 +835,7 @@ run checks the pull request again, with the gate
configuration of its new base.
retarget to a base without the gate in between could close a pull request that
should be
released to project CI.
-The state is `gate_modified`: the pull request is drafted, loses `ready for
maintainer review`
+The state is `gate_modified`: the pull request loses `ready for maintainer
review`
and `needs project ci`, gets an `error` status linking its changed files, and
the closure is
reported to Slack as an attention item. Gate tampering is checked first and
wins. As for gate
tampering, only a reopen by someone other than the author counts: an open pull
request whose
diff --git a/dev/breeze/src/airflow_breeze/utils/fork_ci_gate.py
b/dev/breeze/src/airflow_breeze/utils/fork_ci_gate.py
index 13d59238efb..750f114276b 100644
--- a/dev/breeze/src/airflow_breeze/utils/fork_ci_gate.py
+++ b/dev/breeze/src/airflow_breeze/utils/fork_ci_gate.py
@@ -52,7 +52,7 @@ LABEL_USE_FORK_CI = "use fork ci"
LABEL_READY_FOR_REVIEW = "ready for maintainer review"
LABEL_NEEDS_PROJECT_CI = "needs project ci"
-DEFAULT_CLOSE_AFTER_DAYS = 7
+DEFAULT_CLOSE_AFTER_DAYS = 30
DEFAULT_ENABLED_SINCE = datetime(2026, 1, 1, tzinfo=UTC)
# The release-prep / providers branches whose pushes run project CI in
apache/airflow: the
diff --git a/dev/breeze/src/airflow_breeze/utils/fork_ci_messages.py
b/dev/breeze/src/airflow_breeze/utils/fork_ci_messages.py
index 10cca162083..a68acd8d8ea 100644
--- a/dev/breeze/src/airflow_breeze/utils/fork_ci_messages.py
+++ b/dev/breeze/src/airflow_breeze/utils/fork_ci_messages.py
@@ -144,6 +144,11 @@ def _setup_instructions(fork: str) -> str:
)
+_CHANGES_CI_FILES = (
+ "this pull request changes files that decide which CI jobs run and where
(`.github/`, the breeze "
+ "sources, the breeze installation or `scripts/ci/prek/`), and the fork run
used those changed files."
+)
+
_STATE_TEXTS = {
"setup_required": "**Fork CI: setup required.** CI for this pull request
has not run in your fork yet, "
"because the `Tests (AMD)` workflow is not enabled there.",
@@ -151,14 +156,12 @@ _STATE_TEXTS = {
"run for the latest commit of this pull request. Push the branch again to
start it.",
"running": "**Fork CI: running.** CI for the latest commit is running in
your fork.",
"failed": "**Fork CI: failed.** CI for the latest commit failed in your
fork. Fix the failures and "
- "push again; this pull request stays a draft until the run is green.",
+ "push again.",
"green": "**Fork CI: green.** CI for the latest commit passed in your
fork. The pull request is "
"ready for maintainer review.",
"green_unverified": "**Fork CI: green, needs a maintainer.** CI for the
latest commit passed in your "
- "fork, but this pull request changes files that decide which CI jobs run
and where (`.github/`, "
- "the breeze sources, the breeze installation or `scripts/ci/prek/`), and
the fork run used those "
- "changed files. A maintainer will review the change and run project CI for
it before it can be "
- "merged.",
+ f"fork, but {_CHANGES_CI_FILES} A maintainer will review the change and
run project CI for it before "
+ "it can be merged.",
"closed": "**Fork CI: closed after inactivity.** CI did not run in your
fork in time; see the "
"comment below for how to continue.",
"gate_bypassed": "**Fork CI: closed.** Project CI ran for this pull
request although its CI runs in "
@@ -166,6 +169,14 @@ _STATE_TEXTS = {
"gate_modified": "**Fork CI: closed.** This pull request changes the fork
CI gate; see the comment below.",
}
+_AUTHOR_DRAFT_TEXTS = {
+ "green": "**Fork CI: green.** CI for the latest commit passed in your
fork. The pull request is a "
+ "draft, so maintainers do not review it yet. Mark it ready for review when
it is.",
+ "green_unverified": "**Fork CI: green.** CI for the latest commit passed
in your fork, but "
+ f"{_CHANGES_CI_FILES} The pull request is a draft. Once you mark it ready
for review, a maintainer "
+ "will review the change and run project CI for it.",
+}
+
def behind_warning(behind: int, *, base_ref: str) -> str:
return (
@@ -175,8 +186,11 @@ def behind_warning(behind: int, *, base_ref: str) -> str:
)
-def state_comment(marker: Marker, *, fork: str, run_url: str | None, base_ref:
str = "main") -> str:
- parts = [render_marker(marker), _STATE_TEXTS[marker.state], WHY_FORK_CI]
+def state_comment(
+ marker: Marker, *, fork: str, run_url: str | None, base_ref: str = "main",
author_draft: bool = False
+) -> str:
+ text = _AUTHOR_DRAFT_TEXTS.get(marker.state) if author_draft else None
+ parts = [render_marker(marker), text or _STATE_TEXTS[marker.state],
WHY_FORK_CI]
if (
marker.state in ("green", "green_unverified")
and marker.behind is not None
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 4e2f8cfe6ef..6d666772e38 100644
--- a/dev/breeze/src/airflow_breeze/utils/fork_ci_reconcile.py
+++ b/dev/breeze/src/airflow_breeze/utils/fork_ci_reconcile.py
@@ -344,6 +344,70 @@ def _set_label(plan: Plan, labels: set[str], label: str,
*, present: bool) -> No
plan.remove_labels.append(label)
+@dataclass(frozen=True)
+class Projection:
+ """
+ What a gated pull request shows in a state; None leaves its draft state or
label as it is.
+
+ :param draft: whether the pull request should be a draft.
+ :param ready_for_review: whether it carries `ready for maintainer review`.
+ :param needs_project_ci: whether it carries `needs project ci`.
+ """
+
+ draft: bool | None
+ ready_for_review: bool | None
+ needs_project_ci: bool
+
+
+def compute_projection(state: ForkState, *, author_draft: bool) -> Projection:
+ """
+ Return what a pull request shows in a state.
+
+ :param author_draft: the pull request is a draft that the bot did not
make, including one opened
+ as a draft. It is never undrafted and waits for its author, not for a
maintainer.
+ """
+ if state in (ForkState.GREEN, ForkState.GREEN_UNVERIFIED):
+ if author_draft:
+ return Projection(draft=None, ready_for_review=False,
needs_project_ci=False)
+ return Projection(
+ draft=False,
+ ready_for_review=state == ForkState.GREEN,
+ needs_project_ci=state == ForkState.GREEN_UNVERIFIED,
+ )
+ if state in (ForkState.RELEASED, ForkState.UNGATED):
+ return Projection(
+ draft=None if author_draft else False, ready_for_review=None,
needs_project_ci=False
+ )
+ if state in (ForkState.SETUP_REQUIRED, ForkState.AWAITING_PUSH):
+ return Projection(draft=True, ready_for_review=False,
needs_project_ci=False)
+ # A running or failed pull request keeps its draft state, so a flaky test
after a rebase does
+ # not draft an approved one. A closed one is not drafted, since the
draft-state job runs after
+ # the close and GitHub refuses to draft a closed pull request.
+ if state in (
+ ForkState.RUNNING,
+ ForkState.FAILED,
+ ForkState.CLOSED,
+ ForkState.GATE_BYPASSED,
+ ForkState.GATE_MODIFIED,
+ ):
+ return Projection(draft=None, ready_for_review=False,
needs_project_ci=False)
+ raise ValueError(f"No projection for fork CI state {state.value!r}")
+
+
+def _is_author_draft(pr: PullRequestSnapshot) -> bool:
+ return pr.is_draft and not _is_bot(pr.last_draft_actor)
+
+
+def _enter_state(plan: Plan, pr: PullRequestSnapshot, labels: set[str], state:
ForkState) -> None:
+ plan.state = state
+ projection = compute_projection(state, author_draft=_is_author_draft(pr))
+ if projection.draft is not None and projection.draft != pr.is_draft:
+ plan.draft = projection.draft
+ if projection.ready_for_review is not None:
+ _set_label(plan, labels, LABEL_READY_FOR_REVIEW,
present=projection.ready_for_review)
+ _set_label(plan, labels, LABEL_NEEDS_PROJECT_CI,
present=projection.needs_project_ci)
+
+
def _workflow_file(resource_path: str | None) -> str:
return (resource_path or "").rsplit("/", 1)[-1]
@@ -584,11 +648,8 @@ def classify(
marker = None
def release(state: ForkState, comment: Callable[[Marker], str]) -> Plan:
- plan.state = state
plan.remove_labels.append(LABEL_FORK_CI_REQUIRED)
- _set_label(plan, labels, LABEL_NEEDS_PROJECT_CI, present=False)
- if pr.is_draft and _is_bot(pr.last_draft_actor):
- plan.draft = False
+ _enter_state(plan, pr, labels, state)
since = marker.since if marker and marker.state == state.value else now
_set_marker_comment(plan, pr, comment(Marker(state.value, pr.head_sha,
since)))
_set_status(plan, pr, state.value,
f"https://github.com/{repo}/pull/{pr.number}/checks";)
@@ -610,12 +671,7 @@ def classify(
if reason in TAMPER_CHECKED_REASONS:
bypass_run = find_bypass_run()
if bypass_run is not None:
- plan.state = ForkState.GATE_BYPASSED
- if not pr.is_draft:
- plan.draft = True
- if LABEL_READY_FOR_REVIEW in labels:
- plan.remove_labels.append(LABEL_READY_FOR_REVIEW)
- _set_label(plan, labels, LABEL_NEEDS_PROJECT_CI, present=False)
+ _enter_state(plan, pr, labels, ForkState.GATE_BYPASSED)
_set_status(plan, pr, ForkState.GATE_BYPASSED.value, bypass_run)
_set_marker_comment(
plan,
@@ -642,8 +698,7 @@ def classify(
if marker and marker.state in reopened_since_closing:
reopened = reopened_since_closing[marker.state]
if reopened is None or reopened <= marker.since:
- plan.state = ForkState(marker.state)
- _set_label(plan, labels, LABEL_NEEDS_PROJECT_CI, present=False)
+ _enter_state(plan, pr, labels, ForkState(marker.state))
if plan.state == ForkState.CLOSED:
comment =
inactivity_close_comment(days=config.close_after_days, fork=pr.head_repo)
elif plan.state == ForkState.GATE_MODIFIED:
@@ -676,11 +731,7 @@ def classify(
if gate_checked and not already_checked:
change = find_gate_change()
if change is not None:
- plan.state = ForkState.GATE_MODIFIED
- if not pr.is_draft:
- plan.draft = True
- _set_label(plan, labels, LABEL_READY_FOR_REVIEW, present=False)
- _set_label(plan, labels, LABEL_NEEDS_PROJECT_CI, present=False)
+ _enter_state(plan, pr, labels, ForkState.GATE_MODIFIED)
_set_status(
plan, pr, ForkState.GATE_MODIFIED.value,
f"https://github.com/{repo}/pull/{pr.number}/files";
)
@@ -753,8 +804,7 @@ def classify(
)
if state in CLOSABLE_STATES and now - since >=
timedelta(days=config.close_after_days):
- plan.state = ForkState.CLOSED
- _set_label(plan, labels, LABEL_NEEDS_PROJECT_CI, present=False)
+ _enter_state(plan, pr, labels, ForkState.CLOSED)
_set_marker_comment(
plan,
pr,
@@ -768,26 +818,7 @@ def classify(
plan.close = True
return plan
- if state in (ForkState.GREEN, ForkState.GREEN_UNVERIFIED):
- # A green run that changes CI files still needs a maintainer's review,
which a draft would
- # hide; the `needs project ci` label marks it instead of the review
label.
- if pr.is_draft and _is_bot(pr.last_draft_actor):
- plan.draft = False
- ready = not pr.is_draft or plan.draft is False
- if state == ForkState.GREEN and ready and LABEL_READY_FOR_REVIEW not
in labels:
- plan.add_labels.append(LABEL_READY_FOR_REVIEW)
- elif state == ForkState.GREEN_UNVERIFIED and LABEL_READY_FOR_REVIEW in
labels:
- plan.remove_labels.append(LABEL_READY_FOR_REVIEW)
- else:
- # 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)
- # Lets maintainers list the pull requests that only wait for them to run
project CI, which
- # otherwise look like any other pull request with a pending fork CI status.
- _set_label(plan, labels, LABEL_NEEDS_PROJECT_CI, present=state ==
ForkState.GREEN_UNVERIFIED)
+ _enter_state(plan, pr, labels, state)
target_url = fork.run_url or fork_actions_url(pr.head_repo)
_set_status(plan, pr, state.value, target_url, force=refresh_status)
@@ -809,6 +840,7 @@ def classify(
fork=pr.head_repo,
run_url=fork.run_url,
base_ref=pr.base_ref,
+ author_draft=_is_author_draft(pr),
),
)
return plan
diff --git a/dev/breeze/tests/test_fork_ci_gate.py
b/dev/breeze/tests/test_fork_ci_gate.py
index 29b4d5c820c..3e7932ec6e1 100644
--- a/dev/breeze/tests/test_fork_ci_gate.py
+++ b/dev/breeze/tests/test_fork_ci_gate.py
@@ -341,7 +341,7 @@ def test_parse_config_defaults():
config = parse_config("")
assert config.enabled is False
- assert config.close_after_days == 7
+ assert config.close_after_days == 30
assert config.allowlist == {}
assert config.pilot == {}
assert config.job_statuses is True
diff --git a/dev/breeze/tests/test_fork_ci_messages.py
b/dev/breeze/tests/test_fork_ci_messages.py
index 6bf940c69df..8d565cbecb4 100644
--- a/dev/breeze/tests/test_fork_ci_messages.py
+++ b/dev/breeze/tests/test_fork_ci_messages.py
@@ -90,6 +90,23 @@ def
test_state_comment_warns_about_a_branch_far_behind(state, behind, warned):
assert ("commits behind `v3-1-test`" in text) is warned
[email protected](
+ ("state", "author_draft", "asks_to_mark_ready"),
+ [
+ pytest.param("green", True, True, id="green-author-draft"),
+ pytest.param("green_unverified", True, True,
id="unverified-author-draft"),
+ pytest.param("green", False, False, id="green-ready"),
+ pytest.param("failed", True, False, id="failed-author-draft"),
+ ],
+)
+def test_state_comment_asks_the_author_to_mark_a_green_draft_ready(state,
author_draft, asks_to_mark_ready):
+ text = state_comment(
+ Marker(state, "a", MARKER.since), fork="u/airflow", run_url=None,
author_draft=author_draft
+ )
+
+ assert ("mark it ready for review" in text.lower()) is asks_to_mark_ready
+
+
def test_parse_marker_absent():
assert parse_marker("just a comment") is None
diff --git a/dev/breeze/tests/test_fork_ci_reconcile.py
b/dev/breeze/tests/test_fork_ci_reconcile.py
index de08f6a6f46..7b02221f062 100644
--- a/dev/breeze/tests/test_fork_ci_reconcile.py
+++ b/dev/breeze/tests/test_fork_ci_reconcile.py
@@ -52,6 +52,7 @@ from airflow_breeze.utils.fork_ci_reconcile import (
GateChangeFinder,
JobStatusReporter,
Plan,
+ Projection,
PullRequestChangedError,
PullRequestSnapshot,
apply_plan,
@@ -59,6 +60,7 @@ from airflow_breeze.utils.fork_ci_reconcile import (
build_slack_payload,
changes_ci_definition,
classify,
+ compute_projection,
count_commits_behind,
discover,
drop_reported,
@@ -236,13 +238,13 @@ def test_setup_required_comment_gives_the_command():
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,
+ None,
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):
+def
test_non_green_removes_review_label_and_drafts_only_without_a_fork_run(fork,
draft):
pr = _pr(is_draft=False, labels=("fork ci required", "ready for maintainer
review"))
plan, _, _ = _classify(pr, fork)
@@ -251,6 +253,105 @@ def
test_non_green_removes_review_label_and_drafts_only_when_the_author_must_act
assert plan.remove_labels == ["ready for maintainer review"]
[email protected](
+ ("state", "author_draft", "expected"),
+ [
+ pytest.param(
+ ForkState.SETUP_REQUIRED,
+ False,
+ Projection(draft=True, ready_for_review=False,
needs_project_ci=False),
+ id="setup-required",
+ ),
+ pytest.param(
+ ForkState.AWAITING_PUSH,
+ False,
+ Projection(draft=True, ready_for_review=False,
needs_project_ci=False),
+ id="awaiting-push",
+ ),
+ pytest.param(
+ ForkState.FAILED,
+ False,
+ Projection(draft=None, ready_for_review=False,
needs_project_ci=False),
+ id="failed",
+ ),
+ pytest.param(
+ ForkState.FAILED,
+ True,
+ Projection(draft=None, ready_for_review=False,
needs_project_ci=False),
+ id="failed-author-draft",
+ ),
+ pytest.param(
+ ForkState.RUNNING,
+ False,
+ Projection(draft=None, ready_for_review=False,
needs_project_ci=False),
+ id="running",
+ ),
+ pytest.param(
+ ForkState.GREEN,
+ False,
+ Projection(draft=False, ready_for_review=True,
needs_project_ci=False),
+ id="green",
+ ),
+ pytest.param(
+ ForkState.GREEN,
+ True,
+ Projection(draft=None, ready_for_review=False,
needs_project_ci=False),
+ id="green-author-draft",
+ ),
+ pytest.param(
+ ForkState.GREEN_UNVERIFIED,
+ False,
+ Projection(draft=False, ready_for_review=False,
needs_project_ci=True),
+ id="unverified",
+ ),
+ pytest.param(
+ ForkState.GREEN_UNVERIFIED,
+ True,
+ Projection(draft=None, ready_for_review=False,
needs_project_ci=False),
+ id="unverified-author-draft",
+ ),
+ pytest.param(
+ ForkState.RELEASED,
+ False,
+ Projection(draft=False, ready_for_review=None,
needs_project_ci=False),
+ id="released",
+ ),
+ pytest.param(
+ ForkState.RELEASED,
+ True,
+ Projection(draft=None, ready_for_review=None,
needs_project_ci=False),
+ id="released-author-draft",
+ ),
+ pytest.param(
+ ForkState.UNGATED,
+ False,
+ Projection(draft=False, ready_for_review=None,
needs_project_ci=False),
+ id="ungated",
+ ),
+ pytest.param(
+ ForkState.CLOSED,
+ False,
+ Projection(draft=None, ready_for_review=False,
needs_project_ci=False),
+ id="closed",
+ ),
+ pytest.param(
+ ForkState.GATE_BYPASSED,
+ False,
+ Projection(draft=None, ready_for_review=False,
needs_project_ci=False),
+ id="gate-bypassed",
+ ),
+ pytest.param(
+ ForkState.GATE_MODIFIED,
+ False,
+ Projection(draft=None, ready_for_review=False,
needs_project_ci=False),
+ id="gate-modified",
+ ),
+ ],
+)
+def test_projection_table(state, author_draft, expected):
+ assert compute_projection(state, author_draft=author_draft) == expected
+
+
def test_green_undrafts_what_the_bot_drafted():
plan, _, _ = _classify(_pr(last_draft_actor="github-actions"), GREEN)
@@ -258,11 +359,22 @@ def test_green_undrafts_what_the_bot_drafted():
assert plan.add_labels == ["ready for maintainer review"]
-def test_green_keeps_an_author_draft():
- plan, _, _ = _classify(_pr(last_draft_actor="someone"), GREEN)
[email protected](
+ "last_draft_actor",
+ [
+ pytest.param("someone", id="drafted-by-author"),
+ pytest.param(None, id="opened-as-draft"),
+ ],
+)
+def test_green_keeps_an_author_draft_out_of_the_review_queue(last_draft_actor):
+ pr = _pr(last_draft_actor=last_draft_actor, labels=("fork ci required",
"ready for maintainer review"))
+
+ plan, _, _ = _classify(pr, GREEN)
assert plan.draft is None
assert plan.add_labels == []
+ assert plan.remove_labels == ["ready for maintainer review"]
+ assert "Mark it ready for review when it is." in plan.marker_comment
def test_green_labels_a_ready_pull_request():
@@ -292,18 +404,27 @@ def test_green_is_not_trusted_when_ci_files_change():
@pytest.mark.parametrize(
- ("last_draft_actor", "expected_draft"),
+ ("last_draft_actor", "expected_draft", "expected_labels", "expected_text"),
[
- pytest.param("github-actions[bot]", False, id="bot-draft-undrafted"),
- pytest.param("someone", None, id="author-draft-kept"),
+ pytest.param(
+ "github-actions[bot]",
+ False,
+ ["needs project ci"],
+ "run project CI for it before it can be merged",
+ id="bot-draft-undrafted",
+ ),
+ pytest.param("someone", None, [], "Once you mark it ready for review",
id="author-draft-kept"),
],
)
-def test_green_unverified_does_not_keep_a_bot_draft(last_draft_actor,
expected_draft):
+def test_green_unverified_does_not_keep_a_bot_draft(
+ last_draft_actor, expected_draft, expected_labels, expected_text
+):
plan, _, _ = _classify(_pr(last_draft_actor=last_draft_actor), GREEN,
changes_ci_files=True)
assert plan.state == ForkState.GREEN_UNVERIFIED
assert plan.draft is expected_draft
- assert plan.add_labels == ["needs project ci"]
+ assert plan.add_labels == expected_labels
+ assert expected_text in plan.marker_comment
LABELLED = ("fork ci required", "needs project ci")
@@ -760,7 +881,7 @@ def test_gate_tampering_closes(pr_overrides, checked):
assert plan.close is checked
if checked:
assert plan.state == ForkState.GATE_BYPASSED
- assert plan.draft is True
+ assert plan.draft is None
assert plan.status == (
"error",
PROJECT_RUN_URL,
@@ -2881,7 +3002,7 @@ def test_a_change_to_the_gate_closes():
assert plan.state == ForkState.GATE_MODIFIED
assert plan.close is True
- assert plan.draft is True
+ assert plan.draft is None
assert plan.remove_labels == ["ready for maintainer review"]
assert plan.status == (
"error",
