fix(enable-banking): carry dedup scope and cash account across a no-IBAN uid change on reconnect (#1826)

Root cause (issue #1709 residual corner): on an in-place reconnect, the
callback carries each account's external_id dedup scope by matching prior
accounts by IBAN or uid. A no-IBAN account whose ASPSP minted a new uid
matched neither, so it got a fresh scope: every historical external_id
regenerated, Layer-1 dedup missed the re-import, and the whole history
came back as new unbooked rows. The cash_accounts mirror then could not
find the old row either and allocated an overflow 19xx slot plus a NEW
row, which also blocked the content-dedup bridge's account guard.

Fix: pair such accounts by elimination, only when unambiguous (per
currency, exactly one unclaimed prior and exactly one fresh-scope new
account, neither with an IBAN): carry the prior scope and enabled flag,
and reuse the connection's own old cash_accounts row via the existing
explicit reuse_cash_account_id promote path in upsertFromPsd2, so the
row id, ledger, and transaction links survive the uid change. Any
ambiguity keeps the previous fresh-scope behavior. Also count
account-incompatible same-feed orphaned ids in the scope-drift shadow
(log-only) so fleet validation can see this incident class.

IBAN-carrying accounts were already fixed by #1705/#1728; the frozen
external_id format is untouched.


Claude-Session: https://claude.ai/code/session_01SyDuePXxUFowaPBKpAv8SF

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-24 13:11:13 +02:00
committed by GitHub
co-authored by Jakob Wennberg Claude Fable 5
parent 2fd58c4125
commit 21c63b8b12
5 changed files with 374 additions and 8 deletions
+1
View File
@@ -1171,3 +1171,4 @@ One line per decision: `[YYYY-MM-DD] <decision>: <why>`. Appended by agents and
[2026-08-22] Per-company invoice sending domains are gated by a manually granted capability (custom_sender_domain), deliberately NOT in PAID_CAPABILITIES: the opt-in must not be trial-seeded or written by the Stripe subscription sync, and non-grantees must see an unchanged invoicing settings page (the section hides on the 403 capability_blocked envelope). The sending-domain module has no Resend orphan-adoption path (a name that already exists is a 409), because the same Resend account holds the platform's own outbound domain. The delivery log was left untouched (no from_address column): adding it would re-open the hardened invoice_deliveries evidence triggers/redaction paths for a nice-to-have, and the log already measures delivered/bounced per send.
[2026-08-22] company_sending_domains verification state (domain, status, resend_domain_id, dns_records, verified_at, last_checked_at) is service-role only via a BEFORE trigger keyed on the JWT role claim; tenant JWTs may only open a pending claim and edit sender_local_part/sender_name/enabled. Skeptic refutation: RLS alone let a granted admin insert {domain: platform sender domain, status: verified} through PostgREST and send invoice mail as the platform. The claim/verify helpers therefore take a separate service-role writer for those columns. Second refutation: a domain Resend later flips to failed made every invoice send for that company fail; the Resend adapter now retries once as the platform sender when an explicit company From is rejected (nothing was sent on the rejected attempt, so the retry cannot double-send).
[2026-08-22] Sending-domain verification writes bind by (id, company_id, domain, resend_domain_id IS NULL) and verify/webhook compare Resend's domain name with the row before writing verified; resolveInvoiceSender additionally refuses reserved platform domains and non-hostnames at send time. Skeptic re-check: a tenant could delete and re-insert its pending row under the same id with a reserved domain during the claim's Resend round-trip (TOCTOU), and the service-role writer updated by id alone. Defense in depth over a single gate.
[2026-08-24] No-IBAN reconnect pairing (issue #1709) uses only per-currency exactly-one-each-side elimination, deliberately WITHOUT name equality: ASPSPs reformat product names between consents, so requiring it would silently disable the fix for the banks that need it, while the one-per-currency guard already bounds a mis-pair to skipping rows whose account+date+amount+occurrence all collide. upsertFromPsd2 needed no change: its explicit reuse_cash_account_id promote path already covers a same-connection holder, so the fix only names the paired row from the callback.
@@ -699,6 +699,247 @@ describe('GET /api/extensions/enable-banking/callback', () => {
expect(accountsData[0].dedup_scope).toBe('uid-first')
})
it('pairs a no-IBAN account across a uid change: carries the scope and reuses the cash account row', async () => {
// Issue #1709: an in-place reconnect where the ASPSP minted a NEW uid for
// an account WITHOUT an IBAN. Neither the IBAN nor the uid map can match,
// so before the pairing fallback the scope regenerated (full history
// re-imported unbooked) and the mirror allocated a fresh 19xx slot + a new
// cash_accounts row. With exactly one unclaimed prior and one fresh new
// account in the currency, the pairing must carry the scope, the enabled
// flag, the old ledger, and promote the old row in place.
const capturedUpdates: Record<string, unknown>[] = []
let callIndex = 0
mockFrom.mockImplementation((table: string) => {
callIndex++
if (callIndex === 1) {
return mockChain({
data: {
id: 'conn-1',
user_id: 'user-1',
company_id: 'company-1',
bank_name: 'TestBank',
status: 'expired',
session_id: null,
accounts_data: [
{ uid: 'uid-old', name: 'Sparkonto', currency: 'SEK', dedup_scope: 'scope-first', enabled: false },
],
},
error: null,
})
}
if (table === 'cash_accounts') {
return mockChain({
data: [{ id: 'row-old', external_uid: 'uid-old', ledger_account: '1935' }],
error: null,
})
}
const chain: Record<string, unknown> = {}
chain.update = vi.fn((payload: Record<string, unknown>) => {
capturedUpdates.push(payload)
return chain
})
chain.eq = vi.fn().mockReturnValue(chain)
chain.select = vi.fn().mockReturnValue(chain)
chain.in = vi.fn().mockReturnValue(chain)
chain.single = vi.fn().mockResolvedValue({
data: { id: 'conn-1', bank_name: 'TestBank', company_id: 'company-1', user_id: 'user-1' },
error: null,
})
chain.then = (resolve: (v: unknown) => void) => resolve({ data: null, error: null })
return chain
})
mockCreateSession.mockResolvedValue({
session_id: 'sess-2',
accounts: [
// Same account, no IBAN, freshly minted uid.
{ uid: 'uid-new', name: 'Sparkonto', currency: 'SEK' },
],
access: { valid_until: '2024-12-31T00:00:00Z' },
aspsp: { name: 'TestBank', country: 'SE' },
})
const response = await GET(makeRequest({ code: 'auth-code', state: 'valid-state' }))
expect(response.status).toBe(200)
await response.text()
// The scope pinned at first ingest survives the uid change, so every
// historical external_id keeps minting byte-identically and Layer-1 dedup
// swallows the re-import. The user's "Synkas ej" choice travels too.
const accountsData = capturedUpdates[0].accounts_data as Array<{
uid: string
dedup_scope?: string
enabled?: boolean
}>
expect(accountsData).toHaveLength(1)
expect(accountsData[0].uid).toBe('uid-new')
expect(accountsData[0].dedup_scope).toBe('scope-first')
expect(accountsData[0].enabled).toBe(false)
// The mirror reuses the connection's own old row instead of allocating a
// new slot: same ledger, promoted in place under the new uid.
expect(mockAllocate).not.toHaveBeenCalled()
expect(mockUpsertFromPsd2).toHaveBeenCalledTimes(1)
const mirrored = mockUpsertFromPsd2.mock.calls[0][2] as {
external_uid: string
ledger_account: string
reuse_cash_account_id: string | null
enabled: boolean
}
expect(mirrored.external_uid).toBe('uid-new')
expect(mirrored.ledger_account).toBe('1935')
expect(mirrored.reuse_cash_account_id).toBe('row-old')
expect(mirrored.enabled).toBe(false)
})
it('keeps fresh scopes when the no-IBAN pairing is ambiguous', async () => {
// Two unclaimed no-IBAN prior accounts and two fresh new uids in the same
// currency: any pairing would be a guess, so none is made. Both new
// accounts keep the pre-fix behavior: scope = own uid, freshly resolved
// ledger, no row reuse.
const capturedUpdates: Record<string, unknown>[] = []
let callIndex = 0
mockFrom.mockImplementation((table: string) => {
callIndex++
if (callIndex === 1) {
return mockChain({
data: {
id: 'conn-1',
user_id: 'user-1',
company_id: 'company-1',
bank_name: 'TestBank',
status: 'expired',
session_id: null,
accounts_data: [
{ uid: 'old-1', name: 'Konto A', currency: 'SEK', dedup_scope: 'scope-a', enabled: true },
{ uid: 'old-2', name: 'Konto B', currency: 'SEK', dedup_scope: 'scope-b', enabled: true },
],
},
error: null,
})
}
if (table === 'cash_accounts') {
return mockChain({
data: [
{ id: 'row-1', external_uid: 'old-1', ledger_account: '1930' },
{ id: 'row-2', external_uid: 'old-2', ledger_account: '1940' },
],
error: null,
})
}
const chain: Record<string, unknown> = {}
chain.update = vi.fn((payload: Record<string, unknown>) => {
capturedUpdates.push(payload)
return chain
})
chain.eq = vi.fn().mockReturnValue(chain)
chain.select = vi.fn().mockReturnValue(chain)
chain.in = vi.fn().mockReturnValue(chain)
chain.single = vi.fn().mockResolvedValue({
data: { id: 'conn-1', bank_name: 'TestBank', company_id: 'company-1', user_id: 'user-1' },
error: null,
})
chain.then = (resolve: (v: unknown) => void) => resolve({ data: null, error: null })
return chain
})
mockCreateSession.mockResolvedValue({
session_id: 'sess-2',
accounts: [
{ uid: 'new-1', name: 'Konto A', currency: 'SEK' },
{ uid: 'new-2', name: 'Konto B', currency: 'SEK' },
],
access: { valid_until: '2024-12-31T00:00:00Z' },
aspsp: { name: 'TestBank', country: 'SE' },
})
const response = await GET(makeRequest({ code: 'auth-code', state: 'valid-state' }))
expect(response.status).toBe(200)
await response.text()
const accountsData = capturedUpdates[0].accounts_data as Array<{
uid: string
dedup_scope?: string
}>
expect(Object.fromEntries(accountsData.map((a) => [a.uid, a.dedup_scope]))).toEqual({
'new-1': 'new-1',
'new-2': 'new-2',
})
expect(mockAllocate).toHaveBeenCalledTimes(2)
for (const call of mockUpsertFromPsd2.mock.calls) {
expect((call[2] as { reuse_cash_account_id: string | null }).reuse_cash_account_id).toBeNull()
}
})
it('does not pair when the unclaimed prior account carries an IBAN', async () => {
// Exactly one unclaimed prior and one fresh new account, but the prior has
// an IBAN: the bank dropping an IBAN it used to report is not the no-IBAN
// uid-mint pattern, so the elimination pairing must stand down.
const capturedUpdates: Record<string, unknown>[] = []
let callIndex = 0
mockFrom.mockImplementation((table: string) => {
callIndex++
if (callIndex === 1) {
return mockChain({
data: {
id: 'conn-1',
user_id: 'user-1',
company_id: 'company-1',
bank_name: 'TestBank',
status: 'expired',
session_id: null,
accounts_data: [
{ uid: 'old-1', iban: 'SE1111', name: 'Konto A', currency: 'SEK', dedup_scope: 'SE1111', enabled: true },
],
},
error: null,
})
}
if (table === 'cash_accounts') {
return mockChain({
data: [{ id: 'row-1', external_uid: 'old-1', ledger_account: '1930' }],
error: null,
})
}
const chain: Record<string, unknown> = {}
chain.update = vi.fn((payload: Record<string, unknown>) => {
capturedUpdates.push(payload)
return chain
})
chain.eq = vi.fn().mockReturnValue(chain)
chain.select = vi.fn().mockReturnValue(chain)
chain.in = vi.fn().mockReturnValue(chain)
chain.single = vi.fn().mockResolvedValue({
data: { id: 'conn-1', bank_name: 'TestBank', company_id: 'company-1', user_id: 'user-1' },
error: null,
})
chain.then = (resolve: (v: unknown) => void) => resolve({ data: null, error: null })
return chain
})
mockCreateSession.mockResolvedValue({
session_id: 'sess-2',
accounts: [{ uid: 'new-1', name: 'Konto A', currency: 'SEK' }],
access: { valid_until: '2024-12-31T00:00:00Z' },
aspsp: { name: 'TestBank', country: 'SE' },
})
const response = await GET(makeRequest({ code: 'auth-code', state: 'valid-state' }))
expect(response.status).toBe(200)
await response.text()
const accountsData = capturedUpdates[0].accounts_data as Array<{
uid: string
dedup_scope?: string
}>
expect(accountsData[0].dedup_scope).toBe('new-1')
expect(mockAllocate).toHaveBeenCalledTimes(1)
expect(
(mockUpsertFromPsd2.mock.calls[0][2] as { reuse_cash_account_id: string | null })
.reuse_cash_account_id,
).toBeNull()
})
it('prefers the survivor account explicit dedup scope over a carried sibling scope', async () => {
// A superseded sibling shares the IBAN but was ingested under a different
// scope. The survivor's own row already pinned an explicit scope for this
@@ -374,6 +374,67 @@ async function finalizeConnection(
}
})
// The maps above leave one corner open (issue #1709): a NO-IBAN account
// whose uid changed on an in-place reconnect matches neither by IBAN nor by
// uid, so its scope regenerates, every historical external_id changes, and
// the whole history re-imports as fresh unbooked rows. Pair such accounts by
// elimination, but only when the pairing is unambiguous: per currency,
// EXACTLY ONE prior account left unclaimed (no new account matched it via
// IBAN or uid) and EXACTLY ONE new account with a fresh scope, and neither
// side carries an IBAN. Anything else keeps the fresh-scope behavior. The
// asymmetry is deliberate: a wrong pairing can at worst skip a new
// transaction whose account+date+amount+occurrence all collide with an old
// row, while a missed pairing re-imports the full history unbooked.
const pairedPriorUidByNewUid = new Map<string, string>()
if (priorAccounts.length > 0) {
const newIbans = new Set<string>()
const newUids = new Set<string>()
for (const account of accountsMetadata) {
const normalizedIban = normalizeIban(account.iban)
if (normalizedIban) newIbans.add(normalizedIban)
newUids.add(account.uid)
}
const unclaimedPriorsByCurrency = new Map<string, StoredAccount[]>()
for (const prior of priorAccounts) {
const priorIban = normalizeIban(prior.iban)
if ((priorIban && newIbans.has(priorIban)) || newUids.has(prior.uid)) continue
const currency = (prior.currency || '').toUpperCase()
const bucket = unclaimedPriorsByCurrency.get(currency)
if (bucket) bucket.push(prior)
else unclaimedPriorsByCurrency.set(currency, [prior])
}
const freshScopeByCurrency = new Map<string, StoredAccount[]>()
for (const account of accountsMetadata) {
const normalizedIban = normalizeIban(account.iban)
const matchedPrior =
(normalizedIban ? priorScopeByIban.has(normalizedIban) : false) ||
priorScopeByUid.has(account.uid)
if (matchedPrior) continue
const currency = (account.currency || '').toUpperCase()
const bucket = freshScopeByCurrency.get(currency)
if (bucket) bucket.push(account)
else freshScopeByCurrency.set(currency, [account])
}
for (const [currency, unclaimed] of unclaimedPriorsByCurrency) {
const fresh = freshScopeByCurrency.get(currency) ?? []
if (unclaimed.length !== 1 || fresh.length !== 1) continue
const prior = unclaimed[0]
const survivor = fresh[0]
if (normalizeIban(prior.iban) || normalizeIban(survivor.iban)) continue
survivor.dedup_scope = prior.dedup_scope || prior.uid
// The pairing is an identity claim, so the user's earlier sync choice
// travels with it: a deselected account must not come back pre-checked.
survivor.enabled = prior.enabled !== false
pairedPriorUidByNewUid.set(survivor.uid, prior.uid)
console.log('[enable-banking] Paired no-IBAN account across a uid change', {
connectionId: pendingConnection.id,
currency,
priorUid: prior.uid,
newUid: survivor.uid,
})
}
}
// Stay in 'pending_selection' until the user confirms which accounts to sync.
// The cron and manual sync routes both skip this status, so no transactions
// can be pulled before the user has had a chance to deselect accounts.
@@ -486,12 +547,12 @@ async function finalizeConnection(
// the user already chose instead of overflowing into the next free slots.
const { data: mirroredRows } = await supabase
.from('cash_accounts')
.select('external_uid, ledger_account')
.select('id, external_uid, ledger_account')
.eq('company_id', updatedConnection.company_id)
.eq('bank_connection_id', updatedConnection.id)
const existingLedgerByUid = new Map(
((mirroredRows ?? []) as Array<{ external_uid: string; ledger_account: string }>).map(
(r) => [r.external_uid, r.ledger_account],
const mirroredByUid = new Map(
((mirroredRows ?? []) as Array<{ id: string; external_uid: string; ledger_account: string }>).map(
(r) => [r.external_uid, r],
),
)
// Only ledgers still claimed by a uid the bank returned in THIS session
@@ -504,15 +565,31 @@ async function finalizeConnection(
// holds, whatever its uid.
const sessionUids = new Set(accountsMetadata.map((a) => a.uid))
const assignedLedgers = new Set<string>(
[...existingLedgerByUid.entries()]
.filter(([uid]) => sessionUids.has(uid))
.map(([, ledger]) => ledger),
[...mirroredByUid.values()]
.filter((row) => sessionUids.has(row.external_uid))
.map((row) => row.ledger_account),
)
let accountsDataDirty = carriedScopeDirty
for (const account of accountsMetadata) {
let targetLedger = existingLedgerByUid.get(account.uid)
let targetLedger = mirroredByUid.get(account.uid)?.ledger_account
let reuseCashAccountId: string | null = null
if (!targetLedger) {
// A paired no-IBAN account (uid change on an in-place reconnect) reuses
// this connection's own row for the retired uid: same ledger, same row
// id. upsertFromPsd2 promotes the named row in place, re-keying it to
// the new uid, so transactions.cash_account_id links survive and the
// content-dedup account guard in lib/transactions/ingest.ts keeps
// matching. Without this the resolver would see the old row as a live
// claim and allocate an overflow 19xx slot plus a NEW cash_accounts row,
// which is the second half of issue #1709.
const pairedPriorUid = pairedPriorUidByNewUid.get(account.uid)
const pairedRow = pairedPriorUid ? mirroredByUid.get(pairedPriorUid) : undefined
if (pairedRow && !assignedLedgers.has(pairedRow.ledger_account)) {
targetLedger = pairedRow.ledger_account
reuseCashAccountId = pairedRow.id
}
}
if (!targetLedger) {
const resolved = await resolvePsd2LedgerAccount(
supabase,
@@ -684,6 +684,34 @@ describe('upsertFromPsd2', () => {
expect(stub.upserts).toHaveLength(1)
})
it('promotes a SAME-connection holder under a retired uid when explicitly named via reuse_cash_account_id', async () => {
// Issue #1709: in-place reconnect of a no-IBAN account whose ASPSP minted
// a new uid. The callback pairs the account with the connection's own old
// row and names it explicitly; the promote re-keys that row to the new uid
// in place, so its id (and the transactions linked to it) survive. Without
// the explicit name the plain upsert would INSERT and trip the
// (company_id, ledger_account) UNIQUE constraint; the previous test pins
// that an UNNAMED same-connection holder still routes through the plain
// upsert (the general active-holder rejection is not loosened).
const stub = makeUpsertStub({
holder: { id: 'row-self', bank_connection_id: 'conn-new' },
})
await upsertFromPsd2(makeUpsertSupabase(stub), 'c1', {
...UPSERT_INPUT,
external_uid: 'uid-2',
reuse_cash_account_id: 'row-self',
})
expect(stub.upserts).toHaveLength(0)
expect(stub.updates).toHaveLength(1)
expect(stub.updates[0].id).toBe('row-self')
expect(stub.updates[0].payload).toMatchObject({
bank_connection_id: 'conn-new',
external_uid: 'uid-2',
ledger_account: '1930',
})
})
it('deletes an empty duplicate row for the same connection+uid before promoting', async () => {
// Stuck-user recovery: the reconnect callback mirrored uid-1 onto 1939
// while 1930 was wrongly blocked. On remap to 1930 the empty 1939
+19
View File
@@ -524,6 +524,14 @@ export async function ingestTransactions(
const driftCandidateStoredByBucket = new Map<string, number>()
if (batchIsImportFeed && scopeDriftShadow) {
// Same-feed orphaned-id rows on an INcompatible account are excluded from
// the candidates (a genuinely different account on the same company must
// never bridge), but they are exactly what a reconnect that minted a NEW
// cash_account for the same physical account produces (issue #1709): every
// stored twin then sits on the old account and the shadow stays 0 during
// the incident it was built to measure. Count them separately, log-only,
// so fleet validation can see those incidents.
let accountIncompatibleDriftRows = 0
for (const bucket of [existingMaps.booked, existingMaps.unbookedImported]) {
for (const [k, entries] of bucket) {
for (const entry of entries) {
@@ -535,10 +543,21 @@ export async function ingestTransactions(
const idOrphaned = entry.externalId !== null && !incomingIdSet.has(entry.externalId)
if (sameFeed && accountCompatible && idOrphaned) {
driftCandidateStoredByBucket.set(k, (driftCandidateStoredByBucket.get(k) ?? 0) + 1)
} else if (sameFeed && idOrphaned) {
accountIncompatibleDriftRows++
}
}
}
}
if (accountIncompatibleDriftRows > 0) {
log.info('import dedup shadow: account-incompatible same-feed orphaned ids', {
decision: 'same-feed-scope-drift-cross-account',
mode: 'shadow',
count: accountIncompatibleDriftRows,
cashAccountId,
batchSource,
})
}
}
// ── Shadow-mode date-drift precompute (measure only) ─────────────────────