Repository navigation
ci: stop admitting macOS after a run has already failed - #13969
Merged
Merged
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: manaflow-ai/cmux/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe macOS debounce job now checks for failed jobs after its wait and declines admission when it finds one. Tests cover failure, fail-open conditions, and a moved pull-request head. ChangesmacOS CI admission
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Other Merge Risk: ⚪ Minimal · up to Failed runs decline macOS admission while still producing a failed CI result. No issue identified here needs resolution before merge. 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 1 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
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 <noreply@anthropic.com>
teamleaderleo
force-pushed
the
ci/macos-admission-skips-red-runs
branch
from
September 23, 2026 11:23
ffe5c53 to
93eccb7
Compare
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
teamleaderleo
commented
Sep 23, 2026
teamleaderleo
left a comment
teamleaderleo
left a comment
Collaborator
Author
There was a problem hiding this comment.
Review before merge. I took this over from NightPetrel.
- The verdict can't flip. Every job in
ci.ymlis inci-status'sneeds, andci-statusaccepts onlysuccessorskipped. No job inci.ymlor its reusable workflows has job-levelcontinue-on-error. So once the jobs API reports afailure, the run is already red. Declining macOS changes what it costs, not whether it passes. - Declining by failing is correct. An output would leave
macosskipped under a successful dependency, and "Re-run failed jobs" would never pick it up. The comment in the step says so. Keep it. - It admits when in doubt. If the step can't read the jobs, gets an error body, sees only
cancelledjobs, or is on a re-run attempt, it admits, and the table-driven test covers each of those cases. - Tested on the merged tree. Merged onto the current
origin/main:python3 tests/test_ci_change_areas.pypasses on the merged tree. The fourmacos_admissiontests also pass on the PR head.
— KeyboardCat g1 ⚙️
Run: run_cmux_ci_efficiency_land_13969_and_13990_20260923_c1f522ea
teamleaderleo
enabled auto-merge (squash)
September 23, 2026 14:59
rustybret
pushed a commit
to rustybret/bmux
that referenced
this pull request
Sep 23, 2026
9963f3f ci: stop admitting macOS after a run has already failed (manaflow-ai#13969) c7fb3b1 ci: put the switch for paid macOS capacity in the repository (manaflow-ai#13973) 0044d5a ci: mirror app-host batch output without racing a tail (manaflow-ai#13990) a0b1dea fix(ci): unquote replayed argv so post-manaflow-ai#13831 selectors resolve (manaflow-ai#13974) c14b142 test: target local Undo at the editable responder (manaflow-ai#13989) 5985cf5 Mirror terminal focus before the runtime surface exists (manaflow-ai#13968) afd5c47 ci: let E2E dispatches adopt a compiled product instead of rebuilding (manaflow-ai#13958) # Conflicts: # .github/workflows/ci-guards.yml # .github/workflows/ci-macos-compat.yml # .github/workflows/ci-macos.yml # .github/workflows/ci.yml # .github/workflows/cli-pipe-regressions.yml # .github/workflows/cloud-command-deadlines.yml # .github/workflows/cloud-machine-tests.yml # .github/workflows/cmux-tui-build-package.yml # .github/workflows/iroh-release-gate.yml # .github/workflows/nightly.yml # .github/workflows/perf-activation.yml # .github/workflows/plain-paste-worker.yml # .github/workflows/relay-tls.yml # .github/workflows/release.yml # .github/workflows/terminal-hang-diagnostics.yml # .github/workflows/test-e2e.yml # .github/workflows/tmux-corpus.yml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A pull request that trips a fast Linux guard is red about a minute in. Its
macOS compile admission is queued anyway and starts on a Mac half an hour
later, compiling a revision nobody will look at. In one 2.3-hour window,
25 of the 27 failing CI runs 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.
After this change those shards are never queued. The lane is untouched on
green pull requests, and
blacksmith-6vcpu-macos-15is a shared pool, so thereclaimed capacity comes back as shorter macOS waits for everyone else — one
sampled run sat 33 minutes between its debounce finishing and its compile
starting.
Mechanism
macos-debouncealready waits on a cheap Linux runner, re-reads the pullrequest 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.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
macosis skipped either way, and only afailed dependency makes "Re-run failed jobs" re-run it. An output would
strand that run red until someone re-ran everything, which is worse than the
cosmetic cost. This was caught in review; the first revision of this PR had
exactly that bug.
macosneeds no new condition andtestsneeds no change: a failedmacos-debouncealready skips the lane through the existingresult == 'success' || result == 'skipped'clause.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=0skip the wait and with it thischeck — which is how you ask for those results anyway.
macOS admission gains no dependency edge on the Linux suites, which
test_ci_change_areas.pyforbids, but it now depends on their results insubstance, and racily: whether a run collects macOS results depends on whether
a Linux guard fails before or after the debounce window closes. Flagging that
rather than letting the guard imply otherwise.
If
vars.CI_PERSISTENT_MAC_COMPILEis ever set topilotorall, apersistent Mac compile is dispatched at T≈0, before this checkpoint; a decline
would then leave its product built and unconsumed.
docs/ci/workflow-inventory.mdrecords that the pilot has never run, so this is latent, not live.
Validation
python3 tests/test_ci_change_areas.pypasses, including three new tests.run_macos_debouncehelper drives the real step script fromci.ymlagainst a fakeghthat serves a JSON payload and then runs thestep's own
--jqprogram over it with realjq. A pre-digested answer wouldhave let a wrong accessor pass; this does not.
.jobs[]accessor, countingcancelledas a failure, removing the check entirely, droppingactions: read, and moving the check ahead of the debounce wait.gh, emptyjobs, anerror body, and a
cancelled-only run all admit; a moved head still failsfirst.
actionlint .github/workflows/ci.ymlclean.ci-guards.ymlsweep: three failures, all environmental and unrelated(
catalog-diffneeds workflow-set variables,test_ghostty_zig_version_sync.shneeds the ghostty submodule,
lint-stored-dispatch-work-items.pyneedsvendor/bonsplit).Not established here: the reclaimed minutes are measured over one 2.3-hour
window on 2026-09-23, not a daily average, and the queue-wait improvement is a
single observation rather than a distribution.
🤖 Generated with Claude Code
Summary by CodeRabbit