fix(mcp): add true continuation to invoice list tools (#1327)

* fix(mcp): add true list continuation Signed-off-by: Emil <emilmattsson14@gmail.com> * test(mcp): cover pagination edge cases Signed-off-by: Emil <emilmattsson14@gmail.com> * fix(mcp): validate pagination offsets Signed-off-by: Emil <emilmattsson14@gmail.com> * fix(mcp): preserve continuation without counts Signed-off-by: Emil <emilmattsson14@gmail.com> --------- Signed-off-by: Emil <emilmattsson14@gmail.com>
2026-08-01 19:03:51 +02:00
parent 115147d304
commit 8a2498a987
3 changed files with 230 additions and 6 deletions
@@ -0,0 +1,196 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { eventBus } from '@/lib/events/bus'
+import { createQueuedMockSupabase } from '@/tests/helpers'
+import { tools } from '../server'
+
+const invoiceRow = (id: string) => ({
+  id,
+  invoice_number: 'INV-100',
+  status: 'sent',
+  customer_id: 'customer-1',
+  total: 1250,
+  currency: 'SEK',
+  invoice_date: '2026-08-01',
+  due_date: '2026-08-31',
+  document_type: 'invoice',
+  default_dimensions: null,
+  customers: { name: 'Example Customer AB' },
+})
+
+const scheduleRow = (id: string) => ({
+  id,
+  name: 'Monthly support',
+  status: 'active',
+  customer_id: 'customer-1',
+  day_of_month: 25,
+  send_hour: 9,
+  payment_terms_days: 30,
+  currency: 'SEK',
+  auto_send: false,
+  default_dimensions: null,
+  next_run_date: '2026-08-25',
+  last_run_at: null,
+  last_invoice_id: null,
+  last_run_warning: null,
+  generated_count: 0,
+  customer: { name: 'Example Customer AB' },
+  items: [],
+})
+
+const cases = [
+  {
+    name: 'gnubok_list_invoices',
+    table: 'invoices',
+    itemsKey: 'invoices',
+    primaryOrder: 'invoice_date',
+    row: invoiceRow,
+  },
+  {
+    name: 'gnubok_list_recurring_schedules',
+    table: 'recurring_invoice_schedules',
+    itemsKey: 'schedules',
+    primaryOrder: 'created_at',
+    row: scheduleRow,
+  },
+] as const
+
+beforeEach(() => {
+  vi.clearAllMocks()
+  eventBus.clear()
+})
+
+describe.each(cases)('$name continuation', ({ name, table, itemsKey, primaryOrder, row }) => {
+  const tool = tools.find((candidate) => candidate.name === name)!
+
+  it('advertises optional offset input and the complete pagination envelope', () => {
+    expect(tool).toBeDefined()
+
+    const input = tool.inputSchema as {
+      required?: string[]
+      properties: Record<string, { type?: string; minimum?: number }>
+    }
+    expect(input.properties.offset).toMatchObject({ type: 'integer', minimum: 0 })
+    expect(input.required ?? []).not.toContain('offset')
+
+    const output = tool.outputSchema as { required: string[]; properties: Record<string, unknown> }
+    expect(output.required).toEqual(expect.arrayContaining([itemsKey, 'count', 'total_count', 'has_more']))
+    expect(output.properties.next_offset).toBeDefined()
+  })
+
+  it('continues from offset and returns the next offset when more rows exist', async () => {
+    const { supabase, enqueue, findCall, findCalls } = createQueuedMockSupabase()
+    enqueue({ data: [row('row-5'), row('row-6')], error: null, count: 9 })
+
+    const result = (await tool.execute(
+      { limit: 2, offset: 4 },
+      'company-1',
+      'user-1',
+      supabase as never,
+    )) as Record<string, unknown>
+
+    expect(findCall(table, 'range')).toEqual([4, 6])
+    expect(findCalls(table, 'order')).toEqual([
+      [primaryOrder, { ascending: false }],
+      ['id', { ascending: false }],
+    ])
+    expect((result[itemsKey] as unknown[])).toHaveLength(2)
+    expect(result).toMatchObject({
+      count: 2,
+      total_count: 9,
+      has_more: true,
+      next_offset: 6,
+    })
+  })
+
+  it('preserves the first-page default and omits next_offset on the last page', async () => {
+    const { supabase, enqueue, findCall } = createQueuedMockSupabase()
+    enqueue({ data: [row('only-row')], error: null, count: 1 })
+
+    const result = (await tool.execute(
+      {},
+      'company-1',
+      'user-1',
+      supabase as never,
+    )) as Record<string, unknown>
+
+    expect(findCall(table, 'range')).toEqual([0, 50])
+    expect(result).toMatchObject({
+      count: 1,
+      total_count: 1,
+      has_more: false,
+    })
+    expect(result).not.toHaveProperty('next_offset')
+  })
+
+  it('returns an empty terminal page', async () => {
+    const { supabase, enqueue } = createQueuedMockSupabase()
+    enqueue({ data: [], error: null, count: 0 })
+
+    const result = (await tool.execute(
+      { limit: 2, offset: 4 },
+      'company-1',
+      'user-1',
+      supabase as never,
+    )) as Record<string, unknown>
+
+    expect(result[itemsKey]).toEqual([])
+    expect(result).toMatchObject({
+      count: 0,
+      total_count: 0,
+      has_more: false,
+    })
+    expect(result).not.toHaveProperty('next_offset')
+  })
+
+  it('normalizes fractional offsets for direct execution', async () => {
+    const { supabase, enqueue, findCall } = createQueuedMockSupabase()
+    enqueue({ data: [row('row-5')], error: null, count: 6 })
+
+    const result = (await tool.execute(
+      { limit: 2, offset: 4.9 },
+      'company-1',
+      'user-1',
+      supabase as never,
+    )) as Record<string, unknown>
+
+    expect(findCall(table, 'range')).toEqual([4, 6])
+    expect(result).toMatchObject({
+      count: 1,
+      total_count: 6,
+      has_more: true,
+      next_offset: 5,
+    })
+  })
+
+  it('uses a lookahead row when the exact count is unavailable', async () => {
+    const { supabase, enqueue } = createQueuedMockSupabase()
+    enqueue({ data: [row('row-5'), row('row-6'), row('lookahead-row')], error: null, count: null })
+
+    const result = (await tool.execute(
+      { limit: 2, offset: 4 },
+      'company-1',
+      'user-1',
+      supabase as never,
+    )) as Record<string, unknown>
+
+    expect(result[itemsKey]).toHaveLength(2)
+    expect(result).toMatchObject({
+      count: 2,
+      total_count: 7,
+      has_more: true,
+      next_offset: 6,
+    })
+  })
+
+  it('reports database errors', async () => {
+    const { supabase, enqueue } = createQueuedMockSupabase()
+    enqueue({ data: null, error: { message: 'connection reset' }, count: null })
+
+    await expect(tool.execute(
+      {},
+      'company-1',
+      'user-1',
+      supabase as never,
+    )).rejects.toThrow('Database error: connection reset')
+  })
+})
@@ -4588,6 +4588,7 @@ export const tools: McpTool[] = [
          description: 'Filter by invoice status',
        },
        limit: { type: 'number', description: 'Max results (default 50, max 100)' },
+        offset: { type: 'integer', minimum: 0, description: 'Number of results to skip for pagination (default 0)' },
      },
    },
    outputSchema: paginatedSchema('invoices', { type: 'object' }),
@@ -4599,6 +4600,7 @@ export const tools: McpTool[] = [
    },
    async execute(args, companyId, userId, supabase) {
      const limit = Math.min(Math.max(1, Number(args.limit) || 50), 100)
+      const offset = Math.max(0, Math.floor(Number(args.offset) || 0))
      const status = args.status as string | undefined

      let query = supabase
@@ -4612,11 +4614,13 @@ export const tools: McpTool[] = [

      const { data, error, count } = await query
        .order('invoice_date', { ascending: false })
-        .limit(limit)
+        .order('id', { ascending: false })
+        .range(offset, offset + limit)

      if (error) throw new Error(`Database error: ${error.message}`)

-      const invoices = (data ?? []).map((inv: Record<string, unknown>) => ({
+      const rows = data ?? []
+      const invoices = rows.slice(0, limit).map((inv: Record<string, unknown>) => ({
        id: inv.id,
        invoice_number: inv.invoice_number,
        status: inv.status,
@@ -4629,10 +4633,17 @@ export const tools: McpTool[] = [
        default_dimensions: inv.default_dimensions ?? {},
      }))

+      const hasMore = count == null
+        ? rows.length > limit
+        : offset + invoices.length < count
+      const total = count ?? offset + invoices.length + (hasMore ? 1 : 0)
+
      return {
        invoices,
        count: invoices.length,
-        total_count: count ?? invoices.length,
+        total_count: total,
+        has_more: hasMore,
+        ...(hasMore ? { next_offset: offset + invoices.length } : {}),
      }
    },
  },
@@ -15007,6 +15018,7 @@ export const tools: McpTool[] = [
          description: 'Filter by schedule status',
        },
        limit: { type: 'number', description: 'Max results (default 50, max 100)' },
+        offset: { type: 'integer', minimum: 0, description: 'Number of results to skip for pagination (default 0)' },
      },
    },
    outputSchema: paginatedSchema('schedules', {
@@ -15057,6 +15069,7 @@ export const tools: McpTool[] = [
    catalogVisibility: 'search',
    async execute(args, companyId, userId, supabase) {
      const limit = Math.min(Math.max(1, Number(args.limit) || 50), 100)
+      const offset = Math.max(0, Math.floor(Number(args.offset) || 0))
      const status = args.status as string | undefined

      let query = supabase
@@ -15073,11 +15086,13 @@ export const tools: McpTool[] = [

      const { data, error, count } = await query
        .order('created_at', { ascending: false })
-        .limit(limit)
+        .order('id', { ascending: false })
+        .range(offset, offset + limit)

      if (error) throw new Error(`Database error: ${error.message}`)

-      const schedules = (data ?? []).map((row: Record<string, unknown>) => {
+      const rows = data ?? []
+      const schedules = rows.slice(0, limit).map((row: Record<string, unknown>) => {
        const items = ((row.items as Array<Record<string, unknown>>) ?? [])
          .slice()
          .sort((a, b) => Number(a.sort_order) - Number(b.sort_order))
@@ -15113,10 +15128,17 @@ export const tools: McpTool[] = [
        }
      })

+      const hasMore = count == null
+        ? rows.length > limit
+        : offset + schedules.length < count
+      const total = count ?? offset + schedules.length + (hasMore ? 1 : 0)
+
      return {
        schedules,
        count: schedules.length,
-        total_count: count ?? schedules.length,
+        total_count: total,
+        has_more: hasMore,
+        ...(hasMore ? { next_offset: offset + schedules.length } : {}),
      }
    },
  },