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 index 77c64045..963cd15a 100644 --- 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 @@ -5,6 +5,10 @@ * 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. + * + * Both call sites now resolve through lib/reconciliation/cash-account-scope.ts + * (shared with the bokslut readiness aggregator, which is core code and cannot + * import from @/extensions/), so the same assertions cover the tool handler too. */ import { beforeEach, describe, expect, it, vi } from 'vitest' @@ -18,27 +22,35 @@ vi.mock('@/lib/reconciliation/bank-reconciliation', () => ({ })) import { getReconciliationStatus } from '@/lib/reconciliation/bank-reconciliation' -import { computeVatCloseCheck } from '../server' +import { computeVatCloseCheck, tools } from '../server' const COMPANY_ID = 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa' const PERIOD = { period_type: 'monthly', year: 2026, period: 1 } const getReconciliationStatusMock = vi.mocked(getReconciliationStatus) +const reconStatusTool = tools.find((t) => t.name === 'gnubok_get_reconciliation_status')! interface CashAccountFixture { id: string currency: string is_primary: boolean + ledger_account: string } +/** + * `cashAccounts` is a QUEUE, one entry per cash_accounts lookup: resolving the + * settlement account can take a second query (no 1930 row -> the company's + * primary cash account), and those two must be able to answer differently. + */ function mockSupabase( - cashAccount: CashAccountFixture | null, + cashAccounts: Array, cashAccountError: { message: string } | null = null, ) { const cashAccountFilters: Array<[string, unknown]> = [] + let lookup = 0 const makeChain = ( rows: unknown[], - maybeSingleData: unknown = null, + isCashAccounts = false, eqCalls?: Array<[string, unknown]>, maybeSingleError: { message: string } | null = null, ): Record => { @@ -46,7 +58,12 @@ function mockSupabase( 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.maybeSingle = async () => { + if (!isCashAccounts) return { data: null, error: null } + const row = cashAccounts[lookup] ?? null + lookup += 1 + return { data: row, error: maybeSingleError } + } chain.then = (resolve: (value: unknown) => void) => resolve(settled) for (const method of [ 'order', 'lte', 'gte', 'neq', 'in', 'is', 'select', @@ -63,12 +80,7 @@ function mockSupabase( const from = vi.fn((table: string) => { if (table === 'cash_accounts') { - return makeChain( - cashAccount ? [cashAccount] : [], - cashAccount, - cashAccountFilters, - cashAccountError, - ) + return makeChain([], true, cashAccountFilters, cashAccountError) } return makeChain([]) }) @@ -90,6 +102,12 @@ function mockSupabase( beforeEach(() => { vi.clearAllMocks() + getReconciliationStatusMock.mockResolvedValue({ + is_reconciled: true, + difference: 0, + unmatched_transaction_count: 0, + unmatched_gl_line_count: 0, + } as never) }) describe('gnubok_vat_close_check: reconciliation scope', () => { @@ -98,8 +116,9 @@ describe('gnubok_vat_close_check: reconciliation scope', () => { id: '11111111-1111-4111-8111-111111111111', currency: 'SEK', is_primary: true, + ledger_account: '1930', } - const { supabase, cashAccountFilters } = mockSupabase(cashAccount) + const { supabase, cashAccountFilters } = mockSupabase([cashAccount]) await computeVatCloseCheck(PERIOD, COMPANY_ID, supabase) @@ -124,8 +143,9 @@ describe('gnubok_vat_close_check: reconciliation scope', () => { id: '22222222-2222-4222-8222-222222222222', currency: 'EUR', is_primary: false, + ledger_account: '1930', } - const { supabase } = mockSupabase(cashAccount) + const { supabase } = mockSupabase([cashAccount]) await computeVatCloseCheck(PERIOD, COMPANY_ID, supabase) @@ -142,7 +162,7 @@ describe('gnubok_vat_close_check: reconciliation scope', () => { }) it('keeps the legacy 1930 fallback when the company has no cash_accounts row', async () => { - const { supabase } = mockSupabase(null) + const { supabase } = mockSupabase([null, null]) await computeVatCloseCheck(PERIOD, COMPANY_ID, supabase) @@ -158,12 +178,197 @@ describe('gnubok_vat_close_check: reconciliation scope', () => { ) }) + it('reconciles the primary cash account when the company has no 1930 row at all', async () => { + // The worst instance of #1290, measured on prod: 2 companies run two SEK + // cash accounts each (1935+1936 and 1932+1935), have NO 1930 cash_accounts + // row and ZERO journal_entry_lines on 1930. Scoped to '1930' they compared + // their whole SEK bank volume (2026: 120104.43 kr and -22347.00 kr) against + // an empty GL side, i.e. a high-severity bank_unreconciled blocker with + // count 0 that no user action could clear. + const primary = { + id: '33333333-3333-4333-8333-333333333333', + currency: 'SEK', + is_primary: true, + ledger_account: '1935', + } + const { supabase, cashAccountFilters } = mockSupabase([null, primary]) + + await computeVatCloseCheck(PERIOD, COMPANY_ID, supabase) + + expect(cashAccountFilters).toEqual([ + ['company_id', COMPANY_ID], + ['ledger_account', '1930'], + ['company_id', COMPANY_ID], + ['is_primary', true], + ]) + expect(getReconciliationStatusMock).toHaveBeenCalledWith( + supabase, + COMPANY_ID, + '2026-01-01', + '2026-01-31', + '1935', + 'SEK', + primary.id, + true, + ) + }) + it('fails closed when the cash-account lookup errors instead of reconciling every SEK account', async () => { - const { supabase } = mockSupabase(null, { message: 'connection failed' }) + 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() }) + + it('still raises the blocker when the SCOPED run finds a real difference', async () => { + // Proves these tests isolate the SCOPING, not the blocker logic: a genuine + // scoped difference must keep blocking, since moms is computed from the + // huvudbok and a difference there hides errors. + getReconciliationStatusMock.mockResolvedValue({ + is_reconciled: false, + difference: -5599.97, + unmatched_transaction_count: 0, + unmatched_gl_line_count: 0, + } as never) + const { supabase } = mockSupabase([ + { + id: '11111111-1111-4111-8111-111111111111', + currency: 'SEK', + is_primary: true, + ledger_account: '1930', + }, + ]) + + const result = await computeVatCloseCheck(PERIOD, COMPANY_ID, supabase) + + const blocker = result.blockers.find((b) => b.kind === 'bank_unreconciled')! + expect(blocker).toBeDefined() + expect(blocker.severity).toBe('high') + expect(blocker.message).toContain('-5599.97') + expect(blocker.message).toContain('1930') + }) + + it('names the RESOLVED account in the blocker message, not a hard-coded 1930', async () => { + getReconciliationStatusMock.mockResolvedValue({ + is_reconciled: false, + difference: 120104.43, + unmatched_transaction_count: 0, + unmatched_gl_line_count: 0, + } as never) + const { supabase } = mockSupabase([ + null, + { + id: '33333333-3333-4333-8333-333333333333', + currency: 'SEK', + is_primary: true, + ledger_account: '1935', + }, + ]) + + const result = await computeVatCloseCheck(PERIOD, COMPANY_ID, supabase) + + const blocker = result.blockers.find((b) => b.kind === 'bank_unreconciled')! + // Pointing the user at 1930 here would send them to an account with no + // lines on it whatsoever. + expect(blocker.message).toContain('1935') + expect(blocker.message).not.toContain('1930') + }) +}) + +describe('gnubok_get_reconciliation_status: same shared resolution', () => { + it('resolves the same scope arguments as the close check', async () => { + const cashAccount = { + id: '11111111-1111-4111-8111-111111111111', + currency: 'SEK', + is_primary: true, + ledger_account: '1930', + } + const { supabase, cashAccountFilters } = mockSupabase([cashAccount]) + + await reconStatusTool.execute({}, COMPANY_ID, 'user-1', supabase) + + expect(getReconciliationStatusMock).toHaveBeenCalledWith( + supabase, + COMPANY_ID, + undefined, + undefined, + '1930', + 'SEK', + cashAccount.id, + true, + ) + expect(cashAccountFilters).toEqual([ + ['company_id', COMPANY_ID], + ['ledger_account', '1930'], + ]) + }) + + it('rejects an account_number the company has no cash account for', async () => { + const { supabase, cashAccountFilters } = mockSupabase([null, null]) + + await expect( + reconStatusTool.execute({ account_number: '9999' }, COMPANY_ID, 'user-1', supabase), + ).rejects.toThrow(/Okänt kassakonto 9999/) + expect(getReconciliationStatusMock).not.toHaveBeenCalled() + // A NAMED account must never silently resolve to the primary one: the + // caller asked about 9999, so a status labelled 9999 carrying another + // account's figures would be worse than the error. + expect(cashAccountFilters).toEqual([ + ['company_id', COMPANY_ID], + ['ledger_account', '9999'], + ]) + }) + + it('carries a foreign account currency through to the reconciliation', async () => { + const { supabase } = mockSupabase([ + { + id: '44444444-4444-4444-8444-444444444444', + currency: 'EUR', + is_primary: false, + ledger_account: '1932', + }, + ]) + + await reconStatusTool.execute( + { account_number: '1932', date_from: '2026-01-01', date_to: '2026-01-31' }, + COMPANY_ID, + 'user-1', + supabase, + ) + + expect(getReconciliationStatusMock).toHaveBeenCalledWith( + supabase, + COMPANY_ID, + '2026-01-01', + '2026-01-31', + '1932', + 'EUR', + '44444444-4444-4444-8444-444444444444', + false, + ) + }) + + it('returns the status object itself, not the internal scope wrapper', async () => { + const status = { + is_reconciled: false, + difference: -12.5, + unmatched_transaction_count: 1, + unmatched_gl_line_count: 2, + } + getReconciliationStatusMock.mockResolvedValue(status as never) + const { supabase } = mockSupabase([ + { + id: '11111111-1111-4111-8111-111111111111', + currency: 'SEK', + is_primary: true, + ledger_account: '1930', + }, + ]) + + const result = await reconStatusTool.execute({}, COMPANY_ID, 'user-1', supabase) + + expect(result).toEqual(status) + }) }) diff --git a/extensions/general/mcp-server/server.ts b/extensions/general/mcp-server/server.ts index df9258e6..9c96e9c0 100644 --- a/extensions/general/mcp-server/server.ts +++ b/extensions/general/mcp-server/server.ts @@ -135,6 +135,7 @@ import { } from '@/lib/salary/agi-submission-state' import { generateSupplierLedger } from '@/lib/reports/supplier-ledger' import { getReconciliationStatus } from '@/lib/reconciliation/bank-reconciliation' +import { resolveCashAccountScope } from '@/lib/reconciliation/cash-account-scope' import { createInvoicePaymentJournalEntry, createInvoiceCashEntry, createInvoiceJournalEntry } from '@/lib/bookkeeping/invoice-entries' import { findMatchingInvoices } from '@/lib/invoices/invoice-matching' import { listRotRutCandidates, createRotRutPayoutRequest } from '@/lib/invoices/rot-rut-service' @@ -1764,49 +1765,42 @@ async function countMissingUnderlagInPeriod( * 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. + * + * The lookup itself lives in lib/reconciliation/cash-account-scope.ts so the + * bokslut readiness aggregator (core code, which cannot import from + * @/extensions/) resolves the scope the exact same way. It keeps the fail-closed + * contract this function introduced in #1295: a cash_accounts lookup error + * throws rather than degrading into the unscoped currency-only path. + * + * Pass accountNumber only when the CALLER named an account; leaving it + * undefined means "the company's bank account", which additionally falls back + * to the primary cash account for companies that have no 1930 row at all. */ async function getScopedReconciliationStatus( supabase: SupabaseClient, companyId: string, dateFrom: string | undefined, dateTo: string | undefined, - accountNumber: string, + 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() + const scope = await resolveCashAccountScope(supabase, companyId, accountNumber) - 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') { + if (!scope.found && accountNumber !== undefined && 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( + const status = await getReconciliationStatus( supabase, companyId, dateFrom, dateTo, - accountNumber, - currency, - cashAccountId, - includeUnassigned, + scope.accountNumber, + scope.currency, + scope.cashAccountId, + scope.includeUnassigned, ) + + return { status, scope } } export async function computeVatCloseCheck( @@ -1840,7 +1834,7 @@ export async function computeVatCloseCheck( ) // 4) Blocker scans: run in parallel - const [uncategorizedRes, unapprovedRes, reconRes, missingUnderlag] = await Promise.all([ + const [uncategorizedRes, unapprovedRes, recon, missingUnderlag] = await Promise.all([ supabase .from('transactions') .select('id', { count: 'exact', head: true }) @@ -1853,7 +1847,10 @@ export async function computeVatCloseCheck( .eq('company_id', companyId) .eq('status', 'registered') .gte('invoice_date', start).lte('invoice_date', end), - getScopedReconciliationStatus(supabase, companyId, start, end, '1930'), + // No account_number argument: the close check wants "the company's bank + // account", so the scope resolver may land on the primary cash account for + // a company that has no 1930 row. + getScopedReconciliationStatus(supabase, companyId, start, end), // 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 @@ -1882,12 +1879,17 @@ export async function computeVatCloseCheck( hint: 'Attestera via gnubok_approve_supplier_invoice: ingående moms (ruta 48) påverkas.', }) } + const reconRes = recon.status if (!reconRes.is_reconciled) { blockers.push({ kind: 'bank_unreconciled', severity: Math.abs(reconRes.difference) > 100 ? 'high' : 'medium', count: reconRes.unmatched_transaction_count + reconRes.unmatched_gl_line_count, - message: `Bankavstämning visar differens ${reconRes.difference.toFixed(2)} kr (${reconRes.unmatched_transaction_count} omatchade banktransaktioner, ${reconRes.unmatched_gl_line_count} omatchade huvudbokslinjer på 1930)`, + // The account is named from the RESOLVED scope, not hard-coded: for a + // company with no 1930 row this check now reconciles its primary cash + // account, and a message pointing at 1930 would send the user to an + // account with no lines on it. + message: `Bankavstämning visar differens ${reconRes.difference.toFixed(2)} kr (${reconRes.unmatched_transaction_count} omatchade banktransaktioner, ${reconRes.unmatched_gl_line_count} omatchade huvudbokslinjer på ${recon.scope.accountNumber})`, hint: 'Granska via gnubok_get_reconciliation_status och matcha: moms beräknas från huvudboken så differenser döljer fel.', }) } @@ -8999,7 +9001,7 @@ export const tools: McpTool[] = [ { name: 'gnubok_get_reconciliation_status', title: 'Bank Reconciliation Status', - description: 'Bank reconciliation for one cash account: matched/unmatched counts, bank vs ledger balance, difference. Defaults to 1930; pass account_number for 1940/1932 etc. Optional date range.', + description: 'Bank reconciliation for one cash account: matched/unmatched counts, bank vs ledger balance, difference. Defaults to 1930, or the primary cash account if there is no 1930; pass account_number for 1940/1932 etc. Optional date range.', inputSchema: { type: 'object', additionalProperties: false, @@ -9022,15 +9024,20 @@ export const tools: McpTool[] = [ async execute(args, companyId, userId, supabase) { const dateFrom = args.date_from as string | undefined const dateTo = args.date_to as string | undefined - const accountNumber = (args.account_number as string | undefined) || '1930' + // Passed through as-is, undefined included: an omitted account_number is + // "the company's bank account", which resolves to 1930 and, for a company + // with no 1930 row, to its primary cash account. Substituting a literal + // '1930' here would instead make an unknown account an error case. + const accountNumber = args.account_number as string | undefined - return await getScopedReconciliationStatus( + const { status } = await getScopedReconciliationStatus( supabase, companyId, dateFrom, dateTo, accountNumber, ) + return status }, }, diff --git a/lib/bokslut/__tests__/readiness-aggregator.test.ts b/lib/bokslut/__tests__/readiness-aggregator.test.ts index 6aa61650..268d82a7 100644 --- a/lib/bokslut/__tests__/readiness-aggregator.test.ts +++ b/lib/bokslut/__tests__/readiness-aggregator.test.ts @@ -16,6 +16,8 @@ import { buildBokslutReadinessReport } from '../readiness-aggregator' import { validateYearEndReadiness } from '@/lib/core/bookkeeping/year-end-service' import { getReconciliationStatus } from '@/lib/reconciliation/bank-reconciliation' +const CASH_ACCOUNT_ID = 'dddddddd-dddd-4ddd-8ddd-dddddddddddd' + interface MockBuilder { select: ReturnType eq: ReturnType @@ -26,6 +28,7 @@ interface MockBuilder { function makeSupabase(handlers: { period: { data: unknown; error: unknown } settings: { data: unknown; error: unknown } + cashAccount?: { data: unknown; error: unknown } }) { function makeBuilder(table: string): MockBuilder { const b: MockBuilder = { @@ -40,6 +43,20 @@ function makeSupabase(handlers: { b.single.mockResolvedValue(handlers.period) } else if (table === 'company_settings') { b.maybeSingle.mockResolvedValue(handlers.settings) + } else if (table === 'cash_accounts') { + // The aggregator resolves 1930 to its cash_accounts row so the bank total + // is scoped to that account (#1290). + b.maybeSingle.mockResolvedValue( + handlers.cashAccount ?? { + data: { + id: CASH_ACCOUNT_ID, + currency: 'SEK', + is_primary: true, + ledger_account: '1930', + }, + error: null, + }, + ) } return b } @@ -110,6 +127,40 @@ describe('buildBokslutReadinessReport', () => { expect(report.reminders.map((r) => r.code)).not.toContain('periodiseringsfond_manual') expect(report.reminders.find((r) => r.code === 'ef_skatt_via_ne')).toBeUndefined() expect(report.reconciliation?.is_reconciled).toBe(true) + // Scoped to the resolved 1930 cash account: a 4-arg call left cashAccountId + // undefined, so the bank side pooled every SEK account while the GL side + // stayed on 1930 and the wizard showed a differens with nothing to match + // (#1290). + expect(vi.mocked(getReconciliationStatus)).toHaveBeenCalledWith( + supabase, + 'co-1', + '2025-01-01', + '2025-12-31', + '1930', + 'SEK', + CASH_ACCOUNT_ID, + true, + ) + }) + + it('drops the reconciliation snapshot when the cash-account lookup fails', async () => { + // resolveCashAccountScope fails CLOSED. The aggregator's catch must turn + // that into "no snapshot" rather than into an unscoped 4-arg call, which is + // the pooling path that produced #1290's phantom differens. + vi.mocked(validateYearEndReadiness).mockResolvedValue(baseValidation()) + // getReconciliationStatus is deliberately left un-stubbed: it must never be + // reached, and an unstubbed mock resolving to undefined would break the + // report if it were. + const supabase = makeSupabase({ + period: { data: PERIOD, error: null }, + settings: { data: { entity_type: 'aktiebolag' }, error: null }, + cashAccount: { data: null, error: { code: '57014', message: 'canceling statement' } }, + }) + + const report = await buildBokslutReadinessReport(supabase, 'co-1', 'user-1', 'fp-1') + + expect(report.reconciliation).toBeNull() + expect(vi.mocked(getReconciliationStatus)).not.toHaveBeenCalled() }) it('returns the EF-only reminder for enskild firma', async () => { diff --git a/lib/bokslut/readiness-aggregator.ts b/lib/bokslut/readiness-aggregator.ts index 1eca8f31..4f92167c 100644 --- a/lib/bokslut/readiness-aggregator.ts +++ b/lib/bokslut/readiness-aggregator.ts @@ -1,6 +1,7 @@ import type { SupabaseClient } from '@supabase/supabase-js' import { validateYearEndReadiness } from '@/lib/core/bookkeeping/year-end-service' import { getReconciliationStatus } from '@/lib/reconciliation/bank-reconciliation' +import { resolveCashAccountScope } from '@/lib/reconciliation/cash-account-scope' import { computeEfDeclarationPreview } from '@/lib/bokslut/enskild-firma/ef-declaration-preview' import type { YearEndValidation } from '@/types' @@ -100,11 +101,24 @@ export async function buildBokslutReadinessReport( // to null so the UI degrades gracefully. let reconciliation: BokslutReadinessReport['reconciliation'] = null try { + // Scope to the company's bank account. A 4-arg call leaves cashAccountId + // undefined and the bank side then sums every SEK cash account while the GL + // side stays on 1930 alone: the wizard surfaced that as "Bankavstämningen + // visar en differens" with nothing to match (#1290). + // + // resolveCashAccountScope fails CLOSED on a lookup error, so the catch below + // turns a failed lookup into "no reconciliation snapshot" rather than into + // the unscoped pooling path that produced the phantom difference. + const scope = await resolveCashAccountScope(supabase, companyId) const status = await getReconciliationStatus( supabase, companyId, period.period_start, period.period_end, + scope.accountNumber, + scope.currency, + scope.cashAccountId, + scope.includeUnassigned, ) reconciliation = { is_reconciled: status.is_reconciled, diff --git a/lib/reconciliation/__tests__/bank-reconciliation.test.ts b/lib/reconciliation/__tests__/bank-reconciliation.test.ts index 840837e6..4c9722ee 100644 --- a/lib/reconciliation/__tests__/bank-reconciliation.test.ts +++ b/lib/reconciliation/__tests__/bank-reconciliation.test.ts @@ -5,6 +5,28 @@ * greedy assignment, dry run, manual link/unlink, status calculation. */ import { describe, it, expect, vi, beforeEach } from 'vitest' + +// The #1290 diagnostic is a log line, so the logger is the assertion surface. +// Hoisted spy (the module is imported at the top of bank-reconciliation.ts). +// +// The REAL logger module is kept and only `warn` is swapped. A file-global stub +// exposing just the handful of methods this suite happens to use would be a trap +// for whoever extends it: vi.mock replaces @/lib/logger for the whole module +// graph of this file, so any module reached from here that calls a level the +// stub omitted, or chains `log.child(...).info(...)`, would throw inside an +// unrelated test. Here child() and every other export stay real. +const { logWarn } = vi.hoisted(() => ({ logWarn: vi.fn() })) +vi.mock('@/lib/logger', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + createLogger: (module: string, base?: Parameters[1]) => ({ + ...actual.createLogger(module, base), + warn: logWarn, + }), + } +}) + import { tryReconcileTransaction, runReconciliation, @@ -1708,3 +1730,216 @@ describe('getReconciliationStatus', () => { expect(status.not_reconcilable_reason).toBeNull() }) }) + +// ============================================================ +// Unscoped-run diagnostic (#1290) +// ============================================================ +// +// Leaving cashAccountId undefined makes scopeTransactionsToAccount fall back to +// a currency-only filter: the transaction side pools EVERY same-currency cash +// account while the GL side stays on one accountNumber. On the read path that +// is the phantom difference the issue reported; on the write path it can +// auto-link a savings-account transaction to a 1930 voucher. The engine cannot +// refuse (single-account companies with no cash_accounts row legitimately take +// the same fallback), so it warns exactly when the fetched rows really do span +// more than one account. + +describe('unscoped cash-account diagnostic', () => { + beforeEach(() => { + vi.clearAllMocks() + eventBus.clear() + }) + + // A real cash_accounts.id has to survive the UUID assertion inside + // scopeTransactionsToAccount, so these are shaped like real ones. + const ACCOUNT_A = '11111111-1111-4111-8111-111111111111' + const ACCOUNT_B = '22222222-2222-4222-8222-222222222222' + + /** + * The queue-driven proxy the suites above use, plus a record of every builder + * call so the transactions select list can be pinned. + */ + function createRecordingMockSupabase() { + const resultQueue: { data: unknown; error: unknown }[] = [] + const calls: { method: string; args: unknown[] }[] = [] + + const enqueue = (...results: { data?: unknown; error?: unknown }[]) => { + for (const r of results) resultQueue.push({ data: r.data ?? null, error: r.error ?? null }) + } + + const buildChain = (): unknown => { + const handler: ProxyHandler = { + get(_target, prop) { + if (prop === 'then') { + const next = resultQueue.shift() ?? { data: null, error: null } + return (resolve: (v: unknown) => void) => resolve(next) + } + return (...args: unknown[]) => { + calls.push({ method: String(prop), args }) + return buildChain() + } + }, + } + return new Proxy({}, handler) + } + + const supabase = { + from: vi.fn().mockImplementation((table: string) => { + calls.push({ method: 'from', args: [table] }) + return buildChain() + }), + rpc: vi.fn().mockImplementation((fn: string) => { + calls.push({ method: 'rpc', args: [fn] }) + return buildChain() + }), + } + + return { supabase, enqueue, calls } + } + + /** The four reads getReconciliationStatus makes, in order. */ + function enqueueStatusReads( + enqueue: (...results: { data?: unknown; error?: unknown }[]) => void, + transactions: unknown[], + ) { + enqueue({ data: transactions }) // 1) transactions + enqueue({ data: [] }) // 2) journal_entries page + enqueue({ data: [] }) // 3) journal_entry_lines for those entries + enqueue({ data: [] }) // 4) RPC get_unlinked_1930_lines + } + + function statusTx(cashAccountId: string | null, amount = 100) { + return { + date: '2026-03-05', + amount, + journal_entry_id: 'je-1', + reconciliation_method: 'manual', + is_ignored: false, + cash_account_id: cashAccountId, + } + } + + it('selects cash_account_id on the transactions read (the diagnostic needs it)', async () => { + // Pins the column list: drop cash_account_id from the select and every row + // arrives with it undefined, so the warning below can never fire again. + const { supabase, enqueue, calls } = createRecordingMockSupabase() + enqueueStatusReads(enqueue, []) + + await getReconciliationStatus(supabase as never, 'company-1') + + const firstFrom = calls.find((c) => c.method === 'from') + expect(firstFrom?.args[0]).toBe('transactions') + const firstSelect = calls.find((c) => c.method === 'select') + expect(firstSelect?.args[0]).toBe( + 'date, amount, journal_entry_id, reconciliation_method, is_ignored, cash_account_id', + ) + }) + + it('warns when getReconciliationStatus runs unscoped over more than one cash account', async () => { + const { supabase, enqueue } = createRecordingMockSupabase() + enqueueStatusReads(enqueue, [statusTx(ACCOUNT_A), statusTx(ACCOUNT_B, 250)]) + + await getReconciliationStatus(supabase as never, 'company-1') + + expect(logWarn).toHaveBeenCalledWith( + 'getReconciliationStatus ran unscoped across several cash accounts', + expect.objectContaining({ + companyId: 'company-1', + operation: 'getReconciliationStatus', + entityType: 'cash_account', + details: expect.objectContaining({ + accountNumber: '1930', + currency: 'SEK', + distinctCashAccounts: 2, + }), + }), + ) + }) + + it('stays silent when getReconciliationStatus is scoped to a cash account', async () => { + // The fix the issue asked for: a scoped call is correct by construction, so + // it must not warn even when the mock hands back foreign rows. + const { supabase, enqueue } = createRecordingMockSupabase() + enqueueStatusReads(enqueue, [statusTx(ACCOUNT_A), statusTx(ACCOUNT_B, 250)]) + + await getReconciliationStatus( + supabase as never, + 'company-1', + undefined, + undefined, + '1930', + 'SEK', + ACCOUNT_A, + true, + ) + + expect(logWarn).not.toHaveBeenCalled() + }) + + it('stays silent when an unscoped run sees exactly one cash account', async () => { + const { supabase, enqueue } = createRecordingMockSupabase() + enqueueStatusReads(enqueue, [statusTx(ACCOUNT_A), statusTx(ACCOUNT_A, 250)]) + + await getReconciliationStatus(supabase as never, 'company-1') + + expect(logWarn).not.toHaveBeenCalled() + }) + + it('stays silent when an unscoped run sees only unassigned rows', async () => { + // The legacy single-account company that has no cash_accounts row at all: + // the currency-only fallback is the intended behaviour there, not a bug. + const { supabase, enqueue } = createRecordingMockSupabase() + enqueueStatusReads(enqueue, [statusTx(null), statusTx(null, 250)]) + + await getReconciliationStatus(supabase as never, 'company-1') + + expect(logWarn).not.toHaveBeenCalled() + }) + + it('warns when runReconciliation runs unscoped over more than one cash account', async () => { + // The WRITE path: this sweep applies matches, so pooling here can persist a + // wrong journal_entry_id, not merely display a wrong figure. + const { supabase, enqueue } = createRecordingMockSupabase() + enqueue({ data: [] }) // RPC: unlinked GL lines + enqueue({ + data: [ + makeTransaction({ id: 'tx-1', cash_account_id: ACCOUNT_A }), + makeTransaction({ id: 'tx-2', cash_account_id: ACCOUNT_B }), + ], + }) + + await runReconciliation(supabase as never, 'company-1', 'user-1') + + expect(logWarn).toHaveBeenCalledWith( + 'runReconciliation ran unscoped across several cash accounts', + expect.objectContaining({ + companyId: 'company-1', + operation: 'runReconciliation', + entityType: 'cash_account', + details: expect.objectContaining({ + accountNumber: '1930', + currency: 'SEK', + distinctCashAccounts: 2, + }), + }), + ) + }) + + it('stays silent when runReconciliation is scoped to a cash account', async () => { + const { supabase, enqueue } = createRecordingMockSupabase() + enqueue({ data: [] }) + enqueue({ + data: [ + makeTransaction({ id: 'tx-1', cash_account_id: ACCOUNT_A }), + makeTransaction({ id: 'tx-2', cash_account_id: ACCOUNT_B }), + ], + }) + + await runReconciliation(supabase as never, 'company-1', 'user-1', { + cashAccountId: ACCOUNT_A, + includeUnassigned: true, + }) + + expect(logWarn).not.toHaveBeenCalled() + }) +}) diff --git a/lib/reconciliation/__tests__/cash-account-scope.test.ts b/lib/reconciliation/__tests__/cash-account-scope.test.ts new file mode 100644 index 00000000..70102a08 --- /dev/null +++ b/lib/reconciliation/__tests__/cash-account-scope.test.ts @@ -0,0 +1,236 @@ +/** + * resolveCashAccountScope: turns a settlement account BAS code into the trailing + * scope arguments getReconciliationStatus / runReconciliation take. + * + * The whole point is that callers stop passing 4 positional args and leaving + * cashAccountId undefined: that made scopeTransactionsToAccount fall back to a + * currency-only filter, so the bank side pooled every same-currency cash account + * while the GL side stayed on one account (issue #1290). + */ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import type { SupabaseClient } from '@supabase/supabase-js' + +import { resolveCashAccountScope } from '../cash-account-scope' + +const COMPANY_ID = 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa' +const CASH_1930 = 'cccccccc-cccc-4ccc-8ccc-cccccccccccc' +const CASH_1932 = 'eeeeeeee-eeee-4eee-8eee-eeeeeeeeeeee' +const CASH_1935 = 'dddddddd-dddd-4ddd-8ddd-dddddddddddd' + +interface LookupResult { + data: unknown + error?: unknown +} + +/** + * Table-routed double that answers each cash_accounts lookup from a queue, so + * the "no 1930 row, fall back to the primary account" path can give a different + * answer to the second query than to the first. Every eq() filter pair is + * recorded so the tenant + account scoping can be asserted. + */ +function mockSupabase(results: LookupResult[]) { + const tables: string[] = [] + const eqPairs: Array<[string, unknown]> = [] + const selects: string[] = [] + let call = 0 + + const chain = { + select: vi.fn((cols: string) => { + selects.push(cols) + return chain + }), + eq: vi.fn((col: string, val: unknown) => { + eqPairs.push([col, val]) + return chain + }), + maybeSingle: vi.fn(async () => { + const result = results[call] ?? { data: null } + call += 1 + return { data: result.data, error: result.error ?? null } + }), + } + const supabase = { + from: vi.fn((table: string) => { + tables.push(table) + return chain + }), + } as unknown as SupabaseClient + return { supabase, tables, eqPairs, selects, chain } +} + +const found = (data: unknown): LookupResult[] => [{ data }] + +beforeEach(() => { + vi.clearAllMocks() +}) + +describe('resolveCashAccountScope', () => { + it('resolves the primary 1930 row and claims unassigned rows', async () => { + const { supabase, tables, eqPairs, selects } = mockSupabase( + found({ id: CASH_1930, currency: 'SEK', is_primary: true, ledger_account: '1930' }), + ) + + const scope = await resolveCashAccountScope(supabase, COMPANY_ID) + + expect(scope).toEqual({ + accountNumber: '1930', + currency: 'SEK', + cashAccountId: CASH_1930, + includeUnassigned: true, + found: true, + }) + // One lookup only: the primary fallback must not fire when 1930 resolved. + expect(tables).toEqual(['cash_accounts']) + // Account numbers are strings, never numbers: '1930' must reach the filter + // verbatim, alongside the tenant scope. + expect(eqPairs).toEqual([ + ['company_id', COMPANY_ID], + ['ledger_account', '1930'], + ]) + expect(selects).toEqual(['id, currency, is_primary, ledger_account']) + }) + + it('never claims unassigned rows for a non-primary account', async () => { + // A savings account pulling in the checking account's NULL cash_account_id + // rows is the double-count scopeTransactionsToAccount's includeUnassigned + // flag exists to prevent. + const { supabase } = mockSupabase( + found({ id: CASH_1930, currency: 'SEK', is_primary: false, ledger_account: '1931' }), + ) + + const scope = await resolveCashAccountScope(supabase, COMPANY_ID, '1931') + + expect(scope.includeUnassigned).toBe(false) + expect(scope.cashAccountId).toBe(CASH_1930) + expect(scope.accountNumber).toBe('1931') + expect(scope.found).toBe(true) + }) + + it('carries the account currency so EUR GL movements are not compared with SEK rows', async () => { + const { supabase, eqPairs } = mockSupabase( + found({ id: CASH_1932, currency: 'EUR', is_primary: false, ledger_account: '1932' }), + ) + + const scope = await resolveCashAccountScope(supabase, COMPANY_ID, '1932') + + expect(scope.currency).toBe('EUR') + expect(scope.cashAccountId).toBe(CASH_1932) + expect(eqPairs).toContainEqual(['ledger_account', '1932']) + }) + + it('falls back to the primary cash account when the company has no 1930 row', async () => { + // The worst instance of #1290: 2 prod companies run two SEK cash accounts + // (1935+1936 and 1932+1935) with NO 1930 row and zero journal_entry_lines on + // 1930. Scoping them to '1930' compares their entire SEK bank volume against + // an empty GL side, i.e. a bank_unreconciled blocker worth the whole bank + // balance with 0 unmatched items and nothing the user can do about it. + const { supabase, eqPairs } = mockSupabase([ + { data: null }, + { data: { id: CASH_1935, currency: 'SEK', is_primary: true, ledger_account: '1935' } }, + ]) + + const scope = await resolveCashAccountScope(supabase, COMPANY_ID) + + expect(scope).toEqual({ + accountNumber: '1935', + currency: 'SEK', + cashAccountId: CASH_1935, + includeUnassigned: true, + found: true, + }) + expect(eqPairs).toEqual([ + ['company_id', COMPANY_ID], + ['ledger_account', '1930'], + ['company_id', COMPANY_ID], + ['is_primary', true], + ]) + }) + + it('does not fall back to the primary account when the caller named an account', async () => { + // gnubok_get_reconciliation_status must be able to reject "1931" as unknown. + // Silently answering with the primary account would label the result 1931 + // while reporting a different account's figures. + const { supabase, tables } = mockSupabase([ + { data: null }, + { data: { id: CASH_1935, currency: 'SEK', is_primary: true, ledger_account: '1935' } }, + ]) + + const scope = await resolveCashAccountScope(supabase, COMPANY_ID, '1931') + + expect(scope.found).toBe(false) + expect(scope.accountNumber).toBe('1931') + expect(scope.cashAccountId).toBeUndefined() + expect(tables).toEqual(['cash_accounts']) + }) + + it('falls back to the legacy currency-only scope when the company has no cash accounts at all', async () => { + // Pins the deliberate leniency: companies predating cash_accounts keep the + // exact behaviour they have today rather than losing their bank check. + const { supabase } = mockSupabase([{ data: null }, { data: null }]) + + const scope = await resolveCashAccountScope(supabase, COMPANY_ID) + + expect(scope).toEqual({ + accountNumber: '1930', + currency: 'SEK', + cashAccountId: undefined, + includeUnassigned: true, + found: false, + }) + }) + + it('tolerates null currency / is_primary from the client without widening the scope', async () => { + // Not a reachable DB state: migration 20260519110000 declares both columns + // NOT NULL (is_primary DEFAULT false). This pins the TypeScript-nullable + // shape only, and specifically that an absent is_primary means "do NOT + // claim the unassigned rows" rather than defaulting to the wider filter. + const { supabase } = mockSupabase( + found({ id: CASH_1930, currency: null, is_primary: null, ledger_account: '1930' }), + ) + + const scope = await resolveCashAccountScope(supabase, COMPANY_ID) + + expect(scope.currency).toBe('SEK') + expect(scope.includeUnassigned).toBe(false) + expect(scope.found).toBe(true) + }) + + it('reports found:false when the client resolves without a data key at all', async () => { + // `found` is the shared "no cash_accounts row" contract: callers reject an + // unknown account number on it. A `row !== null` check would report + // undefined as FOUND, so gnubok_get_reconciliation_status would stop + // throwing "Okänt kassakonto 9999" and instead return a status labelled + // 9999 whose bank side is every SEK transaction of the company. + const { supabase } = mockSupabase([{ data: undefined }]) + + const scope = await resolveCashAccountScope(supabase, COMPANY_ID, '9999') + + expect(scope.found).toBe(false) + expect(scope.cashAccountId).toBeUndefined() + }) + + it('fails closed on a lookup error instead of returning the unscoped fallback', async () => { + // A transient failure or an RLS denial would otherwise yield + // cashAccountId: undefined, which is precisely the pooling path #1290 exists + // to remove: the caller would show a phantom difference and call it a + // blocker. This is the contract PR #1295 set for the MCP call site. + const { supabase } = mockSupabase([ + { data: null, error: { code: '57014', message: 'canceling statement' } }, + ]) + + await expect(resolveCashAccountScope(supabase, COMPANY_ID, '1931')).rejects.toThrow( + 'Kunde inte hämta kassakonto 1931', + ) + }) + + it('fails closed when the primary-account fallback lookup errors', async () => { + const { supabase } = mockSupabase([ + { data: null }, + { data: null, error: { code: '57014', message: 'canceling statement' } }, + ]) + + await expect(resolveCashAccountScope(supabase, COMPANY_ID)).rejects.toThrow( + 'Kunde inte hämta företagets primära kassakonto', + ) + }) +}) diff --git a/lib/reconciliation/bank-reconciliation.ts b/lib/reconciliation/bank-reconciliation.ts index bf71db8f..100f687e 100644 --- a/lib/reconciliation/bank-reconciliation.ts +++ b/lib/reconciliation/bank-reconciliation.ts @@ -9,6 +9,9 @@ import { ledgerLineAmountIn, type LedgerLineAmount, } from '@/lib/bookkeeping/ledger-line-amount' +import { createLogger } from '@/lib/logger' + +const log = createLogger('reconciliation.bank') // `ledgerLineAmountIn` moved to lib/bookkeeping/ledger-line-amount.ts verbatim // so the invoice / supplier-invoice voucher matchers share the one rule instead @@ -220,6 +223,21 @@ export interface ReconciliationOptions { * omitted (single-account companies with no row, the '1930' fallback) the pure * currency filter is used and includeUnassigned is moot. * + * Account-scoped callers must resolve the row FIRST (see resolveCashAccountScope + * in lib/reconciliation/cash-account-scope.ts). Omitting cashAccountId is the + * legacy fallback for a company with no cash_accounts row at all; doing it on a + * company that HAS several is issue #1290: the transaction side pools every + * same-currency account while the GL side stays on one account, producing a + * difference with nothing unmatched to point at. + * + * Not every remaining unscoped caller is intentional. The post-sync sweeps in + * app/api/extensions/enable-banking/sync/cron/route.ts and + * extensions/general/enable-banking/index.ts call runReconciliation without a + * scope, which is a known open defect rather than a supported mode: they need + * one run per cash account, not one pooled run. Tracked as issue #1298; + * warnIfUnscopedAcrossCashAccounts below makes both visible in the logs until + * that is fixed. + * * The earlier nested `or(cash_account_id.eq.X,and(cash_account_id.is.null,currency.eq.cur))` * form is intentionally avoided: it silently returned ZERO rows mid-backfill. * A cash account has exactly one currency, so the flat two-term `or` is reliable. @@ -250,6 +268,50 @@ export function scopeTransactionsToAccount r.cash_account_id).filter((id): id is string => Boolean(id)), + ) + if (distinct.size <= 1) return + log.warn(`${operation} ran unscoped across several cash accounts`, { + companyId: ctx.companyId, + operation, + entityType: 'cash_account', + details: { + accountNumber: ctx.accountNumber, + currency: ctx.currency, + distinctCashAccounts: distinct.size, + }, + }) +} + // ============================================================ // In-memory matching: single transaction against GL line pool // ============================================================ @@ -377,6 +439,17 @@ export async function runReconciliation( return query.order('id').range(from, to) }) + // Same diagnostic as the read path, and it matters MORE here: an unscoped run + // matches transactions from every same-currency account against unlinked GL + // lines on accountNumber alone, and (dryRun aside) writes the resulting + // journal_entry_id onto the transaction. + warnIfUnscopedAcrossCashAccounts('runReconciliation', transactions, { + cashAccountId, + companyId, + accountNumber, + currency, + }) + if (transactions.length === 0 || glLines.length === 0) { return { matches: [], applied: 0, errors: 0, skippedBelowThreshold: 0 } } @@ -500,11 +573,12 @@ export async function getReconciliationStatus( journal_entry_id: string | null reconciliation_method: string | null is_ignored: boolean | null + cash_account_id: string | null } const transactions = await fetchAllRows(({ from, to }) => { let txQuery = supabase .from('transactions') - .select('date, amount, journal_entry_id, reconciliation_method, is_ignored') + .select('date, amount, journal_entry_id, reconciliation_method, is_ignored, cash_account_id') .eq('company_id', companyId) txQuery = scopeTransactionsToAccount(txQuery, cashAccountId, currency, includeUnassigned) if (dateFrom) txQuery = txQuery.gte('date', dateFrom) @@ -512,6 +586,13 @@ export async function getReconciliationStatus( return txQuery.order('id').range(from, to) }) + warnIfUnscopedAcrossCashAccounts('getReconciliationStatus', transactions, { + cashAccountId, + companyId, + accountNumber: bankAccount, + currency, + }) + // Get GL bank-account lines. We fetch posted AND reversed entries and count // them TOGETHER: the exact inclusion rule the trial balance and balance sheet // use (see lib/reports/trial-balance.ts, which sums `['posted','reversed']`). diff --git a/lib/reconciliation/cash-account-scope.ts b/lib/reconciliation/cash-account-scope.ts new file mode 100644 index 00000000..cbacd8e7 --- /dev/null +++ b/lib/reconciliation/cash-account-scope.ts @@ -0,0 +1,168 @@ +import type { SupabaseClient } from '@supabase/supabase-js' + +/** + * The settlement account the reconciliation defaults to when a caller does not + * name one. Always a string: BAS codes are identifiers, never numbers. + */ +export const DEFAULT_SETTLEMENT_ACCOUNT = '1930' + +/** + * The trailing arguments getReconciliationStatus / runReconciliation need in + * order to reconcile ONE cash account instead of every same-currency one. + */ +export interface CashAccountScope { + /** + * The BAS account the GL side is filtered on. Equals the requested account + * when one was found, the primary cash account's ledger_account when the + * default '1930' had no row, and '1930' when nothing resolved at all. + */ + accountNumber: string + currency: string + cashAccountId: string | undefined + includeUnassigned: boolean + /** + * false = no cash_accounts row was resolved. That means exactly one thing + * here: the company genuinely has no such row. A failed lookup THROWS (see + * below), it never degrades into found: false. + */ + found: boolean +} + +interface CashAccountRow { + id: string + currency: string | null + is_primary: boolean | null + ledger_account: string +} + +/** + * Boolean(), not `row !== null`: a client that resolves without a `data` key + * leaves row undefined, which `!== null` would report as FOUND. Callers use + * `found` to reject an unknown account number, so a truthy-by-accident value + * would turn "Okänt kassakonto 9999" into a status labelled 9999 whose bank + * side is every SEK transaction of the company. + */ +function toScope(row: CashAccountRow | null | undefined, accountNumber: string): CashAccountScope { + return { + accountNumber: row?.ledger_account ?? accountNumber, + currency: row?.currency ?? 'SEK', + cashAccountId: row?.id, + // Only the company's primary cash account claims rows with a NULL + // cash_account_id; moot when no row exists, since scopeTransactionsToAccount + // ignores the flag on the unscoped path. + // + // Known residual, NOT closed by #1290 and tracked as issue #1299: on a + // primary row this still pulls every booked NULL-cash_account_id transaction + // into the bank total, including ones whose verifikat has no line on this + // account at all (own-account transfers the cash_account_id backfill + // skipped). Measured on prod 2026-07-30 over all time, for the companies + // with >= 2 SEK cash accounts and a primary 1930: 294 booked + // NULL-cash_account_id SEK rows, 22 of them on verifikat with no 1930 line, + // across 4 companies, net -4170.31 kr but with monthly swings from + // -18055.82 kr (2026-07) to +38086.00 kr (2026-06). Those have no + // counterpart on the GL side, so a smaller phantom difference survives for + // companies with unbackfilled rows, still with count 0. Fixing it means + // finishing the cash_account_id backfill, not widening or narrowing this + // flag. + includeUnassigned: row ? Boolean(row.is_primary) : true, + found: Boolean(row), + } +} + +async function lookupByLedgerAccount( + supabase: SupabaseClient, + companyId: string, + accountNumber: string, +): Promise { + // (company_id, ledger_account) is UNIQUE (migration 20260519110000), so + // maybeSingle is exact and index-backed. + const { data, error } = await supabase + .from('cash_accounts') + .select('id, currency, is_primary, ledger_account') + .eq('company_id', companyId) + .eq('ledger_account', accountNumber) + .maybeSingle() + + // Fail CLOSED. A swallowed error yields cashAccountId: undefined, which is + // exactly the unscoped pooling path this helper exists to remove, so a + // transient DB failure or an RLS denial would silently re-create the #1290 + // phantom difference and hand the user a blocker with nothing to match. + // Throwing is the contract PR #1295 established for the MCP call site and it + // now holds for every caller. + if (error) throw new Error(`Kunde inte hämta kassakonto ${accountNumber}`) + + return data as CashAccountRow | null | undefined +} + +async function lookupPrimary( + supabase: SupabaseClient, + companyId: string, +): Promise { + const { data, error } = await supabase + .from('cash_accounts') + .select('id, currency, is_primary, ledger_account') + .eq('company_id', companyId) + .eq('is_primary', true) + .maybeSingle() + + if (error) throw new Error('Kunde inte hämta företagets primära kassakonto') + + return data as CashAccountRow | null | undefined +} + +/** + * Resolve a settlement account to its cash_accounts row and return the scope + * arguments the reconciliation functions take. + * + * Why this is shared: calling getReconciliationStatus with only + * (supabase, companyId, from, to) leaves cashAccountId undefined, and + * scopeTransactionsToAccount then falls back to a currency-only filter. The + * bank side sums EVERY SEK cash account while the GL side stays on one account, + * so a company with a savings account gets a nonzero difference with zero + * unmatched transactions and zero unmatched GL lines to point at (#1290). + * + * Two resolution modes, deliberately different: + * + * - `accountNumber` given (a caller naming an account, e.g. the MCP tool's + * account_number argument): resolve exactly that account, no fallback. An + * unknown account must stay unknown so callers can reject it. + * - `accountNumber` omitted (the checks that just want "the company's bank"): + * try '1930', and if the company has no 1930 row fall back to its PRIMARY + * cash account. Without that fallback the caller compares the company's whole + * SEK bank volume against an empty 1930 GL side: measured on prod, 2 companies + * have no 1930 row while running two SEK cash accounts each (1935+1936 and + * 1932+1935) and zero journal_entry_lines on 1930, so they got a + * bank_unreconciled blocker worth their entire bank volume with 0 unmatched + * items. cash_accounts allows at most one primary row per company (partial + * unique index idx_cash_accounts_one_primary_per_company, migration + * 20260519110000), so maybeSingle is exact here too. For the 1367 companies + * that do have a 1930 row the fallback never runs, so nothing changes for + * them, including the 5 whose 1930 row is not the primary one. + * + * Lookup failures throw; they never degrade into the unscoped path. + * + * NOT a description of every caller in the codebase: the hand-rolled lookups in + * app/api/reconciliation/bank/status/route.ts and .../run/route.ts use + * `Boolean(cashAccount?.is_primary)`, i.e. includeUnassigned = FALSE when no row + * exists, where this helper says true. The difference is inert (with no row + * there is no cashAccountId, and scopeTransactionsToAccount ignores the flag on + * that path), and those routes are deliberately left alone here to keep this + * change scoped to #1290. + */ +export async function resolveCashAccountScope( + supabase: SupabaseClient, + companyId: string, + accountNumber?: string, +): Promise { + const requested = accountNumber ?? DEFAULT_SETTLEMENT_ACCOUNT + + const row = await lookupByLedgerAccount(supabase, companyId, requested) + if (row) return toScope(row, requested) + + if (accountNumber === undefined) { + const primary = await lookupPrimary(supabase, companyId) + if (primary) return toScope(primary, requested) + } + + return toScope(null, requested) +}