fix(mcp): page list tools past PostgREST's silent 1000-row cap (#1572)
gnubok_list_accounts returned exactly 1000 rows for a full BAS 2026 chart (1290 accounts) with no truncation signal: PostgREST caps un-ranged selects at 1000. Wrap the query in fetchAllRows, paging on the unique account_number and re-sorting by sort_order in JS so the visible order is unchanged. Same fix for gnubok_list_customers, gnubok_list_suppliers and gnubok_list_articles (paged on id, re-sorted by name), the Accounted://chart-of-accounts resource, and the REST v1 accounts.list route. All output schemas and registry metadata unchanged. Also note on gnubok_audit_package download_url that the signed URL points at the Supabase storage host, so restricted-egress proxies may 403; offset the added prose by trimming the same tool's own descriptions to keep the tools/list payload under the 59K ceiling. Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Jakob Wennberg
Claude Fable 5
parent
36393b1f8d
commit
7ae3477b36
@@ -106,6 +106,54 @@ describe('GET /api/v1/companies/:companyId/accounts', () => {
|
||||
expect(body.data.accounts[0].account_number).toBe('1930')
|
||||
})
|
||||
|
||||
it('returns all rows when the chart exceeds the 1000-row PostgREST page', async () => {
|
||||
const makeAccount = (n: number, sortOrder: number | null = n) => ({
|
||||
account_number: String(n),
|
||||
account_name: `Konto ${n}`,
|
||||
account_class: Math.floor(n / 1000),
|
||||
account_group: String(n).slice(0, 2),
|
||||
account_type: 'asset',
|
||||
normal_balance: 'debit',
|
||||
is_system_account: false,
|
||||
is_active: true,
|
||||
description: null,
|
||||
default_vat_code: null,
|
||||
sru_code: null,
|
||||
sort_order: sortOrder,
|
||||
})
|
||||
// Page 1: exactly 1000 rows (forces a second range request). Account 1000
|
||||
// gets sort_order 99999 and account 9998 (page 2) a null sort_order, so the
|
||||
// response order proves the sort_order re-sort with nulls last.
|
||||
const page1 = [
|
||||
makeAccount(1000, 99999),
|
||||
...Array.from({ length: 999 }, (_, i) => makeAccount(1001 + i)),
|
||||
]
|
||||
const page2 = [
|
||||
...Array.from({ length: 289 }, (_, i) => makeAccount(2000 + i)),
|
||||
makeAccount(9998, null),
|
||||
]
|
||||
mockServiceClient.mockReturnValue(
|
||||
makeFlexibleSupabase({
|
||||
company_members: { data: { company_id: COMPANY_ID, role: 'owner' }, error: null },
|
||||
chart_of_accounts: [
|
||||
{ data: page1, error: null },
|
||||
{ data: page2, error: null },
|
||||
],
|
||||
}),
|
||||
)
|
||||
const res = await listAccounts(
|
||||
makeRequest(`https://x.test/api/v1/companies/${COMPANY_ID}/accounts`),
|
||||
{ params: Promise.resolve({ companyId: COMPANY_ID }) },
|
||||
)
|
||||
expect(res.status).toBe(200)
|
||||
const body = await res.json()
|
||||
expect(body.data.accounts).toHaveLength(1290)
|
||||
expect(body.data.accounts[0].account_number).toBe('1001')
|
||||
// sort_order 99999 lands second-to-last; null sort_order lands last.
|
||||
expect(body.data.accounts[1288].account_number).toBe('1000')
|
||||
expect(body.data.accounts[1289].account_number).toBe('9998')
|
||||
})
|
||||
|
||||
it('rejects invalid class filter', async () => {
|
||||
mockServiceClient.mockReturnValue(
|
||||
makeFlexibleSupabase({
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
* sort_order: agents can render the BAS hierarchy directly from this.
|
||||
*/
|
||||
import { z } from 'zod'
|
||||
import { fetchAllRows } from '@/lib/supabase/fetch-all'
|
||||
import { ok } from '@/lib/api/v1/response'
|
||||
import { registerEndpoint, dataEnvelope } from '@/lib/api/v1/registry'
|
||||
import { withApiV1 } from '@/lib/api/v1/with-api-v1'
|
||||
@@ -103,17 +104,43 @@ export const GET = withApiV1<{ params: Promise<{ companyId: string }> }>(
|
||||
const f = parsed.data
|
||||
const activeOnly = f.active !== 'false'
|
||||
|
||||
let query = ctx.supabase
|
||||
.from('chart_of_accounts')
|
||||
.select(ACCOUNT_COLUMNS)
|
||||
.eq('company_id', ctx.companyId!)
|
||||
.order('sort_order', { ascending: true })
|
||||
// Paginated (fetchAllRows): PostgREST silently caps un-ranged selects at
|
||||
// 1000 rows and a full BAS 2026 chart holds ~1290 accounts. Paging is on
|
||||
// the unique account_number (fetchAllRows ordering invariant); the rows
|
||||
// are re-sorted by sort_order afterwards to keep the documented response
|
||||
// order (the BAS canonical sequence).
|
||||
type AccountRow = {
|
||||
account_number: string
|
||||
sort_order: number | null
|
||||
[key: string]: unknown
|
||||
}
|
||||
let accounts: AccountRow[]
|
||||
try {
|
||||
accounts = await fetchAllRows<AccountRow>(({ from, to }) => {
|
||||
let query = ctx.supabase
|
||||
.from('chart_of_accounts')
|
||||
.select(ACCOUNT_COLUMNS)
|
||||
.eq('company_id', ctx.companyId!)
|
||||
if (activeOnly) query = query.eq('is_active', true)
|
||||
if (f.class) query = query.eq('account_class', parseInt(f.class, 10))
|
||||
// The concatenated ACCOUNT_COLUMNS string defeats supabase-js's
|
||||
// template-literal column parser, so the row type is asserted here.
|
||||
return query.order('account_number', { ascending: true }).range(from, to) as unknown as PromiseLike<{
|
||||
data: AccountRow[] | null
|
||||
error: { message: string } | null
|
||||
}>
|
||||
})
|
||||
} catch (error) {
|
||||
return v1ErrorResponse(error, ctx.log, { requestId: ctx.requestId })
|
||||
}
|
||||
|
||||
if (activeOnly) query = query.eq('is_active', true)
|
||||
if (f.class) query = query.eq('account_class', parseInt(f.class, 10))
|
||||
|
||||
const { data, error } = await query
|
||||
if (error) return v1ErrorResponse(error, ctx.log, { requestId: ctx.requestId })
|
||||
return ok({ accounts: data ?? [] }, { requestId: ctx.requestId })
|
||||
// Postgres ordered by sort_order ascending with nulls last; keep that
|
||||
// visible order, tie-breaking on account_number for determinism.
|
||||
accounts.sort(
|
||||
(a, b) =>
|
||||
(a.sort_order ?? Number.MAX_SAFE_INTEGER) - (b.sort_order ?? Number.MAX_SAFE_INTEGER) ||
|
||||
a.account_number.localeCompare(b.account_number),
|
||||
)
|
||||
return ok({ accounts }, { requestId: ctx.requestId })
|
||||
},
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user