[KF-020] invoices::update : passer en SELECT FOR UPDATE pour fermer la race no-op KF-004 résiduelle #49

KF ID

KF-020

Symptom

Sous REPEATABLE READ + plain SELECT (pattern actuel de invoices::update après le fix KF-004), si une transaction parallèle commit entre le BEGIN et le no-op check (is_no_op_change), la fonction retourne le snapshot stale v=N à l'utilisateur au lieu d'un 409 OPTIMISTIC_LOCK_CONFLICT. L'utilisateur reçoit 200 OK avec un état pre-commit qu'il croit à jour, jusqu'à un GET ultérieur.

Reproduction (extraite de la spec Story 7-3 §race-condition) :

1. T0 : tx2 BEGIN.
2. T1 : tx2 SELECT before_invoice → snapshot v=N (état committed à T0).
3. T2 (parallèle) : tx1 modifie l'invoice, COMMIT v=N+1 avec un nouveau total_amount.
4. T3 : tx2 vérifie before_invoice.version (N) == version (N) ✅, pas de 409.
5. T4 : tx2 calcule is_no_op_change(&before_invoice, &before_lines, &changes) — before est le snapshot v=N, changes est le payload utilisateur (probablement aussi v=N car chargé avant tx1).
6. T5 : is_no_op_change == true → tx2 ROLLBACK + retourne Ok((before_invoice v=N, before_lines)).
7. T6 : utilisateur tx2 reçoit 200 OK + état v=N stale alors que la DB a v=N+1.

Root cause hypothesis

Le pattern invoices::update utilise un plain SELECT (sans FOR UPDATE) à l'étape before_invoice_opt. Sous l'isolation REPEATABLE READ d'InnoDB, un plain SELECT capture un snapshot consistent read au premier read de la tx — il ne se synchronise pas avec les commits parallèles. Le no-op check court-circuite donc avec un état périmé sans déclencher la version-check ré-évaluée par la DB (qui aurait détecté le bump v=N+1 lors d'un UPDATE conditionnel).

Mitigation : passer le SELECT initial en SELECT ... FOR UPDATE. Le X-lock attendrait la fin de tx1 et la lecture post-lock retournerait v=N+1 → la version-check applicative déclencherait alors OptimisticLockConflict 409 attendu.

bank_accounts::upsert_primary et journal_entries::update sont déjà protégés par SELECT ... FOR UPDATE étape 1 et n'ont donc PAS cette race. C'est invoices::update qui est le cas le plus exposé en pratique : sessions de saisie facture qui durent plusieurs minutes.

Scope of origin

Story 7-3 (KF-004 closure, 2026-04-29) — la race est un résidu acceptable v0.1 documenté dans la spec §race-condition (point 4 des justifications) et tracé via cette issue follow-up + le test E2E no_op_with_parallel_mutation_returns_409_when_sequential (qui couvre uniquement le mode séquentiel — la race exige tokio::join pour se reproduire).

Pattern : crates/kesh-db/src/repositories/invoices.rs:598 (le SELECT ... FROM invoices WHERE id = ? AND company_id = ?).

Blocking impact

• Fenêtre étroite : exige (a) deux clients en concurrence parfaite (~ms), (b) tx1 commit pendant tx2 entre BEGIN et le no-op check, (c) payload utilisateur de tx2 strictement identique au snapshot pré-tx1.
• Symptôme bénin : aucune perte de données (tx2 n'a rien modifié), juste un état affiché stale jusqu'au prochain refresh.
• Pas critique v0.1 : workflow comptable PME = 1-3 utilisateurs simultanés au plus, probabilité combinée négligeable hors stress-test.
• Critique Epic 8 : Import bancaire introduira des flux de réconciliation où plusieurs utilisateurs interagissent avec les mêmes factures (statut payé, ré-affectation). La probabilité de la race augmentera mécaniquement avec la concurrence accrue. Cette issue est un prerequisite Epic 8.

Reproduction

# Test E2E qui démontre le mode séquentiel (NE reproduit PAS la race) :
cargo test -p kesh-api --test kf004_no_op_e2e no_op_with_parallel_mutation_returns_409_when_sequential

# Pour reproduire la race effective, il faut un stress test concurrent
# avec tokio::join et deux pools distincts (exercise pour la story de remédiation).

Remediation story (planned)

Story dédiée à créer dans Epic 8 prerequisite (ou Epic 7 résiduel) :

• Scope : crates/kesh-db/src/repositories/invoices.rs:598 uniquement — passer SELECT ... FROM invoices WHERE id = ? AND company_id = ? en SELECT ... FOR UPDATE.
• Tests à ajouter : stress test concurrent reproduisant le scénario T0-T6 ci-dessus avec tokio::join! + deux pools.
• Test régression à mettre à jour : no_op_with_parallel_mutation_returns_* deviendra _returns_409_under_concurrency (le 200 stale → 409 attendu).
• Hors scope : les 5 autres entités variant A (contacts, products, companies, bank_accounts — déjà FOR UPDATE, company_invoice_settings, accounts, users). Si une issue prod émerge sur l'une d'elles, ouvrir une issue dédiée par entité pour évaluation au cas par cas (la fenêtre de session est typiquement plus courte et le risque comptable plus faible).

Référence : Story 7-3 §race-condition + AC #28-bis + AC #29 + T13.4 + commentaire inline « NOTE concurrence (KF-004) » dans chaque patch variant A & C.