diff --git a/scripts/coordination/merge_ready.py b/scripts/coordination/merge_ready.py index 1f30c86400..d00b85521e 100644 --- a/scripts/coordination/merge_ready.py +++ b/scripts/coordination/merge_ready.py @@ -53,9 +53,15 @@ Comportement : - DRY-RUN par defaut (imprime ce qui serait merge et pourquoi chaque autre PR est skippee) ; ``--apply`` merge reellement. ``--max N`` - (defaut 15) plafonne les merges par run (disjoncteur) ; le run - S'ARRETE sur la premiere erreur inattendue (rc d'un outil hors codes - documents, erreur d'API) -- jamais de merge en aveugle. + (defaut 15) plafonne les merges par run (disjoncteur). Une erreur + inattendue ATTRIBUABLE A UNE PR (reponse d'outil illisible, rc hors + contrat pour cette seule PR) est journalisee ``run-error`` pour elle et + le balayage CONTINUE (#17672 point 3 : une PR bizarre ne gele plus + l'evaluation des suivantes) ; une erreur de PORTEE GENERALE -- jeton + refuse, quota d'API, reseau injoignable, cf ``PASS_WIDE_ERROR_MARKERS`` + -- ARRETE le run, car la repeter sur chaque PR restante ne dirait rien + de plus. ``rc=1`` est rendu des qu'une PR a erreur, isolee ou non, et le + bilan nomme leur nombre. Jamais de merge en aveugle. - Jeton : chaque sous-processus gh recoit ``GH_TOKEN`` epingle depuis ``gh auth token --user myia-ai-01``, resolu UNE fois au depart ; jamais ``gh auth switch``. Jeton irresolu -> exit 2. @@ -147,13 +153,54 @@ class CannotRunError(Exception): class UnexpectedError(Exception): - """Erreur inattendue d'un outil ou de l'API -- le run doit s'arreter.""" + """Erreur inattendue d'un outil ou de l'API, non prevue par le contrat. + + Depuis #17672 (point 3), elle n'arrete plus le run par principe : le + balayage distingue une erreur **attribuable a la PR** en cours (reponse + illisible pour elle, rc hors contrat pour elle) -- journalisee + ``run-error`` et le balayage continue -- d'une erreur de **portee + generale** (jeton, quota, reseau), qui arrete tout (cf + ``is_pass_wide``). Un ECHEC DE MERGE garde son propre disjoncteur + (``MergeFailedError``) : il ne se confond pas avec ces deux cas. + """ class MergeFailedError(Exception): """La commande de merge a echoue -- arret du run (disjoncteur).""" +# Marqueurs d'une erreur de PORTEE GENERALE : ce qui frappe tous les appels de +# la meme facon ne doit pas etre isole par PR, sinon une panne de jeton ou de +# quota se lit comme une collection de ``run-error`` attribuees a des PRs +# innocentes (#17672, point 3). Liste volontairement courte et litterale : un +# texte non reconnu fait ISOLER l'erreur (le balayage continue, rc=1, la PR +# est nommee), jamais l'inverse -- arreter le run sur un motif devine rendrait +# le balayage dependant d'une devinette. +PASS_WIDE_ERROR_MARKERS = ( + "bad credentials", + "requires authentication", + "rate limit", + "http 401", + "http 403", + "could not resolve host", + "no such host", + "connection refused", + "connection reset", + "timed out", +) + + +def is_pass_wide(exc: BaseException) -> bool: + """L'erreur frappe-t-elle TOUTE la passe (jeton, quota, reseau) ou une seule PR ? + + Le texte cherche est celui que les appels gh renseignent avec le stderr de + l'outil (cf ``fetch_pr_view``, ``run_gate``), donc un refus d'authentification + ou un quota epuise y figure tel que gh l'a ecrit. + """ + text = str(exc).lower() + return any(marker in text for marker in PASS_WIDE_ERROR_MARKERS) + + # --- runner injectable -------------------------------------------------------- @@ -693,9 +740,14 @@ def emit_output( merged = sum(1 for v in results if v.merged) would = sum(1 for v in results if v.verdict == "would-merge") skipped = sum(1 for v in results if v.verdict == "skipped") + errored = sum(1 for v in results if v.verdict == "run-error") print( f"bilan : {len(results)} evaluee(s), {merged} merge(s), " f"{would} would-merge, {skipped} skip(s)" + # Une erreur isolee ne s'annonce par aucune ligne « arret » : sans ce + # compte, un run qui a continue malgre une PR en erreur se lirait comme + # un run propre (#17672 point 3). + + (f", {errored} erreur(s) isolee(s)" if errored else "") ) if stopped_reason: print(f"arret : {stopped_reason}") @@ -776,9 +828,17 @@ def run(argv: list[str] | None = None, runner: Runner | None = None) -> int: errored = PRVerdict(pr, None, "run-error", str(exc), False) results.append(errored) append_journal(journal_path, errored) - stopped_reason = f"unexpected-error:{exc}" exit_code = 1 - break + if is_pass_wide(exc): + # Toute la passe est touchee : repeter l'echec sur chacune des + # PRs restantes ne dirait rien de plus. + stopped_reason = f"unexpected-error:{exc}" + break + # Erreur attribuable a CETTE PR : elle est nommee et journalisee, + # le balayage continue (#17672 point 3). `stopped_reason` reste + # None -- le run ne s'est PAS arrete, l'ecrire ici serait un + # constat faux (le bilan, lui, compte les erreurs isolees). + continue emit_output(args, results, stopped_reason, exit_code) return exit_code diff --git a/scripts/tests/test_merge_ready.py b/scripts/tests/test_merge_ready.py index f0d1d3940a..24a4e9c58a 100644 --- a/scripts/tests/test_merge_ready.py +++ b/scripts/tests/test_merge_ready.py @@ -98,6 +98,7 @@ def __init__( nits_rc: int = 0, pulls: list[dict] | None = None, merge_rc: int = 0, + gate_stderr: str = "", ): self.token = token self.token_rc = token_rc @@ -108,6 +109,9 @@ def __init__( self.nits_rc = nits_rc self.pulls = pulls or [{"mergeable_state": "clean", "head": {"sha": HEAD}}] self.merge_rc = merge_rc + # stderr du gate : c'est lui qui porte un motif de portee generale + # (jeton refuse, quota) quand le gate echoue POUR TOUTE la passe. + self.gate_stderr = gate_stderr self.calls: list[tuple[list[str], dict | None]] = [] self.sleeps: list[float] = [] @@ -156,7 +160,7 @@ def run(self, cmd: list[str], env: dict | None = None) -> mr.RunResult: "errors": [], } ) - return mr.RunResult(self.gate_rc, payload, "") + return mr.RunResult(self.gate_rc, payload, self.gate_stderr) if len(c) > 1 and "check_unaddressed_nits.py" in c[1]: return mr.RunResult(self.nits_rc, "", "") raise AssertionError("commande non scriptee : " + " ".join(c)) @@ -416,16 +420,64 @@ def test_max_arrete_le_run(tmp_path): assert [row["pr"] for row in lines] == [101] -def test_erreur_inattendue_arrete_le_run(tmp_path): - # rc 5 du gate : hors codes documents {0,1,2,3} -> arret, exit 1, et la - # PR suivante n'est pas touchee. - runner = ScriptedRunner(prs=(201, 202), gate_rc=5) +def test_erreur_de_portee_generale_arrete_le_run(tmp_path): + # #17672 point 3 : le fail-closed est PRESERVE pour ce qui frappe toute la + # passe -- ici un jeton refuse dans le gate (marqueur « bad credentials »). + # rc 5 du gate : hors codes documents {0,1,2,3} -> arret, exit 1, et la PR + # suivante n'est pas touchee : la repeter ne dirait rien de plus. + runner = ScriptedRunner( + prs=(201, 202), gate_rc=5, gate_stderr="gh: Bad credentials (HTTP 401)" + ) rc, lines, _ = run_organ(tmp_path, runner, extra=("--apply",)) assert rc == 1 assert lines[-1]["verdict"] == "run-error" assert not any("202" in flat for flat in runner.flat()) +def test_erreur_dune_pr_est_isolee_et_le_balayage_continue(tmp_path, capsys): + """#17672 point 3 : une erreur attribuable a UNE PR ne gele plus les autres. + + Falsification : avant le correctif, le `break` de la boucle emportait le + balayage entier -- la PR suivante n'etait meme pas evaluee et le run + s'arretait sur la premiere PR mal formee. Ici la vue de 201 est illisible + (`gh pr view 201 : la reponse n'est pas un objet`), 202 est une PR normale. + """ + runner = ScriptedRunner(prs=(201, 202), views={201: ["pas", "un", "objet"]}) + rc, lines, _ = run_organ(tmp_path, runner, extra=("--apply",)) + + assert rc == 1, "une erreur isolee reste un incident : rc=1" + assert [row["pr"] for row in lines] == [201, 202] + assert lines[0]["verdict"] == "run-error" + assert "n'est pas un objet" in lines[0]["reason"], lines[0]["reason"] + # La PR SUIVANTE est bien evaluee puis mergee : c'est tout l'objet du point 3. + assert lines[1]["verdict"] == "merged", lines[1] + assert any("202" in flat for flat in runner.flat()) + + out = capsys.readouterr().out + # Le run ne s'est PAS arrete : publier « arret » serait un constat faux. + assert "arret :" not in out + # ...mais l'erreur isolee doit etre visible, sinon elle disparait du rapport. + assert "1 erreur(s) isolee(s)" in out, out + + +def test_is_pass_wide_classe_par_le_texte_de_l_outil(): + # Classification pure, sans boucle : un motif reconnu = portee generale ; + # tout le reste = attribuable a la PR (donc isole). Le sens de l'erreur par + # defaut compte : un texte inconnu ne doit JAMAIS arreter le balayage. + assert mr.is_pass_wide( + mr.UnexpectedError("gh pr view 9 rc=1 : API rate limit exceeded") + ) + assert mr.is_pass_wide( + mr.UnexpectedError("gh pr list rc=1 : could not resolve host: api.github.com") + ) + assert not mr.is_pass_wide( + mr.UnexpectedError("gate PR 9 rc=5 hors contrat : (sans message)") + ) + assert not mr.is_pass_wide( + mr.UnexpectedError("gh pr view 9 : la reponse n'est pas un objet") + ) + + def test_merge_echoue_arrete_le_run(tmp_path): # un merge refuse est une erreur inattendue : jamais de merge en aveugle runner = ScriptedRunner(prs=(301, 302), merge_rc=1)