This is an automated email from the ASF dual-hosted git repository. potiuk pushed a commit to branch fork-ci-detect-disabled-workflow in repository https://gitbox.apache.org/repos/asf/airflow.git
commit 58a5463107c8d285badb4d4008321c09af5647bf Author: Jarek Potiuk <[email protected]> AuthorDate: Tue Oct 27 19:13:57 2026 +0100 Tell contributors when their fork CI workflow stops running The fork CI monitor read the fork's workflow state only until it first saw the workflow enabled. When the workflow was disabled afterwards, by the contributor or by GitHub after 60 days of fork inactivity, the pull request kept waiting for a push that could never run CI, while its comment still said the workflow was enabled. Contributors now get a new comment telling them why it stopped and how to turn it back on, starting with syncing their fork's main from apache/airflow. breeze ci audit also stops suggesting committers disable the workflow: a committer trying fork CI needs it, and following the hint is one way a pull request ends up stuck like this. Generated-by: Claude Opus 5 --- contributing-docs/18_contribution_workflow.rst | 10 +-- dev/breeze/doc/08_ci_tasks.rst | 4 +- .../doc/ci/08_fork_ci_for_external_contributors.md | 26 ++++---- dev/breeze/src/airflow_breeze/utils/ci_audit.py | 12 ---- .../src/airflow_breeze/utils/fork_ci_messages.py | 26 ++++++-- .../src/airflow_breeze/utils/fork_ci_reconcile.py | 29 +++++---- dev/breeze/tests/test_ci_audit.py | 4 +- dev/breeze/tests/test_fork_ci_messages.py | 6 ++ dev/breeze/tests/test_fork_ci_reconcile.py | 74 +++++++++++++++++++++- 9 files changed, 141 insertions(+), 50 deletions(-) diff --git a/contributing-docs/18_contribution_workflow.rst b/contributing-docs/18_contribution_workflow.rst index d864040f24c..7ebe12fb120 100644 --- a/contributing-docs/18_contribution_workflow.rst +++ b/contributing-docs/18_contribution_workflow.rst @@ -147,15 +147,17 @@ Set up your checkout and fork once: 2. If fork CI applies to you, enable the ``Tests (AMD)`` workflow in your fork: today that means you joined the pilot or a maintainer labelled your pull request ``use fork ci``; once fork CI is switched on for everyone, it means you are not a committer, a collaborator or a member of an - exempted organisation. GitHub disables workflows in new forks. Otherwise keep it disabled, so - your pushes do not run CI twice: + exempted organisation. GitHub disables workflows in new forks. Sync your fork's ``main`` with + ``apache/airflow`` first, so it has the current version of the workflow: .. code-block:: bash + gh repo sync <you>/airflow --branch main gh workflow enable ci-amd.yml -R <you>/airflow - If the workflow is missing, your fork's ``main`` is older than it; sync it first with - ``gh repo sync <you>/airflow --branch main``. + Keep the workflow enabled for as long as you have open pull requests. If it gets disabled, from + your fork's Actions tab or by GitHub after 60 days without activity in your fork, the fork CI + monitor comments on your pull requests with the commands to turn it back on. 3. Check everything at once. ``breeze ci audit`` prints one row per check and the command that fixes anything missing; ``--fix`` runs those commands after asking: diff --git a/dev/breeze/doc/08_ci_tasks.rst b/dev/breeze/doc/08_ci_tasks.rst index 5e1b9311f32..7147b615096 100644 --- a/dev/breeze/doc/08_ci_tasks.rst +++ b/dev/breeze/doc/08_ci_tasks.rst @@ -168,8 +168,8 @@ Pull requests from external contributors can run their CI in the contributor's f ``breeze ci audit`` checks, read-only, whether your checkout and fork are set up for that: ``gh`` installed and authenticated, the ``upstream`` remote pointing at ``apache/airflow`` and ``origin`` at your fork, whether you are a committer, collaborator, member of an exempted organisation, pilot participant or -external contributor, whether the ``Tests (AMD)`` workflow in your fork is enabled (or disabled, for those -whose pull requests run project CI), how far your fork's ``main`` is behind, and the branch you are on. +external contributor, whether the ``Tests (AMD)`` workflow in your fork is enabled (when your pull requests +use fork CI), how far your fork's ``main`` is behind, and the branch you are on. Each failing check prints the command that fixes it; ``--fix`` runs those commands after asking. These are all available flags of ``audit`` command: 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 ab33c09aec3..ad06dbc54c5 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 @@ -490,16 +490,19 @@ that matters. It is matched by file, not by the workflow's display name, which a change, and only `push` runs count because only they execute the workflow from the head commit itself; a `pull_request` run inside the fork, into the fork's own `main`, runs a merge the contributor controls. For pull requests -whose recorded state is `setup_required` (or that have no recorded state yet), one REST call -per pull request reads `GET /repos/<fork>/actions/workflows/ci-amd.yml` to distinguish -"workflows disabled" from "enabled but not yet pushed". This is bounded by the number of -pull requests in that state, not by the total. +with no fork run for the head commit, one REST call per pull request reads +`GET /repos/<fork>/actions/workflows/ci-amd.yml` to distinguish "workflows disabled" from +"enabled but not yet pushed". It is read on every run, including for pull requests already +recorded as `awaiting_push`: a workflow can be disabled after it was enabled, by the +contributor or by GitHub after 60 days without activity in the fork, and the pull request +would otherwise keep waiting for a push that never runs CI. This is bounded by the number of +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 | |---|---|---|---|---| -| **setup_required**: workflow state is `disabled_fork`, 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 workflow enable ci-amd.yml -R <fork>` line, and `breeze ci audit` | +| **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 | | **failed**: conclusion is failure, cancelled or timed out | draft | remove | `failure` (red), link to the run | update with the run link | @@ -872,7 +875,7 @@ Unknown means any of: that outlasted its retries. - A check suite for `Tests (AMD)`, in the fork or in `apache/airflow`, with a status or conclusion outside the modelled set. -- A workflow state outside `active` / `disabled_fork` / `disabled_manually` / not found. +- A workflow state outside `active` / `disabled_fork` / `disabled_manually` / `disabled_inactivity` / not found. - A mutation that returned an error (label, comment, close, status). - A head repository that no longer exists. - A state comment whose marker cannot be parsed. @@ -949,18 +952,17 @@ prints one row: status, finding, and the command that fixes it. | 2 | Remote naming | a remote named `upstream` points at `apache/airflow` and `origin` does not | `git remote rename`/`add` per `contributing-docs/10_working_with_git.rst` | | 3 | Fork exists | `origin` is a fork of `apache/airflow` owned by the authenticated user | `gh repo fork apache/airflow --remote=false` then add `origin`, unless check 2's fix already adds or repoints it | | 4 | Who you are | classifies the user in the gate's order: pilot participant (`[pilot]`, which gates whatever the role), committer (public `apache` membership, `GET /orgs/apache/public_members/<login>`, since the gate cannot see private membership), collaborator (`.asf.yaml`), exempted (`[allowlist]`), external; an external user whose `apache` membership is private is told to make it public or ask for `[allowlist]`; when the membership call fails for any reason but a 404, the role is r [...] -| 5 | Fork workflows | external or pilot participant: `ci-amd.yml` state on the fork is `active`; committer/collaborator/exempted: informational, with a hint when it is `active` | `gh workflow enable ci-amd.yml -R <fork>`, or `gh workflow disable ...` shown but not run by `--fix` | +| 5 | Fork workflows | external or pilot participant: `ci-amd.yml` state on the fork is `active`; committer/collaborator/exempted: always OK, whatever the state | `gh workflow enable ci-amd.yml -R <fork>` | | 6 | Fork `main` freshness | fork `main` is not behind `upstream/main` by more than 200 commits (`MAX_FORK_MAIN_BEHIND`) | `gh repo sync <fork> --branch main` | | 7 | Current branch | not `main`, has an upstream tracking `origin` | informational | The role is determined before checks 2 and 3, though printed as check 4. For a committer, collaborator or exempted contributor, whose pull requests run project CI, checks 2 and 3 are informational: working from an `apache/airflow` clone without a fork is fine for them, and -`--fix` only runs the fixes of failed checks. The same goes for check 5: an enabled fork -workflow runs every push twice and attaches a second status, so they are told they can disable -it, but it is not a failure, because a committer trying fork CI with the `use fork ci` label -needs it enabled. Forks of a public repository cannot be made private on GitHub, so disabling -the workflow is the only recommendation available. +`--fix` only runs the fixes of failed checks. Check 5 passes for them whether the fork workflow +is enabled or not. The audit never suggests disabling it: a committer trying fork CI with the +`use fork ci` label needs it enabled, and a workflow disabled while a pull request waits for its +fork CI leaves that pull request stuck until the workflow is enabled again. A failed API call is reported as "could not check" rather than as a finding: a failed fork lookup is not "not a fork of apache/airflow", and a failed workflow lookup is not "missing". diff --git a/dev/breeze/src/airflow_breeze/utils/ci_audit.py b/dev/breeze/src/airflow_breeze/utils/ci_audit.py index 3b27ef8eb15..94b0cf7ce9d 100644 --- a/dev/breeze/src/airflow_breeze/utils/ci_audit.py +++ b/dev/breeze/src/airflow_breeze/utils/ci_audit.py @@ -327,18 +327,6 @@ def run_audit(*, run: Runner, config: GateConfig, collaborators: frozenset[str]) enable, ) ) - elif required is False and state == "active": - # Not a failure: a committer trying fork CI with the `use fork ci` label needs it on. - results.append( - CheckResult( - 5, - "fork workflows", - CheckStatus.INFO, - f"`Tests (AMD)` is enabled in {fork}, but your pull requests run project CI, so each " - "push runs CI twice; disable it unless you use the `use fork ci` label", - (f"gh workflow disable {CI_WORKFLOW_FILE} -R {fork}",), - ) - ) elif required is None: results.append( CheckResult( 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 48208ae90a1..90f6b6cba51 100644 --- a/dev/breeze/src/airflow_breeze/utils/fork_ci_messages.py +++ b/dev/breeze/src/airflow_breeze/utils/fork_ci_messages.py @@ -116,11 +116,12 @@ def fork_actions_url(fork: str) -> str: def _setup_instructions(fork: str) -> str: return ( "To get CI for this pull request:\n\n" - "1. Enable the workflow in your fork (once per fork):\n\n" + "1. Sync your fork's `main` with apache/airflow, so that it has the current version of the " + "workflow:\n\n" + f" ```bash\n gh repo sync {fork} --branch main\n ```\n\n" + "2. Enable the workflow in your fork:\n\n" f" ```bash\n gh workflow enable ci-amd.yml -R {fork}\n ```\n\n" - " If your fork's `main` is older than the workflow, sync it first with " - f"`gh repo sync {fork} --branch main`.\n" - "2. Push your branch again. Every push then runs CI in your fork.\n\n" + "3. Push your branch again. Every push then runs CI in your fork.\n\n" "`breeze ci audit` checks your whole local and fork setup and prints the command that fixes " f"anything missing. See [the contribution workflow]({CONTRIBUTING_URL})." ) @@ -195,6 +196,23 @@ def nudge_comment(*, fork: str) -> str: ) +_DISABLED_REASONS = { + "disabled_manually": "was disabled in your fork, from its Actions tab or with `gh workflow disable`", + "disabled_inactivity": "was disabled by GitHub because your fork had no activity for 60 days", + "disabled_fork": "is disabled in your fork", + "missing": "is missing from your fork, for example because its `main` was reset to an older commit", +} + + +def workflow_disabled_comment(*, fork: str, workflow_state: str | None) -> str: + reason = _DISABLED_REASONS.get(workflow_state or "missing", "is not enabled in your fork") + return ( + f"**Fork CI stopped running.** The `Tests (AMD)` workflow {reason}, so pushes to this " + "branch no longer run CI, and this pull request stays a draft until they do.\n\n" + f"{_setup_instructions(fork)}" + FOOTER + ) + + def inactivity_close_comment(*, days: int, fork: str) -> str: return ( f"Closing this pull request because CI has not run in your fork for {days} days. {WHY_FORK_CI}\n\n" 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 6b54f6c7fac..cd9edf5a44c 100644 --- a/dev/breeze/src/airflow_breeze/utils/fork_ci_reconcile.py +++ b/dev/breeze/src/airflow_breeze/utils/fork_ci_reconcile.py @@ -73,6 +73,7 @@ from airflow_breeze.utils.fork_ci_messages import ( status_description, tamper_close_comment, ungated_comment, + workflow_disabled_comment, ) CI_WORKFLOW_FILE = "ci-amd.yml" @@ -144,7 +145,7 @@ JOB_STATUSES_DEFERRED_NOTE = ( RUNNING_STATUSES = frozenset({"QUEUED", "IN_PROGRESS", "PENDING", "WAITING", "REQUESTED"}) FAILED_CONCLUSIONS = frozenset({"FAILURE", "CANCELLED", "TIMED_OUT", "STARTUP_FAILURE", "ACTION_REQUIRED"}) -SETUP_WORKFLOW_STATES = frozenset({"disabled_fork", "disabled_manually", "missing"}) +SETUP_WORKFLOW_STATES = frozenset({"disabled_fork", "disabled_inactivity", "disabled_manually", "missing"}) class ForkState(str, Enum): @@ -284,8 +285,6 @@ def observe_fork_state(fork: ForkObservation, marker: Marker | None) -> tuple[Fo return ForkState.FAILED, None return None, f"unexpected fork check suite conclusion {fork.suite_conclusion!r}" if fork.workflow_state is None: - if marker is not None and marker.state == ForkState.AWAITING_PUSH.value: - return ForkState.AWAITING_PUSH, None return None, "no fork run and the fork workflow state was not read" if fork.workflow_state in SETUP_WORKFLOW_STATES: return ForkState.SETUP_REQUIRED, None @@ -294,10 +293,6 @@ def observe_fork_state(fork: ForkObservation, marker: Marker | None) -> tuple[Fo return None, f"unexpected fork workflow state {fork.workflow_state!r}" -def needs_workflow_state(marker: Marker | None) -> bool: - return marker is None or marker.state != ForkState.AWAITING_PUSH.value - - def has_job_started(job: dict[str, Any]) -> bool: # A job that never reached a runner -- queued, or cancelled while queued -- has no steps and a # `runner_id` of null or 0, but its `status` can be "completed" and its `started_at` set. @@ -587,6 +582,17 @@ def classify( # tracked at https://github.com/apache/airflow/issues/74908 if marker and marker.state == ForkState.SETUP_REQUIRED.value and state == ForkState.AWAITING_PUSH: plan.new_comments.append(nudge_comment(fork=pr.head_repo)) + # A new comment, because the edited state comment would not notify the author that fork CI they + # had working stopped running. Like the nudge, it is not retried when posting it fails; tracked at + # https://github.com/apache/airflow/issues/74908 + if ( + marker + and marker.state not in (ForkState.SETUP_REQUIRED.value, ForkState.CLOSED.value) + and state == ForkState.SETUP_REQUIRED + ): + plan.new_comments.append( + workflow_disabled_comment(fork=pr.head_repo, workflow_state=fork.workflow_state) + ) if state in CLOSABLE_STATES and now - since >= timedelta(days=config.close_after_days): plan.state = ForkState.CLOSED @@ -1501,12 +1507,9 @@ def run_reconcile( if pr.head_status_unread: pr = read_head_status(client, repo, pr) fork = observations.get(pr.number, ForkObservation()) - if ( - pr.head_repo - and fork.suite_status is None - and fork.error is None - and needs_workflow_state(pr.marker) - ): + # Read on every run, not only until the workflow is first seen enabled: it can be + # disabled later, and a pull request waiting for a push would then wait forever. + if pr.head_repo and fork.suite_status is None and fork.error is None: fork = ForkObservation(workflow_state=fetch_workflow_state(client, pr.head_repo)) jobs_reported = None if config.job_statuses: diff --git a/dev/breeze/tests/test_ci_audit.py b/dev/breeze/tests/test_ci_audit.py index 2dbc0d8a3a5..9697a67acd0 100644 --- a/dev/breeze/tests/test_ci_audit.py +++ b/dev/breeze/tests/test_ci_audit.py @@ -260,8 +260,8 @@ def test_fork_problems(repo, message): frozenset(), (0, "active"), Role.COMMITTER, - CheckStatus.INFO, - ("gh workflow disable ci-amd.yml -R someone/airflow",), + CheckStatus.OK, + (), id="committer-enabled-is-not-a-failure", ), pytest.param( diff --git a/dev/breeze/tests/test_fork_ci_messages.py b/dev/breeze/tests/test_fork_ci_messages.py index 407065ca4a0..0c238cff949 100644 --- a/dev/breeze/tests/test_fork_ci_messages.py +++ b/dev/breeze/tests/test_fork_ci_messages.py @@ -42,6 +42,7 @@ from airflow_breeze.utils.fork_ci_messages import ( render_marker, state_comment, tamper_close_comment, + workflow_disabled_comment, ) MARKER = Marker(state="awaiting_push", sha="abc123", since=datetime(2026, 10, 15, 10, 0, tzinfo=UTC)) @@ -114,6 +115,9 @@ def test_parse_marker_malformed(body): pytest.param(nudge_comment(fork="u/airflow"), id="nudge"), pytest.param(inactivity_close_comment(days=7, fork="u/airflow"), id="inactivity"), pytest.param(tamper_close_comment(run_url="https://x";), id="tamper"), + pytest.param( + workflow_disabled_comment(fork="u/airflow", workflow_state="disabled_manually"), id="disabled" + ), ], ) def test_every_comment_ends_with_footer(text): @@ -125,9 +129,11 @@ def test_every_comment_ends_with_footer(text): [ pytest.param(state_comment(Marker("setup_required", "a", MARKER.since), fork="u/fork", run_url=None)), pytest.param(inactivity_close_comment(days=7, fork="u/fork")), + pytest.param(workflow_disabled_comment(fork="u/fork", workflow_state="disabled_manually")), ], ) def test_setup_texts_give_the_exact_command(text): + assert "gh repo sync u/fork --branch main" in text assert "gh workflow enable ci-amd.yml -R u/fork" in text diff --git a/dev/breeze/tests/test_fork_ci_reconcile.py b/dev/breeze/tests/test_fork_ci_reconcile.py index 9f3aa61f751..38977df88b4 100644 --- a/dev/breeze/tests/test_fork_ci_reconcile.py +++ b/dev/breeze/tests/test_fork_ci_reconcile.py @@ -141,12 +141,19 @@ def _classify(pr, fork=None, *, bypass_run=None, changes_ci_files=False, config= id="enabled-no-run", ), pytest.param( - ForkObservation(), + ForkObservation(workflow_state="active"), _marker("awaiting_push"), ForkState.AWAITING_PUSH, "pending", id="still-awaiting", ), + pytest.param( + ForkObservation(workflow_state="disabled_inactivity"), + None, + ForkState.SETUP_REQUIRED, + "error", + id="workflow-disabled-for-inactivity", + ), pytest.param( ForkObservation(suite_status="IN_PROGRESS", run_url=RUN_URL), None, @@ -451,6 +458,37 @@ def test_nudge_on_transition_from_setup_required(): assert "Push this branch again" in plan.new_comments[0] [email protected]( + ("marker", "workflow_state", "expected_reason"), + [ + pytest.param( + _marker("awaiting_push"), "disabled_manually", "was disabled in your fork", id="awaiting-push" + ), + pytest.param(_marker("green"), "disabled_inactivity", "no activity for 60 days", id="green"), + pytest.param(_marker("failed"), "missing", "is missing from your fork", id="failed"), + ], +) +def test_comment_when_workflow_disabled_after_it_worked(marker, workflow_state, expected_reason): + plan, _, _ = _classify(_pr(marker=marker), ForkObservation(workflow_state=workflow_state)) + + assert plan.state == ForkState.SETUP_REQUIRED + assert len(plan.new_comments) == 1 + assert "Fork CI stopped running" in plan.new_comments[0] + assert expected_reason in plan.new_comments[0] + assert "gh workflow enable ci-amd.yml -R someone/airflow" in plan.new_comments[0] + + [email protected]( + "marker", + [pytest.param(None, id="first-run"), pytest.param(_marker("setup_required"), id="still-disabled")], +) +def test_no_disabled_comment_when_workflow_never_worked(marker): + plan, _, _ = _classify(_pr(marker=marker), ForkObservation(workflow_state="disabled_fork")) + + assert plan.state == ForkState.SETUP_REQUIRED + assert plan.new_comments == [] + + @pytest.mark.parametrize( ("marker", "should_close"), [ @@ -1501,6 +1539,40 @@ def test_run_reconcile_isolates_failures_and_reports_them(client, tmp_path): assert client.set_status.call_args.args[:3] == (2, "sha2", "error") +def test_run_reconcile_rereads_workflow_state_while_awaiting_push(client, tmp_path): + marker_comment = { + "databaseId": 5, + "body": render_marker(_marker("awaiting_push", sha="sha1")), + "author": {"login": "github-actions"}, + } + client.graphql.side_effect = [ + _search_result(), + _search_result(), + _search_result(_pr_node(1, comments={"nodes": [marker_comment]})), + {"pr0": {"object": None}}, + ] + client.rest_get_pages.return_value = [{"total_count": 0, "workflow_runs": []}] + + def rest_get(path, paginate=False, items_key=None): + if path == "repos/someone/my-airflow/actions/workflows/ci-amd.yml": + return {"state": "disabled_manually"} + raise AssertionError(path) + + client.rest_get.side_effect = rest_get + + run_reconcile( + client, + CONFIG, + now=NOW, + run_url="https://run";, + output_file=tmp_path / "s.json", + workflows_dir=tmp_path, + ) + + assert client.set_status.call_args.args[:3] == (1, "sha1", "error") + assert "Fork CI stopped running" in client.create_comment.call_args.args[1] + + @pytest.mark.parametrize( ("state", "deletes"), [
