Skip to content

Commit daa2a86

Browse files
authored
Delete the project CI runs that only gate a pull request to fork CI (#75588)
* Delete the project CI runs that only gate a pull request to fork CI A gated Tests (AMD) run lists every project CI job as a skipped check on the pull request, next to the fork's result. GitHub cannot delete or update single Actions check runs, so the fork CI monitor deletes the whole run once only the gate ran in it. Generated-by: Claude Opus 5 * Keep gated runs whose commit changes workflows as tampering evidence Generated-by: Claude Opus 5 * Tell maintainers to close and reopen a gated pull request to start project CI The fork CI monitor now deletes gated runs, so there is no run left to re-run after adding use project ci. Generated-by: Claude Opus 5 * Keep re-run gated runs, whose earlier attempts may have run project CI Generated-by: Claude Opus 5
1 parent 8e3a911 commit daa2a86

8 files changed

Lines changed: 232 additions & 19 deletions

File tree

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@
1818
---
1919
# Drives pull requests whose CI runs in contributor forks (AIP-120): labels, draft state, the
2020
# state comment, the `Fork CI / Tests (AMD)` commit status and closing. It is the only writer for
21-
# those pull requests and never checks out or runs pull request code. See
21+
# those pull requests, and deletes their project CI runs that only skipped every job. It never checks
22+
# out or runs pull request code. See
2223
# dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md.
2324
name: "Fork CI monitor"
2425
on: # yamllint disable-line rule:truthy
@@ -31,8 +32,9 @@ on: # yamllint disable-line rule:truthy
3132
type: boolean
3233
default: false
3334
permissions:
34-
# Listing the runs and jobs of the gate tampering check; unlisted scopes are none.
35-
actions: read
35+
# Listing the runs and jobs of the gate tampering check, and deleting the runs that only gated a
36+
# pull request to fork CI; unlisted scopes are none.
37+
actions: write
3638
contents: read
3739
# Labels, draft state, comments and closing of gated pull requests.
3840
pull-requests: write

‎contributing-docs/25_maintainer_pr_triage.md‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -267,9 +267,10 @@ Everything else the skill does (quality criteria, pings, suspicious-change flags
267267
usual. The `Fork CI / Tests (AMD)` commit status shows the fork CI state and links to the run in
268268
the fork. To move a single pull request back to project CI, add `use project ci`; the reconciler
269269
releases it on its next run; removing `fork ci required` by hand is not enough, the next run
270-
labels a gated pull request again. Adding the label does not start project CI by itself: re-run
271-
the pull request's latest `Tests (AMD)` run (`gh run rerun <run-id> -R apache/airflow`, or
272-
**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
270+
labels a gated pull request again. Adding the label does not start project CI by itself, and the
271+
reconciler deletes the gated `Tests (AMD)` runs, so there is usually nothing to re-run: close and
272+
reopen the pull request (`gh pr close <n> -R apache/airflow && gh pr reopen <n> -R apache/airflow`),
273+
or ask the author to push. Pull requests that change CI-defining files stay in draft even
273274
when the fork run is green and are labelled `needs project ci`, for a maintainer to review and run
274275
project CI with `use project ci`. When the fork run is green and trusted, the reconciler also posts
275276
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‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -221,14 +221,16 @@ Consequences for the other components:
221221

222222
- The gate job reads the pull request's labels live from the API (one read-only call) rather
223223
than from the event payload, so labelling a pull request and pressing "Re-run all jobs"
224-
switches it without a new push.
224+
switches it without a new push. The reconciler deletes gated runs (see "Gated project CI runs
225+
are deleted"), so for a gated pull request closing and reopening it takes the place of the
226+
re-run.
225227
- Discovery in the reconciler also picks up open pull requests carrying `use fork ci`
226228
without `fork ci required`, regardless of `enabled`.
227229
- A pull request carrying both `fork ci required` and `use project ci` is *released*: the
228230
reconciler removes `fork ci required`, undrafts it if the bot drafted it, edits the state
229231
comment to say project CI is in use, and posts a `success` status with the description
230-
"Project CI in use (label `use project ci`)". Project CI then runs on the next push or
231-
re-run.
232+
"Project CI in use (label `use project ci`)". Project CI then runs on the next push, or when
233+
a maintainer closes and reopens the pull request.
232234
- Both override labels on one pull request resolve to project CI and are reported to Slack as
233235
a conflict.
234236

@@ -690,6 +692,20 @@ is the source the ruleset accepts.
690692
job must not report success there; it reports under another name when `build-info` is
691693
skipped, and the reconciler's status is the only `CI passed` the pull request has.
692694

695+
### Gated project CI runs are deleted
696+
697+
A gated run of `ci-amd.yml` lists every project CI job on the pull request as a skipped check,
698+
next to the fork's result in `Fork CI / Tests (AMD)`. GitHub cannot delete or update single check
699+
runs created by GitHub Actions, so the reconciler deletes the whole run once it completed: the
700+
`ci-amd.yml` `pull_request` runs of a gated pull request's branch from the tamper window in which
701+
the gate, `Check if the run is needed` and `CI passed (project CI not run)` succeeded and every
702+
other job was skipped. A run in which anything else happened, including a run the tampering check
703+
counts as project CI work, is kept, and so is a run whose commit changes `.github/workflows/` or
704+
whose change set is too large to check: such a commit could do project CI work under the gate's
705+
name within its time limit, and its run is the evidence. A re-run is kept too: deleting a run
706+
deletes every attempt, and an earlier attempt may have run project CI. Nothing is deleted for a
707+
pull request in the `gate_bypassed` state. A dry run only reports the deletions.
708+
693709
### Far behind the base branch
694710

695711
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.
745761
`.github/workflows/fork-ci-monitor.yml`:
746762

747763
- `on: schedule: cron '*/15 * * * *'` and `workflow_dispatch` with a `dry-run` input.
748-
- `permissions: actions: read, contents: read, pull-requests: write, statuses: write` with a
764+
- `permissions: actions: write, contents: read, pull-requests: write, statuses: write` with a
749765
justification comment. Comments and labels on pull requests are covered by
750766
`pull-requests: write`; the commit status needs `statuses: write`; the gate tampering check
751-
lists runs and their jobs, which needs `actions: read` because every scope a `permissions`
752-
block leaves out is `none`.
767+
lists runs and their jobs, and the reconciler deletes gated runs, which needs `actions: write`
768+
because every scope a `permissions` block leaves out is `none`.
753769
- `runs-on: ubuntu-latest` (the breeze setup action frees disk space with `sudo` and `docker`,
754770
which `ubuntu-slim` does not provide), `timeout-minutes: 20`, `concurrency: fork-ci-monitor`
755771
with `cancel-in-progress: false` so runs never overlap.

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,13 @@ def close(self, pr: int) -> None:
187187
{"state": "closed"},
188188
)
189189

190+
def delete_run(self, pr: int, run_id: int) -> None:
191+
self._mutate(
192+
f"#{pr}: delete workflow run {run_id}",
193+
["api", "-X", "DELETE", f"repos/{self.repo}/actions/runs/{run_id}"],
194+
None,
195+
)
196+
190197
def set_status(
191198
self,
192199
pr: int,

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,8 @@ def released_comment(marker: Marker) -> str:
166166
return (
167167
f"{render_marker(marker)}\n\n"
168168
"**Project CI in use.** A maintainer added the `use project ci` label, so CI for this pull "
169-
"request runs in apache/airflow. It starts on the next push or re-run." + FOOTER
169+
"request runs in apache/airflow. It starts on the next push, or when a maintainer closes and "
170+
"reopens this pull request." + FOOTER
170171
)
171172

172173

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

Lines changed: 63 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@
8888
frozenset({"CI passed (project CI not run)", "CI passed (run cancelled)", "CI passed"}),
8989
)
9090
GATED_RUN_JOB_NAMES = frozenset[str]().union(*GATED_RUN_JOBS)
91+
CHECK_RUN_NEEDED_JOB_NAME = "Check if the run is needed"
92+
PROJECT_CI_NOT_RUN_JOB_NAME = "CI passed (project CI not run)"
9193
BOT_LOGINS = frozenset({"github-actions", "github-actions[bot]"})
9294
DISCOVERY_WINDOW = timedelta(hours=24)
9395
PASS_TWO_PAGE_SIZE = 25
@@ -156,6 +158,7 @@ class WorkflowRun:
156158
updated_at: datetime
157159
status: str
158160
event: str = "pull_request"
161+
run_attempt: int = 1
159162

160163
@property
161164
def workflow_file(self) -> str:
@@ -310,6 +313,25 @@ def jobs_show_project_ci(jobs: Iterable[dict[str, Any]], now: datetime) -> bool:
310313
)
311314

312315

316+
def is_gated_skip_run(jobs: Iterable[dict[str, Any]], now: datetime) -> bool:
317+
"""
318+
Whether a completed ci-amd.yml run is only the gate leaving the pull request to fork CI.
319+
320+
Such a run lists every project CI job as a skipped check on the pull request, next to the
321+
fork's result in the `Fork CI / Tests (AMD)` status. The gate and the check-run-needed job
322+
succeeded, CI passed reported as not run, and every other job was skipped.
323+
"""
324+
jobs = list(jobs)
325+
if jobs_show_project_ci(jobs, now):
326+
return False
327+
succeeded = {GATE_JOB_NAME, CHECK_RUN_NEEDED_JOB_NAME, PROJECT_CI_NOT_RUN_JOB_NAME}
328+
if sorted(job["name"] for job in jobs if job["name"] in succeeded) != sorted(succeeded):
329+
return False
330+
return all(
331+
job.get("conclusion") == ("success" if job["name"] in succeeded else "skipped") for job in jobs
332+
)
333+
334+
313335
def touches(files: Iterable[str], prefixes: tuple[str, ...]) -> bool:
314336
return any(path.startswith(prefixes) for path in files)
315337

@@ -920,6 +942,7 @@ def parse_workflow_run(run: dict[str, Any]) -> WorkflowRun:
920942
updated_at=parse_github_datetime(run["updated_at"]), # type: ignore[arg-type]
921943
status=run["status"],
922944
event=run["event"],
945+
run_attempt=run.get("run_attempt") or 1,
923946
)
924947

925948

@@ -988,6 +1011,16 @@ def __init__(
9881011
)
9891012
self._did_project_ci: dict[int, bool] = {}
9901013
self._changes_workflows: dict[tuple[str, str], bool | None] = {}
1014+
self._jobs: dict[int, list[dict[str, Any]]] = {}
1015+
1016+
def _get_jobs(self, run: WorkflowRun) -> list[dict[str, Any]]:
1017+
if run.id not in self._jobs:
1018+
self._jobs[run.id] = self._client.rest_get(
1019+
f"repos/{self._repo}/actions/runs/{run.id}/jobs?filter=latest&per_page=100",
1020+
paginate=True,
1021+
items_key="jobs",
1022+
)
1023+
return self._jobs[run.id]
9911024

9921025
def _did_run_project_ci(self, run: WorkflowRun) -> bool:
9931026
if run.id not in self._did_project_ci:
@@ -998,15 +1031,33 @@ def _did_run_project_ci(self, run: WorkflowRun) -> bool:
9981031
elif run.workflow_file != CI_WORKFLOW_FILE or not run.is_long_running(self._now):
9991032
result = False
10001033
else:
1001-
jobs = self._client.rest_get(
1002-
f"repos/{self._repo}/actions/runs/{run.id}/jobs?filter=latest&per_page=100",
1003-
paginate=True,
1004-
items_key="jobs",
1005-
)
1006-
result = jobs_show_project_ci(jobs, self._now)
1034+
result = jobs_show_project_ci(self._get_jobs(run), self._now)
10071035
self._did_project_ci[run.id] = result
10081036
return self._did_project_ci[run.id]
10091037

1038+
def find_gated_runs(self, pr: PullRequestSnapshot) -> list[WorkflowRun]:
1039+
"""
1040+
Completed ci-amd.yml pull_request runs of the pull request's branch that only gated it.
1041+
1042+
A commit that changes workflows could do project CI work under the gate's name within its
1043+
time limit, so its runs are kept as evidence, as are runs whose change set is too large
1044+
to check. The jobs read are those of the latest attempt, and deleting a run deletes every
1045+
attempt, so a re-run is kept: an earlier attempt may have run project CI.
1046+
"""
1047+
if not pr.head_repo:
1048+
return []
1049+
return [
1050+
run
1051+
for run in self._runs_by_branch.get((pr.head_repo.casefold(), pr.head_ref), [])
1052+
if run.workflow_file == CI_WORKFLOW_FILE
1053+
and run.event == "pull_request"
1054+
and run.status == "completed"
1055+
and run.run_attempt == 1
1056+
and not self._did_run_project_ci(run)
1057+
and is_gated_skip_run(self._get_jobs(run), self._now)
1058+
and self._commit_changes_workflows(run, pr.base_ref) is False
1059+
]
1060+
10101061
def _commit_changes_workflows(self, run: WorkflowRun, base_ref: str) -> bool | None:
10111062
"""Whether the run's commit changes workflows against the base; None when not knowable."""
10121063
key = (run.head_sha, base_ref)
@@ -1270,6 +1321,12 @@ def run_reconcile(
12701321
count_behind=functools.partial(count_commits_behind, client, repo, pr),
12711322
)
12721323
apply_plan(client, pr, plan)
1324+
# The fork's result is in the `Fork CI / Tests (AMD)` status; the gated run's skipped
1325+
# checks only clutter the pull request. Runs with any project CI work stay for the
1326+
# tampering check.
1327+
if plan.state != ForkState.GATE_BYPASSED:
1328+
for run in bypass_finder.find_gated_runs(pr):
1329+
client.delete_run(pr.number, run.id)
12731330
unknown.extend((pr.number, reason) for reason in plan.unknown)
12741331
attention.extend((pr.number, item) for item in plan.attention)
12751332
except Exception as e:

‎dev/breeze/tests/test_fork_ci_github.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,10 +127,20 @@ def test_dry_run_does_not_mutate(mock_run):
127127

128128
client.close(1)
129129
client.remove_label(1, "fork ci required")
130+
client.delete_run(1, 7)
130131

131132
mock_run.assert_not_called()
132133

133134

135+
@mock.patch("airflow_breeze.utils.fork_ci_github.subprocess.run", autospec=True)
136+
def test_delete_run(mock_run):
137+
mock_run.return_value = _completed()
138+
139+
GitHubClient().delete_run(1, 7)
140+
141+
assert mock_run.call_args.args[0] == ["gh", "api", "-X", "DELETE", "repos/apache/airflow/actions/runs/7"]
142+
143+
134144
@mock.patch("airflow_breeze.utils.fork_ci_github.subprocess.run", autospec=True)
135145
def test_remove_label_quotes_the_name(mock_run):
136146
mock_run.return_value = _completed()

0 commit comments

Comments
 (0)