From 93eccb7e4aa7cde8e189568ac1b55a6f8ae45754 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Wed, 23 Sep 2026 04:06:15 -0700 Subject: [PATCH] ci: stop admitting macOS after a run has already failed A pull request that trips a fast Linux guard is red about a minute in, but its macOS compile admission is queued anyway and starts on a Mac half an hour later. Over 40 failing CI runs in one 2.3-hour window, 25 of the 27 that reached macOS admission were already red when the debounce checkpoint finished, and 416 of their 656 macOS job-minutes went to shards that had not started yet. The debounce job already waits on a cheap Linux runner, re-reads the pull request head, and fails the run rather than admitting macOS when the head moved. It now fails the same way when a job in this run has concluded `failure`: the verdict is settled on this head, and the fix push opens a new run. Declining by failing is load-bearing rather than incidental. A job output would read better -- the debounce job would stay green and the run would carry one red job instead of two -- but `macos` is skipped either way, and only a *failed* dependency makes "Re-run failed jobs" re-run it. An output would strand that run red until someone re-ran everything. Tradeoffs: a red run no longer reports whether macOS would also have failed, so a pull request broken on both platforms needs a second round trip. Re-run attempts and CI_MACOS_ADMISSION_DEBOUNCE_SECONDS=0 skip the wait and with it this check, which is how you ask for those results anyway. Every unreadable case admits -- a failed call, an empty reply, an error body, or a first page that misses a later job. macOS admission gains no dependency edge on the Linux suites, which `test_ci_change_areas.py` forbids, but it does now depend on their results in substance, and racily: whether a run collects macOS results depends on whether a Linux guard fails before or after the debounce window closes. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 30 ++++++++- tests/test_ci_change_areas.py | 112 ++++++++++++++++++++++++++++++++++ 2 files changed, 139 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6a294afa87d7..7dd2bba7f205 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -860,9 +860,10 @@ jobs: # the previous run through the concurrency group, but by then its macOS jobs # have already been billed. Holding macOS admission briefly on a Linux runner # lets a quick follow-up push cancel the run before any Mac starts. A stale - # head SHA after the wait fails this run instead of admitting it. Merge groups - # and dispatches are not debounced. CI_MACOS_ADMISSION_DEBOUNCE_SECONDS=0 - # turns the wait off. + # head SHA after the wait fails this run instead of admitting it, and so does + # a run that has already failed a job. Merge groups and dispatches are not + # debounced. A re-run attempt and CI_MACOS_ADMISSION_DEBOUNCE_SECONDS=0 both + # skip the wait and with it both checks. macos-debounce: name: macOS admission debounce needs: changes @@ -870,6 +871,7 @@ jobs: runs-on: ${{ vars.LINUX_RUNNER || 'blacksmith-4vcpu-ubuntu-2404' }} timeout-minutes: 15 permissions: + actions: read pull-requests: read steps: - name: Wait for follow-up pushes @@ -894,6 +896,28 @@ jobs: echo "::error::PR head moved from $HEAD_SHA to $current; the newer run owns macOS admission." exit 1 fi + # A job that has already failed has settled this run: the pull request + # is red on this head whatever a Mac finds, and the fix push opens a + # new run. Declining costs only work nobody was going to read. + # + # Decline the same way a moved head does, by failing. A job output + # would read better here, but `macos` is skipped either way and only a + # *failed* dependency makes "Re-run failed jobs" re-run it -- an + # output would strand the run red until someone re-ran everything. + # + # Only a concluded `failure` counts: `cancelled` means the run is + # already going away. An unreadable reply, an error body, or a first + # page that misses a later job all admit, like the check above. + if failed="$(gh api \ + "repos/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID/jobs?per_page=100" \ + --jq '[.jobs[] | select(.conclusion == "failure")] | length')" \ + && [ -n "$failed" ] && [ "$failed" != "0" ]; then + echo "::error::$failed job(s) in this run already failed; not admitting macOS. Re-run failed jobs to collect macOS results anyway." + exit 1 + fi + if [ -z "${failed:-}" ]; then + echo "Could not read this run's jobs; admitting macOS." + fi echo "Head $HEAD_SHA is still current; admitting macOS." macos: diff --git a/tests/test_ci_change_areas.py b/tests/test_ci_change_areas.py index bd2a89141f9b..63e05059610a 100755 --- a/tests/test_ci_change_areas.py +++ b/tests/test_ci_change_areas.py @@ -2715,6 +2715,118 @@ def test_macos_admission_waits_for_pull_request_debounce() -> None: assert " - macos-debounce" in workflow_job_block("ci-status") +JOBS_ONE_FAILURE = """{"jobs":[{"name":"changes","conclusion":"success"}, +{"name":"linux-preflight","conclusion":"failure"}, +{"name":"guards","conclusion":null}]}""" +JOBS_CLEAN = """{"jobs":[{"name":"changes","conclusion":"success"}, +{"name":"guards","conclusion":null}]}""" +JOBS_CANCELLED_ONLY = """{"jobs":[{"name":"web","conclusion":"cancelled"}]}""" +JOBS_EMPTY = """{"jobs":[]}""" +JOBS_ERROR_BODY = """{"message":"Not Found"}""" + + +def run_macos_debounce( + *, + jobs_payload: str = JOBS_CLEAN, + pulls_sha: str = "abc", + gh_broken: bool = False, + attempt: str = "1", + debounce_seconds: str = "1", +) -> subprocess.CompletedProcess: + """Run the real debounce step against a fake `gh` and a real `jq`. + + The fake serves the payload and then runs the step's own `--jq` program + over it, so a wrong accessor in the workflow shows up here as the + fail-open it would be in CI rather than passing on a pre-digested answer. + """ + script = workflow_job_step_script("macos-debounce", "Wait for follow-up pushes") + with tempfile.TemporaryDirectory() as raw: + root = Path(raw) + fake_bin = root / "bin" + fake_bin.mkdir() + # The step sleeps for the debounce window before it reads anything. + (fake_bin / "sleep").write_text("#!/bin/sh\nexit 0\n", encoding="utf-8") + (fake_bin / "sleep").chmod(0o755) + payload = root / "jobs.json" + payload.write_text(jobs_payload, encoding="utf-8") + gh = fake_bin / "gh" + if gh_broken: + gh.write_text("#!/bin/sh\nexit 1\n", encoding="utf-8") + else: + gh.write_text( + "#!/bin/sh\n" + 'prog=""\n' + 'prev=""\n' + 'for a in "$@"; do\n' + ' [ "$prev" = "--jq" ] && prog="$a"\n' + ' prev="$a"\n' + "done\n" + 'case "$*" in\n' + f" *pulls*) printf %s '{pulls_sha}' ;;\n" + f' *jobs*) jq -r "$prog" < "{payload}" ;;\n' + " *) exit 1 ;;\n" + "esac\n", + encoding="utf-8", + ) + gh.chmod(0o755) + return subprocess.run( + ["bash", "-c", script], + env={ + "PATH": f"{fake_bin}:{os.environ['PATH']}", + "GITHUB_RUN_ATTEMPT": attempt, + "GITHUB_REPOSITORY": "manaflow-ai/cmux", + "GITHUB_RUN_ID": "9", + "DEBOUNCE_SECONDS": debounce_seconds, + "PR_NUMBER": "1", + "HEAD_SHA": "abc", + "GH_TOKEN": "token", + }, + capture_output=True, + text=True, + check=False, + ) + + +def test_macos_admission_declines_a_run_that_already_failed() -> None: + declined = run_macos_debounce(jobs_payload=JOBS_ONE_FAILURE) + assert declined.returncode == 1 + assert "already failed" in declined.stdout + declined.stderr + # Declining by failing is load-bearing: `macos` is skipped either way, and + # only a failed dependency makes "Re-run failed jobs" re-run it. + debounce = workflow_job_block("macos-debounce") + assert " actions: read" in debounce + assert "$GITHUB_RUN_ID/jobs" in debounce + # The check must sit after the wait, so a re-run attempt and a zero-second + # window bypass it along with the debounce itself. + assert "$GITHUB_RUN_ID/jobs" in debounce.split('sleep "$DEBOUNCE_SECONDS"', 1)[1] + # macOS admission stays uncoupled from the Linux suites as a dependency. + assert "needs.linux-preflight" not in workflow_job_block("macos") + + +def test_macos_admission_admits_whenever_it_cannot_prove_a_failure() -> None: + for label, kwargs in ( + ("no failures", {"jobs_payload": JOBS_CLEAN}), + # A cancelled run is already going away; it is not a verdict. + ("cancelled only", {"jobs_payload": JOBS_CANCELLED_ONLY}), + ("no jobs yet", {"jobs_payload": JOBS_EMPTY}), + # An error body makes the step's own jq program fail, which must admit. + ("api error body", {"jobs_payload": JOBS_ERROR_BODY}), + ("gh unusable", {"gh_broken": True}), + # A re-run is asking for the results this would withhold. + ("re-run attempt", {"attempt": "2", "jobs_payload": JOBS_ONE_FAILURE}), + ("wait disabled", {"debounce_seconds": "0", "jobs_payload": JOBS_ONE_FAILURE}), + ): + result = run_macos_debounce(**kwargs) + assert result.returncode == 0, f"{label}: {result.stdout}{result.stderr}" + assert "already failed" not in result.stdout + result.stderr, label + + +def test_macos_admission_still_declines_a_moved_head_first() -> None: + moved = run_macos_debounce(pulls_sha="def", jobs_payload=JOBS_CLEAN) + assert moved.returncode == 1 + assert "head moved" in (moved.stdout + moved.stderr).lower() + + def run_tests_gate(needs: dict) -> subprocess.CompletedProcess: script = workflow_job_step_script("tests", "Check platform workflow routing") body = script.split("python3 - <<'PY'\n", 1)[1].rsplit("\nPY", 1)[0]