Confrontation Gate 5 — PD-30¶
Analyse des écarts¶
[ECT-01] Traçabilité tests incomplète¶
Verdict : PARTIELLEMENT CONFIRMÉ
Justification :
Le reviewer a raison sur le fond : le plan mentionne "19 TC couverts" dans TASK-8 et dans les critères de succès sans fournir de matrice explicite INV/CA → cas de test → fichier/test id. C'est un manque de traçabilité formelle.
Cependant, l'impact est à nuancer : - La section 3 du plan fournit déjà un mapping INV/CA → Tâche → Fichier principal qui constitue une traçabilité de premier niveau. - Les Code Contracts (CC-30-01 à CC-30-08) établissent le lien Contract → INV couverts. - Ce qui manque réellement est le dernier maillon : CC/INV → test ID spécifique dans le fichier de test.
Correction nécessaire : Ajouter dans TASK-8 une matrice de traçabilité INV-30-XX → TC-XX → describe/it block qui sera vérifiable mécaniquement. Ce n'est pas un risque de faux positif de couverture contractuelle à ce stade (le plan est un plan, pas le code), mais cette matrice doit être exigée comme livrable de TASK-8.
[ECT-02] Mode dégradé du rate limiting non défini¶
Verdict : CONFIRMÉ
Justification :
C'est un écart valide et significatif. TASK-6 spécifie "Sliding window avec Redis sorted sets" mais ne traite pas le cas où Redis est indisponible, alors même que TASK-3 définit explicitement un mode dégradé Redis → Memory pour les sessions.
Les trois options ont des implications sécuritaires radicalement différentes : - Fail-open : bypass du rate limiting → exposition à des attaques par force brute sur les opérations sensibles (REVOKE_ALL, etc.) - Fail-closed : rejet de toutes les opérations sensibles → déni de service fonctionnel - Quota local in-memory : approximation acceptable mais avec risque de contournement multi-instance
Le plan devrait explicitement spécifier le comportement. Ma recommandation technique : fail-closed avec quota local conservateur — en mode dégradé, appliquer un rate limit in-memory avec des seuils réduits (e.g., 1 req/user/15min au lieu de 3), car les opérations protégées sont sensibles et un faux négatif de rate limiting est plus dangereux qu'un faux positif temporaire.
Correction nécessaire : Ajouter dans TASK-6 une section "Comportement en mode dégradé" avec la stratégie choisie et la justification sécuritaire.
[ECT-03] Mitigation "fallback → Redis sync" trop vague¶
Verdict : CONFIRMÉ
Justification :
La mitigation du risque "Perte données fallback → Redis" est formulée comme "Sync explicite au retour mode nominal", ce qui est effectivement insuffisant pour un plan d'implémentation. Les questions suivantes restent ouvertes :
- Ordre des événements : Que se passe-t-il si une session est créée en memory pendant le fallback, puis une session avec le même userId est créée sur une autre instance via Redis (qui n'a jamais basculé) ? Quelle version prévaut ?
- Idempotence : Les opérations de sync doivent être idempotentes (SET avec NX vs SET simple, gestion des TTL résiduels).
- Déduplication : Si le circuit breaker oscille (flapping), comment éviter des syncs multiples ?
- Conflit temporel : Une session expirée en memory mais dont le TTL Redis n'a pas encore expiré crée une incohérence.
Le fait que INV-30-19 impose le rejet des opérations globales en mode dégradé atténue partiellement le risque (pas de REVOKE_ALL incohérent), mais les sessions créées/rafraîchies localement pendant le fallback nécessitent une stratégie de réconciliation définie.
Correction nécessaire : Ajouter dans TASK-3 un protocole de sync explicite couvrant : stratégie de réconciliation (last-write-wins avec timestamp vs merge), idempotence des opérations, protection anti-flapping du circuit breaker, et traitement des sessions expirées pendant le fallback.
[ECT-04] Dépendances audit sous-estimées¶
Verdict : PARTIELLEMENT CONFIRMÉ
Justification :
Le reviewer identifie correctement que TASK-7 (Audit Interceptor) journalise des événements produits par TASK-3 (FALLBACK_ON, FALLBACK_OFF) et TASK-5 (REVOKE_SESSION, REVOKE_DEVICE, REVOKE_ALL), alors que sa seule dépendance déclarée est TASK-1.
Cependant, l'impact est à nuancer : - L'audit interceptor est un intercepteur NestJS qui se positionne au niveau HTTP/controller et capture les événements de manière transversale. Il n'a pas besoin de dépendance de compilation sur TASK-3 ou TASK-5 — il réagit aux événements émis. - Le séquencement (section 6) place TASK-7 en parallèle de TASK-3/TASK-5 avec exécution après TASK-3 dans le graphe : T1, T2 → T3 → T4, T5, T6, T7. TASK-7 est donc bien séquencé après TASK-3. - Le vrai risque n'est pas une dépendance de build mais un risque d'écart d'interface : les types d'événements émis par TASK-3 et TASK-5 doivent correspondre exactement aux types attendus par TASK-7.
Correction nécessaire : Ajouter une dépendance explicite de TASK-7 vers TASK-3 et TASK-5 dans le tableau des dépendances (même si le séquencement le couvre implicitement). Plus important : définir un enum SessionAuditEvent dans TASK-1 qui sera le contrat partagé, garantissant la cohérence entre émetteurs et consommateur.
[ECT-05] SLA invalidation non opérationnalisé¶
Verdict : CONFIRMÉ
Justification :
Le reviewer a raison. Le plan mentionne "p95 < 100ms, p99 < 250ms" comme critère de succès et "Pub/sub Redis + tests de charge" comme mitigation, mais ne définit aucun protocole de mesure reproductible :
- Fenêtre de mesure : Sur combien d'opérations ? 1000 ? 10000 ? La stabilité statistique des percentiles exige un volume minimum.
- Volumétrie : Combien d'instances simultanées ? Le pub/sub Redis a un coût proportionnel au nombre de subscribers.
- Environnement : Redis local (latence ~0.1ms) vs Redis distant (latence ~1-5ms) donne des résultats radicalement différents. Un test en CI avec testcontainers ne reproduit pas les conditions de production.
- Seuil d'acceptation CI : Un test de latence qui passe à 95% du temps est-il un test fiable ou un test flaky ?
TASK-8 mentionne "Tests de latence p95/p99 pour invalidation" sans ces précisions, ce qui rend le critère de succès non vérifiable de manière déterministe.
Correction nécessaire : Définir dans TASK-8 le protocole de mesure : volume minimum (e.g., 1000 invalidations), environnement (testcontainers Redis, single node), tolérance CI (marge de 2x sur les seuils pour absorber la variabilité CI), et distinguer les tests de conformité (CI, seuils relâchés) des tests de performance (dédiés, seuils stricts).
[ECT-06] Staleness <= 2s peu détaillé¶
Verdict : PARTIELLEMENT CONFIRMÉ
Justification :
CA-30-07 impose une staleness <= 2s pour le listing de sessions. Le plan mentionne cette contrainte dans TASK-4 mais ne détaille pas le mécanisme technique.
Cependant, dans le contexte de l'architecture : - Les sessions sont stockées dans Redis (TASK-3), qui est la source de vérité. - TASK-4 (controller) lit directement depuis le store Redis via le service. - Il n'y a pas de cache applicatif intermédiaire ni de read model dédié dans le plan.
Donc la staleness est implicitement 0 en mode nominal (lecture directe Redis) et potentiellement infinie en mode dégradé (lecture du memory store local qui ne reçoit pas les mises à jour des autres instances).
Le vrai problème n'est pas le manque de détail du mécanisme (qui est trivial en mode nominal) mais l'absence de traitement du cas dégradé vis-à-vis de CA-30-07. En mode dégradé, INV-30-19 interdit les opérations globales, ce qui limite l'exposition.
Correction nécessaire : Préciser dans TASK-4 que la staleness <= 2s est garantie par lecture directe Redis (pas de cache), et documenter que cette garantie est suspendue en mode dégradé (couvert par les restrictions INV-30-19).
[ECT-07] Memory store TTL via setTimeout¶
Verdict : PARTIELLEMENT CONFIRMÉ
Justification :
Le reviewer note le risque de scalabilité/memory pressure avec setTimeout pour chaque entrée du memory store. C'est un point valide en théorie mais à contextualiser :
- Le memory store est un fallback temporaire, activé uniquement quand Redis est indisponible. Sa durée d'utilisation est limitée par le circuit breaker (recovery automatique).
- INV-30-19 interdit les opérations globales en mode dégradé, ce qui limite la cardinalité aux sessions créées/rafraîchies pendant le fallback sur une seule instance.
- En pratique, le nombre de sessions concurrentes sur une instance pendant un fallback de quelques minutes/heures est borné (quelques centaines à quelques milliers).
- Node.js gère efficacement des milliers de timers — la pression mémoire vient des données, pas des timers.
Cependant, le plan mentionne "TTL strict + cleanup timer" comme mitigation du risque "Memory leak fallback" sans préciser l'implémentation. Un setInterval de purge périodique (toutes les 30s) serait plus robuste qu'un setTimeout par entrée et éviterait l'accumulation de timers.
Correction nécessaire : Préciser dans TASK-3 la stratégie : Map avec timestamp d'expiration + setInterval de purge batch (e.g., toutes les 30s), plutôt que setTimeout par entrée. Impact faible mais bonne pratique.
[ECT-08] Validation startup config¶
Verdict : PARTIELLEMENT CONFIRMÉ
Justification :
TASK-2 mentionne "Validation des valeurs au démarrage" sans préciser les bornes ni le comportement en cas de configuration invalide. Le reviewer a raison de noter ce manque.
Cependant : - Les valeurs TTL sont contractuelles (INV-30-04, INV-30-07) avec des valeurs exactes spécifiées. La validation consiste principalement à vérifier que les valeurs fournies correspondent aux valeurs contractuelles ou sont dans des bornes raisonnables. - Le pattern NestJS standard (class-validator + class-transformer) gère naturellement la validation avec des décorateurs @IsInt(), @Min(), @Max(). - Le comportement en cas d'échec de validation au démarrage est le comportement NestJS par défaut : le module ne se charge pas, l'application ne démarre pas. C'est le comportement fail-fast attendu.
Correction nécessaire : Ajouter dans TASK-2 les bornes de validation (min: 60s, max: 2592000s/30j) et confirmer le comportement fail-fast (exception au démarrage si config invalide). Impact mineur — c'est du détail d'implémentation plus que du design.
Synthèse¶
| Écart | Sévérité | Verdict | Impact sur le plan |
|---|---|---|---|
| ECT-01 | MAJEUR | PARTIELLEMENT CONFIRMÉ | Ajouter matrice traçabilité comme livrable TASK-8 |
| ECT-02 | MAJEUR | CONFIRMÉ | Ajouter stratégie rate limiting dégradé dans TASK-6 |
| ECT-03 | MAJEUR | CONFIRMÉ | Ajouter protocole de réconciliation dans TASK-3 |
| ECT-04 | MAJEUR | PARTIELLEMENT CONFIRMÉ | Expliciter dépendances TASK-7 + enum partagé |
| ECT-05 | MAJEUR | CONFIRMÉ | Définir protocole de mesure SLA dans TASK-8 |
| ECT-06 | MINEUR | PARTIELLEMENT CONFIRMÉ | Préciser mécanisme staleness + cas dégradé |
| ECT-07 | MINEUR | PARTIELLEMENT CONFIRMÉ | Préciser stratégie cleanup batch |
| ECT-08 | MINEUR | PARTIELLEMENT CONFIRMÉ | Ajouter bornes et comportement fail-fast |
Recommandation¶
Le plan PD-30 nécessite une révision ciblée avant implémentation. Trois écarts majeurs sont confirmés (ECT-02, ECT-03, ECT-05) et deux sont partiellement confirmés (ECT-01, ECT-04). Aucun écart n'est contesté — le reviewer a produit une analyse pertinente.
Corrections obligatoires (bloquantes)¶
- TASK-6 : Ajouter une section "Mode dégradé" spécifiant fail-closed avec quota local conservateur (seuils réduits en in-memory)
- TASK-3 : Définir le protocole de réconciliation fallback → Redis (last-write-wins avec timestamp, idempotence via SET NX + TTL, protection anti-flapping)
- TASK-8 : Ajouter une matrice de traçabilité INV/CA → TC-ID et un protocole de mesure SLA (volume, environnement, tolérance CI)
Corrections recommandées (non bloquantes)¶
- TASK-7 : Déclarer les dépendances TASK-3 et TASK-5 ; définir un enum
SessionAuditEventdans TASK-1 - TASK-4 : Documenter que staleness <= 2s = lecture directe Redis, suspendue en mode dégradé
- TASK-3 : Préciser
Map+setIntervalbatch plutôt quesetTimeoutpar entrée - TASK-2 : Ajouter bornes de validation et confirmer fail-fast
Score de qualité¶
- Avant corrections : 6/10 — le plan est structurellement solide mais insuffisamment défensif sur les cas limites
- Après corrections : 9/10 — les fondamentaux architecturaux sont bons, le mapping INV/CA est complet, le séquencement est cohérent