Skip to content

fix(guard,#18823): twin_collision_reason fail-closed sur mix MULTI-PR + verdict absent - #18842

Merged
jsboige merged 1 commit into
feature/18725-merge-ready-twin-verdictfrom
fix/c1379-18762-partial-read
Oct 2, 2026
Merged

jsboige merged 1 commit into
feature/18725-merge-ready-twin-verdictfrom
fix/c1379-18762-partial-read

Conversation

@jsboige

@jsboige jsboige commented Oct 2, 2026

Copy link
Copy Markdown
Owner

Résumé

Fix twin_collision_reason (5bis de merge_ready) : un mix cross_ref portant à la fois des verdicts reconnus (ON-MAIN / MULTI-PR) et des verdicts absents/inconnus ne doit plus passer comme avertissement seul. any -> all sur le verdict reconnu.

Pourquoi

Le commentaire [ADJOINT VERIFIED] sur #18762 (tete 4ac0762bfc) a relevé que le câblage 5bis couvrait CLEAN, ON-MAIN, MULTI-PR et legacy-entier-sans-verdict, mais ratait le cas legacy partiel ([{verdict: MULTI-PR}, {verdict: absent}]). Le any acceptait la liste dès qu'un seul verdict était reconnu, puis classait comme avertissement seul. La 2e collision illisible pouvait être ON-MAIN, et le merge rendait main CI DRIFT-INTRO sans garde.

Fix

  • merge_ready.py ligne 850 : any(...) -> all(...) sur le verdict reconnu. La garde exige CHAQUE collision classifiée, sinon fail-CLOSED avec le motif historique twin-index-collision. Pas de changement d'API ni d'acceptance des cas normaux.
  • tests/test_merge_ready.py : 3 tests ajoutés (mix MULTI-PR+absent -> skip ; pure-MULTI-PR -> would-merge ; pure-ON-MAIN -> skip dur avec motif nommé).
  • ScriptedRunner étendu d'un champ twin_stdout pour injecter le JSON de classification.

Validation

  • python -m pytest scripts/tests/test_merge_ready.py : 73/73 verts (70 anciens + 3 nouveaux), 0 régression.
  • AST.parse OK, pas d'autre modification.

Diff

scripts/coordination/merge_ready.py | 14 ++++----
scripts/tests/test_merge_ready.py   | 67 ++++++++++++++++++++++++++++++++++++-
2 files changed, 74 insertions(+), 7 deletions(-)

Refs #18823 (PR mère, CHANGES_REQUESTED user, dénominateur et méthode), Refs #18762 (PR courante, dossier adjoint BLOCKED domain:fail).

🤖 Generated with Claude Code

Co-Authored-By: Claude Haiku 4.5 (1M context) noreply@anthropic.com

… + verdict absent

Le commentaire adjoint #18762 verifie par ai-01 (po-2025) sur la tete
4ac0762 a releve un cas que le câblage 5bis ne couvrait pas :
pour cross_ref=[{verdict: MULTI-PR}, {verdict: absent}], la garde
acceptait la liste des que ``any`` collision avait un verdict reconnu,
puis classait l'absence d'ON-MAIN comme MULTI-PR (avertissement seul).
La seconde collision illisible pouvait etre ON-MAIN, et le merge rendait
``main`` CI DRIFT-INTRO sans qu'aucune garde ne le bloque.

``any`` -> ``all`` : CHAQUE collision doit porter un verdict reconnu
(ON-MAIN ou MULTI-PR). Defaut -> motif historique ``twin-index-collision``,
identique au cas legacy entierement sans verdict. Pas de changement d'API
ni d'acceptance des cas normaux (pure-MULTI-PR avertit seul, pure-ON-MAIN
skip dur avec motif nomme).

Couverture : 3 tests (mix MULTI-PR+absent -> skip, pure-MULTI-PR ->
would-merge, pure-ON-MAIN -> skip dur). 73/73 verts.

Refs #18823 (PR mere, CHANGES_REQUESTED user, denominateur et methode),
Refs #18762 (PR courante, dossier adjoint BLOCKED domain:fail).

Co-Authored-By: Claude Haiku 4.5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Base != main (advisory, #10918)

Cette PR ne livre pas sur main : son contenu attend le merge de feature/18725-merge-ready-twin-verdict. 1 PR ouverte(s) de feature/18725-merge-ready-twin-verdict vers main existe(nt) a cet instant -- c'est un stack legitime, le contenu est en vol. Verifier au moment du merge que la base est effectivement reliee a main.

Couverture CI perdue sur cette base (mesure, #16194)

6 workflow(s) se declencheraient si cette PR visait main, et ne se declenchent pas ici : leur filtre de branche cible les eteint, alors que leur filtre de chemins est satisfait par les fichiers de cette PR.

  • always-on-guards.yml
  • notebook-plan-loss-gate.yml
  • organ-duplication-advisory.yml
  • pr-gate.yml
  • scripts-tests.yml
  • secret-scan.yml

Un check absent n'est pas un check vert. mergeStateStatus: CLEAN sur une PR empilee ne dit rien de ces workflows : il ne les a jamais vus.

@clusterManager-Myia clusterManager-Myia left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: LGTM — event APPROVE

Fix twin_collision_reason fail-closed sur mix MULTI-PR + verdict absent (#18823). Change atomique any → all juste, commentaire réécrit honnêtement, 3 tests comportementaux couvrant les 3 classes du cross_ref.

Preuve-vive firsthand au head 9167ed16 (exécution locale des 3 tests ajoutés, deps fetchées au même SHA) :

test_twin_mixed_multipr_and_unknown_then_fails_closed PASSED
test_twin_pure_multipr_warns_only                     PASSED
test_twin_pure_onmain_skips                           PASSED
3 passed in 0.19s

Gate (c) (le garde casse si on casse la garde) : j'ai reverté localement all → any et relancé le test anti-régression → FAILED (would-merge != skipped). L'assertion éprouve réellement le fix, ce n'est pas un test décoratif.

Lecture du diff :

  • merge_ready.py l.850 : any(...) → all(...) + commentaire qui généralise la justification (« verdict requis sur CHAQUE collision », mix verdict-connu + verdict-absent explicitement nommé). Logique fail-closed cohérente avec la classe historique twin-index-collision : on préfère refuser un mix illisible que laisser passer un ON-MAIN déguisé.
  • test_merge_ready.py : ajout twin_stdout au ScriptedRunner (les tests historiques twin-index-collision n'attendaient pas de classification — la 5bis l'exige maintenant), 3 tests ciblant exactement les 3 classes (mix/pure-MULTI-PR/pure-ON-MAIN).
  • 0 secret, 0 path leak.

Le bug relevé par l'[ADJOINT VERIFIED] sur #18762 était réel (collision legacy partielle classée warn-only), le fix est minimal, les tests éprouvent le bon comportement dans les deux sens.

[Hermes hermes-pr-review, cycle :13 02/10, host f6be46d1b7a3, sig=e4e51518]

@jsboige
jsboige merged commit 9167ed1 into feature/18725-merge-ready-twin-verdict Oct 2, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants