From 0b351e3937a4ae07cade69ed26be15253f4f478b Mon Sep 17 00:00:00 2001 From: le king fu Date: Mon, 19 Oct 2026 19:49:30 -0400 Subject: [PATCH 1/5] docs: add REVIEW.md with review levels and a calibration script (#63) REVIEW.md sets the review level of a PR from the files it touches (routing and auth, write surfaces, image, workstation agents, CI and guards), the rules of each zone with their source, the verification commands and the review pitfalls. scripts/review-levels.py replays the levels on the 31 PRs that reached main and checks, offline, that its patterns match the Haut table. CLAUDE.md said "no auto-deploy": since 2026-10-12 a push to main redeploys through the Forgejo webhook. The Deploy section now says so and points to REVIEW.md. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 8 +- REVIEW.md | 162 +++++++++++++++++++++++ scripts/review-levels.py | 269 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 438 insertions(+), 1 deletion(-) create mode 100644 REVIEW.md create mode 100755 scripts/review-levels.py diff --git a/CLAUDE.md b/CLAUDE.md index 9be9dba..a6ca1b3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -65,7 +65,9 @@ Consequence voulue : un appelant non authentifie sur une route inconnue voit 404 ## Deploy -Pas d'auto-deploy : l'app Coolify n'a pas de Source Forgejo (`source_id=null`, migration trackee dans la-compagnie-maximus#133). Apres un merge sur main, trigger manuel : +**Le merge est le deploiement** depuis le 2026-10-12 : un push sur `main` declenche le webhook Forgejo (hook id 9, route « manuelle » de Coolify, `source_id=null` reste vrai) et Coolify redeploie l'app, en meme temps que le job `push` de la CI et sans l'attendre (la-compagnie-maximus#133, `la-compagnie-maximus/docs/coolify-ops.md`, « Auto-deploy au push sur `main` »). Pas de `watch_paths` : un commit de doc ou un `state: sync` redeploie aussi. Les agents de `agent/` ne partent qu'a la copie sur le poste (`agent/README.md`). + +Redeploiement a la main, si besoin : ```bash curl -H "Authorization: Bearer $(cat ~/.coolify-token)" \ @@ -74,6 +76,10 @@ curl -H "Authorization: Bearer $(cat ~/.coolify-token)" \ (ou bouton Redeploy dans l'UI Coolify.) +## Documents de reference + +- `REVIEW.md` : niveaux de revue (haut/normal/bas) par fichier touche, regles par zone, commandes de verification et pieges, lus par `/pr-review`, `/fix-issue`, `/sprint` et `/pr-brief`. Calibre par `scripts/review-levels.py` (`--check` sans reseau compare ses motifs au tableau Haut). + ## Tests - **CI** : `.forgejo/workflows/ci.yml` (issue #24), sur toute PR (pas de filtre `branches:`, pour que les maillons d'une pile soient batis) et sur `push` vers `main`. Un seul job en `container: node:22-alpine`, la base du Dockerfile : `npm ci`, `npm test`, `bash test-curl.sh`, puis `sh scripts/boot-from-copy.sh`. Verdict a lire sur le statut de commit (`GET /repos/maximus/vps-health-api/commits//status`), l'API Actions repondant 404 sur cette instance. diff --git a/REVIEW.md b/REVIEW.md new file mode 100644 index 0000000..cfabed3 --- /dev/null +++ b/REVIEW.md @@ -0,0 +1,162 @@ +# 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/"` 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 date du poste, un fichier déjà rapatrié n'est jamais réécrit, un nom daté impossible ou futur est journalisé et ignoré. Le compte rendu se calcule après la rotation. — `agent/README.md` (pg-backup-pull), 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//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 `-` 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 » diff --git a/scripts/review-levels.py b/scripts/review-levels.py new file mode 100755 index 0000000..e0be162 --- /dev/null +++ b/scripts/review-levels.py @@ -0,0 +1,269 @@ +#!/usr/bin/env python3 +"""Replay the review levels of REVIEW.md on the pull requests that landed (#63). + +Usage: + python3 scripts/review-levels.py [N ...] (default: every PR of LANDED) + 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 PR with its level and the +files that set it, then the distribution. --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. + +Adapted from simpl-resultat/scripts/review-levels.py. The file list comes from +the PR's .diff, never from /pulls/{n}/files: that route answers 500 on a PR +whose base branch is gone, which is every link of a stack merged by +fast-forward. Three PRs (EMPTY_DIFF) have an empty .diff and are read from +the local clone: run the script from the repository. The content conditions of the Haut table are approximated from +the diff: index.js routing and auth, side effects of the shipped modules +(disk writes, environment, body caps, shell commands, outbound calls, +require.main, log lines), guard tests (an existing line changed or removed), +package.json (anything but a version), package-lock.json (a package added or +removed, an origin changed), .gitignore (a line removed). +""" +import itertools +import json +import os +import re +import subprocess +import sys +import urllib.request + +API = "https://git.lacompagniemaximus.com/api/v1/repos/maximus/vps-health-api" +REVIEW_MD = os.path.join(os.path.dirname(os.path.abspath(__file__)), "..", "REVIEW.md") + +# Every PR whose head reached main, frozen by number. Forgejo flags only part +# of them as merged: a stack merged by fast-forward (#18-#23, #31, #34, #37) +# shows as closed. #32 (a red-CI demo) never landed. +LANDED = [2, 5, 6, 8, 9, 18, 19, 20, 21, 22, 23, 26, 28, 30, 31, 34, 37, 39, 42, 43, + 44, 45, 47, 51, 52, 53, 56, 57, 59, 60, 62] + +# Three PRs whose .diff comes back empty: once fast-forwarded into main, their +# merge_base equals their head. Each is one commit, read from the local clone. +EMPTY_DIFF = { + 18: ("39a55fad93414b8e0049724f351710bfe1eb7a77", "79cb813767cfb53282d5fe6f386af3d740bf7f7e"), + 31: ("2b35b78a0beb04dc9ae92c13e42b9ff09fd2d147", "4e7879da6ca5163d37d11b9c5803cf2bed640b59"), + 34: ("4e7879da6ca5163d37d11b9c5803cf2bed640b59", "156ca818ca37441a2166d112251529ac32783afc"), +} + +# REVIEW.md, table Haut: rows without a condition. +HIGH_ALWAYS = [ + "agent/pg-backup-pull/**", + "agent/claude-usage/{limits,push-claude-usage}.js", "agent/push-metrics.js", "agent/**/*.sh", + "Dockerfile", + ".forgejo/workflows/**", + "vitest.config.js", "scripts/boot-from-copy.sh", + ".claude/settings.json", ".mcp.json", +] +SHIPPED = ["{index,metrics,claude-usage,claude-usage-history,backups}.js"] +GUARDS = ["__tests__/{auth,writes}.test.js"] +LOW = ["**/*.md", "docs/**", "LICENSE", "reports/**"] +NEVER_LOW = ["CLAUDE.md", "REVIEW.md", "agent/README.md", ".claude/**"] + +AUTH = re.compile(r"\b(ROUTES|TOKEN_KINDS|resolveRoute|checkAuth|tokenMatches|logRejection|" + r"safeLogValue|handler|timingSafeEqual|createHash|\w*_TOKEN)\b") +EFFECTS = re.compile( + r"\b(writeFile\w*|writeSync|createWriteStream|appendFile\w*|rename\w*|mkdir\w*|unlink\w*|" + r"rmSync|rmdir\w*|chmod\w*|chown\w*|copyFile\w*|symlink\w*|truncate\w*|writeJsonAtomic|" + r"writeHostSnapshot|sweepStaleTempFiles|TEMP_FILE_RE|readBody|\w*MAX_BODY_BYTES|" + r"child_process|execSync|execFile\w*|spawn\w*|fetch|https?\.(request|get))\b" + r"|require\.main") + + +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 parse_diff(diff): + """[{filename, previous_filename, status, block}] from a git diff.""" + files = [] + for block in re.split(r"(?m)^diff --git ", diff)[1:]: + head = block.split("\n@@", 1)[0] + m = re.match(r"a/(\S+) b/(\S+)", block) + old, new = m.group(1), m.group(2) + status = "changed" + if re.search(r"(?m)^new file mode", head): + status = "added" + elif re.search(r"(?m)^deleted file mode", head): + status = "deleted" + elif re.search(r"(?m)^rename from", head) or old != new: + status = "renamed" + files.append({"filename": new, "previous_filename": old if old != new else None, + "status": status, "block": block}) + return files + + +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 touches(block, pattern): + # a hunk whose enclosing function (the @@ context) or a changed line matches + for hunk in re.split(r"(?m)^(?=@@)", block)[1:]: + if pattern.search(hunk.split("\n", 1)[0]) or any(pattern.search(l) for l in changed(hunk)): + return True + return False + + +def package_json_changes(block): + # any key/value change except "version" and a bump of a package already listed + before, after = {}, {} + for l in changed(block): + m = re.match(r'^([+-])\s*"([^"]+)":\s*(.*?),?\s*$', l) + if not m: + return True + (before if m.group(1) == "-" else after)[m.group(2)] = m.group(3) + for key in set(before) | set(after): + if key == "version": + continue + if key in before and key in after and all(re.match(r'^"[\^~]?\d', v) for v in (before[key], after[key])): + continue + 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 + + +def lock_adds_or_removes(block): + if any(re.match(r'^[+-]\s+"node_modules/[^"]+": \{', l) for l in block.split("\n")): + return True + for hunk in re.split(r"(?m)^@@", block)[1:]: + hl = hunk.split("\n") + before, after = set(), set() + for l in hl: + m = re.match(r'^([+-])\s+"resolved": "([^"]+)"', l) + if m: + (before if m.group(1) == "-" else after).add(origin_of(m.group(2))) + if after - before: + return True + if any(re.match(r'^\+\s+"integrity": ', l) for l in hl) and not any(re.match(r'^\+\s+"version": ', l) for l in hl): + return True + return False + + +def review_level(files): + triggers = [] + for f in files: + b = f["block"] + for path in filter(None, {f["filename"], f["previous_filename"]}): + if match(HIGH_ALWAYS, path): + triggers.append(path) + if path == "index.js" and touches(b, AUTH): + triggers.append(path + " (routing or auth)") + if match(SHIPPED, path) and (f["status"] != "changed" or touches(b, EFFECTS)): + triggers.append(path + " (side effect)") + if match(GUARDS, path) and [l for l in changed(b) if l.startswith("-")]: + triggers.append(path + " (existing line changed)") + if path == "package.json" and package_json_changes(b): + triggers.append(path) + if path == "package-lock.json" and lock_adds_or_removes(b): + triggers.append(path) + if path == ".gitignore" and [l for l in changed(b) if l.startswith("-")]: + triggers.append(path + " (line removed)") + if path == "REVIEW.md": + triggers.append(path + " (check the « Niveaux » section by hand)") + parts = path.split("/") + new_hidden = f["status"] == "added" and any( + x.startswith(".") for x in parts[:-1]) and parts[0] not in (".forgejo", ".claude") + if (f["status"] != "changed" and len(parts) == 1 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 = { + "index.js", "{index,metrics,claude-usage,claude-usage-history,backups}.js", + "__tests__/{auth,writes}.test.js", "package.json", "package-lock.json", ".gitignore", + "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) | 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()) + numbers = [int(a) for a in sys.argv[1:]] or LANDED + rows = [] + for n in numbers: + if n in EMPTY_DIFF: + diff = subprocess.run(["git", "diff", *EMPTY_DIFF[n]], capture_output=True, text=True, check=True).stdout + else: + diff = get(f"/pulls/{n}.diff", raw=True).decode("utf-8", "replace") + files = parse_diff(diff) + if not files: + sys.exit(f"#{n}: empty diff, the level would read as bas") + level, triggers = review_level(files) + rows.append((n, level)) + print(f"#{n}\t{level}\t{'; '.join(triggers)}") + counts = {lvl: sum(1 for r in rows if r[1] == lvl) for lvl in ("bas", "normal", "haut")} + print(f"{len(rows)} PRs: {counts} -> haut {100 * counts['haut'] / len(rows):.1f} %") + + +if __name__ == "__main__": + main() -- 2.45.2 From 1d3be3ca4975e75bc6bae89520dcf08b07103bda Mon Sep 17 00:00:00 2001 From: le king fu Date: Mon, 19 Oct 2026 19:53:32 -0400 Subject: [PATCH 2/5] docs: correct the backups-pull mount, the mounts' status and the rotation rule (#63) The host side of the backups-pull mount is /srv/vps-health-api/backups-pull (docker inspect, 2026-10-19), and the three mounts marked "to set up" are in place. The test list gains agent-pg-backup-pull.test.js, missing until now. REVIEW.md stated the rotation was anchored on the workstation's date: it is anchored on the newest valid backup not after today. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 7 ++++--- REVIEW.md | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index a6ca1b3..0d5fe10 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -57,9 +57,9 @@ Consequence voulue : un appelant non authentifie sur une route inconnue voit 404 - `/data/defenseurs` (host) -> `/data/defenseurs:ro` : `status.json` + `findings-state.json` + `agents-map.json`, ecrits directement la par le Sergent. `/home/defenseur/defenseurs/status.json` est un leurre obsolete lu par personne. **Monte en lecture seule** depuis le 2026-09-30 (issue #36). - `/home/defenseur/defenseurs/reports` (host) -> `/data/defenseurs/reports:ro` : rapports de scan (+ sous-dir `archive/`), montage imbrique dans le precedent. Pose le 2026-07-15 (issue #10), recree le 2026-09-30 en Volume Mount avec Source Path pour recevoir le `:ro` (#36). **Monte en lecture seule**. - `/data/hosts` (host) -> `/data/hosts` : snapshots des postes. **Lecture-ecriture** — doit appartenir a l'uid 1000 (`node`, l'utilisateur du conteneur), sinon chaque ingestion repond 500. - - `/data/claude-usage` (host) -> `/data/claude-usage` : snapshot de consommation Claude (issue #48). **Lecture-ecriture**, proprietaire uid 1000, meme regle que `/data/hosts` (sinon chaque `POST /claude-usage` repond 500). Monter le **dossier**, jamais le fichier seul : le snapshot est remplace par `rename`, un bind de fichier resterait accroche a l'ancien inode. A poser par Max avant le premier push de l'agent. Le sous-dossier `history/` (#55) vit sur ce meme montage : rien a poser, le serveur le cree au premier POST de plus d'un jour. - - `/var/backups/postgres-status` (host) -> `/data/backups-status:ro` : `status.json` ecrit par `backup-postgres.sh` (#58), root, dossier 755, fichier 644, lisible par l'uid 1000. **Lecture seule**. Monter le **dossier** : le script remplace le fichier par `rename`. Sans ce montage, `GET /backups` repond avec `vps: null` (pas d'erreur). A poser par Max avant le premier deploy. - - `/data/backups-pull` (host) -> `/data/backups-pull` : compte rendu du tirage (#58). **Lecture-ecriture**, proprietaire uid 1000, le dossier et non le fichier, meme regle que `/data/claude-usage` (sinon chaque `POST /backups/pull` repond 500, `GET /backups` continue de servir le bloc VPS). A poser par Max avant le premier deploy. + - `/data/claude-usage` (host) -> `/data/claude-usage` : snapshot de consommation Claude (issue #48). **Lecture-ecriture**, proprietaire uid 1000, meme regle que `/data/hosts` (sinon chaque `POST /claude-usage` repond 500). Monter le **dossier**, jamais le fichier seul : le snapshot est remplace par `rename`, un bind de fichier resterait accroche a l'ancien inode. Pose (releve du 2026-10-19). Le sous-dossier `history/` (#55) vit sur ce meme montage : rien a poser, le serveur le cree au premier POST de plus d'un jour. + - `/var/backups/postgres-status` (host) -> `/data/backups-status:ro` : `status.json` ecrit par `backup-postgres.sh` (#58), root, dossier 755, fichier 644, lisible par l'uid 1000. **Lecture seule**. Monter le **dossier** : le script remplace le fichier par `rename`. Sans ce montage, `GET /backups` repond avec `vps: null` (pas d'erreur). Pose (releve du 2026-10-19). + - `/srv/vps-health-api/backups-pull` (host) -> `/data/backups-pull` : compte rendu du tirage (#58). **Lecture-ecriture**, proprietaire uid 1000, le dossier et non le fichier, meme regle que `/data/claude-usage` (sinon chaque `POST /backups/pull` repond 500, `GET /backups` continue de servir le bloc VPS). Pose (releve du 2026-10-19). - Deux couches tiennent l'API hors des fichiers Defenseurs. (1) Le mode du montage : `:ro` est un suffixe de la destination dans Coolify, recopie tel quel dans le compose genere (`'/data/defenseurs:/data/defenseurs:ro'`). (2) Les droits Unix : le contenu appartient a `defenseur:defenseur` (uid et gid 1001), dossiers en 755/775, fichiers en 664, et le conteneur tourne en uid 1000 (`node`, gid 1000, sans groupe supplementaire), donc `test -w` y echoue meme sans `:ro`. Ce sont les bits de groupe qui donnent l'ecriture : cette couche tient parce que le groupe proprietaire n'est pas le gid 1000 (groupe de `node` dans l'image, et de `ubuntu` sur l'hote). Un `chgrp ubuntu` ou un `chmod o+w` la ferait tomber sans bruit. Le `:ro` ne vaut que pour la vue du conteneur : le Sergent ecrit cote hote comme avant. - Verification apres tout redeploy, par trois vues : `docker inspect --format '{{range .Mounts}}{{.Source}} -> {{.Destination}} RW={{.RW}}{{println}}{{end}}'` (`RW=false` sur les deux montages Defenseurs et sur `/data/backups-status`, `RW=true` sur `/data/hosts`, `/data/claude-usage` et `/data/backups-pull`) ; `/proc/mounts` dans le conteneur (vue du noyau) ; et le **contenu** des routes de lecture, pas seulement leur code, car un montage manquant repond 200 `no_data`. @@ -94,6 +94,7 @@ curl -H "Authorization: Bearer $(cat ~/.coolify-token)" \ - `__tests__/hosts.test.js` — matrice exhaustive des endpoints postes (177 cas : regex de route comme controle anti-traversee, 401 avant 403, cloisonnement des deux tokens, 503 fail-closed, reconstruction par liste blanche, `loadAvg` a exactement 3 elements, 413, `neverSeen`, fraicheur serveur sous horloge figee, degradation de lecture (dont un `receivedAt` hors ISO complet, #54), temporaire supprime quand le `rename` echoue (#54), **balayage des `.tmp` orphelins** au demarrage, prouve sur le vrai point d'entree, symlink ni suivi ni supprime, ancre `^` de `TEMP_FILE_RE`) - `__tests__/agent.test.js` — agent poste (50 cas : contrat de payload verifie contre `sanitizeSnapshot()`, une seule tentative sans rejeu, timeout, codes de sortie, wrapper shell, et **le token absent de toute sortie d'echec, fragments de 5 caracteres compris**) - `__tests__/agent-claude-usage.test.js` — agent claude-usage du ThinkPad (136 cas). Agregation des transcripts, #49 (52 cas) : jours Toronto et changement d'heure, regles de projet, deduplication des requetes, 30 jours, top 8 + `(autres)`, 12 modeles, seuil de scan pris sur le plus ancien jour ouvert (jour fige futur, trou dans la serie figee, tout fige), noms `__proto__` et `constructor` sans ecriture sur `Object.prototype` ; #54 : requete a cheval sur minuit comptee une seule fois (jours figes ecartes apres la deduplication, egalite d'`output` gagnee par la ligne la plus ancienne quel que soit l'ordre de lecture), erreurs de lecture comptees (`readdir`, `ENOENT` d'un sous-dossier compris, `stat` y compris `ENOENT`, flux, ouverture), racine absente non comptee. Agent, #50 (74 cas) : reponse anonymisee de l'endpoint (`__tests__/fixtures/claude-usage/oauth-usage.json`) vers les items, repli `five_hour`/`seven_day`, tout `null` -> `unavailable` ; `fetch` (double) jamais appele sur jeton expire, `off`, credentials tronques, absents ou sans jeton, version absente ; 429 -> `http_error`/`http_429` avec les dernieres limites valides et leur `fetchedAt` ; corps hostiles -> `detail` dans l'enum ; bearer OAuth envoye a la seule URL Anthropic epinglee, aucune variable d'env ne la change (epingle sur le source), `redirect: "error"` ; **espions sur toutes les ecritures de `node:fs`** : le fichier de credentials n'est que lu ; aucun fragment de 5 caracteres des deux jetons dans le snapshot, l'etat ni la sortie (dont un vrai processus) ; deux passages autour d'un jour vide (fige, puis aucun fichier sous le seuil ouvert) ; `state.json` corrompu, elagage a 35 jours, ecriture atomique en 600, temporaire laisse en 644 ramene a 600 (#54) ; passage avec erreur de lecture : aucun jour fige, snapshot envoye, `read_errors=N` sans chemin, le passage suivant fige (#54) ; reponse de l'endpoint comptee en octets pendant la lecture, 256 Kio pile acceptes, + 1 octet en caracteres de 2 octets refuse, flux sans fin annule (#54) ; snapshot sur fixtures inchange par `sanitizeClaudeUsage` ; `http:` -> 1, > 64 Kio -> 2 sans envoi ; `run-claude-usage.sh` lance deux fois en parallele (faux `node`, HOME temporaire) : le second sort 0 sans travailler. Rattrapage ponctuel, #55 (10 cas) : fenetres passees contigues jusqu'a J-2, jours vides de tete retires, requete a cheval sur la frontiere de deux fenetres comptee une fois (meilleure ligne avant ou apres minuit), rien d'autre que `{ date, models }`, borne `--max-windows`, erreurs de lecture additionnees ; `main` n'ecrit que `--out` (mode 600, jamais ecrase), aucun `fetch`, aucun acces aux credentials, sortie en compteurs seulement, fichier accepte par le parseur d'import du serveur, erreurs d'usage + - `__tests__/agent-pg-backup-pull.test.js` — agent de tirage des sauvegardes du ThinkPad (21 cas, la-compagnie-maximus#220, PR #59 : `ssh`, `rsync` et `curl` remplaces par de faux binaires en tete de `PATH` qui consignent leur argv ; tirage par le port 2222 avec cle d'hote epinglee, `.age` seulement, `--ignore-existing` et jamais `--delete` ; token d'ingestion dans un fichier d'en-tete, jamais en argv ni en sortie, fichier supprime meme sur un `TERM` ; codes de sortie 1, 2 et 3 ; rotation : 30 dossiers dates gardes, jamais la sauvegarde la plus recente, un dossier date futur ou un nom impossible ignores sans rien supprimer, `rotation_failed` ; aucun `set -x`) - `__tests__/ci.test.js` — epingles du workflow et des scripts de CI (49 cas : image du `container:` identique au `FROM` du Dockerfile, les quatre etapes presentes, aucun filtre `branches:` sous `pull_request`, `push` limite a `main`, ni `defaults:` ni `working-directory` sur une etape `uses:`, **aucun `continue-on-error`, sous toute ecriture litterale** (une etape rouge passerait au vert en silence ; detecteur sur le mot nu, sans ancrage ni deux-points exiges, lui-meme verifie sur la cle en bloc, en element de liste, en style flow, entre guillemets et via une ancre YAML ; seules les lignes de commentaire entieres en sont exemptees ; une cle ecrite avec une sequence d'echappement, `"continue\u002don-error"`, passe encore : l'epingle lit du texte, pas du YAML), ce que le boot check suppose du Dockerfile : `USER node`, `CMD` en forme exec, le **refus d'un port deja occupe** (voisin HTTP reel, script lance en `spawn` asynchrone), **aucune requete par un proxy de l'environnement** (boot complet derriere un faux proxy qui repond a tout, sa vivacite prouvee d'abord par un `curl` nu, avec un `no_proxy` qui laisse la boucle locale hors liste : exporter `NO_PROXY` seul ne suffit pas), **`test-curl.sh` lance en entier** (#41 et #46, 22 cas, chacun sur des ports libres qui lui sont propres, passes par `TEST_CURL_PORT` et `TEST_CURL_NOINGEST_PORT` : derriere le meme faux proxy avec `FORCE_COLOR=3`, avec `BASE_URL` et `NOINGEST_URL` pointes sur un port mort, et avec `TOKEN` et `INGEST_TOKEN` herites d'une meme valeur, rien n'atteint le proxy, les 57 cas passent, le nombre etant ecrit en dur dans le test, et les deux ports sont refuses des la premiere tentative a la sortie du script ; **deux passages au meme instant** sur quatre ports libres, 51 cas chacun ; les **refus de port** : valeur non numerique, port 0, port au-dela de 65535, nombre suivi de lettres (`80abc`), chacun sur chacune des deux variables, puis deux ports identiques, et les **deux bornes acceptees**, 1 et 65535, donnees aux deux variables a la fois pour ne rien faire ecouter sur un port fixe (atteindre le refus « must differ » montre que la valeur a passe le controle de plage), message sur stderr et code 1 avant tout lancement ; un **voisin HTTP reel** sur le port principal, puis sur le second : refus avant tout lancement, le voisin n'ayant vu que la sonde `/` ; un **occupant non HTTP** sur le port principal, qui ferme aussitot (code curl 52 ou 56) puis qui accepte sans repondre (28) : meme refus, la regle etant « tout code autre que 7 » ; un **serveur vivant qui n'ecoute jamais** : message d'echeance et code 1 apres les 5 s d'attente ; puis un faux `node` en tete de `PATH` qui sort en 1 pour le port principal, puis le second : message sur stderr, code 1 en moins de 3 s, aucun serveur laisse sur son port ; et deux **epingles sur le source** du script : defauts 3099 et 3098, seules occurrences de ces nombres hors commentaires, et URLs derivees des ports sans lecture de l'environnement. Plus aucun cas n'ecoute sur 3099 ni 3098 : les defauts ne sont exerces que par l'etape CI « Socket-level pass » et par un `bash test-curl.sh` a la main. Les ports viennent de `freePorts()`, qui ouvre ses sondes ensemble puis les ferme avant le lancement : un port repris dans l'intervalle fait echouer le cas avec le message de `require_free`, sans cas lance contre un mauvais serveur), et **des `require` en colonne 0 seulement, sans `import()`**, dans chaque `.js` du `COPY`. Cette derniere epingle est une heuristique sur les appels litteraux, espace avant la parenthese comprise (`require (`, `import (`). Son detecteur, `loadsLate()`, est lui-meme verifie sur six lignes : `require` indente et `import` a toute colonne, avec ou sans espace, signales ; `require` de premier niveau et ligne de commentaire laisses passer. Trous connus, entre autres, laisses ouverts (#35) : une fonction ecrite entiere sur une ligne en colonne 0 (`const lazy = () => require("x");`), un `require(` renvoye a la ligne en colonne 0 dans une fonction (de meme forme que le `} = require("node:fs");` legitime d'`index.js`), et toute indirection, comme un alias de `require`) - `__tests__/writes.test.js` — surfaces d'ecriture : `HOSTS_DIR`, et le dossier de `CLAUDE_USAGE_PATH` pour le seul `POST /claude-usage`, archive `history/` comprise (#55) (#48 : `violations()` prend le dossier autorise du cas, `HOSTS_DIR` par defaut ; deux cas dedies, `POST /claude-usage` n'ecrit que dans son dossier, et ses rejets 401/400/413 et la lecture d'un fichier corrompu n'ecrivent rien du tout ; `GET /claude-usage` entre dans les routes de lecture ; #55 : le cas POST exige le `rename` du fichier mensuel du jour clos, `GET /claude-usage/history` entre dans les routes de lecture, et un mois corrompu ou une plage refusee n'ecrivent rien ; #58 : le dossier de `BACKUPS_PULL_PATH` pour le seul `POST /backups/pull`, `GET /backups` entre dans les routes de lecture, ses rejets 401/400/413 et deux fichiers corrompus puis absents n'ecrivent rien) (18 cas : espions sur toutes les fonctions d'ecriture de `node:fs`, formes callback, Sync et `fs.promises`, et sur `child_process`, poses avant un `require` frais apres purge du cache de **tous** les modules runtime (un `metrics.js` reste en cache garderait les espions restaures d'un test precedent) ; demarrage par le balayage des `.tmp` ; les neuf routes GET n'ecrivent rien, meme une macrotache apres la reponse, contenu reel exige ; `POST /hosts/` n'ecrit que sous `HOSTS_DIR` ; **chemins degrades** : rejets 401/403/404, entrees 400/413, `agents-map.json` corrompu, fichiers Defenseurs absents, snapshot corrompu, avec le contenu degrade exige (`no_data`, `neverSeen`) ; **commandes shell** figees sur `head -1 /proc/stat` et `df -k /`, `/health` servant de temoin ; chaque cas prouve que ses espions sont vivants ; **tout appel est compte, sans attribution par pile** (#40 : une fonction `node:fs` passee telle quelle a `process.nextTick`, a un ecouteur ou a un `.then` s'execute sans frame runtime sur sa pile, et un filtre par pile la laissait passer) ; les retouches de fixtures du cas degrade passent par les fonctions reelles capturees avant la pose des espions, jamais par un `rmSync` recursif, qui appelle en interne le `fs.rmdirSync` public, donc l'espion ; **epingle sur le bloc `require.main`**, ensemble exact des appels, puisqu'il ne tourne jamais dans le processus de test. Limites : un chemin qu'aucun cas n'emprunte, et une ecriture differee au-dela de 20 ms. Contrepartie assumee : une ecriture du lanceur de tests lui-meme pendant un cas le ferait rougir) - Runtime reste 0-dep ; vitest en devDep uniquement diff --git a/REVIEW.md b/REVIEW.md index cfabed3..d7334de 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -110,7 +110,7 @@ Chaque règle cite sa source. Elles valent quel que soit le niveau : une PR norm - 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 date du poste, un fichier déjà rapatrié n'est jamais réécrit, un nom daté impossible ou futur est journalisé et ignoré. Le compte rendu se calcule après la rotation. — `agent/README.md` (pg-backup-pull), 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`) +- 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 » -- 2.45.2 From 9547965c9d4e8534cc06e65fef4734edf42b9d06 Mon Sep 17 00:00:00 2001 From: le king fu Date: Mon, 19 Oct 2026 19:56:48 -0400 Subject: [PATCH 3/5] docs: keep the escaped example escaped, align the script with the table (#63) The escape-sequence pitfall had lost its own example on the way: the backslash-u002d sequence landed decoded as a plain dash. It is written back with the backslash built by chr(92) and checked with od -c. The script's docstring no longer lists the environment and log-line triggers dropped during calibration, and AUTH no longer matches timingSafeEqual and createHash, which the Haut table does not name. Replay unchanged: 20 haut out of 31. Co-Authored-By: Claude Opus 5.5 --- REVIEW.md | 2 +- scripts/review-levels.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/REVIEW.md b/REVIEW.md index d7334de..f13e4f0 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -156,7 +156,7 @@ python3 scripts/review-levels.py --check - **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 » +- **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 » diff --git a/scripts/review-levels.py b/scripts/review-levels.py index e0be162..b2f2cc4 100755 --- a/scripts/review-levels.py +++ b/scripts/review-levels.py @@ -18,8 +18,8 @@ whose base branch is gone, which is every link of a stack merged by fast-forward. Three PRs (EMPTY_DIFF) have an empty .diff and are read from the local clone: run the script from the repository. The content conditions of the Haut table are approximated from the diff: index.js routing and auth, side effects of the shipped modules -(disk writes, environment, body caps, shell commands, outbound calls, -require.main, log lines), guard tests (an existing line changed or removed), +(disk writes, the temp-file sweep, body caps, shell commands, outbound calls, +require.main), guard tests (an existing line changed or removed), package.json (anything but a version), package-lock.json (a package added or removed, an origin changed), .gitignore (a line removed). """ @@ -63,7 +63,7 @@ LOW = ["**/*.md", "docs/**", "LICENSE", "reports/**"] NEVER_LOW = ["CLAUDE.md", "REVIEW.md", "agent/README.md", ".claude/**"] AUTH = re.compile(r"\b(ROUTES|TOKEN_KINDS|resolveRoute|checkAuth|tokenMatches|logRejection|" - r"safeLogValue|handler|timingSafeEqual|createHash|\w*_TOKEN)\b") + r"safeLogValue|handler|\w*_TOKEN)\b") EFFECTS = re.compile( r"\b(writeFile\w*|writeSync|createWriteStream|appendFile\w*|rename\w*|mkdir\w*|unlink\w*|" r"rmSync|rmdir\w*|chmod\w*|chown\w*|copyFile\w*|symlink\w*|truncate\w*|writeJsonAtomic|" -- 2.45.2 From 44d58970ad6244a7f6737cf1e01b6b92b8a84ce1 Mon Sep 17 00:00:00 2001 From: le king fu Date: Mon, 19 Oct 2026 20:21:29 -0400 Subject: [PATCH 4/5] docs: apply three review suggestions, record Max's choices (#63) - .claude/hooks/** joins the always-high row: a hook script runs on the workstation. - Bas narrows to **/*.md and LICENSE: a script or an image added under docs/ or reports/ is no longer low. - package.json: a package moved between devDependencies and dependencies, or any line of the dependencies block, is high; the script now tracks the block each package line belongs to. - Calibration records the choices Max confirmed on 2026-10-19. Replay unchanged: 20 haut out of 31. Co-Authored-By: Claude Opus 5.5 --- REVIEW.md | 8 +++++--- scripts/review-levels.py | 36 +++++++++++++++++++++++++++--------- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/REVIEW.md b/REVIEW.md index f13e4f0..f6d64a7 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -29,11 +29,11 @@ Motifs : chemins depuis la racine. `*` ne traverse jamais un `/` ; `**/` vaut z | `.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.json` | le diff change autre chose que le champ `version` de l'app ou la version d'un paquet déjà listé dans le même bloc : paquet ajouté, retiré ou déplacé entre `devDependencies` et `dependencies`, toute ligne du 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/"` 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 | +| `.claude/settings.json`, `.claude/hooks/**`, `.mcp.json` | toujours : hooks et leurs scripts, 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 : @@ -47,7 +47,7 @@ Restent au moins normaux, et leurs règles de zone s'appliquent quand même : ### 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`. +Une PR est de niveau bas quand tous ses fichiers sont désignés par `**/*.md` (ceux de `docs/` et de `reports/` compris) ou `LICENSE`, hors des exceptions de la liste précédente. Un fichier d'un autre type sous `docs/` ou `reports/` (un script, une image) reste normal. Exemples : `README.md`, `STATE.md`, `spec-*.md`. ### Normal @@ -66,6 +66,8 @@ Niveau haut sur les 31 : routage et authentification (#9, #20, #28, #30, #52, #5 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. +Choix confirmés par Max le 2026-10-19, sur la PR qui ferme #63 : le taux de 65 % est gardé ; une montée de version d'un paquet déjà présent reste normale ; les agents du poste listés au tableau restent toujours hauts, même si leur merge ne déploie rien ; la liste des fichiers jamais bas est celle ci-dessus ; les gardes `auth.test.js` et `writes.test.js` sont hautes dès qu'une ligne existante change ; les nettoyeurs restent normaux, élargir une liste fermée relevant de la règle de zone. Tout écart à ces choix se demande à Max. + 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 diff --git a/scripts/review-levels.py b/scripts/review-levels.py index b2f2cc4..5c129d1 100755 --- a/scripts/review-levels.py +++ b/scripts/review-levels.py @@ -55,11 +55,11 @@ HIGH_ALWAYS = [ "Dockerfile", ".forgejo/workflows/**", "vitest.config.js", "scripts/boot-from-copy.sh", - ".claude/settings.json", ".mcp.json", + ".claude/settings.json", ".claude/hooks/**", ".mcp.json", ] SHIPPED = ["{index,metrics,claude-usage,claude-usage-history,backups}.js"] GUARDS = ["__tests__/{auth,writes}.test.js"] -LOW = ["**/*.md", "docs/**", "LICENSE", "reports/**"] +LOW = ["**/*.md", "LICENSE"] NEVER_LOW = ["CLAUDE.md", "REVIEW.md", "agent/README.md", ".claude/**"] AUTH = re.compile(r"\b(ROUTES|TOKEN_KINDS|resolveRoute|checkAuth|tokenMatches|logRejection|" @@ -147,15 +147,33 @@ def touches(block, pattern): def package_json_changes(block): - # any key/value change except "version" and a bump of a package already listed + # anything but "version" and a bump of a package listed in the same block; + # any line of "dependencies" counts, since the runtime has none before, after = {}, {} - for l in changed(block): - m = re.match(r'^([+-])\s*"([^"]+)":\s*(.*?),?\s*$', l) - if not m: - return True - (before if m.group(1) == "-" else after)[m.group(2)] = m.group(3) + for hunk in re.split(r"(?m)^@@", block)[1:]: + section = None + for l in hunk.split("\n")[1:]: + if l.startswith(("+++", "---")) or l[:1] not in " +-": + continue + m = re.match(r'^[ +-]\s*"([^"]+)":\s*\{', l) + if m: + section = m.group(1) + if l[:1] == " " or not meaningful(l[1:]): + if re.match(r"^[ +-]\s*\}", l): + section = None + continue + if section == "dependencies": + return True + m = re.match(r'^([+-])\s*"([^"]+)":\s*(.*?),?\s*$', l) + if not m: + return True + if m.group(3) == "{": + continue + (before if m.group(1) == "-" else after)[(section, m.group(2))] = m.group(3) + if re.match(r"^[+-]\s*\}", l): + section = None for key in set(before) | set(after): - if key == "version": + if key[1] == "version" and key[0] is None: continue if key in before and key in after and all(re.match(r'^"[\^~]?\d', v) for v in (before[key], after[key])): continue -- 2.45.2 From 1dff0c805ec5efea03524cc24f12e80be120ddba Mon Sep 17 00:00:00 2001 From: le king fu Date: Mon, 19 Oct 2026 20:24:12 -0400 Subject: [PATCH 5/5] docs(review-levels): name the package.json block blind spot (#63) Co-Authored-By: Claude Opus 5.5 --- scripts/review-levels.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/review-levels.py b/scripts/review-levels.py index 5c129d1..ef2a0b8 100755 --- a/scripts/review-levels.py +++ b/scripts/review-levels.py @@ -21,7 +21,10 @@ the diff: index.js routing and auth, side effects of the shipped modules (disk writes, the temp-file sweep, body caps, shell commands, outbound calls, require.main), guard tests (an existing line changed or removed), package.json (anything but a version), package-lock.json (a package added or -removed, an origin changed), .gitignore (a line removed). +removed, an origin changed), .gitignore (a line removed). package.json +knows the block of a package line only when the block's opening line is in +the hunk: a bump deep inside a large "dependencies" block reads as normal, +where the table says high (impossible today, the runtime has none). """ import itertools import json -- 2.45.2