# REVIEW.md — VPS Health API

Règles de revue des PR de ce dépôt : niveau de risque, ce qui compte dans chaque zone, commandes de vérification et pièges connus. Ce fichier complète `CLAUDE.md` sans le remplacer : une zone en reprend ce qui sert à la revue, avec sa source.

À partir de #63, `/pr-review` lit ce fichier sur `main`, y compris pour une PR empilée : une PR qui le modifie est jugée par la version de `main`, et ses nouvelles règles valent pour les PR suivantes.

L'API est exposée sur Internet (`health.lacompagniemaximus.com`), porte quatre tokens et trois surfaces d'écriture, et deux agents de ce dépôt tournent sur le ThinkPad avec un jeton OAuth de Claude Code et la copie hors site des sauvegardes PostgreSQL. **Le merge est le déploiement** : depuis le 2026-10-12, un push sur `main` déclenche le webhook Forgejo (hook id 9) et Coolify redéploie l'app, en même temps que la CI du `push` et sans l'attendre (`la-compagnie-maximus/docs/coolify-ops.md`, « Auto-deploy au push sur `main` »). La CI des PR tourne avant le merge ; celle de `main`, non. Les agents de `agent/`, eux, ne partent qu'à la copie manuelle sur le poste (`agent/README.md`) : leur merge ne déploie rien, mais la copie suivante prendra `main` tel quel.

## Niveaux

Le niveau fixe la profondeur de la revue, que `/pr-review` règle pour chaque niveau. Il se calcule sur les fichiers que touche le diff, jamais sur l'auteur (Max, `/fix-issue`, `/autopilot`) :

- un fichier désigné par une ligne du tableau **Haut** dont la condition est remplie est de niveau haut ; sinon, un fichier désigné par **Bas** est de niveau bas ; sinon il est normal ;
- la PR prend le niveau le plus haut de ses fichiers, et la revue l'annonce avec le fichier qui le fixe ;
- un fichier renommé, déplacé ou supprimé se juge sur son ancien et sur son nouveau chemin : un fichier de niveau haut déplacé reste haut, et la PR qui le déplace met ce tableau à jour ;
- une condition qu'on ne sait pas trancher à la lecture du diff compte comme remplie.

Motifs : chemins depuis la racine. `*` ne traverse jamais un `/` ; `**/` vaut zéro, un ou plusieurs dossiers (`**/*.md` couvre `CLAUDE.md`, à la racine) ; `{a,b}` liste des variantes.

### Haut

| Motif | Condition | Zone |
|---|---|---|
| `index.js` | le diff touche le routage ou l'authentification : `ROUTES` (une route ajoutée comprise), `TOKEN_KINDS`, `resolveRoute`, `checkAuth`, `tokenMatches`, `handler`, `logRejection`, `safeLogValue`, ou une constante de token (`*_TOKEN`) | Routage et authentification |
| code livré : `{index,metrics,claude-usage,claude-usage-history,backups}.js` | fichier ajouté, renommé ou supprimé, ou diff qui ajoute ou change un effet de bord : écriture disque (`writeFile*`, `rename*`, `mkdir*`, `unlink*`, `rmSync`, `chmod*`, `writeJsonAtomic`, `writeHostSnapshot`), le balayage des temporaires (`sweepStaleTempFiles`, `TEMP_FILE_RE`), un plafond de corps (`readBody`, `*MAX_BODY_BYTES`), une commande shell (`child_process`, `execSync`, `spawn*`), un appel sortant (`fetch`, `http.request`), le bloc `require.main` | Surfaces d'écriture et montages ; Image et démarrage |
| `Dockerfile` | toujours : le `COPY` liste les modules un par un, `USER node` donne l'uid 1000 dont dépendent les montages | Image et démarrage |
| `agent/pg-backup-pull/**` | toujours : la rotation efface des dossiers de la seule copie hors site | Agents du poste |
| `agent/claude-usage/{limits,push-claude-usage}.js`, `agent/push-metrics.js`, `agent/**/*.sh` | toujours : ils tiennent le jeton OAuth de Claude Code ou un token d'ingestion, et leur sortie part dans syslog | Agents du poste |
| `.forgejo/workflows/**` | toujours : le runner `linux-builder` est un service de l'hôte de production, à portée instance, et le job `push` de `main` tourne pendant le déploiement | CI et gardes |
| `vitest.config.js`, `scripts/boot-from-copy.sh` | toujours : ils décident quels tests tournent et si le `COPY` est vérifié | CI et gardes |
| `__tests__/{auth,writes}.test.js` | le diff change ou retire une ligne existante hors commentaire ; un cas ajouté sans rien toucher d'autre reste normal | CI et gardes |
| `package.json` | le diff change autre chose que le champ `version` de l'app ou la version d'un paquet déjà listé : paquet ajouté ou retiré, bloc `dependencies` (le runtime est sans dépendance), `scripts`, `main`, `type`, `overrides` | Dépendances |
| `package-lock.json` | le diff ajoute ou retire un paquet (une clé `"node_modules/<nom>"` qui apparaît ou disparaît), change l'origine d'un paquet (hôte ou schéma de `resolved`) ou un `integrity` sans nouvelle version | Dépendances |
| `.gitignore` | le diff retire ou assouplit une ligne (`.env` et `.env.local` en dépendent) | Agents du poste, Dépendances |
| fichier à la racine du dépôt, hors `*.md` | ajouté, renommé ou supprimé, comme tout fichier d'un dossier caché nouveau : une configuration nouvelle peut changer le build Coolify, la CI ou le chargement de vitest sans motif qui la nomme | Image et démarrage, 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 :

- les handlers de lecture d'`index.js` et les nettoyeurs (`sanitizeSnapshot`, `sanitizeClaudeUsage`, `sanitizeDay`, `sanitizeBackupsStatus`, `validateBackupsPull`) tant que le diff ne touche ni le routage ni un effet de bord : ce qu'ils laissent passer finit dans l'arbre React de l'admin, mais échappé ;
- `agent/claude-usage/{aggregate,state,backfill-history}.js` : un jour figé à J+2 ne se recalcule plus, ce qui se juge avec la mutation du niveau normal ;
- `test-curl.sh`, `__tests__/ci.test.js` et le reste de `__tests__/**` : un affaiblissement d'épingle de `ci.test.js` n'ouvre rien que le tableau ne couvre déjà, le workflow et `boot-from-copy.sh` étant de niveau haut ;
- une montée de version d'un paquet déjà présent (vitest, seul paquet direct, en devDependency), correctif d'advisory compris (décision de Max du 2026-10-17 pour simpl-resultat, reprise ici) ;
- `scripts/review-levels.py` ;
- `CLAUDE.md`, `REVIEW.md` hors de la section « Niveaux », `agent/README.md` et `.claude/**` : `CLAUDE.md` porte le contrat des routes et les montages Coolify, `agent/README.md` l'installation des agents et de leurs fichiers de secrets ; ce sont des sources de ce fichier, jamais de niveau bas.

### Bas

Une PR est de niveau bas quand tous ses fichiers sont désignés par `**/*.md`, `docs/**`, `LICENSE` ou `reports/**`, hors des exceptions de la liste précédente. Exemples : `README.md`, `STATE.md`, `spec-*.md`.

### Normal

Tout le reste : tests et fixtures, `test-curl.sh`, `.env.example` (noms de variables seulement), les modules de l'agent qui ne tiennent aucun secret, une modification d'`index.js` limitée à un handler de lecture.

### Calibration

Rejouée le 2026-10-19 (#63) sur les 31 PR dont la tête a atteint `main`, de #2 à #62. Forgejo n'en marque qu'une partie comme mergées : une pile intégrée en avance rapide (#18 à #23, #31, #34, #37) apparaît fermée. #32 (démonstration d'une CI rouge) n'a jamais été intégrée. La liste est figée dans le script (`LANDED`).

| Ensemble | Bas | Normal | Haut |
|---|---|---|---|
| 31 PR intégrées, #2 à #62 (2026-04-22 au 2026-10-13) | 0 | 11 | 20 (65 %) |
| 17 PR depuis la CI, #31 à #62 (depuis le 2026-09-27) | 0 | 6 | 11 (65 %) |

Niveau haut sur les 31 : routage et authentification (#9, #20, #28, #30, #52, #57, #60), surfaces d'écriture et appels sortants (#2, #18, #37, #56), agents du poste (#22, #23, #53, #56, #59), CI et gardes (#9, #31, #34, #42, #44), fichier racine ajouté (#6). Les 11 PR normales sont de la documentation qui touche `CLAUDE.md` (#5, #39), des tests seuls ou la passe `test-curl.sh` (#19, #21, #26, #43, #45, #47), une route de lecture (#8), l'agrégation des transcripts (#51) et le nettoyeur de `files[]` (#62). Aucune PR n'est de niveau bas : chaque PR de documentation a touché `CLAUDE.md`.

L'écart à la cible de 20 % du modèle de simpl-resultat est voulu et justifié dans la PR qui ferme #63 : l'historique du dépôt est celui de la construction de ses surfaces sensibles (quatre tokens, trois routes d'écriture, deux agents qui tiennent un secret ou la copie hors site), chacune livrée par la PR qui l'a créée, et le merge part en production sans attendre la CI. Les PR qui ne font qu'étendre une surface existante, une route de lecture ou un test restent normales. Le script qui calcule ces niveaux est `scripts/review-levels.py` (API Forgejo, lecture seule, plus le clone local pour trois PR au diff vide ; `python3 scripts/review-levels.py 60 62` pour quelques PR) ; `python3 scripts/review-levels.py --check` vérifie, sans réseau, que ses motifs sont ceux du tableau Haut. Il n'approche qu'une partie des conditions et ne sert qu'à la calibration.

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.

## Ce qui compte par zone

Chaque règle cite sa source. Elles valent quel que soit le niveau : une PR normale qui touche une zone les respecte aussi.

### Routage et authentification

- Ordre non négociable : routage, puis `checkAuth()`, puis handler. Une route inconnue répond 404 avant tout contrôle de token ; ne jamais « corriger » en mettant l'auth d'abord. — `CLAUDE.md` « Routage et auth », trois cas `404 (not 401)` de `__tests__/auth.test.js`
- Un seul point de contrôle. Un nouveau token est une entrée de `TOKEN_KINDS`, jamais un `if` dans `checkAuth()` ; un `tokenKind` inconnu fait échouer le chargement du module. — `CLAUDE.md` « Routage et auth », issue #48
- Toute comparaison de token passe par `tokenMatches()` (`timingSafeEqual` sur empreintes SHA-256). Une égalité de chaîne contre `Bearer …` fuit longueur et préfixe ; les épingles sur le source et l'espion `timingSafeEqual` de `auth.test.js` le gardent. — `CLAUDE.md`, PR #30 (issue #17), GHSA-376v-xgjx-7mfr cité dans `index.js`
- Cloisonnement : chaque route d'écriture a son propre token, qui n'ouvre rien d'autre, et aucun autre token ne l'ouvre. Une PR qui ajoute une route l'entre dans la matrice des deux sens (`auth.test.js` pour une lecture, le fichier de tests de la route pour une écriture). — `CLAUDE.md` « Routage et auth », issues #48 et #58
- Fail-closed : `HEALTH_TOKEN` absent donne 401, un token d'ingestion absent donne 503 sur sa seule route, avec un message qui nomme sa variable. 401 passe avant 403 : un appelant sans token ne sonde pas l'allowlist. — `CLAUDE.md` « Routage et auth »
- Chaque rejet 401/403 est journalisé avec des valeurs filtrées en ASCII imprimable (`safeLogValue`) ; jamais un en-tête `Authorization` ni un corps dans le journal. — `CLAUDE.md` « Routage et auth »
- Un nouveau token se déclare dans Coolify en `is_runtime=true, is_buildtime=false`, jumeau preview compris : en buildtime, il sort en clair dans `application_deployment_queues.logs`. La PR le rappelle dans ses étapes de déploiement. — `CLAUDE.md` « Routage et auth », `coolify-ops.md` « Secrets en buildtime », incident #4

### Surfaces d'écriture et montages

- L'API n'écrit que dans `HOSTS_DIR`, le dossier de `CLAUDE_USAGE_PATH` (pour `POST /claude-usage`, archive `history/` comprise) et celui de `BACKUPS_PULL_PATH` (pour `POST /backups/pull`). Les montages Défenseurs et celui du statut des sauvegardes sont en `:ro` : une écriture ajoutée ailleurs passe les tests (fixtures inscriptibles, CI en root) et tombe en production en 500 `EROFS`. Un nouveau chemin qui écrit entre comme cas dans `__tests__/writes.test.js`. — `CLAUDE.md` « Gotchas », PR #39 (REQUEST_CHANGES : la garantie dépassait la couverture), #44
- Écriture atomique : temporaire dans le même dossier, mode 600, `renameSync`. Ne jamais « simplifier » en `writeFileSync` direct. Changer le nom d'un temporaire impose de changer `TEMP_FILE_RE` ; le balayage ne touche que des fichiers réguliers (`lstat`) de plus d'une heure. — `CLAUDE.md` « Gotchas », PR #37 et #43
- Une archive mensuelle illisible ou hors format n'est jamais écrasée : elle garde des jours sortis de la fenêtre. — `CLAUDE.md` (`POST /claude-usage`, archive #55)
- Le plafond de corps compte les octets reçus, jamais le `Content-Length`, et le 413 est rendu avant `req.destroy()`. — `CLAUDE.md` « Gotchas »
- Un nouveau montage se pose dans l'UI de Coolify par Max, sur le dossier et jamais sur le fichier (le fichier est remplacé par `rename`), avec le bon propriétaire (uid 1000 pour un montage en écriture). La PR le dit dans ses étapes de déploiement, puisque le merge déploie : un montage absent au moment du merge donne 500 sur chaque ingestion, ou 200 `no_data` sur une lecture. — `CLAUDE.md` « Config » (montages), `coolify-ops.md` (relevé du 2026-10-16 : `backups-pull` créé en `root:root`)

### Contrats servis au dashboard

- Tout objet persisté ou servi est reconstruit champ par champ depuis une liste blanche, et revalidé à la lecture : le fichier sur disque n'est pas plus digne de confiance que le corps reçu. Un champ inconnu, `__proto__` compris, est jeté. — `CLAUDE.md` « Gotchas », `claude-usage.js`, `backups.js`
- Une valeur invalide est jetée, jamais forcée (sauf `POST /backups/pull`, validé strictement : 400). — `CLAUDE.md` (`POST /claude-usage`, `POST /backups/pull`)
- Aucun texte libre venu d'un amont n'atteint le dashboard : `limits.detail` suit `^[a-z0-9_]{1,32}$`, les codes d'erreur des sauvegardes suivent deux listes fermées (une pour les bases, une pour les fichiers, `encrypt_failed` et `invalid_name` communs). Un code ajouté se justifie et se teste des deux côtés. — `CLAUDE.md` (`GET /backups`), PR #62
- La fraîcheur (`stale`, `online`, `ageSeconds`) est décidée par le serveur seul, sur `receivedAt` ou `generatedAt`, jamais sur une date fournie par le client (`pulledAt`). Le dashboard et `/warmup` lisent `stale` tel quel. — `CLAUDE.md` (`GET /hosts`, `GET /backups`)
- Un contrat partagé avec `la-compagnie-maximus` (`schemaVersion: 1`) ne change pas sans la PR correspondante côté dashboard. — `CLAUDE.md` (`POST /claude-usage`, `spec-plan-suivi-claude.md`)

### Image et démarrage

- Tout nouveau module runtime entre dans le `COPY` du `Dockerfile` ; tous les `require` restent au niveau module, en colonne 0, sans `import()` : le boot check n'appelle que `/health`. — `CLAUDE.md` « Gotchas », épingle de `__tests__/ci.test.js`, issue #11
- Runtime sans dépendance, HTTP natif : pas d'Express. — `CLAUDE.md`
- `agent/` n'entre jamais dans l'image. — `CLAUDE.md` « Gotchas », épingle de `ci.test.js`
- `getHealth()` garde `Promise.all` sur la mesure CPU et le check Logto. — `CLAUDE.md` « Gotchas », test de latence de `__tests__/health.test.js`

### Agents du poste

- Aucun secret dans une sortie : tout ce qu'impriment les agents part dans syslog et journald (`logger -t …`). Le chemin d'erreur ne rend que `err.code` et le code HTTP, jamais un objet d'erreur, des options de requête, un en-tête ou un corps de réponse ; jamais de `set -x` dans un `run-*.sh`. Les tests exigent l'absence de tout fragment de 5 caractères des tokens. — `CLAUDE.md` « Gotchas », `agent/README.md` « No secret in the logs »
- L'agent claude-usage ne rafraîchit jamais le jeton OAuth et n'écrit jamais `~/.claude/.credentials.json` (refresh token à usage unique : Claude Code serait déconnecté). L'URL Anthropic reste une constante de `limits.js`, sans variable d'environnement, avec `redirect: "error"`, pour que le bearer OAuth ne suive jamais l'URL de push configurable. — `CLAUDE.md` « Gotchas », `agent/README.md`, PR #53
- Un passage qui a rencontré une erreur de lecture ne fige aucun jour ; les jours figés sont écartés après la déduplication. — `CLAUDE.md` « Gotchas », PR #51 (REQUEST_CHANGES : seuil de scan qui rendait vides des jours ouverts), #56
- Rien de ce que livre le VPS ne détruit ni n'altère la copie hors site : la rotation s'ancre sur la sauvegarde valide la plus récente qui ne dépasse pas aujourd'hui, et garde ce dossier et les 29 jours qui le précèdent (ni sur aujourd'hui seul, qui ferait vieillir la dernière copie quand le VPS cesse de produire, ni sur le nom le plus récent seul, qu'un dossier daté dans le futur pousserait au-delà de toutes les vraies sauvegardes) ; un fichier déjà rapatrié n'est jamais réécrit (`--ignore-existing`, jamais `--delete`) ; un nom daté impossible ou futur est journalisé et ignoré. — `agent/README.md` (« Rotation is anchored… », « No overwrite »), PR #59 (trois passes REQUEST_CHANGES : rotation qui vidait la copie, VPS qui pouvait l'effacer par un dossier futur ou l'écraser par `rsync`)
- Le token d'ingestion passe à `curl` par un fichier d'en-tête en 0600, jamais en argv ; fichiers d'environnement en 600. — `agent/README.md`
- Le cron pointe vers une copie figée, jamais vers le clone. — `agent/README.md`, mémoire « ThinkPad = cette machine »

### CI et gardes

- Aucun filtre `branches:` sous `pull_request` (chaque maillon d'une pile a sa CI) ; `push` limité à `main`. Ni `defaults:` ni `working-directory` sur une étape `uses:` (le runner rejette le job) ; pas de `concurrency:`. — en-tête de `.forgejo/workflows/ci.yml`, `ci.test.js`
- Aucun `continue-on-error`, sous aucune graphie littérale. — `ci.test.js`, PR #42 (REQUEST_CHANGES : exemple de clé échappée décodé en route)
- L'image du `container:` reste celle du `FROM` du `Dockerfile`. — `ci.test.js`
- `boot-from-copy.sh` et `test-curl.sh` refusent un port déjà pris et ignorent les proxys de l'environnement (`NO_PROXY` et `no_proxy`). — `CLAUDE.md` « Tests », PR #45 (REQUEST_CHANGES : `wait_for` n'écartait pas un voisin), #47
- Le workflow n'utilise aucun secret du dépôt. Un `secrets.*` ou un `pull_request_target` ajouté se lit comme du code exécuté sur l'hôte de production. — `.forgejo/workflows/ci.yml`, `forgejo-ops.md` (runner `linux-builder`, portée instance)

### Règles de revue

- Un changement qui rend faux un fait cité ici (routes, tokens, montages, commandes de la CI, chemins du tableau Haut, mode de déploiement) met ce fichier à jour dans la même PR. — `CLAUDE.md`

## Commandes de vérification

Le workflow fait foi ; ce tableau le résume au 2026-10-19.

| Workflow | Déclencheur | Commandes |
|---|---|---|
| `ci.yml` | toute PR ; `push` sur `main` (en même temps que le déploiement) | dans `node:22-alpine` : `apk add git bash curl`, `npm ci`, `npm test`, `bash test-curl.sh`, `sh scripts/boot-from-copy.sh` |

Le verdict se lit sur le statut de commit (`GET /repos/maximus/vps-health-api/commits/<sha>/status`, contextes `CI / verification (pull_request)` et `(push)`) : l'API Actions répond 404 sur cette instance. — `CLAUDE.md` « Tests »

En local, depuis la racine du worktree, sans variable d'environnement à poser (chaque test fixe ses tokens et ses dossiers) :

```bash
npm ci && npm test
bash test-curl.sh               # ports 3099 et 3098 ; TEST_CURL_PORT et TEST_CURL_NOINGEST_PORT pour en changer
sh scripts/boot-from-copy.sh    # port 3097 ; BOOT_PORT pour en changer
python3 scripts/review-levels.py --check
```

- Deux revues en parallèle prennent chacune leurs propres ports pour `test-curl.sh` et `boot-from-copy.sh` : les deux scripts refusent un port pris, mais un refus n'est pas un vert.
- La CI tourne en root, l'image en uid 1000 : un test qui s'appuie sur des droits de fichiers (`chmod 0500`) ne prouve rien en CI. Le rejouer en utilisateur normal, ou injecter l'échec autrement. — PR #59
- Le conteneur de CI est busybox : `date -d` relatif et `find -printf` y échouent. Un script de l'agent testé en CI doit rester portable. — PR #59
- Une mutation se rejoue sur une copie hors dépôt de la tête, suite complète relancée à chaque fois, comme dans les revues de #39, #42 et #59.

## Pièges de revue

- **Le merge déploie, la CI de `main` arrive après.** Une PR empilée se juge sur son propre statut de commit ; le tip cumulé de la pile ne passe en CI qu'au push sur `main`, donc déjà en production. Le rejouer en local avant le merge. — `coolify-ops.md`, `ci.yml`
- **Une PR qui exige une action de Max sur le VPS** (montage, variable Coolify, propriétaire d'un dossier, `agents-map.json`) doit être mergée après l'action, pas avant : sinon la route répond 500 ou `no_data` dès le déploiement. La revue vérifie que la PR dit l'ordre. — `CLAUDE.md` « Config », « Gotchas »
- **Un montage manquant répond 200 `no_data`.** Après un redéploiement, contrôler le contenu des routes de lecture, pas leur code, et les trois vues des montages (`docker inspect`, `/proc/mounts`, contenu). — `CLAUDE.md` « Config »
- **La garantie écrite dépasse la couverture.** `CLAUDE.md` et les en-têtes de tests affirment beaucoup (« figé par », « exige ») ; chaque affirmation nouvelle se vérifie par une mutation qui fait rougir le test cité. — PR #39, #45
- **Séquences d'échappement décodées en route.** Un `\u002d` tapé dans un outil arrive décodé en `-` dans le fichier : vérifier dans le fichier (`grep`, `od -c`), jamais dans l'outil d'édition. — PR #42, mémoire « Escapes decoded in tool calls »
- **Stack intégrée en avance rapide.** Forgejo marque ces PR « fermées » et non « mergées », `/pulls/{n}/files` répond 500 une fois la branche de base supprimée, et une PR dont la tête est déjà dans `main` a un `.diff` vide. — calibration de #63
- **`/home/defenseur/defenseurs/status.json` est un leurre** : le Sergent écrit sous `/data/defenseurs/`. — `CLAUDE.md` « Config », « Gotchas »
- **La sévérité est asymétrique** : `?severity=LOW` cache INFO, que seul `?severity=INFO` rend. Ce n'est pas un défaut. — `CLAUDE.md` « Gotchas »