diff --git a/.github/workflows/ci-cache-receipts.yml b/.github/workflows/ci-cache-receipts.yml index 05385e947d32..8ebd3005f985 100644 --- a/.github/workflows/ci-cache-receipts.yml +++ b/.github/workflows/ci-cache-receipts.yml @@ -30,6 +30,10 @@ on: permissions: contents: read +concurrency: + group: ci-cache-receipts-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: receipt-contract: runs-on: ${{ vars.LINUX_RUNNER || 'blacksmith-4vcpu-ubuntu-2404' }} diff --git a/.github/workflows/ci-guards.yml b/.github/workflows/ci-guards.yml index f1fa9ac66108..8f9de9db9d3f 100644 --- a/.github/workflows/ci-guards.yml +++ b/.github/workflows/ci-guards.yml @@ -291,6 +291,10 @@ jobs: if: ${{ matrix.group == 'ci' }} run: python3 tests/test_ci_workflow_guards_are_wired.py + - name: Validate required checks are bounded + if: ${{ matrix.group == 'ci' }} + run: python3 tests/test_ci_required_checks_are_bounded.py + - name: Validate triage radar scoring if: ${{ matrix.group == 'ci' }} run: python3 tests/test_triage_radar.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 04d233125467..7f210d5b13c4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -951,6 +951,10 @@ jobs: - tests if: ${{ always() }} runs-on: ${{ vars.LINUX_RUNNER || 'blacksmith-4vcpu-ubuntu-2404' }} + # One step that reads `needs` and decides. Its siblings are bounded at 5 + # to 20 minutes; this one was inheriting the six-hour default, as the + # required check every pull request waits on. + timeout-minutes: 5 steps: - name: Check routed CI jobs env: diff --git a/.github/workflows/cmux-cloud-cli.yml b/.github/workflows/cmux-cloud-cli.yml index aeffa94d5421..fcaf74fb8278 100644 --- a/.github/workflows/cmux-cloud-cli.yml +++ b/.github/workflows/cmux-cloud-cli.yml @@ -14,6 +14,10 @@ on: workflow_dispatch: permissions: contents: read +concurrency: + group: cmux-cloud-cli-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: test: runs-on: blacksmith-4vcpu-ubuntu-2404 diff --git a/.github/workflows/indexnow-tests.yml b/.github/workflows/indexnow-tests.yml index b812bf8f296a..6f7c9ed7b475 100644 --- a/.github/workflows/indexnow-tests.yml +++ b/.github/workflows/indexnow-tests.yml @@ -13,6 +13,10 @@ on: permissions: contents: read +concurrency: + group: indexnow-tests-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: notification-tests: runs-on: ${{ vars.LINUX_RUNNER || 'blacksmith-4vcpu-ubuntu-2404' }} diff --git a/.github/workflows/localization-catalog.yml b/.github/workflows/localization-catalog.yml index 44b4cc6f77be..86c16e870573 100644 --- a/.github/workflows/localization-catalog.yml +++ b/.github/workflows/localization-catalog.yml @@ -17,6 +17,10 @@ on: permissions: contents: read +concurrency: + group: localization-catalog-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: catalog-structure: runs-on: ${{ vars.LINUX_RUNNER || 'blacksmith-4vcpu-ubuntu-2404' }} diff --git a/.github/workflows/r2-upload-tests.yml b/.github/workflows/r2-upload-tests.yml index 20a63becaeac..cc823c420115 100644 --- a/.github/workflows/r2-upload-tests.yml +++ b/.github/workflows/r2-upload-tests.yml @@ -17,6 +17,10 @@ on: permissions: contents: read +concurrency: + group: r2-upload-tests-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: r2-upload-tests: runs-on: ${{ vars.LINUX_RUNNER || 'blacksmith-4vcpu-ubuntu-2404' }} diff --git a/tests/test-execution.toml b/tests/test-execution.toml index 4eba6badc3ca..9b48d598a1a1 100644 --- a/tests/test-execution.toml +++ b/tests/test-execution.toml @@ -614,6 +614,10 @@ lane = "linux-guard" path = "tests/test_ci_release_helper_archs.py" lane = "linux-guard" +[[test]] +path = "tests/test_ci_required_checks_are_bounded.py" +lane = "linux-guard" + [[test]] path = "tests/test_ci_selective_layer_wiring.py" lane = "linux-guard" diff --git a/tests/test_ci_required_checks_are_bounded.py b/tests/test_ci_required_checks_are_bounded.py new file mode 100755 index 000000000000..ef41034a4d49 --- /dev/null +++ b/tests/test_ci_required_checks_are_bounded.py @@ -0,0 +1,123 @@ +#!/usr/bin/env python3 +"""A required check must be bounded, and a superseded PR run must be cancelled. + +Two properties, both of which CI is structurally unable to complain about +because neither one makes anything fail. + +A job with no `timeout-minutes` inherits GitHub's six-hour default. Every job +in `ci.yml` declares one between 5 and 20 minutes except `ci-status`, which is +the required aggregate: the check every pull request in the repository is +blocked on. A hang there blocks the whole repository for six hours and reads +as "still running" the entire time. + +A `pull_request` workflow with no concurrency group starts a fresh run per +push and lets the superseded ones finish. `ci.yml` and most of its neighbours +already cancel; five path-filtered workflows had no group at all. + +Both rules stay derived rather than listed. The required contexts come from +the repository ruleset and are matched to jobs by name, and a context that +stops matching any job fails this test rather than quietly guarding nothing. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +import yaml + + +ROOT = Path(__file__).resolve().parents[1] +WORKFLOWS = ROOT / ".github/workflows" + +# `gh api repos/manaflow-ai/cmux/rules/branches/main`, the +# required_status_checks rule. Update alongside the ruleset. +REQUIRED_CONTEXTS = ( + "CLA Assistant", + "CLA policy guard", + "ci-status", + "Web complexity", + "web-validation", +) + + +def load(path: Path) -> dict: + parsed = yaml.safe_load(path.read_text(encoding="utf-8")) + return parsed if isinstance(parsed, dict) else {} + + +def triggers(workflow: dict) -> set[str]: + # PyYAML resolves a bare `on:` key to the boolean True. + on = workflow.get("on", workflow.get(True, {})) + if isinstance(on, dict): + return set(on) + if isinstance(on, str): + return {on} + return set(on or ()) + + +def context_of(job_id: str, job: dict) -> str: + return job.get("name") or job_id + + +def main() -> int: + workflows = {p: load(p) for p in sorted(WORKFLOWS.glob("*.y*ml"))} + failures: list[str] = [] + + # Which workflows own a required check, by what their jobs are called. + owners: dict[str, Path] = {} + for path, workflow in workflows.items(): + for job_id, job in (workflow.get("jobs") or {}).items(): + if not isinstance(job, dict): + continue + context = context_of(job_id, job) + if context in REQUIRED_CONTEXTS: + owners[context] = path + + unmatched = [c for c in REQUIRED_CONTEXTS if c not in owners] + if unmatched: + failures.append( + "required contexts match no job; the ruleset and this list have " + f"drifted apart: {', '.join(unmatched)}" + ) + + for path in sorted(set(owners.values())): + for job_id, job in (workflows[path].get("jobs") or {}).items(): + if not isinstance(job, dict) or "uses" in job: + continue + if "timeout-minutes" not in job: + failures.append( + f"{path.relative_to(ROOT)}:{job_id} has no " + "timeout-minutes, so it inherits GitHub's six-hour " + "default -- in a workflow that owns a required check" + ) + + for path, workflow in workflows.items(): + if "pull_request" not in triggers(workflow): + continue + # A group is the requirement; what it does with a superseded run is + # the workflow's call. repair-nightly-appcast-content-types.yml sets + # cancel-in-progress: false on purpose because it writes to R2, and + # serializing is as good an answer as cancelling. + if not workflow.get("concurrency"): + failures.append( + f"{path.relative_to(ROOT)} runs on pull_request with no " + "concurrency group, so every push leaves the superseded run " + "running" + ) + + if failures: + print("unbounded CI:", file=sys.stderr) + for failure in failures: + print(f" - {failure}", file=sys.stderr) + return 1 + + print( + f"{len(REQUIRED_CONTEXTS)} required checks are bounded; " + "every pull_request workflow has a concurrency group" + ) + return 0 + + +if __name__ == "__main__": + sys.exit(main())