From 144cc51458b3d8eb54cd60535d342f56ee4a9dc9 Mon Sep 17 00:00:00 2001 From: Mattsson <111893710+mattssonn@users.noreply.github.com> Date: Thu, 30 Jul 2026 11:29:50 +0200 Subject: [PATCH] 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 --- ...t-close-check-reconciliation-scope.test.ts | 169 ++++++++++++++++++ extensions/general/mcp-server/server.ts | 75 +++++--- 2 files changed, 220 insertions(+), 24 deletions(-) create mode 100644 extensions/general/mcp-server/__tests__/vat-close-check-reconciliation-scope.test.ts diff --git a/extensions/general/mcp-server/__tests__/vat-close-check-reconciliation-scope.test.ts b/extensions/general/mcp-server/__tests__/vat-close-check-reconciliation-scope.test.ts new file mode 100644 index 00000000..77c64045 --- /dev/null +++ b/extensions/general/mcp-server/__tests__/vat-close-check-reconciliation-scope.test.ts @@ -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 => { + const chain: Record = {} + 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() + }) +}) diff --git a/extensions/general/mcp-server/server.ts b/extensions/general/mcp-server/server.ts index 6a5fc355..df9258e6 100644 --- a/extensions/general/mcp-server/server.ts +++ b/extensions/general/mcp-server/server.ts @@ -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, 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, ) }, },