Skip to content

fix(db): KF-020 SELECT FOR UPDATE in invoices::update (closes #49) - #64

Merged
guycorbaz merged 1 commit into
mainfrom
fix/kf-020-invoices-select-for-update
May 2, 2026
Merged

guycorbaz merged 1 commit into
mainfrom
fix/kf-020-invoices-select-for-update

Conversation

@guycorbaz

Copy link
Copy Markdown
Owner

Summary

Item #2 du prep sprint Epic 8 (validé Guy 2026-05-01). Ferme la race no-op résiduelle KF-004 documentée Story 7-3 §race-condition.

Scope chirurgical : 1 ligne SQL dans invoices::update (SELECT → SELECT ... FOR UPDATE), même pattern que delete(), validate_invoice(), mark_as_paid() déjà en place dans le même fichier. Hors scope : les autres entités variant A (contacts, products, companies, accounts, users, company_invoice_settings, bank_accounts) — cf. issue #49 « Hors scope ».

Race fermée

Sous REPEATABLE READ + plain SELECT, une tx parallèle qui commit entre BEGIN et le no-op check (is_no_op_change) pouvait laisser fuiter un snapshot stale en 200 OK. Avec FOR UPDATE :

  • Le X-lock attend la fin de la tx concurrente.
  • La lecture post-lock retourne v=N+1.
  • La version-check applicative déclenche le 409 attendu.

Tests

  • Nouveau test_update_concurrent_no_op_vs_mutation_no_stale_snapshot_kf020 dans crates/kesh-db/src/repositories/invoices.rs : 30 itérations tokio::join (mutation A vs no-op B même expected_version=N) avec pool dédié 6 connexions. Asserts les invariants : A=Ok(v=N+1), B=Ok(v=N) ou OptimisticLockConflict, DB final cohérent. Tolère Sqlx(deadlock) rare avec trace eprintln.
  • Docstring AC Story 6-2: Multi-tenant scoping refactor — Pass 3 remediation complete #29 mise à jour dans crates/kesh-api/tests/kf004_no_op_e2e.rs.

Tests verts (release) : 39/39 kesh-db invoices, 6/6 kf004 e2e.

Code review (1 passe Sonnet adversariale)

Approche option B (validée Guy) : 1 passe code review LLM différent de l'implémenteur Opus, contexte frais, sans story BMAD complète (l'issue #49 = la spec).

# Severity Finding Action
H-1 HIGH Test unwrap_or_else(panic!) masque deadlock MariaDB rare ✅ Patché (assertion relax + trace)
M-1 MEDIUM Lock-ordering doc manquant sur update() ✅ Patché (docblock canonique ajouté)
M-2 MEDIUM DRY violation delete() SQL inline ⏸️ Defer (pré-existant, hors scope)
M-3 MEDIUM max_connections=4 serré ✅ Patché (4 → 6)
L-1, L-2 LOW Cosmétique (… vs ..., phrasing) ✅ Patché
L-3 LOW Date 2026-05-02 ⏸️ Defer (today = 05-02 confirmé)
L-4 LOW validate_invoice lock comment ⏸️ Defer (autre fonction)

Verdict reviewer : GO.

Test plan

Suite — prep sprint Epic 8

Items restants :

  1. ✅ Créer epic-8.md (PR docs(epic-8): create planning artifact + open CR-009/CR-010 #63 mergée)
  2. ⏳ KF-020 [KF-020] invoices::update : passer en SELECT FOR UPDATE pour fermer la race no-op KF-004 résiduelle #49 SELECT FOR UPDATE → ce PR
  3. ⏳ KF-002-H-002 [KF-002-H-002] Deadlock-retry middleware for finalize multi-locks #43 deadlock-retry middleware
  4. ⏳ Spike kesh-import crate design

🤖 Generated with Claude Code

Ferme la race no-op résiduelle KF-004 documentée Story 7-3
§race-condition. Sous REPEATABLE READ + plain SELECT, une tx parallèle
qui commit entre BEGIN et le no-op check pouvait laisser fuiter un
snapshot stale en `200 OK`. Avec FOR UPDATE le X-lock attend la fin
de la tx concurrente et la lecture post-lock retourne v=N+1 → la
version-check applicative déclenche le 409 attendu.

Scope chirurgical : 1 ligne SQL (`invoices::update` SELECT initial),
mêmes patterns que `delete()`, `validate_invoice()`, `mark_as_paid()`.
Hors scope : autres entités variant A (cf. issue #49 « Hors scope »).

Tests :

- Nouveau `test_update_concurrent_no_op_vs_mutation_no_stale_snapshot_kf020`
  dans `crates/kesh-db/src/repositories/invoices.rs` : 30 itérations
  tokio::join (mutation A vs no-op B même expected_version=N) avec
  pool dédié 6 connexions. Asserts les invariants : A=Ok(v=N+1),
  B=Ok(v=N) ou OptimisticLockConflict, DB final cohérent. Tolère
  Sqlx(deadlock) rare avec trace eprintln (H-1 review remediation).
- Docstring AC #29 mise à jour dans `crates/kesh-api/tests/kf004_no_op_e2e.rs`
  pour refléter la fermeture v0.1 (KF-021 #50 reste ouvert pour les
  autres entités variant A).

Code review (1 passe Sonnet adversariale, contexte frais) :

- 0 CRITICAL, 1 HIGH (H-1 panic test brittle), 3 MEDIUM (M-1 lock-order
  doc manquant, M-2 DRY pré-existant `delete`, M-3 max_connections=4
  serré), 4 LOW.
- Patches appliqués : H-1 (assertion relax avec trace), M-1 (docblock
  lock-ordering ajouté à `update`), M-3 (4 → 6 connexions), L-1/L-2
  (cosmétique).
- Defers : M-2 (pré-existant, hors scope), L-3 (date OK), L-4
  (autre fonction).
- Verdict reviewer : GO.

Tests verts (release) : 39/39 kesh-db invoices, 6/6 kf004 e2e.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@guycorbaz
guycorbaz merged commit ebdea4b into main May 2, 2026
3 checks passed
@guycorbaz
guycorbaz deleted the fix/kf-020-invoices-select-for-update branch May 2, 2026 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant