diff --git a/extensions/general/mcp-server/server.ts b/extensions/general/mcp-server/server.ts index 7666f4ca..11a0bf91 100644 --- a/extensions/general/mcp-server/server.ts +++ b/extensions/general/mcp-server/server.ts @@ -2777,62 +2777,42 @@ export const tools: McpTool[] = [ const offset = Math.max(0, Number(args.offset) || 0) const since = typeof args.since === 'string' ? args.since : null const minAmount = typeof args.min_amount === 'number' && Number.isFinite(args.min_amount) - ? args.min_amount + ? Math.max(0, args.min_amount) : 0 - // PostgREST left-join filter: journal_entries left-joined to - // document_attachments and filtered to rows where the join produced - // no document. The filter syntax `document_attachments.id=is.null` - // applies the predicate post-join (Supabase: foreign-table is-null). - let query = supabase - .from('journal_entries') - .select( - 'id, voucher_series, voucher_number, entry_date, description, source_type, document_attachments!left(id), journal_entry_lines(debit_amount)', - { count: 'exact' }, - ) - .eq('company_id', companyId) - .eq('status', 'posted') - .is('document_attachments.id', null) - if (since) query = query.gte('entry_date', since) - - const { data, error, count } = await query - .order('entry_date', { ascending: false }) - .order('voucher_number', { ascending: false }) - .range(offset, offset + limit - 1) + // gross_amount is an aggregate over journal_entry_lines, which PostgREST + // cannot filter on — filtering it in memory after .range() made + // total_count ignore min_amount and consecutive pages overlap. The RPC + // filters, counts and paginates in SQL so the total respects the filter + // and next_offset advances by exactly the rows consumed. + const { data, error } = await supabase.rpc('verifikat_without_documents', { + p_company_id: companyId, + p_since: since, + p_min_amount: minAmount, + p_limit: limit, + p_offset: offset, + }) if (error) throw new Error(`Database error: ${error.message}`) - const rows = ((data ?? []) as Array<{ - id: string - voucher_series: string - voucher_number: number - entry_date: string - description: string - source_type: string - journal_entry_lines: { debit_amount: number | string }[] | null - }>).map((e) => { - const lines = e.journal_entry_lines ?? [] - const gross = lines.reduce((sum, l) => sum + (Number(l.debit_amount) || 0), 0) - return { - journal_entry_id: e.id, - voucher_series: e.voucher_series, - voucher_number: e.voucher_number, - entry_date: e.entry_date, - description: e.description, - source_type: e.source_type, - gross_amount: Math.round(gross * 100) / 100, - } - }) + const result = data as { + ok: boolean + code?: string + total_count?: number + verifikat?: unknown[] + } | null + if (!result?.ok) { + throw new Error(`verifikat_without_documents failed: ${result?.code ?? 'unknown error'}`) + } - const filtered = minAmount > 0 ? rows.filter((r) => r.gross_amount >= minAmount) : rows - - const total = count ?? 0 - const hasMore = total > offset + filtered.length + const rows = result.verifikat ?? [] + const total = result.total_count ?? 0 + const hasMore = offset + rows.length < total return { - verifikat: filtered, - count: filtered.length, + verifikat: rows, + count: rows.length, total_count: total, has_more: hasMore, - ...(hasMore ? { next_offset: offset + filtered.length } : {}), + ...(hasMore ? { next_offset: offset + rows.length } : {}), } }, }, diff --git a/supabase/migrations/20260703130000_verifikat_without_documents_rpc.sql b/supabase/migrations/20260703130000_verifikat_without_documents_rpc.sql new file mode 100644 index 00000000..7cb385b1 --- /dev/null +++ b/supabase/migrations/20260703130000_verifikat_without_documents_rpc.sql @@ -0,0 +1,116 @@ +-- RPC: verifikat_without_documents — SQL-side filtering + pagination for the +-- MCP tool gnubok_list_verifikat_without_documents. +-- +-- Bug fix (dev_docs/mcp_optimization_plan.md P0-2): the tool applied +-- min_amount IN MEMORY after the PostgREST .range() page, because +-- gross_amount is an aggregate (sum of debit lines) PostgREST cannot filter +-- on. Consequences: total_count ignored the filter, and next_offset advanced +-- by the filtered row count while the DB page consumed `limit` rows — so the +-- next page overlapped the previous page's tail. Agents paging a backlog got +-- duplicates and could not prove full coverage. +-- +-- This function computes gross_amount, applies since/min_amount, counts and +-- paginates all in SQL, returning a filter-respecting total alongside the +-- page. Ordering carries an id tiebreak so pagination is total and stable. +-- +-- Tenant guard (mirrors 20260615120000): anon/authenticated callers must be +-- members of p_company_id; service_role bypasses (MCP tools already scope by +-- company_id). +-- +-- pg-test: tests/pg/verifikat-without-documents-rpc.pg.test.ts (pagination +-- invariants: disjoint pages, complete union, filter-respecting total, since +-- filter, tenant guard). + +CREATE OR REPLACE FUNCTION public.verifikat_without_documents( + p_company_id uuid, + p_since date DEFAULT NULL, + p_min_amount numeric DEFAULT 0, + p_limit integer DEFAULT 20, + p_offset integer DEFAULT 0 +) +RETURNS jsonb +LANGUAGE plpgsql +STABLE +SECURITY DEFINER +SET search_path TO 'public' +AS $$ +DECLARE + v_jwt_role text := coalesce(nullif(current_setting('request.jwt.claims', true), '')::jsonb ->> 'role', ''); + v_limit integer := least(greatest(coalesce(p_limit, 20), 1), 100); + v_offset integer := greatest(coalesce(p_offset, 0), 0); + v_min numeric := greatest(coalesce(p_min_amount, 0), 0); + v_result jsonb; +BEGIN + IF v_jwt_role IN ('anon', 'authenticated') THEN + -- NOT IN (subquery) evaluates to UNKNOWN when either side yields NULL, + -- which would silently skip this deny branch — use NOT EXISTS and reject + -- a NULL company id outright. + IF p_company_id IS NULL OR NOT EXISTS ( + SELECT 1 FROM public.user_company_ids() AS c(id) WHERE c.id = p_company_id + ) THEN + RETURN jsonb_build_object('ok', false, 'code', 'VERIFIKAT_WITHOUT_DOCUMENTS_FORBIDDEN'); + END IF; + END IF; + + WITH candidates AS ( + SELECT + je.id, + je.voucher_series, + je.voucher_number, + je.entry_date, + je.description, + je.source_type, + round(coalesce(sum(l.debit_amount), 0), 2) AS gross_amount + FROM journal_entries je + LEFT JOIN journal_entry_lines l ON l.journal_entry_id = je.id + WHERE je.company_id = p_company_id + AND je.status = 'posted' + AND NOT EXISTS ( + SELECT 1 FROM document_attachments d WHERE d.journal_entry_id = je.id + ) + AND (p_since IS NULL OR je.entry_date >= p_since) + GROUP BY je.id + HAVING round(coalesce(sum(l.debit_amount), 0), 2) >= v_min + ), + total AS ( + SELECT count(*) AS n FROM candidates + ), + page AS ( + SELECT * FROM candidates + ORDER BY entry_date DESC, voucher_number DESC, id DESC + LIMIT v_limit OFFSET v_offset + ) + SELECT jsonb_build_object( + 'ok', true, + 'total_count', (SELECT n FROM total), + 'verifikat', coalesce( + (SELECT jsonb_agg( + jsonb_build_object( + 'journal_entry_id', p.id, + 'voucher_series', p.voucher_series, + 'voucher_number', p.voucher_number, + 'entry_date', p.entry_date, + 'description', p.description, + 'source_type', p.source_type, + 'gross_amount', p.gross_amount + ) + ORDER BY p.entry_date DESC, p.voucher_number DESC, p.id DESC + ) FROM page p), + '[]'::jsonb + ) + ) + INTO v_result; + + RETURN v_result; +END; +$$; + +REVOKE ALL ON FUNCTION public.verifikat_without_documents(uuid, date, numeric, integer, integer) FROM PUBLIC, anon; +GRANT EXECUTE ON FUNCTION public.verifikat_without_documents(uuid, date, numeric, integer, integer) TO authenticated, service_role; + +-- Anti-join + link lookups both hit document_attachments by journal_entry_id; +-- the FK carries no index by default. +CREATE INDEX IF NOT EXISTS idx_document_attachments_journal_entry_id + ON public.document_attachments (journal_entry_id); + +NOTIFY pgrst, 'reload schema'; diff --git a/tests/pg/verifikat-without-documents-rpc.pg.test.ts b/tests/pg/verifikat-without-documents-rpc.pg.test.ts new file mode 100644 index 00000000..bf29444b --- /dev/null +++ b/tests/pg/verifikat-without-documents-rpc.pg.test.ts @@ -0,0 +1,212 @@ +import { randomUUID } from 'crypto' +import { beforeAll, describe, expect, it } from 'vitest' +import { getPool, withUserContext } from './setup' +import { + seedCompany, + insertAuthUser, + insertDraftJournalEntry, + insertBalancedLines, +} from './fixtures' + +/** + * Invariants for the verifikat_without_documents RPC (mcp_optimization_plan + * P0-2). The predecessor applied min_amount in memory after the DB page: + * total_count ignored the filter and consecutive pages overlapped. These + * tests pin the SQL-side behavior: filter-respecting total, disjoint and + * complete pages, since filter, doc/draft exclusion, tenant guard. + */ + +type RpcRow = { + journal_entry_id: string + voucher_number: number + gross_amount: number +} +type RpcResult = { + ok: boolean + code?: string + total_count?: number + verifikat?: RpcRow[] +} + +async function callRpc(params: { + companyId: string + since?: string | null + minAmount?: number + limit?: number + offset?: number +}): Promise { + const { rows } = await getPool().query<{ result: RpcResult }>( + `SELECT public.verifikat_without_documents($1, $2, $3, $4, $5) AS result`, + [ + params.companyId, + params.since ?? null, + params.minAmount ?? 0, + params.limit ?? 20, + params.offset ?? 0, + ], + ) + return rows[0].result +} + +async function attachDocument(params: { + userId: string + companyId: string + journalEntryId: string +}): Promise { + await getPool().query( + `INSERT INTO public.document_attachments + (id, user_id, company_id, journal_entry_id, file_name, mime_type, + file_size_bytes, storage_path, sha256_hash, upload_source) + VALUES ($1, $2, $3, $4, 'underlag.pdf', 'application/pdf', 1024, $5, $6, 'file_upload')`, + [ + randomUUID(), + params.userId, + params.companyId, + params.journalEntryId, + `documents/${params.companyId}/underlag.pdf`, + randomUUID().replace(/-/g, '').padEnd(64, '0'), + ], + ) +} + +describe('verifikat_without_documents RPC', () => { + let userId: string + let companyId: string + let fiscalPeriodId: string + // voucher_number → { id, amount }; amounts chosen so min_amount splits the set + const seeded = new Map() + + beforeAll(async () => { + const s = await seedCompany() + userId = s.userId + companyId = s.companyId + fiscalPeriodId = s.fiscalPeriodId + + // 6 posted entries without documents: amounts 100..600, dates ascending + for (let i = 1; i <= 6; i++) { + const id = await insertDraftJournalEntry({ + userId, + companyId, + fiscalPeriodId, + status: 'posted', + voucherNumber: i, + entryDate: `2026-06-0${i}`, + description: `no-doc ${i}`, + }) + await insertBalancedLines(id, i * 100) + seeded.set(i, { id, amount: i * 100 }) + } + + // Posted entry WITH a document — must never appear + const withDoc = await insertDraftJournalEntry({ + userId, + companyId, + fiscalPeriodId, + status: 'posted', + voucherNumber: 7, + entryDate: '2026-06-07', + description: 'has doc', + }) + await insertBalancedLines(withDoc, 700) + await attachDocument({ userId, companyId, journalEntryId: withDoc }) + + // Draft entry — must never appear + const draft = await insertDraftJournalEntry({ + userId, + companyId, + fiscalPeriodId, + status: 'draft', + voucherNumber: 8, + entryDate: '2026-06-08', + description: 'draft', + }) + await insertBalancedLines(draft, 800) + }) + + it('lists posted no-doc entries only, newest first, with gross amounts', async () => { + const res = await callRpc({ companyId }) + expect(res.ok).toBe(true) + expect(res.total_count).toBe(6) + const rows = res.verifikat ?? [] + expect(rows.map((r) => r.voucher_number)).toEqual([6, 5, 4, 3, 2, 1]) + expect(rows[0].gross_amount).toBe(600) + }) + + it('total_count respects min_amount', async () => { + const res = await callRpc({ companyId, minAmount: 350 }) + expect(res.ok).toBe(true) + // amounts 400, 500, 600 pass the filter + expect(res.total_count).toBe(3) + expect((res.verifikat ?? []).map((r) => r.voucher_number)).toEqual([6, 5, 4]) + }) + + it('paginates a filtered set with disjoint, complete pages', async () => { + const collected: string[] = [] + let offset = 0 + for (;;) { + const res = await callRpc({ companyId, minAmount: 250, limit: 2, offset }) + expect(res.ok).toBe(true) + expect(res.total_count).toBe(4) // amounts 300..600 + const rows = res.verifikat ?? [] + if (rows.length === 0) break + collected.push(...rows.map((r) => r.journal_entry_id)) + offset += rows.length + if (offset >= (res.total_count ?? 0)) break + } + // No duplicates (the old in-memory filter overlapped pages) … + expect(new Set(collected).size).toBe(collected.length) + // … and no gaps: exactly the 4 entries ≥ 250. + const expected = [3, 4, 5, 6].map((n) => seeded.get(n)!.id).sort() + expect([...collected].sort()).toEqual(expected) + }) + + it('respects the since filter', async () => { + const res = await callRpc({ companyId, since: '2026-06-04' }) + expect(res.ok).toBe(true) + expect(res.total_count).toBe(3) + expect((res.verifikat ?? []).map((r) => r.voucher_number)).toEqual([6, 5, 4]) + }) + + it('offset past the end returns an empty page with a truthful total', async () => { + const res = await callRpc({ companyId, minAmount: 250, limit: 2, offset: 10 }) + expect(res.ok).toBe(true) + expect(res.total_count).toBe(4) + expect(res.verifikat).toEqual([]) + }) + + it('blocks an authenticated caller from another tenant', async () => { + const strangerId = await insertAuthUser() + const res = await withUserContext(strangerId, async (client) => { + const { rows } = await client.query<{ result: RpcResult }>( + `SELECT public.verifikat_without_documents($1, NULL, 0, 20, 0) AS result`, + [companyId], + ) + return rows[0].result + }) + expect(res.ok).toBe(false) + expect(res.code).toBe('VERIFIKAT_WITHOUT_DOCUMENTS_FORBIDDEN') + }) + + it('rejects a NULL company id for authenticated callers (NOT IN → UNKNOWN bypass)', async () => { + const res = await withUserContext(userId, async (client) => { + const { rows } = await client.query<{ result: RpcResult }>( + `SELECT public.verifikat_without_documents(NULL, NULL, 0, 20, 0) AS result`, + ) + return rows[0].result + }) + expect(res.ok).toBe(false) + expect(res.code).toBe('VERIFIKAT_WITHOUT_DOCUMENTS_FORBIDDEN') + }) + + it('allows an authenticated member of the company', async () => { + const res = await withUserContext(userId, async (client) => { + const { rows } = await client.query<{ result: RpcResult }>( + `SELECT public.verifikat_without_documents($1, NULL, 0, 20, 0) AS result`, + [companyId], + ) + return rows[0].result + }) + expect(res.ok).toBe(true) + expect(res.total_count).toBe(6) + }) +})