maximus/Simpl-Resultat
1
0
Fork 0
Code Issues 19 Pull requests 8 Projects Releases 34 Packages Wiki Activity Actions

docs(review): REVIEW.md, calibrated risk levels and review rules per zone (#456) #460

Merged
maximus merged 2 commits from issue-456-review-md into main 2026-10-18 13:17:49 +00:00
Conversation 3 Commits 2 Files changed 3 +585
maximus commented 2026-10-18 12:22:25 +00:00
Owner
Copy link

Fixes #456

Ajoute REVIEW.md à la racine, premier travail du pilote « évolution de /pr-review » (document de départ : maximus/agent-defenseurs#190, section 4), et une ligne de renvoi dans CLAUDE.md. Aucun fichier de code, de build ou de CI n'est touché.

Le fichier

  • Niveaux : le niveau d'une PR est le plus haut de ses fichiers, jamais celui de son auteur. Le tableau Haut compte 20 lignes, chacune avec sa condition et sa zone : licence et jetons, migrations et bases de profils, release et code exécuté avec la clé de signature, dépendances qui changent la confiance (paquet ajouté ou retiré, origine changée, manifeste modifié), CI qui pourrait passer au vert à tort, et une règle qui échoue fermée pour tout fichier ajouté, renommé ou supprimé à la racine ou dans src-tauri/. Bas : la doc et STATE.md seuls. Le reste est normal.
  • Ce qui compte par zone : neuf zones (vie privée ; précision financière et import ; migrations, profils et données ; jetons, licence et édition ; dépendances ; release et mise à jour ; CI et gardes ; i18n et documentation ; règles de revue). Chaque règle cite une section du CLAUDE.md ou un ADR, plus le fichier du dépôt quand le fait n'existe que là (tauri.conf.json, les workflows, .claude/rules/, le skill /release).
  • Commandes de vérification : celles des workflows et leurs déclencheurs, puis ce qui ne se rejoue qu'en local (gardes de check-rust, Windows par cargo-xwin, sonde de npm test).
  • Pièges de revue : onze, dont les trois du skill global pr-review corrigés pour ce dépôt (sa section « CI coverage gap » est fausse ici depuis #232).

CLAUDE.md, section « Documentation technique », renvoie à REVIEW.md et demande qu'un changement qui rend faux un fait cité le mette à jour dans la même PR.

Calibration (C1)

Les deux ensembles. Les « 39 PR de référence » du document de départ sont le résultat de GET /pulls?state=closed&limit=40&sort=recentupdate (39 mergées, plus #374 non mergée) : 39 des 51 PR mergées entre #344 et #452, choisies par date de dernière mise à jour. Les motifs d'essai rejoués sur cet ensemble redonnent exactement 3/19/17, ce qui l'identifie. Ce tri n'est pas stable, d'où la liste figée :

#344 #375 #377 #378 #379 #380 #381 #382 #383 #386 #387 #388 #394 #402 #403 #407 #412 #413 #414 #415 #416 #417 #418 #420 #428 #433 #434 #435 #436 #438 #439 #440 #441 #442 #443 #444 #445 #448 #452

Il laisse de côté #349, #359, #362, #364, #366, #368, #385, #404, #405, #406, #410 et #411, dont quatre PR de licence de niveau haut. La plage complète est donc consignée aussi.

Ensemble Bas Normal Haut
39, motifs d'essai 3 19 17 (43,6 %)
39, motifs de REVIEW.md 3 28 8 (20,5 %)
51, motifs d'essai 6 22 23 (45,1 %)
51, motifs de REVIEW.md 6 33 12 (23,5 %)

Niveau haut, les 8 des 39 :

  • licence : #386, #387, #388 (license_commands.rs, licenseService.ts) ;
  • migrations et bases de profils : #394 (fn migrations() de lib.rs, profile_db.rs, consolidated_schema.sql, profile_commands.rs), #436 (profile_db.rs, profile_commands.rs) ;
  • release : #414 (installation de Node dans release.yml), #416 et #418 (release.yml, scripts/build-latest-json.sh, garde scripts/check-latest-json.jq).

Sur les 51, quatre de plus, toutes de licence : #359, #364, #366, #368.

Sorties du niveau haut :

PR Ce qui la mettait en haut à l'essai Règle de REVIEW.md
#344, #448 (et #349 sur les 51) src-tauri/Cargo.lock montée de version d'un paquet déjà présent : normal (décision de Max)
#417 package-lock.json idem : 40 lignes de version, aucun paquet ajouté ni retiré, aucune origine changée
#375 .forgejo/workflows/** cache, déclencheurs ajoutés, commentaires : rien qui fasse passer au vert à tort
#415 .forgejo/workflows/** garde ajoutée : étape nouvelle qui ne fait que lire
#380 fs_commands.rs, une entrée de generate_handler! dans lib.rs fs_commands.rs normal, entrées de generate_handler! exemptées
#440 fs_commands.rs normal
#402 dataExportService.ts (+8/−1, une notification de données) normal
#433 db.ts normal
#362 (sur les 51) src/services/entitlements.test.ts la matrice d'édition ne verrouille que l'interface (ADR 0017, § 2) : normal

Écart à la cible. 8 sur 39 donne 20,5 %, au-dessus des 20 % (7,8 PR). Descendre à 7 obligerait à rétrograder une zone réellement haute : chacune des 8 touche le code de licence, le chemin des bases de profils et des migrations, ou release.yml, qui détient la clé de signature. Sur les 51, la part monte à 23,5 % parce que la plage contient la vague de licence du go-live (#359 à #368) : la part de niveau haut suit la phase du projet.

Recoupement avec les demandes de changements. Le document relève 4 commentaires REQUEST_CHANGES, sur #415, #416 et #428. Avec REVIEW.md, #416 est haute ; #415 (garde ajoutée) et #428 sont normales, et c'est la revue de profondeur normale, celle d'aujourd'hui, qui a trouvé leurs défauts.

Variante écartée, pour mémoire : toute montée de version d'une dépendance au niveau haut donnait 11/39 (28 %) et 16/51 (31 %).

Niveau de chaque PR (51)
PR Réf. Essai REVIEW.md Fichiers qui fixent le niveau haut
#344 oui haut normal
#349 haut normal
#359 haut haut src-tauri/src/commands/license_commands.rs, src-tauri/src/commands/license_commands.rs (appel réseau), src/services/licenseService.ts
#362 haut normal
#364 haut haut src-tauri/src/commands/entitlements.rs, src-tauri/src/commands/license_commands.rs, src/services/licenseService.ts
#366 haut haut src-tauri/src/commands/license_commands.rs, src/services/licenseService.ts
#368 haut haut src-tauri/src/commands/balance_commands.rs, src-tauri/src/commands/license_commands.rs, src/services/licenseService.ts
#375 oui haut normal
#377 oui normal normal
#378 oui normal normal
#379 oui normal normal
#380 oui haut normal
#381 oui normal normal
#382 oui normal normal
#383 oui normal normal
#385 bas bas
#386 oui haut haut src-tauri/src/commands/license_commands.rs
#387 oui haut haut src-tauri/src/commands/license_commands.rs, src/services/licenseService.ts
#388 oui haut haut src-tauri/src/commands/balance_commands.rs, src-tauri/src/commands/license_commands.rs, src/services/licenseService.ts
#394 oui haut haut src-tauri/Cargo.toml, src-tauri/src/commands/profile_commands.rs, src-tauri/src/database/consolidated_schema.sql et 10 autres
#402 oui haut normal
#403 oui normal normal
#404 normal normal
#405 normal normal
#406 normal normal
#407 oui normal normal
#410 bas bas
#411 bas bas
#412 oui bas bas
#413 oui bas bas
#414 oui haut haut .claude/skills/release/SKILL.md, .forgejo/workflows/audit.yml, .forgejo/workflows/release.yml
#415 oui haut normal
#416 oui haut haut .claude/skills/release/SKILL.md, .claude/skills/release/scripts/verify-release.sh, .forgejo/workflows/release.yml et 2 autres
#417 oui haut normal
#418 oui haut haut .claude/skills/release/SKILL.md, .claude/skills/release/scripts/verify-release.sh, .forgejo/workflows/release.yml et 3 autres
#420 oui bas bas
#428 oui normal normal
#433 oui haut normal
#434 oui normal normal
#435 oui normal normal
#436 oui haut haut src-tauri/src/commands/profile_commands.rs, src-tauri/src/database/profile_db.rs
#438 oui normal normal
#439 oui normal normal
#440 oui haut normal
#441 oui normal normal
#442 oui normal normal
#443 oui normal normal
#444 oui normal normal
#445 oui normal normal
#448 oui haut normal
#452 oui normal normal
Script de calibration

Versionné depuis 24da72b : scripts/review-levels.py, en lecture seule sur l'API Forgejo (jeton $FORGEJO_TOKEN, sinon ~/.forgejo-token). python3 scripts/review-levels.py [premier dernier] rejoue les niveaux, en environ 3 minutes pour les 51 PR ; python3 scripts/review-levels.py --check vérifie, sans réseau, que ses 37 motifs sont ceux du tableau Haut (sortie 1 sinon, prouvé en retirant .npmrc d'une copie du tableau). Dernier passage, à neuf, sur 24da72b : 39 → essai 3/19/17, REVIEW.md 3/28/8 ; 51 → essai 6/22/23, REVIEW.md 6/33/12, niveaux identiques PR par PR au passage de fa1abb1.

Écarts au texte de l'issue

  • Montée de version dans un lock : niveau normal. L'issue proposait « hauts seulement si une dépendance change ». Max a tranché le 2026-10-17 : une montée de version d'un paquet déjà présent reste normale ; le niveau haut vaut pour un paquet ajouté ou retiré, une origine changée ou un manifeste modifié. Décision consignée sur #456.
  • Vie privée : « aucun nouvel appel réseau » devient « un appel ajouté se justifie » (rien de financier, aucun identifiant brut, derrière un consentement ou l'édition). Interprétation de ma part, pas une décision de Max : #368 a ajouté un appel accepté (contrôle de révocation), et une interdiction absolue contredirait le dépôt.
  • Ensemble de référence : les 39 sont figés par numéro, et la plage complète de 51 est consignée à côté.
  • Lecture sur main : REVIEW.md demande à #457 de le lire sur main, pas sur la base de la PR, pour qu'un maillon de pile ne soit pas jugé par une version non relue.
  • Zones ajoutées à la liste de l'issue : « CI et gardes » et « Règles de revue ».

Contrôle du plan

Trois passages refusés, la limite du skill /fix-issue, chacun sur des points réels :

  1. CI affaiblie ou code exécuté au build avec la clé de signature restés au niveau normal ; péremption prévisible avec la milestone prep-2026-10-16-lot-v018 (#449, #453).
  2. Chemins voisins qui contournaient les protections : tauri.*.conf.json propres à une plateforme, src-tauri/.cargo/, .npmrc, workflow ajouté qui lit les secrets.
  3. Une règle de zone sans source CLAUDE.md ou ADR ; l'action de cache de #375 écrit $GITHUB_ENV (règle CI précisée) ; fichiers de configuration voisins (vite.config.js, vitest.config.*, postcss, Cargo.toml à la racine), d'où la règle qui échoue fermée.

Un constat du premier passage a été écarté : #314 n'a pas montré un cron d'audit mort, sa prémisse a été infirmée (STATE.md). Après le troisième passage, les derniers points appliqués, Max a choisi de continuer (commit annoté « plan check bypassed »). Les correctifs ne changent le niveau d'aucune des 51 PR (script relancé à neuf contre l'API).

Vérifications

  • Diff : REVIEW.md (nouveau, 190 lignes), CLAUDE.md (+2 lignes) et scripts/review-levels.py (outil de calibration, lancé à la main, lu par aucun build ni test). Aucun code applicatif.
  • Les 29 liens de REVIEW.md mènent à des fichiers du dépôt, et chaque symbole cité (migration_checksums_match_the_released_ones, prepare_profile_db, infer_floor, toSqlFlag, notifyDataChanged, isEntitled, SR_DEV_EDITION, fetch_price, withTransaction…) existe (git grep). Les chemins non résolus sont des motifs pour des fichiers absents (.npmrc, **/rust-toolchain*, src-tauri/Tauri*.toml), des extensions, un schéma d'URL ou un artefact de release (latest.json).
  • Lignes ajoutées scannées (courriel, préfixe de clé de licence, JWT, clés Stripe, PEM) : 0 occurrence.
  • Suites : sur fa1abb1, aucune CI (deux *.md de la racine, exclus par check-frontend et hors des paths de check-rust). Depuis 24da72b, scripts/ déclenche check-frontend (npm ci, build, npm test), qui ne lit pas le script ; check-rust ne tourne pas.

Après la première revue (commentaire 24517)

Revue APPROVE avec 7 suggestions ; Max a choisi de les appliquer puis de demander une seconde revue. Commit 24da72b, pas encore relu :

  • points 1, 3, 4, 5 et 7 appliqués dans REVIEW.md : exception des CSV compilés dans le piège « CI d'une pile » ; niveau haut pour .claude/settings.json et .mcp.json, un déclencheur pull_request_target, une étape d'installation qui change la toolchain ou le PATH, une origine de paquet dont l'hôte ou le schéma change même avec une nouvelle version, et un diff qui retire ou contourne un consentement ; cargo-xwin et la toolchain parmi ce qui s'exécute avec la clé ; EXPECTED dans la garde windows-sys ; identifier précisé (le service du trousseau est une constante) ;
  • point 2 : le script est versionné et corrigé (pages de 50 lues jusqu'à une page vide, commentaires ignorés par les contrôles de workflow, option --check) ;
  • point 3, db.ts et dataExportService.ts : décision de Max du 2026-10-18, ils restent normaux, et la zone « Migrations, profils et données » nomme le verrou, le changement de profil et la restauration. Contrairement à ce qu'écrivait la revue, une condition sur le verrou aurait fait passer #433 en haut (9/39) ;
  • point 6 (#450 haute par le seul skill /release) : laissé au pilote #458, qui notera si Max juge ce niveau juste ;
  • hors PR et point 1 côté CI : issue de suivi #461 (paths de check-rust, cargo-xwin et toolchain à épingler, commentaire périmé de release.yml:104).

Ces changements ne déplacent le niveau d'aucune des 51 PR. Un premier passage du script les montrait à tort : #375 sortait haute, à cause de commentaires qui contenaient « toolchain » et « PATH ». Les contrôles de workflow ignorent désormais les commentaires.

Suite

  • #457 branche les skills globaux sur ce fichier, hors git. Ensuite, /pr-review 459 sera la première PR mesurée du pilote #458, de niveau haut (src-tauri/Cargo.toml, src-tauri/Cargo.lock, étape de garde de check-rust.yml, skill /release).
  • Commentaires posés sur #449 et #453 : leur PR met REVIEW.md à jour (--locked dans le tableau des commandes ; fin du piège « Faux t » sur les clés présentes dans une seule langue).
  • #461 : suivis CI et release de la première revue.

🤖 Generated with Claude Code

Fixes #456 Ajoute `REVIEW.md` à la racine, premier travail du pilote « évolution de `/pr-review` » (document de départ : maximus/agent-defenseurs#190, section 4), et une ligne de renvoi dans `CLAUDE.md`. Aucun fichier de code, de build ou de CI n'est touché. ## Le fichier - **Niveaux** : le niveau d'une PR est le plus haut de ses fichiers, jamais celui de son auteur. Le tableau Haut compte 20 lignes, chacune avec sa condition et sa zone : licence et jetons, migrations et bases de profils, release et code exécuté avec la clé de signature, dépendances qui changent la confiance (paquet ajouté ou retiré, origine changée, manifeste modifié), CI qui pourrait passer au vert à tort, et une règle qui échoue fermée pour tout fichier ajouté, renommé ou supprimé à la racine ou dans `src-tauri/`. Bas : la doc et `STATE.md` seuls. Le reste est normal. - **Ce qui compte par zone** : neuf zones (vie privée ; précision financière et import ; migrations, profils et données ; jetons, licence et édition ; dépendances ; release et mise à jour ; CI et gardes ; i18n et documentation ; règles de revue). Chaque règle cite une section du `CLAUDE.md` ou un ADR, plus le fichier du dépôt quand le fait n'existe que là (`tauri.conf.json`, les workflows, `.claude/rules/`, le skill `/release`). - **Commandes de vérification** : celles des workflows et leurs déclencheurs, puis ce qui ne se rejoue qu'en local (gardes de `check-rust`, Windows par cargo-xwin, sonde de `npm test`). - **Pièges de revue** : onze, dont les trois du skill global `pr-review` corrigés pour ce dépôt (sa section « CI coverage gap » est fausse ici depuis #232). `CLAUDE.md`, section « Documentation technique », renvoie à `REVIEW.md` et demande qu'un changement qui rend faux un fait cité le mette à jour dans la même PR. ## Calibration (C1) **Les deux ensembles.** Les « 39 PR de référence » du document de départ sont le résultat de `GET /pulls?state=closed&limit=40&sort=recentupdate` (39 mergées, plus #374 non mergée) : 39 des 51 PR mergées entre #344 et #452, choisies par date de dernière mise à jour. Les motifs d'essai rejoués sur cet ensemble redonnent exactement 3/19/17, ce qui l'identifie. Ce tri n'est pas stable, d'où la liste figée : #344 #375 #377 #378 #379 #380 #381 #382 #383 #386 #387 #388 #394 #402 #403 #407 #412 #413 #414 #415 #416 #417 #418 #420 #428 #433 #434 #435 #436 #438 #439 #440 #441 #442 #443 #444 #445 #448 #452 Il laisse de côté #349, #359, #362, #364, #366, #368, #385, #404, #405, #406, #410 et #411, dont quatre PR de licence de niveau haut. La plage complète est donc consignée aussi. | Ensemble | Bas | Normal | Haut | |---|---|---|---| | 39, motifs d'essai | 3 | 19 | 17 (43,6 %) | | 39, motifs de `REVIEW.md` | 3 | 28 | 8 (20,5 %) | | 51, motifs d'essai | 6 | 22 | 23 (45,1 %) | | 51, motifs de `REVIEW.md` | 6 | 33 | 12 (23,5 %) | **Niveau haut, les 8 des 39 :** - licence : #386, #387, #388 (`license_commands.rs`, `licenseService.ts`) ; - migrations et bases de profils : #394 (`fn migrations()` de `lib.rs`, `profile_db.rs`, `consolidated_schema.sql`, `profile_commands.rs`), #436 (`profile_db.rs`, `profile_commands.rs`) ; - release : #414 (installation de Node dans `release.yml`), #416 et #418 (`release.yml`, `scripts/build-latest-json.sh`, garde `scripts/check-latest-json.jq`). Sur les 51, quatre de plus, toutes de licence : #359, #364, #366, #368. **Sorties du niveau haut :** | PR | Ce qui la mettait en haut à l'essai | Règle de `REVIEW.md` | |---|---|---| | #344, #448 (et #349 sur les 51) | `src-tauri/Cargo.lock` | montée de version d'un paquet déjà présent : normal (décision de Max) | | #417 | `package-lock.json` | idem : 40 lignes de version, aucun paquet ajouté ni retiré, aucune origine changée | | #375 | `.forgejo/workflows/**` | cache, déclencheurs ajoutés, commentaires : rien qui fasse passer au vert à tort | | #415 | `.forgejo/workflows/**` | garde ajoutée : étape nouvelle qui ne fait que lire | | #380 | `fs_commands.rs`, une entrée de `generate_handler!` dans `lib.rs` | `fs_commands.rs` normal, entrées de `generate_handler!` exemptées | | #440 | `fs_commands.rs` | normal | | #402 | `dataExportService.ts` (+8/−1, une notification de données) | normal | | #433 | `db.ts` | normal | | #362 (sur les 51) | `src/services/entitlements.test.ts` | la matrice d'édition ne verrouille que l'interface (ADR 0017, § 2) : normal | **Écart à la cible.** 8 sur 39 donne 20,5 %, au-dessus des 20 % (7,8 PR). Descendre à 7 obligerait à rétrograder une zone réellement haute : chacune des 8 touche le code de licence, le chemin des bases de profils et des migrations, ou `release.yml`, qui détient la clé de signature. Sur les 51, la part monte à 23,5 % parce que la plage contient la vague de licence du go-live (#359 à #368) : la part de niveau haut suit la phase du projet. **Recoupement avec les demandes de changements.** Le document relève 4 commentaires `REQUEST_CHANGES`, sur #415, #416 et #428. Avec `REVIEW.md`, #416 est haute ; #415 (garde ajoutée) et #428 sont normales, et c'est la revue de profondeur normale, celle d'aujourd'hui, qui a trouvé leurs défauts. **Variante écartée, pour mémoire :** toute montée de version d'une dépendance au niveau haut donnait 11/39 (28 %) et 16/51 (31 %). <details><summary>Niveau de chaque PR (51)</summary> | PR | Réf. | Essai | `REVIEW.md` | Fichiers qui fixent le niveau haut | |---|---|---|---|---| | #344 | oui | haut | normal | | | #349 | | haut | normal | | | #359 | | haut | haut | `src-tauri/src/commands/license_commands.rs`, `src-tauri/src/commands/license_commands.rs (appel réseau)`, `src/services/licenseService.ts` | | #362 | | haut | normal | | | #364 | | haut | haut | `src-tauri/src/commands/entitlements.rs`, `src-tauri/src/commands/license_commands.rs`, `src/services/licenseService.ts` | | #366 | | haut | haut | `src-tauri/src/commands/license_commands.rs`, `src/services/licenseService.ts` | | #368 | | haut | haut | `src-tauri/src/commands/balance_commands.rs`, `src-tauri/src/commands/license_commands.rs`, `src/services/licenseService.ts` | | #375 | oui | haut | normal | | | #377 | oui | normal | normal | | | #378 | oui | normal | normal | | | #379 | oui | normal | normal | | | #380 | oui | haut | normal | | | #381 | oui | normal | normal | | | #382 | oui | normal | normal | | | #383 | oui | normal | normal | | | #385 | | bas | bas | | | #386 | oui | haut | haut | `src-tauri/src/commands/license_commands.rs` | | #387 | oui | haut | haut | `src-tauri/src/commands/license_commands.rs`, `src/services/licenseService.ts` | | #388 | oui | haut | haut | `src-tauri/src/commands/balance_commands.rs`, `src-tauri/src/commands/license_commands.rs`, `src/services/licenseService.ts` | | #394 | oui | haut | haut | `src-tauri/Cargo.toml`, `src-tauri/src/commands/profile_commands.rs`, `src-tauri/src/database/consolidated_schema.sql` et 10 autres | | #402 | oui | haut | normal | | | #403 | oui | normal | normal | | | #404 | | normal | normal | | | #405 | | normal | normal | | | #406 | | normal | normal | | | #407 | oui | normal | normal | | | #410 | | bas | bas | | | #411 | | bas | bas | | | #412 | oui | bas | bas | | | #413 | oui | bas | bas | | | #414 | oui | haut | haut | `.claude/skills/release/SKILL.md`, `.forgejo/workflows/audit.yml`, `.forgejo/workflows/release.yml` | | #415 | oui | haut | normal | | | #416 | oui | haut | haut | `.claude/skills/release/SKILL.md`, `.claude/skills/release/scripts/verify-release.sh`, `.forgejo/workflows/release.yml` et 2 autres | | #417 | oui | haut | normal | | | #418 | oui | haut | haut | `.claude/skills/release/SKILL.md`, `.claude/skills/release/scripts/verify-release.sh`, `.forgejo/workflows/release.yml` et 3 autres | | #420 | oui | bas | bas | | | #428 | oui | normal | normal | | | #433 | oui | haut | normal | | | #434 | oui | normal | normal | | | #435 | oui | normal | normal | | | #436 | oui | haut | haut | `src-tauri/src/commands/profile_commands.rs`, `src-tauri/src/database/profile_db.rs` | | #438 | oui | normal | normal | | | #439 | oui | normal | normal | | | #440 | oui | haut | normal | | | #441 | oui | normal | normal | | | #442 | oui | normal | normal | | | #443 | oui | normal | normal | | | #444 | oui | normal | normal | | | #445 | oui | normal | normal | | | #448 | oui | haut | normal | | | #452 | oui | normal | normal | | </details> <details><summary>Script de calibration</summary> Versionné depuis `24da72b` : `scripts/review-levels.py`, en lecture seule sur l'API Forgejo (jeton `$FORGEJO_TOKEN`, sinon `~/.forgejo-token`). `python3 scripts/review-levels.py [premier dernier]` rejoue les niveaux, en environ 3 minutes pour les 51 PR ; `python3 scripts/review-levels.py --check` vérifie, sans réseau, que ses 37 motifs sont ceux du tableau Haut (sortie 1 sinon, prouvé en retirant `.npmrc` d'une copie du tableau). Dernier passage, à neuf, sur `24da72b` : 39 → essai 3/19/17, `REVIEW.md` 3/28/8 ; 51 → essai 6/22/23, `REVIEW.md` 6/33/12, niveaux identiques PR par PR au passage de `fa1abb1`. </details> ## Écarts au texte de l'issue - **Montée de version dans un lock : niveau normal.** L'issue proposait « hauts seulement si une dépendance change ». Max a tranché le 2026-10-17 : une montée de version d'un paquet déjà présent reste normale ; le niveau haut vaut pour un paquet ajouté ou retiré, une origine changée ou un manifeste modifié. Décision consignée sur #456. - **Vie privée : « aucun nouvel appel réseau » devient « un appel ajouté se justifie »** (rien de financier, aucun identifiant brut, derrière un consentement ou l'édition). Interprétation de ma part, pas une décision de Max : #368 a ajouté un appel accepté (contrôle de révocation), et une interdiction absolue contredirait le dépôt. - **Ensemble de référence** : les 39 sont figés par numéro, et la plage complète de 51 est consignée à côté. - **Lecture sur `main`** : `REVIEW.md` demande à #457 de le lire sur `main`, pas sur la base de la PR, pour qu'un maillon de pile ne soit pas jugé par une version non relue. - **Zones ajoutées** à la liste de l'issue : « CI et gardes » et « Règles de revue ». ## Contrôle du plan Trois passages refusés, la limite du skill `/fix-issue`, chacun sur des points réels : 1. CI affaiblie ou code exécuté au build avec la clé de signature restés au niveau normal ; péremption prévisible avec la milestone `prep-2026-10-16-lot-v018` (#449, #453). 2. Chemins voisins qui contournaient les protections : `tauri.*.conf.json` propres à une plateforme, `src-tauri/.cargo/`, `.npmrc`, workflow ajouté qui lit les secrets. 3. Une règle de zone sans source `CLAUDE.md` ou ADR ; l'action de cache de #375 écrit `$GITHUB_ENV` (règle CI précisée) ; fichiers de configuration voisins (`vite.config.js`, `vitest.config.*`, `postcss`, `Cargo.toml` à la racine), d'où la règle qui échoue fermée. Un constat du premier passage a été écarté : #314 n'a pas montré un cron d'audit mort, sa prémisse a été infirmée (`STATE.md`). Après le troisième passage, les derniers points appliqués, Max a choisi de continuer (commit annoté « plan check bypassed »). Les correctifs ne changent le niveau d'aucune des 51 PR (script relancé à neuf contre l'API). ## Vérifications - Diff : `REVIEW.md` (nouveau, 190 lignes), `CLAUDE.md` (+2 lignes) et `scripts/review-levels.py` (outil de calibration, lancé à la main, lu par aucun build ni test). Aucun code applicatif. - Les 29 liens de `REVIEW.md` mènent à des fichiers du dépôt, et chaque symbole cité (`migration_checksums_match_the_released_ones`, `prepare_profile_db`, `infer_floor`, `toSqlFlag`, `notifyDataChanged`, `isEntitled`, `SR_DEV_EDITION`, `fetch_price`, `withTransaction`…) existe (`git grep`). Les chemins non résolus sont des motifs pour des fichiers absents (`.npmrc`, `**/rust-toolchain*`, `src-tauri/Tauri*.toml`), des extensions, un schéma d'URL ou un artefact de release (`latest.json`). - Lignes ajoutées scannées (courriel, préfixe de clé de licence, JWT, clés Stripe, PEM) : 0 occurrence. - Suites : sur `fa1abb1`, aucune CI (deux `*.md` de la racine, exclus par `check-frontend` et hors des `paths` de `check-rust`). Depuis `24da72b`, `scripts/` déclenche `check-frontend` (`npm ci`, build, `npm test`), qui ne lit pas le script ; `check-rust` ne tourne pas. ## Après la première revue (commentaire 24517) Revue APPROVE avec 7 suggestions ; Max a choisi de les appliquer puis de demander une seconde revue. Commit `24da72b`, pas encore relu : - points 1, 3, 4, 5 et 7 appliqués dans `REVIEW.md` : exception des CSV compilés dans le piège « CI d'une pile » ; niveau haut pour `.claude/settings.json` et `.mcp.json`, un déclencheur `pull_request_target`, une étape d'installation qui change la toolchain ou le `PATH`, une origine de paquet dont l'hôte ou le schéma change même avec une nouvelle version, et un diff qui retire ou contourne un consentement ; `cargo-xwin` et la toolchain parmi ce qui s'exécute avec la clé ; `EXPECTED` dans la garde `windows-sys` ; `identifier` précisé (le service du trousseau est une constante) ; - point 2 : le script est versionné et corrigé (pages de 50 lues jusqu'à une page vide, commentaires ignorés par les contrôles de workflow, option `--check`) ; - point 3, `db.ts` et `dataExportService.ts` : décision de Max du 2026-10-18, ils restent normaux, et la zone « Migrations, profils et données » nomme le verrou, le changement de profil et la restauration. Contrairement à ce qu'écrivait la revue, une condition sur le verrou aurait fait passer #433 en haut (9/39) ; - point 6 (#450 haute par le seul skill `/release`) : laissé au pilote #458, qui notera si Max juge ce niveau juste ; - hors PR et point 1 côté CI : issue de suivi #461 (`paths` de `check-rust`, `cargo-xwin` et toolchain à épingler, commentaire périmé de `release.yml:104`). Ces changements ne déplacent le niveau d'aucune des 51 PR. Un premier passage du script les montrait à tort : #375 sortait haute, à cause de commentaires qui contenaient « toolchain » et « PATH ». Les contrôles de workflow ignorent désormais les commentaires. ## Suite - #457 branche les skills globaux sur ce fichier, hors git. Ensuite, `/pr-review 459` sera la première PR mesurée du pilote #458, de niveau haut (`src-tauri/Cargo.toml`, `src-tauri/Cargo.lock`, étape de garde de `check-rust.yml`, skill `/release`). - Commentaires posés sur #449 et #453 : leur PR met `REVIEW.md` à jour (`--locked` dans le tableau des commandes ; fin du piège « Faux `t` » sur les clés présentes dans une seule langue). - #461 : suivis CI et release de la première revue. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
maximus added 1 commit 2026-10-18 12:22:26 +00:00 
REVIEW.md sets the review level of a PR from the files its diff touches
(high, normal or low; the highest wins, never the author), lists what
matters in each zone with its source (a CLAUDE.md section or an ADR,
completed by the repo file where the fact lives), the verification
commands of the CI, and the review pitfalls of this repo. The pitfalls
taken from the global pr-review skill are corrected for this repo:
stacked PRs do get CI here since #232.

Calibration on the merged PRs #344 to #452: the 39 reference PRs of the
pilot document come out 3 low / 28 normal / 8 high (20.5 %, target
20 %), the whole range of 51 PRs 6 / 33 / 12 (23.5 %), against 3 / 19 /
17 with the trial patterns. A lockfile-only version bump of a package
already present stays normal, per Max's decision of 2026-10-17.
CLAUDE.md points to the file.

Fixes #456

Note: plan check bypassed after 3 passes, last findings applied (Max's
decision of 2026-10-18).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
maximus commented 2026-10-18 12:43:22 +00:00
Author
Owner
Copy link

Revue — PR #460 (fa1abb1)

Verdict : APPROVE

Niveau selon ses propres règles : haut. REVIEW.md crée la section « Niveaux » (dernière ligne du tableau Haut) et CLAUDE.md n'est jamais bas. Aucune version de REVIEW.md n'existe encore sur main, donc rien ne règle la profondeur : la revue a relu chaque fait vérifiable contre le dépôt et rejoué la calibration.

Résumé. Le fichier tient ce qu'il annonce : la calibration se reproduit à l'identique, les faits cités sont exacts à une exception près (point 1), et les cinq critères d'acceptation de #456 sont remplis, l'écart de 20,5 % étant justifié. Les suggestions portent sur ce fait inexact, un défaut latent du script de calibration et des fichiers à risque que le tableau Haut laisse en normal ; aucune ne bloque le merge.

Ce qui a été exécuté

  • Calibration rejouée. Le script du corps de la PR, extrait tel quel et lancé contre l'API (lecture seule), redonne les chiffres publiés, PR par PR : 39 PR, essai 3/19/17 et REVIEW.md 3/28/8 (20,5 %) ; 51 PR, 6/22/23 et 6/33/12 (23,5 %).
  • Motifs. L'ensemble des motifs du script, comparé par programme à la première colonne du tableau Haut, est identique (seuls *.md et src-tauri/, de la ligne en prose, y sont codés en logique).
  • Sources et liens. 42 règles de zone, chacune avec une source CLAUDE.md ou ADR ; 29 liens, tous résolus ; symboles cités présents (migration_checksums_match_the_released_ones dans profile_db.rs, prepare_profile_db, infer_floor, derive_machine_id, toSqlFlag, notifyDataChanged, COUNTERS_SQL, isEntitled, useEntitlement, withTransaction, SR_DEV_EDITION et dev-override).
  • Faits relus contre les fichiers. Déclencheurs, paths, paths-ignore et commandes des quatre workflows ; gardes, canaris et continue-on-error de check-rust ; absence de --locked ; clé de signature dans les deux étapes npx tauri build ; connect-src et identifier de tauri.conf.json ; permissions de capabilities/default.json (aucune fs) ; single-instance en premier plugin, callback sans code remonté au front ; quatre fichiers reqwest, aucun fetch réseau côté TS ; en-têtes de fetch_price ; champs de FeedbackContext ; court-circuit free en tête de isEntitled ; clés de check-latest-json.jq ; sections citées des ADR 0004, 0005, 0006, 0009, 0017, 0018, 0020, 0021, 0022 et 0023, de docs/architecture.md et des étapes 0 et 10 du skill /release.
  • Dépôt public. Lignes ajoutées scannées (courriel, préfixe SR-, JWT, clés Stripe, PEM, jetons) : rien.
  • CI. Aucun statut sur fa1abb1, ce qui est attendu : deux *.md de la racine, exclus par check-frontend et hors des paths de check-rust. Suites non rejouées : aucun build, test ni workflow ne lit CLAUDE.md ou REVIEW.md (git grep), le diff ne peut pas changer leur résultat.
  • Fermeture. La timeline de #456 porte un pull_ref closes venant de #460 ; commit conventionnel, Fixes #456, trailer présent.

Suggestions (non bloquantes)

1. Fait inexact, piège « CI d'une pile » (REVIEW.md:177). src-tauri/src/commands/fs_commands.rs:218-221 compile src/__fixtures__/samples/exemple-desjardins.csv et exemple-tangerine.csv par include_str!. Ces deux fichiers sont hors des paths de check-rust : une PR qui les renomme, les déplace ou les supprime casse cargo check sans que check-rust tourne (seul check-frontend tourne, et vitest passe si samples/index.ts suit), et la casse n'apparaît qu'au run du lundi ou au pré-vol de /release. « L'état Rust d'une pile est prouvé par le dernier maillon qui en a touché » est donc faux pour eux. Une phrase dans le piège suffit ici ; ajouter src/__fixtures__/samples/** aux paths de check-rust relève d'un suivi.

2. Script de calibration : pages tronquées. Il demande /pulls/{n}/files?limit=100 et s'arrête dès que len(fb) < 100, alors que l'instance plafonne les pages à 50 (max_response_items: 50 ; /pulls?limit=100 rend 50 éléments). Au-delà de 50 fichiers, une PR serait tronquée en silence. Sans effet sur la calibration publiée (la plus grosse PR de la plage, #444, compte 38 fichiers), mais REVIEW.md:71 demande de rejouer ce script à chaque modification de la section : boucler jusqu'à une page vide avant le pilote #458. Versionner le script (dans scripts/, par exemple) éviterait aussi que « ce script » dépende d'un corps de PR modifiable, et ferait changer motifs et script dans le même diff.

3. Fichiers à risque que le tableau Haut laisse en normal. Chaque cas ci-dessous, passé au script, sort normal.

  • .claude/settings.json (absent aujourd'hui ; settings.local.json est ignoré). Un fichier ajouté sous .claude/ ne relève pas d'« un dossier caché nouveau ». Ses hooks lancent des commandes sur le poste à chaque session Claude Code, avec les droits et les jetons de l'utilisateur, et permissions élargit ce que /autopilot fait sans demander ; aucune CI ne le voit (.claude/** exclu). Même raisonnement pour un .mcp.json modifié (son ajout est déjà couvert par la règle des fichiers de la racine).
  • Consentement aux cours. Il vit côté TS (src/components/balance/PriceFetchControl.tsx:128-140, src/components/settings/PriceFetchConsentToggle.tsx), et fetch_price ne le vérifie pas. Un diff qui retire ou contourne cette vérification n'ajoute pas d'appel et ne change pas ce qu'il envoie, au sens littéral de la ligne « code livré » (REVIEW.md:29). Ajouter « retire ou contourne un consentement » à sa condition, et nommer ces deux fichiers dans la zone Vie privée.
  • Intégrité des données. dataExportService.ts (la restauration fait DELETE FROM transactions, categories, keywords…, lignes 666-721) et db.ts (verrou et changement de profil, #408, incident de la v0.10.0) sont normaux en entier. Une condition à la façon de balance_commands.rs (« le diff touche les suppressions de la restauration », « le verrou ou le changement de profil ») les laisserait normaux pour #402 et #433. Choix de calibration, à confirmer par Max.
  • Workflows de vérification (REVIEW.md:40 et 46). « Un déclencheur ajouté » reste normal, pull_request_target compris, alors que ce déclencheur (documenté par Forgejo Actions, non éprouvé sur cette instance) fait tourner la définition de la base avec ses secrets et son jeton pour toute PR, forks compris, sur un dépôt public : à nommer au niveau haut. Préciser aussi si la liste qui suit les deux-points est exhaustive : une étape d'installation modifiée (toolchain épinglée, binaire remplacé dans /root/.cargo/bin, qui est dans le PATH du job) peut faire passer au vert sans y figurer.
  • Locks (REVIEW.md:36). « Change l'origine d'un paquet sans changer sa version » laisse passer une origine changée en même temps que la version (hôte de resolved ou source différent dans le hunk d'une montée). Juger l'hôte et le schéma, pas la version.

4. Zone Release (REVIEW.md:123) : la liste de ce qui voit la clé est incomplète. release.yml:46 installe cargo-xwin sans version épinglée (cargo install --locked cargo-xwin prend la dernière version publiée à chaque tag), puis release.yml:58-63 le lance par --runner cargo-xwin avec TAURI_SIGNING_PRIVATE_KEY dans l'environnement ; la toolchain Rust (stable, release.yml:29) n'est pas épinglée non plus. Hors diff, mais c'est du code qui voit la clé ; épingler cargo-xwin serait un suivi de release.yml.

5. Garde windows-sys (REVIEW.md:117). Le workflow interdit d'éditer CRATES, PIN et EXPECTED pour faire passer l'étape (check-rust.yml:221-224). REVIEW.md nomme la liste, la version et le canari, pas EXPECTED, qui est la correction paresseuse la plus probable quand une version de windows-sys apparaît dans le lock.

6. Pour le pilote #458. Appliquées aux deux autres PR ouvertes, les règles donnent haut pour #459 (attendu) et pour #450, qui ne change qu'une instruction de .claude/skills/release/SKILL.md. Le motif .claude/skills/release/** n'avait aucun cas isolé dans la calibration (#414, #416 et #418 touchaient aussi release.yml) : à confirmer, ou à restreindre à scripts/ et aux étapes de tag et de publication.

7. Précision (REVIEW.md:99). Le service du trousseau est une constante (KEYCHAIN_SERVICE = "com.simpl.resultat", token_store.rs:30), alignée sur l'identifier par convention, pas dérivée : changer l'identifier désaligne le trousseau sans rendre ses jetons OAuth orphelins. Deviennent orphelins les bases (app_config_dir()) et, sous app_data_dir(), la clé de licence, le jeton d'activation et le fichier de repli des jetons.

Hors PR : le commentaire de release.yml:104 annonce encore « a .deb fallback », alors que la clé générique a été retirée le 2026-10-11 (#320, PR #418).

Actions faites

  • Label de #456 : status:review remplacé par status:approved.
  • Copie de travail hors dépôt (/tmp/pr460-calib/ : script extrait et sa sortie) supprimée.
## Revue — PR #460 (`fa1abb1`) **Verdict : APPROVE** **Niveau selon ses propres règles : haut.** `REVIEW.md` crée la section « Niveaux » (dernière ligne du tableau Haut) et `CLAUDE.md` n'est jamais bas. Aucune version de `REVIEW.md` n'existe encore sur `main`, donc rien ne règle la profondeur : la revue a relu chaque fait vérifiable contre le dépôt et rejoué la calibration. **Résumé.** Le fichier tient ce qu'il annonce : la calibration se reproduit à l'identique, les faits cités sont exacts à une exception près (point 1), et les cinq critères d'acceptation de #456 sont remplis, l'écart de 20,5 % étant justifié. Les suggestions portent sur ce fait inexact, un défaut latent du script de calibration et des fichiers à risque que le tableau Haut laisse en normal ; aucune ne bloque le merge. ### Ce qui a été exécuté - **Calibration rejouée.** Le script du corps de la PR, extrait tel quel et lancé contre l'API (lecture seule), redonne les chiffres publiés, PR par PR : 39 PR, essai 3/19/17 et `REVIEW.md` 3/28/8 (20,5 %) ; 51 PR, 6/22/23 et 6/33/12 (23,5 %). - **Motifs.** L'ensemble des motifs du script, comparé par programme à la première colonne du tableau Haut, est identique (seuls `*.md` et `src-tauri/`, de la ligne en prose, y sont codés en logique). - **Sources et liens.** 42 règles de zone, chacune avec une source `CLAUDE.md` ou ADR ; 29 liens, tous résolus ; symboles cités présents (`migration_checksums_match_the_released_ones` dans `profile_db.rs`, `prepare_profile_db`, `infer_floor`, `derive_machine_id`, `toSqlFlag`, `notifyDataChanged`, `COUNTERS_SQL`, `isEntitled`, `useEntitlement`, `withTransaction`, `SR_DEV_EDITION` et `dev-override`). - **Faits relus contre les fichiers.** Déclencheurs, `paths`, `paths-ignore` et commandes des quatre workflows ; gardes, canaris et `continue-on-error` de `check-rust` ; absence de `--locked` ; clé de signature dans les deux étapes `npx tauri build` ; `connect-src` et `identifier` de `tauri.conf.json` ; permissions de `capabilities/default.json` (aucune `fs`) ; single-instance en premier plugin, callback sans `code` remonté au front ; quatre fichiers `reqwest`, aucun `fetch` réseau côté TS ; en-têtes de `fetch_price` ; champs de `FeedbackContext` ; court-circuit `free` en tête de `isEntitled` ; clés de `check-latest-json.jq` ; sections citées des ADR 0004, 0005, 0006, 0009, 0017, 0018, 0020, 0021, 0022 et 0023, de `docs/architecture.md` et des étapes 0 et 10 du skill `/release`. - **Dépôt public.** Lignes ajoutées scannées (courriel, préfixe `SR-`, JWT, clés Stripe, PEM, jetons) : rien. - **CI.** Aucun statut sur `fa1abb1`, ce qui est attendu : deux `*.md` de la racine, exclus par `check-frontend` et hors des `paths` de `check-rust`. Suites non rejouées : aucun build, test ni workflow ne lit `CLAUDE.md` ou `REVIEW.md` (`git grep`), le diff ne peut pas changer leur résultat. - **Fermeture.** La timeline de #456 porte un `pull_ref` `closes` venant de #460 ; commit conventionnel, `Fixes #456`, trailer présent. ### Suggestions (non bloquantes) **1. Fait inexact, piège « CI d'une pile » (`REVIEW.md:177`).** `src-tauri/src/commands/fs_commands.rs:218-221` compile `src/__fixtures__/samples/exemple-desjardins.csv` et `exemple-tangerine.csv` par `include_str!`. Ces deux fichiers sont hors des `paths` de `check-rust` : une PR qui les renomme, les déplace ou les supprime casse `cargo check` sans que `check-rust` tourne (seul `check-frontend` tourne, et vitest passe si `samples/index.ts` suit), et la casse n'apparaît qu'au run du lundi ou au pré-vol de `/release`. « L'état Rust d'une pile est prouvé par le dernier maillon qui en a touché » est donc faux pour eux. Une phrase dans le piège suffit ici ; ajouter `src/__fixtures__/samples/**` aux `paths` de `check-rust` relève d'un suivi. **2. Script de calibration : pages tronquées.** Il demande `/pulls/{n}/files?limit=100` et s'arrête dès que `len(fb) < 100`, alors que l'instance plafonne les pages à 50 (`max_response_items: 50` ; `/pulls?limit=100` rend 50 éléments). Au-delà de 50 fichiers, une PR serait tronquée en silence. Sans effet sur la calibration publiée (la plus grosse PR de la plage, #444, compte 38 fichiers), mais `REVIEW.md:71` demande de rejouer ce script à chaque modification de la section : boucler jusqu'à une page vide avant le pilote #458. Versionner le script (dans `scripts/`, par exemple) éviterait aussi que « ce script » dépende d'un corps de PR modifiable, et ferait changer motifs et script dans le même diff. **3. Fichiers à risque que le tableau Haut laisse en normal.** Chaque cas ci-dessous, passé au script, sort normal. - *`.claude/settings.json`* (absent aujourd'hui ; `settings.local.json` est ignoré). Un fichier ajouté sous `.claude/` ne relève pas d'« un dossier caché nouveau ». Ses `hooks` lancent des commandes sur le poste à chaque session Claude Code, avec les droits et les jetons de l'utilisateur, et `permissions` élargit ce que `/autopilot` fait sans demander ; aucune CI ne le voit (`.claude/**` exclu). Même raisonnement pour un `.mcp.json` modifié (son ajout est déjà couvert par la règle des fichiers de la racine). - *Consentement aux cours.* Il vit côté TS (`src/components/balance/PriceFetchControl.tsx:128-140`, `src/components/settings/PriceFetchConsentToggle.tsx`), et `fetch_price` ne le vérifie pas. Un diff qui retire ou contourne cette vérification n'ajoute pas d'appel et ne change pas ce qu'il envoie, au sens littéral de la ligne « code livré » (`REVIEW.md:29`). Ajouter « retire ou contourne un consentement » à sa condition, et nommer ces deux fichiers dans la zone Vie privée. - *Intégrité des données.* `dataExportService.ts` (la restauration fait `DELETE FROM transactions`, `categories`, `keywords`…, lignes 666-721) et `db.ts` (verrou et changement de profil, #408, incident de la v0.10.0) sont normaux en entier. Une condition à la façon de `balance_commands.rs` (« le diff touche les suppressions de la restauration », « le verrou ou le changement de profil ») les laisserait normaux pour #402 et #433. Choix de calibration, à confirmer par Max. - *Workflows de vérification (`REVIEW.md:40` et 46).* « Un déclencheur ajouté » reste normal, `pull_request_target` compris, alors que ce déclencheur (documenté par Forgejo Actions, non éprouvé sur cette instance) fait tourner la définition de la base avec ses secrets et son jeton pour toute PR, forks compris, sur un dépôt public : à nommer au niveau haut. Préciser aussi si la liste qui suit les deux-points est exhaustive : une étape d'installation modifiée (toolchain épinglée, binaire remplacé dans `/root/.cargo/bin`, qui est dans le `PATH` du job) peut faire passer au vert sans y figurer. - *Locks (`REVIEW.md:36`).* « Change l'origine d'un paquet sans changer sa version » laisse passer une origine changée en même temps que la version (hôte de `resolved` ou `source` différent dans le hunk d'une montée). Juger l'hôte et le schéma, pas la version. **4. Zone Release (`REVIEW.md:123`) : la liste de ce qui voit la clé est incomplète.** `release.yml:46` installe `cargo-xwin` sans version épinglée (`cargo install --locked cargo-xwin` prend la dernière version publiée à chaque tag), puis `release.yml:58-63` le lance par `--runner cargo-xwin` avec `TAURI_SIGNING_PRIVATE_KEY` dans l'environnement ; la toolchain Rust (`stable`, `release.yml:29`) n'est pas épinglée non plus. Hors diff, mais c'est du code qui voit la clé ; épingler `cargo-xwin` serait un suivi de `release.yml`. **5. Garde `windows-sys` (`REVIEW.md:117`).** Le workflow interdit d'éditer `CRATES`, `PIN` et `EXPECTED` pour faire passer l'étape (`check-rust.yml:221-224`). `REVIEW.md` nomme la liste, la version et le canari, pas `EXPECTED`, qui est la correction paresseuse la plus probable quand une version de `windows-sys` apparaît dans le lock. **6. Pour le pilote #458.** Appliquées aux deux autres PR ouvertes, les règles donnent haut pour #459 (attendu) et pour #450, qui ne change qu'une instruction de `.claude/skills/release/SKILL.md`. Le motif `.claude/skills/release/**` n'avait aucun cas isolé dans la calibration (#414, #416 et #418 touchaient aussi `release.yml`) : à confirmer, ou à restreindre à `scripts/` et aux étapes de tag et de publication. **7. Précision (`REVIEW.md:99`).** Le service du trousseau est une constante (`KEYCHAIN_SERVICE = "com.simpl.resultat"`, `token_store.rs:30`), alignée sur l'`identifier` par convention, pas dérivée : changer l'`identifier` désaligne le trousseau sans rendre ses jetons OAuth orphelins. Deviennent orphelins les bases (`app_config_dir()`) et, sous `app_data_dir()`, la clé de licence, le jeton d'activation et le fichier de repli des jetons. Hors PR : le commentaire de `release.yml:104` annonce encore « a .deb fallback », alors que la clé générique a été retirée le 2026-10-11 (#320, PR #418). ### Actions faites - Label de #456 : `status:review` remplacé par `status:approved`. - Copie de travail hors dépôt (`/tmp/pr460-calib/` : script extrait et sa sortie) supprimée.
maximus added 1 commit 2026-10-18 12:55:40 +00:00
docs(review): apply the review of #460, version the calibration script
All checks were successful
PR Check — Frontend / frontend (pull_request) Successful in 1m55s
Details
24da72b7f4 
REVIEW.md, from the suggestions of the first review of #460:
- stacked-PR pitfall: fs_commands.rs compiles two CSV files of
  src/__fixtures__/samples/ through include_str!, outside the paths of
  check-rust;
- high level: .claude/settings.json and .mcp.json, a pull_request_target
  trigger, an install step that changes the toolchain or the PATH (the
  check-workflow list is now given as examples of the principle), a lock
  origin whose host or scheme changes even with a new version, and a diff
  that removes or bypasses a consent (price consent lives on the front);
- db.ts and dataExportService.ts stay normal (Max's decision of
  2026-10-18), and the data zone now names the lock, the profile switch
  and the restore;
- release zone: cargo-xwin and the Rust toolchain, installed unpinned at
  each tag, also run with the signing key;
- windows-sys guard: EXPECTED joins CRATES and PIN;
- identifier: changing it orphans the databases and the app_data_dir
  files (license key, activation token, token fallback); the keychain
  service is a constant and does not follow.

scripts/review-levels.py is the calibration script of the PR body, now in
the repo: pages of 50 files read until an empty page (the instance caps
pages at 50), the new rules above, comment lines ignored by the workflow
checks, and --check, which compares its patterns with the Haut table
without network access. Fresh run: 3/28/8 on the 39 reference PRs and
6/33/12 on the 51, unchanged.

Refs #456

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
maximus commented 2026-10-18 12:56:42 +00:00
Author
Owner
Copy link

Suggestions de la revue 24517 appliquées dans 24da72b, commit ajouté après l'APPROVE et pas encore relu : #456 repasse en status:review, et une seconde revue suit (choix de Max).

  • REVIEW.md : points 1, 3, 4, 5 et 7 (détail dans la section « Après la première revue » du corps de la PR). db.ts et dataExportService.ts restent normaux sur décision de Max ; la zone les nomme.
  • scripts/review-levels.py : le script du corps de la PR, versionné. Pages de 50 lues jusqu'à une page vide, nouvelles règles, commentaires ignorés par les contrôles de workflow, et --check, qui compare les motifs au tableau Haut sans réseau.
  • Preuves :
    • --check passe (37 motifs) et sort 1 sur une copie du tableau privée de .npmrc ;
    • sondes synthétiques rouges : origine changée avec la version (Cargo et npm), toolchain modifiée, branches: ajouté, entrée de paths-ignore ajoutée ;
    • sondes vertes : montée de version simple, commentaire qui cite « branches » ;
    • passage complet à neuf contre l'API : 3/28/8 et 6/33/12, identiques PR par PR au passage de fa1abb1.
  • Point 6 (#450) laissé au pilote #458 ; suivis CI et release dans #461.
Suggestions de la revue 24517 appliquées dans `24da72b`, **commit ajouté après l'APPROVE et pas encore relu** : #456 repasse en `status:review`, et une seconde revue suit (choix de Max). - `REVIEW.md` : points 1, 3, 4, 5 et 7 (détail dans la section « Après la première revue » du corps de la PR). `db.ts` et `dataExportService.ts` restent normaux sur décision de Max ; la zone les nomme. - `scripts/review-levels.py` : le script du corps de la PR, versionné. Pages de 50 lues jusqu'à une page vide, nouvelles règles, commentaires ignorés par les contrôles de workflow, et `--check`, qui compare les motifs au tableau Haut sans réseau. - Preuves : - `--check` passe (37 motifs) et sort 1 sur une copie du tableau privée de `.npmrc` ; - sondes synthétiques rouges : origine changée avec la version (Cargo et npm), toolchain modifiée, `branches:` ajouté, entrée de `paths-ignore` ajoutée ; - sondes vertes : montée de version simple, commentaire qui cite « branches » ; - passage complet à neuf contre l'API : 3/28/8 et 6/33/12, identiques PR par PR au passage de `fa1abb1`. - Point 6 (#450) laissé au pilote #458 ; suivis CI et release dans #461.
maximus commented 2026-10-18 13:11:04 +00:00
Author
Owner
Copy link

Revue — PR #460 (24da72b), seconde passe

Verdict : APPROVE

Niveau selon ses propres règles : haut, fixé par REVIEW.md (création de la section « Niveaux »). main n'a pas encore de REVIEW.md : la revue couvre le diff complet, en priorité 24da72b, ajouté après la première revue. Max doit lire.

Résumé. Les faits ajoutés par 24da72b sont exacts, la calibration se reproduit à l'identique avec le script versionné, et la CI de la tête est verte. Les constats restants appartiennent aux familles déjà relevées par la première revue : des listes énumérées qui oublient un membre, et des conditions que le script approche. Aucun ne bloque. La règle de non-convergence du skill demande donc de merger plutôt que d'ouvrir un troisième tour.

Ce qui a été exécuté

  • Calibration rejouée à neuf avec scripts/review-levels.py de 24da72b, sur une copie hors dépôt, contre l'API en lecture seule, réponses mises en cache. Sur 39 PR : essai 3/19/17, REVIEW.md 3/28/8 (20,5 %). Sur 51 PR : 6/22/23 et 6/33/12 (23,5 %). Les niveaux sont identiques, PR par PR, au tableau du corps.
  • --check passe (37 motifs). Mutations sur une copie du tableau :
    • motif retiré (.mcp.json) : sortie 1 ;
    • ligne retirée (export_import_commands.rs) : sortie 1 ;
    • condition assouplie (src-tauri/src/database/** passé de « toujours » à « le diff ajoute un fichier .sql ») : sortie 0 (point 4).
  • Onze sondes synthétiques passées à review_level() (diffs fabriqués, base locale, aucun réseau) : 5 rendent le niveau du texte, 6 divergent (point 4).
  • Faits de 24da72b relus contre le code :
    • consentement aux cours : aucun contrôle dans balance_commands.rs ; hasConsent() est appelé dans handleClick (PriceFetchControl.tsx:129-134), seul appelant de prices.fetchPrice (balance.service.ts:2812) ;
    • identifier : license.key et activation.token sous app_data_dir() (license_commands.rs:56-113), repli auth/tokens.json au même endroit (token_store.rs:74-86), KEYCHAIN_SERVICE constante (token_store.rs:30) ;
    • verrou de db.ts, withTransaction et générations ; dans runRestore (dataExportService.ts:424-445), notifyDataChanged() vient après withTransaction. Sources vérifiées : tableau de l'ADR 0022, « Reconnexion nécessaire » de l'ADR 0005, docs/architecture.md:145 ;
    • release.yml : rustup sans version (l. 29), cargo install --locked cargo-xwin sans --version (l. 46), --runner cargo-xwin avec la clé (l. 58-63) ;
    • garde windows-sys : CRATES, PIN, EXPECTED et canari (check-rust.yml:222-230) ;
    • include_str! des deux CSV de src/__fixtures__/samples/ (fs_commands.rs:219-221).
  • Liens : 31 liens, tous résolus. Le commit en a ajouté deux (ADR 0022 et 0005), et le corps de la PR annonce encore 29. Lignes ajoutées scannées (courriel, préfixe SR-, JWT, clés Stripe, PEM, jetons) : rien.
  • CI : journal du run 562 de check-frontend sur 24da72b lu (checkout de 24da72b, build, 134 fichiers et 2248 tests verts). check-rust n'a pas tourné, ce qui est attendu.
  • Fermeture : la timeline de #456 porte un pull_ref closes venant de #460 ; commits conventionnels, trailers présents.

Suggestions (non bloquantes)

Les points 1 à 3 prolongent les points 3, 4 et 7 de la première revue : chaque passe trouve un membre de plus d'une liste énumérée. Une phrase de principe à la place de l'énumération ferme la famille. Sinon, les confier au pilote #458 plutôt qu'à un tour de plus.

1. Locks : vérifier l'hôte ne suffit pas, une liste blanche ferme la famille (REVIEW.md:36). Un cas reste normal, au texte comme au script : 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, sonde P5). La même sonde avec un autre hôte sort haute. C'est le cas que vise l'option --validate-package-names de lockfile-lint.

  • Les 252 resolved du lock actuel ont tous la forme canonique https://registry.npmjs.org/<nom de la clé>/-/<nom court>-<version>.tgz, sans alias, et les 665 source de Cargo.lock sont 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 ne change de niveau.

2. Consentements : l'app en a deux (REVIEW.md:29 et :85). La parenthèse de la ligne « code livré » et la zone Vie privée ne nomment que celui des cours.

  • Celui du Feedback Hub vit aussi côté front : openFeedback de LogViewerCard.tsx:20-33 (localStorage, FeedbackConsentDialog.tsx), seul chemin vers FeedbackDialog. Le nommer.
  • Nommer aussi le site d'appel des cours (prices.fetchPrice) : un nouvel appelant hors de PriceFetchControl contourne le consentement sans toucher aux deux fichiers nommés.
  • Précision : 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). La liste omet Node.js, installé par le script NodeSource sans archive épinglée, que release.yml:19-25 désigne lui-même comme lancé avec la clé. Elle omet aussi actions/checkout@v4, une étiquette mobile. Le job tourne dans un seul conteneur : ce qui s'y exécute avant les deux npx tauri build atteint la clé. Proposé : « Tout ce qui s'exécute dans le job build-and-release atteint la clé ; hors de release.yml, les points d'entrée sont : … ». Les niveaux n'en dépendent pas, release.yml étant toujours haut.
  • identifier (REVIEW.md:101). Sous app_data_dir() vivent aussi profiles.json (liste des profils et pin_hash, profile_commands.rs:13-35) et tout le dossier auth/. Proposé : « orpheline tout ce que l'app range sous ces deux dossiers, dont … ».
  • Même famille, pour #458 : un fichier que .claude/settings.json ou .mcp.json fait exécuter (script d'un hook, commande d'un serveur MCP) porte le même risque qu'eux, mais reste normal sous .claude/**.

4. Le script approche les conditions : le dire. Six sondes s'écartent du texte :

  • un env: ajouté sous name: au niveau du workflow (P2a), ou au niveau du job avant runs-on: (P3), sort normal. Le résultat dépend de la place : à côté de permissions: ou de container:, il sort haut ;
  • une étape run: ajoutée qui supprime ou édite un fichier du dépôt par rm ou sed -i sort normale (P4, P4b) ;
  • en sens inverse, une ligne de commentaire seule dans lib.rs (P6) ou src-tauri/Cargo.toml (P7) sort normale. changed() écarte les commentaires, alors que le texte ne les exempte pas pour ces lignes.

Aucune de ces sondes ne déplace l'une des 51 PR : dans les PR normales, lib.rs ne change que des entrées de generate_handler! (#366, #368, #380, #387), et Cargo.toml n'apparaît que dans #394, haute par ailleurs. À faire :

  • la docstring annonce pour les workflows « a change that can turn a failing check green » : écrire que le script n'en voit qu'une partie, et que le niveau d'une PR se lit dans REVIEW.md, pas dans sa sortie (ni #457 ni #458 ne doivent l'utiliser 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 la première colonne (mutation sur la condition, sortie 0). REVIEW.md:71 le dit bien (« ses motifs »), mais :73 demande de mettre le script à jour dans le même diff, et ce contrôle n'en couvre que les motifs. L'écrire : les conditions restent à la revue de niveau haut.

5. #457 contredit REVIEW.md:5. Son point A1 lit REVIEW.md à ?ref=<base>, alors que le fichier demande main. L'écart est consigné dans le corps de cette PR, pas sur l'issue. Commenter #457 avant qu'elle démarre, pour que l'implémentation lise main.

Actions faites

  • Label de #456 : status:review remplacé par status:approved.
  • Copie de travail hors dépôt (/tmp/pr460-review/ : script, REVIEW.md, cache des réponses de l'API, sondes, journal du run 562) supprimée.
## Revue — PR #460 (`24da72b`), seconde passe **Verdict : APPROVE** **Niveau selon ses propres règles : haut**, fixé par `REVIEW.md` (création de la section « Niveaux »). `main` n'a pas encore de `REVIEW.md` : la revue couvre le diff complet, en priorité `24da72b`, ajouté après la première revue. Max doit lire. **Résumé.** Les faits ajoutés par `24da72b` sont exacts, la calibration se reproduit à l'identique avec le script versionné, et la CI de la tête est verte. Les constats restants appartiennent aux familles déjà relevées par la première revue : des listes énumérées qui oublient un membre, et des conditions que le script approche. Aucun ne bloque. La règle de non-convergence du skill demande donc de merger plutôt que d'ouvrir un troisième tour. ### Ce qui a été exécuté - **Calibration rejouée à neuf** avec `scripts/review-levels.py` de `24da72b`, sur une copie hors dépôt, contre l'API en lecture seule, réponses mises en cache. Sur 39 PR : essai 3/19/17, `REVIEW.md` 3/28/8 (20,5 %). Sur 51 PR : 6/22/23 et 6/33/12 (23,5 %). Les niveaux sont identiques, PR par PR, au tableau du corps. - **`--check`** passe (37 motifs). Mutations sur une copie du tableau : - motif retiré (`.mcp.json`) : sortie 1 ; - ligne retirée (`export_import_commands.rs`) : sortie 1 ; - condition assouplie (`src-tauri/src/database/**` passé de « toujours » à « le diff ajoute un fichier `.sql` ») : sortie 0 (point 4). - **Onze sondes synthétiques** passées à `review_level()` (diffs fabriqués, base locale, aucun réseau) : 5 rendent le niveau du texte, 6 divergent (point 4). - **Faits de `24da72b` relus contre le code :** - consentement aux cours : aucun contrôle dans `balance_commands.rs` ; `hasConsent()` est appelé dans `handleClick` (`PriceFetchControl.tsx:129-134`), seul appelant de `prices.fetchPrice` (`balance.service.ts:2812`) ; - `identifier` : `license.key` et `activation.token` sous `app_data_dir()` (`license_commands.rs:56-113`), repli `auth/tokens.json` au même endroit (`token_store.rs:74-86`), `KEYCHAIN_SERVICE` constante (`token_store.rs:30`) ; - verrou de `db.ts`, `withTransaction` et générations ; dans `runRestore` (`dataExportService.ts:424-445`), `notifyDataChanged()` vient après `withTransaction`. Sources vérifiées : tableau de l'ADR 0022, « Reconnexion nécessaire » de l'ADR 0005, `docs/architecture.md:145` ; - `release.yml` : rustup sans version (l. 29), `cargo install --locked cargo-xwin` sans `--version` (l. 46), `--runner cargo-xwin` avec la clé (l. 58-63) ; - garde `windows-sys` : `CRATES`, `PIN`, `EXPECTED` et canari (`check-rust.yml:222-230`) ; - `include_str!` des deux CSV de `src/__fixtures__/samples/` (`fs_commands.rs:219-221`). - **Liens** : 31 liens, tous résolus. Le commit en a ajouté deux (ADR 0022 et 0005), et le corps de la PR annonce encore 29. Lignes ajoutées scannées (courriel, préfixe `SR-`, JWT, clés Stripe, PEM, jetons) : rien. - **CI** : journal du run 562 de `check-frontend` sur `24da72b` lu (checkout de `24da72b`, build, 134 fichiers et 2248 tests verts). `check-rust` n'a pas tourné, ce qui est attendu. - **Fermeture** : la timeline de #456 porte un `pull_ref` `closes` venant de #460 ; commits conventionnels, trailers présents. ### Suggestions (non bloquantes) Les points 1 à 3 prolongent les points 3, 4 et 7 de la première revue : chaque passe trouve un membre de plus d'une liste énumérée. Une phrase de principe à la place de l'énumération ferme la famille. Sinon, les confier au pilote #458 plutôt qu'à un tour de plus. **1. Locks : vérifier l'hôte ne suffit pas, une liste blanche ferme la famille (`REVIEW.md:36`).** Un cas reste normal, au texte comme au script : 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`, sonde P5). La même sonde avec un autre hôte sort haute. C'est le cas que vise l'option `--validate-package-names` de lockfile-lint. - Les 252 `resolved` du lock actuel ont tous la forme canonique `https://registry.npmjs.org/<nom de la clé>/-/<nom court>-<version>.tgz`, sans alias, et les 665 `source` de `Cargo.lock` sont 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 ne change de niveau. **2. Consentements : l'app en a deux (`REVIEW.md:29` et `:85`).** La parenthèse de la ligne « code livré » et la zone Vie privée ne nomment que celui des cours. - Celui du Feedback Hub vit aussi côté front : `openFeedback` de `LogViewerCard.tsx:20-33` (`localStorage`, `FeedbackConsentDialog.tsx`), seul chemin vers `FeedbackDialog`. Le nommer. - Nommer aussi le site d'appel des cours (`prices.fetchPrice`) : un nouvel appelant hors de `PriceFetchControl` contourne le consentement sans toucher aux deux fichiers nommés. - Précision : `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`).* La liste omet Node.js, installé par le script NodeSource sans archive épinglée, que `release.yml:19-25` désigne lui-même comme lancé avec la clé. Elle omet aussi `actions/checkout@v4`, une étiquette mobile. Le job tourne dans un seul conteneur : ce qui s'y exécute avant les deux `npx tauri build` atteint la clé. Proposé : « Tout ce qui s'exécute dans le job `build-and-release` atteint la clé ; hors de `release.yml`, les points d'entrée sont : … ». Les niveaux n'en dépendent pas, `release.yml` étant toujours haut. - *`identifier` (`REVIEW.md:101`).* Sous `app_data_dir()` vivent aussi `profiles.json` (liste des profils et `pin_hash`, `profile_commands.rs:13-35`) et tout le dossier `auth/`. Proposé : « orpheline tout ce que l'app range sous ces deux dossiers, dont … ». - Même famille, pour #458 : un fichier que `.claude/settings.json` ou `.mcp.json` fait exécuter (script d'un hook, commande d'un serveur MCP) porte le même risque qu'eux, mais reste normal sous `.claude/**`. **4. Le script approche les conditions : le dire.** Six sondes s'écartent du texte : - un `env:` ajouté sous `name:` au niveau du workflow (P2a), ou au niveau du job avant `runs-on:` (P3), sort normal. Le résultat dépend de la place : à côté de `permissions:` ou de `container:`, il sort haut ; - une étape `run:` ajoutée qui supprime ou édite un fichier du dépôt par `rm` ou `sed -i` sort normale (P4, P4b) ; - en sens inverse, une ligne de commentaire seule dans `lib.rs` (P6) ou `src-tauri/Cargo.toml` (P7) sort normale. `changed()` écarte les commentaires, alors que le texte ne les exempte pas pour ces lignes. Aucune de ces sondes ne déplace l'une des 51 PR : dans les PR normales, `lib.rs` ne change que des entrées de `generate_handler!` (#366, #368, #380, #387), et `Cargo.toml` n'apparaît que dans #394, haute par ailleurs. À faire : - la docstring annonce pour les workflows « a change that can turn a failing check green » : écrire que le script n'en voit qu'une partie, et que le niveau d'une PR se lit dans `REVIEW.md`, pas dans sa sortie (ni #457 ni #458 ne doivent l'utiliser 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 la première colonne (mutation sur la condition, sortie 0). `REVIEW.md:71` le dit bien (« ses motifs »), mais `:73` demande de mettre le script à jour dans le même diff, et ce contrôle n'en couvre que les motifs. L'écrire : les conditions restent à la revue de niveau haut. **5. #457 contredit `REVIEW.md:5`.** Son point A1 lit `REVIEW.md` à `?ref=<base>`, alors que le fichier demande `main`. L'écart est consigné dans le corps de cette PR, pas sur l'issue. Commenter #457 avant qu'elle démarre, pour que l'implémentation lise `main`. ### Actions faites - Label de #456 : `status:review` remplacé par `status:approved`. - Copie de travail hors dépôt (`/tmp/pr460-review/` : script, `REVIEW.md`, cache des réponses de l'API, sondes, journal du run 562) supprimée.
maximus merged commit 24da72b7f4 into main 2026-10-18 13:17:49 +00:00
maximus deleted branch issue-456-review-md 2026-10-18 13:17:49 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
autopilot:pending-human

PR produced by /autopilot — awaits human review/merge

  
source:analyste

Créée par le Super-Analyste

  
source:defenseur

Créée par l Escouade Défenseur

  
source:human

Créée par un humain

  
source:medic

Créée par le Medic (auto-détection)

  
status:approved

PR approuvée, prête à merger

  
status:blocked

Bloquée (question, dépendance, clarification)

  
status:in-progress

Un agent (Medic/Dev) travaille dessus

  
status:needs-clarification

Issue blocked by ambiguity — Max needs to clarify before re-running autopilot

  
status:needs-fix

Reviewer demande des corrections

  
status:ready

Analysée et documentée, prête à prendre

  
status:review

PR créée, en attente du Reviewer

  
status:triage

Nouvelle issue, pas encore analysée

  
type:bug

Correction de bug → Medic

  
type:feature

Nouvelle fonctionnalité → Dev

  
type:infra

Config, CI/CD, déploiement → Dev

  
type:refactor

Refactoring / amélioration technique → Dev

  
type:schema

Modèle de données / migrations → Dev

  
type:security

Issue de sécurité → Medic ou Dev

No labels
autopilot:pending-human
source:analyste
source:defenseur
source:human
source:medic
status:approved
status:blocked
status:in-progress
status:needs-clarification
status:needs-fix
status:ready
status:review
status:triage
type:bug
type:feature
type:infra
type:refactor
type:schema
type:security
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: maximus/Simpl-Resultat#460
No description provided.
Delete branch "issue-456-review-md"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?