REVIEW.md : principes à la place des listes, conditions du script de calibration (suivis de la seconde revue de #460) #462

New issue

Open

opened 2026-10-18 13:18:31 +00:00 by maximus · 1 comment

maximus commented

2026-10-18 13:18:31 +00:00

Owner

Suivis non bloquants de la seconde revue de la PR #460 (commentaire 24635), mergée le 2026-10-18 (24da72b). La revue recommandait de merger plutôt qu'un troisième tour, et de confier ces points au pilote #458 : chaque passe trouvait un membre de plus des mêmes listes. Lignes citées : REVIEW.md sur 24da72b.

1. Locks : liste blanche de la forme canonique (`REVIEW.md:36`)

Vérifier l'hôte laisse normale une montée de version dont le resolved désigne l'archive d'un autre paquet sur le même hôte (registry.npmjs.org/evil-foo/-/evil-foo-1.2.4.tgz sous la clé node_modules/foo). Les 252 resolved du lock npm ont la forme https://registry.npmjs.org/<nom de la clé>/-/<nom court>-<version>.tgz, et les 665 source de Cargo.lock désignent l'index crates.io. Proposé : « une source autre que l'index crates.io, un resolved hors de la forme canonique de sa clé et de sa version, ou un checksum ou un integrity changés sans nouvelle version ». Aucune des 51 PR de la calibration ne change de niveau.

2. Consentements : l'app en a deux (`REVIEW.md:29` et `:85`)

Celui du Feedback Hub vit côté front : openFeedback de src/components/settings/LogViewerCard.tsx:20-33 (localStorage, FeedbackConsentDialog.tsx), seul chemin vers FeedbackDialog.
Nommer aussi le site d'appel des cours, prices.fetchPrice : un nouvel appelant hors de PriceFetchControl contourne le consentement sans toucher aux fichiers nommés.
PriceFetchConsentToggle.tsx accorde et retire le consentement ; seul PriceFetchControl.tsx le vérifie.

3. Deux énumérations à remplacer par leur principe

Clé de signature (REVIEW.md:126) : tout ce qui s'exécute dans le job build-and-release atteint la clé, dont Node.js, installé par le script NodeSource sans archive épinglée (release.yml:19-25), et actions/checkout@v4, une étiquette mobile.
identifier (REVIEW.md:101) : changer l'identifier orpheline tout ce que l'app range sous app_config_dir() et app_data_dir(), dont profiles.json (liste des profils et pin_hash, profile_commands.rs:13-35) et tout le dossier auth/.
Même famille : un fichier que .claude/settings.json ou .mcp.json fait exécuter (script d'un hook, commande d'un serveur MCP) porte leur risque, mais reste normal sous .claude/**.

4. Le script de calibration approche les conditions : le dire

Six sondes de la revue s'écartent du texte, sans déplacer aucune des 51 PR :

un env: ajouté au niveau du workflow, ou au niveau du job avant runs-on:, sort normal ;
une étape run: ajoutée qui supprime ou édite un fichier du dépôt (rm, sed -i) sort normale ;
en sens inverse, une ligne de commentaire seule dans lib.rs ou src-tauri/Cargo.toml sort normale, alors que le texte ne les exempte pas.

À faire :

la docstring de scripts/review-levels.py dit que le script n'approche les conditions qu'en partie, et que le niveau d'une PR se lit dans REVIEW.md, jamais dans sa sortie (ni #457 ni #458 ne l'utilisent sur une PR ouverte) ;
trancher les commentaires : les exempter dans le texte (« hors commentaires ») ou les faire compter par le script ;
--check ne compare que les motifs, pas les conditions : l'écrire dans REVIEW.md, les conditions restant à la revue de niveau haut.

Critères d'acceptation

Points 1 à 4 traités dans REVIEW.md et scripts/review-levels.py, ou écartés avec une raison écrite ici.
python3 scripts/review-levels.py --check passe, et la calibration rejouée garde 3/28/8 sur les 39 PR, 6/33/12 sur les 51, ou l'écart est justifié.

Suivis non bloquants de la seconde revue de la PR #460 (commentaire 24635), mergée le 2026-10-18 (`24da72b`). La revue recommandait de merger plutôt qu'un troisième tour, et de confier ces points au pilote #458 : chaque passe trouvait un membre de plus des mêmes listes. Lignes citées : `REVIEW.md` sur `24da72b`. ## 1. Locks : liste blanche de la forme canonique (`REVIEW.md:36`) Vérifier l'hôte laisse normale une montée de version dont le `resolved` désigne l'archive d'un autre paquet sur le même hôte (`registry.npmjs.org/evil-foo/-/evil-foo-1.2.4.tgz` sous la clé `node_modules/foo`). Les 252 `resolved` du lock npm ont la forme `https://registry.npmjs.org/<nom de la clé>/-/<nom court>-<version>.tgz`, et les 665 `source` de `Cargo.lock` désignent l'index crates.io. Proposé : « une `source` autre que l'index crates.io, un `resolved` hors de la forme canonique de sa clé et de sa version, ou un `checksum` ou un `integrity` changés sans nouvelle version ». Aucune des 51 PR de la calibration ne change de niveau. ## 2. Consentements : l'app en a deux (`REVIEW.md:29` et `:85`) - Celui du Feedback Hub vit côté front : `openFeedback` de `src/components/settings/LogViewerCard.tsx:20-33` (`localStorage`, `FeedbackConsentDialog.tsx`), seul chemin vers `FeedbackDialog`. - Nommer aussi le site d'appel des cours, `prices.fetchPrice` : un nouvel appelant hors de `PriceFetchControl` contourne le consentement sans toucher aux fichiers nommés. - `PriceFetchConsentToggle.tsx` accorde et retire le consentement ; seul `PriceFetchControl.tsx` le vérifie. ## 3. Deux énumérations à remplacer par leur principe - Clé de signature (`REVIEW.md:126`) : tout ce qui s'exécute dans le job `build-and-release` atteint la clé, dont Node.js, installé par le script NodeSource sans archive épinglée (`release.yml:19-25`), et `actions/checkout@v4`, une étiquette mobile. - `identifier` (`REVIEW.md:101`) : changer l'`identifier` orpheline tout ce que l'app range sous `app_config_dir()` et `app_data_dir()`, dont `profiles.json` (liste des profils et `pin_hash`, `profile_commands.rs:13-35`) et tout le dossier `auth/`. - Même famille : un fichier que `.claude/settings.json` ou `.mcp.json` fait exécuter (script d'un hook, commande d'un serveur MCP) porte leur risque, mais reste normal sous `.claude/**`. ## 4. Le script de calibration approche les conditions : le dire Six sondes de la revue s'écartent du texte, sans déplacer aucune des 51 PR : - un `env:` ajouté au niveau du workflow, ou au niveau du job avant `runs-on:`, sort normal ; - une étape `run:` ajoutée qui supprime ou édite un fichier du dépôt (`rm`, `sed -i`) sort normale ; - en sens inverse, une ligne de commentaire seule dans `lib.rs` ou `src-tauri/Cargo.toml` sort normale, alors que le texte ne les exempte pas. À faire : - la docstring de `scripts/review-levels.py` dit que le script n'approche les conditions qu'en partie, et que le niveau d'une PR se lit dans `REVIEW.md`, jamais dans sa sortie (ni #457 ni #458 ne l'utilisent sur une PR ouverte) ; - trancher les commentaires : les exempter dans le texte (« hors commentaires ») ou les faire compter par le script ; - `--check` ne compare que les motifs, pas les conditions : l'écrire dans `REVIEW.md`, les conditions restant à la revue de niveau haut. ## Critères d'acceptation - [ ] Points 1 à 4 traités dans `REVIEW.md` et `scripts/review-levels.py`, ou écartés avec une raison écrite ici. - [ ] `python3 scripts/review-levels.py --check` passe, et la calibration rejouée garde 3/28/8 sur les 39 PR, 6/33/12 sur les 51, ou l'écart est justifié.

maximus added the

status:triage

type:infra

source:human

labels 2026-10-18 13:18:32 +00:00

maximus referenced this issue

2026-10-18 13:18:32 +00:00

Brancher les skills de revue sur REVIEW.md et les niveaux de risque #457

maximus referenced this issue

2026-10-18 13:33:23 +00:00

Brancher les skills de revue sur REVIEW.md et les niveaux de risque #457

maximus commented

2026-10-18 18:19:23 +00:00

Author

Owner

Ajout au périmètre, jugement de Max du 2026-10-18 dans le pilote #458 (tableau, commentaire 24819) : la PR #450 est sortie haute par le seul motif .claude/skills/release/**, alors qu'elle n'ajoute qu'une instruction à l'étape 0 du skill. Max juge ce niveau trop haut.

À faire dans la même modification de « Niveaux » : restreindre la ligne Release du tableau Haut à .claude/skills/release/scripts/** (verify-release.sh, le code qui contrôle la release publiée). SKILL.md reste au niveau normal, et ne descend jamais au niveau bas (.claude/**). Mettre scripts/review-levels.py à jour dans le même diff, puis rejouer la calibration : #414, #416 et #418 restent hautes par release.yml, aucune PR des 51 ne change de niveau.

Ajout au périmètre, jugement de Max du 2026-10-18 dans le pilote #458 (tableau, commentaire 24819) : la PR #450 est sortie haute par le seul motif `.claude/skills/release/**`, alors qu'elle n'ajoute qu'une instruction à l'étape 0 du skill. **Max juge ce niveau trop haut.** À faire dans la même modification de « Niveaux » : restreindre la ligne Release du tableau Haut à `.claude/skills/release/scripts/**` (`verify-release.sh`, le code qui contrôle la release publiée). `SKILL.md` reste au niveau normal, et ne descend jamais au niveau bas (`.claude/**`). Mettre `scripts/review-levels.py` à jour dans le même diff, puis rejouer la calibration : #414, #416 et #418 restent hautes par `release.yml`, aucune PR des 51 ne change de niveau.

maximus referenced this issue

2026-10-18 18:19:23 +00:00

Pilote pr-review par niveaux : mesurer 20 PR et faire le bilan #458