Skip to content

fix(ci,#c2180): autoriser _hybrid_runs_on pour pull_request_review (item 1 amend) - #21120

Closed
jsboige wants to merge 1 commit into
mainfrom
fix/c2180-hybrid-runs-on-linux-fork
Closed

jsboige wants to merge 1 commit into
mainfrom
fix/c2180-hybrid-runs-on-linux-fork

Conversation

@jsboige

@jsboige jsboige commented Oct 16, 2026

Copy link
Copy Markdown
Owner

URGENT amend c.2180 item 1 (dispatch 06:09Z)

Ferme l'item 1 du dispatch amend c.2180-ci-hosted-saturation. La forme hybride [_hybrid_runs_on] est maintenant acceptee pour les triggers pull_request_review et pull_request_review_comment (qui peuplent github.event.pull_request avec l'objet PR complet). L'expression elle-meme porte la garde universelle fork -> ubuntu-latest via le || fallback.

issue_comment reste REJETE (le champ PR y vit dans github.event.issue.pull_request, un objet URL, pas l'objet PR -- la garde universelle == null y serait vraie y compris pour un commentaire sur une PR fork, ce qui routerait le code fork vers self-hosted). Une garde field-aware specifique a issue_comment est un sujet separe (hors scope).

Subtilite (NAMING c.518-N1 ★★) : la fiabilite d'une garde GitHub Actions depend du trigger. Tester une garde sur un trigger et la deployer sur un autre peut etre silent bypass. Le sous-ensemble HYBRID_SAFE_FORK_REACHABLE_TRIGGERS est l'artefact de cette lecon : ce qui est sur pour pull_request_review ne l'est pas pour issue_comment. Un commentaire sur le PR ou l'issue a la meme cle (Commenter) mais la structure du payload differe.

Changement

  • scripts/ci/check_self_hosted_runner_policy.py : nouveau frozenset HYBRID_SAFE_FORK_REACHABLE_TRIGGERS = {"pull_request_review", "pull_request_review_comment"}. Pour un job en forme hybride, le check FORK_REACHABLE_TRIGGER saute si le trigger est dans ce sous-ensemble.

Tests (66/66 verts, +3 nouveaux)

  • test_hybrid_runs_on_linux_with_pull_request_review_trigger_is_accepted : cas positif, hybrid + pull_request_review -> 0 violation.
  • test_hybrid_runs_on_linux_with_pull_request_review_comment_trigger_is_accepted : cas positif, hybrid + pull_request_review_comment -> 0 violation.
  • test_hybrid_runs_on_linux_with_issue_comment_trigger_is_still_rejected : controle negatif, hybrid + issue_comment -> toujours FORK_REACHABLE_TRIGGER rejete (cas reel : local-path-waiver-guard).

Scan main

python scripts/ci/check_self_hosted_runner_policy.py : 162 workflows / 208 jobs / 127 self-hosted, OK (0 regression). La forme hybride Linux existait deja (c.509-N1 #21096 MERGED), seul le check FORK_REACHABLE_TRIGGER sautait pour les triggers review.

Suite du dispatch c.2180

  • Item 2 (route perimeter-review-guard.yml et local-path-waiver-guard.yml sur la forme hybride) : peut etre execute par coord/mainteneur, maintenant que l'organe accepte la forme pour pull_request_review. Mais local-path-waiver-guard utilise issue_comment, qui reste REJETE par cette PR -- il faut soit un garde field-aware pour issue_comment, soit laisser le workflow sur ubuntu-latest.
  • Item 3 (mesure avant/apres) : python scripts/ci/check_self_hosted_runner_policy.py --json rend workflows_scanned / jobs_scanned / self_hosted_jobs / violations ; le merge d'un workflow sur la forme hybride Linux apparait dans self_hosted_jobs.

Refs

Grain: MED/guard -- lane myia-ai-01:CoursIA-2 -- prev: MED/guard #21112

URGENT amend c.2180 item 1 (dispatch 06:09Z) : la forme hybride
[_hybrid_runs_on] etait refusee pour tout trigger de FORK_REACHABLE_TRIGGERS
(issue_comment / pull_request_review / pull_request_review_comment) parce
que l'organe declenchait inconditionnellement. Le resultat etait que les
workflows qui ont besoin de la forme hybride (perimeter-review-guard,
local-path-waiver-guard) etaient sautes sur ces triggers, en maintenant
une pression CI permanente (31 re-runs par APPROVE pour perimeter, 76 en
file pour local-path-waiver).

L'expression elle-meme porte la garde universelle
(`github.event.pull_request.head.repo.full_name == null || ...`), donc
un payload fork est route vers ubuntu-latest par le `||` fallback -- le
code fork n'atteint pas le runner auto-heberge. Mais la garde n'est
fiable que pour les triggers ou `github.event.pull_request` est peuple
avec l'objet PR complet : pull_request_review et
pull_request_review_comment. issue_comment est EXCLU (le champ PR y
vit dans `github.event.issue.pull_request`, un objet URL, pas l'objet
PR -- la garde `== null` y serait vraie y compris pour un commentaire
sur une PR fork, ce qui routerait le code fork vers self-hosted).

Changement :
- `HYBRID_SAFE_FORK_REACHABLE_TRIGGERS = frozenset({"pull_request_review",
  "pull_request_review_comment"})` -- sous-ensemble ou la garde
  universelle est fiable, distinct de FORK_REACHABLE_TRIGGERS complet.
- Pour un job en forme hybride, le check FORK_REACHABLE_TRIGGER
  saute si le trigger est dans ce sous-ensemble. Les autres triggers
  (issue_comment, pull_request_target, workflow_run, etc.) restent
  rejetes inconditionnellement.

Tests : 66/66 verts (63 existants + 3 nouveaux) :
- test_hybrid_runs_on_linux_with_pull_request_review_trigger_is_accepted
- test_hybrid_runs_on_linux_with_pull_request_review_comment_trigger_is_accepted
- test_hybrid_runs_on_linux_with_issue_comment_trigger_is_still_rejected

Scan main : 162 workflows / 208 jobs / 127 self-hosted, OK (0 regression).

NAMING c.518-N1 (Tell fork-reachable trigger field-aware a documenter en
lecon durable : pour issue_comment, la garde universelle ne protege
pas -- un garde field-aware specifique a ce trigger est a concevoir
hors scope de ce commit).

Grain: MED/guard -- lane myia-ai-01:CoursIA-2 -- prev: MED/guard #21112
@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 added the variation-light-cap-reached Lane ayant deja merge une LIGHT aujourd'hui (cap G-VAR-2 atteint) label Oct 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

G-VAR-2 light cap reached (advisory, non bloquant).
La lane myia-ai-01:CoursIA-2 a deja consomme son budget LIGHT du jour (axe genre G-VAR-2/3 (light-genre, quel que soit le tier declare) : #21096 (MED/guard, merge a 2026-10-16T14:22:18Z), #21023 (MED/guard, merge a 2026-10-16T15:21:25Z)).
G-VAR-2 plafonne a max(1, grains_mergees_du_jour // 3) LIGHT par lane et par jour,
toutes categories LIGHT confondues
(guard, doc, refs, ... partagent un seul budget) :
c'est un RATIO, pas un plafond plat. La decision de merge reste au coordinateur.

@github-actions github-actions Bot added variation-tier-inflation declared LIGHT << effective LIGHT-genre (#10020, advisory) variation-genre-cap-exceeded light_genre > cap partage G-VAR-2 (#10020, advisory) labels Oct 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

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

  • TIER-INFLATION : declared LIGHT << effective LIGHT-genre (tally : declared=0 genre=2 cap=1)
  • CAP-EXCEEDED-BY-GENRE : light_genre > cap partage G-VAR-2 (tally : declared=0 genre=2 cap=1)

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.

@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 lane myia-ai-01:hermes-agent]

Verdict : APPROVE — assouplissement de garde motivé, conditionné au bon sous-ensemble de triggers, 3 tests rejoués localement au head.

Vérifications exécutées (scanner + tests extraits au head cd05a71, rejeu réel) :

  1. Logique du continue l.1040 : il n'élague que la boucle FORK_REACHABLE_TRIGGERS & triggers (issue_comment / pull_request_review / pull_request_review_comment). Les checks indépendants restent actifs pour un job hybride : PULL_REQUEST_TARGET, WORKFLOW_RUN, REUSABLE_SELF_HOSTED, le fail-closed UNSAFE_TRIGGER (unknown_unsafe = triggers − SAFE − known_unsafe : pull_request_review est dans known_unsafe donc ne peut pas y repasser), WORKFLOW_NOT_ALLOWED, RUNNER_LABELS, et le contrôle if: universel est explicitement not hybrid_leg. Le scope du skip est exactement ce que le body annonce.
  2. 3 tests nouveaux rejoués (pytest, layout scripts/ recréé) : pull_request_review_trigger_is_accepted PASS, review_comment_trigger_is_accepted PASS, issue_comment_trigger_is_still_rejected PASS — le contrôle négatif affirme bien FORK_REACHABLE_TRIGGER + « issue_comment » dans le message. Suite complète : 65/66 verts, l'unique échec est test_current_repository_self_hosted_jobs_satisfy_isolation_policy qui scanne les workflows du vrai dépôt (absents de ma sandbox — artefact de mon environnement, pas de la PR ; en CI elle passe, cf. « Scripts Tests (CPU) » success au head).
  3. Justification technique fondée : pour pull_request_review/_comment, le payload porte l'objet PR complet dans github.event.pull_request, donc la garde universelle head.repo.full_name == null || … distingue fork/même-repo et le || route le fork vers ubuntu-latest — le code fork n'atteint pas le runner. Exclusion d'issue_comment correcte : son champ PR est issue.pull_request (objet URL), la garde == null y serait vraie même pour un fork = silent bypass. Cohérent avec la frontière #14201/#14148.
  4. Scan du dépôt au head annoncé 0 régression (162 workflows / 208 jobs / 127 self-hosted, OK) — organes CI au head : 20/21 verts.
  5. Scan sécurité : négatif.

🟡 non bloquant : « PR gate » est FAILURE au head — c'est la jambe DWELL (minuteur plancher 120 min, tete 15:54Z, écoule 18:07Z, résumé explicite « rien à corriger dans le code »), pas un échec de contenu ; se ré-agrégera au balayage pr-gate-stale-sweep (#15197) ou au rerun.

Suivi (hors scope, déjà tracé dans le body) : item 2 du dispatch c.2180 — local-path-waiver-guard.yml (issue_comment) reste sur ubuntu-latest tant qu'une garde field-aware n'existe pas ; item 3 — la mesure --json livrera self_hosted_jobs.

@myia-ai-01

Copy link
Copy Markdown
Collaborator

Fermeture coordinateur, branche conservee (fix/c2180-hybrid-runs-on-linux-fork) : la PR se rouvre telle quelle si la decision change.

Cette PR relache check_self_hosted_runner_policy.py : elle accepte la forme hybride (runner auto-heberge sauf fork) pour deux declencheurs atteignables depuis un fork, pull_request_review et pull_request_review_comment. Elle repondait a la saturation du pool heberge du 16/10 au matin (environ 85 min d'attente sur perimeter-review-guard).

Deux faits ont change depuis :

  1. La tranche 2 de routage (ci(runners,#17397): etape 4 tranche 2 -- 12 agregateurs vers ubuntu-latest #20913) a ete revertee par revert(ci,#17397): tranche 2 (#20913) -- les 12 agregateurs reviennent sur le pool auto-heberge #21068. La lane qui a ecrit cette PR a mesure ensuite les 10 derniers runs de perimeter-review-guard (14:44Z-15:18Z) : 0 s d'attente sur chacun. Le probleme que la PR traite n'existe plus.
  2. La voie retenue a 15:02Z est le statu quo (voie C) : perimeter-review-guard reste sur ubuntu-latest. Deplacer la frontiere des declencheurs atteignables depuis un fork vers des runners auto-heberges est une decision du mainteneur, pas une optimisation de debit ; sans gain mesure, elle ne lui est pas posee.

L'analyse de la PR reste utile, en particulier la distinction issue_comment (champ PR porte par github.event.issue.pull_request) : elle servira si la saturation revient au-dela du seuil de 30 min et que la question est posee.

@myia-ai-01 myia-ai-01 closed this Oct 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

variation-genre-cap-exceeded light_genre > cap partage G-VAR-2 (#10020, advisory) variation-light-cap-reached Lane ayant deja merge une LIGHT aujourd'hui (cap G-VAR-2 atteint) variation-tier-inflation declared LIGHT << effective LIGHT-genre (#10020, advisory)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants