Skip to content

fix(coordination,#17672): merge_ready isole l'erreur d'une PR au lieu d'arreter le balayage - #17742

Merged
myia-ai-01 merged 1 commit into
mainfrom
fix/17672-merge-ready-error-isolation
Sep 25, 2026
Merged

myia-ai-01 merged 1 commit into
mainfrom
fix/17672-merge-ready-error-isolation

Conversation

@jsboige

@jsboige jsboige commented Sep 25, 2026 •

Copy link
Copy Markdown
Owner

Grain: MED/tooling -- lane myia-po-2026:CoursIA -- prev: MED/tooling #17739

Point 3 de #17672 : une PR en erreur ne gele plus le balayage

Le point 3 laissait une decision a trancher : « distinguer les erreurs qui touchent toute la passe (jeton, reseau) de celles qui ne concernent qu'une PR. » Cette PR la tranche en code, avec le test qui tombe avant et passe apres que l'issue demande. Les points 1 (disposition de review a la tete exacte) et 2 (etat REVIEW_READY) restent hors perimetre.

Etat avant

La boucle de run() traitait toute UnexpectedError de la meme facon — break apres avoir journalise la PR fautive :

except UnexpectedError as exc:
    errored = PRVerdict(pr, None, "run-error", str(exc), False)
    results.append(errored); append_journal(journal_path, errored)
    stopped_reason = f"unexpected-error:{exc}"; exit_code = 1
    break        # <- les PRs suivantes ne sont meme pas evaluees

Une reponse illisible pour une seule PR arretait donc l'evaluation de toutes les autres, alors que l'organe est precisement un balayage de queue : c'est la PR suivante qui porte la valeur.

La decision, et pourquoi celle-la

Cas Traitement Raison
Erreur attribuable a la PR en cours (reponse illisible pour elle, rc hors contrat pour elle) run-error journalisee pour elle, le balayage continue l'incident ne dit rien des autres PRs ; continuer coute une PR, s'arreter coute le reste de la queue
Erreur de portee generale — jeton refuse, quota d'API, reseau injoignable (PASS_WIDE_ERROR_MARKERS) arret du run, comme avant la repeter sur chaque PR restante ne dirait rien de plus, et l'isoler fabriquerait N run-error attribuees a des PRs innocentes
Refus de merge (MergeFailedError) arret (inchange) disjoncteur deja separe : l'isolation ne l'absorbe pas

Deux points d'honnetete du rapport, sans quoi la decision fabriquerait un faux vert :

  • stopped_reason reste None quand le run ne s'est pas arrete : ecrire « arret » dans le rapport serait un constat faux (le champ est imprime tel quel par emit_output). La distinction balayage-complet / arret vit dans stopped_reason, pas dans le code de sortie.
  • le bilan nomme les erreurs isolees (bilan : 2 evaluee(s), ..., 1 erreur(s) isolee(s)) : sans ce compte, un run qui a continue malgre une PR en erreur se lirait comme un run propre.
  • rc=1 reste rendu des qu'une PR a erreur — isolee ou non. Une erreur inattendue n'est jamais un run propre ; c'est au lecteur de distinguer les deux cas par stopped_reason et le bilan, pas au code de sortie de les confondre.

La classification par texte est assumee : les appels gh renseignent le message d'erreur avec le stderr de l'outil (fetch_pr_view, run_gate), donc un refus d'authentification ou un quota epuise y figure tel que gh l'ecrit. La liste est volontairement courte et le sens de l'erreur par defaut est celui de l'isolation : un texte non reconnu fait continuer le balayage (rc=1, PR nommee), jamais arreter — arreter sur un motif devine rendrait l'organe dependant d'une devinette.

Falsification mesuree

Test Avant le correctif Apres
test_erreur_dune_pr_est_isolee_et_le_balayage_continue (vue de 201 illisible, 202 normale) ECHOUE — assert [201] == [201, 202] : la PR 202 n'est jamais evaluee, stdout porte arret : unexpected-error:gh pr view 201 ... passe — journal [201 run-error, 202 merged], bilan 1 erreur(s) isolee(s), aucune ligne arret :
test_erreur_de_portee_generale_arrete_le_run (gate rc=5 + Bad credentials (HTTP 401)) passe passe (inchange)
test_merge_echoue_arrete_le_run (merge refuse) passe passe (inchange)
test_is_pass_wide_classe_par_le_texte_de_l_outil erreur AttributeError (la fonction n'existe pas) passe

Le controle de portee generale passe des deux cotes : le fail-closed n'est pas invente par cette PR, il est preserve. C'est ce qui interdit de « reparer » le defaut en adoucissant tout.

Suite et preuves

Preuve Resultat
python -m pytest scripts/tests/test_merge_ready.py -q 43 passed in 0.76s (41 avant : 1 test remplace par 3)
python scripts/coordination/merge_ready.py --help rend l'aide, import sain
Hooks pre-commit verts (gitleaks, encoding= sur text=True)

Limite declaree : les tests passent par le ScriptedRunner de la suite, pas par un gh reel — le declencheur du defaut (une reponse illisible pour une PR) est justement ce qu'un appel reel ne produit pas a la demande. Le declencheur du cas de portee generale (gh refusant le jeton) est lui aussi scripte, pas observe en CI.

Perimetre

2 fichiers, +123 / −11 : scripts/coordination/merge_ready.py (docstring de module et UnexpectedError mis a jour, PASS_WIDE_ERROR_MARKERS + is_pass_wide, la boucle, le bilan) et son test. Aucun notebook, aucun catalogue, aucune dependance. La PR ouverte #17703 touche le meme fichier (gel des campagnes) : hunks disjoints, rebase trivial si besoin.

See #17672 — point 3 livre ; points 1 et 2 restent a faire.

🤖 Generated with Claude Code

… d'arreter le balayage

Le balayage s'arretait sur la PREMIERE erreur inattendue : une seule PR mal
formee (reponse d'outil illisible pour elle) gelait l'evaluation de toutes les
suivantes, alors que rien dans l'incident ne concernait ces PRs.

Le point 3 de #17672 demandait de trancher : distinguer ce qui touche TOUTE la
passe (jeton, quota, reseau) de ce qui ne concerne qu'une PR. Tranche ainsi :

* une erreur attribuable a la PR en cours est journalisee `run-error` pour
  elle et le balayage CONTINUE ;
* une erreur de portee generale (marqueurs PASS_WIDE_ERROR_MARKERS : jeton
  refuse, quota d'API, reseau injoignable) ARRETE le run comme avant -- la
  repeter sur chaque PR restante ne dirait rien de plus ;
* `stopped_reason` reste None quand rien ne s'est arrete (publier « arret »
  serait un constat faux), et le bilan nomme desormais les erreurs isolees,
  sans quoi elles disparaitraient du rapport ;
* MergeFailedError garde son disjoncteur : l'isolation ne l'etend pas au refus
  de merge (test de non-regression inchange).

Falsification mesuree : le test d'isolation tombe AVANT le correctif
(`assert [201] == [201, 202]`, la PR 202 n'etait jamais evaluee) et passe
apres. Le controle de portee generale passe des deux cotes -- le fail-closed
n'est pas invente, il est preserve.

Points 1 (disposition de review a la tete exacte) et 2 (etat REVIEW_READY) de
#17672 restent hors de cette PR.

See #17672

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

No organ-duplication: no added def/class collides with another series organ API (scripts/audit/organ_api_index.yaml).

Detector: python scripts/audit/detect_organ_duplication.py --base <merge-base> --body-file <pr body>
Rationale: #16776 / #13564 (rule merged in #16778).

@github-actions

github-actions Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

prev: genre mot-clé fermant (#10093) — LEVÉ (2026-09-25T04:02:14Z).

aucun genre mots-clé fermant dans le body ni les commits ; prev: accepté(s) : #17739

Run vert du garde : ce commentaire bloquant est obsolète. Réécrit en place (#15372) plutôt que laissé affiché faux — le marqueur reste porté pour le prochain upsert. Historique : runs Always-on guards de la PR.

@github-actions

Copy link
Copy Markdown
Contributor

Path-collision (organ #13359/#13615)

Cette PR #17742 (fix(coordination,#17672): merge_ready isole l'erreur d'une PR au lieu d'arreter le balayage) touche au moins un chemin de fichier aussi modifie par d'autres PRs ouvertes. Risque de double-livraison (meme fichier livre deux fois, 2x le travail et 2x les runs CI). Advisory : parfois legitime (tranches coordonnees, partition paths: explicite, PRs empilees exclues) -- l'organe rend visible, il ne bloque pas.

Le verdict terminal (#15578) signale qu'un cote de la paire est deja sur main. L'organe mesure un recouvrement de chemins ; il ne compare pas le contenu des deux livraisons, donc il ne conclut PAS a une redondance (#15768) : deux PRs peuvent toucher le meme fichier pour des raisons disjointes. L'arbitrage reste a la lane ou au coordinateur.

@github-actions github-actions Bot added the pr-overlap Advisory: another open PR touches the same files (organ #13615) label Sep 25, 2026
@jsboige

jsboige commented Sep 25, 2026

Copy link
Copy Markdown
Owner Author

[ADJOINT PREFLIGHT]
schema: 1
lane: myia-po-2025:CoursIA-2
pr: 17742
head: 88e889f
complete: true
body: read
comments-reviewed: 3
reviews-reviewed: 0
threads-reviewed: 0
threads-unresolved: 0
surfaces-sha256: 3b55243a027382b5537efbf50bc017e7b843a60ba67ccece009cf767c148a0a6
diff-files: 2
diff-additions: 123
diff-deletions: 11
checks: latest-wins-green
b0: clear
scope: pass
domain: not-applicable
verdict: READY
[/ADJOINT PREFLIGHT]

@myia-ai-01
myia-ai-01 merged commit a0bd367 into main Sep 25, 2026
22 of 25 checks passed
jsboige added a commit that referenced this pull request Sep 26, 2026
Conflit merge_ready.py (bilan) et test_merge_ready.py resolu en UNION :
- #17742 (main) : isolation par PR + compte « erreur(s) isolee(s) » au bilan
- #17743 (branche) : review_disposition + reviews dans PR_VIEW_FIELDS
  + compte « candidates : N approved-exact-head, N approval-not-on-head »
Tests : scripts/tests/test_merge_ready.py 56 passed / 0 failed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-overlap Advisory: another open PR touches the same files (organ #13615)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants