Constats mineurs de la review de la pile monitoring des postes #25

Open
opened 2026-08-16 18:28:49 +00:00 by maximus · 0 comments
Owner

Constats non bloquants releves par /pr-review sur la pile #18-#23. Ils sont consignes ici
parce qu'ils meurent avec les PRs fermees. Aucun n'a justifie de retarder le merge ; aucun
n'est un defaut de comportement.

1. loadAvg accepte sans garantie de longueur

sanitizeSnapshot() accepte loadAvg: [] et le README ne promet aucune longueur. Le
consommateur (la carte du dashboard, la-compagnie-maximus#149) fait
loadAvg.map(...).join(" / ") — un tableau vide rend une chaine vide, pas une erreur, mais
rien ne le documente. Soit garantir 3 elements cote serveur, soit l'ecrire dans le contrat.

2. cleanString conserve les caracteres de controle

Choix delibere (ne pas mutiler une valeur legitime), mais la surete repose alors entierement
sur l'echappement du consommateur — React echappe, donc c'est sur aujourd'hui. Ce n'est
documente nulle part. Une ligne dans le README suffirait a rendre la dependance explicite.

3. Debris de fichier temporaire si le process meurt en cours d'ecriture

L'ecriture atomique passe par un fichier temporaire puis renameSync. Un crash entre les deux
laisse un .tmp dans HOSTS_DIR. Sans consequence fonctionnelle (GET /hosts ne lit que
<id>.json), mais rien ne les balaie. Un nettoyage des .tmp plus vieux qu'une heure au
demarrage reglerait la question.

4. frozenRequest et new Date()

L'utilitaire de test qui fige l'horloge n'interfere pas avec new Date(), ce qui est le
comportement voulu, mais ce n'est ecrit nulle part — un futur test qui s'appuierait sur
new Date() sous horloge figee aurait un resultat surprenant.

Criteres d'acceptation

  • Chaque point est soit corrige, soit documente comme choix assume avec sa raison
  • npm test reste vert
Constats non bloquants releves par `/pr-review` sur la pile #18-#23. Ils sont consignes ici parce qu'ils meurent avec les PRs fermees. Aucun n'a justifie de retarder le merge ; aucun n'est un defaut de comportement. ## 1. `loadAvg` accepte sans garantie de longueur `sanitizeSnapshot()` accepte `loadAvg: []` et le README ne promet aucune longueur. Le consommateur (la carte du dashboard, `la-compagnie-maximus#149`) fait `loadAvg.map(...).join(" / ")` — un tableau vide rend une chaine vide, pas une erreur, mais rien ne le documente. Soit garantir 3 elements cote serveur, soit l'ecrire dans le contrat. ## 2. `cleanString` conserve les caracteres de controle Choix delibere (ne pas mutiler une valeur legitime), mais la surete repose alors entierement sur l'echappement du consommateur — React echappe, donc c'est sur aujourd'hui. Ce n'est documente nulle part. Une ligne dans le README suffirait a rendre la dependance explicite. ## 3. Debris de fichier temporaire si le process meurt en cours d'ecriture L'ecriture atomique passe par un fichier temporaire puis `renameSync`. Un crash entre les deux laisse un `.tmp` dans `HOSTS_DIR`. Sans consequence fonctionnelle (`GET /hosts` ne lit que `<id>.json`), mais rien ne les balaie. Un nettoyage des `.tmp` plus vieux qu'une heure au demarrage reglerait la question. ## 4. `frozenRequest` et `new Date()` L'utilitaire de test qui fige l'horloge n'interfere pas avec `new Date()`, ce qui est le comportement voulu, mais ce n'est ecrit nulle part — un futur test qui s'appuierait sur `new Date()` sous horloge figee aurait un resultat surprenant. ## Criteres d'acceptation - [ ] Chaque point est soit corrige, soit documente comme choix assume avec sa raison - [ ] `npm test` reste vert
maximus added the
status:ready
type:refactor
source:human
labels 2026-08-16 18:28:49 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: maximus/vps-health-api#25
No description provided.