fix(enable-banking): only hard-warn about sibling connections at observed one-session banks (#2005)

* fix(enable-banking): only hard-warn about sibling connections at observed one-session banks The same-bank warning dialog fired for every bank whenever the user's other companies held a connection, claiming the siblings could stop syncing. That is only true for banks binding one active AIS session per PSU (observed: SEB). Prod shows Handelsbanken tolerates at least four concurrent sessions syncing daily, and the generic warning made a multi-company user abandon a legitimate renewal of an expired Handelsbanken connection. The decision now lives in a pure, tested module (connection-warning.ts): - One-session banks (SEB): hard warning on fresh connect and renewal alike. - Other banks, renewal: no dialog; the connection already coexisted. - Other banks, fresh connect: calm confirmation so a user who meant to renew notices they are about to create a second connection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016PUKATngZZzqkoDesyxKR3 * fix(enable-banking): fail closed for banks with unknown session policy, exempt shared-session siblings Skeptic findings on the first cut: - Unknown banks were treated like verified multi-session banks (silent renewal), turning absence of evidence into evidence of absence. They now keep the previous hedged warning on both paths; only Handelsbanken (verified 2026-08-28: 2-4 concurrent connections per user on DISTINCT session_ids, all syncing daily) gets the calm tier. - Siblings sharing the session being renewed are carried by fanOutSessionRenewal and never break, so they no longer trigger or inflate the warning; the absolute SEB claim is accurate for the remaining, genuinely separate sessions. - Company names are deduped and phrasing follows distinct company count (one company holding privat + foretag rows is "ett annat bolag"). - Null bank_name no longer throws (DB column is nullable). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016PUKATngZZzqkoDesyxKR3 * fix(enable-banking): count clashing companies by id, names are display-only CodeRabbit: two distinct companies sharing a name, or one whose name fails to resolve, undercounted to "ett annat bolag". SameBankClash now carries companyId as identity; the parenthetical name list stays deduped display. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016PUKATngZZzqkoDesyxKR3 --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-08-28 15:31:38 +02:00
parent 57d4359d1a
commit 7993700589
4 changed files with 373 additions and 29 deletions
@@ -1320,3 +1320,5 @@ One line per decision: `[YYYY-MM-DD] <decision>: <why>`. Appended by agents and
 [2026-08-27] Invite-only brand signup ships accepting a low-severity allowlist enumeration residual: POST /api/auth/signup returns 403 for a non-allowlisted email vs 200/400 for an allowlisted one, and the 403 short-circuits before GoTrue, so it is captcha-free and unthrottled: someone with candidate emails can test which are on a brand's allowlist. Not closed because (a) the app deliberately never holds the Turnstile secret (it lives in Supabase/GoTrue; a repo test forbids TURNSTILE_SECRET_KEY in app env), and (b) the clear "you're not invited, go to Accounted" redirect UX inherently reveals the verdict. It leaks membership of guessed emails, not the list, and no ledger/credential data. Follow-up option if it matters later: add signup-endpoint rate limiting. The related fail-OPEN (a brands-table error was read as unbranded, opening invite-only signup during a DB blip) WAS fixed: the gate now returns lookupFailed and both signup routes answer 503.
 [2026-08-27] Byrå-team invite acceptance was implemented only in POST /api/team/accept, which the email+password signup flow never reaches before the dashboard (hosted requires email confirmation, so the register page gets no session to run its client-side accept, and the auth callback + onboarding recovery only knew company_invitations). A new byrå admin therefore landed on /onboarding instead of /clients. Fix: one shared server helper acceptPendingTeamInviteByToken (lib/company/pending-invites.ts), called by the route (unchanged HTTP contract), the auth callback (accepts BEFORE landing resolves, so resolveLandingDestination sees the membership and sends admins to /clients; cookie cleared on success), and acceptPendingInviteByToken (onboarding/select-company recovery, tries company then team). hasPendingInviteForEmail now checks both invite tables. No migration.
 [2026-08-28] Per-company hiding of standardmallar via new booking_template_hidden table (insert=hide, delete=unhide), not is_active or a library column: system template rows are shared globally, so per-company state must live beside them; hiding is opt-in per company and restorable in settings (user request).
+[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).
@@ -35,6 +35,7 @@ import {
  selectPageAttention,
  sortConnectionsByPrecedence,
 } from '../lib/connection-state'
+import { sameBankWarning } from '../lib/connection-warning'
 import type { BankConnection } from '@/types'
 import type { StoredAccount } from '../types'

@@ -91,7 +92,7 @@ export default function BankingSettingsPanel() {
  const [reusableSessions, setReusableSessions] = useState<ReusableSessionOffer[]>([])
  const [attachingConnectionId, setAttachingConnectionId] = useState<string | null>(null)
  const [otherCompanyConnections, setOtherCompanyConnections] = useState<
-    { bank_name: string; company_id: string }[]
+    { bank_name: string; company_id: string; session_id: string | null }[]
  >([])
  // Set when the OAuth callback pointed at a connection that belongs to a
  // different company than the active one: without this the picker simply
@@ -228,12 +229,14 @@ export default function BankingSettingsPanel() {
      // competing for the bank's one-session-per-login slot.
      const { data: allConnections } = await supabase
        .from('bank_connections')
-        .select('bank_name, company_id, status')
+        .select('bank_name, company_id, session_id, status')
        .in('status', ['active', 'pending_selection'])
      setOtherCompanyConnections(
-        ((allConnections || []) as { bank_name: string; company_id: string }[]).filter(
-          (c) => c.company_id !== company.id
-        )
+        ((allConnections || []) as {
+          bank_name: string
+          company_id: string
+          session_id: string | null
+        }[]).filter((c) => c.company_id !== company.id)
      )

      // Reuse offers. Best-effort: a failure here costs the shortcut, never the
@@ -274,31 +277,35 @@ export default function BankingSettingsPanel() {

  /**
   * Warn before authorizing a bank where the same user already holds live
-   * connections in other companies. Several ASPSPs bind one active AIS session
-   * per PSU, so the new authorization silently invalidates the existing ones,
-   * and nothing in the product tells the user until a sync fails days later.
-   * Advisory only: legitimate multi-company setups must still be able to
-   * proceed, so the dialog always offers a working "Fortsätt".
+   * connections in other companies. The decision table lives in
+   * lib/connection-warning.ts: banks with observed one-session-per-PSU
+   * behavior get the hard warning, everything else gets a calm confirmation
+   * on fresh connects and no dialog at all on renewals. Advisory only:
+   * legitimate multi-company setups must still be able to proceed, so the
+   * dialog always offers a working confirm.
   */
-  async function confirmSameBankConnections(bankName: string): Promise<boolean> {
-    const clashes = otherCompanyConnections.filter((c) => c.bank_name === bankName)
-    if (clashes.length === 0) return true
+  async function confirmSameBankConnections(
+    bankName: string,
+    isReconnect: boolean,
+    currentSessionId?: string | null,
+  ): Promise<boolean> {
+    const clashes = otherCompanyConnections
+      .filter((c) => c.bank_name === bankName)
+      .map((c) => ({
+        companyId: c.company_id,
+        companyName:
+          companies.find((entry) => entry.company.id === c.company_id)?.company.name ?? null,
+        sessionId: c.session_id,
+      }))

-    const names = clashes
-      .map((c) => companies.find((entry) => entry.company.id === c.company_id)?.company.name)
-      .filter((name): name is string => !!name)
-    const companyList = names.length > 0 ? ` (${names.join(', ')})` : ''
-    const count = clashes.length
-
-    return confirm({
-      title: `Du har redan ${count} ${count === 1 ? 'anslutning' : 'anslutningar'} till ${bankName}`,
-      description:
-        `${bankName} är sedan tidigare ansluten i ${count === 1 ? 'ett annat bolag' : 'andra bolag'}${companyList}. ` +
-        'Vissa banker tillåter bara en aktiv anslutning per inloggning: när du slutför den här kan de andra sluta synka ' +
-        'och behöva förnyas. Fortsätt om du vet att din bank tillåter flera.',
-      confirmLabel: 'Fortsätt',
-      variant: 'warning',
+    const warning = sameBankWarning({
+      bankName,
+      clashes,
+      isReconnect,
+      currentSessionId,
    })
+    if (!warning) return true
+    return confirm(warning)
  }

  /**
@@ -370,7 +377,7 @@ export default function BankingSettingsPanel() {
    // indefinitely, and a second click in that window would otherwise sail
    // past the guard above and start a concurrent connect flow.
    connectingRef.current = true
-    if (!(await confirmSameBankConnections(bank.name))) {
+    if (!(await confirmSameBankConnections(bank.name, false))) {
      connectingRef.current = false
      return
    }
@@ -442,7 +449,7 @@ export default function BankingSettingsPanel() {
    if (connectingRef.current) return
    // Lock before the confirm await, same reason as handleConnectBank.
    connectingRef.current = true
-    if (!(await confirmSameBankConnections(connection.bank_name))) {
+    if (!(await confirmSameBankConnections(connection.bank_name, true, connection.session_id))) {
      connectingRef.current = false
      return
    }
@@ -0,0 +1,194 @@
+import { describe, it, expect } from 'vitest'
+
+import {
+  isOneSessionBank,
+  isVerifiedMultiSessionBank,
+  sameBankWarning,
+  type SameBankClash,
+} from '../connection-warning'
+
+function clash(over: Partial<SameBankClash> = {}): SameBankClash {
+  return {
+    companyId: 'company-1',
+    companyName: 'Testbrand AB',
+    sessionId: 'sess-other',
+    ...over,
+  }
+}
+
+describe('bank tier matching', () => {
+  it('matches SEB regardless of casing and whitespace', () => {
+    expect(isOneSessionBank('SEB')).toBe(true)
+    expect(isOneSessionBank(' seb ')).toBe(true)
+  })
+
+  it('matches Handelsbanken as verified multi-session', () => {
+    expect(isVerifiedMultiSessionBank('Handelsbanken')).toBe(true)
+    expect(isVerifiedMultiSessionBank('handelsbanken')).toBe(true)
+  })
+
+  it('does not match banks merely containing the letters, nor null', () => {
+    expect(isOneSessionBank('SEB Kort Bank')).toBe(false)
+    expect(isOneSessionBank(null)).toBe(false)
+    expect(isOneSessionBank(undefined)).toBe(false)
+    expect(isVerifiedMultiSessionBank('Nordea')).toBe(false)
+  })
+})
+
+describe('sameBankWarning', () => {
+  const base = {
+    bankName: 'Handelsbanken',
+    clashes: [clash()],
+    isReconnect: false,
+  }
+
+  it('returns null when nothing clashes', () => {
+    expect(sameBankWarning({ ...base, clashes: [] })).toBeNull()
+  })
+
+  it('is silent when renewing at a verified multi-session bank', () => {
+    // The regression this module fixes: a user abandoned a legitimate
+    // Handelsbanken renewal because the old dialog warned about sibling
+    // connections that HB demonstrably tolerates.
+    expect(sameBankWarning({ ...base, isReconnect: true })).toBeNull()
+  })
+
+  it('calmly confirms a fresh second connection at a verified multi-session bank', () => {
+    const warning = sameBankWarning(base)
+    expect(warning).not.toBeNull()
+    expect(warning?.confirmLabel).toBe('Anslut')
+    expect(warning?.description).not.toContain('sluta synka')
+    expect(warning?.description).toContain('Testbrand AB')
+  })
+
+  it('hard-warns for a one-session bank on fresh connect and reconnect alike', () => {
+    // A renewal also mints a new authorization, which at a one-session bank
+    // revokes the sibling companies' session just like a fresh connect does.
+    for (const isReconnect of [false, true]) {
+      const warning = sameBankWarning({ ...base, bankName: 'SEB', isReconnect })
+      expect(warning?.title).toBe('Du har redan 1 anslutning till SEB')
+      expect(warning?.description).toContain('slutar de andra att synka')
+      expect(warning?.confirmLabel).toBe('Fortsätt ändå')
+    }
+  })
+
+  it('keeps the hedged warning for banks with unknown session policy, on both paths', () => {
+    // Fail closed: absence of evidence about a bank is not evidence that it
+    // tolerates parallel sessions, so unknown banks keep the old behavior.
+    for (const isReconnect of [false, true]) {
+      const warning = sameBankWarning({ ...base, bankName: 'Nordea', isReconnect })
+      expect(warning).not.toBeNull()
+      expect(warning?.description).toContain('Vissa banker tillåter bara en aktiv anslutning')
+      expect(warning?.description).toContain('kan de andra sluta synka')
+      expect(warning?.confirmLabel).toBe('Fortsätt')
+    }
+  })
+
+  it('exempts siblings sharing the session being renewed', () => {
+    // fanOutSessionRenewal carries the renewed session to every sibling on
+    // it, so they never break and must not trigger the warning: even at SEB,
+    // even at an unknown bank.
+    for (const bankName of ['SEB', 'Nordea']) {
+      const warning = sameBankWarning({
+        bankName,
+        clashes: [clash({ sessionId: 'sess-current' })],
+        isReconnect: true,
+        currentSessionId: 'sess-current',
+      })
+      expect(warning).toBeNull()
+    }
+  })
+
+  it('still warns about siblings on OTHER sessions when renewing', () => {
+    const warning = sameBankWarning({
+      bankName: 'SEB',
+      clashes: [
+        clash({ sessionId: 'sess-current', companyName: 'Delad AB' }),
+        clash({ sessionId: 'sess-other', companyName: 'Separat AB' }),
+      ],
+      isReconnect: true,
+      currentSessionId: 'sess-current',
+    })
+    expect(warning?.title).toBe('Du har redan 1 anslutning till SEB')
+    expect(warning?.description).toContain('Separat AB')
+    expect(warning?.description).not.toContain('Delad AB')
+  })
+
+  it('never exempts on null session ids', () => {
+    // A null on either side proves nothing about sharing.
+    const warning = sameBankWarning({
+      bankName: 'SEB',
+      clashes: [clash({ sessionId: null })],
+      isReconnect: true,
+      currentSessionId: null,
+    })
+    expect(warning).not.toBeNull()
+  })
+
+  it('dedupes company names and phrases by company count', () => {
+    // One company can hold two connections (privat + företag) to one bank:
+    // that is still "ett annat bolag", named once.
+    const warning = sameBankWarning({
+      bankName: 'SEB',
+      clashes: [
+        clash({ sessionId: 'a' }),
+        clash({ sessionId: 'b' }),
+      ],
+      isReconnect: false,
+    })
+    expect(warning?.title).toBe('Du har redan 2 anslutningar till SEB')
+    expect(warning?.description).toContain('ett annat bolag (Testbrand AB)')
+    expect(warning?.description).not.toContain('Testbrand AB, Testbrand AB')
+  })
+
+  it('pluralizes across distinct companies', () => {
+    const warning = sameBankWarning({
+      bankName: 'SEB',
+      clashes: [
+        clash({ companyName: 'Testbrand AB' }),
+        clash({ companyId: 'company-2', companyName: 'Provbolaget AB', sessionId: 'sess-2' }),
+      ],
+      isReconnect: false,
+    })
+    expect(warning?.description).toContain('andra bolag (Testbrand AB, Provbolaget AB)')
+  })
+
+  it('counts companies by id, not display name', () => {
+    // Two DISTINCT companies sharing a name, or a company whose name failed
+    // to resolve, must still pluralize: names are display-only.
+    const sameName = sameBankWarning({
+      bankName: 'SEB',
+      clashes: [
+        clash({ companyId: 'company-1', companyName: 'Testbrand AB' }),
+        clash({ companyId: 'company-2', companyName: 'Testbrand AB', sessionId: 'sess-2' }),
+      ],
+      isReconnect: false,
+    })
+    expect(sameName?.description).toContain('andra bolag (Testbrand AB)')
+
+    const mixedNamedUnnamed = sameBankWarning({
+      bankName: 'SEB',
+      clashes: [
+        clash({ companyId: 'company-1', companyName: 'Testbrand AB' }),
+        clash({ companyId: 'company-2', companyName: null, sessionId: 'sess-2' }),
+      ],
+      isReconnect: false,
+    })
+    expect(mixedNamedUnnamed?.description).toContain('andra bolag (Testbrand AB)')
+  })
+
+  it('omits the company list when no names are known', () => {
+    const warning = sameBankWarning({
+      ...base,
+      bankName: 'SEB',
+      clashes: [clash({ companyName: null })],
+    })
+    expect(warning?.description).not.toContain('(')
+  })
+
+  it('survives a null bank name', () => {
+    const warning = sameBankWarning({ ...base, bankName: null })
+    expect(warning).not.toBeNull()
+    expect(warning?.title).toContain('Banken')
+  })
+})
@@ -0,0 +1,141 @@
+/**
+ * Same-bank connection warning: decides IF and HOW to warn before a user
+ * authorizes a bank that already has live connections in their other
+ * companies.
+ *
+ * Three tiers, strictly evidence-based (fail closed for the unknown middle):
+ *
+ * - Banks OBSERVED to bind one active AIS session per PSU get a hard warning
+ *   on fresh connect and renewal alike: completing the authorization kills the
+ *   sibling sessions bank-side.
+ * - Banks VERIFIED to tolerate concurrent sessions get no dialog on renewal
+ *   and a calm confirmation on fresh connect. The old generic scare dialog
+ *   made a real multi-company user abandon a legitimate Handelsbanken renewal.
+ * - Everything else keeps the previous hedged warning on both paths: we do
+ *   not know the bank's session policy, and silence here would let a renewal
+ *   silently kill a sibling company's feed at a one-session bank we have not
+ *   identified yet.
+ *
+ * Siblings that SHARE the session being renewed are not warned about at all:
+ * the renewal callback carries the new session to them (fanOutSessionRenewal
+ * in session-sharing.ts), so they keep syncing whatever the bank's policy is.
+ *
+ * Pure module (no React, no fetch) so the decision table is unit-testable.
+ */
+
+/**
+ * Banks where prod evidence shows one active AIS session per PSU: authorizing
+ * company B kills company A's session bank-side. Matched against the Enable
+ * Banking ASPSP name stored in bank_connections.bank_name.
+ */
+export const ONE_SESSION_BANKS = ['SEB']
+
+/**
+ * Banks VERIFIED to tolerate several concurrent AIS sessions for one PSU.
+ * Evidence bar: distinct session_ids observed syncing concurrently in prod.
+ * Handelsbanken verified 2026-08-28: three users with 2-4 concurrent business
+ * connections each, all on DISTINCT session_ids, all syncing daily.
+ * Extend only with the same standard of evidence; connection count alone is
+ * not enough, since several connections can share one session.
+ */
+export const VERIFIED_MULTI_SESSION_BANKS = ['Handelsbanken']
+
+function matches(list: readonly string[], bankName: string | null | undefined): boolean {
+  const normalized = (bankName ?? '').trim().toUpperCase()
+  if (!normalized) return false
+  return list.some(known => normalized === known.toUpperCase())
+}
+
+export function isOneSessionBank(bankName: string | null | undefined): boolean {
+  return matches(ONE_SESSION_BANKS, bankName)
+}
+
+export function isVerifiedMultiSessionBank(bankName: string | null | undefined): boolean {
+  return matches(VERIFIED_MULTI_SESSION_BANKS, bankName)
+}
+
+/** One clashing connection in another of the user's companies. */
+export interface SameBankClash {
+  /** Identity: the company holding the connection. Names are display-only. */
+  companyId: string
+  companyName: string | null
+  sessionId: string | null
+}
+
+export interface SameBankWarningInput {
+  bankName: string | null | undefined
+  /** The user's OTHER companies' live connections to this bank. */
+  clashes: SameBankClash[]
+  /** True when renewing an existing connection, false for a fresh connect. */
+  isReconnect: boolean
+  /** session_id of the connection being renewed, to exempt shared-session
+   *  siblings (the renewal is fanned out to them, they never break). */
+  currentSessionId?: string | null
+}
+
+export interface SameBankWarning {
+  title: string
+  description: string
+  confirmLabel: string
+  variant: 'warning'
+}
+
+/** The dialog to show before proceeding, or null to proceed silently. */
+export function sameBankWarning(input: SameBankWarningInput): SameBankWarning | null {
+  const { bankName, clashes, isReconnect, currentSessionId } = input
+
+  // Siblings on the session being renewed ride along on the renewal
+  // (fanOutSessionRenewal): they are not at risk and must not inflate the
+  // warning. A null session on either side proves nothing, so it never exempts.
+  const atRisk = currentSessionId
+    ? clashes.filter(c => c.sessionId !== currentSessionId)
+    : clashes
+  if (atRisk.length === 0) return null
+
+  const bank = (bankName ?? '').trim() || 'Banken'
+  const names = [...new Set(atRisk.map(c => c.companyName).filter((n): n is string => !!n))]
+  const companyList = names.length > 0 ? ` (${names.join(', ')})` : ''
+  // Phrase by DISTINCT COMPANY IDS, not names: two companies can share a name
+  // and a company without a resolvable name still counts. Names are only the
+  // parenthetical display. One company can legitimately hold two connections
+  // (privat + företag) to the same bank and stays "ett annat bolag".
+  const companyCount = new Set(atRisk.map(c => c.companyId)).size
+  const inOtherCompanies = companyCount === 1 ? 'ett annat bolag' : 'andra bolag'
+  const count = atRisk.length
+
+  if (isOneSessionBank(bank)) {
+    return {
+      title: `Du har redan ${count} ${count === 1 ? 'anslutning' : 'anslutningar'} till ${bank}`,
+      description:
+        `${bank} är sedan tidigare ansluten i ${inOtherCompanies}${companyList}. ` +
+        `${bank} tillåter bara en aktiv anslutning per inloggning: när du slutför den här slutar ` +
+        'de andra att synka och behöver förnyas.',
+      confirmLabel: 'Fortsätt ändå',
+      variant: 'warning',
+    }
+  }
+
+  if (isVerifiedMultiSessionBank(bank)) {
+    if (isReconnect) return null
+    return {
+      title: `${bank} är redan ansluten i ${inOtherCompanies}`,
+      description:
+        `${bank} är sedan tidigare ansluten i ${inOtherCompanies}${companyList}. ` +
+        'Du kan ansluta banken även för det här bolaget; bolagen får varsin koppling. ' +
+        'Om du i stället ville förnya en befintlig koppling gör du det från bolaget som äger den.',
+      confirmLabel: 'Anslut',
+      variant: 'warning',
+    }
+  }
+
+  // Unknown session policy: keep the previous hedged warning on BOTH paths.
+  return {
+    title: `Du har redan ${count} ${count === 1 ? 'anslutning' : 'anslutningar'} till ${bank}`,
+    description:
+      `${bank} är sedan tidigare ansluten i ${inOtherCompanies}${companyList}. ` +
+      'Vissa banker tillåter bara en aktiv anslutning per inloggning: när du slutför den här kan de andra sluta synka ' +
+      'och behöva förnyas. Fortsätt om du vet att din bank tillåter flera.',
+    confirmLabel: 'Fortsätt',
+    variant: 'warning',
+  }
+}