fix(recon): share the cash-account scope and cover the no-1930 case (#1309)
PR #1295 (144cc514) fixed computeVatCloseCheck on main while this branch was
fixing it a second, different way. This rebuilds the branch on top of that
merge instead of re-landing the duplicate: main's local
getScopedReconciliationStatus stays as the MCP entry point, its lookup body
moves into lib/reconciliation/cash-account-scope.ts, and the pieces main does
not have are added on top.
Why the lookup has to live in lib/: the bokslut readiness aggregator has the
same defect and is core code, which must never import from @/extensions/. It
called getReconciliationStatus with 4 positional args, so cashAccountId stayed
undefined and scopeTransactionsToAccount fell through to its currency-only
filter: the bank side summed every SEK cash account while the GL side stayed on
1930, and the wizard reported "Bankavstamningen visar en differens" with zero
unmatched transactions and zero unmatched GL lines to point at. Same shape as
the MCP blocker in #1290, different surface.
Decisions taken deliberately, not by taking 'ours':
1. Lookup errors fail CLOSED, everywhere. resolveCashAccountScope throws
"Kunde inte hamta kassakonto <n>" instead of returning the unscoped
fallback. The earlier version on this branch logged and fell back, which
turned a transient DB error or an RLS denial straight back into the #1290
pooling path. Main's contract wins; its merged test asserting exactly that
still passes untouched.
2. The default resolution no longer hard-codes 1930. With no account_number
argument the resolver tries 1930 and, only if the company has no such row,
falls back to its primary cash account. Measured read-only on prod
2026-07-30: 2 companies have no 1930 cash_accounts row while running two SEK
cash accounts each and zero journal_entry_lines on 1930, so the check
compared their entire SEK bank volume against an empty GL side, i.e. a
high-severity bank_unreconciled blocker with count 0 that no user action
could clear. Roughly 20x the difference the issue reported. A caller that
NAMES an account gets no fallback, so gnubok_get_reconciliation_status still
rejects "Okant kassakonto 9999" rather than silently answering about a
different account. 1367 companies have a 1930 row and are unaffected,
including the 5 whose 1930 row is not the primary one.
3. The blocker message names the resolved account instead of a literal 1930:
pointing a user at 1930 when the reconciliation ran on 1935 sends them to an
account with no lines on it.
4. warnIfUnscopedAcrossCashAccounts logs a warning when a run left
cashAccountId undefined AND the rows it fetched really do span more than one
cash account. Kept on the write path too: an unscoped runReconciliation can
persist a wrong journal_entry_id, which does not clear itself later.
Duplicate regression suite collapsed: the branch's
vat-close-check-bank-scope.test.ts overlapped main's
vat-close-check-reconciliation-scope.test.ts case for case, so only the cases
main lacked were merged into main's file (the primary-account fallback, the
message naming the resolved account, the blocker still firing on a genuine
scoped difference, and the tool handler's own scope resolution).
Residuals are now tracked issues, not code comments:
- #1298: the post-sync runReconciliation sweeps in
app/api/extensions/enable-banking/sync/cron/route.ts and
extensions/general/enable-banking/index.ts still run unscoped. They are write
paths.
- #1299: booked transactions with a NULL cash_account_id whose verifikat has no
line on the primary account still inflate the bank total. Measured all-time
on prod: 294 booked NULL rows, 22 of them on verifikat with no 1930 line,
4 companies, net -4170.31 kr with monthly swings from -18055.82 kr to
+38086.00 kr. Fixing it means finishing the cash_account_id backfill.
Also: the file-global logger mock in bank-reconciliation.test.ts now wraps the
real module and swaps only warn, instead of substituting a four-method stub
whose child() returned undefined for the entire module graph of that suite.
Verified: npx vitest run over lib/reconciliation, lib/bokslut,
extensions/general/mcp-server, app/api/reconciliation, app/api/extensions,
app/api/v1, app/api/bookkeeping, app/api/transactions, lib/pending-operations,
lib/bookkeeping, lib/invoices, lib/transactions, lib/reports: all green.
eslint on the 8 changed files: 0 errors (18 pre-existing unused-import
warnings in server.ts). tsc --noEmit: 405 errors, byte-identical to the
origin/main baseline. check:guards passes. No migration, so no pg-real test.
Fixes #1290
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
co-authored by
Claude Opus 5
Claude Opus 5
parent
9bb8b40420
commit
4a38fa30ed
+219
-14
@@ -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<CashAccountFixture | null>,
|
||||
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<string, unknown> => {
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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
|
||||
},
|
||||
},
|
||||
|
||||
|
||||
@@ -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<typeof vi.fn>
|
||||
eq: ReturnType<typeof vi.fn>
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<typeof import('@/lib/logger')>()
|
||||
return {
|
||||
...actual,
|
||||
createLogger: (module: string, base?: Parameters<typeof actual.createLogger>[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<object> = {
|
||||
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()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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',
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -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<Q extends {
|
||||
return query.eq('currency', currency)
|
||||
}
|
||||
|
||||
/**
|
||||
* Emit a warning when a run left cashAccountId undefined AND the rows it
|
||||
* fetched really do span more than one cash account.
|
||||
*
|
||||
* That combination is the #1290 shape: the transaction side pools every
|
||||
* same-currency account while the GL side stays on a single accountNumber. On a
|
||||
* read (getReconciliationStatus) it manufactures a difference with nothing
|
||||
* unmatched behind it; on a WRITE (runReconciliation, which applies matches
|
||||
* unless dryRun) it can auto-link a savings-account transaction to an unlinked
|
||||
* 1930 voucher, i.e. persist a wrong journal_entry_id.
|
||||
*
|
||||
* It warns rather than throwing because single-account companies with no
|
||||
* cash_accounts row still legitimately take the currency-only path, and they
|
||||
* never trip this condition (0 or 1 distinct id). The fix for anything this
|
||||
* logs is always at the CALL SITE: resolve the row with resolveCashAccountScope
|
||||
* (lib/reconciliation/cash-account-scope.ts) and pass the scope through.
|
||||
*/
|
||||
function warnIfUnscopedAcrossCashAccounts(
|
||||
operation: string,
|
||||
rows: { cash_account_id?: string | null }[],
|
||||
ctx: {
|
||||
cashAccountId: string | undefined
|
||||
companyId: string
|
||||
accountNumber: string
|
||||
currency: string
|
||||
},
|
||||
): void {
|
||||
if (ctx.cashAccountId) return
|
||||
const distinct = new Set(
|
||||
rows.map((r) => 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<StatusTxRow>(({ 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']`).
|
||||
|
||||
@@ -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<CashAccountRow | null | undefined> {
|
||||
// (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<CashAccountRow | null | undefined> {
|
||||
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<CashAccountScope> {
|
||||
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)
|
||||
}
|
||||
Reference in New Issue
Block a user