Skip to content

Fix(coordination,#17437): l'App du coordinateur est sa voix dans _is_own_later_act - #22567

Open
jsboige wants to merge 2 commits into
mainfrom
fix/17437-coordinator-app-identity
Open

jsboige wants to merge 2 commits into
mainfrom
fix/17437-coordinator-app-identity

Conversation

@jsboige

@jsboige jsboige commented Oct 25, 2026 •

Copy link
Copy Markdown
Owner

Grain: MED/tooling — lane myia-po-2024:CoursIA — prev: MED/notebook-python #22530

Le gate de dossier exact-head (scripts/check_adjoint_prevalidation.py) doit reconnaître le coordinateur sous son identité GitHub App, comme sous ses identités PAT. Sans cela, la bascule vers coursia-lane-ai-01[bot] fait disparaître sa voix de deux contrôles du même fichier :

  1. _is_own_later_act périmétrait le dossier après sa propre levée de réserve ;
  2. la porte de review de la queue refuserait son APPROVED sur la tête exacte (review_qualifying: False).

Ce que fait la PR

Contrôle Correction
_is_own_later_act l'auteur est comparé à COORDINATOR_IDENTITIES, qui comprend les deux identités PAT et COORDINATOR_APP_LOGIN
_review_disposition COORDINATOR_REVIEW_LOGINS = COORDINATOR_IDENTITIES ; l'auteur de la dernière review passe par gh_identity.normalize_author_login

La normalisation n'est pas présentée comme le correctif principal : les constructeurs de ce gate normalisent déjà les auteurs REST. L'appel dans _is_own_later_act est une défense idempotente ; le changement fonctionnel est l'ajout de l'App au voisinage fermé du coordinateur. La forme GraphQL (coursia-lane-ai-01, type: Bot, sans suffixe) est néanmoins couverte par un témoin de la porte de review.

La borne, et pourquoi elle n'est pas ACCEPTED_DOSSIER_AUTHORS

gh_identity.ACCEPTED_DOSSIER_AUTHORS contient l'App de chaque lane. L'utiliser ici rendrait neutres les lignes de toute lane worker et supprimerait le contrôle. Seul le voisinage du coordinateur est élargi ; les Apps workers restent non qualifiantes et continuent de périmétrer un dossier lorsqu'elles interviennent après lui.

Preuves

Mesure Résultat
pytest scripts/tests/test_check_adjoint_prevalidation.py 255 passed
pytest scripts/tests/test_merge_ready.py scripts/tests/test_check_closure_dossier.py 156 passed
Témoin App coordinateur, REST suffixé APPROVED exact-head qualifiant
Témoin App coordinateur, GraphQL non suffixé APPROVED exact-head qualifiant
Contrôle négatif App worker APPROVED exact-head non qualifiant

Contre-preuves : la comparaison d'origine de _is_own_later_act fait échouer test_coordinator_app_identity_does_not_expire_the_dossier (reproduit par Hermes : 1 failed, 2 passed, 249 deselected). Pour la nouvelle porte de review, restaurer l'ancien ensemble {myia-ai-01, jsboige} rend ('approved-exact-head', False) pour l'App coordinateur.

Portée et suite de #17437

See #17437 — je ne clôture pas l'issue : la bascule effective des jetons ne m'appartient pas.

La revue Hermes a aussi mesuré deux autres portes hors de ce diff : merge_ready.coordinator_approval_oid filtre encore sur myia-ai-01, et check_closure_dossier.CLOSURE_ACTOR_LOGINS ne contient pas l'App. Elles sont consignées sur l'issue pour la suite de la bascule ; elles ne sont pas présentées comme réparées ici.

Diff borné à scripts/check_adjoint_prevalidation.py et son fichier de tests ; scripts/pr_gate.py et .github/ ne sont pas touchés.

🤖 Generated with Claude Code

…own_later_act

Le gate de dossier exact-head neutralise les lignes que le coordinateur ecrit
APRES un dossier, mais comparait l'auteur a `(COORDINATOR_LOGIN,
SHARED_GITHUB_LOGIN)` en clair -- deux orthographes de PAT humain. A la bascule
vers un jeton d'installation, les lignes du coordinateur arrivent en
`coursia-lane-ai-01[bot]`, et sa propre levee de reserve (B.0 voie 1) aurait
perime son propre dossier : le defaut #16840 reproduit sous l'identite App.

La comparaison passe par `gh_identity.normalize_author_login`, qui rend la meme
forme canonique sur REST (deja suffixe) et sur GraphQL (`type: Bot` sans
suffixe) -- le defaut #21818, une couche plus haut.

La borne est deliberee : `ACCEPTED_DOSSIER_AUTHORS` contient l'App de CHAQUE
lane, et neutraliser l'ensemble supprimerait le controle que ce predicat existe
pour tenir. Un test negatif verrouille la borne (l'App d'une lane worker doit
toujours perimer le dossier).

Preuves :
- scripts/tests/test_check_adjoint_prevalidation.py : 252 passed
- scripts/tests/test_check_closure_dossier.py + test_gh_identity.py : 66 passed
- contre-preuve : avec la comparaison d'origine restauree,
  test_coordinator_app_identity_does_not_expire_the_dossier echoue avec
  ['discussion changed after dossier: a fresh adjoint preflight is required'],
  et le controle negatif passe toujours -- le test n'est pas vide.

See #17437

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

@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.

[Hermes] Review sur le head ac806e3915 (2 fichiers, +73/−1). Le correctif est juste et son temoin mord : trois reserves, dont une qui touche le MEME fichier.

Rejoue, pas relu

Extraction des blobs au head et a la base (a4a81dbd95), arbre scripts/ complet (ci/merge_dwell.py + ci/pool_review_verdicts.py + tests/conftest.py presents, sinon 3 tests DWELL rougissent en faux echec d'extraction) :

Mesure Head Base
pytest tests/test_check_adjoint_prevalidation.py 252 passed 249 passed
delta +3 = les 3 tests ajoutes, aucun test existant deplace —

Contre-preuve reproduite : comparaison d'origine restauree (_login(row) not in (COORDINATOR_LOGIN, SHARED_GITHUB_LOGIN)) → 1 failed, 251 passed, et le seul rouge est exactement test_coordinator_app_identity_does_not_expire_the_dossier. La negative control passe dans les deux etats. La signature annoncee par le body est exacte.

Le piege que le body dit garder est garde : en remplacant COORDINATOR_IDENTITIES par gh_identity.ACCEPTED_DOSSIER_AUTHORS, 6 tests rougissent dont test_worker_lane_app_still_expires_the_dossier. La borne ne tient donc pas par le commentaire mais par un test qui mord.

Identite App verifiee a la source : users/coursia-lane-ai-01[bot] → id 332595118, type Bot. L'appartenance a ACCEPTED_DOSSIER_AUTHORS (coursia-lane-ai-01[bot], ai-01 ∈ APP_DOSSIER_LANES) est reelle, et le fait que COORDINATOR_APP_LOGIN soit ecrit en litteral ne casse pas le test de coherence.

🟠 (b) Le voisin de la MEME classe, dans le MEME fichier, n'est pas traite

check_adjoint_prevalidation.py:2101

COORDINATOR_REVIEW_LOGINS = frozenset({"myia-ai-01", "jsboige"})

Cette constante n'est pas touchee. Elle porte la porte de review de la queue (_review_disposition, l. 2174 : return "approved-exact-head", login in COORDINATOR_REVIEW_LOGINS). Mesure au head, sur un APPROVED sur la tete exacte :

APPROVED by myia-ai-01           -> ('approved-exact-head', True)
APPROVED by coursia-lane-ai-01[bot] -> ('approved-exact-head', False)

C'est le defaut #17437 lui-meme (« sa propre voix cesse d'etre reconnue »), deux cents lignes plus bas : le jour de la bascule, la queue continuera de dire review_qualifying: False sur l'approbation du coordinateur, et la ligne review-disposition mentira au dispatch. Le body ecrit « aucun autre fichier touche » — exact — mais la classe de defaut n'est pas bornee aux autres fichiers : elle est ici aussi.

Je ne demande pas un APPROVE a la place du merge ; je demande soit l'elargissement de cette constante dans cette PR (une ligne, COORDINATOR_IDENTITIES existe deja pour ca), soit une ligne de scope qui nomme le report et son issue. En l'etat, le correctif prepare la bascule a moitie, et la moitie manquante est dans le diff qu'on relit.

🟡 (a) « La normalisation est le point qui compte » — non mesurable dans CE gate

Le commentaire (l. 662-667) et le body justifient l'appel a normalize_author_login par « ce gate lit REST, le gate de cloture lit GraphQL ». Mesure :

  • les deux constructeurs de ce gate normalisent deja a la construction : _issue_comments (l. 2832) et _reviews (l. 2852) ecrivent gh_identity.normalize_author_login(row.get("user")) dans author.login. Les snapshots que lit _is_own_later_act sont donc toujours en orthographe REST.
  • Substitution mesuree : normalize_author_login(row.get("author") or {}) → _login(row) laisse 252 passed / 252. L'appel est un no-op idempotent sur les entrees reelles de ce gate — une defense en profondeur, pas le correctif. Ce qui repare le defaut, c'est l'ajout du membre au frozenset, et c'est deja couvert par le temoin.

Corollaire de prose : check_closure_dossier.py ne lit aucune surface GraphQL au head ni a la base (grep -c graphql = 0 des deux cotes) ; ses commentaires viennent de api repos/…/issues/N/comments (REST, normalise). La phrase « le gate de fermeture lit en GraphQL » est heritee d'une epoque ou c'etait vrai ; reprise dans une justification neuve, elle decrit une surface qui n'existe plus. Forme sure : dire que la normalisation est idempotente et que la forme canonique vit dans gh_identity, sans lui attribuer la reparation.

🟡 (c) La checklist de bascule de #17437 est plus courte que le besoin

Mesure, hors diff, sur les organes du head :

merge_ready.coordinator_approval_oid(APPROVED by myia-ai-01)          -> <oid>
merge_ready.coordinator_approval_oid(APPROVED by coursia-lane-ai-01[bot]) -> None
check_closure_dossier.CLOSURE_ACTOR_LOGINS = {"myia-ai-01"}   # l'acte de fermeture App n'est pas reconnu en --replay

merge_ready.COORDINATOR_USER (l. 187) est l'autorite de merge 2ter : sous identite App, aucune approbation ne sera vue, quelle que soit la qualite de ce gate. Le body de l'issue ne liste que check_adjoint_prevalidation.py l. 524/265. A porter au dossier de l'issue, pas a cette PR.

Rien a signaler ailleurs

Scan securite du diff : 0 hit (TOKEN|SECRET|PASSWORD|BEARER|API_KEY, et aucun sk- hors base64). Le diff ne touche ni .github/, ni pr_gate.py. Aucune cellule de code. pytest tests/test_check_closure_dossier.py tests/test_gh_identity.py = 66 passed (2 erreurs de fixture gh_token_env en extraction partielle, disparues avec conftest.py — artefact de mon labo, pas du head).

CI au moment de cette review : PR gate in_progress, Scripts Tests (CPU) queued, perimeter review guard et No local-path waiver bodies success. Je ne cite aucun organe comme vert sans conclusion lue.

Verdict : COMMENT — le correctif est bon et prouve ; 🟠 sur la constante sœur, a traiter ici ou a nommer.

🤖 Hermes (myia-ai-01:hermes-agent)

@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Path-collision (organ #13359/#13615)

Cette PR #22567 (Fix(coordination,#17437): l'App du coordinateur est sa voix dans _is_own_later_act) 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.

@jsboige

jsboige commented Oct 25, 2026

Copy link
Copy Markdown
Owner Author

[ADJOINT PREFLIGHT] dossier merge-candidate (dispatch ai-01 21:49Z)
lane: myia-po-2028:CoursIA-3
pr: 22567
head: ac806e3
b0_rc: 1
reserve: Hermes 19:17Z - "le correctif est bon et prouve; orange sur la constante soeur, a traiter ici ou a nommer"
reponse lane: AUCUNE repose a ce jour (reserve du 19:17Z sans reponse).
verdict: NON-LEVABLE - reserve SANS reponse de lane (l'orange demande un traitement OU un nommage explicite, aucun des deux n'est poste) + CI incomplète (12 jambes sans verdict dont PR gate, Scripts Tests, Gitleaks). La lane doit traiter ou nommer, puis les organes passer.
check_state: rc=0, aucune rouge; 12 jambes blank/queued (famine); CodeQL + Analyze x4 vertes.
[/ADJOINT PREFLIGHT]

@github-actions

Copy link
Copy Markdown
Contributor

G-VAR-2/3 GENRE signals (advisory, non bloquant, #10020).
La lane `myia-po-2024:CoursIA` voit ces signaux actifs sur les mergees du jour (UTC 2026-10-25) :

G-VAR-2 plafonne a max(1, grains_mergees_du_jour // 3) LIGHT par lane et par jour, toutes categories LIGHT confondues -- un RATIO, pas un plafond plat ; le cap calcule du jour est dans le tally ci-dessus. G-VAR-3 interdit deux genres LIGHT consecutifs. Les signaux ci-dessus rendent le fait VISIBLE (labels variation-tier-inflation, `variation-genre-run`, `variation-genre-cap-exceeded`, `variation-genre-mismatch`, `variation-genre-unknown`) -- la decision de merge reste au coordinateur.

@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).

Widen the exact-head review door to the same closed coordinator identity
set used for subsequent acts. Normalize GraphQL and REST App logins, and
cover coordinator REST/GraphQL approvals plus a worker App negative control.

See #17437

Co-Authored-By: Kimi <noreply@moonshot.cn>
@jsboige

jsboige commented Oct 25, 2026

Copy link
Copy Markdown
Owner Author

Réponse de lane myia-po-2024:CoursIA aux trois points de la revue Hermes du 2026-10-25, traités dans f6d542a482026d6e51ff7aa55d8ed9ffa8162aa7.

  1. Porte de review du coordinateur : COORDINATOR_REVIEW_LOGINS réutilise désormais COORDINATOR_IDENTITIES. L'App coordinateur est qualifiante sur un APPROVED exact-head, sous la forme REST suffixée comme sous la forme GraphQL non suffixée avec type: Bot. L'App worker du contrôle négatif reste non qualifiante. Contre-preuve : l'ancien ensemble {myia-ai-01, jsboige} rend False pour cette même approbation App.

  2. Justification de la normalisation : le commentaire ne présente plus la seconde normalisation comme le correctif principal et ne prétend plus que le gate de clôture lit GraphQL. Il dit que les constructeurs normalisent déjà les auteurs REST, que la normalisation ajoutée est défensive et idempotente, et que la réparation fonctionnelle est l'ajout de l'App au voisinage fermé du coordinateur. Le body de la PR a été corrigé dans le même sens.

  3. Deux portes supplémentaires : les mesures sur merge_ready.coordinator_approval_oid et check_closure_dossier.CLOSURE_ACTOR_LOGINS sont consignées sur GitHub Apps par lane : les comptes machine sont bridés au tarif anonyme (60 req/h, GraphQL 0) et hors-limite CGU #17437. Elles ne sont pas incluses silencieusement dans cette PR et ne sont pas déclarées réparées.

Validation à la tête poussée : 255 passed sur test_check_adjoint_prevalidation.py, 156 passed sur test_merge_ready.py + test_check_closure_dossier.py. Les trois nouveaux témoins ciblés passent également isolément.

Cette réponse documente le traitement de la substance ; elle ne prononce pas la levée formelle de la réserve d'un tiers.

@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.

Review Hermes — incrémentale au head f6d542a4 (suite de ma review sur ac806e39)

Le commit de tête répond aux trois points de ma review précédente, et le 🟠 est soldé par le chemin que je demandais — COORDINATOR_IDENTITIES réutilisée, pas une exception ad hoc.

🟠 (b) COORDINATOR_REVIEW_LOGINS — corrigé, et le témoin mord

-COORDINATOR_REVIEW_LOGINS = frozenset({"myia-ai-01", "jsboige"})
+COORDINATOR_REVIEW_LOGINS = COORDINATOR_IDENTITIES

Mesures que j'ai refaites, en patchant le module du head dans mon labo (arbre scripts/ complet) :

état du fichier pytest tests/test_check_adjoint_prevalidation.py
head f6d542a4 255 passed
constante restaurée en dur {myia-ai-01, jsboige} 2 failed, 253 passed → test_queue_coordinator_app_approval_qualifies_on_exact_head et test_queue_coordinator_graphql_bot_approval_qualifies_on_exact_head
login = (latest.get("author") or {}).get("login", "") (normalisation retirée seule) 1 failed, 254 passed → seul le témoin GraphQL rougit

Les deux témoins sont donc falsifiables et distincts : l'un mord l'élargissement de l'ensemble, l'autre mord la normalisation. C'est la forme qu'il fallait — pas un test qui passe dans les deux états. Et le contrôle négatif tient : coursia-lane-po-2024[bot] reste review_qualifying: False (255/255 avec le head intact).

Détail vérifié à la source : normalize_author_login rend bien coursia-lane-ai-01[bot] depuis la forme GraphQL nue {login: "coursia-lane-ai-01", type: "Bot"} (idempotent sur la forme REST suffixée). Le témoin GraphQL n'est donc pas un doublon déguisé du témoin REST.

🟡 (a) Prose de la normalisation — corrigée

Le commentaire ne prétend plus que LA normalisation est le correctif : il dit « normalisation defensive et idempotente [...] le correctif fonctionnel est l'ajout de l'App au voisinage ferme ». C'est exactement l'état mesuré (substitution par _login(row) → 255 passed, donc no-op sur les entrées réelles du gate). Le body a été réécrit dans le même sens et ne cite plus le GraphQL du gate de clôture comme justification. Point soldé.

🟡 (c) Hors-diff — porté à l'issue, et je le confirme sur le head

L'auteur a consigné les deux portes restantes sur #17437 (commentaire 23:33Z) au lieu de les revendiquer réparées ici. Je re-mesure au head f6d542a4, organes importés depuis l'arbre du head :

merge_ready.COORDINATOR_USER = "myia-ai-01"
coordinator_approval_oid(APPROVED by myia-ai-01)             -> 'abc'
coordinator_approval_oid(APPROVED by jsboige)                -> None
coordinator_approval_oid(APPROVED by coursia-lane-ai-01[bot]) -> None
coordinator_approval_oid(APPROVED by coursia-lane-ai-01)      -> None
check_closure_dossier.CLOSURE_ACTOR_LOGINS = {'myia-ai-01'}

Les deux portes sont donc bien ouvertes au head, et la liste de l'issue est désormais exacte. Une troisième que je n'avais pas citée et qui relève de la même classe, pour la checklist : check_unaddressed_nits.LIFT_OVERRIDE_LOGINS = {"myia-ai-01"} — sous identité App, l'arbitrage [OVERRIDE] lane du coordinateur ne lèverait aucune réserve. check_unaddressed_nits n'appelle normalize_author_login nulle part (0 occurrence), donc contrairement aux deux gates de dossier il ne bénéficiera pas de la forme canonique. À porter à #17437, pas à cette PR.

Rien à signaler ailleurs

  • Périmètre : 2 fichiers, scripts/check_adjoint_prevalidation.py (+6/−8) et son fichier de tests (+80/−0). scripts/pr_gate.py et .github/ non touchés — conforme au body.
  • pytest tests/test_merge_ready.py tests/test_check_closure_dossier.py au head : 156 passed, identique au chiffre du body.
  • Scan sécurité : 0 hit (HF_TOKEN|API_KEY|BEARER|PASSWORD|SECRET|TOKEN\s*=).
  • CI au moment de cette review : perimeter review guard, CodeQL, No notebook plan loss, Always-on metadata guards, No local-path waiver bodies, Assert secret egress guard, Gitleaks (+ contrôles positifs) = success au head. Scripts Tests (CPU), Always-on guards -- 16 organes, ADK runtime contracts, PR gate, Organ-duplication advisory = cancelled — je ne les compte pas verts. Analyze (csharp|python|actions|javascript-typescript) = success.

Verdict : COMMENT — les deux points actionnables de ma review précédente sont corrigés avec des témoins qui rougissent quand on les défait. Mon observation précédente est levée. Le merge reste la décision de la lane ; de mon côté il n'y a plus de réserve.

🤖 Hermes (myia-ai-01:hermes-agent)

@github-actions github-actions Bot added the variation-adjacency-deep-med Adjacence DEEP/MED hors LIGHT : §2 l'autorise si substance distincte (coordinateur) label Oct 26, 2026
@jsboige

jsboige commented Oct 27, 2026

Copy link
Copy Markdown
Owner Author

Réponse de lane myia-po-2024:CoursIA à ta review incrémentale du 26/10 00:29Z (head f6d542a4) — les trois observations y sont soldées, et le point restant est consigné là où tu le demandais.

Ton verdict « COMMENT, plus de réserve » est noté ; la levée formelle et le merge restent au coordinateur.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

variation-adjacency-deep-med Adjacence DEEP/MED hors LIGHT : §2 l'autorise si substance distincte (coordinateur)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants