Dette passe 1, lot L4-bis : alertes même si l'écriture échoue, délai unique de 600 s, typage des capteurs, ordre des accusés MQTT #6

Open
Thomas wants to merge 3 commits from pr/dette-l4bis into pr/dette-l5
Member

Objet

Lot L4-bis de la passe 1 de dette : corrections demandées par l'utilisateur à la relecture du lot L4 (et du lot L5 pour l'ordre des accusés). Quatre points : les alertes partent même si l'écriture de la mesure échoue, un délai unique de 600 s pour tous les types d'alerte, le typage des capteurs (valeur rattachée au capteur de sa famille), l'ordre des accusés MQTT exigé par MQTT 3.1.1.

Avant déploiement : appliquer la migration 003 (scripts/db/apply-migration.sh, voir plus bas).

Corrections et preuves

Point Correction Preuve
1, alertes et écriture Alertes évaluées et tentées sur Inserted et aussi quand l'écriture de la mesure échoue (journal « mesure non enregistrée, alertes évaluées quand même ») ; Duplicate n'en lève aucune. Un échec passager est rejoué sans accusé, comme en L5. Lecture des capteurs en échec : nouvelle variante DataError::Sensors, ni écriture ni alerte. Chaque alerte légionellose écrite seule : le délai ne démarre que pour les sondes dont l'alerte est en base. aquaserveur/src/data_handler.rs, database/store.rs (store_temp_alert), tests/data_alert_rule.rs
2, délai unique ALERT_COOLDOWN = 600 s pour la légionellose, sw420 et tout autre type ; LEGIONELLA_COOLDOWN et hardware_cooldown supprimés ; AlertManager prend le même délai. Clés distinctes inchangées : (boîtier, sonde) et (boîtier, type). aquaserveur/src/alert_limits.rs, server.rs
3, typage des capteurs Module sensors.rs : famille ext ou int déduite du type (2, ds18b20, dsb18b20 ; 1, dht11, dht22) ou lue dans la nouvelle colonne famille ; la Nième valeur ext va au Nième capteur ext ; repli « par rang » si un capteur n'a pas de famille (pH, brome), avec avertissement. Seuils légionellose sur les seules sondes d'eau (ext, °C), numéro de sonde = colonne position comme les seuils du legacy. Colonne famille ENUM('ext','int') NULL INVISIBLE écrite au provisionnement. aquaserveur/src/sensors.rs, database/schema.rs, database/store.rs, provisioning/handler.rs, tests/sensor_typing.rs
4, ordre des accusés MQTT 3.1.1, section 4.6 [MQTT-4.6.0-2] : PUBACK dans l'ordre de réception des PUBLISH QoS 1. Ticket pris par le routeur à la réception, rendu par le worker avec l'accusé, séquenceur qui envoie chaque PUBACK quand les précédents sont rendus ; un ticket abandonné libère son numéro sans PUBACK (renvoi par le broker). Les écritures restent parallèles. aquaserveur/src/ack_order.rs, server.rs, ingest.rs, worker.rs, tests/mqtt_server_integration.rs

Conséquence du point 3, à connaître : pour les 271 boîtiers legacy de la base, le rattachement « par rang » stockait la première sonde d'eau sous le capteur interne ; chaque valeur va désormais à son capteur, et les alertes 14 portent capteur_id 2 à 7 (au lieu de 1 à 6).

Coût du point 4, à connaître : un message lent (verrou, rejeu) retient les accusés des messages reçus après lui ; au-delà de 20 messages non accusés (max_inflight_messages), le broker attend.

Tests et résultats

  • TDD : ALERT_COOLDOWN absent, sensors 11 échecs sur 11, ack_order 6 échecs sur 7, ordre des accusés sur broker réel rouge sur le code L5.
  • Contre-épreuves (neutralisation puis restauration vérifiée) : retour d'erreur avant les alertes, délai non démarré après insertion, rattachement par rang forcé, famille non écrite, séquenceur sans remise en ordre. Une contre-épreuve non concluante : arrêt sans attendre le séquenceur (garantie de conception, sans test qui la prouve).
  • Ajouts : unitaires alert_limits (2), sensors (12), ack_order (7), data_handler, schema ; 8 tests ignorés sur base et broker réels (data_alert_rule.rs, sensor_typing.rs, capteur_famille_migration.rs, mqtt_server_integration.rs).
  • Test modifié volontairement : a_frame_of_two_thousand_values_alerts_only_for_known_sensors attend une alerte au lieu de trois (la fixture 4000 n'a qu'une sonde d'eau ; c'est le défaut corrigé).
  • cargo fmt --check, cargo clippy ... -D warnings : OK.
  • cargo test --workspace : 350 passés, 0 échec, 76 ignorés (L5 : 332 et 68).
  • Tests ignorés sur MariaDB 10.11 jetable (dump, fixtures, migrations 001 et 002 ; 003 appliquée par son test) et deux Mosquitto 2.0 jetables : 76 passés, 0 échec, deux passages.
  • Binaire réel contre Mosquitto en log_type all : PUBACK Mid 2 (4000, écrit après) puis Mid 3 (4001, écrit avant). docker stop pendant un verrou de 6 s : arrêt en 4,3 s, 10 lignes, 10 PUBACK dans l'ordre, aucun renvoi au redémarrage.
  • Dépendance de test ajoutée : flume (déjà dans l'arbre par rumqttc).

Migration et actions de déploiement

  1. Appliquer la migration 003 avant le nouveau serveur : scripts/db/apply-migration.sh <conteneur> <fichier_env> scripts/db/migrations/003_capteur_famille.sql (droits ALTER et UPDATE, mot de passe par MYSQL_PWD). Idempotente : ajoute famille à chaque %_boitier_capteurs, remplit les lignes déductibles, ne modifie pas une famille déjà renseignée. Lire le bilan (tables_capteurs, colonnes_ajoutees, capteurs_types, capteurs_sans_famille, boitiers_rattaches_par_rang) ; d'après le dump, boitiers_rattaches_par_rang devrait valoir 0 en production.
  2. Sans la migration, le serveur déduit la famille de type (erreur 1054 gérée) et se comporte de la même façon pour le legacy ; la migration reste nécessaire pour renseigner à la main les familles non déductibles (pH, brome).
  3. Déployer le serveur.

Choix faits

  • Points 1 et 3 dans un même commit : la règle d'alerte et le rattachement typé réécrivent la même fonction de data_handler.rs.
  • Colonne INVISIBLE, comme mesure_empreinte (L4) : absente de SELECT * et des INSERT sans liste de colonnes de Laravel.
  • Séquenceur plutôt que INGEST_WORKERS=1 : ordre strict sans perdre les écritures parallèles.
  • Repli « par rang » journalisé plutôt que refus de trame quand une famille manque.

Hors périmètre, pistes

  • Alerte dédiée « échec d'écriture de mesure » : proposée en réflexion (compteur et supervision des journaux d'abord), non implémentée.
  • Seuils légionellose par sonde : fetch_legionella_thresholds lit encore une seule paire (LIMIT 1).
  • Règles en mémoire (AlertManager) : résultats non écrits, à revoir ou retirer.
  • Authentification et ACL Mosquitto : L6. Dockerfiles : L7.

Retour arrière

Revenir les 3 commits et redéployer l'image précédente. La colonne famille est invisible et nullable : l'ancien code et Laravel l'ignorent ; la retirer n'est pas nécessaire.


Pile de PR

Base de cette PR : pr/dette-l5. Fusion dans l'ordre de la pile ; apres chaque fusion, vigie.py retarget rebase la PR suivante sur main.

  1. #2 pr/dette-l2 : Dette passe 1, lot L2 : contrats partagés dans aquashared et client MQTT du boîtier
  2. #3 pr/dette-l3 : Dette passe 1, lot L3 : provisioning non destructif et identifiant validé à l'entrée MQTT
  3. #4 pr/dette-l4 : Dette passe 1, lot L4 : mesures, alertes et déduplication
  4. #5 pr/dette-l5 : Dette passe 1, lot L5 : boucle MQTT du serveur, livraison de la configuration et journaux
  5. #6 pr/dette-l4bis : Dette passe 1, lot L4-bis : alertes même si l'écriture échoue, délai unique de 600 s, typage des capteurs, ordre des accusés MQTT (cette PR)
  6. #7 pr/dette-l7 : Dette passe 1, lot L7 : builds reproductibles, dépendances, durcissement des conteneurs
  7. #8 pr/dette-l6 : Dette passe 1, lot L6 : authentification Mosquitto, ACL par compte, healthchecks réels, pile e2e isolée
  8. #9 pr/dette-l8 : Dette passe 1, lot L8 : tests non destructifs, fixtures synthétiques, script de tests sur base jetable

Commits du lot

  • 000adbf refactor: one 600 s alert delay for every alert kind
  • d1b0be0 feat: type frame values per sensor family and alert even when the write fails
  • dfd85f7 fix: send MQTT acknowledgements in reception order
## Objet Lot L4-bis de la passe 1 de dette : corrections demandées par l'utilisateur à la relecture du lot L4 (et du lot L5 pour l'ordre des accusés). Quatre points : les alertes partent même si l'écriture de la mesure échoue, un délai unique de 600 s pour tous les types d'alerte, le typage des capteurs (valeur rattachée au capteur de sa famille), l'ordre des accusés MQTT exigé par MQTT 3.1.1. **Avant déploiement : appliquer la migration 003** (`scripts/db/apply-migration.sh`, voir plus bas). ## Corrections et preuves | Point | Correction | Preuve | |---|---|---| | 1, alertes et écriture | Alertes évaluées et tentées sur `Inserted` et aussi quand l'écriture de la mesure échoue (journal « mesure non enregistrée, alertes évaluées quand même ») ; `Duplicate` n'en lève aucune. Un échec passager est rejoué sans accusé, comme en L5. Lecture des capteurs en échec : nouvelle variante `DataError::Sensors`, ni écriture ni alerte. Chaque alerte légionellose écrite seule : le délai ne démarre que pour les sondes dont l'alerte est en base. | `aquaserveur/src/data_handler.rs`, `database/store.rs` (`store_temp_alert`), `tests/data_alert_rule.rs` | | 2, délai unique | `ALERT_COOLDOWN = 600 s` pour la légionellose, `sw420` et tout autre type ; `LEGIONELLA_COOLDOWN` et `hardware_cooldown` supprimés ; `AlertManager` prend le même délai. Clés distinctes inchangées : `(boîtier, sonde)` et `(boîtier, type)`. | `aquaserveur/src/alert_limits.rs`, `server.rs` | | 3, typage des capteurs | Module `sensors.rs` : famille `ext` ou `int` déduite du type (`2`, `ds18b20`, `dsb18b20` ; `1`, `dht11`, `dht22`) ou lue dans la nouvelle colonne `famille` ; la Nième valeur `ext` va au Nième capteur `ext` ; repli « par rang » si un capteur n'a pas de famille (pH, brome), avec avertissement. Seuils légionellose sur les seules sondes d'eau (`ext`, °C), numéro de sonde = colonne `position` comme les seuils du legacy. Colonne `famille ENUM('ext','int') NULL INVISIBLE` écrite au provisionnement. | `aquaserveur/src/sensors.rs`, `database/schema.rs`, `database/store.rs`, `provisioning/handler.rs`, `tests/sensor_typing.rs` | | 4, ordre des accusés | MQTT 3.1.1, section 4.6 [MQTT-4.6.0-2] : PUBACK dans l'ordre de réception des PUBLISH QoS 1. Ticket pris par le routeur à la réception, rendu par le worker avec l'accusé, séquenceur qui envoie chaque PUBACK quand les précédents sont rendus ; un ticket abandonné libère son numéro sans PUBACK (renvoi par le broker). Les écritures restent parallèles. | `aquaserveur/src/ack_order.rs`, `server.rs`, `ingest.rs`, `worker.rs`, `tests/mqtt_server_integration.rs` | Conséquence du point 3, à connaître : pour les 271 boîtiers legacy de la base, le rattachement « par rang » stockait la première sonde d'eau sous le capteur interne ; chaque valeur va désormais à son capteur, et les alertes 14 portent `capteur_id` 2 à 7 (au lieu de 1 à 6). Coût du point 4, à connaître : un message lent (verrou, rejeu) retient les accusés des messages reçus après lui ; au-delà de 20 messages non accusés (`max_inflight_messages`), le broker attend. ## Tests et résultats - TDD : `ALERT_COOLDOWN` absent, `sensors` 11 échecs sur 11, `ack_order` 6 échecs sur 7, ordre des accusés sur broker réel rouge sur le code L5. - Contre-épreuves (neutralisation puis restauration vérifiée) : retour d'erreur avant les alertes, délai non démarré après insertion, rattachement par rang forcé, famille non écrite, séquenceur sans remise en ordre. Une contre-épreuve non concluante : arrêt sans attendre le séquenceur (garantie de conception, sans test qui la prouve). - Ajouts : unitaires `alert_limits` (2), `sensors` (12), `ack_order` (7), `data_handler`, `schema` ; 8 tests ignorés sur base et broker réels (`data_alert_rule.rs`, `sensor_typing.rs`, `capteur_famille_migration.rs`, `mqtt_server_integration.rs`). - Test modifié volontairement : `a_frame_of_two_thousand_values_alerts_only_for_known_sensors` attend une alerte au lieu de trois (la fixture 4000 n'a qu'une sonde d'eau ; c'est le défaut corrigé). - `cargo fmt --check`, `cargo clippy ... -D warnings` : OK. - `cargo test --workspace` : 350 passés, 0 échec, 76 ignorés (L5 : 332 et 68). - Tests ignorés sur MariaDB 10.11 jetable (dump, fixtures, migrations 001 et 002 ; 003 appliquée par son test) et deux Mosquitto 2.0 jetables : 76 passés, 0 échec, deux passages. - Binaire réel contre Mosquitto en `log_type all` : PUBACK `Mid 2` (4000, écrit après) puis `Mid 3` (4001, écrit avant). `docker stop` pendant un verrou de 6 s : arrêt en 4,3 s, 10 lignes, 10 PUBACK dans l'ordre, aucun renvoi au redémarrage. - Dépendance de test ajoutée : `flume` (déjà dans l'arbre par `rumqttc`). ## Migration et actions de déploiement 1. **Appliquer la migration 003 avant le nouveau serveur** : `scripts/db/apply-migration.sh <conteneur> <fichier_env> scripts/db/migrations/003_capteur_famille.sql` (droits `ALTER` et `UPDATE`, mot de passe par `MYSQL_PWD`). Idempotente : ajoute `famille` à chaque `%_boitier_capteurs`, remplit les lignes déductibles, ne modifie pas une famille déjà renseignée. Lire le bilan (`tables_capteurs`, `colonnes_ajoutees`, `capteurs_types`, `capteurs_sans_famille`, `boitiers_rattaches_par_rang`) ; d'après le dump, `boitiers_rattaches_par_rang` devrait valoir 0 en production. 2. Sans la migration, le serveur déduit la famille de `type` (erreur 1054 gérée) et se comporte de la même façon pour le legacy ; la migration reste nécessaire pour renseigner à la main les familles non déductibles (pH, brome). 3. Déployer le serveur. ## Choix faits - Points 1 et 3 dans un même commit : la règle d'alerte et le rattachement typé réécrivent la même fonction de `data_handler.rs`. - Colonne `INVISIBLE`, comme `mesure_empreinte` (L4) : absente de `SELECT *` et des `INSERT` sans liste de colonnes de Laravel. - Séquenceur plutôt que `INGEST_WORKERS=1` : ordre strict sans perdre les écritures parallèles. - Repli « par rang » journalisé plutôt que refus de trame quand une famille manque. ## Hors périmètre, pistes - Alerte dédiée « échec d'écriture de mesure » : proposée en réflexion (compteur et supervision des journaux d'abord), non implémentée. - Seuils légionellose par sonde : `fetch_legionella_thresholds` lit encore une seule paire (`LIMIT 1`). - Règles en mémoire (`AlertManager`) : résultats non écrits, à revoir ou retirer. - Authentification et ACL Mosquitto : L6. Dockerfiles : L7. ## Retour arrière Revenir les 3 commits et redéployer l'image précédente. La colonne `famille` est invisible et nullable : l'ancien code et Laravel l'ignorent ; la retirer n'est pas nécessaire. --- ### Pile de PR Base de cette PR : `pr/dette-l5`. Fusion dans l'ordre de la pile ; apres chaque fusion, `vigie.py retarget` rebase la PR suivante sur `main`. 1. #2 `pr/dette-l2` : Dette passe 1, lot L2 : contrats partagés dans aquashared et client MQTT du boîtier 2. #3 `pr/dette-l3` : Dette passe 1, lot L3 : provisioning non destructif et identifiant validé à l'entrée MQTT 3. #4 `pr/dette-l4` : Dette passe 1, lot L4 : mesures, alertes et déduplication 4. #5 `pr/dette-l5` : Dette passe 1, lot L5 : boucle MQTT du serveur, livraison de la configuration et journaux 5. #6 `pr/dette-l4bis` : Dette passe 1, lot L4-bis : alertes même si l'écriture échoue, délai unique de 600 s, typage des capteurs, ordre des accusés MQTT (cette PR) 6. #7 `pr/dette-l7` : Dette passe 1, lot L7 : builds reproductibles, dépendances, durcissement des conteneurs 7. #8 `pr/dette-l6` : Dette passe 1, lot L6 : authentification Mosquitto, ACL par compte, healthchecks réels, pile e2e isolée 8. #9 `pr/dette-l8` : Dette passe 1, lot L8 : tests non destructifs, fixtures synthétiques, script de tests sur base jetable ### Commits du lot - `000adbf` refactor: one 600 s alert delay for every alert kind - `d1b0be0` feat: type frame values per sensor family and alert even when the write fails - `dfd85f7` fix: send MQTT acknowledgements in reception order <!-- vigie:stack -->
Replace the per-type hardware delay table and the legionella delay with a
single documented ALERT_COOLDOWN, also used by the in-memory alert rules.
Keys stay per boitier and per probe or per hardware type, so an alert of
one kind never holds another.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UH3JcaVWsmFLC6kyEsVL18
Each ext and int value of a frame now goes to the sensor of the same family
and rank. The family comes from a new invisible famille column, written at
provisioning and added by the idempotent migration 003, or is deduced from
the type column (legacy codes 1 and 2, DS18B20, DHT11, DHT22). Legacy
boitiers, whose internal sensor sits first in position_trame, get their
values and legionella alerts on the right sensor; probes are numbered by
their position, like their thresholds. Legionella thresholds apply to water
temperature probes only. Without a family for every sensor, the former
rank mapping stays, with a warning.

Alert rule: alerts are raised on an inserted measure and also when the
measure write fails (error logged, failure still returned so a transient
error is replayed unacknowledged); a duplicate raises none. Each legionella
alert is written on its own; a failed alert write is logged with boitier,
probe and value and starts no delay, so the replay retries it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UH3JcaVWsmFLC6kyEsVL18
fix: send MQTT acknowledgements in reception order
Some checks failed
aquaprocess/revue-statique echec : semgrep (regles du depot)
dfd85f78db
MQTT 3.1.1 [MQTT-4.6.0-2] requires PUBACKs in the order the QoS 1 PUBLISH
packets were received, while the L5 workers write in parallel and acked
after their own write. The router now gives every QoS 1 or 2 message a
reception ticket; workers hand the ticket back once the message is
processed, and a sequencer task sends the acks in reception order. Writes
stay parallel. A ticket dropped without ack (full queue, shutdown during a
database outage) frees its number without a PUBACK, so the broker
redelivers that message and later acks are never stuck. At shutdown the
sequencer drains before DISCONNECT.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UH3JcaVWsmFLC6kyEsVL18
Thomas left a comment

Revue statique Vigie : PR #6

Tete dfd85f78db, base pr/dette-l5 (74df6f6fa5), 3 commit(s), 19 fichier(s) ajoutes ou modifies.

Statut aquaprocess/revue-statique : failure (semgrep (regles du depot))

Constats : 1 bloquant, 1 majeur, 7 mineur, 0 style. Commentaires en ligne : 2 (mode important, plafond 10).

Verifications automatiques

Verification Resultat Detail
cargo fmt --check success aucune difference
cargo clippy -D warnings success aucun avertissement
cargo test --workspace success 350 passes, 0 echec(s), 76 ignores
tests ignores (base jetable) success 76 passes, 0 echec(s) ; brokers MQTT : mosquitto, mosquitto-nolimit ; dump charge en 35 s ; 1 fichier(s) de fixtures ; migrations du depot : 001_mesure_empreinte.sql, 002_config_delivery.sql, 003_capteur_famille.sql
cargo audit attention introduite(s) : aucune ; deja presente(s) sur la base : RUSTSEC-2026-0049 (rustls-webpki), RUSTSEC-2026-0098 (rustls-webpki), RUSTSEC-2026-0099 (rustls-webpki), RUSTSEC-2026-0104 (rustls-webpki), RUSTSEC-2026-0258 (h2), RUSTSEC-2026-0285 (rustls), 2 ignoree(s) par audit.toml
semgrep (regles du depot) failure 16 fichier(s) ; lignes ajoutees : 12 resultat(s) hors tests, tests : no-unwrap-in-production x52, sqlx-no-format-in-query x9 ; 70 sur lignes inchangees, dont 1 de severite bloquante
gitleaks (plage de commits) success 3 commit(s) analyses, aucune fuite
format des commits success 3 commit(s) conformes
emoji dans le code ajoute success 2037 ligne(s) ajoutee(s), aucune

Analyse qualitative

Verdict : les quatre corrections demandées sont faites et bien testées. Statut failure à cause d'un seul point mécanique (règle semgrep sqlx-no-format-in-query sur une ligne de production, voir plus bas), facile à lever. 1 majeur de conception (retenue globale des accusés derrière un message en rejeu sans fin), le reste en mineurs.

Plage revue : 74df6f6..dfd85f7 (3 commits du lot), sans les lots précédents.

Ce que cette PR corrige parmi les constats des PR #2 à #5

Constat d'origine État
#4 majeur data_handler.rs:239 : alertes court-circuitées par un échec d'écriture corrigé : alertes évaluées et tentées sur échec d'écriture, Duplicate seul les écarte (data_handler.rs:282 à 307)
#4 mineur alert_limits.rs:41 : trois sources de délai corrigé : ALERT_COOLDOWN unique (alert_limits.rs:49), repris par server.rs
#4 mineur store.rs:272 : alertes légionellose hors transaction, délai non démarré pour les sondes écrites corrigé : une écriture par sonde, délai démarré pour chaque sonde écrite (store_temp_alert, store.rs:342)
#5 mineur worker.rs:172 : Duplicate après une écriture dont la réponse s'est perdue, alertes jamais parties corrigé pour ce cas (alertes tentées au premier passage) ; reste : une alerte en échec passager sur une mesure écrite est accusée sans rejeu (voir data_handler.rs:34)

Déjà relevé dans la pile, toujours présent (non repris en constat ici)

  • #5 bloquant worker.rs:183 (désormais worker.rs:179, inchangé) : une trame refusée journalise error = %e, qui recopie un fragment du payload (Invalid ext sensor value: '...'). Ce lot ne le corrige pas ; il reste bloquant pour la pile.
  • #5 majeur server.rs:282 : un message refusé (file pleine) ou abandonné garde sa place inflight, aucune reconnexion n'est provoquée. Le séquenceur le reconduit tel quel (Release::Skip).
  • #5 mineur worker.rs:99 : rejeu sans plafond des erreurs passagères. Aggravé par ce lot, voir le majeur ack_order.rs:30.
  • #5 mineur worker.rs:121 : accusé d'un message de l'ancienne connexion avec son ancien pkid ; le séquenceur garde ces tickets d'une connexion à l'autre, même risque.
  • #5 mineur server.rs:177 : la tâche de livraison garde un clone des files, le vidage d'arrêt attend souvent 8 s.
  • #4 majeur data_handler.rs:234 (désormais :275) : trame hors fenêtre d'epoch refusée sans alerte.
  • #4 majeur config_ack_handler.rs:127 : plafond par minute partagé entre l'alerte 15 et les alertes sanitaires.
  • #4 mineurs data_handler.rs:248 (sw420 perdu sur un Duplicate) et store.rs:55 (toute violation 1062 classée doublon).

Contrôle automatique bloquant

aquaserveur/src/provisioning/handler.rs:456 : sqlx::query(&format!("ALTER TABLE {table_capteurs} ADD COLUMN IF NOT EXISTS {FAMILLE_COLUMN_DDL}")). Les deux parties dynamiques sont sûres (nom de table dérivé d'un BoitierId entier, constante), mais la règle du dépôt bloque toute requête construite dans l'appel. Correction minimale, celle du lot L7 pour les tests : let sql = format!(...); sqlx::query(&sql). Attention : la fusion de L7 (PR #7) ne la reprend pas, la ligne est identique à la tête de #7.

Correction

  • Règle d'alerte : conforme à la décision. DataError::Sensors (lecture des capteurs en échec) est bien distinguée de l'échec d'écriture et rejouée si la base est indisponible. Rejeu après échec passager : le délai par sonde évite la seconde alerte, prouvé sur base réelle par a_successful_replay_after_a_transient_failure_raises_no_second_alert.
  • Délai unique : une constante, des clés distinctes, deux tables ; tests every_alert_kind_waits_the_same_single_delay et an_alert_of_one_kind_does_not_hold_another_kind justes.
  • Typage : assign_typed est correct (Nième ext au Nième capteur ext, position comme numéro de sonde, surplus compté). Lecture tolérante à une table sans famille (erreur 1054 seule, autre erreur rendue). Le repli « par rang » garde l'ancien défaut pour les sondes non température (voir sensors.rs:185).
  • Ordre des accusés : AckOrder est simple et juste ; ticket rendu une seule fois (consommé par ack, Drop sinon) ; canal non borné justifié (borné en pratique par max_inflight_messages). Arrêt : routeur, puis workers, puis séquenceur, puis DISCONNECT, dans cet ordre. Le coût (un message lent retient tous les accusés suivants) est documenté ; c'est sa combinaison avec le rejeu sans plafond qui le rend grave (ack_order.rs:30).
  • Migration 003 : idempotente, ne modifie pas une famille renseignée, bilan utile ; même faiblesse d'échappement que 001 (003_capteur_famille.sql:75). Colonne INVISIBLE identique au caractère près à FAMILLE_COLUMN_DDL, vérifiée par test.
  • Budget d'alertes consommé par une écriture en échec (data_handler.rs:382, ligne déplacée, donc non commentée en ligne).

Sécurité

Aucune donnée personnelle ajoutée aux journaux : les nouveaux journaux portent boitier_id, probe, alert_list_id, value et la nature de l'erreur. Les identifiants SQL dynamiques ajoutés viennent d'un BoitierId validé ou de constantes. gitleaks : rien.

Cohérence avec le compte rendu

Références vérifiées à la tête dfd85f7, toutes exactes : alert_limits.rs:49, schema.rs:35, store.rs:186, :235, :342, data_handler.rs:260, :311, :344, sensors.rs:75, :106, :137, :177, ack_order.rs:63, :133, :174, server.rs:181, :224, ingest.rs:134, worker.rs:85, handler.rs:457. Résultats reproduits par Vigie : cargo test 350 passés, 76 ignorés ; 76 tests ignorés passés sur base et brokers jetables, migration 003 appliquée par le dépôt.

Écart : « le rejeu la tente de nouveau » ne vaut que pour une mesure non écrite (data_handler.rs:34).

Tests

TDD et contre-épreuves solides, y compris côté broker (log_type all). Limites : l'attente du séquenceur avant DISCONNECT n'a aucun test qui la prouve (contre-épreuve non concluante, reconnue par le compte rendu) ; aucun test du repli « par rang » avec une sonde pH en ext ; aucun test d'un rejeu long qui retient les accusés des autres boîtiers.

Questions ouvertes

  • Quelle durée maximale de rejeu d'un message accepte-t-on avant d'abandonner son ticket et de forcer une reconnexion ?
  • Faut-il, en repli « par rang », filtrer la légionellose sur l'unité °C du capteur de même rang ?

Autres constats (non publies en ligne)

Gravite Emplacement Source Constat
mineur aquaserveur/src/ack_order.rs:26 revue « le broker renvoie ce message à la reconnexion » : rien ne provoque cette reconnexion. Le numéro est libéré côté serveur, mais la place inflight du broker reste prise jusqu'à la prochaine coupure (même constat que #5, server.rs:282, non corrigé par ce lot). De plus, accuser n+1 alors que n ne le sera jamais sur cette connexion s'écarte de la lecture stricte de MQTT-4.6.0-2 que ce module invoque ; Mosquitto le tolère, mais le commentaire devrait le dire. Piste : forcer une reconnexion après un Skip hors arrêt.
mineur aquaserveur/src/ack_order.rs:271 semgrep semgrep no-unwrap-in-production (WARNING) : .unwrap() ou .expect() peut paniquer en production. 10 occurrence(s) sur lignes ajoutees (271, 272, 296, 297, 298, 315, 326, 326, 339, 340), y compris d'eventuels modules de test internes.
mineur aquaserveur/src/ack_order.rs:337 semgrep semgrep tokio-spawn-fire-and-forget (WARNING) : tokio::spawn() sans stocker le JoinHandle (fire-and-forget). 1 occurrence(s) sur lignes ajoutees (337), y compris d'eventuels modules de test internes.
mineur aquaserveur/src/data_handler.rs:34 revue « le rejeu la tente de nouveau » ne vaut que si l'écriture de la mesure a échoué de façon passagère. Mesure Inserted puis alerte légionellose ou vibration en échec passager : l'erreur est journalisée, la trame est rendue Processed et accusée, sans rejeu ; l'alerte n'est retentée qu'à la prochaine trame hors seuil (c'est la seconde moitié du constat #5 worker.rs:172, partiellement traitée). Acceptable si la condition persiste, mais à écrire tel quel dans l'en-tête, ou à traiter en rendant Step::Retry quand une alerte échoue en erreur passagère (le délai déjà démarré des autres sondes évite les doublons).
mineur aquaserveur/src/data_handler.rs:382 revue admit_legionella consomme le plafond par minute avant l'écriture ; une alerte dont l'écriture échoue ne démarre pas de délai mais garde l'insertion consommée. Pendant le rejeu d'une écriture de mesure en échec (1 s doublé jusqu'à 30 s, sans plafond), chaque passage reconsomme le budget : il peut s'épuiser sans aucune alerte en base et refuser ensuite l'alerte 15 ou une autre sonde. Piste : ne consommer le budget qu'après une écriture réussie, ou le rendre en cas d'échec.
mineur aquaserveur/src/sensors.rs:185 revue En repli « par rang », toutes les valeurs ext sont évaluées en légionellose, sans contrôle d'unité : une sonde pH ou brome déclarée en ext (cas cité dans le compte rendu, firstco-C.sh) produit des alertes 14 sur une valeur qui n'est pas une température, ce que l'en-tête du module (ligne 28) annonce justement exclu. Le numéro de sonde y est aussi le rang et non position : le capteur_id des alertes d'un même boîtier change le jour où un opérateur renseigne famille. Piste : en repli, ne garder que les valeurs dont le capteur de même rang est en °C, et numéroter par position quand elle existe.
mineur scripts/db/migrations/003_capteur_famille.sql:75 revue Même construction que la migration 001 (constat #4, 001_mesure_empreinte.sql:82) : le nom de table lu dans information_schema est concaténé entre accents graves sans échappement dans trois EXECUTE IMMEDIATE (lignes 75, 85, 93), exécutés avec les droits ALTER et UPDATE. Défense simple : REPLACE(t, '', '``')ou refus parSIGNALd'un nom hors^[0-9A-Za-z_]+$`.

Limites

Revue publiee en COMMENT : l'auteur des PR et le relecteur sont le meme compte, Forgejo refuse APPROVE et REQUEST_CHANGES. Le blocage s'exprime par le statut de commit aquaprocess/revue-statique. Analyse statique et tests automatises seulement, sans fusion ni deploiement.

## Revue statique Vigie : PR #6 Tete `dfd85f78db`, base `pr/dette-l5` (`74df6f6fa5`), 3 commit(s), 19 fichier(s) ajoutes ou modifies. **Statut `aquaprocess/revue-statique` : failure** (semgrep (regles du depot)) Constats : 1 bloquant, 1 majeur, 7 mineur, 0 style. Commentaires en ligne : 2 (mode `important`, plafond 10). ### Verifications automatiques | Verification | Resultat | Detail | |---|---|---| | cargo fmt --check | success | aucune difference | | cargo clippy -D warnings | success | aucun avertissement | | cargo test --workspace | success | 350 passes, 0 echec(s), 76 ignores | | tests ignores (base jetable) | success | 76 passes, 0 echec(s) ; brokers MQTT : mosquitto, mosquitto-nolimit ; dump charge en 35 s ; 1 fichier(s) de fixtures ; migrations du depot : 001_mesure_empreinte.sql, 002_config_delivery.sql, 003_capteur_famille.sql | | cargo audit | attention | introduite(s) : aucune ; deja presente(s) sur la base : RUSTSEC-2026-0049 (rustls-webpki), RUSTSEC-2026-0098 (rustls-webpki), RUSTSEC-2026-0099 (rustls-webpki), RUSTSEC-2026-0104 (rustls-webpki), RUSTSEC-2026-0258 (h2), RUSTSEC-2026-0285 (rustls), 2 ignoree(s) par audit.toml | | semgrep (regles du depot) | failure | 16 fichier(s) ; lignes ajoutees : 12 resultat(s) hors tests, tests : no-unwrap-in-production x52, sqlx-no-format-in-query x9 ; 70 sur lignes inchangees, dont 1 de severite bloquante | | gitleaks (plage de commits) | success | 3 commit(s) analyses, aucune fuite | | format des commits | success | 3 commit(s) conformes | | emoji dans le code ajoute | success | 2037 ligne(s) ajoutee(s), aucune | ### Analyse qualitative Verdict : les quatre corrections demandées sont faites et bien testées. Statut `failure` à cause d'un seul point mécanique (règle semgrep `sqlx-no-format-in-query` sur une ligne de production, voir plus bas), facile à lever. 1 majeur de conception (retenue globale des accusés derrière un message en rejeu sans fin), le reste en mineurs. Plage revue : `74df6f6..dfd85f7` (3 commits du lot), sans les lots précédents. ### Ce que cette PR corrige parmi les constats des PR #2 à #5 | Constat d'origine | État | |---|---| | #4 majeur `data_handler.rs:239` : alertes court-circuitées par un échec d'écriture | **corrigé** : alertes évaluées et tentées sur échec d'écriture, `Duplicate` seul les écarte (`data_handler.rs:282` à `307`) | | #4 mineur `alert_limits.rs:41` : trois sources de délai | **corrigé** : `ALERT_COOLDOWN` unique (`alert_limits.rs:49`), repris par `server.rs` | | #4 mineur `store.rs:272` : alertes légionellose hors transaction, délai non démarré pour les sondes écrites | **corrigé** : une écriture par sonde, délai démarré pour chaque sonde écrite (`store_temp_alert`, `store.rs:342`) | | #5 mineur `worker.rs:172` : `Duplicate` après une écriture dont la réponse s'est perdue, alertes jamais parties | **corrigé** pour ce cas (alertes tentées au premier passage) ; **reste** : une alerte en échec passager sur une mesure écrite est accusée sans rejeu (voir `data_handler.rs:34`) | ### Déjà relevé dans la pile, toujours présent (non repris en constat ici) - **#5 bloquant `worker.rs:183`** (désormais `worker.rs:179`, inchangé) : une trame refusée journalise `error = %e`, qui recopie un fragment du payload (`Invalid ext sensor value: '...'`). Ce lot ne le corrige pas ; il reste bloquant pour la pile. - #5 majeur `server.rs:282` : un message refusé (file pleine) ou abandonné garde sa place inflight, aucune reconnexion n'est provoquée. Le séquenceur le reconduit tel quel (`Release::Skip`). - #5 mineur `worker.rs:99` : rejeu sans plafond des erreurs passagères. **Aggravé** par ce lot, voir le majeur `ack_order.rs:30`. - #5 mineur `worker.rs:121` : accusé d'un message de l'ancienne connexion avec son ancien pkid ; le séquenceur garde ces tickets d'une connexion à l'autre, même risque. - #5 mineur `server.rs:177` : la tâche de livraison garde un clone des files, le vidage d'arrêt attend souvent 8 s. - #4 majeur `data_handler.rs:234` (désormais `:275`) : trame hors fenêtre d'epoch refusée sans alerte. - #4 majeur `config_ack_handler.rs:127` : plafond par minute partagé entre l'alerte 15 et les alertes sanitaires. - #4 mineurs `data_handler.rs:248` (`sw420` perdu sur un `Duplicate`) et `store.rs:55` (toute violation 1062 classée doublon). ### Contrôle automatique bloquant `aquaserveur/src/provisioning/handler.rs:456` : `sqlx::query(&format!("ALTER TABLE {table_capteurs} ADD COLUMN IF NOT EXISTS {FAMILLE_COLUMN_DDL}"))`. Les deux parties dynamiques sont sûres (nom de table dérivé d'un `BoitierId` entier, constante), mais la règle du dépôt bloque toute requête construite dans l'appel. Correction minimale, celle du lot L7 pour les tests : `let sql = format!(...); sqlx::query(&sql)`. Attention : la fusion de L7 (PR #7) ne la reprend pas, la ligne est identique à la tête de #7. ### Correction - Règle d'alerte : conforme à la décision. `DataError::Sensors` (lecture des capteurs en échec) est bien distinguée de l'échec d'écriture et rejouée si la base est indisponible. Rejeu après échec passager : le délai par sonde évite la seconde alerte, prouvé sur base réelle par `a_successful_replay_after_a_transient_failure_raises_no_second_alert`. - Délai unique : une constante, des clés distinctes, deux tables ; tests `every_alert_kind_waits_the_same_single_delay` et `an_alert_of_one_kind_does_not_hold_another_kind` justes. - Typage : `assign_typed` est correct (Nième `ext` au Nième capteur `ext`, `position` comme numéro de sonde, surplus compté). Lecture tolérante à une table sans `famille` (erreur 1054 seule, autre erreur rendue). Le repli « par rang » garde l'ancien défaut pour les sondes non température (voir `sensors.rs:185`). - Ordre des accusés : `AckOrder` est simple et juste ; ticket rendu une seule fois (consommé par `ack`, `Drop` sinon) ; canal non borné justifié (borné en pratique par `max_inflight_messages`). Arrêt : routeur, puis workers, puis séquenceur, puis `DISCONNECT`, dans cet ordre. Le coût (un message lent retient tous les accusés suivants) est documenté ; c'est sa combinaison avec le rejeu sans plafond qui le rend grave (`ack_order.rs:30`). - Migration 003 : idempotente, ne modifie pas une famille renseignée, bilan utile ; même faiblesse d'échappement que 001 (`003_capteur_famille.sql:75`). Colonne `INVISIBLE` identique au caractère près à `FAMILLE_COLUMN_DDL`, vérifiée par test. - Budget d'alertes consommé par une écriture en échec (`data_handler.rs:382`, ligne déplacée, donc non commentée en ligne). ### Sécurité Aucune donnée personnelle ajoutée aux journaux : les nouveaux journaux portent `boitier_id`, `probe`, `alert_list_id`, `value` et la nature de l'erreur. Les identifiants SQL dynamiques ajoutés viennent d'un `BoitierId` validé ou de constantes. gitleaks : rien. ### Cohérence avec le compte rendu Références vérifiées à la tête `dfd85f7`, toutes exactes : `alert_limits.rs:49`, `schema.rs:35`, `store.rs:186`, `:235`, `:342`, `data_handler.rs:260`, `:311`, `:344`, `sensors.rs:75`, `:106`, `:137`, `:177`, `ack_order.rs:63`, `:133`, `:174`, `server.rs:181`, `:224`, `ingest.rs:134`, `worker.rs:85`, `handler.rs:457`. Résultats reproduits par Vigie : `cargo test` 350 passés, 76 ignorés ; 76 tests ignorés passés sur base et brokers jetables, migration 003 appliquée par le dépôt. Écart : « le rejeu la tente de nouveau » ne vaut que pour une mesure non écrite (`data_handler.rs:34`). ### Tests TDD et contre-épreuves solides, y compris côté broker (`log_type all`). Limites : l'attente du séquenceur avant `DISCONNECT` n'a aucun test qui la prouve (contre-épreuve non concluante, reconnue par le compte rendu) ; aucun test du repli « par rang » avec une sonde pH en `ext` ; aucun test d'un rejeu long qui retient les accusés des autres boîtiers. ### Questions ouvertes - Quelle durée maximale de rejeu d'un message accepte-t-on avant d'abandonner son ticket et de forcer une reconnexion ? - Faut-il, en repli « par rang », filtrer la légionellose sur l'unité °C du capteur de même rang ? ### Autres constats (non publies en ligne) | Gravite | Emplacement | Source | Constat | |---|---|---|---| | mineur | `aquaserveur/src/ack_order.rs:26` | revue | « le broker renvoie ce message à la reconnexion » : rien ne provoque cette reconnexion. Le numéro est libéré côté serveur, mais la place inflight du broker reste prise jusqu'à la prochaine coupure (même constat que #5, server.rs:282, non corrigé par ce lot). De plus, accuser n+1 alors que n ne le sera jamais sur cette connexion s'écarte de la lecture stricte de MQTT-4.6.0-2 que ce module invoque ; Mosquitto le tolère, mais le commentaire devrait le dire. Piste : forcer une reconnexion après un Skip hors arrêt. | | mineur | `aquaserveur/src/ack_order.rs:271` | semgrep | semgrep `no-unwrap-in-production` (WARNING) : `.unwrap()` ou `.expect()` peut paniquer en production. 10 occurrence(s) sur lignes ajoutees (271, 272, 296, 297, 298, 315, 326, 326, 339, 340), y compris d'eventuels modules de test internes. | | mineur | `aquaserveur/src/ack_order.rs:337` | semgrep | semgrep `tokio-spawn-fire-and-forget` (WARNING) : `tokio::spawn()` sans stocker le JoinHandle (fire-and-forget). 1 occurrence(s) sur lignes ajoutees (337), y compris d'eventuels modules de test internes. | | mineur | `aquaserveur/src/data_handler.rs:34` | revue | « le rejeu la tente de nouveau » ne vaut que si l'écriture de la mesure a échoué de façon passagère. Mesure `Inserted` puis alerte légionellose ou vibration en échec passager : l'erreur est journalisée, la trame est rendue `Processed` et accusée, sans rejeu ; l'alerte n'est retentée qu'à la prochaine trame hors seuil (c'est la seconde moitié du constat #5 worker.rs:172, partiellement traitée). Acceptable si la condition persiste, mais à écrire tel quel dans l'en-tête, ou à traiter en rendant `Step::Retry` quand une alerte échoue en erreur passagère (le délai déjà démarré des autres sondes évite les doublons). | | mineur | `aquaserveur/src/data_handler.rs:382` | revue | `admit_legionella` consomme le plafond par minute avant l'écriture ; une alerte dont l'écriture échoue ne démarre pas de délai mais garde l'insertion consommée. Pendant le rejeu d'une écriture de mesure en échec (1 s doublé jusqu'à 30 s, sans plafond), chaque passage reconsomme le budget : il peut s'épuiser sans aucune alerte en base et refuser ensuite l'alerte 15 ou une autre sonde. Piste : ne consommer le budget qu'après une écriture réussie, ou le rendre en cas d'échec. | | mineur | `aquaserveur/src/sensors.rs:185` | revue | En repli « par rang », toutes les valeurs `ext` sont évaluées en légionellose, sans contrôle d'unité : une sonde pH ou brome déclarée en `ext` (cas cité dans le compte rendu, `firstco-C.sh`) produit des alertes 14 sur une valeur qui n'est pas une température, ce que l'en-tête du module (ligne 28) annonce justement exclu. Le numéro de sonde y est aussi le rang et non `position` : le `capteur_id` des alertes d'un même boîtier change le jour où un opérateur renseigne `famille`. Piste : en repli, ne garder que les valeurs dont le capteur de même rang est en °C, et numéroter par `position` quand elle existe. | | mineur | `scripts/db/migrations/003_capteur_famille.sql:75` | revue | Même construction que la migration 001 (constat #4, 001_mesure_empreinte.sql:82) : le nom de table lu dans information_schema est concaténé entre accents graves sans échappement dans trois `EXECUTE IMMEDIATE` (lignes 75, 85, 93), exécutés avec les droits ALTER et UPDATE. Défense simple : `REPLACE(t, '`', '``')` ou refus par `SIGNAL` d'un nom hors `^[0-9A-Za-z_]+$`. | ### Limites Revue publiee en `COMMENT` : l'auteur des PR et le relecteur sont le meme compte, Forgejo refuse APPROVE et REQUEST_CHANGES. Le blocage s'exprime par le statut de commit `aquaprocess/revue-statique`. Analyse statique et tests automatises seulement, sans fusion ni deploiement. <!-- vigie:review -->
@ -0,0 +27,4 @@
//! Aucun numéro ne reste donc en attente pour toujours.
//!
//! ## Coût
//! Un message lent (base en attente d'un verrou, rejeu d'une erreur
Author
Member

[majeur] Le coût est documenté, mais combiné au rejeu sans plafond des erreurs passagères (process_until_done, worker.rs:105, relevé en mineur sur #5 à worker.rs:99), il change d'échelle : un seul message en rejeu (verrou sur la table d'un boîtier, erreur 1205 répétée, ALTER Laravel, Protocol classé passager) retient désormais les PUBACK de tous les boîtiers. Au-delà de 20 messages non accusés (max_inflight_messages), Mosquitto cesse d'envoyer tout QoS 1 au serveur : la réception entière s'arrête, sans fin, alors qu'en L5 seul le worker concerné était suspendu. Piste : borner la durée de rejeu d'un message (par exemple 60 s), puis abandonner son ticket (Skip) et provoquer une reconnexion pour que le broker le renvoie, avec un compteur et un journal error ; ou isoler un boîtier bloqué comme le propose la section 9 du compte rendu.

Source : revue.

**[majeur]** Le coût est documenté, mais combiné au rejeu sans plafond des erreurs passagères (`process_until_done`, worker.rs:105, relevé en mineur sur #5 à worker.rs:99), il change d'échelle : un seul message en rejeu (verrou sur la table d'un boîtier, erreur 1205 répétée, ALTER Laravel, `Protocol` classé passager) retient désormais les PUBACK de tous les boîtiers. Au-delà de 20 messages non accusés (`max_inflight_messages`), Mosquitto cesse d'envoyer tout QoS 1 au serveur : la réception entière s'arrête, sans fin, alors qu'en L5 seul le worker concerné était suspendu. Piste : borner la durée de rejeu d'un message (par exemple 60 s), puis abandonner son ticket (Skip) et provoquer une reconnexion pour que le broker le renvoie, avec un compteur et un journal `error` ; ou isoler un boîtier bloqué comme le propose la section 9 du compte rendu. _Source : revue._
@ -448,2 +453,4 @@
);
sqlx::query(&create_capteurs).execute(pool).await?;
// Table laissée par une reprise incomplète, antérieure à la colonne
sqlx::query(&format!(
Author
Member

[bloquant] semgrep sqlx-no-format-in-query (ERROR) : Construction SQL dynamique détectée avec sqlx.

Source : semgrep.

**[bloquant]** semgrep `sqlx-no-format-in-query` (ERROR) : Construction SQL dynamique détectée avec sqlx. _Source : semgrep._
Some checks failed
aquaprocess/revue-statique echec : semgrep (regles du depot)
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin pr/dette-l4bis:pr/dette-l4bis
git switch pr/dette-l4bis

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch pr/dette-l5
git merge --no-ff pr/dette-l4bis
git switch pr/dette-l4bis
git rebase pr/dette-l5
git switch pr/dette-l5
git merge --ff-only pr/dette-l4bis
git switch pr/dette-l4bis
git rebase pr/dette-l5
git switch pr/dette-l5
git merge --no-ff pr/dette-l4bis
git switch pr/dette-l5
git merge --squash pr/dette-l4bis
git switch pr/dette-l5
git merge --ff-only pr/dette-l4bis
git switch pr/dette-l5
git merge pr/dette-l4bis
git push origin pr/dette-l5
Sign in to join this conversation.
No reviewers
No labels
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
AcadeNice/Acquarefactoring-Thomas!6
No description provided.