Dette passe 1, lot L2 : contrats partagés dans aquashared et client MQTT du boîtier #2
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "pr/dette-l2"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Objet
Lot L2 de la passe 1 de dette : les contrats MQTT échangés entre le boîtier et le serveur sont définis une seule fois dans
aquashared, et le client MQTT du boîtier est reconstruit sur ces contrats. Le format de référence defirstcoest l'enveloppe{"boitier": {...}, "client": {...}}du fichierconfig_boitier.jsonde la passerelle historique.Dettes traitées et preuves
firstcomark_firstco_donene modifie queboitier.first_connect; écriture atomique (fichier temporaire puis renommage)aquaboitier-embedded/src/firstco/mod.rs,src/persist.rs; avant correction, le test de contrat échouait avecmissing field boitierfirstcoConnAck: abonnement àfirstco_ACK-{id}etmaj_serv-{id}, puis publication ; réessai jusqu'à un ACK valide, 10 s doublés jusqu'à 300 saquaboitier-embedded/src/session.rsfirstcoaccepté seulement sistatus = oketid_boitierégal à l'id du boîtier ;maj_serv: commandes 0 ou 1,version_configdécimale canonique, version plus ancienne refusée (y compris après redémarrage) ; refus acquitté par un ACKerrorsrc/session.rs,src/config_sync/mod.rs,aquashared/src/config.rsConnAck, reconnexion avecBackoffde 1 s à 60 saquaboitier-embedded/src/main.rsMQTT_CLIENT_IDs'il est fourni, sinonaquaboitier-{id};entrypoint.shne force plus le nom d'hôtesrc/settings.rs,entrypoint.shappsur/dataet/app/conf, cheminsBOITIER_CONFIG_PATHetBOITIER_STATE_PATHuniques ; identité absente : arrêt (code 1), saufBOITIER_DEV_MODE=1aquaboitier-embedded/Dockerfile,Dockerfile.prod,src/settings.rs,src/main.rsContenu d'
aquashared:id(BoitierId, analyse canonique),topics(constantes, enumTopic),provisioning(typesfirstcodéplacés du serveur,FirstConnectAck),config(BoitierConfig, validation,ConfigAck),frame(build_custom_framedéplacé du boîtier),mqtt(limites de taille),backoff. Le serveur et le boîtier réexportent ces types : plus aucune duplication.Côté serveur, la logique de provisioning n'est pas modifiée : désérialisation par les types partagés, ACK
firstcoproduit par le type partagé (format identique à l'octet près,{"status":"ok","id_boitier":N}).Tests et résultats
aquashared, 22 sur 63 pour la session du boîtier).aquashared; 63 tests unitaires dans le boîtier ;aquaserveur/tests/contract_boitier_serveur.rs, 9 tests de contrat entre crates (le payload produit par la session du boîtier est routé et analysé par le serveur, et inversement pour les ACK etmaj_serv).cargo fmt --checketcargo clippy --all-targets --all-features --workspace -D warnings: OK.cargo test --workspace: 233 passés, 0 échec, 39 ignorés (159 passés avant le lot).maj_serv, reconnexion après redémarrage du broker, refus des ACK forgés.Migrations et actions de déploiement
mode_boost,generator_state_aetgenerator_state_bdeviennent obligatoires dansmaj_serv. Le serveur et le boîtier de ce lot doivent être déployés ensemble.docker compose -f docker-compose.dev.yml up -d --build boitier-dev) pour bénéficier deBOITIER_DEV_MODE=1, puis relancere2e/boitier/run-all-boitier.sh.Choix faits
firstco(global) :firstco-{id}est défini ici mais routé par le serveur seulement à partir du lot L3.first_connectvaut"ok"après ACK (sémantique « non vide », comme la passerelle).error: le serveur crée l'alerte 15 (plafond d'insertions prévu en L4).firstcosans limite de tentatives, délai plafonné à 300 s.Hors périmètre
Validation canonique de l'id côté serveur et routage par
Topic::parse(L3) ; limites de taille côté serveur et broker (L5) ; documentation générale (L9). Inchangés et préexistants : 6 tests de la featureaquaboitier-phase-green(échouent à l'identique sur674dc89), avertissement[profile.release]du boîtier.Retour arrière
Revenir les 4 commits du lot (
git revert) et redéployer ensemble serveur et boîtier : aucune donnée ni schéma n'est modifié par ce lot.Pile de PR
Base de cette PR :
main. Fusion dans l'ordre de la pile ; apres chaque fusion,vigie.py retargetrebase la PR suivante surmain.pr/dette-l2: Dette passe 1, lot L2 : contrats partagés dans aquashared et client MQTT du boîtier (cette PR)pr/dette-l3: Dette passe 1, lot L3 : provisioning non destructif et identifiant validé à l'entrée MQTTpr/dette-l4: Dette passe 1, lot L4 : mesures, alertes et déduplicationpr/dette-l5: Dette passe 1, lot L5 : boucle MQTT du serveur, livraison de la configuration et journauxpr/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 MQTTpr/dette-l7: Dette passe 1, lot L7 : builds reproductibles, dépendances, durcissement des conteneurspr/dette-l6: Dette passe 1, lot L6 : authentification Mosquitto, ACL par compte, healthchecks réels, pile e2e isoléepr/dette-l8: Dette passe 1, lot L8 : tests non destructifs, fixtures synthétiques, script de tests sur base jetableCommits du lot
88b4119feat: define boitier and server contracts in aquashared0fe084frefactor: deserialize server messages with the aquashared contractsa55c728fix: rebuild the boitier MQTT client on the shared contracts55fc805fix: give the boitier container a writable state and a single config pathAdd the shared contracts both crates exchange: canonical boitier id (digits only, no sign, no leading zero, non zero), MQTT topic names with building and parsing (data, firstco, firstco-{id}, firstco_ACK, maj_serv, maj_boitier_ACK), firstco payload and its ACK, maj_serv configuration with range and version checks, maj_boitier_ACK, MPT v2 frame building next to its parser, public epoch validation, MQTT packet size limits and a bounded backoff. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UH3JcaVWsmFLC6kyEsVL18N3: the boitier now publishes the reference firstco payload, the config_boitier.json envelope of the legacy gateway ({"boitier":{..., "noSerie"},"client":{...}}), which the server parses. The local identity file uses the same format; marking firstco done only sets boitier.first_connect and keeps the other fields (atomic write). N6: firstco_ACK-{id} is subscribed before firstco is published, then firstco is published again with a doubling delay (10 s up to 300 s) until a valid ACK arrives. N2: the firstco ACK must carry status ok and the boitier id; maj_serv commands must be 0 or 1 and its version must not be older than the applied one (persisted, survives restarts). Refusals and apply failures are answered with an error ACK; unreadable payloads get none. N5: every ConnAck resubscribes to the boitier topics; reconnection uses a bounded backoff and MQTT requests no longer block the event loop. Q5: the MQTT client id is derived from the identity (aquaboitier-{id}) unless MQTT_CLIENT_ID is set. A missing or invalid identity stops the process unless BOITIER_DEV_MODE is set, instead of running as id 0. Topics, payload types, frame building and the boitier id come from aquashared. Cross-crate contract tests produce each message with the boitier code and read it with the server code, and the reverse. The e2e fixtures follow the reference format and maj_serv versions are numeric. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UH3JcaVWsmFLC6kyEsVL18Revue statique Vigie : PR #2
Tete
55fc8050c3, basemain(674dc89dc2), 4 commit(s), 39 fichier(s) ajoutes ou modifies.Statut
aquaprocess/revue-statique: successConstats : 0 bloquant, 4 majeur, 22 mineur, 1 style. Commentaires en ligne : 4 (mode
important, plafond 10).Verifications automatiques
Analyse qualitative
Verdict : aucun bloquant ; 4 constats majeurs à traiter avant la production (anti-retour en arrière sans borne, écriture non durable, republication de firstco qui réécrit les commandes, données client sur un topic global), 9 mineurs et 1 de style. Le lot tient l'essentiel de ce qu'il annonce : contrats centralisés dans aquashared, session du boîtier pure et bien testée, ACK contrôlés.
Sécurité
configs/mosquitto/mosquitto.conf) accepte les clients anonymes sans ACL. Les contrôles N2 protègent donc contre un message mal formé, pas contre un message forgé : une version égale est réappliquée, donc unmaj_servforgé avec la version courante et d'autres commandes passe. C'est cohérent avec le renvoi de l'ACL à L6, mais il faut le dire clairement dans la suite.aquashared/src/config.rs:87). Sans borne haute ni voie de réinitialisation, c'est un déni de service durable.client(nom, adresse) surfirstcoglobal, lisible par tout abonné à#(aquaboitier-embedded/src/session.rs:178).aquaboitier-embedded/src/session.rs:223).BoitierId::parse) etTopic::parsecôté boîtier écartent les alias (maj_serv-04000), et l'ACKfirstcon'est accepté que pour le bon boîtier avecstatus = ok. Aucun secret ni donnée réelle dans les fixtures (valeurs fictives 4990).Correction
write_atomic(aquaboitier-embedded/src/persist.rs:17) n'appelle aucunsync_all: l'atomicité vaut pour un arrêt du processus, pas pour une coupure d'alimentation, alors que le commentaire l'affirme. Un fichier d'identité vide arrête le boîtier.firstco: l'upsert serveur réécritmodeBoost,generatorStateA/Betupdated_atà partir des valeurs figées du fichier d'identité. Le choix de passer opérationnel quand la persistance échoue, avec republication au démarrage suivant (aquaboitier-embedded/src/session.rs:196), peut donc annuler des commandes posées par l'administrateur, que le poller repousse ensuite au boîtier.aquaboitier-embedded/src/main.rs) : le passage àtry_publish/try_subscribesupprime bien le risque d'interblocage, mais un échec n'est que journalisé (main.rs:35), sans nouvel essai avant la reconnexion suivante.firstco(10 s doublés jusqu'à 300 s, arrêt hors connexion et à l'ACK) est correct ; il manque une gigue pour éviter la synchronisation de toute une flotte (aquashared/src/backoff.rs:26).aquaboitier-embedded/Dockerfile.prod:73) : la version appliquée et l'identité disparaissent à la recréation du conteneur.mode_boost: row.mode_boost.unwrap_or(0)(aquaserveur/src/config_poller.rs:53, ligne non modifiée) : le défaut silencieux que le contrat veut supprimer côté boîtier existe toujours, mais il est appliqué côté serveur.parse_broker_urlretombe silencieusement sur des défauts (settings.rs:86), processus PID 1 sans gestion de SIGTERM (main.rs:60), version e2e tirée de l'horloge de l'hôte et jamais nettoyée (e2e/boitier/s7-config-sync.sh:37).Cohérence avec le compte rendu
Affirmations vérifiées sur la tête
55fc805(références fichier:ligne exactes) :aquashared/src/id.rs:31et:42(BoitierId,parsecanonique) : conforme.aquashared/src/topics.rs:20-25,:63,:107-127: conforme.aquashared/src/config.rs:23,:54,:68,:84,:100;frame.rs:48,:55,:82,:170;mqtt.rs:14-22;backoff.rs:16-33: conforme.session.rs:29-31,:113,:132,:167,:185,:204et le comportement annoncé (abonnement avant publication, délais 10 à 300 s sans limite, rien hors connexion, ACK forgés refusés, payload illisible sans ACK) : conforme.main.rs:56-60,:85,:104,:116;settings.rs:18,:57,:70;entrypoint.sh:11;Dockerfile:86,:98-99;Dockerfile.prod:73,:85-86;docker-compose.dev.yml:103;common-boitier.sh:40: conforme.provisioning/types.rs:7etconfig_sync.rs:16réexportent les types partagés,firstco_ack_payloadenconfig_sync.rs:94, appel enmain.rs:218-219, format{"status":"ok","id_boitier":N}testé à l'octet : conforme.mark_firstco_doneconserve les autres champs : conforme (testé), mais l'ordre des clés et les permissions changent (firstco/mod.rs:89).Écarts :
persist.rs:17).firstco/mod.rs:7) : le payload est resérialisé, les champs non modélisés sont perdus.docker restart, faux pour une recréation du conteneur.sessioncompte 16 tests (17 annoncés) etconfig_sync10 (9 annoncés) ; le total de 63 tests unitaires du boîtier horsalert_detectorest cohérent.cargo test(233 passés, 39 ignorés) et la vérification de bout en bout n'ont pas pu être rejoués ici (revue en lecture seule, sans cargo ni docker).Tests
status error,"1", vide, non UTF-8), bornes, version plus ancienne, redémarrage, échec d'écriture. Pas desleep, répertoires temporaires suffixés par le pid.main.rs(sélection entrepollet minuterie,retry_at, échecstry_*) n'a aucun test automatisé ; elle ne repose que sur la vérification manuelle décrite.aquaserveur/tests/contract_boitier_serveur.rs:113) : mêmes types des deux côtés, fixtures réécrites dans ce lot.aquashared/tests/integration_test.rs(test_every_message_has_its_topic_and_type) a des assertions faibles (accessibilité de l'API plus qu'un comportement).Questions ouvertes
EventLoop::pollde rumqttc 0.25 est-il sûr à l'annulation dans letokio::select!demain.rs:102-125? Une annulation pendant la connexion ou une lecture partielle mérite une vérification dans la documentation de la version utilisée.boitiers.updated_at: si c'est un DATETIME local,UNIX_TIMESTAMPn'est pas monotone au changement d'heure (25 octobre 2026), et une configuration légitime pourrait être refusée comme plus ancienne.first_connectne serait jamais persisté.max_packet_size: simple erreur ou déconnexion répétée ?Autres constats (non publies en ligne)
aquaboitier-embedded/Dockerfile.prod:73aquaboitier-embedded/src/config_ack/mod.rs:35no-unwrap-in-production(WARNING) :.unwrap()ou.expect()peut paniquer en production. 4 occurrence(s) sur lignes ajoutees (35, 49, 60, 60), y compris d'eventuels modules de test internes.aquaboitier-embedded/src/config_sync/mod.rs:297no-unwrap-in-production(WARNING) :.unwrap()ou.expect()peut paniquer en production. 3 occurrence(s) sur lignes ajoutees (297, 298, 315), y compris d'eventuels modules de test internes.aquaboitier-embedded/src/firstco/mod.rs:7aquaboitier-embedded/src/firstco/mod.rs:162no-unwrap-in-production(WARNING) :.unwrap()ou.expect()peut paniquer en production. 18 occurrence(s) sur lignes ajoutees (162, 169, 173, 183, 198, 199, 211, 221, 231, 238, 240, 242 ...), y compris d'eventuels modules de test internes.aquaboitier-embedded/src/main.rs:18tokio-spawn-fire-and-forget(WARNING) :tokio::spawn()sans stocker le JoinHandle (fire-and-forget). 2 occurrence(s) sur lignes ajoutees (18, 18), y compris d'eventuels modules de test internes.aquaboitier-embedded/src/main.rs:35aquaboitier-embedded/src/main.rs:60aquaboitier-embedded/src/persist.rs:29no-unwrap-in-production(WARNING) :.unwrap()ou.expect()peut paniquer en production. 4 occurrence(s) sur lignes ajoutees (29, 31, 33, 37), y compris d'eventuels modules de test internes.aquaboitier-embedded/src/session.rs:223aquaboitier-embedded/src/session.rs:268no-unwrap-in-production(WARNING) :.unwrap()ou.expect()peut paniquer en production. 10 occurrence(s) sur lignes ajoutees (268, 273, 292, 293, 340, 357, 361, 525, 586, 591), y compris d'eventuels modules de test internes.aquaboitier-embedded/src/settings.rs:86aquaboitier-embedded/src/settings.rs:119no-unwrap-in-production(WARNING) :.unwrap()ou.expect()peut paniquer en production. 6 occurrence(s) sur lignes ajoutees (119, 210, 215, 218, 218, 221), y compris d'eventuels modules de test internes.aquaserveur/src/config_sync.rs:99no-unwrap-in-production(WARNING) :.unwrap()ou.expect()peut paniquer en production. 1 occurrence(s) sur lignes ajoutees (99), y compris d'eventuels modules de test internes.aquaserveur/tests/contract_boitier_serveur.rs:113aquashared/src/backoff.rs:26aquashared/src/config.rs:152no-unwrap-in-production(WARNING) :.unwrap()ou.expect()peut paniquer en production. 5 occurrence(s) sur lignes ajoutees (152, 162, 248, 258, 262), y compris d'eventuels modules de test internes.aquashared/src/frame.rs:214no-unwrap-in-production(WARNING) :.unwrap()ou.expect()peut paniquer en production. 10 occurrence(s) sur lignes ajoutees (214, 223, 230, 237, 243, 250, 256, 349, 356, 363), y compris d'eventuels modules de test internes.aquashared/src/id.rs:164no-unwrap-in-production(WARNING) :.unwrap()ou.expect()peut paniquer en production. 1 occurrence(s) sur lignes ajoutees (164), y compris d'eventuels modules de test internes.aquashared/src/provisioning.rs:209no-unwrap-in-production(WARNING) :.unwrap()ou.expect()peut paniquer en production. 12 occurrence(s) sur lignes ajoutees (209, 215, 225, 232, 253, 254, 258, 265, 271, 277, 283, 284), y compris d'eventuels modules de test internes.aquashared/src/topics.rs:136no-unwrap-in-production(WARNING) :.unwrap()ou.expect()peut paniquer en production. 1 occurrence(s) sur lignes ajoutees (136), y compris d'eventuels modules de test internes.e2e/boitier/s7-config-sync.sh:37aquaboitier-embedded/src/firstco/mod.rs:89Limites
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 commitaquaprocess/revue-statique. Analyse statique et tests automatises seulement, sans fusion ni deploiement.@ -0,0 +14,4 @@let mut tmp_name = file_name.to_os_string();tmp_name.push(".tmp");let tmp = path.with_file_name(tmp_name);std::fs::write(&tmp, contents).with_context(|| format!("write {}", tmp.display()))?;[majeur] write_atomic écrit le fichier temporaire puis le renomme sans sync_all() du fichier ni fsync du répertoire. Le renommage est atomique vis-à-vis d'un arrêt du processus, mais pas d'une coupure d'alimentation : selon le système de fichiers de la carte, le fichier renommé peut se retrouver vide après redémarrage, ce que le commentaire des lignes 7 à 9 affirme pourtant éviter. Pour le fichier d'identité, un contenu vide arrête le boîtier (code 1) au démarrage suivant. Piste : File::create, write_all, sync_all sur le temporaire, rename, puis ouverture et sync_all du répertoire parent.
Source : revue.
@ -0,0 +175,4 @@self.firstco.on_publish();vec![Outgoing::Publish {topic: topics::FIRSTCO.to_string(),[majeur] Le boîtier publie désormais l'enveloppe complète, objet client compris (customerName, address, postcode, city), sur le topic global firstco. Avec la configuration actuelle du broker (anonyme, sans ACL), tout client abonné à firstco ou à # reçoit ces données personnelles pour chaque boîtier ; l'ancien boîtier Rust ne publiait que trois champs. Piste : conditionner la mise en production à l'ACL (L6) et au topic firstco-{id}, ou ne publier que les champs dont le provisioning a besoin.
Source : revue.
@ -0,0 +193,4 @@}if let Err(e) = mark_firstco_done(&self.paths.config) {// Le serveur a provisionné le boîtier : rester en attente ferait// republier firstco indéfiniment. Le prochain démarrage republiera.[majeur] Quand l'ACK n'est pas persisté, le boîtier passe opérationnel et compte sur le prochain démarrage pour republier firstco ; de même un ACK perdu provoque une republication. Or le payload firstco porte modeBoost et config.generatorStateA/B figés dans le fichier d'identité, et l'upsert serveur (provisioning/handler.rs, ON DUPLICATE KEY UPDATE modeBoost, generatorStateA/B, updated_at) les réécrit en base : toute republication tardive annule les commandes posées par l'administrateur, et le poller repousse ces valeurs anciennes au boîtier. Piste : retenter la persistance au lieu de passer opérationnel, ou ne pas réécrire les commandes lors d'un upsert sur un boîtier déjà provisionné (côté serveur, L3).
Source : revue.
@ -0,0 +84,4 @@pub fn validate_against(&self, applied: Option<u64>) -> Result<u64, ConfigRejection> {let received = self.validate()?;match applied {Some(applied) if received < applied => {[majeur] L'anti-retour en arrière n'a ni borne haute ni voie de réinitialisation : une configuration acceptée avec une version très grande (par exemple "18446744073709551615", ou un epoch futur venant d'une horloge décalée) est persistée dans /data/config_state.json, puis toute configuration légitime ultérieure est refusée comme Stale, redémarrage compris, avec une alerte 15 côté serveur à chaque refus. Le broker actuel (allow_anonymous true, sans ACL) permet de le provoquer depuis le réseau, et une restauration de base (updated_at plus anciens) produit le même blocage. Piste : borner la version (longueur et plausibilité, par exemple au plus horloge du boîtier plus une marge), et prévoir un moyen explicite de réinitialiser la version appliquée.
Source : revue.
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.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.