From 15da88bd363bcc98044fba420e08f0a60d6992f5 Mon Sep 17 00:00:00 2001 From: jsboige Date: Wed, 21 Oct 2026 15:25:11 +0200 Subject: [PATCH] fix(gate,#21818): l'auteur d'un dossier est normalise une seule fois (GraphQL vs REST) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Le gate de fermeture lisait l'auteur d'un commentaire en GraphQL, le gate de prevalidation adjointe le lisait en REST. Les deux surfaces ne rendent pas le MÊME login pour un acteur de type `Bot` -- mesure firsthand sur ai-01 (2026-10-21, commentaire `github-actions` sur #21390) : GraphQL `author { __typename login }` : login = "github-actions" (SANS suffixe) REST `user` : login = "github-actions[bot]" (AVEC suffixe) `ACCEPTED_DOSSIER_AUTHORS` est ecrit en orthographe REST : le gate de fermeture refusait donc une App de lane (`comment author 'coursia-lane-po-2026' is not an accepted dossier author`, signale par la lane secretaire), alors que le gate adjoint l'acceptait. La normalisation est portE par un point de passage UNIQUE, `gh_identity.normalize_author_login`, qui remet l'auteur en orthographe REST. La reconstruction du suffixe exige le TYPE de l'acteur (`__typename == "Bot"` en GraphQL, `type == "Bot"` en REST) : un compte UTILISATEUR nomme comme le slug de l'App reste refuse. Un auteur de type inconnu n'est pas un bot -- le doute profite au refus. Le gate de fermeture lit maintenant ses commentaires en REST, qui porte deja l'orthographe et le type ; le gate adjoint passe par la meme normalisation (idempotente sur du REST). Effet de bord corrige au passage : le test `login.endswith("[bot]")` qui ignore les commentaires de bots ne pouvait jamais reconnaitre un bot sous l'orthographe GraphQL. Tests : 49 verts dans `test_check_closure_dossier.py` (dont 6 nouveaux -- controle positif App, controle negatif du compte utilisateur homonyme, non-regression des comptes humains, chemin de lecture reel), 260 verts dans `test_check_adjoint_prevalidation.py` + `test_gh_identity.py`. Verification sur issues reelles : le gate rend KEEP sur #19969 et #18941 via le nouveau chemin REST. Co-Authored-By: Claude Haiku 4.5 (1M context) --- scripts/check_adjoint_prevalidation.py | 15 ++- scripts/check_closure_dossier.py | 43 +++++++-- scripts/gh_identity.py | 71 ++++++++++++++ scripts/tests/test_check_closure_dossier.py | 101 ++++++++++++++++++++ 4 files changed, 219 insertions(+), 11 deletions(-) diff --git a/scripts/check_adjoint_prevalidation.py b/scripts/check_adjoint_prevalidation.py index 33deb2d7713..4e2f63183fc 100644 --- a/scripts/check_adjoint_prevalidation.py +++ b/scripts/check_adjoint_prevalidation.py @@ -2746,10 +2746,18 @@ def _issue_comments(pr: int) -> list[dict[str, Any]]: ]) if not isinstance(rows, list): raise RuntimeError("issue comments response is not a list") + # #21818 : l'auteur est lu en REST ici, donc deja en orthographe REST -- + # mais la normalisation passe par le point de passage UNIQUE + # (`gh_identity.normalize_author_login`), pour que les deux gates + # comparent la MEME forme quelle que soit la surface qui a lu l'auteur + # (GraphQL rend le login d'un `Bot` SANS le suffixe `[bot]`). return [ { "id": row.get("node_id") or row.get("id"), - "author": {"login": (row.get("user") or {}).get("login", "")}, + "author": { + "login": gh_identity.normalize_author_login(row.get("user") or {}), + "type": (row.get("user") or {}).get("type") or "", + }, "createdAt": row.get("created_at"), "body": row.get("body") or "", } @@ -2766,7 +2774,10 @@ def _reviews(pr: int) -> list[dict[str, Any]]: return [ { "id": row.get("node_id") or row.get("id"), - "author": {"login": (row.get("user") or {}).get("login", "")}, + "author": { + "login": gh_identity.normalize_author_login(row.get("user") or {}), + "type": (row.get("user") or {}).get("type") or "", + }, "submittedAt": row.get("submitted_at"), "state": row.get("state"), "commit": {"oid": row.get("commit_id")}, diff --git a/scripts/check_closure_dossier.py b/scripts/check_closure_dossier.py index 00f30766d74..d207a7cdf6e 100644 --- a/scripts/check_closure_dossier.py +++ b/scripts/check_closure_dossier.py @@ -275,22 +275,47 @@ def _open_pr_refs(repo: str, number: int) -> list[int]: return [int(item["number"]) for item in out if item.get("number")] +def _issue_comments(repo: str, number: int) -> list[dict[str, Any]]: + """Commentaires de l'issue, avec un auteur NORMALISE (#21818). + + La lecture se fait en REST (`user`), pas via `gh issue view --json + comments` (GraphQL) : GraphQL rend le login d'un acteur de type `Bot` + SANS le suffixe `[bot]` (`"github-actions"`), REST le rend AVEC + (`"github-actions[bot]"`) et fournit en plus le type de l'acteur + (`user.type`). `ACCEPTED_DOSSIER_AUTHORS` est ecrit en orthographe REST : + sans cette normalisation, une App de lane signe un dossier que le gate + refuse -- symptome mesure : "comment author 'coursia-lane-po-2026' is not + an accepted dossier author" (issue #21818). + + La normalisation passe par `gh_identity.normalize_author_login` : c'est le + point de passage UNIQUE, partage avec le gate de prevalidation adjointe. + """ + rows = gh_json([ + "api", f"repos/{repo}/issues/{number}/comments", "--paginate", + ]) or [] + out = [] + for row in rows: + user = row.get("user") or {} + out.append({ + "author": { + "login": gh_identity.normalize_author_login(user), + "type": user.get("type") or "", + }, + "created_at": row.get("created_at") or "", + "body": row.get("body") or "", + }) + return out + + def load_snapshot(repo: str, number: int) -> dict[str, Any]: """Tout ce que le gate lit sur une issue, en un endroit.""" issue = gh_json([ "issue", "view", str(number), "--repo", repo, - "--json", "number,title,state,labels,comments,createdAt,closedAt", + "--json", "number,title,state,labels,createdAt,closedAt", ]) if not isinstance(issue, dict) or "number" not in issue: raise RuntimeError(f"issue #{number} introuvable") - comments = [ - { - "author": c.get("author") or {}, - "created_at": c.get("createdAt") or "", - "body": c.get("body") or "", - } - for c in (issue.get("comments") or []) - ] + comments = _issue_comments(repo, number) return { "repo": repo, "number": int(issue["number"]), diff --git a/scripts/gh_identity.py b/scripts/gh_identity.py index ff18b430b2f..c44b2347556 100644 --- a/scripts/gh_identity.py +++ b/scripts/gh_identity.py @@ -93,6 +93,77 @@ + tuple(f"{APP_LOGIN_PREFIX}{lane}[bot]" for lane in APP_DOSSIER_LANES) ) +# --- Normalisation de l'auteur lu sur une surface GraphQL ou REST ---------- +# +# Les deux surfaces ne rendent pas le MEME login pour un acteur de type +# `Bot`. Mesure firsthand (2026-10-21, ai-01, commentaire `github-actions` +# sur #21390) : +# +# GraphQL `author { __typename login }` : login = "github-actions" +# (SANS suffixe), __typename = "Bot" +# REST `user` : login = "github-actions[bot]" +# (AVEC suffixe), type = "Bot" +# +# `ACCEPTED_DOSSIER_AUTHORS` est ecrit en orthographe REST : toute lecture +# doit donc etre normalisee AVANT la comparaison. Le gate de fermeture lit +# en GraphQL, le gate de prevalidation adjointe en REST : sans normalisation, +# le premier refuse une App de lane que le second accepte (issue #21818, +# symptome mesure par la lane secretaire : "comment author +# 'coursia-lane-po-2026' is not an accepted dossier author"). +# +# La reconstruction du suffixe s'appuie sur le TYPE de l'acteur, jamais sur la +# forme du login : sans cette exigence, un compte UTILISATEUR nomme comme le +# slug de l'App ("coursia-lane-po-2026", sans suffixe) passerait le gate. +# --------------------------------------------------------------------------- + + +def author_is_bot(author: dict) -> bool: + """L'acteur est-il une App (type `Bot`) ? + + Accepte les deux formes rencontrees : `__typename` (GraphQL) et `type` + (REST). Un auteur dont le type est inconnu n'est PAS un bot : le doute + profite au refus. + """ + if not isinstance(author, dict): + return False + typename = str(author.get("__typename") or "").strip() + if typename: + return typename == "Bot" + atype = str(author.get("type") or "").strip() + if atype: + return atype == "Bot" + return False + + +def normalize_author_login(author: dict) -> str: + """Login d'auteur en orthographe REST (suffixe `[bot]` pour une App). + + Point de passage UNIQUE : les deux gates qui comparent l'auteur d'un + dossier passent par ici, donc une seule definition de la forme canonique. + Un login deja suffixe est renvoye tel quel (idempotent) : la lecture REST + ne change rien, la lecture GraphQL est corrigee. + """ + if not isinstance(author, dict): + return "" + login = str(author.get("login") or "").strip() + if not login: + return "" + if login.endswith("[bot]"): + return login + if author_is_bot(author): + return login + "[bot]" + return login + + +def is_accepted_dossier_author(author: dict) -> bool: + """L'auteur est-il un signataire de dossier accepte ? + + La comparaison se fait sur le login NORMALISE : un dossier signe sous une + App de lane est accepte quelle que soit la surface qui a lu l'auteur. + """ + return normalize_author_login(author) in ACCEPTED_DOSSIER_AUTHORS + + RATE_LIMIT_RE = re.compile(r"rate limit", re.IGNORECASE) diff --git a/scripts/tests/test_check_closure_dossier.py b/scripts/tests/test_check_closure_dossier.py index 59688e36ba3..e46f2f5ff4f 100644 --- a/scripts/tests/test_check_closure_dossier.py +++ b/scripts/tests/test_check_closure_dossier.py @@ -717,3 +717,104 @@ def test_delivering_lanes_refuse_le_point_final_de_phrase(): assert "myia-po-2023:CoursIA-2" in lanes assert "myia-po-2024:CoursIA" in lanes assert not any(ln.endswith(".") for ln in lanes), sorted(lanes) + + +# --- #21818 : l'auteur d'un dossier lu en GraphQL, compare en orthographe REST + + +def test_normalisation_rend_une_app_graphql_en_orthographe_rest(): + """GraphQL rend le login d'un acteur de type `Bot` SANS le suffixe + `[bot]` ; REST le rend AVEC. La normalisation unique remet les deux + surfaces d'accord sur la forme canonique.""" + gh = ccd.gh_identity + assert gh.normalize_author_login( + {"login": "coursia-lane-po-2026", "__typename": "Bot"} + ) == "coursia-lane-po-2026[bot]" + assert gh.normalize_author_login( + {"login": "coursia-lane-po-2026[bot]", "type": "Bot"} + ) == "coursia-lane-po-2026[bot]" + assert gh.normalize_author_login( + {"login": "jsboige", "__typename": "User"} + ) == "jsboige" + # Un auteur dont le type est inconnu n'est pas un bot : le doute profite + # au refus, pas a l'acceptation. + assert gh.normalize_author_login({"login": "coursia-lane-po-2026"}) == ( + "coursia-lane-po-2026") + assert gh.author_is_bot({"login": "x"}) is False + assert gh.author_is_bot({"login": "x", "__typename": "Bot"}) is True + assert gh.author_is_bot({"login": "x", "type": "Bot"}) is True + assert gh.author_is_bot({"login": "x", "__typename": "User"}) is False + + +def test_un_compte_utilisateur_homonyme_est_refuse(): + """Le suffixe n'est reconstruit que si le TYPE de l'acteur est `Bot`. + Un compte UTILISATEUR nomme comme le slug de l'App + ("coursia-lane-po-2026", sans suffixe) doit donc rester refuse : c'est + le controle negatif du correctif.""" + gh = ccd.gh_identity + assert gh.normalize_author_login( + {"login": "coursia-lane-po-2026", "__typename": "User"} + ) == "coursia-lane-po-2026" + assert gh.is_accepted_dossier_author( + {"login": "coursia-lane-po-2026", "__typename": "User"}) is False + assert gh.is_accepted_dossier_author( + {"login": "coursia-lane-po-2026", "__typename": "Bot"}) is True + assert gh.is_accepted_dossier_author( + {"login": "coursia-lane-po-2026[bot]", "type": "Bot"}) is True + + +def test_comptes_humains_non_regresses(): + """Non-regression : le login partage reste accepte, un compte humain + quelconque reste refuse.""" + gh = ccd.gh_identity + assert gh.is_accepted_dossier_author( + {"login": "jsboige", "type": "User"}) is True + assert gh.is_accepted_dossier_author( + {"login": "quelquun-dautre", "type": "User"}) is False + assert gh.is_accepted_dossier_author( + {"login": "coursia-lane-po-2099[bot]", "type": "Bot"}) is False + + +def test_le_gate_accepte_un_dossier_signe_par_une_app_lue_en_rest(monkeypatch): + """Chemin de lecture reel : `_issue_comments` (REST) normalise l'auteur, + et le gate accepte ensuite le dossier signe sous une App de lane.""" + rows = [{ + "user": {"login": "coursia-lane-po-2026[bot]", "type": "Bot"}, + "created_at": "2026-09-20T10:00:00Z", + "body": _dossier_body(), + }] + monkeypatch.setattr(ccd, "gh_json", lambda args: rows) + comments = ccd._issue_comments("o/r", 17900) + assert comments[0]["author"]["login"] == "coursia-lane-po-2026[bot]" + snap = _snapshot(comments=comments) + verdict, errors, _ = evaluate(snap) + assert verdict == "CLOSE", errors + assert errors == [], errors + + +def test_le_gate_refuse_un_compte_utilisateur_homonyme(monkeypatch): + """Meme chemin de lecture, mais l'acteur est un `User` : le suffixe n'est + PAS reconstruit, donc le dossier est refuse.""" + rows = [{ + "user": {"login": "coursia-lane-po-2026", "type": "User"}, + "created_at": "2026-09-20T10:00:00Z", + "body": _dossier_body(), + }] + monkeypatch.setattr(ccd, "gh_json", lambda args: rows) + comments = ccd._issue_comments("o/r", 17900) + assert comments[0]["author"]["login"] == "coursia-lane-po-2026" + snap = _snapshot(comments=comments) + _, errors, _ = evaluate(snap) + assert any( + "is not an accepted dossier author" in e for e in errors + ), errors + + +def test_les_deux_gates_partagent_le_meme_point_de_normalisation(): + """Les deux gates comparent l'auteur au meme ensemble : une seule + definition de la forme canonique, sinon l'un accepterait un dossier que + l'autre refuse (la demi-migration dont #17437 dit le risque).""" + import check_adjoint_prevalidation as cap + assert cap.gh_identity is ccd.gh_identity + assert cap.gh_identity.ACCEPTED_DOSSIER_AUTHORS is ( + ccd.gh_identity.ACCEPTED_DOSSIER_AUTHORS)