fix(mcp): link_document_to_voucher marks the inbox item handled; document lists stop misleading agents (#2170)

Three MCP feedback reports (seq 265062, 288474, 288577; three companies) hit the same hole: link_document_to_voucher attaches the document to the verifikat but never stamps the inbox item it came from, and both inbox read surfaces derive "handled" from the inbox row's own link columns, never from document_attachments.journal_entry_id. So an attached document stayed "unprocessed" forever: agents re-saw it as missing underlag (one reporter paged 750 rows to find the ~15 real ones), and one user read the five leftover rows as duplicates and was about to delete the only copies of underlag sitting on posted verifikat. - commitLinkDocumentToVoucher and the bulk twin now stamp invoice_inbox_items.created_journal_entry_id, keyed on document_id, CAS on both link columns, 23505 tolerated (samlingsverifikat). Same shape as the create_voucher + inbox_item_id stamp; best-effort so inbox bookkeeping never rolls back a committed link. - gnubok_list_unmatched_documents returns file_name (same embed list_inbox_items uses) and the extraction's page coverage, so an agent can tell "no total on this document" from "we read 3 of 38 pages" and does not have to fetch each document to learn what it is (seq 265062, 288574). - gnubok_list_transactions_without_documents no longer echoes the column default "uncategorized" on rows that are booked by construction: list_uncategorized_transactions uses the same word for "no journal entry yet", and an agent read the label and tried to re-book an already-booked share-capital deposit (seq 288574). No tools/list payload change: the unmatched-documents item schema is untyped and the category field already allowed null. Claude-Session: https://claude.ai/code/session_013yw62FMXGSzo6icFDiBwP3 Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-04 08:57:53 +02:00
parent bac9e01e2d
commit 01d903d3a3
5 changed files with 181 additions and 4 deletions
@@ -76,6 +76,26 @@ describe('gnubok_list_transactions_without_documents', () => {
    expect(result.transactions[0].journal_entry_id).toBe('je-1')
  })

+  it('never echoes the column default "uncategorized" on rows that are booked by construction', async () => {
+    // transactions.category keeps its default when a row is booked through a
+    // manual voucher + link (link-journal-entry.ts never writes category), and
+    // gnubok_list_uncategorized_transactions uses the same word to mean "no
+    // journal entry yet". Feedback seq 288574: an agent read the label and
+    // tried to book an already-booked share-capital deposit (A-2, 1930/2081).
+    const { supabase, enqueue } = createQueuedMockSupabase()
+    enqueue(envelope([
+      { ...row('t1', 'je-1'), category: 'uncategorized' },
+      { ...row('t2', 'je-2'), category: 'office_supplies' },
+    ], 2))
+
+    const result = (await tool.execute({}, 'company-1', 'user-1', supabase as never)) as {
+      transactions: Array<{ transaction_id: string; category: string | null; journal_entry_id: string }>
+    }
+
+    expect(result.transactions[0]).toMatchObject({ transaction_id: 't1', category: null, journal_entry_id: 'je-1' })
+    expect(result.transactions[1]).toMatchObject({ transaction_id: 't2', category: 'office_supplies' })
+  })
+
  it('returns empty result when nothing matches', async () => {
    const { supabase, enqueue } = createQueuedMockSupabase()
    enqueue(envelope([], 0))
@@ -77,10 +77,12 @@ describe('gnubok_list_unmatched_documents', () => {
          email_subject: null,
          email_received_at: null,
          created_at: '2026-06-01T00:00:00Z',
+          document_attachments: { file_name: 'V82_2025-08-05_DNB-Finans.pdf' },
          extracted_data: {
            supplier: { name: 'DNB Finans', orgNumber: '5164060161' },
            invoice: { currency: 'SEK', invoiceDate: '2025-08-05', paymentReference: '9581810307' },
            totals: { total: 13428 },
+            pages: { total: 38, analyzed: 3 },
          },
        },
        {
@@ -100,7 +102,13 @@ describe('gnubok_list_unmatched_documents', () => {
    enqueue({ data: [{ document_id: 'doc-2' }], error: null })

    const result = (await tool.execute({ limit: 20 }, 'company-1', 'user-1', supabase as never)) as {
-      items: Array<{ inbox_item_id: string; vendor_name: string | null; amount: number | null }>
+      items: Array<{
+        inbox_item_id: string
+        vendor_name: string | null
+        amount: number | null
+        file_name: string | null
+        pages: { total: number; analyzed: number } | null
+      }>
      count: number
    }

@@ -108,6 +116,39 @@ describe('gnubok_list_unmatched_documents', () => {
    expect(result.items[0].inbox_item_id).toBe('inbox-1')
    expect(result.items[0].vendor_name).toBe('DNB Finans')
    expect(result.items[0].amount).toBe(13428)
+    // The original file name and the page coverage of the extraction travel
+    // with the row (feedback seq 265062 / 288574): the agent must not have to
+    // fetch every document to learn what it is, and amount: null on a
+    // sliced 38-page PDF is not a fact about the file.
+    expect(result.items[0].file_name).toBe('V82_2025-08-05_DNB-Finans.pdf')
+    expect(result.items[0].pages).toEqual({ total: 38, analyzed: 3 })
+  })
+
+  it('tolerates the embed coming back as an array and a document with no page info', async () => {
+    const { supabase, enqueue } = createQueuedMockSupabase()
+    enqueue({
+      data: [
+        {
+          id: 'inbox-3',
+          document_id: 'doc-3',
+          source: 'email',
+          email_from: 'a@b.se',
+          email_subject: 'Kvitto',
+          email_received_at: '2026-06-02T00:00:00Z',
+          created_at: '2026-06-02T00:00:00Z',
+          document_attachments: [{ file_name: 'kvitto.jpg' }],
+          extracted_data: { totals: { total: 99 } },
+        },
+      ],
+      error: null,
+    })
+    enqueue({ data: [], error: null })
+
+    const result = (await tool.execute({ limit: 20 }, 'company-1', 'user-1', supabase as never)) as {
+      items: Array<{ file_name: string | null; pages: unknown }>
+    }
+    expect(result.items[0].file_name).toBe('kvitto.jpg')
+    expect(result.items[0].pages).toBeNull()
  })

  it('returns an empty result when the inbox query has nothing pending', async () => {
@@ -5536,7 +5536,18 @@ export const tools: McpTool[] = [
        throw new Error(`transactions_without_documents failed: ${result?.code ?? 'unknown error'}`)
      }

-      const rows = result.transactions ?? []
+      // Every row here is booked by construction (the RPC joins
+      // journal_entries), but transactions.category keeps its column default
+      // 'uncategorized' when a row is booked by a manual voucher plus link
+      // (lib/transactions/link-journal-entry.ts never writes category).
+      // gnubok_list_uncategorized_transactions uses that same word to mean
+      // "no journal entry yet", so echoing it here made agents try to book an
+      // already-booked deposit (feedback seq 288574). null = no category
+      // label; the journal_entry_id is the booking truth.
+      const rows = (result.transactions ?? []).map((row) => {
+        const r = row as Record<string, unknown>
+        return r.category === 'uncategorized' ? { ...r, category: null } : r
+      })
      const total = result.total_count ?? 0
      return { transactions: rows, ...pageTail(rows, total, offset) }
    },
@@ -13488,7 +13499,7 @@ export const tools: McpTool[] = [
      const fetchSize = limit * 2
      let inboxQuery = supabase
        .from('invoice_inbox_items')
-        .select('id, document_id, source, email_from, email_subject, email_received_at, extracted_data, created_at')
+        .select('id, document_id, source, email_from, email_subject, email_received_at, extracted_data, created_at, document_attachments(file_name)')
        .eq('company_id', companyId)
        .not('document_id', 'is', null)
        .is('created_supplier_invoice_id', null)
@@ -13533,8 +13544,17 @@ export const tools: McpTool[] = [
          let currency: string | null = null
          let invoiceDate: string | null = null
          let paymentReference: string | null = null
+          // Page coverage of the extraction (set only when the PDF was sliced,
+          // extensions/general/invoice-inbox/lib/upload-and-extract.ts). Lets
+          // the agent tell "this document has no total" from "we read 3 of 38
+          // pages" instead of reading amount: null as a fact about the file.
+          let pages: { total: number; analyzed: number } | null = null

          if (extracted) {
+            const pageInfo = extracted.pages as { total?: unknown; analyzed?: unknown } | undefined
+            if (typeof pageInfo?.total === 'number' && typeof pageInfo?.analyzed === 'number') {
+              pages = { total: pageInfo.total, analyzed: pageInfo.analyzed }
+            }
            const supplier = extracted.supplier as Record<string, unknown> | undefined
            const invoice = extracted.invoice as Record<string, unknown> | undefined
            const totals = extracted.totals as Record<string, unknown> | undefined
@@ -13551,9 +13571,22 @@ export const tools: McpTool[] = [
            paymentReference = (invoice?.paymentReference as string) || null
          }

+          // Original file name, same embed gnubok_list_inbox_items uses: the
+          // archive file name was a better date signal than the OCR'd
+          // invoice_date in every case one reporter checked (feedback seq
+          // 265062), and without it the agent must fetch each document just
+          // to learn what it is.
+          const attachment = item.document_attachments as
+            | { file_name?: string | null }
+            | Array<{ file_name?: string | null }>
+            | null
+            | undefined
+          const fileName = (Array.isArray(attachment) ? attachment[0]?.file_name : attachment?.file_name) ?? null
+
          return {
            inbox_item_id: item.id,
            document_id: item.document_id,
+            file_name: fileName,
            source: item.source,
            created_at: item.created_at,
            email_from: item.email_from,
@@ -13565,6 +13598,7 @@ export const tools: McpTool[] = [
            currency,
            invoice_date: invoiceDate,
            payment_reference: paymentReference,
+            pages,
          }
        })

@@ -1461,6 +1461,7 @@ describe('commitPendingOperation: link_document_to_voucher', () => {
      data: { id: 'doc-1', file_name: 'kvitto.pdf', journal_entry_id: 'je-1', journal_entry_line_id: null },
      error: null,
    })                                                                           // linkToJournalEntry: doc update
+    enqueue({ data: null, error: null })                                         // inbox stamp (best-effort)
    enqueue({ data: null, error: null })                                         // dispatcher commit update

    const result = await commitPendingOperation(
@@ -1470,7 +1471,7 @@ describe('commitPendingOperation: link_document_to_voucher', () => {
  })

  it('happy path: links doc to verifikation with no prior journal_entry_id', async () => {
-    const { supabase, enqueue } = createQueuedMockSupabase()
+    const { supabase, enqueue, calls, findCall } = createQueuedMockSupabase()
    enqueue({ data: { id: 'op-1' }, error: null })                              // CAS claim
    enqueue({ data: { id: 'doc-1', journal_entry_id: null }, error: null })     // doc fetch
    enqueue({ data: { id: 'je-1' }, error: null })                              // linkToJournalEntry: JE ownership
@@ -1478,6 +1479,7 @@ describe('commitPendingOperation: link_document_to_voucher', () => {
      data: { id: 'doc-1', file_name: 'faktura.pdf', journal_entry_id: 'je-1', journal_entry_line_id: null },
      error: null,
    })                                                                           // linkToJournalEntry: doc update
+    enqueue({ data: null, error: null })                                         // inbox stamp (best-effort)
    enqueue({ data: null, error: null })                                         // dispatcher commit update

    const result = await commitPendingOperation(
@@ -1488,6 +1490,39 @@ describe('commitPendingOperation: link_document_to_voucher', () => {
      document_id: 'doc-1',
      journal_entry_id: 'je-1',
    })
+    // The inbox item the document came from is stamped as handled, keyed on
+    // document_id and CAS-guarded on both link columns: otherwise
+    // list_inbox_items / list_unmatched_documents keep listing an attached
+    // document as unprocessed forever.
+    expect(findCall('invoice_inbox_items', 'update')).toEqual([{ created_journal_entry_id: 'je-1' }])
+    const inboxFilters = calls
+      .filter((c) => c.table === 'invoice_inbox_items' && (c.method === 'eq' || c.method === 'is'))
+      .map((c) => c.args)
+    expect(inboxFilters).toEqual([
+      ['document_id', 'doc-1'],
+      ['company_id', 'company-1'],
+      ['created_journal_entry_id', null],
+      ['created_supplier_invoice_id', null],
+    ])
+  })
+
+  it('inbox stamp is best-effort: a failed stamp never fails the committed link', async () => {
+    const { supabase, enqueue } = createQueuedMockSupabase()
+    enqueue({ data: { id: 'op-1' }, error: null })                              // CAS claim
+    enqueue({ data: { id: 'doc-1', journal_entry_id: null }, error: null })     // doc fetch
+    enqueue({ data: { id: 'je-1' }, error: null })                              // linkToJournalEntry: JE ownership
+    enqueue({
+      data: { id: 'doc-1', file_name: 'faktura.pdf', journal_entry_id: 'je-1', journal_entry_line_id: null },
+      error: null,
+    })                                                                           // linkToJournalEntry: doc update
+    enqueue({ data: null, error: { code: '23505', message: 'duplicate key value' } }) // inbox stamp: samlingsverifikat already claimed
+    enqueue({ data: null, error: null })                                         // dispatcher commit update
+
+    const result = await commitPendingOperation(
+      supabase as never, 'user-1', 'company-1', makePendingOp(baseOp),
+    )
+    expect(result.status).toBe('committed')
+    expect(result.data).toMatchObject({ document_id: 'doc-1', journal_entry_id: 'je-1' })
  })

  it('auto-rejects 409 when linkToJournalEntry throws a period-lock error', async () => {
@@ -1532,6 +1567,7 @@ describe('commitPendingOperation: link_documents_to_vouchers', () => {
      data: { id: 'doc-1', file_name: 'kvitto1.pdf', journal_entry_id: 'je-1', journal_entry_line_id: null },
      error: null,
    })                                                                       // linkToJournalEntry: doc update
+    enqueue({ data: null, error: null })                                   // inbox stamp (best-effort)
    // row 2: doc-2 -> je-2
    enqueue({ data: { id: 'doc-2', journal_entry_id: null }, error: null })  // doc fetch
    enqueue({ data: { id: 'je-2' }, error: null })                          // linkToJournalEntry: JE ownership
@@ -1539,6 +1575,7 @@ describe('commitPendingOperation: link_documents_to_vouchers', () => {
      data: { id: 'doc-2', file_name: 'kvitto2.pdf', journal_entry_id: 'je-2', journal_entry_line_id: null },
      error: null,
    })                                                                       // linkToJournalEntry: doc update
+    enqueue({ data: null, error: null })                                   // inbox stamp (best-effort)
    enqueue({ data: null, error: null })                                    // dispatcher commit update

    const result = await commitPendingOperation(
@@ -1563,6 +1600,7 @@ describe('commitPendingOperation: link_documents_to_vouchers', () => {
      data: { id: 'doc-2', file_name: 'kvitto2.pdf', journal_entry_id: 'je-2', journal_entry_line_id: null },
      error: null,
    })                                                                      // linkToJournalEntry: doc update
+    enqueue({ data: null, error: null })                                  // inbox stamp (best-effort)
    enqueue({ data: null, error: null })                                   // dispatcher commit update

    const result = await commitPendingOperation(
@@ -1592,6 +1630,7 @@ describe('commitPendingOperation: link_documents_to_vouchers', () => {
      data: { id: 'doc-2', file_name: 'kvitto2.pdf', journal_entry_id: 'je-2', journal_entry_line_id: null },
      error: null,
    })                                                                              // linkToJournalEntry: doc update
+    enqueue({ data: null, error: null })                                          // inbox stamp (best-effort)
    enqueue({ data: null, error: null })                                           // dispatcher commit update

    const result = await commitPendingOperation(
@@ -3999,6 +3999,47 @@ async function precheckDocumentLink(
  return { ok: true }
 }

+/**
+ * After a document is linked to a verifikat, stamp the inbox item it came
+ * from (if any) so the inbox reads "handled". Both inbox read surfaces derive
+ * "done" from the inbox row's own link columns, never from
+ * document_attachments.journal_entry_id: gnubok_list_inbox_items computes
+ * processed from matched_transaction_id / created_supplier_invoice_id /
+ * created_journal_entry_id, and gnubok_list_unmatched_documents filters on the
+ * same two nulls. Without this stamp a document attached through
+ * link_document_to_voucher stays "unprocessed" forever: agents re-see it as
+ * missing underlag, and one user read the five leftover rows as duplicates
+ * and nearly deleted the only copies of underlag sitting on posted verifikat
+ * (feedback 2026-08-24..26, three companies).
+ *
+ * Same shape as the create_voucher + inbox_item_id stamp: CAS on the null
+ * link columns so a concurrent claim stays a no-op, and unique_violation
+ * tolerated because the UNIQUE on created_journal_entry_id lets only one
+ * inbox item point at a samlingsverifikat. Best-effort by design: the link is
+ * already committed and inbox bookkeeping must not roll it back.
+ */
+async function stampInboxItemForLinkedDocument(
+  supabase: SupabaseClient,
+  companyId: string,
+  documentId: string,
+  journalEntryId: string,
+): Promise<void> {
+  const { error } = await supabase
+    .from('invoice_inbox_items')
+    .update({ created_journal_entry_id: journalEntryId })
+    .eq('document_id', documentId)
+    .eq('company_id', companyId)
+    .is('created_journal_entry_id', null)
+    .is('created_supplier_invoice_id', null)
+  if (error && error.code !== '23505') {
+    log.warn('Failed to mark inbox item handled after document link (link still committed)', {
+      documentId,
+      journalEntryId,
+      error: error.message,
+    })
+  }
+}
+
 /**
 * Shared failure mapping for both link executors. A locked period is the one
 * case worth its own sentence: it is recoverable by unlocking, unlike the rest,
@@ -4040,6 +4081,7 @@ async function commitLinkDocumentToVoucher(
      journalEntryId,
      journalEntryLineId,
    )
+    await stampInboxItemForLinkedDocument(supabase, companyId, documentId, journalEntryId)
    return {
      data: {
        document_id: updated.id,
@@ -4095,6 +4137,7 @@ async function commitLinkDocumentsToVouchers(
      const updated = await linkToJournalEntry(
        supabase, companyId, documentId, journalEntryId, link.journal_entry_line_id ?? undefined,
      )
+      await stampInboxItemForLinkedDocument(supabase, companyId, documentId, journalEntryId)
      linked.push({ document_id: updated.id, journal_entry_id: updated.journal_entry_id as string })
    } catch (err) {
      // Same mapping as the single executor, including its locked-period