bug: bank-account sync re-emits a detected event for every transaction every run (no dedup/cursor → unbounded stream) #54

New issue

Closed

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

momsse commented

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

Owner

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

Severity: correctness/scalability (CONFIRMED by local review).

decideSynchroniseBankAccount (bank-account.aggregate.ts:178) maps every command.detectedTransactions to a BankTransactionDetectedEvent with no dedup against already-known external ids — the aggregate state holds none — and the recorded lastTransactionCheckAt cursor is never fed back into bankApi.fetchTransactions (its input is only { bankAccountId, bankId }, synchronise-bank-account.use-case.ts:56).

Scenario: on each scheduled sync the bank API returns its transaction list and the aggregate re-appends a detected event for all of them, so the bank-account event stream grows without bound. Every subsequent sync reconstitutes an ever-larger stream and re-drives the async TransactionMatchingSaga to re-scan documents per event; sync latency climbs until the daemon times out / exhausts memory, silently stopping ingestion.

Fix direction: track known external ids (or a high-water cursor) in BankAccountState and only emit detected events for genuinely new transactions; pass lastTransactionCheckAt into fetchTransactions so the adapter can fetch incrementally. Relates to #50 (sync activation) and #44 (matcher scan cost).

Surfaced by the local review during #41.

> _Migré depuis [viziertronic/octant#118](https://github.com/viziertronic/octant/issues/118) — ouvert le 2026-06-27 par @momsse._ **Severity: correctness/scalability (CONFIRMED by local review).** `decideSynchroniseBankAccount` (bank-account.aggregate.ts:178) maps **every** `command.detectedTransactions` to a `BankTransactionDetectedEvent` with no dedup against already-known external ids — the aggregate state holds none — and the recorded `lastTransactionCheckAt` cursor is never fed back into `bankApi.fetchTransactions` (its input is only `{ bankAccountId, bankId }`, synchronise-bank-account.use-case.ts:56). **Scenario:** on each scheduled sync the bank API returns its transaction list and the aggregate re-appends a detected event for all of them, so the bank-account event stream grows without bound. Every subsequent sync reconstitutes an ever-larger stream and re-drives the async `TransactionMatchingSaga` to re-scan documents per event; sync latency climbs until the daemon times out / exhausts memory, silently stopping ingestion. **Fix direction:** track known external ids (or a high-water cursor) in `BankAccountState` and only emit detected events for genuinely new transactions; pass `lastTransactionCheckAt` into `fetchTransactions` so the adapter can fetch incrementally. Relates to #50 (sync activation) and #44 (matcher scan cost). Surfaced by the local review during #41.

momsse closed this issue

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

momsse referenced this issue

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

harden: bound BankAccount dedup state + snapshot-safe codec #59