diff --git a/DECISIONS.md b/DECISIONS.md index ac9ed27c..a4e4a6a3 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1171,3 +1171,4 @@ One line per decision: `[YYYY-MM-DD] : `. 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. diff --git a/app/api/extensions/enable-banking/callback/__tests__/route.test.ts b/app/api/extensions/enable-banking/callback/__tests__/route.test.ts index b3fbc653..aec23cf7 100644 --- a/app/api/extensions/enable-banking/callback/__tests__/route.test.ts +++ b/app/api/extensions/enable-banking/callback/__tests__/route.test.ts @@ -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[] = [] + 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 = {} + chain.update = vi.fn((payload: Record) => { + 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[] = [] + 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 = {} + chain.update = vi.fn((payload: Record) => { + 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[] = [] + 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 = {} + chain.update = vi.fn((payload: Record) => { + 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 diff --git a/app/api/extensions/enable-banking/callback/route.ts b/app/api/extensions/enable-banking/callback/route.ts index 0cde4c84..66e330f2 100644 --- a/app/api/extensions/enable-banking/callback/route.ts +++ b/app/api/extensions/enable-banking/callback/route.ts @@ -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() + if (priorAccounts.length > 0) { + const newIbans = new Set() + const newUids = new Set() + for (const account of accountsMetadata) { + const normalizedIban = normalizeIban(account.iban) + if (normalizedIban) newIbans.add(normalizedIban) + newUids.add(account.uid) + } + const unclaimedPriorsByCurrency = new Map() + 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() + 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( - [...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, diff --git a/lib/cash-accounts/__tests__/service.test.ts b/lib/cash-accounts/__tests__/service.test.ts index 5293999e..d09cbefe 100644 --- a/lib/cash-accounts/__tests__/service.test.ts +++ b/lib/cash-accounts/__tests__/service.test.ts @@ -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 diff --git a/lib/transactions/ingest.ts b/lib/transactions/ingest.ts index c6ef4f41..085df1b1 100644 --- a/lib/transactions/ingest.ts +++ b/lib/transactions/ingest.ts @@ -524,6 +524,14 @@ export async function ingestTransactions( const driftCandidateStoredByBucket = new Map() 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) ─────────────────────