fix(hosts): require 3 loadAvg values and sweep orphan temp files (#25) #37

Closed

maximus wants to merge 0 commits from issue-25-hosts-review-followups into main

maximus commented

2026-09-30 00:30:22 +00:00

Owner

Fixes #25

Les quatre constats de la revue de la pile monitoring des postes (#18-#23), selon les décisions de l'analyse du 2026-09-29.

1. loadAvg : exactement 3 nombres. sanitizeCpu() refuse toute autre longueur (cpu.loadAvg must hold exactly 3 finite numbers, 400, rien n'est écrit). Avant, [] était accepté et [1..5] tronqué. Un fichier déjà stocké hors format dégrade en neverSeen à la relecture. L'agent envoie os.loadavg(), toujours 3 valeurs : aucune rupture côté poste.

2. cleanString : commentaire et phrase dans le README. Les chaînes sont tronquées, pas filtrées, et la sûreté repose sur l'échappement du consommateur.

3. Débris .tmp : sweepStaleTempFiles(), appelée au démarrage avant listen. Elle ne touche que le format exact .<id>.<pid>.<ms>.tmp (même regex d'id que la route), que des fichiers réguliers (lstat), et seulement au-delà d'une heure : plus jeune, un autre conteneur peut être en train d'écrire pendant un redéploiement. Un HOSTS_DIR absent reste silencieux ; tout autre échec est journalisé sans bloquer le démarrage.

4. frozenRequest : le commentaire précise que new Date() lit l'horloge réelle, y compris le receivedAt d'un POST.

Tests : 155 -> 171 dans hosts.test.js (329 au total). Le test du démarrage lance le vrai node index.js sur un HOSTS_DIR préparé : ce n'est pas une épingle sur le source.

Preuve par mutation (index.js restauré depuis une copie, git diff --stat identique avant/après) :

Mutation	Rouges
retour à `slice(0, 3)` sans contrôle de longueur	5 (4 lignes de la matrice, 1 dégradation)
filtre d'âge retiré	2 (fichier récent, démarrage)
regex de nom élargie à `/\.tmp$/`	5 (noms hors format)
regex sans double échappement (`.` joker)	2
garde `isFile()` retirée	1 (le répertoire passe par le chemin d'erreur, l'espion `console.error` le voit)
appel au démarrage retiré	1

Suite locale : 329/329 vitest, 45/45 test-curl.sh, boot-from-copy: PASS.

Déploiement : index.js change, donc il faut redéployer après le merge. Aucune nouvelle variable d'environnement, aucun nouveau module (le COPY du Dockerfile est inchangé).

🤖 Generated with Claude Code

Fixes #25 Les quatre constats de la revue de la pile monitoring des postes (#18-#23), selon les décisions de l'analyse du 2026-09-29. **1. `loadAvg` : exactement 3 nombres.** `sanitizeCpu()` refuse toute autre longueur (`cpu.loadAvg must hold exactly 3 finite numbers`, 400, rien n'est écrit). Avant, `[]` était accepté et `[1..5]` tronqué. Un fichier déjà stocké hors format dégrade en `neverSeen` à la relecture. L'agent envoie `os.loadavg()`, toujours 3 valeurs : aucune rupture côté poste. **2. `cleanString`** : commentaire et phrase dans le README. Les chaînes sont tronquées, pas filtrées, et la sûreté repose sur l'échappement du consommateur. **3. Débris `.tmp`** : `sweepStaleTempFiles()`, appelée au démarrage avant `listen`. Elle ne touche que le format exact `.<id>.<pid>.<ms>.tmp` (même regex d'id que la route), que des fichiers réguliers (`lstat`), et seulement au-delà d'une heure : plus jeune, un autre conteneur peut être en train d'écrire pendant un redéploiement. Un `HOSTS_DIR` absent reste silencieux ; tout autre échec est journalisé sans bloquer le démarrage. **4. `frozenRequest`** : le commentaire précise que `new Date()` lit l'horloge réelle, y compris le `receivedAt` d'un POST. **Tests** : 155 -> 171 dans `hosts.test.js` (329 au total). Le test du démarrage lance le vrai `node index.js` sur un `HOSTS_DIR` préparé : ce n'est pas une épingle sur le source. **Preuve par mutation** (`index.js` restauré depuis une copie, `git diff --stat` identique avant/après) : | Mutation | Rouges | |---|---| | retour à `slice(0, 3)` sans contrôle de longueur | 5 (4 lignes de la matrice, 1 dégradation) | | filtre d'âge retiré | 2 (fichier récent, démarrage) | | regex de nom élargie à `/\.tmp$/` | 5 (noms hors format) | | regex sans double échappement (`.` joker) | 2 | | garde `isFile()` retirée | 1 (le répertoire passe par le chemin d'erreur, l'espion `console.error` le voit) | | appel au démarrage retiré | 1 | **Suite locale** : 329/329 vitest, 45/45 `test-curl.sh`, `boot-from-copy: PASS`. **Déploiement** : `index.js` change, donc il faut redéployer après le merge. Aucune nouvelle variable d'environnement, aucun nouveau module (le `COPY` du Dockerfile est inchangé). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

maximus added 1 commit 2026-09-30 00:30:23 +00:00

fix(hosts): require 3 loadAvg values and sweep orphan temp files (#25 )

CI / verification (pull_request) Successful in 21s

Details

CI / verification (push) Successful in 20s

Details

523d90bf7c

Follow-ups from the review of the workstation monitoring stack (#18-#23):

- loadAvg must hold exactly 3 finite numbers (1/5/15-min averages). The
  admin card joins them, so a shorter array rendered a partial line and a
  longer one was silently truncated; both are now a 400, and a stored
  snapshot off that format degrades to neverSeen on read.
- sweepStaleTempFiles() runs at startup, before listen, and removes the
  `.<id>.<pid>.<ms>.tmp` files a process killed mid-write leaves behind,
  once older than an hour. Exact name format only, regular files only
  (lstat), best effort: a missing HOSTS_DIR stays silent, other failures
  are logged and never stop the server.
- Document that cleanString keeps control characters on purpose (safety
  rests on the consumer escaping at render time), and that frozenRequest
  does not freeze `new Date()`, which stamps receivedAt on POST.

Fixes #25

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

maximus commented

2026-09-30 00:44:10 +00:00

Author

Owner

Verdict : APPROVE

Résumé : les quatre points de #25 sont traités comme l'analyse l'a tranché. Le contrôle de longueur de loadAvg et le balayage des .tmp sont corrects et bornés. Chaque mutation annoncée dans le corps de la PR, rejouée par le relecteur, fait tomber le nombre de tests déclaré.

Vérifié

Suite rejouée sur une copie hors dépôt de 523d90b : 329/329 vitest, 45/45 test-curl.sh, boot-from-copy: PASS. Statut de commit CI : success.
Mutations du relecteur (sur la copie, index.js restauré puis cmp identique) :

Mutation	Rouges
retrait du contrôle `length !== 3` (retour au comportement sans troncature)	5
`length < 3` au lieu de `!== 3`	1 (la ligne à 4 éléments de la matrice)
filtre d'âge retiré	2
regex élargie à `/\.tmp$/`	5
garde `isFile()` retirée	1
appel au démarrage retiré	1
balayage déplacé après `listen` (`setTimeout`)	1 : le test de démarrage prouve bien l'ordre
`ENOENT` journalisé	1
`readdir` qui lève au lieu de rendre `[]`	1

Timeline de #25 : pull_ref depuis #37 avec ref_action: "closes", donc la fermeture se fera au merge, comme voulu.
TEMP_FILE_RE est construit depuis HOST_ID_PATTERN (même source que la route). Pas de point possible dans l'id, donc pas d'ambiguïté de découpage.
Aucun secret dans le diff, aucune nouvelle variable d'environnement, le COPY du Dockerfile est inchangé. Décompte : 155 + 11 (balayage) + 4 (matrice) + 1 (dégradation) = 171, et 313 + 16 = 329.

Suggestions (non bloquantes)

Symlink non épinglé (index.js:451) : le commentaire affirme qu'un symlink « is neither followed nor removed », mais remplacer lstatSync par statSync laisse la suite verte (221/221 sur hosts et agent). Avec stat, un symlink au bon nom qui pointe vers un vieux fichier régulier serait supprimé (le lien seulement, pas la cible, donc sans gravité). Un test fs.symlinkSync vers un fichier vieux, qui vérifie que le lien et la cible survivent, figerait l'affirmation.
Ancre ^ non épinglée (index.js:434) : la retirer laisse aussi la suite verte, parce qu'aucun des noms hors format ne contient de sous-chaîne valide. Un cas "prefix.thinkpad.4242.1700000000000.tmp" dans le test.each « never touches » la couvrirait.
Frontière <= contre < à exactement une heure : non testée. C'est cosmétique, rien à faire.

🤖 Generated with Claude Code

## Verdict : APPROVE **Résumé** : les quatre points de #25 sont traités comme l'analyse l'a tranché. Le contrôle de longueur de `loadAvg` et le balayage des `.tmp` sont corrects et bornés. Chaque mutation annoncée dans le corps de la PR, rejouée par le relecteur, fait tomber le nombre de tests déclaré. ### Vérifié - **Suite rejouée sur une copie hors dépôt de `523d90b`** : 329/329 vitest, 45/45 `test-curl.sh`, `boot-from-copy: PASS`. Statut de commit CI : `success`. - **Mutations du relecteur** (sur la copie, `index.js` restauré puis `cmp` identique) : | Mutation | Rouges | |---|---| | retrait du contrôle `length !== 3` (retour au comportement sans troncature) | 5 | | `length < 3` au lieu de `!== 3` | 1 (la ligne à 4 éléments de la matrice) | | filtre d'âge retiré | 2 | | regex élargie à `/\.tmp$/` | 5 | | garde `isFile()` retirée | 1 | | appel au démarrage retiré | 1 | | balayage déplacé **après** `listen` (`setTimeout`) | 1 : le test de démarrage prouve bien l'ordre | | `ENOENT` journalisé | 1 | | `readdir` qui lève au lieu de rendre `[]` | 1 | - Timeline de #25 : `pull_ref` depuis #37 avec `ref_action: "closes"`, donc la fermeture se fera au merge, comme voulu. - `TEMP_FILE_RE` est construit depuis `HOST_ID_PATTERN` (même source que la route). Pas de point possible dans l'id, donc pas d'ambiguïté de découpage. - Aucun secret dans le diff, aucune nouvelle variable d'environnement, le `COPY` du Dockerfile est inchangé. Décompte : 155 + 11 (balayage) + 4 (matrice) + 1 (dégradation) = 171, et 313 + 16 = 329. ### Suggestions (non bloquantes) 1. **Symlink non épinglé** (`index.js:451`) : le commentaire affirme qu'un symlink « is neither followed nor removed », mais remplacer `lstatSync` par `statSync` laisse la suite **verte** (221/221 sur hosts et agent). Avec `stat`, un symlink au bon nom qui pointe vers un vieux fichier régulier serait supprimé (le lien seulement, pas la cible, donc sans gravité). Un test `fs.symlinkSync` vers un fichier vieux, qui vérifie que le lien et la cible survivent, figerait l'affirmation. 2. **Ancre `^` non épinglée** (`index.js:434`) : la retirer laisse aussi la suite verte, parce qu'aucun des noms hors format ne contient de sous-chaîne valide. Un cas `"prefix.thinkpad.4242.1700000000000.tmp"` dans le `test.each` « never touches » la couvrirait. 3. Frontière `<=` contre `<` à exactement une heure : non testée. C'est cosmétique, rien à faire. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

maximus commented

2026-09-30 00:48:54 +00:00

Author

Owner

Mergé en fast-forward sur main (523d90b) après rejeu local du tip : npm ci, 329/329 vitest, 45/45 test-curl.sh, boot-from-copy: PASS. Forgejo ne détecte pas le ff local, fermeture manuelle. Les deux tests manquants signalés par la revue (symlink, ancre ^) sont reportés dans une issue de suivi.

Mergé en fast-forward sur `main` (`523d90b`) après rejeu local du tip : `npm ci`, 329/329 vitest, 45/45 `test-curl.sh`, `boot-from-copy: PASS`. Forgejo ne détecte pas le ff local, fermeture manuelle. Les deux tests manquants signalés par la revue (symlink, ancre `^`) sont reportés dans une issue de suivi.