JMR-dev/LibreMail
1
0
Fork 0
Code Issues 40 Pull Requests 2 Actions Packages Projects Releases 2 Wiki Activity

fix(ci): let any strictly-higher PR reclaim a broken/draft run (#342)

Browse Source 
Restore (and extend to drafts) the old bash's broken-reclaim behaviour that the
initial Python refactor had dropped. runs_to_cancel now cancels an OTHER PR's
active/queued runs when EITHER:
  (a) THIS PR is P0 and that PR is strictly-lower (reclaim every lower runner); OR
  (b) that PR is broken/draft (effective priority 10) and THIS PR is strictly-higher
      (effective priority < 10) — a wasted run any ready PR may reclaim.

P1-P9 still never bump a *normal* (non-broken/draft) lower run; a broken/draft PR
(P10) preempts nothing (nothing is strictly-lower than the bottom, and the
equal-or-higher invariant means a P10 never cancels another P10). Self / main-push /
equal-or-higher invariants unchanged.

Updates the module docstring + ci.yml comments (the "only P0 preempts" wording
becomes: P0 preempts everything strictly-lower; additionally, any strictly-higher PR
preempts a broken/draft run) and the job step/permission/needs comments. Adds unit
tests: P3 reclaims a broken P10 run and a draft P10 run; P3 does not bump a normal P5
run; a P10 self preempts nothing; plus an end-to-end P5-reclaims-draft-then-waits
scenario. 37 unit tests pass; ci.yml parses clean; --dry-run shows a P3 cancelling a
draft (and broken) run while still yielding to a higher P1.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
JMR-dev
2026-07-04 22:19:01 -05:00
co-authored by Claude Opus 4.8
parent ab41d8f24b
commit a34db54b97
3 changed files with 99 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/scripts/test_traffic_control.py
+47 -9
View File
@@ -3,10 +3,11 @@
"""Unit tests for the pure decision core of traffic_control.py (no network).
Covers: priority resolution (P-label / broken / draft / default P5), PASS 1
preemption (only P0 cancels strictly-lower; P1-P9 never bump; self / main /
equal-or-higher never cancelled), PASS 2 hold-back (yield to strictly-higher with
an active run; same-level running-first then oldest-first), and a few end-to-end
decision scenarios."""
preemption (P0 reclaims all strictly-lower; ANY higher PR reclaims a broken/draft
lower run; P1-P9 never bump a *normal* lower run; self / main / equal-or-higher
never cancelled), PASS 2 hold-back (yield to strictly-higher with an active run;
same-level running-first then oldest-first), and a few end-to-end decision
scenarios."""
from __future__ import annotations
@@ -63,17 +64,40 @@ class EffectivePriorityTests(unittest.TestCase):
class RunsToCancelTests(unittest.TestCase):
def test_non_p0_self_cancels_nothing(self):
def test_non_p0_self_does_not_bump_normal_lower_run(self):
# P1-P9 never preempt a *normal* strictly-lower run — they yield instead.
me = pr(1, ["P1"])
others = [pr(2, ["P5"], status=tc.RUNNING, run_ids=[200])]
self.assertEqual(tc.runs_to_cancel(me, [me, *others]), [])
def test_non_p0_self_never_cancels_even_broken(self):
# Behaviour change vs the old bash (see module/PR notes): P1-P9 no longer
# reclaim a broken target's runner — only P0 preempts.
def test_non_p0_self_reclaims_broken_lower_run(self):
# Any higher-priority PR (not just P0) may reclaim a broken target's runner.
me = pr(1, ["P3"])
broken = pr(2, ["broken"], status=tc.RUNNING, run_ids=[200])
self.assertEqual(tc.runs_to_cancel(me, [me, broken]), [])
self.assertEqual(tc.runs_to_cancel(me, [me, broken]), [200])
def test_non_p0_self_reclaims_draft_lower_run(self):
# A draft is not merge-ready — its run is likewise reclaimable by any higher PR.
me = pr(1, ["P3"])
draft = pr(2, [], draft=True, status=tc.QUEUED, run_ids=[200])
self.assertEqual(tc.runs_to_cancel(me, [me, draft]), [200])
def test_broken_self_does_not_cancel_equal_broken(self):
# Both effective P10 — the equal-or-higher invariant still forbids cancelling.
me = pr(1, ["broken"])
peer = pr(2, ["broken"], status=tc.RUNNING, run_ids=[200])
self.assertEqual(tc.runs_to_cancel(me, [me, peer]), [])
def test_bottom_self_preempts_nothing(self):
# A broken/draft PR (P10) is the bottom: nothing is strictly-lower, so it
# cancels neither a higher (P5) nor an equal (P10) run.
me = pr(1, [], draft=True) # P10
prs = [
me,
pr(2, ["P5"], status=tc.RUNNING, run_ids=[200]), # higher
pr(3, ["broken"], status=tc.RUNNING, run_ids=[300]), # equal P10
]
self.assertEqual(tc.runs_to_cancel(me, prs), [])
def test_p0_cancels_strictly_lower_active_runs(self):
me = pr(1, ["P0"])
@@ -203,6 +227,20 @@ class EndToEndDecisionTests(unittest.TestCase):
self.assertEqual(dec.cancel_run_ids, ())
self.assertTrue(dec.proceed)
def test_p5_reclaims_draft_then_waits_behind_higher(self):
# A non-P0 PR can BOTH reclaim a broken/draft lower run (PASS 1) AND still
# yield to a strictly-higher PR (PASS 2) in the same evaluation.
me = pr(40, ["P5"], status=tc.RUNNING, run_ids=[4000])
prs = [
me,
pr(41, ["P2"], status=tc.RUNNING, run_ids=[4100]), # higher — blocks
pr(42, [], draft=True, status=tc.RUNNING, run_ids=[4200]), # draft — reclaimed
]
dec = tc.decide(me, prs)
self.assertEqual(list(dec.cancel_run_ids), [4200])
self.assertFalse(dec.proceed)
self.assertEqual([b.number for b in dec.blockers], [41])
class SnapshotParsingTests(unittest.TestCase):
def test_from_json_label_objects_and_fields(self):
.github/scripts/traffic_control.py
+30 -16
View File
@@ -19,9 +19,10 @@ ORDER OF OPERATIONS (issue #342)
1. Effective priority orders everything: the lowest-numbered `P0`-`P9` label present
(P0 = highest), default `P5` if none. A `broken` OR `draft` PR is effectively P10
(bottom, below P9), overriding any P0-P9 label.
2. PASS 1 - preemption: **only a P0 (emergency) preempts.** A P0 cancels the
in-progress / queued CI runs of ALL strictly-lower OTHER open PRs to reclaim their
runners. P1-P9 never bump a lower run mid-flight.
2. PASS 1 - preemption: a strictly-lower OTHER PR's in-progress / queued run is
cancelled iff THIS PR is P0 (an emergency reclaims ALL lower runners) OR the target
is broken/draft (a wasted run any higher-priority PR may reclaim). P1-P9 never bump
a *normal* lower run mid-flight — only a P0 does that.
3. PASS 2 - bounded hold-back: a non-P0 PR yields (cancels nothing) to any strictly-
higher-priority OTHER PR that has an active/queued run, and — among its OWN
priority level — to any PR ordered ahead of it (running-first, then oldest by
@@ -169,20 +170,29 @@ def runs_to_cancel(
*,
self_run_id: int | None = None,
) -> list[int]:
"""PASS 1. Run ids to cancel — **empty unless THIS PR is P0**. A P0 preempts the
active (running/queued) runs of every strictly-lower OTHER PR. Invariants: never
cancel self (by number or run id), never cancel an equal-or-higher-priority PR."""
if effective_priority(this_pr) != TOP_PRIORITY:
return [] # only P0 preempts; P1-P9 never bump
"""PASS 1. Run ids to cancel. A strictly-lower OTHER PR's active (running/queued)
run is cancelled iff keeping it running is wasteful, i.e. EITHER:
* THIS PR is P0 — an emergency reclaims every strictly-lower runner now; OR
* the target is broken/draft (effective priority 10) — its run can't merge /
isn't merge-ready, so ANY higher-priority PR may reclaim its runner.
P1-P9 never cancel a *normal* strictly-lower run — they yield in PASS 2 instead.
Invariants: never cancel self (by number or run id), never cancel an
equal-or-higher-priority PR (only strictly-lower, prio > self)."""
self_prio = effective_priority(this_pr)
to_cancel: list[int] = []
seen: set[int] = set()
for pr in all_prs:
if pr.number == this_pr.number:
continue # never cancel self
if effective_priority(pr) <= TOP_PRIORITY:
continue # only strictly-lower (skip other P0s)
target_prio = effective_priority(pr)
if target_prio <= self_prio:
continue # only strictly-lower (skip equal-or-higher)
if pr.run_status not in ACTIVE:
continue # nothing running/queued to cancel
# Strictly lower: preemptible iff we're P0 OR the target is broken/draft
# (a bottom, priority-10, wasted run that any higher PR may reclaim).
if self_prio != TOP_PRIORITY and target_prio < BOTTOM_PRIORITY:
continue # P1-P9 don't bump a *normal* lower run
for rid in pr.run_ids:
if self_run_id is not None and rid == self_run_id:
continue # never cancel our own run
@@ -377,13 +387,14 @@ def run_live() -> int:
_log(f"This PR #{self_number} effective priority: {priority_label(self_prio)} "
"(P0 = highest/emergency, P9 = lowest, broken/draft = bottom).")
# ── PASS 1: PREEMPTION (only a P0 self) ──────────────────────────────────
# ── PASS 1: PREEMPTION (P0 reclaims all lower; anyone reclaims broken/draft) ──
to_cancel = runs_to_cancel(this_pr, all_prs, self_run_id=self_run_id)
if not to_cancel:
if self_prio == TOP_PRIORITY:
_log("P0 emergency — no strictly-lower active runs to cancel.")
else:
_log("Not P0 — no preemption (P1-P9 never cancel a lower run mid-flight).")
_log("No preemptible runs (P1-P9 only reclaim broken/draft lower runs; "
"none active).")
else:
cancelled = 0
for rid in to_cancel:
@@ -448,11 +459,14 @@ def run_dry(text: str) -> int:
dec = decide(this_pr, all_prs, self_run_id=self_run_id)
_log(f"This PR #{dec.self_number} effective priority: "
f"{priority_label(dec.self_priority)}")
if dec.self_priority == TOP_PRIORITY:
_log(f"PASS 1 (preemption): P0 — cancel run ids: "
f"{list(dec.cancel_run_ids) or '(none active)'}")
if dec.cancel_run_ids:
why = ("P0 emergency (reclaims all strictly-lower)"
if dec.self_priority == TOP_PRIORITY
else "reclaiming broken/draft lower runs")
_log(f"PASS 1 (preemption): {why} — cancel run ids: "
f"{list(dec.cancel_run_ids)}")
else:
_log("PASS 1 (preemption): not P0 — no cancellations.")
_log("PASS 1 (preemption): nothing to cancel.")
if dec.proceed:
_log("PASS 2 (hold-back): PROCEED — no blockers.")
else:
.github/workflows/ci.yml
+22 -17
View File
@@ -34,29 +34,34 @@ jobs:
# P0 PREEMPTS: it cancels the in-progress / queued CI runs of ALL strictly-
# LOWER-priority OTHER open PRs to grab their runners immediately. A preempted
# PR simply re-runs on its next push / autoupdate rebase. P0 is the ONLY
# priority that preempts — P1–P9 never cancel a lower run mid-flight.
# priority that preempts a *normal* lower run — P1–P9 never bump those (a
# higher PR may still reclaim a broken/draft lower run — see below).
#
# • P1–P9 = YIELD WITHOUT BUMPING. They NEVER cancel a lower-priority run that is
# already going — a higher-priority PR does not evict it, it just takes the next
# free slot. Mechanism: a bounded hold-back. This job defers (up to
# • P1–P9 = YIELD WITHOUT BUMPING a *normal* lower run. They do NOT cancel a
# normal lower-priority run already going — a higher-priority PR does not evict
# it, it just takes the next free slot (it MAY still reclaim a broken/draft
# lower run — see below). Mechanism: a bounded hold-back. This job defers (up to
# HOLD_BACK_BUDGET_SECONDS, kept well under timeout-minutes) while any strictly-
# higher-priority OTHER open PR still has an active/queued CI run, and — within
# its OWN priority level — while any peer is ordered ahead of it (an in-flight
# run keeps its place; then oldest createdAt first). It proceeds the moment it
# is at the front, or when the budget elapses (a PR never blocks itself).
#
# • `broken` / `draft` = BOTTOM (effective P10). Always yields, never preempts.
# A maintainer applies `broken` to a stuck/failing PR to deprioritise it below
# everything so others aren't blocked behind it; a draft isn't merge-ready, so
# it likewise waits behind every ready PR. Only a P0 may cancel a bottom PR's
# run (the same strictly-lower rule as any other target).
# • `broken` / `draft` = BOTTOM (effective P10). Always yields, never preempts —
# and because its run is wasted (a broken PR can't merge; a draft isn't merge-
# ready), ANY higher-priority PR (not just P0) MAY cancel that run to reclaim
# its runner (still the strictly-lower rule: broken/draft is the bottom, so any
# ready PR outranks it). A maintainer marks a stuck/failing PR `broken` to drop
# it below everything so others aren't blocked behind it AND may reclaim its
# runner; a draft behaves the same until it is marked ready for review.
#
# Hard safety invariants, enforced in the script:
# • never cancels a run on main / a push event (the gh query filters
# --event pull_request and drops headBranch == main);
# • never cancels THIS PR's own run (skips self by PR number + run id);
# • never cancels an equal-or-higher-priority PR (only strictly-lower, prio > self);
# • P1–P9 cancel NOTHING — they only wait (bounded), then proceed.
# • P1–P9 never bump a *normal* lower run (they only reclaim broken/draft) —
# otherwise they just wait (bounded), then proceed.
#
# Honest limitation: GitHub Actions has no native priority queue and assigns
# runners roughly FIFO, so the hold-back is a BEST-EFFORT head-start, not a hard
@@ -76,7 +81,7 @@ jobs:
timeout-minutes: 6 # hard backstop; the P1–P9 hold-back budget stays well under this
permissions:
contents: read # check out .github/scripts/traffic_control.py
actions: write # cancel lower-priority runs (P0 emergencies)
actions: write # cancel lower-priority runs (P0 emergencies + broken/draft reclaim)
pull-requests: read # read PR P0–P9 labels + draft state
env:
GH_TOKEN: ${{ github.token }}
@@ -94,13 +99,13 @@ jobs:
# gh is auto-configured from GH_TOKEN / GH_REPO; python3 is preinstalled on the
# runner. The script guards every API call and always exits 0 (belt-and-braces
# with continue-on-error), so it can never fail or block CI.
- name: Apply runner priority (P0 preempts; P1–P9 hold back)
- name: Apply runner priority (P0/broken/draft preempt; P1–P9 hold back)
continue-on-error: true
run: python3 .github/scripts/traffic_control.py
debug-build:
name: Debug build
needs: traffic-control # order after runner-priority orchestration (P0 preempts; P1–P9 hold-back)
needs: traffic-control # order after runner-priority orchestration (P0/broken/draft preempt; P1–P9 hold-back)
# x86_64: Linux-arm64 runners can't set up this SDK — android-actions/setup-android's sdkmanager
# fails (exit 1) on the android-37.0 preview platform, and the emulator package has no arm64-Linux
# build. Build/unit-test results are host-arch-independent anyway (R8/AGP/JVM); real arm64
@@ -137,7 +142,7 @@ jobs:
unit-tests:
name: Unit tests
needs: traffic-control # order after runner-priority orchestration (P0 preempts; P1–P9 hold-back)
needs: traffic-control # order after runner-priority orchestration (P0/broken/draft preempt; P1–P9 hold-back)
runs-on: ubuntu-latest
steps:
- name: Check out source
@@ -194,7 +199,7 @@ jobs:
static-analysis:
name: Static analysis
needs: traffic-control # order after runner-priority orchestration (P0 preempts; P1–P9 hold-back)
needs: traffic-control # order after runner-priority orchestration (P0/broken/draft preempt; P1–P9 hold-back)
runs-on: ubuntu-latest
steps:
- name: Check out source
@@ -232,7 +237,7 @@ jobs:
e2e:
name: E2E
needs: traffic-control # order after runner-priority orchestration (P0 preempts; P1–P9 hold-back)
needs: traffic-control # order after runner-priority orchestration (P0/broken/draft preempt; P1–P9 hold-back)
runs-on: ubuntu-latest
strategy:
fail-fast: false
@@ -343,7 +348,7 @@ jobs:
# 37 into the main `e2e` matrix and delete this job.
e2e-preview:
name: E2E (API 37 preview)
needs: traffic-control # order after runner-priority orchestration (P0 preempts; P1–P9 hold-back)
needs: traffic-control # order after runner-priority orchestration (P0/broken/draft preempt; P1–P9 hold-back)
runs-on: ubuntu-latest
timeout-minutes: 35
env: