From 33b5ac65fc441c843a33f9226de2803be994c4a7 Mon Sep 17 00:00:00 2001 From: Drexuxux Date: Sat, 18 Jul 2026 05:14:54 +0300 Subject: [PATCH] fix(ci): restore fork-safe token fallback on the CI timing-report job MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #66373 swapped `GITHUB_TOKEN` → `AUTOFIX_BOT_PAT` across the workflows; that PAT is empty on fork PRs (forks get no repo secrets). 1e01a4bbe restored the `|| github.token` fallback for detect-changes / lint / supply-chain, but the `ci-timings` job was missed — its "Collect timings" step still passed a bare `GITHUB_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT }}`. So on every fork PR the timing-report step received an empty `GITHUB_TOKEN` and `timings_report.py` crashed at `expect_env("GITHUB_TOKEN")` with `ValueError: missing environment variable GITHUB_TOKEN`, reddening the PR — even though the job's own contract is "a missing report must never redden the PR" (it already exits 0 on `TimingsUnavailable`). Two-layer fix: - ci.yml: add the `|| github.token` fallback so the observability job gets the run's read-only token on fork PRs (mirrors the detect-changes fallback), so timings are actually collected. - timings_report.py: treat an absent/empty `GITHUB_TOKEN` as `TimingsUnavailable` and route it through the existing graceful degraded path (placeholder report + summary, exit 0) instead of a hard crash — keeping the "never reddens the PR" invariant true regardless of how the token is wired. Tests: `tests/ci/test_timings_report.py` — an unset and an empty `GITHUB_TOKEN` both exit 0 with a placeholder report and no cached JSON; before the fix the run raised `ValueError`. --- .github/workflows/ci.yml | 6 ++- scripts/ci/timings_report.py | 11 ++++- tests/ci/test_timings_report.py | 76 +++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 2 deletions(-) create mode 100644 tests/ci/test_timings_report.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 72e34a06d64c..1f83647fe433 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -226,7 +226,11 @@ jobs: - name: Collect timings and generate report env: - GITHUB_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT }} + # Forks get no repo secrets (AUTOFIX_BOT_PAT is empty); fall back to + # the run's read-only github.token so this observability job doesn't + # crash on a missing GITHUB_TOKEN and redden every fork PR. Mirrors + # the detect-changes fallback above. + GITHUB_TOKEN: ${{ secrets.AUTOFIX_BOT_PAT || github.token }} run: | python3 scripts/ci/timings_report.py \ --baseline ci-timings-baseline.json \ diff --git a/scripts/ci/timings_report.py b/scripts/ci/timings_report.py index 9df3fc519aac..c308c9dba536 100644 --- a/scripts/ci/timings_report.py +++ b/scripts/ci/timings_report.py @@ -923,11 +923,20 @@ def main(): with open(args.from_json, encoding="utf-8") as f: timings = json.load(f) else: - token = expect_env("GITHUB_TOKEN") + # Fork PRs get no repo secrets, so the workflow's ``AUTOFIX_BOT_PAT`` + # is empty and ``GITHUB_TOKEN`` can arrive unset. This is an + # observability job — degrade gracefully (the handler below) rather + # than reddening the PR with a hard "missing env var" crash before the + # collect even runs. (The workflow also falls back to ``github.token``; + # this keeps the invariant "a missing report never reddens the PR" true + # regardless of how the token is wired.) + token = os.environ.get("GITHUB_TOKEN") or "" repo = expect_env("GITHUB_REPOSITORY") run_id = expect_env("GITHUB_RUN_ID") head_sha = expect_env("GITHUB_SHA") try: + if not token: + raise TimingsUnavailable("GITHUB_TOKEN not available (fork PR?)") timings = collect_timings(token, repo, run_id, head_sha) except TimingsUnavailable as e: # Observability job: a missing report must never redden the PR. diff --git a/tests/ci/test_timings_report.py b/tests/ci/test_timings_report.py new file mode 100644 index 000000000000..87d3d9751084 --- /dev/null +++ b/tests/ci/test_timings_report.py @@ -0,0 +1,76 @@ +"""Tests for scripts/ci/timings_report.py. + +The CI timing report is an *observability* job: its own contract is that "a +missing report must never redden the PR". A fork PR gets no repo secrets, so +the workflow's ``AUTOFIX_BOT_PAT`` is empty and the job can receive an unset +``GITHUB_TOKEN``. The script must then degrade gracefully (exit 0 with a +placeholder report/summary), not crash with "missing environment variable +GITHUB_TOKEN" and fail every fork PR. +""" + +from __future__ import annotations + +import importlib.util +import sys +from pathlib import Path + +import pytest + +_PATH = Path(__file__).resolve().parents[2] / "scripts" / "ci" / "timings_report.py" +_spec = importlib.util.spec_from_file_location("timings_report", _PATH) +if _spec is None or _spec.loader is None: + raise ImportError("Failed to load timings_report.py") +_mod = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(_mod) + + +def _run_main(monkeypatch, tmp_path, *, token): + summary = tmp_path / "summary.md" + output = tmp_path / "report.html" + json_out = tmp_path / "timings.json" + if token is None: + monkeypatch.delenv("GITHUB_TOKEN", raising=False) + else: + monkeypatch.setenv("GITHUB_TOKEN", token) + monkeypatch.setenv("GITHUB_REPOSITORY", "org/repo") + monkeypatch.setenv("GITHUB_RUN_ID", "123") + monkeypatch.setenv("GITHUB_SHA", "deadbeef") + monkeypatch.setattr( + sys, + "argv", + [ + "timings_report.py", + "--summary-out", str(summary), + "--output", str(output), + "--json-out", str(json_out), + ], + ) + return summary, output, json_out + + +def test_missing_github_token_degrades_instead_of_crashing(monkeypatch, tmp_path): + """Empty GITHUB_TOKEN (fork PR) must exit 0 with a placeholder report, not + raise ``ValueError: missing environment variable GITHUB_TOKEN``.""" + summary, output, json_out = _run_main(monkeypatch, tmp_path, token=None) + + with pytest.raises(SystemExit) as exc_info: + _mod.main() + + assert exc_info.value.code == 0 + # A placeholder HTML report + a degraded summary are emitted... + assert output.exists() + assert "unavailable" in summary.read_text(encoding="utf-8").lower() + # ...but NO JSON, so an empty run can never be cached as the main baseline. + assert not json_out.exists() + + +def test_empty_string_github_token_also_degrades(monkeypatch, tmp_path): + """A present-but-empty GITHUB_TOKEN (``AUTOFIX_BOT_PAT`` resolving to '') + is treated the same as unset.""" + summary, output, _ = _run_main(monkeypatch, tmp_path, token="") + + with pytest.raises(SystemExit) as exc_info: + _mod.main() + + assert exc_info.value.code == 0 + assert output.exists()