vps-health-api/REVIEW.md

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) :

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 - tapé dans un outil arrive décodé 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 »