diff --git a/scripts/ci/pr_gate_route.py b/scripts/ci/pr_gate_route.py index a0c49a72e0a..b75aefa06dc 100644 --- a/scripts/ci/pr_gate_route.py +++ b/scripts/ci/pr_gate_route.py @@ -18,10 +18,34 @@ status != completed -> `queued` for longer than --stale-hours ? yes -> cancel, wait for `completed` (bounded), then action=rerun + cancel REFUSED with a terminal marker + -> action=impasse (no retry can succeed) no -> skip (genuinely in flight) completed + success -> skip completed + other -> rerun +The cancel refusal is not one condition but two, and they are told apart by +the refusal REASON -- which is why `cancel_run` returns it instead of a bool. +A transient refusal (rate limit, brief inconsistency) really is retried by the +next sweep. GitHub's two terminal refusals never are (#10928, fourth cause, +measured on #18243): + + - "Cannot cancel a workflow re-run that has not yet queued." (HTTP 409) -- + the run reports `queued` for 7 h with 0 jobs and no `PR gate` check-run, + while `gh run rerun` answers "already running": BOTH levers are refused, + which is the impasse #17680 closes, reached by a route #17680 did not + anticipate. Re-dispatching cannot change either answer. + - "Cannot cancel a workflow run that is completed." -- the status endpoint + and the cancel endpoint disagree about the same run. + +On a terminal refusal the route emits `impasse`, which no job consumes (the +workflow matches only `rerun` and `aggregate_absent`): the point is that the +verdict stops reading like a retry that will happen. It deliberately does NOT +fall through to `aggregate_absent`: that route's safety argument (#16624) is +"no run exists on this SHA, so nothing bearing the required name can ever be +ANDed against" -- and here a run DOES exist. The remedy that creates a fresh +run in the same concurrency group is `gh pr close N && gh pr reopen N`. + The stale threshold must sit above the measured queue latency (scripts/ci/gh_queue_health.py); the default of 2 h follows the issue's proposal (dead attempts measured at 19h30, healthy ones served in minutes). @@ -170,11 +194,41 @@ def classify(run: dict, now: datetime, stale_after: timedelta) -> tuple[str, str return "rerun", f"completed with conclusion {conclusion}" -def cancel_run(repo: str, run_id: int) -> bool: +# GitHub's terminal cancel refusals (#10928 fourth cause). Both mean the run +# can never be cancelled, so "the next sweep retries" would be a false +# promise: the retry issues the same request and reads the same answer. +TERMINAL_REFUSAL_MARKERS = ( + "cannot cancel a workflow re-run that has not yet queued", + "cannot cancel a workflow run that is completed", +) + + +def classify_refusal(reason: str) -> str: + """`terminal` when no retry can ever succeed, `transient` otherwise. + + Anything unrecognised stays `transient`: a marker this function has never + seen is not evidence that the next sweep is wasted, and treating it as + terminal would replace a false retry promise with a false impasse. + """ + lowered = reason.lower() + return ( + "terminal" + if any(marker in lowered for marker in TERMINAL_REFUSAL_MARKERS) + else "transient" + ) + + +def cancel_run(repo: str, run_id: int) -> tuple[bool, str]: + """Attempt the cancel -> (accepted, refusal_reason). + + The reason is returned rather than collapsed into a bool: it is the + discriminating measurement between a refusal the next sweep can retry and + one it can never get past (see `classify_refusal`). + """ completed = _run_gh( ["gh", "api", "-X", "POST", f"repos/{repo}/actions/runs/{run_id}/cancel"] ) - return completed.returncode == 0 + return completed.returncode == 0, (completed.stderr or "").strip() def wait_completed( @@ -257,7 +311,28 @@ def main(argv: list[str] | None = None) -> int: if action == "cancel_rerun": print(f"{tag}{reason}") - if not cancel_run(args.repo, run_id): + accepted, refusal = cancel_run(args.repo, run_id) + if not accepted: + if classify_refusal(refusal) == "terminal": + # Measured, not asserted: how many `PR gate` check-runs this + # SHA carries is what separates "verdict absent" (#10928, the + # syndrome where a green-looking PR is BLOCKED forever) from + # "verdict red". Fail-closed probe, same one the twin guard + # uses -- an unreadable API counts as "one exists". + existing = existing_self_check_runs(args.repo, args.sha) + print( + f"{tag}IMPASSE -- cancel refused for run {run_id} and the " + f"refusal is terminal: {refusal!r}. Both levers are shut " + "(`gh run rerun` answers 'already running'), so 'the next " + "sweep retries' would be a false promise -- action=impasse " + f"instead. 'PR gate' check-runs on {args.sha}: {existing} " + "(0 = verdict ABSENT, the #10928 syndrome). Remedy that " + "creates a FRESH run in the same concurrency group: " + "`gh pr close N && gh pr reopen N` -- do NOT re-dispatch " + "this harness, it re-reads the same two refusals." + ) + _emit("impasse", run_id) + return 0 print(f"{tag}cancel refused for run {run_id} -- skip (next sweep retries)") _emit("skip", run_id) return 0 diff --git a/scripts/tests/test_pr_gate_route.py b/scripts/tests/test_pr_gate_route.py index 8921fa2e4c4..071969bf80b 100644 --- a/scripts/tests/test_pr_gate_route.py +++ b/scripts/tests/test_pr_gate_route.py @@ -110,10 +110,11 @@ def test_attempt_started_at_falls_back_when_never_started(): class FakeGh: """Dispatch gh_api/_run_gh par path ; enregistre les cancel.""" - def __init__(self, run=None, check_runs=(), statuses=()): + def __init__(self, run=None, check_runs=(), statuses=(), cancel_refusal=""): self.run = run self.check_runs = list(check_runs) self.statuses = list(statuses) # consommes par les sondes post-cancel + self.cancel_refusal = cancel_refusal # stderr du POST /cancel self.cancels = [] self.lookups = [] @@ -134,6 +135,10 @@ def run_gh(self, args): self.cancels.append(args[-1]) import types + if self.cancel_refusal: + return types.SimpleNamespace( + returncode=1, stdout="", stderr=self.cancel_refusal + ) return types.SimpleNamespace(returncode=0, stdout="", stderr="") raise AssertionError(f"appel _run_gh inattendu: {args}") @@ -197,6 +202,92 @@ def test_revive_timeout_downgrades_to_skip(monkeypatch, tmp_path): assert fake.cancels # le cancel a bien ete tente +# --- refus de cancel : terminal vs transitoire (#10928 4e cause) ------------- +# +# Mesure 2026-09-28 sur #18243 : la tentative de re-run 2 du run 36452308519 +# est `status=queued` depuis 16:36Z avec 0 job et AUCUN check-run `PR gate`, +# tandis que `gh run rerun` repond « already running » et le POST /cancel +# repond 409. Les DEUX leviers sont fermes : c'est l'impasse que #17680 ferme, +# atteinte par une route que #17680 n'avait pas prevue. Le message historique +# (« next sweep retries ») y est faux -- le sweep rejoue la meme requete. + +# Forme exacte rendue par l'API (stderr de `gh api`), mesuree firsthand. +REFUSAL_RERUN_NOT_QUEUED = ( + 'gh: Cannot cancel a workflow re-run that has not yet queued. (HTTP 409)\n' + '{"message":"Cannot cancel a workflow re-run that has not yet queued.",' + '"documentation_url":"https://docs.github.com/rest/actions/workflow-runs' + '#cancel-a-workflow-run","status":"409"}' +) +REFUSAL_ALREADY_COMPLETED = ( + "Cannot cancel a workflow run that is completed. (HTTP 409)" +) + + +def test_classify_refusal_tells_the_two_apart(): + """Les deux refus terminaux mesures sont `terminal` ; tout le reste -- + y compris un message jamais vu -- reste `transient`.""" + assert route.classify_refusal(REFUSAL_RERUN_NOT_QUEUED) == "terminal" + assert route.classify_refusal(REFUSAL_ALREADY_COMPLETED) == "terminal" + # Controle negatif : un refus inconnu ne doit PAS etre classe terminal -- + # sinon on remplacerait une fausse promesse de retry par une fausse + # impasse, et le sweep renoncerait sur une condition qui se resorbe. + assert route.classify_refusal("HTTP 429: rate limit exceeded") == "transient" + assert route.classify_refusal("") == "transient" + + +def test_terminal_cancel_refusal_emits_impasse(monkeypatch, tmp_path): + """Tentative queued morte + refus terminal -> action=impasse, et le cancel + a bien ete tente (le refus est mesure, pas suppose).""" + fake = FakeGh( + run=run_payload(run_started_at=None, updated_at="2026-09-23T18:52:00Z"), + cancel_refusal=REFUSAL_RERUN_NOT_QUEUED, + ) + rc, action, run_id = _drive(monkeypatch, tmp_path, fake) + assert rc == 0 + assert action == "impasse" + assert run_id == "35895035044" + assert fake.cancels == ["repos/jsboige/CoursIA/actions/runs/35895035044/cancel"] + + +def test_impasse_is_never_a_rerun(monkeypatch, tmp_path): + """L'impasse ne doit pas deguiser un rerun : `gh run rerun` est refuse sur + ce run (« already running »), donc l'emettre relancerait la meme boucle.""" + fake = FakeGh( + run=run_payload(run_started_at=None, updated_at="2026-09-23T18:52:00Z"), + cancel_refusal=REFUSAL_RERUN_NOT_QUEUED, + ) + _rc, action, _rid = _drive(monkeypatch, tmp_path, fake) + assert action != "rerun" + + +def test_transient_cancel_refusal_keeps_the_retry_promise(monkeypatch, tmp_path): + """Controle negatif du chemin : un refus transitoire garde `skip`, qui est + la bonne reponse -- le prochain sweep peut reellement aboutir.""" + fake = FakeGh( + run=run_payload(run_started_at=None, updated_at="2026-09-23T18:52:00Z"), + cancel_refusal="HTTP 429: rate limit exceeded", + ) + _rc, action, _rid = _drive(monkeypatch, tmp_path, fake) + assert action == "skip" + + +def test_impasse_has_no_consumer_in_the_workflow(): + """Propriete de surete : `impasse` doit rester INERTE cote workflow. Un job + qui le consommerait rouvrirait un levier (rerun ou POST) sur une population + ou les deux sont deja refuses -- et le POST rouvrirait le danger d'AND + #11519, puisque contrairement a #16624 un run EXISTE sur ce SHA.""" + data = _load(WORKFLOW) + consumed = { + job.get("if", "") + for job in data["jobs"].values() + } + assert not any("impasse" in cond for cond in consumed), ( + "un job consomme desormais l'action `impasse` : verifier que ce levier " + "n'est pas deja refuse (rerun) et qu'il ne POSTe pas de check-run " + "jumeau sur un SHA qui porte deja un run (#11519)" + ) + + def test_completed_failure_routes_rerun_no_cancel(monkeypatch, tmp_path): fake = FakeGh(run=run_payload(status="completed", conclusion="failure")) _rc, action, _rid = _drive(monkeypatch, tmp_path, fake)