From 2a654c3aa5b1fb63f3e117016faa7400698ea9cd Mon Sep 17 00:00:00 2001 From: jsboige Date: Mon, 14 Sep 2026 05:57:52 +0200 Subject: [PATCH 1/2] fix(scripts,#16103): nits organ -- SHA hexa-francais, rebase-amend fichiers intacts, crash cp1252 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Defaut 1 : un token 100% lettres a-f (« effacee ») etait lu comme SHA cite -- exiger au moins un chiffre en plus de la lettre existante. Defaut 2 : un rebase-amend voide une levee valide (arbre different par construction) alors que les fichiers de la PR sont byte-identiques -- comparer les blobs des CHEMINS de la PR via listings recursifs d'arbres (+1 appel/SHA), artefact reason="pr_files_unchanged" non bloquant, fail-closed (doute = refus conserve). Reintroduction documentee de l'echappatoire retiree en #15566 : #16103 (14/09) postdate le ruling et la demande en forme exacte. Defaut 3 : crash cp1252 dans _print_unevaluated apres le verdict -- _ensure_utf8_stdout() aux entrees gate/audit (rc ne depend plus de la page de code). Tests : 12 nouveaux (repro + controles fail-closed + symetrie vraie reserve), famille nits 569 verts. Live : gate 15440 (repro crash) RC=0. Closes #16103 Co-Authored-By: Claude Sonnet 5 --- scripts/check_unaddressed_nits.py | 117 ++++++++- .../test_check_unaddressed_nits_16103.py | 231 ++++++++++++++++++ 2 files changed, 342 insertions(+), 6 deletions(-) create mode 100644 scripts/tests/test_check_unaddressed_nits_16103.py diff --git a/scripts/check_unaddressed_nits.py b/scripts/check_unaddressed_nits.py index bbfb643ab3..40bafb8c2e 100644 --- a/scripts/check_unaddressed_nits.py +++ b/scripts/check_unaddressed_nits.py @@ -3246,16 +3246,26 @@ def collect_followup_lifts(pr_data: dict, cutoff: datetime, def _cited_shas(body: str) -> set[str]: - """SHAs cites dans un corps : 7-40 hex, avec AU MOINS une lettre. + """SHAs cites dans un corps : 7-40 hex, avec AU MOINS une lettre ET AU + MOINS un chiffre. Un token 100% numerique de 7+ chiffres (une date 20260830, un run-id) est hex-compatible mais n'est quasi jamais un SHA -- l'exiger lettree evite de partir resoudre une date cote serveur pour rien. + + #16103 defaut 1 : un token 100% LETTRES de a-f est la meme classe de + bruit en francais -- « effacee », « effacees », « deface » satisfont + le motif hexa et se font lire comme des empreintes (levee d'ai-01 du + 2026-09-14T02:45:56Z sur #16022 rendue « cite effacee ... absent des + commits »). Une empreinte Git de 7+ caracteres sans AUCUN chiffre est + astronomiquement improbable ((6/16)^7 ~ 1e-4 au format court) ; + l'exiger chiffre supprime la classe entiere. """ out: set[str] = set() for m in _SHA_CITED.finditer((body or "").lower()): tok = m.group(0) - if any(ch in "abcdef" for ch in tok): + if (any(ch in "abcdef" for ch in tok) + and any(ch.isdigit() for ch in tok)): out.add(tok) return out @@ -3380,6 +3390,19 @@ def _resolve_absent_sha_state(data: dict, cap: int = 5) -> dict[str, dict]: # blobs par chemin -- 3 appels API par SHA contre 1 -- et rouvrirait une # surface fail-open sur un organe de merge-gate. Le critere d'arbre suffit # au remede demontre (#15492) ; le rebase retombe sur le refus conservateur. +# +# #16103 defaut 2 (2026-09-14, POSTDATE ce ruling) : le rebase-amend au +# geste ordinaire (`gh pr update-branch` compris) retombait TOUJOURS dans +# le refus conservateur -- levee valide voidee alors que les fichiers de +# la PR etaient byte-pour-byte identiques, seule la base avait avance. +# L'echappatoire est REINTRODUITE en forme exacte et fail-closed : +# comparaison des blobs des CHEMINS de la PR (pas de l'arbre entier) via +# les listings recursifs d'arbres -- +1 appel par SHA absent et +1 pour +# la tete, loin des 3/SHA redoutes ; tout doute (arbre introuvable, +# listing tronque, chemin absent, fichiers de PR inconnus) retombe sur +# le refus. Le motif d'impression « fichiers de la PR inchangés », +# vestige inatteignable depuis le retrait, redevient atteignable -- +# c'est lui que #16103 croyait deja vivant. def _pr_head_oid(data: dict) -> str: @@ -3395,12 +3418,57 @@ def _pr_head_oid(data: dict) -> str: return "" +def _tree_blob_map(tree_sha: str) -> dict[str, str] | None: + """Carte chemin -> blob du listing recursif d'un arbre (#16103 defaut 2). + + None = DOUTE (appel echoue OU listing tronque par l'API) : l'appelant + retombe sur le refus conservateur, jamais sur une identite non prouvee. + """ + if not tree_sha: + return None + try: + payload = gh_json(["api", + f"repos/{REPO}/git/trees/{tree_sha}?recursive=1"]) + except subprocess.CalledProcessError: + return None + if payload.get("truncated"): + return None + return {e["path"]: e["sha"] for e in payload.get("tree") or [] + if e.get("type") == "blob" and e.get("path") and e.get("sha")} + + +def _pr_files_unchanged(pr_files, cited_blobs, head_blobs) -> bool: + """#16103 defaut 2 : les fichiers de la PR sont-ils byte-identiques ? + + Vrai UNIQUEMENT si chaque chemin liste par la PR porte le meme blob au + commit rembobine et a la tete -- preuve exacte (egalite d'OID de blob), + pas heuristique. Faux sur tout doute (cartes absentes, chemin absent + d'une carte) : le refus conservateur reste la voie par defaut d'un + organe de merge-gate (#15566). + """ + if not pr_files or not cited_blobs or not head_blobs: + return False + for f in pr_files: + path = f.get("path") if isinstance(f, dict) else None + if not path: + return False + if cited_blobs.get(path) is None or head_blobs.get(path) is None: + return False + if cited_blobs[path] != head_blobs[path]: + return False + return True + + def _attach_absent_sha_context(data: dict) -> None: """Resolution serveur du contexte SHA, AVANT analyse (qui reste pure). - Assemble les deux vues que `analyse` consulte : messages (rattachement - #13639) et arbres des commits rembobines plus arbre de la tete - (#15556) -- un appel reseau par SHA, plus un pour la tete. + Assemble les vues que `analyse` consulte : messages (rattachement + #13639), arbres des commits rembobines plus arbre de la tete + (#15556) -- un appel reseau par SHA, plus un pour la tete. #16103 + defaut 2 : cartes chemin->blob des arbres cites et de la tete, + UNIQUEMENT quand la vue porte les fichiers de la PR (gate et + analyse_pr ; l'audit retro ne les demande pas et ne peut donc jamais + emprunter cette voie -- fail-closed). """ state = _resolve_absent_sha_state(data) data["_absent_sha_messages"] = {s: v["message"] for s, v in state.items() @@ -3417,6 +3485,13 @@ def _attach_absent_sha_context(data: dict) -> None: except subprocess.CalledProcessError: head_tree = "" data["_head_tree"] = head_tree + if state and data.get("files"): + data["_head_blobs"] = _tree_blob_map(head_tree) + data["_absent_sha_blobs"] = { + s: _tree_blob_map(v.get("tree") or "") for s, v in state.items()} + else: + data["_head_blobs"] = None + data["_absent_sha_blobs"] = {} def can_lift(comment: dict) -> bool: @@ -4122,6 +4197,19 @@ def _lift_eligible(lift_author: str, nit_author: str, if artifact is None: artifact = (sha, "same_tree") continue + # #16103 defaut 2 : arbre DIFFERENT (rebase ordinaire) + # mais fichiers de la PR byte-identiques -> ARTEFACT, + # pas reserve. Fail-closed : sans preuve d'identite + # (cartes absentes, chemin manquant, fichiers de PR + # inconnus), le refus conservateur est conserve. + cited_blobs = (pr_data.get("_absent_sha_blobs") + or {}).get(sha) + if _pr_files_unchanged(pr_data.get("files"), + cited_blobs, + pr_data.get("_head_blobs")): + if artifact is None: + artifact = (sha, "pr_files_unchanged") + continue refused = sha if tree and head_tree: refused_known_change = True @@ -4499,7 +4587,7 @@ def _lift_eligible(lift_author: str, nit_author: str, FIELDS = ("number,title,body,mergedAt,author,comments,reviews,commits,url," - "state,headRefOid") + "state,headRefOid,files") # `commits` porte une connection `authors` par commit : sur un `gh pr list` large, # GraphQL depasse son plafond de 500 000 noeuds. L'audit retro liste donc SANS @@ -4593,7 +4681,23 @@ def analyse_pr(pr: int) -> dict: dismissed_improperly=improper_dismissals(pr)) +def _ensure_utf8_stdout() -> None: + """#16103 defaut 3 : le verdict ne doit jamais dependre de la page de + code de la console. Sous cp1252 (Windows), l'impression d'un commentaire + a relire portant un caractere hors page (`→` levait UnicodeEncodeError + dans _print_unevaluated) crashait APRES le verdict -- rc=1 faux rouge + pour tout consommateur scripte alors que l'analyse disait OK. + Idempotent ; silencieux sous un stdout non reconfigurable (buffers + de test). + """ + try: + sys.stdout.reconfigure(encoding="utf-8", errors="replace") + except (AttributeError, ValueError, OSError): + pass + + def gate(pr: int, as_json: bool) -> int: + _ensure_utf8_stdout() data = gh_json(["pr", "view", str(pr), "--repo", REPO, "--json", FIELDS]) # #13639 + #15556 : resolution serveur du contexte SHA (messages, # arbres rembobines, arbre de tete), AVANT analyse (qui reste pure). @@ -4628,6 +4732,7 @@ def gate(pr: int, as_json: bool) -> int: def audit(limit: int, search: str | None = None) -> int: + _ensure_utf8_stdout() cmd = ["pr", "list", "--repo", REPO, "--state", "merged", "--limit", str(limit), "--json", LIST_FIELDS] if search: diff --git a/scripts/tests/test_check_unaddressed_nits_16103.py b/scripts/tests/test_check_unaddressed_nits_16103.py new file mode 100644 index 0000000000..fd275c3d5d --- /dev/null +++ b/scripts/tests/test_check_unaddressed_nits_16103.py @@ -0,0 +1,231 @@ +"""Tests pour les trois defauts de l'organe B.0 (#16103). + +Defaut 1 -- un mot francais compose uniquement de lettres hexadecimales +(« effacee ») etait lu comme une empreinte citee : la levee d'ai-01 du +2026-09-14T02:45:56Z sur #16022 etait rendue « cite effacee ... absent +des commits ». Remede : un token cite doit porter AU MOINS un chiffre en +plus de l'exigence existante d'au moins une lettre. + +Defaut 2 -- un rebase-amend (geste ordinaire, `gh pr update-branch` +compris) voide une levee valide : l'arbre du commit cite differe +mecaniquement de la tete apres rebase, donc tree_differs -> refus +BLOQUANT, alors que les fichiers de la PR sont byte-pour-byte identiques. +Remede : quand les arbres different, comparer les blobs des CHEMINS de +la PR (cartes chemin->blob des listings recursifs) ; identite exacte -> +artefact non bloquant reason="pr_files_unchanged" (le motif d'impression +existait deja, inatteignable depuis le retrait #15566). Fail-closed : +tout doute retombe sur le refus conservateur. + +Defaut 3 -- crash cp1252 dans _print_unevaluated : le verdict OK etait +imprime puis l'organe crashait (rc=1 faux rouge) sur un commentaire a +relire contenant `→`. Remede : _ensure_utf8_stdout() aux entrees qui +impriment (gate, audit). +""" +import importlib.util +import io +import sys +from datetime import datetime, timezone +from pathlib import Path + +import pytest + +HERE = Path(__file__).resolve().parent +CHECK_PATH = HERE.parent / "check_unaddressed_nits.py" + +spec = importlib.util.spec_from_file_location("check_unaddressed_nits", CHECK_PATH) +mod = importlib.util.module_from_spec(spec) +sys.modules["check_unaddressed_nits"] = mod +spec.loader.exec_module(mod) + +MERGED = datetime(2026, 9, 14, 12, 0, tzinfo=timezone.utc) + +CITED = "230b1194" # 8 hex, lettres ET chiffre -> reste un SHA cite apres fix + +LIFT = { + "author": {"login": "myia-ai-01"}, + "createdAt": "2026-09-13T19:32:00Z", + "body": "Levee de la reserve -- verifie firsthand sur 230b1194, " + "l'arbre est propre.", +} + + +def run(**extra): + data = { + "number": 1234, + "title": "t", + "author": {"login": "jsboige"}, + "comments": [LIFT], + "reviews": [], + "commits": [{"oid": "f" * 40, + "committedDate": "2026-09-13T20:00:00Z"}], + "files": [{"path": "a.py"}, {"path": "b.md"}], + } + data.update(extra) + return mod.analyse(data, [], MERGED) + + +# --------------------------------------------------------------------------- +# Defaut 1 -- extraction d'empreintes +# --------------------------------------------------------------------------- + +def test_defaut1_mot_francais_hexa_nest_pas_un_sha(): + """Repro #16103 : « effacee » (e-f-f-a-c-e-e, 7 lettres toutes dans + [0-9a-f]) satisfaisait le motif hexa et etait extrait comme SHA.""" + cited = mod._cited_shas( + "Levee -- verifie, le contenu effacee par le push est propre.") + assert "effacee" not in cited + assert cited == set() + + +def test_defaut1_vrai_sha_avec_chiffre_conserve(): + body = f"Levee de la reserve -- verifie firsthand sur {CITED}." + assert mod._cited_shas(body) == {CITED} + + +def test_defaut1_token_lettres_seules_rejete_avec_controles(): + """Toute la classe : lettres-seules rejetees, numeriques-seules + rejetees (regle existante), melange lettres+chiffre accepte.""" + assert mod._cited_shas("deadbeef") == set() # lettres seules + assert mod._cited_shas("20260830") == set() # chiffres seuls (regle anterieure) + assert mod._cited_shas("effacee2") == {"effacee2"} # melange : reste candidat + + +# --------------------------------------------------------------------------- +# Defaut 2 -- rebase-amend : identite de CONTENU des fichiers de la PR +# --------------------------------------------------------------------------- + +def _rebase_context(cited_blobs, head_blobs): + """Fixtures du chemin rebase : SHA resolu rattache a la PR (#1234), + arbres DIFFERENTS (rebase), cartes de blobs maitrisees.""" + return dict( + _absent_sha_messages={CITED: "rebase: base rafraichie (#1234)"}, + _absent_sha_trees={CITED: "treeCITED"}, + _head_tree="treeHEAD", + _absent_sha_blobs={CITED: cited_blobs}, + _head_blobs=head_blobs, + ) + + +def test_defaut2_rebase_fichiers_pr_intacts_est_artefact_non_bloquant(): + """Repro #16103 defaut 2 : apres rebase, arbre DIFFERENT mais blobs + des chemins de la PR identiques -> rewind_artifact (non bloquant), + plus voided_lift.""" + res = run(**_rebase_context( + {"a.py": "blob1", "b.md": "blob2"}, + {"a.py": "blob1", "b.md": "blob2"})) + assert res["voided_lifts"] == [] + assert len(res["rewind_artifacts"]) == 1 + art = res["rewind_artifacts"][0] + assert art["sha"] == CITED + assert art["reason"] == "pr_files_unchanged" + + +def test_defaut2_vrai_changement_de_contenu_reste_bloquant(): + """Symetrique (question posee par l'issue) : un vrai rembobinage de + contenu -- un blob de la PR a change -- reste une reserve bloquante.""" + res = run(**_rebase_context( + {"a.py": "blob1", "b.md": "blob2"}, + {"a.py": "blobMODIFIE", "b.md": "blob2"})) + assert res["rewind_artifacts"] == [] + assert len(res["voided_lifts"]) == 1 + assert res["voided_lifts"][0]["sha"] == CITED + assert res["voided_lifts"][0]["tree_differs"] is True + + +def test_defaut2_doute_carte_absente_retombe_sur_le_refus(): + """Fail-closed : cartes de blobs introuvables (API muette, listing + tronque) -> refus conservateur, jamais une identite non prouvee.""" + res = run(**_rebase_context( + {"a.py": "blob1", "b.md": "blob2"}, + None)) # _head_blobs = None : doute + assert res["rewind_artifacts"] == [] + assert len(res["voided_lifts"]) == 1 + + +def test_defaut2_chemin_absent_dune_carte_retombe_sur_le_refus(): + """Fail-closed : un chemin de la PR manquant a l'une des cartes (fichier + ajoute par le rebase) n'est pas une identite prouvable.""" + res = run(**_rebase_context( + {"a.py": "blob1"}, # b.md absent cote cite + {"a.py": "blob1", "b.md": "blob2"})) + assert res["rewind_artifacts"] == [] + assert len(res["voided_lifts"]) == 1 + + +def test_defaut2_sans_vue_fichiers_pr_pas_de_voie_permissive(): + """L'audit retro (sans `files`) ne peut JAMAIS emprunter l'echappatoire : + pas de cartes, pas d'identite, refus conservateur inchange.""" + res = run(files=None, **_rebase_context( + {"a.py": "blob1"}, {"a.py": "blob1"})) + assert res["rewind_artifacts"] == [] + assert len(res["voided_lifts"]) == 1 + + +def test_defaut2_meme_arbre_reste_le_premier_critere(): + """Regression #15556 : push muet (arbre identique) -> same_tree, + independamment des cartes de blobs.""" + res = run( + _absent_sha_messages={CITED: "wake-commit (#1234)"}, + _absent_sha_trees={CITED: "treeHEAD"}, + _head_tree="treeHEAD", + _absent_sha_blobs={}, _head_blobs=None) + assert res["voided_lifts"] == [] + assert res["rewind_artifacts"][0]["reason"] == "same_tree" + + +# --------------------------------------------------------------------------- +# Defaut 3 -- crash cp1252 de l'impression +# --------------------------------------------------------------------------- + +UNEVAL_RESULT = { + "unevaluated": [{ + "author": "reviewer", + "at": "2026-09-14T03:00:00Z", + "body": "commentaire a relire portant → et du texte", + "after_last_commit": False, + }], + "unevaluated_total": 1, +} + + +def test_defaut3_repro_crash_cp1252_avant_remade(): + """Repro : sous stdout cp1252, l'impression du commentaire non evalue + leve UnicodeEncodeError (le verdict OK etait deja imprime).""" + old = sys.stdout + sys.stdout = io.TextIOWrapper(io.BytesIO(), encoding="cp1252") + try: + with pytest.raises(UnicodeEncodeError): + mod._print_unevaluated(UNEVAL_RESULT) + finally: + sys.stdout = old + + +def test_defaut3_ensure_utf8_stdout_guerit_limpression(): + """Remede : apres _ensure_utf8_stdout(), la meme impression passe et + encode le caractere en utf-8.""" + old = sys.stdout + buf = io.BytesIO() + sys.stdout = io.TextIOWrapper(buf, encoding="cp1252") + try: + mod._ensure_utf8_stdout() + mod._print_unevaluated(UNEVAL_RESULT) # ne doit pas lever + sys.stdout.flush() + assert "→".encode("utf-8") in buf.getvalue() + finally: + sys.stdout = old + + +def test_defaut3_ensure_utf8_stdout_silencieux_si_non_reconfigurable(): + """Un stdout sans reconfigure (capture exotique, StringIO de test) ne + fait pas echouer l'organe : le remede est silencieux, jamais bloquant.""" + + class NoReconfigure(io.StringIO): + def reconfigure(self, *a, **k): + raise AttributeError("no reconfigure") + + old = sys.stdout + sys.stdout = NoReconfigure() + try: + mod._ensure_utf8_stdout() # ne doit pas lever + finally: + sys.stdout = old From 49303ebfc011af569a7d2b5a37178ebb63c0e1da Mon Sep 17 00:00:00 2001 From: jsboige Date: Mon, 14 Sep 2026 10:12:17 +0200 Subject: [PATCH 2/2] fix(nits,#16103): taux du defaut 1 corrige -- (6/16)^7 ~ 1e-3, pas 1e-4 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Le chiffre contredisait sa propre formule : (6/16)^7 = 1.0428e-3. Mesure au meme moment : 0 occurrence de "6/16" sur origin/main, 1 sur cette tete (l. 3261), et dans ce diff la ligne est un AJOUT -- c'est donc un fix dans le scope de la PR, pas une issue de suivi. "astronomiquement" retire au passage : a 1e-3 (~1 fois sur 960) l'adverbe surestime le taux, et la classe est de toute facon portee par des mots francais reels (« effacee »), pas par une collision aleatoire. Co-Authored-By: Claude Sonnet 5 --- scripts/check_unaddressed_nits.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/check_unaddressed_nits.py b/scripts/check_unaddressed_nits.py index 082d3a954a..53941b9579 100644 --- a/scripts/check_unaddressed_nits.py +++ b/scripts/check_unaddressed_nits.py @@ -3258,7 +3258,7 @@ def _cited_shas(body: str) -> set[str]: le motif hexa et se font lire comme des empreintes (levee d'ai-01 du 2026-09-14T02:45:56Z sur #16022 rendue « cite effacee ... absent des commits »). Une empreinte Git de 7+ caracteres sans AUCUN chiffre est - astronomiquement improbable ((6/16)^7 ~ 1e-4 au format court) ; + improbable ((6/16)^7 ~ 1e-3 au format court) ; l'exiger chiffre supprime la classe entiere. """ out: set[str] = set()