docs(review): REVIEW.md, calibrated risk levels and review rules per zone (#456) #460
2 changed files with 410 additions and 14 deletions
31
REVIEW.md
31
REVIEW.md
|
|
@ -26,25 +26,27 @@ Motifs : chemins depuis la racine. `*` ne traverse jamais un `/` ; `**/` vaut z
|
|||
| `src-tauri/src/commands/balance_commands.rs` | le diff touche `fetch_price` ou la lecture du jeton d'activation | Jetons, licence et édition |
|
||||
| `src-tauri/src/commands/export_import_commands.rs` | toujours | Jetons, licence et édition (export chiffré) |
|
||||
| `src-tauri/src/commands/feedback_commands.rs`, `src/services/feedbackService.ts` | toujours | Vie privée |
|
||||
| code livré : `src/**`, `src-tauri/src/**`, `index.html`, `public/**` | le diff ajoute un appel réseau, change ce qu'un appel existant envoie, ou ouvre une URL qui porte une donnée de l'utilisateur (donnée financière, identifiant, contenu d'un profil, ligne du Journal) | Vie privée |
|
||||
| code livré : `src/**`, `src-tauri/src/**`, `index.html`, `public/**` | le diff ajoute un appel réseau, change ce qu'un appel existant envoie, ouvre une URL qui porte une donnée de l'utilisateur (donnée financière, identifiant, contenu d'un profil, ligne du Journal), ou retire ou contourne un consentement (celui des cours vit côté front : `src/components/balance/PriceFetchControl.tsx`, `src/components/settings/PriceFetchConsentToggle.tsx`) | Vie privée |
|
||||
| `src-tauri/capabilities/**` | toujours | Vie privée |
|
||||
| `src-tauri/{tauri*.conf.json*,Tauri*.toml}` | sauf si le diff se limite au champ `"version"` ; un fichier propre à une plateforme est fusionné avec `tauri.conf.json` | Vie privée (CSP), Release (updater, `beforeBuildCommand`) |
|
||||
| `.forgejo/workflows/release.yml`, `scripts/build-latest-json*`, `scripts/check-latest-json.jq`, `.claude/skills/release/**` | toujours | Release et mise à jour |
|
||||
| `src-tauri/build.rs`, `{vite,vitest,postcss}.config.*`, `.postcssrc*`, `.npmrc`, `**/rust-toolchain*`, `Cargo.{toml,lock}` à la racine | toujours | Release et mise à jour, CI et gardes (lu ou exécuté par le build de la release ou par `npm test`) |
|
||||
| fichier à la racine du dépôt ou directement dans `src-tauri/`, hors `*.md` | ajouté, renommé ou supprimé, comme tout fichier d'un dossier caché nouveau : une configuration nouvelle peut changer le build ou la CI sans motif qui la nomme | Release et mise à jour, CI et gardes |
|
||||
| `src-tauri/Cargo.toml`, `package.json` | sauf si le diff se limite au champ `version` de l'app (dépendances, features dont `dev-override`, `overrides`, scripts) | Dépendances ; Release pour les `scripts` de `package.json` |
|
||||
| `src-tauri/Cargo.lock`, `package-lock.json` | le diff ajoute ou retire un paquet (un bloc `[[package]]` dont la ligne `name =` apparaît ou disparaît, une seconde version d'un crate comprise ; une clé `"node_modules/<nom>"` qui apparaît ou disparaît), ou change l'origine d'un paquet sans changer sa version (`source`, `checksum` ; `resolved`, `integrity`) | Dépendances |
|
||||
| `src-tauri/Cargo.lock`, `package-lock.json` | le diff ajoute ou retire un paquet (un bloc `[[package]]` dont la ligne `name =` apparaît ou disparaît, une seconde version d'un crate comprise ; une clé `"node_modules/<nom>"` qui apparaît ou disparaît), ou change l'origine d'un paquet : hôte ou schéma de `source` (Cargo) ou de `resolved` (npm), avec ou sans nouvelle version, ou `checksum`, `integrity` changés sans nouvelle version | Dépendances |
|
||||
| `**/.cargo/**` | toujours | Dépendances |
|
||||
| `.forgejo/workflows/**` | fichier ajouté, ou diff qui ajoute une référence à `secrets` | CI et gardes |
|
||||
| `.forgejo/workflows/**` | fichier ajouté, ou diff qui ajoute une référence à `secrets` ou un déclencheur `pull_request_target`, qui exécute la définition de la base avec ses secrets pour toute PR, forks compris | CI et gardes |
|
||||
| `.forgejo/workflows/audit.yml` | le diff change une ligne active : sans déclencheur `pull_request`, il tourne pour la première fois sur `main` | CI et gardes |
|
||||
| `.forgejo/workflows/{check-rust,check-frontend}.yml` | le diff peut faire passer au vert ce qui échouait : déclencheur retiré ou restreint (`paths`, `paths-ignore`, `branches`, `schedule`) ; étape de verdict modifiée ou retirée (`npm ci`, `npm run build`, `npm test`, `cargo check`, `cargo test`, `Verify …`, `cargo audit`) ; `continue-on-error` ou `\|\| true` ajouté ; étape `run:` ajoutée qui écrit `$GITHUB_ENV`, `$GITHUB_PATH` ou un fichier du dépôt ; action `uses:` ajoutée, hors cache ; `if:`, `defaults:`, `permissions:`, `env:` ou `container:` modifiés | CI et gardes |
|
||||
| `.forgejo/workflows/{check-rust,check-frontend}.yml` | le diff peut faire passer au vert ce qui échouait, par exemple : déclencheur retiré ou restreint (`paths`, `paths-ignore`, `branches`, `schedule`) ; étape de verdict modifiée ou retirée (`npm ci`, `npm run build`, `npm test`, `cargo check`, `cargo test`, `Verify …`, `cargo audit`) ; `continue-on-error` ou `\|\| true` ajouté ; étape `run:` ajoutée qui écrit `$GITHUB_ENV`, `$GITHUB_PATH` ou un fichier du dépôt ; action `uses:` ajoutée, hors cache ; étape d'installation modifiée (toolchain, binaire placé dans le `PATH`) ; `if:`, `defaults:`, `permissions:`, `env:` ou `container:` modifiés | CI et gardes |
|
||||
| `.claude/settings.json`, `.mcp.json` | toujours : hooks, permissions et serveurs MCP décident de ce que les agents exécutent sur le poste, sans CI pour le voir | CI et gardes |
|
||||
| `REVIEW.md` | le diff touche la section « Niveaux » | Règles de revue |
|
||||
|
||||
Restent au moins normaux, et leurs règles de zone s'appliquent quand même :
|
||||
|
||||
- une montée de version d'un paquet déjà présent dans un lock, correctif d'advisory compris (décision de Max du 2026-10-17, consignée sur #456), et une arête re-résolue vers une version déjà présente : seul le garde `windows-sys` de la CI en surveille, pour ses sept crates ([ADR 0023](docs/adr/0023-windows-sys-pinning.md)) ;
|
||||
- dans `check-rust.yml` et `check-frontend.yml` : un déclencheur ajouté, une étape `run:` ajoutée qui ne fait que lire, un cache (`Swatinem/rust-cache`, `actions/cache`), un commentaire, la version de Node ;
|
||||
- `src/services/db.ts`, `src/services/dataExportService.ts`, `src-tauri/src/commands/{fs_commands,backup_commands}.rs`, et le reste de `balance_commands.rs` ;
|
||||
- dans `check-rust.yml` et `check-frontend.yml` : un déclencheur ajouté, sauf `pull_request_target`, une étape `run:` ajoutée qui ne fait que lire, un cache (`Swatinem/rust-cache`, `actions/cache`), un commentaire, la version de Node ;
|
||||
- `src/services/db.ts` et `src/services/dataExportService.ts`, même quand le diff touche le verrou, le changement de profil ou les suppressions de la restauration (décision de Max du 2026-10-18, revue de #460) : contre leurs défauts, la mutation du niveau normal est le bon outil ;
|
||||
- `src-tauri/src/commands/{fs_commands,backup_commands}.rs`, et le reste de `balance_commands.rs` ;
|
||||
- `src/shared/entitlements.ts` : la matrice ne verrouille que l'interface ([ADR 0017](docs/adr/0017-feature-gating-par-tier.md), § 2) ;
|
||||
- `CLAUDE.md`, `REVIEW.md` hors de la section « Niveaux », `docs/adr/**`, `docs/architecture.md`, `docs/qa-update-cycle.md` et `.claude/**` : sources de ce fichier et instructions des agents, jamais de niveau bas.
|
||||
|
||||
|
|
@ -66,9 +68,9 @@ Rejouée le 2026-10-17 (#456) sur les PR mergées de #344 à #452 : les 39 PR de
|
|||
| 39 PR de référence, ces motifs | 3 | 28 | 8 (21 %, pour une cible de 20 %) |
|
||||
| 51 PR de la plage, ces motifs | 6 | 33 | 12 (24 %) |
|
||||
|
||||
Niveau haut sur les 51 : licence (#359, #364, #366, #368, #386, #387, #388), migrations et profils (#394, #436), release (#414, #416, #418) ; parmi les 39 de référence, #386, #387, #388, #394, #414, #416, #418 et #436. L'écart à la cible est justifié dans la PR qui ferme #456, qui consigne aussi le niveau de chaque PR et le script qui les calcule.
|
||||
Niveau haut sur les 51 : licence (#359, #364, #366, #368, #386, #387, #388), migrations et profils (#394, #436), release (#414, #416, #418) ; parmi les 39 de référence, #386, #387, #388, #394, #414, #416, #418 et #436. L'écart à la cible est justifié dans la PR qui ferme #456, qui consigne aussi le niveau de chaque PR. Le script qui les calcule est `scripts/review-levels.py` (API Forgejo, lecture seule) ; `python3 scripts/review-levels.py --check` vérifie, sans réseau, que ses motifs sont ceux du tableau Haut.
|
||||
|
||||
Une PR qui modifie cette section dit, pour chaque motif retiré ou assoupli, ce qu'il protégeait, et rejoue ce script sur les mêmes PR ; le pilote #458 mesure si les niveaux tiennent.
|
||||
Une PR qui modifie cette section dit, pour chaque motif retiré ou assoupli, ce qu'il protégeait, met `scripts/review-levels.py` à jour dans le même diff et le rejoue sur les mêmes PR ; le pilote #458 mesure si les niveaux tiennent.
|
||||
|
||||
## Ce qui compte par zone
|
||||
|
||||
|
|
@ -78,7 +80,7 @@ Chaque règle cite sa source. Elles valent quel que soit le niveau : une PR norm
|
|||
|
||||
- Aucune donnée financière ne quitte le poste : ni télémétrie, ni analytics, et le CSV comme la base restent locaux. Un appel réseau ajouté se justifie dans la PR : il n'envoie rien de financier ni aucun identifiant brut, et il passe derrière un consentement ou l'édition. — `CLAUDE.md` « Privacy-first — NON NÉGOCIABLE », [ADR 0009](docs/adr/0009-proxy-price-fetching-via-maximus-api.md), [ADR 0020](docs/adr/0020-identifiant-machine-derive.md)
|
||||
- Les appels sortants partent tous du Rust (`reqwest`) : `auth_commands.rs` (compte), `license_commands.rs` (licence et activation), `balance_commands.rs` (`fetch_price`, qui porte le jeton d'activation) et `feedback_commands.rs`, plus l'endpoint du plugin d'updater. Le `connect-src` de `app.security.csp` (`api.`, `auth.` et `feedback.lacompagniemaximus.com`) ne borne que la webview : lire chaque URL, chaque en-tête et chaque corps envoyé. — `CLAUDE.md` « Privacy-first — NON NÉGOCIABLE », `src-tauri/tauri.conf.json`, `src-tauri/src/commands/`
|
||||
- Les cours passent par `maximus-api`, après un consentement explicite par profil, avec les seuls en-têtes `Authorization`, `Accept` et `User-Agent` : le fournisseur tiers ne voit jamais le poste. — [ADR 0009](docs/adr/0009-proxy-price-fetching-via-maximus-api.md)
|
||||
- Les cours passent par `maximus-api`, après un consentement explicite par profil, avec les seuls en-têtes `Authorization`, `Accept` et `User-Agent` : le fournisseur tiers ne voit jamais le poste. Ce consentement n'est vérifié que côté front (`PriceFetchControl.tsx`, `PriceFetchConsentToggle.tsx`) : `fetch_price` ne le revérifie pas. — [ADR 0009](docs/adr/0009-proxy-price-fetching-via-maximus-api.md), `src-tauri/src/commands/balance_commands.rs`
|
||||
- Seule l'activation envoie un identifiant de la machine, et seulement le dérivé (`HMAC-SHA256` de l'identifiant du système) ; un jeton émis avant la dérivation porte encore l'identifiant brut jusqu'à la réactivation. — [ADR 0020](docs/adr/0020-identifiant-machine-derive.md), § 2 et 5
|
||||
- Le Feedback Hub reçoit le texte saisi, suivi au choix des dernières erreurs du Journal, un `userId` facultatif et un contexte borné par `FeedbackContext` (page, langue, thème, fenêtre, user-agent, horodatage) : un champ ajouté au contexte élargit ce qui part. — `CLAUDE.md` « Privacy-first — NON NÉGOCIABLE », `src/services/feedbackService.ts`, `src/components/settings/FeedbackDialog.tsx`
|
||||
- La webview n'a que les permissions de `src-tauri/capabilities/default.json` : l'accès aux fichiers passe par les commandes Rust, jamais par une permission `fs`, et une permission ajoutée se justifie. — `CLAUDE.md` « Règles d'architecture »
|
||||
|
|
@ -96,7 +98,8 @@ Chaque règle cite sa source. Elles valent quel que soit le niveau : une PR norm
|
|||
- Une migration atteint deux chemins et reste correcte sur les deux : le profil par défaut, migré par tauri-plugin-sql (sqlx), et chaque profil secondaire, rattrapé à son ouverture par `prepare_profile_db` (rusqlite, clés étrangères actives). — [ADR 0021](docs/adr/0021-migrations-profils-secondaires.md), `.claude/rules/sql-migrations.md`
|
||||
- Tout changement de schéma et toute correction de données (seed, mots-clés, catégories) se reportent dans `consolidated_schema.sql` : un profil neuf naît avec l'historique complet et ne rejoue aucune migration. Aucun marqueur ajouté à `infer_floor`, réservé aux bases créées avant #390. — [ADR 0021](docs/adr/0021-migrations-profils-secondaires.md), `.claude/rules/sql-migrations.md`
|
||||
- Les comptes de tables, d'index et de migrations de `CLAUDE.md` et de `docs/architecture.md` suivent. — `CLAUDE.md` « Base de données » et « Documentation technique »
|
||||
- Les bases vivent sous `app_config_dir()`, qui n'est pas `app_data_dir()` sous Linux. Ce dossier dérive de l'`identifier` de `tauri.conf.json`, auquel s'aligne aussi le service du trousseau : changer l'`identifier` orpheline les profils et les jetons existants. — [ADR 0021](docs/adr/0021-migrations-profils-secondaires.md), [ADR 0006](docs/adr/0006-oauth-tokens-keychain.md), `CLAUDE.md` « Base de données »
|
||||
- Les bases vivent sous `app_config_dir()`, qui n'est pas `app_data_dir()` sous Linux. Les deux dossiers dérivent de l'`identifier` de `tauri.conf.json` : le changer orpheline les bases des profils et, sous `app_data_dir()`, la clé de licence, le jeton d'activation et le fichier de repli des jetons OAuth. Le service du trousseau (`KEYCHAIN_SERVICE` de `token_store.rs`) est une constante alignée sur l'`identifier` par convention : il ne suit pas. — [ADR 0021](docs/adr/0021-migrations-profils-secondaires.md), [ADR 0006](docs/adr/0006-oauth-tokens-keychain.md), `CLAUDE.md` « Base de données », `src-tauri/src/commands/license_commands.rs`
|
||||
- Toute requête passe par le verrou unique de `db.ts`, et une transaction par `withTransaction` et son handle ; un changement de profil ferme puis rouvre la connexion. La restauration d'une sauvegarde (`runRestore` de `dataExportService.ts`) efface puis réinsère dans une seule transaction et ne notifie qu'après le `COMMIT`, une fois le verrou relâché. — [ADR 0022](docs/adr/0022-canal-evenements-donnees.md), [ADR 0005](docs/adr/0005-multi-profile-db.md), `docs/architecture.md` « Accès DB et transactions »
|
||||
- Un booléen se lie en `1`/`0` (`toSqlFlag`), jamais en `true`/`false`. — [ADR 0021](docs/adr/0021-migrations-profils-secondaires.md)
|
||||
- Une base par profil, PIN haché par Argon2. — [ADR 0005](docs/adr/0005-multi-profile-db.md)
|
||||
- Une nouvelle écriture d'une table que lit `COUNTERS_SQL` appelle `notifyDataChanged()` après succès, avec un test qui prouve la notification. — [ADR 0022](docs/adr/0022-canal-evenements-donnees.md)
|
||||
|
|
@ -114,13 +117,13 @@ Chaque règle cite sa source. Elles valent quel que soit le niveau : une PR norm
|
|||
### Dépendances
|
||||
|
||||
- `.cargo/audit.toml` n'admet une advisory que si son crate est absent de toutes les cibles livrées (`cargo tree -i <crate> --target <triple>`, Linux et Windows, jamais `--target all`) ou si aucun correctif n'existe. Une entrée porte un ID, jamais un crate ; elle ajoute son crate à la liste du garde de `check-rust.yml` et a son issue de suivi, qui porte la condition de retrait. — [ADR 0018](docs/adr/0018-suppression-advisories-non-atteignables.md), en-tête de `.cargo/audit.toml`, `CLAUDE.md` « CI/CD »
|
||||
- Garde `windows-sys` : s'il rougit après un `cargo update`, remettre les arêtes à la main selon l'ADR ; ne jamais modifier la liste gardée, la version épinglée ni le canari pour le faire passer. Un `cargo update` ciblé peut déplacer d'autres arêtes sans le dire : chaque entrée qui bouge dans le lock doit s'expliquer. — [ADR 0023](docs/adr/0023-windows-sys-pinning.md), `CLAUDE.md` « CI/CD »
|
||||
- Garde `windows-sys` : s'il rougit après un `cargo update`, remettre les arêtes à la main selon l'ADR ; ne jamais modifier la liste gardée (`CRATES`), la version épinglée (`PIN`), l'ensemble attendu (`EXPECTED`) ni le canari pour le faire passer. Un `cargo update` ciblé peut déplacer d'autres arêtes sans le dire : chaque entrée qui bouge dans le lock doit s'expliquer. — [ADR 0023](docs/adr/0023-windows-sys-pinning.md), `CLAUDE.md` « CI/CD »
|
||||
- Une version montée exécute son nouveau code au build de la release, clé de signature dans l'environnement : scripts de build et macros des crates, code des paquets npm que lancent `npm run build` et `npx tauri build`. Lire ce que change la version, pas seulement son numéro. — `CLAUDE.md` « CI/CD », `docs/architecture.md` « `release.yml` — Build et publication »
|
||||
- Une advisory corrigée se prouve par `cargo audit` lancé depuis la racine, là où `.cargo/audit.toml` s'applique. `npm audit` n'est pas un gate de CI. — `CLAUDE.md` « CI/CD »
|
||||
|
||||
### Release et mise à jour
|
||||
|
||||
- `release.yml` met la clé de signature de l'updater (`TAURI_SIGNING_PRIVATE_KEY`) dans l'environnement des deux étapes `npx tauri build`. S'exécutent avec elle : `beforeBuildCommand` (`npm run build`, donc `tsc`, `vite.config.ts` et les `scripts` de `package.json`), `@tauri-apps/cli` et son binaire natif, qui lit la clé, `src-tauri/build.rs`, les scripts de build et les macros des crates, et tout module JS qu'une directive CSS `@plugin` ou `@config` fait charger. `npm ci` tourne avant, sans elle, mais ce qu'il installe s'exécute ensuite avec elle. — `CLAUDE.md` « CI/CD », `docs/architecture.md` « `release.yml` — Build et publication »
|
||||
- `release.yml` met la clé de signature de l'updater (`TAURI_SIGNING_PRIVATE_KEY`) dans l'environnement des deux étapes `npx tauri build`. S'exécutent avec elle : `beforeBuildCommand` (`npm run build`, donc `tsc`, `vite.config.ts` et les `scripts` de `package.json`), `@tauri-apps/cli` et son binaire natif, qui lit la clé, `cargo-xwin` (`--runner cargo-xwin` pour Windows) et la toolchain Rust, installés à chaque tag sans version épinglée, `src-tauri/build.rs`, les scripts de build et les macros des crates, et tout module JS qu'une directive CSS `@plugin` ou `@config` fait charger. `npm ci` tourne avant, sans elle, mais ce qu'il installe s'exécute ensuite avec elle. — `CLAUDE.md` « CI/CD », `docs/architecture.md` « `release.yml` — Build et publication »
|
||||
- `latest.json` porte une clé par paquet (`linux-x86_64-deb`, `linux-x86_64-rpm`, `windows-x86_64`), contrôlée par `scripts/check-latest-json.jq` avant le `DELETE` du registre ; `scripts/build-latest-json.test.mjs` tourne dans `npm test`. — `CLAUDE.md` « CI/CD », `docs/architecture.md` « `release.yml` — Build et publication »
|
||||
- `release.yml` signe les binaires et publie le JSON d'updater que lisent toutes les installations : une clé publique de `plugins.updater` changée sans bascule prévue coupe leurs mises à jour, comme un endpoint déplacé. — `CLAUDE.md` « CI/CD » et « Fonctionnalités principales »
|
||||
- Une release qui touche l'updater, ses dépendances, `release.yml` ou la config `updater` exige un cycle réel de mise à jour (étape 10 de `/release`, `docs/qa-update-cycle.md`) : la PR le signale pour la release suivante. `verify-release.sh` contrôle la release publiée. — `CLAUDE.md` « CI/CD », `.claude/skills/release/SKILL.md`
|
||||
|
|
@ -131,7 +134,7 @@ Chaque règle cite sa source. Elles valent quel que soit le niveau : une PR norm
|
|||
- `audit.yml` est le seul canal des avis RustSec : quotidien et bloquant. L'étape `cargo audit` de `check-rust` est non bloquante par choix. — `CLAUDE.md` « CI/CD »
|
||||
- Une garde affiche chacune de ses vérifications, y compris celles qui passent, et lit un canari : une garde muette ne prouve rien. — [ADR 0018](docs/adr/0018-suppression-advisories-non-atteignables.md) (règle 3), [ADR 0023](docs/adr/0023-windows-sys-pinning.md)
|
||||
- Une PR qui ne touche que `audit.yml` ou `release.yml` n'a aucune CI : les `paths` de `check-rust.yml` ne les listent pas, et le `paths-ignore` de `check-frontend.yml` les exclut. La revue rejoue elle-même ce qui peut l'être. — `CLAUDE.md` « CI/CD », `check-rust.yml`, `check-frontend.yml`
|
||||
- Les secrets du dépôt (clé de signature et son mot de passe, jeton du registre de paquets qui remplace `latest.json`) sont lisibles par tout workflow : un workflow ajouté, ou une référence à `secrets` ajoutée, se lit comme du code de release. — `CLAUDE.md` « CI/CD », `release.yml`
|
||||
- Les secrets du dépôt (clé de signature et son mot de passe, jeton du registre de paquets qui remplace `latest.json`) sont lisibles par tout workflow : un workflow ajouté, une référence à `secrets` ajoutée ou un déclencheur `pull_request_target` se lit comme du code de release. — `CLAUDE.md` « CI/CD », `release.yml`
|
||||
|
||||
### i18n et documentation
|
||||
|
||||
|
|
@ -174,7 +177,7 @@ cargo audit --file src-tauri/Cargo.lock # depuis la racine : .cargo/audit.toml
|
|||
|
||||
Les trois premiers viennent du skill global `pr-review`, corrigés pour ce dépôt.
|
||||
|
||||
- **CI d'une pile.** Chaque maillon a sa CI depuis #232 : la section « CI coverage gap » du skill (CI réservée aux PR qui ciblent `main`) ne vaut pas ici. `check-rust` ne tourne que si le diff du maillon touche ses chemins : l'état Rust d'une pile est prouvé par le dernier maillon qui en a touché. — `CLAUDE.md` « CI/CD »
|
||||
- **CI d'une pile.** Chaque maillon a sa CI depuis #232 : la section « CI coverage gap » du skill (CI réservée aux PR qui ciblent `main`) ne vaut pas ici. `check-rust` ne tourne que si le diff du maillon touche ses chemins : l'état Rust d'une pile est prouvé par le dernier maillon qui en a touché. Exception : `fs_commands.rs` compile par `include_str!` les deux CSV de `src/__fixtures__/samples/`, hors de ces chemins ; une PR qui les renomme, les déplace ou les supprime casse `cargo check` sans que `check-rust` tourne. — `CLAUDE.md` « CI/CD », `src-tauri/src/commands/fs_commands.rs`
|
||||
- **Un filtre `paths:` n'est pas prouvé par une PR qui touche le chemin exact.** Le glob (`src-tauri/**`) l'est par une PR qui touche un autre fichier du dossier, comme la PR #344 pour `check-rust`. — skill `pr-review` (section CI), #232
|
||||
- **CI Rust et revue en fork.** Un fork qui attend la CI pour poster son verdict peut s'arrêter sans rien publier (PR #303) : poster les constats, surveiller le statut depuis la session principale, relancer la revue au vert. `check-rust` prend environ 3 min 30 avec le cache et 10 min à froid, sans compter la file du runner. — skill `pr-review` (délégation), #234
|
||||
- **Un vert de `check-rust` ne dit rien de l'audit** : son étape `cargo audit` est `continue-on-error`. Lire le journal : `cargo-audit` écrit `1 vulnerability found` mais `N vulnerabilities found` (chercher `vulnerabilit(y|ies) found`), et le journal de `check-rust` contient des codes ANSI à retirer avant tout `grep`. Un `audit.yml` vert veut dire « zéro advisory hors de la liste acceptée », pas « zéro advisory ». — `CLAUDE.md` « CI/CD », [ADR 0018](docs/adr/0018-suppression-advisories-non-atteignables.md), #447
|
||||
|
|
|
|||
393
scripts/review-levels.py
Executable file
393
scripts/review-levels.py
Executable file
|
|
@ -0,0 +1,393 @@
|
|||
#!/usr/bin/env python3
|
||||
"""Replay the review levels of REVIEW.md on merged pull requests (#456).
|
||||
|
||||
Usage:
|
||||
python3 scripts/review-levels.py [FIRST LAST] (default: 344 452)
|
||||
python3 scripts/review-levels.py --check (no network)
|
||||
|
||||
The first form reads the Forgejo API (token in $FORGEJO_TOKEN, else
|
||||
~/.forgejo-token; read-only calls) and prints each merged PR of the range
|
||||
with its trial level (pilot document, section 4.3) and its REVIEW.md level,
|
||||
then the distributions for the 39 reference PRs and for the whole range.
|
||||
--check compares the patterns of this script with the first column of the
|
||||
Haut table of REVIEW.md and exits 1 if they differ: a PR that changes the
|
||||
Niveaux section updates this script in the same diff.
|
||||
|
||||
The content conditions of the Haut table are approximated from the diff:
|
||||
lib.rs (only generate_handler![] entries are exempt), Tauri configuration
|
||||
and manifests (only the app's version is exempt), lock files (a package
|
||||
added or removed, an origin changed), check workflows (a change that can
|
||||
turn a failing check green, read against the PR's merge base), network
|
||||
calls (reqwest in Rust, fetch to an absolute URL in TS), consent files.
|
||||
"""
|
||||
import itertools
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
import sys
|
||||
import urllib.request
|
||||
|
||||
API = "https://git.lacompagniemaximus.com/api/v1/repos/maximus/simpl-resultat"
|
||||
REVIEW_MD = os.path.join(os.path.dirname(os.path.abspath(__file__)), "..", "REVIEW.md")
|
||||
|
||||
# The 39 reference PRs of the pilot document, frozen by number: the listing
|
||||
# that produced them (closed PRs, limit=40, sort=recentupdate) is not stable.
|
||||
REF39 = [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]
|
||||
|
||||
TRIAL_HIGH = [
|
||||
"src-tauri/src/commands/{auth_commands,token_store,license_commands,entitlements,account_cache,backup_commands,fs_commands,export_import_commands}.rs",
|
||||
"src-tauri/src/lib.rs", "src-tauri/src/database/**", "src-tauri/tauri.conf.json",
|
||||
"src-tauri/capabilities/**", ".forgejo/workflows/**", ".cargo/**", "src-tauri/Cargo.{toml,lock}",
|
||||
"package.json", "package-lock.json",
|
||||
"src/services/{authService,licenseService,db,feedbackService,dataExportService}.ts",
|
||||
"src/services/entitlements*", "scripts/build-latest-json*",
|
||||
]
|
||||
TRIAL_LOW = ["**/*.md", "docs/**", "reports/**", "tasks/**", "public/CHANGELOG*", "LICENSE"]
|
||||
|
||||
# REVIEW.md, table Haut: rows without a condition.
|
||||
HIGH_ALWAYS = [
|
||||
"src-tauri/src/database/**",
|
||||
"src-tauri/src/commands/profile_commands.rs",
|
||||
"src-tauri/src/commands/{license_commands,entitlements,auth_commands,token_store,account_cache}.rs",
|
||||
"src/services/{licenseService,authService}.ts",
|
||||
".claude/skills/issue-license/**",
|
||||
"src-tauri/src/commands/export_import_commands.rs",
|
||||
"src-tauri/src/commands/feedback_commands.rs", "src/services/feedbackService.ts",
|
||||
"src-tauri/capabilities/**",
|
||||
".forgejo/workflows/release.yml", "scripts/build-latest-json*", "scripts/check-latest-json.jq",
|
||||
".claude/skills/release/**",
|
||||
"src-tauri/build.rs", "{vite,vitest,postcss}.config.*", ".postcssrc*", ".npmrc", "**/rust-toolchain*",
|
||||
"Cargo.{toml,lock}",
|
||||
"**/.cargo/**",
|
||||
".claude/settings.json", ".mcp.json",
|
||||
]
|
||||
CONSENT_FILES = {"src/components/balance/PriceFetchControl.tsx",
|
||||
"src/components/settings/PriceFetchConsentToggle.tsx"}
|
||||
TAURI_CONFIG = ["src-tauri/{tauri*.conf.json*,Tauri*.toml}"]
|
||||
CHECK_WORKFLOWS = {".forgejo/workflows/check-rust.yml", ".forgejo/workflows/check-frontend.yml"}
|
||||
SHIPPED = ["src/**", "src-tauri/src/**", "index.html", "public/**"]
|
||||
LOW = ["**/*.md", "docs/**", "LICENSE", "tasks/**", "reports/**"]
|
||||
NEVER_LOW = ["CLAUDE.md", "REVIEW.md", "docs/adr/**", "docs/architecture.md", "docs/qa-update-cycle.md", ".claude/**"]
|
||||
|
||||
|
||||
def token():
|
||||
return os.environ.get("FORGEJO_TOKEN") or open(os.path.expanduser("~/.forgejo-token")).read().strip()
|
||||
|
||||
|
||||
def get(path, raw=False):
|
||||
req = urllib.request.Request(API + path, headers={"Authorization": "token " + token()})
|
||||
data = urllib.request.urlopen(req).read()
|
||||
return data if raw else json.loads(data)
|
||||
|
||||
|
||||
def expand_braces(pat):
|
||||
m = re.search(r"\{([^{}]*)\}", pat)
|
||||
if not m:
|
||||
return [pat]
|
||||
return list(itertools.chain.from_iterable(
|
||||
expand_braces(pat[:m.start()] + alt + pat[m.end():]) for alt in m.group(1).split(",")))
|
||||
|
||||
|
||||
def glob_re(pat):
|
||||
# '**/' spans zero or more directories, '*' and '?' never cross '/'
|
||||
i, out = 0, ""
|
||||
while i < len(pat):
|
||||
if pat.startswith("**/", i):
|
||||
out += "(?:.*/)?"; i += 3; continue
|
||||
if pat.startswith("**", i):
|
||||
out += ".*"; i += 2; continue
|
||||
c = pat[i]
|
||||
out += "[^/]*" if c == "*" else "[^/]" if c == "?" else re.escape(c)
|
||||
i += 1
|
||||
return re.compile("^" + out + "$")
|
||||
|
||||
|
||||
def match(pats, path):
|
||||
# '{a,b}' alternates, wildcards allowed inside
|
||||
return any(glob_re(p).match(path) for pat in pats for p in expand_braces(pat))
|
||||
|
||||
|
||||
def file_blocks(diff):
|
||||
out = {}
|
||||
for block in re.split(r"(?m)^diff --git ", diff)[1:]:
|
||||
out[block.split("\n", 1)[0].split(" b/")[-1]] = block
|
||||
return out
|
||||
|
||||
|
||||
def meaningful(text):
|
||||
t = text.strip()
|
||||
return t != "" and not t.startswith(("#", "//"))
|
||||
|
||||
|
||||
def changed(block):
|
||||
return [l for l in block.split("\n")
|
||||
if l[:1] in "+-" and not l.startswith(("+++", "---")) and meaningful(l[1:])]
|
||||
|
||||
|
||||
def lock_adds_or_removes(name, block):
|
||||
cargo = name.endswith("Cargo.lock")
|
||||
lines = block.split("\n")
|
||||
if cargo and any(re.match(r'^[+-]name = ', l) for l in lines):
|
||||
return True
|
||||
if not cargo and any(re.match(r'^[+-]\s+"node_modules/[^"]+": \{', l) for l in lines):
|
||||
return True
|
||||
source = r'^([+-])source = "([^"]+)"' if cargo else r'^([+-])\s+"resolved": "([^"]+)"'
|
||||
digest = r'^\+(checksum) = ' if cargo else r'^\+\s+"(integrity)": '
|
||||
version = r'^\+version = ' if cargo else r'^\+\s+"version": '
|
||||
for hunk in re.split(r"(?m)^@@", block)[1:]:
|
||||
hl = hunk.split("\n")
|
||||
# an origin whose host or scheme changes, with or without a new version
|
||||
before, after = set(), set()
|
||||
for l in hl:
|
||||
m = re.match(source, l)
|
||||
if m:
|
||||
(before if m.group(1) == "-" else after).add(origin_of(m.group(2)))
|
||||
if after - before:
|
||||
return True
|
||||
# a checksum or integrity that changes without a new version
|
||||
if any(re.match(digest, l) for l in hl) and not any(re.match(version, l) for l in hl):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def origin_of(url):
|
||||
m = re.match(r"^([a-z+]+)://([^/]+)", url)
|
||||
return m.group(1) + "://" + m.group(2) if m else url
|
||||
|
||||
|
||||
STEP = re.compile(r"^\s*- (name|uses):")
|
||||
VERDICT = re.compile(r"npm ci|npm run build|npm test|cargo check|cargo test|cargo audit|cargo-audit|Verify", re.I)
|
||||
|
||||
|
||||
def regions(base):
|
||||
lines = base.split("\n")
|
||||
region, top, job_key = {}, None, None
|
||||
for i, l in enumerate(lines, 1):
|
||||
m = re.match(r"^([A-Za-z_][\w-]*):", l)
|
||||
if m:
|
||||
top = m.group(1)
|
||||
m = re.match(r"^ ([A-Za-z_][\w-]*):", l)
|
||||
if m:
|
||||
job_key = m.group(1)
|
||||
elif re.match(r"^\S", l) or re.match(r"^ \S", l):
|
||||
job_key = None
|
||||
if top in ("on", "true"):
|
||||
region[i] = "on"
|
||||
elif top == "permissions" or job_key in ("env", "container", "permissions"):
|
||||
region[i] = "jobcfg"
|
||||
else:
|
||||
region[i] = "other"
|
||||
starts = [i for i, l in enumerate(lines, 1) if STEP.match(l)]
|
||||
for k, s in enumerate(starts):
|
||||
e = starts[k + 1] - 1 if k + 1 < len(starts) else len(lines)
|
||||
if VERDICT.search("\n".join(lines[s - 1:e])):
|
||||
for i in range(s, e + 1):
|
||||
if region[i] == "other":
|
||||
region[i] = "verdict"
|
||||
return region, lines
|
||||
|
||||
|
||||
def weakening(block, base):
|
||||
"""Ways a check-workflow diff could turn a failing check green (empty = none)."""
|
||||
region, lines = regions(base)
|
||||
reasons, old, plus = set(), 0, []
|
||||
|
||||
def flush(pos):
|
||||
texts = [p[1:] for p in plus]
|
||||
active = [t for t in texts if meaningful(t)] # comments never count
|
||||
if any(re.search(r"continue-on-error|\|\|\s*true", t) for t in active):
|
||||
reasons.add("continue-on-error or || true added")
|
||||
if any(re.search(r"GITHUB_ENV|GITHUB_PATH|npm pkg set|git checkout|git restore", t) for t in active):
|
||||
reasons.add("writes the job environment or the repository")
|
||||
if any(re.search(r"^\s*(- )?uses:", t) and not re.search(r"Swatinem/rust-cache|actions/cache", t) for t in active):
|
||||
reasons.add("action added (not a cache)")
|
||||
if any(re.search(r"rustup|toolchain|\bPATH\b|/bin/", t) for t in active):
|
||||
reasons.add("install step changes the toolchain or the PATH")
|
||||
if any(re.match(r"^(\s{4})?(if|defaults):", t) for t in active):
|
||||
reasons.add("job-level if/defaults changed")
|
||||
first = next((t for t in texts if meaningful(t)), None)
|
||||
if first is None or STEP.match(first):
|
||||
return
|
||||
prev, nxt = region.get(pos - 1, "other"), region.get(pos, "other")
|
||||
if "on" in (prev, nxt):
|
||||
if any(re.search(r"branches|paths-ignore", t) for t in active):
|
||||
reasons.add("trigger narrowed")
|
||||
j = pos - 1
|
||||
while j > 0 and not re.match(r"^\s*[\w-]+:", lines[j - 1]):
|
||||
j -= 1
|
||||
if j > 0 and "paths-ignore" in lines[j - 1] and any(re.match(r"^\s*- ", t) for t in active):
|
||||
reasons.add("paths-ignore entry added")
|
||||
elif prev == nxt == "verdict":
|
||||
reasons.add("verdict step modified")
|
||||
elif "jobcfg" in (prev, nxt):
|
||||
reasons.add("job env/container/permissions modified")
|
||||
|
||||
for line in block.split("\n"):
|
||||
m = re.match(r"^@@ -(\d+)(?:,\d+)? \+\d+(?:,\d+)? @@", line)
|
||||
if m:
|
||||
flush(old); plus = []
|
||||
old = int(m.group(1)); continue
|
||||
if line.startswith("+") and not line.startswith("+++"):
|
||||
plus.append(line); continue
|
||||
flush(old); plus = []
|
||||
if line.startswith("-") and not line.startswith("---"):
|
||||
if meaningful(line[1:]):
|
||||
if re.match(r"^(\s{4})?(if|defaults):", line[1:]):
|
||||
reasons.add("job-level if/defaults changed")
|
||||
if re.search(r"rustup|toolchain|\bPATH\b|/bin/", line[1:]):
|
||||
reasons.add("install step changes the toolchain or the PATH")
|
||||
r = region.get(old, "other")
|
||||
if r == "on":
|
||||
reasons.add("trigger removed or changed")
|
||||
elif r == "verdict":
|
||||
reasons.add("verdict step modified or removed")
|
||||
elif r == "jobcfg":
|
||||
reasons.add("job env/container/permissions modified")
|
||||
old += 1
|
||||
elif line.startswith(" "):
|
||||
old += 1
|
||||
flush(old)
|
||||
return sorted(reasons)
|
||||
|
||||
|
||||
def adds_network_call(path, block):
|
||||
if not match(SHIPPED, path) or re.search(r"test", path):
|
||||
return False
|
||||
for l in block.split("\n"):
|
||||
if l.startswith("+") and not l.startswith("+++"):
|
||||
if path.endswith(".rs") and "reqwest" in l:
|
||||
return True
|
||||
if re.search(r"\.(ts|tsx)$", path) and re.search(r"fetch\(\s*[`'\"]https?://", l):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def trial_level(paths):
|
||||
if any(match(TRIAL_HIGH, p) for p in paths):
|
||||
return "haut"
|
||||
return "bas" if all(match(TRIAL_LOW, p) for p in paths) else "normal"
|
||||
|
||||
|
||||
def review_level(number, files, diff, merge_base):
|
||||
blocks = file_blocks(diff)
|
||||
triggers = []
|
||||
for f in files:
|
||||
b = blocks.get(f["filename"], "")
|
||||
for path in filter(None, {f["filename"], f.get("previous_filename")}):
|
||||
if match(HIGH_ALWAYS, path):
|
||||
triggers.append(path)
|
||||
elif path == "src-tauri/src/lib.rs":
|
||||
if [l for l in changed(b) if not re.match(r"^[+-]\s*commands::\w+,?\s*$", l)]:
|
||||
triggers.append(path)
|
||||
elif path == "src-tauri/src/commands/balance_commands.rs":
|
||||
hunks = re.split(r"(?m)^(?=@@)", b)[1:]
|
||||
if any(re.search(r"fetch_price|activation", h.split("\n")[0]) or
|
||||
any(re.search(r"fetch_price|activation", l) for l in changed(h)) for h in hunks):
|
||||
triggers.append(path)
|
||||
elif match(TAURI_CONFIG, path) or path in ("src-tauri/Cargo.toml", "package.json"):
|
||||
if [l for l in changed(b) if not re.match(r'^[+-]\s*"?version"?\s*[:=]', l)]:
|
||||
triggers.append(path)
|
||||
elif path in ("src-tauri/Cargo.lock", "package-lock.json"):
|
||||
if lock_adds_or_removes(path, b):
|
||||
triggers.append(path)
|
||||
elif path == ".forgejo/workflows/audit.yml" and changed(b):
|
||||
triggers.append(path)
|
||||
elif path in CHECK_WORKFLOWS:
|
||||
wf = path.split("/")[-1]
|
||||
base = get(f"/raw/.forgejo/workflows/{wf}?ref={merge_base}", raw=True).decode()
|
||||
if weakening(b, base):
|
||||
triggers.append(path)
|
||||
elif path.startswith(".forgejo/workflows/") and f["status"] == "added":
|
||||
triggers.append(path + " (new workflow)")
|
||||
elif path == "REVIEW.md":
|
||||
triggers.append(path + " (check the « Niveaux » section by hand)")
|
||||
if adds_network_call(path, b):
|
||||
triggers.append(path + " (network call)")
|
||||
added = [l for l in b.split("\n") if l.startswith("+") and not l.startswith("+++")]
|
||||
if path.startswith(".forgejo/workflows/") and any(re.search(r"\bsecrets\b|pull_request_target", l) for l in added):
|
||||
triggers.append(path + " (secrets or pull_request_target added)")
|
||||
if path in CONSENT_FILES and any(re.search(r"consent", l, re.I) for l in changed(b)):
|
||||
triggers.append(path + " (consent)")
|
||||
parts = path.split("/")
|
||||
top_level = len(parts) == 1 or (parts[0] == "src-tauri" and len(parts) == 2)
|
||||
new_hidden = f["status"] == "added" and any(
|
||||
x.startswith(".") for x in parts[:-1]) and parts[0] not in (".forgejo", ".claude", ".cargo", ".github")
|
||||
# Forgejo reports an edited file as "changed"
|
||||
if (f["status"] not in ("changed", "modified") and top_level and not path.endswith(".md")) or new_hidden:
|
||||
triggers.append(path + " (" + f["status"] + " at top level)")
|
||||
paths = [f["filename"] for f in files]
|
||||
if triggers:
|
||||
return "haut", sorted(set(triggers))
|
||||
if all(match(LOW, p) and not match(NEVER_LOW, p) for p in paths):
|
||||
return "bas", []
|
||||
return "normal", []
|
||||
|
||||
|
||||
CONDITIONAL = {
|
||||
"src-tauri/src/lib.rs", "src-tauri/src/commands/balance_commands.rs",
|
||||
"src-tauri/Cargo.toml", "package.json", "src-tauri/Cargo.lock", "package-lock.json",
|
||||
".forgejo/workflows/**", ".forgejo/workflows/audit.yml",
|
||||
".forgejo/workflows/{check-rust,check-frontend}.yml", "REVIEW.md",
|
||||
}
|
||||
|
||||
|
||||
def check():
|
||||
"""Compare this script's patterns with the first column of the Haut table."""
|
||||
text = open(REVIEW_MD, encoding="utf-8").read()
|
||||
table = text.split("### Haut", 1)[1].split("\n\n", 2)[1]
|
||||
documented = set()
|
||||
for row in table.split("\n"):
|
||||
if not row.startswith("| ") or row.startswith("| Motif") or row.startswith("| fichier à la racine"):
|
||||
continue
|
||||
documented.update(re.findall(r"`([^`]+)`", row.split(" | ")[0]))
|
||||
coded = set(HIGH_ALWAYS) | set(TAURI_CONFIG) | set(SHIPPED) | CONDITIONAL
|
||||
for label, extra in (("REVIEW.md only", documented - coded), ("script only", coded - documented)):
|
||||
for pattern in sorted(extra):
|
||||
print(f"{label}: {pattern}")
|
||||
if documented != coded:
|
||||
return 1
|
||||
print(f"ok: {len(coded)} patterns, identical to the Haut table of REVIEW.md")
|
||||
return 0
|
||||
|
||||
|
||||
def main():
|
||||
if sys.argv[1:] == ["--check"]:
|
||||
sys.exit(check())
|
||||
first, last = (int(sys.argv[1]), int(sys.argv[2])) if len(sys.argv) == 3 else (344, 452)
|
||||
merged, page = [], 1
|
||||
while True:
|
||||
batch = get(f"/pulls?state=closed&limit=50&page={page}")
|
||||
if not batch:
|
||||
break
|
||||
merged += [p for p in batch if p.get("merged") and first <= p["number"] <= last]
|
||||
page += 1
|
||||
merged.sort(key=lambda p: p["number"])
|
||||
rows = []
|
||||
for p in merged:
|
||||
n = p["number"]
|
||||
files, fpage = [], 1
|
||||
while True:
|
||||
# the instance caps a page at 50 items: read until an empty page
|
||||
fb = get(f"/pulls/{n}/files?limit=50&page={fpage}")
|
||||
if not fb:
|
||||
break
|
||||
files += fb
|
||||
fpage += 1
|
||||
diff = get(f"/pulls/{n}.diff", raw=True).decode("utf-8", "replace")
|
||||
merge_base = get(f"/pulls/{n}")["merge_base"]
|
||||
trial = trial_level([f["filename"] for f in files])
|
||||
level, triggers = review_level(n, files, diff, merge_base)
|
||||
rows.append((n, trial, level, triggers))
|
||||
print(f"#{n}\t{'ref' if n in REF39 else ' '}\t{trial}\t{level}\t{'; '.join(triggers)}")
|
||||
for label, keep in (("39 reference PRs", lambda n: n in REF39), (f"{len(rows)} PRs #{first}-#{last}", lambda n: True)):
|
||||
sel = [r for r in rows if keep(r[0])]
|
||||
for col, name in ((1, "trial"), (2, "REVIEW.md")):
|
||||
counts = {lvl: sum(1 for r in sel if r[col] == lvl) for lvl in ("bas", "normal", "haut")}
|
||||
share = 100 * counts["haut"] / len(sel)
|
||||
print(f"{label}, {name}: {counts} -> haut {share:.1f} %")
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
Loading…
Reference in a new issue