Repository navigation
ci: nightly failover and janitor exemptions cover delivery runs only - #28374
Conversation
… no delivery run Red on base: delivery_workflows has no delivers(), never_fails_over() takes only a path, and the janitor protects and reserves every nightly-named run, pull request test runs included. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-0b4091cd6a9b72e19edf
One predicate, delivery_workflows.delivers(): a delivery workflow's run from any event but a pull request's. The rescue's open-head failover exclusion (never_fails_over), the janitor's protected_reason and orphan protection, and reserves_pool() all use it, so a pull request's test run of nightly-next-canary.yml or nightly-next-launch-smoke.yml fails over, is swept and reserves no label, while scheduled, dispatched and pushed delivery runs stay protected and reserved. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-0b4091cd6a9b72e19edf
|
All contributors have signed the CLA ✍️ ✅ |
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Classify delivery events scripts/ci/delivery_workflows.py, tests/test_delivery_workflows_cancel.py |
The shared helpers classify pull_request, pull_request_target, and merge_group as non-delivery events. Tests cover delivery classification across workflow names and events. |
Apply event-aware rescue failover scripts/ci/owned_pool_rescue.py, tests/test_ci_owned_pool_rescue.py |
Rescue targets record their event, and failover exclusions depend on that event. Tests cover pull-request targets and rescue behavior. |
Apply event-aware queue handling scripts/ci/queue_janitor.py, tests/test_ci_queue_janitor.py |
Workflow protection and pool reservation use event-aware delivery classification. Tests cover pull-request, scheduled, and manually dispatched runs. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix
Suggested reviewers: lawrencecchen
Merge Risk: 
Minimal · up to 30486
-
The change distinguishes pull-request test runs from delivery runs. The identified DEV daily reservation gap predates this PR and does not block merging this change after normal checks.
Pre-merge checks: 
24 · 
1
-
Failed checks: 1 warning Passed checks: 24 passed-
Full details: Description check
-
Explanation
The description is mostly complete. It explains the problem, resulting behavior, implementation scope, tests run, observed results, unverified live sweep, and changelog status. The Proof and Checklist sections from the template are omitted, but they are non-critical for this CI-only change.
Full details: Cmux Cloud Persistent Session And Early Input
-
Explanation
PASS: The pull request changes only CI delivery classification, failover, janitor logic, and related tests. It does not change Cloud terminal creation, cmux-tui transport, manual renderers, Ghostty runtime admission, input routing, or auth/idempotency/revision/lease handling. The cmux references are workflow names and comments only, so the cloud persistent-session rule is not applicable.
Full details: Cmux Swift Blocking Runtime
-
Explanation
The pull request changes only Python files under
scripts/ciand test files. The reviewed diff contains no Swift or Apple project files, so it does not introduce or expand Swift blocking or timing-based synchronization.
Full details: Cmux Browser Automation Off-Main
-
Explanation
The pull request changes only Python CI modules and their tests:
scripts/ci/*andtests/*. It changes no browser socket automation source, WebKit/AppKit routing, worker policy, or policy tests. The browser-automation rule therefore has no applicable failure condition.
Full details: Cmux Expensive Synchronous Load
-
Explanation
PASS: The pull request changes only Python files under scripts/ci and tests. The authoritative diff contains no Swift files or Swift production code, so it cannot add or move an expensive synchronous Swift agent-history load.
Full details: Cmux Cache Substitution Correctness
-
Explanation
PASS: The pull request changes only Python files under scripts/ci and tests. It contains no production Swift, TypeScript, or JavaScript changes, so the cache-substitution correctness condition does not apply.
Full details: Cmux No Hacky Sleeps
-
Explanation
The production diff only adds event-aware delivery classification and propagates run events. It introduces no sleep, timer, polling, fixed delay, or wall-clock synchronization. Existing timing code is not materially changed, and the test changes are test scaffolding.
Full details: Cmux Algorithmic Complexity
-
Explanation
The production diff adds only bounded classification work.
is_delivery_workflow()scans the fixed four-entryDELIVERY_GLOBS, andis_delivery_run()scans the bounded run-name arguments and fixed prefix tuple. The janitor adds one such check per run in its existingpool_load_snapshot()pass; it does not rescan runs or jobs. The rescue uses fixed-set membership and the same bounded helper. No scalable collection, per-target full rescan, sorting, filtering, or in-memory join was introduced.
Full details: Cmux Swift Concurrency
-
Explanation
PASS: The pull request changes only Python CI scripts and Python tests. The reviewed diff contains no Swift files or cmux-owned Swift code, so it does not introduce or expand any legacy Swift concurrency pattern.
Full details: Cmux Swiftpm Lockfiles
-
Explanation
PASS. The pull request changes only Python CI scripts and Python tests. It does not change a SwiftPM package, Package.swift, Package.resolved, Xcode project or workspace files, .gitignore files, workflows, or dependency pins. Therefore none of the SwiftPM lockfile failure conditions apply.
Full details: Cmux Swift Logging
-
Explanation
PASS: The pull request changes only Python CI modules and Python tests. The authoritative diff contains no Swift app/runtime changes and adds no logging statements covered by
.github/review-bot-rules/swift-logging.md.
Full details: Cmux User-Facing Error Privacy
-
Explanation
PASS — The diff changes internal CI policy in
scripts/ci/delivery_workflows.py,scripts/ci/owned_pool_rescue.py, andscripts/ci/queue_janitor.py, plus tests. The scripts run from CI workflows (ci-queue-janitor.ymlandci-owned-pool-rescue.yml). The changes add event classification and alter failover, janitor, and pool-reservation decisions. They do not add or change cmux user-facing errors, alerts, API bodies, or recovery copy, and no concrete path to a cmux end user exists.
Full details: Cmux Full Internationalization
-
Explanation
The PR changes only Python CI logic, comments/docstrings, and tests. It adds no Swift UI text, app localization entries, web UI/API copy, metadata, rendered markdown, or changelog content. The added prose is developer-facing and falls under the allowed cases.
Full details: Cmux Swiftui State Layout
-
Explanation
PASS: The pull request changes only Python CI scripts and Python tests. The authoritative diff contains no Swift or SwiftUI files and no new SwiftUI state, layout measurement, lazy-row store reference, or render-time state mutation.
Full details: Cmux Architecture Rethink
-
Explanation
PASS: The pull request changes only Python CI modules and Python tests. It introduces no Swift, SwiftUI, AppKit, lifecycle, timing, locking, observer, or duplicate Swift entrypoint changes. The Swift architectural rethink rule is therefore not applicable.
Full details: Cmux Swift Auxiliary Window Close Shortcuts
-
Explanation
PASS: The pull request changes only Python files under scripts/ci and tests/. It adds no Swift NSWindow, NSPanel, NSWindowController, SwiftUI Window, or WindowGroup code, so the auxiliary-window shortcut rule is not applicable.
Full details: Cmux Source Artifacts
-
Explanation
All six changed paths are hand-written CI source or tests. The diff adds event classification, failover/janitor logic, and focused test coverage. It adds no logs, screenshots, recordings, temporary or cache directories, dependency checkouts, build output, or broad artifact directories.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 18 functions across 6 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Merge receipt for |
Summary
dep-fleet's ruling (#27521) says: never cancel a started nightly, DEV or promote build. It protects delivery runs, meaning runs that publish, sign, roll back or file things. It never meant that a pull request's test run of a post-publish check can't fail over. Three exemptions read it by workflow path or name instead, so the pull request runs of
nightly-next-canary.ymlandnightly-next-launch-smoke.yml(both trigger onpull_requestfor edits to themselves) were covered too:owned_pool_rescue.never_fails_over()exempted every path inNEVER_FAILOVER_PATHSand any file name containing "nightly", so the open-head failover never moved a pull request run of the canary or launch smoke off a dead label.queue_janitor.PROTECTED_WORKFLOWmatched "nightly" in the run name or path, so the sweep left every canary and smoke run alone, pull request runs included.queue_janitor.reserves_pool()(ci(janitor): nightly notarization follow-ups reserve their Blacksmith macOS label #28364) matched delivery workflows by path only, so queued pull request jobs of the canary and smoke counted asreserved_queued. The 14:16Z snapshot showedreserved_queued: 8onglaeda-side-std-xcode-26.6.One shared predicate fixes all three:
delivery_workflows.delivers(path, event, *names). It is true for a delivery workflow's run (is_delivery_run(), unchanged) from any event except a pull request's (pull_request,pull_request_target,merge_group). An unread event still counts as a delivery event, so a run whose event we can't read keeps its protection.Targetnow records the run's event.never_fails_over(path, event)is true only for a delivery run: a run of aNEVER_FAILOVER_PATHSworkflow or any delivery workflow, from an event that can deliver. Runs ofnightly.yml, DEV daily,release.yml, sign, train, rollback, verdict and feed-watch keep their exemption. Most of those never have a pull request run anyway, and the failover already skips non-PR targets.protected_reason()drops the "nightly" match only for a pull request run of a delivery workflow. Release, publish, notarization, TestFlight and App Store names still protect it.orphan_protected_reason()andreserves_pool()usedelivers().Nothing changes for scheduled, dispatched or pushed runs, nightly-branch runs included: they stay protected and reserved. A started nightly delivery build is still never cancelled.
The diff is small and touches
never_fails_over()and theTargetconstructors only. #28249 (feat-cmux-next) does not touch those lines.Testing
Red on base (
0368f4e1, tests only on top of7ca718f7), green on head:python3 tests/test_delivery_workflows_cancel.py,DeliveryRunTests. Base: 32 errors (deliversmissing). Head: OK (29 tests).python3 tests/test_ci_owned_pool_rescue.py, three tests. A pull request run of the canary or smoke is not excluded. A PR run of the canary or smoke on a dead label is cancelled and re-run. Every delivery run (schedule, dispatch, push, workflow_run, unread event) is still excluded. Base: 2 failures ('cancel' not found in ['pull', 'run']) and 2 errors. Head: OK (403 tests).python3 tests/test_ci_queue_janitor.py,DeliveryRunScopeTests. A PR run of the canary or smoke is unprotected, classifiedstale-prwhen its PR is closed, kept at its open head, and reserves 0. Scheduled, main and nightly-next dispatch runs and a nightly-next push ofnightly.ymlstay protected and reserve 1. Base: 2 failures ('release/nightly/TestFlight/App Store workflow' is not None). Head: OK (253 tests).Observed / Not run
tests/*.pythat importsqueue_janitor,owned_pool_rescueordelivery_workflows(health report, owned pool sources, late placement, pool pickers, starved labels, nightly delivery concurrency, nightly-next feed watch and cancelled tip, run_e2e, app-host product reuse), all pass on head.Changelog
none
Agent session: agent session
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Limits the nightly failover exemption and janitor protection to delivery runs, so a pull request's test run of the nightly canary or launch smoke workflow is treated like any ordinary PR run.
Adds
delivery_workflows.delivers(), true for a delivery workflow's run from any event except a pull request's (pull_request,pull_request_target,merge_group). An unread event still counts as a delivery one, so protection stays when the event can't be read. The rescue'snever_fails_over(), the janitor'sprotected_reason()/orphan_protected_reason(), andreserves_pool()all use it.nightly-next-canary.ymlandnightly-next-launch-smoke.yml; those runs publish, sign, roll back and file nothing, so dep-fleet's ruling (ci(owned-pool rescue): stall-aware failover at an open head only when nothing has started #27521) never protected them.Written for commit 304864d. Summary will update on new commits.
Summary by CodeRabbit