From 4e1eb3d6626440436bca83d379c7af10bb366262 Mon Sep 17 00:00:00 2001 From: Jakob Wennberg Date: Sat, 29 Aug 2026 00:26:47 +0200 Subject: [PATCH] fix(cash-accounts): never propose or accept an orphaned twin ledger as counter-account; match and re-point across sibling ledgers (#1643) (#2010) * fix(cash-accounts): never propose or accept an orphaned cash-account ledger as counter-account (#1643) A broken bank reconnect leaves cash_accounts rows that share the live account's IBAN (held by a revoked connection, or demoted to manual by the #916 fix). Three consequences are fixed here: - Problem 4 (silent mis-booking): the own-account transfer detector paired with such an orphan and proposed its ledger as the counter-account, and a counterparty template learned from that result replayed as 1940/1931 in the booking dialog. The detector now tolerates several rows on one IBAN, never pairs with the transaction's own row, a disabled row, or a revoked holder; the mapping engine drops a "transfer" whose counter equals the settlement account; suggest-categories withholds learned suggestions that reference an orphaned ledger; and both commit paths (POST /api/transactions/[id]/categorize, categorizeMatchedTransaction) reject with the new TX_CATEGORIZE_ORPHANED_COUNTER_ACCOUNT (400). Orphans are only refused in the COUNTER position: a stranded row still settles on its own ledger, and a manual account without a live IBAN twin is never treated as orphaned, so transfers between two live accounts keep booking. - Problem 1 (match dialog): the ranked unmatched-entries path also offers vouchers booked on sibling ledgers of the same IBAN, and manualLink accepts a voucher line on a sibling ledger. When it does, the same locked UPDATE re-points transactions.cash_account_id to the live sibling row (currency-gated, like PATCH /api/transactions/[id]/cash-account) so the account-keyed reconciliation does not count a cross-account link as an imbalance on both ledgers. - Problem 3 (naming): allocatePsd2LedgerAccount names the chart account BAS-style (BAS reference name for a standard slot, else "Bankkonto ") instead of the ASPSP-reported holder name. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015nAd8XJ2RPCmG2eKoLBdna * fix(cash-accounts): address review findings on the orphaned-ledger guards (#1643) One in-memory topology (cash_accounts rows + bank_connections status) now defines "live", "orphaned" and "same physical account" for the transfer detector, the match/link flows and every commit guard, so a proposal is never made that a guard later rejects. - Finding 1/4/6 (own IBAN as counterparty): findPairableCashAccountByIban treats the transaction's own IBAN as "not a transfer": every same-currency row on that IBAN is the same physical account, whichever is live, so interest stamped with the own IBAN never pairs with a twin (two active rows, a demoted-manual twin, or a live twin of a stranded row). Only a pocket in another currency on that IBAN can still pair. guardCounterLegs refuses a same-IBAN same-currency twin in the counter position on every commit path, even when both rows are active. - Finding 3: with several surviving candidates (currency pockets with no discriminator, or two active twins) the finder returns null instead of picking the lowest ledger, which is what the pre-PR lookup did. - Finding 5: the finder drops every row in the orphaned set, the same predicate the commit guards use (demoted-manual twins included). - Finding 9: "live" means enabled + connection status 'active'; an expired/error twin of a live row is orphaned, a lone expired connection (re-auth window) is not. - Finding 2: siblings are keyed on (normalized IBAN, currency) in describeCashAccountSiblings and the unmatched-entries route, so a SEK transaction can no longer link to a voucher whose only bank leg is on the EUR pocket of the same IBAN; manualLink rejects that as before. - Finding 8: manualLink re-points a row only when the voucher sits on the LIVE sibling and the own row is not live; the reverse direction links without moving the row. - Finding 7: the v1 REST categorize route runs the same guardCounterLegs check after account_override and returns TX_CATEGORIZE_ORPHANED_COUNTER_ACCOUNT. MCP stages through categorizeMatchedTransaction, already covered. - Finding 10: a learned template whose stale 19xx leg is a twin of the settlement row is rewritten to the settlement account (it is the bank leg, not the counter) instead of refused; suggest-categories exempts each transaction's own settlement ledger before withholding a suggestion. The error message now covers both the twin and the disconnected case. Tests pin each behavior (service, detector, manualLink, unmatched-entries, dashboard and v1 categorize routes, suggest-categories). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015nAd8XJ2RPCmG2eKoLBdna * fix(cash-accounts): address round-2 review findings (#1643) 1+3. Orphan derivation keyed on (IBAN, currency): loadCashAccountTopology now keys the live twin on normalized IBAN plus currency (the rule every other "same physical account" check in the PR already used), so a manual or deselected GBP/EUR pocket beside a live SEK pocket of a multi-currency account is never orphaned, still pairs in the transfer detector and is accepted as counter at commit. Twin computation is shared (twinLedgersOf). 2. suggest-categories mirrors guardCounterLegs: a learned 19xx leg that is a twin of the transaction's own row is rewritten to the settlement ledger in the offered suggestion instead of being withheld; only a true counter-position orphan (or a twin that would book the settlement ledger against itself) is withheld. One topology load per batch (loadCounterLegTopology). 4. The free-form dialog path (POST /api/transactions/[id]/book) gets a line-level guard (guardBookedCounterLines): a 19xx line that is a twin of the transaction's own row or an orphaned ledger, alongside the settlement leg, is refused with TX_CATEGORIZE_ORPHANED_COUNTER_ACCOUNT. Only runs when the lines touch two distinct 19xx ledgers. The twin rewrite in suggest-categories (2) covers the both-active shape before the dialog is even opened. 5. manualLink re-points the row onto the sibling ledger the voucher was booked on whenever the sibling is live or the own row is not (both-live twins and both-dead rows included); only a live row whose voucher sits on a dead sibling links without moving. unmatched-entries now uses describeCashAccountSiblings and does not offer dead-sibling vouchers to a live row. DECISIONS.md: the PR's existing review follow-up line amended. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015nAd8XJ2RPCmG2eKoLBdna * fix(cash-accounts): address round-3 review findings (#1643) 1. Revoked-held rows are no longer orphaned unconditionally. A row whose connection is revoked is orphaned only under the twin rule (not live AND a live row shares its normalized IBAN + currency), so a disconnected-but-real account (the company's only 1930, or two real accounts on one revoked connection) stays pairable by the transfer detector and bookable as counter on every guarded path. Tests cover the no-twin case for getOrphanedCounterLedgers, findPairableCashAccountByIban, detectOwnAccountTransfer, guardCounterLegs and guardBookedCounterLines; the existing revoked tests now use a twin shape. 2. manualLink / unmatched-entries decide the re-point on the destination: a new shouldRepointToSibling moves onto a live sibling, or onto a dead one only when the own row's holder is gone (released: bank_connection_id null or revoked) and no sibling is live. An expired/error/pending own row links without moving. SiblingCashAccount gains `released`. Tests: expired own row + demoted twin links without moving and the twin's vouchers are not offered. 3. loadCounterLegTopology is exercised directly: settlement ledger and twins, other-currency pocket, null/unknown ids, cache, orphan set equal to guardCounterLegs' refusals on the same fixture, lookup failure. 4. guardBookedCounterLines docstring and the /book route comment now state that only the two-cash-legs shape is inspected; a single hand-typed 19xx line is not (covering it would cost a cash_accounts lookup on every ordinary booking). DECISIONS.md lines amended accordingly. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015nAd8XJ2RPCmG2eKoLBdna * fix(cash-accounts): address round-4 review findings (#1643) 1. Same-connection re-registration twins (the dominant prod shape): two enabled rows on one active connection sharing (IBAN, currency) are now told apart by balance_updated_at; only the most recently synced row is live, the other is a stale twin (orphaned as a counter, never a re-point destination, and the transfer detector pairs with the syncing row alone). Rows with no stamp or the same stamp both stay live. 2. POST /book: a single 19xx line that is a sibling ledger the row should move to (the live twin of a stranded row) re-points cash_account_id in the same locked UPDATE that links the voucher, mirroring manualLink. guardBookedCounterLines returns { refusedLedger, repointCashAccountId }; an ordinary booking pays one PK read of the own row. 3. manualLink refuses the link (success:false, Swedish error) when the voucher sits only on a dead sibling instead of writing a cross-account link with a server-side warn; the REST and MCP link callers reach it without the unmatched-entries filter. 4. manualLink judges a voucher touching several sibling ledgers on the best of them (a live sibling, else the first the row may move to) instead of the first line PostgREST returns. Tests pinned in lib/cash-accounts, lib/reconciliation and the /book route; the two DECISIONS.md lines for #1643 amended in place. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015nAd8XJ2RPCmG2eKoLBdna * fix(cash-accounts): address round-5 review findings (#1643) 1/2/5. Same-connection twin liveness no longer ranks on cash_accounts.balance_updated_at (a connect-time snapshot the sync never refreshes, inverted on prod in 4 of 5 stamped groups). The live row is the one whose external_uid the bank still lists in bank_connections.accounts_data (rewritten on every sync); no listing, both listed or neither listed keeps both rows live (round-3 behavior). getConnectionStatuses selects accounts_data in the same query. 3. guardBookedCounterLines single-19xx-line shape: a twin the row may not move to (dead or disabled) is refused with TX_CATEGORIZE_ORPHANED_COUNTER_ACCOUNT instead of posting the only bank leg on the dead ledger; an unrelated 19xx line still posts as typed. Route test added. 4. Disabled cash_accounts rows are never siblings, so neither manualLink nor /book re-points a transaction onto a deselected row; a voucher booked only there is refused as a cross-account link. 6. PR body rewritten to the final rules; DECISIONS.md round-4 line amended (signal correction, /book refusal, disabled siblings). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015nAd8XJ2RPCmG2eKoLBdna * fix(cash-accounts): never treat a null external_uid as listed by the bank (#1643) CashAccount.external_uid is nullable in the shared type; the same-connection twin rule now skips null uids instead of passing them to Set.has, which failed the strict type check in CI. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015nAd8XJ2RPCmG2eKoLBdna * fix(cash-accounts): drop the same-connection twin liveness rule; both rows stay live (#1643) Two enabled rows on one active bank connection sharing (IBAN, currency) are no longer ranked. Round 4 ranked on cash_accounts.balance_updated_at and round 5 on external_uid presence in bank_connections.accounts_data; each was verified against prod and each was contradicted by it (ingest routes by the accounts_data entry's ledger_account, which in two groups points at the OLD row, so the "stale" row is the one still being fed). Restores the round-3 behavior: neither twin is orphaned, the transfer finder returns null when both survive, no guard refuses either, and shouldRepointToSibling treats both as live siblings. No replacement signal; how to model the shape is a founder decision (PR #2010 review). getConnectionStatuses no longer selects accounts_data. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015nAd8XJ2RPCmG2eKoLBdna --------- Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5 --- DECISIONS.md | 2 + .../unmatched-entries/__tests__/route.test.ts | 120 +++ .../bank/unmatched-entries/route.ts | 51 +- .../[id]/book/__tests__/route.test.ts | 114 +++ app/api/transactions/[id]/book/route.ts | 25 + .../[id]/categorize/__tests__/route.test.ts | 184 +++- app/api/transactions/[id]/categorize/route.ts | 24 + .../__tests__/route.test.ts | 243 +++++ .../transactions/suggest-categories/route.ts | 71 +- .../[id]/categorize/__tests__/route.test.ts | 58 ++ .../transactions/[id]/categorize/route.ts | 23 + .../mapping-engine-transfer-guard.test.ts | 77 ++ .../__tests__/own-account-detector.test.ts | 207 +++- lib/bookkeeping/mapping-engine.ts | 7 +- lib/bookkeeping/own-account-detector.ts | 12 +- .../__tests__/orphaned-ledgers.test.ts | 946 ++++++++++++++++++ lib/cash-accounts/__tests__/service.test.ts | 32 +- lib/cash-accounts/service.ts | 616 +++++++++++- lib/errors/structured-errors.ts | 11 + .../__tests__/bank-reconciliation.test.ts | 305 +++++- lib/reconciliation/bank-reconciliation.ts | 79 +- .../categorize-core.orphaned-counter.test.ts | 122 +++ lib/transactions/categorize-core.ts | 24 + 23 files changed, 3270 insertions(+), 83 deletions(-) create mode 100644 app/api/transactions/suggest-categories/__tests__/route.test.ts create mode 100644 lib/bookkeeping/__tests__/mapping-engine-transfer-guard.test.ts create mode 100644 lib/cash-accounts/__tests__/orphaned-ledgers.test.ts create mode 100644 lib/transactions/__tests__/categorize-core.orphaned-counter.test.ts diff --git a/DECISIONS.md b/DECISIONS.md index 49af4979..52b9019c 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1323,6 +1323,8 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-08-28] AR-PDF minus fix uses ASCII hyphen formatting, not font embedding: registering a Unicode TTF for react-pdf would change the whole document's typography and bundle size to fix one glyph; formatPdfKronor keeps built-in Helvetica and sidesteps WinAnsi's missing U+2212. [2026-08-28] Same-bank warning limited to observed one-session banks (SEB only): prod shows Handelsbanken tolerates 4 concurrent sessions, and the generic warning made a user abandon a legitimate renewal. Planned sync-death visibility work was dropped: already shipped via #1271 (health probe), #1727 (stale state), #1969 (cron unstarve). [2026-08-28] Same-bank warning revised to three tiers after skeptic refutation: hard warn SEB, silent/calm only for verified multi-session banks (Handelsbanken, 4 distinct session_ids observed), legacy hedged warning for unknown banks (fail closed), shared-session siblings exempt (fan-out carries them). +[2026-08-28] Issue #1643 (orphaned cash_accounts after a broken reconnect): an "orphaned" ledger is defined as a non-active cash_accounts row (demoted to manual, disabled, held by a REVOKED or expired/error connection) whose normalized IBAN also belongs to a row on an ACTIVE connection (the #916 demote-to-manual path keeps the IBAN, which is what makes the twin detectable); a row without a live twin is never orphaned, whether manual/CSV or still pointing at a revoked connection (round 3 dropped the unconditional revoked rule: prod holds 16 revoked-held rows with no twin, mostly the company's only 1930 with up to 425 transactions, and #916 already treats revoked holders like manual ones), so transfers between two of the user's own accounts keep booking. Orphans are only refused in the COUNTER position (commit guards in the categorize route and categorizeMatchedTransaction, plus withheld counterparty suggestions); the settlement leg of a stranded row still settles on the orphan. manualLink accepts a voucher line on a sibling ledger (same IBAN) and RE-POINTS transactions.cash_account_id to the live sibling in the same locked UPDATE, gated on matching currency, and REFUSES the link when no sibling qualifies as a destination (round 4): a cross-account link is exactly what the account-keyed reconciliation counts as an imbalance on both ledgers, and the REST/MCP link callers reach manualLink without the unmatched-entries filter. allocatePsd2LedgerAccount names the chart account BAS-style (BAS reference name for a standard slot, else "Bankkonto "), never the ASPSP holder name. Deferred: migrating stranded rows on reconnect / sync dedup across cash_accounts (problem 2), garbage-collecting empty auto-created chart accounts (needs a deletion-vs-deactivation call), and the rättelse-chain match candidates (problem 5, separate root cause). +[2026-08-28] Issue #1643 review follow-up: "same physical account" is keyed on (normalized IBAN, currency), not IBAN alone, because multi-currency accounts (Revolut, Wise) copy one IBAN onto every currency pocket; a same-IBAN other-currency row is a different account. The transfer detector treats the transaction's OWN IBAN as "not a transfer" (interest/fees stamped with the own IBAN never pair with a same-currency twin, live or not) and returns null instead of picking by ledger number when several candidates survive, which is what the pre-PR single-row lookup did. One topology loader defines "live" (enabled + status 'active'; expired/error twins of a live row count as stale) and "orphaned" (a non-live row whose (IBAN, currency) belongs to a live row; a same-IBAN other-currency pocket is never orphaned) for the detector, the match/link flows, the suggest-categories withholding and every commit guard (dashboard, categorizeMatchedTransaction, the v1 REST categorize route, and the free-form /book route at line level). A learned template whose stale 19xx leg is a twin of the settlement row is rewritten to the settlement account (it is the bank leg, not the counter) rather than refused, both at commit and in the suggestion the transactions page is offered; only a twin in the counter position is refused. manualLink re-points a row onto the sibling ledger the voucher was booked on when that sibling is live, or when the own row's holder is definitively gone (bank_connection_id null or revoked) and no sibling is live (same IBAN + same currency = same physical account, and the voucher is the source of truth for where the money went); a row on a live OR merely expired/error/pending connection whose voucher sits only on a dead sibling is REFUSED the link (round 3 left it linking without moving; round 4 turned that into a refusal, since "not live" is not "dead", re-auth renews the same row, moving it would strand the transaction on the orphan, and writing the link anyway is the cross-account link the line check exists to refuse), and unmatched-entries applies the same shouldRepointToSibling predicate so dead-sibling vouchers are not offered to such a row. A voucher touching several sibling ledgers is judged on the best of them (a live sibling, else the first the row may move to), never on the first line PostgREST happens to return. Same-connection re-registration twins (two enabled rows on one active connection sharing IBAN and currency) are deliberately left BOTH live and out of scope: two liveness signals (balance_updated_at, accounts_data uid) were each contradicted by prod data (ingest routes by the entry's ledger_account), so the shape needs a founder decision, see PR #2010 review. The free-form /book guard now covers the single-19xx-line shape too: an ordinary booking pays one PK read of the own row, and when the single bank line is a sibling ledger the row should move to (shouldRepointToSibling) the route re-points cash_account_id in the same locked UPDATE that links the voucher, mirroring manualLink; a twin the row may NOT move to (a dead or disabled twin of a live or still-held row) is refused (round 5: round 4 posted it as typed, which stranded the only bank leg on the dead ledger, problem 4's failure mode), while an unrelated 19xx ledger posts as typed. A disabled (deselected) row is never a sibling, so no automatic re-point lands on a row the transactions page hides; a voucher booked only there is refused like any other cross-account link. [2026-08-28] PDF auto-extract page budget: raised to 8 on pdf-native backends and slice now keeps the last page: the 3-page cap was Bedrock-latency legacy (issue #553), but kept a cap at all so 100-page statements do not burn tokens; last page kept because totals/OCR sit there. Truncation retry doubles AI_EXTRACTION_MAX_TOKENS once instead of raising the default: keeps steady-state cost flat. [2026-08-28] Per-run salary agent gap: new gnubok_set_run_salary staged tool + v1 PATCH instead of extending gnubok_update_payslip_line: the base-salary value lives on salary_run_employees (draft-gated), not on the payslip line (review-agnostic display copy); overloading the line tool would hide the status semantics and keep the recalc-overwrite trap. [2026-08-28] Inbox underlag divergence (#1548) is a separate underlag_status field, not a nulled matched_transaction_journal_entry_id: the book-direct and bulk-book routes 409 on an already-booked transaction, so hiding the verifikat would make the rail re-offer a booking that always fails; the UI keeps divergent items in Att göra with an explanation and a link instead. Anchored-elsewhere conflicts are counted and logged by the daily reconcile cron, never auto-resolved (moving a document between verifikat is a human decision; never-steal is the 2026-08-13 invariant), and not escalated to processing_history from the inline booking path (no dedupe key; it would fire on every booking). The reconcile's 'InboxUnderlagReconciled' event needed a processing_event_types row (FK): the script's old 'InboxUnderlagBackfilled' type was never registered, so its appends had always failed silently. diff --git a/app/api/reconciliation/bank/unmatched-entries/__tests__/route.test.ts b/app/api/reconciliation/bank/unmatched-entries/__tests__/route.test.ts index 398c004b..7e6f93da 100644 --- a/app/api/reconciliation/bank/unmatched-entries/__tests__/route.test.ts +++ b/app/api/reconciliation/bank/unmatched-entries/__tests__/route.test.ts @@ -178,6 +178,126 @@ describe('GET /api/reconciliation/bank/unmatched-entries', () => { expect(body.data[0]).not.toHaveProperty('confidence') }) + // ---------------------------------------------------------------- + // Orphaned reconnect rows (#1643 problem 1): the ranked (dialog) path also + // offers vouchers booked on sibling ledgers of the same physical account + // ---------------------------------------------------------------- + + it('widens ranked candidates to sibling ledgers sharing the account IBAN', async () => { + const iban = 'SE4550000000058398257466' + // The transaction's own (orphaned) row: ledger 1931, carries the IBAN. + enqueue({ data: { id: 'cash-orphan', currency: 'SEK', iban } }) + // Sibling scan (describeCashAccountSiblings): the live row for the same + // physical account sits on 1940 (spaced IBAN variant to prove + // normalization), plus an unrelated account. + enqueue({ + data: [ + { id: 'cash-orphan', ledger_account: '1931', iban, currency: 'SEK', enabled: true, bank_connection_id: null }, + { id: 'cash-live', ledger_account: '1940', iban: 'SE45 5000 0000 0583 9825 7466', currency: 'SEK', enabled: true, bank_connection_id: 'conn-live' }, + // Same IBAN, EUR pocket of a multi-currency account: not a sibling. + { id: 'cash-eur', ledger_account: '1932', iban, currency: 'EUR', enabled: true, bank_connection_id: 'conn-live' }, + { id: 'cash-other', ledger_account: '1935', iban: 'SE1112223334445556667778', currency: 'SEK', enabled: true, bank_connection_id: 'conn-live' }, + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) // bank_connections statuses + enqueueTransaction({ currency: 'SEK', amount: 217.04, date: '2026-03-10' }) + + fetchGLLinesForMatchingMock.mockImplementation((_supabase, _companyId, account) => + account === '1940' + ? Promise.resolve([ + makeGLLine({ + line_id: 'line-live', + journal_entry_id: 'je-live', + debit_amount: 217.04, + credit_amount: 0, + entry_date: '2026-03-10', + }), + ]) + : Promise.resolve([]), + ) + + const response = await GET( + request({ account_number: '1931', transaction_id: TX_ID }), + emptyParams, + ) + const { status, body } = await parseJsonResponse(response) + + expect(status).toBe(200) + // The voucher booked on the LIVE ledger is offered (and ranks as an exact + // hit) even though the row itself is stamped on the orphaned 1931. + expect(body.data).toHaveLength(1) + expect(body.data[0].journal_entry_id).toBe('je-live') + expect(body.data[0].confidence).toBe(0.95) + + const fetchedAccounts = fetchGLLinesForMatchingMock.mock.calls.map((c) => c[2]) + expect(fetchedAccounts).toEqual(['1931', '1940']) + }) + + it('does not offer vouchers on a DEAD sibling to a transaction on the live row (#1643 round 2)', async () => { + const iban = 'SE4550000000058398257466' + // manualLink would not move a live row onto the dead sibling, so the + // dead sibling's vouchers are not candidates for it. + enqueue({ data: { id: 'cash-live', currency: 'SEK', iban } }) + enqueue({ + data: [ + { id: 'cash-live', ledger_account: '1940', iban, currency: 'SEK', enabled: true, bank_connection_id: 'conn-live' }, + { id: 'cash-orphan', ledger_account: '1931', iban, currency: 'SEK', enabled: true, bank_connection_id: 'conn-old' }, + ], + }) + enqueue({ + data: [ + { id: 'conn-live', status: 'active' }, + { id: 'conn-old', status: 'revoked' }, + ], + }) + enqueueTransaction({ currency: 'SEK', amount: 217.04, date: '2026-03-10' }) + fetchGLLinesForMatchingMock.mockResolvedValue([]) + + const response = await GET( + request({ account_number: '1940', transaction_id: TX_ID }), + emptyParams, + ) + expect(response.status).toBe(200) + const fetchedAccounts = fetchGLLinesForMatchingMock.mock.calls.map((c) => c[2]) + expect(fetchedAccounts).toEqual(['1940']) + }) + + it('does not offer a demoted twin\'s vouchers to a row on an EXPIRED connection (#1643 round 3)', async () => { + const iban = 'SE4550000000058398257466' + // manualLink would not move a renewable row onto the dead twin either. + enqueue({ data: { id: 'cash-expired', currency: 'SEK', iban } }) + enqueue({ + data: [ + { id: 'cash-expired', ledger_account: '1930', iban, currency: 'SEK', enabled: true, bank_connection_id: 'conn-expired' }, + { id: 'cash-demoted', ledger_account: '1931', iban, currency: 'SEK', enabled: true, bank_connection_id: null }, + ], + }) + enqueue({ data: [{ id: 'conn-expired', status: 'expired' }] }) + enqueueTransaction({ currency: 'SEK', amount: 217.04, date: '2026-03-10' }) + fetchGLLinesForMatchingMock.mockResolvedValue([]) + + const response = await GET( + request({ account_number: '1930', transaction_id: TX_ID }), + emptyParams, + ) + expect(response.status).toBe(200) + const fetchedAccounts = fetchGLLinesForMatchingMock.mock.calls.map((c) => c[2]) + expect(fetchedAccounts).toEqual(['1930']) + }) + + it('does not widen the unranked (reconciliation view) path', async () => { + const iban = 'SE4550000000058398257466' + enqueue({ data: { id: 'cash-orphan', currency: 'SEK', iban } }) + fetchGLLinesForMatchingMock.mockResolvedValue([makeGLLine()]) + + const response = await GET(request({ account_number: '1931' }), emptyParams) + const { status } = await parseJsonResponse(response) + + expect(status).toBe(200) + expect(fetchGLLinesForMatchingMock).toHaveBeenCalledTimes(1) + expect(fetchGLLinesForMatchingMock.mock.calls[0][2]).toBe('1931') + }) + // ---------------------------------------------------------------- // Foreign currency: never ranked against an unconverted SEK ledger leg // ---------------------------------------------------------------- diff --git a/app/api/reconciliation/bank/unmatched-entries/route.ts b/app/api/reconciliation/bank/unmatched-entries/route.ts index 3a630714..a2e4695c 100644 --- a/app/api/reconciliation/bank/unmatched-entries/route.ts +++ b/app/api/reconciliation/bank/unmatched-entries/route.ts @@ -5,6 +5,7 @@ import { ledgerLineAmountIn, tryReconcileTransaction, } from '@/lib/reconciliation/bank-reconciliation' +import { describeCashAccountSiblings, normalizeIban, shouldRepointToSibling } from '@/lib/cash-accounts/service' import type { Transaction } from '@/types' export const GET = withRouteContext( @@ -34,9 +35,11 @@ export const GET = withRouteContext( // companies on first connection. // `currency` comes along because it, not the transaction's own currency, is // the unit this account is reconciled in: see the ranking block below. + // `iban` identifies the physical bank account so the ranked (dialog) path + // can widen the candidate set to sibling ledgers of the same account. const { data: cashAccount } = await supabase .from('cash_accounts') - .select('id, currency') + .select('id, currency, iban') .eq('company_id', companyId) .eq('ledger_account', accountNumber) .maybeSingle() @@ -53,9 +56,53 @@ export const GET = withRouteContext( // into "no currency" and disable ranking for a SEK company. const accountCurrency = (cashAccount.currency as string | null) ?? 'SEK' - const lines = await fetchGLLinesForMatching(supabase, companyId, accountNumber, dateFrom, dateTo, includeMatched) + let lines = await fetchGLLinesForMatching(supabase, companyId, accountNumber, dateFrom, dateTo, includeMatched) if (transactionId) { + // The "Matcha mot befintlig verifikation" dialog resolves the account + // from the TRANSACTION's own cash_accounts row. When that row is an + // orphan of a broken reconnect (issue #1643 problem 1), the verifikat + // the row settles is booked on the LIVE ledger of the same physical + // account (e.g. row stamped 1931, voucher on 1940) and could never be + // offered. Sibling rows share the account's IBAN, so the candidate set + // widens to every ledger carrying it in the SAME currency (the other + // currency pockets of a multi-currency account share the IBAN but are + // different accounts). Accounts without an IBAN (manual, CSV) have no + // siblings and keep the exact single-ledger behavior. + // A sibling that is NOT live (demoted or held by a revoked/expired + // connection) is only offered when manualLink would re-point the row + // onto it (shouldRepointToSibling: the own row's holder is gone and no + // sibling is live): a live row, or one on a merely expired connection + // that re-auth renews, must never be parked on a row no connection can + // sync again. + const ownIban = normalizeIban((cashAccount as { iban?: string | null }).iban ?? null) + const siblingInfo = ownIban + ? await describeCashAccountSiblings(supabase, companyId, cashAccount.id as string) + : null + if (siblingInfo && siblingInfo.siblings.length > 0) { + const siblingLedgers = [ + ...new Set( + siblingInfo.siblings + .filter((row) => shouldRepointToSibling(siblingInfo, row)) + .map((row) => row.ledger_account), + ), + ] + if (siblingLedgers.length > 0) { + const merged = [...lines] + const seen = new Set(lines.map((l) => l.line_id)) + for (const ledger of siblingLedgers) { + const siblingLines = await fetchGLLinesForMatching( + supabase, companyId, ledger, dateFrom, dateTo, includeMatched, + ) + for (const line of siblingLines) { + if (seen.has(line.line_id)) continue + seen.add(line.line_id) + merged.push(line) + } + } + lines = merged + } + } // company-scoped fetch (defense-in-depth). A malformed/foreign id yields no // row → we fall through to the unranked list rather than erroring. const { data: tx } = await supabase diff --git a/app/api/transactions/[id]/book/__tests__/route.test.ts b/app/api/transactions/[id]/book/__tests__/route.test.ts index f9d81857..d96140cf 100644 --- a/app/api/transactions/[id]/book/__tests__/route.test.ts +++ b/app/api/transactions/[id]/book/__tests__/route.test.ts @@ -237,6 +237,120 @@ describe('POST /api/transactions/[id]/book', () => { ) }) + it('returns 400 TX_CATEGORIZE_ORPHANED_COUNTER_ACCOUNT when a line books the settlement row against its active twin (#1643)', async () => { + // The issue's dialog shape: 1930 and 1931 both enabled on one active + // connection; "Ändra rader" pre-filled 1930 debit / 1931 credit from a + // template learned on 1931. Booking it would move money between two + // ledgers of one physical account with nothing reaching the P&L. + const iban = 'SE4550000000058398257466' + const tx = makeTransaction({ id: 'tx-1', amount: 500, journal_entry_id: null, cash_account_id: 'ca-1930' }) + enqueue({ data: tx, error: null }) // fetch transaction + enqueue({ + data: [ + { id: 'ca-1930', ledger_account: '1930', iban, currency: 'SEK', enabled: true, bank_connection_id: 'conn-live' }, + { id: 'ca-1931', ledger_account: '1931', iban, currency: 'SEK', enabled: true, bank_connection_id: 'conn-live' }, + ], + }) // cash_accounts topology + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) // bank_connections statuses + + const request = createMockRequest('/api/transactions/tx-1/book', { + method: 'POST', + body: { + ...validBody, + lines: [ + { account_number: '1930', debit_amount: 500, credit_amount: 0 }, + { account_number: '1931', debit_amount: 0, credit_amount: 500 }, + ], + }, + }) + const response = await POST(request, createMockRouteParams({ id: 'tx-1' })) + const { status, body } = await parseJsonResponse<{ error: { code: string; details: { accountNumber: string } } }>(response) + + expect(status).toBe(400) + expect(body.error.code).toBe('TX_CATEGORIZE_ORPHANED_COUNTER_ACCOUNT') + expect(body.error.details.accountNumber).toBe('1931') + expect(mockCreateJournalEntry).not.toHaveBeenCalled() + }) + + it('re-points a stranded row onto the live sibling when its single bank line is that ledger (#1643 round 4)', async () => { + // Nyte-shape: the transaction sits on the demoted 1931, the user books it + // with the bank leg on the live 1940 against a P&L account. The voucher + // posts on 1940 and the row moves there in the same locked UPDATE, so + // neither ledger's reconciliation is left with a half. + const iban = 'SE4550000000058398257466' + const tx = makeTransaction({ id: 'tx-1', amount: 500, journal_entry_id: null, cash_account_id: 'ca-orphan' }) + enqueue({ data: tx, error: null }) // fetch transaction + enqueue({ data: { ledger_account: '1931' } }) // own row + enqueue({ + data: [ + { id: 'ca-orphan', ledger_account: '1931', iban, currency: 'SEK', enabled: true, bank_connection_id: null }, + { id: 'ca-live', ledger_account: '1940', iban, currency: 'SEK', enabled: true, bank_connection_id: 'conn-live' }, + ], + }) // cash_accounts topology + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) // bank_connections statuses + mockCreateJournalEntry.mockResolvedValue(makeJournalEntry({ id: 'je-new' })) + enqueue({ data: [{ id: 'tx-1' }], error: null }) // link update + + const request = createMockRequest('/api/transactions/tx-1/book', { + method: 'POST', + body: { + ...validBody, + lines: [ + { account_number: '1940', debit_amount: 500, credit_amount: 0 }, + { account_number: '8311', debit_amount: 0, credit_amount: 500 }, + ], + }, + }) + const response = await POST(request, createMockRouteParams({ id: 'tx-1' })) + + expect(response.status).toBe(200) + expect(mockCreateJournalEntry).toHaveBeenCalledTimes(1) + expect(findCalls('transactions', 'update')).toContainEqual([ + expect.objectContaining({ journal_entry_id: 'je-new', cash_account_id: 'ca-live' }), + ]) + }) + + it('returns 400 TX_CATEGORIZE_ORPHANED_COUNTER_ACCOUNT when the single bank line sits on a dead twin of the live own row (#1643 round 5)', async () => { + // Problem 4: the transaction sits on the live 1940, the dialog pre-fills + // [1931, 3011] from a template learned before the reconnect, and 1931 is + // the revoked twin. Posting would strand the only bank leg on 1931 while + // the transaction stays on 1940, so it is refused before the engine runs. + const iban = 'SE4550000000058398257466' + const tx = makeTransaction({ id: 'tx-1', amount: 500, journal_entry_id: null, cash_account_id: 'ca-live' }) + enqueue({ data: tx, error: null }) // fetch transaction + enqueue({ data: { ledger_account: '1940' } }) // own row + enqueue({ + data: [ + { id: 'ca-live', ledger_account: '1940', iban, currency: 'SEK', enabled: true, bank_connection_id: 'conn-live' }, + { id: 'ca-orphan', ledger_account: '1931', iban, currency: 'SEK', enabled: true, bank_connection_id: 'conn-old' }, + ], + }) // cash_accounts topology + enqueue({ + data: [ + { id: 'conn-live', status: 'active' }, + { id: 'conn-old', status: 'revoked' }, + ], + }) // bank_connections statuses + + const request = createMockRequest('/api/transactions/tx-1/book', { + method: 'POST', + body: { + ...validBody, + lines: [ + { account_number: '1931', debit_amount: 500, credit_amount: 0 }, + { account_number: '3011', debit_amount: 0, credit_amount: 500 }, + ], + }, + }) + const response = await POST(request, createMockRouteParams({ id: 'tx-1' })) + const { status, body } = await parseJsonResponse<{ error: { code: string; details: { accountNumber: string } } }>(response) + + expect(status).toBe(400) + expect(body.error.code).toBe('TX_CATEGORIZE_ORPHANED_COUNTER_ACCOUNT') + expect(body.error.details.accountNumber).toBe('1931') + expect(mockCreateJournalEntry).not.toHaveBeenCalled() + }) + it('atomically unignores an ignored transaction when booking it', async () => { const tx = makeTransaction({ id: 'tx-1', diff --git a/app/api/transactions/[id]/book/route.ts b/app/api/transactions/[id]/book/route.ts index 7879c1a5..4ee957a4 100644 --- a/app/api/transactions/[id]/book/route.ts +++ b/app/api/transactions/[id]/book/route.ts @@ -3,6 +3,7 @@ import { eventBus } from '@/lib/events' import { ensureInitialized } from '@/lib/init' import { withRouteContext } from '@/lib/api/with-route-context' import { createJournalEntry } from '@/lib/bookkeeping/engine' +import { guardBookedCounterLines } from '@/lib/cash-accounts/service' import { reverseOrphanedJournalEntry } from '@/lib/bookkeeping/cancel-orphaned-entry' import { bookkeepingErrorResponse } from '@/lib/bookkeeping/errors' import { validateBody } from '@/lib/api/validate' @@ -143,6 +144,29 @@ export const POST = withRouteContext<{ params: Promise<{ id: string }> }>( dupLog.warn('booking-time duplicate detection failed (continuing)', err as Error) } + // A 19xx counter line that is a twin of the transaction's own cash account + // (same IBAN, same currency: the other ledger of one connection, or a + // stale row left by a broken reconnect) or an orphaned ledger books one + // physical account against itself or onto a junk balance-sheet account. + // The dialog pre-fills such lines from learned templates (issue #1643 + // problem 4); this is the same refusal the categorize paths apply. A + // booking whose single 19xx line is a sibling ledger the row should move + // to (the live twin of a stranded row) instead re-points the transaction + // there in the locked UPDATE below, as manualLink does for the same + // voucher (see guardBookedCounterLines). + const { refusedLedger, repointCashAccountId } = await guardBookedCounterLines( + supabase, + companyId, + lines.map((line) => line.account_number), + transaction.cash_account_id ?? null, + ) + if (refusedLedger) { + return errorResponseFromCode('TX_CATEGORIZE_ORPHANED_COUNTER_ACCOUNT', log, { + requestId, + details: { accountNumber: refusedLedger }, + }) + } + // Create journal entry via the engine let journalEntry try { @@ -174,6 +198,7 @@ export const POST = withRouteContext<{ params: Promise<{ id: string }> }>( is_business: true, is_ignored: false, category: 'uncategorized', + ...(repointCashAccountId ? { cash_account_id: repointCashAccountId } : {}), }) .eq('id', id) .eq('company_id', companyId) diff --git a/app/api/transactions/[id]/categorize/__tests__/route.test.ts b/app/api/transactions/[id]/categorize/__tests__/route.test.ts index 8ccb83bb..28c78537 100644 --- a/app/api/transactions/[id]/categorize/__tests__/route.test.ts +++ b/app/api/transactions/[id]/categorize/__tests__/route.test.ts @@ -76,7 +76,11 @@ vi.mock('@/lib/bookkeeping/mapping-engine', () => ({ }, })) -vi.mock('@/lib/bookkeeping/counterparty-templates', () => ({ +// Spread the real module so buildMappingResultFromCounterpartyTemplate (pure) +// replays a learned template exactly as production does; only the learning +// write is stubbed. +vi.mock('@/lib/bookkeeping/counterparty-templates', async (importActual) => ({ + ...(await importActual()), upsertCounterpartyTemplate: vi.fn().mockResolvedValue(undefined), })) @@ -1552,4 +1556,182 @@ describe('POST /api/transactions/[id]/categorize', () => { expect(body.error.details.accountNumber).toBe('5420') expect(mockCreateTransactionJournalEntry).not.toHaveBeenCalled() }) + + // ---------------------------------------------------------------- + // Orphaned counter-account guard (issue #1643 problem 4) + // ---------------------------------------------------------------- + + it('returns 400 TX_CATEGORIZE_ORPHANED_COUNTER_ACCOUNT when a learned template credits a ledger held by a revoked connection', async () => { + // The issue's exact shape: +217,04 interest on the live 1940 account, and + // a template learned while the detector paired with the orphan row + // proposes 1940 debit / 1931 credit (revenue onto a junk asset account). + const tx = makeTransaction({ + id: 'tx-1', + amount: 217.04, + merchant_name: 'SEB', + journal_entry_id: null, + cash_account_id: 'ca-live', + }) + + enqueue({ data: tx, error: null }) + enqueue({ data: { entity_type: 'aktiebolag', fiscal_year_start_month: 1 }, error: null }) + // categorization_templates: the learned counterparty template + enqueue({ + data: { + id: '11111111-1111-4111-8111-111111111111', + company_id: 'company-1', + counterparty_name: 'SEB', + counterparty_aliases: [], + debit_account: '1940', + credit_account: '1931', + vat_treatment: null, + vat_account: null, + category: null, + line_pattern: null, + occurrence_count: 3, + confidence: 0.9, + source: 'auto_learned', + is_active: true, + }, + error: null, + }) + // resolveSettlementAccount: the row's own cash account is the live 1940 + enqueue({ data: { ledger_account: '1940' }, error: null }) + // Guard: cash_accounts scan + bank_connections status lookup + enqueue({ + data: [ + { id: 'ca-live', ledger_account: '1940', bank_connection_id: 'conn-new', iban: 'SE455', enabled: true }, + { id: 'ca-orphan', ledger_account: '1931', bank_connection_id: 'conn-old', iban: 'SE455', enabled: true }, + ], + error: null, + }) + enqueue({ + data: [ + { id: 'conn-new', status: 'active' }, + { id: 'conn-old', status: 'revoked' }, + ], + error: null, + }) + + const request = createMockRequest('/api/transactions/tx-1/categorize', { + method: 'POST', + body: { is_business: true, counterparty_template_id: '11111111-1111-4111-8111-111111111111' }, + }) + const response = await POST(request, createMockRouteParams({ id: 'tx-1' })) + const { status, body } = await parseJsonResponse<{ + error: { code: string; details: { accountNumber?: string } } + }>(response) + + expect(status).toBe(400) + expect(body.error.code).toBe('TX_CATEGORIZE_ORPHANED_COUNTER_ACCOUNT') + expect(body.error.details.accountNumber).toBe('1931') + expect(mockCreateTransactionJournalEntry).not.toHaveBeenCalled() + }) + + it('rewrites a learned template whose stale BANK leg is a twin of the settlement row instead of refusing (#1643)', async () => { + // Template learned as 5010 / 1931 while the account sat on 1931; the + // transaction now settles on the live 1940 row of the same IBAN. The + // counter (5010) is fine, only the bank side is stale. + const tx = makeTransaction({ + id: 'tx-1', + amount: -1200, + merchant_name: 'Hyresvärden', + journal_entry_id: null, + cash_account_id: 'ca-live', + }) + + enqueue({ data: tx, error: null }) + enqueue({ data: { entity_type: 'aktiebolag', fiscal_year_start_month: 1 }, error: null }) + enqueue({ + data: { + id: '11111111-1111-4111-8111-111111111111', + company_id: 'company-1', + counterparty_name: 'Hyresvärden', + counterparty_aliases: [], + debit_account: '5010', + credit_account: '1931', + vat_treatment: null, + vat_account: null, + category: null, + line_pattern: null, + occurrence_count: 3, + confidence: 0.9, + source: 'auto_learned', + is_active: true, + }, + error: null, + }) + enqueue({ data: { ledger_account: '1940' }, error: null }) // resolveSettlementAccount + enqueue({ + data: [ + { id: 'ca-live', ledger_account: '1940', bank_connection_id: 'conn-new', iban: 'SE455', enabled: true, currency: 'SEK' }, + { id: 'ca-orphan', ledger_account: '1931', bank_connection_id: null, iban: 'SE455', enabled: true, currency: 'SEK' }, + ], + error: null, + }) + enqueue({ data: [{ id: 'conn-new', status: 'active' }], error: null }) + // ensureFiscalPeriod: existing period + enqueue({ data: [{ id: 'period-1' }], error: null }) + mockCreateTransactionJournalEntry.mockResolvedValue({ id: 'je-1' }) + mockSaveUserMappingRule.mockResolvedValue(undefined) + // Update transaction (CAS guard) + enqueue({ data: [{ id: 'tx-1' }], error: null }) + + const request = createMockRequest('/api/transactions/tx-1/categorize', { + method: 'POST', + body: { is_business: true, counterparty_template_id: '11111111-1111-4111-8111-111111111111' }, + }) + const response = await POST(request, createMockRouteParams({ id: 'tx-1' })) + const { status, body } = await parseJsonResponse(response) + + expect(status, JSON.stringify(body)).toBe(200) + expect(mockCreateTransactionJournalEntry).toHaveBeenCalledTimes(1) + const mapping = mockCreateTransactionJournalEntry.mock.calls[0][4] as { + debit_account: string + credit_account: string + } + expect(mapping.debit_account).toBe('5010') + expect(mapping.credit_account).toBe('1940') + }) + + it('still books a transfer whose 19xx counter is a live cash account', async () => { + const tx = makeTransaction({ + id: 'tx-1', + amount: -500, + merchant_name: 'Sparkonto', + journal_entry_id: null, + }) + + enqueue({ data: tx, error: null }) + enqueue({ data: { entity_type: 'aktiebolag', fiscal_year_start_month: 1 }, error: null }) + enqueue({ data: [], error: null }) // resolveSettlementAccount: no enabled cash accounts -> 1930 + // account_override chart lookup: 1940 exists and is active + enqueue({ data: { account_number: '1940', account_class: 1 }, error: null }) + // Guard: 1940 is held by an ACTIVE connection, so it is a genuine transfer target + enqueue({ + data: [{ id: 'ca-live', ledger_account: '1940', bank_connection_id: 'conn-live', iban: 'SE455', enabled: true }], + error: null, + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }], error: null }) + // ensureFiscalPeriod: existing period + enqueue({ data: [{ id: 'period-1' }], error: null }) + + mockCreateTransactionJournalEntry.mockResolvedValue({ id: 'je-1' }) + mockSaveUserMappingRule.mockResolvedValue(undefined) + + // Update transaction (CAS guard) + enqueue({ data: [{ id: 'tx-1' }], error: null }) + + const request = createMockRequest('/api/transactions/tx-1/categorize', { + method: 'POST', + body: { is_business: true, category: 'expense_other', account_override: '1940' }, + }) + const response = await POST(request, createMockRouteParams({ id: 'tx-1' })) + const { status, body } = await parseJsonResponse<{ success: boolean; journal_entry_id: string }>(response) + + expect(status).toBe(200) + expect(body.success).toBe(true) + expect(body.journal_entry_id).toBe('je-1') + expect(mockCreateTransactionJournalEntry).toHaveBeenCalledTimes(1) + }) }) diff --git a/app/api/transactions/[id]/categorize/route.ts b/app/api/transactions/[id]/categorize/route.ts index 57d24dde..b2b30a2e 100644 --- a/app/api/transactions/[id]/categorize/route.ts +++ b/app/api/transactions/[id]/categorize/route.ts @@ -12,6 +12,7 @@ import { detectBookingDuplicate } from '@/lib/transactions/booking-duplicate-det import { appendProcessingHistory } from '@/lib/processing-history/append' import { saveUserMappingRule, applySettlementAccount } from '@/lib/bookkeeping/mapping-engine' import { resolveSettlementAccount } from '@/lib/bookkeeping/settlement-account' +import { guardCounterLegs } from '@/lib/cash-accounts/service' import { upsertCounterpartyTemplate, buildMappingResultFromCounterpartyTemplate } from '@/lib/bookkeeping/counterparty-templates' import { withRouteContext } from '@/lib/api/with-route-context' import { errorResponse, errorResponseFromCode, getStructuredError } from '@/lib/errors/get-structured-error' @@ -415,6 +416,29 @@ export const POST = withRouteContext( }) } + // Issue #1643 problem 4: a learned template or transfer proposal must never + // book the COUNTER leg onto an orphaned cash-account ledger, or onto a twin + // ledger of the transaction's own bank account. Confirming such a proposal + // silently drops revenue/expense from the P&L onto a junk balance-sheet + // account. A twin that is merely the stale BANK leg of a learned template + // is rewritten to the settlement account instead (see guardCounterLegs). + { + const guarded = await guardCounterLegs( + supabase, + companyId!, + mappingResult, + settlementAccount, + transaction.cash_account_id, + ) + if (guarded.refusedLedger) { + return errorResponseFromCode('TX_CATEGORIZE_ORPHANED_COUNTER_ACCOUNT', txLog, { + requestId, + details: { accountNumber: guarded.refusedLedger }, + }) + } + mappingResult = guarded.mappingResult + } + // Pre-validate every account the engine will resolve. Templates, // counterparty templates, and category defaults can all reference accounts // that aren't activated in this company's kontoplan. Without this check, diff --git a/app/api/transactions/suggest-categories/__tests__/route.test.ts b/app/api/transactions/suggest-categories/__tests__/route.test.ts new file mode 100644 index 00000000..24744674 --- /dev/null +++ b/app/api/transactions/suggest-categories/__tests__/route.test.ts @@ -0,0 +1,243 @@ +/** + * POST /api/transactions/suggest-categories: counterparty suggestions whose + * learned accounts reference an ORPHANED cash-account ledger are withheld + * (issue #1643 problem 4). A learned template carries the ledger it was + * learned on; replaying it after a broken reconnect would pre-fill a junk + * balance-sheet account as the booking dialog's counter-account. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { NextResponse } from 'next/server' +import { createQueuedMockSupabase, createMockRequest, parseJsonResponse } from '@/tests/helpers' + +const { supabase, enqueue, reset } = createQueuedMockSupabase() + +const requireAuthMock = vi.fn() +vi.mock('@/lib/auth/require-auth', () => ({ + requireAuth: (...args: unknown[]) => requireAuthMock(...args), +})) + +vi.mock('@/lib/company/context', () => ({ + getActiveCompanyId: vi.fn().mockResolvedValue('company-1'), + requireCompanyId: vi.fn().mockResolvedValue('company-1'), +})) + +const findCounterpartyTemplatesBatchMock = vi.fn() +// Spread the real module: category-suggestions also imports pure helpers from +// here (normalizeCounterpartyName), and only the DB-backed batch lookup is stubbed. +vi.mock('@/lib/bookkeeping/counterparty-templates', async (importActual) => ({ + ...(await importActual()), + findCounterpartyTemplatesBatch: (...args: unknown[]) => findCounterpartyTemplatesBatchMock(...args), +})) + +const loadCounterLegTopologyMock = vi.fn() +vi.mock('@/lib/cash-accounts/service', () => ({ + loadCounterLegTopology: (...args: unknown[]) => loadCounterLegTopologyMock(...args), +})) + +/** Topology stub mirroring lib/cash-accounts/service: orphan set + per-row context. */ +function topology( + orphaned: string[], + contexts: Record = {}, +) { + return { + orphaned: new Set(orphaned), + contextFor: (cashAccountId: string | null | undefined) => { + const ctx = cashAccountId ? contexts[cashAccountId] : undefined + return ctx + ? { settlementLedger: ctx.settlementLedger, twins: new Set(ctx.twins) } + : { settlementLedger: null, twins: new Set() } + }, + } +} + +import { POST } from '../route' + +const emptyParams = { params: Promise.resolve({}) } +const TX_ID = '22222222-2222-4222-8222-222222222222' + +function makeTemplate(overrides: Record = {}) { + return { + id: 'cpt-1', + user_id: null, + company_id: 'company-1', + counterparty_name: 'SEB', + counterparty_aliases: [], + debit_account: '1940', + credit_account: '1931', + vat_treatment: null, + vat_account: null, + category: null, + line_pattern: null, + occurrence_count: 3, + confidence: 0.9, + last_seen_date: '2026-07-01', + source: 'auto_learned', + is_active: true, + created_at: '2026-07-01T00:00:00Z', + updated_at: '2026-07-01T00:00:00Z', + ...overrides, + } +} + +/** Queue the four queries the route always runs, in order. */ +function enqueueBaseQueries() { + enqueue({ data: [{ id: TX_ID, amount: 217.04, currency: 'SEK', description: 'Ränta' }] }) // transactions + enqueue({ data: [] }) // mapping_rules + enqueue({ data: [] }) // historical transactions + enqueue({ data: { entity_type: 'aktiebolag' } }) // company_settings +} + +function request() { + return createMockRequest('/api/transactions/suggest-categories', { + method: 'POST', + body: { transaction_ids: [TX_ID] }, + }) +} + +type Body = { + template_suggestions: Record> +} + +describe('POST /api/transactions/suggest-categories', () => { + beforeEach(() => { + vi.clearAllMocks() + reset() + requireAuthMock.mockResolvedValue({ user: { id: 'user-1' }, supabase }) + loadCounterLegTopologyMock.mockResolvedValue(topology([])) + }) + + it('returns 401 when unauthenticated', async () => { + requireAuthMock.mockResolvedValue({ + user: null, + supabase, + error: NextResponse.json({ error: 'Unauthorized' }, { status: 401 }), + }) + const response = await POST(request(), emptyParams) + expect(response.status).toBe(401) + }) + + it('withholds a counterparty suggestion whose learned accounts hit an orphaned ledger', async () => { + enqueueBaseQueries() + findCounterpartyTemplatesBatchMock.mockResolvedValue( + new Map([[TX_ID, { template: makeTemplate(), confidence: 0.9 }]]), + ) + loadCounterLegTopologyMock.mockResolvedValue(topology(['1931'])) + + const response = await POST(request(), emptyParams) + const { status, body } = await parseJsonResponse(response) + + expect(status).toBe(200) + const suggestions = body.template_suggestions[TX_ID] ?? [] + expect(suggestions.find((s) => s.template_id?.startsWith('cp:'))).toBeUndefined() + }) + + it('keeps a template whose stale BANK leg is a twin of the live row, shown on the settlement ledger (#1643 round 2)', async () => { + // Transaction on the live 1940 row; the template was learned as 5010 / + // 1931 before the reconnect moved the account. The commit guard rewrites + // 1931 to 1940 and books it, so the suggestion must be offered the same way. + enqueue({ data: [{ id: TX_ID, amount: -1200, currency: 'SEK', description: 'Hyra', cash_account_id: 'ca-live' }] }) + enqueue({ data: [] }) // mapping_rules + enqueue({ data: [] }) // historical transactions + enqueue({ data: { entity_type: 'aktiebolag' } }) // company_settings + findCounterpartyTemplatesBatchMock.mockResolvedValue( + new Map([[TX_ID, { template: makeTemplate({ debit_account: '5010', credit_account: '1931' }), confidence: 0.9 }]]), + ) + loadCounterLegTopologyMock.mockResolvedValue( + topology(['1931'], { 'ca-live': { settlementLedger: '1940', twins: ['1931'] } }), + ) + + const response = await POST(request(), emptyParams) + const { status, body } = await parseJsonResponse(response) + + expect(status).toBe(200) + const suggestions = body.template_suggestions[TX_ID] ?? [] + expect(suggestions.some((s) => s.debit_account === '5010' && s.credit_account === '1940')).toBe(true) + expect(suggestions.some((s) => s.credit_account === '1931')).toBe(false) + }) + + it('rewrites a both-active twin leg (two ledgers on one connection) to the settlement ledger (#1643 round 2)', async () => { + // Nothing is orphaned: 1930 and 1931 are both enabled on the active + // connection. The template learned on 1931 is still the same account. + enqueue({ data: [{ id: TX_ID, amount: -1200, currency: 'SEK', description: 'Hyra', cash_account_id: 'ca-1930' }] }) + enqueue({ data: [] }) + enqueue({ data: [] }) + enqueue({ data: { entity_type: 'aktiebolag' } }) + findCounterpartyTemplatesBatchMock.mockResolvedValue( + new Map([[TX_ID, { template: makeTemplate({ debit_account: '5010', credit_account: '1931' }), confidence: 0.9 }]]), + ) + loadCounterLegTopologyMock.mockResolvedValue( + topology([], { 'ca-1930': { settlementLedger: '1930', twins: ['1931'] } }), + ) + + const response = await POST(request(), emptyParams) + const { body } = await parseJsonResponse(response) + const suggestions = body.template_suggestions[TX_ID] ?? [] + expect(suggestions.some((s) => s.debit_account === '5010' && s.credit_account === '1930')).toBe(true) + }) + + it('withholds a template whose twin leg sits in the COUNTER position (settlement against itself)', async () => { + enqueue({ data: [{ id: TX_ID, amount: 217.04, currency: 'SEK', description: 'Ränta', cash_account_id: 'ca-live' }] }) + enqueue({ data: [] }) + enqueue({ data: [] }) + enqueue({ data: { entity_type: 'aktiebolag' } }) + findCounterpartyTemplatesBatchMock.mockResolvedValue( + new Map([[TX_ID, { template: makeTemplate({ debit_account: '1940', credit_account: '1931' }), confidence: 0.9 }]]), + ) + loadCounterLegTopologyMock.mockResolvedValue( + topology([], { 'ca-live': { settlementLedger: '1940', twins: ['1931'] } }), + ) + + const response = await POST(request(), emptyParams) + const { body } = await parseJsonResponse(response) + const suggestions = body.template_suggestions[TX_ID] ?? [] + expect(suggestions.find((s) => s.template_id?.startsWith('cp:'))).toBeUndefined() + }) + + it('keeps a learned suggestion whose only 19xx leg is the transaction\'s OWN (orphaned) settlement ledger', async () => { + // A transaction still stranded on the orphaned 1931 row: the template + // learned as 5010 / 1931 is valid for it, the 1931 leg is its bank side. + enqueue({ data: [{ id: TX_ID, amount: -1200, currency: 'SEK', description: 'Hyra', cash_account_id: 'ca-orphan' }] }) + enqueue({ data: [] }) // mapping_rules + enqueue({ data: [] }) // historical transactions + enqueue({ data: { entity_type: 'aktiebolag' } }) // company_settings + findCounterpartyTemplatesBatchMock.mockResolvedValue( + new Map([[TX_ID, { template: makeTemplate({ debit_account: '5010', credit_account: '1931' }), confidence: 0.9 }]]), + ) + loadCounterLegTopologyMock.mockResolvedValue( + topology(['1931'], { 'ca-orphan': { settlementLedger: '1931', twins: ['1940'] } }), + ) + + const response = await POST(request(), emptyParams) + const { status, body } = await parseJsonResponse(response) + + expect(status).toBe(200) + const suggestions = body.template_suggestions[TX_ID] ?? [] + expect(suggestions.some((s) => s.debit_account === '5010' && s.credit_account === '1931')).toBe(true) + }) + + it('keeps a counterparty suggestion whose accounts are clean', async () => { + enqueueBaseQueries() + findCounterpartyTemplatesBatchMock.mockResolvedValue( + new Map([[TX_ID, { template: makeTemplate({ debit_account: '1930', credit_account: '8311' }), confidence: 0.9 }]]), + ) + loadCounterLegTopologyMock.mockResolvedValue(topology(['1931'])) + + const response = await POST(request(), emptyParams) + const { status, body } = await parseJsonResponse(response) + + expect(status).toBe(200) + const suggestions = body.template_suggestions[TX_ID] ?? [] + expect(suggestions.some((s) => s.debit_account === '1930' && s.credit_account === '8311')).toBe(true) + }) + + it('does not query the orphan set when no counterparty template references a 19xx account', async () => { + enqueueBaseQueries() + findCounterpartyTemplatesBatchMock.mockResolvedValue( + new Map([[TX_ID, { template: makeTemplate({ debit_account: '6570', credit_account: '2440' }), confidence: 0.9 }]]), + ) + + const response = await POST(request(), emptyParams) + expect(response.status).toBe(200) + expect(loadCounterLegTopologyMock).not.toHaveBeenCalled() + }) +}) diff --git a/app/api/transactions/suggest-categories/route.ts b/app/api/transactions/suggest-categories/route.ts index 8d767003..a70deb50 100644 --- a/app/api/transactions/suggest-categories/route.ts +++ b/app/api/transactions/suggest-categories/route.ts @@ -2,7 +2,8 @@ import { NextResponse } from 'next/server' import { withRouteContext } from '@/lib/api/with-route-context' import { getSuggestedCategories, getSuggestedTemplates, buildMerchantHistory, merchantHistoryFor, buildCounterpartySuggestion, type SuggestedCategory, type SuggestedTemplate } from '@/lib/transactions/category-suggestions' import { findCounterpartyTemplatesBatch } from '@/lib/bookkeeping/counterparty-templates' -import type { Transaction, EntityType } from '@/types' +import { loadCounterLegTopology, type CounterLegTopology } from '@/lib/cash-accounts/service' +import type { Transaction, EntityType, CategorizationTemplate } from '@/types' /** * POST /api/transactions/suggest-categories @@ -82,12 +83,76 @@ export const POST = withRouteContext( template_suggestions[tx.id] = await getSuggestedTemplates(tx as Transaction, entityType, mappingRules || undefined) } - // Inject counterparty template matches as top suggestions + // Inject counterparty template matches as top suggestions. A learned + // template can carry the ledger it was learned on. The same rules + // guardCounterLegs applies at commit (issue #1643 problem 4) decide what + // the transactions page is offered, so a suggestion is never shown that + // the commit guard would refuse, and never withheld that it would book: + // - a 19xx leg that is a TWIN of the transaction's own row (same IBAN, + // same currency: the stale bank leg of a template learned before a + // reconnect moved the account, or the other enabled ledger of one + // connection) is rewritten to the settlement ledger; if that leaves + // the settlement ledger against itself the suggestion is withheld, + // - a remaining 19xx leg in the orphaned set (revoked connection, or a + // stale twin of some live account) is a counter-position orphan and + // the suggestion is withheld: it would pre-fill a junk balance-sheet + // account in the booking dialog. + // Static library templates only reference BAS business accounts plus the + // literal 1930 settlement placeholder, so they never need this check. + // The transaction's OWN settlement ledger is exempt: a transaction still + // stranded on the orphaned row settles there. + let counterLegTopology: CounterLegTopology | null | undefined + const guardLearnedTemplate = async ( + tmpl: CategorizationTemplate, + tx: Transaction, + ): Promise => { + const isCashLedger = (a: string | null | undefined): a is string => !!a && /^19\d{2}$/.test(a) + const accounts = [ + tmpl.debit_account, + tmpl.credit_account, + ...(tmpl.line_pattern ?? []).map((entry) => entry.account), + ].filter(isCashLedger) + if (accounts.length === 0) return tmpl + if (counterLegTopology === undefined) { + counterLegTopology = await loadCounterLegTopology(supabase, companyId) + } + if (!counterLegTopology) return tmpl + const { settlementLedger, twins } = counterLegTopology.contextFor(tx.cash_account_id) + + let guarded = tmpl + if (settlementLedger && accounts.some((a) => twins.has(a))) { + const rewrite = (a: string): string => (twins.has(a) ? settlementLedger : a) + guarded = { + ...tmpl, + debit_account: rewrite(tmpl.debit_account), + credit_account: rewrite(tmpl.credit_account), + line_pattern: tmpl.line_pattern + ? tmpl.line_pattern.map((entry) => ({ ...entry, account: rewrite(entry.account) })) + : tmpl.line_pattern, + } + if (guarded.debit_account === settlementLedger && guarded.credit_account === settlementLedger) { + return null + } + } + + const remaining = [ + guarded.debit_account, + guarded.credit_account, + ...(guarded.line_pattern ?? []).map((entry) => entry.account), + ].filter(isCashLedger) + const orphanHit = remaining.some( + (a) => a !== settlementLedger && counterLegTopology!.orphaned.has(a), + ) + return orphanHit ? null : guarded + } + for (const tx of transactions) { const cpMatch = counterpartyMatches.get(tx.id) if (!cpMatch) continue + const template = await guardLearnedTemplate(cpMatch.template, tx as Transaction) + if (!template) continue - const cpSuggestion = buildCounterpartySuggestion(cpMatch.template, cpMatch.confidence) + const cpSuggestion = buildCounterpartySuggestion(template, cpMatch.confidence) const existing = template_suggestions[tx.id] || [] template_suggestions[tx.id] = [cpSuggestion, ...existing] diff --git a/app/api/v1/companies/[companyId]/transactions/[id]/categorize/__tests__/route.test.ts b/app/api/v1/companies/[companyId]/transactions/[id]/categorize/__tests__/route.test.ts index bd52de9a..afc0838d 100644 --- a/app/api/v1/companies/[companyId]/transactions/[id]/categorize/__tests__/route.test.ts +++ b/app/api/v1/companies/[companyId]/transactions/[id]/categorize/__tests__/route.test.ts @@ -393,3 +393,61 @@ describe('POST /api/v1/.../transactions/{id}/categorize CAS race', () => { expect(inserts['voucher_gap_explanations']).toBeUndefined() }) }) + +describe('POST /api/v1/.../transactions/{id}/categorize orphaned counter-account guard (#1643)', () => { + it('returns TX_CATEGORIZE_ORPHANED_COUNTER_ACCOUNT for an account_override on a revoked-held twin of the live row', async () => { + const { supabase } = makeFlexibleSupabase({ + company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null }, + transactions: { + data: { + id: TX_ID, + company_id: COMPANY_ID, + date: '2026-05-12', + amount: 217.04, + currency: 'SEK', + merchant_name: 'SEB', + cash_account_id: 'ca-live', + journal_entry_id: null, + }, + error: null, + }, + company_settings: { data: { entity_type: 'aktiebolag' }, error: null }, + chart_of_accounts: { + data: { account_number: '1931', account_class: 1, is_active: true }, + error: null, + }, + cash_accounts: [ + // 1: resolveSettlementAccount reads the row's own ledger (1930). + { data: { ledger_account: '1930' }, error: null }, + // 2: the guard's topology scan: 1931 is held by a revoked connection + // and shares the live row's (IBAN, currency): a stale twin. + { + data: [ + { id: 'ca-live', ledger_account: '1930', bank_connection_id: 'conn-live', iban: 'SE111', enabled: true, currency: 'SEK' }, + { id: 'ca-orphan', ledger_account: '1931', bank_connection_id: 'conn-old', iban: 'SE111', enabled: true, currency: 'SEK' }, + ], + error: null, + }, + ], + bank_connections: { + data: [ + { id: 'conn-live', status: 'active' }, + { id: 'conn-old', status: 'revoked' }, + ], + error: null, + }, + }) + mockServiceClient.mockReturnValue(supabase) + + const res = await POST( + makeRequest({ is_business: true, category: 'income_services', account_override: '1931' }), + routeParams(), + ) + + const body = await res.json() + expect(res.status).toBe(400) + expect(body.error.code).toBe('TX_CATEGORIZE_ORPHANED_COUNTER_ACCOUNT') + expect(body.error.details.accountNumber).toBe('1931') + expect(createTxJE).not.toHaveBeenCalled() + }) +}) diff --git a/app/api/v1/companies/[companyId]/transactions/[id]/categorize/route.ts b/app/api/v1/companies/[companyId]/transactions/[id]/categorize/route.ts index 2aa092c8..1ad24802 100644 --- a/app/api/v1/companies/[companyId]/transactions/[id]/categorize/route.ts +++ b/app/api/v1/companies/[companyId]/transactions/[id]/categorize/route.ts @@ -41,6 +41,7 @@ import { createTransactionJournalEntry } from '@/lib/bookkeeping/transaction-ent import { reverseOrphanedJournalEntry } from '@/lib/bookkeeping/cancel-orphaned-entry' import { saveUserMappingRule, applySettlementAccount } from '@/lib/bookkeeping/mapping-engine' import { resolveSettlementAccount } from '@/lib/bookkeeping/settlement-account' +import { guardCounterLegs } from '@/lib/cash-accounts/service' import { AccountsNotInChartError } from '@/lib/bookkeeping/errors' import { collectMappingResultAccounts, findUnresolvableAccounts } from '@/lib/bookkeeping/account-validation' import { propagateUnderlagForBookedTransaction } from '@/lib/transactions/inbox-underlag' @@ -315,6 +316,28 @@ export const POST = withApiV1<{ params: Promise<{ companyId: string; id: string mappingResult.dimensions = body.dimensions } + // Issue #1643 problem 4: same guard as the dashboard route. A learned + // counterparty template or an account_override must never book the + // COUNTER leg onto an orphaned cash-account ledger or a twin ledger of the + // transaction's own bank account; the agent doors have no human looking + // at a running-balance preview, so the refusal has to live here too. + { + const guarded = await guardCounterLegs( + ctx.supabase, + ctx.companyId!, + mappingResult, + settlementAccount, + transaction.cash_account_id, + ) + if (guarded.refusedLedger) { + return v1ErrorResponseFromCode('TX_CATEGORIZE_ORPHANED_COUNTER_ACCOUNT', txLog, { + requestId: ctx.requestId, + details: { accountNumber: guarded.refusedLedger }, + }) + } + mappingResult = guarded.mappingResult + } + if (!mappingResult.debit_account || !mappingResult.credit_account) { return v1ErrorResponseFromCode('TX_CATEGORIZE_INVALID_MAPPING', txLog, { requestId: ctx.requestId, diff --git a/lib/bookkeeping/__tests__/mapping-engine-transfer-guard.test.ts b/lib/bookkeeping/__tests__/mapping-engine-transfer-guard.test.ts new file mode 100644 index 00000000..fc65d99c --- /dev/null +++ b/lib/bookkeeping/__tests__/mapping-engine-transfer-guard.test.ts @@ -0,0 +1,77 @@ +/** + * Issue #1643 problem 4: an own-account "transfer" whose counter leg IS the + * settlement account is not a transfer. It happens when the bank stamps the + * account's own IBAN as counterparty (interest, fees) and a sibling + * cash_accounts row still carries that IBAN. The engine must fall through to + * normal categorization instead of proposing debit == credit on one ledger. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { createQueuedMockSupabase, makeTransaction } from '@/tests/helpers' + +const detectOwnAccountTransferMock = vi.fn() +vi.mock('../own-account-detector', () => ({ + detectOwnAccountTransfer: (...args: unknown[]) => detectOwnAccountTransferMock(...args), +})) + +vi.mock('../booking-templates', () => ({ + findMatchingTemplates: vi.fn().mockReturnValue([]), + buildMappingResultFromTemplate: vi.fn(), +})) + +vi.mock('../counterparty-templates', () => ({ + findCounterpartyTemplate: vi.fn().mockResolvedValue(null), + buildMappingResultFromCounterpartyTemplate: vi.fn(), +})) + +import { evaluateMappingRules } from '../mapping-engine' + +describe('evaluateMappingRules own-account transfer guard', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('books a genuine transfer when the counter differs from the settlement account', async () => { + const { supabase } = createQueuedMockSupabase() + detectOwnAccountTransferMock.mockResolvedValue({ + counterCashAccountId: 'ca-eur', + counterLedgerAccount: '1932', + counterCurrency: 'SEK', + pairTransactionId: null, + }) + + const result = await evaluateMappingRules( + supabase as never, + 'company-1', + makeTransaction({ amount: -1000, currency: 'SEK' }), + undefined, + '1930', + ) + + expect(result.debit_account).toBe('1932') + expect(result.credit_account).toBe('1930') + }) + + it('falls through to normal categorization when the counter equals the settlement account', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + detectOwnAccountTransferMock.mockResolvedValue({ + counterCashAccountId: 'ca-self', + counterLedgerAccount: '1940', + counterCurrency: 'SEK', + pairTransactionId: null, + }) + enqueue({ data: [] }) // mapping_rules lookup: none + + const result = await evaluateMappingRules( + supabase as never, + 'company-1', + makeTransaction({ amount: 217.04, currency: 'SEK' }), + undefined, + '1940', + ) + + // Not a transfer result: neither leg may be the 1940 counter proposal. + // The default fallback books bank vs the uncategorized suspense account. + expect(result.debit_account).toBe('1940') + expect(result.credit_account).not.toBe('1940') + }) +}) diff --git a/lib/bookkeeping/__tests__/own-account-detector.test.ts b/lib/bookkeeping/__tests__/own-account-detector.test.ts index 4a0a4625..edad5037 100644 --- a/lib/bookkeeping/__tests__/own-account-detector.test.ts +++ b/lib/bookkeeping/__tests__/own-account-detector.test.ts @@ -26,6 +26,7 @@ function makeTx(overrides: Partial = {}): Transaction { potential_invoice_id: null, potential_supplier_invoice_id: null, journal_entry_id: null, + cash_account_id: null, mcc_code: null, merchant_name: null, receipt_id: null, @@ -40,6 +41,21 @@ function makeTx(overrides: Partial = {}): Transaction { created_at: '2026-06-12T00:00:00Z', updated_at: '2026-06-12T00:00:00Z', ...overrides, + } as Transaction +} + +function makeCashRow(overrides: Record = {}) { + return { + id: 'ca-eur', + company_id: 'company-1', + bank_connection_id: 'conn-eur', + currency: 'EUR', + ledger_account: '1932', + iban: 'SE9550000000054910000003', + is_primary: false, + enabled: true, + source: 'enable_banking', + ...overrides, } } @@ -56,7 +72,7 @@ describe('detectOwnAccountTransfer', () => { it('returns null when IBAN does not match any cash account for the company', async () => { const { supabase, enqueue } = createQueuedMockSupabase() - enqueue({ data: null }) // findByIban miss + enqueue({ data: [] }) // IBAN candidate lookup miss const result = await detectOwnAccountTransfer( supabase as never, 'company-1', @@ -67,20 +83,8 @@ describe('detectOwnAccountTransfer', () => { it('matches IBAN and returns counter ledger account when present', async () => { const { supabase, enqueue } = createQueuedMockSupabase() - // findByIban hit - enqueue({ - data: { - id: 'ca-eur', - company_id: 'company-1', - bank_connection_id: 'conn-eur', - currency: 'EUR', - ledger_account: '1932', - iban: 'SE9550000000054910000003', - is_primary: false, - enabled: true, - source: 'enable_banking', - }, - }) + enqueue({ data: [makeCashRow()] }) // IBAN candidate lookup hit + enqueue({ data: [{ id: 'conn-eur', status: 'active' }] }) // revoked-connection check // pair candidate lookup: find the matching EUR-side leg enqueue({ data: [{ id: 'tx-eur-leg', amount: 90.50, date: '2026-06-12' }], @@ -100,19 +104,8 @@ describe('detectOwnAccountTransfer', () => { it('returns pairTransactionId: null when the other leg has not been ingested yet', async () => { const { supabase, enqueue } = createQueuedMockSupabase() - enqueue({ - data: { - id: 'ca-eur', - company_id: 'company-1', - bank_connection_id: 'conn-eur', - currency: 'EUR', - ledger_account: '1932', - iban: 'SE9550000000054910000003', - is_primary: false, - enabled: true, - source: 'enable_banking', - }, - }) + enqueue({ data: [makeCashRow()] }) + enqueue({ data: [{ id: 'conn-eur', status: 'active' }] }) enqueue({ data: [] }) // pair not present yet const result = await detectOwnAccountTransfer( @@ -128,18 +121,18 @@ describe('detectOwnAccountTransfer', () => { it('refuses to pair when the counter ledger code is outside the cash class (19xx)', async () => { const { supabase, enqueue } = createQueuedMockSupabase() enqueue({ - data: { - id: 'ca-bad', - company_id: 'company-1', - bank_connection_id: null, - currency: 'SEK', - ledger_account: '6991', // not a cash account - iban: 'SE9550000000054910000003', - is_primary: false, - enabled: true, - source: 'manual', - }, + data: [ + makeCashRow({ + id: 'ca-bad', + bank_connection_id: null, + currency: 'SEK', + ledger_account: '6991', // not a cash account + source: 'manual', + }), + ], }) + // No connection ids on the candidate → the revoked check short-circuits + // without a query. const result = await detectOwnAccountTransfer( supabase as never, 'company-1', @@ -157,4 +150,140 @@ describe('detectOwnAccountTransfer', () => { ) expect(result).toBeNull() }) + + // Issue #1643: a broken reconnect leaves orphaned cash_accounts rows sharing + // the live account's IBAN. Pairing with one proposed the orphaned ledger as + // the booking dialog's counter-account, silently booking revenue onto a junk + // balance-sheet account. + describe('orphaned-row hardening (#1643)', () => { + it('never pairs with the transaction\'s OWN cash account (self-transfer)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + // The bank stamps the account's own IBAN as counterparty (interest); the + // only row carrying it is the transaction's own. + enqueue({ data: [makeCashRow({ id: 'ca-own', ledger_account: '1931', currency: 'SEK' })] }) + enqueue({ data: [{ id: 'conn-eur', status: 'active' }] }) + + const result = await detectOwnAccountTransfer( + supabase as never, + 'company-1', + makeTx({ amount: 217.04, cash_account_id: 'ca-own' }), + ) + expect(result).toBeNull() + }) + + it('never pairs an own-IBAN counterparty with the live twin when the transaction sits on the orphan row', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + // The issue's population: +217,04 interest stranded on the demoted 1931 + // row, the live claim on the same IBAN sits on 1940, and the bank stamps + // the account's own IBAN as counterparty. 1940 is the SAME account. + enqueue({ + data: [ + makeCashRow({ id: 'ca-orphan', bank_connection_id: null, ledger_account: '1931', currency: 'SEK' }), + makeCashRow({ id: 'ca-live', bank_connection_id: 'conn-new', ledger_account: '1940', currency: 'SEK' }), + ], + }) + enqueue({ data: [{ id: 'conn-new', status: 'active' }] }) + + const result = await detectOwnAccountTransfer( + supabase as never, + 'company-1', + makeTx({ amount: 217.04, cash_account_id: 'ca-orphan', currency: 'SEK' }), + ) + expect(result).toBeNull() + }) + + it('never pairs a transaction on the live row with a demoted-manual twin sharing its IBAN', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + makeCashRow({ id: 'ca-live', bank_connection_id: 'conn-new', ledger_account: '1940', currency: 'SEK' }), + makeCashRow({ id: 'ca-orphan', bank_connection_id: null, ledger_account: '1931', currency: 'SEK' }), + ], + }) + enqueue({ data: [{ id: 'conn-new', status: 'active' }] }) + + const result = await detectOwnAccountTransfer( + supabase as never, + 'company-1', + makeTx({ amount: 217.04, cash_account_id: 'ca-live', currency: 'SEK' }), + ) + expect(result).toBeNull() + }) + + it('never pairs an own-IBAN counterparty when two ACTIVE same-currency rows share the IBAN', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + makeCashRow({ id: 'ca-1930', bank_connection_id: 'conn-new', ledger_account: '1930', currency: 'SEK' }), + makeCashRow({ id: 'ca-1931', bank_connection_id: 'conn-new', ledger_account: '1931', currency: 'SEK' }), + ], + }) + enqueue({ data: [{ id: 'conn-new', status: 'active' }] }) + + const result = await detectOwnAccountTransfer( + supabase as never, + 'company-1', + makeTx({ amount: 217.04, cash_account_id: 'ca-1930', currency: 'SEK' }), + ) + expect(result).toBeNull() + }) + + it('still pairs with a REVOKED-held row that has no live twin (a disconnected but real account, #1643 round 3)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [makeCashRow({ id: 'ca-old', bank_connection_id: 'conn-old', ledger_account: '1931', currency: 'SEK' })], + }) + enqueue({ data: [{ id: 'conn-old', status: 'revoked' }] }) + enqueue({ data: [] }) // pair leg not ingested + + const result = await detectOwnAccountTransfer( + supabase as never, + 'company-1', + makeTx({ amount: 217.04 }), + ) + expect(result).not.toBeNull() + expect(result!.counterLedgerAccount).toBe('1931') + }) + + it('never pairs with a disabled row', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: [makeCashRow({ id: 'ca-disabled', enabled: false })] }) + enqueue({ data: [{ id: 'conn-eur', status: 'active' }] }) + + const result = await detectOwnAccountTransfer( + supabase as never, + 'company-1', + makeTx(), + ) + expect(result).toBeNull() + }) + + it('picks the actively connected row when an orphan shares the IBAN', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + // Two rows on one IBAN: the orphan (revoked connection, 1931) and the + // live account (active connection, 1940). The old maybeSingle lookup + // errored on this shape and disabled detection entirely. + enqueue({ + data: [ + makeCashRow({ id: 'ca-orphan', bank_connection_id: 'conn-old', ledger_account: '1931', currency: 'SEK' }), + makeCashRow({ id: 'ca-live', bank_connection_id: 'conn-new', ledger_account: '1940', currency: 'SEK' }), + ], + }) + enqueue({ + data: [ + { id: 'conn-old', status: 'revoked' }, + { id: 'conn-new', status: 'active' }, + ], + }) + enqueue({ data: [] }) // pair leg not ingested + + const result = await detectOwnAccountTransfer( + supabase as never, + 'company-1', + makeTx({ amount: -500 }), + ) + expect(result).not.toBeNull() + expect(result!.counterLedgerAccount).toBe('1940') + }) + }) }) diff --git a/lib/bookkeeping/mapping-engine.ts b/lib/bookkeeping/mapping-engine.ts index 0fe39429..cddc093e 100644 --- a/lib/bookkeeping/mapping-engine.ts +++ b/lib/bookkeeping/mapping-engine.ts @@ -93,7 +93,12 @@ export async function evaluateMappingRules( // priority rules (which would mis-categorize the outflow as an expense). try { const transfer = await detectOwnAccountTransfer(supabase, companyId, transaction) - if (transfer) { + // A "transfer" whose counter leg is the settlement account itself is not a + // transfer (debit == credit on one ledger): it happens when the bank stamps + // the account's own IBAN as counterparty (interest, fees) and a sibling + // cash_accounts row still carries that IBAN (issue #1643). Fall through to + // normal categorization instead of proposing a cash ledger as the counter. + if (transfer && transfer.counterLedgerAccount !== bankAccount) { const isFx = (transaction.currency || '').toUpperCase() !== (transfer.counterCurrency || '').toUpperCase() diff --git a/lib/bookkeeping/own-account-detector.ts b/lib/bookkeeping/own-account-detector.ts index 8c2c6d3d..ca0d5d05 100644 --- a/lib/bookkeeping/own-account-detector.ts +++ b/lib/bookkeeping/own-account-detector.ts @@ -1,6 +1,6 @@ import type { SupabaseClient } from '@supabase/supabase-js' import type { Transaction } from '@/types' -import { findByIban } from '@/lib/cash-accounts/service' +import { findPairableCashAccountByIban } from '@/lib/cash-accounts/service' import { createLogger } from '@/lib/logger' const log = createLogger('own-account-detector') @@ -32,6 +32,12 @@ export interface OwnAccountTransfer { * - the transaction has no counterparty IBAN (manual entries, SIE imports, * older PSD2 rows before counterparty_iban capture) * - the counterparty IBAN doesn't match any cash account for this company + * - the counterparty IBAN is the transaction's OWN IBAN (interest, fees: + * every same-currency row on that IBAN is the same physical account, + * whichever of them is live), the only matches are disabled or orphaned + * rows, or several candidates survive and nothing tells them apart + * (issue #1643: a broken reconnect leaves rows sharing the live account's + * IBAN, and pairing with one proposed a junk ledger as the counter) * * No amount-only heuristic fallback: silent false positives at FX boundaries * would mis-book legitimate external transfers as own-account moves. @@ -44,7 +50,9 @@ export async function detectOwnAccountTransfer( const cpIban = transaction.counterparty_iban?.trim() if (!cpIban) return null - const counterAccount = await findByIban(supabase, companyId, cpIban) + const counterAccount = await findPairableCashAccountByIban(supabase, companyId, cpIban, { + excludeCashAccountId: transaction.cash_account_id ?? null, + }) if (!counterAccount) return null // Defense-in-depth: refuse to route to a non-cash BAS account. cash_accounts diff --git a/lib/cash-accounts/__tests__/orphaned-ledgers.test.ts b/lib/cash-accounts/__tests__/orphaned-ledgers.test.ts new file mode 100644 index 00000000..d6a55b42 --- /dev/null +++ b/lib/cash-accounts/__tests__/orphaned-ledgers.test.ts @@ -0,0 +1,946 @@ +/** + * Issue #1643: helpers that keep orphaned cash_accounts rows (leftovers of a + * broken bank reconnect) out of counter-account proposals, and that let a + * transaction stranded on an orphan match/link against the live ledger of the + * same physical account (identified by IBAN). + */ +import { describe, it, expect } from 'vitest' +import { createQueuedMockSupabase } from '@/tests/helpers' +import { + getOrphanedCounterLedgers, + findOrphanedCounterLedger, + listSiblingCashAccounts, + listSiblingLedgerAccounts, + describeCashAccountSiblings, + findPairableCashAccountByIban, + guardCounterLegs, + guardBookedCounterLines, + loadCounterLegTopology, + shouldRepointToSibling, +} from '../service' + +const IBAN = 'SE4550000000058398257466' + +function row(overrides: Record = {}) { + return { + id: 'ca-1', + ledger_account: '1930', + bank_connection_id: 'conn-live', + external_uid: 'uid-1', + iban: IBAN, + enabled: true, + is_primary: false, + currency: 'SEK', + ...overrides, + } +} + +describe('getOrphanedCounterLedgers', () => { + it('flags a revoked-held twin of an actively connected row', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-live', ledger_account: '1940', bank_connection_id: 'conn-live' }), + row({ id: 'ca-orphan', ledger_account: '1931', bank_connection_id: 'conn-old' }), + ], + }) + enqueue({ + data: [ + { id: 'conn-live', status: 'active' }, + { id: 'conn-old', status: 'revoked' }, + ], + }) + + const orphaned = await getOrphanedCounterLedgers(supabase as never, 'company-1') + expect(orphaned.has('1931')).toBe(true) + expect(orphaned.has('1940')).toBe(false) + }) + + it('does NOT flag a revoked-held row without a live twin (a disconnected but real account, #1643 round 3)', async () => { + // Prod shape: the company's only 1930 on a bank-side revoked connection + // (never demoted), or two distinct real accounts on one revoked + // connection. Both are accounts the user still tracks, not junk. + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-1930', ledger_account: '1930', bank_connection_id: 'conn-old' }), + row({ id: 'ca-1940', ledger_account: '1940', bank_connection_id: 'conn-old', iban: 'SE1112223334445556667778' }), + ], + }) + enqueue({ data: [{ id: 'conn-old', status: 'revoked' }] }) + + const orphaned = await getOrphanedCounterLedgers(supabase as never, 'company-1') + expect(orphaned.size).toBe(0) + }) + + it('flags a demoted-to-manual twin whose IBAN belongs to an actively connected row', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-live', ledger_account: '1940', bank_connection_id: 'conn-live' }), + // Demoted by upsertFromPsd2: connection released, IBAN kept. + row({ id: 'ca-orphan', ledger_account: '1931', bank_connection_id: null }), + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + + const orphaned = await getOrphanedCounterLedgers(supabase as never, 'company-1') + expect(orphaned.has('1931')).toBe(true) + }) + + it('flags an expired-connection twin of an actively connected row', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-live', ledger_account: '1940', bank_connection_id: 'conn-live' }), + row({ id: 'ca-expired', ledger_account: '1931', bank_connection_id: 'conn-expired' }), + ], + }) + enqueue({ + data: [ + { id: 'conn-live', status: 'active' }, + { id: 'conn-expired', status: 'expired' }, + ], + }) + + const orphaned = await getOrphanedCounterLedgers(supabase as never, 'company-1') + expect(orphaned.has('1931')).toBe(true) + expect(orphaned.has('1940')).toBe(false) + }) + + it('does NOT flag a lone expired connection (re-auth window)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: [row({ id: 'ca-expired', ledger_account: '1930', bank_connection_id: 'conn-expired' })] }) + enqueue({ data: [{ id: 'conn-expired', status: 'expired' }] }) + + const orphaned = await getOrphanedCounterLedgers(supabase as never, 'company-1') + expect(orphaned.size).toBe(0) + }) + + it('does NOT flag a manual account without a live IBAN twin', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-live', ledger_account: '1930', bank_connection_id: 'conn-live', iban: IBAN }), + // A genuinely manual account (CSV-imported savings, own IBAN): legit. + row({ id: 'ca-manual', ledger_account: '1940', bank_connection_id: null, iban: 'SE1112223334445556667778' }), + row({ id: 'ca-kassa', ledger_account: '1910', bank_connection_id: null, iban: null }), + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + + const orphaned = await getOrphanedCounterLedgers(supabase as never, 'company-1') + expect(orphaned.size).toBe(0) + }) + + it('returns an empty set on lookup failure', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: null, error: { message: 'boom' } }) + const orphaned = await getOrphanedCounterLedgers(supabase as never, 'company-1') + expect(orphaned.size).toBe(0) + }) + + it('does NOT flag another-currency pocket of a live row that shares its IBAN (Wise/Revolut reconnect)', async () => { + // Two pockets on one IBAN were demoted by a disconnect; the reconnect + // picked only the SEK pocket, so 1935 is live and the GBP pocket 1937 is + // still a manual row carrying the same IBAN. It is a distinct account. + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-sek', ledger_account: '1935', currency: 'SEK', bank_connection_id: 'conn-live' }), + row({ id: 'ca-gbp', ledger_account: '1937', currency: 'GBP', bank_connection_id: null }), + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + + const orphaned = await getOrphanedCounterLedgers(supabase as never, 'company-1') + expect(orphaned.size).toBe(0) + }) +}) + +describe('findOrphanedCounterLedger', () => { + it('returns the first non-settlement 19xx account in the orphaned set', () => { + expect( + findOrphanedCounterLedger(['1940', '1931'], '1940', new Set(['1931'])), + ).toBe('1931') + }) + + it('exempts the settlement account itself', () => { + // A transaction stranded on the orphan still settles there: only the + // COUNTER position is forbidden. + expect( + findOrphanedCounterLedger(['1931', '8311'], '1931', new Set(['1931'])), + ).toBeNull() + }) + + it('ignores non-cash accounts and clean cash accounts', () => { + expect( + findOrphanedCounterLedger(['3001', '2611', '1940'], '1930', new Set(['1931'])), + ).toBeNull() + }) +}) + +describe('listSiblingCashAccounts', () => { + it('returns the same-currency sibling rows with id, ledger, currency and liveness so a link can re-point at one', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-orphan', ledger_account: '1931', bank_connection_id: null }), + row({ id: 'ca-live', ledger_account: '1940', iban: 'SE45 5000 0000 0583 9825 7466', bank_connection_id: 'conn-live' }), + // Same IBAN, other currency: a pocket of a multi-currency account, not a sibling. + row({ id: 'ca-eur', ledger_account: '1932', currency: 'EUR', bank_connection_id: 'conn-live' }), + row({ id: 'ca-other', ledger_account: '1935', iban: 'SE1112223334445556667778' }), + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + + const siblings = await listSiblingCashAccounts(supabase as never, 'company-1', 'ca-orphan') + expect(siblings).toEqual([{ id: 'ca-live', ledger_account: '1940', currency: 'SEK', live: true, released: false }]) + }) + + it('returns [] on a lookup failure', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: null, error: { message: 'boom' } }) + + const siblings = await listSiblingCashAccounts(supabase as never, 'company-1', 'ca-orphan') + expect(siblings).toEqual([]) + }) +}) + +describe('same-connection re-registration twins (#1643, deliberately out of scope)', () => { + it('keeps two enabled rows on ONE active connection sharing IBAN+currency BOTH live: neither is orphaned and a transfer to that IBAN pairs with nothing', async () => { + // The bank re-registered the account under a new external_uid, so two + // enabled rows sit on the same active connection. No liveness signal + // has held up on prod (balance_updated_at, accounts_data uid presence), + // so the rows are not ranked: both stay live until the founder decides + // how to model the shape. + const twins = () => [ + row({ id: 'ca-old', ledger_account: '1930', external_uid: 'uid-old' }), + row({ id: 'ca-new', ledger_account: '1931', external_uid: 'uid-new' }), + ] + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: twins() }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + expect((await getOrphanedCounterLedgers(supabase as never, 'company-1')).size).toBe(0) + + const again = createQueuedMockSupabase() + again.enqueue({ + data: [...twins(), row({ id: 'ca-savings', ledger_account: '1940', external_uid: 'uid-savings', iban: 'SE1112223334445556667778' })], + }) + again.enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + const paired = await findPairableCashAccountByIban(again.supabase as never, 'company-1', IBAN, { + excludeCashAccountId: 'ca-savings', + }) + expect(paired).toBeNull() + }) +}) + +describe('describeCashAccountSiblings', () => { + it('never lists a disabled row as a sibling (not a re-point destination, round 5)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-released', ledger_account: '1931', bank_connection_id: null }), + row({ id: 'ca-disabled', ledger_account: '1930', enabled: false, bank_connection_id: null }), + ], + }) + const described = await describeCashAccountSiblings(supabase as never, 'company-1', 'ca-released') + expect(described?.siblings).toEqual([]) + }) + + it('reports the own row as not live when its connection is revoked', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-orphan', ledger_account: '1931', bank_connection_id: 'conn-old' }), + row({ id: 'ca-live', ledger_account: '1940', bank_connection_id: 'conn-live' }), + ], + }) + enqueue({ + data: [ + { id: 'conn-old', status: 'revoked' }, + { id: 'conn-live', status: 'active' }, + ], + }) + + const described = await describeCashAccountSiblings(supabase as never, 'company-1', 'ca-orphan') + expect(described?.own).toEqual({ id: 'ca-orphan', ledger_account: '1931', currency: 'SEK', live: false, released: true }) + expect(described?.siblings).toEqual([{ id: 'ca-live', ledger_account: '1940', currency: 'SEK', live: true, released: false }]) + }) + + it('reports an expired-connection row as neither live nor released (re-auth renews it in place)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-expired', ledger_account: '1930', bank_connection_id: 'conn-expired' }), + row({ id: 'ca-manual', ledger_account: '1931', bank_connection_id: null }), + ], + }) + enqueue({ data: [{ id: 'conn-expired', status: 'expired' }] }) + + const described = await describeCashAccountSiblings(supabase as never, 'company-1', 'ca-expired') + expect(described?.own).toEqual({ id: 'ca-expired', ledger_account: '1930', currency: 'SEK', live: false, released: false }) + expect(described?.siblings).toEqual([{ id: 'ca-manual', ledger_account: '1931', currency: 'SEK', live: false, released: true }]) + }) +}) + +describe('shouldRepointToSibling', () => { + const sib = (overrides: Partial<{ id: string; live: boolean; released: boolean }> = {}) => ({ + id: 'ca-sib', + ledger_account: '1931', + currency: 'SEK', + live: false, + released: true, + ...overrides, + }) + const own = (overrides: Partial<{ live: boolean; released: boolean }> = {}) => ({ + id: 'ca-own', + ledger_account: '1930', + currency: 'SEK', + live: false, + released: false, + ...overrides, + }) + + it('always moves onto a live sibling', () => { + const sibling = sib({ live: true, released: false }) + expect(shouldRepointToSibling({ own: own({ live: true }), siblings: [sibling] }, sibling)).toBe(true) + }) + + it('moves onto a dead sibling only when the own holder is gone and no sibling is live', () => { + const dead = sib() + expect(shouldRepointToSibling({ own: own({ released: true }), siblings: [dead] }, dead)).toBe(true) + const live = sib({ id: 'ca-live', live: true, released: false }) + expect(shouldRepointToSibling({ own: own({ released: true }), siblings: [dead, live] }, dead)).toBe(false) + }) + + it('never moves an expired/error/pending own row (not live, not released) onto a dead sibling', () => { + const dead = sib() + expect(shouldRepointToSibling({ own: own(), siblings: [dead] }, dead)).toBe(false) + }) + + it('never moves a live own row onto a dead sibling', () => { + const dead = sib() + expect(shouldRepointToSibling({ own: own({ live: true }), siblings: [dead] }, dead)).toBe(false) + }) +}) + +describe('listSiblingLedgerAccounts', () => { + it('returns the other ledgers sharing the row\'s IBAN, normalized', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + // Own row: orphan on 1931 with a spaced IBAN variant. + row({ id: 'ca-orphan', ledger_account: '1931', iban: 'SE45 5000 0000 0583 9825 7466', bank_connection_id: null }), + row({ id: 'ca-live', ledger_account: '1940', bank_connection_id: 'conn-live' }), + row({ id: 'ca-other', ledger_account: '1935', iban: 'SE1112223334445556667778' }), + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + + const siblings = await listSiblingLedgerAccounts(supabase as never, 'company-1', 'ca-orphan') + expect(siblings).toEqual(['1940']) + }) + + it('returns [] for a row without an IBAN', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: [row({ id: 'ca-kassa', ledger_account: '1910', iban: null, bank_connection_id: null })] }) + const siblings = await listSiblingLedgerAccounts(supabase as never, 'company-1', 'ca-kassa') + expect(siblings).toEqual([]) + }) + + it('returns [] when the row cannot be found', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: [] }) + const siblings = await listSiblingLedgerAccounts(supabase as never, 'company-1', 'missing') + expect(siblings).toEqual([]) + }) +}) + +describe('findPairableCashAccountByIban', () => { + it('prefers the actively connected row and drops orphans', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-orphan', ledger_account: '1931', bank_connection_id: 'conn-old' }), + row({ id: 'ca-live', ledger_account: '1940', bank_connection_id: 'conn-new' }), + ], + }) + enqueue({ + data: [ + { id: 'conn-old', status: 'revoked' }, + { id: 'conn-new', status: 'active' }, + ], + }) + + const account = await findPairableCashAccountByIban(supabase as never, 'company-1', IBAN) + expect(account?.id).toBe('ca-live') + }) + + it('drops a demoted-to-manual twin of the live row (same orphan definition as the commit guard)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-orphan', ledger_account: '1931', bank_connection_id: null }), + row({ id: 'ca-live', ledger_account: '1940', bank_connection_id: 'conn-new' }), + ], + }) + enqueue({ data: [{ id: 'conn-new', status: 'active' }] }) + + const account = await findPairableCashAccountByIban(supabase as never, 'company-1', IBAN) + expect(account?.id).toBe('ca-live') + }) + + it('drops an expired-connection twin of the live row', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-expired', ledger_account: '1931', bank_connection_id: 'conn-expired' }), + row({ id: 'ca-live', ledger_account: '1940', bank_connection_id: 'conn-new' }), + ], + }) + enqueue({ + data: [ + { id: 'conn-expired', status: 'expired' }, + { id: 'conn-new', status: 'active' }, + ], + }) + + const account = await findPairableCashAccountByIban(supabase as never, 'company-1', IBAN) + expect(account?.id).toBe('ca-live') + }) + + it('still pairs with a lone expired connection (re-auth window: it is still the real account)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: [row({ id: 'ca-expired', ledger_account: '1940', bank_connection_id: 'conn-expired' })] }) + enqueue({ data: [{ id: 'conn-expired', status: 'expired' }] }) + + const account = await findPairableCashAccountByIban(supabase as never, 'company-1', IBAN) + expect(account?.id).toBe('ca-expired') + }) + + it('still pairs with a revoked-held row that has no live twin (bank switch: closing balance moved into the disconnected account, #1643 round 3)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-new', ledger_account: '1940', bank_connection_id: 'conn-live', iban: 'SE1112223334445556667778' }), + row({ id: 'ca-old', ledger_account: '1930', bank_connection_id: 'conn-old', iban: IBAN }), + ], + }) + enqueue({ + data: [ + { id: 'conn-live', status: 'active' }, + { id: 'conn-old', status: 'revoked' }, + ], + }) + + const found = await findPairableCashAccountByIban(supabase as never, 'company-1', IBAN, { + excludeCashAccountId: 'ca-new', + }) + expect(found?.ledger_account).toBe('1930') + }) + + it('excludes the requested cash account id (self)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: [row({ id: 'ca-own', ledger_account: '1931' })] }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + + const account = await findPairableCashAccountByIban(supabase as never, 'company-1', IBAN, { + excludeCashAccountId: 'ca-own', + }) + expect(account).toBeNull() + }) + + it('returns null for the own IBAN when two ACTIVE same-currency rows share it (the same physical account)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + // Both rows enabled, both on the same active connection: the shape a + // duplicate reconnect row leaves behind. Neither is a transfer target for + // a transaction on the other. + enqueue({ + data: [ + row({ id: 'ca-1930', ledger_account: '1930' }), + row({ id: 'ca-1931', ledger_account: '1931' }), + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + + const account = await findPairableCashAccountByIban(supabase as never, 'company-1', IBAN, { + excludeCashAccountId: 'ca-1930', + }) + expect(account).toBeNull() + }) + + it('returns null for the own IBAN when the twin is the live row and the own row is a demoted orphan', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-orphan', ledger_account: '1931', bank_connection_id: null }), + row({ id: 'ca-live', ledger_account: '1940', bank_connection_id: 'conn-live' }), + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + + const account = await findPairableCashAccountByIban(supabase as never, 'company-1', IBAN, { + excludeCashAccountId: 'ca-orphan', + }) + expect(account).toBeNull() + }) + + it('pairs an own-IBAN counterparty with the single other-currency pocket of a multi-currency account', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-sek', ledger_account: '1935', currency: 'SEK' }), + row({ id: 'ca-gbp', ledger_account: '1937', currency: 'GBP' }), + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + + const account = await findPairableCashAccountByIban(supabase as never, 'company-1', IBAN, { + excludeCashAccountId: 'ca-sek', + }) + expect(account?.id).toBe('ca-gbp') + }) + + it('returns null when several candidates in different currencies survive (no discriminator to pick a pocket)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-sek', ledger_account: '1931', currency: 'SEK' }), + row({ id: 'ca-eur', ledger_account: '1932', currency: 'EUR' }), + row({ id: 'ca-usd', ledger_account: '1933', currency: 'USD' }), + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + + const account = await findPairableCashAccountByIban(supabase as never, 'company-1', IBAN, { + excludeCashAccountId: 'ca-sek', + }) + expect(account).toBeNull() + }) + + it('returns null when only disabled rows carry the IBAN', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: [row({ id: 'ca-disabled', enabled: false })] }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + const account = await findPairableCashAccountByIban(supabase as never, 'company-1', IBAN) + expect(account).toBeNull() + }) +}) + +describe('guardCounterLegs', () => { + const mapping = (debit: string, credit: string) => ({ + rule: null, + debit_account: debit, + credit_account: credit, + risk_level: 'LOW' as const, + confidence: 0.9, + requires_review: false, + default_private: false, + vat_lines: [], + description: 'test', + }) + + it('does not touch the database when no non-settlement 19xx leg is present', async () => { + const { supabase, calls } = createQueuedMockSupabase() + const result = await guardCounterLegs(supabase as never, 'company-1', mapping('5010', '1940'), '1940', 'ca-live') + expect(result.refusedLedger).toBeNull() + expect(calls).toHaveLength(0) + }) + + it('rewrites a learned BANK leg on a twin ledger to the settlement account instead of refusing', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + // Template learned as 5010 / 1931 while the account sat on 1931; the + // transaction now settles on the live 1940 row of the same IBAN. + enqueue({ + data: [ + row({ id: 'ca-live', ledger_account: '1940', bank_connection_id: 'conn-live' }), + row({ id: 'ca-orphan', ledger_account: '1931', bank_connection_id: null }), + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + + const result = await guardCounterLegs(supabase as never, 'company-1', mapping('5010', '1931'), '1940', 'ca-live') + expect(result.refusedLedger).toBeNull() + expect(result.mappingResult.debit_account).toBe('5010') + expect(result.mappingResult.credit_account).toBe('1940') + }) + + it('refuses a twin of the settlement account as counter even when both rows are active', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-1930', ledger_account: '1930' }), + row({ id: 'ca-1931', ledger_account: '1931' }), + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + + // Interest proposed as a "transfer" 1930 / 1931: both legs are the same + // physical account, nothing reaches the P&L. + const result = await guardCounterLegs(supabase as never, 'company-1', mapping('1930', '1931'), '1930', 'ca-1930') + expect(result.refusedLedger).toBe('1931') + }) + + it('refuses a transfer to the live twin from a transaction stranded on the orphan row', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-orphan', ledger_account: '1931', bank_connection_id: null }), + row({ id: 'ca-live', ledger_account: '1940', bank_connection_id: 'conn-live' }), + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + + const result = await guardCounterLegs(supabase as never, 'company-1', mapping('1940', '1931'), '1931', 'ca-orphan') + expect(result.refusedLedger).toBe('1940') + }) + + it('leaves another-currency pocket of the same IBAN alone (a real FX pocket transfer)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-sek', ledger_account: '1935', currency: 'SEK' }), + row({ id: 'ca-gbp', ledger_account: '1937', currency: 'GBP' }), + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + + const result = await guardCounterLegs(supabase as never, 'company-1', mapping('1937', '1935'), '1935', 'ca-sek') + expect(result.refusedLedger).toBeNull() + expect(result.mappingResult.debit_account).toBe('1937') + }) + + it('refuses a revoked-held twin of a live row', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-live', ledger_account: '1940', bank_connection_id: 'conn-live' }), + row({ id: 'ca-orphan', ledger_account: '1931', bank_connection_id: 'conn-old' }), + row({ id: 'ca-savings', ledger_account: '1930', bank_connection_id: 'conn-live', iban: 'SE1112223334445556667778' }), + ], + }) + enqueue({ + data: [ + { id: 'conn-live', status: 'active' }, + { id: 'conn-old', status: 'revoked' }, + ], + }) + + const result = await guardCounterLegs(supabase as never, 'company-1', mapping('1931', '1930'), '1930', 'ca-savings') + expect(result.refusedLedger).toBe('1931') + }) + + it('accepts a revoked-held row without a live twin as counter (two real accounts on one disconnected bank, #1643 round 3)', async () => { + // Company shape from prod: 1930 and 1940 (different IBANs) both on one + // revoked connection; a 1940 -> 1930 transfer must still book. + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-1930', ledger_account: '1930', bank_connection_id: 'conn-old' }), + row({ id: 'ca-1940', ledger_account: '1940', bank_connection_id: 'conn-old', iban: 'SE1112223334445556667778' }), + ], + }) + enqueue({ data: [{ id: 'conn-old', status: 'revoked' }] }) + + const result = await guardCounterLegs(supabase as never, 'company-1', mapping('1930', '1940'), '1940', 'ca-1940') + expect(result.refusedLedger).toBeNull() + expect(result.mappingResult.debit_account).toBe('1930') + }) +}) + +describe('loadCounterLegTopology (suggest-categories batch)', () => { + const fixture = () => [ + row({ id: 'ca-live', ledger_account: '1940', bank_connection_id: 'conn-live', currency: 'SEK' }), + // Demoted same-IBAN same-currency twin: orphaned, and a twin of ca-live. + row({ id: 'ca-twin', ledger_account: '1931', bank_connection_id: null, currency: 'SEK' }), + // Same IBAN, other currency: a real pocket, neither orphaned nor a twin. + row({ id: 'ca-gbp', ledger_account: '1937', bank_connection_id: null, currency: 'GBP' }), + // Revoked-held, no twin: a real disconnected account. + row({ id: 'ca-old', ledger_account: '1930', bank_connection_id: 'conn-old', iban: 'SE1112223334445556667778' }), + ] + const statuses = () => [ + { id: 'conn-live', status: 'active' }, + { id: 'conn-old', status: 'revoked' }, + ] + const mapping = (debit: string, credit: string) => ({ + rule: null, + debit_account: debit, + credit_account: credit, + risk_level: 'LOW' as const, + confidence: 0.9, + requires_review: false, + default_private: false, + vat_lines: [], + description: 'test', + }) + + it('derives the settlement ledger and the same-currency twins of a row', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: fixture() }) + enqueue({ data: statuses() }) + + const topology = await loadCounterLegTopology(supabase as never, 'company-1') + expect(topology).not.toBeNull() + const context = topology!.contextFor('ca-live') + expect(context.settlementLedger).toBe('1940') + expect([...context.twins]).toEqual(['1931']) + }) + + it('gives an other-currency pocket no twins, and null/unknown ids no settlement ledger', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: fixture() }) + enqueue({ data: statuses() }) + + const topology = await loadCounterLegTopology(supabase as never, 'company-1') + expect(topology!.contextFor('ca-gbp')).toEqual({ settlementLedger: '1937', twins: new Set() }) + expect(topology!.contextFor(null)).toEqual({ settlementLedger: null, twins: new Set() }) + expect(topology!.contextFor(undefined)).toEqual({ settlementLedger: null, twins: new Set() }) + expect(topology!.contextFor('ca-unknown')).toEqual({ settlementLedger: null, twins: new Set() }) + }) + + it('caches the per-row context and never re-queries', async () => { + const { supabase, enqueue, findCalls } = createQueuedMockSupabase() + enqueue({ data: fixture() }) + enqueue({ data: statuses() }) + + const topology = await loadCounterLegTopology(supabase as never, 'company-1') + const first = topology!.contextFor('ca-live') + expect(topology!.contextFor('ca-live')).toBe(first) + expect(findCalls('cash_accounts', 'select')).toHaveLength(1) + expect(findCalls('bank_connections', 'select')).toHaveLength(1) + }) + + it('exposes the same orphan set the commit guard refuses on the same fixture', async () => { + const batch = createQueuedMockSupabase() + batch.enqueue({ data: fixture() }) + batch.enqueue({ data: statuses() }) + const topology = await loadCounterLegTopology(batch.supabase as never, 'company-1') + expect([...topology!.orphaned]).toEqual(['1931']) + + // guardCounterLegs on the same fixture: the twin is refused as counter, + // the GBP pocket and the revoked-held 1930 are accepted. + for (const [counter, expected] of [['1931', '1931'], ['1937', null], ['1930', null]] as const) { + const commit = createQueuedMockSupabase() + commit.enqueue({ data: fixture() }) + commit.enqueue({ data: statuses() }) + const result = await guardCounterLegs(commit.supabase as never, 'company-1', mapping(counter, '1940'), '1940', 'ca-live') + expect(result.refusedLedger).toBe(expected) + } + }) + + it('returns null when the row lookup fails (nothing withheld)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: null, error: { message: 'boom' } }) + expect(await loadCounterLegTopology(supabase as never, 'company-1')).toBeNull() + }) +}) + +describe('#1643 round 2: same-IBAN other-currency pocket beside a live pocket', () => { + const mapping = (debit: string, credit: string) => ({ + rule: null, + debit_account: debit, + credit_account: credit, + risk_level: 'LOW' as const, + confidence: 0.9, + requires_review: false, + default_private: false, + vat_lines: [], + description: 'test', + }) + const pockets = () => [ + row({ id: 'ca-sek', ledger_account: '1935', currency: 'SEK', bank_connection_id: 'conn-live' }), + row({ id: 'ca-gbp', ledger_account: '1937', currency: 'GBP', bank_connection_id: null }), + ] + + it('guardCounterLegs accepts the manual GBP pocket as counter of a SEK->GBP conversion', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: pockets() }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + + const result = await guardCounterLegs(supabase as never, 'company-1', mapping('1937', '1935'), '1935', 'ca-sek') + expect(result.refusedLedger).toBeNull() + expect(result.mappingResult.debit_account).toBe('1937') + }) + + it('findPairableCashAccountByIban still proposes the manual GBP pocket', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: pockets() }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + + const found = await findPairableCashAccountByIban(supabase as never, 'company-1', IBAN, { + excludeCashAccountId: 'ca-sek', + }) + expect(found?.ledger_account).toBe('1937') + }) +}) + +describe('guardBookedCounterLines (POST /book)', () => { + it('refuses a 19xx line that is an active twin of the transaction\'s own row', async () => { + // The issue's dialog shape: 1930 and 1931 both enabled on one active + // connection, a learned template pre-fills 1930 debit / 1931 credit. + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-1930', ledger_account: '1930' }), + row({ id: 'ca-1931', ledger_account: '1931' }), + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + + const refused = await guardBookedCounterLines(supabase as never, 'company-1', ['1930', '1931'], 'ca-1930') + expect(refused.refusedLedger).toBe('1931') + }) + + it('refuses a 19xx line on a revoked-held twin of the transaction\'s own row', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-live', ledger_account: '1940' }), + row({ id: 'ca-orphan', ledger_account: '1931', bank_connection_id: 'conn-old' }), + ], + }) + enqueue({ + data: [ + { id: 'conn-live', status: 'active' }, + { id: 'conn-old', status: 'revoked' }, + ], + }) + + const refused = await guardBookedCounterLines(supabase as never, 'company-1', ['1931', '1940'], 'ca-live') + expect(refused.refusedLedger).toBe('1931') + }) + + it('accepts a transfer to a revoked-held account without a live twin (#1643 round 3)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-live', ledger_account: '1940' }), + row({ id: 'ca-old', ledger_account: '1930', bank_connection_id: 'conn-old', iban: 'SE1112223334445556667778' }), + ], + }) + enqueue({ + data: [ + { id: 'conn-live', status: 'active' }, + { id: 'conn-old', status: 'revoked' }, + ], + }) + + expect(await guardBookedCounterLines(supabase as never, 'company-1', ['1930', '1940'], 'ca-live')).toEqual({ refusedLedger: null, repointCashAccountId: null }) + }) + + it('accepts another-currency pocket of the same IBAN and a real transfer to another account', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ + data: [ + row({ id: 'ca-sek', ledger_account: '1935', currency: 'SEK' }), + row({ id: 'ca-gbp', ledger_account: '1937', currency: 'GBP', bank_connection_id: null }), + row({ id: 'ca-savings', ledger_account: '1940', iban: 'SE1112223334445556667778' }), + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + + expect(await guardBookedCounterLines(supabase as never, 'company-1', ['1935', '1937'], 'ca-sek')).toEqual({ refusedLedger: null, repointCashAccountId: null }) + + const again = createQueuedMockSupabase() + again.enqueue({ + data: [ + row({ id: 'ca-sek', ledger_account: '1935', currency: 'SEK' }), + row({ id: 'ca-savings', ledger_account: '1940', iban: 'SE1112223334445556667778' }), + ], + }) + again.enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + expect(await guardBookedCounterLines(again.supabase as never, 'company-1', ['1935', '1940'], 'ca-sek')).toEqual({ refusedLedger: null, repointCashAccountId: null }) + }) + + it('only reads the own row for an ordinary booking whose single bank line is the own ledger', async () => { + const { supabase, enqueue, findCalls } = createQueuedMockSupabase() + enqueue({ data: { ledger_account: '1930' } }) + expect(await guardBookedCounterLines(supabase as never, 'company-1', ['6200', '2640', '1930'], 'ca-1930')).toEqual({ refusedLedger: null, repointCashAccountId: null }) + expect(findCalls('cash_accounts', 'select')).toEqual([['ledger_account']]) + expect(findCalls('bank_connections', 'select')).toHaveLength(0) + }) + + it('does not look anything up when the transaction has no cash_accounts row', async () => { + const { supabase, findCalls } = createQueuedMockSupabase() + expect(await guardBookedCounterLines(supabase as never, 'company-1', ['6200', '2640', '1930'], null)).toEqual({ refusedLedger: null, repointCashAccountId: null }) + expect(findCalls('cash_accounts', 'select')).toHaveLength(0) + }) + + it('re-points a stranded row when its single bank line is the live sibling ledger (#1643 round 4)', async () => { + // The #1643 prod shape: the transaction sits on the demoted 1931, the + // user types the bank leg on the live 1940 (where the money is) against + // a P&L account. The booking posts on 1940 and the row moves there. + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: { ledger_account: '1931' } }) + enqueue({ + data: [ + row({ id: 'ca-orphan', ledger_account: '1931', bank_connection_id: null }), + row({ id: 'ca-live', ledger_account: '1940' }), + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + + expect(await guardBookedCounterLines(supabase as never, 'company-1', ['1940', '8311'], 'ca-orphan')).toEqual({ + refusedLedger: null, + repointCashAccountId: 'ca-live', + }) + }) + + it('refuses a single bank line on a dead twin instead of stranding the bank leg there (#1643 round 5)', async () => { + // Live 1940 row, dialog pre-filled the bank leg on the revoked 1931 twin + // from a template learned before the reconnect (problem 4): posting it + // would put the only bank leg on a ledger no connection feeds while the + // transaction stays on 1940. Refused, as manualLink refuses the same + // voucher. + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: { ledger_account: '1940' } }) + enqueue({ + data: [ + row({ id: 'ca-live', ledger_account: '1940' }), + row({ id: 'ca-orphan', ledger_account: '1931', bank_connection_id: 'conn-old' }), + ], + }) + enqueue({ + data: [ + { id: 'conn-live', status: 'active' }, + { id: 'conn-old', status: 'revoked' }, + ], + }) + expect(await guardBookedCounterLines(supabase as never, 'company-1', ['1931', '8311'], 'ca-live')).toEqual({ + refusedLedger: '1931', + repointCashAccountId: null, + }) + }) + + it('refuses a single bank line on a disabled twin even when the own row is released (never moves onto a deselected row, round 5)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: { ledger_account: '1931' } }) + enqueue({ + data: [ + row({ id: 'ca-released', ledger_account: '1931', bank_connection_id: null }), + row({ id: 'ca-disabled', ledger_account: '1930', enabled: false, bank_connection_id: null }), + ], + }) + expect(await guardBookedCounterLines(supabase as never, 'company-1', ['1930', '8311'], 'ca-released')).toEqual({ + refusedLedger: '1930', + repointCashAccountId: null, + }) + }) + + it('posts as typed when the single bank line is an unrelated ledger (a transfer to another account)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: { ledger_account: '1930' } }) + enqueue({ + data: [ + row({ id: 'ca-1930', ledger_account: '1930' }), + row({ id: 'ca-savings', ledger_account: '1940', iban: 'SE1112223334445556667778' }), + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + expect(await guardBookedCounterLines(supabase as never, 'company-1', ['1940', '8311'], 'ca-1930')).toEqual({ + refusedLedger: null, + repointCashAccountId: null, + }) + }) +}) diff --git a/lib/cash-accounts/__tests__/service.test.ts b/lib/cash-accounts/__tests__/service.test.ts index d09cbefe..68576818 100644 --- a/lib/cash-accounts/__tests__/service.test.ts +++ b/lib/cash-accounts/__tests__/service.test.ts @@ -249,16 +249,44 @@ describe('allocatePsd2LedgerAccount', () => { const [, companyId, userId, mappings] = mockSyncMappedAccounts.mock.calls[0] expect(companyId).toBe('c1') expect(userId).toBe('u1') + // The chart account gets a BAS-style name, never the bank-reported account + // name: ASPSPs report the account HOLDER (the company) as the name, and + // every failed reconnect used to persist another 19xx chart account named + // after the company (issue #1643 problem 3). expect(mappings).toEqual([ expect.objectContaining({ sourceAccount: '1931', targetAccount: '1931', - sourceName: 'Sparkonto', + sourceName: 'Bankkonto SEK', }), ]) }) - it('uses a currency fallback name when the bank account has none', async () => { + it('uses the BAS reference name when the allocated slot is a standard account', async () => { + const supabase = makeSupabase([{ ledger_account: '1930', bank_connection_id: 'conn-1' }]) + // Every free-use slot below 1940 is already assigned earlier in the + // caller's loop, so the allocator lands on 1940 Övriga bankkonton. + const exclude = new Set(['1931', '1935', '1936', '1937', '1938', '1939']) + + const ledger = await allocatePsd2LedgerAccount(supabase, 'c1', 'u1', { + currency: 'SEK', + accountName: 'Arcim AB', + exclude, + }) + + expect(ledger).toBe('1940') + const [, , , mappings] = mockSyncMappedAccounts.mock.calls[0] + expect(mappings).toEqual([ + expect.objectContaining({ + sourceAccount: '1940', + targetAccount: '1940', + sourceName: 'Övriga bankkonton', + targetName: 'Övriga bankkonton', + }), + ]) + }) + + it('names the chart account after the currency regardless of accountName', async () => { const supabase = makeSupabase([]) await allocatePsd2LedgerAccount(supabase, 'c1', 'u1', { currency: 'EUR' }) diff --git a/lib/cash-accounts/service.ts b/lib/cash-accounts/service.ts index efd03443..71a921f1 100644 --- a/lib/cash-accounts/service.ts +++ b/lib/cash-accounts/service.ts @@ -1,7 +1,8 @@ import type { SupabaseClient } from '@supabase/supabase-js' -import type { CashAccount, CashAccountSource } from '@/types' +import type { CashAccount, CashAccountSource, MappingResult } from '@/types' import { createLogger } from '@/lib/logger' import { syncMappedAccounts } from '@/lib/import/account-sync' +import { getBASReference } from '@/lib/bookkeeping/bas-reference' const log = createLogger('cash-accounts') @@ -133,23 +134,588 @@ export async function getPrimary( return null } -export async function findByIban( +/** + * One in-memory picture of the company's cash_accounts rows and the status of + * the bank connections holding them. Every #1643 helper below derives from it, + * so "orphaned", "live" and "same physical account" mean the same thing in the + * transfer detector, the match/link flows and the commit guards. + */ +interface CashAccountTopology { + rows: CashAccount[] + /** bank_connection_id -> bank_connections.status */ + statuses: Map + /** Ledger accounts that must never be PROPOSED or accepted as a counter leg. */ + orphaned: Set + /** A row on an ACTIVE connection: the live claim on that physical account. */ + isLive: (row: CashAccount) => boolean + /** + * A row no connection holds a claim on any more: demoted to manual + * (bank_connection_id null) or still pointing at a REVOKED connection. + * Distinct from "not live": an expired/error/pending connection still + * holds the row and can come back through re-auth. + */ + isReleased: (row: CashAccount) => boolean +} + +function currencyKey(currency: string | null | undefined): string { + return String(currency ?? '').toUpperCase() +} + +/** + * Load the topology, or null when the row lookup fails. A failed connection + * lookup degrades to "no connection is known to be active": nothing is + * flagged orphaned (the conservative pre-fix behavior) and no row ranks as + * live. + */ +async function loadCashAccountTopology( supabase: SupabaseClient, companyId: string, - iban: string, -): Promise { - if (!iban) return null +): Promise { const { data, error } = await supabase .from('cash_accounts') .select('*') .eq('company_id', companyId) - .eq('iban', iban) - .maybeSingle() if (error) { - log.warn('findByIban failed', { companyId, iban, error: error.message }) + log.warn('cash_accounts topology lookup failed', { companyId, error: error.message }) return null } - return (data as CashAccount | null) ?? null + const rows = (data ?? []) as CashAccount[] + const connectionIds = [ + ...new Set(rows.map((r) => r.bank_connection_id).filter((id): id is string => id !== null)), + ] + const statuses = await getConnectionStatuses(supabase, companyId, connectionIds) + + // Two enabled rows on ONE active connection sharing (IBAN, currency) are + // deliberately BOTH live: no liveness signal has held up on prod (see + // DECISIONS.md, #1643), so nothing ranks them here. + const isLive = (r: CashAccount): boolean => + r.enabled && r.bank_connection_id !== null && statuses.get(r.bank_connection_id) === 'active' + const isReleased = (r: CashAccount): boolean => + r.bank_connection_id === null || statuses.get(r.bank_connection_id) === 'revoked' + + const orphaned = new Set() + // Stale IBAN twins of a live row: the live row IS that physical account + // now, so a demoted-to-manual, disabled, revoked-held, or expired/error- + // connection row carrying the same IBAN in the same currency is a leftover + // of a broken reconnect. A row held by a REVOKED connection is NOT orphaned + // on its own: the disconnect and supersede paths demote such rows to + // manual holders (bank_connection_id null, #916), and a row revoked bank- + // side or before that demotion existed is the same thing with the stale + // FK kept, i.e. a real account the user still tracks (commonly the + // company's only 1930). The key is (IBAN, currency), never the IBAN alone: multi- + // currency accounts (Revolut, Wise) copy one IBAN onto every currency + // pocket, and a manual or deselected GBP pocket beside a live SEK pocket is + // a distinct account the user still tracks, not an orphan. + const liveByAccount = new Map() + for (const row of rows) { + const key = physicalAccountKey(row) + if (key && isLive(row)) liveByAccount.set(key, row) + } + for (const row of rows) { + if (isLive(row)) continue + const key = physicalAccountKey(row) + if (!key) continue + const live = liveByAccount.get(key) + if (live && live.ledger_account !== row.ledger_account) orphaned.add(row.ledger_account) + } + + return { rows, statuses, orphaned, isLive, isReleased } +} + +/** + * Identity of the physical bank account a row represents: normalized IBAN + * plus currency, or null for rows without an IBAN (manual, CSV, kassa). + */ +function physicalAccountKey(row: Pick): string | null { + const iban = normalizeIban(row.iban) + return iban ? `${iban}|${currencyKey(row.currency)}` : null +} + +/** + * Ledger accounts of the OTHER rows that represent the same physical account + * as `own` (same normalized IBAN, same currency), whatever their liveness. + * The row on `settlementAccount` is never a twin of itself. + */ +function twinLedgersOf( + topology: CashAccountTopology, + own: CashAccount | null, + settlementAccount: string, +): Set { + const twins = new Set() + const ownKey = own ? physicalAccountKey(own) : null + if (!own || !ownKey) return twins + for (const row of topology.rows) { + if (row.id === own.id || row.ledger_account === settlementAccount) continue + if (physicalAccountKey(row) !== ownKey) continue + twins.add(row.ledger_account) + } + return twins +} + +/** + * Find the cash account an own-account TRANSFER may pair with, by IBAN. + * + * Replaces the old findByIban for this purpose (issue #1643): a broken reconnect + * can leave several rows carrying the same IBAN (the live account plus orphans + * held by a revoked connection, or demoted to manual), and proposing an orphan + * as the transfer's counter-account books real money onto a junk balance-sheet + * ledger. This finder therefore: + * - tolerates multiple rows on one IBAN (the old single-row lookup errored), + * - drops disabled rows and every row in the orphaned set (the same + * definition the commit guards use, so a proposal is never rejected later), + * - treats the transaction's OWN IBAN as "not a transfer": when the bank + * stamps the account's own IBAN as counterparty (interest, fees) every + * row on that IBAN in the same currency is the same physical account, + * whichever of them happens to be live. Only a pocket in ANOTHER currency + * on that IBAN (a multi-currency account exchanging between pockets) can + * still pair. + * When more than one candidate survives (two active twins of one account, or + * several currency pockets with nothing to pick between them) the finder + * returns null rather than guessing by ledger number: no proposal beats a + * wrong one, and that is also what the single-row lookup did before. + */ +export async function findPairableCashAccountByIban( + supabase: SupabaseClient, + companyId: string, + iban: string, + opts: { excludeCashAccountId?: string | null } = {}, +): Promise { + const wanted = normalizeIban(iban) + if (!wanted) return null + const topology = await loadCashAccountTopology(supabase, companyId) + if (!topology) return null + + const onIban = topology.rows.filter((row) => normalizeIban(row.iban) === wanted) + if (onIban.length === 0) return null + + const ownId = opts.excludeCashAccountId ?? null + const own = ownId ? (onIban.find((row) => row.id === ownId) ?? null) : null + + let rows = onIban.filter( + (row) => row.enabled && row.id !== ownId && !topology.orphaned.has(row.ledger_account), + ) + if (own) { + const ownCurrency = currencyKey(own.currency) + rows = rows.filter((row) => currencyKey(row.currency) !== ownCurrency) + } + if (rows.length === 0) return null + if (rows.length > 1) { + log.warn('several pairable cash accounts share the counterparty IBAN: not pairing', { + companyId, + ledgers: rows.map((row) => row.ledger_account), + }) + return null + } + return rows[0] +} + +export interface SiblingCashAccount { + id: string + ledger_account: string + currency: string | null + /** Held by an ACTIVE bank connection and enabled. */ + live: boolean + /** + * No connection holds the row any more (bank_connection_id null or the + * connection is revoked). False for an expired/error/pending connection, + * which can still be renewed onto this row. + */ + released: boolean +} + +export interface CashAccountSiblings { + own: SiblingCashAccount + siblings: SiblingCashAccount[] +} + +/** + * The transaction's own cash_accounts row plus the OTHER rows that carry the + * same (normalized) IBAN in the same currency, i.e. the same physical bank + * account on a different ledger. Multi-currency accounts (Revolut, Wise) copy + * one IBAN onto every currency pocket, so an IBAN match alone would present a + * EUR pocket as a sibling of the SEK pocket; the currency key keeps those + * apart. + * + * A broken reconnect strands transactions on an orphaned row (e.g. 1931) while + * the live claim on the same underlying account sits on another row (e.g. + * 1940). Matching and linking against "the transaction's own ledger" then + * permanently misses vouchers booked on the live ledger; this helper names the + * sibling rows such flows may additionally consider, and lets manualLink + * re-point a stranded row at the live sibling (issue #1643). + * + * Returns null when the row cannot be found or on any lookup failure, and an + * empty sibling list when the row has no IBAN (manual/CSV accounts): + * broadening is an enhancement, never a requirement. + */ +export async function describeCashAccountSiblings( + supabase: SupabaseClient, + companyId: string, + cashAccountId: string, +): Promise { + const topology = await loadCashAccountTopology(supabase, companyId) + if (!topology) return null + const ownRow = topology.rows.find((row) => row.id === cashAccountId) + if (!ownRow) return null + return describeSiblingsFromTopology(topology, ownRow) +} + +function describeSiblingsFromTopology( + topology: CashAccountTopology, + ownRow: CashAccount, +): CashAccountSiblings { + const toSibling = (row: CashAccount): SiblingCashAccount => ({ + id: row.id, + ledger_account: row.ledger_account, + currency: row.currency ?? null, + live: topology.isLive(row), + released: topology.isReleased(row), + }) + const own = toSibling(ownRow) + const wanted = normalizeIban(ownRow.iban) + if (!wanted) return { own, siblings: [] } + + const ownCurrency = currencyKey(ownRow.currency) + const seenLedgers = new Set() + const siblings: SiblingCashAccount[] = [] + for (const row of topology.rows) { + if (row.id === ownRow.id) continue + if (row.ledger_account === ownRow.ledger_account) continue + // A row the user deselected is never a destination (round 5): an + // automatic move must not land on a row the transactions page hides. + // A voucher booked only there is then refused as a cross-account link. + if (!row.enabled) continue + if (normalizeIban(row.iban) !== wanted) continue + if (currencyKey(row.currency) !== ownCurrency) continue + // UNIQUE (company_id, ledger_account) makes this a no-op in practice; + // kept so a duplicate row can never yield two siblings on one ledger. + if (seenLedgers.has(row.ledger_account)) continue + seenLedgers.add(row.ledger_account) + siblings.push(toSibling(row)) + } + return { own, siblings } +} + +/** + * Whether a link against a voucher booked on `sibling` should MOVE the + * transaction's cash_account_id there (manualLink) and, equivalently, whether + * that sibling's vouchers should be offered to the row at all + * (unmatched-entries). The decision is about the destination: move onto a + * live sibling always; onto a dead one only when the own row's holder is + * definitively gone (released) and no sibling is live. An own row on an + * expired/error/pending connection is still the syncing account, so its + * transactions never leave it for a dead twin (issue #1643, round 3). + */ +export function shouldRepointToSibling( + described: CashAccountSiblings, + sibling: SiblingCashAccount, +): boolean { + if (sibling.live) return true + if (!described.own.released) return false + return !described.siblings.some((row) => row.live) +} + +/** + * The sibling rows of `cashAccountId` (see describeCashAccountSiblings), or [] + * when the row has no IBAN or the lookup fails. + */ +export async function listSiblingCashAccounts( + supabase: SupabaseClient, + companyId: string, + cashAccountId: string, +): Promise { + const described = await describeCashAccountSiblings(supabase, companyId, cashAccountId) + return described?.siblings ?? [] +} + +/** + * Ledger accounts of the sibling rows returned by listSiblingCashAccounts. + */ +export async function listSiblingLedgerAccounts( + supabase: SupabaseClient, + companyId: string, + cashAccountId: string, +): Promise { + const siblings = await listSiblingCashAccounts(supabase, companyId, cashAccountId) + return siblings.map((row) => row.ledger_account) +} + +/** + * Ledger accounts that must never be PROPOSED (or accepted) as the + * counter-account of a booking, because their cash_accounts row is orphaned + * (issue #1643): a row that is not live (demoted-to-manual, disabled, held + * by a REVOKED connection, or by an expired/error connection) whose (IBAN, + * currency) also belongs to a row on an ACTIVE connection. The active row IS + * that physical account now, so the stale twin is a leftover of a broken + * reconnect. + * A manual/CSV account without a live twin is NOT orphaned, and neither is a + * row held by a revoked connection without a live twin (a disconnected but + * real account, often the company's only 1930): transfers to such an account + * are legitimate, and so is another currency pocket of a multi-currency + * account that shares the live pocket's IBAN. + */ +export async function getOrphanedCounterLedgers( + supabase: SupabaseClient, + companyId: string, +): Promise> { + const topology = await loadCashAccountTopology(supabase, companyId) + return topology?.orphaned ?? new Set() +} + +/** + * The first account in a mapping result that would book the COUNTER leg onto + * an orphaned cash-account ledger, or null when the result is clean. The + * settlement account itself is exempt: a transaction stranded on an orphaned + * row still books its own bank leg there (the only leg that belongs there). + */ +export function findOrphanedCounterLedger( + accounts: Array, + settlementAccount: string, + orphanedLedgers: ReadonlySet, +): string | null { + for (const account of accounts) { + if (!account || account === settlementAccount) continue + if (!/^19\d{2}$/.test(account)) continue + if (orphanedLedgers.has(account)) return account + } + return null +} + +export interface CounterLegGuardResult { + mappingResult: MappingResult + /** The 19xx ledger the result must not book its counter leg on, or null. */ + refusedLedger: string | null +} + +/** + * Commit-time guard shared by every categorize path (issue #1643 problem 4). + * Runs after applySettlementAccount, on the legs that are NOT the settlement + * account: + * 1. A 19xx leg that is a twin of the settlement row (same IBAN, same + * currency) is that same physical account's bank leg learned on another + * ledger (a counterparty template learned while the account sat on 1931, + * replayed after the reconnect moved it to 1940). It is rewritten to the + * settlement account rather than refused: the business account is fine, + * only the bank side is stale. + * 2. If that leaves the result booking the settlement account against + * itself (a "transfer" between two ledgers of one physical account, e.g. + * interest whose counterparty IBAN is the account's own), the twin is + * refused: no revenue or expense would reach the P&L. + * 3. Any remaining 19xx counter leg in the orphaned set is refused. + * The cash_accounts lookup only runs when a non-settlement 19xx leg is + * present, so ordinary bookings pay nothing. + */ +export async function guardCounterLegs( + supabase: SupabaseClient, + companyId: string, + mappingResult: MappingResult, + settlementAccount: string, + settlementCashAccountId: string | null | undefined, +): Promise { + const isCounterCashLeg = (a: string | null | undefined): a is string => + !!a && a !== settlementAccount && /^19\d{2}$/.test(a) + const legs = [ + mappingResult.debit_account, + mappingResult.credit_account, + ...mappingResult.vat_lines.map((l) => l.account_number), + ].filter(isCounterCashLeg) + if (legs.length === 0) return { mappingResult, refusedLedger: null } + + const topology = await loadCashAccountTopology(supabase, companyId) + if (!topology) return { mappingResult, refusedLedger: null } + + const own = settlementCashAccountId + ? (topology.rows.find((row) => row.id === settlementCashAccountId) ?? null) + : null + const twins = twinLedgersOf(topology, own, settlementAccount) + + let result = mappingResult + const rewrittenTwin = legs.find((leg) => twins.has(leg)) ?? null + if (rewrittenTwin) { + const rewrite = (a: string): string => (twins.has(a) ? settlementAccount : a) + result = { + ...mappingResult, + debit_account: rewrite(mappingResult.debit_account), + credit_account: rewrite(mappingResult.credit_account), + vat_lines: mappingResult.vat_lines.map((l) => ({ + ...l, + account_number: rewrite(l.account_number), + })), + } + if (result.debit_account === settlementAccount && result.credit_account === settlementAccount) { + return { mappingResult, refusedLedger: rewrittenTwin } + } + } + + const remaining = [ + result.debit_account, + result.credit_account, + ...result.vat_lines.map((l) => l.account_number), + ].filter(isCounterCashLeg) + const orphaned = findOrphanedCounterLedger(remaining, settlementAccount, topology.orphaned) + return { mappingResult: result, refusedLedger: orphaned } +} + +export interface CounterLegContext { + /** Ledger of the transaction's own cash_accounts row, or null when unknown. */ + settlementLedger: string | null + /** Other ledgers of the same physical account (same IBAN, same currency). */ + twins: ReadonlySet +} + +export interface CounterLegTopology { + /** Ledgers that must never be PROPOSED or accepted as a counter leg. */ + orphaned: ReadonlySet + contextFor: (cashAccountId: string | null | undefined) => CounterLegContext +} + +/** + * One topology load for a batch of transactions (the suggest-categories + * route), exposing the same twin and orphan rules guardCounterLegs applies at + * commit: a learned 19xx leg that is a twin of the transaction's own row is + * that account's stale BANK leg (rewrite it to the settlement ledger, never + * withhold), and only a true counter-position orphan disqualifies a + * suggestion. Returns null when the lookup fails (nothing is withheld). + */ +export async function loadCounterLegTopology( + supabase: SupabaseClient, + companyId: string, +): Promise { + const topology = await loadCashAccountTopology(supabase, companyId) + if (!topology) return null + const cache = new Map() + return { + orphaned: topology.orphaned, + contextFor: (cashAccountId) => { + if (!cashAccountId) return { settlementLedger: null, twins: new Set() } + const cached = cache.get(cashAccountId) + if (cached) return cached + const own = topology.rows.find((row) => row.id === cashAccountId) ?? null + const context: CounterLegContext = own + ? { settlementLedger: own.ledger_account, twins: twinLedgersOf(topology, own, own.ledger_account) } + : { settlementLedger: null, twins: new Set() } + cache.set(cashAccountId, context) + return context + }, + } +} + +/** + * Line-level counterpart of guardCounterLegs for the free-form booking + * dialog (POST /api/transactions/[id]/book), which submits explicit lines + * instead of a mapping result. Two shapes are covered: + * - Two or more distinct 19xx ledgers, one of them the transaction's own + * settlement ledger: a 19xx line that is a same-IBAN same-currency twin + * of the own row or an orphaned ledger is refused, since such a + * "transfer" books one physical account against itself or onto a junk + * ledger. + * - A single 19xx line that is NOT the own ledger (the user typed the bank + * leg where the money physically is, e.g. the live 1940 for a row + * stranded on the orphaned 1931): when that ledger is a sibling the row + * should move to (shouldRepointToSibling), the caller re-points + * transactions.cash_account_id there in the same locked UPDATE that + * links the voucher, exactly as manualLink does for the identical + * voucher reached through "Matcha mot befintlig verifikation". A twin + * the row may not move to (a dead or disabled twin) is refused, since + * posting would strand the only bank leg on a ledger no connection + * feeds (round 5); a non-twin foreign 19xx line posts as typed. + * An ordinary booking (single 19xx line on the own ledger) pays one PK read + * of the own row and nothing more; the topology is only loaded when a twin + * or foreign 19xx leg is present. Both fields are null when clean, not + * covered, or when the transaction has no cash_accounts row. + */ +export interface BookedLinesGuardResult { + /** The 19xx ledger the booking must not put in the counter position, or null. */ + refusedLedger: string | null + /** cash_accounts row the transaction should be moved to on booking, or null. */ + repointCashAccountId: string | null +} + +const CLEAN_BOOKED_LINES: BookedLinesGuardResult = { refusedLedger: null, repointCashAccountId: null } + +export async function guardBookedCounterLines( + supabase: SupabaseClient, + companyId: string, + accountNumbers: readonly string[], + settlementCashAccountId: string | null | undefined, +): Promise { + const cashLegs = [...new Set(accountNumbers.filter((a) => /^19\d{2}$/.test(a)))] + if (cashLegs.length === 0 || !settlementCashAccountId) return CLEAN_BOOKED_LINES + + if (cashLegs.length === 1) { + const { data: ownRow, error } = await supabase + .from('cash_accounts') + .select('ledger_account') + .eq('id', settlementCashAccountId) + .eq('company_id', companyId) + .maybeSingle() + if (error) { + log.warn('cash_accounts own-row lookup failed', { companyId, error: error.message }) + return CLEAN_BOOKED_LINES + } + const ownLedger = (ownRow as { ledger_account?: string } | null)?.ledger_account ?? null + if (!ownLedger || ownLedger === cashLegs[0]) return CLEAN_BOOKED_LINES + + const topology = await loadCashAccountTopology(supabase, companyId) + const own = topology?.rows.find((row) => row.id === settlementCashAccountId) ?? null + if (!topology || !own) return CLEAN_BOOKED_LINES + // A foreign 19xx line that is NOT a twin of the own row (a transfer to + // another physical account) posts as typed. + if (!twinLedgersOf(topology, own, ownLedger).has(cashLegs[0])) return CLEAN_BOOKED_LINES + const described = describeSiblingsFromTopology(topology, own) + const sibling = described.siblings.find((row) => row.ledger_account === cashLegs[0]) ?? null + if (sibling && shouldRepointToSibling(described, sibling)) { + return { refusedLedger: null, repointCashAccountId: sibling.id } + } + // A twin the row may not move to (a dead or disabled twin of a live or + // still-held row): posting would put the only bank leg on a ledger no + // connection feeds while the transaction stays here (issue #1643 + // problem 4), the shape the categorize paths rewrite and manualLink + // refuses. Refuse it (round 5). + return { refusedLedger: cashLegs[0], repointCashAccountId: null } + } + + const topology = await loadCashAccountTopology(supabase, companyId) + if (!topology) return CLEAN_BOOKED_LINES + const own = topology.rows.find((row) => row.id === settlementCashAccountId) ?? null + if (!own) return CLEAN_BOOKED_LINES + const settlementAccount = own.ledger_account + if (!cashLegs.includes(settlementAccount)) return CLEAN_BOOKED_LINES + + const twins = twinLedgersOf(topology, own, settlementAccount) + const counterLegs = cashLegs.filter((a) => a !== settlementAccount) + const twin = counterLegs.find((a) => twins.has(a)) ?? null + if (twin) return { refusedLedger: twin, repointCashAccountId: null } + return { + refusedLedger: findOrphanedCounterLedger(counterLegs, settlementAccount, topology.orphaned), + repointCashAccountId: null, + } +} + +/** + * bank_connections.status for the given connection ids. Missing ids (and a + * failed lookup, which returns an empty map) read as "status unknown". + */ +async function getConnectionStatuses( + supabase: SupabaseClient, + companyId: string, + connectionIds: readonly string[], +): Promise> { + if (connectionIds.length === 0) return new Map() + + const { data, error } = await supabase + .from('bank_connections') + .select('id, status') + .eq('company_id', companyId) + .in('id', [...connectionIds]) + + if (error) { + log.warn('bank_connections status lookup failed', { companyId, error: error.message }) + return new Map() + } + + return new Map( + ((data ?? []) as Array<{ id: string; status: string }>).map((c) => [c.id, c.status]), + ) } /** @@ -167,24 +733,8 @@ export async function getRevokedConnectionIds( companyId: string, connectionIds: readonly string[], ): Promise> { - if (connectionIds.length === 0) return new Set() - - const { data, error } = await supabase - .from('bank_connections') - .select('id, status') - .eq('company_id', companyId) - .in('id', [...connectionIds]) - - if (error) { - log.warn('getRevokedConnectionIds lookup failed', { companyId, error: error.message }) - return new Set() - } - - return new Set( - ((data ?? []) as Array<{ id: string; status: string }>) - .filter(c => c.status === 'revoked') - .map(c => c.id), - ) + const statuses = await getConnectionStatuses(supabase, companyId, connectionIds) + return new Set([...statuses.entries()].filter(([, status]) => status === 'revoked').map(([id]) => id)) } /** @@ -311,12 +861,22 @@ export async function allocatePsd2LedgerAccount( supabase: SupabaseClient, companyId: string, userId: string, + // accountName is accepted for caller compatibility but no longer names the + // chart account: see the BAS-style naming note in the function body (#1643). input: { currency: string; accountName?: string | null; exclude?: ReadonlySet }, ): Promise { const ledger = await findFreeLedgerAccount(supabase, companyId, input.currency, input.exclude ?? new Set()) if (!ledger) return null - const name = input.accountName?.trim() || `Bankkonto ${input.currency.toUpperCase()}` + // The CHART account always gets a BAS-style name: the BAS reference name + // when the slot is a standard account (1930 Företagskonto, 1940 Övriga + // bankkonton, ...), otherwise "Bankkonto " for a free-use sub-account + // (1931, 1935, ...). ASPSPs report the account holder (i.e. the company) as + // the account name, and every failed reconnect used to persist another 19xx + // chart account named after the company (issue #1643 problem 3). The bank's + // display name still lands on cash_accounts.name via upsertFromPsd2, which + // is what the pickers show; input.accountName is deliberately ignored here. + const name = getBASReference(ledger)?.account_name ?? `Bankkonto ${input.currency.toUpperCase()}` const sync = await syncMappedAccounts( supabase, companyId, diff --git a/lib/errors/structured-errors.ts b/lib/errors/structured-errors.ts index 785aa03f..1c7a1caa 100644 --- a/lib/errors/structured-errors.ts +++ b/lib/errors/structured-errors.ts @@ -455,6 +455,17 @@ const TRANSACTIONS: Record = { message_sv: 'Bokföringsmallen är ogiltig eller passar inte din bolagsform.', message_en: 'The supplied booking template is invalid or does not match the entity type.', }, + TX_CATEGORIZE_ORPHANED_COUNTER_ACCOUNT: { + httpStatus: 400, + message_sv: + 'Motkontot är ett bankkonto som hör till transaktionens eget konto eller till en frånkopplad bankanslutning och kan inte användas. Välj ett intäkts- eller kostnadskonto i stället.', + message_en: + 'The counter-account is a bank ledger of the transaction\'s own account or of a disconnected bank connection and cannot be used. Pick a revenue or expense account instead.', + remediation: { + description: 'Choose a revenue or expense account as the counter-account; a twin or orphaned bank ledger must not receive new postings.', + resource: 'Accounted://chart-of-accounts', + }, + }, TX_CATEGORIZE_INVALID_MAPPING: { httpStatus: 400, message_sv: 'Konteringen saknar debet- eller kreditkonto.', diff --git a/lib/reconciliation/__tests__/bank-reconciliation.test.ts b/lib/reconciliation/__tests__/bank-reconciliation.test.ts index 198f92e3..1747f60e 100644 --- a/lib/reconciliation/__tests__/bank-reconciliation.test.ts +++ b/lib/reconciliation/__tests__/bank-reconciliation.test.ts @@ -872,6 +872,9 @@ describe('manualLink', () => { function createQueueMockSupabase() { const resultQueue: { data: unknown; error: unknown }[] = [] + // Every chained method call, in order, so a test can assert on the + // payload handed to .update(...) (the #1643 re-point cases below). + const calls: Array<{ method: string; args: unknown[] }> = [] const enqueue = (...results: { data?: unknown; error?: unknown }[]) => { for (const r of results) { @@ -886,7 +889,10 @@ describe('manualLink', () => { const next = resultQueue.shift() ?? { data: null, error: null } return (resolve: (v: unknown) => void) => resolve(next) } - return (..._args: unknown[]) => buildChain() + return (...args: unknown[]) => { + calls.push({ method: String(prop), args }) + return buildChain() + } }, } return new Proxy({}, handler) @@ -897,7 +903,10 @@ describe('manualLink', () => { rpc: vi.fn().mockImplementation(() => buildChain()), } - return { supabase, enqueue } + const updatePayloads = () => + calls.filter((c) => c.method === 'update').map((c) => c.args[0] as Record) + + return { supabase, enqueue, updatePayloads } } it('rejects when transaction not found', async () => { @@ -1029,6 +1038,8 @@ describe('manualLink', () => { enqueue({ data: { id: 'je-1', user_id: 'company-1', status: 'posted' } }) // Cross-check: cash account maps to the account being reconciled enqueue({ data: { ledger_account: '1930' } }) + // Sibling-ledger scan (#1643): a row without an IBAN has no siblings. + enqueue({ data: [{ id: 'ca-1930', iban: null, ledger_account: '1930', currency: 'SEK', enabled: true, bank_connection_id: null }] }) // Line exists on 1930 enqueue({ data: [{ debit_amount: 1000, credit_amount: 0, account_number: '1930' }] }) // Update succeeds: .select('id') returns the updated row @@ -1039,6 +1050,296 @@ describe('manualLink', () => { expect(result.success).toBe(true) }) + it('links a transaction stranded on an orphaned row to a voucher on the live sibling ledger and re-points it there (#1643)', async () => { + const { supabase, enqueue, updatePayloads } = createQueueMockSupabase() + const iban = 'SE4550000000058398257466' + // Transaction stamped on the orphaned reconnect row (ledger 1931); the + // verifikat it settles is booked on the live ledger 1940 of the SAME + // physical account (same IBAN). + const tx = makeTransaction({ + id: 'tx-1', + journal_entry_id: null, + cash_account_id: 'ca-orphan', + currency: 'SEK', + }) + + enqueue({ data: tx }) + enqueue({ data: { id: 'je-1', user_id: 'company-1', status: 'posted' } }) + // Cross-check: the transaction's own ledger IS the requested account. + enqueue({ data: { ledger_account: '1931' } }) + // Sibling scan: the own (demoted) row carries the IBAN and the live row + // shares it on 1940. + enqueue({ + data: [ + { id: 'ca-orphan', iban, ledger_account: '1931', currency: 'SEK', enabled: true, bank_connection_id: null }, + { id: 'ca-live', iban, ledger_account: '1940', currency: 'SEK', enabled: true, bank_connection_id: 'conn-live' }, + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + // The voucher's bank leg sits on the sibling ledger, not on 1931. + enqueue({ data: [{ debit_amount: 217.04, credit_amount: 0, account_number: '1940' }] }) + enqueue({ data: [{ id: 'tx-1' }] }) + + const result = await manualLink(supabase as never, 'company-1', 'tx-1', 'je-1', 'user-1', '1931') + + expect(result.success).toBe(true) + // The same locked UPDATE that stamps the link moves the row to the live + // sibling row, so neither account's reconciliation counts a cross-account + // link as an imbalance. + expect(updatePayloads()).toEqual([ + expect.objectContaining({ journal_entry_id: 'je-1', cash_account_id: 'ca-live' }), + ]) + }) + + it('does not re-point when the voucher line is on the transaction\'s own ledger (#1643)', async () => { + const { supabase, enqueue, updatePayloads } = createQueueMockSupabase() + const iban = 'SE4550000000058398257466' + const tx = makeTransaction({ id: 'tx-1', journal_entry_id: null, cash_account_id: 'ca-orphan' }) + + enqueue({ data: tx }) + enqueue({ data: { id: 'je-1', user_id: 'company-1', status: 'posted' } }) + enqueue({ data: { ledger_account: '1931' } }) + enqueue({ + data: [ + { id: 'ca-orphan', iban, ledger_account: '1931', currency: 'SEK', enabled: true, bank_connection_id: null }, + { id: 'ca-live', iban, ledger_account: '1940', currency: 'SEK', enabled: true, bank_connection_id: 'conn-live' }, + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + // Bank leg on 1931 itself: an ordinary same-ledger link. + enqueue({ data: [{ debit_amount: 217.04, credit_amount: 0, account_number: '1931' }] }) + enqueue({ data: [{ id: 'tx-1' }] }) + + const result = await manualLink(supabase as never, 'company-1', 'tx-1', 'je-1', 'user-1', '1931') + + expect(result.success).toBe(true) + expect(updatePayloads()[0]).not.toHaveProperty('cash_account_id') + }) + + it('rejects a voucher whose only bank line is on another-currency pocket of the same IBAN (#1643)', async () => { + const { supabase, enqueue, updatePayloads } = createQueueMockSupabase() + const iban = 'SE4550000000058398257466' + const tx = makeTransaction({ + id: 'tx-1', + journal_entry_id: null, + cash_account_id: 'ca-sek', + currency: 'SEK', + }) + + enqueue({ data: tx }) + enqueue({ data: { id: 'je-1', user_id: 'company-1', status: 'posted' } }) + enqueue({ data: { ledger_account: '1931' } }) + // Multi-currency account: the EUR pocket shares the IBAN but is another + // account, so it is not a sibling and the line check stays on 1931. + enqueue({ + data: [ + { id: 'ca-sek', iban, ledger_account: '1931', currency: 'SEK', enabled: true, bank_connection_id: 'conn-live' }, + { id: 'ca-eur', iban, ledger_account: '1932', currency: 'EUR', enabled: true, bank_connection_id: 'conn-live' }, + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + enqueue({ data: [] }) // no line on 1931 (the voucher only has a 1932 leg) + + const result = await manualLink(supabase as never, 'company-1', 'tx-1', 'je-1', 'user-1', '1931') + + expect(result.success).toBe(false) + expect(result.error).toBe('Verifikationen saknar rad på 1931') + expect(updatePayloads()).toEqual([]) + }) + + it('refuses a voucher that sits only on a sibling that is NOT the live row (#1643 round 4)', async () => { + const { supabase, enqueue, updatePayloads } = createQueueMockSupabase() + const iban = 'SE4550000000058398257466' + // Reverse direction: the transaction synced onto the live 1940 row, the + // voucher was booked on the now-revoked 1931 before the reconnect. The + // voucher is what is wrong; the row must not be parked on the dead row, + // and a cross-account link (money on 1940, voucher on 1931) would show + // as an imbalance on both ledgers, so the link is refused outright. + const tx = makeTransaction({ + id: 'tx-1', + journal_entry_id: null, + cash_account_id: 'ca-live', + currency: 'SEK', + }) + + enqueue({ data: tx }) + enqueue({ data: { id: 'je-1', user_id: 'company-1', status: 'posted' } }) + enqueue({ data: { ledger_account: '1940' } }) + enqueue({ + data: [ + { id: 'ca-live', iban, ledger_account: '1940', currency: 'SEK', enabled: true, bank_connection_id: 'conn-live' }, + { id: 'ca-orphan', iban, ledger_account: '1931', currency: 'SEK', enabled: true, bank_connection_id: 'conn-old' }, + ], + }) + enqueue({ + data: [ + { id: 'conn-live', status: 'active' }, + { id: 'conn-old', status: 'revoked' }, + ], + }) + enqueue({ data: [{ debit_amount: 0, credit_amount: 1000, account_number: '1931' }] }) + + const result = await manualLink(supabase as never, 'company-1', 'tx-1', 'je-1', 'user-1', '1940') + + expect(result.success).toBe(false) + expect(result.error).toBe( + 'Verifikationen är bokförd på 1931, som inte är transaktionens konto (1940). Rätta verifikationen eller flytta transaktionen först.', + ) + expect(updatePayloads()).toEqual([]) + }) + + it('refuses to link an EXPIRED own row to a voucher only on a demoted twin (renewable consent, #1643 rounds 3-4)', async () => { + const { supabase, enqueue, updatePayloads } = createQueueMockSupabase() + const iban = 'SE4550000000058398257466' + // Prod shape: 1930 on an expired connection (still the syncing account, + // re-auth renews it in place) beside a demoted manual twin 1931. Moving + // the row onto 1931 would strand it on the orphan once consent is renewed. + const tx = makeTransaction({ id: 'tx-1', journal_entry_id: null, cash_account_id: 'ca-expired', currency: 'SEK' }) + + enqueue({ data: tx }) + enqueue({ data: { id: 'je-1', user_id: 'company-1', status: 'posted' } }) + enqueue({ data: { ledger_account: '1930' } }) + enqueue({ + data: [ + { id: 'ca-expired', iban, ledger_account: '1930', currency: 'SEK', enabled: true, bank_connection_id: 'conn-expired' }, + { id: 'ca-demoted', iban, ledger_account: '1931', currency: 'SEK', enabled: true, bank_connection_id: null }, + ], + }) + enqueue({ data: [{ id: 'conn-expired', status: 'expired' }] }) + enqueue({ data: [{ debit_amount: 0, credit_amount: 1000, account_number: '1931' }] }) + + const result = await manualLink(supabase as never, 'company-1', 'tx-1', 'je-1', 'user-1', '1930') + + expect(result.success).toBe(false) + expect(result.error).toContain('bokförd på 1931') + expect(updatePayloads()).toEqual([]) + }) + + it('re-points to the LIVE sibling when the voucher touches a dead twin and the live twin, whatever the line order (#1643 round 4)', async () => { + const { supabase, enqueue, updatePayloads } = createQueueMockSupabase() + const iban = 'SE4550000000058398257466' + // An old cross-ledger "transfer" between two rows of one physical + // account: lines on the revoked 1931 and the live 1940, none on the + // demoted 1930 the transaction sits on. The query has no ORDER BY, so + // the dead line comes first here; the destination must still be 1940. + const tx = makeTransaction({ id: 'tx-1', journal_entry_id: null, cash_account_id: 'ca-1930', currency: 'SEK' }) + + enqueue({ data: tx }) + enqueue({ data: { id: 'je-1', user_id: 'company-1', status: 'posted' } }) + enqueue({ data: { ledger_account: '1930' } }) + enqueue({ + data: [ + { id: 'ca-1930', iban, ledger_account: '1930', currency: 'SEK', enabled: true, bank_connection_id: null }, + { id: 'ca-1931', iban, ledger_account: '1931', currency: 'SEK', enabled: true, bank_connection_id: 'conn-old' }, + { id: 'ca-1940', iban, ledger_account: '1940', currency: 'SEK', enabled: true, bank_connection_id: 'conn-live' }, + ], + }) + enqueue({ + data: [ + { id: 'conn-old', status: 'revoked' }, + { id: 'conn-live', status: 'active' }, + ], + }) + enqueue({ + data: [ + { debit_amount: 0, credit_amount: 1000, account_number: '1931' }, + { debit_amount: 1000, credit_amount: 0, account_number: '1940' }, + ], + }) + enqueue({ data: [{ id: 'tx-1' }] }) + + const result = await manualLink(supabase as never, 'company-1', 'tx-1', 'je-1', 'user-1', '1930') + + expect(result.success).toBe(true) + expect(updatePayloads()).toEqual([ + expect.objectContaining({ journal_entry_id: 'je-1', cash_account_id: 'ca-1940' }), + ]) + }) + + it('re-points to the sibling the voucher was booked on when BOTH rows are live (#1643 round 2)', async () => { + const { supabase, enqueue, updatePayloads } = createQueueMockSupabase() + const iban = 'SE4550000000058398257466' + // 1930 and 1931 both enabled on one active connection (the common prod + // shape): the voucher settled on 1931, the transaction sits on 1930. A + // cross-account link would show a difference on both ledgers. + const tx = makeTransaction({ id: 'tx-1', journal_entry_id: null, cash_account_id: 'ca-1930', currency: 'SEK' }) + + enqueue({ data: tx }) + enqueue({ data: { id: 'je-1', user_id: 'company-1', status: 'posted' } }) + enqueue({ data: { ledger_account: '1930' } }) + enqueue({ + data: [ + { id: 'ca-1930', iban, ledger_account: '1930', currency: 'SEK', enabled: true, bank_connection_id: 'conn-live' }, + { id: 'ca-1931', iban, ledger_account: '1931', currency: 'SEK', enabled: true, bank_connection_id: 'conn-live' }, + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + enqueue({ data: [{ debit_amount: 0, credit_amount: 1000, account_number: '1931' }] }) + enqueue({ data: [{ id: 'tx-1' }] }) + + const result = await manualLink(supabase as never, 'company-1', 'tx-1', 'je-1', 'user-1', '1930') + + expect(result.success).toBe(true) + expect(updatePayloads()).toEqual([ + expect.objectContaining({ journal_entry_id: 'je-1', cash_account_id: 'ca-1931' }), + ]) + }) + + it('re-points to the sibling the voucher was booked on when NEITHER row is live (#1643 round 2)', async () => { + const { supabase, enqueue, updatePayloads } = createQueueMockSupabase() + const iban = 'SE4550000000058398257466' + // Full disconnect: both rows demoted to manual, IBAN kept. The voucher is + // the source of truth for where the money was booked. + const tx = makeTransaction({ id: 'tx-1', journal_entry_id: null, cash_account_id: 'ca-1931', currency: 'SEK' }) + + enqueue({ data: tx }) + enqueue({ data: { id: 'je-1', user_id: 'company-1', status: 'posted' } }) + enqueue({ data: { ledger_account: '1931' } }) + enqueue({ + data: [ + { id: 'ca-1931', iban, ledger_account: '1931', currency: 'SEK', enabled: true, bank_connection_id: null }, + { id: 'ca-1940', iban, ledger_account: '1940', currency: 'SEK', enabled: true, bank_connection_id: null }, + ], + }) + // No bank_connection ids: the status lookup is skipped. + enqueue({ data: [{ debit_amount: 217.04, credit_amount: 0, account_number: '1940' }] }) + enqueue({ data: [{ id: 'tx-1' }] }) + + const result = await manualLink(supabase as never, 'company-1', 'tx-1', 'je-1', 'user-1', '1931') + + expect(result.success).toBe(true) + expect(updatePayloads()).toEqual([ + expect.objectContaining({ journal_entry_id: 'je-1', cash_account_id: 'ca-1940' }), + ]) + }) + + it('still rejects a voucher with no line on the account or any sibling ledger', async () => { + const { supabase, enqueue } = createQueueMockSupabase() + const iban = 'SE4550000000058398257466' + const tx = makeTransaction({ + id: 'tx-1', + journal_entry_id: null, + cash_account_id: 'ca-orphan', + }) + + enqueue({ data: tx }) + enqueue({ data: { id: 'je-1', user_id: 'company-1', status: 'posted' } }) + enqueue({ data: { ledger_account: '1931' } }) + enqueue({ + data: [ + { id: 'ca-orphan', iban, ledger_account: '1931', currency: 'SEK', enabled: true, bank_connection_id: null }, + { id: 'ca-live', iban, ledger_account: '1940', currency: 'SEK', enabled: true, bank_connection_id: 'conn-live' }, + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + enqueue({ data: [] }) // no line on 1931 OR 1940 + + const result = await manualLink(supabase as never, 'company-1', 'tx-1', 'je-1', 'user-1', '1931') + + expect(result.success).toBe(false) + expect(result.error).toBe('Verifikationen saknar rad på 1931 eller 1940') + }) + it('allows N:1, does not reject when the verifikat already has a linked transaction', async () => { const { supabase, enqueue } = createQueueMockSupabase() // This transaction is itself unlinked; the TARGET entry already has another diff --git a/lib/reconciliation/bank-reconciliation.ts b/lib/reconciliation/bank-reconciliation.ts index 14f577e4..e7533932 100644 --- a/lib/reconciliation/bank-reconciliation.ts +++ b/lib/reconciliation/bank-reconciliation.ts @@ -10,6 +10,11 @@ import { ledgerLineAmountIn, type LedgerLineAmount, } from '@/lib/bookkeeping/ledger-line-amount' +import { + describeCashAccountSiblings, + shouldRepointToSibling, + type CashAccountSiblings, +} from '@/lib/cash-accounts/service' import { createLogger } from '@/lib/logger' const log = createLogger('reconciliation.bank') @@ -1094,6 +1099,16 @@ export async function manualLink( // reconciled. A transaction bound to 1930 must not be linked against a 1931 // voucher even if the caller passes accountNumber=1931. Legacy rows with no // cash_account_id fall through (the UI list already gates them by currency). + // + // Sibling ledgers of the SAME physical account (rows sharing the IBAN, in + // the same currency) are additionally accepted for the voucher-line check + // below: a transaction stranded on an orphaned reconnect row (e.g. 1931) + // must be linkable to the verifikat booked on the live ledger of that same + // account (e.g. 1940), issue #1643 problem 1. Unrelated accounts, and the + // other currency pockets of a multi-currency account (same IBAN, other + // currency), stay rejected. + let allowedLineAccounts: string[] = [accountNumber] + let siblingInfo: CashAccountSiblings | null = null if (tx.cash_account_id) { const { data: txCa } = await supabase .from('cash_accounts') @@ -1107,19 +1122,76 @@ export async function manualLink( error: `Transaktionen hör till ${txCa.ledger_account}, inte ${accountNumber}`, } } + siblingInfo = await describeCashAccountSiblings(supabase, companyId, tx.cash_account_id) + if (siblingInfo && siblingInfo.siblings.length > 0) { + allowedLineAccounts = [ + ...new Set([accountNumber, ...siblingInfo.siblings.map((row) => row.ledger_account)]), + ] + } } - // Check for a bank account line on the SELECTED settlement account. The old + // Check for a bank account line on the SELECTED settlement account (or a + // sibling ledger of the same physical account, see above). The old // "any 19xx line" check let a 1930 transaction link to a voucher that only // touched 1931: a cross-account link that silently hides a real imbalance. const { data: lines } = await supabase .from('journal_entry_lines') .select('debit_amount, credit_amount, account_number') .eq('journal_entry_id', journalEntryId) - .eq('account_number', accountNumber) + .in('account_number', allowedLineAccounts) if (!lines || lines.length === 0) { - return { success: false, error: `Verifikationen saknar rad på ${accountNumber}` } + return { success: false, error: `Verifikationen saknar rad på ${allowedLineAccounts.join(' eller ')}` } + } + + // When the voucher's bank leg sits on a SIBLING ledger only, the row moves + // to that sibling in the same write that links it: siblings are the same + // physical account in the same currency, and the voucher is the source of + // truth for where the money was booked. A cross-account link would leave + // the money on one ledger while the voucher settles on the other, and the + // account-keyed reconciliation would count it as an imbalance on BOTH + // accounts. Same gate as PATCH /api/transactions/[id]/cash-account: the row + // is unbooked by construction (the locked UPDATE below asserts that). This + // covers the stranded row linking to the live ledger, two live twins of one + // connection, and two demoted rows after a full disconnect. The decision + // is about the DESTINATION: the row moves when the sibling is live, or + // when its own holder is definitively gone (demoted to manual or revoked) + // and no other sibling is live either. A row whose connection is merely + // expired/error/pending is still the syncing account (re-auth renews it in + // place), so a voucher booked ONLY on a dead sibling is REFUSED (round 4): + // the voucher is what is wrong, moving the row would strand it on the + // orphan the moment consent is renewed, and writing the link anyway would + // be the cross-account link the line check above exists to refuse (the + // REST and MCP callers reach this directly, without the unmatched-entries + // filter that hides such vouchers from the dialog). The same rule keeps a + // live row from being parked on a row no connection can sync again. + // A voucher touching several sibling ledgers (an old "transfer" between + // two rows of one physical account) is judged on the best of them, never + // on whichever line the query happened to return first: a live sibling + // wins, else the first sibling the row may move to. + const typedLines = lines as Array<{ account_number: string }> + let repointCashAccountId: string | null = null + if (!typedLines.some((line) => line.account_number === accountNumber)) { + const siblingLedgers = [...new Set(typedLines.map((line) => line.account_number))] + const candidates = siblingLedgers + .map((ledger) => siblingInfo?.siblings.find((row) => row.ledger_account === ledger) ?? null) + .filter((row): row is NonNullable => row !== null) + .filter((row) => siblingInfo !== null && shouldRepointToSibling(siblingInfo, row)) + const destination = candidates.find((row) => row.live) ?? candidates[0] ?? null + if (destination) { + repointCashAccountId = destination.id + } else { + log.warn('manualLink: refused a link to a voucher booked only on a dead sibling ledger', { + companyId, + transactionId, + accountNumber, + siblingLedgers, + }) + return { + success: false, + error: `Verifikationen är bokförd på ${siblingLedgers.join(' och ')}, som inte är transaktionens konto (${accountNumber}). Rätta verifikationen eller flytta transaktionen först.`, + } + } } // N:1 is intentionally allowed: several bank transactions may settle ONE @@ -1148,6 +1220,7 @@ export async function manualLink( journal_entry_id: journalEntryId, reconciliation_method: 'manual' as ReconciliationMethod, is_business: true, + ...(repointCashAccountId ? { cash_account_id: repointCashAccountId } : {}), }) .eq('id', transactionId) .eq('company_id', companyId) diff --git a/lib/transactions/__tests__/categorize-core.orphaned-counter.test.ts b/lib/transactions/__tests__/categorize-core.orphaned-counter.test.ts new file mode 100644 index 00000000..623cd40c --- /dev/null +++ b/lib/transactions/__tests__/categorize-core.orphaned-counter.test.ts @@ -0,0 +1,122 @@ +/** + * Issue #1643 problem 4: the categorize commit path must never book the + * COUNTER leg onto an orphaned cash-account ledger (a 19xx account held by a + * revoked bank connection, or a stale IBAN twin of a live account). Confirming + * such a proposal silently drops revenue/expense from the P&L onto a junk + * balance-sheet account. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { createQueuedMockSupabase } from '@/tests/helpers' +import { eventBus } from '@/lib/events' + +const mockCreateJE = vi.fn() +vi.mock('@/lib/bookkeeping/transaction-entries', () => ({ + createTransactionJournalEntry: (...args: unknown[]) => mockCreateJE(...args), +})) +vi.mock('@/lib/bookkeeping/cancel-orphaned-entry', () => ({ + reverseOrphanedJournalEntry: vi.fn().mockResolvedValue(undefined), +})) +vi.mock('@/lib/transactions/booking-duplicate-detection', () => ({ + detectBookingDuplicate: vi.fn().mockResolvedValue(null), +})) +vi.mock('@/lib/transactions/inbox-underlag', () => ({ + propagateUnderlagForBookedTransaction: vi.fn().mockResolvedValue(undefined), +})) +vi.mock('@/lib/bookkeeping/counterparty-templates', () => ({ + upsertCounterpartyTemplate: vi.fn().mockResolvedValue(undefined), +})) +vi.mock('@/lib/transactions/link-journal-entry', () => ({ + hasLiveJournalEntryLink: vi.fn().mockResolvedValue(false), +})) +vi.mock('@/lib/processing-history/append', () => ({ + appendProcessingHistory: vi.fn().mockResolvedValue(undefined), +})) + +import { categorizeMatchedTransaction } from '../categorize-core' + +const TX_ID = '00000000-0000-4000-8000-0000000000dd' + +const txRow = (over: Record = {}) => ({ + id: TX_ID, + company_id: 'company-1', + date: '2026-07-10', + amount: 217.04, + currency: 'SEK', + amount_sek: 217.04, + exchange_rate: 1, + description: 'Insättningsränta', + merchant_name: null, + cash_account_id: null, + document_id: null, + journal_entry_id: null, + ...over, +}) + +const settingsRow = { entity_type: 'aktiebolag', fiscal_year_start_month: 1 } + +beforeEach(() => { + vi.clearAllMocks() + eventBus.clear() + mockCreateJE.mockResolvedValue({ id: 'je-1' }) +}) + +describe('categorizeMatchedTransaction: orphaned counter-account guard', () => { + it('refuses to book when the override counter is a revoked-held twin of a live row', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: txRow() }) // transactions select + enqueue({ data: settingsRow }) // company_settings + enqueue({ data: [] }) // resolveSettlementAccount: no enabled cash accounts -> 1930 + // Override chart hit: 1931 exists and is active in the chart (the orphaned + // ledger was auto-created there by the broken reconnect). + enqueue({ data: { account_number: '1931', account_class: 1, is_active: true } }) + // Guard: cash_accounts scan finds 1931 held by a revoked connection AND + // sharing its (IBAN, currency) with the live 1940 row: a stale twin. + enqueue({ + data: [ + { id: 'ca-live', ledger_account: '1940', bank_connection_id: 'conn-live', iban: 'SE455', enabled: true, currency: 'SEK' }, + { id: 'ca-orphan', ledger_account: '1931', bank_connection_id: 'conn-old', iban: 'SE455', enabled: true, currency: 'SEK' }, + ], + }) + enqueue({ + data: [ + { id: 'conn-live', status: 'active' }, + { id: 'conn-old', status: 'revoked' }, + ], + }) + + const result = await categorizeMatchedTransaction( + supabase as never, 'user-1', 'company-1', TX_ID, + { category: 'income_services', vatTreatment: 'exempt', accountOverride: '1931' }, + ) + + expect(result.status).toBe(400) + expect(result.error).toMatch(/1931/) + expect(result.error).toMatch(/frånkopplad/) + expect(mockCreateJE).not.toHaveBeenCalled() + }) + + it('still books when the 19xx counter is a live cash account (genuine transfer target)', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: txRow({ amount: -500, amount_sek: -500 }) }) + enqueue({ data: settingsRow }) + enqueue({ data: [] }) // resolveSettlementAccount fallback -> 1930 + enqueue({ data: { account_number: '1940', account_class: 1, is_active: true } }) // override chart hit + // Guard: 1940 is held by an ACTIVE connection -> not orphaned. + enqueue({ + data: [ + { id: 'ca-live', ledger_account: '1940', bank_connection_id: 'conn-live', iban: 'SE455', enabled: true }, + ], + }) + enqueue({ data: [{ id: 'conn-live', status: 'active' }] }) + enqueue({ data: [{ id: 'fp-1' }] }) // ensureFiscalPeriod + enqueue({ data: [txRow({ journal_entry_id: 'je-1' })] }) // transactions update + + const result = await categorizeMatchedTransaction( + supabase as never, 'user-1', 'company-1', TX_ID, + { category: 'expense_other', vatTreatment: 'exempt', accountOverride: '1940' }, + ) + + expect(result.error).toBeUndefined() + expect(mockCreateJE).toHaveBeenCalled() + }) +}) diff --git a/lib/transactions/categorize-core.ts b/lib/transactions/categorize-core.ts index 77b5d103..19a77602 100644 --- a/lib/transactions/categorize-core.ts +++ b/lib/transactions/categorize-core.ts @@ -30,6 +30,7 @@ import { buildMappingResultFromCategory } from '@/lib/bookkeeping/category-mappi import { applyAccountOverride } from '@/lib/bookkeeping/account-override' import { applySettlementAccount } from '@/lib/bookkeeping/mapping-engine' import { resolveSettlementAccount } from '@/lib/bookkeeping/settlement-account' +import { guardCounterLegs } from '@/lib/cash-accounts/service' import { createTransactionJournalEntry } from '@/lib/bookkeeping/transaction-entries' import { reverseOrphanedJournalEntry } from '@/lib/bookkeeping/cancel-orphaned-entry' import { getEarliestFiscalPeriodStart } from '@/lib/core/bookkeeping/period-service' @@ -404,6 +405,29 @@ export async function categorizeMatchedTransaction( return { error: `No account mapping for category "${category}" with entity type "${entityType}".`, status: 400 } } + // Issue #1643 problem 4: never book the COUNTER leg onto an orphaned + // cash-account ledger or a twin ledger of the transaction's own bank + // account. An account_override or learned mapping pointing there would + // silently drop revenue/expense from the P&L onto a junk balance-sheet + // account. A twin that is merely the stale BANK leg is rewritten to the + // settlement account instead (see guardCounterLegs). + { + const guarded = await guardCounterLegs( + supabase, + companyId, + mappingResult, + settlementAccount, + transaction.cash_account_id, + ) + if (guarded.refusedLedger) { + return { + error: `Motkontot ${guarded.refusedLedger} är ett bankkonto som hör till transaktionens eget konto eller till en frånkopplad bankanslutning och kan inte användas. Välj ett intäkts- eller kostnadskonto i stället.`, + status: 400, + } + } + mappingResult = guarded.mappingResult + } + await ensureFiscalPeriod(supabase, userId, companyId, transaction.date, fiscalYearStartMonth) let journalEntryId: string | null = null