Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 44 additions & 6 deletions scripts/pr_gate.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,6 @@
{"failure", "timed_out", "cancelled", "action_required", "stale", "startup_failure"}
)

# GitHub check_run.status values that mean "not finished".
STATUS_PENDING = frozenset({"queued", "in_progress", "waiting", "pending", "requested"})

DEFAULT_SELF_NAME = "PR gate"

# Substring that marks a check as advisory (see rule 6). Matched
Expand Down Expand Up @@ -415,6 +412,18 @@ def dedupe_latest(checks: Sequence[dict]) -> list[dict]:
return [entry[1] for entry in best.values()]


def _pending_label(name: str, status: str, conclusion: str) -> str:
"""Render a pending constituent with its observed status/conclusion couple.

#14976 acceptance 3: a STARVED message that names `X` without saying what X
actually reported is indistinguishable from a real wedge. The frozen record
that cost the 90-minute diagnosis on #14967 (`Detect notebook changes`
[`in_progress/success`]) looked identical to a slow-but-alive check until
the couple was printed.
"""
return f"{name} [{status or 'none'}/{conclusion or 'none'}]"


def classify(
checks: Sequence[dict],
self_name: str = DEFAULT_SELF_NAME,
Expand Down Expand Up @@ -450,6 +459,14 @@ def classify(
NOT that classify mishandles in-flight checks (it waits on them correctly)
but that the *workflow trigger* never asks it to look again -- that trigger
fix lives in pr-gate.yml, not here.

#14976 nuance: `conclusion` is authoritative. A check-run can carry a
terminal conclusion while `status` is still `in_progress` (frozen record,
measured on #14967). Such a check is SETTLED by its conclusion -- never
held in `pending` because of a stale status. `status` only decides when
`conclusion` is null, and then the check is pending regardless (see
``_pending_label`` for why the observed couple is carried into the wait
log and the starvation message).
"""
pending: list[str] = []
bad: list[str] = []
Expand Down Expand Up @@ -480,8 +497,17 @@ def classify(
ok.append(name)
continue

if status in STATUS_PENDING or not conclusion:
pending.append(name)
# #14976: conclusion is authoritative. A check-run can freeze at
# `status=in_progress` while already carrying a terminal conclusion --
# measured on #14967, `Detect notebook changes` sat at
# `in_progress/success` 90 min past its own job's `completed_at`,
# confirmed on two endpoints. The old status-first test (status
# pending OR no conclusion) polled that green job until the budget
# burned and verdicted STARVED on a PR where nothing was red. Status
# only speaks when conclusion is null -- and then the check is pending
# regardless of what `status` claims.
if not conclusion:
pending.append(_pending_label(name, status, conclusion))
elif conclusion in CONCLUSION_OK:
ok.append(name)
elif conclusion in CONCLUSION_BAD:
Expand Down Expand Up @@ -516,6 +542,13 @@ def verdict(pending: Sequence[str], bad: Sequence[str], settled: bool) -> tuple[
unknown), but the driver renders it as CANCELLED, not FAILURE, so the
stale-sweep re-drives the leg instead of a human reading a red lie.

#14976 acceptance 4: each pending constituent renders as ``name [status/
conclusion]`` (see ``_pending_label``), so a green-but-wedged record
(`Detect notebook changes [in_progress/success]`) is distinguishable from
a slow-but-alive one. The message also names the repair gesture -- rerun
the CHILD run carrying the frozen check-run, never the aggregator gate --
because that is what actually clears the wedge (#14967 measured it).

Note (#11751): the wait loop now re-reads the check set one last time at
the deadline; an empty `pending` AND empty `bad` at that point is treated
as `settled=True` BEFORE this function is reached, so this `not settled`
Expand All @@ -528,7 +561,12 @@ def verdict(pending: Sequence[str], bad: Sequence[str], settled: bool) -> tuple[
return 1, "FAIL -- failing checks: " + ", ".join(bad)
if not settled:
if pending:
return 1, "STARVED -- timed out waiting for: " + ", ".join(pending)
return 1, (
"STARVED -- timed out waiting for: " + ", ".join(pending)
+ " -- a constituent showing a terminal conclusion above is "
"wedged, not slow: rerun its CHILD run (gh run rerun <id>), "
"never the gate (#14976)"
)
return 1, (
"FAIL -- timed out with empty wait set (gate bug, see #11751)"
)
Expand Down
61 changes: 57 additions & 4 deletions scripts/tests/test_pr_gate.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,60 @@ def boom(*_args, **_kwargs):
def test_completed_without_conclusion_is_pending_not_pass():
"""`status=completed, conclusion=null` is a transient GitHub state."""
pending, bad, ok, _adv = pr_gate.classify([run("Odd", None)])
assert pending == ["Odd"] and not bad and not ok
assert pending == ["Odd [completed/none]"] and not bad and not ok


# --- #14976 -- frozen check-runs: conclusion is authoritative -----------------
#
# Measured on #14967 (2026-09-06): `Detect notebook changes` (check-run
# 101568709885) sat at `status=in_progress` with `conclusion=success` for
# 90 min past its own job's completed_at, confirmed on two independent
# endpoints. The status-first test (status pending OR no conclusion) polled
# that green job until the 45-min budget burned and the verdict came back
# STARVED on a PR where nothing was red. Conclusion is authoritative; status
# only speaks when conclusion is null.


def test_frozen_check_with_terminal_conclusion_settles_14976():
"""Positive control (#14976 acceptance 2): RED on the pre-fix classify,
GREEN after. The frozen shape -- `status=in_progress` carrying
`conclusion=success` -- must land in `ok`, not `pending`."""
checks = [
run("Detect notebook changes", "success", status="in_progress", rid=1),
run("Lean CI", "success", rid=2),
]
pending, bad, ok, _adv = pr_gate.classify(checks, "PR gate")
assert pending == [] and bad == []
assert ok == ["Detect notebook changes", "Lean CI"]


def test_frozen_check_with_failure_conclusion_is_bad_14976():
"""The frozen shape with a red conclusion must fail fast, not STARVE: a
wedged record carrying `failure` is a real red the operator must see."""
checks = [run("Detect notebook changes", "failure", status="in_progress")]
pending, bad, _ok, _adv = pr_gate.classify(checks, "PR gate")
assert bad == ["Detect notebook changes"] and pending == []


def test_pending_labels_carry_the_observed_couple_14976():
"""Acceptance 3: a pending constituent renders with the status/conclusion
actually observed, else a green-but-wedged check is indistinguishable
from a slow-but-alive one (the 90-minute #14967 diagnosis)."""
checks = [run("Slow CI", None, status="in_progress")]
pending, _bad, _ok, _adv = pr_gate.classify(checks, "PR gate")
assert pending == ["Slow CI [in_progress/none]"]


def test_starved_message_documents_the_child_rerun_repair_14976():
"""Acceptance 4: the repair gesture -- rerun the CHILD run carrying the
frozen check-run, never the aggregator -- lives in the starvation message
the operator actually reads."""
code, msg = pr_gate.verdict(
pending=["Slow CI [in_progress/success]"], bad=[], settled=False
)
assert code == 1
assert msg.startswith("STARVED")
assert "child" in msg.lower()


# --- 2. de-duplication by check name ----------------------------------------
Expand Down Expand Up @@ -175,7 +228,7 @@ def test_the_9858_outage_shape_fails():

A gate that verdicts PASS on this shape is the exact failure mode that let
175 PRs through unpoliced. Both sub-cases must land in `pending` (classify:
`status in STATUS_PENDING or not conclusion`) and the verdict must FAIL --
no conclusion) and the verdict must FAIL --
the bias-to-fail rule (rule 1) applied to the outage, parallel to the #9762
replay above (which pins the failure-side, this pins the no-verdict-side).
"""
Expand Down Expand Up @@ -530,7 +583,7 @@ def test_pending_legacy_status_blocks_settling():
"started_at": "2026-08-07T00:00:00Z", "id": 9},
]
pending, _bad, _ok, _adv = pr_gate.classify(checks)
assert pending == ["legacy/lint"]
assert pending == ["legacy/lint [in_progress/none]"]


# --- fork exemption (issue #10072) -------------------------------------------
Expand Down Expand Up @@ -970,7 +1023,7 @@ def test_inflight_required_check_settles_to_pass_on_reroll():

# Rollup 1: guard still in flight -> pending -> verdict timed out (FAIL).
pending1, bad1, ok1, _adv = pr_gate.classify([inflight], pr_gate.DEFAULT_SELF_NAME)
assert pending1 == [name] and not bad1 and not ok1
assert pending1 == [name + " [in_progress/none]"] and not bad1 and not ok1
code1, _msg1 = pr_gate.verdict(pending1, bad1, settled=False)
assert code1 == 1

Expand Down
Loading