From f8033cb32d0b0d88f54ce230242a0327efc786f4 Mon Sep 17 00:00:00 2001 From: Alexander Reinthal Date: Thu, 16 Jul 2026 16:15:06 +0200 Subject: [PATCH] fix(transactions): bind manually-fed transactions to a cash account (#1016) (#1017) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(transactions): bind manually-fed transactions to a cash account (#1016) create_transactions inserted rows with cash_account_id = null, so ledger accounts fed via MCP/CSV without a PSD2 feed (e.g. 1935 Wise SEK) had no kassakonto: get_reconciliation_status 404'd with "Okänt kassakonto" and the "Matcha mot befintlig verifikation" dialog fell back to 1930. No schema change: cash_accounts.bank_connection_id is already nullable and source='manual' already exists (every company is seeded a manual 1930). This is the creation-side leg of the #985-#987 root cause: the resolution chain was fixed, but manually-fed accounts never got the cash_account_id link. - Add ensureManualCashAccount (lib/cash-accounts/service.ts): find-or-create a manual (source='manual', bank_connection_id=null) cash_accounts row for a ledger slot, tolerating the (company_id, ledger_account) UNIQUE race. - Add an optional ledger_account hint (^19xx) to gnubok_create_transactions; commitCreateTransaction resolves/creates the manual account and sets cash_account_id on the inserted row. Reconciliation and voucher matching then resolve the real account unchanged. Forward-looking; historical cash_account_id=null remediation stays in #1001. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Alexander Reinthal * fix(cash-accounts): guard ensureManualCashAccount against currency mismatch (CodeRabbit #1017) The existing-row lookup matched only on (company_id, ledger_account) and returned the row id ignoring currency, so a SEK transaction hinting at a ledger already claimed for USD would bind to the wrong-currency cash account. Since that pair is UNIQUE (one currency per ledger), a mismatch is a real conflict: throw instead of silently mis-binding. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Alexander Reinthal --------- Signed-off-by: Alexander Reinthal Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: Jakob Wennberg --- DECISIONS.md | 1 + extensions/general/mcp-server/server.ts | 8 ++ lib/cash-accounts/__tests__/service.test.ts | 108 ++++++++++++++++++ lib/cash-accounts/service.ts | 83 ++++++++++++++ .../__tests__/executors.test.ts | 47 ++++++++ lib/pending-operations/commit.ts | 15 +++ 6 files changed, 262 insertions(+) diff --git a/DECISIONS.md b/DECISIONS.md index a70b8d31..9edd3624 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -167,3 +167,4 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-07-16] Two bugs from one customer report (an AB). Bug 1 (acct 2893 showed 2393's "langfristig del" memo after an andringsverifikation): root cause = CorrectionEntryDialog never re-derived line_description on account change (JournalEntryForm does). Fixed forward via a pure helper (correction-line-description.ts) that refreshes the memo only when it is empty or still equals the prev account's name (preserves hand-typed memos). Chose NO prod data repair: the wrong memo sits on a POSTED verifikat (immutable per migration-017 trigger); it is cosmetic (account number + amounts correct, all reports key off the number); ~26 posted lines across 11 cos share this stale-echo pattern, all fix-forward only. Deferred the twin entry-level header fix (#1031). Bug 2 (auto tax-deadlines never appeared): root cause = generation only fired on a settings save where a TAX field CHANGED value (didTaxFieldsChange); settings are filled once at onboarding so re-saving generated nothing -> only 5/776 real cos had system deadlines. Chose count-based self-heal (regenerate when the company has 0 system deadlines) over always-regenerate, because generateTaxDeadlinesForUser deletes+reinserts and would reset is_completed/status on every unrelated save. Also wired the /deadlines empty-state to the existing (dead) /api/tax-deadlines/generate route, and fixed a 1000-row PostgREST cap in the annual cron. Backfilled 771 real cos with zero system deadlines via scripts/backfill-tax-deadlines.ts. Deferred moms_period=yearly config (#1030, 295 filers, largest VAT cohort): helarsmoms deadline (SFL 26 kap. 33-33b) depends on EU-trade status (no flag in CompanySettingsForDeadlines) and, for AB, the income-tax-return date. [2026-07-15] Repaired the single legacy paid credit note blocking invoices_credit_note_not_paid validation by normalizing its invoice metadata to sent, clearing payment fields, setting zero payable remainder, and linking its existing balanced posted V44 reversal: the immutable voucher already exactly reversed V42 and was not edited or duplicated. +[2026-07-14] Issue #1016 (create_transactions never binds cash_account_id): fixed forward-only via an optional ledger_account hint on gnubok_create_transactions + commitCreateTransaction, resolved through a new ensureManualCashAccount find-or-create helper (lib/cash-accounts/service.ts). No migration: cash_accounts.bank_connection_id is already nullable and source='manual' already exists (the seeded 1930 is a manual row), so the handoff's premise that manual kassakonton need a schema change was wrong. Pre-creating a manual row does NOT race upsertFromPsd2 (the ingest.ts "never auto-create" worry): a later PSD2 connection promotes a manual holder in place, the intended flow (#916/#56). Hint restricted to ^19\d{2}$ (BAS kassa/bank group) so a transaction can't bind to a non-cash account. Scoped to the MCP create path per user decision; POST /api/cash-accounts + settings UI, relaxing ingest.ts's settlement_account auto-create, and historical backfill of cash_account_id=null rows (deferred to #1001) are follow-ups. diff --git a/extensions/general/mcp-server/server.ts b/extensions/general/mcp-server/server.ts index 48510d01..c5143ffb 100644 --- a/extensions/general/mcp-server/server.ts +++ b/extensions/general/mcp-server/server.ts @@ -2831,6 +2831,7 @@ export const tools: McpTool[] = [ amount: { type: 'number', description: 'Positive = income, negative = expense.' }, description: { type: 'string', description: 'Free-text description shown in /transactions.' }, currency: { type: 'string', description: 'ISO 4217 code. Default SEK.' }, + ledger_account: { type: 'string', description: 'Optional BAS 19xx cash account (e.g. "1935") this row settles on. Binds it to a manual kassakonto so reconciliation and voucher matching resolve the right account instead of falling back to 1930.' }, bank_connection_id: { type: 'string', description: 'Optional UUID of a bank_connections row to associate with.' }, external_id: { type: 'string', description: 'Optional external reference (e.g., Airtable record ID). Shown in the preview; the DB enforces uniqueness per user, so the second commit of the same external_id will fail at approval.' }, }, @@ -2862,6 +2863,7 @@ export const tools: McpTool[] = [ const amount = Number(item.amount) const description = ((item.description as string) ?? '').trim() const currency = ((item.currency as string) || 'SEK').toUpperCase() + const ledgerAccount = (item.ledger_account as string) || null const bankConnectionId = (item.bank_connection_id as string) || null const externalId = (item.external_id as string) || null @@ -2874,12 +2876,18 @@ export const tools: McpTool[] = [ if (!description) { throw new Error(`transactions[${i}].description is required.`) } + // Restrict the hint to BAS group 19 (kassa/bank): binding a transaction + // to a non-cash account would misroute reconciliation and matching. + if (ledgerAccount && !/^19\d{2}$/.test(ledgerAccount)) { + throw new Error(`transactions[${i}].ledger_account must be a BAS 19xx cash account (e.g. "1935").`) + } const params = { date, amount, description, currency, + ledger_account: ledgerAccount, bank_connection_id: bankConnectionId, external_id: externalId, } diff --git a/lib/cash-accounts/__tests__/service.test.ts b/lib/cash-accounts/__tests__/service.test.ts index 50eaebf8..156b0910 100644 --- a/lib/cash-accounts/__tests__/service.test.ts +++ b/lib/cash-accounts/__tests__/service.test.ts @@ -16,6 +16,7 @@ import { defaultLedgerForCurrency, getRevokedConnectionIds, upsertFromPsd2, + ensureManualCashAccount, } from '../service' type CashRow = { ledger_account: string; bank_connection_id: string | null } @@ -556,3 +557,110 @@ describe('upsertFromPsd2', () => { ).rejects.toThrow(/duplicate key/) }) }) + +// ── ensureManualCashAccount ────────────────────────────────────────────── + +interface ManualStub { + lookup: { data: { id: string; currency?: string } | null; error?: { message: string } | null } + insert?: { data: { id: string } | null; error?: { message: string; code?: string } | null } + reread?: { data: { id: string } | null; error?: { message: string } | null } + inserted: Array> + lookupCount: number +} + +function makeManualSupabase(stub: ManualStub): SupabaseClient { + return { + from: vi.fn((table: string) => { + expect(table).toBe('cash_accounts') + return { + // lookup / reread path: select().eq().eq().maybeSingle() + select: vi.fn(() => ({ + eq: vi.fn(() => ({ + eq: vi.fn(() => ({ + maybeSingle: vi.fn(() => { + stub.lookupCount += 1 + // First maybeSingle = initial lookup; a second = post-23505 reread. + const r = stub.lookupCount === 1 ? stub.lookup : stub.reread ?? { data: null } + return Promise.resolve({ data: r.data, error: r.error ?? null }) + }), + })), + })), + })), + // insert().select('id').single() + insert: vi.fn((payload: Record) => { + stub.inserted.push(payload) + return { + select: vi.fn(() => ({ + single: vi.fn(() => + Promise.resolve({ + data: stub.insert?.data ?? null, + error: stub.insert?.error ?? null, + }), + ), + })), + } + }), + } + }), + } as unknown as SupabaseClient +} + +describe('ensureManualCashAccount', () => { + it('returns the existing row id without inserting when the currency matches', async () => { + const stub: ManualStub = { lookup: { data: { id: 'ca-1', currency: 'SEK' } }, inserted: [], lookupCount: 0 } + const id = await ensureManualCashAccount(makeManualSupabase(stub), 'c1', '1935', 'sek') + expect(id).toBe('ca-1') + expect(stub.inserted).toHaveLength(0) + }) + + it('throws when the existing row is a different currency (UNIQUE ledger conflict)', async () => { + const stub: ManualStub = { lookup: { data: { id: 'ca-usd', currency: 'USD' } }, inserted: [], lookupCount: 0 } + await expect( + ensureManualCashAccount(makeManualSupabase(stub), 'c1', '1935', 'SEK'), + ).rejects.toThrow(/denominated in USD, not SEK/) + expect(stub.inserted).toHaveLength(0) + }) + + it('creates a manual row (source=manual, uppercased currency) when none exists', async () => { + const stub: ManualStub = { + lookup: { data: null }, + insert: { data: { id: 'ca-new' } }, + inserted: [], + lookupCount: 0, + } + const id = await ensureManualCashAccount(makeManualSupabase(stub), 'c1', '1935', 'sek') + expect(id).toBe('ca-new') + expect(stub.inserted[0]).toMatchObject({ + company_id: 'c1', + ledger_account: '1935', + currency: 'SEK', + source: 'manual', + is_primary: false, + enabled: true, + }) + }) + + it('re-reads the winner on a 23505 race instead of throwing', async () => { + const stub: ManualStub = { + lookup: { data: null }, + insert: { data: null, error: { message: 'duplicate key', code: '23505' } }, + reread: { data: { id: 'ca-winner' } }, + inserted: [], + lookupCount: 0, + } + const id = await ensureManualCashAccount(makeManualSupabase(stub), 'c1', '1935', 'SEK') + expect(id).toBe('ca-winner') + }) + + it('throws on a non-race insert failure', async () => { + const stub: ManualStub = { + lookup: { data: null }, + insert: { data: null, error: { message: 'boom' } }, + inserted: [], + lookupCount: 0, + } + await expect( + ensureManualCashAccount(makeManualSupabase(stub), 'c1', '1935', 'SEK'), + ).rejects.toThrow(/boom/) + }) +}) diff --git a/lib/cash-accounts/service.ts b/lib/cash-accounts/service.ts index 472b2149..2ea68e57 100644 --- a/lib/cash-accounts/service.ts +++ b/lib/cash-accounts/service.ts @@ -486,6 +486,89 @@ export async function upsertFromPsd2( } } +/** + * Find (or create) a manual cash account for a BAS ledger slot, so transactions + * ingested outside the PSD2 flow (create_transactions / CSV) can carry a real + * cash_account_id instead of NULL. Without the link, reconciliation 404s on the + * account and the match dialog falls back to 1930 (issue #1016). + * + * Manual rows (source='manual', bank_connection_id=null) are already first-class: + * every company is seeded a manual 1930 the same way, and upsertFromPsd2 promotes + * a manual holder in place if a bank later claims the slot. So pre-creating one + * here does NOT race the PSD2 sync (the concern noted in lib/transactions/ingest.ts): + * the worst case is a later connection promoting this row, which is the intended flow. + * + * Keyed on the (company_id, ledger_account) UNIQUE constraint: a concurrent + * insert surfaces as 23505, which we treat as "someone else won the race" and + * re-read. The row's currency follows the first transaction that created it; a + * ledger account holds one currency by that same constraint. + */ +export async function ensureManualCashAccount( + supabase: SupabaseClient, + companyId: string, + ledgerAccount: string, + currency: string, + name?: string | null, +): Promise { + const existing = await supabase + .from('cash_accounts') + .select('id, currency') + .eq('company_id', companyId) + .eq('ledger_account', ledgerAccount) + .maybeSingle() + if (existing.error) { + throw new Error(`ensureManualCashAccount lookup failed: ${existing.error.message}`) + } + if (existing.data) { + const row = existing.data as { id: string; currency: string | null } + // (company_id, ledger_account) is UNIQUE, so a ledger holds exactly one + // currency. A different-currency transaction pointing at the same ledger is + // a real conflict (e.g. a SEK row landing on a ledger already claimed for + // USD): fail loudly instead of binding it to the wrong-currency account. + if (row.currency && row.currency.toUpperCase() !== currency.toUpperCase()) { + throw new Error( + `Cash account ${ledgerAccount} is denominated in ${row.currency}, not ${currency.toUpperCase()}`, + ) + } + return row.id + } + + const insert = await supabase + .from('cash_accounts') + .insert({ + company_id: companyId, + ledger_account: ledgerAccount, + currency: currency.toUpperCase(), + name: name?.trim() || `Bankkonto ${currency.toUpperCase()}`, + enabled: true, + is_primary: false, + source: 'manual' as CashAccountSource, + }) + .select('id') + .single() + + if (insert.error) { + // Lost the (company_id, ledger_account) race: re-read the winner's row. + if (insert.error.code === '23505') { + const reread = await supabase + .from('cash_accounts') + .select('id') + .eq('company_id', companyId) + .eq('ledger_account', ledgerAccount) + .maybeSingle() + if (reread.data) return (reread.data as { id: string }).id + } + log.error('ensureManualCashAccount insert failed', { + companyId, + ledgerAccount, + error: insert.error.message, + }) + throw new Error(`ensureManualCashAccount insert failed: ${insert.error.message}`) + } + + return (insert.data as { id: string }).id +} + /** * Toggle a cash account's enabled flag. Used by the AccountPicker when a user * opts in or out of syncing a particular PSD2 account. diff --git a/lib/pending-operations/__tests__/executors.test.ts b/lib/pending-operations/__tests__/executors.test.ts index a9c2b921..9c4fdf88 100644 --- a/lib/pending-operations/__tests__/executors.test.ts +++ b/lib/pending-operations/__tests__/executors.test.ts @@ -304,6 +304,53 @@ describe('commitPendingOperation: create_transaction', () => { expect(result.http_status).toBe(409) expect(result.error).toMatch(/already exists/) }) + + it('binds cash_account_id when ledger_account is given, creating the manual account', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim + enqueue({ data: null, error: null }) // ensureManualCashAccount lookup miss + enqueue({ data: { id: 'ca-1935' }, error: null }) // ensureManualCashAccount insert + enqueue({ data: { id: 'tx-9' }, error: null }) // executor transactions insert + enqueue({ data: null, error: null }) // dispatcher's update + + const op = makePendingOp({ + operation_type: 'create_transaction', + params: { + date: '2026-05-01', + amount: -200, + description: 'WISE KORT ST', + currency: 'SEK', + ledger_account: '1935', + }, + }) + + const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op) + + expect(result.status).toBe('committed') + expect(result.data).toMatchObject({ transaction_id: 'tx-9' }) + }) + + it('rejects a non-19xx ledger_account with 400', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: { id: 'op-1' }, error: null }) // CAS claim + enqueue({ data: null, error: null }) // dispatcher's reject update + + const op = makePendingOp({ + operation_type: 'create_transaction', + params: { + date: '2026-05-01', + amount: -200, + description: 'WISE KORT ST', + ledger_account: '3001', + }, + }) + + const result = await commitPendingOperation(supabase as never, 'user-1', 'company-1', op) + + expect(result.status).toBe('failed') + expect(result.http_status).toBe(400) + expect(result.error).toMatch(/19xx cash account/) + }) }) // ─── import_sie ───────────────────────────────────────────────────── diff --git a/lib/pending-operations/commit.ts b/lib/pending-operations/commit.ts index cdf5f921..81c95a25 100644 --- a/lib/pending-operations/commit.ts +++ b/lib/pending-operations/commit.ts @@ -26,6 +26,7 @@ import { createCreditNoteJournalEntry, } from '@/lib/bookkeeping/invoice-entries' import { resolveSettlementAccount } from '@/lib/bookkeeping/settlement-account' +import { ensureManualCashAccount } from '@/lib/cash-accounts/service' import { createJournalEntry, findFiscalPeriod, reverseEntry, validateBalance } from '@/lib/bookkeeping/engine' import { coerceDimensionsBag } from '@/lib/bookkeeping/dimension-resolver' import { cancelOrphanedPaymentEntry } from '@/lib/bookkeeping/cancel-orphaned-entry' @@ -648,12 +649,25 @@ async function commitCreateTransaction( const amount = Number(params.amount) const description = (params.description as string) ?? '' const currency = ((params.currency as string) || 'SEK') as Currency + const ledgerAccount = (params.ledger_account as string) || null const bankConnectionId = (params.bank_connection_id as string) || null const externalId = (params.external_id as string) || null if (!date || !description.trim() || !Number.isFinite(amount)) { return { error: 'date, description, and amount are required', status: 400 } } + if (ledgerAccount && !/^19\d{2}$/.test(ledgerAccount)) { + return { error: 'ledger_account must be a BAS 19xx cash account', status: 400 } + } + + // Bind the row to a manual kassakonto when a ledger account is given, so + // reconciliation and voucher matching resolve the real account instead of + // falling back to 1930 (issue #1016). Find-or-create; the row's currency + // follows this transaction. + let cashAccountId: string | null = null + if (ledgerAccount) { + cashAccountId = await ensureManualCashAccount(supabase, companyId, ledgerAccount, currency) + } const { data, error } = await supabase .from('transactions') @@ -661,6 +675,7 @@ async function commitCreateTransaction( user_id: userId, company_id: companyId, bank_connection_id: bankConnectionId, + cash_account_id: cashAccountId, external_id: externalId, date, description: description.trim(),