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
  <CUR>") instead of the ASPSP-reported holder name.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
2026-08-29 00:26:47 +02:00
committed by GitHub
co-authored by Claude Fable 5 Jakob Wennberg
parent 1a8fe36bc4
commit 4e1eb3d662
23 changed files with 3270 additions and 83 deletions
+2
View File
@@ -1323,6 +1323,8 @@ One line per decision: `[YYYY-MM-DD] <decision>: <why>`. 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 <CUR>"), 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.