From cba9f3ab442ed9d1cec254c742defee07815215a Mon Sep 17 00:00:00 2001 From: jsboige Date: Tue, 3 Nov 2026 22:29:42 +0100 Subject: [PATCH 1/2] Fix(guard,#22299): juger l'arbre de fusion dans la porte d'index twin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit La porte d'index twin compare **deux revisions prises separement** (la base et la tete). La base lue est celle du moment de la lecture : une collision introduite sur `main` **entretemps** par une autre PR n'y est pas visible. Mesure du 2026-11-03 : #23832 (po-2025, fusion 2026-11-03T00:20:34Z) a alloue les ordinals 0024 a 0027 sur `search-05-geneticalgorithms`, et #24010 (po-2024, fusion 2026-11-03T20:32:42Z) a alloue les memes 0024 a 0026 -- la seconde PR a donc ete mergee **apres** la premiere, et `main` porte aujourd'hui trois prefixes dupliques sur cette paire. **Le nouvel organ** : `check_twin_index_collisions.py --merged` calcule l'**arbre de fusion** (`git merge-tree --write-tree base tete`) et juge ce qu'il porte. C'est la seule revision qui repond a la question du merge : « `main` sera-t-il rouge apres ce merge ? ». Le predicat bloque uniquement ce que le MERGE INTRODUIT : un doublon de l'arbre de fusion qui n'existait pas dans la base sous les memes noms. Une dette que la base porte deja et que la tete REPAIRE (la PR de renumerotation) ne bloque pas -- sinon la PR qui supprime la dette ne pourrait plus passer. **Le câblage** : `merge_ready.py` 5bis appelle l'organe avec `--merged`. Un doublon que le merge introduit fait sauter le merge (motif `twin-collision-on-merge:/`), et un arbre de fusion ILLISIBLE (merge en conflit) fait sauter aussi : « je n'ai pas pu lire » n'est pas « c'est propre ». Le gate CI (`twin-index-collision-gate.yml`) garde sa base au push : c'est une porte bloquante fleet-wide, et passer sa base a `main` courant change sa semantique. La decision revient au coordinateur ; le merge est desormais couvert par le jugement d'arbre de fusion. **Tests** : 8 tests du mode `--merged` (predicat pur + integration sur un vrai depot : introduction, dette reparee, index libre, aggravation, merge en conflit) et 3 tests de `merge_ready` (appel en `--merged`, collision introduite par le merge, arbre illisible). Aucun carnet modifie -> aucune re-execution due (C.2/C.3). Part of #22299 Co-Authored-By: Claude Haiku 5.5 --- scripts/coordination/merge_ready.py | 52 ++++- .../check_twin_index_collisions.py | 136 ++++++++++++ .../test_twin_index_collisions_merged.py | 209 ++++++++++++++++++ scripts/tests/test_merge_ready.py | 53 +++++ 4 files changed, 442 insertions(+), 8 deletions(-) create mode 100644 scripts/notebook_tools/tests/test_twin_index_collisions_merged.py diff --git a/scripts/coordination/merge_ready.py b/scripts/coordination/merge_ready.py index 5d9e1ec0536..01dc735ef90 100644 --- a/scripts/coordination/merge_ready.py +++ b/scripts/coordination/merge_ready.py @@ -884,6 +884,15 @@ def twin_collision_reason( mergee gagne, la seconde renumerote. Avertissement seul, le merge continue (le gate CI bloque de toute facon si la seconde arrive avant renumerotation). + + Le jugement le plus tranchant est celui de l'ARBRE DE FUSION (`--merged`) : + il repond a la seule question qui compte au merge -- « `main` sera-t-il + rouge apres ce merge ? ». Le jugement base+tete compare deux revisions + prises separement, et la base lue est celle du MOMENT DE LA LECTURE ; + une collision introduite sur `main` par une autre PR entretemps n'y est + pas visible, alors qu'elle est dans l'arbre de fusion (mesure du + 2026-11-03 : #23832 et #24010 ont alloue les memes ordinals 0024-0026 + sur `search-05-geneticalgorithms`, et les deux PRs sont passees). """ touched = any( str((row or {}).get("path") or "").startswith(TWIN_REGISTRY_PREFIX) @@ -899,19 +908,46 @@ def twin_collision_reason( return "twin-collision-unreadable:fetch" res = runner.run( [sys.executable, str(TWIN_PATH), "--repo", str(REPO_ROOT), - "--base", "origin/main", "--head", head, "--json"] + "--base", "origin/main", "--head", head, "--merged", "--json"] ) + # Le rapport JSON est lu MEME quand l'organe rend 0 : le mode `--merged` + # peut etre ILLISIBLE (merge en conflit) alors que le jugement classique + # ne bloque rien -- et « je n'ai pas pu lire » n'est pas « c'est propre ». + # Lecture JSON tolérante : si le champ `verdict` est absent (organe + # anterieur a #18683), on retombe sur le motif historique + # ``twin-index-collision`` pour rétro-compat. + try: + data = json.loads(res.stdout) + except (json.JSONDecodeError, ValueError): + data = None + if isinstance(data, dict): + # --- ARBRE DE FUSION (le jugement qui repond a la question du merge) + # Le jugement base+tete dit ce que la base et la tete portent + # separement. L'arbre de fusion dit ce que le MERGE rend visible -- + # c'est le seul verdict qui repond a « `main` sera-t-il rouge apres + # ce merge ? ». Il est juge AVANT le classement ON-MAIN/MULTI-PR : + # un doublon que le merge INTRODUIT ne se classe pas (il n'existe + # dans aucune revision prise isolement), et le laisser passer parce + # que `cross_ref` est vide laisserait merger la PR qui casse `main`. + merged_reasons = data.get("merged_reasons") or [] + if merged_reasons: + first = merged_reasons[0] + return ( + "twin-collision-on-merge:%s/%s -- %s" + % (first.get("pair"), first.get("index"), + first.get("gist") + or "renumeroter le fichier que la tete ajoute vers le " + "premier index libre de la paire par `git mv` PUR") + ) + if data.get("merged_unreadable"): + # « je n'ai pas pu lire » n'est pas « c'est propre » : l'arbre de + # fusion n'est pas definissable (merge en conflit). Skip dur -- + # la PR sera rejugee apres rebasage. + return "twin-collision-unreadable:merged" if res.returncode == 0: return None if res.returncode == 1: # L'organe a detecte une collision ; on la classifie par verdict. - # Lecture JSON tolérante : si le champ `verdict` est absent (organe - # anterieur a #18683), on retombe sur le motif historique - # ``twin-index-collision`` pour rétro-compat. - try: - data = json.loads(res.stdout) - except (json.JSONDecodeError, ValueError): - return "twin-index-collision" if not isinstance(data, dict): return "twin-index-collision" cross_ref = data.get("cross_ref") or [] diff --git a/scripts/notebook_tools/check_twin_index_collisions.py b/scripts/notebook_tools/check_twin_index_collisions.py index 0d2297175ea..ac417dc0390 100644 --- a/scripts/notebook_tools/check_twin_index_collisions.py +++ b/scripts/notebook_tools/check_twin_index_collisions.py @@ -286,6 +286,75 @@ def gate_verdict(cross: list, intra: list, *, base_ref: str) -> dict: return {"fail": bool(reasons), "reasons": reasons} +def merged_gate_reasons(base_reg: dict, merged_reg: dict) -> list: + """Raisons portees par l'ARBRE DE FUSION : ce que le MERGE rend visible. + + Fonction pure : prend les deux registres deja lus. Le registre de + l'arbre de fusion dit ce que `main` portera **apres** le merge -- la + seule revision qui repond a « `main` sera-t-il rouge apres ce merge ? ». + Le registre de la base dit ce que `main` porte **avant**. + + Un doublon de l'arbre de fusion ne bloque que s'il n'existait PAS dans la + base sous les memes noms : + * dette de la base, inchangee par le merge -> la PR la REPAIRE, elle + doit passer (une PR de renumerotation ne peut pas etre bloquee par + la dette qu'elle supprime) ; + * un nom que le merge AJOUTE a un index deja double, ou un index que + le merge rend double -> le MERGE INTRODUIT la collision, elle bloque. + + Le predicat est donc : `len(noms fusion) >= 2` ET `noms fusion - noms + base` non vide. + """ + reasons: list = [] + for pair in sorted(set(merged_reg) | set(base_reg)): + base_pair = base_reg.get(pair, {}) + merged_pair = merged_reg.get(pair, {}) + for index in sorted(merged_pair): + names = set(merged_pair[index]) + if len(names) < 2: + continue + added = names - set(base_pair.get(index, ())) + if not added: + # Dette de la base, inchangee : le merge ne l'aggrave pas. + continue + reasons.append({ + "kind": "merged_ref", + "pair": pair, + "index": index, + "names": sorted(names), + "added": sorted(added), + "gist": ( + "l'arbre de fusion porte deux noms a l'index %s dans " + "'%s' (%s) : le MERGE rend `main` rouge sur le garde " + "d'unicite. Renumeroter le fichier que la tete ajoute " + "(%s) vers le premier index libre de la paire par " + "`git mv` PUR (aucun contenu a toucher, aucune " + "re-execution)." + % (index, pair, ", ".join(sorted(names)), + ", ".join(sorted(added))) + ), + }) + return reasons + + +def merge_tree_sha(base: str, head: str, repo: Path) -> str: + """SHA de l'arbre resultat du MERGE de `base` et `head`. + + `git merge-tree --write-tree` calcule le merge sans toucher l'index ni + l'arbre de travail. Il echoue (rc != 0) quand le merge CONFLIT : c'est + le seul cas ou l'arbre de fusion n'est pas definissable, et l'appelant + en fait un skip (« je n'ai pas pu lire » n'est pas « c'est propre »). + + Necessite git >= 2.38. En dessous, `merge-tree` ne connait pas + `--write-tree` : l'appelant recoit alors une GitError et applique la + meme politique de skip. + """ + out = _git(repo, "merge-tree", "--write-tree", base, head).strip() + if not out: + raise GitError("merge-tree --write-tree : aucune sortie") + return out.splitlines()[0].strip() + + def ordinal_correction_gist(pair, index, *, by_ref, base_ref): """Geste de correction nomme dans l'issue #18683, isole pour les tests. @@ -326,6 +395,12 @@ def main(argv: list | None = None) -> int: "collision d'index (les collisions heritees de la " "base ne bloquent pas : la PR qui les repare doit " "passer). Impose --head.") + ap.add_argument("--merged", action="store_true", + help="juge l'ARBRE DE FUSION (base x tete) au lieu de la " + "seule union base+tete : c'est la seule revision qui " + "dit si `main` sera rouge apres le merge. Ne bloque " + "que les doublons que le MERGE INTRODUIT. Impose " + "--head.") ap.add_argument("--repo", default=None, help="racine du depot (defaut: auto)") ap.add_argument("--registry", default=REGISTRY_REL, help="chemin du registre, relatif a la racine") @@ -336,6 +411,12 @@ def main(argv: list | None = None) -> int: "tete de la PR ; sans tete, il n'y a rien a confronter).") print("VERDICT: INDETERMINE (0 comparaison effectuee)") return 2 + if args.merged and not args.head: + print("ERREUR: --merged exige --head (l'arbre de fusion se calcule " + "entre la base et la tete ; sans tete, il n'y a rien a " + "fusionner).") + print("VERDICT: INDETERMINE (0 comparaison effectuee)") + return 2 repo = Path(args.repo).resolve() if args.repo else Path( _git(Path.cwd(), "rev-parse", "--show-toplevel").strip()) @@ -379,6 +460,36 @@ def main(argv: list | None = None) -> int: # une PR sans condamner la flotte (#23208). gate = gate_verdict(cross, intra, base_ref=args.base) if args.gate else None + # Arbre de FUSION : la seule revision qui repond a « `main` sera-t-il + # rouge apres ce merge ? ». L'union base+tete ne repond pas a la + # question : elle voit l'etat de la base AU PUSH, et une collision + # introduite sur `main` ENTRETEMPS par une autre PR lui est invisible. + # L'arbre de fusion, lui, porte les DEUX et revele ce que le merge + # rend visible (mesure du 2026-11-03 : #23832 et #24010 ont alloue les + # memes ordinals 0024-0026 et sont passees tous les deux). + merged_ref = None + merged_reasons: list = [] + merged_unreadable = None + if args.merged: + try: + merged_ref = merge_tree_sha(args.base, args.head[0], repo) + merged_reg = registry_of_ref(merged_ref, repo, args.registry) + merged_reasons = merged_gate_reasons(refs.get(args.base, {}), + merged_reg) + except GitError as exc: + # Merge en conflit : l'arbre de fusion n'est pas definissable. + # « je n'ai pas pu lire » n'est pas « c'est propre » -- le motif + # est expose dans le rapport, et l'appelant decide (la porte de + # merge applique le skip ; le gate CI se contente de le dire). + merged_unreadable = str(exc) + if gate is not None and merged_reasons: + deja_vus = {(r["pair"], r["index"]) for r in gate["reasons"]} + for r in merged_reasons: + if (r["pair"], r["index"]) in deja_vus: + continue # deja signale par le jugement base+tete + gate["reasons"].append(r) + gate["fail"] = bool(gate["reasons"]) + unreadable_note = (", ".join("%s illisible" % r for r in sorted(unreadable)) or "aucune") if args.json: @@ -400,6 +511,10 @@ def main(argv: list | None = None) -> int: "gate": bool(args.gate), "gate_fail": bool(gate and gate["fail"]), "gate_reasons": (gate or {}).get("reasons", []), + "merged_checked": bool(args.merged), + "merged_ref": merged_ref, + "merged_unreadable": merged_unreadable, + "merged_reasons": merged_reasons, }, indent=2, sort_keys=True)) else: print("revisions comparees : %d (%s ; %s)" @@ -448,6 +563,27 @@ def main(argv: list | None = None) -> int: print(" effet du merge) : relancer avec --in-tree pour en faire un") print(" verdict bloquant, ou laisser le garde CI le porter.") + if args.merged: + print("") + if merged_unreadable: + print("ARBRE DE FUSION : ILLISIBLE") + print(" %s" % merged_unreadable) + print(" (le merge est en conflit : la revision fusionnee n'est") + print(" pas definissable. « je n'ai pas pu lire » n'est pas") + print(" « c'est propre » -- la porte de merge applique un skip.)") + else: + print("ARBRE DE FUSION : %s" % merged_ref[:12]) + if merged_reasons: + print(" %d collision(s) INTRODUITE(S) par le merge :" + % len(merged_reasons)) + for r in merged_reasons: + print(" %s / index %s (%s)" + % (r["pair"], r["index"], r["kind"])) + print(" noms en jeu : %s" % ", ".join(r["names"])) + print(" geste : %s" % r["gist"]) + else: + print(" aucune collision introduite par le merge") + # UNE seule ligne de verdict : un « OK » qui suivrait un « DOUBLON » se # lirait comme un acquittement du doublon. rc = exit_code(cross, intra, in_tree=args.in_tree) diff --git a/scripts/notebook_tools/tests/test_twin_index_collisions_merged.py b/scripts/notebook_tools/tests/test_twin_index_collisions_merged.py new file mode 100644 index 00000000000..065ad987a78 --- /dev/null +++ b/scripts/notebook_tools/tests/test_twin_index_collisions_merged.py @@ -0,0 +1,209 @@ +"""Tests du mode `--merged` de `check_twin_index_collisions.py`. + +Le mode `--merged` juge l'ARBRE DE FUSION (base x tete) plutot que la +seule union base+tete. C'est la seule revision qui repond a la question qui +compte au merge : « `main` sera-t-il rouge apres ce merge ? ». + +Trois predicats sont testes : + * le merge INTRODUIT un doublon -> il bloque (le concurrent est arrive sur + `main` entre le push et le merge ; l'union base+tete, elle, ne le voit pas) ; + * le doublon est une DETTE DE LA BASE, que la tete REPAIRE -> il ne bloque + pas (une PR de renumerotation ne peut pas etre bloquee par la dette + qu'elle supprime) ; + * le merge n'introduit rien -> il ne bloque pas. + +Le test d'integration rejoue le cas historique mesure le 2026-11-03 : +#23832 et #24010 ont alloue les memes ordinals 0024-0026 sur +`search-05-geneticalgorithms`, et les deux PRs sont passees. +""" + +from __future__ import annotations + +import json +import subprocess +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +from check_twin_index_collisions import ( # noqa: E402 + GitError, + merged_gate_reasons, + merge_tree_sha, +) + +ORG = "scripts/notebook_tools/twin_pairs.d" +PAIR = "app" + + +def _reg(spec: dict) -> dict: + """{paire: [(index, nom), ...]} -> {paire: {index: [noms]}}.""" + out: dict = {} + for pair, entries in spec.items(): + d: dict = {} + for idx, name in entries: + d.setdefault(idx, []).append(name) + out[pair] = {k: sorted(v) for k, v in d.items()} + return out + + +# --- predicat pur ------------------------------------------------------------ + + +def test_le_merge_qui_introduit_un_doublon_bloque(): + """La base porte 0007 ; la tete ajoute un AUTRE nom au meme index. Le + merge porte deux noms a 0007 : c'est une introduction, donc un refus.""" + base = _reg({PAIR: [("0007", "0007-a.yaml")]}) + merged = _reg({PAIR: [("0007", "0007-a.yaml"), ("0007", "0007-b.yaml")]}) + reasons = merged_gate_reasons(base, merged) + assert len(reasons) == 1 + assert reasons[0]["pair"] == PAIR + assert reasons[0]["index"] == "0007" + assert reasons[0]["added"] == ["0007-b.yaml"] + assert "git mv" in reasons[0]["gist"] + + +def test_la_dette_de_la_base_non_reperee_ne_bloque_pas(): + """La base porte deja deux noms a 0007 ; le merge ne change rien. C'est + une dette de la base : la PR qui ne la touche pas ne doit pas etre + bloquee par elle.""" + base = _reg({PAIR: [("0007", "0007-a.yaml"), ("0007", "0007-b.yaml")]}) + merged = _reg({PAIR: [("0007", "0007-a.yaml"), ("0007", "0007-b.yaml")]}) + assert merged_gate_reasons(base, merged) == [] + + +def test_la_pr_qui_repare_la_dette_ne_bloque_pas(): + """La base porte deux noms a 0007 ; la tete en renumerote un. L'arbre de + fusion ne porte plus qu'un nom a 0007 : la PR REPAIRE, elle passe.""" + base = _reg({PAIR: [("0007", "0007-a.yaml"), ("0007", "0007-b.yaml")]}) + merged = _reg({PAIR: [("0007", "0007-b.yaml"), ("0009", "0009-a.yaml")]}) + assert merged_gate_reasons(base, merged) == [] + + +def test_un_index_libre_reste_libre(): + """Le cas nominal : la tete ajoute un nom a un index que personne ne + porte. Le merge n'introduit rien.""" + base = _reg({PAIR: [("0007", "0007-a.yaml")]}) + merged = _reg({PAIR: [("0007", "0007-a.yaml"), ("0009", "0009-a.yaml")]}) + assert merged_gate_reasons(base, merged) == [] + + +def test_aggraver_un_doublon_existant_bloque(): + """La base porte deux noms a 0007 ; la tete en ajoute un troisieme. Le + merge aggrave la dette : l'introduction est le nom nouveau, et elle + bloque.""" + base = _reg({PAIR: [("0007", "0007-a.yaml"), ("0007", "0007-b.yaml")]}) + merged = _reg({PAIR: [("0007", "0007-a.yaml"), ("0007", "0007-b.yaml"), + ("0007", "0007-c.yaml")]}) + reasons = merged_gate_reasons(base, merged) + assert len(reasons) == 1 + assert reasons[0]["added"] == ["0007-c.yaml"] + + +# --- integration : un vrai depot, un vrai merge ------------------------------ + +def _git(repo: Path, *args: str) -> str: + return subprocess.run( + ["git", "-C", str(repo), "-c", "user.name=lane", + "-c", "user.email=lane@example.invalid", *args], + capture_output=True, text=True, encoding="utf-8", + errors="replace", check=True).stdout + + +def _write_registry(repo: Path, entries: dict[str, str]) -> None: + for rel, content in entries.items(): + path = repo / ORG / PAIR / rel + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(content, encoding="utf-8") + + +def _make_repo(tmp_path: Path) -> Path: + repo = tmp_path / "repo" + repo.mkdir() + _git(repo, "init", "-q", "-b", "main") + _write_registry(repo, {"0007-2026-10-01-a.yaml": "date: '2026-10-01'\n"}) + _git(repo, "add", "-A") + _git(repo, "commit", "-q", "-m", "base") + return repo + + +def _run_organ(repo: Path, base: str, head: str) -> dict: + out = subprocess.run( + [sys.executable, + str(Path(__file__).resolve().parents[1] / + "check_twin_index_collisions.py"), + "--repo", str(repo), "--base", base, "--head", head, + "--merged", "--gate", "--json"], + capture_output=True, text=True, encoding="utf-8", + errors="replace") + return json.loads(out.stdout) + + +def test_le_mode_merged_signale_le_doublon_que_le_merge_cree(tmp_path): + """Scenario de concurrence de lanes : la base porte 0007, la tete ajoute + un second nom a 0007. L'arbre de fusion porte les deux -> le mode + `--merged` refuse.""" + repo = _make_repo(tmp_path) + _git(repo, "checkout", "-q", "-b", "pr") + _write_registry(repo, {"0007-2026-10-02-b.yaml": "date: '2026-10-02'\n"}) + _git(repo, "add", "-A") + _git(repo, "commit", "-q", "-m", "commit") + + data = _run_organ(repo, "main", "pr") + assert data["merged_checked"] is True + assert data["merged_unreadable"] is None + assert data["gate_fail"] is True, data + pairs = {(r["pair"], r["index"]) for r in data["merged_reasons"]} + assert pairs == {(PAIR, "0007")} + # Le classique jugement base+tete voit la MEME collision ici (la base ne + # porte qu'un nom) : les deux jugements concordent sur ce scenario. + assert data["gate_reasons"], data + + +def test_le_mode_merged_laisse_passer_la_pr_qui_repare(tmp_path): + """La base porte deux noms a 0007 (dette) ; la tete renumerote l'un. Le + merge ne porte plus qu'un nom a 0007 : la porte reste verte.""" + repo = tmp_path / "repo" + repo.mkdir() + _git(repo, "init", "-q", "-b", "main") + _write_registry(repo, { + "0007-2026-10-01-a.yaml": "date: '2026-10-01'\n", + "0007-2026-10-02-b.yaml": "date: '2026-10-02'\n", + }) + _git(repo, "add", "-A") + _git(repo, "commit", "-q", "-m", "base") + _git(repo, "checkout", "-q", "-b", "pr") + _git(repo, "mv", f"{ORG}/{PAIR}/0007-2026-10-01-a.yaml", + f"{ORG}/{PAIR}/0009-2026-10-01-a.yaml") + _git(repo, "commit", "-q", "-m", "renumerotation") + + data = _run_organ(repo, "main", "pr") + assert data["gate_fail"] is False, data + assert data["merged_reasons"] == [] + + +def test_un_merge_en_conflit_reste_illisible_et_ne_pretend_pas_etre_propre(tmp_path): + """« je n'ai pas pu lire » n'est pas « c'est propre » : l'arbre de fusion + d'un merge en conflit n'est pas definissable, et l'organe le dit. + + Le conflit est construit comme un vrai conflit : les DEUX cotes modifient + le meme fichier depuis la meme base (une seule partie modifiee se fusionne + sans conflit, et ne prouverait rien).""" + repo = _make_repo(tmp_path) + # La branche `pr` part de la base, avant que `main` ne modifie le fichier. + _git(repo, "branch", "pr") + _write_registry(repo, {"0007-2026-10-01-a.yaml": "date: 'main'\n"}) + _git(repo, "add", "-A") + _git(repo, "commit", "-q", "-m", "main modifie") + _git(repo, "checkout", "-q", "pr") + _write_registry(repo, {"0007-2026-10-01-a.yaml": "date: 'pr'\n"}) + _git(repo, "add", "-A") + _git(repo, "commit", "-q", "-m", "pr modifie") + + with pytest.raises(GitError): + merge_tree_sha("main", "pr", repo) + + data = _run_organ(repo, "main", "pr") + assert data["merged_unreadable"], data + assert data["merged_reasons"] == [] diff --git a/scripts/tests/test_merge_ready.py b/scripts/tests/test_merge_ready.py index 194b53d6dc2..a847fe66d41 100644 --- a/scripts/tests/test_merge_ready.py +++ b/scripts/tests/test_merge_ready.py @@ -1103,6 +1103,59 @@ def test_twin_organ_unreadable_is_fail_closed(tmp_path): assert lines[0]["reason"] == "twin-collision-unreadable:rc=2" +def test_twin_organ_is_called_with_the_merged_judgment(tmp_path): + """L'organe est appele en mode `--merged` : c'est l'arbre de fusion qui + repond a « `main` sera-t-il rouge apres ce merge ? » (mesure du + 2026-11-03 : #23832 et #24010 ont alloue les memes ordinals et sont + passees tous les deux).""" + view = default_view(files=(TWIN_FILE,)) + runner = ScriptedRunner(views={123: view}, twin_rc=0) + run_organ(tmp_path, runner) + twin = [c for c in runner.cmds() + if len(c) > 1 and "check_twin_index_collisions.py" in c[1]] + assert len(twin) == 1 + assert "--merged" in twin[0] + + +def test_twin_collision_introduite_par_le_merge_skips(tmp_path): + """Un doublon que le MERGE introduit (et qu'aucune revision ne porte + isolement) doit faire sauter le merge : c'est exactement le cas que le + seul jugement base+tete laissait passer.""" + payload = json.dumps({ + "cross_ref": [], + "merged_reasons": [{ + "pair": "search-05-geneticalgorithms", + "index": "0024", + "names": ["0024-a.yaml", "0024-b.yaml"], + "added": ["0024-b.yaml"], + "gist": "renumeroter par `git mv` PUR", + }], + "merged_unreadable": None, + }) + view = default_view(files=(TWIN_FILE,)) + runner = ScriptedRunner(views={123: view}, twin_rc=1, twin_stdout=payload) + rc, lines, _ = run_organ(tmp_path, runner) + assert lines[0]["verdict"] == "skipped" + assert lines[0]["reason"].startswith("twin-collision-on-merge:") + assert "search-05-geneticalgorithms/0024" in lines[0]["reason"] + + +def test_twin_arbre_de_fusion_illisible_is_fail_closed(tmp_path): + """« je n'ai pas pu lire » n'est pas « c'est propre » : l'arbre de fusion + illisible (merge en conflit) fait sauter le merge, meme quand le jugement + classique ne bloque rien (rc 0).""" + payload = json.dumps({ + "cross_ref": [], + "merged_reasons": [], + "merged_unreadable": "merge-tree --write-tree : conflit", + }) + view = default_view(files=(TWIN_FILE,)) + runner = ScriptedRunner(views={123: view}, twin_rc=0, twin_stdout=payload) + rc, lines, _ = run_organ(tmp_path, runner) + assert lines[0]["verdict"] == "skipped" + assert lines[0]["reason"] == "twin-collision-unreadable:merged" + + def test_twin_fetch_failure_is_fail_closed(tmp_path): view = default_view(files=(TWIN_FILE,)) runner = ScriptedRunner(views={123: view}, fetch_rc=128) From ac66734cf2639aee43a58b0b9db4c6d372a65a75 Mon Sep 17 00:00:00 2001 From: jsboige Date: Wed, 4 Nov 2026 04:25:40 +0100 Subject: [PATCH 2/2] Fix(tooling,#22299): la porte voit les ordinals revendiques par les PRs encore ouvertes La course d'index twin a trois couches ; le depot en portait deux. 1. Base au push : la PR est jugee contre SA base, qui est perimee au moment du push. Couvree par la couche de base existante. 2. Arbre de fusion : ce que `main` portera apres la fusion (#24519). 3. **PR ouverte concurrente** : une PR encore non fusionnee revendique le meme ordinal. Invisible pour les deux couches precedentes -- les deux PRs passent. C'etait la couche manquante. Nouvelle : `scripts/notebook_tools/twin_open_claims.py` -- collecte les revendications d'ordinal des PRs ouvertes via l'API GitHub (statut `added` / `renamed` : une PR qui MODIFIE une attestation garde son index et n'en revendique pas un nouveau, sinon le registre se signalerait lui-meme). `check_twin_index_collisions.py` accepte `--open-claims` / `--head-pr` et rend un verdict `open_pr` nomme, avec le geste : renumeroter vers le premier index libre par `git mv` PUR (aucun contenu a toucher, aucune re-execution). `merge_ready.py` (etape 5bis) branche la couche : il collecte les revendications, les passe a la porte, et refuse le merge avec `twin-collision-open-pr:/`. Revendications illisibles -> fail-CLOSED (`twin-collision-unreadable:open-claims`). Mesure (PR #24430, en direct) : - arbre de fusion seul -> `gate_fail = False` (la porte passe) - + couche OPEN-PR -> `gate_fail = True`, verdicts `open_pr` sur `csp-6-hybridization` 0024 et 0025, rc = 1 Sur l'ensemble des PRs ouvertes : **5 collisions vivantes** (23 PRs revendiquent un ordinal) -- - csp-1-fundamentals 0034 : #24293 / #24340 - csp-6-hybridization 0024 : #23322 / #24430 - csp-6-hybridization 0025 : #23322 / #24430 - gametheory-15-cooperativegames 0044 : #24174 / #24392 - search-13-limited-discrepancy-search 0013 : #24422 / #24476 Les deux PRs d'un couple ne se voient pas au merge : la porte parle a la derniere poussee, qui est donc celle qui renumerote. Tests : 14 tests du nouvel organe, 2 tests du merge_ready (skip + fail-CLOSED), suite merge_ready 112 passed, famille twin 39 passed, gardeurs absorbes 49 passed, garde des compteurs en prose OK. See #22299 Co-Authored-By: Claude Haiku 5.5 --- .../workflows/twin-index-collision-gate.yml | 62 +++- scripts/ci/fast_lane_registry.py | 1 + scripts/coordination/merge_ready.py | 54 +++- .../check_twin_index_collisions.py | 102 +++++++ .../tests/test_twin_open_claims.py | 212 +++++++++++++ scripts/notebook_tools/twin_open_claims.py | 284 ++++++++++++++++++ scripts/tests/test_merge_ready.py | 47 +++ 7 files changed, 754 insertions(+), 8 deletions(-) create mode 100644 scripts/notebook_tools/tests/test_twin_open_claims.py create mode 100644 scripts/notebook_tools/twin_open_claims.py diff --git a/.github/workflows/twin-index-collision-gate.yml b/.github/workflows/twin-index-collision-gate.yml index 5353af5a659..6ba22da7bfb 100644 --- a/.github/workflows/twin-index-collision-gate.yml +++ b/.github/workflows/twin-index-collision-gate.yml @@ -65,11 +65,28 @@ name: Twin index collision gate # premiere fois, et le `GITHUB_TOKEN` d'une PR de fork est en **lecture # seule**. # -# 2. LA BASE LUE. La porte compare `pull_request.base.sha`, c'est-a-dire -# `main` **au moment du push**. Une collision introduite sur `main` -# **entretemps** par une autre PR n'est pas vue ici : la porte referme la -# course au push suivant, pas au merge. Le garde de merge reste donc +# 2. LA COURSE ENTRE DEUX PRs PARALLELES (fermee par la couche OPEN-PR). +# La porte compare `pull_request.base.sha` -- `main` **au moment du push**. +# Deux PRs qui prennent le meme ordinal pendant que ni l'une ni l'autre +# n'est mergee passent donc **toutes les deux** sur cette base : `main` ne +# porte encore rien, et le verdict est VERT des deux cotes. +# C'est exactement ce qui est arrive a #24010 : au push, sa base ne +# portait pas les ordinals `0024`-`0027` apportes par #23832, encore +# OUVERTE. Les deux ont ete mergees, et `main` a recu trois doublons +# (`0024`, `0025`, `0026`) que #24518 a dus renumeroter deux jours plus +# tard -- en rouge fleet-wide sur `Scripts Tests (CPU)`. +# La couche OPEN-PR ferme cette course **avant** le merge : l'ordinal +# revendique par une autre PR OUVERTE est refuse, avec le meme geste +# (`git mv` PUR) que les autres verdicts. Elle lit l'API des fichiers des +# PRs ouvertes -- l'ordinal est dans le NOM du fichier, aucun fetch n'est # necessaire. +# Ce qui subsiste : une PR creee apres le passage de la porte d'une autre +# qui revendique le meme ordinal (la porte ne se rejoue pas sur un push +# qui ne touche pas le registre). La PR creee en DERNIER voit toujours +# l'autre -- une PR existe des son push -- : c'est donc toujours la +# plus recente qui est refusee, et l'ordinal est renumerote avant qu'une +# seule des deux ne merge. Le garde de merge reste necessaire pour le +# cas ou la porte n'a pas pu lire l'API. on: pull_request: @@ -77,10 +94,16 @@ on: - 'scripts/notebook_tools/twin_pairs.d/**' - 'scripts/notebook_tools/check_twin_index_collisions.py' - 'scripts/notebook_tools/check_twin_parity.py' + - 'scripts/notebook_tools/twin_open_claims.py' workflow_dispatch: permissions: contents: read + # Les ordinals revendiques par les PRs OUVERTES se lisent par l'API des + # fichiers d'une PR (`pull-requests: read`) : c'est la couche qui ferme la + # course entre deux PRs paralleles (cf. « La course que la base ne voit + # pas », plus bas). + pull-requests: read concurrency: group: twin-index-collision-gate-${{ github.ref }} @@ -104,6 +127,30 @@ jobs: with: python-version: '3.11' + - name: Collect the ordinal claims of the open PRs + # La porte de base juge cette PR contre SA base au push : deux PRs + # paralleles qui prennent le meme ordinal passent donc toutes les deux, + # et la collision n'apparait qu'apres le merge -- sur `main`, en rouge + # fleet-wide (mesure #22299 : #24010 vs #23832, `0024`-`0026`). + # L'ordinal est dans le NOM du fichier : la liste des fichiers d'une + # PR suffit, aucun fetch n'est necessaire. + # Un API illisible ne BLOQUE pas la PR (la porte de base, elle, reste + # appliquee) : le motif est affiche par l'organe, pas masque. + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + set +e + python scripts/notebook_tools/twin_open_claims.py --json \ + > twin_open_claims.json 2>twin_open_claims_stderr.log + EXIT=$? + set -e + if [ "$EXIT" != "0" ]; then + echo "::notice title=Twin ordinal claims illisibles::L'API des PRs ouvertes n'a pas pu etre lue (rc=$EXIT). La porte d'index s'applique sans la couche OPEN-PR." + cat twin_open_claims_stderr.log || true + rm -f twin_open_claims.json + fi + - name: Gate the attestation index collisions # `EVENT`/`PR_BASE`/`PR_HEAD` sont lus dans l'evenement, jamais deduits # de la forme du checkout : le checkout d'un evenement `pull_request` @@ -114,6 +161,7 @@ jobs: EVENT: ${{ github.event_name }} PR_BASE: ${{ github.event.pull_request.base.sha }} PR_HEAD: ${{ github.event.pull_request.head.sha }} + PR_NUMBER: ${{ github.event.pull_request.number }} run: | set +e BASE=origin/main @@ -123,8 +171,12 @@ jobs: HEAD_SHA="$PR_HEAD" fi echo "Twin index collision : base=$BASE head=$HEAD_SHA" + CLAIMS_ARGS="" + if [ -s twin_open_claims.json ]; then + CLAIMS_ARGS="--open-claims twin_open_claims.json --head-pr $PR_NUMBER" + fi python scripts/notebook_tools/check_twin_index_collisions.py \ - --base "$BASE" --head "$HEAD_SHA" --gate --json \ + --base "$BASE" --head "$HEAD_SHA" --gate --json $CLAIMS_ARGS \ > twin_index_collision.json 2>twin_index_collision_stderr.log EXIT=$? set -e diff --git a/scripts/ci/fast_lane_registry.py b/scripts/ci/fast_lane_registry.py index c3c84567a23..39e8e62a51f 100644 --- a/scripts/ci/fast_lane_registry.py +++ b/scripts/ci/fast_lane_registry.py @@ -3286,6 +3286,7 @@ def _body_md_hierarchy(rc: int, payload: str, ctx: dict) -> str: paths=[ "scripts/notebook_tools/twin_pairs.d/**", "scripts/notebook_tools/check_twin_index_collisions.py", + "scripts/notebook_tools/twin_open_claims.py", "scripts/ci/fast_lane.py", "scripts/ci/fast_lane_registry.py", ], diff --git a/scripts/coordination/merge_ready.py b/scripts/coordination/merge_ready.py index 01dc735ef90..e60134a367a 100644 --- a/scripts/coordination/merge_ready.py +++ b/scripts/coordination/merge_ready.py @@ -139,6 +139,7 @@ import re import subprocess import sys +import tempfile import time from dataclasses import dataclass from pathlib import Path @@ -188,6 +189,7 @@ GATE_PATH = SCRIPTS_DIR / "check_adjoint_prevalidation.py" NITS_PATH = SCRIPTS_DIR / "check_unaddressed_nits.py" TWIN_PATH = SCRIPTS_DIR / "notebook_tools" / "check_twin_index_collisions.py" +TWIN_CLAIMS_PATH = SCRIPTS_DIR / "notebook_tools" / "twin_open_claims.py" TWIN_REGISTRY_PREFIX = "scripts/notebook_tools/twin_pairs.d/" # Issue #20774 : au merge, le slot-reservation-guard doit lire la source # `open_prs` (mode en ligne) pour qu'une collision cross-PR rougisse avant @@ -906,10 +908,40 @@ def twin_collision_reason( ) if fetch.returncode != 0: return "twin-collision-unreadable:fetch" - res = runner.run( - [sys.executable, str(TWIN_PATH), "--repo", str(REPO_ROOT), - "--base", "origin/main", "--head", head, "--merged", "--json"] + # Les PRs OUVERTES : l'ordinal qu'une PR CONCURRENTE revendique deja. Ni + # le jugement base+tete ni l'arbre de fusion ne le voient -- les deux ne + # portent que des revisions MERGEES. L'API des PRs ouvertes, elle, sait. + # Mesure du 2026-11-04 : 5 collisions de ce type etaient VIVANTES entre PRs + # ouvertes (csp-1-fundamentals 0034, csp-6-hybridization 0024/0025, + # gametheory-15-cooperativegames 0044, search-13-limited-discrepancy-search + # 0013) -- aucune n'etait visible de l'arbre de fusion, puisque aucune des + # deux PRs n'etait mergee. + claims_res = runner.run( + [sys.executable, str(TWIN_CLAIMS_PATH), "--json"] ) + if claims_res.returncode != 0: + # « je n'ai pas pu lire » n'est pas « c'est propre » : skip dur, comme + # pour le fetch de l'arbre de fusion. La PR sera rejugee. + return "twin-collision-unreadable:open-claims" + try: + fd, claims_path = tempfile.mkstemp(prefix="twin-claims-", + suffix=".json") + with os.fdopen(fd, "w", encoding="utf-8") as fh: + fh.write(claims_res.stdout) + except OSError: + return "twin-collision-unreadable:open-claims" + try: + res = runner.run( + [sys.executable, str(TWIN_PATH), "--repo", str(REPO_ROOT), + "--base", "origin/main", "--head", head, "--merged", "--json", + "--open-claims", claims_path, "--head-pr", str(pr)] + ) + finally: + # L'organe a lu le fichier ; il ne sert plus. + try: + os.unlink(claims_path) + except OSError: + pass # Le rapport JSON est lu MEME quand l'organe rend 0 : le mode `--merged` # peut etre ILLISIBLE (merge en conflit) alors que le jugement classique # ne bloque rien -- et « je n'ai pas pu lire » n'est pas « c'est propre ». @@ -944,6 +976,22 @@ def twin_collision_reason( # fusion n'est pas definissable (merge en conflit). Skip dur -- # la PR sera rejugee apres rebasage. return "twin-collision-unreadable:merged" + # --- PRs OUVERTES : l'ordinal revendique par une PR concurrente + # L'arbre de fusion repond a « que portera `main` apres ce merge ? ». + # Il ne voit pas une PR CONCURRENTE non encore mergee : les deux PRs + # revendiquent le meme ordinal, aucune n'est dans l'arbre de fusion, et + # la collision n'apparait qu'apres le second merge -- en rouge + # fleet-wide. Merger celle-ci la rendrait inevitable. + open_reasons = data.get("open_pr_reasons") or [] + if open_reasons: + first = open_reasons[0] + return ( + "twin-collision-open-pr:%s/%s -- %s" + % (first.get("pair"), first.get("index"), + first.get("gist") + or "renumeroter le fichier de la tete vers le premier " + "index libre de la paire par `git mv` PUR") + ) if res.returncode == 0: return None if res.returncode == 1: diff --git a/scripts/notebook_tools/check_twin_index_collisions.py b/scripts/notebook_tools/check_twin_index_collisions.py index ac417dc0390..d3ba8736ec7 100644 --- a/scripts/notebook_tools/check_twin_index_collisions.py +++ b/scripts/notebook_tools/check_twin_index_collisions.py @@ -86,6 +86,7 @@ sys.path.insert(0, str(Path(__file__).resolve().parent)) from check_twin_parity import audit_index # noqa: E402 +from twin_open_claims import collisions_by_ordinal # noqa: E402 REGISTRY_REL = "scripts/notebook_tools/twin_pairs.d" WORKTREE_REF = "worktree" @@ -286,6 +287,57 @@ def gate_verdict(cross: list, intra: list, *, base_ref: str) -> dict: return {"fail": bool(reasons), "reasons": reasons} +# --- verdicts OPEN-PR : l'ordinal revendique par une AUTRE PR ouverte ------- +# La porte de base juge une PR contre SA base au push, et l'arbre de fusion +# ( `--merged`, cf. `merged_gate_reasons`) repond a « que portera `main` apres +# ce merge ? ». Ni l'un ni l'autre ne voit ce qui n'est pas encore merge : +# deux PRs qui prennent le meme ordinal pendant que ni l'une ni l'autre +# n'est mergee passent donc TOUTES LES DEUX, et la collision n'apparait +# qu'apres coup -- sur `main`, en rouge fleet-wide. +# +# Mesure fondatrice (#22299) : au push de #24010 (2026-11-01/02), sa base ne +# portait pas encore les ordinals `0024`-`0027` apportes par #23832 -- encore +# OUVERTE -- : la porte a rendu un VERT. Les deux PRs sont passees ; `main` a +# recu trois doublons (`0024`, `0025`, `0026`) que #24518 a dus renumeroter +# deux jours plus tard. +# +# Ce verdict porte sur ce que l'API sait et que le git ne sait pas encore : les +# ordinals REVENDIQUES par les PRs ouvertes. Il est BLOQUANT -- l'ordinal se +# renumerote avant le merge, pas apres -- et son geste est le meme `git mv` +# PUR que les autres verdicts. +def open_pr_reasons(claims_by_pr: dict, *, head_pr: int | None) -> list[dict]: + """Verdicts OPEN-PR : un ordinal revendique par CETTE PR et par une autre. + + Pure : prend les revendications `{pr: [claims]}` et le numero de la PR + interrogee. Sans `head_pr`, aucune raison n'est produite : une collision + entre deux PRs qui ne sont pas celle-ci ne se decide pas ici (chacune la + verra a son tour, a son propre push). + """ + if head_pr is None: + return [] + out: list[dict] = [] + for c in collisions_by_ordinal(claims_by_pr or {}): + if int(head_pr) not in [int(p) for p in c["prs"]]: + continue + autres = [p for p in c["prs"] if int(p) != int(head_pr)] + out.append({ + "kind": "open_pr", + "pair": c["pair"], + "index": c["index"], + "names": list(c["names"]), + "prs": list(c["prs"]), + "gist": ( + "l'ordinal %s de la paire '%s' est deja revendique par la PR " + "#%s, ENCORE OUVERTE : renumeroter VOTRE fichier vers le " + "premier index libre de la paire par `git mv` PUR (aucun " + "contenu a toucher, aucune re-execution). Attendre le merge " + "ferait porter la collision par `main`." + % (c["index"], c["pair"], ", #".join(str(p) for p in autres)) + ), + }) + return out + + def merged_gate_reasons(base_reg: dict, merged_reg: dict) -> list: """Raisons portees par l'ARBRE DE FUSION : ce que le MERGE rend visible. @@ -401,6 +453,13 @@ def main(argv: list | None = None) -> int: "dit si `main` sera rouge apres le merge. Ne bloque " "que les doublons que le MERGE INTRODUIT. Impose " "--head.") + ap.add_argument("--open-claims", default=None, + help="fichier JSON des revendications d'ordinal des PRs " + "ouvertes (produit par twin_open_claims.py). Ajoute " + "les verdicts OPEN-PR : un ordinal revendique par une " + "AUTRE PR ouverte est refuse ici, avant le merge.") + ap.add_argument("--head-pr", type=int, default=None, + help="numero de la PR interrogee (verdicts OPEN-PR)") ap.add_argument("--repo", default=None, help="racine du depot (defaut: auto)") ap.add_argument("--registry", default=REGISTRY_REL, help="chemin du registre, relatif a la racine") @@ -460,6 +519,34 @@ def main(argv: list | None = None) -> int: # une PR sans condamner la flotte (#23208). gate = gate_verdict(cross, intra, base_ref=args.base) if args.gate else None + # Verdicts OPEN-PR : les ordinals revendiques par les PRs OUVERTES. Ni la + # base x tete ni l'arbre de fusion ne les voient : les deux ne portent que + # des revisions MERGEES (cf. `open_pr_reasons`). + open_reasons: list = [] + open_note = "" + if args.open_claims: + try: + payload = json.loads( + Path(args.open_claims).read_text(encoding="utf-8")) + except (OSError, ValueError) as exc: + # « je n'ai pas pu lire » n'est pas « c'est propre » : le motif est + # AFFICHE, et les verdicts OPEN-PR ne sont pas rendus. La porte de + # base (base x tete) et l'arbre de fusion restent, eux, appliques. + open_note = ("revendications OPEN-PR illisibles (%s) -- non " + "utilisees" % exc) + else: + claims_by_pr: dict = {} + for row in (payload.get("claims") or []): + claims_by_pr.setdefault(str(row.get("pr")), []).append(row) + open_reasons = open_pr_reasons(claims_by_pr, head_pr=args.head_pr) + if gate is not None and open_reasons: + deja_vus = {(r["pair"], r["index"]) for r in gate["reasons"]} + for r in open_reasons: + if (r["pair"], r["index"]) in deja_vus: + continue + gate["reasons"].append(r) + gate["fail"] = bool(gate["reasons"]) + # Arbre de FUSION : la seule revision qui repond a « `main` sera-t-il # rouge apres ce merge ? ». L'union base+tete ne repond pas a la # question : elle voit l'etat de la base AU PUSH, et une collision @@ -511,6 +598,8 @@ def main(argv: list | None = None) -> int: "gate": bool(args.gate), "gate_fail": bool(gate and gate["fail"]), "gate_reasons": (gate or {}).get("reasons", []), + "open_pr_reasons": open_reasons, + "open_pr_note": open_note, "merged_checked": bool(args.merged), "merged_ref": merged_ref, "merged_unreadable": merged_unreadable, @@ -562,6 +651,19 @@ def main(argv: list | None = None) -> int: print(" C'est le predicat du garde CI sur CETTE revision (pas un") print(" effet du merge) : relancer avec --in-tree pour en faire un") print(" verdict bloquant, ou laisser le garde CI le porter.") + if open_note: + print("") + print("REVENDICATIONS OPEN-PR : %s" % open_note) + if open_reasons: + print("") + print("COLLISIONS AVEC UNE PR OUVERTE (%d)" % len(open_reasons)) + for r in open_reasons: + print(" %s / index %s (PR #%s)" + % (r["pair"], r["index"], ", #".join( + str(p) for p in r["prs"]))) + for name in r["names"]: + print(" %s" % name) + print(" geste: %s" % r["gist"]) if args.merged: print("") diff --git a/scripts/notebook_tools/tests/test_twin_open_claims.py b/scripts/notebook_tools/tests/test_twin_open_claims.py new file mode 100644 index 00000000000..158337a9bdf --- /dev/null +++ b/scripts/notebook_tools/tests/test_twin_open_claims.py @@ -0,0 +1,212 @@ +"""Tests de `twin_open_claims.py` et du verdict OPEN-PR de la porte. + +La regle testee, en deux temps : + +1. **Qui revendique un ordinal** : une PR qui AJOUTE ou RENOMME le fichier + qui le porte. Une PR qui MODIFIE une attestation existante garde l'ordinal + et n'en revendique pas un nouveau -- sinon le registre se signalerait + lui-meme sur chaque retouche. + +2. **Quand y a-t-il collision** : quand deux PRs distinctes revendiquent le + meme (paire, ordinal). C'est le cas que la porte de base ne voit pas : + elle juge une PR contre SA base au push, et deux PRs paralleles passent + donc toutes les deux (mesure #22299 : #24010 vs #23832, `0024`-`0026`). + +Le sens du refus est essential : la PR **creee en DERNIER** voit l'autre (une +PR existe des son push), c'est donc elle qui renumerote -- avant qu'aucune des +deux ne merge. +""" + +from __future__ import annotations + +import json +import subprocess +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +from check_twin_index_collisions import open_pr_reasons # noqa: E402 +from twin_open_claims import ( # noqa: E402 + claims_from_files, + collisions_by_ordinal, + parse_claim, +) + +REPO = Path(__file__).resolve().parents[3] + +PREFIX = "scripts/notebook_tools/twin_pairs.d" + + +def _path(index: str, lane: str = "myia-po-2025-CoursIA") -> str: + return "%s/app/%s-2026-11-01-%s.yaml" % (PREFIX, index, lane) + + +# --- 1. qui revendique un ordinal ------------------------------------------- + + +def test_un_chemin_du_registre_revendique_son_ordinal(): + claim = parse_claim(_path("0024")) + assert claim == { + "pair": "app", + "index": "0024", + "path": _path("0024"), + } + + +def test_un_chemin_hors_registre_ne_revendique_rien(): + assert parse_claim("scripts/ci/fast_lane_registry.py") is None + assert parse_claim("") is None + + +def test_un_ordinal_non_numerique_ne_revendique_rien(): + """Le nom du fichier porte l'index, pas l'inverse : un fichier du + registre sans index numerique n'est pas une revendication d'ordinal.""" + assert parse_claim("%s/app/notes-2026-11-01-x.yaml" % PREFIX) is None + + +def test_seul_un_fichier_ajoute_ou_renomme_revendique_un_ordinal(): + files = [ + {"path": _path("0024"), "status": "added"}, + {"path": _path("0025"), "status": "renamed"}, + {"path": _path("0026"), "status": "modified"}, + {"path": _path("0027"), "status": "removed"}, + ] + claims = claims_from_files(files) + assert sorted(c["index"] for c in claims) == ["0024", "0025"] + + +def test_un_fichier_modifie_ne_revendique_pas_d_ordinal(): + """Une PR qui touche une attestation SANS la deplacer garde l'ordinal de + la paire : le revendiquer ferait signaler le registre contre lui-meme.""" + assert claims_from_files([{"path": _path("0024"), "status": "modified"}]) == [] + + +def test_un_fichier_sans_statut_ne_revendique_pas_d_ordinal(): + """Information absente != information negative : on ne bloque pas sur ce + qu'on ne sait pas.""" + assert claims_from_files([{"path": _path("0024")}]) == [] + + +# --- 2. quand y a-t-il collision -------------------------------------------- + + +def test_deux_prs_qui_revendiquent_le_meme_ordinal_collisionnent(): + claims = { + "23832": [{"pair": "app", "index": "0024", "path": _path("0024")}], + "24010": [{"pair": "app", "index": "0024", + "path": _path("0024", "myia-po-2024-CoursIA")}], + } + collisions = collisions_by_ordinal(claims) + assert len(collisions) == 1 + assert collisions[0]["pair"] == "app" + assert collisions[0]["index"] == "0024" + assert collisions[0]["prs"] == [23832, 24010] + + +def test_une_seule_pr_ne_collisionne_pas_avec_elle_meme(): + claims = { + "23832": [ + {"pair": "app", "index": "0024", "path": _path("0024")}, + {"pair": "app", "index": "0024", "path": _path("0024")}, + ], + } + assert collisions_by_ordinal(claims) == [] + + +def test_des_ordinals_de_paires_differentes_ne_collisionnent_pas(): + claims = { + "23832": [{"pair": "app", "index": "0024", "path": _path("0024")}], + "24010": [{"pair": "autre", "index": "0024", + "path": "%s/autre/0024-2026-11-01-x.yaml" % PREFIX}], + } + assert collisions_by_ordinal(claims) == [] + + +def test_le_meme_nom_revendique_par_deux_prs_nest_pas_un_doublon(): + """Deux PRs qui ajoutent le MEME fichier ne produisent qu'un fichier sur + `main` : git refuse le merge (`add/add`) et un humain tranche. C'est un + conflit de merge, pas un doublon d'index -- la regle de l'organe + historique l'exclut deja (le nom identique vu des deux cotes n'est pas une + collision).""" + claims = { + "23832": [{"pair": "app", "index": "0024", "path": _path("0024")}], + "24010": [{"pair": "app", "index": "0024", "path": _path("0024")}], + } + assert collisions_by_ordinal(claims) == [] + + +# --- 3. le verdict de la porte ---------------------------------------------- + + +def _claims(*pairs): + """[(pr, pair, index, lane)] -> {'claims': [...]} au format de l'organe.""" + return {"claims": [ + {"pr": pr, "pair": pair, "index": index, + "path": "%s/%s/%s-2026-11-01-%s.yaml" % (PREFIX, pair, index, lane)} + for pr, pair, index, lane in pairs + ]} + + +def test_la_porte_refuse_l_ordinal_revendique_par_une_autre_pr_ouverte(): + payload = _claims((24010, "app", "0024", "myia-po-2024-CoursIA"), + (23832, "app", "0024", "myia-po-2025-CoursIA")) + by_pr: dict = {} + for row in payload["claims"]: + by_pr.setdefault(str(row["pr"]), []).append(row) + reasons = open_pr_reasons(by_pr, head_pr=24010) + assert len(reasons) == 1 + assert reasons[0]["kind"] == "open_pr" + assert reasons[0]["pair"] == "app" + assert reasons[0]["index"] == "0024" + assert reasons[0]["prs"] == [23832, 24010] + # le geste doit etre nommé : un `git mv` PUR, sans re-execution + assert "git mv" in reasons[0]["gist"] + assert "23832" in reasons[0]["gist"] + + +def test_la_porte_ne_juge_pas_une_collision_qui_ne_la_concerne_pas(): + payload = _claims((24010, "app", "0024", "myia-po-2024-CoursIA"), + (23832, "app", "0024", "myia-po-2025-CoursIA")) + by_pr: dict = {} + for row in payload["claims"]: + by_pr.setdefault(str(row["pr"]), []).append(row) + assert open_pr_reasons(by_pr, head_pr=12345) == [] + + +def test_sans_numeros_de_pr_le_verdict_ne_se_prononce_pas(): + assert open_pr_reasons({}, head_pr=None) == [] + + +# --- 4. le contrat de sortie de l'organe ------------------------------------- + + +def _has_ref(ref: str) -> bool: + return subprocess.run( + ["git", "-C", str(REPO), "rev-parse", "--verify", "--quiet", ref], + capture_output=True).returncode == 0 + + +def test_la_porte_en_cli_refuse_le_verdict_open_pr(tmp_path): + """Contrôle de bout en bout : la porte rend `gate_fail` a cause du SEUL + verdict OPEN-PR -- la base et la tete ne portent aucune collision (le + registre de la revision testee est propre). Sans ce controle, une + integration qui nposerait pas le verdict OPEN-PR passerait quand meme.""" + if not (_has_ref("origin/main") and _has_ref("HEAD")): + return + claims = tmp_path / "claims.json" + claims.write_text(json.dumps(_claims( + (24010, "app", "0024", "myia-po-2024-CoursIA"), + (23832, "app", "0024", "myia-po-2025-CoursIA"), + )), encoding="utf-8") + proc = subprocess.run( + [sys.executable, + "scripts/notebook_tools/check_twin_index_collisions.py", + "--repo", str(REPO), "--base", "origin/main", "--head", "HEAD", + "--gate", "--json", + "--open-claims", str(claims), "--head-pr", "24010"], + cwd=str(REPO), capture_output=True, text=True, encoding="utf-8") + report = json.loads(proc.stdout) + assert report["gate_fail"] is True + kinds = [r["kind"] for r in report["gate_reasons"]] + assert "open_pr" in kinds + assert proc.returncode == 1 diff --git a/scripts/notebook_tools/twin_open_claims.py b/scripts/notebook_tools/twin_open_claims.py new file mode 100644 index 00000000000..430a6bae417 --- /dev/null +++ b/scripts/notebook_tools/twin_open_claims.py @@ -0,0 +1,284 @@ +"""Revendications d'ordinal d'attestation twin portees par les PRs ouvertes. + +L'index `NNNN` du nom de fichier est la CLE DE TRI du journal d'une paire, et +il doit etre UNIQUE par paire. Deux lanes qui attestent la meme paire le meme +jour allouent le meme ordinal : aucun conflit git, aucun nom identique, +`MERGEABLE` et souvent `CLEAN` -- et pourtant `main` finit par porter deux +fichiers au meme ordinal. + +Pourquoi ce module existe (#22299) +----------------------------------- +La porte `check_twin_index_collisions.py --gate` juge une PR contre **sa base +au push**. Mesure fondatrice : au push de #24010 (2026-11-01/02), sa base ne +portait pas encore les ordinals `0024`-`0027` apportes par #23832 -- encore +OUVERTE a ce moment -- : la porte a rendu un VERT. Les deux PRs sont passees, +et `main` a recu trois doublons (`0024`, `0025`, `0026`) que #24518 a dus +renumeroter le 2026-11-04. + +La porte ne peut pas voir ce que l'API seule sait : **les ordinals revendiques +par les PRs ouvertes**. Ce module les rend, sans fetch : l'ordinal est dans le +NOM du fichier, donc la liste des fichiers d'une PR suffit. + +Ce que le module est, et ce qu'il n'est pas +-------------------------------------------- +* `claims_from_files` -- pur : les fichiers d'une PR -> les ordinals qu'elle + REVENDIQUE. Seuls les statuts `added` et `renamed` revendiquent un ordinal : + une PR qui MODIFIE une attestation existante garde le sien et n'en prend pas + un nouveau (sinon le registre se signalerait lui-meme). +* `collect_open_claims` -- lecture de l'API : les ordinals revendiques par + toutes les PRs ouvertes, PR par PR. +* `collisions_by_ordinal` -- pur : les (paire, ordinal) revendiques par deux + PRs ou plus. + +Ce module NE decide pas si une PR peut merger : il produit la MATIERE. La +porte (`check_twin_index_collisions.py --open-claims`) en tire le verdict, et +c'est elle qui reste l'organe unique du verdict. + +Contrat de restitution : stdout ne porte QUE le JSON -- la ligne de verdict +part sur stderr. Un rapport machine melange a une ligne de texte n'est plus un +JSON, et le parseur du consumer echoue sur un fichier casse. +""" + +from __future__ import annotations + +import argparse +import json +import os +import re +import subprocess +import sys +from pathlib import Path + +REGISTRY_PREFIX = "scripts/notebook_tools/twin_pairs.d/" + +# `scripts/notebook_tools/twin_pairs.d//--.yaml` +_CLAIM_RE = re.compile( + r"^scripts/notebook_tools/twin_pairs\.d/(?P[^/]+)/" + r"(?P\d{4})-[^/]*\.yaml$" +) + +# Statuts qui REVENDIQUENT un ordinal : le fichier n'existait pas dans la base +# de la PR. `modified` n'en revendique pas (l'ordinal appartient deja a la +# paire, sur `main` ou dans une base). +NEW_STATUSES = ("added", "renamed") + + +class GhError(RuntimeError): + """La lecture de l'API a echoue : ce n'est pas « aucune revendication ».""" + + +def parse_claim(path: str) -> dict | None: + """Chemin de fichier -> revendication {pair, index, path}, ou None. + + Un chemin hors registre (ou dont l'ordinal n'est pas numerique) ne + revendique rien : c'est un non-fait, pas une panne. + """ + norm = (path or "").replace("\\", "/") + m = _CLAIM_RE.match(norm) + if not m: + return None + return { + "pair": m.group("pair"), + "index": m.group("index"), + "path": norm, + } + + +def claims_from_files(files, *, statuses=NEW_STATUSES) -> list[dict]: + """Fichiers d'une PR -> ordinals revendiques. + + `files` : iterable de dicts `{"path": ..., "status": ...}`. Un fichier + sans statut connu ne revendique rien (fail-closed cote faux positif : on + ne bloque pas sur une information absente). + """ + out: list[dict] = [] + for row in files or []: + status = str((row or {}).get("status") or "").lower() + if statuses and status not in statuses: + continue + claim = parse_claim(str((row or {}).get("path") or "")) + if claim: + claim["status"] = status + out.append(claim) + return out + + +def collisions_by_ordinal(claims_by_pr: dict) -> list[dict]: + """{pr: [claims]} -> les (paire, ordinal) revendiques par 2 PRs ou plus. + + Pur. Deux conditions, et la seconde est celle de l'organe historique : + + 1. la revendication est PORTEE par deux PRs DISTINCTES -- une PR unique ne + peut pas collisionner avec elle-meme ; + 2. les NOMS vus pour cet ordinal sont au moins deux -- deux PRs qui + revendiquent le meme ordinal **sous le meme nom de fichier** ne + produisent qu'un seul fichier sur `main` : git refuse le merge + (`add/add`) et un humain tranche. C'est un conflit de merge, pas un + doublon d'index, et la regle de l'organe l'exclut deja (le nom + identique vu des deux cotes n'est pas une collision). + """ + owners: dict = {} + for pr, claims in (claims_by_pr or {}).items(): + for claim in claims or []: + key = (claim["pair"], claim["index"]) + owners.setdefault(key, {}).setdefault(int(pr), []).append( + os.path.basename(str(claim.get("path") or ""))) + out: list[dict] = [] + for (pair, index), by_pr in sorted(owners.items()): + if len(by_pr) < 2: + continue + names = sorted({n for paths in by_pr.values() for n in paths}) + if len(names) < 2: + # meme nom des deux cotes : conflit git `add/add`, pas un doublon. + continue + out.append({ + "pair": pair, + "index": index, + "prs": sorted(by_pr), + "names": names, + }) + return out + + +def _gh(args: list[str], *, cwd: Path | None = None) -> str: + """Appelle `gh` et rend sa sortie. Leve `GhError` si elle echoue.""" + proc = subprocess.run( + ["gh", *args], + cwd=str(cwd) if cwd else None, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + ) + if proc.returncode != 0: + raise GhError( + "gh %s -> rc=%d : %s" + % (" ".join(args[:3]), proc.returncode, + (proc.stderr or proc.stdout or "").strip()[:300])) + return proc.stdout + + +def repo_slug(cwd: Path | None = None) -> str: + """`owner/repo` du depot courant.""" + out = _gh(["repo", "view", "--json", "nameWithOwner", + "-q", ".nameWithOwner"], cwd=cwd).strip() + if not out: + raise GhError("gh repo view n'a rien rendu") + return out + + +def open_prs_touching_registry(slug: str, *, limit: int = 200, + cwd: Path | None = None) -> list[int]: + """Numeros des PRs ouvertes dont les fichiers touchent le registre twin. + + Le filtre se fait sur les CHEMINS (deja connus de `gh pr list`), donc une + seule requete suffit pour ecarter la quasi-totalite du parc. + """ + raw = _gh(["pr", "list", "--repo", slug, "--state", "open", + "--json", "number,files", "--limit", str(limit)], cwd=cwd) + try: + rows = json.loads(raw) + except (json.JSONDecodeError, ValueError) as exc: + raise GhError("gh pr list n'a pas rendu du JSON : %s" % exc) from exc + out: list[int] = [] + for row in rows or []: + paths = [str((f or {}).get("path") or "") + for f in (row.get("files") or [])] + if any(p.replace("\\", "/").startswith(REGISTRY_PREFIX) + for p in paths): + out.append(int(row["number"])) + return sorted(out) + + +def pr_new_claims(slug: str, number: int, *, cwd: Path | None = None + ) -> list[dict]: + """Ordinals revendiques par la PR `number` (fichiers ajoutes/renommes).""" + raw = _gh(["api", "--paginate", "-H", "Accept: application/vnd.github+json", + "repos/%s/pulls/%d/files" % (slug, number)], cwd=cwd) + try: + rows = json.loads(raw) + except (json.JSONDecodeError, ValueError) as exc: + raise GhError("gh api pulls/%d/files n'a pas rendu du JSON : %s" + % (number, exc)) from exc + if isinstance(rows, dict): # pagination : l'API rend une liste par page + rows = rows.get("items") or [] + files = [{"path": r.get("filename"), "status": r.get("status")} + for r in rows or []] + return claims_from_files(files) + + +def collect_open_claims(slug: str, *, cwd: Path | None = None, + limit: int = 200) -> dict: + """Les ordinals revendiques par chaque PR ouverte qui touche le registre. + + Rend `{"prs": {num: [claims]}}`. Une PR dont la lecture echoue est + **absente** du rendu, et l'appelant doit pouvoir dire « incomplet » : + c'est pourquoi `collect_open_claims` leve `GhError` des que la liste des + PRs candidates n'a pas pu etre lue. + """ + numbers = open_prs_touching_registry(slug, limit=limit, cwd=cwd) + prs: dict = {} + for number in numbers: + try: + claims = pr_new_claims(slug, number, cwd=cwd) + except GhError: + continue # PR illisible : elle ne peut pas etre accusee a tort + if claims: + prs[str(number)] = claims + return {"prs": prs} + + +def claims_to_flat_list(claims_by_pr: dict) -> list[dict]: + """Aplatit `{"prs": {num: [claims]}}` en une liste porteuse du `pr`.""" + out: list[dict] = [] + for number, claims in (claims_by_pr.get("prs") or {}).items(): + for claim in claims or []: + out.append({"pr": int(number), **claim}) + return out + + +def main(argv: list | None = None) -> int: + ap = argparse.ArgumentParser( + description="Revendications d'ordinal d'attestation twin par les PRs " + "ouvertes (organe complementaire de la porte d'index).") + ap.add_argument("--repo", default=None, + help="owner/repo (defaut: depot courant)") + ap.add_argument("--pr", type=int, default=None, + help="numero de la PR a interroger (claims d'une PR)") + ap.add_argument("--json", action="store_true", help="sortie machine") + args = ap.parse_args(argv) + + slug = args.repo + if not slug: + try: + slug = repo_slug() + except GhError as exc: + print("ERREUR: %s" % exc, file=sys.stderr) + print("VERDICT: INDETERMINE (API illisible)", file=sys.stderr) + return 2 + + try: + if args.pr is not None: + claims = pr_new_claims(slug, args.pr) + payload = {"prs": {str(args.pr): claims}} + else: + payload = collect_open_claims(slug) + except GhError as exc: + print("ERREUR: %s" % exc, file=sys.stderr) + print("VERDICT: INDETERMINE (API illisible)", file=sys.stderr) + return 2 + + flat = claims_to_flat_list(payload) + if args.json: + print(json.dumps({"claims": flat}, indent=2, sort_keys=True)) + else: + for row in flat: + print("PR #%d revendique %s / %s (%s)" + % (row["pr"], row["pair"], row["index"], row["path"])) + print("VERDICT: %d revendication(s) sur %d PR(s)" + % (len(flat), len(payload.get("prs") or {}))) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/tests/test_merge_ready.py b/scripts/tests/test_merge_ready.py index a847fe66d41..f3c1c1ffb32 100644 --- a/scripts/tests/test_merge_ready.py +++ b/scripts/tests/test_merge_ready.py @@ -160,6 +160,11 @@ def __init__( fetch_rc: int = 0, twin_rc: int = 0, twin_stdout: str = "", + # Revendications d'ordinal des PRs ouvertes (organe + # twin_open_claims.py) : 0 = liste vide, 1 = liste portee par + # claims_stdout, 2 = API illisible (fail-closed). + claims_rc: int = 0, + claims_stdout: str = '{"claims": []}', # Issue #20774 : organe slot-reservation-guard en mode en ligne, au merge. # 0 = clear, 1 = collision, 2 = unreadable. Cf. SLOT_DOCUMENTED_RC. slot_rc: int = 0, @@ -189,6 +194,8 @@ def __init__( # Vide par defaut (les tests historiques ``twin-index-collision`` # n'attendent pas de classification). self.twin_stdout = twin_stdout + self.claims_rc = claims_rc + self.claims_stdout = claims_stdout # #20774 : organe slot-reservation-guard, mode en ligne. Meme # convention que twin_rc/twin_stdout. self.slot_rc = slot_rc @@ -284,6 +291,8 @@ def run(self, cmd: list[str], env: dict | None = None) -> mr.RunResult: return mr.RunResult(self.fetch_rc, "", "") if len(c) > 1 and "check_twin_index_collisions.py" in c[1]: return mr.RunResult(self.twin_rc, self.twin_stdout, "") + if len(c) > 1 and "twin_open_claims.py" in c[1]: + return mr.RunResult(self.claims_rc, self.claims_stdout, "") if len(c) > 1 and "check_slot_reservation.py" in c[1]: return mr.RunResult(self.slot_rc, self.slot_stdout, "") raise AssertionError("commande non scriptee : " + " ".join(c)) @@ -1156,6 +1165,44 @@ def test_twin_arbre_de_fusion_illisible_is_fail_closed(tmp_path): assert lines[0]["reason"] == "twin-collision-unreadable:merged" +def test_twin_collision_avec_une_pr_ouverte_skips(tmp_path): + """Un ordinal revendique par une PR CONCURRENTE encore ouverte doit faire + sauter le merge : ni le jugement base+tete ni l'arbre de fusion ne le + voient (les deux ne portent que des revisions mergees), et la collision + n'apparaitrait qu'apres le second merge -- en rouge fleet-wide (mesure du + 2026-11-04 : 5 collisions de ce type etaient vivantes entre PRs + ouvertes).""" + payload = json.dumps({ + "cross_ref": [], + "merged_reasons": [], + "merged_unreadable": None, + "open_pr_reasons": [{ + "pair": "csp-6-hybridization", + "index": "0024", + "names": ["0024-a.yaml", "0024-b.yaml"], + "prs": [123, 999], + "gist": "renumeroter par `git mv` PUR", + }], + }) + view = default_view(files=(TWIN_FILE,)) + runner = ScriptedRunner(views={123: view}, twin_rc=1, twin_stdout=payload) + rc, lines, _ = run_organ(tmp_path, runner) + assert lines[0]["verdict"] == "skipped" + assert lines[0]["reason"].startswith("twin-collision-open-pr:") + assert "csp-6-hybridization/0024" in lines[0]["reason"] + + +def test_twin_revendications_illisibles_is_fail_closed(tmp_path): + """« je n'ai pas pu lire » n'est pas « c'est propre » : l'API des PRs + ouvertes illisible fait sauter le merge, comme le fetch de l'arbre de + fusion.""" + view = default_view(files=(TWIN_FILE,)) + runner = ScriptedRunner(views={123: view}, claims_rc=2) + rc, lines, _ = run_organ(tmp_path, runner) + assert lines[0]["verdict"] == "skipped" + assert lines[0]["reason"] == "twin-collision-unreadable:open-claims" + + def test_twin_fetch_failure_is_fail_closed(tmp_path): view = default_view(files=(TWIN_FILE,)) runner = ScriptedRunner(views={123: view}, fetch_rc=128)