fix(mcp): unblock query_journal text search across line + entry descriptions (#605)
* 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
co-authored by
Claude Opus 4.7
Claude Opus 4.7
parent
7eb8715417
commit
6629964780
@@ -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<Array<{ column: string; value: unknown }>> = []
|
||||
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/)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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 = <T extends ReturnType<typeof buildBaseQuery>>(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<string, LineRow>()
|
||||
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,
|
||||
|
||||
Reference in New Issue
Block a user