diff --git a/.github/workflows/ci-guards.yml b/.github/workflows/ci-guards.yml index 76de44788816..928bd19173ab 100644 --- a/.github/workflows/ci-guards.yml +++ b/.github/workflows/ci-guards.yml @@ -261,6 +261,10 @@ jobs: if: ${{ matrix.group == 'ci' }} run: python3 tests/test_ci_select_package_tests.py + - name: Validate workflow_run triggers name workflows that exist + if: ${{ matrix.group == 'ci' }} + run: python3 tests/test_ci_workflow_run_sources.py + - name: Validate shallow complexity comparison fetching if: ${{ matrix.group == 'ci' }} run: python3 tests/test_ci_fetch_complexity_base.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ef89fc600920..ca021a9d892a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -947,6 +947,7 @@ jobs: PY suite-coverage: + timeout-minutes: 5 # Compile admission builds the test bundle and stops, so a pull request # that edits cmuxTests/ and skips the full suite runs none of the code it # changed. The policy that allows the skip is written for a merge queue diff --git a/.github/workflows/cmux-tui-release-delivery.yml b/.github/workflows/cmux-tui-release-delivery.yml index 942dba935056..c3709e3cace2 100644 --- a/.github/workflows/cmux-tui-release-delivery.yml +++ b/.github/workflows/cmux-tui-release-delivery.yml @@ -11,6 +11,13 @@ on: permissions: contents: read +env: + # The file that must carry the display name in the trigger above. Renaming a + # workflow silently stops every `workflow_run` consumer of it, so + # tests/test_ci_workflow_run_sources.py derives the expected name from this + # path and fails the pull request that renames it. + SOURCE_WORKFLOW_PATHS: .github/workflows/cmux-tui-release.yml + concurrency: group: cmux-tui-registry-delivery cancel-in-progress: false diff --git a/.github/workflows/merge-group-fail-fast.yml b/.github/workflows/merge-group-fail-fast.yml index e21fd42554da..ba47dcacd87e 100644 --- a/.github/workflows/merge-group-fail-fast.yml +++ b/.github/workflows/merge-group-fail-fast.yml @@ -19,6 +19,16 @@ on: permissions: {} +env: + # `workflows:` above can only spell a display name, which is presentation and + # not identity: renaming that workflow would stop this one from triggering at + # all, with nothing red to say so. This records the file that must carry the + # name, tests/test_ci_workflow_run_sources.py derives the name from the file, + # and the step below confirms it again at run time against the run's own + # `path` -- display names are not unique, and the workflow_run object that + # arrives here carries the stable path and workflow id next to the name. + SOURCE_WORKFLOW_PATHS: .github/workflows/merge-group-policy-checks.yml + jobs: watch: name: watch merge group run @@ -30,15 +40,22 @@ jobs: - name: Locate and watch the merge-group CI run env: GH_TOKEN: ${{ github.token }} + SOURCE_PATH: ${{ github.event.workflow_run.path }} SOURCE_EVENT: ${{ github.event.workflow_run.event }} HEAD_SHA: ${{ github.event.workflow_run.head_sha }} + # The CI run is located by workflow file, never by display name. CI_RUNS: repos/${{ github.repository }}/actions/workflows/ci.yml/runs REPOSITORY: ${{ github.repository }} run: | set -euo pipefail + if [ "$SOURCE_PATH" != "$SOURCE_WORKFLOW_PATHS" ]; then + echo "::error::Started by $SOURCE_PATH, not $SOURCE_WORKFLOW_PATHS; refusing to cancel anything." + exit 1 + fi + if [ "$SOURCE_EVENT" != "merge_group" ]; then - echo "::error::Merge-group policy checks unexpectedly ran for '$SOURCE_EVENT'." + echo "::error::$SOURCE_WORKFLOW_PATHS unexpectedly ran for '$SOURCE_EVENT'." exit 1 fi diff --git a/.github/workflows/persistent-macos-router.yml b/.github/workflows/persistent-macos-router.yml index 79cdf405cd52..7e1b76168fa6 100644 --- a/.github/workflows/persistent-macos-router.yml +++ b/.github/workflows/persistent-macos-router.yml @@ -20,6 +20,13 @@ on: permissions: {} +env: + # The file that must carry the display name in the trigger above. Renaming a + # workflow silently stops every `workflow_run` consumer of it, so + # tests/test_ci_workflow_run_sources.py derives the expected name from this + # path and fails the pull request that renames it. + SOURCE_WORKFLOW_PATHS: .github/workflows/ci.yml + concurrency: group: persistent-mac-router-${{ github.event.workflow_run.id }}-${{ github.event.workflow_run.run_attempt }} cancel-in-progress: true diff --git a/.github/workflows/update-homebrew.yml b/.github/workflows/update-homebrew.yml index a9f9ca1859df..1f6fbfab0440 100644 --- a/.github/workflows/update-homebrew.yml +++ b/.github/workflows/update-homebrew.yml @@ -18,6 +18,14 @@ permissions: # apart from a failure in a job the cask does not consume. actions: read +env: + # The file that must carry the display name in the trigger above. Renaming a + # workflow silently stops every `workflow_run` consumer of it, so + # tests/test_ci_workflow_run_sources.py derives the expected name from this + # path and fails the pull request that renames it. The job name the gate + # below matches is pinned the same way, by tests/test_release_homebrew_gate.py. + SOURCE_WORKFLOW_PATHS: .github/workflows/release.yml + jobs: gate: name: Require a signed macOS build diff --git a/tests/test-execution.toml b/tests/test-execution.toml index 721dfe240852..d2e34e23bac4 100644 --- a/tests/test-execution.toml +++ b/tests/test-execution.toml @@ -414,6 +414,10 @@ lane = "linux-guard" path = "tests/test_ci_select_package_tests.py" lane = "linux-guard" +[[test]] +path = "tests/test_ci_workflow_run_sources.py" +lane = "linux-guard" + [[test]] path = "tests/test_ci_sidebar_lazy_layout_guard.py" lane = "linux-guard" diff --git a/tests/test_ci_workflow_run_sources.py b/tests/test_ci_workflow_run_sources.py new file mode 100644 index 000000000000..09bd886c68f5 --- /dev/null +++ b/tests/test_ci_workflow_run_sources.py @@ -0,0 +1,185 @@ +#!/usr/bin/env python3 +"""A `workflow_run` trigger spells a display name; pin it to the file behind it. + +`on.workflow_run.workflows:` matches the triggering workflow's `name:`. That +name is presentation text. Renaming a workflow for clarity stops every consumer +of it from triggering, and nothing turns red: the consumer simply never runs +again. `merge-group-fail-fast.yml` is the expensive version of that failure -- +it cancels a merge group's CI run at the first failed job, so when it stops +firing every queue entry behind a doomed one burns full runner time instead. + +The Actions API does carry stable identity. A workflow run object has `path` +(`.github/workflows/ci.yml`) and `workflow_id` next to the presentation `name`, +and a run can be located by file path through +`repos/:owner/:repo/actions/workflows//runs`. The trigger filter accepts +neither, so the name has to stay in the trigger. What changes is that the name +stops being the only copy: each consumer declares the file it means in +`env.SOURCE_WORKFLOW_PATHS`, and this test derives the expected names from +those files. Renaming a producer now fails this test on the pull request that +renames it. + +Matching a *job* by display name has no stable alternative at all: the jobs API +reports `name` and never the YAML job id. Those matches are listed below so the +same derivation applies to them. The one in `update-homebrew.yml` is already +pinned by `tests/test_release_homebrew_gate.py` and is not repeated here. +""" + +from __future__ import annotations + +import re +import sys +from pathlib import Path + +import yaml + +ROOT = Path(__file__).resolve().parents[1] +WORKFLOWS = ROOT / ".github" / "workflows" + +# The declaration a `workflow_run` consumer carries: the workflow files whose +# `name:` its `workflows:` filter spells out, space separated. +DECLARATION = "SOURCE_WORKFLOW_PATHS" + +# Consumers that match a job by display name. Each entry is +# (consumer, producer, job id); the expected display name is derived from the +# producer's job rather than written out a second time here. +PINNED_JOB_NAMES = ( + ( + ".github/workflows/ios-testflight.yml", + ".github/workflows/ios-testflight.yml", + "upload", + ), +) + +# The fail-fast watcher is started by a display name but must not act on one. +# Display names are not unique, and the run object that arrives here carries +# the stable path, so it can confirm what woke it before it cancels anything. +IDENTITY_CHECKED = ".github/workflows/merge-group-fail-fast.yml" + +# `repos/:owner/:repo/actions/workflows//runs` is the identity-based way +# to find another workflow's runs. The file still has to exist. +RUNS_BY_PATH = re.compile(r"actions/workflows/([A-Za-z0-9._-]+\.ya?ml)/runs") + + +def load(path: Path) -> dict: + document = yaml.safe_load(path.read_text(encoding="utf-8")) + return document if isinstance(document, dict) else {} + + +def triggers(document: dict) -> dict: + # PyYAML reads the bare key `on` as boolean True. + on = document.get("on", document.get(True)) + return on if isinstance(on, dict) else {} + + +def display_name(path: Path, document: dict) -> str: + name = document.get("name") + # A workflow without `name:` is listed, and matched, by its path. + return str(name) if name is not None else path.relative_to(ROOT).as_posix() + + +def as_list(value: object) -> list[str]: + if isinstance(value, str): + return [value] + if isinstance(value, list): + return [str(item) for item in value] + return [] + + +def check_workflow_run_sources(documents: dict[Path, dict], failures: list[str]) -> None: + for path, document in documents.items(): + workflow_run = triggers(document).get("workflow_run") + if not isinstance(workflow_run, dict): + continue + relative = path.relative_to(ROOT).as_posix() + named = as_list(workflow_run.get("workflows")) + environment = document.get("env") + declared = str((environment or {}).get(DECLARATION, "")).split() + + if not declared: + failures.append( + f"{relative}: triggers on the display names {sorted(named)} and " + f"declares no {DECLARATION}, so renaming any of those workflows " + "stops this one from ever triggering and nothing reports it" + ) + continue + + produced: list[str] = [] + for source in declared: + source_path = ROOT / source + if source_path not in documents: + failures.append( + f"{relative}: {DECLARATION} names {source}, which is not a " + "workflow in .github/workflows" + ) + continue + produced.append(display_name(source_path, documents[source_path])) + + if sorted(produced) != sorted(named): + failures.append( + f"{relative}: the trigger matches {sorted(named)} but " + f"{DECLARATION} resolves to {sorted(produced)}; the workflows " + "this one means no longer carry the names it waits for" + ) + + +def check_pinned_job_names(documents: dict[Path, dict], failures: list[str]) -> None: + for consumer, producer, job_id in PINNED_JOB_NAMES: + producer_jobs = documents.get(ROOT / producer, {}).get("jobs") or {} + job = producer_jobs.get(job_id) + if not isinstance(job, dict): + failures.append( + f"{consumer}: matches a job of {producer} that no longer exists: {job_id}" + ) + continue + # The API name is the mapping key unless `name:` overrides it. + name = str(job.get("name", job_id)) + text = (ROOT / consumer).read_text(encoding="utf-8") + if f'"{name}"' not in text and f"'{name}'" not in text: + failures.append( + f"{consumer}: no longer matches {producer}'s {job_id} job, whose " + f"display name is now {name!r}" + ) + + +def check_runs_lookups_by_path(documents: dict[Path, dict], failures: list[str]) -> None: + known = {path.name for path in documents} + for path in documents: + text = path.read_text(encoding="utf-8") + for filename in sorted(set(RUNS_BY_PATH.findall(text))): + if filename not in known: + failures.append( + f"{path.relative_to(ROOT).as_posix()}: looks up runs of " + f"{filename}, which is not a workflow in .github/workflows" + ) + + +def check_identity_before_acting(failures: list[str]) -> None: + watcher = (ROOT / IDENTITY_CHECKED).read_text(encoding="utf-8") + if "github.event.workflow_run.path" not in watcher or f"${DECLARATION}" not in watcher: + failures.append( + f"{IDENTITY_CHECKED}: must compare github.event.workflow_run.path " + f"against {DECLARATION} before it cancels anything, so a workflow " + "that merely shares a display name cannot drive it" + ) + + +def main() -> int: + paths = sorted([*WORKFLOWS.glob("*.yml"), *WORKFLOWS.glob("*.yaml")]) + documents = {path: load(path) for path in paths} + + failures: list[str] = [] + check_workflow_run_sources(documents, failures) + check_pinned_job_names(documents, failures) + check_runs_lookups_by_path(documents, failures) + check_identity_before_acting(failures) + + if failures: + for failure in failures: + print(f"FAIL: {failure}") + return 1 + print(f"PASS: {len(documents)} workflows match the workflows they name by file, not by display text") + return 0 + + +if __name__ == "__main__": + sys.exit(main())