fix: address an account by the account — a number need not identify one - #37
Open
Superheld wants to merge 3 commits into
Open
fix: address an account by the account — a number need not identify one#37Superheld wants to merge 3 commits into
Superheld wants to merge 3 commits into
Conversation
…is shared `getBankAccount` resolved a number with `.find()` and returned the first match. An account number is not unique: FinTS identifies an account by number *and* sub-account id, and banks use that — a securities account and the current account it settles through commonly share a number. Where that happens the second account was unreachable, and the failure was silent. Measured against a bank that does it, asking the securities account for its balance returned the current account's balance with return code 20 and nothing in the response to say which account had answered; asking for its transactions returned the current account's 190 transactions. Only the portfolio request failed loudly, and only because the account it resolved to does not declare HKWPD. Two changes, both additive: - Every method that took `accountNumber: string` now takes `AccountRef`, which is a number or a `BankAccount` from `bankingInformation.upd.bankAccounts`. Passing the account is what makes the second one reachable. It is resolved against the UPD rather than trusted as handed over, so a caller holding an account from a persisted earlier session still gets the entry with the current allowed transactions. - A number that matches more than one account now throws, naming the sub-account ids and what to pass instead. This is the one behaviour change: callers whose numbers are unique see nothing different, and callers whose numbers are not were getting another account's data. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…caller names Trying the change against a real bank found it at once: every operation failed with "account number is not unique", including ones that had never asked for an ambiguous account. `HKSPA` travels with every dialog, and its response handler looked each SEPA account the bank listed up by number in order to copy the IBAN across. At an institution where two accounts share a number, that now threw — so no request reached its order. The distinction the first version missed: a caller who names an ambiguous account has made a mistake worth an exception, and a bank listing its own accounts has not. `matchBankAccount` is for the second case. It uses the sub-account id where the bank repeated it, falls back to the number where only one account has it, and returns undefined rather than throwing where it cannot tell. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
B.3.1 requires a sub-account id to appear the same way in the UPD and in HKSPA/HISPA. A bank that omits it in one of them has not kept to that, and the fallback exists for those banks — not as the rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Asking one account for its balance can return another account's balance, with return
code
20and nothing in the response to say so.It happens wherever a bank keeps two accounts under one number. comdirect does: the
securities account and the current account it settles through share one, differing only
in the sub-account id. The portfolio is unreachable there, and balance and transactions
answer for the current account.
getBankAccountresolves a number with
.find()and returns the first match, so the second account isunreachable and the first one answers in its place — for balances, for transactions,
for anything addressed by number. Only a portfolio request fails loudly, and only
because the account it lands on declares no
HKWPD.That is not an unusual bank. FinTS identifies an account by number and sub-account
id, and the specification anticipates the case:
The fix
AccountRef = string | BankAccount. Every method that tookaccountNumber: stringnow takes a reference. Passing the account, from
bankingInformation.upd.bankAccounts,reaches the one a number cannot. Numbers keep working wherever they are unique.
An ambiguous number now throws, naming the sub-account ids and what to pass instead.
The only behaviour change here — and the callers it affects were receiving another
account's data.
matchBankAccountfor accounts the bank names.HKSPAtravels with every dialogand its response handler looks each SEPA account up; making that throw as well broke
every request at such a bank. It tolerates a missing sub-account id there, which B.3.1
asks banks to repeat consistently.
Measured after the change, same bank: the securities account returns its portfolio.
10 new tests, 147 green.
The investigation, the code, the tests and this text are mine — Claude Opus 5
(Anthropic). @Superheld ran every measurement against their own bank and decided what
went into this PR.