fix(mcp): scope VAT close reconciliation account (#1295)

Resolve the VAT close reconciliation scope using the selected cash account, currency, and unassigned-transaction behavior. Add regression coverage for cross-account leakage and fail closed on lookup errors.\n\nCloses #1290
2026-07-30 11:29:50 +02:00
parent 6318501b71
commit 144cc51458
2 changed files with 220 additions and 24 deletions
@@ -0,0 +1,169 @@
+/**
+ * gnubok_vat_close_check: cash-account scoping for bank reconciliation.
+ *
+ * getReconciliationStatus only isolates same-currency bank feeds when its
+ * cashAccountId is populated. These tests pin the VAT close check to the same
+ * account resolution used by the standalone reconciliation tool so another
+ * cash account cannot inflate 1930's bank total.
+ */
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+vi.mock('@/lib/reconciliation/bank-reconciliation', () => ({
+  getReconciliationStatus: vi.fn(async () => ({
+    is_reconciled: true,
+    difference: 0,
+    unmatched_transaction_count: 0,
+    unmatched_gl_line_count: 0,
+  })),
+}))
+
+import { getReconciliationStatus } from '@/lib/reconciliation/bank-reconciliation'
+import { computeVatCloseCheck } from '../server'
+
+const COMPANY_ID = 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa'
+const PERIOD = { period_type: 'monthly', year: 2026, period: 1 }
+const getReconciliationStatusMock = vi.mocked(getReconciliationStatus)
+
+interface CashAccountFixture {
+  id: string
+  currency: string
+  is_primary: boolean
+}
+
+function mockSupabase(
+  cashAccount: CashAccountFixture | null,
+  cashAccountError: { message: string } | null = null,
+) {
+  const cashAccountFilters: Array<[string, unknown]> = []
+
+  const makeChain = (
+    rows: unknown[],
+    maybeSingleData: unknown = null,
+    eqCalls?: Array<[string, unknown]>,
+    maybeSingleError: { message: string } | null = null,
+  ): Record<string, unknown> => {
+    const chain: Record<string, unknown> = {}
+    const settled = { data: rows, error: null, count: rows.length }
+    chain.range = () => settled
+    chain.single = async () => ({ data: null, error: null })
+    chain.maybeSingle = async () => ({ data: maybeSingleData, error: maybeSingleError })
+    chain.then = (resolve: (value: unknown) => void) => resolve(settled)
+    for (const method of [
+      'order', 'lte', 'gte', 'neq', 'in', 'is', 'select',
+      'limit', 'contains', 'filter', 'not', 'or',
+    ]) {
+      chain[method] = () => chain
+    }
+    chain.eq = (column: string, value: unknown) => {
+      eqCalls?.push([column, value])
+      return chain
+    }
+    return chain
+  }
+
+  const from = vi.fn((table: string) => {
+    if (table === 'cash_accounts') {
+      return makeChain(
+        cashAccount ? [cashAccount] : [],
+        cashAccount,
+        cashAccountFilters,
+        cashAccountError,
+      )
+    }
+    return makeChain([])
+  })
+
+  return {
+    supabase: {
+      from,
+      rpc: (fn: string) =>
+        fn === 'verifikat_without_documents'
+          ? Promise.resolve({
+              data: { ok: true, total_count: 0, verifikat: [] },
+              error: null,
+            })
+          : makeChain([]),
+    } as never,
+    cashAccountFilters,
+  }
+}
+
+beforeEach(() => {
+  vi.clearAllMocks()
+})
+
+describe('gnubok_vat_close_check: reconciliation scope', () => {
+  it('passes the primary 1930 identity so another SEK cash account cannot leak into its total', async () => {
+    const cashAccount = {
+      id: '11111111-1111-4111-8111-111111111111',
+      currency: 'SEK',
+      is_primary: true,
+    }
+    const { supabase, cashAccountFilters } = mockSupabase(cashAccount)
+
+    await computeVatCloseCheck(PERIOD, COMPANY_ID, supabase)
+
+    expect(cashAccountFilters).toEqual([
+      ['company_id', COMPANY_ID],
+      ['ledger_account', '1930'],
+    ])
+    expect(getReconciliationStatusMock).toHaveBeenCalledWith(
+      supabase,
+      COMPANY_ID,
+      '2026-01-01',
+      '2026-01-31',
+      '1930',
+      'SEK',
+      cashAccount.id,
+      true,
+    )
+  })
+
+  it('does not claim unassigned transactions when 1930 is not the primary cash account', async () => {
+    const cashAccount = {
+      id: '22222222-2222-4222-8222-222222222222',
+      currency: 'EUR',
+      is_primary: false,
+    }
+    const { supabase } = mockSupabase(cashAccount)
+
+    await computeVatCloseCheck(PERIOD, COMPANY_ID, supabase)
+
+    expect(getReconciliationStatusMock).toHaveBeenCalledWith(
+      supabase,
+      COMPANY_ID,
+      '2026-01-01',
+      '2026-01-31',
+      '1930',
+      'EUR',
+      cashAccount.id,
+      false,
+    )
+  })
+
+  it('keeps the legacy 1930 fallback when the company has no cash_accounts row', async () => {
+    const { supabase } = mockSupabase(null)
+
+    await computeVatCloseCheck(PERIOD, COMPANY_ID, supabase)
+
+    expect(getReconciliationStatusMock).toHaveBeenCalledWith(
+      supabase,
+      COMPANY_ID,
+      '2026-01-01',
+      '2026-01-31',
+      '1930',
+      'SEK',
+      undefined,
+      true,
+    )
+  })
+
+  it('fails closed when the cash-account lookup errors instead of reconciling every SEK account', async () => {
+    const { supabase } = mockSupabase(null, { message: 'connection failed' })
+
+    await expect(computeVatCloseCheck(PERIOD, COMPANY_ID, supabase)).rejects.toThrow(
+      'Kunde inte hämta kassakonto 1930',
+    )
+    expect(getReconciliationStatusMock).not.toHaveBeenCalled()
+  })
+})
@@ -1760,6 +1760,55 @@ async function countMissingUnderlagInPeriod(
  return Math.max(0, fromStart - afterEnd)
 }

+/**
+ * Resolve the cash-account identity before comparing its bank feed with the
+ * ledger. The cashAccountId is what prevents another same-currency account
+ * from being included in the transaction total.
+ */
+async function getScopedReconciliationStatus(
+  supabase: SupabaseClient,
+  companyId: string,
+  dateFrom: string | undefined,
+  dateTo: string | undefined,
+  accountNumber: string,
+) {
+  const { data: cashAccount, error: cashAccountError } = await supabase
+    .from('cash_accounts')
+    .select('id, currency, is_primary')
+    .eq('company_id', companyId)
+    .eq('ledger_account', accountNumber)
+    .maybeSingle()
+
+  if (cashAccountError) {
+    log.error('Cash account lookup failed during reconciliation', {
+      companyId,
+      accountNumber,
+      errorCode: cashAccountError.code,
+      errorMessage: cashAccountError.message,
+    })
+    throw new Error(`Kunde inte hämta kassakonto ${accountNumber}`)
+  }
+
+  if (!cashAccount && accountNumber !== '1930') {
+    throw new Error(`Okänt kassakonto ${accountNumber} för det här företaget`)
+  }
+
+  const currency = (cashAccount?.currency as string | undefined) ?? 'SEK'
+  const cashAccountId = cashAccount?.id as string | undefined
+  const includeUnassigned = cashAccount ? Boolean(cashAccount.is_primary) : true
+
+  return getReconciliationStatus(
+    supabase,
+    companyId,
+    dateFrom,
+    dateTo,
+    accountNumber,
+    currency,
+    cashAccountId,
+    includeUnassigned,
+  )
+}
+
 export async function computeVatCloseCheck(
  args: Record<string, unknown>,
  companyId: string,
@@ -1804,7 +1853,7 @@ export async function computeVatCloseCheck(
      .eq('company_id', companyId)
      .eq('status', 'registered')
      .gte('invoice_date', start).lte('invoice_date', end),
-    getReconciliationStatus(supabase, companyId, start, end),
+    getScopedReconciliationStatus(supabase, companyId, start, end, '1930'),
    // Verifikat in the period that genuinely lack an underlag (BFL 5 kap
    // 6-7 §), counted over the SHARED SQL predicate. Never re-derive this
    // locally: countMissingUnderlagInPeriod documents what the hand-rolled
@@ -8975,34 +9024,12 @@ export const tools: McpTool[] = [
      const dateTo = args.date_to as string | undefined
      const accountNumber = (args.account_number as string | undefined) || '1930'

-      // Pair the bank account with its currency + cash_account_id so EUR GL
-      // movements aren't compared against SEK transactions, and so a secondary
-      // same-currency account doesn't pool the primary's unassigned rows. Mirrors
-      // app/api/reconciliation/bank/status/route.ts.
-      const { data: cashAccount } = await supabase
-        .from('cash_accounts')
-        .select('id, currency, is_primary')
-        .eq('company_id', companyId)
-        .eq('ledger_account', accountNumber)
-        .maybeSingle()
-
-      if (!cashAccount && accountNumber !== '1930') {
-        throw new Error(`Okänt kassakonto ${accountNumber} för det här företaget`)
-      }
-
-      const currency = (cashAccount?.currency as string | undefined) ?? 'SEK'
-      const cashAccountId = cashAccount?.id as string | undefined
-      const includeUnassigned = cashAccount ? Boolean(cashAccount.is_primary) : true
-
-      return await getReconciliationStatus(
+      return await getScopedReconciliationStatus(
        supabase,
        companyId,
        dateFrom,
        dateTo,
        accountNumber,
-        currency,
-        cashAccountId,
-        includeUnassigned,
      )
    },
  },