bug: manual confirm can double-establish one document against two bank transactions (no document-side supersession) #53

New issue

Closed

opened 2026-07-04 12:11:48 +00:00 by momsse · 0 comments

momsse commented

2026-07-04 12:11:48 +00:00

Owner

Migré depuis viziertronic/octant#117 — ouvert le 2026-06-27 par @momsse.

Severity: correctness (CONFIRMED by local review).

SupersedeReconciliationSaga keys supersession only on bankTransactionId — its sole lookup is findEstablishedByBankTransactionIdExcluding(bankTransactionId, …) (supersede-reconciliation.saga.ts:74), reacting to ReconciliationEstablishedEvent to supersede a prior reconciliation on the same transaction. There is no document-side supersession.

ReconcileHourlyUseCase won't auto-establish a reconciliation whose document is already established against another transaction, but manual confirm (ConfirmReconciliationUseCase, added in #41/PR #114) bypasses that dedup: it just calls decideConfirmReconciliation.

Scenario: a reviewer manually confirms a still-pending reconciliation whose document is already established against a different bank transaction. The supersede saga finds no established reconciliation for the new transaction id, so both reconciliations stay established → the same document/invoice is reconciled against two bank transactions, double-counting it.

Fix direction: add document-side supersession (a findEstablishedByDocumentIdExcluding lookup in the supersede saga) and/or a write-side uniqueness guard on confirm (an inline projection + UNIQUE constraint on the established document, mirroring how the write-side dedups transactions).

Surfaced by the local review during #41.

> _Migré depuis [viziertronic/octant#117](https://github.com/viziertronic/octant/issues/117) — ouvert le 2026-06-27 par @momsse._ **Severity: correctness (CONFIRMED by local review).** `SupersedeReconciliationSaga` keys supersession **only** on `bankTransactionId` — its sole lookup is `findEstablishedByBankTransactionIdExcluding(bankTransactionId, …)` (supersede-reconciliation.saga.ts:74), reacting to `ReconciliationEstablishedEvent` to supersede a prior reconciliation on the **same** transaction. There is no document-side supersession. `ReconcileHourlyUseCase` won't auto-establish a reconciliation whose document is already established against another transaction, but **manual confirm** (`ConfirmReconciliationUseCase`, added in #41/PR #114) bypasses that dedup: it just calls `decideConfirmReconciliation`. **Scenario:** a reviewer manually confirms a still-pending reconciliation whose document is already established against a *different* bank transaction. The supersede saga finds no established reconciliation for the *new* transaction id, so both reconciliations stay `established` → the same document/invoice is reconciled against two bank transactions, double-counting it. **Fix direction:** add document-side supersession (a `findEstablishedByDocumentIdExcluding` lookup in the supersede saga) and/or a write-side uniqueness guard on confirm (an inline projection + UNIQUE constraint on the established document, mirroring how the write-side dedups transactions). Surfaced by the local review during #41.

momsse closed this issue

2026-07-04 12:11:49 +00:00

momsse referenced this issue

2026-07-04 12:18:58 +00:00

harden: supersede ALL prior established reconciliations per dimension + UNIQUE guard #58