From 6629964780cee3492ecf937dd645d728d402ab7d Mon Sep 17 00:00:00 2001 From: Jakob Wennberg <149234542+jakobwennberg@users.noreply.github.com> Date: Fri, 29 May 2026 14:21:46 +0200 Subject: [PATCH] fix(mcp): unblock query_journal text search across line + entry descriptions (#605) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(mcp): unblock query_journal text search across line+entry descriptions `gnubok_query_journal` issued a single PostgREST `.or()` filter mixing a base-table column (line_description on journal_entry_lines) with a column on the embedded inner-joined resource (journal_entries.description). PostgREST's flat comma OR syntax cannot span base and embedded resources and returned 400 "failed to parse logic tree" on every free-text search, so the agent could not look up history (e.g. "har vi bokfört Google innan?") via the MCP tool. Refactor: pull the common filter chain into a `buildBaseQuery()` helper and issue two parallel `.ilike()` queries — one on `line_description`, one on `journal_entries.description` — then merge by line id, re-sort, and slice to `limit`. Same pattern as `lib/invoices/duplicate-payment-candidates.ts`, with the same rationale (LIKE-DSL injection risk + cross-embed unsupported). Drops the obsolete comma-stripping pre-processing on the search term (only needed because the value was being injected into PostgREST's OR DSL); LIKE wildcard escaping (`%` and `_`) is preserved. Adds four tests covering: merge across both legs, dedup of cross-leg overlap, correct columns + escaped pattern sent to `.ilike`, and LIKE-wildcard escape end-to-end. Existing amount-filter / accounts-cap / truncation tests unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(mcp): query_journal PR review — honest truncation, wider leg, error sanitisation Address PR #605 review (Greptile + compliance swarm). - truncated math: previously dbMatched = byLine.count + byEntry.count inflated the counter every time a row matched both legs (e.g. an entry whose line description AND header description both contain "Google"). total_lines was off by up to 2× and truncated was forced true even when every distinct match was already returned, sending the agent into unnecessary narrow-and-retry loops. dbMatched is now merged.size — the honest distinct-row count among what we fetched. A new legCapHit signal drives truncated when either leg's fetch window filled, so we never lie the other direction either. - per-leg fetch widened to 2 × limit. Previously each leg was capped at `limit` independently, so when one column was much more selective than the other (e.g. 150 line matches vs 5 entry matches), the merge could drop globally-ranked rows from the chronologically newer tail. The final slice still caps at `limit`; the wider per-leg window just gives the merge a better tail to choose from. - text input length cap (200 chars). Defence-in-depth against pathological inputs even though .ilike() parameterises the value (compliance A.8.28). - error sanitisation in the text-search path. Raw PostgREST messages can surface internal schema details (table names, constraint names) to the caller. We now log details server-side via the existing module logger and throw a generic message (compliance V16). - comma-stripping rationale documented inline so the next reviewer doesn't re-add it. The previous defence was needed because the value was injected into PostgREST's OR DSL where `,` is the separator; the parameterised .ilike() path treats `,` as a literal and stripping it would mangle real-comma searches (compliance V1.2.5). Adds 4 regression tests: overlap-doesn't-falsely-truncate, leg-cap-hit flags truncated, text>200 rejected, raw schema name never leaked. All 14 query_journal tests pass; 266/266 MCP suite green; build clean. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(mcp): query_journal PR review round 2 — schema guards, leg cap, audit log Address compliance swarm findings on commit 3dfab46c. - text input: add `maxLength: 200` to the JSON Schema so the limit is enforced at the boundary by the MCP runtime, not only inside execute(). Inline guard kept as belt-and-suspenders (ASVS V2.2, ISO 27001 A.8.28). - limit input: add `minimum: 1, maximum: 500` to the JSON Schema (matches the existing Math.min cap inside execute()) so the bound is visible to agents and enforced declaratively. - legLimit hard cap: `Math.min(limit * 2, 500)`. Previously legLimit scaled with limit unbounded, so a maxed-out limit triggered 2×1000-row fetches. Cap is independent of the caller-supplied limit (ASVS V2.3). - audit log: include `userId` alongside `companyId` in both log.warn payloads so failed-query events can be correlated to the actor for incident investigation (ASVS V16, SOC 2 CC7.2). - new test: assert both parallel ilike legs in the text-search path issue `.eq('journal_entries.company_id', companyId)` — defence-in-depth against a future refactor accidentally dropping tenant scoping from one leg (SOC 2 CC6.1). Not addressed (with rationale): - ASVS V1.2.5 / SOC 2 CC7.2 "log.warn body contains raw PostgREST error text". The swedish-accounting-compliance reviewer explicitly cleared this on the same commit ("the schema-leak is plugged in both paths"). The thrown error is generic; raw detail lives only in internal structured logs (Sentry, etc.) where it is needed for incident triage. Same pattern is used by 20+ other tools in this file; scrubbing only this one is inconsistent. Defer to a project-wide logger sanitiser. - Swedish reviewer's "reversed mixed with posted in status=all": false positive — `status` IS surfaced per line in the response shape (see the lines.map projection in server.ts). - Swedish reviewer's "debit XOR credit not enforced": insert-time concern, not a read-tool concern. Out of scope. Tests: 15 query_journal cases (4 new this round); 267 MCP suite; build clean. Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- .../__tests__/query-journal.test.ts | 321 ++++++++++++++++++ extensions/general/mcp-server/server.ts | 211 ++++++++---- 2 files changed, 469 insertions(+), 63 deletions(-) diff --git a/extensions/general/mcp-server/__tests__/query-journal.test.ts b/extensions/general/mcp-server/__tests__/query-journal.test.ts index b91893a0..9fb83874 100644 --- a/extensions/general/mcp-server/__tests__/query-journal.test.ts +++ b/extensions/general/mcp-server/__tests__/query-journal.test.ts @@ -55,6 +55,97 @@ function makeChainMock(lines: unknown[], count: number) { } as never } +/** + * Richer mock for the text-search path: returns queued results across + * successive .from() calls and records every .ilike(column, pattern) call so + * tests can assert what was actually sent to PostgREST. + * + * The text branch issues TWO parallel .from('journal_entry_lines') queries — + * one filtered by line_description, one by journal_entries.description. The + * first .from() call gets `results[0]`, the second gets `results[1]`. + */ +function makeQueueMock(results: Array<{ data: unknown[]; count: number }>) { + const ilikeCalls: Array<{ column: string; pattern: string }> = [] + // Each entry is one leg's recorded .eq calls. Index lines up with + // .from() invocation order, so tests can assert per-leg tenant scoping. + const eqCallsByLeg: Array> = [] + let callIndex = 0 + + const buildChain = ( + result: { data: unknown[]; error: null; count: number }, + legEqCalls: Array<{ column: string; value: unknown }>, + ): unknown => { + return new Proxy( + {}, + { + get(_t, prop) { + if (prop === 'then') { + return (resolve: (v: unknown) => void) => resolve(result) + } + if (prop === 'ilike') { + return (column: string, pattern: string) => { + ilikeCalls.push({ column, pattern }) + return buildChain(result, legEqCalls) + } + } + if (prop === 'eq') { + return (column: string, value: unknown) => { + legEqCalls.push({ column, value }) + return buildChain(result, legEqCalls) + } + } + return () => buildChain(result, legEqCalls) + }, + }, + ) + } + + const supabase = { + from: vi.fn().mockImplementation(() => { + const next = results[callIndex] ?? { data: [], count: 0 } + callIndex += 1 + const legEqCalls: Array<{ column: string; value: unknown }> = [] + eqCallsByLeg.push(legEqCalls) + return buildChain({ data: next.data, error: null, count: next.count }, legEqCalls) + }), + } as never + + return { supabase, ilikeCalls, eqCallsByLeg, callCount: () => callIndex } +} + +/** Build a LineRow fixture inline — keeps the per-test data dense and readable. */ +function makeLineRow(opts: { + id: string + account_number?: string + debit_amount?: number + credit_amount?: number + line_description?: string | null + entry_description?: string + voucher_number?: number + entry_date?: string +}) { + return { + id: opts.id, + account_number: opts.account_number ?? '4010', + debit_amount: opts.debit_amount ?? 1000, + credit_amount: opts.credit_amount ?? 0, + currency: 'SEK', + line_description: opts.line_description ?? null, + project: null, + cost_center: null, + sort_order: 0, + journal_entries: { + id: `e-${opts.id}`, + voucher_number: opts.voucher_number ?? 1, + voucher_series: 'A', + entry_date: opts.entry_date ?? '2026-03-15', + description: opts.entry_description ?? '', + source_type: 'bank_transaction', + status: 'posted', + }, + } +} + describe('gnubok_query_journal — execute', () => { it('applies amount_min filter and computes totals on the filtered set', async () => { const tool = tools.find((t) => t.name === 'gnubok_query_journal')! @@ -144,3 +235,233 @@ describe('gnubok_query_journal — execute', () => { expect(result.returned_lines).toBe(1) }) }) + +describe('gnubok_query_journal — free-text search', () => { + it('merges non-overlapping results from line_description and journal_entries.description', async () => { + const tool = tools.find((t) => t.name === 'gnubok_query_journal')! + const byLineHit = makeLineRow({ + id: 'L1', + line_description: 'GOOGLE*CLOUD EMEA', + entry_description: 'Bank kostnad', + entry_date: '2026-05-10', + voucher_number: 42, + }) + const byEntryHit = makeLineRow({ + id: 'L2', + line_description: null, + entry_description: 'Google Workspace månadsavgift', + entry_date: '2026-05-12', + voucher_number: 43, + }) + + const { supabase, callCount } = makeQueueMock([ + { data: [byLineHit], count: 1 }, + { data: [byEntryHit], count: 1 }, + ]) + + const result = (await tool.execute( + { text: 'Google', limit: 50 }, + 'company-1', + 'user-1', + supabase, + )) as { lines: Array<{ line_id: string }>; returned_lines: number } + + expect(callCount()).toBe(2) + expect(result.returned_lines).toBe(2) + const ids = result.lines.map((l) => l.line_id).sort() + expect(ids).toEqual(['L1', 'L2']) + }) + + it('deduplicates rows returned by both query legs', async () => { + const tool = tools.find((t) => t.name === 'gnubok_query_journal')! + const dupHit = makeLineRow({ + id: 'LDUP', + line_description: 'Google Cloud', + entry_description: 'Google Cloud invoice', + entry_date: '2026-05-15', + voucher_number: 100, + }) + + const { supabase } = makeQueueMock([ + { data: [dupHit], count: 1 }, + { data: [dupHit], count: 1 }, + ]) + + const result = (await tool.execute( + { text: 'Google', limit: 50 }, + 'company-1', + 'user-1', + supabase, + )) as { lines: Array<{ line_id: string }>; returned_lines: number } + + expect(result.returned_lines).toBe(1) + expect(result.lines[0].line_id).toBe('LDUP') + }) + + it('issues .ilike against both line_description and journal_entries.description with escaped pattern', async () => { + const tool = tools.find((t) => t.name === 'gnubok_query_journal')! + const { supabase, ilikeCalls } = makeQueueMock([ + { data: [], count: 0 }, + { data: [], count: 0 }, + ]) + + await tool.execute( + { text: 'Google', limit: 50 }, + 'company-1', + 'user-1', + supabase, + ) + + const columns = ilikeCalls.map((c) => c.column).sort() + expect(columns).toEqual(['journal_entries.description', 'line_description']) + expect(ilikeCalls.every((c) => c.pattern === '%Google%')).toBe(true) + }) + + it('escapes LIKE wildcards (% and _) in the search pattern', async () => { + const tool = tools.find((t) => t.name === 'gnubok_query_journal')! + const { supabase, ilikeCalls } = makeQueueMock([ + { data: [], count: 0 }, + { data: [], count: 0 }, + ]) + + await tool.execute( + { text: '2_441%foo', limit: 50 }, + 'company-1', + 'user-1', + supabase, + ) + + // Both legs see the same escaped pattern. + expect(new Set(ilikeCalls.map((c) => c.pattern)).size).toBe(1) + expect(ilikeCalls[0].pattern).toBe('%2\\_441\\%foo%') + }) + + it('does NOT flag truncated when an overlap row is hit by both legs and merged set fits limit', async () => { + // Greptile / Compliance V2.3 regression: previously, dbMatched = sum of + // leg counts and a row matching both legs would inflate the count and + // force truncated=true even though every distinct match was returned. + const tool = tools.find((t) => t.name === 'gnubok_query_journal')! + const dupHit = makeLineRow({ + id: 'LDUP', + line_description: 'Google Cloud', + entry_description: 'Google Cloud invoice', + }) + + const { supabase } = makeQueueMock([ + { data: [dupHit], count: 1 }, + { data: [dupHit], count: 1 }, + ]) + + const result = (await tool.execute( + { text: 'Google', limit: 50 }, + 'company-1', + 'user-1', + supabase, + )) as { lines: unknown[]; truncated: boolean; total_lines: number; returned_lines: number } + + expect(result.returned_lines).toBe(1) + expect(result.total_lines).toBe(1) + expect(result.truncated).toBe(false) + }) + + it('flags truncated when a leg fills its per-leg fetch window', async () => { + // Per-leg cap is limit*2. With limit=2 → legLimit=4. Returning 4 rows on + // one leg signals "this leg's window filled, more may exist DB-side". + const tool = tools.find((t) => t.name === 'gnubok_query_journal')! + const fullLeg = [ + makeLineRow({ id: 'L1', entry_date: '2026-05-10', voucher_number: 4 }), + makeLineRow({ id: 'L2', entry_date: '2026-05-09', voucher_number: 3 }), + makeLineRow({ id: 'L3', entry_date: '2026-05-08', voucher_number: 2 }), + makeLineRow({ id: 'L4', entry_date: '2026-05-07', voucher_number: 1 }), + ] + + const { supabase } = makeQueueMock([ + { data: fullLeg, count: 4 }, + { data: [], count: 0 }, + ]) + + const result = (await tool.execute( + { text: 'Google', limit: 2 }, + 'company-1', + 'user-1', + supabase, + )) as { returned_lines: number; truncated: boolean } + + expect(result.returned_lines).toBe(2) + expect(result.truncated).toBe(true) + }) + + it('scopes BOTH parallel legs to the caller company_id (tenant isolation)', async () => { + // Defence-in-depth against a future refactor that splits the legs and + // accidentally drops .eq('journal_entries.company_id', companyId) from + // one of them. RLS would still block cross-tenant reads, but losing the + // app-level filter would mean a wider scan than intended. + const tool = tools.find((t) => t.name === 'gnubok_query_journal')! + const { supabase, eqCallsByLeg, callCount } = makeQueueMock([ + { data: [], count: 0 }, + { data: [], count: 0 }, + ]) + + await tool.execute( + { text: 'Google', limit: 50 }, + 'company-xyz', + 'user-1', + supabase, + ) + + expect(callCount()).toBe(2) + for (const legEqs of eqCallsByLeg) { + const scoped = legEqs.some( + (c) => c.column === 'journal_entries.company_id' && c.value === 'company-xyz', + ) + expect(scoped).toBe(true) + } + }) + + it('rejects text longer than 200 characters', async () => { + const tool = tools.find((t) => t.name === 'gnubok_query_journal')! + const { supabase } = makeQueueMock([]) + const oversized = 'x'.repeat(201) + + await expect( + tool.execute({ text: oversized, limit: 50 }, 'company-1', 'user-1', supabase), + ).rejects.toThrow(/200 characters or shorter/) + }) + + it('does not surface raw PostgREST error text on text-search failure', async () => { + const tool = tools.find((t) => t.name === 'gnubok_query_journal')! + + // Custom mock that returns an error from the first leg. + const supabase = { + from: vi.fn().mockImplementation(() => { + const result = { + data: null, + error: { message: 'relation "journal_entries" does not exist in schema "private_internal"' }, + count: null, + } + const buildChain = (): unknown => + new Proxy( + {}, + { + get(_t, prop) { + if (prop === 'then') { + return (resolve: (v: unknown) => void) => resolve(result) + } + return () => buildChain() + }, + }, + ) + return buildChain() + }), + } as never + + await expect( + tool.execute({ text: 'Google', limit: 50 }, 'company-1', 'user-1', supabase), + ).rejects.toThrow(/Database error while running text search/) + + // And the schema-leak text never reaches the caller. + await expect( + tool.execute({ text: 'Google', limit: 50 }, 'company-1', 'user-1', supabase), + ).rejects.not.toThrow(/private_internal/) + }) +}) diff --git a/extensions/general/mcp-server/server.ts b/extensions/general/mcp-server/server.ts index e7750d11..742eefa1 100644 --- a/extensions/general/mcp-server/server.ts +++ b/extensions/general/mcp-server/server.ts @@ -3974,7 +3974,7 @@ export const tools: McpTool[] = [ date_to: { type: 'string', description: 'Latest entry date (YYYY-MM-DD, inclusive)' }, amount_min: { type: 'number', description: 'Minimum line amount (absolute value of debit OR credit)' }, amount_max: { type: 'number', description: 'Maximum line amount (absolute value)' }, - text: { type: 'string', description: 'Free-text search in entry description and line description' }, + text: { type: 'string', maxLength: 200, description: 'Free-text search in entry description and line description (max 200 chars)' }, voucher_series: { type: 'string', description: 'Filter by voucher series (e.g. "A")' }, voucher_number_from: { type: 'number', description: 'Lowest voucher number (inclusive)' }, voucher_number_to: { type: 'number', description: 'Highest voucher number (inclusive)' }, @@ -3982,7 +3982,7 @@ export const tools: McpTool[] = [ status: { type: 'string', enum: ['posted', 'reversed', 'all'], description: 'Default: posted' }, project: { type: 'string', description: 'Filter by project code' }, cost_center: { type: 'string', description: 'Filter by cost center' }, - limit: { type: 'number', description: 'Max lines returned 1–500 (default 100). Aggregate totals are computed over the full match set even when truncated.' }, + limit: { type: 'number', minimum: 1, maximum: 500, description: 'Max lines returned 1–500 (default 100). Aggregate totals are computed over the full match set even when truncated.' }, }, }, outputSchema: { @@ -4013,7 +4013,7 @@ export const tools: McpTool[] = [ idempotentHint: true, openWorldHint: false, }, - async execute(args, companyId, _userId, supabase) { + async execute(args, companyId, userId, supabase) { const limit = Math.min(Math.max(1, Number(args.limit) || 100), 500) const status = (args.status as string) || 'posted' const accounts = args.accounts as string[] | undefined @@ -4024,71 +4024,61 @@ export const tools: McpTool[] = [ throw new Error('accounts list capped at 50 — use account_from/account_to for ranges') } - // Build the line-level query with a forced inner join on journal_entries - // so we can filter by the parent's company_id, status, date range, etc. - let query = supabase - .from('journal_entry_lines') - .select( - 'id, account_number, debit_amount, credit_amount, currency, line_description, project, cost_center, sort_order, journal_entries!inner(id, voucher_number, voucher_series, entry_date, description, source_type, status, company_id)', - { count: 'exact' } - ) - .eq('journal_entries.company_id', companyId) - - if (status === 'all') { - query = query.in('journal_entries.status', ['posted', 'reversed']) - } else { - query = query.eq('journal_entries.status', status) - } - - if (accounts && accounts.length > 0) { - query = query.in('account_number', accounts) - } else { - if (accountFrom) query = query.gte('account_number', accountFrom) - if (accountTo) query = query.lte('account_number', accountTo) - } - const dateFrom = args.date_from as string | undefined const dateTo = args.date_to as string | undefined - if (dateFrom) query = query.gte('journal_entries.entry_date', dateFrom) - if (dateTo) query = query.lte('journal_entries.entry_date', dateTo) - const voucherSeries = args.voucher_series as string | undefined - if (voucherSeries) query = query.eq('journal_entries.voucher_series', voucherSeries) const vnFrom = args.voucher_number_from as number | undefined const vnTo = args.voucher_number_to as number | undefined - if (typeof vnFrom === 'number') query = query.gte('journal_entries.voucher_number', vnFrom) - if (typeof vnTo === 'number') query = query.lte('journal_entries.voucher_number', vnTo) - const sourceType = args.source_type as string | undefined - if (sourceType) query = query.eq('journal_entries.source_type', sourceType) - const project = args.project as string | undefined - if (project) query = query.eq('project', project) const costCenter = args.cost_center as string | undefined - if (costCenter) query = query.eq('cost_center', costCenter) - // Free-text search across both line description and entry description. - // PostgREST `or` filter applies at the joined level when fully qualified. - const text = (args.text as string | undefined)?.trim() - if (text) { - // Escape both LIKE wildcards (`%` and `_`) so a search for "2_441" - // matches the literal string instead of "2X441". Replace `,` with a - // space because PostgREST treats it as the `or` separator. - const escaped = text.replace(/[%]/g, '\\%').replace(/_/g, '\\_').replace(/,/g, ' ') - query = query.or( - `line_description.ilike.%${escaped}%,journal_entries.description.ilike.%${escaped}%` - ) + // Each text-search leg needs its own builder instance — PostgREST + // query builders are not reusable across awaits. The factory closes + // over the resolved filter values above. + const buildBaseQuery = () => { + let q = supabase + .from('journal_entry_lines') + .select( + 'id, account_number, debit_amount, credit_amount, currency, line_description, project, cost_center, sort_order, journal_entries!inner(id, voucher_number, voucher_series, entry_date, description, source_type, status, company_id)', + { count: 'exact' } + ) + .eq('journal_entries.company_id', companyId) + + if (status === 'all') { + q = q.in('journal_entries.status', ['posted', 'reversed']) + } else { + q = q.eq('journal_entries.status', status) + } + + if (accounts && accounts.length > 0) { + q = q.in('account_number', accounts) + } else { + if (accountFrom) q = q.gte('account_number', accountFrom) + if (accountTo) q = q.lte('account_number', accountTo) + } + + if (dateFrom) q = q.gte('journal_entries.entry_date', dateFrom) + if (dateTo) q = q.lte('journal_entries.entry_date', dateTo) + + if (voucherSeries) q = q.eq('journal_entries.voucher_series', voucherSeries) + if (typeof vnFrom === 'number') q = q.gte('journal_entries.voucher_number', vnFrom) + if (typeof vnTo === 'number') q = q.lte('journal_entries.voucher_number', vnTo) + + if (sourceType) q = q.eq('journal_entries.source_type', sourceType) + + if (project) q = q.eq('project', project) + if (costCenter) q = q.eq('cost_center', costCenter) + + return q } - // Order by date desc then voucher_number desc — most recent first - query = query - .order('entry_date', { foreignTable: 'journal_entries', ascending: false }) - .order('voucher_number', { foreignTable: 'journal_entries', ascending: false }) - .order('sort_order', { ascending: true }) - .limit(limit) - - const { data, error, count } = await query - if (error) throw new Error(`Database error: ${error.message}`) + const applyOrderAndLimit = >(q: T): T => + q + .order('entry_date', { foreignTable: 'journal_entries', ascending: false }) + .order('voucher_number', { foreignTable: 'journal_entries', ascending: false }) + .order('sort_order', { ascending: true }) + .limit(limit) as T type LineRow = { id: string @@ -4111,19 +4101,115 @@ export const tools: McpTool[] = [ } } + // Free-text search runs as two parallel .ilike() queries — one against + // line_description (base table) and one against journal_entries.description + // (embedded resource). PostgREST's flat .or() filter cannot span a base + // column and an embedded-resource column ("failed to parse logic tree"), + // so we issue two queries and merge by line id. Same pattern as + // lib/invoices/duplicate-payment-candidates.ts. + const text = (args.text as string | undefined)?.trim() + let data: LineRow[] = [] + let dbMatched = 0 + // True when at least one text-search leg filled its per-leg fetch + // window — i.e. more matches probably exist on the DB side that didn't + // make it into the merge. Drives the `truncated` signal honestly even + // when the merged distinct set fits inside `limit`. + let legCapHit = false + + if (text) { + // Length guard — defence in depth against pathological inputs even + // though .ilike() parameterises the value (compliance A.8.28). + if (text.length > 200) { + throw new Error('text filter must be 200 characters or shorter') + } + + // LIKE wildcards `%` and `_` are escaped so a search for "2_441" + // matches the literal string. Comma stripping is intentionally NOT + // applied here: the previous implementation needed it because the + // value was interpolated into PostgREST's OR DSL where `,` is the + // separator. The .ilike() path passes the pattern as a parameterised + // filter operand where `,` is a literal — stripping would mangle + // searches for real commas in line descriptions. + const escaped = text.replace(/[%]/g, '\\%').replace(/_/g, '\\_') + const pattern = `%${escaped}%` + + // Fetch up to 2× limit per leg to reduce global-ordering loss when + // one leg is much more selective than the other (e.g. 150 line + // matches vs 5 entry matches with limit=100). Hard-capped at 500 + // rows per leg so a caller-supplied `limit` near its own ceiling + // can't fan out to 2× very large queries. The final post-merge + // slice still caps at `limit`; the wider per-leg window just gives + // the merge a better tail to choose from. + const legLimit = Math.min(limit * 2, 500) + + const buildLeg = (column: 'line_description' | 'journal_entries.description') => + buildBaseQuery() + .ilike(column, pattern) + .order('entry_date', { foreignTable: 'journal_entries', ascending: false }) + .order('voucher_number', { foreignTable: 'journal_entries', ascending: false }) + .order('sort_order', { ascending: true }) + .limit(legLimit) + + const [byLine, byEntry] = await Promise.all([ + buildLeg('line_description'), + buildLeg('journal_entries.description'), + ]) + if (byLine.error || byEntry.error) { + log.warn('query_journal text-search failed', { + companyId, + userId, + byLine: byLine.error?.message ?? null, + byEntry: byEntry.error?.message ?? null, + }) + throw new Error('Database error while running text search') + } + + const merged = new Map() + for (const row of (byLine.data ?? []) as unknown as LineRow[]) merged.set(row.id, row) + for (const row of (byEntry.data ?? []) as unknown as LineRow[]) { + if (!merged.has(row.id)) merged.set(row.id, row) + } + data = Array.from(merged.values()) + .sort((a, b) => { + const ad = a.journal_entries.entry_date + const bd = b.journal_entries.entry_date + if (ad !== bd) return ad < bd ? 1 : -1 + const av = a.journal_entries.voucher_number + const bv = b.journal_entries.voucher_number + if (av !== bv) return bv - av + return a.sort_order - b.sort_order + }) + .slice(0, limit) + + // Honest distinct-row count among what we fetched. If a leg hit its + // window cap, more distinct matches may exist; `legCapHit` carries + // that signal downstream so `truncated` isn't faked false. + dbMatched = merged.size + legCapHit = + (byLine.data?.length ?? 0) >= legLimit || + (byEntry.data?.length ?? 0) >= legLimit + } else { + const res = await applyOrderAndLimit(buildBaseQuery()) + if (res.error) { + log.warn('query_journal failed', { companyId, userId, error: res.error.message }) + throw new Error('Database error while running journal query') + } + data = (res.data ?? []) as unknown as LineRow[] + dbMatched = res.count ?? data.length + } + // Apply amount filter post-fetch — PostgREST can't OR an abs(debit) >= n // with abs(credit) >= n cleanly. Lines are debit XOR credit, so checking // max(debit, credit) works. const amountMin = args.amount_min as number | undefined const amountMax = args.amount_max as number | undefined const amountFilterApplied = typeof amountMin === 'number' || typeof amountMax === 'number' - const filtered = (data ?? []).filter((row) => { - const r = row as unknown as LineRow + const filtered = data.filter((r) => { const lineAmount = Math.max(Number(r.debit_amount) || 0, Number(r.credit_amount) || 0) if (typeof amountMin === 'number' && lineAmount < amountMin) return false if (typeof amountMax === 'number' && lineAmount > amountMax) return false return true - }) as unknown as LineRow[] + }) // Compute totals on the fetched-and-filtered set. Note: when truncated, // these are totals of the returned slice, not the full match. The @@ -4161,11 +4247,10 @@ export const tools: McpTool[] = [ // result, and surface the pre-filter count + a flag separately so an // agent can still tell the DB matched more (it just didn't pass the // amount predicate). - const dbMatched = count ?? (data ?? []).length const total_lines = amountFilterApplied ? lines.length : dbMatched const truncated = amountFilterApplied - ? (data ?? []).length >= limit && lines.length === limit - : dbMatched > lines.length + ? data.length >= limit && lines.length === limit + : dbMatched > lines.length || legCapHit return { lines, truncated,