Skip to content

ci: bound the required aggregate check and group the ungrouped PR workflows - #13804

Merged
teamleaderleo merged 2 commits into
mainfrom
ci/bound-required-checks
Sep 23, 2026
Merged

teamleaderleo merged 2 commits into
mainfrom
ci/bound-required-checks

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

Two invariants nothing in CI can complain about, because neither one makes anything fail. Both are measured, and the first commit shows the lint failing on main with exactly six findings.

ci-status had no timeout

A job without timeout-minutes inherits GitHub's six-hour default. ci-status is the required aggregate — the check every pull request in this repository is blocked on — and it runs one step that reads needs and decides. A hang there blocks the repository for six hours and reads as "still running" the whole time.

Every one of its ten siblings in ci.yml is bounded between 5 and 20 minutes, and across all five workflows that own a required context it was the only job without a bound. That makes it drift rather than intent, so it now matches web-status, the equivalent aggregate in web-validation.yml, at 5.

Five pull_request workflows had no concurrency group

ci-cache-receipts, cmux-cloud-cli, indexnow-tests, localization-catalog, r2-upload-tests. Without a group, each push starts a fresh run and lets the superseded ones finish. They are all path-filtered so they fire rarely — but when they fire it is on a branch someone is actively pushing to, which is exactly when superseded runs pile up.

They get a ref-keyed group that cancels only on pull_request, so a push to main is never interrupted. That is the idiom cloud-machine-tests.yml and cloud-task-local-tests.yml already use.

The lint

Both rules stay derived rather than listed. Required contexts are matched to the jobs that produce them, and a context that stops matching any job fails the test rather than quietly guarding nothing.

Having a group is the requirement, not what it does with a superseded run — repair-nightly-appcast-content-types.yml sets cancel-in-progress: false deliberately because it writes to R2, and serializing answers the same problem. My first draft flagged it; that was the lint being wrong, not the workflow.

The shape is borrowed from quarry's tests/test_ci_policy.py, which keeps this class of invariant in one readable file instead of hoping nobody regresses it.

Also measured, deliberately not changed

47 of 233 jobs repo-wide declare no timeout-minutes. None of them run on macOS — the expensive class is already fully bounded, which is why this PR does not touch them. The remaining ones are Linux, where a six-hour hang costs less, and picking 47 timeout values I cannot validate would trade a small bill for new flakiness. Worth a follow-up by someone who knows the expected runtimes.

ci-web.yml has three untimed jobs and zero runs in recent history despite being edited on 2026-09-21. I did not touch it; someone should confirm whether it is still meant to fire.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Bound the required ci-status aggregate job and give five path-filtered pull_request workflows a concurrency group, so a hung check can't block the repo for six hours and superseded runs don't pile up on active branches.

  • ci-status gets a 5-minute timeout, matching web-status; every other job in ci.yml was already bounded.
  • ci-cache-receipts, cmux-cloud-cli, indexnow-tests, localization-catalog, and r2-upload-tests get a ref-keyed group that cancels only on pull_request.
  • Adds a lint that fails CI if any workflow owning a required context has an unbounded job or runs on pull_request without a concurrency group.

Written for commit 195b301. Summary will update on new commits.

Review in cubic

teamleaderleo and others added 2 commits September 22, 2026 16:53
Two properties nothing in CI can complain about, because neither one makes
anything fail.

A job without `timeout-minutes` inherits GitHub's six-hour default. Every job
in ci.yml declares one between 5 and 20 minutes except `ci-status` -- the
required aggregate, the check every pull request in this repository is
blocked on. A hang there blocks the repository for six hours and reads as
"still running" the whole time. Across all five workflows that own a required
context, it is the only job without a bound.

A `pull_request` workflow with no concurrency group starts a fresh run per
push and lets the superseded ones run to completion. Most of the repository
already groups; five path-filtered workflows have no group at all.

Both rules are derived, not listed. Required contexts are matched to the jobs
that produce them, and a context that stops matching any job fails this test
rather than quietly guarding nothing. Having a group is the requirement, not
what it does with a superseded run: repair-nightly-appcast-content-types.yml
sets `cancel-in-progress: false` on purpose because it writes to R2.

The shape is borrowed from quarry's tests/test_ci_policy.py, which keeps this
kind of invariant in one readable place instead of hoping nobody regresses it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`ci-status` gets `timeout-minutes: 5`, matching `web-status`, the equivalent
aggregate in web-validation.yml. It runs one step that reads `needs` and
decides; every other job in ci.yml was already bounded between 5 and 20
minutes.

ci-cache-receipts, cmux-cloud-cli, indexnow-tests, localization-catalog and
r2-upload-tests get a concurrency group keyed by ref, cancelling only on
pull_request so a push to main is never interrupted. That is the idiom
cloud-machine-tests.yml and cloud-task-local-tests.yml already use.

All five are path-filtered, so they fire rarely -- but when they do fire they
fire on a branch someone is actively pushing to, which is exactly when
superseded runs pile up.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 22, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 9 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 6146612f-6970-4725-92d4-338e49107eea

📥 Commits

Reviewing files that changed from the base of the PR and between abd614f and 195b301.

📒 Files selected for processing (9)
  • .github/workflows/ci-cache-receipts.yml
  • .github/workflows/ci-guards.yml
  • .github/workflows/ci.yml
  • .github/workflows/cmux-cloud-cli.yml
  • .github/workflows/indexnow-tests.yml
  • .github/workflows/localization-catalog.yml
  • .github/workflows/r2-upload-tests.yml
  • tests/test-execution.toml
  • tests/test_ci_required_checks_are_bounded.py

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@teamleaderleo
teamleaderleo merged commit d33943f into main Sep 23, 2026
50 of 52 checks passed
@teamleaderleo
teamleaderleo deleted the ci/bound-required-checks branch September 23, 2026 00:03
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Correction to the last paragraph of this PR's description.

I wrote that ci-web.yml "has three untimed jobs and zero runs in recent history despite being edited on 2026-09-21" and suggested someone confirm whether it is still meant to fire. It is: ci-web.yml is workflow_call, invoked from ci.yml:659. A reusable workflow has no runs of its own, so gh run list --workflow=ci-web.yml returns [] however often it runs. Nobody needs to go look at whether it is dead.

That makes its untimed jobs a live problem rather than a dormant one — they run on every web pull request and gate ci-status. #13811 extends this PR's guard to follow uses: ./.github/workflows/... out of a required-check workflow, which finds those three plus ci-guards.yml:workflow-guard-tests, and bounds all four.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant