Delete the project CI runs that only gate a pull request to fork CI

.github/workflows/fork-ci-monitor.yml

@@ -18,7 +18,8 @@
 ---
 # Drives pull requests whose CI runs in contributor forks (AIP-120): labels, draft state, the
 # state comment, the `Fork CI / Tests (AMD)` commit status and closing. It is the only writer for
-# those pull requests and never checks out or runs pull request code. See
+# those pull requests, and deletes their project CI runs that only skipped every job. It never checks
+# out or runs pull request code. See
 # dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md.
 name: "Fork CI monitor"
 on:  # yamllint disable-line rule:truthy
@@ -31,8 +32,9 @@ on:  # yamllint disable-line rule:truthy
         type: boolean
         default: false
 permissions:
-  # Listing the runs and jobs of the gate tampering check; unlisted scopes are none.
-  actions: read
+  # Listing the runs and jobs of the gate tampering check, and deleting the runs that only gated a
+  # pull request to fork CI; unlisted scopes are none.
+  actions: write
   contents: read
   # Labels, draft state, comments and closing of gated pull requests.
   pull-requests: write

contributing-docs/25_maintainer_pr_triage.md

@@ -267,9 +267,10 @@ Everything else the skill does (quality criteria, pings, suspicious-change flags
 usual. The `Fork CI / Tests (AMD)` commit status shows the fork CI state and links to the run in
 the fork. To move a single pull request back to project CI, add `use project ci`; the reconciler
 releases it on its next run; removing `fork ci required` by hand is not enough, the next run
-labels a gated pull request again. Adding the label does not start project CI by itself: re-run
-the pull request's latest `Tests (AMD)` run (`gh run rerun <run-id> -R apache/airflow`, or
-**Re-run all jobs** on the run's page), or ask the author to push. Pull requests that change CI-defining files stay in draft even
+labels a gated pull request again. Adding the label does not start project CI by itself, and the
+reconciler deletes the gated `Tests (AMD)` runs, so there is usually nothing to re-run: close and
+reopen the pull request (`gh pr close <n> -R apache/airflow && gh pr reopen <n> -R apache/airflow`),
+or ask the author to push. Pull requests that change CI-defining files stay in draft even
 when the fork run is green and are labelled `needs project ci`, for a maintainer to review and run
 project CI with `use project ci`. When the fork run is green and trusted, the reconciler also posts
 the `CI passed` status the merge ruleset requires, so these pull requests can be merged or

dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md

@@ -221,14 +221,16 @@ Consequences for the other components:
 
 - The gate job reads the pull request's labels live from the API (one read-only call) rather
   than from the event payload, so labelling a pull request and pressing "Re-run all jobs"
-  switches it without a new push.
+  switches it without a new push. The reconciler deletes gated runs (see "Gated project CI runs
+  are deleted"), so for a gated pull request closing and reopening it takes the place of the
+  re-run.
 - Discovery in the reconciler also picks up open pull requests carrying `use fork ci`
   without `fork ci required`, regardless of `enabled`.
 - A pull request carrying both `fork ci required` and `use project ci` is *released*: the
   reconciler removes `fork ci required`, undrafts it if the bot drafted it, edits the state
   comment to say project CI is in use, and posts a `success` status with the description
-  "Project CI in use (label `use project ci`)". Project CI then runs on the next push or
-  re-run.
+  "Project CI in use (label `use project ci`)". Project CI then runs on the next push, or when
+  a maintainer closes and reopens the pull request.
 - Both override labels on one pull request resolve to project CI and are reported to Slack as
   a conflict.
 
@@ -690,6 +692,20 @@ is the source the ruleset accepts.
   job must not report success there; it reports under another name when `build-info` is
   skipped, and the reconciler's status is the only `CI passed` the pull request has.
 
+### Gated project CI runs are deleted
+
+A gated run of `ci-amd.yml` lists every project CI job on the pull request as a skipped check,
+next to the fork's result in `Fork CI / Tests (AMD)`. GitHub cannot delete or update single check
+runs created by GitHub Actions, so the reconciler deletes the whole run once it completed: the
+`ci-amd.yml` `pull_request` runs of a gated pull request's branch from the tamper window in which
+the gate, `Check if the run is needed` and `CI passed (project CI not run)` succeeded and every
+other job was skipped. A run in which anything else happened, including a run the tampering check
+counts as project CI work, is kept, and so is a run whose commit changes `.github/workflows/` or
+whose change set is too large to check: such a commit could do project CI work under the gate's
+name within its time limit, and its run is the evidence. A re-run is kept too: deleting a run
+deletes every attempt, and an earlier attempt may have run project CI. Nothing is deleted for a
+pull request in the `gate_bypassed` state. A dry run only reports the deletions.
+
 ### Far behind the base branch
 
 A fork run tests the pushed commit as it is, while project CI tests it merged with the base
@@ -745,11 +761,11 @@ budget stays observable.
 `.github/workflows/fork-ci-monitor.yml`:
 
 - `on: schedule: cron '*/15 * * * *'` and `workflow_dispatch` with a `dry-run` input.
-- `permissions: actions: read, contents: read, pull-requests: write, statuses: write` with a
+- `permissions: actions: write, contents: read, pull-requests: write, statuses: write` with a
   justification comment. Comments and labels on pull requests are covered by
   `pull-requests: write`; the commit status needs `statuses: write`; the gate tampering check
-  lists runs and their jobs, which needs `actions: read` because every scope a `permissions`
-  block leaves out is `none`.
+  lists runs and their jobs, and the reconciler deletes gated runs, which needs `actions: write`
+  because every scope a `permissions` block leaves out is `none`.
 - `runs-on: ubuntu-latest` (the breeze setup action frees disk space with `sudo` and `docker`,
   which `ubuntu-slim` does not provide), `timeout-minutes: 20`, `concurrency: fork-ci-monitor`
   with `cancel-in-progress: false` so runs never overlap.

dev/breeze/src/airflow_breeze/utils/fork_ci_github.py

@@ -187,6 +187,13 @@ def close(self, pr: int) -> None:
             {"state": "closed"},
         )
 
+    def delete_run(self, pr: int, run_id: int) -> None:
+        self._mutate(
+            f"#{pr}: delete workflow run {run_id}",
+            ["api", "-X", "DELETE", f"repos/{self.repo}/actions/runs/{run_id}"],
+            None,
+        )
+
     def set_status(
         self,
         pr: int,

dev/breeze/src/airflow_breeze/utils/fork_ci_messages.py

@@ -166,7 +166,8 @@ def released_comment(marker: Marker) -> str:
     return (
         f"{render_marker(marker)}\n\n"
         "**Project CI in use.** A maintainer added the `use project ci` label, so CI for this pull "
-        "request runs in apache/airflow. It starts on the next push or re-run." + FOOTER
+        "request runs in apache/airflow. It starts on the next push, or when a maintainer closes and "
+        "reopens this pull request." + FOOTER
     )
 
 

dev/breeze/src/airflow_breeze/utils/fork_ci_reconcile.py

@@ -88,6 +88,8 @@
     frozenset({"CI passed (project CI not run)", "CI passed (run cancelled)", "CI passed"}),
 )
 GATED_RUN_JOB_NAMES = frozenset[str]().union(*GATED_RUN_JOBS)
+CHECK_RUN_NEEDED_JOB_NAME = "Check if the run is needed"
+PROJECT_CI_NOT_RUN_JOB_NAME = "CI passed (project CI not run)"
 BOT_LOGINS = frozenset({"github-actions", "github-actions[bot]"})
 DISCOVERY_WINDOW = timedelta(hours=24)
 PASS_TWO_PAGE_SIZE = 25
@@ -156,6 +158,7 @@ class WorkflowRun:
     updated_at: datetime
     status: str
     event: str = "pull_request"
+    run_attempt: int = 1
 
     @property
     def workflow_file(self) -> str:
@@ -310,6 +313,25 @@ def jobs_show_project_ci(jobs: Iterable[dict[str, Any]], now: datetime) -> bool:
     )
 
 
+def is_gated_skip_run(jobs: Iterable[dict[str, Any]], now: datetime) -> bool:
+    """
+    Whether a completed ci-amd.yml run is only the gate leaving the pull request to fork CI.
+
+    Such a run lists every project CI job as a skipped check on the pull request, next to the
+    fork's result in the `Fork CI / Tests (AMD)` status. The gate and the check-run-needed job
+    succeeded, CI passed reported as not run, and every other job was skipped.
+    """
+    jobs = list(jobs)
+    if jobs_show_project_ci(jobs, now):
+        return False
+    succeeded = {GATE_JOB_NAME, CHECK_RUN_NEEDED_JOB_NAME, PROJECT_CI_NOT_RUN_JOB_NAME}
+    if sorted(job["name"] for job in jobs if job["name"] in succeeded) != sorted(succeeded):
+        return False
+    return all(
+        job.get("conclusion") == ("success" if job["name"] in succeeded else "skipped") for job in jobs
+    )
+
+
 def touches(files: Iterable[str], prefixes: tuple[str, ...]) -> bool:
     return any(path.startswith(prefixes) for path in files)
 
@@ -920,6 +942,7 @@ def parse_workflow_run(run: dict[str, Any]) -> WorkflowRun:
         updated_at=parse_github_datetime(run["updated_at"]),  # type: ignore[arg-type]
         status=run["status"],
         event=run["event"],
+        run_attempt=run.get("run_attempt") or 1,
     )
 
 
@@ -988,6 +1011,16 @@ def __init__(
         )
         self._did_project_ci: dict[int, bool] = {}
         self._changes_workflows: dict[tuple[str, str], bool | None] = {}
+        self._jobs: dict[int, list[dict[str, Any]]] = {}
+
+    def _get_jobs(self, run: WorkflowRun) -> list[dict[str, Any]]:
+        if run.id not in self._jobs:
+            self._jobs[run.id] = self._client.rest_get(
+                f"repos/{self._repo}/actions/runs/{run.id}/jobs?filter=latest&per_page=100",
+                paginate=True,
+                items_key="jobs",
+            )
+        return self._jobs[run.id]
 
     def _did_run_project_ci(self, run: WorkflowRun) -> bool:
         if run.id not in self._did_project_ci:
@@ -998,15 +1031,33 @@ def _did_run_project_ci(self, run: WorkflowRun) -> bool:
             elif run.workflow_file != CI_WORKFLOW_FILE or not run.is_long_running(self._now):
                 result = False
             else:
-                jobs = self._client.rest_get(
-                    f"repos/{self._repo}/actions/runs/{run.id}/jobs?filter=latest&per_page=100",
-                    paginate=True,
-                    items_key="jobs",
-                )
-                result = jobs_show_project_ci(jobs, self._now)
+                result = jobs_show_project_ci(self._get_jobs(run), self._now)
             self._did_project_ci[run.id] = result
         return self._did_project_ci[run.id]
 
+    def find_gated_runs(self, pr: PullRequestSnapshot) -> list[WorkflowRun]:
+        """
+        Completed ci-amd.yml pull_request runs of the pull request's branch that only gated it.
+
+        A commit that changes workflows could do project CI work under the gate's name within its
+        time limit, so its runs are kept as evidence, as are runs whose change set is too large
+        to check. The jobs read are those of the latest attempt, and deleting a run deletes every
+        attempt, so a re-run is kept: an earlier attempt may have run project CI.
+        """
+        if not pr.head_repo:
+            return []
+        return [
+            run
+            for run in self._runs_by_branch.get((pr.head_repo.casefold(), pr.head_ref), [])
+            if run.workflow_file == CI_WORKFLOW_FILE
+            and run.event == "pull_request"
+            and run.status == "completed"
+            and run.run_attempt == 1
+            and not self._did_run_project_ci(run)
+            and is_gated_skip_run(self._get_jobs(run), self._now)
+            and self._commit_changes_workflows(run, pr.base_ref) is False
+        ]
+
     def _commit_changes_workflows(self, run: WorkflowRun, base_ref: str) -> bool | None:
         """Whether the run's commit changes workflows against the base; None when not knowable."""
         key = (run.head_sha, base_ref)
@@ -1270,6 +1321,12 @@ def run_reconcile(
                 count_behind=functools.partial(count_commits_behind, client, repo, pr),
             )
             apply_plan(client, pr, plan)
+            # The fork's result is in the `Fork CI / Tests (AMD)` status; the gated run's skipped
+            # checks only clutter the pull request. Runs with any project CI work stay for the
+            # tampering check.
+            if plan.state != ForkState.GATE_BYPASSED:
+                for run in bypass_finder.find_gated_runs(pr):
+                    client.delete_run(pr.number, run.id)
             unknown.extend((pr.number, reason) for reason in plan.unknown)
             attention.extend((pr.number, item) for item in plan.attention)
         except Exception as e:

dev/breeze/tests/test_fork_ci_github.py

@@ -127,10 +127,20 @@ def test_dry_run_does_not_mutate(mock_run):
 
     client.close(1)
     client.remove_label(1, "fork ci required")
+    client.delete_run(1, 7)
 
     mock_run.assert_not_called()
 
 
+@mock.patch("airflow_breeze.utils.fork_ci_github.subprocess.run", autospec=True)
+def test_delete_run(mock_run):
+    mock_run.return_value = _completed()
+
+    GitHubClient().delete_run(1, 7)
+
+    assert mock_run.call_args.args[0] == ["gh", "api", "-X", "DELETE", "repos/apache/airflow/actions/runs/7"]
+
+
 @mock.patch("airflow_breeze.utils.fork_ci_github.subprocess.run", autospec=True)
 def test_remove_label_quotes_the_name(mock_run):
     mock_run.return_value = _completed()