diff --git a/extensions/general/mcp-server/__tests__/categorize-category-required.test.ts b/extensions/general/mcp-server/__tests__/categorize-category-required.test.ts new file mode 100644 index 00000000..b7f301bc --- /dev/null +++ b/extensions/general/mcp-server/__tests__/categorize-category-required.test.ts @@ -0,0 +1,172 @@ +/** + * gnubok_categorize_transaction / gnubok_bulk_book_inbox_items: the `category` + * argument is required, and hosts don't always enforce inputSchema `required`. + * + * Issue #1662: a call carrying only account_override reached the enum check + * and surfaced as 'Invalid category "undefined"'. These tests pin: + * - missing category -> a clear "category is required" error (before DB work), + * - an unknown category string -> the enum error, unchanged, + * - the happy path stages with the category intact. + * Companion suite: categorize-account-override.test.ts. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { createQueuedMockSupabase } from '@/tests/helpers' +import { eventBus } from '@/lib/events' + +const mockDetectDup = vi.fn() +vi.mock('@/lib/transactions/booking-duplicate-detection', () => ({ + detectBookingDuplicate: (...args: unknown[]) => mockDetectDup(...args), +})) + +import { tools } from '../server' + +const categorize = tools.find((t) => t.name === 'gnubok_categorize_transaction')! +const bulkBookInbox = tools.find((t) => t.name === 'gnubok_bulk_book_inbox_items')! + +const TX_ID = '00000000-0000-4000-8000-0000000000cc' + +/** `transactions` row for categorizeTransactionCore's select('*'). Synthetic. */ +const coreTxRow = () => ({ + id: TX_ID, + date: '2026-07-10', + amount: -479, + currency: 'SEK', + amount_sek: -479, + exchange_rate: 1, + description: 'SECOND HAND BUTIK', + merchant_name: null, + cash_account_id: null, + document_id: null, + journal_entry_id: null, + is_business: true, +}) + +/** The narrower projection the tool re-fetches for the guard + title. */ +const guardTxRow = () => ({ + description: 'SECOND HAND BUTIK', + merchant_name: null, + amount: -479, + currency: 'SEK', + amount_sek: -479, + exchange_rate: 1, + date: '2026-07-10', + cash_account_id: null, +}) + +const settingsRow = { entity_type: 'aktiebolag', fiscal_year_start_month: 1 } + +beforeEach(() => { + vi.clearAllMocks() + eventBus.clear() + mockDetectDup.mockResolvedValue(null) +}) + +describe('gnubok_categorize_transaction: category presence guard (#1662)', () => { + it('declares category as required in the input schema', () => { + const schema = categorize.inputSchema as { required?: string[] } + expect(schema.required).toContain('category') + }) + + it('rejects a call with only account_override with a clear "category is required" error', async () => { + const { supabase, calls } = createQueuedMockSupabase() + let thrown: unknown + try { + await categorize.execute( + { transaction_id: TX_ID, account_override: '4020' }, + 'company-1', + 'user-1', + supabase as never, + ) + } catch (err) { + thrown = err + } + expect(thrown).toBeInstanceOf(Error) + const message = (thrown as Error).message + expect(message).toMatch( + /^category is required; account_override only overrides the category's default account/, + ) + // Never the old 'Invalid category "undefined"' shape, and no DB work. + expect(message).not.toMatch(/undefined/) + expect(calls).toHaveLength(0) + }) + + it('rejects a non-string or blank category the same way', async () => { + const { supabase } = createQueuedMockSupabase() + for (const category of [42, '', ' ', null]) { + await expect( + categorize.execute( + { transaction_id: TX_ID, category }, + 'company-1', + 'user-1', + supabase as never, + ), + ).rejects.toThrow(/category is required/) + } + }) + + it('still returns the enum error for an unknown category string', async () => { + const { supabase } = createQueuedMockSupabase() + await expect( + categorize.execute( + { transaction_id: TX_ID, category: 'expense_unicorns' }, + 'company-1', + 'user-1', + supabase as never, + ), + ).rejects.toThrow(/Invalid category "expense_unicorns"\. Valid categories:/) + }) + + it('happy path: a valid category stages with the category intact', async () => { + const { supabase, enqueue, findCall } = createQueuedMockSupabase() + enqueue({ data: coreTxRow() }) // core: transactions + enqueue({ data: settingsRow }) // core: company_settings + enqueue({ data: guardTxRow() }) // tool: transactions re-fetch + enqueue({ data: null }) // resolvePeriodStatusForDate: company_settings + enqueue({ data: null }) // resolvePeriodStatusForDate: fiscal_periods + enqueue({ data: { id: 'op-cat-1' } }) // pending_operations insert + + const result = (await categorize.execute( + { transaction_id: TX_ID, category: 'expense_other' }, + 'company-1', + 'user-1', + supabase as never, + { type: 'api_key' }, + )) as { staged: boolean; operation_id?: string; preview: Record } + + expect(result.staged).toBe(true) + expect(result.operation_id).toBe('op-cat-1') + expect(result.preview.category).toBe('expense_other') + + const insertArgs = findCall('pending_operations', 'insert') + expect(insertArgs).toBeDefined() + const payload = (insertArgs as unknown[])[0] as { params?: { category?: string } } + expect(payload.params?.category).toBe('expense_other') + }) +}) + +describe('gnubok_bulk_book_inbox_items: category guard at staging', () => { + it('rejects a missing category before any DB work', async () => { + const { supabase, calls } = createQueuedMockSupabase() + await expect( + bulkBookInbox.execute( + { item_ids: ['i1'] }, + 'company-1', + 'user-1', + supabase as never, + ), + ).rejects.toThrow(/^category is required/) + expect(calls).toHaveLength(0) + }) + + it('rejects an unknown category string with the enum error', async () => { + const { supabase } = createQueuedMockSupabase() + await expect( + bulkBookInbox.execute( + { item_ids: ['i1'], category: 'expense_unicorns' }, + 'company-1', + 'user-1', + supabase as never, + ), + ).rejects.toThrow(/Invalid category "expense_unicorns"/) + }) +}) diff --git a/extensions/general/mcp-server/server.ts b/extensions/general/mcp-server/server.ts index a64b9095..7e4b1360 100644 --- a/extensions/general/mcp-server/server.ts +++ b/extensions/general/mcp-server/server.ts @@ -4287,7 +4287,7 @@ export const tools: McpTool[] = [ category: { type: 'string', description: 'Transaction category', enum: [...VALID_CATEGORIES] }, vat_treatment: { type: 'string', description: 'VAT treatment override. Defaults to standard_25 for business expenses. Set reverse_charge ONLY when the underlag confirms the seller did NOT charge VAT (omvänd skattskyldighet). An invoice with foreign VAT already debited is NOT reverse charge.', enum: [...VALID_VAT_TREATMENTS] }, vat_amount: { type: 'number', exclusiveMinimum: 0, description: 'The underlag\'s exact moms (> 0) when it differs from rate × belopp: e.g. dricks carries no VAT. Requires a rate-based vat_treatment. Swedish moms only: foreign VAT is never deductible. For a 0-moms document use vat_treatment="exempt".' }, - account_override: { type: 'string', pattern: '^\\d{4}$', description: 'Books the business side (debit when money goes out, credit when money comes in) on this kontoplan account instead of the category default: the ONLY way to reach company-custom accounts (e.g. VMB). Must exist and be active (gnubok_list_accounts; create via gnubok_create_account). VMB purchases/sales carry no deductible moms: use vat_treatment "exempt". Without an explicit vat_treatment (or vat_amount) the override books GROSS with no auto-VAT line: a moms leg is never guessed onto a custom account. Class-2 overrides outside 2610-2649 always drop auto-VAT. Not valid with category "private". State the actual affärshändelse in notes (BFL 5 kap).' }, + account_override: { type: 'string', pattern: '^\\d{4}$', description: 'Books the business side (debit when money goes out, credit when money comes in) on this kontoplan account instead of the category default: the ONLY way to reach company-custom accounts (e.g. VMB). category is still required: it decides direction and VAT; the override only replaces its default account. Must exist and be active (gnubok_list_accounts; create via gnubok_create_account). VMB purchases/sales carry no deductible moms: use vat_treatment "exempt". Without an explicit vat_treatment (or vat_amount) the override books GROSS with no auto-VAT line: a moms leg is never guessed onto a custom account. Class-2 overrides outside 2610-2649 always drop auto-VAT. Not valid with category "private". State the actual affärshändelse in notes (BFL 5 kap).' }, notes: { type: 'string', description: 'Audit-trail context appended to the verifikation description. For category=representation use this to record deltagare + syfte ("Anna Andersson (Acme AB), kundmöte om Y"). For project work, include the project ref. Keep under 200 chars; pure metadata, not a re-description of the transaction.' }, dimensions: { type: 'object', @@ -4322,10 +4322,22 @@ export const tools: McpTool[] = [ throw new Error('account_override must be exactly 4 digits, e.g. "4020".') } + // Presence guard (hosts don't always enforce inputSchema `required`). + // Without it a call carrying only account_override reached the enum + // check and surfaced as 'Invalid category "undefined"'. The enum check + // in categorizeTransactionCore still handles unknown category strings. + const category = typeof args.category === 'string' ? args.category.trim() : '' + if (!category) { + throw new Error( + 'category is required; account_override only overrides the category\'s default account. ' + + `Valid categories: ${VALID_CATEGORIES.join(', ')}`, + ) + } + // Compute the preview (accounts, amounts, VAT lines) const result = await categorizeTransactionCore( args.transaction_id as string, - args.category as TransactionCategory, + category as TransactionCategory, args.vat_treatment as VatTreatment | undefined, vatAmount, accountOverride, @@ -4408,7 +4420,7 @@ export const tools: McpTool[] = [ `Kategorisera: ${txDesc}`, { transaction_id: args.transaction_id, - category: args.category, + category, vat_treatment: args.vat_treatment || null, vat_amount: vatAmount ?? null, account_override: accountOverride ?? null, @@ -8902,6 +8914,16 @@ export const tools: McpTool[] = [ async execute(args, companyId, userId, supabase, actor) { const itemIds = args.item_ids as string[] if (!Array.isArray(itemIds) || itemIds.length === 0) throw new Error('item_ids is required (non-empty)') + // Presence + enum guard at the boundary (hosts don't always enforce + // inputSchema `required`/`enum`): without it a missing or unknown + // category was staged as-is and only rejected at approval time. + const category = typeof args.category === 'string' ? args.category.trim() : '' + if (!category) { + throw new Error(`category is required. Valid categories: ${VALID_CATEGORIES.join(', ')}`) + } + if (!VALID_CATEGORIES.includes(category as typeof VALID_CATEGORIES[number])) { + throw new Error(`Invalid category "${category}". Valid categories: ${VALID_CATEGORIES.join(', ')}`) + } const vatAmount = typeof args.vat_amount === 'number' && Number.isFinite(args.vat_amount) ? args.vat_amount : undefined @@ -8971,7 +8993,7 @@ export const tools: McpTool[] = [ // Stage only the bookable items: the executor re-checks each and // skips any that changed state between staging and approval. item_ids: bookable.map((it) => it.id as string), - category: args.category, + category, vat_treatment: args.vat_treatment ?? null, vat_amount: vatAmount ?? null, notes: notes ?? null, @@ -8988,7 +9010,7 @@ export const tools: McpTool[] = [ already_booked: alreadyBooked, not_found: notFound, total_sek: Math.round(totalSek * 100) / 100, - category: args.category, + category, vat_treatment: args.vat_treatment ?? null, ...(resolvedDimensions && Object.keys(resolvedDimensions).length > 0 ? { dimensions: resolvedDimensions }