diff --git a/CHANGELOG.md b/CHANGELOG.md index 588bdb7e..997a3817 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,8 @@ Le contenu est rédigé en français à destination des **fiduciaires, PME, ind - **Le rapprochement bancaire ne reconnaissait pas le virement qui soldait une facture déjà réglée en partie ([#420](https://github.com/guycorbaz/kesh/issues/420)).** Une facture de 1 000.— TTC réglée de 400.— n'était même pas proposée pour un virement de 600.— — exactement ce que le client devait encore —, parce que la recherche comparait le montant d'origine : il fallait rapprocher à la main un paiement parfaitement identifiable. Le rapprochement cherche et note désormais les factures sur leur **reste dû** ; la proposition affiche ce reste, suivi du total d'origine (« 600, reste dû sur 1000 ») quand la facture est déjà réglée en partie. *(API : le champ `invoiceAmount` des propositions porte désormais le reste dû — identique au TTC tant que rien n'est réglé — et le nouveau champ `invoiceTotalTtc` le total d'origine, seulement quand les deux diffèrent.)* Le manuel décrit enfin le score tel qu'il est calculé — il annonçait un score gradué, un critère de date et des seuils de confiance qui n'existent pas. +- **Un rapprochement et un règlement manuel enregistrés au même moment pouvaient régler deux fois la même facture ([#480](https://github.com/guycorbaz/kesh/issues/480)).** Un règlement manuel **partiel** ne marquait pas la facture comme modifiée : une acceptation de rapprochement en cours, qui avait lu le reste dû juste avant, l'encaissait une seconde fois — une facture de 1 000.— finissait réglée 1 400.—, et le compte clients devenait créditeur sans que rien ne le signale. Chaque règlement marque désormais la facture, et l'acceptation qui arrive trop tard est refusée sans rien écrire (« la facture a changé ») : il suffit de relire les propositions. Un interblocage entre deux opérations simultanées, qui rendait une erreur interne, est maintenant rejoué par le serveur. + ### Changed - **Le bouton *Créer un avoir* s'affiche aussi sur une facture payée**, et c'est voulu. Il était masqué sur une facture payée mais pas sur une facture réglée en partie : l'écran ne couvrait que la moitié de la règle. Il reste désormais visible, et Kesh explique le refus dans le dialogue — comme le bouton *Dévalider*. diff --git a/_bmad-output/implementation-artifacts/25-4-c2-review-prompt-p1.md b/_bmad-output/implementation-artifacts/25-4-c2-review-prompt-p1.md new file mode 100644 index 00000000..0e9db6af --- /dev/null +++ b/_bmad-output/implementation-artifacts/25-4-c2-review-prompt-p1.md @@ -0,0 +1,52 @@ +# Prompts — revue de code P1, Story 25-4-c2 (le verrou à l'acceptation) + +*Versionné le 2026-09-30. Trois lentilles en parallèle, **Sonnet**, contexte frais. Diff : le commit +d'implémentation, `git diff 6810cd97 177135f2 -- . ':(exclude)_bmad-output'`, écrit dans +`/tmp/claude-1000/-home-gcorbaz-devel-kesh/379e6f94-8029-42cb-9720-fa27c2fb204c/scratchpad/25-4-c2-p1.diff`.* + +## Commun aux trois lentilles + +**Rendu** : findings avec sévérité (CRITICAL/HIGH/MEDIUM/LOW), endroit exact (`fichier:ligne`), +**preuve** (code lu cité, commande et sortie), correction proposée. ⛔ **La liste des axes réellement +exercés ET de ceux qui ne l'ont pas été** — un rapport sans elle ne compte pas. + +**Interdits** : n'écrire aucun fichier du dépôt ; aucune commande qui écrit dans le dépôt ou dans une +base — `scripts/prepare-release.sh`, `scripts/regen-test-schema.sh`, `scripts/install-hooks.sh`, +`scripts/test-fast.sh`, `scripts/mem-guard.sh`, `make`, `latexmk`, tout `git commit`/`push`/`add`/ +`stash`/`reset`/`rebase`/`checkout`/`switch`/`worktree`, `sqlx migrate`, `cargo test`/`nextest`, +`npm run`, `npx playwright`, `gh issue create`/`comment`/`edit`. Autorisés : lecture, `grep`, +`git log`/`show`/`diff`, `gh issue view`, `pdftotext` vers le scratchpad, `cargo check`. + +## Lentille 1 — Blind Hunter (`bmad-review-adversarial-general`) + +Reçoit **le diff seul**, aucun contexte projet. Revue adversariale générale. + +## Lentille 2 — Edge Case Hunter (`bmad-review-edge-case-hunter`) + +Diff **et** lecture du dépôt (MariaDB 10.11, REPEATABLE READ, `innodb_snapshot_isolation` OFF). +- `accept_batch` : chaque chemin où la transaction peut être annulée sous le lot — l'erreur remonte-t-elle + toujours en `TransactionAborted` ou en 1213 direct ? Et `RELEASE SAVEPOINT` après une proposition + réussie, `SAVEPOINT` suivant, `commit` ? Un 1205 (attente dépassée) est-il pris à tort pour un + interblocage ? +- `post_accept` / `accept_once` : le rejeu est-il sûr — effets hors transaction (audit, e-mails, + fichiers, métriques), `GET_LOCK` relâché puis repris, corps réutilisé ? Les validations et le + pré-vol faits une seule fois restent-ils valables à la tentative suivante ? +- `settle_invoice` : l'`UPDATE` inconditionnel (`paid_at` à `NULL` sur un partiel) peut-il effacer un + `paid_at` légitime ? `rows_affected` non vérifié — une facture non `validated` ? +- Les tests de course : montages réellement déterministes ? `LOCK TABLES` sur une connexion du pool + rendue au pool verrouillée si le test panique avant `UNLOCK TABLES` ? Le test d'interblocage : la + victime est-elle garantie ? Un test qui passerait à vide ? +- Inventaire : tout autre écrivain du reste dû ou du statut d'une facture validée qui n'incrémente pas + `version` (`grep -rn "UPDATE invoices\|invoice_settlements\|credit_note" crates/*/src`). + +## Lentille 3 — Acceptance Auditor + +Diff, fiche `_bmad-output/implementation-artifacts/25-4-c2-verrou-acceptation.md`, lecture du dépôt. +Chaque AC (1 à 7) contre le code ; « Ce qu'il ne faut pas faire » (aucun `FOR UPDATE`, pas d'élargissement +de `is_deadlock_error`, pas de classement par le texte, pas de `sleep`, pas de changement d'isolation) ; +le Dev Agent Record (affirme-t-il seulement ce qui a tourné ? décomptes recomptés depuis la source ? +la dérogation au montage de l'AC 5 est-elle justifiée ?). ⛔ **Le manuel** : +`docs/manual/fr/user-manual.tex` (règlement manuel, réconciliation) et `docs/api-external.md` — +l'entrée *Accepter des propositions de rapprochement* dit-elle vrai sur le code (codes, statuts, +champs) ? PDF aplati : `pdftotext docs/manual/fr/user-manual.pdf - | tr '\n' ' ' | tr -s ' '`. Le +CHANGELOG dit-il vrai ? diff --git a/_bmad-output/implementation-artifacts/25-4-c2-review-prompt-p2.md b/_bmad-output/implementation-artifacts/25-4-c2-review-prompt-p2.md new file mode 100644 index 00000000..ee58b682 --- /dev/null +++ b/_bmad-output/implementation-artifacts/25-4-c2-review-prompt-p2.md @@ -0,0 +1,42 @@ +# Prompt — revue de code P2 ciblée, Story 25-4-c2 (le verrou à l'acceptation) + +*Versionné le 2026-09-30. **Passe ciblée** (CLAUDE.md § « La passe ciblée ») : une lentille (Haiku 4.5), +contexte frais, braquée sur la remédiation P1 seule — `git show b3a740a8`, écrit dans +`/tmp/claude-1000/-home-gcorbaz-devel-kesh/379e6f94-8029-42cb-9720-fa27c2fb204c/scratchpad/25-4-c2-p2.diff`. +Dépôt `/home/gcorbaz/devel/kesh`, lecture seule. Fiche : `_bmad-output/implementation-artifacts/25-4-c2-verrou-acceptation.md`.* + +## Axes — tous obligatoires + +1. **Les gardes `rows_affected() != 1`** (`crates/kesh-db/src/repositories/invoice_settlements_write.rs`, + dans `settle_invoice` et `cancel_settlement_in_tx`) : lis les deux fonctions en entier dans le fichier + courant. Un chemin **légitime** peut-il rendre 0 ligne et désormais échouer à tort (facture `cancelled` + par un avoir, facture dont le règlement s'annule après avoir été créditée, `WHERE` sans garde de + statut dans l'annulation) ? Qui appelle `cancel_settlement_in_tx` (`grep -rn "cancel_settlement_in_tx" crates/`), + et chacun garantit-il la ligne ? Comment `DbError::Invariant` est-il rendu au client + (`crates/kesh-api/src/errors.rs`) ? +2. **`RELEASE SAVEPOINT`** dans `accept_batch` (`crates/kesh-api/src/routes/reconciliation.rs`) : la + nouvelle branche est-elle correcte ; l'erreur d'origine est-elle préservée ? +3. **`transaction_aborted_outside_accept`** : les trois sites l'appellent-ils, et `drop(tx_outer)` est-il + toujours fait **avant** ? +4. **Le test** : la connexion détachée (`.detach()`) — `&mut verrou` sur un `MySqlConnection`, les deux + requêtes passent-elles bien par elle ? Une connexion détachée non fermée explicitement bloque-t-elle + quoi que ce soit en fin de test ? +5. **`docs/api-external.md`** : la phrase ajoutée est-elle exacte sur le code (`500 INTERNAL_ERROR`, + « rien n'a été écrit ») ? + +## Ce que tu rends + +- **Findings** : sévérité, endroit exact, **preuve** : la commande exécutée **et sa sortie copiée**, ou + l'extrait de code lu avec son numéro de ligne dans le fichier courant. Pour toute affirmation qu'un + élément est absent, le `grep -rnF` qui le prouve. Un finding sans preuve ne sera pas retenu ; un + « 0 finding » sans preuve non plus. +- ⛔ **La liste des axes réellement exercés ET de ceux qui ne l'ont pas été.** + +## Interdits + +⛔ N'écris aucun fichier du dépôt ; aucune commande qui écrit dans le dépôt ou dans une base — +`scripts/prepare-release.sh`, `scripts/regen-test-schema.sh`, `scripts/install-hooks.sh`, +`scripts/test-fast.sh`, `scripts/mem-guard.sh`, `make`, `latexmk`, tout `git commit`/`push`/`add`/ +`stash`/`reset`/`rebase`/`checkout`/`switch`/`worktree`, `sqlx migrate`, `cargo test`/`nextest`, +`npm run`, `npx playwright`, `gh issue create`/`comment`/`edit`. Autorisés : lecture, `grep`, +`git log`/`show`/`diff`, `gh issue view`, `cargo check`. diff --git a/_bmad-output/implementation-artifacts/25-4-c2-validate-prompt-p1.md b/_bmad-output/implementation-artifacts/25-4-c2-validate-prompt-p1.md new file mode 100644 index 00000000..9f0a6624 --- /dev/null +++ b/_bmad-output/implementation-artifacts/25-4-c2-validate-prompt-p1.md @@ -0,0 +1,62 @@ +# Prompt — validation P1, Story 25-4-c2 (le verrou à l'acceptation) + +*Versionné le 2026-09-30. **Une lentille** (Sonnet), contexte frais.* + +Dépôt `/home/gcorbaz/devel/kesh`, branche `story/25-4-c2-verrou-acceptation` (empilée sur la 25-4-c, +PR #483). Fiche à valider : `_bmad-output/implementation-artifacts/25-4-c2-verrou-acceptation.md`. +Source des faits : `25-4-c-residuel-au-rapprochement.md` (Change Log, validation P3, findings F1–F3). +Issue : `gh issue view 480`. Règles : `CLAUDE.md`. Checklist : `.claude/skills/bmad-create-story/checklist.md`. + +Le choix par défaut (rejeu inclus) est **retenu** : ne pas le contester, en contester la mise en œuvre. + +## Axes — tous obligatoires + +1. **Chaque référence `fichier:ligne`** existe et dit ce que la fiche affirme. +2. **L'inventaire des écrivains** — pars du symptôme, pas de la liste : tout ce qui change le reste dû + d'une facture client, c'est-à-dire `invoice_settlements`, les lignes de facture (`invoice_lines`), + les avoirs (`credit_notes`, `credit_note_lines`), le statut (`invoices.status`). + `grep -rn "invoice_settlements\|invoice_lines\|credit_note" crates/*/src --include=*.rs | grep -iE "insert|delete|update"`. + Chacun incrémente-t-il `invoices.version` dans la même transaction ? Un écrivain oublié qui + n'incrémente pas est au moins HIGH : il rouvre la course. +3. **Le raisonnement d'isolation** : sous REPEATABLE READ (MariaDB 10.11, `innodb_snapshot_isolation` + OFF), le verrou optimiste `UPDATE … WHERE version = ?` ferme-t-il réellement la course n° 1 une + fois l'invariant posé ? Cherche un contre-exemple : un ordre d'événements où l'acceptation lit un + reste périmé et où son `UPDATE` réussit quand même. Et `due_after` / `fully_settled`, calculés sur + l'instantané après l'insertion du règlement : peuvent-ils poser `paid_at` à tort quand le contrôle + de version a réussi ? +4. **Le rejeu (AC 4)** : l'affirmation « le 1213 remonte en 1305 au `ROLLBACK TO SAVEPOINT` » est-elle + exacte sur MariaDB 10.11 (lis la doc de MariaDB si tu peux ; sinon dis-le) ? Les autres chemins du + lot (`accept_one_split`, `accept_one_rule`) sont-ils touchés par la même reconnaissance ? Rejouer + un lot entier est-il sûr (effets hors transaction : audit, e-mails, fichiers) ? Le verrou + `GET_LOCK` est-il bien relâché puis repris entre deux tentatives ? +5. **Faisabilité des tests (AC 5)** : le montage proposé (verrou `fiscal_years` tenu par le test, + règlement concurrent daté dans un autre exercice) fonctionne-t-il vraiment ? L'acceptation + atteint-elle `find_open_covering_date` **après** la garde de trop-perçu ? Un règlement manuel + dans un autre exercice est-il permis par `settle_invoice` (date ≥ date de facture, exercice ouvert) ? + Un interblocage déterministe est-il montable ? Mutations tuables ? +6. **Effets de bord de l'invariant** : qui lit `invoices.version` d'une facture validée (frontend, + API, clés d'API, tests) et casserait si un règlement partiel l'incrémente ? La réponse de + `POST /invoices/{id}/settlements` porte-t-elle la facture relue après validation ? +7. **Le manuel et les textes** : `docs/manual/fr/user-manual.tex` (règlement manuel, réconciliation) + et `docs/api-external.md` — que promettent-ils sur la concurrence ou sur `version` ? PDF aplati + (`pdftotext docs/manual/fr/user-manual.pdf - | tr '\n' ' ' | tr -s ' '` vers + `/tmp/claude-1000/-home-gcorbaz-devel-kesh/379e6f94-8029-42cb-9720-fa27c2fb204c/scratchpad/`). +8. **Périmètre et cohérence** : AC ↔ tâches, modules recomptés ; la story laisse-t-elle un état pire + qu'aujourd'hui sur un point quelconque ? + +## Ce que tu rends + +- **Findings** : sévérité (CRITICAL/HIGH/MEDIUM/LOW), endroit exact, **preuve** (commande exécutée et + sa sortie, ou code lu cité), correction proposée. +- ⛔ **La liste des axes réellement exercés ET de ceux qui ne l'ont pas été.** Un rapport sans elle + ne compte pas. + +## Interdits + +⛔ N'écris aucun fichier du dépôt ; aucune commande qui écrit dans le dépôt ou dans une base — +`scripts/prepare-release.sh`, `scripts/regen-test-schema.sh`, `scripts/install-hooks.sh`, +`scripts/test-fast.sh`, `scripts/mem-guard.sh`, `make`, `latexmk`, tout `git commit`/`push`/`add`/ +`stash`/`reset`/`rebase`/`checkout`/`switch`/`worktree`, `sqlx migrate`, `cargo test`/`nextest`, +`npm run`, `npx playwright`, `gh issue create`/`comment`/`edit`, toute requête d'écriture sur MariaDB. +Autorisés : lecture, `grep`, `git log`/`show`/`diff`, `gh issue view`, `pdftotext` vers le +scratchpad, `cargo check`, requêtes `SELECT`/`SHOW` en lecture seule. diff --git a/_bmad-output/implementation-artifacts/25-4-c2-validate-prompt-p2.md b/_bmad-output/implementation-artifacts/25-4-c2-validate-prompt-p2.md new file mode 100644 index 00000000..a1e9957d --- /dev/null +++ b/_bmad-output/implementation-artifacts/25-4-c2-validate-prompt-p2.md @@ -0,0 +1,43 @@ +# Prompt — validation P2, Story 25-4-c2 (le verrou à l'acceptation) + +*Versionné le 2026-09-30. **Une lentille** (Haiku 4.5), contexte frais.* + +Dépôt `/home/gcorbaz/devel/kesh`, branche `story/25-4-c2-verrou-acceptation`. Fiche : +`_bmad-output/implementation-artifacts/25-4-c2-verrou-acceptation.md` — **lis le fichier dans son état +actuel**. Ce que la P1 a changé : `git show cc792871 -- _bmad-output/implementation-artifacts/25-4-c2-verrou-acceptation.md` +(contexte seulement ; les numéros de ligne font foi dans les fichiers courants, jamais dans le diff). +Règles : `CLAUDE.md`. + +## Axes — tous obligatoires + +1. **La remédiation P1** : + - AC 4 : une erreur typée dédiée posée par `accept_batch` quand `ROLLBACK TO SAVEPOINT` échoue — + est-ce réalisable dans `crates/kesh-reconciliation/src/errors.rs` (`ReconciliationError`) et le + `match` de la route (`crates/kesh-api/src/routes/reconciliation.rs`, autour de `lock_result`) ? + Ce `match` est-il exhaustif, et la nouvelle variante y aurait-elle un bras ? Le prédicat de + `retry_with` voit-il un `AppError` ou un `ReconciliationError` ? `with_account_lock` + (`crates/kesh-reconciliation/src/mutex.rs:66`) relâche-t-il `GET_LOCK` quand la closure échoue ? + - Les deux inventaires : `credit_notes.rs:283` et `:586`, `invoices.rs:1453` et `:1577` — lis-les. + Existe-t-il un autre écrivain du statut d'une facture validée ou de son avoir + (`grep -rn "UPDATE invoices SET" crates/*/src`) qui n'incrémente pas `version` ? + - AC 7 : la section `docs/api-external.md:287` et suivantes. +2. **Le contre-exemple** : cherche un ordre d'événements où, l'invariant posé, l'acceptation écrit un + règlement fondé sur un reste périmé **et** réussit son `UPDATE invoices … version = ?`. +3. **Cohérence interne** : AC ↔ tâches, « Ce qu'il ne faut pas faire », Change Log. + +## Ce que tu rends + +- **Findings** : sévérité, endroit exact, **preuve** : la commande exécutée **et sa sortie copiée**, ou + l'extrait de code lu avec son numéro de ligne. Pour toute affirmation qu'un élément est **absent**, le + `grep -rnF` qui le prouve et sa sortie. Un finding sans preuve ne sera pas retenu ; un « 0 finding » + sans preuve non plus. +- ⛔ **La liste des axes réellement exercés ET de ceux qui ne l'ont pas été.** + +## Interdits + +⛔ N'écris aucun fichier du dépôt ; aucune commande qui écrit dans le dépôt ou dans une base — +`scripts/prepare-release.sh`, `scripts/regen-test-schema.sh`, `scripts/install-hooks.sh`, +`scripts/test-fast.sh`, `scripts/mem-guard.sh`, `make`, `latexmk`, tout `git commit`/`push`/`add`/ +`stash`/`reset`/`rebase`/`checkout`/`switch`/`worktree`, `sqlx migrate`, `cargo test`/`nextest`, +`npm run`, `npx playwright`, `gh issue create`/`comment`/`edit`. Autorisés : lecture, `grep`, +`git log`/`show`/`diff`, `gh issue view`, `cargo check`. diff --git a/_bmad-output/implementation-artifacts/25-4-c2-verrou-acceptation.md b/_bmad-output/implementation-artifacts/25-4-c2-verrou-acceptation.md new file mode 100644 index 00000000..339db6c2 --- /dev/null +++ b/_bmad-output/implementation-artifacts/25-4-c2-verrou-acceptation.md @@ -0,0 +1,371 @@ +# Story 25.4-c2 : Le verrou à l'acceptation d'un rapprochement + +Status: done + +**Issue : [#480]** — ⛔ la PR porte `closes #480`, titre ET corps (§ *Issue Tracking Rule*). + +**Née du découpage de la 25-4-c** (validation P3, sévérité P2 → P3 non décroissante ; accord de +Guy le 2026-09-29). Les findings P3 F1–F3 de la 25-4-c sont la source des faits, recontrôlés ici +dans le code. **Empilée sur la 25-4-c** (PR #483, non mergée) : branche +`story/25-4-c2-verrou-acceptation` tirée de `4c36ac99`. La 25-4-c fait d'une facture partiellement +réglée un candidat **ordinaire** du rapprochement : la course décrite ici devient atteignable d'un +clic, d'où l'ordre. + +## Story + +En tant que comptable, +je veux qu'un rapprochement et un règlement manuel enregistrés en même temps sur la même facture ne +puissent pas la régler deux fois, +afin que le compte clients ne devienne jamais créditeur sans que rien ne le signale. + +## Le défaut, vérifié dans le code + +**Isolation réelle** : MariaDB **10.11** en dev, en CI et en production (`docker-compose*.yml`, +`.github/workflows/ci.yml`), `REPEATABLE-READ`, `innodb_snapshot_isolation = OFF`, attente de verrou +50 s (relevé le 2026-09-30 sur `kesh-mariadb-dev`). Conséquence : une lecture **simple** lit +l'**instantané** de la transaction (fixé à sa première lecture InnoDB) ; une lecture **verrouillante** +(`FOR UPDATE`) et un `UPDATE` lisent la **dernière version validée**. + +### Comment l'acceptation se protège aujourd'hui : le verrou optimiste + +`accept_one_invoice` (`crates/kesh-api/src/routes/reconciliation.rs:1067`) lit la facture sans +verrou (`find_invoice_by_id_for_company`, étape 5, `:1115`), retient `invoice_version_pre` +(`:1255`), lit le reste dû sans verrou (`invoice_settlements::amount_due`, garde de trop-perçu +`:1341`), écrit l'encaissement, puis termine par +`UPDATE invoices … WHERE … AND version = ?` (`:1510-1518`). Cet `UPDATE` lit la version **courante** : +si une autre transaction a validé une écriture qui a **incrémenté `version`** depuis l'instantané, il +ne touche aucune ligne et la proposition sort en `RECONCILIATION_INVOICE_NOT_ELIGIBLE` +(`race_during_update`, `:1527-1533`) — son point de sauvegarde est annulé, rien n'est écrit. + +**Le verrou optimiste ne vaut que si TOUT écrit qui change le reste dû incrémente `version`.** + +### L'inventaire des écrivains de `invoice_settlements` (ensemble clos) + +`grep -rn "INSERT INTO invoice_settlements\|DELETE FROM invoice_settlements\|UPDATE invoice_settlements\|invoice_settlements::create_in_tx" crates/*/src` +(le 2026-09-30) : + +| Écrivain | Site | Incrémente `version` ? | +|---|---|---| +| Acceptation d'un rapprochement | `reconciliation.rs:1428` (création) + `:1510` | **toujours** | +| Règlement manuel — `settle_invoice` | `invoice_settlements_write.rs:218` + `:233-246` | ⛔ **seulement au solde** (`if fully_settled`) | +| Annulation d'un règlement — `cancel_settlement_in_tx` (règlement manuel **et** annulation d'un rapprochement, `reconciliation_cancel.rs:352`) | `invoice_settlements_write.rs:451` + `:464-470` | **toujours** (les deux branches) | +| Rejeu post-restauration | `post_restore/20260828000001_invoice_settlements_type.sql:23` | hors ligne (import d'installation) | +| Tests | `invoices.rs:4220`, `:4257` | `mod tests` | + +**Au-delà des règlements** — le reste dû soustrait aussi l'**avoir émis** +(`INVOICE_AMOUNT_DUE_DERIVED_SQL`, `invoice_settlements.rs:105-106`), et l'acceptation exige le statut +`validated`. Les autres écrivains qui touchent ces grandeurs, vérifiés sûrs : + +| Écrivain | Site | Incrémente `version` ? | +|---|---|---| +| Émission d'un avoir — `create_credit_note` | `credit_notes.rs:283` (`FOR UPDATE` en tête) + `:586` (`status = 'cancelled'`) | **toujours** | +| Dévalidation — `invoices::unvalidate` | `invoices.rs:1453` (`FOR UPDATE` en tête) + `:1577` | **toujours** | +| Lignes de facture (`invoice_lines`) | brouillon seulement : une facture validée est immuable | sans objet | + +L'`UPDATE … AND version = ? AND status = 'validated'` d'`accept_one_invoice` refuse donc déjà ces deux +courses. **C'est l'ensemble complet** qu'énonce l'invariant de l'AC 1. + +**Le trou est unique** : un règlement manuel **partiel** ne touche pas `version`. D'où les deux +courses : + +1. **Acceptation contre règlement manuel partiel** — l'acceptation fige son instantané, le règlement + manuel partiel (400 sur 1 000) valide **sans** incrémenter `version`, l'acceptation lit le reste + périmé (1 000), passe la garde de trop-perçu avec 1 000, et son `UPDATE … version = ?` **réussit**. + Facture réglée 1 400 pour 1 000 : **compte clients créditeur**, sans erreur. +2. **Acceptation contre acceptation** (deux comptes bancaires, donc deux `GET_LOCK` distincts) — la + seconde est **déjà** refusée par le verrou optimiste, puisque l'acceptation incrémente toujours. + À garder tel quel. + +Symétriquement, un règlement manuel qui suit une acceptation en cours attend sur la ligne `invoices` +(son `FOR UPDATE`, `:63-68`, est sa **première** instruction ; l'acceptation tient un verrou exclusif +depuis son `UPDATE`) puis lit un état à jour : ce sens-là est sûr. + +### ⛔ Pourquoi PAS un `FOR UPDATE` à l'acceptation (P3 F1 de la 25-4-c) + +Posé au chargement de la facture, il lirait `version` **à jour** — donc le contrôle `version = ?` +passerait toujours — alors que `amount_due`, lecture simple, lirait encore l'**instantané** +(`reconciliation_cancel.rs:292-293` et `users.rs:100-107` documentent la même règle). Il **désarmerait** +le verrou optimiste qui refuse aujourd'hui la course n° 2, sans fermer la n° 1. Le patron de +`settle_invoice` ne se transpose pas : son `FOR UPDATE` est la première instruction de SA transaction, +alors que dans un lot l'instantané est fixé dès la première proposition. + +### Les interblocages, et leur issue actuelle (P3 F3) + +Le lot garde, d'une proposition à l'autre, le verrou d'exercice (`find_open_covering_date … FOR +UPDATE`, `:1362`) et celui de la numérotation des écritures ; le règlement manuel et l'annulation +prennent la **facture puis** l'exercice (`invoice_settlements_write.rs:68` → `:174` ; +`reconciliation_cancel.rs:296` → `:309`). Sur deux comptes bancaires distincts, InnoDB peut donc +choisir l'acceptation comme victime d'un interblocage (1213) — **préexistant** (le commentaire +`reconciliation.rs:3616-3619` le décrit déjà pour l'annulation). Il annule **toute** la transaction ; +l'instruction fautive ressort en `FailedProposal` `DATABASE_ERROR`, puis `ROLLBACK TO SAVEPOINT` +(`:979`) échoue faute de point de sauvegarde, le `?` produit `ReconciliationError::Database`, et la +réponse est un **HTTP 500** — sans rejeu : `retry_with` n'existe que sur l'annulation (`:3624-3646`). +Une attente dépassée (1205) n'annule, elle, que l'instruction (`innodb_rollback_on_timeout` par +défaut) : le point de sauvegarde survit, la proposition sort en échec au bout de 50 s. + +## Acceptance Criteria + +**AC 1 — L'invariant.** Tout écrit qui change le reste dû d'une facture incrémente `invoices.version` +dans la **même** transaction. Concrètement : `settle_invoice` incrémente `version` (et `updated_at`) +**à chaque règlement**, partiel compris — `paid_at` n'est posé qu'au solde, comme aujourd'hui. Le +doc-comment de `settle_invoice` et celui d'`accept_one_invoice` (au `UPDATE … version = ?`) énoncent +l'invariant et nomment les **deux** inventaires ci-dessus (règlements, et avoir / dévalidation) : le +prochain écrivain qui y manquerait doit trouver la règle écrite là où il écrit. + +**AC 2 — La course n° 1 est refusée.** Une acceptation dont l'instantané précède un règlement manuel +partiel validé depuis sort en `RECONCILIATION_INVOICE_NOT_ELIGIBLE` (`race_during_update`), sans rien +écrire : aucun règlement, aucune écriture d'encaissement, transaction bancaire toujours `pending`. +**Test déterministe** (sans `sleep`, AC 5). + +**AC 3 — La course n° 2 reste refusée.** Deux acceptations du même solde sur la même facture, depuis +deux comptes bancaires : une acceptée, l'autre refusée, jamais deux règlements. Test déterministe. + +**AC 4 — Un interblocage se rejoue.** Quand la transaction du lot est annulée par InnoDB (1213), la +route d'acceptation **rejoue toute l'opération** — transaction neuve, verrou de compte repris —, par +`retry_with`, patron de `post_cancel_reconciliation` (`reconciliation.rs:3612-3646`). ⚠️ L'erreur qui +remonte aujourd'hui n'est **pas** le 1213 mais l'échec du `ROLLBACK TO SAVEPOINT` (1305, mode d'échec +connu du couple interblocage / point de sauvegarde) : `accept_batch` doit reconnaître qu'une +transaction a été annulée sous lui et le faire remonter par une **erreur typée dédiée** (par exemple +un variant `ReconciliationError::TransactionAborted`, posé explicitement quand `ROLLBACK TO SAVEPOINT` +échoue en 1305), que le prédicat de rejeu **de la route d'acceptation** reconnaît, au même titre que +`is_deadlock_error`. ⛔ **Ne pas élargir `kesh_db::retry::is_deadlock_sqlx` / `is_deadlock_error`** +(`retry.rs:71-85`, 1213 seulement) : ils sont partagés par tout le crate, et reclasser tout 1305 en +interblocage masquerait ailleurs un vrai défaut de point de sauvegarde. ⛔ **Ne pas classer en lisant +le texte d'un message d'erreur** : le code d'erreur MySQL, pas la chaîne. +Chemin à tenir, vérifié dans le code : le prédicat de `retry_with` voit un **`AppError`** (patron +`:3635`) ; la variante typée traverse donc le `match lock_result` de la route (exhaustif, un bras à +ajouter) vers une variante d'`AppError` que le prédicat reconnaît, et qui, faute de rejeu possible, +rend un 500. Un 1213 qui remonte **directement** par un `?` (`SAVEPOINT`, `RELEASE SAVEPOINT`) arrive +déjà en `AppError::Database(DbError::Sqlx(1213))` : `is_deadlock_error` le couvre. `GET_LOCK` est un +verrou de **session**, relâché par `with_account_lock` même quand la closure échoue +(`mutex.rs:66-160`) : une nouvelle tentative, transaction neuve, le reprend proprement. Après `DEFAULT_MAX_DEADLOCK_ATTEMPTS` tentatives, l'erreur finale reste un 500. +Test : un interblocage provoqué de façon déterministe (deux connexions, +`attendre_une_requete_en_cours`) est rejoué et la proposition finit acceptée. Si un interblocage +déterministe s'avère impossible à monter, le Dev Agent Record le dit et le test porte sur la +reconnaissance de l'annulation (1305 après `ROLLBACK`) — **jamais** un test qui passe à vide. + +**AC 5 — Tests déterministes, et qui auraient échoué avant.** Chaque test de course : +- met l'acceptation en attente **à un point connu** (par exemple : une connexion de test tient + `FOR UPDATE` sur la ligne `fiscal_years` que l'acceptation va verrouiller en (d), **après** sa garde + de trop-perçu), attend qu'elle y soit (`kesh_db::test_fixtures::attendre_une_requete_en_cours`, + `test_fixtures.rs:514` ; patron `credit_notes_repository.rs:594-640`), fait valider l'écriture + concurrente, puis relâche ; +- ⚠️ le règlement manuel concurrent ne doit pas dépendre du verrou que tient le test : le dater dans + un **autre exercice ouvert** que celui que l'acceptation attend, ou tout autre montage qui évite de + bloquer le règlement derrière le verrou de test ; +- ⚠️ ne met dans le lot **que** la proposition sous test : `ROLLBACK TO SAVEPOINT` ne relâche pas les + verrous de ligne pris après le point de sauvegarde (seuls ceux des lignes insérées) — une + proposition antérieure sur la même facture garderait la ligne `invoices` verrouillée jusqu'à la fin + du lot et fausserait le montage ; +- est **éprouvé par mutation** : l'incrément de `version` retiré du règlement partiel, le test AC 2 + échoue (double règlement) ; le rejeu retiré, le test AC 4 échoue. + +**AC 6 — Rien d'autre ne change.** Aucun `FOR UPDATE` ajouté dans `accept_one_invoice` ; aucune +modification de la formule du reste dû ; l'ordre des gardes (score avant trop-perçu) intact. Les +écrans qui gardent `version` après un règlement la relisent : `+page.svelte` de la facture reprend +`res.invoice` (`invoices/[id]/+page.svelte:435-436`) — vérifier que la réponse du règlement porte la +facture **relue après validation**, sans quoi un `pauseDunning` qui suit (`:512`, version en garde) +sortirait en 409. Les tests existants qui assertent la `version` d'une facture après un règlement +partiel sont mis à jour, et le Dev Agent Record les nomme. + +**AC 7 — Textes.** `docs/api-external.md` ne documente **pas** `POST /api/v1/reconciliation/accept` +(la section *Annuler un rapprochement bancaire*, `:287`, ne couvre que la consultation et +l'annulation) : y ajouter une entrée courte pour l'acceptation — accès, corps, succès partiel +`{ accepted, failed }`, et le rejeu d'un interblocage transitoire, dans les termes de l'annulation +(`:291`). CHANGELOG `[0.12.1]` *Fixed*. Le manuel : relire la section *Réconciliation +bancaire* et le règlement manuel — n'y rien écrire s'ils ne promettent rien sur la concurrence, et le +dire dans le Dev Agent Record. + +## Tasks / Subtasks + +- [x] **T1 — l'invariant** (AC 1, 6) : `settle_invoice` incrémente `version` à chaque règlement ; + doc-comments ; réponse du règlement relue après validation ; tests existants de `version`. +- [x] **T2 — le rejeu** (AC 4) : `accept_batch` reconnaît la transaction annulée (erreur typée + dédiée) ; route d'acceptation sous `retry_with` (une fonction « une tentative », comme + `cancel_reconciliation_once`), prédicat local à la route. +- [x] **T3 — tests de course et mutations** (AC 2, 3, 5). +- [x] **T4 — textes** (AC 7). +- [x] **T5 — gates** : backend complet (base remise à zéro — ⛔ `kesh-db` touché : gate complet même en + cours de boucle), frontend complet, **E2E complet**. + +## Dev Notes + +### Ce qu'il ne faut pas faire + +- ⛔ **`FOR UPDATE` sur la facture dans `accept_one_invoice`** — il désarme le verrou optimiste (voir + plus haut). Toute autre lecture verrouillante qui rafraîchirait `invoice_version_pre` aussi. +- ⛔ **Classer une erreur par son texte** (`contains("Deadlock")`) : le code MySQL. +- ⛔ **Élargir `is_deadlock_error` au 1305** : prédicat partagé ; la reconnaissance du point de + sauvegarde perdu reste locale à l'acceptation (`reconciliation.rs` est le seul site à utiliser des + `SAVEPOINT` nommés du dépôt). +- ⛔ **Un test de course avec `sleep`** : il passe à vide un jour sur deux (`tests-qui-prouvent-moins`). +- ⛔ **Changer le niveau d'isolation** (`READ COMMITTED`, `SERIALIZABLE`) pour régler la course : + `pool.rs:17-21` assume REPEATABLE READ pour tout le crate. +- ⚠️ **Mise à niveau de MariaDB** : à partir de la 11.6, `innodb_snapshot_isolation` vaut `ON` par + défaut — une lecture verrouillante ou un `UPDATE` sur une ligne modifiée depuis l'instantané échoue + alors (1020, *Record has changed since last read*) au lieu de lire la dernière version. Le verrou + optimiste reste juste dans les deux régimes, mais l'erreur change de forme : le dire dans le + doc-comment d'`accept_one_invoice`, pour que la mise à niveau ne le découvre pas en production. + +### Où regarder + +| Fichier | Pourquoi | +|---|---| +| `crates/kesh-api/src/routes/reconciliation.rs:700-990` | route d'acceptation, `with_account_lock`, `accept_batch` et ses points de sauvegarde | +| `crates/kesh-api/src/routes/reconciliation.rs:1067-1575` | `accept_one_invoice` : lectures, gardes, `UPDATE … version = ?` | +| `crates/kesh-api/src/routes/reconciliation.rs:3612-3680` | patron du rejeu (`post_cancel_reconciliation`, `cancel_reconciliation_once`) | +| `crates/kesh-db/src/repositories/invoice_settlements_write.rs:40-260` | `settle_invoice` : verrou, garde, incrément conditionnel à changer | +| `crates/kesh-db/src/repositories/invoice_settlements_write.rs:371-480` | `cancel_settlement_in_tx` : incrément inconditionnel, le modèle | +| `crates/kesh-db/src/retry.rs:39-160` | `retry_with`, `is_deadlock_error`, `DEFAULT_MAX_DEADLOCK_ATTEMPTS` | +| `crates/kesh-reconciliation/src/mutex.rs:66-120` | `with_account_lock` (`GET_LOCK`), à reprendre à chaque tentative | +| `crates/kesh-db/src/test_fixtures.rs:505-540`, `crates/kesh-db/tests/credit_notes_repository.rs:594-640` | attente déterministe d'une requête bloquée | +| `crates/kesh-api/tests/reconciliation_e2e.rs` | `setup_company`, `seed_validated_invoice`, `post_accept_one`, `seed_partially_settled_vat_invoice` (25-4-c) | +| `frontend/src/routes/(app)/invoices/[id]/+page.svelte:430-440, 505-520` | version relue après règlement, `pauseDunning` | + +### Gardes-fous du dépôt + +- Aucune migration. `kesh-db` touché (repository) ⇒ **gate complet même en cours de boucle**. +- Modules : `kesh-db`, `kesh-api` (+ `docs`) — sous le seuil de découpage. +- ⚠️ La base partagée : les tests de course sont des `#[sqlx::test]` (base éphémère), jamais sur + `kesh` — deux connexions du même pool éphémère. + +## Questions pour Guy + +Aucune bloquante. Un choix est fait par défaut et révisable : **le rejeu (AC 4) est inclus**. L'issue +#480 le pose comme question (« `retry_with` ? ») ; sans lui, la story laisse un 500 sur un +interblocage que la 25-4-c rend plus probable, et le patron existe déjà sur l'annulation. + +## Dev Agent Record + +### Agent Model Used + +Claude Opus 5.5 (`claude-opus-5-5`). + +### Debug Log References + +- **Le montage proposé par l'AC 5 ne tient pas** : la recherche d'exercice (`find_open_covering_date … FOR + UPDATE`) parcourt les exercices de la société par ordre de date de début (index + `uq_fiscal_years_company_start_date`) et verrouille chaque ligne examinée. Le règlement manuel et + l'acceptation se croisent donc **toujours** sur les exercices, quel que soit celui que le test tient + — premier essai : le règlement attend 50 s (1205). La vraie fenêtre de la course va de la première + lecture de l'acceptation à sa recherche d'exercice, et l'acceptation n'y fait que des lectures + simples. **Montage retenu** : une lecture simple n'attend pas un verrou de ligne, mais elle attend un + verrou de **métadonnées** — `LOCK TABLES contacts WRITE` arrête l'acceptation à la lecture du contact + (étape 5bis), instantané figé, **avant** la garde de trop-perçu. Le règlement manuel ne lit pas + `contacts`. +- **Rouge avant correctif, pour la bonne raison** : `accept_refuses_when_a_partial_manual_settlement_lands_meanwhile` + a rendu `accepted: [1]` — la facture de 1 000.— réglée 1 400.—. +- L'acceptation bute sur la facture dès l'**insertion du règlement** (la clé étrangère vers `invoices` + y pose un verrou partagé), pas à l'`UPDATE invoices` : motif d'attente du test d'interblocage ajusté. +- Mutations (chacune restaurée, fichier touché ensuite) : incrément conditionnel rétabli au règlement + partiel → test de la course n° 1 rouge (double règlement) ; prédicat de rejeu à `false` → test + d'interblocage rouge (500 `INTERNAL_ERROR`) ; `is_savepoint_lost` à `false` → idem ; `version = ?` + relâché en `version >= ?` dans l'acceptation → les tests des courses n° 1 **et** n° 2 rouges. +- Gate E2E : 225 passed / 19 skipped / 10 failed. Les 10, un par un : 7 KF-029 (#97) ; + `sidebar-navigation.spec.ts:75` = KF-046 (#424), à la liste, vert rejoué seul deux fois ; + `accounts.spec.ts:145` et `product-revenue-account.spec.ts:133` rouges rejoués sur la base salie par + la suite, **verts sur `kesh_e2e` reconstruite** — pollution d'état. KF-045 non déclenchée (run à + 14:13 UTC). + +### Completion Notes List + +- **L'invariant (AC 1)** : `settle_invoice` incrémente `version` et `updated_at` à **chaque** règlement ; + `paid_at` n'est posé qu'au solde (`NULL` sinon — il l'était déjà : une facture payée n'accepte plus de + règlement). Doc-comments sur `settle_invoice` et sur l'`UPDATE … version = ?` d'`accept_one_invoice`, + qui nomment les deux inventaires, l'interdit du `FOR UPDATE` et la bascule MariaDB 11.6. +- **Le rejeu (AC 4)** : variante `ReconciliationError::TransactionAborted`, posée par `accept_batch` + quand `ROLLBACK TO SAVEPOINT` échoue en **1305** (`is_savepoint_lost`, sur le code MySQL) ; + `AppError::ReconciliationTransactionAborted` → 500 `INTERNAL_ERROR` une fois les tentatives épuisées ; + `post_accept` rejoue `accept_once` (transaction neuve, `GET_LOCK` repris) par `retry_with`, prédicat + **local** : `is_deadlock_error` **ou** la variante. `is_deadlock_error` inchangé (1213 seul). Les + quatre autres `match` exhaustifs de `ReconciliationError` reçoivent un bras défensif. +- **AC 6** : aucun `FOR UPDATE` ajouté dans `accept_one_invoice`, formule du reste dû et ordre des gardes + intacts. La réponse de `POST /invoices/{id}/settlements` relit déjà la facture après validation + (`routes/invoices.rs`, validation P1) : aucun écran ne garde une version périmée. **Aucun test + existant** n'assertait la version d'une facture après un règlement partiel : le gate backend passe + de 2513 à 2516, les trois nouveaux seuls. +- **Tests** (périmètre : `4c36ac99` → cette branche) : +3 e2e dans `reconciliation_e2e.rs` — course + n° 1 (règlement manuel partiel pendant l'acceptation), course n° 2 (deux acceptations, deux comptes), + interblocage dont l'acceptation est la victime (transaction de test alourdie de 500 lignes, cycle + facture / exercice), chacun sans `sleep`. +- **Textes (AC 7)** : `docs/api-external.md` — entrée neuve *Accepter des propositions de + rapprochement* (accès, corps, succès partiel, `race_during_update`, rejeu, champs de la 25-4-c) ; + CHANGELOG `[0.12.1]` *Fixed*. **Le manuel ne promet rien** sur les opérations simultanées dans le + règlement ni le rapprochement : rien écrit. En le vérifiant, une affirmation fausse trouvée ailleurs + (numérotation des factures « dans une transaction `SERIALIZABLE` », qui n'existe nulle part) → + **issue #484**. + +### File List + +- `CHANGELOG.md` +- `crates/kesh-api/src/errors.rs` +- `crates/kesh-api/src/routes/reconciliation.rs` +- `crates/kesh-api/tests/reconciliation_e2e.rs` +- `crates/kesh-db/src/repositories/invoice_settlements_write.rs` +- `crates/kesh-reconciliation/src/errors.rs` +- `docs/api-external.md` +- `_bmad-output/implementation-artifacts/25-4-c2-verrou-acceptation.md` +- `_bmad-output/implementation-artifacts/25-4-c2-validate-prompt-p1.md` +- `_bmad-output/implementation-artifacts/25-4-c2-validate-prompt-p2.md` +- `_bmad-output/implementation-artifacts/25-4-c2-review-prompt-p1.md` +- `_bmad-output/implementation-artifacts/25-4-c2-review-prompt-p2.md` +- `_bmad-output/implementation-artifacts/sprint-status.yaml` + +## Change Log + +- **2026-09-30** — Créée depuis #480 et les findings P3 F1–F3 de la 25-4-c, recontrôlés dans le code : + isolation réelle relevée sur MariaDB 10.11 ; inventaire clos des écrivains de `invoice_settlements` + — **un seul** n'incrémente pas `version` (le règlement manuel partiel) ; la solution retenue est + l'invariant d'incrément, **pas** un `FOR UPDATE` ; rejeu sur interblocage inclus par défaut. +- **2026-09-30** — Validation P1 (Sonnet, prompt `25-4-c2-validate-prompt-p1.md`) : **1 HIGH, + 2 MEDIUM, 2 LOW**, vérifiés. HIGH : `is_deadlock_error` ne teste que le 1213 (`retry.rs:71-85`), l'AC 4 + le nommait pour reconnaître un 1305 → erreur typée dédiée, prédicat local, interdiction d'élargir le + prédicat partagé. MEDIUM : `api-external.md` ne documente pas `/accept` → une entrée à écrire, pas une + phrase. MEDIUM : l'inventaire ne couvrait que `invoice_settlements` → avoir et dévalidation ajoutés + (vérifiés : `FOR UPDATE` en tête, incrément inconditionnel). LOW : `pool.rs:17-21`, + `reconciliation_cancel.rs:292-293`. Remarque intégrée à l'AC 5 : `ROLLBACK TO SAVEPOINT` ne relâche + pas les verrous de ligne. +- **2026-09-30** — Validation P2 (Haiku, prompt `25-4-c2-validate-prompt-p2.md`) : 1 CRITICAL, 1 HIGH, + 1 MEDIUM annoncés — **tous écartés, erreur de catégorie** : la lentille reproche au code de ne pas + encore porter ce que la fiche prévoit (variante typée, `retry_with`, entrée d'API), ce qui est + l'objet de l'implémentation. Axes utiles, preuves jointes : inventaire avoir / dévalidation vérifié ; + **aucun contre-exemple** à l'invariant (relève aussi la suspension des rappels, qui incrémente + `version` : une acceptation concurrente est refusée, sens prudent). Axe mal exercé (faisabilité du + rejeu) **repris par l'orchestrateur** : `GET_LOCK` de session relâché sur erreur, prédicat sur + `AppError`, 1213 direct déjà couvert — précisions ajoutées à l'AC 4. **Validation close : 0 > LOW.** + + **Bilan** — P1 Sonnet 1H/2M/2L → P2 Haiku 0 > LOW (3 annoncés, écartés). Modèles : Sonnet, Haiku. +- **2026-09-30** — Implémentée (`bmad-dev-story`) : invariant d'incrément au règlement manuel, rejeu sur + interblocage, trois tests de course éprouvés par mutation, entrée d'API, issue #484. ⚠️ Le montage + de test de l'AC 5 (verrou d'exercice) s'est révélé impossible — les deux flux se croisent toujours sur + les exercices — et a été remplacé par un verrou de métadonnées (Debug Log). Gates **réellement + exécutés** : backend complet sur base remise à zéro **2516/2516** (4 ignorés), fmt, clippy ; + frontend `check`, `lint-i18n-ownership`, **836/836**, build ; **E2E 225 / 19 / 10**, les 10 expliqués + un par un. Statut → `review`. +- **2026-09-30** — Revue de code P1 (3 lentilles Sonnet, prompt `25-4-c2-review-prompt-p1.md`, diff + `6810cd97..177135f2`) : Blind Hunter 1 HIGH / 3 MEDIUM / 7 LOW, Edge Case Hunter 5 findings non cotés, + Acceptance Auditor **0** (sept AC, interdits, manuel, API vérifiés). Vérifiés sur le code. **Corrigés** : + HIGH (Blind + Edge) — l'`UPDATE` de `settle_invoice`, ligne dont dépend l'invariant, ne vérifiait pas + `rows_affected` (sûr aujourd'hui par le `FOR UPDATE` initial, muet s'il disparaissait) → `DbError::Invariant` + si ≠ 1, **et** même garde dans `cancel_settlement_in_tx` ; MEDIUM — `RELEASE SAVEPOINT` sans la lecture du + 1305 de la branche d'échec → symétrique ; MEDIUM — `LOCK TABLES` sur une connexion rendue au pool + verrouillée si le test panique (la suppression de la base éphémère attendrait sans fin) → connexion + **détachée** ; MEDIUM — trois bras défensifs identiques → `transaction_aborted_outside_accept` ; + MEDIUM — `api-external.md` taisait le `500` quand les tentatives s'épuisent → écrit. **Laissés LOW** : + archivage du compte non revérifié entre deux tentatives (fenêtre ≤ 150 ms, préexistante hors verrou) ; + `RELEASE_LOCK` en échec après une erreur métier (préexistant, le rejeu ne l'aggrave pas) ; message brut + du 1305 (déjà journalisé) ; `drop` contre `rollback` (cohérent avec les bras voisins) ; heuristique de + la victime (le test asserte que sa transaction survit, il ne peut passer à vide) ; branche non-1305 non + testée ; doublon documentaire de l'invariant. Gate **complet** (`kesh-db` touché) : **2516/2516**. +- **2026-09-30** — Revue de code P2 **ciblée** (Haiku, prompt `25-4-c2-review-prompt-p2.md`, sur + `b3a740a8`) : **0 > LOW**, cinq axes exercés. Recoupée par l'orchestrateur sur l'axe le plus sensible : + `cancel_settlement_in_tx` verrouille la facture en tête et son `UPDATE` n'a pas de garde de statut — la + nouvelle garde ne peut pas refuser à tort ; `DbError::Invariant` rend un `500` journalisé. **Revue + close.** Gate complet au dernier commit : backend **2516/2516** (base remise à zéro), frontend inchangé + depuis le gate précédent (836/836), **E2E 228 passed / 19 skipped / 7 failed** — les 7 KF-029, rien + d'autre (run à 14:43 UTC). + + **Bilan de la revue** — P1 Sonnet ×3 : 1 HIGH / 4 MEDIUM corrigés, 7 LOW laissés → P2 Haiku ciblée : + 0 > LOW. Statut → `done`. + +[#480]: https://github.com/guycorbaz/kesh/issues/480 diff --git a/_bmad-output/implementation-artifacts/sprint-status.yaml b/_bmad-output/implementation-artifacts/sprint-status.yaml index a1b1a193..59fe6b28 100644 --- a/_bmad-output/implementation-artifacts/sprint-status.yaml +++ b/_bmad-output/implementation-artifacts/sprint-status.yaml @@ -360,7 +360,7 @@ development_status: 25-4-b2-residuel-aux-rappels: done # 2026-09-27 revue CLOSE en 2 passes (1H/1M/1L -> 0 >LOW), gates verts (2510, 835, E2E conforme). PR a ouvrir : closes #416. # 2026-09-27 CREEE : frais non comptabilises => QR = reste du SANS frais (regle existante, admin-manual:1200) ; PDF de rappel = variante titree ; Q1 ligne "Frais de rappel", Q2 deja regle masque a zero, M5 papier hors perimetre (#477) ; validation CLOSE en 5 passes (1H/3M -> 0+1M -> 5M -> 1M -> 0 >LOW), pas de decoupage (Guy). # [#416, closes] le rappel reclame le reste du (texte, QR, PDF montant initial / deja regle / reste), frais configurables masques a zero ; point ouvert : frais non comptabilises. 25-4-b-residuel-aux-agregats: split # 2026-09-27 DECOUPEE en b1 (agregats) / b2 (rappels) — six modules apres les arbitrages Q1. # [#416] + montant des RELANCES (TTC complet, trouve a l'inventaire) — Q1 TRANCHEE 2026-09-27 : la QR du rappel porte le RESTE DU (= TTC sans reglement), reste a payer sur le PDF et dans le texte ; point ouvert : frais de rappel non comptabilises vs trop-percu. 25-4-c-residuel-au-rapprochement: done # 2026-09-30 REVUE CLOSE en 2 passes (P1 Sonnet x3 1H/7M -> P2 Haiku x2 0 >LOW apres verification) ; gate complet vert (2513, 836, E2E 226/19/9 attendus) ; #482 ouverte (signe a l acceptation) ; non poussee — PR closes #420 sur accord de Guy. # 2026-09-30 IMPLEMENTEE — gates backend 2513/2513, frontend 836/836, E2E 226/19/9 (9 attendus) ; #481 ouverte (manuel). # 2026-09-29 VALIDATION CLOSE en 5 passes (P4 1M/1L, P5 ciblee 0 >LOW) — prete pour dev. # 2026-09-29 PERIMETRE REDUIT au reste du dans le rapprochement (verrou -> c2 #480, arrondi -> c3 #476) ; validation P1 3H, P2 2M (orchestrateur), P3 1C/3H -> decoupage ; validation reprend en P4. # 2026-09-28 CREEE : le filtre SQL EXCLUT la facture (pas seulement score 0) ; 3 questions a Guy (Q1 inclure #476 arrondi, Q2 manuel du score faux, Q3 montant affiche). # [#420] — DEUX sites : filtre SQL des candidats (HAVING total_ttc) avant le score, puis score et re-score a l'acceptation. - 25-4-c2-verrou-acceptation: backlog # [#480] 2026-09-29 NEE du decoupage de la 25-4-c (validation P3, severite P2 -> P3 non decroissante ; accord de Guy) — accept_one_invoice ne verrouille pas la facture : double reglement possible contre un reglement manuel partiel (settle_invoice ne bumpe version qu'au solde). ⛔ un simple FOR UPDATE au chargement desarme le controle optimiste (REPEATABLE READ : version lue a jour, amount_due perime) — lecture verrouillante du reste du, ou bump de version au reglement partiel. Interblocage lot/exercice -> 500 sans rejeu (retry_with ?). Findings P3 F1-F3 dans le Change Log de la 25-4-c. DOIT suivre la 25-4-c. + 25-4-c2-verrou-acceptation: done # 2026-09-30 REVUE CLOSE en 2 passes (P1 Sonnet x3 1H/4M -> P2 Haiku ciblee 0 >LOW) ; gate complet vert (2516, 836, E2E 228/19/7 KF-029) ; non poussee, empilee sur #483 — PR closes #480 apres le merge de #483. # 2026-09-30 IMPLEMENTEE — gates 2516, 836, E2E 225/19/10 (tous expliques) ; #484 ouverte. # 2026-09-30 VALIDATION CLOSE en 2 passes (P1 1H/2M -> P2 0 >LOW). CREEE (empilee sur 25-4-c, PR #483) : invariant « tout ecrit sur invoice_settlements incremente invoices.version » — seul le reglement manuel partiel y manque ; PAS de FOR UPDATE ; rejeu sur interblocage (1213 remonte en 1305 au ROLLBACK TO SAVEPOINT) inclus par defaut. # [#480] 2026-09-29 NEE du decoupage de la 25-4-c (validation P3, severite P2 -> P3 non decroissante ; accord de Guy) — accept_one_invoice ne verrouille pas la facture : double reglement possible contre un reglement manuel partiel (settle_invoice ne bumpe version qu'au solde). ⛔ un simple FOR UPDATE au chargement desarme le controle optimiste (REPEATABLE READ : version lue a jour, amount_due perime) — lecture verrouillante du reste du, ou bump de version au reglement partiel. Interblocage lot/exercice -> 500 sans rejeu (retry_with ?). Findings P3 F1-F3 dans le Change Log de la 25-4-c. DOIT suivre la 25-4-c. 25-4-c3-arrondi-centime: backlog # [#476] 2026-09-29 NEE du decoupage de la 25-4-c — le reste du peut porter 4 decimales ; comparer au centime : 8 sites (filtre, score, gardes de trop-percu :1336 et settlements_write:167, soldes :1451 et :233, reouverture :464, dialogue SettleInvoiceDialog:111) ; helper sur Money::round_to_centimes, reminder_amount_due rebranche. ⚠️ QUESTION COMPTABLE A GUY : payer 10.01 une facture de 10.0050 laisse la creance crediteur de 0.0050 au grand livre (DECIMAL(19,4)) et affiche « Reste du -0.01 » — assumer, afficher arrondi, ecriture d'ecart, ou arrondir le TTC a la validation. Findings P1 F1-F3, P3 F4/F8/F10/F12 dans le Change Log de la 25-4-c. 25-4-d-solder-le-reste: backlog # [#384] ramenee de la 25-6 le 2026-09-27 (Guy) — perte sur debiteur, escompte, frais : clore une facture partiellement reglee ; la part TVA reduit la TVA due. 25-4-propager-le-residuel: split # 2026-09-26 DECOUPEE (six modules > 5) en 25-4-a / 25-4-b / 25-4-c, puis 25-4-d (#384, 2026-09-27) ; #455 et #456 rattachees. # [#416] [#420] — la 24-2 a introduit le reglement PARTIEL ; le residuel n'est propage ni a la balance agee ni a l'echeancier, et **le score de rapprochement compare au TTC et non au residuel** : le virement du solde d'une facture partiellement reglee **score 0**. diff --git a/crates/kesh-api/src/errors.rs b/crates/kesh-api/src/errors.rs index 3b2b5723..2ae3314c 100644 --- a/crates/kesh-api/src/errors.rs +++ b/crates/kesh-api/src/errors.rs @@ -719,6 +719,13 @@ pub enum AppError { #[error("Échec de libération du verrou de réconciliation")] ReconciliationLockReleaseFailed { bank_account_id: i64 }, + /// Story 25-4-c2 (#480) — la transaction d'un lot d'acceptation a été + /// annulée par InnoDB (interblocage). **Rejouable** : `post_accept` la + /// rejoue ; elle n'arrive au client, en `500`, qu'une fois les tentatives + /// épuisées. + #[error("Transaction de réconciliation annulée par un interblocage")] + ReconciliationTransactionAborted, + // ----- Story 8-5a-base — réconciliation manuelle FR45 ----- /// Le `bank_account` ciblé n'a pas de `journal_account_id` /// configuré (8-5a-zero foundation). Le user doit configurer le @@ -1944,6 +1951,14 @@ impl IntoResponse for AppError { }); (StatusCode::CONFLICT, Json(body)).into_response() } + AppError::ReconciliationTransactionAborted => { + tracing::error!("reconciliation accept: transaction aborted, retries exhausted"); + build_response( + StatusCode::INTERNAL_SERVER_ERROR, + "INTERNAL_ERROR", + &t("error-internal", "Erreur interne"), + ) + } AppError::ReconciliationLockReleaseFailed { bank_account_id } => { let msg = t( "reconciliation-errors-lock-release-failed", diff --git a/crates/kesh-api/src/routes/reconciliation.rs b/crates/kesh-api/src/routes/reconciliation.rs index e0403484..9c798ded 100644 --- a/crates/kesh-api/src/routes/reconciliation.rs +++ b/crates/kesh-api/src/routes/reconciliation.rs @@ -807,25 +807,76 @@ pub async fn post_accept( ))); } - // Step 1 — Acquire UN seul lock pour tout le batch (H5). - let mut tx_outer = state - .pool - .begin() - .await - .map_err(|e| AppError::Database(DbError::Sqlx(e)))?; - let bank_account_id = body.bank_account_id; - let proposals = body.proposals.clone(); - let user_id = current_user.user_id; - // Story 17-2a (DC5 cat ii) — attribution PAT propagée dans les helpers/closures. - let actor_api_key_id = current_user.api_key_id; - let company_id = current_user.company_id; - // C2 Pass 1 — `tx_map` (snapshot pré-flight 0ter) n'est plus passé // dans le lock : `accept_one` recharge la BankTransaction inside // lock pour fermer la fenêtre TOCTOU entre le pré-flight (hors-lock) // et l'UPDATE step 8. Le pré-flight 0ter reste utile pour valider // le batch ownership avant d'acquérir le lock (fail-fast 400). drop(tx_map); + + // Step 1 — UN seul lock pour tout le batch (H5), dans une tentative que + // `retry_with` rejoue en entier sur interblocage (Story 25-4-c2, #480). + // + // ⛔ **Rejeu au plus dehors** — transaction neuve, verrou de compte repris —, + // patron de `post_cancel_reconciliation`. Deux causes, deux formes : + // - un 1213 qui remonte **directement** par un `?` (`SAVEPOINT`, `RELEASE`) + // arrive en `AppError::Database(DbError::Sqlx(_))`, que `is_deadlock_error` + // reconnaît ; + // - un 1213 levé **dans** une proposition y est absorbé en `FailedProposal` ; + // InnoDB a pourtant annulé toute la transaction, et c'est le + // `ROLLBACK TO SAVEPOINT` suivant qui échoue (1305) — `accept_batch` le + // remonte en `ReconciliationError::TransactionAborted`, rendu ici en + // `AppError::ReconciliationTransactionAborted`. + // ⚠️ Prédicat LOCAL à cette route : `is_deadlock_error` reste 1213 seul, un + // 1305 ailleurs dans le crate n'a pas ce sens. + use kesh_db::retry::{DEFAULT_MAX_DEADLOCK_ATTEMPTS, is_deadlock_error, retry_with}; + let company_id = current_user.company_id; + let user_id = current_user.user_id; + // Story 17-2a (DC5 cat ii) — attribution PAT propagée dans les helpers/closures. + let actor_api_key_id = current_user.api_key_id; + let bank_account_id = body.bank_account_id; + retry_with( + DEFAULT_MAX_DEADLOCK_ATTEMPTS, + |err: &AppError| { + matches!(err, AppError::Database(db) if is_deadlock_error(db)) + || matches!(err, AppError::ReconciliationTransactionAborted) + }, + || { + let pool = state.pool.clone(); + let proposals = body.proposals.clone(); + async move { + accept_once( + &pool, + company_id, + bank_account_id, + user_id, + actor_api_key_id, + proposals, + ) + .await + } + }, + ) + .await + .map(Json) +} + +/// Une tentative de `POST /accept` : transaction neuve, verrou du compte +/// bancaire, lot, commit (Story 25-4-c2 — extraite de `post_accept` pour que +/// `retry_with` puisse la rejouer en entier). +async fn accept_once( + pool: &sqlx::MySqlPool, + company_id: i64, + bank_account_id: i64, + user_id: i64, + actor_api_key_id: Option, + proposals: Vec, +) -> Result { + let mut tx_outer = pool + .begin() + .await + .map_err(|e| AppError::Database(DbError::Sqlx(e)))?; + let lock_result = with_account_lock( &mut tx_outer, company_id, @@ -851,7 +902,14 @@ pub async fn post_accept( .commit() .await .map_err(|e| AppError::Database(DbError::Sqlx(e)))?; - Ok(Json(response)) + Ok(response) + } + // Story 25-4-c2 — la transaction du lot a été annulée sous lui par + // InnoDB (interblocage) : rejouable, cf. `post_accept`. + Err(ReconciliationError::TransactionAborted { source }) => { + drop(tx_outer); + tracing::warn!(error = %source, "reconciliation accept: transaction aborted underneath (deadlock), retrying"); + Err(AppError::ReconciliationTransactionAborted) } Err(ReconciliationError::AccountLocked { bank_account_id, @@ -970,15 +1028,33 @@ async fn accept_batch( .await { Ok(entry) => { - sqlx::query(&format!("RELEASE SAVEPOINT {savepoint}")) + // Même lecture que la branche d'échec : un point de sauvegarde + // disparu signale une transaction annulée sous le lot. + if let Err(e) = sqlx::query(&format!("RELEASE SAVEPOINT {savepoint}")) .execute(&mut **tx_outer) - .await?; + .await + { + if is_savepoint_lost(&e) { + return Err(ReconciliationError::TransactionAborted { source: e }); + } + return Err(ReconciliationError::Database(e)); + } accepted.push(entry); } Err(failure) => { - sqlx::query(&format!("ROLLBACK TO SAVEPOINT {savepoint}")) + // Story 25-4-c2 (#480) — un 1213 absorbé dans la proposition a + // annulé TOUTE la transaction : le point de sauvegarde n'existe + // plus (1305). Le signaler comme tel, pour que la route rejoue + // le lot au lieu de rendre un 500 opaque. + if let Err(e) = sqlx::query(&format!("ROLLBACK TO SAVEPOINT {savepoint}")) .execute(&mut **tx_outer) - .await?; + .await + { + if is_savepoint_lost(&e) { + return Err(ReconciliationError::TransactionAborted { source: e }); + } + return Err(ReconciliationError::Database(e)); + } failed.push(failure); } } @@ -987,6 +1063,26 @@ async fn accept_batch( Ok(AcceptResponse { accepted, failed }) } +/// `TransactionAborted` n'est posée que par `accept_batch` : ailleurs, elle +/// signale un défaut, rendu en `500` (Story 25-4-c2 — bras défensif partagé par +/// `post_reject`, `post_manual` et `post_split`). +fn transaction_aborted_outside_accept(source: &sqlx::Error) -> AppError { + tracing::error!(error = %source, "unexpected TransactionAborted outside accept_batch"); + AppError::ReconciliationTransactionAborted +} + +/// Code MariaDB `ER_SP_DOES_NOT_EXIST` — ici, un point de sauvegarde disparu +/// avec la transaction qu'InnoDB a annulée. +const MARIADB_SAVEPOINT_DOES_NOT_EXIST: u16 = 1305; + +/// Le point de sauvegarde du lot a-t-il disparu ? Lu sur le **code** MySQL, +/// jamais sur le texte du message (Story 25-4-c2). +fn is_savepoint_lost(err: &sqlx::Error) -> bool { + err.as_database_error() + .and_then(|db| db.try_downcast_ref::()) + .is_some_and(|my| my.number() == MARIADB_SAVEPOINT_DOES_NOT_EXIST) +} + /// Helper interne — dispatch sur le variant `AcceptProposalInput` Q2. /// /// Story 8-5a-bis F3''' Pass 3 Opus + H1 Pass 4 : pattern match top-level @@ -1502,6 +1598,28 @@ async fn accept_one_invoice( // ⛔ `paid_at` seulement si le solde est tombé à zéro (Story 24-2). Un // encaissement partiel bump la version et `updated_at` — la facture a bien // changé d'état — mais laisse `paid_at` à NULL. + // + // ⛔ **Ce `version = ?` est le verrou qui interdit de régler deux fois** + // (Story 25-4-c2, #480). Le reste dû (`due_before`, garde (c)) est lu sur + // l'**instantané** de la transaction, fixé dès sa première lecture : un + // écrit validé depuis est invisible. Cet `UPDATE`, lui, lit la version + // **courante** : il ne touche aucune ligne si la facture a changé depuis + // l'instantané, et la proposition est refusée sans rien écrire. Cela ne + // vaut que par l'invariant : **tout écrit qui change le reste dû incrémente + // `version`** — l'acceptation (ici), le règlement manuel + // (`invoice_settlements_write::settle_invoice`, partiel compris), l'annulation + // d'un règlement (`cancel_settlement_in_tx`), l'émission d'un avoir + // (`credit_notes::create_credit_note`), la dévalidation (`invoices::unvalidate`). + // + // ⛔ **Pas de `FOR UPDATE` sur la facture en amont** : il lirait `version` à + // jour — ce contrôle passerait toujours — alors que le reste dû resterait lu + // sur l'instantané. Il désarmerait le verrou au lieu de le renforcer. + // + // ⚠️ MariaDB 10.11, `innodb_snapshot_isolation = OFF`. À partir de la 11.6, + // la valeur par défaut passe à `ON` : un `UPDATE` sur une ligne modifiée + // depuis l'instantané échoue alors en 1020 (*Record has changed since last + // read*) au lieu de lire la version courante. Le refus reste juste, mais il + // sortirait en `DATABASE_ERROR` et non en `race_during_update`. let paid_at_to_set: Option = if fully_settled { Some(paid_at_dt) } else { @@ -2538,6 +2656,10 @@ pub async fn post_reject( } // Story 8-5a-bis — branche exhaustive uniquement (reject_batch // n'émet pas SplitImbalance). Unreachable en pratique. + Err(ReconciliationError::TransactionAborted { source }) => { + drop(tx_outer); + Err(transaction_aborted_outside_accept(&source)) + } Err(ReconciliationError::SplitImbalance { expected, actual, @@ -3065,6 +3187,10 @@ pub async fn post_manual( } // Story 8-5a-bis — branche exhaustive (post_manual ne fait pas // de split, unreachable en pratique). + Err(ReconciliationError::TransactionAborted { source }) => { + drop(tx_outer); + Err(transaction_aborted_outside_accept(&source)) + } Err(ReconciliationError::SplitImbalance { expected, actual, @@ -3503,6 +3629,10 @@ pub async fn post_split( let _ = tx_outer.rollback().await; Err(AppError::Database(DbError::Sqlx(e))) } + Err(ReconciliationError::TransactionAborted { source }) => { + drop(tx_outer); + Err(transaction_aborted_outside_accept(&source)) + } Err(ReconciliationError::SplitImbalance { expected, actual, @@ -3713,6 +3843,10 @@ async fn cancel_reconciliation_once( // c'est ce que lit le prédicat du rejeu. ReconciliationError::Db(db) => AppError::Database(db), ReconciliationError::Database(e) => AppError::Database(DbError::Sqlx(e)), + // Story 25-4-c2 — posée par `accept_batch` seulement ; défensif ici. + ReconciliationError::TransactionAborted { .. } => { + AppError::ReconciliationTransactionAborted + } other => { tracing::error!(error = %other, "variante inattendue au dé-rapprochement"); AppError::Internal("internal: unexpected reconciliation error".into()) diff --git a/crates/kesh-api/tests/reconciliation_e2e.rs b/crates/kesh-api/tests/reconciliation_e2e.rs index 3a0e05f9..685913db 100644 --- a/crates/kesh-api/tests/reconciliation_e2e.rs +++ b/crates/kesh-api/tests/reconciliation_e2e.rs @@ -4231,3 +4231,443 @@ async fn an_api_key_cancels_and_is_named_in_the_audit(pool: MySqlPool) { .unwrap(); assert_eq!((actor_type.as_str(), actor_key), ("api_key", Some(key_id))); } + +// ============================================================ +// Story 25-4-c2 (#480) — l'acceptation contre les écritures concurrentes +// ============================================================ + +/// Lance `POST /reconciliation/accept` pour une seule proposition, **en tâche** : +/// le test garde la main pour agir pendant que l'acceptation attend. Rend le +/// statut HTTP et le corps. +fn accept_in_background( + app: &TestApp, + ctx: &CompanyCtx, + tx_id: i64, + inv_id: i64, +) -> tokio::task::JoinHandle<(u16, Value)> { + let client = app.client.clone(); + let url = app.url("/api/v1/reconciliation/accept"); + let jwt = ctx.jwt.clone(); + let body = serde_json::json!({ + "bankAccountId": ctx.bank_account_id, + "proposals": [{ "type": "invoice", "bankTransactionId": tx_id, "invoiceId": inv_id }], + }); + tokio::spawn(async move { + let resp = client + .post(url) + .bearer_auth(jwt) + .json(&body) + .send() + .await + .unwrap(); + let status = resp.status().as_u16(); + (status, resp.json().await.unwrap_or(Value::Null)) + }) +} + +/// ⛔ **#480, la course n° 1 : un règlement manuel PARTIEL validé pendant +/// qu'une acceptation est en cours ne doit pas laisser régler la facture deux fois.** +/// +/// La fenêtre réelle de la course va de la **première lecture** de la +/// transaction d'acceptation (l'instantané se fige à l'étape 2) jusqu'à sa +/// recherche d'exercice (d) : dans cet intervalle elle ne pose aucun verrou de +/// ligne, et un règlement manuel peut s'y valider entièrement. Après (d), elle +/// tient les exercices de la société — la recherche d'exercice les parcourt en +/// les verrouillant —, et le règlement manuel l'attend : ce sens-là est sûr. +/// +/// Montage, sans `sleep` — une lecture simple n'attend jamais un verrou de +/// ligne, mais elle attend un verrou de **métadonnées** : +/// 1. une connexion de test tient `LOCK TABLES contacts WRITE` ; +/// 2. l'acceptation (1 000.— sur une facture de 1 000.—) part en tâche : elle +/// fige son instantané, lit la facture, puis s'arrête à la lecture du +/// contact (étape 5bis), **avant** sa garde de trop-perçu — on l'y attend ; +/// 3. un règlement manuel de 400.— est validé (il ne lit pas `contacts`) ; +/// 4. `UNLOCK TABLES` : l'acceptation reprend, sur un instantané où la +/// facture doit encore 1 000.—. +/// +/// Avant la 25-4-c2, le règlement partiel n'incrémentait pas `version` : +/// l'`UPDATE invoices … version = ?` de l'acceptation réussissait, et la +/// facture finissait réglée 1 400.— pour 1 000.—, compte clients créditeur. +#[sqlx::test(migrations = "../kesh-db/test-schema")] +async fn accept_refuses_when_a_partial_manual_settlement_lands_meanwhile(pool: MySqlPool) { + use kesh_db::entities::SettlementChoice; + use kesh_db::repositories::invoice_settlements_write; + + let ctx = setup_company(&pool, "Course1", "CH4431999123000889012", Role::Comptable).await; + let day = NaiveDate::from_ymd_opt(2026, 5, 15).unwrap(); + let inv_date = NaiveDate::from_ymd_opt(2026, 4, 20).unwrap(); + let (inv_id, _) = seed_validated_invoice( + &pool, + ctx.company_id, + ctx.contact_id, + "INV-RACE-1", + inv_date, + dec!(1000.00), + ) + .await; + let tx_ids = seed_bank_transactions( + &pool, + ctx.company_id, + ctx.bank_account_id, + ctx.user_id, + &unique_hash("race_partial_manual"), + day, + day, + vec![make_new_tx( + ctx.company_id, + ctx.bank_account_id, + day, + Some(day), + dec!(1000.00), + "CHF", + "INV-RACE-1", + Some("Course1 Client"), + )], + ) + .await; + let app = spawn_app(pool.clone()).await; + + // (1) Le verrou de métadonnées, sur une connexion hors transaction — + // DÉTACHÉE du pool : si le test panique avant `UNLOCK TABLES`, elle se + // ferme au lieu de retourner au pool verrouillée, et sa session emporte le + // verrou (sinon la suppression de la base éphémère attendrait sans fin). + let mut verrou = pool.acquire().await.unwrap().detach(); + sqlx::query("LOCK TABLES contacts WRITE") + .execute(&mut verrou) + .await + .unwrap(); + + // (2) L'acceptation, jusqu'à la lecture du contact. + let acceptation = accept_in_background(&app, &ctx, tx_ids[0], inv_id); + let bloquee = + kesh_db::test_fixtures::attendre_une_requete_en_cours(&pool, &["FROM contacts"], || { + acceptation.is_finished() + }) + .await; + assert!( + bloquee, + "l'acceptation devait attendre sur `contacts` — le montage ne prouve rien sinon" + ); + + // (3) Le règlement manuel partiel, validé pendant l'attente. + let partiel = invoice_settlements_write::settle_invoice( + &pool, + ctx.user_id, + ctx.company_id, + inv_id, + SettlementChoice::BankTransfer { + bank_account_id: ctx.bank_account_id, + }, + dec!(400.00), + day, + ) + .await + .expect("règlement manuel partiel"); + assert!(!partiel.fully_settled); + + // (4) Relâcher : l'acceptation reprend sur un reste périmé. + sqlx::query("UNLOCK TABLES") + .execute(&mut verrou) + .await + .unwrap(); + drop(verrou); + let (status, body) = acceptation.await.unwrap(); + assert_eq!(status, 200, "succès partiel = succès HTTP ; corps = {body}"); + assert!( + body["accepted"].as_array().unwrap().is_empty(), + "⛔ l'acceptation a réglé une facture déjà réglée de 400.— entre-temps : {body}" + ); + let failed = body["failed"].as_array().unwrap(); + assert_eq!(failed.len(), 1); + assert_eq!( + failed[0]["errorCode"], + "RECONCILIATION_INVOICE_NOT_ELIGIBLE" + ); + assert_eq!(failed[0]["details"]["reason"], "race_during_update"); + + // Rien d'écrit par l'acceptation : un seul règlement, le manuel. + let settled: Vec = + sqlx::query_scalar("SELECT amount FROM invoice_settlements WHERE invoice_id = ?") + .bind(inv_id) + .fetch_all(&pool) + .await + .unwrap(); + assert_eq!(settled, vec![dec!(400.0000)]); + let tx_status: String = sqlx::query_scalar("SELECT status FROM bank_transactions WHERE id = ?") + .bind(tx_ids[0]) + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!(tx_status, "pending", "la transaction reste à rapprocher"); + assert_eq!( + receivable_balance(&pool, ctx.receivable_account_id).await, + dec!(600.00), + "le compte clients porte le reste, jamais un solde créditeur" + ); +} + +/// ⛔ **#480 — un interblocage dont l'acceptation est la victime se REJOUE.** +/// +/// Avant la 25-4-c2, InnoDB annulait toute la transaction du lot, l'instruction +/// fautive ressortait en `FailedProposal` `DATABASE_ERROR`, le `ROLLBACK TO +/// SAVEPOINT` suivant échouait (1305) et la route rendait un **500** — sans +/// rejeu, `retry_with` n'existant que sur l'annulation. +/// +/// Montage d'un interblocage **déterministe**, dont l'acceptation est la +/// victime : +/// 1. une transaction de test s'alourdit (500 lignes insérées : InnoDB choisit +/// pour victime la transaction la plus légère), puis verrouille la facture ; +/// 2. l'acceptation prend l'exercice (d), passe son écriture, puis attend la +/// facture dès l'insertion du règlement — la clé étrangère vers `invoices` +/// y pose un verrou partagé sur la ligne — : on l'y attend ; +/// 3. le test demande l'exercice : cycle, InnoDB annule l'acceptation ; +/// 4. le test annule sa transaction ; le rejeu repart à neuf et accepte. +#[sqlx::test(migrations = "../kesh-db/test-schema")] +async fn accept_replays_the_batch_when_it_is_the_deadlock_victim(pool: MySqlPool) { + let ctx = setup_company(&pool, "Victime", "CH4431999123000889012", Role::Comptable).await; + let day = NaiveDate::from_ymd_opt(2026, 5, 15).unwrap(); + let inv_date = NaiveDate::from_ymd_opt(2026, 4, 20).unwrap(); + let (inv_id, _) = seed_validated_invoice( + &pool, + ctx.company_id, + ctx.contact_id, + "INV-DEADLOCK-1", + inv_date, + dec!(250.00), + ) + .await; + let tx_ids = seed_bank_transactions( + &pool, + ctx.company_id, + ctx.bank_account_id, + ctx.user_id, + &unique_hash("deadlock_victim"), + day, + day, + vec![make_new_tx( + ctx.company_id, + ctx.bank_account_id, + day, + Some(day), + dec!(250.00), + "CHF", + "INV-DEADLOCK-1", + Some("Victime Client"), + )], + ) + .await; + let fy_2026: i64 = sqlx::query_scalar( + "SELECT id FROM fiscal_years WHERE company_id = ? AND start_date <= ? AND end_date >= ? \ + AND status = 'Open' LIMIT 1", + ) + .bind(ctx.company_id) + .bind(day) + .bind(day) + .fetch_one(&pool) + .await + .unwrap(); + // Une table de lest, hors du flux d'acceptation (DDL hors transaction). + sqlx::query( + "CREATE TABLE lest_interblocage (id INT AUTO_INCREMENT PRIMARY KEY, x INT) ENGINE=InnoDB", + ) + .execute(&pool) + .await + .unwrap(); + let app = spawn_app(pool.clone()).await; + + // (1) La transaction de test : lourde, puis la facture. + let mut lourde = pool.begin().await.unwrap(); + for _ in 0..10 { + sqlx::query("INSERT INTO lest_interblocage (x) SELECT seq FROM seq_1_to_50") + .execute(&mut *lourde) + .await + .unwrap(); + } + sqlx::query("SELECT id FROM invoices WHERE id = ? FOR UPDATE") + .bind(inv_id) + .fetch_one(&mut *lourde) + .await + .unwrap(); + + // (2) L'acceptation, jusqu'à l'insertion de son règlement. + let acceptation = accept_in_background(&app, &ctx, tx_ids[0], inv_id); + let bloquee = kesh_db::test_fixtures::attendre_une_requete_en_cours( + &pool, + &["INSERT INTO invoice_settlements"], + || acceptation.is_finished(), + ) + .await; + assert!( + bloquee, + "l'acceptation devait attendre la facture — le montage ne prouve rien sinon" + ); + + // (3) Le cycle : l'exercice, que tient l'acceptation. La transaction de + // test DOIT l'obtenir — sinon c'est elle la victime, et le test ne dit rien. + sqlx::query("SELECT id FROM fiscal_years WHERE id = ? FOR UPDATE") + .bind(fy_2026) + .fetch_one(&mut *lourde) + .await + .expect("la transaction de test doit survivre à l'interblocage"); + + // (4) Relâcher : le rejeu repart à neuf. + lourde.rollback().await.unwrap(); + let (status, body) = acceptation.await.unwrap(); + assert_eq!( + status, 200, + "⛔ un interblocage doit être rejoué, pas rendu en erreur : {body}" + ); + assert_eq!( + body["accepted"].as_array().unwrap().len(), + 1, + "le rejeu accepte la proposition ; corps = {body}" + ); + let settled: Vec = + sqlx::query_scalar("SELECT amount FROM invoice_settlements WHERE invoice_id = ?") + .bind(inv_id) + .fetch_all(&pool) + .await + .unwrap(); + assert_eq!( + settled, + vec![dec!(250.0000)], + "un seul règlement, celui du rejeu" + ); +} + +/// #480, la course n° 2 — **deux acceptations du même solde, depuis deux +/// comptes bancaires** (donc deux `GET_LOCK` distincts) : une seule règle la +/// facture. Déjà refusée avant la 25-4-c2, puisque l'acceptation incrémente +/// toujours `version` ; ce test la garde, parce que c'est ce refus qu'un +/// `FOR UPDATE` mal placé désarmerait. +/// +/// Montage : une transaction de test tient la facture ; l'acceptation A prend +/// l'exercice et bute sur la facture (insertion du règlement) ; l'acceptation B, +/// son instantané déjà figé, bute sur l'exercice que tient A. Le test relâche : +/// A règle, B lit un reste périmé et doit être refusée. +#[sqlx::test(migrations = "../kesh-db/test-schema")] +async fn two_acceptances_of_the_same_balance_settle_it_once(pool: MySqlPool) { + let ctx = setup_company(&pool, "Course2", "CH4431999123000889012", Role::Comptable).await; + let day = NaiveDate::from_ymd_opt(2026, 5, 15).unwrap(); + let inv_date = NaiveDate::from_ymd_opt(2026, 4, 20).unwrap(); + let (inv_id, _) = seed_validated_invoice( + &pool, + ctx.company_id, + ctx.contact_id, + "INV-RACE-2", + inv_date, + dec!(700.00), + ) + .await; + // Le second compte bancaire, câblé sur le même compte du grand livre. + let second_account = create_bank_account(&pool, ctx.company_id, "CH9300762011623852957").await; + sqlx::query("UPDATE bank_accounts SET journal_account_id = ? WHERE id = ?") + .bind(ctx.bank_ledger_account_id) + .bind(second_account) + .execute(&pool) + .await + .unwrap(); + let tx_a = seed_bank_transactions( + &pool, + ctx.company_id, + ctx.bank_account_id, + ctx.user_id, + &unique_hash("race2_a"), + day, + day, + vec![make_new_tx( + ctx.company_id, + ctx.bank_account_id, + day, + Some(day), + dec!(700.00), + "CHF", + "INV-RACE-2", + Some("Course2 Client"), + )], + ) + .await[0]; + let tx_b = seed_bank_transactions( + &pool, + ctx.company_id, + second_account, + ctx.user_id, + &unique_hash("race2_b"), + day, + day, + vec![make_new_tx( + ctx.company_id, + second_account, + day, + Some(day), + dec!(700.00), + "CHF", + "INV-RACE-2", + Some("Course2 Client"), + )], + ) + .await[0]; + let app = spawn_app(pool.clone()).await; + let ctx_b = CompanyCtx { + bank_account_id: second_account, + jwt: ctx.jwt.clone(), + ..ctx + }; + + let mut verrou = pool.begin().await.unwrap(); + sqlx::query("SELECT id FROM invoices WHERE id = ? FOR UPDATE") + .bind(inv_id) + .fetch_one(&mut *verrou) + .await + .unwrap(); + + let acceptation_a = accept_in_background(&app, &ctx, tx_a, inv_id); + assert!( + kesh_db::test_fixtures::attendre_une_requete_en_cours( + &pool, + &["INSERT INTO invoice_settlements"], + || acceptation_a.is_finished(), + ) + .await, + "A devait attendre la facture" + ); + let acceptation_b = accept_in_background(&app, &ctx_b, tx_b, inv_id); + assert!( + kesh_db::test_fixtures::attendre_une_requete_en_cours( + &pool, + &["FROM fiscal_years", "FOR UPDATE"], + || acceptation_b.is_finished(), + ) + .await, + "B devait attendre l'exercice que tient A" + ); + + verrou.rollback().await.unwrap(); + let (status_a, body_a) = acceptation_a.await.unwrap(); + let (status_b, body_b) = acceptation_b.await.unwrap(); + assert_eq!( + (status_a, status_b), + (200, 200), + "A = {body_a} ; B = {body_b}" + ); + assert_eq!( + body_a["accepted"].as_array().unwrap().len(), + 1, + "A règle : {body_a}" + ); + assert!( + body_b["accepted"].as_array().unwrap().is_empty(), + "⛔ B a réglé une seconde fois une facture déjà soldée : {body_b}" + ); + assert_eq!( + body_b["failed"][0]["details"]["reason"], + "race_during_update" + ); + let settled: Vec = + sqlx::query_scalar("SELECT amount FROM invoice_settlements WHERE invoice_id = ?") + .bind(inv_id) + .fetch_all(&pool) + .await + .unwrap(); + assert_eq!(settled, vec![dec!(700.0000)], "un seul règlement"); +} diff --git a/crates/kesh-db/src/repositories/invoice_settlements_write.rs b/crates/kesh-db/src/repositories/invoice_settlements_write.rs index 02d4185d..46fd6645 100644 --- a/crates/kesh-db/src/repositories/invoice_settlements_write.rs +++ b/crates/kesh-db/src/repositories/invoice_settlements_write.rs @@ -229,19 +229,40 @@ pub async fn settle_invoice( .await?; // (8) ⛔ `paid_at` est la PROJECTION du résiduel à zéro, pas un drapeau. + // + // ⛔ **`version` bouge À CHAQUE règlement, partiel compris** (Story 25-4-c2, + // #480). C'est l'invariant dont dépend le verrou optimiste de l'acceptation + // d'un rapprochement (`accept_one_invoice`, `UPDATE invoices … AND version = ?`) : + // **tout écrit qui change le reste dû d'une facture incrémente `version` dans + // la même transaction.** L'acceptation lit le reste dû sur son instantané ; + // un règlement partiel validé depuis, qui n'incrémentait pas `version`, + // passait sous son contrôle — la facture finissait réglée deux fois. Les + // autres écrivains le tiennent déjà : l'acceptation elle-même, l'annulation + // d'un règlement (`cancel_settlement_in_tx`, les deux branches), l'émission + // d'un avoir (`credit_notes::create_credit_note`) et la dévalidation + // (`invoices::unvalidate`). Un nouvel écrivain qui y manquerait rouvrirait + // la course. let due_after = invoice_settlements::amount_due(&mut *tx, invoice_id).await?; let fully_settled = due_after <= Decimal::ZERO; - if fully_settled { - sqlx::query( - "UPDATE invoices SET paid_at = ?, version = version + 1, updated_at = NOW(3) \ - WHERE id = ? AND company_id = ? AND status = 'validated'", - ) - .bind(settled_on.and_hms_opt(0, 0, 0).expect("minuit est valide")) - .bind(invoice_id) - .bind(company_id) - .execute(&mut *tx) - .await - .map_err(map_db_error)?; + let paid_at = + fully_settled.then(|| settled_on.and_hms_opt(0, 0, 0).expect("minuit est valide")); + let marked = sqlx::query( + "UPDATE invoices SET paid_at = ?, version = version + 1, updated_at = NOW(3) \ + WHERE id = ? AND company_id = ? AND status = 'validated'", + ) + .bind(paid_at) + .bind(invoice_id) + .bind(company_id) + .execute(&mut *tx) + .await + .map_err(map_db_error)?; + // Le verrou de l'étape (1) rend ce cas impossible aujourd'hui ; s'il se + // produisait, le règlement serait écrit SANS incrémenter `version` et + // l'invariant ci-dessus serait rompu en silence : on refuse tout. + if marked.rows_affected() != 1 { + return Err(DbError::Invariant( + "règlement : la facture n'a pas pu être marquée modifiée (version)".into(), + )); } // (9) ⛔ L'audit, dans la MÊME transaction. Un règlement est un fait @@ -469,12 +490,20 @@ pub async fn cancel_settlement_in_tx( "UPDATE invoices SET version = version + 1, updated_at = NOW(3) \ WHERE id = ? AND company_id = ?" }; - sqlx::query(sql) + let marked = sqlx::query(sql) .bind(invoice_id) .bind(company_id) .execute(&mut **tx) .await .map_err(map_db_error)?; + // Même garde que `settle_invoice` (Story 25-4-c2) : une annulation qui ne + // marquerait pas la facture romprait l'invariant de version. + if marked.rows_affected() != 1 { + return Err(DbError::Invariant( + "annulation de règlement : la facture n'a pas pu être marquée modifiée (version)" + .into(), + )); + } // (7) L'audit du GESTE. ⚠️ La contre-passation a déjà écrit // `journal_entry.reversed` sur l'écriture : deux lignes, c'est voulu — diff --git a/crates/kesh-reconciliation/src/errors.rs b/crates/kesh-reconciliation/src/errors.rs index 2a077a69..c0c1769f 100644 --- a/crates/kesh-reconciliation/src/errors.rs +++ b/crates/kesh-reconciliation/src/errors.rs @@ -33,6 +33,20 @@ pub enum ReconciliationError { #[error("database error: {0}")] Database(#[from] sqlx::Error), + /// Story 25-4-c2 (#480) — la transaction d'un lot a été **annulée sous lui** + /// par InnoDB : un interblocage (1213) levé dans une proposition y a été + /// absorbé en `FailedProposal`, mais il a annulé toute la transaction, et le + /// `ROLLBACK TO SAVEPOINT` qui suit échoue faute de point de sauvegarde + /// (1305). Le lot entier est rejouable ; la route d'acceptation le rejoue. + /// + /// ⚠️ Posée par `accept_batch` **seulement** : c'est le seul site du dépôt à + /// utiliser des `SAVEPOINT` nommés. Ailleurs, un 1305 n'a pas ce sens. + #[error("transaction aborted underneath the batch (deadlock victim)")] + TransactionAborted { + #[source] + source: sqlx::Error, + }, + /// Story 8-5a-base — l'exercice fiscal couvrant `entry_date` est /// soit inexistant soit `Closed` (helper /// `fiscal_years::find_open_covering_date` retourne `None` pour diff --git a/docs/api-external.md b/docs/api-external.md index 22a95812..74b569c3 100644 --- a/docs/api-external.md +++ b/docs/api-external.md @@ -284,6 +284,12 @@ Refus, dans l'ordre de précédence : `SUPPLIER_INVOICE_CANCELLED` (`409`, déj ⚠️ **`FISCAL_YEAR_CLOSED` rend ici `409`**, alors que la dévalidation le rend en `400` : c'est un refus du **geste** d'annulation, qui porte sur l'exercice du **règlement** ; la contre-passation, elle, serait datée d'un exercice ouvert. +### Accepter des propositions de rapprochement + +**`POST /api/v1/reconciliation/accept`** — écriture (`read-write`), rôle Comptable, ouverte aux clés comme les autres routes de réconciliation ; la clé est nommée au journal d'audit (`reconciliation.accepted`). Corps : `{ bankAccountId, proposals: [...] }`, chaque proposition portant `type` (`invoice`, `split` ou `rule`) et `bankTransactionId` — plus `invoiceId` pour une facture. Les propositions viennent de `GET /api/v1/reconciliation/proposals`, où `invoiceAmount` est le **reste dû** de la facture (le TTC tant que rien n'est réglé) et `invoiceTotalTtc` son total d'origine, présent seulement quand les deux diffèrent. + +La réponse est un **succès partiel** en `200` : `{ accepted: [...], failed: [{ bankTransactionId, errorCode, details }] }` — une proposition refusée n'empêche pas les autres. Pour une facture, le score est recalculé par le serveur, et un montant supérieur au reste dû est refusé (`RECONCILIATION_OVERPAYMENT`, jamais écrit). Si la facture change **pendant** l'acceptation — un règlement, un avoir, une dévalidation enregistrés au même moment —, la proposition est refusée en `RECONCILIATION_INVOICE_NOT_ELIGIBLE` (`details.reason` = `race_during_update`) sans rien écrire : relire les propositions et recommencer. Un interblocage transitoire avec une autre opération est **rejoué** par le serveur, sans être montré ; s'il persiste après plusieurs tentatives, la requête finit en `500 INTERNAL_ERROR` et peut être renvoyée telle quelle — rien n'a été écrit. Refus de la requête entière : `400` (corps invalide), `404` (compte bancaire inconnu), `409 RECONCILIATION_ACCOUNT_LOCKED` (une autre acceptation tient le compte). + ### Annuler un rapprochement bancaire **`GET /api/v1/reconciliation/transactions/{id}`** — lecture, rôle Comptable (comme les propositions). La transaction bancaire (mêmes champs que dans le détail d'un import, dont `matchedEntryId`), `kind` (`invoice_settlement` : le rapprochement a réglé une facture client ; `entry` : une écriture que seule la transaction possède — éclatement, règle, rapprochement manuel ; `null` : la transaction n'est pas rapprochée), `invoiceId`, `invoiceNumber`, et **`cancellable`** — calculé par la fonction même qui refuserait l'annulation, lu **dans un seul instantané**. Quand il vaut `false`, `cancelBlockedBy` porte le code du motif, `cancelBlockedLabel` le numéro du compte archivé et `cancelBlockedDocumentId` l'**autre** transaction qui pointe la même écriture. ⚠️ Calculé pour **une** transaction : le détail d'un import ne le porte pas.