Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 33 additions & 6 deletions scripts/check_unaddressed_nits.py
Original file line number Diff line number Diff line change
Expand Up @@ -3352,9 +3352,19 @@ def ts(value: str | None) -> datetime | None:
return datetime.fromisoformat(value.replace("Z", "+00:00"))


# #19643 (residu) -- borne PAR APPEL sur chaque `gh` de l'organe. Mesure de
# l'issue (commentaire 2026-10-09T09:20:40Z) : 195 appels sans borne sur 200,
# environ 0,75 s par appel. 90 s est un choix, pas la queue mesuree : large
# pour un `gh api --paginate` sur une PR chargee, sous le timeout des tools
# shell des agents (~115-120 s). Une expiration remonte (TimeoutExpired),
# jamais avalee. La borne ne couvre pas le total de la chaine B.0.
GH_CALL_TIMEOUT_S = 90


def gh_json(args: list[str]) -> object:
out = subprocess.run(
["gh", *args], capture_output=True, text=True, encoding="utf-8", check=True
["gh", *args], capture_output=True, text=True, encoding="utf-8", check=True,
timeout=GH_CALL_TIMEOUT_S,
).stdout
return json.loads(out)

Expand Down Expand Up @@ -3405,6 +3415,10 @@ def _gh_fetch_issue(n: int) -> dict | None:
if n not in _ISSUE_INFO_CACHE:
try:
d = gh_json(["api", "repos/" + REPO + "/issues/" + str(n)])
except subprocess.TimeoutExpired:
# #19643 (residu) : une expiration n'est pas « numero qui n'est pas
# une issue » -- elle ne doit pas etre mise en cache comme None.
raise
except Exception:
_ISSUE_INFO_CACHE[n] = None
return None
Expand Down Expand Up @@ -4369,6 +4383,10 @@ def improper_dismissals(pr: int) -> set[str]:
try:
reviews = gh_json(["api", f"repos/{REPO}/pulls/{pr}/reviews", "--paginate"])
events = gh_json(["api", f"repos/{REPO}/issues/{pr}/timeline", "--paginate"])
except subprocess.TimeoutExpired:
# #19643 (residu) : un ensemble vide lirait la reserve d'un tiers comme
# levee. Une expiration remonte jusqu'au verdict non-CLEAN de main().
raise
except Exception:
return set() # timeline illisible : on retombe sur l'ancien comportement
if not isinstance(reviews, list) or not isinstance(events, list):
Expand Down Expand Up @@ -5668,11 +5686,20 @@ def main() -> int:
ap.add_argument("--search", help="filtre gh (ex: 'merged:2026-07-01..2026-07-15')")
ap.add_argument("--json", action="store_true", help="sortie machine (mode gate)")
args = ap.parse_args()
if args.audit:
return audit(args.limit, args.search)
if args.pr is None:
ap.error("fournir un numero de PR, ou --audit")
return gate(args.pr, args.json)
try:
if args.audit:
return audit(args.limit, args.search)
if args.pr is None:
ap.error("fournir un numero de PR, ou --audit")
return gate(args.pr, args.json)
except subprocess.TimeoutExpired as exc:
# #19643 (residu) : gh n'a pas repondu dans sa borne -- aucun verdict.
# Non-CLEAN (rc=1, fail-closed), jamais un rc=0 ni une trace nue.
# ASCII seul : stderr peut etre une console cp1252 (cf #16103).
print(f"NON MESURE : gh n'a pas repondu en {exc.timeout}s "
f"({' '.join(map(str, exc.cmd))[:120]}) -- aucun verdict, rc=1 (fail-closed)",
file=sys.stderr)
return 1


if __name__ == "__main__":
Expand Down
96 changes: 96 additions & 0 deletions scripts/tests/test_check_unaddressed_nits_19643.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
"""Regression #19643 (residu) : aucun appel `gh` de l'organe B.0 n'est sans borne.

Le 2026-10-09, sur `origin/main`, `scripts/check_unaddressed_nits.py` ne
contenait aucune occurrence de `timeout` : ses 11 sites d'appel passent par
`gh_json`, qui lancait `subprocess.run` sans borne. Un `gh` qui ralentit
(secondary rate limit) bloquait donc la lecture du merge-gate B.0 sans
jamais rendre de verdict.

Remede : `gh_json` porte une borne par appel (`GH_CALL_TIMEOUT_S`). Une
expiration n'est JAMAIS avalee :

- `_gh_fetch_issue` ne met pas `None` en cache sur expiration (le `None`
signifie « numero qui n'est pas une issue », pas « gh n'a pas repondu ») ;
- `improper_dismissals` ne rend pas `set()` sur expiration (un ensemble vide
lirait la reserve d'un tiers comme levee : rc=0 faux) ;
- `main()` transforme l'expiration en « NON MESURE » sur stderr et rend 1
(non-CLEAN, fail-closed).

Les appels `gh` sont remplaces par des fausses fonctions : aucun reseau.
"""
import importlib.util
import subprocess
import sys
from pathlib import Path

import pytest

SCRIPT = Path(__file__).resolve().parents[1] / "check_unaddressed_nits.py"
spec = importlib.util.spec_from_file_location("check_unaddressed_nits_19643", SCRIPT)
mod = importlib.util.module_from_spec(spec)
spec.loader.exec_module(mod)


def _expiration(argv, **kwargs):
"""Remplace `subprocess.run` : simule un `gh` qui depasse sa borne."""
raise subprocess.TimeoutExpired(cmd=argv, timeout=kwargs.get("timeout", 0))


def test_gh_json_porte_une_borne_par_appel(monkeypatch):
# La borne est passee a subprocess.run : sans elle, un gh bloque reste
# sans borne (l'etat de origin/main avant #19643-residu).
appels = {}

def faux_run(argv, **kwargs):
appels.update(kwargs)
return subprocess.CompletedProcess(argv, 0, stdout="{}", stderr="")

monkeypatch.setattr(subprocess, "run", faux_run)
assert mod.gh_json(["api", "repos/jsboige/CoursIA/pulls/1/reviews"]) == {}
assert "timeout" in appels, "gh_json doit passer timeout= a subprocess.run"
assert appels["timeout"] == mod.GH_CALL_TIMEOUT_S
assert 0 < mod.GH_CALL_TIMEOUT_S <= 120


def test_improper_dismissals_propage_l_expiration(monkeypatch):
# Avant le correctif : l'expiration tombait dans `except Exception` et
# rendait set(), donc une dismissal d'un tiers etait lue comme levee.
monkeypatch.setattr(subprocess, "run", _expiration)
with pytest.raises(subprocess.TimeoutExpired):
mod.improper_dismissals(1)


def test_gh_fetch_issue_ne_cache_pas_none_sur_expiration(monkeypatch):
# Avant le correctif : l'expiration donnait None, mis en cache pour ce
# numero (« pas une issue »), donc une levee par issue etait refusee a
# tort pour le reste du processus.
cache = {}
monkeypatch.setattr(mod, "_ISSUE_INFO_CACHE", cache)
monkeypatch.setattr(subprocess, "run", _expiration)
with pytest.raises(subprocess.TimeoutExpired):
mod._gh_fetch_issue(999991)
assert 999991 not in cache


def test_gh_fetch_issue_404_reste_none_et_cache(monkeypatch):
# Controle positif : un echec franc (404, numero qui n'est pas une issue)
# garde son sens d'origine, None mis en cache (#13725).
cache = {}
monkeypatch.setattr(mod, "_ISSUE_INFO_CACHE", cache)

def not_found(argv, **kwargs):
raise subprocess.CalledProcessError(1, argv)

monkeypatch.setattr(subprocess, "run", not_found)
assert mod._gh_fetch_issue(999992) is None
assert cache == {999992: None}


def test_main_rend_non_mesure_sur_expiration(monkeypatch, capsys):
# Avant le correctif : l'expiration remontait nue jusqu'a main() (traceback,
# aucun verdict). Apres : « NON MESURE » sur stderr et rc=1, jamais rc=0.
monkeypatch.setattr(mod.gh_identity, "pin_gh_token", lambda: "")
monkeypatch.setattr(subprocess, "run", _expiration)
monkeypatch.setattr(sys, "argv", ["check_unaddressed_nits.py", "123"])
assert mod.main() == 1
assert "NON MESURE" in capsys.readouterr().err
Loading